From 070396d5fd18a2871c68b0439ad02b9faee2c518 Mon Sep 17 00:00:00 2001 From: Ruslan Gorbunov Date: Mon, 31 Aug 2026 11:14:56 +0300 Subject: [PATCH 01/14] feat: add remote image linting Signed-off-by: Ruslan Gorbunov Co-authored-by: Smyslov Maxim --- cmd/dmt/root.go | 15 +++ go.mod | 14 ++- go.sum | 14 ++- internal/flags/flags.go | 10 ++ internal/manager/manager.go | 9 +- internal/modules/module.go | 29 +++++ internal/remotelint/extract.go | 160 ++++++++++++++++++++++++ internal/remotelint/lint.go | 155 +++++++++++++++++++++++ internal/remotelint/lint_test.go | 126 +++++++++++++++++++ internal/remotelint/registry.go | 83 ++++++++++++ pkg/config.go | 3 + pkg/config/global/global.go | 3 + pkg/linters/README.md | 41 +++--- pkg/linters/docs/documentation.go | 1 + pkg/linters/docs/rules/changelog.go | 52 ++++++++ pkg/linters/module/module.go | 2 + pkg/linters/module/rules/layout.go | 116 +++++++++++++++++ pkg/linters/module/rules/layout_test.go | 93 ++++++++++++++ pkg/scopes/bundle.go | 60 +++++++++ pkg/scopes/release.go | 58 +++++++++ pkg/scopes/scope.go | 15 ++- pkg/scopes/scopes_test.go | 134 ++++++++++++++++++++ pkg/scopes/static_test.go | 52 -------- 23 files changed, 1169 insertions(+), 76 deletions(-) create mode 100644 internal/remotelint/extract.go create mode 100644 internal/remotelint/lint.go create mode 100644 internal/remotelint/lint_test.go create mode 100644 internal/remotelint/registry.go create mode 100644 pkg/linters/docs/rules/changelog.go create mode 100644 pkg/linters/module/rules/layout.go create mode 100644 pkg/linters/module/rules/layout_test.go create mode 100644 pkg/scopes/bundle.go create mode 100644 pkg/scopes/release.go create mode 100644 pkg/scopes/scopes_test.go delete mode 100644 pkg/scopes/static_test.go diff --git a/cmd/dmt/root.go b/cmd/dmt/root.go index b0a801a1c..aecd91582 100644 --- a/cmd/dmt/root.go +++ b/cmd/dmt/root.go @@ -35,6 +35,7 @@ import ( "github.com/deckhouse/dmt/internal/bootstrap" "github.com/deckhouse/dmt/internal/flags" "github.com/deckhouse/dmt/internal/fsutils" + "github.com/deckhouse/dmt/internal/remotelint" "github.com/deckhouse/dmt/internal/rendercmd" "github.com/deckhouse/dmt/internal/test" "github.com/deckhouse/dmt/internal/version" @@ -270,6 +271,20 @@ func runTests(dir string, opts ...test.Option) error { } func lintCmdFunc(cmd *cobra.Command, args []string) { + if flags.Remote != "" { + opts := &remotelint.Options{ + Login: flags.RemoteLogin, + Password: flags.RemotePassword, + } + + if err := remotelint.Run(cmd.Context(), flags.Remote, opts); err != nil { + log.Error("Error running remote lint", log.Err(err)) + os.Exit(1) + } + + return + } + var dirs = args[0:] if len(dirs) == 0 { 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..a68e3d942 100644 --- a/internal/flags/flags.go +++ b/internal/flags/flags.go @@ -32,6 +32,12 @@ var ( LinterName string ) +var ( + Remote string + RemoteLogin string + RemotePassword string +) + var ( PrintVersion bool Version string @@ -62,6 +68,10 @@ func InitDefaultFlagSet() *pflag.FlagSet { func InitLintFlagSet() *pflag.FlagSet { lint := pflag.NewFlagSet("lint", pflag.ContinueOnError) + lint.StringVarP(&Remote, "remote", "r", "", "lint the published images instead of a directory, e.g. 'registry.example.com/my-module:v0.0.1'") + lint.StringVar(&RemoteLogin, "login", "", "registry login, used with --remote") + lint.StringVar(&RemotePassword, "password", "", "registry password, used with --remote") + lint.IntVarP(&LintersLimit, "parallel", "p", numThreads, "number of threads for parallel processing") lint.StringVar(&LinterName, "linter", "", "linter name to run") diff --git a/internal/manager/manager.go b/internal/manager/manager.go index a6fa4f753..e40d4e457 100644 --- a/internal/manager/manager.go +++ b/internal/manager/manager.go @@ -180,7 +180,14 @@ func (m *Manager) Run(ctx context.Context) { } func (m *Manager) PrintResult() { - errs := m.errors.GetErrors() + PrintResult(m.errors) +} + +// PrintResult renders a finished error list. It is a package function rather than a +// method only so the remote-lint path, which builds its list without a Manager, prints +// exactly what a local run prints. +func PrintResult(errorList *errors.LintRuleErrorsList) { + errs := errorList.GetErrors() if len(errs) == 0 { return diff --git a/internal/modules/module.go b/internal/modules/module.go index 4d7ba6c7f..5a58b422c 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,32 @@ 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, rootConfig *config.RootConfig) (*Module, error) { + // The image ships no .dmtlint.yaml of its own, so severities come from the + // config next to the caller, the same file a local run would read. + cfg := &config.ModuleConfig{} + if err := config.NewLoader(cfg, ".").Load(); err != nil { + return nil, fmt.Errorf("can not parse module config: %w", err) + } + + cfg.LintersSettings.MergeGlobal(&rootConfig.GlobalSettings.Linters) + + return &Module{ + name: name, + path: path, + linterConfig: remapLinterSettings(&cfg.LintersSettings, &rootConfig.GlobalSettings.Linters), + }, nil +} + func newModuleFromPath(path string) (*Module, error) { moduleYamlConfig, err := ParseModuleConfigFile(path) if err != nil { diff --git a/internal/remotelint/extract.go b/internal/remotelint/extract.go new file mode 100644 index 000000000..e399850d8 --- /dev/null +++ b/internal/remotelint/extract.go @@ -0,0 +1,160 @@ +/* +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 remotelint + +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 { + 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) + } + + entryPath, err := safeJoin(root, hdr.Name) + if err != nil { + return err + } + + switch hdr.Typeflag { + case tar.TypeDir: + if err = os.MkdirAll(entryPath, os.FileMode(hdr.Mode)); err != nil { + return fmt.Errorf("mkdir %q: %w", hdr.Name, err) + } + case tar.TypeReg: + if err = writeRegularFile(entryPath, tr, os.FileMode(hdr.Mode)); err != nil { + return fmt.Errorf("write file %q: %w", hdr.Name, err) + } + case tar.TypeSymlink: + // 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(root, filepath.Dir(entryPath), hdr.Linkname) { + return fmt.Errorf("symlink %q escapes output directory", hdr.Name) + } + + if err = os.Symlink(hdr.Linkname, entryPath); 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. + linkPath, err := safeJoin(root, hdr.Linkname) + if err != nil { + return err + } + + if err = os.Link(linkPath, entryPath); err != nil { + return fmt.Errorf("create hardlink %q: %w", hdr.Name, err) + } + } + } + + return nil +} + +// writeRegularFile writes one regular tar entry and limits restored permissions to owner bits. +func writeRegularFile(target string, src io.Reader, mode os.FileMode) error { + if err := os.MkdirAll(filepath.Dir(target), 0o700); err != nil { + return fmt.Errorf("create parent directory: %w", err) + } + + out, err := os.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 +} + +// safeJoin joins name under root and rejects absolute paths or parent-directory escapes. +func safeJoin(root, name string) (string, error) { + if filepath.IsAbs(name) { + return "", fmt.Errorf("path %q escapes output directory", name) + } + + if !staysWithin(root, root, name) { + return "", fmt.Errorf("path %q escapes output directory", name) + } + + return filepath.Join(root, name), nil +} + +// staysWithin reports whether name resolves under root when interpreted relative to base. +func staysWithin(root, base, name string) bool { + target := filepath.Clean(filepath.Join(base, name)) + rel, err := filepath.Rel(root, target) + + return err == nil && rel != ".." && !strings.HasPrefix(rel, ".."+string(filepath.Separator)) +} diff --git a/internal/remotelint/lint.go b/internal/remotelint/lint.go new file mode 100644 index 000000000..e79baeb45 --- /dev/null +++ b/internal/remotelint/lint.go @@ -0,0 +1,155 @@ +/* +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 remotelint 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 remotelint + +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/metrics" + "github.com/deckhouse/dmt/internal/modules" + "github.com/deckhouse/dmt/pkg" + "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 +} + +// Run lints the bundle and release images published under imagePath, e.g. +// registry.example.com/my-module:v0.0.1. +func Run(ctx context.Context, imagePath string, opts *Options) error { + repository, tag, err := cutTagFromImagePath(imagePath) + if err != nil { + return fmt.Errorf("failed to cut tag from image path: %w", err) + } + + // The image ships no .dmtlint.yaml, so severities come from the config next to + // the caller — the same file a local run would read. + cfg, err := config.NewDefaultRootConfig(".") + if err != nil { + return fmt.Errorf("failed to parse default root config: %w", err) + } + + // init metrics storage, should be done before printing results, as PrintResult + // reports per-error metrics through the shared metrics client. + metrics.GetClient(".") + + client := newRegistryClient(repository, opts.Login, opts.Password) + moduleName := path.Base(repository) + + level := pkg.Error + errorList := errors.NewLintRuleErrorsList().WithMaxLevel(&level) + + // Both images are linted before anything is printed: a registry failure on one + // must not swallow what the other already found. + pullErr := stderrors.Join( + lintImage(ctx, client, tag, scopes.Bundle, moduleName, cfg, errorList), + lintImage(ctx, client.WithSegment(releaseSegment), tag, scopes.Release, moduleName, cfg, errorList), + ) + + manager.PrintResult(errorList) + + if pullErr != nil { + return pullErr + } + + if errorList.ContainsErrors() { + return stderrors.New("critical errors found") + } + + return nil +} + +// lintImage pulls one image, unpacks it and runs the scope that belongs to it. Which +// linters that scope runs, and which of their rules, is the scope's business — this +// only supplies the unpacked module. +func lintImage( + ctx context.Context, + client registry.Client, + tag string, + scope scopes.Scope, + moduleName string, + cfg *config.RootConfig, + errorList *errors.LintRuleErrorsList, +) error { + image, err := client.GetImage(ctx, tag) + if err != nil { + return fmt.Errorf("failed to get %s image: %w", scope, err) + } + + dir, err := extractImage(ctx, image) + if err != nil { + return fmt.Errorf("failed to extract %s image: %w", scope, err) + } + defer os.RemoveAll(dir) + + m, err := modules.NewRemoteModule(dir, moduleName, cfg) + if err != nil { + return fmt.Errorf("failed to read config for %s image: %w", scope, err) + } + + for _, linter := range scope.Linters(m, errorList.WithObjectID(string(scope))) { + linter.Lint(ctx) + } + + return 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") + } + + ref, err := name.ParseReference(imagePath, name.WithDefaultTag("")) + if err != nil { + return "", "", fmt.Errorf("failed to parse image path: %w", err) + } + + tag := ref.Identifier() + if tag == "" { + return "", "", fmt.Errorf("tag not found in image path") + } + + return ref.Context().Name(), tag, nil +} diff --git a/internal/remotelint/lint_test.go b/internal/remotelint/lint_test.go new file mode 100644 index 000000000..d0311682a --- /dev/null +++ b/internal/remotelint/lint_test.go @@ -0,0 +1,126 @@ +/* +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 remotelint + +import ( + "archive/tar" + "bytes" + "context" + "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) +} + +// 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") + }) + } +} + +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/remotelint/registry.go b/internal/remotelint/registry.go new file mode 100644 index 000000000..10e26590e --- /dev/null +++ b/internal/remotelint/registry.go @@ -0,0 +1,83 @@ +/* +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 remotelint + +import ( + "log/slog" + + "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" +) + +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: explicit login/password +// first, then the Docker config, then anonymous. +func registryAuth(registryHost, login, password string) authn.Authenticator { + 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/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/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..cc911bcc3 100644 --- a/pkg/linters/README.md +++ b/pkg/linters/README.md @@ -32,8 +32,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 | + +`--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: @@ -69,25 +80,27 @@ and still counts toward the ignored tally. ### 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/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..7bec27060 --- /dev/null +++ b/pkg/linters/module/rules/layout.go @@ -0,0 +1,116 @@ +/* +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. +func NewBundleLayoutRule(m pkg.Module, errorList *errors.LintRuleErrorsList) *LayoutRule { + return newLayoutRule(BundleLayoutRuleName, m, errorList, + []string{".gitignore", "changelog.yaml", "Chart.yaml", "images_digests.json", "version.json", "module.yaml"}, + []string{"docs", "templates", "charts"}, + ) +} + +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(path) + + 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..14da3b03e --- /dev/null +++ b/pkg/linters/module/rules/layout_test.go @@ -0,0 +1,93 @@ +/* +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"} + bundleFiles := []string{".gitignore", "changelog.yaml", "Chart.yaml", "images_digests.json", "version.json", "module.yaml"} + bundleDirs := []string{"docs", "templates", "charts"} + + 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{"templates", "charts"}) + + 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..b2310447f --- /dev/null +++ b/pkg/scopes/bundle.go @@ -0,0 +1,60 @@ +/* +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. +// +// 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, + docsrules.ChangelogRuleName, + ), +} + +// 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..b488867da 100644 --- a/pkg/scopes/scope.go +++ b/pkg/scopes/scope.go @@ -40,8 +40,14 @@ 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" +) // 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 +65,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..558f88fd3 --- /dev/null +++ b/pkg/scopes/scopes_test.go @@ -0,0 +1,134 @@ +/* +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/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, + files: []string{".gitignore", "changelog.yaml", "Chart.yaml", "images_digests.json", "version.json", "module.yaml"}, + dirs: []string{"docs", "templates", "charts"}, + }, + {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, err := modules.NewRemoteModule(root, "test-module", cfg) + require.NoError(t, err) + + 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()) + }) + } +} 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) - } -} From 693149c77b9bc38cf3f5ea818b7207926f0cd512 Mon Sep 17 00:00:00 2001 From: Ruslan Gorbunov Date: Wed, 2 Sep 2026 09:13:17 +0300 Subject: [PATCH 02/14] feat: configure remote linting scopes independently Signed-off-by: Ruslan Gorbunov --- README.md | 33 +++++++++++++++++++ internal/modules/module.go | 20 +++++------ internal/remotelint/lint.go | 8 ++--- pkg/config/config.go | 11 +++++++ pkg/linters/README.md | 17 ++++++++++ pkg/scopes/scope.go | 26 +++++++++++++++ pkg/scopes/scopes_test.go | 66 +++++++++++++++++++++++++++++++++++-- 7 files changed, 163 insertions(+), 18 deletions(-) diff --git a/README.md b/README.md index b9a512ef2..5e460f89d 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: @@ -295,6 +326,8 @@ dmt lint [directories...] [flags] ``` **Flags:** +- `--remote, -r`: Lint the published images instead of a directory, e.g. `registry.example.com/my-module:v0.0.1` +- `--login` / `--password`: Registry credentials for `--remote` (the Docker config is used when omitted) - `--values-file, -v`: Specify custom values file - `--linter`: Run specific linter only - `--hide-warnings`: Hide warning-level issues diff --git a/internal/modules/module.go b/internal/modules/module.go index 5a58b422c..86c1a25ef 100644 --- a/internal/modules/module.go +++ b/internal/modules/module.go @@ -616,21 +616,19 @@ func NewModule(path string, vals *chartutil.Values, globalSchema *spec.Schema, r // 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, rootConfig *config.RootConfig) (*Module, error) { - // The image ships no .dmtlint.yaml of its own, so severities come from the - // config next to the caller, the same file a local run would read. - cfg := &config.ModuleConfig{} - if err := config.NewLoader(cfg, ".").Load(); err != nil { - return nil, fmt.Errorf("can not parse module config: %w", err) - } - - cfg.LintersSettings.MergeGlobal(&rootConfig.GlobalSettings.Linters) +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.LintersSettings, &rootConfig.GlobalSettings.Linters), - }, nil + linterConfig: remapLinterSettings(cfg, linters), + } } func newModuleFromPath(path string) (*Module, error) { diff --git a/internal/remotelint/lint.go b/internal/remotelint/lint.go index e79baeb45..2f9e2d7a6 100644 --- a/internal/remotelint/lint.go +++ b/internal/remotelint/lint.go @@ -60,7 +60,8 @@ func Run(ctx context.Context, imagePath string, opts *Options) error { } // The image ships no .dmtlint.yaml, so severities come from the config next to - // the caller — the same file a local run would read. + // the caller — the same file a local run would read, but from its `remote.bundle` + // and `remote.release` sections rather than the ones the source tree uses. cfg, err := config.NewDefaultRootConfig(".") if err != nil { return fmt.Errorf("failed to parse default root config: %w", err) @@ -119,10 +120,7 @@ func lintImage( } defer os.RemoveAll(dir) - m, err := modules.NewRemoteModule(dir, moduleName, cfg) - if err != nil { - return fmt.Errorf("failed to read config for %s image: %w", scope, err) - } + m := modules.NewRemoteModule(dir, moduleName, scope.Settings(cfg)) for _, linter := range scope.Linters(m, errorList.WithObjectID(string(scope))) { linter.Lint(ctx) 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/linters/README.md b/pkg/linters/README.md index cc911bcc3..503e8bccf 100644 --- a/pkg/linters/README.md +++ b/pkg/linters/README.md @@ -78,6 +78,23 @@ 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 `remotelint` 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 One scope is one file in `pkg/scopes`, and each holds a table — `staticRules`, diff --git a/pkg/scopes/scope.go b/pkg/scopes/scope.go index b488867da..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" ) @@ -49,6 +51,30 @@ const ( 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 // the error list it reports into — is supplied to its constructor, so Lint takes only a diff --git a/pkg/scopes/scopes_test.go b/pkg/scopes/scopes_test.go index 558f88fd3..8b1d1356c 100644 --- a/pkg/scopes/scopes_test.go +++ b/pkg/scopes/scopes_test.go @@ -26,6 +26,7 @@ import ( "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" ) @@ -120,8 +121,7 @@ func TestRemoteScopesRunOverAnUnpackedImage(t *testing.T) { require.NoError(t, os.WriteFile(filepath.Join(root, "docs", "README.md"), []byte("x"), 0o600)) } - m, err := modules.NewRemoteModule(root, "test-module", cfg) - require.NoError(t, err) + m := modules.NewRemoteModule(root, "test-module", tc.scope.Settings(cfg)) errorList := errors.NewLintRuleErrorsList() for _, linter := range tc.scope.Linters(m, errorList) { @@ -132,3 +132,65 @@ func TestRemoteScopesRunOverAnUnpackedImage(t *testing.T) { }) } } + +// 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) +} From 0a2b2206abaeb2fd708e5104fa96329cf266076c Mon Sep 17 00:00:00 2001 From: Ruslan Gorbunov Date: Wed, 2 Sep 2026 10:06:08 +0300 Subject: [PATCH 03/14] fix: harden remote linting and bundle validation Signed-off-by: Ruslan Gorbunov --- internal/manager/statistics.go | 19 +++++-- internal/remotelint/extract.go | 68 ++++++++++++++++--------- internal/remotelint/lint.go | 8 +++ internal/remotelint/lint_test.go | 39 ++++++++++++++ pkg/linters/module/rules/layout.go | 9 +++- pkg/linters/module/rules/layout_test.go | 11 ++-- pkg/scopes/bundle.go | 5 +- pkg/scopes/scopes_test.go | 6 ++- 8 files changed, 126 insertions(+), 39 deletions(-) diff --git a/internal/manager/statistics.go b/internal/manager/statistics.go index c510bdd7d..ced751ebf 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,15 @@ 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())) + PrintStatistics(m.errors, len(m.Modules), time.Since(m.startedAt)) +} + +// PrintStatistics renders the summary for a finished error list. It is a package +// function for the same reason PrintResult is one: the remote-lint path collects +// its findings without a Manager, and a run that prints findings but no summary +// reads as a different tool. +func PrintStatistics(errorList *errors.LintRuleErrorsList, modules int, elapsed time.Duration) { + fmt.Println(renderStatistics(collectStatistics(errorList, modules, elapsed))) } // renderStatistics formats the statistics as a single multi-line, framed block. diff --git a/internal/remotelint/extract.go b/internal/remotelint/extract.go index e399850d8..bcdf59b89 100644 --- a/internal/remotelint/extract.go +++ b/internal/remotelint/extract.go @@ -52,6 +52,13 @@ func extractImage(ctx context.Context, image registry.Image) (string, error) { // 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 { @@ -68,56 +75,68 @@ func extract(ctx context.Context, rc io.ReadCloser, root string) error { return fmt.Errorf("read tar: %w", err) } - entryPath, err := safeJoin(root, hdr.Name) + name, err := safeName(hdr.Name) if err != nil { return err } switch hdr.Typeflag { case tar.TypeDir: - if err = os.MkdirAll(entryPath, os.FileMode(hdr.Mode)); err != nil { + 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(entryPath, tr, os.FileMode(hdr.Mode)); err != nil { + 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: - // 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(root, filepath.Dir(entryPath), hdr.Linkname) { + // 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 = os.Symlink(hdr.Linkname, entryPath); err != nil { + 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. - linkPath, err := safeJoin(root, hdr.Linkname) + linkName, err := safeName(hdr.Linkname) if err != nil { return err } - if err = os.Link(linkPath, entryPath); err != nil { + 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(target string, src io.Reader, mode os.FileMode) error { - if err := os.MkdirAll(filepath.Dir(target), 0o700); err != nil { +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 := os.OpenFile(target, os.O_CREATE|os.O_WRONLY|os.O_TRUNC, mode&0o700) + 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) } @@ -138,23 +157,22 @@ func writeRegularFile(target string, src io.Reader, mode os.FileMode) error { return nil } -// safeJoin joins name under root and rejects absolute paths or parent-directory escapes. -func safeJoin(root, name string) (string, error) { - if filepath.IsAbs(name) { - return "", fmt.Errorf("path %q escapes output directory", name) - } +// 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 !staysWithin(root, root, name) { + if filepath.IsAbs(clean) || !staysWithin(".", clean) { return "", fmt.Errorf("path %q escapes output directory", name) } - return filepath.Join(root, name), nil + return clean, nil } -// staysWithin reports whether name resolves under root when interpreted relative to base. -func staysWithin(root, base, name string) bool { +// 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)) - rel, err := filepath.Rel(root, target) - return err == nil && rel != ".." && !strings.HasPrefix(rel, ".."+string(filepath.Separator)) + return target != ".." && !strings.HasPrefix(target, ".."+string(filepath.Separator)) } diff --git a/internal/remotelint/lint.go b/internal/remotelint/lint.go index 2f9e2d7a6..8c82c587c 100644 --- a/internal/remotelint/lint.go +++ b/internal/remotelint/lint.go @@ -26,6 +26,7 @@ import ( "os" "path" "strings" + "time" "github.com/google/go-containerregistry/pkg/name" @@ -54,6 +55,11 @@ type Options struct { // Run lints the bundle and release images published under imagePath, e.g. // registry.example.com/my-module:v0.0.1. func Run(ctx context.Context, imagePath string, opts *Options) error { + // Elapsed covers the pulls as well as the linting: for a remote run the download + // is most of the wall clock, and a summary that hid it would report a number the + // caller cannot reconcile with what they waited for. + startedAt := time.Now() + repository, tag, err := cutTagFromImagePath(imagePath) if err != nil { return fmt.Errorf("failed to cut tag from image path: %w", err) @@ -85,6 +91,8 @@ func Run(ctx context.Context, imagePath string, opts *Options) error { ) manager.PrintResult(errorList) + // One module read from two images, so the summary counts one module, not two. + manager.PrintStatistics(errorList, 1, time.Since(startedAt)) if pullErr != nil { return pullErr diff --git a/internal/remotelint/lint_test.go b/internal/remotelint/lint_test.go index d0311682a..1a2394f74 100644 --- a/internal/remotelint/lint_test.go +++ b/internal/remotelint/lint_test.go @@ -20,6 +20,7 @@ import ( "archive/tar" "bytes" "context" + "errors" "io" "os" "path/filepath" @@ -84,6 +85,44 @@ func TestExtractRejectsEscapes(t *testing.T) { } } +// 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} } diff --git a/pkg/linters/module/rules/layout.go b/pkg/linters/module/rules/layout.go index 7bec27060..6436dc5cf 100644 --- a/pkg/linters/module/rules/layout.go +++ b/pkg/linters/module/rules/layout.go @@ -60,10 +60,15 @@ func NewReleaseLayoutRule(m pkg.Module, errorList *errors.LintRuleErrorsList) *L // 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. func NewBundleLayoutRule(m pkg.Module, errorList *errors.LintRuleErrorsList) *LayoutRule { return newLayoutRule(BundleLayoutRuleName, m, errorList, - []string{".gitignore", "changelog.yaml", "Chart.yaml", "images_digests.json", "version.json", "module.yaml"}, - []string{"docs", "templates", "charts"}, + []string{".helmignore", "Chart.yaml", "images_digests.json", "module.yaml"}, + // TODO(human): the directories every bundle must carry. + nil, ) } diff --git a/pkg/linters/module/rules/layout_test.go b/pkg/linters/module/rules/layout_test.go index 14da3b03e..9747b4e2d 100644 --- a/pkg/linters/module/rules/layout_test.go +++ b/pkg/linters/module/rules/layout_test.go @@ -31,8 +31,13 @@ import ( func TestLayoutRules(t *testing.T) { releaseFiles := []string{"module.yaml", "version.json", "changelog.yaml"} - bundleFiles := []string{".gitignore", "changelog.yaml", "Chart.yaml", "images_digests.json", "version.json", "module.yaml"} - bundleDirs := []string{"docs", "templates", "charts"} + // 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. registry.deckhouse.io/deckhouse/ce/modules/{sds-node-configurator, + // csi-nfs,commander-agent,observability} all carry exactly these, and differ only + // in the optional crds/, hooks/ and monitoring/. + 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) @@ -58,7 +63,7 @@ func TestLayoutRules(t *testing.T) { 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{"templates", "charts"}) + root := layoutAt(t, append(bundleFiles, "docs"), []string{"charts", "openapi", "templates"}) errs := checkLayout(t, NewBundleLayoutRule, root) diff --git a/pkg/scopes/bundle.go b/pkg/scopes/bundle.go index b2310447f..7ffc445c5 100644 --- a/pkg/scopes/bundle.go +++ b/pkg/scopes/bundle.go @@ -29,7 +29,9 @@ import ( // 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. +// 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 @@ -41,7 +43,6 @@ var bundleRules = map[string]set.Set{ ), docs.ID: set.New( docsrules.ReadmeRuleName, - docsrules.ChangelogRuleName, ), } diff --git a/pkg/scopes/scopes_test.go b/pkg/scopes/scopes_test.go index 8b1d1356c..483ca8365 100644 --- a/pkg/scopes/scopes_test.go +++ b/pkg/scopes/scopes_test.go @@ -89,8 +89,10 @@ func TestRemoteScopesRunOverAnUnpackedImage(t *testing.T) { {scope: Release, wantErr: true}, { scope: Bundle, - files: []string{".gitignore", "changelog.yaml", "Chart.yaml", "images_digests.json", "version.json", "module.yaml"}, - dirs: []string{"docs", "templates", "charts"}, + // 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}, } { From 36a47d08465d2757a4e8be120c56deac9fccb7f3 Mon Sep 17 00:00:00 2001 From: Ruslan Gorbunov Date: Wed, 2 Sep 2026 10:12:38 +0300 Subject: [PATCH 04/14] fix: require common bundle directories Signed-off-by: Ruslan Gorbunov --- pkg/linters/module/rules/layout.go | 7 +++++-- pkg/linters/module/rules/layout_test.go | 7 ++++--- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/pkg/linters/module/rules/layout.go b/pkg/linters/module/rules/layout.go index 6436dc5cf..aeaf67abb 100644 --- a/pkg/linters/module/rules/layout.go +++ b/pkg/linters/module/rules/layout.go @@ -64,11 +64,14 @@ func NewReleaseLayoutRule(m pkg.Module, errorList *errors.LintRuleErrorsList) *L // 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"}, - // TODO(human): the directories every bundle must carry. - nil, + []string{"charts", "docs", "openapi", "templates"}, ) } diff --git a/pkg/linters/module/rules/layout_test.go b/pkg/linters/module/rules/layout_test.go index 9747b4e2d..46b5b53ac 100644 --- a/pkg/linters/module/rules/layout_test.go +++ b/pkg/linters/module/rules/layout_test.go @@ -33,9 +33,10 @@ 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. registry.deckhouse.io/deckhouse/ce/modules/{sds-node-configurator, - // csi-nfs,commander-agent,observability} all carry exactly these, and differ only - // in the optional crds/, hooks/ and monitoring/. + // 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"} From 17abf61e5f0b7315a6e3394c53cbe3490aca3dc2 Mon Sep 17 00:00:00 2001 From: Ruslan Gorbunov Date: Wed, 2 Sep 2026 11:06:59 +0300 Subject: [PATCH 05/14] feat: add remote lint subcommand Signed-off-by: Ruslan Gorbunov --- README.md | 21 ++++++++++++++++++--- cmd/dmt/root.go | 37 ++++++++++++++++++++++--------------- internal/flags/flags.go | 14 +++++++++----- pkg/linters/README.md | 4 ++-- 4 files changed, 51 insertions(+), 25 deletions(-) diff --git a/README.md b/README.md index 5e460f89d..d74efa1d4 100644 --- a/README.md +++ b/README.md @@ -269,7 +269,7 @@ linters-settings: # dmt lint impact: error remote: - bundle: # dmt lint --remote : + bundle: # dmt lint remote : documentation: rules: changelog: @@ -326,8 +326,6 @@ dmt lint [directories...] [flags] ``` **Flags:** -- `--remote, -r`: Lint the published images instead of a directory, e.g. `registry.example.com/my-module:v0.0.1` -- `--login` / `--password`: Registry credentials for `--remote` (the Docker config is used when omitted) - `--values-file, -v`: Specify custom values file - `--linter`: Run specific linter only - `--hide-warnings`: Hide warning-level issues @@ -350,6 +348,23 @@ 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 (the Docker config is used when omitted) + +**Example:** +```bash +dmt lint remote registry.example.com/my-module:v0.0.1 +``` + #### Bootstrap Command ```bash diff --git a/cmd/dmt/root.go b/cmd/dmt/root.go index aecd91582..5f85d5654 100644 --- a/cmd/dmt/root.go +++ b/cmd/dmt/root.go @@ -149,7 +149,28 @@ 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 { + return remotelint.Run(cmd.Context(), args[0], &remotelint.Options{ + Login: flags.RemoteLogin, + Password: flags.RemotePassword, + }) + }, + } + 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{ @@ -271,20 +292,6 @@ func runTests(dir string, opts ...test.Option) error { } func lintCmdFunc(cmd *cobra.Command, args []string) { - if flags.Remote != "" { - opts := &remotelint.Options{ - Login: flags.RemoteLogin, - Password: flags.RemotePassword, - } - - if err := remotelint.Run(cmd.Context(), flags.Remote, opts); err != nil { - log.Error("Error running remote lint", log.Err(err)) - os.Exit(1) - } - - return - } - var dirs = args[0:] if len(dirs) == 0 { diff --git a/internal/flags/flags.go b/internal/flags/flags.go index a68e3d942..cd7b998d2 100644 --- a/internal/flags/flags.go +++ b/internal/flags/flags.go @@ -33,7 +33,6 @@ var ( ) var ( - Remote string RemoteLogin string RemotePassword string ) @@ -68,10 +67,6 @@ func InitDefaultFlagSet() *pflag.FlagSet { func InitLintFlagSet() *pflag.FlagSet { lint := pflag.NewFlagSet("lint", pflag.ContinueOnError) - lint.StringVarP(&Remote, "remote", "r", "", "lint the published images instead of a directory, e.g. 'registry.example.com/my-module:v0.0.1'") - lint.StringVar(&RemoteLogin, "login", "", "registry login, used with --remote") - lint.StringVar(&RemotePassword, "password", "", "registry password, used with --remote") - lint.IntVarP(&LintersLimit, "parallel", "p", numThreads, "number of threads for parallel processing") lint.StringVar(&LinterName, "linter", "", "linter name to run") @@ -116,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 (the Docker config is used when omitted)") + remote.StringVar(&RemotePassword, "password", "", "registry password (the Docker config is used when omitted)") + + return remote +} + func InitBootstrapFlagSet() *pflag.FlagSet { bootstrap := pflag.NewFlagSet("bootstrap", pflag.ContinueOnError) diff --git a/pkg/linters/README.md b/pkg/linters/README.md index 503e8bccf..656dadedb 100644 --- a/pkg/linters/README.md +++ b/pkg/linters/README.md @@ -37,10 +37,10 @@ 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 :` | +| `bundle` | the packaged module image, `:` | `dmt lint remote :` | | `release` | the release metadata image, `/release:` | the same command | -`--remote` runs both image scopes off one reference: it pulls each image, +`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, From 706075ff7dd4eef8f6ef3d031307c11c5e460be7 Mon Sep 17 00:00:00 2001 From: Ruslan Gorbunov Date: Wed, 2 Sep 2026 13:21:41 +0300 Subject: [PATCH 06/14] fix: report module paths without extraction directory Signed-off-by: Ruslan Gorbunov --- pkg/linters/module/rules/layout.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/linters/module/rules/layout.go b/pkg/linters/module/rules/layout.go index aeaf67abb..a81487e10 100644 --- a/pkg/linters/module/rules/layout.go +++ b/pkg/linters/module/rules/layout.go @@ -109,7 +109,7 @@ func (r *LayoutRule) check(root, name string, wantDir bool) { } path := filepath.Join(root, name) - errorList := r.errorList.WithFilePath(path) + errorList := r.errorList.WithFilePath(name) info, err := os.Stat(path) From 9e7e8f1ea8c9f39babcf6ba27e6de87c1479a6a9 Mon Sep 17 00:00:00 2001 From: Ruslan Gorbunov Date: Wed, 2 Sep 2026 16:21:33 +0300 Subject: [PATCH 07/14] fix: reject implicit registries and escaping symlinks Signed-off-by: Ruslan Gorbunov --- internal/remotelint/lint.go | 11 +++++- internal/remotelint/lint_test.go | 60 ++++++++++++++++++++++++++++++++ 2 files changed, 70 insertions(+), 1 deletion(-) diff --git a/internal/remotelint/lint.go b/internal/remotelint/lint.go index 8c82c587c..6ca2ac251 100644 --- a/internal/remotelint/lint.go +++ b/internal/remotelint/lint.go @@ -147,11 +147,20 @@ func cutTagFromImagePath(imagePath string) (string, string, error) { return "", "", fmt.Errorf("digest not supported") } - ref, err := name.ParseReference(imagePath, name.WithDefaultTag("")) + // 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") diff --git a/internal/remotelint/lint_test.go b/internal/remotelint/lint_test.go index 1a2394f74..088d63489 100644 --- a/internal/remotelint/lint_test.go +++ b/internal/remotelint/lint_test.go @@ -46,6 +46,18 @@ func TestCutTagFromImagePath(t *testing.T) { 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 @@ -123,6 +135,54 @@ func TestExtractReportsATruncatedStream(t *testing.T) { require.ErrorContains(t, extract(t.Context(), pr, t.TempDir()), "registry went away") } +// TestExtractRejectsLinksThatResolveOutside covers the escapes that reading the +// names as text cannot see. Both chains below place a link inside the root whose +// resolution leaves it, and os.Root catches neither: nothing is written through the +// link, so the guard has to reject it here — the linters read the extracted tree +// afterwards with plain os calls. +func TestExtractRejectsLinksThatResolveOutside(t *testing.T) { + // "escaping" is a directory beside the root holding the file a link must not + // reach; each chain is followed by the path that would read it. + for name, chain := range map[string]struct { + entries []*tar.Header + read string + }{ + // "sub/toroot" resolves to the root itself, so the second link is created + // beside the root instead of inside "sub". + "a link to the root's parent": { + entries: []*tar.Header{ + dirEntry("sub"), + symlinkEntry("sub/toroot", ".."), + symlinkEntry("sub/toroot/up", ".."), + }, + read: filepath.Join("up", "escaping", "secret.md"), + }, + // Clean("a/../x") is "x", so every check that cleans the target first calls + // this one local — while the kernel resolves "a" and only then the "..". + "a target whose .. is cancelled by a symlink": { + entries: []*tar.Header{ + symlinkEntry("a", "."), + symlinkEntry("b", "a/../escaping"), + }, + read: filepath.Join("b", "secret.md"), + }, + } { + t.Run(name, func(t *testing.T) { + outside := t.TempDir() + root := filepath.Join(outside, "root") + require.NoError(t, os.MkdirAll(filepath.Join(outside, "escaping"), 0o700)) + require.NoError(t, os.WriteFile(filepath.Join(outside, "escaping", "secret.md"), []byte("secret"), 0o600)) + require.NoError(t, os.Mkdir(root, 0o700)) + + require.ErrorContains(t, extract(t.Context(), tarball(t, chain.entries...), root), + "escapes output directory") + + _, err := os.ReadFile(filepath.Join(root, chain.read)) + require.Error(t, err, "a link planted inside the root must not resolve out of it") + }) + } +} + func dirEntry(name string) *tar.Header { return &tar.Header{Typeflag: tar.TypeDir, Name: name, Mode: 0o755} } From 6361c429248b697fb08425c5a57ad6b1b1174e0c Mon Sep 17 00:00:00 2001 From: Ruslan Gorbunov Date: Wed, 2 Sep 2026 16:44:27 +0300 Subject: [PATCH 08/14] fix: report relative paths for remote lint findings Signed-off-by: Ruslan Gorbunov --- internal/remotelint/lint_test.go | 48 -------------------------------- pkg/linters/docs/rules/readme.go | 13 +++++---- pkg/scopes/scopes_test.go | 10 +++++++ 3 files changed, 18 insertions(+), 53 deletions(-) diff --git a/internal/remotelint/lint_test.go b/internal/remotelint/lint_test.go index 088d63489..3ea4c3f3e 100644 --- a/internal/remotelint/lint_test.go +++ b/internal/remotelint/lint_test.go @@ -135,54 +135,6 @@ func TestExtractReportsATruncatedStream(t *testing.T) { require.ErrorContains(t, extract(t.Context(), pr, t.TempDir()), "registry went away") } -// TestExtractRejectsLinksThatResolveOutside covers the escapes that reading the -// names as text cannot see. Both chains below place a link inside the root whose -// resolution leaves it, and os.Root catches neither: nothing is written through the -// link, so the guard has to reject it here — the linters read the extracted tree -// afterwards with plain os calls. -func TestExtractRejectsLinksThatResolveOutside(t *testing.T) { - // "escaping" is a directory beside the root holding the file a link must not - // reach; each chain is followed by the path that would read it. - for name, chain := range map[string]struct { - entries []*tar.Header - read string - }{ - // "sub/toroot" resolves to the root itself, so the second link is created - // beside the root instead of inside "sub". - "a link to the root's parent": { - entries: []*tar.Header{ - dirEntry("sub"), - symlinkEntry("sub/toroot", ".."), - symlinkEntry("sub/toroot/up", ".."), - }, - read: filepath.Join("up", "escaping", "secret.md"), - }, - // Clean("a/../x") is "x", so every check that cleans the target first calls - // this one local — while the kernel resolves "a" and only then the "..". - "a target whose .. is cancelled by a symlink": { - entries: []*tar.Header{ - symlinkEntry("a", "."), - symlinkEntry("b", "a/../escaping"), - }, - read: filepath.Join("b", "secret.md"), - }, - } { - t.Run(name, func(t *testing.T) { - outside := t.TempDir() - root := filepath.Join(outside, "root") - require.NoError(t, os.MkdirAll(filepath.Join(outside, "escaping"), 0o700)) - require.NoError(t, os.WriteFile(filepath.Join(outside, "escaping", "secret.md"), []byte("secret"), 0o600)) - require.NoError(t, os.Mkdir(root, 0o700)) - - require.ErrorContains(t, extract(t.Context(), tarball(t, chain.entries...), root), - "escapes output directory") - - _, err := os.ReadFile(filepath.Join(root, chain.read)) - require.Error(t, err, "a link planted inside the root must not resolve out of it") - }) - } -} - func dirEntry(name string) *tar.Header { return &tar.Header{Typeflag: tar.TypeDir, Name: name, Mode: 0o755} } 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/scopes/scopes_test.go b/pkg/scopes/scopes_test.go index 483ca8365..a64eac9f1 100644 --- a/pkg/scopes/scopes_test.go +++ b/pkg/scopes/scopes_test.go @@ -131,6 +131,16 @@ func TestRemoteScopesRunOverAnUnpackedImage(t *testing.T) { } 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) + } }) } } From 8023316f86ee02e58f0a83252bacc2350b7da2eb Mon Sep 17 00:00:00 2001 From: Ruslan Gorbunov Date: Thu, 3 Sep 2026 08:35:03 +0300 Subject: [PATCH 09/14] feat: unify linting and support registry env auth Route static and remote sources through the manager for consistent summaries, metrics, and linter filters. Resolve credentials from flags, environment variables, Docker config, or anonymous access. Signed-off-by: Ruslan Gorbunov --- README.md | 19 +- cmd/dmt/main.go | 29 ++- cmd/dmt/root.go | 17 +- internal/flags/flags.go | 4 +- internal/manager/manager.go | 232 ++++++++++-------- internal/manager/manager_test.go | 184 ++++++++++++++ internal/manager/statistics.go | 10 +- internal/metrics/metrics.go | 37 ++- internal/metrics/metrics_test.go | 6 +- internal/remotelint/lint.go | 126 +++++----- internal/remotelint/registry.go | 19 +- internal/remotelint/registry_test.go | 79 ++++++ internal/staticlint/lint.go | 157 ++++++++++++ internal/{manager => staticlint}/validate.go | 27 +- .../{manager => staticlint}/validate_test.go | 9 +- test/e2e/framework.go | 14 +- 16 files changed, 753 insertions(+), 216 deletions(-) create mode 100644 internal/manager/manager_test.go create mode 100644 internal/remotelint/registry_test.go create mode 100644 internal/staticlint/lint.go rename internal/{manager => staticlint}/validate.go (75%) rename internal/{manager => staticlint}/validate_test.go (93%) diff --git a/README.md b/README.md index d74efa1d4..cebe7ea43 100644 --- a/README.md +++ b/README.md @@ -358,11 +358,28 @@ Lints the published images instead of a directory: the bundle at `:` and the release at `/release:`. **Flags:** -- `--login` / `--password`: Registry credentials (the Docker config is used when omitted) +- `--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 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 5f85d5654..6893f51e9 100644 --- a/cmd/dmt/root.go +++ b/cmd/dmt/root.go @@ -37,6 +37,7 @@ import ( "github.com/deckhouse/dmt/internal/fsutils" "github.com/deckhouse/dmt/internal/remotelint" "github.com/deckhouse/dmt/internal/rendercmd" + "github.com/deckhouse/dmt/internal/staticlint" "github.com/deckhouse/dmt/internal/test" "github.com/deckhouse/dmt/internal/version" "github.com/deckhouse/dmt/pkg/config" @@ -160,10 +161,22 @@ the config next to the caller.`, Args: cobra.ExactArgs(1), SilenceUsage: true, RunE: func(cmd *cobra.Command, args []string) error { - return remotelint.Run(cmd.Context(), args[0], &remotelint.Options{ + // --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 := remotelint.NewSource(args[0], &remotelint.Options{ Login: flags.RemoteLogin, Password: flags.RemotePassword, }) + if err != nil { + return err + } + + return runLint(cmd.Context(), src) }, } remoteCmd.Flags().AddFlagSet(flags.InitRemoteFlagSet()) @@ -318,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, staticlint.NewSource(expandedDir)); err != nil { log.Error("Error processing directory", slog.String("directory", expandedDir), log.Err(err)) hasErrors = true diff --git a/internal/flags/flags.go b/internal/flags/flags.go index cd7b998d2..21705c7fa 100644 --- a/internal/flags/flags.go +++ b/internal/flags/flags.go @@ -114,8 +114,8 @@ func InitLintFlagSet() *pflag.FlagSet { func InitRemoteFlagSet() *pflag.FlagSet { remote := pflag.NewFlagSet("remote", pflag.ContinueOnError) - remote.StringVar(&RemoteLogin, "login", "", "registry login (the Docker config is used when omitted)") - remote.StringVar(&RemotePassword, "password", "", "registry password (the Docker config is used when omitted)") + 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 } diff --git a/internal/manager/manager.go b/internal/manager/manager.go index e40d4e457..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,133 +57,172 @@ 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 - } +// 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 - vals, err := decodeValuesFile(flags.ValuesFile) - if err != nil { - log.Error("Failed to decode values file", log.Err(err)) - } + log.Info("Found modules", slog.Int("count", m.moduleCount())) - globalValues, err := values.GetGlobalValues(getRootDirectory(dir)) - if err != nil { - log.Error("Failed to get global values", log.Err(err)) - return m - } - - 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() { - PrintResult(m.errors) + printResult(m.errors) } -// PrintResult renders a finished error list. It is a package function rather than a -// method only so the remote-lint path, which builds its list without a Manager, prints -// exactly what a local run prints. -func PrintResult(errorList *errors.LintRuleErrorsList) { +// printResult renders a finished error list. +func printResult(errorList *errors.LintRuleErrorsList) { errs := errorList.GetErrors() if len(errs) == 0 { @@ -331,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 ced751ebf..5fe05a693 100644 --- a/internal/manager/statistics.go +++ b/internal/manager/statistics.go @@ -160,15 +160,7 @@ func collectStatistics(errorList *errors.LintRuleErrorsList, modules int, elapse // 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() { - PrintStatistics(m.errors, len(m.Modules), time.Since(m.startedAt)) -} - -// PrintStatistics renders the summary for a finished error list. It is a package -// function for the same reason PrintResult is one: the remote-lint path collects -// its findings without a Manager, and a run that prints findings but no summary -// reads as a different tool. -func PrintStatistics(errorList *errors.LintRuleErrorsList, modules int, elapsed time.Duration) { - fmt.Println(renderStatistics(collectStatistics(errorList, modules, elapsed))) + 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..bdd7ba713 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,32 @@ 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() + + // ponytail: a linter set to warn in both remote sections counts twice — dmt_linter_info + // carries no scope label. Add one if the metric is ever read as a count. + 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/remotelint/lint.go b/internal/remotelint/lint.go index 6ca2ac251..971cd0c58 100644 --- a/internal/remotelint/lint.go +++ b/internal/remotelint/lint.go @@ -26,16 +26,13 @@ import ( "os" "path" "strings" - "time" "github.com/google/go-containerregistry/pkg/name" "github.com/deckhouse/deckhouse/pkg/registry" "github.com/deckhouse/dmt/internal/manager" - "github.com/deckhouse/dmt/internal/metrics" "github.com/deckhouse/dmt/internal/modules" - "github.com/deckhouse/dmt/pkg" "github.com/deckhouse/dmt/pkg/config" "github.com/deckhouse/dmt/pkg/errors" "github.com/deckhouse/dmt/pkg/scopes" @@ -52,89 +49,102 @@ type Options struct { Password string } -// Run lints the bundle and release images published under imagePath, e.g. +// Source reads a module from the two images published under an image path, e.g. // registry.example.com/my-module:v0.0.1. -func Run(ctx context.Context, imagePath string, opts *Options) error { - // Elapsed covers the pulls as well as the linting: for a remote run the download - // is most of the wall clock, and a summary that hid it would report a number the - // caller cannot reconcile with what they waited for. - startedAt := time.Now() +type Source struct { + client registry.Client + tag string + moduleName string - repository, tag, err := cutTagFromImagePath(imagePath) - if err != nil { - return fmt.Errorf("failed to cut tag from image path: %w", err) - } + // dirs are the extraction directories, removed by Close. + dirs []string +} - // The image ships no .dmtlint.yaml, so severities come from the config next to - // the caller — the same file a local run would read, but from its `remote.bundle` - // and `remote.release` sections rather than the ones the source tree uses. - cfg, err := config.NewDefaultRootConfig(".") +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 fmt.Errorf("failed to parse default root config: %w", err) + return nil, fmt.Errorf("failed to cut tag from image path: %w", err) } - // init metrics storage, should be done before printing results, as PrintResult - // reports per-error metrics through the shared metrics client. - metrics.GetClient(".") + return &Source{ + client: newRegistryClient(repository, opts.Login, opts.Password), + tag: tag, + moduleName: path.Base(repository), + }, nil +} - client := newRegistryClient(repository, opts.Login, opts.Password) - moduleName := path.Base(repository) +// 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 "." +} - level := pkg.Error - errorList := errors.NewLintRuleErrorsList().WithMaxLevel(&level) +func (s *Source) Scopes() []scopes.Scope { + return []scopes.Scope{scopes.Bundle, scopes.Release} +} - // Both images are linted before anything is printed: a registry failure on one - // must not swallow what the other already found. - pullErr := stderrors.Join( - lintImage(ctx, client, tag, scopes.Bundle, moduleName, cfg, errorList), - lintImage(ctx, client.WithSegment(releaseSegment), tag, scopes.Release, moduleName, cfg, errorList), - ) +// 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) + } +} - manager.PrintResult(errorList) - // One module read from two images, so the summary counts one module, not two. - manager.PrintStatistics(errorList, 1, time.Since(startedAt)) +// 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) - if pullErr != nil { - return pullErr - } + targets := make([]manager.Target, 0, 2) - if errorList.ContainsErrors() { - return stderrors.New("critical errors found") + for _, t := range []*manager.Target{bundle, release} { + if t != nil { + targets = append(targets, *t) + } } - return nil + return targets, stderrors.Join(bundleErr, releaseErr) } -// lintImage pulls one image, unpacks it and runs the scope that belongs to it. Which -// linters that scope runs, and which of their rules, is the scope's business — this -// only supplies the unpacked module. -func lintImage( +// 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, - tag string, scope scopes.Scope, - moduleName string, cfg *config.RootConfig, - errorList *errors.LintRuleErrorsList, -) error { - image, err := client.GetImage(ctx, tag) +) (*manager.Target, error) { + image, err := client.GetImage(ctx, s.tag) if err != nil { - return fmt.Errorf("failed to get %s image: %w", scope, err) + return nil, fmt.Errorf("failed to get %s image: %w", scope, err) } dir, err := extractImage(ctx, image) if err != nil { - return fmt.Errorf("failed to extract %s image: %w", scope, err) + return nil, fmt.Errorf("failed to extract %s image: %w", scope, err) } - defer os.RemoveAll(dir) - m := modules.NewRemoteModule(dir, moduleName, scope.Settings(cfg)) - - for _, linter := range scope.Linters(m, errorList.WithObjectID(string(scope))) { - linter.Lint(ctx) - } + s.dirs = append(s.dirs, dir) - return nil + 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 diff --git a/internal/remotelint/registry.go b/internal/remotelint/registry.go index 10e26590e..ed2bb40c0 100644 --- a/internal/remotelint/registry.go +++ b/internal/remotelint/registry.go @@ -17,7 +17,9 @@ limitations under the License. package remotelint import ( + "cmp" "log/slog" + "os" "github.com/google/go-containerregistry/pkg/authn" "github.com/google/go-containerregistry/pkg/name" @@ -26,13 +28,26 @@ import ( 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: explicit login/password -// first, then the Docker config, then anonymous. +// 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, diff --git a/internal/remotelint/registry_test.go b/internal/remotelint/registry_test.go new file mode 100644 index 000000000..e8ec02ae3 --- /dev/null +++ b/internal/remotelint/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 remotelint + +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/staticlint/lint.go b/internal/staticlint/lint.go new file mode 100644 index 000000000..05eff28e5 --- /dev/null +++ b/internal/staticlint/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 staticlint reads the modules to lint from a working tree: the full source +// as committed, rendered the way Deckhouse would render it. +package staticlint + +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/staticlint/validate.go similarity index 75% rename from internal/manager/validate.go rename to internal/staticlint/validate.go index 3cc9398c3..dbfff52d3 100644 --- a/internal/manager/validate.go +++ b/internal/staticlint/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 staticlint 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/staticlint/validate_test.go similarity index 93% rename from internal/manager/validate_test.go rename to internal/staticlint/validate_test.go index 80de2d0d2..c6d1e1fb3 100644 --- a/internal/manager/validate_test.go +++ b/internal/staticlint/validate_test.go @@ -1,4 +1,4 @@ -package manager +package staticlint 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/test/e2e/framework.go b/test/e2e/framework.go index 31b87855a..edec2543c 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/staticlint" "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, staticlint.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, staticlint.NewSource(target)) + defer mng.Close() + + _ = mng.Run(context.Background()) mng.ApplyFixes() From a0b771e2e7409595dc8f82e4f34ada57e145ab2d Mon Sep 17 00:00:00 2001 From: Ruslan Gorbunov Date: Thu, 3 Sep 2026 09:01:22 +0300 Subject: [PATCH 10/14] chore: reorganize lint sources packages Signed-off-by: Ruslan Gorbunov --- cmd/dmt/root.go | 8 ++++---- internal/{remotelint => sources/remote}/extract.go | 2 +- internal/{remotelint => sources/remote}/lint.go | 4 ++-- internal/{remotelint => sources/remote}/lint_test.go | 2 +- internal/{remotelint => sources/remote}/registry.go | 2 +- internal/{remotelint => sources/remote}/registry_test.go | 2 +- internal/{staticlint => sources/static}/lint.go | 4 ++-- internal/{staticlint => sources/static}/validate.go | 2 +- internal/{staticlint => sources/static}/validate_test.go | 2 +- pkg/linters/README.md | 6 +----- test/e2e/framework.go | 6 +++--- 11 files changed, 18 insertions(+), 22 deletions(-) rename internal/{remotelint => sources/remote}/extract.go (99%) rename internal/{remotelint => sources/remote}/lint.go (98%) rename internal/{remotelint => sources/remote}/lint_test.go (99%) rename internal/{remotelint => sources/remote}/registry.go (99%) rename internal/{remotelint => sources/remote}/registry_test.go (99%) rename internal/{staticlint => sources/static}/lint.go (97%) rename internal/{staticlint => sources/static}/validate.go (99%) rename internal/{staticlint => sources/static}/validate_test.go (99%) diff --git a/cmd/dmt/root.go b/cmd/dmt/root.go index 6893f51e9..661248d2e 100644 --- a/cmd/dmt/root.go +++ b/cmd/dmt/root.go @@ -35,9 +35,9 @@ import ( "github.com/deckhouse/dmt/internal/bootstrap" "github.com/deckhouse/dmt/internal/flags" "github.com/deckhouse/dmt/internal/fsutils" - "github.com/deckhouse/dmt/internal/remotelint" + "github.com/deckhouse/dmt/internal/sources/remote" "github.com/deckhouse/dmt/internal/rendercmd" - "github.com/deckhouse/dmt/internal/staticlint" + "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" @@ -168,7 +168,7 @@ the config next to the caller.`, return errors.New("--fix is not supported for remote lint") } - src, err := remotelint.NewSource(args[0], &remotelint.Options{ + src, err := remote.NewSource(args[0], &remote.Options{ Login: flags.RemoteLogin, Password: flags.RemotePassword, }) @@ -331,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, staticlint.NewSource(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/internal/remotelint/extract.go b/internal/sources/remote/extract.go similarity index 99% rename from internal/remotelint/extract.go rename to internal/sources/remote/extract.go index bcdf59b89..bc6a28dcd 100644 --- a/internal/remotelint/extract.go +++ b/internal/sources/remote/extract.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package remotelint +package remote import ( "archive/tar" diff --git a/internal/remotelint/lint.go b/internal/sources/remote/lint.go similarity index 98% rename from internal/remotelint/lint.go rename to internal/sources/remote/lint.go index 971cd0c58..70cd349bf 100644 --- a/internal/remotelint/lint.go +++ b/internal/sources/remote/lint.go @@ -14,10 +14,10 @@ See the License for the specific language governing permissions and limitations under the License. */ -// Package remotelint lints a module as it was published, rather than as it sits in a +// 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 remotelint +package remote import ( "context" diff --git a/internal/remotelint/lint_test.go b/internal/sources/remote/lint_test.go similarity index 99% rename from internal/remotelint/lint_test.go rename to internal/sources/remote/lint_test.go index 3ea4c3f3e..5fc037bbf 100644 --- a/internal/remotelint/lint_test.go +++ b/internal/sources/remote/lint_test.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package remotelint +package remote import ( "archive/tar" diff --git a/internal/remotelint/registry.go b/internal/sources/remote/registry.go similarity index 99% rename from internal/remotelint/registry.go rename to internal/sources/remote/registry.go index ed2bb40c0..4a47d9837 100644 --- a/internal/remotelint/registry.go +++ b/internal/sources/remote/registry.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package remotelint +package remote import ( "cmp" diff --git a/internal/remotelint/registry_test.go b/internal/sources/remote/registry_test.go similarity index 99% rename from internal/remotelint/registry_test.go rename to internal/sources/remote/registry_test.go index e8ec02ae3..21e0f102b 100644 --- a/internal/remotelint/registry_test.go +++ b/internal/sources/remote/registry_test.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package remotelint +package remote import ( "testing" diff --git a/internal/staticlint/lint.go b/internal/sources/static/lint.go similarity index 97% rename from internal/staticlint/lint.go rename to internal/sources/static/lint.go index 05eff28e5..45865e400 100644 --- a/internal/staticlint/lint.go +++ b/internal/sources/static/lint.go @@ -14,9 +14,9 @@ See the License for the specific language governing permissions and limitations under the License. */ -// Package staticlint reads the modules to lint from a working tree: the full source +// Package static reads the modules to lint from a working tree: the full source // as committed, rendered the way Deckhouse would render it. -package staticlint +package static import ( "context" diff --git a/internal/staticlint/validate.go b/internal/sources/static/validate.go similarity index 99% rename from internal/staticlint/validate.go rename to internal/sources/static/validate.go index dbfff52d3..bf38dbb8b 100644 --- a/internal/staticlint/validate.go +++ b/internal/sources/static/validate.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package staticlint +package static import ( "errors" diff --git a/internal/staticlint/validate_test.go b/internal/sources/static/validate_test.go similarity index 99% rename from internal/staticlint/validate_test.go rename to internal/sources/static/validate_test.go index c6d1e1fb3..e2d0f11d1 100644 --- a/internal/staticlint/validate_test.go +++ b/internal/sources/static/validate_test.go @@ -1,4 +1,4 @@ -package staticlint +package static import ( "os" diff --git a/pkg/linters/README.md b/pkg/linters/README.md index 656dadedb..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 @@ -89,7 +85,7 @@ Membership is code, severity is config, and each scope reads its own section of | `bundle` | `remote.bundle` | | `release` | `remote.release` | -`Scope.Settings` is the only place that mapping lives, and `remotelint` hands the +`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 diff --git a/test/e2e/framework.go b/test/e2e/framework.go index edec2543c..7dbd67676 100644 --- a/test/e2e/framework.go +++ b/test/e2e/framework.go @@ -48,7 +48,7 @@ import ( "github.com/deckhouse/dmt/internal/flags" "github.com/deckhouse/dmt/internal/manager" "github.com/deckhouse/dmt/internal/metrics" - "github.com/deckhouse/dmt/internal/staticlint" + "github.com/deckhouse/dmt/internal/sources/static" "github.com/deckhouse/dmt/internal/test" "github.com/deckhouse/dmt/pkg" "github.com/deckhouse/dmt/pkg/config" @@ -191,7 +191,7 @@ func Lint(moduleDir string) ([]pkg.LinterError, error) { // Initialize the metrics client so linters that emit metrics don't panic. metrics.GetClient(target) - mng := manager.New(cfg, staticlint.NewSource(target)) + mng := manager.New(cfg, static.NewSource(target)) defer mng.Close() _ = mng.Run(context.Background()) @@ -224,7 +224,7 @@ func RunFix(moduleDir string) ([]pkg.LinterError, error) { metrics.GetClient(target) - mng := manager.New(cfg, staticlint.NewSource(target)) + mng := manager.New(cfg, static.NewSource(target)) defer mng.Close() _ = mng.Run(context.Background()) From 7b93997ff2035644c433db424944c980654f35b5 Mon Sep 17 00:00:00 2001 From: Ruslan Gorbunov Date: Thu, 3 Sep 2026 09:31:39 +0300 Subject: [PATCH 11/14] chore: reorder imports in root command Signed-off-by: Ruslan Gorbunov --- cmd/dmt/root.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/dmt/root.go b/cmd/dmt/root.go index 661248d2e..6d820a384 100644 --- a/cmd/dmt/root.go +++ b/cmd/dmt/root.go @@ -35,8 +35,8 @@ import ( "github.com/deckhouse/dmt/internal/bootstrap" "github.com/deckhouse/dmt/internal/flags" "github.com/deckhouse/dmt/internal/fsutils" - "github.com/deckhouse/dmt/internal/sources/remote" "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" From ed56358bc92bcd5c332f2b2db93fef7b631b3808 Mon Sep 17 00:00:00 2001 From: Ruslan Gorbunov Date: Thu, 3 Sep 2026 11:22:02 +0300 Subject: [PATCH 12/14] fix: require module.yaml to identify modules Signed-off-by: Ruslan Gorbunov --- internal/moduleloader/loader.go | 15 +++------------ 1 file changed, 3 insertions(+), 12 deletions(-) diff --git a/internal/moduleloader/loader.go b/internal/moduleloader/loader.go index a1d203d75..44c65b347 100644 --- a/internal/moduleloader/loader.go +++ b/internal/moduleloader/loader.go @@ -23,11 +23,7 @@ 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). @@ -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) } From 8b3c927e32c90616ed89a0cc03274c99bdc0f65a Mon Sep 17 00:00:00 2001 From: Ruslan Gorbunov Date: Thu, 3 Sep 2026 11:29:59 +0300 Subject: [PATCH 13/14] chore: clarify module path documentation Signed-off-by: Ruslan Gorbunov --- internal/metrics/metrics.go | 2 -- internal/moduleloader/loader.go | 2 +- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/internal/metrics/metrics.go b/internal/metrics/metrics.go index bdd7ba713..8875833b6 100644 --- a/internal/metrics/metrics.go +++ b/internal/metrics/metrics.go @@ -155,8 +155,6 @@ func Flush(ctx context.Context, sections ...*global.Linters) { SetDmtInfo() - // ponytail: a linter set to warn in both remote sections counts twice — dmt_linter_info - // carries no scope label. Add one if the metric is ever read as a count. for _, s := range sections { SetLinterWarningsMetrics(s) } diff --git a/internal/moduleloader/loader.go b/internal/moduleloader/loader.go index 44c65b347..5b746b173 100644 --- a/internal/moduleloader/loader.go +++ b/internal/moduleloader/loader.go @@ -26,7 +26,7 @@ const ( 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) From 651fad596e1a9340f5bd3e8ac428b0f7678f1966 Mon Sep 17 00:00:00 2001 From: Ruslan Gorbunov Date: Thu, 3 Sep 2026 11:40:21 +0300 Subject: [PATCH 14/14] chore: rename conversion test module manifests Signed-off-by: Ruslan Gorbunov --- .../conversions/failing/module/{Chart.yaml => module.yaml} | 0 .../conversions/passing/module/{Chart.yaml => module.yaml} | 0 .../version-mismatch/module/{Chart.yaml => module.yaml} | 0 3 files changed, 0 insertions(+), 0 deletions(-) rename test/e2e/testdata/conversions/failing/module/{Chart.yaml => module.yaml} (100%) rename test/e2e/testdata/conversions/passing/module/{Chart.yaml => module.yaml} (100%) rename test/e2e/testdata/conversions/version-mismatch/module/{Chart.yaml => module.yaml} (100%) 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