Fix panic in aws_route53recoverycontrolconfig_safety_rule create on API error - #6578
Draft
skymoore wants to merge 1 commit into
Draft
Fix panic in aws_route53recoverycontrolconfig_safety_rule create on API error#6578skymoore wants to merge 1 commit into
skymoore wants to merge 1 commit into
Conversation
Carries hashicorp/terraform-provider-aws#49155. The safety_rule create path dereferenced the CreateSafetyRule response before checking the returned error, so any API error (for example ServiceQuotaExceededException) panicked the provider and cascaded connection-refused failures to other in-flight resources. Tracking issue: pulumi#6577
1 task
Contributor
|
PR is now waiting for a maintainer to run the acceptance tests. |
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
aws_route53recoverycontrolconfig_safety_rulepanics with a nil pointer dereference whenCreateSafetyRulereturns an error.createAssertionRuleandcreateGatingRuleassignresult := output.AssertionRule/output.GatingRulebefore checking the error, so whenoutputis nil the dereference crashes the provider. Through the bridge this takes down the plugin process and cascadesconnect: connection refusedto every other in-flight resource. It is easy to hit: Route 53 ARC limits a control panel to 20 safety rules, and exceeding it returnsServiceQuotaExceededException.patches/0030-Fix-panic-in-route53recoverycontrolconfig-safety_rul.patch, which moves theresultassignment below the error check and the nil-response check in both functions, folding anoutput == nilguard into the existing nil-response check. Carries the upstream fix verbatim.This PR is intentionally opened as a draft pending maintainer guidance on the required regression coverage and CI selection (see the open items below).
Change Type
provider/)upstream/,patches/,scripts/upstream.sh).ci-mgmt.yaml)Carried Upstream Patch
route53recoverycontrolconfigpackage has no unit-test harness for the create path, and its acceptance tests exercise live safety rules against the 20-per-control-panel limit. Asserting the panic-to-diagnostic behavior needs an account with an already-full control panel. Requesting guidance on whether a live test is warranted for a statement-reorder crash fix..github/workflows/aws-upstream-tests.yml: pending the decision above.Validation Evidence
The patch applies cleanly against the pinned upstream submodule, and the fix was verified end to end against live AWS through the bridge (pulumi-aws 7.39.0 with this patch built into the provider binary). Before the patch, creating a safety rule on a full control panel crashed with SIGSEGV. After the patch, the same operation returned the underlying diagnostic:
I have not run
make test_providerormake lintin this branch: the change is a patch file only, with noprovider/Go changes, and the underlying reorder is validated upstream in hashicorp/terraform-provider-aws#49155. I will run any repository-specific validation the maintainers require before this leaves draft.Command output snippets
Risk
internal/service/route53recoverycontrolconfig/safety_rule.go. No success-path behavior change; only the previously unreachable error path changes from panic to a returned diagnostic.output == niland non-nil-but-empty response shapes.Rollback
patches/0030-*.patchand rerun the patch pipeline.AI assistance disclosure: I used an AI coding assistant to help trace the panic to the faulty statement ordering, draft the patch header, and draft this description. I own and reviewed the change; it is a statement reorder in two functions and I understand its behavior and removal conditions.