amadan

carlos / platform Public

zdd plan 2 final fix: parser-only generation refusals; closed helper docs

T8: the malformed-generation test passed for unrelated reasons (no
file, or a file naming another host). Each case now has an otherwise
valid, host-matching launch file planted wherever a lenient parser could
look, the planted file is shown to run for the well-formed instance, and
every malformed case must fail with launchTarget's own error and no
exec. Adds the traversal (../x:1) and extra-colon (a:b:1) cases; the
misplaced-host case is kept separately.

T12: spawnClaimedGen's doc and its test comment now describe a closed
helper, and the doc notes that main's RoleLaunch.Start is closed too, so
a later plan needs an isolated root launch protocol that carries the
reservations, not a re-route.

Final review triage T8 and T12.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
claude 00d0dea98fb21c6ae5fccf46189ed878a7b83bfa parent 273ed2b
3 files changed, +60 −17
  • internal/activator/claimspawn.go +10 −6
  • internal/activator/claimspawn_test.go +4 −3
  • internal/instanceexec/exec_test.go +46 −8
diff --git a/internal/activator/claimspawn.go b/internal/activator/claimspawn.go
index 578260f65..09a6c7a83 100644
--- a/internal/activator/claimspawn.go
+++ b/internal/activator/claimspawn.go
@@ -22,12 +22,16 @@ func spawnClaimedBefore(ctx context.Context, enabled bool, manager *cgroup.Manag
return nil, nil, ErrClaimIsolationUnavailable
}
-// spawnClaimedGen pins and reserves a generation child under the entry's
-// parent for the generation layout under claims (zero-downtime spec section
-// 7.3). It stays closed until main's isolated root launcher can carry claim
-// reservations, like spawnClaimedBefore. A later plan must route claims-on
-// generation spawns through that launcher rather than reopen an in-process
-// path.
+// spawnClaimedGen is the claims-on spawn of a generation child under the
+// entry's parent (zero-downtime spec section 7.3), and it is closed: it
+// pins nothing, reserves nothing, never builds the command and never
+// enters the host write. Invalid arguments answer hostwrite.ErrUnavailable;
+// every other call answers ErrClaimIsolationUnavailable, like
+// spawnClaimedBefore. Main's RoleLaunch.Start is closed too, so routing a
+// claimed generation spawn there would not open it either. A later plan
+// needs an isolated root launch protocol that carries the claim
+// reservations and the final authority check, rather than a re-route or
+// an in-process path.
func spawnClaimedGen(ctx context.Context, parent *cgroup.Prepared, n uint64, tier cgroup.Limit, selected hostwrite.Work, identity hostwrite.Identity, build func() *exec.Cmd, before func() error) (*exec.Cmd, *cgroup.Prepared, error) {
if parent == nil || selected == nil || build == nil || n == 0 {
return nil, nil, hostwrite.ErrUnavailable
diff --git a/internal/activator/claimspawn_test.go b/internal/activator/claimspawn_test.go
index 5c5d45236..0d17f4672 100644
--- a/internal/activator/claimspawn_test.go
+++ b/internal/activator/claimspawn_test.go
@@ -40,9 +40,10 @@ func TestClaimSpawnNeverFallsBack(t *testing.T) {
}
}
-// spawnClaimedGen never falls back to an ordinary start: an invalid
-// argument refuses before the host write is entered, and a refused work or
-// a parent that cannot pin a child never builds the command.
+// spawnClaimedGen is closed: an invalid argument answers
+// hostwrite.ErrUnavailable, and every other call answers
+// ErrClaimIsolationUnavailable, whatever the work would decide. It never
+// enters the host write, never calls before and never builds the command.
func TestClaimSpawnGenNeverFallsBack(t *testing.T) {
calls, builds := 0, 0
beforeCalled := false
diff --git a/internal/instanceexec/exec_test.go b/internal/instanceexec/exec_test.go
index 3dbd9d89b..fc1165991 100644
--- a/internal/instanceexec/exec_test.go
+++ b/internal/instanceexec/exec_test.go
@@ -179,29 +179,67 @@ func TestExecLaunchSplitsAGenerationInstance(t *testing.T) {
}
}
+// Each malformed instance string has an otherwise valid, host-matching
+// launch file planted wherever a lenient parser could look, so only the
+// parser can refuse it: the refusal must be launchTarget's own.
func TestExecLaunchRefusesMalformedOrMisplacedGenerations(t *testing.T) {
data := t.TempDir()
host := "bill.example.com"
dir := filepath.Join(data, "instances", host)
- if err := os.MkdirAll(dir, 0o700); err != nil {
- t.Fatal(err)
+ plant := func(dir string, n uint64, l instancectl.Launch) {
+ t.Helper()
+ if err := os.MkdirAll(dir, 0o700); err != nil {
+ t.Fatal(err)
+ }
+ raw, err := json.Marshal(l)
+ if err != nil {
+ t.Fatal(err)
+ }
+ if err := os.WriteFile(filepath.Join(dir, instancectl.GenLaunchFileName(n)), raw, 0o600); err != nil {
+ t.Fatal(err)
+ }
}
- other, _ := json.Marshal(instancectl.Launch{Schema: 1, Host: "other.example.com", Argv: []string{"/opt/carlos/live/other.example.com"}})
- if err := os.WriteFile(filepath.Join(dir, instancectl.GenLaunchFileName(7)), other, 0o600); err != nil {
- t.Fatal(err)
+ valid := instancectl.Launch{Schema: 1, Host: host, Argv: []string{"/opt/carlos/live/" + host}}
+ for _, n := range []uint64{0, 1, 7} {
+ plant(dir, n, valid)
}
+ // Where a parser that let "", ".." or "a:b" through would look.
+ plant(filepath.Join(data, "instances"), 7, valid)
+ plant(filepath.Join(data, "x"), 1, valid)
+ plant(filepath.Join(data, "instances", "a:b"), 1, valid)
+ plant(filepath.Join(data, "instances", "a"), 1, valid)
+
+ // The planted file is otherwise valid: the well-formed instance runs it.
+ var ok captured
+ if err := execLaunch(data, instancectl.RoleRun, host+":7", ok.exec); err != nil || ok.calls != 1 {
+ t.Fatalf("the planted file was refused for a valid instance: %v, %d execs", err, ok.calls)
+ }
+
for _, tc := range []struct {
role instancectl.Role
inst string
}{
{instancectl.RoleRun, host + ":0"}, {instancectl.RoleRun, host + ":07"},
{instancectl.RoleRun, ":7"}, {instancectl.RoleRun, host + ":"},
+ {instancectl.RoleRun, "../x:1"}, {instancectl.RoleRun, "a:b:1"},
{instancectl.RoleSidecar, host + ":7"}, {instancectl.RoleSeed, host + ":7"},
- {instancectl.RoleRun, host + ":7"}, // the file names another host
} {
+ _, _, perr := launchTarget(tc.role, tc.inst)
+ if perr == nil {
+ t.Errorf("launchTarget(%s, %q) accepted it", tc.role, tc.inst)
+ continue
+ }
var c captured
- if err := execLaunch(data, tc.role, tc.inst, c.exec); err == nil || c.calls != 0 {
- t.Errorf("execLaunch(%s, %q) = %v after %d execs; want a refusal", tc.role, tc.inst, err, c.calls)
+ err := execLaunch(data, tc.role, tc.inst, c.exec)
+ if err == nil || c.calls != 0 || err.Error() != perr.Error() {
+ t.Errorf("execLaunch(%s, %q) = %v after %d execs; want the parser's refusal %q and no exec", tc.role, tc.inst, err, c.calls, perr)
}
}
+
+ // Misplaced: a well-formed generation whose file names another host.
+ plant(dir, 9, instancectl.Launch{Schema: 1, Host: "other.example.com", Argv: []string{"/opt/carlos/live/other.example.com"}})
+ var c captured
+ if err := execLaunch(data, instancectl.RoleRun, host+":9", c.exec); err == nil || c.calls != 0 {
+ t.Errorf("a generation file naming another host: %v after %d execs; want a refusal", err, c.calls)
+ }
}