diff --git a/Makefile b/Makefile index 260ef119..bfe3c7ba 100644 --- a/Makefile +++ b/Makefile @@ -1,4 +1,10 @@ -VERSION=$(shell git describe --tags) +# Full describe provenance for display: a clean exact tag yields the release +# version; any other state carries the describe suffix and/or -dirty marker, +# which releaseVersion in main.go classifies as a non-release build — so +# release-only behavior (the chart-drift checks) stays off for WIP builds, +# whose embedded charts are a moving target, while `talm --version` still +# identifies the build in bug reports. +VERSION=$(shell git describe --tags --dirty 2>/dev/null || echo dev) TALOS_VERSION=$(shell go list -m github.com/siderolabs/talos | awk '{sub(/^v/, "", $$NF); print $$NF}') build: diff --git a/README.md b/README.md index b82e1381..59aa039e 100644 --- a/README.md +++ b/README.md @@ -226,6 +226,59 @@ talm template -f nodes/node1.yaml -I > > `talm template -f node.yaml` (with or without `-I`) does **not** apply the same overlay: its output is the rendered template plus the modeline and the auto-generated warning, byte-identical to what the template alone would produce. Routing it through the patcher would drop every YAML comment (including the modeline) and re-sort keys, breaking downstream commands that read the file back. Use `apply --dry-run` if you want to preview the exact bytes that will be sent to the node. +## Keeping charts in sync after a binary upgrade + +`talm init` **vendors** its preset and library charts into the project directory — the preset templates plus a copy of the talm library chart under `charts/talm/`: + +```text +mycluster/ +├── Chart.yaml +├── values.yaml +├── .talm-preset.lock # pinned preset baseline — commit it (see "Preset drift") +├── templates/ # preset templates — you own and edit these +└── charts/ + └── talm/ # talm library chart — vendored from the binary + ├── Chart.yaml + └── templates/_helpers.tpl +``` + +Render commands (`template`, `apply`, `upgrade`) read this **local** copy, never the binary's built-in charts. That makes a project self-contained and reproducible — but it also means upgrading the `talm` binary does not touch `charts/talm/`. The vendored library stays frozen at whatever version last ran `init`, so a binary upgrade can leave you rendering with stale chart logic. + +Re-sync the vendored library with `talm init --update`: + +```bash +talm init --update --preset +``` + +This refreshes `charts/talm/` (always) and offers to update the preset templates (interactively, since you may have edited them). Your `values.yaml`, secrets, and node files are left untouched. + +To catch drift automatically, a release build compares the vendored `charts/talm/` against its own built-in copy on every config-loading command. The comparison is by **content**, not version number — re-vendoring after a binary bump that did not change the library is a no-op and raises no warning. When the content genuinely differs, talm prints a non-fatal warning to stderr (stdout and the exit code are unchanged): + +```text +WARN: project's vendored charts/talm/ library differs from the copy built into talm (modified: templates/_helpers.tpl); run `talm init --update --preset ` to re-sync (or ignore if this is intentional) +``` + +The remediation needs the preset name because `talm init --update` resolves the preset from `Chart.yaml`, which an init'd project does not record — pass `--preset ` (the one you ran `talm init` with) explicitly. + +Teams that want this enforced can turn the warning into a hard error (exit 1): set `strictCharts: true` in `Chart.yaml` so the whole team and CI inherit it, or pass `--strict-charts` for a single run. Strict mode applies to every config-loading command, including read-only ones such as `talm get` — run `talm init --update --preset `, or drop the flag / unset `strictCharts`, to unblock. Strict mode also escalates a check that cannot run at all — an unreadable `charts/talm/` or a corrupted `.talm-preset.lock` — into the same hard error, where the default behaviour degrades it to a `WARN: could not check drift` line: an unverifiable baseline passing silently would defeat the enforcement. A *missing* baseline (no `charts/talm/`, no `.talm-preset.lock`) blocks under strict for the same reason — deleting the baseline must not be a quieter bypass than corrupting it — while staying silent without strict, so projects generated before baseline pinning are not nagged. The check stays silent for `dev`/source builds, whose embedded charts are a moving target the developer controls. + +### Preset drift + +The vendored library (`charts/talm/`) is not the whole story. `talm init` also copies the **preset** — the `templates/` that render your machine config (sysctls, etcd args, the cozystack opinions) — into the project, and those you are *expected* to edit. That makes content comparison the wrong tool: it would flag every legitimate customization. So the preset is tracked differently. At `init` (and `init --update`) time talm pins the hash of the preset **as shipped** into `.talm-preset.lock`: + +```yaml +preset: cozystack +presetHash: +``` + +A release build then compares the binary's *current* preset hash against that pinned baseline — never against your edited `templates/` — so operator customizations are never reported as drift. When a newer binary ships changed preset defaults, the baseline no longer matches and talm warns: + +```text +WARN: project's cozystack preset differs from the copy built into talm ; run `talm init --update --preset cozystack` to pull the new preset defaults (your templates/ edits are preserved via the interactive diff) +``` + +`talm init --update --preset ` shows you an interactive diff of the new preset against your `templates/`, lets you merge what you want, and advances the baseline — which clears the warning even if you decline individual diffs to keep your customizations. `--strict-charts` / `strictCharts: true` escalate this to a hard error exactly as for the library. Projects with no `.talm-preset.lock` (generated before preset pinning) stay silent — there is no baseline to compare — unless strict mode is on, which treats a missing baseline as a blocker. Commit `.talm-preset.lock` so the baseline is shared across your team. + ## Apply with side-patches `talm apply -f` accepts a chain of files. The FIRST `-f` is the **anchor** — it must carry a `# talm: nodes=[…], templates=[…]` modeline and live under a `talm init`'d project (Chart.yaml + secrets.yaml). Any subsequent `-f` files are **side-patches**: they are merged in order on top of the anchor's rendered config, and a single `ApplyConfiguration` is issued per node carrying the composed result. diff --git a/charts/charts.go b/charts/charts.go index 1c663fd3..2592dd46 100644 --- a/charts/charts.go +++ b/charts/charts.go @@ -1,10 +1,14 @@ package charts import ( + "crypto/sha256" "embed" + "encoding/hex" + "fmt" "io/fs" "path" "regexp" + "sort" "strings" "github.com/cockroachdb/errors" @@ -12,14 +16,45 @@ import ( const presetGenericName = "generic" +// talmLibraryName is the directory of the talm library chart inside the +// embedded tree (and the name it is vendored under at charts/talm/ in a +// project). It is talm-owned — unlike the preset templates, which the +// operator edits — so it is the only tree the drift check compares. +const talmLibraryName = "talm" + +// chartYamlName is the conventional Helm chart metadata filename. +const chartYamlName = "Chart.yaml" + //go:embed all:cozystack all:generic all:talm var embeddedCharts embed.FS +// chartMetaRegex matches the TOP-LEVEL `name:`/`version:` metadata lines of +// a Chart.yaml. Both are rewritten to a %s placeholder so the init flow can +// substitute the real project name/version, and so the drift check can +// compare two chart trees without a version stamp counting as content. The +// `(?m)^` anchor keeps nested occurrences (dependencies[].name/version, +// maintainers[].name) intact: rewriting those would mask real dependency +// drift and inject extra verbs into the two-argument fmt template the init +// flow writes Chart.yaml with. +var chartMetaRegex = regexp.MustCompile(`(?m)^(name|version): \S+`) + +// NormalizeChartMeta rewrites the name/version lines of a Chart.yaml to %s +// placeholders. Files other than Chart.yaml pass through unchanged. base is +// the file's base name (e.g. path.Base(p)). Keeping a single normalizer +// means the init-time substitution and the content-drift comparison treat +// chart metadata identically. +func NormalizeChartMeta(base, content string) string { + if base != chartYamlName { + return content + } + + return chartMetaRegex.ReplaceAllString(content, "$1: %s") +} + // PresetFiles returns a map of file paths to their contents. // For Chart.yaml files, name and version are replaced with %s placeholders. func PresetFiles() (map[string]string, error) { filesMap := make(map[string]string) - regex := regexp.MustCompile(`(name|version): \S+`) err := fs.WalkDir(embeddedCharts, ".", func(filePath string, entry fs.DirEntry, err error) error { if err != nil { @@ -47,12 +82,8 @@ func PresetFiles() (map[string]string, error) { return errors.Wrapf(err, "reading embedded chart file %q", filePath) } - content := string(data) - - // For Chart.yaml files, replace name and version with %s - if path.Base(filePath) == "Chart.yaml" { - content = regex.ReplaceAllString(content, "$1: %s") - } + // For Chart.yaml files, replace name and version with %s. + content := NormalizeChartMeta(path.Base(filePath), string(data)) // Use the file path as-is (relative to charts directory) filesMap[filePath] = content @@ -66,6 +97,67 @@ func PresetFiles() (map[string]string, error) { return filesMap, nil } +// TalmLibraryFiles returns the embedded talm library chart keyed by path +// relative to the talm/ root (e.g. "Chart.yaml", "templates/_helpers.tpl"), +// so the keys line up with a project's vendored charts/talm/ tree. Chart.yaml +// metadata is normalized via NormalizeChartMeta, so a version stamp is not +// treated as content. It is the embedded counterpart compared against the +// vendored copy to surface chart drift after a binary upgrade. +func TalmLibraryFiles() (map[string]string, error) { + filesMap := make(map[string]string) + + err := fs.WalkDir(embeddedCharts, talmLibraryName, func(filePath string, entry fs.DirEntry, err error) error { + if err != nil { + return errors.Wrapf(err, "walking embedded talm library at %q", filePath) + } + + if entry.IsDir() { + return nil + } + + data, err := embeddedCharts.ReadFile(filePath) + if err != nil { + return errors.Wrapf(err, "reading embedded talm file %q", filePath) + } + + // Strip the talm/ prefix so keys match a vendored charts/talm/ tree. + rel := strings.TrimPrefix(filePath, talmLibraryName+"/") + filesMap[rel] = NormalizeChartMeta(path.Base(filePath), string(data)) + + return nil + }) + if err != nil { + return nil, errors.Wrap(err, "collecting embedded talm library") + } + + return filesMap, nil +} + +// HashChartFiles returns a deterministic digest of a chart tree described as +// a path→content map. The digest folds in the sorted set of (path, content) +// pairs and is independent of map iteration order. Each path and content is +// length-prefixed so distinct trees cannot collide by a fortunate alignment +// of concatenated bytes. Two trees hash equal iff they carry the same files +// with the same bytes — the signal the drift check relies on. +func HashChartFiles(files map[string]string) string { + paths := make([]string, 0, len(files)) + for p := range files { + paths = append(paths, p) + } + + sort.Strings(paths) + + hasher := sha256.New() + for _, p := range paths { + // Length-prefix both the path and its content so the boundary + // between them (and between successive entries) is unambiguous. + fmt.Fprintf(hasher, "%d:%s%d:", len(p), p, len(files[p])) + hasher.Write([]byte(files[p])) + } + + return hex.EncodeToString(hasher.Sum(nil)) +} + // AvailablePresets returns a list of available preset chart names. // The presetGenericName preset is always first if it exists. func AvailablePresets() ([]string, error) { diff --git a/charts/library_test.go b/charts/library_test.go new file mode 100644 index 00000000..2bab66e7 --- /dev/null +++ b/charts/library_test.go @@ -0,0 +1,200 @@ +package charts + +import ( + "strings" + "testing" +) + +// TestTalmLibraryFiles_NormalizesAndStripsPrefix pins the embedded talm +// library collector. Keys must be relative to the talm/ root (so they +// line up with a project's vendored charts/talm/ tree), the library +// Chart.yaml must come back with its name/version normalized to %s (so a +// version stamp never counts as content), and the helpers template must +// be present verbatim. +func TestTalmLibraryFiles_NormalizesAndStripsPrefix(t *testing.T) { + files, err := TalmLibraryFiles() + if err != nil { + t.Fatalf("TalmLibraryFiles: %v", err) + } + + if len(files) == 0 { + t.Fatal("TalmLibraryFiles returned empty map; embedded talm library is missing") + } + + for path := range files { + if strings.HasPrefix(path, "talm/") { + t.Errorf("key %q still carries the talm/ prefix; it would not match a vendored charts/talm/ relative path", path) + } + } + + chart, ok := files["Chart.yaml"] + if !ok { + t.Fatal("expected Chart.yaml entry keyed relative to the talm/ root") + } + if !strings.Contains(chart, "version: %s") { + t.Errorf("library Chart.yaml not normalized; a version stamp would be treated as content drift:\n%s", chart) + } + + if _, ok := files["templates/_helpers.tpl"]; !ok { + t.Error("expected templates/_helpers.tpl in the embedded talm library") + } +} + +// TestHashChartFiles_OrderIndependent pins that the digest depends on the +// set of (path, content) pairs, not on map iteration order. Go map order +// is randomized, so a digest that folded order in would be non- +// deterministic across runs and falsely report drift. +func TestHashChartFiles_OrderIndependent(t *testing.T) { + a := map[string]string{ + "Chart.yaml": "name: %s\nversion: %s\n", + "templates/_helpers.tpl": "{{- define \"x\" -}}{{- end -}}", + } + b := map[string]string{ + "templates/_helpers.tpl": "{{- define \"x\" -}}{{- end -}}", + "Chart.yaml": "name: %s\nversion: %s\n", + } + + if HashChartFiles(a) != HashChartFiles(b) { + t.Error("digest depends on map order; it must be deterministic over the (path, content) set") + } +} + +// TestHashChartFiles_ContentSensitive pins that any change to a file's +// content changes the digest. This is the signal the drift check relies +// on: a stale helpers template must hash differently from a fresh one. +func TestHashChartFiles_ContentSensitive(t *testing.T) { + base := map[string]string{"templates/_helpers.tpl": "old"} + changed := map[string]string{"templates/_helpers.tpl": "new"} + + if HashChartFiles(base) == HashChartFiles(changed) { + t.Error("digest is insensitive to content; real chart drift would go undetected") + } +} + +// TestHashChartFiles_PathBoundaryUnambiguous guards the length-prefixed +// framing: two different file trees must not collide just because their +// concatenated path+content bytes happen to line up. Without framing, +// {"ab": "c"} and {"a": "bc"} would hash identically. +func TestHashChartFiles_PathBoundaryUnambiguous(t *testing.T) { + left := map[string]string{"ab": "c"} + right := map[string]string{"a": "bc"} + + if HashChartFiles(left) == HashChartFiles(right) { + t.Error("path/content boundary is ambiguous; distinct trees collide to the same digest") + } +} + +// TestNormalizeChartMeta_VersionStampDoesNotAffectHash is the core +// correctness guard for the whole drift feature: two library trees that +// differ ONLY in the Chart.yaml version stamp must hash identically once +// normalized. A binary version bump that left charts/talm/ byte-identical +// must not be reported as drift — that false positive is exactly what the +// version-number comparison approach got wrong. +func TestNormalizeChartMeta_VersionStampDoesNotAffectHash(t *testing.T) { + old := map[string]string{ + "Chart.yaml": NormalizeChartMeta("Chart.yaml", "name: talm\nversion: 0.27.0\ntype: library\n"), + "templates/_helpers.tpl": "{{- define \"x\" -}}{{- end -}}", + } + fresh := map[string]string{ + "Chart.yaml": NormalizeChartMeta("Chart.yaml", "name: talm\nversion: 0.30.0\ntype: library\n"), + "templates/_helpers.tpl": "{{- define \"x\" -}}{{- end -}}", + } + + if HashChartFiles(old) != HashChartFiles(fresh) { + t.Error("version-only difference changed the digest; a pure version bump would raise a false drift warning") + } +} + +// TestNormalizeChartMeta_LeavesNonChartYamlUntouched pins that the +// normalizer only rewrites Chart.yaml. A helpers template that happens to +// contain a `version:` line (e.g. inside a rendered manifest snippet) must +// pass through unchanged, or genuine drift in that template would be +// masked. +func TestNormalizeChartMeta_LeavesNonChartYamlUntouched(t *testing.T) { + const tpl = "version: 1.2.3 # part of a rendered example, not chart metadata" + if got := NormalizeChartMeta("_helpers.tpl", tpl); got != tpl { + t.Errorf("NormalizeChartMeta rewrote a non-Chart.yaml file: %q", got) + } +} + +// TestEmbeddedCharts_NoCRLF makes the LF-only invariant of the embedded +// tree explicit. The drift comparison normalizes CRLF on the vendored side +// only, relying on .gitattributes (`* text=auto eol=lf`) to keep the +// embedded side LF on every checkout; an embedded file that ever ships +// CRLF would drift against every project forever, and `init --update` +// could not clear it (the vendored copy normalizes to LF on read). +func TestEmbeddedCharts_NoCRLF(t *testing.T) { + for name, files := range map[string]func() (map[string]string, error){ + "PresetFiles": PresetFiles, + "TalmLibraryFiles": TalmLibraryFiles, + } { + all, err := files() + if err != nil { + t.Fatalf("%s: %v", name, err) + } + + for filePath, content := range all { + if strings.Contains(content, "\r\n") { + t.Errorf("%s: embedded %s contains CRLF; the drift check assumes an LF-only embedded tree (enforced by .gitattributes)", name, filePath) + } + } + } +} + +// TestNormalizeChartMeta_PreservesNestedNameVersion pins that only the +// TOP-LEVEL name/version metadata lines are folded to placeholders. Nested +// occurrences — dependencies[].name/version, maintainers[].name — must +// survive verbatim: rewriting them would (a) mask real dependency drift in +// the content comparison and (b) inject extra %s verbs into the fmt +// template the init flow renders with exactly two arguments, writing +// %!s(MISSING) into the project's Chart.yaml. +func TestNormalizeChartMeta_PreservesNestedNameVersion(t *testing.T) { + const chart = "apiVersion: v2\n" + + "name: cozystack\n" + + "version: 0.1.0\n" + + "dependencies:\n" + + " - name: talm\n" + + " version: 1.2.3\n" + + "maintainers:\n" + + " - name: maintainer-handle\n" + + got := NormalizeChartMeta("Chart.yaml", chart) + + if !strings.Contains(got, "name: %s\nversion: %s\n") { + t.Errorf("top-level name/version not normalized:\n%s", got) + } + + if !strings.Contains(got, " - name: talm\n version: 1.2.3\n") { + t.Errorf("nested dependency name/version were rewritten:\n%s", got) + } + + if !strings.Contains(got, " - name: maintainer-handle\n") { + t.Errorf("nested maintainer name was rewritten:\n%s", got) + } + + if n := strings.Count(got, "%s"); n != 2 { + t.Errorf("expected exactly 2 placeholders for the init-time fmt template, got %d:\n%s", n, got) + } +} + +// TestNormalizeChartMeta_PreservesApiAndAppVersion pins that only the +// `name`/`version` metadata is folded to a placeholder. The camelCase +// `apiVersion`/`appVersion` keys must survive verbatim — otherwise a real +// change to those fields would be normalized away and hidden from the drift +// comparison. Guards against a future regex tweak (e.g. adding `(?i)`) that +// would start eating them. +func TestNormalizeChartMeta_PreservesApiAndAppVersion(t *testing.T) { + const chart = "apiVersion: v2\nname: talm\nversion: 0.1.0\nappVersion: 1.30.0\ntype: library\n" + + got := NormalizeChartMeta("Chart.yaml", chart) + + if !strings.Contains(got, "apiVersion: v2") { + t.Errorf("apiVersion was rewritten:\n%s", got) + } + if !strings.Contains(got, "appVersion: 1.30.0") { + t.Errorf("appVersion was rewritten:\n%s", got) + } + if !strings.Contains(got, "name: %s") || !strings.Contains(got, "version: %s") { + t.Errorf("name/version not normalized:\n%s", got) + } +} diff --git a/docs/manual-test-plan.md b/docs/manual-test-plan.md index 8003e820..2384d161 100644 --- a/docs/manual-test-plan.md +++ b/docs/manual-test-plan.md @@ -222,11 +222,98 @@ Expected: `Updated.` on stdout. The rendered body replaces the previous body of Regression anchor: write `nodes/node0.yaml` as `# Operator note A\n# Operator note B\n# talm: ...\n`. After `template -I -f nodes/node0.yaml`, the first two lines (`# Operator note A`, `# Operator note B`) MUST still be there, followed by the modeline, the talm-rendered warning header, then the body. Re-run idempotent: a second `template -I` keeps the same prefix structure — leading comments don't drift, multiply, or disappear. -### B5. Render with stale chart preset +### B5. Render with stale chart preset (chart-drift detection) -When the local `charts/talm/` is older than the talm binary's embedded preset, `talm template` succeeds against the local preset — it does NOT auto-bump. The operator must run `init --update`. Confirm by inspecting `talm version` against `Chart.yaml`. +`talm` vendors its library chart into `charts/talm/` at `init` time; render commands read that local copy, never the binary's embedded charts. Upgrading the binary therefore leaves `charts/talm/` frozen at the version that last ran `init`. A release build surfaces this drift by **content** — the `Chart.yaml` version stamp is normalized away, so a pure version bump is never flagged. -**Regression anchor**: `template -I` is rewrite, not merge — verify by adding a `# my comment` line above the modeline in `nodes/node0.yaml`, running B4, and confirming the comment is GONE in the new body. If the comment survives, a behaviour change shipped (could be either an intentional new `--preserve-comments` flag or an undocumented merge mode — neither should appear silently). +Simulate drift by editing the vendored library, then render: + +```bash +printf '\n{{- /* stale edit */ -}}\n' >> charts/talm/templates/_helpers.tpl +talm template -f nodes/node0.yaml --offline 2>&1 >/dev/null | grep '^WARN:' +``` + +Expected (default): a single line on **stderr** — `WARN: project's vendored charts/talm/ library differs from the copy built into talm (modified: templates/_helpers.tpl); run talm init --update --preset to re-sync ...` — while stdout (the rendered config) and the exit code are unchanged. The parenthesised sample names up to 5 differing paths (`modified:` / `extra:` / `missing:`) so the cause is locatable without a manual tree diff. Clear it and re-confirm silence: + +```bash +talm init --update --preset cozystack --force +talm template -f nodes/node0.yaml --offline 2>&1 >/dev/null | grep '^WARN:' && echo "FAIL: drift not cleared" || echo "OK: drift cleared" +``` + +The `--preset` is required: `talm init --update` alone resolves the preset from `Chart.yaml`, which an init'd project does not record, so it errors with "preset not found in Chart.yaml dependencies" (pinned by A8) and never re-vendors. Expected: `OK: drift cleared`. + +`init --update` re-syncs `charts/talm/` exactly: files the embedded library does not ship (an `extra:` path in the WARN — e.g. `.DS_Store`, an editor backup, a file a newer release dropped) are pruned with a `Removed charts/talm/` line. Verify by dropping a stray file in `charts/talm/`, observing the `extra:` WARN, and re-running the clear step above (pinned by `TestInitUpdate_PrunesExtraneousVendoredFiles`, `TestCheckChartDrift_ExtraneousFile_DriftNamesPath`). + +### B5a. Render with stale preset templates (preset-drift detection) + +Unlike the library, the preset `templates/` are operator-editable, so preset drift is tracked by a pinned baseline hash in `.talm-preset.lock` (written at `init`), NOT by content-comparing the live `templates/`. A release build compares the binary's current preset hash against that baseline. + +First confirm a fresh project is silent AND that editing `templates/` does NOT trigger a false positive: + +```bash +cat .talm-preset.lock # preset: + presetHash: +printf '\n# operator customization\n' >> templates/_helpers.tpl +talm template -f nodes/node0.yaml --offline 2>&1 >/dev/null | grep '^WARN:.*preset' && echo "FAIL: operator edit misreported as drift" || echo "OK: operator edits are not drift" +``` + +Expected: `OK: operator edits are not drift` — the baseline pins the pristine preset, the edited `templates/` is never read. + +Now simulate a newer binary shipping a changed preset by corrupting the pinned baseline, then render: + +```bash +sed -i.bak 's/^presetHash:.*/presetHash: 0000000000000000000000000000000000000000000000000000000000000000/' .talm-preset.lock +talm template -f nodes/node0.yaml --offline 2>&1 >/dev/null | grep '^WARN:.*preset' +``` + +Expected (default): a single line on **stderr** — `WARN: project's preset differs from the copy built into talm ; run talm init --update --preset to pull the new preset defaults ...` — stdout and exit code unchanged. Under `--strict-charts` (or `strictCharts: true`) the same mismatch is a hard error (exit 1) raised before the command body. Clear it: + +```bash +talm init --update --preset --force +talm template -f nodes/node0.yaml --offline 2>&1 >/dev/null | grep '^WARN:.*preset' && echo "FAIL: preset drift not cleared" || echo "OK: preset drift cleared" +``` + +`init --update` rewrites `.talm-preset.lock` to the current baseline, so the warning clears even if you declined individual preset-file diffs. Expected: `OK: preset drift cleared`. Note: `--force` auto-accepts every preset-file diff, overwriting the operator edit made earlier in this scenario — the "edits are preserved via the interactive diff" promise holds only without `--force`; this test stand is disposable, a real project should run the clear step interactively. + +Regression anchors: `TestPresetLock_RoundTrip_NoDrift`, `TestCheckPresetDrift_StaleBaseline_Drift`, `TestCheckPresetDrift_OperatorEditedTemplates_NoDrift` (the false-positive guard), `TestCheckPresetDrift_NoLock_NoBaselineSentinel`, and `TestEvaluatePresetDrift` pin every branch above. + +Strict mode turns the same drift into a hard error (exit 1) raised before the command body runs. Opt in per project via `strictCharts: true` in `Chart.yaml`, or per invocation via `--strict-charts`: + +```bash +talm template -f nodes/node0.yaml --offline --strict-charts; echo "exit=$?" +``` + +Expected: exit 1, the drift message plus a hint pointing at `talm init --update --preset `, and no rendered output — the command body never runs (no modeline/render errors follow the hint). + +### B5b. Drift check failure: warn by default, block under strict + +A check that cannot run at all (corrupted `.talm-preset.lock`, unreadable `charts/talm/`) degrades to a non-fatal warning by default, but under strict mode it is a hard error — an unverifiable baseline silently passing would defeat the enforcement the team opted into: + +```bash +printf 'preset: [unclosed\n' > .talm-preset.lock +talm template -f nodes/node0.yaml --offline 2>&1 >/dev/null | grep '^WARN: could not check drift'; echo "exit=$?" +talm template -f nodes/node0.yaml --offline --strict-charts >/dev/null; echo "exit=$?" +talm init --update --preset --force # restore a valid lock +``` + +Expected: the first render prints `WARN: could not check drift: ...` and exits 0; the `--strict-charts` render fails (exit 1) with `drift check failed under strict mode` and a hint to repair the baseline; `init --update` rewrites a valid lock. Regression anchors: the error-path subtests of `TestEvaluateChartDrift` / `TestEvaluatePresetDrift`. + +A *missing* baseline follows the same split — silent by default (projects generated before baseline pinning are not nagged), blocked under strict (deleting the baseline must not bypass enforcement): + +```bash +rm .talm-preset.lock +talm template -f nodes/node0.yaml --offline 2>&1 >/dev/null | grep '^WARN:'; echo "exit=$?" # no WARN, exit 0 from talm +talm template -f nodes/node0.yaml --offline --strict-charts >/dev/null; echo "exit=$?" # exit 1: drift baseline missing under strict mode +talm init --update --preset --force # re-pin the baseline +``` + +Regression anchors: the missing-baseline subtests of `TestEvaluateChartDrift` / `TestEvaluatePresetDrift`, `TestCheckChartDrift_MissingVendoredDir_NoBaselineSentinel`, `TestCheckPresetDrift_NoLock_NoBaselineSentinel`. + +No-false-alarm checks — each MUST stay silent (no `WARN:` line): + +- A project vendored by an older release, run under a newer binary whose `charts/talm/` is byte-identical: the version stamp differs but the content does not. +- A `dev`/source build: its embedded charts are a moving target the developer controls, so the check is a no-op. +- A freshly synced project (`talm init --update --preset `) under the matching binary. +- A vendored `charts/talm/` whose files carry CRLF line endings (a Windows clone with `core.autocrlf=true`): line endings are checkout artifacts, not content (pinned by `TestCheckChartDrift_CRLFVendoredCopy_NoDrift`). ### B6. Scope-filter symmetry across v1.11 and v1.12 renders diff --git a/main.go b/main.go index 86d9299d..4f2ed927 100644 --- a/main.go +++ b/main.go @@ -5,6 +5,7 @@ import ( "fmt" "os" "path/filepath" + "regexp" "slices" "strings" "time" @@ -44,12 +45,28 @@ const ( // command's Use field and via -X main.cmdNameTalm in build tooling. const cmdNameTalm = "talm" -// Version is the talm release tag baked at build time via ldflags -// (`-X main.Version=v0.27.0`); the literal value here is the local -// development fallback. +// Version is the talm build version baked in at link time via ldflags +// (`-X main.Version=...`). The two release build paths inject different +// forms: goreleaser strips the leading "v" (e.g. `0.27.0`, from +// `{{.Version}}`) while the Makefile's `git describe --tags` keeps it +// (e.g. `v0.27.0`). releaseVersion normalizes both. The literal "dev" +// here is the local source-build fallback. // //nolint:gochecknoglobals // ldflags-injected build version, idiomatic for go release tooling. -var Version = "dev" +var Version = devVersion + +// devVersion is the build-version sentinel for a local source build (no +// ldflags injection). releaseVersion treats it as "not a release", so +// release-only behavior such as the chart-drift check stays off. +const devVersion = "dev" + +// strictChartsFlag is bound to the --strict-charts persistent flag. When set +// (or when Chart.yaml carries strictCharts: true), a content difference +// between the project's vendored charts/talm/ and the binary's built-in copy +// becomes a hard error instead of a warning. +// +//nolint:gochecknoglobals // cobra persistent flag binds to package-level state, consistent with the rest of this file. +var strictChartsFlag bool // skipConfigCommands lists commands that should not load Chart.yaml config. // - init: creates the config, so it doesn't exist yet @@ -121,6 +138,11 @@ func registerRootFlags(cmd *cobra.Command) { cmd.PersistentFlags().StringVar(&commands.GlobalArgs.Cluster, "cluster", "", "Cluster to connect to if a proxy endpoint is used.") cmd.PersistentFlags().BoolVar(&commands.GlobalArgs.SkipVerify, "skip-verify", false, "skip TLS certificate verification (keeps client authentication)") cmd.PersistentFlags().Bool("version", false, "Print the version number of the application") + // No backticks in this usage string: pflag's UnquoteUsage treats the + // first backtick-quoted word as the flag's value-placeholder name, which + // on a bool flag misrenders --help as `--strict-charts talm init --update` + // (as if it took an argument). + cmd.PersistentFlags().BoolVar(&strictChartsFlag, "strict-charts", false, "fail if the project's vendored charts/talm/ or pinned preset baseline differs from the talm binary (run talm init --update --preset to re-sync)") // Shell completion for root persistent flags. --nodes / // --endpoints draw from the in-scope talosconfig contexts. @@ -185,6 +207,10 @@ func init() { if err != nil { return errors.Wrap(err, "error loading configuration") } + + if err := surfaceChartDrift(); err != nil { + return err + } } // Ensure talosconfig path is set to project root if not explicitly set via flag @@ -219,6 +245,151 @@ func init() { } } +// describeSuffixRegex matches the suffixes `git describe --tags --dirty` +// appends for a non-release tree: "--g" on a non-tag commit +// and/or "-dirty" for local edits — including bare "-dirty" on an exact tag, +// where the edits may be to the embedded charts themselves. A version +// carrying either is a developer's WIP tree, not a release. +var describeSuffixRegex = regexp.MustCompile(`(-\d+-g[0-9a-f]+)?-dirty$|-\d+-g[0-9a-f]+$`) + +// releaseVersion interprets the ldflags-injected build version. It returns +// the version with any leading "v" stripped and true for a tagged release +// build, or ("", false) for a dev/source build. Both release build paths +// must be accepted: goreleaser injects "0.30.0" (no "v") and the Makefile +// injects "v0.30.0" on an exact tag. Gating on the "v" prefix alone would +// silently disable release-only behavior on the goreleaser artifacts users +// actually download. +// +// A `git describe` suffix ("v0.29.0-5-gabc1234") marks a build from a +// non-tag commit: its embedded charts are a moving target the developer +// controls, so release-only behavior (the drift checks) must stay off — +// otherwise every contributor build raises false drift in any real project +// and hard-fails strict ones. The Makefile emits "dev" off-tag, but the +// parser rejects the describe shape regardless of how it was injected. +func releaseVersion(raw string) (string, bool) { + if raw == "" || raw == devVersion { + return "", false + } + + if describeSuffixRegex.MatchString(raw) { + return "", false + } + + return strings.TrimPrefix(raw, "v"), true +} + +// evaluateChartDrift decides the drift outcome for a build. It returns +// (warning, error): a non-empty warning to print to stderr, or a non-nil +// error to abort the command (strict mode), or both empty for the silent +// cases. Taking the version, project root, and strict flag as arguments +// keeps the warn-vs-fail decision pure (modulo the filesystem read inside +// CheckChartDrift) so it is unit-testable without the package globals. +// +// Cases: dev/source build → silent (embedded charts are a moving target the +// developer controls); drift-check I/O error → non-fatal warning (best +// effort, never blocks the command); drift + strict → hard error with a +// remediation hint; drift + non-strict → warning; no drift → silent. +func evaluateChartDrift(rawVersion, rootDir string, strict bool) (string, error) { + version, ok := releaseVersion(rawVersion) + if !ok { + return "", nil + } + + drift, msg, err := commands.CheckChartDrift(rootDir, version) + + return decideDrift(drift, msg, err, strict) +} + +// evaluatePresetDrift is the preset-template counterpart of +// evaluateChartDrift. Same release-only gating and warn-vs-fail decision, but +// it consults CheckPresetDrift: the binary's preset hash vs the baseline +// pinned in .talm-preset.lock at init. Kept separate (rather than folded into +// evaluateChartDrift) so the library and preset drift signals stay +// independently testable and independently silenceable. +func evaluatePresetDrift(rawVersion, rootDir string, strict bool) (string, error) { + version, ok := releaseVersion(rawVersion) + if !ok { + return "", nil + } + + drift, msg, err := commands.CheckPresetDrift(rootDir, version) + + return decideDrift(drift, msg, err, strict) +} + +// decideDrift folds a (drift, msg, err) result into the (warning, error) +// outcome shared by both drift checks: a check failure downgrades to a +// non-fatal warning (best effort, never blocks a command) — except under +// strict, where an unverifiable baseline is a hard error: the operator opted +// into enforcement, and a corrupted lock or unreadable vendored tree passing +// silently would defeat it exactly when the baseline broke; a MISSING +// baseline (commands.ErrNoBaseline) is silence without strict — projects +// from before baseline pinning should not nag — but a blocker under strict, +// where deleting the baseline must not pass more quietly than corrupting +// it; drift under strict is a hard error with a remediation hint; drift +// otherwise is a warning; no drift is silent. +func decideDrift(drift bool, msg string, err error, strict bool) (string, error) { + switch { + case errors.Is(err, commands.ErrNoBaseline) && !strict: + return "", nil + case errors.Is(err, commands.ErrNoBaseline): + //nolint:wrapcheck // cockroachdb/errors.WithHint at boundary; project idiom. + return "", errors.WithHint( + errors.Wrap(err, "drift baseline missing under strict mode"), + "run `talm init --update --preset ` to vendor the library and pin the preset baseline, or unset strictCharts / drop --strict-charts", + ) + case err != nil && strict: + //nolint:wrapcheck // cockroachdb/errors.WithHint at boundary; project idiom. + return "", errors.WithHint( + errors.Wrap(err, "drift check failed under strict mode"), + "repair the baseline (re-run `talm init --update --preset `), or unset strictCharts / drop --strict-charts to downgrade this to a warning", + ) + case err != nil: + return fmt.Sprintf("could not check drift: %v", err), nil + case drift && strict: + // The shared drift message ends with "(or ignore if this is + // intentional)" — sound advice on a warning, contradictory on an + // error the command just refused to run past. Strip it here + // rather than threading a second message through both checkers. + msg = strings.Replace(msg, " (or ignore if this is intentional)", "", 1) + + //nolint:wrapcheck // originating error built with errors.New; WithHint adds operator-facing guidance and is the project idiom. + return "", errors.WithHint( + errors.New(msg), + "run `talm init --update --preset `, or unset strictCharts / drop --strict-charts to downgrade this to a warning", + ) + case drift: + return msg, nil + default: + return "", nil + } +} + +// surfaceChartDrift wires the drift evaluators to the package globals and +// emits any warning to stderr. The strict input is the OR of the committed +// Chart.yaml field and the per-run flag. Both the vendored library +// (charts/talm/) and the preset baseline (.talm-preset.lock) are checked; a +// strict failure on either aborts before the command body runs. +func surfaceChartDrift() error { + strict := commands.Config.StrictCharts || strictChartsFlag + + for _, eval := range []func(string, string, bool) (string, error){ + evaluateChartDrift, + evaluatePresetDrift, + } { + warning, err := eval(Version, commands.Config.RootDir, strict) + if err != nil { + return err + } + + if warning != "" { + fmt.Fprintf(os.Stderr, "WARN: %s\n", warning) + } + } + + return nil +} + func initConfig() { if len(os.Args) < 2 { return @@ -236,8 +407,12 @@ func initConfig() { } if strings.HasPrefix(cmd.Use, initSubcommandName) { - if strings.HasPrefix(Version, "v") { - commands.Config.InitOptions.Version = strings.TrimPrefix(Version, `v`) + // Stamp the real release version into the vendored charts; fall back + // to the dev sentinel for source builds. Gating on the "v" prefix + // here would stamp "0.1.0" on every goreleaser release (which injects + // the version without "v"). + if version, ok := releaseVersion(Version); ok { + commands.Config.InitOptions.Version = version } else { commands.Config.InitOptions.Version = "0.1.0" } diff --git a/main_test.go b/main_test.go index d9031a86..2a93cbf6 100644 --- a/main_test.go +++ b/main_test.go @@ -8,7 +8,9 @@ import ( "github.com/cockroachdb/errors" "github.com/cozystack/talm/pkg/commands" + "github.com/cozystack/talm/pkg/generated" "github.com/spf13/cobra" + "github.com/spf13/pflag" ) // buildCommandHierarchy creates a cobra command hierarchy from a path like @@ -182,6 +184,83 @@ func TestLoadConfig_EmptyApplyTimeoutResolvesDefault(t *testing.T) { } } +// TestLoadConfig_StrictChartsParses pins the Chart.yaml → Config channel +// of strict enforcement: `strictCharts: true` must land in +// commands.Config.StrictCharts. This is the team/CI-wide form the README +// recommends; a yaml-tag typo would silently disable it while the +// per-invocation --strict-charts flag kept working, and no other test +// would notice. +func TestLoadConfig_StrictChartsParses(t *testing.T) { + dir := t.TempDir() + chartPath := filepath.Join(dir, "Chart.yaml") + body := "apiVersion: v2\nname: test\nversion: 0.1.0\nstrictCharts: true\n" + if err := os.WriteFile(chartPath, []byte(body), 0o644); err != nil { + t.Fatalf("write Chart.yaml: %v", err) + } + + snapshotConfigState(t) + + if err := loadConfig(chartPath); err != nil { + t.Fatalf("loadConfig: %v", err) + } + + if !commands.Config.StrictCharts { + t.Error("strictCharts: true in Chart.yaml did not set Config.StrictCharts; the committed enforcement channel is broken") + } +} + +// TestSurfaceChartDrift_StrictSources pins the OR between the two strict +// opt-ins consumed by surfaceChartDrift: the committed Chart.yaml field +// (Config.StrictCharts) and the per-invocation --strict-charts flag. Either +// alone must block on a drifted project; with both off the same project +// must only warn. +func TestSurfaceChartDrift_StrictSources(t *testing.T) { + snapshot := func(t *testing.T, root string) { + t.Helper() + snapshotConfigState(t) + + savedVersion := Version + savedFlag := strictChartsFlag + t.Cleanup(func() { + Version = savedVersion + strictChartsFlag = savedFlag + }) + + Version = "0.30.0" + commands.Config.RootDir = root + } + + t.Run("Chart.yaml strictCharts alone blocks", func(t *testing.T) { + snapshot(t, writeDriftedTalmProject(t)) + commands.Config.StrictCharts = true + strictChartsFlag = false + + if err := surfaceChartDrift(); err == nil { + t.Error("strictCharts: true from Chart.yaml must block on drift without the flag") + } + }) + + t.Run("--strict-charts flag alone blocks", func(t *testing.T) { + snapshot(t, writeDriftedTalmProject(t)) + commands.Config.StrictCharts = false + strictChartsFlag = true + + if err := surfaceChartDrift(); err == nil { + t.Error("--strict-charts must block on drift without the Chart.yaml field") + } + }) + + t.Run("neither source set only warns", func(t *testing.T) { + snapshot(t, writeDriftedTalmProject(t)) + commands.Config.StrictCharts = false + strictChartsFlag = false + + if err := surfaceChartDrift(); err != nil { + t.Errorf("drift without strict opt-in must warn, not block: %v", err) + } + }) +} + // TestRegisterRootFlags_NodesHasNoShorthand pins that talm's // root `--nodes` does NOT claim the `-n` shorthand. With `-n` // registered as the alias for `--nodes`, the global captures any @@ -217,6 +296,343 @@ func TestRegisterRootFlags_NodesHasNoShorthand(t *testing.T) { } } +// TestRegisterRootFlags_StrictChartsRendersWithoutValuePlaceholder pins that +// --strict-charts renders in --help as a plain bool flag, not as one taking +// an argument. pflag's UnquoteUsage treats the first backtick-quoted word in +// a flag's usage string as the flag's value-placeholder name; a backtick in +// the --strict-charts usage made --help show `--strict-charts talm init +// --update`, as if the bool flag took a string argument. +func TestRegisterRootFlags_StrictChartsRendersWithoutValuePlaceholder(t *testing.T) { + snapshotConfigState(t) + + cmd := &cobra.Command{Use: "talm-test"} + registerRootFlags(cmd) + + flag := cmd.PersistentFlags().Lookup("strict-charts") + if flag == nil { + t.Fatal("expected --strict-charts to be registered, got nil") + } + + name, _ := pflag.UnquoteUsage(flag) + if name != "" { + t.Errorf("--strict-charts (a bool flag) renders with value placeholder %q; remove backticks from its usage string", name) + } +} + +// writeDriftedTalmProject creates a project whose vendored charts/talm/ +// cannot match the binary's embedded library (the helpers template carries +// a local edit), so CheckChartDrift reports drift. +func writeDriftedTalmProject(t *testing.T) string { + t.Helper() + + root := t.TempDir() + dir := filepath.Join(root, "charts", "talm", "templates") + if err := os.MkdirAll(dir, 0o755); err != nil { + t.Fatalf("mkdir: %v", err) + } + if err := os.WriteFile(filepath.Join(root, "charts", "talm", "Chart.yaml"), + []byte("apiVersion: v2\nname: talm\nversion: 0.30.0\ntype: library\n"), 0o644); err != nil { + t.Fatalf("write Chart.yaml: %v", err) + } + if err := os.WriteFile(filepath.Join(dir, "_helpers.tpl"), + []byte("{{- /* drifted local edit, not the embedded copy */ -}}\n"), 0o644); err != nil { + t.Fatalf("write helpers: %v", err) + } + + return root +} + +// writeMatchingTalmProject materializes a vendored charts/talm/ that is +// byte-identical to the embedded library (stamping version into Chart.yaml), +// so CheckChartDrift reports no drift. +func writeMatchingTalmProject(t *testing.T, version string) string { + t.Helper() + + files, err := generated.TalmLibraryFiles() + if err != nil { + t.Fatalf("TalmLibraryFiles: %v", err) + } + + root := t.TempDir() + for rel, content := range files { + if filepath.Base(rel) == "Chart.yaml" { + content = strings.ReplaceAll(content, "name: %s", "name: talm") + content = strings.ReplaceAll(content, "version: %s", "version: "+version) + } + + dest := filepath.Join(root, "charts", "talm", filepath.FromSlash(rel)) + if err := os.MkdirAll(filepath.Dir(dest), 0o755); err != nil { + t.Fatalf("mkdir: %v", err) + } + if err := os.WriteFile(dest, []byte(content), 0o644); err != nil { + t.Fatalf("write: %v", err) + } + } + + return root +} + +func hintsContain(err error, substr string) bool { + for _, hint := range errors.GetAllHints(err) { + if strings.Contains(hint, substr) { + return true + } + } + + return false +} + +// writeDriftedPresetProject materializes a project whose .talm-preset.lock +// pins a baseline hash that cannot match the embedded cozystack preset, so +// CheckPresetDrift reports drift (the older-binary-init scenario). +func writeDriftedPresetProject(t *testing.T) string { + t.Helper() + + root := t.TempDir() + lock := "preset: cozystack\npresetHash: " + strings.Repeat("0", 64) + "\n" + if err := os.WriteFile(filepath.Join(root, ".talm-preset.lock"), []byte(lock), 0o644); err != nil { + t.Fatalf("write preset lock: %v", err) + } + + return root +} + +// TestEvaluateChartDrift pins the warn-vs-fail decision that drives the +// user-facing behavior: strict mode aborts with a remediation hint, +// non-strict drift warns without blocking, a dev build is silent even with +// drift present, and a content-identical library never trips on a +// version-only difference. +func TestEvaluateChartDrift(t *testing.T) { + t.Run("strict drift returns a hard error hinting at init --update", func(t *testing.T) { + root := writeDriftedTalmProject(t) + + warning, err := evaluateChartDrift("0.30.0", root, true) + if err == nil { + t.Fatal("expected a strict-mode error for a drifted project") + } + if warning != "" { + t.Errorf("strict error path must not also emit a warning, got %q", warning) + } + if !hintsContain(err, "talm init --update --preset") { + t.Errorf("strict-mode error must hint at `talm init --update --preset`, hints: %v", errors.GetAllHints(err)) + } + if strings.Contains(err.Error(), "or ignore if this is intentional") { + t.Errorf("strict-mode error must not carry the warning's ignore suggestion — the command just refused to run: %v", err) + } + }) + + t.Run("non-strict drift warns without blocking", func(t *testing.T) { + root := writeDriftedTalmProject(t) + + warning, err := evaluateChartDrift("0.30.0", root, false) + if err != nil { + t.Fatalf("non-strict drift must not block the command: %v", err) + } + if !strings.Contains(warning, "charts/talm") { + t.Errorf("expected a drift warning mentioning charts/talm, got %q", warning) + } + }) + + t.Run("unreadable vendored tree under strict blocks the command", func(t *testing.T) { + // charts/talm as a file: the drift check cannot read the tree. + // Strict mode exists for enforcement; an unverifiable baseline + // passing silently would defeat it exactly when the project is + // broken. + root := t.TempDir() + if err := os.MkdirAll(filepath.Join(root, "charts"), 0o755); err != nil { + t.Fatalf("mkdir: %v", err) + } + if err := os.WriteFile(filepath.Join(root, "charts", "talm"), []byte("not a directory"), 0o644); err != nil { + t.Fatalf("write file: %v", err) + } + + warning, err := evaluateChartDrift("0.30.0", root, true) + if err == nil { + t.Fatal("a drift check failure must block under strict mode, not silently pass") + } + if warning != "" { + t.Errorf("strict error path must not also emit a warning, got %q", warning) + } + }) + + t.Run("unreadable vendored tree without strict downgrades to a warning", func(t *testing.T) { + root := t.TempDir() + if err := os.MkdirAll(filepath.Join(root, "charts"), 0o755); err != nil { + t.Fatalf("mkdir: %v", err) + } + if err := os.WriteFile(filepath.Join(root, "charts", "talm"), []byte("not a directory"), 0o644); err != nil { + t.Fatalf("write file: %v", err) + } + + warning, err := evaluateChartDrift("0.30.0", root, false) + if err != nil { + t.Fatalf("a non-strict drift check failure must not block the command: %v", err) + } + if !strings.Contains(warning, "could not check drift") { + t.Errorf("expected a could-not-check warning, got %q", warning) + } + }) + + t.Run("missing vendored library under strict blocks the command", func(t *testing.T) { + // Deleting charts/talm/ is the cheapest bypass of the check — + // under opt-in enforcement a missing baseline must block, not + // pass more quietly than a corrupted one. + root := t.TempDir() + + warning, err := evaluateChartDrift("0.30.0", root, true) + if err == nil { + t.Fatal("a missing vendored library must block under strict mode, not silently pass") + } + if warning != "" { + t.Errorf("strict error path must not also emit a warning, got %q", warning) + } + if !hintsContain(err, "talm init --update --preset") { + t.Errorf("strict-mode error must hint at `talm init --update --preset`, hints: %v", errors.GetAllHints(err)) + } + }) + + t.Run("missing vendored library without strict is silent", func(t *testing.T) { + root := t.TempDir() + + warning, err := evaluateChartDrift("0.30.0", root, false) + if err != nil || warning != "" { + t.Errorf("a project with nothing vendored must stay silent without strict, got warning=%q err=%v", warning, err) + } + }) + + t.Run("dev build is silent even with drift present and strict set", func(t *testing.T) { + root := writeDriftedTalmProject(t) + + warning, err := evaluateChartDrift("dev", root, true) + if err != nil || warning != "" { + t.Errorf("dev build must be a no-op, got warning=%q err=%v", warning, err) + } + }) + + t.Run("matching library is silent under strict mode despite a newer binary version", func(t *testing.T) { + root := writeMatchingTalmProject(t, "0.30.0") + + warning, err := evaluateChartDrift("0.31.0", root, true) + if err != nil || warning != "" { + t.Errorf("a content-identical library must not drift on a version-only difference, got warning=%q err=%v", warning, err) + } + }) +} + +// TestEvaluatePresetDrift mirrors TestEvaluateChartDrift for the preset +// baseline (.talm-preset.lock): strict mode aborts with a remediation hint, +// non-strict drift warns without blocking, a dev build is silent, a +// freshly-pinned preset never drifts, and a project with no lock is silent. +func TestEvaluatePresetDrift(t *testing.T) { + t.Run("strict drift returns a hard error hinting at init --update", func(t *testing.T) { + root := writeDriftedPresetProject(t) + + warning, err := evaluatePresetDrift("0.30.0", root, true) + if err == nil { + t.Fatal("expected a strict-mode error for a drifted preset") + } + if warning != "" { + t.Errorf("strict error path must not also emit a warning, got %q", warning) + } + if !hintsContain(err, "talm init --update --preset") { + t.Errorf("strict-mode error must hint at `talm init --update --preset`, hints: %v", errors.GetAllHints(err)) + } + }) + + t.Run("non-strict drift warns without blocking", func(t *testing.T) { + root := writeDriftedPresetProject(t) + + warning, err := evaluatePresetDrift("0.30.0", root, false) + if err != nil { + t.Fatalf("non-strict drift must not block the command: %v", err) + } + if !strings.Contains(warning, "preset") || !strings.Contains(warning, "cozystack") { + t.Errorf("expected a preset drift warning naming the preset, got %q", warning) + } + }) + + t.Run("dev build is silent even with drift present and strict set", func(t *testing.T) { + root := writeDriftedPresetProject(t) + + warning, err := evaluatePresetDrift("dev", root, true) + if err != nil || warning != "" { + t.Errorf("dev build must be a no-op, got warning=%q err=%v", warning, err) + } + }) + + t.Run("freshly-pinned preset is silent under strict mode despite a newer binary version", func(t *testing.T) { + root := t.TempDir() + if err := commands.WritePresetLock(root, "cozystack"); err != nil { + t.Fatalf("WritePresetLock: %v", err) + } + + warning, err := evaluatePresetDrift("0.31.0", root, true) + if err != nil || warning != "" { + t.Errorf("a freshly-pinned preset must not drift, got warning=%q err=%v", warning, err) + } + }) + + t.Run("project without a preset lock is silent without strict", func(t *testing.T) { + root := t.TempDir() + + warning, err := evaluatePresetDrift("0.30.0", root, false) + if err != nil || warning != "" { + t.Errorf("a project with no preset lock must be silent without strict, got warning=%q err=%v", warning, err) + } + }) + + t.Run("missing preset lock under strict blocks the command", func(t *testing.T) { + // A merge conflict resolved as "delete .talm-preset.lock" must + // not pass MORE quietly than a corrupted lock — under opt-in + // enforcement a missing baseline is indistinguishable from a + // deleted one. + root := t.TempDir() + + warning, err := evaluatePresetDrift("0.30.0", root, true) + if err == nil { + t.Fatal("a missing preset baseline must block under strict mode, not silently pass") + } + if warning != "" { + t.Errorf("strict error path must not also emit a warning, got %q", warning) + } + if !hintsContain(err, "talm init --update --preset") { + t.Errorf("strict-mode error must hint at `talm init --update --preset`, hints: %v", errors.GetAllHints(err)) + } + }) + + t.Run("malformed lock under strict blocks the command", func(t *testing.T) { + // A lock corrupted by a bad merge is exactly the moment the + // team's strictCharts enforcement must fire, not silently pass. + root := t.TempDir() + if err := os.WriteFile(filepath.Join(root, ".talm-preset.lock"), []byte("preset: [unclosed\n"), 0o644); err != nil { + t.Fatalf("write lock: %v", err) + } + + warning, err := evaluatePresetDrift("0.30.0", root, true) + if err == nil { + t.Fatal("an unreadable preset baseline must block under strict mode, not silently pass") + } + if warning != "" { + t.Errorf("strict error path must not also emit a warning, got %q", warning) + } + }) + + t.Run("malformed lock without strict downgrades to a warning", func(t *testing.T) { + root := t.TempDir() + if err := os.WriteFile(filepath.Join(root, ".talm-preset.lock"), []byte("preset: [unclosed\n"), 0o644); err != nil { + t.Fatalf("write lock: %v", err) + } + + warning, err := evaluatePresetDrift("0.30.0", root, false) + if err != nil { + t.Fatalf("a non-strict drift check failure must not block the command: %v", err) + } + if !strings.Contains(warning, "could not check drift") { + t.Errorf("expected a could-not-check warning, got %q", warning) + } + }) +} + func TestSkipConfigCommands(t *testing.T) { tests := []struct { name string @@ -283,3 +699,52 @@ func TestSkipConfigCommands(t *testing.T) { }) } } + +// TestReleaseVersion pins that both release build paths are recognized. +// goreleaser injects the version WITHOUT the "v" prefix (`-X +// main.Version={{.Version}}` → "0.30.0") while the Makefile's `git describe +// --tags` keeps it ("v0.30.0"). A previous gate accepted only the +// "v"-prefixed form, which silently disabled the chart-drift check and the +// init version stamp on every downloaded release. Both forms must parse to +// the same version and report isRelease=true; dev/empty builds must not. +func TestReleaseVersion(t *testing.T) { + tests := []struct { + name string + raw string + wantVersion string + wantRelease bool + }{ + {"goreleaser form (no v)", "0.30.0", "0.30.0", true}, + {"makefile form (with v)", "v0.30.0", "0.30.0", true}, + {"dev source build", "dev", "", false}, + {"empty version", "", "", false}, + {"prerelease no v", "0.30.0-rc.1", "0.30.0-rc.1", true}, + // git describe on a non-tag commit appends "--g". + // Such a build is a developer's WIP tree, not a release: its + // embedded charts are a moving target, so treating it as a + // release would raise false drift on every command (and hard- + // fail strict projects). The Makefile now emits "dev" off-tag, + // but the parser must reject the describe shape regardless of + // how the string was injected. + {"describe suffix (with v)", "v0.29.0-5-gabc1234", "", false}, + {"describe suffix (no v)", "0.29.0-5-gabc1234", "", false}, + {"describe suffix dirty", "v0.29.0-5-gabc1234-dirty", "", false}, + // git describe --dirty on an EXACT tag with local edits yields + // "v0.29.0-dirty" (no commit suffix). Local edits can include the + // embedded charts themselves, so this build must not pose as the + // release it was forked from. + {"dirty at exact tag", "v0.29.0-dirty", "", false}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + version, isRelease := releaseVersion(tt.raw) + if isRelease != tt.wantRelease { + t.Errorf("releaseVersion(%q) isRelease = %v, want %v", tt.raw, isRelease, tt.wantRelease) + } + if version != tt.wantVersion { + t.Errorf("releaseVersion(%q) version = %q, want %q", tt.raw, version, tt.wantVersion) + } + }) + } +} diff --git a/pkg/commands/chart_drift.go b/pkg/commands/chart_drift.go new file mode 100644 index 00000000..40cad387 --- /dev/null +++ b/pkg/commands/chart_drift.go @@ -0,0 +1,316 @@ +// Copyright Cozystack Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package commands + +import ( + "fmt" + "io/fs" + "os" + "path" + "path/filepath" + "sort" + "strings" + + "github.com/cockroachdb/errors" + "gopkg.in/yaml.v3" + + "github.com/cozystack/talm/pkg/generated" +) + +// ErrNoBaseline marks the "nothing to compare" drift outcome: the project +// has no vendored charts/talm/ or no .talm-preset.lock. Non-strict callers +// treat it as silence — a project generated before baseline pinning should +// not nag. Strict callers escalate instead: under opt-in enforcement a +// missing baseline is indistinguishable from a deleted one, and letting it +// pass would make deletion the cheapest bypass of the check. +var ErrNoBaseline = errors.New("no drift baseline to compare") + +// vendoredTalmFiles reads a project's vendored talm library from +// rootDir/charts/talm/, keyed by forward-slash path relative to that +// directory (so the keys line up with the embedded TalmLibraryFiles output) +// and with Chart.yaml metadata normalized the same way. The bool is false +// when no vendored library exists — an unconfigured or freshly cloned +// project; the caller decides whether that is silence or a strict blocker. +// +// Reads go through an os.Root rooted at charts/talm/, confining traversal to +// that subtree so a symlink inside it cannot redirect a read outside the +// project (gosec G122). +func vendoredTalmFiles(rootDir string) (map[string]string, bool, error) { + base := filepath.Join(rootDir, "charts", "talm") + + root, err := os.OpenRoot(base) + if err != nil { + if errors.Is(err, fs.ErrNotExist) { + return nil, false, nil + } + + return nil, false, errors.Wrapf(err, "opening vendored talm library %q", base) + } + defer root.Close() + + fsys := root.FS() + filesMap := make(map[string]string) + + err = fs.WalkDir(fsys, ".", func(filePath string, entry fs.DirEntry, err error) error { + if err != nil { + return errors.Wrapf(err, "walking vendored talm library at %q", filePath) + } + + if entry.IsDir() { + return nil + } + + data, err := fs.ReadFile(fsys, filePath) + if err != nil { + return errors.Wrapf(err, "reading vendored talm file %q", filePath) + } + + // Normalize CRLF to LF before comparing: a Windows clone with + // core.autocrlf=true (the Git for Windows default) materializes the + // vendored tree with CRLF while the embedded library is LF-only. + // Line endings are checkout artifacts, not chart content — without + // this, every command on such a clone reports false drift, and + // strictCharts: true hard-fails a byte-identical-modulo-EOL tree. + content := strings.ReplaceAll(string(data), "\r\n", "\n") + + // root.FS() keys are already "/"-separated and relative to base, so + // they match the embedded TalmLibraryFiles output on every platform. + filesMap[filePath] = generated.NormalizeChartMeta(path.Base(filePath), content) + + return nil + }) + if err != nil { + return nil, false, errors.Wrap(err, "collecting vendored talm library") + } + + return filesMap, true, nil +} + +// CheckChartDrift reports whether a project's vendored charts/talm/ library +// diverges, by content, from the copy built into this talm binary. +// +// talm vendors its library chart into the project at `talm init` time; +// rendering (template/apply/upgrade) reads that local copy, never the +// binary's embedded charts. Upgrading the binary therefore leaves +// charts/talm/ frozen at the version that last ran init. CheckChartDrift +// surfaces that staleness so the operator can re-run +// `talm init --update --preset `. +// +// The comparison is by content: the Chart.yaml version stamp is normalized +// away on both sides, so a pure version bump that left the library +// byte-identical is NOT reported as drift. binaryVersion is used only for +// the operator-facing message. +// +// When the project has no vendored library to compare, it returns an error +// matching ErrNoBaseline (errors.Is) so the caller can choose silence +// (non-strict) or a blocker (strict). A read or walk failure is returned as +// a plain error; non-strict callers treat drift detection as best-effort +// and must not block a command on it. +func CheckChartDrift(rootDir, binaryVersion string) (bool, string, error) { + vendored, ok, err := vendoredTalmFiles(rootDir) + if err != nil { + return false, "", err + } + + if !ok { + return false, "", errors.Wrap(ErrNoBaseline, "project has no vendored charts/talm/ library") + } + + embedded, err := generated.TalmLibraryFiles() + if err != nil { + return false, "", errors.Wrap(err, "loading embedded talm library") + } + + if generated.HashChartFiles(vendored) == generated.HashChartFiles(embedded) { + return false, "", nil + } + + return true, fmt.Sprintf( + "project's vendored charts/talm/ library differs from the copy built into talm %s (%s); "+ + "run `talm init --update --preset ` to re-sync (or ignore if this is intentional)", + binaryVersion, summarizeChartDiff(vendored, embedded), + ), nil +} + +// diffSampleLimit caps how many differing paths the drift message carries. +// Enough to locate the cause at a glance without turning a one-line WARN +// into a page when a whole release's worth of templates changed. +const diffSampleLimit = 5 + +// summarizeChartDiff renders the paths where two chart trees diverge — +// modified (both sides, different content), extra (vendored only), missing +// (embedded only) — sorted, capped at diffSampleLimit with a "+N more" +// tail. Without the paths, the operator cannot tell a stale library from +// an extraneous file (.DS_Store) that `init --update` re-vendoring alone +// would not have cleared. +func summarizeChartDiff(vendored, embedded map[string]string) string { + var diffs []string + + for filePath, content := range vendored { + embeddedContent, ok := embedded[filePath] + + switch { + case !ok: + diffs = append(diffs, "extra: "+filePath) + case content != embeddedContent: + diffs = append(diffs, "modified: "+filePath) + } + } + + for filePath := range embedded { + if _, ok := vendored[filePath]; !ok { + diffs = append(diffs, "missing: "+filePath) + } + } + + sort.Strings(diffs) + + more := "" + if len(diffs) > diffSampleLimit { + more = fmt.Sprintf(", +%d more", len(diffs)-diffSampleLimit) + diffs = diffs[:diffSampleLimit] + } + + return strings.Join(diffs, ", ") + more +} + +// presetLockName is the project-root file that records the pristine preset a +// project was generated from. Unlike charts/talm/ (vendored library, never +// operator-edited, content-checked by CheckChartDrift), the preset templates +// land in templates/ and ARE meant to be operator-edited — so they cannot be +// content-checked without false-positiving on every customization. The lock +// instead pins the hash of the preset AS SHIPPED at init time; drift is the +// binary's current preset hash diverging from that pinned baseline, which is +// independent of whatever the operator did to their templates/. +const presetLockName = ".talm-preset.lock" + +// presetLockHeader is prepended to the written lock so an operator who opens +// it understands it is machine-managed. +const presetLockHeader = "# Managed by talm. Records the preset this project was generated from and the\n" + + "# content hash of that preset at init time, so talm can warn when the installed\n" + + "# binary ships a newer preset. Do not edit by hand; run\n" + + "# `talm init --update --preset ` to refresh.\n" + +// presetLock is the on-disk shape of presetLockName. +type presetLock struct { + Preset string `yaml:"preset"` + PresetHash string `yaml:"presetHash"` +} + +// embeddedPresetHash returns the content digest of the named preset as built +// into this binary. PresetFiles() returns every preset keyed by a +// "/..." path; the subset for one preset is hashed with those keys +// intact, so the same call at init time (WritePresetLock) and at check time +// (CheckPresetDrift) yields the same digest for an unchanged preset. Chart +// metadata is already normalized by PresetFiles, so a pure version bump in +// the preset's Chart.yaml is not seen as content drift — matching the +// library check's contract. Returns an error when the preset is unknown to +// this binary (no files carry its prefix). +func embeddedPresetHash(preset string) (string, error) { + all, err := generated.PresetFiles() + if err != nil { + return "", errors.Wrap(err, "loading embedded preset files") + } + + prefix := preset + "/" + subset := make(map[string]string) + + for filePath, content := range all { + if strings.HasPrefix(filePath, prefix) { + subset[filePath] = content + } + } + + if len(subset) == 0 { + //nolint:wrapcheck // origin error built in-function with cockroachdb/errors.Newf; nothing upstream to wrap. + return "", errors.Newf("unknown preset %q: no embedded files carry that prefix", preset) + } + + return generated.HashChartFiles(subset), nil +} + +// WritePresetLock pins the current pristine hash of preset into +// rootDir/.talm-preset.lock. Called after `talm init` (and `init --update`) +// has materialized the preset into the project, so a later binary upgrade +// that changes the preset can be surfaced by CheckPresetDrift. The hash is of +// the EMBEDDED preset, not the project's (possibly operator-edited) copy. +func WritePresetLock(rootDir, preset string) error { + hash, err := embeddedPresetHash(preset) + if err != nil { + return err + } + + body, err := yaml.Marshal(presetLock{Preset: preset, PresetHash: hash}) + if err != nil { + return errors.Wrap(err, "marshaling preset lock") + } + + dest := filepath.Join(rootDir, presetLockName) + if err := os.WriteFile(dest, append([]byte(presetLockHeader), body...), presetFileMode); err != nil { + return errors.Wrapf(err, "writing preset lock %q", dest) + } + + return nil +} + +// CheckPresetDrift reports whether the preset built into this talm binary has +// changed since the project was generated, by comparing the binary's current +// preset hash against the baseline pinned in rootDir/.talm-preset.lock at +// init time. +// +// For a project with no lock file (one generated before preset pinning +// existed, or never init'd from a preset) it returns an error matching +// ErrNoBaseline (errors.Is): non-strict callers stay silent — there is no +// baseline to compare, and inventing drift would nag every such project — +// while strict callers escalate. binaryVersion is used only for the +// operator-facing message. +// +// Crucially this never reads the project's templates/, so operator edits to +// the rendered preset are NOT drift: the baseline is the pristine preset hash +// at init, and the comparison is binary-now vs that baseline. +func CheckPresetDrift(rootDir, binaryVersion string) (bool, string, error) { + data, err := os.ReadFile(filepath.Join(rootDir, presetLockName)) + if err != nil { + if errors.Is(err, fs.ErrNotExist) { + return false, "", errors.Wrap(ErrNoBaseline, "project has no "+presetLockName) + } + + return false, "", errors.Wrapf(err, "reading preset lock in %q", rootDir) + } + + var lock presetLock + if err := yaml.Unmarshal(data, &lock); err != nil { + return false, "", errors.Wrap(err, "parsing preset lock") + } + + if lock.Preset == "" || lock.PresetHash == "" { + return false, "", errors.New("preset lock is missing preset or presetHash") + } + + current, err := embeddedPresetHash(lock.Preset) + if err != nil { + return false, "", err + } + + if current == lock.PresetHash { + return false, "", nil + } + + return true, fmt.Sprintf( + "project's %s preset differs from the copy built into talm %s; "+ + "run `talm init --update --preset %s` to pull the new preset defaults (your templates/ edits are preserved via the interactive diff)", + lock.Preset, binaryVersion, lock.Preset, + ), nil +} diff --git a/pkg/commands/chart_drift_test.go b/pkg/commands/chart_drift_test.go new file mode 100644 index 00000000..bed047a2 --- /dev/null +++ b/pkg/commands/chart_drift_test.go @@ -0,0 +1,432 @@ +// Copyright Cozystack Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package commands + +import ( + "io/fs" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/cockroachdb/errors" + + "github.com/cozystack/talm/pkg/generated" +) + +// writeVendoredTalmLibrary materializes a project's charts/talm/ tree from +// the binary's embedded library, stamping version into the library +// Chart.yaml so the result looks like a real `talm init` would produce. It +// returns the project root. +func writeVendoredTalmLibrary(t *testing.T, version string) string { + t.Helper() + + files, err := generated.TalmLibraryFiles() + if err != nil { + t.Fatalf("TalmLibraryFiles: %v", err) + } + + root := t.TempDir() + for rel, content := range files { + // TalmLibraryFiles normalizes Chart.yaml name/version to %s; fill + // them back in (name, version) so the vendored copy mirrors init. + if filepath.Base(rel) == "Chart.yaml" { + content = strings.ReplaceAll(content, "name: %s", "name: talm") + content = strings.ReplaceAll(content, "version: %s", "version: "+version) + } + + dest := filepath.Join(root, "charts", "talm", filepath.FromSlash(rel)) + if err := os.MkdirAll(filepath.Dir(dest), 0o755); err != nil { + t.Fatalf("mkdir %q: %v", dest, err) + } + if err := os.WriteFile(dest, []byte(content), 0o644); err != nil { + t.Fatalf("write %q: %v", dest, err) + } + } + + return root +} + +// TestCheckChartDrift_MatchingCopy_NoDrift pins the happy path: a project +// whose vendored charts/talm/ matches the embedded library reports no +// drift, even though its Chart.yaml carries a concrete version stamp. +func TestCheckChartDrift_MatchingCopy_NoDrift(t *testing.T) { + root := writeVendoredTalmLibrary(t, "0.30.0") + + drift, msg, err := CheckChartDrift(root, "0.30.0") + if err != nil { + t.Fatalf("CheckChartDrift: %v", err) + } + if drift { + t.Errorf("reported drift for a matching vendored copy: %s", msg) + } +} + +// TestCheckChartDrift_VersionStampOnly_NoDrift is the core regression +// guard. A project vendored by an older release carries an older version +// stamp in charts/talm/Chart.yaml, but its helpers are byte-identical to +// the running binary's. That MUST NOT be reported as drift — flagging a +// pure version difference is exactly the false positive the version-number +// comparison approach produced. +func TestCheckChartDrift_VersionStampOnly_NoDrift(t *testing.T) { + root := writeVendoredTalmLibrary(t, "0.1.0") + + drift, msg, err := CheckChartDrift(root, "0.30.0") + if err != nil { + t.Fatalf("CheckChartDrift: %v", err) + } + if drift { + t.Errorf("reported drift for a version-only difference; this is a false positive: %s", msg) + } +} + +// TestCheckChartDrift_ContentChange_Drift pins detection: a vendored +// helpers template that diverges from the embedded copy is real drift and +// must be surfaced. +func TestCheckChartDrift_ContentChange_Drift(t *testing.T) { + root := writeVendoredTalmLibrary(t, "0.30.0") + + helpers := filepath.Join(root, "charts", "talm", "templates", "_helpers.tpl") + data, err := os.ReadFile(helpers) + if err != nil { + t.Fatalf("read helpers: %v", err) + } + if err := os.WriteFile(helpers, append(data, []byte("\n{{- /* stale local edit */ -}}\n")...), 0o644); err != nil { + t.Fatalf("write helpers: %v", err) + } + + drift, msg, err := CheckChartDrift(root, "0.30.0") + if err != nil { + t.Fatalf("CheckChartDrift: %v", err) + } + if !drift { + t.Error("did not detect a divergent vendored helpers template; real drift went unreported") + } + // The remediation must carry --preset: bare `talm init --update` cannot + // resolve the preset from an init'd project's Chart.yaml and errors out + // without re-vendoring, so pointing at it would send operators to a + // command that does not clear the drift. + if !strings.Contains(msg, "talm init --update --preset") { + t.Errorf("drift message must point at `talm init --update --preset`; got %q", msg) + } +} + +// TestCheckChartDrift_ExtraneousFile_DriftNamesPath pins two contracts for +// the "extra file in the talm-owned tree" shape (.DS_Store, editor backup, +// a file a newer library dropped): it IS drift — the vendored tree no +// longer matches the binary — and the message must name the offending +// path. Without the path the operator gets a warning that +// `init --update --preset` alone cannot clear and no way to locate why. +func TestCheckChartDrift_ExtraneousFile_DriftNamesPath(t *testing.T) { + root := writeVendoredTalmLibrary(t, "0.30.0") + + if err := os.WriteFile(filepath.Join(root, "charts", "talm", ".DS_Store"), []byte{0x00, 0x01}, 0o644); err != nil { + t.Fatalf("write extraneous file: %v", err) + } + + drift, msg, err := CheckChartDrift(root, "0.30.0") + if err != nil { + t.Fatalf("CheckChartDrift: %v", err) + } + + if !drift { + t.Fatal("an extraneous file in the vendored tree must be reported as drift") + } + + if !strings.Contains(msg, "extra: .DS_Store") { + t.Errorf("drift message must name the extraneous path; got %q", msg) + } +} + +// TestCheckChartDrift_ModifiedFile_DriftNamesPath pins that a content +// change is reported with the modified path, so the operator can see WHAT +// drifted instead of diffing the tree by hand. +func TestCheckChartDrift_ModifiedFile_DriftNamesPath(t *testing.T) { + root := writeVendoredTalmLibrary(t, "0.30.0") + + helpers := filepath.Join(root, "charts", "talm", "templates", "_helpers.tpl") + if err := os.WriteFile(helpers, []byte("{{- /* divergent */ -}}\n"), 0o644); err != nil { + t.Fatalf("write helpers: %v", err) + } + + drift, msg, err := CheckChartDrift(root, "0.30.0") + if err != nil { + t.Fatalf("CheckChartDrift: %v", err) + } + + if !drift { + t.Fatal("a modified vendored file must be reported as drift") + } + + if !strings.Contains(msg, "modified: templates/_helpers.tpl") { + t.Errorf("drift message must name the modified path; got %q", msg) + } +} + +// TestCheckChartDrift_CRLFVendoredCopy_NoDrift pins EOL-insensitivity: a +// project cloned on Windows with core.autocrlf=true (the Git for Windows +// default) materializes the vendored charts/talm/ with CRLF endings while +// the embedded library is LF. Line endings are checkout artifacts, not +// chart content — flagging them would WARN on every command for a +// byte-identical-modulo-EOL tree, and hard-fail teams that set +// strictCharts: true precisely for CI enforcement. +func TestCheckChartDrift_CRLFVendoredCopy_NoDrift(t *testing.T) { + root := writeVendoredTalmLibrary(t, "0.30.0") + + base := filepath.Join(root, "charts", "talm") + walkErr := filepath.WalkDir(base, func(p string, d fs.DirEntry, err error) error { + if err != nil { + t.Fatalf("walking vendored tree at %q: %v", p, err) + } + + if d.IsDir() { + return nil + } + + data, readErr := os.ReadFile(p) + if readErr != nil { + t.Fatalf("read %q: %v", p, readErr) + } + + crlf := strings.ReplaceAll(string(data), "\n", "\r\n") + if writeErr := os.WriteFile(p, []byte(crlf), 0o644); writeErr != nil { + t.Fatalf("write %q: %v", p, writeErr) + } + + return nil + }) + if walkErr != nil { + t.Fatalf("converting vendored tree to CRLF: %v", walkErr) + } + + drift, msg, err := CheckChartDrift(root, "0.30.0") + if err != nil { + t.Fatalf("CheckChartDrift: %v", err) + } + + if drift { + t.Errorf("CRLF line endings were misreported as drift: %s", msg) + } +} + +// TestCheckChartDrift_VendoredTalmIsFile_ErrorsGracefully pins that a +// charts/talm that exists as a file (not a directory) is surfaced as an +// error rather than a panic or a spurious drift result. Callers +// (evaluateChartDrift) downgrade this to a non-fatal warning, so a +// malformed project never crashes a command. +func TestCheckChartDrift_VendoredTalmIsFile_ErrorsGracefully(t *testing.T) { + root := t.TempDir() + if err := os.MkdirAll(filepath.Join(root, "charts"), 0o755); err != nil { + t.Fatalf("mkdir: %v", err) + } + if err := os.WriteFile(filepath.Join(root, "charts", "talm"), []byte("not a directory"), 0o644); err != nil { + t.Fatalf("write file: %v", err) + } + + drift, _, err := CheckChartDrift(root, "0.30.0") + if err == nil { + t.Error("expected an error when charts/talm is a file, got nil") + } + if drift { + t.Error("must not report drift when the vendored tree cannot be read") + } +} + +// TestCheckChartDrift_MissingVendoredDir_NoBaselineSentinel pins the +// missing-baseline contract: a project without a charts/talm/ directory +// (nothing vendored yet) yields no drift and an error matching +// ErrNoBaseline, so the caller can keep non-strict runs silent while +// strict runs block — deleting the vendored tree must not be a quieter +// bypass than corrupting it. +func TestCheckChartDrift_MissingVendoredDir_NoBaselineSentinel(t *testing.T) { + root := t.TempDir() + + drift, msg, err := CheckChartDrift(root, "0.30.0") + if !errors.Is(err, ErrNoBaseline) { + t.Fatalf("expected an ErrNoBaseline-matching error for a missing vendored dir, got: %v", err) + } + if drift { + t.Errorf("reported drift with no vendored library to compare: %s", msg) + } +} + +// === preset drift (.talm-preset.lock) === + +// TestPresetLock_RoundTrip_NoDrift pins the happy path: WritePresetLock pins +// the current embedded preset hash, and CheckPresetDrift against the same +// binary reports no drift. +func TestPresetLock_RoundTrip_NoDrift(t *testing.T) { + root := t.TempDir() + if err := WritePresetLock(root, "cozystack"); err != nil { + t.Fatalf("WritePresetLock: %v", err) + } + + drift, msg, err := CheckPresetDrift(root, "0.30.0") + if err != nil { + t.Fatalf("CheckPresetDrift: %v", err) + } + if drift { + t.Errorf("reported preset drift for a freshly-pinned lock: %s", msg) + } +} + +// TestCheckPresetDrift_StaleBaseline_Drift simulates a project pinned by an +// older binary whose preset content differs: the lock carries a baseline +// hash that no longer matches the embedded preset, which is real drift and +// must point the operator at `talm init --update --preset `. +func TestCheckPresetDrift_StaleBaseline_Drift(t *testing.T) { + root := t.TempDir() + // A baseline hash that cannot match the embedded cozystack preset. + lock := "preset: cozystack\npresetHash: 0000000000000000000000000000000000000000000000000000000000000000\n" + if err := os.WriteFile(filepath.Join(root, ".talm-preset.lock"), []byte(lock), 0o644); err != nil { + t.Fatalf("write lock: %v", err) + } + + drift, msg, err := CheckPresetDrift(root, "0.30.0") + if err != nil { + t.Fatalf("CheckPresetDrift: %v", err) + } + if !drift { + t.Fatal("did not detect drift against a stale preset baseline") + } + if !strings.Contains(msg, "talm init --update --preset cozystack") { + t.Errorf("drift message must point at `talm init --update --preset cozystack`; got %q", msg) + } +} + +// TestCheckPresetDrift_NoLock_NoBaselineSentinel pins the missing-baseline +// contract: a project with no .talm-preset.lock (generated before preset +// pinning, or never init'd from a preset) yields no drift and an error +// matching ErrNoBaseline. The caller keeps non-strict runs silent — no +// baseline, no nag — while strict runs block, so a lock deleted by a bad +// merge resolution cannot pass more quietly than a corrupted one. +func TestCheckPresetDrift_NoLock_NoBaselineSentinel(t *testing.T) { + root := t.TempDir() + + drift, msg, err := CheckPresetDrift(root, "0.30.0") + if !errors.Is(err, ErrNoBaseline) { + t.Fatalf("expected an ErrNoBaseline-matching error with no lock, got: %v", err) + } + if drift { + t.Errorf("reported preset drift with no baseline lock: %s", msg) + } +} + +// TestCheckPresetDrift_OperatorEditedTemplates_NoDrift is the core +// false-positive guard and the whole reason the preset check is +// baseline-hash-based rather than content-based: the operator is EXPECTED to +// edit templates/, and that must never be reported as preset drift. The lock +// pins the pristine embedded hash; editing the project's templates/ leaves it +// untouched, so the check stays silent. +func TestCheckPresetDrift_OperatorEditedTemplates_NoDrift(t *testing.T) { + root := t.TempDir() + if err := WritePresetLock(root, "cozystack"); err != nil { + t.Fatalf("WritePresetLock: %v", err) + } + + tmpl := filepath.Join(root, "templates") + if err := os.MkdirAll(tmpl, 0o755); err != nil { + t.Fatalf("mkdir templates: %v", err) + } + if err := os.WriteFile(filepath.Join(tmpl, "_helpers.tpl"), []byte("{{- /* heavily customized by the operator */ -}}\n"), 0o644); err != nil { + t.Fatalf("write operator template: %v", err) + } + + drift, msg, err := CheckPresetDrift(root, "0.30.0") + if err != nil { + t.Fatalf("CheckPresetDrift: %v", err) + } + if drift { + t.Errorf("operator edits to templates/ were misreported as preset drift: %s", msg) + } +} + +// TestCheckPresetDrift_MalformedLock_ErrorsGracefully pins the failure mode +// for a corrupted .talm-preset.lock: unparseable YAML must surface as an +// error with drift=false — never a panic, never a spurious drift verdict. +// Callers (evaluatePresetDrift → decideDrift) downgrade the error to a +// non-fatal warning, so a mangled lock never blocks a command. +func TestCheckPresetDrift_MalformedLock_ErrorsGracefully(t *testing.T) { + root := t.TempDir() + if err := os.WriteFile(filepath.Join(root, ".talm-preset.lock"), []byte("preset: [unclosed\n"), 0o644); err != nil { + t.Fatalf("write lock: %v", err) + } + + drift, _, err := CheckPresetDrift(root, "0.30.0") + if err == nil { + t.Error("expected an error for an unparseable lock, got nil") + } + + if drift { + t.Error("must not report drift when the lock cannot be parsed") + } +} + +// TestCheckPresetDrift_LockMissingFields_ErrorsGracefully pins that a lock +// which parses as YAML but lacks preset or presetHash (hand-edited, or +// truncated by a bad merge) is an error with drift=false. Treating it as +// "no drift" would silently disable the check; inventing drift would nag on +// a baseline that was never pinned. +func TestCheckPresetDrift_LockMissingFields_ErrorsGracefully(t *testing.T) { + root := t.TempDir() + if err := os.WriteFile(filepath.Join(root, ".talm-preset.lock"), []byte("preset: cozystack\n"), 0o644); err != nil { + t.Fatalf("write lock: %v", err) + } + + drift, _, err := CheckPresetDrift(root, "0.30.0") + if err == nil { + t.Error("expected an error for a lock without presetHash, got nil") + } + + if drift { + t.Error("must not report drift for a lock with no pinned baseline") + } +} + +// TestCheckPresetDrift_LockUnknownPreset_ErrorsGracefully pins the +// cross-binary shape: a well-formed lock naming a preset this binary does +// not ship (pinned by a different talm build) is an error with drift=false, +// so the caller's warning names the unknown preset instead of fabricating a +// drift verdict against a baseline that cannot be recomputed. +func TestCheckPresetDrift_LockUnknownPreset_ErrorsGracefully(t *testing.T) { + root := t.TempDir() + lock := "preset: does-not-exist\npresetHash: 0000000000000000000000000000000000000000000000000000000000000000\n" + if err := os.WriteFile(filepath.Join(root, ".talm-preset.lock"), []byte(lock), 0o644); err != nil { + t.Fatalf("write lock: %v", err) + } + + drift, _, err := CheckPresetDrift(root, "0.30.0") + if err == nil { + t.Error("expected an error for a lock naming an unknown preset, got nil") + } + + if drift { + t.Error("must not report drift when the baseline preset is not embedded") + } +} + +// TestWritePresetLock_UnknownPreset_Errors pins that pinning a preset the +// binary does not ship fails loudly rather than writing an empty baseline. +func TestWritePresetLock_UnknownPreset_Errors(t *testing.T) { + root := t.TempDir() + + err := WritePresetLock(root, "does-not-exist") + if err == nil { + t.Fatal("expected an error pinning an unknown preset, got nil") + } + if !strings.Contains(err.Error(), "does-not-exist") { + t.Errorf("error should name the offending preset; got: %v", err) + } +} diff --git a/pkg/commands/init.go b/pkg/commands/init.go index f0ccc8de..35abe6d5 100644 --- a/pkg/commands/init.go +++ b/pkg/commands/init.go @@ -19,6 +19,7 @@ import ( "bytes" "fmt" "io" + "io/fs" "net" "net/url" "os" @@ -75,12 +76,13 @@ const ( // secret-bearing files so an over-permissive umask cannot widen // access. secureDirMode os.FileMode = 0o700 - // reportVerbCreated and reportVerbUpdated are the operator-facing - // verbs the init flow prints when materialising or rewriting a - // project artefact. Hoisted so goconst sees a single canonical - // reference for each. + // reportVerbCreated, reportVerbUpdated, and reportVerbRemoved are + // the operator-facing verbs the init flow prints when materialising, + // rewriting, or pruning a project artefact. Hoisted so goconst sees + // a single canonical reference for each. reportVerbCreated = "Created" reportVerbUpdated = "Updated" + reportVerbRemoved = "Removed" ) // resolveTalosconfigEndpoints picks the endpoint list to embed in @@ -778,6 +780,15 @@ var initCmd = &cobra.Command{ } } + // Pin the preset baseline (.talm-preset.lock) so a later talm + // upgrade that ships a changed preset is surfaced by + // CheckPresetDrift. The baseline is the pristine embedded preset + // hash; templates/ is operator-editable and never consulted, so + // the check stays false-positive-free. + if err := WritePresetLock(Config.RootDir, initCmdFlags.preset); err != nil { + return err + } + // Print warning about secrets and key backup (only once, at the end, if key was created) if keyWasCreated { printSecretsWarning() @@ -808,6 +819,81 @@ func writeSecretsBundleToFile(bundle *secrets.Bundle) error { return writeSecureToDestination(bundleBytes, secretsFile) } +// pruneVendoredTalmLibrary removes files under charts/talm/ that the +// embedded library does not ship — leftovers from an older talm release or +// strays (.DS_Store, editor backups). The tree is talm-owned and never +// operator-edited (see talmLibraryName in charts/charts.go), so removal is +// safe; keeping such files would leave the content-drift check permanently +// firing on a tree that `init --update` claims to have re-synced. +// presetFiles is the embedded set keyed "/"; a vendored file +// survives iff "talm/" is a key. Directories the prune leaves empty +// are removed too, so the tree matches the embedded library exactly. A +// missing charts/talm/ is a no-op. All operations go through an os.Root +// scoped to charts/talm/ (mirroring vendoredTalmFiles), so a symlink inside +// the tree cannot redirect a removal outside the project; symlinks are +// removed as entries, never followed. +func pruneVendoredTalmLibrary(presetFiles map[string]string) error { + base := filepath.Join(Config.RootDir, "charts", "talm") + + root, err := os.OpenRoot(base) + if err != nil { + if errors.Is(err, fs.ErrNotExist) { + return nil + } + + return errors.Wrapf(err, "opening vendored talm library %q", base) + } + defer root.Close() + + var seenDirs []string + + walkErr := fs.WalkDir(root.FS(), ".", func(filePath string, entry fs.DirEntry, err error) error { + if err != nil { + return errors.Wrapf(err, "walking vendored talm library at %q", filePath) + } + + if entry.IsDir() { + if filePath != "." { + seenDirs = append(seenDirs, filePath) + } + + return nil + } + + // root.FS() keys are "/"-separated and relative to base, matching + // the "/" shape of the embedded set. + if _, ok := presetFiles[presetTalmLibrary+"/"+filePath]; ok { + return nil + } + + if err := root.Remove(filePath); err != nil { + return errors.Wrapf(err, "pruning extraneous vendored file %q", filePath) + } + + fmt.Fprintf(os.Stderr, "%s %s\n", reportVerbRemoved, filepath.Join("charts", "talm", filepath.FromSlash(filePath))) + + return nil + }) + if walkErr != nil { + return errors.Wrap(walkErr, "pruning vendored talm library") + } + + // Drop directories the prune emptied, deepest-first (reverse + // lexicographic order puts "a/b" before "a"). Remove refuses + // non-empty directories, which is exactly the keep signal — failures + // are the expected outcome for live dirs, so they are not errors. + slices.Sort(seenDirs) + slices.Reverse(seenDirs) + + for _, dir := range seenDirs { + if err := root.Remove(dir); err == nil { + fmt.Fprintf(os.Stderr, "%s %s\n", reportVerbRemoved, filepath.Join("charts", "talm", filepath.FromSlash(dir))) + } + } + + return nil +} + // readChartYamlPreset reads Chart.yaml and determines the preset name from dependencies. func readChartYamlPreset() (string, error) { chartYamlPath := filepath.Join(Config.RootDir, chartYamlName) @@ -1334,6 +1420,23 @@ func updateTalmLibraryChart() error { // the same fact. return err } + + // Validate the inferred preset BEFORE any writes. Without this + // fail-fast, an unknown preset only surfaces in WritePresetLock — + // after Step 1 has already rewritten charts/talm/, leaving the + // project half-updated. + availablePresets, err := generated.AvailablePresets() + if err != nil { + return errors.Wrap(err, "failed to get available presets") + } + + if !isValidPreset(presetName, availablePresets) { + //nolint:wrapcheck // cockroachdb/errors.WithHint at boundary. + return errors.WithHint( + errors.Newf("preset %q from Chart.yaml is not embedded in this talm binary. Valid presets are: %v", presetName, availablePresets), + "pass --preset with one of the embedded presets, or use a talm build that ships your project's preset", + ) + } } presetFiles, err := generated.PresetFiles() @@ -1376,6 +1479,15 @@ func updateTalmLibraryChart() error { } } + // Prune files the embedded library no longer ships (and strays like + // .DS_Store or editor backups). charts/talm/ is talm-owned and never + // operator-edited, so an exact re-sync is safe — and without it the + // content-drift warning (or the strictCharts hard error) stays on + // forever while its hint keeps pointing at this very command. + if err := pruneVendoredTalmLibrary(presetFiles); err != nil { + return err + } + // Step 2: Update preset template files (with interactive confirmation) if presetName != "" { fmt.Fprintf(os.Stderr, "Updating preset templates...\n") @@ -1415,6 +1527,14 @@ func updateTalmLibraryChart() error { } } } + + // Advance the preset baseline to the refreshed preset so the drift + // warning clears: the operator has now reconciled against this + // binary's preset, even if they declined individual file diffs to + // keep local customizations. + if err := WritePresetLock(Config.RootDir, presetName); err != nil { + return err + } } return nil diff --git a/pkg/commands/init_test.go b/pkg/commands/init_test.go index 708cdc31..ce121eb3 100644 --- a/pkg/commands/init_test.go +++ b/pkg/commands/init_test.go @@ -419,6 +419,135 @@ func TestInitUpdate_PresetNotFoundError_SingleWrapped(t *testing.T) { } } +// TestInitUpdate_UnknownInferredPreset_FailsBeforeWrites pins the fail-fast +// contract for `talm init --update` with the preset inferred from +// Chart.yaml: a preset this binary does not embed must be rejected BEFORE +// Step 1 touches charts/talm/. Without up-front validation the inferred +// path rewrites the whole vendored library and only then fails in +// WritePresetLock, leaving the project half-updated. +func TestInitUpdate_UnknownInferredPreset_FailsBeforeWrites(t *testing.T) { + imageOrig := initCmdFlags.image + presetOrig := initCmdFlags.preset + rootOrig := Config.RootDir + t.Cleanup(func() { + initCmdFlags.image = imageOrig + initCmdFlags.preset = presetOrig + Config.RootDir = rootOrig + }) + + initCmdFlags.image = "" + initCmdFlags.preset = "" + + dir := t.TempDir() + Config.RootDir = dir + + chartYaml := "apiVersion: v2\nname: test\nversion: 0.1.0\n" + + "dependencies:\n" + + " - name: talm\n" + + " version: 0.1.0\n" + + " - name: no-such-preset\n" + + " version: 0.1.0\n" + if err := os.WriteFile(filepath.Join(dir, "Chart.yaml"), []byte(chartYaml), 0o644); err != nil { + t.Fatalf("seed Chart.yaml: %v", err) + } + + err := updateTalmLibraryChart() + if err == nil { + t.Fatal("expected --update with an unknown inferred preset to error; got nil") + } + + if !strings.Contains(err.Error(), "no-such-preset") { + t.Errorf("error must name the offending preset; got: %v", err) + } + + hints := errors.GetAllHints(err) + if len(hints) == 0 { + t.Fatalf("expected a hint guiding the operator to a valid preset; got bare error: %v", err) + } + + // Fail-fast contract: nothing may have been written before the + // validation fired. + if _, statErr := os.Stat(filepath.Join(dir, "charts")); !os.IsNotExist(statErr) { + t.Error("charts/ was created before the inferred preset was validated; --update must fail before any writes") + } +} + +// TestInitUpdate_PrunesExtraneousVendoredFiles pins the re-sync promise: +// after `talm init --update`, the talm-owned charts/talm/ tree matches the +// embedded library exactly. Without pruning, an extraneous file (.DS_Store, +// editor backup, a file a newer library dropped) survives every update and +// keeps the drift warning — or the strictCharts hard error — permanently +// on, while the remediation hint keeps pointing at the very command that +// cannot clear it. +func TestInitUpdate_PrunesExtraneousVendoredFiles(t *testing.T) { + imageOrig := initCmdFlags.image + presetOrig := initCmdFlags.preset + forceOrig := initCmdFlags.force + rootOrig := Config.RootDir + versionOrig := Config.InitOptions.Version + t.Cleanup(func() { + initCmdFlags.image = imageOrig + initCmdFlags.preset = presetOrig + initCmdFlags.force = forceOrig + Config.RootDir = rootOrig + Config.InitOptions.Version = versionOrig + }) + + initCmdFlags.image = "" + initCmdFlags.preset = "generic" + initCmdFlags.force = true + // An empty version would render "version: " in the rewritten library + // Chart.yaml, which the \S+ normalizer cannot fold to a placeholder — + // masking the pruning assertion behind a bogus Chart.yaml mismatch. + Config.InitOptions.Version = "0.30.0" + + dir := writeVendoredTalmLibrary(t, "0.30.0") + Config.RootDir = dir + + // Step 2 of --update substitutes the cluster name from the existing + // project Chart.yaml. + chartYaml := "apiVersion: v2\nname: test\nversion: 0.1.0\n" + if err := os.WriteFile(filepath.Join(dir, "Chart.yaml"), []byte(chartYaml), 0o644); err != nil { + t.Fatalf("seed Chart.yaml: %v", err) + } + + stray := filepath.Join(dir, "charts", "talm", ".DS_Store") + if err := os.WriteFile(stray, []byte{0x00, 0x01}, 0o644); err != nil { + t.Fatalf("write extraneous file: %v", err) + } + + // A whole stray subtree — e.g. a directory a newer library release + // dropped — must disappear as well, including the emptied directory. + strayDir := filepath.Join(dir, "charts", "talm", "templates", "dropped") + if err := os.MkdirAll(strayDir, 0o755); err != nil { + t.Fatalf("mkdir stray dir: %v", err) + } + if err := os.WriteFile(filepath.Join(strayDir, "leftover.tpl"), []byte("{{- /* gone upstream */ -}}\n"), 0o644); err != nil { + t.Fatalf("write stray subtree file: %v", err) + } + + if err := updateTalmLibraryChart(); err != nil { + t.Fatalf("updateTalmLibraryChart: %v", err) + } + + if _, err := os.Stat(stray); !os.IsNotExist(err) { + t.Error("extraneous charts/talm/.DS_Store survived init --update; the talm-owned tree must be re-synced exactly") + } + + if _, err := os.Stat(strayDir); !os.IsNotExist(err) { + t.Error("emptied stray directory survived init --update; the talm-owned tree must match the embedded library exactly") + } + + drift, msg, err := CheckChartDrift(dir, "0.30.0") + if err != nil { + t.Fatalf("CheckChartDrift after update: %v", err) + } + + if drift { + t.Errorf("drift not cleared by init --update: %s", msg) + } +} + // TestValidateImageRefShape_RejectsMalformed pins the shape check // that runs before any --image value reaches the preset rewrite. // Catches operator typos (::malformed, no-slash:tag, trailing diff --git a/pkg/commands/root.go b/pkg/commands/root.go index 802f90e1..f149e6c2 100644 --- a/pkg/commands/root.go +++ b/pkg/commands/root.go @@ -42,7 +42,12 @@ var GlobalArgs global.Args var Config struct { RootDir string RootDirExplicit bool // true if --root was explicitly set - GlobalOptions struct { + // StrictCharts turns vendored-chart drift into a hard error instead of a + // warning. Opt-in per project via Chart.yaml (strictCharts: true) so a + // whole team/CI inherits it; absent means a warning only (the historical + // behavior). The --strict-charts flag forces it on for a single run. + StrictCharts bool `yaml:"strictCharts"` + GlobalOptions struct { Talosconfig string `yaml:"talosconfig"` Kubeconfig string `yaml:"kubeconfig"` } `yaml:"globalOptions"` diff --git a/pkg/generated/presets.go b/pkg/generated/presets.go index f5eaea3a..1b467f1e 100644 --- a/pkg/generated/presets.go +++ b/pkg/generated/presets.go @@ -13,3 +13,21 @@ func PresetFiles() (map[string]string, error) { func AvailablePresets() ([]string, error) { return charts.AvailablePresets() } + +// TalmLibraryFiles returns the embedded talm library chart keyed relative to +// the talm/ root, with Chart.yaml metadata normalized to %s placeholders. +func TalmLibraryFiles() (map[string]string, error) { + return charts.TalmLibraryFiles() +} + +// HashChartFiles returns a deterministic, order-independent digest of a chart +// tree described as a path→content map. +func HashChartFiles(files map[string]string) string { + return charts.HashChartFiles(files) +} + +// NormalizeChartMeta rewrites a Chart.yaml's name/version lines to %s +// placeholders; non-Chart.yaml files pass through unchanged. +func NormalizeChartMeta(base, content string) string { + return charts.NormalizeChartMeta(base, content) +}