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
Collaborator

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

This comment has been minimized.

Copy link
Collaborator Author

commented May 17, 2016

BTW, Seth Vargo has also used raise in his gist

@Ginja

This comment has been minimized.

Copy link
Collaborator

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 legal90:raise_on_failure branch from 0d4bdc1 to 93c4316 Jun 20, 2016
@codecov-io

This comment has been minimized.

Copy link

commented Jun 20, 2016

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

This comment has been minimized.

Copy link
Collaborator Author

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
3 checks passed
3 checks passed
codecov/patch 100% of diff hit (target 51.57%)
Details
codecov/project 73.68% (+22.10%) compared to 504136d
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@legal90 legal90 deleted the legal90:raise_on_failure branch Jul 1, 2016
@lock

This comment has been minimized.

Copy link

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