From 22d23a2e63681e794f2a990d58e4af0c22e4e57b Mon Sep 17 00:00:00 2001 From: Antoine Toussaint <312536060+antoinetoussaint-byte@users.noreply.github.com> Date: Sat, 24 Oct 2026 10:54:24 -0400 Subject: [PATCH] fix(gitops): deliver from codefly's ghcr.io image and restart a squash-merged promotion branch Two defects in `codefly deploy gitops publish`: - Every delivery Job (and every cell file's delivery carrier) pinned docker.io/curlimages/curl. It now runs codefly's own base companion, ghcr.io/codefly-dev/codefly pinned by the digest of its 0.0.4 multi-arch index, which carries every command the delivery script calls. Nothing in a delivery is pulled from Docker Hub. - A reviewed publish continued an existing promotion branch even when the base had moved past it (its pull request squash-merged, the branch kept), so the new commit forked from a stale base and conflicted add/add on every file. It now continues a branch only when the branch contains the base; otherwise it starts from the base and replaces the branch with a push leased on the stale tip, recorded in the plan as replacesBranchRevision (part of the plan ID) and printed by `plan`. A stale branch carrying a change outside the module path is still refused, never overwritten. Co-Authored-By: Claude Opus 5.5 --- cmd/deploy/gitops.go | 3 + docs/commands.md | 6 + docs/module-manifests.md | 20 ++- pkg/gitops/delivery_carrier_contract_test.go | 13 ++ pkg/gitops/delivery_jobs.go | 19 ++- pkg/gitops/delivery_jobs_test.go | 7 +- pkg/gitops/publish.go | 110 +++++++++----- pkg/gitops/publish_test.go | 151 ++++++++++++++++--- pkg/gitops/types.go | 39 +++-- 9 files changed, 285 insertions(+), 83 deletions(-) diff --git a/cmd/deploy/gitops.go b/cmd/deploy/gitops.go index 1303e184..5991efd8 100644 --- a/cmd/deploy/gitops.go +++ b/cmd/deploy/gitops.go @@ -567,6 +567,9 @@ func printPublishPlan(plan *gitops.PublishPlan) { cli.Info("Repository %s", plan.Repository) cli.Info("Base %s@%s", plan.BaseBranch, plan.BaseRevision) cli.Info("Promotion branch %s", plan.PromotionBranch) + if plan.ReplacesBranchRevision != "" { + cli.Warning("Promotion branch %s at %s does not contain %s (its pull request was squash-merged, or the base moved past it); this publish starts again from %s and replaces the branch, leased on %s", plan.PromotionBranch, plan.ReplacesBranchRevision, plan.BaseBranch, plan.BaseBranch, plan.ReplacesBranchRevision) + } cli.Info("Render digest %s", plan.RenderDigest) cli.Info("Service snapshot %s", plan.SnapshotRevision) cli.Info("Changed files:") diff --git a/docs/commands.md b/docs/commands.md index 4c5c4781..ca23799e 100644 --- a/docs/commands.md +++ b/docs/commands.md @@ -1131,6 +1131,12 @@ Publish clones the selected GitOps repository, commits and advertises the immutable service/module snapshot, derives exact AppProject authority from that snapshot, and adds CLI-owned Applications pinned to its commit and paths. It then creates the signed publication commit and opens or updates a pull request. +An existing promotion branch is continued only when it contains the base +branch; one the base does not contain (its pull request squash-merged and the +branch kept) is restarted from the base and replaced with a push leased on the +tip the plan reviewed, which the plan prints as the revision it replaces. A +stale branch carrying a change outside the module's path is refused, never +overwritten. Planning does not advertise the snapshot or mutate the remote. Observe requires an approved, merged pull request and verifies the publication digest, the snapshot revision bound into every Application, exact service paths, project diff --git a/docs/module-manifests.md b/docs/module-manifests.md index 434ceb71..b5639925 100644 --- a/docs/module-manifests.md +++ b/docs/module-manifests.md @@ -487,8 +487,12 @@ The Job is an Argo CD **Sync hook** (`argocd.argoproj.io/hook: Sync`, on every sync from state that still exists, so a replay after a restore is an ordinary sync, and a sync that changes nothing re-POSTs a generation the host replays. Its token is projected for the host's audience and re-read per -request; it runs a digest-pinned `curl` image, read-only, as a non-root user -with every capability dropped. The script spells its variables `$NAME`, never +request; it runs codefly's own base companion image +(`ghcr.io/codefly-dev/codefly`, pinned by digest — never a Docker Hub image), +read-only, as a non-root user with every capability dropped. The cell file +declares that same image as the namespace's delivery carrier, so moving its +digest changes every cell file: re-render the cell and publish it with, or +before, the first Job that runs the new image. The script spells its variables `$NAME`, never `${NAME}` (the promotable ruleset refuses `${…}` as an unresolved placeholder), and the token file is named `DELIVERY_IDENTITY_FILE` (a variable named `*_TOKEN` carrying a value is a credential to that ruleset). @@ -870,6 +874,18 @@ environment — or of a local one whose host lists no developer — moves only b a merge after review, as before. `remote up` with no module serves the latest promotion, and re-running it after a new one moves Argo CD to it. +The hosted counterpart of the same rule: a reviewed publish continues its +promotion branch only while that branch **contains the base**. A branch the +base does not contain — its pull request was squash-merged and the branch +kept, or another promotion moved the base past it — is stale, and a commit +stacked on it would fork from the old base and conflict add/add on every file +the base already holds. So the publish starts again from the base and replaces +the branch with a push leased on the stale tip it reviewed +(`--force-with-lease`), recorded in the plan as `replacesBranchRevision` and +part of its ID: a branch that moved after review refuses the publish. A stale +branch carrying anything outside the module's own path is someone else's work +and is refused by name, never overwritten. + **How a local pod's identity reaches the host.** Not through anything the run projects: there is no `MODULE_IDENTITY_PREFIX`, no secret and no `MODULE_PRINCIPALS` line, and nothing self-registers (cli#855 removed that on diff --git a/pkg/gitops/delivery_carrier_contract_test.go b/pkg/gitops/delivery_carrier_contract_test.go index 001b8b9b..ea3f9803 100644 --- a/pkg/gitops/delivery_carrier_contract_test.go +++ b/pkg/gitops/delivery_carrier_contract_test.go @@ -49,3 +49,16 @@ func TestTheCellSelectorIsTheLabelSetTheCarrierPodCarries(t *testing.T) { require.Equal(t, deliveryServiceAccount, delivery.ServiceAccount, "the cell admits the account the delivery Job runs as") } + +// The cell file declares the carrier's image as codefly's own repository on +// ghcr.io, pinned by digest: the image a cell admits for delivery is the one +// the render pins, and it is never pulled from Docker Hub. +func TestTheCellDeclaresCodeflysOwnDeliveryImage(t *testing.T) { + env := &environments.Environment{Name: "prod", Host: testHost()} + delivery, err := cellDelivery(env, "example-prod-shop") + require.NoError(t, err) + require.Equal(t, "ghcr.io/codefly-dev/codefly", delivery.Image.Repository) + require.Regexp(t, `^sha256:[0-9a-f]{64}$`, delivery.Image.Digest) + require.Equal(t, delivery.Image.Repository+"@"+delivery.Image.Digest, deliveryImage, + "the cell's carrier image is the image the delivery Job runs") +} diff --git a/pkg/gitops/delivery_jobs.go b/pkg/gitops/delivery_jobs.go index 30b62568..f6ef19f4 100644 --- a/pkg/gitops/delivery_jobs.go +++ b/pkg/gitops/delivery_jobs.go @@ -55,13 +55,18 @@ import ( // rather than discovered when a solution never appears. const ( - // deliveryImage is the image the delivery Jobs run: a digest-pinned curl. - // It is the second third-party image the render itself pins (remoteImage - // is the first), and it is pinned the same way, by digest, so that what a - // Job runs is what was reviewed. A codefly-owned delivery image would - // replace it; until one is published, the pinned digest is the honest - // record of what delivers. - deliveryImage = "docker.io/curlimages/curl@sha256:d94d07ba9e7d6de898b6d96c1a072f6f8266c687af78a74f380087a0addf5d17" + // deliveryImage is the image the delivery Jobs run: codefly's own base + // companion, published by codefly on ghcr.io and pinned by digest (the + // multi-arch index of tag 0.0.4), so that what a Job runs is what was + // reviewed. It carries everything deliveryScript calls — /bin/sh, curl, + // date, basename, cat, tail, sed, printf, sleep — and runs as the + // non-root user on a read-only root filesystem. A delivery Job never + // pulls from Docker Hub: a cell's nodes are not promised a route to it, + // and its anonymous pulls are rate-limited. The cell file declares this + // same image (cellDelivery), so moving the digest changes every cell file + // and the cell must be re-rendered and published with, or before, the + // first Job that runs the new one. + deliveryImage = "ghcr.io/codefly-dev/codefly@sha256:8fcaaea34622eb1743a5b755c7c6540a0be245cb37c8b2188732f45c8207a833" // deliveryContainerName is the one container of a delivery Job. deliveryContainerName = "deliver" diff --git a/pkg/gitops/delivery_jobs_test.go b/pkg/gitops/delivery_jobs_test.go index 8ae1e642..571fae02 100644 --- a/pkg/gitops/delivery_jobs_test.go +++ b/pkg/gitops/delivery_jobs_test.go @@ -71,9 +71,14 @@ func TestDeliveryJobIsASyncHookThatPostsEveryCarrier(t *testing.T) { if spec.ServiceAccountName != deliveryServiceAccount || spec.AutomountServiceAccountToken || spec.RestartPolicy != "OnFailure" { t.Fatalf("pod spec %+v", spec) } - if len(spec.Containers) != 1 || spec.Containers[0].Image != deliveryImage || !strings.Contains(deliveryImage, "@sha256:") { + if len(spec.Containers) != 1 || spec.Containers[0].Image != deliveryImage { t.Fatalf("container %+v", spec.Containers) } + // The carrier is codefly's own image, pinned by digest: never a Docker Hub + // image a cell's nodes may not reach and whose anonymous pulls are capped. + if !strings.HasPrefix(deliveryImage, "ghcr.io/codefly-dev/") || !strings.Contains(deliveryImage, "@sha256:") { + t.Fatalf("delivery image %q is not a digest-pinned ghcr.io/codefly-dev/ image", deliveryImage) + } env := map[string]string{} for _, entry := range spec.Containers[0].Env { env[entry.Name] = entry.Value diff --git a/pkg/gitops/publish.go b/pkg/gitops/publish.go index f512b577..fe3ee67f 100644 --- a/pkg/gitops/publish.go +++ b/pkg/gitops/publish.go @@ -203,7 +203,7 @@ func preparePublish( if err != nil { return nil, err } - repo, cleanup, baseRevision, branchRevision, err := clonePromotionRepository(ctx, source, baseBranch, promotionBranch, selfPromotes, bound) + repo, cleanup, baseRevision, branchRevision, replacesRevision, err := clonePromotionRepository(ctx, source, baseBranch, promotionBranch, selfPromotes, bound) if err != nil { return nil, err } @@ -216,10 +216,10 @@ func preparePublish( if err != nil { return fail(err) } - if branchRevision != "" { - if err = refuseUnrelatedPromotionChanges(ctx, repo, baseRevision, branchRevision, promotionBranch, targetPath); err != nil { - return fail(err) - } + // A stale branch about to be replaced is held to the module path too: one + // carrying someone else's change is refused, never overwritten. + if err = refuseUnrelatedPromotionChanges(ctx, repo, baseRevision, promotionBranch, targetPath, branchRevision, replacesRevision); err != nil { + return fail(err) } startRevision := baseRevision if branchRevision != "" { @@ -268,8 +268,8 @@ func preparePublish( Repository: config.RepoURL, RepositorySlug: repositorySlug, Path: targetPath, BaseBranch: baseBranch, BaseRevision: baseRevision, PromotionBranch: promotionBranch, BranchRevision: branchRevision, - ExistingCommit: branchRevision, - Module: request.Module, Environment: request.Environment, + ExistingCommit: branchRevision, ReplacesBranchRevision: replacesRevision, + Module: request.Module, Environment: request.Environment, RenderDigest: inventory.Digest, SnapshotRevision: snapshotRevision, Changed: changed, Diff: diff, ContractChecks: contractChecks, Delivery: inventory.Delivery, @@ -527,17 +527,23 @@ func refuseSharedDeliveryPath(workspace *resources.Workspace, environment string return nil } -// refuseUnrelatedPromotionChanges holds an existing promotion branch to the -// module's own path and its cell: anything else on it is someone else's work, -// which this publish must neither carry nor overwrite. -func refuseUnrelatedPromotionChanges(ctx context.Context, repo, baseRevision, branchRevision, promotionBranch, targetPath string) error { - existing, err := changedPathsBetween(ctx, repo, baseRevision, branchRevision) - if err != nil { - return err - } - for _, changed := range existing { - if changed != targetPath && !strings.HasPrefix(changed, targetPath+"/") { - return fmt.Errorf("promotion branch %s contains unrelated change %s", promotionBranch, changed) +// refuseUnrelatedPromotionChanges holds an existing promotion branch — the +// tip a publish continues, or the stale tip it replaces — to the module's own +// path and its cell: anything else on it is someone else's work, which this +// publish must neither carry nor overwrite. An empty revision is no branch. +func refuseUnrelatedPromotionChanges(ctx context.Context, repo, baseRevision, promotionBranch, targetPath string, branchRevisions ...string) error { + for _, branchRevision := range branchRevisions { + if branchRevision == "" { + continue + } + existing, err := changedPathsBetween(ctx, repo, baseRevision, branchRevision) + if err != nil { + return err + } + for _, changed := range existing { + if changed != targetPath && !strings.HasPrefix(changed, targetPath+"/") { + return fmt.Errorf("promotion branch %s contains unrelated change %s", promotionBranch, changed) + } } } return nil @@ -648,7 +654,8 @@ func stageRollbackPublication( // the base branch (refs/remotes/origin/baseBranch) rather than repo's checked- // out working tree. The working tree is the promotion branch, which is // long-lived and reused across every publish for the module owning it -// (clonePromotionRepository checks it out as-is when it already exists); only +// (clonePromotionRepository checks it out as-is when it already exists and +// contains the base); only // the one module path actively being republished ever gets refreshed from // history, so any *other* module's working-tree copy can be arbitrarily stale // relative to main. Reading the git blob directly from the base branch is the @@ -1878,8 +1885,18 @@ func commitAndPublish(ctx context.Context, workspace *resources.Workspace, prepa return PublishResult{}, err } } else if len(prepared.plan.Changed) > 0 { - refspec := "refs/heads/" + prepared.plan.PromotionBranch + ":refs/heads/" + prepared.plan.PromotionBranch - if _, err = gitCommand(ctx, prepared.dir, "push", "--porcelain", "--set-upstream", "--", "origin", refspec); err != nil { + branchRef := "refs/heads/" + prepared.plan.PromotionBranch + refspec := branchRef + ":" + branchRef + push := []string{"push", "--porcelain", "--set-upstream"} + if replaces := prepared.plan.ReplacesBranchRevision; replaces != "" { + // The reviewed plan replaces a stale branch: only the exact tip it + // reviewed is overwritten, and a branch that moved since refuses. + push = append(push, "--force-with-lease="+branchRef+":"+replaces) + } + if _, err = gitCommand(ctx, prepared.dir, append(push, "--", "origin", refspec)...); err != nil { + if prepared.plan.ReplacesBranchRevision != "" { + return PublishResult{}, fmt.Errorf("replace stale promotion branch %s (leased at %s): %w", prepared.plan.PromotionBranch, prepared.plan.ReplacesBranchRevision, err) + } return PublishResult{}, fmt.Errorf("push promotion branch without force: %w", err) } } @@ -1916,10 +1933,15 @@ func commitAndPublish(ctx context.Context, workspace *resources.Workspace, prepa return result, nil } -func clonePromotionRepository(ctx context.Context, repository, baseBranch, promotionBranch string, selfPromotes bool, bound *LocalRepositoryIdentity) (string, func(), string, string, error) { +// clonePromotionRepository checks the promotion branch out of a fresh clone of +// the delivery repository. It returns the clone, its cleanup, the base +// revision, the revision of the promotion branch the publish continues ("" +// when it starts from the base), and the tip of a stale promotion branch the +// publish replaces ("" when none). +func clonePromotionRepository(ctx context.Context, repository, baseBranch, promotionBranch string, selfPromotes bool, bound *LocalRepositoryIdentity) (string, func(), string, string, string, error) { temp, err := os.MkdirTemp("", "codefly-gitops-publish-") if err != nil { - return "", nil, "", "", fmt.Errorf("create publication checkout: %w", err) + return "", nil, "", "", "", fmt.Errorf("create publication checkout: %w", err) } cleanup := func() { _ = os.RemoveAll(temp) } repo := filepath.Join(temp, "repo") @@ -1930,25 +1952,26 @@ func clonePromotionRepository(ctx context.Context, repository, baseBranch, promo } if err != nil { cleanup() - return "", nil, "", "", err + return "", nil, "", "", "", err } if _, err = gitCommand(ctx, repo, "check-ref-format", "--branch", baseBranch); err != nil { cleanup() - return "", nil, "", "", fmt.Errorf("invalid configured GitOps branch %q: %w", baseBranch, err) + return "", nil, "", "", "", fmt.Errorf("invalid configured GitOps branch %q: %w", baseBranch, err) } baseRef := "refs/remotes/origin/" + baseBranch baseRevision, err := gitCommand(ctx, repo, "rev-parse", baseRef+"^{commit}") if err != nil { cleanup() - return "", nil, "", "", fmt.Errorf("resolve configured GitOps branch %q: %w", baseBranch, err) + return "", nil, "", "", "", fmt.Errorf("resolve configured GitOps branch %q: %w", baseBranch, err) } if _, err = gitCommand(ctx, repo, "check-ref-format", "--branch", promotionBranch); err != nil { cleanup() - return "", nil, "", "", fmt.Errorf("invalid promotion branch %q: %w", promotionBranch, err) + return "", nil, "", "", "", fmt.Errorf("invalid promotion branch %q: %w", promotionBranch, err) } remoteRef := "refs/remotes/origin/" + promotionBranch branchRevision, branchErr := gitCommand(ctx, repo, "rev-parse", "--verify", remoteRef+"^{commit}") - if branchErr == nil && selfPromotes { + continues := branchErr == nil + if continues && selfPromotes { // A local promotion continues from the last one: a promotion branch // the base already contains (this module's, or one another module's // promotion since moved past) starts again from the base, so the @@ -1960,19 +1983,32 @@ func clonePromotionRepository(ctx context.Context, repository, baseBranch, promo remoteRef, branchRevision = baseRef, baseRevision } else if !gitIsAncestor(ctx, repo, baseRevision, branchRevision) { cleanup() - return "", nil, "", "", fmt.Errorf("local promotion branch %s carries commits the base branch %s does not, and %s has moved past it; a local promotion fast-forwards %s and never forces, so delete the stale branch in the local delivery repository and publish again", promotionBranch, baseBranch, baseBranch, baseBranch) - } - } - if branchErr == nil { + return "", nil, "", "", "", fmt.Errorf("local promotion branch %s carries commits the base branch %s does not, and %s has moved past it; a local promotion fast-forwards %s and never forces, so delete the stale branch in the local delivery repository and publish again", promotionBranch, baseBranch, baseBranch, baseBranch) + } + } + var replacesRevision string + if continues && !selfPromotes && !gitIsAncestor(ctx, repo, baseRevision, branchRevision) { + // A reviewed promotion continues its branch only while the branch + // contains the base. One that does not is stale — its pull request + // was squash-merged and the branch kept, or the base moved past it — + // and a commit on it would fork from an old base, so every file it + // shares with the base conflicts. The publish starts from the base + // and replaces the branch, leased on the tip it found + // (ReplacesBranchRevision); the caller still holds that tip to the + // module path, so someone else's change on it is refused, never + // overwritten. + replacesRevision, continues = branchRevision, false + } + if continues { if _, err = gitCommand(ctx, repo, "checkout", "--quiet", "-b", promotionBranch, remoteRef); err != nil { cleanup() - return "", nil, "", "", err + return "", nil, "", "", "", err } } else { branchRevision = "" if _, err = gitCommand(ctx, repo, "checkout", "--quiet", "-b", promotionBranch, baseRef); err != nil { cleanup() - return "", nil, "", "", err + return "", nil, "", "", "", err } // A local promotion with no branch yet starts from the last // promotion, which is the base: nothing new is then "already @@ -1984,13 +2020,13 @@ func clonePromotionRepository(ctx context.Context, repository, baseBranch, promo status, err := gitCommand(ctx, repo, "status", "--porcelain=v1") if err != nil { cleanup() - return "", nil, "", "", err + return "", nil, "", "", "", err } if status != "" { cleanup() - return "", nil, "", "", fmt.Errorf("publication checkout is unexpectedly dirty") + return "", nil, "", "", "", fmt.Errorf("publication checkout is unexpectedly dirty") } - return repo, cleanup, baseRevision, branchRevision, nil + return repo, cleanup, baseRevision, branchRevision, replacesRevision, nil } // restoreDeliveredOverlays puts the delivery directories of a module path back diff --git a/pkg/gitops/publish_test.go b/pkg/gitops/publish_test.go index 707971a0..2b8d3c14 100644 --- a/pkg/gitops/publish_test.go +++ b/pkg/gitops/publish_test.go @@ -1122,6 +1122,110 @@ func TestPublishPreservesSnapshotLineageAfterSquashAndBranchDeletion(t *testing. } } +// A promotion whose pull request was squash-merged while its branch was kept +// leaves a branch that does not contain the base. The next publish starts +// again from the base and replaces that branch under a lease on its old tip, +// rather than stacking a commit that forks from the old base and conflicts +// add/add on every file the squash already delivered. +func TestPublishRestartsAPromotionBranchItsSquashMergeLeftBehind(t *testing.T) { + ctx := context.Background() + remote := createBareRepository(t) + workspace := loadGitopsWorkspace(t, remote) + renderPublishFixture(t, workspace.Dir(), "payments", "production", "api") + configureSSHSigning(t) + request := PublishRequest{ + Module: "payments", Environment: "production", Local: true, + PromotionBranch: "codefly/promote-payments-production", + } + firstPlan, err := PlanPublish(ctx, workspace, &request) + if err != nil { + t.Fatal(err) + } + first, err := Publish(ctx, workspace, &PublishMutation{Request: request, PlanID: firstPlan.ID}, preparedPermit) + if err != nil { + t.Fatal(err) + } + + work := t.TempDir() + gitRun(t, "", "clone", remote, work) + gitRun(t, work, "config", "user.name", "Codefly Test") + gitRun(t, work, "config", "user.email", "codefly@example.com") + gitRun(t, work, "config", "commit.gpgsign", "false") + gitRun(t, work, "merge", "--squash", "origin/"+request.PromotionBranch) + gitRun(t, work, "commit", "-m", "squash promotion") + gitRun(t, work, "push", "origin", "main") + + renderPublishFixture(t, workspace.Dir(), "payments", "production", "worker") + secondPlan, err := PlanPublish(ctx, workspace, &request) + if err != nil { + t.Fatal(err) + } + if secondPlan.ReplacesBranchRevision != first.Commit || secondPlan.BranchRevision != "" || secondPlan.ExistingCommit != "" { + t.Fatalf("plan replaces %q (branch %q, existing %q), want it to replace the squash-merged tip %s", + secondPlan.ReplacesBranchRevision, secondPlan.BranchRevision, secondPlan.ExistingCommit, first.Commit) + } + second, err := Publish(ctx, workspace, &PublishMutation{Request: request, PlanID: secondPlan.ID}, preparedPermit) + if err != nil { + t.Fatal(err) + } + branch := "refs/heads/" + request.PromotionBranch + if tip := gitOutput(t, "", "--git-dir", remote, "rev-parse", branch); tip != second.Commit { + t.Fatalf("promotion branch is at %s, want the new commit %s", tip, second.Commit) + } + gitRun(t, "", "--git-dir", remote, "merge-base", "--is-ancestor", "main", branch) + for _, changed := range strings.Split(gitOutput(t, "", "--git-dir", remote, "diff", "--name-only", "main", branch), "\n") { + if changed != secondPlan.Path && !strings.HasPrefix(changed, secondPlan.Path+"/") { + t.Fatalf("the replaced branch changes %q outside the module path %s", changed, secondPlan.Path) + } + } + gitRun(t, "", "--git-dir", remote, "merge-tree", "--write-tree", "main", branch) + gitRun(t, "", "--git-dir", remote, "merge-base", "--is-ancestor", first.SnapshotRevision, second.SnapshotRevision) + if first.SnapshotRevision == second.SnapshotRevision { + t.Fatal("changed service tree reused the previous snapshot") + } +} + +// A stale promotion branch is replaced only when it carries nothing but the +// module's own path: one holding someone else's change is refused even after +// the base moved past it, never overwritten. +func TestPublishRefusesToReplaceAStaleBranchCarryingUnrelatedChanges(t *testing.T) { + remote := createBareRepository(t) + workspace := loadGitopsWorkspace(t, remote) + renderPublishFixture(t, workspace.Dir(), "payments", "production", "api") + + work := t.TempDir() + gitRun(t, "", "clone", remote, work) + gitRun(t, work, "config", "user.name", "Codefly Test") + gitRun(t, work, "config", "user.email", "codefly@example.com") + gitRun(t, work, "config", "commit.gpgsign", "false") + gitRun(t, work, "checkout", "-b", "codefly/promote-payments-production") + if err := os.WriteFile(filepath.Join(work, "unrelated.txt"), []byte("outside promotion\n"), 0o644); err != nil { + t.Fatal(err) + } + gitRun(t, work, "add", "unrelated.txt") + gitRun(t, work, "commit", "-m", "unrelated") + gitRun(t, work, "push", "origin", "codefly/promote-payments-production") + stale := gitOutput(t, work, "rev-parse", "HEAD") + gitRun(t, work, "checkout", "main") + if err := os.WriteFile(filepath.Join(work, "elsewhere.txt"), []byte("the base moves on\n"), 0o644); err != nil { + t.Fatal(err) + } + gitRun(t, work, "add", "elsewhere.txt") + gitRun(t, work, "commit", "-m", "base moves") + gitRun(t, work, "push", "origin", "main") + + _, err := PlanPublish(context.Background(), workspace, &PublishRequest{ + Module: "payments", Environment: "production", Local: true, + PromotionBranch: "codefly/promote-payments-production", + }) + if err == nil || !strings.Contains(err.Error(), "unrelated change") { + t.Fatalf("plan error = %v", err) + } + if tip := gitOutput(t, "", "--git-dir", remote, "rev-parse", "refs/heads/codefly/promote-payments-production"); tip != stale { + t.Fatalf("the stale branch moved to %s, want it left at %s", tip, stale) + } +} + func TestPublishRejectsUnrelatedExistingPromotionChanges(t *testing.T) { remote := createBareRepository(t) workspace := loadGitopsWorkspace(t, remote) @@ -1288,27 +1392,34 @@ func TestRollbackRefusesUnresolvedConsumedContractUnlessAllowed(t *testing.T) { Package: "saas.accounts.v1", Digest: testContractDigestA, Version: "0.1.0", }} destination := filepath.Join(workspace.Dir(), "deployments", "modules", "payments") - _, err := RenderOwnedTree(ctx, &RenderOptions{ - Destination: destination, Module: "payments", UnitNames: []string{"api"}, - OwnedPath: filepath.ToSlash(filepath.Join("environments", "modules", "payments")), - Units: graph, - Environment: "production", Namespace: "payments", AppProject: "payments", Promotable: true, - }, func(_ context.Context, stage string) error { - return writePublishServiceFixture(stage, "production", "api") - }) - if err != nil { - t.Fatal(err) - } - - plan, err := PlanPublish(ctx, workspace, &request) - if err != nil { - t.Fatal(err) - } - published, err := Publish(ctx, workspace, &PublishMutation{Request: request, PlanID: plan.ID}, preparedPermit) - if err != nil { - t.Fatal(err) + // Each promotion is reviewed and merged; the rollback then targets the + // first while the base delivers the second, so it has a tree to re-promote. + promote := func(service string) PublishResult { + t.Helper() + _, err := RenderOwnedTree(ctx, &RenderOptions{ + Destination: destination, Module: "payments", UnitNames: []string{"api"}, + OwnedPath: filepath.ToSlash(filepath.Join("environments", "modules", "payments")), + Units: graph, + Environment: "production", Namespace: "payments", AppProject: "payments", Promotable: true, + }, func(_ context.Context, stage string) error { + return writePublishServiceFixture(stage, "production", service) + }) + if err != nil { + t.Fatal(err) + } + plan, err := PlanPublish(ctx, workspace, &request) + if err != nil { + t.Fatal(err) + } + published, err := Publish(ctx, workspace, &PublishMutation{Request: request, PlanID: plan.ID}, preparedPermit) + if err != nil { + t.Fatal(err) + } + mergePromotionToMain(t, remote, request.PromotionBranch) + return published } - mergePromotionToMain(t, remote, request.PromotionBranch) + published := promote("api") + promote("worker") if err := writeReceipt(workspace.Dir(), "evidence", "first.json", Evidence{ SchemaVersion: EvidenceSchemaVersion, Module: "payments", Environment: "production", RenderDigest: published.RenderDigest, SignedCommit: published.Commit, Tree: published.Tree, diff --git a/pkg/gitops/types.go b/pkg/gitops/types.go index 111c9d9b..5e19f335 100644 --- a/pkg/gitops/types.go +++ b/pkg/gitops/types.go @@ -463,22 +463,29 @@ type PublishRequest struct { } type PublishPlan struct { - ID string `json:"id"` - Repository string `json:"repository"` - RepositorySlug string `json:"repositorySlug,omitempty"` - Path string `json:"path"` - BaseBranch string `json:"baseBranch"` - BaseRevision string `json:"baseRevision"` - PromotionBranch string `json:"promotionBranch"` - BranchRevision string `json:"branchRevision,omitempty"` - ExistingCommit string `json:"existingCommit,omitempty"` - Module string `json:"module"` - Environment string `json:"environment"` - RenderDigest string `json:"renderDigest"` - SnapshotRevision string `json:"snapshotRevision"` - Changed []string `json:"changed"` - Diff string `json:"diff"` - ContractChecks []ContractCheck `json:"contractChecks,omitempty"` // Delivery is what the publication settled and signed, for the plan's + ID string `json:"id"` + Repository string `json:"repository"` + RepositorySlug string `json:"repositorySlug,omitempty"` + Path string `json:"path"` + BaseBranch string `json:"baseBranch"` + BaseRevision string `json:"baseRevision"` + PromotionBranch string `json:"promotionBranch"` + BranchRevision string `json:"branchRevision,omitempty"` + ExistingCommit string `json:"existingCommit,omitempty"` + // ReplacesBranchRevision is the tip of a stale promotion branch this + // publish replaces: a branch that does not contain the base branch (its + // pull request was squash-merged and the branch kept, or the base moved + // past it), which the publish restarts from the base and pushes over with + // a lease on exactly this revision. Part of the plan's identity, so a + // branch that moves after review refuses the publish. + ReplacesBranchRevision string `json:"replacesBranchRevision,omitempty"` + Module string `json:"module"` + Environment string `json:"environment"` + RenderDigest string `json:"renderDigest"` + SnapshotRevision string `json:"snapshotRevision"` + Changed []string `json:"changed"` + Diff string `json:"diff"` + ContractChecks []ContractCheck `json:"contractChecks,omitempty"` // Delivery is what the publication settled and signed, for the plan's // reader and the result. Delivery *InventoryDelivery `json:"delivery,omitempty"` // StaleConsumers names the modules the delivery base branch holds at