diff --git a/README.md b/README.md index 2000eb8..47ba661 100644 --- a/README.md +++ b/README.md @@ -89,7 +89,9 @@ Usage: helm-values schema [flags] chart_dir [...chart_dir] Flags: + --check check that values.schema.json is up to date and that Chart.yaml's values-schema annotation references the chart's current version, without writing changes (exit non-zero if not) --dry-run don't write changes to disk + --git-add stage changes with git add (useful for pre-commit hooks) -h, --help help for schema --log-level string log level (debug, info, warn, error, fatal, panic) (default "warn") --stdout write to stdout @@ -121,11 +123,14 @@ Usage: helm-values docs [flags] chart_dir [...chart_dir] Flags: + --check check that the rendered docs file is up to date, without writing changes (exit non-zero if not) --dry-run don't write changes to disk --extra-templates string glob path to extra templates + --git-add stage changes with git add (useful for pre-commit hooks) -h, --help help for docs --log-level string log level (debug, info, warn, error, fatal, panic) (default "warn") --markup string markup language (md, markdown, rst, restructuredtext) + --order string order of values (preserve, alphabetical) (default "preserve") --output string path to output (defaults to README.md or README.rst based on markup) --stdout write to stdout --strict fail on doc comment parsing errors @@ -471,6 +476,8 @@ and [helm-docs](https://github.com/norwoodj/helm-docs). - [x] Pre-Commit Hook support - [x] Values modeline support - 0.4.0 + - [x] `schema`/`docs` `--check` flag for CI validation +- 0.5.0 - [ ] Schema Generation - [ ] Warn on ignored jsonschema property (in cases of $ref/$schema usage) - [ ] Json-Schema Draft 7 support? @@ -482,14 +489,14 @@ and [helm-docs](https://github.com/norwoodj/helm-docs). - [ ] Support "Deprecated" indicator - [ ] Template: Chart Values - [x] Values groups -- 0.5.0 +- 0.6.0 - [ ] Schema Generation - [ ] Support declaring and using yaml anchors in doc comments - [ ] Root level one-of/any-of/all-of - [ ] Docs Generation - [ ] TODO: Detect recursive templates - [ ] TODO: markdown/rst escaping -- 0.6.0 +- 0.7.0 - [ ] Template: Chart Dependencies (defined in Chart.yaml) - some day - [ ] validate examples against schema diff --git a/cmd/helm-values/internal/config/docs.go b/cmd/helm-values/internal/config/docs.go index 8e5851c..b9b7c3b 100644 --- a/cmd/helm-values/internal/config/docs.go +++ b/cmd/helm-values/internal/config/docs.go @@ -125,10 +125,13 @@ func (c *DocsConfig) BindFlags(cmd *cobra.Command) error { cmd.Flags().String("output", "", "path to output (defaults to README.md or README.rst based on markup)") cmd.Flags().String("template", "", "path to template (defaults to README.md.tmpl or README.rst.tmpl based on markup)") cmd.Flags().String("extra-templates", "", "glob path to extra templates") + cmd.Flags().Bool("check", false, + "check that the rendered docs file is up to date, without writing "+ + "changes (exit non-zero if not)") for _, name := range []string{ "stdout", "git-add", "strict", "dry-run", logLevelFlag, "markup", - "order", "use-default", "output", "template", "extra-templates", + "order", "use-default", "output", "template", "extra-templates", "check", } { if err := bindFlag(c.Viper, cmd, name); err != nil { return err @@ -166,6 +169,7 @@ func (c *DocsConfig) ToPackageConfig() (*docs.Config, error) { Strict: c.GetBool("strict"), DryRun: c.GetBool("dry-run"), GitAdd: c.GetBool("git-add"), + Check: c.GetBool("check"), UseDefault: c.UseDefault(), Output: c.Output(), Template: c.GetString("template"), diff --git a/cmd/helm-values/internal/config/schema.go b/cmd/helm-values/internal/config/schema.go index ef7e125..1ee9799 100644 --- a/cmd/helm-values/internal/config/schema.go +++ b/cmd/helm-values/internal/config/schema.go @@ -54,9 +54,13 @@ func (c *SchemaConfig) BindFlags(cmd *cobra.Command) error { cmd.Flags().Bool("dry-run", false, "don't write changes to disk") cmd.Flags().String(logLevelFlag, "warn", "log level (debug, info, warn, error, fatal, panic)") cmd.Flags().Bool("write-modeline", true, "write modeline to values file") + cmd.Flags().Bool("check", false, + "check that values.schema.json is up to date and that Chart.yaml's "+ + "values-schema annotation references the chart's current version, "+ + "without writing changes (exit non-zero if not)") for _, name := range []string{ - "stdout", "strict", "git-add", "dry-run", logLevelFlag, "write-modeline", + "stdout", "strict", "git-add", "dry-run", logLevelFlag, "write-modeline", "check", } { if err := bindFlag(c.Viper, cmd, name); err != nil { return err @@ -79,6 +83,7 @@ func (c *SchemaConfig) ToPackageConfig() (*schema.Config, error) { DryRun: c.GetBool("dry-run"), GitAdd: c.GetBool("git-add"), WriteModeline: c.GetBool("write-modeline"), + Check: c.GetBool("check"), LogLevel: logLevel, } diff --git a/pkg/docs/config.go b/pkg/docs/config.go index fb0df31..6dbbab1 100644 --- a/pkg/docs/config.go +++ b/pkg/docs/config.go @@ -19,6 +19,7 @@ type Config struct { Strict bool DryRun bool GitAdd bool + Check bool UseDefault mo.Option[bool] Output mo.Option[string] Template string diff --git a/pkg/docs/docs.go b/pkg/docs/docs.go index 75d2db1..4e56c4a 100644 --- a/pkg/docs/docs.go +++ b/pkg/docs/docs.go @@ -37,13 +37,32 @@ func GenerateDocs(logger *logrus.Logger, cfg *Config, chartDirs []string) error } // Iterate through plans, generating the docs for each + var problems []string + for _, plan := range plans { - if err := generateChartDoc(logger, cfg, staticPaths, plan); err != nil { + chartProblems, err := generateChartDoc(logger, cfg, staticPaths, plan) + if err != nil { return err } + + problems = append(problems, chartProblems...) + } + + return reportProblems(problems) +} + +// reportProblems prints every check problem found and, if any were found, +// returns an error summarizing how many. +func reportProblems(problems []string) error { + if len(problems) == 0 { + return nil + } + + for _, p := range problems { + fmt.Fprintln(os.Stderr, p) } - return nil + return fmt.Errorf("check failed: %d problem(s) found", len(problems)) } // collectPlans builds a Plan for each discovered chart, logging its details and @@ -69,18 +88,18 @@ func collectPlans(logger *logrus.Logger, cfg *Config, chartsFound []*charts.Char return plans, nil } -// generateChartDoc renders and writes the documentation for a single chart's plan, -// using staticPaths and cfg.ExtraTemplates as additional templates. -func generateChartDoc(logger *logrus.Logger, cfg *Config, staticPaths []string, plan *Plan) error { +// generateChartDoc renders the documentation for a single chart's plan, using staticPaths +// and cfg.ExtraTemplates as additional templates, then either writes or checks it +// depending on the plan's configuration. The returned problems are non-empty only when +// plan.Check() is set and the rendered content doesn't match what's on disk. +func generateChartDoc(logger *logrus.Logger, cfg *Config, staticPaths []string, plan *Plan) ([]string, error) { logger.Infof("docs: %s: starting generation", plan.Chart().Details.Name) logger.Debugf("docs: %s: reading values file", plan.Chart().Details.Name) jsonschema, err := schema.NewGenerator(logger, plan.SchemaPlan()).Generate() if err != nil { - logger.Error(err.Error()) - - return nil + return nil, fmt.Errorf("docs: %s: %w", plan.Chart().Details.Name, err) } logger.Tracef("docs: %s: jsonschema properties: %+v", plan.Chart().Details.Name, jsonschema.Properties) @@ -95,7 +114,7 @@ func generateChartDoc(logger *logrus.Logger, cfg *Config, staticPaths []string, t, err := buildChartTemplate(logger, cfg, staticPaths, plan) if err != nil { - return err + return nil, err } buf := new(bytes.Buffer) @@ -104,18 +123,22 @@ func generateChartDoc(logger *logrus.Logger, cfg *Config, staticPaths []string, err = t.Execute(buf, table) if err != nil { - return fmt.Errorf("rendering template: %w", err) + return nil, fmt.Errorf("rendering template: %w", err) + } + + if plan.Check() { + return plan.CheckReadme(buf.String()) } logger.Debugf("docs: %s: writing output", plan.Chart().Details.Name) if err := plan.WriteReadme(logger, buf.String()); err != nil { - return err + return nil, err } logger.Infof("docs: %s: finished", plan.Chart().Details.Name) - return nil + return nil, nil } // buildChartTemplate collects the static, extra, and (if applicable) custom template diff --git a/pkg/docs/plan.go b/pkg/docs/plan.go index 39fa235..eb45478 100644 --- a/pkg/docs/plan.go +++ b/pkg/docs/plan.go @@ -31,6 +31,7 @@ func NewPlan(cfg *Config, chart *charts.Chart) *Plan { DryRun: cfg.DryRun, GitAdd: cfg.GitAdd, WriteModeline: false, + Check: false, LogLevel: cfg.LogLevel, } schemaPlan := schema.NewPlan(schemaCfg, chart) @@ -103,6 +104,37 @@ func (p *Plan) DryRun() bool { return p.cfg.DryRun } +// Check reports whether the plan should verify (rather than write) the +// chart's rendered documentation. +func (p *Plan) Check() bool { + return p.cfg.Check +} + +// CheckReadme reports whether the plan's rendered content matches what's +// currently on disk. An empty, nil-error result means it's up to date. +func (p *Plan) CheckReadme(content string) ([]string, error) { + outputPath, err := p.DocsOutputPath() + if err != nil { + return nil, err + } + + // outputPath is derived from the chart's own configured/derived README path + // (or a user-supplied --output flag), not attacker-controlled input. + //nolint:gosec // outputPath is a user-opted-in, non-attacker-controlled file path + existing, err := os.ReadFile(outputPath) + // A missing or unreadable output file is itself a check problem (stale/missing docs), + // not a fatal error, so it's intentionally not propagated as one. + //nolint:nilerr // see comment above + if err != nil || string(existing) != content { + return []string{fmt.Sprintf( + "%s is stale -- run `helm-values docs %s` and commit the result", + outputPath, p.chart.RootPath(), + )}, nil + } + + return nil, nil +} + // DocsTargetTemplate resolves the template path to render, returning whether the // built-in default template should be used, or an error if no template can be found. func (p *Plan) DocsTargetTemplate() (string, bool, error) { diff --git a/pkg/docs/plan_test.go b/pkg/docs/plan_test.go new file mode 100644 index 0000000..b65f59f --- /dev/null +++ b/pkg/docs/plan_test.go @@ -0,0 +1,72 @@ +package docs_test + +import ( + "os" + "path/filepath" + "testing" + + "helmvalues/pkg/charts" + "helmvalues/pkg/docs" + "helmvalues/pkg/docs/templates" + + "github.com/samber/mo" + "github.com/sirupsen/logrus" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func strPtr(s string) *string { return &s } + +func TestPlanCheckReadme(t *testing.T) { + t.Parallel() + + const rendered = "# test-chart\n\nrendered content\n" + + tests := []struct { + name string + existingFile *string + wantProblems int + }{ + {name: "readme matches", existingFile: strPtr(rendered), wantProblems: 0}, + {name: "readme stale", existingFile: strPtr("old content\n"), wantProblems: 1}, + {name: "readme missing", existingFile: nil, wantProblems: 1}, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + dir := t.TempDir() + chartYaml := "name: test-chart\ndescription: a test chart\nversion: 1.0.0\n" + require.NoError(t, os.WriteFile(filepath.Join(dir, "Chart.yaml"), []byte(chartYaml), 0o600)) + require.NoError(t, os.WriteFile(filepath.Join(dir, "values.yaml"), []byte("foo: bar\n"), 0o600)) + + chart, err := charts.NewChart(dir) + require.NoError(t, err) + + if tc.existingFile != nil { + require.NoError(t, os.WriteFile(chart.ReadmeMdFilePath(), []byte(*tc.existingFile), 0o600)) + } + + cfg := &docs.Config{ + LogLevel: logrus.WarnLevel, + StdOut: false, + Strict: false, + DryRun: false, + GitAdd: false, + Check: true, + UseDefault: mo.None[bool](), + Output: mo.None[string](), + Template: "", + ExtraTemplates: nil, + Markup: mo.None[templates.Markup](), + Order: docs.ValuesOrderPreserve, + } + plan := docs.NewPlan(cfg, chart) + + problems, err := plan.CheckReadme(rendered) + require.NoError(t, err) + assert.Len(t, problems, tc.wantProblems) + }) + } +} diff --git a/pkg/schema/config.go b/pkg/schema/config.go index 5319bb0..795dfec 100644 --- a/pkg/schema/config.go +++ b/pkg/schema/config.go @@ -11,5 +11,6 @@ type Config struct { DryRun bool GitAdd bool WriteModeline bool + Check bool LogLevel logrus.Level } diff --git a/pkg/schema/plan.go b/pkg/schema/plan.go index 079804b..12ba34c 100644 --- a/pkg/schema/plan.go +++ b/pkg/schema/plan.go @@ -1,11 +1,13 @@ package schema import ( + "bytes" "context" "encoding/json" "fmt" "os" "os/exec" + "strings" "helmvalues/pkg" "helmvalues/pkg/charts" @@ -75,6 +77,45 @@ func (p *Plan) DryRun() bool { return p.cfg.DryRun } +// Check reports whether the plan should verify (rather than write) the +// chart's generated schema and Chart.yaml metadata. +func (p *Plan) Check() bool { + return p.cfg.Check +} + +// CheckSchema reports problems with the chart's on-disk schema and +// Chart.yaml metadata: whether values.schema.json matches the freshly +// generated schema, and whether the values-schema annotation references +// the chart's current version. An empty result means everything is up to +// date. +func (p *Plan) CheckSchema(schema *pkg.JsonSchema) []string { + var problems []string + + content, err := json.MarshalIndent(schema, "", " ") + if err != nil { + return []string{fmt.Sprintf("%s: failed to marshal generated schema: %s", p.chart.Details.Name, err)} + } + + existing, err := os.ReadFile(p.chart.SchemaFilePath()) + if err != nil || !bytes.Equal(existing, content) { + problems = append(problems, fmt.Sprintf( + "%s is stale -- run `helm-values schema %s` and commit the result", + p.chart.SchemaFilePath(), p.chart.RootPath(), + )) + } + + if schemaURL := p.chart.Details.ValuesSchema(); schemaURL != "" { + if !strings.Contains(schemaURL, p.chart.Details.Version) { + problems = append(problems, fmt.Sprintf( + "%s: values-schema annotation does not reference current chart version %s (found: %s)", + p.chart.ChartFilePath(), p.chart.Details.Version, schemaURL, + )) + } + } + + return problems +} + // WriteSchema encodes and writes the generated schema per the plan's config. func (p *Plan) WriteSchema(_ *logrus.Logger, schema *pkg.JsonSchema) error { content, err := json.MarshalIndent(schema, "", " ") diff --git a/pkg/schema/plan_test.go b/pkg/schema/plan_test.go new file mode 100644 index 0000000..e812762 --- /dev/null +++ b/pkg/schema/plan_test.go @@ -0,0 +1,118 @@ +package schema_test + +import ( + "encoding/json" + "os" + "path/filepath" + "testing" + + "helmvalues/pkg" + "helmvalues/pkg/charts" + "helmvalues/pkg/schema" + + "github.com/sirupsen/logrus" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// writeTestChart writes a minimal chart to a temp directory, with the given version and +// (optionally empty) annotations block appended to Chart.yaml, and loads it. +func writeTestChart(t *testing.T, version, annotations string) *charts.Chart { + t.Helper() + + dir := t.TempDir() + + chartYaml := "name: test-chart\ndescription: a test chart\nversion: " + version + "\n" + annotations + require.NoError(t, os.WriteFile(filepath.Join(dir, "Chart.yaml"), []byte(chartYaml), 0o600)) + require.NoError(t, os.WriteFile(filepath.Join(dir, "values.yaml"), []byte("foo: bar\n"), 0o600)) + + chart, err := charts.NewChart(dir) + require.NoError(t, err) + + return chart +} + +// testChartVersion is the chart version used across TestPlanCheckSchema's test cases. +const testChartVersion = "1.2.3" + +func TestPlanCheckSchema(t *testing.T) { + t.Parallel() + + testSchema := pkg.NewJsonSchema() + testSchema.Title = "test-chart values" + + content, err := json.MarshalIndent(testSchema, "", " ") + require.NoError(t, err) + + tests := []struct { + name string + version string + annotations string + existingFile []byte + wantProblems int + }{ + { + name: "schema matches, no annotation", + version: testChartVersion, + annotations: "", + existingFile: content, + wantProblems: 0, + }, + { + name: "schema file missing", + version: testChartVersion, + annotations: "", + existingFile: nil, + wantProblems: 1, + }, + { + name: "schema file stale", + version: testChartVersion, + annotations: "", + existingFile: []byte("{}"), + wantProblems: 1, + }, + { + name: "annotation references current version", + version: testChartVersion, + annotations: "annotations:\n" + + " values-schema: https://example.com/refs/tags/helm-chart-" + testChartVersion + "/values.schema.json\n", + existingFile: content, + wantProblems: 0, + }, + { + name: "annotation references stale version", + version: testChartVersion, + annotations: "annotations:\n" + + " values-schema: https://example.com/refs/tags/helm-chart-1.0.0/values.schema.json\n", + existingFile: content, + wantProblems: 1, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + chart := writeTestChart(t, tc.version, tc.annotations) + + if tc.existingFile != nil { + require.NoError(t, os.WriteFile(chart.SchemaFilePath(), tc.existingFile, 0o600)) + } + + cfg := &schema.Config{ + StdOut: false, + Strict: false, + DryRun: false, + GitAdd: false, + WriteModeline: false, + Check: true, + LogLevel: logrus.WarnLevel, + } + plan := schema.NewPlan(cfg, chart) + + problems := plan.CheckSchema(testSchema) + assert.Len(t, problems, tc.wantProblems) + }) + } +} diff --git a/pkg/schema/schema.go b/pkg/schema/schema.go index 500deac..3a051df 100644 --- a/pkg/schema/schema.go +++ b/pkg/schema/schema.go @@ -2,6 +2,7 @@ package schema import ( "fmt" + "os" "helmvalues/pkg/charts" @@ -29,22 +30,26 @@ func GenerateSchema(logger *logrus.Logger, cfg *Config, chartDirs []string) erro } // Iterate through plans again, this time generating the schema + var problems []string + for _, plan := range plans { logger.Infof("schema: %s: starting generation", plan.Chart().Details.Name) schema, err := NewGenerator(logger, plan).Generate() if err != nil { - logger.Error(err.Error()) + return fmt.Errorf("schema: %s: %w", plan.Chart().Details.Name, err) + } + + if plan.Check() { + problems = append(problems, plan.CheckSchema(schema)...) - return nil + continue } logger.Debugf("schema: %s: writing output", plan.Chart().Details.Name) if err := plan.WriteSchema(logger, schema); err != nil { - logger.Error(err.Error()) - - return nil + return fmt.Errorf("schema: %s: %w", plan.Chart().Details.Name, err) } if cfg.WriteModeline { @@ -57,9 +62,7 @@ func GenerateSchema(logger *logrus.Logger, cfg *Config, chartDirs []string) erro plan.DryRun(), ) if err != nil { - logger.Error(err.Error()) - - return nil + return fmt.Errorf("schema: %s: %w", plan.Chart().Details.Name, err) } } else { logger.Debugf("schema: %s: skipping modeline write", plan.Chart().Details.Name) @@ -68,5 +71,19 @@ func GenerateSchema(logger *logrus.Logger, cfg *Config, chartDirs []string) erro logger.Infof("schema: %s: finished", plan.Chart().Details.Name) } - return nil + return reportProblems(problems) +} + +// reportProblems prints every check problem found and, if any were found, +// returns an error summarizing how many. +func reportProblems(problems []string) error { + if len(problems) == 0 { + return nil + } + + for _, p := range problems { + fmt.Fprintln(os.Stderr, p) + } + + return fmt.Errorf("check failed: %d problem(s) found", len(problems)) } diff --git a/taskfile.yaml b/taskfile.yaml index efcda13..eee2b07 100644 --- a/taskfile.yaml +++ b/taskfile.yaml @@ -49,6 +49,7 @@ tasks: desc: Run all end-to-end tests cmds: - task: test:e2e:pre-commit + - task: test:e2e:check test:e2e:pre-commit: desc: Run pre-commit hook tests @@ -56,6 +57,12 @@ tasks: cmds: - bats ./tests.bats + test:e2e:check: + desc: Run schema/docs --check tests + dir: ./tests/check + cmds: + - bats ./tests.bats + clean: desc: Clean build artifacts cmds: diff --git a/tests/check/tests.bats b/tests/check/tests.bats new file mode 100644 index 0000000..b14ce9d --- /dev/null +++ b/tests/check/tests.bats @@ -0,0 +1,84 @@ +#!/usr/bin/env bats + +setup() { + if command -v brew &> /dev/null; then + BREW_PREFIX="$(brew --prefix)" + load "${BREW_PREFIX}/lib/bats-support/load.bash" + load "${BREW_PREFIX}/lib/bats-assert/load.bash" + else + NPM_PREFIX="$(npm config get prefix)" + load "${NPM_PREFIX}/lib/node_modules/bats-support/load.bash" + load "${NPM_PREFIX}/lib/node_modules/bats-assert/load.bash" + fi + + cat > Chart.yaml <<'EOF' +apiVersion: v2 +name: check-test-chart +description: A chart for exercising --check +version: 0.1.0 +annotations: + values-schema: https://example.com/refs/tags/helm-chart-0.1.0/values.schema.json +EOF + + cat > values.yaml <<'EOF' +foo: bar +EOF +} + +teardown() { + rm -f Chart.yaml values.yaml values.schema.json README.md +} + +@test "schema --check fails before the schema has been generated" { + run helm values schema . --check + assert_failure + assert_output --partial "values.schema.json is stale" +} + +@test "schema --check passes once the schema is up to date" { + helm values schema . --write-modeline=false + + run helm values schema . --check + assert_success +} + +@test "schema --check fails when values.yaml changes without regenerating" { + helm values schema . --write-modeline=false + echo "baz: qux" >> values.yaml + + run helm values schema . --check + assert_failure + assert_output --partial "values.schema.json is stale" +} + +@test "schema --check fails when the values-schema annotation doesn't reference the chart version" { + helm values schema . --write-modeline=false + sed -i.bak 's/version: 0.1.0/version: 0.2.0/' Chart.yaml + rm -f Chart.yaml.bak + + run helm values schema . --check + assert_failure + assert_output --partial "does not reference current chart version" +} + +@test "docs --check fails before the docs have been generated" { + run helm values docs . --check + assert_failure + assert_output --partial "README.md is stale" +} + +@test "docs --check passes once the docs are up to date" { + helm values docs . + + run helm values docs . --check + assert_success +} + +@test "docs --check fails when values.yaml changes without regenerating" { + helm values docs . + echo "baz: qux" >> values.yaml + + run helm values docs . --check + assert_failure + assert_output --partial "README.md is stale" +}