The Puppet Labs Issue Tracker has Moved:

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 See the following page for information on filing tickets with JIRA:

Refactor #5439

Puppet::Property::KeyValue should be more generic

Added by Stefan Schulte over 5 years ago. Updated over 4 years ago.

Status:AcceptedStart date:12/01/2010
Priority:NormalDue date:
Assignee:-% Done:


Target version:-
Affected Puppet version: Branch:

We've Moved!

Ticket tracking is now hosted in JIRA:


In my opinion puppet should provide a small set of good property classes that can be used by anyone who wants to write a custom type. While some types define their own classes (take a look in zone.rb), puppet/property looks like a good place for general ones. Unfortunately KeyValue has some really specific behaviour (its only used for the user_role_add to store attributes for /etc/user_attr).

Where to use KeyValue? Have a look at /etc/user_attr. This is one example line (taken from


Field separator is ‘:’ and all the keyvalue pairs in the last field is perfect for KeyValue. You can represent this last field with a hash attributes => {:auths => 'solaris.*,solaris.grant' , :profiles => 'All', :type => 'normal' }

KeyValue now also allows us to use minimal membership: If I want attributes => {:type => ‘role’}, puppet will change the type while leaving all the other attributes in tact (merging what the provider retrieves as IS and what the user defined as SHOULD)

So far I explained why I like the KeyValue. What I dont like is, its weired behaviour: When you query the should-value you destory your current value. Let’s say my provider retrieves {:key1 => 'value1' } as the is-value, and the user specified 'key2=value2' which translates to {:key2 => 'value2'}. When I call should on this property, I expect it to be {:key2 => 'value2'} if membership is set to inclusive and I expect {:key1 => 'value1', :key2 => 'value2'} if membership is set to minimum, But if you have membership inclusive the result will be {:key1 => nil, :key2 => 'value2'} AND your is-value is now {:key1 => nil} (if your provider doesnt duplicates the hash in attributes()). The modification on the original Hashobject is done in property/keyvalue.rb:process_current_hash. This behaviour is not obvious and I think incorrect in most cases.

I think the reason of setting every key to => nil that appears in IS but not in SHOULD is there because in user_role_add.rb the author tries to modify /etc/user_attr with usermod and usermod has no function to delete a key. For a few attributes it works when you pass “key=” to delete the key. (It’s not working for projects by the way, usermod -K project= user throws an error) so I tried to build a parsedfile provider for /etc/user_attr. But now I’m getting a hash that doesnt represent my real should)

Long story short: Puppet::Property::KeyValue Function process_current_hash should return an empty Hash if membership is set to :inclusive and if user_role_add depends on the current behaviour it should subclass and overwrite this method.


#1 Updated by James Turnbull over 5 years ago

  • Status changed from Unreviewed to Needs Decision
  • Assignee set to Nigel Kersten

#2 Updated by Nigel Kersten over 4 years ago

  • Status changed from Needs Decision to Accepted
  • Assignee deleted (Nigel Kersten)

Also available in: Atom PDF