From 7add0e1946ffff6e665500e7d1acb6b627d206fc Mon Sep 17 00:00:00 2001 From: Daniel Garcia Date: Sat, 18 Jul 2026 11:19:02 -0500 Subject: [PATCH] Add apx client verify generate-and-compile gate A spec can be valid OpenAPI 3 and pass lint/breaking yet produce a client that does not compile (e.g. redundant _limit/limit query params collapsing to one Go field, or a path param named url shadowing net/url). Add 'apx client verify', which generates a client and compiles it, failing when any generated client does not build. --warn-only (or release.verify_clients.warn_only) downgrades a failure to a warning for staged rollout; a generator whose toolchain is absent is skipped, so the gate covers Go always and TypeScript where Node is present. Centralize the Builder-or-npm build dispatch in internal/client, shared by generate --build, publish, and verify. Add a regression fixture reproducing both patterns plus network-gated E2E tests. Fixes #40 --- CHANGELOG.md | 15 ++ apx.example.yaml | 5 + cmd/apx/commands/client.go | 158 +++++++++++++----- docs/cli-reference/configuration.md | 27 +++ docs/releasing/release-guardrails.md | 20 +++ internal/client/angular.go | 13 ++ internal/client/build.go | 54 ++++++ internal/client/goclient.go | 9 + internal/client/verify.go | 161 ++++++++++++++++++ internal/client/verify_e2e_test.go | 74 +++++++++ internal/client/verify_test.go | 201 +++++++++++++++++++++++ internal/config/config.go | 18 +- internal/config/schema.go | 19 +++ testdata/goclient/collision.openapi.yaml | 66 ++++++++ 14 files changed, 796 insertions(+), 44 deletions(-) create mode 100644 internal/client/build.go create mode 100644 internal/client/verify.go create mode 100644 internal/client/verify_e2e_test.go create mode 100644 internal/client/verify_test.go create mode 100644 testdata/goclient/collision.openapi.yaml diff --git a/CHANGELOG.md b/CHANGELOG.md index 10e8a97..c44d607 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,21 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added + +- **`apx client verify`** — a generate-and-compile release gate. It generates an + API client and **compiles** it, failing when any generated client does not + build, so a spec that is valid OpenAPI 3 and passes `apx lint`/`apx breaking` + but produces a non-compiling client (e.g. redundant `_limit`/`limit` query + params that normalize to the same Go field, or a path parameter named `url` + that shadows the `net/url` import) is caught before release. Each generator + runs in a throwaway directory; a generator whose toolchain is absent is skipped + (not failed), so the gate covers Go always and TypeScript where Node is + present. `--warn-only` (or `release.verify_clients.warn_only`) downgrades a + failure to a warning for staged rollout; `release.verify_clients.generators` + sets the matrix. The Builder-or-npm build dispatch is now centralized in + `internal/client` and shared by `generate --build`, `publish`, and `verify`. + ### Fixed - **Setup APX action** now installs `spectral` so the OpenAPI lint re-validation diff --git a/apx.example.yaml b/apx.example.yaml index 9d815fc..8126714 100644 --- a/apx.example.yaml +++ b/apx.example.yaml @@ -76,6 +76,11 @@ policy: release: tag_format: "{subdir}/v{version}" ci_only: true + # Generate-and-compile gate (apx client verify): generate a client and build + # it so a spec that cannot produce a buildable client is caught before release. + verify_clients: + generators: [go, typescript-angular] # empty => default matrix + warn_only: false # gate by default; --warn-only overrides tools: buf: version: v1.66.1 diff --git a/cmd/apx/commands/client.go b/cmd/apx/commands/client.go index 68d5dd8..2edfc4a 100644 --- a/cmd/apx/commands/client.go +++ b/cmd/apx/commands/client.go @@ -1,7 +1,6 @@ package commands import ( - "context" "fmt" "os" "os/exec" @@ -26,6 +25,7 @@ func newClientCmd() *cobra.Command { } cmd.AddCommand(newClientGenerateCmd()) cmd.AddCommand(newClientPublishCmd()) + cmd.AddCommand(newClientVerifyCmd()) return cmd } @@ -75,6 +75,119 @@ func newClientPublishCmd() *cobra.Command { return cmd } +func newClientVerifyCmd() *cobra.Command { + cmd := &cobra.Command{ + Use: "verify [target]", + Short: "Generate and compile a client to prove a spec is buildable", + Long: fmt.Sprintf("Generate an API client for one or more generators and compile it, failing the\n"+ + "command if any client does not build. This is the release gate that stops a spec\n"+ + "which cannot produce a buildable client from being published (a valid OpenAPI\n"+ + "spec can still generate Go that does not compile).\n\n"+ + "By default it verifies the %s generators; TypeScript is skipped where its Node\n"+ + "toolchain is absent. Use --generator to scope, and --warn-only to downgrade a\n"+ + "generate/compile failure to a warning (exit 0) instead of failing the gate.\n\n"+ + "Available generators: %s", + strings.Join(client.DefaultVerifyGenerators, ", "), strings.Join(client.Names(), ", ")), + Args: cobra.MaximumNArgs(1), + RunE: clientVerifyAction, + } + cmd.Flags().String("input", "", "OpenAPI spec path (overrides config/auto-detect)") + cmd.Flags().String("from", "", "api-id of an apx.lock dependency to source the spec from (unreleased override)") + cmd.Flags().StringSlice("generator", nil, "generator(s) to verify (repeatable); default: release.verify_clients.generators, else go + typescript-angular") + cmd.Flags().String("package", "", "package/module name stamped into the generated client") + cmd.Flags().String("scope", "", "npm scope for the package (e.g. @example)") + cmd.Flags().Bool("warn-only", false, "downgrade a generate/compile failure to a warning and exit 0 (default: fail the gate)") + return cmd +} + +// clientVerifyAction resolves a spec and generator matrix, generates+compiles a +// client per generator, and turns the aggregate result into an exit code: it +// fails when any verified client does not build, unless --warn-only (or the +// release.verify_clients.warn_only config default) downgrades that to a warning. +func clientVerifyAction(cmd *cobra.Command, args []string) error { + target := "" + if len(args) > 0 { + target = args[0] + } + input, _ := cmd.Flags().GetString("input") + from, _ := cmd.Flags().GetString("from") + pkg, _ := cmd.Flags().GetString("package") + scope, _ := cmd.Flags().GetString("scope") + generators, _ := cmd.Flags().GetStringSlice("generator") + + // Best-effort config load; a valid apx.yaml is not required (e.g. with --input). + cfg, _ := config.LoadRaw("") + + var ct *config.ClientTarget + if target != "" { + ct = findClientTarget(cfg, target) + if ct == nil { + return fmt.Errorf("no client target %q found in apx.yaml", target) + } + if scope == "" { + scope = ct.Scope + } + if pkg == "" { + pkg = ct.Package + } + } + + specPath, err := resolveClientSpec(input, from, ct, target) + if err != nil { + return err + } + + // Generator matrix precedence: flag > config > built-in default (the empty + // slice is resolved to client.DefaultVerifyGenerators inside Verify). + if len(generators) == 0 && cfg != nil { + generators = cfg.Release.VerifyClients.Generators + } + + // warn-only precedence: an explicit flag wins; otherwise the config default. + warnOnly, _ := cmd.Flags().GetBool("warn-only") + if !cmd.Flags().Changed("warn-only") && cfg != nil { + warnOnly = cfg.Release.VerifyClients.WarnOnly + } + + ui.Info("Verifying client build for %s ...", specPath) + report, err := client.Verify(cmd.Context(), client.VerifyOptions{ + SpecPath: specPath, + Generators: generators, + PackageName: pkg, + Scope: scope, + }) + if err != nil { + return fmt.Errorf("verifying client: %w", err) + } + + for _, r := range report.Results { + switch { + case r.Skipped: + ui.Warning(" %s: skipped (%s)", r.Generator, r.Reason) + case r.OK: + ui.Success(" %s: ok", r.Generator) + default: + ui.Error(" %s: FAILED — %v", r.Generator, r.Err) + } + } + + if report.Ran() == 0 { + ui.Warning("No generators were verified (all skipped). Install the required toolchain(s) to run the gate.") + return nil + } + + if report.Failed() { + if warnOnly { + ui.Warning("Client verification failed, but --warn-only is set — not failing the gate.") + return nil + } + return fmt.Errorf("client verification failed: one or more generated clients did not build") + } + + ui.Success("All verified clients built successfully") + return nil +} + // resolveClientContext applies the shared target/flag/spec resolution and // returns the selected generator plus the fully-resolved GenerateContext. // @@ -175,7 +288,7 @@ func clientGenerateAction(cmd *cobra.Command, args []string) error { } if doBuild { - if err := buildClient(cmd.Context(), gen, res); err != nil { + if err := client.Build(cmd.Context(), gen, res); err != nil { return fmt.Errorf("building client package: %w", err) } } @@ -203,7 +316,7 @@ func clientPublishAction(cmd *cobra.Command, args []string) error { // Build first: a Builder-aware generator (e.g. go) compile-verifies; the npm // path builds dist/ which `npm publish` needs. - if err := buildClient(cmd.Context(), gen, res); err != nil { + if err := client.Build(cmd.Context(), gen, res); err != nil { return fmt.Errorf("building client package: %w", err) } @@ -365,45 +478,6 @@ func defaultPackageName(specPath string) string { return base + "-client" } -// buildClient compile-verifies the generated package. A Builder-aware generator -// (e.g. the go generator: `go mod tidy` + `go build`) supplies its own build; -// generators without a Builder (e.g. typescript-angular) use the npm build path. -func buildClient(ctx context.Context, gen client.Generator, res client.Result) error { - if b, ok := gen.(client.Builder); ok { - return b.Build(ctx, res) - } - return buildClientPackage(res.PackageDir) -} - -// buildClientPackage runs npm install then npm run build in dir to prove the -// generated package compiles. -func buildClientPackage(dir string) error { - if _, err := exec.LookPath("npm"); err != nil { - return fmt.Errorf("npm required for --build; install Node >=18: %w", err) - } - - ui.Info("Running npm install in %s ...", dir) - install := exec.Command("npm", "install") - install.Dir = dir - install.Env = os.Environ() - install.Stdout = os.Stdout - install.Stderr = os.Stderr - if err := install.Run(); err != nil { - return fmt.Errorf("npm install failed: %w", err) - } - - ui.Info("Running npm run build in %s ...", dir) - build := exec.Command("npm", "run", "build") - build.Dir = dir - build.Env = os.Environ() - build.Stdout = os.Stdout - build.Stderr = os.Stderr - if err := build.Run(); err != nil { - return fmt.Errorf("npm run build failed: %w", err) - } - return nil -} - // publishClientPackage runs `npm publish` (or `npm publish --dry-run`) in dir. // The package's publishConfig selects the registry (GitHub Packages); auth for // a real publish is supplied by the environment (e.g. GITHUB_TOKEN in CI). diff --git a/docs/cli-reference/configuration.md b/docs/cli-reference/configuration.md index a3b54ee..3999ff9 100644 --- a/docs/cli-reference/configuration.md +++ b/docs/cli-reference/configuration.md @@ -74,6 +74,9 @@ version: 1 | `release` | struct | no | | | Release configuration | | `release.tag_format` | string | no | `{subdir}/v{version}` | | Tag pattern; must contain {version} | | `release.ci_only` | boolean | no | `true` | | Restrict releasing to CI environments | +| `release.verify_clients` | struct | no | | | Generate-and-compile gate (`apx client verify`) | +| `release.verify_clients.generators` | list | no | | | Generators to verify; empty means the default matrix (go, typescript-angular) | +| `release.verify_clients.warn_only` | boolean | no | `false` | | Downgrade a generate/compile failure to a warning instead of failing the gate (overridden by `--warn-only`) | | `tools` | struct | no | | | Pinned tool versions | | `tools.buf` | struct | no | | | Buf CLI settings | | `tools.buf.version` | string | no | | | Buf CLI version | @@ -383,6 +386,30 @@ Available generators: - `typescript-angular` (default) — orchestrates `ng-openapi-gen`, emits an npm package; built/published with `npm`. - `go` — orchestrates [`oapi-codegen`](https://github.com/oapi-codegen/oapi-codegen), emits a buildable Go module (typed client + models). For this generator `--package` is the Go **module path** (e.g. `github.com/you/your-client`) and `--scope` is ignored. `apx client generate --generator go --build` compile-verifies with `go build`; `apx client publish --generator go` records a `go-module` release artifact (Go modules are published by git tag in their own repo). The generated types honor the enriched OpenAPI contract from devedge-sdk (`enum`, `required`, `readOnly`/`writeOnly`); `x-aip-*` vendor extensions are ignored by the tool. +#### `apx client verify` — the generate-and-compile gate + +`apx client verify` generates a client and **compiles** it, failing when any generated client does not build. A spec can be valid OpenAPI 3 and pass `apx lint`/`apx breaking` yet still produce a client that does not compile — e.g. redundant `_limit`/`limit` query params that normalize to the same Go field, or a path parameter named `url` that shadows the `net/url` import. This gate catches those before a module is released. + +```bash +# Verify the auto-detected spec across the default matrix (go + typescript-angular): +apx client verify + +# Scope to one generator (typical in release CI): +apx client verify --generator go --input openapi/notesd.openapi.yaml + +# Soft-launch: report failures without failing the build: +apx client verify --warn-only +``` + +Each generator runs in a throwaway directory. A generator whose toolchain is absent is **skipped** (not failed), so a bare `apx client verify` covers Go always and TypeScript where Node is present. The command exits non-zero when any verified client fails to build, unless `--warn-only` (or [`release.verify_clients.warn_only`](#release)) downgrades that to a warning. Configure the matrix and default policy under `release.verify_clients`: + +```yaml +release: + verify_clients: + generators: [go, typescript-angular] # empty => default matrix + warn_only: false # gate by default; --warn-only overrides +``` + ### `external_apis` Registers third-party APIs for inclusion in the catalog and dependency system. See [External API Registration](../dependencies/external-apis.md) for full workflow documentation. diff --git a/docs/releasing/release-guardrails.md b/docs/releasing/release-guardrails.md index 0dcad95..9bae996 100644 --- a/docs/releasing/release-guardrails.md +++ b/docs/releasing/release-guardrails.md @@ -45,6 +45,26 @@ APX rejects backward transitions (e.g., `stable` → `beta`). | Breaking change on `v1+` stable | Rejected — requires new major line | | Breaking change on `deprecated` | Rejected — maintenance only | +## Client Build Verification + +A spec can be valid OpenAPI 3 and pass `apx lint`/`apx breaking` yet still +produce a client that does not compile — for example redundant `_limit`/`limit` +query params that normalize to the same Go field, or a path parameter named +`url` that shadows the `net/url` import. `apx client verify` closes that gap: it +generates a client and **compiles** it, failing the release when any generated +client does not build. + +| Scenario | APX behavior | +|----------|-------------| +| Generated client compiles | Passes | +| Generated client does not compile | Rejected — fails the gate | +| Generator toolchain absent (e.g. Node for TypeScript) | Skipped — not a failure | +| `--warn-only` (or `release.verify_clients.warn_only`) set | Failure downgraded to a warning (exit 0) | + +Run it in release CI between `apx breaking` and `apx release prepare`. See +[`apx client verify`](../cli-reference/configuration.md#apx-client-verify--the-generate-and-compile-gate) +for configuration. + ## See Also - [Lifecycle Reference](lifecycle.md) — full lifecycle state definitions and policies diff --git a/internal/client/angular.go b/internal/client/angular.go index cabf5a2..805518b 100644 --- a/internal/client/angular.go +++ b/internal/client/angular.go @@ -26,6 +26,19 @@ type angularGenerator struct{} func (g *angularGenerator) Name() string { return "typescript-angular" } +// ToolchainAvailable reports whether the Node toolchain (npx for ng-openapi-gen, +// npm for the build) is on PATH. Implements ToolchainChecker, so client +// verification skips this generator (rather than failing) where Node is absent. +func (g *angularGenerator) ToolchainAvailable() (bool, string) { + if _, err := exec.LookPath("npx"); err != nil { + return false, "npx not found on PATH (install Node >=18)" + } + if _, err := exec.LookPath("npm"); err != nil { + return false, "npm not found on PATH (install Node >=18)" + } + return true, "" +} + func (g *angularGenerator) Generate(ctx context.Context, gc GenerateContext) (Result, error) { // Preflight: the generator shells out to npx. if _, err := exec.LookPath("npx"); err != nil { diff --git a/internal/client/build.go b/internal/client/build.go new file mode 100644 index 0000000..3edf6b6 --- /dev/null +++ b/internal/client/build.go @@ -0,0 +1,54 @@ +package client + +import ( + "context" + "fmt" + "os" + "os/exec" + + "github.com/infobloxopen/apx/internal/ui" +) + +// Build compile-verifies the generated package described by res. A Builder-aware +// generator (e.g. the go generator: `go mod tidy` + `go build`) supplies its own +// build; generators without a Builder (e.g. typescript-angular) use the npm build +// path. The command layer MUST NOT publish a package whose Build failed. +// +// Centralized here (rather than in the command layer) so `generate --build`, +// `publish`, and `verify` all compile-verify identically. +func Build(ctx context.Context, gen Generator, res Result) error { + if b, ok := gen.(Builder); ok { + return b.Build(ctx, res) + } + return npmBuild(res.PackageDir) +} + +// npmBuild runs `npm install` then `npm run build` in dir to prove the generated +// package compiles. Output streams to the process stdio so build failures are +// visible verbatim. +func npmBuild(dir string) error { + if _, err := exec.LookPath("npm"); err != nil { + return fmt.Errorf("npm required to build the client; install Node >=18: %w", err) + } + + ui.Info("Running npm install in %s ...", dir) + install := exec.Command("npm", "install") + install.Dir = dir + install.Env = os.Environ() + install.Stdout = os.Stdout + install.Stderr = os.Stderr + if err := install.Run(); err != nil { + return fmt.Errorf("npm install failed: %w", err) + } + + ui.Info("Running npm run build in %s ...", dir) + build := exec.Command("npm", "run", "build") + build.Dir = dir + build.Env = os.Environ() + build.Stdout = os.Stdout + build.Stderr = os.Stderr + if err := build.Run(); err != nil { + return fmt.Errorf("npm run build failed: %w", err) + } + return nil +} diff --git a/internal/client/goclient.go b/internal/client/goclient.go index e6407a2..7593024 100644 --- a/internal/client/goclient.go +++ b/internal/client/goclient.go @@ -37,6 +37,15 @@ type goGenerator struct{} func (g *goGenerator) Name() string { return "go" } +// ToolchainAvailable reports whether the Go toolchain (needed for oapi-codegen +// via `go run` and for `go build`) is on PATH. Implements ToolchainChecker. +func (g *goGenerator) ToolchainAvailable() (bool, string) { + if _, err := exec.LookPath("go"); err != nil { + return false, "Go toolchain not found on PATH (install Go >=1.22)" + } + return true, "" +} + func (g *goGenerator) Generate(ctx context.Context, gc GenerateContext) (Result, error) { // Preflight: the generator shells out to `go run` for oapi-codegen. if _, err := exec.LookPath("go"); err != nil { diff --git a/internal/client/verify.go b/internal/client/verify.go new file mode 100644 index 0000000..98ed2e1 --- /dev/null +++ b/internal/client/verify.go @@ -0,0 +1,161 @@ +package client + +import ( + "context" + "fmt" + "os" + "path/filepath" + "strings" + + "github.com/infobloxopen/apx/internal/ui" +) + +// DefaultVerifyGenerators is the generator matrix Verify runs when a caller does +// not specify one. Go always runs — its toolchain ships with apx's release +// environment and it catches Go-specific codegen hazards (identifier collisions, +// reserved-name params). typescript-angular runs where a Node toolchain is +// present and is skipped otherwise (see ToolchainChecker). +var DefaultVerifyGenerators = []string{"go", "typescript-angular"} + +// defaultVerifyPackage is the throwaway package/module name stamped into a +// client generated purely to compile-verify a spec. +const defaultVerifyPackage = "apx-verify-client" + +// ToolchainChecker is an optional interface a Generator implements to report +// whether the external toolchain it orchestrates is available. Verify SKIPS +// (rather than fails) a generator whose toolchain is absent, so a bare +// `apx client verify` covers Go always and TypeScript "where a toolchain is +// present". Mirrors the optional Builder/Publisher interfaces. +type ToolchainChecker interface { + // ToolchainAvailable reports whether the generator can run in this + // environment. When false, reason explains what is missing. + ToolchainAvailable() (ok bool, reason string) +} + +// VerifyOptions configures a client-verification run. +type VerifyOptions struct { + // SpecPath is the OpenAPI v3 spec to verify (required). + SpecPath string + // Generators names the generators to verify; empty => DefaultVerifyGenerators. + Generators []string + // PackageName is the package/module name stamped into each generated client; + // empty => defaultVerifyPackage. + PackageName string + // Scope is the npm scope for generators that use one (ignored by go). + Scope string + // WorkDir is the parent directory for throwaway per-generator output. When + // empty, Verify uses a temp dir it removes before returning. + WorkDir string +} + +// GeneratorResult is the outcome of verifying a single generator. +type GeneratorResult struct { + Generator string + OK bool // generated AND compiled + Skipped bool // toolchain unavailable — not a gate failure + Reason string // human-readable skip reason + Err error // generate/build error when !OK && !Skipped +} + +// VerifyReport aggregates the per-generator results of a run. +type VerifyReport struct { + Results []GeneratorResult +} + +// Failed reports whether any verified (non-skipped) generator produced a client +// that did not generate+compile. This is the release-gate signal. +func (r VerifyReport) Failed() bool { + for _, res := range r.Results { + if !res.Skipped && !res.OK { + return true + } + } + return false +} + +// Ran reports how many generators were actually verified (not skipped). Callers +// use it to warn when every generator was skipped, so an all-skipped run does +// not masquerade as a passing gate. +func (r VerifyReport) Ran() int { + n := 0 + for _, res := range r.Results { + if !res.Skipped { + n++ + } + } + return n +} + +// Verify generates a client for each requested generator and compiles it, +// proving the spec produces a buildable client. Each generator runs in its own +// throwaway directory. Generate/compile failures are collected per generator +// (see VerifyReport.Failed) so a single run reports every broken generator; a +// genuinely unusable request — a missing spec or an unknown generator name — +// returns an error directly. +func Verify(ctx context.Context, opts VerifyOptions) (VerifyReport, error) { + var report VerifyReport + + if strings.TrimSpace(opts.SpecPath) == "" { + return report, fmt.Errorf("a spec path is required to verify a client") + } + if _, err := os.Stat(opts.SpecPath); err != nil { + return report, fmt.Errorf("OpenAPI spec not found at %q: %w", opts.SpecPath, err) + } + + gens := opts.Generators + if len(gens) == 0 { + gens = DefaultVerifyGenerators + } + + pkg := strings.TrimSpace(opts.PackageName) + if pkg == "" { + pkg = defaultVerifyPackage + } + + base := opts.WorkDir + if base == "" { + tmp, err := os.MkdirTemp("", "apx-client-verify-") + if err != nil { + return report, fmt.Errorf("creating temp work dir: %w", err) + } + defer os.RemoveAll(tmp) + base = tmp + } + + for _, name := range gens { + gen := Get(name) + if gen == nil { + return report, fmt.Errorf("unknown client generator %q; available: %s", + name, strings.Join(Names(), ", ")) + } + report.Results = append(report.Results, verifyGenerator(ctx, gen, GenerateContext{ + SpecPath: opts.SpecPath, + OutputDir: filepath.Join(base, name), + PackageName: pkg, + Scope: opts.Scope, + })) + } + return report, nil +} + +// verifyGenerator generates and compiles one client, classifying the outcome. +func verifyGenerator(ctx context.Context, gen Generator, gc GenerateContext) GeneratorResult { + name := gen.Name() + if tc, ok := gen.(ToolchainChecker); ok { + if avail, reason := tc.ToolchainAvailable(); !avail { + ui.Warning("Skipping %s client verification: %s", name, reason) + return GeneratorResult{Generator: name, Skipped: true, Reason: reason} + } + } + + ui.Info("Verifying %s client ...", name) + res, err := gen.Generate(ctx, gc) + if err != nil { + return GeneratorResult{Generator: name, Err: fmt.Errorf("generate: %w", err)} + } + if err := Build(ctx, gen, res); err != nil { + return GeneratorResult{Generator: name, Err: fmt.Errorf("build: %w", err)} + } + ui.Success("%s client generated and compiled", name) + return GeneratorResult{Generator: name, OK: true} +} diff --git a/internal/client/verify_e2e_test.go b/internal/client/verify_e2e_test.go new file mode 100644 index 0000000..11bbcf2 --- /dev/null +++ b/internal/client/verify_e2e_test.go @@ -0,0 +1,74 @@ +package client + +import ( + "context" + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// TestVerify_E2E_RejectsBrokenSpec is the regression guard: a spec that passes +// lint/breaking but whose generated Go client does not compile MUST fail +// verification. It runs the real oapi-codegen + go build via the go generator, +// so it is skipped under -short and skipped (not failed) when tool/module fetch +// is unavailable — matching TestGoGenerator_E2E. +func TestVerify_E2E_RejectsBrokenSpec(t *testing.T) { + if testing.Short() { + t.Skip("skipping network/tool e2e in -short mode") + } + report, err := Verify(context.Background(), VerifyOptions{ + SpecPath: brokenClientFixture(t), + Generators: []string{"go"}, + WorkDir: t.TempDir(), + }) + // Verify itself succeeds — the per-generator failure is carried in the report. + require.NoError(t, err) + require.Len(t, report.Results, 1) + + res := report.Results[0] + if res.Err != nil && looksLikeNetworkErr(res.Err) { + t.Skipf("skipping e2e: tool/module fetch unavailable: %v", res.Err) + } + assert.True(t, report.Failed(), "broken spec must fail client verification") + assert.False(t, res.OK) + require.Error(t, res.Err) + // The client is generated but does not compile: the failure is at build. + assert.Contains(t, res.Err.Error(), "build") +} + +// TestVerify_E2E_AcceptsGoodSpec proves the gate passes a spec whose generated +// Go client compiles cleanly (the enriched toy fixture). +func TestVerify_E2E_AcceptsGoodSpec(t *testing.T) { + if testing.Short() { + t.Skip("skipping network/tool e2e in -short mode") + } + report, err := Verify(context.Background(), VerifyOptions{ + SpecPath: goClientFixture(t), + Generators: []string{"go"}, + PackageName: "github.com/example/toy-client", + WorkDir: t.TempDir(), + }) + require.NoError(t, err) + require.Len(t, report.Results, 1) + + res := report.Results[0] + if res.Err != nil && looksLikeNetworkErr(res.Err) { + t.Skipf("skipping e2e: tool/module fetch unavailable: %v", res.Err) + } + assert.False(t, report.Failed(), "a good spec must pass verification") + assert.True(t, res.OK) +} + +// brokenClientFixture returns the path to the CICD regression spec that produces +// a non-compiling Go client (patterns A and B). +func brokenClientFixture(t *testing.T) string { + t.Helper() + p := filepath.Join("..", "..", "testdata", "goclient", "collision.openapi.yaml") + if _, err := os.Stat(p); err != nil { + t.Fatalf("fixture missing at %s: %v", p, err) + } + return p +} diff --git a/internal/client/verify_test.go b/internal/client/verify_test.go new file mode 100644 index 0000000..4435758 --- /dev/null +++ b/internal/client/verify_test.go @@ -0,0 +1,201 @@ +package client + +import ( + "context" + "errors" + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// fakeGen is a configurable Generator/Builder/ToolchainChecker test double. It +// never shells out: Generate returns a Result pointing at the output dir and +// Build returns buildErr, so Verify can be exercised without a real toolchain. +type fakeGen struct { + name string + genErr error + buildErr error + toolMisss bool // when true, ToolchainAvailable reports unavailable + toolReason string +} + +func (f *fakeGen) Name() string { return f.name } + +func (f *fakeGen) Generate(_ context.Context, gc GenerateContext) (Result, error) { + if f.genErr != nil { + return Result{}, f.genErr + } + return Result{PackageDir: gc.OutputDir, PackageName: gc.PackageName}, nil +} + +func (f *fakeGen) Build(_ context.Context, _ Result) error { return f.buildErr } + +func (f *fakeGen) ToolchainAvailable() (bool, string) { + if f.toolMisss { + return false, f.toolReason + } + return true, "" +} + +func dummySpec(t *testing.T) string { + t.Helper() + p := filepath.Join(t.TempDir(), "spec.yaml") + require.NoError(t, os.WriteFile(p, []byte("openapi: 3.0.0\n"), 0o644)) + return p +} + +func resultFor(rep VerifyReport, gen string) (GeneratorResult, bool) { + for _, r := range rep.Results { + if r.Generator == gen { + return r, true + } + } + return GeneratorResult{}, false +} + +func TestVerify_AllPass(t *testing.T) { + restore := resetForTesting() + defer restore() + Register(&fakeGen{name: "go"}) + Register(&fakeGen{name: "ts"}) + + rep, err := Verify(context.Background(), VerifyOptions{ + SpecPath: dummySpec(t), + Generators: []string{"go", "ts"}, + WorkDir: t.TempDir(), + }) + require.NoError(t, err) + assert.False(t, rep.Failed()) + assert.Equal(t, 2, rep.Ran()) + require.Len(t, rep.Results, 2) + for _, r := range rep.Results { + assert.True(t, r.OK, "%s should pass", r.Generator) + } +} + +func TestVerify_BuildFailureGates(t *testing.T) { + restore := resetForTesting() + defer restore() + Register(&fakeGen{name: "go", buildErr: errors.New("Limit redeclared")}) + + rep, err := Verify(context.Background(), VerifyOptions{ + SpecPath: dummySpec(t), + Generators: []string{"go"}, + WorkDir: t.TempDir(), + }) + require.NoError(t, err) + assert.True(t, rep.Failed()) + res, ok := resultFor(rep, "go") + require.True(t, ok) + assert.False(t, res.OK) + require.Error(t, res.Err) + assert.Contains(t, res.Err.Error(), "build") + assert.Contains(t, res.Err.Error(), "Limit redeclared") +} + +func TestVerify_GenerateFailureGates(t *testing.T) { + restore := resetForTesting() + defer restore() + Register(&fakeGen{name: "go", genErr: errors.New("oapi-codegen boom")}) + + rep, err := Verify(context.Background(), VerifyOptions{ + SpecPath: dummySpec(t), + Generators: []string{"go"}, + WorkDir: t.TempDir(), + }) + require.NoError(t, err) + assert.True(t, rep.Failed()) + res, _ := resultFor(rep, "go") + assert.Contains(t, res.Err.Error(), "generate") +} + +func TestVerify_SkipsWhenToolchainMissing(t *testing.T) { + restore := resetForTesting() + defer restore() + Register(&fakeGen{name: "ts", toolMisss: true, toolReason: "npx not found on PATH"}) + Register(&fakeGen{name: "go"}) + + rep, err := Verify(context.Background(), VerifyOptions{ + SpecPath: dummySpec(t), + Generators: []string{"ts", "go"}, + WorkDir: t.TempDir(), + }) + require.NoError(t, err) + assert.False(t, rep.Failed(), "a skip is not a gate failure") + assert.Equal(t, 1, rep.Ran(), "only go actually ran") + + ts, _ := resultFor(rep, "ts") + assert.True(t, ts.Skipped) + assert.Equal(t, "npx not found on PATH", ts.Reason) + assert.False(t, ts.OK) + + goRes, _ := resultFor(rep, "go") + assert.True(t, goRes.OK) +} + +func TestVerify_UnknownGeneratorErrors(t *testing.T) { + restore := resetForTesting() + defer restore() + Register(&fakeGen{name: "go"}) + + _, err := Verify(context.Background(), VerifyOptions{ + SpecPath: dummySpec(t), + Generators: []string{"bogus"}, + WorkDir: t.TempDir(), + }) + require.Error(t, err) + assert.Contains(t, err.Error(), "unknown client generator") +} + +func TestVerify_MissingSpecErrors(t *testing.T) { + restore := resetForTesting() + defer restore() + Register(&fakeGen{name: "go"}) + + _, err := Verify(context.Background(), VerifyOptions{ + SpecPath: filepath.Join(t.TempDir(), "does-not-exist.yaml"), + Generators: []string{"go"}, + WorkDir: t.TempDir(), + }) + require.Error(t, err) +} + +func TestVerify_EmptySpecErrors(t *testing.T) { + restore := resetForTesting() + defer restore() + _, err := Verify(context.Background(), VerifyOptions{Generators: []string{"go"}}) + require.Error(t, err) + assert.Contains(t, err.Error(), "spec path is required") +} + +func TestVerify_DefaultGeneratorMatrix(t *testing.T) { + restore := resetForTesting() + defer restore() + // Register fakes under the default matrix names so the empty-Generators path + // resolves without a real toolchain. + Register(&fakeGen{name: "go"}) + Register(&fakeGen{name: "typescript-angular"}) + + rep, err := Verify(context.Background(), VerifyOptions{ + SpecPath: dummySpec(t), + WorkDir: t.TempDir(), + }) + require.NoError(t, err) + require.Len(t, rep.Results, 2) + assert.Equal(t, []string{"go", "typescript-angular"}, DefaultVerifyGenerators) +} + +// TestVerify_RealGeneratorsAreToolchainCheckers guards that the shipped go and +// typescript-angular generators implement ToolchainChecker, so verification can +// skip them gracefully when their toolchains are absent. +func TestVerify_RealGeneratorsAreToolchainCheckers(t *testing.T) { + for _, name := range DefaultVerifyGenerators { + g := Get(name) + require.NotNil(t, g, "generator %q must be registered", name) + _, ok := g.(ToolchainChecker) + assert.True(t, ok, "generator %q must implement ToolchainChecker", name) + } +} diff --git a/internal/config/config.go b/internal/config/config.go index e46e028..f40db8f 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -104,8 +104,22 @@ type Policy struct { // ReleaseConfig represents release configuration type ReleaseConfig struct { - TagFormat string `yaml:"tag_format"` - CIOnly bool `yaml:"ci_only"` + TagFormat string `yaml:"tag_format"` + CIOnly bool `yaml:"ci_only"` + VerifyClients VerifyClientsConfig `yaml:"verify_clients,omitempty"` +} + +// VerifyClientsConfig configures the `apx client verify` release gate, which +// generates a client and compiles it so a spec that cannot produce a buildable +// client is caught before release. Absent/omitted, the gate still runs with the +// built-in default generator matrix when `apx client verify` is invoked. +type VerifyClientsConfig struct { + // Generators lists the generators to verify. Empty means the built-in + // default matrix (go + typescript-angular). + Generators []string `yaml:"generators,omitempty"` + // WarnOnly downgrades a generate/compile failure to a warning (exit 0) + // instead of failing the gate. The --warn-only flag overrides this. + WarnOnly bool `yaml:"warn_only,omitempty"` } // Tools represents tool configuration diff --git a/internal/config/schema.go b/internal/config/schema.go index 886a208..9b09c1f 100644 --- a/internal/config/schema.go +++ b/internal/config/schema.go @@ -313,6 +313,25 @@ func buildV1Schema() SchemaVersion { Description: "Restrict releasing to CI environments", Default: true, }, + "verify_clients": { + Name: "verify_clients", + Type: TypeStruct, + Description: "Generate-and-compile gate: generate a client and build it so a spec that cannot produce a buildable client is caught before release (apx client verify)", + Children: map[string]FieldDef{ + "generators": { + Name: "generators", + Type: TypeList, + Description: "Generators to verify; empty means the default matrix (go, typescript-angular)", + ItemDef: &FieldDef{Name: "generator", Type: TypeString}, + }, + "warn_only": { + Name: "warn_only", + Type: TypeBool, + Description: "Downgrade a generate/compile failure to a warning instead of failing the gate (overridden by --warn-only)", + Default: false, + }, + }, + }, }, }, "tools": { diff --git a/testdata/goclient/collision.openapi.yaml b/testdata/goclient/collision.openapi.yaml new file mode 100644 index 0000000..fdca709 --- /dev/null +++ b/testdata/goclient/collision.openapi.yaml @@ -0,0 +1,66 @@ +openapi: 3.0.3 +info: + title: apx client-verify regression fixture + version: 0.0.0 + description: > + Reproduces two patterns that pass `apx lint`/`apx breaking` but produce a Go + client that does not compile — the gap the generate-and-compile gate closes. + Both constructs are valid OpenAPI 3: + + (A) An operation exposing redundant pagination query params: `_limit`+`limit` + and `_offset`+`offset`. oapi-codegen strips the leading underscore, so + each pair normalizes to the same Go field -> `Limit`/`Offset` redeclared. + + (B) An operation with a path parameter named `url`. oapi-codegen emits a + `url` argument that shadows the `net/url` import, so the generated + client's `url.Parse(...)` fails to compile. +paths: + /labels: + get: + operationId: listLabels + summary: Pattern A — redundant pagination params collide to one Go field. + parameters: + - name: _limit + in: query + schema: + type: integer + format: int32 + - name: limit + in: query + schema: + type: integer + format: int32 + - name: _offset + in: query + schema: + type: integer + format: int32 + - name: offset + in: query + schema: + type: integer + format: int32 + responses: + "200": + description: OK + content: + application/json: + schema: + type: object + /verify/{url}: + get: + operationId: verifyUrl + summary: Pattern B — path param named `url` shadows the net/url import. + parameters: + - name: url + in: path + required: true + schema: + type: string + responses: + "200": + description: OK + content: + application/json: + schema: + type: object