From 1963d84a7a453436e8ada54c500310ceb2b13899 Mon Sep 17 00:00:00 2001 From: jochen Date: Thu, 22 Oct 2026 18:13:17 +0200 Subject: [PATCH] =?UTF-8?q?Refuse,=20in=20the=20one=20function=20every=20a?= =?UTF-8?q?sk=20goes=20through,=20an=20ask=20of=20build-agent=20for=20any?= =?UTF-8?q?=20module=20but=20the=20builder's=20own=20(issue=201851,=20A12,?= =?UTF-8?q?=20ADR=200453=20=C2=A75)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- cmd/mesh-controller/builders_own_test.go | 61 ++++++++++++++++++++++++ cmd/mesh-controller/release_plan.go | 49 +++++++++++++++++++ 2 files changed, 110 insertions(+) create mode 100644 cmd/mesh-controller/builders_own_test.go diff --git a/cmd/mesh-controller/builders_own_test.go b/cmd/mesh-controller/builders_own_test.go new file mode 100644 index 00000000..3c2d717b --- /dev/null +++ b/cmd/mesh-controller/builders_own_test.go @@ -0,0 +1,61 @@ +package main + +import ( + "context" + "errors" + "strings" + "testing" +) + +// A12 (ADR 0453 §5, ADR 0498 §7; issue 1851): the one function every ask goes through refuses any module but the +// builder's own, naming the builder's key it is taken from instead; the builder's own module passes the refusal. +func TestBuildAgentIsAskedForTheBuildersOwnModuleAlone(t *testing.T) { + was := askedModuleOf + t.Cleanup(func() { askedModuleOf = was }) + askedModuleOf = func(_ context.Context, _ buildSource, path string) (string, error) { + return strings.TrimPrefix(path, "modules/"), nil + } + _, err := askABuild(t.Context(), buildSource{Repository: "novox/mesh-catalog", Seat: "git"}, "modules/sudo", "main") + if !errors.Is(err, errNotTheBuildersOwn) || !strings.Contains(err.Error(), "sudo is taken from the builder's done key") || + !strings.Contains(err.Error(), "mesh-delivery-builder_builds") { + t.Fatalf("an ask of another module: %v", err) + } + if err := onlyTheBuildersOwn(t.Context(), buildSource{Repository: "novox/mesh-delivery", Seat: "git"}, + "modules/mesh-delivery-builder"); err != nil { + t.Fatalf("the builder's own module is refused: %v", err) + } + askedModuleOf = func(context.Context, buildSource, string) (string, error) { return "", errors.New("no store") } + if err := onlyTheBuildersOwn(t.Context(), buildSource{Repository: "novox/x"}, ""); !errors.Is(err, errNotTheBuildersOwn) { + t.Fatalf("an ask whose module cannot be read went on: %v", err) + } +} + +// A12: a walk of the builder's own module and one other asks build-agent for the first alone; the other is taken from +// its key. +func TestAWalkAsksBuildAgentForTheBuildersOwnModuleAlone(t *testing.T) { + var asked []string + was := askABuild + askABuild = func(_ context.Context, _ buildSource, path, _ string) (string, error) { + asked = append(asked, path) + return "build-1", nil + } + t.Cleanup(func() { askABuild = was }) + p, byName := aWalkOfOne("sudo") + p.Tiers = [][]string{{"mesh-delivery-builder", "sudo"}} + byName["mesh-delivery-builder"] = byName["sudo"] + e := byName["mesh-delivery-builder"] + e.Source.Repository, e.Source.Path = "novox/mesh-delivery", "modules/mesh-delivery-builder" + byName["mesh-delivery-builder"] = e + for _, m := range p.Tiers[0] { + askModule(t.Context(), p, m, byName) + } + if len(asked) != 1 || asked[0] != "modules/mesh-delivery-builder" { + t.Fatalf("build-agent was asked for %v, want the builder's own module alone", asked) + } + if s := p.Modules["mesh-delivery-builder"]; s.Key || s.Build != "build-1" { + t.Fatalf("the builder's own module is not asked: %+v", s) + } + if s := p.Modules["sudo"]; !s.Key || s.Build != "" { + t.Fatalf("the other module is not taken from its key: %+v", s) + } +} diff --git a/cmd/mesh-controller/release_plan.go b/cmd/mesh-controller/release_plan.go index 8cc11981..1238d769 100644 --- a/cmd/mesh-controller/release_plan.go +++ b/cmd/mesh-controller/release_plan.go @@ -6,6 +6,7 @@ import ( "errors" "flag" "fmt" + sdkdelivery "git.novox.be/novox/mesh-sdk/go/delivery" "os" "slices" "sort" @@ -410,11 +411,59 @@ var askABuild func(ctx context.Context, source buildSource, path, ref string) (s func init() { askABuild = func(ctx context.Context, source buildSource, path, ref string) (string, error) { + // **Only the builder's own module is asked of build-agent** (novox/hq ADR 0453 §5, ADR 0498 §7; issue 1851, + // A12): refused here, in the one function every ask goes through, whoever calls it. + if err := onlyTheBuildersOwn(ctx, source, path); err != nil { + return "", err + } // Kept by each asker with its own name once the ask is made (novox/hq issue 325). return buildOneAsked(ctx, source, path, ref, 0, false, "") } } +// errNotTheBuildersOwn says an ask of build-agent was refused: from R4e build-agent builds the builder's own module +// alone, and every other is taken from the builder's done key. +var errNotTheBuildersOwn = errors.New("build-agent is asked for the builder's own module alone (ADR 0453 §5)") + +// askedModuleOf is the module the catalogue builds from a source and path, empty when it builds none from it. A variable +// so a test needs no store. +var askedModuleOf = func(ctx context.Context, source buildSource, path string) (string, error) { + open, err := openStores(ctx) + if err != nil { + return "", err + } + defer open.Close() + entries, err := open.inventory.Catalogued(ctx) + if err != nil { + return "", err + } + forms := newSourceForms(ctx, open.inventory) + for _, e := range entries { + if forms.same(e.Source.Repository, e.Source.Seat, source.Repository, source.Seat) && + strings.Trim(e.Source.Path, "/") == strings.Trim(path, "/") { + return e.Manifest.Module, nil + } + } + return "", nil +} + +// onlyTheBuildersOwn refuses an ask of any module but the builder's own, naming the key it is taken from instead. +func onlyTheBuildersOwn(ctx context.Context, source buildSource, path string) error { + module, err := askedModuleOf(ctx, source, path) + if err != nil { + return fmt.Errorf("%w: which module %s at %q is cannot be read, so nothing is asked: %v", errNotTheBuildersOwn, + source, orRoot(path), err) + } + if isBuildersOwn(module) { + return nil + } + if module == "" { + module = fmt.Sprintf("%s at %q", source, orRoot(path)) + } + return fmt.Errorf("%w: %s is taken from the builder's done key of its fingerprint in %s (ADR 0501), and no build "+ + "of it is asked", errNotTheBuildersOwn, module, sdkdelivery.BuilderBuildsBucket) +} + // pullsOfWalk are the pull requests of the merges a walk answers, each once: what its end lets go of (forgetChecksOfWalk) // and what a check of one was (same_program.go). func pullsOfWalk(p *inventory.Plan) []link.PullRef {