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

Vault 0.5.3 -> 0.6.0 is breaking. Cookbook major version should have been rev'd. #70

Closed
eherot opened this issue Aug 16, 2016 · 4 comments

Comments

@eherot
Copy link

eherot commented Aug 16, 2016

See the Vault docs on breaking changes in 0.6.

eherot added a commit to evertrue/et_secrets-cookbook that referenced this issue Aug 16, 2016
Because these guys [can't be trusted to use SemVer properly](sous-chefs/hashicorp-vault#70).
@legal90
Copy link
Contributor

legal90 commented Aug 22, 2016

Hi @eherot,
Yes, Vault 0.6.0 is actually breaking.
But I don't think that it's reasonable to change cookbook version by the same way as the version of software it installs. Actually, cookbook version is representing only changes in the cookbook itself.

Since the behavior of recipes & resources hasn't been changed so much, there is no reason to bump a cookbook's major version. Also, it should be noticed that Vault's version is set by an attribute, so it means that it is possible to install a "breaking" version with any cookbook revision, including future releases.

Just make sure you've set the proper attribute value before upgrading the cookbook. By my own experience, I'd recommend to use wrapper cookbooks and override such important attributes there. It helps to avoid such unexpected changes. And, for sure, tests are extremely important.

eherot added a commit to evertrue/et_secrets-cookbook that referenced this issue Aug 24, 2016
Because vault-cookbook [doesn't seem to use SemVer properly](sous-chefs/hashicorp-vault#70).
@eherot
Copy link
Author

eherot commented Aug 24, 2016

Actually, cookbook version is representing only changes in the cookbook itself.

I would argue that in the context of Chef, because the cookbook does not provide any kind of abstraction layer on top of Vault (the way that an API would), changes to the underlying packages are fundamentally the same as changes to the cookbook itself. If the end user has not taken the proactive step of pinning the version attribute, this change has the potential to break their production environment at unexpected times.

Since the behavior of recipes & resources hasn't been changed so much, there is no reason to bump a cookbook's major version.

But the point is that if the Vault package is upgraded, the behavior of the attributes may change because attributes update config files, and config files are interpreted by Vault directly. If your cookbook were interpreting the attributes and then delivering them to Vault in a way that was guaranteed to be compatible with the new version, I would agree that this would be a reason not to do a major version bump, but that is not the case here.

Also, it should be noticed that Vault's version is set by an attribute, so it means that it is possible to install a "breaking" version with any cookbook revision, including future releases.

Sure, I could break my own system if I wanted to. The point is it's not nice if packages that I depend on break my system without warning.

Just make sure you've set the proper attribute value before upgrading the cookbook.

This is actually a practice that you probably don't want to encourage because now you (the package maintainer) have to worry about compatibility between the cookbook and the Vault package even when only upgrading across minor versions. For example: Suppose between Vault 0.6.0 and 0.6.1 a new config directive is added. Normally you could just add a new default for this directive to the cookbook templates and bump the Vault package version, but if people have pinned version 0.6.0 in the attributes, they may get an error that the config file contains an unrecognized attribute. Additionally, this puts the user in the position of having to be knowledgeable about which package versions are compatible with which cookbook versions. This partially defeats the purpose of using a community cookbook in the first place.

@johnbellone
Copy link
Contributor

johnbellone commented Oct 5, 2016

If the end user has not taken the proactive step of pinning the version attribute, this change has the potential to break their production environment at unexpected times.

This is the key point to my argument below. If you're using any cookbook in production you need to be pinning to specific revisions to have any kind of semblance of consistency. This is also the reason why you should absolutely not use roles or environments (because it is very difficult to do this right in that kind of situation), but that's another argument entirely.

I would argue that in the context of Chef, because the cookbook does not provide any kind of abstraction layer on top of Vault (the way that an API would), changes to the underlying packages are fundamentally the same as changes to the cookbook itself.

In the past I have been very much against increasing the major version of a cookbook to match a new release of the underlying software. The whole point of having a cookbook with custom resources is to provide a layer of abstraction on top of the installation and configuration of the underlying software.

But the point is that if the Vault package is upgraded, the behavior of the attributes may change because attributes update config files, and config files are interpreted by Vault directly. If your cookbook were interpreting the attributes and then delivering them to Vault in a way that was guaranteed to be compatible with the new version, I would agree that this would be a reason not to do a major version bump, but that is not the case here.

I very much do agree with with your point here. If the underlying configuration of the software has changed drastically that requires the custom resource itself to change then that warrants a major version bump. But that is only when the custom resource in this library changes. If that is something that we missed in this particular release then I apologize. We should catch that in the future, and we need to be more careful. But that doesn't change my position on major version increases for each Vault release.

The main takeaways here should be that this cookbook, and any other cookbook that you rely on, are themselves versioned libraries and they need to be treated as such. That means, just as you would with an application, you should be locking dependencies using either the environment cookbook pattern or Policyfiles.

My apologies if I haven't explained this adequately, it is 4am here and I haven't had any coffee yet this morning. Please feel free to reach out directly and we can talk.

@lock
Copy link

lock bot commented May 19, 2019

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.

@lock lock bot locked as resolved and limited conversation to collaborators May 19, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants