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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix race condition on duplicate entity alias creation #1429

Merged
merged 9 commits into from
May 2, 2022

Conversation

benashz
Copy link
Contributor

@benashz benashz commented Apr 28, 2022

Fix race condition on duplicate entity alias creation

The creation of entity aliases should be serialized, since we want to
handle the case when the user is attempting to create duplicate entity
aliases during the same Teraform execution. A duplicate entity alias
shares the same mount accessor and name with another entity alias.

The fix is to lock the /identity/entity-alias/id path on all create,
update, and delete operations. This allows the provider to reliably
detect the duplicate entity alias condition, and prevent duplicate
alias creation.

Other fixes:

  • all CRUD functions are of the *Context type, and can return meaningful
    diag.Diagnostics
  • created a new entity package under internal/identity/entity with some
    helper functions

Community Note

  • Please vote on this pull request by adding a 馃憤 reaction to the original pull request comment to help the community and maintainers prioritize this request
  • Please do not leave "+1" comments, they generate extra noise for pull request followers and do not help prioritize the request

Closes #669

Release note for CHANGELOG:


Output from acceptance testing:

$ make testacc TESTARGS='-v -test.run TestAccIdentityEntity*'

==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test -v -v -test.run TestAccIdentityEntity* -timeout 30m ./...

ok      github.com/hashicorp/terraform-provider-vault/util      (cached) [no tests to run]
=== RUN   TestAccIdentityEntityAlias
--- PASS: TestAccIdentityEntityAlias (3.08s)
=== RUN   TestAccIdentityEntityAliasDuplicateFlow
--- PASS: TestAccIdentityEntityAliasDuplicateFlow (6.13s)
=== RUN   TestAccIdentityEntityAlias_Update
--- PASS: TestAccIdentityEntityAlias_Update (3.06s)
=== RUN   TestAccIdentityEntityAlias_Metadata
--- PASS: TestAccIdentityEntityAlias_Metadata (3.07s)
=== RUN   TestAccIdentityEntityPoliciesExclusive
--- PASS: TestAccIdentityEntityPoliciesExclusive (2.98s)
=== RUN   TestAccIdentityEntityPoliciesNonExclusive
--- PASS: TestAccIdentityEntityPoliciesNonExclusive (2.96s)
=== RUN   TestAccIdentityEntity
--- PASS: TestAccIdentityEntity (1.58s)
=== RUN   TestAccIdentityEntityUpdate
--- PASS: TestAccIdentityEntityUpdate (2.81s)
=== RUN   TestAccIdentityEntityUpdateRemoveValues
--- PASS: TestAccIdentityEntityUpdateRemoveValues (2.82s)
=== RUN   TestAccIdentityEntityUpdateRemovePolicies
--- PASS: TestAccIdentityEntityUpdateRemovePolicies (2.85s)
PASS
ok      github.com/hashicorp/terraform-provider-vault/vault     31.782s


...

The creation of entity aliases should be serialized, since we want to
handle the case when the user is attempting to create duplicate entity
aliases during the same Teraform execution. A duplicate entity alias
shares the same mount accessor and name with another entity alias.

The fix is to lock the /identity/entity-alias/id path on all create,
update, and delete operations. This allows the provider to reliably
detect the duplicate entity alias condition, and prevent duplicate
alias creation.

Other fixes:
- all CRUD functions are of the *Context type, and can return meaning
  diag.Diagnostics
- created a new entity package under internal/identity/entity with some
  helper functions
Copy link
Contributor

@vinay-gopalan vinay-gopalan left a comment

Choose a reason for hiding this comment

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

LGTM! Great work 馃槃

vault/resource_identity_entity_alias.go Outdated Show resolved Hide resolved
@benashz benashz merged commit 58e9721 into main May 2, 2022
@benashz benashz deleted the VAULT-4878/fix-id-entity-alias-updates branch May 2, 2022 20:55
@benashz benashz added this to the 3.6.0 milestone May 18, 2022
marcboudreau pushed a commit to marcboudreau/terraform-provider-vault that referenced this pull request Nov 6, 2022
The creation of entity aliases should be serialized, since we want to
handle the case when the user is attempting to create duplicate entity
aliases during the same Terraform execution. A duplicate entity alias
shares the same mount accessor and name with another entity alias.

The fix is to lock the /identity/entity-alias/id path on all create,
update, and delete operations. This allows the provider to reliably
detect the duplicate entity alias condition, and prevent duplicate
alias creation.

Other fixes:
- all CRUD functions are of the *Context type, and can return meaning
  diag.Diagnostics
- created a new entity package under internal/identity/entity with some
  helper functions
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.

vault_identity_entity_alias fails update in place
2 participants