Hello, puppet users Reviewing a friend's puppet module, puppet-lint shows he didn't use the explicit namespace on variables. He told me that he used the Style Guide[1] and the Beginner's Guide to Modules[2] which are a little unclear about the right way to do this, so I wanted to write to clarify this issue.
The Guide to Modules describes using a params.pp file and having a class inherits directive to pass variables to the anchored classes, like in the puppetlabs' ntp module. The style guide says the following: "You must scope all variables except for local or inherited variables. Scope inherited variables, when appropriate, for clarity. You should not mask/shadow inherited variables." We interpreted this paragraph differently: he says because they are inherited it is not needed to scope the variables, but I say that for clarity they should be scoped. If it is better to scope them all the time, why is there the possibility of using not scoped variables with class inheritance in the first place? I've seen people on #puppet, and on this list, say that every variable should be scoped for consistency and clarity, but this isn't explicitly said in those guides. In this case, variables aren't being mixed from different scopes, so there isn't a clarity issue. In fact the argument is that if you explicitly scope every variable, but there is no other module where variables are coming from, then you need to read every variable to check to see if there is a variable coming from another scope (::module1::foo vs. ::module2:bar), but if everyone has the same scope then you are unnecessarily reading more things to check for another scope, when there isn't one. Technically, you can just use the variable short name when using 'inherits'... but its frowned on, but that frowning isn't detailed anywhere in the style guides or docs. https://docs.puppetlabs.com/guides/scope_and_puppet.html#qualify-your-variables says: Whenever you need to refer to a variable in another class, give the variable an explicit namespace: instead of simply referring to $packagelist, use $git::core::packagelist. This is a win in readability — any casual observer can tell exactly where the variable is being set, without having to model your code in their head — and it saves you from accidentally getting the value of some completely unrelated $packagelist variable. For complete clarity and consistency you will probably want to do this even when it isn’t absolutely neccessary. This last sentence seems to implore you to do it when its not necessary because it is more clear and consistent everywhere, but this is only really detailed in the context of setting variables and dynamic scope, which isn't there anymore, modules are different as they are always inside the class. So, clarification needed only when you are calling it from another place, or clarification everywhere (eg. for someone else who is reading the code)? More confusing is that it seems like these types of params modules are sometimes discouraged on this list, although the guides seem to use these params and inherits models (and this style is often used by the puppetlabs modules). So should we avoid this type of module, or not? Can we get a more clarity around this issue? If the best practices is actually different from what these guides show, then why aren't these updated to reflect that? Personally, because inheritance is tricky, dynamic scoping was problematic, and namespace changes have been hard to keep track of over time, I like to namespace everything... but I've been doing this for a while. New people might not think this is such a big deal now. According to this puppet-lint issue it should not use inherited variables but then scope them: https://github.com/rodjek/puppet-lint/issues/304 As it works in both cases, We wonder which should be the default way of writing puppet modules. The controversial part is related to the following code: # init.pp class myservice ( $service_variable = $myservice::params::service_variable ) inherits myservice::params { anchor { 'myservice::begin': } -> class { '::myservice::install': } -> anchor { 'myservice::end': } } # params.pp class myservice::params { $service_variable = 'variable' } # install.pp class myservice::install inherits myservice { file { $service_variable: ensure => 'file' } } Should $service_variable be changed to $myservice::params::service_variable (or even $::myservice::params::service_variable) If that's the case, can we update the Guide to Modules or give more clarity to the style guide? [1]: https://docs.puppetlabs.com/guides/style_guide.html [2]: https://docs.puppetlabs.com/guides/module_guides/bgtm.html -- You received this message because you are subscribed to the Google Groups "Puppet Users" group. To unsubscribe from this group and stop receiving emails from it, send an email to puppet-users+unsubscr...@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/puppet-users/f4fe1bd4-2996-41b8-8117-73feb1ada413%40googlegroups.com. For more options, visit https://groups.google.com/d/optout.
