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

Read paths unmodified if KVv2 status check fails #1253

Merged
merged 1 commit into from
Aug 9, 2019

Conversation

notnoop
Copy link

@notnoop notnoop commented Aug 9, 2019

When vault.read fails to determine whether a path is KVv2, assume it's
KVv1 and read from the path as passed from user.

Here we fallback to behavior prior to #1180,
so KVv2 check is for enhancement only. If user truely lacks access, the
secret lookup call would fail and we report error back.

A customer hit a regression after they upgraded nomad with updated
consul-template, where their templates failed to read approle info
because isKVv2 returned an error. The case is reproduced in
TestVaultReadQuery_Fetch_NonSecrets test.

When vault.read fails to determine whether a path is KVv2, assume it's
KVv1 and read from the path as passed from user.

Here we fallback to behavior prior to hashicorp#1180,
so KVv2 check is for enhancement only.  If user truely lacks access, the
secret lookup call would fail and we report error back.

A customer hit a regression after they upgraded nomad with updated
consul-template, where their templates failed to read approle info
because isKVv2 returned an error.  The case is reproduced in
`TestVaultReadQuery_Fetch_NonSecrets` test.
@notnoop
Copy link
Author

notnoop commented Aug 9, 2019

FWIW, I was very conservative in #1231 change - this fully rollback behavior to prior to #1180 more generally to all errors and logs them. I kept the special casing for anonymous requests to avoid unnecessary warn logging.

@eikenb
Copy link
Contributor

eikenb commented Aug 9, 2019

I like this change better than what cv and I talked about yesterday. Simple and clear.
👍

@eikenb eikenb merged commit 1bdf012 into hashicorp:master Aug 9, 2019
@notnoop notnoop deleted the b-ignore-vault-read-kv2-err branch August 10, 2019 17:05
notnoop pushed a commit to hashicorp/nomad that referenced this pull request Aug 12, 2019
Pick up hashicorp/consul-template#1253

Fixed dependencies related to `github.com/burntsushi/toml` to
`github.com/burntsushi/toml` change.
@eikenb eikenb added the nomad Related to ingetration in Nomad label Aug 12, 2019
@eikenb eikenb added this to the 0.21.1 milestone Aug 12, 2019
tgross added a commit to hashicorp/nomad that referenced this pull request Aug 12, 2019
pulls in configuration option for blacklisting template functions from:
hashicorp/consul-template#1243
hashicorp/consul-template#1246

pulls in configuration option for file sandboxing from:
hashicorp/consul-template#1249
hashicorp/consul-template#1254

pulls in vault KVv2 read fixes from:
hashicorp/consul-template#1253
tgross added a commit to hashicorp/nomad that referenced this pull request Aug 12, 2019
pulls in configuration option for blacklisting template functions from:
hashicorp/consul-template#1243
hashicorp/consul-template#1246

pulls in configuration option for file sandboxing from:
hashicorp/consul-template#1249
hashicorp/consul-template#1254

pulls in vault KVv2 read fixes from:
hashicorp/consul-template#1253
tgross added a commit to hashicorp/nomad that referenced this pull request Aug 12, 2019
pulls in configuration option for blacklisting template functions from:
hashicorp/consul-template#1243
hashicorp/consul-template#1246

pulls in configuration option for file sandboxing from:
hashicorp/consul-template#1249
hashicorp/consul-template#1254

pulls in vault KVv2 read fixes from:
hashicorp/consul-template#1253
tgross added a commit to hashicorp/nomad that referenced this pull request Aug 12, 2019
pulls in configuration option for blacklisting template functions from:
hashicorp/consul-template#1243
hashicorp/consul-template#1246

pulls in configuration option for file sandboxing from:
hashicorp/consul-template#1249
hashicorp/consul-template#1254

pulls in vault KVv2 read fixes from:
hashicorp/consul-template#1253
@eikenb eikenb added the bug label Aug 13, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug nomad Related to ingetration in Nomad
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants