Allow encrypting a previously unencrypted data bag

[vijaymmali1990]

Description

After creating an unencrypted data bag, and trying to edit/encrypt, It was raising exception as shown below.
There was a minor miss while editing. If data is not encrypted, we should be returning data only instead of returning the entire item.

// Creating an unencrypted Data Bag
> knife data bag create test box 
Data bag test already exists
Created data_bag_item[box]

// Edit and add encryption
> knife data bag edit test box  --secret 'ab'
Encrypting data bag using provided secret.
Saved data_bag_item[box]

// Raises Exception:
> knife data bag show test box 
ERROR: Chef::Exceptions::ValidationFailed: Property data_bag's value {"encrypted_data"=>"zuDd2Da3eyZiuPehburpE3DD+oJt2TA=\n", "iv"=>"jPNhkdghdNd+IsEP\n", "auth_tag"=>"V772V241NSkUwy+102R6Dg==\n", "version"=>3, "cipher"=>"aes-256-gcm"} does not match regular expression /^[\-[:alnum:]_]+$/

Issues Resolved

Fixes #8282

Check List

• New functionality includes tests
• All tests pass
• RELEASE_NOTES.md has been updated if required (not required for bugfixes, required for API changes)
• All commits have been signed-off for the Developer Certificate of Origin. See https://github.com/chef/chef/blob/master/CONTRIBUTING.md#developer-certification-of-origin-dco

[Vasu1105]

@tas50 and @btm This LGTM. Please review it from your side.

[btm]

@lamont-granquist review?

It "smells" a little that it's such a simple fix, but this looks fine. I dug around and I can't find a clear direct regression, but you commented on #4815 that there may have been changes to the data bag hash that caused this.

I do wonder if we could craft up a test in spec/integration/knife/data_bag_show_spec.rb for this condition.

[btm]

We had a little further discussion at code review and we could add a missing set of tests to spec/integration/knife/data_bag_edit_spec.rb (the file doesn't exist yet) that fake an editor by modifying JSON directly. We should do that.

[lamont-granquist]

yeah if we had functional tests this would probably break those for the other use case.

and the correct patch should look like a patch to the object model itself, not to knife. so Chef::DataBagItem would need to probably have def self.from_hash modified to be able to accept either format.

but then that also needs to get done for data bags, roles, and environments at a minimum.

and there needs to be some attention made to DRY'ing up this code.

[lamont-granquist]

ah, i think i see now.

i think this fixes #8282 which i just cut, and i deleted all the comments in #4815 which had nothing to do with the original issue. my prior comment only makes sense in the context of where i thought this was addressing what is now #4815.

so, yes, this might fix #8282

could really use some func or integration tests though.

[btm]

This has functional tests now. Should be good to go.

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