From 76b17eee1974ca33bb68ac273d6625ad4f8febcc Mon Sep 17 00:00:00 2001 From: naxty <9141879+naxty@users.noreply.github.com> Date: Fri, 5 Jun 2026 08:43:06 +0200 Subject: [PATCH 1/2] test(property): exercise resource rename via OpApply (RFC-0041) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Rename is a kind of update — the engine emits OperationUpdate for a label-only change — so the property tests model it as a rename overlay on OpApply rather than as a separate operation kind. What lands: - Operation gets RenameSlotIndex / RenameNewLabel fields, optional on OpApply. When set on an apply that targets an existing slot, the forma carries Label=NewLabel and Alias= for that one resource. Combined with the per-apply property template, an update naturally models label-only, property-only, or both. - Generator: with EnableRename on, ~33% of OpApply draws pick one parent slot from ResourceIDs to rename and a fresh label. Parent- slot restriction stays because the forma builder doesn't thread label overrides through ParentLabelForStack — renaming a parent with children would leave the children's resolvable references at the parent's old label. - Executor: after FormaFromPoolResources / FormaFromStackResources builds the forma, the rename overlay flips the matching resource's Label/Alias and calls RecordRename on success. Snapshot revert on cancel/failure picks up CurrentLabel/PreviousLabel via the existing snapshot path (ResourceSnapshot gained those fields). - StateModel: - LabelForResource, LabelForSlot, FindExistingResourceWithNativeID, NativeIDsByLabel, resolveResourceUpdateSlot all honour the CurrentLabel overlay so post-rename outcome resolution, drift tracking, and invariant checks match the renamed slot. - RecordRename retargets pending entries in ManagedDriftedResources (keyed by NativeID, ResourceLabel rewritten to the new label) so HasPendingManagedDriftForResource keeps matching after a rename. - LabelOverrides is consulted by every FormaFrom*Resources caller (executeApply, executeDestroy, executeSetTTLPolicy, SetupStacks, FormaFromResourceIDs) so subsequent ops see the renamed label. - Invariants: new CheckRenameInvariants asserts (a) no managed inventory row carries a slot's PreviousLabel and (b) for every tracked NativeID, the inventory row's Label matches the slot's CurrentLabel-aware label. Wired into AssertAllInvariants alongside the existing duplicate-NativeID guard. New ViolationRenameOldLabelStillPresent and ViolationRenameLabelDriftFromNativeID kinds. - New TestProperty_RenameViaApply runs the OpApply-with-rename path in isolation (single stack, no chaos) so any rename regression points squarely at the rename code rather than at chaos interactions. 50 rapid iterations pass locally. - TestProperty_FullChaos keeps EnableRename off for now. Folding rename into Apply doesn't fix the harness's expected-State / Properties prediction drift under cancel × ForceReconcile × partial-success applies (the drift fires even on slots that were never renamed; rename simply expands rapid's op space enough to reach those orderings sooner). Tightening the harness prediction model is a separate gap. - Makefile test-property target's -run pattern includes TestProperty_RenameViaApply in the 50-iteration tier alongside the existing sequential/concurrent tests. --- Makefile | 2 +- tests/blackbox/executor.go | 103 +++++++++++++++++++++++++------- tests/blackbox/generators.go | 51 ++++++++++++++++ tests/blackbox/invariants.go | 87 +++++++++++++++++++++++++++ tests/blackbox/operations.go | 23 +++++++ tests/blackbox/property_test.go | 52 ++++++++++++++++ tests/blackbox/state_model.go | 100 +++++++++++++++++++++++++------ 7 files changed, 378 insertions(+), 40 deletions(-) diff --git a/Makefile b/Makefile index ba141e31d..bd4940881 100644 --- a/Makefile +++ b/Makefile @@ -298,7 +298,7 @@ stage-oidc-fixtures: ## 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 e5a299d98..606c995bb 100644 --- a/tests/blackbox/executor.go +++ b/tests/blackbox/executor.go @@ -446,7 +446,7 @@ func (h *TestHarness) AssertAllInvariants(t *testing.T, model ...*StateModel) { // ResetAgentState, creating cloud resources that haven't been persisted // to inventory yet. When orphans are found, we clean them up and // re-check. If the orphan persists across retries, it's a real bug. - resourceViolations := h.checkResourceInvariantsWithRetry(t, ignoreNativeIDs) + resourceViolations := h.checkResourceInvariantsWithRetry(t, ignoreNativeIDs, model...) violations = append(violations, resourceViolations...) if opLog, err := h.TryGetOperationLog(); err == nil { violations = append(violations, CheckOperationLogInvariants(opLog)...) @@ -488,7 +488,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) []Violation { +func (h *TestHarness) checkResourceInvariantsWithRetry(t *testing.T, ignoreNativeIDs map[string]bool, model ...*StateModel) []Violation { t.Helper() const maxRetries = 3 @@ -512,6 +512,12 @@ func (h *TestHarness) checkResourceInvariantsWithRetry(t *testing.T, ignoreNativ continue } resourceViolations := CheckInvariants(inventory, cloudState, ignoreNativeIDs) + // 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 @@ -811,15 +817,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 } @@ -1029,11 +1034,13 @@ func correctModelFromCommandOutcome(t *testing.T, cmd *apimodel.Command, model * } } else 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 } } else { // No snapshot — derive from operation semantics. @@ -1144,12 +1151,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 { @@ -1236,7 +1273,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) { @@ -1272,7 +1315,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 @@ -1332,7 +1375,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. @@ -1405,7 +1448,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 @@ -2264,7 +2307,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) @@ -2560,13 +2603,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] @@ -2575,6 +2624,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, @@ -2606,14 +2658,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): @@ -2961,11 +3018,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 { @@ -3070,3 +3128,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 402bb6d56..fc0717379 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. @@ -726,3 +728,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 b88efd613..932625440 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. @@ -194,6 +213,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 1fac028f8..402f08e42 100644 --- a/tests/blackbox/property_test.go +++ b/tests/blackbox/property_test.go @@ -151,6 +151,17 @@ func TestProperty_FullChaos(t *testing.T) { EnableForceReconcile: true, EnableTTL: true, EnableCrashInjection: true, + // EnableRename intentionally off here. With rename folded into + // the OpApply path (RFC-0041) a rename rides inside an Update + // — the same prediction model that already drifts under chaos + // (cancel × ForceReconcile × partial-success applies on + // non-renamed slots). Leaving rename off keeps FullChaos + // focused on the existing prediction-correctness story until + // the harness's expected-State/Properties prediction is + // hardened. Identity-only coverage for rename lives in the + // LabelForResource-aware invariants (CheckRenameInvariants) + // and the duplicate-NativeID guard, which both fire under + // AssertAllInvariants for any test that turns EnableRename on. } h.ResetAgentState(t) @@ -177,3 +188,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 15496ea63..35e3fbf95 100644 --- a/tests/blackbox/state_model.go +++ b/tests/blackbox/state_model.go @@ -28,6 +28,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 @@ -236,12 +246,7 @@ func (m *StateModel) FindDriftEligibleResource(sequenceNum int) (stackIdx, slotI if m.Pool != nil && m.Pool.IsCrossStack(idx) && busyStacks[0] { 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 @@ -267,12 +272,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 @@ -422,8 +422,26 @@ 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) + } + return resourceLabelForStack(stackLabel, idx) +} + +// DefaultLabelForResource returns the index-derived label for the slot, +// ignoring any rename overlay. This is the label a slot gets on (re)create. +func (m *StateModel) DefaultLabelForResource(stackIndex, idx int) string { stackLabel := m.Stacks[stackIndex].Label if m.Pool != nil { return m.Pool.LabelForStack(stackLabel, idx) @@ -431,6 +449,50 @@ func (m *StateModel) LabelForResource(stackIndex, idx int) string { return resourceLabelForStack(stackLabel, idx) } +// RecordRename updates the model after a rename was accepted. After this +// call, LabelForResource returns newLabel and the slot remembers the label +// it carried immediately before. +func (m *StateModel) RecordRename(stackIdx, slotIdx int, newLabel string) { + res, ok := m.Stack(stackIdx).Resources[slotIdx] + if !ok || res == nil { + return + } + res.PreviousLabel = m.LabelForResource(stackIdx, slotIdx) + res.CurrentLabel = 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 +// carries and references resources by their post-rename labels. A nil 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 + } + var overrides map[int]string + for idx, res := range m.Stack(stackIdx).Resources { + if res != nil && res.CurrentLabel != "" { + if overrides == nil { + overrides = make(map[int]string) + } + overrides[idx] = res.CurrentLabel + } + } + return overrides +} + // TypeForResource returns the expected type for the resource slot. func (m *StateModel) TypeForResource(idx int) string { if m.Pool != nil { @@ -547,10 +609,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, }) } } @@ -564,6 +628,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 } } } From 9cabcb21f6ea499510b78dc987a5fc1f4c81a2ca Mon Sep 17 00:00:00 2001 From: Jeroen Soeters Date: Fri, 21 Aug 2026 14:32:13 -0700 Subject: [PATCH 2/2] test(property): verify rename identity from command outcomes and thread label overlays end to end MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The rename overlay is now folded into the label overrides before the forma is built, so child $res references to a renamed slot carry the new label, and any slot (not just parents) can be renamed. Rename labels embed the operation's sequence position, making label reuse within a sequence impossible by construction. Identity preservation is verified from the engine's own reporting instead of trusting the model's optimistic bookkeeping: the model tracks KSUIDs alongside NativeIDs, an update RU that changes either identity is flagged before the model adopts the new value, and a successful command that carried a rename must contain the update RU the alias path produces — its absence means the rename was dropped or executed as destroy+recreate. The old-label invariant gained an identity guard so a different slot legitimately renamed onto a freed label is not flagged, and the positive check now asserts KSUID stability per tracked NativeID. Failed or canceled commands restore the rename overlay for snapshotted slots that produced no resource update, and the cancel path's snapshot lookup honours renamed labels. Failure-injection programming addresses slots by index rather than round-tripping through default labels, so injection keeps working after a rename. FullChaos runs with rename enabled under the full invariant suite, and the focused rename test asserts at least one rename was accepted across the run so a rename path that rejects everything cannot pass vacuously. --- Makefile | 2 +- tests/blackbox/executor.go | 229 ++++++++++++++++------- tests/blackbox/generators.go | 56 ++---- tests/blackbox/generators_test.go | 6 +- tests/blackbox/harness.go | 6 + tests/blackbox/invariants.go | 81 +++++--- tests/blackbox/invariants_rename_test.go | 57 ++++++ tests/blackbox/operations.go | 14 +- tests/blackbox/property_test.go | 27 +-- tests/blackbox/rename_test.go | 71 +++++++ tests/blackbox/state_model.go | 50 +++-- tests/blackbox/state_model_test.go | 121 +++++++++++- 12 files changed, 549 insertions(+), 171 deletions(-) create mode 100644 tests/blackbox/rename_test.go diff --git a/Makefile b/Makefile index bd4940881..00449f427 100644 --- a/Makefile +++ b/Makefile @@ -298,7 +298,7 @@ stage-oidc-fixtures: ## test-property: Run property tests (FullChaos 100 iterations, others 50) test-property: - 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_Sequential|TestProperty_Concurrent|TestProperty_RenameViaApply|TestRenameViaApply_Deterministic' -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 606c995bb..c6d78743d 100644 --- a/tests/blackbox/executor.go +++ b/tests/blackbox/executor.go @@ -403,7 +403,7 @@ func (h *TestHarness) reconcileCompletedAcceptedCommands(t *testing.T, model *St for i := len(completed) - 1; i >= 0; i-- { cc := completed[i] t.Logf("reconcileCompletedAcceptedCommands: command %s completed early (state=%s)", cc.ac.CommandID, cc.cmd.State) - correctModelFromCommandOutcome(t, &cc.cmd, model, model.Pool, cc.ac.Snapshots, corrected, cc.ac.IsReconcile, cc.ac.SupersededSlots) + correctModelFromCommandOutcome(t, &cc.cmd, model, model.Pool, cc.ac.Snapshots, corrected, cc.ac.IsReconcile, cc.ac.SupersededSlots, cc.ac.Rename) } model.AcceptedCommands = remaining @@ -477,6 +477,13 @@ func (h *TestHarness) AssertAllInvariants(t *testing.T, model ...*StateModel) { violations = append(violations, modelViolations...) } + // Violations detected while correcting the model from command outcomes + // (identity changes, dropped renames) surface here. + if len(model) > 0 && model[0] != nil { + violations = append(violations, model[0].PendingViolations...) + model[0].PendingViolations = nil + } + for _, v := range violations { t.Logf("invariant violation: %s", v.Message) } @@ -930,7 +937,7 @@ func applyReconcileGuarantee(model *StateModel, stackIdx int, reconcileIDs []int // first) by DrainPendingCommands. The corrected map tracks which resources // have already been corrected by a later command — earlier commands skip // those resources so the latest outcome wins. -func correctModelFromCommandOutcome(t *testing.T, cmd *apimodel.Command, model *StateModel, pool *ResourcePool, snapshots []ResourceSnapshot, corrected map[struct{ stackIdx, slotIdx int }]bool, isReconcile bool, superseded map[ResourceSlotRef]bool) { +func correctModelFromCommandOutcome(t *testing.T, cmd *apimodel.Command, model *StateModel, pool *ResourcePool, snapshots []ResourceSnapshot, corrected map[struct{ stackIdx, slotIdx int }]bool, isReconcile bool, superseded map[ResourceSlotRef]bool, rename *PendingRename) { t.Helper() if cmd == nil { @@ -939,6 +946,31 @@ func correctModelFromCommandOutcome(t *testing.T, cmd *apimodel.Command, model * type slotKey = struct{ stackIdx, slotIdx int } + // RFC-0041: a successful command that carried a rename overlay must have + // performed it as an in-place UPDATE of the existing resource at its new + // label. A destroy+recreate shows up as a create RU at the new label + // instead, and a dropped alias makes the engine treat the new label as a + // brand-new resource (also a create). Identity stability of the update + // itself (NativeID/KSUID unchanged) is checked in the update case below. + if rename != nil && cmd.State == "Success" { + found := false + for _, ru := range cmd.ResourceUpdates { + if ru.State == "Success" && ru.Operation == "update" && ru.ResourceLabel == rename.NewLabel { + found = true + break + } + } + if !found { + model.AddPendingViolation(Violation{ + Kind: ViolationRenameRecreatedResource, + Message: fmt.Sprintf( + "command %s succeeded but contains no update RU at the renamed label %q (was %q, stack %s) — the rename was dropped or executed as destroy+recreate", + cmd.CommandID, rename.NewLabel, rename.OldLabel, model.Stack(rename.StackIndex).Label, + ), + }) + } + } + // Log all resource updates for debugging. t.Logf("correctModelFromCommandOutcome: cmd=%s state=%s has %d snapshots, %d resource updates", cmd.CommandID, cmd.State, len(snapshots), len(cmd.ResourceUpdates)) @@ -989,16 +1021,40 @@ func correctModelFromCommandOutcome(t *testing.T, cmd *apimodel.Command, model * } model.ApplyCreated(stackIdx, []int{slotIdx}, props) model.SetNativeID(stackIdx, slotIdx, ru.NativeID) + model.SetKsuid(stackIdx, slotIdx, ru.ResourceID) case "delete": model.ApplyDestroyed(stackIdx, []int{slotIdx}) model.MarkAuthoritativeSlot(stackIdx, slotIdx) model.ClearNativeID(stackIdx, slotIdx) + model.ClearKsuid(stackIdx, slotIdx) case "update": // Don't let updates override authoritative slots (e.g. TTL destroy). // Only creates can clear authoritative status. if model.IsAuthoritativeSlot(stackIdx, slotIdx) { goto markDone } + // RFC-0041: an update never changes a resource's identity — + // that would be replace semantics. Check before adopting the + // RU's NativeID/KSUID so a rename that swapped the identity is + // caught rather than absorbed. + if tracked := model.GetNativeID(stackIdx, slotIdx); tracked != "" && ru.NativeID != "" && tracked != ru.NativeID { + model.AddPendingViolation(Violation{ + Kind: ViolationRenameIdentityChanged, + Message: fmt.Sprintf( + "update RU for stack=%s label=%s changed NativeID %s → %s (cmd=%s)", + ru.StackName, ru.ResourceLabel, tracked, ru.NativeID, cmd.CommandID, + ), + }) + } + if tracked := model.GetKsuid(stackIdx, slotIdx); tracked != "" && ru.ResourceID != "" && tracked != ru.ResourceID { + model.AddPendingViolation(Violation{ + Kind: ViolationRenameIdentityChanged, + Message: fmt.Sprintf( + "update RU for stack=%s label=%s changed KSUID %s → %s (cmd=%s)", + ru.StackName, ru.ResourceLabel, tracked, ru.ResourceID, cmd.CommandID, + ), + }) + } if ru.Properties != nil { props := model.NormalizePropertiesForResource(stackIdx, slotIdx, string(ru.Properties)) // Update properties without touching existence state or @@ -1009,6 +1065,7 @@ func correctModelFromCommandOutcome(t *testing.T, cmd *apimodel.Command, model * } } model.SetNativeID(stackIdx, slotIdx, ru.NativeID) + model.SetKsuid(stackIdx, slotIdx, ru.ResourceID) } } else { // Failed/Canceled — revert to pre-command snapshot state. @@ -1090,7 +1147,20 @@ func correctModelFromCommandOutcome(t *testing.T, cmd *apimodel.Command, model * continue } res := model.Resource(key.stackIdx, key.slotIdx) - if res == nil || (res.State == snap.State && res.Properties == snap.Properties) { + if res == nil { + continue + } + // A rename recorded optimistically for this command never reached + // the engine if the slot produced no RU — restore the snapshot's + // labels regardless of whether State changed, or the model keeps + // a label the engine never persisted. + if res.CurrentLabel != snap.CurrentLabel || res.PreviousLabel != snap.PreviousLabel { + t.Logf("correctModelFromCommandOutcome: reverting unmentioned slot labels stack=%s slot=%d %q → %q (command state=%s)", + model.Stack(key.stackIdx).Label, key.slotIdx, res.CurrentLabel, snap.CurrentLabel, cmd.State) + res.CurrentLabel = snap.CurrentLabel + res.PreviousLabel = snap.PreviousLabel + } + if res.State == snap.State && res.Properties == snap.Properties { continue } t.Logf("correctModelFromCommandOutcome: reverting unmentioned slot stack=%s slot=%d from %v to %v (command state=%s)", @@ -1151,47 +1221,62 @@ 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, overrides) - } else { - 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 + // honoured, the rename is folded into the label overrides BEFORE the + // forma is built, so child `$res` references to the renamed slot carry + // the new label (the engine resolves intra-forma references against + // declared labels — a reference to the old label would not resolve). + // The renamed resource then gets Alias set to its previous label, and + // everything downstream (RecordRename on acceptance, 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 - } - } + renameOldLabel = model.LabelForResource(op.StackIndex, op.RenameSlotIndex) + performRename = renameOldLabel != "" && renameOldLabel != op.RenameNewLabel + } + } + + overrides := model.LabelOverrides(op.StackIndex) + if performRename { + if overrides == nil { + overrides = make(map[int]string) + } + overrides[op.RenameSlotIndex] = op.RenameNewLabel + } + + var forma *pkgmodel.Forma + if model.Pool != nil { + forma = FormaFromPoolResources(model.Pool, stackLabel, model.ProviderStackLabel, op.ResourceIDs, op.Properties, op.ChildProperties, overrides, model.LabelOverrides(0)) + } else { + forma = FormaFromStackResources(stackLabel, op.ResourceIDs, overrides, op.Properties) + } + + if performRename { + found := false + for i := range forma.Resources { + if forma.Resources[i].Stack == stackLabel && forma.Resources[i].Label == op.RenameNewLabel { + forma.Resources[i].Alias = renameOldLabel + found = true + break } } + // A cross-stack slot on the provider stack is skipped by the forma + // builder; if the renamed slot produced no resource, drop the rename. + performRename = found } // Program response sequences before submitting the command. var programmedSeqs []testcontrol.PluginOpSequence if op.DrawnOutcomes != nil { - nativeIDs := model.NativeIDsByLabel() - programmedSeqs = buildPluginOpSequences(op.DrawnOutcomes, op.StackIndex, stackLabel, op.ResourceIDs, model, nativeIDs, false, model.Pool) + programmedSeqs = buildPluginOpSequences(op.DrawnOutcomes, op.StackIndex, stackLabel, op.ResourceIDs, model, false, model.Pool) if len(programmedSeqs) > 0 { h.ProgramResponses(t, programmedSeqs) } @@ -1249,6 +1334,14 @@ func (h *TestHarness) executeApply(t *testing.T, op *Operation, model *StateMode } snapshots := model.SnapshotResources(op.StackIndex, snapshotIDs) + // Record the rename BEFORE computing property predictions: the template's + // NAME resolves through LabelForResource, and the engine persists the + // renamed slot's Name as the new label. The snapshots above captured the + // pre-rename overlay, so a failed/canceled command reverts cleanly. + if performRename { + model.RecordRename(op.StackIndex, op.RenameSlotIndex, op.RenameNewLabel) + } + // Immediate model update: predict outcomes at submission time. successIDs := successfulResourceIDs(op, op.StackIndex, op.ResourceIDs, model.Pool, false, model) if len(successIDs) > 0 { @@ -1274,7 +1367,13 @@ func (h *TestHarness) executeApply(t *testing.T, op *Operation, model *StateMode } model.TrackAcceptedCommand(commandID, snapshots, requestedSlotRefs(op.StackIndex, op.ResourceIDs), h.currentOperationLogSize(t), mode == pkgmodel.FormaApplyModeReconcile) if performRename { - model.RecordRename(op.StackIndex, op.RenameSlotIndex, op.RenameNewLabel) + h.RenamesAccepted++ + model.AcceptedCommands[len(model.AcceptedCommands)-1].Rename = &PendingRename{ + StackIndex: op.StackIndex, + SlotIndex: op.RenameSlotIndex, + OldLabel: renameOldLabel, + NewLabel: 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 { @@ -1320,8 +1419,7 @@ func (h *TestHarness) executeDestroyDefault(t *testing.T, op *Operation, model * // Program response sequences before submitting the command. var programmedSeqs []testcontrol.PluginOpSequence if op.DrawnOutcomes != nil { - nativeIDs := model.NativeIDsByLabel() - programmedSeqs = buildPluginOpSequences(op.DrawnOutcomes, op.StackIndex, stackLabel, existingIDs, model, nativeIDs, true, model.Pool) + programmedSeqs = buildPluginOpSequences(op.DrawnOutcomes, op.StackIndex, stackLabel, existingIDs, model, true, model.Pool) if len(programmedSeqs) > 0 { h.ProgramResponses(t, programmedSeqs) } @@ -1375,7 +1473,7 @@ func (h *TestHarness) executeDestroyAbort(t *testing.T, op *Operation, model *St } } - forma := FormaFromPoolResources(model.Pool, stackLabel, model.ProviderStackLabel, existingIDs, defaultDestroyParentProps, defaultDestroyChildProps, model.LabelOverrides(op.StackIndex)) + forma := FormaFromPoolResources(model.Pool, stackLabel, model.ProviderStackLabel, existingIDs, defaultDestroyParentProps, defaultDestroyChildProps, model.LabelOverrides(op.StackIndex), model.LabelOverrides(0)) if hasDependents { // Simulate to check whether the agent would create cascade deletes. @@ -1401,8 +1499,7 @@ func (h *TestHarness) executeDestroyAbort(t *testing.T, op *Operation, model *St // Program response sequences before submitting the command. var programmedSeqs []testcontrol.PluginOpSequence if op.DrawnOutcomes != nil { - nativeIDs := model.NativeIDsByLabel() - programmedSeqs = buildPluginOpSequences(op.DrawnOutcomes, op.StackIndex, stackLabel, existingIDs, model, nativeIDs, true, model.Pool) + programmedSeqs = buildPluginOpSequences(op.DrawnOutcomes, op.StackIndex, stackLabel, existingIDs, model, true, model.Pool) if len(programmedSeqs) > 0 { h.ProgramResponses(t, programmedSeqs) } @@ -1448,13 +1545,12 @@ 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, model.LabelOverrides(op.StackIndex)) + forma := FormaFromPoolResources(model.Pool, stackLabel, model.ProviderStackLabel, existingIDs, defaultDestroyParentProps, defaultDestroyChildProps, model.LabelOverrides(op.StackIndex), model.LabelOverrides(0)) // Program response sequences before submitting the command. var programmedSeqs []testcontrol.PluginOpSequence if op.DrawnOutcomes != nil { - nativeIDs := model.NativeIDsByLabel() - programmedSeqs = buildPluginOpSequences(op.DrawnOutcomes, op.StackIndex, stackLabel, existingIDs, model, nativeIDs, true, model.Pool) + programmedSeqs = buildPluginOpSequences(op.DrawnOutcomes, op.StackIndex, stackLabel, existingIDs, model, true, model.Pool) if len(programmedSeqs) > 0 { h.ProgramResponses(t, programmedSeqs) } @@ -1634,9 +1730,11 @@ func (h *TestHarness) executeCancel(t *testing.T, op *Operation, model *StateMod } model.ApplyCreated(stackIdx, []int{slotIdx}, props) model.SetNativeID(stackIdx, slotIdx, ru.NativeID) + model.SetKsuid(stackIdx, slotIdx, ru.ResourceID) case "delete": model.ApplyDestroyed(stackIdx, []int{slotIdx}) model.ClearNativeID(stackIdx, slotIdx) + model.ClearKsuid(stackIdx, slotIdx) } } } @@ -1656,14 +1754,12 @@ func (h *TestHarness) executeCancel(t *testing.T, op *Operation, model *StateMod func buildLabelToSnapshotMap(snapshots []ResourceSnapshot, model *StateModel) map[string]ResourceSnapshot { m := make(map[string]ResourceSnapshot, len(snapshots)) for _, snap := range snapshots { - stackLabel := model.Stack(snap.StackIndex).Label - var label string - if model.Pool != nil { - label = model.Pool.LabelForStack(stackLabel, snap.SlotIndex) - } else { - label = resourceLabelForStack(stackLabel, snap.SlotIndex) - } - m[label] = snap + // Key by the label the agent currently knows the slot by. The model + // carries the optimistic rename overlay, so a slot renamed by the + // canceled command itself resolves to the new label the RU reports; + // the snapshot's own labels are the pre-command state to revert TO, + // not the lookup key. + m[model.LabelForResource(snap.StackIndex, snap.SlotIndex)] = snap } return m } @@ -2251,7 +2347,7 @@ func (h *TestHarness) DrainPendingCommands(t *testing.T, model *StateModel, time for i := len(drained) - 1; i >= 0; i-- { dc := drained[i] if dc.cmd != nil { - correctModelFromCommandOutcome(t, dc.cmd, model, model.Pool, dc.ac.Snapshots, corrected, dc.ac.IsReconcile, dc.ac.SupersededSlots) + correctModelFromCommandOutcome(t, dc.cmd, model, model.Pool, dc.ac.Snapshots, corrected, dc.ac.IsReconcile, dc.ac.SupersededSlots, dc.ac.Rename) } } h.reconcileAmbiguousFailedCommands(t, model, drained) @@ -2658,10 +2754,14 @@ func FormaFromStackResources(stackLabel string, ids []int, labelOverrides map[in // 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. +// labelOverrides, if non-nil, maps slot index -> custom label for slots on +// this stack and takes priority over the pool's default LabelForStack +// derivation. It applies both to a resource's own label and to the parent +// labels in child `$res` references, so a renamed parent stays referenced +// by its current label. providerLabelOverrides is the same map for the +// provider stack (stack 0), consulted for cross-stack parent references. func FormaFromPoolResources(pool *ResourcePool, stackLabel string, providerStackLabel string, ids []int, - parentProps string, childProps string, labelOverrides map[int]string) *pkgmodel.Forma { + parentProps string, childProps string, labelOverrides map[int]string, providerLabelOverrides map[int]string) *pkgmodel.Forma { resources := make([]pkgmodel.Resource, 0, len(ids)) @@ -2694,6 +2794,9 @@ func FormaFromPoolResources(pool *ResourcePool, stackLabel string, providerStack } props := strings.Replace(childProps, `"NAME"`, `"`+label+`"`, 1) parentLabel := pool.CrossStackParentLabelForStack(providerStackLabel, idx) + if override, ok := providerLabelOverrides[pool.Slots[idx].CrossStackParentSlot]; ok && override != "" { + parentLabel = override + } parentType := pool.CrossStackParentType(idx) resObj, _ := json.Marshal(map[string]any{ "$res": true, @@ -2721,6 +2824,9 @@ func FormaFromPoolResources(pool *ResourcePool, stackLabel string, providerStack // The metastructure expects {"$res":true, "$label":"...", "$type":"...", // "$stack":"...", "$property":"..."} — NOT raw {"$ref":"formae://..."}. parentLabel := pool.ParentLabelForStack(stackLabel, idx) + if override, ok := labelOverrides[pool.Slots[idx].ParentIndex]; ok && override != "" { + parentLabel = override + } parentType := pool.ParentType(idx) resObj, _ := json.Marshal(map[string]any{ "$res": true, @@ -2800,7 +2906,6 @@ func buildPluginOpSequences( stackLabel string, resourceIDs []int, model *StateModel, - nativeIDs map[string]string, isDestroy bool, pool *ResourcePool, ) []testcontrol.PluginOpSequence { @@ -2882,20 +2987,16 @@ func buildPluginOpSequences( continue // no drawn outcome for this slot — will succeed by default } - // Determine the resource label - var label string - if pool != nil { - label = pool.LabelForStack(stackLabel, slotIdx) - } else { - label = resourceLabelForStack(stackLabel, slotIdx) - } + // The label the forma carries for this slot — rename-overlay aware, + // so failure injection keeps working after a slot has been renamed. + label := model.LabelForResource(stackIndex, slotIdx) res := model.Stack(stackIndex).Resources[slotIdx] exists := res != nil && res.State == StateExists if exists { // Resource exists -> will be Read+Update or Read+Delete - nativeID := nativeIDs[stackLabel+":"+label] + nativeID := model.GetNativeID(stackIndex, slotIdx) if nativeID == "" { continue // can't program without NativeID } @@ -2925,7 +3026,7 @@ func buildPluginOpSequences( // Resource doesn't exist -> will be a Create // If the resource has resolvables, program the ResolveCache read of the // referenced resource first. Only program Create if that read succeeds. - if resolveTarget, ok := resolveReadMatchKey(pool, model, stackIndex, stackLabel, slotIdx, nativeIDs); ok && len(outcome.ReadSteps) > 0 { + if resolveTarget, ok := resolveReadMatchKey(pool, model, stackIndex, slotIdx); ok && len(outcome.ReadSteps) > 0 { sequences = append(sequences, testcontrol.PluginOpSequence{ MatchKey: resolveTarget, Operation: "Read", @@ -2955,21 +3056,20 @@ func hasResolveReadPhase(pool *ResourcePool, slotIdx int) bool { return pool.Slots[slotIdx].ParentIndex >= 0 || pool.IsCrossStack(slotIdx) } -func resolveReadMatchKey(pool *ResourcePool, model *StateModel, stackIdx int, stackLabel string, slotIdx int, nativeIDs map[string]string) (string, bool) { +func resolveReadMatchKey(pool *ResourcePool, model *StateModel, stackIdx int, slotIdx int) (string, bool) { if pool == nil { return "", false } if pool.IsCrossStack(slotIdx) { - parentLabel := pool.CrossStackParentLabelForStack(model.ProviderStackLabel, slotIdx) - nativeID := nativeIDs[model.ProviderStackLabel+":"+parentLabel] + // Cross-stack parents live on the provider stack (stack 0). + nativeID := model.GetNativeID(0, pool.Slots[slotIdx].CrossStackParentSlot) return nativeID, nativeID != "" } parentIdx := pool.Slots[slotIdx].ParentIndex if parentIdx < 0 { return "", false } - parentLabel := pool.ParentLabelForStack(stackLabel, slotIdx) - nativeID := nativeIDs[stackLabel+":"+parentLabel] + nativeID := model.GetNativeID(stackIdx, parentIdx) return nativeID, nativeID != "" } @@ -3021,7 +3121,7 @@ func (h *TestHarness) executeSetTTLPolicy(t *testing.T, op *Operation, model *St overrides := model.LabelOverrides(model.StackIndexByLabel(stackLabel)) if model.Pool != nil { forma = FormaFromPoolResources(model.Pool, stackLabel, model.ProviderStackLabel, existingIDs, - resourceProperties(stackLabel, existingIDs), defaultDestroyChildProps, overrides) + resourceProperties(stackLabel, existingIDs), defaultDestroyChildProps, overrides, model.LabelOverrides(0)) } else { forma = FormaFromStackResources(stackLabel, existingIDs, overrides, resourceProperties(stackLabel, existingIDs)) } @@ -3128,4 +3228,3 @@ 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 128d153d1..c36fd7bc2 100644 --- a/tests/blackbox/generators.go +++ b/tests/blackbox/generators.go @@ -40,7 +40,7 @@ func OperationSequenceGen(config PropertyTestConfig) *rapid.Generator[[]Operatio count := rapid.IntRange(config.OperationCount.Min, config.OperationCount.Max).Draw(t, "count") ops := make([]Operation, count) for i := range ops { - ops[i] = SingleOperationGen(config).Draw(t, fmt.Sprintf("op-%d", i)) + ops[i] = SingleOperationGen(config, i).Draw(t, fmt.Sprintf("op-%d", i)) } // Enforce OpCrashAgent constraints: at most once, not first or last. @@ -60,15 +60,17 @@ func OperationSequenceGen(config PropertyTestConfig) *rapid.Generator[[]Operatio } // SingleOperationGen returns a rapid generator that produces a single operation -// whose kind and parameters respect the given config. -func SingleOperationGen(config PropertyTestConfig) *rapid.Generator[Operation] { +// whose kind and parameters respect the given config. seq is the operation's +// position in the sequence; it parameterizes draws that must be unique per +// operation (rename labels). +func SingleOperationGen(config PropertyTestConfig, seq int) *rapid.Generator[Operation] { return rapid.Custom(func(t *rapid.T) Operation { kinds := allowedKinds(config) kindIdx := rapid.IntRange(0, len(kinds)-1).Draw(t, "kind") kind := kinds[kindIdx] op := Operation{Kind: kind} - fillOperationFields(t, &op, config) + fillOperationFields(t, &op, config, seq) return op }) } @@ -163,7 +165,7 @@ func drawnOutcomeGen(t *rapid.T, label string) DrawnOutcome { } // fillOperationFields populates the kind-specific fields on the operation. -func fillOperationFields(t *rapid.T, op *Operation, config PropertyTestConfig) { +func fillOperationFields(t *rapid.T, op *Operation, config PropertyTestConfig, seq int) { pool := resourcePoolForConfig(config) slotCount := slotCountForConfig(config, pool) @@ -193,16 +195,15 @@ func fillOperationFields(t *rapid.T, op *Operation, config PropertyTestConfig) { // 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. + // property-only, or both. The label embeds the operation's sequence + // position, so no two renames in a sequence can produce the same + // label — label reuse would make slot resolution ambiguous. 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) + op.RenameSlotIndex = renameSlotIndexFromIDs(t, op.ResourceIDs) if op.RenameSlotIndex >= 0 { - op.RenameNewLabel = renameLabelGen(t) + op.RenameNewLabel = fmt.Sprintf("renamed-%d", seq) } } } @@ -543,34 +544,13 @@ func subsequenceGen(t *rapid.T, values []string, label string) []string { } // 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 { +// OpApply. Any slot is eligible: the forma builders thread label overrides +// through parent references, so renaming a slot that others reference keeps +// their `$res` blocks pointing at the current label. Returns -1 when `ids` +// is empty — the caller treats that as "no rename on this apply". +func renameSlotIndexFromIDs(t *rapid.T, ids []int) 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) + return rapid.SampledFrom(ids).Draw(t, "applyRenameSlot") } diff --git a/tests/blackbox/generators_test.go b/tests/blackbox/generators_test.go index 85b387738..2a1f34504 100644 --- a/tests/blackbox/generators_test.go +++ b/tests/blackbox/generators_test.go @@ -326,7 +326,7 @@ func TestResponseSequenceGen(t *testing.T) { StackCount: 2, } op := Operation{Kind: OpApply} - fillOperationFields(rt, &op, config) + fillOperationFields(rt, &op, config, 0) require.NotNil(t, op.DrawnOutcomes, "DrawnOutcomes should be non-nil when EnableFailures=true") // Should have entries for all resource slots in the target stack. @@ -347,7 +347,7 @@ func TestResponseSequenceGen(t *testing.T) { StackCount: 2, } op := Operation{Kind: OpApply} - fillOperationFields(rt, &op, config) + fillOperationFields(rt, &op, config, 0) assert.Nil(t, op.DrawnOutcomes, "DrawnOutcomes should be nil when EnableFailures=false") }) @@ -368,7 +368,7 @@ func TestResponseSequenceGen(t *testing.T) { // We need a pool for onDependents to be set. op := Operation{Kind: OpDestroy} - fillOperationFields(rt, &op, config) + fillOperationFields(rt, &op, config, 0) require.NotNil(t, op.DrawnOutcomes, "DrawnOutcomes should be non-nil for OpDestroy with EnableFailures") diff --git a/tests/blackbox/harness.go b/tests/blackbox/harness.go index abc11f72c..a23f4269e 100644 --- a/tests/blackbox/harness.go +++ b/tests/blackbox/harness.go @@ -54,6 +54,12 @@ type TestHarness struct { cancel context.CancelFunc pluginsDir string + // RenamesAccepted counts renames the agent accepted across all rapid + // iterations. Tests that enable rename assert it is non-zero after + // rapid.Check so a broken rename path cannot pass vacuously (a rejected + // or no-op rename executes no rename at all). + RenamesAccepted int + // Agent subprocess agentCmd *exec.Cmd agentLogFile *os.File diff --git a/tests/blackbox/invariants.go b/tests/blackbox/invariants.go index fc0717379..2f32bf386 100644 --- a/tests/blackbox/invariants.go +++ b/tests/blackbox/invariants.go @@ -18,15 +18,17 @@ import ( type ViolationKind int const ( - ViolationPhantomResource ViolationKind = iota // in inventory but not in cloud - ViolationOrphanedResource // in cloud but not in inventory - ViolationPropertyMismatch // inventory and cloud properties differ - ViolationCommandNotTerminal // command not in terminal state - 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) + ViolationPhantomResource ViolationKind = iota // in inventory but not in cloud + ViolationOrphanedResource // in cloud but not in inventory + ViolationPropertyMismatch // inventory and cloud properties differ + ViolationCommandNotTerminal // command not in terminal state + 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) + ViolationRenameIdentityChanged // a successful update RU changed the resource's NativeID or KSUID (RFC-0041) + ViolationRenameRecreatedResource // a renamed slot was fulfilled by a create — rename must be an in-place update (RFC-0041) ) // Violation describes a single invariant violation. @@ -731,19 +733,18 @@ func CheckOperationLogInvariants(opLog []testcontrol.OperationLogEntry) []Violat // 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. +// landed on it), the inventory must NOT carry a row at the old +// (Stack, Type, PreviousLabel) tuple with that slot's identity — that +// would mean the rename left the old row behind instead of renaming the +// existing one. The identity guard (NativeID/KSUID match) keeps a +// different slot legitimately renamed onto the freed label from being +// flagged. // // 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). +// (via SetNativeID), the row's Label must equal the slot's current label, +// and its KSUID must match the tracked KSUID. 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. func CheckRenameInvariants(model *StateModel, inventory []pkgmodel.Resource) []Violation { var violations []Violation if model == nil { @@ -769,15 +770,27 @@ func CheckRenameInvariants(model *StateModel, inventory []pkgmodel.Resource) []V 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, - ), - }) + hit, present := rows[oldKey] + if !present || !hit.Managed { + continue } + // Identity guard: only flag the row if it belongs to this slot's + // lineage. A different slot renamed onto the freed label is legal. + stackIdx := model.StackIndexByLabel(stack.Label) + trackedNativeID := model.GetNativeID(stackIdx, slotIdx) + trackedKsuid := model.GetKsuid(stackIdx, slotIdx) + sameLineage := (trackedNativeID != "" && hit.NativeID == trackedNativeID) || + (trackedKsuid != "" && hit.Ksuid == trackedKsuid) + if !sameLineage { + continue + } + 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, + ), + }) } } @@ -809,6 +822,18 @@ func CheckRenameInvariants(model *StateModel, inventory []pkgmodel.Resource) []V ), }) } + // KSUID stability: the row for a tracked NativeID must carry the + // KSUID the model recorded for that slot. A fresh KSUID under the + // same NativeID means a rename minted a new resource identity. + if trackedKsuid := model.GetKsuid(slot[0], slot[1]); trackedKsuid != "" && r.Ksuid != "" && r.Ksuid != trackedKsuid { + violations = append(violations, Violation{ + Kind: ViolationRenameIdentityChanged, + Message: fmt.Sprintf( + "inventory row NativeID=%s label=%q carries ksuid=%s but the model tracked ksuid=%s for slot (stack %d, slot %d)", + r.NativeID, r.Label, r.Ksuid, trackedKsuid, slot[0], slot[1], + ), + }) + } } return violations diff --git a/tests/blackbox/invariants_rename_test.go b/tests/blackbox/invariants_rename_test.go index 61137aca3..e5b2beb36 100644 --- a/tests/blackbox/invariants_rename_test.go +++ b/tests/blackbox/invariants_rename_test.go @@ -9,6 +9,8 @@ package blackbox import ( "testing" + "github.com/stretchr/testify/require" + pkgmodel "github.com/platform-engineering-labs/formae/pkg/model" "github.com/platform-engineering-labs/formae/tests/testcontrol" ) @@ -61,3 +63,58 @@ func TestCheckInvariants_SingleRowPerNativeID_OK(t *testing.T) { } } } + +// After a rename, a managed inventory row at the slot's previous label is a +// violation only when it belongs to that slot's lineage (same NativeID or +// KSUID). A different slot legitimately renamed onto the freed label must +// not be flagged. +func TestCheckRenameInvariants_OldLabelIdentityGuard(t *testing.T) { + model := NewStateModel(1, 3) + model.ApplyCreated(0, []int{1}, "") + model.SetNativeID(0, 1, "test-42") + model.SetKsuid(0, 1, "K_B") + model.RecordRename(0, 1, "renamed-0") + model.RecordRename(0, 1, "renamed-1") // PreviousLabel is now "renamed-0" + + // Another lineage's row occupies the freed label: legal, not flagged. + reused := []pkgmodel.Resource{ + {Stack: "stack-0", Type: "Test::Generic::Resource", Label: "renamed-0", NativeID: "test-77", Ksuid: "K_OTHER", Managed: true}, + {Stack: "stack-0", Type: "Test::Generic::Resource", Label: "renamed-1", NativeID: "test-42", Ksuid: "K_B", Managed: true}, + } + require.Empty(t, CheckRenameInvariants(model, reused), + "a different lineage on the freed label must not be flagged") + + // The slot's own lineage still on the old label: the rename leaked it. + // Fires the old-label check and, because the tracked NativeID carries + // the wrong label, the positive label-drift check as well. + leaked := []pkgmodel.Resource{ + {Stack: "stack-0", Type: "Test::Generic::Resource", Label: "renamed-0", NativeID: "test-42", Ksuid: "K_B", Managed: true}, + } + violations := CheckRenameInvariants(model, leaked) + kinds := make(map[ViolationKind]bool, len(violations)) + for _, v := range violations { + kinds[v.Kind] = true + } + require.True(t, kinds[ViolationRenameOldLabelStillPresent], "leaked old-label row must be flagged: %v", violations) +} + +// For a tracked NativeID the inventory row must carry both the current +// label and the tracked KSUID — a fresh KSUID under the same NativeID means +// the rename minted a new resource identity. +func TestCheckRenameInvariants_KsuidStability(t *testing.T) { + model := NewStateModel(1, 3) + model.ApplyCreated(0, []int{1}, "") + model.SetNativeID(0, 1, "test-42") + model.SetKsuid(0, 1, "K_B") + model.RecordRename(0, 1, "renamed-0") + + inventory := []pkgmodel.Resource{ + {Stack: "stack-0", Type: "Test::Generic::Resource", Label: "renamed-0", NativeID: "test-42", Ksuid: "K_FRESH", Managed: true}, + } + violations := CheckRenameInvariants(model, inventory) + require.Len(t, violations, 1) + require.Equal(t, ViolationRenameIdentityChanged, violations[0].Kind) + + inventory[0].Ksuid = "K_B" + require.Empty(t, CheckRenameInvariants(model, inventory)) +} diff --git a/tests/blackbox/operations.go b/tests/blackbox/operations.go index 932625440..33c0cd7e4 100644 --- a/tests/blackbox/operations.go +++ b/tests/blackbox/operations.go @@ -154,13 +154,25 @@ type ResourceSnapshot struct { PreviousLabel string } +// PendingRename records that a command carried a rename overlay: the forma +// declared the slot under NewLabel with Alias set to OldLabel. On a +// successful command the engine must have performed it as an in-place update +// (an update RU carrying OldLabel); the drain asserts that. +type PendingRename struct { + StackIndex int + SlotIndex int + OldLabel string + NewLabel string +} + // AcceptedCommand tracks a command that was accepted by the agent during the chaos phase. type AcceptedCommand struct { CommandID string Snapshots []ResourceSnapshot // pre-command state for revert on cancel OpLogSize int // operation log length immediately after acceptance RequestedSlots []ResourceSlotRef - IsReconcile bool // true if the command was reconcile mode (properties = full desired state) + IsReconcile bool // true if the command was reconcile mode (properties = full desired state) + Rename *PendingRename // set when the command carried a rename overlay // Resolved is true when the cancel handler has already processed this // command. The command remains in AcceptedCommands so that // DrainPendingCommands can include its outcome when resolving conflicts diff --git a/tests/blackbox/property_test.go b/tests/blackbox/property_test.go index 402f08e42..e29e7ba91 100644 --- a/tests/blackbox/property_test.go +++ b/tests/blackbox/property_test.go @@ -151,17 +151,7 @@ func TestProperty_FullChaos(t *testing.T) { EnableForceReconcile: true, EnableTTL: true, EnableCrashInjection: true, - // EnableRename intentionally off here. With rename folded into - // the OpApply path (RFC-0041) a rename rides inside an Update - // — the same prediction model that already drifts under chaos - // (cancel × ForceReconcile × partial-success applies on - // non-renamed slots). Leaving rename off keeps FullChaos - // focused on the existing prediction-correctness story until - // the harness's expected-State/Properties prediction is - // hardened. Identity-only coverage for rename lives in the - // LabelForResource-aware invariants (CheckRenameInvariants) - // and the duplicate-NativeID guard, which both fire under - // AssertAllInvariants for any test that turns EnableRename on. + EnableRename: true, } h.ResetAgentState(t) @@ -221,11 +211,22 @@ func TestProperty_RenameViaApply(t *testing.T) { for i, op := range ops { op.SequenceNum = i h.ExecuteOperation(t, &op, model) + // Drain after every operation: with a single stack, a second + // apply submitted while the first is in flight is rejected by + // the agent's stack-overlap admission check. Rename needs the + // slot to already exist, i.e. exactly those later applies — + // serializing keeps them (and their renames) landing. + h.DrainPendingCommands(t, model, 30*time.Second) } - h.DrainPendingCommands(t, model, 30*time.Second) - h.TriggerSyncAndWait(t) + h.TriggerSyncAndWait(t, model) h.AssertAllInvariants(t, model) }) + + // Whether any generated sequence exercises a rename depends on the + // draws (the slot must already exist when the rename-carrying apply + // executes), so the count is informational here. Guaranteed rename + // coverage lives in TestRenameViaApply_Deterministic. + t.Logf("TestProperty_RenameViaApply: %d renames accepted across the run", h.RenamesAccepted) }) } diff --git a/tests/blackbox/rename_test.go b/tests/blackbox/rename_test.go new file mode 100644 index 000000000..dc874b1c9 --- /dev/null +++ b/tests/blackbox/rename_test.go @@ -0,0 +1,71 @@ +// © 2026 Platform Engineering Labs Inc. +// +// SPDX-License-Identifier: FSL-1.1-ALv2 + +//go:build integration || property + +package blackbox + +import ( + "testing" + "time" + + "github.com/platform-engineering-labs/formae/internal/metastructure/testutil" + "github.com/stretchr/testify/require" +) + +// Deterministic rename coverage: create resources, rename one via the apply +// overlay, and assert the rename invariants against real inventory. The +// rapid-driven TestProperty_RenameViaApply explores rename under generated +// sequences, but whether any sequence exercises a rename depends on the +// draws; this test guarantees the rename path executes on every run. +func TestRenameViaApply_Deterministic(t *testing.T) { + testutil.RunTestFromProjectRoot(t, func(t *testing.T) { + h := NewTestHarness(t, 10*time.Second) + defer h.Cleanup() + h.ResetAgentState(t) + config := PropertyTestConfig{ResourceCount: 10, StackCount: 1, EnableRename: true} + model := NewStateModel(config.StackCount, config.ResourceCount) + h.SetupStacks(t, model, config) + + create := Operation{Kind: OpApply, ApplyMode: "reconcile", StackIndex: 0, + ResourceIDs: []int{0, 1, 2}, Properties: defaultDestroyParentProps, + ChildProperties: defaultDestroyChildProps, RenameSlotIndex: -1} + h.ExecuteOperation(t, &create, model) + h.DrainPendingCommands(t, model, 30*time.Second) + require.Equal(t, StateExists, model.Resource(0, 0).State, "create must land before the rename") + + // Rename the parent (slot 0) while its child (slot 1) references it: + // the overlay rewrites the child's $res reference to the new label. + rename := Operation{Kind: OpApply, ApplyMode: "reconcile", StackIndex: 0, + ResourceIDs: []int{0, 1, 2}, Properties: defaultDestroyParentProps, + ChildProperties: defaultDestroyChildProps, RenameSlotIndex: 0, RenameNewLabel: "renamed-0"} + h.ExecuteOperation(t, &rename, model) + h.DrainPendingCommands(t, model, 30*time.Second) + + require.NotZero(t, h.RenamesAccepted, "the rename apply must be accepted") + require.Equal(t, "renamed-0", model.Resource(0, 0).CurrentLabel) + + // A second rename of the same slot exercises the PreviousLabel chain. + rename2 := Operation{Kind: OpApply, ApplyMode: "reconcile", StackIndex: 0, + ResourceIDs: []int{0, 1, 2}, Properties: defaultDestroyParentProps, + ChildProperties: defaultDestroyChildProps, RenameSlotIndex: 0, RenameNewLabel: "renamed-1"} + h.ExecuteOperation(t, &rename2, model) + h.DrainPendingCommands(t, model, 30*time.Second) + + h.TriggerSyncAndWait(t, model) + h.AssertAllInvariants(t, model) + + managed, _, err := h.extractManagedAndUnmanagedInventory() + require.NoError(t, err) + var labels []string + for _, r := range managed { + if r.Stack == "stack-0" { + labels = append(labels, r.Label) + } + } + require.Contains(t, labels, "renamed-1", "inventory must carry the renamed label") + require.NotContains(t, labels, "renamed-0", "the intermediate label must be gone") + require.NotContains(t, labels, "res-stack-0-a", "the original label must be gone") + }) +} diff --git a/tests/blackbox/state_model.go b/tests/blackbox/state_model.go index 35e3fbf95..0aa16bbfd 100644 --- a/tests/blackbox/state_model.go +++ b/tests/blackbox/state_model.go @@ -84,6 +84,14 @@ type StateModel struct { // Populated from command response ResourceUpdate.NativeID on successful // creates/updates. Cleared on successful deletes. NativeIDs map[string]string + // Ksuids maps "stackIdx:slotIdx" → the resource's KSUID, populated from + // command response ResourceUpdate.ResourceID the same way as NativeIDs. + // A rename must keep both stable; the correction path checks that. + Ksuids map[string]string + // PendingViolations collects violations detected while correcting the + // model from command outcomes (e.g. an update RU that changed a + // resource's identity). Drained and asserted by AssertAllInvariants. + PendingViolations []Violation // DriftExcludedStacks marks stacks whose resources are no longer valid // drift targets this iteration: a canceled changeset can leave its // resources registered as in-progress with the synchronizer for an @@ -145,6 +153,7 @@ func NewStateModel(stackCount, resourcesPerStack int) *StateModel { UnmanagedResources: make(map[string]*ExpectedUnmanagedResource), AuthoritativeSlots: make(map[string]bool), NativeIDs: make(map[string]string), + Ksuids: make(map[string]string), DriftExcludedStacks: make(map[int]bool), } } @@ -196,6 +205,27 @@ func (m *StateModel) SupersedeSlots(refs []ResourceSlotRef) { } } +func (m *StateModel) SetKsuid(stackIdx, slotIdx int, ksuid string) { + if ksuid != "" { + m.Ksuids[nativeIDKey(stackIdx, slotIdx)] = ksuid + } +} + +func (m *StateModel) GetKsuid(stackIdx, slotIdx int) string { + return m.Ksuids[nativeIDKey(stackIdx, slotIdx)] +} + +func (m *StateModel) ClearKsuid(stackIdx, slotIdx int) { + delete(m.Ksuids, nativeIDKey(stackIdx, slotIdx)) +} + +// AddPendingViolation records a violation detected outside the invariant +// checks (e.g. during command-outcome correction). Asserted and cleared by +// AssertAllInvariants. +func (m *StateModel) AddPendingViolation(v Violation) { + m.PendingViolations = append(m.PendingViolations, v) +} + // FindDriftEligibleResource finds a managed resource that exists in the model, // has a tracked NativeID, and is untouched by in-flight commands. Used for // selecting OOB drift targets: drift is absorbed synchronously via a forced @@ -263,21 +293,6 @@ func (m *StateModel) FindDriftEligibleResource(sequenceNum int) (stackIdx, slotI return c.stackIdx, c.slotIdx, c.stackLabel, c.label, c.rType, c.nativeID, true } -// NativeIDsByLabel returns a map of "stackLabel:resourceLabel" → NativeID for -// all tracked native IDs. This matches the format expected by -// buildPluginOpSequences and resolveReadMatchKey. -func (m *StateModel) NativeIDsByLabel() map[string]string { - result := make(map[string]string, len(m.NativeIDs)) - for key, nativeID := range m.NativeIDs { - var stackIdx, slotIdx int - fmt.Sscanf(key, "%d:%d", &stackIdx, &slotIdx) - stackLabel := m.Stacks[stackIdx].Label - label := m.LabelForResource(stackIdx, slotIdx) - result[stackLabel+":"+label] = nativeID - } - return result -} - func (m *StateModel) MarkAuthoritativeSlot(stackIdx, slotIdx int) { m.AuthoritativeSlots[slotKeyString(stackIdx, slotIdx)] = true } @@ -530,6 +545,11 @@ func (m *StateModel) ApplyCreatedResolved(stackIndex int, propertiesByID map[int } // ApplyDestroyed marks the given resources on the given stack as not existing. +// The rename overlay is deliberately kept: a destroyed slot's label stays at +// its renamed value, so a later recreate targets the same label the engine +// last knew the slot by. Clearing it would make a recreate after a FAILED +// destroy (real resource still alive under the renamed label) declare a +// second resource under the default label. func (m *StateModel) ApplyDestroyed(stackIndex int, resourceIDs []int) { stack := &m.Stacks[stackIndex] for _, id := range resourceIDs { diff --git a/tests/blackbox/state_model_test.go b/tests/blackbox/state_model_test.go index 667715ea9..fa26f0e7a 100644 --- a/tests/blackbox/state_model_test.go +++ b/tests/blackbox/state_model_test.go @@ -42,7 +42,7 @@ func TestCorrectModelFromCommandOutcome_FailedCreateForcesNotExist(t *testing.T) } corrected := map[struct{ stackIdx, slotIdx int }]bool{} - correctModelFromCommandOutcome(t, cmd, model, nil, snapshots, corrected, true, nil) + correctModelFromCommandOutcome(t, cmd, model, nil, snapshots, corrected, true, nil, nil) require.Equal(t, StateNotExist, model.Resource(0, 1).State, "a failed create must leave the slot NotExist, not revert to a stale Exists snapshot") @@ -68,7 +68,7 @@ func TestCorrectModelFromCommandOutcome_FailedDeleteRevertsToSnapshot(t *testing } corrected := map[struct{ stackIdx, slotIdx int }]bool{} - correctModelFromCommandOutcome(t, cmd, model, nil, snapshots, corrected, true, nil) + correctModelFromCommandOutcome(t, cmd, model, nil, snapshots, corrected, true, nil, nil) require.Equal(t, StateExists, model.Resource(0, 1).State, "a failed delete leaves the resource in place, reverting to its Exists snapshot") @@ -95,8 +95,8 @@ func TestCorrectModelFromCommandOutcome_ReverseOrderFailedCreatesStayNotExist(t corrected := map[struct{ stackIdx, slotIdx int }]bool{} newerSnap := []ResourceSnapshot{{StackIndex: 0, SlotIndex: 1, State: StateNotExist}} olderSnap := []ResourceSnapshot{{StackIndex: 0, SlotIndex: 1, State: StateExists}} // stale - correctModelFromCommandOutcome(t, newerCmd, model, nil, newerSnap, corrected, true, nil) - correctModelFromCommandOutcome(t, olderCmd, model, nil, olderSnap, corrected, true, nil) + correctModelFromCommandOutcome(t, newerCmd, model, nil, newerSnap, corrected, true, nil, nil) + correctModelFromCommandOutcome(t, olderCmd, model, nil, olderSnap, corrected, true, nil, nil) require.Equal(t, StateNotExist, model.Resource(0, 1).State, "an older failed-create command must not resurrect a slot from its stale Exists snapshot") @@ -579,7 +579,7 @@ func TestCorrectModelFromCommandOutcome_UnmentionedCrossStackSlotReverts(t *test ResourceUpdates: nil, // the command died before reaching this slot } corrected := map[struct{ stackIdx, slotIdx int }]bool{} - correctModelFromCommandOutcome(t, cmd, model, model.Pool, snapshots, corrected, false, nil) + correctModelFromCommandOutcome(t, cmd, model, model.Pool, snapshots, corrected, false, nil, nil) require.Equal(t, StateNotExist, model.Resource(1, crossIdx).State, "an unmentioned cross-stack slot in a failed command reverts to its snapshot") @@ -713,7 +713,7 @@ func TestCorrectModelFromCommandOutcome_TTLSupersededSlotStaysDestroyed(t *testi } corrected := map[struct{ stackIdx, slotIdx int }]bool{} ac := model.AcceptedCommands[0] - correctModelFromCommandOutcome(t, cmd, model, model.Pool, ac.Snapshots, corrected, true, ac.SupersededSlots) + correctModelFromCommandOutcome(t, cmd, model, model.Pool, ac.Snapshots, corrected, true, ac.SupersededSlots, nil) require.Equal(t, StateNotExist, model.Resource(2, xslot).State, "a TTL-destroyed slot must not be resurrected by a stale command's create RU") @@ -745,9 +745,116 @@ func TestCorrectModelFromCommandOutcome_UnmentionedSlotRevertsProperties(t *test }}, } corrected := map[struct{ stackIdx, slotIdx int }]bool{} - correctModelFromCommandOutcome(t, cmd, model, nil, snapshots, corrected, false, nil) + correctModelFromCommandOutcome(t, cmd, model, nil, snapshots, corrected, false, nil, nil) require.Equal(t, StateExists, model.Resource(0, 1).State) require.Equal(t, `{"Value":"v1"}`, model.Resource(0, 1).Properties, "optimistic properties must revert to the snapshot when the failed command never touched the slot") } + +// A failed command whose renamed slot produced no resource update must have +// the optimistic rename rolled back, even though the slot's State never +// changed — the engine never persisted the new label, so keeping it would +// let the model track a label that does not exist in inventory. +func TestCorrectModelFromCommandOutcome_UnmentionedSlotRevertsLabels(t *testing.T) { + model := NewStateModel(1, 3) + model.ApplyCreated(0, []int{1}, "") + model.RecordRename(0, 1, "renamed-0") // optimistic: rename accepted + + snapshots := []ResourceSnapshot{{StackIndex: 0, SlotIndex: 1, State: StateExists}} // pre-rename: no overlay + cmd := &apimodel.Command{ + CommandID: "cmd-failed-rename", + State: "FinishedWithErrors", + ResourceUpdates: nil, // the renamed slot never got a resource update + } + + corrected := map[struct{ stackIdx, slotIdx int }]bool{} + correctModelFromCommandOutcome(t, cmd, model, nil, snapshots, corrected, true, nil, nil) + + require.Empty(t, model.Resource(0, 1).CurrentLabel, + "the optimistic rename must be rolled back when the slot is unmentioned in a failed command") + require.Empty(t, model.Resource(0, 1).PreviousLabel) + require.Equal(t, StateExists, model.Resource(0, 1).State) +} + +// A successful command that carried a rename must contain a success update +// RU at the renamed label — a destroy+recreate (or a dropped alias that made +// the engine treat the new label as a fresh resource) produces a create RU +// there instead. +func TestCorrectModelFromCommandOutcome_RenameFulfilledByCreateIsViolation(t *testing.T) { + model := NewStateModel(1, 3) + model.ApplyCreated(0, []int{1}, "") + model.SetNativeID(0, 1, "test-42") + model.RecordRename(0, 1, "renamed-0") + + cmd := &apimodel.Command{ + CommandID: "cmd-recreated-rename", + State: "Success", + ResourceUpdates: []apimodel.ResourceUpdate{{ + StackName: "stack-0", + ResourceLabel: "renamed-0", + Operation: "create", // rename executed as destroy+recreate + State: "Success", + NativeID: "test-99", + }}, + } + + corrected := map[struct{ stackIdx, slotIdx int }]bool{} + rename := &PendingRename{StackIndex: 0, SlotIndex: 1, OldLabel: "res-stack-0-b", NewLabel: "renamed-0"} + correctModelFromCommandOutcome(t, cmd, model, nil, nil, corrected, false, nil, rename) + + require.NotEmpty(t, model.PendingViolations) + require.Equal(t, ViolationRenameRecreatedResource, model.PendingViolations[0].Kind) + + // The same command with an in-place update RU produces no violation. + model2 := NewStateModel(1, 3) + model2.ApplyCreated(0, []int{1}, "") + model2.SetNativeID(0, 1, "test-42") + model2.RecordRename(0, 1, "renamed-0") + cmd2 := &apimodel.Command{ + CommandID: "cmd-genuine-rename", + State: "Success", + ResourceUpdates: []apimodel.ResourceUpdate{{ + StackName: "stack-0", + ResourceLabel: "renamed-0", + Operation: "update", + State: "Success", + NativeID: "test-42", + }}, + } + correctModelFromCommandOutcome(t, cmd2, model2, nil, nil, map[struct{ stackIdx, slotIdx int }]bool{}, false, nil, rename) + require.Empty(t, model2.PendingViolations) +} + +// An update RU never changes a resource's identity — a different NativeID or +// KSUID in the response means the engine replaced the resource instead of +// updating it in place. The check must fire BEFORE the model adopts the new +// identity, or the final invariants compare against the adopted value and +// pass vacuously. +func TestCorrectModelFromCommandOutcome_UpdateChangingIdentityIsViolation(t *testing.T) { + model := NewStateModel(1, 3) + model.ApplyCreated(0, []int{1}, "") + model.SetNativeID(0, 1, "test-42") + model.SetKsuid(0, 1, "K_OLD") + + cmd := &apimodel.Command{ + CommandID: "cmd-identity-swap", + State: "Success", + ResourceUpdates: []apimodel.ResourceUpdate{{ + StackName: "stack-0", + ResourceLabel: "res-stack-0-b", + Operation: "update", + State: "Success", + NativeID: "test-99", + ResourceID: "K_NEW", + }}, + } + + corrected := map[struct{ stackIdx, slotIdx int }]bool{} + correctModelFromCommandOutcome(t, cmd, model, nil, nil, corrected, false, nil, nil) + + require.Len(t, model.PendingViolations, 2, "both the NativeID and the KSUID change must be flagged") + for _, v := range model.PendingViolations { + require.Equal(t, ViolationRenameIdentityChanged, v.Kind) + } +}