Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
11 changes: 11 additions & 0 deletions action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
8 changes: 7 additions & 1 deletion cmd/cdclint/corpus_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
83 changes: 74 additions & 9 deletions cmd/cdclint/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (
"encoding/json"
"flag"
"fmt"
"io"
"os"
"strings"

Expand All @@ -36,23 +37,27 @@ 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")
showVer = fs.Bool("version", false, "print the version and exit")
)
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()
Expand All @@ -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"`
Expand All @@ -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 {
Expand All @@ -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
Expand Down
90 changes: 90 additions & 0 deletions cmd/cdclint/main_test.go
Original file line number Diff line number Diff line change
@@ -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)
}
}
28 changes: 28 additions & 0 deletions internal/engine/rules.go
Original file line number Diff line number Diff line change
@@ -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
}