Rule to detect that node attribute setting methods are used #81

Closed
jtimberman opened this Issue Oct 16, 2012 · 5 comments

Projects

None yet

5 participants

@jtimberman
Contributor

Chef 11 will bring some breaking changes to the ways that node attributes must be set. This is documented in greater detail here:

http://wiki.opscode.com/display/chef/Breaking+Changes+in+Chef+11

The short version is, do this:

node.default["my_attribute"] = "Some Value"

This syntax is valid in the current release of Chef and is preferred, too. Any of the node's attribute setting methods can be used (default, normal, set, override, etc).

This is appropriate in attributes files and recipes. Because this form works in the current version:

node["my_attribute"] = "Some Value"

It might get accidentally used elsewhere than just attributes or recipes.

Related to this, foodcritic doesn't currently detect attribute access via method_missing in attribute files. For example, we had code like this in an attributes file in our test suite:

ldap_server "ops1prod"
ldap_basedn "dc=hjksolutions,dc=com"
ldap_replication_password "yes"

I poked around with the AST and found that you should be able to identify method_missing calls in attribute files as vcall nodes at the rightmost part of the tree (whereas accessing a local variable is a var_ref).

Unfortunately, I don't think I'll have time to update the code that detects attribute access (in the near future, anyway) since I have quite a bit of Chef 11 work to do.

Collaborator
jaymzh commented Nov 2, 2012

For a variety or reasons I enforce using setters in recipes... but only through code-review. I'd love a rule to enforce always using setters when setting.

Contributor

Ran into this today where the recipe still had node['attr'] << "value" - triggered an Chef::Exceptions::ImmutableAttributeModification on Chef 11.4.
Made me wince.

Owner
acrmp commented Sep 16, 2013

This was released in foodcritic 3.0.0 as FC047: Attribute assignment does not specify precedence.

Thanks,

Andrew.

@acrmp acrmp closed this Sep 16, 2013

@acrmp Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment