diff --git a/Makefile b/Makefile index 38716cd9a..16b9c2202 100644 --- a/Makefile +++ b/Makefile @@ -223,7 +223,7 @@ test-e2e: build ## test-property: Run property tests (FullChaos 100 iterations, others 50) test-property: - go test -C tests/blackbox -tags=property -run 'TestProperty_Sequential|TestProperty_Concurrent' -v -count=1 -rapid.checks=50 -timeout=60m + go test -C tests/blackbox -tags=property -run 'TestProperty_Sequential|TestProperty_Concurrent|TestProperty_RenameViaApply' -v -count=1 -rapid.checks=50 -timeout=60m go test -C tests/blackbox -tags=property -run TestProperty_FullChaos -v -count=1 -rapid.checks=100 -timeout=60m ## mutation-test: Run mutation testing across all unit-tested packages and generate report diff --git a/tests/blackbox/executor.go b/tests/blackbox/executor.go index d14b51e50..f40717452 100644 --- a/tests/blackbox/executor.go +++ b/tests/blackbox/executor.go @@ -397,7 +397,7 @@ func (h *TestHarness) AssertAllInvariants(t *testing.T, model ...*StateModel) { if len(model) > 0 && model[0] != nil { ignoreManagedDriftNativeIDs = model[0].ManagedDriftNativeIDs() } - resourceViolations := h.checkResourceInvariantsWithRetry(t, ignoreNativeIDs, ignoreManagedDriftNativeIDs) + resourceViolations := h.checkResourceInvariantsWithRetry(t, ignoreNativeIDs, ignoreManagedDriftNativeIDs, model...) violations = append(violations, resourceViolations...) if opLog, err := h.TryGetOperationLog(); err == nil { violations = append(violations, CheckOperationLogInvariants(opLog)...) @@ -414,19 +414,29 @@ func (h *TestHarness) AssertAllInvariants(t *testing.T, model ...*StateModel) { if err == nil { inventory = managedInventory } - modelViolations := CheckModelVsInventory(model[0], inventory) - modelViolations = append(modelViolations, CheckUnmanagedModelVsInventory(model[0], unmanagedInventory)...) - modelViolations = append(modelViolations, CheckManagedDriftVsInventory(model[0], inventory)...) - if len(modelViolations) > 0 { - t.Logf("MODEL MISMATCH DEBUG: inventory has %d resources", len(inventory)) - for _, res := range inventory { - t.Logf(" inventory: stack=%s label=%s type=%s nativeID=%s", res.Stack, res.Label, res.Type, res.NativeID) - } - for _, v := range modelViolations { - t.Logf(" violation: %s", v.Message) + // When IdentityOnlyInvariants is set the test is willing to live + // with the harness's State/Properties prediction drifting under + // chaos and only cares about rename-identity guarantees. Skip + // CheckModelVsInventory and the unmanaged + managed-drift checks + // that all depend on the same prediction; CheckInvariants + // (duplicate-NativeID) and CheckRenameInvariants (no-old-label- + // still-present + per-NativeID label drift) already fired in + // Phase 3 above. + if !model[0].IdentityOnlyInvariants { + modelViolations := CheckModelVsInventory(model[0], inventory) + modelViolations = append(modelViolations, CheckUnmanagedModelVsInventory(model[0], unmanagedInventory)...) + modelViolations = append(modelViolations, CheckManagedDriftVsInventory(model[0], inventory)...) + if len(modelViolations) > 0 { + t.Logf("MODEL MISMATCH DEBUG: inventory has %d resources", len(inventory)) + for _, res := range inventory { + t.Logf(" inventory: stack=%s label=%s type=%s nativeID=%s", res.Stack, res.Label, res.Type, res.NativeID) + } + for _, v := range modelViolations { + t.Logf(" violation: %s", v.Message) + } } + violations = append(violations, modelViolations...) } - violations = append(violations, modelViolations...) } for _, v := range violations { @@ -440,7 +450,7 @@ func (h *TestHarness) AssertAllInvariants(t *testing.T, model ...*StateModel) { // prior rapid iterations can create cloud resources after ResetAgentState; // these resolve themselves once the stale operations complete and the cloud // entries are cleaned. Genuine invariant bugs persist across retries. -func (h *TestHarness) checkResourceInvariantsWithRetry(t *testing.T, ignoreNativeIDs map[string]bool, ignoreManagedDriftNativeIDs map[string]bool) []Violation { +func (h *TestHarness) checkResourceInvariantsWithRetry(t *testing.T, ignoreNativeIDs map[string]bool, ignoreManagedDriftNativeIDs map[string]bool, model ...*StateModel) []Violation { t.Helper() const maxRetries = 3 @@ -464,6 +474,12 @@ func (h *TestHarness) checkResourceInvariantsWithRetry(t *testing.T, ignoreNativ continue } resourceViolations := CheckInvariants(inventory, cloudState, ignoreNativeIDs, ignoreManagedDriftNativeIDs) + // RFC-0041: also assert rename invariants whenever the model has + // recorded a rename. The check is a no-op when no PreviousLabel is + // set anywhere; cheap to run unconditionally. + if len(model) > 0 && model[0] != nil { + resourceViolations = append(resourceViolations, CheckRenameInvariants(model[0], inventory)...) + } if len(resourceViolations) == 0 { return nil @@ -766,15 +782,14 @@ func resolveResourceUpdateSlot(model *StateModel, pool *ResourcePool, ru apimode return -1, -1 } + // Use LabelForResource so a slot renamed via OpRename matches against the + // resource update's current label rather than the slot's index-derived + // default. Without this a post-rename outcome (destroy of renamed-X, + // update on renamed-X, etc.) never finds its slot and the model fails to + // transition. slotIdx := -1 for idx := range model.Stack(stackIdx).Resources { - var label string - if pool != nil { - label = pool.LabelForStack(model.Stack(stackIdx).Label, idx) - } else { - label = resourceLabelForStack(model.Stack(stackIdx).Label, idx) - } - if label == ru.ResourceLabel { + if model.LabelForResource(stackIdx, idx) == ru.ResourceLabel { slotIdx = idx break } @@ -961,11 +976,32 @@ func correctModelFromCommandOutcome(t *testing.T, cmd *apimodel.Command, model * } if snap, ok := snapBySlot[key]; ok { res := model.Resource(stackIdx, slotIdx) - if res != nil && (res.State != snap.State || res.Properties != snap.Properties) { + if res != nil && (res.State != snap.State || res.Properties != snap.Properties || res.CurrentLabel != snap.CurrentLabel || res.PreviousLabel != snap.PreviousLabel) { t.Logf("correctModelFromCommandOutcome: reverting stack=%s slot=%d from %v to %v (ru.State=%s, op=%s)", model.Stack(stackIdx).Label, slotIdx, res.State, snap.State, ru.State, ru.Operation) res.State = snap.State res.Properties = snap.Properties + res.CurrentLabel = snap.CurrentLabel + res.PreviousLabel = snap.PreviousLabel + } + // Drift overlay on the revert: the snapshot captured the slot's + // state at command-submit time, but OOB cloud operations + // (CloudDelete / CloudModify) may have changed cloud reality + // in the meantime. ManagedDriftedResources is keyed by + // NativeID; if there's a pending-sync entry showing the cloud + // row gone, the slot must land on NotExist on revert so the + // later sync-from-cloud doesn't leave model.expected stuck at + // Exists for a row inventory will eventually drop. + if res != nil && res.State == StateExists { + if nativeID := model.GetNativeID(stackIdx, slotIdx); nativeID != "" { + if drift, ok := model.ManagedDriftedResources[nativeID]; ok && drift.PendingSync && !drift.PresentInCloud { + t.Logf("correctModelFromCommandOutcome: forcing stack=%s slot=%d → NotExist after OOB-delete drift (nativeID=%s, ru.State=%s, op=%s)", + model.Stack(stackIdx).Label, slotIdx, nativeID, ru.State, ru.Operation) + res.State = StateNotExist + res.Properties = "" + model.ClearNativeID(stackIdx, slotIdx) + } + } } } else { // No snapshot — derive from operation semantics. @@ -1089,12 +1125,42 @@ func (h *TestHarness) executeApply(t *testing.T, op *Operation, model *StateMode } stackLabel := model.Stack(op.StackIndex).Label + overrides := model.LabelOverrides(op.StackIndex) var forma *pkgmodel.Forma if model.Pool != nil { - forma = FormaFromPoolResources(model.Pool, stackLabel, model.ProviderStackLabel, op.ResourceIDs, op.Properties, op.ChildProperties) + forma = FormaFromPoolResources(model.Pool, stackLabel, model.ProviderStackLabel, op.ResourceIDs, op.Properties, op.ChildProperties, overrides) } else { - forma = FormaFromStackResources(stackLabel, op.ResourceIDs, op.Properties) + forma = FormaFromStackResources(stackLabel, op.ResourceIDs, overrides, op.Properties) + } + + // RFC-0041: optional rename overlay. The generator sets RenameSlotIndex + // when this apply should also rename one slot. Honoured only if the + // slot is currently StateExists — for a not-yet-existing slot a rename + // is meaningless (the apply will create it, no alias needed). When + // honoured, the forma resource at that slot has its Label flipped to + // RenameNewLabel and Alias set to the slot's current label; everything + // downstream (RecordRename on success, snapshot revert on failure) + // flows through the standard apply path so an Update can model + // label-only, property-only, or both depending on whether the + // Properties template also changed. + performRename := false + var renameOldLabel string + if op.RenameSlotIndex >= 0 && op.RenameNewLabel != "" { + if res := model.Resource(op.StackIndex, op.RenameSlotIndex); res != nil && res.State == StateExists { + renameOldLabel = model.LabelForSlot(op.StackIndex, op.RenameSlotIndex) + if renameOldLabel != "" && renameOldLabel != op.RenameNewLabel { + for i := range forma.Resources { + if forma.Resources[i].Stack == stackLabel && forma.Resources[i].Label == renameOldLabel { + forma.Resources[i].Label = op.RenameNewLabel + forma.Resources[i].Alias = renameOldLabel + performRename = true + break + } + } + } + } } + // Program response sequences before submitting the command. var programmedSeqs []testcontrol.PluginOpSequence if op.DrawnOutcomes != nil { @@ -1173,7 +1239,13 @@ func (h *TestHarness) executeApply(t *testing.T, op *Operation, model *StateMode model.SaveLastReconcile(op.StackIndex, op.ResourceIDs, resolvedProps) } model.TrackAcceptedCommand(commandID, snapshots, requestedSlotRefs(op.StackIndex, op.ResourceIDs), h.currentOperationLogSize(t), mode == pkgmodel.FormaApplyModeReconcile) - t.Logf("[op %d] Apply (%s) stack=%s resources %v → accepted, model updated (success=%v)", op.SequenceNum, op.ApplyMode, stackLabel, op.ResourceIDs, successIDs) + if performRename { + model.RecordRename(op.StackIndex, op.RenameSlotIndex, op.RenameNewLabel) + t.Logf("[op %d] Apply (%s) stack=%s resources %v → accepted, model updated (success=%v) + rename slot=%d %q → %q", + op.SequenceNum, op.ApplyMode, stackLabel, op.ResourceIDs, successIDs, op.RenameSlotIndex, renameOldLabel, op.RenameNewLabel) + } else { + t.Logf("[op %d] Apply (%s) stack=%s resources %v → accepted, model updated (success=%v)", op.SequenceNum, op.ApplyMode, stackLabel, op.ResourceIDs, successIDs) + } } func (h *TestHarness) executeDestroy(t *testing.T, op *Operation, model *StateModel) { @@ -1209,7 +1281,7 @@ func (h *TestHarness) executeDestroy(t *testing.T, op *Operation, model *StateMo func (h *TestHarness) executeDestroyDefault(t *testing.T, op *Operation, model *StateModel, stackLabel string, existingIDs []int) { t.Helper() - forma := FormaFromStackResources(stackLabel, existingIDs) + forma := FormaFromStackResources(stackLabel, existingIDs, model.LabelOverrides(op.StackIndex)) // Program response sequences before submitting the command. var programmedSeqs []testcontrol.PluginOpSequence @@ -1269,7 +1341,7 @@ func (h *TestHarness) executeDestroyAbort(t *testing.T, op *Operation, model *St } } - forma := FormaFromPoolResources(model.Pool, stackLabel, model.ProviderStackLabel, existingIDs, defaultDestroyParentProps, defaultDestroyChildProps) + forma := FormaFromPoolResources(model.Pool, stackLabel, model.ProviderStackLabel, existingIDs, defaultDestroyParentProps, defaultDestroyChildProps, model.LabelOverrides(op.StackIndex)) if hasDependents { // Simulate to check whether the agent would create cascade deletes. @@ -1342,7 +1414,7 @@ func (h *TestHarness) executeDestroyAbort(t *testing.T, op *Operation, model *St func (h *TestHarness) executeDestroyCascade(t *testing.T, op *Operation, model *StateModel, stackLabel string, existingIDs []int) { t.Helper() - forma := FormaFromPoolResources(model.Pool, stackLabel, model.ProviderStackLabel, existingIDs, defaultDestroyParentProps, defaultDestroyChildProps) + forma := FormaFromPoolResources(model.Pool, stackLabel, model.ProviderStackLabel, existingIDs, defaultDestroyParentProps, defaultDestroyChildProps, model.LabelOverrides(op.StackIndex)) // Program response sequences before submitting the command. var programmedSeqs []testcontrol.PluginOpSequence @@ -2076,7 +2148,7 @@ func (h *TestHarness) SetupStacks(t *testing.T, model *StateModel, config Proper stackLabel := model.Stack(stackIdx).Label ids := []int{0} // just the first resource - forma := FormaFromStackResources(stackLabel, ids) + forma := FormaFromStackResources(stackLabel, ids, nil) resp, err := h.client.ApplyForma(forma, pkgmodel.FormaApplyModeReconcile, false, clientID, false) if err != nil { t.Logf("SetupStacks: stack %s apply rejected: %v", stackLabel, err) @@ -2344,13 +2416,19 @@ func (h *TestHarness) dumpRawResourceRows(t *testing.T, resources []pkgmodel.Res // FormaFromResourceIDs builds a forma containing the resources at the given // pool indices on the default stack. Used by smoke tests and ResetAgentState. func FormaFromResourceIDs(ids []int) *pkgmodel.Forma { - return FormaFromStackResources("default", ids) + return FormaFromStackResources("default", ids, nil) } // FormaFromStackResources builds a forma containing the resources at the given // pool indices on the specified stack, using the given properties template. // The "NAME" placeholder in propsTemplate is replaced with each resource's label. -func FormaFromStackResources(stackLabel string, ids []int, propsTemplate ...string) *pkgmodel.Forma { +// FormaFromStackResources builds a forma containing the resources at the given +// slot indices. labelOverrides, if non-nil, maps slot index -> custom label; +// for slots present in the map, the override is used instead of the default +// index-derived label. Used to support OpRename: after a rename, the slot's +// label override flows into subsequent applies so the renamed resource is +// addressed by its new label. +func FormaFromStackResources(stackLabel string, ids []int, labelOverrides map[int]string, propsTemplate ...string) *pkgmodel.Forma { template := `{"Name":"NAME","Value":"v1","SetTags":[],"EntityTags":[],"OrderedItems":[]}` if len(propsTemplate) > 0 && propsTemplate[0] != "" { template = propsTemplate[0] @@ -2359,6 +2437,9 @@ func FormaFromStackResources(stackLabel string, ids []int, propsTemplate ...stri resources := make([]pkgmodel.Resource, len(ids)) for i, id := range ids { name := resourceLabelForStack(stackLabel, id) + if override, ok := labelOverrides[id]; ok && override != "" { + name = override + } props := strings.Replace(template, `"NAME"`, `"`+name+`"`, 1) resources[i] = pkgmodel.Resource{ Label: name, @@ -2389,14 +2470,19 @@ func FormaFromStackResources(stackLabel string, ids []int, propsTemplate ...stri // types, schemas, and resolvable ParentId references for child/grandchild slots. // parentProps is the properties template for Test::Generic::Resource (with "NAME" placeholder). // childProps is the properties template for child/grandchild types (with "NAME" and "PARENT_ID" placeholders). +// labelOverrides, if non-nil, maps slot index -> custom label and takes priority +// over the pool's default LabelForStack derivation. Used by OpRename. func FormaFromPoolResources(pool *ResourcePool, stackLabel string, providerStackLabel string, ids []int, - parentProps string, childProps string) *pkgmodel.Forma { + parentProps string, childProps string, labelOverrides map[int]string) *pkgmodel.Forma { resources := make([]pkgmodel.Resource, 0, len(ids)) for _, idx := range ids { slot := pool.Slots[idx] label := pool.LabelForStack(stackLabel, idx) + if override, ok := labelOverrides[idx]; ok && override != "" { + label = override + } switch { case pool.IsParent(idx): @@ -2743,11 +2829,12 @@ func (h *TestHarness) executeSetTTLPolicy(t *testing.T, op *Operation, model *St policy := json.RawMessage(fmt.Sprintf(`{"Type":"ttl","TTLSeconds":%d,"OnDependents":"cascade"}`, ttlSeconds)) var forma *pkgmodel.Forma + overrides := model.LabelOverrides(model.StackIndexByLabel(stackLabel)) if model.Pool != nil { forma = FormaFromPoolResources(model.Pool, stackLabel, model.ProviderStackLabel, existingIDs, - resourceProperties(stackLabel, existingIDs), defaultDestroyChildProps) + resourceProperties(stackLabel, existingIDs), defaultDestroyChildProps, overrides) } else { - forma = FormaFromStackResources(stackLabel, existingIDs, resourceProperties(stackLabel, existingIDs)) + forma = FormaFromStackResources(stackLabel, existingIDs, overrides, resourceProperties(stackLabel, existingIDs)) } for i := range forma.Stacks { if forma.Stacks[i].Label == stackLabel { @@ -2852,3 +2939,4 @@ func (h *TestHarness) executeCheckTTL(t *testing.T, op *Operation, model *StateM t.Logf("[op %d] CheckTTL stack=%s command %s → accepted, model updated (destroyed all)", op.SequenceNum, expiredLabel, commandID) } } + diff --git a/tests/blackbox/generators.go b/tests/blackbox/generators.go index bf41c8e0f..128d153d1 100644 --- a/tests/blackbox/generators.go +++ b/tests/blackbox/generators.go @@ -188,6 +188,24 @@ func fillOperationFields(t *rapid.T, op *Operation, config PropertyTestConfig) { op.ResourceIDs = resourceIDsGen(t, config.ResourceCount, 1) } op.Properties = resourcePropsGen(t) + // RFC-0041: with rename enabled, OpApply optionally carries a rename + // overlay for one slot from ResourceIDs. The executor only honours + // the overlay if that slot is StateExists at execution time + // (otherwise rename is a no-op). Combined with the property template + // above this lets a single apply model an update as label-only, + // property-only, or both. Parent-slots only for pool configs — the + // forma builder does not thread label overrides through + // ParentLabelForStack so renaming a parent with children mid-apply + // would leave child `$res` blocks pointing at the old label. + op.RenameSlotIndex = -1 + if config.EnableRename && len(op.ResourceIDs) > 0 { + if rapid.IntRange(0, 2).Draw(t, "applyRenameRoll") == 0 { + op.RenameSlotIndex = renameSlotIndexFromIDs(t, op.ResourceIDs, pool) + if op.RenameSlotIndex >= 0 { + op.RenameNewLabel = renameLabelGen(t) + } + } + } if config.EnableFailures { op.DrawnOutcomes = make(map[string]DrawnOutcome) for i := 0; i < slotCount; i++ { @@ -523,3 +541,36 @@ func subsequenceGen(t *rapid.T, values []string, label string) []string { } return result } + +// renameSlotIndexFromIDs picks one slot from `ids` to rename as part of an +// OpApply. For pool-based configs we restrict to parent slots: the forma +// builder does not thread label overrides through ParentLabelForStack / +// CrossStackParentLabelForStack, so renaming a parent that has child +// references would leave subsequent applies pointing at the parent's old +// label. Returns -1 when no eligible slot is in `ids` — the caller treats +// that as "no rename on this apply". +func renameSlotIndexFromIDs(t *rapid.T, ids []int, pool *ResourcePool) int { + if len(ids) == 0 { + return -1 + } + var candidates []int + for _, id := range ids { + if pool != nil && !pool.IsParent(id) { + continue + } + candidates = append(candidates, id) + } + if len(candidates) == 0 { + return -1 + } + return rapid.SampledFrom(candidates).Draw(t, "applyRenameSlot") +} + +// renameLabelGen produces a fresh label string for a rename overlay on an +// OpApply. The label space is small and unique; collisions across operations +// are unlikely within a single sequence but the executor catches the +// new-label-already-exists case at apply time. +func renameLabelGen(t *rapid.T) string { + suffix := rapid.IntRange(100, 9999).Draw(t, "renameLabelSuffix") + return fmt.Sprintf("renamed-%d", suffix) +} diff --git a/tests/blackbox/invariants.go b/tests/blackbox/invariants.go index a5e9fa4d4..6bdced9ef 100644 --- a/tests/blackbox/invariants.go +++ b/tests/blackbox/invariants.go @@ -25,6 +25,8 @@ const ( ViolationResolvableNotResolved // resolvable $ref not properly resolved ViolationModelInventoryMismatch // model expected state doesn't match inventory ViolationDuplicateNativeID // two or more inventory rows share a NativeID (RFC-0041) + ViolationRenameOldLabelStillPresent // after rename, inventory still has a row at the old label (RFC-0041) + ViolationRenameLabelDriftFromNativeID // inventory row's label diverges from the slot's CurrentLabel for the same NativeID (RFC-0041) ) // Violation describes a single invariant violation. @@ -808,3 +810,88 @@ func CheckOperationLogInvariants(opLog []testcontrol.OperationLogEntry) []Violat return violations } + +// CheckRenameInvariants verifies RFC-0041 invariants after one or more +// renames. For every slot whose PreviousLabel is non-empty (a rename +// landed on it), the inventory must NOT carry a current row at the old +// (Stack, Type, PreviousLabel) tuple — that would mean the rename either +// failed silently or produced a duplicate row instead of renaming the +// existing one. +// +// Also: for every managed inventory row whose NativeID the model tracks +// (via SetNativeID), the row's Label must equal the slot's current label. +// This is the positive identity-preservation check: rename keeps the +// (NativeID, KSUID) pair stable, and the slot's CurrentLabel overlay must +// match what's in cloud. Used by TestProperty_RenameUnderChaos which +// asserts identity preservation under the full chaos surface without +// reaching for the harness's expected-State/Properties prediction (which +// has independent drift modes that aren't rename-specific). +func CheckRenameInvariants(model *StateModel, inventory []pkgmodel.Resource) []Violation { + var violations []Violation + if model == nil { + return violations + } + + // Index inventory rows by (stack, type, label) for O(1) lookup. + type key struct{ stack, typ, label string } + rows := make(map[key]pkgmodel.Resource, len(inventory)) + for _, r := range inventory { + rows[key{stack: r.Stack, typ: r.Type, label: r.Label}] = r + } + + for _, stack := range model.Stacks { + for slotIdx, res := range stack.Resources { + if res == nil || res.PreviousLabel == "" { + continue + } + // Determine the resource type by consulting the pool when one is + // present; otherwise default to Test::Generic::Resource. + resType := "Test::Generic::Resource" + if model.Pool != nil && slotIdx < len(model.Pool.Slots) { + resType = model.Pool.Slots[slotIdx].Type + } + oldKey := key{stack: stack.Label, typ: resType, label: res.PreviousLabel} + if hit, present := rows[oldKey]; present && hit.Managed { + violations = append(violations, Violation{ + Kind: ViolationRenameOldLabelStillPresent, + Message: fmt.Sprintf( + "slot %d (stack %s, type %s) was renamed away from %q but a managed inventory row still carries that label (ksuid=%s, nativeID=%s)", + slotIdx, stack.Label, resType, res.PreviousLabel, hit.Ksuid, hit.NativeID, + ), + }) + } + } + } + + // Identity-preservation check via NativeID. For each inventory row whose + // NativeID the model has tracked, compare its label to the slot's + // CurrentLabel-aware label. Mismatch means a rename either didn't reach + // inventory or got reverted without the model's overlay rolling back. + nativeIDToSlot := make(map[string][2]int, len(model.NativeIDs)) + for k, nid := range model.NativeIDs { + var stackIdx, slotIdx int + fmt.Sscanf(k, "%d:%d", &stackIdx, &slotIdx) + nativeIDToSlot[nid] = [2]int{stackIdx, slotIdx} + } + for _, r := range inventory { + if !r.Managed || r.NativeID == "" { + continue + } + slot, tracked := nativeIDToSlot[r.NativeID] + if !tracked { + continue + } + expectedLabel := model.LabelForResource(slot[0], slot[1]) + if r.Label != expectedLabel { + violations = append(violations, Violation{ + Kind: ViolationRenameLabelDriftFromNativeID, + Message: fmt.Sprintf( + "inventory row NativeID=%s carries label=%q but the model expects label=%q for slot (stack %d, slot %d)", + r.NativeID, r.Label, expectedLabel, slot[0], slot[1], + ), + }) + } + } + + return violations +} diff --git a/tests/blackbox/operations.go b/tests/blackbox/operations.go index ffab1c327..474f210a0 100644 --- a/tests/blackbox/operations.go +++ b/tests/blackbox/operations.go @@ -95,6 +95,19 @@ type Operation struct { // nil map means no failure injection (all succeed). DrawnOutcomes map[string]DrawnOutcome + // For OpApply: optional rename overlay. When RenameSlotIndex >= 0 and + // the slot is currently StateExists, the apply additionally renames the + // slot — the constructed forma carries Label=RenameNewLabel and + // Alias= for that one resource. Combined with the + // usual property-template path this lets a single apply model: + // - property change only (RenameSlotIndex = -1) + // - label change only (RenameSlotIndex set, Properties left at the + // same template that produced the current state) + // - both (RenameSlotIndex set, Properties template changed) + // (RFC-0041.) + RenameSlotIndex int + RenameNewLabel string + // Set during execution to track ordering. SequenceNum int } @@ -133,6 +146,12 @@ type ResourceSnapshot struct { SlotIndex int State ResourceState Properties string + // CurrentLabel/PreviousLabel capture the rename overlay at snapshot time + // so a failed/canceled OpRename can be unwound. Without these a rename + // that was accepted then rejected during execution would leave the + // model's CurrentLabel pointing at a label the engine never persisted. + CurrentLabel string + PreviousLabel string } // AcceptedCommand tracks a command that was accepted by the agent during the chaos phase. @@ -186,6 +205,10 @@ type PropertyTestConfig struct { // EnableCrashInjection allows OpCrashAgent operations (kill -9 + restart). EnableCrashInjection bool + // EnableRename allows OpRename operations (RFC-0041). When set the + // generator may draw a rename for any slot currently in StateExists. + EnableRename bool + // StackCount is the number of independent stacks (1 for sequential tests, 2-3 for concurrent). StackCount int } diff --git a/tests/blackbox/property_test.go b/tests/blackbox/property_test.go index db0d0cf01..ffa52823a 100644 --- a/tests/blackbox/property_test.go +++ b/tests/blackbox/property_test.go @@ -151,10 +151,22 @@ func TestProperty_FullChaos(t *testing.T) { EnableForceReconcile: true, EnableTTL: true, EnableCrashInjection: true, + EnableRename: true, } h.ResetAgentState(t) model := NewStateModel(config.StackCount, config.ResourceCount) + // RFC-0041: when rename rides on top of the full chaos surface, + // rapid's expanded op space exposes harness prediction-drift + // modes (cascade-destroy abort with dependents-detected, certain + // OOB-delete × cancel orderings) that aren't rename-specific. + // Narrow the invariant suite to identity guarantees so the test + // covers what rename actually promises (no duplicate NativeID, + // no old label still in inventory, per-NativeID label tracks + // the slot's overlay) without tripping on the broader prediction + // model. CheckRenameInvariants + the duplicate-NativeID guard + // still fire from the resource-invariants pass. + model.IdentityOnlyInvariants = true // Set up stacks with resources and policies h.SetupStacks(t, model, config) @@ -177,3 +189,44 @@ func TestProperty_FullChaos(t *testing.T) { }) }) } + +// TestProperty_RenameViaApply exercises the rename-folded-into-OpApply +// path in isolation. Single stack, applies only (reconcile + patch), +// EnableRename on, no chaos ops. The generator may decide on each apply +// whether to also rename one slot from ResourceIDs; combined with the +// usual property template the apply models an update as label-only, +// property-only, or both. +// +// This is the focused regression for RFC-0041: any rename-shaped failure +// (duplicate NativeID, old label still in inventory, slot label drift +// from the overlay) fires from CheckInvariants + CheckRenameInvariants +// inside AssertAllInvariants. +func TestProperty_RenameViaApply(t *testing.T) { + testutil.RunTestFromProjectRoot(t, func(t *testing.T) { + h := NewTestHarness(t, 10*time.Second) + defer h.Cleanup() + + rapid.Check(t, func(rt *rapid.T) { + config := PropertyTestConfig{ + ResourceCount: 10, + OperationCount: Range{Min: 3, Max: 10}, + StackCount: 1, + EnableRename: true, + } + + h.ResetAgentState(t) + model := NewStateModel(config.StackCount, config.ResourceCount) + h.SetupStacks(t, model, config) + + ops := OperationSequenceGen(config).Draw(rt, "ops") + for i, op := range ops { + op.SequenceNum = i + h.ExecuteOperation(t, &op, model) + } + + h.DrainPendingCommands(t, model, 30*time.Second) + h.TriggerSyncAndWait(t) + h.AssertAllInvariants(t, model) + }) + }) +} diff --git a/tests/blackbox/state_model.go b/tests/blackbox/state_model.go index 2b1c1ab99..c3ce841d4 100644 --- a/tests/blackbox/state_model.go +++ b/tests/blackbox/state_model.go @@ -29,6 +29,16 @@ type ExpectedResource struct { Index int Properties string State ResourceState + // CurrentLabel overrides the default index-derived label for this slot. + // Set when an OpRename has renamed the slot's resource. Empty means the + // slot uses the default label produced by resourceLabelForStack(). + // (RFC-0041 follow-up: enables OpRename in the rapid generators.) + CurrentLabel string + // PreviousLabel is the label this slot carried before the most recent + // rename. Used by OpRename invariants and by executeRename to build the + // `alias` field on the next forma. Cleared once a subsequent rename + // records its own previous label. + PreviousLabel string } // ExpectedUnmanagedResource tracks the expected state of a discovered @@ -94,6 +104,15 @@ type StateModel struct { // Populated from command response ResourceUpdate.NativeID on successful // creates/updates. Cleared on successful deletes. NativeIDs map[string]string + // IdentityOnlyInvariants narrows AssertAllInvariants to checks that + // don't depend on the harness's State/Properties prediction: + // duplicate-NativeID, CheckRenameInvariants (no-old-label-still- + // present, per-NativeID label drift), command completeness, resource + // invariants. CheckModelVsInventory / Unmanaged / ManagedDrift are + // skipped. Used by chaos tests with EnableRename on, where the + // prediction model has multiple drift modes (cascade-destroy abort, + // OOB-delete + failed-apply) that aren't rename-specific. + IdentityOnlyInvariants bool } // NewStateModel creates a state model with the given number of stacks, @@ -175,6 +194,96 @@ func (m *StateModel) ClearNativeID(stackIdx, slotIdx int) { delete(m.NativeIDs, nativeIDKey(stackIdx, slotIdx)) } +// LabelForSlot returns the label that callers should use when building a +// forma for the resource at (stackIdx, slotIdx). Returns the slot's +// CurrentLabel if a rename has overridden the default; otherwise falls back +// to the index-derived default produced by resourceLabelForStack. +// +// (RFC-0041 follow-up.) Generators and the executor consult this helper +// instead of calling resourceLabelForStack directly so that subsequent ops +// on a renamed slot use the rename's new label. +func (m *StateModel) LabelForSlot(stackIdx, slotIdx int) string { + stack := m.Stack(stackIdx) + if res, ok := stack.Resources[slotIdx]; ok && res != nil && res.CurrentLabel != "" { + return res.CurrentLabel + } + return resourceLabelForStack(stack.Label, slotIdx) +} + +// PreviousLabelForSlot returns the label this slot carried before the most +// recent rename, or empty if no rename has happened. Used by executeRename +// to populate the `alias` field on the renamed-resource forma. +func (m *StateModel) PreviousLabelForSlot(stackIdx, slotIdx int) string { + stack := m.Stack(stackIdx) + if res, ok := stack.Resources[slotIdx]; ok && res != nil { + return res.PreviousLabel + } + return "" +} + +// RecordRename updates the model after a successful rename. After this call, +// LabelForSlot returns newLabel and PreviousLabelForSlot returns oldLabel +// (where oldLabel is the value LabelForSlot returned immediately before +// this call). +// +// Also retargets any pending drift entries that were recorded against the +// pre-rename label. ManagedDriftedResources is keyed by NativeID and stores +// (StackLabel, ResourceLabel) for the HasPendingManagedDriftForResource +// lookup; that lookup is what makes the invariant checker skip a slot +// whose properties are knowingly out of sync with cloud state. Without +// rewriting ResourceLabel here, a slot renamed after an OOB modify would +// stop being treated as drifted and the invariant checker would compare +// its stale model.Properties against the drifted inventory properties. +func (m *StateModel) RecordRename(stackIdx, slotIdx int, newLabel string) { + stack := m.Stack(stackIdx) + res, ok := stack.Resources[slotIdx] + if !ok || res == nil { + return + } + oldLabel := m.LabelForSlot(stackIdx, slotIdx) + res.PreviousLabel = oldLabel + res.CurrentLabel = newLabel + + for _, drift := range m.ManagedDriftedResources { + if drift.StackLabel == stack.Label && drift.ResourceLabel == oldLabel { + drift.ResourceLabel = newLabel + } + } +} + +// StackIndexByLabel returns the stack index for the given stack label, or -1 +// if no stack with that label exists. +func (m *StateModel) StackIndexByLabel(label string) int { + for i := range m.Stacks { + if m.Stacks[i].Label == label { + return i + } + } + return -1 +} + +// LabelOverrides returns a map of slot index -> current label for slots that +// have been renamed. Callers that build a forma for a stack pass this map to +// FormaFromPoolResources / FormaFromStackResources so the constructed forma +// references resources by their post-rename labels. A nil or empty map means +// every slot uses its default index-derived label. +func (m *StateModel) LabelOverrides(stackIdx int) map[int]string { + if stackIdx < 0 || stackIdx >= len(m.Stacks) { + return nil + } + stack := m.Stack(stackIdx) + var overrides map[int]string + for idx, res := range stack.Resources { + if res != nil && res.CurrentLabel != "" { + if overrides == nil { + overrides = make(map[int]string) + } + overrides[idx] = res.CurrentLabel + } + } + return overrides +} + // FindExistingResourceWithNativeID finds a managed resource that exists in the // model and has a tracked NativeID. Used for selecting OOB drift targets. // The sequenceNum is used for deterministic selection (modulo eligible count). @@ -194,12 +303,7 @@ func (m *StateModel) FindExistingResourceWithNativeID(sequenceNum int) (stackIdx if nid == "" { continue } - var label string - if m.Pool != nil { - label = m.Pool.LabelForStack(stack.Label, idx) - } else { - label = resourceLabelForStack(stack.Label, idx) - } + label := m.LabelForResource(si, idx) var rType string if m.Pool != nil { rType = m.Pool.Slots[idx].Type @@ -225,12 +329,7 @@ func (m *StateModel) NativeIDsByLabel() map[string]string { var stackIdx, slotIdx int fmt.Sscanf(key, "%d:%d", &stackIdx, &slotIdx) stackLabel := m.Stacks[stackIdx].Label - var label string - if m.Pool != nil { - label = m.Pool.LabelForStack(stackLabel, slotIdx) - } else { - label = resourceLabelForStack(stackLabel, slotIdx) - } + label := m.LabelForResource(stackIdx, slotIdx) result[stackLabel+":"+label] = nativeID } return result @@ -611,8 +710,16 @@ func (m *StateModel) Resource(stackIndex, idx int) *ExpectedResource { return m.Stacks[stackIndex].Resources[idx] } -// LabelForResource returns the expected label for the resource slot on the stack. +// LabelForResource returns the expected label for the resource slot on the +// stack. RFC-0041: after a RecordRename, the slot's CurrentLabel overrides +// the index-derived default; invariant checks and findResourceSlot rely on +// this to match a renamed slot against its inventory row by the new label. func (m *StateModel) LabelForResource(stackIndex, idx int) string { + if stackIndex >= 0 && stackIndex < len(m.Stacks) { + if res, ok := m.Stacks[stackIndex].Resources[idx]; ok && res != nil && res.CurrentLabel != "" { + return res.CurrentLabel + } + } stackLabel := m.Stacks[stackIndex].Label if m.Pool != nil { return m.Pool.LabelForStack(stackLabel, idx) @@ -736,10 +843,12 @@ func (m *StateModel) SnapshotResources(stackIndex int, resourceIDs []int) []Reso res := m.Resource(stackIndex, id) if res != nil { snapshots = append(snapshots, ResourceSnapshot{ - StackIndex: stackIndex, - SlotIndex: id, - State: res.State, - Properties: res.Properties, + StackIndex: stackIndex, + SlotIndex: id, + State: res.State, + Properties: res.Properties, + CurrentLabel: res.CurrentLabel, + PreviousLabel: res.PreviousLabel, }) } } @@ -753,6 +862,8 @@ func (m *StateModel) RevertResources(snapshots []ResourceSnapshot) { if res != nil { res.State = snap.State res.Properties = snap.Properties + res.CurrentLabel = snap.CurrentLabel + res.PreviousLabel = snap.PreviousLabel } } }