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_secret: Raise an exception if Vault read has failed #61

Merged
merged 1 commit into from
Jun 23, 2016

Conversation

legal90
Copy link
Contributor

@legal90 legal90 commented May 17, 2016

I think, like in Chef Data Bags, there should be a run failure if the specified secret could not be read by some reason. Otherwise, it makes people to write additional wrappers for checking the secret value.

@legal90
Copy link
Contributor Author

legal90 commented May 17, 2016

BTW, Seth Vargo has also used raise in his gist

@Ginja
Copy link
Contributor

Ginja commented May 17, 2016

I see why this would be useful, but sometimes secrets are used in one-time resources (e.g. an execute). It would be annoying to have it fail on one of these secrets if there was a temporary issue reaching the Vault server. What do you think about exposing this as another property? exit_on_error and set it to false by default?

Also, I think I'd rather see Chef::Application.fatal!('') than raise. It's more chef-friendly & consistent with how it exits the run. I lied, continue using raise.

@legal90 legal90 force-pushed the raise_on_failure branch from 0d4bdc1 to 93c4316 Compare June 20, 2016 16:06
@codecov-io
Copy link

Current coverage is 73.68%

Merging #61 into master will increase coverage by 22.10%

@@             master        #61   diff @@
==========================================
  Files             2          2          
  Lines            95         95          
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
+ Hits             49         70    +21   
+ Misses           46         25    -21   
  Partials          0          0          

Powered by Codecov. Last updated by 504136d...93c4316

@legal90
Copy link
Contributor Author

legal90 commented Jun 20, 2016

Sorry for a long delay.
I've updated the pull-request by adding exit_on_error option.

It would be annoying to have it fail on one of these secrets if there was a temporary issue reaching the Vault server.

Sorry, I don't think that it is a reason to make this resource loyal to failures like this. If the secret could not be read by any reason, then it should be considered as an error and an operator should be notified. That's why I've set exit_on_error to true by default.

@johnbellone What do you think about it?

@johnbellone johnbellone merged commit 6a4cd02 into sous-chefs:master Jun 23, 2016
@legal90 legal90 deleted the raise_on_failure branch July 1, 2016 11:34
@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

Successfully merging this pull request may close these issues.

4 participants