Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 26 additions & 0 deletions docs/UPGRADING.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,32 @@ guidance that the changelog's breaking-change entries link to.
We are pre-1.0, so breaking changes bump the **minor** version (release-please is configured with
`bump-minor-pre-major`) rather than the major. Read the relevant entry before upgrading across it.

## Unreleased — a `digest:` override no longer strips the tag out of your source file (next patch; bug fix)

**If any of your kustomizations use `images:` with `digest:`, or `newTag:` on an image
that carries a digest, the operator has been rewriting your source manifests. This stops.**

kustomize's image transformer treats tag and digest as mutually exclusive — its own code
says *"overriding tag or digest will replace both original tag and digest values"*. Our
re-implementation set the two components independently, so:

| Source image | `images:` entry | kustomize renders | We believed |
|---|---|---|---|
| `app:1.0.0` | `digest: sha256:bbb` | `app@sha256:bbb` | `app:1.0.0@sha256:bbb` |
| `app@sha256:old` | `newTag: "2.0"` | `app:2.0` | `app:2.0@sha256:old` |

Believing the wrong render, the projection compared the real live object against it,
concluded the user had *removed* the tag, and wrote the tag out of the source document —
`app:1.0.0` became `app`. On every reconcile, silently, with no refusal and no diagnostic.

**Migration**

- **Check the affected files.** Any manifest referenced by a kustomization whose `images:`
entry sets `digest:` may have lost its tag in Git. The fix stops the rewrite but does not
restore what was already written; recover the tag from history if you need it.
- Nothing to configure. The behaviour is simply correct now, and pinned against a real
`kustomize build` so it cannot regress.

## Unreleased — `kustomization.yaml` is now read by kustomize itself (next minor; behavior change)

The analyzer used to decode `kustomization.yaml` with a hand-written walk over a generic
Expand Down
15 changes: 15 additions & 0 deletions docs/design/support-boundary/kustomize-support-boundary.md
Original file line number Diff line number Diff line change
Expand Up @@ -284,6 +284,21 @@ kustomize" means two different things:
renderer. The re-implementation is being removed.** This reverses the earlier
position — kept here because the reasoning was wrong in a specific, instructive way.

> **Landed so far.** The typed `kustomization.yaml` parse (#229), and
> [`kustomize_render.go`](../../../internal/manifestanalyzer/kustomize_render.go) — a
> sandboxed `krusty` build that returns each object with kustomize's own provenance
> (`config.kubernetes.io/origin` says which file produced it;
> `alpha.config.kubernetes.io/transformations` says which kustomization's transformers
> touched it, in order). The renderer is not yet on the write path: it is currently the
> differential oracle the re-implementation is checked against, which is what makes
> removing the re-implementation safe rather than brave.
>
> **What the oracle found immediately:** the re-implemented image transformer treated
> tag and digest as independent, where kustomize replaces both. A folder using `digest:`
> had the tag written out of its source manifest on every reconcile. Two silent-corruption
> bugs, in shipped code, found by the first differential run — which is the whole argument
> for this decision, made concrete.

The old position was that re-implementing the narrow transformer subset "keeps the
refusal boundary honest: we refuse exactly what we do not model," and that `krusty`
was at best a *verification oracle* comparing against our own projection.
Expand Down
4 changes: 3 additions & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ require (
sigs.k8s.io/cli-utils v0.37.2
sigs.k8s.io/controller-runtime v0.24.1
sigs.k8s.io/kustomize/api v0.21.1
sigs.k8s.io/kustomize/kyaml v0.21.1
sigs.k8s.io/yaml v1.6.0
)

Expand Down Expand Up @@ -88,6 +89,7 @@ require (
github.com/klauspost/cpuid/v2 v2.3.0 // indirect
github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd // indirect
github.com/modern-go/reflect2 v1.0.3-0.20250322232337-35a7c28c31ee // indirect
github.com/monochromegane/go-gitignore v0.0.0-20200626010858-205db1a8cc00 // indirect
github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 // indirect
github.com/pjbgf/sha1cd v0.6.0 // indirect
github.com/prometheus/client_model v0.6.2 // indirect
Expand All @@ -100,6 +102,7 @@ require (
github.com/stretchr/objx v0.5.3 // indirect
github.com/x448/float16 v0.8.4 // indirect
github.com/xanzy/ssh-agent v0.3.3 // indirect
github.com/xlab/treeprint v1.2.0 // indirect
github.com/yuin/gopher-lua v1.1.2 // indirect
go.opentelemetry.io/auto/sdk v1.2.1 // indirect
go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.68.0 // indirect
Expand Down Expand Up @@ -137,7 +140,6 @@ require (
k8s.io/streaming v0.36.2 // indirect
sigs.k8s.io/apiserver-network-proxy/konnectivity-client v0.34.0 // indirect
sigs.k8s.io/json v0.0.0-20250730193827-2d320260d730 // indirect
sigs.k8s.io/kustomize/kyaml v0.21.1 // indirect
sigs.k8s.io/randfill v1.0.0 // indirect
sigs.k8s.io/structured-merge-diff/v6 v6.4.0 // indirect
)
6 changes: 6 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,8 @@ github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd/go.mod h1:6dJ
github.com/modern-go/reflect2 v1.0.2/go.mod h1:yWuevngMOJpCy52FWWMvUC8ws7m/LJsjYzDa0/r8luk=
github.com/modern-go/reflect2 v1.0.3-0.20250322232337-35a7c28c31ee h1:W5t00kpgFdJifH4BDsTlE89Zl93FEloxaWZfGcifgq8=
github.com/modern-go/reflect2 v1.0.3-0.20250322232337-35a7c28c31ee/go.mod h1:yWuevngMOJpCy52FWWMvUC8ws7m/LJsjYzDa0/r8luk=
github.com/monochromegane/go-gitignore v0.0.0-20200626010858-205db1a8cc00 h1:n6/2gBQ3RWajuToeY6ZtZTIKv2v7ThUy5KKusIT0yc0=
github.com/monochromegane/go-gitignore v0.0.0-20200626010858-205db1a8cc00/go.mod h1:Pm3mSP3c5uWn86xMLZ5Sa7JB9GsEZySvHYXCTK4E9q4=
github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 h1:C3w9PqII01/Oq1c1nUAm88MOHcQC9l5mIlSMApZMrHA=
github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822/go.mod h1:+n7T8mK8HuQTcFwEeznm/DIxMOiR9yIdICNftLE1DvQ=
github.com/mwitkow/go-conntrack v0.0.0-20190716064945-2f068394615f h1:KUppIJq7/+SVif2QVs3tOP0zanoHgBEVAwHxUSIzRqU=
Expand Down Expand Up @@ -225,6 +227,7 @@ github.com/stretchr/objx v0.5.3/go.mod h1:rDQraq+vQZU7Fde9LOZLr8Tax6zZvy4kuNKF+Q
github.com/stretchr/testify v1.2.2/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXfy6kDkUVs=
github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI=
github.com/stretchr/testify v1.4.0/go.mod h1:j7eGeouHqKxXV5pUuKE4zz7dFj8WfuZ+81PSLYec5m4=
github.com/stretchr/testify v1.7.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg=
github.com/stretchr/testify v1.11.1 h1:7s2iGBzp5EwR7/aIZr8ao5+dra3wiQyKjjFuvgVKu7U=
github.com/stretchr/testify v1.11.1/go.mod h1:wZwfW3scLgRK+23gO65QZefKpKQRnfz6sD981Nm4B6U=
github.com/tidwall/gjson v1.18.0 h1:FIDeeyB800efLX89e5a8Y0BNH+LOngJyGrIWxG2FKQY=
Expand All @@ -239,6 +242,8 @@ github.com/x448/float16 v0.8.4 h1:qLwI1I70+NjRFUR3zs1JPUCgaCXSh3SW62uAKT1mSBM=
github.com/x448/float16 v0.8.4/go.mod h1:14CWIYCyZA/cWjXOioeEpHeN/83MdbZDRQHoFcYsOfg=
github.com/xanzy/ssh-agent v0.3.3 h1:+/15pJfg/RsTxqYcX6fHqOXZwwMP+2VyYWJeWM2qQFM=
github.com/xanzy/ssh-agent v0.3.3/go.mod h1:6dzNDKs0J9rVPHPhaGCukekBHKqfl+L3KghI1Bc68Uw=
github.com/xlab/treeprint v1.2.0 h1:HzHnuAF1plUN2zGlAFHbSQP2qJ0ZAD3XF5XD7OesXRQ=
github.com/xlab/treeprint v1.2.0/go.mod h1:gj5Gd3gPdKtR1ikdDK6fnFLdmIS0X30kTTuNd/WEJu0=
github.com/yuin/gopher-lua v1.1.2 h1:yF/FjE3hD65tBbt0VXLE13HWS9h34fdzJmrWRXwobGA=
github.com/yuin/gopher-lua v1.1.2/go.mod h1:7aRmXIWl37SqRf0koeyylBEzJ+aPt8A+mmkQ4f1ntR8=
github.com/zeebo/xxh3 v1.1.0 h1:s7DLGDK45Dyfg7++yxI0khrfwq9661w9EN78eP/UZVs=
Expand Down Expand Up @@ -336,6 +341,7 @@ gopkg.in/warnings.v0 v0.1.2 h1:wFXVbFY8DY5/xOe1ECiWdKCzZlxgshcYVNkBHstARME=
gopkg.in/warnings.v0 v0.1.2/go.mod h1:jksf8JmL6Qr/oQM2OXTHunEvvTAsrWBLb6OOjuVWRNI=
gopkg.in/yaml.v2 v2.2.2/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI=
gopkg.in/yaml.v2 v2.4.0/go.mod h1:RDklbk79AGWmwhnvt/jBztapEOGDOx6ZbXqjP6csGnQ=
gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=
gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA=
gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=
k8s.io/api v0.36.2 h1:TF6YDLIzKfccK7cq9YpTcGX8TJmEkHVRv78DM51fRYY=
Expand Down
263 changes: 263 additions & 0 deletions internal/manifestanalyzer/kustomize_render.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,263 @@
// SPDX-License-Identifier: Apache-2.0

package manifestanalyzer

import (
"errors"
"fmt"
"path"

"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"sigs.k8s.io/kustomize/api/krusty"
"sigs.k8s.io/kustomize/api/resmap"
kustypes "sigs.k8s.io/kustomize/api/types"
"sigs.k8s.io/kustomize/kyaml/filesys"
"sigs.k8s.io/yaml"

"github.com/ConfigButler/gitops-reverser/internal/git/manifestedit"
)

// This file renders a kustomize render root with kustomize itself, rather than
// re-implementing its transformers. It is the ground truth the projection is
// checked against: what we believe a folder renders to becomes what the library
// Flux renders with says it renders to.
//
// See docs/design/support-boundary/kustomize-support-boundary.md §7.

// The provenance kustomize emits when buildMetadata asks for it. These are the
// reason we need not walk the resources graph ourselves:
//
// config.kubernetes.io/origin: path: ../base/deployment.yaml
// alpha.config.kubernetes.io/transformations:
// - configuredIn: ../base/kustomization.yaml
// configuredBy: {apiVersion: builtin, kind: ImageTagTransformer}
//
// The first says which source file produced the object. The second says which
// kustomization's transformers touched it, in build order — the override chain,
// handed to us by the renderer that applies it.
const (
originAnnotation = "config.kubernetes.io/origin"
transformationsAnnotation = "alpha.config.kubernetes.io/transformations"
)

// imageTagTransformer and replicaCountTransformer are the builtin transformers
// behind the two edit-through channels, as kustomize names them in the
// transformations annotation.
const (
imageTagTransformer = "ImageTagTransformer"
replicaCountTransformer = "ReplicaCountTransformer"
)

// errRemoteBase refuses a build whose kustomization reaches outside the repository.
//
// The check must run BEFORE krusty, never inside it: kustomize resolves a remote
// base by shelling out to `git fetch`, and it does so under LoadRestrictionsRootOnly
// and under an in-memory filesystem alike (both measured). No build option turns it
// off, so refusing first is the only thing that keeps "the operator never fetches a
// remote base" true.
var errRemoteBase = errors.New("kustomization reaches a remote base; the operator never fetches one")

// renderedObject is one object kustomize produced, with the provenance saying
// where it came from and what shaped it.
type renderedObject struct {
// Object is the rendered result: what the GitOps controller will apply.
Object *unstructured.Unstructured
// OriginPath is the source file that produced it, relative to the scan root.
// Empty for a generated resource (which the acceptance gate refuses anyway).
OriginPath string
// TransformedBy lists the kustomizations whose transformers touched it, in
// build order (innermost base first) — the override chain, from kustomize.
TransformedBy []transformation
}

// transformation is one entry of kustomize's transformations annotation: which
// kustomization configured which builtin transformer.
type transformation struct {
// ConfiguredIn is the kustomization file, relative to the scan root.
ConfiguredIn string
// Kind is the builtin transformer, e.g. "ImageTagTransformer".
Kind string
}

// renderMountPoint is where the scanned tree is mounted in the in-memory
// filesystem. The whole scan is mounted, not just the render root, so a
// kustomization can read a base beside or below it. This is a READ scope: the
// write jail is enforced in the writer (L1) and is not this function's job.
const renderMountPoint = "/scan"

// renderRoot builds one render root with kustomize and returns every object it
// produces, carrying provenance. rootDir is slash-relative to the scan root, and
// files are the scan's YAML files, which become an in-memory filesystem — so the
// build never touches the real disk, never executes a plugin, and never reaches the
// network.
func renderRoot(files []manifestedit.FileContent, rootDir string) ([]renderedObject, error) {
if err := refuseRemoteBases(parseKustomizations(files), rootDir); err != nil {
return nil, err
}
fSys, err := renderFilesystem(files, rootDir)
if err != nil {
return nil, err
}
k := krusty.MakeKustomizer(&krusty.Options{
LoadRestrictions: kustypes.LoadRestrictionsRootOnly,
PluginConfig: kustypes.DisabledPluginConfig(), // no exec, no Go plugins
})
resMap, err := k.Run(fSys, path.Join(renderMountPoint, rootDir))
if err != nil {
return nil, fmt.Errorf("kustomize build: %w", err)
}
return collectRendered(resMap, rootDir)
}

// refuseRemoteBases refuses the build when any kustomization THIS ROOT REACHES
// declares a remote base.
//
// Scoping it to the reachable graph is deliberate, and it is both safer and more
// accurate than a scan-wide check: kustomize only fetches what it actually loads,
// so a remote base in an unrelated sibling folder cannot make this build reach the
// network — and refusing on its account would refuse a folder that is perfectly
// renderable.
func refuseRemoteBases(kusts map[string]*kustomizationDoc, rootDir string) error {
visited := map[string]struct{}{}
var walk func(dir string) error
walk = func(dir string) error {
if _, seen := visited[dir]; seen {
return nil
}
visited[dir] = struct{}{}
cur := kusts[dir]
if cur == nil {
return nil
}
if hasRemoteResource(cur.resources) {
return fmt.Errorf("%s: %w", cur.path, errRemoteBase)
}
for _, entry := range cur.resources {
target := cleanJoin(dir, entry)
if target == "" {
continue
}
if _, isKust := kusts[target]; isKust {
if err := walk(target); err != nil {
return err
}
}
}
return nil
}
return walk(rootDir)
}

// renderFilesystem materialises the scan's files in memory and asks the render root
// for provenance.
func renderFilesystem(files []manifestedit.FileContent, rootDir string) (filesys.FileSystem, error) {
fSys := filesys.MakeFsInMemory()
rootKust := path.Join(rootDir, "kustomization.yaml")

for _, f := range files {
rel := filepathToSlash(f.Path)
content := f.Content

// Only the root needs to ask for provenance: the annotations describe the
// whole build, bases included.
if rel == rootKust {
var k kustypes.Kustomization
if err := k.Unmarshal(content); err != nil {
return nil, fmt.Errorf("%s: %w", rel, err)
}
k.FixKustomization()
var err error
if content, err = withBuildMetadata(k); err != nil {
return nil, fmt.Errorf("%s: %w", rel, err)
}
}
if err := fSys.WriteFile(path.Join(renderMountPoint, rel), content); err != nil {
return nil, fmt.Errorf("%s: %w", rel, err)
}
}
return fSys, nil
}

// withBuildMetadata re-serialises a kustomization with the provenance build
// metadata added. It rewrites only our in-memory render copy; the user's file is
// never touched, so losing their comments here costs nothing.
func withBuildMetadata(k kustypes.Kustomization) ([]byte, error) {
k.BuildMetadata = []string{kustypes.OriginAnnotations, kustypes.TransformerAnnotations}
out, err := yaml.Marshal(&k)
if err != nil {
return nil, fmt.Errorf("re-serialising kustomization for render: %w", err)
}
return out, nil
}

// collectRendered turns kustomize's ResMap into rendered objects, lifting the
// provenance off each one and then stripping it: the annotations are our
// scaffolding, not part of what the folder renders to, and an object carrying them
// would compare unequal to the live object it describes.
func collectRendered(resMap resmap.ResMap, rootDir string) ([]renderedObject, error) {
out := make([]renderedObject, 0, resMap.Size())
for _, res := range resMap.Resources() {
m, err := res.Map()
if err != nil {
return nil, fmt.Errorf("reading rendered resource: %w", err)
}
obj := &unstructured.Unstructured{Object: m}
ro := renderedObject{
Object: obj,
OriginPath: originOf(obj, rootDir),
TransformedBy: transformationsOf(obj, rootDir),
}
unstructured.RemoveNestedField(obj.Object, "metadata", "annotations", originAnnotation)
unstructured.RemoveNestedField(obj.Object, "metadata", "annotations", transformationsAnnotation)
if len(obj.GetAnnotations()) == 0 {
unstructured.RemoveNestedField(obj.Object, "metadata", "annotations")
}
out = append(out, ro)
}
return out, nil
}

// originOf reads the source file an object was rendered from, normalised from
// render-root-relative ("../base/deployment.yaml") to scan-root-relative.
func originOf(obj *unstructured.Unstructured, rootDir string) string {
raw := obj.GetAnnotations()[originAnnotation]
if raw == "" {
return ""
}
var origin struct {
Path string `json:"path"`
}
if err := yaml.Unmarshal([]byte(raw), &origin); err != nil || origin.Path == "" {
return ""
}
return path.Clean(path.Join(rootDir, origin.Path))
}

// transformationsOf reads the ordered transformer chain kustomize applied, so the
// writer knows which kustomizations govern this object and in what order.
func transformationsOf(obj *unstructured.Unstructured, rootDir string) []transformation {
raw := obj.GetAnnotations()[transformationsAnnotation]
if raw == "" {
return nil
}
var entries []struct {
ConfiguredIn string `json:"configuredIn"`
ConfiguredBy struct {
Kind string `json:"kind"`
} `json:"configuredBy"`
}
if err := yaml.Unmarshal([]byte(raw), &entries); err != nil {
return nil
}
out := make([]transformation, 0, len(entries))
for _, e := range entries {
if e.ConfiguredIn == "" {
continue
}
out = append(out, transformation{
ConfiguredIn: path.Clean(path.Join(rootDir, e.ConfiguredIn)),
Kind: e.ConfiguredBy.Kind,
})
}
return out
}
Loading