Add AWS Auth WIF fields #2243
Add AWS Auth WIF fields #2243
Conversation
|
Not sure what happened with the CI tests🫤 |
|
@Zlaticanin TFVP doesn't run the CI build for pushes that don't update any source code files. So changelog-only pushes won't trigger it. Your latest build was https://github.com/hashicorp/terraform-provider-vault/actions/runs/9291889661/job/25571472671 for commit 83ada4d so I think you are good. 👍 |
| Read: provider.ReadWrapper(awsAuthBackendRead), | ||
| Update: awsAuthBackendWrite, | ||
| Delete: awsAuthBackendDelete, | ||
| Exists: awsAuthBackendExists, |
There was a problem hiding this comment.
We need to be careful here since we are changing how TFVP does existence checks. We want to make sure awsAuthBackendRead matches the existence logic from awsAuthBackendExists. If we do decide to change something, we should probably make a Change entry in the changelog.
There was a problem hiding this comment.
+1 on being mindful when removing Exists methods. There shouldn't be a change in behavior in this case, since the Read method matches the existence logic from Exists. We should be okay to not call things out in the CL explicitly.
It is also beneficial to update all of these methods to Context functions instead, especially the Read method. According to the docs, Exists has been deprecated in favor of ReadContext.
Looks like we already made the upgrades, so we should be good 👍🏼
There was a problem hiding this comment.
LGTM! Had a couple of comments, but none of them are blocking.
FYI, for full WIF functionality in Auth engines, we also need to add the identity_token_key field to the Vault Auth Backend resource. I am taking care of that in the GCP Auth PR though, so I'm good with moving ahead with this PR's updates
| Read: provider.ReadWrapper(awsAuthBackendRead), | ||
| Update: awsAuthBackendWrite, | ||
| Delete: awsAuthBackendDelete, | ||
| Exists: awsAuthBackendExists, |
There was a problem hiding this comment.
+1 on being mindful when removing Exists methods. There shouldn't be a change in behavior in this case, since the Read method matches the existence logic from Exists. We should be okay to not call things out in the CL explicitly.
It is also beneficial to update all of these methods to Context functions instead, especially the Read method. According to the docs, Exists has been deprecated in favor of ReadContext.
Looks like we already made the upgrades, so we should be good 👍🏼
No description provided.