diff --git a/go/catalogue/bucket_shares.go b/go/catalogue/bucket_shares.go new file mode 100644 index 0000000..9443f7c --- /dev/null +++ b/go/catalogue/bucket_shares.go @@ -0,0 +1,124 @@ +package catalogue + +import ( + "encoding/json" + "fmt" + "regexp" + "sort" +) + +// BucketShare is one entry of `shares` on a module's requirement of `s3-bucket` (novox/hq ADR 0468 §8): who else may +// put or get in the consumer's bucket, and with which methods. Not the NFS shares of shares.go (ADR 0263). +// +// "requires": [{"name": "s3-bucket", "shares": [{"to": "seat:node-files", "methods": ["put"]}, +// {"to": "print-and-scan", "methods": ["get"]}]}] +// +// `to` is a module name, or `seat:`. `methods` is a non-empty subset of get and put, without a repeat. +// Shares are allowed on `s3-bucket` only, and no two entries name the same `to`. +type BucketShare struct { + To string `json:"to"` + Methods []string `json:"methods"` +} + +// BucketRequirement is the requirement `shares` may be written on. +const BucketRequirement = "s3-bucket" + +// RequireSharesKey is where a stored manifest keeps the shares of its requirements: beside `requires`, which stays a +// plain list of names, for the reason OptionalRequiresKey is beside it. An older controller sets the unknown key +// aside instead of failing the catalogue. +const RequireSharesKey = "requires-shares" + +// SharesValue is the key of the value a consumer gives the provider of s3-bucket in its grant: the list of +// {to, methods} sorted by `to`. A consumer's own `contributes` may not name it. +const SharesValue = "shares" + +var shareTarget = regexp.MustCompile(`^(seat:)?[a-z0-9][a-z0-9-]*$`) + +// checkBucketShares refuses a malformed `shares` of one requirement, naming the entry and ADR 0468 §8. +func checkBucketShares(requirement string, shares []BucketShare) error { + if len(shares) == 0 { + return nil + } + if requirement != BucketRequirement { + return fmt.Errorf("requires %s: `shares` is allowed on %s only (novox/hq ADR 0468 §8)", requirement, BucketRequirement) + } + seen := map[string]bool{} + for i, s := range shares { + at := fmt.Sprintf("requires %s: shares[%d] (to %q)", requirement, i, s.To) + if !shareTarget.MatchString(s.To) { + return fmt.Errorf("%s: `to` is a module name or seat: (novox/hq ADR 0468 §8)", at) + } + if seen[s.To] { + return fmt.Errorf("%s: a second entry for the same `to` (novox/hq ADR 0468 §8)", at) + } + seen[s.To] = true + if len(s.Methods) == 0 { + return fmt.Errorf("%s: `methods` is empty; name get, put or both (novox/hq ADR 0468 §8)", at) + } + used := map[string]bool{} + for _, method := range s.Methods { + if method != "get" && method != "put" { + return fmt.Errorf("%s: method %q is not get or put (novox/hq ADR 0468 §8)", at, method) + } + if used[method] { + return fmt.Errorf("%s: method %q twice (novox/hq ADR 0468 §8)", at, method) + } + used[method] = true + } + } + return nil +} + +// readRequireShares reads the stored form's side key. +func readRequireShares(raw json.RawMessage) (map[string][]BucketShare, error) { + var out map[string][]BucketShare + if err := json.Unmarshal(raw, &out); err != nil { + return nil, fmt.Errorf("%s: an object of requirement to a list of {to, methods}: %w", RequireSharesKey, err) + } + for requirement, shares := range out { + if err := checkBucketShares(requirement, shares); err != nil { + return nil, err + } + } + return out, nil +} + +// sortedShares is a requirement's shares as they are handed to the provider: sorted by `to` (a file whose lines move +// looks changed), each entry's methods sorted, and a copy so the manifest is not reordered. +func sortedShares(shares []BucketShare) []BucketShare { + out := make([]BucketShare, 0, len(shares)) + for _, s := range shares { + methods := append([]string(nil), s.Methods...) + sort.Strings(methods) + out = append(out, BucketShare{To: s.To, Methods: methods}) + } + sort.Slice(out, func(i, j int) bool { return out[i].To < out[j].To }) + return out +} + +// withShares adds this module's shares of the requirement to the values it gives the provider. A module whose own +// `contributes` already names `shares` is refused: the field is the only way to say it (ADR 0468 §8). +func withShares(m Manifest, requirement string, values map[string]any) (map[string]any, error) { + shares := m.RequireShares[requirement] + if _, named := values[SharesValue]; named && requirement == BucketRequirement { + return nil, fmt.Errorf("%s contributing to %s names `shares`; write it on its requirement instead (novox/hq ADR 0468 §8)", + m.Module, requirement) + } + if len(shares) == 0 { + return values, nil + } + out := make(map[string]any, len(values)+1) + for k, v := range values { + out[k] = v + } + listed := []any{} + for _, s := range sortedShares(shares) { + methods := []any{} + for _, method := range s.Methods { + methods = append(methods, method) + } + listed = append(listed, map[string]any{"to": s.To, "methods": methods}) + } + out[SharesValue] = listed + return out, nil +} diff --git a/go/catalogue/bucket_shares_test.go b/go/catalogue/bucket_shares_test.go new file mode 100644 index 0000000..87345f4 --- /dev/null +++ b/go/catalogue/bucket_shares_test.go @@ -0,0 +1,185 @@ +package catalogue + +import ( + "encoding/json" + "reflect" + "strings" + "testing" +) + +// Issue 1591: `shares` on a requirement of s3-bucket (novox/hq ADR 0468 §8). + +const filesWithShares = `{"module":"files","version":"1","requires":["shell", + {"name":"s3-bucket","shares":[{"to":"seat:node-files","methods":["put"]},{"to":"print-and-scan","methods":["get","put"]}]}]}` + +func minioLike() Manifest { + return Manifest{Module: "minio", Version: "1", Provides: Offers("s3-bucket"), + Receives: map[string]string{"s3-bucket": "/grants/mesh.json"}} +} + +func givenToMinio(t *testing.T, out []map[string]any) []Contribution { + t.Helper() + for _, r := range out { + if r["path"] != "/grants/mesh.json" { + continue + } + var parsed struct { + Given []Contribution `json:"given"` + } + if err := json.Unmarshal([]byte(r["content"].(string)), &parsed); err != nil { + t.Fatal(err) + } + return parsed.Given + } + t.Fatalf("minio was given no file: %v", out) + return nil +} + +func sharesOf(t *testing.T, values map[string]any) string { + t.Helper() + raw, err := json.Marshal(values["shares"]) + if err != nil { + t.Fatal(err) + } + return string(raw) +} + +const sortedSharesJSON = `[{"methods":["get","put"],"to":"print-and-scan"},{"methods":["put"],"to":"seat:node-files"}]` + +// 1591/AC1: parsed, the shares are on the manifest; stored, `requires` is plain names and the side key holds them. +func TestSharesParseAndAreStoredBesideRequires(t *testing.T) { + m := mustParse(t, filesWithShares) + want := []BucketShare{{To: "seat:node-files", Methods: []string{"put"}}, {To: "print-and-scan", Methods: []string{"get", "put"}}} + if !reflect.DeepEqual(m.RequireShares["s3-bucket"], want) { + t.Fatalf("the shares read as %+v", m.RequireShares) + } + raw, err := json.Marshal(m) + if err != nil { + t.Fatal(err) + } + var keys map[string]json.RawMessage + if err := json.Unmarshal(raw, &keys); err != nil { + t.Fatal(err) + } + if string(keys["requires"]) != `["shell","s3-bucket"]` { + t.Errorf("the stored requires is not plain names: %s", keys["requires"]) + } + if !strings.Contains(string(keys[RequireSharesKey]), `"seat:node-files"`) { + t.Errorf("the side key does not hold the shares: %s", raw) + } + var again Manifest + if err := json.Unmarshal(raw, &again); err != nil { + t.Fatal(err) + } + if !reflect.DeepEqual(again, m) { + t.Fatalf("not lossless:\n%+v\n%+v", again, m) + } +} + +// 1591/AC2: a malformed share is refused, naming the entry and ADR 0468 §8. +func TestAMalformedShareIsRefused(t *testing.T) { + cases := map[string]string{ + "unknown method": `{"name":"s3-bucket","shares":[{"to":"a","methods":["delete"]}]}`, + "empty to": `{"name":"s3-bucket","shares":[{"to":"","methods":["get"]}]}`, + "malformed to": `{"name":"s3-bucket","shares":[{"to":"Seat:X y","methods":["get"]}]}`, + "empty methods": `{"name":"s3-bucket","shares":[{"to":"a","methods":[]}]}`, + "duplicate to": `{"name":"s3-bucket","shares":[{"to":"a","methods":["get"]},{"to":"a","methods":["put"]}]}`, + "duplicate verb": `{"name":"s3-bucket","shares":[{"to":"a","methods":["get","get"]}]}`, + "other require": `{"name":"route","shares":[{"to":"a","methods":["get"]}]}`, + } + for name, entry := range cases { + _, err := ParseManifest([]byte(`{"module":"files","version":"1","requires":[` + entry + `]}`)) + if err == nil { + t.Errorf("%s: accepted", name) + continue + } + if !strings.Contains(err.Error(), "ADR 0468 §8") || !strings.Contains(err.Error(), "requires") { + t.Errorf("%s: the error does not name the entry and the ADR: %v", name, err) + } + } + // The stored form is held to the same rules. + _, err := ParseManifest([]byte(`{"module":"files","version":"1","requires":["s3-bucket"], + "requires-shares":{"s3-bucket":[{"to":"a","methods":[]}]}}`)) + if err == nil || !strings.Contains(err.Error(), "ADR 0468 §8") { + t.Errorf("the stored form was accepted: %v", err) + } +} + +// 1591/AC3: the values a consumer gives carry the shares sorted by `to`, remote (grant) and co-located alike. +func TestTheGrantCarriesTheSharesSorted(t *testing.T) { + m := mustParse(t, filesWithShares) + remote := Resolution{Node: "laptop", Modules: []Manifest{m}} + values, asks, err := remote.ContributionsFrom("s3-bucket", "files", SettingsBy{}) + if err != nil || !asks { + t.Fatalf("asks %v, err %v", asks, err) + } + if got := sharesOf(t, values); got != sortedSharesJSON { + t.Fatalf("the grant values carry %s", got) + } + // As the provider's file shows it, through a grant. + provider, err := Resolve(shelf(minioLike()), []string{"minio"}, workstation(), World{}) + if err != nil { + t.Fatal(err) + } + out, err := provider.Declaration(Rendering{Grants: []Grant{{Provision: "s3-bucket", Consumer: "laptop", From: "files", Values: values}}}) + if err != nil { + t.Fatal(err) + } + given := givenToMinio(t, out) + if len(given) != 1 || sharesOf(t, given[0].Values) != sortedSharesJSON { + t.Fatalf("the remote consumer's shares did not reach the file: %+v", given) + } + + // Co-located: the consumer contributes to s3-bucket too, and no grant is passed. + co := mustParse(t, `{"module":"files","version":"1","contributes":{"s3-bucket":{"name":"files"}}, + "requires":[{"name":"s3-bucket","shares":[{"to":"seat:node-files","methods":["put"]},{"to":"print-and-scan","methods":["put","get"]}]}]}`) + both, err := Resolve(shelf(minioLike(), co), []string{"files"}, workstation(), World{}) + if err != nil { + t.Fatal(err) + } + out, err = both.Declaration(Rendering{}) + if err != nil { + t.Fatal(err) + } + given = givenToMinio(t, out) + if len(given) != 1 || sharesOf(t, given[0].Values) != sortedSharesJSON || given[0].Values["name"] != "files" { + t.Fatalf("the co-located consumer's shares did not reach the file: %+v", given) + } +} + +// 1591/AC4: only the shares change, so the module is not recreated and held for no one; the provider's file changes. +func TestChangingOnlyTheSharesDoesNotRecreateTheConsumer(t *testing.T) { + was := mustParse(t, filesWithShares) + now := mustParse(t, `{"module":"files","version":"1","requires":["shell", + {"name":"s3-bucket","shares":[{"to":"print-and-scan","methods":["get"]}]}]}`) + if said := Recreates(was, now).Say("files"); said != "" { + t.Errorf("a change of shares recreates the module: %s", said) + } + if said := Holds(was, now); said != "" { + t.Errorf("a change of shares holds the module: %s", said) + } + before := Resolution{Node: "laptop", Modules: []Manifest{was}} + after := Resolution{Node: "laptop", Modules: []Manifest{now}} + a, _, _ := before.ContributionsFrom("s3-bucket", "files", SettingsBy{}) + b, _, _ := after.ContributionsFrom("s3-bucket", "files", SettingsBy{}) + if sharesOf(t, a) == sharesOf(t, b) { + t.Fatalf("the provider's values did not change: %s", sharesOf(t, a)) + } +} + +// 1591/AC5: a consumer whose contributes to s3-bucket names `shares` itself is refused. +func TestAConsumerCannotContributeSharesItself(t *testing.T) { + _, err := ParseManifest([]byte(`{"module":"files","version":"1", + "contributes":{"s3-bucket":{"shares":"x"}}, + "requires":[{"name":"s3-bucket","shares":[{"to":"a","methods":["get"]}]}]}`)) + if err == nil || !strings.Contains(err.Error(), "ADR 0468 §8") { + t.Errorf("both written, accepted: %v", err) + } + // Without the field the grant is still refused, so the provider is never told something the manifest did not say. + m := Manifest{Module: "files", Requires: []string{"s3-bucket"}, + Contributes: map[string]map[string]any{"s3-bucket": {"shares": "x"}}} + r := Resolution{Node: "laptop", Modules: []Manifest{m}} + if _, _, err := r.ContributionsFrom("s3-bucket", "files", SettingsBy{}); err == nil { + t.Error("a contributed `shares` reached the grant") + } +} diff --git a/go/catalogue/declaration.go b/go/catalogue/declaration.go index a2d7597..35895c8 100644 --- a/go/catalogue/declaration.go +++ b/go/catalogue/declaration.go @@ -1783,6 +1783,10 @@ func (r Resolution) contributions(settings SettingsBy, grants []Grant, if err != nil { return nil, err } + // What the module shares of its bucket reaches the provider with the rest (novox/hq ADR 0468 §8). + if values, err = withShares(m, to, values); err != nil { + return nil, err + } out[to] = append(out[to], Contribution{From: m.Module, Values: values}) } // Several contributions to one requirement (ADR 0094's sibling for `contributes`): an @@ -2169,11 +2173,19 @@ func (r Resolution) ContributionsFrom(requirement, module string, settings Setti mine = append(mine, g.Values) } } + var asker Manifest + for _, m := range r.Modules { + if m.Module == module { + asker = m + } + } if len(mine) == 1 { + // Its shares are already in, put there where its contribution was composed (contributions). return mine[0], true, nil } if len(mine) > 1 { - return map[string]any{}, true, nil + values, err := withShares(asker, requirement, map[string]any{}) + return values, err == nil, err } // It contributes no payload — but a require-only consumer of a parameterless provision (one whose // `serves` names no consumer-supplied key: `redis-cache`, `amqp`) still ASKS for it and must be @@ -2186,7 +2198,8 @@ func (r Resolution) ContributionsFrom(requirement, module string, settings Setti } for _, req := range m.Requires { if req == requirement { - return map[string]any{}, true, nil + values, err := withShares(m, requirement, map[string]any{}) + return values, err == nil, err } } } diff --git a/go/catalogue/manifest.go b/go/catalogue/manifest.go index cd48fdf..0464654 100644 --- a/go/catalogue/manifest.go +++ b/go/catalogue/manifest.go @@ -494,6 +494,9 @@ type Manifest struct { // by UnmarshalJSON from the long form of a `requires` entry, {name, optional}, or from the stored // form's `optional-requires` list; never written by hand. OptionalRequires map[string]bool `json:"-"` + // RequireShares is who else may use the bucket a module requires, by requirement (BucketShare, novox/hq ADR 0468 + // §8): filled from the long form's `shares`, or from the stored form's `requires-shares`; never written by hand. + RequireShares map[string][]BucketShare `json:"-"` // Emits are the events this module publishes, named **locally**: `order.placed`, not a subject // and not a routing key. The mesh derives where it lands (design 29 §1), so reorganising the @@ -1538,8 +1541,9 @@ func (m *Manifest) UnmarshalJSON(raw []byte) error { delete(keys, "contributes") } var optionalRequires map[string]bool + var requireShares map[string][]BucketShare if requires, ok := keys["requires"]; ok && string(requires) != "null" { - names, optional, err := readRequires(requires) + names, optional, shares, err := readRequires(requires) if err != nil { return err } @@ -1549,6 +1553,23 @@ func (m *Manifest) UnmarshalJSON(raw []byte) error { } keys["requires"] = plainNames optionalRequires = optional + requireShares = shares + } + // The stored form's shares (RequireSharesKey). + if stored, ok := keys[RequireSharesKey]; ok { + if string(stored) != "null" { + stood, err := readRequireShares(stored) + if err != nil { + return err + } + for requirement, shares := range stood { + if requireShares == nil { + requireShares = map[string][]BucketShare{} + } + requireShares[requirement] = shares + } + } + delete(keys, RequireSharesKey) } // The stored form: `requires` plain, and which are optional beside it (OptionalRequiresKey). if stored, ok := keys[OptionalRequiresKey]; ok { @@ -1599,6 +1620,7 @@ func (m *Manifest) UnmarshalJSON(raw []byte) error { m.unknown = unknown m.forms = forms m.OptionalRequires = optionalRequires + m.RequireShares = requireShares if len(plain) > 0 { m.Secrets = plain } @@ -1611,6 +1633,21 @@ func (m *Manifest) UnmarshalJSON(raw []byte) error { if len(contributesMany) > 0 { m.ContributesMany = contributesMany } + if err := m.checkSharesNotContributed(); err != nil { + return err + } + return nil +} + +// checkSharesNotContributed refuses a module whose own `contributes` to s3-bucket names `shares` (ADR 0468 §8). +func (m Manifest) checkSharesNotContributed() error { + if len(m.RequireShares[BucketRequirement]) == 0 { + return nil + } + if _, named := m.Contributes[BucketRequirement][SharesValue]; named { + return fmt.Errorf("contributes.%s names `shares` and so does its requirement; keep one, the requirement's (novox/hq ADR 0468 §8)", + BucketRequirement) + } return nil } @@ -1655,7 +1692,7 @@ func (m Manifest) marshalFields() ([]byte, error) { if err != nil { return nil, err } - if len(m.SecretsMany) == 0 && len(m.ContributesMany) == 0 && len(m.OptionalRequires) == 0 { + if len(m.SecretsMany) == 0 && len(m.ContributesMany) == 0 && len(m.OptionalRequires) == 0 && len(m.RequireShares) == 0 { return raw, nil } var keys map[string]json.RawMessage @@ -1671,6 +1708,13 @@ func (m Manifest) marshalFields() ([]byte, error) { } keys[OptionalRequiresKey] = listed } + if len(m.RequireShares) > 0 { + listed, err := json.Marshal(m.RequireShares) + if err != nil { + return nil, err + } + keys[RequireSharesKey] = listed + } if len(m.ContributesMany) > 0 { mergedContributes := map[string]any{} for to, values := range m.Contributes { diff --git a/go/catalogue/optional_requirements.go b/go/catalogue/optional_requirements.go index f2db8ad..921b1d7 100644 --- a/go/catalogue/optional_requirements.go +++ b/go/catalogue/optional_requirements.go @@ -32,17 +32,20 @@ const OnlyWhenBound = "only-when-bound" type requirementForm struct { Name string `json:"name"` Optional bool `json:"optional,omitempty"` + // Shares is who else may use the consumer's bucket (BucketShare, novox/hq ADR 0468 §8), on s3-bucket only. + Shares []BucketShare `json:"shares,omitempty"` } // readRequires reads `requires` in both of its forms — a bare name, or {name, optional} — into the // names and which of them are optional. -func readRequires(raw json.RawMessage) ([]string, map[string]bool, error) { +func readRequires(raw json.RawMessage) ([]string, map[string]bool, map[string][]BucketShare, error) { var entries []json.RawMessage if err := json.Unmarshal(raw, &entries); err != nil { - return nil, nil, fmt.Errorf("requires: a list of names, or of {name, optional}: %w", err) + return nil, nil, nil, fmt.Errorf("requires: a list of names, or of {name, optional}: %w", err) } var names []string optional := map[string]bool{} + var shares map[string][]BucketShare for i, e := range entries { var plain string if err := json.Unmarshal(e, &plain); err == nil { @@ -53,12 +56,21 @@ func readRequires(raw json.RawMessage) ([]string, map[string]bool, error) { dec := json.NewDecoder(bytes.NewReader(e)) dec.DisallowUnknownFields() if err := dec.Decode(&full); err != nil { - return nil, nil, fmt.Errorf("requires[%d]: a name, or {name, optional} (novox/hq ADR 0330): %w", i, err) + return nil, nil, nil, fmt.Errorf("requires[%d]: a name, or {name, optional} (novox/hq ADR 0330): %w", i, err) } if strings.TrimSpace(full.Name) == "" { - return nil, nil, fmt.Errorf("requires[%d] names no requirement: {name, optional}", i) + return nil, nil, nil, fmt.Errorf("requires[%d] names no requirement: {name, optional}", i) } names = append(names, full.Name) + if err := checkBucketShares(full.Name, full.Shares); err != nil { + return nil, nil, nil, fmt.Errorf("requires[%d]: %w", i, err) + } + if len(full.Shares) > 0 { + if shares == nil { + shares = map[string][]BucketShare{} + } + shares[full.Name] = full.Shares + } if full.Optional { optional[full.Name] = true } @@ -66,7 +78,7 @@ func readRequires(raw json.RawMessage) ([]string, map[string]bool, error) { if len(optional) == 0 { optional = nil } - return names, optional, nil + return names, optional, shares, nil } // OptionalRequiresKey is where a stored manifest keeps which of its requirements are optional: beside