From c43277f9c6e3846c7b9f6d547001393fcad7f8d0 Mon Sep 17 00:00:00 2001 From: jochen Date: Thu, 8 Oct 2026 12:02:43 +0200 Subject: [PATCH] Compose an account given only groups where its module wrote it (hq ADR 0252, issue 247) Composed first, an account a module puts in its daemon's group was put in a group the package had not made yet, and only the next apply healed it. An account that sets a shell or home still goes first (issue 213). --- internal/catalogue/declaration.go | 19 +++++++++++++- internal/catalogue/resolve.go | 6 ++--- internal/catalogue/shared_account_test.go | 31 +++++++++++++++++++++++ 3 files changed, 52 insertions(+), 4 deletions(-) diff --git a/internal/catalogue/declaration.go b/internal/catalogue/declaration.go index 5e32350e..7c25a7bc 100644 --- a/internal/catalogue/declaration.go +++ b/internal/catalogue/declaration.go @@ -2396,9 +2396,15 @@ func withRestartOn(have any, add []string) []any { // accountsFirst splits a module's resources into its accounts and everything else, each in the order // written. +// +// **An account declared only to be put in groups keeps its written place** (novox/hq ADR 0252). It is not +// the module's own account — that one says how it logs in or where it lives — but one the module needs in +// a group: the operator's, in the group of a daemon the module installs. The group is made by the package +// that installs the daemon, so put first, the account was put in a group the machine did not have yet, and +// only the next apply healed it. Written after its packages, it is applied after them. func accountsFirst(resources []map[string]any) (accounts, rest []map[string]any) { for _, r := range resources { - if fmt.Sprint(r["type"]) == "user" { + if fmt.Sprint(r["type"]) == "user" && !onlyGroups(r) { accounts = append(accounts, r) continue } @@ -2407,6 +2413,17 @@ func accountsFirst(resources []map[string]any) (accounts, rest []map[string]any) return accounts, rest } +// onlyGroups is a user resource that names groups and nothing else of the account: no shell, home or +// lingering. +func onlyGroups(r map[string]any) bool { + for _, field := range []string{"shell", "home", "linger"} { + if v, ok := r[field]; ok && v != nil && v != "" { + return false + } + } + return len(stringsIn(r["groups"])) > 0 +} + // generatedHere is what each computed module's generator answers for this machine, by module — // absent for a module whose machine is not yet part of what it generates — held to the rule every // module is held to: no two modules on a machine declare one path, unit, name or package (novox/hq diff --git a/internal/catalogue/resolve.go b/internal/catalogue/resolve.go index 163a4351..8d140fef 100644 --- a/internal/catalogue/resolve.go +++ b/internal/catalogue/resolve.go @@ -949,9 +949,9 @@ func checkResources(modules []Manifest) []string { if fmt.Sprint(r["type"]) == "user" { // **An account is shared; what it is set to is not.** Several modules may need one // login: the shell's module sets its shell, the container runtime's puts it in the - // `docker` group. The host only ever adds groups — it never takes the account out of - // one, not even when the resource that named it is undeclared — so groups from - // several modules cannot contradict each other and are not owned. A shell or a home + // `docker` group. The host only ever adds groups, and takes back only a group it put the + // account in, when no declared resource still asks for it (novox/hq ADR 0252) — so groups + // from several modules cannot contradict each other and are not owned. A shell or a home // is one value, and two modules setting it would each be undone by the other's // apply: each stays one module's per node, and two are refused naming both. name, _ := r["name"].(string) diff --git a/internal/catalogue/shared_account_test.go b/internal/catalogue/shared_account_test.go index 16a7f74e..aab013d0 100644 --- a/internal/catalogue/shared_account_test.go +++ b/internal/catalogue/shared_account_test.go @@ -40,3 +40,34 @@ func TestTwoModulesSettingOneAccountsShellOrHomeAreRefused(t *testing.T) { } } } + +// An account a module declares only to put in a group is applied where it is written, after the package +// that makes the group (novox/hq ADR 0252); one that says how it logs in still goes first (issue 213). +func TestAnAccountOnlyGivenGroupsKeepsItsWrittenPlace(t *testing.T) { + compose := func(raw string) []map[string]any { + t.Helper() + m, err := ParseManifest([]byte(raw)) + if err != nil { + t.Fatal(err) + } + out, err := Resolution{Node: "workstation", Modules: []Manifest{m}}.Declaration(Rendering{}) + if err != nil { + t.Fatal(err) + } + return out + } + out := compose(`{"module": "lights", "version": "1", "resources": [ + {"id": "daemon", "type": "package", "package": "lights-daemon"}, + {"id": "account", "type": "user", "name": "op", "groups": ["lights"]} + ]}`) + if pkg, acct := indexOf(out, "lights.daemon"), indexOf(out, "lights.account"); pkg < 0 || acct < pkg { + t.Fatalf("the account (%d) is not after the package that makes its group (%d): %v", acct, pkg, out) + } + out = compose(`{"module": "shell", "version": "1", "resources": [ + {"id": "package", "type": "package", "package": "zsh"}, + {"id": "login", "type": "user", "name": "op", "shell": "/usr/bin/zsh", "groups": ["wheel"]} + ]}`) + if pkg, acct := indexOf(out, "shell.package"), indexOf(out, "shell.login"); acct < 0 || acct > pkg { + t.Fatalf("an account with a shell is no longer first (%d, package %d): %v", acct, pkg, out) + } +}