Consider and use the protection bypass for automation secret from the plan #295
Conversation
vercel/resource_project.go
Outdated
| } | ||
| currentSecret := state.ProtectionBypassForAutomationSecret.ValueString() | ||
| plannedSecret := plan.ProtectionBypassForAutomationSecret.ValueString() | ||
| if state.ProtectionBypassForAutomation != plan.ProtectionBypassForAutomation && currentSecret != plannedSecret { |
There was a problem hiding this comment.
| if state.ProtectionBypassForAutomation != plan.ProtectionBypassForAutomation && currentSecret != plannedSecret { | |
| if state.ProtectionBypassForAutomation != plan.ProtectionBypassForAutomation || currentSecret != plannedSecret { |
should this be an or? I don't think this actually accomplishes rotating the secret unless protection_bypass_for_automation also changes state.
There was a problem hiding this comment.
🤦 Yes, you're absolutely right. Which also implies that this needs to be split up, deleting the secret when it exists but plan.ProtectionBypassForAutomation is false.
There was a problem hiding this comment.
seems like that should be a validation that fails the plan instead of handling at the rpc boundary
… plan This fixes vercel#294. Before this change, the vercel_project resource did not pass on changes to the protection_bypass_for_automation_secret attribute on to the API. This commit contains the following changes: * `client/project_protection_bypass_for_automation_update.go`: Change the anonymous struct member to accurately reflect what is marshalled * `projectResource.Create()`: always save the user-provided secret in the state, even if it is empty. This is required so that the update logic below doesn't try to reset the secret when it is generated by the vercel API. * `projectResource.Update()`: correctly handle the different update scenarios ## Limitations This change stops API-generated secrets to be returned into the state. This will break existing setups that rely on this undocumented fact.
cc4f951 to
f99122c
Compare
|
Hi, I've updated this now to handle all the update scenarios and address two other minor things I've noticed along the way. I'm still not certain if this is fully correct:
I'll need help with that. |
There was a problem hiding this comment.
@DavidS-ovm thanks so much for raising this PR, and the wonderful issue, super clear 💅
I have tested this change, and noticed there are still a few tweaks needed. I raised a draft PR #305, you can take the changes from there and add them to this PR if that is easiest.
Note, to test this locally, these are the steps:
- run
task buildfrom the root of the repo - create a
main.tfin the root of the repo:terraform { required_providers { vercel = { source = "vercel/vercel" } } } provider "vercel" { api_token = "<your team token>" team = "<your team slug>" } resource "vercel_project" "<team name>" { name = "<team name>" protection_bypass_for_automation = true protection_bypass_for_automation_secret = "12345678912345678912345678912347" } - run
terraform applyfrom the root of the repo
This fixes #294.
Before this change, the vercel_project resource did not pass on changes
to the protection_bypass_for_automation_secret attribute on to the API.
This commit contains the following changes:
client/project_protection_bypass_for_automation_update.go: Change the anonymous struct member to accurately reflect what is marshalledprojectResource.Create(): always save the user-provided secret in the state, even if it is empty. This is required so that the update logic below doesn't try to reset the secret when it is generated by the vercel API.projectResource.Update(): correctly handle the different update scenariosLimitations
This change stops API-generated secrets to be returned into the state. This
will break existing setups that rely on this undocumented fact.