diff --git a/README.md b/README.md index b9a512ef2..cebe7ea43 100644 --- a/README.md +++ b/README.md @@ -257,6 +257,37 @@ linters-settings: - /var/lib/kubelet ``` +#### Per-scope settings + +`linters-settings` configures the source tree. The two images a published module +consists of are configured separately, under `remote`: they carry different files +and are linted by different rules, so each gets its own section. + +```yaml +linters-settings: # dmt lint + openapi: + impact: error + +remote: + bundle: # dmt lint remote : + documentation: + rules: + changelog: + impact: warn + release: # ... the same command, /release: + module: + rules: + release-layout: + impact: error +``` + +A `remote` section has the same shape as `global.linters-settings` — a linter +`impact` and per-rule impacts. `exclude-rules` are not read there. + +The sections are independent: nothing from `linters-settings` or `global` reaches +a remote scope, and a section left out means the built-in severities rather than +the ones the source tree happens to be tuned to. + ### Rule: mount-points The `mount-points` rule validates that volume mounts in pod controllers match the declarations in `mount-points.yaml` files (and vice versa). It runs in two directions: @@ -317,6 +348,40 @@ dmt lint ./my-module --values-file custom-values.yaml dmt lint ./my-module --log-level debug ``` +#### Lint Remote Command + +```bash +dmt lint remote : [flags] +``` + +Lints the published images instead of a directory: the bundle at `:` +and the release at `/release:`. + +**Flags:** +- `--login` / `--password`: Registry credentials + +Credentials are resolved in this order: the flags, then the `DMT_REGISTRY_LOGIN` / +`DMT_REGISTRY_PASSWORD` environment variables, then the Docker config +(`~/.docker/config.json`), then anonymous access. Each field falls back on its own, so +the login can come from a flag and the password from a secret. In CI prefer the +environment variables — a password passed as a flag shows up in the process list and +in the job log. + +The `lint` flags are inherited, so `--linter`, `--hide-warnings`, `--show-ignored` and +`--log-level` apply here too. + +**Example:** +```bash +dmt lint remote registry.example.com/my-module:v0.0.1 + +# Lint only the module linter in both published images +dmt lint remote registry.example.com/my-module:v0.0.1 --linter module + +# CI: credentials from the environment +DMT_REGISTRY_LOGIN=license-token DMT_REGISTRY_PASSWORD="$REGISTRY_TOKEN" \ + dmt lint remote registry.example.com/my-module:v0.0.1 +``` + #### Bootstrap Command ```bash diff --git a/cmd/dmt/main.go b/cmd/dmt/main.go index 9cb078e5a..a6f98dc5f 100644 --- a/cmd/dmt/main.go +++ b/cmd/dmt/main.go @@ -19,6 +19,7 @@ package main import ( "context" "errors" + "fmt" "log/slog" "os" "runtime" @@ -34,14 +35,16 @@ import ( "github.com/deckhouse/dmt/internal/metrics" "github.com/deckhouse/dmt/internal/version" "github.com/deckhouse/dmt/pkg/config" - "github.com/deckhouse/dmt/pkg/scopes" ) func main() { execute() } -func runLint(ctx context.Context, dir string) error { +// runLint is the whole of a lint run: everything but where the modules come from, +// which is the source's business. Both `dmt lint ` and `dmt lint remote ` +// enter here, so setup and teardown exist once. +func runLint(ctx context.Context, src manager.Source) error { if flags.PprofFile != "" { log.Info("Profiling enabled", slog.String("file", flags.PprofFile)) @@ -76,16 +79,20 @@ func runLint(ctx context.Context, dir string) error { log.Info("DMT version", slog.String("version", version.Version), slog.String("commit", version.Commit), slog.String("date", version.Date)) + dir := src.ConfigDir() + cfg, err := config.NewDefaultRootConfig(dir) if err != nil { - log.Fatal("default root config", log.Err(err)) //nolint:gocritic + return fmt.Errorf("default root config: %w", err) } // init metrics storage, should be done before running manager metrics.GetClient(dir) - mng := manager.NewManager(dir, cfg, scopes.Static) - mng.Run(ctx) + mng := manager.New(cfg, src) + defer mng.Close() + + sourceErr := mng.Run(ctx) if flags.Fix { mng.ApplyFixes() @@ -94,13 +101,13 @@ func runLint(ctx context.Context, dir string) error { mng.PrintResult() mng.PrintStatistics() - metrics.SetDmtInfo() - metrics.SetLinterWarningsMetrics(cfg.GlobalSettings) - metrics.SetDmtRuntimeDuration() - metrics.SetDmtRuntimeDurationSeconds() + metrics.Flush(ctx, mng.MetricsSections()...) - metricsClient := metrics.GetClient(dir) - metricsClient.Send(context.WithoutCancel(ctx)) + // A source failure — a registry that would not answer — is reported after the + // findings, never instead of them: whatever was linted still has to be seen. + if sourceErr != nil { + return sourceErr + } if mng.HasCriticalErrors() { return errors.New("critical errors found") diff --git a/cmd/dmt/root.go b/cmd/dmt/root.go index b0a801a1c..6d820a384 100644 --- a/cmd/dmt/root.go +++ b/cmd/dmt/root.go @@ -36,6 +36,8 @@ import ( "github.com/deckhouse/dmt/internal/flags" "github.com/deckhouse/dmt/internal/fsutils" "github.com/deckhouse/dmt/internal/rendercmd" + "github.com/deckhouse/dmt/internal/sources/remote" + "github.com/deckhouse/dmt/internal/sources/static" "github.com/deckhouse/dmt/internal/test" "github.com/deckhouse/dmt/internal/version" "github.com/deckhouse/dmt/pkg/config" @@ -148,7 +150,40 @@ func execute() { }, } - lintCmd.Flags().AddFlagSet(flags.InitLintFlagSet()) + remoteCmd := &cobra.Command{ + Use: "remote :", + Short: "lint the published images instead of a directory", + Long: `Lints a module as it was published: pulls the bundle image at : and +the release image at /release:, and runs the scopes that belong to +them. Severities come from the 'remote.bundle' and 'remote.release' sections of +the config next to the caller.`, + Example: " dmt lint remote registry.example.com/my-module:v0.0.1", + Args: cobra.ExactArgs(1), + SilenceUsage: true, + RunE: func(cmd *cobra.Command, args []string) error { + // --fix comes down from `lint`, but there is nothing here to fix: the tree + // is an extracted image that is thrown away at the end of the run, and + // applying a fix would drop the finding from the report for nothing. + if flags.Fix { + return errors.New("--fix is not supported for remote lint") + } + + src, err := remote.NewSource(args[0], &remote.Options{ + Login: flags.RemoteLogin, + Password: flags.RemotePassword, + }) + if err != nil { + return err + } + + return runLint(cmd.Context(), src) + }, + } + remoteCmd.Flags().AddFlagSet(flags.InitRemoteFlagSet()) + + // Persistent, so 'lint remote' inherits --log-level and friends. + lintCmd.PersistentFlags().AddFlagSet(flags.InitLintFlagSet()) + lintCmd.AddCommand(remoteCmd) bootstrapCmd.Flags().AddFlagSet(flags.InitBootstrapFlagSet()) testCmd := &cobra.Command{ @@ -296,7 +331,7 @@ func runLintMultiple(ctx context.Context, dirs []string) error { log.Info("Processing directory", slog.String("directory", expandedDir)) // Run lint for this directory as a separate execution - if err := runLint(ctx, expandedDir); err != nil { + if err := runLint(ctx, static.NewSource(expandedDir)); err != nil { log.Error("Error processing directory", slog.String("directory", expandedDir), log.Err(err)) hasErrors = true diff --git a/go.mod b/go.mod index fcddec302..bc0a720e6 100644 --- a/go.mod +++ b/go.mod @@ -8,6 +8,7 @@ require ( github.com/Masterminds/sprig/v3 v3.3.0 github.com/bmatcuk/doublestar v1.3.4 github.com/deckhouse/deckhouse/pkg/log v0.2.1 + github.com/deckhouse/deckhouse/pkg/registry v0.0.0-20260831072828-0356cb79de29 github.com/fatih/color v1.19.0 github.com/go-openapi/spec v0.22.4 github.com/gogo/protobuf v1.3.2 @@ -80,6 +81,7 @@ require ( github.com/containerd/errdefs v0.3.0 // indirect github.com/containerd/log v0.1.0 // indirect github.com/containerd/platforms v0.2.1 // indirect + github.com/containerd/stargz-snapshotter/estargz v0.16.3 // indirect github.com/cyphar/filepath-securejoin v0.4.1 // indirect github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc // indirect github.com/dennwc/varint v1.0.0 // indirect @@ -87,10 +89,10 @@ require ( github.com/djherbis/buffer v1.2.0 // indirect github.com/djherbis/nio/v3 v3.0.1 // indirect github.com/dlclark/regexp2 v1.11.4 // indirect - github.com/docker/cli v27.1.1+incompatible // indirect + github.com/docker/cli v29.2.0+incompatible // indirect github.com/docker/distribution v2.8.3+incompatible // indirect github.com/docker/docker v27.4.1+incompatible // indirect - github.com/docker/docker-credential-helpers v0.8.2 // indirect + github.com/docker/docker-credential-helpers v0.9.3 // indirect github.com/docker/go-connections v0.5.0 // indirect github.com/docker/go-metrics v0.0.1 // indirect github.com/dominikbraun/graph v0.23.0 // indirect @@ -214,6 +216,7 @@ require ( github.com/tidwall/match v1.1.1 // indirect github.com/tidwall/pretty v1.2.1 // indirect github.com/tidwall/sjson v1.2.5 // indirect + github.com/vbatts/tar-split v0.12.1 // indirect github.com/wI2L/jsondiff v0.5.0 // indirect github.com/werf/common-go v0.0.0-20251113140850-a1a98e909e9b // indirect github.com/werf/kubedog v0.13.1-0.20260616105957-2c00b08fb99e // indirect @@ -277,3 +280,10 @@ require ( sigs.k8s.io/randfill v1.0.0 // indirect sigs.k8s.io/structured-merge-diff/v6 v6.3.0 // indirect ) + +// deckhouse/pkg/registry pulls docker/cli v29, whose types.AuthConfig no longer +// converts from docker/docker's registry.AuthConfig — which breaks oras.land/oras-go, +// reached through werf/nelm and unfixed on every v1.2.x. docker/cli is an indirect +// requirement of the registry client (go-containerregistry's Docker-config keychain), +// so holding it at the version the rest of the tree already builds against is enough. +replace github.com/docker/cli => github.com/docker/cli v27.1.1+incompatible diff --git a/go.sum b/go.sum index dc60960c9..dd29a2a7c 100644 --- a/go.sum +++ b/go.sum @@ -118,6 +118,8 @@ github.com/containerd/log v0.1.0 h1:TCJt7ioM2cr/tfR8GPbGf9/VRAX8D2B4PjzCpfX540I= github.com/containerd/log v0.1.0/go.mod h1:VRRf09a7mHDIRezVKTRCrOq78v577GXq3bSa3EhrzVo= github.com/containerd/platforms v0.2.1 h1:zvwtM3rz2YHPQsF2CHYM8+KtB5dvhISiXh5ZpSBQv6A= github.com/containerd/platforms v0.2.1/go.mod h1:XHCb+2/hzowdiut9rkudds9bE5yJ7npe7dG/wG+uFPw= +github.com/containerd/stargz-snapshotter/estargz v0.16.3 h1:7evrXtoh1mSbGj/pfRccTampEyKpjpOnS3CyiV1Ebr8= +github.com/containerd/stargz-snapshotter/estargz v0.16.3/go.mod h1:uyr4BfYfOj3G9WBVE8cOlQmXAbPN9VEQpBBeJIuOipU= github.com/coreos/go-systemd/v22 v22.5.0 h1:RrqgGjYQKalulkV8NGVIfkXQf6YYmOyiJKk8iXXhfZs= github.com/coreos/go-systemd/v22 v22.5.0/go.mod h1:Y58oyj3AT4RCenI/lSvhwexgC+NSVTIJ3seZv2GcEnc= github.com/cpuguy83/go-md2man/v2 v2.0.6/go.mod h1:oOW0eioCTA6cOiMLiUPZOpcVxMig6NIQQ7OS05n1F4g= @@ -131,6 +133,8 @@ github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc h1:U9qPSI2PIWSS1 github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/deckhouse/deckhouse/pkg/log v0.2.1 h1:7SSD+QJPnziAO3l8ycgRwN5wlQeC312Tf7f2Wa/hWDg= github.com/deckhouse/deckhouse/pkg/log v0.2.1/go.mod h1:pbAxTSDcPmwyl3wwKDcEB3qdxHnRxqTV+J0K+sha8bw= +github.com/deckhouse/deckhouse/pkg/registry v0.0.0-20260831072828-0356cb79de29 h1:Zy/Lj8Sj6ZQ0pXzAN0uyChezenIcgPVJUP8I/1OsR38= +github.com/deckhouse/deckhouse/pkg/registry v0.0.0-20260831072828-0356cb79de29/go.mod h1:KDf44MqEif8jAKCehKJqOg0k4sJcnetKJKDGd0IFQjI= github.com/dennwc/varint v1.0.0 h1:kGNFFSSw8ToIy3obO/kKr8U9GZYUAxQEVuix4zfDWzE= github.com/dennwc/varint v1.0.0/go.mod h1:hnItb35rvZvJrbTALZtY/iQfDs48JKRG1RPpgziApxA= github.com/dgryski/go-rendezvous v0.0.0-20200823014737-9f7001d12a5f h1:lO4WD4F/rVNCu3HqELle0jiPLLBs70cWOduZpkS1E78= @@ -154,8 +158,8 @@ github.com/docker/distribution v2.8.3+incompatible h1:AtKxIZ36LoNK51+Z6RpzLpddBi github.com/docker/distribution v2.8.3+incompatible/go.mod h1:J2gT2udsDAN96Uj4KfcMRqY0/ypR+oyYUYmja8H+y+w= github.com/docker/docker v27.4.1+incompatible h1:ZJvcY7gfwHn1JF48PfbyXg7Jyt9ZCWDW+GGXOIxEwp4= github.com/docker/docker v27.4.1+incompatible/go.mod h1:eEKB0N0r5NX/I1kEveEz05bcu8tLC/8azJZsviup8Sk= -github.com/docker/docker-credential-helpers v0.8.2 h1:bX3YxiGzFP5sOXWc3bTPEXdEaZSeVMrFgOr3T+zrFAo= -github.com/docker/docker-credential-helpers v0.8.2/go.mod h1:P3ci7E3lwkZg6XiHdRKft1KckHiO9a2rNtyFbZ/ry9M= +github.com/docker/docker-credential-helpers v0.9.3 h1:gAm/VtF9wgqJMoxzT3Gj5p4AqIjCBS4wrsOh9yRqcz8= +github.com/docker/docker-credential-helpers v0.9.3/go.mod h1:x+4Gbw9aGmChi3qTLZj8Dfn0TD20M/fuWy0E5+WDeCo= github.com/docker/go-connections v0.5.0 h1:USnMq7hx7gwdVZq1L49hLXaFtUdTADjXGp+uj1Br63c= github.com/docker/go-connections v0.5.0/go.mod h1:ov60Kzw0kKElRwhNs9UlUHAE/F9Fe6GLaXnqyDdmEXc= github.com/docker/go-events v0.0.0-20190806004212-e31b211e4f1c h1:+pKlWGMw7gf6bQ+oDZB4KHQFypsfjYlq/C4rfL7D3g8= @@ -398,8 +402,8 @@ github.com/json-iterator/go v1.1.7/go.mod h1:KdQUCv79m/52Kvf8AW2vK1V8akMuk1QjK/u github.com/json-iterator/go v1.1.12 h1:PV8peI4a0ysnczrg+LtxykD8LfKY9ML6u2jnxaEnrnM= github.com/json-iterator/go v1.1.12/go.mod h1:e30LSqwooZae/UwlEbR2852Gd8hjQvJoHmT4TnhNGBo= github.com/julienschmidt/httprouter v1.2.0/go.mod h1:SYymIcj16QtmaHHD7aYtjjsJG7VTCxuUUipMqKk8s4w= -github.com/keybase/go-keychain v0.0.0-20231219164618-57a3676c3af6 h1:IsMZxCuZqKuao2vNdfD82fjjgPLfyHLpR41Z88viRWs= -github.com/keybase/go-keychain v0.0.0-20231219164618-57a3676c3af6/go.mod h1:3VeWNIJaW+O5xpRQbPp0Ybqu1vJd/pm7s2F473HRrkw= +github.com/keybase/go-keychain v0.0.1 h1:way+bWYa6lDppZoZcgMbYsvC7GxljxrskdNInRtuthU= +github.com/keybase/go-keychain v0.0.1/go.mod h1:PdEILRW3i9D8JcdM+FmY6RwkHGnhHxXwkPPMeUgOK1k= github.com/kisielk/errcheck v1.5.0/go.mod h1:pFxgyoBC7bSaBwPgfKdkLd5X25qrDl4LWUI2bnpBCr8= github.com/kisielk/gotool v1.0.0/go.mod h1:XhKaO+MFFWcvkIS/tQcRk01m1F5IRFswLeQ+oQHNcck= github.com/klauspost/compress v1.18.0 h1:c/Cqfb0r+Yi+JtIEq73FWXVkRonBlf0CRNYc8Zttxdo= @@ -635,6 +639,8 @@ github.com/tidwall/pretty v1.2.1 h1:qjsOFOWWQl+N3RsoF5/ssm1pHmJJwhjlSbZ51I6wMl4= github.com/tidwall/pretty v1.2.1/go.mod h1:ITEVvHYasfjBbM0u2Pg8T2nJnzm8xPwvNhhsoaGGjNU= github.com/tidwall/sjson v1.2.5 h1:kLy8mja+1c9jlljvWTlSazM7cKDRfJuR/bOJhcY5NcY= github.com/tidwall/sjson v1.2.5/go.mod h1:Fvgq9kS/6ociJEDnK0Fk1cpYF4FIW6ZF7LAe+6jwd28= +github.com/vbatts/tar-split v0.12.1 h1:CqKoORW7BUWBe7UL/iqTVvkTBOF8UvOMKOIZykxnnbo= +github.com/vbatts/tar-split v0.12.1/go.mod h1:eF6B6i6ftWQcDqEn3/iGFRFRo8cBIMSJVOpnNdfTMFA= github.com/vultr/govultr/v2 v2.17.2 h1:gej/rwr91Puc/tgh+j33p/BLR16UrIPnSr+AIwYWZQs= github.com/vultr/govultr/v2 v2.17.2/go.mod h1:ZFOKGWmgjytfyjeyAdhQlSWwTjh2ig+X49cAp50dzXI= github.com/wI2L/jsondiff v0.5.0 h1:RRMTi/mH+R2aXcPe1VYyvGINJqQfC3R+KSEakuU1Ikw= diff --git a/internal/flags/flags.go b/internal/flags/flags.go index 456200cc6..21705c7fa 100644 --- a/internal/flags/flags.go +++ b/internal/flags/flags.go @@ -32,6 +32,11 @@ var ( LinterName string ) +var ( + RemoteLogin string + RemotePassword string +) + var ( PrintVersion bool Version string @@ -106,6 +111,15 @@ func InitLintFlagSet() *pflag.FlagSet { return lint } +func InitRemoteFlagSet() *pflag.FlagSet { + remote := pflag.NewFlagSet("remote", pflag.ContinueOnError) + + remote.StringVar(&RemoteLogin, "login", "", "registry login (defaults to $DMT_REGISTRY_LOGIN, then the Docker config)") + remote.StringVar(&RemotePassword, "password", "", "registry password (defaults to $DMT_REGISTRY_PASSWORD, then the Docker config)") + + return remote +} + func InitBootstrapFlagSet() *pflag.FlagSet { bootstrap := pflag.NewFlagSet("bootstrap", pflag.ContinueOnError) diff --git a/internal/manager/manager.go b/internal/manager/manager.go index a6fa4f753..e365f4753 100644 --- a/internal/manager/manager.go +++ b/internal/manager/manager.go @@ -22,7 +22,6 @@ import ( "context" "fmt" "log/slog" - "path/filepath" "slices" "strings" "sync" @@ -32,18 +31,16 @@ import ( "github.com/fatih/color" "github.com/kyokomi/emoji" "github.com/mitchellh/go-wordwrap" - "helm.sh/helm/v3/pkg/chartutil" "github.com/deckhouse/deckhouse/pkg/log" "github.com/deckhouse/dmt/internal/flags" - "github.com/deckhouse/dmt/internal/fsutils" "github.com/deckhouse/dmt/internal/metrics" - "github.com/deckhouse/dmt/internal/moduleloader" "github.com/deckhouse/dmt/internal/modules" - "github.com/deckhouse/dmt/internal/modules/values" + "github.com/deckhouse/dmt/internal/set" "github.com/deckhouse/dmt/pkg" "github.com/deckhouse/dmt/pkg/config" + "github.com/deckhouse/dmt/pkg/config/global" "github.com/deckhouse/dmt/pkg/errors" "github.com/deckhouse/dmt/pkg/scopes" ) @@ -60,127 +57,173 @@ func generateDocumentationURL(linterID, ruleID string) string { return fmt.Sprintf("%s/pkg/linters/%s#%s", baseRepoURL, linterID, ruleID) } -type Manager struct { - cfg *config.RootConfig - Modules []*modules.Module - - // scope decides which linters run and which of their rules each one is asked for. - scope scopes.Scope +// Target is one unit of work: a module and the scope it is linted in. The unit is a +// pair rather than a module because a remote run reads one module from two images and +// lints each in its own scope. +type Target struct { + Module *modules.Module + Scope scopes.Scope + // ModuleID is what the source calls one module. Targets sharing it count as one + // module in the summary, which is how a remote run reports the two images it read + // as the single module they are. It is not the module's name: a tree can hold two + // directories declaring the same name, and those are two modules. + ModuleID string + // ObjectID tags this target's findings. The remote source sets the scope name so + // bundle and release findings stay apart in the output; empty means no tag. + ObjectID string +} - errors *errors.LintRuleErrorsList +// Source supplies the modules a run lints. It is the only thing that differs between +// linting a source tree and linting the images a release published: everything after +// the modules exist — running the linters, printing, statistics, metrics — is the +// Manager's, and identical for both. +type Source interface { + // ConfigDir is the directory .dmtlint.yaml is looked up from and the metrics + // labels are derived from. It is read before Targets, to load the config Targets + // is then handed. + ConfigDir() string + // Scopes are the scopes this source produces targets in. Answered without loading + // anything, so a run that finds no modules still reports the sections it would + // have linted with. + Scopes() []scopes.Scope + // Targets loads the modules to lint. Findings made while loading go into + // errorList; a returned error is one the run could not fold into a finding, e.g. + // a registry failure, and is reported after the findings are printed rather than + // instead of them. + Targets(ctx context.Context, cfg *config.RootConfig, errorList *errors.LintRuleErrorsList) ([]Target, error) + // Close releases what the source allocated, e.g. image extraction directories. + // It runs after the findings are printed, so a finding must never name a path + // that only exists until Close. + Close() +} +type Manager struct { + cfg *config.RootConfig + source Source + targets []Target + errors *errors.LintRuleErrorsList // startedAt marks the beginning of the run; PrintStatistics reports the // wall-clock time elapsed since it, matching the mirror summary's Elapsed line. + // It is taken before the source loads anything, so for a remote run the pulls + // are inside the number the caller waited for. startedAt time.Time } -func NewManager(dir string, rootConfig *config.RootConfig, sc scopes.Scope) *Manager { +func New(cfg *config.RootConfig, src Source) *Manager { managerLevel := pkg.Error - m := &Manager{ - cfg: rootConfig, - scope: sc, + return &Manager{ + cfg: cfg, + source: src, errors: errors.NewLintRuleErrorsList().WithMaxLevel(&managerLevel), startedAt: time.Now(), } - - return m.initManager(dir) } -func (m *Manager) initManager(dir string) *Manager { - paths, err := moduleloader.GetModulePaths(dir) - if err != nil { - log.Error("Error getting module paths", log.Err(err)) - return m - } - - vals, err := decodeValuesFile(flags.ValuesFile) - if err != nil { - log.Error("Failed to decode values file", log.Err(err)) - } +// Run loads the source's targets and lints them. The returned error is the source's: +// the findings collected before it are still on the Manager, so the caller prints +// them first and reports the error afterwards. +func (m *Manager) Run(ctx context.Context) error { + targets, sourceErr := m.source.Targets(ctx, m.cfg, m.errors.WithLinterID("manager")) + m.targets = targets - globalValues, err := values.GetGlobalValues(getRootDirectory(dir)) - if err != nil { - log.Error("Failed to get global values", log.Err(err)) - return m - } + log.Info("Found modules", slog.Int("count", m.moduleCount())) - errorList := m.errors.WithLinterID("manager") + wg := new(sync.WaitGroup) + // The send below happens before the goroutine that drains it, so an unbuffered + // channel deadlocks. --parallel is a user-supplied number and every caller is not + // a cobra command, so the floor lives here rather than at each entry point. + processingCh := make(chan struct{}, max(flags.LintersLimit, 1)) - for i := range paths { - moduleName := filepath.Base(paths[i]) - log.Debug("Found module", slog.String("module", moduleName)) + for _, target := range targets { + processingCh <- struct{}{} - if err := m.validateModule(paths[i]); err != nil { - // linting errors are already logged - continue - } + wg.Add(1) - mdl, err := modules.NewModule(paths[i], &vals, globalValues, m.cfg, errorList) - if err != nil { - errorList. - WithFilePath(paths[i]).WithModule(moduleName). - WithValue(err.Error()). - Errorf("cannot create module `%s`", moduleName) + go func() { + defer func() { + <-processingCh + wg.Done() + }() - continue - } + log.Info("Run linters for module", + slog.String("module", target.Module.GetName()), + slog.String("scope", string(target.Scope)), + ) - m.Modules = append(m.Modules, mdl) + lintModule(ctx, target.Scope, target.Module, m.errorsFor(target)) + }() } - log.Info("Found modules", slog.Int("count", len(m.Modules))) + wg.Wait() - return m + return sourceErr } -func decodeValuesFile(path string) (chartutil.Values, error) { - if path == "" { - return nil, nil - } +// Close releases the source's resources. It must be called after the findings are +// printed: a source may be holding the directory the run linted. +func (m *Manager) Close() { + m.source.Close() +} - valuesFile, err := fsutils.ExpandDir(path) - if err != nil { - return nil, err +// errorsFor decorates the shared error list for one target. +func (m *Manager) errorsFor(t Target) *errors.LintRuleErrorsList { + if t.ObjectID == "" { + return m.errors } - return chartutil.ReadValuesFile(valuesFile) + return m.errors.WithObjectID(t.ObjectID) } -func (m *Manager) Run(ctx context.Context) { - wg := new(sync.WaitGroup) - processingCh := make(chan struct{}, flags.LintersLimit) +// moduleCount counts modules, not targets: a remote run reads one module from two +// images, and the summary must call that one module. +func (m *Manager) moduleCount() int { + ids := set.New() + for _, t := range m.targets { + ids.Add(t.ModuleID) + } - for _, module := range m.Modules { - processingCh <- struct{}{} + return ids.Size() +} - wg.Add(1) +// MetricsSections returns the config sections this run linted with, in the form +// metrics.Flush takes. They follow from the source's scopes — `linters-settings` for +// a source tree, `remote.bundle` and `remote.release` for the published images — so +// the caller does not have to know which run it started. +func (m *Manager) MetricsSections() []*global.Linters { + sc := m.source.Scopes() + sections := make([]*global.Linters, 0, len(sc)) - go func() { - defer func() { - <-processingCh - wg.Done() - }() + for _, s := range sc { + sections = append(sections, s.Settings(m.cfg)) + } - log.Info("Run linters for module", slog.String("module", module.GetName())) + return sections +} - for _, linter := range m.scope.Linters(module, m.errors) { - if flags.LinterName != "" && linter.GetName() != flags.LinterName { - continue - } +// lintModule runs the scope's linters over one module. +func lintModule(ctx context.Context, sc scopes.Scope, m *modules.Module, errorList *errors.LintRuleErrorsList) { + for _, linter := range sc.Linters(m, errorList) { + if flags.LinterName != "" && linter.GetName() != flags.LinterName { + continue + } - log.Debug("Running linter", slog.String("linter", linter.GetName()), slog.String("module", module.GetName())) + log.Debug("Running linter", + slog.String("linter", linter.GetName()), + slog.String("module", m.GetName()), + ) - linter.Lint(ctx) - } - }() + linter.Lint(ctx) } - - wg.Wait() } func (m *Manager) PrintResult() { - errs := m.errors.GetErrors() + printResult(m.errors) +} + +// printResult renders a finished error list. +func printResult(errorList *errors.LintRuleErrorsList) { + errs := errorList.GetErrors() if len(errs) == 0 { return @@ -324,23 +367,3 @@ func prepareString(input string) string { return w.String() } - -func getRootDirectory(dir string) string { - for { - if fsutils.IsDir(filepath.Join(dir, "global-hooks", "openapi")) && - fsutils.IsDir(filepath.Join(dir, "modules")) && - fsutils.IsFile(filepath.Join(dir, "global-hooks", "openapi", "config-values.yaml")) && - fsutils.IsFile(filepath.Join(dir, "global-hooks", "openapi", "values.yaml")) { - return dir - } - - parent := filepath.Dir(dir) - if dir == parent || parent == "" { - break - } - - dir = parent - } - - return "" -} diff --git a/internal/manager/manager_test.go b/internal/manager/manager_test.go new file mode 100644 index 000000000..c1d485140 --- /dev/null +++ b/internal/manager/manager_test.go @@ -0,0 +1,184 @@ +/* +Copyright 2025 Flant JSC + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package manager + +import ( + "context" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/deckhouse/dmt/internal/flags" + "github.com/deckhouse/dmt/internal/modules" + "github.com/deckhouse/dmt/internal/set" + "github.com/deckhouse/dmt/pkg/config" + "github.com/deckhouse/dmt/pkg/config/global" + "github.com/deckhouse/dmt/pkg/errors" + "github.com/deckhouse/dmt/pkg/scopes" +) + +// fakeSource hands the Manager a fixed target list, so a test can describe the shape +// of a run — how many modules, in how many scopes — without a registry or a tree. +type fakeSource struct { + scopes []scopes.Scope + targets []Target + err error + closed bool +} + +func (s *fakeSource) ConfigDir() string { return "." } +func (s *fakeSource) Scopes() []scopes.Scope { return s.scopes } +func (s *fakeSource) Close() { s.closed = true } + +func (s *fakeSource) Targets(_ context.Context, _ *config.RootConfig, _ *errors.LintRuleErrorsList) ([]Target, error) { + return s.targets, s.err +} + +// TestModuleCountCountsModulesNotTargets pins the number the summary reports. A +// remote run reads one module from two images and lints each in its own scope, so it +// has two targets and one module — the literal 1 the remote path used to pass in. The +// same-name case is the reason the count keys on the source's ModuleID and not on the +// module name: a tree can hold two directories that declare the same name. +func TestModuleCountCountsModulesNotTargets(t *testing.T) { + cfg, err := config.NewDefaultRootConfig(t.TempDir()) + require.NoError(t, err) + + mdl := func(name string) *modules.Module { + return modules.NewRemoteModule(t.TempDir(), name, scopes.Bundle.Settings(cfg)) + } + + for name, tc := range map[string]struct { + targets []Target + want int + }{ + "one module in two scopes": { + targets: []Target{ + {Module: mdl("mod"), Scope: scopes.Bundle, ModuleID: "mod"}, + {Module: mdl("mod"), Scope: scopes.Release, ModuleID: "mod"}, + }, + want: 1, + }, + "two modules in one scope": { + targets: []Target{ + {Module: mdl("first"), Scope: scopes.Bundle, ModuleID: "a/first"}, + {Module: mdl("second"), Scope: scopes.Bundle, ModuleID: "a/second"}, + }, + want: 2, + }, + "two directories declaring the same name": { + targets: []Target{ + {Module: mdl("same"), Scope: scopes.Bundle, ModuleID: "a/same"}, + {Module: mdl("same"), Scope: scopes.Bundle, ModuleID: "b/same"}, + }, + want: 2, + }, + "nothing found": {want: 0}, + } { + t.Run(name, func(t *testing.T) { + m := New(cfg, &fakeSource{targets: tc.targets}) + require.NoError(t, m.Run(t.Context())) + + assert.Equal(t, tc.want, m.moduleCount()) + }) + } +} + +// TestMetricsSectionsFollowTheSourceScopes pins which config sections a run reports +// against: the source tree is configured by `linters-settings` and the two published +// images by `remote.bundle` and `remote.release`, and neither may be flushed under +// the other's severities. +func TestMetricsSectionsFollowTheSourceScopes(t *testing.T) { + cfg, err := config.NewDefaultRootConfig(t.TempDir()) + require.NoError(t, err) + + static := New(cfg, &fakeSource{scopes: []scopes.Scope{scopes.Static}}) + assert.Equal(t, []*global.Linters{&cfg.GlobalSettings.Linters}, static.MetricsSections()) + + remote := New(cfg, &fakeSource{scopes: []scopes.Scope{scopes.Bundle, scopes.Release}}) + assert.Equal(t, []*global.Linters{&cfg.Remote.Bundle, &cfg.Remote.Release}, remote.MetricsSections()) +} + +// TestRunReportsTheSourceError covers the ordering the remote path depends on: a +// registry failure comes back from Run, but the findings collected before it are +// still on the Manager for the caller to print. +func TestRunReportsTheSourceError(t *testing.T) { + cfg, err := config.NewDefaultRootConfig(t.TempDir()) + require.NoError(t, err) + + src := &fakeSource{ + err: assert.AnError, + targets: []Target{ + { + Module: modules.NewRemoteModule(t.TempDir(), "mod", scopes.Bundle.Settings(cfg)), + Scope: scopes.Bundle, + ModuleID: "mod", + }, + }, + } + + m := New(cfg, src) + require.ErrorIs(t, m.Run(t.Context()), assert.AnError) + assert.NotEmpty(t, m.GetErrors(), "findings collected before the failure must survive it") + + m.Close() + assert.True(t, src.closed) +} + +// TestLintModuleHonoursTheLinterFilter is the guard for the reason lintModule exists: the +// remote-lint path used to run this loop itself and silently ignored --linter, so +// `dmt lint remote --linter=` reported a module clean without having checked it. +// Both paths go through this function now, so one test covers both. +// +// The bundle scope over an empty directory is the fixture: none of the files that scope +// looks for are there, so both of its linters report — module via bundle-layout, +// documentation via readme. The unfiltered case is what makes the filtered one mean +// something; without it the filter would look correct even if nothing reported at all. +func TestLintModuleHonoursTheLinterFilter(t *testing.T) { + cfg, err := config.NewDefaultRootConfig(t.TempDir()) + require.NoError(t, err) + + // lint returns the linters that reported, which is the only thing the filter changes. + lint := func(t *testing.T, linterName string) []string { + t.Helper() + + // flags.LinterName is process-global, so this test must not run in parallel. + flags.LinterName = linterName + + t.Cleanup(func() { flags.LinterName = "" }) + + m := modules.NewRemoteModule(t.TempDir(), "test-module", scopes.Bundle.Settings(cfg)) + + errorList := errors.NewLintRuleErrorsList() + lintModule(t.Context(), scopes.Bundle, m, errorList) + + reported := set.New() + for _, e := range errorList.GetErrors() { + reported.Add(e.LinterID) + } + + return reported.Slice() + } + + t.Run("unfiltered", func(t *testing.T) { + assert.ElementsMatch(t, []string{"module", "documentation"}, lint(t, "")) + }) + + t.Run("filtered", func(t *testing.T) { + assert.Equal(t, []string{"module"}, lint(t, "module")) + }) +} diff --git a/internal/manager/statistics.go b/internal/manager/statistics.go index c510bdd7d..5fe05a693 100644 --- a/internal/manager/statistics.go +++ b/internal/manager/statistics.go @@ -27,6 +27,7 @@ import ( "github.com/fatih/color" "github.com/deckhouse/dmt/pkg" + "github.com/deckhouse/dmt/pkg/errors" ) // The statistics summary is rendered as a single framed block that intentionally @@ -115,15 +116,15 @@ type statistics struct { // collectStatistics tallies every collected finding by severity and by linter. // Counts are taken over all findings regardless of the --hide-warnings / // --show-ignored display flags: the summary is meant to give the full picture. -func (m *Manager) collectStatistics() statistics { +func collectStatistics(errorList *errors.LintRuleErrorsList, modules int, elapsed time.Duration) statistics { s := statistics{ - modules: len(m.Modules), - elapsed: time.Since(m.startedAt), + modules: modules, + elapsed: elapsed, } perLinter := make(map[string]int) - errs := m.errors.GetErrors() + errs := errorList.GetErrors() for idx := range errs { s.total++ @@ -159,7 +160,7 @@ func (m *Manager) collectStatistics() statistics { // styled identically to the deckhouse-cli mirror summaries. It is meant to be // called after PrintResult, once all findings have been listed. func (m *Manager) PrintStatistics() { - fmt.Println(renderStatistics(m.collectStatistics())) + fmt.Println(renderStatistics(collectStatistics(m.errors, m.moduleCount(), time.Since(m.startedAt)))) } // renderStatistics formats the statistics as a single multi-line, framed block. diff --git a/internal/metrics/metrics.go b/internal/metrics/metrics.go index a4c4e43c0..8875833b6 100644 --- a/internal/metrics/metrics.go +++ b/internal/metrics/metrics.go @@ -18,7 +18,7 @@ package metrics import ( "cmp" - "fmt" + "context" "os" "reflect" "strings" @@ -77,8 +77,8 @@ func SetDmtInfo() { } // TODO: refactor this ASAP -func SetLinterWarningsMetrics(cfg *global.Global) { - processLinterConfig("", reflect.ValueOf(&cfg.Linters).Elem()) +func SetLinterWarningsMetrics(linters *global.Linters) { + processLinterConfig("", reflect.ValueOf(linters).Elem()) } func processLinterConfig(parent string, v reflect.Value) { @@ -97,8 +97,6 @@ func processLinterConfig(parent string, v reflect.Value) { name = fType.Name } - fmt.Println(strings.ToLower(name)) - metrics.CounterAdd("dmt_linter_info", 1, prometheus.Labels{ "id": metrics.id, "linter": strings.ToLower(name), @@ -141,3 +139,30 @@ func SetDmtRuntimeDurationSeconds() { "repository": metrics.repository, }) } + +// Flush records the run-level metrics and ships everything collected during the run. +// Both lint paths end with it: the remote path used to collect its findings metrics +// through IncDmtLinterErrorsCount and then never send them, which looked like a working +// run right up until nobody could find its data. +// +// sections are the config sections the run linted with — `linters-settings` for the +// source tree, `remote.bundle` and `remote.release` for the published images. +func Flush(ctx context.Context, sections ...*global.Linters) { + // Nothing was collected without a client, and every setter below writes through it. + if metrics == nil { + return + } + + SetDmtInfo() + + for _, s := range sections { + SetLinterWarningsMetrics(s) + } + + SetDmtRuntimeDuration() + SetDmtRuntimeDurationSeconds() + + // The send must outlive a cancelled run: these metrics describe the run that just + // ended, and losing them to its own cancellation is the one case they are for. + metrics.Send(context.WithoutCancel(ctx)) +} diff --git a/internal/metrics/metrics_test.go b/internal/metrics/metrics_test.go index 6ce940319..803c1d714 100644 --- a/internal/metrics/metrics_test.go +++ b/internal/metrics/metrics_test.go @@ -34,7 +34,7 @@ func Test_SetLinterWarningsMetrics_AddsWarningsForAllLinters(t *testing.T) { cfg.Linters.Documentation.Impact = pkg.Warn.String() cfg.Linters.Templates.Impact = pkg.Warn.String() - SetLinterWarningsMetrics(cfg) + SetLinterWarningsMetrics(&cfg.Linters) num, err := testutil.GatherAndCount(metrics.Gatherer, "dmt_linter_info") require.NoError(t, err) @@ -47,7 +47,7 @@ func Test_SetLinterWarningsMetrics_NoWarningsWhenNoLinters(t *testing.T) { cfg := &global.Global{ Linters: global.Linters{}, } - SetLinterWarningsMetrics(cfg) + SetLinterWarningsMetrics(&cfg.Linters) num, err := testutil.GatherAndCount(metrics.Gatherer, "dmt_linter_info") require.NoError(t, err) @@ -66,7 +66,7 @@ func Test_SetLinterWarningsMetrics_AddsWarningsForSpecificLinters(t *testing.T) cfg.Linters.Container.Impact = pkg.Warn.String() - SetLinterWarningsMetrics(cfg) + SetLinterWarningsMetrics(&cfg.Linters) num, err := testutil.GatherAndCount(metrics.Gatherer, "dmt_linter_info") require.NoError(t, err) diff --git a/internal/moduleloader/loader.go b/internal/moduleloader/loader.go index a1d203d75..5b746b173 100644 --- a/internal/moduleloader/loader.go +++ b/internal/moduleloader/loader.go @@ -23,14 +23,10 @@ import ( ) const ( - ChartConfigFilename = "Chart.yaml" - ModuleYamlFilename = "module.yaml" - HooksDir = "hooks" - ImagesDir = "images" - OpenAPIDir = "openapi" + moduleYamlFilename = "module.yaml" ) -// GetModulePaths returns all paths that contain a module (Chart.yaml or module.yaml). +// GetModulePaths returns all paths that contain a module (module.yaml). // modulesDir can be a module directory or a directory that contains helm charts in subdirectories. func GetModulePaths(modulesDir string) ([]string, error) { var chartDirs = make([]string, 0) @@ -44,13 +40,8 @@ func GetModulePaths(modulesDir string) ([]string, error) { return nil } - // A module is identified by having Chart.yaml or module.yaml - // OR having Chart.yaml + (hooks|images|openapi) subdirs - if isExistsOnFilesystem(path, ModuleYamlFilename) || - (isExistsOnFilesystem(path, ChartConfigFilename) && - (isExistsOnFilesystem(path, HooksDir) || - isExistsOnFilesystem(path, ImagesDir) || - isExistsOnFilesystem(path, OpenAPIDir))) { + // A module is identified by having module.yaml + if isExistsOnFilesystem(path, moduleYamlFilename) { chartDirs = append(chartDirs, path) } diff --git a/internal/modules/module.go b/internal/modules/module.go index 4d7ba6c7f..86c1a25ef 100644 --- a/internal/modules/module.go +++ b/internal/modules/module.go @@ -342,6 +342,7 @@ func mapDocumentationRules(linterSettings *pkg.LintersSettings, configSettings * // render), so it defaults to error via fallbackImpact — unlike the style/soft // markdownlint and size rules above. A per-rule impact in config still overrides. rules.FrontMatterRule.SetLevel(globalRules.FrontMatterRule.Impact, fallbackImpact) + rules.ChangelogRule.SetLevel(globalRules.ChangelogRule.Impact, fallbackImpact) } func mapModuleRules(linterSettings *pkg.LintersSettings, configSettings *config.LintersSettings, globalConfig *global.Linters) { @@ -359,6 +360,8 @@ func mapModuleRules(linterSettings *pkg.LintersSettings, configSettings *config. rules.ModulePackageConsistencyRule.SetLevel(globalRules.ModulePackageConsistencyRule.Impact, fallbackImpact) rules.LegacyReleaseFileRule.SetLevel(globalRules.LegacyReleaseFileRule.Impact, fallbackImpact) rules.EnabledScriptRule.SetLevel(globalRules.EnabledScriptRule.Impact, fallbackImpact) + rules.ReleaseLayoutRule.SetLevel(globalRules.ReleaseLayoutRule.Impact, fallbackImpact) + rules.BundleLayoutRule.SetLevel(globalRules.BundleLayoutRule.Impact, fallbackImpact) } // mapTemplatesRules configures Templates linter rules @@ -604,6 +607,30 @@ func NewModule(path string, vals *chartutil.Values, globalSchema *spec.Schema, r return module, nil } +// NewRemoteModule builds a module from a directory extracted out of a registry +// image. An image carries neither a loadable chart nor rendered objects, so this +// skips the render pipeline NewModule runs: the release and bundle scopes only +// ask for rules that need the path. +// +// The invariant that buys is one those scope tables have to keep — a rule reaching +// for GetChart, GetObjectStore or GetValues here finds nil. name comes from the +// image reference rather than module.yaml, because a module.yaml missing from the +// image is one of the things the layout rules are there to report. +func NewRemoteModule(path, name string, linters *global.Linters) *Module { + // The image ships no .dmtlint.yaml of its own, and a remote scope is configured + // independently of the source tree: severities come from its own `remote.` + // section of the caller's config, so nothing from `linters-settings` leaks in. + // An absent section leaves every impact empty, which remaps to the defaults. + cfg := &config.LintersSettings{} + cfg.MergeGlobal(linters) + + return &Module{ + name: name, + path: path, + linterConfig: remapLinterSettings(cfg, linters), + } +} + func newModuleFromPath(path string) (*Module, error) { moduleYamlConfig, err := ParseModuleConfigFile(path) if err != nil { diff --git a/internal/sources/remote/extract.go b/internal/sources/remote/extract.go new file mode 100644 index 000000000..bc6a28dcd --- /dev/null +++ b/internal/sources/remote/extract.go @@ -0,0 +1,178 @@ +/* +Copyright 2025 Flant JSC + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package remote + +import ( + "archive/tar" + "context" + stderrors "errors" + "fmt" + "io" + "os" + "path/filepath" + "strings" + + "github.com/deckhouse/deckhouse/pkg/registry" +) + +// extractImage flattens the image into a temporary directory and returns its path. +// The caller owns the directory and must remove it. +func extractImage(ctx context.Context, image registry.Image) (string, error) { + tempDir, err := os.MkdirTemp("", "dmt-*") + if err != nil { + return "", fmt.Errorf("failed to create temp directory: %w", err) + } + + rc := image.Extract() + defer rc.Close() + + if err = extract(ctx, rc, tempDir); err != nil { + os.RemoveAll(tempDir) + + return "", fmt.Errorf("failed to extract image: %w", err) + } + + return tempDir, nil +} + +// extract unpacks a tar stream under root, rejecting any entry that would write or +// point outside it. +func extract(ctx context.Context, rc io.ReadCloser, root string) error { + dir, err := os.OpenRoot(root) + if err != nil { + return fmt.Errorf("open output directory: %w", err) + } + + defer dir.Close() + + tr := tar.NewReader(rc) + + for { + if err := ctx.Err(); err != nil { + return err + } + + hdr, err := tr.Next() + if stderrors.Is(err, io.EOF) { + break + } + + if err != nil { + return fmt.Errorf("read tar: %w", err) + } + + name, err := safeName(hdr.Name) + if err != nil { + return err + } + + switch hdr.Typeflag { + case tar.TypeDir: + if err = dir.MkdirAll(name, os.FileMode(hdr.Mode)); err != nil { + return fmt.Errorf("mkdir %q: %w", hdr.Name, err) + } + case tar.TypeReg: + if err = writeRegularFile(dir, name, tr, os.FileMode(hdr.Mode)); err != nil { + return fmt.Errorf("write file %q: %w", hdr.Name, err) + } + case tar.TypeSymlink: + // dir stops a link from being followed out of the tree, but the linters + // that read the extracted files afterwards use plain os calls, so a link + // pointing out of it has to be rejected here as well. A symlink target is + // resolved by whoever follows it, relative to the directory the link sits + // in. So the base is that directory while the boundary stays the + // extraction root — passing the entry itself as the boundary would reject + // a link to its own sibling. + if filepath.IsAbs(hdr.Linkname) || !staysWithin(filepath.Dir(name), hdr.Linkname) { + return fmt.Errorf("symlink %q escapes output directory", hdr.Name) + } + + if err = dir.Symlink(hdr.Linkname, name); err != nil { + return fmt.Errorf("create symlink %q: %w", hdr.Name, err) + } + case tar.TypeLink: + // A hardlink names an already-extracted entry by its path within the + // archive, so it resolves against the root and not against this entry. + linkName, err := safeName(hdr.Linkname) + if err != nil { + return err + } + + if err = dir.Link(linkName, name); err != nil { + return fmt.Errorf("create hardlink %q: %w", hdr.Name, err) + } + } + } + + // tr.Next reports io.EOF as soon as it reads the archive terminator, and the + // producer writes that terminator from a deferred Close before it records its + // own failure — a registry error mid-pull therefore surfaces only on the next + // read of the stream. Draining it is what tells a truncated image apart from a + // module that is genuinely missing half its files. + if _, err := io.Copy(io.Discard, rc); err != nil { + return fmt.Errorf("read tar: %w", err) + } + + return nil +} + +// writeRegularFile writes one regular tar entry and limits restored permissions to owner bits. +func writeRegularFile(dir *os.Root, target string, src io.Reader, mode os.FileMode) error { + if err := dir.MkdirAll(filepath.Dir(target), 0o700); err != nil { + return fmt.Errorf("create parent directory: %w", err) + } + + out, err := dir.OpenFile(target, os.O_CREATE|os.O_WRONLY|os.O_TRUNC, mode&0o700) + if err != nil { + return fmt.Errorf("create file: %w", err) + } + + if _, err = io.Copy(out, src); err != nil { + closeErr := out.Close() + if closeErr != nil { + return fmt.Errorf("copy file: %w; close file: %v", err, closeErr) + } + + return fmt.Errorf("copy file: %w", err) + } + + if err = out.Close(); err != nil { + return fmt.Errorf("close file: %w", err) + } + + return nil +} + +// safeName cleans a tar entry name into a path relative to the extraction root and +// rejects absolute paths or parent-directory escapes. +func safeName(name string) (string, error) { + clean := filepath.Clean(filepath.FromSlash(name)) + + if filepath.IsAbs(clean) || !staysWithin(".", clean) { + return "", fmt.Errorf("path %q escapes output directory", name) + } + + return clean, nil +} + +// staysWithin reports whether name resolves under the extraction root when +// interpreted relative to base. Both are paths relative to that root. +func staysWithin(base, name string) bool { + target := filepath.Clean(filepath.Join(base, name)) + + return target != ".." && !strings.HasPrefix(target, ".."+string(filepath.Separator)) +} diff --git a/internal/sources/remote/lint.go b/internal/sources/remote/lint.go new file mode 100644 index 000000000..70cd349bf --- /dev/null +++ b/internal/sources/remote/lint.go @@ -0,0 +1,180 @@ +/* +Copyright 2025 Flant JSC + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +// Package remote lints a module as it was published, rather than as it sits in a +// working tree: it pulls the two images a release produces and runs the scopes that +// belong to them. +package remote + +import ( + "context" + stderrors "errors" + "fmt" + "os" + "path" + "strings" + + "github.com/google/go-containerregistry/pkg/name" + + "github.com/deckhouse/deckhouse/pkg/registry" + + "github.com/deckhouse/dmt/internal/manager" + "github.com/deckhouse/dmt/internal/modules" + "github.com/deckhouse/dmt/pkg/config" + "github.com/deckhouse/dmt/pkg/errors" + "github.com/deckhouse/dmt/pkg/scopes" +) + +// releaseSegment is the repository segment a module's release image sits under: +// the bundle is : and the release is /release:. +const releaseSegment = "release" + +type Options struct { + // Login is the username to use for the registry, e.g. license-token. + Login string + // Password is the password to use for the registry. + Password string +} + +// Source reads a module from the two images published under an image path, e.g. +// registry.example.com/my-module:v0.0.1. +type Source struct { + client registry.Client + tag string + moduleName string + + // dirs are the extraction directories, removed by Close. + dirs []string +} + +var _ manager.Source = (*Source)(nil) + +// NewSource resolves the image path up front, so an unusable reference fails before +// the run prints anything. +func NewSource(imagePath string, opts *Options) (*Source, error) { + repository, tag, err := cutTagFromImagePath(imagePath) + if err != nil { + return nil, fmt.Errorf("failed to cut tag from image path: %w", err) + } + + return &Source{ + client: newRegistryClient(repository, opts.Login, opts.Password), + tag: tag, + moduleName: path.Base(repository), + }, nil +} + +// ConfigDir is the caller's working directory: the image ships no .dmtlint.yaml, so +// severities come from the config next to whoever started the run — read from its +// `remote.bundle` and `remote.release` sections rather than the ones the source tree +// uses. +func (s *Source) ConfigDir() string { + return "." +} + +func (s *Source) Scopes() []scopes.Scope { + return []scopes.Scope{scopes.Bundle, scopes.Release} +} + +// Close removes the extraction directories. It runs after the findings are printed, +// which is why every remote rule reports a module-relative path. +func (s *Source) Close() { + for _, dir := range s.dirs { + os.RemoveAll(dir) + } +} + +// Targets pulls and unpacks both images. Both are attempted before either error is +// returned: a registry failure on one must not cost the caller what the other holds. +func (s *Source) Targets( + ctx context.Context, + cfg *config.RootConfig, + _ *errors.LintRuleErrorsList, +) ([]manager.Target, error) { + bundle, bundleErr := s.target(ctx, s.client, scopes.Bundle, cfg) + release, releaseErr := s.target(ctx, s.client.WithSegment(releaseSegment), scopes.Release, cfg) + + targets := make([]manager.Target, 0, 2) + + for _, t := range []*manager.Target{bundle, release} { + if t != nil { + targets = append(targets, *t) + } + } + + return targets, stderrors.Join(bundleErr, releaseErr) +} + +// target pulls one image and unpacks it into a module. Which linters the scope runs +// over it, and which of their rules, is the scope's business. +func (s *Source) target( + ctx context.Context, + client registry.Client, + scope scopes.Scope, + cfg *config.RootConfig, +) (*manager.Target, error) { + image, err := client.GetImage(ctx, s.tag) + if err != nil { + return nil, fmt.Errorf("failed to get %s image: %w", scope, err) + } + + dir, err := extractImage(ctx, image) + if err != nil { + return nil, fmt.Errorf("failed to extract %s image: %w", scope, err) + } + + s.dirs = append(s.dirs, dir) + + return &manager.Target{ + Module: modules.NewRemoteModule(dir, s.moduleName, scope.Settings(cfg)), + Scope: scope, + // Both images are the same module, so the summary counts one. + ModuleID: s.moduleName, + ObjectID: string(scope), + }, nil +} + +// cutTagFromImagePath splits an image path into repository and tag, turning +// "registry.example.com/my-module:v0.0.1" into "registry.example.com/my-module" and +// "v0.0.1". +func cutTagFromImagePath(imagePath string) (string, string, error) { + // The release image is addressed by the same tag under a sibling repository, and a + // digest names one manifest only — there is no tag to carry over to it. + if strings.Contains(imagePath, "@") { + return "", "", fmt.Errorf("digest not supported") + } + + // Without an empty default registry, name invents one: "deckhouse/my-module:v0.0.1" + // becomes index.docker.io/deckhouse/my-module, and a bare "registry.example.com:5000" + // becomes index.docker.io/library/registry.example.com with "5000" as its tag. The + // pull would then send --login/--password to Docker Hub, so the host is required + // here rather than guessed. + ref, err := name.ParseReference(imagePath, name.WithDefaultTag(""), name.WithDefaultRegistry("")) + if err != nil { + return "", "", fmt.Errorf("failed to parse image path: %w", err) + } + + if ref.Context().RegistryStr() == "" { + return "", "", fmt.Errorf("registry not found in image path, expected /:") + } + + tag := ref.Identifier() + if tag == "" { + return "", "", fmt.Errorf("tag not found in image path") + } + + return ref.Context().Name(), tag, nil +} diff --git a/internal/sources/remote/lint_test.go b/internal/sources/remote/lint_test.go new file mode 100644 index 000000000..5fc037bbf --- /dev/null +++ b/internal/sources/remote/lint_test.go @@ -0,0 +1,177 @@ +/* +Copyright 2025 Flant JSC + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package remote + +import ( + "archive/tar" + "bytes" + "context" + "errors" + "io" + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/require" +) + +func TestCutTagFromImagePath(t *testing.T) { + repository, tag, err := cutTagFromImagePath("registry.example.com/deckhouse/my-module:v0.0.1") + require.NoError(t, err) + require.Equal(t, "registry.example.com/deckhouse/my-module", repository) + require.Equal(t, "v0.0.1", tag) + + // A digest names one manifest, so the release image's tag cannot be derived. + _, _, err = cutTagFromImagePath("registry.example.com/deckhouse/my-module@sha256:1234567890") + require.ErrorContains(t, err, "digest not supported") + + _, _, err = cutTagFromImagePath("registry.example.com/deckhouse/my-module") + require.ErrorContains(t, err, "tag not found in image path") + + repository, tag, err = cutTagFromImagePath("registry.example.com:8080/deckhouse/my-module:v0.0.1") + require.NoError(t, err) + require.Equal(t, "registry.example.com:8080/deckhouse/my-module", repository) + require.Equal(t, "v0.0.1", tag) + + // A reference with no registry of its own would be normalized to Docker Hub, and + // the credentials would go to a registry the caller never named. The last one is + // the worst of the three: its port is read as a tag and its host as a repository. + for _, imagePath := range []string{ + "deckhouse/my-module:v0.0.1", + "my-module:v1", + "registry.example.com:5000", + } { + _, _, err = cutTagFromImagePath(imagePath) + require.ErrorContains(t, err, "registry not found in image path", imagePath) + } +} + +// TestExtractLinks covers the two entry kinds whose paths are resolved against +// something other than the entry itself: a symlink resolves against the directory it +// sits in, a hardlink against the archive root. Getting either boundary wrong rejects +// or misplaces links that are perfectly legal. +func TestExtractLinks(t *testing.T) { + dir := t.TempDir() + + require.NoError(t, extract(context.Background(), tarball(t, + dirEntry("docs"), + fileEntry("docs/README.md", "hello"), + symlinkEntry("docs/README.ru.md", "README.md"), + hardlinkEntry("docs/COPY.md", "docs/README.md"), + ), dir)) + + target, err := os.Readlink(filepath.Join(dir, "docs", "README.ru.md")) + require.NoError(t, err) + require.Equal(t, "README.md", target) + + content, err := os.ReadFile(filepath.Join(dir, "docs", "COPY.md")) + require.NoError(t, err) + require.Equal(t, "hello", string(content)) +} + +func TestExtractRejectsEscapes(t *testing.T) { + for name, entry := range map[string]*tar.Header{ + "parent path": fileEntry("../evil", "x"), + "absolute path": fileEntry("/evil", "x"), + "symlink escape": symlinkEntry("link", "../../evil"), + "hardlink escape": hardlinkEntry("hard", "../evil"), + } { + t.Run(name, func(t *testing.T) { + err := extract(context.Background(), tarball(t, entry), t.TempDir()) + require.ErrorContains(t, err, "escapes output directory") + }) + } +} + +// TestExtractRejectsASymlinkChain is the escape a name check cannot see: both links +// below resolve inside the archive when read as text, but once they are on disk the +// second one is followed through the first, so the file written through them lands +// beside the extraction root instead of inside it. Only resolving each path component +// against what is already on disk — what os.Root does — catches that. +func TestExtractRejectsASymlinkChain(t *testing.T) { + outside := t.TempDir() + root := filepath.Join(outside, "root") + require.NoError(t, os.Mkdir(root, 0o700)) + + err := extract(t.Context(), tarball(t, + symlinkEntry("d1", "."), + symlinkEntry("d1/d2", ".."), + fileEntry("d1/d2/pwned", "x"), + ), root) + + require.Error(t, err) + require.NoFileExists(t, filepath.Join(outside, "pwned")) +} + +// TestExtractReportsATruncatedStream covers a producer that fails part-way through: +// the tar writer's deferred Close puts a valid archive terminator into the stream +// before the failure is recorded, so tr.Next sees a clean io.EOF and the extraction +// looks complete. Without the drain at the end of extract, a registry outage is +// reported as a module missing its files. +func TestExtractReportsATruncatedStream(t *testing.T) { + pr, pw := io.Pipe() + + go func() { + tw := tar.NewWriter(pw) + _ = tw.WriteHeader(dirEntry("docs")) + _ = tw.Close() + _ = pw.CloseWithError(errors.New("registry went away")) + }() + + require.ErrorContains(t, extract(t.Context(), pr, t.TempDir()), "registry went away") +} + +func dirEntry(name string) *tar.Header { + return &tar.Header{Typeflag: tar.TypeDir, Name: name, Mode: 0o755} +} + +func fileEntry(name, content string) *tar.Header { + return &tar.Header{Typeflag: tar.TypeReg, Name: name, Mode: 0o644, Size: int64(len(content)), Linkname: content} +} + +func symlinkEntry(name, target string) *tar.Header { + return &tar.Header{Typeflag: tar.TypeSymlink, Name: name, Mode: 0o777, Linkname: target} +} + +func hardlinkEntry(name, target string) *tar.Header { + return &tar.Header{Typeflag: tar.TypeLink, Name: name, Mode: 0o644, Linkname: target} +} + +// tarball builds an archive from headers. A regular entry carries its content in +// Linkname, which is unused for that type and saves a second parameter everywhere. +func tarball(t *testing.T, headers ...*tar.Header) io.ReadCloser { + t.Helper() + + buf := new(bytes.Buffer) + tw := tar.NewWriter(buf) + + for _, hdr := range headers { + content := "" + if hdr.Typeflag == tar.TypeReg { + content, hdr.Linkname = hdr.Linkname, "" + } + + require.NoError(t, tw.WriteHeader(hdr)) + + _, err := tw.Write([]byte(content)) + require.NoError(t, err) + } + + require.NoError(t, tw.Close()) + + return io.NopCloser(buf) +} diff --git a/internal/sources/remote/registry.go b/internal/sources/remote/registry.go new file mode 100644 index 000000000..4a47d9837 --- /dev/null +++ b/internal/sources/remote/registry.go @@ -0,0 +1,98 @@ +/* +Copyright 2025 Flant JSC + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package remote + +import ( + "cmp" + "log/slog" + "os" + + "github.com/google/go-containerregistry/pkg/authn" + "github.com/google/go-containerregistry/pkg/name" + + "github.com/deckhouse/deckhouse/pkg/log" + regclient "github.com/deckhouse/deckhouse/pkg/registry/client" +) + +// Environment variables the registry credentials can come from, so a CI job need not +// put a secret on the command line, where it lands in the process list and in the +// job's own command echo. +const ( + loginEnv = "DMT_REGISTRY_LOGIN" + passwordEnv = "DMT_REGISTRY_PASSWORD" +) + +func newRegistryClient(registryHost, login, password string) *regclient.Client { + return regclient.New(registryHost, regclient.WithAuth(registryAuth(registryHost, login, password))) +} + +// registryAuth resolves credentials for the source registry: the explicit +// login/password first, then DMT_REGISTRY_LOGIN/DMT_REGISTRY_PASSWORD, then the +// Docker config, then anonymous. Each field falls back on its own, so a CI job can +// keep the login in the pipeline definition and the password in a secret. +func registryAuth(registryHost, login, password string) authn.Authenticator { + login = cmp.Or(login, os.Getenv(loginEnv)) + password = cmp.Or(password, os.Getenv(passwordEnv)) + + if login != "" { + return authn.FromConfig(authn.AuthConfig{ + Username: login, + Password: password, + }) + } + + if auth, ok := dockerConfigAuth(registryHost); ok { + return auth + } + + log.Debug("using anonymous access for the source registry", slog.String("registry", registryHost)) + + return authn.Anonymous +} + +// dockerConfigAuth resolves credentials for registryHost from the Docker config +// (~/.docker/config.json, written by `d8 dk cr login`). ok is false when the config +// holds no usable entry for the host. +func dockerConfigAuth(registryHost string) (authn.Authenticator, bool) { + ref, err := name.ParseReference(registryHost) + if err != nil { + return nil, false + } + + reg, err := name.NewRegistry(ref.Context().RegistryStr()) + if err != nil { + return nil, false + } + + auth, err := authn.DefaultKeychain.Resolve(reg) + if err != nil || auth == authn.Anonymous { + return nil, false + } + + cfg, err := auth.Authorization() + if err != nil { + return nil, false + } + + if cfg.Username == "" && cfg.Password == "" && cfg.Auth == "" && cfg.IdentityToken == "" { + return nil, false + } + + log.Debug("using Docker config credentials", slog.String("registry", reg.String())) + + return auth, true +} diff --git a/internal/sources/remote/registry_test.go b/internal/sources/remote/registry_test.go new file mode 100644 index 000000000..21e0f102b --- /dev/null +++ b/internal/sources/remote/registry_test.go @@ -0,0 +1,79 @@ +/* +Copyright 2025 Flant JSC + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package remote + +import ( + "testing" + + "github.com/google/go-containerregistry/pkg/authn" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// TestRegistryAuthPrecedence pins the order credentials are resolved in: the flags +// win over the environment, and each field falls back on its own so a CI job can keep +// the login in the pipeline definition and the password in a secret. +// +// DOCKER_CONFIG points at an empty directory throughout, so the Docker config holds +// nothing and the "neither" case is anonymous on any machine — otherwise a developer +// logged into the registry would see their own credentials answer instead. +func TestRegistryAuthPrecedence(t *testing.T) { + const registryHost = "registry.example.com" + + for name, tc := range map[string]struct { + login, password string + envLogin, envPassword string + wantUser, wantPass string + }{ + "flags win over the environment": { + login: "from-flag", password: "flag-secret", + envLogin: "from-env", envPassword: "env-secret", + wantUser: "from-flag", wantPass: "flag-secret", + }, + "the environment answers when no flag was given": { + envLogin: "from-env", envPassword: "env-secret", + wantUser: "from-env", wantPass: "env-secret", + }, + "the login comes from a flag and the password from a secret": { + login: "license-token", + envLogin: "ignored", envPassword: "env-secret", + wantUser: "license-token", wantPass: "env-secret", + }, + "neither is anonymous": {}, + } { + t.Run(name, func(t *testing.T) { + // t.Setenv forbids t.Parallel here. + t.Setenv("DOCKER_CONFIG", t.TempDir()) + t.Setenv(loginEnv, tc.envLogin) + t.Setenv(passwordEnv, tc.envPassword) + + auth := registryAuth(registryHost, tc.login, tc.password) + + if tc.wantUser == "" { + assert.Equal(t, authn.Anonymous, auth) + + return + } + + cfg, err := auth.Authorization() + require.NoError(t, err) + + assert.Equal(t, tc.wantUser, cfg.Username) + assert.Equal(t, tc.wantPass, cfg.Password) + }) + } +} diff --git a/internal/sources/static/lint.go b/internal/sources/static/lint.go new file mode 100644 index 000000000..45865e400 --- /dev/null +++ b/internal/sources/static/lint.go @@ -0,0 +1,157 @@ +/* +Copyright 2025 Flant JSC + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +// Package static reads the modules to lint from a working tree: the full source +// as committed, rendered the way Deckhouse would render it. +package static + +import ( + "context" + "log/slog" + "path/filepath" + + "helm.sh/helm/v3/pkg/chartutil" + + "github.com/deckhouse/deckhouse/pkg/log" + + "github.com/deckhouse/dmt/internal/flags" + "github.com/deckhouse/dmt/internal/fsutils" + "github.com/deckhouse/dmt/internal/manager" + "github.com/deckhouse/dmt/internal/moduleloader" + "github.com/deckhouse/dmt/internal/modules" + "github.com/deckhouse/dmt/internal/modules/values" + "github.com/deckhouse/dmt/pkg/config" + "github.com/deckhouse/dmt/pkg/errors" + "github.com/deckhouse/dmt/pkg/scopes" +) + +// Source reads modules from a directory on disk. +type Source struct { + dir string +} + +var _ manager.Source = (*Source)(nil) + +// NewSource lints every module found under dir. +func NewSource(dir string) *Source { + return &Source{dir: dir} +} + +// ConfigDir is the linted directory itself: .dmtlint.yaml is looked up from the tree +// it configures. +func (s *Source) ConfigDir() string { + return s.dir +} + +func (s *Source) Scopes() []scopes.Scope { + return []scopes.Scope{scopes.Static} +} + +// Close has nothing to release: the modules are the caller's own files. +func (s *Source) Close() {} + +// Targets walks the tree for modules and builds each one. A module that cannot be +// read is reported as a finding and skipped, not returned as an error: one broken +// module must not cost the caller the findings of every other. +func (s *Source) Targets( + _ context.Context, + cfg *config.RootConfig, + errorList *errors.LintRuleErrorsList, +) ([]manager.Target, error) { + paths, err := moduleloader.GetModulePaths(s.dir) + if err != nil { + log.Error("Error getting module paths", log.Err(err)) + + return nil, nil + } + + vals, err := decodeValuesFile(flags.ValuesFile) + if err != nil { + log.Error("Failed to decode values file", log.Err(err)) + } + + globalValues, err := values.GetGlobalValues(getRootDirectory(s.dir)) + if err != nil { + log.Error("Failed to get global values", log.Err(err)) + + return nil, nil + } + + targets := make([]manager.Target, 0, len(paths)) + + for i := range paths { + moduleName := filepath.Base(paths[i]) + log.Debug("Found module", slog.String("module", moduleName)) + + if err := validateModule(paths[i], errorList); err != nil { + // linting errors are already logged + continue + } + + mdl, err := modules.NewModule(paths[i], &vals, globalValues, cfg, errorList) + if err != nil { + errorList. + WithFilePath(paths[i]).WithModule(moduleName). + WithValue(err.Error()). + Errorf("cannot create module `%s`", moduleName) + + continue + } + + targets = append(targets, manager.Target{ + Module: mdl, + Scope: scopes.Static, + // The directory, not the name: two directories may declare the same + // module name, and the summary must still count them separately. + ModuleID: paths[i], + }) + } + + return targets, nil +} + +func decodeValuesFile(path string) (chartutil.Values, error) { + if path == "" { + return nil, nil + } + + valuesFile, err := fsutils.ExpandDir(path) + if err != nil { + return nil, err + } + + return chartutil.ReadValuesFile(valuesFile) +} + +func getRootDirectory(dir string) string { + for { + if fsutils.IsDir(filepath.Join(dir, "global-hooks", "openapi")) && + fsutils.IsDir(filepath.Join(dir, "modules")) && + fsutils.IsFile(filepath.Join(dir, "global-hooks", "openapi", "config-values.yaml")) && + fsutils.IsFile(filepath.Join(dir, "global-hooks", "openapi", "values.yaml")) { + return dir + } + + parent := filepath.Dir(dir) + if dir == parent || parent == "" { + break + } + + dir = parent + } + + return "" +} diff --git a/internal/manager/validate.go b/internal/sources/static/validate.go similarity index 75% rename from internal/manager/validate.go rename to internal/sources/static/validate.go index 3cc9398c3..bf38dbb8b 100644 --- a/internal/manager/validate.go +++ b/internal/sources/static/validate.go @@ -1,4 +1,20 @@ -package manager +/* +Copyright 2025 Flant JSC + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package static import ( "errors" @@ -8,12 +24,17 @@ import ( "strings" "github.com/deckhouse/dmt/internal/modules" + dmtErrors "github.com/deckhouse/dmt/pkg/errors" ) -func (m *Manager) validateModule(path string) error { +// validateModule is the source tree's pre-flight check. It reports through the +// `module`/`definition-file` rule rather than under its own name, because what it +// checks is what that rule checks — the remote scopes reach the same ground through +// the bundle-layout and release-layout rules instead. +func validateModule(path string, errorList *dmtErrors.LintRuleErrorsList) error { var errs error - errorList := m.errors.WithLinterID("module").WithRule("definition-file").WithFilePath(path) + errorList = errorList.WithLinterID("module").WithRule("definition-file").WithFilePath(path) // validate module.yaml and Chart.yaml chartYamlFile, err := modules.ParseChartFile(path) if err != nil { diff --git a/internal/manager/validate_test.go b/internal/sources/static/validate_test.go similarity index 93% rename from internal/manager/validate_test.go rename to internal/sources/static/validate_test.go index 80de2d0d2..e2d0f11d1 100644 --- a/internal/manager/validate_test.go +++ b/internal/sources/static/validate_test.go @@ -1,4 +1,4 @@ -package manager +package static import ( "os" @@ -21,12 +21,7 @@ func TestValidateModule(t *testing.T) { _ = os.WriteFile(filepath.Join(tempDir, "openapi", "values.yaml"), []byte(""), 0600) _ = os.WriteFile(filepath.Join(tempDir, "openapi", "config-values.yaml"), []byte(""), 0600) - m := &Manager{ - errors: &errors.LintRuleErrorsList{}, - } - - err := m.validateModule(tempDir) - require.NoError(t, err) + require.NoError(t, validateModule(tempDir, errors.NewLintRuleErrorsList())) } func TestGetNamespace(t *testing.T) { diff --git a/pkg/config.go b/pkg/config.go index c69a6d408..c4efcc0cc 100644 --- a/pkg/config.go +++ b/pkg/config.go @@ -90,6 +90,7 @@ type DocumentationLinterRules struct { MarkdownlintRule RuleConfig SizeRule RuleConfig FrontMatterRule RuleConfig + ChangelogRule RuleConfig } type NoCyrillicLinterConfig struct { @@ -247,6 +248,8 @@ type ModuleLinterRules struct { ModulePackageConsistencyRule RuleConfig LegacyReleaseFileRule RuleConfig EnabledScriptRule RuleConfig + ReleaseLayoutRule RuleConfig + BundleLayoutRule RuleConfig } type OSSRuleSettings struct { Disable bool diff --git a/pkg/config/config.go b/pkg/config/config.go index f1630829e..e4446f283 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -24,6 +24,17 @@ import ( // RootConfig encapsulates the config data specified in the YAML config file. type RootConfig struct { GlobalSettings *global.Global `mapstructure:"global"` + Remote RemoteSettings `mapstructure:"remote"` +} + +// RemoteSettings holds the linter settings of the scopes that lint a published +// module. The two images carry different files and are linted by different rules, +// so each gets its own section — and neither inherits from `linters-settings`, +// which configures the source tree only. A section left out means built-in +// defaults, not the severities the source tree is linted with. +type RemoteSettings struct { + Bundle global.Linters `mapstructure:"bundle"` + Release global.Linters `mapstructure:"release"` } type ModuleConfig struct { diff --git a/pkg/config/global/global.go b/pkg/config/global/global.go index d8a2f7ee5..738369443 100644 --- a/pkg/config/global/global.go +++ b/pkg/config/global/global.go @@ -100,6 +100,7 @@ type DocumentationRules struct { MarkdownlintRule RuleConfig `mapstructure:"markdownlint"` SizeRule RuleConfig `mapstructure:"size"` FrontMatterRule RuleConfig `mapstructure:"front-matter"` + ChangelogRule RuleConfig `mapstructure:"changelog"` } type OpenAPILinterConfig struct { @@ -129,6 +130,8 @@ type ModuleLinterRules struct { ModulePackageConsistencyRule RuleConfig `mapstructure:"module-package-consistency"` LegacyReleaseFileRule RuleConfig `mapstructure:"legacy-release-file"` EnabledScriptRule RuleConfig `mapstructure:"enabled-script"` + ReleaseLayoutRule RuleConfig `mapstructure:"release-layout"` + BundleLayoutRule RuleConfig `mapstructure:"bundle-layout"` } type TemplatesLinterConfig struct { diff --git a/pkg/linters/README.md b/pkg/linters/README.md index 9506cf03b..7492c2ec5 100644 --- a/pkg/linters/README.md +++ b/pkg/linters/README.md @@ -6,10 +6,6 @@ for. Both rules and linters have a single interface, so a linter's rule set is data rather than a sequence of hand-written calls — which is what lets a scope select from it. -This mirrors `internal/verify` in -[d8-package-plugin](https://fox.flant.com/deckhouse/runtime/plugins/d8-package-plugin/-/tree/main/internal/verify), -which is where this shape comes from. - ## The interfaces ```go @@ -32,8 +28,19 @@ a context, no matter what the rule actually looks at. ## Scopes -A scope is a source a module is read from. `static` — the only one today — lints -the committed source tree; a scope for a built image will join it later. +A scope is a source a module is read from. There are three: + +| Scope | Source | Run by | +|---|---|---| +| `static` | the committed source tree | `dmt lint ` | +| `bundle` | the packaged module image, `:` | `dmt lint remote :` | +| `release` | the release metadata image, `/release:` | the same command | + +`dmt lint remote` runs both image scopes off one reference: it pulls each image, +unpacks it to a temporary directory and lints that as a module. The module +behind an image comes from `modules.NewRemoteModule`, which skips the chart load +and the render — so `GetChart`, `GetObjectStore` and `GetValues` are nil there, +and the `release` and `bundle` tables must not ask for a rule that reads them. **Linters and rules know nothing about scopes.** A linter is handed its config and a `set.Set` of rule IDs, and that is the whole of what a scope tells it: @@ -67,27 +74,46 @@ off. Note the difference between the two — a rule a scope never asks for produces nothing at all, while `impact: ignored` silences a rule that *did* run and still counts toward the ignored tally. +### Where a scope's severities come from + +Membership is code, severity is config, and each scope reads its own section of +`.dmtlint.yaml`: + +| Scope | Section | +|---|---| +| `static` | `global.linters-settings` | +| `bundle` | `remote.bundle` | +| `release` | `remote.release` | + +`Scope.Settings` is the only place that mapping lives, and `sources/remote` hands the +branch it returns to `modules.NewRemoteModule` rather than the whole root config — +a remote scope has no way to reach the source tree's settings even by accident. +The sections do not inherit from one another: an image is linted with the +severities written for it, or with the built-in defaults. + ### The table is the authority -`pkg/scopes/static.go` holds `staticRules`: for each linter, the rule IDs static -asks it for. That table is the only statement of membership. A linter does not -publish the list of rules it carries, and **nothing checks a scope's table -against that list** — deliberately. +One scope is one file in `pkg/scopes`, and each holds a table — `staticRules`, +`releaseRules`, `bundleRules` — of the rule IDs that scope asks each linter for. +That table is the only statement of membership. A linter does not publish the +list of rules it carries, and **nothing checks a scope's table against that +list** — deliberately. -The tempting check is "static must ask every linter for all of its rules", which -is true today and stops being true the moment a rule belongs to a built image and -not to the source tree. Encoding it would mean deleting the check as soon as the -second scope lands, and until then it would push back against the very thing -scopes exist to express. So the table is written out by hand and trusted. +The tempting check is "a scope must ask every linter for all of its rules". It +held while `static` was alone and stopped holding the moment `release` and +`bundle` landed: `release-layout` belongs to a built image and never runs over a +source tree, `markdownlint` is the other way round. A check like that would push +back against the very thing scopes exist to express, so the tables are written +out by hand and trusted. What that buys, and what it costs: - a rule can be in one scope and not another with no ceremony — add its ID where it belongs and nowhere else; - a rule added to a linter's `rules()` and forgotten in every table **silently - does not run**. `pkg/scopes/static_test.go` cannot catch that; it only checks - that the table's keys match the linters `staticLinters` builds, and that none - of them is asked for an empty set. + does not run**. `pkg/scopes/scopes_test.go` cannot catch that; it only checks, + for every scope, that the table's keys match the linters that scope builds, and + that none of them is asked for an empty set. Tests that need a linter exercised whole derive the ID set from `rules()` rather than naming one — see `everyRule` in `pkg/linters/container/container_test.go`. diff --git a/pkg/linters/docs/documentation.go b/pkg/linters/docs/documentation.go index 09df54b17..e8f1babfa 100644 --- a/pkg/linters/docs/documentation.go +++ b/pkg/linters/docs/documentation.go @@ -59,6 +59,7 @@ func (l *Documentation) rules() []pkg.Rule { rules.NewMarkdownRule(m, errorList.WithMaxLevel(l.cfg.Rules.MarkdownlintRule.GetLevel())), rules.NewSizeRule(m, errorList.WithMaxLevel(l.cfg.Rules.SizeRule.GetLevel())), rules.NewFrontMatterRule(m, errorList.WithMaxLevel(l.cfg.Rules.FrontMatterRule.GetLevel())), + rules.NewChangelogRule(m, errorList.WithMaxLevel(l.cfg.Rules.ChangelogRule.GetLevel())), } } diff --git a/pkg/linters/docs/rules/changelog.go b/pkg/linters/docs/rules/changelog.go new file mode 100644 index 000000000..70e900cf9 --- /dev/null +++ b/pkg/linters/docs/rules/changelog.go @@ -0,0 +1,52 @@ +// Copyright 2025 Flant JSC +// Licensed under the Apache License, Version 2.0 + +package rules + +import ( + "context" + "os" + "path/filepath" + + "github.com/deckhouse/dmt/pkg" + "github.com/deckhouse/dmt/pkg/errors" +) + +const ( + ChangelogRuleName = "changelog" +) + +func NewChangelogRule(m pkg.Module, errorList *errors.LintRuleErrorsList) *ChangelogRule { + return &ChangelogRule{ + RuleMeta: pkg.RuleMeta{ + Name: ChangelogRuleName, + }, + module: m, + errorList: errorList.WithRule(ChangelogRuleName), + } +} + +type ChangelogRule struct { + pkg.RuleMeta + + module pkg.Module + errorList *errors.LintRuleErrorsList +} + +var _ pkg.Rule = (*ChangelogRule)(nil) + +func (r *ChangelogRule) Check(_ context.Context) { + path := filepath.Join(r.module.GetPath(), "changelog.yaml") + errorList := r.errorList.WithFilePath(path) + + info, err := os.Stat(path) + + switch { + case os.IsNotExist(err): + errorList.Error("changelog.yaml file is missing") + case err != nil: + errorList.WithValue(err.Error()).Error("failed to check changelog.yaml file") + case info.Size() == 0: + errorList.Error("changelog.yaml file is empty") + } +} diff --git a/pkg/linters/docs/rules/readme.go b/pkg/linters/docs/rules/readme.go index cd4f9b9b5..69973e0cd 100644 --- a/pkg/linters/docs/rules/readme.go +++ b/pkg/linters/docs/rules/readme.go @@ -43,12 +43,15 @@ func (r *ReadmeRule) Check(_ context.Context) { return } - modulePath := m.GetPath() - path := filepath.Join(modulePath, "docs", "README.md") + // relPath is what the finding names and path is what is read: for a remote scope + // the module path is a temporary extraction directory, removed before findings + // are printed, so reporting it would point at nothing. + relPath := filepath.Join("docs", "README.md") + path := filepath.Join(m.GetPath(), relPath) if _, err := os.Stat(path); err != nil { errorList. - WithFilePath(path). + WithFilePath(relPath). Error("README.md file is missing in docs/ directory") return @@ -57,7 +60,7 @@ func (r *ReadmeRule) Check(_ context.Context) { info, err := os.Stat(path) if err != nil { errorList. - WithFilePath(path). + WithFilePath(relPath). WithValue(err.Error()). Error("failed to check README.md file") @@ -66,7 +69,7 @@ func (r *ReadmeRule) Check(_ context.Context) { if info.Size() == 0 { errorList. - WithFilePath(path). + WithFilePath(relPath). Error("README.md file is empty") } } diff --git a/pkg/linters/module/module.go b/pkg/linters/module/module.go index 607b103e7..72d004b90 100644 --- a/pkg/linters/module/module.go +++ b/pkg/linters/module/module.go @@ -79,6 +79,8 @@ func (l *Module) rules() []pkg.Rule { rules.NewModulePackageConsistencyRule(m, level(cfg.Rules.ModulePackageConsistencyRule)), rules.NewLegacyReleaseFileRule(m, level(cfg.Rules.LegacyReleaseFileRule)), rules.NewEnabledScriptRule(m, level(cfg.Rules.EnabledScriptRule)), + rules.NewReleaseLayoutRule(m, level(cfg.Rules.ReleaseLayoutRule)), + rules.NewBundleLayoutRule(m, level(cfg.Rules.BundleLayoutRule)), } } diff --git a/pkg/linters/module/rules/layout.go b/pkg/linters/module/rules/layout.go new file mode 100644 index 000000000..a81487e10 --- /dev/null +++ b/pkg/linters/module/rules/layout.go @@ -0,0 +1,124 @@ +/* +Copyright 2025 Flant JSC + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package rules + +import ( + "context" + "os" + "path/filepath" + + "github.com/deckhouse/dmt/pkg" + "github.com/deckhouse/dmt/pkg/errors" +) + +const ( + ReleaseLayoutRuleName = "release-layout" + BundleLayoutRuleName = "bundle-layout" +) + +// LayoutRule reports the files and directories a built image is missing from its +// package root. +// +// It exists as its own rule rather than as a flag on the rules that parse those +// files because presence is a property of the scope, not of the file: a source +// tree may legitimately lack version.json, an image may not. The parsing rules +// therefore keep returning quietly when a file is absent, and this rule is what +// the release and bundle scopes ask for to make the absence a finding. +type LayoutRule struct { + pkg.RuleMeta + + module pkg.Module + errorList *errors.LintRuleErrorsList + files []string + dirs []string +} + +var _ pkg.Rule = (*LayoutRule)(nil) + +// NewReleaseLayoutRule checks the root of a release image, which carries only the +// metadata Deckhouse reads to decide whether to install the version. +func NewReleaseLayoutRule(m pkg.Module, errorList *errors.LintRuleErrorsList) *LayoutRule { + return newLayoutRule(ReleaseLayoutRuleName, m, errorList, + []string{"module.yaml", "version.json", "changelog.yaml"}, + nil, + ) +} + +// NewBundleLayoutRule checks the root of a bundle image, which carries the whole +// packaged module — chart, templates and docs included. +// +// The list is what a published bundle actually holds, which is not what its source +// tree holds: changelog.yaml and version.json ship in the sibling release image, and +// the ignore file the package carries is .helmignore, not .gitignore. +// +// It is the intersection of eight published CE bundles, not every path they carry — +// crds/, hooks/, monitoring/ and .werf/ appear in some and not others, so requiring +// any of them would fail the modules that legitimately have nothing to put there. +func NewBundleLayoutRule(m pkg.Module, errorList *errors.LintRuleErrorsList) *LayoutRule { + return newLayoutRule(BundleLayoutRuleName, m, errorList, + []string{".helmignore", "Chart.yaml", "images_digests.json", "module.yaml"}, + []string{"charts", "docs", "openapi", "templates"}, + ) +} + +func newLayoutRule(name string, m pkg.Module, errorList *errors.LintRuleErrorsList, files, dirs []string) *LayoutRule { + return &LayoutRule{ + RuleMeta: pkg.RuleMeta{Name: name}, + module: m, + errorList: errorList.WithRule(name), + files: files, + dirs: dirs, + } +} + +func (r *LayoutRule) Check(_ context.Context) { + root := r.module.GetPath() + if root == "" { + return + } + + for _, name := range r.files { + r.check(root, name, false) + } + + for _, name := range r.dirs { + r.check(root, name, true) + } +} + +// check reports the three outcomes apart: the path is absent, it exists but is of +// the wrong kind, or it could not be read at all. +func (r *LayoutRule) check(root, name string, wantDir bool) { + kind := "file" + if wantDir { + kind = "directory" + } + + path := filepath.Join(root, name) + errorList := r.errorList.WithFilePath(name) + + info, err := os.Stat(path) + + switch { + case os.IsNotExist(err): + errorList.Errorf("%s %s is missing in package root", name, kind) + case err != nil: + errorList.WithValue(err.Error()).Errorf("failed to check %s %s", name, kind) + case info.IsDir() != wantDir: + errorList.Errorf("%s must be a %s in package root", name, kind) + } +} diff --git a/pkg/linters/module/rules/layout_test.go b/pkg/linters/module/rules/layout_test.go new file mode 100644 index 000000000..46b5b53ac --- /dev/null +++ b/pkg/linters/module/rules/layout_test.go @@ -0,0 +1,99 @@ +/* +Copyright 2025 Flant JSC + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package rules + +import ( + "context" + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/deckhouse/dmt/pkg" + "github.com/deckhouse/dmt/pkg/errors" +) + +func TestLayoutRules(t *testing.T) { + releaseFiles := []string{"module.yaml", "version.json", "changelog.yaml"} + // The bundle fixture is the root of a real published bundle, not a copy of the + // rule's own list — deriving it from the rule would only prove the rule agrees + // with itself. Eight CE bundles (sds-node-configurator, sds-local-volume, + // sds-replicated-volume, csi-nfs, console, commander-agent, observability, + // secrets-store-integration) all carry these, and differ only in the optional + // crds/, hooks/, monitoring/ and .werf/. + bundleFiles := []string{".helmignore", "Chart.yaml", "images_digests.json", "module.yaml"} + bundleDirs := []string{"charts", "docs", "openapi", "templates"} + + t.Run("release layout is complete", func(t *testing.T) { + root := layoutAt(t, releaseFiles, nil) + + assert.Empty(t, checkLayout(t, NewReleaseLayoutRule, root)) + }) + + t.Run("bundle layout is complete", func(t *testing.T) { + root := layoutAt(t, bundleFiles, bundleDirs) + + assert.Empty(t, checkLayout(t, NewBundleLayoutRule, root)) + }) + + t.Run("a missing file is one finding", func(t *testing.T) { + root := layoutAt(t, releaseFiles, nil) + require.NoError(t, os.Remove(filepath.Join(root, "version.json"))) + + errs := checkLayout(t, NewReleaseLayoutRule, root) + + require.Len(t, errs, 1) + assert.Contains(t, errs[0].Text, "version.json file is missing") + }) + + t.Run("the wrong kind is reported as such", func(t *testing.T) { + // docs is a directory in a bundle; a file by that name is not the same thing. + root := layoutAt(t, append(bundleFiles, "docs"), []string{"charts", "openapi", "templates"}) + + errs := checkLayout(t, NewBundleLayoutRule, root) + + require.Len(t, errs, 1) + assert.Contains(t, errs[0].Text, "docs must be a directory") + }) +} + +func checkLayout(t *testing.T, newRule func(pkg.Module, *errors.LintRuleErrorsList) *LayoutRule, root string) []pkg.LinterError { + t.Helper() + + errorList := errors.NewLintRuleErrorsList() + newRule(moduleAt(t, root), errorList).Check(context.Background()) + + return errorList.GetErrors() +} + +func layoutAt(t *testing.T, files, dirs []string) string { + t.Helper() + + root := t.TempDir() + + for _, name := range files { + require.NoError(t, os.WriteFile(filepath.Join(root, name), []byte("x"), DefaultFilePerm)) + } + + for _, name := range dirs { + require.NoError(t, os.Mkdir(filepath.Join(root, name), DefaultDirPerm)) + } + + return root +} diff --git a/pkg/scopes/bundle.go b/pkg/scopes/bundle.go new file mode 100644 index 000000000..7ffc445c5 --- /dev/null +++ b/pkg/scopes/bundle.go @@ -0,0 +1,61 @@ +/* +Copyright 2025 Flant JSC + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package scopes + +import ( + "github.com/deckhouse/dmt/internal/modules" + "github.com/deckhouse/dmt/internal/set" + "github.com/deckhouse/dmt/pkg" + "github.com/deckhouse/dmt/pkg/errors" + "github.com/deckhouse/dmt/pkg/linters/docs" + docsrules "github.com/deckhouse/dmt/pkg/linters/docs/rules" + moduleLinter "github.com/deckhouse/dmt/pkg/linters/module" + modulerules "github.com/deckhouse/dmt/pkg/linters/module/rules" +) + +// bundleRules is the rule membership of the bundle scope: the bundle image +// (:) holds the packaged module — chart, templates, docs and digests — +// so bundle-layout asks for the whole of that shape. The changelog rule is not part +// of it: changelog.yaml ships in the release image, and release-layout is what makes +// its absence a finding there. +// +// What it deliberately does not ask for is anything under the templates or container +// linters. A bundle carries rendered-looking directories but the module behind this +// scope comes from modules.NewRemoteModule, whose object store is nil; those linters +// would work off a chart that was never loaded. +var bundleRules = map[string]set.Set{ + moduleLinter.ID: set.New( + modulerules.BundleLayoutRuleName, + ), + docs.ID: set.New( + docsrules.ReadmeRuleName, + ), +} + +// bundleLinters builds the linters of the bundle scope, handing each one its slice of +// the module config and the rule IDs bundleRules asks it for. +func bundleLinters(m *modules.Module, errList *errors.LintRuleErrorsList) []Linter { + cfg := m.GetModuleConfig() + if cfg == nil { + cfg = &pkg.LintersSettings{} + } + + return []Linter{ + moduleLinter.New(&cfg.Module, bundleRules[moduleLinter.ID], m, errList), + docs.New(&cfg.Documentation, bundleRules[docs.ID], m, errList), + } +} diff --git a/pkg/scopes/release.go b/pkg/scopes/release.go new file mode 100644 index 000000000..c83c3a635 --- /dev/null +++ b/pkg/scopes/release.go @@ -0,0 +1,58 @@ +/* +Copyright 2025 Flant JSC + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package scopes + +import ( + "github.com/deckhouse/dmt/internal/modules" + "github.com/deckhouse/dmt/internal/set" + "github.com/deckhouse/dmt/pkg" + "github.com/deckhouse/dmt/pkg/errors" + moduleLinter "github.com/deckhouse/dmt/pkg/linters/module" + modulerules "github.com/deckhouse/dmt/pkg/linters/module/rules" +) + +// releaseRules is the rule membership of the release scope: the release image +// (/release:) holds only the metadata Deckhouse reads to decide whether +// to install a version, so the scope asks for the rules that live in that metadata +// and for nothing that needs a chart or a rendered object — the module behind this +// scope is built by modules.NewRemoteModule and has neither. +// +// release-layout is what makes a missing file a finding. definition-file and +// package-yaml validate the contents of module.yaml and package.yaml and stay quiet +// when the file is absent, which is why the presence check is a rule of its own. +// package.yaml is not in the layout list on purpose: it is validated when the image +// ships one, and its absence is not an error. +var releaseRules = map[string]set.Set{ + moduleLinter.ID: set.New( + modulerules.ReleaseLayoutRuleName, + modulerules.DefinitionFileRuleName, + modulerules.PackageYAMLRuleName, + ), +} + +// releaseLinters builds the linters of the release scope, handing each one its slice +// of the module config and the rule IDs releaseRules asks it for. +func releaseLinters(m *modules.Module, errList *errors.LintRuleErrorsList) []Linter { + cfg := m.GetModuleConfig() + if cfg == nil { + cfg = &pkg.LintersSettings{} + } + + return []Linter{ + moduleLinter.New(&cfg.Module, releaseRules[moduleLinter.ID], m, errList), + } +} diff --git a/pkg/scopes/scope.go b/pkg/scopes/scope.go index 21d728418..c9c825902 100644 --- a/pkg/scopes/scope.go +++ b/pkg/scopes/scope.go @@ -33,6 +33,8 @@ import ( "context" "github.com/deckhouse/dmt/internal/modules" + "github.com/deckhouse/dmt/pkg/config" + "github.com/deckhouse/dmt/pkg/config/global" "github.com/deckhouse/dmt/pkg/errors" ) @@ -40,8 +42,38 @@ import ( type Scope string // Static lints a module directory on disk, i.e. the full source tree as committed. -// A scope for a built image will join it later. -const Static Scope = "static" +// Release and Bundle lint the two images a published module consists of, unpacked +// from the registry: the bundle is the packaged module itself, the release is the +// metadata Deckhouse reads to decide whether to install a version. +const ( + Static Scope = "static" + Release Scope = "release" + Bundle Scope = "bundle" +) + +// Settings returns the linter settings that configure this scope. Each scope reads its +// own section of .dmtlint.yaml — `linters-settings` under `global` for the source tree, +// `remote.bundle` and `remote.release` for the two published images — so the same rule +// can carry a different severity depending on where it is checked. The sections are +// independent: an image is not linted with the severities the source tree was tuned to, +// and a section left out means the built-in defaults. +func (s Scope) Settings(cfg *config.RootConfig) *global.Linters { + switch s { + case Release: + return &cfg.Remote.Release + case Bundle: + return &cfg.Remote.Bundle + default: + // The loader always fills GlobalSettings in, but a RootConfig built by hand can + // leave it nil. An empty tree is the right answer there: every impact remaps to + // its default, which is what a missing config means everywhere else. + if cfg.GlobalSettings == nil { + return &global.Linters{} + } + + return &cfg.GlobalSettings.Linters + } +} // Linter is the common interface implemented by all lint passes. Everything a linter // needs — its config, the rule IDs the scope asked it for, the module it inspects and @@ -59,8 +91,11 @@ type Linter interface { // concrete type only. That is deliberate: slicing the config per linter is the scope's // job, and a linter must not be able to reach a sibling's settings. func (s Scope) Linters(m *modules.Module, errList *errors.LintRuleErrorsList) []Linter { - //nolint: gocritic switch s { + case Release: + return releaseLinters(m, errList) + case Bundle: + return bundleLinters(m, errList) default: return staticLinters(m, errList) } diff --git a/pkg/scopes/scopes_test.go b/pkg/scopes/scopes_test.go new file mode 100644 index 000000000..a64eac9f1 --- /dev/null +++ b/pkg/scopes/scopes_test.go @@ -0,0 +1,208 @@ +/* +Copyright 2025 Flant JSC + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package scopes + +import ( + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/deckhouse/dmt/internal/modules" + "github.com/deckhouse/dmt/internal/set" + "github.com/deckhouse/dmt/pkg" + "github.com/deckhouse/dmt/pkg/config" + "github.com/deckhouse/dmt/pkg/errors" +) + +// TestTablesCoverExactlyTheirLinters keeps every scope's table and its constructor +// list in step. The two failures it catches are the ones a hand-written table cannot +// survive: a set for a linter the scope does not build is dead weight, and a linter +// built with no entry in the table gets a nil set and runs nothing at all while +// reporting the module clean. +// +// Note what is deliberately *not* asserted: that a scope asks each linter for every +// rule it carries. A rule meant for a built image and not for the source tree — or +// the other way round — is exactly what scopes exist to express, so the table is the +// authority on membership and nothing checks it against a linter's full rule set. +func TestTablesCoverExactlyTheirLinters(t *testing.T) { + for _, tc := range []struct { + name string + rules map[string]set.Set + linters func(*modules.Module, *errors.LintRuleErrorsList) []Linter + }{ + {"static", staticRules, staticLinters}, + {"release", releaseRules, releaseLinters}, + {"bundle", bundleRules, bundleLinters}, + } { + t.Run(tc.name, func(t *testing.T) { + built := make([]string, 0, len(tc.rules)) + for _, l := range tc.linters(&modules.Module{}, errors.NewLintRuleErrorsList()) { + built = append(built, l.GetName()) + } + + for id := range tc.rules { + assert.Contains(t, built, id, "%sRules holds %q, which %sLinters does not build", tc.name, id, tc.name) + } + + for _, id := range built { + asked, ok := tc.rules[id] + assert.True(t, ok, "%sLinters builds %q, which %sRules has no set for", tc.name, id, tc.name) + assert.NotZero(t, asked.Size(), "%sRules asks %q for no rules, so it would run nothing", tc.name, id) + } + }) + } +} + +// TestRemoteScopesRunOverAnUnpackedImage is the guard for the invariant the release +// and bundle tables carry: their module comes from modules.NewRemoteModule, which +// loads no chart and renders nothing, so a rule reaching for GetChart, GetObjectStore +// or GetValues panics on nil. Running both scopes over a directory is what catches a +// rule ID added to one of those tables that cannot survive there. +func TestRemoteScopesRunOverAnUnpackedImage(t *testing.T) { + cfg, err := config.NewDefaultRootConfig(".") + require.NoError(t, err) + + for _, tc := range []struct { + scope Scope + files []string + dirs []string + wantErr bool + }{ + {scope: Release, files: []string{"module.yaml", "version.json", "changelog.yaml"}}, + {scope: Release, wantErr: true}, + { + scope: Bundle, + // The root of a real published bundle — see layout_test.go for where it + // comes from. Deriving it from bundleRules would test nothing. + files: []string{".helmignore", "Chart.yaml", "images_digests.json", "module.yaml"}, + dirs: []string{"charts", "docs", "openapi", "templates"}, + }, + {scope: Bundle, wantErr: true}, + } { + name := string(tc.scope) + " complete" + if tc.wantErr { + name = string(tc.scope) + " empty" + } + + t.Run(name, func(t *testing.T) { + root := t.TempDir() + for _, f := range tc.files { + require.NoError(t, os.WriteFile(filepath.Join(root, f), []byte("x"), 0o600)) + } + + for _, d := range tc.dirs { + require.NoError(t, os.Mkdir(filepath.Join(root, d), 0o755)) + } + + // The layout rules only want the paths to exist, but definition-file parses + // module.yaml and the readme rule reads docs/README.md, so the complete + // cases need real content in both. + if len(tc.files) > 0 { + require.NoError(t, os.WriteFile(filepath.Join(root, "module.yaml"), + []byte("name: test-module\nstage: General Availability\ndescriptions:\n en: a module\n"), 0o600)) + } + + if len(tc.dirs) > 0 { + require.NoError(t, os.WriteFile(filepath.Join(root, "docs", "README.md"), []byte("x"), 0o600)) + } + + m := modules.NewRemoteModule(root, "test-module", tc.scope.Settings(cfg)) + + errorList := errors.NewLintRuleErrorsList() + for _, linter := range tc.scope.Linters(m, errorList) { + linter.Lint(t.Context()) + } + + assert.Equal(t, tc.wantErr, errorList.ContainsErrors(), "findings: %v", errorList.GetErrors()) + + // A remote scope's module path is a temp extraction directory that is + // removed before the findings are printed, so a rule that reports the path + // it read instead of the path inside the module names a file nobody can + // open. This holds every rule either table may be given to that, not just + // the ones it holds today. + for _, e := range errorList.GetErrors() { + assert.NotContains(t, e.FilePath, root, + "rule %q reports the extraction directory", e.RuleID) + } + }) + } +} + +// TestScopeSettings pins the config layout the scopes read: `linters-settings` under +// `global` configures the source tree, `remote.bundle` and `remote.release` configure +// the two published images, and neither remote section inherits from the other two. +// The last part is what the test is really for — a scope silently falling back to the +// source-tree severities would look like a working config right up until someone +// relaxes a rule locally and finds the published images relaxed with it. +func TestScopeSettings(t *testing.T) { + dir := t.TempDir() + + dmtlint := ` +global: + linters-settings: + openapi: + rules: + bilingual: + impact: error + module: + impact: ignored + +linters-settings: + openapi: + impact: error + +remote: + release: + openapi: + rules: + bilingual: + impact: warn + bundle: + openapi: + rules: + bilingual: + impact: ignored +` + require.NoError(t, os.WriteFile(filepath.Join(dir, ".dmtlint.yaml"), []byte(dmtlint), 0o600)) + + cfg, err := config.NewDefaultRootConfig(dir) + require.NoError(t, err) + + for _, tc := range []struct { + scope Scope + bilingual string + }{ + {Static, "error"}, + {Release, "warn"}, + {Bundle, "ignored"}, + } { + t.Run(string(tc.scope), func(t *testing.T) { + assert.Equal(t, tc.bilingual, tc.scope.Settings(cfg).OpenAPI.Rules.BilingualRule.Impact) + }) + } + + // The module linter is configured for the source tree only, so a remote scope must + // see it unset and fall back to the built-in severity rather than to `ignored`. + assert.Empty(t, Release.Settings(cfg).Module.Impact) + + m := modules.NewRemoteModule(dir, "test-module", Release.Settings(cfg)) + assert.Equal(t, pkg.Warn, *m.GetModuleConfig().OpenAPI.Rules.BilingualRule.GetLevel()) + assert.Equal(t, pkg.Error, *m.GetModuleConfig().Module.Impact) +} diff --git a/pkg/scopes/static_test.go b/pkg/scopes/static_test.go deleted file mode 100644 index 28abeda74..000000000 --- a/pkg/scopes/static_test.go +++ /dev/null @@ -1,52 +0,0 @@ -/* -Copyright 2025 Flant JSC - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package scopes - -import ( - "testing" - - "github.com/stretchr/testify/assert" - - "github.com/deckhouse/dmt/internal/modules" - "github.com/deckhouse/dmt/pkg/errors" -) - -// TestStaticTableCoversExactlyItsLinters keeps the table and the constructor list in step. -// The two failures it catches are the ones the table cannot survive: a set for a linter -// static does not build is dead weight, and a linter built with no entry in the table gets -// a nil set and runs nothing at all while reporting the module clean. -// -// Note what is deliberately *not* asserted: that static asks each linter for every rule it -// carries. That would only hold while static is the only scope. A rule meant for a built -// image and not for the source tree is exactly what scopes exist to express, so the table -// is the authority on membership and nothing checks it against a linter's full rule set. -func TestStaticTableCoversExactlyItsLinters(t *testing.T) { - built := make([]string, 0, len(staticRules)) - for _, l := range staticLinters(&modules.Module{}, errors.NewLintRuleErrorsList()) { - built = append(built, l.GetName()) - } - - for id := range staticRules { - assert.Contains(t, built, id, "staticRules holds %q, which staticLinters does not build", id) - } - - for _, id := range built { - asked, ok := staticRules[id] - assert.True(t, ok, "staticLinters builds %q, which staticRules has no set for", id) - assert.NotZero(t, asked.Size(), "staticRules asks %q for no rules, so it would run nothing", id) - } -} diff --git a/test/e2e/framework.go b/test/e2e/framework.go index 31b87855a..7dbd67676 100644 --- a/test/e2e/framework.go +++ b/test/e2e/framework.go @@ -48,10 +48,10 @@ import ( "github.com/deckhouse/dmt/internal/flags" "github.com/deckhouse/dmt/internal/manager" "github.com/deckhouse/dmt/internal/metrics" + "github.com/deckhouse/dmt/internal/sources/static" "github.com/deckhouse/dmt/internal/test" "github.com/deckhouse/dmt/pkg" "github.com/deckhouse/dmt/pkg/config" - "github.com/deckhouse/dmt/pkg/scopes" ) // Case kinds. A case either lints a module (KindLint, the default) or runs the @@ -191,8 +191,10 @@ func Lint(moduleDir string) ([]pkg.LinterError, error) { // Initialize the metrics client so linters that emit metrics don't panic. metrics.GetClient(target) - mng := manager.NewManager(target, cfg, scopes.Static) - mng.Run(context.Background()) + mng := manager.New(cfg, static.NewSource(target)) + defer mng.Close() + + _ = mng.Run(context.Background()) return mng.GetErrors(), nil } @@ -222,8 +224,10 @@ func RunFix(moduleDir string) ([]pkg.LinterError, error) { metrics.GetClient(target) - mng := manager.NewManager(target, cfg, scopes.Static) - mng.Run(context.Background()) + mng := manager.New(cfg, static.NewSource(target)) + defer mng.Close() + + _ = mng.Run(context.Background()) mng.ApplyFixes() diff --git a/test/e2e/testdata/conversions/failing/module/Chart.yaml b/test/e2e/testdata/conversions/failing/module/module.yaml similarity index 100% rename from test/e2e/testdata/conversions/failing/module/Chart.yaml rename to test/e2e/testdata/conversions/failing/module/module.yaml diff --git a/test/e2e/testdata/conversions/passing/module/Chart.yaml b/test/e2e/testdata/conversions/passing/module/module.yaml similarity index 100% rename from test/e2e/testdata/conversions/passing/module/Chart.yaml rename to test/e2e/testdata/conversions/passing/module/module.yaml diff --git a/test/e2e/testdata/conversions/version-mismatch/module/Chart.yaml b/test/e2e/testdata/conversions/version-mismatch/module/module.yaml similarity index 100% rename from test/e2e/testdata/conversions/version-mismatch/module/Chart.yaml rename to test/e2e/testdata/conversions/version-mismatch/module/module.yaml