fix(push_group): save state after deactivating during delete - #2904
Open
exitcode0 wants to merge 1 commit into
Open
fix(push_group): save state after deactivating during delete#2904exitcode0 wants to merge 1 commit into
exitcode0 wants to merge 1 commit into
Conversation
Delete makes two mutating API calls: it deactivates the mapping (the Okta API only allows deleting an INACTIVE mapping) and then deletes it. No state was written between them, so when the deactivation succeeded and the delete then failed (for example the transport giving up after repeated 429s in a rate-limited org), Terraform preserved the prior state recording status = "ACTIVE" while the mapping in Okta was INACTIVE and had stopped pushing group membership downstream. State written during Delete is preserved when error diagnostics are returned and discarded when Delete reports no errors, so recording the new status between the two calls keeps state accurate on the failure path without affecting the success path. The two calls also reported the same error summary, which gave the operator no way to tell "could not deactivate" from "deactivated but could not delete". They are now distinct, and the delete failure says that group membership is no longer being pushed downstream.
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.
Fixes #2903
Deleteforokta_push_groupmakes two mutating API calls: it deactivates the mapping (the Okta API only allows deleting anINACTIVEmapping, as the provider's own guard message says) and then deletes it. No state was written between them.If the deactivation succeeds and the delete then fails (for example the transport giving up after repeated 429s in a rate-limited org, or a 5xx),
Deletereturns error diagnostics without writing state, so Terraform preserves the prior state:INACTIVEand has stopped pushing group membership to the downstream applicationstatus = "ACTIVE"failed to delete push group mapping: ..., identical to the message emitted when the deactivation itself fails, with no indication that group push has stoppedIt does self-heal on the next successful apply, but silently, and only if someone re-runs.
Changes
Deleterecordsstatus = "INACTIVE"in the response state before attempting the deletefailed to deactivate push group mapping before deleteandfailed to delete push group mapping, and the latter states that the mapping is no longer pushing group membership downstreamState written during
Deleteis preserved by Terraform when error diagnostics are returned, and discarded whenDeletereports no errors, so the write affects only the failure path. Ininternal/fwserver/server_deleteresource.go(framework v1.18.0) the response state is seeded from prior state,RemoveResourceis called only when there are no error diagnostics, andresp.NewStateis assigned unconditionally. See the plugin framework Delete documentation.Testing
gofmt -l,go build ./...,go vet ./okta/...— cleango test ./okta/...(unit, noTF_ACC) — passTestAccResourceOktaPushGroup_crudreplayed locally against the existing cassette (OKTA_VCR_TF_ACC=play TF_ACC=1,classic-00) — PASS. It exercises the deactivate-then-delete path, and no new HTTP calls are introduced, so the cassette replays unchangedNotes
Read/Update/Deleteof this resource; that is a different failure mode (mapping already gone) and is not duplicated here. The changes are complementary and should merge cleanly in either order, with a small textual conflict at most around the deactivation error return