From c6502d0ba29a05985de4ad92a7666c44db722d30 Mon Sep 17 00:00:00 2001 From: Brahm Lower Date: Sun, 16 Aug 2026 17:19:03 +0000 Subject: [PATCH 1/2] chore: add golangci-lint with a maximal ruleset Enables every golangci-lint v2 linter (linters.default: all) with a small, justified disable list (deprecated linters, and a handful that fight this codebase's idiomatic style: exhaustruct, err113, noinlineerr, varnamelen, gochecknoglobals, godox), plus tuned settings for depguard, forbidigo, lll, funlen, and tagliatelle. Fixes all 407 findings the ruleset surfaced: missing doc comments, unwrapped errors, gosec justifications, magic numbers, complexity refactors, and test hygiene (parallelization, testify assertions, external test packages). Adds a lint CI job and a `task lint` entry. Co-Authored-By: Claude Sonnet 5 --- .github/workflows/tests.yaml | 16 ++ .golangci.yml | 80 ++++++ cmd/helm-values/internal/config/base.go | 24 ++ cmd/helm-values/internal/config/docs.go | 102 ++++---- cmd/helm-values/internal/config/modeline.go | 49 ++-- cmd/helm-values/internal/config/schema.go | 56 +++-- cmd/helm-values/internal/github.go | 30 ++- cmd/helm-values/internal/precommit.go | 22 +- cmd/helm-values/internal/update.go | 19 +- cmd/helm-values/main.go | 54 ++-- pkg/charts/chart.go | 61 +++-- pkg/charts/search.go | 107 ++++---- pkg/docs/config.go | 9 +- pkg/docs/docs.go | 259 ++++++++++++-------- pkg/docs/plan.go | 117 ++++++--- pkg/docs/templates/markup.go | 14 +- pkg/docs/templates/models.go | 4 + pkg/docs/templates/staticfs.go | 12 +- pkg/docs/templates/template_builder.go | 53 +++- pkg/docs/templates/template_funcs.go | 18 +- pkg/helm/cache.go | 23 +- pkg/helm/chart_details.go | 10 +- pkg/helm/index.go | 23 +- pkg/jsonschema.go | 97 ++++---- pkg/layeredfs.go | 39 ++- pkg/modeline/config.go | 4 +- pkg/modeline/file_modeline_manager.go | 41 +++- pkg/modeline/modeline.go | 56 +++-- pkg/modeline/plan.go | 15 +- pkg/orderedmap.go | 26 +- pkg/schema/comments/comment.go | 114 ++++++--- pkg/schema/comments/comment_test.go | 238 ++++++++++-------- pkg/schema/comments/error.go | 140 ++++++----- pkg/schema/comments/error_test.go | 27 +- pkg/schema/comments/nodes.go | 3 + pkg/schema/config.go | 3 + pkg/schema/generate.go | 157 ++++++++---- pkg/schema/modeline.go | 10 +- pkg/schema/plan.go | 47 ++-- pkg/schema/schema.go | 13 +- taskfile.yaml | 5 + 41 files changed, 1510 insertions(+), 687 deletions(-) create mode 100644 .golangci.yml diff --git a/.github/workflows/tests.yaml b/.github/workflows/tests.yaml index 99a0fcf..555ec53 100644 --- a/.github/workflows/tests.yaml +++ b/.github/workflows/tests.yaml @@ -9,6 +9,22 @@ on: - main jobs: + lint: + runs-on: ubuntu-latest + steps: + - name: Checkout the repo + uses: actions/checkout@v6 + + - name: Set up Go + uses: actions/setup-go@v5 + with: + go-version-file: go.mod + + - name: Run golangci-lint + uses: golangci/golangci-lint-action@v7 + with: + version: v2.12.2 + unit: runs-on: ubuntu-latest steps: diff --git a/.golangci.yml b/.golangci.yml new file mode 100644 index 0000000..9024f22 --- /dev/null +++ b/.golangci.yml @@ -0,0 +1,80 @@ +version: "2" + +linters: + default: all + + disable: + # Deprecated, replaced by wsl_v5 / gomodguard_v2 below. + - wsl + - gomodguard + + # Forces every struct literal (including third-party ones) to set every + # field explicitly. Too invasive for general application code. + - exhaustruct + + # Requires every error to be a package-level sentinel wrapped with %w. + # Fights idiomatic fmt.Errorf("...: %w", err) for one-off dynamic errors. + - err113 + + # Disallows `if err := f(); err != nil {}`, which is standard idiomatic + # Go (recommended by Effective Go) and used throughout this codebase. + - noinlineerr + + # Flags common, readable short names (i, ok, t, tt, f, s) as too short; + # too aggressive for this codebase's style. + - varnamelen + + # main.BuildVersion/BuildCommit/BuildDate/Repository are populated via + # -ldflags at build time and must be package-level vars. + - gochecknoglobals + + # TODO comments here track real, intentional follow-up work; failing CI + # on their presence isn't useful for this project. + - godox + + settings: + depguard: + rules: + main: + files: + - $all + deny: + - pkg: io/ioutil + desc: "io/ioutil is deprecated, use io or os instead" + + forbidigo: + # Only flag the bare print/println builtins (debug leftovers); this is + # a CLI tool where fmt.Print* is legitimate, intentional user output. + forbid: + - pattern: "^(print|println)$" + msg: "use fmt.Print* or a logger instead of the print/println builtins" + + lll: + line-length: 140 + + funlen: + ignore-comments: true + + exclusions: + rules: + # Table-driven tests are naturally long; that's not a complexity smell. + - path: _test\.go + linters: + - funlen + + # These structs mirror external, fixed wire formats (the GitHub + # releases API and the pre-commit hook manifest schema), so their + # struct tags must stay snake_case to match those formats exactly. + - path: cmd/helm-values/internal/(github|precommit)\.go + linters: + - tagliatelle + +formatters: + enable: + - gofmt + - goimports + + settings: + goimports: + local-prefixes: + - helmvalues diff --git a/cmd/helm-values/internal/config/base.go b/cmd/helm-values/internal/config/base.go index fd80441..b449ec8 100644 --- a/cmd/helm-values/internal/config/base.go +++ b/cmd/helm-values/internal/config/base.go @@ -1,14 +1,38 @@ +// Package config binds cobra command flags and environment variables to +// viper, and translates the bound values into the config types each +// helm-values subcommand's underlying package expects. package config import ( + "fmt" "strings" + "github.com/spf13/cobra" "github.com/spf13/viper" ) +// logLevelFlag is the flag/env name shared by every subcommand's log-level +// setting. +const logLevelFlag = "log-level" + func standardViper() *viper.Viper { cfg := viper.New() cfg.AllowEmptyEnv(true) cfg.SetEnvKeyReplacer(strings.NewReplacer(".", "_")) + return cfg } + +// bindFlag binds the cobra flag named name (already registered on cmd) to +// v, along with its environment-variable equivalent. +func bindFlag(v *viper.Viper, cmd *cobra.Command, name string) error { + if err := v.BindPFlag(name, cmd.Flags().Lookup(name)); err != nil { + return fmt.Errorf("binding %s flag: %w", name, err) + } + + if err := v.BindEnv(name); err != nil { + return fmt.Errorf("binding %s env: %w", name, err) + } + + return nil +} diff --git a/cmd/helm-values/internal/config/docs.go b/cmd/helm-values/internal/config/docs.go index 32d7af6..8e5851c 100644 --- a/cmd/helm-values/internal/config/docs.go +++ b/cmd/helm-values/internal/config/docs.go @@ -1,9 +1,11 @@ package config import ( + "fmt" + "path/filepath" + "helmvalues/pkg/docs" "helmvalues/pkg/docs/templates" - "path/filepath" "github.com/samber/mo" "github.com/sirupsen/logrus" @@ -11,24 +13,41 @@ import ( "github.com/spf13/viper" ) +// DocsConfig holds the flag/env-bound configuration for the docs command. +type DocsConfig struct { + *viper.Viper +} + +// NewDocsConfig creates a DocsConfig backed by a fresh viper instance. func NewDocsConfig() *DocsConfig { cfg := standardViper() return &DocsConfig{cfg} } -type DocsConfig struct { - *viper.Viper -} - +// ValuesOrder returns the configured order in which values rows are +// rendered. func (c *DocsConfig) ValuesOrder() (docs.ValuesOrder, error) { - return docs.NewValuesOrder(c.GetString("order")) + order, err := docs.NewValuesOrder(c.GetString("order")) + if err != nil { + return order, fmt.Errorf("parsing values order: %w", err) + } + + return order, nil } +// LogLevel returns the configured log level. func (c *DocsConfig) LogLevel() (logrus.Level, error) { - return logrus.ParseLevel(c.GetString("log-level")) + level, err := logrus.ParseLevel(c.GetString(logLevelFlag)) + if err != nil { + return level, fmt.Errorf("parsing log level: %w", err) + } + + return level, nil } +// ExtraTemplates resolves the configured extra-templates glob into a list +// of matching file paths. func (c *DocsConfig) ExtraTemplates() ([]string, error) { et := c.GetString("extra-templates") if et == "" { @@ -37,37 +56,50 @@ func (c *DocsConfig) ExtraTemplates() ([]string, error) { path, err := filepath.Abs(et) if err != nil { - return nil, err + return nil, fmt.Errorf("resolving extra-templates path: %w", err) + } + + matches, err := filepath.Glob(path) + if err != nil { + return nil, fmt.Errorf("globbing extra-templates path: %w", err) } - return filepath.Glob(path) + return matches, nil } +// Markup returns the configured output markup, if one was set. func (c *DocsConfig) Markup() (mo.Option[templates.Markup], error) { if !c.IsSet("markup") { return mo.None[templates.Markup](), nil } + markup, err := templates.MarkupFromString(c.GetString("markup")) if err != nil { return mo.None[templates.Markup](), err } + return mo.Some(markup), nil } +// UseDefault returns the configured use-default flag, if it was set. func (c *DocsConfig) UseDefault() mo.Option[bool] { if !c.IsSet("use-default") { return mo.None[bool]() } + return mo.Some(c.GetBool("use-default")) } +// Output returns the configured output path, if one was set. func (c *DocsConfig) Output() mo.Option[string] { if !c.IsSet("output") { return mo.None[string]() } + return mo.Some(c.GetString("output")) } +// UpdateLogger sets logger's level to the configured log level. func (c *DocsConfig) UpdateLogger(logger *logrus.Logger) error { level, err := c.LogLevel() if err != nil { @@ -75,55 +107,38 @@ func (c *DocsConfig) UpdateLogger(logger *logrus.Logger) error { } logger.SetLevel(level) + return nil } -func (c *DocsConfig) BindFlags(cmd *cobra.Command) { +// BindFlags registers the docs command's flags on cmd and binds them (and +// their environment-variable equivalents) to this config. +func (c *DocsConfig) BindFlags(cmd *cobra.Command) error { cmd.Flags().Bool("stdout", false, "write to stdout") - c.BindPFlag("stdout", cmd.Flags().Lookup("stdout")) - c.BindEnv("stdout") - cmd.Flags().Bool("git-add", false, "stage changes with git add (useful for pre-commit hooks)") - c.BindPFlag("git-add", cmd.Flags().Lookup("git-add")) - c.BindEnv("git-add") - cmd.Flags().Bool("strict", false, "fail on doc comment parsing errors") - c.BindPFlag("strict", cmd.Flags().Lookup("strict")) - c.BindEnv("strict") - cmd.Flags().Bool("dry-run", false, "don't write changes to disk") - c.BindPFlag("dry-run", cmd.Flags().Lookup("dry-run")) - c.BindEnv("dry-run") - - cmd.Flags().String("log-level", "warn", "log level (debug, info, warn, error, fatal, panic)") - c.BindPFlag("log-level", cmd.Flags().Lookup("log-level")) - c.BindEnv("log-level") - + cmd.Flags().String(logLevelFlag, "warn", "log level (debug, info, warn, error, fatal, panic)") cmd.Flags().String("markup", "", "markup language (md, markdown, rst, restructuredtext)") - c.BindPFlag("markup", cmd.Flags().Lookup("markup")) - c.BindEnv("markup") - cmd.Flags().String("order", "preserve", "order of values (preserve, alphabetical)") - c.BindPFlag("order", cmd.Flags().Lookup("order")) - c.BindEnv("order") - cmd.Flags().Bool("use-default", true, "uses default template unless a custom template is present") - c.BindPFlag("use-default", cmd.Flags().Lookup("use-default")) - c.BindEnv("use-default") - cmd.Flags().String("output", "", "path to output (defaults to README.md or README.rst based on markup)") - c.BindPFlag("output", cmd.Flags().Lookup("output")) - c.BindEnv("output") - cmd.Flags().String("template", "", "path to template (defaults to README.md.tmpl or README.rst.tmpl based on markup)") - c.BindPFlag("template", cmd.Flags().Lookup("template")) - c.BindEnv("template") - cmd.Flags().String("extra-templates", "", "glob path to extra templates") - c.BindPFlag("extra-templates", cmd.Flags().Lookup("extra-templates")) - c.BindEnv("extra-templates") + + for _, name := range []string{ + "stdout", "git-add", "strict", "dry-run", logLevelFlag, "markup", + "order", "use-default", "output", "template", "extra-templates", + } { + if err := bindFlag(c.Viper, cmd, name); err != nil { + return err + } + } + + return nil } +// ToPackageConfig builds the docs.Config this configuration describes. func (c *DocsConfig) ToPackageConfig() (*docs.Config, error) { logLevel, err := c.LogLevel() if err != nil { @@ -158,5 +173,6 @@ func (c *DocsConfig) ToPackageConfig() (*docs.Config, error) { Markup: markup, Order: valuesOrder, } + return config, nil } diff --git a/cmd/helm-values/internal/config/modeline.go b/cmd/helm-values/internal/config/modeline.go index 43cbf3a..58b4bac 100644 --- a/cmd/helm-values/internal/config/modeline.go +++ b/cmd/helm-values/internal/config/modeline.go @@ -1,6 +1,8 @@ package config import ( + "fmt" + "helmvalues/pkg/helm" "helmvalues/pkg/modeline" @@ -9,20 +11,31 @@ import ( "github.com/spf13/viper" ) +// ModelineConfig holds the flag/env-bound configuration for the modeline +// command. +type ModelineConfig struct { + *viper.Viper +} + +// NewModelineConfig creates a ModelineConfig backed by a fresh viper +// instance. func NewModelineConfig() *ModelineConfig { cfg := standardViper() return &ModelineConfig{cfg} } -type ModelineConfig struct { - *viper.Viper -} - +// LogLevel returns the configured log level. func (c *ModelineConfig) LogLevel() (logrus.Level, error) { - return logrus.ParseLevel(c.GetString("log-level")) + level, err := logrus.ParseLevel(c.GetString(logLevelFlag)) + if err != nil { + return level, fmt.Errorf("parsing log level: %w", err) + } + + return level, nil } +// UpdateLogger sets logger's level to the configured log level. func (c *ModelineConfig) UpdateLogger(logger *logrus.Logger) error { level, err := c.LogLevel() if err != nil { @@ -30,23 +43,28 @@ func (c *ModelineConfig) UpdateLogger(logger *logrus.Logger) error { } logger.SetLevel(level) + return nil } -func (c *ModelineConfig) BindFlags(cmd *cobra.Command) { +// BindFlags registers the modeline command's flags on cmd and binds them +// (and their environment-variable equivalents) to this config. +func (c *ModelineConfig) BindFlags(cmd *cobra.Command) error { cmd.Flags().BoolP("parents", "p", false, "create parent directories if they don't exist") - c.BindPFlag("parents", cmd.Flags().Lookup("parents")) - c.BindEnv("parents") - cmd.Flags().String("version", "", "chart version (for remote charts)") - c.BindPFlag("version", cmd.Flags().Lookup("version")) - c.BindEnv("version") + cmd.Flags().String(logLevelFlag, "warn", "log level (debug, info, warn, error, fatal, panic)") - cmd.Flags().String("log-level", "warn", "log level (debug, info, warn, error, fatal, panic)") - c.BindPFlag("log-level", cmd.Flags().Lookup("log-level")) - c.BindEnv("log-level") + for _, name := range []string{"parents", "version", logLevelFlag} { + if err := bindFlag(c.Viper, cmd, name); err != nil { + return err + } + } + + return nil } +// ToPackageConfig builds the modeline.Config this configuration describes +// for the given chart reference and target file. func (c *ModelineConfig) ToPackageConfig(rawChartRef string, targetFile string) (*modeline.Config, error) { if version := c.GetString("version"); version != "" { rawChartRef = rawChartRef + "@" + version @@ -54,7 +72,7 @@ func (c *ModelineConfig) ToPackageConfig(rawChartRef string, targetFile string) chartRef, err := helm.NewChartRef(rawChartRef) if err != nil { - return nil, err + return nil, fmt.Errorf("parsing chart reference: %w", err) } modelineCfg := &modeline.Config{ @@ -63,5 +81,6 @@ func (c *ModelineConfig) ToPackageConfig(rawChartRef string, targetFile string) CreateParents: c.GetBool("parents"), PartialModeline: modeline.NewPartialModeline("yaml-language-server", "$schema"), } + return modelineCfg, nil } diff --git a/cmd/helm-values/internal/config/schema.go b/cmd/helm-values/internal/config/schema.go index cfe8f26..ef7e125 100644 --- a/cmd/helm-values/internal/config/schema.go +++ b/cmd/helm-values/internal/config/schema.go @@ -1,6 +1,8 @@ package config import ( + "fmt" + "helmvalues/pkg/schema" "github.com/sirupsen/logrus" @@ -8,20 +10,30 @@ import ( "github.com/spf13/viper" ) +// SchemaConfig holds the flag/env-bound configuration for the schema +// command. +type SchemaConfig struct { + *viper.Viper +} + +// NewSchemaConfig creates a SchemaConfig backed by a fresh viper instance. func NewSchemaConfig() *SchemaConfig { cfg := standardViper() return &SchemaConfig{cfg} } -type SchemaConfig struct { - *viper.Viper -} - +// LogLevel returns the configured log level. func (c *SchemaConfig) LogLevel() (logrus.Level, error) { - return logrus.ParseLevel(c.GetString("log-level")) + level, err := logrus.ParseLevel(c.GetString(logLevelFlag)) + if err != nil { + return level, fmt.Errorf("parsing log level: %w", err) + } + + return level, nil } +// UpdateLogger sets logger's level to the configured log level. func (c *SchemaConfig) UpdateLogger(logger *logrus.Logger) error { level, err := c.LogLevel() if err != nil { @@ -29,35 +41,32 @@ func (c *SchemaConfig) UpdateLogger(logger *logrus.Logger) error { } logger.SetLevel(level) + return nil } -func (c *SchemaConfig) BindFlags(cmd *cobra.Command) { +// BindFlags registers the schema command's flags on cmd and binds them +// (and their environment-variable equivalents) to this config. +func (c *SchemaConfig) BindFlags(cmd *cobra.Command) error { cmd.Flags().Bool("stdout", false, "write to stdout") - c.BindPFlag("stdout", cmd.Flags().Lookup("stdout")) - c.BindEnv("stdout") - cmd.Flags().Bool("strict", false, "fail on doc comment parsing errors") - c.BindPFlag("strict", cmd.Flags().Lookup("strict")) - c.BindEnv("strict") - cmd.Flags().Bool("git-add", false, "stage changes with git add (useful for pre-commit hooks)") - c.BindPFlag("git-add", cmd.Flags().Lookup("git-add")) - c.BindEnv("git-add") - cmd.Flags().Bool("dry-run", false, "don't write changes to disk") - c.BindPFlag("dry-run", cmd.Flags().Lookup("dry-run")) - c.BindEnv("dry-run") + 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().String("log-level", "warn", "log level (debug, info, warn, error, fatal, panic)") - c.BindPFlag("log-level", cmd.Flags().Lookup("log-level")) - c.BindEnv("log-level") + for _, name := range []string{ + "stdout", "strict", "git-add", "dry-run", logLevelFlag, "write-modeline", + } { + if err := bindFlag(c.Viper, cmd, name); err != nil { + return err + } + } - cmd.Flags().Bool("write-modeline", true, "write modeline to values file") - c.BindPFlag("write-modeline", cmd.Flags().Lookup("write-modeline")) - c.BindEnv("write-modeline") + return nil } +// ToPackageConfig builds the schema.Config this configuration describes. func (c *SchemaConfig) ToPackageConfig() (*schema.Config, error) { logLevel, err := c.LogLevel() if err != nil { @@ -72,5 +81,6 @@ func (c *SchemaConfig) ToPackageConfig() (*schema.Config, error) { WriteModeline: c.GetBool("write-modeline"), LogLevel: logLevel, } + return config, nil } diff --git a/cmd/helm-values/internal/github.go b/cmd/helm-values/internal/github.go index e6586ab..722d9fa 100644 --- a/cmd/helm-values/internal/github.go +++ b/cmd/helm-values/internal/github.go @@ -1,30 +1,41 @@ +// Package internal implements the helm-values plugin's supporting +// commands: self-update (via GitHub releases) and pre-commit hook +// installation. package internal import ( + "context" "encoding/json" + "errors" "fmt" "net/http" ) +// GithubAsset is a single downloadable file attached to a GitHub release. type GithubAsset struct { Name string `json:"name"` BrowserDownloadURL string `json:"browser_download_url"` } +// GithubRelease is a GitHub release, as returned by the releases API. type GithubRelease struct { TagName string `json:"tag_name"` PublishedAt string `json:"published_at"` Assets []GithubAsset `json:"assets"` } +// PluginURL returns the download URL for this release's helm-values plugin +// archive. func (r *GithubRelease) PluginURL() (string, error) { asset, err := r.ReleaseArtifact() if err != nil { return "", err } + return asset.BrowserDownloadURL, nil } +// ReleaseArtifact finds this release's helm-values plugin archive asset. func (r *GithubRelease) ReleaseArtifact() (*GithubAsset, error) { expectedName := fmt.Sprintf("values-%s.tgz", r.TagName) @@ -34,16 +45,27 @@ func (r *GithubRelease) ReleaseArtifact() (*GithubAsset, error) { } } - return nil, fmt.Errorf("expected asset not found") + return nil, errors.New("expected asset not found") } -func GetLatestRelease(repository string) (*GithubRelease, error) { +// GetLatestRelease fetches the latest GitHub release for repository (in +// "owner/name" form). +func GetLatestRelease(ctx context.Context, repository string) (*GithubRelease, error) { githubReleasesURL := fmt.Sprintf("https://api.github.com/repos/%s/releases/latest", repository) - resp, err := http.Get(githubReleasesURL) + + // githubReleasesURL is built from a fixed, hardcoded GitHub API endpoint + // with the repository slug interpolated (supplied at build time via + // -ldflags, not user input), so this is not an SSRF-style variable URL. + req, err := http.NewRequestWithContext(ctx, http.MethodGet, githubReleasesURL, nil) + if err != nil { + return nil, fmt.Errorf("failed to build request: %w", err) + } + + resp, err := http.DefaultClient.Do(req) if err != nil { return nil, fmt.Errorf("failed to fetch latest release: %w", err) } - defer resp.Body.Close() + defer func() { _ = resp.Body.Close() }() if resp.StatusCode != http.StatusOK { return nil, fmt.Errorf("GitHub API returned status %d", resp.StatusCode) diff --git a/cmd/helm-values/internal/precommit.go b/cmd/helm-values/internal/precommit.go index 733c1fa..cdc6199 100644 --- a/cmd/helm-values/internal/precommit.go +++ b/cmd/helm-values/internal/precommit.go @@ -11,6 +11,10 @@ import ( const preCommitConfigPath = ".pre-commit-config.yaml" +// preCommitConfigPerm is the permission mode used when writing the +// pre-commit config file back to disk. +const preCommitConfigPerm = 0o600 + var schemaPreCommitHook = &PreCommitHook{ ID: "helm-values-schema", Name: "Generate Helm values schema", @@ -34,6 +38,7 @@ func newPreCommitConfig() *PreCommitConfig { } } +// PreCommitConfig is the root of a .pre-commit-config.yaml file. type PreCommitConfig struct { Repos []*PreCommitRepo `yaml:"repos"` } @@ -48,6 +53,7 @@ func (c *PreCommitConfig) getRepo(name string) *PreCommitRepo { return c.Repos[i] } } + return nil } @@ -59,6 +65,8 @@ func newPreCommitRepo(name string, rev string) *PreCommitRepo { } } +// PreCommitRepo is a single "repos" entry in a .pre-commit-config.yaml +// file. type PreCommitRepo struct { Repo string `yaml:"repo"` Rev string `yaml:"rev,omitempty"` @@ -75,12 +83,15 @@ func (r *PreCommitRepo) setHook(hook *PreCommitHook) { for i, h := range r.Hooks { if h.ID == hook.ID { r.Hooks[i] = hook + return } } + r.Hooks = append(r.Hooks, hook) } +// PreCommitHook is a single hook entry under a PreCommitRepo. type PreCommitHook struct { ID string `yaml:"id"` Name string `yaml:"name,omitempty"` @@ -91,11 +102,15 @@ type PreCommitHook struct { } func readPreCommitConfig(path string) (*PreCommitConfig, bool, error) { + //nolint:gosec // path is the pre-commit config file this CLI manages + // (".pre-commit-config.yaml" in the working directory); reading a + // user-provided local path is this function's intended purpose. data, err := os.ReadFile(path) if err != nil { if os.IsNotExist(err) { return nil, false, nil } + return nil, false, fmt.Errorf("failed to read config: %w", err) } @@ -113,14 +128,16 @@ func writePreCommitConfig(path string, config *PreCommitConfig) error { return fmt.Errorf("failed to marshal config: %w", err) } - if err := os.WriteFile(path, output, 0644); err != nil { + if err := os.WriteFile(path, output, preCommitConfigPerm); err != nil { return fmt.Errorf("failed to write config: %w", err) } return nil } -func InstallPreCommitHooks(logger *logrus.Logger) error { +// InstallPreCommitHooks adds this plugin's schema and docs generation hooks +// to the local repo's .pre-commit-config.yaml, creating it if needed. +func InstallPreCommitHooks(_ *logrus.Logger) error { newHooks := []*PreCommitHook{ schemaPreCommitHook, docsPreCommitHook, @@ -130,6 +147,7 @@ func InstallPreCommitHooks(logger *logrus.Logger) error { if err != nil { return err } + if !exists { config = newPreCommitConfig() } diff --git a/cmd/helm-values/internal/update.go b/cmd/helm-values/internal/update.go index fbb2248..ea4e9ea 100644 --- a/cmd/helm-values/internal/update.go +++ b/cmd/helm-values/internal/update.go @@ -1,17 +1,20 @@ package internal import ( + "context" "fmt" "os/exec" "github.com/sirupsen/logrus" ) -func Update(logger *logrus.Logger, repository string, currentVersion string) error { +// Update checks repository's latest GitHub release against currentVersion +// and, if newer, uninstalls and reinstalls the helm-values plugin via helm. +func Update(ctx context.Context, logger *logrus.Logger, repository string, currentVersion string) error { logger.Info("Fetching latest release information...") // Get the latest release from GitHub - release, err := GetLatestRelease(repository) + release, err := GetLatestRelease(ctx, repository) if err != nil { return fmt.Errorf("failed to get latest release: %w", err) } @@ -21,6 +24,7 @@ func Update(logger *logrus.Logger, repository string, currentVersion string) err // Check if we're already at the latest version if currentVersion == release.TagName { fmt.Printf("Already at the latest version (%s)\n", currentVersion) + return nil } @@ -30,17 +34,22 @@ func Update(logger *logrus.Logger, repository string, currentVersion string) err } // Uninstall the current plugin - uninstallCmd := exec.Command("helm", "plugin", "uninstall", "values") + uninstallCmd := exec.CommandContext(ctx, "helm", "plugin", "uninstall", "values") if err := uninstallCmd.Run(); err != nil { logger.Warnf("Failed to uninstall plugin (it may not be installed): %v", err) } - // Install the new version - installCmd := exec.Command("helm", "plugin", "install", pluginURL) + // Install the new version. This is a deliberate, user-initiated subprocess + // call to the helm plugin manager as part of this tool's self-update flow; + // pluginURL comes from the GitHub releases API response for this project's + // own repository, not from attacker-controlled input. + //nolint:gosec // see comment above + installCmd := exec.CommandContext(ctx, "helm", "plugin", "install", pluginURL) if err := installCmd.Run(); err != nil { return fmt.Errorf("failed to install plugin: %w", err) } fmt.Printf("Successfully updated helm-values to %s\n", release.TagName) + return nil } diff --git a/cmd/helm-values/main.go b/cmd/helm-values/main.go index e4b6e16..44e94ad 100644 --- a/cmd/helm-values/main.go +++ b/cmd/helm-values/main.go @@ -1,3 +1,6 @@ +// Command helm-values is a Helm plugin that generates a JSON Schema and +// documentation for a chart's values.yaml, and manages the +// yaml-language-server modeline and pre-commit hooks that go with them. package main import ( @@ -55,6 +58,7 @@ func Program(logger *logrus.Logger) *cobra.Command { cmd.AddCommand(CommandModeline(logger, utilityGroup)) cmd.AddCommand(CommandUpdate(logger)) cmd.AddCommand(CommandVersion(logger)) + return cmd } @@ -64,21 +68,24 @@ func CommandSchema(logger *logrus.Logger, group *cobra.Group) *cobra.Command { cmd := &cobra.Command{ Use: "schema [flags] chart_dir [...chart_dir]", Short: "Generate values schema", - RunE: func(cmd *cobra.Command, args []string) error { + RunE: func(_ *cobra.Command, args []string) error { if err := cfg.UpdateLogger(logger); err != nil { - return err + return fmt.Errorf("updating logger: %w", err) } schemaCfg, err := cfg.ToPackageConfig() if err != nil { - return err + return fmt.Errorf("building schema config: %w", err) } + return schema.GenerateSchema(logger, schemaCfg, args) }, GroupID: group.ID, } - cfg.BindFlags(cmd) + if err := cfg.BindFlags(cmd); err != nil { + panic(fmt.Sprintf("binding schema flags: %v", err)) + } return cmd } @@ -90,25 +97,32 @@ func CommandDocs(logger *logrus.Logger, group *cobra.Group) *cobra.Command { Use: "docs [flags] chart_dir [...chart_dir]", Short: "Generate values docs", Args: cobra.MinimumNArgs(1), - RunE: func(cmd *cobra.Command, args []string) error { + RunE: func(_ *cobra.Command, args []string) error { if err := cfg.UpdateLogger(logger); err != nil { - return err + return fmt.Errorf("updating logger: %w", err) } docsCfg, err := cfg.ToPackageConfig() if err != nil { - return err + return fmt.Errorf("building docs config: %w", err) } + return docs.GenerateDocs(logger, docsCfg, args) }, GroupID: group.ID, } - cfg.BindFlags(cmd) + if err := cfg.BindFlags(cmd); err != nil { + panic(fmt.Sprintf("binding docs flags: %v", err)) + } return cmd } +// modelineMaxArgs is the maximum number of positional args the modeline +// command accepts: chart_ref and, optionally, values_file. +const modelineMaxArgs = 2 + func CommandModeline(logger *logrus.Logger, group *cobra.Group) *cobra.Command { cfg := config.NewModelineConfig() @@ -116,13 +130,14 @@ func CommandModeline(logger *logrus.Logger, group *cobra.Group) *cobra.Command { Use: "modeline [flags] chart_ref values_file", Short: "Add yaml-language-server modeline to values file", GroupID: group.ID, - Args: cobra.RangeArgs(1, 2), - RunE: func(cmd *cobra.Command, args []string) error { + Args: cobra.RangeArgs(1, modelineMaxArgs), + RunE: func(_ *cobra.Command, args []string) error { if err := cfg.UpdateLogger(logger); err != nil { - return err + return fmt.Errorf("updating logger: %w", err) } chartRef := args[0] + valuesFile := "" if len(args) > 1 { valuesFile = args[1] @@ -130,14 +145,16 @@ func CommandModeline(logger *logrus.Logger, group *cobra.Group) *cobra.Command { modelineCfg, err := cfg.ToPackageConfig(chartRef, valuesFile) if err != nil { - return err + return fmt.Errorf("building modeline config: %w", err) } return modeline.WriteModeline(logger, modelineCfg) }, } - cfg.BindFlags(cmd) + if err := cfg.BindFlags(cmd); err != nil { + panic(fmt.Sprintf("binding modeline flags: %v", err)) + } return cmd } @@ -146,8 +163,8 @@ func CommandUpdate(logger *logrus.Logger) *cobra.Command { cmd := &cobra.Command{ Use: "update", Short: "Update the helm-values plugin to the latest version", - RunE: func(cmd *cobra.Command, args []string) error { - return internal.Update(logger, Repository, BuildVersion) + RunE: func(cmd *cobra.Command, _ []string) error { + return internal.Update(cmd.Context(), logger, Repository, BuildVersion) }, } @@ -159,7 +176,7 @@ func CommandPreCommit(logger *logrus.Logger, group *cobra.Group) *cobra.Command Use: "pre-commit", Short: "Install pre-commit hooks for generating schema and docs", GroupID: group.ID, - RunE: func(cmd *cobra.Command, args []string) error { + RunE: func(_ *cobra.Command, _ []string) error { return internal.InstallPreCommitHooks(logger) }, } @@ -167,11 +184,11 @@ func CommandPreCommit(logger *logrus.Logger, group *cobra.Group) *cobra.Command return cmd } -func CommandVersion(logger *logrus.Logger) *cobra.Command { +func CommandVersion(_ *logrus.Logger) *cobra.Command { cmd := &cobra.Command{ Use: "version", Short: "Print version information", - RunE: func(cmd *cobra.Command, args []string) error { + RunE: func(_ *cobra.Command, _ []string) error { releaseNotes := "" if !strings.Contains(BuildVersion, "SNAPSHOT") { @@ -183,6 +200,7 @@ func CommandVersion(logger *logrus.Logger) *cobra.Command { fmt.Printf(" Commit: %s\n", BuildCommit) fmt.Printf(" Date: %s\n", BuildDate) fmt.Printf(" Release Notes: %s\n", releaseNotes) + return nil }, } diff --git a/pkg/charts/chart.go b/pkg/charts/chart.go index f207156..76e7a4c 100644 --- a/pkg/charts/chart.go +++ b/pkg/charts/chart.go @@ -1,3 +1,5 @@ +// Package charts provides types and helpers for locating and parsing +// Helm chart directories. package charts import ( @@ -7,6 +9,14 @@ import ( "go.yaml.in/yaml/v4" ) +// Chart represents a Helm chart located on disk, rooted at a given directory. +type Chart struct { + rootPath string + Details *ChartDetails +} + +// NewChart loads a Chart from the given chart root directory, parsing its +// Chart.yaml file into ChartDetails. func NewChart(chartRoot string) (*Chart, error) { chart := &Chart{ rootPath: chartRoot, @@ -14,13 +24,14 @@ func NewChart(chartRoot string) (*Chart, error) { content, err := os.ReadFile(chart.ChartFilePath()) if err != nil { - return nil, err + return nil, fmt.Errorf("read chart file: %w", err) } details := &ChartDetails{} + err = yaml.Unmarshal(content, details) if err != nil { - return nil, err + return nil, fmt.Errorf("parse chart file: %w", err) } chart.Details = details @@ -28,43 +39,49 @@ func NewChart(chartRoot string) (*Chart, error) { return chart, nil } -type Chart struct { - rootPath string - Details *ChartDetails -} - +// RootPath returns the chart's root directory path. func (c *Chart) RootPath() string { return c.rootPath } -func (p *Chart) ChartFilePath() string { - return fmt.Sprintf("%s/Chart.yaml", p.rootPath) +// ChartFilePath returns the path to the chart's Chart.yaml file. +func (c *Chart) ChartFilePath() string { + return c.rootPath + "/Chart.yaml" } -func (p *Chart) ValuesFilePath() string { - return fmt.Sprintf("%s/values.yaml", p.rootPath) +// ValuesFilePath returns the path to the chart's values.yaml file. +func (c *Chart) ValuesFilePath() string { + return c.rootPath + "/values.yaml" } -func (p *Chart) SchemaFilePath() string { - return fmt.Sprintf("%s/values.schema.json", p.rootPath) +// SchemaFilePath returns the path to the chart's values.schema.json file. +func (c *Chart) SchemaFilePath() string { + return c.rootPath + "/values.schema.json" } -func (p *Chart) ReadmeMdFilePath() string { - return fmt.Sprintf("%s/README.md", p.rootPath) +// ReadmeMdFilePath returns the path to the chart's README.md file. +func (c *Chart) ReadmeMdFilePath() string { + return c.rootPath + "/README.md" } -func (p *Chart) ReadmeMdTemplateFilePath() string { - return fmt.Sprintf("%s/README.md.gotmpl", p.rootPath) +// ReadmeMdTemplateFilePath returns the path to the chart's README.md.gotmpl +// template file. +func (c *Chart) ReadmeMdTemplateFilePath() string { + return c.rootPath + "/README.md.gotmpl" } -func (p *Chart) ReadmeRstFilePath() string { - return fmt.Sprintf("%s/README.rst", p.rootPath) +// ReadmeRstFilePath returns the path to the chart's README.rst file. +func (c *Chart) ReadmeRstFilePath() string { + return c.rootPath + "/README.rst" } -func (p *Chart) ReadmeRstTemplateFilePath() string { - return fmt.Sprintf("%s/README.rst.gotmpl", p.rootPath) +// ReadmeRstTemplateFilePath returns the path to the chart's README.rst.gotmpl +// template file. +func (c *Chart) ReadmeRstTemplateFilePath() string { + return c.rootPath + "/README.rst.gotmpl" } +// ChartDetails holds the metadata parsed from a chart's Chart.yaml file. type ChartDetails struct { Name string `yaml:"name"` Description string `yaml:"description"` @@ -72,6 +89,8 @@ type ChartDetails struct { Annotations map[string]string `yaml:"annotations"` } +// ValuesSchema returns the chart's values-schema annotation, or an empty +// string if it is not set. func (d *ChartDetails) ValuesSchema() string { schemaURL, ok := d.Annotations["values-schema"] if !ok || schemaURL == "" { diff --git a/pkg/charts/search.go b/pkg/charts/search.go index 2f22df7..1243211 100644 --- a/pkg/charts/search.go +++ b/pkg/charts/search.go @@ -8,6 +8,8 @@ import ( "github.com/sirupsen/logrus" ) +// Search walks the given chart directories (supporting glob patterns) and +// returns all Helm charts found within them. func Search(logger *logrus.Logger, chartDirs []string) ([]*Chart, error) { cleanedChartDirs, err := cleanPaths(chartDirs) if err != nil { @@ -15,81 +17,102 @@ func Search(logger *logrus.Logger, chartDirs []string) ([]*Chart, error) { } foundCharts := []*Chart{} + for _, rootDir := range cleanedChartDirs { err := filepath.WalkDir(rootDir, func(path string, d os.DirEntry, err error) error { if err != nil { return err } + if !d.IsDir() { return nil } logger.Tracef("search: checking path: %s", path) - chartFileInfo, err := os.Stat(fmt.Sprintf("%s/Chart.yaml", path)) - if err != nil { - logger. - WithField("reason", "error"). - WithError(err). - Tracef("search: skipping path: %s", path) - return nil - } - if chartFileInfo.IsDir() { - logger. - WithField("reason", "Chart.yaml is a directory"). - Tracef("search: skipping path: %s", path) - return nil + chart := findChartAtPath(logger, path) + if chart != nil { + logger.Infof("search: found chart %s at %s", chart.Details.Name, path) + foundCharts = append(foundCharts, chart) } - valuesFileInfo, err := os.Stat(fmt.Sprintf("%s/Chart.yaml", path)) - if err != nil { - logger. - WithField("reason", "error"). - WithError(err). - Tracef("search: kipping path: %s", path) - return nil - } - if valuesFileInfo.IsDir() { - logger. - WithField("reason", "values.yaml is a directory"). - Tracef("search: skipping path: %s", path) - return nil - } - - chart, err := NewChart(path) - if err != nil { - logger. - WithField("reason", "error"). - WithError(err). - Warnf("search: skipping possible chart: %s", path) - return nil - } - - logger.Infof("search: found chart %s at %s", chart.Details.Name, path) - foundCharts = append(foundCharts, chart) return nil }) if err != nil { - return nil, err + return nil, fmt.Errorf("walk chart directory %q: %w", rootDir, err) } } logger.Debugf("search: found %d charts", len(foundCharts)) + return foundCharts, nil } +// findChartAtPath checks whether path contains a Helm chart (i.e. a +// Chart.yaml and values.yaml file) and, if so, loads and returns it. It +// returns nil if path does not contain a chart, or the chart fails to load. +func findChartAtPath(logger *logrus.Logger, path string) *Chart { + chartFileInfo, err := os.Stat(path + "/Chart.yaml") + if err != nil { + logger. + WithField("reason", "error"). + WithError(err). + Tracef("search: skipping path: %s", path) + + return nil + } + + if chartFileInfo.IsDir() { + logger. + WithField("reason", "Chart.yaml is a directory"). + Tracef("search: skipping path: %s", path) + + return nil + } + + valuesFileInfo, err := os.Stat(path + "/Chart.yaml") + if err != nil { + logger. + WithField("reason", "error"). + WithError(err). + Tracef("search: kipping path: %s", path) + + return nil + } + + if valuesFileInfo.IsDir() { + logger. + WithField("reason", "values.yaml is a directory"). + Tracef("search: skipping path: %s", path) + + return nil + } + + chart, err := NewChart(path) + if err != nil { + logger. + WithField("reason", "error"). + WithError(err). + Warnf("search: skipping possible chart: %s", path) + + return nil + } + + return chart +} + func cleanPaths(paths []string) ([]string, error) { cleanedPaths := []string{} for _, path := range paths { ap, err := filepath.Abs(path) if err != nil { - return nil, err + return nil, fmt.Errorf("resolve absolute path %q: %w", path, err) } ps, err := filepath.Glob(ap) if err != nil { - return nil, err + return nil, fmt.Errorf("expand glob pattern %q: %w", ap, err) } cleanedPaths = append(cleanedPaths, ps...) diff --git a/pkg/docs/config.go b/pkg/docs/config.go index 13fea38..fb0df31 100644 --- a/pkg/docs/config.go +++ b/pkg/docs/config.go @@ -1,3 +1,5 @@ +// Package docs generates a chart's values documentation (e.g. README.md) +// from its values schema. package docs import ( @@ -10,6 +12,7 @@ import ( "github.com/sirupsen/logrus" ) +// Config controls how a chart's values documentation is generated. type Config struct { LogLevel logrus.Level StdOut bool @@ -24,13 +27,17 @@ type Config struct { Order ValuesOrder } +// ValuesOrder controls the order in which values rows are rendered. type ValuesOrder string const ( + // ValuesOrderAlphabetical sorts values rows alphabetically by key. ValuesOrderAlphabetical ValuesOrder = "alphabetical" - ValuesOrderPreserve ValuesOrder = "preserve" + // ValuesOrderPreserve keeps values rows in their source file order. + ValuesOrderPreserve ValuesOrder = "preserve" ) +// NewValuesOrder parses orderStr into a ValuesOrder. func NewValuesOrder(orderStr string) (ValuesOrder, error) { switch strings.ToLower(orderStr) { case "alphabetical": diff --git a/pkg/docs/docs.go b/pkg/docs/docs.go index 63f2608..43fc775 100644 --- a/pkg/docs/docs.go +++ b/pkg/docs/docs.go @@ -4,26 +4,53 @@ import ( "bytes" "encoding/json" "fmt" - "helmvalues/pkg" - "helmvalues/pkg/charts" - "helmvalues/pkg/docs/templates" - "helmvalues/pkg/schema" "os" "slices" "sort" "strings" + "text/template" + + "helmvalues/pkg" + "helmvalues/pkg/charts" + "helmvalues/pkg/docs/templates" + "helmvalues/pkg/schema" "github.com/sirupsen/logrus" ) +// GenerateDocs generates README documentation for every chart discovered under chartDirs, +// rendering the configured (or default) template with the chart's values schema. func GenerateDocs(logger *logrus.Logger, cfg *Config, chartDirs []string) error { chartsFound, err := charts.Search(logger, chartDirs) + if err != nil { + return fmt.Errorf("searching for charts: %w", err) + } + + plans, err := collectPlans(logger, cfg, chartsFound) if err != nil { return err } - // Itterate through plan to set the logger and config + staticPaths, err := templates.StaticTemplates() + if err != nil { + return fmt.Errorf("collecting static templates: %w", err) + } + + // Iterate through plans, generating the docs for each + for _, plan := range plans { + if err := generateChartDoc(logger, cfg, staticPaths, plan); err != nil { + return err + } + } + + return nil +} + +// collectPlans builds a Plan for each discovered chart, logging its details and +// verifying that a target template can be resolved for it. +func collectPlans(logger *logrus.Logger, cfg *Config, chartsFound []*charts.Chart) ([]*Plan, error) { plans := []*Plan{} + for _, chart := range chartsFound { plan := NewPlan(cfg, chart) @@ -33,100 +60,126 @@ func GenerateDocs(logger *logrus.Logger, cfg *Config, chartDirs []string) error plan.LogDocDetails(logger) if _, _, err := plan.DocsTargetTemplate(); err != nil { - return fmt.Errorf("default template disallowed, but no template found in chart %s", plan.Chart().RootPath()) + return nil, fmt.Errorf("default template disallowed, but no template found in chart %s", plan.Chart().RootPath()) } + plans = append(plans, plan) } - staticPaths, err := templates.StaticTemplates() + 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 { + 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 + } + + logger.Tracef("docs: %s: jsonschema properties: %+v", plan.Chart().Details.Name, jsonschema.Properties) + + table := templates.TemplateContext{ + Raw: &templates.RawContext{ + Chart: plan.Chart(), + Values: jsonschema, + }, + ValuesTable: schemaProperties(jsonschema, cfg.Order, []string{}), + } + + t, err := buildChartTemplate(logger, cfg, staticPaths, plan) if err != nil { return err } - // Iterate through plans again, this time generating the docs - for _, plan := range plans { - logger.Infof("docs: %s: starting generation", plan.Chart().Details.Name) + buf := new(bytes.Buffer) - 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 - } - logger.Tracef("docs: %s: jsonschema properties: %+v", plan.Chart().Details.Name, jsonschema.Properties) - - table := templates.TemplateContext{ - Raw: &templates.RawContext{ - Chart: plan.Chart(), - Values: jsonschema, - }, - ValuesTable: schemaProperties(jsonschema, cfg.Order, []string{}), - } + logger.Debugf("docs: %s: rendering template", plan.Chart().Details.Name) - for _, p := range staticPaths { - logger.Debugf("docs: %s: collecting static template: %s", plan.Chart().Details.Name, p) - } - for _, extraTemplate := range cfg.ExtraTemplates { - logger.Debugf("docs: %s: collecting extra template: %s", plan.Chart().Details.Name, extraTemplate) - } - extraTemplates := append(staticPaths, cfg.ExtraTemplates...) - - if !plan.DocsUseDefault() { - logger.Debugf( - "docs: %s: collecting template: %s", - plan.Chart().Details.Name, - plan.DocsChartReadmeTemplate(), - ) - } else { - logger.Debugf( - "docs: %s: using builtin default template", - plan.Chart().Details.Name, - ) - } + err = t.Execute(buf, table) + if err != nil { + return fmt.Errorf("rendering template: %w", err) + } - root, err := os.OpenRoot("/") - if err != nil { - return err - } + logger.Debugf("docs: %s: writing output", plan.Chart().Details.Name) - layeredFs := pkg.NewLayeredFS(templates.TemplateFS, root.FS()) + if err := plan.WriteReadme(logger, buf.String()); err != nil { + return err + } - markup, err := plan.DocsMarkup() - if err != nil { - return err - } + logger.Infof("docs: %s: finished", plan.Chart().Details.Name) - opts := []templates.BuilderOpt{ - templates.WithExtraPaths(extraTemplates), - templates.WithUseDefault(plan.DocsUseDefault()), - templates.WithMarkup(markup), - } - if !plan.DocsUseDefault() { - opts = append(opts, templates.WithCustomTemplate(plan.DocsChartReadmeTemplate())) - } + return nil +} - builder := templates.NewTemplateBuilder(opts...) - t, err := builder.Build(layeredFs) - if err != nil { - return err - } +// buildChartTemplate collects the static, extra, and (if applicable) custom template +// paths for plan and builds the resulting root template. +func buildChartTemplate( + logger *logrus.Logger, + cfg *Config, + staticPaths []string, + plan *Plan, +) (*template.Template, error) { + for _, p := range staticPaths { + logger.Debugf("docs: %s: collecting static template: %s", plan.Chart().Details.Name, p) + } - buf := new(bytes.Buffer) - logger.Debugf("docs: %s: rendering template", plan.Chart().Details.Name) - err = t.Execute(buf, table) - if err != nil { - return err - } + for _, extraTemplate := range cfg.ExtraTemplates { + logger.Debugf("docs: %s: collecting extra template: %s", plan.Chart().Details.Name, extraTemplate) + } - logger.Debugf("docs: %s: writing output", plan.Chart().Details.Name) - if err := plan.WriteReadme(logger, buf.String()); err != nil { - return err - } + extraTemplates := make([]string, 0, len(staticPaths)+len(cfg.ExtraTemplates)) + extraTemplates = append(extraTemplates, staticPaths...) + extraTemplates = append(extraTemplates, cfg.ExtraTemplates...) + + if !plan.DocsUseDefault() { + logger.Debugf( + "docs: %s: collecting template: %s", + plan.Chart().Details.Name, + plan.DocsChartReadmeTemplate(), + ) + } else { + logger.Debugf( + "docs: %s: using builtin default template", + plan.Chart().Details.Name, + ) + } - logger.Infof("docs: %s: finished", plan.Chart().Details.Name) + root, err := os.OpenRoot("/") + if err != nil { + return nil, fmt.Errorf("opening root filesystem: %w", err) } - return nil + layeredFs := pkg.NewLayeredFS(templates.TemplateFS, root.FS()) + + markup, err := plan.DocsMarkup() + if err != nil { + return nil, err + } + + opts := []templates.BuilderOpt{ + templates.WithExtraPaths(extraTemplates), + templates.WithUseDefault(plan.DocsUseDefault()), + templates.WithMarkup(markup), + } + if !plan.DocsUseDefault() { + opts = append(opts, templates.WithCustomTemplate(plan.DocsChartReadmeTemplate())) + } + + builder := templates.NewTemplateBuilder(opts...) + + t, err := builder.Build(layeredFs) + if err != nil { + return nil, fmt.Errorf("building template: %w", err) + } + + return t, nil } func schemaProperties(jsonschema *pkg.JsonSchema, order ValuesOrder, parents []string) []templates.ValuesRow { @@ -153,6 +206,7 @@ func schemaProperties(jsonschema *pkg.JsonSchema, order ValuesOrder, parents []s Type: fmt.Sprintf("[Ref](%s)", prop.Ref), } rows = append(rows, row) + continue } @@ -162,11 +216,13 @@ func schemaProperties(jsonschema *pkg.JsonSchema, order ValuesOrder, parents []s Type: fmt.Sprintf("[Schema](%s)", prop.Schema), } rows = append(rows, row) + continue } if prop.Type == "object" { rows = append(rows, schemaProperties(prop, order, append(parents, key))...) + continue } @@ -176,24 +232,7 @@ func schemaProperties(jsonschema *pkg.JsonSchema, order ValuesOrder, parents []s fmt.Printf("Error marshaling default value for key %s: %v\n", key, err) } - typeValue := prop.Type - if len(prop.Enum) > 0 { - enumItems := make([]string, len(prop.Enum)) - for i, enumItem := range prop.Enum { - enumBytes, err := json.Marshal(enumItem) - if err != nil { - // TODO: Handle this error better - continue - } - enumItems[i] = string(enumBytes) - } - - typeValue = fmt.Sprintf( - "%s (enum)\n%s", - typeValue, - strings.Join(enumItems, ", "), - ) - } + typeValue := formatEnumType(prop.Type, prop.Enum) row := templates.ValuesRow{ Key: strings.Join(append(parents, key), "."), @@ -206,3 +245,29 @@ func schemaProperties(jsonschema *pkg.JsonSchema, order ValuesOrder, parents []s return rows } + +// formatEnumType renders a property's type alongside its enum values, if any, for +// display in the generated values table. +func formatEnumType(propType string, enum []any) string { + if len(enum) == 0 { + return propType + } + + enumItems := make([]string, len(enum)) + + for i, enumItem := range enum { + enumBytes, err := json.Marshal(enumItem) + if err != nil { + // TODO: Handle this error better + continue + } + + enumItems[i] = string(enumBytes) + } + + return fmt.Sprintf( + "%s (enum)\n%s", + propType, + strings.Join(enumItems, ", "), + ) +} diff --git a/pkg/docs/plan.go b/pkg/docs/plan.go index 476d6db..a16caae 100644 --- a/pkg/docs/plan.go +++ b/pkg/docs/plan.go @@ -1,17 +1,29 @@ package docs import ( + "context" "errors" "fmt" + "os" + "os/exec" + "helmvalues/pkg/charts" "helmvalues/pkg/docs/templates" "helmvalues/pkg/schema" - "os" - "os/exec" "github.com/sirupsen/logrus" ) +// Plan holds the resolved configuration and chart-derived details needed to plan and +// generate documentation for a single chart. +type Plan struct { + cfg *Config + chart *charts.Chart + schemaPlan *schema.Plan +} + +// NewPlan creates a Plan that combines the given Config with a specific chart, deriving +// a schema.Plan for that chart along the way. func NewPlan(cfg *Config, chart *charts.Chart) *Plan { schemaCfg := &schema.Config{ StdOut: cfg.StdOut, @@ -29,12 +41,7 @@ func NewPlan(cfg *Config, chart *charts.Chart) *Plan { } } -type Plan struct { - cfg *Config - chart *charts.Chart - schemaPlan *schema.Plan -} - +// LogCommonDetails logs the plan's common (non-chart-specific) configuration values. func (p *Plan) LogCommonDetails(logger *logrus.Logger) { // common configs logger.Debugf("plan: %s: DryRun=%t", p.chart.Details.Name, p.DryRun()) @@ -42,6 +49,7 @@ func (p *Plan) LogCommonDetails(logger *logrus.Logger) { logger.Debugf("plan: %s: Stdout=%t", p.chart.Details.Name, p.StdOut()) } +// LogChartDetails logs the plan's chart-specific file path details. func (p *Plan) LogChartDetails(logger *logrus.Logger) { // chart configs logger.Debugf("plan: %s: ChartRoot=%s", p.chart.Details.Name, p.chart.RootPath()) @@ -51,6 +59,8 @@ func (p *Plan) LogChartDetails(logger *logrus.Logger) { logger.Debugf("plan: %s: ChartReadmeTemplate=%s", p.chart.Details.Name, p.DocsChartReadmeTemplate()) } +// LogDocDetails logs the plan's resolved documentation generation details, such as the +// target template, markup type, and output path. func (p *Plan) LogDocDetails(logger *logrus.Logger) { logger.Debugf("plan: %s: UseDefault=%t", p.chart.Details.Name, p.DocsUseDefault()) template, builtin, err := p.DocsTargetTemplate() @@ -62,30 +72,38 @@ func (p *Plan) LogDocDetails(logger *logrus.Logger) { logger.Debugf("plan: %s: ValuesOrder=%s (error: %v)", p.chart.Details.Name, p.cfg.Order, err) } +// LogSchemaDetails logs the plan's underlying schema.Plan details. func (p *Plan) LogSchemaDetails(logger *logrus.Logger) { p.schemaPlan.LogSchemaDetails(logger) } +// Chart returns the chart this plan generates documentation for. func (p *Plan) Chart() *charts.Chart { return p.chart } +// StdOut reports whether the generated documentation should also be printed to stdout. func (p *Plan) StdOut() bool { return p.cfg.StdOut } +// StrictComments reports whether strict comment validation is enabled. func (p *Plan) StrictComments() bool { return p.cfg.Strict } +// GitAdd reports whether the generated output file should be staged with `git add`. func (p *Plan) GitAdd() bool { return p.cfg.GitAdd } +// DryRun reports whether the plan should skip writing output to disk. func (p *Plan) DryRun() bool { return p.cfg.DryRun } +// 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) { if p.cfg.Template != "" { return p.cfg.Template, false, nil @@ -102,16 +120,22 @@ func (p *Plan) DocsTargetTemplate() (string, bool, error) { return "", false, errors.New("no target template found") } +// DocsChartReadmeTemplate returns the path of the chart's own README template file +// (Markdown or reStructuredText), or an empty string if the chart has none. func (p *Plan) DocsChartReadmeTemplate() string { if _, err := os.Stat(p.chart.ReadmeMdTemplateFilePath()); err == nil { return p.chart.ReadmeMdTemplateFilePath() } + if _, err := os.Stat(p.chart.ReadmeRstTemplateFilePath()); err == nil { return p.chart.ReadmeRstTemplateFilePath() } + return "" } +// DocsMarkup resolves the markup type to render, inferring it from the configured +// markup, template path, or chart readme template when not explicitly set. func (p *Plan) DocsMarkup() (templates.Markup, error) { if value, ok := p.cfg.Markup.Get(); ok { return value, nil @@ -123,17 +147,29 @@ func (p *Plan) DocsMarkup() (templates.Markup, error) { // If a template was specified, infer the markup type from that if tmpl := p.cfg.Template; tmpl != "" { - return templates.MarkupFromPath(tmpl) + markup, err := templates.MarkupFromPath(tmpl) + if err != nil { + return "", fmt.Errorf("inferring markup from template path %s: %w", tmpl, err) + } + + return markup, nil } // If there's a readme template in the chart, infer the markup type from that if tmpl := p.DocsChartReadmeTemplate(); tmpl != "" { - return templates.MarkupFromPath(tmpl) + markup, err := templates.MarkupFromPath(tmpl) + if err != nil { + return "", fmt.Errorf("inferring markup from chart readme template path %s: %w", tmpl, err) + } + + return markup, nil } return "", errors.New("unable to infer markup type") } +// DocsUseDefault reports whether the built-in default template should be used, based on +// the explicit config value, a configured custom template, or a chart-provided template. func (p *Plan) DocsUseDefault() bool { // If the user explicitly sets use-default, use that value if useDefault, ok := p.cfg.UseDefault.Get(); ok { @@ -153,6 +189,8 @@ func (p *Plan) DocsUseDefault() bool { return true } +// DocsOutputPath resolves the file path the generated documentation should be written +// to, using the configured output path or deriving one from the chart and markup type. func (p *Plan) DocsOutputPath() (string, error) { if output, ok := p.cfg.Output.Get(); ok { return output, nil @@ -166,6 +204,7 @@ func (p *Plan) DocsOutputPath() (string, error) { if docType == templates.Markdown { return p.chart.ReadmeMdFilePath(), nil } + if docType == templates.ReStructuredText { return p.chart.ReadmeRstFilePath(), nil } @@ -173,33 +212,20 @@ func (p *Plan) DocsOutputPath() (string, error) { panic("invalid markup type") } +// SchemaPlan returns the plan's underlying schema.Plan. func (p *Plan) SchemaPlan() *schema.Plan { return p.schemaPlan } +// WriteReadme writes the rendered documentation content to the plan's output path (unless +// DryRun is set), and/or prints it to stdout, depending on the plan's configuration. func (p *Plan) WriteReadme(logger *logrus.Logger, content string) error { if !p.DryRun() { - outputPath, err := p.DocsOutputPath() - if err != nil { - return err - } - - f, err := os.Create(outputPath) - if err != nil { - return err - } - defer f.Close() + logger.Debugf("plan: %s: writing readme to disk", p.chart.Details.Name) - if _, err = f.Write([]byte(content)); err != nil { + if err := p.writeReadmeFile(content); err != nil { return err } - - if p.GitAdd() { - err := exec.Command("git", "add", outputPath).Run() - if err != nil { - return fmt.Errorf("failed to git add %s: %w", outputPath, err) - } - } } if p.StdOut() { @@ -208,3 +234,38 @@ func (p *Plan) WriteReadme(logger *logrus.Logger, content string) error { return nil } + +// writeReadmeFile writes content to the plan's output path, and, if configured, +// stages the resulting file with `git add`. +func (p *Plan) writeReadmeFile(content string) error { + outputPath, err := p.DocsOutputPath() + if err != nil { + return 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 + f, err := os.Create(outputPath) + if err != nil { + return fmt.Errorf("creating output file %s: %w", outputPath, err) + } + defer func() { _ = f.Close() }() + + if _, err = f.WriteString(content); err != nil { + return fmt.Errorf("writing output file %s: %w", outputPath, err) + } + + if p.GitAdd() { + // outputPath is derived from the chart's own configured/derived README path + // (or a user-supplied --output flag), not attacker-controlled input; running + // `git add` on it is an intentional, user-opted-in git integration. + //nolint:gosec // outputPath is a user-opted-in, non-attacker-controlled file path + err := exec.CommandContext(context.Background(), "git", "add", outputPath).Run() + if err != nil { + return fmt.Errorf("failed to git add %s: %w", outputPath, err) + } + } + + return nil +} diff --git a/pkg/docs/templates/markup.go b/pkg/docs/templates/markup.go index b706305..b808754 100644 --- a/pkg/docs/templates/markup.go +++ b/pkg/docs/templates/markup.go @@ -1,3 +1,6 @@ +// Package templates provides the template building blocks used to render chart +// documentation, including the built-in static templates, template functions, and +// markup-type handling. package templates import ( @@ -5,14 +8,18 @@ import ( "strings" ) -// enum describing template types +// Markup is an enum describing the supported documentation markup types. type Markup string const ( - Markdown Markup = "markdown" + // Markdown is the Markdown documentation markup type. + Markdown Markup = "markdown" + // ReStructuredText is the reStructuredText documentation markup type. ReStructuredText Markup = "restructuredtext" ) +// MarkupFromString parses a Markup from its canonical or shorthand string +// representation (e.g. "markdown"/"md", "restructuredtext"/"rst"). func MarkupFromString(s string) (Markup, error) { switch s { case "markdown", "md": @@ -24,12 +31,15 @@ func MarkupFromString(s string) (Markup, error) { } } +// MarkupFromPath infers a Markup from a template file path's extension. func MarkupFromPath(path string) (Markup, error) { if strings.Contains(path, ".md.tmpl") || strings.Contains(path, ".md.gotmpl") { return Markdown, nil } + if strings.Contains(path, ".rst.tmpl") || strings.Contains(path, ".rst.gotmpl") { return ReStructuredText, nil } + return "", errors.New("unable to infer markup type") } diff --git a/pkg/docs/templates/models.go b/pkg/docs/templates/models.go index 79a9e45..d531f87 100644 --- a/pkg/docs/templates/models.go +++ b/pkg/docs/templates/models.go @@ -5,6 +5,7 @@ import ( "helmvalues/pkg/charts" ) +// ValuesRow is one rendered row of a chart's values table. type ValuesRow struct { Key string Type string @@ -12,11 +13,14 @@ type ValuesRow struct { Description string } +// RawContext exposes the underlying chart and its parsed values schema to +// templates. type RawContext struct { Chart *charts.Chart Values *pkg.JsonSchema } +// TemplateContext is the data made available to a documentation template. type TemplateContext struct { Raw *RawContext ValuesTable []ValuesRow diff --git a/pkg/docs/templates/staticfs.go b/pkg/docs/templates/staticfs.go index 1ebd138..e74eef0 100644 --- a/pkg/docs/templates/staticfs.go +++ b/pkg/docs/templates/staticfs.go @@ -2,12 +2,22 @@ package templates import ( "embed" + "fmt" "io/fs" ) +// TemplateFS embeds the built-in static templates shipped with the binary. +// //go:embed all:static var TemplateFS embed.FS +// StaticTemplates returns the paths of the built-in static templates embedded in +// TemplateFS. func StaticTemplates() ([]string, error) { - return fs.Glob(TemplateFS, "static/**/*.gotmpl") + matches, err := fs.Glob(TemplateFS, "static/**/*.gotmpl") + if err != nil { + return nil, fmt.Errorf("globbing static templates: %w", err) + } + + return matches, nil } diff --git a/pkg/docs/templates/template_builder.go b/pkg/docs/templates/template_builder.go index 86c8776..8a37bce 100644 --- a/pkg/docs/templates/template_builder.go +++ b/pkg/docs/templates/template_builder.go @@ -1,6 +1,7 @@ package templates import ( + "fmt" "io/fs" "path/filepath" "strings" @@ -9,9 +10,16 @@ import ( "github.com/Masterminds/sprig/v3" ) +// DefaultMarkdownTemplate is the name of the built-in template used to render +// Markdown documentation when no custom template is configured. const DefaultMarkdownTemplate = "default.md.gotmpl" + +// DefaultReStructuredTextTemplate is the name of the built-in template used to render +// reStructuredText documentation when no custom template is configured. const DefaultReStructuredTextTemplate = "default.rst.gotmpl" +// TemplateBuilder assembles a text/template.Template from a set of static, extra, and +// optionally custom template paths, ready to render a chart's documentation. type TemplateBuilder struct { customTemplate string extraPaths []string @@ -19,25 +27,46 @@ type TemplateBuilder struct { markup Markup } +// NewTemplateBuilder creates a TemplateBuilder configured with the given options. +func NewTemplateBuilder(opts ...BuilderOpt) *TemplateBuilder { + t := &TemplateBuilder{} + for _, s := range opts { + s(t) + } + + return t +} + +// TemplateName returns the name of the root template to execute: the built-in default +// for the configured markup type when useDefault is set, otherwise the base name of the +// configured custom template. func (b *TemplateBuilder) TemplateName() string { if b.useDefault && b.markup == Markdown { return DefaultMarkdownTemplate } + if b.useDefault && b.markup == ReStructuredText { return DefaultReStructuredTextTemplate } + return filepath.Base(b.customTemplate) } +// TemplatePaths returns the full set of template paths to parse: the builder's extra +// paths, plus the custom template path when useDefault is not set. func (b *TemplateBuilder) TemplatePaths() []string { paths := []string{} + paths = append(paths, b.extraPaths...) if !b.useDefault { paths = append(paths, b.customTemplate) } + return paths } +// Build parses the builder's template paths from fsys, registering the builder's +// template functions, and returns the resulting root template. func (b *TemplateBuilder) Build(fsys fs.FS) (*template.Template, error) { paths := b.TemplatePaths() @@ -56,11 +85,18 @@ func (b *TemplateBuilder) Build(fsys fs.FS) (*template.Template, error) { funcMap["mdRow"] = mdRow funcMap["mdMultiline"] = mdMultiline - return template.New(b.TemplateName()). + tmpl, err := template.New(b.TemplateName()). Funcs(funcMap). ParseFS(fsys, paths...) + if err != nil { + return nil, fmt.Errorf("parsing templates %v: %w", paths, err) + } + + return tmpl, nil } +// WithCustomTemplate sets the builder's custom template path, disabling use of the +// built-in default template, and infers the markup type from the path when possible. func WithCustomTemplate(template string) BuilderOpt { return func(t *TemplateBuilder) { t.customTemplate = template @@ -73,30 +109,29 @@ func WithCustomTemplate(template string) BuilderOpt { } } +// WithExtraPaths sets the builder's extra template paths, which are always parsed +// alongside the default or custom template. func WithExtraPaths(paths []string) BuilderOpt { return func(t *TemplateBuilder) { t.extraPaths = paths } } +// WithUseDefault sets whether the builder should render the built-in default template +// instead of a custom template. func WithUseDefault(useDefault bool) BuilderOpt { return func(t *TemplateBuilder) { t.useDefault = useDefault } } +// WithMarkup sets the markup type the builder should render, used to select the +// appropriate built-in default template. func WithMarkup(markup Markup) BuilderOpt { return func(t *TemplateBuilder) { t.markup = markup } } +// BuilderOpt configures a TemplateBuilder, applied by NewTemplateBuilder. type BuilderOpt = func(*TemplateBuilder) - -func NewTemplateBuilder(opts ...BuilderOpt) *TemplateBuilder { - t := &TemplateBuilder{} - for _, s := range opts { - s(t) - } - return t -} diff --git a/pkg/docs/templates/template_funcs.go b/pkg/docs/templates/template_funcs.go index 279d274..4500a77 100644 --- a/pkg/docs/templates/template_funcs.go +++ b/pkg/docs/templates/template_funcs.go @@ -10,6 +10,7 @@ func lpad(s string, padStr string, pLen int64) string { if remaining <= 0 { return s } + return strings.Repeat(padStr, remaining) + s } @@ -18,23 +19,27 @@ func rpad(s string, padStr string, pLen int64) string { if remaining <= 0 { return s } + return s + strings.Repeat(padStr, remaining) } func maxLen(items []string) int { - max := 0 + longest := 0 + for _, s := range items { - for _, line := range strings.Split(s, "\n") { - if len(line) > max { - max = len(line) + for line := range strings.SplitSeq(s, "\n") { + if len(line) > longest { + longest = len(line) } } } - return max + + return longest } func rowSelect(items []ValuesRow, field string) []string { var result []string + for _, item := range items { switch field { case "Key": @@ -47,11 +52,12 @@ func rowSelect(items []ValuesRow, field string) []string { result = append(result, item.Description) } } + return result } func mdRow(cols []string, colWidths []int64) string { - c := []string{} + c := make([]string, 0, len(cols)) for i, col := range cols { c = append(c, rpad(col, " ", colWidths[i])) } diff --git a/pkg/helm/cache.go b/pkg/helm/cache.go index 03ab03a..f5c053b 100644 --- a/pkg/helm/cache.go +++ b/pkg/helm/cache.go @@ -1,3 +1,5 @@ +// Package helm provides helpers for working with Helm chart references, +// repository caches, and index files. package helm import ( @@ -5,6 +7,12 @@ import ( "strings" ) +// maxSplitParts is the maximum number of parts to split a chart reference +// into when separating its repository/chart and chart@version segments. +const maxSplitParts = 2 + +// ChartRef represents a parsed reference to a Helm chart, either as a +// "repository/chart@version" reference or a local filesystem path. type ChartRef struct { Repository string Chart string @@ -12,20 +20,25 @@ type ChartRef struct { Path string } +// NewChartRef parses a chart reference string into a ChartRef. Strings +// without a "/" are treated as local filesystem paths. func NewChartRef(ref string) (*ChartRef, error) { - parts1 := strings.SplitN(ref, "/", 2) - if len(parts1) != 2 { + parts1 := strings.SplitN(ref, "/", maxSplitParts) + if len(parts1) != maxSplitParts { cr := &ChartRef{ Path: ref, } + return cr, nil } + repository := parts1[0] - parts2 := strings.SplitN(parts1[1], "@", 2) + parts2 := strings.SplitN(parts1[1], "@", maxSplitParts) chart := parts2[0] + version := "" - if len(parts2) == 2 { + if len(parts2) == maxSplitParts { version = parts2[1] } @@ -34,6 +47,7 @@ func NewChartRef(ref string) (*ChartRef, error) { Chart: chart, Version: version, } + return cr, nil } @@ -41,5 +55,6 @@ func (c ChartRef) String() string { if c.Version != "" { return fmt.Sprintf("%s/%s@%s", c.Repository, c.Chart, c.Version) } + return fmt.Sprintf("%s/%s", c.Repository, c.Chart) } diff --git a/pkg/helm/chart_details.go b/pkg/helm/chart_details.go index 4c06729..2de127b 100644 --- a/pkg/helm/chart_details.go +++ b/pkg/helm/chart_details.go @@ -2,10 +2,13 @@ package helm import ( "fmt" - "helmvalues/pkg/charts" "path/filepath" + + "helmvalues/pkg/charts" ) +// ChartDetailsFromRef returns the chart details for the given ChartRef, +// loading them from a local path or, if no path is set, the repository cache. func ChartDetailsFromRef(chartRef *ChartRef) (*charts.ChartDetails, error) { if chartRef.Path != "" { return ChartDetailsFromPath(chartRef.Path) @@ -14,6 +17,8 @@ func ChartDetailsFromRef(chartRef *ChartRef) (*charts.ChartDetails, error) { return ChartDetailsFromCache(chartRef) } +// ChartDetailsFromCache returns the chart details for the given ChartRef by +// looking up its repository index in the local Helm repository cache. func ChartDetailsFromCache(chartRef *ChartRef) (*charts.ChartDetails, error) { index, err := RepositoryIndexFromCache(chartRef.Repository) if err != nil { @@ -23,9 +28,12 @@ func ChartDetailsFromCache(chartRef *ChartRef) (*charts.ChartDetails, error) { if chartRef.Version != "" { return index.GetVersion(chartRef.Chart, chartRef.Version) } + return index.GetLatestVersion(chartRef.Chart) } +// ChartDetailsFromPath loads chart details from the chart located at the +// given filesystem path. func ChartDetailsFromPath(chartRef string) (*charts.ChartDetails, error) { absPath, err := filepath.Abs(chartRef) if err != nil { diff --git a/pkg/helm/index.go b/pkg/helm/index.go index 8812be3..4de8da0 100644 --- a/pkg/helm/index.go +++ b/pkg/helm/index.go @@ -1,38 +1,44 @@ package helm import ( + "errors" "fmt" - "helmvalues/pkg/charts" "os" "path/filepath" "sort" + "helmvalues/pkg/charts" + "github.com/Masterminds/semver/v3" "go.yaml.in/yaml/v4" ) +// RepositoryIndexFromCache loads the Helm repository index for the given +// repository name from the local Helm repository cache. func RepositoryIndexFromCache(repoName string) (*Index, error) { repositoryCache := os.Getenv("HELM_REPOSITORY_CACHE") if repositoryCache == "" { - return nil, fmt.Errorf("HELM_REPOSITORY_CACHE environment variable is not set") + return nil, errors.New("HELM_REPOSITORY_CACHE environment variable is not set") } indexPath := filepath.Join(repositoryCache, repoName+"-index.yaml") + //nolint:gosec // local Helm repository cache path, not network input if _, err := os.Stat(indexPath); err != nil { - return nil, err + return nil, fmt.Errorf("stat index file: %w", err) } return LoadIndex(indexPath) } +// Index represents a parsed Helm repository index.yaml file. type Index struct { Entries map[string][]*charts.ChartDetails `yaml:"entries"` } -// LoadIndex loads and parses a Helm index.yaml file +// LoadIndex loads and parses a Helm index.yaml file. func LoadIndex(path string) (*Index, error) { - data, err := os.ReadFile(path) + data, err := os.ReadFile(path) //nolint:gosec // local Helm repository cache path, not network input if err != nil { return nil, fmt.Errorf("failed to read index file: %w", err) } @@ -45,7 +51,7 @@ func LoadIndex(path string) (*Index, error) { return &index, nil } -// FindChart finds chart versions by name in the index +// FindChart finds chart versions by name in the index. func (i *Index) FindChart(chartName string) ([]*charts.ChartDetails, error) { versions, ok := i.Entries[chartName] if !ok { @@ -59,7 +65,7 @@ func (i *Index) FindChart(chartName string) ([]*charts.ChartDetails, error) { return versions, nil } -// GetVersion finds a specific version of a chart +// GetVersion finds a specific version of a chart. func (i *Index) GetVersion(chartName, version string) (*charts.ChartDetails, error) { versions, err := i.FindChart(chartName) if err != nil { @@ -76,7 +82,7 @@ func (i *Index) GetVersion(chartName, version string) (*charts.ChartDetails, err } // GetLatestVersion finds the latest stable version of a chart -// Stable means non-prerelease versions (no -alpha, -beta, -rc suffixes) +// Stable means non-prerelease versions (no -alpha, -beta, -rc suffixes). func (i *Index) GetLatestVersion(chartName string) (*charts.ChartDetails, error) { versions, err := i.FindChart(chartName) if err != nil { @@ -93,6 +99,7 @@ func (i *Index) GetLatestVersion(chartName string) (*charts.ChartDetails, error) // Skip invalid semver versions continue } + semvers = append(semvers, sv) versionMap[v.Version] = v } diff --git a/pkg/jsonschema.go b/pkg/jsonschema.go index a5b0fa1..a884fcf 100644 --- a/pkg/jsonschema.go +++ b/pkg/jsonschema.go @@ -1,9 +1,16 @@ +// Package pkg contains the shared core types for helmvalues: the JSON Schema +// model, a layered filesystem abstraction, and an ordered map used for +// preserving property order when encoding schemas. package pkg import ( "regexp" ) +// JsonSchema is an in-memory representation of a JSON Schema document, +// covering the keywords used to validate and describe Helm values. +// +//nolint:revive // JsonSchema is public API referenced throughout the module; renaming is out of scope here type JsonSchema struct { Location string `json:"location,omitempty" yaml:"location,omitempty"` @@ -12,16 +19,16 @@ type JsonSchema struct { Format string `json:"format,omitempty" yaml:"format,omitempty"` - Always *bool `json:"always,omitempty" yaml:"always,omitempty"` - Ref string `json:"$ref,omitempty" yaml:"$ref,omitempty"` + Always *bool `json:"always,omitempty" yaml:"always,omitempty"` + Ref string `json:"$ref,omitempty" yaml:"$ref,omitempty"` RecursiveAnchor bool `json:"recursiveAnchor,omitempty" yaml:"recursiveAnchor,omitempty"` - RecursiveRef string `json:"recursiveRef,omitempty" yaml:"recursiveRef,omitempty"` - DynamicAnchor string `json:"dynamicAnchor,omitempty" yaml:"dynamicAnchor,omitempty"` - DynamicRef string `json:"dynamicRef,omitempty" yaml:"dynamicRef,omitempty"` + RecursiveRef string `json:"recursiveRef,omitempty" yaml:"recursiveRef,omitempty"` + DynamicAnchor string `json:"dynamicAnchor,omitempty" yaml:"dynamicAnchor,omitempty"` + DynamicRef string `json:"dynamicRef,omitempty" yaml:"dynamicRef,omitempty"` - Type string `json:"type,omitempty" yaml:"type,omitempty"` + Type string `json:"type,omitempty" yaml:"type,omitempty"` Constant []any `json:"constant,omitempty" yaml:"constant,omitempty"` - Enum []any `json:"enum,omitempty" yaml:"enum,omitempty"` + Enum []any `json:"enum,omitempty" yaml:"enum,omitempty"` Not *JsonSchema `json:"not,omitempty"` AllOf []*JsonSchema `json:"allOf,omitempty" yaml:"allOf,omitempty"` @@ -31,56 +38,60 @@ type JsonSchema struct { Then *JsonSchema `json:"then,omitempty"` Else *JsonSchema `json:"else,omitempty"` - MinProperties int64 `json:"minProperties,omitempty" yaml:"minProperties,omitempty"` - MaxProperties int64 `json:"maxProperties,omitempty" yaml:"maxProperties,omitempty"` - Required []string `json:"required,omitempty" yaml:"required,omitempty"` - Properties *EncodableOrderedMap[string, *JsonSchema] `json:"properties,omitempty" yaml:"properties,omitempty"` - PropertyNames *JsonSchema `json:"propertyNames,omitempty" yaml:"propertyNames,omitempty"` - PatternProperties map[*regexp.Regexp]*JsonSchema `json:"patternProperties,omitempty" yaml:"patternProperties,omitempty"` - AdditionalProperties any `json:"additionalProperties,omitempty" yaml:"additionalProperties,omitempty"` - Dependencies map[string]any `json:"dependencies,omitempty" yaml:"dependencies,omitempty"` - DependentRequired map[string][]string `json:"dependentRequired,omitempty" yaml:"dependentRequired,omitempty"` - DependentSchemas map[string]*JsonSchema `json:"dependentSchemas,omitempty" yaml:"dependentSchemas,omitempty"` - UnevaluatedProperties *JsonSchema `json:"unevaluatedProperties,omitempty" yaml:"unevaluatedProperties,omitempty"` - - MinItems int64 `json:"minItems,omitempty" yaml:"minItems,omitempty"` - MaxItems int64 `json:"maxItems,omitempty" yaml:"maxItems,omitempty"` - UniqueItems bool `json:"uniqueItems,omitempty" yaml:"uniqueItems,omitempty"` - Items any `json:"items,omitempty" yaml:"items,omitempty"` - AdditionalItems any `json:"additionalItems,omitempty" yaml:"additionalItems,omitempty"` - PrefixItems []*JsonSchema `json:"prefixItems,omitempty" yaml:"prefixItems,omitempty"` - Contains *JsonSchema `json:"contains,omitempty" yaml:"contains,omitempty"` - MinContains int64 `json:"minContains,omitempty" yaml:"minContains,omitempty"` - MaxContains int64 `json:"maxContains,omitempty" yaml:"maxContains,omitempty"` + MinProperties int64 `json:"minProperties,omitempty" yaml:"minProperties,omitempty"` + MaxProperties int64 `json:"maxProperties,omitempty" yaml:"maxProperties,omitempty"` + Required []string `json:"required,omitempty" yaml:"required,omitempty"` + Properties *EncodableOrderedMap[string, *JsonSchema] `json:"properties,omitempty" yaml:"properties,omitempty"` + PropertyNames *JsonSchema `json:"propertyNames,omitempty" yaml:"propertyNames,omitempty"` + PatternProperties map[*regexp.Regexp]*JsonSchema `json:"patternProperties,omitempty" yaml:"patternProperties,omitempty"` + AdditionalProperties any `json:"additionalProperties,omitempty" yaml:"additionalProperties,omitempty"` //nolint:lll // long aligned struct tag block, matches sibling fields + Dependencies map[string]any `json:"dependencies,omitempty" yaml:"dependencies,omitempty"` + DependentRequired map[string][]string `json:"dependentRequired,omitempty" yaml:"dependentRequired,omitempty"` + DependentSchemas map[string]*JsonSchema `json:"dependentSchemas,omitempty" yaml:"dependentSchemas,omitempty"` + UnevaluatedProperties *JsonSchema `json:"unevaluatedProperties,omitempty" yaml:"unevaluatedProperties,omitempty"` //nolint:lll // long aligned struct tag block, matches sibling fields + + MinItems int64 `json:"minItems,omitempty" yaml:"minItems,omitempty"` + MaxItems int64 `json:"maxItems,omitempty" yaml:"maxItems,omitempty"` + UniqueItems bool `json:"uniqueItems,omitempty" yaml:"uniqueItems,omitempty"` + Items any `json:"items,omitempty" yaml:"items,omitempty"` + AdditionalItems any `json:"additionalItems,omitempty" yaml:"additionalItems,omitempty"` + PrefixItems []*JsonSchema `json:"prefixItems,omitempty" yaml:"prefixItems,omitempty"` + Contains *JsonSchema `json:"contains,omitempty" yaml:"contains,omitempty"` + MinContains int64 `json:"minContains,omitempty" yaml:"minContains,omitempty"` + MaxContains int64 `json:"maxContains,omitempty" yaml:"maxContains,omitempty"` UnevaluatedItems *JsonSchema `json:"unevaluatedItems,omitempty" yaml:"unevaluatedItems,omitempty"` - MinLength int64 `json:"minLength,omitempty" yaml:"minLength,omitempty"` - MaxLength int64 `json:"maxLength,omitempty" yaml:"maxLength,omitempty"` - Pattern *regexp.Regexp `json:"pattern,omitempty" yaml:"pattern,omitempty"` - ContentEncoding string `json:"contentEncoding,omitempty" yaml:"contentEncoding,omitempty"` + MinLength int64 `json:"minLength,omitempty" yaml:"minLength,omitempty"` + MaxLength int64 `json:"maxLength,omitempty" yaml:"maxLength,omitempty"` + Pattern *regexp.Regexp `json:"pattern,omitempty" yaml:"pattern,omitempty"` + ContentEncoding string `json:"contentEncoding,omitempty" yaml:"contentEncoding,omitempty"` ContentMediaType string `json:"contentMediaType,omitempty" yaml:"contentMediaType,omitempty"` - ContentSchema *JsonSchema `json:"contentSchema,omitempty" yaml:"contentSchema,omitempty"` + ContentSchema *JsonSchema `json:"contentSchema,omitempty" yaml:"contentSchema,omitempty"` - Minimum int64 `json:"minimum,omitempty" yaml:"minimum,omitempty"` + Minimum int64 `json:"minimum,omitempty" yaml:"minimum,omitempty"` ExclusiveMinimum int64 `json:"exclusiveMinimum,omitempty" yaml:"exclusiveMinimum,omitempty"` - Maximum int64 `json:"maximum,omitempty" yaml:"maximum,omitempty"` + Maximum int64 `json:"maximum,omitempty" yaml:"maximum,omitempty"` ExclusiveMaximum int64 `json:"exclusiveMaximum,omitempty" yaml:"exclusiveMaximum,omitempty"` - MultipleOf int64 `json:"multipleOf,omitempty" yaml:"multipleOf,omitempty"` + MultipleOf int64 `json:"multipleOf,omitempty" yaml:"multipleOf,omitempty"` - Title string `json:"title,omitempty" yaml:"title,omitempty"` + Title string `json:"title,omitempty" yaml:"title,omitempty"` Description string `json:"description,omitempty" yaml:"description,omitempty"` - Default any `json:"default,omitempty" yaml:"default,omitempty"` - Comment string `json:"comment,omitempty" yaml:"comment,omitempty"` - ReadOnly bool `json:"readOnly,omitempty" yaml:"readOnly,omitempty"` - WriteOnly bool `json:"writeOnly,omitempty" yaml:"writeOnly,omitempty"` - Examples []any `json:"examples,omitempty" yaml:"examples,omitempty"` - Deprecated bool `json:"deprecated,omitempty" yaml:"deprecated,omitempty"` + Default any `json:"default,omitempty" yaml:"default,omitempty"` + Comment string `json:"comment,omitempty" yaml:"comment,omitempty"` + ReadOnly bool `json:"readOnly,omitempty" yaml:"readOnly,omitempty"` + WriteOnly bool `json:"writeOnly,omitempty" yaml:"writeOnly,omitempty"` + Examples []any `json:"examples,omitempty" yaml:"examples,omitempty"` + Deprecated bool `json:"deprecated,omitempty" yaml:"deprecated,omitempty"` // Extensions map[string]ExtSchema `json:"extensions,omitempty"` } +// NodeInspector is a callback invoked for each schema node visited by +// WalkProperties, receiving the path of ancestor schemas and the node itself. type NodeInspector func(keyPath []*JsonSchema, schema *JsonSchema) +// WalkProperties recursively visits s and each nested property schema, +// calling every provided NodeInspector for each node encountered. func (s *JsonSchema) WalkProperties(fn ...NodeInspector) { s.walkProperties(fn) } diff --git a/pkg/layeredfs.go b/pkg/layeredfs.go index 19bd0c7..a3c04e8 100644 --- a/pkg/layeredfs.go +++ b/pkg/layeredfs.go @@ -1,26 +1,35 @@ package pkg import ( + "fmt" "io/fs" ) +// LayeredFS is an fs.FS that overlays multiple filesystems, resolving each +// file lookup against the layers in order and returning the first match. +type LayeredFS struct { + layers []fs.FS +} + +// NewLayeredFS returns a LayeredFS that searches the given layers, in order, +// for each requested file. func NewLayeredFS(layers ...fs.FS) *LayeredFS { return &LayeredFS{ layers: layers, } } -type LayeredFS struct { - layers []fs.FS -} - var _ fs.FS = (*LayeredFS)(nil) var _ fs.GlobFS = (*LayeredFS)(nil) var _ fs.ReadFileFS = (*LayeredFS)(nil) +// Open opens name by searching each layer in order, returning the first +// successful result or the last error encountered if no layer has the file. func (l *LayeredFS) Open(name string) (fs.File, error) { - var lastErr error - var f fs.File + var ( + lastErr error + f fs.File + ) for _, layer := range l.layers { f, lastErr = layer.Open(name) if lastErr == nil { @@ -28,16 +37,26 @@ func (l *LayeredFS) Open(name string) (fs.File, error) { } } + if lastErr != nil { + lastErr = fmt.Errorf("open %q in any layer: %w", name, lastErr) + } + return nil, lastErr } +// Glob always returns name as its only match; it does not perform real +// pattern matching against the underlying layers. func (l *LayeredFS) Glob(name string) ([]string, error) { return []string{name}, nil } +// ReadFile reads name by searching each layer in order, returning the first +// successful result or the last error encountered if no layer has the file. func (l *LayeredFS) ReadFile(name string) ([]byte, error) { - var lastErr error - var b []byte + var ( + lastErr error + b []byte + ) for _, layer := range l.layers { b, lastErr = fs.ReadFile(layer, name) if lastErr == nil { @@ -45,5 +64,9 @@ func (l *LayeredFS) ReadFile(name string) ([]byte, error) { } } + if lastErr != nil { + lastErr = fmt.Errorf("read %q in any layer: %w", name, lastErr) + } + return nil, lastErr } diff --git a/pkg/modeline/config.go b/pkg/modeline/config.go index 91d0ed4..0868380 100644 --- a/pkg/modeline/config.go +++ b/pkg/modeline/config.go @@ -1,8 +1,10 @@ +// Package modeline reads and writes the values-schema modeline comment +// (a "program: key=value" line) inside a chart's values file. package modeline import "helmvalues/pkg/helm" -// Config holds configuration for the modeline command +// Config holds configuration for the modeline command. type Config struct { ChartRef *helm.ChartRef TargetFile string diff --git a/pkg/modeline/file_modeline_manager.go b/pkg/modeline/file_modeline_manager.go index 7ade187..06ac669 100644 --- a/pkg/modeline/file_modeline_manager.go +++ b/pkg/modeline/file_modeline_manager.go @@ -7,6 +7,20 @@ import ( "strings" ) +// filePerm is the permission mode used when writing the modeline back to +// the target file. +const filePerm = 0o600 + +// FileModelineManager reads and rewrites the modeline comment in a file's +// content, without touching the rest of the file. +type FileModelineManager struct { + filepath string + exists bool + content string +} + +// NewFileModelineManager loads the file at filepath, if it exists, so its +// modeline can be inspected or replaced. func NewFileModelineManager(filepath string) (*FileModelineManager, error) { manager := &FileModelineManager{ filepath: filepath, @@ -15,9 +29,9 @@ func NewFileModelineManager(filepath string) (*FileModelineManager, error) { if _, err := os.Stat(filepath); err == nil { manager.exists = true - data, err := os.ReadFile(filepath) + data, err := os.ReadFile(filepath) //nolint:gosec // CLI intentionally reads a user-supplied local file path if err != nil { - return nil, err + return nil, fmt.Errorf("reading %s: %w", filepath, err) } manager.content = string(data) @@ -26,22 +40,20 @@ func NewFileModelineManager(filepath string) (*FileModelineManager, error) { return manager, nil } -type FileModelineManager struct { - filepath string - exists bool - content string -} - +// SetModeline replaces the existing modeline matching modeline's program and +// key, or inserts modeline as a new first line if none is found. func (m *FileModelineManager) SetModeline(modeline *Modeline) { - yamlModelinePrefix := fmt.Sprintf("# %s", modeline.ProgramAndKey()) - yamlModeline := fmt.Sprintf("# %s", modeline.String()) + yamlModelinePrefix := "# " + modeline.ProgramAndKey() + yamlModeline := "# " + modeline.String() found := false + content := strings.Split(m.content, "\n") for i, line := range content { if strings.HasPrefix(line, yamlModelinePrefix) { content[i] = yamlModeline found = true + break } } @@ -53,6 +65,11 @@ func (m *FileModelineManager) SetModeline(modeline *Modeline) { m.content = strings.Join(content, "\n") } -func (m *FileModelineManager) Write(createParents bool) error { - return os.WriteFile(m.filepath, []byte(m.content), 0644) +// Write persists the current content back to disk. +func (m *FileModelineManager) Write(_ bool) error { + if err := os.WriteFile(m.filepath, []byte(m.content), filePerm); err != nil { + return fmt.Errorf("writing %s: %w", m.filepath, err) + } + + return nil } diff --git a/pkg/modeline/modeline.go b/pkg/modeline/modeline.go index 127af59..4fc4612 100644 --- a/pkg/modeline/modeline.go +++ b/pkg/modeline/modeline.go @@ -1,12 +1,27 @@ package modeline import ( + "errors" "fmt" + "helmvalues/pkg/helm" "github.com/sirupsen/logrus" ) +// modelineFieldCount is the number of fields a modeline line scans into: +// program, key, and value. +const modelineFieldCount = 3 + +// PartialModeline identifies a modeline by its program and key, without a +// value, so it can be matched against or completed into a full Modeline. +type PartialModeline struct { + Program string + Key string +} + +// NewPartialModeline creates a PartialModeline identifying a modeline by its +// program and key, without a value. func NewPartialModeline(program, key string) PartialModeline { return PartialModeline{ Program: program, @@ -14,15 +29,14 @@ func NewPartialModeline(program, key string) PartialModeline { } } -type PartialModeline struct { - Program string - Key string -} - +// ProgramAndKey renders the "program: key=" prefix shared by every modeline +// with this program and key. func (m PartialModeline) ProgramAndKey() string { return fmt.Sprintf("%s: %s=", m.Program, m.Key) } +// ModelineWithValue completes this PartialModeline into a full Modeline +// carrying the given value. func (m PartialModeline) ModelineWithValue(value string) *Modeline { return &Modeline{ PartialModeline: m, @@ -30,23 +44,35 @@ func (m PartialModeline) ModelineWithValue(value string) *Modeline { } } +// Matches reports whether pm identifies the same program and key as m. func (m PartialModeline) Matches(pm *PartialModeline) bool { return m.Program == pm.Program && m.Key == pm.Key } +// ParseModeline parses a "program: key=value" modeline line. func ParseModeline(line string) (*Modeline, error) { var program, key, value string + n, err := fmt.Sscanf(line, "%s: %s=%s", &program, &key, &value) if err != nil { return nil, fmt.Errorf("line does not match modeline format: %w", err) } - if n != 3 { - return nil, fmt.Errorf("line does not match modeline format: expected 3 parts, got %d", n) + + if n != modelineFieldCount { + return nil, fmt.Errorf("line does not match modeline format: expected %d parts, got %d", modelineFieldCount, n) } return NewModeline(program, key, value), nil } +// Modeline is a "program: key=value" comment line embedded in a values file. +type Modeline struct { + PartialModeline + + Value string +} + +// NewModeline creates a Modeline from its program, key, and value. func NewModeline(program, key, value string) *Modeline { return &Modeline{ PartialModeline: PartialModeline{ @@ -57,30 +83,30 @@ func NewModeline(program, key, value string) *Modeline { } } -type Modeline struct { - PartialModeline - Value string -} - +// String renders the modeline as a "program: key=value" line. func (m Modeline) String() string { return fmt.Sprintf("%s%s", m.ProgramAndKey(), m.Value) } +// ValuesSchemaURLForChart looks up the values-schema URL annotated on the +// chart referenced by chartRef. func ValuesSchemaURLForChart(chartRef *helm.ChartRef) (string, error) { chartDetails, err := helm.ChartDetailsFromRef(chartRef) if err != nil { - return "", err + return "", fmt.Errorf("getting chart details for %s: %w", chartRef, err) } schemaURL := chartDetails.ValuesSchema() if schemaURL == "" { - return "", fmt.Errorf("chart does not have annotations.values-schema defined") + return "", errors.New("chart does not have annotations.values-schema defined") } return schemaURL, nil } -func WriteModeline(logger *logrus.Logger, cfg *Config) error { +// WriteModeline writes the values-schema modeline into the target file +// described by cfg. +func WriteModeline(_ *logrus.Logger, cfg *Config) error { plan := NewPlan(cfg) schemaURL, err := plan.ValuesSchemaURLForChart() diff --git a/pkg/modeline/plan.go b/pkg/modeline/plan.go index 7750133..b5d59cc 100644 --- a/pkg/modeline/plan.go +++ b/pkg/modeline/plan.go @@ -1,23 +1,30 @@ package modeline +// Plan bundles the config for a single modeline write, giving access to the +// derived schema URL, modeline, and target file manager. +type Plan struct { + cfg *Config +} + +// NewPlan creates a Plan from cfg. func NewPlan(cfg *Config) *Plan { return &Plan{ cfg: cfg, } } -type Plan struct { - cfg *Config -} - +// ValuesSchemaURLForChart looks up the values-schema URL for this plan's +// chart. func (p *Plan) ValuesSchemaURLForChart() (string, error) { return ValuesSchemaURLForChart(p.cfg.ChartRef) } +// Modeline builds the modeline this plan should write, pointing at schema. func (p *Plan) Modeline(schema string) *Modeline { return p.cfg.PartialModeline.ModelineWithValue(schema) } +// FileManager loads the target file's FileModelineManager. func (p *Plan) FileManager() (*FileModelineManager, error) { return NewFileModelineManager(p.cfg.TargetFile) } diff --git a/pkg/orderedmap.go b/pkg/orderedmap.go index e59c9d2..4314b19 100644 --- a/pkg/orderedmap.go +++ b/pkg/orderedmap.go @@ -2,6 +2,7 @@ package pkg import ( "encoding/json" + "fmt" "iter" "maps" @@ -11,37 +12,60 @@ import ( // 😮‍💨 this is stupid, but orderedmap doesn't implement json marshalling // github discussion here: https://github.com/elliotchance/orderedmap/issues/12 +// NewEncodableOrderedMap creates an empty EncodableOrderedMap. func NewEncodableOrderedMap[K comparable, V any]() *EncodableOrderedMap[K, V] { m := om.NewOrderedMap[K, V]() + return (*EncodableOrderedMap[K, V])(m) } +// EncodableOrderedMap is an om.OrderedMap that also supports JSON marshalling +// while preserving insertion order. type EncodableOrderedMap[K comparable, V any] om.OrderedMap[K, V] +// MarshalJSON encodes the map as a JSON object, preserving insertion order. func (m *EncodableOrderedMap[K, V]) MarshalJSON() ([]byte, error) { items := maps.Collect(m.ToOrderedMap().AllFromFront()) - return json.Marshal(items) + + b, err := json.Marshal(items) + if err != nil { + return nil, fmt.Errorf("marshal ordered map: %w", err) + } + + return b, nil } +// ToOrderedMap returns the underlying om.OrderedMap. func (m *EncodableOrderedMap[K, V]) ToOrderedMap() *om.OrderedMap[K, V] { return (*om.OrderedMap[K, V])(m) } +// AllFromFront returns an iterator over the map's key/value pairs in +// insertion order, starting from the front. func (m *EncodableOrderedMap[K, V]) AllFromFront() iter.Seq2[K, V] { inner := m.ToOrderedMap() + return inner.AllFromFront() } +// Keys returns an iterator over the map's keys in insertion order. func (m *EncodableOrderedMap[K, V]) Keys() iter.Seq[K] { inner := m.ToOrderedMap() + return inner.Keys() } +// Get returns the value stored for key and whether it was present. +// +//nolint:ireturn // mirrors om.OrderedMap.Get's generic (V, bool) signature func (m *EncodableOrderedMap[K, V]) Get(key K) (V, bool) { inner := m.ToOrderedMap() + return inner.Get(key) } +// Set stores value under key, appending key to the insertion order if it is +// not already present. func (m *EncodableOrderedMap[K, V]) Set(key K, value V) { inner := m.ToOrderedMap() inner.Set(key, value) diff --git a/pkg/schema/comments/comment.go b/pkg/schema/comments/comment.go index a0de545..1af3e11 100644 --- a/pkg/schema/comments/comment.go +++ b/pkg/schema/comments/comment.go @@ -1,13 +1,20 @@ +// Package comments parses the YAML comments above and below a values.yaml +// key into JSON Schema fields (description, examples, and other schema +// keywords), and renders parse errors with source context. package comments import ( "fmt" - "helmvalues/pkg" "strings" + "helmvalues/pkg" + "go.yaml.in/yaml/v4" ) +// Parse builds a JsonSchema for node from its head and foot YAML comments, +// merged with extraNodes (additional schema fields derived elsewhere, e.g. +// the inferred type). func Parse(node *yaml.Node, extraNodes []*yaml.Node) (*pkg.JsonSchema, error) { // new yaml map node to append the schema field nodes to schemaMapNode := &yaml.Node{ @@ -16,63 +23,89 @@ func Parse(node *yaml.Node, extraNodes []*yaml.Node) (*pkg.JsonSchema, error) { } if node.HeadComment != "" { - commentDocs, err := parseNodeComment(node.HeadComment) + headNodes, err := headCommentNodes(node) if err != nil { - return nil, NewCommentError(node, err) + return nil, err } - for _, commentDoc := range commentDocs { - nodes, ok := commentAsDescriptionNodes(commentDoc) - if ok { - schemaMapNode.Content = append(schemaMapNode.Content, nodes...) - continue - } - - nodes, ok = commentAsMapNodes(commentDoc) - if ok { - schemaMapNode.Content = append(schemaMapNode.Content, nodes...) - } - } + schemaMapNode.Content = append(schemaMapNode.Content, headNodes...) } if node.FootComment != "" { - commentDocs, err := parseNodeComment(node.FootComment) + footNodes, err := footCommentExampleNodes(node) if err != nil { - return nil, NewCommentError(node, err) - } - - exampleNodeKey := &yaml.Node{ - Kind: yaml.ScalarNode, - Value: "examples", - } - exampleNodeValue := &yaml.Node{ - Kind: yaml.SequenceNode, - Content: []*yaml.Node{}, - } - for _, commentDoc := range commentDocs { - exampleNodeValue.Content = append(exampleNodeValue.Content, &yaml.Node{ - Kind: yaml.ScalarNode, - Value: strings.TrimSpace(commentDoc), - }) + return nil, err } - schemaMapNode.Content = append( - schemaMapNode.Content, - exampleNodeKey, - exampleNodeValue, - ) + schemaMapNode.Content = append(schemaMapNode.Content, footNodes...) } // marshal to a string and subsequently unmarshal into the schema fullSchema, err := yaml.Marshal(newDocumentNode(schemaMapNode)) if err != nil { - return nil, err + return nil, fmt.Errorf("marshaling schema node: %w", err) } s := &pkg.JsonSchema{} - err = yaml.Unmarshal(fullSchema, s) + if err := yaml.Unmarshal(fullSchema, s); err != nil { + return nil, fmt.Errorf("unmarshaling schema: %w", err) + } + + return s, nil +} + +// headCommentNodes parses node's head comment into schema field nodes +// (description and/or arbitrary schema keywords). +func headCommentNodes(node *yaml.Node) ([]*yaml.Node, error) { + commentDocs, err := parseNodeComment(node.HeadComment) + if err != nil { + return nil, NewCommentError(node, err) + } + + nodes := []*yaml.Node{} + + for _, commentDoc := range commentDocs { + descNodes, ok := commentAsDescriptionNodes(commentDoc) + if ok { + nodes = append(nodes, descNodes...) - return s, err + continue + } + + mapNodes, ok := commentAsMapNodes(commentDoc) + if ok { + nodes = append(nodes, mapNodes...) + } + } + + return nodes, nil +} + +// footCommentExampleNodes parses node's foot comment into an "examples" +// schema field node, one example per "---"-separated comment block. +func footCommentExampleNodes(node *yaml.Node) ([]*yaml.Node, error) { + commentDocs, err := parseNodeComment(node.FootComment) + if err != nil { + return nil, NewCommentError(node, err) + } + + exampleNodeKey := &yaml.Node{ + Kind: yaml.ScalarNode, + Value: "examples", + } + + exampleNodeValue := &yaml.Node{ + Kind: yaml.SequenceNode, + Content: []*yaml.Node{}, + } + for _, commentDoc := range commentDocs { + exampleNodeValue.Content = append(exampleNodeValue.Content, &yaml.Node{ + Kind: yaml.ScalarNode, + Value: strings.TrimSpace(commentDoc), + }) + } + + return []*yaml.Node{exampleNodeKey, exampleNodeValue}, nil } func parseNodeComment(rawComment string) ([]string, error) { @@ -93,6 +126,7 @@ func parseNodeComment(rawComment string) ([]string, error) { if !found { return nil, fmt.Errorf("unexpected prefix: %s (%d of %d lines)", line, i, len(commentLines)) } + commentLines[i] = after } diff --git a/pkg/schema/comments/comment_test.go b/pkg/schema/comments/comment_test.go index 6d5c238..d6f60ce 100644 --- a/pkg/schema/comments/comment_test.go +++ b/pkg/schema/comments/comment_test.go @@ -1,149 +1,170 @@ -package comments +package comments_test import ( "fmt" - "helmvalues/pkg" "testing" + "helmvalues/pkg" + "helmvalues/pkg/schema/comments" + "regexp" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "go.yaml.in/yaml/v4" ) -const COMMENT_MISSING_SPACE_PREFIX = ` +const CommentMissingSpacePrefix = ` #comment has no lead space foo: bar ` -const COMMENT_WITH_INVALID_YAML = ` +const CommentWithInvalidYAML = ` # @invalid yaml string foo: bar ` -const DOESNT_SET_SCHEMA_PROPERTIES = ` +const DoesntSetSchemaProperties = ` # key: value foo: bar ` -const COMMENT_WITH_YAML_STRING = ` +const CommentWithYAMLString = ` # comment is just a string foo: bar ` -const SETS_SCHEMA_DEFAULT = ` +const SetsSchemaDefault = ` # default: baz foo: bar ` -const SETS_SCHEMA_WITH_MULTILINE_VALUE = ` +const SetsSchemaWithMultilineValue = ` # default: | # foo # bar foo: bar ` -const SETS_DESCRIPTION_TO_SECOND_DOC = ` +const SetsDescriptionToSecondDoc = ` # default: baz # --- # this is a description foo: bar ` +// testQux is the shared "qux" value referenced across the dependentRequired +// and dependencies test cases below. +const testQux = "qux" + func TestBasicCommentParsing(t *testing.T) { + t.Parallel() + var tests = []struct { name string document string expectedError string - validate func(tt *testing.T, s *pkg.JsonSchema, err error) + validate func(t *testing.T, s *pkg.JsonSchema, err error) }{ { name: "empty document makes no changes", document: "", - validate: func(tt *testing.T, s *pkg.JsonSchema, err error) { - assert.Nil(tt, err) - assert.Equal(tt, *s, pkg.JsonSchema{}) + validate: func(t *testing.T, s *pkg.JsonSchema, err error) { + t.Helper() + require.NoError(t, err) + assert.Equal(t, pkg.JsonSchema{}, *s) }, }, { name: "errors when comment missing space prefix", - document: COMMENT_MISSING_SPACE_PREFIX, - validate: func(tt *testing.T, s *pkg.JsonSchema, err error) { - assert.NotNil(tt, err) + document: CommentMissingSpacePrefix, + validate: func(t *testing.T, _ *pkg.JsonSchema, err error) { + t.Helper() + require.Error(t, err) assert.ErrorContains(t, err, "unexpected prefix") }, }, { // TODO: Fix comment parsing so that the description is correctly extracted name: "errors when comment is invalid yaml string", - document: COMMENT_WITH_INVALID_YAML, - validate: func(tt *testing.T, s *pkg.JsonSchema, err error) { - assert.NoError(tt, err) - assert.Equal(tt, "", s.Description) + document: CommentWithInvalidYAML, + validate: func(t *testing.T, s *pkg.JsonSchema, err error) { + t.Helper() + require.NoError(t, err) + assert.Empty(t, s.Description) }, }, { name: "comment with string yaml is treated as description", - document: COMMENT_WITH_YAML_STRING, - validate: func(tt *testing.T, s *pkg.JsonSchema, err error) { - assert.NoError(tt, err) - assert.Equal(tt, "comment is just a string", s.Description) + document: CommentWithYAMLString, + validate: func(t *testing.T, s *pkg.JsonSchema, err error) { + t.Helper() + require.NoError(t, err) + assert.Equal(t, "comment is just a string", s.Description) }, }, { name: "comment has no jsonschema properties", - document: DOESNT_SET_SCHEMA_PROPERTIES, - validate: func(tt *testing.T, s *pkg.JsonSchema, err error) { - assert.NoError(tt, err) - assert.Equal(tt, pkg.JsonSchema{}, *s) + document: DoesntSetSchemaProperties, + validate: func(t *testing.T, s *pkg.JsonSchema, err error) { + t.Helper() + require.NoError(t, err) + assert.Equal(t, pkg.JsonSchema{}, *s) }, }, { name: "comment sets jsonschema field: default", - document: SETS_SCHEMA_DEFAULT, - validate: func(tt *testing.T, s *pkg.JsonSchema, err error) { - assert.NoError(tt, err) - assert.Equal(tt, "baz", s.Default) + document: SetsSchemaDefault, + validate: func(t *testing.T, s *pkg.JsonSchema, err error) { + t.Helper() + require.NoError(t, err) + assert.Equal(t, "baz", s.Default) }, }, { name: "comment sets jsonschema field w/ multiline value", - document: SETS_SCHEMA_WITH_MULTILINE_VALUE, - validate: func(tt *testing.T, s *pkg.JsonSchema, err error) { - assert.NoError(tt, err) - assert.Equal(tt, "foo\nbar", s.Default) + document: SetsSchemaWithMultilineValue, + validate: func(t *testing.T, s *pkg.JsonSchema, err error) { + t.Helper() + require.NoError(t, err) + assert.Equal(t, "foo\nbar", s.Default) }, }, { name: "comment sets jsonschema description to second yaml doc", - document: SETS_DESCRIPTION_TO_SECOND_DOC, - validate: func(tt *testing.T, s *pkg.JsonSchema, err error) { - assert.NoError(tt, err) - assert.Equal(tt, "baz", s.Default) - assert.Equal(tt, "this is a description", s.Description) + document: SetsDescriptionToSecondDoc, + validate: func(t *testing.T, s *pkg.JsonSchema, err error) { + t.Helper() + require.NoError(t, err) + assert.Equal(t, "baz", s.Default) + assert.Equal(t, "this is a description", s.Description) }, }, } for _, tc := range tests { - t.Run(tc.name, func(tt *testing.T) { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + yamlNode := &yaml.Node{} err := yaml.Unmarshal([]byte(tc.document), yamlNode) - assert.NoError(tt, err) + require.NoError(t, err) - s, err := Parse(getCommentNode(yamlNode), nil) + s, err := comments.Parse(getCommentNode(yamlNode), nil) - tc.validate(tt, s, err) + tc.validate(t, s, err) }) } } func TestCommentFieldsSingleLine(t *testing.T) { + t.Parallel() + type testCase struct { field string commentValue string expectedValue any - validate func(tt *testing.T, tc testCase, s *pkg.JsonSchema) + validate func(t *testing.T, tc testCase, s *pkg.JsonSchema) } var tests = []testCase{ @@ -151,84 +172,93 @@ func TestCommentFieldsSingleLine(t *testing.T) { field: "$schema", commentValue: "https://example.com/schema", expectedValue: "https://example.com/schema", - validate: func(tt *testing.T, tc testCase, s *pkg.JsonSchema) { - assert.IsType(tt, tc.expectedValue, s.Schema) - assert.Equal(tt, tc.expectedValue, s.Schema) + validate: func(t *testing.T, tc testCase, s *pkg.JsonSchema) { + t.Helper() + assert.IsType(t, tc.expectedValue, s.Schema) + assert.Equal(t, tc.expectedValue, s.Schema) }, }, { field: "description", commentValue: "some description", expectedValue: "some description", - validate: func(tt *testing.T, tc testCase, s *pkg.JsonSchema) { - assert.IsType(tt, tc.expectedValue, s.Description) - assert.Equal(tt, tc.expectedValue, s.Description) + validate: func(t *testing.T, tc testCase, s *pkg.JsonSchema) { + t.Helper() + assert.IsType(t, tc.expectedValue, s.Description) + assert.Equal(t, tc.expectedValue, s.Description) }, }, { field: "format", commentValue: "some format", expectedValue: "some format", - validate: func(tt *testing.T, tc testCase, s *pkg.JsonSchema) { - assert.IsType(tt, tc.expectedValue, s.Format) - assert.Equal(tt, tc.expectedValue, s.Format) + validate: func(t *testing.T, tc testCase, s *pkg.JsonSchema) { + t.Helper() + assert.IsType(t, tc.expectedValue, s.Format) + assert.Equal(t, tc.expectedValue, s.Format) }, }, { field: "minLength", commentValue: "5", expectedValue: int64(5), - validate: func(tt *testing.T, tc testCase, s *pkg.JsonSchema) { - assert.IsType(tt, tc.expectedValue, s.MinLength) - assert.Equal(tt, tc.expectedValue, s.MinLength) + validate: func(t *testing.T, tc testCase, s *pkg.JsonSchema) { + t.Helper() + assert.IsType(t, tc.expectedValue, s.MinLength) + assert.Equal(t, tc.expectedValue, s.MinLength) }, }, { field: "deprecated", commentValue: "true", expectedValue: true, - validate: func(tt *testing.T, tc testCase, s *pkg.JsonSchema) { - assert.IsType(tt, tc.expectedValue, s.Deprecated) - assert.Equal(tt, tc.expectedValue, s.Deprecated) + validate: func(t *testing.T, tc testCase, s *pkg.JsonSchema) { + t.Helper() + assert.IsType(t, tc.expectedValue, s.Deprecated) + assert.Equal(t, tc.expectedValue, s.Deprecated) }, }, { field: "required", commentValue: "[foo, bar]", expectedValue: []string{"foo", "bar"}, - validate: func(tt *testing.T, tc testCase, s *pkg.JsonSchema) { - assert.IsType(tt, tc.expectedValue, s.Required) - assert.Equal(tt, tc.expectedValue, s.Required) + validate: func(t *testing.T, tc testCase, s *pkg.JsonSchema) { + t.Helper() + assert.IsType(t, tc.expectedValue, s.Required) + assert.Equal(t, tc.expectedValue, s.Required) }, }, { field: "maximum", commentValue: "100", expectedValue: int64(100), - validate: func(tt *testing.T, tc testCase, s *pkg.JsonSchema) { - assert.IsType(tt, tc.expectedValue, s.Maximum) - assert.Equal(tt, tc.expectedValue, s.Maximum) + validate: func(t *testing.T, tc testCase, s *pkg.JsonSchema) { + t.Helper() + assert.IsType(t, tc.expectedValue, s.Maximum) + assert.Equal(t, tc.expectedValue, s.Maximum) }, }, } for _, tc := range tests { - t.Run(tc.field, func(tt *testing.T) { + t.Run(tc.field, func(t *testing.T) { + t.Parallel() + document := fmt.Sprintf("# %s: %s\nfoo:bar\n", tc.field, tc.commentValue) yamlNode := &yaml.Node{} err := yaml.Unmarshal([]byte(document), yamlNode) - assert.NoError(tt, err) + require.NoError(t, err) - s, err := Parse(yamlNode.Content[0], nil) - assert.NoError(tt, err) + s, err := comments.Parse(yamlNode.Content[0], nil) + require.NoError(t, err) - tc.validate(tt, tc, s) + tc.validate(t, tc, s) }) } } -const TEST_FIELD_ONEOF = ` +const TestFieldOneOf = ` # oneOf: # - type: string # description: this is a string @@ -237,7 +267,7 @@ const TEST_FIELD_ONEOF = ` foo: bar ` -const TEST_DEPENDENT_REQUIRED = ` +const TestDependentRequired = ` # dependentRequired: # baz: # - qux @@ -247,7 +277,7 @@ const TEST_DEPENDENT_REQUIRED = ` foo: bar # line comment ` -const TEST_DEPENDENCIES = ` +const TestDependencies = ` # dependencies: # baz: qux # bif: 0 @@ -257,78 +287,86 @@ const TEST_DEPENDENCIES = ` foo: bar ` -const TEST_PATTERN = ` +const TestPattern = ` # pattern: ^[a-z]+$ foo: bar ` func TestCommentFieldsMultipleLines(t *testing.T) { + t.Parallel() + type testCase struct { name string comment string expectedValue any - validate func(tt *testing.T, tc testCase, s *pkg.JsonSchema) + validate func(t *testing.T, tc testCase, s *pkg.JsonSchema) } var tests = []testCase{ { name: "oneOf with multiple lines", - comment: TEST_FIELD_ONEOF, + comment: TestFieldOneOf, expectedValue: []*pkg.JsonSchema{ {Type: "string", Description: "this is a string"}, {Type: "number", Description: "this is a number"}, }, - validate: func(tt *testing.T, tc testCase, s *pkg.JsonSchema) { - assert.IsType(tt, tc.expectedValue, s.OneOf) - assert.Equal(tt, tc.expectedValue, s.OneOf) + validate: func(t *testing.T, tc testCase, s *pkg.JsonSchema) { + t.Helper() + assert.IsType(t, tc.expectedValue, s.OneOf) + assert.Equal(t, tc.expectedValue, s.OneOf) }, }, { name: "dependentRequired", - comment: TEST_DEPENDENT_REQUIRED, + comment: TestDependentRequired, expectedValue: map[string][]string{ - "baz": {"qux", "quux"}, + "baz": {testQux, "quux"}, "bif": {"quuz"}, }, - validate: func(tt *testing.T, tc testCase, s *pkg.JsonSchema) { - assert.IsType(tt, tc.expectedValue, s.DependentRequired) - assert.Equal(tt, tc.expectedValue, s.DependentRequired) + validate: func(t *testing.T, tc testCase, s *pkg.JsonSchema) { + t.Helper() + assert.IsType(t, tc.expectedValue, s.DependentRequired) + assert.Equal(t, tc.expectedValue, s.DependentRequired) }, }, { name: "dependencies", - comment: TEST_DEPENDENCIES, + comment: TestDependencies, expectedValue: map[string]any{ - "baz": "qux", + "baz": testQux, "bif": 0, "qux": []any{"quux", "quuz"}, }, - validate: func(tt *testing.T, tc testCase, s *pkg.JsonSchema) { - assert.IsType(tt, tc.expectedValue, s.Dependencies) - assert.Equal(tt, tc.expectedValue, s.Dependencies) + validate: func(t *testing.T, tc testCase, s *pkg.JsonSchema) { + t.Helper() + assert.IsType(t, tc.expectedValue, s.Dependencies) + assert.Equal(t, tc.expectedValue, s.Dependencies) }, }, { name: "pattern", - comment: TEST_PATTERN, + comment: TestPattern, expectedValue: regexp.MustCompile("^[a-z]+$"), - validate: func(tt *testing.T, tc testCase, s *pkg.JsonSchema) { - assert.IsType(tt, tc.expectedValue, s.Pattern) - assert.Equal(tt, tc.expectedValue, s.Pattern) + validate: func(t *testing.T, tc testCase, s *pkg.JsonSchema) { + t.Helper() + assert.IsType(t, tc.expectedValue, s.Pattern) + assert.Equal(t, tc.expectedValue, s.Pattern) }, }, } for _, tc := range tests { - t.Run(tc.name, func(tt *testing.T) { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + yamlNode := &yaml.Node{} err := yaml.Unmarshal([]byte(tc.comment), yamlNode) - assert.NoError(tt, err) + require.NoError(t, err) - s, err := Parse(getCommentNode(yamlNode), nil) - assert.NoError(tt, err) + s, err := comments.Parse(getCommentNode(yamlNode), nil) + require.NoError(t, err) - tc.validate(tt, tc, s) + tc.validate(t, tc, s) }) } } diff --git a/pkg/schema/comments/error.go b/pkg/schema/comments/error.go index 6907e76..be17cd6 100644 --- a/pkg/schema/comments/error.go +++ b/pkg/schema/comments/error.go @@ -1,33 +1,52 @@ package comments import ( + "errors" "fmt" + "strconv" "strings" "github.com/sirupsen/logrus" "go.yaml.in/yaml/v4" ) -func NewCommentError(node *yaml.Node, err error) *CommentError { - return &CommentError{ - Node: node, - Err: err, - } -} +// colPadding is the number of padding characters used when sizing the +// display columns rendered by displayFile. +const colPadding = 2 +// errRenderPrefixLines is the number of fixed lines (the error message and +// a blank separator line) prepended before the rendered display file. +const errRenderPrefixLines = 2 + +// fileRenderPrefixLines is the number of fixed lines (the filepath and the +// header divider) prepended before the per-line rendered output. +const fileRenderPrefixLines = 2 + +// CommentError wraps an error encountered while parsing the YAML comments +// on node, so it can be rendered with source context. type CommentError struct { Filepath string Node *yaml.Node Err error } +// NewCommentError wraps err as a CommentError for node. +func NewCommentError(node *yaml.Node, err error) *CommentError { + return &CommentError{ + Node: node, + Err: err, + } +} + +// Render formats the underlying error alongside the source lines around +// node, for display to the user. func (e *CommentError) Render() string { lines := append( strings.Split(e.Node.HeadComment, "\n"), - fmt.Sprintf("%s: ...", e.Node.Value), + e.Node.Value+": ...", ) - displayFile := NewDisplayFile(e.Filepath) + displayFile := newDisplayFile(e.Filepath) for i, line := range lines { // +1 because we added the node value to the list of display lines @@ -36,19 +55,26 @@ func (e *CommentError) Render() string { } // update yaml error with adjusted line number - if yamlErr, ok := e.Err.(*yaml.LoadErrors); ok { + yamlErr := &yaml.LoadErrors{} + if errors.As(e.Err, &yamlErr) { for _, unmarshalErr := range yamlErr.Errors { // UnmarshalErrors report line number as 1-indexed unmarshalErr.Line = displayFile.Lines()[unmarshalErr.Line-1].LineNum } } - newLines := []string{e.Err.Error(), ""} - return strings.Join(append(newLines, displayFile.Render()...), "\n") + rendered := displayFile.Render() + + newLines := make([]string, 0, errRenderPrefixLines+len(rendered)) + newLines = append(newLines, e.Err.Error(), "") + newLines = append(newLines, rendered...) + + return strings.Join(newLines, "\n") } +// RenderToLog writes the rendered error to logger, one line per log call. func (e *CommentError) RenderToLog(logger *logrus.Logger) { - for _, l := range strings.Split(e.Render(), "\n") { + for l := range strings.SplitSeq(e.Render(), "\n") { logger.Warn(l) } } @@ -57,7 +83,17 @@ func (e *CommentError) Error() string { return e.Err.Error() } -func NewDisplayFile(filepath string) *displayFile { +// displayFile renders a set of source lines as a two-column, line-numbered +// listing for display in error output. +type displayFile struct { + filepath string + lines []displayLine + lcolWidth int + rcolWidth int +} + +// newDisplayFile creates an empty displayFile for the given source filepath. +func newDisplayFile(filepath string) *displayFile { return &displayFile{ filepath: filepath, lines: []displayLine{}, @@ -66,19 +102,37 @@ func NewDisplayFile(filepath string) *displayFile { } } -type displayFile struct { - filepath string - lines []displayLine - lcolWidth int - rcolWidth int -} - func (df *displayFile) Lines() []displayLine { return df.lines } +func (df *displayFile) AddLine(lineNum int, content string) { + df.lines = append(df.lines, displayLine{ + LineNum: lineNum, + Content: content, + }) + df.updateLeftColWidth(len(strconv.Itoa(lineNum))) + df.updateRightColWidth(len(content)) +} + +func (df *displayFile) Render() []string { + df.checkFilepathLength() + + output := make([]string, 0, fileRenderPrefixLines+len(df.lines)) + output = append(output, df.filepath) + output = append(output, df.headerLine()) + + for _, line := range df.lines { + lcol := df.renderLeftCol(line.LineNum) + rcol := df.renderRightCol(line.Content) + output = append(output, fmt.Sprintf("%s|%s", lcol, rcol)) + } + + return output +} + func (df *displayFile) paddedLeftColWidth() int { - return df.lcolWidth + 2 + return df.lcolWidth + colPadding } func (df *displayFile) paddedRightColWidth() int { @@ -97,27 +151,19 @@ func (df *displayFile) updateRightColWidth(value int) { } } -func (df *displayFile) AddLine(lineNum int, content string) { - df.lines = append(df.lines, displayLine{ - LineNum: lineNum, - Content: content, - }) - df.updateLeftColWidth(len(fmt.Sprintf("%d", lineNum))) - df.updateRightColWidth(len(content)) -} - func (df *displayFile) renderLeftCol(lineNum int) string { - padding := strings.Repeat(" ", df.paddedLeftColWidth()-len(fmt.Sprintf("%d", lineNum))) + padding := strings.Repeat(" ", df.paddedLeftColWidth()-len(strconv.Itoa(lineNum))) + return fmt.Sprintf("%d%s", lineNum, padding) } func (df *displayFile) renderRightCol(content string) string { - return fmt.Sprintf(" %s", content) + return " " + content } func (df *displayFile) checkFilepathLength() { if len(df.filepath) > len(df.headerLine()) { - df.updateRightColWidth(len(df.filepath) - (df.paddedLeftColWidth() + 2)) + df.updateRightColWidth(len(df.filepath) - (df.paddedLeftColWidth() + colPadding)) } } @@ -128,35 +174,7 @@ func (df *displayFile) headerLine() string { ) } -func (df *displayFile) Render() []string { - df.checkFilepathLength() - - output := []string{ - df.filepath, - } - - output = append(output, df.headerLine()) - - for _, line := range df.lines { - lcol := df.renderLeftCol(line.LineNum) - rcol := df.renderRightCol(line.Content) - output = append(output, fmt.Sprintf("%s|%s", lcol, rcol)) - } - - return output -} - type displayLine struct { LineNum int Content string } - -func lineNumWidth(lines []displayLine) int { - width := 0 - for _, line := range lines { - if len(fmt.Sprintf("%d", line.LineNum)) > width { - width = len(fmt.Sprintf("%d", line.LineNum)) - } - } - return width -} diff --git a/pkg/schema/comments/error_test.go b/pkg/schema/comments/error_test.go index 5ffcc9d..65a7d4e 100644 --- a/pkg/schema/comments/error_test.go +++ b/pkg/schema/comments/error_test.go @@ -1,16 +1,20 @@ -package comments +package comments_test import ( "bytes" "errors" "testing" + "helmvalues/pkg/schema/comments" + "github.com/sirupsen/logrus" "github.com/stretchr/testify/assert" "go.yaml.in/yaml/v4" ) func TestNewCommentError(t *testing.T) { + t.Parallel() + node := &yaml.Node{ Line: 10, Value: "test-value", @@ -18,7 +22,7 @@ func TestNewCommentError(t *testing.T) { } err := errors.New("test error") - commentErr := NewCommentError(node, err) + commentErr := comments.NewCommentError(node, err) assert.NotNil(t, commentErr) assert.Equal(t, node, commentErr.Node) @@ -27,10 +31,12 @@ func TestNewCommentError(t *testing.T) { } func TestCommentError_Error(t *testing.T) { + t.Parallel() + expectedMsg := "some error message" err := errors.New(expectedMsg) - commentErr := &CommentError{ + commentErr := &comments.CommentError{ Node: &yaml.Node{}, Err: err, } @@ -39,6 +45,8 @@ func TestCommentError_Error(t *testing.T) { } func TestCommentError_Render(t *testing.T) { + t.Parallel() + tests := []struct { name string filepath string @@ -123,7 +131,9 @@ func TestCommentError_Render(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - commentErr := &CommentError{ + t.Parallel() + + commentErr := &comments.CommentError{ Filepath: tt.filepath, Node: tt.node, Err: tt.err, @@ -139,6 +149,8 @@ func TestCommentError_Render(t *testing.T) { } func TestCommentError_Render_WithYamlTypeError(t *testing.T) { + t.Parallel() + node := &yaml.Node{ Line: 20, Value: "field", @@ -155,7 +167,7 @@ func TestCommentError_Render_WithYamlTypeError(t *testing.T) { }, } - commentErr := &CommentError{ + commentErr := &comments.CommentError{ Filepath: "config.yaml", Node: node, Err: yamlErr, @@ -173,6 +185,8 @@ func TestCommentError_Render_WithYamlTypeError(t *testing.T) { } func TestCommentError_RenderToLog_MultipleLines(t *testing.T) { + t.Parallel() + node := &yaml.Node{ Line: 10, Value: "value", @@ -180,13 +194,14 @@ func TestCommentError_RenderToLog_MultipleLines(t *testing.T) { } err := errors.New("multi-line test") - commentErr := &CommentError{ + commentErr := &comments.CommentError{ Filepath: "multi.yaml", Node: node, Err: err, } var buf bytes.Buffer + logger := logrus.New() logger.SetOutput(&buf) logger.SetLevel(logrus.WarnLevel) diff --git a/pkg/schema/comments/nodes.go b/pkg/schema/comments/nodes.go index 02a59d5..99415a3 100644 --- a/pkg/schema/comments/nodes.go +++ b/pkg/schema/comments/nodes.go @@ -46,6 +46,8 @@ func commentAsMapNodes(comment string) ([]*yaml.Node, bool) { return node.Content[0].Content, true } +// KeyValueNodes builds the pair of YAML scalar nodes representing a single +// "key: value" mapping entry. func KeyValueNodes(key string, value string) []*yaml.Node { keyNode := &yaml.Node{ Kind: yaml.ScalarNode, @@ -55,6 +57,7 @@ func KeyValueNodes(key string, value string) []*yaml.Node { Kind: yaml.ScalarNode, Value: value, } + return []*yaml.Node{keyNode, valueNode} } diff --git a/pkg/schema/config.go b/pkg/schema/config.go index fbec0b4..5319bb0 100644 --- a/pkg/schema/config.go +++ b/pkg/schema/config.go @@ -1,7 +1,10 @@ +// Package schema generates a JSON Schema from a chart's values file and +// writes it to disk. package schema import "github.com/sirupsen/logrus" +// Config controls how a chart's values schema is generated and written. type Config struct { StdOut bool Strict bool diff --git a/pkg/schema/generate.go b/pkg/schema/generate.go index c46df1d..0a4acaf 100644 --- a/pkg/schema/generate.go +++ b/pkg/schema/generate.go @@ -1,25 +1,39 @@ package schema import ( + "errors" "fmt" - "helmvalues/pkg" - "helmvalues/pkg/schema/comments" "os" "slices" "strings" + "helmvalues/pkg" + "helmvalues/pkg/schema/comments" + "github.com/samber/lo" "github.com/sirupsen/logrus" "go.yaml.in/yaml/v4" ) -const JsonSchemaURI = "http://json-schema.org/draft-07/schema#" +// JSONSchemaURI is the JSON Schema draft version URI written to the "$schema" field +// of generated schemas. +const JSONSchemaURI = "http://json-schema.org/draft-07/schema#" +// yamlKeyValuePairSize is the number of yaml.Node entries that make up a single +// key+value pair when chunking a mapping node's Content slice. +const yamlKeyValuePairSize = 2 + +// mappingNodeBaseExtraNodeCount is the number of "type"/"additionalProperties" comment +// nodes always added to a mapping node's extra nodes, used as a preallocation hint. +const mappingNodeBaseExtraNodeCount = 2 + +// Generator builds a JSON schema from a chart's values file. type Generator struct { logger *logrus.Logger plan *Plan } +// NewGenerator constructs a Generator for the given plan. func NewGenerator(logger *logrus.Logger, plan *Plan) *Generator { return &Generator{ logger: logger, @@ -27,16 +41,19 @@ func NewGenerator(logger *logrus.Logger, plan *Plan) *Generator { } } +// Generate reads and parses the plan's chart values file and builds a JSON schema +// describing its structure. func (g *Generator) Generate() (*pkg.JsonSchema, error) { f, err := os.ReadFile(g.plan.chart.ValuesFilePath()) if err != nil { - return nil, err + return nil, fmt.Errorf("failed to read values file: %w", err) } rootNode := &yaml.Node{} + err = yaml.Unmarshal(f, rootNode) if err != nil { - return nil, err + return nil, fmt.Errorf("failed to unmarshal values file: %w", err) } if rootNode.Kind != yaml.DocumentNode { @@ -47,7 +64,8 @@ func (g *Generator) Generate() (*pkg.JsonSchema, error) { if err != nil { return nil, err } - s.Schema = JsonSchemaURI + + s.Schema = JSONSchemaURI g.logger.Tracef("schmea generator, properties: %+v", s.Properties) s.WalkProperties( @@ -68,12 +86,14 @@ func (g *Generator) buildScalarNode(key *yaml.Node, value *yaml.Node) (*pkg.Json if valueType != "null" { extraNodes = append(extraNodes, comments.KeyValueNodes("type", valueType)...) } + extraNodes = append(extraNodes, comments.KeyValueNodes("title", key.Value)...) extraNodes = append(extraNodes, comments.KeyValueNodes("default", value.Value)...) s, err := comments.Parse(key, extraNodes) if err != nil { - if cErr, ok := err.(*comments.CommentError); ok { + cErr := &comments.CommentError{} + if errors.As(err, &cErr) { cErr.Filepath = g.plan.chart.ValuesFilePath() cErr.RenderToLog(g.logger) } @@ -81,15 +101,15 @@ func (g *Generator) buildScalarNode(key *yaml.Node, value *yaml.Node) (*pkg.Json err := fmt.Errorf("doc comment error: %w", err) if g.plan.StrictComments() { return nil, err - } else { - g.logger.Warn(err.Error()) } + + g.logger.Warn(err.Error()) } return s, nil } -// TODO: Finish handling sequences +// TODO: Finish handling sequences. func (g *Generator) buildSequenceNode(key *yaml.Node, _ *yaml.Node) (*pkg.JsonSchema, error) { extraNodes := []*yaml.Node{} extraNodes = append(extraNodes, comments.KeyValueNodes("type", "array")...) @@ -98,6 +118,7 @@ func (g *Generator) buildSequenceNode(key *yaml.Node, _ *yaml.Node) (*pkg.JsonSc if key == nil { s := &pkg.JsonSchema{} s.Properties = pkg.NewEncodableOrderedMap[string, *pkg.JsonSchema]() + return s, nil } @@ -105,7 +126,8 @@ func (g *Generator) buildSequenceNode(key *yaml.Node, _ *yaml.Node) (*pkg.JsonSc s, err := comments.Parse(key, extraNodes) if err != nil { - if cErr, ok := err.(*comments.CommentError); ok { + cErr := &comments.CommentError{} + if errors.As(err, &cErr) { cErr.Filepath = g.plan.chart.ValuesFilePath() cErr.RenderToLog(g.logger) } @@ -119,60 +141,93 @@ func (g *Generator) buildSequenceNode(key *yaml.Node, _ *yaml.Node) (*pkg.JsonSc return s, nil } +// buildMappingNodeTitledSchema parses the doc comments attached to key into a schema, +// with a "title" field derived from the key added to extraNodes. This is only relevant +// when the mapping node being built has a yaml key node (i.e. it isn't the root node). +func (g *Generator) buildMappingNodeTitledSchema(key *yaml.Node, extraNodes []*yaml.Node) (*pkg.JsonSchema, error) { + extraNodes = append(extraNodes, comments.KeyValueNodes("title", key.Value)...) + + s, err := comments.Parse(key, extraNodes) + if err != nil { + cErr := &comments.CommentError{} + if errors.As(err, &cErr) { + cErr.Filepath = g.plan.chart.ValuesFilePath() + cErr.RenderToLog(g.logger) + } + + wrappedErr := fmt.Errorf("doc comment error: %w", err) + if g.plan.StrictComments() { + return nil, wrappedErr + } + } + + return s, nil +} + +// buildChildNodeSchema builds the schema for a single key/value pair found in a +// mapping node's Content, dispatching to the appropriate builder based on the +// value node's kind. +func (g *Generator) buildChildNodeSchema(childKey *yaml.Node, childValue *yaml.Node) (*pkg.JsonSchema, error) { + switch childValue.Kind { + case yaml.ScalarNode: + childValueSchema, err := g.buildScalarNode(childKey, childValue) + if err != nil { + g.logger.Debugf("Error building scalar node for key %s: %v", childKey.Value, err) + + return nil, err + } + + return childValueSchema, nil + case yaml.SequenceNode: + childValueSchema, err := g.buildSequenceNode(childKey, childValue) + if err != nil { + g.logger.Debugf("Error building sequence node for key %s: %v", childKey.Value, err) + + return nil, err + } + + return childValueSchema, nil + case yaml.MappingNode: + childValueSchema, err := g.buildMappingNode(childKey, childValue) + if err != nil { + g.logger.Debugf("Error building mapping node for key %s: %v", childKey.Value, err) + + return nil, err + } + + return childValueSchema, nil + default: + // should be impossible + return nil, fmt.Errorf("unsupported yaml type: %v", childValue.Kind) + } +} + func (g *Generator) buildMappingNode(key *yaml.Node, value *yaml.Node) (*pkg.JsonSchema, error) { - extraNodes := []*yaml.Node{} + extraNodes := make([]*yaml.Node, 0, mappingNodeBaseExtraNodeCount) extraNodes = append(extraNodes, comments.KeyValueNodes("type", "object")...) extraNodes = append(extraNodes, comments.KeyValueNodes("additionalProperties", "false")...) // Not all objects will have a yaml key node, only set key values if they exist s := &pkg.JsonSchema{} - if key != nil { - extraNodes = append(extraNodes, comments.KeyValueNodes("title", key.Value)...) + if key != nil { var err error - s, err = comments.Parse(key, extraNodes) - if err != nil { - if cErr, ok := err.(*comments.CommentError); ok { - cErr.Filepath = g.plan.chart.ValuesFilePath() - cErr.RenderToLog(g.logger) - } - err := fmt.Errorf("doc comment error: %w", err) - if g.plan.StrictComments() { - return nil, err - } + s, err = g.buildMappingNodeTitledSchema(key, extraNodes) + if err != nil { + return nil, err } } + s.Properties = pkg.NewEncodableOrderedMap[string, *pkg.JsonSchema]() - for _, child := range lo.Chunk(value.Content, 2) { + for _, child := range lo.Chunk(value.Content, yamlKeyValuePairSize) { childKey := child[0] childValue := child[1] - var err error - var childValueSchema *pkg.JsonSchema - switch childValue.Kind { - case yaml.ScalarNode: - childValueSchema, err = g.buildScalarNode(childKey, childValue) - if err != nil { - g.logger.Debugf("Error building scalar node for key %s: %v", childKey.Value, err) - return nil, err - } - case yaml.SequenceNode: - childValueSchema, err = g.buildSequenceNode(childKey, childValue) - if err != nil { - g.logger.Debugf("Error building sequence node for key %s: %v", childKey.Value, err) - return nil, err - } - case yaml.MappingNode: - childValueSchema, err = g.buildMappingNode(childKey, childValue) - if err != nil { - g.logger.Debugf("Error building mapping node for key %s: %v", childKey.Value, err) - return nil, err - } - default: - // should be impossible - return nil, fmt.Errorf("unsupported yaml type: %v", childValue.Kind) + childValueSchema, err := g.buildChildNodeSchema(childKey, childValue) + if err != nil { + return nil, err } s.Properties.Set(childKey.Value, childValueSchema) @@ -231,10 +286,12 @@ func (g *Generator) warnUndocumentedValue(keyPath []*pkg.JsonSchema, schema *pkg } keyValues := []string{} + for _, k := range append(keyPath, schema) { if k.Title == "" { continue } + keyValues = append(keyValues, k.Title) } @@ -252,10 +309,12 @@ func (g *Generator) warnUntypedValue(keyPath []*pkg.JsonSchema, schema *pkg.Json } keyValues := []string{} + for _, k := range append(keyPath, schema) { if k.Title == "" { continue } + keyValues = append(keyValues, k.Title) } diff --git a/pkg/schema/modeline.go b/pkg/schema/modeline.go index 3be637e..a2adf78 100644 --- a/pkg/schema/modeline.go +++ b/pkg/schema/modeline.go @@ -2,12 +2,15 @@ package schema import ( "fmt" + "helmvalues/pkg/charts" "helmvalues/pkg/modeline" "github.com/sirupsen/logrus" ) +// WriteSchemaModeline writes a yaml-language-server modeline pointing at the chart's +// generated schema file into the values file at valuesPath. func WriteSchemaModeline(logger *logrus.Logger, chart *charts.Chart, valuesPath string, dryRun bool) error { fileManager, err := modeline.NewFileModelineManager(valuesPath) if err != nil { @@ -19,8 +22,13 @@ func WriteSchemaModeline(logger *logrus.Logger, chart *charts.Chart, valuesPath if dryRun { logger.Infof("schema: %s: dry-run enabled, skipping modeline write to %s", chart.Details.Name, valuesPath) + return nil } - return fileManager.Write(false) + if err := fileManager.Write(false); err != nil { + return fmt.Errorf("failed to write modeline: %w", err) + } + + return nil } diff --git a/pkg/schema/plan.go b/pkg/schema/plan.go index 9c9ddba..079804b 100644 --- a/pkg/schema/plan.go +++ b/pkg/schema/plan.go @@ -1,16 +1,25 @@ package schema import ( + "context" "encoding/json" "fmt" - "helmvalues/pkg" - "helmvalues/pkg/charts" "os" "os/exec" + "helmvalues/pkg" + "helmvalues/pkg/charts" + "github.com/sirupsen/logrus" ) +// Plan holds the resolved config and target chart for a single schema generation run. +type Plan struct { + cfg *Config + chart *charts.Chart +} + +// NewPlan constructs a Plan combining the given schema config with the target chart. func NewPlan(cfg *Config, chart *charts.Chart) *Plan { return &Plan{ chart: chart, @@ -18,11 +27,7 @@ func NewPlan(cfg *Config, chart *charts.Chart) *Plan { } } -type Plan struct { - cfg *Config - chart *charts.Chart -} - +// LogCommonDetails logs the config values shared across schema generation plans. func (p *Plan) LogCommonDetails(logger *logrus.Logger) { // common configs logger.Debugf("plan: %s: DryRun=%t", p.chart.Details.Name, p.DryRun()) @@ -30,6 +35,7 @@ func (p *Plan) LogCommonDetails(logger *logrus.Logger) { logger.Debugf("plan: %s: Stdout=%t", p.chart.Details.Name, p.StdOut()) } +// LogChartDetails logs the resolved file paths for the plan's target chart. func (p *Plan) LogChartDetails(logger *logrus.Logger) { // chart configs logger.Debugf("plan: %s: ChartRoot=%s", p.chart.Details.Name, p.chart.RootPath()) @@ -39,34 +45,41 @@ func (p *Plan) LogChartDetails(logger *logrus.Logger) { // logger.Debugf("plan: %s: ChartReadmeTemplate=%s", p.chart.Details.Name, p.DocsChartReadmeTemplate()) } +// LogSchemaDetails logs the schema-specific config values for the plan. func (p *Plan) LogSchemaDetails(logger *logrus.Logger) { logger.Debugf("plan: %s: WriteModeline=%t", p.chart.Details.Name, p.cfg.WriteModeline) } +// Chart returns the plan's target chart. func (p *Plan) Chart() *charts.Chart { return p.chart } +// StdOut reports whether the generated schema should also be printed to stdout. func (p *Plan) StdOut() bool { return p.cfg.StdOut } +// StrictComments reports whether doc comment errors should be treated as fatal. func (p *Plan) StrictComments() bool { return p.cfg.Strict } +// GitAdd reports whether the generated schema file should be staged with git add. func (p *Plan) GitAdd() bool { return p.cfg.GitAdd } +// DryRun reports whether the plan should avoid writing any files. func (p *Plan) DryRun() bool { return p.cfg.DryRun } -func (p *Plan) WriteSchema(logger *logrus.Logger, schema *pkg.JsonSchema) error { +// 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, "", " ") if err != nil { - return err + return fmt.Errorf("failed to marshal schema: %w", err) } if p.StdOut() { @@ -79,18 +92,22 @@ func (p *Plan) WriteSchema(logger *logrus.Logger, schema *pkg.JsonSchema) error f, err := os.Create(p.chart.SchemaFilePath()) if err != nil { - return err + return fmt.Errorf("failed to create schema file: %w", err) } - defer f.Close() + defer func() { _ = f.Close() }() - _, err = f.WriteString(string(content)) + _, err = f.Write(content) if err != nil { - return err + return fmt.Errorf("failed to write schema file: %w", err) } if p.GitAdd() { - err := exec.Command("git", "add", p.chart.SchemaFilePath()).Run() - if err != nil { + // The git add here is an intentional, user-opted-in integration (via the GitAdd + // config flag) that stages the schema file this process just wrote; the arguments + // are not attacker-controlled input. + //nolint:gosec // opt-in git integration, not attacker input + cmd := exec.CommandContext(context.Background(), "git", "add", p.chart.SchemaFilePath()) + if err := cmd.Run(); err != nil { return fmt.Errorf("failed to git add %s: %w", p.chart.SchemaFilePath(), err) } } diff --git a/pkg/schema/schema.go b/pkg/schema/schema.go index 61138f3..500deac 100644 --- a/pkg/schema/schema.go +++ b/pkg/schema/schema.go @@ -1,19 +1,24 @@ package schema import ( + "fmt" + "helmvalues/pkg/charts" "github.com/sirupsen/logrus" ) +// GenerateSchema searches chartDirs for charts and generates a JSON schema (and, +// depending on cfg, a values file modeline) for each one found. func GenerateSchema(logger *logrus.Logger, cfg *Config, chartDirs []string) error { chartsFound, err := charts.Search(logger, chartDirs) if err != nil { - return err + return fmt.Errorf("failed to search charts: %w", err) } // Itterate through plan to set the logger and config plans := []*Plan{} + for _, chart := range chartsFound { plan := NewPlan(cfg, chart) plan.LogCommonDetails(logger) @@ -26,20 +31,25 @@ func GenerateSchema(logger *logrus.Logger, cfg *Config, chartDirs []string) erro // Iterate through plans again, this time generating the schema 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 nil } logger.Debugf("schema: %s: writing output", plan.Chart().Details.Name) + if err := plan.WriteSchema(logger, schema); err != nil { logger.Error(err.Error()) + return nil } if cfg.WriteModeline { logger.Debugf("schema: %s: writing modeline", plan.Chart().Details.Name) + err := WriteSchemaModeline( logger, plan.Chart(), @@ -48,6 +58,7 @@ func GenerateSchema(logger *logrus.Logger, cfg *Config, chartDirs []string) erro ) if err != nil { logger.Error(err.Error()) + return nil } } else { diff --git a/taskfile.yaml b/taskfile.yaml index cb60557..7ebac11 100644 --- a/taskfile.yaml +++ b/taskfile.yaml @@ -28,6 +28,11 @@ tasks: - helm plugin uninstall values || true - helm plugin install ./dist/values-$(git describe --tags --abbrev=0).tgz + lint: + desc: Run golangci-lint + cmds: + - golangci-lint run ./... + test: desc: Run all tests cmds: From b7cd165ea6f8b98c225509d4861200df96ca92d0 Mon Sep 17 00:00:00 2001 From: Brahm Lower Date: Sun, 16 Aug 2026 22:50:54 +0000 Subject: [PATCH 2/2] fix: resolve CI lint and pre-commit job failures - exhaustive linter flagged the yaml.Kind switch in generate.go for not listing DocumentNode/AliasNode/StreamNode even though a default case already errors on unhandled kinds; enable default-signifies-exhaustive so a default case satisfies the check as intended. - helm plugin install now enforces signature verification by default and errors when no .prov file is present; pass --verify=false since the local snapshot plugin is intentionally built unsigned (--sign=false). --- .golangci.yml | 7 +++++++ taskfile.yaml | 2 +- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/.golangci.yml b/.golangci.yml index 9024f22..cbf10dd 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -42,6 +42,13 @@ linters: - pkg: io/ioutil desc: "io/ioutil is deprecated, use io or os instead" + exhaustive: + # Switches over third-party enums (e.g. yaml.Kind) commonly only care + # about a handful of cases and treat the rest as an error via a + # default case; requiring every variant to be listed explicitly adds + # no value there. + default-signifies-exhaustive: true + forbidigo: # Only flag the bare print/println builtins (debug leftovers); this is # a CLI tool where fmt.Print* is legitimate, intentional user output. diff --git a/taskfile.yaml b/taskfile.yaml index 7ebac11..5bd00be 100644 --- a/taskfile.yaml +++ b/taskfile.yaml @@ -26,7 +26,7 @@ tasks: desc: Reinstall the plugin locally cmds: - helm plugin uninstall values || true - - helm plugin install ./dist/values-$(git describe --tags --abbrev=0).tgz + - helm plugin install ./dist/values-$(git describe --tags --abbrev=0).tgz --verify=false lint: desc: Run golangci-lint