|
Resource defaults set in 'class' scope currently leak into adjacent classes taking effect in the order they are parsed. I believe this is known internally as "dynamic scoping".
As the use of puppet forge and modules in general has started to increase, this is starting to cause problems that are hard to diagnose and fix.
This problem typically manifests itself as incorrect file ownership due to setting resource defaults for the File resource, but of-course could affect any resource type in puppet.
Problems this causes
Breaks idempotency of include function
The parse order of include statements changes the effect of the final catalogue - see testcase
Resource defaults leak into unrelated classes
Depending on parse order, its possible for resource defaults to leak into unrelated classes and alter the effect of them in subtle ways - in the past this has broken the concat module to the extent that puppet fails to run (see linked issues). I'm sure there are other examples of this too.
Hard to fix modules elegantly
Unwanted resource defaults are not easy to fix elegantly as they can require invasive code changes to protect against external influences. I have personal experience of this with the linked bug report to the concat module. In this case, you could argue the 'module above/including us' is the one with the problem since concat normally works fine. It becomes hard to see elegant ways to fix problems or indeed which module has a problem in the first place - the including one? the included one? both? puppet itself? See link to discussion on hunner-wordpress module's github issues page.
Against the puppet ideology
IMO puppet is supposed to be idempotent, with transparent and intuative results. The "dynamic scoping" of resource defaults goes against these principles.
Testcase
The attached testcase has two test files that should be funtionally identical but are not, because they include classes in different orders. This results in different file ownership for /tmp/defaults_testcase file, and worse, the /tmp/unrelated file which picks up the unwanted resource defaults even thought its in another module:
[root@master /etc/puppetlabs/puppet]# puppet apply modules/defaults_testcase/tests/order1.pp
|
Notice: Compiled catalog for master.puppetlabs.vm in environment production in 0.11 seconds
|
Notice: /Stage[main]/Defaults_testcase::Class_c/File[/tmp/defaults_testcase]/ensure: created
|
Notice: /Stage[main]/Unrelated/File[/tmp/unrelated]/ensure: created
|
Notice: Finished catalog run in 0.15 seconds
|
[root@master /etc/puppetlabs/puppet]# ls -l /tmp/defaults_testcase /tmp/unrelated
|
-rw-r--r-- 1 wordpress wordpress 0 Nov 24 07:28 /tmp/defaults_testcase
|
-rw-r--r-- 1 wordpress wordpress 0 Nov 24 07:28 /tmp/unrelated
|
[root@master /etc/puppetlabs/puppet]# puppet apply modules/defaults_testcase/tests/order2.pp
|
Notice: Compiled catalog for master.puppetlabs.vm in environment production in 0.12 seconds
|
Notice: /Stage[main]/Defaults_testcase::Class_c/File[/tmp/defaults_testcase]/owner: owner changed 'wordpress' to 'drupal'
|
Notice: /Stage[main]/Defaults_testcase::Class_c/File[/tmp/defaults_testcase]/group: group changed 'wordpress' to 'drupal'
|
Notice: /Stage[main]/Unrelated/File[/tmp/unrelated]/owner: owner changed 'wordpress' to 'drupal'
|
Notice: /Stage[main]/Unrelated/File[/tmp/unrelated]/group: group changed 'wordpress' to 'drupal'
|
Notice: Finished catalog run in 0.16 seconds
|
[root@master /etc/puppetlabs/puppet]# ls -l /tmp/defaults_testcase /tmp/unrelated
|
-rw-r--r-- 1 drupal drupal 0 Nov 24 07:28 /tmp/defaults_testcase
|
-rw-r--r-- 1 drupal drupal 0 Nov 24 07:28 /tmp/unrelated
|
Current workaround
The current workaround to protect against this is to explicitly set file defaults at the top of each manifest (or to explicitly set all parameters of interest).
Suggested (by me) workaround/expected behaviour
Resource defaults should only apply to the class they are defined in within class scope. Resource defaults defined in other classes (class scope) should not be used, instead the next highest scope (node) should be processed instead.
|
