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

Add support for a KV V2 Secret Metadata resource #1687

Merged
merged 13 commits into from
Jan 5, 2023

Conversation

vinay-gopalan
Copy link
Contributor

@vinay-gopalan vinay-gopalan commented Dec 2, 2022

Adds support for adding custom metadata to a KV V2 secret resource.

Output from acceptance testing:

$ make testacc TESTARGS='-run=TestAccKVSecretV2'
=== RUN   TestAccKVSecretV2
--- PASS: TestAccKVSecretV2 (2.35s)
PASS
...

@github-actions github-actions bot added the size/L label Dec 2, 2022
@joao-oneill-unbabel
Copy link

@vinay-gopalan Seems like this is the same as #1239

@vinay-gopalan
Copy link
Contributor Author

vinay-gopalan commented Dec 5, 2022

@vinay-gopalan Seems like this is the same as #1239

@joao-oneill-unbabel the two are definitely similar! Though the code in this PR slightly differs from the one you mentioned, in that this resource is compatible with the latest dedicated KV V2 Secret engine support that was added to the TFVP in v3.7.0 as opposed to the legacy resource vault_generic_secret 😄

website/docs/r/kv_secret_metadata_v2.html.md Outdated Show resolved Hide resolved
website/docs/r/kv_secret_metadata_v2.html.md Outdated Show resolved Hide resolved
website/docs/r/kv_secret_metadata_v2.html.md Outdated Show resolved Hide resolved
@github-actions github-actions bot added size/L and removed size/XL labels Dec 23, 2022
@vinay-gopalan vinay-gopalan marked this pull request as draft December 23, 2022 21:57
@vinay-gopalan vinay-gopalan marked this pull request as ready for review January 3, 2023 21:21
Copy link
Contributor

@benashz benashz left a comment

Choose a reason for hiding this comment

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

Looking good. Have some initial feedback for you.

website/docs/r/kv_secret_v2.html.md Outdated Show resolved Hide resolved
website/docs/r/kv_secret_backend_v2.html.md Outdated Show resolved Hide resolved
vault/resource_kv_secret_v2.go Outdated Show resolved Hide resolved
vault/resource_kv_secret_v2.go Outdated Show resolved Hide resolved
vault/resource_kv_secret_v2.go Outdated Show resolved Hide resolved
consts.FieldData: {
Type: schema.TypeMap,
Optional: true,
Computed: true,
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this ever be a computed value? Presumably the default is empty.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I wanted to make sure we are able to import the custom metadata and set it to the TF state for the KV secret, specifically in the cases that the secret metadata was created outside of TF and via Vault. That was my thought process around setting this field to also be possibly computed. Is that overkill/unnecessary?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I removed the Computed from the individual nested fields, but still set Computed: true for the parent custom_metadata field

vault/resource_kv_secret_v2.go Outdated Show resolved Hide resolved
vault/resource_kv_secret_v2.go Outdated Show resolved Hide resolved
vault/resource_kv_secret_v2.go Outdated Show resolved Hide resolved
vault/resource_kv_secret_v2_test.go Show resolved Hide resolved
Copy link
Contributor

@benashz benashz left a comment

Choose a reason for hiding this comment

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

A few minor nits to address, then 👍

vault/resource_kv_secret_v2.go Outdated Show resolved Hide resolved
vault/resource_kv_secret_v2.go Outdated Show resolved Hide resolved
website/docs/r/kv_secret_v2.html.md Outdated Show resolved Hide resolved
@vinay-gopalan
Copy link
Contributor Author

1 acceptance test suite is failing due to GH request rate limiting; proceeding to merge since the work done here does not affect the failing tests

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants