This commit is contained in:
Prabesh 2026-02-03 17:47:35 +00:00 committed by GitHub
commit 79f5344dc7
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 76 additions and 4 deletions

View file

@ -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

View file

@ -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

View file

@ -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)
}
}