Leave a module out of a walk when its check built it the same as what runs (hq issue 721)

A one-sentence change in a package mesh-builder imports but never reaches moved
build-agent. A merge check now compares each module it builds with the build the
mesh holds, the whole image without file times, labels or creation time, and a
walk is cut without a module every check of its pull requests found the same,
while the mesh still holds that build. A comparison that fails moves the module
as before.

internal/builder/check_builds.go

@@ -57,6 +57,11 @@ type CheckBuilt struct {
 	// Kept is the check whose build this check kept instead of building, its key unchanged (novox/hq issue 703):
 	// the verdict names that build, by key and digest, as that check's. Empty when this check built it.
 	Kept string
+	// Same says every artifact came out the same as the build the mesh holds of the module (novox/hq issue 721), and
+	// SameAs names, per artifact, the held reference it was compared with: the walk leaves the module where it is
+	// while the mesh still holds that build. Empty when it differs, or could not be compared.
+	Same   string
+	SameAs map[string]string
 }
 
 // skipsTheBuilds is why a check builds nothing for its merge; empty when it builds.
@@ -171,6 +176,18 @@ func makeForTheMerge(ctx context.Context, run Runner, spec CheckSpec, with Check
 				case result.ReuseKey == "":
 					built.NotBuilt = "it has no reuse key: " + result.NoKey
 				}
+				if built.NotBuilt == "" {
+					// **Compared with what runs** (novox/hq issue 721): a failed comparison says nothing, and the
+					// module moves as before.
+					same, against, err := sameAsHeld(ctx, run, dir, b.Module, result.Built, with.Held)
+					switch {
+					case err != nil:
+						say("build", "%s could not be compared with the build the mesh holds, so it moves: %v", b.Module, err)
+					case same != "":
+						built.Same, built.SameAs = same, against
+						say("build", "%s %s", b.Module, same)
+					}
+				}
 				out[i], results[i] = built, result
 			}(i, b, repository, at, filepath.Join(place, name))
 		}

internal/builder/same_program.go

@@ -0,0 +1,180 @@
+package builder
+
+import (
+	"archive/tar"
+	"context"
+	"crypto/sha256"
+	"encoding/hex"
+	"encoding/json"
+	"errors"
+	"fmt"
+	"io"
+	"os"
+	"path/filepath"
+	"sort"
+	"strings"
+
+	"github.com/novox/mesh-controller/internal/catalogue"
+)
+
+// A module moves only when its program changes (novox/hq issue 721). A merge check builds every module the delivery
+// plan moves; a module whose every artifact comes out the same as the build the mesh holds of it — the same files,
+// the same modes and owners, the same entrypoint and environment — is said unchanged, and the walk leaves it where it
+// is. **The whole image is compared, never its main binary alone**: a module whose image differs in any file moves.
+// What is ignored is only what a rebuild cannot help changing: each file's time, the image's labels and its creation
+// time. **A comparison that cannot be made says nothing**, and the module is built and moved as before.
+
+// sameAsHeld says whether every artifact made is the same as the build the mesh holds of the module: the sentence
+// that says so and, per artifact, the held reference it was compared with. Empty when any artifact differs, is not
+// an image, has no held build, or could not be compared.
+func sameAsHeld(ctx context.Context, run Runner, dir, module string, made []catalogue.Built,
+	held map[string]string) (string, map[string]string, error) {
+	if len(made) == 0 {
+		return "", nil, nil
+	}
+	against := map[string]string{}
+	for _, m := range made {
+		if m.Kind != "image" || m.Reference == "" {
+			return "", nil, nil
+		}
+		was := held[module+"/"+m.Name]
+		if was == "" {
+			return "", nil, nil
+		}
+		if digestPart(was) != "" && digestPart(was) == digestPart(m.Reference) {
+			against[m.Name] = was
+			continue
+		}
+		now, err := imageContent(ctx, run, dir, m.Reference)
+		if err != nil {
+			return "", nil, fmt.Errorf("%s's new image: %w", m.Name, err)
+		}
+		before, err := imageContent(ctx, run, dir, was)
+		if err != nil {
+			return "", nil, fmt.Errorf("%s's held image: %w", m.Name, err)
+		}
+		if now != before {
+			return "", nil, nil
+		}
+		against[m.Name] = was
+	}
+	var names []string
+	for name, ref := range against {
+		names = append(names, name+" "+shortRef(ref))
+	}
+	sort.Strings(names)
+	return "unchanged: its program is the same as running build " + strings.Join(names, ", "), against, nil
+}
+
+// digestPart is a reference's "sha256:…", empty when it is not pinned.
+func digestPart(ref string) string {
+	if i := strings.LastIndex(ref, "@"); i >= 0 {
+		return ref[i+1:]
+	}
+	return ""
+}
+
+// shortRef names a reference by its digest's first twelve characters when it has one.
+func shortRef(ref string) string {
+	if d := strings.TrimPrefix(digestPart(ref), "sha256:"); d != "" {
+		return twelve(d)
+	}
+	return ref
+}
+
+// imageContent is an image's content as one digest: its platform, its configuration without labels, and every file
+// of its filesystem by path, type, mode, owner, link, extended attributes and bytes — never by time.
+func imageContent(ctx context.Context, run Runner, dir, ref string) (string, error) {
+	if _, err := run(ctx, dir, "docker", "pull", "--quiet", ref); err != nil {
+		return "", fmt.Errorf("pulling %s: %w", ref, err)
+	}
+	config, err := run(ctx, dir, "docker", "image", "inspect", "--format",
+		"{{.Os}}/{{.Architecture}}/{{.Variant}} {{json .Config}}", ref)
+	if err != nil {
+		return "", fmt.Errorf("reading %s's configuration: %w", ref, err)
+	}
+	said, err := configContent(config)
+	if err != nil {
+		return "", fmt.Errorf("reading %s's configuration: %w", ref, err)
+	}
+	id, err := run(ctx, dir, "docker", "create", "--entrypoint", "/", ref)
+	if err != nil {
+		return "", fmt.Errorf("opening %s's filesystem: %w", ref, err)
+	}
+	id = strings.TrimSpace(id)
+	if id == "" {
+		return "", errors.New("the container engine named no container for " + ref)
+	}
+	defer func() { _, _ = run(context.WithoutCancel(ctx), dir, "docker", "rm", "--force", id) }()
+	if err := os.MkdirAll(dir, 0o700); err != nil {
+		return "", err
+	}
+	file := filepath.Join(dir, "content-"+twelve(id)+".tar")
+	defer os.Remove(file)
+	if _, err := run(ctx, dir, "docker", "export", "--output", file, id); err != nil {
+		return "", fmt.Errorf("reading %s's filesystem: %w", ref, err)
+	}
+	f, err := os.Open(file)
+	if err != nil {
+		return "", err
+	}
+	defer f.Close()
+	files, err := treeContent(f)
+	if err != nil {
+		return "", fmt.Errorf("reading %s's filesystem: %w", ref, err)
+	}
+	sum := sha256.Sum256([]byte(said + "\n" + files))
+	return "sha256:" + hex.EncodeToString(sum[:]), nil
+}
+
+// configContent is an image's platform and configuration as compared: Labels, the parent's id and the hostname the
+// builder ran under are left out — the first holds the commit, the others no part of what runs.
+func configContent(inspected string) (string, error) {
+	platform, raw, ok := strings.Cut(strings.TrimSpace(inspected), " ")
+	if !ok {
+		return "", fmt.Errorf("unexpected inspection %q", inspected)
+	}
+	var config map[string]json.RawMessage
+	if err := json.Unmarshal([]byte(raw), &config); err != nil {
+		return "", err
+	}
+	for _, k := range []string{"Labels", "Image", "Hostname", "Domainname"} {
+		delete(config, k)
+	}
+	out, err := json.Marshal(config) // map keys are sorted
+	if err != nil {
+		return "", err
+	}
+	return platform + " " + string(out), nil
+}
+
+// treeContent is a filesystem tarball's entries, sorted by path, each with everything but its times.
+func treeContent(r io.Reader) (string, error) {
+	tr := tar.NewReader(r)
+	var lines []string
+	for {
+		h, err := tr.Next()
+		if err == io.EOF {
+			break
+		}
+		if err != nil {
+			return "", err
+		}
+		sum := sha256.New()
+		if _, err := io.Copy(sum, tr); err != nil {
+			return "", err
+		}
+		var xattrs []string
+		for k, v := range h.PAXRecords {
+			if strings.HasPrefix(k, "SCHILY.xattr.") {
+				xattrs = append(xattrs, k+"="+hex.EncodeToString([]byte(v)))
+			}
+		}
+		sort.Strings(xattrs)
+		lines = append(lines, fmt.Sprintf("%q %c %o %d:%d %q %d,%d %d %x %s",
+			strings.TrimPrefix(h.Name, "./"), h.Typeflag, h.Mode, h.Uid, h.Gid, h.Linkname, h.Devmajor, h.Devminor,
+			h.Size, sum.Sum(nil), strings.Join(xattrs, ";")))
+	}
+	sort.Strings(lines)
+	return strings.Join(lines, "\n"), nil
+}

internal/builder/same_program_test.go

@@ -0,0 +1,126 @@
+package builder
+
+import (
+	"archive/tar"
+	"bytes"
+	"context"
+	"fmt"
+	"os"
+	"strings"
+	"testing"
+	"time"
+
+	"github.com/novox/mesh-controller/internal/catalogue"
+)
+
+// aFilesystem is a filesystem tarball of files, each written at the time given.
+func aFilesystem(t *testing.T, at time.Time, files map[string]string) []byte {
+	t.Helper()
+	var buf bytes.Buffer
+	tw := tar.NewWriter(&buf)
+	for _, name := range []string{"usr/local/bin/mesh-builder", "etc/apk/world"} {
+		body, ok := files[name]
+		if !ok {
+			continue
+		}
+		if err := tw.WriteHeader(&tar.Header{Name: name, Mode: 0o755, Size: int64(len(body)), ModTime: at,
+			Typeflag: tar.TypeReg}); err != nil {
+			t.Fatal(err)
+		}
+		if _, err := tw.Write([]byte(body)); err != nil {
+			t.Fatal(err)
+		}
+	}
+	if err := tw.Close(); err != nil {
+		t.Fatal(err)
+	}
+	return buf.Bytes()
+}
+
+// anEngine answers the container engine's commands for images by reference: each image's filesystem and labels.
+func anEngine(images map[string][]byte, labels map[string]string, failing string) Runner {
+	created := map[string]string{}
+	return func(_ context.Context, _ string, name string, args ...string) (string, error) {
+		joined := strings.Join(args, " ")
+		ref := args[len(args)-1]
+		if failing != "" && strings.Contains(joined, failing) {
+			return "", fmt.Errorf("the engine refused %s", joined)
+		}
+		switch {
+		case args[0] == "pull", args[0] == "rm":
+			return "", nil
+		case args[0] == "image":
+			return fmt.Sprintf(`linux/amd64/ {"Entrypoint":["/usr/local/bin/mesh-builder"],"Labels":{"commit":%q}}`,
+				labels[ref]), nil
+		case args[0] == "create":
+			id := fmt.Sprintf("container%04d", len(created))
+			created[id] = ref
+			return id + "\n", nil
+		case args[0] == "export":
+			return "", os.WriteFile(args[2], images[created[ref]], 0o600)
+		}
+		return "", fmt.Errorf("unexpected %s %s", name, joined)
+	}
+}
+
+// TestAModuleWhoseProgramDidNotChangeIsSaidUnchanged is novox/hq issue 721: mesh-controller #335 changed one sentence
+// in a package mesh-builder imports and never reaches, and build-agent moved. Built again, its image differs only in
+// the files' times and its commit label, and the check says it is the same as the build the mesh holds.
+func TestAModuleWhoseProgramDidNotChangeIsSaidUnchanged(t *testing.T) {
+	held := map[string]string{"build-agent/agent": "store/build-agent@sha256:aaaa"}
+	made := []catalogue.Built{{Name: "agent", Kind: "image", Reference: "scratch/build-agent@sha256:bbbb"}}
+	files := map[string]string{"usr/local/bin/mesh-builder": "the same program", "etc/apk/world": "docker-cli\ngit\n"}
+	images := map[string][]byte{
+		"store/build-agent@sha256:aaaa":   aFilesystem(t, time.Unix(1000, 0), files),
+		"scratch/build-agent@sha256:bbbb": aFilesystem(t, time.Unix(2000, 0), files),
+	}
+	labels := map[string]string{"store/build-agent@sha256:aaaa": "a5500f73", "scratch/build-agent@sha256:bbbb": "c2ec0a60"}
+	same, against, err := sameAsHeld(context.Background(), anEngine(images, labels, ""), t.TempDir(), "build-agent",
+		made, held)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if !strings.HasPrefix(same, "unchanged: its program is the same as running build agent") ||
+		against["agent"] != held["build-agent/agent"] {
+		t.Fatalf("an image the same but for its times and labels is not said unchanged: %q %v", same, against)
+	}
+
+	// **Any file differing moves it**, not only its program.
+	changed := map[string]string{"usr/local/bin/mesh-builder": "the same program", "etc/apk/world": "docker-cli\ngit\ncurl\n"}
+	images["scratch/build-agent@sha256:bbbb"] = aFilesystem(t, time.Unix(2000, 0), changed)
+	if same, _, err := sameAsHeld(context.Background(), anEngine(images, labels, ""), t.TempDir(), "build-agent", made,
+		held); err != nil || same != "" {
+		t.Fatalf("an image with another file is said unchanged: %q %v", same, err)
+	}
+
+	// **A comparison that fails says nothing**, and the module moves.
+	images["scratch/build-agent@sha256:bbbb"] = aFilesystem(t, time.Unix(2000, 0), files)
+	if same, _, err := sameAsHeld(context.Background(), anEngine(images, labels, "export"), t.TempDir(), "build-agent",
+		made, held); err == nil || same != "" {
+		t.Fatalf("a comparison that failed said %q, %v", same, err)
+	}
+
+	// No held build, or an artifact that is not an image: nothing compared.
+	if same, _, _ := sameAsHeld(context.Background(), anEngine(images, labels, ""), t.TempDir(), "build-agent", made,
+		nil); same != "" {
+		t.Fatalf("a module the mesh holds no build of is said unchanged: %q", same)
+	}
+	archive := []catalogue.Built{{Name: "agent", Kind: "archive", Reference: "scratch/build-agent@sha256:bbbb"}}
+	if same, _, _ := sameAsHeld(context.Background(), anEngine(images, labels, ""), t.TempDir(), "build-agent",
+		archive, held); same != "" {
+		t.Fatalf("an archive is said unchanged without being compared: %q", same)
+	}
+}
+
+// TestAnImageWithAnotherEntrypointIsNotTheSame: the configuration counts, but for its labels.
+func TestAnImageWithAnotherEntrypointIsNotTheSame(t *testing.T) {
+	a, err := configContent(`linux/amd64/ {"Entrypoint":["/a"],"Labels":{"commit":"1"}}`)
+	if err != nil {
+		t.Fatal(err)
+	}
+	b, _ := configContent(`linux/amd64/ {"Labels":{"commit":"2"},"Entrypoint":["/a"]}`)
+	c, _ := configContent(`linux/amd64/ {"Entrypoint":["/b"],"Labels":{"commit":"1"}}`)
+	if a != b || a == c {
+		t.Fatalf("labels count, or the entrypoint does not: %q %q %q", a, b, c)
+	}
+}