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

This issue tracker is now in read-only archive mode and automatic ticket export has been disabled. Redmine users will need to create a new JIRA account to file tickets using https://tickets.puppetlabs.com. See the following page for information on filing tickets with JIRA:

Bug #12543

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

Added by Chris Price over 3 years ago. Updated over 2 years 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


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 3 years ago

  • Description updated (diff)

#2 Updated by Chris Price over 3 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 3 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 3 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 3 years ago

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

#6 Updated by Chris Price about 3 years ago

  • Assignee deleted (Chris Price)

#7 Updated by Anonymous over 2 years ago

  • Target version deleted (2.7.x)

Also available in: Atom PDF