diff --git a/.changes/v1.15/BUG FIXES-20260122-061500.yaml b/.changes/v1.15/BUG FIXES-20260122-061500.yaml new file mode 100644 index 000000000000..2e65252ebfe5 --- /dev/null +++ b/.changes/v1.15/BUG FIXES-20260122-061500.yaml @@ -0,0 +1,3 @@ +kind: BUG FIXES +body: 'core: Fixed panic when duplicate check status reports occur during concurrent resource evaluation, particularly during resource recreation with preconditions' +time: 2026-01-22T06:15:00.000000Z diff --git a/internal/checks/state_report.go b/internal/checks/state_report.go index afe07740e2c5..fa8e14f57b9c 100644 --- a/internal/checks/state_report.go +++ b/internal/checks/state_report.go @@ -64,8 +64,10 @@ func (c *State) ReportCheckableObjects(configAddr addrs.ConfigCheckable, objectA // or if the check index is out of bounds for the number of checks expected // of the given type, this method will panic to indicate a bug in the caller. // -// This method will also panic if the specified check already had a known -// status; each check should have its result reported only once. +// If the specified check already had a known status, this method will log +// a warning or error (depending on whether the status conflicts) and preserve +// the original status. This is a defensive measure against race conditions or +// duplicate reporting during concurrent graph walks. func (c *State) ReportCheckResult(objectAddr addrs.Checkable, checkType addrs.CheckRuleType, index int, status Status) { c.mu.Lock() defer c.mu.Unlock() @@ -111,8 +113,18 @@ func (c *State) reportCheckResult(objectAddr addrs.Checkable, checkType addrs.Ch if index >= len(checks[checkType]) { panic(fmt.Sprintf("%s index %d out of range for %s", checkType, index, objectAddr)) } - if checks[checkType][index] != StatusUnknown { - panic(fmt.Sprintf("duplicate status report for %s %s %d", objectAddr, checkType, index)) + + existingStatus := checks[checkType][index] + if existingStatus != StatusUnknown { + // Duplicate report detected; preserve first-write-wins semantics. + if existingStatus == status { + // Same status reported twice, likely a benign race condition. + log.Printf("[WARN] checks: duplicate status report for %s %s %d", objectAddr, checkType, index) + return + } + // Conflicting status reported; keep the original and log for debugging. + log.Printf("[ERROR] checks: conflicting status report for %s %s %d: was %s, got %s", objectAddr, checkType, index, existingStatus, status) + return } checks[checkType][index] = status diff --git a/internal/checks/state_test.go b/internal/checks/state_test.go index 7a0c4c5476a6..f5a009dac7a5 100644 --- a/internal/checks/state_test.go +++ b/internal/checks/state_test.go @@ -226,3 +226,60 @@ func TestChecksHappyPath(t *testing.T) { } } } + +func TestChecksDuplicateReport(t *testing.T) { + const fixtureDir = "testdata/happypath" + loader, close := configload.NewLoaderForTests(t) + defer close() + inst := initwd.NewModuleInstaller(loader.ModulesDir(), loader, nil) + _, instDiags := inst.InstallModules(context.Background(), fixtureDir, "tests", true, false, initwd.ModuleInstallHooksImpl{}) + if instDiags.HasErrors() { + t.Fatal(instDiags.Err()) + } + if err := loader.RefreshModules(); err != nil { + t.Fatalf("failed to refresh modules after installation: %s", err) + } + + ///////////////////////////////////////////////////////////////////////// + + cfg, hclDiags := loader.LoadConfig(fixtureDir) + if hclDiags.HasErrors() { + t.Fatalf("invalid configuration: %s", hclDiags.Error()) + } + + resourceA := addrs.Resource{ + Mode: addrs.ManagedResourceMode, + Type: "null_resource", + Name: "a", + }.InModule(addrs.RootModule) + + checks := NewState(cfg) + + resourceInstA := resourceA.Resource.Absolute(addrs.RootModuleInstance).Instance(addrs.NoKey) + + checks.ReportCheckableObjects(resourceA, addrs.MakeSet[addrs.Checkable](resourceInstA)) + + // Report initial check results for resource A (2 preconditions, 1 postcondition) + checks.ReportCheckResult(resourceInstA, addrs.ResourcePrecondition, 0, StatusPass) + checks.ReportCheckResult(resourceInstA, addrs.ResourcePrecondition, 1, StatusPass) + checks.ReportCheckResult(resourceInstA, addrs.ResourcePostcondition, 0, StatusPass) + + if got, want := checks.ObjectCheckStatus(resourceInstA), StatusPass; got != want { + t.Fatalf("incorrect check status after first report: got %s, want %s", got, want) + } + + // Reporting the same status again should not panic or change the result + checks.ReportCheckResult(resourceInstA, addrs.ResourcePrecondition, 0, StatusPass) + + if got, want := checks.ObjectCheckStatus(resourceInstA), StatusPass; got != want { + t.Errorf("incorrect check status after duplicate report: got %s, want %s", got, want) + } + + // Reporting a conflicting status should not panic, and should preserve + // the original status (first write wins) + checks.ReportCheckResult(resourceInstA, addrs.ResourcePrecondition, 0, StatusFail) + + if got, want := checks.ObjectCheckStatus(resourceInstA), StatusPass; got != want { + t.Errorf("check status should be preserved after conflicting report: got %s, want %s", got, want) + } +}