Issue #5439 has been reported by Stefan Schulte. ---------------------------------------- Refactor #5439: Puppet::Property::KeyValue should be more generic https://projects.puppetlabs.com/issues/5439
Author: Stefan Schulte Status: Unreviewed Priority: Normal Assignee: Category: Target version: Affected Puppet version: Branch: 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 http://docs.sun.com/app/docs/doc/816-0219/6m6njqbd1?a=view) root::::auths=solaris.*,solaris.grant;profiles=All;type=normal 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. -- You have received this notification because you have either subscribed to it, or are involved in it. To change your notification preferences, please click here: http://projects.puppetlabs.com/my/account -- You received this message because you are subscribed to the Google Groups "Puppet Bugs" group. To post to this group, send email to [email protected]. To unsubscribe from this group, send email to [email protected]. For more options, visit this group at http://groups.google.com/group/puppet-bugs?hl=en.
