vault_secret: Save lease ID to the nested attribute

[legal90]

It allows avoiding conflicts with other top-level attributes.
Related comment: 7f2f49d#commitcomment-17324222

cc: @Ginja

[Ginja]

Great!

One small suggestion. Should we expose this as a property and have its default value come from an attribute?

[legal90]

I've made a mistake in the PR title. In fact, we use node attributes only to save secret lease ID, not the entire secret.

So, this attribute is designed only for the internal technical reason - to validate lease ID in upcoming Chef client runs. IMO, there is no need to customize this property.

However, it is reasonable to make run_state nesting level customizable, because that's exactly how users access secrets in the recipe code. In this PR I haven't touch it: node.run_state[new_resource.path]. Should I do this?

[Ginja]

So, this attribute is designed only for the internal technical reason - to validate lease ID in upcoming Chef client runs. IMO, there is no need to customize this property.

True, but we are storing something on node object, and it would be nice to let people change it if they wish (perhaps there is another use we're not thinking of). However, this can always be added later.

I agree with making the reference in run_state customizable. One way would be to change new_resource.path to new_resource.name. However, that may be un-intuitive for users, and may lead to confusion if they don't realize. An additional run state reference property could be added with a default value of nil and if it wasn't declared, the reference would default to new_resource.path.

[codecov-io]

Current coverage is 52.58%

Merging #56 into master will increase coverage by -21.64%

@@           master        #56   diff @@
========================================
  Files           2          2          
  Lines          97         97          
  Methods         0          0          
  Branches        0          0          
========================================
- Hits           72         51    -21   
- Misses         25         46    +21   
  Partials        0          0          

1. File ...ries/vault_config.rb (not in diff) was modified. more
   • Misses +21
   • Partials 0
   • Hits -21

Powered by Codecov. Last updated by 53d073a...5518fd4

[legal90]

@Ginja I'va added resource property run_state_reference. Is it OK, or should be renamed/modified ?

[Ginja]

Looks great. Thanks!

[lock]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.