From 97895851f8e41470f63ca5552d010d3617d0cd08 Mon Sep 17 00:00:00 2001 From: jochen Date: Thu, 22 Oct 2026 17:31:40 +0200 Subject: [PATCH] =?UTF-8?q?Check=20each=20done=20artifact's=20reference=20?= =?UTF-8?q?against=20its=20kind,=20and=20say=20when=20a=20key=20can=20be?= =?UTF-8?q?=20delivered=20(issue=201587,=20S13,=20ADR=200498=20=C2=A71=20?= =?UTF-8?q?=C2=A75=20=C2=A76=20=C2=A78)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A bundle or archive names its artifact-store blob, an image or upstream copy the registry's reference to its digest (L1), a package name@version; Referenced names the artifacts without one, since the controller delivers only from a key with all. --- go/delivery/builder.go | 70 +++++++++++++++++++++++- go/delivery/builder_reference_test.go | 76 +++++++++++++++++++++++++++ go/delivery/builder_test.go | 2 +- 3 files changed, 145 insertions(+), 3 deletions(-) create mode 100644 go/delivery/builder_reference_test.go diff --git a/go/delivery/builder.go b/go/delivery/builder.go index 205e5d5..85fbbe8 100644 --- a/go/delivery/builder.go +++ b/go/delivery/builder.go @@ -63,8 +63,18 @@ func ParseBuilderBuildKey(key string) (module, fingerprint string, err error) { return module, fingerprint, nil } -// BuiltArtifact is one artifact of a done build: its name in the manifest, its kind, its content digest and, for an -// image, the reference it was pushed to. +// BuiltArtifact is one artifact of a done build: its name in the manifest, its kind, its content digest (`sha256:` and +// 64 hex digits) and the reference that names where it is (novox/hq ADR 0498 clauses 1, 5 and 6; the lead's decision +// L1 of R4e), by kind: +// +// bundle, archive artifact-store:////blobs/ the blob pushed to the artifact store +// image, upstream //@ the image registry's reference, which +// artifact-store:///@ catalogue.Recorded keeps without its address +// package @ the version published from the trunk +// +// is Name and is Digest. A reference is empty for a candidate's image or package, which is never +// pushed or published (ADR 0498 clause 6), and in a key done before R4e; the controller delivers only from a key whose +// every artifact names one (Referenced, clause 8). A kind left empty (a key written before kinds) is not checked. type BuiltArtifact struct { Name string `json:"name"` Kind string `json:"kind,omitempty"` @@ -72,6 +82,59 @@ type BuiltArtifact struct { Reference string `json:"reference,omitempty"` } +const artifactStore = "artifact-store://" + +// referenceProblem says what is wrong with a's reference for its kind, or "". +func (a BuiltArtifact) referenceProblem() string { + r := a.Reference + if r == "" || a.Kind == "" { + return "" + } + switch a.Kind { + case "bundle", "archive": + path, ok := strings.CutPrefix(r, artifactStore) + module, rest, _ := strings.Cut(path, "/") + if !ok || module == "" || rest != a.Name+"/blobs/"+a.Digest { + return "is not artifact-store:///" + a.Name + "/blobs/" + a.Digest + } + case "image", "upstream": + path, digest, ok := strings.Cut(r, "@") + if !ok || digest != a.Digest { + return "does not name its digest " + a.Digest + } + var host, module, name string + if rest, kept := strings.CutPrefix(path, artifactStore); kept { + module, name, _ = strings.Cut(rest, "/") + host = "kept" + } else { + parts := strings.Split(path, "/") + if len(parts) == 3 { + host, module, name = parts[0], parts[1], parts[2] + } + } + if host == "" || !(host == "kept" || host == "localhost" || strings.ContainsAny(host, ".:")) || module == "" || name != a.Name { + return "is not //" + a.Name + "@" + a.Digest + } + case "package": + at := strings.LastIndex(r, "@") + if at <= 0 || at == len(r)-1 || r[:at] == "@" { + return "is not @" + } + } + return "" +} + +// Referenced says whether b is done and every artifact names where it is, which the controller delivers from (novox/hq +// ADR 0498 clause 8), and names the artifacts that name no reference. +func (b BuilderBuild) Referenced() (ok bool, missing []string) { + for _, a := range b.Artifacts { + if a.Reference == "" { + missing = append(missing, a.Name) + } + } + return b.State == BuildDone && len(b.Artifacts) > 0 && len(missing) == 0, missing +} + // ToolchainUsed is what a build used for one language (ADR 0405 clauses 2 and 7): the installed toolchain's version and // the build module's recipe version. type ToolchainUsed struct { @@ -120,6 +183,9 @@ func (b BuilderBuild) check() error { if a.Digest == "" { return fmt.Errorf("delivery: the artifact %q of the build has no digest", a.Name) } + if p := a.referenceProblem(); p != "" { + return fmt.Errorf("delivery: the reference %q of the %s %q %s", a.Reference, a.Kind, a.Name, p) + } } switch b.State { case BuildQueued, BuildBuilding: diff --git a/go/delivery/builder_reference_test.go b/go/delivery/builder_reference_test.go new file mode 100644 index 0000000..d277440 --- /dev/null +++ b/go/delivery/builder_reference_test.go @@ -0,0 +1,76 @@ +package delivery + +import ( + "strings" + "testing" +) + +func artifactOf(name, kind, digest, reference string) BuiltArtifact { + return BuiltArtifact{Name: name, Kind: kind, Digest: digest, Reference: reference} +} + +// A done key names where each artifact is (novox/hq ADR 0498 clauses 1, 5 and 6; the lead's decision L1 of R4e): a +// bundle or an archive by its artifact-store blob, an image or an upstream copy by the registry's reference to its +// digest (or the kept artifact-store form), a package by name and version. A reference of the wrong form for its kind, +// or naming another digest or artifact, is refused, naming the artifact. +func TestADoneKeyNamesEachArtifactsReferenceInTheFormOfItsKind(t *testing.T) { + d := "sha256:" + digestOf('6') + good := []BuiltArtifact{ + artifactOf("code", "bundle", d, "artifact-store://mail/code/blobs/"+d), + artifactOf("files", "archive", d, "artifact-store://mail/files/blobs/"+d), + artifactOf("server", "image", d, "10.0.0.1:5000/mail/server@"+d), + artifactOf("server", "image", d, "artifact-store://mail/server@"+d), + artifactOf("nats", "upstream", d, "registry.example/nats/nats@"+d), + artifactOf("lib", "package", d, "@novox/mesh-sdk@0.1.15"), + artifactOf("lib", "package", d, "left-pad@1.3.0"), + artifactOf("old", "", d, "anything at all"), + } + for _, a := range good { + b := doneBuild() + b.Artifacts = []BuiltArtifact{a} + if _, err := EncodeBuilderBuild(b); err != nil { + t.Errorf("%+v was refused: %v", a, err) + } + } + other := "sha256:" + digestOf('7') + bad := []BuiltArtifact{ + artifactOf("code", "bundle", d, "artifact-store://mail/code/blobs/"+other), + artifactOf("code", "bundle", d, "artifact-store://mail/other/blobs/"+d), + artifactOf("code", "bundle", d, "10.0.0.1:5000/mail/code@"+d), + artifactOf("server", "image", d, "10.0.0.1:5000/mail@"+d), + artifactOf("server", "image", d, "10.0.0.1:5000/mail/server@"+other), + artifactOf("server", "image", d, "mail/server@"+d), + artifactOf("server", "image", d, "artifact-store://mail/server/blobs/"+d), + artifactOf("lib", "package", d, "@novox/mesh-sdk"), + artifactOf("lib", "package", d, "0.1.15"), + } + for _, a := range bad { + b := doneBuild() + b.Artifacts = []BuiltArtifact{a} + if _, err := EncodeBuilderBuild(b); err == nil || !strings.Contains(err.Error(), `"`+a.Name+`"`) { + t.Errorf("%+v: %v, wanted a refusal naming the artifact", a, err) + } + } +} + +// The controller delivers only from a done key whose every artifact names a reference (ADR 0498 clause 8): Referenced +// says so and names the artifacts without one. +func TestReferencedNamesTheArtifactsWithoutAReference(t *testing.T) { + d := "sha256:" + digestOf('6') + b := doneBuild() + b.Artifacts = []BuiltArtifact{ + artifactOf("code", "bundle", d, "artifact-store://mail/code/blobs/"+d), + artifactOf("server", "image", d, ""), + } + if ok, missing := b.Referenced(); ok || len(missing) != 1 || missing[0] != "server" { + t.Fatalf("Referenced = %v, %v; want false, [server]", ok, missing) + } + b.Artifacts[1].Reference = "10.0.0.1:5000/mail/server@" + d + if ok, missing := b.Referenced(); !ok || len(missing) != 0 { + t.Fatalf("Referenced = %v, %v; want true", ok, missing) + } + if ok, _ := (BuilderBuild{State: BuildQueued}).Referenced(); ok { + t.Fatal("a queued key is referenced") + } +} + diff --git a/go/delivery/builder_test.go b/go/delivery/builder_test.go index 2731819..ebe3003 100644 --- a/go/delivery/builder_test.go +++ b/go/delivery/builder_test.go @@ -15,7 +15,7 @@ func doneBuild() BuilderBuild { Manifest: digestOf('4'), Artifacts: []BuiltArtifact{ {Name: "code", Kind: "bundle", Digest: "sha256:" + digestOf('5')}, - {Name: "server", Kind: "image", Digest: "sha256:" + digestOf('6'), Reference: "registry.example/mail@sha256:" + digestOf('6')}, + {Name: "server", Kind: "image", Digest: "sha256:" + digestOf('6'), Reference: "registry.example/mail/server@sha256:" + digestOf('6')}, }, Toolchains: map[string]ToolchainUsed{ "go": {Version: "1.26.0", Recipe: "3"},