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

Issue1252: vault write broken #1257

Merged
merged 2 commits into from Aug 13, 2019
Merged

Issue1252: vault write broken #1257

merged 2 commits into from Aug 13, 2019

Conversation

eikenb
Copy link
Contributor

@eikenb eikenb commented Aug 13, 2019

Writing to a Vault KV v1 and v2 were both broken.

Specifically to reproduce GH-1252 in a test.
@eikenb eikenb added bug vault Related to the Vault integration labels Aug 13, 2019
@eikenb eikenb added this to the 0.21.1 milestone Aug 13, 2019
@eikenb eikenb requested review from a team August 13, 2019 00:28
Copy link

@freddygv freddygv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good overall @eikenb, added some minor questions inline.

dependency/vault_write_test.go Show resolved Hide resolved
dependency/vault_write_test.go Show resolved Hide resolved
dependency/vault_write.go Outdated Show resolved Hide resolved
dependency/vault_write_test.go Outdated Show resolved Hide resolved
@eikenb eikenb force-pushed the issue1252-vault-write-broken branch from 4a84b72 to c0b4e2d Compare August 13, 2019 17:40
Writing to a Vault KV v1 and v2 were both broken.

V1 had a nil pointer dereference panic in Fetch where it tried to access
variables on the replied Secret which is always nil with V1.

V2 didn't get the sent in data structure correct as it didn't have the
additional map[string]interface{"data":...} wrapping around the values
that is required by V2.

Fixes #1252
@eikenb eikenb force-pushed the issue1252-vault-write-broken branch from c0b4e2d to 884e161 Compare August 13, 2019 17:45
Copy link

@freddygv freddygv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@eikenb eikenb merged commit 016658e into master Aug 13, 2019
@eikenb eikenb deleted the issue1252-vault-write-broken branch August 13, 2019 19:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug vault Related to the Vault integration
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants