From f4bcb320fecbb5c178152387c41942711029e707 Mon Sep 17 00:00:00 2001 From: jochen Date: Thu, 24 Sep 2026 18:36:08 +0200 Subject: [PATCH] contributes: a module may answer one requirement several times MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A module's contributes was map[string]map[string]any — one JSON object key per requirement, structurally exactly one contribution to "route" ever. minio needs two public hostnames (the S3 API and the console), which is two different contributions to route from one module, and nothing let it say so. This is the same shape of problem ADR 0094 solved for secrets (a module needing several values from one provider that gives one per pair): contributes now accepts either the ordinary {label, port} object, or an object of local names to several such objects. Detected per requirement key by what's inside, since (unlike secrets' string-vs-object split) both shapes are JSON objects: an ordinary contribution's fields are scalars, the several-instance shape is local-name -> object. Confirmed against every module.json in mesh-catalog before relying on that split. Both route-proxy and the migration-era route-adapter already key generated routers off the composed hostname (Values["name"]), not the module name, so two contributions with the same From reach them as two independent routes with no changes needed on the receiving side. --- internal/catalogue/declaration.go | 15 +++ internal/catalogue/manifest.go | 118 ++++++++++++++++- .../catalogue/several_contributions_test.go | 124 ++++++++++++++++++ 3 files changed, 250 insertions(+), 7 deletions(-) create mode 100644 internal/catalogue/several_contributions_test.go diff --git a/internal/catalogue/declaration.go b/internal/catalogue/declaration.go index fefd2ff..c0ccc82 100644 --- a/internal/catalogue/declaration.go +++ b/internal/catalogue/declaration.go @@ -914,6 +914,21 @@ func (r Resolution) contributions(settings SettingsBy, grants []Grant, composeName(values, r.PublicDomain) out[to] = append(out[to], Contribution{From: m.Module, Values: values}) } + // Several contributions to one requirement (ADR 0094's sibling for `contributes`): an + // object store's data API and its console are two different public names from one module, + // not one. Never in `granted` — a route names a host, not a credential — so every local + // name always reaches the provider from here. + for _, to := range sortedKeys(m.ContributesMany) { + for _, local := range sortedKeys(m.ContributesMany[to]) { + values, err := settle(m.ContributesMany[to][local], settings[m.Module], nil, + m.Module+" contributing "+local+" to "+to) + if err != nil { + return nil, fmt.Errorf("%s contributing %s to %s: %w", m.Module, local, to, err) + } + composeName(values, r.PublicDomain) + out[to] = append(out[to], Contribution{From: m.Module, Values: values}) + } + } } return out, nil } diff --git a/internal/catalogue/manifest.go b/internal/catalogue/manifest.go index e39fa17..6e42dd9 100644 --- a/internal/catalogue/manifest.go +++ b/internal/catalogue/manifest.go @@ -233,6 +233,18 @@ type Manifest struct { // module that had to say both would eventually say one. Contributes map[string]map[string]any `json:"contributes,omitempty"` + // ContributesMany is the same key, `contributes`, where a module tells one provider several + // things under local names — `"route": {"api": {"label": "files-api", "port": 9000}, "console": + // {"label": "files", "port": 9001}}` — because a module may answer one requirement more than + // once: an object store with a data API and a console are two different public names, not one + // (novox/hq ADR 0094's sibling for `contributes` rather than `secrets` — "a module may need more + // than one value from a provider that gives one per pair" applies exactly as well to what a + // module gives a provider as to what it keeps from one). Each local name is a contribution of + // its own, reaching the provider as its own entry in the file it receives. + // + // Filled from the manifest's `contributes` object by UnmarshalJSON; never written by hand. + ContributesMany map[string]map[string]map[string]any `json:"-"` + // Receives is where this module wants its consumers' contributions written, per requirement // it provides. // @@ -615,16 +627,24 @@ func AccessID(path string) string { return "access-" + strings.TrimPrefix(path, // it contributes to. func (m Manifest) Wants() []string { out := append([]string{}, m.Requires...) - for to := range m.Contributes { - var already bool + add := func(to string) { for _, r := range m.Requires { if r == to { - already = true + return } } - if !already { - out = append(out, to) + for _, already := range out { + if already == to { + return + } } + out = append(out, to) + } + for to := range m.Contributes { + add(to) + } + for to := range m.ContributesMany { + add(to) } sort.Strings(out) return out @@ -679,6 +699,47 @@ func (m *Manifest) UnmarshalJSON(raw []byte) error { } delete(keys, "secrets") } + contributesPlain := map[string]map[string]any{} + contributesMany := map[string]map[string]map[string]any{} + if contributes, ok := keys["contributes"]; ok && string(contributes) != "null" { + var byTo map[string]json.RawMessage + if err := json.Unmarshal(contributes, &byTo); err != nil { + return fmt.Errorf("contributes: an object of requirement to values, or to {local name: values}: %w", err) + } + for to, v := range byTo { + // Both shapes are JSON objects, unlike secrets' path-vs-object split, so the shapes are + // told apart by what is INSIDE: an ordinary contribution's fields are scalars (a label, + // a port); the several-instance shape is an object of local names, each itself an + // object of fields. Confirmed against the whole catalogue before relying on it — no + // contribution anywhere has an object-valued field. + var fields map[string]json.RawMessage + if err := json.Unmarshal(v, &fields); err != nil { + return fmt.Errorf("contributes.%s: an object of values, or of local name to values: %w", to, err) + } + many := len(fields) > 0 + for _, field := range fields { + trimmed := bytes.TrimSpace(field) + if len(trimmed) == 0 || trimmed[0] != '{' { + many = false + break + } + } + if many { + var locals map[string]map[string]any + if err := json.Unmarshal(v, &locals); err != nil { + return fmt.Errorf("contributes.%s: an object of local name to values: %w", to, err) + } + contributesMany[to] = locals + continue + } + var values map[string]any + if err := json.Unmarshal(v, &values); err != nil { + return fmt.Errorf("contributes.%s: an object of values: %w", to, err) + } + contributesPlain[to] = values + } + delete(keys, "contributes") + } rest, err := json.Marshal(keys) if err != nil { return err @@ -696,22 +757,46 @@ func (m *Manifest) UnmarshalJSON(raw []byte) error { if len(many) > 0 { m.SecretsMany = many } + if len(contributesPlain) > 0 { + m.Contributes = contributesPlain + } + if len(contributesMany) > 0 { + m.ContributesMany = contributesMany + } return nil } -// MarshalJSON writes `secrets` back in the shape it was read: paths, and objects of local names. +// MarshalJSON writes `secrets` and `contributes` back in the shape they were read: single values, +// 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 { + if len(m.SecretsMany) == 0 && len(m.ContributesMany) == 0 { return raw, nil } var keys map[string]json.RawMessage if err := json.Unmarshal(raw, &keys); err != nil { return nil, err } + if len(m.ContributesMany) > 0 { + mergedContributes := map[string]any{} + for to, values := range m.Contributes { + mergedContributes[to] = values + } + for to, locals := range m.ContributesMany { + mergedContributes[to] = locals + } + contributes, err := json.Marshal(mergedContributes) + if err != nil { + return nil, err + } + keys["contributes"] = contributes + } + if len(m.SecretsMany) == 0 { + return json.Marshal(keys) + } merged := map[string]any{} for to, path := range m.Secrets { merged[to] = path @@ -872,6 +957,25 @@ func ParseManifest(raw []byte) (Manifest, error) { "%s contributes nothing to %q; if it only needs one, require it", m.Module, to)) } } + for to, locals := range m.ContributesMany { + if !name.MatchString(to) { + problems = append(problems, fmt.Sprintf("%q is not a usable name to contribute to", to)) + } + if len(locals) == 0 { + problems = append(problems, fmt.Sprintf( + "%s contributes nothing to %q; if it only needs one, require it", m.Module, to)) + } + for local, values := range locals { + if !name.MatchString(local) { + problems = append(problems, fmt.Sprintf( + "%s contributes to %q under %q, which is not a usable name", m.Module, to, local)) + } + if len(values) == 0 { + problems = append(problems, fmt.Sprintf( + "%s contributes nothing to %q under %q", m.Module, to, local)) + } + } + } problems = append(problems, m.Build.problems(m.Module)...) // **What provides the artifact store cannot be delivered through it** (novox/hq 04-ISSUES/029). // diff --git a/internal/catalogue/several_contributions_test.go b/internal/catalogue/several_contributions_test.go new file mode 100644 index 0000000..61b04c0 --- /dev/null +++ b/internal/catalogue/several_contributions_test.go @@ -0,0 +1,124 @@ +package catalogue + +import ( + "encoding/json" + "testing" +) + +// A module may answer one requirement more than once, the sibling of ADR 0094 for `contributes` +// rather than `secrets`: an object store's data API and its console are two different public +// names, not one. `contributes` maps a requirement to several sets of values under local names, +// each reaching the provider as its own entry — the same "several from one" shape ADR 0094 gave +// `secrets`, applied to the other half of an edge. + +const twoRoutes = `{"module":"minio","version":"1","requires":["route"], + "contributes":{"route":{"api":{"label":"files-api","port":9000},"console":{"label":"files","port":9001}}}}` + +func TestContributesReadsBothShapesAndWritesThemBack(t *testing.T) { + m, err := ParseManifest([]byte(twoRoutes)) + if err != nil { + t.Fatal(err) + } + locals := m.ContributesMany["route"] + if len(locals) != 2 || locals["api"]["label"] != "files-api" || locals["console"]["port"] != float64(9001) { + t.Fatalf("two contributions under local names: %+v", locals) + } + plain, err := ParseManifest([]byte(`{"module":"board","version":"1","requires":["route"], + "contributes":{"route":{"label":"board","port":8080}}}`)) + if err != nil { + t.Fatal(err) + } + if got := plain.Contributes["route"]; got["label"] != "board" || len(plain.ContributesMany) != 0 { + t.Fatalf("the plain shape is one contribution with no local names: %+v / %+v", got, plain.ContributesMany) + } + // 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.ContributesMany["route"]) != 2 { + t.Fatalf("the local names did not survive a round trip:\n%s", raw) + } +} + +func TestAContributionLocalNameMustBeUsable(t *testing.T) { + for _, bad := range []string{ + // Not a usable name. + `{"module":"minio","version":"1","requires":["route"], + "contributes":{"route":{"Not OK":{"label":"files","port":9000}}}}`, + // A local contribution with nothing in it. + `{"module":"minio","version":"1","requires":["route"], + "contributes":{"route":{"api":{}}}}`, + } { + if _, err := ParseManifest([]byte(bad)); err == nil { + t.Errorf("accepted:\n%s", bad) + } + } +} + +func minimalRouteProxy() Manifest { + return Manifest{Module: "route-proxy", Version: "1", + Provides: FromAnywhere("route"), + Receives: map[string]string{"route": "/var/lib/route-proxy/routes/mesh.json"}, + } +} + +func TestAModuleWithTwoRoutesGivesTheProviderTwoContributions(t *testing.T) { + minio, err := ParseManifest([]byte(twoRoutes)) + if err != nil { + t.Fatal(err) + } + got, err := Resolve(shelf(minimalRouteProxy(), minio), []string{"route-proxy", "minio"}, workstation(), World{}) + if err != nil { + t.Fatal(err) + } + out, err := got.Declaration(Rendering{}) + if err != nil { + t.Fatal(err) + } + var given []Contribution + for _, r := range out { + if r["path"] != "/var/lib/route-proxy/routes/mesh.json" { + continue + } + var parsed struct { + Given []Contribution `json:"given"` + } + if err := json.Unmarshal([]byte(r["content"].(string)), &parsed); err != nil { + t.Fatal(err) + } + given = parsed.Given + } + if len(given) != 2 { + t.Fatalf("two named routes from one module are two contributions: %+v", given) + } + byPort := map[float64]string{} + for _, g := range given { + if g.From != "minio" { + t.Fatalf("both contributions are minio's: %+v", g) + } + port, _ := g.Values["port"].(float64) + label, _ := g.Values["label"].(string) + byPort[port] = label + } + if byPort[9000] != "files-api" || byPort[9001] != "files" { + t.Fatalf("the two routes did not both survive: %+v", given) + } +} + +// A module with the ordinary, single-contribution shape resolves exactly as it did before — +// ContributesMany being empty must change nothing about it. +func TestASingleRouteStillResolvesTheOrdinaryWay(t *testing.T) { + got, err := Resolve(shelf(proxy(), published("board", "board", 8080)), []string{"board"}, workstation(), World{}) + if err != nil { + t.Fatal(err) + } + given := received(t, mustDeclare(t, got)) + if len(given) != 1 || given[0].From != "board" { + t.Fatalf("the plain shape regressed: %+v", given) + } +} -- 2.54.0