From 452fd2ecf9d62e9a943e6d35ce830aff90d531ef Mon Sep 17 00:00:00 2001 From: Ignacio Van Droogenbroeck Date: Sun, 31 May 2026 20:51:53 -0300 Subject: [PATCH 1/3] feat(db,measurement): db admin + measurement listing (PR3) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR3 of the phased arcctl build-out. Adds `arcctl db {list,show, create,drop}` and `arcctl measurement list` subcommands plus the matching internal/client/database.go. Wire surface (verified against arc/internal/api/databases.go): - GET /api/v1/databases (any-valid-token) - POST /api/v1/databases (any-valid-token) *see #471 - GET /api/v1/databases/:name (any-valid-token) - GET /api/v1/databases/:name/measurements (any-valid-token) - DELETE /api/v1/databases/:name?confirm=true (admin + delete.enabled=true) Highlights - client/database.go: 5 typed wrappers with bounded response reads, consistent error decoding (re-uses PR2's decodeWriteError), URL escaping on :name path segments - client.go: new setCrossDBHeaders() so cross-database endpoints never accidentally inherit the client's default x-arc-database header — the "absence is explicit" pattern - db drop: client-side y/N prompt (per CLAUDE.md destructive-ops rule), --yes / -y to skip. Surfaces the server's verbatim error on 403 (e.g. "Set delete.enabled=true in arc.toml to enable") - All list/show output formats (table / json / csv) sort by name so the three formats agree on row order; server-reported Count is passed through verbatim (not re-derived from len(slice)) - measurement list falls back to active connection's default_database when --database omitted; client-side error before any network call if neither set - Three flag helpers (addCommonConnectionFlags, addTimeoutFlag, confirmDestructive) so future PRs don't drift on flag wording Verification - 13 client tests (httptest): list/get/create/delete + cross-db header pinning + URL escaping + delete-disabled-error + not-admin-error + reserved-name-rejection - 12 command tests: render dispatch (table/json/csv), empty-result paths, sort stability, JSON round-trip, CSV column order, confirmDestructive table-driven cases (y/yes/empty/n/NO/EOF/ whitespace) - End-to-end smoke against `arc serve` (with delete.enabled=true AND =false): every command + every error path verified. The new drop prompt smoked through 4 cases (empty=abort, n=abort, y=delete, --yes=no prompt) Internal review (matrix + single deep reviewer) addressed: - B1 Blocker: db drop missing y/N prompt + --yes flag → fixed (matches CLAUDE.md destructive-ops rule, mirrors config delete) - H1: db show JSON Count now mirrors server's list.Count (not re-derived from len(measurements)) — safer against future server-side pagination - M2 + M3: JSON output now sorts before encoding, so table/json/ csv agree on row order across all list commands - S2: new test pinning the setCommonHeaders default-fallback semantic so accidental cross-db drift gets caught - S3 + S4: README roadmap flipped, addTimeoutFlag factors out 5x duplicated --timeout flag definition - M1 deferred: db show partial-failure UX (fail-loud is defensible per matrix; will revisit if real-world friction surfaces) Related Arc issue - #471 — POST /api/v1/databases accepts any-valid-token; should require admin. Discovered while building this PR; filed separately. Companion docs PR lands in docs.basekick.net (docs/cli/{db, measurement}.md + index.md status table update). Co-Authored-By: Claude Opus 4.7 --- README.md | 32 ++- internal/client/client.go | 28 +- internal/client/client_test.go | 21 ++ internal/client/database.go | 223 +++++++++++++++ internal/client/database_test.go | 313 +++++++++++++++++++++ internal/commands/db.go | 449 +++++++++++++++++++++++++++++++ internal/commands/db_test.go | 273 +++++++++++++++++++ internal/commands/measurement.go | 156 +++++++++++ internal/commands/root.go | 7 +- 9 files changed, 1495 insertions(+), 7 deletions(-) create mode 100644 internal/client/database.go create mode 100644 internal/client/database_test.go create mode 100644 internal/commands/db.go create mode 100644 internal/commands/db_test.go create mode 100644 internal/commands/measurement.go diff --git a/README.md b/README.md index 3f6451a..49bb9f4 100644 --- a/README.md +++ b/README.md @@ -2,7 +2,7 @@ CLI for [Arc](https://github.com/Basekick-Labs/arc) — operator-facing client for Arc time-series databases. -> **Status:** v0.2.0-dev (PR2). Manages connection profiles, runs SQL queries, and writes line protocol with table / JSON / CSV / Arrow IPC output. `import` / `db` / `auth` / `cluster` subcommands ship in follow-up PRs. +> **Status:** v0.3.0-dev (PR3). Manages connection profiles, runs SQL queries, writes line protocol, and administers databases + measurements. `import` / `auth` / `cluster` subcommands ship in follow-up PRs. ## Why @@ -124,6 +124,34 @@ echo "cpu v=1 1700000000" | arcctl write --precision s `--precision` accepts `ns`, `us`, `ms`, or `s` (anything else is rejected client-side before the request goes out). The body is streamed end-to-end — `cat huge.lp | arcctl write` never buffers the whole payload in memory. +## Database & measurement admin + +```bash +# List every database the active token can see +arcctl db list + +# Inspect one database (info + its measurements) +arcctl db show production + +# Create an empty database (server validates name: alphanumeric + `_-`, +# max 64 chars, "system" / "internal" / "_internal" are reserved) +arcctl db create metrics + +# Drop a database and ALL its files. Prompts for y/N; pass --yes to +# skip in scripts. The server requires delete.enabled=true in arc.toml +# AND an admin token — if either is missing the server's error message +# surfaces verbatim ("Set delete.enabled=true in arc.toml to enable."). +arcctl db drop old_metrics +arcctl db drop --yes ci_scratch # no prompt + +# List measurements inside a database (same data shown by `db show`, +# different default view) +arcctl measurement list --database metrics +arcctl measurement list -c prod --database logs -o json +``` + +`db list`, `db show`, and `measurement list` all support `-o table|json|csv` (no `-o arrow` — these endpoints return JSON, not Arrow IPC). + ## TLS For HTTPS endpoints, certificate verification is on by default. To skip verification (lab / self-signed certs only), use either: @@ -139,7 +167,7 @@ This repo is being built in [phased PRs](https://github.com/Basekick-Labs/arcctl - ~~**PR1** — scaffold, `config` subcommand tree, multi-connection store~~ ✅ shipped - ~~**PR2** — `arcctl query`, `arcctl write`, output formats: table/json/csv/arrow~~ ✅ shipped -- **PR3** — `arcctl db {list,create,drop,show}`, `arcctl measurement list` +- ~~**PR3** — `arcctl db {list,show,create,drop}`, `arcctl measurement list`~~ ✅ shipped - **PR4** — `arcctl import {csv,lp,parquet,msgpack}` - **PR5** — `arcctl auth {token,whoami}` - **PR6** — `arcctl cluster {status,nodes}`, `arcctl compaction`, `arcctl retention` diff --git a/internal/client/client.go b/internal/client/client.go index 5a4a1fd..1ebe4a1 100644 --- a/internal/client/client.go +++ b/internal/client/client.go @@ -105,6 +105,15 @@ func New(cfg Config) (*Client, error) { // Useful for `-v` verbose output. func (c *Client) Endpoint() string { return c.cfg.Endpoint } +// DefaultDatabase returns the database the client falls back to when +// a per-call override is empty. May itself be empty (in which case +// Arc applies its server-side default, "default"). Exposed so the +// command layer can implement "use the connection's default DB" +// semantics for endpoints whose URL is database-scoped (e.g. +// /api/v1/databases/:name/measurements) where the server can't +// apply a fallback for us. +func (c *Client) DefaultDatabase() string { return c.cfg.Database } + // resolveDatabase picks the per-call override if set, else the // client's default. Both can be empty (Arc falls back to "default"). func (c *Client) resolveDatabase(override string) string { @@ -114,9 +123,9 @@ func (c *Client) resolveDatabase(override string) string { return c.cfg.Database } -// setCommonHeaders writes Authorization + (optional) x-arc-database -// onto a *http.Request. Used by every Do* call so the headers are set -// in exactly one place. +// setCommonHeaders writes Authorization, x-arc-database (per the +// resolveDatabase rules), and User-Agent onto a *http.Request. Use +// for per-database endpoints (query, write). func (c *Client) setCommonHeaders(req *http.Request, database string) { req.Header.Set("Authorization", "Bearer "+c.cfg.Token) if db := c.resolveDatabase(database); db != "" { @@ -124,3 +133,16 @@ func (c *Client) setCommonHeaders(req *http.Request, database string) { } req.Header.Set("User-Agent", "arcctl") } + +// setCrossDBHeaders writes Authorization + User-Agent but NEVER sends +// x-arc-database. Use for endpoints whose URL is itself the database +// selector (e.g. /api/v1/databases, /api/v1/databases/:name/...) or +// that are inherently cross-database (the list endpoints). +// +// Distinct from setCommonHeaders so the no-DB case is intentional, not +// accidental — calling setCommonHeaders(req, "") on a client with a +// non-empty default Database would silently include the wrong header. +func (c *Client) setCrossDBHeaders(req *http.Request) { + req.Header.Set("Authorization", "Bearer "+c.cfg.Token) + req.Header.Set("User-Agent", "arcctl") +} diff --git a/internal/client/client_test.go b/internal/client/client_test.go index 3873a5e..d6206ca 100644 --- a/internal/client/client_test.go +++ b/internal/client/client_test.go @@ -113,6 +113,27 @@ func TestQueryJSON_DatabaseOverride(t *testing.T) { } } +func TestQueryJSON_FallsBackToClientDefault(t *testing.T) { + // Pinning the contract: when per-call override is empty AND the + // client has a configured default DB, the request DOES send the + // x-arc-database header with that default. Cross-DB callers MUST + // use setCrossDBHeaders instead — see database.go. + var got string + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + got = r.Header.Get("x-arc-database") + _, _ = io.WriteString(w, `{"columns":[],"data":[],"row_count":0}`) + })) + defer srv.Close() + + c := freshClient(t, srv, "metrics") // client default = "metrics" + if _, err := c.QueryJSON(context.Background(), "SELECT 1", ""); err != nil { + t.Fatalf("QueryJSON: %v", err) + } + if got != "metrics" { + t.Errorf("expected x-arc-database=metrics (client default), got %q", got) + } +} + func TestQueryJSON_NoDatabaseHeaderWhenBothEmpty(t *testing.T) { // Empty client default + empty per-call -> header should be absent // so Arc applies its server-side default ("default"). diff --git a/internal/client/database.go b/internal/client/database.go new file mode 100644 index 0000000..ae97b96 --- /dev/null +++ b/internal/client/database.go @@ -0,0 +1,223 @@ +package client + +import ( + "bytes" + "context" + "encoding/json" + "fmt" + "io" + "net/http" + "net/url" +) + +// DatabaseInfo is one database in the response from /api/v1/databases. +// Mirrors the server struct in arc/internal/api/databases.go. +type DatabaseInfo struct { + Name string `json:"name"` + MeasurementCount int `json:"measurement_count"` + CreatedAt string `json:"created_at,omitempty"` +} + +// DatabaseListResponse is the response body of GET /api/v1/databases. +type DatabaseListResponse struct { + Databases []DatabaseInfo `json:"databases"` + Count int `json:"count"` +} + +// DatabaseMeasurement is one measurement inside a database. +type DatabaseMeasurement struct { + Name string `json:"name"` + FileCount int `json:"file_count,omitempty"` +} + +// MeasurementListResponse is the response body of +// GET /api/v1/databases/:name/measurements. +type MeasurementListResponse struct { + Database string `json:"database"` + Measurements []DatabaseMeasurement `json:"measurements"` + Count int `json:"count"` +} + +// createDatabaseRequest is the on-the-wire body shape for +// POST /api/v1/databases. +type createDatabaseRequest struct { + Name string `json:"name"` +} + +// ListDatabases returns every database the caller can see, with +// measurement counts pre-computed by the server. +func (c *Client) ListDatabases(ctx context.Context) (*DatabaseListResponse, error) { + req, err := http.NewRequestWithContext(ctx, http.MethodGet, c.cfg.Endpoint+"/api/v1/databases", nil) + if err != nil { + return nil, fmt.Errorf("build request: %w", err) + } + req.Header.Set("Accept", "application/json") + // Cross-database operation; the x-arc-database header is never + // applicable here, so we use setCrossDBHeaders to make the + // absence explicit. + c.setCrossDBHeaders(req) + + resp, err := c.http.Do(req) + if err != nil { + return nil, fmt.Errorf("list databases: %w", err) + } + defer resp.Body.Close() + + body, err := io.ReadAll(io.LimitReader(resp.Body, 16<<20)) + if err != nil { + return nil, fmt.Errorf("read response: %w", err) + } + if resp.StatusCode < 200 || resp.StatusCode >= 300 { + return nil, decodeWriteError(resp.StatusCode, body) + } + var out DatabaseListResponse + if err := json.Unmarshal(body, &out); err != nil { + return nil, fmt.Errorf("decode response: %w", err) + } + return &out, nil +} + +// GetDatabase returns metadata for one database. Returns a +// recognisable "not found" error for HTTP 404 so the command layer can +// distinguish "missing" from "broken." +func (c *Client) GetDatabase(ctx context.Context, name string) (*DatabaseInfo, error) { + if name == "" { + return nil, fmt.Errorf("database name is required") + } + u := c.cfg.Endpoint + "/api/v1/databases/" + url.PathEscape(name) + req, err := http.NewRequestWithContext(ctx, http.MethodGet, u, nil) + if err != nil { + return nil, fmt.Errorf("build request: %w", err) + } + req.Header.Set("Accept", "application/json") + c.setCrossDBHeaders(req) + + resp, err := c.http.Do(req) + if err != nil { + return nil, fmt.Errorf("get database: %w", err) + } + defer resp.Body.Close() + + body, err := io.ReadAll(io.LimitReader(resp.Body, 1<<20)) + if err != nil { + return nil, fmt.Errorf("read response: %w", err) + } + if resp.StatusCode < 200 || resp.StatusCode >= 300 { + return nil, decodeWriteError(resp.StatusCode, body) + } + var out DatabaseInfo + if err := json.Unmarshal(body, &out); err != nil { + return nil, fmt.Errorf("decode response: %w", err) + } + return &out, nil +} + +// CreateDatabase creates a new empty database. Returns the server's +// freshly-created metadata on HTTP 201. The server enforces name +// validation (alphanumeric + `_-`, ≤ 64 chars, not a reserved name like +// "system" / "internal"); arcctl forwards whatever the user typed and +// lets the server produce the canonical error. +func (c *Client) CreateDatabase(ctx context.Context, name string) (*DatabaseInfo, error) { + if name == "" { + return nil, fmt.Errorf("database name is required") + } + body, err := json.Marshal(createDatabaseRequest{Name: name}) + if err != nil { + return nil, fmt.Errorf("encode request: %w", err) + } + req, err := http.NewRequestWithContext(ctx, http.MethodPost, c.cfg.Endpoint+"/api/v1/databases", bytes.NewReader(body)) + if err != nil { + return nil, fmt.Errorf("build request: %w", err) + } + req.Header.Set("Content-Type", "application/json") + req.Header.Set("Accept", "application/json") + c.setCrossDBHeaders(req) + + resp, err := c.http.Do(req) + if err != nil { + return nil, fmt.Errorf("create database: %w", err) + } + defer resp.Body.Close() + + respBody, err := io.ReadAll(io.LimitReader(resp.Body, 1<<20)) + if err != nil { + return nil, fmt.Errorf("read response: %w", err) + } + if resp.StatusCode < 200 || resp.StatusCode >= 300 { + return nil, decodeWriteError(resp.StatusCode, respBody) + } + var out DatabaseInfo + if err := json.Unmarshal(respBody, &out); err != nil { + return nil, fmt.Errorf("decode response: %w", err) + } + return &out, nil +} + +// DeleteDatabase removes a database and ALL its data from the server. +// Always passes `?confirm=true` because the server requires it; the +// CLI's safety story is layered on top (the command refuses unless the +// user explicitly invokes `db drop`). +// +// The server may refuse with HTTP 403 if `delete.enabled` is false in +// arc.toml; the error message is surfaced verbatim so the operator +// knows to enable the flag server-side. +func (c *Client) DeleteDatabase(ctx context.Context, name string) error { + if name == "" { + return fmt.Errorf("database name is required") + } + u := c.cfg.Endpoint + "/api/v1/databases/" + url.PathEscape(name) + "?confirm=true" + req, err := http.NewRequestWithContext(ctx, http.MethodDelete, u, nil) + if err != nil { + return fmt.Errorf("build request: %w", err) + } + req.Header.Set("Accept", "application/json") + c.setCrossDBHeaders(req) + + resp, err := c.http.Do(req) + if err != nil { + return fmt.Errorf("delete database: %w", err) + } + defer resp.Body.Close() + + if resp.StatusCode >= 200 && resp.StatusCode < 300 { + // Drain any small body the server may include. + _, _ = io.Copy(io.Discard, io.LimitReader(resp.Body, 4<<10)) + return nil + } + body, _ := io.ReadAll(io.LimitReader(resp.Body, 64<<10)) + return decodeWriteError(resp.StatusCode, body) +} + +// ListMeasurements returns measurements inside a single database via +// GET /api/v1/databases/:name/measurements. +func (c *Client) ListMeasurements(ctx context.Context, database string) (*MeasurementListResponse, error) { + if database == "" { + return nil, fmt.Errorf("database name is required") + } + u := c.cfg.Endpoint + "/api/v1/databases/" + url.PathEscape(database) + "/measurements" + req, err := http.NewRequestWithContext(ctx, http.MethodGet, u, nil) + if err != nil { + return nil, fmt.Errorf("build request: %w", err) + } + req.Header.Set("Accept", "application/json") + c.setCrossDBHeaders(req) + + resp, err := c.http.Do(req) + if err != nil { + return nil, fmt.Errorf("list measurements: %w", err) + } + defer resp.Body.Close() + + body, err := io.ReadAll(io.LimitReader(resp.Body, 16<<20)) + if err != nil { + return nil, fmt.Errorf("read response: %w", err) + } + if resp.StatusCode < 200 || resp.StatusCode >= 300 { + return nil, decodeWriteError(resp.StatusCode, body) + } + var out MeasurementListResponse + if err := json.Unmarshal(body, &out); err != nil { + return nil, fmt.Errorf("decode response: %w", err) + } + return &out, nil +} diff --git a/internal/client/database_test.go b/internal/client/database_test.go new file mode 100644 index 0000000..5f5892b --- /dev/null +++ b/internal/client/database_test.go @@ -0,0 +1,313 @@ +package client + +import ( + "context" + "encoding/json" + "io" + "net/http" + "net/http/httptest" + "strings" + "testing" +) + +func TestListDatabases_Success(t *testing.T) { + var gotPath, gotMethod, gotAuth string + gotDB := "sentinel" // sentinel: should be overwritten to "" since this op is cross-db + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + gotPath, gotMethod, gotAuth = r.URL.Path, r.Method, r.Header.Get("Authorization") + gotDB = r.Header.Get("x-arc-database") + w.Header().Set("Content-Type", "application/json") + _, _ = io.WriteString(w, `{"databases":[{"name":"metrics","measurement_count":3,"created_at":"2026-05-31T12:00:00Z"},{"name":"logs","measurement_count":1}],"count":2}`) + })) + defer srv.Close() + + c := freshClient(t, srv, "metrics") // client default DB is "metrics" + list, err := c.ListDatabases(context.Background()) + if err != nil { + t.Fatalf("ListDatabases: %v", err) + } + if gotPath != "/api/v1/databases" { + t.Errorf("path = %q", gotPath) + } + if gotMethod != "GET" { + t.Errorf("method = %q", gotMethod) + } + if gotAuth != "Bearer test-token" { + t.Errorf("auth = %q", gotAuth) + } + // Cross-db op: x-arc-database header should NOT be sent even though + // the client has a default. + if gotDB != "" { + t.Errorf("x-arc-database = %q; expected empty for cross-db list", gotDB) + } + if list.Count != 2 || len(list.Databases) != 2 { + t.Errorf("decoded %+v", list) + } + if list.Databases[0].Name != "metrics" || list.Databases[0].MeasurementCount != 3 { + t.Errorf("first db = %+v", list.Databases[0]) + } + if list.Databases[1].CreatedAt != "" { + t.Errorf("second db should have omitempty CreatedAt, got %q", list.Databases[1].CreatedAt) + } +} + +func TestGetDatabase_Success(t *testing.T) { + var gotPath string + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + gotPath = r.URL.Path + _, _ = io.WriteString(w, `{"name":"metrics","measurement_count":7}`) + })) + defer srv.Close() + + c := freshClient(t, srv, "") + info, err := c.GetDatabase(context.Background(), "metrics") + if err != nil { + t.Fatalf("GetDatabase: %v", err) + } + if gotPath != "/api/v1/databases/metrics" { + t.Errorf("path = %q", gotPath) + } + if info.Name != "metrics" || info.MeasurementCount != 7 { + t.Errorf("info = %+v", info) + } +} + +func TestGetDatabase_NotFound(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusNotFound) + _, _ = io.WriteString(w, `{"error":"Database 'nope' not found"}`) + })) + defer srv.Close() + + c := freshClient(t, srv, "") + _, err := c.GetDatabase(context.Background(), "nope") + if err == nil { + t.Fatal("expected error, got nil") + } + if !strings.Contains(err.Error(), "not found") { + t.Errorf("error %q lacks server message", err) + } + if !strings.Contains(err.Error(), "HTTP 404") { + t.Errorf("error %q lacks status", err) + } +} + +func TestGetDatabase_EscapesName(t *testing.T) { + // Name with characters that require URL escaping. Server should + // receive the encoded form in the path. + var gotPath string + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + gotPath = r.URL.EscapedPath() + _, _ = io.WriteString(w, `{"name":"weird/name","measurement_count":0}`) + })) + defer srv.Close() + + c := freshClient(t, srv, "") + if _, err := c.GetDatabase(context.Background(), "weird/name"); err != nil { + t.Fatalf("GetDatabase: %v", err) + } + if gotPath != "/api/v1/databases/weird%2Fname" { + t.Errorf("path = %q; expected /api/v1/databases/weird%%2Fname", gotPath) + } +} + +func TestGetDatabase_EmptyName(t *testing.T) { + // Defensive client-side check: don't even send the request. + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + t.Fatal("server should not be called for empty name") + })) + defer srv.Close() + c := freshClient(t, srv, "") + if _, err := c.GetDatabase(context.Background(), ""); err == nil { + t.Fatal("expected error, got nil") + } +} + +func TestCreateDatabase_Success(t *testing.T) { + var ( + gotPath string + gotMethod string + gotCT string + gotBody []byte + ) + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + gotPath, gotMethod = r.URL.Path, r.Method + gotCT = r.Header.Get("Content-Type") + gotBody, _ = io.ReadAll(r.Body) + w.WriteHeader(http.StatusCreated) + _, _ = io.WriteString(w, `{"name":"new_db","measurement_count":0,"created_at":"2026-05-31T12:00:00Z"}`) + })) + defer srv.Close() + + c := freshClient(t, srv, "") + info, err := c.CreateDatabase(context.Background(), "new_db") + if err != nil { + t.Fatalf("CreateDatabase: %v", err) + } + if gotMethod != "POST" || gotPath != "/api/v1/databases" { + t.Errorf("path/method = %q / %q", gotPath, gotMethod) + } + if gotCT != "application/json" { + t.Errorf("content-type = %q", gotCT) + } + var sentReq createDatabaseRequest + if err := json.Unmarshal(gotBody, &sentReq); err != nil { + t.Fatalf("body not JSON: %v", err) + } + if sentReq.Name != "new_db" { + t.Errorf("sent name = %q", sentReq.Name) + } + if info.Name != "new_db" || info.CreatedAt == "" { + t.Errorf("decoded info = %+v", info) + } +} + +func TestCreateDatabase_Conflict(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusConflict) + _, _ = io.WriteString(w, `{"error":"Database 'metrics' already exists"}`) + })) + defer srv.Close() + + c := freshClient(t, srv, "") + _, err := c.CreateDatabase(context.Background(), "metrics") + if err == nil { + t.Fatal("expected error, got nil") + } + if !strings.Contains(err.Error(), "already exists") { + t.Errorf("error %q lacks server message", err) + } +} + +func TestCreateDatabase_ReservedName(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusBadRequest) + _, _ = io.WriteString(w, `{"error":"Database name 'system' is reserved"}`) + })) + defer srv.Close() + + c := freshClient(t, srv, "") + _, err := c.CreateDatabase(context.Background(), "system") + if err == nil { + t.Fatal("expected error, got nil") + } + if !strings.Contains(err.Error(), "reserved") { + t.Errorf("error %q lacks server message", err) + } +} + +func TestDeleteDatabase_Success(t *testing.T) { + var ( + gotPath string + gotMethod string + gotQuery string + ) + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + gotPath, gotMethod = r.URL.Path, r.Method + gotQuery = r.URL.RawQuery + w.WriteHeader(http.StatusNoContent) + })) + defer srv.Close() + + c := freshClient(t, srv, "") + if err := c.DeleteDatabase(context.Background(), "old_db"); err != nil { + t.Fatalf("DeleteDatabase: %v", err) + } + if gotMethod != "DELETE" { + t.Errorf("method = %q", gotMethod) + } + if gotPath != "/api/v1/databases/old_db" { + t.Errorf("path = %q", gotPath) + } + // confirm=true is required by the server; client always sends it. + if gotQuery != "confirm=true" { + t.Errorf("query = %q; expected confirm=true", gotQuery) + } +} + +func TestDeleteDatabase_DeleteDisabled(t *testing.T) { + // Server sends 403 with the explicit "enable in arc.toml" message + // when delete is gated. We surface the message verbatim so the + // operator knows what to do. + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusForbidden) + _, _ = io.WriteString(w, `{"error":"Delete operations are disabled. Set delete.enabled=true in arc.toml to enable."}`) + })) + defer srv.Close() + + c := freshClient(t, srv, "") + err := c.DeleteDatabase(context.Background(), "x") + if err == nil { + t.Fatal("expected error, got nil") + } + if !strings.Contains(err.Error(), "delete.enabled=true") { + t.Errorf("error %q lacks the server's enable hint", err) + } +} + +func TestDeleteDatabase_NotAdmin(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusForbidden) + _, _ = io.WriteString(w, `{"error":"Admin permission required"}`) + })) + defer srv.Close() + + c := freshClient(t, srv, "") + err := c.DeleteDatabase(context.Background(), "x") + if err == nil { + t.Fatal("expected error, got nil") + } + if !strings.Contains(err.Error(), "Admin permission required") { + t.Errorf("error %q lacks server message", err) + } +} + +func TestListMeasurements_Success(t *testing.T) { + var gotPath string + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + gotPath = r.URL.Path + _, _ = io.WriteString(w, `{"database":"metrics","measurements":[{"name":"cpu","file_count":12},{"name":"mem"}],"count":2}`) + })) + defer srv.Close() + + c := freshClient(t, srv, "") + list, err := c.ListMeasurements(context.Background(), "metrics") + if err != nil { + t.Fatalf("ListMeasurements: %v", err) + } + if gotPath != "/api/v1/databases/metrics/measurements" { + t.Errorf("path = %q", gotPath) + } + if list.Database != "metrics" || list.Count != 2 || len(list.Measurements) != 2 { + t.Errorf("decoded %+v", list) + } + if list.Measurements[0].FileCount != 12 || list.Measurements[1].FileCount != 0 { + t.Errorf("file counts off: %+v", list.Measurements) + } +} + +func TestListMeasurements_DatabaseNotFound(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusNotFound) + _, _ = io.WriteString(w, `{"error":"Database 'nope' not found"}`) + })) + defer srv.Close() + + c := freshClient(t, srv, "") + _, err := c.ListMeasurements(context.Background(), "nope") + if err == nil { + t.Fatal("expected error, got nil") + } + if !strings.Contains(err.Error(), "not found") { + t.Errorf("error %q lacks server message", err) + } +} + +func TestDefaultDatabase(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {})) + defer srv.Close() + c := freshClient(t, srv, "metrics") + if c.DefaultDatabase() != "metrics" { + t.Errorf("DefaultDatabase() = %q, want metrics", c.DefaultDatabase()) + } +} diff --git a/internal/commands/db.go b/internal/commands/db.go new file mode 100644 index 0000000..0bdc2de --- /dev/null +++ b/internal/commands/db.go @@ -0,0 +1,449 @@ +// db subcommand: manage databases on an Arc cluster. +// +// Maps directly onto Arc's /api/v1/databases endpoints (list / create +// / get / delete + per-database measurement listing). `drop` is a thin +// wrapper around the server-side DELETE which itself requires +// `delete.enabled=true` in arc.toml and admin auth — when those aren't +// met the server's verbatim error surfaces to the user. +package commands + +import ( + "bufio" + "context" + "encoding/csv" + "encoding/json" + "fmt" + "io" + "sort" + "strconv" + "strings" + "time" + + "github.com/spf13/cobra" + + "github.com/basekick-labs/arcctl/internal/client" + "github.com/basekick-labs/arcctl/internal/output" +) + +func newDBCmd() *cobra.Command { + c := &cobra.Command{ + Use: "db", + Short: "Manage databases on an Arc cluster", + Long: `Manage databases on an Arc cluster. + +Maps onto Arc's /api/v1/databases endpoints. "drop" requires the server +to have delete.enabled=true in arc.toml AND an admin token; the server's +error surfaces verbatim when those aren't met.`, + } + c.AddCommand( + newDBListCmd(), + newDBShowCmd(), + newDBCreateCmd(), + newDBDropCmd(), + ) + return c +} + +// ---- list ----------------------------------------------------------------- + +func newDBListCmd() *cobra.Command { + var ( + connectionName string + endpoint string + token string + insecure bool + outputFormat string + noHeader bool + timeout time.Duration + ) + c := &cobra.Command{ + Use: "list", + Short: "List databases on the active cluster", + Long: `List databases on the active cluster (GET /api/v1/databases). + +Output formats: + table (default) | json | csv + +Each row shows the database name, its measurement count, and (when set +by the server) the creation timestamp.`, + RunE: func(cmd *cobra.Command, args []string) error { + if timeout <= 0 { + return fmt.Errorf("--timeout must be > 0 (got %s)", timeout) + } + if !validListFormat(outputFormat) { + return fmt.Errorf("invalid --output %q (valid: table, json, csv)", outputFormat) + } + cli, _, err := buildClient(cmd.ErrOrStderr(), connectionName, endpoint, token, insecure, timeout) + if err != nil { + return err + } + ctx, cancel := context.WithTimeout(cmd.Context(), timeout) + defer cancel() + + list, err := cli.ListDatabases(ctx) + if err != nil { + return err + } + return renderDatabaseList(cmd, list, outputFormat, noHeader) + }, + } + addCommonConnectionFlags(c, &connectionName, &endpoint, &token, &insecure) + c.Flags().StringVarP(&outputFormat, "output", "o", output.FormatTable, "output format: table|json|csv") + c.Flags().BoolVar(&noHeader, "no-header", false, "suppress column header row (table + csv)") + addTimeoutFlag(c, &timeout) + return c +} + +// ---- show ----------------------------------------------------------------- + +func newDBShowCmd() *cobra.Command { + var ( + connectionName string + endpoint string + token string + insecure bool + outputFormat string + noHeader bool + timeout time.Duration + ) + c := &cobra.Command{ + Use: "show ", + Short: "Show one database plus its measurements", + Long: `Show one database plus the measurements it contains. + +Combines GET /api/v1/databases/:name with +GET /api/v1/databases/:name/measurements so the operator sees +"everything about this database" in one call. + +Output formats: + table (default — two stacked tables, db info then measurements) + json (single object: {"database": {...}, "measurements": [...]}) + csv (measurements only — db metadata is one row, not table-shaped)`, + Args: cobra.ExactArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + if timeout <= 0 { + return fmt.Errorf("--timeout must be > 0 (got %s)", timeout) + } + if !validListFormat(outputFormat) { + return fmt.Errorf("invalid --output %q (valid: table, json, csv)", outputFormat) + } + name := args[0] + cli, _, err := buildClient(cmd.ErrOrStderr(), connectionName, endpoint, token, insecure, timeout) + if err != nil { + return err + } + ctx, cancel := context.WithTimeout(cmd.Context(), timeout) + defer cancel() + + info, err := cli.GetDatabase(ctx, name) + if err != nil { + return err + } + measurements, err := cli.ListMeasurements(ctx, name) + if err != nil { + return err + } + return renderDatabaseShow(cmd, info, measurements, outputFormat, noHeader) + }, + } + addCommonConnectionFlags(c, &connectionName, &endpoint, &token, &insecure) + c.Flags().StringVarP(&outputFormat, "output", "o", output.FormatTable, "output format: table|json|csv") + c.Flags().BoolVar(&noHeader, "no-header", false, "suppress column headers (table + csv)") + addTimeoutFlag(c, &timeout) + return c +} + +// ---- create --------------------------------------------------------------- + +func newDBCreateCmd() *cobra.Command { + var ( + connectionName string + endpoint string + token string + insecure bool + timeout time.Duration + ) + c := &cobra.Command{ + Use: "create ", + Short: "Create a new empty database", + Long: `Create a new empty database (POST /api/v1/databases). + +Server-side validation: + - name must start with a letter and contain only alphanumeric, + underscore, or hyphen characters (max 64) + - names "system", "internal", "_internal" are reserved + - 409 Conflict if the database already exists`, + Args: cobra.ExactArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + if timeout <= 0 { + return fmt.Errorf("--timeout must be > 0 (got %s)", timeout) + } + name := args[0] + cli, _, err := buildClient(cmd.ErrOrStderr(), connectionName, endpoint, token, insecure, timeout) + if err != nil { + return err + } + ctx, cancel := context.WithTimeout(cmd.Context(), timeout) + defer cancel() + + info, err := cli.CreateDatabase(ctx, name) + if err != nil { + return err + } + if info.CreatedAt != "" { + fmt.Fprintf(cmd.OutOrStdout(), "Created database %q (created_at: %s)\n", info.Name, info.CreatedAt) + } else { + fmt.Fprintf(cmd.OutOrStdout(), "Created database %q\n", info.Name) + } + return nil + }, + } + addCommonConnectionFlags(c, &connectionName, &endpoint, &token, &insecure) + addTimeoutFlag(c, &timeout) + return c +} + +// ---- drop ----------------------------------------------------------------- + +func newDBDropCmd() *cobra.Command { + var ( + connectionName string + endpoint string + token string + insecure bool + yes bool + timeout time.Duration + ) + c := &cobra.Command{ + Use: "drop ", + Short: "Delete a database and ALL its data", + Long: `Delete a database and ALL its files (DELETE /api/v1/databases/:name). + +Destructive. Prompts for y/N confirmation by default; pass --yes (-y) +to skip the prompt (CI / scripted use). + +The server enforces its own layered safety: + - Requires delete.enabled=true in arc.toml (server returns 403 if not) + - Requires an admin-permission token + - Reserved names ("system", "internal", "_internal") are blocked + - Server enforces ?confirm=true on the request URL; arcctl always sends it + +When the server refuses, its error message surfaces verbatim — including +the "Set delete.enabled=true in arc.toml to enable" hint when the server +config gate is the reason.`, + Args: cobra.ExactArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + if timeout <= 0 { + return fmt.Errorf("--timeout must be > 0 (got %s)", timeout) + } + name := args[0] + if !yes { + if !confirmDestructive(cmd, fmt.Sprintf("Delete database %q and ALL its files?", name)) { + fmt.Fprintln(cmd.OutOrStdout(), "Aborted.") + return nil + } + } + cli, _, err := buildClient(cmd.ErrOrStderr(), connectionName, endpoint, token, insecure, timeout) + if err != nil { + return err + } + ctx, cancel := context.WithTimeout(cmd.Context(), timeout) + defer cancel() + + if err := cli.DeleteDatabase(ctx, name); err != nil { + return err + } + fmt.Fprintf(cmd.OutOrStdout(), "Deleted database %q\n", name) + return nil + }, + } + addCommonConnectionFlags(c, &connectionName, &endpoint, &token, &insecure) + c.Flags().BoolVarP(&yes, "yes", "y", false, "skip the confirmation prompt (destructive!)") + addTimeoutFlag(c, &timeout) + return c +} + +// confirmDestructive reads one line from cmd.InOrStdin() and returns +// true only if the user typed "y" or "yes" (case-insensitive). Reading +// from cmd.InOrStdin() (not os.Stdin) means tests can drive the prompt +// via cmd.SetIn(strings.NewReader("y\n")). +// +// Anything else — empty input, "n", EOF, read error — returns false. +// That's the safe default for a destructive prompt: when in doubt, +// don't delete. +func confirmDestructive(cmd *cobra.Command, question string) bool { + fmt.Fprintf(cmd.OutOrStdout(), "%s [y/N] ", question) + r := bufio.NewReader(cmd.InOrStdin()) + line, err := r.ReadString('\n') + if err != nil && line == "" { + return false + } + resp := strings.TrimSpace(line) + return strings.EqualFold(resp, "y") || strings.EqualFold(resp, "yes") +} + +// ---- helpers -------------------------------------------------------------- + +// addCommonConnectionFlags registers the shared `-c/--connection`, +// `--endpoint`, `--token`, `--insecure` flags on a cobra command. These +// behave identically to the same flags on `query` and `write`. +func addCommonConnectionFlags(c *cobra.Command, connectionName, endpoint, token *string, insecure *bool) { + c.Flags().StringVarP(connectionName, "connection", "c", "", "named connection (overrides active)") + c.Flags().StringVar(endpoint, "endpoint", "", "ad-hoc Arc endpoint URL") + c.Flags().StringVar(token, "token", "", "ad-hoc bearer token") + c.Flags().BoolVar(insecure, "insecure", false, "skip TLS certificate verification (warns to stderr)") +} + +// addTimeoutFlag registers the per-request HTTP `--timeout` flag with +// the project-wide default (60s). Factored so the flag's wording and +// default stay consistent across every command. +func addTimeoutFlag(c *cobra.Command, timeout *time.Duration) { + c.Flags().DurationVar(timeout, "timeout", 60*time.Second, "per-request HTTP timeout") +} + +// validListFormat reports whether the format is one of the three +// supported by list / show commands (table, json, csv — no arrow, since +// these endpoints return JSON not Arrow IPC). +func validListFormat(s string) bool { + switch s { + case output.FormatTable, output.FormatJSON, output.FormatCSV: + return true + } + return false +} + +// renderDatabaseList writes the list-databases response in the chosen +// format. Databases are sorted by name so screenshots / tests are +// stable regardless of server iteration order. JSON sorts too so the +// three formats agree on row order. +func renderDatabaseList(cmd *cobra.Command, list *client.DatabaseListResponse, format string, noHeader bool) error { + // Defensive copy: we mustn't mutate the caller's slice when we sort, + // since the same response object may be reused (e.g. in tests). + dbs := append([]client.DatabaseInfo(nil), list.Databases...) + sort.Slice(dbs, func(i, j int) bool { return dbs[i].Name < dbs[j].Name }) + + switch format { + case output.FormatJSON: + enc := json.NewEncoder(cmd.OutOrStdout()) + enc.SetIndent("", " ") + // Re-wrap with the sorted slice so JSON output matches + // table+CSV row order. Preserve the server-reported Count + // rather than re-deriving it from len(dbs) so any future + // server-side pagination surface still round-trips correctly. + return enc.Encode(client.DatabaseListResponse{Databases: dbs, Count: list.Count}) + case output.FormatCSV: + return writeDBListCSV(cmd.OutOrStdout(), dbs, noHeader) + default: // FormatTable + if len(dbs) == 0 { + _, err := fmt.Fprintln(cmd.OutOrStdout(), "(no databases)") + return err + } + rows := make([][]string, 0, len(dbs)) + for _, d := range dbs { + rows = append(rows, []string{ + d.Name, + strconv.Itoa(d.MeasurementCount), + d.CreatedAt, + }) + } + headers := []string{"NAME", "MEASUREMENTS", "CREATED_AT"} + if noHeader { + headers = nil + } + return output.Table(cmd.OutOrStdout(), headers, rows) + } +} + +func writeDBListCSV(w io.Writer, dbs []client.DatabaseInfo, noHeader bool) error { + cw := csv.NewWriter(w) + if !noHeader { + if err := cw.Write([]string{"name", "measurement_count", "created_at"}); err != nil { + return err + } + } + for _, d := range dbs { + if err := cw.Write([]string{d.Name, strconv.Itoa(d.MeasurementCount), d.CreatedAt}); err != nil { + return err + } + } + cw.Flush() + return cw.Error() +} + +// renderDatabaseShow combines the database info row with its +// measurements list. JSON output is a single composed object; table +// output stacks two visual blocks; CSV emits only measurements (the +// db-info block isn't tabular-shaped). +func renderDatabaseShow(cmd *cobra.Command, info *client.DatabaseInfo, list *client.MeasurementListResponse, format string, noHeader bool) error { + measurements := append([]client.DatabaseMeasurement(nil), list.Measurements...) + sort.Slice(measurements, func(i, j int) bool { return measurements[i].Name < measurements[j].Name }) + + switch format { + case output.FormatJSON: + // Compose db info + measurements into one object so a single + // JSON read covers the whole view. `Count` mirrors the server's + // reported value (list.Count) rather than len(measurements), + // matching MeasurementListResponse semantics. + composed := struct { + Database *client.DatabaseInfo `json:"database"` + Measurements []client.DatabaseMeasurement `json:"measurements"` + Count int `json:"count"` + }{ + Database: info, + Measurements: measurements, + Count: list.Count, + } + enc := json.NewEncoder(cmd.OutOrStdout()) + enc.SetIndent("", " ") + return enc.Encode(composed) + + case output.FormatCSV: + return writeMeasurementsCSV(cmd.OutOrStdout(), measurements, noHeader) + + default: // FormatTable + w := cmd.OutOrStdout() + fmt.Fprintf(w, "Database: %s\n", info.Name) + fmt.Fprintf(w, "Measurements: %d\n", info.MeasurementCount) + if info.CreatedAt != "" { + fmt.Fprintf(w, "Created at: %s\n", info.CreatedAt) + } + fmt.Fprintln(w) + if len(measurements) == 0 { + fmt.Fprintln(w, "(no measurements yet)") + return nil + } + rows := make([][]string, 0, len(measurements)) + for _, m := range measurements { + fc := "" + if m.FileCount > 0 { + fc = strconv.Itoa(m.FileCount) + } + rows = append(rows, []string{m.Name, fc}) + } + headers := []string{"MEASUREMENT", "FILES"} + if noHeader { + headers = nil + } + return output.Table(w, headers, rows) + } +} + +func writeMeasurementsCSV(w io.Writer, ms []client.DatabaseMeasurement, noHeader bool) error { + cw := csv.NewWriter(w) + if !noHeader { + if err := cw.Write([]string{"name", "file_count"}); err != nil { + return err + } + } + for _, m := range ms { + fc := "" + if m.FileCount > 0 { + fc = strconv.Itoa(m.FileCount) + } + if err := cw.Write([]string{m.Name, fc}); err != nil { + return err + } + } + cw.Flush() + return cw.Error() +} diff --git a/internal/commands/db_test.go b/internal/commands/db_test.go new file mode 100644 index 0000000..0a3f90e --- /dev/null +++ b/internal/commands/db_test.go @@ -0,0 +1,273 @@ +package commands + +import ( + "bytes" + "encoding/csv" + "encoding/json" + "io" + "strings" + "testing" + + "github.com/spf13/cobra" + + "github.com/basekick-labs/arcctl/internal/client" +) + +func TestValidListFormat(t *testing.T) { + for _, f := range []string{"table", "json", "csv"} { + if !validListFormat(f) { + t.Errorf("validListFormat(%q) = false", f) + } + } + for _, f := range []string{"arrow", "", "yaml", "ARROW"} { + if validListFormat(f) { + t.Errorf("validListFormat(%q) = true (should be false — arrow not valid for list endpoints)", f) + } + } +} + +func newTestCmd() *cobra.Command { + // Bare cobra.Command is enough — renderDatabaseList only uses + // cmd.OutOrStdout() which works on a default-constructed command. + return &cobra.Command{Use: "test"} +} + +func TestRenderDatabaseList_TableEmpty(t *testing.T) { + // Server returned databases:[] / count:0. Must print SOMETHING. + var buf bytes.Buffer + cmd := newTestCmd() + cmd.SetOut(&buf) + list := &client.DatabaseListResponse{Databases: nil, Count: 0} + if err := renderDatabaseList(cmd, list, "table", false); err != nil { + t.Fatalf("render: %v", err) + } + if !strings.Contains(buf.String(), "(no databases)") { + t.Errorf("empty render = %q, want '(no databases)'", buf.String()) + } +} + +func TestRenderDatabaseList_TableSortsByName(t *testing.T) { + var buf bytes.Buffer + cmd := newTestCmd() + cmd.SetOut(&buf) + list := &client.DatabaseListResponse{ + Databases: []client.DatabaseInfo{ + {Name: "zeta", MeasurementCount: 1}, + {Name: "alpha", MeasurementCount: 3, CreatedAt: "2026-01-01"}, + {Name: "mike", MeasurementCount: 2}, + }, + Count: 3, + } + if err := renderDatabaseList(cmd, list, "table", false); err != nil { + t.Fatalf("render: %v", err) + } + out := buf.String() + // Confirm alpha appears before mike before zeta in the output. + if !(strings.Index(out, "alpha") < strings.Index(out, "mike") && strings.Index(out, "mike") < strings.Index(out, "zeta")) { + t.Errorf("rows not sorted by name:\n%s", out) + } +} + +func TestRenderDatabaseList_JSON_RoundTrips(t *testing.T) { + var buf bytes.Buffer + cmd := newTestCmd() + cmd.SetOut(&buf) + src := &client.DatabaseListResponse{ + Databases: []client.DatabaseInfo{{Name: "m", MeasurementCount: 1}}, + Count: 1, + } + if err := renderDatabaseList(cmd, src, "json", false); err != nil { + t.Fatalf("render: %v", err) + } + var decoded client.DatabaseListResponse + if err := json.Unmarshal(buf.Bytes(), &decoded); err != nil { + t.Fatalf("output not valid JSON: %v\n%s", err, buf.String()) + } + if decoded.Count != 1 || decoded.Databases[0].Name != "m" { + t.Errorf("decoded = %+v", decoded) + } +} + +func TestRenderDatabaseList_CSV_ShapeAndOrdering(t *testing.T) { + var buf bytes.Buffer + cmd := newTestCmd() + cmd.SetOut(&buf) + src := &client.DatabaseListResponse{ + Databases: []client.DatabaseInfo{ + {Name: "b", MeasurementCount: 2, CreatedAt: "2026-02"}, + {Name: "a", MeasurementCount: 1}, + }, + Count: 2, + } + if err := renderDatabaseList(cmd, src, "csv", false); err != nil { + t.Fatalf("render: %v", err) + } + rows, err := csv.NewReader(&buf).ReadAll() + if err != nil { + t.Fatalf("invalid CSV: %v", err) + } + // header + 2 rows; sorted alphabetically so "a" comes first + if len(rows) != 3 { + t.Fatalf("got %d rows, want 3", len(rows)) + } + if rows[0][0] != "name" { + t.Errorf("header row = %v", rows[0]) + } + if rows[1][0] != "a" || rows[2][0] != "b" { + t.Errorf("rows not alphabetical: %v / %v", rows[1], rows[2]) + } + // "a" has no CreatedAt → empty cell + if rows[1][2] != "" { + t.Errorf("expected empty CreatedAt for 'a', got %q", rows[1][2]) + } +} + +func TestRenderDatabaseShow_TableEmptyMeasurements(t *testing.T) { + var buf bytes.Buffer + cmd := newTestCmd() + cmd.SetOut(&buf) + info := &client.DatabaseInfo{Name: "empty_db", MeasurementCount: 0} + list := &client.MeasurementListResponse{Database: "empty_db", Measurements: nil, Count: 0} + if err := renderDatabaseShow(cmd, info, list, "table", false); err != nil { + t.Fatalf("render: %v", err) + } + out := buf.String() + if !strings.Contains(out, "Database: empty_db") { + t.Errorf("missing header: %s", out) + } + if !strings.Contains(out, "(no measurements yet)") { + t.Errorf("missing empty hint: %s", out) + } +} + +func TestRenderDatabaseShow_JSONComposesBothPayloads(t *testing.T) { + var buf bytes.Buffer + cmd := newTestCmd() + cmd.SetOut(&buf) + info := &client.DatabaseInfo{Name: "metrics", MeasurementCount: 2} + list := &client.MeasurementListResponse{ + Database: "metrics", + Measurements: []client.DatabaseMeasurement{{Name: "cpu"}, {Name: "mem", FileCount: 5}}, + Count: 2, + } + if err := renderDatabaseShow(cmd, info, list, "json", false); err != nil { + t.Fatalf("render: %v", err) + } + var got struct { + Database client.DatabaseInfo `json:"database"` + Measurements []client.DatabaseMeasurement `json:"measurements"` + Count int `json:"count"` + } + if err := json.Unmarshal(buf.Bytes(), &got); err != nil { + t.Fatalf("invalid JSON: %v\n%s", err, buf.String()) + } + if got.Database.Name != "metrics" { + t.Errorf("db.name = %q", got.Database.Name) + } + if got.Count != 2 || len(got.Measurements) != 2 { + t.Errorf("decoded measurements = %+v", got) + } +} + +func TestRenderMeasurementList_CSVIncludesDatabaseColumn(t *testing.T) { + var buf bytes.Buffer + cmd := newTestCmd() + cmd.SetOut(&buf) + list := &client.MeasurementListResponse{ + Database: "metrics", + Measurements: []client.DatabaseMeasurement{{Name: "cpu", FileCount: 3}, {Name: "mem"}}, + Count: 2, + } + if err := renderMeasurementList(cmd, list, "csv", false); err != nil { + t.Fatalf("render: %v", err) + } + rows, err := csv.NewReader(&buf).ReadAll() + if err != nil { + t.Fatalf("invalid CSV: %v", err) + } + if len(rows) != 3 { + t.Fatalf("got %d rows, want 3", len(rows)) + } + // Header row should be database,measurement,file_count + if rows[0][0] != "database" || rows[0][1] != "measurement" || rows[0][2] != "file_count" { + t.Errorf("header = %v", rows[0]) + } + // Every data row should have the database value in col 0. + if rows[1][0] != "metrics" || rows[2][0] != "metrics" { + t.Errorf("database column missing in data rows: %v / %v", rows[1], rows[2]) + } + // FileCount=0 should render as empty (matches the omitempty wire convention). + if rows[2][2] != "" { + t.Errorf("expected empty FileCount for 'mem', got %q", rows[2][2]) + } +} + +func TestRenderMeasurementList_TableEmpty(t *testing.T) { + var buf bytes.Buffer + cmd := newTestCmd() + cmd.SetOut(&buf) + list := &client.MeasurementListResponse{Database: "empty_db", Measurements: nil} + if err := renderMeasurementList(cmd, list, "table", false); err != nil { + t.Fatalf("render: %v", err) + } + if !strings.Contains(buf.String(), "(no measurements in database \"empty_db\")") { + t.Errorf("empty render = %q", buf.String()) + } +} + +// confirmDestructive tests — the prompt is the only client-side gate +// on db drop, so it has to be testable without spinning up an arc. + +func TestConfirmDestructive_AcceptsY(t *testing.T) { + cmd := newTestCmd() + var out bytes.Buffer + cmd.SetOut(&out) + cmd.SetIn(strings.NewReader("y\n")) + if !confirmDestructive(cmd, "Delete X?") { + t.Error("y should accept") + } + if !strings.Contains(out.String(), "Delete X? [y/N]") { + t.Errorf("prompt missing: %q", out.String()) + } +} + +func TestConfirmDestructive_AcceptsYes(t *testing.T) { + cmd := newTestCmd() + cmd.SetIn(strings.NewReader("YES\n")) + cmd.SetOut(io.Discard) + if !confirmDestructive(cmd, "Delete X?") { + t.Error("YES should accept (case-insensitive)") + } +} + +func TestConfirmDestructive_DefaultsNo(t *testing.T) { + cases := []struct { + name string + input string + }{ + {"empty newline", "\n"}, + {"n", "n\n"}, + {"NO", "NO\n"}, + {"random", "delete it\n"}, + {"EOF no newline", ""}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + cmd := newTestCmd() + cmd.SetIn(strings.NewReader(tc.input)) + cmd.SetOut(io.Discard) + if confirmDestructive(cmd, "Delete X?") { + t.Errorf("input %q should NOT confirm", tc.input) + } + }) + } +} + +func TestConfirmDestructive_TrimsWhitespace(t *testing.T) { + cmd := newTestCmd() + cmd.SetIn(strings.NewReader(" y \n")) + cmd.SetOut(io.Discard) + if !confirmDestructive(cmd, "Delete X?") { + t.Error("' y ' (whitespace-padded) should accept") + } +} diff --git a/internal/commands/measurement.go b/internal/commands/measurement.go new file mode 100644 index 0000000..8be0f85 --- /dev/null +++ b/internal/commands/measurement.go @@ -0,0 +1,156 @@ +// measurement subcommand: list measurements inside a database. +// +// Thin wrapper over GET /api/v1/databases/:name/measurements. The same +// data is also available via `arcctl db show `; this exposes it +// in a measurement-first flow that mirrors `kubectl get pods -n ns` +// rather than `kubectl describe namespace`. +package commands + +import ( + "context" + "encoding/csv" + "encoding/json" + "fmt" + "io" + "sort" + "strconv" + "time" + + "github.com/spf13/cobra" + + "github.com/basekick-labs/arcctl/internal/client" + "github.com/basekick-labs/arcctl/internal/output" +) + +func newMeasurementCmd() *cobra.Command { + c := &cobra.Command{ + Use: "measurement", + Short: "Inspect measurements within a database", + } + c.AddCommand(newMeasurementListCmd()) + return c +} + +func newMeasurementListCmd() *cobra.Command { + var ( + connectionName string + endpoint string + token string + insecure bool + database string + outputFormat string + noHeader bool + timeout time.Duration + ) + c := &cobra.Command{ + Use: "list", + Short: "List measurements inside a database", + Long: `List measurements inside a database (GET /api/v1/databases/:name/measurements). + +The database name comes from --database, or (when --database is omitted) +from the active connection's default_database. If neither is set the +command errors before any network call.`, + Example: ` arcctl measurement list --database metrics + arcctl measurement list -c prod --database logs -o json`, + RunE: func(cmd *cobra.Command, args []string) error { + if timeout <= 0 { + return fmt.Errorf("--timeout must be > 0 (got %s)", timeout) + } + if !validListFormat(outputFormat) { + return fmt.Errorf("invalid --output %q (valid: table, json, csv)", outputFormat) + } + cli, _, err := buildClient(cmd.ErrOrStderr(), connectionName, endpoint, token, insecure, timeout) + if err != nil { + return err + } + // Per-call --database overrides the client default; if both + // are empty, the server would 400 on a missing :name path + // segment, but it's friendlier to catch it client-side. + db := database + if db == "" { + db = cli.DefaultDatabase() + } + if db == "" { + return fmt.Errorf("no database specified (pass --database or set default_database on the active connection)") + } + ctx, cancel := context.WithTimeout(cmd.Context(), timeout) + defer cancel() + + list, err := cli.ListMeasurements(ctx, db) + if err != nil { + return err + } + return renderMeasurementList(cmd, list, outputFormat, noHeader) + }, + } + addCommonConnectionFlags(c, &connectionName, &endpoint, &token, &insecure) + c.Flags().StringVar(&database, "database", "", "database to list measurements from (defaults to connection's default_database)") + c.Flags().StringVarP(&outputFormat, "output", "o", output.FormatTable, "output format: table|json|csv") + c.Flags().BoolVar(&noHeader, "no-header", false, "suppress column header row (table + csv)") + addTimeoutFlag(c, &timeout) + return c +} + +func renderMeasurementList(cmd *cobra.Command, list *client.MeasurementListResponse, format string, noHeader bool) error { + // Defensive copy + sort: same rationale as renderDatabaseList — + // keep table/csv/json row order consistent, don't mutate the caller's + // slice. + ms := append([]client.DatabaseMeasurement(nil), list.Measurements...) + sort.Slice(ms, func(i, j int) bool { return ms[i].Name < ms[j].Name }) + + switch format { + case output.FormatJSON: + enc := json.NewEncoder(cmd.OutOrStdout()) + enc.SetIndent("", " ") + // Encode with the sorted slice so JSON output matches table/CSV + // order. Preserve list.Count and list.Database from the server. + return enc.Encode(client.MeasurementListResponse{ + Database: list.Database, + Measurements: ms, + Count: list.Count, + }) + + case output.FormatCSV: + return writeMeasurementListCSV(cmd.OutOrStdout(), list.Database, ms, noHeader) + + default: // FormatTable + w := cmd.OutOrStdout() + if len(ms) == 0 { + fmt.Fprintf(w, "(no measurements in database %q)\n", list.Database) + return nil + } + rows := make([][]string, 0, len(ms)) + for _, m := range ms { + fc := "" + if m.FileCount > 0 { + fc = strconv.Itoa(m.FileCount) + } + rows = append(rows, []string{m.Name, fc}) + } + headers := []string{"MEASUREMENT", "FILES"} + if noHeader { + headers = nil + } + return output.Table(w, headers, rows) + } +} + +func writeMeasurementListCSV(w io.Writer, db string, ms []client.DatabaseMeasurement, noHeader bool) error { + cw := csv.NewWriter(w) + if !noHeader { + if err := cw.Write([]string{"database", "measurement", "file_count"}); err != nil { + return err + } + } + for _, m := range ms { + fc := "" + if m.FileCount > 0 { + fc = strconv.Itoa(m.FileCount) + } + if err := cw.Write([]string{db, m.Name, fc}); err != nil { + return err + } + } + cw.Flush() + return cw.Error() +} diff --git a/internal/commands/root.go b/internal/commands/root.go index 0b699c2..d72fff2 100644 --- a/internal/commands/root.go +++ b/internal/commands/root.go @@ -1,7 +1,8 @@ // Package commands wires the arcctl cobra command tree. // -// Each top-level command lives in its own file. PR1 ships `root` + -// `config`. Later PRs add: query, write, import, db, auth, cluster, ops. +// Each top-level command lives in its own file. PR1 shipped `root` + +// `config`; PR2 added `query` + `write`; PR3 added `db` + `measurement`. +// Later PRs add: import, auth, cluster, ops. package commands import "github.com/spf13/cobra" @@ -32,6 +33,8 @@ First-time setup: newConfigCmd(), newQueryCmd(), newWriteCmd(), + newDBCmd(), + newMeasurementCmd(), ) return root } From a77d5144d1727003ba55d48baa71e1e9e3b567d3 Mon Sep 17 00:00:00 2001 From: Ignacio Van Droogenbroeck Date: Sun, 31 May 2026 20:55:49 -0300 Subject: [PATCH 2/3] fix(db): build client before destructive prompt (Gemini PR #2) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Move the buildClient call ahead of the y/N confirmation in `db drop` so a misconfigured connection fails fast instead of making the user confirm a delete that can't be sent. No functional change beyond ordering — same code, two blocks swapped + a four-line "why" comment. Found by Gemini in https://github.com/Basekick-Labs/arcctl/pull/2 inline comment on internal/commands/db.go:240. The original matrix + deep-reviewer pass assumed a valid client config and never exercised the prompt-then-fail UX path. Tests + lint clean; existing confirmDestructive tests still pass since they exercise the prompt mechanics in isolation, not the wrapping order. --- internal/commands/db.go | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/internal/commands/db.go b/internal/commands/db.go index 0bdc2de..248aa84 100644 --- a/internal/commands/db.go +++ b/internal/commands/db.go @@ -237,16 +237,20 @@ config gate is the reason.`, return fmt.Errorf("--timeout must be > 0 (got %s)", timeout) } name := args[0] + // Build the client BEFORE the confirmation prompt so a + // misconfigured connection fails fast instead of making + // the user say "yes" to a delete that can't be sent. + // (Gemini PR #2 finding — better UX, no functional change.) + cli, _, err := buildClient(cmd.ErrOrStderr(), connectionName, endpoint, token, insecure, timeout) + if err != nil { + return err + } if !yes { if !confirmDestructive(cmd, fmt.Sprintf("Delete database %q and ALL its files?", name)) { fmt.Fprintln(cmd.OutOrStdout(), "Aborted.") return nil } } - cli, _, err := buildClient(cmd.ErrOrStderr(), connectionName, endpoint, token, insecure, timeout) - if err != nil { - return err - } ctx, cancel := context.WithTimeout(cmd.Context(), timeout) defer cancel() From 6a1cf41aaac31df2d5c6bd91793d89688b305f85 Mon Sep 17 00:00:00 2001 From: Ignacio Van Droogenbroeck Date: Sun, 31 May 2026 21:01:13 -0300 Subject: [PATCH 3/3] =?UTF-8?q?fix(db,measurement):=20empty=20server=20res?= =?UTF-8?q?ponse=20=E2=86=92=20`[]`=20not=20`null`=20(Gemini=20PR=20#2)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Switch from `append([]T(nil), src...)` to `make([]T, len(src)); copy()` in the three render-paths that emit JSON. The append-to-nil form preserves nil when src is empty/nil; the make form always yields a non-nil slice. The difference is invisible for table/csv output but matters for `-o json` consumers (jq, frontends, JSON-schema validators with `minItems: 0`) that distinguish `"databases": null` from `"databases": []`. Fixed sites - internal/commands/db.go:323-329 renderDatabaseList - internal/commands/db.go:385-389 renderDatabaseShow - internal/commands/measurement.go:95-101 renderMeasurementList Regression tests - TestRenderDatabaseList_JSONEmpty_IsArrayNotNull - TestRenderDatabaseShow_JSONEmpty_IsArrayNotNull - TestRenderMeasurementList_JSONEmpty_IsArrayNotNull Tests verified via mutation: reverting one site to append-to-nil makes the corresponding test fail with the exact `"databases": null` output Gemini flagged; reapplying the fix passes. Found by Gemini in https://github.com/Basekick-Labs/arcctl/pull/2 on commit a77d514 (three inline comments at db.go:326, db.go:382, measurement.go:98). The earlier matrix exercised empty-result paths for table/csv output but didn't check the JSON shape — adding a "nil-vs-empty slice JSON encoding" axis to the destructive-ops matrix template for future PRs. --- internal/commands/db.go | 16 ++++++--- internal/commands/db_test.go | 61 ++++++++++++++++++++++++++++++++ internal/commands/measurement.go | 10 ++++-- 3 files changed, 80 insertions(+), 7 deletions(-) diff --git a/internal/commands/db.go b/internal/commands/db.go index 248aa84..46d5f9a 100644 --- a/internal/commands/db.go +++ b/internal/commands/db.go @@ -321,9 +321,13 @@ func validListFormat(s string) bool { // stable regardless of server iteration order. JSON sorts too so the // three formats agree on row order. func renderDatabaseList(cmd *cobra.Command, list *client.DatabaseListResponse, format string, noHeader bool) error { - // Defensive copy: we mustn't mutate the caller's slice when we sort, - // since the same response object may be reused (e.g. in tests). - dbs := append([]client.DatabaseInfo(nil), list.Databases...) + // Defensive copy: we mustn't mutate the caller's slice when we + // sort. Use make+copy (not append-to-nil) so that an empty + // list.Databases produces an empty-but-non-nil slice; the + // difference matters for JSON output where nil encodes as + // `null` and []T{} encodes as `[]`. (Gemini PR #2 finding.) + dbs := make([]client.DatabaseInfo, len(list.Databases)) + copy(dbs, list.Databases) sort.Slice(dbs, func(i, j int) bool { return dbs[i].Name < dbs[j].Name }) switch format { @@ -379,7 +383,11 @@ func writeDBListCSV(w io.Writer, dbs []client.DatabaseInfo, noHeader bool) error // output stacks two visual blocks; CSV emits only measurements (the // db-info block isn't tabular-shaped). func renderDatabaseShow(cmd *cobra.Command, info *client.DatabaseInfo, list *client.MeasurementListResponse, format string, noHeader bool) error { - measurements := append([]client.DatabaseMeasurement(nil), list.Measurements...) + // make+copy (not append-to-nil) so an empty server response + // encodes as `"measurements": []` rather than `"measurements": + // null` in JSON output. (Gemini PR #2 finding.) + measurements := make([]client.DatabaseMeasurement, len(list.Measurements)) + copy(measurements, list.Measurements) sort.Slice(measurements, func(i, j int) bool { return measurements[i].Name < measurements[j].Name }) switch format { diff --git a/internal/commands/db_test.go b/internal/commands/db_test.go index 0a3f90e..402c3e5 100644 --- a/internal/commands/db_test.go +++ b/internal/commands/db_test.go @@ -46,6 +46,67 @@ func TestRenderDatabaseList_TableEmpty(t *testing.T) { } } +// Regression for Gemini PR #2 finding: when the server returns no +// databases, JSON output MUST be `"databases": []`, not `"databases": +// null`. Many JSON consumers (jq filters, JSON-schema validators with +// `minItems: 0`, frontends doing `.map()`) treat null and [] as +// semantically distinct — null is "unknown/missing", [] is "known +// empty." Catching this in a test pins the make+copy idiom inside +// renderDatabaseList so a future refactor back to append-to-nil +// fails CI loudly. +func TestRenderDatabaseList_JSONEmpty_IsArrayNotNull(t *testing.T) { + var buf bytes.Buffer + cmd := newTestCmd() + cmd.SetOut(&buf) + list := &client.DatabaseListResponse{Databases: nil, Count: 0} + if err := renderDatabaseList(cmd, list, "json", false); err != nil { + t.Fatalf("render: %v", err) + } + out := buf.String() + if strings.Contains(out, "null") { + t.Errorf("JSON output contains `null` — empty slice must encode as `[]`. Got:\n%s", out) + } + // Positive form: confirm the bracketed-empty shape is actually present. + if !strings.Contains(out, `"databases": []`) { + t.Errorf("expected `\"databases\": []` in output, got:\n%s", out) + } +} + +func TestRenderDatabaseShow_JSONEmpty_IsArrayNotNull(t *testing.T) { + var buf bytes.Buffer + cmd := newTestCmd() + cmd.SetOut(&buf) + info := &client.DatabaseInfo{Name: "empty_db", MeasurementCount: 0} + list := &client.MeasurementListResponse{Database: "empty_db", Measurements: nil, Count: 0} + if err := renderDatabaseShow(cmd, info, list, "json", false); err != nil { + t.Fatalf("render: %v", err) + } + out := buf.String() + if strings.Contains(out, "null") { + t.Errorf("JSON output contains `null` — empty measurements slice must encode as `[]`. Got:\n%s", out) + } + if !strings.Contains(out, `"measurements": []`) { + t.Errorf("expected `\"measurements\": []` in output, got:\n%s", out) + } +} + +func TestRenderMeasurementList_JSONEmpty_IsArrayNotNull(t *testing.T) { + var buf bytes.Buffer + cmd := newTestCmd() + cmd.SetOut(&buf) + list := &client.MeasurementListResponse{Database: "metrics", Measurements: nil, Count: 0} + if err := renderMeasurementList(cmd, list, "json", false); err != nil { + t.Fatalf("render: %v", err) + } + out := buf.String() + if strings.Contains(out, "null") { + t.Errorf("JSON output contains `null` — empty measurements slice must encode as `[]`. Got:\n%s", out) + } + if !strings.Contains(out, `"measurements": []`) { + t.Errorf("expected `\"measurements\": []` in output, got:\n%s", out) + } +} + func TestRenderDatabaseList_TableSortsByName(t *testing.T) { var buf bytes.Buffer cmd := newTestCmd() diff --git a/internal/commands/measurement.go b/internal/commands/measurement.go index 8be0f85..d963b3a 100644 --- a/internal/commands/measurement.go +++ b/internal/commands/measurement.go @@ -93,9 +93,13 @@ command errors before any network call.`, func renderMeasurementList(cmd *cobra.Command, list *client.MeasurementListResponse, format string, noHeader bool) error { // Defensive copy + sort: same rationale as renderDatabaseList — - // keep table/csv/json row order consistent, don't mutate the caller's - // slice. - ms := append([]client.DatabaseMeasurement(nil), list.Measurements...) + // keep table/csv/json row order consistent, don't mutate the + // caller's slice. make+copy (not append-to-nil) guarantees the + // result is a non-nil empty slice when the server returned no + // measurements, so JSON output stays `"measurements": []` rather + // than `null`. (Gemini PR #2 finding.) + ms := make([]client.DatabaseMeasurement, len(list.Measurements)) + copy(ms, list.Measurements) sort.Slice(ms, func(i, j int) bool { return ms[i].Name < ms[j].Name }) switch format {