diff --git a/go/catalogue/setting_defaults_test.go b/go/catalogue/setting_defaults_test.go index 7b08f4b..1d6489f 100644 --- a/go/catalogue/setting_defaults_test.go +++ b/go/catalogue/setting_defaults_test.go @@ -31,6 +31,7 @@ func parsed(t *testing.T, m Manifest) error { } func TestAnUnsetPreferenceTakesItsDefault(t *testing.T) { + t.Run("the node over the mesh over the default", theNodeOverTheMeshOverTheDefault) m := notifier() for _, layers := range [][]Layer{nil, {{From: MeshWideLayer, Values: map[string]any{}}}} { file := map[string]any{} @@ -49,7 +50,7 @@ func TestAnUnsetPreferenceTakesItsDefault(t *testing.T) { } } -func TestTheNodeOverTheMeshOverTheDefault(t *testing.T) { +func theNodeOverTheMeshOverTheDefault(t *testing.T) { m := notifier() layers := []Layer{ {From: MeshWideLayer, Values: map[string]any{"width": float64(300)}}, diff --git a/go/compose/frombus/frombus.go b/go/compose/frombus/frombus.go index 1bc385a..a74764f 100644 --- a/go/compose/frombus/frombus.go +++ b/go/compose/frombus/frombus.go @@ -545,10 +545,16 @@ func PlansHeld(ctx context.Context, r Reader, mesh Mesh, s Scope) (plans map[str // plansHeldOf is the plans of node over in, every module in held held, and each module a refusal is of held too until // the plans compose (culprit's rule over the Planning mode: its stub first, then its bare stub). func plansHeldOf(in compose.Input, node string, held map[string]string) (map[string]catalogue.Plan, error) { + return heldOf(in, node, held, func(in compose.Input) (map[string]catalogue.Plan, error) { return plansIn(in, node, compose.Planning) }) +} + +// heldOf composes compose1 over in with every module in held held, and each module a refusal is of held too until it +// composes (culprit's rule: its stub first, then its bare stub); what compose1 answers of a held module is left out. +// Plans and data items are composed so (PlansHeld, DataItemsHeld): one module never ends the read (novox/hq ADR 0578 §3). +func heldOf[V any](in compose.Input, node string, held map[string]string, compose1 func(compose.Input) (map[string]V, error)) (map[string]V, error) { for _, m := range sortedKeys(held) { in = stubbed(in, node, m, "") } - compose1 := func(in compose.Input) (map[string]catalogue.Plan, error) { return plansIn(in, node, compose.Planning) } // Each try holds one module more or leaves out all of one held module's offer: bounded by twice the modules placed. for tries := 0; tries <= 2*len(assignedOn(in, node))+1; tries++ { plans, err := compose1(in) @@ -585,7 +591,7 @@ func plansHeldOf(in compose.Input, node string, held map[string]string) (map[str } in = stubbed(in, node, module, leave) } - return nil, fmt.Errorf("frombus: the plans of %s do not compose with %d module(s) held: %s", node, len(held), + return nil, fmt.Errorf("frombus: what %s composes does not compose with %d module(s) held: %s", node, len(held), strings.Join(sortedKeys(held), ", ")) } @@ -1057,9 +1063,34 @@ func DataItemsHeld(ctx context.Context, r Reader, mesh Mesh, s Scope) (items map if err != nil { return nil, nil, held, err } - items, unplaced, err = compose.DataItems(in, s.Node) - for m := range held { - delete(items, m) - } + // Held module by module, as PlansHeld holds them: a refused stub or one module's composition refusal (an item's + // template) holds that module and never ends the read. + items, err = heldOf(in, s.Node, held, func(in compose.Input) (map[string][]catalogue.PlacedDataItem, error) { + var err error + items, unplaced, err = compose.DataItems(withoutHeldData(in, s.Node), s.Node) + return items, err + }) return items, unplaced, held, err } + +// withoutHeldData is in with no data item of a module held on node: none of a held module's items is measured, so its +// data never refuses the rest (its stub keeps its data for the backups the plans compose). +func withoutHeldData(in compose.Input, node string) compose.Input { + out := in + out.Nodes = append([]compose.Node(nil), in.Nodes...) + for i := range out.Nodes { + n := &out.Nodes[i] + if n.Name != node || len(n.Held) == 0 { + continue + } + kept := maps.Clone(n.Kept) + for m := range n.Held { + if man, ok := kept[m]; ok { + man.Data = nil + kept[m] = man + } + } + n.Kept = kept + } + return out +} diff --git a/go/compose/frombus/frombustest/frombustest_test.go b/go/compose/frombus/frombustest/frombustest_test.go index 5d6d751..0db70b2 100644 --- a/go/compose/frombus/frombustest/frombustest_test.go +++ b/go/compose/frombus/frombustest/frombustest_test.go @@ -29,16 +29,25 @@ func TestTheProvisionedMeshHandsAPlanWithItsProvision(t *testing.T) { // refused is the indexer's verdict on bad, a problem its judge found (novox/hq ADR 0578 §1): Plans then holds // backup too, and never ends the read. refused string + // dataHeld says the data items' read holds backup too: a refusal it meets (a refused manifest, an unusable build, + // an item composition refuses), not one only a plan meets. + dataHeld bool + // dataOnly checks the data items' read alone: the other reads blame restic, whose templates collect the item. + dataOnly bool }{ {name: "every module composes"}, {name: "a built manifest that cannot be used", bad: `{"module":"backup","version":"1","build":{"artifacts":[{"name":"files","kind":"archive","from":"files"}]}}`, - built: `{"module":"backup","version":"1","requires":"amqp"}`, why: "backup"}, + built: `{"module":"backup","version":"1","requires":"amqp"}`, why: "backup", dataHeld: true}, {name: "a plan that refuses", bad: `{"module":"backup","version":"1","resources":[{"id":"conf","type":"file","path":"/etc/backup.conf",` + `"mode":"0600","content":"key=${secret:nope}"}]}`, why: "${secret:nope}"}, {name: "an indexer's manifest the mesh refuses", bad: unusableItem, why: `"shows" is none of them`, - refused: `"shows" is none of them`}, + refused: `"shows" is none of them`, dataHeld: true}, + // ADR 0578 §3: an accepted manifest whose data item its composition refuses is held by the culprit rule, in every read. + {name: "a data item its composition refuses", + bad: `{"module":"backup","version":"1","data":{"own":[{"id":"d","path":"${dir:nope}","class":"cache"}]}}`, + why: "nope", dataHeld: true, dataOnly: true}, {name: "an indexer's manifest the mesh refuses, placed elsewhere", bad: unusableItem, elsewhere: true, refused: `"shows" is none of them`}, } { @@ -65,6 +74,13 @@ func TestTheProvisionedMeshHandsAPlanWithItsProvision(t *testing.T) { b.Builds["backup "+fp] = &delivery.BuilderBuild{State: delivery.BuildDone, Manifest: builtDigest, Artifacts: []delivery.BuiltArtifact{{Name: "files", Kind: "archive", Reference: "artifact-store://backup/files/blobs/x", Digest: "sha256:x"}}} } + if row.dataOnly { + items, _, iheld, err := frombus.DataItemsHeld(context.Background(), b, frombus.Mesh{}, frombus.Scope{Node: Node}) + if why, ok := iheld["backup"]; err != nil || !ok || !strings.Contains(why, row.why) || len(items["restic"]) == 0 { + t.Errorf("%s: data items %v held %v: %v", row.name, items, iheld, err) + } + continue + } plans, err := frombus.Plans(context.Background(), b, frombus.Mesh{}, frombus.Scope{Node: Node}) switch { case row.refused != "" && (err != nil || !planned(plans, "restic") || planned(plans, "backup")): @@ -111,6 +127,13 @@ func TestTheProvisionedMeshHandsAPlanWithItsProvision(t *testing.T) { if _, planned := plans["backup"]; planned { t.Errorf("%s: the held module has a plan", row.name) } + // The data items are read by the same rule (ADR 0578 §3: disk-load's read): the bad module held, the read whole. + items, _, iheld, err := frombus.DataItemsHeld(context.Background(), b, frombus.Mesh{}, frombus.Scope{Node: Node}) + // A refusal only a plan meets (a file's secret) holds nothing here: data items render no such file. + mustHold := row.dataHeld + if why, ok := iheld["backup"]; err != nil || ok != mustHold || mustHold && !strings.Contains(why, row.why) || items["backup"] != nil { + t.Errorf("%s: data items %v held %v: %v", row.name, items, iheld, err) + } } } diff --git a/go/judge_free_test.go b/go/judge_free_test.go new file mode 100644 index 0000000..86bb607 --- /dev/null +++ b/go/judge_free_test.go @@ -0,0 +1,175 @@ +// The same scan as mesh-catalog's checks/sdkjudge, run by mesh-sdk's own check (novox/hq ADR 0578 §4: the change that +// adds a manifest judge to mesh-sdk is refused), so a change to mesh-sdk alone is refused too. Copied from mesh-catalog +// feat/judge-once checks/sdkjudge/main.go; the two lists of composition's refusals are kept the same. +package stdio + +import ( + "fmt" + "go/ast" + "go/parser" + "go/token" + "io/fs" + "path/filepath" + "regexp" + "sort" + "strings" + "testing" +) + +// composition is every function or table the check lets stay in mesh-sdk, as "path-from-go/:name", and why. It names no +// manifest judgement: each is the composer's refusal of composition itself (ADR 0578 clause 5). +var composition = map[string]string{ + "catalogue/consumers.go:consumersPlaceholderProblems": "composition: rendering a consumer's template", + "catalogue/collections.go:collectedPlaceholderProblems": "composition: placeholders a contribution renders", + "catalogue/environment_into.go:placeholderProblems": "composition: placeholders a contribution renders", + "catalogue/seat_contributions.go:seatPlaceholderProblems": "composition: placeholders a contribution renders", + "catalogue/collections.go:itemTemplateProblem": "composition: filling an item's template", + "catalogue/setting_takes.go:takenSettingsProblems": "composition: whether stored settings still compose", + "catalogue/template.go:HeldTemplateProblem": "composition: the take-in refusal of a resolved build", +} + +// judgeProblems is what mesh-sdk's Go tree under root (its go/ directory) holds of a manifest judge, one line each, but for +// what composition lets stay (the map the caller gives; the test of the real tree gives the package's), and the +// entries of composition it no longer holds. +func judgeProblems(root string, composition map[string]string) ([]string, error) { + var out []string + seen := map[string]bool{} + err := filepath.WalkDir(root, func(path string, d fs.DirEntry, err error) error { + if err != nil { + return err + } + if d.IsDir() { + if n := d.Name(); n == "testdata" || n == "vendor" || strings.HasPrefix(n, ".") { + return filepath.SkipDir + } + return nil + } + if !strings.HasSuffix(path, ".go") || strings.HasSuffix(path, "_test.go") { + return nil + } + rel, _ := filepath.Rel(root, path) + file, err := parser.ParseFile(token.NewFileSet(), path, nil, parser.SkipObjectResolution) + if err != nil { + return fmt.Errorf("%s: %w", rel, err) + } + inCatalogue := filepath.Base(filepath.Dir(path)) == "catalogue" + for _, d := range file.Decls { + switch d := d.(type) { + case *ast.FuncDecl: + if why := judgeFunc(d); why != "" { + out = append(out, note(composition, seen, rel, d.Name.Name, why)) + } + case *ast.GenDecl: + if !inCatalogue || d.Tok != token.VAR { + continue + } + for _, spec := range d.Specs { + v, _ := spec.(*ast.ValueSpec) + if v == nil { + continue + } + for i, name := range v.Names { + if i < len(v.Values) && allowName.MatchString(name.Name) && isTable(v.Values[i]) { + out = append(out, note(composition, seen, rel, name.Name, + "a table of what a manifest may declare (an allow-list)")) + } + } + } + } + } + return nil + }) + if err != nil { + return nil, err + } + var kept []string + for _, line := range out { + if line != "" { + kept = append(kept, line) + } + } + for key, why := range composition { + if !seen[key] { + kept = append(kept, fmt.Sprintf("composition lists %s (%s), which mesh-sdk no longer holds: take it off the list", key, why)) + } + } + sort.Strings(kept) + return kept, nil +} + +// note is the line refusing name in rel for why, or "" when composition lets it stay. +func note(composition map[string]string, seen map[string]bool, rel, name, why string) string { + key := rel + ":" + name + if _, ok := composition[key]; ok { + seen[key] = true + return "" + } + return fmt.Sprintf("%s: %s is %s; mesh-sdk carries the contract only, the judge is the indexer's and the checker's (ADR 0578 §4)", rel, name, why) +} + +// allowName is what names a table of what is allowed. +var allowName = regexp.MustCompile(`(?i)allow|permit|removable|reserved|forbid|denied`) + +// isTable says whether an expression is a map or a list literal of more than one entry. +func isTable(e ast.Expr) bool { + c, ok := e.(*ast.CompositeLit) + if !ok || len(c.Elts) < 2 { + return false + } + switch c.Type.(type) { + case *ast.MapType, *ast.ArrayType: + return true + } + return false +} + +// judgeFunc says why a function is a manifest judge, or "". +func judgeFunc(f *ast.FuncDecl) string { + name := f.Name.Name + if judgeEntry.MatchString(name) { + return "a function that judges a manifest" + } + if !strings.HasSuffix(name, "Problems") && !strings.HasSuffix(name, "Problem") { + return "" + } + if takes(f.Recv) || takes(f.Type.Params) { + return "a function that gathers a manifest's problems" + } + return "" +} + +// judgeEntry is the names of the judge's entry points: the strict parsers and the validators of a manifest or a shelf. +var judgeEntry = regexp.MustCompile(`^(ParseManifest(With|ForInputs)?|Validate(Manifest|Shelf)\w*)$`) + +// takes says whether a list of parameters names a Manifest or a Shelf anywhere in a type. +func takes(fields *ast.FieldList) bool { + if fields == nil { + return false + } + found := false + for _, f := range fields.List { + ast.Inspect(f.Type, func(n ast.Node) bool { + if id, ok := n.(*ast.Ident); ok && (id.Name == "Manifest" || id.Name == "Shelf") { + found = true + } + return !found + }) + } + return found +} + +// mesh-sdk holds no manifest judge: no function gathering a manifest's problems, no judge entry point and no allow-list +// table in the catalogue, but for composition's own refusals, each listed with why (ADR 0578 §4, §5). +func TestMeshSDKHoldsNoManifestJudge(t *testing.T) { + root, err := filepath.Abs(".") + if err != nil { + t.Fatal(err) + } + problems, err := judgeProblems(root, composition) + if err != nil { + t.Fatal(err) + } + for _, p := range problems { + t.Error(p) + } +}