apache module class order issue with ENC

[treydock]

I'm attempting to use puppetlabs-apache with all my classes defined via Foreman (1.5.1) with Puppet 3.4.3.  Servers are all CentOS 6.5.

With puppetlabs-apache-1.0.1 when I applied both the apache class and apache::mod::ssl class in Foreman I received this error on the server:

Error: Could not retrieve catalog from remote server: Error 400 on SERVER: undefined method `>=' for :undef:Symbol at /etc/puppet/environments/production/modules/apache/manifests/mod/ssl.pp:36 on node web01.brazos.tamu.edu

Warning: Not using cache on failed catalog

Error: Could not retrieve catalog; skipping run

I found the commit that changed how "apache_version" was handled and deployed that and then I get:

Error: Could not retrieve catalog from remote server: Error 400 on SERVER: Failed to parse template apache/mod/ssl.conf.erb:

  Filepath: /usr/lib/ruby/site_ruby/1.8/puppet/util/package.rb

  Line: 4

  Detail: private method `scan' called for nil:NilClass

 at /etc/puppet/environments/production/modules/apache/manifests/mod/ssl.pp:51 on node web01.brazos.tamu.edu

Warning: Not using cache on failed catalog

Error: Could not retrieve catalog; skipping run

This time the "@apache_version" variable in the template is being passed the to versoncmp function, producing that error.

I don't know if this is a symptom of using an ENC, or a bug in the apache module, but the class parameters in apache::mod::ssl reference the apache class and despite the class being defined, the variables are all 'undef'.

I also ran into this same issue when I applied the apache::mod::wsgi class:

Error: Could not retrieve catalog from remote server: Error 400 on SERVER: Invalid relationship: File[wsgi.conf] { before => File[undef] }, because File[undef] doesn't seem to be in the catalog

Warning: Not using cache on failed catalog

Error: Could not retrieve catalog; skipping run

In that case the fix was easier, just add "include ::apache" to the top of the class.

Thanks

- Trey

[jcbollinger]

On Sunday, June 22, 2014 12:43:20 AM UTC-5, treydock wrote:

I'm attempting to use puppetlabs-apache with all my classes defined via Foreman (1.5.1) with Puppet 3.4.3.  Servers are all CentOS 6.5.

With puppetlabs-apache-1.0.1 when I applied both the apache class and apache::mod::ssl class in Foreman I received this error on the server:

Error: Could not retrieve catalog from remote server: Error 400 on SERVER: undefined method `>=' for :undef:Symbol at /etc/puppet/environments/production/modules/apache/manifests/mod/ssl.pp:36 on node web01.brazos.tamu.edu

Warning: Not using cache on failed catalog

Error: Could not retrieve catalog; skipping run

That error is simply complaining that one of the items being compared (presumably the $apache_version variable) was not numeric.  Note in particular that '2.2.0' is not numeric, whereas '2.2' is.

 

I found the commit that changed how "apache_version" was handled and deployed that and then I get:

Version numbers simply should not be compared via numeric comparison operators, so fixing the module to avoid that is certainly the right thing to do.  I have no clue what change you actually made, however.

 

Error: Could not retrieve catalog from remote server: Error 400 on SERVER: Failed to parse template apache/mod/ssl.conf.erb:

  Filepath: /usr/lib/ruby/site_ruby/1.8/puppet/util/package.rb

  Line: 4

  Detail: private method `scan' called for nil:NilClass

 at /etc/puppet/environments/production/modules/apache/manifests/mod/ssl.pp:51 on node web01.brazos.tamu.edu

Warning: Not using cache on failed catalog

Error: Could not retrieve catalog; skipping run

This time the "@apache_version" variable in the template is being passed the to versoncmp function, producing that error.

That seems to be saying that the $apache_version variable does not exist / has no value in the context where the template is being evaluated.  Did you run the module's tests after modifying it?  (And before?)  I'm inclined to think you broke it (worse than it was already).

 

I don't know if this is a symptom of using an ENC

Not in a general sense, no, but possibly this particular ENC or the particular way you are using it.

 

, or a bug in the apache module

There was a bug, now there's a different one.

 

, but the class parameters in apache::mod::ssl reference the apache class and despite the class being defined, the variables are all 'undef'.

That sounds like an evaluation-order issue.  Class 'apache::mod::ssl' is depending on class 'apache' to be evaluated first.  If you are relying exclusively on Hiera for data binding (i.e. not Foreman) then it is safe to address that issue by putting "include 'apache'" at the top of the body of 'apache::mod::ssl'.  PL cannot do that in the stock version, however, because it's not safe when class parameters are bound to 'apache' via ENC or resource-like declarations.

 

I also ran into this same issue when I applied the apache::mod::wsgi class:

Error: Could not retrieve catalog from remote server: Error 400 on SERVER: Invalid relationship: File[wsgi.conf] { before => File[undef] }, because File[undef] doesn't seem to be in the catalog

Warning: Not using cache on failed catalog

Error: Could not retrieve catalog; skipping run

In that case the fix was easier, just add "include ::apache" to the top of the class.

It sounds like the problem there as another evaluation-order issue, just like in apache::mod::ssl. If that fix worked for mod wsdl, then it should work also for mod ssl.  Did you try that?

John

[Trey Dockendorf]

I was a bit vague in my report initially, will try to clarify.

The error about "undefined method >= for :undef" is due to using
version 1.0.1 of the module which did not have the fix for
MODULES-910. The fact the value was :undef is the other issue.

The error "private method scan called on nil:NilClass" is indeed due
to the $apache_version being undefined, which should not be the case.
The bug I ran into on the stable version of the module would not have
occurred because if that value would have been correctly pulled from
$::apache::apache_version, the value would be 2.2 (CentOS 6.5).

I'm using only Foreman, no Hiera at this time. Here is the relevant
lines outputted by Foreman:

---
classes:
apache:
default_ssl_vhost: true
default_vhost: true
purge_configs: false
server_signature: 'Off'
server_tokens: Prod
trace_enable: 'On'
apache::mod::ssl:
apache::mod::wsgi:
wsgi_socket_prefix: /var/run/wsgi

Using puppetlabs-apache-1.0.1 and the above ENC output, I get the following

Error: Could not retrieve catalog from remote server: Error 400 on
SERVER: undefined method `>=' for :undef:Symbol at

/etc/puppet/environments/production/modules/apache/manifests/mod/ssl.pp:35

on node web01.brazos.tamu.edu
Warning: Not using cache on failed catalog
Error: Could not retrieve catalog; skipping run

(Yes I'm editing production by hand, but this infrastructure is still
non-production :-) )

This change was on the code from puppetlabs-apache-1.0.1 pulled from the forge.

--- production/modules/apache/manifests/mod/ssl.pp.orig 2014-06-24
09:36:57.085572966 -0500
+++ production/modules/apache/manifests/mod/ssl.pp 2014-06-24
09:38:28.356519037 -0500
@@ -1,8 +1,12 @@
class apache::mod::ssl (
$ssl_compression = false,
$ssl_options = [ 'StdEnvVars' ],
- $apache_version = $::apache::apache_version,
) {
+
+ include ::apache
+
+ $apache_version = $::apache::apache_version
+
$session_cache = $::osfamily ? {
'debian' => '${APACHE_RUN_DIR}/ssl_scache(512000)',
'redhat' => '/var/cache/mod_ssl/scache(512000)',

The change above with the same ENC output results in no error.

Here's where things get interesting. If I change my ENC output to
provide no parameters for apache::mod::wsgi, then I end up with all
variables using the $::apache:: reference being 'undef'.

ENC:

---
classes:
apache:
default_ssl_vhost: true
default_vhost: true
purge_configs: false
server_signature: 'Off'
server_tokens: Prod
trace_enable: 'On'
apache::mod::ssl:
apache::mod::wsgi:

Error:

Error: Could not retrieve catalog from remote server: Error 400 on
SERVER: Invalid relationship: File[wsgi.conf] { before => File[undef]
}, because File[undef] doesn't seem to be in the catalog
Warning: Not using cache on failed catalog
Error: Could not retrieve catalog; skipping run

That seems like a bug in how Puppet is handling classes defined from an ENC.

Finally, if I go back to stock puppetlabs-apache-1.0.1 and define a
parameter for apache::mode::ssl ...

ENC:

---
classes:
apache:
default_ssl_vhost: true
default_vhost: true
purge_configs: false
server_signature: 'Off'
server_tokens: Prod
trace_enable: 'On'
apache::mod::ssl:
ssl_options:
- StdEnvVars
apache::mod::wsgi:
wsgi_socket_prefix: /var/run/wsgi

NO ERRORS.

Thoughts? It seems as though a class defined by an ENC with no
parameters is treated differently than a class defined with
parameters. I'd expect and thought that a class with no parameters
was called in a similar was as doing "class { 'apache::mod::ssl': }".
Should this be filed as a bug against Puppet?

Thanks,
- Trey

> --
> You received this message because you are subscribed to a topic in the
> Google Groups "Puppet Users" group.
> To unsubscribe from this topic, visit
> https://groups.google.com/d/topic/puppet-users/vQRNCDQacOc/unsubscribe.
> To unsubscribe from this group and all its topics, send an email to
> puppet-users...@googlegroups.com.
> To view this discussion on the web visit
> https://groups.google.com/d/msgid/puppet-users/88895d38-8735-414b-9b53-cec03b8e8166%40googlegroups.com.
>
> For more options, visit https://groups.google.com/d/optout.

[jcbollinger]

On Tuesday, June 24, 2014 9:49:19 AM UTC-5, treydock wrote:

I was a bit vague in my report initially, will try to clarify.

The error about "undefined method >= for :undef" is due to using
version 1.0.1 of the module which did not have the fix for
MODULES-910.  The fact the value was :undef is the other issue.

The error "private method scan called on nil:NilClass" is indeed due
to the $apache_version being undefined, which should not be the case.
The bug I ran into on the stable version of the module would not have
occurred because if that value would have been correctly pulled from
$::apache::apache_version, the value would be 2.2 (CentOS 6.5).

You say "correctly" as if Puppet were in some manner behaving wrongly.  I see no reason to think so.  The module -- especially the version you started with -- suffers from some evaluation-order issues that limit how it can safely be used.

 

I'm using only Foreman, no Hiera at this time.  Here is the relevant
lines outputted by Foreman:

---
classes:
  apache:
    default_ssl_vhost: true
    default_vhost: true
    purge_configs: false
    server_signature: 'Off'
    server_tokens: Prod
    trace_enable: 'On'
  apache::mod::ssl:
  apache::mod::wsgi:
    wsgi_socket_prefix: /var/run/wsgi

Using puppetlabs-apache-1.0.1 and the above ENC output, I get the following

Error: Could not retrieve catalog from remote server: Error 400 on
SERVER: undefined method `>=' for :undef:Symbol at
/etc/puppet/environments/production/modules/apache/manifests/mod/ssl.pp:35
on node web01.brazos.tamu.edu
Warning: Not using cache on failed catalog
Error: Could not retrieve catalog; skipping run

As I said before, I'm fairly certain that's an evaluation-order issue.  Puppet is evaluating class 'apache::mod::ssl' before it evaluates class 'apache', which it is allowed to do given the manifests of that version of the module.

 

(Yes I'm editing production by hand, but this infrastructure is still
non-production :-) )

This change was on the code from puppetlabs-apache-1.0.1 pulled from the forge.

--- production/modules/apache/manifests/mod/ssl.pp.orig 2014-06-24
09:36:57.085572966 -0500
+++ production/modules/apache/manifests/mod/ssl.pp      2014-06-24
09:38:28.356519037 -0500
@@ -1,8 +1,12 @@
 class apache::mod::ssl (
   $ssl_compression = false,
   $ssl_options     = [ 'StdEnvVars' ],
-  $apache_version  = $::apache::apache_version,
 ) {
+
+  include ::apache
+
+  $apache_version  = $::apache::apache_version
+
   $session_cache = $::osfamily ? {
     'debian'  => '${APACHE_RUN_DIR}/ssl_scache(512000)',
     'redhat'  => '/var/cache/mod_ssl/scache(512000)',

The change above with the same ENC output results in no error.

And that's better from an evaluation-order perspective, because "include ::apache" at the beginning of the class body forces class '::apache' to be evaluated then if it has not been evaluated already.  It's still not perfect, however, because it will interact poorly with a resource-like declaration of class '::apache' appearing elsewhere.  In that sense, there is still an evaluation-order issue.

I am uncertain whether an ENC-based class declaration with parameters corresponds to a parameterized-style declaration or to a combination of 'include'-like declaration and semi-separate data.  The former is more straightforward, so that would be my guess, but the latter would be far superior for the purpose of avoiding evaluation-order problems.

 

Here's where things get interesting.  If I change my ENC output to
provide no parameters for apache::mod::wsgi, then I end up with all
variables using the $::apache:: reference being 'undef'.

ENC:

---
classes:
  apache:
    default_ssl_vhost: true
    default_vhost: true
    purge_configs: false
    server_signature: 'Off'
    server_tokens: Prod
    trace_enable: 'On'
  apache::mod::ssl:
  apache::mod::wsgi:

Error:

Error: Could not retrieve catalog from remote server: Error 400 on
SERVER: Invalid relationship: File[wsgi.conf] { before => File[undef]
}, because File[undef] doesn't seem to be in the catalog
Warning: Not using cache on failed catalog
Error: Could not retrieve catalog; skipping run

That seems like a bug in how Puppet is handling classes defined from an ENC.

See my previous comments.  I think Puppet could better handle ENC output containing class parameter values, but the main problem is that the module isn't safe to use as you are trying to use it.  At minimum that indicates a flaw in the module documentation (it doesn't explain this limitation).  I think the intent is for the module to support your use, but I don't think it reliably can under any current Puppet.

 

Finally, if I go back to stock puppetlabs-apache-1.0.1 and define a
parameter for apache::mode::ssl ...

ENC:

---
classes:
  apache:
    default_ssl_vhost: true
    default_vhost: true
    purge_configs: false
    server_signature: 'Off'
    server_tokens: Prod
    trace_enable: 'On'
  apache::mod::ssl:
    ssl_options:
    - StdEnvVars
  apache::mod::wsgi:
    wsgi_socket_prefix: /var/run/wsgi

NO ERRORS.

Thoughts?

I think Puppet is doing something along the lines of first processing those ENC class declarations for which parameter values are given, and then making a second pass to process the others.  That would be an attempt to solve some of the inherent evaluation-order problems, but not a wholly effective one.

 

 It seems as though a class defined by an ENC with no
parameters is treated differently than a class defined with
parameters.

Indeed.

 

 I'd expect and thought that a class with no parameters
was called in a similar was as doing "class { 'apache::mod::ssl': }".

Goodness, no.  It should be equivalent to doing "include 'apache::mod::ssl'".  And indeed, I think it is.

The problem is that ENC-declared classes with parameters should also be handled as if declared via 'include', with the provided parameters forming a highest-priority data source for automated parameter binding.  That would avoid issues with that same class being declared also somewhere else (such as class 'apache' being declared in in 'apache::mod::ssl').

 

Should this be filed as a bug against Puppet?

I could see filing an RFE, but I don't think it's a bug per se because Puppet is behaving as designed.

I will bring this up for discussion on the puppet-dev list, and an RFE could come from that.  But you can certainly file your own if you wish.

In the mean time, The best solution under current Puppet is probably to put your data into Hiera instead of feeding it via the ENC output.  If you don't want to do that, then the best alternative that avoids Hiera is probably to take control of evaluation order by interposing your own class between the ENC and the apache stack.  Maybe something like this:

class site::apache_stack(
  $default_ssl_vhost = true,
  $default_vhost = true,
  $purge_configs = false,
  $server_signature = 'Off',
  $server_tokens = 'Prod',
  $trace_enable = 'On',
  $wsgi_socket_prefix = '/var/run/wsgi'
) {
  class { 'apache':
    default_ssl_vhost => $default_ssl_vhost,
    default_vhost => $default_vhost,
    purge_configs => $purge_configs,
    server_signature => $server_signature,
    server_tokens => $server_tokens,
    trace_enable => $trace_enable
  }

  include 'apache::mod::ssl'

  class { 'apache::mod::wsgi':
    wsgi_socket_prefix => $wsgi_socket_prefix
  }
}

The ENC would then need to output something like this instead of what it did before:

classes:
  site::apache_stack:

    default_ssl_vhost: true
    default_vhost: true
    purge_configs: false
    server_signature: 'Off'
    server_tokens: Prod
    trace_enable: 'On'

    wsgi_socket_prefix: /var/run/wsgi


Obviously, that's somewhat inflexible, in that your interposed class must accept all parameters you want to be able to declare via your ENC, and it must declare all the component classes in a suitable order.  Furthermore, this approach could be mooted if you declare any of the apache classes elsewhere, whether directly or indirectly.


John