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

Add privacy flag #2460

Merged
merged 2 commits into from
Aug 18, 2015
Merged

Add privacy flag #2460

merged 2 commits into from
Aug 18, 2015

Conversation

raskchanky
Copy link
Contributor

This adds a privacy flag to metadata.rb which will mark a cookbook as private (or not). If you try to upload a cookbook marked private to Supermarket, and Supermarket is configured to enforce privacy, then the upload will fail.

Here is the original issue this was spawned from: chef/supermarket#832
Here is the matching PR to Supermarket itself: chef/supermarket#912

I tested this by doing knife cookbook upload of a cookbook with the privacy attribute set to true, against Chef Server 12.2.0.dev.0 running in a local VM and it succeeded.

Is there any further testing I should do or anything I've overlooked here?

@lamont-granquist
Copy link
Contributor

Have you tested this against Hosted/EC12/EC11 with a knife cookbook upload whatever and/or knife upload /cookbooks/whatever and shown that it'll get through the validation checking that erchef does on input? Also should test with berks upload to make sure that does not choke on the new metadata as well.

@lamont-granquist
Copy link
Contributor

And I'm pretty surprised that it succeeded against EC 12.2 -- is your code just not posting the new metadata field on a cookbook upload? My understanding was that erchef had incredibly annoying validation checking on top level fields, so I'd like a bit more investigation to understand how that is succeeding (and if it got recently patched to be more permissive or something).

@stevendanna
Copy link
Contributor

The validation checking in erchef for cookbook_version defines VALID_KEYS here:

https://github.com/opscode/chef_objects/blob/master/src/chef_cookbook_version.erl#L90-L94

which is then checked with the strictly_valid function here:

https://github.com/opscode/chef_objects/blob/master/src/chef_cookbook_version.erl#L211-L214

which I believe is supposed to error if any keys not in the passed list exist.

@lamont-granquist
Copy link
Contributor

Also 'privacy true' in metadata is somewhat awkward, while 'private true' reads a lot better to me.

@raskchanky
Copy link
Contributor Author

@lamont-granquist It reads better to me too, but I was concerned about that conflicting with the private that specifies private methods. Is that not an issue?

@lamont-granquist
Copy link
Contributor

lol, possibly. i'm not sure if we instance_eval into an instance or into a class instance so not sure if it collides, and its possible that even then the arity of the function would make it distinct but it might still be confusing.

@raskchanky
Copy link
Contributor Author

@lamont-granquist Here are the results of my testing. All of the below results use chefdk 0.3.2, which contains chef 11.16.0.

  • Hosted -> fails with "undefined method 'privacy'"
  • EC12 locally -> fails with "undefined method 'privacy'"
  • EC11 locally -> fails with "undefined method 'privacy'"
  • berks upload -> can't parse metadata.rb, undefined method 'privacy'

But, when I use the version of knife from inside the bin dir on this branch (chef 12.2.0.dev.0) and upload to EC12 locally, then it succeeds. That's the only combination I've found that allows the upload to work. That version of knife won't upload to EC11 - it fails with a 400 "Chef Client version between 10 and 11 required".

I'm honestly not sure what to make of these results. Any thoughts?

@lamont-granquist
Copy link
Contributor

In chef-dk you have to update /opt/chef/embedded/apps/chef -- /opt/chef/bin/knife has magical Gemfile.lock-like pinning to the chef gem installed in that directory. If you just install the gem in ruby you won't be invoking it. You can bypass that with /opt/chef/embedded/bin/knife but you'll still need to worry about which gem version gets picked up out of the installed gems (which will probably be highest rev unless you create a bundle and bundle exec against it).

@lamont-granquist
Copy link
Contributor

(I tested awhile ago that we don't break erchef validation if we add metadata -- yay! -- so we can just merge this, and I don't care that much about what the flag is called, and the supermarket-side is already merged)

@coderanger
Copy link
Contributor

+1 for changing this to private too, this doesn't sound correct.

@lamont-granquist lamont-granquist deleted the jb/add-privacy-flag branch August 19, 2015 21:55
@chef chef locked and limited conversation to collaborators Nov 16, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants