From e293f788f05ace3c7fb9bb15d374bef3309713f4 Mon Sep 17 00:00:00 2001 From: Antoine Toussaint <312536060+antoinetoussaint-byte@users.noreply.github.com> Date: Tue, 3 Nov 2026 23:33:25 -0500 Subject: [PATCH 1/2] feat(gitops): a render refuses what the cell's admission would, and a publish holds a tree to its declaration Three defects reached a cell through every gate in one day. Each was visible from the rendered tree alone, and nothing in the render path looked. 1. An exec probe. A service's agent rendered `livenessProbe.exec`; the cell's execution-admission policy refuses a pod with an exec hook or probe ("a command of the delivering party's choosing inside the pod that already holds the workload identity"), so the workload ran with zero replicas for a day while render, doctor and the promotion's checks were all green. The cell derivation now refuses such a workload by container and field (refuseUnadmittedExecution), and a pod template opting out of the mesh with it: the two rules of that policy that read only the pod template. The cell file this render writes is what generates the policy, so a workload it would describe and the policy would refuse is refused here. 2. A tree of a module the workspace no longer composes. A solution withdrawn from the composition kept its rendered tree, and a module nothing pinned kept one from an earlier composition. deriveCell walked the modules directory and described every tree for the environment, so the cell file approved what the composition had withdrawn; once a cell file was derived without one, the cell refused that tree's pods while the delivery still ran them. refuseUncomposedRender names the tree for removal; with refuseUnrenderedModules the rendered trees and the composed modules are now the same set. 3. A tree rendered before the environment's declaration changed. A commit moved a service's service-config and service-identity rows and nothing compared the rendered tree with the declaration it came from, so the delivery branch ran the pod on the value before the change. The inventory now records a digest of those rows for the module's services (environmentDeclarationDigest, beside the workspace configuration group digests), and publish refuses a tree whose stamp is not the environment's current one, naming the module and the render that lifts it. The services are read from the inventory at both ends, so a tree is never refused for a service the publish could not see; a tree with no stamp is held to nothing. The byte-identical fixture digest moves with the new inventory field, as it has for every inventory change before it; both cases still agree. Co-Authored-By: Claude Fable 5.1 --- pkg/gitops/cell.go | 72 ++++++++++++++++++ pkg/gitops/cell_test.go | 83 +++++++++++++++++++++ pkg/gitops/container_ports_render_test.go | 9 ++- pkg/gitops/environment_declaration.go | 86 ++++++++++++++++++++++ pkg/gitops/environment_declaration_test.go | 73 ++++++++++++++++++ pkg/gitops/publish.go | 3 + pkg/gitops/render.go | 5 ++ pkg/gitops/types.go | 5 ++ pkg/gitops/workloads.go | 38 +++++++++- 9 files changed, 369 insertions(+), 5 deletions(-) create mode 100644 pkg/gitops/environment_declaration.go create mode 100644 pkg/gitops/environment_declaration_test.go diff --git a/pkg/gitops/cell.go b/pkg/gitops/cell.go index 5f32b224c..7ac550a70 100644 --- a/pkg/gitops/cell.go +++ b/pkg/gitops/cell.go @@ -156,6 +156,9 @@ 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 { + return cellfile.File{}, CellResult{}, err + } // A tree rendered before the environment excluded one of its services // still delivers it; describing it would put back what the // composition withdrew. @@ -226,6 +229,72 @@ func refuseUnrenderedModules(ctx context.Context, workspace *resources.Workspace env.Name, env.Name, strings.Join(missing, ", "), strings.Join(commands, "; "), env.Name) } +// refuseUncomposedRender refuses a tree rendered for this environment whose +// module the workspace no longer composes: a solution withdrawn from the +// composition, or a module nothing pins any more. Its render still stands +// under the modules directory, so without this it would be described into +// the cell file as if it were composed — or, when the cell file is derived +// without it, the tree's pods are refused by the cell's admission while the +// delivery still runs them, each refusal reading as an application fault. +// The converse hold is refuseUnrenderedModules: together they make the +// rendered trees and the composed modules the same set. The tree is named +// for removal rather than skipped, because a skipped tree is still delivered. +func refuseUncomposedRender(workspace *resources.Workspace, env *environments.Environment, tree string, inventory *Inventory) error { + for _, reference := range workspace.Modules { + if reference != nil && reference.Name == inventory.Module { + return nil + } + } + return fmt.Errorf("environment %s's cell file is not written: %s holds a render of module %s for %s, but the workspace composes no module of that name, so nothing approves what that tree delivers (the cell refuses its pods while the delivery still runs them); remove that render and the module's delivered tree, or compose the module again", + env.Name, tree, inventory.Module, env.Name) +} + +// meshOptOutLabels and meshOptOutAnnotations are the pod-template fields that +// take a pod out of ambient capture: istio.io/dataplane-mode and +// sidecar.istio.io/inject as labels; ambient.istio.io/bypass-inbound-capture +// and sidecar.istio.io/status (which istio-cni reads as already injected) as +// annotations. The cell's execution-admission policy refuses a pod carrying +// any of them, because STRICT mTLS and every allow derived from the cell file +// stop applying to a pod ztunnel does not capture. +var ( + meshOptOutLabels = []string{"istio.io/dataplane-mode", "sidecar.istio.io/inject"} + meshOptOutAnnotations = []string{"ambient.istio.io/bypass-inbound-capture", "sidecar.istio.io/status"} +) + +// refuseUnadmittedExecution refuses a rendered workload the cell's +// execution-admission policy would refuse on its pod template alone — before +// any digest is compared, so before the cell file approves it. Two of the +// policy's rules read only the template: no container may declare an exec +// lifecycle hook or an exec probe (a command of the delivering party's +// choosing, run inside the pod that already holds the workload identity: the +// build binding undone one field further down from the image), and no pod may +// opt out of the mesh. The cell file this render derives is what generates +// that policy, so a workload it would describe and the policy would refuse is +// a workload with zero replicas and a refusal that reads as an application +// fault; it is refused here, by container and field. +func refuseUnadmittedExecution(module, unit string, rendered *renderedWorkload) error { + for _, containers := range [][]renderedContainer{rendered.Containers, rendered.InitContainers} { + for _, container := range containers { + if len(container.execs) == 0 { + continue + } + return fmt.Errorf("unit %s/%s workload %s: container %s declares %s, which runs a command inside the pod that holds the workload identity; the cell's execution admission refuses such a pod, so the cell file does not approve it. Use an HTTP, TCP or gRPC probe, and put start-up work in the approved image", + module, unit, rendered.Name, container.Name, strings.Join(container.execs, ", ")) + } + } + for _, label := range meshOptOutLabels { + if _, set := rendered.templateLabels[label]; set { + return fmt.Errorf("unit %s/%s workload %s: its pod template carries the %s label, which opts the pod out of the mesh; the cell enrols the namespace and refuses a pod that overrides it, so the cell file does not approve it", module, unit, rendered.Name, label) + } + } + for _, annotation := range meshOptOutAnnotations { + if _, set := rendered.templateAnnotations[annotation]; set { + return fmt.Errorf("unit %s/%s workload %s: its pod template carries the %s annotation, which opts the pod out of the mesh; the cell enrols the namespace and refuses a pod that overrides it, so the cell file does not approve it", module, unit, rendered.Name, annotation) + } + } + return nil +} + // writeCell encodes the cell file and writes it at path. func writeCell(file *cellfile.File, path string) error { body, err := file.Encode() @@ -362,6 +431,9 @@ func cellNamespace(ctx context.Context, workspace *resources.Workspace, env *env if reserved := refuseReservedIdentity(inventory.Module, unit.Name, rendered, selector); reserved != nil { return cellfile.Namespace{}, reserved } + if unadmitted := refuseUnadmittedExecution(inventory.Module, unit.Name, rendered); unadmitted != nil { + return cellfile.Namespace{}, unadmitted + } authenticating, err := authenticatingContainer(unit.Name, rendered) if err != nil { return cellfile.Namespace{}, fmt.Errorf("unit %s/%s workload %s: %w", inventory.Module, unit.Name, rendered.Name, err) diff --git a/pkg/gitops/cell_test.go b/pkg/gitops/cell_test.go index f6825e006..bd8555b10 100644 --- a/pkg/gitops/cell_test.go +++ b/pkg/gitops/cell_test.go @@ -694,3 +694,86 @@ func TestTheCellNamesTheDeliveryEndpointAndNotOnlyItsWorkload(t *testing.T) { }) } } + +// A container declaring an exec probe or an exec lifecycle hook is refused by +// name and field before the cell file approves it: the cell's execution +// admission refuses such a pod (a command of the delivering party's choosing +// runs inside the pod that holds the workload identity), so a cell file that +// described it would approve a workload with zero running replicas. An HTTP +// probe on the same container is admitted. +func TestCellFileRefusesAnExecProbeTheCellWouldRefuse(t *testing.T) { + workspace := writeCellWorkspace(t) + env := selectedEnvironment(t, workspace, "staging") + renderCellTree(t, workspace, "billing", "ledger", "acme-billing", "staging") + result := renderCellTree(t, workspace, "shop", "api", "acme-shop", "staging") + rendered := filepath.Join(result.Path, "services", "api", "overlays", "staging", "deployment.yaml") + data, err := os.ReadFile(rendered) + require.NoError(t, err) + const image = "image: registry.example.test/acme/api:1.2.0@sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + require.Contains(t, string(data), image) + + for _, probe := range []struct{ field, yaml string }{ + {"livenessProbe.exec", "livenessProbe:\n exec:\n command: [python3, -m, api.server, --health]"}, + {"startupProbe.exec", "startupProbe:\n exec:\n command: [true]"}, + {"lifecycle.postStart.exec", "lifecycle:\n postStart:\n exec:\n command: [true]"}, + } { + require.NoError(t, os.WriteFile(rendered, []byte(strings.Replace(string(data), image, image+"\n "+probe.yaml, 1)), 0o644)) + _, err = RenderCell(context.Background(), workspace, env) + require.Error(t, err, probe.field) + require.Contains(t, err.Error(), "unit shop/api workload api: container api declares "+probe.field) + require.Contains(t, err.Error(), "Use an HTTP, TCP or gRPC probe") + } + + httpProbe := "livenessProbe:\n httpGet:\n path: /healthz\n port: http" + require.NoError(t, os.WriteFile(rendered, []byte(strings.Replace(string(data), image, image+"\n "+httpProbe, 1)), 0o644)) + _, err = RenderCell(context.Background(), workspace, env) + require.NoError(t, err, "an HTTP probe is admitted") +} + +// A pod template that opts out of the mesh is refused: the cell enrols the +// namespace and its admission refuses a pod that overrides it, so the file +// does not approve one. +func TestCellFileRefusesAPodThatOptsOutOfTheMesh(t *testing.T) { + workspace := writeCellWorkspace(t) + env := selectedEnvironment(t, workspace, "staging") + renderCellTree(t, workspace, "billing", "ledger", "acme-billing", "staging") + result := renderCellTree(t, workspace, "shop", "api", "acme-shop", "staging") + rendered := filepath.Join(result.Path, "services", "api", "overlays", "staging", "deployment.yaml") + data, err := os.ReadFile(rendered) + require.NoError(t, err) + const templateLabels = " labels:\n app.kubernetes.io/name: api\n" + require.Contains(t, string(data), templateLabels) + + require.NoError(t, os.WriteFile(rendered, []byte(strings.Replace(string(data), templateLabels, templateLabels+" istio.io/dataplane-mode: none\n", 1)), 0o644)) + _, err = RenderCell(context.Background(), workspace, env) + require.ErrorContains(t, err, "unit shop/api workload api: its pod template carries the istio.io/dataplane-mode label, which opts the pod out of the mesh") + + require.NoError(t, os.WriteFile(rendered, []byte(strings.Replace(string(data), templateLabels, templateLabels+" annotations:\n ambient.istio.io/bypass-inbound-capture: \"true\"\n", 1)), 0o644)) + _, err = RenderCell(context.Background(), workspace, env) + require.ErrorContains(t, err, "carries the ambient.istio.io/bypass-inbound-capture annotation") +} + +// A tree rendered for the environment whose module the workspace no longer +// composes — a solution withdrawn from the composition, a module nothing pins +// — refuses the cell file by name, with the tree to remove: described, it +// would approve a workload the composition does not declare; skipped, its +// pods would be refused by the cell while the delivery still runs them. +// Removed, the file is written as before. +func TestCellFileRefusesATreeOfAModuleTheWorkspaceDoesNotCompose(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") + withdrawn := renderCellTree(t, workspace, "proposals", "api", "acme-proposals", "staging") + + _, err := RenderCell(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) +} diff --git a/pkg/gitops/container_ports_render_test.go b/pkg/gitops/container_ports_render_test.go index 85b77f075..c73b90e58 100644 --- a/pkg/gitops/container_ports_render_test.go +++ b/pkg/gitops/container_ports_render_test.go @@ -310,7 +310,14 @@ func treeDigest(t *testing.T, root string) string { // place of the presence, authority and endpoint documents): the fixture // declares no manifest, and the digest moves with the schemaVersion alone — // verified by rendering it at schema 7, which reproduces the previous digest. -const undeclaredFixtureDigest = "49cb877e08a7a2079deea3c862b33ad5e592ede4d449aebc02682ede9f60021f" +// +// And once more when the inventory started recording the environment +// declaration digest (environmentDeclarationDigest: the service-config and +// service-identity rows of the module's services, which a publish holds the +// tree to). The fixture declares no such row, so the stamp is the digest of an +// empty declaration; the manifests are unchanged and the two cases below still +// agree, which is the property this digest pins. +const undeclaredFixtureDigest = "8bb67b42d7ab8a8a9a0c05ee0023ad273e5ef32278efe8a1b6a30808d7687df8" func TestRenderModuleWithoutDeclarationsIsByteIdentical(t *testing.T) { installFakeAgents(t) diff --git a/pkg/gitops/environment_declaration.go b/pkg/gitops/environment_declaration.go new file mode 100644 index 000000000..9748b84de --- /dev/null +++ b/pkg/gitops/environment_declaration.go @@ -0,0 +1,86 @@ +package gitops + +import ( + "crypto/sha256" + "encoding/hex" + "fmt" + "sort" + "strings" + + "github.com/codefly-dev/cli/pkg/environments" +) + +// --- The environment declaration a render bakes in --- +// +// Beside the workspace configuration groups (configuration_groups.go), a +// render bakes in the environment's own rows for the module's services: the +// service-config values it injects into each container, and the +// service-identity each workload runs as. Change those rows and publish +// nothing, and the delivery branch holds a tree rendered from a declaration +// that no longer exists, with nothing in the tree saying so: the workload keeps +// running on the value before the change and looks correctly configured. +// +// So every render records a digest of those rows as the environment declared +// them, over the services its inventory names, and a publish re-digests the +// same services against the environment as it is now and refuses a tree the +// declaration has moved past. The services are read from the inventory at +// both ends, so the two digests cover one set and a render is never refused +// for a service the publish could not see. + +// serviceUnitNames is the name of every service unit an inventory records, +// managed units included: a managed service's identity rows are declared the +// same way, and a unit with no rows digests the same at both ends. +func serviceUnitNames(units []InventoryUnit) []string { + var names []string + for _, unit := range units { + if unit.Kind == UnitKindService && unit.Name != "" { + names = append(names, unit.Name) + } + } + sort.Strings(names) + return names +} + +// environmentDeclarationDigest digests, for the named services of a module, +// the environment's service-config values and service-identity as it declares +// them now. The result is a sha256 over one line per declared row, sorted, so +// two declarations that differ only in order digest the same. +func environmentDeclarationDigest(env *environments.Environment, module string, services []string) string { + var lines []string + for _, service := range services { + for key, value := range env.ServiceConfig.ValuesFor(module, service) { + lines = append(lines, fmt.Sprintf("service-config %s/%s %s=%s", module, service, key, value)) + } + identity := env.WorkloadIdentityFor(module, service) + if identity == nil { + continue + } + lines = append(lines, fmt.Sprintf("service-identity %s/%s kind=%s principal=%s", module, service, identity.Kind, identity.Principal)) + for key, value := range identity.Annotations { + lines = append(lines, fmt.Sprintf("service-identity %s/%s annotation %s=%s", module, service, key, value)) + } + for key, value := range identity.Labels { + lines = append(lines, fmt.Sprintf("service-identity %s/%s label %s=%s", module, service, key, value)) + } + } + sort.Strings(lines) + sum := sha256.Sum256([]byte(strings.Join(lines, "\n"))) + return "sha256:" + hex.EncodeToString(sum[:]) +} + +// refuseStaleDeclaration refuses to publish a tree whose recorded environment +// declaration digest is not the environment's current one for the same +// services. A tree with no recorded digest was rendered by a CLI that did not +// record one and is held to nothing here: the hold cannot say what that +// render baked in, and the next render records it. +func refuseStaleDeclaration(env *environments.Environment, inventory *Inventory) error { + if env == nil || inventory == nil || inventory.EnvironmentDeclarationDigest == "" { + return nil + } + current := environmentDeclarationDigest(env, inventory.Module, serviceUnitNames(inventory.Units)) + if current == inventory.EnvironmentDeclarationDigest { + return nil + } + return fmt.Errorf("the rendered tree of %s was rendered before environment %s's declaration for its services changed (service-config or service-identity rows of %s); render %s again for %s before publishing it", + inventory.Module, env.Name, strings.Join(serviceUnitNames(inventory.Units), ", "), inventory.Module, env.Name) +} diff --git a/pkg/gitops/environment_declaration_test.go b/pkg/gitops/environment_declaration_test.go new file mode 100644 index 000000000..baf15208c --- /dev/null +++ b/pkg/gitops/environment_declaration_test.go @@ -0,0 +1,73 @@ +package gitops + +import ( + "context" + "testing" + + "github.com/codefly-dev/cli/pkg/environments" + "github.com/codefly-dev/core/resources" + "github.com/stretchr/testify/require" +) + +// declaredWorkspace is the cell fixture with service-config and +// service-identity rows for shop/api in staging, at the given mode and +// principal. +func declaredWorkspace(t *testing.T, mode, principal string) (*resources.Workspace, *environments.Environment) { + t.Helper() + rows := " service-config:\n services:\n shop/api: {values: {MODE: " + mode + "}}\n" + + " service-identity:\n services:\n shop/api: {principal: " + principal + ", annotations: {iam.gke.io/gcp-service-account: " + principal + "}}\n" + workspace := writeCellWorkspace(t, [3]string{resources.WorkspaceConfigurationName, " host:\n", rows + " host:\n"}) + return workspace, selectedEnvironment(t, workspace, "staging") +} + +// The digest covers the environment's service-config and service-identity rows +// for the module's own services: it moves when either row of shop/api moves, +// in any order of declaration, and not for billing, whose rows did not. +func TestTheDeclarationDigestFollowsTheModulesOwnRows(t *testing.T) { + _, before := declaredWorkspace(t, "a", "shop@example.test") + _, config := declaredWorkspace(t, "b", "shop@example.test") + _, identity := declaredWorkspace(t, "a", "other@example.test") + _, same := declaredWorkspace(t, "a", "shop@example.test") + + shop := environmentDeclarationDigest(before, "shop", []string{"api"}) + require.Equal(t, shop, environmentDeclarationDigest(same, "shop", []string{"api"})) + require.NotEqual(t, shop, environmentDeclarationDigest(config, "shop", []string{"api"}), "a service-config value moved") + require.NotEqual(t, shop, environmentDeclarationDigest(identity, "shop", []string{"api"}), "the service-identity moved") + billing := environmentDeclarationDigest(before, "billing", []string{"ledger"}) + require.Equal(t, billing, environmentDeclarationDigest(config, "billing", []string{"ledger"}), "another module's rows are not billing's") +} + +// A render for a target environment stamps the digest into its inventory, and +// a publish holds the tree to it: the same rows at publish admit it, moved +// rows refuse it by module and environment, and a tree rendered by a CLI that +// recorded no stamp is held to nothing here. +func TestAPublishRefusesATreeRenderedBeforeTheDeclarationMoved(t *testing.T) { + workspace, env := declaredWorkspace(t, "a", "shop@example.test") + result := renderCellTreeWith(t, &RenderOptions{ + Destination: moduleRenderDestination(workspace, "shop"), + Module: "shop", Environment: "staging", Namespace: "acme-shop", AppProject: "acme-staging", + Promotable: true, OwnedPath: "deployments/modules/shop", + Units: unitsWithPorts("shop", "api"), + Target: env, + }, "api", "staging") + inventory, err := LoadInventory(result.Path) + require.NoError(t, err) + require.Equal(t, environmentDeclarationDigest(env, "shop", []string{"api"}), inventory.EnvironmentDeclarationDigest) + require.NoError(t, refuseStaleDeclaration(env, &inventory)) + + _, moved := declaredWorkspace(t, "a", "rotated@example.test") + err = refuseStaleDeclaration(moved, &inventory) + require.ErrorContains(t, err, "the rendered tree of shop was rendered before environment staging's declaration for its services changed") + require.ErrorContains(t, err, "render shop again for staging before publishing it") + + unstamped := inventory + unstamped.EnvironmentDeclarationDigest = "" + require.NoError(t, refuseStaleDeclaration(moved, &unstamped)) + + // A render with no target environment records no stamp. + plain := renderCellTree(t, workspace, "billing", "ledger", "acme-billing", "staging") + billing, err := LoadInventory(plain.Path) + require.NoError(t, err) + require.Empty(t, billing.EnvironmentDeclarationDigest) + _ = context.Background() +} diff --git a/pkg/gitops/publish.go b/pkg/gitops/publish.go index feb771147..308f6d997 100644 --- a/pkg/gitops/publish.go +++ b/pkg/gitops/publish.go @@ -516,6 +516,9 @@ func holdRenderedGroups(ctx context.Context, workspace *resources.Workspace, req if err = refuseStaleRender(request.Module, request.Environment, inventory.WorkspaceConfigurationDigests, current); err != nil { return err } + if err = refuseStaleDeclaration(env, inventory); err != nil { + return err + } return refuseStaleGroupConsumers(workspace.Dir(), request.Module, request.Environment, inventory.WorkspaceConfigurationDigests) } diff --git a/pkg/gitops/render.go b/pkg/gitops/render.go index 2310ae6e4..83e900f75 100644 --- a/pkg/gitops/render.go +++ b/pkg/gitops/render.go @@ -1676,6 +1676,11 @@ func inventoryHead(opts *RenderOptions) Inventory { sort.Slice(inventory.Units, func(i, j int) bool { return inventory.Units[i].Name < inventory.Units[j].Name }) + // Stamped once the units are known, over the same service names a + // publish reads back out of this inventory. + if opts.Target != nil { + inventory.EnvironmentDeclarationDigest = environmentDeclarationDigest(opts.Target, inventory.Module, serviceUnitNames(inventory.Units)) + } return inventory } diff --git a/pkg/gitops/types.go b/pkg/gitops/types.go index e4dd7c62e..26c383893 100644 --- a/pkg/gitops/types.go +++ b/pkg/gitops/types.go @@ -121,6 +121,11 @@ type Inventory struct { // and recording another digest was rendered against another value, and the // render refuses to deliver two values of one group. WorkspaceConfigurationDigests map[string]string `json:"workspaceConfigurationDigests,omitempty"` + // EnvironmentDeclarationDigest records a digest of the environment's + // service-config and service-identity rows for this module's services as + // the render read them (environment_declaration.go). Publish re-digests + // the same rows and refuses a tree the declaration has moved past. + EnvironmentDeclarationDigest string `json:"environmentDeclarationDigest,omitempty"` // MeshEdges are the L4 edges into this module's workloads, as the // composition declares them: the data the cell's applier compiles into // its mesh policy (mesh_edges.go). Absent when the environment declares diff --git a/pkg/gitops/workloads.go b/pkg/gitops/workloads.go index 62bab5b8e..3b203e5b9 100644 --- a/pkg/gitops/workloads.go +++ b/pkg/gitops/workloads.go @@ -35,8 +35,10 @@ type renderedWorkload struct { volumes []string // mounts are every container's volume mounts, by container name. mounts map[string][]containerMount - // templateAnnotations are the pod template's annotations. + // templateAnnotations are the pod template's annotations, and + // templateLabels its labels. templateAnnotations map[string]any + templateLabels map[string]any } // renderedContainer is one container of a pod template. Its image is pinned @@ -48,6 +50,13 @@ type renderedContainer struct { // the two halves so no reader parses them back out of one string. Repository string Digest string + // execs are the fields of this container that run a command of the + // delivering party's choosing inside the pod — an exec lifecycle hook or + // an exec probe — named as the pod spec spells them. A cell admits a pod + // by the digest of what runs in it, and an exec field says what the + // approved bytes do, so the cell's admission refuses such a pod; the + // render refuses it first (refuseUnadmittedExecution). + execs []string } // tokenMount is one projected ServiceAccount token volume carrying an @@ -109,8 +118,9 @@ func workloadsFromManifests(manifests []manifest) ([]renderedWorkload, error) { } workload := renderedWorkload{Name: metadataString(item.value, "name"), Kind: item.kind, tokenMounts: map[string][]tokenMount{}, mounts: map[string][]containerMount{}} workload.ServiceAccount, _ = spec["serviceAccountName"].(string) - workload.templateAnnotations = mapField(mapField(mapField(item.value, "spec"), "template"), "metadata") - workload.templateAnnotations = mapField(workload.templateAnnotations, "annotations") + templateMetadata := mapField(mapField(mapField(item.value, "spec"), "template"), "metadata") + workload.templateAnnotations = mapField(templateMetadata, "annotations") + workload.templateLabels = mapField(templateMetadata, "labels") if workload.ServiceAccount == "" { workload.ServiceAccount = defaultServiceAccount } @@ -191,7 +201,7 @@ func renderedContainers(workload string, raw []any, tokens map[string][]tokenSou if err != nil { return nil, fmt.Errorf("workload %s container %s: %w", workload, name, err) } - rendered := renderedContainer{Name: name, Repository: repository, Digest: digest} + rendered := renderedContainer{Name: name, Repository: repository, Digest: digest, execs: execFields(container)} for _, raw := range sliceField(container, "volumeMounts") { mount, _ := raw.(map[string]any) volume, _ := mount["name"].(string) @@ -218,6 +228,26 @@ func renderedContainers(workload string, raw []any, tokens map[string][]tokenSou return containers, nil } +// execFields names the exec hooks and probes a container declares, as the pod +// spec spells them: lifecycle.postStart.exec, lifecycle.preStop.exec, +// livenessProbe.exec, readinessProbe.exec, startupProbe.exec. The list is the +// one the cell's execution-admission policy judges, in its order. +func execFields(container map[string]any) []string { + var fields []string + lifecycle := mapField(container, "lifecycle") + for _, hook := range []string{"postStart", "preStop"} { + if _, declared := mapField(lifecycle, hook)["exec"]; declared { + fields = append(fields, "lifecycle."+hook+".exec") + } + } + for _, probe := range []string{"livenessProbe", "readinessProbe", "startupProbe"} { + if _, declared := mapField(container, probe)["exec"]; declared { + fields = append(fields, probe+".exec") + } + } + return fields +} + // Keep the staged root while selecting a unit. Libraries see a checked tree, // never a root opened on an unchecked service path. func renderedWorkloadsInTree(tree *renderRoot, unitDir, environment string) ([]renderedWorkload, error) { From 57e2fb71dac2ec8e1d5c318f070eba2d58c499ab Mon Sep 17 00:00:00 2001 From: Antoine Toussaint <312536060+antoinetoussaint-byte@users.noreply.github.com> Date: Tue, 3 Nov 2026 23:42:41 -0500 Subject: [PATCH 2/2] fix(gitops): read a CronJob's pod template where it stands Review found the mesh opt-out check a false pass for CronJobs: the template metadata was read at spec.template, and a CronJob keeps its pod template under spec.jobTemplate.spec.template, so an opt-out label there passed the render and would be refused by the cell at run time. The metadata is read through podTemplate, as the pod spec already was, and the opt-out test adds a CronJob. Also drops a stray context import from the declaration test and states that solution units are outside the declaration digest. Co-Authored-By: Claude Fable 5.1 --- pkg/gitops/cell_test.go | 27 ++++++++++++++++++++++ pkg/gitops/environment_declaration.go | 7 +++--- pkg/gitops/environment_declaration_test.go | 2 -- pkg/gitops/workloads.go | 5 +++- 4 files changed, 35 insertions(+), 6 deletions(-) diff --git a/pkg/gitops/cell_test.go b/pkg/gitops/cell_test.go index bd8555b10..142e186d9 100644 --- a/pkg/gitops/cell_test.go +++ b/pkg/gitops/cell_test.go @@ -751,6 +751,33 @@ func TestCellFileRefusesAPodThatOptsOutOfTheMesh(t *testing.T) { require.NoError(t, os.WriteFile(rendered, []byte(strings.Replace(string(data), templateLabels, templateLabels+" annotations:\n ambient.istio.io/bypass-inbound-capture: \"true\"\n", 1)), 0o644)) _, err = RenderCell(context.Background(), workspace, env) require.ErrorContains(t, err, "carries the ambient.istio.io/bypass-inbound-capture annotation") + + // A CronJob's pod template stands under spec.jobTemplate.spec.template; + // an opt-out there is read where it stands. + cronJob := `--- +apiVersion: batch/v1 +kind: CronJob +metadata: + name: api-sweep +spec: + schedule: "0 * * * *" + jobTemplate: + spec: + template: + metadata: + labels: + codefly.dev/bootstrap-service: api + sidecar.istio.io/inject: "false" + spec: + serviceAccountName: api + restartPolicy: OnFailure + containers: + - name: api + image: registry.example.test/acme/api-sweep@sha256:eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee +` + require.NoError(t, os.WriteFile(rendered, append([]byte(data), []byte(cronJob)...), 0o644)) + _, err = RenderCell(context.Background(), workspace, env) + require.ErrorContains(t, err, "workload api-sweep: its pod template carries the sidecar.istio.io/inject label") } // A tree rendered for the environment whose module the workspace no longer diff --git a/pkg/gitops/environment_declaration.go b/pkg/gitops/environment_declaration.go index 9748b84de..50fbe1189 100644 --- a/pkg/gitops/environment_declaration.go +++ b/pkg/gitops/environment_declaration.go @@ -27,9 +27,10 @@ import ( // both ends, so the two digests cover one set and a render is never refused // for a service the publish could not see. -// serviceUnitNames is the name of every service unit an inventory records, -// managed units included: a managed service's identity rows are declared the -// same way, and a unit with no rows digests the same at both ends. +// serviceUnitNames is the name of every unit of Kind service an inventory +// records, managed ones (Kind service, Managed true) included: a managed +// service's identity rows are declared the same way, and a unit with no rows +// digests the same at both ends. Solution units are not among them. func serviceUnitNames(units []InventoryUnit) []string { var names []string for _, unit := range units { diff --git a/pkg/gitops/environment_declaration_test.go b/pkg/gitops/environment_declaration_test.go index baf15208c..7aace9eb7 100644 --- a/pkg/gitops/environment_declaration_test.go +++ b/pkg/gitops/environment_declaration_test.go @@ -1,7 +1,6 @@ package gitops import ( - "context" "testing" "github.com/codefly-dev/cli/pkg/environments" @@ -69,5 +68,4 @@ func TestAPublishRefusesATreeRenderedBeforeTheDeclarationMoved(t *testing.T) { billing, err := LoadInventory(plain.Path) require.NoError(t, err) require.Empty(t, billing.EnvironmentDeclarationDigest) - _ = context.Background() } diff --git a/pkg/gitops/workloads.go b/pkg/gitops/workloads.go index 3b203e5b9..b50f8d3ca 100644 --- a/pkg/gitops/workloads.go +++ b/pkg/gitops/workloads.go @@ -118,7 +118,10 @@ func workloadsFromManifests(manifests []manifest) ([]renderedWorkload, error) { } workload := renderedWorkload{Name: metadataString(item.value, "name"), Kind: item.kind, tokenMounts: map[string][]tokenMount{}, mounts: map[string][]containerMount{}} workload.ServiceAccount, _ = spec["serviceAccountName"].(string) - templateMetadata := mapField(mapField(mapField(item.value, "spec"), "template"), "metadata") + // podTemplate, not spec.template: a CronJob's template stands under + // spec.jobTemplate.spec.template, and a label read at the wrong + // path is a mesh opt-out the render would never see. + templateMetadata := mapField(podTemplate(item), "metadata") workload.templateAnnotations = mapField(templateMetadata, "annotations") workload.templateLabels = mapField(templateMetadata, "labels") if workload.ServiceAccount == "" {