fix(build): a published release is adopted unless its code changed

docs/commands.md

@@ -4042,14 +4042,28 @@ two environments, is one image. Before a pushed build of a released version
 (an exact `x.y.z`, never the unversioned `0.0.0`), the render asks the registry
 for `<image>:<version>`. Every build records its input digest on the image as
 the label `org.codefly.build.inputs`, and the published image is adopted —
-nothing is built — when that label equals this build's inputs. A published
-image recording **other** inputs refuses the build, and the fix is a new
-release, never a rebuild under the old version. Every build also records the
-agent that emitted its recipe as `org.codefly.build.agent`, so the refusal says
-which input differs: another agent built the published image (a tag that holds
-a build the release does not name), or the same agent did and the service's
-code or a base image changed without its version moving. One recording no
-inputs (pushed before builds recorded them) is built again. With a registry
+nothing is built — when that label equals this build's inputs. Beside it, every
+build records each class of its inputs: `org.codefly.build.inputs.code` (the
+build context and the Go module manifests: the service's code),
+`org.codefly.build.inputs.recipe` (the recipe tree its agent emitted, the
+Dockerfile and the build invocation) and `org.codefly.build.inputs.bases` (the
+resolved base images), plus the agent that emitted the recipe as
+`org.codefly.build.agent`. A published image recording **other** inputs is
+judged by its code, because the refusal exists to catch a version that did not
+move with its code:
+
+| The published image | The render |
+|---|---|
+| records this build's code; its recipe or bases differ (another checkout, another builder, a re-pushed base) | adopts it — it is the release's build — and warns naming the class that differs |
+| records other code | refuses: the code changed without its version moving, and the fix is a new release, never a rebuild under the old version |
+| records another agent than the release pins | refuses: the tag holds a build the release does not name (made under an [agent override](#moving-the-agent-version-of-composed-services) by a CLI that pushed it under the release tag) |
+| records only the aggregate (labelled before the classes existed) | adopts it with a warning that its code was not compared; `--rebuild` builds it again instead |
+
+Adopting is the default for an image that cannot be compared because the two
+alternatives are worse: refusing blocks every render of every module on a cell
+that holds one such image (a render needs every workload's digest), and
+rebuilding overwrites a release wherever tags are mutable. One recording no
+inputs at all (pushed before builds recorded them) is built again. With a registry
 Only the registry answering that the tag does not exist lets a released version
 be built: any other failed read (an authorization refusal, a timeout, an
 unreachable registry) stops the render, because building over it would

pkg/orchestration/build_plan.go

@@ -19,6 +19,7 @@ import (
 	"github.com/codefly-dev/cli/pkg/environments"
 	coreservices "github.com/codefly-dev/core/agents/services"
 	builderv0 "github.com/codefly-dev/core/generated/go/codefly/services/builder/v0"
+	"github.com/codefly-dev/core/resources"
 	"github.com/codefly-dev/core/wool"
 )
 
@@ -193,22 +194,17 @@ func (b *Builder) decideImageReuse(
 		if err == nil && digest != "" {
 			// The version alone is not proof: a service whose version was not moved
 			// with its code would hand back another release's image. The published
-			// image records the inputs it was built from, and only those inputs
-			// adopt it.
-			labels, _ := resolveImageBuildLabels(ctx, recipe.GetImage(), probeBuilder)
-			published := labels[buildInputsLabel]
-			switch {
-			case identity.Key != "" && published == identity.Key:
-				decision.entry = &imageCacheEntry{
-					Schema: imageCacheSchema, Key: identity.Key, Service: b.instance.Unique(), Recipe: recipe.GetName(),
-					Image: recipe.GetImage(), Pushed: true, Digest: digest, Platforms: recipe.GetPlatforms(),
-				}
+			// image records the inputs it was built from: the same inputs adopt it,
+			// and other inputs adopt it only when its code is this build's code
+			// (publishedReleaseVerdict) — what else differs is the agent's or the
+			// registry's, and the release's build is the one already published.
+			adopt, err := b.judgePublishedRelease(ctx, w, recipe, decision, digest, probeBuilder)
+			if err != nil {
+				return decision, err
+			}
+			if adopt != nil {
+				decision.entry = adopt
 				return decision, nil
-			case identity.Key != "" && published != "":
-				return decision, w.Wrap(publishedFromOtherInputs(recipe.GetImage(), b.instance.Service.Agent, labels[buildAgentLabel]))
-			default:
-				w.Warn("the published image records no build inputs; building it again",
-					wool.Field("image", recipe.GetImage()), wool.Field("recipe", recipe.GetName()))
 			}
 		}
 	}
@@ -229,6 +225,39 @@ func (b *Builder) decideImageReuse(
 	return decision, nil
 }
 
+// judgePublishedRelease reads the build labels of a released tag the registry
+// holds at digest and returns the entry that adopts it, nothing when it is to
+// be built again (it records no inputs), or the refusal.
+func (b *Builder) judgePublishedRelease(ctx context.Context, w *wool.Wool, recipe *builderv0.DockerBuildRecipe, decision imageReuse, digest, probeBuilder string) (*imageCacheEntry, error) {
+	identity := decision.identity
+	labels, _ := resolveImageBuildLabels(ctx, recipe.GetImage(), probeBuilder)
+	published := labels[buildInputsLabel]
+	adopted := &imageCacheEntry{
+		Schema: imageCacheSchema, Key: identity.Key, Service: b.instance.Unique(), Recipe: recipe.GetName(),
+		Image: recipe.GetImage(), Pushed: true, Digest: digest, Platforms: recipe.GetPlatforms(),
+	}
+	switch {
+	case identity.Key != "" && published == identity.Key:
+		return adopted, nil
+	case identity.Key != "" && published != "" && decision.image != "":
+		// A build-qualified tag names one identity; it is never adopted from
+		// other inputs.
+		return nil, w.Wrap(publishedFromOtherInputs(recipe.GetImage(), b.instance.Service.Agent, labels[buildAgentLabel]))
+	case identity.Key != "" && published != "":
+		warning, refusal := publishedReleaseVerdict(recipe.GetImage(), identity, b.instance.Service.Agent, labels)
+		if refusal != nil {
+			return nil, w.Wrap(refusal)
+		}
+		w.Warn(warning, wool.Field("image", recipe.GetImage()), wool.Field("recipe", recipe.GetName()),
+			wool.Field("published_inputs", published), wool.Field("identity", identity.Key))
+		return adopted, nil
+	default:
+		w.Warn("the published image records no build inputs; building it again",
+			wool.Field("image", recipe.GetImage()), wool.Field("recipe", recipe.GetName()))
+		return nil, nil
+	}
+}
+
 // placeholderProxies stands in for the Go module proxies and git source mirrors
 // a recipe declares before they are fetched. normalizedBuildxArgs rewrites every fetched proxy
 // path to this same placeholder, so an identity computed over these equals the
@@ -437,7 +466,7 @@ func (b *Builder) buildRecipe(
 
 	args, err := cachedBuildxArgs(recipe, dockerfile, contextDir, shouldPush, multiArch, metadataFile, builderName, cache, private)
 	if err == nil {
-		args = withBuildAgentLabel(withBuildInputsLabel(args, identity.Key), b.instance.Service.Agent)
+		args = withBuildAgentLabel(withBuildInputsLabel(args, identity), b.instance.Service.Agent)
 		if shouldPush {
 			args = withSourceLabel(args, b.world.Env, workspaceSourceRepository(ctx, b.world.Workspace.Dir()), multiArch)
 		}
@@ -1058,14 +1087,79 @@ var releasedVersion = regexp.MustCompile(`^(0|[1-9][0-9]*)\.(0|[1-9][0-9]*)\.(0|
 // inputs (decideImageReuse).
 const buildInputsLabel = "org.codefly.build.inputs"
 
-// withBuildInputsLabel records the build's input identity on the image. It is
-// added to the build's own argv after the identity was computed from it, so the
-// label is never an input to the identity it records.
-func withBuildInputsLabel(args []string, identityKey string) []string {
-	if identityKey == "" {
+// The per-class labels a build records beside the aggregate (buildClasses), so
+// a published release can be compared by what moved: its code, or only what
+// the agent and the registry contribute.
+const (
+	buildCodeLabel   = buildInputsLabel + ".code"
+	buildRecipeLabel = buildInputsLabel + ".recipe"
+	buildBasesLabel  = buildInputsLabel + ".bases"
+)
+
+// buildLabels is every label resolveImageBuildLabels reads back.
+var buildLabels = []string{buildInputsLabel, buildCodeLabel, buildRecipeLabel, buildBasesLabel, buildAgentLabel}
+
+// withBuildInputsLabel records the build's input identity on the image: the
+// aggregate and each class. It is added to the build's own argv after the
+// identity was computed from it, so no label is an input to what it records.
+func withBuildInputsLabel(args []string, identity imageIdentity) []string {
+	if identity.Key == "" {
 		return args
 	}
-	return withBuildFlags(args, "--label", buildInputsLabel+"="+identityKey)
+	flags := []string{"--label", buildInputsLabel + "=" + identity.Key}
+	if classes := identity.Classes; classes != nil {
+		for _, class := range [][2]string{{buildCodeLabel, classes.Code}, {buildRecipeLabel, classes.Recipe}, {buildBasesLabel, classes.Bases}} {
+			if class[1] != "" {
+				flags = append(flags, "--label", class[0]+"="+class[1])
+			}
+		}
+	}
+	return withBuildFlags(args, flags...)
+}
+
+// publishedReleaseVerdict decides a released tag whose image records other
+// inputs than this build's. The refusal exists to catch a version that did not
+// move with its code, so only a code difference refuses; the published image is
+// otherwise the release's build — rebuilding it would overwrite a release, and
+// refusing would block every render of every cell holding it — and is adopted
+// with a warning naming what differs. An agent recorded as other than the one
+// the release pins still refuses: that tag holds a build made under an agent
+// override (override_build.go), which the release does not name.
+//
+// An image labelled before the classes were recorded carries only the
+// aggregate, so its code cannot be compared: it is adopted, saying so, and
+// --rebuild is the operator's way to build it again instead.
+func publishedReleaseVerdict(image string, identity imageIdentity, agent *resources.Agent, labels map[string]string) (string, error) {
+	const release = "release a new version (codefly publish) instead of rebuilding a published one"
+	publishedAgent := labels[buildAgentLabel]
+	if agent != nil && publishedAgent != "" && publishedAgent != agent.Identifier() {
+		return "", publishedFromOtherInputs(image, agent, publishedAgent)
+	}
+	classes := identity.Classes
+	if classes == nil {
+		classes = &buildClasses{}
+	}
+	code := labels[buildCodeLabel]
+	switch {
+	case code == "":
+		return "the published image predates per-class build labels, so its code was not compared with this build's; " +
+			"adopting it as the release's build (--rebuild builds it again)", nil
+	case code != classes.Code:
+		return "", fmt.Errorf("%s is already published from other code: the service's build context or Go module manifests differ from the published build's, "+
+			"so its code changed without its version moving; %s", image, release)
+	}
+	var differs []string
+	if labels[buildRecipeLabel] != classes.Recipe {
+		differs = append(differs, "the build recipe its agent emitted or the build invocation")
+	}
+	if labels[buildBasesLabel] != classes.Bases {
+		differs = append(differs, "the resolved base images")
+	}
+	if len(differs) == 0 {
+		differs = append(differs, "an input no class records")
+	}
+	return "the published image was built from this build's code, and " + strings.Join(differs, " and ") +
+		" differ; adopting it as the release's build", nil
 }
 
 // withBuildFlags inserts flags right after the `build` subcommand of a buildx
@@ -1122,7 +1216,7 @@ var workspaceSourceRepository = func(ctx context.Context, dir string) string {
 }
 
 // resolveImageBuildLabels reads the build labels every platform of a published
-// image agrees on (buildInputsLabel, buildAgentLabel); a label a platform lacks
+// image agrees on (buildLabels); a label a platform lacks
 // or disagrees on is absent. A seam so a test needs no registry.
 var resolveImageBuildLabels = func(ctx context.Context, reference, builderName string) (map[string]string, error) {
 	probe, cancel := context.WithTimeout(ctx, imageRegistryProbeTimeout)
@@ -1152,7 +1246,7 @@ func buildLabelsFromImageConfig(raw []byte) map[string]string {
 	labels := map[string]string{}
 	var single config
 	if err := json.Unmarshal(raw, &single); err == nil && single.Config.Labels != nil {
-		for _, key := range []string{buildInputsLabel, buildAgentLabel} {
+		for _, key := range buildLabels {
 			if value := single.Config.Labels[key]; value != "" {
 				labels[key] = value
 			}
@@ -1163,7 +1257,7 @@ func buildLabelsFromImageConfig(raw []byte) map[string]string {
 	if err := json.Unmarshal(raw, &platforms); err != nil || len(platforms) == 0 {
 		return labels
 	}
-	for _, key := range []string{buildInputsLabel, buildAgentLabel} {
+	for _, key := range buildLabels {
 		value := ""
 		for _, platform := range platforms {
 			label := platform.Config.Labels[key]

pkg/orchestration/image_cache.go

@@ -229,6 +229,54 @@ func (cache *imageBuildCache) store(ctx context.Context, entry *imageCacheEntry)
 type imageIdentity struct {
 	Key         string
 	ContextTree string
+	// Classes digests the same inputs as Key, split by what moves them, so a
+	// published release's build can be compared class by class (buildClasses).
+	Classes *buildClasses
+}
+
+// buildClasses splits an identity's inputs by what moves them: Code is the
+// service's own bytes (its build context and the Go module manifests the
+// prefetch reads), Recipe is what its agent emitted and how it is invoked (the
+// recipe tree, Dockerfile and buildx command), and Bases is the resolved base
+// images. Each is recorded on the image as a label of its own beside the
+// aggregate, never folded into Key: Key is unchanged, so an image labelled
+// before the classes existed still matches its own inputs.
+type buildClasses struct {
+	Code   string
+	Recipe string
+	Bases  string
+}
+
+// classDigest is the digest of one class's inputs, under the cache schema so a
+// schema move separates the classes the same way it separates the aggregate.
+func classDigest(class string, inputs any) (string, error) {
+	payload, err := json.Marshal(struct {
+		Schema int    `json:"schema"`
+		Class  string `json:"class"`
+		Inputs any    `json:"inputs"`
+	}{imageCacheSchema, class, inputs})
+	if err != nil {
+		return "", err
+	}
+	sum := sha256.Sum256(payload)
+	return "sha256:" + hex.EncodeToString(sum[:]), nil
+}
+
+// identityClasses digests each class of inputs.
+func identityClasses(inputs *imageInputs) (*buildClasses, error) {
+	code, err := classDigest("code", []any{inputs.Context, inputs.Dockerignore, inputs.ContextTree, inputs.GoModules})
+	if err != nil {
+		return nil, err
+	}
+	recipe, err := classDigest("recipe", []any{inputs.Recipe, inputs.Dockerfile, inputs.RecipeTree, inputs.Command})
+	if err != nil {
+		return nil, err
+	}
+	bases, err := classDigest("bases", inputs.Bases)
+	if err != nil {
+		return nil, err
+	}
+	return &buildClasses{Code: code, Recipe: recipe, Bases: bases}, nil
 }
 
 // imageBuildScope is where one recipe's inputs live. It travels as a unit so the
@@ -289,8 +337,12 @@ func imageBuildIdentity(ctx context.Context, recipe *builderv0.DockerBuildRecipe
 	if err != nil {
 		return imageIdentity{}, err
 	}
+	classes, err := identityClasses(&inputs)
+	if err != nil {
+		return imageIdentity{}, err
+	}
 	sum := sha256.Sum256(payload)
-	return imageIdentity{Key: "sha256:" + hex.EncodeToString(sum[:]), ContextTree: contextTree}, nil
+	return imageIdentity{Key: "sha256:" + hex.EncodeToString(sum[:]), ContextTree: contextTree, Classes: classes}, nil
 }
 
 // proxyPlaceholder is what every fetched Go module proxy path reads as in an

pkg/orchestration/image_cache_test.go

@@ -855,17 +855,15 @@ func TestAPublishedVersionFromTheSameInputsIsAdoptedNotRebuilt(t *testing.T) {
 	require.Equal(t, 1, harness.docker.calls(t, "build"), "the published version is adopted, not rebuilt")
 }
 
-// The version alone is not proof: a version that was not moved with its code
-// would hand back another release's image, so a published image recording other
-// inputs refuses the build instead of being adopted or overwritten.
-func TestAPublishedVersionFromOtherInputsRefusesTheBuild(t *testing.T) {
+// An image labelled before builds recorded their input classes carries only the
+// aggregate, which cannot say whether its code is this build's. It is the
+// release's build, so it is adopted, not rebuilt over and not refused.
+func TestAPublishedVersionLabelledBeforeClassesIsAdopted(t *testing.T) {
 	harness := newImageCacheHarness(t, true, true)
 	harness.peer.image = "example.test/app:1.2.3"
-	stubPublishedImage(t, "sha256:"+strings.Repeat("d", 64), "sha256:"+strings.Repeat("e", 64))
-	ctx, cancel := context.WithTimeout(t.Context(), time.Minute)
-	defer cancel()
-	err := harness.tryBuild(ctx, t)
-	require.ErrorContains(t, err, "already published from other inputs")
+	published := "sha256:" + strings.Repeat("d", 64)
+	stubPublishedImage(t, published, "sha256:"+strings.Repeat("e", 64))
+	require.Equal(t, published, harness.build(t))
 	require.Equal(t, 0, harness.docker.calls(t, "build"))
 }
 
@@ -888,7 +886,13 @@ func TestBuildInputsAreReadFromEveryPlatform(t *testing.T) {
 	disagree := []byte(`{"linux/amd64":{"config":{"Labels":{"` + buildInputsLabel + `":"` + key + `"}}},"linux/arm64":{"config":{"Labels":{}}}}`)
 	require.Equal(t, "", buildLabelsFromImageConfig(disagree)[buildInputsLabel], "platforms that disagree prove nothing")
 	require.Equal(t, []string{"buildx", "build", "--label", buildInputsLabel + "=" + key, "--push", "."},
-		withBuildInputsLabel([]string{"buildx", "build", "--push", "."}, key))
+		withBuildInputsLabel([]string{"buildx", "build", "--push", "."}, imageIdentity{Key: key}))
+	classes := &buildClasses{Code: "sha256:c", Recipe: "sha256:r", Bases: "sha256:b"}
+	require.Equal(t, []string{"buildx", "build", "--label", buildInputsLabel + "=" + key,
+		"--label", buildCodeLabel + "=sha256:c", "--label", buildRecipeLabel + "=sha256:r", "--label", buildBasesLabel + "=sha256:b", "."},
+		withBuildInputsLabel([]string{"buildx", "build", "."}, imageIdentity{Key: key, Classes: classes}))
+	labelled := []byte(`{"config":{"Labels":{"` + buildCodeLabel + `":"sha256:c","` + buildBasesLabel + `":"sha256:b"}}}`)
+	require.Equal(t, map[string]string{buildCodeLabel: "sha256:c", buildBasesLabel: "sha256:b"}, buildLabelsFromImageConfig(labelled))
 }
 
 // stubPublishedImage makes the registry answer that the reference exists at