amadan
hostbox: Policy.Check takes a rollback's hold owner from the claim's intent (Task 3b fix round 1)
Under restoring, in the source's recovery phases (claimed, probing, probed, opened), the registry hold must be the claim's intent.rollback, a valid rollback id; otherwise no hold matches, and there is no fallback to the op. Landing, moving and reverting keep the selection's op. End-to-end host test through Policy.Check and MovesClaimPolicy.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
3 files changed,
+155
−2
internal/host/movespolicy_test.go+39 −0internal/hostbox/wakepolicy.go+32 −2internal/hostbox/wakepolicy_test.go+84 −0
diff --git a/internal/host/movespolicy_test.go b/internal/host/movespolicy_test.go| index 39982b4e9..a1fe4a571 100644 |
| --- a/internal/host/movespolicy_test.go |
| +++ b/internal/host/movespolicy_test.go |
| @@ -1193,3 +1193,42 @@ func TestMovesClaimPolicyBindsRecoveryInputs(t *testing.T) { |
| rpRefused(t, moves(t.Context(), c, mpSelection(c, "held-config"), cfgOf(recovered)), "the config is not the pinned one") |
| }) |
| } |
| + |
| +// End to end (Task 3b fix round 1): the source's return writes through |
| +// hostbox.Policy.Check with its row held by the rollback id, as the J3 |
| +// hand-over leaves it, and MovesClaimPolicy as Policy.Moves. Policy.Check |
| +// takes the hold's owner from the claim's intent.rollback; the move id's |
| +// hold, another rollback's, the target asking and the wrong phase refuse. |
| +func TestMovesClaimPolicyRestoringThroughPolicyCheck(t *testing.T) { |
| + through := func(hold string, rec *provision.Record, c *hostclaim.Claim, s hostbox.Selection, id hostwrite.Identity) error { |
| + p := hostbox.Policy{Moves: MovesClaimPolicy((&mpReader{rec: rec}).read), Held: func(context.Context, string) (string, error) { return hold, nil }} |
| + return p.Check(t.Context(), c, s, id) |
| + } |
| + for phase, action := range map[string]string{ |
| + provision.RbSourceLocal: "held-timers", |
| + provision.RbSourceConfigured: "held-config", |
| + provision.RbSourceRestored: "install", |
| + provision.RbSourceProbing: "spawn", |
| + } { |
| + c := rpRestoringClaim(rpClaimPhase(phase)) |
| + s := rpSelection(c, action) |
| + if err := through(rpRB, rpRestoringRecord(phase), c, s, rpIdentity(action)); err != nil { |
| + t.Errorf("%s/%s held by the rollback: %v", phase, action, err) |
| + } |
| + for _, hold := range []string{mpOp, "rb_zzzzzzzzzzzzzzzzzzzzzzzzzz"} { |
| + if err := through(hold, rpRestoringRecord(phase), c, s, rpIdentity(action)); !errors.Is(err, hostbox.ErrChanged) { |
| + t.Errorf("%s/%s held by %s: %v", phase, action, hold, err) |
| + } |
| + } |
| + } |
| + c := rpRestoringClaim(hostclaim.PhaseClaimed) |
| + s := rpSelection(c, "held-timers") |
| + s.Box, s.Region = mpTarget, "uk" |
| + if err := through(rpRB, rpRestoringRecord(provision.RbSourceLocal), c, s, hostwrite.Identity{}); err == nil { |
| + t.Fatal("the target's timers granted under the rollback's hold") |
| + } |
| + s = rpSelection(c, "held-timers") |
| + if err := through(rpRB, rpRestoringRecord(provision.RbSourceConfigured), c, s, hostwrite.Identity{}); !errors.Is(err, ErrMovesRefused) { |
| + t.Fatalf("held-timers at configured: %v", err) |
| + } |
| +} |
diff --git a/internal/hostbox/wakepolicy.go b/internal/hostbox/wakepolicy.go| index 9c3c00d65..51f7c0a0b 100644 |
| --- a/internal/hostbox/wakepolicy.go |
| +++ b/internal/hostbox/wakepolicy.go |
| @@ -6,6 +6,7 @@ import ( |
| "github.com/carlosframework/platform/internal/hostclaim" |
| "github.com/carlosframework/platform/internal/hostwrite" |
| + "github.com/carlosframework/platform/internal/provision" |
| ) |
| // WakePhase is the K6 execution table. Participant membership alone never starts code. |
| @@ -68,14 +69,15 @@ func (p Policy) Check(ctx context.Context, c *hostclaim.Claim, s Selection, id h |
| if p.Moves == nil || s.Op == "" || c.Op.ID != s.Op || c.Op.Exec != s.Box { |
| return ErrChanged |
| } |
| + owner, owned := movesHoldOwner(c, s) |
| switch s.Action { |
| case "held-config", "held-release", "held-timers": |
| - if hold != s.Op { |
| + if !owned || hold != owner { |
| return ErrChanged |
| } |
| case "restore", "spawn", "seed", "writer", "install": |
| if hold != "" { |
| - if hold != s.Op { |
| + if !owned || hold != owner { |
| return ErrChanged |
| } |
| } else if err := WakePhase(c, s.Box, false); err != nil { |
| @@ -87,4 +89,32 @@ func (p Policy) Check(ctx context.Context, c *hostclaim.Claim, s Selection, id h |
| return p.Moves(ctx, c, s, id) |
| } |
| +// movesHoldOwner is the op a moves selection's registry hold must name, and |
| +// false when no hold can be the selection's. A landing, a moving op and a |
| +// revert hold the row by their op id, the selection's. A rollback keeps the |
| +// move id as the claim's op id (hostclaim.RollBack) and hands the row to the |
| +// rollback id (ruling J3), so a restoring op in the source's recovery phases |
| +// expects the rollback id. That id comes only from the claim's |
| +// intent.rollback, as RollBack recorded it, never from the selection, the |
| +// request or a registry field, and it must be a valid rollback id. A |
| +// restoring op without one, or outside those phases, matches no hold and |
| +// never falls back to the selection's op (Plan 4 Task 3b fix round 1). |
| +func movesHoldOwner(c *hostclaim.Claim, s Selection) (string, bool) { |
| + if c.Op.Kind != hostclaim.KindRestoring { |
| + return s.Op, true |
| + } |
| + rb := c.Op.Intent["rollback"] |
| + if !restoringHoldPhases[c.Op.Phase] || !provision.ValidRollbackID(rb) { |
| + return "", false |
| + } |
| + return rb, true |
| +} |
| + |
| +// restoringHoldPhases are the source's recovery phases of a rollback |
| +// (hostclaim.RecoveryPhaseOrder(KindRestoring) from claimed on): the source |
| +// executes, and its registry row is held by the rollback id. |
| +var restoringHoldPhases = map[string]bool{ |
| + hostclaim.PhaseClaimed: true, hostclaim.PhaseProbing: true, hostclaim.PhaseProbed: true, hostclaim.PhaseOpened: true, |
| +} |
| + |
| const RequestLimit = 120 * time.Second |
diff --git a/internal/hostbox/wakepolicy_test.go b/internal/hostbox/wakepolicy_test.go| index f7dd9add5..b655bfc3b 100644 |
| --- a/internal/hostbox/wakepolicy_test.go |
| +++ b/internal/hostbox/wakepolicy_test.go |
| @@ -97,3 +97,87 @@ func TestPolicyOrdinaryWakeAfterReleaseHold(t *testing.T) { |
| } |
| } |
| } |
| + |
| +// A rollback keeps the move id as the claim's op id (hostclaim.RollBack) and |
| +// hands the registry row to the rollback id (ruling J3), so under restoring, |
| +// in the source's recovery phases, the hold Policy.Check expects is the |
| +// claim's intent.rollback, a valid rollback id (Plan 4 Task 3b fix round 1). |
| +// Landing, moving and reverting keep the selection's op. A restoring op with |
| +// no valid rollback id, or outside those phases, matches no hold and never |
| +// falls back to the op. |
| +func TestPolicyHoldOwnerUnderRollback(t *testing.T) { |
| + const mv, rb, otherRB = "mv_move", "rb_abcdefghijklmnopqrstuvwxyz", "rb_zzzzzzzzzzzzzzzzzzzzzzzzzz" |
| + check := func(kind, phase, hold string, intent map[string]string, action string) error { |
| + c := &hostclaim.Claim{Holder: &hostclaim.Holder{}, Op: hostclaim.Op{Kind: kind, Phase: phase, ID: mv, Exec: "box", Intent: intent}} |
| + p := Policy{Held: func(context.Context, string) (string, error) { return hold, nil }, Moves: func(context.Context, *hostclaim.Claim, Selection, hostwrite.Identity) error { return nil }} |
| + return p.Check(t.Context(), c, Selection{Host: "x.example", Box: "box", Op: mv, Action: action}, hostwrite.Identity{}) |
| + } |
| + unheldWake := map[string]bool{"restore": true, "spawn": true, "seed": true, "writer": true, "install": true} |
| + restoring := map[string]string{"rollback": rb, "to": "uk"} |
| + actions := []string{"held-config", "held-release", "held-timers", "restore", "spawn", "seed", "writer", "install"} |
| + sourcePhases := []string{hostclaim.PhaseClaimed, hostclaim.PhaseProbing, hostclaim.PhaseProbed, hostclaim.PhaseOpened} |
| + for _, action := range actions { |
| + for _, phase := range sourcePhases { |
| + if err := check(hostclaim.KindRestoring, phase, rb, restoring, action); err != nil { |
| + t.Errorf("restoring/%s, held by its rollback, %s: %v", phase, action, err) |
| + } |
| + for name, hold := range map[string]string{"the move id": mv, "another rollback": otherRB, "an unrelated op": "op"} { |
| + if err := check(hostclaim.KindRestoring, phase, hold, restoring, action); !errors.Is(err, ErrChanged) { |
| + t.Errorf("restoring/%s, held by %s, %s: %v", phase, name, action, err) |
| + } |
| + } |
| + } |
| + for name, intent := range map[string]map[string]string{ |
| + "no intent": {}, |
| + "no rollback": {"to": "uk"}, |
| + "an empty rollback": {"rollback": ""}, |
| + "the move id as rollback": {"rollback": mv}, |
| + "a short rollback id": {"rollback": "rb_abc"}, |
| + "an upper-case rollback": {"rollback": "RB_ABCDEFGHIJKLMNOPQRSTUVWXYZ"}, |
| + "a non-base32 rollback id": {"rollback": "rb_abcdefghijklmnopqrstuvwx01"}, |
| + "a padded rollback id": {"rollback": rb + " "}, |
| + } { |
| + for _, hold := range []string{mv, rb, intent["rollback"], ""} { |
| + if hold == "" && unheldWake[action] { |
| + continue // unheld: WakePhase decides, unchanged |
| + } |
| + if err := check(hostclaim.KindRestoring, hostclaim.PhaseClaimed, hold, intent, action); !errors.Is(err, ErrChanged) { |
| + t.Errorf("restoring with %s, held by %q, %s: %v", name, hold, action, err) |
| + } |
| + } |
| + } |
| + for _, phase := range []string{hostclaim.PhaseReleasing, hostclaim.PhaseReleased, "", hostclaim.PhaseSealed, hostclaim.PhaseCommitted} { |
| + for _, hold := range []string{rb, mv} { |
| + if err := check(hostclaim.KindRestoring, phase, hold, restoring, action); !errors.Is(err, ErrChanged) { |
| + t.Errorf("restoring outside the source's recovery phases (%q), held by %q, %s: %v", phase, hold, action, err) |
| + } |
| + } |
| + } |
| + for _, kind := range []string{hostclaim.KindLanding, hostclaim.KindMoving, hostclaim.KindReverting} { |
| + if err := check(kind, hostclaim.PhaseProbing, rb, restoring, action); !errors.Is(err, ErrChanged) { |
| + t.Errorf("%s held by a rollback id, %s: %v", kind, action, err) |
| + } |
| + if err := check(kind, hostclaim.PhaseProbing, mv, restoring, action); err != nil { |
| + t.Errorf("%s held by its op, %s: %v", kind, action, err) |
| + } |
| + } |
| + } |
| + // The rollback id comes from the claim alone: a selection naming the |
| + // rollback id as its op does not match the claim's op. |
| + c := &hostclaim.Claim{Holder: &hostclaim.Holder{}, Op: hostclaim.Op{Kind: hostclaim.KindRestoring, Phase: hostclaim.PhaseClaimed, ID: mv, Exec: "box", Intent: restoring}} |
| + p := Policy{Held: func(context.Context, string) (string, error) { return rb, nil }, Moves: func(context.Context, *hostclaim.Claim, Selection, hostwrite.Identity) error { return nil }} |
| + if err := p.Check(t.Context(), c, Selection{Host: "x.example", Box: "box", Op: rb, Action: "held-timers"}, hostwrite.Identity{}); !errors.Is(err, ErrChanged) { |
| + t.Fatalf("a selection naming the rollback: %v", err) |
| + } |
| + // The source's recovery phases are hostclaim's restoring order from |
| + // claimed on. |
| + order := hostclaim.RecoveryPhaseOrder(hostclaim.KindRestoring) |
| + if len(order) != 6 || order[2] != hostclaim.PhaseClaimed || len(restoringHoldPhases) != 4 { |
| + t.Fatalf("restoring order %v, hold phases %v", order, restoringHoldPhases) |
| + } |
| + for _, phase := range order[2:] { |
| + if !restoringHoldPhases[phase] { |
| + t.Fatalf("%s is not a hold phase", phase) |
| + } |
| + } |
| +} |