gcp: skip terracurl destroy for migration exec instead of failing on stale token - #280
Open
ankur512512 wants to merge 1 commit into
Open
gcp: skip terracurl destroy for migration exec instead of failing on stale token#280ankur512512 wants to merge 1 commit into
ankur512512 wants to merge 1 commit into
Conversation
…stale token The destroy_url/destroy_method/destroy_headers on terracurl_request.exec send an Authorization bearer token that was captured at create/apply time via data.google_client_config.default.access_token. That token is baked into resource state and will have expired (OAuth access tokens are short-lived, ~1 hour) by the time this resource is actually destroyed in most real deployments, since exec_migration is typically only flipped back to false well after the initial apply. When that happens, GCP rejects the destroy request, and the terracurl provider's retry loop fails to record the actual response as the retry error (a bug in the provider itself: internal/provider/resource_curl.go sets lastError = err after a non-matching response code, but err is still nil from the preceding successful client.Do() call), so the surfaced error is an unhelpful "request failed, retries exceeded: <nil>" with no indication it was a stale-token 401/403. The destroy call was already documented as a no-op formality (it never undoes the migration), so destroy_skip = true avoids the fragile path entirely instead of retrying a request that is expected to fail.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
terracurl_request.exec(ingcp/byo-project/cloud_run.tf, used to trigger the Fleet DB migration job) wires up a realdestroy_url/destroy_method/destroy_headersfor its destroy step, but theAuthorizationbearer token indestroy_headersis captured once at create/apply time viadata.google_client_config.default.access_tokenand frozen into resource state.exec_migrationis usually only flipped back tofalsewell after the initial apply, so by the time this resource is destroyed, the stored token has expired and GCP rejects the destroy request.terraform apply/destroyfails with an unhelpfulError: unable to make request: request failed, retries exceeded: %!s(<nil>)— no indication that it was actually a stale-token 401/403. (Root cause traced into theterracurlprovider itself:resource_curl.go's destroy retry loop setslastError = errafter getting back a non-matching response code, buterris stillnilfrom the preceding successfulclient.Do()call a few lines up — the real per-attempt error is never captured.)destroy_skip = trueand drops the now-unuseddestroy_url/destroy_method/destroy_response_codes/destroy_headersattributes.Reproduction (hit this on a real deploy)
terraform applywithfleet_config.exec_migration = trueto run initial migrations.exec_migration = falseandterraform applyagain to remove the trigger resource.retries exceeded: <nil>error above; a manualcurlwith a freshly minted token against the same destroy URL succeeds immediately with200, confirming it's a stale-token issue, not a permissions/network one.Test plan
terraform fmt -check -diffongcp/byo-project— cleanterraform init && terraform validateongcp/byo-project— succeedsterraform applyno longer attempts the fragile destroy path whenexec_migrationis toggled off after the token would otherwise have expired.Rollback
Revert this commit — restores the previous
destroy_url/destroy_method/destroy_response_codes/destroy_headersblock exactly as it was.