Pass a container's groups to the runtime as supplementary groups and report container-groups (issues 1637, ADR 0480 clause 7)

mesh-sdk is pinned to feat/r4d-groups f0f66d5b8d42 (mesh-sdk #58) until it merges.

go.mod

@@ -3,7 +3,7 @@ module github.com/novox/mesh-host
 go 1.26.0
 
 require (
-	git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261020085642-ab0c16ddad26
+	git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261021120119-f0f66d5b8d42
 	github.com/nats-io/nats.go v1.54.0
 	golang.org/x/crypto v0.57.0
 	golang.org/x/sys v0.48.0

go.sum

@@ -1,5 +1,7 @@
 git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261020085642-ab0c16ddad26 h1:oT1l3ZdjyF82MK14QMLKzoH2+iOFH/+UEYdlK3T2f7k=
 git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261020085642-ab0c16ddad26/go.mod h1:ouERNpZizzwZ0HLEtAypsY9FR20FyUgUYRni6J7kVCY=
+git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261021120119-f0f66d5b8d42 h1:Q6llMoEzdMF+1moEBSLA2MC+ztqN7h6GWtBZ2WcMju4=
+git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261021120119-f0f66d5b8d42/go.mod h1:ouERNpZizzwZ0HLEtAypsY9FR20FyUgUYRni6J7kVCY=
 github.com/klauspost/compress v1.20.0 h1:a3C1ke2ohxFymNlb2HWAHjDeKCI90scRskErZkR0ezA=
 github.com/klauspost/compress v1.20.0/go.mod h1:LUdAzn7YLVvxLpc7y3V1m40wESHTgc1422pwwBSKYuI=
 github.com/nats-io/nats.go v1.54.0 h1:vsXoOxjHp/GmPUN+EcI7uOf/uB+iAP+kEsAFNQN0yzA=

internal/apply/apply.go

@@ -2599,6 +2599,11 @@ func containerSpecReading(r *declaration.Container, declares, reads map[string]s
 	if args := identityArgs(r); len(args) > 0 {
 		b.WriteString("identity " + strings.Join(args, " ") + "\n")
 	}
+	// And the supplementary groups it joins (novox/hq ADR 0480 clause 7): fixed when the container is created, so
+	// a changed set is a container recreated. Only when there are some, so no other container has its digest moved.
+	if args := groupArgs(r); len(args) > 0 {
+		b.WriteString("groups " + strings.Join(args, " ") + "\n")
+	}
 	// **What this container reads is part of what it is.**
 	//
 	// A container takes its environment and its mounted files once, at start, and never looks
@@ -2827,54 +2832,7 @@ func applyContainer(ctx context.Context, r *declaration.Container, run Runner,
 		}
 	}
 
-	args := []string{"run", "--detach", "--name", r.Name, "--restart", "unless-stopped"}
-	for _, file := range r.EnvFile {
-		args = append(args, "--env-file", file)
-	}
-	if r.Network != "" {
-		args = append(args, "--network", r.Network)
-	}
-	for _, c := range r.Capabilities {
-		args = append(args, "--cap-add", c)
-	}
-	if r.Logging != "" {
-		// The journal keeps the container's name on every line (CONTAINER_NAME), which is what a
-		// jail matches on (novox/hq ADR 0179); `docker logs` keeps working against the journal.
-		args = append(args, "--log-driver", r.Logging)
-	}
-	for _, d := range r.Dns {
-		args = append(args, "--dns", d)
-	}
-	if r.IP != "" {
-		args = append(args, "--ip", r.IP)
-	}
-	args = append(args,
-		"--label", specLabel+"="+want, "--label", idLabel+"="+r.ID)
-	for _, k := range sortedKeys(r.Env) {
-		args = append(args, "--env", k+"="+r.Env[k])
-	}
-	for _, p := range r.Ports {
-		args = append(args, "--publish", p)
-	}
-	for _, v := range r.Volumes {
-		args = append(args, "--volume", v)
-	}
-	args = append(args, identityArgs(r)...)
-	// **The one check the container carries is the one the module declared** (ADR 0240 rule 3): an
-	// exec command, or the image's own adopted by name, with the declared timing. Nothing else on the
-	// machine sets a container's check.
-	if r.Health.RunByRuntime() {
-		args = append(args, runtimeCheckArgs(r.Health)...)
-	}
-	for _, h := range r.Hosts {
-		// Written into the container's own hosts file by the runtime. Per container rather than
-		// by editing the machine's resolver configuration: that file belongs to something else on
-		// most machines, and a host that edited it would be fighting whatever owns it on every
-		// boot — the fault this host exists to avoid, in the place it would be hardest to see.
-		args = append(args, "--add-host", h)
-	}
-	args = append(args, r.Image)
-	args = append(args, r.Args...)
+	args := detachedRunArgs(r, want)
 
 	if _, err := run(ctx, cri, args...); err != nil {
 		return out, fmt.Errorf("starting container %s: %w", r.Name, err)
@@ -2929,6 +2887,60 @@ func applyContainer(ctx context.Context, r *declaration.Container, run Runner,
 	return out, nil
 }
 
+// detachedRunArgs is the `docker run` that starts a service container and leaves it running.
+func detachedRunArgs(r *declaration.Container, want string) []string {
+	args := []string{"run", "--detach", "--name", r.Name, "--restart", "unless-stopped"}
+	for _, file := range r.EnvFile {
+		args = append(args, "--env-file", file)
+	}
+	if r.Network != "" {
+		args = append(args, "--network", r.Network)
+	}
+	for _, c := range r.Capabilities {
+		args = append(args, "--cap-add", c)
+	}
+	if r.Logging != "" {
+		// The journal keeps the container's name on every line (CONTAINER_NAME), which is what a
+		// jail matches on (novox/hq ADR 0179); `docker logs` keeps working against the journal.
+		args = append(args, "--log-driver", r.Logging)
+	}
+	for _, d := range r.Dns {
+		args = append(args, "--dns", d)
+	}
+	if r.IP != "" {
+		args = append(args, "--ip", r.IP)
+	}
+	args = append(args,
+		"--label", specLabel+"="+want, "--label", idLabel+"="+r.ID)
+	for _, k := range sortedKeys(r.Env) {
+		args = append(args, "--env", k+"="+r.Env[k])
+	}
+	for _, p := range r.Ports {
+		args = append(args, "--publish", p)
+	}
+	for _, v := range r.Volumes {
+		args = append(args, "--volume", v)
+	}
+	args = append(args, identityArgs(r)...)
+	args = append(args, groupArgs(r)...)
+	// **The one check the container carries is the one the module declared** (ADR 0240 rule 3): an
+	// exec command, or the image's own adopted by name, with the declared timing. Nothing else on the
+	// machine sets a container's check.
+	if r.Health.RunByRuntime() {
+		args = append(args, runtimeCheckArgs(r.Health)...)
+	}
+	for _, h := range r.Hosts {
+		// Written into the container's own hosts file by the runtime. Per container rather than
+		// by editing the machine's resolver configuration: that file belongs to something else on
+		// most machines, and a host that edited it would be fighting whatever owns it on every
+		// boot — the fault this host exists to avoid, in the place it would be hardest to see.
+		args = append(args, "--add-host", h)
+	}
+	args = append(args, r.Image)
+	args = append(args, r.Args...)
+	return args
+}
+
 // runtimeCheckArgs is a declared check as the runtime takes it (novox/hq ADR 0240 rule 3, to-be 48 §3):
 // its timing, and for an exec check its command, run by the container's shell. The runtime's own retries
 // are the declared failing looks, and its start period the grace — so its retries and start period do the
@@ -3019,6 +3031,7 @@ func foregroundRunArgs(r *declaration.Container, want string) []string {
 		args = append(args, "--volume", v)
 	}
 	args = append(args, identityArgs(r)...)
+	args = append(args, groupArgs(r)...)
 	for _, h := range r.Hosts {
 		args = append(args, "--add-host", h)
 	}
@@ -3027,6 +3040,16 @@ func foregroundRunArgs(r *declaration.Container, want string) []string {
 	return args
 }
 
+// groupArgs is the runtime's arguments that add a container's supplementary groups, one per number
+// (novox/hq ADR 0480 clause 7, issue 1637). None for a container that declares none.
+func groupArgs(r *declaration.Container) []string {
+	var args []string
+	for _, g := range r.Groups {
+		args = append(args, "--group-add", strconv.FormatInt(g, 10))
+	}
+	return args
+}
+
 // specWithoutIdentity is the spec r had before it was told who to run as.
 func specWithoutIdentity(r *declaration.Container, declares, reads map[string]string) string {
 	plain := *r

internal/apply/container_groups_test.go

@@ -0,0 +1,54 @@
+package apply
+
+import (
+	"strings"
+	"testing"
+
+	"git.novox.be/novox/mesh-sdk/go/declaration"
+)
+
+// Issue 1637 (novox/hq ADR 0480 clause 7): a container's supplementary groups reach the runtime as --group-add.
+
+func groupsContainer(groups ...int64) *declaration.Container {
+	return &declaration.Container{ID: "n8n.app", Name: "n8n", Image: pinned, Groups: groups}
+}
+
+// 1637/AC1: both run commands carry one --group-add per group, and none without groups.
+func TestAContainerIsAddedItsSupplementaryGroups(t *testing.T) {
+	with := groupsContainer(2000, 2001)
+	for name, args := range map[string][]string{
+		"detached":   detachedRunArgs(with, "x"),
+		"foreground": foregroundRunArgs(with, "x"),
+	} {
+		got := " " + strings.Join(args, " ") + " "
+		if !strings.Contains(got, " --group-add 2000 ") || !strings.Contains(got, " --group-add 2001 ") {
+			t.Errorf("1637/AC1: the %s run does not add both groups: %s", name, got)
+		}
+	}
+	without := groupsContainer()
+	for name, args := range map[string][]string{
+		"detached":   detachedRunArgs(without, "x"),
+		"foreground": foregroundRunArgs(without, "x"),
+	} {
+		if strings.Contains(strings.Join(args, " "), "--group-add") {
+			t.Errorf("1637/AC1: the %s run adds a group none was declared", name)
+		}
+	}
+}
+
+// 1637/AC2: a changed set of groups recreates the container; no groups leaves the spec as it was.
+func TestAChangeOfGroupsRecreatesTheContainer(t *testing.T) {
+	none, a, b := groupsContainer(), groupsContainer(2000), groupsContainer(2000, 2001)
+	if containerSpec(none, inputs{}) == containerSpec(a, inputs{}) || containerSpec(a, inputs{}) == containerSpec(b, inputs{}) {
+		t.Error("1637/AC2: a change of groups does not change the container's spec")
+	}
+	same := groupsContainer(2000)
+	if containerSpec(a, inputs{}) != containerSpec(same, inputs{}) {
+		t.Error("1637/AC2: the same groups changed the spec")
+	}
+	plain := *none
+	plain.Groups = nil
+	if containerSpec(none, inputs{}) != containerSpec(&plain, inputs{}) {
+		t.Error("1637/AC2: a container with no groups has its spec moved")
+	}
+}

internal/profile/container_groups_test.go

@@ -0,0 +1,21 @@
+package profile
+
+import (
+	"context"
+	"testing"
+	"time"
+)
+
+// 1637/AC3: the node's report carries the capability that says the node-engine reads a container's groups.
+func TestTheProfileSaysTheNodeEngineReadsContainerGroups(t *testing.T) {
+	got := Detect(context.Background(), Default(func(context.Context, string, ...string) (string, error) {
+		return "", nil
+	}), time.Second)
+	v := verdictOf(t, got, CapContainerGroups)
+	if !v.Present || v.Detail == "" || v.How == "" {
+		t.Errorf("1637/AC3: container-groups = %+v, want present with a detail and a method", v)
+	}
+	if CapContainerGroups != "container-groups" {
+		t.Errorf("1637/AC3: the capability is named %q", CapContainerGroups)
+	}
+}

internal/profile/detectors.go

@@ -29,6 +29,11 @@ const (
 	// an unprivileged user, never by a version string or a sysctl alone.
 	CapUserNamespaces = "user-namespaces"
 
+	// CapContainerGroups says this node-engine reads a container's `groups` and passes each to the runtime as a
+	// supplementary group (novox/hq ADR 0480 clause 7, issue 1637). A fact about the program, not the machine: the
+	// controller sends the field only to a node that reports it, since an older node-engine refuses the key.
+	CapContainerGroups = "container-groups"
+
 	// The network manager this machine runs, one capability per dialect (novox/hq ADR 0161): the
 	// uplink seat's holder declares its own, so the holder for a manager the machine does not run
 	// is refused the way any missing capability is, naming it. Active, not installed — a machine
@@ -152,6 +157,20 @@ func (privileged) Detect(context.Context) Verdict {
 	}
 }
 
+// containerGroups reports that this node-engine reads a container's groups. It is true of the program that is
+// running, so it is always present: its absence on a node is what tells the controller the node-engine is older.
+type containerGroups struct{}
+
+func (containerGroups) Name() string { return CapContainerGroups }
+
+func (containerGroups) Detect(context.Context) Verdict {
+	return Verdict{
+		Name: CapContainerGroups, Present: true,
+		Detail: "passes a container's groups to the runtime as supplementary groups",
+		How:    "a fact about this program, not probed: the node-engine that answers reads the field",
+	}
+}
+
 // graphicalSession reports whether anything could display a window here.
 //
 // Environment rather than a probe, because a display server is reachable through a socket a
@@ -190,6 +209,7 @@ func Default(runner Runner) []Detector {
 	}
 	return []Detector{
 		privileged{},
+		containerGroups{},
 		graphicalSession{},
 		seat{},
 		battery{},