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

fix KVv2 data source when specifying a version #1677

Merged
merged 2 commits into from
Dec 2, 2022
Merged

Conversation

surian
Copy link
Contributor

@surian surian commented Nov 23, 2022

Fixes #1612 by using ReadWithData instead of using ?version= in path with Read leading to a bad URL encoding when request is submitted to Vault.

@hashicorp-cla
Copy link

hashicorp-cla commented Nov 23, 2022

CLA assistant check
All committers have signed the CLA.

Copy link
Contributor

@swenson swenson 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 great! Would it be possible to add a unit test for this as well?

@github-actions github-actions bot added size/M and removed size/XS labels Dec 1, 2022
@surian
Copy link
Contributor Author

surian commented Dec 1, 2022

This looks great! Would it be possible to add a unit test for this as well?

I added a unit test. Same test as the current one but testing by providing the version parameter.

Without the patch:

=== RUN   TestDataSourceKVV2Secret
    data_source_kv_secret_v2_test.go:21: Step 2/2 error: Error running pre-apply refresh: exit status 1

        Error: no secret found at "tf-kv-4323879699321682032/data/foo-1286120807726163543?version=1"

          on terraform_plugin_test.tf line 27, in data "vault_kv_secret_v2" "test":
          27: data "vault_kv_secret_v2" "test" {


--- FAIL: TestDataSourceKVV2Secret (4.81s)

With the patch:

=== RUN   TestDataSourceKVV2Secret
--- PASS: TestDataSourceKVV2Secret (7.23s)
PASS

@surian surian requested a review from swenson December 1, 2022 17:46
Copy link
Contributor

@swenson swenson left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks!

@swenson swenson merged commit 9e6d309 into hashicorp:main Dec 2, 2022
@benashz benashz added this to the 3.12.0 milestone Dec 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

data.vault_kv_secret_v2 version arg urlencodes "?"
4 participants