diff --git a/README.md b/README.md index 192a350..7296bf6 100644 --- a/README.md +++ b/README.md @@ -176,6 +176,15 @@ on every pull request by default, against the base branch's tip. Paths on the command line are relative to the current directory, in the working tree and at the base alike, so it runs from a subdirectory of a monorepo. +`--disable RULE[,RULE]` leaves rules out: their findings are not shown and do +not fail the run, and the output ends with what was left out, for example +`not shown (--disable): source-column-not-captured 161`, so a filtered run +never reads as a clean one. `--disable source-column-not-captured` is the usual +one, for a repository that has read its inventory of uncaptured columns and +does not want it on every run. Disabling `schema-before-connector` is the same +as leaving out `--base`. An unknown rule name is an error, not a filter that +hides nothing. + Files in, findings out, non-zero exit. No database, no daemon, no credentials. Under a second on a laptop. `--fail-on warning` or `info` raises the bar; `--format json` is for anything that wants to post findings somewhere. A diff --git a/action.yml b/action.yml index 0e9b284..f7cd0cf 100644 --- a/action.yml +++ b/action.yml @@ -35,6 +35,13 @@ inputs: description: "Exit non-zero at this severity or above: error, warning or info." required: false default: error + disable: + description: >- + Rules to leave out, comma-separated, such as source-column-not-captured. + Their findings are not shown and do not fail the step; the log says how + many were left out. An unknown rule name fails the step. + required: false + default: "" version: description: cdclint release to run, as a tag such as v0.1.0; latest by default. required: false @@ -121,11 +128,15 @@ runs: SINKS: ${{ inputs.sink }} SINK_CONNECTORS: ${{ inputs.sink-connector }} FAIL_ON: ${{ inputs.fail-on }} + DISABLE: ${{ inputs.disable }} BASE: ${{ inputs.base }} PR_BASE_SHA: ${{ github.event.pull_request.base.sha }} run: | set -uo pipefail args=(--migrations "$MIGRATIONS" --connector "$CONNECTOR" --fail-on "$FAIL_ON") + if [ -n "$DISABLE" ]; then + args+=(--disable "$DISABLE") + fi # The diff rule needs the base commit's objects, and # actions/checkout fetches one commit by default. Whatever form the # base was given in (main, origin/main, a tag, a commit id), it is diff --git a/cmd/cdclint/corpus_test.go b/cmd/cdclint/corpus_test.go index c2e76d9..174aa63 100644 --- a/cmd/cdclint/corpus_test.go +++ b/cmd/cdclint/corpus_test.go @@ -82,7 +82,13 @@ func TestCorpus(t *testing.T) { if _, err := os.Stat(filepath.Join(name, "base")); err == nil { in.Base = baseFromDir(t, filepath.Join(name, "base"), filepath.Join(name, "connector.json")) } - got := Render(engine.Run(in)) + findings := engine.Run(in) + for _, f := range findings { + if !engine.KnownRule(f.Rule) { + t.Errorf("finding from rule %q, which engine.Rules does not list; --disable could not name it", f.Rule) + } + } + got := Render(findings) expectedPath := filepath.Join(name, "expected.txt") if *update { if err := os.WriteFile(expectedPath, []byte(got), 0o644); err != nil { diff --git a/cmd/cdclint/main.go b/cmd/cdclint/main.go index b2ee952..7dba74a 100644 --- a/cmd/cdclint/main.go +++ b/cmd/cdclint/main.go @@ -14,6 +14,7 @@ import ( "encoding/json" "flag" "fmt" + "io" "os" "strings" @@ -36,16 +37,18 @@ func (m *multi) String() string { return strings.Join(*m, ",") } func (m *multi) Set(v string) error { *m = append(*m, v); return nil } func main() { - os.Exit(run(os.Args[1:])) + os.Exit(run(os.Args[1:], os.Stdout, os.Stderr)) } -func run(args []string) int { +func run(args []string, stdout, stderr io.Writer) int { fs := flag.NewFlagSet("cdclint", flag.ContinueOnError) + fs.SetOutput(stderr) var ( migrations = fs.String("migrations", "", "directory of source migrations, applied in name order; Postgres or MySQL, from the connector class, or say it with postgres:DIR or mysql:DIR") connector = fs.String("connector", "", "Debezium source connector JSON") sinks multi sinkConns multi + disable multi format = fs.String("format", "text", "output format: text or json") minSev = fs.String("fail-on", "error", "exit non-zero at this severity or above: error, warning, info") base = fs.String("base", "", "git ref of the change's base (a branch, a commit, origin/main); enables schema-before-connector, which judges the diff") @@ -53,6 +56,8 @@ func run(args []string) int { ) fs.Var(&sinks, "sink", "sink DDL directory as [dialect:]DIR; dialect is clickhouse (default), bigquery, snowflake or iceberg; repeatable") fs.Var(&sinkConns, "sink-connector", "Kafka Connect sink connector JSON; repeatable") + fs.Var(&disable, "disable", "rule names to leave out, comma-separated or repeated: "+strings.Join(engine.Rules, ", ")+ + "; their findings are not shown and do not fail the run, and the output says how many were left out") fs.Usage = func() { fmt.Fprintln(fs.Output(), "usage: cdclint --migrations DIR --connector FILE --sink [dialect:]DIR [--sink-connector FILE]...") fs.PrintDefaults() @@ -61,30 +66,47 @@ func run(args []string) int { return 2 } if *showVer || (fs.NArg() > 0 && fs.Arg(0) == "version") { - fmt.Println("cdclint", version) + fmt.Fprintln(stdout, "cdclint", version) return 0 } + disabled := map[string]bool{} + for _, v := range disable { + for _, name := range strings.Split(v, ",") { + if name = strings.TrimSpace(name); name == "" { + continue + } + if !engine.KnownRule(name) { + fmt.Fprintf(stderr, "cdclint: --disable: unknown rule %q; the rules are %s\n", name, strings.Join(engine.Rules, ", ")) + return 2 + } + disabled[name] = true + } + } if *migrations == "" || *connector == "" || len(sinks) == 0 { fs.Usage() return 2 } in, err := load(*migrations, *connector, sinks, sinkConns) if err != nil { - fmt.Fprintln(os.Stderr, "cdclint:", err) + fmt.Fprintln(stderr, "cdclint:", err) return 2 } - if *base != "" { + // Disabling the diff rule is the same as not giving --base: filtering + // its findings afterwards would also drop the columns it raised, which + // source-column-not-captured then leaves out of its list, so they + // would appear nowhere. + if *base != "" && !disabled["schema-before-connector"] { b, err := LoadBase(*base, *migrations, *connector) if err != nil { - fmt.Fprintln(os.Stderr, "cdclint:", err) + fmt.Fprintln(stderr, "cdclint:", err) return 2 } in.Base = b } - findings := engine.Run(in) + findings, hidden := without(engine.Run(in), disabled) switch *format { case "json": - enc := json.NewEncoder(os.Stdout) + enc := json.NewEncoder(stdout) enc.SetIndent("", " ") type out struct { Rule string `json:"rule"` @@ -102,8 +124,19 @@ func run(args []string) int { rows = []out{} } _ = enc.Encode(rows) + // The array's shape is what consumers parse, so the note goes + // to stderr rather than into it. + if note := hiddenNote(hidden); note != "" { + fmt.Fprint(stderr, "cdclint: "+note) + } default: - os.Stdout.WriteString(Render(findings)) + if len(findings) == 0 && len(hidden) > 0 { + // "Agree" would claim more than was checked. + fmt.Fprintln(stdout, "ok: nothing to report outside the disabled rules") + } else { + fmt.Fprint(stdout, Render(findings)) + } + fmt.Fprint(stdout, hiddenNote(hidden)) } threshold := model.Error switch *minSev { @@ -120,6 +153,38 @@ func run(args []string) int { return 0 } +// without removes the findings of disabled rules and counts them by rule. +func without(findings []model.Finding, disabled map[string]bool) ([]model.Finding, map[string]int) { + if len(disabled) == 0 { + return findings, nil + } + var kept []model.Finding + hidden := map[string]int{} + for _, f := range findings { + if disabled[f.Rule] { + hidden[f.Rule]++ + continue + } + kept = append(kept, f) + } + return kept, hidden +} + +// hiddenNote says what --disable left out, so a filtered run never reads as +// a clean one. It is empty when nothing was left out. +func hiddenNote(hidden map[string]int) string { + if len(hidden) == 0 { + return "" + } + var parts []string + for _, r := range engine.Rules { + if n := hidden[r]; n > 0 { + parts = append(parts, fmt.Sprintf("%s %d", r, n)) + } + } + return "not shown (--disable): " + strings.Join(parts, ", ") + "\n" +} + // Render is the text output: findings, then a one-line summary. func Render(findings []model.Finding) string { var b strings.Builder diff --git a/cmd/cdclint/main_test.go b/cmd/cdclint/main_test.go new file mode 100644 index 0000000..d7055b3 --- /dev/null +++ b/cmd/cdclint/main_test.go @@ -0,0 +1,90 @@ +package main + +import ( + "bytes" + "strings" + "testing" +) + +func cli(t *testing.T, args ...string) (int, string, string) { + t.Helper() + var out, errOut bytes.Buffer + code := run(args, &out, &errOut) + return code, out.String(), errOut.String() +} + +func entry(name string) []string { + d := "../../corpus/" + name + return []string{"--migrations", d + "/migrations", "--connector", d + "/connector.json", "--sink", d + "/sink"} +} + +func TestDisableHidesARuleAndSaysSo(t *testing.T) { + // column-never-captured reports two source-column-not-captured infos + // and nothing else. + code, out, _ := cli(t, append(entry("column-never-captured"), "--disable", "source-column-not-captured")...) + want := "ok: nothing to report outside the disabled rules\n" + + "not shown (--disable): source-column-not-captured 2\n" + if code != 0 || out != want { + t.Fatalf("exit %d, output:\n%s\nwant:\n%s", code, out, want) + } +} + +func TestADisabledRuleDoesNotFailTheRun(t *testing.T) { + // include-list-typo: one sink-column-not-captured error and one + // captured-column-missing warning; exit 1 at the default --fail-on error. + if code, _, _ := cli(t, entry("include-list-typo")...); code != 1 { + t.Fatalf("without --disable: exit %d, want 1", code) + } + code, out, _ := cli(t, append(entry("include-list-typo"), "--disable", "sink-column-not-captured")...) + if code != 0 { + t.Errorf("exit %d, want 0: the only error was disabled", code) + } + if !strings.Contains(out, "warning captured-column-missing") || strings.Contains(out, "sink-column-not-captured include-list") { + t.Errorf("output:\n%s", out) + } + if !strings.HasSuffix(out, "0 error(s), 1 warning(s), 0 info\nnot shown (--disable): sink-column-not-captured 1\n") { + t.Errorf("summary:\n%s", out) + } +} + +func TestDisableTakesCommasAndRepeats(t *testing.T) { + a, outA, _ := cli(t, append(entry("include-list-typo"), "--disable", "sink-column-not-captured,captured-column-missing")...) + b, outB, _ := cli(t, append(entry("include-list-typo"), "--disable", "sink-column-not-captured", "--disable", " captured-column-missing ")...) + want := "ok: nothing to report outside the disabled rules\n" + + "not shown (--disable): sink-column-not-captured 1, captured-column-missing 1\n" + if a != 0 || b != 0 || outA != want || outB != want { + t.Fatalf("commas: exit %d\n%s\nrepeats: exit %d\n%s\nwant:\n%s", a, outA, b, outB, want) + } +} + +func TestAnUnknownRuleIsAnErrorNotASilentFilter(t *testing.T) { + code, out, errOut := cli(t, append(entry("include-list-typo"), "--disable", "source-column-not-capturd")...) + if code != 2 || out != "" { + t.Fatalf("exit %d, stdout %q", code, out) + } + if !strings.Contains(errOut, `unknown rule "source-column-not-capturd"`) || !strings.Contains(errOut, "source-column-not-captured") { + t.Errorf("stderr: %s", errOut) + } +} + +func TestDisablingTheDiffRuleSkipsTheBase(t *testing.T) { + // A ref that does not exist fails the base load; with the diff rule + // disabled the base is never read, exactly as without --base. + if code, _, _ := cli(t, append(entry("clean"), "--base", "no-such-ref-cdclint")...); code != 2 { + t.Fatalf("the base should have been read and failed: exit %d", code) + } + code, out, errOut := cli(t, append(entry("clean"), "--base", "no-such-ref-cdclint", "--disable", "schema-before-connector")...) + if code != 0 || out != "ok: source, connector and sink agree\n" { + t.Fatalf("exit %d, stdout %q, stderr %q", code, out, errOut) + } +} + +func TestJSONKeepsItsShapeAndNotesOnStderr(t *testing.T) { + code, out, errOut := cli(t, append(entry("include-list-typo"), "--format", "json", "--disable", "sink-column-not-captured")...) + if code != 0 || !strings.HasPrefix(strings.TrimSpace(out), "[") || strings.Contains(out, "sink-column-not-captured") { + t.Fatalf("exit %d, stdout:\n%s", code, out) + } + if errOut != "cdclint: not shown (--disable): sink-column-not-captured 1\n" { + t.Errorf("stderr %q", errOut) + } +} diff --git a/internal/engine/rules.go b/internal/engine/rules.go new file mode 100644 index 0000000..bbc5829 --- /dev/null +++ b/internal/engine/rules.go @@ -0,0 +1,28 @@ +package engine + +// Rules names every rule Run can report, in the README's order. --disable +// checks names against it, so a typo is an error rather than a filter that +// silently hides nothing, and the corpus test checks that every finding's +// rule is here, so a new rule cannot be missed. +var Rules = []string{ + "sink-column-not-captured", + "sink-table-not-captured", + "sink-column-unknown", + "source-column-not-captured", + "captured-column-missing", + "captured-table-missing", + "topic-table-mapping", + "sink-column-flattened", + "mv-column-match", + "schema-before-connector", +} + +// KnownRule reports whether name is one of Rules. +func KnownRule(name string) bool { + for _, r := range Rules { + if r == name { + return true + } + } + return false +}