diff --git a/modules/mesh-delivery-indexer/internal/index/verdicts.go b/modules/mesh-delivery-indexer/internal/index/verdicts.go index 4e27b03c..83f84a7c 100644 --- a/modules/mesh-delivery-indexer/internal/index/verdicts.go +++ b/modules/mesh-delivery-indexer/internal/index/verdicts.go @@ -72,10 +72,14 @@ func (ix *Indexer) judgeOver(ctx context.Context, fresh map[string]bool, over ma if err != nil { return out, err } + trunkOf := map[string]string{} // a judged change's module to the trunk's manifest digest it changes (DLV-I15) if over != nil { fresh = map[string]bool{} for module, digest := range over { m := held[module] + if m.Manifest != "" && m.Manifest != digest { + trunkOf[module] = m.Manifest + } m.Manifest = digest held[module] = m fresh[digest] = true @@ -111,6 +115,26 @@ func (ix *Indexer) judgeOver(ctx context.Context, fresh map[string]bool, over ma counts = ix.heldOldSections(ctx) } refused, waits, unnamed := judge.ShelfWaiting(raws, predefined, 0, counts) + // A change's renamed-from is judged against the trunk's manifest of the same module (DLV-I15, design 32). + for module, digest := range trunkOf { + if _, waiting := waits[module]; waiting || raws[module] == nil { + continue + } + value, _, found, err := ix.Manifests.Get(ctx, digest) + kept, derr := delivery.DecodeManifest(value) + if err != nil || !found || derr != nil { + ix.logf("judge: the trunk's manifest %s of %s cannot be read, so its renames are not judged against it", digest, module) + continue + } + head, herr := catalogue.Decode(raws[module]) + trunk, terr := catalogue.Decode([]byte(kept.JSON)) + if herr != nil || terr != nil { + continue // the manifest's own problems say why + } + if p := judge.RenameProblems(module, head, trunk, predefined); len(p) > 0 { + refused[module] = append(refused[module], p...) + } + } for _, p := range unnamed { ix.logf("judge: a problem between the manifests that no one module's removal ends, so it refuses none: %s", p) } diff --git a/modules/mesh-delivery-indexer/internal/index/verdicts_test.go b/modules/mesh-delivery-indexer/internal/index/verdicts_test.go index 665c80ba..ddc4a105 100644 --- a/modules/mesh-delivery-indexer/internal/index/verdicts_test.go +++ b/modules/mesh-delivery-indexer/internal/index/verdicts_test.go @@ -221,6 +221,25 @@ func TestTheIndexerJudgesEveryManifestOnce(t *testing.T) { if _, _, err := NewSayAccepted("flaky", "", "jochen@shanks", time.Now()); err == nil { t.Error("an ask saying no why was taken") } + // DLV-I15: a change's renamed-from is judged against the trunk's manifest of the module: a name the trunk declares is + // accepted, one it does not is refused naming the key. + trunkRenamer := hold("renamer", `{"module":"renamer","version":"1","config":{"font-name":{"type":"string","description":"The font.","required":true,"runtime":true}}}`) + if _, err := ix.JudgeAll(ctx, map[string]bool{trunkRenamer: true}); err != nil { + t.Fatal(err) + } + for raw, want := range map[string]string{ + `{"module":"renamer","version":"1","config":{"font":{"type":"string","description":"The font.","required":true,"runtime":true,"renamed-from":"font-name"}}}`: "", + `{"module":"renamer","version":"1","config":{"font":{"type":"string","description":"The font.","required":true,"runtime":true,"renamed-from":"typeface"}}}`: `renamed from "typeface"`, + } { + head, err := ix.keepManifest(ctx, "renamer", raw) + if err != nil { + t.Fatal(err) + } + ix.JudgeAgain(ctx, "renamer", head) + if v := verdict(head); (want == "") != !v.Refused() || (want != "" && !strings.Contains(strings.Join(v.Problems, " "), want)) { + t.Errorf("the change %s: %+v, want refused naming %q", raw, v, want) + } + } // 1966/AC11: modules using state and none using logs but the foundation: logs counts 0, so a manifest of another // module using logs is refused and the foundation's is accepted. ix.OldSections = newMem() diff --git a/modules/mesh-delivery-indexer/judge/renames.go b/modules/mesh-delivery-indexer/judge/renames.go new file mode 100644 index 00000000..27221b36 --- /dev/null +++ b/modules/mesh-delivery-indexer/judge/renames.go @@ -0,0 +1,52 @@ +package judge + +import ( + "fmt" + "maps" + "slices" + + "git.novox.be/novox/mesh-sdk/go/catalogue" +) + +// RenameProblems judge a change's `renamed-from` against the trunk's manifest of the same module (DLV-I15; ADR 0609 ยง4, +// design 32 *A config value renamed*): the old name is a config value the trunk declares and the head does not; it is +// no secret and no key of the predefined list; the old and the new value have the same trust (declared, or the trust of +// the predefined key it takes its definition from). Each refusal names the module and the key. A module the trunk does +// not hold yet renames nothing. +func RenameProblems(module string, head, trunk catalogue.Manifest, predefined catalogue.PredefinedList) []string { + trust := func(d catalogue.SettingDeclaration) bool { + if d.Predefined != "" { + return predefined[d.Predefined].Trusted + } + return d.Trusted + } + var problems []string + for _, key := range slices.Sorted(maps.Keys(head.Settings)) { + d := head.Settings[key] + old := d.RenamedFrom + if old == "" { + continue + } + say := func(f string, a ...any) { + problems = append(problems, fmt.Sprintf("%s: the config value %q is renamed from %q, ", module, key, old)+fmt.Sprintf(f, a...)+" (design 32)") + } + _, ownSecret := trunk.OwnSecrets[old] + _, secret := trunk.Secrets[old] + was, declared := trunk.Settings[old] + _, still := head.Settings[old] + _, isPredefined := predefined[old] + switch { + case ownSecret || secret: + say("which is a secret; a secret is never renamed") + case isPredefined: + say("which is a key of the predefined list, not this module's own value") + case !declared: + say("which the trunk's manifest of %s does not declare", module) + case still: + say("and the change declares %q still: the old and the new name are not both declared", old) + case trust(was) != trust(d): + say("and the rename would change its trust (trusted %v on the trunk, %v here); a rename keeps a value's trust", trust(was), trust(d)) + } + } + return problems +} diff --git a/modules/mesh-delivery-indexer/judge/rules_test.go b/modules/mesh-delivery-indexer/judge/rules_test.go index 11a1fd69..983acb06 100644 --- a/modules/mesh-delivery-indexer/judge/rules_test.go +++ b/modules/mesh-delivery-indexer/judge/rules_test.go @@ -3392,6 +3392,35 @@ func TestConfigRefusesAnUntrustedValueFallingBackToATrustedKey(t *testing.T) { if _, err := parseRaw(t, m); err != nil { t.Fatalf("a trusted value falling back to a trusted key is refused: %v", err) } + // DLV-I15 (design 32 *A config value renamed*): a rename over change and trunk keeps a module's own value and its trust. + predefined := catalogue.PredefinedList{"font-size": {Type: "integer"}, "theme": {Type: "string", Trusted: true}} + str := catalogue.SettingDeclaration{Type: "string"} + renamed := func(from string, trusted bool) catalogue.SettingDeclaration { + d := str + d.RenamedFrom, d.Trusted = from, trusted + return d + } + trunk := catalogue.Manifest{Module: "c", Settings: map[string]catalogue.SettingDeclaration{"font-name": str, + "mode": {Type: "string", Trusted: true}, "look": {Type: "string", Predefined: "theme"}}, + OwnSecrets: catalogue.OwnSecrets{"token": {}}} + for _, c := range []struct { + name string + head map[string]catalogue.SettingDeclaration + refused string + }{ + {"a value the trunk declares, at the same trust", map[string]catalogue.SettingDeclaration{"font": renamed("font-name", false)}, ""}, + {"trust by the predefined key it took", map[string]catalogue.SettingDeclaration{"style": renamed("look", true)}, ""}, + {"a name the trunk does not declare", map[string]catalogue.SettingDeclaration{"font": renamed("typeface", false)}, `"font" is renamed from "typeface", which the trunk's manifest of c does not declare`}, + {"both names declared", map[string]catalogue.SettingDeclaration{"font": renamed("font-name", false), "font-name": str}, `declares "font-name" still`}, + {"a secret", map[string]catalogue.SettingDeclaration{"key": renamed("token", false)}, "which is a secret"}, + {"a predefined key", map[string]catalogue.SettingDeclaration{"size": renamed("font-size", false)}, "a key of the predefined list"}, + {"a change of trust", map[string]catalogue.SettingDeclaration{"kind": renamed("mode", false)}, "would change its trust"}, + } { + got := strings.Join(RenameProblems("c", catalogue.Manifest{Module: "c", Settings: c.head}, trunk, predefined), "\n") + if (c.refused == "") != (got == "") || !strings.Contains(got, c.refused) { + t.Errorf("%s: %q, want %q", c.name, got, c.refused) + } + } } // Rule 8: a placeholder naming an undeclared value.