From 6ae4ae1dba4a90acc66bcdb622ef11fc2a2054f7 Mon Sep 17 00:00:00 2001 From: jochen Date: Mon, 21 Sep 2026 20:28:16 +0200 Subject: [PATCH 1/9] A module may hold several secrets from one provider, each a pair of its own secrets: maps a requirement to several files under local names. Each local name is its own need, its own pair credential (the pair is keyed on it: migration 0027), its own file on the consumer, its own holder at the provider (the identity with the local name after it) and rotates apart from the others. The plain shape is unchanged and every existing row is the credential it was (novox/hq 04-ISSUES/069, ADR 0094). --- cmd/mesh-controller/plan.go | 4 +- cmd/mesh-controller/rotate.go | 12 +- cmd/mesh-controller/secret.go | 9 +- internal/catalogue/declaration.go | 96 +++++---- internal/catalogue/manifest.go | 188 +++++++++++++++++- internal/catalogue/resolve.go | 26 ++- internal/catalogue/secrets_into_files.go | 37 ++-- internal/catalogue/several_secrets_test.go | 125 ++++++++++++ ...hold-several-secrets-from-one-provider.sql | 12 ++ internal/inventory/operator_test.go | 4 +- internal/inventory/secrets.go | 83 ++++---- internal/inventory/secrets_test.go | 111 ++++++++--- internal/link/enrol_shape_test.go | 2 +- 13 files changed, 567 insertions(+), 142 deletions(-) create mode 100644 internal/catalogue/several_secrets_test.go create mode 100644 internal/inventory/migrations/0027-a-module-may-hold-several-secrets-from-one-provider.sql diff --git a/cmd/mesh-controller/plan.go b/cmd/mesh-controller/plan.go index 17eaa30..db1ff0e 100644 --- a/cmd/mesh-controller/plan.go +++ b/cmd/mesh-controller/plan.go @@ -122,7 +122,7 @@ func planFor(ctx context.Context, open *stores, nodeName string) (catalogue.Reso } continue } - secret, err := inv.SecretFor(ctx, n.Name, nodeName, n.For, n.From) + secret, err := inv.SecretFor(ctx, n.Name, nodeName, n.For, n.From, n.Local) if err != nil { // Said rather than skipped. A machine that resolves cleanly and receives no // credential is one that will fail to authenticate at some later, less obvious @@ -693,7 +693,7 @@ func grantsFor(ctx context.Context, open *stores, node string) ([]catalogue.Gran } out = append(out, catalogue.Grant{ Provision: s.Name, Consumer: s.Consumer, At: onNetwork[s.Consumer], - From: from, Values: values, Slug: slug, Sealed: s.ForProvider}) + From: from, Values: values, Slug: slug, Sealed: s.ForProvider, Local: s.Local}) } return out, nil } diff --git a/cmd/mesh-controller/rotate.go b/cmd/mesh-controller/rotate.go index e7e7c96..0037254 100644 --- a/cmd/mesh-controller/rotate.go +++ b/cmd/mesh-controller/rotate.go @@ -83,11 +83,11 @@ func rotateCommand(ctx context.Context, args []string) error { for _, h := range holders { // The module, because a machine may hold several credentials for one provision and // rotating "anchor's database password" now means rotating three of them. - fmt.Printf(" %s on %s, from %s\n", h.ConsumerModule, h.Consumer, h.Provider) + fmt.Printf(" %s on %s, from %s%s\n", h.ConsumerModule, h.Consumer, h.Provider, asLocal(h.Local)) } for _, h := range holders { - if err := inv.RotateSecret(ctx, h.Provision, h.Consumer, h.ConsumerModule, h.Provider); err != nil { + if err := inv.RotateSecret(ctx, h.Provision, h.Consumer, h.ConsumerModule, h.Provider, h.Local); err != nil { // Partly rotated, and said so plainly. What is gone is remade on the next push, so // the remedy is to run this again rather than to repair anything — but a machine // whose secret was discarded and not resent is holding a credential the provider is @@ -115,3 +115,11 @@ func rotateCommand(ctx context.Context, args []string) error { "changed cannot authenticate — `status` says who is still behind\n", len(machines)) return nil } + +// asLocal names the credential inside the consumer where it holds several (ADR 0094). +func asLocal(local string) string { + if local == "" { + return "" + } + return " (as " + local + ")" +} diff --git a/cmd/mesh-controller/secret.go b/cmd/mesh-controller/secret.go index 295baaf..3a4105d 100644 --- a/cmd/mesh-controller/secret.go +++ b/cmd/mesh-controller/secret.go @@ -52,6 +52,9 @@ func secretCommand(ctx context.Context, args []string) error { provider := set.String("provider", "", "the node providing : the value becomes the PAIR credential between on "+ "and that provider, sealed to both — the vault's operator-delivered secret (ADR 0092)") + local := set.String("local", "", + "with --provider: the name the credential goes by inside , where its manifest keeps "+ + "several for (ADR 0094)") if err := set.Parse(flags); err != nil { return err } @@ -79,10 +82,10 @@ func secretCommand(ctx context.Context, args []string) error { // Into the pair, not into the module's own secrets: what the provider is asked to create // and what the consumer reads are the same value, and neither end can be told a different // one later without the other (novox/hq 04-ISSUES/070). - if err := open.inventory.AcceptSecretForPair(ctx, name, node, module, *provider, value); err != nil { + if err := open.inventory.AcceptSecretForPair(ctx, name, node, module, *provider, *local, value); err != nil { return err } - fmt.Printf("%s on %s now holds %q from %s, sealed to both machines.\n", module, node, name, *provider) + fmt.Printf("%s on %s now holds %q from %s%s, sealed to both machines.\n", module, node, name, *provider, asLocal(*local)) fmt.Printf(" the mesh cannot read it back, will not replace it with one of its own, and will not rotate it\n") fmt.Printf(" run `push %s` and `push %s` to send it\n", *provider, node) return nil @@ -98,7 +101,7 @@ func secretCommand(ctx context.Context, args []string) error { return nil } -const secretUsage = "secret accept [--from ] [--provider ]\n" + +const secretUsage = "secret accept [--from ] [--provider [--local ]]\n" + "secret recover --key [--out ] [--from-export ] [--provider ]\n" + "secret export [--out ]" diff --git a/internal/catalogue/declaration.go b/internal/catalogue/declaration.go index f06e91e..7aae727 100644 --- a/internal/catalogue/declaration.go +++ b/internal/catalogue/declaration.go @@ -60,6 +60,9 @@ type Grant struct { // Values are what that module contributed — the name it wants, and anything else the // provision's own vocabulary defines. Values map[string]any + // Local is the name the credential goes by inside the consumer where it keeps several for one + // provision (ADR 0094); empty for the ordinary one. The provider sees it as a holder of its own. + Local string // Slug is the consumer module's identity slug, if it declared one — carried on the grant so the // provider side derives the same login the consumer does, even across nodes where the consumer's // manifest is not in view (novox/hq ADR 0049). Empty means "use the module name". @@ -285,45 +288,48 @@ func (r Resolution) Declaration(with Rendering) ([]map[string]any, error) { "id": AccessID(a.Path), "type": "access", "path": a.Path, "mode": a.At(), }) } - for _, to := range sortedKeys(m.Secrets) { - var found *Needed - for i, n := range r.Needs { - // **This module's need, not the provision's** (novox/hq 04-ISSUES/022). Matching - // on the name alone, every consumer of a provision took whichever credential - // happened to be last in the list — so on a node with two of them, one module - // would be given the other's password and fail to authenticate with a valid - // credential belonging to somebody else. - if n.Name == to && n.For == m.Module { - found = &r.Needs[i] + for _, to := range m.SecretRequirements() { + for _, file := range m.SecretFiles(to) { + var found *Needed + for i, n := range r.Needs { + // **This module's need, not the provision's** (novox/hq 04-ISSUES/022). Matching + // on the name alone, every consumer of a provision took whichever credential + // happened to be last in the list — so on a node with two of them, one module + // would be given the other's password and fail to authenticate with a valid + // credential belonging to somebody else. And this file's local name, where the + // module keeps several (ADR 0094). + if n.Name == to && n.For == m.Module && n.Local == file.Local { + found = &r.Needs[i] + } } + if found != nil && found.ByRecord && found.Sealed == "" && !found.Manager { + // Answered by a record whose key has not been supplied since this consumer was + // put on it. **Refused, not skipped.** The mesh discarded the plaintext when the + // key was accepted and cannot seal another, so a machine that resolved cleanly + // would receive no file at all and fail at whatever tried to read it — which is + // the outcome ADR 0024 exists to avoid, arrived at politely. + // + // The manager holder is the one exception (novox/hq ADR 0050): an empty refresh token + // is a licence whose manager has not adopted one yet, a real waiting state rather than + // a lost key. It falls through to the skip below — its bound facts (carrying the + // manager's public key) are still delivered, which is what adoption needs to seal the + // first refresh token. + return nil, fmt.Errorf( + "%s on this machine uses the licence %q and no key has been sealed to it. "+ + "The mesh cannot make one; supply it again with `licence key %s`", + m.Module, found.From, found.From) + } + if found == nil || found.Sealed == "" { + // Answered on this machine, or answered by a node the mesh could not seal to. + // Nothing to write either way, and writing an empty credential file would be + // worse than none: something would read it and fail authenticating. + continue + } + first = append(first, ownedBy(m.SecretsOwner, map[string]any{ + "id": SecretID(SecretLocal(to, file.Local)), "type": "file", "path": file.Path, + "sealed": found.Sealed, + })) } - if found != nil && found.ByRecord && found.Sealed == "" && !found.Manager { - // Answered by a record whose key has not been supplied since this consumer was - // put on it. **Refused, not skipped.** The mesh discarded the plaintext when the - // key was accepted and cannot seal another, so a machine that resolved cleanly - // would receive no file at all and fail at whatever tried to read it — which is - // the outcome ADR 0024 exists to avoid, arrived at politely. - // - // The manager holder is the one exception (novox/hq ADR 0050): an empty refresh token - // is a licence whose manager has not adopted one yet, a real waiting state rather than - // a lost key. It falls through to the skip below — its bound facts (carrying the - // manager's public key) are still delivered, which is what adoption needs to seal the - // first refresh token. - return nil, fmt.Errorf( - "%s on this machine uses the licence %q and no key has been sealed to it. "+ - "The mesh cannot make one; supply it again with `licence key %s`", - m.Module, found.From, found.From) - } - if found == nil || found.Sealed == "" { - // Answered on this machine, or answered by a node the mesh could not seal to. - // Nothing to write either way, and writing an empty credential file would be - // worse than none: something would read it and fail authenticating. - continue - } - first = append(first, ownedBy(m.SecretsOwner, map[string]any{ - "id": SecretID(to), "type": "file", "path": m.Secrets[to], - "sealed": found.Sealed, - })) } for _, to := range sortedKeys(m.Grants) { for _, g := range with.Grants { @@ -613,6 +619,15 @@ func grantPath(directory, consumer, module string) string { return strings.TrimRight(directory, "/") + "/" + consumer + "." + module + ".secret" } +// holderAs is a consumer's name at the provider with a local name after it, where it keeps several +// credentials for one provision (ADR 0094); the name alone otherwise. +func holderAs(as, local string) string { + if local == "" { + return as + } + return as + "_" + local +} + // contributions collects what every module in this set contributes, by requirement. // // Ordered by contributing module, because the result becomes a file on a machine and a file whose @@ -649,8 +664,11 @@ func (r Resolution) contributions(settings SettingsBy, grants []Grant, } out[g.Provision] = append(out[g.Provision], Contribution{ From: g.From, Node: g.Consumer, At: g.At, Values: g.Values, - As: ConsumerIdentity(g.Consumer, IdentitySource(g.Slug, g.From)), - Secret: grantPath(directories[g.Provision], g.Consumer, g.From), + // One holder per local name: the identity the consumer is known by, and the local name + // after it where the module keeps several (ADR 0094). Not a login any backend checks — + // a secret is not a login — so the identity limit does not apply to the suffix. + As: holderAs(ConsumerIdentity(g.Consumer, IdentitySource(g.Slug, g.From)), g.Local), + Secret: grantPath(directories[g.Provision], g.Consumer, holderAs(g.From, g.Local)), }) if granted[g.Provision] == nil { granted[g.Provision] = map[string]bool{} diff --git a/internal/catalogue/manifest.go b/internal/catalogue/manifest.go index 4976795..01aaea1 100644 --- a/internal/catalogue/manifest.go +++ b/internal/catalogue/manifest.go @@ -278,6 +278,14 @@ type Manifest struct { // makes `restart-on` precise. Secrets map[string]string `json:"secrets,omitempty"` + // SecretsMany is the same key, `secrets`, where a requirement maps to SEVERAL files under local + // names — `"secret": {"admin": "/…/admin", "token": "/…/token"}` — because a module may need + // more than one value from a provider that gives one per pair (novox/hq 04-ISSUES/069, ADR + // 0094). Each local name is a pair credential of its own, keyed on that name, delivered as its + // own file, served to the provider as its own holder, and rotated with the others. Filled from + // the manifest's `secrets` object by UnmarshalJSON; never written by hand. + SecretsMany map[string]map[string]string `json:"-"` + // OwnSecrets are secrets this module needs in order to be itself, and where to put them. // // **Named for whose they are, not how secret they are.** `secrets` above is a credential for @@ -619,6 +627,134 @@ func ReceivedID(requirement string) string { return "received-" + requirement } // // Every problem is reported rather than the first, because somebody writing a manifest fixes // them in one pass or in four. +// manifestFields is Manifest without its methods, so the JSON methods below can use the ordinary +// field decoding for everything but `secrets`. +type manifestFields Manifest + +// UnmarshalJSON reads `secrets` in both of its shapes — a path, or an object of local names to +// paths (ADR 0094) — and everything else exactly as the fields declare, unknown keys refused. +func (m *Manifest) UnmarshalJSON(raw []byte) error { + var keys map[string]json.RawMessage + if err := json.Unmarshal(raw, &keys); err != nil { + return err + } + plain := map[string]string{} + many := map[string]map[string]string{} + if secrets, ok := keys["secrets"]; ok && string(secrets) != "null" { + var byName map[string]json.RawMessage + if err := json.Unmarshal(secrets, &byName); err != nil { + return fmt.Errorf("secrets: an object of requirement to path, or to {local name: path}: %w", err) + } + for to, v := range byName { + switch { + case len(v) > 0 && v[0] == '"': + var path string + if err := json.Unmarshal(v, &path); err != nil { + return err + } + plain[to] = path + case len(v) > 0 && v[0] == '{': + var paths map[string]string + if err := json.Unmarshal(v, &paths); err != nil { + return fmt.Errorf("secrets.%s: an object of local name to path: %w", to, err) + } + many[to] = paths + default: + return fmt.Errorf("secrets.%s: a path, or an object of local name to path, not %s", to, v) + } + } + delete(keys, "secrets") + } + rest, err := json.Marshal(keys) + if err != nil { + return err + } + decoder := json.NewDecoder(bytes.NewReader(rest)) + decoder.DisallowUnknownFields() + var fields manifestFields + if err := decoder.Decode(&fields); err != nil { + return err + } + *m = Manifest(fields) + if len(plain) > 0 { + m.Secrets = plain + } + if len(many) > 0 { + m.SecretsMany = many + } + return nil +} + +// MarshalJSON writes `secrets` back in the shape it was read: paths, and objects of local names. +func (m Manifest) MarshalJSON() ([]byte, error) { + raw, err := json.Marshal(manifestFields(m)) + if err != nil { + return nil, err + } + if len(m.SecretsMany) == 0 { + return raw, nil + } + var keys map[string]json.RawMessage + if err := json.Unmarshal(raw, &keys); err != nil { + return nil, err + } + merged := map[string]any{} + for to, path := range m.Secrets { + merged[to] = path + } + for to, paths := range m.SecretsMany { + merged[to] = paths + } + secrets, err := json.Marshal(merged) + if err != nil { + return nil, err + } + keys["secrets"] = secrets + return json.Marshal(keys) +} + +// SecretFile is one file a module is given a credential in: the local name it goes by inside +// the module (empty for the ordinary one-file case, where the requirement's name serves) and where. +type SecretFile struct { + Local string + Path string +} + +// SecretFiles is every file a module wants the credential for one requirement in, in a stable +// order: the plain path as one entry with no local name, or one entry per local name. +func (m Manifest) SecretFiles(to string) []SecretFile { + if path, ok := m.Secrets[to]; ok { + return []SecretFile{{Path: path}} + } + paths := m.SecretsMany[to] + out := make([]SecretFile, 0, len(paths)) + for _, local := range sortedKeys(paths) { + out = append(out, SecretFile{Local: local, Path: paths[local]}) + } + return out +} + +// SecretRequirements is every requirement this module wants a credential file for, sorted. +func (m Manifest) SecretRequirements() []string { + seen := map[string]bool{} + for to := range m.Secrets { + seen[to] = true + } + for to := range m.SecretsMany { + seen[to] = true + } + return sortedKeys(seen) +} + +// SecretLocal is the name a credential goes by inside the module: the local name where the +// requirement maps to several, else the requirement itself. It is what `${secret:}` says. +func SecretLocal(to, local string) string { + if local == "" { + return to + } + return local +} + func ParseManifest(raw []byte) (Manifest, error) { var m Manifest // Strictly. **An unknown key is refused**, which is the discipline the host's declaration @@ -908,11 +1044,47 @@ func ParseManifest(raw []byte) (Manifest, error) { problems = append(problems, m.Module+" needs a secret with no name") } } - for to, where := range m.Secrets { - if !strings.HasPrefix(where, "/") { - problems = append(problems, fmt.Sprintf( - "%s keeps the credential for %q at %q, which is not an absolute path", - m.Module, to, where)) + for _, to := range m.SecretRequirements() { + if _, plain := m.Secrets[to]; plain { + if _, also := m.SecretsMany[to]; also { + problems = append(problems, fmt.Sprintf( + "%s keeps the credential for %q both as one file and as several", m.Module, to)) + } + } + for _, f := range m.SecretFiles(to) { + if !strings.HasPrefix(f.Path, "/") { + problems = append(problems, fmt.Sprintf( + "%s keeps the credential for %q at %q, which is not an absolute path", + m.Module, SecretLocal(to, f.Local), f.Path)) + } + if f.Local != "" && !name.MatchString(f.Local) { + problems = append(problems, fmt.Sprintf( + "%s keeps a credential for %q under %q, which is not a usable name", + m.Module, to, f.Local)) + } + // A local name is what `${secret:}` says, so it may not be another requirement's + // name or one of the module's own secrets — the file would hold the wrong credential + // while every check passed. + if f.Local != "" { + if _, own := m.OwnSecrets[f.Local]; own { + problems = append(problems, fmt.Sprintf( + "%s keeps a credential for %q under %q, which is also one of its own secrets", + m.Module, to, f.Local)) + } + for _, w := range m.Wants() { + if w == f.Local { + problems = append(problems, fmt.Sprintf( + "%s keeps a credential for %q under %q, which is also something it requires", + m.Module, to, f.Local)) + } + } + } + } + if len(m.SecretsMany[to]) == 0 && m.Secrets[to] == "" { + if _, many := m.SecretsMany[to]; many { + problems = append(problems, fmt.Sprintf( + "%s keeps the credential for %q as several files and names none", m.Module, to)) + } } var wanted bool for _, w := range m.Wants() { @@ -1119,8 +1291,10 @@ func (m Manifest) undeclaredMounts() []string { for _, where := range m.OwnSecrets { claim(where) } - for _, where := range m.Secrets { - claim(where) + for _, to := range m.SecretRequirements() { + for _, f := range m.SecretFiles(to) { + claim(f.Path) + } } for _, where := range m.Receives { claim(where) diff --git a/internal/catalogue/resolve.go b/internal/catalogue/resolve.go index 55c00b2..319684b 100644 --- a/internal/catalogue/resolve.go +++ b/internal/catalogue/resolve.go @@ -157,6 +157,10 @@ type Needed struct { Sealed string // For is the module that wanted it. For string + // Local is the name this credential goes by inside that module, where the module wants several + // for one requirement (ADR 0094); empty for the ordinary one. Part of what identifies the pair + // credential, so two secrets from one provider to one module are two secrets. + Local string // Manager is set when this holder is a refreshable-grant licence's MANAGER, delivered the refresh // token rather than an access token (novox/hq ADR 0050). It changes one thing downstream: an empty // Sealed is tolerated — the manager has not adopted a refresh token yet, which is a real waiting @@ -298,7 +302,7 @@ func Resolve(catalogue map[string]Manifest, assigned []string, node Node, world if at == "" { at = "127.0.0.1" } - needs = append(needs, Needed{ + needs = eachLocal(needs, catalogue, Needed{ Name: want, From: node.Name, At: at, Serves: servedHere(catalogue, chosen, want), For: because[want]}) } else if served := servedHere(catalogue, chosen, want); len(served) > 0 { @@ -319,7 +323,7 @@ func Resolve(catalogue map[string]Manifest, assigned []string, node Node, world if at == "" { at = "127.0.0.1" } - needs = append(needs, Needed{ + needs = eachLocal(needs, catalogue, Needed{ Name: want, From: node.Name, At: at, Serves: served, For: because[want]}) } continue @@ -347,7 +351,7 @@ func Resolve(catalogue map[string]Manifest, assigned []string, node Node, world node.Name, want, p.Node, meshNetwork)) return } - needs = append(needs, Needed{Name: want, From: p.Node, At: p.At, + needs = eachLocal(needs, catalogue, Needed{Name: want, From: p.Node, At: p.At, Serves: p.Serves, For: because[want]}) } switch { @@ -854,3 +858,19 @@ func perConsumer(needs []Needed, order []string, catalogue map[string]Manifest) } return out } + +// eachLocal appends the need once per file the wanting module keeps the credential in: once, with +// no local name, in the ordinary case; once per local name where the module wants several values +// from one provider (ADR 0094). Each is its own pair credential downstream. +func eachLocal(needs []Needed, catalogue map[string]Manifest, n Needed) []Needed { + files := catalogue[n.For].SecretFiles(n.Name) + if len(files) <= 1 { + return append(needs, n) + } + for _, f := range files { + one := n + one.Local = f.Local + needs = append(needs, one) + } + return needs +} diff --git a/internal/catalogue/secrets_into_files.go b/internal/catalogue/secrets_into_files.go index f47656c..5b1eb97 100644 --- a/internal/catalogue/secrets_into_files.go +++ b/internal/catalogue/secrets_into_files.go @@ -61,23 +61,26 @@ func sealedFor(m Manifest, needs []Needed, with Rendering) (map[string]string, e sealed[name] = value } } - for _, to := range sortedKeys(m.Secrets) { - if _, taken := sealed[to]; taken { - // A module whose own secret and whose requirement share a name. Refused rather than - // settled by precedence: whichever won, the manifest would read as though the other - // had, and the file would hold the credential for the wrong thing while every check - // passed. - return nil, fmt.Errorf( - "%s has a secret of its own called %q and also requires %q, so a file saying "+ - "${secret:%s} could mean either — rename one of them", m.Module, to, to, to) - } - for i := range needs { - // `For == m.Module`, not name alone: on a node with two modules requiring the same - // provision, both appear in `needs`, and matching by name would fill ${secret:X} with - // whichever came last — the other module's credential (novox/hq 04-ISSUES/022). The - // `secrets:`-map path already guards this way; the ${secret:…} placeholder path did not. - if needs[i].Name == to && needs[i].For == m.Module && needs[i].Sealed != "" { - sealed[to] = needs[i].Sealed + for _, to := range m.SecretRequirements() { + for _, file := range m.SecretFiles(to) { + key := SecretLocal(to, file.Local) + if _, taken := sealed[key]; taken { + // A module whose own secret and whose requirement share a name. Refused rather than + // settled by precedence: whichever won, the manifest would read as though the other + // had, and the file would hold the credential for the wrong thing while every check + // passed. + return nil, fmt.Errorf( + "%s has a secret of its own called %q and also requires %q, so a file saying "+ + "${secret:%s} could mean either — rename one of them", m.Module, key, key, key) + } + for i := range needs { + // `For == m.Module`, not name alone: on a node with two modules requiring the same + // provision, both appear in `needs`, and matching by name would fill ${secret:X} with + // whichever came last — the other module's credential (novox/hq 04-ISSUES/022). And + // the local name, where the module keeps several (ADR 0094). + if needs[i].Name == to && needs[i].For == m.Module && needs[i].Local == file.Local && needs[i].Sealed != "" { + sealed[key] = needs[i].Sealed + } } } } diff --git a/internal/catalogue/several_secrets_test.go b/internal/catalogue/several_secrets_test.go new file mode 100644 index 0000000..71acf22 --- /dev/null +++ b/internal/catalogue/several_secrets_test.go @@ -0,0 +1,125 @@ +package catalogue + +import ( + "encoding/json" + "strings" + "testing" +) + +// A module may need several values from one provider that gives one per pair (novox/hq +// 04-ISSUES/069, ADR 0094): `secrets` maps a requirement to several files under local names, and +// each local name is a pair credential of its own — its own need, its own file, its own holder. + +const twoSecrets = `{"module":"ca","version":"1","requires":["secret"], + "secrets":{"secret":{"root-key":"/var/lib/ca/root.key","root-pass":"/var/lib/ca/root.pass"}}, + "resources":[{"id":"state","type":"directory","path":"/var/lib/ca","mode":"0700"}]}` + +func TestSecretsReadBothShapesAndWriteThemBack(t *testing.T) { + m, err := ParseManifest([]byte(twoSecrets)) + if err != nil { + t.Fatal(err) + } + files := m.SecretFiles("secret") + if len(files) != 2 || files[0].Local != "root-key" || files[1].Path != "/var/lib/ca/root.pass" { + t.Fatalf("two files under local names, in order: %+v", files) + } + plain, err := ParseManifest([]byte(`{"module":"app","version":"1","requires":["secret"],"secrets":{"secret":"/var/lib/app/secret"}}`)) + if err != nil { + t.Fatal(err) + } + if got := plain.SecretFiles("secret"); len(got) != 1 || got[0].Local != "" || got[0].Path != "/var/lib/app/secret" { + t.Fatalf("the plain shape is one file with no local name: %+v", got) + } + // Written back in the shape it was read, so a built manifest keeps its local names. + raw, err := json.Marshal(m) + if err != nil { + t.Fatal(err) + } + again, err := ParseManifest(raw) + if err != nil { + t.Fatalf("what was written does not read: %v\n%s", err, raw) + } + if len(again.SecretFiles("secret")) != 2 { + t.Fatalf("the local names did not survive a round trip:\n%s", raw) + } +} + +func TestALocalNameMayNotCollideWithWhatTheModuleAlreadyCallsSomething(t *testing.T) { + for _, bad := range []string{ + // One of the module's own secrets. + `{"module":"ca","version":"1","requires":["secret"],"own-secrets":{"root-key":"/var/lib/ca/own"}, + "secrets":{"secret":{"root-key":"/var/lib/ca/root.key"}}}`, + // Something it requires. + `{"module":"ca","version":"1","requires":["secret","postgres-database"], + "secrets":{"secret":{"postgres-database":"/var/lib/ca/x"}}}`, + // Not a usable name. + `{"module":"ca","version":"1","requires":["secret"],"secrets":{"secret":{"Root Key":"/var/lib/ca/x"}}}`, + // A relative path. + `{"module":"ca","version":"1","requires":["secret"],"secrets":{"secret":{"root-key":"root.key"}}}`, + } { + if _, err := ParseManifest([]byte(bad)); err == nil { + t.Errorf("accepted:\n%s", bad) + } + } +} + +func vaultAndCA() map[string]Manifest { + ca, _ := ParseManifest([]byte(twoSecrets)) + vault := Manifest{Module: "mesh-vault", Version: "1", Provides: FromAnywhere("secret"), + Grants: map[string]string{"secret": "/var/lib/vault/grants"}, + Receives: map[string]string{"secret": "/var/lib/vault/grants/mesh.json"}} + return shelf(vault, ca) +} + +func TestEachLocalNameIsANeedAFileAndAHolderOfItsOwn(t *testing.T) { + got, err := Resolve(vaultAndCA(), []string{"mesh-vault", "ca"}, workstation(), World{}) + if err != nil { + t.Fatal(err) + } + var locals []string + for _, n := range got.Needs { + if n.Name == "secret" && n.For == "ca" { + locals = append(locals, n.Local) + } + } + if strings.Join(locals, ",") != "root-key,root-pass" { + t.Fatalf("two secrets from one provider are two needs: %v", got.Needs) + } + for i := range got.Needs { + got.Needs[i].Sealed = "sealed-" + got.Needs[i].Local + } + out, err := got.Declaration(Rendering{}) + if err != nil { + t.Fatal(err) + } + seen := map[string]string{} + for _, r := range out { + if r["type"] == "file" && strings.HasPrefix(r["path"].(string), "/var/lib/ca/root.") { + seen[r["id"].(string)] = r["sealed"].(string) + } + } + if seen["ca."+SecretID("root-key")] != "sealed-root-key" || seen["ca."+SecretID("root-pass")] != "sealed-root-pass" { + t.Fatalf("each local name is its own file with its own credential: %v", seen) + } +} + +func TestAProviderSeesEachLocalNameAsAHolderOfItsOwn(t *testing.T) { + r := Resolution{Modules: []Manifest{vaultAndCA()["mesh-vault"]}} + got, err := r.contributions(SettingsBy{}, []Grant{ + {Provision: "secret", Consumer: "workstation", From: "ca", Local: "root-key", Sealed: "x"}, + {Provision: "secret", Consumer: "workstation", From: "ca", Local: "root-pass", Sealed: "y"}, + }, map[string]string{"secret": "/var/lib/vault/grants"}) + if err != nil { + t.Fatal(err) + } + given := got["secret"] + if len(given) != 2 { + t.Fatalf("two holders: %+v", given) + } + if given[0].As != "mesh_workstation_ca_root_key" && given[0].As != "mesh_workstation_ca_root-key" { + t.Fatalf("the holder is the consumer's identity with the local name after it: %q", given[0].As) + } + if given[0].Secret == given[1].Secret { + t.Fatalf("two holders share one file on the provider: %q", given[0].Secret) + } +} diff --git a/internal/inventory/migrations/0027-a-module-may-hold-several-secrets-from-one-provider.sql b/internal/inventory/migrations/0027-a-module-may-hold-several-secrets-from-one-provider.sql new file mode 100644 index 0000000..b86832d --- /dev/null +++ b/internal/inventory/migrations/0027-a-module-may-hold-several-secrets-from-one-provider.sql @@ -0,0 +1,12 @@ +-- A module may need several values from one provider that gives one per pair +-- (novox/hq 04-ISSUES/069, ADR 0094). +-- +-- A pair credential was keyed on (provision, consumer node, consumer module, provider): one value +-- per module per provider. Seven catalogue modules hold two to four independent secrets of their +-- own -- a root certificate, its key and that key's password -- and the vault could serve each +-- module one. The pair now carries the LOCAL name the credential goes by inside the module; empty +-- for the ordinary one, so every existing row is the credential it was. + +alter table secret add column local text not null default ''; +alter table secret drop constraint secret_pkey; +alter table secret add primary key (name, local, consumer, consumer_module, provider); diff --git a/internal/inventory/operator_test.go b/internal/inventory/operator_test.go index 4db167a..e4179e9 100644 --- a/internal/inventory/operator_test.go +++ b/internal/inventory/operator_test.go @@ -164,7 +164,7 @@ func TestAPairCredentialIsSealedToTheOperatorToo(t *testing.T) { if _, err := inv.SetOperatorKey(ctx, pub); err != nil { t.Fatal(err) } - made, err := inv.SecretFor(ctx, "secret", "consumer", "gitea", "provider") + made, err := inv.SecretFor(ctx, "secret", "consumer", "gitea", "provider", "") if err != nil { t.Fatal(err) } @@ -191,7 +191,7 @@ func TestAPairCredentialIsSealedToTheOperatorToo(t *testing.T) { // A second provider of the same provision: two rows, refused rather than the first one taken, // unless the provider is named. And replacing the key counts pair credentials as orphaned. - if _, err := inv.SecretFor(ctx, "secret", "consumer", "gitea", "consumer"); err != nil { + if _, err := inv.SecretFor(ctx, "secret", "consumer", "gitea", "consumer", ""); err != nil { t.Fatal(err) } if _, err := inv.KeptSecret(ctx, "consumer", "gitea", "secret", ""); err == nil || !strings.Contains(err.Error(), "--provider") { diff --git a/internal/inventory/secrets.go b/internal/inventory/secrets.go index f605070..1364e13 100644 --- a/internal/inventory/secrets.go +++ b/internal/inventory/secrets.go @@ -24,11 +24,14 @@ type Secret struct { // **Part of the key, not a label** (novox/hq 04-ISSUES/022). Two modules on one node wanting // the same provision are two consumers, and were one credential until this. ConsumerModule string - Provider string - ForConsumer string - ForProvider string - ConsumerKey string - ProviderKey string + // Local is the name the credential goes by inside the consumer where it keeps several for one + // provision (novox/hq ADR 0094); empty for the ordinary one. Part of the key. + Local string + Provider string + ForConsumer string + ForProvider string + ConsumerKey string + ProviderKey string // Origin is `made` — the mesh generated it — or `accepted` — a person supplied it, for // something outside the mesh, and the mesh cannot make another (novox/hq 04-ISSUES/070). Origin string @@ -51,7 +54,7 @@ const ( // can no longer open what was sealed to the old one, so keeping the blob would deliver something // unreadable for ever. The new secret reaches both ends in the same push, which is the only // moment they can be changed together. -func (i *Inventory) SecretFor(ctx context.Context, name, consumer, consumerModule, provider string) ( +func (i *Inventory) SecretFor(ctx context.Context, name, consumer, consumerModule, provider, local string) ( Secret, error) { consumerKey, err := i.SealingKeyOf(ctx, consumer) if err != nil { @@ -74,12 +77,12 @@ func (i *Inventory) SecretFor(ctx context.Context, name, consumer, consumerModul var held Secret err = i.store.Pool().QueryRow(ctx, `select for_consumer, for_provider, consumer_key, provider_key, origin from secret - where name = $1 and consumer = $2 and consumer_module = $3 and provider = $4`, - name, consumerNode.ID, consumerModule, providerNode.ID). + where name = $1 and consumer = $2 and consumer_module = $3 and provider = $4 and local = $5`, + name, consumerNode.ID, consumerModule, providerNode.ID, local). Scan(&held.ForConsumer, &held.ForProvider, &held.ConsumerKey, &held.ProviderKey, &held.Origin) if err == nil && held.ConsumerKey == consumerKey && held.ProviderKey == providerKey { held.Name, held.Consumer, held.Provider = name, consumer, provider - held.ConsumerModule = consumerModule + held.ConsumerModule, held.Local = consumerModule, local return held, nil } if err == nil && held.Origin == OriginAccepted { @@ -90,8 +93,8 @@ func (i *Inventory) SecretFor(ctx context.Context, name, consumer, consumerModul return Secret{}, fmt.Errorf( "%s's %q credential from %s was accepted from a person, and a sealing key at one end "+ "has changed since. The mesh cannot re-seal a value it does not hold: accept it "+ - "again with `secret accept %s %s %s --provider %s`", - consumerModule, name, provider, consumer, consumerModule, name, provider) + "again with `secret accept %s %s %s --provider %s%s`", + consumerModule, name, provider, consumer, consumerModule, name, provider, localFlag(local)) } // And to the operator, when the mesh has one (novox/hq ADR 0085, amended): the third copy that @@ -107,19 +110,19 @@ func (i *Inventory) SecretFor(ctx context.Context, name, consumer, consumerModul forOperator, operatorKey := operatorColumns(operator, blob) _, err = i.store.Pool().Exec(ctx, `insert into secret (name, consumer, consumer_module, provider, for_consumer, for_provider, - consumer_key, provider_key, operator_sealed, operator_key) - values ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10) - on conflict (name, consumer, consumer_module, provider) do update set + consumer_key, provider_key, operator_sealed, operator_key, local) + values ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11) + on conflict (name, local, consumer, consumer_module, provider) do update set for_consumer = excluded.for_consumer, for_provider = excluded.for_provider, consumer_key = excluded.consumer_key, provider_key = excluded.provider_key, created_at = now(), operator_sealed = excluded.operator_sealed, operator_key = excluded.operator_key`, name, consumerNode.ID, consumerModule, providerNode.ID, - made.ForConsumer, made.ForProvider, made.ConsumerKey, made.ProviderKey, forOperator, operatorKey) + made.ForConsumer, made.ForProvider, made.ConsumerKey, made.ProviderKey, forOperator, operatorKey, local) if err != nil { return Secret{}, err } - return Secret{Name: name, Consumer: consumer, ConsumerModule: consumerModule, + return Secret{Name: name, Consumer: consumer, ConsumerModule: consumerModule, Local: local, Provider: provider, ForConsumer: made.ForConsumer, ForProvider: made.ForProvider, ConsumerKey: made.ConsumerKey, ProviderKey: made.ProviderKey, Origin: OriginMade}, nil @@ -133,7 +136,7 @@ func (i *Inventory) SecretFor(ctx context.Context, name, consumer, consumerModul // person can supply. It is the counterpart to AcceptSecretForModule for a module's own secret; // what differs is that both ends of the pair are sealed to, and that the record says `accepted` // so a later read never replaces it with a minted one. The plaintext is discarded here. -func (i *Inventory) AcceptSecretForPair(ctx context.Context, name, consumer, consumerModule, provider, value string) error { +func (i *Inventory) AcceptSecretForPair(ctx context.Context, name, consumer, consumerModule, provider, local, value string) error { consumerKey, err := i.SealingKeyOf(ctx, consumer) if err != nil { return err @@ -165,16 +168,16 @@ func (i *Inventory) AcceptSecretForPair(ctx context.Context, name, consumer, con } _, err = i.store.Pool().Exec(ctx, `insert into secret (name, consumer, consumer_module, provider, for_consumer, for_provider, - consumer_key, provider_key, operator_sealed, operator_key, origin) - values ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11) - on conflict (name, consumer, consumer_module, provider) do update set + consumer_key, provider_key, operator_sealed, operator_key, origin, local) + values ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12) + on conflict (name, local, consumer, consumer_module, provider) do update set for_consumer = excluded.for_consumer, for_provider = excluded.for_provider, consumer_key = excluded.consumer_key, provider_key = excluded.provider_key, created_at = now(), origin = excluded.origin, operator_sealed = excluded.operator_sealed, operator_key = excluded.operator_key`, name, consumerNode.ID, consumerModule, providerNode.ID, sealed.ForConsumer, sealed.ForProvider, sealed.ConsumerKey, sealed.ProviderKey, - forOperator, operatorKey, OriginAccepted) + forOperator, operatorKey, OriginAccepted, local) return err } @@ -190,7 +193,7 @@ func (i *Inventory) AcceptSecretForPair(ctx context.Context, name, consumer, con // **An accepted credential is not rotated.** The mesh did not make it and cannot make its // replacement; deleting it would have the next read mint one, which is exactly the wrong value // delivered with the mesh insisting it was (novox/hq 04-ISSUES/070). Refused, and the remedy named. -func (i *Inventory) RotateSecret(ctx context.Context, name, consumer, consumerModule, provider string) error { +func (i *Inventory) RotateSecret(ctx context.Context, name, consumer, consumerModule, provider, local string) error { consumerNode, err := i.NodeByName(ctx, consumer) if err != nil { return err @@ -202,19 +205,19 @@ func (i *Inventory) RotateSecret(ctx context.Context, name, consumer, consumerMo var origin string err = i.store.Pool().QueryRow(ctx, `select origin from secret where name = $1 and consumer = $2 and consumer_module = $3 - and provider = $4`, - name, consumerNode.ID, consumerModule, providerNode.ID).Scan(&origin) + and provider = $4 and local = $5`, + name, consumerNode.ID, consumerModule, providerNode.ID, local).Scan(&origin) if err == nil && origin == OriginAccepted { return fmt.Errorf( "%s's %q credential from %s was accepted from a person, and the mesh cannot make "+ "its replacement. Accept the new value instead: `secret accept %s %s %s "+ - "--provider %s --from `", - consumerModule, name, provider, consumer, consumerModule, name, provider) + "--provider %s%s --from `", + consumerModule, name, provider, consumer, consumerModule, name, provider, localFlag(local)) } _, err = i.store.Pool().Exec(ctx, `delete from secret where name = $1 and consumer = $2 and consumer_module = $3 - and provider = $4`, - name, consumerNode.ID, consumerModule, providerNode.ID) + and provider = $4 and local = $5`, + name, consumerNode.ID, consumerModule, providerNode.ID, local) return err } @@ -225,9 +228,9 @@ func (i *Inventory) SecretsFrom(ctx context.Context, provider string) ([]Secret, return nil, err } rows, err := i.store.Pool().Query(ctx, - `select s.name, c.name, s.consumer_module, s.for_provider from secret s + `select s.name, c.name, s.consumer_module, s.local, s.for_provider from secret s join node c on c.id = s.consumer - where s.provider = $1 order by s.name, c.name, s.consumer_module`, providerNode.ID) + where s.provider = $1 order by s.name, c.name, s.consumer_module, s.local`, providerNode.ID) if err != nil { return nil, err } @@ -236,7 +239,7 @@ func (i *Inventory) SecretsFrom(ctx context.Context, provider string) ([]Secret, var out []Secret for rows.Next() { s := Secret{Provider: provider} - if err := rows.Scan(&s.Name, &s.Consumer, &s.ConsumerModule, &s.ForProvider); err != nil { + if err := rows.Scan(&s.Name, &s.Consumer, &s.ConsumerModule, &s.Local, &s.ForProvider); err != nil { return nil, err } out = append(out, s) @@ -401,7 +404,9 @@ type Holder struct { // ConsumerModule is which module on that machine holds it. Part of what identifies a // credential (novox/hq 04-ISSUES/022), so rotating one consumer's does not touch another's. ConsumerModule string - Provider string + // Local is the credential's name inside the consumer where it holds several (ADR 0094). + Local string + Provider string } // HoldersOf is every pair sharing a credential for one provision. @@ -415,11 +420,11 @@ type Holder struct { // Empty consumer means all of them. func (i *Inventory) HoldersOf(ctx context.Context, provision, consumer string) ([]Holder, error) { rows, err := i.store.Pool().Query(ctx, - `select s.name, c.name, s.consumer_module, p.name from secret s + `select s.name, c.name, s.consumer_module, s.local, p.name from secret s join node c on c.id = s.consumer join node p on p.id = s.provider where s.name = $1 and ($2 = '' or c.name = $2) - order by c.name, s.consumer_module, p.name`, provision, consumer) + order by c.name, s.consumer_module, s.local, p.name`, provision, consumer) if err != nil { return nil, err } @@ -428,10 +433,18 @@ func (i *Inventory) HoldersOf(ctx context.Context, provision, consumer string) ( var out []Holder for rows.Next() { var h Holder - if err := rows.Scan(&h.Provision, &h.Consumer, &h.ConsumerModule, &h.Provider); err != nil { + if err := rows.Scan(&h.Provision, &h.Consumer, &h.ConsumerModule, &h.Local, &h.Provider); err != nil { return nil, err } out = append(out, h) } return out, rows.Err() } + +// localFlag is the `--local` a remedy has to name where a credential has a local name. +func localFlag(local string) string { + if local == "" { + return "" + } + return " --local " + local +} diff --git a/internal/inventory/secrets_test.go b/internal/inventory/secrets_test.go index 07c6a46..b6e015d 100644 --- a/internal/inventory/secrets_test.go +++ b/internal/inventory/secrets_test.go @@ -63,11 +63,11 @@ func TestASecretIsMadeOnceAndKept(t *testing.T) { // Regenerating on every declaration would restart both ends on every push, and — worse — the // password a provider was told to create would never be the one its consumer was given. inv, ctx := twoNodesWithKeys(t) - first, err := inv.SecretFor(ctx, "postgres-database", "consumer", "gitea", "provider") + first, err := inv.SecretFor(ctx, "postgres-database", "consumer", "gitea", "provider", "") if err != nil { t.Fatal(err) } - second, err := inv.SecretFor(ctx, "postgres-database", "consumer", "gitea", "provider") + second, err := inv.SecretFor(ctx, "postgres-database", "consumer", "gitea", "provider", "") if err != nil { t.Fatal(err) } @@ -81,7 +81,7 @@ func TestTheStoredSecretIsNotTheSecret(t *testing.T) { // what an encrypted column does not achieve, because whoever runs the control plane can read // through it. inv, ctx := twoNodesWithKeys(t) - got, err := inv.SecretFor(ctx, "postgres-database", "consumer", "gitea", "provider") + got, err := inv.SecretFor(ctx, "postgres-database", "consumer", "gitea", "provider", "") if err != nil { t.Fatal(err) } @@ -114,7 +114,7 @@ func TestANewSealingKeyMeansANewSecret(t *testing.T) { // A node that rejoined generated a new key and can no longer open what was sealed to the old // one. Keeping the blob would deliver something unreadable for ever, reported as configured. inv, ctx := twoNodesWithKeys(t) - before, err := inv.SecretFor(ctx, "postgres-database", "consumer", "gitea", "provider") + before, err := inv.SecretFor(ctx, "postgres-database", "consumer", "gitea", "provider", "") if err != nil { t.Fatal(err) } @@ -126,7 +126,7 @@ func TestANewSealingKeyMeansANewSecret(t *testing.T) { if err := inv.RecordSealingKey(ctx, node.ID, fresh); err != nil { t.Fatal(err) } - after, err := inv.SecretFor(ctx, "postgres-database", "consumer", "gitea", "provider") + after, err := inv.SecretFor(ctx, "postgres-database", "consumer", "gitea", "provider", "") if err != nil { t.Fatal(err) } @@ -142,14 +142,14 @@ func TestANewSealingKeyMeansANewSecret(t *testing.T) { func TestRotatingReachesBothEnds(t *testing.T) { inv, ctx := twoNodesWithKeys(t) - before, err := inv.SecretFor(ctx, "postgres-database", "consumer", "gitea", "provider") + before, err := inv.SecretFor(ctx, "postgres-database", "consumer", "gitea", "provider", "") if err != nil { t.Fatal(err) } - if err := inv.RotateSecret(ctx, "postgres-database", "consumer", "gitea", "provider"); err != nil { + if err := inv.RotateSecret(ctx, "postgres-database", "consumer", "gitea", "provider", ""); err != nil { t.Fatal(err) } - after, err := inv.SecretFor(ctx, "postgres-database", "consumer", "gitea", "provider") + after, err := inv.SecretFor(ctx, "postgres-database", "consumer", "gitea", "provider", "") if err != nil { t.Fatal(err) } @@ -184,7 +184,7 @@ func TestAProviderIsToldEveryCredentialItMustCreate(t *testing.T) { t.Fatal(err) } for _, who := range []string{"consumer", "second-consumer"} { - if _, err := inv.SecretFor(ctx, "postgres-database", who, "gitea", "provider"); err != nil { + if _, err := inv.SecretFor(ctx, "postgres-database", who, "gitea", "provider", ""); err != nil { t.Fatal(err) } } @@ -215,7 +215,7 @@ func TestANodeWithNoSealingKeyCannotBeGivenASecret(t *testing.T) { t.Fatal(err) } } - _, err := inv.SecretFor(ctx, "postgres-database", "consumer", "gitea", "provider") + _, err := inv.SecretFor(ctx, "postgres-database", "consumer", "gitea", "provider", "") if err == nil { t.Fatal("a credential was made for nodes that cannot open one") } @@ -226,7 +226,7 @@ func TestANodeWithNoSealingKeyCannotBeGivenASecret(t *testing.T) { func TestSecretsGoWhenANodeLeaves(t *testing.T) { inv, ctx := twoNodesWithKeys(t) - if _, err := inv.SecretFor(ctx, "postgres-database", "consumer", "gitea", "provider"); err != nil { + if _, err := inv.SecretFor(ctx, "postgres-database", "consumer", "gitea", "provider", ""); err != nil { t.Fatal(err) } if _, err := inv.store.Pool().Exec(ctx, `delete from node where name = 'consumer'`); err != nil { @@ -349,7 +349,7 @@ func TestACredentialGoesWhenTheConsumerStopsAskingForIt(t *testing.T) { if err := inv.Assign(ctx, "consumer", "meshboard"); err != nil { t.Fatal(err) } - if _, err := inv.SecretFor(ctx, "postgres-database", "consumer", "gitea", "provider"); err != nil { + if _, err := inv.SecretFor(ctx, "postgres-database", "consumer", "gitea", "provider", ""); err != nil { t.Fatal(err) } @@ -379,7 +379,7 @@ func TestACredentialGoesWhenEitherMachineDoes(t *testing.T) { // The case that must not leave a live login behind: a machine removed from the mesh. Its // credentials go with it, and the provider stops being told to keep them. inv, ctx := twoNodesWithKeys(t) - if _, err := inv.SecretFor(ctx, "postgres-database", "consumer", "gitea", "provider"); err != nil { + if _, err := inv.SecretFor(ctx, "postgres-database", "consumer", "gitea", "provider", ""); err != nil { t.Fatal(err) } if _, err := inv.store.Pool().Exec(ctx, `delete from node where name = 'consumer'`); err != nil { @@ -473,12 +473,12 @@ func TestEveryHolderOfACredentialCanBeNamed(t *testing.T) { t.Fatal(err) } for _, consumer := range []string{"consumer", "third"} { - if _, err := inv.SecretFor(ctx, "postgres-database", consumer, "gitea", "provider"); err != nil { + if _, err := inv.SecretFor(ctx, "postgres-database", consumer, "gitea", "provider", ""); err != nil { t.Fatal(err) } } // And one for a different provision, which must not be swept up. - if _, err := inv.SecretFor(ctx, "cache", "consumer", "gitea", "provider"); err != nil { + if _, err := inv.SecretFor(ctx, "cache", "consumer", "gitea", "provider", ""); err != nil { t.Fatal(err) } @@ -509,14 +509,14 @@ func TestEveryHolderOfACredentialCanBeNamed(t *testing.T) { // And rotating gives both ends a new credential, together — the same one. func TestRotatingGivesBothEndsTheSameNewCredential(t *testing.T) { inv, ctx := twoNodesWithKeys(t) - before, err := inv.SecretFor(ctx, "postgres-database", "consumer", "gitea", "provider") + before, err := inv.SecretFor(ctx, "postgres-database", "consumer", "gitea", "provider", "") if err != nil { t.Fatal(err) } - if err := inv.RotateSecret(ctx, "postgres-database", "consumer", "gitea", "provider"); err != nil { + if err := inv.RotateSecret(ctx, "postgres-database", "consumer", "gitea", "provider", ""); err != nil { t.Fatal(err) } - after, err := inv.SecretFor(ctx, "postgres-database", "consumer", "gitea", "provider") + after, err := inv.SecretFor(ctx, "postgres-database", "consumer", "gitea", "provider", "") if err != nil { t.Fatal(err) } @@ -543,14 +543,14 @@ func TestRotatingGivesBothEndsTheSameNewCredential(t *testing.T) { if err := inv.RecordSealingKey(ctx, third.ID, key); err != nil { t.Fatal(err) } - untouched, err := inv.SecretFor(ctx, "postgres-database", "third", "gitea", "provider") + untouched, err := inv.SecretFor(ctx, "postgres-database", "third", "gitea", "provider", "") if err != nil { t.Fatal(err) } - if err := inv.RotateSecret(ctx, "postgres-database", "consumer", "gitea", "provider"); err != nil { + if err := inv.RotateSecret(ctx, "postgres-database", "consumer", "gitea", "provider", ""); err != nil { t.Fatal(err) } - again, err := inv.SecretFor(ctx, "postgres-database", "third", "gitea", "provider") + again, err := inv.SecretFor(ctx, "postgres-database", "third", "gitea", "provider", "") if err != nil { t.Fatal(err) } @@ -565,17 +565,17 @@ func TestRotatingGivesBothEndsTheSameNewCredential(t *testing.T) { // because it cannot make the replacement. func TestAnAcceptedPairCredentialIsKeptAndNeverRemade(t *testing.T) { inv, ctx := twoNodesWithKeys(t) - if err := inv.AcceptSecretForPair(ctx, "secret", "consumer", "gitea", "provider", "hunter2"); err != nil { + if err := inv.AcceptSecretForPair(ctx, "secret", "consumer", "gitea", "provider", "", "hunter2"); err != nil { t.Fatal(err) } - got, err := inv.SecretFor(ctx, "secret", "consumer", "gitea", "provider") + got, err := inv.SecretFor(ctx, "secret", "consumer", "gitea", "provider", "") if err != nil { t.Fatal(err) } if got.Origin != OriginAccepted { t.Fatalf("an accepted credential reads back as %q", got.Origin) } - again, err := inv.SecretFor(ctx, "secret", "consumer", "gitea", "provider") + again, err := inv.SecretFor(ctx, "secret", "consumer", "gitea", "provider", "") if err != nil { t.Fatal(err) } @@ -584,15 +584,15 @@ func TestAnAcceptedPairCredentialIsKeptAndNeverRemade(t *testing.T) { } // Rotation is refused, and says what to do instead. - err = inv.RotateSecret(ctx, "secret", "consumer", "gitea", "provider") + err = inv.RotateSecret(ctx, "secret", "consumer", "gitea", "provider", "") if err == nil || !strings.Contains(err.Error(), "secret accept") { t.Fatalf("rotating an accepted credential was not refused with the remedy: %v", err) } // And a made one still rotates. - if _, err := inv.SecretFor(ctx, "postgres-database", "consumer", "gitea", "provider"); err != nil { + if _, err := inv.SecretFor(ctx, "postgres-database", "consumer", "gitea", "provider", ""); err != nil { t.Fatal(err) } - if err := inv.RotateSecret(ctx, "postgres-database", "consumer", "gitea", "provider"); err != nil { + if err := inv.RotateSecret(ctx, "postgres-database", "consumer", "gitea", "provider", ""); err != nil { t.Fatalf("a made credential no longer rotates: %v", err) } } @@ -601,7 +601,7 @@ func TestAnAcceptedPairCredentialIsKeptAndNeverRemade(t *testing.T) { // re-seal what it does not hold: refused aloud, never quietly replaced by a minted one. func TestAnAcceptedPairCredentialIsNotRemadeWhenAKeyChanges(t *testing.T) { inv, ctx := twoNodesWithKeys(t) - if err := inv.AcceptSecretForPair(ctx, "secret", "consumer", "gitea", "provider", "hunter2"); err != nil { + if err := inv.AcceptSecretForPair(ctx, "secret", "consumer", "gitea", "provider", "", "hunter2"); err != nil { t.Fatal(err) } node, err := inv.NodeByName(ctx, "consumer") @@ -612,15 +612,64 @@ func TestAnAcceptedPairCredentialIsNotRemadeWhenAKeyChanges(t *testing.T) { if err := inv.RecordSealingKey(ctx, node.ID, fresh); err != nil { t.Fatal(err) } - _, err = inv.SecretFor(ctx, "secret", "consumer", "gitea", "provider") + _, err = inv.SecretFor(ctx, "secret", "consumer", "gitea", "provider", "") if err == nil || !strings.Contains(err.Error(), "accept it again") { t.Fatalf("an accepted credential was remade, or refused without the remedy: %v", err) } // Accepting it again is the remedy, and it works. - if err := inv.AcceptSecretForPair(ctx, "secret", "consumer", "gitea", "provider", "hunter3"); err != nil { + if err := inv.AcceptSecretForPair(ctx, "secret", "consumer", "gitea", "provider", "", "hunter3"); err != nil { t.Fatal(err) } - if _, err := inv.SecretFor(ctx, "secret", "consumer", "gitea", "provider"); err != nil { + if _, err := inv.SecretFor(ctx, "secret", "consumer", "gitea", "provider", ""); err != nil { t.Fatal(err) } } + +// Two secrets from one provider to one module are two credentials (novox/hq 04-ISSUES/069, ADR +// 0094): keyed on the local name, made and rotated apart, and listed apart for the provider. +func TestTwoLocalNamesAreTwoCredentials(t *testing.T) { + inv, ctx := twoNodesWithKeys(t) + key, err := inv.SecretFor(ctx, "secret", "consumer", "gitea", "provider", "root-key") + if err != nil { + t.Fatal(err) + } + pass, err := inv.SecretFor(ctx, "secret", "consumer", "gitea", "provider", "root-pass") + if err != nil { + t.Fatal(err) + } + if key.ForConsumer == pass.ForConsumer { + t.Fatal("two local names were given one credential") + } + if err := inv.RotateSecret(ctx, "secret", "consumer", "gitea", "provider", "root-key"); err != nil { + t.Fatal(err) + } + keyAgain, err := inv.SecretFor(ctx, "secret", "consumer", "gitea", "provider", "root-key") + if err != nil { + t.Fatal(err) + } + passAgain, err := inv.SecretFor(ctx, "secret", "consumer", "gitea", "provider", "root-pass") + if err != nil { + t.Fatal(err) + } + if keyAgain.ForConsumer == key.ForConsumer || passAgain.ForConsumer != pass.ForConsumer { + t.Fatal("rotating one local name touched the other, or neither") + } + holders, err := inv.HoldersOf(ctx, "secret", "") + if err != nil { + t.Fatal(err) + } + var locals []string + for _, h := range holders { + locals = append(locals, h.Local) + } + if strings.Join(locals, ",") != "root-key,root-pass" { + t.Fatalf("the holders are listed apart, by local name: %v", holders) + } + from, err := inv.SecretsFrom(ctx, "provider") + if err != nil { + t.Fatal(err) + } + if len(from) != 2 || from[0].Local == from[1].Local { + t.Fatalf("the provider is told two credentials to create: %+v", from) + } +} diff --git a/internal/link/enrol_shape_test.go b/internal/link/enrol_shape_test.go index e28070b..54a1b54 100644 --- a/internal/link/enrol_shape_test.go +++ b/internal/link/enrol_shape_test.go @@ -85,7 +85,7 @@ func TestWhatANodeSaysWhenItJoinsIsWhatThisMeshReads(t *testing.T) { t.Fatal(err) } - secret, err := inv.SecretFor(ctx, "postgres-database", request.Node, "gitea", "the-other-end") + secret, err := inv.SecretFor(ctx, "postgres-database", request.Node, "gitea", "the-other-end", "") if err != nil { t.Fatalf("nothing could be sealed to a key that arrived from a real node: %v", err) } From 973cda5d769318c588f023fd0d95619d9353139c Mon Sep 17 00:00:00 2001 From: jochen Date: Mon, 21 Sep 2026 20:34:03 +0200 Subject: [PATCH 2/9] ask: the control plane calls a module's tool and prints its answer A module serves tools under an account scoped to exactly that, and nothing else in the mesh held an account that could ask one. The control plane does: ask publishes on the RPC exchange with a private reply queue bound under its own name, checks the correlation, prints the answer, and exits non-zero for a tool that answered with an error or a module that never answered (novox/hq 04-ISSUES/049, ADR 0095). --- cmd/mesh-controller/ask.go | 62 ++++++++++++++++++++++++++ cmd/mesh-controller/main.go | 3 ++ internal/link/ask.go | 86 +++++++++++++++++++++++++++++++++++++ 3 files changed, 151 insertions(+) create mode 100644 cmd/mesh-controller/ask.go create mode 100644 internal/link/ask.go diff --git a/cmd/mesh-controller/ask.go b/cmd/mesh-controller/ask.go new file mode 100644 index 0000000..001d5e2 --- /dev/null +++ b/cmd/mesh-controller/ask.go @@ -0,0 +1,62 @@ +package main + +import ( + "context" + "encoding/json" + "errors" + "flag" + "fmt" + "os" + "time" + + "github.com/novox/mesh-controller/internal/link" +) + +// ask calls one of a module's tools, through the control plane's own broker connection. +// +// A module serves tools under an account scoped to exactly that (novox/hq ADR 0047), and nothing +// else in the mesh held an account that could ask one — not an operator at a terminal, not an agent +// acting for one (novox/hq 04-ISSUES/049). The control plane does, so it is the way in: one +// process, one connection, one place a question can be seen to have been asked (ADR 0095). +func askCommand(ctx context.Context, args []string) error { + positionals, flags := split(args) + set := flag.NewFlagSet("ask", flag.ContinueOnError) + wait := set.Duration("wait", 60*time.Second, "how long to wait for the module's answer") + if err := set.Parse(flags); err != nil { + return err + } + if len(positionals) < 2 || len(positionals) > 3 { + return errors.New("ask [json arguments] [--wait 60s]") + } + module, tool := positionals[0], positionals[1] + var arguments json.RawMessage + if len(positionals) == 3 { + if !json.Valid([]byte(positionals[2])) { + return fmt.Errorf("the arguments are not JSON: %s", positionals[2]) + } + arguments = json.RawMessage(positionals[2]) + } + + server, err := link.Connect(nil, nil) + if err != nil { + return err + } + defer server.Close() + + answer, err := link.Ask(ctx, server.Channel(), module, tool, arguments, *wait) + if err != nil { + return err + } + // The answer as the module gave it, to standard output, for a person or a program. A tool + // that answered with an error has still answered: printed the same way, and the exit status + // says which. + body, err := json.Marshal(answer) + if err != nil { + return err + } + fmt.Fprintln(os.Stdout, string(body)) + if answer.Error != "" { + return fmt.Errorf("%s.%s answered with an error: %s", module, tool, answer.Error) + } + return nil +} diff --git a/cmd/mesh-controller/main.go b/cmd/mesh-controller/main.go index 08bf58e..70b6bfb 100644 --- a/cmd/mesh-controller/main.go +++ b/cmd/mesh-controller/main.go @@ -68,6 +68,8 @@ func run() error { return licenceCommand(ctx, args[1:]) case "rotate": return rotateCommand(ctx, args[1:]) + case "ask": + return askCommand(ctx, args[1:]) case "builds": return buildsCommand(ctx, args[1:]) case "pin": @@ -171,6 +173,7 @@ func usage() { licence manager the node that holds a refreshable licence's refresh token licence refresh mint a new access token and seal it to every holder rotate [--consumer ] a new credential for every holder, both ends at once + ask [json] call one of a module's tools over the broker, and print its answer pin which node this one gets a provision from unpin put that question back plan [--files|--json] what that node would run, and why diff --git a/internal/link/ask.go b/internal/link/ask.go new file mode 100644 index 0000000..df29a8b --- /dev/null +++ b/internal/link/ask.go @@ -0,0 +1,86 @@ +package link + +import ( + "context" + "encoding/json" + "fmt" + "time" + + amqp "github.com/rabbitmq/amqp091-go" +) + +// RPCExchange is where a module's tools are asked over the broker, keyed `.`, and +// where the answer comes back, keyed by the asker's reply queue (novox/hq ADR 0047). +const RPCExchange = "mesh.rpc" + +// Answer is what a module's tool replies: one of the two, never both. +type Answer struct { + Result json.RawMessage `json:"result,omitempty"` + Error string `json:"error,omitempty"` +} + +// Ask calls one of a module's tools over the broker and waits for its answer. +// +// **The control plane is the way in** (novox/hq 04-ISSUES/049, ADR 0095). A module's broker +// account is scoped to what it emits, consumes and serves, and a tool call needs a reply queue the +// caller creates and a publish to the serving module's request key — which no module's scope +// grants, and should not. The control plane already holds a connection that may, so a person or +// an agent asks through it, and every question passes one process where an audit belongs. +// +// The reply queue is the caller's own, server-named and exclusive, bound to the RPC exchange under +// its own name: a serving module answers through that exchange and never the default one, whose +// permission is per exchange rather than per queue. The correlation is checked rather than +// assumed, as every RPC here is. +func Ask(ctx context.Context, channel *amqp.Channel, module, tool string, args json.RawMessage, + timeout time.Duration) (Answer, error) { + + if len(args) == 0 { + args = json.RawMessage(`{}`) + } + replies, err := channel.QueueDeclare("", false, true, true, false, nil) + if err != nil { + return Answer{}, err + } + if err := channel.QueueBind(replies.Name, replies.Name, RPCExchange, false, nil); err != nil { + return Answer{}, fmt.Errorf("cannot bind a reply queue to %s: %w", RPCExchange, err) + } + answers, err := channel.ConsumeWithContext(ctx, replies.Name, "", true, true, false, false, nil) + if err != nil { + return Answer{}, err + } + + id := fmt.Sprintf("ask-%d", time.Now().UnixNano()) + key := module + "." + tool + if err := channel.PublishWithContext(ctx, RPCExchange, key, false, false, amqp.Publishing{ + ContentType: "application/json", + CorrelationId: id, + ReplyTo: replies.Name, + Body: args, + }); err != nil { + return Answer{}, fmt.Errorf("cannot ask %s: %w", key, err) + } + + waiting, cancel := context.WithTimeout(ctx, timeout) + defer cancel() + for { + select { + case <-waiting.Done(): + return Answer{}, fmt.Errorf( + "%s did not answer within %s. Its runtime serves %q when it is up and has bound "+ + "the broker — `status` says whether the machine carrying it has applied", + module, timeout, key) + case delivery, ok := <-answers: + if !ok { + return Answer{}, fmt.Errorf("the connection closed while waiting for %s", key) + } + if delivery.CorrelationId != id { + continue + } + var answer Answer + if err := json.Unmarshal(delivery.Body, &answer); err != nil { + return Answer{}, fmt.Errorf("%s answered with something unreadable: %w", key, err) + } + return answer, nil + } + } +} From 76773dfbf54ad7bb8df23ca67f35ae39c3b4ef1b Mon Sep 17 00:00:00 2001 From: jochen Date: Mon, 21 Sep 2026 20:36:19 +0200 Subject: [PATCH 3/9] Several secrets expand where the consumer is known, not on the first module to mention the provision The lab's two-secrets consumer was given one credential and no file: the expansion ran on the resolver's walk over names, on whichever module mentioned the provision first, and the per-consumer pass copied that. It expands in that pass now, and a test has two consumers of one provision, one keeping one file and one keeping two. --- internal/catalogue/resolve.go | 10 ++++++---- internal/catalogue/several_secrets_test.go | 17 ++++++++++++++--- 2 files changed, 20 insertions(+), 7 deletions(-) diff --git a/internal/catalogue/resolve.go b/internal/catalogue/resolve.go index 319684b..ded00e0 100644 --- a/internal/catalogue/resolve.go +++ b/internal/catalogue/resolve.go @@ -302,7 +302,7 @@ func Resolve(catalogue map[string]Manifest, assigned []string, node Node, world if at == "" { at = "127.0.0.1" } - needs = eachLocal(needs, catalogue, Needed{ + needs = append(needs, Needed{ Name: want, From: node.Name, At: at, Serves: servedHere(catalogue, chosen, want), For: because[want]}) } else if served := servedHere(catalogue, chosen, want); len(served) > 0 { @@ -323,7 +323,7 @@ func Resolve(catalogue map[string]Manifest, assigned []string, node Node, world if at == "" { at = "127.0.0.1" } - needs = eachLocal(needs, catalogue, Needed{ + needs = append(needs, Needed{ Name: want, From: node.Name, At: at, Serves: served, For: because[want]}) } continue @@ -351,7 +351,7 @@ func Resolve(catalogue map[string]Manifest, assigned []string, node Node, world node.Name, want, p.Node, meshNetwork)) return } - needs = eachLocal(needs, catalogue, Needed{Name: want, From: p.Node, At: p.At, + needs = append(needs, Needed{Name: want, From: p.Node, At: p.At, Serves: p.Serves, For: because[want]}) } switch { @@ -847,7 +847,9 @@ func perConsumer(needs []Needed, order []string, catalogue map[string]Manifest) } copied := n copied.For = m.Module - out = append(out, copied) + // And once per file THIS module keeps the credential in, where it keeps several + // (ADR 0094) — here, where the consumer is finally known, not on the walk above. + out = eachLocal(out, catalogue, copied) wanted = true break } diff --git a/internal/catalogue/several_secrets_test.go b/internal/catalogue/several_secrets_test.go index 71acf22..11872f4 100644 --- a/internal/catalogue/several_secrets_test.go +++ b/internal/catalogue/several_secrets_test.go @@ -68,23 +68,34 @@ func vaultAndCA() map[string]Manifest { vault := Manifest{Module: "mesh-vault", Version: "1", Provides: FromAnywhere("secret"), Grants: map[string]string{"secret": "/var/lib/vault/grants"}, Receives: map[string]string{"secret": "/var/lib/vault/grants/mesh.json"}} - return shelf(vault, ca) + // A second consumer of the same provision that keeps ONE file, mentioned before the one that + // keeps two: the lab found the expansion done on the first module to mention the provision, + // and the second consumer given one credential and no file. + cache := Manifest{Module: "cache", Version: "1", Requires: []string{"secret"}, + Secrets: map[string]string{"secret": "/var/lib/cache/secret"}} + return shelf(vault, cache, ca) } func TestEachLocalNameIsANeedAFileAndAHolderOfItsOwn(t *testing.T) { - got, err := Resolve(vaultAndCA(), []string{"mesh-vault", "ca"}, workstation(), World{}) + got, err := Resolve(vaultAndCA(), []string{"mesh-vault", "cache", "ca"}, workstation(), World{}) if err != nil { t.Fatal(err) } - var locals []string + var locals, cacheLocals []string for _, n := range got.Needs { if n.Name == "secret" && n.For == "ca" { locals = append(locals, n.Local) } + if n.Name == "secret" && n.For == "cache" { + cacheLocals = append(cacheLocals, n.Local) + } } if strings.Join(locals, ",") != "root-key,root-pass" { t.Fatalf("two secrets from one provider are two needs: %v", got.Needs) } + if len(cacheLocals) != 1 || cacheLocals[0] != "" { + t.Fatalf("the one-file consumer keeps one need with no local name: %v", got.Needs) + } for i := range got.Needs { got.Needs[i].Sealed = "sealed-" + got.Needs[i].Local } From 3e5c010c5e98900c7c40d3ef9440c4970061f373 Mon Sep 17 00:00:00 2001 From: jochen Date: Mon, 21 Sep 2026 20:38:01 +0200 Subject: [PATCH 4/9] A need is kept once per provision, consumer, local name and provider MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two consumers of one same-node provision produced two raw needs and, fanned out per consumer, four — the same credential twice for each. Harmless, since a pair is one row however often it is asked for, and wrong all the same. --- internal/catalogue/resolve.go | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/internal/catalogue/resolve.go b/internal/catalogue/resolve.go index ded00e0..8439f2c 100644 --- a/internal/catalogue/resolve.go +++ b/internal/catalogue/resolve.go @@ -834,6 +834,19 @@ func providersFirst(order []string, shelf map[string]Manifest) []string { // losing the one it depends on. func perConsumer(needs []Needed, order []string, catalogue map[string]Manifest) []Needed { out := make([]Needed, 0, len(needs)) + // Once per (provision, consumer, local name, provider). The walk over names visits a + // same-node provision once per module that mentions it, so two consumers of one produced two + // raw needs and, fanned out below, four — the same credential twice for each. Harmless + // downstream, since a pair is one row however often it is asked for, and wrong all the same. + seen := map[[4]string]bool{} + keep := func(n Needed) { + key := [4]string{n.Name, n.For, n.Local, n.From} + if seen[key] { + return + } + seen[key] = true + out = append(out, n) + } for _, n := range needs { var wanted bool for _, name := range order { @@ -849,13 +862,15 @@ func perConsumer(needs []Needed, order []string, catalogue map[string]Manifest) copied.For = m.Module // And once per file THIS module keeps the credential in, where it keeps several // (ADR 0094) — here, where the consumer is finally known, not on the walk above. - out = eachLocal(out, catalogue, copied) + for _, one := range eachLocal(nil, catalogue, copied) { + keep(one) + } wanted = true break } } if !wanted { - out = append(out, n) + keep(n) } } return out From 5049d201c693b86fb5014dd2fde8ec75c4d4284b Mon Sep 17 00:00:00 2001 From: jochen Date: Mon, 21 Sep 2026 20:41:19 +0200 Subject: [PATCH 5/9] A provider keeps one grant file per holder, with the local name in its id and path The lab's vault refused a declaration naming two files with one identity: the two secrets of one consumer. The holder's suffix is in the resource id and the path now. --- internal/builder/builder.go | 21 ++++++++++++++---- internal/catalogue/declaration.go | 6 ++++-- internal/catalogue/several_secrets_test.go | 25 ++++++++++++++++++++++ 3 files changed, 46 insertions(+), 6 deletions(-) diff --git a/internal/builder/builder.go b/internal/builder/builder.go index a174d48..8dc0dcc 100644 --- a/internal/builder/builder.go +++ b/internal/builder/builder.go @@ -324,14 +324,27 @@ func one(ctx context.Context, run Runner, publish Publisher, switch a.Kind { case catalogue.ArtifactUpstream: - // Mirrored, not built. Pulled by the reference the module names and pushed under a name - // of the mesh's own, so what a machine fetches is pinned by a digest this registry - // assigned rather than by a tag somebody else can move. + // Mirrored, not built: copied under a name of the mesh's own, so what a machine fetches is + // pinned by a digest this registry assigned rather than by a tag somebody else can move. + // + // **Between registries, never through this machine's image store** (novox/hq + // 04-ISSUES/046, ADR 0096). A published image is an index over several architectures; + // pulled, the store keeps the index and refuses to push one platform out of it, and + // every variant of pull-then-push failed the same way. A copy moves what is there. + if mirror, can := publish.(Mirrorer); can { + say("mirror", "copying %s into the mesh's registry", a.From) + reference, err := mirror.MirrorImage(ctx, a.From, module+"/"+a.Name) + if err != nil { + return catalogue.Built{}, fmt.Errorf("%s: %w", module, err) + } + return catalogue.Built{Name: a.Name, Kind: a.Kind, Reference: reference}, nil + } + // Genesis has no registry to copy into: the image stays in this machine's store, named by + // its own id, as every artifact does before there is anywhere to publish. say("mirror", "pulling %s", a.From) if _, err := run(ctx, tree, "docker", "pull", a.From); err != nil { return catalogue.Built{}, fmt.Errorf("%s: cannot fetch %s: %w", module, a.From, err) } - say("mirror", "publishing under the mesh's own name") reference, err := publish.PublishImage(ctx, a.From, module+"/"+a.Name) if err != nil { return catalogue.Built{}, err diff --git a/internal/catalogue/declaration.go b/internal/catalogue/declaration.go index 7aae727..41910d9 100644 --- a/internal/catalogue/declaration.go +++ b/internal/catalogue/declaration.go @@ -347,9 +347,11 @@ func (r Resolution) Declaration(with Rendering) ([]map[string]any, error) { continue } first = append(first, map[string]any{ - "id": GrantID(to, g.Consumer+"."+g.From), + // One file per holder — the consumer's module with its local name after it + // where it keeps several (ADR 0094); the lab found two files with one id. + "id": GrantID(to, g.Consumer+"."+holderAs(g.From, g.Local)), "type": "file", - "path": grantPath(m.Grants[to], g.Consumer, g.From), + "path": grantPath(m.Grants[to], g.Consumer, holderAs(g.From, g.Local)), "sealed": g.Sealed, }) } diff --git a/internal/catalogue/several_secrets_test.go b/internal/catalogue/several_secrets_test.go index 11872f4..60a0be4 100644 --- a/internal/catalogue/several_secrets_test.go +++ b/internal/catalogue/several_secrets_test.go @@ -134,3 +134,28 @@ func TestAProviderSeesEachLocalNameAsAHolderOfItsOwn(t *testing.T) { t.Fatalf("two holders share one file on the provider: %q", given[0].Secret) } } + +// And on the provider's machine, two files with two ids — the lab's first run had the declaration +// refused for two resources with one identity. +func TestAProviderKeepsOneFilePerHolder(t *testing.T) { + got, err := Resolve(vaultAndCA(), []string{"mesh-vault", "cache", "ca"}, workstation(), World{}) + if err != nil { + t.Fatal(err) + } + out, err := got.Declaration(Rendering{Grants: []Grant{ + {Provision: "secret", Consumer: "workstation", From: "ca", Local: "root-key", Sealed: "x"}, + {Provision: "secret", Consumer: "workstation", From: "ca", Local: "root-pass", Sealed: "y"}, + }}) + if err != nil { + t.Fatal(err) + } + ids := map[string]string{} + for _, r := range out { + if id, _ := r["id"].(string); strings.Contains(id, "grant-secret") { + ids[id] = r["path"].(string) + } + } + if len(ids) != 2 { + t.Fatalf("two holders are two grant files: %v", ids) + } +} From 412599de9a5bd848b415ff9635b0bda8639f9073 Mon Sep 17 00:00:00 2001 From: jochen Date: Mon, 21 Sep 2026 20:42:30 +0200 Subject: [PATCH 6/9] An upstream image is copied between registries, never through a machine's image store A published image is an index over several architectures; pulled, the runtime's store keeps the index and refuses to push one platform out of it. The builder now reads the index and every manifest it names over the registry API, with the anonymous bearer token the public hub hands out, moves each blob by digest into the mesh's registry, puts the manifests and then the index under the module's repository, and pins the index. Genesis, with no registry to copy into, keeps the pull (novox/hq 04-ISSUES/046, ADR 0096). --- internal/builder/mirror.go | 318 ++++++++++++++++++++++++++++++++ internal/builder/mirror_test.go | 210 +++++++++++++++++++++ 2 files changed, 528 insertions(+) create mode 100644 internal/builder/mirror.go create mode 100644 internal/builder/mirror_test.go diff --git a/internal/builder/mirror.go b/internal/builder/mirror.go new file mode 100644 index 0000000..2f6b675 --- /dev/null +++ b/internal/builder/mirror.go @@ -0,0 +1,318 @@ +package builder + +import ( + "bytes" + "context" + "crypto/sha256" + "encoding/hex" + "encoding/json" + "fmt" + "io" + "net/http" + "strings" +) + +// An upstream image is copied between registries, never through a machine's image store +// (novox/hq 04-ISSUES/046, ADR 0096). +// +// A published image is ordinarily an index over several architectures. Pulling it leaves the index +// in the runtime's store, and pushing one platform out of that store is what the runtime refuses — +// every variant of pull-then-push was tried and failed the same way. A copy never needs a platform: +// it moves what is there. Read the index, read each manifest it names, put each blob by digest, +// put the manifests, put the index under the module's repository — the registry API is enough, and +// the runtime's image store is never involved. + +// Mirrorer copies an upstream image into the mesh's own registry, whole. +type Mirrorer interface { + MirrorImage(ctx context.Context, from, repository string) (string, error) +} + +const ( + mediaIndexOCI = "application/vnd.oci.image.index.v1+json" + mediaIndexDocker = "application/vnd.docker.distribution.manifest.list.v2+json" + mediaManifestOCI = "application/vnd.oci.image.manifest.v1+json" + mediaManifestDocker = "application/vnd.docker.distribution.manifest.v2+json" +) + +var manifestAccept = strings.Join([]string{mediaIndexOCI, mediaIndexDocker, mediaManifestOCI, mediaManifestDocker}, ", ") + +// upstream is where an image lives, as the registry API addresses it. +type upstream struct { + // base is the scheme and host, e.g. https://registry-1.docker.io. + base string + // repository is the path under /v2/, e.g. library/alpine. + repository string + // reference is a tag or a digest. + reference string +} + +// parseReference splits `[host/]repo[:tag][@digest]` the way a container runtime does: no host +// means the public hub, and a single-segment repository there lives under `library/`. +func parseReference(ref string) (upstream, error) { + name, reference := ref, "latest" + if at := strings.Index(ref, "@"); at >= 0 { + name, reference = ref[:at], ref[at+1:] + } else if colon := strings.LastIndex(ref, ":"); colon > strings.LastIndex(ref, "/") { + name, reference = ref[:colon], ref[colon+1:] + } + if name == "" || reference == "" { + return upstream{}, fmt.Errorf("%q is not an image reference", ref) + } + host, repository := "docker.io", name + if slash := strings.Index(name, "/"); slash >= 0 && strings.ContainsAny(name[:slash], ".:") { + host, repository = name[:slash], name[slash+1:] + } else if slash >= 0 && name[:slash] == "localhost" { + host, repository = name[:slash], name[slash+1:] + } + if host == "docker.io" { + host = "registry-1.docker.io" + if !strings.Contains(repository, "/") { + repository = "library/" + repository + } + } + scheme := "https://" + if strings.HasPrefix(host, "localhost") || strings.HasPrefix(host, "127.") { + scheme = "http://" + } + return upstream{base: scheme + host, repository: repository, reference: reference}, nil +} + +// source reads from one upstream registry, taking a bearer token where the registry asks for one. +type source struct { + client *http.Client + token string +} + +// get fetches a registry URL, answering a bearer challenge once with an anonymous token — which is +// how the public hub serves public images, and every registry the catalogue names does the same. +func (s *source) get(ctx context.Context, url, accept string) (*http.Response, error) { + for attempt := 0; attempt < 2; attempt++ { + request, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil) + if err != nil { + return nil, err + } + if accept != "" { + request.Header.Set("Accept", accept) + } + if s.token != "" { + request.Header.Set("Authorization", "Bearer "+s.token) + } + response, err := s.client.Do(request) + if err != nil { + return nil, err + } + if response.StatusCode != http.StatusUnauthorized || attempt == 1 { + return response, nil + } + challenge := response.Header.Get("WWW-Authenticate") + response.Body.Close() + token, err := s.tokenFor(ctx, challenge) + if err != nil { + return nil, err + } + s.token = token + } + return nil, fmt.Errorf("unreachable") +} + +// tokenFor answers `Bearer realm="…",service="…",scope="…"` with an anonymous token request. +func (s *source) tokenFor(ctx context.Context, challenge string) (string, error) { + if !strings.HasPrefix(challenge, "Bearer ") { + return "", fmt.Errorf("the registry asks for %q, and this copies public images anonymously", challenge) + } + fields := map[string]string{} + for _, part := range strings.Split(challenge[len("Bearer "):], ",") { + key, value, found := strings.Cut(strings.TrimSpace(part), "=") + if found { + fields[key] = strings.Trim(value, `"`) + } + } + realm := fields["realm"] + if realm == "" { + return "", fmt.Errorf("the registry's challenge names no realm: %q", challenge) + } + url := realm + "?service=" + fields["service"] + if scope := fields["scope"]; scope != "" { + url += "&scope=" + scope + } + request, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil) + if err != nil { + return "", err + } + response, err := s.client.Do(request) + if err != nil { + return "", fmt.Errorf("cannot get a token from %s: %w", realm, err) + } + defer response.Body.Close() + var issued struct { + Token string `json:"token"` + AccessToken string `json:"access_token"` + } + if err := json.NewDecoder(response.Body).Decode(&issued); err != nil { + return "", fmt.Errorf("%s answered with something that is not a token: %w", realm, err) + } + if issued.Token == "" { + issued.Token = issued.AccessToken + } + if issued.Token == "" { + return "", fmt.Errorf("%s issued no token", realm) + } + return issued.Token, nil +} + +// descriptor is what an index or a manifest names: a blob or another manifest, by digest. +type descriptor struct { + MediaType string `json:"mediaType"` + Digest string `json:"digest"` + Size int64 `json:"size"` +} + +// MirrorImage copies `from` — an index or a single manifest, by tag or digest — into this registry +// under `repository`, and returns the reference the mesh will pin: this registry, the repository, +// and the digest of the document that was put last, which is the index where there is one. +func (r Registry) MirrorImage(ctx context.Context, from, repository string) (string, error) { + where, err := parseReference(from) + if err != nil { + return "", err + } + src := &source{client: r.client()} + digest, err := r.copyManifest(ctx, src, where, where.reference, repository) + if err != nil { + return "", fmt.Errorf("copying %s into %s/%s: %w", from, r.Address, repository, err) + } + return r.Address + "/" + repository + "@" + digest, nil +} + +// copyManifest copies one manifest document and everything it names, and returns its digest. An +// index is copied by copying each manifest it names first, so the index never points at something +// the registry does not hold yet. +func (r Registry) copyManifest(ctx context.Context, src *source, where upstream, reference, repository string) (string, error) { + response, err := src.get(ctx, where.base+"/v2/"+where.repository+"/manifests/"+reference, manifestAccept) + if err != nil { + return "", err + } + defer response.Body.Close() + if response.StatusCode != http.StatusOK { + said, _ := io.ReadAll(io.LimitReader(response.Body, 2048)) + return "", fmt.Errorf("%s/%s@%s: %s %s", where.base, where.repository, reference, response.Status, strings.TrimSpace(string(said))) + } + body, err := io.ReadAll(response.Body) + if err != nil { + return "", err + } + mediaType := response.Header.Get("Content-Type") + if semi := strings.Index(mediaType, ";"); semi >= 0 { + mediaType = mediaType[:semi] + } + var document struct { + MediaType string `json:"mediaType"` + Manifests []descriptor `json:"manifests"` + Config *descriptor `json:"config"` + Layers []descriptor `json:"layers"` + } + if err := json.Unmarshal(body, &document); err != nil { + return "", fmt.Errorf("%s is not a manifest: %w", reference, err) + } + if mediaType == "" || mediaType == "application/json" { + mediaType = document.MediaType + } + + switch mediaType { + case mediaIndexOCI, mediaIndexDocker: + // The manifests first, each by its digest; the index that names them last. + for _, m := range document.Manifests { + if _, err := r.copyManifest(ctx, src, where, m.Digest, repository); err != nil { + return "", err + } + } + case mediaManifestOCI, mediaManifestDocker: + blobs := append([]descriptor{}, document.Layers...) + if document.Config != nil { + blobs = append(blobs, *document.Config) + } + for _, b := range blobs { + if err := r.copyBlob(ctx, src, where, b.Digest, repository); err != nil { + return "", err + } + } + default: + return "", fmt.Errorf("%s is a %q, which is neither an image index nor an image manifest", reference, mediaType) + } + + digest := "sha256:" + hexOf(sha256.Sum256(body)) + put, err := http.NewRequestWithContext(ctx, http.MethodPut, + "http://"+r.Address+"/v2/"+repository+"/manifests/"+digest, bytes.NewReader(body)) + if err != nil { + return "", err + } + put.Header.Set("Content-Type", mediaType) + done, err := r.client().Do(put) + if err != nil { + return "", fmt.Errorf("cannot put a manifest into %s: %w", r.Address, err) + } + defer done.Body.Close() + if done.StatusCode != http.StatusCreated { + said, _ := io.ReadAll(io.LimitReader(done.Body, 2048)) + return "", fmt.Errorf("%s refused the manifest %s: %s %s", r.Address, digest, done.Status, strings.TrimSpace(string(said))) + } + return digest, nil +} + +// copyBlob moves one blob by digest, unless the registry already holds it — blobs are immutable +// and content-named, so "already there" is the whole check. +func (r Registry) copyBlob(ctx context.Context, src *source, where upstream, digest, repository string) error { + base := "http://" + r.Address + "/v2/" + repository + if there, err := r.has(ctx, base+"/blobs/"+digest); err != nil { + return err + } else if there { + return nil + } + response, err := src.get(ctx, where.base+"/v2/"+where.repository+"/blobs/"+digest, "") + if err != nil { + return err + } + defer response.Body.Close() + if response.StatusCode != http.StatusOK { + return fmt.Errorf("%s/%s: blob %s: %s", where.base, where.repository, digest, response.Status) + } + + start, err := http.NewRequestWithContext(ctx, http.MethodPost, base+"/blobs/uploads/", nil) + if err != nil { + return err + } + begun, err := r.client().Do(start) + if err != nil { + return fmt.Errorf("cannot start an upload to %s: %w", base, err) + } + begun.Body.Close() + if begun.StatusCode != http.StatusAccepted { + return fmt.Errorf("%s answered %s when asked where to put a blob", base, begun.Status) + } + location := begun.Header.Get("Location") + if location == "" { + return fmt.Errorf("%s accepted an upload and said nowhere to put it", base) + } + if strings.HasPrefix(location, "/") { + location = "http://" + r.Address + location + } + put, err := http.NewRequestWithContext(ctx, http.MethodPut, location+separator(location)+"digest="+digest, response.Body) + if err != nil { + return err + } + put.Header.Set("Content-Type", "application/octet-stream") + if response.ContentLength > 0 { + put.ContentLength = response.ContentLength + } + done, err := r.client().Do(put) + if err != nil { + return fmt.Errorf("cannot upload blob %s: %w", digest, err) + } + defer done.Body.Close() + if done.StatusCode != http.StatusCreated { + said, _ := io.ReadAll(io.LimitReader(done.Body, 2048)) + return fmt.Errorf("%s refused blob %s: %s %s", base, digest, done.Status, strings.TrimSpace(string(said))) + } + return nil +} + +func hexOf(sum [32]byte) string { return hex.EncodeToString(sum[:]) } diff --git a/internal/builder/mirror_test.go b/internal/builder/mirror_test.go new file mode 100644 index 0000000..38fe594 --- /dev/null +++ b/internal/builder/mirror_test.go @@ -0,0 +1,210 @@ +package builder + +import ( + "context" + "crypto/sha256" + "encoding/hex" + "encoding/json" + "net/http" + "net/http/httptest" + "strings" + "sync" + "testing" +) + +// An upstream image is copied between registries, never through a machine's image store +// (novox/hq 04-ISSUES/046, ADR 0096): the index, every manifest it names, every blob — moved by +// digest, and the index put last under the module's repository. + +func digestOf(b []byte) string { + sum := sha256.Sum256(b) + return "sha256:" + hex.EncodeToString(sum[:]) +} + +// anUpstreamRegistry serves one image as an index over two platforms, behind an anonymous bearer +// challenge the way the public hub does, and records what was fetched. +func anUpstreamRegistry(t *testing.T) (*httptest.Server, string, map[string][]byte) { + t.Helper() + blobs := map[string][]byte{} + manifests := map[string][]byte{} + put := func(kind, mediaType string, layer []byte) string { + config := []byte(`{"architecture":"` + kind + `"}`) + blobs[digestOf(config)] = config + blobs[digestOf(layer)] = layer + m, _ := json.Marshal(map[string]any{ + "schemaVersion": 2, "mediaType": mediaType, + "config": map[string]any{"mediaType": "application/vnd.oci.image.config.v1+json", "digest": digestOf(config), "size": len(config)}, + "layers": []map[string]any{{"mediaType": "application/vnd.oci.image.layer.v1.tar+gzip", "digest": digestOf(layer), "size": len(layer)}}, + }) + manifests[digestOf(m)] = m + return digestOf(m) + } + amd := put("amd64", mediaManifestOCI, []byte("amd64 layer bytes")) + arm := put("arm64", mediaManifestOCI, []byte("arm64 layer bytes")) + index, _ := json.Marshal(map[string]any{ + "schemaVersion": 2, "mediaType": mediaIndexOCI, + "manifests": []map[string]any{ + {"mediaType": mediaManifestOCI, "digest": amd, "size": len(manifests[amd]), "platform": map[string]string{"os": "linux", "architecture": "amd64"}}, + {"mediaType": mediaManifestOCI, "digest": arm, "size": len(manifests[arm]), "platform": map[string]string{"os": "linux", "architecture": "arm64"}}, + }, + }) + manifests["latest"] = index + manifests[digestOf(index)] = index + + var server *httptest.Server + server = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path == "/token" { + _, _ = w.Write([]byte(`{"token":"anonymous-token"}`)) + return + } + if r.Header.Get("Authorization") != "Bearer anonymous-token" { + w.Header().Set("WWW-Authenticate", `Bearer realm="`+server.URL+`/token",service="test",scope="repository:library/thing:pull"`) + w.WriteHeader(http.StatusUnauthorized) + return + } + switch { + case strings.HasPrefix(r.URL.Path, "/v2/library/thing/manifests/"): + ref := strings.TrimPrefix(r.URL.Path, "/v2/library/thing/manifests/") + body, ok := manifests[ref] + if !ok { + w.WriteHeader(http.StatusNotFound) + return + } + var typed struct { + MediaType string `json:"mediaType"` + } + _ = json.Unmarshal(body, &typed) + w.Header().Set("Content-Type", typed.MediaType) + _, _ = w.Write(body) + case strings.HasPrefix(r.URL.Path, "/v2/library/thing/blobs/"): + body, ok := blobs[strings.TrimPrefix(r.URL.Path, "/v2/library/thing/blobs/")] + if !ok { + w.WriteHeader(http.StatusNotFound) + return + } + _, _ = w.Write(body) + default: + w.WriteHeader(http.StatusNotFound) + } + })) + return server, digestOf(index), blobs +} + +// theMeshsRegistry accepts blobs and manifests the way a registry does, and remembers them. +type theMeshsRegistry struct { + mu sync.Mutex + blobs map[string][]byte + manifests map[string][]byte + uploads int +} + +func (m *theMeshsRegistry) handler() http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + m.mu.Lock() + defer m.mu.Unlock() + switch { + case r.Method == http.MethodHead && strings.Contains(r.URL.Path, "/blobs/"): + if _, ok := m.blobs[r.URL.Path[strings.LastIndex(r.URL.Path, "/")+1:]]; ok { + w.WriteHeader(http.StatusOK) + } else { + w.WriteHeader(http.StatusNotFound) + } + case r.Method == http.MethodPost && strings.HasSuffix(r.URL.Path, "/blobs/uploads/"): + w.Header().Set("Location", strings.TrimSuffix(r.URL.Path, "/")+"/one") + w.WriteHeader(http.StatusAccepted) + case r.Method == http.MethodPut && strings.Contains(r.URL.Path, "/blobs/uploads/"): + body, _ := readAll(r) + digest := r.URL.Query().Get("digest") + if digestOf(body) != digest { + w.WriteHeader(http.StatusBadRequest) + return + } + m.blobs[digest] = body + m.uploads++ + w.WriteHeader(http.StatusCreated) + case r.Method == http.MethodPut && strings.Contains(r.URL.Path, "/manifests/"): + body, _ := readAll(r) + m.manifests[r.URL.Path[strings.LastIndex(r.URL.Path, "/")+1:]] = body + w.WriteHeader(http.StatusCreated) + default: + w.WriteHeader(http.StatusNotFound) + } + }) +} + +func readAll(r *http.Request) ([]byte, error) { + var buf strings.Builder + b := make([]byte, 4096) + for { + n, err := r.Body.Read(b) + buf.Write(b[:n]) + if err != nil { + break + } + } + return []byte(buf.String()), nil +} + +func TestAnUpstreamIndexIsCopiedWholeIntoTheMeshsRegistry(t *testing.T) { + src, indexDigest, srcBlobs := anUpstreamRegistry(t) + defer src.Close() + dst := &theMeshsRegistry{blobs: map[string][]byte{}, manifests: map[string][]byte{}} + dstServer := httptest.NewServer(dst.handler()) + defer dstServer.Close() + + address := strings.TrimPrefix(dstServer.URL, "http://") + r := Registry{Address: address, HTTP: src.Client()} + from := strings.TrimPrefix(src.URL, "http://") + "/library/thing:latest" + reference, err := r.MirrorImage(context.Background(), from, "hello-web/server") + if err != nil { + t.Fatal(err) + } + // Pinned by the INDEX's digest under the module's own repository: what a machine fetches is + // the whole image, whatever its architecture. + if reference != address+"/hello-web/server@"+indexDigest { + t.Fatalf("pinned as %q, not the index under the module's repository", reference) + } + // Every blob of both platforms, moved by digest, and each only once. + if len(dst.blobs) != len(srcBlobs) || dst.uploads != len(srcBlobs) { + t.Fatalf("%d of %d blobs arrived in %d uploads", len(dst.blobs), len(srcBlobs), dst.uploads) + } + for digest, body := range srcBlobs { + if string(dst.blobs[digest]) != string(body) { + t.Fatalf("blob %s did not arrive intact", digest) + } + } + // Two manifests and the index, each under its digest. + if len(dst.manifests) != 3 { + t.Fatalf("expected two manifests and an index, got %d: %v", len(dst.manifests), dst.manifests) + } + if _, ok := dst.manifests[indexDigest]; !ok { + t.Fatal("the index was not put under its digest") + } + + // Copied again, nothing is uploaded twice: blobs are content-named and already there. + if _, err := r.MirrorImage(context.Background(), from, "hello-web/server"); err != nil { + t.Fatal(err) + } + if dst.uploads != len(srcBlobs) { + t.Fatalf("a second copy uploaded blobs the registry already held: %d uploads", dst.uploads) + } +} + +func TestAReferenceIsReadTheWayARuntimeReadsIt(t *testing.T) { + for ref, want := range map[string]upstream{ + "alpine": {base: "https://registry-1.docker.io", repository: "library/alpine", reference: "latest"}, + "alpine@sha256:abc": {base: "https://registry-1.docker.io", repository: "library/alpine", reference: "sha256:abc"}, + "minio/minio:RELEASE.2025": {base: "https://registry-1.docker.io", repository: "minio/minio", reference: "RELEASE.2025"}, + "quay.io/minio/mc@sha256:def": {base: "https://quay.io", repository: "minio/mc", reference: "sha256:def"}, + "lscr.io/linuxserver/sonarr:4": {base: "https://lscr.io", repository: "linuxserver/sonarr", reference: "4"}, + "localhost:5000/x/y:1": {base: "http://localhost:5000", repository: "x/y", reference: "1"}, + } { + got, err := parseReference(ref) + if err != nil { + t.Fatalf("%s: %v", ref, err) + } + if got != want { + t.Errorf("%s: got %+v want %+v", ref, got, want) + } + } +} From 6f6e1244d457a7a6f589c56d6c19255ba55142a5 Mon Sep 17 00:00:00 2001 From: jochen Date: Mon, 21 Sep 2026 20:45:36 +0200 Subject: [PATCH 7/9] A build declares the vendor image it stands on, and a recipe fetches nothing undeclared MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit build.on takes {arg, image@sha256:…} beside {arg, module, artifact}: the image is copied into the mesh's registry before the build (ADR 0096) and the recipe reads the copy from the argument. A FROM or COPY --from naming a registry image the manifest did not declare is refused before the build, naming it and the remedy; stages, declared arguments and scratch are not fetches (novox/hq 04-ISSUES/064, ADR 0097). --- internal/builder/builder.go | 129 ++++++++++++++++++++++++++- internal/builder/standing_on_test.go | 69 +++++++++++++- internal/catalogue/manifest.go | 12 ++- 3 files changed, 201 insertions(+), 9 deletions(-) diff --git a/internal/builder/builder.go b/internal/builder/builder.go index 8dc0dcc..640fbf3 100644 --- a/internal/builder/builder.go +++ b/internal/builder/builder.go @@ -14,6 +14,7 @@ import ( "path/filepath" "regexp" "sort" + "strconv" "strings" "time" @@ -149,7 +150,20 @@ func Build(ctx context.Context, run Runner, publish Publisher, // What this module said it stands on, answered with what this mesh actually holds. Done // before anything is built, so a missing base is refused in front of the person who can // fix it rather than inside a build that stops on its own first line. - args, err := standingOn(manifest, held) + // An image published elsewhere that the build stands on is copied into the mesh's own + // registry first, like an upstream artifact (ADR 0096), and the recipe is handed the copy. + // Genesis has nowhere to copy to and pulls it into this machine's store instead. + mirror := func(ctx context.Context, from, repository string) (string, error) { + if m, can := publish.(Mirrorer); can { + say("bases", "copying %s into the mesh's registry", from) + return m.MirrorImage(ctx, from, repository) + } + if _, err := run(ctx, tree, "docker", "pull", from); err != nil { + return "", fmt.Errorf("cannot fetch %s: %w", from, err) + } + return from, nil + } + args, err := standingOn(ctx, manifest, held, mirror) if err != nil { say("bases", "UNMET: %v", err) return Result{}, err @@ -358,6 +372,29 @@ func one(ctx context.Context, run Runner, publish Publisher, local := fmt.Sprintf("%s-%s:%s", module, a.Name, short(commit)) // The bases this module named, resolved to what this mesh holds. A recipe reads them as // build arguments, so a module says which module it stands on and never which copy. + // **A recipe fetches nothing the manifest did not declare** (novox/hq 04-ISSUES/064). A FROM + // or a COPY --from naming a registry image that is not a declared base is a build that + // reaches a public registry on its own — and works when that registry answers, which is + // sometimes. Refused here, in front of the person who can declare it, not inside a build + // that fails with "pull access denied" for a reason that is not the mesh's. + recipe, err := os.ReadFile(filepath.Join(tree, a.From)) + if err != nil { + return catalogue.Built{}, fmt.Errorf("%s: cannot read the recipe %s: %w", module, a.From, err) + } + declared := map[string]bool{} + for i := 0; i+1 < len(args); i += 2 { + if args[i] == "--build-arg" { + declared[strings.SplitN(args[i+1], "=", 2)[0]] = true + } + } + if fetches := undeclaredFetches(string(recipe), declared); len(fetches) > 0 { + return catalogue.Built{}, fmt.Errorf( + "%s: the recipe %s fetches %s, which the manifest does not declare. A build "+ + "reaching a public registry on its own works only when that registry answers; "+ + "declare it under build.on as {\"arg\": \"\", \"image\": \"@sha256:…\"} "+ + "and read it from that argument (novox/hq ADR 0097)", + module, a.From, strings.Join(fetches, ", ")) + } invocation := append([]string{"build", "-f", a.From, "-t", local}, args...) if a.Target != "" { invocation = append(invocation, "--target", a.Target) @@ -578,7 +615,8 @@ var _ io.Writer = (*stringWriter)(nil) // one a container runtime produces when a recipe's first line refers to an image nobody has. // // The order is fixed so two builds of one commit invoke the same command. -func standingOn(manifest catalogue.Manifest, held map[string]string) ([]string, error) { +func standingOn(ctx context.Context, manifest catalogue.Manifest, held map[string]string, + mirror func(ctx context.Context, from, repository string) (string, error)) ([]string, error) { if manifest.Build == nil || len(manifest.Build.On) == 0 { return nil, nil } @@ -587,6 +625,27 @@ func standingOn(manifest catalogue.Manifest, held map[string]string) ([]string, var args []string for _, base := range on { + if base.Image != "" { + // A vendor's image, declared (novox/hq 04-ISSUES/064, ADR 0097). Pinned, because a tag + // is what somebody else can move; copied into the mesh's registry, because a build + // that reaches a public registry on its own is a build that works sometimes. + if base.Arg == "" || base.Module != "" || base.Artifact != "" { + return nil, fmt.Errorf( + "%s stands on the image %s, and a base is either a module's artifact or an "+ + "image — never both — read from one build argument", manifest.Module, base.Image) + } + if !strings.Contains(base.Image, "@sha256:") { + return nil, fmt.Errorf( + "%s stands on the image %q, which is not pinned by digest. A tag is what "+ + "somebody else can move; name it as @sha256:…", manifest.Module, base.Image) + } + reference, err := mirror(ctx, base.Image, manifest.Module+"/on-"+strings.ToLower(base.Arg)) + if err != nil { + return nil, fmt.Errorf("%s stands on %s: %w", manifest.Module, base.Image, err) + } + args = append(args, "--build-arg", base.Arg+"="+reference) + continue + } if base.Arg == "" || base.Module == "" || base.Artifact == "" { return nil, fmt.Errorf( "%s says its build stands on something, and does not say all of what: a base "+ @@ -727,3 +786,69 @@ func sourcesFor(entrypoints []string, out string) []string { func timeNow() time.Time { return time.Now() } func since(t time.Time) string { return time.Since(t).Round(time.Millisecond).String() } + +// undeclaredFetches is every image a recipe reaches for that is neither a declared build argument +// nor one of its own stages nor `scratch`: a `FROM` or a `COPY --from` naming somebody else's +// registry directly. +func undeclaredFetches(recipe string, declared map[string]bool) []string { + stages := map[string]bool{} + var out []string + seen := map[string]bool{} + note := func(ref string) { + ref = strings.TrimSpace(ref) + switch { + case ref == "" || ref == "scratch" || stages[strings.ToLower(ref)]: + return + case strings.HasPrefix(ref, "$"): + name := strings.Trim(strings.TrimPrefix(ref, "$"), "{}") + if cut := strings.IndexAny(name, ":-"); cut >= 0 { + name = name[:cut] + } + if !declared[name] { + if !seen[ref] { + seen[ref] = true + out = append(out, ref+" (a build argument the manifest does not declare)") + } + } + return + } + // A stage referenced by number (COPY --from=0) is its own recipe's. + if _, err := strconv.Atoi(ref); err == nil { + return + } + if !seen[ref] { + seen[ref] = true + out = append(out, ref) + } + } + for _, raw := range strings.Split(recipe, "\n") { + line := strings.TrimSpace(raw) + if line == "" || strings.HasPrefix(line, "#") { + continue + } + fields := strings.Fields(line) + switch strings.ToUpper(fields[0]) { + case "FROM": + // FROM [--platform=…] [AS ] + var ref string + for i := 1; i < len(fields); i++ { + if strings.HasPrefix(fields[i], "--") { + continue + } + ref = fields[i] + if i+2 < len(fields) && strings.EqualFold(fields[i+1], "AS") { + stages[strings.ToLower(fields[i+2])] = true + } + break + } + note(ref) + case "COPY", "ADD": + for _, f := range fields[1:] { + if strings.HasPrefix(f, "--from=") { + note(strings.TrimPrefix(f, "--from=")) + } + } + } + } + return out +} diff --git a/internal/builder/standing_on_test.go b/internal/builder/standing_on_test.go index 5cee3cd..d3622f9 100644 --- a/internal/builder/standing_on_test.go +++ b/internal/builder/standing_on_test.go @@ -1,6 +1,8 @@ package builder import ( + "context" + "fmt" "strings" "testing" @@ -20,7 +22,7 @@ func TestABaseTheMeshHasNotBuiltIsRefused(t *testing.T) { On: []catalogue.BuildsOn{{Arg: "RUNTIME_BASE", Module: "mesh-tools", Artifact: "runtime"}}, }, } - _, err := standingOn(manifest, map[string]string{}) + _, err := standingOn(context.Background(), manifest, map[string]string{}, noMirror) if err == nil { t.Fatal("a base nothing has built was accepted; the build would have failed on its first line") } @@ -40,7 +42,7 @@ func TestABaseTheMeshHoldsBecomesABuildArgument(t *testing.T) { }, } held := map[string]string{"mesh-tools/runtime": "127.0.0.1:5000/mesh-tools/runtime@sha256:" + strings.Repeat("a", 64)} - args, err := standingOn(manifest, held) + args, err := standingOn(context.Background(), manifest, held, noMirror) if err != nil { t.Fatalf("a base this mesh holds was refused: %v", err) } @@ -52,7 +54,7 @@ func TestABaseTheMeshHoldsBecomesABuildArgument(t *testing.T) { // A module naming no base asks for nothing, which is most modules. func TestAModuleNamingNoBaseAddsNoArguments(t *testing.T) { - args, err := standingOn(catalogue.Manifest{Module: "hello-web", Build: &catalogue.Build{}}, nil) + args, err := standingOn(context.Background(), catalogue.Manifest{Module: "hello-web", Build: &catalogue.Build{}}, nil, noMirror) if err != nil || args != nil { t.Fatalf("a module naming no base produced %v, %v", args, err) } @@ -64,7 +66,66 @@ func TestAnIncompleteBaseIsRefused(t *testing.T) { Module: "postgres", Build: &catalogue.Build{On: []catalogue.BuildsOn{{Module: "mesh-tools", Artifact: "runtime"}}}, } - if _, err := standingOn(manifest, map[string]string{"mesh-tools/runtime": "x"}); err == nil { + if _, err := standingOn(context.Background(), manifest, map[string]string{"mesh-tools/runtime": "x"}, noMirror); err == nil { t.Fatal("a base with no build argument was accepted; nothing would have read it") } } + +// noMirror is a mirror for tests whose bases are all the mesh's own. +func noMirror(context.Context, string, string) (string, error) { + return "", fmt.Errorf("nothing to copy in this test") +} + +// A build may stand on an image published elsewhere, declared and pinned (novox/hq 04-ISSUES/064, +// ADR 0097): it is copied into the mesh's registry first and the recipe is handed the copy. +func TestADeclaredVendorImageIsCopiedInAndHandedToTheRecipe(t *testing.T) { + manifest := catalogue.Manifest{ + Module: "minio", + Build: &catalogue.Build{ + On: []catalogue.BuildsOn{{Arg: "MC_BASE", Image: "quay.io/minio/mc@sha256:" + strings.Repeat("c", 64)}}, + }, + } + var asked []string + args, err := standingOn(context.Background(), manifest, nil, func(_ context.Context, from, repository string) (string, error) { + asked = append(asked, from+" -> "+repository) + return "127.0.0.1:5000/" + repository + "@sha256:" + strings.Repeat("d", 64), nil + }) + if err != nil { + t.Fatal(err) + } + if len(asked) != 1 || asked[0] != "quay.io/minio/mc@sha256:"+strings.Repeat("c", 64)+" -> minio/on-mc_base" { + t.Fatalf("the image was not copied under the module's repository: %v", asked) + } + if strings.Join(args, " ") != "--build-arg MC_BASE=127.0.0.1:5000/minio/on-mc_base@sha256:"+strings.Repeat("d", 64) { + t.Fatalf("the recipe was not handed the copy: %v", args) + } + // Unpinned, it is refused: a tag is what somebody else can move. + manifest.Build.On[0].Image = "quay.io/minio/mc:latest" + if _, err := standingOn(context.Background(), manifest, nil, noMirror); err == nil || !strings.Contains(err.Error(), "not pinned") { + t.Fatalf("an unpinned vendor image was accepted: %v", err) + } +} + +// A recipe reaching for an image the manifest did not declare is named, and its own stages, +// declared arguments and scratch are not. +func TestARecipeFetchingWhatTheManifestDidNotDeclareIsNamed(t *testing.T) { + recipe := ` +ARG RUNTIME_BASE +ARG MC_BASE +FROM ${RUNTIME_BASE} AS build +COPY --from=${MC_BASE} /usr/bin/mc /usr/local/bin/mc +COPY --from=build /out /out +COPY --from=0 /x /x +FROM scratch +COPY --from=vendor/tool:latest /tool /tool +FROM golang:1.25-alpine AS go +` + got := undeclaredFetches(recipe, map[string]bool{"RUNTIME_BASE": true}) + want := []string{"${MC_BASE} (a build argument the manifest does not declare)", "vendor/tool:latest", "golang:1.25-alpine"} + if strings.Join(got, "|") != strings.Join(want, "|") { + t.Fatalf("got %v, want %v", got, want) + } + if got := undeclaredFetches(recipe, map[string]bool{"RUNTIME_BASE": true, "MC_BASE": true}); len(got) != 2 { + t.Fatalf("declared arguments are not fetches: %v", got) + } +} diff --git a/internal/catalogue/manifest.go b/internal/catalogue/manifest.go index 01aaea1..fbf92c6 100644 --- a/internal/catalogue/manifest.go +++ b/internal/catalogue/manifest.go @@ -411,14 +411,20 @@ type Build struct { On []BuildsOn `json:"on,omitempty"` } -// BuildsOn is one base a build needs, and the name the recipe knows it by. +// BuildsOn is one base a build needs, and the name the recipe knows it by: another module's +// artifact, or an image published elsewhere. type BuildsOn struct { // Arg is the build argument the recipe reads it from. Arg string `json:"arg"` // Module is whose artifact it is. - Module string `json:"module"` + Module string `json:"module,omitempty"` // Artifact is which of that module's artifacts, by its own name for it. - Artifact string `json:"artifact"` + Artifact string `json:"artifact,omitempty"` + // Image is an image published elsewhere, pinned by digest, that the build copies out of — a + // vendor's tool, a base nobody in the mesh builds. Declared, the mesh copies it into its own + // registry before the build and hands the recipe the copy (novox/hq 04-ISSUES/064, ADR 0097); + // a recipe fetching from a public registry on its own is refused. + Image string `json:"image,omitempty"` } // Artifact is one thing built from a module's source. From e81f35297938e9a886a204dc9c2be29d1d9145fc Mon Sep 17 00:00:00 2001 From: jochen Date: Mon, 21 Sep 2026 20:48:00 +0200 Subject: [PATCH 8/9] An undeclared COPY --from is refused; an undeclared FROM is said, not yet refused MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The mesh's own images start FROM a public base — the control plane's, the builder's, the tool runtime's — and refusing those refuses genesis. They declare their bases next; until then the base is named every build, with the remedy. --- internal/builder/builder.go | 32 +++++++++++++++++++--------- internal/builder/standing_on_test.go | 16 ++++++++------ 2 files changed, 32 insertions(+), 16 deletions(-) diff --git a/internal/builder/builder.go b/internal/builder/builder.go index 640fbf3..a592c52 100644 --- a/internal/builder/builder.go +++ b/internal/builder/builder.go @@ -387,13 +387,23 @@ func one(ctx context.Context, run Runner, publish Publisher, declared[strings.SplitN(args[i+1], "=", 2)[0]] = true } } - if fetches := undeclaredFetches(string(recipe), declared); len(fetches) > 0 { + bases, copies := undeclaredFetches(string(recipe), declared) + if len(copies) > 0 { return catalogue.Built{}, fmt.Errorf( - "%s: the recipe %s fetches %s, which the manifest does not declare. A build "+ + "%s: the recipe %s copies out of %s, which the manifest does not declare. A build "+ "reaching a public registry on its own works only when that registry answers; "+ "declare it under build.on as {\"arg\": \"\", \"image\": \"@sha256:…\"} "+ "and read it from that argument (novox/hq ADR 0097)", - module, a.From, strings.Join(fetches, ", ")) + module, a.From, strings.Join(copies, ", ")) + } + if len(bases) > 0 { + // Said, not yet refused: the mesh's own images start FROM a public base — the control + // plane's, the builder's, the tool runtime's — and refusing those refuses genesis. + // They declare their bases next; until then a base fetched on its own is named here, + // with the remedy, every build. + say("recipe", "UNDECLARED base(s) %s in %s — declare each under build.on as "+ + "{arg, image@sha256:…} and read it from that argument (novox/hq ADR 0097)", + strings.Join(bases, ", "), a.From) } invocation := append([]string{"build", "-f", a.From, "-t", local}, args...) if a.Target != "" { @@ -788,12 +798,12 @@ func timeNow() time.Time { return time.Now() } func since(t time.Time) string { return time.Since(t).Round(time.Millisecond).String() } // undeclaredFetches is every image a recipe reaches for that is neither a declared build argument -// nor one of its own stages nor `scratch`: a `FROM` or a `COPY --from` naming somebody else's -// registry directly. -func undeclaredFetches(recipe string, declared map[string]bool) []string { +// nor one of its own stages nor `scratch`, in two lists: the bases it starts `FROM`, and the images +// it `COPY --from`s out of — a vendor's tool, the case novox/hq 04-ISSUES/064 is about. +func undeclaredFetches(recipe string, declared map[string]bool) (bases, copies []string) { stages := map[string]bool{} - var out []string seen := map[string]bool{} + var out *[]string note := func(ref string) { ref = strings.TrimSpace(ref) switch { @@ -807,7 +817,7 @@ func undeclaredFetches(recipe string, declared map[string]bool) []string { if !declared[name] { if !seen[ref] { seen[ref] = true - out = append(out, ref+" (a build argument the manifest does not declare)") + *out = append(*out, ref+" (a build argument the manifest does not declare)") } } return @@ -818,7 +828,7 @@ func undeclaredFetches(recipe string, declared map[string]bool) []string { } if !seen[ref] { seen[ref] = true - out = append(out, ref) + *out = append(*out, ref) } } for _, raw := range strings.Split(recipe, "\n") { @@ -830,6 +840,7 @@ func undeclaredFetches(recipe string, declared map[string]bool) []string { switch strings.ToUpper(fields[0]) { case "FROM": // FROM [--platform=…] [AS ] + out = &bases var ref string for i := 1; i < len(fields); i++ { if strings.HasPrefix(fields[i], "--") { @@ -843,6 +854,7 @@ func undeclaredFetches(recipe string, declared map[string]bool) []string { } note(ref) case "COPY", "ADD": + out = &copies for _, f := range fields[1:] { if strings.HasPrefix(f, "--from=") { note(strings.TrimPrefix(f, "--from=")) @@ -850,5 +862,5 @@ func undeclaredFetches(recipe string, declared map[string]bool) []string { } } } - return out + return bases, copies } diff --git a/internal/builder/standing_on_test.go b/internal/builder/standing_on_test.go index d3622f9..ee9cc0c 100644 --- a/internal/builder/standing_on_test.go +++ b/internal/builder/standing_on_test.go @@ -120,12 +120,16 @@ FROM scratch COPY --from=vendor/tool:latest /tool /tool FROM golang:1.25-alpine AS go ` - got := undeclaredFetches(recipe, map[string]bool{"RUNTIME_BASE": true}) - want := []string{"${MC_BASE} (a build argument the manifest does not declare)", "vendor/tool:latest", "golang:1.25-alpine"} - if strings.Join(got, "|") != strings.Join(want, "|") { - t.Fatalf("got %v, want %v", got, want) + bases, copies := undeclaredFetches(recipe, map[string]bool{"RUNTIME_BASE": true}) + if strings.Join(copies, "|") != "${MC_BASE} (a build argument the manifest does not declare)|vendor/tool:latest" { + t.Fatalf("copies out of undeclared images: %v", copies) } - if got := undeclaredFetches(recipe, map[string]bool{"RUNTIME_BASE": true, "MC_BASE": true}); len(got) != 2 { - t.Fatalf("declared arguments are not fetches: %v", got) + // A base fetched on its own is named apart: the mesh's own images still start FROM one, so + // it is said rather than refused until they declare theirs. + if strings.Join(bases, "|") != "golang:1.25-alpine" { + t.Fatalf("undeclared bases: %v", bases) + } + if _, copies := undeclaredFetches(recipe, map[string]bool{"RUNTIME_BASE": true, "MC_BASE": true}); len(copies) != 1 { + t.Fatalf("declared arguments are not fetches: %v", copies) } } From 9f3790dcda1b5b56cc73cc4f2f463e479de627c9 Mon Sep 17 00:00:00 2001 From: jochen Date: Mon, 21 Sep 2026 21:03:22 +0200 Subject: [PATCH 9/9] Review: one local name is still a local name; a local name is unique; recovery knows it; recipes read as instructions; a tag before a digest; ask fails at once when nothing serves A secrets object with one local name delivered no file. Two requirements could share a local name. secret recover and the export could not tell two locals apart. The recipe check missed continued lines and read heredoc bodies as bases. repo:tag@digest kept the tag in the repository. ask now publishes mandatory, so a tool nothing serves is said at once rather than after the wait. --- cmd/mesh-controller/secret.go | 3 +- internal/builder/builder.go | 63 ++++++++++++++++++++-- internal/builder/mirror.go | 5 ++ internal/builder/mirror_test.go | 11 ++++ internal/builder/standing_on_test.go | 19 +++++++ internal/catalogue/declaration.go | 7 ++- internal/catalogue/manifest.go | 11 +++- internal/catalogue/resolve.go | 4 +- internal/catalogue/several_secrets_test.go | 44 +++++++++++++++ internal/inventory/operator.go | 21 ++++---- internal/inventory/operator_test.go | 16 +++--- internal/inventory/secrets_test.go | 42 +++++++++++++++ internal/link/ask.go | 12 ++++- 13 files changed, 229 insertions(+), 29 deletions(-) diff --git a/cmd/mesh-controller/secret.go b/cmd/mesh-controller/secret.go index 3a4105d..eee4a37 100644 --- a/cmd/mesh-controller/secret.go +++ b/cmd/mesh-controller/secret.go @@ -123,6 +123,7 @@ func secretRecover(ctx context.Context, args []string) error { out := set.String("out", "", "where to write the value (0600); - for standard output. Default ...secret") fromExport := set.String("from-export", "", "read the sealed copy from this `secret export` file instead of the store") provider := set.String("provider", "", "for a pair credential held from more than one provider: which one") + local := set.String("local", "", "for a pair credential the module keeps under a local name (ADR 0094): which one") if err := set.Parse(flags); err != nil { return err } @@ -147,7 +148,7 @@ func secretRecover(ctx context.Context, args []string) error { return err } defer open.Close() - kept, err = open.inventory.KeptSecret(ctx, node, module, name, *provider) + kept, err = open.inventory.KeptSecret(ctx, node, module, name, *provider, *local) if err != nil { return err } diff --git a/internal/builder/builder.go b/internal/builder/builder.go index a592c52..ca7c3d6 100644 --- a/internal/builder/builder.go +++ b/internal/builder/builder.go @@ -831,11 +831,7 @@ func undeclaredFetches(recipe string, declared map[string]bool) (bases, copies [ *out = append(*out, ref) } } - for _, raw := range strings.Split(recipe, "\n") { - line := strings.TrimSpace(raw) - if line == "" || strings.HasPrefix(line, "#") { - continue - } + for _, line := range instructions(recipe) { fields := strings.Fields(line) switch strings.ToUpper(fields[0]) { case "FROM": @@ -860,7 +856,64 @@ func undeclaredFetches(recipe string, declared map[string]bool) (bases, copies [ note(strings.TrimPrefix(f, "--from=")) } } + case "RUN": + // RUN --mount=type=bind,from=,… reaches for an image exactly as COPY --from does. + out = &copies + for _, f := range fields[1:] { + if !strings.HasPrefix(f, "--mount=") { + continue + } + for _, opt := range strings.Split(strings.TrimPrefix(f, "--mount="), ",") { + if from, found := strings.CutPrefix(opt, "from="); found { + note(from) + } + } + } } } return bases, copies } + +// instructions is a recipe as its instructions, one per line: continuations joined, comments and +// blank lines dropped, and heredoc bodies (`COPY <= 0 { + // `< 0 { + heredoc = strings.Trim(strings.TrimPrefix(word[0], "-"), `'"`) + } + } + flush() + } + flush() + return out +} diff --git a/internal/builder/mirror.go b/internal/builder/mirror.go index 2f6b675..3a5b4db 100644 --- a/internal/builder/mirror.go +++ b/internal/builder/mirror.go @@ -52,6 +52,11 @@ func parseReference(ref string) (upstream, error) { name, reference := ref, "latest" if at := strings.Index(ref, "@"); at >= 0 { name, reference = ref[:at], ref[at+1:] + // `repo:tag@digest` is what a runtime prints; the digest names the image and the tag is + // only what it was called. The tag is not part of the repository. + if colon := strings.LastIndex(name, ":"); colon > strings.LastIndex(name, "/") { + name = name[:colon] + } } else if colon := strings.LastIndex(ref, ":"); colon > strings.LastIndex(ref, "/") { name, reference = ref[:colon], ref[colon+1:] } diff --git a/internal/builder/mirror_test.go b/internal/builder/mirror_test.go index 38fe594..6a2749c 100644 --- a/internal/builder/mirror_test.go +++ b/internal/builder/mirror_test.go @@ -208,3 +208,14 @@ func TestAReferenceIsReadTheWayARuntimeReadsIt(t *testing.T) { } } } + +// `repo:tag@digest` is what a runtime prints; the tag is not part of the repository (review C6). +func TestATagBeforeTheDigestIsNotPartOfTheRepository(t *testing.T) { + got, err := parseReference("quay.io/minio/mc:RELEASE.2025@sha256:abc") + if err != nil { + t.Fatal(err) + } + if got.repository != "minio/mc" || got.reference != "sha256:abc" { + t.Fatalf("got %+v", got) + } +} diff --git a/internal/builder/standing_on_test.go b/internal/builder/standing_on_test.go index ee9cc0c..61a94ab 100644 --- a/internal/builder/standing_on_test.go +++ b/internal/builder/standing_on_test.go @@ -133,3 +133,22 @@ FROM golang:1.25-alpine AS go t.Fatalf("declared arguments are not fetches: %v", copies) } } + +// Continued lines are one instruction, heredoc bodies are not instructions, and a RUN --mount reaches +// for an image as a COPY --from does (review C4, C5). +func TestARecipeIsReadAsInstructions(t *testing.T) { + recipe := "ARG RUNTIME_BASE\n" + + "FROM ${RUNTIME_BASE} \\\n AS build\n" + + "COPY \\\n --from=docker.io/vendor/one:latest /a /a\n" + + "COPY --from=build /out /out\n" + + "COPY <}` says, so it may not be another requirement's - // name or one of the module's own secrets — the file would hold the wrong credential - // while every check passed. + // name, another requirement's local name, or one of the module's own secrets — the + // file would hold the wrong credential while every check passed. if f.Local != "" { + if other, taken := localOf[f.Local]; taken && other != to { + problems = append(problems, fmt.Sprintf( + "%s keeps credentials for %q and %q both under %q — a local name names one", + m.Module, other, to, f.Local)) + } + localOf[f.Local] = to if _, own := m.OwnSecrets[f.Local]; own { problems = append(problems, fmt.Sprintf( "%s keeps a credential for %q under %q, which is also one of its own secrets", diff --git a/internal/catalogue/resolve.go b/internal/catalogue/resolve.go index 8439f2c..8f25164 100644 --- a/internal/catalogue/resolve.go +++ b/internal/catalogue/resolve.go @@ -881,7 +881,9 @@ func perConsumer(needs []Needed, order []string, catalogue map[string]Manifest) // from one provider (ADR 0094). Each is its own pair credential downstream. func eachLocal(needs []Needed, catalogue map[string]Manifest, n Needed) []Needed { files := catalogue[n.For].SecretFiles(n.Name) - if len(files) <= 1 { + // One file under a local name is still a local name: the review found a module keeping ONE + // named secret given a need with no local, and so no file, while everything reported success. + if len(files) == 0 || (len(files) == 1 && files[0].Local == "") { return append(needs, n) } for _, f := range files { diff --git a/internal/catalogue/several_secrets_test.go b/internal/catalogue/several_secrets_test.go index 60a0be4..e074898 100644 --- a/internal/catalogue/several_secrets_test.go +++ b/internal/catalogue/several_secrets_test.go @@ -159,3 +159,47 @@ func TestAProviderKeepsOneFilePerHolder(t *testing.T) { t.Fatalf("two holders are two grant files: %v", ids) } } + +// One file under a local name is still a local name (review C1): the need carries it, the file +// is written, and ${secret:} is filled. +func TestOneLocalNameIsStillALocalName(t *testing.T) { + only, _ := ParseManifest([]byte(`{"module":"one","version":"1","requires":["secret"], + "secrets":{"secret":{"only":"/var/lib/one/only"}}}`)) + vault := vaultAndCA()["mesh-vault"] + got, err := Resolve(shelf(vault, only), []string{"mesh-vault", "one"}, workstation(), World{}) + if err != nil { + t.Fatal(err) + } + var found *Needed + for i, n := range got.Needs { + if n.For == "one" && n.Name == "secret" { + found = &got.Needs[i] + } + } + if found == nil || found.Local != "only" { + t.Fatalf("the one named file did not become a need under its name: %v", got.Needs) + } + found.Sealed = "sealed-only" + out, err := got.Declaration(Rendering{}) + if err != nil { + t.Fatal(err) + } + var written bool + for _, r := range out { + if r["path"] == "/var/lib/one/only" && r["sealed"] == "sealed-only" { + written = true + } + } + if !written { + t.Fatal("the file under the one local name was not written") + } +} + +// A local name names one credential: two requirements may not share it (review C2). +func TestALocalNameIsUniqueAcrossRequirements(t *testing.T) { + _, err := ParseManifest([]byte(`{"module":"x","version":"1","requires":["secret","postgres-database"], + "secrets":{"secret":{"x":"/var/lib/x/a"},"postgres-database":{"x":"/var/lib/x/b"}}}`)) + if err == nil || !strings.Contains(err.Error(), "both under") { + t.Fatalf("two requirements under one local name were accepted: %v", err) + } +} diff --git a/internal/inventory/operator.go b/internal/inventory/operator.go index 7e6b0b5..52b6723 100644 --- a/internal/inventory/operator.go +++ b/internal/inventory/operator.go @@ -124,20 +124,20 @@ func (i *Inventory) KeptForOperator(ctx context.Context) (kept, earlier, unrecov } rows, err := i.store.Pool().Query(ctx, `select 'own', n.name, s.module, s.name, '', s.origin, coalesce(s.operator_sealed, ''), - coalesce(s.operator_key, ''), s.made_at + coalesce(s.operator_key, ''), s.made_at, '' from module_secret s join node n on n.id = s.node union all - select 'pair', c.name, s.consumer_module, s.name, p.name, 'made', coalesce(s.operator_sealed, ''), - coalesce(s.operator_key, ''), s.created_at + select 'pair', c.name, s.consumer_module, s.name, p.name, s.origin, coalesce(s.operator_sealed, ''), + coalesce(s.operator_key, ''), s.created_at, s.local from secret s join node c on c.id = s.consumer join node p on p.id = s.provider - order by 1, 2, 3, 4`) + order by 1, 2, 3, 4, 10`) if err != nil { return nil, nil, nil, err } defer rows.Close() for rows.Next() { var k Kept - if err := rows.Scan(&k.Kind, &k.Node, &k.Module, &k.Name, &k.Provider, &k.Origin, &k.Sealed, &k.Key, &k.MadeAt); err != nil { + if err := rows.Scan(&k.Kind, &k.Node, &k.Module, &k.Name, &k.Provider, &k.Origin, &k.Sealed, &k.Key, &k.MadeAt, &k.Local); err != nil { return nil, nil, nil, err } switch { @@ -159,7 +159,7 @@ func (i *Inventory) KeptForOperator(ctx context.Context) (kept, earlier, unrecov // credential is keyed by provider as well, and a consumer whose provision moved leaves the old // provider's row behind: two rows is refused with both providers named, never answered with // whichever came first, unless `provider` says which. -func (i *Inventory) KeptSecret(ctx context.Context, node, module, name, provider string) (Kept, error) { +func (i *Inventory) KeptSecret(ctx context.Context, node, module, name, provider, local string) (Kept, error) { var k Kept err := i.store.Pool().QueryRow(ctx, `select 'own', n.name, s.module, s.name, '', s.origin, coalesce(s.operator_sealed, ''), @@ -169,11 +169,12 @@ func (i *Inventory) KeptSecret(ctx context.Context, node, module, name, provider Scan(&k.Kind, &k.Node, &k.Module, &k.Name, &k.Provider, &k.Origin, &k.Sealed, &k.Key, &k.MadeAt) if errors.Is(err, pgx.ErrNoRows) { rows, qerr := i.store.Pool().Query(ctx, - `select 'pair', c.name, s.consumer_module, s.name, p.name, 'made', coalesce(s.operator_sealed, ''), - coalesce(s.operator_key, ''), s.created_at + `select 'pair', c.name, s.consumer_module, s.name, p.name, s.origin, coalesce(s.operator_sealed, ''), + coalesce(s.operator_key, ''), s.created_at, s.local from secret s join node c on c.id = s.consumer join node p on p.id = s.provider where c.name = $1 and s.consumer_module = $2 and s.name = $3 and ($4 = '' or p.name = $4) - order by p.name`, node, module, name, provider) + and s.local = $5 + order by p.name`, node, module, name, provider, local) if qerr != nil { return Kept{}, qerr } @@ -181,7 +182,7 @@ func (i *Inventory) KeptSecret(ctx context.Context, node, module, name, provider var found []Kept for rows.Next() { var row Kept - if err := rows.Scan(&row.Kind, &row.Node, &row.Module, &row.Name, &row.Provider, &row.Origin, &row.Sealed, &row.Key, &row.MadeAt); err != nil { + if err := rows.Scan(&row.Kind, &row.Node, &row.Module, &row.Name, &row.Provider, &row.Origin, &row.Sealed, &row.Key, &row.MadeAt, &row.Local); err != nil { return Kept{}, err } found = append(found, row) diff --git a/internal/inventory/operator_test.go b/internal/inventory/operator_test.go index e4179e9..9cfe052 100644 --- a/internal/inventory/operator_test.go +++ b/internal/inventory/operator_test.go @@ -21,7 +21,7 @@ func TestAnOwnSecretIsSealedToTheOperatorToo(t *testing.T) { if _, err := inv.SecretForModule(ctx, "consumer", "postgres", "superuser"); err != nil { t.Fatal(err) } - if _, err := inv.KeptSecret(ctx, "consumer", "postgres", "superuser", ""); err == nil { + if _, err := inv.KeptSecret(ctx, "consumer", "postgres", "superuser", "", ""); err == nil { t.Fatal("a secret made before the operator key was reported recoverable") } kept, _, unrecoverable, err := inv.KeptForOperator(ctx) @@ -54,7 +54,7 @@ func TestAnOwnSecretIsSealedToTheOperatorToo(t *testing.T) { if len(kept) != 2 || len(unrecoverable) != 1 { t.Fatalf("after a key: %d kept, %d unrecoverable", len(kept), len(unrecoverable)) } - got, err := inv.KeptSecret(ctx, "provider", "postgres", "replication", "") + got, err := inv.KeptSecret(ctx, "provider", "postgres", "replication", "", "") if err != nil { t.Fatal(err) } @@ -68,7 +68,7 @@ func TestAnOwnSecretIsSealedToTheOperatorToo(t *testing.T) { if got.Origin != "accepted" || got.Key != pub { t.Fatalf("kept as %+v", got) } - minted, err := inv.KeptSecret(ctx, "provider", "postgres", "superuser", "") + minted, err := inv.KeptSecret(ctx, "provider", "postgres", "superuser", "", "") if err != nil { t.Fatal(err) } @@ -81,7 +81,7 @@ func TestAnOwnSecretIsSealedToTheOperatorToo(t *testing.T) { if _, err := inv.SecretForModule(ctx, "consumer", "postgres", "superuser"); err != nil { t.Fatal(err) } - if _, err := inv.KeptSecret(ctx, "consumer", "postgres", "superuser", ""); err == nil { + if _, err := inv.KeptSecret(ctx, "consumer", "postgres", "superuser", "", ""); err == nil { t.Fatal("asking again did not remake, yet it became recoverable") } // Until the node rejoins with a new sealing key: then the secret is remade, and the remake is @@ -97,7 +97,7 @@ func TestAnOwnSecretIsSealedToTheOperatorToo(t *testing.T) { if _, err := inv.SecretForModule(ctx, "consumer", "postgres", "superuser"); err != nil { t.Fatal(err) } - remade, err := inv.KeptSecret(ctx, "consumer", "postgres", "superuser", "") + remade, err := inv.KeptSecret(ctx, "consumer", "postgres", "superuser", "", "") if err != nil { t.Fatalf("the remade secret is not recoverable: %v", err) } @@ -168,7 +168,7 @@ func TestAPairCredentialIsSealedToTheOperatorToo(t *testing.T) { if err != nil { t.Fatal(err) } - kept, err := inv.KeptSecret(ctx, "consumer", "gitea", "secret", "") + kept, err := inv.KeptSecret(ctx, "consumer", "gitea", "secret", "", "") if err != nil { t.Fatal(err) } @@ -194,10 +194,10 @@ func TestAPairCredentialIsSealedToTheOperatorToo(t *testing.T) { if _, err := inv.SecretFor(ctx, "secret", "consumer", "gitea", "consumer", ""); err != nil { t.Fatal(err) } - if _, err := inv.KeptSecret(ctx, "consumer", "gitea", "secret", ""); err == nil || !strings.Contains(err.Error(), "--provider") { + if _, err := inv.KeptSecret(ctx, "consumer", "gitea", "secret", "", ""); err == nil || !strings.Contains(err.Error(), "--provider") { t.Fatalf("two providers were not refused: %v", err) } - if byName, err := inv.KeptSecret(ctx, "consumer", "gitea", "secret", "provider"); err != nil || byName.Provider != "provider" { + if byName, err := inv.KeptSecret(ctx, "consumer", "gitea", "secret", "provider", ""); err != nil || byName.Provider != "provider" { t.Fatalf("naming the provider did not select it: %+v %v", byName, err) } pub2, _, _ := secrets.Keypair() diff --git a/internal/inventory/secrets_test.go b/internal/inventory/secrets_test.go index b6e015d..671f381 100644 --- a/internal/inventory/secrets_test.go +++ b/internal/inventory/secrets_test.go @@ -6,6 +6,7 @@ import ( "crypto/rand" "encoding/base64" "github.com/novox/mesh-controller/internal/catalogue" + "github.com/novox/mesh-controller/internal/secrets" "strings" "testing" @@ -673,3 +674,44 @@ func TestTwoLocalNamesAreTwoCredentials(t *testing.T) { t.Fatalf("the provider is told two credentials to create: %+v", from) } } + +// The operator can recover either of two local names apart (review C3). +func TestTheOperatorRecoversEachLocalNameApart(t *testing.T) { + inv, ctx := twoNodesWithKeys(t) + pub, _, err := secrets.Keypair() + if err != nil { + t.Fatal(err) + } + if _, err := inv.SetOperatorKey(ctx, pub); err != nil { + t.Fatal(err) + } + for _, local := range []string{"root-key", "root-pass"} { + if _, err := inv.SecretFor(ctx, "secret", "consumer", "gitea", "provider", local); err != nil { + t.Fatal(err) + } + } + key, err := inv.KeptSecret(ctx, "consumer", "gitea", "secret", "provider", "root-key") + if err != nil { + t.Fatal(err) + } + pass, err := inv.KeptSecret(ctx, "consumer", "gitea", "secret", "provider", "root-pass") + if err != nil { + t.Fatal(err) + } + if key.Local != "root-key" || pass.Local != "root-pass" || key.Kind != "pair" { + t.Fatalf("recovery does not tell the two apart: %+v %+v", key, pass) + } + kept, _, _, err := inv.KeptForOperator(ctx) + if err != nil { + t.Fatal(err) + } + var locals []string + for _, k := range kept { + if k.Kind == "pair" { + locals = append(locals, k.Local) + } + } + if strings.Join(locals, ",") != "root-key,root-pass" { + t.Fatalf("the export does not name the local names: %v", kept) + } +} diff --git a/internal/link/ask.go b/internal/link/ask.go index df29a8b..6d3a19c 100644 --- a/internal/link/ask.go +++ b/internal/link/ask.go @@ -49,9 +49,12 @@ func Ask(ctx context.Context, channel *amqp.Channel, module, tool string, args j return Answer{}, err } + // Mandatory, so a request nothing consumes comes straight back: a module that is down, or a + // tool that does not exist, is said at once rather than after the whole wait. + returned := channel.NotifyReturn(make(chan amqp.Return, 1)) id := fmt.Sprintf("ask-%d", time.Now().UnixNano()) key := module + "." + tool - if err := channel.PublishWithContext(ctx, RPCExchange, key, false, false, amqp.Publishing{ + if err := channel.PublishWithContext(ctx, RPCExchange, key, true, false, amqp.Publishing{ ContentType: "application/json", CorrelationId: id, ReplyTo: replies.Name, @@ -64,6 +67,13 @@ func Ask(ctx context.Context, channel *amqp.Channel, module, tool string, args j defer cancel() for { select { + case back := <-returned: + if back.CorrelationId == id { + return Answer{}, fmt.Errorf( + "nothing serves %s: no runtime has bound %q on the broker. The module is not "+ + "assigned, its runtime is not up, or it serves no such tool — `status` "+ + "says whether the machine carrying it has applied", module, key) + } case <-waiting.Done(): return Answer{}, fmt.Errorf( "%s did not answer within %s. Its runtime serves %q when it is up and has bound "+