Skip to content

fix: Instead of panicking, the code now handles duplicates gracefully:#38085

Open
pgaijin66 wants to merge 3 commits intohashicorp:mainfrom
pgaijin66:fix-duplicate-status-report-panic
Open

fix: Instead of panicking, the code now handles duplicates gracefully:#38085
pgaijin66 wants to merge 3 commits intohashicorp:mainfrom
pgaijin66:fix-duplicate-status-report-panic

Conversation

@pgaijin66
Copy link

Summary

Replace panic with graceful error handling when duplicate check status reports occur in the internal checks system.

Background

This panic was encountered during a terraform apply operation that was recreating AWS ACM certificates and API Gateway resources. During the apply phase, Terraform's internal checks system attempted to report the same resource precondition status multiple times for the same resource, triggering a panic that crashed the entire apply operation.

What Was Happening

Running the following command:

cd "$TF_DIR"                                                                                                                                                      
terraform apply -auto-approve tfplan                                                                                                                              

The Terraform plan was recreating production ACM certificates that were in PENDING_VALIDATION status along with their associated API Gateway resources. During concurrent resource evaluation, Terraform's checks system received duplicate status reports for the same precondition check on the ACM certificate resource.

Root Cause

The issue occurs in internal/checks/state_report.go:114 where the code panics when it detects that a check status has already been reported (i.e., the status is not StatusUnknown). In concurrent execution scenarios, particularly during resource recreation or when evaluating resource preconditions, it's possible for the same check to be reported multiple times due to race conditions or retry logic in the graph walker.

The previous implementation assumed this should never happen and treated it as a fatal error, causing Terraform to crash instead of handling the duplicate gracefully.

Changes

  • When the same status is reported twice for a check, log a warning and continue without updating (likely a benign race condition)
  • When a different status is reported, log an error and keep the first status (first-write-wins semantics)
  • Add comprehensive test coverage for both duplicate report scenarios

Error Before Fix

!!!!!!!!!!!!!!!!!!!!!!!!!!! TERRAFORM CRASH !!!!!!!!!!!!!!!!!!!!!!!!!!!!                                                                                          
                                                                                                                                                                  
Terraform crashed! This is always indicative of a bug within Terraform.                                                                                           
Please report the crash with Terraform[1] so that we can fix this.                                                                                                
                                                                                                                                                                  
When reporting bugs, please include your terraform version, the stack trace                                                                                       
shown below, and any additional information which may help replicate the issue.                                                                                   
                                                                                                                                                                  
[1]: https://github.com/hashicorp/terraform/issues                                                                                                                
                                                                                                                                                                  
!!!!!!!!!!!!!!!!!!!!!!!!!!! TERRAFORM CRASH !!!!!!!!!!!!!!!!!!!!!!!!!!!!                                                                                          
                                                                                                                                                                  
duplicate status report for module.production.module.admin_certificate.aws_acm_certificate.this[0] ResourcePrecondition 0                                         
goroutine 993 [running]:                                                                                                                                          
runtime/debug.Stack()                                                                                                                                             
     /opt/hostedtoolcache/go/1.21.3/x64/src/runtime/debug/stack.go:24 +0x5e                                                                                       
runtime/debug.PrintStack()                                                                                                                                        
     /opt/hostedtoolcache/go/1.21.3/x64/src/runtime/debug/stack.go:16 +0x13                                                                                       
github.com/hashicorp/terraform/internal/logging.PanicHandler()                                                                                                    
     /home/runner/work/terraform/terraform/internal/logging/panic.go:58 +0x13b                                                                                    
panic({0x2b98480?, 0xc0024fa390?})                                                                                                                                
     /opt/hostedtoolcache/go/1.21.3/x64/src/runtime/panic.go:920 +0x270                                                                                           
github.com/hashicorp/terraform/internal/checks.(*State).reportCheckResult(0xc0036a3068, {0x390ead8, 0xc005ff7810}, 0xc008604e10?, 0x0, 0x50)                      
     /home/runner/work/terraform/terraform/internal/checks/state_report.go:113 +0x234                                                                             
github.com/hashicorp/terraform/internal/checks.(*State).ReportCheckResult(0xc0000d67b0?, {0x390ead8?, 0xc005ff7810?}, 0xc00175dcd0?, 0x1?, 0x0?)                  
     /home/runner/work/terraform/terraform/internal/checks/state_report.go:71 +0xc5                                                                               
github.com/hashicorp/terraform/internal/terraform.evalCheckRules(0x38faff8?, {0xc000a84540, 0x1, 0xc000f3e0c0?}, {0x3927380, 0xc000fdbce0}, {0x390ead8,           
0xc005ff7810}, {{{{0x3909eb0, 0xc000012c58}}, ...}, ...}, ...)                                                                                                    
     /home/runner/work/terraform/terraform/internal/terraform/eval_conditions.go:61 +0x45b                                                                        
github.com/hashicorp/terraform/internal/terraform.(*NodeAbstractResourceInstance).plan(0xc000a0e600, {0x3927380, 0xc000fdbce0}, 0xc005e849c0, 0x0, 0x0, {0x0,     
0x0, 0x101?})                                                                                                                                                     
     /home/runner/work/terraform/terraform/internal/terraform/node_resource_abstract_instance.go:699 +0x4a5                                                       
github.com/hashicorp/terraform/internal/terraform.(*NodeApplyableResourceInstance).managedResourceExecute(0xc0071ceb80, {0x3927380, 0xc000fdbce0})                
     /home/runner/work/terraform/terraform/internal/terraform/node_resource_apply_instance.go:278 +0xa72                                                          
github.com/hashicorp/terraform/internal/terraform.(*NodeApplyableResourceInstance).Execute(0x7f972522ec28?, {0x3927380?, 0xc000fdbce0?}, 0x80?)                   
     /home/runner/work/terraform/terraform/internal/terraform/node_resource_apply_instance.go:145 +0x9a                                                           
github.com/hashicorp/terraform/internal/terraform.(*ContextGraphWalker).Execute(0xc007f846c0, {0x3927380, 0xc000fdbce0}, {0x7f97252dea60, 0xc0071ceb80})          
     /home/runner/work/terraform/terraform/internal/terraform/graph_walk_context.go:143 +0xbe                                                                     
github.com/hashicorp/terraform/internal/terraform.(*Graph).walk.func1({0x314fce0, 0xc0071ceb80})                                                                  
     /home/runner/work/terraform/terraform/internal/terraform/graph.go:78 +0x375                                                                                  
github.com/hashicorp/terraform/internal/dag.(*Walker).walkVertex(0xc0018ce660, {0x314fce0, 0xc0071ceb80}, 0xc001862440)                                           
     /home/runner/work/terraform/terraform/internal/dag/walk.go:384 +0x2e5                                                                                        
created by github.com/hashicorp/terraform/internal/dag.(*Walker).Update in goroutine 218                                                                          
     /home/runner/work/terraform/terraform/internal/dag/walk.go:307 +0xde8                                                                                        

Test Plan

Added TestChecksDuplicateReport that verifies:

  • Duplicate reports with the same status don't panic
  • Duplicate reports with different statuses don't panic
  • First status is preserved when conflicts occur (first-write-wins semantics)

Impact

This change makes Terraform more resilient to duplicate or conflicting check reports while maintaining data integrity. It prevents unnecessary crashes during legitimate concurrent execution scenarios, particularly when applying plans that involve resource recreation or complex dependency graphs.

Target Release

1.15.x

Rollback Plan

  • If a change needs to be reverted, we will roll out an update to the code within 7 days.

Changes to Security Controls

Are there any changes to security controls (access controls, encryption, logging) in this pull request? If so, explain.

CHANGELOG entry

  • This change is user-facing and I added a changelog entry.
  • This change is not user-facing.

@pgaijin66 pgaijin66 requested a review from a team as a code owner January 22, 2026 06:15
@hashicorp-cla-app
Copy link

hashicorp-cla-app bot commented Jan 22, 2026

CLA assistant check
All committers have signed the CLA.

@hashicorp-cla-app
Copy link

CLA assistant check

Thank you for your submission! We require that all contributors sign our Contributor License Agreement ("CLA") before we can accept the contribution. Read and sign the agreement

Learn more about why HashiCorp requires a CLA and what the CLA includes

Have you signed the CLA already but the status is still pending? Recheck it.

@pgaijin66 pgaijin66 marked this pull request as draft January 22, 2026 06:20
@pgaijin66 pgaijin66 force-pushed the fix-duplicate-status-report-panic branch from b318390 to 0e12a87 Compare January 22, 2026 06:26
jar-b and others added 2 commits January 21, 2026 22:26
Incrementing the `aws-sdk-go-base` dependecy will also pull in the latest version of the AWS SDK for Go V2, which includes support for the newly launched `aws login` authentication method.
- Logging a warning and continuing if the same status is reported twice
- Logging an error and preserving the first status if conflicting statuses
  are reported (first-write-wins semantics)
@pgaijin66 pgaijin66 force-pushed the fix-duplicate-status-report-panic branch from 0e12a87 to 29a4403 Compare January 22, 2026 06:26
@pgaijin66 pgaijin66 marked this pull request as ready for review January 22, 2026 06:29
@mildwonkey
Copy link
Contributor

Hello @pgaijin66 ! Thank you for submitting this.

I have a concern about your report: if this issue is sometimes triggered by a race condition, we must look into that. A race condition in terraform is never benign 😅 ! I would rather leave the panic logic (replacing it with a DuplicateCheck error, but still causing failures) and fix the underlying issue. Race conditions that seem benign in one configuration can be disastrous in a larger one, so we don't just paper over them.

We can add handling for duplicate entires, but it's important that we do so deliberately - terraform should know that it's processing streaming events and in that case use a different method (ReportPossiblyDuplicateCheck 🤷🏻). If we're still getting a check twice for other reasons, we can return the previously-mentioned DuplicateCheck error.

I'd also question your assertion that the first check result is the one saved, and this is a genuine question - if it's due to a retry, isn't it possible that what we want is the "final" check value? If the resource is slow to provision I would think it's possible the first check value wouldn't have all the information.

@pgaijin66
Copy link
Author

@mildwonkey Thank you for the thorough feedback, you're absolutely right that we shouldn't paper over race conditions. I appreciate you taking the time to explain the deeper concerns here.

After reading your feedback, I agree that my current approach is treating the symptom rather than the disease. A race condition that seems benign in my specific scenario could absolutely cause serious issues in larger configurations.

Would you prefer I close this PR and open a new one after proper investigation? Or should I update this PR with findings and a revised approach?

I want to get this right rather than rush a fix that might mask real problems.

@mildwonkey
Copy link
Contributor

Excellent, and thank you for understanding! (fixing a panic is a good instinct no matter how you look at it 😁)

🤔 I think the easiest would be if you updated this pr to return an error instead of panic when duplicate checks are registered, and then someone (you, the core team, some other kindly contributor) can look into the other items in follow up PRs. That or open a new PR, either works!

If you have any more information on the race condition you mentioned, that would be greatly appreciated, however you wish to share it. A reproduction or even a test that fails occasionally would be great, but whatever you have or know is great 👍🏻 (right now I'm fighting with race conditions that only show up in ~1% of test runs; it's not always easy to reproduce!)

@pgaijin66
Copy link
Author

Sounds good. I'll update the PR. Thank you for the quick response. Appreciate it alot.

@mildwonkey mildwonkey self-assigned this Jan 22, 2026
Copy link
Contributor

@mildwonkey mildwonkey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(nothing new, sorry for the noise - I just realized I should've left this with my first comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

Comments