Merge pull request 'Let a container name supplementary groups, from a number or a trusted config key, so n8n can join the media group without an image of its own (issue 1636, R4d)' (#58) from feat/r4d-groups into main

go/catalogue/groups_test.go

@@ -0,0 +1,118 @@
+package catalogue
+
+import (
+	"reflect"
+	"strings"
+	"testing"
+)
+
+// A container's supplementary groups may be an operator's value (novox/hq ADR 0480 clause 7, ADR 0348 §11,
+// issue 1636): `groups: ["${config:media-gid}"]`, each element a whole placeholder or a whole number, filled at
+// composition with the number the config value holds.
+
+func aContainerWithGroups(groups ...any) map[string]any {
+	return map[string]any{"id": "server", "type": "container", "name": "n8n", "image": "x@sha256:" + strings.Repeat("a", 64),
+		"groups": groups}
+}
+
+// 1636/AC3: a manifest container with groups: ["${setting:media-gid}"] resolves to the number the setting says.
+func TestAContainersGroupIsFilledFromTheSetting(t *testing.T) {
+	r := aContainerWithGroups("${setting:media-gid}", float64(2001))
+	layers := []Layer{{From: "ace", Values: map[string]any{"media-gid": float64(2000)}}}
+	if err := settingInto(r, layers, "n8n"); err != nil {
+		t.Fatal(err)
+	}
+	if want := []any{2000, 2001}; !reflect.DeepEqual(r["groups"], want) {
+		t.Fatalf("composed as %#v, want %#v", r["groups"], want)
+	}
+	// A value the operator wrote as text is read as the number it spells.
+	r = aContainerWithGroups("${setting:media-gid}")
+	if err := settingInto(r, []Layer{{From: "ace", Values: map[string]any{"media-gid": "2000"}}}, "n8n"); err != nil {
+		t.Fatal(err)
+	}
+	if want := []any{2000}; !reflect.DeepEqual(r["groups"], want) {
+		t.Fatalf("composed as %#v, want %#v", r["groups"], want)
+	}
+}
+
+// 1636/AC3: an unset value refuses the module by name, and the key and the remedy are named; nothing is dropped.
+func TestAnUnsetGroupRefusesTheModuleByName(t *testing.T) {
+	r := aContainerWithGroups("${setting:media-gid}")
+	err := settingInto(r, nil, "n8n")
+	if err == nil || !strings.Contains(err.Error(), "n8n") || !strings.Contains(err.Error(), "media-gid") {
+		t.Fatalf("want a refusal naming n8n and media-gid, got %v", err)
+	}
+	if got, _ := r["groups"].([]any); len(got) != 1 || got[0] != "${setting:media-gid}" {
+		t.Fatalf("the resource was changed: %#v", r["groups"])
+	}
+}
+
+// 1636/AC2: a value that is not a whole number above zero cannot be a group.
+func TestAGroupThatIsNotAWholeNumberAboveZeroIsRefused(t *testing.T) {
+	for _, bad := range []any{"media", "-1", "0", float64(0), float64(2000.5), "2000:2000", "", true} {
+		r := aContainerWithGroups("${setting:media-gid}")
+		err := settingInto(r, []Layer{{From: "ace", Values: map[string]any{"media-gid": bad}}}, "n8n")
+		if err == nil || !strings.Contains(err.Error(), "media-gid") {
+			t.Errorf("value %#v: want a refusal naming the key, got %v", bad, err)
+		}
+	}
+}
+
+// A manifest in the config form registers with the group a config value fills, and the value is placed (a container's
+// groups count as a place for it); a placeholder in any other shape is refused where the manifest is read.
+func TestAManifestWhoseContainerTakesItsGroupFromConfigRegisters(t *testing.T) {
+	m := map[string]any{
+		"module": "n8n", "version": "1",
+		"config": map[string]any{"media-gid": map[string]any{"type": "number", "minimum": 1, "required": true,
+			"description": "the group that owns the media library on this machine"}},
+		"resources": []any{aContainerWithGroups("${config:media-gid}")},
+	}
+	if _, err := parseRaw(t, m); err != nil {
+		t.Fatalf("refused: %v", err)
+	}
+	for _, shape := range []string{"x${config:media-gid}", "${config:media-gid}:1", "${config:other}"} {
+		m["resources"] = []any{aContainerWithGroups(shape)}
+		if _, err := parseRaw(t, m); err == nil {
+			t.Errorf("groups %q registered", shape)
+		}
+	}
+}
+
+// 1636/AC3 composed for a machine, the way a push writes it: n8n's group is the number the machine's config value
+// says, written as a number, and the module is left out by name when the machine says none.
+func TestAComposedMachineGetsTheGroupTheConfigValueNames(t *testing.T) {
+	manifest, err := parseRaw(t, map[string]any{
+		"module": "n8n", "version": "1",
+		"config": map[string]any{"media-gid": map[string]any{"type": "number", "minimum": 1, "required": true,
+			"description": "the group that owns the media library on this machine"}},
+		"resources": []any{aContainerWithGroups("${config:media-gid}")},
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+	r := anAdoptedAnchor()
+	r.Modules = append(r.Modules, manifest)
+	with := anchorRendering(false)
+	with.Settings["n8n"] = []Layer{{From: "anchor", Values: map[string]any{"media-gid": float64(2000)}}}
+	composed, err := r.Compose(with)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if why, left := composed.LeftOut["n8n"]; left {
+		t.Fatalf("left out: %s", why)
+	}
+	if got := byID(composed.Resources)["n8n.server"]["groups"]; !reflect.DeepEqual(got, []any{2000}) {
+		t.Fatalf("composed groups %#v", got)
+	}
+
+	without := anchorRendering(false)
+	composed, err = r.Compose(without)
+	if err == nil {
+		why, left := composed.LeftOut["n8n"]
+		if !left || !strings.Contains(why, "media-gid") {
+			t.Fatalf("a machine that says no group kept n8n or left it out without the key: %q (%v)", why, left)
+		}
+	} else if !strings.Contains(err.Error(), "media-gid") {
+		t.Fatalf("refused without the key: %v", err)
+	}
+}

go/catalogue/manifest.go

@@ -2214,6 +2214,7 @@ func parseManifest(raw []byte, predefined PredefinedList) (Manifest, error) {
 	problems = append(problems, replacesProblems(m)...)
 	problems = append(problems, UnitSettingProblems(m)...)
 	problems = append(problems, OwnerSettingProblems(m)...)
+	problems = append(problems, GroupSettingProblems(m)...)
 	problems = append(problems, endpointNameProblems(m)...)
 	problems = append(problems, RouteProblems(m)...)
 	for _, port := range m.Guards {

go/catalogue/setting_into.go

@@ -85,6 +85,9 @@ func settingInto(resource map[string]any, layers []Layer, module string) error {
 	if fmt.Sprint(resource["type"]) == "directory" {
 		return settingIntoOwner(resource, layers, module)
 	}
+	if fmt.Sprint(resource["type"]) == "container" {
+		return settingIntoGroups(resource, layers, module)
+	}
 	if fmt.Sprint(resource["type"]) != "file" {
 		return nil
 	}
@@ -343,6 +346,12 @@ func settingKeysAskedOutsideOwners(m Manifest) map[string]bool {
 			if unit, ok := r["unit"].(string); ok {
 				note(unit)
 			}
+		case "container":
+			for _, g := range groupsOf(r) {
+				if s, ok := g.(string); ok {
+					note(s)
+				}
+			}
 		}
 	}
 	inValues := func(values map[string]any) {
@@ -365,3 +374,100 @@ func settingKeysAskedOutsideOwners(m Manifest) map[string]bool {
 	}
 	return used
 }
+
+// A container's supplementary groups may be an operator's value (novox/hq ADR 0480 clause 7, ADR 0348 §11, issue 1636):
+// n8n joins the media library's group, whose number is the machine's, never the definition's (ADR 0112 §1). Each
+// element of `groups` is a whole placeholder or a whole number; a placeholder is filled with the number the
+// config value holds, and the list reaches the node-engine as numbers.
+
+// groupSide is one element of a container's groups: a whole placeholder or a whole number.
+var groupSide = regexp.MustCompile(`^(?:\$\{setting:[a-z0-9][a-z0-9_.-]*\}|[0-9]+)$`)
+
+// groupsOf is a container resource's groups as the manifest wrote them (text or numbers), none when it names none.
+func groupsOf(r map[string]any) []any {
+	groups, _ := r["groups"].([]any)
+	return groups
+}
+
+// GroupSettingProblems refuses, where a manifest is read, a container whose groups hold anything but whole
+// placeholders and whole numbers, or a placeholder whose key the module neither declares nor asks for elsewhere.
+func GroupSettingProblems(m Manifest) []string {
+	var problems []string
+	for _, r := range m.Resources {
+		if fmt.Sprint(r["type"]) != "container" {
+			continue
+		}
+		for _, g := range groupsOf(r) {
+			s, isText := g.(string)
+			if !isText {
+				continue // a number: the declaration judges its value
+			}
+			if !groupSide.MatchString(s) {
+				problems = append(problems, fmt.Sprintf("%s: the container %v's group %q is not a whole "+
+					"${config:key} or a whole number: a supplementary group is a number, and nothing else is "+
+					"written into it (novox/hq ADR 0480 clause 7)", m.Module, r["id"], s))
+				continue
+			}
+			if m.Templated() {
+				continue
+			}
+			for _, key := range settingsUsed(s) {
+				if _, declared := m.Settings[key]; !declared && !m.DeclaresConfig() {
+					continue // the settings form judges its own keys
+				}
+				if _, declared := m.Settings[key]; !declared {
+					problems = append(problems, fmt.Sprintf("%s: the container %v's group says ${config:%s}, and the module "+
+						"declares no such value (novox/hq ADR 0348 §4)", m.Module, r["id"], key))
+				}
+			}
+		}
+	}
+	return problems
+}
+
+// settingIntoGroups fills ${setting:…} in a container's groups, refusing — with the key and why, the resource left as
+// it was — a key nothing sets and a value that is not a whole number above zero. A number is kept; the list that
+// results is numbers alone.
+func settingIntoGroups(resource map[string]any, layers []Layer, module string) error {
+	groups := groupsOf(resource)
+	if len(groups) == 0 {
+		return nil
+	}
+	filled := make([]any, 0, len(groups))
+	for _, g := range groups {
+		s, isText := g.(string)
+		if !isText {
+			if f, isNumber := g.(float64); isNumber && f == float64(int(f)) {
+				g = int(f) // a number the manifest wrote, as the numbers a placeholder gives
+			}
+			filled = append(filled, g)
+			continue
+		}
+		keys := settingsUsed(s)
+		if len(keys) != 1 || settingRef.ReplaceAllString(s, "") != "" {
+			filled = append(filled, g) // not a whole placeholder: the manifest check refuses it
+			continue
+		}
+		key := keys[0]
+		value, set := settingValue(layers, key)
+		if !set {
+			return &UnsetSettingError{Module: module, Setting: key,
+				where: fmt.Sprintf("a container %v whose groups", resource["id"]), setToday: orNoSettings(layers),
+				said: fmt.Sprintf(
+					"%s has a container %v whose groups say ${setting:%s}, and nothing sets %q for it — an operator's "+
+						"value is the assignment's, never the definition's (novox/hq ADR 0112): "+
+						"`settings set %s <file>` with {%q: …}%s",
+					module, resource["id"], key, key, module, key, orNoSettings(layers))}
+		}
+		v := plainly(value)
+		id, whole := wholeID(v)
+		if !whole || id == 0 {
+			return fmt.Errorf("%s: the setting %q is %q, which cannot be a group of the container %v: a supplementary "+
+				"group is a whole number above zero, never a name or other text (novox/hq ADR 0480 clause 7, ADR 0328 §15)",
+				module, key, v, resource["id"])
+		}
+		filled = append(filled, int(id))
+	}
+	resource["groups"] = filled
+	return nil
+}

go/catalogue/unconsumed_placeholder.go

@@ -56,7 +56,7 @@ type filler struct {
 // seatInto, machineInto and contributionsInto. A pass that comes to read another field adds it here,
 // or the sweep refuses the placeholder it would have filled.
 var fillers = []filler{
-	{namespace: "setting", patterns: []*regexp.Regexp{settingRef}, fields: map[string][]string{"file": {"content"}, "service": {"unit"}, "directory": {"owner"}}},
+	{namespace: "setting", patterns: []*regexp.Regexp{settingRef}, fields: map[string][]string{"file": {"content"}, "service": {"unit"}, "directory": {"owner"}, "container": {"groups"}}},
 	{namespace: "dir", patterns: []*regexp.Regexp{dirRef}, fields: map[string][]string{"*": {"path", "content", "volumes", "env", "env-file"}}},
 	{namespace: "access", patterns: []*regexp.Regexp{accessRef}, fields: map[string][]string{"*": {"path", "content", "volumes", "env", "env-file"}}},
 	{namespace: "secret", patterns: []*regexp.Regexp{placeholder}, fields: map[string][]string{"file": {"content"}},

go/declaration/declaration.go

@@ -1454,6 +1454,14 @@ type Container struct {
 	// container runs as its image says. Its key is `identity`, the manifest's word; the field is not, because
 	// a resource's Identity is its id.
 	RunAs *ContainerIdentity `json:"identity,omitempty"`
+
+	// Groups are the supplementary groups the container's process joins, as numbers, each passed to the runtime
+	// as a group to add (novox/hq ADR 0480 clause 7, issue 1636): n8n writes the media library through the
+	// library's group, which a stock image knows nothing of. Each is above zero and below the kernel's "no group"
+	// (ADR 0328 §15: never root). Part of the spec, so a change recreates the container. Absent: none are added.
+	// An older node-engine refuses the key (strict decoding), so the controller sends it only to a node that
+	// reports it reads it.
+	Groups []int64 `json:"groups,omitempty"`
 }
 
 // The ways an image is told who to run as (novox/hq ADR 0325 §2).
@@ -1521,6 +1529,30 @@ func (i *ContainerIdentity) problems(where string, env map[string]string) []stri
 	return problems
 }
 
+// groupProblems refuses a supplementary group that is root's or below it, the kernel's "no group" number, or a
+// number named twice (novox/hq ADR 0480 clause 7, ADR 0328 §15).
+func (c *Container) groupProblems(where string) []string {
+	var problems []string
+	seen := map[int64]bool{}
+	for _, g := range c.Groups {
+		switch {
+		case g == 0:
+			problems = append(problems, fmt.Sprintf("%s: container %s adds group 0, and the mesh never makes or keeps anything "+
+				"as root: a supplementary group is never root's (novox/hq ADR 0328 §15)", where, c.Name))
+		case g < 0:
+			problems = append(problems, fmt.Sprintf("%s: container %s adds group %d, below zero: a group is a whole number "+
+				"above zero", where, c.Name, g))
+		case g >= 1<<32-1:
+			problems = append(problems, fmt.Sprintf("%s: container %s adds group %d, which is the kernel's \"no group\" "+
+				"number or beyond it", where, c.Name, g))
+		case seen[g]:
+			problems = append(problems, fmt.Sprintf("%s: container %s adds group %d twice", where, c.Name, g))
+		}
+		seen[g] = true
+	}
+	return problems
+}
+
 func (c *Container) Identity() string { return c.ID }
 func (c *Container) Kind() Type       { return TypeContainer }
 func (c *Container) Target() string   { return c.Name }
@@ -1603,6 +1635,7 @@ func (c *Container) validate(where string, _ bool) []string {
 			"runs and exits, and joins nothing afterwards")
 	}
 	problems = append(problems, c.RunAs.problems(where+": container "+c.Name, c.Env)...)
+	problems = append(problems, c.groupProblems(where)...)
 	return append(problems, checkImage(where, c.Image)...)
 }
 

go/declaration/groups_test.go

@@ -0,0 +1,55 @@
+package declaration
+
+import (
+	"encoding/json"
+	"strings"
+	"testing"
+)
+
+// A container may be given supplementary groups (novox/hq ADR 0480 clause 7, issue 1636): numbers, each above zero.
+
+// 1636/AC1: a declaration with groups decodes, is written back with them, and a declaration without them has none.
+func TestAContainerWithGroupsDecodesAndIsWrittenBack(t *testing.T) {
+	d, err := Parse([]byte(`{"declaration":1,"resources":[
+	  {"id":"n8n.server","type":"container","name":"n8n","image":"` + anImage + `","groups":[2000,2001]},
+	  {"id":"n8n.plain","type":"container","name":"plain","image":"` + anImage + `"}]}`))
+	if err != nil {
+		t.Fatal(err)
+	}
+	c := d.Resources[0].(*Container)
+	if len(c.Groups) != 2 || c.Groups[0] != 2000 || c.Groups[1] != 2001 {
+		t.Fatalf("groups read as %v", c.Groups)
+	}
+	raw, err := json.Marshal(c)
+	if err != nil || !strings.Contains(string(raw), `"groups":[2000,2001]`) {
+		t.Fatalf("written as %s (%v)", raw, err)
+	}
+	plain, _ := json.Marshal(d.Resources[1].(*Container))
+	if strings.Contains(string(plain), "groups") {
+		t.Fatalf("a container without groups is written with the key: %s", plain)
+	}
+	if g := d.Resources[1].(*Container).Groups; g != nil {
+		t.Fatalf("a container without groups reads %v", g)
+	}
+}
+
+// 1636/AC2: zero, a number below it, one the kernel keeps for "no group" and a number twice are refused, naming why.
+func TestAContainersGroupsAreWholeNumbersAboveZeroAndNotRepeated(t *testing.T) {
+	for want, groups := range map[string]string{
+		"never root": `[0]`,
+		"below zero": `[-5]`,
+		"no group":   `[4294967295]`,
+		"twice":      `[2000,2000]`,
+	} {
+		_, err := Parse([]byte(`{"declaration":1,"resources":[{"id":"m.c","type":"container","name":"c","image":"` +
+			anImage + `","groups":` + groups + `}]}`))
+		if err == nil || !strings.Contains(err.Error(), want) {
+			t.Errorf("groups %s: want a refusal saying %q, got %v", groups, want, err)
+		}
+	}
+	// Text where a number stands is refused by the decoder itself.
+	if _, err := Parse([]byte(`{"declaration":1,"resources":[{"id":"m.c","type":"container","name":"c","image":"` +
+		anImage + `","groups":["media"]}]}`)); err == nil {
+		t.Error("a group named by text was accepted")
+	}
+}