diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index c31819e8..82ca2291 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -15,6 +15,20 @@ permissions: contents: read jobs: + schema-drift: + name: Schema drift check + # Verifies the embedded internal/schema/ingest.v1.json matches + # tracebloc/data-ingestors' master. A green PR that silently + # diverges from upstream is a real correctness hazard — a + # customer's YAML could pass `tracebloc ingest validate` locally + # but be rejected by jobs-manager (or vice versa). Forcing the + # sync as a PR step keeps drift visible. + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - name: scripts/sync-schema.sh --check + run: ./scripts/sync-schema.sh --check + test: name: Test runs-on: ubuntu-latest diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 8c1bf3f9..2bd03cbc 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -2,14 +2,19 @@ ## Local development -```bash -go build -o tracebloc ./cmd/tracebloc -./tracebloc version +The `Makefile` mirrors the CI pipeline — `make ci` runs the exact same checks that PR #N's GitHub Actions run. If `make ci` passes locally, CI will too (modulo non-deterministic flakes). **Run `make ci` before pushing.** Skipping it has cost us at least one PR's worth of fix-up commits per bug class so far. -go test ./... -golangci-lint run # https://golangci-lint.run/usage/install/ +```bash +make ci # vet + test + lint + fmt-check + schema-check (run this before push) +make build # produces ./tracebloc +make fmt # fixes gofmt -s drift in place +make schema-sync # pulls latest ingest.v1.json from data-ingestors master ``` +Individual targets are also runnable in isolation — `make test`, `make lint`, etc. See the `Makefile` for the full list. + +Requires [`golangci-lint`](https://golangci-lint.run/usage/install/) (install via `brew install golangci-lint` or your platform's equivalent). + Cobra autocomplete for `bash` / `zsh` / `fish` / `powershell` is available via the `completion` subcommand. Useful while developing too: ```bash diff --git a/Makefile b/Makefile new file mode 100644 index 00000000..0384697c --- /dev/null +++ b/Makefile @@ -0,0 +1,75 @@ +# Top-level Makefile for tracebloc/cli. +# +# Purpose: keep the local feedback loop the same shape as the CI +# loop. Anything that fails in `make ci` would have failed on a PR, +# and vice versa. Don't add targets here that aren't also enforced +# by .github/workflows/build.yml — divergence between local and CI +# is the bug this file exists to prevent. + +# ---- toggles ----------------------------------------------------- + +GO ?= go +GOLANGCI_LINT ?= golangci-lint +PKGS := ./... + +# ---- top-level targets ------------------------------------------- + +.PHONY: ci +ci: vet test lint fmt-check schema-check + @echo "==> ci: all green" + +.PHONY: build +build: + $(GO) build -o tracebloc ./cmd/tracebloc + +.PHONY: install +install: + $(GO) install ./cmd/tracebloc + +# ---- individual targets (also runnable in isolation) ------------- + +.PHONY: vet +vet: + $(GO) vet $(PKGS) + +.PHONY: test +test: + $(GO) test -race -cover $(PKGS) + +.PHONY: lint +lint: + @command -v $(GOLANGCI_LINT) >/dev/null 2>&1 || { \ + echo "==> $(GOLANGCI_LINT) not on PATH"; \ + echo " install via: brew install golangci-lint"; \ + echo " or see: https://golangci-lint.run/usage/install/"; \ + exit 1; \ + } + $(GOLANGCI_LINT) run + +.PHONY: fmt +fmt: + gofmt -s -w . + +.PHONY: fmt-check +fmt-check: + @diff="$$(gofmt -s -l . 2>/dev/null)"; \ + if [ -n "$$diff" ]; then \ + echo "==> gofmt -s needed on:"; \ + echo "$$diff" | sed 's/^/ /'; \ + echo "==> run \`make fmt\` to fix"; \ + exit 1; \ + fi + +.PHONY: schema-check +schema-check: + ./scripts/sync-schema.sh --check + +.PHONY: schema-sync +schema-sync: + ./scripts/sync-schema.sh + +# ---- cleanup ----------------------------------------------------- + +.PHONY: clean +clean: + rm -rf tracebloc dist/ coverage.out coverage.html diff --git a/cmd/tracebloc/main.go b/cmd/tracebloc/main.go index 071bba4a..a38a4461 100644 --- a/cmd/tracebloc/main.go +++ b/cmd/tracebloc/main.go @@ -18,6 +18,7 @@ package main import ( + "fmt" "os" "github.com/tracebloc/cli/internal/cli" @@ -34,13 +35,34 @@ var ( ) func main() { - if err := cli.NewRootCmd(cli.BuildInfo{ + err := cli.NewRootCmd(cli.BuildInfo{ Version: version, GitSHA: gitSHA, BuildDate: buildDate, - }).Execute(); err != nil { - // cobra has already printed the error + usage to stderr by - // the time we get here; just propagate a non-zero exit. - os.Exit(1) + }).Execute() + if err == nil { + return } + + // Print the error to stderr before exiting. The root command + // sets SilenceErrors: true to keep cobra from prepending its + // own "Error: ..." line on top of structured handler output + // — but that puts the burden on us to surface the error + // message ourselves. Without this, every non-schema-violation + // failure (file-read errors, YAML parse errors, schema-compile + // errors) exits non-zero with NO message to the customer. + // + // Handlers that have already printed their own diagnostic + // (e.g. `ingest validate` prints per-violation lines) signal + // "silent" by returning an exitError with a nil inner — see + // cli.IsSilentError for the contract. + if !cli.IsSilentError(err) { + fmt.Fprintln(os.Stderr, "Error:", err) + } + + // Map command-defined exit codes through. Handlers that want a + // specific exit code (e.g. `ingest validate` returns 2 for + // schema violations, 3 for parse errors) return a *cli.ExitError + // the package exports; everything else gets the default 1. + os.Exit(cli.ExitCodeFromError(err)) } diff --git a/go.mod b/go.mod index 796f785f..8c5a9db1 100644 --- a/go.mod +++ b/go.mod @@ -2,7 +2,12 @@ module github.com/tracebloc/cli go 1.22 -require github.com/spf13/cobra v1.8.1 +require ( + github.com/santhosh-tekuri/jsonschema/v6 v6.0.2 + github.com/spf13/cobra v1.8.1 + golang.org/x/text v0.16.0 + gopkg.in/yaml.v3 v3.0.1 +) require ( github.com/inconshreveable/mousetrap v1.1.0 // indirect diff --git a/go.sum b/go.sum index 912390a7..3cf027a8 100644 --- a/go.sum +++ b/go.sum @@ -1,10 +1,18 @@ github.com/cpuguy83/go-md2man/v2 v2.0.4/go.mod h1:tgQtvFlXSQOSOSIRvRPT7W67SCa46tRHOmNcaadrF8o= +github.com/dlclark/regexp2 v1.11.0 h1:G/nrcoOa7ZXlpoa/91N3X7mM3r8eIlMBBJZvsz/mxKI= +github.com/dlclark/regexp2 v1.11.0/go.mod h1:DHkYz0B9wPfa6wondMfaivmHpzrQ3v9q8cnmRbL6yW8= github.com/inconshreveable/mousetrap v1.1.0 h1:wN+x4NVGpMsO7ErUn/mUI3vEoE6Jt13X2s0bqwp9tc8= github.com/inconshreveable/mousetrap v1.1.0/go.mod h1:vpF70FUmC8bwa3OWnCshd2FqLfsEA9PFc4w1p2J65bw= github.com/russross/blackfriday/v2 v2.1.0/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQDYRxCVz55jmeOWTM= +github.com/santhosh-tekuri/jsonschema/v6 v6.0.2 h1:KRzFb2m7YtdldCEkzs6KqmJw4nqEVZGK7IN2kJkjTuQ= +github.com/santhosh-tekuri/jsonschema/v6 v6.0.2/go.mod h1:JXeL+ps8p7/KNMjDQk3TCwPpBy0wYklyWTfbkIzdIFU= github.com/spf13/cobra v1.8.1 h1:e5/vxKd/rZsfSJMUX1agtjeTDf+qv1/JdBF8gg5k9ZM= github.com/spf13/cobra v1.8.1/go.mod h1:wHxEcudfqmLYa8iTfL+OuZPbBZkmvliBWKIezN3kD9Y= github.com/spf13/pflag v1.0.5 h1:iy+VFUOCP1a+8yFto/drg2CJ5u0yRoB7fZw3DKv/JXA= github.com/spf13/pflag v1.0.5/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An2Bg= +golang.org/x/text v0.16.0 h1:a94ExnEXNtEwYLGJSIUxnWoxoRz/ZcCsV63ROupILh4= +golang.org/x/text v0.16.0/go.mod h1:GhwF1Be+LQoKShO3cGOHzqOgRrGaYc9AvblQOmPVHnI= +gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= +gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= diff --git a/internal/cli/exit.go b/internal/cli/exit.go new file mode 100644 index 00000000..bdbe10aa --- /dev/null +++ b/internal/cli/exit.go @@ -0,0 +1,68 @@ +package cli + +// ExitCodeFromError extracts an exit code from a handler-returned +// error. Handlers that need a specific exit code wrap their return +// in an *exitError (see ingest.go); everything else defaults to 1. +// +// Public-but-package-keyed: main.go is the only intended caller, +// and the exitError type itself stays unexported so subcommand +// handlers go through the constructor. +func ExitCodeFromError(err error) int { + if err == nil { + return 0 + } + var ee *exitError + if asExitError(err, &ee) { + return ee.code + } + return 1 +} + +// IsSilentError reports whether a handler-returned error wants +// main() to suppress its own "Error: ..." stderr line on the way +// out. The contract: a handler that has already printed a +// structured diagnostic itself (e.g. the schema-validate path +// prints per-violation lines to stderr) returns +// `&exitError{code: N, err: nil}` to signal "exit non-zero but +// don't print anything more." Errors with a non-nil inner err +// (file-read failures, parse errors, schema-compile bugs) are +// NOT silent — main() prints them so the customer doesn't see a +// bare non-zero exit with no explanation. +// +// Caller pattern in main.go: +// +// if err != nil && !cli.IsSilentError(err) { +// fmt.Fprintln(os.Stderr, "Error:", err) +// } +// os.Exit(cli.ExitCodeFromError(err)) +func IsSilentError(err error) bool { + if err == nil { + return false + } + var ee *exitError + if asExitError(err, &ee) { + return ee.err == nil + } + return false +} + +// asExitError walks the wrapped-error chain looking for an +// *exitError. Same pattern as errors.As but with a typed target so +// callers don't have to import errors at every site. +func asExitError(err error, target **exitError) bool { + for cur := err; cur != nil; cur = unwrapError(cur) { + if ee, ok := cur.(*exitError); ok { + *target = ee + return true + } + } + return false +} + +func unwrapError(err error) error { + type unwrapper interface{ Unwrap() error } + if u, ok := err.(unwrapper); ok { + return u.Unwrap() + } + return nil +} diff --git a/internal/cli/ingest.go b/internal/cli/ingest.go new file mode 100644 index 00000000..b49d6928 --- /dev/null +++ b/internal/cli/ingest.go @@ -0,0 +1,143 @@ +package cli + +import ( + "fmt" + "os" + + "github.com/spf13/cobra" + + "github.com/tracebloc/cli/internal/schema" +) + +// newIngestCmd implements the `tracebloc ingest` subtree. Today it +// has only one verb — `validate` — which runs the schema check +// locally without touching the cluster. Future verbs (status, retry, +// cancel) hang off this same parent in later phases. +func newIngestCmd() *cobra.Command { + cmd := &cobra.Command{ + Use: "ingest", + Short: "Inspect and manage ingestion configurations", + Long: `Commands for working with ingest.yaml files and ingestion runs. + +Today only ` + "`validate`" + ` is implemented — it runs the same schema check +the cluster's jobs-manager runs, but locally and instantly. Use it to +catch typos and missing fields before submitting an ingestion. + +Future verbs (status, retry, cancel) will land alongside the +push/list/show commands in later phases.`, + } + + cmd.AddCommand(newIngestValidateCmd()) + return cmd +} + +// newIngestValidateCmd implements `tracebloc ingest validate `. +// Reads a YAML file from disk, validates it against the embedded v1 +// schema, prints any violations in the same format the Python +// implementation uses, exits non-zero on any violation. +// +// The output format is deliberately matched to +// tracebloc_ingestor.cli.run._format_errors so a customer's editor +// or CI can grep both implementations' output uniformly. See +// internal/schema/validate.go for the formatting contract. +func newIngestValidateCmd() *cobra.Command { + cmd := &cobra.Command{ + Use: "validate ", + Short: "Validate an ingest.yaml against the embedded v1 schema, locally", + Long: `Reads , parses it as YAML, and validates it against the bundled +ingest.v1.json schema (synced from tracebloc/data-ingestors). Prints +violations in the same JSON-pointer-prefixed format the cluster's +jobs-manager uses, and exits non-zero if any are found. + +Useful as a pre-flight before ` + "`tracebloc dataset push`" + ` lands in a +future phase; for now, customers running the Helm chart can validate +their ` + "`ingest.yaml`" + ` before invoking ` + "`helm install`" + `, getting +millisecond local feedback instead of a multi-second cluster round +trip. + +Exit codes: + 0 YAML parses and validates cleanly + 2 YAML parses but has schema violations (printed to stderr) + 3 YAML doesn't parse or file isn't readable`, + Args: cobra.ExactArgs(1), + RunE: runIngestValidate, + } + return cmd +} + +func runIngestValidate(cmd *cobra.Command, args []string) error { + path := args[0] + + body, err := os.ReadFile(path) + if err != nil { + // fileError is exit-code 3 territory. We use a sentinel + // exit-coded error so cobra propagates the right code via + // the main()-side os.Exit mapping (in a follow-up commit + // we'll wire main.go to inspect for these). + return &exitError{code: 3, err: fmt.Errorf("reading %s: %w", path, err)} + } + + v, err := schema.NewV1Validator() + if err != nil { + // Schema-compilation failures are infrastructure-side, not + // customer-side — we bundle the schema, so this only fires + // if the build is broken. Treat as exit-code-2 so CI can + // distinguish from a customer file problem. + return &exitError{code: 2, err: fmt.Errorf("loading embedded schema: %w", err)} + } + + _, violations, parseErr := v.ValidateYAML(body) + if parseErr != nil { + return &exitError{code: 3, err: fmt.Errorf("%s: %w", path, parseErr)} + } + + if len(violations) == 0 { + // Explicit discard: Fprintf returns an error when the + // underlying writer fails (closed pipe, etc.). For the + // success summary we'd rather still exit 0 even if the + // downstream consumer dropped the connection — they got + // what they needed (the exit code), and propagating a + // pipe-write error would convert success into failure for + // reasons unrelated to validation. + _, _ = fmt.Fprintf(cmd.OutOrStdout(), "%s: ok\n", path) + return nil + } + + // Print violations to stderr so success can be piped without + // interference; error lines are diagnostic, not data. Same + // pipe-write rationale as the ok-path above: don't let a + // stderr-write failure mask the real exit-2 schema-violation + // signal. + _, _ = fmt.Fprintf(cmd.ErrOrStderr(), "%s: schema validation failed (%d issue%s)\n", + path, len(violations), plural(len(violations))) + _, _ = fmt.Fprintln(cmd.ErrOrStderr(), schema.FormatErrors(violations)) + return &exitError{code: 2, err: nil} // err==nil so cobra doesn't print "Error: ..." on top +} + +func plural(n int) string { + if n == 1 { + return "" + } + return "s" +} + +// exitError carries a process exit code alongside (or instead of) +// an error message. main.go inspects for this type before calling +// os.Exit, mapping the code through. Other handlers can opt in to +// specific exit codes by returning an exitError. +type exitError struct { + code int + err error +} + +func (e *exitError) Error() string { + if e.err == nil { + return fmt.Sprintf("exit %d", e.code) + } + return e.err.Error() +} + +func (e *exitError) Unwrap() error { return e.err } + +// Code returns the process exit code main() should propagate. +func (e *exitError) Code() int { return e.code } diff --git a/internal/cli/ingest_test.go b/internal/cli/ingest_test.go new file mode 100644 index 00000000..4e7dd085 --- /dev/null +++ b/internal/cli/ingest_test.go @@ -0,0 +1,190 @@ +package cli + +import ( + "bytes" + "os" + "path/filepath" + "strings" + "testing" +) + +// writeTmpYAML drops a small YAML doc into the test's t.TempDir and +// returns the path. Using TempDir guarantees cleanup on test exit; +// callers don't have to defer os.Remove themselves. +func writeTmpYAML(t *testing.T, content string) string { + t.Helper() + path := filepath.Join(t.TempDir(), "ingest.yaml") + if err := os.WriteFile(path, []byte(content), 0o600); err != nil { + t.Fatalf("WriteFile: %v", err) + } + return path +} + +// execIngestValidate drives the full cobra dispatch for +// `tracebloc ingest validate ` and returns the exit code, the +// captured stdout, and the captured stderr. Tests should never share +// a *cobra.Command across cases — cobra holds flag state on the +// command tree and stale trees leak one test's args into the next. +func execIngestValidate(t *testing.T, path string) (exitCode int, stdout, stderr string) { + t.Helper() + root := NewRootCmd(BuildInfo{Version: "test"}) + var so, se bytes.Buffer + root.SetOut(&so) + root.SetErr(&se) + root.SetArgs([]string{"ingest", "validate", path}) + + err := root.Execute() + return ExitCodeFromError(err), so.String(), se.String() +} + +func TestIngestValidate_HappyPath(t *testing.T) { + path := writeTmpYAML(t, ` +apiVersion: tracebloc.io/v1 +kind: IngestConfig +table: cats_dogs_train +intent: train +category: image_classification +csv: /data/labels.csv +images: /data/images/ +label: image_label +`) + + code, stdout, stderr := execIngestValidate(t, path) + if code != 0 { + t.Fatalf("expected exit 0, got %d\nstderr:\n%s", code, stderr) + } + if !strings.Contains(stdout, "ok") { + t.Errorf("expected 'ok' on stdout, got: %q", stdout) + } + if stderr != "" { + t.Errorf("expected empty stderr on success, got: %q", stderr) + } +} + +func TestIngestValidate_SchemaFailureExitsTwo(t *testing.T) { + // Missing required fields → schema violation → exit 2. + path := writeTmpYAML(t, ` +kind: IngestConfig +category: image_classification +table: t +csv: /data/labels.csv +images: /data/images/ +label: image_label +`) + + code, _, stderr := execIngestValidate(t, path) + if code != 2 { + t.Fatalf("expected exit 2 for schema failure, got %d", code) + } + // Errors print to stderr (so success can be piped without + // interference). Both the count line + the per-error lines + // should be there. + for _, want := range []string{"schema validation failed", "apiVersion"} { + if !strings.Contains(stderr, want) { + t.Errorf("expected stderr to mention %q, got:\n%s", want, stderr) + } + } +} + +func TestIngestValidate_UnreadableFileExitsThree(t *testing.T) { + // Distinct exit code (3) for file-level problems (missing, + // permission-denied, etc.) — separates from schema violations + // (2) so callers can branch on the cause. + code, _, _ := execIngestValidate(t, "/tmp/definitely-does-not-exist-"+t.Name()) + if code != 3 { + t.Errorf("expected exit 3 for missing file, got %d", code) + } +} + +func TestIngestValidate_NonMappingExitsThree(t *testing.T) { + // A top-level YAML sequence (vs mapping) is a parse-shape + // problem, not a schema problem — surface it as the file-level + // exit code so the customer knows their file isn't an + // ingest-config at all, vs being one with the wrong fields. + path := writeTmpYAML(t, "- one\n- two\n") + + code, _, _ := execIngestValidate(t, path) + if code != 3 { + t.Errorf("expected exit 3 for non-mapping YAML, got %d", code) + } +} + +func TestIngestValidate_RequiresExactlyOneArg(t *testing.T) { + // Cobra catches arg-count violations before our RunE runs; + // confirm we exit non-zero (the specific code is cobra's + // default 1, not our exitError 2/3 — that's intentional). + root := NewRootCmd(BuildInfo{Version: "test"}) + var so, se bytes.Buffer + root.SetOut(&so) + root.SetErr(&se) + root.SetArgs([]string{"ingest", "validate"}) // no path + + err := root.Execute() + if err == nil { + t.Fatalf("expected error from missing path arg, got nil") + } + if got := ExitCodeFromError(err); got == 0 { + t.Errorf("expected non-zero exit, got %d", got) + } +} + +func TestExitCodeFromError(t *testing.T) { + // Defensive: pin the exitError dispatch behavior since this is + // the only thing main() depends on from this package. + if got := ExitCodeFromError(nil); got != 0 { + t.Errorf("nil err should map to 0, got %d", got) + } + if got := ExitCodeFromError(&exitError{code: 7}); got != 7 { + t.Errorf("explicit exit code should propagate, got %d", got) + } + // A wrapped exitError still reports its code (asExitError walks + // the unwrap chain). + wrapped := &exitError{code: 9, err: &exitError{code: 0}} + if got := ExitCodeFromError(wrapped); got != 9 { + t.Errorf("outermost exitError wins, got %d", got) + } +} + +// IsSilentError is the main()-side hook that decides whether to +// print an "Error: ..." stderr line on the way out. Pin the +// contract so main.go doesn't silently regress to swallowing +// errors (the high-severity bugbot finding that led to this +// being added in the first place). +func TestIsSilentError(t *testing.T) { + cases := []struct { + name string + err error + want bool + }{ + {"nil error", nil, false}, + { + "exitError with nil inner (e.g. schema-violation already-printed)", + &exitError{code: 2, err: nil}, + true, + }, + { + "exitError with non-nil inner (e.g. file read failure)", + &exitError{code: 3, err: io_eof_or_similar()}, + false, + }, + {"plain error from cobra (e.g. unknown command)", errorString("unknown command"), false}, + } + + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + if got := IsSilentError(c.err); got != c.want { + t.Errorf("IsSilentError(%v) = %v, want %v", c.err, got, c.want) + } + }) + } +} + +// errorString is the simplest possible error implementation, used +// in tests that need a "plain" error without any custom Unwrap +// behavior. Same as the stdlib errors.New() result; defined inline +// to keep the test file self-contained. +type errorString string + +func (e errorString) Error() string { return string(e) } + +func io_eof_or_similar() error { return errorString("read: file does not exist") } diff --git a/internal/cli/root.go b/internal/cli/root.go index 8ce9ac34..5fcf634f 100644 --- a/internal/cli/root.go +++ b/internal/cli/root.go @@ -64,6 +64,7 @@ roadmap. Subsequent phases land subcommands incrementally.`, // Subcommands. New phases append here. root.AddCommand(newVersionCmd(info)) + root.AddCommand(newIngestCmd()) return root } diff --git a/internal/schema/embed.go b/internal/schema/embed.go new file mode 100644 index 00000000..9fa8f871 --- /dev/null +++ b/internal/schema/embed.go @@ -0,0 +1,30 @@ +// Package schema holds the embedded ingest schema(s) and the +// validator built on top of them. +// +// Today only v1 is supported. When a v2 lands, this package grows +// a SchemaVersion dispatch (per the ingest.yaml's apiVersion field) +// without changing the validator's public API. +// +// Why embed? Two reasons. +// +// 1. Local validation. `tracebloc ingest validate ` runs +// entirely offline — no cluster, no network — which is the +// whole point of doing it on the CLI rather than waiting for +// jobs-manager's POST-time check. +// 2. Drift detection. Bundling the schema makes drift between +// the CLI's view and data-ingestors' canonical source +// observable at build time, not at runtime. scripts/sync-schema.sh +// enforces parity in CI. +package schema + +import _ "embed" + +// V1Bytes is the raw JSON of ingest.v1.json, vendored from +// tracebloc/data-ingestors at build time via scripts/sync-schema.sh. +// +// Exposed as []byte (not parsed) so consumers can either pipe it +// into their preferred JSON-Schema implementation or inspect the +// raw $id / $schema fields to assert which version they got. +// +//go:embed ingest.v1.json +var V1Bytes []byte diff --git a/internal/schema/ingest.v1.json b/internal/schema/ingest.v1.json new file mode 100644 index 00000000..21a0c460 --- /dev/null +++ b/internal/schema/ingest.v1.json @@ -0,0 +1,380 @@ +{ + "$schema": "http://json-schema.org/draft-07/schema#", + "$id": "https://tracebloc.io/schemas/ingest.v1.json", + "title": "tracebloc IngestConfig (v1)", + "description": "Declarative configuration for the tracebloc data ingestor. Replaces the per-customer Python script + Dockerfile pattern. Customers describe their dataset; the official image runs it.", + "type": "object", + "additionalProperties": false, + + "required": [ + "apiVersion", + "kind", + "category", + "table", + "intent" + ], + + "properties": { + "apiVersion": { + "const": "tracebloc.io/v1", + "description": "Schema version. Breaking changes require v2." + }, + "kind": { + "const": "IngestConfig" + }, + + "category": { + "type": "string", + "enum": [ + "image_classification", + "object_detection", + "keypoint_detection", + "semantic_segmentation", + "instance_segmentation", + "text_classification", + "tabular_classification", + "tabular_regression", + "time_series_forecasting", + "time_to_event_prediction", + "masked_language_modeling" + ], + "description": "Task category. Drives convention defaults: validators, data_format, default columns, default file extensions, default validator set." + }, + + "table": { + "type": "string", + "minLength": 1, + "description": "Destination table name in the cluster-internal MySQL. Used to identify the dataset in subsequent runs." + }, + + "intent": { + "type": "string", + "enum": ["train", "test"], + "description": "Whether this dataset is for training or held-out testing." + }, + + "csv": { + "type": "string", + "minLength": 1, + "description": "Path to the labels CSV. Mutually exclusive with `json`. The dominant source type." + }, + + "json": { + "type": "string", + "minLength": 1, + "description": "Path to a JSON dataset. Mutually exclusive with `csv`." + }, + + "images": { + "type": "string", + "minLength": 1, + "description": "Directory holding the image files referenced by the labels CSV. Required for image-based categories." + }, + + "annotations": { + "type": "string", + "minLength": 1, + "description": "Directory holding annotation files (e.g. Pascal VOC XML). Required for object_detection." + }, + + "masks": { + "type": "string", + "minLength": 1, + "description": "Directory holding segmentation mask images (PNG). Required for semantic_segmentation." + }, + + "texts": { + "type": "string", + "minLength": 1, + "description": "Directory holding text files referenced by the labels CSV. Required for text_classification." + }, + + "sequences": { + "type": "string", + "minLength": 1, + "description": "Directory holding sequence text files (.txt). Required for masked_language_modeling." + }, + + "label": { + "oneOf": [ + { + "type": "string", + "minLength": 1, + "description": "Shorthand: column name in the CSV that holds the label. Implies policy=passthrough." + }, + { + "type": "object", + "additionalProperties": false, + "required": ["column"], + "properties": { + "column": { + "type": "string", + "minLength": 1, + "description": "Column name in the CSV that holds the label." + }, + "policy": { + "type": "string", + "enum": ["passthrough", "bucket"], + "description": "How label values are sent to the central backend. `passthrough` (default for classification) sends the raw value. `bucket` (required for regression-class tasks) bins the value before send so the central backend never sees the raw target." + } + } + } + ], + "description": "Label specification. Can be a string (column name) for the common case, or an object for explicit policy control." + }, + + "schema": { + "type": "object", + "minProperties": 1, + "additionalProperties": { + "type": "string" + }, + "description": "Column → SQL type map (e.g. {'age': 'INT', 'name': 'VARCHAR(255)'}). Required for tabular and time-series categories." + }, + + "time_column": { + "type": "string", + "minLength": 1, + "description": "Name of the time column for time_to_event_prediction. Falls back to a column named `time` if unset." + }, + + "data_id": { + "type": "object", + "additionalProperties": false, + "required": ["strategy"], + "properties": { + "strategy": { + "type": "string", + "enum": ["uuid", "column"], + "default": "uuid", + "description": "How `data_id` is generated. `uuid` (default) generates a fresh UUID per record — no source column leaves the cluster. `column` maps a source column's value, which is louder (logged warning) and only safe when the column doesn't carry PII." + }, + "column": { + "type": "string", + "minLength": 1, + "description": "Required when strategy=column. The source column whose values become `data_id`." + } + }, + "if": { "properties": { "strategy": { "const": "column" } }, "required": ["strategy"] }, + "then": { "required": ["column"] } + }, + + "spec": { + "type": "object", + "additionalProperties": false, + "description": "Advanced overrides. Most customers don't touch this; convention defaults from `category` cover the dominant case.", + "properties": { + "csv_options": { + "type": "object", + "additionalProperties": false, + "properties": { + "chunk_size": { "type": "integer", "minimum": 1 }, + "delimiter": { "type": "string", "minLength": 1 }, + "quotechar": { "type": "string", "minLength": 1 }, + "escapechar": { "type": "string", "minLength": 1 } + }, + "description": "Pandas read_csv passthrough. Defaults: chunk_size=1000, delimiter=',', quotechar='\"', escapechar='\\\\'." + }, + "file_options": { + "type": "object", + "additionalProperties": true, + "properties": { + "target_size": { + "type": "array", + "items": { "type": "integer", "minimum": 1 }, + "minItems": 2, + "maxItems": 2, + "description": "[height, width]. Image categories only. Default [512, 512]." + }, + "extension": { + "type": "string", + "enum": [".jpg", ".jpeg", ".png", ".txt", ".text", ".xml"], + "description": "Allowed extension for sidecar files. Defaults: .jpg for image categories, .txt for text_classification." + } + } + }, + "validators": { + "type": "array", + "items": { "type": "string" }, + "description": "Override the default validator set resolved by `map_validators(category)`. Rarely needed." + }, + "sidecars": { + "type": "array", + "items": { + "type": "object", + "additionalProperties": false, + "required": ["column", "source"], + "properties": { + "column": { "type": "string", "minLength": 1 }, + "source": { "type": "string", "minLength": 1 }, + "dest": { "type": "string", "minLength": 1 }, + "transform": { + "type": "object", + "additionalProperties": false, + "properties": { + "resize": { + "type": "array", + "items": { "type": "integer", "minimum": 1 }, + "minItems": 2, + "maxItems": 2 + } + } + }, + "validators": { + "type": "array", + "items": { "type": "string" } + } + } + }, + "description": "Advanced: explicit sidecar-file declaration when the convention defaults don't fit. Most users rely on `images`/`annotations`/`masks`/`texts` shorthand instead." + }, + "processors": { + "type": "array", + "items": { + "type": "object", + "additionalProperties": false, + "required": ["script", "class"], + "properties": { + "script": { + "type": "string", + "minLength": 1, + "description": "Path inside the pod where the script body has been mounted (typically a ConfigMap mount under /custom/...)." + }, + "class": { + "type": "string", + "minLength": 1, + "description": "Class name within the script. Must subclass tracebloc_ingestor.processors.BaseProcessor." + }, + "args": { + "type": "object", + "description": "Keyword arguments passed to the processor's constructor." + } + } + }, + "description": "Custom-processor escape hatch. The Helm subchart (client#86) takes the script body as a chart value, writes a ConfigMap, mounts it. The official image dynamically imports the class and applies it to every record. No customer-built Docker image required." + } + } + } + }, + + "allOf": [ + { + "description": "Exactly one data source: csv or json.", + "oneOf": [ + { "required": ["csv"], "not": { "required": ["json"] } }, + { "required": ["json"], "not": { "required": ["csv"] } } + ] + }, + { + "description": "Image-based categories require `images`.", + "if": { + "properties": { + "category": { + "enum": [ + "image_classification", + "object_detection", + "keypoint_detection", + "semantic_segmentation", + "instance_segmentation" + ] + } + }, + "required": ["category"] + }, + "then": { "required": ["images"] } + }, + { + "description": "object_detection requires `annotations`.", + "if": { + "properties": { "category": { "const": "object_detection" } }, + "required": ["category"] + }, + "then": { "required": ["annotations"] } + }, + { + "description": "semantic_segmentation requires `masks`.", + "if": { + "properties": { "category": { "const": "semantic_segmentation" } }, + "required": ["category"] + }, + "then": { "required": ["masks"] } + }, + { + "description": "text_classification requires `texts`.", + "if": { + "properties": { "category": { "const": "text_classification" } }, + "required": ["category"] + }, + "then": { "required": ["texts"] } + }, + { + "description": "masked_language_modeling requires `sequences`.", + "if": { + "properties": { "category": { "const": "masked_language_modeling" } }, + "required": ["category"] + }, + "then": { "required": ["sequences"] } + }, + { + "description": "tabular and time-series categories require `schema`.", + "if": { + "properties": { + "category": { + "enum": [ + "tabular_classification", + "tabular_regression", + "time_series_forecasting", + "time_to_event_prediction" + ] + } + }, + "required": ["category"] + }, + "then": { "required": ["schema"] } + }, + { + "description": "Regression-class tasks require an explicit label.policy decision (must be the object form).", + "if": { + "properties": { + "category": { + "enum": [ + "tabular_regression", + "time_series_forecasting", + "time_to_event_prediction" + ] + } + }, + "required": ["category"] + }, + "then": { + "properties": { + "label": { + "type": "object", + "required": ["column", "policy"] + } + }, + "required": ["label"] + } + }, + { + "description": "Most categories require `label`.", + "if": { + "properties": { + "category": { + "enum": [ + "image_classification", + "object_detection", + "keypoint_detection", + "semantic_segmentation", + "instance_segmentation", + "text_classification", + "tabular_classification" + ] + } + }, + "required": ["category"] + }, + "then": { "required": ["label"] } + } + ] +} diff --git a/internal/schema/validate.go b/internal/schema/validate.go new file mode 100644 index 00000000..b71266e8 --- /dev/null +++ b/internal/schema/validate.go @@ -0,0 +1,238 @@ +package schema + +import ( + "bytes" + "encoding/json" + "fmt" + "sort" + "strings" + + "github.com/santhosh-tekuri/jsonschema/v6" + "golang.org/x/text/language" + "golang.org/x/text/message" + "gopkg.in/yaml.v3" +) + +// englishPrinter is the per-package message.Printer we pass to +// jsonschema's ErrorKind.LocalizedString. The library panics on a +// nil printer (it tries to construct one with a nil language tag); +// having a process-wide English printer avoids that and keeps the +// output language stable across customer environments. Internationalizing +// the validator output is a v2 concern. +var englishPrinter = message.NewPrinter(language.English) + +// V1SchemaID is the $id of the embedded v1 schema. Used as the +// "source URL" when compiling — jsonschema/v6 needs a name to +// associate the loaded bytes with, and the canonical $id keeps +// error messages anchored to the same identifier the Python +// implementation uses. +const V1SchemaID = "https://tracebloc.io/schemas/ingest.v1.json" + +// ValidationError is a single schema violation, normalized to the +// ">: " shape that +// tracebloc_ingestor.cli.run._format_errors emits in Python. Pinning +// the format lets us tell customers "the CLI and the server-side +// validator say the same thing about your YAML" with high +// confidence. +type ValidationError struct { + // Path is the json-pointer-style location, e.g. "spec.processors.0.script". + // "" for top-level violations (additionalProperties, + // missing required fields at the document level, etc.). + Path string + + // Message is the human-readable description of the violation, + // taken directly from the underlying jsonschema/v6 library's + // per-error message. The wording is stable across the library's + // minor versions; we don't post-process it. + Message string +} + +// Format returns the canonical " : " line, matching +// the indentation the Python implementation produces. The leading +// two spaces are deliberate — they match _format_errors so customer +// docs / runbooks can reference one wording across both +// implementations. +func (e ValidationError) Format() string { + return fmt.Sprintf(" %s: %s", e.Path, e.Message) +} + +// FormatErrors renders a slice of violations as one error per line, +// deterministically ordered by Path then Message. Mirrors +// _format_errors. +// +// Pure: the input slice's order is preserved. An earlier version +// called sort.Slice directly on errs, which silently reordered the +// caller's underlying array — bugbot caught this as a hidden side +// effect that contradicted the function's name + doc. Copying the +// slice header before sorting keeps FormatErrors a true formatter. +func FormatErrors(errs []ValidationError) string { + // slices.Clone would be cleaner but it's Go 1.21+; the manual + // copy is portable and the allocation is tiny relative to the + // schema validation itself. + sorted := make([]ValidationError, len(errs)) + copy(sorted, errs) + + sort.Slice(sorted, func(i, j int) bool { + if sorted[i].Path != sorted[j].Path { + return sorted[i].Path < sorted[j].Path + } + return sorted[i].Message < sorted[j].Message + }) + + var b strings.Builder + for i, e := range sorted { + if i > 0 { + b.WriteByte('\n') + } + b.WriteString(e.Format()) + } + return b.String() +} + +// Validator wraps a compiled jsonschema.Schema for repeated use. +// Compile once at process startup; validate many. Schema compilation +// is non-trivial (the v1 schema has nested if/then/oneOf chains); +// reusing the compiled form is meaningfully faster for batch +// validation. +type Validator struct { + schema *jsonschema.Schema +} + +// NewV1Validator compiles the embedded v1 schema. Returns an error +// only if the embedded JSON is malformed — which can only happen if +// scripts/sync-schema.sh wrote garbage, in which case the build's +// embed step or the CI drift check would have caught it. The error +// path stays for defense-in-depth. +func NewV1Validator() (*Validator, error) { + var raw any + if err := json.Unmarshal(V1Bytes, &raw); err != nil { + return nil, fmt.Errorf("embedded schema is not valid JSON: %w", err) + } + + c := jsonschema.NewCompiler() + if err := c.AddResource(V1SchemaID, raw); err != nil { + return nil, fmt.Errorf("registering embedded schema: %w", err) + } + s, err := c.Compile(V1SchemaID) + if err != nil { + return nil, fmt.Errorf("compiling embedded schema: %w", err) + } + return &Validator{schema: s}, nil +} + +// ValidateYAML parses the input as YAML and validates the resulting +// document against the schema. Returns the parsed document (useful +// for callers that want to inspect category/table/etc. after a +// successful validation) plus the list of violations. +// +// Two failure modes are distinct and important to separate: +// +// - The input isn't valid YAML at all (parseErr != nil): callers +// should surface this as a parse-level error, separately from +// schema violations. +// - The input parses but doesn't match the schema (errs non-empty): +// these are the customer-facing "your config has problems" cases. +func (v *Validator) ValidateYAML(input []byte) (parsed map[string]any, errs []ValidationError, parseErr error) { + if len(bytes.TrimSpace(input)) == 0 { + return nil, nil, fmt.Errorf("input is empty") + } + + var doc any + if err := yaml.Unmarshal(input, &doc); err != nil { + return nil, nil, fmt.Errorf("not valid YAML: %w", err) + } + + // The schema expects a mapping at the top. Anything else (a + // sequence, a scalar) gets surfaced as a parse-level error so + // the customer doesn't see a wall of unhelpful schema messages + // blaming the document's type. + asMap, ok := doc.(map[string]any) + if !ok { + return nil, nil, fmt.Errorf( + "document must be a YAML mapping at the top level (apiVersion / kind / category / ...); got %T", + doc, + ) + } + + // Schema validators expect canonical Go-native types + // (map[string]any, []any, string, float64, bool, nil). yaml.v3 + // produces those directly from Unmarshal-into-any, so no + // conversion needed. + if err := v.schema.Validate(doc); err != nil { + // Recurse the error tree into our flat ValidationError list. + // jsonschema/v6 returns a *ValidationError tree where each + // node may have child Causes — we flatten to leaves so the + // customer sees one line per actual problem, not an outline + // of the validator's traversal path. + var ve *jsonschema.ValidationError + if errors_as(err, &ve) { + errs = flattenValidationError(ve) + } else { + // Defensive: shouldn't happen with current jsonschema/v6, + // but fall back to the raw error string rather than + // crashing. + errs = []ValidationError{{Path: "", Message: err.Error()}} + } + } + + return asMap, errs, nil +} + +// flattenValidationError walks the jsonschema error tree to produce +// one leaf error per real violation. The library returns a tree +// because oneOf / anyOf / allOf can fail in multiple ways at once; +// we want each leaf surfaced individually so the customer can see +// every problem with a single validate run. +func flattenValidationError(ve *jsonschema.ValidationError) []ValidationError { + if ve == nil { + return nil + } + + // Internal nodes (with causes) carry the structural context; + // the actual violations live at the leaves. + if len(ve.Causes) > 0 { + var out []ValidationError + for _, c := range ve.Causes { + out = append(out, flattenValidationError(c)...) + } + return out + } + + return []ValidationError{{ + Path: instanceLocationToPath(ve.InstanceLocation), + Message: ve.ErrorKind.LocalizedString(englishPrinter), + }} +} + +// instanceLocationToPath converts jsonschema's slice-of-strings +// instance pointer into the dotted form data-ingestors uses +// (matching _format_errors's `".".join(...)`). An empty location +// becomes "" so the customer sees something concrete instead +// of a blank-looking error line. +func instanceLocationToPath(loc []string) string { + if len(loc) == 0 { + return "" + } + return strings.Join(loc, ".") +} + +// errors_as is a tiny inlining of errors.As to avoid the import +// just for this one site. Keeps the dependency graph of this file +// minimal; the only third-party imports stay jsonschema + yaml. +func errors_as(err error, target **jsonschema.ValidationError) bool { + for cur := err; cur != nil; cur = unwrap(cur) { + if ve, ok := cur.(*jsonschema.ValidationError); ok { + *target = ve + return true + } + } + return false +} + +func unwrap(err error) error { + type unwrapper interface{ Unwrap() error } + if u, ok := err.(unwrapper); ok { + return u.Unwrap() + } + return nil +} diff --git a/internal/schema/validate_test.go b/internal/schema/validate_test.go new file mode 100644 index 00000000..c09ae593 --- /dev/null +++ b/internal/schema/validate_test.go @@ -0,0 +1,405 @@ +package schema + +import ( + "os" + "path/filepath" + "strings" + "testing" +) + +// Build the validator once; reuse across cases. Compilation cost +// shouldn't be in the critical path of every test. +func mustValidator(t *testing.T) *Validator { + t.Helper() + v, err := NewV1Validator() + if err != nil { + t.Fatalf("compile embedded schema: %v", err) + } + return v +} + +func TestEmbeddedSchemaCompiles(t *testing.T) { + // Compiling is the most basic invariant — if the bundled JSON + // is malformed, every other test fails too, but this gives the + // clearest first signal. + _ = mustValidator(t) +} + +func TestEmbeddedSchemaIsV1ByID(t *testing.T) { + // Spot-check that the bundled bytes are actually the v1 schema + // (not, say, a stale draft or a v2 that got pulled by accident). + // We grep on the canonical $id, not on schema structure, so a + // future field rename or example update doesn't break this test + // for spurious reasons. + if !strings.Contains(string(V1Bytes), V1SchemaID) { + t.Fatalf("embedded schema doesn't mention its expected $id %q", V1SchemaID) + } +} + +// Each of the canonical examples in tracebloc/data-ingestors must +// validate cleanly — if any fail, either the example is broken or +// the schema's drifted from what the examples assume. Pin them by +// inlining (the alternative is loading from an out-of-tree path +// which makes the test brittle to data-ingestors layout changes). +// +// The fixtures intentionally mirror examples/yaml/.yaml +// from data-ingestors. When that set grows, this slice grows too. +func TestValidate_HappyPath_AllCategories(t *testing.T) { + cases := []struct { + name string + yaml string + }{ + { + name: "image_classification", + yaml: ` +apiVersion: tracebloc.io/v1 +kind: IngestConfig +table: chest_xrays_train +intent: train +category: image_classification +csv: /data/labels.csv +images: /data/images/ +label: image_label +`, + }, + { + name: "object_detection", + yaml: ` +apiVersion: tracebloc.io/v1 +kind: IngestConfig +table: visdrone_train +intent: train +category: object_detection +csv: /data/labels.csv +images: /data/images/ +annotations: /data/annotations/ +label: image_label +`, + }, + { + name: "semantic_segmentation", + yaml: ` +apiVersion: tracebloc.io/v1 +kind: IngestConfig +table: tumors_train +intent: train +category: semantic_segmentation +csv: /data/labels.csv +images: /data/images/ +masks: /data/masks/ +label: image_label +`, + }, + { + name: "text_classification", + yaml: ` +apiVersion: tracebloc.io/v1 +kind: IngestConfig +table: support_tickets_train +intent: train +category: text_classification +csv: /data/labels.csv +texts: /data/texts/ +schema: + text_id: VARCHAR(255) + label: VARCHAR(64) +label: label +`, + }, + { + name: "tabular_classification", + yaml: ` +apiVersion: tracebloc.io/v1 +kind: IngestConfig +table: churn_train +intent: train +category: tabular_classification +csv: /data/customers.csv +schema: + age: INT + tenure_months: INT + churned: VARCHAR(8) +label: churned +`, + }, + { + name: "tabular_regression_requires_bucket_policy", + yaml: ` +apiVersion: tracebloc.io/v1 +kind: IngestConfig +table: house_prices_train +intent: train +category: tabular_regression +csv: /data/houses.csv +schema: + square_feet: FLOAT + price: FLOAT +label: + column: price + policy: bucket +`, + }, + } + + v := mustValidator(t) + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + _, errs, parseErr := v.ValidateYAML([]byte(c.yaml)) + if parseErr != nil { + t.Fatalf("YAML parse failed: %v", parseErr) + } + if len(errs) > 0 { + t.Errorf("expected clean validation, got %d issue(s):\n%s", + len(errs), FormatErrors(errs)) + } + }) + } +} + +// Negative cases — each exercises a different schema rule. The +// assertion is on the path the error reports (not the exact message +// wording, which is jsonschema/v6's responsibility to define) so +// library version bumps don't break the test for cosmetic reasons. +func TestValidate_NegativeCases(t *testing.T) { + cases := []struct { + name string + yaml string + wantPaths []string // substring match on Path field of any violation + }{ + { + name: "missing apiVersion + intent", + yaml: ` +kind: IngestConfig +category: image_classification +table: t +csv: /data/labels.csv +images: /data/images/ +label: my_label +`, + wantPaths: []string{""}, // both go to root level + }, + { + name: "category not in enum", + yaml: ` +apiVersion: tracebloc.io/v1 +kind: IngestConfig +table: t +intent: train +category: not_a_category +csv: /data/labels.csv +images: /data/images/ +label: my_label +`, + wantPaths: []string{"category"}, + }, + { + name: "image category missing images key", + yaml: ` +apiVersion: tracebloc.io/v1 +kind: IngestConfig +table: t +intent: train +category: image_classification +csv: /data/labels.csv +label: my_label +`, + wantPaths: []string{""}, // missing required at root + }, + { + name: "regression without explicit label.policy", + yaml: ` +apiVersion: tracebloc.io/v1 +kind: IngestConfig +table: t +intent: train +category: tabular_regression +csv: /data/houses.csv +schema: + price: FLOAT +label: price +`, + wantPaths: []string{"label"}, // schema enforces object form for regression + }, + { + name: "tabular missing schema block", + yaml: ` +apiVersion: tracebloc.io/v1 +kind: IngestConfig +table: t +intent: train +category: tabular_classification +csv: /data/customers.csv +label: churned +`, + wantPaths: []string{""}, + }, + } + + v := mustValidator(t) + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + _, errs, parseErr := v.ValidateYAML([]byte(c.yaml)) + if parseErr != nil { + t.Fatalf("YAML parse failed: %v", parseErr) + } + if len(errs) == 0 { + t.Fatalf("expected at least one schema violation, got none") + } + + gotPaths := make([]string, 0, len(errs)) + for _, e := range errs { + gotPaths = append(gotPaths, e.Path) + } + + for _, want := range c.wantPaths { + found := false + for _, got := range gotPaths { + if strings.Contains(got, want) { + found = true + break + } + } + if !found { + t.Errorf("expected a violation with path containing %q, got paths: %v\nfull errors:\n%s", + want, gotPaths, FormatErrors(errs)) + } + } + }) + } +} + +// Parse-level failures (vs schema violations) need a separate +// failure mode so callers can render them differently in the UI — +// "your file isn't YAML" vs "your file is YAML but doesn't match +// the schema" are different problems with different remediations. +func TestValidate_ParseFailures(t *testing.T) { + v := mustValidator(t) + + t.Run("empty input", func(t *testing.T) { + _, _, parseErr := v.ValidateYAML([]byte("")) + if parseErr == nil { + t.Fatal("expected parseErr on empty input, got nil") + } + if !strings.Contains(parseErr.Error(), "empty") { + t.Errorf("expected message to mention empty, got: %v", parseErr) + } + }) + + t.Run("whitespace only", func(t *testing.T) { + _, _, parseErr := v.ValidateYAML([]byte(" \n\t \n")) + if parseErr == nil { + t.Fatal("expected parseErr on whitespace-only input") + } + }) + + t.Run("non-mapping top level (sequence)", func(t *testing.T) { + _, _, parseErr := v.ValidateYAML([]byte("- one\n- two\n")) + if parseErr == nil { + t.Fatal("expected parseErr on top-level sequence") + } + if !strings.Contains(parseErr.Error(), "mapping") { + t.Errorf("expected message to mention 'mapping', got: %v", parseErr) + } + }) + + t.Run("malformed YAML", func(t *testing.T) { + // Trailing colon + missing value, mixed indent — yaml.v3 + // flags this. + _, _, parseErr := v.ValidateYAML([]byte("foo:\n bar:\n\tbaz: 1\n")) + if parseErr == nil { + t.Fatal("expected parseErr on malformed YAML") + } + }) +} + +// FormatErrors is the contract with the UI layer; pin its exact +// output shape so any change is intentional. The leading two +// spaces mirror tracebloc_ingestor.cli.run._format_errors so +// customers see one wording across both implementations. +func TestFormatErrors_ContractShape(t *testing.T) { + errs := []ValidationError{ + {Path: "category", Message: "value must be one of …"}, + {Path: "", Message: "missing properties 'foo'"}, + } + got := FormatErrors(errs) + + // Sorted output: < category alphabetically. + want := " : missing properties 'foo'\n category: value must be one of …" + if got != want { + t.Errorf("FormatErrors mismatch\ngot:\n%s\nwant:\n%s", got, want) + } +} + +// Pins that FormatErrors does NOT mutate its input. An earlier +// version sorted in place, which made callers that wanted to +// further process `violations` after formatting see a different +// order from the one they passed in. Bugbot flagged this as a +// hidden side effect; this test ensures the regression can't +// silently come back. +func TestFormatErrors_DoesNotMutateInput(t *testing.T) { + in := []ValidationError{ + {Path: "z_field", Message: "bad"}, + {Path: "a_field", Message: "bad"}, + } + original := append([]ValidationError(nil), in...) // capture pre-call order + + _ = FormatErrors(in) + + if in[0] != original[0] || in[1] != original[1] { + t.Errorf("FormatErrors mutated input order.\nbefore: %v\nafter: %v", + original, in) + } +} + +// Round-trip test against the canonical example YAMLs in the +// data-ingestors repo. If that checkout is missing, skip — we +// don't want CI to fail for a local layout mismatch. CI runs the +// test in the runner's filesystem where data-ingestors isn't +// checked out alongside us, so this only fires for local devs. +// +// Opt in by setting TRACEBLOC_INGESTORS_EXAMPLES to the absolute +// path of data-ingestors/examples/yaml (or any directory of +// `.yaml` files that follow the v1 schema). Without the env var +// the test skips silently — a hardcoded absolute path like +// `/Volumes/VPPD/...` was bugbot-flagged as leaking one +// developer's filesystem layout and being unusable by anyone else. +func TestValidate_AgainstRealExamplesIfPresent(t *testing.T) { + examplesDir := os.Getenv("TRACEBLOC_INGESTORS_EXAMPLES") + if examplesDir == "" { + t.Skip("TRACEBLOC_INGESTORS_EXAMPLES not set; skipping the round-trip-against-real-examples test " + + "(point it at data-ingestors/examples/yaml to enable)") + } + entries, err := os.ReadDir(examplesDir) + if err != nil { + t.Skipf("TRACEBLOC_INGESTORS_EXAMPLES=%q is not readable, skipping: %v", examplesDir, err) + } + + v := mustValidator(t) + for _, e := range entries { + if e.IsDir() || filepath.Ext(e.Name()) != ".yaml" { + continue + } + t.Run(e.Name(), func(t *testing.T) { + body, err := os.ReadFile(filepath.Join(examplesDir, e.Name())) + if err != nil { + t.Fatalf("read: %v", err) + } + _, errs, parseErr := v.ValidateYAML(body) + if parseErr != nil { + t.Fatalf("parse: %v", parseErr) + } + if len(errs) > 0 { + // custom_processor.yaml may legitimately not match + // (the example shows a feature deferred to v1.1); + // skip rather than fail so the test stays useful as + // the canonical examples evolve. + if strings.Contains(e.Name(), "custom_processor") { + t.Skipf("custom_processor example exercises deferred features; %d issue(s)", + len(errs)) + } + t.Errorf("expected clean, got %d issue(s):\n%s", + len(errs), FormatErrors(errs)) + } + }) + } +} diff --git a/scripts/sync-schema.sh b/scripts/sync-schema.sh new file mode 100755 index 00000000..a7f13d48 --- /dev/null +++ b/scripts/sync-schema.sh @@ -0,0 +1,86 @@ +#!/usr/bin/env bash +# Sync ingest.v1.json from tracebloc/data-ingestors into the CLI's +# embedded copy at internal/schema/ingest.v1.json. +# +# The CLI validates locally using this schema. Drift between the +# CLI's copy and data-ingestors' canonical source is a real +# correctness hazard — a customer's YAML could pass `tracebloc ingest +# validate` locally but be rejected by jobs-manager (or vice versa). +# +# Run this script when bumping the schema version. CI invokes it in +# check-mode (`--check`) to fail builds that have drifted without +# running the sync first. +# +# Usage: +# scripts/sync-schema.sh # write into internal/schema/ +# scripts/sync-schema.sh --check # verify in-tree copy matches upstream; exit non-zero on drift +# +# Env knobs: +# SCHEMA_SOURCE_URL override the upstream URL (default: data-ingestors' master) +# SCHEMA_OUT override the in-tree destination (default: internal/schema/ingest.v1.json) +# +# Future: when we cut a v2 schema, this script will need to learn +# about multiple versions (e.g. embed v1 AND v2 side-by-side, picked +# at runtime based on the ingest.yaml's apiVersion). Keeping the path +# explicit in SCHEMA_OUT makes that extension easy. + +set -euo pipefail + +readonly DEFAULT_URL="https://raw.githubusercontent.com/tracebloc/data-ingestors/master/tracebloc_ingestor/schema/ingest.v1.json" +readonly DEFAULT_OUT="internal/schema/ingest.v1.json" + +SCHEMA_SOURCE_URL="${SCHEMA_SOURCE_URL:-$DEFAULT_URL}" +SCHEMA_OUT="${SCHEMA_OUT:-$DEFAULT_OUT}" + +CHECK_MODE=false +if [[ "${1:-}" == "--check" ]]; then + CHECK_MODE=true +fi + +# Stage the fetched schema in a temp file so a half-failed curl doesn't +# leave a truncated file in the repo. +tmp=$(mktemp) +trap 'rm -f "$tmp"' EXIT + +echo "==> fetching $SCHEMA_SOURCE_URL" +curl -fsSL "$SCHEMA_SOURCE_URL" -o "$tmp" + +# Make sure what came back is valid JSON before we trust it. +if ! python3 -m json.tool < "$tmp" > /dev/null 2>&1; then + echo "error: upstream response is not valid JSON" >&2 + echo "first 200 bytes of response:" >&2 + head -c 200 "$tmp" >&2 + exit 2 +fi + +mkdir -p "$(dirname "$SCHEMA_OUT")" + +if $CHECK_MODE; then + if [[ ! -f "$SCHEMA_OUT" ]]; then + echo "error: $SCHEMA_OUT does not exist" >&2 + echo "run \`scripts/sync-schema.sh\` (without --check) to seed it." >&2 + exit 1 + fi + if ! diff -q "$tmp" "$SCHEMA_OUT" >/dev/null; then + echo "error: $SCHEMA_OUT has drifted from upstream." >&2 + echo "diff (upstream → in-tree):" >&2 + diff -u "$SCHEMA_OUT" "$tmp" | head -40 >&2 || true + echo >&2 + echo "to fix, run \`scripts/sync-schema.sh\` and commit the result." >&2 + exit 1 + fi + echo "==> $SCHEMA_OUT matches upstream — no drift" + exit 0 +fi + +# Write mode. Only touch the destination if the content actually +# changed, so re-running the script on an already-current schema +# produces no file-mtime churn. +if [[ -f "$SCHEMA_OUT" ]] && diff -q "$tmp" "$SCHEMA_OUT" >/dev/null; then + echo "==> $SCHEMA_OUT already matches upstream — no change" + exit 0 +fi + +mv "$tmp" "$SCHEMA_OUT" +trap - EXIT # the temp file is now in place; don't try to rm it +echo "==> wrote $SCHEMA_OUT ($(wc -c < "$SCHEMA_OUT" | tr -d ' ') bytes)"