Puppet::Type.hash2resource behavior depends on hash iteration order
|Affected Puppet version:||Branch:||https://github.com/cprice-puppet/puppet/tree/bug/master/12543-hash2resource-depends-on-iteration-order|
When writing a test that creates an inline instance of a type using a hash construct, e.g.:
sleep_exec = Puppet::Type.type(:exec).new( :name => 'exec_spec sleep command', :command => "/usr/bin/ruby -e 'sleep 0.02'", :timeout => '0.01')
the behavior may vary depending on the order that the particular ruby version happens to iterate over the hash values.
The problem occurs when the hash contains a value for both “:name” and whatever the namevar is for the type in question (in the example above, “:command”). In lib/puppet/type.rb, in hash2resource, there is a block that does this:
hash.each do |param, value| resource[param] = value end
This calls into Puppet::Resource.=, which contains this line:
parameters[parameter_name(param)] = value
The “parameter_name” method, in turn, will convert “:name” to the namevar value.
So… if the hash iteration in this example happens to iterate over “:command” before “:name”, then the value for “:name” will overwrite the value for “:command”. In this particular example that will completely break the code, because my “:name” value was never intended to be interpreted as a command.
This is probably a pathological case, because I think I was confusing the intent of “:title” with the intent of “:name”; however, there is still a bit of code smell here.
I’ll add a link to a branch where this can be repro’d. But, adding a print statement to the iteration loop in hash2resource, I see the following differences in output:
Linux, Ruby 1.8.7-p334
$ rspec -e "should fail if timeout is exceeded" ./spec/unit/type/exec_spec.rb ... HASH2RESOURCE called NEXT ITEM IN HASH: ':timeout' NEXT ITEM IN HASH: ':name' NEXT ITEM IN HASH: ':command' ...
Win2k3, Ruby 1.8.7-p334
$ rspec -e "should fail if timeout is exceeded" ./spec/unit/type/exec_spec.rb ... HASH2RESOURCE called NEXT ITEM IN HASH: ':timeout' NEXT ITEM IN HASH: ':command' NEXT ITEM IN HASH: ':name' ...
On linux, “command” is the last value iterated over, so the resource gets the correct command and the test passes.
On windows, “name” is the last value iterated over, so it blows away the value of “command” and the test fails.
#3 Updated by Chris Price almost 2 years ago
Working repro example for this is here:
The test you would want to run is:
rspec -e "should fail if timeout is exceeded" ./spec/unit/type/exec_spec.rb
This will likely pass on unix systems, but should fail on the out-of-the-box win2k3 VM that Josh has set up.
#4 Updated by Luke Kanies almost 2 years ago
This is definitely just a normal bug – too many layers of history, and we lost some ordering logic in a recent change.
There’s some plumbing in there to provide ordering of parameter names, and that should be used for all of this kind of iteration; the fact that it’s not sure looks like a bug.