Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .changes/v1.15/BUG FIXES-20260122-061500.yaml
Original file line number Diff line number Diff line change
@@ -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
20 changes: 16 additions & 4 deletions internal/checks/state_report.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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
Expand Down
57 changes: 57 additions & 0 deletions internal/checks/state_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}