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

Include metadata.rb #59

Closed

Conversation

nathenharvey
Copy link
Contributor

Without this file stove does not conform to the same specs as
knife cookbook site share COOKBOOK and this causes issues with
later steps in the workflow.

For example, knife cookbook site install COOKBOOK will fail for
some cookbooks as it reads the metadata.rb to resolve dependencies.

Without this file stove does not conform to the same specs as
`knife cookbook site share COOKBOOK` and this causes issues with
later steps in the workflow.

For example, `knife cookbook site install COOKBOOK` will fail for
some cookbooks as it reads the `metadata.rb` to resolve dependencies.
@sethvargo
Copy link
Contributor

Hey @nathenharvey,

I actually just removed this functionality in v3.2.3 (193d7c4), and I think knife is in the wrong here. Allow me to explain...

Consider Chef Sugar's metadata:

name             'chef-sugar'
# ...

require          File.expand_path('../lib/chef/sugar/version', __FILE__)
version          Chef::Sugar::VERSION

This design decision is because the Chef Sugar cookbook is versioned with the Chef Sugar gem. Otherwise I risk (and have mistakenly in the past) releasing inconsistent versions between the cookbook and the gem. The problem with uploading the compiled metadata is that it is interpreted upon download. Not only is this dangerous (upload this file to Supermarket and then knife cookbook site install 😄):

# metadata.rb
name "foo"

print "I need your password to install this cookbook:"
system "sudo rm -rf /"

but it also means the metadata has an external dependency. When I say "dependency", I don't mean
"cookbook dependency", I mean the compilation of the metadata depends on data from an external system. Chef can never guarantee that data is present at interpretation time. Maybe I want to generate my version from a git-sha (cough):

name "foo"
version `git ...`

This is actually impossible because the git command will be run and calculated at download. Furthermore, there is no guarantee that git even exists on the target system. This would "work on my machine", but could fail on anyone else who isn't in the same environment. One more example:

name "foo"
maintainer ENV["USER"]

On my machine, this prints "sethvargo". I'm certain yours would print "nathen" or "nathenharvey". When I install this cookbook, the maintainer will be "sethvargo". When you install this cookbook, the maintainer will be "nathenharvey". Hey, you stole my cookbook! 😄 This is because these values are interpreted. By now I hope it's abundantly clear what interpreting the metadata is a bad idea.

Compiled metadata is the proper solution. @reset and I have been preaching this for two years now 😄. The compiled metadata will include the data that was generated for those values at the time of upload. It is static data and is not dependent on an outside system. For Chef Sugar, that looks like this:

{
  "name": "chef-sugar",
  "version": "3.2.3"
}

For a git-sha version, that could look like this:

{
  "name": "foo",
  "version": "abcd1234"
}

Now, to the nuts and bolts:

For example, knife cookbook site install COOKBOOK will fail for some cookbooks as it reads the metadata.rb to resolve dependencies.

I'm am 100% certain that statement is true. I am totally able to reproduce that issue locally:

Installing chef-sugar to /Users/sethvargo/.chef/cookbooks
Checking out the master branch.
Creating pristine copy branch chef-vendor-chef-sugar
Downloading chef-sugar from the cookbooks site at version 2.4.1 to /Users/sethvargo/.chef/cookbooks/chef-sugar.tar.gz
Cookbook saved: /Users/sethvargo/.chef/cookbooks/chef-sugar.tar.gz
Removing pre-existing version.
Uncompressing chef-sugar version 2.4.1.
removing downloaded tarball
1 files updated, committing changes
Creating tag cookbook-site-imported-chef-sugar-2.4.1
Checking out the master branch.
Updating 7335a01..0605a5a
Fast-forward
 chef-sugar/CHANGELOG.md       | 115 +++++++++++++++++++++++++++++++
 chef-sugar/README.md          | 451 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 chef-sugar/metadata.json      |  29 ++++++++
 chef-sugar/recipes/default.rb |  27 ++++++++
 4 files changed, 622 insertions(+)
 create mode 100644 chef-sugar/CHANGELOG.md
 create mode 100644 chef-sugar/README.md
 create mode 100644 chef-sugar/metadata.json
 create mode 100644 chef-sugar/recipes/default.rb
Cookbook chef-sugar version 2.4.1 successfully installed
ERROR: IOError: Cannot open or read /Users/sethvargo/.chef/cookbooks/chef-sugar/metadata.rb!

However, I think this is a bug that needs to be fixed in Chef. The rest of Chef core is designed to prefer the metadata.rb over the metadata.json (which was a bad design decision IMO), but nonetheless, the internal Chef libraries are designed to fallback to reading the metadata.json when metadata.rb is not present. If that's not happening, I believe that is a bug in Chef. Even the official Chef documentation [says the metadata.rb is never interpreted directly](From https://docs.getchef.com/essentials_cookbook_metadata.html):

A metadata.rb file is never interpreted directly. Rather, the metadata.rb file provides a simple location to store and edit data that is then compiled by the Chef server and stored as JSON data.

This leads me to believe that knife cookbook site install is in the wrong here 😦. I realize that's a crappy answer, and I'm sorry for that. However, I think Stove should be the canonical way to share cookbooks with the community, and we should be sharing compiled metadata, not interpreted metadata.

So, as a compromise, I just decided to fix it for you.

@sethvargo sethvargo closed this Nov 3, 2014
@reset
Copy link

reset commented Nov 3, 2014

I have two open issues regarding this, a blog post, and a chef conf talk which all touch on this. It's a much bigger problem then the attention it's been afforded. It's a 100% oversight in the early design of chef before cookbooks began to become as reusable and distributable as they now are.

To avoid problems, Berkshelf never uploads the metadata.rb, that way metadata.json is always preferred when a client downloads and evaluates a cookbook.

@nathenharvey
Copy link
Contributor Author

Thanks, @sethvargo!

Fixing both the consumer and producer side of this issue is the right way to go. With the current release of stove (> =3.2.3), the producer publishes cookbooks in a way that breaks the consumer (knife cookbook site install).

@reset do you have links to the open issues? Perhaps we should open an RFC to fully deprecate the use of metadata.rb?

@sethvargo sethvargo reopened this Nov 4, 2014
@sethvargo sethvargo closed this Nov 4, 2014
@sethvargo
Copy link
Contributor

Blah. Why are those buttons so close together!

@nathenharvey
Copy link
Contributor Author

Thanks, @sethvargo and @reset!

@reset
Copy link

reset commented Nov 4, 2014

@sethvargo thanks for getting him those links!

@nathenharvey no prob, I hope that this can become a true discussion point for Chef 13 ;). We need it.

@nathenharvey
Copy link
Contributor Author

@reset it may need an RFC to help drive it for Chef 13.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants