From 0b807a960ad1fc40f2f3004fc5cd52a4df812a3f Mon Sep 17 00:00:00 2001 From: Antoine Toussaint <312536060+antoinetoussaint-byte@users.noreply.github.com> Date: Sat, 24 Oct 2026 14:46:37 -0400 Subject: [PATCH] fix(build): a build under an agent override is tagged apart from the release `codefly deploy gitops render` refused a release whose module had not moved (compute/excel:0.3.2, model/model:0.1.0, records/records:0.4.2) as "already published from other inputs: its version was not moved with its code" once the workspace's agent-overrides moved the agent its services build with. The agent emits the build recipe, so the image identity moved with the agent while the release tag already held the build of the previous agent. A released version names the build its release pins. A pushed build of a released version whose agent the workspace overrides away from the module's own pin is now pushed as `-build.<16 hex of its input identity>`, never under the release tag; the release check does not apply to it, another checkout adopts it from the registry by that tag, and its digest is always captured so the deployment pins it. Every build also records the agent that emitted its recipe (org.codefly.build.agent), and the refusal that remains names which input class differs: another agent, or the same agent with changed code or base image. Co-Authored-By: Claude Opus 5.5 --- docs/commands.md | 41 ++++-- pkg/gitops/cell_test.go | 13 +- pkg/orchestration/build_plan.go | 102 ++++++++++----- pkg/orchestration/image_cache_test.go | 76 +++++++---- pkg/orchestration/override_build.go | 105 +++++++++++++++ pkg/orchestration/override_build_test.go | 159 +++++++++++++++++++++++ 6 files changed, 428 insertions(+), 68 deletions(-) create mode 100644 pkg/orchestration/override_build.go create mode 100644 pkg/orchestration/override_build_test.go diff --git a/docs/commands.md b/docs/commands.md index 0727f4da..55913561 100644 --- a/docs/commands.md +++ b/docs/commands.md @@ -2058,8 +2058,11 @@ Every composed service whose agent is that publisher and name runs against that version — in `codefly run`, `codefly deploy gitops render` and every `codefly ci` verb alike, because core applies it at the one place a composed module loads a service. Only the version moves; the agent's publisher, name and kind -never do, and the module's own files are untouched. Each run or render states -it once per overridden agent: +never do, and the module's own files are untouched. A release built under an +override is not the release's build, so its image is tagged apart from the +release ([Keeping an image whose inputs did not +change](#keeping-an-image-whose-inputs-did-not-change)). Each run or render +states it once per overridden agent: ```text agent codefly.dev/go-grpc overridden to 0.1.47 by workspace.codefly.yaml (3 services; module pins: 0.1.46) @@ -4040,16 +4043,36 @@ two environments, is one image. Before a pushed build of a released version 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: its version was not moved -with its code, and the fix is a new release, never a rebuild under the old -version. One recording none (pushed before builds recorded their inputs) is -built again. 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 overwrite a released tag on any registry whose tags are mutable. With a +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 +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 +overwrite a released tag on any registry whose tags are mutable. With a registry whose tags are immutable, each version then holds exactly one build for good. +**A released version names the build its release pins.** A workspace's +[`agent-overrides`](#moving-the-agent-version-of-composed-services) moves the +agent a composed service builds with, and the agent emits the build recipe, so +the same release built under an override is another build. It is never pushed +under the release's tag: it is tagged `-build.` — the first +16 hex characters of its input digest, e.g. `excel:0.3.2-build.3f9c0a1b2d4e5f60` +— and the release check does not apply to it. That tag holds one build; another +checkout composing the same override adopts it from the registry the same way. +Where the environment pulls through a remote repository, the qualified tag is +pushed and probed at the push registry like any other (`ghcr.io/…:0.3.2-build.…`). +The deployment still names the release's tag in the repository the cell pulls +from, pinned to the qualified build's digest (the digest is always captured for such a build), and the cell file +records only the repository and that digest, so nothing admits the wrong image. +An override to the version the module already pins moves nothing and builds +the release as usual. + A build also reuses the image this workspace already built from the same inputs instead of building it again. A configuration-only change — an edit to a value under `configurations/`, a workspace or module manifest, anything a service reads diff --git a/pkg/gitops/cell_test.go b/pkg/gitops/cell_test.go index 35985897..6b214baa 100644 --- a/pkg/gitops/cell_test.go +++ b/pkg/gitops/cell_test.go @@ -524,10 +524,15 @@ func TestAHostStatingAnEnvelopeCarriesDelivery(t *testing.T) { // `@` followed by another algorithm, a short digest or upper-case hex is a // digest no running pod reports, and is refused. func TestOnlyASha256DigestPinsAnImage(t *testing.T) { - repository, digest, err := splitPinnedImage("registry.example.test/acme/api:1.2.0@sha256:" + strings.Repeat("a", 64)) - require.NoError(t, err) - require.Equal(t, "registry.example.test/acme/api", repository) - require.Equal(t, "sha256:"+strings.Repeat("a", 64), digest) + // The tag is informational: a build pushed under a build-qualified tag (an + // agent the workspace overrides) is deployed under the release's tag with + // its own digest, and the cell names the repository and that digest alone. + for _, tag := range []string{":1.2.0", ":1.2.0-build.0123456789abcdef", ""} { + repository, digest, err := splitPinnedImage("registry.example.test/acme/api" + tag + "@sha256:" + strings.Repeat("a", 64)) + require.NoError(t, err) + require.Equal(t, "registry.example.test/acme/api", repository, tag) + require.Equal(t, "sha256:"+strings.Repeat("a", 64), digest, tag) + } for _, image := range []string{ "registry.example.test/acme/api:1.2.0", "registry.example.test/acme/api@latest", diff --git a/pkg/orchestration/build_plan.go b/pkg/orchestration/build_plan.go index 01527610..67822504 100644 --- a/pkg/orchestration/build_plan.go +++ b/pkg/orchestration/build_plan.go @@ -88,6 +88,10 @@ func (b *Builder) buildFromPlan(ctx context.Context, outputDir string, plan *bui type imageReuse struct { identity imageIdentity entry *imageCacheEntry + // image, when set, is the reference the recipe is pushed under instead of + // the one the agent named: a build-qualified tag for a build whose agent the + // workspace overrides (override_build.go). + image string } // decideImageReuse takes the reuse decision for a recipe as it will be built @@ -149,6 +153,25 @@ func (b *Builder) decideImageReuse( wool.Field("recipe", recipe.GetName()), wool.ErrField(identityErr)) } decision := imageReuse{identity: identity} + // A build under a workspace agent override is not the build the release + // names, so it is pushed under a tag of its own (override_build.go). + if shouldPush && releasedVersionTag(recipe.GetImage()) { + pinned, overrideErr := b.overriddenAgent(ctx) + if overrideErr != nil { + return decision, w.Wrap(overrideErr) + } + if pinned != "" { + if identity.Key == "" { + return decision, w.NewError("%s builds with agent %s, which the workspace overrides from the %s its release pins, and an image input cannot be bound to name that build apart from the release: %v", + b.instance.Unique(), b.instance.Service.Agent.Identifier(), pinned, identityErr) + } + recipe = proto.CloneOf(recipe) + recipe.Image = overrideBuildImage(recipe.GetImage(), identity.Key) + decision.image = recipe.GetImage() + w.Info("the agent is overridden, so this is not the release's build; tagging it apart", + wool.Field("agent", b.instance.Service.Agent.Identifier()), wool.Field("release_pins", pinned), wool.Field("image", decision.image)) + } + } if b.world.RebuildImages { return decision, nil } @@ -159,7 +182,7 @@ func (b *Builder) decideImageReuse( // image, whichever environment, machine or checkout pushed it, and nothing is // built. An unversioned service (0.0.0, never published) is not a release and // is built as before. - if shouldPush && releasedVersionTag(recipe.GetImage()) { + if shouldPush && (releasedVersionTag(recipe.GetImage()) || decision.image != "") { digest, err := resolveImageManifestDigest(ctx, recipe.GetImage(), probeBuilder) if err != nil && !isImageNotFound(err) { // Only the registry saying the tag does not exist means it is not @@ -172,7 +195,8 @@ func (b *Builder) decideImageReuse( // 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. - published, _ := resolveImageBuildInputs(ctx, recipe.GetImage(), probeBuilder) + labels, _ := resolveImageBuildLabels(ctx, recipe.GetImage(), probeBuilder) + published := labels[buildInputsLabel] switch { case identity.Key != "" && published == identity.Key: decision.entry = &imageCacheEntry{ @@ -181,9 +205,7 @@ func (b *Builder) decideImageReuse( } return decision, nil case identity.Key != "" && published != "": - return decision, w.NewError( - "%s is already published from other inputs: its version was not moved with its code; release a new version (codefly publish) instead of rebuilding a published one", - recipe.GetImage()) + 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())) @@ -326,10 +348,6 @@ func (b *Builder) buildRecipe( return w.Wrapf(err, "cannot build recipe %s of %s", recipe.GetName(), b.instance.Unique()) } - // A pushed build records the immutable manifest digest a snapshot pins and a - // targeted single-service build returns to its caller. A non-pushed build - // never lands in a registry, so there is no digest to capture. - captureDigest := shouldPush && (b.world.Mode == SnapshotMode || b.world.CaptureImageDigest) // Image evidence is owed by every recipe, not only the one a snapshot pins, // so asking for it needs the digest of each pushed image too. resolveEvidence := b.world.CollectImageSBOM @@ -343,6 +361,17 @@ func (b *Builder) buildRecipe( } reuse = &decided } + if reuse.image != "" { + recipe = proto.CloneOf(recipe) + recipe.Image = reuse.image + } + // A pushed build records the immutable manifest digest a snapshot pins and a + // targeted single-service build returns to its caller. A non-pushed build + // never lands in a registry, so there is no digest to capture. A build pushed + // under a qualified tag is deployed by its digest whatever the mode: the + // agent names the deployed image by the release's tag, which holds another + // build. + captureDigest := shouldPush && (b.world.Mode == SnapshotMode || b.world.CaptureImageDigest || reuse.image != "") if reuse.entry != nil { w.Info("no image input changed; keeping the built image", wool.Field("image", reuse.entry.Image), wool.Field("digest", reuse.entry.Digest), @@ -408,7 +437,7 @@ func (b *Builder) buildRecipe( args, err := cachedBuildxArgs(recipe, dockerfile, contextDir, shouldPush, multiArch, metadataFile, builderName, cache, private) if err == nil { - args = withBuildInputsLabel(args, identity.Key) + args = withBuildAgentLabel(withBuildInputsLabel(args, identity.Key), b.instance.Service.Agent) if shouldPush { args = withSourceLabel(args, b.world.Env, workspaceSourceRepository(ctx, b.world.Workspace.Dir()), multiArch) } @@ -1092,10 +1121,10 @@ var workspaceSourceRepository = func(ctx context.Context, dir string) string { return "https://github.com/" + match[1] + "/" + match[2] } -// resolveImageBuildInputs reads the build-inputs label every platform of a -// published image carries, or "" when it carries none or its platforms disagree. -// A seam so a test needs no registry. -var resolveImageBuildInputs = func(ctx context.Context, reference, builderName string) (string, error) { +// resolveImageBuildLabels reads the build labels every platform of a published +// image agrees on (buildInputsLabel, buildAgentLabel); 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) defer cancel() args := []string{buildxCommand, "imagetools", "inspect"} @@ -1105,35 +1134,48 @@ var resolveImageBuildInputs = func(ctx context.Context, reference, builderName s args = append(args, "--format", "{{json .Image}}", reference) out, err := exec.CommandContext(probe, "docker", args...).Output() if err != nil { - return "", err + return nil, err } - return buildInputsFromImageConfig(out), nil + return buildLabelsFromImageConfig(out), nil } -// buildInputsFromImageConfig reads the label out of `imagetools inspect`'s image -// configuration: one configuration for a single-platform image, one per platform -// otherwise. Every platform must record the same value. -func buildInputsFromImageConfig(raw []byte) string { +// buildLabelsFromImageConfig reads the build labels out of `imagetools +// inspect`'s image configuration: one configuration for a single-platform image, +// one per platform otherwise. A label is kept only when every platform records +// the same value. +func buildLabelsFromImageConfig(raw []byte) map[string]string { type config struct { Config struct { Labels map[string]string `json:"Labels"` } `json:"config"` } + labels := map[string]string{} var single config if err := json.Unmarshal(raw, &single); err == nil && single.Config.Labels != nil { - return single.Config.Labels[buildInputsLabel] + for _, key := range []string{buildInputsLabel, buildAgentLabel} { + if value := single.Config.Labels[key]; value != "" { + labels[key] = value + } + } + return labels } var platforms map[string]config if err := json.Unmarshal(raw, &platforms); err != nil || len(platforms) == 0 { - return "" - } - value := "" - for _, platform := range platforms { - label := platform.Config.Labels[buildInputsLabel] - if label == "" || (value != "" && label != value) { - return "" + return labels + } + for _, key := range []string{buildInputsLabel, buildAgentLabel} { + value := "" + for _, platform := range platforms { + label := platform.Config.Labels[key] + if label == "" || (value != "" && label != value) { + value = "" + break + } + value = label + } + if value != "" { + labels[key] = value } - value = label } - return value + return labels } diff --git a/pkg/orchestration/image_cache_test.go b/pkg/orchestration/image_cache_test.go index a399baa7..cc1430fd 100644 --- a/pkg/orchestration/image_cache_test.go +++ b/pkg/orchestration/image_cache_test.go @@ -17,6 +17,7 @@ import ( "github.com/stretchr/testify/require" "google.golang.org/grpc" "google.golang.org/grpc/credentials/insecure" + "gopkg.in/yaml.v3" ) // dockerStub stands in for the container engine at the only boundary an image @@ -42,6 +43,7 @@ record() { printf '%s\n' "$1" >> "$STATE/calls"; } case "$1 $2" in "buildx build") record build + printf '%s\n' "$*" >> "$STATE/build-args" count=0 if [ -f "$STATE/count" ]; then read -r count < "$STATE/count"; fi count=$((count+1)) @@ -171,6 +173,11 @@ type imageCacheHarness struct { rebuild bool // env is the environment the build targets, nil for none. env *environments.Environment + // agentOverride, when set, is the version the workspace's agent-overrides + // moves the service's agent to, away from the 0.1.0 its own file pins. + agentOverride string + // noCapture builds without asking for the pushed digest, as a deploy does. + noCapture bool // goCalls records every go command the prefetch of each build runs. goCalls []goCall // gitCalls records every git command the git source prefetch runs. @@ -197,7 +204,8 @@ func newImageCacheHarness(t *testing.T, push, registryServes bool) *imageCacheHa client: &coreservices.BuilderAgent{BuilderClient: builderv0.NewBuilderClient(conn)}, push: push, } - harness.write(t, filepath.Join(serviceDir, "service.codefly.yaml"), "name: api\n") + harness.write(t, filepath.Join(serviceDir, "service.codefly.yaml"), + "name: api\nversion: 1.0.0\nagent:\n kind: codefly:service\n publisher: codefly.dev\n name: go\n version: 0.1.0\n") harness.write(t, filepath.Join(serviceDir, "code", "main.go"), "package main\n\nfunc main() {}\n") harness.write(t, filepath.Join(serviceDir, "code", "go.mod"), "module example.test/api\n\ngo 1.25\n") harness.write(t, filepath.Join(serviceDir, "code", "go.sum"), "") @@ -206,6 +214,36 @@ func newImageCacheHarness(t *testing.T, push, registryServes bool) *imageCacheHa return harness } +// composition is the workspace and the service as the module composes it: the +// service's own file pins agent codefly.dev/go 0.1.0, and an agentOverride moves +// the loaded service to another version the way core applies agent-overrides. +func (harness *imageCacheHarness) composition(t *testing.T) (*resources.Workspace, *resources.Service) { + t.Helper() + workspace := &resources.Workspace{Name: "platform"} + workspace.WithDir(harness.workspaceDir) + agent := &resources.Agent{Kind: resources.ServiceAgent, Publisher: "codefly.dev", Name: "go", Version: "0.1.0"} + if harness.agentOverride != "" { + var node yaml.Node + require.NoError(t, yaml.Unmarshal([]byte("codefly.dev/go: "+harness.agentOverride+"\n"), &node)) + workspace.Extensions = map[string]resources.YAMLValue{resources.AgentOverridesKey: {Node: *node.Content[0]}} + agent = &resources.Agent{Kind: resources.ServiceAgent, Publisher: "codefly.dev", Name: "go", Version: harness.agentOverride} + } + service := &resources.Service{Name: "api", Version: "1.0.0", Agent: agent} + service.WithDir(harness.serviceDir) + return workspace, service +} + +// buildArgs is the argv of every `buildx build` the stub ran, in order. +func (stub *dockerStub) buildArgs(t *testing.T) []string { + t.Helper() + payload, err := os.ReadFile(filepath.Join(stub.dir, "build-args")) + if os.IsNotExist(err) { + return nil + } + require.NoError(t, err) + return strings.Split(strings.TrimSpace(string(payload)), "\n") +} + func (harness *imageCacheHarness) write(t *testing.T, path, content string) { t.Helper() require.NoError(t, os.MkdirAll(filepath.Dir(path), 0o755)) @@ -227,21 +265,14 @@ func (harness *imageCacheHarness) build(t *testing.T) string { ctx, cancel := context.WithTimeout(t.Context(), 2*time.Minute) defer cancel() - workspace := &resources.Workspace{Name: "platform"} - workspace.WithDir(harness.workspaceDir) - service := &resources.Service{ - Name: "api", - Version: "1.0.0", - Agent: &resources.Agent{Publisher: "codefly.dev", Name: "go", Version: "0.1.0"}, - } - service.WithDir(harness.serviceDir) + workspace, service := harness.composition(t) instance := &services.Instance{Service: service, Identity: &resources.ServiceIdentity{Name: "api", Module: "saas"}} instance.Builder = &services.BuilderInstance{Instance: instance, Builder: harness.client} world := &World{ Workspace: workspace, Env: harness.env, Push: harness.push, - CaptureImageDigest: harness.push, + CaptureImageDigest: harness.push && !harness.noCapture, BuildxBuilder: "stub", RebuildImages: harness.rebuild, } @@ -851,11 +882,11 @@ func TestAPublishedVersionWithNoInputRecordIsBuiltAgain(t *testing.T) { func TestBuildInputsAreReadFromEveryPlatform(t *testing.T) { key := "sha256:" + strings.Repeat("a", 64) single := []byte(`{"config":{"Labels":{"` + buildInputsLabel + `":"` + key + `"}}}`) - require.Equal(t, key, buildInputsFromImageConfig(single)) + require.Equal(t, key, buildLabelsFromImageConfig(single)[buildInputsLabel]) both := []byte(`{"linux/amd64":{"config":{"Labels":{"` + buildInputsLabel + `":"` + key + `"}}},"linux/arm64":{"config":{"Labels":{"` + buildInputsLabel + `":"` + key + `"}}}}`) - require.Equal(t, key, buildInputsFromImageConfig(both)) + require.Equal(t, key, buildLabelsFromImageConfig(both)[buildInputsLabel]) disagree := []byte(`{"linux/amd64":{"config":{"Labels":{"` + buildInputsLabel + `":"` + key + `"}}},"linux/arm64":{"config":{"Labels":{}}}}`) - require.Equal(t, "", buildInputsFromImageConfig(disagree), "platforms that disagree prove nothing") + 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)) } @@ -865,8 +896,8 @@ func TestBuildInputsAreReadFromEveryPlatform(t *testing.T) { // restore for a test that needs the real seams back mid-way. func stubPublishedImage(t *testing.T, digest, inputs string) func() { t.Helper() - previousDigest, previousInputs := resolveImageManifestDigest, resolveImageBuildInputs - restore := func() { resolveImageManifestDigest, resolveImageBuildInputs = previousDigest, previousInputs } + previousDigest, previousInputs := resolveImageManifestDigest, resolveImageBuildLabels + restore := func() { resolveImageManifestDigest, resolveImageBuildLabels = previousDigest, previousInputs } t.Cleanup(restore) resolveImageManifestDigest = func(ctx context.Context, reference, builder string) (string, error) { if strings.HasSuffix(reference, ":1.2.3") { @@ -877,7 +908,9 @@ func stubPublishedImage(t *testing.T, digest, inputs string) func() { } return previousDigest(ctx, reference, builder) } - resolveImageBuildInputs = func(context.Context, string, string) (string, error) { return inputs, nil } + resolveImageBuildLabels = func(context.Context, string, string) (map[string]string, error) { + return map[string]string{buildInputsLabel: inputs}, nil + } return restore } @@ -899,17 +932,10 @@ func (harness *imageCacheHarness) run(ctx context.Context, instance *services.In // returned rather than required away. func (harness *imageCacheHarness) tryBuild(ctx context.Context, t *testing.T) error { t.Helper() - workspace := &resources.Workspace{Name: "platform"} - workspace.WithDir(harness.workspaceDir) - service := &resources.Service{ - Name: "api", - Version: "1.0.0", - Agent: &resources.Agent{Publisher: "codefly.dev", Name: "go", Version: "0.1.0"}, - } - service.WithDir(harness.serviceDir) + workspace, service := harness.composition(t) instance := &services.Instance{Service: service, Identity: &resources.ServiceIdentity{Name: "api", Module: "saas"}} instance.Builder = &services.BuilderInstance{Instance: instance, Builder: harness.client} - world := &World{Workspace: workspace, Env: harness.env, Push: harness.push, CaptureImageDigest: harness.push, BuildxBuilder: "stub"} + world := &World{Workspace: workspace, Env: harness.env, Push: harness.push, CaptureImageDigest: harness.push && !harness.noCapture, BuildxBuilder: "stub"} prefetch := newGoModulePrefetch() prefetch.run = recordingGo(&harness.goCalls, nil) world.goModulePrefetch = prefetch diff --git a/pkg/orchestration/override_build.go b/pkg/orchestration/override_build.go new file mode 100644 index 00000000..c0b9ceaf --- /dev/null +++ b/pkg/orchestration/override_build.go @@ -0,0 +1,105 @@ +package orchestration + +import ( + "context" + "fmt" + "strings" + + "github.com/codefly-dev/core/resources" +) + +// A released version names one build: the one made with the agent the release +// itself pins. A workspace's agent-overrides block moves the agent a composed +// service runs on without moving the module, and the agent emits the build +// recipe, so the same release built under an override is another build. It is +// never pushed under the release's tag — where it would overwrite the release's +// build, or be refused as a release whose version did not move with its code — +// but under a tag qualified by its own input identity, which no other build can +// claim. The cell pins every image by digest, so the tag a deployment names is +// informational; what the qualification protects is the registry's release tag. + +// buildAgentLabel records the agent that emitted an image's build recipe, so a +// refusal can say whether the agent is what differs from a published build. +const buildAgentLabel = "org.codefly.build.agent" + +// overrideBuildTagInfix separates the release version from the identity in a +// build-qualified tag: -build.. Semver reads it as a +// prerelease of the version, which is what releasedVersionTag refuses to +// treat as a release. +const overrideBuildTagInfix = "-build." + +// overrideBuildIdentityLength is how much of the input identity a qualified tag +// carries: 64 bits, far past any chance of two builds of one version colliding, +// and a published image's inputs label is still checked before it is adopted. +const overrideBuildIdentityLength = 16 + +// overriddenAgent reports the agent version the service's own module pins when +// the workspace's agent-overrides moved it to another one, and "" when the +// service builds with its module's pin. Core applies an override when a +// composed module loads the service (resources.Module.LoadServiceFromReference) +// and keeps no record of what it replaced; the service's own file, loaded on +// its own, is that record (as composition.ResolveAgentOverrides reads it). +func (b *Builder) overriddenAgent(ctx context.Context) (string, error) { + service := b.instance.Service + if b.world == nil || b.world.Workspace == nil || service == nil || service.Agent == nil { + return "", nil + } + overrides, err := b.world.Workspace.AgentOverrides() + if err != nil { + return "", err + } + matched := false + for _, override := range overrides { + matched = matched || override.Matches(service.Agent) + } + if !matched { + return "", nil + } + declared, err := resources.LoadServiceFromDir(ctx, service.Dir()) + if err != nil { + return "", fmt.Errorf("cannot read the agent %s pins: %w", b.instance.Unique(), err) + } + if declared.Agent == nil || declared.Agent.Version == service.Agent.Version { + return "", nil + } + return declared.Agent.Version, nil +} + +// overrideBuildImage is image with its release tag qualified by the build's +// input identity: example.test/app:1.2.3 → example.test/app:1.2.3-build.<16 hex>. +func overrideBuildImage(image, identityKey string) string { + identity := strings.TrimPrefix(identityKey, "sha256:") + if len(identity) > overrideBuildIdentityLength { + identity = identity[:overrideBuildIdentityLength] + } + return image + overrideBuildTagInfix + identity +} + +// withBuildAgentLabel records the agent that emitted the recipe on the image. +// Like the inputs label it is added after the identity was computed. +func withBuildAgentLabel(args []string, agent *resources.Agent) []string { + if agent == nil { + return args + } + return withBuildFlags(args, "--label", buildAgentLabel+"="+agent.Identifier()) +} + +// publishedFromOtherInputs is the refusal for a released tag that holds a build +// of other inputs, naming which class of input differs when the published image +// says: the agent that emitted its recipe, or — the same agent — the service's +// code or a base image. +func publishedFromOtherInputs(image string, agent *resources.Agent, publishedAgent string) error { + const release = "release a new version (codefly publish) instead of rebuilding a published one" + switch { + case agent != nil && publishedAgent != "" && publishedAgent != agent.Identifier(): + return fmt.Errorf("%s is already published from a build by agent %s, and this build runs %s, the agent the release pins: "+ + "the tag holds a build the release does not name (made under a workspace agent override by a CLI that published it under the release tag); "+ + "%s, or remove that tag from the registry", image, publishedAgent, agent.Identifier(), release) + case publishedAgent != "": + return fmt.Errorf("%s is already published from other inputs by the same agent %s: the service's code or a base image changed without its version moving; %s", + image, publishedAgent, release) + default: + return fmt.Errorf("%s is already published from other inputs: the service's code, its agent's build recipe or a base image differ, and the published image does not record which; %s", + image, release) + } +} diff --git a/pkg/orchestration/override_build_test.go b/pkg/orchestration/override_build_test.go new file mode 100644 index 00000000..584bda72 --- /dev/null +++ b/pkg/orchestration/override_build_test.go @@ -0,0 +1,159 @@ +package orchestration + +import ( + "context" + "fmt" + "os" + "path/filepath" + "regexp" + "strings" + "testing" + "time" + + "github.com/codefly-dev/core/resources" + "github.com/stretchr/testify/require" +) + +// stubRegistry makes the registry answer for exactly the references it holds, +// each with its digest and build labels; any other reference is not published. +func stubRegistry(t *testing.T, held map[string]publishedImage) { + t.Helper() + previousDigest, previousLabels := resolveImageManifestDigest, resolveImageBuildLabels + t.Cleanup(func() { resolveImageManifestDigest, resolveImageBuildLabels = previousDigest, previousLabels }) + resolveImageManifestDigest = func(ctx context.Context, reference, builder string) (string, error) { + if image, ok := held[reference]; ok { + return image.digest, nil + } + if strings.Contains(reference, "@") { + return previousDigest(ctx, reference, builder) + } + return "", fmt.Errorf("%w: not published", errImageNotFound) + } + resolveImageBuildLabels = func(_ context.Context, reference, _ string) (map[string]string, error) { + return held[reference].labels, nil + } +} + +type publishedImage struct { + digest string + labels map[string]string +} + +var qualifiedTag = regexp.MustCompile(`-t example\.test/app:1\.2\.3-build\.[0-9a-f]{16} `) + +// The obin-blue defect: a release is published, the workspace overrides the +// agent its services build with, and the render refused the release's own +// version as "not moved with its code". The overridden build is another build, +// so it is pushed under a tag of its own — never the release's — is not +// refused, and is deployed by the digest it was pushed at even where the flow +// asked for no digest. +func TestAnOverriddenAgentBuildOfAPublishedVersionIsTaggedApart(t *testing.T) { + harness := newImageCacheHarness(t, true, true) + harness.peer.image = "example.test/app:1.2.3" + harness.agentOverride = "0.2.0" + harness.noCapture = true + release := publishedImage{digest: "sha256:" + strings.Repeat("d", 64), labels: map[string]string{ + buildInputsLabel: "sha256:" + strings.Repeat("e", 64), buildAgentLabel: "codefly.dev/go:0.1.0", + }} + stubRegistry(t, map[string]publishedImage{"example.test/app:1.2.3": release}) + + digest := harness.build(t) + require.Equal(t, 1, harness.docker.calls(t, "build")) + require.NotEqual(t, release.digest, digest, "the release's build is not the overridden one") + args := harness.docker.buildArgs(t) + require.Len(t, args, 1) + require.Regexp(t, qualifiedTag, args[0], "pushed under a build-qualified tag") + require.NotContains(t, args[0], "-t example.test/app:1.2.3 ", "never under the release's tag") + require.Contains(t, args[0], "--label "+buildAgentLabel+"=codefly.dev/go:0.2.0") + + // Another checkout composing the same override adopts that build from the + // registry by its qualified tag instead of pushing a second one over it. + key := harness.key(t) + qualified := overrideBuildImage("example.test/app:1.2.3", key) + require.Contains(t, args[0], "-t "+qualified+" ") + require.NoError(t, os.RemoveAll(filepath.Join(harness.workspaceDir, filepath.FromSlash(imageCacheDir)))) + stubRegistry(t, map[string]publishedImage{ + "example.test/app:1.2.3": release, + qualified: {digest: digest, labels: map[string]string{buildInputsLabel: key}}, + }) + require.Equal(t, digest, harness.build(t)) + require.Equal(t, 1, harness.docker.calls(t, "build"), "the qualified build is adopted, not rebuilt") +} + +// An override entry naming the version the module already pins moves nothing: +// the build is the release's, under the release's tag. +func TestAnOverrideToTheModulesOwnPinIsTheReleaseBuild(t *testing.T) { + harness := newImageCacheHarness(t, true, true) + harness.peer.image = "example.test/app:1.2.3" + harness.agentOverride = "0.1.0" + stubRegistry(t, nil) + harness.build(t) + args := harness.docker.buildArgs(t) + require.Len(t, args, 1) + 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) { + for name, tc := range map[string]struct { + agent, want 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"}, + } { + 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 + } + 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")) + }) + } +} + +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) + require.False(t, releasedVersionTag(image), "a qualified tag never reads as the release") + tag := image[strings.LastIndex(image, ":")+1:] + require.Regexp(t, `^[A-Za-z0-9_][A-Za-z0-9_.-]{0,127}$`, tag) + require.Equal(t, []string{"buildx", "build", "--label", buildAgentLabel + "=codefly.dev/go:0.2.0", "."}, + withBuildAgentLabel([]string{"buildx", "build", "."}, &resources.Agent{Publisher: "codefly.dev", Name: "go", Version: "0.2.0"})) +} + +// The qualified tag composes with a pull-through registry: the overridden build +// is probed and pushed at ghcr.io under the qualified tag, while the plan — and +// so the deployed manifest the agent renders — keeps the repository the cell +// pulls from, pinned to the pushed digest. +func TestAnOverriddenBuildIsPushedQualifiedAtTheRegistryItPullsThrough(t *testing.T) { + harness := newImageCacheHarness(t, true, true) + harness.env = pullThroughEnv() + harness.agentOverride = "0.2.0" + harness.noCapture = true + pulled := remoteRepository + "/obin-ai/saas/api:1.2.3" + harness.peer.image = pulled + stubRegistry(t, map[string]publishedImage{"ghcr.io/obin-ai/saas/api:1.2.3": { + digest: "sha256:" + strings.Repeat("d", 64), labels: map[string]string{buildInputsLabel: "sha256:" + strings.Repeat("e", 64)}, + }}) + + digest := harness.build(t) + args := harness.docker.buildArgs(t) + require.Len(t, args, 1) + pushed := overrideBuildImage("ghcr.io/obin-ai/saas/api:1.2.3", harness.key(t)) + require.Contains(t, args[0], "-t "+pushed+" ", "pushed at ghcr.io under the qualified tag") + require.NotContains(t, args[0], remoteRepository) + require.NotContains(t, args[0], "-t ghcr.io/obin-ai/saas/api:1.2.3 ") + // The agent renders the deployed image from its own plan name (the pull + // repository) and this digest; the cell keeps that repository and digest. + require.Equal(t, "sha256:"+fmt.Sprintf("%064d", 1), digest, "the digest the deployment pins is the pushed one") +}