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

Make the TLS certificate management optional #13

Merged
merged 1 commit into from Aug 28, 2015

Conversation

@jeffbyrnes
Copy link
Contributor

jeffbyrnes commented Aug 23, 2015

The prescription by this cookbook to manage the TLS certificate, using chef-vault, is overly strong, and, while it is an excellent practice, may not jibe with some potential users’ workflows.

Builds on #4 to use the VaultConfig#tls? method to control whether or not the certificate files are managed. All credit to @zarry!

@johnbellone

This comment has been minimized.

Copy link
Contributor

johnbellone commented Aug 24, 2015

I am going to need to think about this a little bit further. I am trying to keep direct node access from within the library itself. If we do this I would prefer feeding it in as a resource attribute name.

@jeffbyrnes jeffbyrnes force-pushed the evertrue:manage_certificate_optional branch from 9fe9c2d to 7775850 Aug 24, 2015
@jeffbyrnes

This comment has been minimized.

Copy link
Contributor Author

jeffbyrnes commented Aug 24, 2015

@johnbellone as in, you do not want to access node attributes from within the library, but instead pass them in via the resource’s attributes? E.g.,

vault_config 'xyz' do
  # other attributes
  manage_cert false
end
@jeffbyrnes jeffbyrnes mentioned this pull request Aug 24, 2015
@johnbellone

This comment has been minimized.

Copy link
Contributor

johnbellone commented Aug 24, 2015

@jeffbyrnes Yes, correct.

The prescription by this cookbook to manage the TLS certificate, using
chef-vault, is overly strong, and, while it is an excellent practice,
may not jibe with some potential users’ workflows.

Tests for the two logic flows in disabling the certificate management
are also present.
@jeffbyrnes jeffbyrnes force-pushed the evertrue:manage_certificate_optional branch from 7775850 to 1b24cc9 Aug 25, 2015
@jeffbyrnes

This comment has been minimized.

Copy link
Contributor Author

jeffbyrnes commented Aug 25, 2015

@johnbellone ok, I think that should do the trick. I’ve rebased it onto the current master, and added integration tests as well.

johnbellone added a commit that referenced this pull request Aug 28, 2015
Make the TLS certificate management optional
@johnbellone johnbellone merged commit 56de60f into sous-chefs:master Aug 28, 2015
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@jeffbyrnes jeffbyrnes deleted the evertrue:manage_certificate_optional branch Aug 28, 2015
@lock

This comment has been minimized.

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.
Projects
None yet
3 participants
You can’t perform that action at this time.