From c6433128eaf272a917b7ee8f822440460402bcd7 Mon Sep 17 00:00:00 2001 From: DIZAMU <117295462+DIZAMU@users.noreply.github.com> Date: Fri, 23 Oct 2026 17:25:30 +0200 Subject: [PATCH 1/4] fix(hooks): reject coding-assistant session lines The hook also rejects Claude-Session trailers and claude.ai/code session links. test-commit-msg.sh covers both, and still accepts a normal message that mentions sessions. --- .githooks/commit-msg | 7 +++++++ scripts/test-commit-msg.sh | 3 +++ 2 files changed, 10 insertions(+) diff --git a/.githooks/commit-msg b/.githooks/commit-msg index ba5d7f2..379aabf 100755 --- a/.githooks/commit-msg +++ b/.githooks/commit-msg @@ -20,4 +20,11 @@ if grep -qiE '^[^[:alnum:]]*Generated with' "$msg_file"; then exit 1 fi +# Reject session trailers and links that coding assistants append, such as +# "Claude-Session: https://claude.ai/code/session_...". +if grep -qiE '^[[:space:]]*Claude-Session:|claude\.ai/code/session' "$msg_file"; then + echo "commit-msg: rejected -- a coding-assistant session line is not allowed in this repo." >&2 + exit 1 +fi + exit 0 diff --git a/scripts/test-commit-msg.sh b/scripts/test-commit-msg.sh index 33b6c80..80fc9c0 100755 --- a/scripts/test-commit-msg.sh +++ b/scripts/test-commit-msg.sh @@ -25,6 +25,9 @@ expect reject $'feat: x\n\nCo-authored-by: Someone ' expect reject $'feat: x\n\n co-authored-by: someone ' expect reject $'feat: x\n\nGenerated with some tool' expect reject $'feat: x\n\n\xf0\x9f\xa4\x96 Generated with [Some Tool](https://example.com)' +expect reject $'feat: x\n\nClaude-Session: https://claude.ai/code/session_0123' +expect reject $'feat: x\n\nSee https://claude.ai/code/session_0123' +expect accept $'docs: explain sessions\n\nA session is one attempt at one variant.' if [ "$failures" -gt 0 ]; then echo "$failures case(s) failed" From ed8bc635a59096fa22ed4ee0b8f8c3ba3fa5f6e2 Mon Sep 17 00:00:00 2001 From: DIZAMU <117295462+DIZAMU@users.noreply.github.com> Date: Fri, 23 Oct 2026 17:25:39 +0200 Subject: [PATCH 2/4] fix(dbt): build from the embedded project and isolate the tests that run dbt The dbt project is embedded and extracted read-only per build. Containers are labelled by database, stale ones are removed first, a failed build keeps its dbt.log under the user cache dir, and a build must run every node the project defines. Tests that run dbt use a database of their own. Integration tests skip only when their database or Docker is not configured, and fail when it is configured but broken (RC_REQUIRE_DOCKER=1 makes missing Docker fatal). --- dbt/embed.go | 13 + internal/cli/next.go | 3 +- internal/dbt/dbt.go | 360 +++++++++++++++++++++++-- internal/dbt/dbt_test.go | 171 +++++++++++- internal/dbt/integration_test.go | 220 +++++++++++++++ internal/play/play.go | 4 +- internal/runtime/docker/docker_test.go | 10 +- internal/selector/integration_test.go | 40 +-- internal/server/server_test.go | 15 +- internal/session/engine_test.go | 23 +- internal/session/reaper_test.go | 43 +-- internal/session/reset_test.go | 15 +- internal/store/migrate_test.go | 3 +- internal/store/store.go | 16 +- internal/store/store_test.go | 31 ++- internal/testenv/testenv.go | 121 +++++++++ 16 files changed, 928 insertions(+), 160 deletions(-) create mode 100644 dbt/embed.go create mode 100644 internal/dbt/integration_test.go create mode 100644 internal/testenv/testenv.go diff --git a/dbt/embed.go b/dbt/embed.go new file mode 100644 index 0000000..ac7915b --- /dev/null +++ b/dbt/embed.go @@ -0,0 +1,13 @@ +// Package dbtproject embeds the dbt project (spec section 9.2) in the binary. +// internal/dbt extracts it into a fresh directory for each build, so a build +// never reads the working tree, where a file caught mid-save could silently +// drop models or tests from the run. +package dbtproject + +import "embed" + +// FS holds the project files dbt reads: the project and profile files, the +// models and the singular tests. +// +//go:embed dbt_project.yml profiles.yml models tests +var FS embed.FS diff --git a/internal/cli/next.go b/internal/cli/next.go index b044bad..0d59645 100644 --- a/internal/cli/next.go +++ b/internal/cli/next.go @@ -5,7 +5,6 @@ import ( "errors" "fmt" "math/rand" - "path/filepath" "time" "github.com/DIZAMU/RootCause/internal/play" @@ -119,7 +118,7 @@ func (a *App) refreshAfterSession(ctx context.Context) { // buildStats runs dbt against the app's database, under the stats lock. func (a *App) buildStats(ctx context.Context) error { - return play.BuildStats(ctx, a.st, filepath.Join(a.root, "dbt"), a.dsn) + return play.BuildStats(ctx, a.st, a.dsn) } // warnRefresh prints a failed refresh as a warning, with its detail. diff --git a/internal/dbt/dbt.go b/internal/dbt/dbt.go index e3ad41d..d14cf1c 100644 --- a/internal/dbt/dbt.go +++ b/internal/dbt/dbt.go @@ -1,5 +1,7 @@ -// Package dbt runs the dbt project in dbt/ (spec section 9.2) in a container, -// against the database the engine uses. The container is locked down like a +// Package dbt runs the dbt project (spec section 9.2) in a container, against +// the database the engine uses. The project is embedded in the binary +// (package dbtproject) and extracted into a fresh directory for each build, so +// a build never reads the working tree. The container is locked down like a // sandbox: no capabilities, a read-only root filesystem, the project mounted // read-only, and the compose network only (never the host network). The // password reaches the container only through its environment. @@ -12,12 +14,20 @@ import ( "encoding/hex" "errors" "fmt" + "io/fs" "os" "os/exec" + "path" + "path/filepath" + "regexp" + "slices" + "strconv" "strings" "time" + dbtproject "github.com/DIZAMU/RootCause/dbt" "github.com/jackc/pgx/v5/pgconn" + "go.yaml.in/yaml/v3" ) // Image is the dbt Core image with the Postgres adapter (spec section 2), @@ -37,14 +47,37 @@ const ( composeService = "postgres" ) +// Labels on every dbt container. Before a build, containers left by an +// earlier build of the same database (a killed engine) are removed. +const ( + labelDBT = "rootcause.dbt" + labelDatabase = "rootcause.database" +) + +// Environment variables that point dbt at a Postgres container outside the +// compose project: the docker network to join and the host name on it. Only +// tests and CI set them (dbt.yml runs Postgres as a service container). +const ( + envNetwork = "RC_DBT_NETWORK" + envHost = "RC_DBT_HOST" +) + // outputTail is how many of dbt's last output lines an error includes. const outputTail = 30 -// Build runs `dbt build` (models and tests) for the project in projectDir -// against the database in dsn. It returns nil on success. dbt's exit 1 (a -// model or test failed) and exit 2 (dbt could not run), and any other exit, -// are errors that end with dbt's last output lines. -func Build(ctx context.Context, projectDir, dsn string) error { +// keepLogs is how many saved dbt.log files of failed builds are kept. +const keepLogs = 20 + +// Build runs `dbt build` (models and tests) for the embedded project against +// the database in dsn. It returns nil only when dbt exits 0 and ran every +// node the project defines. On any failure the full dbt.log is saved under +// the user cache directory, and the error ends with dbt's last output lines +// and the log's path. +// +// Callers hold the statistics lock (store.WithStatsLock), so one build runs +// at a time per database. Build relies on that when it removes the +// containers of earlier builds of the same database. +func Build(ctx context.Context, dsn string) error { cfg, err := pgconn.ParseConfig(dsn) if err != nil { return fmt.Errorf("parse database DSN: %w", err) @@ -52,17 +85,31 @@ func Build(ctx context.Context, projectDir, dsn string) error { if !isLocal(cfg.Host) || cfg.Port != 5432 { return ErrUnsupportedDatabase } - network, err := composeNetwork(ctx) + network, host, err := target(ctx) if err != nil { return err } + want, err := ExpectedNodes(dbtproject.FS) + if err != nil { + return fmt.Errorf("count the project's nodes: %w", err) + } ctx, cancel := context.WithTimeout(ctx, Timeout) defer cancel() + if err := removeStale(ctx, cfg.Database); err != nil { + return err + } + removeOldExtracts() + project, err := extract(dbtproject.FS) + if err != nil { + return fmt.Errorf("extract the dbt project: %w", err) + } + defer os.RemoveAll(project) + name := "rootcause-dbt-" + randomSuffix() env := map[string]string{ - "DBT_HOST": composeService, + "DBT_HOST": host, "DBT_PORT": "5432", "DBT_USER": cfg.User, "DBT_PASSWORD": cfg.Password, @@ -76,8 +123,13 @@ func Build(ctx context.Context, projectDir, dsn string) error { "DBT_SEND_ANONYMOUS_USAGE_STATS": "false", "DBT_VERSION_CHECK": "false", } + // No --rm: after a failure, dbt.log is copied out before the container + // is removed. /tmp/logs is an anonymous volume, not part of the /tmp + // tmpfs, because a tmpfs is emptied when the container stops. args := []string{ - "run", "--rm", "--name", name, + "run", "--name", name, + "--label", labelDBT + "=true", + "--label", labelDatabase + "=" + cfg.Database, "--network", network, "--cap-drop", "ALL", "--security-opt", "no-new-privileges", @@ -85,7 +137,8 @@ func Build(ctx context.Context, projectDir, dsn string) error { "--memory", "1g", "--read-only", "--tmpfs", "/tmp", - "--mount", "type=bind,src=" + projectDir + ",dst=/usr/app/dbt,readonly", + "--mount", "type=volume,dst=/tmp/logs", + "--mount", "type=bind,src=" + project + ",dst=/usr/app/dbt,readonly", } // -e NAME with no value: docker reads the value from its own environment, // so no value, the password included, appears on a command line. @@ -101,26 +154,64 @@ func Build(ctx context.Context, projectDir, dsn string) error { } var out bytes.Buffer cmd.Stdout, cmd.Stderr = &out, &out - err = cmd.Run() - - // A killed docker client leaves its container running: remove it. This - // covers both the timeout and a cancelled ctx (the dashboard server - // shutting down). - if ctx.Err() != nil { - _ = exec.Command("docker", "rm", "-f", name).Run() - if errors.Is(ctx.Err(), context.Canceled) { - return fmt.Errorf("dbt build cancelled:\n%s", tail(out.String(), outputTail)) + runErr := cmd.Run() + + failure := verdict(ctx.Err(), runErr, out.String(), want) + if failure == "" { + removeContainer(name) + return nil + } + logPath, logErr := saveLog(name, cfg.Database, out.Bytes()) + removeContainer(name) + msg := failure + ":\n" + tail(out.String(), outputTail) + if logErr != nil { + return fmt.Errorf("%s\n(dbt.log not saved: %v)", msg, logErr) + } + return fmt.Errorf("%s\nfull log: %s", msg, logPath) +} + +// verdict says why a finished build failed, or returns "" when it passed: +// the context still live, dbt exited 0, and its summary counts exactly want +// nodes. A build that exits 0 but ran fewer nodes, for example because dbt +// parsed a file caught mid-save, is a failure. +func verdict(ctxErr, runErr error, output string, want int) string { + switch { + case errors.Is(ctxErr, context.Canceled): + return "dbt build cancelled" + case ctxErr != nil: + return fmt.Sprintf("dbt build did not finish within %s", Timeout) + case runErr != nil: + var ee *exec.ExitError + if !errors.As(runErr, &ee) { + return fmt.Sprintf("run dbt container: %v", runErr) } - return fmt.Errorf("dbt build did not finish within %s:\n%s", Timeout, tail(out.String(), outputTail)) + return fmt.Sprintf("dbt build exited %d", ee.ExitCode()) } - if err == nil { - return nil + got, ok := executedNodes(output) + if !ok { + return "dbt build printed no node total" } - var ee *exec.ExitError - if errors.As(err, &ee) { - return fmt.Errorf("dbt build exited %d:\n%s", ee.ExitCode(), tail(out.String(), outputTail)) + if got != want { + return fmt.Sprintf("dbt build ran %d nodes, the project has %d", got, want) } - return fmt.Errorf("run dbt container: %w", err) + return "" +} + +// target returns the docker network the dbt container joins and the +// Postgres host name on it: the compose network and service by default, or +// RC_DBT_NETWORK and RC_DBT_HOST when both are set (tests and CI only). +func target(ctx context.Context) (network, host string, err error) { + network, host = os.Getenv(envNetwork), os.Getenv(envHost) + switch { + case network == "" && host == "": + network, err = composeNetwork(ctx) + return network, composeService, err + case network == "" || host == "": + return "", "", fmt.Errorf("set both %s and %s, or neither", envNetwork, envHost) + case network == "host" || network == "none" || strings.HasPrefix(network, "container:"): + return "", "", fmt.Errorf("%s=%s: dbt never uses the host network, and needs a network to reach Postgres", envNetwork, network) + } + return network, host, nil } // isLocal reports whether host is this machine's loopback address. @@ -147,6 +238,221 @@ func composeNetwork(ctx context.Context) (string, error) { return network, nil } +// removeStale removes every dbt container of database db, running or not, +// with its log volume. Such a container is left only when the process that +// started it was killed: Build removes its own container on every path. +func removeStale(ctx context.Context, db string) error { + out, err := exec.CommandContext(ctx, "docker", "ps", "-aq", + "--filter", "label="+labelDBT+"=true", + "--filter", "label="+labelDatabase+"="+db).Output() + if err != nil { + return fmt.Errorf("list stale dbt containers: %w", err) + } + ids := strings.Fields(string(out)) + if len(ids) == 0 { + return nil + } + if out, err := exec.CommandContext(ctx, "docker", append([]string{"rm", "-f", "-v"}, ids...)...).CombinedOutput(); err != nil { + return fmt.Errorf("remove stale dbt containers: %w: %s", err, strings.TrimSpace(string(out))) + } + return nil +} + +// removeContainer removes a build's container and its log volume. It runs +// with its own context, because the build's context may already be over. +func removeContainer(name string) { + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) + defer cancel() + _ = exec.CommandContext(ctx, "docker", "rm", "-f", "-v", name).Run() +} + +// extractPrefix starts the name of every extracted project directory. +const extractPrefix = "rootcause-dbt-" + +// removeOldExtracts deletes extracted project directories that a killed +// build left behind. A build lasts at most Timeout, so a directory older than +// twice that belongs to no running build, whatever its database. +func removeOldExtracts() { + matches, _ := filepath.Glob(filepath.Join(os.TempDir(), extractPrefix+"*")) + for _, m := range matches { + if fi, err := os.Stat(m); err == nil && fi.IsDir() && time.Since(fi.ModTime()) > 2*Timeout { + _ = os.RemoveAll(m) + } + } +} + +// extract writes the project in fsys to a new temporary directory and returns +// its path. Directories are 0755 and files 0644: under userns-remap the +// container's root is an unprivileged host user, which reads them as "other". +func extract(fsys fs.FS) (string, error) { + dir, err := os.MkdirTemp("", extractPrefix) + if err != nil { + return "", err + } + err = fs.WalkDir(fsys, ".", func(p string, d fs.DirEntry, err error) error { + if err != nil { + return err + } + dst := filepath.Join(dir, filepath.FromSlash(p)) + if d.IsDir() { + return os.MkdirAll(dst, 0o755) + } + data, err := fs.ReadFile(fsys, p) + if err != nil { + return err + } + return os.WriteFile(dst, data, 0o644) + }) + if err == nil { + err = os.Chmod(dir, 0o755) // MkdirTemp creates it 0700 + } + if err != nil { + os.RemoveAll(dir) + return "", err + } + return dir, nil +} + +// LogDir is where the dbt.log of failed builds is saved. +func LogDir() (string, error) { + cache, err := os.UserCacheDir() + if err != nil { + return "", err + } + return filepath.Join(cache, "rootcause", "dbt-logs"), nil +} + +// saveLog copies dbt.log out of the container into LogDir and returns its +// path. If the container holds no dbt.log (dbt never started), the docker +// output is saved instead. Only the newest keepLogs files are kept. +func saveLog(name, db string, output []byte) (string, error) { + dir, err := LogDir() + if err != nil { + return "", err + } + if err := os.MkdirAll(dir, 0o700); err != nil { + return "", err + } + dst := filepath.Join(dir, time.Now().UTC().Format("20060102T150405Z")+"-"+db+"-"+strings.TrimPrefix(name, "rootcause-dbt-")+".log") + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) + defer cancel() + if err := exec.CommandContext(ctx, "docker", "cp", name+":/tmp/logs/dbt.log", dst).Run(); err != nil { + if err := os.WriteFile(dst, output, 0o600); err != nil { + return "", err + } + } + pruneLogs(dir) + return dst, nil +} + +// pruneLogs removes all but the newest keepLogs files in dir. Names start +// with a UTC timestamp, so name order is age order. +func pruneLogs(dir string) { + entries, err := os.ReadDir(dir) + if err != nil { + return + } + var names []string + for _, e := range entries { + if !e.IsDir() && strings.HasSuffix(e.Name(), ".log") { + names = append(names, e.Name()) + } + } + slices.Sort(names) + for len(names) > keepLogs { + _ = os.Remove(filepath.Join(dir, names[0])) + names = names[1:] + } +} + +// doneLine matches dbt's summary, for example +// "Done. PASS=55 WARN=0 ERROR=0 SKIP=0 NO-OP=0 TOTAL=55". +var doneLine = regexp.MustCompile(`(?m)Done\. .*\bTOTAL=(\d+)`) + +// executedNodes returns the TOTAL from dbt's last summary line in output. +func executedNodes(output string) (int, bool) { + m := doneLine.FindAllStringSubmatch(output, -1) + if m == nil { + return 0, false + } + n, err := strconv.Atoi(m[len(m)-1][1]) + return n, err == nil +} + +// ExpectedNodes counts the nodes `dbt build` runs for the project in fsys: +// one per model (.sql under models/), one per singular test (.sql under +// tests/) and one per generic test listed in a properties file (.yml under +// models/), on a model, a source table or one of their columns. It reads the +// files itself, so a dbt run that silently skips nodes shows up as a +// mismatch. +func ExpectedNodes(fsys fs.FS) (int, error) { + n := 0 + err := fs.WalkDir(fsys, ".", func(p string, d fs.DirEntry, err error) error { + if err != nil || d.IsDir() { + return err + } + inModels, inTests := strings.HasPrefix(p, "models/"), strings.HasPrefix(p, "tests/") + switch ext := path.Ext(p); { + case ext == ".sql" && (inModels || inTests): + n++ + case (ext == ".yml" || ext == ".yaml") && inModels: + data, err := fs.ReadFile(fsys, p) + if err != nil { + return err + } + tests, err := genericTests(data) + if err != nil { + return fmt.Errorf("%s: %w", p, err) + } + n += tests + } + return nil + }) + return n, err +} + +// testList is a node's generic tests. dbt accepts both keys. +type testList struct { + Tests []yaml.Node `yaml:"tests"` + DataTests []yaml.Node `yaml:"data_tests"` +} + +type testedNode struct { + testList `yaml:",inline"` + Columns []testList `yaml:"columns"` +} + +func (t testedNode) count() int { + n := len(t.Tests) + len(t.DataTests) + for _, c := range t.Columns { + n += len(c.Tests) + len(c.DataTests) + } + return n +} + +// genericTests counts the generic tests in one properties file. +func genericTests(data []byte) (int, error) { + var f struct { + Models []testedNode `yaml:"models"` + Sources []struct { + Tables []testedNode `yaml:"tables"` + } `yaml:"sources"` + } + if err := yaml.Unmarshal(data, &f); err != nil { + return 0, err + } + n := 0 + for _, m := range f.Models { + n += m.count() + } + for _, s := range f.Sources { + for _, t := range s.Tables { + n += t.count() + } + } + return n, nil +} + // tail returns the last n lines of s. func tail(s string, n int) string { lines := strings.Split(strings.TrimRight(s, "\n"), "\n") diff --git a/internal/dbt/dbt_test.go b/internal/dbt/dbt_test.go index 334c0e3..6e7518d 100644 --- a/internal/dbt/dbt_test.go +++ b/internal/dbt/dbt_test.go @@ -3,7 +3,16 @@ package dbt import ( "context" "errors" + "fmt" + "io/fs" + "os" + "os/exec" + "path/filepath" + "strings" "testing" + "testing/fstest" + + dbtproject "github.com/DIZAMU/RootCause/dbt" ) // TestBuildRefusesOtherHosts checks that a DSN for any database but the @@ -13,12 +22,166 @@ func TestBuildRefusesOtherHosts(t *testing.T) { "postgres://u:p@db.example.com:5432/rootcause_test", "postgres://u:p@127.0.0.1:5433/rootcause_test", } { - if err := Build(context.Background(), t.TempDir(), dsn); !errors.Is(err, ErrUnsupportedDatabase) { + if err := Build(context.Background(), dsn); !errors.Is(err, ErrUnsupportedDatabase) { t.Errorf("%s: err = %v, want ErrUnsupportedDatabase", dsn, err) } } } -// Build itself, against the test database, is covered by the selector's -// TestScriptedSelection, the only test that runs dbt. Two dbt builds at once -// would rebuild the same tables, so the build is tested in one place. +// TestTargetOverride checks RC_DBT_NETWORK and RC_DBT_HOST: both or neither, +// and never the host network or no network. +func TestTargetOverride(t *testing.T) { + t.Setenv(envNetwork, "ci_net") + t.Setenv(envHost, "postgres") + if n, h, err := target(context.Background()); err != nil || n != "ci_net" || h != "postgres" { + t.Errorf("target = %q, %q, %v; want ci_net, postgres", n, h, err) + } + for _, bad := range [][2]string{{"ci_net", ""}, {"", "postgres"}, {"host", "postgres"}, {"none", "postgres"}, {"container:pg", "postgres"}} { + t.Setenv(envNetwork, bad[0]) + t.Setenv(envHost, bad[1]) + if _, _, err := target(context.Background()); err == nil { + t.Errorf("target with network %q host %q: no error", bad[0], bad[1]) + } + } +} + +// TestExpectedNodesOfProject pins the node count of the embedded project. +// A change to the models or tests must update it on purpose. +func TestExpectedNodesOfProject(t *testing.T) { + n, err := ExpectedNodes(dbtproject.FS) + if err != nil { + t.Fatal(err) + } + if n != 55 { + t.Errorf("ExpectedNodes = %d, want 55 (8 models, 3 singular tests, 44 generic tests)", n) + } +} + +// TestExpectedNodesCounts covers each place a node can come from. +func TestExpectedNodesCounts(t *testing.T) { + fsys := fstest.MapFS{ + "dbt_project.yml": {Data: []byte("name: x\n")}, + "models/a/m1.sql": {Data: []byte("select 1")}, + "models/b/m2.sql": {Data: []byte("select 1")}, + "models/b/notes.md": {Data: []byte("not a node")}, + "tests/one_row.sql": {Data: []byte("select 1 where false")}, + "analyses/ignore.sql": {Data: []byte("select 1")}, + "models/a/schema.yml": {Data: []byte(` +version: 2 +models: + - name: m1 + data_tests: [one_model_test] + columns: + - name: id + tests: [unique, not_null] + - name: kind + data_tests: + - accepted_values: + values: [a, b] +`)}, + "models/b/sources.yaml": {Data: []byte(` +version: 2 +sources: + - name: engine + tables: + - name: session + columns: + - name: id + tests: [not_null] + - name: event +`)}, + } + // 2 models + 1 singular test + 4 generic tests on m1 + 1 on a source. + if n, err := ExpectedNodes(fsys); err != nil || n != 8 { + t.Errorf("ExpectedNodes = %d, %v; want 8", n, err) + } +} + +// TestVerdict checks every outcome, including a run that exits 0 but ran +// fewer nodes than the project has (the review's TOTAL=36 instead of 55). +func TestVerdict(t *testing.T) { + exit1 := exec.Command("sh", "-c", "exit 1").Run() + full := "12:00:00 Done. PASS=55 WARN=0 ERROR=0 SKIP=0 NO-OP=0 TOTAL=55\n" + cases := []struct { + name string + ctxErr, runErr error + output, want string + }{ + {"pass", nil, nil, full, ""}, + {"shrunken run", nil, nil, "Done. PASS=36 WARN=0 ERROR=0 SKIP=0 NO-OP=0 TOTAL=36\n", "ran 36 nodes, the project has 55"}, + {"no summary", nil, nil, "Encountered an error\n", "printed no node total"}, + {"exit 1", nil, exit1, full, "exited 1"}, + {"cancelled", context.Canceled, exit1, "", "cancelled"}, + {"timeout", context.DeadlineExceeded, exit1, "", "did not finish"}, + } + for _, c := range cases { + got := verdict(c.ctxErr, c.runErr, c.output, 55) + if (c.want == "") != (got == "") || !strings.Contains(got, c.want) { + t.Errorf("%s: verdict = %q, want %q", c.name, got, c.want) + } + } +} + +// TestExtract checks that the extracted project matches the embedded one and +// is readable by any user, as the userns-remapped container root needs. +func TestExtract(t *testing.T) { + dir, err := extract(dbtproject.FS) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { os.RemoveAll(dir) }) + files := 0 + err = fs.WalkDir(dbtproject.FS, ".", func(p string, d fs.DirEntry, err error) error { + if err != nil { + return err + } + fi, err := os.Stat(filepath.Join(dir, filepath.FromSlash(p))) + if err != nil { + return err + } + if d.IsDir() { + if fi.Mode().Perm() != 0o755 { + t.Errorf("%s: mode %v, want 0755", p, fi.Mode().Perm()) + } + return nil + } + files++ + if fi.Mode().Perm() != 0o644 { + t.Errorf("%s: mode %v, want 0644", p, fi.Mode().Perm()) + } + want, _ := fs.ReadFile(dbtproject.FS, p) + got, _ := os.ReadFile(filepath.Join(dir, filepath.FromSlash(p))) + if string(got) != string(want) { + t.Errorf("%s: extracted content differs", p) + } + return nil + }) + if err != nil { + t.Fatal(err) + } + if files == 0 { + t.Fatal("nothing extracted") + } + if _, err := os.Stat(filepath.Join(dir, "embed.go")); err == nil { + t.Error("embed.go was extracted; only the dbt project belongs in the mount") + } +} + +// TestPruneLogs keeps only the newest keepLogs logs. +func TestPruneLogs(t *testing.T) { + dir := t.TempDir() + for i := 0; i < keepLogs+5; i++ { + name := filepath.Join(dir, fmt.Sprintf("202601%02dT000000Z-db-x.log", i+1)) + if err := os.WriteFile(name, nil, 0o600); err != nil { + t.Fatal(err) + } + } + pruneLogs(dir) + entries, _ := os.ReadDir(dir) + if len(entries) != keepLogs { + t.Fatalf("%d logs left, want %d", len(entries), keepLogs) + } + if entries[0].Name() != "20260106T000000Z-db-x.log" { + t.Errorf("oldest kept = %s, want the 6th oldest", entries[0].Name()) + } +} diff --git a/internal/dbt/integration_test.go b/internal/dbt/integration_test.go new file mode 100644 index 0000000..698c657 --- /dev/null +++ b/internal/dbt/integration_test.go @@ -0,0 +1,220 @@ +package dbt_test + +import ( + "context" + "math/rand" + "net/url" + "os" + "os/exec" + "regexp" + "strconv" + "strings" + "testing" + "time" + + "github.com/DIZAMU/RootCause/internal/dbt" + "github.com/DIZAMU/RootCause/internal/play" + "github.com/DIZAMU/RootCause/internal/store" + "github.com/DIZAMU/RootCause/internal/testenv" + "github.com/jackc/pgx/v5/pgconn" +) + +// helperDSN, when set, turns TestHelperBuild into a dbt build in a child +// process that the parent test kills, like a crashed engine. +const helperDSN = "RC_DBT_HELPER_DSN" + +// TestHelperBuild is not a test on its own: it runs one build when a parent +// test starts the test binary as a child process with RC_DBT_HELPER_DSN set. +func TestHelperBuild(t *testing.T) { + dsn := os.Getenv(helperDSN) + if dsn == "" { + return + } + _ = dbt.Build(context.Background(), dsn) +} + +// buildWorld is a database of the test's own, migrated and empty. +type buildWorld struct { + t *testing.T + st *store.Store + dsn string + db string +} + +func newBuildWorld(t *testing.T) *buildWorld { + t.Helper() + testenv.Docker(t) + dsn := testenv.OwnDatabase(t, "dbt") + cfg, err := pgconn.ParseConfig(dsn) + if err != nil { + t.Fatal(err) + } + // Logs of failed builds go to a temporary cache, not the user's. + t.Setenv("XDG_CACHE_HOME", t.TempDir()) + return &buildWorld{t: t, st: testenv.Store(t, dsn), dsn: dsn, db: cfg.Database} +} + +// build runs the production path: dbt under the statistics lock. +func (w *buildWorld) build(ctx context.Context) error { + return play.BuildStats(ctx, w.st, w.dsn) +} + +// containers lists this database's dbt containers, running or not. +func (w *buildWorld) containers() []string { + w.t.Helper() + out, err := exec.Command("docker", "ps", "-aq", + "--filter", "label=rootcause.dbt=true", + "--filter", "label=rootcause.database="+w.db).Output() + if err != nil { + w.t.Fatalf("docker ps: %v", err) + } + return strings.Fields(string(out)) +} + +// mustBuild builds and checks that no dbt container of this database is +// left afterwards. +func (w *buildWorld) mustBuild(step string) { + w.t.Helper() + ctx, cancel := context.WithTimeout(context.Background(), 3*time.Minute) + defer cancel() + if err := w.build(ctx); err != nil { + w.t.Fatalf("%s: build: %v", step, err) + } + if left := w.containers(); len(left) > 0 { + w.t.Fatalf("%s: dbt containers left after the build: %v", step, left) + } +} + +// killedChild starts a build in a child process, kills the child (not the +// container) after delay, and returns. If the child was killed while its +// container ran, that container is now stale, as after an engine crash. +func (w *buildWorld) killedChild(delay time.Duration) { + w.t.Helper() + cmd := exec.Command(os.Args[0], "-test.run=^TestHelperBuild$", "-test.count=1") + cmd.Env = append(os.Environ(), helperDSN+"="+w.dsn) + if err := cmd.Start(); err != nil { + w.t.Fatalf("start child build: %v", err) + } + time.Sleep(delay) // the random kill point itself, not a wait for readiness + _ = cmd.Process.Kill() + _ = cmd.Wait() +} + +// waitForContainer polls until this database has a running dbt container. +func (w *buildWorld) waitForContainer(timeout time.Duration) bool { + deadline := time.Now().Add(timeout) + for time.Now().Before(deadline) { + out, _ := exec.Command("docker", "ps", "-q", "--filter", "status=running", + "--filter", "label=rootcause.dbt=true", "--filter", "label=rootcause.database="+w.db).Output() + if len(strings.TrimSpace(string(out))) > 0 { + return true + } + time.Sleep(100 * time.Millisecond) + } + return false +} + +// TestBuildRecoversFromKilledEngine: an engine killed during a build leaves +// its container running. The next build removes it and succeeds, and every +// build leaves no container behind. +func TestBuildRecoversFromKilledEngine(t *testing.T) { + w := newBuildWorld(t) + w.mustBuild("first build") + + cmd := exec.Command(os.Args[0], "-test.run=^TestHelperBuild$", "-test.count=1") + cmd.Env = append(os.Environ(), helperDSN+"="+w.dsn) + if err := cmd.Start(); err != nil { + t.Fatalf("start child build: %v", err) + } + if !w.waitForContainer(time.Minute) { + _ = cmd.Process.Kill() + _ = cmd.Wait() + t.Fatal("the child build's container never ran") + } + _ = cmd.Process.Kill() + _ = cmd.Wait() + if len(w.containers()) == 0 { + t.Fatal("no stale container after killing the child; the test proves nothing") + } + w.mustBuild("build after the kill") +} + +// TestBuildFailureKeepsLog: a failed build saves the full dbt.log under the +// cache directory, names it in the error, and removes its container. +func TestBuildFailureKeepsLog(t *testing.T) { + w := newBuildWorld(t) + u, err := url.Parse(w.dsn) + if err != nil { + t.Fatal(err) + } + u.User = url.UserPassword(u.User.Username(), "wrong-password") + + ctx, cancel := context.WithTimeout(context.Background(), 3*time.Minute) + defer cancel() + err = play.BuildStats(ctx, w.st, u.String()) + if err == nil { + t.Fatal("build with a wrong password succeeded") + } + if strings.Contains(err.Error(), "wrong-password") { + t.Error("the error shows the password") + } + m := regexp.MustCompile(`full log: (\S+)`).FindStringSubmatch(err.Error()) + if m == nil { + t.Fatalf("error names no log: %v", err) + } + dir, _ := dbt.LogDir() + if !strings.HasPrefix(m[1], dir+string(os.PathSeparator)) { + t.Errorf("log %s is not under %s", m[1], dir) + } + data, rerr := os.ReadFile(m[1]) + if rerr != nil { + t.Fatalf("read saved log: %v", rerr) + } + // dbt.log is dbt's debug log, far longer than the console output. + if !strings.Contains(string(data), "Running with dbt=") || !strings.Contains(string(data), "[debug]") { + t.Errorf("saved log is not dbt.log:\n%s", data) + } + if left := w.containers(); len(left) > 0 { + t.Errorf("failed build left containers: %v", left) + } + w.mustBuild("build after the failure") +} + +// TestBuildStress kills builds at random points, half by killing the +// engine's process and half by cancelling the build, and checks that the +// next build always succeeds and runs every node. It runs only with +// RC_DBT_STRESS=1; RC_DBT_STRESS_ROUNDS sets the rounds (default 10). +func TestBuildStress(t *testing.T) { + if os.Getenv("RC_DBT_STRESS") != "1" { + t.Skip("set RC_DBT_STRESS=1 to run the dbt stress test") + } + rounds := 10 + if s := os.Getenv("RC_DBT_STRESS_ROUNDS"); s != "" { + n, err := strconv.Atoi(s) + if err != nil || n < 1 { + t.Fatalf("RC_DBT_STRESS_ROUNDS=%q", s) + } + rounds = n + } + w := newBuildWorld(t) + start := time.Now() + w.mustBuild("first build") + full := time.Since(start) + seed := time.Now().UnixNano() + t.Logf("a full build takes %s; seed %d", full.Round(time.Millisecond), seed) + rng := rand.New(rand.NewSource(seed)) + + for i := 1; i <= rounds; i++ { + delay := time.Duration(rng.Int63n(int64(full))) + if i%2 == 1 { + t.Logf("round %d: kill the engine after %s", i, delay.Round(time.Millisecond)) + w.killedChild(delay) + } else { + t.Logf("round %d: cancel the build after %s", i, delay.Round(time.Millisecond)) + ctx, cancel := context.WithTimeout(context.Background(), delay) + _ = w.build(ctx) + cancel() + } + w.mustBuild("rebuild in round " + strconv.Itoa(i)) + } +} diff --git a/internal/play/play.go b/internal/play/play.go index d588317..1f3c21e 100644 --- a/internal/play/play.go +++ b/internal/play/play.go @@ -103,8 +103,8 @@ func Seed(exercisesDir string, pick selector.Pick, random func() int64) (int64, // BuildStats rebuilds the marts with dbt (spec section 9.3) while holding the // store's stats lock, so the CLI and the dashboard never build at once. -func BuildStats(ctx context.Context, st *store.Store, projectDir, dsn string) error { - return st.WithStatsLock(ctx, func() error { return dbt.Build(ctx, projectDir, dsn) }) +func BuildStats(ctx context.Context, st *store.Store, dsn string) error { + return st.WithStatsLock(ctx, func() error { return dbt.Build(ctx, dsn) }) } // ErrNoExercises means the exercises folder cannot be read. diff --git a/internal/runtime/docker/docker_test.go b/internal/runtime/docker/docker_test.go index e601874..b72b343 100644 --- a/internal/runtime/docker/docker_test.go +++ b/internal/runtime/docker/docker_test.go @@ -12,18 +12,18 @@ import ( "time" "github.com/DIZAMU/RootCause/internal/runtime" + "github.com/DIZAMU/RootCause/internal/testenv" ) // newOrSkip returns a Docker runtime, or skips the test if docker is not -// usable here (e.g. CI without a daemon). These are integration tests. +// usable here. With RC_REQUIRE_DOCKER=1 (CI) that fails the test instead. +// These are integration tests. func newOrSkip(t *testing.T) *Runtime { t.Helper() + testenv.Docker(t) r, err := New() if err != nil { - t.Skip("docker CLI not available:", err) - } - if err := exec.Command("docker", "info").Run(); err != nil { - t.Skip("docker daemon not reachable:", err) + testenv.NoDocker(t, "docker runtime:", err) } return r } diff --git a/internal/selector/integration_test.go b/internal/selector/integration_test.go index f564678..69b5ff7 100644 --- a/internal/selector/integration_test.go +++ b/internal/selector/integration_test.go @@ -2,23 +2,21 @@ package selector_test import ( "context" - "errors" "fmt" "maps" "math/rand" - "os/exec" "path/filepath" "slices" "testing" "time" - "github.com/DIZAMU/RootCause/internal/dbt" "github.com/DIZAMU/RootCause/internal/exercise" "github.com/DIZAMU/RootCause/internal/play" "github.com/DIZAMU/RootCause/internal/runtime/scripted" "github.com/DIZAMU/RootCause/internal/selector" "github.com/DIZAMU/RootCause/internal/session" "github.com/DIZAMU/RootCause/internal/store" + "github.com/DIZAMU/RootCause/internal/testenv" ) // result is how a scripted session ends. @@ -40,7 +38,6 @@ type world struct { rt *scripted.Runtime learner int exDir string - project string dsn string catalog []selector.Exercise lastSeed map[string]int64 @@ -105,7 +102,7 @@ func (w *world) pick(refresh bool) (selector.Pick, selector.Loaded) { w.t.Helper() deps := selector.Deps{Store: w.st, Level: w.eng.TopicLevel} if refresh { - deps.Refresh = func(ctx context.Context) error { return dbt.Build(ctx, w.project, w.dsn) } + deps.Refresh = func(ctx context.Context) error { return play.BuildStats(ctx, w.st, w.dsn) } } loaded, err := selector.Load(w.ctx, deps, w.learner, w.catalog) if err != nil { @@ -138,36 +135,23 @@ func (w *world) expect(step, wantID string, wantReason selector.Reason) selector // after a scripted series of sessions, the selector picks the expected // exercises. Sessions run through the real engine and store, the marts are // built by the real dbt container, and picks go through the real Load and -// Next. It needs Docker and RC_TEST_DATABASE_URL, and uses its own learner. +// Next. It needs Docker and RC_TEST_DATABASE_URL. dbt reads every row in its +// database, so the test runs in a database of its own, which no other test +// package writes to while it runs. func TestScriptedSelection(t *testing.T) { - dsn := store.TestDSN() - if dsn == "" { - t.Skip("RC_TEST_DATABASE_URL not set to a database ending in _test") - } - if err := exec.Command("docker", "info").Run(); err != nil { - t.Skip("docker daemon not reachable:", err) - } + testenv.Docker(t) + dsn := testenv.OwnDatabase(t, "selector") ctx, cancel := context.WithTimeout(context.Background(), 10*time.Minute) defer cancel() - st, err := store.Open(ctx, dsn) - if err != nil { - t.Skip("db not reachable:", err) - } - t.Cleanup(st.Close) - if err := st.Migrate(ctx); err != nil { - t.Fatalf("migrate: %v", err) - } - project, _ := filepath.Abs("../../dbt") - if err := dbt.Build(ctx, project, dsn); errors.Is(err, dbt.ErrUnsupportedDatabase) { - t.Skip("dbt needs the compose Postgres:", err) - } else if err != nil { + st := testenv.Store(t, dsn) + if err := play.BuildStats(ctx, st, dsn); err != nil { t.Fatalf("first dbt build: %v", err) } exDir, _ := filepath.Abs("../../exercises") rt := &scripted.Runtime{} w := &world{ - t: t, ctx: ctx, st: st, rt: rt, exDir: exDir, project: project, dsn: dsn, + t: t, ctx: ctx, st: st, rt: rt, exDir: exDir, dsn: dsn, eng: session.New(rt, rt, nil, st, exDir), learner: testLearner(t, st), catalog: loadCatalog(t, exDir), @@ -276,8 +260,8 @@ func TestScriptedSelection(t *testing.T) { if loaded.Input.Review != nil { t.Errorf("fallback has a review queue: %v", loaded.Input.Review) } - // Leave the marts built for whoever runs next. - if err := dbt.Build(ctx, project, dsn); err != nil { + // The marts build again from scratch. + if err := play.BuildStats(ctx, st, dsn); err != nil { t.Fatalf("rebuild marts: %v", err) } } diff --git a/internal/server/server_test.go b/internal/server/server_test.go index f391950..c865105 100644 --- a/internal/server/server_test.go +++ b/internal/server/server_test.go @@ -23,6 +23,7 @@ import ( "github.com/DIZAMU/RootCause/internal/server" "github.com/DIZAMU/RootCause/internal/session" "github.com/DIZAMU/RootCause/internal/store" + "github.com/DIZAMU/RootCause/internal/testenv" "github.com/DIZAMU/RootCause/locales" ) @@ -233,19 +234,9 @@ type world struct { func newWorld(t *testing.T, assets bool) *world { t.Helper() - dsn := store.TestDSN() - if dsn == "" { - t.Skip("RC_TEST_DATABASE_URL not set to a database ending in _test") - } + dsn := testenv.DSN(t) ctx := context.Background() - st, err := store.Open(ctx, dsn) - if err != nil { - t.Skip("db not reachable:", err) - } - t.Cleanup(st.Close) - if err := st.Migrate(ctx); err != nil { - t.Fatalf("migrate: %v", err) - } + st := testenv.Store(t, dsn) bundle, err := i18n.Load(locales.FS) if err != nil { t.Fatal(err) diff --git a/internal/session/engine_test.go b/internal/session/engine_test.go index 74aeea7..fa73b65 100644 --- a/internal/session/engine_test.go +++ b/internal/session/engine_test.go @@ -3,13 +3,12 @@ package session import ( "context" "os" - "os/exec" "path/filepath" "testing" "time" "github.com/DIZAMU/RootCause/internal/runtime/docker" - "github.com/DIZAMU/RootCause/internal/store" + "github.com/DIZAMU/RootCause/internal/testenv" ) // TestPlaySvc01 drives svc-01 through the lifecycle end to end: start (fault @@ -17,28 +16,16 @@ import ( // asserts the right events are logged. It needs both Docker and Postgres; it // skips otherwise. func TestPlaySvc01(t *testing.T) { - dsn := store.TestDSN() - if dsn == "" { - t.Skip("RC_TEST_DATABASE_URL not set to a database ending in _test") - } - if err := exec.Command("docker", "info").Run(); err != nil { - t.Skip("docker not reachable:", err) - } + dsn := testenv.DSN(t) + testenv.Docker(t) ctx, cancel := context.WithTimeout(context.Background(), 3*time.Minute) defer cancel() - st, err := store.Open(ctx, dsn) - if err != nil { - t.Skip("db not reachable:", err) - } - t.Cleanup(st.Close) - if err := st.Migrate(ctx); err != nil { - t.Fatalf("migrate: %v", err) - } + st := testenv.Store(t, dsn) rt, err := docker.New() if err != nil { - t.Skip("docker runtime:", err) + testenv.NoDocker(t, "docker runtime:", err) } learnerID := testLearner(t, st, rt) exDir, _ := filepath.Abs("../../exercises") diff --git a/internal/session/reaper_test.go b/internal/session/reaper_test.go index 6e69dfb..0a29650 100644 --- a/internal/session/reaper_test.go +++ b/internal/session/reaper_test.go @@ -2,41 +2,28 @@ package session import ( "context" - "os/exec" "path/filepath" "testing" "time" "github.com/DIZAMU/RootCause/internal/exercise" "github.com/DIZAMU/RootCause/internal/runtime/docker" - "github.com/DIZAMU/RootCause/internal/store" + "github.com/DIZAMU/RootCause/internal/testenv" ) // TestReaperExpiresDueSession starts a real session, backdates its TTL, and // checks the reaper expires it: status expired, sandbox gone, expired event. func TestReaperExpiresDueSession(t *testing.T) { - dsn := store.TestDSN() - if dsn == "" { - t.Skip("RC_TEST_DATABASE_URL not set to a database ending in _test") - } - if err := exec.Command("docker", "info").Run(); err != nil { - t.Skip("docker not reachable:", err) - } + dsn := testenv.DSN(t) + testenv.Docker(t) ctx, cancel := context.WithTimeout(context.Background(), 2*time.Minute) defer cancel() - st, err := store.Open(ctx, dsn) - if err != nil { - t.Skip("db not reachable:", err) - } - t.Cleanup(st.Close) - if err := st.Migrate(ctx); err != nil { - t.Fatalf("migrate: %v", err) - } + st := testenv.Store(t, dsn) rt, err := docker.New() if err != nil { - t.Skip("docker runtime:", err) + testenv.NoDocker(t, "docker runtime:", err) } learnerID := testLearner(t, st, rt) exDir, _ := filepath.Abs("../../exercises") @@ -96,27 +83,15 @@ func TestReaperExpiresDueSession(t *testing.T) { // this database: the selftest sandbox, the sandbox owned by another database // and the sandbox of an active session all survive. func TestReaperReclaimsOrphans(t *testing.T) { - dsn := store.TestDSN() - if dsn == "" { - t.Skip("RC_TEST_DATABASE_URL not set to a database ending in _test") - } - if err := exec.Command("docker", "info").Run(); err != nil { - t.Skip("docker not reachable:", err) - } + dsn := testenv.DSN(t) + testenv.Docker(t) ctx, cancel := context.WithTimeout(context.Background(), 2*time.Minute) defer cancel() - st, err := store.Open(ctx, dsn) - if err != nil { - t.Skip("db not reachable:", err) - } - t.Cleanup(st.Close) - if err := st.Migrate(ctx); err != nil { - t.Fatalf("migrate: %v", err) - } + st := testenv.Store(t, dsn) rt, err := docker.New() if err != nil { - t.Skip("docker runtime:", err) + testenv.NoDocker(t, "docker runtime:", err) } learnerID := testLearner(t, st, rt) exDir, _ := filepath.Abs("../../exercises") diff --git a/internal/session/reset_test.go b/internal/session/reset_test.go index 7faef47..1838833 100644 --- a/internal/session/reset_test.go +++ b/internal/session/reset_test.go @@ -9,6 +9,7 @@ import ( "github.com/DIZAMU/RootCause/internal/runtime" "github.com/DIZAMU/RootCause/internal/store" + "github.com/DIZAMU/RootCause/internal/testenv" ) // brokenRuntime creates sandboxes that pass the readiness probe, but every @@ -37,20 +38,10 @@ func (b *brokenRuntime) List(context.Context) ([]runtime.Sandbox, error) { retur // prepared voids the session, logs a voided event with the reason, and // destroys the half-built sandbox. No Docker needed; it needs the test DB. func TestResetVoidsOnFailure(t *testing.T) { - dsn := store.TestDSN() - if dsn == "" { - t.Skip("RC_TEST_DATABASE_URL not set to a database ending in _test") - } + dsn := testenv.DSN(t) ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) defer cancel() - st, err := store.Open(ctx, dsn) - if err != nil { - t.Skip("db not reachable:", err) - } - t.Cleanup(st.Close) - if err := st.Migrate(ctx); err != nil { - t.Fatalf("migrate: %v", err) - } + st := testenv.Store(t, dsn) rt := &brokenRuntime{} learnerID := testLearner(t, st, rt) diff --git a/internal/store/migrate_test.go b/internal/store/migrate_test.go index b982e60..7ac7804 100644 --- a/internal/store/migrate_test.go +++ b/internal/store/migrate_test.go @@ -28,7 +28,8 @@ func freshSchemaStores(t *testing.T, n int) []*Store { }) // Point new connections at the empty schema only. - u, err := url.Parse(TestDSN()) + dsn, _ := TestDSN() // openOrSkip above already checked it + u, err := url.Parse(dsn) if err != nil { t.Fatalf("parse dsn: %v", err) } diff --git a/internal/store/store.go b/internal/store/store.go index dde217c..b7740e3 100644 --- a/internal/store/store.go +++ b/internal/store/store.go @@ -4,6 +4,7 @@ package store import ( "context" + "errors" "fmt" "os" "strings" @@ -52,21 +53,26 @@ func (s *Store) Pool() *pgxpool.Pool { return s.pool } // DSNFromEnv returns the database DSN from RC_DATABASE_URL, or "" if unset. func DSNFromEnv() string { return os.Getenv("RC_DATABASE_URL") } +// ErrUnsafeTestDSN means RC_TEST_DATABASE_URL is set but names a database +// whose name does not end in "_test". +var ErrUnsafeTestDSN = errors.New("RC_TEST_DATABASE_URL must name a database ending in _test") + // TestDSN returns the DSN integration tests may use: RC_TEST_DATABASE_URL, and // only if its database name ends in "_test". Tests create sessions, change the // language and re-sync exercises, so they must never touch the learner's real -// database. It returns "" when no safe test database is configured. -func TestDSN() string { +// database. It returns "" and no error when the variable is unset, and +// ErrUnsafeTestDSN when it names any other database. +func TestDSN() (string, error) { dsn := os.Getenv("RC_TEST_DATABASE_URL") if dsn == "" { - return "" + return "", nil } path := dsn if i := strings.IndexAny(path, "?"); i >= 0 { path = path[:i] } if !strings.HasSuffix(path, "_test") { - return "" + return "", ErrUnsafeTestDSN } - return dsn + return dsn, nil } diff --git a/internal/store/store_test.go b/internal/store/store_test.go index 671cd82..dbd7169 100644 --- a/internal/store/store_test.go +++ b/internal/store/store_test.go @@ -2,25 +2,32 @@ package store import ( "context" + "errors" "testing" "time" ) -// openOrSkip connects to the test database, or skips if none is configured. -// The store is closed by t.Cleanup. Cleanups run in reverse order, so this -// one runs after every cleanup the test registers later, which still need the -// connection. (A deferred Close would run before them.) +// openOrSkip connects to the test database. It skips only when +// RC_TEST_DATABASE_URL is unset, and fails when it is set but unsafe or +// unreachable (the rule internal/testenv applies to the other packages, which +// this package's tests cannot import). The store is closed by t.Cleanup. +// Cleanups run in reverse order, so this one runs after every cleanup the test +// registers later, which still need the connection. (A deferred Close would +// run before them.) func openOrSkip(t *testing.T) *Store { t.Helper() - dsn := TestDSN() + dsn, err := TestDSN() + if err != nil { + t.Fatal(err) + } if dsn == "" { - t.Skip("RC_TEST_DATABASE_URL not set to a database ending in _test; skipping") + t.Skip("RC_TEST_DATABASE_URL not set") } ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) defer cancel() s, err := Open(ctx, dsn) if err != nil { - t.Skip("database not reachable:", err) + t.Fatalf("test database not reachable: %v", err) } t.Cleanup(s.Close) return s @@ -102,16 +109,20 @@ func TestMigrateAndSync(t *testing.T) { func TestTestDSNRefusesRealDatabase(t *testing.T) { cases := map[string]string{ - "postgres://u:p@127.0.0.1:5432/rootcause?sslmode=disable": "", + "": "", "postgres://u:p@127.0.0.1:5432/rootcause_test?sslmode=disable": "postgres://u:p@127.0.0.1:5432/rootcause_test?sslmode=disable", "postgres://u:p@127.0.0.1:5432/rootcause_test": "postgres://u:p@127.0.0.1:5432/rootcause_test", } for dsn, want := range cases { t.Setenv("RC_TEST_DATABASE_URL", dsn) - if got := TestDSN(); got != want { - t.Errorf("TestDSN(%q) = %q, want %q", dsn, got, want) + if got, err := TestDSN(); got != want || err != nil { + t.Errorf("TestDSN(%q) = %q, %v; want %q, nil", dsn, got, err, want) } } + t.Setenv("RC_TEST_DATABASE_URL", "postgres://u:p@127.0.0.1:5432/rootcause?sslmode=disable") + if got, err := TestDSN(); got != "" || !errors.Is(err, ErrUnsafeTestDSN) { + t.Errorf("TestDSN(real database) = %q, %v; want ErrUnsafeTestDSN", got, err) + } } // TestWithStatsLock: two builds, here from two stores as from the CLI and the diff --git a/internal/testenv/testenv.go b/internal/testenv/testenv.go new file mode 100644 index 0000000..94ab9de --- /dev/null +++ b/internal/testenv/testenv.go @@ -0,0 +1,121 @@ +// Package testenv gives integration tests one rule for when they may skip. +// A test skips only when what it needs was never configured: RC_TEST_DATABASE_URL +// is unset, or Docker is missing and RC_REQUIRE_DOCKER is not 1. Anything +// configured but broken (an unsafe or unreachable database, a missing Docker +// that CI requires) fails the test, so CI can never pass by skipping. +package testenv + +import ( + "context" + "net/url" + "os" + "os/exec" + "regexp" + "strings" + "testing" + "time" + + "github.com/DIZAMU/RootCause/internal/store" + "github.com/jackc/pgx/v5" +) + +// DSN returns the test database DSN. It skips the test when +// RC_TEST_DATABASE_URL is unset and fails it when the variable names a +// database that does not end in _test. +func DSN(t testing.TB) string { + t.Helper() + dsn, err := store.TestDSN() + if err != nil { + t.Fatal(err) + } + if dsn == "" { + t.Skip("RC_TEST_DATABASE_URL not set") + } + return dsn +} + +// Store opens and migrates the database in dsn, failing the test if it is +// unreachable. The store is closed by t.Cleanup, so register it before any +// cleanup that still needs the connection. +func Store(t testing.TB, dsn string) *store.Store { + t.Helper() + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) + defer cancel() + st, err := store.Open(ctx, dsn) + if err != nil { + t.Fatalf("test database not reachable: %v", err) + } + t.Cleanup(st.Close) + if err := st.Migrate(ctx); err != nil { + t.Fatalf("migrate: %v", err) + } + return st +} + +// NoDocker skips the test, or fails it when RC_REQUIRE_DOCKER is 1. Call it +// when Docker, or a Docker runtime the test needs, is not usable. +func NoDocker(t testing.TB, args ...any) { + t.Helper() + if os.Getenv("RC_REQUIRE_DOCKER") == "1" { + t.Fatal(append([]any{"docker required (RC_REQUIRE_DOCKER=1):"}, args...)...) + } + t.Skip(args...) +} + +// Docker checks that the docker daemon answers, and calls NoDocker if not. +func Docker(t testing.TB) { + t.Helper() + if err := exec.Command("docker", "info").Run(); err != nil { + NoDocker(t, "docker daemon not reachable:", err) + } +} + +var ownName = regexp.MustCompile(`^[a-z0-9_]+$`) + +// OwnDatabase creates an empty database used only by the calling test and +// returns its DSN. Its name is the test database's name with "_" put +// before the "_test" suffix, so rootcause_test gives rootcause__test, +// and the _test guard still holds. A copy left by an earlier, killed run is +// dropped first. The database is dropped again after the test. +// +// Tests that run dbt use it: a dbt build reads every row in the database, so +// rows that other test packages add or delete at the same time would make its +// tests fail at random. +func OwnDatabase(t testing.TB, name string) string { + t.Helper() + if !ownName.MatchString(name) { + t.Fatalf("OwnDatabase: name %q must match %s", name, ownName) + } + base := DSN(t) + u, err := url.Parse(base) + if err != nil { + t.Fatalf("parse RC_TEST_DATABASE_URL: %v", err) + } + baseName := strings.TrimPrefix(u.Path, "/") + db := strings.TrimSuffix(baseName, "_test") + "_" + name + "_test" + quoted := pgx.Identifier{db}.Sanitize() + + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) + defer cancel() + admin, err := store.Open(ctx, base) + if err != nil { + t.Fatalf("test database not reachable: %v", err) + } + t.Cleanup(admin.Close) + for _, sql := range []string{"DROP DATABASE IF EXISTS " + quoted + " WITH (FORCE)", "CREATE DATABASE " + quoted} { + if _, err := admin.Pool().Exec(ctx, sql); err != nil { + t.Fatalf("%s: %v", sql, err) + } + } + // Registered after admin.Close, so it runs before it. + t.Cleanup(func() { + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) + defer cancel() + if _, err := admin.Pool().Exec(ctx, "DROP DATABASE IF EXISTS "+quoted+" WITH (FORCE)"); err != nil { + t.Errorf("drop %s: %v", db, err) + } + }) + + u.Path = "/" + db + return u.String() +} From 9d9f6daad7fd625d72fbcd41473c0e7fc1a1b9c7 Mon Sep 17 00:00:00 2001 From: DIZAMU <117295462+DIZAMU@users.noreply.github.com> Date: Fri, 23 Oct 2026 17:25:48 +0200 Subject: [PATCH 3/4] ci: add the ci, exercises, dbt and commit-policy workflows Actions are pinned by commit SHA, jobs get read-only contents access, and any skipped test fails CI. exercises.yml validates changed exercises with 3 seeds (all on tags), dbt.yml builds against a Postgres service container, and commit-policy.yml checks trailers, emails and GitHub signature verification. --- .github/workflows/ci.yml | 109 ++++++++++++++++++++++++++++ .github/workflows/commit-policy.yml | 32 ++++++++ .github/workflows/dbt.yml | 93 ++++++++++++++++++++++++ .github/workflows/exercises.yml | 78 ++++++++++++++++++++ .golangci.yml | 14 ++++ scripts/check-commit-policy.sh | 100 +++++++++++++++++++++++++ scripts/ci-changed-exercises.sh | 46 ++++++++++++ scripts/ci-enable-userns.sh | 31 ++++++++ scripts/ci-go-test.sh | 72 ++++++++++++++++++ 9 files changed, 575 insertions(+) create mode 100644 .github/workflows/ci.yml create mode 100644 .github/workflows/commit-policy.yml create mode 100644 .github/workflows/dbt.yml create mode 100644 .github/workflows/exercises.yml create mode 100644 .golangci.yml create mode 100755 scripts/check-commit-policy.sh create mode 100755 scripts/ci-changed-exercises.sh create mode 100755 scripts/ci-enable-userns.sh create mode 100755 scripts/ci-go-test.sh diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml new file mode 100644 index 0000000..449b364 --- /dev/null +++ b/.github/workflows/ci.yml @@ -0,0 +1,109 @@ +# CI for every pull request and every push to main (spec section 14). +# A push to a PR branch already runs through pull_request, so pushes to other +# branches are left out to save Actions minutes. +name: ci + +on: + pull_request: + push: + branches: [main] + +permissions: {} + +concurrency: + group: ci-${{ github.ref }} + cancel-in-progress: ${{ github.event_name == 'pull_request' }} + +env: + # CI tool, not a Go dependency: installed with go install at this version, + # which verifies every module against the Go checksum database. + GOLANGCI_LINT_VERSION: v2.15.0 + +jobs: + checks: + name: ci-checks + runs-on: ubuntu-24.04 + timeout-minutes: 15 + permissions: + contents: read + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + persist-credentials: false + - uses: actions/setup-go@b7ad1dad31e06c5925ef5d2fc7ad053ef454303e # v7.0.0 + with: + go-version-file: go.mod + - uses: actions/setup-node@949feb2413d6458794dcd2491c4babbbce0c15c1 # v7.1.0 + with: + node-version-file: web/.nvmrc + cache: npm + cache-dependency-path: web/package-lock.json + + - name: gofmt + run: | + out="$(gofmt -l .)" + if [ -n "$out" ]; then echo "not gofmt-formatted:"; echo "$out"; exit 1; fi + - name: frontend type check and build + run: scripts/build-web.sh + - name: go vet + run: | + go vet ./... + go vet -tags webdist ./... + - name: golangci-lint + run: | + go install "github.com/golangci/golangci-lint/v2/cmd/golangci-lint@${GOLANGCI_LINT_VERSION}" + golangci-lint run ./... + golangci-lint run --build-tags webdist ./... + - name: shellcheck + run: | + shellcheck --version + shellcheck scripts/*.sh .githooks/commit-msg exercises/*/*.sh + - name: commit-msg hook cases + run: bash scripts/test-commit-msg.sh + - name: i18n key parity + run: | + go test -count=1 -run 'TestLocaleParity' ./internal/i18n/ + for d in exercises/*/; do go run ./cmd/rootcause validate "$d"; done + + test: + name: ci-test + runs-on: ubuntu-24.04 # cgroup v2, which the init: systemd exercises need + timeout-minutes: 30 + permissions: + contents: read + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + fetch-depth: 0 # origin/main, for the no-test-files check + persist-credentials: false + - uses: actions/setup-go@b7ad1dad31e06c5925ef5d2fc7ad053ef454303e # v7.0.0 + with: + go-version-file: go.mod + - uses: actions/setup-node@949feb2413d6458794dcd2491c4babbbce0c15c1 # v7.1.0 + with: + node-version-file: web/.nvmrc + cache: npm + cache-dependency-path: web/package-lock.json + + - name: build the dashboard for -tags webdist + run: scripts/build-web.sh + # Before any container starts: the restart switches Docker's storage. + - name: enable Docker userns-remap + run: sudo scripts/ci-enable-userns.sh + - name: start Postgres with rootcause init + run: | + pw="$(openssl rand -hex 24)" + echo "::add-mask::$pw" + printf 'POSTGRES_USER=rootcause\nPOSTGRES_DB=rootcause\nPOSTGRES_PASSWORD=%s\n' "$pw" > deploy/compose/.env + go run ./cmd/rootcause init + docker exec rootcause-postgres-1 createdb -U rootcause rootcause_test + echo "RC_TEST_DATABASE_URL=postgres://rootcause:${pw}@127.0.0.1:5432/rootcause_test?sslmode=disable" >> "$GITHUB_ENV" + - name: go test (fails on any skip) + env: + RC_REQUIRE_DOCKER: "1" + run: >- + scripts/ci-go-test.sh + --require TestScriptedSelection + --require TestBuildRecoversFromKilledEngine + --require TestPlaySvc01 + -tags webdist ./... diff --git a/.github/workflows/commit-policy.yml b/.github/workflows/commit-policy.yml new file mode 100644 index 0000000..b1f66c5 --- /dev/null +++ b/.github/workflows/commit-policy.yml @@ -0,0 +1,32 @@ +# Commit policy (spec sections 14 and 15): every commit in a pull request has +# no Co-authored-by trailer or tool attribution line, the owner's author and +# committer email, and a signature GitHub verifies. +name: commit-policy + +on: + pull_request: + +permissions: {} + +concurrency: + group: commit-policy-${{ github.ref }} + cancel-in-progress: true + +jobs: + commit-policy: + name: commit-policy + runs-on: ubuntu-24.04 + timeout-minutes: 5 + permissions: + contents: read # git history, and the commits API for signature status + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + fetch-depth: 0 + persist-credentials: false + - name: check every commit in the pull request + env: + GH_TOKEN: ${{ github.token }} + BASE: ${{ github.event.pull_request.base.sha }} + HEAD: ${{ github.event.pull_request.head.sha }} + run: scripts/check-commit-policy.sh --signatures api "$BASE" "$HEAD" diff --git a/.github/workflows/dbt.yml b/.github/workflows/dbt.yml new file mode 100644 index 0000000..bd795aa --- /dev/null +++ b/.github/workflows/dbt.yml @@ -0,0 +1,93 @@ +# dbt build against a Postgres service container with seed data (spec +# section 14). The seed data is TestScriptedSelection's series of sessions, +# played through the real engine; the dbt package tests add a killed-engine +# recovery and a failed build that keeps its log. +# +# The dbt container reaches the service container over the job's docker +# network by its service name, set through RC_DBT_NETWORK and RC_DBT_HOST +# (tests and CI only; the engine itself uses the compose Postgres). +# +# The workflow always runs and checks the changed paths itself, so it can +# become a required check (see exercises.yml). +name: dbt + +on: + pull_request: + push: + branches: [main] + workflow_dispatch: + +permissions: {} + +concurrency: + group: dbt-${{ github.ref }} + cancel-in-progress: ${{ github.event_name == 'pull_request' }} + +jobs: + build: + name: dbt + runs-on: ubuntu-24.04 + timeout-minutes: 20 + permissions: + contents: read + services: + postgres: + image: postgres:17 + env: + POSTGRES_USER: rootcause + POSTGRES_DB: rootcause_test + # A throwaway database that lives only for this job. + POSTGRES_PASSWORD: ci-only-throwaway + ports: + - 127.0.0.1:5432:5432 + options: >- + --health-cmd "pg_isready -U rootcause -d rootcause_test" + --health-interval 2s --health-timeout 3s --health-retries 30 + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + fetch-depth: 0 + persist-credentials: false + + - name: did the dbt project or its inputs change? + id: changed + env: + EVENT: ${{ github.event_name }} + PR_BASE: ${{ github.event.pull_request.base.sha }} + PR_HEAD: ${{ github.event.pull_request.head.sha }} + PUSH_BEFORE: ${{ github.event.before }} + PUSH_AFTER: ${{ github.sha }} + run: | + case "$EVENT" in + workflow_dispatch) run=true ;; + pull_request) range="$PR_BASE $PR_HEAD" ;; + *) if [ -z "$PUSH_BEFORE" ] || [ "$PUSH_BEFORE" = 0000000000000000000000000000000000000000 ]; then run=true; else range="$PUSH_BEFORE $PUSH_AFTER"; fi ;; + esac + if [ -z "${run:-}" ]; then + # shellcheck disable=SC2086 # two commits, split on purpose + if git diff --name-only $range | grep -qE '^(dbt/|internal/dbt/|internal/store/migrations/|internal/selector/|internal/testenv/|\.github/workflows/dbt\.yml$)'; then + run=true + else + run=false + fi + fi + echo "run dbt: $run" + echo "run=$run" >> "$GITHUB_OUTPUT" + + - uses: actions/setup-go@b7ad1dad31e06c5925ef5d2fc7ad053ef454303e # v7.0.0 + if: steps.changed.outputs.run == 'true' + with: + go-version-file: go.mod + - name: dbt build with seed data (fails on any skip) + if: steps.changed.outputs.run == 'true' + env: + RC_TEST_DATABASE_URL: postgres://rootcause:ci-only-throwaway@127.0.0.1:5432/rootcause_test?sslmode=disable + RC_DBT_NETWORK: ${{ job.services.postgres.network }} + RC_DBT_HOST: postgres + RC_REQUIRE_DOCKER: "1" + run: >- + scripts/ci-go-test.sh + --require TestScriptedSelection + --require TestBuildRecoversFromKilledEngine + --require TestBuildFailureKeepsLog + ./internal/dbt/ ./internal/selector/ diff --git a/.github/workflows/exercises.yml b/.github/workflows/exercises.yml new file mode 100644 index 0000000..5681327 --- /dev/null +++ b/.github/workflows/exercises.yml @@ -0,0 +1,78 @@ +# Exercise validation (spec section 14): for each changed exercise and seeds +# 1, 2 and 3, build the image; break then check must fail; break, solution, +# check must pass. Pull requests and pushes to main validate only the changed +# exercises; tags and manual runs validate all of them. +# +# The workflow always runs and finds the changed exercises itself, instead of +# using a paths filter, so it can become a required check: a filtered +# workflow that never starts leaves a required check pending forever. +name: exercises + +on: + pull_request: + push: + branches: [main] + tags: ["v*"] + workflow_dispatch: + +permissions: {} + +concurrency: + group: exercises-${{ github.ref }} + cancel-in-progress: ${{ github.event_name == 'pull_request' }} + +jobs: + validate: + name: exercises + runs-on: ubuntu-24.04 # cgroup v2, which the init: systemd exercises need + timeout-minutes: 60 + permissions: + contents: read + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + fetch-depth: 0 + persist-credentials: false + + - name: find the exercises to validate + id: list + env: + EVENT: ${{ github.event_name }} + REF_TYPE: ${{ github.ref_type }} + PR_BASE: ${{ github.event.pull_request.base.sha }} + PR_HEAD: ${{ github.event.pull_request.head.sha }} + PUSH_BEFORE: ${{ github.event.before }} + PUSH_AFTER: ${{ github.sha }} + run: | + if [ "$EVENT" = workflow_dispatch ] || [ "$REF_TYPE" = tag ]; then + ids="$(scripts/ci-changed-exercises.sh all)" + elif [ "$EVENT" = pull_request ]; then + ids="$(scripts/ci-changed-exercises.sh "$PR_BASE" "$PR_HEAD")" + elif [ -z "$PUSH_BEFORE" ] || [ "$PUSH_BEFORE" = 0000000000000000000000000000000000000000 ]; then + ids="$(scripts/ci-changed-exercises.sh all)" + else + ids="$(scripts/ci-changed-exercises.sh "$PUSH_BEFORE" "$PUSH_AFTER")" + fi + echo "exercises to validate: ${ids:-none}" + # One line, space-separated; empty when nothing changed. + echo "ids=$(tr '\n' ' ' <<<"$ids" | xargs)" >> "$GITHUB_OUTPUT" + + - uses: actions/setup-go@b7ad1dad31e06c5925ef5d2fc7ad053ef454303e # v7.0.0 + if: steps.list.outputs.ids != '' + with: + go-version-file: go.mod + - name: enable Docker userns-remap + if: steps.list.outputs.ids != '' + run: sudo scripts/ci-enable-userns.sh + - name: validate, seeds 1 2 3 + if: steps.list.outputs.ids != '' + env: + IDS: ${{ steps.list.outputs.ids }} + run: | + for id in $IDS; do + for seed in 1 2 3; do + echo "::group::$id seed $seed" + scripts/validate-exercise.sh "$id" "$seed" + echo "::endgroup::" + done + done diff --git a/.golangci.yml b/.golangci.yml new file mode 100644 index 0000000..4478dec --- /dev/null +++ b/.golangci.yml @@ -0,0 +1,14 @@ +# golangci-lint configuration. CI installs the version pinned in +# .github/workflows/ci.yml. +version: "2" + +linters: + default: standard + exclusions: + # Unchecked errors from Close, Flush, os.Remove and fmt printing, the + # standard idioms whose errors carry nothing to act on. + presets: [std-error-handling] + rules: + # A deferred Rollback after Commit returns pgx.ErrTxClosed by design. + - linters: [errcheck] + source: 'defer tx\.Rollback\(' diff --git a/scripts/check-commit-policy.sh b/scripts/check-commit-policy.sh new file mode 100755 index 0000000..b75055f --- /dev/null +++ b/scripts/check-commit-policy.sh @@ -0,0 +1,100 @@ +#!/usr/bin/env bash +# check-commit-policy.sh checks every commit in .. against the +# authorship rules (spec section 15): +# +# 1. The message passes .githooks/commit-msg, the same check the local hook +# runs: no Co-authored-by trailer, no tool attribution line. +# 2. Author and committer email are the owner's GitHub noreply address. +# The one exception: a merge commit that GitHub itself created in the +# branch ("Update branch"), whose committer is GitHub. Its committer +# check is skipped, and the output says so. Its author and signature are +# still checked. +# 3. The commit is signed and verified: +# --signatures api GitHub's verification (commit.verification), the +# "Verified" badge. Needs gh, GITHUB_TOKEN and +# GITHUB_REPOSITORY. The default on GitHub Actions. +# --signatures git git's own check (%G? must be G), against the +# local gpg.ssh.allowedSignersFile. The default +# elsewhere. It cannot check the merge commits +# GitHub signs with its own GPG key (status E). +# +# Usage: scripts/check-commit-policy.sh [--signatures api|git] +set -euo pipefail + +owner_email="117295462+DIZAMU@users.noreply.github.com" +github_committer="noreply@github.com" + +mode=git +if [ "${GITHUB_ACTIONS:-}" = true ]; then + mode=api +fi +if [ "${1:-}" = --signatures ]; then + mode="${2:?--signatures needs api or git}" + shift 2 +fi +case "$mode" in api|git) ;; *) echo "unknown signature mode: $mode" >&2; exit 2 ;; esac +[ "$#" -eq 2 ] || { echo "usage: check-commit-policy.sh [--signatures api|git] " >&2; exit 2; } +base="$1" head="$2" + +root="$(cd "$(dirname "$0")/.." && pwd)" +hook="$root/.githooks/commit-msg" +msg="$(mktemp)" +trap 'rm -f "$msg"' EXIT + +commits="$(git rev-list --reverse "$base..$head")" +if [ -z "$commits" ]; then + echo "no commits in $base..$head" + exit 0 +fi + +failures=0 +fail() { + echo "FAIL $1: $2" + failures=$((failures + 1)) +} + +for sha in $commits; do + short="${sha:0:12}" + before=$failures + IFS=$'\t' read -r author committer committer_name parents <<<"$(git log -1 --format=$'%ae\t%ce\t%cn\t%p' "$sha")" + nparents=$(wc -w <<<"$parents") + + git log -1 --format=%B "$sha" > "$msg" + if ! out="$(bash "$hook" "$msg" 2>&1)"; then + fail "$short" "message: ${out#commit-msg: }" + fi + + if [ "$author" != "$owner_email" ]; then + fail "$short" "author email $author is not the owner's" + fi + if [ "$committer" != "$owner_email" ]; then + if [ "$committer" = "$github_committer" ] && [ "$nparents" -gt 1 ]; then + echo "note $short: merge commit created by GitHub (committer $committer_name <$committer>); committer check skipped" + else + fail "$short" "committer email $committer is not the owner's" + fi + fi + + if [ "$mode" = api ]; then + if ! v="$(gh api "repos/$GITHUB_REPOSITORY/commits/$sha" --jq '.commit.verification | "\(.verified) \(.reason)"')"; then + fail "$short" "could not read GitHub's signature verification" + elif [ "${v%% *}" != true ]; then + fail "$short" "signature not verified by GitHub (reason: ${v#* })" + fi + else + status="$(git log -1 --format=%G? "$sha")" + if [ "$status" != G ]; then + fail "$short" "signature status $status, want G (good signature from an allowed signer)" + fi + fi + + if [ "$failures" -eq "$before" ]; then + echo "ok $short $(git log -1 --format=%s "$sha")" + fi +done + +if [ "$failures" -gt 0 ]; then + echo "$failures problem(s) found. See spec section 15." + exit 1 +fi +echo "ok: every commit in $base..$head follows the commit policy" diff --git a/scripts/ci-changed-exercises.sh b/scripts/ci-changed-exercises.sh new file mode 100755 index 0000000..2d4c090 --- /dev/null +++ b/scripts/ci-changed-exercises.sh @@ -0,0 +1,46 @@ +#!/usr/bin/env bash +# ci-changed-exercises.sh prints the ids of the exercises CI must validate, +# one per line (spec section 14): +# +# all every exercise +# the exercises with a file changed between the two commits. +# Every exercise when the validation itself changed (this +# script, validate-exercise.sh or exercises.yml). +# +# Exercises deleted in the change are left out. +# +# Usage: scripts/ci-changed-exercises.sh all +# scripts/ci-changed-exercises.sh +set -euo pipefail + +root="$(cd "$(dirname "$0")/.." && pwd)" +cd "$root" + +all() { + for f in exercises/*/exercise.yaml; do + basename "$(dirname "$f")" + done +} + +case "$#" in + 1) + [ "$1" = all ] || { echo "usage: ci-changed-exercises.sh all | " >&2; exit 2; } + all + ;; + 2) + changed="$(git diff --name-only "$1" "$2")" + if grep -qxE 'scripts/ci-changed-exercises\.sh|scripts/validate-exercise\.sh|\.github/workflows/exercises\.yml' <<<"$changed"; then + all + exit 0 + fi + sed -nE 's#^exercises/([^/]+)/.*#\1#p' <<<"$changed" | sort -u | while read -r id; do + if [ -f "exercises/$id/exercise.yaml" ]; then + echo "$id" + fi + done + ;; + *) + echo "usage: ci-changed-exercises.sh all | " >&2 + exit 2 + ;; +esac diff --git a/scripts/ci-enable-userns.sh b/scripts/ci-enable-userns.sh new file mode 100755 index 0000000..1478fae --- /dev/null +++ b/scripts/ci-enable-userns.sh @@ -0,0 +1,31 @@ +#!/usr/bin/env bash +# ci-enable-userns.sh turns on Docker's user namespace remapping on a CI +# runner, the isolation the engine requires for sandboxes (spec section 13.1, +# OQ-3). It merges "userns-remap": "default" into /etc/docker/daemon.json, +# restarts Docker, and waits until `docker info` reports name=userns. +# +# Restarting Docker with userns-remap switches its storage root, so start +# containers (Postgres, sandboxes) only after this script. +# +# Usage (as root): scripts/ci-enable-userns.sh +set -euo pipefail + +conf=/etc/docker/daemon.json +current='{}' +if [ -s "$conf" ]; then + current="$(cat "$conf")" +fi +printf '%s\n' "$current" | jq '. + {"userns-remap": "default"}' > "$conf.new" +mv "$conf.new" "$conf" +echo "daemon.json:" +cat "$conf" + +systemctl restart docker + +# Poll for the concrete condition, with a time limit. +if ! timeout 60 bash -c 'until docker info --format "{{.SecurityOptions}}" 2>/dev/null | grep -q "name=userns"; do sleep 1; done'; then + echo "ci-enable-userns: docker did not come back with userns-remap within 60s" >&2 + docker info >&2 || true + exit 1 +fi +docker info --format 'security options: {{.SecurityOptions}}, cgroup v{{.CgroupVersion}}' diff --git a/scripts/ci-go-test.sh b/scripts/ci-go-test.sh new file mode 100755 index 0000000..3d8400a --- /dev/null +++ b/scripts/ci-go-test.sh @@ -0,0 +1,72 @@ +#!/usr/bin/env bash +# ci-go-test.sh runs go test for CI and fails when tests did not really run: +# +# - go test fails, or +# - any test is skipped (except the ones in allowed_skips), or +# - a test named with --require did not pass, or +# - a package that has test files on origin/main reports "no test files". +# +# Integration tests skip only when their database or Docker was never +# configured, so in CI a skip means a broken setup, not a pass. +# +# Usage: scripts/ci-go-test.sh [--require TestName]... [go test args]... +# Example: scripts/ci-go-test.sh --require TestScriptedSelection -tags webdist ./... +set -euo pipefail + +# Skipped unless asked for on purpose (RC_DBT_STRESS=1). +allowed_skips=" TestBuildStress " + +required=() +while [ "${1:-}" = "--require" ]; do + required+=("${2:?--require needs a test name}") + shift 2 +done +[ "$#" -gt 0 ] || { echo "usage: ci-go-test.sh [--require TestName]... [go test args]..." >&2; exit 2; } + +root="$(cd "$(dirname "$0")/.." && pwd)" +log="$(mktemp)" +trap 'rm -f "$log"' EXIT + +cd "$root" +status=0 +go test -count=1 -v "$@" 2>&1 | tee "$log" || status=$? +if [ "$status" -ne 0 ]; then + echo "ci-go-test: go test failed (exit $status)" >&2 + exit 1 +fi + +failed=0 + +while read -r name; do + case "$allowed_skips" in + *" $name "*) ;; + *) echo "ci-go-test: $name was skipped" >&2; failed=1 ;; + esac +done < <(sed -nE 's/^[[:space:]]*--- SKIP: ([^ ]+).*/\1/p' "$log") + +for name in "${required[@]}"; do + if ! grep -qE "^[[:space:]]*--- PASS: ${name} " "$log"; then + echo "ci-go-test: required test $name did not pass" >&2 + failed=1 + fi +done + +if git rev-parse --verify --quiet origin/main >/dev/null; then + module="$(go list -m)" + while read -r dir; do + [ -d "$dir" ] || continue + pkg="$module" + [ "$dir" = "." ] || pkg="$module/$dir" + if grep -qE "^\?[[:space:]]+${pkg}[[:space:]]+\[no test files\]" "$log"; then + echo "ci-go-test: $pkg has tests on origin/main but ran none" >&2 + failed=1 + fi + done < <(git ls-tree -r --name-only origin/main | grep '_test\.go$' | xargs -r -n1 dirname | sort -u) +else + echo "ci-go-test: origin/main not fetched; skipping the no-test-files check" >&2 +fi + +if [ "$failed" -ne 0 ]; then + exit 1 +fi +echo "ci-go-test: ok" From 9c9644647291e21224a532ed1ba7225d023ab19c Mon Sep 17 00:00:00 2001 From: DIZAMU <117295462+DIZAMU@users.noreply.github.com> Date: Fri, 23 Oct 2026 17:25:55 +0200 Subject: [PATCH 4/4] docs: document the CI workflows, the test rules and the dbt build --- README.md | 41 +++++++++++++++++++++++++++++++++++++++++ docs/SPEC.md | 16 +++++++++++----- 2 files changed, 52 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index b79da11..fbead57 100644 --- a/README.md +++ b/README.md @@ -28,6 +28,47 @@ The server listens on 127.0.0.1 only. WSL2 forwards `localhost` from Windows to Exercises that boot systemd (such as svc-02) need a host with cgroup v2. WSL2 uses cgroup v2 by default. Check with `docker info --format '{{.CgroupVersion}}'`, which must print `2`. +## Tests + +Unit tests need nothing: `go test ./...`. Integration tests need a database whose name ends in `_test`, and Docker with userns-remap: + +```bash +docker exec rootcause-postgres-1 createdb -U rootcause rootcause_test +export RC_TEST_DATABASE_URL="postgres://rootcause:@127.0.0.1:5432/rootcause_test?sslmode=disable" +go test -count=1 ./... +``` + +- A test skips only when `RC_TEST_DATABASE_URL` is unset, or when Docker is missing. If the variable is set but the database is unreachable or not a `_test` database, the test fails. +- `RC_REQUIRE_DOCKER=1` makes a missing Docker fail the tests too. CI sets it. +- Tests that run dbt create and drop their own database (for example `rootcause_selector_test`), because a dbt build reads every row in its database. +- `RC_DBT_STRESS=1 go test -run TestBuildStress ./internal/dbt/` kills dbt builds at random points and checks that the next build succeeds. `RC_DBT_STRESS_ROUNDS` sets the rounds (default 10). +- A failed dbt build saves its full `dbt.log` under `~/.cache/rootcause/dbt-logs/` and prints the path. The newest 20 are kept. + +## CI + +GitHub Actions runs four workflows (spec section 14). Every action is pinned by commit SHA, and every job gets read-only access to the repository and nothing else. + +| Workflow | Check name | Does | +|---|---|---| +| `ci.yml` | `ci-checks`, `ci-test` | gofmt, go vet, golangci-lint, shellcheck, i18n parity, the dashboard build, and every Go test with Postgres and Docker. Fails if any test is skipped | +| `exercises.yml` | `exercises` | Validates the changed exercises with seeds 1, 2 and 3. All of them on tags `v*` and on manual runs | +| `dbt.yml` | `dbt` | dbt build against a Postgres service container, when the dbt project or its inputs changed | +| `commit-policy.yml` | `commit-policy` | Rejects Co-authored-by trailers, tool attribution lines, author emails other than the owner's, and commits GitHub does not show as Verified | + +Run the same commit check locally with `scripts/check-commit-policy.sh origin/main HEAD`. + +### When the repo goes public + +On the free plan, branch protection does not apply to private repositories, so a red check does not block the merge button. The rule until then: never merge a PR with a red check. When the repo goes public, add a ruleset under Settings, Rules, Rulesets: + +- Target: the default branch (`main`). Enforcement: Active. No bypass list. +- Restrict deletions, and block force pushes. +- Require a pull request before merging, with merge commits only (squash and rebase drop the SSH signatures). +- Require signed commits. +- Require status checks to pass, with the branch up to date: `ci-checks`, `ci-test`, `exercises`, `dbt`, `commit-policy`. Every workflow starts on every PR and decides inside the job what to run, so none of these checks stays pending. + +Known gap to close when the ruleset goes on: the workflows, `scripts/check-commit-policy.sh` and `.githooks/commit-msg` run from the PR's own commits, so a PR could weaken the checks that judge it. Before the repo goes public, make `commit-policy.yml` check out the base branch's copy of the script and hook, and require review of changes under `.github/`, `scripts/` and `.githooks/`. + ## License PolyForm Noncommercial 1.0.0. See [`LICENSE`](LICENSE). diff --git a/docs/SPEC.md b/docs/SPEC.md index ecbc24b..7460fc3 100644 --- a/docs/SPEC.md +++ b/docs/SPEC.md @@ -311,7 +311,9 @@ dbt reads the source tables in schema `public` and writes every model to schema Tests: unique and not-null keys, accepted values for event types, statuses and outcomes, relationships between sessions and events. Singular tests assert one level row per learner and topic, and one review row per learner and exercise. -The project lives in `dbt/`. `internal/dbt` runs `dbt build` in the pinned `dbt-postgres` image with `docker run`. The container joins the compose network and reaches Postgres by service name. It runs with no capabilities, a read-only root filesystem, the project mounted read-only, and targets and logs in `/tmp`. The connection comes from the engine's database URL, and the password reaches the container only through its environment. dbt runs only against the compose Postgres, never over the host network. +The project lives in `dbt/` and is embedded in the binary. `internal/dbt` extracts it into a fresh temporary directory for each build, so a build never reads the working tree, and runs `dbt build` in the pinned `dbt-postgres` image with `docker run`. The container joins the compose network and reaches Postgres by service name. It runs with no capabilities, a read-only root filesystem, the project mounted read-only, and targets and logs in `/tmp`. The connection comes from the engine's database URL, and the password reaches the container only through its environment. dbt runs only against the compose Postgres, never over the host network. For tests and CI only, `RC_DBT_NETWORK` and `RC_DBT_HOST` name another Postgres container's network and host, such as a CI service container. + +Every dbt container is labelled with its database. Before a build, containers left by an earlier build of the same database are removed. A build passes only when dbt exits 0 and runs exactly as many nodes as the project defines, so a run that silently skips models or tests fails. A failed build keeps its full `dbt.log` under the user cache directory and prints the path. ### 9.3 When dbt runs @@ -394,13 +396,15 @@ Implementation: | Workflow | Runs on | Does | |---|---|---| -| `ci.yml` | Every push and PR | go vet, golangci-lint, go test, shellcheck on all scripts, frontend type check and build, i18n key parity | +| `ci.yml` | Every PR and push to main | go vet, golangci-lint, go test, shellcheck on all scripts, frontend type check and build, i18n key parity. Postgres comes from `rootcause init`, and any skipped test fails the run | | `exercises.yml` | Changes under `exercises/` | For each changed exercise and 3 seeds: build the image; break then check must fail; break, solution, check must pass | -| `dbt.yml` | Changes under `dbt/` | dbt build against a Postgres service container with seed data | +| `dbt.yml` | Changes under `dbt/` | dbt build against a Postgres service container with seed data: a scripted series of sessions played through the engine | | `commit-policy.yml` | Every PR | Rejects Co-authored-by trailers, tool attribution lines, author emails other than the owner's, and unsigned commits | | `release.yml` (v2) | Tags on main | Builds and pushes images to GHCR, updates image tags in `deploy/k8s`. Argo CD syncs | - Private repo on the free plan means limited Actions minutes. Only changed exercises are validated per PR. Full validation runs on tags. +- `exercises.yml` and `dbt.yml` start on every PR and push to main, and decide inside the job whether anything changed, so they can be required checks. +- Every action is pinned by commit SHA. Workflows have no permissions by default, and each job gets read access to the repository only. - GHCR images are private while the repo is private, so the cluster needs an image pull secret. - Images target linux/amd64 only, until something needs ARM. @@ -510,6 +514,7 @@ Done when: a merged change reaches the cluster without manual kubectl. The owner decides when the repo goes public. Ready means: - Phases 0 to 6 complete, CI green. +- A ruleset on `main` requires the CI checks, signed commits and pull requests, as listed in the README. - README explains the purpose and architecture, and how to run it locally in under 10 minutes. - Demo recording in the README. - Git history scanned for secrets, none found. @@ -545,15 +550,16 @@ RootCause/ │ ├── dbt/ runs the dbt project in a container │ ├── store/ Postgres access, migrations │ ├── server/ HTTP API for the dashboard +│ ├── testenv/ shared skip and fail rules for integration tests │ └── i18n/ ├── web/ React + TypeScript (Vite); embed.go embeds dist/ with -tags webdist ├── exercises// -├── dbt/ +├── dbt/ dbt project; embed.go embeds it in the binary ├── deploy/ │ ├── compose/ v1: Postgres (dbt runs with docker run, see 9.2) │ └── k8s/ v2: manifests ├── locales/ en.json, no.json -├── scripts/ validate-exercise.sh, build-web.sh +├── scripts/ validate-exercise.sh, build-web.sh, CI helpers ├── .githooks/ commit-msg ├── .github/workflows/ ├── docs/