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"