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

Update Sync Association resource to include sync metadata for all subkeys #2202

Merged
merged 12 commits into from
Mar 26, 2024

Conversation

vinay-gopalan
Copy link
Contributor

@vinay-gopalan vinay-gopalan commented Mar 21, 2024

Description

Previously, the association resource did not account for the secret-key granularity, which would create a sync target for each subkey of a KV secret. Now, the association resource must keep track of the sync statuses for all subkeys of this associated secret.

Notable improvements/changes:

  • Adds a new list schema field metadata which tracks the association metadata for each subkey (sync_status, updated_at)
  • Removes the schema types sync_status and updated_at
  • Adds granularity to all destination resources.
  • Makes the Association resource importable. Now, by providing the ID with the right format (encoding all necessary info), TFVP can enable a user to import their association metadata to track the lifecycle of all subkeys of an associated secret.
  • Fixes some tests

Checklist

  • Added CHANGELOG entry (only for user-facing changes)
  • Acceptance tests where run against all supported Vault Versions

Output from acceptance testing:

$ make testacc TESTARGS='-run=TestSecretsSyncAssociation_gh'
=== RUN   TestSecretsSyncAssociation_gh
    resource_secrets_sync_association_test.go:38: Vault server version "1.16.0+ent"
--- PASS: TestSecretsSyncAssociation_gh (2.67s)
PASS

@@ -77,8 +77,6 @@ resource "vault_secrets_sync_gh_destination" "test" {
access_token = "%s"
repository_owner = "%s"
repository_name = "%s"
app_name = "test-app-name"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed these from this test since these require additional setup in Vault and Secrets Sync for this to work, which was causing the test to fail.

@vinay-gopalan vinay-gopalan changed the title Update Sync Association resource to include sync status for all subkeys Update Sync Association resource to include sync metadata for all subkeys Mar 25, 2024
Copy link
Contributor

@fairclothjm fairclothjm left a comment

Choose a reason for hiding this comment

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

LGTM

@vinay-gopalan vinay-gopalan added this to the 4.2.0 milestone Mar 25, 2024
internal/consts/consts.go Show resolved Hide resolved
vault/resource_secrets_sync_association.go Show resolved Hide resolved
vault/resource_secrets_sync_association_test.go Outdated Show resolved Hide resolved
vault/resource_secrets_sync_aws_destination.go Outdated Show resolved Hide resolved
website/docs/r/secrets_sync_association.html.md Outdated Show resolved Hide resolved
website/docs/r/secrets_sync_association.html.md Outdated Show resolved Hide resolved
Copy link
Member

@robmonte robmonte left a comment

Choose a reason for hiding this comment

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

LGTM just my prior comments

internal/consts/consts.go Show resolved Hide resolved
@vinay-gopalan vinay-gopalan mentioned this pull request Mar 26, 2024
2 tasks
@vinay-gopalan vinay-gopalan merged commit c89a45a into main Mar 26, 2024
12 checks passed
@vinay-gopalan vinay-gopalan deleted the VAULT-25228/add-association-subkeys branch March 26, 2024 20:45
@raymonstah
Copy link
Contributor

LGTM!

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