diff --git a/go/catalogue/groups_test.go b/go/catalogue/groups_test.go new file mode 100644 index 0000000..fe43f99 --- /dev/null +++ b/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) + } +} diff --git a/go/catalogue/manifest.go b/go/catalogue/manifest.go index 0464654..870ea4d 100644 --- a/go/catalogue/manifest.go +++ b/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 { diff --git a/go/catalogue/setting_into.go b/go/catalogue/setting_into.go index fbb966f..73bd2b0 100644 --- a/go/catalogue/setting_into.go +++ b/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 ` 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 +} diff --git a/go/catalogue/unconsumed_placeholder.go b/go/catalogue/unconsumed_placeholder.go index 39df9ba..b017d5a 100644 --- a/go/catalogue/unconsumed_placeholder.go +++ b/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"}}, diff --git a/go/declaration/declaration.go b/go/declaration/declaration.go index ea16b91..b2fcccf 100644 --- a/go/declaration/declaration.go +++ b/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)...) } diff --git a/go/declaration/groups_test.go b/go/declaration/groups_test.go new file mode 100644 index 0000000..193d8f5 --- /dev/null +++ b/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") + } +}