Skip to content

Commit 45cafe5

Browse files
committed
feat(version): warn when vendored charts drift from the talm binary
talm renders from the project's vendored charts/talm/, never the binary's built-in charts, so upgrading the binary leaves the library frozen at the version that last ran init — drift that was previously invisible. On every config-loading command a release build now compares the two and, by default, prints a non-fatal warning to stderr; stdout and the exit code are unchanged. A project (strictCharts: true in Chart.yaml) or an operator (--strict-charts) can promote the warning to a hard error raised before the command body runs. The build version is interpreted through releaseVersion, which accepts both release forms — goreleaser injects it without a leading "v" (0.30.0), the Makefile's `git describe --tags` keeps it (v0.30.0) — and treats only "dev"/empty as a non-release. Gating on the "v" prefix alone would disable the check on every downloaded release and stamp the dev sentinel 0.1.0 into the vendored charts at init time; releaseVersion is used in both places. The decision logic is factored into a pure evaluateChartDrift for direct test coverage of the warn/strict/silent branches. The --strict-charts usage string carries no backticks, so pflag does not misrender the bool flag as taking a value. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la>
1 parent dbc2a57 commit 45cafe5

2 files changed

Lines changed: 275 additions & 6 deletions

File tree

main.go

Lines changed: 101 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -44,12 +44,28 @@ const (
4444
// command's Use field and via -X main.cmdNameTalm in build tooling.
4545
const cmdNameTalm = "talm"
4646

47-
// Version is the talm release tag baked at build time via ldflags
48-
// (`-X main.Version=v0.27.0`); the literal value here is the local
49-
// development fallback.
47+
// Version is the talm build version baked in at link time via ldflags
48+
// (`-X main.Version=...`). The two release build paths inject different
49+
// forms: goreleaser strips the leading "v" (e.g. `0.27.0`, from
50+
// `{{.Version}}`) while the Makefile's `git describe --tags` keeps it
51+
// (e.g. `v0.27.0`). releaseVersion normalizes both. The literal "dev"
52+
// here is the local source-build fallback.
5053
//
5154
//nolint:gochecknoglobals // ldflags-injected build version, idiomatic for go release tooling.
52-
var Version = "dev"
55+
var Version = devVersion
56+
57+
// devVersion is the build-version sentinel for a local source build (no
58+
// ldflags injection). releaseVersion treats it as "not a release", so
59+
// release-only behavior such as the chart-drift check stays off.
60+
const devVersion = "dev"
61+
62+
// strictChartsFlag is bound to the --strict-charts persistent flag. When set
63+
// (or when Chart.yaml carries strictCharts: true), a content difference
64+
// between the project's vendored charts/talm/ and the binary's built-in copy
65+
// becomes a hard error instead of a warning.
66+
//
67+
//nolint:gochecknoglobals // cobra persistent flag binds to package-level state, consistent with the rest of this file.
68+
var strictChartsFlag bool
5369

5470
// skipConfigCommands lists commands that should not load Chart.yaml config.
5571
// - init: creates the config, so it doesn't exist yet
@@ -121,6 +137,11 @@ func registerRootFlags(cmd *cobra.Command) {
121137
cmd.PersistentFlags().StringVar(&commands.GlobalArgs.Cluster, "cluster", "", "Cluster to connect to if a proxy endpoint is used.")
122138
cmd.PersistentFlags().BoolVar(&commands.GlobalArgs.SkipVerify, "skip-verify", false, "skip TLS certificate verification (keeps client authentication)")
123139
cmd.PersistentFlags().Bool("version", false, "Print the version number of the application")
140+
// No backticks in this usage string: pflag's UnquoteUsage treats the
141+
// first backtick-quoted word as the flag's value-placeholder name, which
142+
// on a bool flag misrenders --help as `--strict-charts talm init --update`
143+
// (as if it took an argument).
144+
cmd.PersistentFlags().BoolVar(&strictChartsFlag, "strict-charts", false, "fail if the project's vendored charts/talm/ differs from the talm binary's built-in copy (run talm init --update to re-sync)")
124145

125146
// Shell completion for root persistent flags. --nodes /
126147
// --endpoints draw from the in-scope talosconfig contexts.
@@ -185,6 +206,10 @@ func init() {
185206
if err != nil {
186207
return errors.Wrap(err, "error loading configuration")
187208
}
209+
210+
if err := surfaceChartDrift(); err != nil {
211+
return err
212+
}
188213
}
189214

190215
// Ensure talosconfig path is set to project root if not explicitly set via flag
@@ -219,6 +244,72 @@ func init() {
219244
}
220245
}
221246

247+
// releaseVersion interprets the ldflags-injected build version. It returns
248+
// the version with any leading "v" stripped and true for a tagged release
249+
// build, or ("", false) for a dev/source build. Both release build paths
250+
// must be accepted: goreleaser injects "0.30.0" (no "v") and the Makefile's
251+
// `git describe --tags` injects "v0.30.0". Gating on the "v" prefix alone
252+
// would silently disable release-only behavior on the goreleaser artifacts
253+
// users actually download.
254+
func releaseVersion(raw string) (string, bool) {
255+
if raw == "" || raw == devVersion {
256+
return "", false
257+
}
258+
259+
return strings.TrimPrefix(raw, "v"), true
260+
}
261+
262+
// evaluateChartDrift decides the drift outcome for a build. It returns
263+
// (warning, error): a non-empty warning to print to stderr, or a non-nil
264+
// error to abort the command (strict mode), or both empty for the silent
265+
// cases. Taking the version, project root, and strict flag as arguments
266+
// keeps the warn-vs-fail decision pure (modulo the filesystem read inside
267+
// CheckChartDrift) so it is unit-testable without the package globals.
268+
//
269+
// Cases: dev/source build → silent (embedded charts are a moving target the
270+
// developer controls); drift-check I/O error → non-fatal warning (best
271+
// effort, never blocks the command); drift + strict → hard error with a
272+
// remediation hint; drift + non-strict → warning; no drift → silent.
273+
func evaluateChartDrift(rawVersion, rootDir string, strict bool) (string, error) {
274+
version, ok := releaseVersion(rawVersion)
275+
if !ok {
276+
return "", nil
277+
}
278+
279+
drift, msg, err := commands.CheckChartDrift(rootDir, version)
280+
281+
switch {
282+
case err != nil:
283+
return fmt.Sprintf("could not check chart drift: %v", err), nil
284+
case drift && strict:
285+
//nolint:wrapcheck // originating error built with errors.New; WithHint adds operator-facing guidance and is the project idiom.
286+
return "", errors.WithHint(
287+
errors.New(msg),
288+
"run `talm init --update --preset <preset>`, or unset strictCharts / drop --strict-charts to downgrade this to a warning",
289+
)
290+
case drift:
291+
return msg, nil
292+
default:
293+
return "", nil
294+
}
295+
}
296+
297+
// surfaceChartDrift wires evaluateChartDrift to the package globals and
298+
// emits the warning to stderr. The strict input is the OR of the committed
299+
// Chart.yaml field and the per-run flag.
300+
func surfaceChartDrift() error {
301+
warning, err := evaluateChartDrift(Version, commands.Config.RootDir, commands.Config.StrictCharts || strictChartsFlag)
302+
if err != nil {
303+
return err
304+
}
305+
306+
if warning != "" {
307+
fmt.Fprintf(os.Stderr, "WARN: %s\n", warning)
308+
}
309+
310+
return nil
311+
}
312+
222313
func initConfig() {
223314
if len(os.Args) < 2 {
224315
return
@@ -236,8 +327,12 @@ func initConfig() {
236327
}
237328

238329
if strings.HasPrefix(cmd.Use, initSubcommandName) {
239-
if strings.HasPrefix(Version, "v") {
240-
commands.Config.InitOptions.Version = strings.TrimPrefix(Version, `v`)
330+
// Stamp the real release version into the vendored charts; fall back
331+
// to the dev sentinel for source builds. Gating on the "v" prefix
332+
// here would stamp "0.1.0" on every goreleaser release (which injects
333+
// the version without "v").
334+
if version, ok := releaseVersion(Version); ok {
335+
commands.Config.InitOptions.Version = version
241336
} else {
242337
commands.Config.InitOptions.Version = "0.1.0"
243338
}

main_test.go

Lines changed: 174 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,9 @@ import (
88

99
"github.com/cockroachdb/errors"
1010
"github.com/cozystack/talm/pkg/commands"
11+
"github.com/cozystack/talm/pkg/generated"
1112
"github.com/spf13/cobra"
13+
"github.com/spf13/pflag"
1214
)
1315

1416
// buildCommandHierarchy creates a cobra command hierarchy from a path like
@@ -217,6 +219,144 @@ func TestRegisterRootFlags_NodesHasNoShorthand(t *testing.T) {
217219
}
218220
}
219221

222+
// TestRegisterRootFlags_StrictChartsRendersWithoutValuePlaceholder pins that
223+
// --strict-charts renders in --help as a plain bool flag, not as one taking
224+
// an argument. pflag's UnquoteUsage treats the first backtick-quoted word in
225+
// a flag's usage string as the flag's value-placeholder name; a backtick in
226+
// the --strict-charts usage made --help show `--strict-charts talm init
227+
// --update`, as if the bool flag took a string argument.
228+
func TestRegisterRootFlags_StrictChartsRendersWithoutValuePlaceholder(t *testing.T) {
229+
snapshotConfigState(t)
230+
231+
cmd := &cobra.Command{Use: "talm-test"}
232+
registerRootFlags(cmd)
233+
234+
flag := cmd.PersistentFlags().Lookup("strict-charts")
235+
if flag == nil {
236+
t.Fatal("expected --strict-charts to be registered, got nil")
237+
}
238+
239+
name, _ := pflag.UnquoteUsage(flag)
240+
if name != "" {
241+
t.Errorf("--strict-charts (a bool flag) renders with value placeholder %q; remove backticks from its usage string", name)
242+
}
243+
}
244+
245+
// writeDriftedTalmProject creates a project whose vendored charts/talm/
246+
// cannot match the binary's embedded library (the helpers template carries
247+
// a local edit), so CheckChartDrift reports drift.
248+
func writeDriftedTalmProject(t *testing.T) string {
249+
t.Helper()
250+
251+
root := t.TempDir()
252+
dir := filepath.Join(root, "charts", "talm", "templates")
253+
if err := os.MkdirAll(dir, 0o755); err != nil {
254+
t.Fatalf("mkdir: %v", err)
255+
}
256+
if err := os.WriteFile(filepath.Join(root, "charts", "talm", "Chart.yaml"),
257+
[]byte("apiVersion: v2\nname: talm\nversion: 0.30.0\ntype: library\n"), 0o644); err != nil {
258+
t.Fatalf("write Chart.yaml: %v", err)
259+
}
260+
if err := os.WriteFile(filepath.Join(dir, "_helpers.tpl"),
261+
[]byte("{{- /* drifted local edit, not the embedded copy */ -}}\n"), 0o644); err != nil {
262+
t.Fatalf("write helpers: %v", err)
263+
}
264+
265+
return root
266+
}
267+
268+
// writeMatchingTalmProject materializes a vendored charts/talm/ that is
269+
// byte-identical to the embedded library (stamping version into Chart.yaml),
270+
// so CheckChartDrift reports no drift.
271+
func writeMatchingTalmProject(t *testing.T, version string) string {
272+
t.Helper()
273+
274+
files, err := generated.TalmLibraryFiles()
275+
if err != nil {
276+
t.Fatalf("TalmLibraryFiles: %v", err)
277+
}
278+
279+
root := t.TempDir()
280+
for rel, content := range files {
281+
if filepath.Base(rel) == "Chart.yaml" {
282+
content = strings.ReplaceAll(content, "name: %s", "name: talm")
283+
content = strings.ReplaceAll(content, "version: %s", "version: "+version)
284+
}
285+
286+
dest := filepath.Join(root, "charts", "talm", filepath.FromSlash(rel))
287+
if err := os.MkdirAll(filepath.Dir(dest), 0o755); err != nil {
288+
t.Fatalf("mkdir: %v", err)
289+
}
290+
if err := os.WriteFile(dest, []byte(content), 0o644); err != nil {
291+
t.Fatalf("write: %v", err)
292+
}
293+
}
294+
295+
return root
296+
}
297+
298+
func hintsContain(err error, substr string) bool {
299+
for _, hint := range errors.GetAllHints(err) {
300+
if strings.Contains(hint, substr) {
301+
return true
302+
}
303+
}
304+
305+
return false
306+
}
307+
308+
// TestEvaluateChartDrift pins the warn-vs-fail decision that drives the
309+
// user-facing behavior: strict mode aborts with a remediation hint,
310+
// non-strict drift warns without blocking, a dev build is silent even with
311+
// drift present, and a content-identical library never trips on a
312+
// version-only difference.
313+
func TestEvaluateChartDrift(t *testing.T) {
314+
t.Run("strict drift returns a hard error hinting at init --update", func(t *testing.T) {
315+
root := writeDriftedTalmProject(t)
316+
317+
warning, err := evaluateChartDrift("0.30.0", root, true)
318+
if err == nil {
319+
t.Fatal("expected a strict-mode error for a drifted project")
320+
}
321+
if warning != "" {
322+
t.Errorf("strict error path must not also emit a warning, got %q", warning)
323+
}
324+
if !hintsContain(err, "talm init --update --preset") {
325+
t.Errorf("strict-mode error must hint at `talm init --update --preset`, hints: %v", errors.GetAllHints(err))
326+
}
327+
})
328+
329+
t.Run("non-strict drift warns without blocking", func(t *testing.T) {
330+
root := writeDriftedTalmProject(t)
331+
332+
warning, err := evaluateChartDrift("0.30.0", root, false)
333+
if err != nil {
334+
t.Fatalf("non-strict drift must not block the command: %v", err)
335+
}
336+
if !strings.Contains(warning, "charts/talm") {
337+
t.Errorf("expected a drift warning mentioning charts/talm, got %q", warning)
338+
}
339+
})
340+
341+
t.Run("dev build is silent even with drift present and strict set", func(t *testing.T) {
342+
root := writeDriftedTalmProject(t)
343+
344+
warning, err := evaluateChartDrift("dev", root, true)
345+
if err != nil || warning != "" {
346+
t.Errorf("dev build must be a no-op, got warning=%q err=%v", warning, err)
347+
}
348+
})
349+
350+
t.Run("matching library is silent under strict mode despite a newer binary version", func(t *testing.T) {
351+
root := writeMatchingTalmProject(t, "0.30.0")
352+
353+
warning, err := evaluateChartDrift("0.31.0", root, true)
354+
if err != nil || warning != "" {
355+
t.Errorf("a content-identical library must not drift on a version-only difference, got warning=%q err=%v", warning, err)
356+
}
357+
})
358+
}
359+
220360
func TestSkipConfigCommands(t *testing.T) {
221361
tests := []struct {
222362
name string
@@ -283,3 +423,37 @@ func TestSkipConfigCommands(t *testing.T) {
283423
})
284424
}
285425
}
426+
427+
// TestReleaseVersion pins that both release build paths are recognized.
428+
// goreleaser injects the version WITHOUT the "v" prefix (`-X
429+
// main.Version={{.Version}}` → "0.30.0") while the Makefile's `git describe
430+
// --tags` keeps it ("v0.30.0"). A previous gate accepted only the
431+
// "v"-prefixed form, which silently disabled the chart-drift check and the
432+
// init version stamp on every downloaded release. Both forms must parse to
433+
// the same version and report isRelease=true; dev/empty builds must not.
434+
func TestReleaseVersion(t *testing.T) {
435+
tests := []struct {
436+
name string
437+
raw string
438+
wantVersion string
439+
wantRelease bool
440+
}{
441+
{"goreleaser form (no v)", "0.30.0", "0.30.0", true},
442+
{"makefile form (with v)", "v0.30.0", "0.30.0", true},
443+
{"dev source build", "dev", "", false},
444+
{"empty version", "", "", false},
445+
{"prerelease no v", "0.30.0-rc.1", "0.30.0-rc.1", true},
446+
}
447+
448+
for _, tt := range tests {
449+
t.Run(tt.name, func(t *testing.T) {
450+
version, isRelease := releaseVersion(tt.raw)
451+
if isRelease != tt.wantRelease {
452+
t.Errorf("releaseVersion(%q) isRelease = %v, want %v", tt.raw, isRelease, tt.wantRelease)
453+
}
454+
if version != tt.wantVersion {
455+
t.Errorf("releaseVersion(%q) version = %q, want %q", tt.raw, version, tt.wantVersion)
456+
}
457+
})
458+
}
459+
}

0 commit comments

Comments
 (0)