cookstyle icon indicating copy to clipboard operation
cookstyle copied to clipboard

Ban setting node attributes to symbol values

Open lamont-granquist opened this issue 4 years ago • 0 comments

Node attributes round trip to JSON and symbols get squashed to strings.

For policyfiles, they are rendered into JSON and those are always interpreted as strings, which means it is impossible to set an attribute to a symbol from within a policyfile. That is not a bug against policyfiles, that is the way it must work to be rendered to JSON.

That makes cookbooks that expect node attributes as symbols always buggy, and any code which sets a node attribute to a symbol always potentially buggy.

Cookbooks should be free to accept symbols, but they MUST always accept strings, and their default values should be strings. They should to_sym any string where it is consumed in order to turn it into a symbol.

So new rule:

in an attribute file:

bad:

default["whatever"] = :nosymbols

good:

default["whatever"] = "nosymbols"

and in recipe file:

bad:

node.default["whatever"] = :nosymbols

good:

node.default["whatever"] = "nosymbols"

Should cover all the writers, so default, force_default, normal, override, force_override, default_unless, normal_unless, override_unless, along with the last argument to write(*args) being a symbol.

This may trigger on people's wrapper cookbooks that set symbols to upstream cookbooks that require symbols. The upstream should be fixed since it is definitely buggy.

This will not catch all use cases by any means.

lamont-granquist avatar May 01 '20 19:05 lamont-granquist