Nginx location as exported resource not working anymore

[ITler]

Affected Puppet, Ruby, OS and module versions/distributions

• Puppet: current pc1 with puppet 4.8.0
• Ruby: from pc1(irrelevant for this issue)
• Distribution: Ubuntu 14
• Module version: 0.5.0 (forge)

How to reproduce (e.g. Puppet code you use)

I need to define a location on an application server as an exported resource, that will get realized on another host with nginx installed. So on my appserver I only use:

create_resources('@@nginx::resource::location', $locations, $location_params)

What are you seeing

In an older version of jfryman-nginx (some version not far before the last 9999999) this worked fine. With the last version of jfryman and the current 0.5.0 from voxpupli this doesn't work anymore.

What behaviour did you expect instead

I would have expected to have things working, like they did before.

Output log

Error: Could not retrieve catalog from remote server: Error 500 on SERVER: Server Error: Evaluation Error: Error while evaluating a Resource Statement, Evaluation Error: Unknown variable: '::nginx::config::proxy_redirect'. at /etc/puppetlabs/code/environments/dev/modules/nginx/manifests/resource/location.pp:165:27 at [...]

Any additional information you'd like to impart

Of cause, the appserver doesn't have an nginx installation, so I can't call include ::nginx

However when explicitly defining proxy_redirect in $location_params this seem to work. Then issue cascading to next parameter.. and so on... This means, that every parameter definition of nginx/manifests/resource/location.pp assigning a default value from $::nginx will cause this issue. I think the obvious approach (usual pattern) should be to assign default values from $::nginx::parms, which could be included beforehand.

Kind regards ITler

[wyardley]

I kind of get what you're doing here, but I'm not totally sure what the expected behavior is, or what changes exactly caused this.

Is it possible to test with current git master branch and see if the behavior works as you expect? The next release will get rid of the nginx::config namespace that was introduced a while back (i.e., all parameters will just be defined at top scope again). However, I'm not sure if that will change this behavior. fwiw, proxy_redirect defaults to undef, not sure if that relates to your problem, though I think you could set an explicit default when you override the class (when you instantiate the class, or in hiera).

Usually, as I understand it, it's parameters that vary depending on, say, OS or other factors, are in params, other defaults are just assigned directly in the class params (if they're overridable). Does that kind of make sense?

[ITler]

Hy ... I'll test with git master as soon as possible.

If I understand correctly, it seems, that it is not fully clear, what I'm doing and why. I'll try to explain in other words: The expected behaviour is, to not get syntax errors from nginx module, when only defining location defined type on a node, that may not include nginx or nginx::config classes. That's because nginx is not to be installed on that machine. However location needs to be defined on appserver, because I can only gather several information on that specific node (appserver) Using exported resources and puppetdb then allow me to transfer the location resource to real nginx host to be implemented. You said:

Usually, as I understand it, it's parameters that vary depending on, say, OS or other factors, are in params, other defaults are just assigned directly in the class params (if they're overridable). Does that kind of make sense?

I think this makes sense, however currently (where my issue comes from, i.e. for proxy_redirect), this pattern is broken, as location uses default values defined by nginx::config (which also does implementation). I agree, as far as I know the common design pattern is to serve default values, only, from a nginx::params class, which could get included elsewhere, but does no real configuration to a node.

Conclusion

So there are 2 possible approaches. Define useful parameter defaults in classes or defined types directly or encapsulate those in a ::params class, which only hold variable OS dependent and independent values, but does no configuration like file, exec, etc.

Greets

[ITler]

Additional Info:
It looks like this problem occurs only on younger puppet systems, with strict_variables = true. Older versions seem to not respect this setting. However, as strict_variables is a recommendation by puppet, this module's code should work. This issue escalates to a show stopper for a planned migration project next week. Is it possible for you to refactor sources?

[zachfi]

The error in the OP references ::nginx::config::proxy_redirect. Has this been updated to be ::nginx::proxy_redirect? If so, could we get the latest error please?

[wyardley]

@ITler: Current version (git master, and soon to be released version) does not have an interface tonginx::config at all, it's a private class, and all parameters are now defined at top scope. So I think most recent changes should resolve these issues for you; if not, agree with @xaque208, show how you're invoking the module and include updated error messages. Any references to ::nginx::config::foo should become ::nginx::foo

[ITler]

Yes, I understood, that nginx::config is obsolete/removed and you moved variables to top scope: ::nginx.
My problem comes from a different scenario/use case/starting point. We're talking about different things.

In my use case "define location as exported resource on application server node", puppet is not able to access/resolve ::nginx::foo or ::nginx::proxy_redirect or any variable from ::nginx during catalog build, because I intentionally do not include ::nginx. (I can't include it, as this would install nginx to the node (application server), but this may not happen in my configuration.) As ::nginx::<any variable> was never defined, puppet 4.8. and strict_variables = true throws Unknown variable exception.

Maybe another example:
Current master (50cced967bcf50fef80b89cdd2d8e334b46a9009) location.pp line 174 has:

$fastcgi_params = "${::nginx::conf_dir}/fastcgi_params",

without include ::nginx the variable conf_dir can never be resolved. So path for fastcgi_params will not be correct. With configuration strict_variables = true puppet throws Unknown variable exception, with strict_variables = false it just evaluates to "empty", which is just wrong (and hard to debug/trace, so maybe this is the reason, why puppetlabs introduced strict_variables setting in their configuration)

Somehow (I can't explain why - maybe bug in puppet itself) I've got no problem on older puppet server version, but I'm forced to upgrade. So I run into that issues here. Also strict_variables = true will become built-in with puppet 5 so for future compatibility this needs to be addressed.

[ITler]

Latest error in current master (50cced967bcf50fef80b89cdd2d8e334b46a9009) location.pp line 179 :

$uwsgi_params = "${nginx::config::conf_dir}/uwsgi_params",

[ITler]

PR #974 solves my use case with bug fixed from previous comment and has minimal impact on code. Please merge asap.