diff --git a/docs/commands.md b/docs/commands.md index 55913561..49704038 100644 --- a/docs/commands.md +++ b/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 `:`. 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 diff --git a/pkg/orchestration/build_plan.go b/pkg/orchestration/build_plan.go index 67822504..832f0a7b 100644 --- a/pkg/orchestration/build_plan.go +++ b/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] diff --git a/pkg/orchestration/image_cache.go b/pkg/orchestration/image_cache.go index 2fa41604..5e0ba2fa 100644 --- a/pkg/orchestration/image_cache.go +++ b/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 diff --git a/pkg/orchestration/image_cache_test.go b/pkg/orchestration/image_cache_test.go index cc1430fd..393dbc50 100644 --- a/pkg/orchestration/image_cache_test.go +++ b/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 diff --git a/pkg/orchestration/override_build_test.go b/pkg/orchestration/override_build_test.go index 584bda72..b7ec1061 100644 --- a/pkg/orchestration/override_build_test.go +++ b/pkg/orchestration/override_build_test.go @@ -93,34 +93,97 @@ func TestAnOverrideToTheModulesOwnPinIsTheReleaseBuild(t *testing.T) { require.Contains(t, args[0], "-t example.test/app:1.2.3 ") } -// Without an override the release check stands, and the refusal says which -// class of input differs when the published image records its agent. -func TestARefusedReleaseNamesWhatDiffers(t *testing.T) { +// classLabels reads the input labels a build recorded off its argv. +func classLabels(t *testing.T, args string) map[string]string { + t.Helper() + labels := map[string]string{} + for _, match := range regexp.MustCompile(`--label (org\.codefly\.build\.[a-z.]+)=(\S+)`).FindAllStringSubmatch(args, -1) { + labels[match[1]] = match[2] + } + for _, key := range []string{buildInputsLabel, buildCodeLabel, buildRecipeLabel, buildBasesLabel, buildAgentLabel} { + require.NotEmpty(t, labels[key], "every build records %s", key) + } + return labels +} + +// The 2026-10-24 defect: a release published from another checkout, whose +// recipe or bases differ from this one's, refused every later render of every +// module on the cell. The refusal exists for a version not moved with its +// code, so only a code difference refuses; anything else adopts the release's +// build. An agent other than the release's pin still refuses (#1146). +func TestAPublishedReleaseIsJudgedByItsCode(t *testing.T) { + harness := newImageCacheHarness(t, true, true) + harness.peer.image = "example.test/app:1.2.3" + stubRegistry(t, nil) + harness.build(t) + args := harness.docker.buildArgs(t) + require.Len(t, args, 1) + ours := classLabels(t, args[0]) + other := "sha256:" + strings.Repeat("e", 64) + published := "sha256:" + strings.Repeat("d", 64) + with := func(changes map[string]string) map[string]string { + labels := map[string]string{} + for key, value := range ours { + labels[key] = value + } + for key, value := range changes { + if value == "" { + delete(labels, key) + continue + } + labels[key] = value + } + return labels + } for name, tc := range map[string]struct { - agent, want string + labels map[string]string + refuse string }{ - "another agent built it": {agent: "codefly.dev/go:0.0.9", want: "from a build by agent codefly.dev/go:0.0.9, and this build runs codefly.dev/go:0.1.0"}, - "the same agent": {agent: "codefly.dev/go:0.1.0", want: "by the same agent codefly.dev/go:0.1.0: the service's code or a base image changed"}, - "no agent recorded": {want: "the published image does not record which"}, + "same code, other recipe and bases": {labels: with(map[string]string{buildInputsLabel: other, buildRecipeLabel: other, buildBasesLabel: other})}, + "labelled before the classes": {labels: with(map[string]string{ + buildInputsLabel: other, buildCodeLabel: "", buildRecipeLabel: "", buildBasesLabel: "", + })}, + "other code": {labels: with(map[string]string{buildInputsLabel: other, buildCodeLabel: other}), refuse: "is already published from other code"}, + "other agent": {labels: with(map[string]string{buildInputsLabel: other, buildAgentLabel: "codefly.dev/go:0.0.9"}), refuse: "from a build by agent codefly.dev/go:0.0.9"}, } { t.Run(name, func(t *testing.T) { - harness := newImageCacheHarness(t, true, true) - harness.peer.image = "example.test/app:1.2.3" - labels := map[string]string{buildInputsLabel: "sha256:" + strings.Repeat("e", 64)} - if tc.agent != "" { - labels[buildAgentLabel] = tc.agent + require.NoError(t, os.RemoveAll(filepath.Join(harness.workspaceDir, filepath.FromSlash(imageCacheDir)))) + stubRegistry(t, map[string]publishedImage{"example.test/app:1.2.3": {digest: published, labels: tc.labels}}) + if tc.refuse == "" { + require.Equal(t, published, harness.build(t)) + } else { + ctx, cancel := context.WithTimeout(t.Context(), time.Minute) + defer cancel() + require.ErrorContains(t, harness.tryBuild(ctx, t), tc.refuse) } - stubRegistry(t, map[string]publishedImage{"example.test/app:1.2.3": {digest: "sha256:" + strings.Repeat("d", 64), labels: labels}}) - ctx, cancel := context.WithTimeout(t.Context(), time.Minute) - defer cancel() - err := harness.tryBuild(ctx, t) - require.ErrorContains(t, err, "example.test/app:1.2.3 is already published") - require.ErrorContains(t, err, tc.want) - require.Equal(t, 0, harness.docker.calls(t, "build")) + require.Equal(t, 1, harness.docker.calls(t, "build"), "a published release is never rebuilt") }) } } +// The warning says which class differs, and the refusal names the code. +func TestPublishedReleaseVerdictNamesTheClass(t *testing.T) { + agent := &resources.Agent{Publisher: "codefly.dev", Name: "go", Version: "0.1.0"} + ours := imageIdentity{Key: "sha256:k", Classes: &buildClasses{Code: "sha256:c", Recipe: "sha256:r", Bases: "sha256:b"}} + labels := func(code, recipe, bases string) map[string]string { + return map[string]string{buildInputsLabel: "sha256:other", buildCodeLabel: code, buildRecipeLabel: recipe, buildBasesLabel: bases} + } + warning, err := publishedReleaseVerdict("app:1.2.3", ours, agent, labels("sha256:c", "sha256:x", "sha256:b")) + require.NoError(t, err) + require.Contains(t, warning, "the build recipe its agent emitted") + require.NotContains(t, warning, "base images") + warning, err = publishedReleaseVerdict("app:1.2.3", ours, agent, labels("sha256:c", "sha256:r", "sha256:x")) + require.NoError(t, err) + require.Contains(t, warning, "the resolved base images") + require.NotContains(t, warning, "recipe") + warning, err = publishedReleaseVerdict("app:1.2.3", ours, agent, map[string]string{buildInputsLabel: "sha256:other"}) + require.NoError(t, err) + require.Contains(t, warning, "predates per-class build labels") + _, err = publishedReleaseVerdict("app:1.2.3", ours, agent, labels("sha256:x", "sha256:r", "sha256:b")) + require.ErrorContains(t, err, "app:1.2.3 is already published from other code") + require.ErrorContains(t, err, "release a new version") +} + func TestOverrideBuildImageIsAValidDockerTag(t *testing.T) { image := overrideBuildImage("registry.example.test/compute/excel:0.3.2", "sha256:"+strings.Repeat("ab", 32)) require.Equal(t, "registry.example.test/compute/excel:0.3.2-build.abababababababab", image)