From 4374e595a2a5960b18f8cf6b03e722362aa688fa Mon Sep 17 00:00:00 2001 From: Julian Blacher Date: Sun, 18 Oct 2026 02:36:54 -0300 Subject: [PATCH] Lint several Dockerfiles per run and read a .docker-lint.yaml config Paths may now be files, directories searched recursively for Dockerfiles, or glob patterns with **, expanded by docker-lint so they behave the same on Windows. A directory or pattern that matches nothing is fatal, so CI cannot pass without linting anything; a file that fails to parse is reported while the others are still linted. A single file keeps its JSON object output; several files produce an array, and SARIF a single run covering all of them. .docker-lint.yaml, found up to the repository root, sets ignored rules, per-rule severities (or off), strict, quiet and exclude patterns. Flags win over the file, and unknown keys are errors so typos fail loudly. The GitHub Action takes one path or pattern per line and a config input. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/action.yml | 23 +++ CHANGELOG.md | 9 + README.md | 71 +++++++- action.yml | 38 +++- cmd/docker-lint/main.go | 220 ++++++++++++++++++----- cmd/docker-lint/multi_test.go | 288 +++++++++++++++++++++++++++++++ go.mod | 1 + go.sum | 3 + internal/analyzer/analyzer.go | 8 + internal/config/config.go | 166 ++++++++++++++++++ internal/config/config_test.go | 140 +++++++++++++++ internal/files/files.go | 247 ++++++++++++++++++++++++++ internal/files/files_test.go | 197 +++++++++++++++++++++ internal/formatter/json.go | 27 ++- internal/formatter/report.go | 36 ++++ internal/formatter/sarif.go | 56 +++--- testdata/config/no-from-off.yaml | 4 + 17 files changed, 1446 insertions(+), 88 deletions(-) create mode 100644 cmd/docker-lint/multi_test.go create mode 100644 internal/config/config.go create mode 100644 internal/config/config_test.go create mode 100644 internal/files/files.go create mode 100644 internal/files/files_test.go create mode 100644 internal/formatter/report.go create mode 100644 testdata/config/no-from-off.yaml diff --git a/.github/workflows/action.yml b/.github/workflows/action.yml index ccf4174..99a645e 100644 --- a/.github/workflows/action.yml +++ b/.github/workflows/action.yml @@ -88,3 +88,26 @@ jobs: # No empty report is left behind for an upload step to choke on. test -z "$SARIF_FILE" test ! -e fatal.sarif + + - name: Lint several Dockerfiles with a configuration + id: multi + uses: ./ + with: + version: source + dockerfile: | + testdata/**/minimal.Dockerfile + testdata/invalid/no-from.Dockerfile + config: testdata/config/no-from-off.yaml + sarif-file: multi.sarif + + - name: Check the multi-file run + shell: bash + env: + EXIT_CODE: ${{ steps.multi.outputs.exit-code }} + run: | + set -euo pipefail + test "$EXIT_CODE" = 0 + # Both files are in one report, and the disabled rule is gone. + grep -q '"uri": "testdata/valid/minimal.Dockerfile"' multi.sarif + grep -q '"uri": "testdata/invalid/no-from.Dockerfile"' multi.sarif + ! grep -q '"ruleId": "DKL001"' multi.sarif diff --git a/CHANGELOG.md b/CHANGELOG.md index e691c14..4b8dc63 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -38,6 +38,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - GitHub Action (`uses: devblac/docker-lint@`) that installs a checksum-verified release binary, annotates pull requests and can upload SARIF to code scanning; tested on Linux, macOS and Windows runners +- Lint several Dockerfiles in one run: pass several paths, a directory (searched + recursively for Dockerfiles) or a glob pattern with `**`; JSON output becomes + an array and SARIF a single run covering every file +- `.docker-lint.yaml` configuration file, found in the current directory or a + parent up to the repository root: ignored rules, per-rule severity overrides + (including `off`), `strict`, `quiet` and `exclude` patterns; `--config` and + `--no-config` flags +- The GitHub Action lints several paths or patterns (one per line) and takes a + `config` input ### Changed - **Breaking:** rule IDs move from `DLxxxx`, which collide with hadolint's IDs for diff --git a/README.md b/README.md index ca262c0..fef60ab 100644 --- a/README.md +++ b/README.md @@ -54,9 +54,15 @@ go build -o docker-lint ./cmd/docker-lint ## Usage ```bash -docker-lint [flags] [file] +docker-lint [flags] [path ...] ``` +Each path is a Dockerfile, a directory, or a glob pattern. A directory is searched recursively for files named +`Dockerfile`, `Containerfile`, `Dockerfile.*` or `*.Dockerfile` (skipping `.git` and `node_modules`). Patterns +support `**` for any number of directories and are expanded by docker-lint itself, so quote them to make them +work the same in every shell, including on Windows. A directory or pattern that matches no Dockerfile is a fatal +error, so a CI job cannot pass without linting anything. With no path, docker-lint reads from stdin. + ### Flags | Flag | Short | Description | @@ -68,6 +74,8 @@ docker-lint [flags] [file] | `--quiet` | `-q` | Suppress informational messages (show only warnings and errors) | | `--strict` | `-s` | Treat warnings as errors (exit code 1 if any warnings) | | `--ignore ` | | Comma-separated list of rule IDs to ignore | +| `--config ` | `-c` | Configuration file (default: the nearest `.docker-lint.yaml`) | +| `--no-config` | | Do not read a configuration file | | `--rules` | | List all available rules with descriptions | ### Examples @@ -76,6 +84,12 @@ docker-lint [flags] [file] # Analyze a Dockerfile docker-lint Dockerfile +# Analyze every Dockerfile in a repository +docker-lint . + +# Analyze Dockerfiles matching a pattern +docker-lint 'services/**/Dockerfile' + # Analyze from stdin cat Dockerfile | docker-lint @@ -107,13 +121,48 @@ FROM ubuntu:latest # No USER instruction needed for this build stage ``` +### Configuration File + +docker-lint reads `.docker-lint.yaml` (or `.docker-lint.yml`) from the current directory, or from the nearest +parent directory up to the repository root. Use `--config` to pick another file and `--no-config` to read none. + +```yaml +# Rules that are never reported. +ignore: + - DKL103 + +# Change a rule's severity: error, warning, info, or off to disable it. +severity: + DKL303: error # fail the build when the final image runs as root + DKL404: off # disable the rule + +# Fail on warnings too (same as --strict). +strict: true + +# Hide informational findings (same as --quiet). +quiet: false + +# Skip matching files when a directory or pattern is expanded. A pattern without +# a slash matches a file or directory name anywhere, as in .gitignore. +exclude: + - vendor + - "examples/**" +``` + +Command-line flags win over the file: `--strict=false` turns off `strict: true`, and `--ignore` adds to the +ignored rules. Unknown keys and invalid severities are fatal errors, so a typo does not silently disable the +configuration. A file named explicitly on the command line is always linted, even if it matches `exclude`. + ## Exit Codes | Code | Meaning | |------|---------| | 0 | Success - no errors found (warnings allowed in non-strict mode) | | 1 | Lint errors found, or warnings found in strict mode | -| 2 | Fatal error - file not found, permission denied, or parse error | +| 2 | Fatal error - invalid arguments or configuration, file not found, or parse error | + +When several files are linted, one that cannot be read or parsed is reported and the others are still linted; +the exit code is then 2. ## Rules @@ -182,7 +231,8 @@ Dockerfile:3:1: [info] DKL202: Found 2 consecutive RUN instructions that could b ### JSON (`--format json`) -Machine-readable JSON output for CI/CD integration: +Machine-readable JSON output for CI/CD integration. A single file (or stdin) produces one object; several files +produce an array with one such object per file: ```json { @@ -228,8 +278,8 @@ The repository's CI workflow runs `go test ./... -cover`. Coverage uploads to Co ### GitHub Actions -The docker-lint action installs a release binary (checksum-verified), lints a Dockerfile and annotates the -pull request diff with the findings: +The docker-lint action installs a release binary (checksum-verified), lints your Dockerfiles and annotates +the pull request diff with the findings: ```yaml - uses: actions/checkout@v7 @@ -250,7 +300,9 @@ steps: - uses: actions/checkout@v7 - uses: devblac/docker-lint@v0.1.0 with: - dockerfile: services/api/Dockerfile + dockerfile: | + Dockerfile + services/**/Dockerfile strict: true ignore: DKL404 upload-sarif: true @@ -258,7 +310,8 @@ steps: | Input | Default | Description | |-------|---------|-------------| -| `dockerfile` | `Dockerfile` | Path to the Dockerfile, relative to the repository root | +| `dockerfile` | `Dockerfile` | Files, directories or glob patterns to lint, one per line | +| `config` | | Configuration file (default: `.docker-lint.yaml` at the repository root) | | `version` | `latest` | docker-lint release to install, e.g. `v0.1.0` | | `format` | `github` | Log output: `github` (annotations), `text`, `json` or `sarif` | | `strict` | `false` | Fail on warnings, not only on errors | @@ -268,8 +321,8 @@ steps: | `upload-sarif` | `false` | Upload the SARIF report to GitHub code scanning | The action fails the step with docker-lint's exit code and exposes it as the `exit-code` output. A SARIF -report is still written and uploaded when the lint fails; a Dockerfile that cannot be parsed produces no -report rather than an empty one. +report is still written and uploaded when the lint fails, and covers every Dockerfile that could be parsed; +when none could, no report is written rather than an empty one. ### GitLab CI diff --git a/action.yml b/action.yml index 710dca0..6733e09 100644 --- a/action.yml +++ b/action.yml @@ -7,8 +7,16 @@ branding: inputs: dockerfile: - description: Path to the Dockerfile to lint, relative to the repository root. + description: >- + What to lint, one entry per line: a Dockerfile, a directory searched + recursively for Dockerfiles, or a glob such as "services/**/Dockerfile". + Paths are relative to the repository root. default: Dockerfile + config: + description: >- + Configuration file. Defaults to the .docker-lint.yaml at the repository + root, when there is one. + default: "" version: description: >- docker-lint release to use: "latest" or a tag such as "v0.1.0". "source" @@ -124,6 +132,7 @@ runs: shell: bash env: DOCKERFILE: ${{ inputs.dockerfile }} + CONFIG: ${{ inputs.config }} FORMAT: ${{ inputs.format }} STRICT: ${{ inputs.strict }} QUIET: ${{ inputs.quiet }} @@ -138,25 +147,40 @@ runs: if [ "$STRICT" = true ]; then args+=(--strict); fi if [ "$QUIET" = true ]; then args+=(--quiet); fi if [ -n "$IGNORE" ]; then args+=(--ignore "$IGNORE"); fi + if [ -n "$CONFIG" ]; then args+=(--config "$CONFIG"); fi + + # One path or pattern per line. Patterns stay quoted: docker-lint + # expands them itself, including ** on every runner OS. + paths=() + while IFS= read -r entry; do + entry="${entry#"${entry%%[![:space:]]*}"}" + entry="${entry%"${entry##*[![:space:]]}"}" + if [ -n "$entry" ]; then paths+=("$entry"); fi + done <<< "$DOCKERFILE" + if [ ${#paths[@]} -eq 0 ]; then + echo "::error::docker-lint: the dockerfile input is empty" + echo "exit-code=2" >> "$GITHUB_OUTPUT" + exit 0 + fi # Record the exit code instead of failing here, so the SARIF report is # still written and uploaded when the lint finds errors. code=0 - docker-lint ${args[@]+"${args[@]}"} --format "$FORMAT" "$DOCKERFILE" || code=$? + docker-lint ${args[@]+"${args[@]}"} --format "$FORMAT" "${paths[@]}" || code=$? echo "exit-code=$code" >> "$GITHUB_OUTPUT" if [ "$UPLOAD_SARIF" = true ] && [ -z "$SARIF_FILE" ]; then SARIF_FILE=docker-lint.sarif fi if [ -n "$SARIF_FILE" ]; then - # A fatal error (exit 2) writes nothing to stdout; an empty report - # would only make the upload fail with a confusing message. - docker-lint ${args[@]+"${args[@]}"} --format sarif "$DOCKERFILE" > "$SARIF_FILE" 2>/dev/null || true + # When no file can be analyzed nothing is written to stdout; an empty + # report would only make the upload fail with a confusing message. + docker-lint ${args[@]+"${args[@]}"} --format sarif "${paths[@]}" > "$SARIF_FILE" 2>/dev/null || true if [ -s "$SARIF_FILE" ]; then echo "sarif-file=$SARIF_FILE" >> "$GITHUB_OUTPUT" else rm -f "$SARIF_FILE" - echo "::warning::docker-lint: no SARIF report was written because the Dockerfile could not be analyzed" + echo "::warning::docker-lint: no SARIF report was written because no Dockerfile could be analyzed" fi fi @@ -175,5 +199,5 @@ runs: case "$EXIT_CODE" in 0) ;; 1) echo "::error::docker-lint found errors (or warnings with strict enabled)"; exit 1 ;; - *) echo "::error::docker-lint could not analyze the Dockerfile (exit code $EXIT_CODE)"; exit "$EXIT_CODE" ;; + *) echo "::error::docker-lint could not analyze every Dockerfile (exit code $EXIT_CODE)"; exit "$EXIT_CODE" ;; esac diff --git a/cmd/docker-lint/main.go b/cmd/docker-lint/main.go index 55bf64f..51b3134 100644 --- a/cmd/docker-lint/main.go +++ b/cmd/docker-lint/main.go @@ -11,6 +11,8 @@ import ( "github.com/devblac/docker-lint/internal/analyzer" "github.com/devblac/docker-lint/internal/ast" + "github.com/devblac/docker-lint/internal/config" + "github.com/devblac/docker-lint/internal/files" "github.com/devblac/docker-lint/internal/formatter" "github.com/devblac/docker-lint/internal/parser" "github.com/devblac/docker-lint/internal/rules" @@ -43,8 +45,10 @@ func run(args []string, stdin io.Reader, stdout, stderr io.Writer) int { strict bool versionFlg bool rulesFlag bool + noConfig bool ignoreCSV string formatFlag string + configPath string ) fs := flag.NewFlagSet("docker-lint", flag.ContinueOnError) @@ -60,9 +64,12 @@ func run(args []string, stdin io.Reader, stdout, stderr io.Writer) int { fs.BoolVar(&versionFlg, "version", false, "") fs.BoolVar(&versionFlg, "v", false, "") fs.BoolVar(&rulesFlag, "rules", false, "") + fs.BoolVar(&noConfig, "no-config", false, "") fs.StringVar(&ignoreCSV, "ignore", "", "") fs.StringVar(&formatFlag, "format", "", "") fs.StringVar(&formatFlag, "f", "", "") + fs.StringVar(&configPath, "config", "", "") + fs.StringVar(&configPath, "c", "", "") if err := fs.Parse(args); err != nil { if errors.Is(err, flag.ErrHelp) { @@ -90,47 +97,134 @@ func run(args []string, stdin io.Reader, stdout, stderr io.Writer) int { return ExitFatal } - positional := fs.Args() - if len(positional) > 1 { - fmt.Fprintln(stderr, "Error: too many arguments: only one Dockerfile path is supported") + if noConfig && configPath != "" { + fmt.Fprintln(stderr, "Error: --config conflicts with --no-config") + return ExitFatal + } + cfg, cfgPath, err := loadConfig(configPath, noConfig) + if err != nil { + fmt.Fprintf(stderr, "Error: invalid configuration: %v\n", err) return ExitFatal } - filename := "stdin" - reader := stdin - if reader == nil { - reader = os.Stdin + // Flags given on the command line win over the configuration file. + explicit := make(map[string]bool) + fs.Visit(func(f *flag.Flag) { explicit[f.Name] = true }) + if !explicit["strict"] && !explicit["s"] && cfg.Strict != nil { + strict = *cfg.Strict + } + if !explicit["quiet"] && !explicit["q"] && cfg.Quiet != nil { + quiet = *cfg.Quiet + } + + cliIgnores := parseIgnoreList(ignoreCSV) + warnUnknownRules(cliIgnores, "--ignore", stderr) + if cfgPath != "" { + warnUnknownRules(cfg.RuleIDs(), cfgPath, stderr) + } + + var ignoreRules []string + ignoreRules = append(ignoreRules, cfg.Ignore...) + ignoreRules = append(ignoreRules, cfg.DisabledRules()...) + ignoreRules = append(ignoreRules, cliIgnores...) + + anlzr := analyzer.NewWithDefaults(analyzer.Config{ + IgnoreRules: ignoreRules, + SeverityOverrides: cfg.SeverityOverrides(), + }) + + reports, single, fatal := lintInputs(fs.Args(), cfg.Exclude, stdin, anlzr, stderr) + + out := output{format: outputFormat, quiet: quiet, tool: toolInfo(cfg)} + if formatErr := out.write(reports, single, stdout); formatErr != nil { + fmt.Fprintf(stderr, "Error: failed to format output: %v\n", formatErr) + return ExitFatal } - if len(positional) == 1 { - filename = positional[0] - file, err := os.Open(filename) + if fatal { + return ExitFatal + } + var all []ast.Finding + for _, r := range reports { + all = append(all, r.Findings...) + } + return determineExitCode(all, strict) +} + +// loadConfig returns the configuration to use: the file named by --config, +// none with --no-config, or else the nearest .docker-lint.yaml. It also +// returns the path the configuration came from, "" when there is none. +func loadConfig(path string, disabled bool) (*config.Config, string, error) { + if disabled { + return &config.Config{}, "", nil + } + if path == "" { + found, err := config.Find(".") + if err != nil || found == "" { + return &config.Config{}, "", err + } + path = found + } + cfg, err := config.Load(path) + if err != nil { + return nil, "", err + } + return cfg, path, nil +} + +// lintInputs lints stdin when no path is given, or else every Dockerfile the +// paths expand to. A file that cannot be read or parsed is reported on stderr +// and skipped, and fatal is set; the other files are still linted. single is +// true when the input is stdin or one file named directly. +func lintInputs(paths, exclude []string, stdin io.Reader, anlzr *analyzer.Analyzer, stderr io.Writer) (reports []formatter.Report, single, fatal bool) { + if len(paths) == 0 { + if stdin == nil { + stdin = os.Stdin + } + dockerfile, err := parser.ParseReader(stdin) if err != nil { - fmt.Fprintf(stderr, "Error: failed to open file: %v\n", err) - return ExitFatal + fmt.Fprintf(stderr, "Error: failed to parse Dockerfile: %v\n", err) + return nil, true, true } - defer file.Close() - reader = file + return []formatter.Report{{Filename: "stdin", Findings: anlzr.Analyze(dockerfile)}}, true, false } - dockerfile, err := parser.ParseReader(reader) + expanded, err := files.Expand(paths, exclude) if err != nil { - fmt.Fprintf(stderr, "Error: failed to parse Dockerfile: %v\n", err) - return ExitFatal + fmt.Fprintf(stderr, "Error: %v\n", err) + return nil, false, true } + single = len(paths) == 1 && len(expanded) == 1 && expanded[0] == paths[0] - ignoreRules := parseIgnoreList(ignoreCSV) - warnUnknownRules(ignoreRules, stderr) - - anlzr := analyzer.NewWithDefaults(analyzer.Config{IgnoreRules: ignoreRules}) - findings := anlzr.Analyze(dockerfile) + reports = make([]formatter.Report, 0, len(expanded)) + for _, path := range expanded { + dockerfile, err := parseFile(path) + if err != nil { + if single { + fmt.Fprintf(stderr, "Error: %v\n", err) + } else { + fmt.Fprintf(stderr, "Error: %s: %v\n", path, err) + } + fatal = true + continue + } + reports = append(reports, formatter.Report{Filename: path, Findings: anlzr.Analyze(dockerfile)}) + } + return reports, single, fatal +} - if formatErr := newFormatter(outputFormat, filename, quiet).Format(findings, stdout); formatErr != nil { - fmt.Fprintf(stderr, "Error: failed to format output: %v\n", formatErr) - return ExitFatal +func parseFile(path string) (*ast.Dockerfile, error) { + file, err := os.Open(path) + if err != nil { + return nil, fmt.Errorf("failed to open file: %w", err) } + defer file.Close() - return determineExitCode(findings, strict) + dockerfile, err := parser.ParseReader(file) + if err != nil { + return nil, fmt.Errorf("failed to parse Dockerfile: %w", err) + } + return dockerfile, nil } // Output formats accepted by --format. @@ -163,35 +257,63 @@ func resolveFormat(format string, jsonOutput bool) (string, error) { } } -// findingFormatter writes findings in one output format. -type findingFormatter interface { +// reportFormatter writes findings in one output format, for a single file +// (Format) or for several (FormatReports). +type reportFormatter interface { Format(findings []ast.Finding, w io.Writer) error + FormatReports(reports []formatter.Report, w io.Writer) error } -// newFormatter returns the formatter for a resolved output format. -func newFormatter(format, filename string, quiet bool) findingFormatter { - switch format { +// output writes the lint reports in the chosen format. +type output struct { + format string + quiet bool + tool formatter.ToolInfo +} + +// write writes the reports. A single input keeps the single-file output shape +// (a JSON object rather than an array); nothing is written when no file could +// be linted, so a fatal error leaves stdout empty. +func (o output) write(reports []formatter.Report, single bool, w io.Writer) error { + switch { + case len(reports) == 0: + return nil + case single: + return o.formatter(reports[0].Filename).Format(reports[0].Findings, w) + default: + return o.formatter("").FormatReports(reports, w) + } +} + +func (o output) formatter(filename string) reportFormatter { + switch o.format { case formatJSON: - return formatter.NewJSONFormatter(filename, quiet) + return formatter.NewJSONFormatter(filename, o.quiet) case formatSARIF: - return formatter.NewSARIFFormatter(filename, quiet, toolInfo()) + return formatter.NewSARIFFormatter(filename, o.quiet, o.tool) case formatGitHub: - return formatter.NewGitHubFormatter(filename, quiet) + return formatter.NewGitHubFormatter(filename, o.quiet) default: - return formatter.NewTextFormatter(filename, quiet) + return formatter.NewTextFormatter(filename, o.quiet) } } -// toolInfo describes docker-lint and every built-in rule for SARIF output. -func toolInfo() formatter.ToolInfo { +// toolInfo describes docker-lint and every built-in rule for SARIF output, +// with default severities as changed by the configuration. +func toolInfo(cfg *config.Config) formatter.ToolInfo { + overrides := cfg.SeverityOverrides() all := rules.DefaultRegistry.All() infos := make([]formatter.RuleInfo, 0, len(all)) for _, rule := range all { + severity := rule.Severity() + if s, ok := overrides[rule.ID()]; ok { + severity = s + } infos = append(infos, formatter.RuleInfo{ ID: rule.ID(), Name: rule.Name(), Description: rule.Description(), - Severity: rule.Severity(), + Severity: severity, }) } return formatter.ToolInfo{ @@ -234,19 +356,19 @@ func parseIgnoreList(csv string) []string { return result } -// warnUnknownRules reports --ignore entries that match no rule, which would -// otherwise silently ignore nothing. IDs from before the DKL rename get their -// replacement named. -func warnUnknownRules(ids []string, stderr io.Writer) { +// warnUnknownRules reports rule IDs from --ignore or the configuration file +// (source) that match no rule, which would otherwise silently do nothing. IDs +// from before the DKL rename get their replacement named. +func warnUnknownRules(ids []string, source string, stderr io.Writer) { for _, id := range ids { if rules.DefaultRegistry.Get(id) != nil { continue } if replacement, ok := rules.LegacyRuleIDs[id]; ok { - fmt.Fprintf(stderr, "Warning: rule %s was renamed to %s; --ignore %s has no effect\n", id, replacement, id) + fmt.Fprintf(stderr, "Warning: rule %s was renamed to %s; %s in %s has no effect\n", id, replacement, id, source) continue } - fmt.Fprintf(stderr, "Warning: unknown rule %s in --ignore; run --rules to list available rules\n", id) + fmt.Fprintf(stderr, "Warning: unknown rule %s in %s; run --rules to list available rules\n", id, source) } } @@ -254,9 +376,11 @@ func printUsage(w io.Writer) { fmt.Fprint(w, `docker-lint - static analyzer for Dockerfiles Usage: - docker-lint [flags] [file] + docker-lint [flags] [path ...] -Reads from stdin when no file is given. +Each path is a Dockerfile, a directory searched recursively for Dockerfiles, +or a glob pattern such as "services/**/Dockerfile". Reads from stdin when no +path is given. Flags: -h, --help Show this help message @@ -266,12 +390,14 @@ Flags: -q, --quiet Suppress informational messages (show only warnings and errors) -s, --strict Treat warnings as errors --ignore Comma-separated list of rule IDs to ignore + -c, --config Configuration file (default: nearest .docker-lint.yaml) + --no-config Do not read a configuration file --rules List all available rules with descriptions Exit Codes: 0 No errors found (warnings allowed in non-strict mode) 1 Lint errors found, or warnings found in strict mode - 2 Fatal error (invalid arguments, file not found, parse error) + 2 Fatal error (invalid arguments or configuration, file not found, parse error) `) } diff --git a/cmd/docker-lint/multi_test.go b/cmd/docker-lint/multi_test.go new file mode 100644 index 0000000..6a82f50 --- /dev/null +++ b/cmd/docker-lint/multi_test.go @@ -0,0 +1,288 @@ +package main + +import ( + "bytes" + "encoding/json" + "os" + "path/filepath" + "strings" + "testing" +) + +// writeTree creates files below a temporary directory and returns its path. +func writeTree(t *testing.T, files map[string]string) string { + t.Helper() + root := t.TempDir() + for name, content := range files { + path := filepath.Join(root, filepath.FromSlash(name)) + if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(path, []byte(content), 0o644); err != nil { + t.Fatal(err) + } + } + return root +} + +// chdir changes the working directory for the rest of the test. +func chdir(t *testing.T, dir string) { + t.Helper() + wd, err := os.Getwd() + if err != nil { + t.Fatal(err) + } + if err := os.Chdir(dir); err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = os.Chdir(wd) }) +} + +const ( + cleanDockerfile = "FROM alpine:3.20\nUSER nobody\n" + warningDockerfile = "FROM alpine\nUSER nobody\n" // DKL101 + errorDockerfile = "RUN echo hi\n" // DKL001 +) + +func TestLintDirectory(t *testing.T) { + chdir(t, writeTree(t, map[string]string{ + "services/api/Dockerfile": warningDockerfile, + "services/web/web.Dockerfile": errorDockerfile, + "services/web/README.md": "not a Dockerfile", + })) + + var stdout, stderr bytes.Buffer + code := run([]string{"-q", "services"}, nil, &stdout, &stderr) + + if code != ExitLintErrors { + t.Errorf("exit code %d, want %d; stderr: %s", code, ExitLintErrors, stderr.String()) + } + out := stdout.String() + for _, want := range []string{ + "services/api/Dockerfile:1:1: [warning] DKL101", + "services/web/web.Dockerfile:1:1: [error] DKL001", + } { + if !strings.Contains(out, want) { + t.Errorf("output missing %q:\n%s", want, out) + } + } +} + +func TestLintGlobJSONIsArray(t *testing.T) { + chdir(t, writeTree(t, map[string]string{ + "a/Dockerfile": cleanDockerfile, + "b/Dockerfile": warningDockerfile, + })) + + var stdout, stderr bytes.Buffer + if code := run([]string{"--json", "**/Dockerfile"}, nil, &stdout, &stderr); code != ExitSuccess { + t.Fatalf("exit code %d; stderr: %s", code, stderr.String()) + } + + var outputs []struct { + File string `json:"file"` + Summary struct { + Warnings int `json:"warnings"` + } `json:"summary"` + } + if err := json.Unmarshal(stdout.Bytes(), &outputs); err != nil { + t.Fatalf("expected a JSON array: %v\n%s", err, stdout.String()) + } + if len(outputs) != 2 || outputs[0].File != "a/Dockerfile" || outputs[1].File != "b/Dockerfile" { + t.Fatalf("unexpected files: %+v", outputs) + } + if outputs[1].Summary.Warnings == 0 { + t.Errorf("expected warnings for b/Dockerfile: %+v", outputs[1]) + } +} + +func TestLintSingleFileJSONIsObject(t *testing.T) { + path := createTempDockerfile(t, cleanDockerfile) + var stdout, stderr bytes.Buffer + run([]string{"--json", path}, nil, &stdout, &stderr) + + var output map[string]interface{} + if err := json.Unmarshal(stdout.Bytes(), &output); err != nil { + t.Fatalf("expected a JSON object for a single file: %v\n%s", err, stdout.String()) + } +} + +func TestLintMultipleFilesSARIFIsOneRun(t *testing.T) { + chdir(t, writeTree(t, map[string]string{ + "a/Dockerfile": warningDockerfile, + "b/Dockerfile": errorDockerfile, + })) + + var stdout, stderr bytes.Buffer + if code := run([]string{"-f", "sarif", "a/Dockerfile", "b/Dockerfile"}, nil, &stdout, &stderr); code != ExitLintErrors { + t.Fatalf("exit code %d; stderr: %s", code, stderr.String()) + } + + var log struct { + Runs []struct { + Results []struct { + Locations []struct { + PhysicalLocation struct { + ArtifactLocation struct { + URI string `json:"uri"` + } `json:"artifactLocation"` + } `json:"physicalLocation"` + } `json:"locations"` + } `json:"results"` + } `json:"runs"` + } + if err := json.Unmarshal(stdout.Bytes(), &log); err != nil { + t.Fatal(err) + } + if len(log.Runs) != 1 { + t.Fatalf("len(runs) = %d, want 1", len(log.Runs)) + } + uris := make(map[string]bool) + for _, r := range log.Runs[0].Results { + uris[r.Locations[0].PhysicalLocation.ArtifactLocation.URI] = true + } + if !uris["a/Dockerfile"] || !uris["b/Dockerfile"] { + t.Errorf("results should cover both files, got %v", uris) + } +} + +// TestParseErrorDoesNotStopOtherFiles verifies one broken file is reported and +// makes the run fatal, while the other files are still linted. +func TestParseErrorDoesNotStopOtherFiles(t *testing.T) { + chdir(t, writeTree(t, map[string]string{ + "a/Dockerfile": "FROM alpine:3.20\nFOOBAR x\n", + "b/Dockerfile": warningDockerfile, + })) + + var stdout, stderr bytes.Buffer + code := run([]string{"."}, nil, &stdout, &stderr) + + if code != ExitFatal { + t.Errorf("exit code %d, want %d", code, ExitFatal) + } + if !strings.Contains(stderr.String(), "Error: a/Dockerfile: failed to parse Dockerfile") { + t.Errorf("stderr should name the broken file: %q", stderr.String()) + } + if !strings.Contains(stdout.String(), "b/Dockerfile:1:1: [warning] DKL101") { + t.Errorf("the other file should still be linted:\n%s", stdout.String()) + } +} + +func TestLintPathErrors(t *testing.T) { + chdir(t, writeTree(t, map[string]string{"docs/README.md": "x"})) + + for _, args := range [][]string{ + {"docs"}, + {"**/Dockerfile"}, + {"missing/Dockerfile"}, + } { + var stdout, stderr bytes.Buffer + if code := run(args, nil, &stdout, &stderr); code != ExitFatal { + t.Errorf("%v: exit code %d, want %d", args, code, ExitFatal) + } + if stdout.Len() != 0 { + t.Errorf("%v: expected no output, got %q", args, stdout.String()) + } + } +} + +func TestConfigFileDiscovered(t *testing.T) { + chdir(t, writeTree(t, map[string]string{ + ".git/HEAD": "", + ".docker-lint.yaml": ` +ignore: [DKL101] +severity: + DKL303: error + DKL404: "off" +exclude: [vendor] +`, + "Dockerfile": "FROM alpine\n", // DKL101, DKL303, DKL404 + "vendor/Dockerfile": errorDockerfile, + })) + + var stdout, stderr bytes.Buffer + code := run([]string{"."}, nil, &stdout, &stderr) + + out := stdout.String() + if code != ExitLintErrors { + t.Errorf("exit code %d, want %d (DKL303 raised to error); output:\n%s", code, ExitLintErrors, out) + } + if !strings.Contains(out, "[error] DKL303") { + t.Errorf("DKL303 should be reported as an error:\n%s", out) + } + for _, unwanted := range []string{"DKL101", "DKL404", "vendor/"} { + if strings.Contains(out, unwanted) { + t.Errorf("output should not contain %q:\n%s", unwanted, out) + } + } + + // --no-config restores the defaults. + stdout.Reset() + run([]string{"--no-config", "Dockerfile"}, nil, &stdout, &stderr) + if !strings.Contains(stdout.String(), "[warning] DKL303") || !strings.Contains(stdout.String(), "DKL101") { + t.Errorf("--no-config should ignore the configuration:\n%s", stdout.String()) + } +} + +func TestFlagsOverrideConfig(t *testing.T) { + dir := writeTree(t, map[string]string{ + "lint.yaml": "strict: true\nquiet: true\n", + "Dockerfile": warningDockerfile, + }) + cfg := filepath.Join(dir, "lint.yaml") + dockerfile := filepath.Join(dir, "Dockerfile") + + var stdout, stderr bytes.Buffer + if code := run([]string{"--config", cfg, dockerfile}, nil, &stdout, &stderr); code != ExitLintErrors { + t.Errorf("strict from config: exit code %d, want %d", code, ExitLintErrors) + } + if strings.Contains(stdout.String(), "[info]") { + t.Errorf("quiet from config should hide info findings:\n%s", stdout.String()) + } + + stdout.Reset() + if code := run([]string{"-c", cfg, "--strict=false", "--quiet=false", dockerfile}, nil, &stdout, &stderr); code != ExitSuccess { + t.Errorf("--strict=false should win over the config: exit code %d", code) + } + if !strings.Contains(stdout.String(), "[info]") { + t.Errorf("--quiet=false should win over the config:\n%s", stdout.String()) + } +} + +func TestConfigErrors(t *testing.T) { + dir := writeTree(t, map[string]string{ + "bad.yaml": "severity:\n DKL101: critical\n", + "Dockerfile": cleanDockerfile, + }) + dockerfile := filepath.Join(dir, "Dockerfile") + + for _, args := range [][]string{ + {"--config", filepath.Join(dir, "bad.yaml"), dockerfile}, + {"--config", filepath.Join(dir, "missing.yaml"), dockerfile}, + {"--config", filepath.Join(dir, "bad.yaml"), "--no-config", dockerfile}, + } { + var stdout, stderr bytes.Buffer + if code := run(args, nil, &stdout, &stderr); code != ExitFatal { + t.Errorf("%v: exit code %d, want %d", args, code, ExitFatal) + } + if !strings.Contains(stderr.String(), "Error:") { + t.Errorf("%v: expected an error, got %q", args, stderr.String()) + } + } +} + +func TestConfigUnknownRuleWarns(t *testing.T) { + dir := writeTree(t, map[string]string{ + "lint.yaml": "ignore: [DL3006, DKL999]\n", + "Dockerfile": cleanDockerfile, + }) + + var stdout, stderr bytes.Buffer + run([]string{"-c", filepath.Join(dir, "lint.yaml"), filepath.Join(dir, "Dockerfile")}, nil, &stdout, &stderr) + + for _, want := range []string{"DL3006 was renamed to DKL101", "unknown rule DKL999 in "} { + if !strings.Contains(stderr.String(), want) { + t.Errorf("stderr missing %q: %q", want, stderr.String()) + } + } +} diff --git a/go.mod b/go.mod index fd0c075..e66c17c 100644 --- a/go.mod +++ b/go.mod @@ -5,6 +5,7 @@ go 1.22.0 require ( github.com/leanovate/gopter v0.2.11 github.com/moby/buildkit v0.20.2 + gopkg.in/yaml.v3 v3.0.1 ) require ( diff --git a/go.sum b/go.sum index 427fccf..37e842f 100644 --- a/go.sum +++ b/go.sum @@ -178,8 +178,10 @@ github.com/jtolds/gls v4.20.0+incompatible/go.mod h1:QJZ7F/aHp+rZTRtaJ1ow/lLfFfV 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/kr/fs v0.1.0/go.mod h1:FFnZGqtBN9Gxj7eW1uZ42v5BccTP0vu6NEaFoC2HwRg= +github.com/kr/pretty v0.1.0 h1:L/CwN0zerZDmRFUapSPitk6f+Q3+0za1rQkzVuMiMFI= github.com/kr/pretty v0.1.0/go.mod h1:dAy3ld7l9f0ibDNOQOHHMYYIIbhfbHSm3C4ZsoJORNo= github.com/kr/pty v1.1.1/go.mod h1:pFQYn66WHrOpPYNljwOMqo10TkYh1fy3cYio2l3bCsQ= +github.com/kr/text v0.1.0 h1:45sCR5RtlFHMR4UwH9sdQ5TC8v0qDQCHnXt+kaKSTVE= github.com/kr/text v0.1.0/go.mod h1:4Jbv+DJW3UT/LiOwJeYQe1efqtUx/iVham/4vfdArNI= github.com/leanovate/gopter v0.2.11 h1:vRjThO1EKPb/1NsDXuDrzldR28RLkBflWYcU9CvzWu4= github.com/leanovate/gopter v0.2.11/go.mod h1:aK3tzZP/C+p1m3SPRE4SYZFGP7jjkuSI4f7Xvpt0S9c= @@ -611,6 +613,7 @@ google.golang.org/protobuf v1.26.0/go.mod h1:9q0QmTI4eRPtz6boOQmLYwt+qCgq0jsYwAQ google.golang.org/protobuf v1.35.2 h1:8Ar7bF+apOIoThw1EdZl0p1oWvMqTHmpA2fRTyZO8io= google.golang.org/protobuf v1.35.2/go.mod h1:9fA7Ob0pmnwhb644+1+CVWFRbNajQ6iRojtC/QF5bRE= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= +gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127 h1:qIbj1fsPNlZgppZ+VLlY7N33q108Sa+fhmuc+sWQYwY= gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/errgo.v2 v2.1.0/go.mod h1:hNsd1EY+bozCKY1Ytp96fpM3vjJbqLJn88ws8XvfDNI= gopkg.in/ini.v1 v1.62.0/go.mod h1:pNLf8WUiyNEtQjuu5G5vTm06TEv9tsIgeAvK8hOrP4k= diff --git a/internal/analyzer/analyzer.go b/internal/analyzer/analyzer.go index 2c78819..6ba92b9 100644 --- a/internal/analyzer/analyzer.go +++ b/internal/analyzer/analyzer.go @@ -12,6 +12,8 @@ import ( type Config struct { // IgnoreRules is a list of rule IDs to skip during analysis. IgnoreRules []string + // SeverityOverrides replaces the severity of a rule's findings. + SeverityOverrides map[string]ast.Severity } // Analyzer orchestrates the execution of lint rules against a Dockerfile AST. @@ -63,6 +65,9 @@ func (a *Analyzer) Analyze(dockerfile *ast.Dockerfile) []ast.Finding { if a.isIgnoredByInlineComment(dockerfile, finding) { continue } + if severity, ok := a.config.SeverityOverrides[finding.RuleID]; ok { + finding.Severity = severity + } allFindings = append(allFindings, finding) } } @@ -142,6 +147,9 @@ func (a *Analyzer) AnalyzeWithRules(dockerfile *ast.Dockerfile, ruleIDs []string if a.isIgnoredByInlineComment(dockerfile, finding) { continue } + if severity, ok := a.config.SeverityOverrides[finding.RuleID]; ok { + finding.Severity = severity + } allFindings = append(allFindings, finding) } } diff --git a/internal/config/config.go b/internal/config/config.go new file mode 100644 index 0000000..95dcf39 --- /dev/null +++ b/internal/config/config.go @@ -0,0 +1,166 @@ +// Package config loads docker-lint's project configuration file. +package config + +import ( + "bytes" + "errors" + "fmt" + "io" + "os" + "path/filepath" + "sort" + "strings" + + "gopkg.in/yaml.v3" + + "github.com/devblac/docker-lint/internal/ast" +) + +// FileNames are the configuration file names looked up, in order of preference. +var FileNames = []string{".docker-lint.yaml", ".docker-lint.yml"} + +// SeverityOff disables a rule from the severity map. +const SeverityOff = "off" + +// Config is the content of a .docker-lint.yaml file. +type Config struct { + // Ignore lists rule IDs that are never reported. + Ignore []string `yaml:"ignore"` + // Severity overrides the severity of individual rules: error, warning, + // info, or off to disable the rule. + Severity map[string]string `yaml:"severity"` + // Strict fails on warnings, not only on errors. Nil means unset. + Strict *bool `yaml:"strict"` + // Quiet hides informational findings. Nil means unset. + Quiet *bool `yaml:"quiet"` + // Exclude lists path patterns skipped when a directory or glob is expanded. + Exclude []string `yaml:"exclude"` +} + +// Load reads and validates the configuration file at path. +func Load(path string) (*Config, error) { + data, err := os.ReadFile(path) + if err != nil { + return nil, err + } + cfg, err := Parse(data) + if err != nil { + return nil, fmt.Errorf("%s: %w", path, err) + } + return cfg, nil +} + +// Parse decodes and validates configuration from YAML. Unknown keys are an +// error, so a misspelled option fails loudly instead of being ignored. +func Parse(data []byte) (*Config, error) { + cfg := &Config{} + decoder := yaml.NewDecoder(bytes.NewReader(data)) + decoder.KnownFields(true) + if err := decoder.Decode(cfg); err != nil && !errors.Is(err, io.EOF) { + return nil, err + } + if err := cfg.validate(); err != nil { + return nil, err + } + return cfg, nil +} + +func (c *Config) validate() error { + ids := make([]string, 0, len(c.Severity)) + for id := range c.Severity { + ids = append(ids, id) + } + sort.Strings(ids) + for _, id := range ids { + value := strings.ToLower(strings.TrimSpace(c.Severity[id])) + if value == SeverityOff { + continue + } + if _, ok := parseSeverity(value); !ok { + return fmt.Errorf("severity of %s: unknown value %q (want error, warning, info or off)", id, c.Severity[id]) + } + } + return nil +} + +// DisabledRules returns the rules set to "off" in the severity map, sorted. +func (c *Config) DisabledRules() []string { + var ids []string + for id, value := range c.Severity { + if strings.EqualFold(strings.TrimSpace(value), SeverityOff) { + ids = append(ids, id) + } + } + sort.Strings(ids) + return ids +} + +// SeverityOverrides returns the severity map without the disabled rules. +func (c *Config) SeverityOverrides() map[string]ast.Severity { + overrides := make(map[string]ast.Severity) + for id, value := range c.Severity { + if severity, ok := parseSeverity(strings.ToLower(strings.TrimSpace(value))); ok { + overrides[id] = severity + } + } + return overrides +} + +// RuleIDs returns every rule ID the configuration mentions, sorted, so callers +// can warn about IDs that match no rule. +func (c *Config) RuleIDs() []string { + seen := make(map[string]bool) + var ids []string + add := func(id string) { + if !seen[id] { + seen[id] = true + ids = append(ids, id) + } + } + for _, id := range c.Ignore { + add(id) + } + for id := range c.Severity { + add(id) + } + sort.Strings(ids) + return ids +} + +func parseSeverity(value string) (ast.Severity, bool) { + switch value { + case "error": + return ast.SeverityError, true + case "warning": + return ast.SeverityWarning, true + case "info": + return ast.SeverityInfo, true + } + return 0, false +} + +// Find looks for a configuration file in dir and then in each parent +// directory, stopping after the repository root (the first directory that +// contains .git) or the filesystem root. It returns "" when there is none. +func Find(dir string) (string, error) { + dir, err := filepath.Abs(dir) + if err != nil { + return "", err + } + for { + for _, name := range FileNames { + candidate := filepath.Join(dir, name) + if info, err := os.Stat(candidate); err == nil && !info.IsDir() { + return candidate, nil + } + } + if _, err := os.Stat(filepath.Join(dir, ".git")); err == nil { + return "", nil + } + parent := filepath.Dir(dir) + if parent == dir { + return "", nil + } + dir = parent + } +} diff --git a/internal/config/config_test.go b/internal/config/config_test.go new file mode 100644 index 0000000..ecbf00a --- /dev/null +++ b/internal/config/config_test.go @@ -0,0 +1,140 @@ +package config + +import ( + "os" + "path/filepath" + "reflect" + "strings" + "testing" + + "github.com/devblac/docker-lint/internal/ast" +) + +func TestParse(t *testing.T) { + cfg, err := Parse([]byte(` +ignore: [DKL103] +severity: + DKL404: warning + DKL202: "off" + DKL303: Error +strict: true +exclude: + - vendor + - "examples/**" +`)) + if err != nil { + t.Fatalf("Parse() error = %v", err) + } + + if !reflect.DeepEqual(cfg.Ignore, []string{"DKL103"}) { + t.Errorf("Ignore = %q", cfg.Ignore) + } + if cfg.Strict == nil || !*cfg.Strict { + t.Errorf("Strict = %v, want true", cfg.Strict) + } + if cfg.Quiet != nil { + t.Errorf("Quiet = %v, want unset", *cfg.Quiet) + } + if !reflect.DeepEqual(cfg.Exclude, []string{"vendor", "examples/**"}) { + t.Errorf("Exclude = %q", cfg.Exclude) + } + if got := cfg.DisabledRules(); !reflect.DeepEqual(got, []string{"DKL202"}) { + t.Errorf("DisabledRules() = %q", got) + } + wantOverrides := map[string]ast.Severity{"DKL404": ast.SeverityWarning, "DKL303": ast.SeverityError} + if got := cfg.SeverityOverrides(); !reflect.DeepEqual(got, wantOverrides) { + t.Errorf("SeverityOverrides() = %v, want %v", got, wantOverrides) + } + if got := cfg.RuleIDs(); !reflect.DeepEqual(got, []string{"DKL103", "DKL202", "DKL303", "DKL404"}) { + t.Errorf("RuleIDs() = %q", got) + } +} + +// TestParseUnquotedOff verifies a bare off is read as the string "off", not as +// a YAML 1.1 boolean. +func TestParseUnquotedOff(t *testing.T) { + cfg, err := Parse([]byte("severity:\n DKL404: off\n DKL202: OFF\n")) + if err != nil { + t.Fatalf("Parse() error = %v", err) + } + if got := cfg.DisabledRules(); !reflect.DeepEqual(got, []string{"DKL202", "DKL404"}) { + t.Errorf("DisabledRules() = %q", got) + } +} + +func TestParseEmpty(t *testing.T) { + for _, input := range []string{"", "# only a comment\n"} { + cfg, err := Parse([]byte(input)) + if err != nil { + t.Fatalf("Parse(%q) error = %v", input, err) + } + if cfg.Strict != nil || len(cfg.Ignore) != 0 { + t.Errorf("Parse(%q) = %+v, want an empty config", input, cfg) + } + } +} + +func TestParseErrors(t *testing.T) { + tests := []struct { + input string + want string + }{ + {"ignores: [DKL101]\n", "field ignores not found"}, // typo is not silently ignored + {"severity:\n DKL101: critical\n", `unknown value "critical"`}, + {"strict: maybe\n", "cannot unmarshal"}, + {"ignore: DKL101\n", "cannot unmarshal"}, + } + for _, tt := range tests { + _, err := Parse([]byte(tt.input)) + if err == nil || !strings.Contains(err.Error(), tt.want) { + t.Errorf("Parse(%q) error = %v, want it to mention %q", tt.input, err, tt.want) + } + } +} + +func TestFind(t *testing.T) { + root := t.TempDir() + repo := filepath.Join(root, "repo") + deep := filepath.Join(repo, "services", "api") + if err := os.MkdirAll(deep, 0o755); err != nil { + t.Fatal(err) + } + if err := os.Mkdir(filepath.Join(repo, ".git"), 0o755); err != nil { + t.Fatal(err) + } + write := func(path string) { + t.Helper() + if err := os.WriteFile(path, []byte("strict: true\n"), 0o644); err != nil { + t.Fatal(err) + } + } + + // A config above the repository root is not picked up. + write(filepath.Join(root, ".docker-lint.yaml")) + if got, err := Find(deep); err != nil || got != "" { + t.Errorf("Find() = %q, %v; want no config outside the repository", got, err) + } + + write(filepath.Join(repo, ".docker-lint.yml")) + if got, _ := Find(deep); got != filepath.Join(repo, ".docker-lint.yml") { + t.Errorf("Find() = %q, want the repository root config", got) + } + + // The nearest file wins, and .yaml is preferred over .yml. + write(filepath.Join(deep, ".docker-lint.yml")) + write(filepath.Join(deep, ".docker-lint.yaml")) + if got, _ := Find(deep); got != filepath.Join(deep, ".docker-lint.yaml") { + t.Errorf("Find() = %q, want the nearest .yaml config", got) + } +} + +func TestLoadReportsPath(t *testing.T) { + path := filepath.Join(t.TempDir(), ".docker-lint.yaml") + if err := os.WriteFile(path, []byte("strict: maybe\n"), 0o644); err != nil { + t.Fatal(err) + } + _, err := Load(path) + if err == nil || !strings.Contains(err.Error(), path) { + t.Errorf("Load() error = %v, want it to name %s", err, path) + } +} diff --git a/internal/files/files.go b/internal/files/files.go new file mode 100644 index 0000000..f75663b --- /dev/null +++ b/internal/files/files.go @@ -0,0 +1,247 @@ +// Package files expands the paths given on the command line into the list of +// Dockerfiles to lint. +package files + +import ( + "errors" + "fmt" + "io/fs" + "os" + "path" + "path/filepath" + "sort" + "strings" +) + +// Expand turns each argument into Dockerfile paths: +// +// - a file is linted as given, whatever its name; +// - a directory is searched recursively for Dockerfiles (see IsDockerfile), +// skipping .git and node_modules; +// - a pattern with *, ? or [ is matched against every file below its fixed +// leading directories, with ** matching any number of directories. +// +// Exclude patterns drop files found through a directory or a pattern; a file +// named explicitly is always linted. A directory or pattern that matches no +// file is an error, so a CI job cannot pass without linting anything. +// +// Paths found by expansion use forward slashes. The result has no duplicates +// and keeps the order of the arguments. +func Expand(args []string, exclude []string) ([]string, error) { + var result []string + seen := make(map[string]bool) + add := func(p string) { + key := filepath.Clean(p) + if !seen[key] { + seen[key] = true + result = append(result, p) + } + } + + for _, arg := range args { + if hasMeta(arg) { + matches, err := expandPattern(arg, exclude) + if err != nil { + return nil, err + } + if len(matches) == 0 { + return nil, fmt.Errorf("%s: pattern matched no files", arg) + } + for _, m := range matches { + add(m) + } + continue + } + + info, err := os.Stat(arg) + if err != nil { + return nil, err + } + if !info.IsDir() { + add(arg) + continue + } + + found, err := expandDir(arg, exclude) + if err != nil { + return nil, err + } + if len(found) == 0 { + return nil, fmt.Errorf("%s: no Dockerfiles found in directory", arg) + } + for _, f := range found { + add(f) + } + } + return result, nil +} + +// IsDockerfile reports whether a file name follows a Dockerfile naming +// convention: Dockerfile, Containerfile, Dockerfile. or +// .Dockerfile, in any letter case. Dockerfile-specific ignore files +// (Dockerfile.dockerignore) are not Dockerfiles. +func IsDockerfile(name string) bool { + lower := strings.ToLower(name) + if strings.HasSuffix(lower, ".dockerignore") { + return false + } + for _, base := range []string{"dockerfile", "containerfile"} { + if lower == base || strings.HasPrefix(lower, base+".") || strings.HasSuffix(lower, "."+base) { + return true + } + } + return false +} + +func expandDir(dir string, exclude []string) ([]string, error) { + var found []string + err := filepath.WalkDir(dir, func(p string, d fs.DirEntry, err error) error { + if err != nil { + return err + } + rel := filepath.ToSlash(p) + if d.IsDir() { + if p != dir && skipDir(d.Name()) { + return filepath.SkipDir + } + return nil + } + if IsDockerfile(d.Name()) && !Excluded(rel, exclude) { + found = append(found, rel) + } + return nil + }) + sort.Strings(found) + return found, err +} + +func expandPattern(pattern string, exclude []string) ([]string, error) { + pattern = filepath.ToSlash(pattern) + if _, err := path.Match(strings.ReplaceAll(pattern, "**", "*"), ""); err != nil { + return nil, fmt.Errorf("%s: %w", pattern, err) + } + + root := patternRoot(pattern) + var found []string + err := filepath.WalkDir(root, func(p string, d fs.DirEntry, err error) error { + if err != nil { + if errors.Is(err, fs.ErrNotExist) && p == root { + return filepath.SkipAll + } + return err + } + rel := filepath.ToSlash(p) + if d.IsDir() { + if p != root && skipDir(d.Name()) { + return filepath.SkipDir + } + return nil + } + if Match(pattern, rel) && !Excluded(rel, exclude) { + found = append(found, rel) + } + return nil + }) + sort.Strings(found) + return found, err +} + +// patternRoot returns the leading directories of a pattern that contain no +// wildcard, which is where the walk for matches starts. +func patternRoot(pattern string) string { + segments := strings.Split(pattern, "/") + var fixed []string + for _, s := range segments[:len(segments)-1] { + if hasMeta(s) { + break + } + fixed = append(fixed, s) + } + if len(fixed) == 0 { + return "." + } + root := strings.Join(fixed, "/") + if root == "" { + return "/" // absolute pattern such as /*/Dockerfile + } + return root +} + +// skipDir reports whether a directory is skipped while searching: the Git +// metadata and installed JavaScript packages, which hold no project +// Dockerfiles. Other hidden directories such as .devcontainer are searched. +func skipDir(name string) bool { + return name == ".git" || name == "node_modules" +} + +// Excluded reports whether a slash-separated path matches any exclude pattern. +// A pattern without a slash matches a file or directory name anywhere in the +// path, as in .gitignore; other patterns match the path from its start, and a +// directory pattern also excludes everything inside it. +func Excluded(p string, patterns []string) bool { + p = strings.TrimPrefix(path.Clean(p), "./") + segments := strings.Split(p, "/") + for _, pattern := range patterns { + pattern = strings.TrimSuffix(strings.TrimPrefix(filepath.ToSlash(pattern), "./"), "/") + if pattern == "" { + continue + } + if !strings.Contains(pattern, "/") { + for _, s := range segments { + if ok, _ := path.Match(pattern, s); ok { + return true + } + } + continue + } + // Match the pattern against the path and each of its parent directories. + for i := len(segments); i > 0; i-- { + if Match(pattern, strings.Join(segments[:i], "/")) { + return true + } + } + } + return false +} + +// Match reports whether a slash-separated path matches a pattern in which each +// segment follows path.Match syntax and a ** segment matches zero or more +// directories. A leading "./" on either side is ignored. +func Match(pattern, p string) bool { + pattern = strings.TrimPrefix(pattern, "./") + p = strings.TrimPrefix(path.Clean(p), "./") + return matchSegments(strings.Split(pattern, "/"), strings.Split(p, "/")) +} + +func matchSegments(pattern, segments []string) bool { + for len(pattern) > 0 { + if pattern[0] == "**" { + // Collapse repeated ** and try every possible split point. + rest := pattern[1:] + for len(rest) > 0 && rest[0] == "**" { + rest = rest[1:] + } + if len(rest) == 0 { + return true + } + for i := 0; i <= len(segments); i++ { + if matchSegments(rest, segments[i:]) { + return true + } + } + return false + } + if len(segments) == 0 { + return false + } + if ok, err := path.Match(pattern[0], segments[0]); err != nil || !ok { + return false + } + pattern, segments = pattern[1:], segments[1:] + } + return len(segments) == 0 +} + +func hasMeta(s string) bool { + return strings.ContainsAny(s, "*?[") +} diff --git a/internal/files/files_test.go b/internal/files/files_test.go new file mode 100644 index 0000000..2f305a5 --- /dev/null +++ b/internal/files/files_test.go @@ -0,0 +1,197 @@ +package files + +import ( + "os" + "path/filepath" + "reflect" + "strings" + "testing" +) + +func TestIsDockerfile(t *testing.T) { + tests := map[string]bool{ + "Dockerfile": true, + "dockerfile": true, + "Containerfile": true, + "Dockerfile.prod": true, + "api.Dockerfile": true, + "api.dockerfile": true, + "Containerfile.dev": true, + "Dockerfile.dockerignore": false, + ".dockerignore": false, + "Dockerfiles": false, + "MyDockerfile": false, + "docker-compose.yml": false, + } + for name, want := range tests { + if got := IsDockerfile(name); got != want { + t.Errorf("IsDockerfile(%q) = %v, want %v", name, got, want) + } + } +} + +func TestMatch(t *testing.T) { + tests := []struct { + pattern, path string + want bool + }{ + {"Dockerfile", "Dockerfile", true}, + {"*/Dockerfile", "api/Dockerfile", true}, + {"*/Dockerfile", "services/api/Dockerfile", false}, + {"**/Dockerfile", "Dockerfile", true}, + {"**/Dockerfile", "services/api/Dockerfile", true}, + {"services/**/Dockerfile", "services/Dockerfile", true}, + {"services/**/Dockerfile", "services/a/b/Dockerfile", true}, + {"services/**/Dockerfile", "other/a/Dockerfile", false}, + {"services/**", "services/a/b/Dockerfile", true}, + {"**/*.Dockerfile", "a/b/api.Dockerfile", true}, + {"**/**/x", "x", true}, + {"./api/*", "api/Dockerfile", true}, + {"api/*", "./api/Dockerfile", true}, + {"[", "[", false}, // malformed pattern never matches + } + for _, tt := range tests { + if got := Match(tt.pattern, tt.path); got != tt.want { + t.Errorf("Match(%q, %q) = %v, want %v", tt.pattern, tt.path, got, tt.want) + } + } +} + +func TestExcluded(t *testing.T) { + tests := []struct { + path string + patterns []string + want bool + }{ + {"vendor/x/Dockerfile", []string{"vendor"}, true}, // name anywhere + {"a/vendor/Dockerfile", []string{"vendor"}, true}, // name anywhere + {"a/Dockerfile.test", []string{"*.test"}, true}, // basename glob + {"examples/a/Dockerfile", []string{"examples/"}, true}, // directory with trailing slash + {"examples/a/Dockerfile", []string{"examples/*"}, true}, // parent directory matched + {"a/examples/Dockerfile", []string{"examples/*"}, false}, + {"test/fixtures/Dockerfile", []string{"**/fixtures"}, true}, + {"services/api/Dockerfile", []string{"vendor", "examples/"}, false}, + {"./vendor/Dockerfile", []string{"./vendor"}, true}, + } + for _, tt := range tests { + if got := Excluded(tt.path, tt.patterns); got != tt.want { + t.Errorf("Excluded(%q, %q) = %v, want %v", tt.path, tt.patterns, got, tt.want) + } + } +} + +// makeTree creates files (with empty content) below a temporary directory and +// changes into it for the rest of the test. +func makeTree(t *testing.T, paths ...string) { + t.Helper() + root := t.TempDir() + for _, p := range paths { + full := filepath.Join(root, filepath.FromSlash(p)) + if err := os.MkdirAll(filepath.Dir(full), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(full, []byte("FROM scratch\n"), 0o644); err != nil { + t.Fatal(err) + } + } + wd, err := os.Getwd() + if err != nil { + t.Fatal(err) + } + if err := os.Chdir(root); err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = os.Chdir(wd) }) +} + +func TestExpand(t *testing.T) { + makeTree(t, + "Dockerfile", + "README.md", + "services/api/Dockerfile", + "services/api/Dockerfile.dockerignore", + "services/web/web.Dockerfile", + "services/web/node_modules/pkg/Dockerfile", + ".devcontainer/Dockerfile", + ".git/Dockerfile", + "vendor/lib/Dockerfile", + "notes/custom-name", + ) + + tests := []struct { + name string + args []string + exclude []string + want []string + }{ + { + name: "directory", + args: []string{"."}, + want: []string{".devcontainer/Dockerfile", "Dockerfile", "services/api/Dockerfile", "services/web/web.Dockerfile", "vendor/lib/Dockerfile"}, + }, + { + name: "directory with exclude", + args: []string{"."}, + exclude: []string{"vendor", ".devcontainer"}, + want: []string{"Dockerfile", "services/api/Dockerfile", "services/web/web.Dockerfile"}, + }, + { + name: "subdirectory", + args: []string{"services"}, + want: []string{"services/api/Dockerfile", "services/web/web.Dockerfile"}, + }, + { + name: "recursive pattern", + args: []string{"**/Dockerfile"}, + want: []string{".devcontainer/Dockerfile", "Dockerfile", "services/api/Dockerfile", "vendor/lib/Dockerfile"}, + }, + { + name: "pattern below a fixed directory", + args: []string{"services/*/*"}, + want: []string{"services/api/Dockerfile", "services/api/Dockerfile.dockerignore", "services/web/web.Dockerfile"}, + }, + { + name: "explicit file is linted whatever its name and excludes", + args: []string{"notes/custom-name", "vendor/lib/Dockerfile"}, + exclude: []string{"vendor"}, + want: []string{"notes/custom-name", "vendor/lib/Dockerfile"}, + }, + { + name: "duplicates are dropped, order is kept", + args: []string{"services/api/Dockerfile", "services", "Dockerfile"}, + want: []string{"services/api/Dockerfile", "services/web/web.Dockerfile", "Dockerfile"}, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := Expand(tt.args, tt.exclude) + if err != nil { + t.Fatalf("Expand() error = %v", err) + } + if !reflect.DeepEqual(got, tt.want) { + t.Errorf("Expand(%q) =\n %q\nwant\n %q", tt.args, got, tt.want) + } + }) + } +} + +func TestExpandErrors(t *testing.T) { + makeTree(t, "docs/README.md", "Dockerfile") + + tests := []struct { + args []string + want string + }{ + {[]string{"missing/Dockerfile"}, "missing"}, + {[]string{"docs"}, "no Dockerfiles found"}, + {[]string{"**/*.Containerfile"}, "matched no files"}, + {[]string{"nope/**/Dockerfile"}, "matched no files"}, + {[]string{"Dockerfile", "[bad"}, "syntax error"}, + } + for _, tt := range tests { + _, err := Expand(tt.args, nil) + if err == nil || !strings.Contains(err.Error(), tt.want) { + t.Errorf("Expand(%q) error = %v, want it to mention %q", tt.args, err, tt.want) + } + } +} diff --git a/internal/formatter/json.go b/internal/formatter/json.go index ca81a85..e76d969 100644 --- a/internal/formatter/json.go +++ b/internal/formatter/json.go @@ -50,8 +50,29 @@ func NewJSONFormatter(filename string, quiet bool) *JSONFormatter { // Format writes the findings to the given writer as valid JSON. func (f *JSONFormatter) Format(findings []ast.Finding, w io.Writer) error { + return encodeJSON(w, f.output(f.Filename, findings)) +} + +// FormatReports writes the findings of several files as a JSON array with one +// object per file, each shaped like the output of Format. +func (f *JSONFormatter) FormatReports(reports []Report, w io.Writer) error { + outputs := make([]JSONOutput, 0, len(reports)) + for _, r := range reports { + outputs = append(outputs, f.output(r.Filename, r.Findings)) + } + return encodeJSON(w, outputs) +} + +func encodeJSON(w io.Writer, v interface{}) error { + encoder := json.NewEncoder(w) + encoder.SetIndent("", " ") + return encoder.Encode(v) +} + +// output builds the JSON object for one file. +func (f *JSONFormatter) output(filename string, findings []ast.Finding) JSONOutput { output := JSONOutput{ - File: f.Filename, + File: filename, Findings: make([]JSONFinding, 0), Summary: JSONSummary{ Total: 0, @@ -89,7 +110,5 @@ func (f *JSONFormatter) Format(findings []ast.Finding, w io.Writer) error { output.Summary.Total++ } - encoder := json.NewEncoder(w) - encoder.SetIndent("", " ") - return encoder.Encode(output) + return output } diff --git a/internal/formatter/report.go b/internal/formatter/report.go new file mode 100644 index 0000000..34900e2 --- /dev/null +++ b/internal/formatter/report.go @@ -0,0 +1,36 @@ +package formatter + +import ( + "io" + + "github.com/devblac/docker-lint/internal/ast" +) + +// Report holds the findings of one linted file. +type Report struct { + // Filename is the path of the file, as shown in the output. + Filename string + // Findings are the file's findings, sorted by line. + Findings []ast.Finding +} + +// FormatReports writes the findings of several files as consecutive text +// blocks; each line already names its file. +func (f *TextFormatter) FormatReports(reports []Report, w io.Writer) error { + for _, r := range reports { + if err := NewTextFormatter(r.Filename, f.Quiet).Format(r.Findings, w); err != nil { + return err + } + } + return nil +} + +// FormatReports writes the workflow commands of several files. +func (f *GitHubFormatter) FormatReports(reports []Report, w io.Writer) error { + for _, r := range reports { + if err := NewGitHubFormatter(r.Filename, f.Quiet).Format(r.Findings, w); err != nil { + return err + } + } + return nil +} diff --git a/internal/formatter/sarif.go b/internal/formatter/sarif.go index 864238c..859fb0c 100644 --- a/internal/formatter/sarif.go +++ b/internal/formatter/sarif.go @@ -120,6 +120,12 @@ type sarifRegion struct { // Format writes the findings to w as a SARIF log with a single run. func (f *SARIFFormatter) Format(findings []ast.Finding, w io.Writer) error { + return f.FormatReports([]Report{{Filename: f.Filename, Findings: findings}}, w) +} + +// FormatReports writes the findings of several files as one SARIF run, the +// shape code scanning expects for a single analysis of a repository. +func (f *SARIFFormatter) FormatReports(reports []Report, w io.Writer) error { rules := make([]sarifRule, 0, len(f.Tool.Rules)) ruleIndex := make(map[string]int, len(f.Tool.Rules)) for i, rule := range f.Tool.Rules { @@ -135,9 +141,34 @@ func (f *SARIFFormatter) Format(findings []ast.Finding, w io.Writer) error { }) } - uri := artifactURI(f.Filename) - results := make([]sarifResult, 0, len(findings)) - for _, finding := range findings { + results := make([]sarifResult, 0) + for _, report := range reports { + results = f.appendResults(results, report, ruleIndex) + } + + log := sarifLog{ + Schema: sarifSchema, + Version: sarifVersion, + Runs: []sarifRun{{ + Tool: sarifTool{Driver: sarifDriver{ + Name: f.Tool.Name, + Version: f.Tool.Version, + InformationURI: f.Tool.InformationURI, + Rules: rules, + }}, + Results: results, + }}, + } + + encoder := json.NewEncoder(w) + encoder.SetIndent("", " ") + return encoder.Encode(log) +} + +// appendResults converts the findings of one file into SARIF results. +func (f *SARIFFormatter) appendResults(results []sarifResult, report Report, ruleIndex map[string]int) []sarifResult { + uri := artifactURI(report.Filename) + for _, finding := range report.Findings { if f.Quiet && finding.Severity == ast.SeverityInfo { continue } @@ -163,24 +194,7 @@ func (f *SARIFFormatter) Format(findings []ast.Finding, w io.Writer) error { } results = append(results, result) } - - log := sarifLog{ - Schema: sarifSchema, - Version: sarifVersion, - Runs: []sarifRun{{ - Tool: sarifTool{Driver: sarifDriver{ - Name: f.Tool.Name, - Version: f.Tool.Version, - InformationURI: f.Tool.InformationURI, - Rules: rules, - }}, - Results: results, - }}, - } - - encoder := json.NewEncoder(w) - encoder.SetIndent("", " ") - return encoder.Encode(log) + return results } // sarifLevel maps a severity to a SARIF result level. diff --git a/testdata/config/no-from-off.yaml b/testdata/config/no-from-off.yaml new file mode 100644 index 0000000..1b87763 --- /dev/null +++ b/testdata/config/no-from-off.yaml @@ -0,0 +1,4 @@ +# Used by the action workflow: turns the DKL001 error off, so linting +# testdata/invalid/no-from.Dockerfile succeeds. +severity: + DKL001: "off"