Skip to content

Commit 4d077bf

Browse files
fix(gitops): the cell file never silently drops a composed module's workloads (#1150)
The cell file is regenerated whole from the per-module render records (deployments/modules/<m>/.codefly-render.json), which are uncommitted render outputs. After a clean of deployments/, rendering saas and robin wrote a cell file with 4 insertions and 1026 deletions: 25 workloads of 9 other modules gone, and nothing warned. - A render refuses to write the cell file while a module the workspace composes has no render record for the environment (missing, or rendered for another environment), naming each module and the command that renders it. It does not carry the module's entries over from the file on disk: an entry is derived from the whole composition, so a carried-over entry is a claim about a composition that may no longer exist. - plan/publish hold the workspace's cell file to the delivery base branch's and refuse one that removes a namespace, workload, endpoint or endpoint consumer the base admits, unless --allow-cell-removal names the module the removal concerns; the plan lists every removal it lets through. Co-authored-by: Antoine Toussaint <312536060+antoinetoussaint-byte@users.noreply.github.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
1 parent a36ceed commit 4d077bf

9 files changed

Lines changed: 368 additions & 7 deletions

File tree

‎cmd/deploy/gitops.go‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -486,6 +486,7 @@ func publishRequest(module string) gitops.PublishRequest {
486486
Title: gitOpsTitle, Body: gitOpsBody, Local: gitOpsLocal,
487487
AllowUnresolvedContracts: gitOpsAllowUnresolvedContracts,
488488
Resign: gitOpsResign,
489+
AllowCellRemovals: gitOpsAllowCellRemovals,
489490
}
490491
}
491492

@@ -580,6 +581,9 @@ func printPublishPlan(plan *gitops.PublishPlan) {
580581
cli.Info("Contract checks:")
581582
cli.Info("%s", formatContractChecks(plan.ContractChecks))
582583
}
584+
for _, removal := range plan.CellRemovals {
585+
cli.Warning("The cell file removes %s, as --allow-cell-removal names", removal)
586+
}
583587
for _, stale := range plan.StaleConsumers {
584588
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)
585589
}
@@ -603,6 +607,7 @@ var (
603607
gitOpsLocal bool
604608
gitOpsAllowUnresolvedContracts bool
605609
gitOpsResign bool
610+
gitOpsAllowCellRemovals []string
606611
gitOpsValidateCluster bool
607612
gitOpsRebuild bool
608613
)
@@ -640,6 +645,10 @@ func init() {
640645
command.Flags().BoolVar(&gitOpsResign, "resign", false,
641646
"Sign again a delivered document whose carrier the release policy no longer admits (a rotated signing identity); deliberate, never implied")
642647
}
648+
for _, command := range []*cobra.Command{gitOpsPlanCmd, gitOpsPublishCmd} {
649+
command.Flags().StringSliceVar(&gitOpsAllowCellRemovals, "allow-cell-removal", nil,
650+
"Module whose removal from the environment's cell file, relative to the base branch's, is intended (repeatable); any other removal refuses")
651+
}
643652
for _, command := range []*cobra.Command{gitOpsPublishCmd, gitOpsRollbackCmd} {
644653
command.Flags().StringVar(&gitOpsMessage, "message", "", "Signed commit message")
645654
command.Flags().StringVar(&gitOpsTitle, "title", "", "Promotion pull request title")

‎docs/cli-reference.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2090,6 +2090,7 @@ codefly deploy gitops plan [module]
20902090
Flags:
20912091

20922092
```
2093+
--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
20932094
--allow-unresolved-contracts Downgrade a consumed contract whose exposing module is not yet deployed to a warning, for bootstrap ordering
20942095
--env string Environment to promote (default "local")
20952096
--local Use a disposable local file Git remote for k3d qualification
@@ -2109,6 +2110,7 @@ codefly deploy gitops publish [module]
21092110
Flags:
21102111

21112112
```
2113+
--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
21122114
--allow-unresolved-contracts Downgrade a consumed contract whose exposing module is not yet deployed to a warning, for bootstrap ordering
21132115
--body string Promotion pull request body
21142116
--env string Environment to promote (default "local")

‎docs/commands.md‎

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -888,6 +888,34 @@ and verifier allows from, instead of transcribing them. The consumer validates
888888
it as another repository's output; this side writes it deterministically and
889889
reads it back through its own reader.
890890

891+
Because absence is removal, **the cell file is never written without a module
892+
the workspace composes.** The per-module render records
893+
(`deployments/modules/<module>/.codefly-render.json`) are the file's only
894+
source, and they are uncommitted render outputs: a `git clean` of
895+
`deployments/` removes them. A render that finds a composed module with no
896+
record for the environment — none at all, or one rendered for another
897+
environment — writes no cell file, leaves the one on disk as it was, and names
898+
each such module with the command that renders it (`codefly deploy gitops
899+
render <module> --env <env>`, or `render --all`). It does not keep that
900+
module's entries from the file already on disk: an entry is derived from the
901+
whole composition (its consumers are other modules' dependencies, its bindings
902+
and egress the environment's), so an entry carried over is a claim about a
903+
composition that may no longer exist, and no digest is invented for a module
904+
that was not rendered.
905+
906+
**A publish holds the cell file to the base branch's.** The cell file is
907+
committed with the rendered trees, and a cell admits only what it names, so
908+
`plan` and `publish` read the workspace's cell file for a hosted environment
909+
and the one the delivery base branch carries at
910+
`<gitops path>/cells/<environment>/cell.yaml`, and refuse when the workspace's
911+
removes a namespace, a workload, an endpoint or a consumer of an endpoint the
912+
base admits — naming each removal and the modules it concerns. A removal that
913+
is intended (a module the workspace no longer composes, a dependency dropped)
914+
is named with `--allow-cell-removal <module>` (repeatable); a consumer edge is
915+
covered by naming either the endpoint's module or the consumer's, and the plan
916+
lists every removal it lets through. A base with no cell file has nothing to
917+
hold the workspace's to.
918+
891919
For a complete module promotion, use the GitOps lifecycle:
892920

893921
```bash
@@ -912,7 +940,9 @@ one command renders one composition and a stale sibling tree is never a reason
912940
to run the render again. A single-module `render` re-derives the cell file
913941
from its own tree and the sibling trees already rendered; a sibling the cell
914942
file cannot read, or one left at another inventory schema, is reported as
915-
stale by name rather than failing the render. An image whose inputs did not
943+
stale by name rather than failing the render, and so is a composed module with
944+
no render at all: the module's tree is written, the cell file is not, and the
945+
warning names the renders still owed. An image whose inputs did not
916946
change is kept rather than rebuilt, and that decision is taken before any Go
917947
module graph is fetched, so a render of an up-to-date composition fetches
918948
nothing; `--rebuild` builds every image regardless. A service that declares

‎pkg/gitops/cell.go‎

Lines changed: 41 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,10 @@ import (
3434
// environment, in the shape the consumer reads (pkg/gitops/cellfile).
3535
//
3636
// It is regenerated whole after every module render, so absence is correct: a
37-
// workload not in the file is not delivered. It cannot live in a module's owned
37+
// workload not in the file is not delivered. That is why it is never written
38+
// while a module the workspace composes has no render for the environment
39+
// (refuseUnrenderedModules), and why a publication holds it to the delivery
40+
// base branch's file (holdCellToBase). It cannot live in a module's owned
3841
// tree, where the promotable ruleset admits only Kubernetes manifests and
3942
// Kustomizations; it lives beside the module trees, under
4043
// <gitops path>/cells/<environment>/cell.yaml, and is committed with them.
@@ -159,6 +162,9 @@ func deriveCell(ctx context.Context, workspace *resources.Workspace, env *enviro
159162
if len(file.Namespaces) == 0 {
160163
return cellfile.File{}, CellResult{}, fmt.Errorf("no module is rendered for environment %s under %s", env.Name, modulesDir)
161164
}
165+
if err = refuseUnrenderedModules(workspace, env, result.Modules); err != nil {
166+
return cellfile.File{}, CellResult{}, err
167+
}
162168
sort.Slice(file.Namespaces, func(i, j int) bool { return file.Namespaces[i].Name < file.Namespaces[j].Name })
163169
sort.Strings(result.Modules)
164170
sort.Strings(result.Skipped)
@@ -168,6 +174,40 @@ func deriveCell(ctx context.Context, workspace *resources.Workspace, env *enviro
168174
return file, result, nil
169175
}
170176

177+
// refuseUnrenderedModules refuses a cell file that would leave out a module
178+
// the workspace composes because no render of it for this environment is in
179+
// the checkout: the record is missing (never rendered, or cleaned away with
180+
// the rest of the uncommitted render outputs) or describes another
181+
// environment. The file is regenerated whole and absence is removal, so such
182+
// a file would withdraw every workload that module delivers from the cell's
183+
// admission set, and nothing would say so.
184+
//
185+
// It refuses rather than keeping the module's entries from the cell file
186+
// already on disk: an entry is derived from the composition as a whole — its
187+
// consumers are other modules' declared dependencies, its bindings and egress
188+
// the environment's — so an entry carried over from an earlier render is a
189+
// claim about a composition that may no longer exist, and the file it was
190+
// read from is as likely to have been cleaned as the records were. No digest
191+
// is invented either: the render of the module is what knows it.
192+
func refuseUnrenderedModules(workspace *resources.Workspace, env *environments.Environment, rendered []string) error {
193+
var missing []string
194+
for _, reference := range workspace.Modules {
195+
if reference != nil && !slices.Contains(rendered, reference.Name) {
196+
missing = append(missing, reference.Name)
197+
}
198+
}
199+
if len(missing) == 0 {
200+
return nil
201+
}
202+
sort.Strings(missing)
203+
commands := make([]string, 0, len(missing))
204+
for _, module := range missing {
205+
commands = append(commands, fmt.Sprintf("codefly deploy gitops render %s --env %s", module, env.Name))
206+
}
207+
return fmt.Errorf("environment %s's cell file is not written: this checkout holds no render for %s of the composed module(s) %s, and a cell file without them would withdraw every workload they deliver from the cell; render them (%s), or every module at once with `codefly deploy gitops render --all --env %s`",
208+
env.Name, env.Name, strings.Join(missing, ", "), strings.Join(commands, "; "), env.Name)
209+
}
210+
171211
// writeCell encodes the cell file and writes it at path.
172212
func writeCell(file *cellfile.File, path string) error {
173213
body, err := file.Encode()

‎pkg/gitops/cell_removals.go‎

Lines changed: 150 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,150 @@
1+
package gitops
2+
3+
// A render is a claim about a cell: the cell file is regenerated whole and a
4+
// cell admits only what it names, so a workload, a namespace or a consumer
5+
// absent from it is withdrawn. A publication promotes a render, so before it
6+
// does, the workspace's cell file is held to the one the delivery base branch
7+
// carries, and a removal it does not name is refused: a cell file derived
8+
// from a checkout missing some modules' renders would otherwise remove those
9+
// modules from admission without anyone having said so.
10+
11+
import (
12+
"context"
13+
"errors"
14+
"fmt"
15+
"io/fs"
16+
"os"
17+
"path/filepath"
18+
"slices"
19+
"strings"
20+
21+
"github.com/codefly-dev/cli/pkg/environments"
22+
"github.com/codefly-dev/cli/pkg/gitops/cellfile"
23+
"github.com/codefly-dev/core/resources"
24+
)
25+
26+
// cellRemoval is one thing the base cell file admits that the next one does
27+
// not, and the modules it concerns: the namespace's module, and for a
28+
// consumer, the consumer's module too.
29+
type cellRemoval struct {
30+
modules []string
31+
what string
32+
}
33+
34+
// cellRemovals names every namespace, workload, endpoint and endpoint
35+
// consumer base carries that next does not, in base's order.
36+
func cellRemovals(base, next *cellfile.File) []cellRemoval {
37+
var removals []cellRemoval
38+
for n := range base.Namespaces {
39+
was := &base.Namespaces[n]
40+
index := slices.IndexFunc(next.Namespaces, func(candidate cellfile.Namespace) bool { return candidate.Name == was.Name })
41+
if index < 0 {
42+
removals = append(removals, cellRemoval{modules: []string{was.Module}, what: fmt.Sprintf("namespace %s (module %s, %d workload(s))", was.Name, was.Module, len(was.Workloads))})
43+
continue
44+
}
45+
is := &next.Namespaces[index]
46+
for w := range was.Workloads {
47+
workload := &was.Workloads[w]
48+
found := slices.IndexFunc(is.Workloads, func(candidate cellfile.Workload) bool {
49+
return candidate.Name == workload.Name && candidate.Kind == workload.Kind
50+
})
51+
if found < 0 {
52+
removals = append(removals, cellRemoval{modules: []string{was.Module}, what: fmt.Sprintf("workload %s/%s %s", was.Name, workload.Kind, workload.Name)})
53+
continue
54+
}
55+
removals = append(removals, endpointRemovals(was, workload, &is.Workloads[found])...)
56+
}
57+
}
58+
return removals
59+
}
60+
61+
// endpointRemovals names the endpoints of one workload, and the consumers of
62+
// each, that the next file no longer carries.
63+
func endpointRemovals(namespace *cellfile.Namespace, was, is *cellfile.Workload) []cellRemoval {
64+
var removals []cellRemoval
65+
for e := range was.Endpoints {
66+
endpoint := &was.Endpoints[e]
67+
where := fmt.Sprintf("%s/%s/%s", namespace.Name, was.Name, endpoint.Name)
68+
found := slices.IndexFunc(is.Endpoints, func(candidate cellfile.Endpoint) bool { return candidate.Name == endpoint.Name })
69+
if found < 0 {
70+
removals = append(removals, cellRemoval{modules: []string{namespace.Module}, what: "endpoint " + where})
71+
continue
72+
}
73+
for _, consumer := range endpoint.Consumers {
74+
if slices.Contains(is.Endpoints[found].Consumers, consumer) {
75+
continue
76+
}
77+
consumerModule, _, _ := strings.Cut(consumer, "/")
78+
removals = append(removals, cellRemoval{modules: []string{namespace.Module, consumerModule}, what: fmt.Sprintf("consumer %s of %s", consumer, where)})
79+
}
80+
}
81+
return removals
82+
}
83+
84+
// deliveryCellPath is the environment's cell file in the delivery
85+
// repository: beside the modules tree, as the render lays it out in the
86+
// workspace (CellPath).
87+
func deliveryCellPath(gitopsPath, environment string) string {
88+
if gitopsPath == "" {
89+
gitopsPath = DefaultGitopsPath
90+
}
91+
return filepath.ToSlash(filepath.Join(gitopsPath, cellsDir, environment, cellfile.FileName))
92+
}
93+
94+
// holdCellToBase refuses the publication when the workspace's cell file for a
95+
// hosted environment removes anything the delivery base revision's file
96+
// carries, unless every removal concerns a module allowed names. It returns
97+
// the removals allowed, for the plan to show. A workspace with no cell file,
98+
// or a base with none, has nothing to hold.
99+
func holdCellToBase(ctx context.Context, workspace *resources.Workspace, env *environments.Environment, repo, baseBranch, baseRevision, gitopsPath string, allowed []string) ([]string, error) {
100+
if env.Host == nil {
101+
return nil, nil
102+
}
103+
path, err := CellPath(workspace, env)
104+
if err != nil {
105+
return nil, err
106+
}
107+
data, err := os.ReadFile(path)
108+
if errors.Is(err, fs.ErrNotExist) {
109+
return nil, nil
110+
}
111+
if err != nil {
112+
return nil, fmt.Errorf("read environment %s's cell file: %w", env.Name, err)
113+
}
114+
next, err := cellfile.Parse(data)
115+
if err != nil {
116+
return nil, fmt.Errorf("environment %s's cell file %s cannot be read, so it cannot be held to %s's: %w", env.Name, path, baseBranch, err)
117+
}
118+
basePath := deliveryCellPath(gitopsPath, env.Name)
119+
baseData, err := gitCommandBytes(ctx, repo, "show", baseRevision+":"+basePath)
120+
if err != nil {
121+
if gitSaysAbsent(err) {
122+
return nil, nil
123+
}
124+
return nil, fmt.Errorf("read %s's cell file %s: %w", baseBranch, basePath, err)
125+
}
126+
base, err := cellfile.Parse(baseData)
127+
if err != nil {
128+
return nil, fmt.Errorf("%s's cell file %s cannot be read: %w", baseBranch, basePath, err)
129+
}
130+
var refused, admitted []string
131+
unnamed := map[string]bool{}
132+
for _, removal := range cellRemovals(base, next) {
133+
if slices.ContainsFunc(removal.modules, func(module string) bool { return slices.Contains(allowed, module) }) {
134+
admitted = append(admitted, removal.what)
135+
continue
136+
}
137+
refused = append(refused, removal.what)
138+
unnamed[removal.modules[len(removal.modules)-1]] = true
139+
}
140+
if len(refused) > 0 {
141+
modules := make([]string, 0, len(unnamed))
142+
for module := range unnamed {
143+
modules = append(modules, module)
144+
}
145+
slices.Sort(modules)
146+
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)",
147+
env.Name, path, baseBranch, strings.Join(refused, "; "), env.Name, strings.Join(modules, ", "))
148+
}
149+
return admitted, nil
150+
}

0 commit comments

Comments
 (0)