diff --git a/cmd/deploy/gitops.go b/cmd/deploy/gitops.go index a4837d85..84543f76 100644 --- a/cmd/deploy/gitops.go +++ b/cmd/deploy/gitops.go @@ -594,6 +594,12 @@ func printPublishPlan(plan *gitops.PublishPlan) { for _, removal := range plan.CellRemovals { cli.Warning("The cell file removes %s, as --allow-cell-removal names", removal) } + for _, removal := range plan.UndeclaredCellRemovals { + cli.Warning("The cell file removes %s", removal) + } + for _, removed := range plan.RemovedModules { + cli.Warning("Removed %s from the delivery: the composition no longer declares it", removed) + } for _, stale := range plan.StaleConsumers { cli.Warning("Delivered consumer %s bakes in another value of a workspace configuration group this publication carries; publish it next, so %s does not keep delivering two values of one group", stale, plan.Environment) } @@ -657,7 +663,7 @@ func init() { } for _, command := range []*cobra.Command{gitOpsPlanCmd, gitOpsPublishCmd} { command.Flags().StringSliceVar(&gitOpsAllowCellRemovals, "allow-cell-removal", nil, - "Module whose removal from the environment's cell file, relative to the base branch's, is intended (repeatable); any other removal refuses") + "Module whose removal from the environment's cell file, relative to the base branch's, is intended (repeatable); a module the composition no longer declares is removed without it, and any other removal refuses") } for _, command := range []*cobra.Command{gitOpsPublishCmd, gitOpsRollbackCmd} { command.Flags().StringVar(&gitOpsMessage, "message", "", "Signed commit message") @@ -696,6 +702,9 @@ func printCell(cell *gitops.CellResult) { if len(cell.Skipped) > 0 { cli.Info("Cell file leaves out %s: rendered for another environment", strings.Join(cell.Skipped, ", ")) } + for _, removed := range cell.Removed { + cli.Info("removed %s: the composition no longer declares it", removed) + } if !cell.Source { cli.Info("Cell file names no source commit: the environment declares no gitops repository and branch, or the workspace is not a git checkout") } diff --git a/docs/cli-reference.md b/docs/cli-reference.md index 376398e3..7321a7ae 100644 --- a/docs/cli-reference.md +++ b/docs/cli-reference.md @@ -2274,7 +2274,7 @@ codefly deploy gitops plan [module] Flags: ``` - --allow-cell-removal strings Module whose removal from the environment's cell file, relative to the base branch's, is intended (repeatable); any other removal refuses + --allow-cell-removal strings Module whose removal from the environment's cell file, relative to the base branch's, is intended (repeatable); a module the composition no longer declares is removed without it, and any other removal refuses --allow-unresolved-contracts Downgrade a consumed contract whose exposing module is not yet deployed to a warning, for bootstrap ordering --env string Environment to promote (default "local") --local Use a disposable local file Git remote for k3d qualification @@ -2294,7 +2294,7 @@ codefly deploy gitops publish [module] Flags: ``` - --allow-cell-removal strings Module whose removal from the environment's cell file, relative to the base branch's, is intended (repeatable); any other removal refuses + --allow-cell-removal strings Module whose removal from the environment's cell file, relative to the base branch's, is intended (repeatable); a module the composition no longer declares is removed without it, and any other removal refuses --allow-unresolved-contracts Downgrade a consumed contract whose exposing module is not yet deployed to a warning, for bootstrap ordering --body string Promotion pull request body --env string Environment to promote (default "local") diff --git a/docs/commands.md b/docs/commands.md index 8af8d527..ae8248c7 100644 --- a/docs/commands.md +++ b/docs/commands.md @@ -1150,6 +1150,16 @@ and egress the environment's), so an entry carried over is a claim about a composition that may no longer exist, and no digest is invented for a module that was not rendered. +The file is derived from the **composition**, not from whatever records the +checkout holds. A module removed from `modules:` leaves its render record +behind, and so does one that was never composed here; such a record is left +out of the file and deleted by the render that derives it — nothing could +render that module again — and the render says so, one line per record +(`removed deployments/modules/: the composition no longer declares +it`). Every render that re-derives the cell file prunes this way, a single +module's and `--all` alike, because that is where the claim about the cell is +made; an environment with no `host:` has no cell file and nothing is pruned. + **A publish carries the cell file.** A cell admits only the builds its cell file names, so a promotion that delivered a module's tree and left the file behind would deliver builds the cluster refuses — every refused workload @@ -1188,9 +1198,39 @@ committed with the rendered trees, and a cell admits only what it names, so and the one the delivery base branch carries at `/cells//cell.yaml`, and refuse when the workspace's removes a namespace, a workload, an endpoint or a consumer of an endpoint the -base admits — naming each removal and the modules it concerns. A removal that -is intended (a module the workspace no longer composes, a dependency dropped) -is named with `--allow-cell-removal ` (repeatable); a consumer edge is +base admits — naming each removal and the modules it concerns. A removal the +composition explains needs no naming: when the module it concerns is no +longer in `modules:` (the namespace of a module removed from the composition, +or its consumer edge on another module's endpoint), **and the checkout has +seen the composition the base's file was rendered from**, the hold lets it +through and the plan reports it (`undeclaredCellRemovals`), and the +publication also removes that module's delivered tree from +`/modules/` with everything else it writes (`removedModules`): +the cell no longer admits anything of it, and the ApplicationSet that tree +carries would keep stamping Applications whose every pod the cell refuses. A +checkout that never had a module and one that dropped it derive the same cell +file, so the history decides: when the workspace repository is the delivery +repository (it holds the base commit, or the delivery holds the workspace +revision the base's file names as its `source.revision`), the checkout must +contain the delivery base commit itself — the file's source revision is the +render's parent, and a `modules:` edit rides in the render's own commit, so an +unfetched checkout at that parent has not seen it; when the two repositories +are separate, the checkout must contain that source revision, and the +workspace's own configuration **as committed at that revision** must have +declared the module (a render may run on an uncommitted `modules:` edit, and +a module of an imported workspace is another repository's history), or +nothing in this history vouches the module was removed rather than never +composed. A checkout that fails either test — a branch behind the base, an +unfetched one, a base file naming no source, a composition never committed — +is held as before, the refusal says what the history lacked, and naming the +module confirms the removal without pruning the tree. A promotion +of a module owns, on its own branch, the removal of such trees, so continuing +or retrying the publication that removed one is not refused as someone else's +change. A rollback removes none, and a directory under the modules path with +no render record is not a delivery and is left alone. Any other removal — a +module still composed whose render is missing from the checkout, a dependency +dropped, a service withdrawn — is +named with `--allow-cell-removal ` (repeatable); a consumer edge is covered by naming either the endpoint's module or the consumer's, and the plan lists every removal it lets through. A base with no cell file has nothing to hold the workspace's to. The file held is the file carried — the same bytes, diff --git a/pkg/gitops/cell.go b/pkg/gitops/cell.go index 7ac550a7..27e9b8e4 100644 --- a/pkg/gitops/cell.go +++ b/pkg/gitops/cell.go @@ -52,6 +52,10 @@ type CellResult struct { // Skipped names module trees rendered for another environment, which // describe another cell. Skipped []string + // Removed are the render records of modules the composition no longer + // declares, deleted by this render (pruneUndeclaredRecords), as paths + // relative to the workspace. + Removed []string // Source reports whether the file names its repository, branch and base // revision. It does not when the environment declares no gitops // repository or the workspace is not a git checkout. @@ -90,16 +94,78 @@ func CellPath(workspace *resources.Workspace, env *environments.Environment) (st // composition that produced them, and writes it. It runs after a module // render, so the file always describes the trees as they stand. func RenderCell(ctx context.Context, workspace *resources.Workspace, env *environments.Environment) (CellResult, error) { + removed, err := pruneUndeclaredRecords(workspace) + if err != nil { + return CellResult{}, err + } file, result, err := deriveCell(ctx, workspace, env) if err != nil { return CellResult{}, err } - if err := writeCell(&file, result.Path); err != nil { + if err = writeCell(&file, result.Path); err != nil { return CellResult{}, err } + result.Removed = removed return result, nil } +// pruneUndeclaredRecords deletes, before the cell file is derived, the render +// records of modules the composition no longer declares, for any environment. +// Nothing could render such a module again (a render refuses a module the +// workspace does not compose), so its record describes a composition that no +// longer exists; left in place it was read back into every cell file derived +// here, and a module removed from `modules:` kept its namespace, workloads and +// consumer edges in the cell. The derivation's own hold on such a tree +// (refuseUncomposedRender) stays what it is: a render satisfies it by removing +// the tree, which is what that refusal asks for, and answers the paths it +// removed, relative to the workspace, for the render to print. A record that +// cannot be read is left for the derivation to name. +func pruneUndeclaredRecords(workspace *resources.Workspace) ([]string, error) { + modulesDir := filepath.Dir(moduleRenderDestination(workspace, "module")) + entries, err := os.ReadDir(modulesDir) + if err != nil { + if errors.Is(err, fs.ErrNotExist) { + return nil, nil + } + return nil, fmt.Errorf("list rendered modules: %w", err) + } + declared := composedModules(workspace) + var removed []string + for _, entry := range entries { + if !entry.IsDir() { + continue + } + tree := filepath.Join(modulesDir, entry.Name()) + inventory, loadErr := LoadInventory(tree) + if loadErr != nil || declared[inventory.Module] { + continue + } + if err = os.RemoveAll(tree); err != nil { + return nil, fmt.Errorf("remove the render record of module %s, which the composition no longer declares: %w", inventory.Module, err) + } + relative, relErr := filepath.Rel(workspace.Dir(), tree) + if relErr != nil { + relative = tree + } + removed = append(removed, filepath.ToSlash(relative)) + } + sort.Strings(removed) + return removed, nil +} + +// composedModules names every module the composition declares, in this +// workspace or one it imports: the set a cell file describes and the set a +// removal from it is judged against. +func composedModules(workspace *resources.Workspace) map[string]bool { + declared := make(map[string]bool, len(workspace.Modules)) + for _, reference := range workspace.Modules { + if reference != nil { + declared[reference.Name] = true + } + } + return declared +} + // deriveCell is RenderCell without the write: the cell file the rendered // trees and the composition describe, and the path it belongs at. func deriveCell(ctx context.Context, workspace *resources.Workspace, env *environments.Environment) (cellfile.File, CellResult, error) { @@ -156,7 +222,7 @@ func deriveCell(ctx context.Context, workspace *resources.Workspace, env *enviro result.Skipped = append(result.Skipped, inventory.Module) continue } - if err := refuseUncomposedRender(workspace, env, tree, &inventory); err != nil { + if err = refuseUncomposedRender(workspace, env, tree, &inventory); err != nil { return cellfile.File{}, CellResult{}, err } // A tree rendered before the environment excluded one of its services diff --git a/pkg/gitops/cell_removals.go b/pkg/gitops/cell_removals.go index e4e25637..e325725c 100644 --- a/pkg/gitops/cell_removals.go +++ b/pkg/gitops/cell_removals.go @@ -4,19 +4,29 @@ package gitops // cell admits only what it names, so a workload, a namespace or a consumer // absent from it is withdrawn. A publication promotes a render, so before it // does, the workspace's cell file is held to the one the delivery base branch -// carries, and a removal it does not name is refused: a cell file derived +// carries, and a removal nothing explains is refused: a cell file derived // from a checkout missing some modules' renders would otherwise remove those -// modules from admission without anyone having said so. +// modules from admission without anyone having said so. A removal the +// composition explains — the module left `modules:` — is carried without +// more, and the module's delivered tree goes with it: the cell no longer +// admits anything of it, and the ApplicationSet that tree carries would keep +// stamping Applications whose every pod the cell refuses. import ( "context" + "errors" "fmt" + "io/fs" + "os" + "path" "path/filepath" "slices" "strings" "github.com/codefly-dev/cli/pkg/environments" "github.com/codefly-dev/cli/pkg/gitops/cellfile" + "github.com/codefly-dev/core/resources" + "gopkg.in/yaml.v3" ) // cellRemoval is one thing the base cell file admits that the next one does @@ -87,20 +97,219 @@ func deliveryCellPath(gitopsPath, environment string) string { return filepath.ToSlash(filepath.Join(gitopsPath, cellsDir, environment, cellfile.FileName)) } -// holdCellToBase refuses the publication when the cell file it carries for a -// hosted environment removes anything the delivery base revision's file -// carries, unless every removal concerns a module allowed names. It returns -// the removals allowed, for the plan to show. A publication carrying no cell -// file (publicationCellFile), or a base with none, has nothing to hold. -// -// It is the cell file the publication CARRIES that is held — the same bytes, -// read once — so the file delivered and the file held can never be two -// different files, and the hold runs before anything is carried. -func holdCellToBase(ctx context.Context, cell *publicationCell, env *environments.Environment, repo, baseBranch, baseRevision string, allowed []string) ([]string, error) { +// compositionClaim is what a publication may remove from the cell and the +// delivery on the composition's word alone: a module the composition no +// longer declares — provided this checkout's composition has moved FORWARD +// from the one that rendered the delivery base's cell file, and that +// composition DECLARED the module. A checkout that never had a module and one +// that dropped it derive the same cell file, and only the history tells them +// apart: a checkout that contains the commit the base's file came from, whose +// composition named the module, has seen it declared, so its absence is a +// removal; one that does not is stale, or saw a composition that named the +// module only in an uncommitted edit or an imported workspace, and its +// publication keeps needing --allow-cell-removal, as every withdrawal nobody +// stated does. +type compositionClaim struct { + // module is the one being published: never a removal of its own. + module string + // declared is the composition's module set (composedModules). + declared map[string]bool + // advanced is whether the checkout descends from revision; revision is the + // commit it had to contain, empty when the base names none. + advanced bool + revision string + // declaredAt is the module set the workspace's own configuration named at + // revision (declaredAtRevision): what the checkout can vouch was removed. + declaredAt map[string]bool +} + +// newCompositionClaim judges the checkout against the delivery base. The +// commit the checkout must contain is the base revision itself whenever the +// workspace repository and the delivery repository are one — the workspace +// holds the base commit, or the delivery clone holds the workspace revision +// the base's cell file says it was rendered from (cellSource) — because that +// revision is the render's PARENT and a `modules:` edit rides in the render's +// own commit, so a checkout forked at the parent has not seen it; an unfetched +// checkout of one repository is therefore not advanced, and the hold says +// which commit it lacks. Only when the two repositories are separate (the +// clone does not hold the source revision) is the source revision what the +// checkout must contain — and there a render may have run on an uncommitted +// `modules:` edit, so the judged revision's own configuration must also have +// declared the module (declaredAtRevision), or nothing vouches the module was +// ever composed in this history. A base with no cell file, or one naming no +// source while the workspace lacks the base commit, leaves nothing to descend +// from. +func newCompositionClaim(ctx context.Context, workspace *resources.Workspace, clone, module, baseRevision string, base *cellfile.File) *compositionClaim { + claim := &compositionClaim{module: module, declared: composedModules(workspace)} + if base == nil { + return claim + } + dir := workspace.Dir() + head, err := gitCommand(ctx, dir, "rev-parse", "HEAD") + if err != nil { + return claim + } + // The source revision is read from the delivery branch's file and spliced + // into git arguments: only a full commit hash is one. + source := "" + if base.Source != nil && isCommitHash(base.Source.Revision) { + source = base.Source.Revision + } + switch { + case gitHasCommit(ctx, dir, baseRevision), source != "" && gitHasCommit(ctx, clone, source): + claim.revision = baseRevision + case source != "": + claim.revision = source + default: + return claim + } + claim.advanced = gitIsAncestor(ctx, dir, claim.revision, head) + if claim.advanced { + claim.declaredAt = declaredAtRevision(ctx, dir, claim.revision) + } + return claim +} + +// committedComposition is the part of a workspace configuration that names the +// modules it composes itself: a modules layout lists them, a flat layout IS +// one module named after the workspace. +type committedComposition struct { + Name string `yaml:"name"` + Layout string `yaml:"layout"` + Modules []struct{ Name string } `yaml:"modules"` + Solutions []struct{ Name string } `yaml:"solutions"` +} + +// declaredAtRevision reads the workspace's own configuration as committed at +// revision and names the modules it declared there: its `modules:` and +// `solutions:`, or the workspace itself when flat. A module reached only +// through an imported workspace is not among them — that import's history is +// another repository's — and a configuration the revision does not carry, or +// that does not parse, declares nothing, so the claim explains nothing. +func declaredAtRevision(ctx context.Context, dir, revision string) map[string]bool { + prefix, err := gitCommand(ctx, dir, "rev-parse", "--show-prefix") + if err != nil { + return nil + } + data, err := gitCommandBytes(ctx, dir, "show", revision+":"+path.Join(prefix, resources.WorkspaceConfigurationName)) + if err != nil { + return nil + } + var committed committedComposition + if err = yaml.Unmarshal(data, &committed); err != nil { + return nil + } + declared := map[string]bool{} + if committed.Layout == string(resources.LayoutKindFlat) && committed.Name != "" { + declared[committed.Name] = true + } + for _, list := range [][]struct{ Name string }{committed.Modules, committed.Solutions} { + for _, reference := range list { + if reference.Name != "" { + declared[reference.Name] = true + } + } + } + return declared +} + +// isCommitHash reports whether value is a full lowercase SHA-1 commit hash, +// the one shape a revision read from a file may take before it reaches git. +func isCommitHash(value string) bool { + if len(value) != 40 { + return false + } + for _, r := range value { + if (r < '0' || r > '9') && (r < 'a' || r > 'f') { + return false + } + } + return true +} + +// gitHasCommit reports whether the repository holds the commit. +func gitHasCommit(ctx context.Context, repo, revision string) bool { + _, err := gitCommand(ctx, repo, "cat-file", "-e", revision+"^{commit}") + return err == nil +} + +// undeclared reports a module the composition does not declare, other than +// the one being published. +func (claim *compositionClaim) undeclared(module string) bool { + return module != claim.module && !claim.declared[module] +} + +// explains reports a removal the composition accounts for: the module is no +// longer declared, by a checkout that descends from a composition that +// declared it. +func (claim *compositionClaim) explains(module string) bool { + return claim.advanced && claim.undeclared(module) && claim.declaredAt[module] +} + +// whyNotExplained says, for the modules the checkout does not declare whose +// removal it could not vouch for, what the history lacked: the revision to +// contain, or a composition at it that named them. +func (claim *compositionClaim) whyNotExplained(baseBranch string, modules []string) string { + names := strings.Join(modules, ", ") + switch { + case claim.revision == "": + return fmt.Sprintf(". This checkout's composition does not declare %s, but %s's cell file names no revision it was rendered from, so whether the composition removed %s or never had it cannot be told from here", names, baseBranch, names) + case !claim.advanced: + return fmt.Sprintf(". This checkout's composition does not declare %s, but the checkout does not contain %s, which %s's cell file comes from, so the removal may be a stale checkout's rather than the composition's: bring the checkout up to date and render again, or name it", names, claim.revision, baseBranch) + default: + return fmt.Sprintf(". This checkout's composition does not declare %s, and neither did its own configuration at %s, which %s's cell file comes from (a composition rendered before it was committed, or a module of an imported workspace), so nothing here vouches that %s was removed rather than never composed: name it", names, claim.revision, baseBranch, names) + } +} + +// owns answers, for the promotion-branch guard, whether a delivery-repository +// path lies in the delivered tree of a module the composition no longer +// declares. A promotion of this module owns those trees whether or not the +// clone still holds them: the branch it continues, or the stale tip it +// replaces, carries this module's own earlier removal of such a tree, and the +// base may since have taken it, so neither tree says what the branch removed. +func (claim *compositionClaim) owns(gitopsPath string) func(path string) bool { + prefix := deliveryModulesPath(gitopsPath) + "/" + return func(path string) bool { + rest, under := strings.CutPrefix(path, prefix) + if !under { + return false + } + name, _, _ := strings.Cut(rest, "/") + return name != "" && claim.undeclared(name) + } +} + +// compositionBase is where a publication's composition claim is judged: the +// promotion clone, the delivery base it was cut from, and the publication. +type compositionBase struct { + repo, gitopsPath, branch, revision string + module, restoreRevision string +} + +// judgeCompositionRemovals is the one step a publication takes on the +// composition's word: it reads the base's cell file, judges the checkout's +// claim against it and removes from the clone the delivered trees that claim +// explains, answering all three for the hold, the branch guard and the plan. +func judgeCompositionRemovals(ctx context.Context, workspace *resources.Workspace, env *environments.Environment, cell *publicationCell, base *compositionBase) (*cellfile.File, *compositionClaim, []string, error) { + baseCell, err := baseCellFile(ctx, cell, base.repo, base.branch, base.revision) + if err != nil { + return nil, nil, nil, err + } + claim := newCompositionClaim(ctx, workspace, base.repo, base.module, base.revision, baseCell) + removed, err := removeUndeclaredDeliveries(base.repo, base.gitopsPath, env, base.restoreRevision, claim) + if err != nil { + return nil, nil, nil, err + } + return baseCell, claim, removed, nil +} + +// baseCellFile reads the cell file the delivery base revision carries at the +// path the publication delivers to, nil when the base has none. A publication +// carrying no cell file is held to nothing, and reads none. +func baseCellFile(ctx context.Context, cell *publicationCell, repo, baseBranch, baseRevision string) (*cellfile.File, error) { if cell == nil { return nil, nil } - path, next := cell.source, cell.file baseData, err := gitCommandBytes(ctx, repo, "show", baseRevision+":"+cell.delivered) if err != nil { if gitSaysAbsent(err) { @@ -112,24 +321,125 @@ func holdCellToBase(ctx context.Context, cell *publicationCell, env *environment if err != nil { return nil, fmt.Errorf("%s's cell file %s cannot be read: %w", baseBranch, cell.delivered, err) } - var refused, admitted []string + return base, nil +} + +// holdCellToBase refuses the publication when the cell file it carries for a +// hosted environment removes anything the delivery base revision's file (base) +// carries, unless the removal is explained: the composition no longer +// declares the module it concerns and the checkout has seen it declared +// (compositionClaim), or a module allowed names. It returns the removals +// allowed by name, and the removals the composition explains, for the plan to +// show. A publication carrying no cell file, or a base with none, has nothing +// to hold. +// +// A removal the composition explains needs no naming: the flag confirms a +// removal nothing else accounts for — a module still composed whose render is +// missing from the checkout, a dependency dropped — where a module gone from +// `modules:` is the removal's own statement of intent, and asking for it again +// made every publication after such a change refuse until someone typed the +// module's name back in. +// +// It is the cell file the publication CARRIES that is held — the same bytes, +// read once — so the file delivered and the file held can never be two +// different files, and the hold runs before anything is carried. +func holdCellToBase(cell *publicationCell, env *environments.Environment, baseBranch string, base *cellfile.File, allowed []string, claim *compositionClaim) (named, undeclared []string, err error) { + if cell == nil || base == nil { + return nil, nil, nil + } + path, next := cell.source, cell.file + var refused, stale []string unnamed := map[string]bool{} for _, removal := range cellRemovals(base, next) { + if gone := slices.IndexFunc(removal.modules, claim.explains); gone >= 0 { + undeclared = append(undeclared, fmt.Sprintf("%s: the composition no longer declares module %s", removal.what, removal.modules[gone])) + continue + } if slices.ContainsFunc(removal.modules, func(module string) bool { return slices.Contains(allowed, module) }) { - admitted = append(admitted, removal.what) + named = append(named, removal.what) continue } refused = append(refused, removal.what) unnamed[removal.modules[len(removal.modules)-1]] = true + if gone := slices.IndexFunc(removal.modules, claim.undeclared); gone >= 0 && !slices.Contains(stale, removal.modules[gone]) { + stale = append(stale, removal.modules[gone]) + } + } + if len(refused) == 0 { + return named, undeclared, nil } - if len(refused) > 0 { - modules := make([]string, 0, len(unnamed)) - for module := range unnamed { - modules = append(modules, module) + modules := make([]string, 0, len(unnamed)) + for module := range unnamed { + modules = append(modules, module) + } + slices.Sort(modules) + message := fmt.Sprintf("environment %s's cell file %s removes from the cell what %s's admits, and says nothing of it: %s. Publishing it would withdraw those from the cell's admission set. If a module's render is missing from this checkout, render it (`codefly deploy gitops render --all --env %s`); if the removal is intended — a dependency dropped, a service withdrawn — name each module it concerns with --allow-cell-removal (%s)", + env.Name, path, baseBranch, strings.Join(refused, "; "), env.Name, strings.Join(modules, ", ")) + if len(stale) > 0 { + slices.Sort(stale) + message += claim.whyNotExplained(baseBranch, stale) + } + return nil, nil, errors.New(message) +} + +// undeclaredDeliveries names the module trees the delivery repository holds +// under the environment's modules path, as the promotion clone stands, whose +// removal the composition explains (compositionClaim): delivered once, and +// left behind when the module left `modules:`. A tree is a delivery when it +// carries a render record (InventoryFilename); anything else under that path +// is not this package's to judge. Paths are relative to the repository, +// slash-separated. +func undeclaredDeliveries(repo, gitopsPath string, claim *compositionClaim) ([]string, error) { + modulesPath := deliveryModulesPath(gitopsPath) + modulesDir, err := confinedJoin(repo, modulesPath) + if err != nil { + return nil, err + } + entries, err := os.ReadDir(modulesDir) + if errors.Is(err, fs.ErrNotExist) { + return nil, nil + } + if err != nil { + return nil, fmt.Errorf("list the delivered module trees under %s: %w", modulesPath, err) + } + var stale []string + for _, entry := range entries { + if !entry.IsDir() || !claim.explains(entry.Name()) { + continue + } + if _, err := os.Stat(filepath.Join(modulesDir, entry.Name(), InventoryFilename)); err != nil { + continue + } + stale = append(stale, deliveryModulePath(gitopsPath, entry.Name())) + } + return stale, nil +} + +// removeUndeclaredDeliveries deletes from the promotion clone the delivered +// trees of modules the composition no longer declares (undeclaredDeliveries), +// and answers their paths for the publication to stage and show. A forward +// publication to a hosted environment carries the composition's claim whole — +// the cell file it carries no longer admits those modules — so it is the one +// that removes them; a rollback restores one module's tree and claims nothing +// about the rest, and an environment with no host has no cell whose claim the +// trees contradict. Nothing is staged here: the removal reaches the promotion +// only with everything else the publication writes, after the hold. +func removeUndeclaredDeliveries(repo, gitopsPath string, env *environments.Environment, restoreRevision string, claim *compositionClaim) ([]string, error) { + if env == nil || env.Host == nil || restoreRevision != "" { + return nil, nil + } + deliveries, err := undeclaredDeliveries(repo, gitopsPath, claim) + if err != nil { + return nil, err + } + for _, delivery := range deliveries { + target, err := confinedJoin(repo, delivery) + if err != nil { + return nil, err + } + if err = os.RemoveAll(target); err != nil { + return nil, fmt.Errorf("remove the delivered tree %s of a module the composition no longer declares: %w", delivery, err) } - slices.Sort(modules) - return nil, fmt.Errorf("environment %s's cell file %s removes from the cell what %s's admits, and says nothing of it: %s. Publishing it would withdraw those from the cell's admission set. If a module's render is missing from this checkout, render it (`codefly deploy gitops render --all --env %s`); if the removal is intended — a module the workspace no longer composes, a dependency dropped — name each module it concerns with --allow-cell-removal (%s)", - env.Name, path, baseBranch, strings.Join(refused, "; "), env.Name, strings.Join(modules, ", ")) } - return admitted, nil + return deliveries, nil } diff --git a/pkg/gitops/cell_removals_test.go b/pkg/gitops/cell_removals_test.go index f94d3887..5b6b417d 100644 --- a/pkg/gitops/cell_removals_test.go +++ b/pkg/gitops/cell_removals_test.go @@ -4,6 +4,7 @@ import ( "context" "os" "path/filepath" + "strings" "testing" "github.com/codefly-dev/cli/pkg/environments" @@ -37,6 +38,26 @@ func publicationCellOf(t *testing.T, workspace *resources.Workspace, env *enviro return cell } +// holdAgainst holds the publication's cell file to the base revision's, as a +// publication does: the base file read from the delivery repository, the +// claim given. +func holdAgainst(t *testing.T, ctx context.Context, cell *publicationCell, env *environments.Environment, repo, baseRevision string, allowed []string, claim *compositionClaim) (named, explained []string, err error) { + t.Helper() + base, err := baseCellFile(ctx, cell, repo, "main", baseRevision) + require.NoError(t, err) + return holdCellToBase(cell, env, "main", base, allowed, claim) +} + +// advancedClaim is the claim of a checkout that descends from a composition +// that declared the modules named (removed) and no longer declares them. +func advancedClaim(workspace *resources.Workspace, removed ...string) *compositionClaim { + claim := &compositionClaim{declared: composedModules(workspace), advanced: true, revision: strings.Repeat("b", 40), declaredAt: map[string]bool{}} + for _, module := range removed { + claim.declaredAt[module] = true + } + return claim +} + // A publication holds the workspace's cell file to the one its delivery base // branch carries: a file that drops a module's namespace (and with it that // module's consumer edges on another's endpoints) is refused, naming each @@ -55,9 +76,11 @@ func TestPublishRefusesACellFileThatRemovesWhatTheBaseAdmits(t *testing.T) { require.NoError(t, err) repo, base := cellBase(t, complete) - removals, err := holdCellToBase(ctx, publicationCellOf(t, workspace, env), env, repo, "main", base, nil) + claim := advancedClaim(workspace) + removals, explained, err := holdAgainst(t, ctx, publicationCellOf(t, workspace, env), env, repo, base, nil, claim) require.NoError(t, err, "a full render removes nothing") require.Empty(t, removals) + require.Empty(t, explained) // The cell file a render of shop alone wrote before it refused to: billing // is gone, and so is billing/ledger as a caller of shop's grpc endpoint. @@ -73,25 +96,191 @@ func TestPublishRefusesACellFileThatRemovesWhatTheBaseAdmits(t *testing.T) { require.NoError(t, os.WriteFile(full.Path, dropped, 0o600)) dropping := publicationCellOf(t, workspace, env) - _, err = holdCellToBase(ctx, dropping, env, repo, "main", base, nil) + _, _, err = holdAgainst(t, ctx, dropping, env, repo, base, nil, claim) require.Error(t, err) require.Contains(t, err.Error(), "namespace acme-billing (module billing, 2 workload(s))") require.Contains(t, err.Error(), "consumer billing/ledger of acme-shop/api/grpc") require.Contains(t, err.Error(), "--allow-cell-removal (billing)") - _, err = holdCellToBase(ctx, dropping, env, repo, "main", base, []string{"shop"}) + _, _, err = holdAgainst(t, ctx, dropping, env, repo, base, []string{"shop"}, claim) require.Error(t, err, "naming the endpoint's module covers its consumer edge, but not billing's namespace") require.NotContains(t, err.Error(), "consumer billing/ledger") require.Contains(t, err.Error(), "namespace acme-billing") - removals, err = holdCellToBase(ctx, dropping, env, repo, "main", base, []string{"billing"}) + removals, explained, err = holdAgainst(t, ctx, dropping, env, repo, base, []string{"billing"}, claim) require.NoError(t, err) require.Equal(t, []string{"namespace acme-billing (module billing, 2 workload(s))", "consumer billing/ledger of acme-shop/api/grpc"}, removals) + require.Empty(t, explained, "billing is still composed: its removal is the flag's to confirm") empty, _ := cellBase(t, nil) require.NoError(t, os.Remove(filepath.Join(empty, filepath.FromSlash(deliveryCellPath("", "staging"))))) runGit(t, empty, "commit", "-am", "no cell file") emptyBase := runGit(t, empty, "rev-parse", "HEAD") - removals, err = holdCellToBase(ctx, dropping, env, empty, "main", emptyBase, nil) + removals, _, err = holdAgainst(t, ctx, dropping, env, empty, emptyBase, nil, claim) require.NoError(t, err, "a base with no cell file has nothing to hold the workspace's to") require.Empty(t, removals) } + +// A removal the composition explains needs no naming. Once legacy leaves +// `modules:`, a render of the composition derives a cell file without it — and +// prunes its record — and a publication of that file passes the hold with no +// flag: the namespace and legacy's consumer edge on shop's endpoint are +// reported as removals the composition explains, for the reviewer, rather than +// refused until someone names a module the workspace no longer composes. +func TestARemovalTheCompositionExplainsNeedsNoNaming(t *testing.T) { + ctx := context.Background() + workspace := writeCellWorkspace(t) + workspace = composeLegacyModule(t, workspace) + env := selectedEnvironment(t, workspace, "staging") + renderCellTree(t, workspace, "shop", "api", "acme-shop", "staging") + renderCellTree(t, workspace, "billing", "ledger", "acme-billing", "staging") + renderCellTree(t, workspace, "legacy", "worker", "acme-legacy", "staging") + full, err := RenderCell(ctx, workspace, env) + require.NoError(t, err) + require.Equal(t, []string{"billing", "legacy", "shop"}, full.Modules) + complete, err := os.ReadFile(full.Path) + require.NoError(t, err) + repo, base := cellBase(t, complete) + + // legacy leaves the composition; its module directory and its render + // record are still in the checkout, as they are after a `modules:` edit. + workspace = editWorkspaceConfiguration(t, workspace, " - name: legacy\n", "") + env = selectedEnvironment(t, workspace, "staging") + + rendered, err := RenderCell(ctx, workspace, env) + require.NoError(t, err) + require.Equal(t, []string{"billing", "shop"}, rendered.Modules) + require.Equal(t, []string{"deployments/modules/legacy"}, rendered.Removed) + require.NoDirExists(t, moduleRenderDestination(workspace, "legacy")) + + named, explained, err := holdAgainst(t, ctx, publicationCellOf(t, workspace, env), env, repo, base, nil, advancedClaim(workspace, "legacy")) + require.NoError(t, err, "a removal the composition explains needs no --allow-cell-removal") + require.Empty(t, named) + require.Equal(t, []string{ + "namespace acme-legacy (module legacy, 2 workload(s)): the composition no longer declares module legacy", + "consumer legacy/worker of acme-shop/api/grpc: the composition no longer declares module legacy", + }, explained) + + // The same cell file from a checkout that has not seen the base's + // composition derives the same absence, and the composition explains + // nothing: the removal is the flag's to confirm, and the refusal says why. + stale := &compositionClaim{declared: composedModules(workspace), revision: strings.Repeat("c", 40)} + _, _, err = holdAgainst(t, ctx, publicationCellOf(t, workspace, env), env, repo, base, nil, stale) + require.Error(t, err) + require.Contains(t, err.Error(), "--allow-cell-removal (legacy)") + require.Contains(t, err.Error(), "does not contain "+strings.Repeat("c", 40)) + require.Contains(t, err.Error(), "may be a stale checkout's") + _, _, err = holdAgainst(t, ctx, publicationCellOf(t, workspace, env), env, repo, base, nil, &compositionClaim{declared: composedModules(workspace)}) + require.Error(t, err) + require.Contains(t, err.Error(), "names no revision it was rendered from") + // A checkout past the revision whose composition never named legacy + // either: the render that named it ran on an uncommitted edit. + _, _, err = holdAgainst(t, ctx, publicationCellOf(t, workspace, env), env, repo, base, nil, advancedClaim(workspace)) + require.Error(t, err) + require.Contains(t, err.Error(), "--allow-cell-removal (legacy)") + require.Contains(t, err.Error(), "neither did its own configuration at "+strings.Repeat("b", 40)) + named, explained, err = holdAgainst(t, ctx, publicationCellOf(t, workspace, env), env, repo, base, []string{"legacy"}, stale) + require.NoError(t, err, "naming the module confirms the removal as before") + require.Len(t, named, 2) + require.Empty(t, explained) +} + +// The composition's claim is advanced only from a checkout that has seen the +// composition the base's cell file was rendered from. The workspace +// repository holding the delivery base commit is the exact test (the file's +// render was committed there); otherwise the revision the base file names is +// what the checkout must descend from; a base naming none advances nothing. +func TestTheCompositionClaimIsAdvancedOnlyFromACheckoutThatSawTheBase(t *testing.T) { + ctx := context.Background() + workspace := composeLegacyModule(t, writeCellWorkspace(t)) + dir := workspace.Dir() + runGit(t, dir, "init", "--initial-branch=main") + runGit(t, dir, "add", "-A") + runGit(t, dir, "commit", "-q", "-m", "the composition that named legacy") + first := runGit(t, dir, "rev-parse", "HEAD") + // legacy leaves the composition, uncommitted: HEAD is the composition + // that named it, the working tree the one that does not. + workspace = editWorkspaceConfiguration(t, workspace, " - name: legacy\n", "") + elsewhere := strings.Repeat("d", 40) + // A delivery repository of its own, holding none of the workspace's history. + separate := t.TempDir() + runGit(t, separate, "init", "--initial-branch=main") + runGit(t, separate, "commit", "-q", "--allow-empty", "-m", "delivery") + + base := &cellfile.File{Source: &cellfile.Source{Revision: first}} + claim := newCompositionClaim(ctx, workspace, separate, "shop", elsewhere, base) + require.True(t, claim.advanced, "a checkout at the revision the base was rendered from has seen its composition") + require.Equal(t, first, claim.revision) + require.True(t, claim.explains("legacy")) + require.False(t, claim.explains("shop"), "a composed module is never explained away") + require.False(t, claim.explains("billing")) + require.False(t, claim.explains("compute"), "a module the judged composition never named is not vouched for") + require.Equal(t, map[string]bool{"shop": true, "billing": true, "legacy": true}, claim.declaredAt) + require.False(t, newCompositionClaim(ctx, workspace, separate, "shop", elsewhere, nil).advanced, "a base with no cell file advances nothing") + require.False(t, newCompositionClaim(ctx, workspace, separate, "shop", elsewhere, &cellfile.File{}).advanced, "a base naming no source advances nothing") + option := newCompositionClaim(ctx, workspace, separate, "shop", elsewhere, &cellfile.File{Source: &cellfile.Source{Revision: "--output=/tmp/x"}}) + require.False(t, option.advanced, "a source revision that is not a commit hash never reaches git") + require.Empty(t, option.revision) + + // The workspace is the delivery repository: the base commit is the one + // that committed the render, a child of the revision the file names. A + // checkout at that revision has NOT seen it. + require.NoError(t, os.WriteFile(filepath.Join(dir, "rendered.txt"), []byte("the render, and the modules: edit that rode with it\n"), 0o644)) + runGit(t, dir, "add", "-A") + runGit(t, dir, "commit", "-q", "-m", "render") + second := runGit(t, dir, "rev-parse", "HEAD") + runGit(t, dir, "checkout", "-q", "--detach", first) + claim = newCompositionClaim(ctx, workspace, separate, "shop", second, base) + require.False(t, claim.advanced, "the checkout does not contain the delivery base commit the workspace repository holds") + require.Equal(t, second, claim.revision, "the base commit is judged, not the parent the file names") + runGit(t, dir, "checkout", "-q", "main") + require.True(t, newCompositionClaim(ctx, workspace, separate, "shop", second, base).advanced) + + // The same one repository, from a checkout that has not fetched the base + // commit: the delivery clone holds the workspace revision the file names, + // which is what tells one repository from two, so the base commit is still + // the one to contain, and the checkout at the render's parent lacks it. + clone := t.TempDir() + runGit(t, "", "clone", "-q", dir, clone) + runGit(t, dir, "checkout", "-q", "--detach", first) + unfetched := strings.Repeat("f", 40) + claim = newCompositionClaim(ctx, workspace, clone, "shop", unfetched, base) + require.False(t, claim.advanced, "an unfetched checkout of the delivery repository is at the render's parent, not past it") + require.Equal(t, unfetched, claim.revision) + runGit(t, dir, "checkout", "-q", "main") + + // A checkout whose history does not contain the named revision at all. + runGit(t, dir, "checkout", "-q", "--orphan", "stale") + runGit(t, dir, "commit", "-q", "--allow-empty", "-m", "a branch that never saw it") + require.False(t, newCompositionClaim(ctx, workspace, separate, "shop", elsewhere, base).advanced) +} + +// composeLegacyModule adds a third module, legacy, to the cell fixture: one +// service, worker, calling shop's api, so dropping the module later removes a +// namespace and a consumer edge at once. +func composeLegacyModule(t *testing.T, workspace *resources.Workspace) *resources.Workspace { + t.Helper() + files := map[string]string{ + filepath.Join("modules", "legacy", resources.ModuleConfigurationName): "kind: module\nname: legacy\nservices:\n - name: worker\n", + filepath.Join("modules", "legacy", "services", "worker", resources.ServiceConfigurationName): cellServiceYAML("worker") + "service-dependencies:\n - name: api\n module: shop\n endpoints:\n - name: grpc\n", + } + for rel, content := range files { + full := filepath.Join(workspace.Dir(), rel) + require.NoError(t, os.MkdirAll(filepath.Dir(full), 0o755)) + require.NoError(t, os.WriteFile(full, []byte(content), 0o644)) + } + return editWorkspaceConfiguration(t, workspace, " - name: billing\n", " - name: billing\n - name: legacy\n") +} + +// editWorkspaceConfiguration replaces one passage of the workspace's +// configuration and loads the workspace again. +func editWorkspaceConfiguration(t *testing.T, workspace *resources.Workspace, old, replacement string) *resources.Workspace { + t.Helper() + path := filepath.Join(workspace.Dir(), resources.WorkspaceConfigurationName) + config, err := os.ReadFile(path) + require.NoError(t, err) + require.Contains(t, string(config), old) + require.NoError(t, os.WriteFile(path, []byte(strings.Replace(string(config), old, replacement, 1)), 0o644)) + edited, err := resources.LoadWorkspaceFromDir(context.Background(), workspace.Dir()) + require.NoError(t, err) + return edited +} diff --git a/pkg/gitops/cell_test.go b/pkg/gitops/cell_test.go index 142e186d..be85ec0a 100644 --- a/pkg/gitops/cell_test.go +++ b/pkg/gitops/cell_test.go @@ -327,6 +327,52 @@ func TestCellFileRefusesToDropAComposedModuleWithNoRender(t *testing.T) { require.Equal(t, []string{"billing", "shop"}, again.Modules) } +// The cell file is derived from the composition, not from whatever records +// the checkout holds. A module removed from `modules:` leaves its render +// record behind (and so does one that was never composed here), and the file +// used to read it back: the module kept its namespace, workloads and consumer +// edges in the cell after the composition dropped it. The render now leaves +// such records out of the file, deletes them — nothing could render them again +// — and says so. +func TestRenderCellPrunesTheRecordOfAModuleTheCompositionNoLongerDeclares(t *testing.T) { + workspace := writeCellWorkspace(t) + env := selectedEnvironment(t, workspace, "staging") + renderCellTree(t, workspace, "shop", "api", "acme-shop", "staging") + renderCellTree(t, workspace, "billing", "ledger", "acme-billing", "staging") + // Records left by modules the composition does not declare: one for this + // environment, one rendered for another. Neither can be rendered again. + renderCellTreeWith(t, &RenderOptions{ + Destination: moduleRenderDestination(workspace, "proposals"), + Module: "proposals", Environment: "staging", Namespace: "acme-proposals", AppProject: "acme-staging", + Promotable: true, OwnedPath: "deployments/modules/proposals", Units: unitsWithPorts("proposals", "web"), + }, "web", "staging") + renderCellTreeWith(t, &RenderOptions{ + Destination: moduleRenderDestination(workspace, "compute"), + Module: "compute", Environment: "production", Namespace: "acme-compute", AppProject: "acme-production", + Promotable: true, OwnedPath: "deployments/modules/compute", Units: unitsWithPorts("compute", "worker"), + }, "worker", "production") + + result, err := RenderCell(context.Background(), workspace, env) + require.NoError(t, err) + require.Equal(t, []string{"billing", "shop"}, result.Modules) + require.Empty(t, result.Skipped, "a record of an undeclared module is pruned, not reported as another environment's") + require.Equal(t, []string{"deployments/modules/compute", "deployments/modules/proposals"}, result.Removed) + require.NoDirExists(t, moduleRenderDestination(workspace, "proposals")) + require.NoDirExists(t, moduleRenderDestination(workspace, "compute")) + require.DirExists(t, moduleRenderDestination(workspace, "shop")) + + data, err := os.ReadFile(result.Path) + require.NoError(t, err) + file, err := cellfile.Parse(data) + require.NoError(t, err) + names := make([]string, 0, len(file.Namespaces)) + for _, namespace := range file.Namespaces { + names = append(names, namespace.Name) + } + require.Equal(t, []string{"acme-billing", "acme-shop"}, names) + require.NotContains(t, string(data), "proposals") +} + // A workload whose authenticating container cannot be told from a sidecar is // refused by name rather than guessed. func TestCellFileRefusesAWorkloadWhoseAuthenticatingContainerCannotBeTold(t *testing.T) { @@ -793,14 +839,17 @@ func TestCellFileRefusesATreeOfAModuleTheWorkspaceDoesNotCompose(t *testing.T) { renderCellTree(t, workspace, "billing", "ledger", "acme-billing", "staging") withdrawn := renderCellTree(t, workspace, "proposals", "api", "acme-proposals", "staging") - _, err := RenderCell(context.Background(), workspace, env) + // The derivation holds the tree; a render satisfies the hold by removing + // it (TestRenderCellPrunesTheRecordOfAModuleTheCompositionNoLongerDeclares). + _, _, err := deriveCell(context.Background(), workspace, env) require.Error(t, err) require.Contains(t, err.Error(), "cell file is not written") require.Contains(t, err.Error(), withdrawn.Path+" holds a render of module proposals for staging, but the workspace composes no module of that name") require.Contains(t, err.Error(), "remove that render and the module's delivered tree") - require.NoError(t, os.RemoveAll(withdrawn.Path)) again, err := RenderCell(context.Background(), workspace, env) require.NoError(t, err) require.Equal(t, []string{"billing", "shop"}, again.Modules) + require.Equal(t, []string{"deployments/modules/proposals"}, again.Removed) + require.NoDirExists(t, withdrawn.Path) } diff --git a/pkg/gitops/publish.go b/pkg/gitops/publish.go index 308f6d99..25a683cc 100644 --- a/pkg/gitops/publish.go +++ b/pkg/gitops/publish.go @@ -220,7 +220,15 @@ func preparePublish( if err != nil { return fail(err) } - if err = refuseUnrelatedPromotionChanges(ctx, repo, baseRevision, promotionBranch, owned, branchRevision, replacesRevision); err != nil { + baseCell, claim, removedModules, err := judgeCompositionRemovals(ctx, workspace, env, cell, &compositionBase{ + repo: repo, gitopsPath: pathRoot, branch: baseBranch, revision: baseRevision, + module: request.Module, restoreRevision: restoreRevision, + }) + if err != nil { + return fail(err) + } + publishedPaths = append(publishedPaths, removedModules...) + if err = refuseUnrelatedPromotionChanges(ctx, repo, baseRevision, promotionBranch, owned, claim.owns(pathRoot), branchRevision, replacesRevision); err != nil { return fail(err) } startRevision := baseRevision @@ -249,7 +257,7 @@ func preparePublish( return fail(err) } staleConsumers := describeStaleConsumers(staleOnBase) - allowedRemovals, err := holdCellToBase(ctx, cell, env, repo, baseBranch, baseRevision, request.AllowCellRemovals) + allowedRemovals, undeclaredRemovals, err := holdCellToBase(cell, env, baseBranch, baseCell, request.AllowCellRemovals, claim) if err != nil { return fail(err) } @@ -273,14 +281,16 @@ func preparePublish( Module: request.Module, Environment: request.Environment, RenderDigest: inventory.Digest, SnapshotRevision: snapshotRevision, Changed: changed, Diff: diff, ContractChecks: contractChecks, - Delivery: inventory.Delivery, - StaleConsumers: staleConsumers, - CellRemovals: allowedRemovals, - CellFile: carriedCell, - CellNotice: cellNotice(env, cell, restoreRevision, request.Module), - Carriers: publication.options.Carriers, - SelfPromotes: selfPromotes, - LocalRepository: bound, + Delivery: inventory.Delivery, + StaleConsumers: staleConsumers, + CellRemovals: allowedRemovals, + UndeclaredCellRemovals: undeclaredRemovals, + RemovedModules: removedModules, + CellFile: carriedCell, + CellNotice: cellNotice(env, cell, restoreRevision, request.Module), + Carriers: publication.options.Carriers, + SelfPromotes: selfPromotes, + LocalRepository: bound, } if bound != nil { if err = bound.verify(ctx, config.RepoURL); err != nil { @@ -565,14 +575,14 @@ func refuseSharedDeliveryPath(workspace *resources.Workspace, environment string // 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 string, published []string, branchRevisions ...string) error { +func refuseUnrelatedPromotionChanges(ctx context.Context, repo, baseRevision, promotionBranch string, published []string, alsoOwns func(string) bool, branchRevisions ...string) error { owned := func(changed string) bool { for _, path := range published { if changed == path || strings.HasPrefix(changed, path+"/") { return true } } - return false + return alsoOwns != nil && alsoOwns(changed) } for _, branchRevision := range branchRevisions { if branchRevision == "" { diff --git a/pkg/gitops/publish_cell_test.go b/pkg/gitops/publish_cell_test.go index 16652c6f..feb64b8c 100644 --- a/pkg/gitops/publish_cell_test.go +++ b/pkg/gitops/publish_cell_test.go @@ -19,22 +19,315 @@ import ( // repository, so a publication has a base branch's approved-build list to be // held to. func seedCellFile(t *testing.T, remote, gitopsPath, environment string, data []byte) { + t.Helper() + seedDelivered(t, remote, "seed the cell file", map[string][]byte{deliveryCellPath(gitopsPath, environment): data}) +} + +// seedDelivered commits files directly onto main of the bare delivery +// repository, at the repository-relative paths given. +func seedDelivered(t *testing.T, remote, message string, files map[string][]byte) { t.Helper() 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") - path := filepath.Join(work, filepath.FromSlash(deliveryCellPath(gitopsPath, environment))) - if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil { + for relative, data := range files { + path := filepath.Join(work, filepath.FromSlash(relative)) + if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(path, data, 0o644); err != nil { + t.Fatal(err) + } + } + gitRun(t, work, "add", "-A") + gitRun(t, work, "commit", "-m", message) + gitRun(t, work, "push", "origin", "main") +} + +// committedWorkspace makes the fixture workspace a git checkout whose HEAD +// commits a composition that also declares the modules named (removed), so a +// cell file rendered there names that revision, and a later publication from +// the working tree — which no longer declares them — can be judged against it. +// The fixture is flat, so a declared module is a sibling `modules:` entry. +func committedWorkspace(t *testing.T, workspace *resources.Workspace, removed ...string) string { + t.Helper() + dir := workspace.Dir() + config := filepath.Join(dir, resources.WorkspaceConfigurationName) + current, err := os.ReadFile(config) + if err != nil { t.Fatal(err) } - if err := os.WriteFile(path, data, 0o644); err != nil { + committed := string(current) + if len(removed) > 0 { + committed += "modules:\n" + for _, module := range removed { + committed += " - name: " + module + "\n" + } + } + if err = os.WriteFile(config, []byte(committed), 0o644); err != nil { t.Fatal(err) } - gitRun(t, work, "add", "-A") - gitRun(t, work, "commit", "-m", "seed the cell file") - gitRun(t, work, "push", "origin", "main") + gitRun(t, dir, "init", "-q", "--initial-branch=main") + gitRun(t, dir, "config", "user.name", "Codefly Test") + gitRun(t, dir, "config", "user.email", "codefly@example.com") + gitRun(t, dir, "config", "commit.gpgsign", "false") + gitRun(t, dir, "add", "-A") + gitRun(t, dir, "commit", "-q", "-m", "the composition") + if err = os.WriteFile(config, current, 0o644); err != nil { + t.Fatal(err) + } + return gitOutput(t, dir, "rev-parse", "HEAD") +} + +// renderedFrom is the cell file data as the base carries it after a render +// from revision: the base's provenance, stated as the render that produced it +// states it (cellSource), whatever this fixture's own render recorded. +func renderedFrom(t *testing.T, data []byte, revision string) []byte { + t.Helper() + file, err := cellfile.Parse(data) + if err != nil { + t.Fatal(err) + } + file.Source = &cellfile.Source{Repo: "https://example.test/delivery.git", Revision: revision, Branch: "main"} + encoded, err := file.Encode() + if err != nil { + t.Fatal(err) + } + return encoded +} + +// legacyDelivery is a delivered tree of a module the fixture's composition +// does not declare, plus a directory under the modules path that is not a +// delivery, as files to seed on the delivery base. +func legacyDelivery() (string, map[string][]byte) { + legacy := deliveryModulePath("environments", "legacy") + return legacy, map[string][]byte{ + legacy + "/" + InventoryFilename: []byte(`{"module":"legacy","environment":"production"}`), + legacy + "/services/worker/kustomization.yaml": []byte("resources: []\n"), + deliveryModulesPath("environments") + "/notes/README.md": []byte("not a delivery\n"), + } +} + +// TestPublishRemovesADeliveredModuleTheCompositionNoLongerDeclares: a module +// removed from `modules:` left its delivered tree in the delivery repository, +// and with it the ApplicationSet that kept stamping Applications whose every +// pod the cell — which no longer named the module — refused. A forward +// publication to a hosted environment, from a checkout that has seen the +// base's composition, now removes such a tree with the rest of what it +// writes, shows it in the plan, and leaves alone a directory under the +// modules path that is not a delivery (no render record). +func TestPublishRemovesADeliveredModuleTheCompositionNoLongerDeclares(t *testing.T) { + ctx, workspace, remote := developerPublishWorkspace(t) + head := committedWorkspace(t, workspace, "legacy") + env := selectedEnvironment(t, workspace, "production") + _, data := renderedCell(t, ctx, workspace, env) + legacy, files := legacyDelivery() + files[deliveryCellPath("environments", "production")] = renderedFrom(t, data, head) + seedDelivered(t, remote, "a module since removed from the composition", files) + + request := PublishRequest{ + Module: "payments", Environment: "production", Local: true, Signer: &fakeSigner{nonce: true}, + PromotionBranch: "codefly/promote-payments-production", + } + plan, err := PlanPublish(ctx, workspace, &request) + if err != nil { + t.Fatal(err) + } + if !slices.Equal(plan.RemovedModules, []string{legacy}) { + t.Fatalf("the plan removes %v, want %v", plan.RemovedModules, []string{legacy}) + } + for _, path := range []string{legacy + "/" + InventoryFilename, legacy + "/services/worker/kustomization.yaml"} { + if !slices.Contains(plan.Changed, path) { + t.Fatalf("the plan's changed files %v do not carry the removal of %s", plan.Changed, path) + } + } + if slices.ContainsFunc(plan.Changed, func(path string) bool { return strings.Contains(path, "/notes/") }) { + t.Fatalf("a directory that is not a delivery was removed: %v", plan.Changed) + } + + result, err := Publish(ctx, workspace, &PublishMutation{Request: request, PlanID: plan.ID, Carriers: plan.Carriers}, preparedPermit) + if err != nil { + t.Fatal(err) + } + if exec.Command("git", "--git-dir", remote, "cat-file", "-e", result.Commit+":"+legacy+"/"+InventoryFilename).Run() == nil { + t.Fatalf("the published commit still delivers %s", legacy) + } + if exec.Command("git", "--git-dir", remote, "cat-file", "-e", result.Commit+":"+deliveryModulesPath("environments")+"/notes/README.md").Run() != nil { + t.Fatal("the published commit removed a directory that is not a delivery") + } +} + +// TestAPromotionOwnsTheRemovalItsOwnBranchCarries: the branch guard holds a +// promotion branch to what this module's publication writes, and a removed +// tree is no longer in the clone to be listed — the branch's earlier commit +// deleted it — so a second publish on the module's own branch (its promotion +// still open, or a retry) must own the removal by rule rather than by what it +// finds on disk, or it refuses its own earlier work as someone else's. +func TestAPromotionOwnsTheRemovalItsOwnBranchCarries(t *testing.T) { + ctx, workspace, remote := developerPublishWorkspace(t) + head := committedWorkspace(t, workspace, "legacy") + env := selectedEnvironment(t, workspace, "production") + _, data := renderedCell(t, ctx, workspace, env) + legacy, files := legacyDelivery() + files[deliveryCellPath("environments", "production")] = renderedFrom(t, data, head) + seedDelivered(t, remote, "a module since removed from the composition", files) + + // The module's earlier promotion: the cell file carried, legacy removed. + work := t.TempDir() + gitRun(t, "", "clone", "-q", 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", "-q", "-b", "codefly/promote-payments-production") + gitRun(t, work, "rm", "-r", "-q", legacy) + gitRun(t, work, "commit", "-q", "-m", "an earlier promotion of payments, removing legacy") + gitRun(t, work, "push", "-q", "origin", "codefly/promote-payments-production") + + plan, err := PlanPublish(ctx, workspace, &PublishRequest{ + Module: "payments", Environment: "production", Local: true, Signer: &fakeSigner{nonce: true}, + PromotionBranch: "codefly/promote-payments-production", + }) + if err != nil { + t.Fatalf("a promotion branch carrying the module's own removal was refused: %v", err) + } + if len(plan.RemovedModules) != 0 { + t.Fatalf("the continued branch already removed %s; the plan removes %v again", legacy, plan.RemovedModules) + } +} + +// TestAStaleCheckoutStillNeedsTheFlagToWithdrawAModule: a checkout that never +// had a module and one that dropped it derive the same cell file. Only a +// checkout that has seen the composition the base's file was rendered from +// may remove on the composition's word; from any other the removal is still +// the flag's to confirm, and even named, the delivered tree is not pruned. +func TestAStaleCheckoutStillNeedsTheFlagToWithdrawAModule(t *testing.T) { + ctx, workspace, remote := developerPublishWorkspace(t) + committedWorkspace(t, workspace) + env := selectedEnvironment(t, workspace, "production") + _, data := renderedCell(t, ctx, workspace, env) + // The base's file: rendered from a composition this checkout never saw, + // which named a module legacy this checkout does not declare. + file, err := cellfile.Parse(renderedFrom(t, data, strings.Repeat("e", 40))) + if err != nil { + t.Fatal(err) + } + file.Namespaces = append(file.Namespaces, cellfile.Namespace{Name: "legacy", Module: "legacy", Workloads: []cellfile.Workload{{Name: "worker", Kind: "Deployment"}}}) + base, err := file.Encode() + if err != nil { + t.Fatal(err) + } + legacy, files := legacyDelivery() + files[deliveryCellPath("environments", "production")] = base + seedDelivered(t, remote, "a composition this checkout never saw", files) + + request := PublishRequest{ + Module: "payments", Environment: "production", Local: true, Signer: &fakeSigner{nonce: true}, + PromotionBranch: "codefly/promote-payments-production", + } + _, err = PlanPublish(ctx, workspace, &request) + if err == nil || !strings.Contains(err.Error(), "--allow-cell-removal (legacy)") || !strings.Contains(err.Error(), "may be a stale checkout's") { + t.Fatalf("a stale checkout withdrew a module on the composition's word: %v", err) + } + request.AllowCellRemovals = []string{"legacy"} + plan, err := PlanPublish(ctx, workspace, &request) + if err != nil { + t.Fatal(err) + } + if len(plan.RemovedModules) != 0 || slices.ContainsFunc(plan.Changed, func(path string) bool { return strings.HasPrefix(path, legacy) }) { + t.Fatalf("a removal the flag confirms pruned the delivered tree: %v %v", plan.RemovedModules, plan.Changed) + } + if len(plan.CellRemovals) != 1 || len(plan.UndeclaredCellRemovals) != 0 { + t.Fatalf("the plan reports %v named and %v explained; want one named", plan.CellRemovals, plan.UndeclaredCellRemovals) + } +} + +// TestACompositionThatNeverDeclaredTheModuleDoesNotRemoveIt: with a separate +// delivery repository the base's cell file names the render's parent, and a +// render may run on an uncommitted `modules:` edit — so a checkout at that +// parent descends from it without ever having composed the module the render +// named. Nothing in its history vouches the module was removed rather than +// never composed, and the removal stays the flag's to confirm. +func TestACompositionThatNeverDeclaredTheModuleDoesNotRemoveIt(t *testing.T) { + ctx, workspace, remote := developerPublishWorkspace(t) + head := committedWorkspace(t, workspace) + env := selectedEnvironment(t, workspace, "production") + _, data := renderedCell(t, ctx, workspace, env) + file, err := cellfile.Parse(renderedFrom(t, data, head)) + if err != nil { + t.Fatal(err) + } + file.Namespaces = append(file.Namespaces, cellfile.Namespace{Name: "legacy", Module: "legacy", Workloads: []cellfile.Workload{{Name: "worker", Kind: "Deployment"}}}) + base, err := file.Encode() + if err != nil { + t.Fatal(err) + } + legacy, files := legacyDelivery() + files[deliveryCellPath("environments", "production")] = base + seedDelivered(t, remote, "a render on an uncommitted composition that named legacy", files) + + request := PublishRequest{ + Module: "payments", Environment: "production", Local: true, Signer: &fakeSigner{nonce: true}, + PromotionBranch: "codefly/promote-payments-production", + } + _, err = PlanPublish(ctx, workspace, &request) + if err == nil || !strings.Contains(err.Error(), "--allow-cell-removal (legacy)") || !strings.Contains(err.Error(), "neither did its own configuration at "+head) { + t.Fatalf("a checkout whose composition never named legacy withdrew it on the composition's word: %v", err) + } + request.AllowCellRemovals = []string{"legacy"} + plan, err := PlanPublish(ctx, workspace, &request) + if err != nil { + t.Fatal(err) + } + if len(plan.RemovedModules) != 0 || slices.ContainsFunc(plan.Changed, func(path string) bool { return strings.HasPrefix(path, legacy) }) { + t.Fatalf("a removal the flag confirms pruned the delivered tree: %v %v", plan.RemovedModules, plan.Changed) + } +} + +// TestAnUnfetchedCheckoutOfTheDeliveryRepositoryStillNeedsTheFlag: when the +// workspace repository is the delivery repository, the base's cell file names +// the render's parent as its source, and a checkout at that parent that has +// not fetched the base commit descends from it — yet never saw the `modules:` +// edit that rode in the render's commit. The delivery clone holding that +// workspace revision is what tells one repository from two, so the base +// commit stays the one to contain, and the removal is the flag's to confirm. +func TestAnUnfetchedCheckoutOfTheDeliveryRepositoryStillNeedsTheFlag(t *testing.T) { + ctx, workspace, remote := developerPublishWorkspace(t) + head := committedWorkspace(t, workspace, "legacy") + // One repository: the delivery remote holds the workspace's history. + gitRun(t, workspace.Dir(), "push", "-q", remote, "HEAD:refs/heads/workspace-history") + env := selectedEnvironment(t, workspace, "production") + _, data := renderedCell(t, ctx, workspace, env) + file, err := cellfile.Parse(renderedFrom(t, data, head)) + if err != nil { + t.Fatal(err) + } + file.Namespaces = append(file.Namespaces, cellfile.Namespace{Name: "legacy", Module: "legacy", Workloads: []cellfile.Workload{{Name: "worker", Kind: "Deployment"}}}) + base, err := file.Encode() + if err != nil { + t.Fatal(err) + } + legacy, files := legacyDelivery() + files[deliveryCellPath("environments", "production")] = base + seedDelivered(t, remote, "the render that named legacy, committed past this checkout", files) + + request := PublishRequest{ + Module: "payments", Environment: "production", Local: true, Signer: &fakeSigner{nonce: true}, + PromotionBranch: "codefly/promote-payments-production", + } + _, err = PlanPublish(ctx, workspace, &request) + if err == nil || !strings.Contains(err.Error(), "--allow-cell-removal (legacy)") || !strings.Contains(err.Error(), "may be a stale checkout's") { + t.Fatalf("an unfetched checkout of the delivery repository withdrew a module on the composition's word: %v", err) + } + request.AllowCellRemovals = []string{"legacy"} + plan, err := PlanPublish(ctx, workspace, &request) + if err != nil { + t.Fatal(err) + } + if len(plan.RemovedModules) != 0 || slices.ContainsFunc(plan.Changed, func(path string) bool { return strings.HasPrefix(path, legacy) }) { + t.Fatalf("a removal the flag confirms pruned the delivered tree: %v %v", plan.RemovedModules, plan.Changed) + } } // renderedCell renders the environment's cell file from the workspace's module diff --git a/pkg/gitops/types.go b/pkg/gitops/types.go index 26c38389..b314961f 100644 --- a/pkg/gitops/types.go +++ b/pkg/gitops/types.go @@ -478,7 +478,9 @@ type PublishRequest struct { // AllowCellRemovals names the modules whose removal from the hosted // environment's cell file this publish intends: a namespace, workload, // endpoint or consumer the delivery base branch's cell file carries and - // the workspace's does not. Any other removal refuses the publish. + // the workspace's does not. A removal of a module the composition no + // longer declares needs no naming; any other unnamed removal refuses the + // publish. AllowCellRemovals []string `json:"allowCellRemovals,omitempty"` } @@ -517,6 +519,14 @@ type PublishPlan struct { // CellRemovals are the removals from the base branch's cell file the // request named (AllowCellRemovals), for the reviewer to see. CellRemovals []string `json:"cellRemovals,omitempty"` + // UndeclaredCellRemovals are the removals from the base branch's cell + // file the composition explains — their module is no longer declared — + // which no request needs to name. + UndeclaredCellRemovals []string `json:"undeclaredCellRemovals,omitempty"` + // RemovedModules are the delivered module trees this publication removes, + // relative to the delivery repository: modules the composition no longer + // declares, which the cell file it carries no longer admits. + RemovedModules []string `json:"removedModules,omitempty"` // CellFile is the environment's cell file this publication carries into // the delivery repository, at the path the cell's infrastructure reads it // from. A cell admits only the builds that file names, so a promotion of