identity/group-alias: add api client lock #2140
Conversation
| @@ -5,6 +5,9 @@ FEATURES: | |||
| * Add support for configuration of plugin WIF to the AWS Secret Backend. Requires Vault 1.16+ ([#2138](https://github.com/hashicorp/terraform-provider-vault/pull/2138)). | |||
| * Add support for Oracle database plugin configuration options `split_statements` and `disconnect_sessions`: ([#2085](https://github.com/hashicorp/terraform-provider-vault/pull/2085)) | |||
|
|
|||
| IMPROVEMENTS: | |||
There was a problem hiding this comment.
Or BUG? I went with improvement since I have not been able to verify that this fixes the reported error.
|
Looking into the test timeout failures: https://github.com/hashicorp/terraform-provider-vault/actions/runs/7890026265/job/21531339605 |
There was a problem hiding this comment.
Thanks for looking into this! Though it is hard to come up with a testcase using just the TF testing framework that can validate the exact fix for the issue, I do think this is a valid addition to the code, as it matches the Group Alias resource behavior to the Entity Alias resource (both of which support performance secondaries and standbys). Had 1 nit but looks good to me otherwise 👍🏼
| @@ -49,6 +52,10 @@ func identityGroupAliasResource() *schema.Resource { | |||
| } | |||
|
|
|||
| func identityGroupAliasCreate(d *schema.ResourceData, meta interface{}) error { | |||
| lock, unlock := getEntityLockFuncs(d, identityGroupAliasIDPath) | |||
There was a problem hiding this comment.
nit: is it worth updating the function name to something more generic, like getLockFuncs since it is not specific to entities?
There was a problem hiding this comment.
I think I will keep it as-is since an Identity Group is a collection of Entities and the lock keys that we create are specific to entities in that the key is tied to the mount accessor. I don't see this function being used much or at all outside of entities use cases.
Description
Relates #1695
I haven't been able to reproduce the issue from #1695 yet, but the lack of locking here is quite suspicious.
Checklist