The Puppet Labs Issue Tracker has Moved: https://tickets.puppetlabs.com

Bug #12543

Puppet::Type.hash2resource behavior depends on hash iteration order

Added by Chris Price over 2 years ago. Updated over 1 year ago.

Status:AcceptedStart date:02/09/2012
Priority:NormalDue date:
Assignee:-% Done:

0%

Category:-
Target version:-
Affected Puppet version: Branch:https://github.com/cprice-puppet/puppet/tree/bug/master/12543-hash2resource-depends-on-iteration-order
Keywords:

We've Moved!

Ticket tracking is now hosted in JIRA: https://tickets.puppetlabs.com

This ticket may be automatically exported to the PUP project on JIRA using the button below:


Description

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.

History

#1 Updated by Chris Price over 2 years ago

  • Description updated (diff)

#2 Updated by Chris Price over 2 years ago

  • Branch set to https://github.com/cprice-puppet/puppet/tree/bug/master/12543-hash2resource-depends-on-iteration-order

#3 Updated by Chris Price over 2 years ago

Working repro example for this is here:

https://github.com/cprice-puppet/puppet/tree/bug/master/12543-hash2resource-depends-on-iteration-order

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 over 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.

#5 Updated by Chris Price over 2 years ago

  • Status changed from Needs Decision to Accepted
  • Target version set to 2.7.x

#6 Updated by Chris Price about 2 years ago

  • Assignee deleted (Chris Price)

#7 Updated by Andrew Parker over 1 year ago

  • Target version deleted (2.7.x)

Also available in: Atom PDF