Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New Hash of Hashs format for specifying profiles does not work #339

Closed
jquick opened this issue Sep 28, 2018 · 9 comments
Closed

New Hash of Hashs format for specifying profiles does not work #339

jquick opened this issue Sep 28, 2018 · 9 comments
Labels
Type: Bug Does not work as expected.

Comments

@jquick
Copy link
Contributor

jquick commented Sep 28, 2018

Description

Issue Description

New Hash of Hashs format for specifying profiles for the audit cookbook causes an error if actually used.

Issue Summary: In the README.md for the Audit cookbook there is a section that shows examples for specifying profiles to run using a Hash of Hashes (https://github.com/chef-cookbooks/audit#configure-node second set of examples).

Unfortunately because the "attributes/default.rb" defaults to create the attributes with

default['audit']['profiles'] = []

this causes an "no implicit conversion of String into Integer" error to be thrown when attempting to run the wrapper cookbook.

Expected Results: Specifying the profiles using the hash of hashes format should converge successfully without any errors

Actual Results: Converge errors with a "no implicit conversion of String into Integer" TypeError.

Repro Steps (if possible):

  1. Create a wrapper cookbook for the Audit cookbook >= 7.1.0 and specify a profile using the following example of the Hash of Hashes format in the README.md.
# Hash of hashes, works with Policyfile includes
default['audit']['profiles']['linux'] = { 'compliance': 'base/linux' }
  1. Converge the cookbook and watch for the error.

Request for Developers: Product Fix

Full exception from kitchen converge


       ================================================================================

       Recipe Compile Error in /tmp/kitchen/cache/cookbooks/test/recipes/default.rb

       ================================================================================
   TypeError
   ---------
   no implicit conversion of String into Integer

   Cookbook Trace:
   ---------------
     /tmp/kitchen/cache/cookbooks/test/recipes/default.rb:29:in `from_file'

   Relevant File Content:
   ----------------------
   /tmp/kitchen/cache/cookbooks/test/recipes/default.rb:

    22:  end
    23:
    24:  # node.default['audit']['interval']['enabled'] = true
    25:  node.default['audit']['reporter'] = 'chef-server-automate'
    26:  node.default['audit']['fetcher'] = 'chef-server'
    27:  node.default['audit']['insecure'] = true
    28:
    29>> node.default['audit']['profiles']['DevSec SSH Baseline'] = { compliance: 'admin/ssh-baseline' }
    30:
    31:  include_recipe 'audit::default'
    32:

@Vasu1105
Copy link

Vasu1105 commented Jan 3, 2019

@jquick from the logs it looks like the format in which you specified the ['audit'] ['profiles'] is not right the compliance key should be given in single quotes. Please verify and let me know if that works.

@Vasu1105
Copy link

Vasu1105 commented Jan 3, 2019

Verified it does have a problem.

@jkovarick
Copy link

any update on this issue? I'm also impacted.

@Vasu1105
Copy link

Vasu1105 commented Jan 7, 2019

@jkovarick we are working on it.

@jerryaldrichiii
Copy link
Contributor

For posterities sake, I notice @mattray claiming that this does work:

#346
#328 (comment)

Not sure where the disconnect here is.

@bmiller08
Copy link

@jerryaldrichiii

I looked at those exact things this morning and was coming up with the same error referenced in the original post here. I was hoping it worked, because I'd prefer the hash of hashes setup.

I think the disconnect here is that setting the attributes in a cookbook as a default attribute (like in the original post's example) results in TypeError: no implicit conversion of String into Integer. However, it would appear that setting these attributes as force_default or above results in the expected behavior. Tests run fine.

This would mean that users testing behavior in locations with high precedence are likely getting the expected behavior where users assigning profiles to run as part of a cookbook or in a wrapper cookbook for audit are getting unexpected behavior (the TypeError error).

Since policyfile attributes share the role precedence level (I believe). The examples given by @mattray work in that context. However, it's still broken otherwise.

This appears to be related to the fact that the audit cookbook sets this attribute to an array by default. Setting this to {} instead also results in the expected behavior. Tests run without error. However, as you probably know, doing so breaks the array of hashes method of setting profiles. So, it breaks the intended (I think) backwards compatibility with the old style.

I think the issue of supporting both is discussed here #347 and it would seem that someone is exploring migrating the way profiles are added all together in #357.

In the meantime people can work around the problem by being mindful of attribute precedence and being sure they are only using one type of profile setup in a given run.

@lamont-granquist
Copy link
Contributor

Policyfiles work because you just wind up autovivifying the node.role_default hash, which is not the same as the node.default level. You wind up deep merging an Array with an Hash, but sometime in chef-12-era i think i fixed that to work and the higher precedence level (policyfile level) will overwrite the Array with its Hash).

@lamont-granquist
Copy link
Contributor

(also explains why force_default works because that also is a totally separate data structure which gets processed by the deep merge algorithm)

@alexpop
Copy link
Contributor

alexpop commented Sep 19, 2019

Profiles attribute is now a hash starting from version 9 of the cookbook. Closing this ticket.
Thank you for your contributions!

@alexpop alexpop closed this as completed Sep 19, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Bug Does not work as expected.
7 participants