From 25aa085a50b7d55a0dd5a19e4ab1a8eeb7969980 Mon Sep 17 00:00:00 2001 From: Brad Fair Date: Wed, 22 Apr 2026 12:21:26 -0500 Subject: [PATCH 1/9] feat(cli): add DeclareInt helper for inheritable int flags MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Group.Flags closures need a Lookup-guarded int wrapper — stdlib flag.IntVar panics on duplicate declarations, and the existing DeclareString/DeclareBool helpers already cover string/bool. This completes the primitive set so dialect Groups can inherit-declare integer flags (e.g. Databricks --port) without clobbering leaves. --- internal/cli/CLAUDE.md | 2 +- internal/cli/cli_test.go | 43 ++++++++++++++++++++++++++++++++++++++++ internal/cli/root.go | 10 ++++++++++ 3 files changed, 54 insertions(+), 1 deletion(-) diff --git a/internal/cli/CLAUDE.md b/internal/cli/CLAUDE.md index ef1b1c9..1990e80 100644 --- a/internal/cli/CLAUDE.md +++ b/internal/cli/CLAUDE.md @@ -6,7 +6,7 @@ Argument-dispatch core shared by every verb. Defines the `Command` interface, th - `cli.go` — `Command`, `IO`, `DefaultIO`, `Group` (nested-verb dispatcher with auto-generated help listing; optional `Flags` closure declares group-level flags that descend to every leaf via `WithAncestorFlags`), `dispatchChild` (the Group/Dispatch handoff that scans a resolved leaf's args for `--help`/`-h` and renders its `Help()` before `Run` is called — and if the leaf implements `Flagger`, appends a `Flags:` block enumerating own + ancestor flags via `renderFlagsAsText`; Groups are skipped so the flag reaches the deepest leaf, and bare positional `help` is left alone so leaves can receive it as an argument), `Flagger` (opt-in interface: leaves that implement `Flags(fs)` get the ancestor-aware `Flags:` block in `--help`), and `renderFlagsAsText` (sorted `--name usage (default: X)` enumeration). Precedence when ancestor and leaf declare the same name: **leaf wins**, because leaves call `ApplyAncestorFlags` AFTER declaring their own flags, and ancestor registrars use `DeclareString` / `DeclareBool` Lookup-guards that skip already-declared names (stdlib `flag.FlagSet.StringVar` panics on duplicate names, verified empirically). - `dispatch.go` — `Dispatch` (root entry: short-circuits help, parses globals, routes to the matching verb via `dispatchChild`) and `RootHelp`. -- `root.go` — `Global` shape, `WithGlobal`/`GlobalFrom` context helpers (both require a non-nil ctx per stdlib `context.WithValue` convention — nil panics), `ParseGlobal` (stdlib-style front-anchored parse: stops at the first positional), and `StripGlobals` (position-tolerant: walks argv once, consumes known global flags wherever they appear, passes everything else through in order so the leaf's FlagSet reports unknown-flag errors). The two share the authoritative `globalFlagRegistry` list — `TestGlobalFlagsRegistrySync` enforces that the registry matches `ParseGlobal`'s FlagSet shape. `Dispatch` uses `StripGlobals`; `cmd/ana/main.go`'s early config-resolution pre-pass uses it too so `ana org show --profile prod` honours `--profile` even when it's placed after the verb. `globalFlagsHelp` renders the canonical `Global Flags:` block that both `RootHelp` and the leaf `--help` path append so `--json`/`--endpoint`/`--token-file`/`--profile` are discoverable from every help surface. Phase 2 flag-registrar stack: `WithAncestorFlags(ctx, reg)` / `ApplyAncestorFlags(ctx, fs)` (context-carried slice of `func(*flag.FlagSet)` closures that `Group.Run` appends to and leaves replay on their own `FlagSet`), plus `DeclareString` / `DeclareBool` (Lookup-guarded wrappers ancestor closures use instead of raw `StringVar` / `BoolVar` to avoid the stdlib redeclaration panic when a leaf already declared the same name). +- `root.go` — `Global` shape, `WithGlobal`/`GlobalFrom` context helpers (both require a non-nil ctx per stdlib `context.WithValue` convention — nil panics), `ParseGlobal` (stdlib-style front-anchored parse: stops at the first positional), and `StripGlobals` (position-tolerant: walks argv once, consumes known global flags wherever they appear, passes everything else through in order so the leaf's FlagSet reports unknown-flag errors). The two share the authoritative `globalFlagRegistry` list — `TestGlobalFlagsRegistrySync` enforces that the registry matches `ParseGlobal`'s FlagSet shape. `Dispatch` uses `StripGlobals`; `cmd/ana/main.go`'s early config-resolution pre-pass uses it too so `ana org show --profile prod` honours `--profile` even when it's placed after the verb. `globalFlagsHelp` renders the canonical `Global Flags:` block that both `RootHelp` and the leaf `--help` path append so `--json`/`--endpoint`/`--token-file`/`--profile` are discoverable from every help surface. Phase 2 flag-registrar stack: `WithAncestorFlags(ctx, reg)` / `ApplyAncestorFlags(ctx, fs)` (context-carried slice of `func(*flag.FlagSet)` closures that `Group.Run` appends to and leaves replay on their own `FlagSet`), plus `DeclareString` / `DeclareBool` / `DeclareInt` (Lookup-guarded wrappers ancestor closures use instead of raw `StringVar` / `BoolVar` / `IntVar` to avoid the stdlib redeclaration panic when a leaf already declared the same name). - `flags.go` — `ParseFlags`, which tolerates positional args interleaved with flags (stdlib `FlagSet.Parse` stops at the first non-flag, silently dropping later flags); `FlagWasSet`, the `fs.Visit` wrapper partial-update verbs use to tell "user left this alone" from "user explicitly passed the zero value"; `RequireFlags`, which emits a single sorted `missing required flags: --a, --b` usage error for any name not explicitly set on fs; and three typed `flag.Value` constructors — `EnumFlag` (allow-list validation at parse time), `IntListFlag` (CSV → `[]int` with whitespace tolerance), `SinceFlag` (accepts non-negative `time.ParseDuration` or RFC3339, stored UTC via an injected clock). The stdlib `flag.Parse` re-wraps `Set` errors with `%v`, so the `ErrUsage` chain survives only through the outer `ParseFlags` wrap — tests that exercise these helpers must go through `ParseFlags`, not bare `fs.Parse`. - `token.go` — `Token` (named `string` type whose `String`/`Format` always render the `RedactToken` mask, so any accidental `%s`/`%v`/`%+v`/`%#v`/`%q` on a logger or error can't leak a bearer token) and its `Value()` escape hatch. `config.Profile.Token` and `auth.Config.Token` are declared as `cli.Token`; the only authorized raw-emit call sites are `cmd/ana/main.go`'s `tokenFn` (Authorization header) and `auth/keys.go:emitPlaintextToken` (one-shot API key print). Since the underlying kind is `string`, `encoding/json` marshals/unmarshals transparently and untyped string literals still work in tests (`cli.Token("t")` — or just `"t"` in a `Token`-typed field). - `helpers.go` — shared verb-package helpers extracted from per-verb duplicates: `NewFlagSet`, `UsageErrf`, `WriteJSON`, `Remarshal`, `RenderOutput` (generic JSON-vs-typed-render dispatch: `--json` → `WriteJSON`; else `Remarshal` into `*T` then call the render closure), `RequireStringID`, `RequireIntID`, `RenderTwoCol`, `ReadToken` (shared login/profile-add stdin reader with JWT-sized buffer boost; trims surrounding whitespace), `ReadPassword` (same JWT-sized buffer but strips ONLY the trailing line terminator — passwords can legitimately begin or end with whitespace, so surrounding bytes must not be mutated), `NewTableWriter` (canonical tabwriter config), `FirstLine`, `DashIfEmpty` (table-cell rendering primitives; fall-through chains use stdlib `cmp.Or`), `RedactToken` (masks bearer tokens for human-readable echoes — `(unset)` or `********** (last 4: xxxx)`). Phase 0 of the shared-cli-kit refactor; Phases 1–10 migrate each verb package over and delete its local copies. diff --git a/internal/cli/cli_test.go b/internal/cli/cli_test.go index b81839f..360f336 100644 --- a/internal/cli/cli_test.go +++ b/internal/cli/cli_test.go @@ -1041,6 +1041,49 @@ func TestDeclareStringFreshDeclaration(t *testing.T) { } } +func TestDeclareIntGuardsAgainstRedeclare(t *testing.T) { + t.Parallel() + fs := flag.NewFlagSet("t", flag.ContinueOnError) + var leafT, ancT int + fs.IntVar(&leafT, "port", 1, "leaf usage") + DeclareInt(fs, &ancT, "port", 2, "anc usage") + if err := fs.Parse([]string{"--port", "7"}); err != nil { + t.Fatalf("parse err=%v", err) + } + if leafT != 7 { + t.Errorf("leafT=%d want 7", leafT) + } + if ancT != 0 { + t.Errorf("ancT=%d want 0 (ancestor should not have been bound)", ancT) + } +} + +func TestDeclareIntFreshDeclaration(t *testing.T) { + t.Parallel() + fs := flag.NewFlagSet("t", flag.ContinueOnError) + var target int + DeclareInt(fs, &target, "port", 443, "usage") + if err := fs.Parse([]string{"--port", "5432"}); err != nil { + t.Fatalf("parse err=%v", err) + } + if target != 5432 { + t.Errorf("target=%d want 5432", target) + } +} + +func TestDeclareIntDefault(t *testing.T) { + t.Parallel() + fs := flag.NewFlagSet("t", flag.ContinueOnError) + var target int + DeclareInt(fs, &target, "port", 443, "usage") + if err := fs.Parse(nil); err != nil { + t.Fatalf("parse err=%v", err) + } + if target != 443 { + t.Errorf("target=%d want 443 default", target) + } +} + func TestRenderFlagsAsTextDefaultAndEmptyType(t *testing.T) { t.Parallel() fs := flag.NewFlagSet("t", flag.ContinueOnError) diff --git a/internal/cli/root.go b/internal/cli/root.go index 8461fff..11c86e7 100644 --- a/internal/cli/root.go +++ b/internal/cli/root.go @@ -71,6 +71,16 @@ func DeclareBool(fs *flag.FlagSet, target *bool, name string, def bool, usage st } } +// DeclareInt is the int counterpart to DeclareString. Same guard against +// panicking on duplicate names — used by Group.Flags closures that want to +// inherit-declare an integer flag (e.g. the Databricks Group's shared +// `--port`) without clobbering a leaf that already declared the same name. +func DeclareInt(fs *flag.FlagSet, target *int, name string, def int, usage string) { + if fs.Lookup(name) == nil { + fs.IntVar(target, name, def, usage) + } +} + // Flagger is an optional opt-in for leaf commands whose help should include // a flag enumeration that stacks ancestor-declared flags with the leaf's // own. Leaves that implement Flags(fs) get an automatic Flags: block From fa0d2acc18feb17cba278c2ee513452867ff0567 Mon Sep 17 00:00:00 2001 From: Brad Fair Date: Wed, 22 Apr 2026 12:21:43 -0500 Subject: [PATCH 2/9] =?UTF-8?q?feat(connector):=20Databricks=20create=20?= =?UTF-8?q?=E2=80=94=20access-token,=20client-credentials,=20OAuth=20SSO,?= =?UTF-8?q?=20OAuth=20individual?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Mirrors the Snowflake create structure. One dialect Group with the shared workspace flags (name, host, http-path, port, catalog, schema) inherited by every auth-mode leaf via cli.ApplyAncestorFlags; four leaves, one per auth mode, each populating a nested `databricksAuth` one-of variant: - `access-token` → `databricksAuth.pat.token` (authStrategy=service_role) - `client-credentials` → `databricksAuth.clientCredentials.{clientId, clientSecret}` (authStrategy=service_role) - `oauth-sso` → `databricksAuth.oauthU2m.{clientId, clientSecret}` (authStrategy=oauth_sso); prints the configured endpoint in the success note so self-hosted operators get the right callback URL - `oauth-individual` → same `oauthU2m` variant with authStrategy=per_member_oauth; prints a per-member-lazy note Wire label mismatch: OAuth modes populate `oauthU2m`, NOT `oauthSso` — the server rejects the latter with `databricks.databricks_auth requires pat, client_credentials, or oauth_u2m`. Captured in the types_databricks.go doc block so future contributors don't rediscover it. --- internal/connector/CLAUDE.md | 7 ++ internal/connector/create.go | 5 +- internal/connector/create_databricks.go | 101 +++++++++++++++++ .../create_databricks_access_token.go | 96 ++++++++++++++++ .../create_databricks_client_credentials.go | 101 +++++++++++++++++ .../create_databricks_oauth_individual.go | 101 +++++++++++++++++ .../connector/create_databricks_oauth_sso.go | 107 ++++++++++++++++++ internal/connector/types.go | 11 +- internal/connector/types_databricks.go | 54 +++++++++ 9 files changed, 576 insertions(+), 7 deletions(-) create mode 100644 internal/connector/create_databricks.go create mode 100644 internal/connector/create_databricks_access_token.go create mode 100644 internal/connector/create_databricks_client_credentials.go create mode 100644 internal/connector/create_databricks_oauth_individual.go create mode 100644 internal/connector/create_databricks_oauth_sso.go create mode 100644 internal/connector/types_databricks.go diff --git a/internal/connector/CLAUDE.md b/internal/connector/CLAUDE.md index d96d277..65e3925 100644 --- a/internal/connector/CLAUDE.md +++ b/internal/connector/CLAUDE.md @@ -8,6 +8,7 @@ The `ana connector` verb tree: `list`, `get`, `create`, `update`, `delete`, `tes - `types.go` — shared wire shapes for create + update. - `types_postgres.go` — Postgres wire spec. - `types_snowflake.go` — Snowflake wire spec. +- `types_databricks.go` — Databricks wire spec: `databricksSpec` + nested `databricksAuth` one-of (`pat`/`clientCredentials`/`oauthU2m`). - `list.go` / `get.go` — readonly `GetConnectors` / `GetConnector`. - `create.go` — dialect-selector Group and the shared `resolveSecret` helper. - `create_postgres.go` — Postgres dialect Group; sibling files add auth-mode leaves. @@ -16,6 +17,11 @@ The `ana connector` verb tree: `list`, `get`, `create`, `update`, `delete`, `tes - `create_snowflake_keypair.go` — `snowflake keypair` leaf; reads PEM key from file. - `create_snowflake_oauth_sso.go` — `snowflake oauth-sso` leaf. - `create_snowflake_oauth_individual.go` — `snowflake oauth-individual` leaf. +- `create_databricks.go` — Databricks dialect Group + shared `requireDatabricksCommon` validator; sibling files add auth-mode leaves. +- `create_databricks_access_token.go` — `databricks access-token` leaf (PAT → `databricksAuth.pat`). +- `create_databricks_client_credentials.go` — `databricks client-credentials` leaf (M2M → `databricksAuth.clientCredentials`). +- `create_databricks_oauth_sso.go` — `databricks oauth-sso` leaf (U2M → `databricksAuth.oauthU2m`, `authStrategy=oauth_sso`). +- `create_databricks_oauth_individual.go` — `databricks oauth-individual` leaf (same `oauthU2m` variant, `authStrategy=per_member_oauth`). - `update.go` — `UpdateConnector`; pre-fetches baseline to merge partial updates. - `delete.go`, `test.go`, `tables.go`, `examples.go` — remaining CRUD + diagnostic verbs. - `connector_test.go` — shared `fakeDeps`, `errReader`, `TestNew*`/`TestHelp*`. @@ -24,4 +30,5 @@ The `ana connector` verb tree: `list`, `get`, `create`, `update`, `delete`, `tes - `create_snowflake_keypair_test.go` — covers the keypair leaf including key-file edge cases. - `create_snowflake_oauth_sso_test.go` — covers the oauth-sso leaf. - `create_snowflake_oauth_individual_test.go` — covers the oauth-individual leaf. +- `create_databricks_access_token_test.go` / `create_databricks_client_credentials_test.go` / `create_databricks_oauth_sso_test.go` / `create_databricks_oauth_individual_test.go` — one per Databricks leaf; same structure as the Snowflake equivalents. - `list_test.go` / `get_test.go` / `update_test.go` / `delete_test.go` / `test_test.go` / `tables_test.go` / `examples_test.go` — one per non-create source file. diff --git a/internal/connector/create.go b/internal/connector/create.go index 62aebe5..099ba0a 100644 --- a/internal/connector/create.go +++ b/internal/connector/create.go @@ -19,8 +19,9 @@ func newCreateGroup(deps Deps) *cli.Group { return &cli.Group{ Summary: "Create a new connector. Pick a dialect, then an auth mode.", Children: map[string]cli.Command{ - "postgres": newPostgresCreateGroup(deps), - "snowflake": newSnowflakeCreateGroup(deps), + "postgres": newPostgresCreateGroup(deps), + "snowflake": newSnowflakeCreateGroup(deps), + "databricks": newDatabricksCreateGroup(deps), }, } } diff --git a/internal/connector/create_databricks.go b/internal/connector/create_databricks.go new file mode 100644 index 0000000..8b21686 --- /dev/null +++ b/internal/connector/create_databricks.go @@ -0,0 +1,101 @@ +package connector + +import ( + "flag" + + "github.com/highperformance-tech/ana-cli/internal/cli" +) + +// newDatabricksCreateGroup returns the Databricks create-dialect Group. Flags +// common to every Databricks auth-mode leaf are declared on the Group's +// inheritable Flags closure; each auth-mode leaf declares its own +// credential-specific flags and reads the Group's via cli.ApplyAncestorFlags. +// +// All five workspace fields (`--host`, `--http-path`, `--port`, `--catalog`, +// `--schema`) are required by the TextQL UI across every Databricks auth mode, +// so they live on the Group rather than being duplicated per-leaf. `--host` is +// the Databricks workspace hostname without scheme; `--http-path` is the SQL +// warehouse path in the `/sql/1.0/warehouses/` format that Databricks' +// SQL Warehouse "Connection details" panel prints verbatim. +// +// `--port` defaults to 443 (the Databricks SQL Warehouse HTTPS port); the CLI +// accepts any 1..65535 value so self-hosted forwarders aren't blocked. +func newDatabricksCreateGroup(deps Deps) *cli.Group { + var ( + name string + host string + httpPath string + port int + catalog string + schema string + ) + return &cli.Group{ + Summary: "Create a Databricks connector. Pick an auth mode.", + Flags: func(fs *flag.FlagSet) { + cli.DeclareString(fs, &name, "name", "", "connector name (required)") + cli.DeclareString(fs, &host, "host", "", "Databricks workspace hostname without scheme, e.g. dbc-xxxx.cloud.databricks.com (required)") + cli.DeclareString(fs, &httpPath, "http-path", "", "SQL warehouse path, e.g. /sql/1.0/warehouses/abc123 (required)") + cli.DeclareInt(fs, &port, "port", 443, "SQL warehouse port (defaults to 443)") + cli.DeclareString(fs, &catalog, "catalog", "", "Unity Catalog name (required)") + cli.DeclareString(fs, &schema, "schema", "", "default schema (required)") + }, + Children: map[string]cli.Command{ + "access-token": &databricksAccessTokenCmd{ + deps: deps, + name: &name, + host: &host, + httpPath: &httpPath, + port: &port, + catalog: &catalog, + schema: &schema, + }, + "client-credentials": &databricksClientCredentialsCmd{ + deps: deps, + name: &name, + host: &host, + httpPath: &httpPath, + port: &port, + catalog: &catalog, + schema: &schema, + }, + "oauth-sso": &databricksOAuthSSOCmd{ + deps: deps, + name: &name, + host: &host, + httpPath: &httpPath, + port: &port, + catalog: &catalog, + schema: &schema, + }, + "oauth-individual": &databricksOAuthIndividualCmd{ + deps: deps, + name: &name, + host: &host, + httpPath: &httpPath, + port: &port, + catalog: &catalog, + schema: &schema, + }, + }, + } +} + +// requireDatabricksCommon enforces non-empty values for the shared ancestor +// flags + validates --port sits in the TCP range. Returned as a helper so every +// Databricks leaf applies the same validation before building its request. +func requireDatabricksCommon(prefix, name, host, httpPath string, port int, catalog, schema string) error { + for _, p := range []struct { + name, val string + }{ + {"name", name}, {"host", host}, {"http-path", httpPath}, + {"catalog", catalog}, {"schema", schema}, + } { + if p.val == "" { + return cli.UsageErrf("%s: --%s must not be empty", prefix, p.name) + } + } + if port <= 0 || port > 65535 { + return cli.UsageErrf("%s: --port must be in 1..65535 (got %d)", prefix, port) + } + return nil +} diff --git a/internal/connector/create_databricks_access_token.go b/internal/connector/create_databricks_access_token.go new file mode 100644 index 0000000..8797e2d --- /dev/null +++ b/internal/connector/create_databricks_access_token.go @@ -0,0 +1,96 @@ +package connector + +import ( + "context" + "flag" + "fmt" + "io" + + "github.com/highperformance-tech/ana-cli/internal/cli" +) + +// databricksAccessTokenCmd is the leaf for +// `ana connector create databricks access-token`. Ancestor flag pointers +// (--name, --host, --http-path, --port, --catalog, --schema) come from +// newDatabricksCreateGroup's closure. +// +// authStrategy=service_role — same value Databricks client-credentials uses. +// The server discriminates Access Token vs Client Credentials by which +// nested `databricksAuth.*` variant is populated (`pat` vs +// `clientCredentials`), NOT by authStrategy (matches Snowflake's +// password-vs-keypair discrimination pattern). +type databricksAccessTokenCmd struct { + deps Deps + + // Ancestor-flag targets. + name *string + host *string + httpPath *string + port *int + catalog *string + schema *string + + // Leaf-specific flag targets. + token string + tokenStdin bool +} + +func (c *databricksAccessTokenCmd) Help() string { + return "access-token Personal Access Token (PAT) Databricks auth (authStrategy=service_role).\n" + + "Usage: ana connector create databricks access-token --name --host --http-path

--catalog --schema (--token |--token-stdin) [--port

]" +} + +func (c *databricksAccessTokenCmd) Flags(fs *flag.FlagSet) { + fs.StringVar(&c.token, "token", "", "Databricks personal access token (discouraged; prefer --token-stdin)") + fs.BoolVar(&c.tokenStdin, "token-stdin", false, "read access token from the first stdin line") +} + +func (c *databricksAccessTokenCmd) Run(ctx context.Context, args []string, stdio cli.IO) error { + fs := cli.NewFlagSet("connector create databricks access-token") + c.Flags(fs) + cli.ApplyAncestorFlags(ctx, fs) + if err := cli.ParseFlags(fs, args); err != nil { + return err + } + if err := cli.RequireFlags(fs, "connector create databricks access-token", + "name", "host", "http-path", "catalog", "schema"); err != nil { + return err + } + if err := requireDatabricksCommon("connector create databricks access-token", + *c.name, *c.host, *c.httpPath, *c.port, *c.catalog, *c.schema); err != nil { + return err + } + resolvedToken, err := resolveSecret("token", c.token, c.tokenStdin, stdio.Stdin) + if err != nil { + return fmt.Errorf("connector create databricks access-token: %w", err) + } + + req := createReq{Config: configEnvelope{ + ConnectorType: "DATABRICKS", + Name: *c.name, + AuthStrategy: "service_role", + Databricks: &databricksSpec{ + Host: *c.host, + HTTPPath: *c.httpPath, + Port: *c.port, + Catalog: *c.catalog, + Schema: *c.schema, + Auth: &databricksAuthSpec{ + Pat: &databricksPatAuth{Token: resolvedToken}, + }, + }, + }} + var raw map[string]any + if err := c.deps.Unary(ctx, servicePath+"/CreateConnector", req, &raw); err != nil { + return fmt.Errorf("connector create databricks access-token: %w", err) + } + var typed createResp + if err := cli.RenderOutput(stdio.Stdout, raw, cli.GlobalFrom(ctx).JSON, &typed, func(w io.Writer, t *createResp) error { + _, err := fmt.Fprintf(w, "connectorId: %d\nname: %s\nconnectorType: %s\n", + t.ConnectorID, t.Name, t.ConnectorType) + return err + }); err != nil { + return fmt.Errorf("connector create databricks access-token: %w", err) + } + return nil +} diff --git a/internal/connector/create_databricks_client_credentials.go b/internal/connector/create_databricks_client_credentials.go new file mode 100644 index 0000000..c6745e2 --- /dev/null +++ b/internal/connector/create_databricks_client_credentials.go @@ -0,0 +1,101 @@ +package connector + +import ( + "context" + "flag" + "fmt" + "io" + + "github.com/highperformance-tech/ana-cli/internal/cli" +) + +// databricksClientCredentialsCmd is the leaf for +// `ana connector create databricks client-credentials`. authStrategy=service_role +// (same as Access Token) — server distinguishes by populating +// `databricksAuth.clientCredentials` instead of `databricksAuth.pat`. +// +// OAuth 2.0 M2M using a Databricks Service Principal's OAuth +// clientId + clientSecret. The SP must have CAN_USE on the target SQL +// warehouse. `clientId` is a UUID and is echoed back by GetConnector; +// `clientSecret` is kept server-side. +type databricksClientCredentialsCmd struct { + deps Deps + + name *string + host *string + httpPath *string + port *int + catalog *string + schema *string + + clientID string + clientSecret string + secretStdin bool +} + +func (c *databricksClientCredentialsCmd) Help() string { + return "client-credentials OAuth M2M via Service Principal (authStrategy=service_role).\n" + + "Usage: ana connector create databricks client-credentials --name --host --http-path

--catalog --schema --client-id (--client-secret |--client-secret-stdin) [--port

]" +} + +func (c *databricksClientCredentialsCmd) Flags(fs *flag.FlagSet) { + fs.StringVar(&c.clientID, "client-id", "", "Databricks Service Principal OAuth client id / applicationId (required)") + fs.StringVar(&c.clientSecret, "client-secret", "", "Databricks Service Principal OAuth client secret (discouraged; prefer --client-secret-stdin)") + fs.BoolVar(&c.secretStdin, "client-secret-stdin", false, "read client secret from the first stdin line") +} + +func (c *databricksClientCredentialsCmd) Run(ctx context.Context, args []string, stdio cli.IO) error { + fs := cli.NewFlagSet("connector create databricks client-credentials") + c.Flags(fs) + cli.ApplyAncestorFlags(ctx, fs) + if err := cli.ParseFlags(fs, args); err != nil { + return err + } + if err := cli.RequireFlags(fs, "connector create databricks client-credentials", + "name", "host", "http-path", "catalog", "schema", "client-id"); err != nil { + return err + } + if err := requireDatabricksCommon("connector create databricks client-credentials", + *c.name, *c.host, *c.httpPath, *c.port, *c.catalog, *c.schema); err != nil { + return err + } + if c.clientID == "" { + return cli.UsageErrf("connector create databricks client-credentials: --client-id must not be empty") + } + resolvedSecret, err := resolveSecret("client-secret", c.clientSecret, c.secretStdin, stdio.Stdin) + if err != nil { + return fmt.Errorf("connector create databricks client-credentials: %w", err) + } + + req := createReq{Config: configEnvelope{ + ConnectorType: "DATABRICKS", + Name: *c.name, + AuthStrategy: "service_role", + Databricks: &databricksSpec{ + Host: *c.host, + HTTPPath: *c.httpPath, + Port: *c.port, + Catalog: *c.catalog, + Schema: *c.schema, + Auth: &databricksAuthSpec{ + ClientCredentials: &databricksClientCredentialsAuth{ + ClientID: c.clientID, + ClientSecret: resolvedSecret, + }, + }, + }, + }} + var raw map[string]any + if err := c.deps.Unary(ctx, servicePath+"/CreateConnector", req, &raw); err != nil { + return fmt.Errorf("connector create databricks client-credentials: %w", err) + } + var typed createResp + if err := cli.RenderOutput(stdio.Stdout, raw, cli.GlobalFrom(ctx).JSON, &typed, func(w io.Writer, t *createResp) error { + _, err := fmt.Fprintf(w, "connectorId: %d\nname: %s\nconnectorType: %s\n", + t.ConnectorID, t.Name, t.ConnectorType) + return err + }); err != nil { + return fmt.Errorf("connector create databricks client-credentials: %w", err) + } + return nil +} diff --git a/internal/connector/create_databricks_oauth_individual.go b/internal/connector/create_databricks_oauth_individual.go new file mode 100644 index 0000000..07117dd --- /dev/null +++ b/internal/connector/create_databricks_oauth_individual.go @@ -0,0 +1,101 @@ +package connector + +import ( + "context" + "flag" + "fmt" + "io" + + "github.com/highperformance-tech/ana-cli/internal/cli" +) + +// databricksOAuthIndividualCmd is the leaf for +// `ana connector create databricks oauth-individual` +// (authStrategy=per_member_oauth). +// +// Wire shape is identical to oauth-sso (both populate `databricksAuth.oauthU2m`) +// — only the envelope-level `authStrategy` differs. Unlike oauth-sso, no +// up-front handshake is needed: each member authenticates lazily at their +// first query. The CLI just creates the row. +type databricksOAuthIndividualCmd struct { + deps Deps + + name *string + host *string + httpPath *string + port *int + catalog *string + schema *string + + clientID string + clientSecret string + secretStdin bool +} + +func (c *databricksOAuthIndividualCmd) Help() string { + return "oauth-individual Per-member OAuth U2M auth (authStrategy=per_member_oauth).\n" + + "Usage: ana connector create databricks oauth-individual --name --host --http-path

--catalog --schema --client-id (--client-secret |--client-secret-stdin) [--port

]\n" + + "Note: each member authenticates lazily at first query; no up-front browser step." +} + +func (c *databricksOAuthIndividualCmd) Flags(fs *flag.FlagSet) { + fs.StringVar(&c.clientID, "client-id", "", "Databricks OAuth app client id (required)") + fs.StringVar(&c.clientSecret, "client-secret", "", "Databricks OAuth app client secret (discouraged; prefer --client-secret-stdin)") + fs.BoolVar(&c.secretStdin, "client-secret-stdin", false, "read client secret from the first stdin line") +} + +func (c *databricksOAuthIndividualCmd) Run(ctx context.Context, args []string, stdio cli.IO) error { + fs := cli.NewFlagSet("connector create databricks oauth-individual") + c.Flags(fs) + cli.ApplyAncestorFlags(ctx, fs) + if err := cli.ParseFlags(fs, args); err != nil { + return err + } + if err := cli.RequireFlags(fs, "connector create databricks oauth-individual", + "name", "host", "http-path", "catalog", "schema", "client-id"); err != nil { + return err + } + if err := requireDatabricksCommon("connector create databricks oauth-individual", + *c.name, *c.host, *c.httpPath, *c.port, *c.catalog, *c.schema); err != nil { + return err + } + if c.clientID == "" { + return cli.UsageErrf("connector create databricks oauth-individual: --client-id must not be empty") + } + resolvedSecret, err := resolveSecret("client-secret", c.clientSecret, c.secretStdin, stdio.Stdin) + if err != nil { + return fmt.Errorf("connector create databricks oauth-individual: %w", err) + } + + req := createReq{Config: configEnvelope{ + ConnectorType: "DATABRICKS", + Name: *c.name, + AuthStrategy: "per_member_oauth", + Databricks: &databricksSpec{ + Host: *c.host, + HTTPPath: *c.httpPath, + Port: *c.port, + Catalog: *c.catalog, + Schema: *c.schema, + Auth: &databricksAuthSpec{ + OAuthU2M: &databricksOAuthU2MAuth{ + ClientID: c.clientID, + ClientSecret: resolvedSecret, + }, + }, + }, + }} + var raw map[string]any + if err := c.deps.Unary(ctx, servicePath+"/CreateConnector", req, &raw); err != nil { + return fmt.Errorf("connector create databricks oauth-individual: %w", err) + } + var typed createResp + if err := cli.RenderOutput(stdio.Stdout, raw, cli.GlobalFrom(ctx).JSON, &typed, func(w io.Writer, t *createResp) error { + _, err := fmt.Fprintf(w, "connectorId: %d\nname: %s\nconnectorType: %s\nnote: members authenticate lazily at first query\n", + t.ConnectorID, t.Name, t.ConnectorType) + return err + }); err != nil { + return fmt.Errorf("connector create databricks oauth-individual: %w", err) + } + return nil +} diff --git a/internal/connector/create_databricks_oauth_sso.go b/internal/connector/create_databricks_oauth_sso.go new file mode 100644 index 0000000..db85b53 --- /dev/null +++ b/internal/connector/create_databricks_oauth_sso.go @@ -0,0 +1,107 @@ +package connector + +import ( + "context" + "flag" + "fmt" + "io" + + "github.com/highperformance-tech/ana-cli/internal/cli" +) + +// databricksOAuthSSOCmd is the leaf for +// `ana connector create databricks oauth-sso` (authStrategy=oauth_sso). +// +// Shared-credential OAuth U2M (user-to-machine) — one refresh token serves +// the whole workspace. Server accepts the row in a pending-OAuth state; the +// browser handshake at `app.textql.com/auth/databricks/callback` happens +// separately and produces the refresh token. CLI users must complete that +// in a browser — the redirect URI is hard-coded and a CLI cannot receive +// the callback. +// +// Wire/UI label mismatch: the oneof variant the server accepts is +// `oauthU2m`, NOT `oauthSso`. The SSO-vs-Individual split lives on the +// envelope's `authStrategy`, not in the nested `databricksAuth` one-of. +type databricksOAuthSSOCmd struct { + deps Deps + + name *string + host *string + httpPath *string + port *int + catalog *string + schema *string + + clientID string + clientSecret string + secretStdin bool +} + +func (c *databricksOAuthSSOCmd) Help() string { + return "oauth-sso Shared-token OAuth U2M auth (authStrategy=oauth_sso).\n" + + "Usage: ana connector create databricks oauth-sso --name --host --http-path

--catalog --schema --client-id (--client-secret |--client-secret-stdin) [--port

]\n" + + "Note: a human must complete the OAuth handshake in the TextQL web app you're pointed at after create — the CLI cannot receive the redirect. The success message prints the exact URL based on the active profile." +} + +func (c *databricksOAuthSSOCmd) Flags(fs *flag.FlagSet) { + fs.StringVar(&c.clientID, "client-id", "", "Databricks OAuth app client id (required)") + fs.StringVar(&c.clientSecret, "client-secret", "", "Databricks OAuth app client secret (discouraged; prefer --client-secret-stdin)") + fs.BoolVar(&c.secretStdin, "client-secret-stdin", false, "read client secret from the first stdin line") +} + +func (c *databricksOAuthSSOCmd) Run(ctx context.Context, args []string, stdio cli.IO) error { + fs := cli.NewFlagSet("connector create databricks oauth-sso") + c.Flags(fs) + cli.ApplyAncestorFlags(ctx, fs) + if err := cli.ParseFlags(fs, args); err != nil { + return err + } + if err := cli.RequireFlags(fs, "connector create databricks oauth-sso", + "name", "host", "http-path", "catalog", "schema", "client-id"); err != nil { + return err + } + if err := requireDatabricksCommon("connector create databricks oauth-sso", + *c.name, *c.host, *c.httpPath, *c.port, *c.catalog, *c.schema); err != nil { + return err + } + if c.clientID == "" { + return cli.UsageErrf("connector create databricks oauth-sso: --client-id must not be empty") + } + resolvedSecret, err := resolveSecret("client-secret", c.clientSecret, c.secretStdin, stdio.Stdin) + if err != nil { + return fmt.Errorf("connector create databricks oauth-sso: %w", err) + } + + req := createReq{Config: configEnvelope{ + ConnectorType: "DATABRICKS", + Name: *c.name, + AuthStrategy: "oauth_sso", + Databricks: &databricksSpec{ + Host: *c.host, + HTTPPath: *c.httpPath, + Port: *c.port, + Catalog: *c.catalog, + Schema: *c.schema, + Auth: &databricksAuthSpec{ + OAuthU2M: &databricksOAuthU2MAuth{ + ClientID: c.clientID, + ClientSecret: resolvedSecret, + }, + }, + }, + }} + var raw map[string]any + if err := c.deps.Unary(ctx, servicePath+"/CreateConnector", req, &raw); err != nil { + return fmt.Errorf("connector create databricks oauth-sso: %w", err) + } + endpoint := c.deps.resolveEndpoint() + var typed createResp + if err := cli.RenderOutput(stdio.Stdout, raw, cli.GlobalFrom(ctx).JSON, &typed, func(w io.Writer, t *createResp) error { + _, err := fmt.Fprintf(w, "connectorId: %d\nname: %s\nconnectorType: %s\nnote: complete OAuth at %s to activate\n", + t.ConnectorID, t.Name, t.ConnectorType, endpoint) + return err + }); err != nil { + return fmt.Errorf("connector create databricks oauth-sso: %w", err) + } + return nil +} diff --git a/internal/connector/types.go b/internal/connector/types.go index 07a0420..37b3d85 100644 --- a/internal/connector/types.go +++ b/internal/connector/types.go @@ -23,11 +23,12 @@ type updateReq struct { // (`config.authStrategy`, not nested under a dialect sub-object); it's empty // for Postgres, populated for Snowflake/Databricks. type configEnvelope struct { - ConnectorType string `json:"connectorType,omitempty"` - Name string `json:"name,omitempty"` - AuthStrategy string `json:"authStrategy,omitempty"` - Postgres *postgresSpec `json:"postgres,omitempty"` - Snowflake *snowflakeSpec `json:"snowflake,omitempty"` + ConnectorType string `json:"connectorType,omitempty"` + Name string `json:"name,omitempty"` + AuthStrategy string `json:"authStrategy,omitempty"` + Postgres *postgresSpec `json:"postgres,omitempty"` + Snowflake *snowflakeSpec `json:"snowflake,omitempty"` + Databricks *databricksSpec `json:"databricks,omitempty"` } // createResp is the `{connectorId, name, connectorType}` captured response. diff --git a/internal/connector/types_databricks.go b/internal/connector/types_databricks.go new file mode 100644 index 0000000..5990b9d --- /dev/null +++ b/internal/connector/types_databricks.go @@ -0,0 +1,54 @@ +package connector + +// databricksSpec matches the oneof leaf for the DATABRICKS dialect. Host/ +// httpPath/port/catalog/schema are required by the UI across every auth mode; +// the four auth modes differ only in which `databricksAuth` variant is +// populated (and in the envelope's top-level `authStrategy` for the OAuth +// split). Every field uses omitempty so each leaf emits only the fields its +// mode actually uses. Wire name asymmetry vs response side: request key is +// `databricks`, persisted response uses `databricksMetadata`. +type databricksSpec struct { + Host string `json:"host,omitempty"` + HTTPPath string `json:"httpPath,omitempty"` + Port int `json:"port,omitempty"` + Catalog string `json:"catalog,omitempty"` + Schema string `json:"schema,omitempty"` + Auth *databricksAuthSpec `json:"databricksAuth,omitempty"` +} + +// databricksAuthSpec is the nested oneof discriminator on +// `databricks.databricksAuth`. Exactly one of `Pat`, `ClientCredentials`, +// `OAuthU2M` is populated per request. Wire label mismatch note: OAuth (SSO) +// and OAuth (Individual) both populate `OAuthU2M` — `oauthU2m`, NOT +// `oauthSso`, is the only OAuth variant the server accepts. The SSO vs +// Individual split lives on the envelope's `authStrategy` (oauth_sso vs +// per_member_oauth), not on this nested oneof. +type databricksAuthSpec struct { + Pat *databricksPatAuth `json:"pat,omitempty"` + ClientCredentials *databricksClientCredentialsAuth `json:"clientCredentials,omitempty"` + OAuthU2M *databricksOAuthU2MAuth `json:"oauthU2m,omitempty"` +} + +// databricksPatAuth carries the Access Token (Personal Access Token) variant. +// Single field `token` — the server never echoes it back on GetConnector. +type databricksPatAuth struct { + Token string `json:"token,omitempty"` +} + +// databricksClientCredentialsAuth is the M2M Service-Principal variant. +// `clientId` is a non-sensitive UUID (the SP's applicationId) and IS echoed +// on GetConnector; `clientSecret` is the server-side secret and is absent +// from the persisted echo. +type databricksClientCredentialsAuth struct { + ClientID string `json:"clientId,omitempty"` + ClientSecret string `json:"clientSecret,omitempty"` +} + +// databricksOAuthU2MAuth is the U2M (user-to-machine) variant — same struct +// serves both `oauth-sso` (shared refresh token) and `oauth-individual` +// (per-member lazy OAuth); the envelope's `authStrategy` is the only thing +// that differs. `clientId` is echoed on GetConnector; `clientSecret` is not. +type databricksOAuthU2MAuth struct { + ClientID string `json:"clientId,omitempty"` + ClientSecret string `json:"clientSecret,omitempty"` +} From 36fbff2e594687fda6a3461b65a1dba1d379fc75 Mon Sep 17 00:00:00 2001 From: Brad Fair Date: Wed, 22 Apr 2026 12:21:57 -0500 Subject: [PATCH 3/9] test(connector): unit-test coverage for every Databricks create leaf One _test.go file per leaf, mirroring the Snowflake layout: happy path (wire shape assertions + omitempty negative assertions), stdin secret read, empty-stdin usage error, read-error propagation, missing secret, missing flags, empty-string validation on every required ancestor flag, JSON bypass, render-write error, bad-flag rejection, Unary error, and remarshal error. Access-token adds a port-range validation test + a custom-port happy path; OAuth SSO adds a custom-endpoint echo test that locks in the self-hosted-endpoint behavior. Also expands TestCreateGroupHelpMentionsDialects to require `databricks` so future dialect adds trip the same guard. Keeps the 100% coverage gate on `./internal/...` green. --- .../create_databricks_access_token_test.go | 270 ++++++++++++++++++ ...eate_databricks_client_credentials_test.go | 219 ++++++++++++++ ...create_databricks_oauth_individual_test.go | 217 ++++++++++++++ .../create_databricks_oauth_sso_test.go | 250 ++++++++++++++++ .../create_postgres_password_test.go | 2 +- 5 files changed, 957 insertions(+), 1 deletion(-) create mode 100644 internal/connector/create_databricks_access_token_test.go create mode 100644 internal/connector/create_databricks_client_credentials_test.go create mode 100644 internal/connector/create_databricks_oauth_individual_test.go create mode 100644 internal/connector/create_databricks_oauth_sso_test.go diff --git a/internal/connector/create_databricks_access_token_test.go b/internal/connector/create_databricks_access_token_test.go new file mode 100644 index 0000000..1379d41 --- /dev/null +++ b/internal/connector/create_databricks_access_token_test.go @@ -0,0 +1,270 @@ +package connector + +import ( + "bytes" + "context" + "errors" + "strings" + "testing" + + "github.com/highperformance-tech/ana-cli/internal/cli" + "github.com/highperformance-tech/ana-cli/internal/testcli" +) + +// databricksAccessTokenArgs returns the full dispatch args for +// `connector create databricks access-token ...` with every required flag +// set (token via --token for the happy path; the stdin variants build args +// inline). Routes through newCreateGroup so ancestor-flag plumbing declared +// on the Databricks Group (--name, --host, --http-path, --port, --catalog, +// --schema) is exercised. +func databricksAccessTokenArgs() []string { + return []string{ + "databricks", "access-token", + "--name", "db1", + "--host", "dbc-xxxx.cloud.databricks.com", + "--http-path", "/sql/1.0/warehouses/abc123", + "--catalog", "main", + "--schema", "default", + "--token", "pat-tok", + } +} + +func runDatabricksAccessToken(t *testing.T, deps Deps, args []string, stdin string) (*bytes.Buffer, error) { + t.Helper() + g := newCreateGroup(deps) + stdio, out, _ := testcli.NewIO(strings.NewReader(stdin)) + return out, g.Run(context.Background(), args, stdio) +} + +func TestCreateDatabricksAccessTokenHappy(t *testing.T) { + t.Parallel() + f := &fakeDeps{ + unaryFn: func(_ context.Context, _ string, _, resp any) error { + out := resp.(*map[string]any) + *out = map[string]any{"connectorId": 42.0, "name": "db1", "connectorType": "DATABRICKS"} + return nil + }, + } + out, err := runDatabricksAccessToken(t, f.deps(), databricksAccessTokenArgs(), "") + if err != nil { + t.Fatalf("err=%v", err) + } + s := out.String() + if !strings.Contains(s, "connectorId: 42") || !strings.Contains(s, "name: db1") { + t.Errorf("stdout=%q", s) + } + req := string(f.lastRawReq) + for _, want := range []string{ + `"connectorType":"DATABRICKS"`, `"name":"db1"`, `"authStrategy":"service_role"`, + `"databricks":`, `"host":"dbc-xxxx.cloud.databricks.com"`, + `"httpPath":"/sql/1.0/warehouses/abc123"`, `"port":443`, + `"catalog":"main"`, `"schema":"default"`, + `"databricksAuth":`, `"pat":`, `"token":"pat-tok"`, + } { + if !strings.Contains(req, want) { + t.Errorf("req missing %s in %s", want, req) + } + } + // Other oneof variants must be absent. + for _, unwanted := range []string{`"clientCredentials":`, `"oauthU2m":`, `"snowflake":`, `"postgres":`} { + if strings.Contains(req, unwanted) { + t.Errorf("req unexpectedly contains %s in %s", unwanted, req) + } + } + if f.lastPath != servicePath+"/CreateConnector" { + t.Errorf("path=%s", f.lastPath) + } +} + +func TestCreateDatabricksAccessTokenCustomPort(t *testing.T) { + t.Parallel() + f := &fakeDeps{ + unaryFn: func(_ context.Context, _ string, _, resp any) error { + out := resp.(*map[string]any) + *out = map[string]any{"connectorId": 1.0, "name": "db1", "connectorType": "DATABRICKS"} + return nil + }, + } + args := append(databricksAccessTokenArgs(), "--port", "8443") + _, err := runDatabricksAccessToken(t, f.deps(), args, "") + if err != nil { + t.Fatalf("err=%v", err) + } + if !strings.Contains(string(f.lastRawReq), `"port":8443`) { + t.Errorf("req=%s", string(f.lastRawReq)) + } +} + +func TestCreateDatabricksAccessTokenTokenStdin(t *testing.T) { + t.Parallel() + f := &fakeDeps{} + args := []string{ + "databricks", "access-token", + "--name", "db1", + "--host", "h", "--http-path", "/p", "--catalog", "c", "--schema", "s", + "--token-stdin", + } + _, err := runDatabricksAccessToken(t, f.deps(), args, "stdin-tok\n") + if err != nil { + t.Fatalf("err=%v", err) + } + if !strings.Contains(string(f.lastRawReq), `"token":"stdin-tok"`) { + t.Errorf("req=%s", string(f.lastRawReq)) + } +} + +func TestCreateDatabricksAccessTokenStdinEmpty(t *testing.T) { + t.Parallel() + args := []string{ + "databricks", "access-token", + "--name", "n", "--host", "h", "--http-path", "/p", + "--catalog", "c", "--schema", "s", + "--token-stdin", + } + _, err := runDatabricksAccessToken(t, (&fakeDeps{}).deps(), args, "") + if !errors.Is(err, cli.ErrUsage) { + t.Errorf("err=%v", err) + } +} + +func TestCreateDatabricksAccessTokenStdinReadErr(t *testing.T) { + t.Parallel() + args := []string{ + "databricks", "access-token", + "--name", "n", "--host", "h", "--http-path", "/p", + "--catalog", "c", "--schema", "s", + "--token-stdin", + } + g := newCreateGroup((&fakeDeps{}).deps()) + stdio, _, _ := testcli.NewIO(errReader{err: errors.New("read fail")}) + err := g.Run(context.Background(), args, stdio) + if err == nil || !strings.Contains(err.Error(), "read fail") { + t.Errorf("err=%v", err) + } +} + +func TestCreateDatabricksAccessTokenMissingToken(t *testing.T) { + t.Parallel() + args := []string{ + "databricks", "access-token", + "--name", "n", "--host", "h", "--http-path", "/p", + "--catalog", "c", "--schema", "s", + } + _, err := runDatabricksAccessToken(t, (&fakeDeps{}).deps(), args, "") + if !errors.Is(err, cli.ErrUsage) { + t.Errorf("err=%v", err) + } +} + +func TestCreateDatabricksAccessTokenMissingFlags(t *testing.T) { + t.Parallel() + _, err := runDatabricksAccessToken(t, (&fakeDeps{}).deps(), []string{"databricks", "access-token"}, "") + if !errors.Is(err, cli.ErrUsage) { + t.Errorf("err=%v", err) + } +} + +func TestCreateDatabricksAccessTokenEmptyString(t *testing.T) { + t.Parallel() + for _, flag := range []string{"name", "host", "http-path", "catalog", "schema"} { + t.Run(flag, func(t *testing.T) { + t.Parallel() + args := append(databricksAccessTokenArgs(), "--"+flag, "") + _, err := runDatabricksAccessToken(t, (&fakeDeps{}).deps(), args, "") + if !errors.Is(err, cli.ErrUsage) || !strings.Contains(err.Error(), "--"+flag) { + t.Errorf("err=%v", err) + } + }) + } +} + +func TestCreateDatabricksAccessTokenPortRange(t *testing.T) { + t.Parallel() + for _, port := range []string{"0", "-1", "65536", "100000"} { + t.Run(port, func(t *testing.T) { + t.Parallel() + args := append(databricksAccessTokenArgs(), "--port", port) + _, err := runDatabricksAccessToken(t, (&fakeDeps{}).deps(), args, "") + if !errors.Is(err, cli.ErrUsage) || !strings.Contains(err.Error(), "--port") { + t.Errorf("port=%s err=%v", port, err) + } + }) + } +} + +func TestCreateDatabricksAccessTokenJSONBypass(t *testing.T) { + t.Parallel() + f := &fakeDeps{ + unaryFn: func(_ context.Context, _ string, _, resp any) error { + out := resp.(*map[string]any) + *out = map[string]any{"connectorId": 1.0, "name": "n", "connectorType": "DATABRICKS"} + return nil + }, + } + ctx := cli.WithGlobal(context.Background(), cli.Global{JSON: true}) + g := newCreateGroup(f.deps()) + stdio, out, _ := testcli.NewIO(strings.NewReader("")) + if err := g.Run(ctx, databricksAccessTokenArgs(), stdio); err != nil { + t.Fatalf("err=%v", err) + } + if !strings.Contains(out.String(), "\"connectorId\"") { + t.Errorf("stdout=%q", out.String()) + } +} + +func TestCreateDatabricksAccessTokenRenderWriteErr(t *testing.T) { + t.Parallel() + f := &fakeDeps{ + unaryFn: func(_ context.Context, _ string, _, resp any) error { + out := resp.(*map[string]any) + *out = map[string]any{"connectorId": 1.0, "name": "db1", "connectorType": "DATABRICKS"} + return nil + }, + } + g := newCreateGroup(f.deps()) + err := g.Run(context.Background(), databricksAccessTokenArgs(), testcli.FailingIO()) + if err == nil || !strings.Contains(err.Error(), "boom") { + t.Errorf("err=%v want boom", err) + } +} + +func TestCreateDatabricksAccessTokenBadFlag(t *testing.T) { + t.Parallel() + args := []string{"databricks", "access-token", "--nope"} + _, err := runDatabricksAccessToken(t, (&fakeDeps{}).deps(), args, "") + if !errors.Is(err, cli.ErrUsage) { + t.Errorf("err=%v", err) + } +} + +func TestCreateDatabricksAccessTokenUnaryErr(t *testing.T) { + t.Parallel() + f := &fakeDeps{unaryFn: func(_ context.Context, _ string, _, _ any) error { return errors.New("boom") }} + _, err := runDatabricksAccessToken(t, f.deps(), databricksAccessTokenArgs(), "") + if err == nil || !strings.Contains(err.Error(), "boom") { + t.Errorf("err=%v", err) + } +} + +func TestCreateDatabricksAccessTokenRemarshalErr(t *testing.T) { + t.Parallel() + f := &fakeDeps{ + unaryFn: func(_ context.Context, _ string, _, resp any) error { + out := resp.(*map[string]any) + *out = map[string]any{"connectorId": "not-an-int"} + return nil + }, + } + _, err := runDatabricksAccessToken(t, f.deps(), databricksAccessTokenArgs(), "") + if err == nil || !strings.Contains(err.Error(), "decode response") { + t.Errorf("err=%v", err) + } +} + +func TestCreateGroupUnknownDatabricksAuthMode(t *testing.T) { + t.Parallel() + _, err := runDatabricksAccessToken(t, (&fakeDeps{}).deps(), []string{"databricks", "basic-auth"}, "") + if !errors.Is(err, cli.ErrUsage) { + t.Errorf("err=%v", err) + } +} diff --git a/internal/connector/create_databricks_client_credentials_test.go b/internal/connector/create_databricks_client_credentials_test.go new file mode 100644 index 0000000..24c889f --- /dev/null +++ b/internal/connector/create_databricks_client_credentials_test.go @@ -0,0 +1,219 @@ +package connector + +import ( + "bytes" + "context" + "errors" + "strings" + "testing" + + "github.com/highperformance-tech/ana-cli/internal/cli" + "github.com/highperformance-tech/ana-cli/internal/testcli" +) + +func databricksClientCredentialsArgs() []string { + return []string{ + "databricks", "client-credentials", + "--name", "db1", + "--host", "dbc-xxxx.cloud.databricks.com", + "--http-path", "/sql/1.0/warehouses/abc123", + "--catalog", "main", + "--schema", "default", + "--client-id", "cid", + "--client-secret", "csec", + } +} + +func runDatabricksClientCredentials(t *testing.T, deps Deps, args []string, stdin string) (*bytes.Buffer, error) { + t.Helper() + g := newCreateGroup(deps) + stdio, out, _ := testcli.NewIO(strings.NewReader(stdin)) + return out, g.Run(context.Background(), args, stdio) +} + +func TestCreateDatabricksClientCredentialsHappy(t *testing.T) { + t.Parallel() + f := &fakeDeps{ + unaryFn: func(_ context.Context, _ string, _, resp any) error { + out := resp.(*map[string]any) + *out = map[string]any{"connectorId": 55.0, "name": "db1", "connectorType": "DATABRICKS"} + return nil + }, + } + out, err := runDatabricksClientCredentials(t, f.deps(), databricksClientCredentialsArgs(), "") + if err != nil { + t.Fatalf("err=%v", err) + } + s := out.String() + if !strings.Contains(s, "connectorId: 55") || !strings.Contains(s, "connectorType: DATABRICKS") { + t.Errorf("stdout=%q", s) + } + req := string(f.lastRawReq) + for _, want := range []string{ + `"connectorType":"DATABRICKS"`, `"name":"db1"`, `"authStrategy":"service_role"`, + `"host":"dbc-xxxx.cloud.databricks.com"`, `"port":443`, + `"clientCredentials":`, `"clientId":"cid"`, `"clientSecret":"csec"`, + } { + if !strings.Contains(req, want) { + t.Errorf("req missing %s in %s", want, req) + } + } + for _, unwanted := range []string{`"pat":`, `"oauthU2m":`, `"token":`} { + if strings.Contains(req, unwanted) { + t.Errorf("req unexpectedly contains %s in %s", unwanted, req) + } + } +} + +func TestCreateDatabricksClientCredentialsSecretStdin(t *testing.T) { + t.Parallel() + f := &fakeDeps{} + args := []string{ + "databricks", "client-credentials", + "--name", "db1", + "--host", "h", "--http-path", "/p", "--catalog", "c", "--schema", "s", + "--client-id", "cid", + "--client-secret-stdin", + } + _, err := runDatabricksClientCredentials(t, f.deps(), args, "stdin-secret\n") + if err != nil { + t.Fatalf("err=%v", err) + } + if !strings.Contains(string(f.lastRawReq), `"clientSecret":"stdin-secret"`) { + t.Errorf("req=%s", string(f.lastRawReq)) + } +} + +func TestCreateDatabricksClientCredentialsSecretStdinEmpty(t *testing.T) { + t.Parallel() + args := []string{ + "databricks", "client-credentials", + "--name", "db1", "--host", "h", "--http-path", "/p", + "--catalog", "c", "--schema", "s", + "--client-id", "cid", "--client-secret-stdin", + } + _, err := runDatabricksClientCredentials(t, (&fakeDeps{}).deps(), args, "") + if !errors.Is(err, cli.ErrUsage) { + t.Errorf("err=%v", err) + } +} + +func TestCreateDatabricksClientCredentialsSecretStdinReadErr(t *testing.T) { + t.Parallel() + args := []string{ + "databricks", "client-credentials", + "--name", "db1", "--host", "h", "--http-path", "/p", + "--catalog", "c", "--schema", "s", + "--client-id", "cid", "--client-secret-stdin", + } + g := newCreateGroup((&fakeDeps{}).deps()) + stdio, _, _ := testcli.NewIO(errReader{err: errors.New("read fail")}) + err := g.Run(context.Background(), args, stdio) + if err == nil || !strings.Contains(err.Error(), "read fail") { + t.Errorf("err=%v", err) + } +} + +func TestCreateDatabricksClientCredentialsMissingSecret(t *testing.T) { + t.Parallel() + args := []string{ + "databricks", "client-credentials", + "--name", "db1", "--host", "h", "--http-path", "/p", + "--catalog", "c", "--schema", "s", + "--client-id", "cid", + } + _, err := runDatabricksClientCredentials(t, (&fakeDeps{}).deps(), args, "") + if !errors.Is(err, cli.ErrUsage) { + t.Errorf("err=%v", err) + } +} + +func TestCreateDatabricksClientCredentialsMissingFlags(t *testing.T) { + t.Parallel() + _, err := runDatabricksClientCredentials(t, (&fakeDeps{}).deps(), []string{"databricks", "client-credentials"}, "") + if !errors.Is(err, cli.ErrUsage) { + t.Errorf("err=%v", err) + } +} + +func TestCreateDatabricksClientCredentialsEmptyString(t *testing.T) { + t.Parallel() + for _, flag := range []string{"name", "host", "http-path", "catalog", "schema", "client-id"} { + t.Run(flag, func(t *testing.T) { + t.Parallel() + args := append(databricksClientCredentialsArgs(), "--"+flag, "") + _, err := runDatabricksClientCredentials(t, (&fakeDeps{}).deps(), args, "") + if !errors.Is(err, cli.ErrUsage) || !strings.Contains(err.Error(), "--"+flag) { + t.Errorf("flag=%s err=%v", flag, err) + } + }) + } +} + +func TestCreateDatabricksClientCredentialsJSONBypass(t *testing.T) { + t.Parallel() + f := &fakeDeps{ + unaryFn: func(_ context.Context, _ string, _, resp any) error { + out := resp.(*map[string]any) + *out = map[string]any{"connectorId": 1.0, "name": "n", "connectorType": "DATABRICKS"} + return nil + }, + } + ctx := cli.WithGlobal(context.Background(), cli.Global{JSON: true}) + g := newCreateGroup(f.deps()) + stdio, out, _ := testcli.NewIO(strings.NewReader("")) + if err := g.Run(ctx, databricksClientCredentialsArgs(), stdio); err != nil { + t.Fatalf("err=%v", err) + } + if !strings.Contains(out.String(), "\"connectorId\"") { + t.Errorf("stdout=%q", out.String()) + } +} + +func TestCreateDatabricksClientCredentialsRenderWriteErr(t *testing.T) { + t.Parallel() + f := &fakeDeps{ + unaryFn: func(_ context.Context, _ string, _, resp any) error { + out := resp.(*map[string]any) + *out = map[string]any{"connectorId": 1.0, "name": "db1", "connectorType": "DATABRICKS"} + return nil + }, + } + g := newCreateGroup(f.deps()) + err := g.Run(context.Background(), databricksClientCredentialsArgs(), testcli.FailingIO()) + if err == nil || !strings.Contains(err.Error(), "boom") { + t.Errorf("err=%v want boom", err) + } +} + +func TestCreateDatabricksClientCredentialsBadFlag(t *testing.T) { + t.Parallel() + _, err := runDatabricksClientCredentials(t, (&fakeDeps{}).deps(), []string{"databricks", "client-credentials", "--nope"}, "") + if !errors.Is(err, cli.ErrUsage) { + t.Errorf("err=%v", err) + } +} + +func TestCreateDatabricksClientCredentialsUnaryErr(t *testing.T) { + t.Parallel() + f := &fakeDeps{unaryFn: func(_ context.Context, _ string, _, _ any) error { return errors.New("boom") }} + _, err := runDatabricksClientCredentials(t, f.deps(), databricksClientCredentialsArgs(), "") + if err == nil || !strings.Contains(err.Error(), "boom") { + t.Errorf("err=%v", err) + } +} + +func TestCreateDatabricksClientCredentialsRemarshalErr(t *testing.T) { + t.Parallel() + f := &fakeDeps{ + unaryFn: func(_ context.Context, _ string, _, resp any) error { + out := resp.(*map[string]any) + *out = map[string]any{"connectorId": "not-an-int"} + return nil + }, + } + _, err := runDatabricksClientCredentials(t, f.deps(), databricksClientCredentialsArgs(), "") + if err == nil || !strings.Contains(err.Error(), "decode response") { + t.Errorf("err=%v", err) + } +} diff --git a/internal/connector/create_databricks_oauth_individual_test.go b/internal/connector/create_databricks_oauth_individual_test.go new file mode 100644 index 0000000..8dedb70 --- /dev/null +++ b/internal/connector/create_databricks_oauth_individual_test.go @@ -0,0 +1,217 @@ +package connector + +import ( + "bytes" + "context" + "errors" + "strings" + "testing" + + "github.com/highperformance-tech/ana-cli/internal/cli" + "github.com/highperformance-tech/ana-cli/internal/testcli" +) + +func databricksOAuthIndividualArgs() []string { + return []string{ + "databricks", "oauth-individual", + "--name", "db1", + "--host", "dbc-xxxx.cloud.databricks.com", + "--http-path", "/sql/1.0/warehouses/abc123", + "--catalog", "main", + "--schema", "default", + "--client-id", "cid", + "--client-secret", "csec", + } +} + +func runDatabricksOAuthIndividual(t *testing.T, deps Deps, args []string, stdin string) (*bytes.Buffer, error) { + t.Helper() + g := newCreateGroup(deps) + stdio, out, _ := testcli.NewIO(strings.NewReader(stdin)) + return out, g.Run(context.Background(), args, stdio) +} + +func TestCreateDatabricksOAuthIndividualHappy(t *testing.T) { + t.Parallel() + f := &fakeDeps{ + unaryFn: func(_ context.Context, _ string, _, resp any) error { + out := resp.(*map[string]any) + *out = map[string]any{"connectorId": 99.0, "name": "db1", "connectorType": "DATABRICKS"} + return nil + }, + } + out, err := runDatabricksOAuthIndividual(t, f.deps(), databricksOAuthIndividualArgs(), "") + if err != nil { + t.Fatalf("err=%v", err) + } + s := out.String() + if !strings.Contains(s, "connectorId: 99") || !strings.Contains(s, "lazily at first query") { + t.Errorf("stdout=%q", s) + } + req := string(f.lastRawReq) + for _, want := range []string{ + `"connectorType":"DATABRICKS"`, `"authStrategy":"per_member_oauth"`, + `"oauthU2m":`, `"clientId":"cid"`, `"clientSecret":"csec"`, + } { + if !strings.Contains(req, want) { + t.Errorf("req missing %s in %s", want, req) + } + } + for _, unwanted := range []string{`"pat":`, `"clientCredentials":`, `"oauthSso":`, `"token":`} { + if strings.Contains(req, unwanted) { + t.Errorf("req unexpectedly contains %s in %s", unwanted, req) + } + } +} + +func TestCreateDatabricksOAuthIndividualSecretStdin(t *testing.T) { + t.Parallel() + f := &fakeDeps{} + args := []string{ + "databricks", "oauth-individual", + "--name", "db1", "--host", "h", "--http-path", "/p", + "--catalog", "c", "--schema", "s", + "--client-id", "cid", "--client-secret-stdin", + } + _, err := runDatabricksOAuthIndividual(t, f.deps(), args, "stdin-secret\n") + if err != nil { + t.Fatalf("err=%v", err) + } + if !strings.Contains(string(f.lastRawReq), `"clientSecret":"stdin-secret"`) { + t.Errorf("req=%s", string(f.lastRawReq)) + } +} + +func TestCreateDatabricksOAuthIndividualSecretStdinEmpty(t *testing.T) { + t.Parallel() + args := []string{ + "databricks", "oauth-individual", + "--name", "db1", "--host", "h", "--http-path", "/p", + "--catalog", "c", "--schema", "s", + "--client-id", "cid", "--client-secret-stdin", + } + _, err := runDatabricksOAuthIndividual(t, (&fakeDeps{}).deps(), args, "") + if !errors.Is(err, cli.ErrUsage) { + t.Errorf("err=%v", err) + } +} + +func TestCreateDatabricksOAuthIndividualSecretStdinReadErr(t *testing.T) { + t.Parallel() + args := []string{ + "databricks", "oauth-individual", + "--name", "db1", "--host", "h", "--http-path", "/p", + "--catalog", "c", "--schema", "s", + "--client-id", "cid", "--client-secret-stdin", + } + g := newCreateGroup((&fakeDeps{}).deps()) + stdio, _, _ := testcli.NewIO(errReader{err: errors.New("read fail")}) + err := g.Run(context.Background(), args, stdio) + if err == nil || !strings.Contains(err.Error(), "read fail") { + t.Errorf("err=%v", err) + } +} + +func TestCreateDatabricksOAuthIndividualMissingSecret(t *testing.T) { + t.Parallel() + args := []string{ + "databricks", "oauth-individual", + "--name", "db1", "--host", "h", "--http-path", "/p", + "--catalog", "c", "--schema", "s", + "--client-id", "cid", + } + _, err := runDatabricksOAuthIndividual(t, (&fakeDeps{}).deps(), args, "") + if !errors.Is(err, cli.ErrUsage) { + t.Errorf("err=%v", err) + } +} + +func TestCreateDatabricksOAuthIndividualMissingFlags(t *testing.T) { + t.Parallel() + _, err := runDatabricksOAuthIndividual(t, (&fakeDeps{}).deps(), []string{"databricks", "oauth-individual"}, "") + if !errors.Is(err, cli.ErrUsage) { + t.Errorf("err=%v", err) + } +} + +func TestCreateDatabricksOAuthIndividualEmptyString(t *testing.T) { + t.Parallel() + for _, flag := range []string{"name", "host", "http-path", "catalog", "schema", "client-id"} { + t.Run(flag, func(t *testing.T) { + t.Parallel() + args := append(databricksOAuthIndividualArgs(), "--"+flag, "") + _, err := runDatabricksOAuthIndividual(t, (&fakeDeps{}).deps(), args, "") + if !errors.Is(err, cli.ErrUsage) || !strings.Contains(err.Error(), "--"+flag) { + t.Errorf("flag=%s err=%v", flag, err) + } + }) + } +} + +func TestCreateDatabricksOAuthIndividualJSONBypass(t *testing.T) { + t.Parallel() + f := &fakeDeps{ + unaryFn: func(_ context.Context, _ string, _, resp any) error { + out := resp.(*map[string]any) + *out = map[string]any{"connectorId": 1.0, "name": "n", "connectorType": "DATABRICKS"} + return nil + }, + } + ctx := cli.WithGlobal(context.Background(), cli.Global{JSON: true}) + g := newCreateGroup(f.deps()) + stdio, out, _ := testcli.NewIO(strings.NewReader("")) + if err := g.Run(ctx, databricksOAuthIndividualArgs(), stdio); err != nil { + t.Fatalf("err=%v", err) + } + if !strings.Contains(out.String(), "\"connectorId\"") { + t.Errorf("stdout=%q", out.String()) + } +} + +func TestCreateDatabricksOAuthIndividualRenderWriteErr(t *testing.T) { + t.Parallel() + f := &fakeDeps{ + unaryFn: func(_ context.Context, _ string, _, resp any) error { + out := resp.(*map[string]any) + *out = map[string]any{"connectorId": 1.0, "name": "db1", "connectorType": "DATABRICKS"} + return nil + }, + } + g := newCreateGroup(f.deps()) + err := g.Run(context.Background(), databricksOAuthIndividualArgs(), testcli.FailingIO()) + if err == nil || !strings.Contains(err.Error(), "boom") { + t.Errorf("err=%v want boom", err) + } +} + +func TestCreateDatabricksOAuthIndividualBadFlag(t *testing.T) { + t.Parallel() + _, err := runDatabricksOAuthIndividual(t, (&fakeDeps{}).deps(), []string{"databricks", "oauth-individual", "--nope"}, "") + if !errors.Is(err, cli.ErrUsage) { + t.Errorf("err=%v", err) + } +} + +func TestCreateDatabricksOAuthIndividualUnaryErr(t *testing.T) { + t.Parallel() + f := &fakeDeps{unaryFn: func(_ context.Context, _ string, _, _ any) error { return errors.New("boom") }} + _, err := runDatabricksOAuthIndividual(t, f.deps(), databricksOAuthIndividualArgs(), "") + if err == nil || !strings.Contains(err.Error(), "boom") { + t.Errorf("err=%v", err) + } +} + +func TestCreateDatabricksOAuthIndividualRemarshalErr(t *testing.T) { + t.Parallel() + f := &fakeDeps{ + unaryFn: func(_ context.Context, _ string, _, resp any) error { + out := resp.(*map[string]any) + *out = map[string]any{"connectorId": "not-an-int"} + return nil + }, + } + _, err := runDatabricksOAuthIndividual(t, f.deps(), databricksOAuthIndividualArgs(), "") + if err == nil || !strings.Contains(err.Error(), "decode response") { + t.Errorf("err=%v", err) + } +} diff --git a/internal/connector/create_databricks_oauth_sso_test.go b/internal/connector/create_databricks_oauth_sso_test.go new file mode 100644 index 0000000..9872fb4 --- /dev/null +++ b/internal/connector/create_databricks_oauth_sso_test.go @@ -0,0 +1,250 @@ +package connector + +import ( + "bytes" + "context" + "errors" + "strings" + "testing" + + "github.com/highperformance-tech/ana-cli/internal/cli" + "github.com/highperformance-tech/ana-cli/internal/testcli" +) + +func databricksOAuthSSOArgs() []string { + return []string{ + "databricks", "oauth-sso", + "--name", "db1", + "--host", "dbc-xxxx.cloud.databricks.com", + "--http-path", "/sql/1.0/warehouses/abc123", + "--catalog", "main", + "--schema", "default", + "--client-id", "cid", + "--client-secret", "csec", + } +} + +func runDatabricksOAuthSSO(t *testing.T, deps Deps, args []string, stdin string) (*bytes.Buffer, error) { + t.Helper() + g := newCreateGroup(deps) + stdio, out, _ := testcli.NewIO(strings.NewReader(stdin)) + return out, g.Run(context.Background(), args, stdio) +} + +func TestCreateDatabricksOAuthSSOHappy(t *testing.T) { + t.Parallel() + f := &fakeDeps{ + unaryFn: func(_ context.Context, _ string, _, resp any) error { + out := resp.(*map[string]any) + *out = map[string]any{"connectorId": 66.0, "name": "db1", "connectorType": "DATABRICKS"} + return nil + }, + } + out, err := runDatabricksOAuthSSO(t, f.deps(), databricksOAuthSSOArgs(), "") + if err != nil { + t.Fatalf("err=%v", err) + } + s := out.String() + if !strings.Contains(s, "connectorId: 66") || !strings.Contains(s, "complete OAuth at https://app.textql.com") { + t.Errorf("stdout=%q", s) + } + req := string(f.lastRawReq) + for _, want := range []string{ + `"connectorType":"DATABRICKS"`, `"authStrategy":"oauth_sso"`, + `"oauthU2m":`, `"clientId":"cid"`, `"clientSecret":"csec"`, + } { + if !strings.Contains(req, want) { + t.Errorf("req missing %s in %s", want, req) + } + } + // Wire label check: must NOT be `oauthSso` (server rejects that). + if strings.Contains(req, `"oauthSso":`) { + t.Errorf("req contains forbidden wire label oauthSso: %s", req) + } + // Other databricksAuth variants must be absent. + for _, unwanted := range []string{`"pat":`, `"clientCredentials":`, `"token":`} { + if strings.Contains(req, unwanted) { + t.Errorf("req unexpectedly contains %s in %s", unwanted, req) + } + } +} + +func TestCreateDatabricksOAuthSSOCustomEndpoint(t *testing.T) { + t.Parallel() + // Self-hosted / non-prod operators resolve an endpoint that is not + // app.textql.com; the success note must echo that URL so users complete + // the OAuth handshake in the right web app. + f := &fakeDeps{ + unaryFn: func(_ context.Context, _ string, _, resp any) error { + out := resp.(*map[string]any) + *out = map[string]any{"connectorId": 77.0, "name": "db1", "connectorType": "DATABRICKS"} + return nil + }, + } + deps := f.deps() + deps.Endpoint = "https://staging.example.com" + g := newCreateGroup(deps) + stdio, out, _ := testcli.NewIO(strings.NewReader("")) + if err := g.Run(context.Background(), databricksOAuthSSOArgs(), stdio); err != nil { + t.Fatalf("err=%v", err) + } + s := out.String() + if !strings.Contains(s, "complete OAuth at https://staging.example.com") { + t.Errorf("stdout=%q missing custom endpoint URL", s) + } + if strings.Contains(s, "https://app.textql.com") { + t.Errorf("stdout=%q leaked default endpoint", s) + } +} + +func TestCreateDatabricksOAuthSSOSecretStdin(t *testing.T) { + t.Parallel() + f := &fakeDeps{} + args := []string{ + "databricks", "oauth-sso", + "--name", "db1", "--host", "h", "--http-path", "/p", + "--catalog", "c", "--schema", "s", + "--client-id", "cid", "--client-secret-stdin", + } + _, err := runDatabricksOAuthSSO(t, f.deps(), args, "stdin-secret\n") + if err != nil { + t.Fatalf("err=%v", err) + } + if !strings.Contains(string(f.lastRawReq), `"clientSecret":"stdin-secret"`) { + t.Errorf("req=%s", string(f.lastRawReq)) + } +} + +func TestCreateDatabricksOAuthSSOSecretStdinEmpty(t *testing.T) { + t.Parallel() + args := []string{ + "databricks", "oauth-sso", + "--name", "db1", "--host", "h", "--http-path", "/p", + "--catalog", "c", "--schema", "s", + "--client-id", "cid", "--client-secret-stdin", + } + _, err := runDatabricksOAuthSSO(t, (&fakeDeps{}).deps(), args, "") + if !errors.Is(err, cli.ErrUsage) { + t.Errorf("err=%v", err) + } +} + +func TestCreateDatabricksOAuthSSOSecretStdinReadErr(t *testing.T) { + t.Parallel() + args := []string{ + "databricks", "oauth-sso", + "--name", "db1", "--host", "h", "--http-path", "/p", + "--catalog", "c", "--schema", "s", + "--client-id", "cid", "--client-secret-stdin", + } + g := newCreateGroup((&fakeDeps{}).deps()) + stdio, _, _ := testcli.NewIO(errReader{err: errors.New("read fail")}) + err := g.Run(context.Background(), args, stdio) + if err == nil || !strings.Contains(err.Error(), "read fail") { + t.Errorf("err=%v", err) + } +} + +func TestCreateDatabricksOAuthSSOMissingSecret(t *testing.T) { + t.Parallel() + args := []string{ + "databricks", "oauth-sso", + "--name", "db1", "--host", "h", "--http-path", "/p", + "--catalog", "c", "--schema", "s", + "--client-id", "cid", + } + _, err := runDatabricksOAuthSSO(t, (&fakeDeps{}).deps(), args, "") + if !errors.Is(err, cli.ErrUsage) { + t.Errorf("err=%v", err) + } +} + +func TestCreateDatabricksOAuthSSOMissingFlags(t *testing.T) { + t.Parallel() + _, err := runDatabricksOAuthSSO(t, (&fakeDeps{}).deps(), []string{"databricks", "oauth-sso"}, "") + if !errors.Is(err, cli.ErrUsage) { + t.Errorf("err=%v", err) + } +} + +func TestCreateDatabricksOAuthSSOEmptyString(t *testing.T) { + t.Parallel() + for _, flag := range []string{"name", "host", "http-path", "catalog", "schema", "client-id"} { + t.Run(flag, func(t *testing.T) { + t.Parallel() + args := append(databricksOAuthSSOArgs(), "--"+flag, "") + _, err := runDatabricksOAuthSSO(t, (&fakeDeps{}).deps(), args, "") + if !errors.Is(err, cli.ErrUsage) || !strings.Contains(err.Error(), "--"+flag) { + t.Errorf("flag=%s err=%v", flag, err) + } + }) + } +} + +func TestCreateDatabricksOAuthSSOJSONBypass(t *testing.T) { + t.Parallel() + f := &fakeDeps{ + unaryFn: func(_ context.Context, _ string, _, resp any) error { + out := resp.(*map[string]any) + *out = map[string]any{"connectorId": 1.0, "name": "n", "connectorType": "DATABRICKS"} + return nil + }, + } + ctx := cli.WithGlobal(context.Background(), cli.Global{JSON: true}) + g := newCreateGroup(f.deps()) + stdio, out, _ := testcli.NewIO(strings.NewReader("")) + if err := g.Run(ctx, databricksOAuthSSOArgs(), stdio); err != nil { + t.Fatalf("err=%v", err) + } + if !strings.Contains(out.String(), "\"connectorId\"") { + t.Errorf("stdout=%q", out.String()) + } +} + +func TestCreateDatabricksOAuthSSORenderWriteErr(t *testing.T) { + t.Parallel() + f := &fakeDeps{ + unaryFn: func(_ context.Context, _ string, _, resp any) error { + out := resp.(*map[string]any) + *out = map[string]any{"connectorId": 1.0, "name": "db1", "connectorType": "DATABRICKS"} + return nil + }, + } + g := newCreateGroup(f.deps()) + err := g.Run(context.Background(), databricksOAuthSSOArgs(), testcli.FailingIO()) + if err == nil || !strings.Contains(err.Error(), "boom") { + t.Errorf("err=%v want boom", err) + } +} + +func TestCreateDatabricksOAuthSSOBadFlag(t *testing.T) { + t.Parallel() + _, err := runDatabricksOAuthSSO(t, (&fakeDeps{}).deps(), []string{"databricks", "oauth-sso", "--nope"}, "") + if !errors.Is(err, cli.ErrUsage) { + t.Errorf("err=%v", err) + } +} + +func TestCreateDatabricksOAuthSSOUnaryErr(t *testing.T) { + t.Parallel() + f := &fakeDeps{unaryFn: func(_ context.Context, _ string, _, _ any) error { return errors.New("boom") }} + _, err := runDatabricksOAuthSSO(t, f.deps(), databricksOAuthSSOArgs(), "") + if err == nil || !strings.Contains(err.Error(), "boom") { + t.Errorf("err=%v", err) + } +} + +func TestCreateDatabricksOAuthSSORemarshalErr(t *testing.T) { + t.Parallel() + f := &fakeDeps{ + unaryFn: func(_ context.Context, _ string, _, resp any) error { + out := resp.(*map[string]any) + *out = map[string]any{"connectorId": "not-an-int"} + return nil + }, + } + _, err := runDatabricksOAuthSSO(t, f.deps(), databricksOAuthSSOArgs(), "") + if err == nil || !strings.Contains(err.Error(), "decode response") { + t.Errorf("err=%v", err) + } +} diff --git a/internal/connector/create_postgres_password_test.go b/internal/connector/create_postgres_password_test.go index 380aca2..18e682e 100644 --- a/internal/connector/create_postgres_password_test.go +++ b/internal/connector/create_postgres_password_test.go @@ -323,7 +323,7 @@ func TestCreateGroupHelpMentionsDialects(t *testing.T) { t.Parallel() g := newCreateGroup(Deps{}) h := g.Help() - for _, d := range []string{"postgres", "snowflake"} { + for _, d := range []string{"postgres", "snowflake", "databricks"} { if !strings.Contains(h, d) { t.Errorf("create Help missing dialect %q: %q", d, h) } From 43e1f6476bdddaab3c7fa20baaf77db69f7a8806 Mon Sep 17 00:00:00 2001 From: Brad Fair Date: Wed, 22 Apr 2026 12:22:12 -0500 Subject: [PATCH 4/9] feat(e2e): Databricks connector live smoke coverage One test per auth mode, each env-gated: - access-token: ANA_E2E_DBX_TOKEN + workspace env - client-credentials: ANA_E2E_DBX_CLIENT_{ID,SECRET} + workspace env - oauth-sso: ANA_E2E_DBX_OAUTH_CLIENT_{ID,SECRET} + workspace env (asserts the success note references ANA_E2E_ENDPOINT) - oauth-individual: same OAuth env (asserts the per-member-lazy note) Reuses the Snowflake suite's id-extraction + LIFO cleanup primitives (extractConnectorID + RegisterConnectorCleanupByName) and gates every test on a shared ANA_E2E_DBX_{HOST,HTTP_PATH,CATALOG,SCHEMA} env tuple. README documents every env var; cli-readiness bumps the Databricks row from Probed to Implemented across all four auth modes. --- docs/cli-readiness.md | 6 +- e2e/CLAUDE.md | 1 + e2e/README.md | 37 ++++++ e2e/connector_databricks_test.go | 195 +++++++++++++++++++++++++++++++ 4 files changed, 236 insertions(+), 3 deletions(-) create mode 100644 e2e/connector_databricks_test.go diff --git a/docs/cli-readiness.md b/docs/cli-readiness.md index 29d94a2..455edd7 100644 --- a/docs/cli-readiness.md +++ b/docs/cli-readiness.md @@ -19,7 +19,7 @@ Confidence key: ✅ full CRUD verified · 🟡 partial / readonly verified · | --- | --- | --- | | Auth (API key) | ✅ | None for personal keys. Service-account keys: untested end-to-end — we created an SA and then deleted it before creating a key scoped to it. | | Chats | ✅ | `CreateChat.paradigm` — only `universal` observed; unknown if other paradigms exist (SQL-only? notebook?). Tool-selection flags (`sqlEnabled`, `pythonEnabled`, `webSearchEnabled`) defaults undocumented — must inspect `CreateChat` sample. `UpdateChat` only verified to touch `summary`; other mutable fields unknown. | -| Connectors | 🟡 | Three dialects fully probed: Postgres (password), Snowflake (password / key-pair / oauth-sso / oauth-individual — all four shipped), Databricks (access-token / client-credentials / oauth-sso / oauth-individual — probed, not yet shipped). See connector grid below. Remaining dialects (BigQuery, Redshift, MySQL, SQLServer, Supabase, MotherDuck, Tableau, PowerBI) still unverified. OAuth leaves POST the full create payload in a pending-OAuth state; user completes the browser handshake at `app.textql.com` to activate. | +| Connectors | 🟡 | Three dialects fully shipped: Postgres (password), Snowflake (password / key-pair / oauth-sso / oauth-individual — all four shipped), Databricks (access-token / client-credentials / oauth-sso / oauth-individual — all four shipped). See connector grid below. Remaining dialects (BigQuery, Redshift, MySQL, SQLServer, Supabase, MotherDuck, Tableau, PowerBI) still unverified. OAuth leaves POST the full create payload in a pending-OAuth state; user completes the browser handshake at `app.textql.com` to activate. | | Service accounts | 🟡 | Created + deleted. NOT verified: creating an API key *on* a service account (the kebab menu has "Create API Key" — would need to confirm whether that uses `CreateApiKey` with a `memberId` override or a different RPC). | | Dashboards | 🟡 | List/get/spawn/health covered. Create/update/delete NOT probed. | | Playbooks | 🟡 | Get/list/reports/lineage covered. Create/update/delete/run-now NOT probed. | @@ -65,7 +65,7 @@ Grading: ✅ Live-tested (probe + at least one real connector created) · 🟢 I | --- | --- | --- | --- | --- | --- | --- | | Postgres | 🟢 | — | — | — | — | — | | Snowflake | 🟢 | 🟢 | — | — | 🟢* | 🟢* | -| Databricks | — | — | 🟦 | 🟦 | 🟦* | 🟦* | +| Databricks | — | — | 🟢 | 🟢 | 🟢* | 🟢* | | BigQuery | ⚪ | ⚪ | ⚪ | ⚪ | ⚪ | ⚪ | | Redshift | ⚪ | — | — | — | — | — | | MySQL | ⚪ | — | — | — | — | — | @@ -75,7 +75,7 @@ Grading: ✅ Live-tested (probe + at least one real connector created) · 🟢 I | Tableau | ⚪ | — | — | — | — | — | | PowerBI | ⚪ | — | — | — | — | — | -*OAuth leaves POST the full `CreateConnector` payload (including `clientId` / `clientSecret`) through the normal wire shape; the server accepts the row in a pending-OAuth state. The user then completes the browser handshake at `app.textql.com/auth//callback` to activate the connector — the CLI prints a note to that effect on success. Databricks OAuth leaves (not yet shipped) will follow the same pattern once probed. +*OAuth leaves POST the full `CreateConnector` payload (including `clientId` / `clientSecret`) through the normal wire shape; the server accepts the row in a pending-OAuth state. The user then completes the browser handshake at `app.textql.com/auth//callback` to activate the connector — the CLI prints a note to that effect on success. Cells that show `—` are auth modes the dialect doesn't expose in the webapp's UI. diff --git a/e2e/CLAUDE.md b/e2e/CLAUDE.md index e18a5a0..2f38ea9 100644 --- a/e2e/CLAUDE.md +++ b/e2e/CLAUDE.md @@ -14,6 +14,7 @@ Live-smoke tests that drive real `app.textql.com` RPCs through the same verb pac | `chat_test.go` | Chat CRUD + streaming `send`, `--json` envelopes on list/show, CLI-path `chat new`, `history`/`bookmark`/`unbookmark`, `show ` error-path. | | `connector_test.go` | Connector CRUD + `--json` envelopes, CLI postgres create matrix (password-stdin × ssl on/off), `update --password-stdin`, `tables`/`examples`/`test` leaves, `get ` error-path. | | `connector_snowflake_test.go` | Snowflake create leaves (password/keypair/oauth-sso/oauth-individual), per-mode env-gated. | +| `connector_databricks_test.go` | Databricks create leaves (access-token/client-credentials/oauth-sso/oauth-individual), per-mode env-gated on `ANA_E2E_DBX_*`. | | `dashboard_test.go` | Dashboard list/get/folders read leaves (default + `--json`); `health`/`spawn` env-gated on `ANA_E2E_DASHBOARD_ID`. | | `playbook_test.go` | Playbook list/get/reports/lineage read leaves (default + `--json`); id discovered via `list --json`. | | `ontology_test.go` | Ontology list/get read leaves (default + `--json`); id is integer on the wire. | diff --git a/e2e/README.md b/e2e/README.md index e04329c..8765dea 100644 --- a/e2e/README.md +++ b/e2e/README.md @@ -92,6 +92,43 @@ differ in wire `authStrategy`: | `ANA_E2E_SF_OAUTH_CLIENT_ID` | Snowflake OAuth client id | | `ANA_E2E_SF_OAUTH_CLIENT_SECRET` | Client secret; piped via `--oauth-client-secret-stdin` | +### Databricks connector env + +Databricks tests (`e2e/connector_databricks_test.go`) skip per-test when +their required vars are absent — same treatment as Snowflake. Four vars are +shared across every mode; set all four or every Databricks test skips: + +| Variable | Meaning | +|-------------------------|----------------------------------------------------------------| +| `ANA_E2E_DBX_HOST` | Workspace hostname without scheme (e.g. `dbc-xxxx.cloud.databricks.com`) | +| `ANA_E2E_DBX_HTTP_PATH` | SQL warehouse path (`/sql/1.0/warehouses/`) | +| `ANA_E2E_DBX_CATALOG` | Unity Catalog name | +| `ANA_E2E_DBX_SCHEMA` | Default schema | +| `ANA_E2E_DBX_PORT` | Optional port override (defaults to 443 when unset) | + +Access Token mode (`TestConnectorCreateDatabricksAccessToken`): + +| Variable | Meaning | +|-----------------------|----------------------------------------------------------------| +| `ANA_E2E_DBX_TOKEN` | Personal Access Token; piped via `--token-stdin` | + +Client Credentials mode (`TestConnectorCreateDatabricksClientCredentials`): + +| Variable | Meaning | +|-----------------------------|----------------------------------------------------------------| +| `ANA_E2E_DBX_CLIENT_ID` | Service Principal OAuth applicationId (UUID) | +| `ANA_E2E_DBX_CLIENT_SECRET` | Service Principal OAuth secret; piped via `--client-secret-stdin` | + +OAuth SSO + OAuth individual (`TestConnectorCreateDatabricksOAuthSSO`, +`TestConnectorCreateDatabricksOAuthIndividual`) share the same vars — the +Databricks OAuth app credentials, distinct from the Service Principal +credentials used by Client Credentials mode: + +| Variable | Meaning | +|-----------------------------------|---------------------------------------------------------| +| `ANA_E2E_DBX_OAUTH_CLIENT_ID` | Databricks OAuth app client id | +| `ANA_E2E_DBX_OAUTH_CLIENT_SECRET` | OAuth app secret; piped via `--client-secret-stdin` | + Invocations: ```sh diff --git a/e2e/connector_databricks_test.go b/e2e/connector_databricks_test.go new file mode 100644 index 0000000..79f6c7c --- /dev/null +++ b/e2e/connector_databricks_test.go @@ -0,0 +1,195 @@ +package e2e + +import ( + "fmt" + "os" + "strings" + "testing" + + "github.com/highperformance-tech/ana-cli/e2e/harness" +) + +// dbxCommonEnv holds the Databricks workspace fields every auth mode shares. +// `port` is optional here because the CLI's --port already defaults to 443; +// only override when the env sets a non-default value. +type dbxCommonEnv struct { + host string + httpPath string + catalog string + schema string + port string +} + +// databricksCommonEnvOrSkip reads the mode-agnostic ANA_E2E_DBX_* env vars +// and skips the calling test if any required field (HOST, HTTP_PATH, CATALOG, +// SCHEMA) is empty. Mirrors snowflakeCommonEnvOrSkip — server does up-front +// validation so submitting a made-up spec would drown the suite in noise. +func databricksCommonEnvOrSkip(t *testing.T) dbxCommonEnv { + t.Helper() + env := dbxCommonEnv{ + host: os.Getenv("ANA_E2E_DBX_HOST"), + httpPath: os.Getenv("ANA_E2E_DBX_HTTP_PATH"), + catalog: os.Getenv("ANA_E2E_DBX_CATALOG"), + schema: os.Getenv("ANA_E2E_DBX_SCHEMA"), + port: os.Getenv("ANA_E2E_DBX_PORT"), + } + if env.host == "" || env.httpPath == "" || env.catalog == "" || env.schema == "" { + t.Skip("e2e: ANA_E2E_DBX_HOST, ANA_E2E_DBX_HTTP_PATH, ANA_E2E_DBX_CATALOG, and ANA_E2E_DBX_SCHEMA must be set for Databricks tests") + } + return env +} + +// databricksCommonArgs returns the --name/--host/--http-path/--catalog/--schema +// (+ optional --port override) flags shared by every Databricks auth-mode leaf. +func databricksCommonArgs(h *harness.H, suffix string, env dbxCommonEnv) []string { + args := []string{ + "--name", h.ResourceName(suffix), + "--host", env.host, + "--http-path", env.httpPath, + "--catalog", env.catalog, + "--schema", env.schema, + } + if env.port != "" { + args = append(args, "--port", env.port) + } + return args +} + +// TestConnectorCreateDatabricksAccessToken smokes +// `connector create databricks access-token --token-stdin`. Requires +// ANA_E2E_DBX_TOKEN in addition to the common workspace env. +func TestConnectorCreateDatabricksAccessToken(t *testing.T) { + common := databricksCommonEnvOrSkip(t) + token := os.Getenv("ANA_E2E_DBX_TOKEN") + if token == "" { + t.Skip("e2e: ANA_E2E_DBX_TOKEN required for Databricks access-token mode") + } + + h := harness.Begin(t) + h.RegisterConnectorCleanupByName(h.ResourceName("dbx-access-token")) + args := append([]string{"connector", "create", "databricks", "access-token"}, + databricksCommonArgs(h, "dbx-access-token", common)...) + args = append(args, "--token-stdin") + + stdout, stderr, err := h.RunStdin(token+"\n", args...) + if err != nil { + t.Fatalf("connector create databricks access-token: %v\nstderr: %s", err, stderr) + } + if h.DryRun() { + return + } + id := extractConnectorID(t, stdout) + h.RegisterConnectorCleanup(id) + if !strings.Contains(stdout, "connectorType: DATABRICKS") { + t.Errorf("stdout missing connectorType: DATABRICKS:\n%s", stdout) + } + if _, estderr, gerr := h.Run("connector", "get", fmt.Sprint(id)); gerr != nil { + t.Fatalf("connector get %d: %v\nstderr: %s", id, gerr, estderr) + } +} + +// TestConnectorCreateDatabricksClientCredentials smokes the M2M leaf. +// Requires ANA_E2E_DBX_CLIENT_ID + ANA_E2E_DBX_CLIENT_SECRET (Service +// Principal applicationId + OAuth secret) alongside the workspace env. +func TestConnectorCreateDatabricksClientCredentials(t *testing.T) { + common := databricksCommonEnvOrSkip(t) + clientID := os.Getenv("ANA_E2E_DBX_CLIENT_ID") + clientSecret := os.Getenv("ANA_E2E_DBX_CLIENT_SECRET") + if clientID == "" || clientSecret == "" { + t.Skip("e2e: ANA_E2E_DBX_CLIENT_ID and ANA_E2E_DBX_CLIENT_SECRET required for Databricks client-credentials mode") + } + + h := harness.Begin(t) + h.RegisterConnectorCleanupByName(h.ResourceName("dbx-client-credentials")) + args := append([]string{"connector", "create", "databricks", "client-credentials"}, + databricksCommonArgs(h, "dbx-client-credentials", common)...) + args = append(args, "--client-id", clientID, "--client-secret-stdin") + + stdout, stderr, err := h.RunStdin(clientSecret+"\n", args...) + if err != nil { + t.Fatalf("connector create databricks client-credentials: %v\nstderr: %s", err, stderr) + } + if h.DryRun() { + return + } + id := extractConnectorID(t, stdout) + h.RegisterConnectorCleanup(id) + if !strings.Contains(stdout, "connectorType: DATABRICKS") { + t.Errorf("stdout missing connectorType: DATABRICKS:\n%s", stdout) + } +} + +// TestConnectorCreateDatabricksOAuthSSO smokes the oauth-sso leaf. Asserts +// the success note references the configured endpoint (matches the Snowflake +// pattern). Requires ANA_E2E_DBX_OAUTH_CLIENT_ID + +// ANA_E2E_DBX_OAUTH_CLIENT_SECRET (Databricks OAuth app credentials, distinct +// from Service Principal credentials used by client-credentials). +func TestConnectorCreateDatabricksOAuthSSO(t *testing.T) { + common := databricksCommonEnvOrSkip(t) + clientID := os.Getenv("ANA_E2E_DBX_OAUTH_CLIENT_ID") + clientSecret := os.Getenv("ANA_E2E_DBX_OAUTH_CLIENT_SECRET") + if clientID == "" || clientSecret == "" { + t.Skip("e2e: ANA_E2E_DBX_OAUTH_CLIENT_ID and ANA_E2E_DBX_OAUTH_CLIENT_SECRET required for Databricks oauth-sso mode") + } + + h := harness.Begin(t) + h.RegisterConnectorCleanupByName(h.ResourceName("dbx-oauth-sso")) + args := append([]string{"connector", "create", "databricks", "oauth-sso"}, + databricksCommonArgs(h, "dbx-oauth-sso", common)...) + args = append(args, "--client-id", clientID, "--client-secret-stdin") + + stdout, stderr, err := h.RunStdin(clientSecret+"\n", args...) + if err != nil { + t.Fatalf("connector create databricks oauth-sso: %v\nstderr: %s", err, stderr) + } + if h.DryRun() { + return + } + id := extractConnectorID(t, stdout) + h.RegisterConnectorCleanup(id) + if !strings.Contains(stdout, "connectorType: DATABRICKS") { + t.Errorf("stdout missing connectorType: DATABRICKS:\n%s", stdout) + } + endpoint := os.Getenv("ANA_E2E_ENDPOINT") + if endpoint == "" { + t.Fatalf("ANA_E2E_ENDPOINT should be set inside Begin — got empty") + } + if !strings.Contains(stdout, "complete OAuth at "+endpoint) { + t.Errorf("oauth-sso note should reference ANA_E2E_ENDPOINT %q:\n%s", endpoint, stdout) + } +} + +// TestConnectorCreateDatabricksOAuthIndividual smokes the oauth-individual +// leaf. Asserts the per-member-lazy note since that's the only leaf-unique +// piece of stdout. Reuses the same ANA_E2E_DBX_OAUTH_CLIENT_* env pair — +// oauth-sso and oauth-individual share the same Databricks OAuth app. +func TestConnectorCreateDatabricksOAuthIndividual(t *testing.T) { + common := databricksCommonEnvOrSkip(t) + clientID := os.Getenv("ANA_E2E_DBX_OAUTH_CLIENT_ID") + clientSecret := os.Getenv("ANA_E2E_DBX_OAUTH_CLIENT_SECRET") + if clientID == "" || clientSecret == "" { + t.Skip("e2e: ANA_E2E_DBX_OAUTH_CLIENT_ID and ANA_E2E_DBX_OAUTH_CLIENT_SECRET required for Databricks oauth-individual mode") + } + + h := harness.Begin(t) + h.RegisterConnectorCleanupByName(h.ResourceName("dbx-oauth-individual")) + args := append([]string{"connector", "create", "databricks", "oauth-individual"}, + databricksCommonArgs(h, "dbx-oauth-individual", common)...) + args = append(args, "--client-id", clientID, "--client-secret-stdin") + + stdout, stderr, err := h.RunStdin(clientSecret+"\n", args...) + if err != nil { + t.Fatalf("connector create databricks oauth-individual: %v\nstderr: %s", err, stderr) + } + if h.DryRun() { + return + } + id := extractConnectorID(t, stdout) + h.RegisterConnectorCleanup(id) + if !strings.Contains(stdout, "connectorType: DATABRICKS") { + t.Errorf("stdout missing connectorType: DATABRICKS:\n%s", stdout) + } + if !strings.Contains(stdout, "lazily at first query") { + t.Errorf("oauth-individual note should mention lazy per-member auth:\n%s", stdout) + } +} From 21916174d0925c1b832152e55d495ca70a073a74 Mon Sep 17 00:00:00 2001 From: Brad Fair Date: Wed, 22 Apr 2026 13:05:12 -0500 Subject: [PATCH 5/9] refactor(connector): hoist Databricks --client-id empty-guard to helper MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit cli.RequireFlags only asserts explicit presence, so '--client-id ""' slips through — every Databricks leaf that takes --client-id needs an extra empty-string guard. Extract the inline check into a shared requireDatabricksClientID helper so the three leaves (client-credentials, oauth-sso, oauth-individual) apply identical validation and the intent is documented in one place. --- internal/connector/create_databricks.go | 13 +++++++++++++ .../create_databricks_client_credentials.go | 4 ++-- .../connector/create_databricks_oauth_individual.go | 4 ++-- internal/connector/create_databricks_oauth_sso.go | 4 ++-- 4 files changed, 19 insertions(+), 6 deletions(-) diff --git a/internal/connector/create_databricks.go b/internal/connector/create_databricks.go index 8b21686..24aa68c 100644 --- a/internal/connector/create_databricks.go +++ b/internal/connector/create_databricks.go @@ -99,3 +99,16 @@ func requireDatabricksCommon(prefix, name, host, httpPath string, port int, cata } return nil } + +// requireDatabricksClientID guards against `--client-id ""` on every Databricks +// leaf that takes one (client-credentials, oauth-sso, oauth-individual). +// cli.RequireFlags only checks that the flag was explicitly set, so a +// deliberately empty value still slips through — this helper rejects it +// consistently. Split from requireDatabricksCommon because the access-token +// leaf doesn't use --client-id. +func requireDatabricksClientID(prefix, clientID string) error { + if clientID == "" { + return cli.UsageErrf("%s: --client-id must not be empty", prefix) + } + return nil +} diff --git a/internal/connector/create_databricks_client_credentials.go b/internal/connector/create_databricks_client_credentials.go index c6745e2..28ab82a 100644 --- a/internal/connector/create_databricks_client_credentials.go +++ b/internal/connector/create_databricks_client_credentials.go @@ -59,8 +59,8 @@ func (c *databricksClientCredentialsCmd) Run(ctx context.Context, args []string, *c.name, *c.host, *c.httpPath, *c.port, *c.catalog, *c.schema); err != nil { return err } - if c.clientID == "" { - return cli.UsageErrf("connector create databricks client-credentials: --client-id must not be empty") + if err := requireDatabricksClientID("connector create databricks client-credentials", c.clientID); err != nil { + return err } resolvedSecret, err := resolveSecret("client-secret", c.clientSecret, c.secretStdin, stdio.Stdin) if err != nil { diff --git a/internal/connector/create_databricks_oauth_individual.go b/internal/connector/create_databricks_oauth_individual.go index 07117dd..7a08a6e 100644 --- a/internal/connector/create_databricks_oauth_individual.go +++ b/internal/connector/create_databricks_oauth_individual.go @@ -59,8 +59,8 @@ func (c *databricksOAuthIndividualCmd) Run(ctx context.Context, args []string, s *c.name, *c.host, *c.httpPath, *c.port, *c.catalog, *c.schema); err != nil { return err } - if c.clientID == "" { - return cli.UsageErrf("connector create databricks oauth-individual: --client-id must not be empty") + if err := requireDatabricksClientID("connector create databricks oauth-individual", c.clientID); err != nil { + return err } resolvedSecret, err := resolveSecret("client-secret", c.clientSecret, c.secretStdin, stdio.Stdin) if err != nil { diff --git a/internal/connector/create_databricks_oauth_sso.go b/internal/connector/create_databricks_oauth_sso.go index db85b53..184c6ea 100644 --- a/internal/connector/create_databricks_oauth_sso.go +++ b/internal/connector/create_databricks_oauth_sso.go @@ -64,8 +64,8 @@ func (c *databricksOAuthSSOCmd) Run(ctx context.Context, args []string, stdio cl *c.name, *c.host, *c.httpPath, *c.port, *c.catalog, *c.schema); err != nil { return err } - if c.clientID == "" { - return cli.UsageErrf("connector create databricks oauth-sso: --client-id must not be empty") + if err := requireDatabricksClientID("connector create databricks oauth-sso", c.clientID); err != nil { + return err } resolvedSecret, err := resolveSecret("client-secret", c.clientSecret, c.secretStdin, stdio.Stdin) if err != nil { From bdc0c0255ca491c0fba31cd98c794f903c9a33db Mon Sep 17 00:00:00 2001 From: Brad Fair Date: Wed, 22 Apr 2026 13:05:23 -0500 Subject: [PATCH 6/9] test(e2e): expose harness.Endpoint() and drop redundant env re-reads The OAuth-SSO smoke tests in both the Snowflake and Databricks suites were re-fetching ANA_E2E_ENDPOINT via os.Getenv and asserting non-empty, but harness.Begin already validates the var and skips the test if it's unset. Add a thin H.Endpoint() accessor so tests that need the endpoint (to match it against CLI stdout) pull it from the harness rather than re-reading the environment. --- e2e/connector_databricks_test.go | 7 ++----- e2e/connector_snowflake_test.go | 7 ++----- e2e/harness/harness.go | 5 +++++ 3 files changed, 9 insertions(+), 10 deletions(-) diff --git a/e2e/connector_databricks_test.go b/e2e/connector_databricks_test.go index 79f6c7c..b7eefd5 100644 --- a/e2e/connector_databricks_test.go +++ b/e2e/connector_databricks_test.go @@ -150,12 +150,9 @@ func TestConnectorCreateDatabricksOAuthSSO(t *testing.T) { if !strings.Contains(stdout, "connectorType: DATABRICKS") { t.Errorf("stdout missing connectorType: DATABRICKS:\n%s", stdout) } - endpoint := os.Getenv("ANA_E2E_ENDPOINT") - if endpoint == "" { - t.Fatalf("ANA_E2E_ENDPOINT should be set inside Begin — got empty") - } + endpoint := h.Endpoint() if !strings.Contains(stdout, "complete OAuth at "+endpoint) { - t.Errorf("oauth-sso note should reference ANA_E2E_ENDPOINT %q:\n%s", endpoint, stdout) + t.Errorf("oauth-sso note should reference harness endpoint %q:\n%s", endpoint, stdout) } } diff --git a/e2e/connector_snowflake_test.go b/e2e/connector_snowflake_test.go index 2fdb44a..7665152 100644 --- a/e2e/connector_snowflake_test.go +++ b/e2e/connector_snowflake_test.go @@ -186,12 +186,9 @@ func TestConnectorCreateSnowflakeOAuthSSO(t *testing.T) { if !strings.Contains(stdout, "connectorType: SNOWFLAKE") { t.Errorf("stdout missing connectorType: SNOWFLAKE:\n%s", stdout) } - endpoint := os.Getenv("ANA_E2E_ENDPOINT") - if endpoint == "" { - t.Fatalf("ANA_E2E_ENDPOINT should be set inside Begin — got empty") - } + endpoint := h.Endpoint() if !strings.Contains(stdout, "complete OAuth at "+endpoint) { - t.Errorf("oauth-sso note should reference ANA_E2E_ENDPOINT %q:\n%s", endpoint, stdout) + t.Errorf("oauth-sso note should reference harness endpoint %q:\n%s", endpoint, stdout) } } diff --git a/e2e/harness/harness.go b/e2e/harness/harness.go index 0732eb4..2aad299 100644 --- a/e2e/harness/harness.go +++ b/e2e/harness/harness.go @@ -132,6 +132,11 @@ func (h *H) Client() *transport.Client { return h.client } // ExpectOrgID returns the ANA_E2E_EXPECT_ORG_ID value the harness validated. func (h *H) ExpectOrgID() string { return h.env.expectOrgID } +// Endpoint returns the ANA_E2E_ENDPOINT value the harness validated at Begin. +// Tests that need to assert endpoint-referencing stdout (e.g. OAuth callback +// URLs) should read it through here rather than re-fetching the env var. +func (h *H) Endpoint() string { return h.env.endpoint } + // defer registers fn to run in LIFO order when End fires. Exported indirectly // via Register so callers can deregister on successful delete. func (h *H) Register(fn func()) { From 5d14d5c22c3b6677ab22c5d78fc140228f88823e Mon Sep 17 00:00:00 2001 From: Brad Fair Date: Wed, 22 Apr 2026 13:05:33 -0500 Subject: [PATCH 7/9] test(cli): route Declare* flag-helper tests through ParseFlags stdlib flag.Parse re-wraps Set errors with %v, so the ErrUsage chain that callers rely on only survives when parsing goes through cli.ParseFlags. The six Declare{String,Bool,Int}* tests were calling fs.Parse directly, bypassing the production path. Route them through ParseFlags to match the rule already enforced in flags_test.go. --- internal/cli/cli_test.go | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/internal/cli/cli_test.go b/internal/cli/cli_test.go index 360f336..fe4b6ab 100644 --- a/internal/cli/cli_test.go +++ b/internal/cli/cli_test.go @@ -986,7 +986,7 @@ func TestDeclareStringGuardsAgainstRedeclare(t *testing.T) { var leafT, ancT string fs.StringVar(&leafT, "foo", "leafdef", "leaf usage") DeclareString(fs, &ancT, "foo", "ancdef", "anc usage") - if err := fs.Parse([]string{"--foo", "x"}); err != nil { + if err := ParseFlags(fs, []string{"--foo", "x"}); err != nil { t.Fatalf("parse err=%v", err) } if leafT != "x" { @@ -1003,7 +1003,7 @@ func TestDeclareBoolGuardsAgainstRedeclare(t *testing.T) { var leafT, ancT bool fs.BoolVar(&leafT, "v", false, "leaf usage") DeclareBool(fs, &ancT, "v", false, "anc usage") - if err := fs.Parse([]string{"--v"}); err != nil { + if err := ParseFlags(fs, []string{"--v"}); err != nil { t.Fatalf("parse err=%v", err) } if !leafT { @@ -1020,7 +1020,7 @@ func TestDeclareBoolFreshDeclaration(t *testing.T) { fs := flag.NewFlagSet("t", flag.ContinueOnError) var target bool DeclareBool(fs, &target, "v", false, "usage") - if err := fs.Parse([]string{"--v"}); err != nil { + if err := ParseFlags(fs, []string{"--v"}); err != nil { t.Fatalf("parse err=%v", err) } if !target { @@ -1033,7 +1033,7 @@ func TestDeclareStringFreshDeclaration(t *testing.T) { fs := flag.NewFlagSet("t", flag.ContinueOnError) var target string DeclareString(fs, &target, "s", "def", "usage") - if err := fs.Parse([]string{"--s", "x"}); err != nil { + if err := ParseFlags(fs, []string{"--s", "x"}); err != nil { t.Fatalf("parse err=%v", err) } if target != "x" { @@ -1047,7 +1047,7 @@ func TestDeclareIntGuardsAgainstRedeclare(t *testing.T) { var leafT, ancT int fs.IntVar(&leafT, "port", 1, "leaf usage") DeclareInt(fs, &ancT, "port", 2, "anc usage") - if err := fs.Parse([]string{"--port", "7"}); err != nil { + if err := ParseFlags(fs, []string{"--port", "7"}); err != nil { t.Fatalf("parse err=%v", err) } if leafT != 7 { @@ -1063,7 +1063,7 @@ func TestDeclareIntFreshDeclaration(t *testing.T) { fs := flag.NewFlagSet("t", flag.ContinueOnError) var target int DeclareInt(fs, &target, "port", 443, "usage") - if err := fs.Parse([]string{"--port", "5432"}); err != nil { + if err := ParseFlags(fs, []string{"--port", "5432"}); err != nil { t.Fatalf("parse err=%v", err) } if target != 5432 { @@ -1076,7 +1076,7 @@ func TestDeclareIntDefault(t *testing.T) { fs := flag.NewFlagSet("t", flag.ContinueOnError) var target int DeclareInt(fs, &target, "port", 443, "usage") - if err := fs.Parse(nil); err != nil { + if err := ParseFlags(fs, nil); err != nil { t.Fatalf("parse err=%v", err) } if target != 443 { From 0f50b552cafb6388e0814d15836af0ac1b78e892 Mon Sep 17 00:00:00 2001 From: Brad Fair Date: Wed, 22 Apr 2026 13:19:02 -0500 Subject: [PATCH 8/9] test(e2e): verify create with connector get across all auth-mode leaves Databricks client-credentials and oauth-individual were missing the post-create `connector get` verification that access-token already had. Same drift had crept into Snowflake keypair/oauth-sso/oauth-individual. Add the get round-trip to every create-leaf smoke so the server read-back is uniformly asserted. --- e2e/connector_databricks_test.go | 9 +++++++++ e2e/connector_snowflake_test.go | 9 +++++++++ 2 files changed, 18 insertions(+) diff --git a/e2e/connector_databricks_test.go b/e2e/connector_databricks_test.go index b7eefd5..93c98b2 100644 --- a/e2e/connector_databricks_test.go +++ b/e2e/connector_databricks_test.go @@ -117,6 +117,9 @@ func TestConnectorCreateDatabricksClientCredentials(t *testing.T) { if !strings.Contains(stdout, "connectorType: DATABRICKS") { t.Errorf("stdout missing connectorType: DATABRICKS:\n%s", stdout) } + if _, estderr, gerr := h.Run("connector", "get", fmt.Sprint(id)); gerr != nil { + t.Fatalf("connector get %d: %v\nstderr: %s", id, gerr, estderr) + } } // TestConnectorCreateDatabricksOAuthSSO smokes the oauth-sso leaf. Asserts @@ -154,6 +157,9 @@ func TestConnectorCreateDatabricksOAuthSSO(t *testing.T) { if !strings.Contains(stdout, "complete OAuth at "+endpoint) { t.Errorf("oauth-sso note should reference harness endpoint %q:\n%s", endpoint, stdout) } + if _, estderr, gerr := h.Run("connector", "get", fmt.Sprint(id)); gerr != nil { + t.Fatalf("connector get %d: %v\nstderr: %s", id, gerr, estderr) + } } // TestConnectorCreateDatabricksOAuthIndividual smokes the oauth-individual @@ -189,4 +195,7 @@ func TestConnectorCreateDatabricksOAuthIndividual(t *testing.T) { if !strings.Contains(stdout, "lazily at first query") { t.Errorf("oauth-individual note should mention lazy per-member auth:\n%s", stdout) } + if _, estderr, gerr := h.Run("connector", "get", fmt.Sprint(id)); gerr != nil { + t.Fatalf("connector get %d: %v\nstderr: %s", id, gerr, estderr) + } } diff --git a/e2e/connector_snowflake_test.go b/e2e/connector_snowflake_test.go index 7665152..9865453 100644 --- a/e2e/connector_snowflake_test.go +++ b/e2e/connector_snowflake_test.go @@ -155,6 +155,9 @@ func TestConnectorCreateSnowflakeKeypair(t *testing.T) { if !strings.Contains(stdout, "connectorType: SNOWFLAKE") { t.Errorf("stdout missing connectorType: SNOWFLAKE:\n%s", stdout) } + if _, estderr, gerr := h.Run("connector", "get", fmt.Sprint(id)); gerr != nil { + t.Fatalf("connector get %d: %v\nstderr: %s", id, gerr, estderr) + } } // TestConnectorCreateSnowflakeOAuthSSO smokes the oauth-sso leaf. Asserts the @@ -190,6 +193,9 @@ func TestConnectorCreateSnowflakeOAuthSSO(t *testing.T) { if !strings.Contains(stdout, "complete OAuth at "+endpoint) { t.Errorf("oauth-sso note should reference harness endpoint %q:\n%s", endpoint, stdout) } + if _, estderr, gerr := h.Run("connector", "get", fmt.Sprint(id)); gerr != nil { + t.Fatalf("connector get %d: %v\nstderr: %s", id, gerr, estderr) + } } // TestConnectorCreateSnowflakeOAuthIndividual smokes the oauth-individual @@ -224,4 +230,7 @@ func TestConnectorCreateSnowflakeOAuthIndividual(t *testing.T) { if !strings.Contains(stdout, "lazily at first query") { t.Errorf("oauth-individual note should mention lazy per-member auth:\n%s", stdout) } + if _, estderr, gerr := h.Run("connector", "get", fmt.Sprint(id)); gerr != nil { + t.Fatalf("connector get %d: %v\nstderr: %s", id, gerr, estderr) + } } From 36c61acfd800ae07e837dd4f8b187d6783fdd9c6 Mon Sep 17 00:00:00 2001 From: Brad Fair Date: Wed, 22 Apr 2026 13:34:27 -0500 Subject: [PATCH 9/9] refactor(e2e): extract connectorCreateLeaf helper shared by Snowflake + Databricks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The create/dry-run/id/cleanup/connectorType/get round-trip was copy-pasted across all 8 Snowflake + Databricks create-leaf smokes, and the parity gap that motivated 0f50b55 (missing `connector get` on 5 of 8 leaves) could only recur because each test hand-rolled the sequence. Hoist the pattern into connectorCreateLeaf{Name,Args,Stdin,ConnectorType, Extra}.Run — the only way to skip the `get` round-trip now is to bypass the helper on purpose. Also moves extractConnectorID (and its shared regex) into the new dialect-neutral helpers file so it no longer lives awkwardly next to Snowflake-only code. Leaf-specific stdout assertions (OAuth endpoint note, per-member-lazy note) ride along on the `Extra` hook so each test stays a single declarative block. --- e2e/CLAUDE.md | 1 + e2e/connector_create_leaves_test.go | 86 ++++++++++++++++ e2e/connector_databricks_test.go | 126 ++++++++--------------- e2e/connector_snowflake_test.go | 149 ++++++++-------------------- 4 files changed, 173 insertions(+), 189 deletions(-) create mode 100644 e2e/connector_create_leaves_test.go diff --git a/e2e/CLAUDE.md b/e2e/CLAUDE.md index 2f38ea9..f7dad90 100644 --- a/e2e/CLAUDE.md +++ b/e2e/CLAUDE.md @@ -13,6 +13,7 @@ Live-smoke tests that drive real `app.textql.com` RPCs through the same verb pac | `auth_test.go` | Keys + service-accounts CLI-driven create/rotate/revoke/delete (helper-backed legacy tests still live here for coverage); `--json` shape checks + error-path smokes for usage guards. | | `chat_test.go` | Chat CRUD + streaming `send`, `--json` envelopes on list/show, CLI-path `chat new`, `history`/`bookmark`/`unbookmark`, `show ` error-path. | | `connector_test.go` | Connector CRUD + `--json` envelopes, CLI postgres create matrix (password-stdin × ssl on/off), `update --password-stdin`, `tables`/`examples`/`test` leaves, `get ` error-path. | +| `connector_create_leaves_test.go` | Dialect-neutral `connectorCreateLeaf` helper + shared `extractConnectorID`; every Snowflake/Databricks create-leaf smoke runs its create/dry-run/id/cleanup/connectorType/get round-trip through this helper so the pattern can't drift. | | `connector_snowflake_test.go` | Snowflake create leaves (password/keypair/oauth-sso/oauth-individual), per-mode env-gated. | | `connector_databricks_test.go` | Databricks create leaves (access-token/client-credentials/oauth-sso/oauth-individual), per-mode env-gated on `ANA_E2E_DBX_*`. | | `dashboard_test.go` | Dashboard list/get/folders read leaves (default + `--json`); `health`/`spawn` env-gated on `ANA_E2E_DASHBOARD_ID`. | diff --git a/e2e/connector_create_leaves_test.go b/e2e/connector_create_leaves_test.go new file mode 100644 index 0000000..cd4a56a --- /dev/null +++ b/e2e/connector_create_leaves_test.go @@ -0,0 +1,86 @@ +package e2e + +import ( + "fmt" + "regexp" + "strconv" + "strings" + "testing" + + "github.com/highperformance-tech/ana-cli/e2e/harness" +) + +// connectorIDRE extracts `connectorId: ` from the first line of non-JSON +// stdout emitted by every `connector create ` leaf. +var connectorIDRE = regexp.MustCompile(`(?m)^connectorId:\s+(\d+)\s*$`) + +// extractConnectorID pulls the integer id out of `connectorId: ` stdout. +// Fails the test if no match — every create leaf's contract is to emit this +// line on success, so a miss means the output shape drifted. +func extractConnectorID(t *testing.T, stdout string) int { + t.Helper() + m := connectorIDRE.FindStringSubmatch(stdout) + if len(m) != 2 { + t.Fatalf("could not find connectorId in stdout:\n%s", stdout) + } + id, err := strconv.Atoi(m[1]) + if err != nil { + t.Fatalf("connectorId %q is not an int: %v", m[1], err) + } + return id +} + +// connectorCreateLeaf bundles the invariants every connector-create smoke +// shares: run the command, skip post-create assertions in dry-run, extract + +// register the id, assert `connectorType: `, run any leaf-specific +// stdout checks, then read the row back via `connector get` to confirm the +// server persisted the new connector. +// +// The helper exists so a parity slip (e.g., a new leaf forgetting the `get` +// round-trip) can only happen if a test intentionally bypasses this wrapper. +type connectorCreateLeaf struct { + // Name is the leaf identifier used in fatal error messages — typically + // "databricks access-token" or "snowflake oauth-sso". + Name string + // Args is the full argv passed to `h.RunStdin`, starting with + // "connector", "create", , , ... + Args []string + // Stdin is the stdin payload for secret flags (token, password, etc.). + // Empty when no --*-stdin flag is used. + Stdin string + // ConnectorType is the dialect tag asserted in stdout, e.g. "DATABRICKS" + // or "SNOWFLAKE". Matched against the literal `connectorType: ` line. + ConnectorType string + // Extra runs after the common assertions and before the `connector get` + // round-trip. Use it for leaf-unique stdout fragments (OAuth endpoint + // note, per-member-lazy note, etc.). May be nil. + Extra func(stdout string) +} + +// Run executes the leaf smoke. On non-dry-run success, the created connector +// id is registered for cleanup and read back via `connector get`. Returns the +// created id so callers can chain additional assertions if needed; in dry-run +// mode the returned id is 0. +func (l connectorCreateLeaf) Run(t *testing.T, h *harness.H) int { + t.Helper() + stdout, stderr, err := h.RunStdin(l.Stdin, l.Args...) + if err != nil { + t.Fatalf("connector create %s: %v\nstderr: %s", l.Name, err, stderr) + } + if h.DryRun() { + return 0 + } + id := extractConnectorID(t, stdout) + h.RegisterConnectorCleanup(id) + typeLine := "connectorType: " + l.ConnectorType + if !strings.Contains(stdout, typeLine) { + t.Errorf("stdout missing %s:\n%s", typeLine, stdout) + } + if l.Extra != nil { + l.Extra(stdout) + } + if _, estderr, gerr := h.Run("connector", "get", fmt.Sprint(id)); gerr != nil { + t.Fatalf("connector get %d: %v\nstderr: %s", id, gerr, estderr) + } + return id +} diff --git a/e2e/connector_databricks_test.go b/e2e/connector_databricks_test.go index 93c98b2..77e4b92 100644 --- a/e2e/connector_databricks_test.go +++ b/e2e/connector_databricks_test.go @@ -1,7 +1,6 @@ package e2e import ( - "fmt" "os" "strings" "testing" @@ -55,6 +54,15 @@ func databricksCommonArgs(h *harness.H, suffix string, env dbxCommonEnv) []strin return args } +// databricksLeafArgs builds the full argv for `connector create databricks +// ` using the shared workspace flags. `suffix` seeds the name-based +// cleanup safety-net; `extra` carries the auth-mode-specific flags. +func databricksLeafArgs(h *harness.H, authMode, suffix string, env dbxCommonEnv, extra ...string) []string { + args := append([]string{"connector", "create", "databricks", authMode}, + databricksCommonArgs(h, suffix, env)...) + return append(args, extra...) +} + // TestConnectorCreateDatabricksAccessToken smokes // `connector create databricks access-token --token-stdin`. Requires // ANA_E2E_DBX_TOKEN in addition to the common workspace env. @@ -67,25 +75,12 @@ func TestConnectorCreateDatabricksAccessToken(t *testing.T) { h := harness.Begin(t) h.RegisterConnectorCleanupByName(h.ResourceName("dbx-access-token")) - args := append([]string{"connector", "create", "databricks", "access-token"}, - databricksCommonArgs(h, "dbx-access-token", common)...) - args = append(args, "--token-stdin") - - stdout, stderr, err := h.RunStdin(token+"\n", args...) - if err != nil { - t.Fatalf("connector create databricks access-token: %v\nstderr: %s", err, stderr) - } - if h.DryRun() { - return - } - id := extractConnectorID(t, stdout) - h.RegisterConnectorCleanup(id) - if !strings.Contains(stdout, "connectorType: DATABRICKS") { - t.Errorf("stdout missing connectorType: DATABRICKS:\n%s", stdout) - } - if _, estderr, gerr := h.Run("connector", "get", fmt.Sprint(id)); gerr != nil { - t.Fatalf("connector get %d: %v\nstderr: %s", id, gerr, estderr) - } + connectorCreateLeaf{ + Name: "databricks access-token", + Args: databricksLeafArgs(h, "access-token", "dbx-access-token", common, "--token-stdin"), + Stdin: token + "\n", + ConnectorType: "DATABRICKS", + }.Run(t, h) } // TestConnectorCreateDatabricksClientCredentials smokes the M2M leaf. @@ -101,25 +96,12 @@ func TestConnectorCreateDatabricksClientCredentials(t *testing.T) { h := harness.Begin(t) h.RegisterConnectorCleanupByName(h.ResourceName("dbx-client-credentials")) - args := append([]string{"connector", "create", "databricks", "client-credentials"}, - databricksCommonArgs(h, "dbx-client-credentials", common)...) - args = append(args, "--client-id", clientID, "--client-secret-stdin") - - stdout, stderr, err := h.RunStdin(clientSecret+"\n", args...) - if err != nil { - t.Fatalf("connector create databricks client-credentials: %v\nstderr: %s", err, stderr) - } - if h.DryRun() { - return - } - id := extractConnectorID(t, stdout) - h.RegisterConnectorCleanup(id) - if !strings.Contains(stdout, "connectorType: DATABRICKS") { - t.Errorf("stdout missing connectorType: DATABRICKS:\n%s", stdout) - } - if _, estderr, gerr := h.Run("connector", "get", fmt.Sprint(id)); gerr != nil { - t.Fatalf("connector get %d: %v\nstderr: %s", id, gerr, estderr) - } + connectorCreateLeaf{ + Name: "databricks client-credentials", + Args: databricksLeafArgs(h, "client-credentials", "dbx-client-credentials", common, "--client-id", clientID, "--client-secret-stdin"), + Stdin: clientSecret + "\n", + ConnectorType: "DATABRICKS", + }.Run(t, h) } // TestConnectorCreateDatabricksOAuthSSO smokes the oauth-sso leaf. Asserts @@ -137,29 +119,18 @@ func TestConnectorCreateDatabricksOAuthSSO(t *testing.T) { h := harness.Begin(t) h.RegisterConnectorCleanupByName(h.ResourceName("dbx-oauth-sso")) - args := append([]string{"connector", "create", "databricks", "oauth-sso"}, - databricksCommonArgs(h, "dbx-oauth-sso", common)...) - args = append(args, "--client-id", clientID, "--client-secret-stdin") - - stdout, stderr, err := h.RunStdin(clientSecret+"\n", args...) - if err != nil { - t.Fatalf("connector create databricks oauth-sso: %v\nstderr: %s", err, stderr) - } - if h.DryRun() { - return - } - id := extractConnectorID(t, stdout) - h.RegisterConnectorCleanup(id) - if !strings.Contains(stdout, "connectorType: DATABRICKS") { - t.Errorf("stdout missing connectorType: DATABRICKS:\n%s", stdout) - } endpoint := h.Endpoint() - if !strings.Contains(stdout, "complete OAuth at "+endpoint) { - t.Errorf("oauth-sso note should reference harness endpoint %q:\n%s", endpoint, stdout) - } - if _, estderr, gerr := h.Run("connector", "get", fmt.Sprint(id)); gerr != nil { - t.Fatalf("connector get %d: %v\nstderr: %s", id, gerr, estderr) - } + connectorCreateLeaf{ + Name: "databricks oauth-sso", + Args: databricksLeafArgs(h, "oauth-sso", "dbx-oauth-sso", common, "--client-id", clientID, "--client-secret-stdin"), + Stdin: clientSecret + "\n", + ConnectorType: "DATABRICKS", + Extra: func(stdout string) { + if !strings.Contains(stdout, "complete OAuth at "+endpoint) { + t.Errorf("oauth-sso note should reference harness endpoint %q:\n%s", endpoint, stdout) + } + }, + }.Run(t, h) } // TestConnectorCreateDatabricksOAuthIndividual smokes the oauth-individual @@ -176,26 +147,15 @@ func TestConnectorCreateDatabricksOAuthIndividual(t *testing.T) { h := harness.Begin(t) h.RegisterConnectorCleanupByName(h.ResourceName("dbx-oauth-individual")) - args := append([]string{"connector", "create", "databricks", "oauth-individual"}, - databricksCommonArgs(h, "dbx-oauth-individual", common)...) - args = append(args, "--client-id", clientID, "--client-secret-stdin") - - stdout, stderr, err := h.RunStdin(clientSecret+"\n", args...) - if err != nil { - t.Fatalf("connector create databricks oauth-individual: %v\nstderr: %s", err, stderr) - } - if h.DryRun() { - return - } - id := extractConnectorID(t, stdout) - h.RegisterConnectorCleanup(id) - if !strings.Contains(stdout, "connectorType: DATABRICKS") { - t.Errorf("stdout missing connectorType: DATABRICKS:\n%s", stdout) - } - if !strings.Contains(stdout, "lazily at first query") { - t.Errorf("oauth-individual note should mention lazy per-member auth:\n%s", stdout) - } - if _, estderr, gerr := h.Run("connector", "get", fmt.Sprint(id)); gerr != nil { - t.Fatalf("connector get %d: %v\nstderr: %s", id, gerr, estderr) - } + connectorCreateLeaf{ + Name: "databricks oauth-individual", + Args: databricksLeafArgs(h, "oauth-individual", "dbx-oauth-individual", common, "--client-id", clientID, "--client-secret-stdin"), + Stdin: clientSecret + "\n", + ConnectorType: "DATABRICKS", + Extra: func(stdout string) { + if !strings.Contains(stdout, "lazily at first query") { + t.Errorf("oauth-individual note should mention lazy per-member auth:\n%s", stdout) + } + }, + }.Run(t, h) } diff --git a/e2e/connector_snowflake_test.go b/e2e/connector_snowflake_test.go index 9865453..43da69c 100644 --- a/e2e/connector_snowflake_test.go +++ b/e2e/connector_snowflake_test.go @@ -1,21 +1,13 @@ package e2e import ( - "fmt" "os" - "regexp" - "strconv" "strings" "testing" "github.com/highperformance-tech/ana-cli/e2e/harness" ) -// connectorId: is the first line of non-JSON stdout from every -// snowflake create leaf; this regex extracts the id so the test can register -// cleanup and assert on the value. -var snowflakeConnectorIDRE = regexp.MustCompile(`(?m)^connectorId:\s+(\d+)\s*$`) - // sfCommonEnv holds the Snowflake connection fields every auth mode shares. // Empty optional fields are preserved so tests can exercise omitempty paths // (warehouse/schema/role). @@ -67,20 +59,13 @@ func snowflakeCommonArgs(h *harness.H, suffix string, env sfCommonEnv) []string return args } -// extractConnectorID pulls the integer id out of `connectorId: ` stdout. -// Fails the test if no match — the leaf's contract is to always emit this line -// on success, so a miss means the output shape drifted. -func extractConnectorID(t *testing.T, stdout string) int { - t.Helper() - m := snowflakeConnectorIDRE.FindStringSubmatch(stdout) - if len(m) != 2 { - t.Fatalf("could not find connectorId in stdout:\n%s", stdout) - } - id, err := strconv.Atoi(m[1]) - if err != nil { - t.Fatalf("connectorId %q is not an int: %v", m[1], err) - } - return id +// snowflakeLeafArgs builds the full argv for `connector create snowflake +// ` using the shared connection flags. `suffix` seeds the +// name-based cleanup safety-net; `extra` carries the auth-mode-specific flags. +func snowflakeLeafArgs(h *harness.H, authMode, suffix string, env sfCommonEnv, extra ...string) []string { + args := append([]string{"connector", "create", "snowflake", authMode}, + snowflakeCommonArgs(h, suffix, env)...) + return append(args, extra...) } // TestConnectorCreateSnowflakePassword smokes `connector create snowflake @@ -98,26 +83,12 @@ func TestConnectorCreateSnowflakePassword(t *testing.T) { // Pre-register a name-based safety-net cleanup so a successful create // followed by a failing extractConnectorID can't orphan the connector. h.RegisterConnectorCleanupByName(h.ResourceName("sf-password")) - args := append([]string{"connector", "create", "snowflake", "password"}, - snowflakeCommonArgs(h, "sf-password", common)...) - args = append(args, "--user", user, "--password-stdin") - - stdout, stderr, err := h.RunStdin(password+"\n", args...) - if err != nil { - t.Fatalf("connector create snowflake password: %v\nstderr: %s", err, stderr) - } - if h.DryRun() { - return - } - id := extractConnectorID(t, stdout) - h.RegisterConnectorCleanup(id) - if !strings.Contains(stdout, "connectorType: SNOWFLAKE") { - t.Errorf("stdout missing connectorType: SNOWFLAKE:\n%s", stdout) - } - // Verify the server can read the new row back. - if _, estderr, gerr := h.Run("connector", "get", fmt.Sprint(id)); gerr != nil { - t.Fatalf("connector get %d: %v\nstderr: %s", id, gerr, estderr) - } + connectorCreateLeaf{ + Name: "snowflake password", + Args: snowflakeLeafArgs(h, "password", "sf-password", common, "--user", user, "--password-stdin"), + Stdin: password + "\n", + ConnectorType: "SNOWFLAKE", + }.Run(t, h) } // TestConnectorCreateSnowflakeKeypair smokes the keypair leaf. Requires @@ -134,30 +105,18 @@ func TestConnectorCreateSnowflakeKeypair(t *testing.T) { h := harness.Begin(t) h.RegisterConnectorCleanupByName(h.ResourceName("sf-keypair")) - args := append([]string{"connector", "create", "snowflake", "keypair"}, - snowflakeCommonArgs(h, "sf-keypair", common)...) - args = append(args, "--user", user, "--private-key-file", keyPath) + extra := []string{"--user", user, "--private-key-file", keyPath} stdin := "" if passphrase != "" { - args = append(args, "--private-key-passphrase-stdin") + extra = append(extra, "--private-key-passphrase-stdin") stdin = passphrase + "\n" } - - stdout, stderr, err := h.RunStdin(stdin, args...) - if err != nil { - t.Fatalf("connector create snowflake keypair: %v\nstderr: %s", err, stderr) - } - if h.DryRun() { - return - } - id := extractConnectorID(t, stdout) - h.RegisterConnectorCleanup(id) - if !strings.Contains(stdout, "connectorType: SNOWFLAKE") { - t.Errorf("stdout missing connectorType: SNOWFLAKE:\n%s", stdout) - } - if _, estderr, gerr := h.Run("connector", "get", fmt.Sprint(id)); gerr != nil { - t.Fatalf("connector get %d: %v\nstderr: %s", id, gerr, estderr) - } + connectorCreateLeaf{ + Name: "snowflake keypair", + Args: snowflakeLeafArgs(h, "keypair", "sf-keypair", common, extra...), + Stdin: stdin, + ConnectorType: "SNOWFLAKE", + }.Run(t, h) } // TestConnectorCreateSnowflakeOAuthSSO smokes the oauth-sso leaf. Asserts the @@ -173,29 +132,18 @@ func TestConnectorCreateSnowflakeOAuthSSO(t *testing.T) { h := harness.Begin(t) h.RegisterConnectorCleanupByName(h.ResourceName("sf-oauth-sso")) - args := append([]string{"connector", "create", "snowflake", "oauth-sso"}, - snowflakeCommonArgs(h, "sf-oauth-sso", common)...) - args = append(args, "--oauth-client-id", clientID, "--oauth-client-secret-stdin") - - stdout, stderr, err := h.RunStdin(clientSecret+"\n", args...) - if err != nil { - t.Fatalf("connector create snowflake oauth-sso: %v\nstderr: %s", err, stderr) - } - if h.DryRun() { - return - } - id := extractConnectorID(t, stdout) - h.RegisterConnectorCleanup(id) - if !strings.Contains(stdout, "connectorType: SNOWFLAKE") { - t.Errorf("stdout missing connectorType: SNOWFLAKE:\n%s", stdout) - } endpoint := h.Endpoint() - if !strings.Contains(stdout, "complete OAuth at "+endpoint) { - t.Errorf("oauth-sso note should reference harness endpoint %q:\n%s", endpoint, stdout) - } - if _, estderr, gerr := h.Run("connector", "get", fmt.Sprint(id)); gerr != nil { - t.Fatalf("connector get %d: %v\nstderr: %s", id, gerr, estderr) - } + connectorCreateLeaf{ + Name: "snowflake oauth-sso", + Args: snowflakeLeafArgs(h, "oauth-sso", "sf-oauth-sso", common, "--oauth-client-id", clientID, "--oauth-client-secret-stdin"), + Stdin: clientSecret + "\n", + ConnectorType: "SNOWFLAKE", + Extra: func(stdout string) { + if !strings.Contains(stdout, "complete OAuth at "+endpoint) { + t.Errorf("oauth-sso note should reference harness endpoint %q:\n%s", endpoint, stdout) + } + }, + }.Run(t, h) } // TestConnectorCreateSnowflakeOAuthIndividual smokes the oauth-individual @@ -211,26 +159,15 @@ func TestConnectorCreateSnowflakeOAuthIndividual(t *testing.T) { h := harness.Begin(t) h.RegisterConnectorCleanupByName(h.ResourceName("sf-oauth-individual")) - args := append([]string{"connector", "create", "snowflake", "oauth-individual"}, - snowflakeCommonArgs(h, "sf-oauth-individual", common)...) - args = append(args, "--oauth-client-id", clientID, "--oauth-client-secret-stdin") - - stdout, stderr, err := h.RunStdin(clientSecret+"\n", args...) - if err != nil { - t.Fatalf("connector create snowflake oauth-individual: %v\nstderr: %s", err, stderr) - } - if h.DryRun() { - return - } - id := extractConnectorID(t, stdout) - h.RegisterConnectorCleanup(id) - if !strings.Contains(stdout, "connectorType: SNOWFLAKE") { - t.Errorf("stdout missing connectorType: SNOWFLAKE:\n%s", stdout) - } - if !strings.Contains(stdout, "lazily at first query") { - t.Errorf("oauth-individual note should mention lazy per-member auth:\n%s", stdout) - } - if _, estderr, gerr := h.Run("connector", "get", fmt.Sprint(id)); gerr != nil { - t.Fatalf("connector get %d: %v\nstderr: %s", id, gerr, estderr) - } + connectorCreateLeaf{ + Name: "snowflake oauth-individual", + Args: snowflakeLeafArgs(h, "oauth-individual", "sf-oauth-individual", common, "--oauth-client-id", clientID, "--oauth-client-secret-stdin"), + Stdin: clientSecret + "\n", + ConnectorType: "SNOWFLAKE", + Extra: func(stdout string) { + if !strings.Contains(stdout, "lazily at first query") { + t.Errorf("oauth-individual note should mention lazy per-member auth:\n%s", stdout) + } + }, + }.Run(t, h) }