From 514c8dcb89546fbbb55f46629103ac1ee5bbcaea Mon Sep 17 00:00:00 2001 From: Alexandra Primakina Date: Wed, 15 Jul 2026 10:21:24 +0200 Subject: [PATCH 1/8] feat(db): accept a read replica ID wherever a service ID is accepted The db connection commands (connection-string, connect/psql, test-connection, schema) and the db_execute_query / db_schema MCP tools now resolve a read replica set ID as well as a service ID: it's tried as a service first, then matched against the project's read replica sets. A replica ID connects to that replica's endpoint while credentials resolve against the parent service (replicas share the primary's credentials). When --pooled is requested for a replica that has no pooler, all surfaces warn and fall back to a direct connection. Service-management commands still treat a replica ID as not found. Co-Authored-By: Claude Opus 4.8 --- CLAUDE.md | 1 + README.md | 2 +- internal/tiger/cmd/db.go | 137 ++++++++++---- internal/tiger/cmd/read_replica.go | 26 +-- internal/tiger/cmd/read_replica_test.go | 21 --- internal/tiger/cmd/replica_connect_test.go | 196 +++++++++++++++++++++ internal/tiger/common/connection.go | 40 +++++ internal/tiger/common/replica.go | 77 ++++++++ internal/tiger/common/replica_test.go | 167 ++++++++++++++++++ internal/tiger/common/schema_fetch.go | 11 +- internal/tiger/mcp/db_tools.go | 49 +++--- specs/spec.md | 5 +- specs/spec_mcp.md | 3 +- 13 files changed, 626 insertions(+), 109 deletions(-) create mode 100644 internal/tiger/cmd/replica_connect_test.go create mode 100644 internal/tiger/common/replica.go create mode 100644 internal/tiger/common/replica_test.go diff --git a/CLAUDE.md b/CLAUDE.md index 399800a7..82d4cd94 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -273,6 +273,7 @@ Tiger CLI is a Go-based command-line interface for managing Tiger, the modern da - `service.go` - Service management commands (list, create, get, fork, start, stop, resize, delete, update-password, logs) - `db.go` - Database operation commands (connection-string, connect, test-connection) - `read_replica.go` - Read replica selection flow for `db connect`/`psql`: in an interactive terminal, when the service has one or more active read replicas (listed via the `/replicaSets` API), prompts to connect to the primary or one of the replicas. Skipped when stdin is not a TTY, when `--no-replica-prompt` is set, or when the service has no read replicas. + - Read replica ID as a connection target: the db connection commands (`db connection-string`, `db connect`/`psql`, `db test-connection`, `db schema`) and the `db_execute_query`/`db_schema` MCP tools accept a read replica set ID anywhere a service ID is accepted. Resolution is `common.ResolveServiceOrReplica` (MCP) / `resolveConnectionTarget` in `db.go` (CLI): try the ID as a service first, and on failure scan the project's services' embedded `read_replica_sets` for a matching ID (`common.FindReplicaByID`). A matched replica connects to the replica's endpoint while credentials/password reset resolve against the parent service. - `config.go` - Configuration management commands (show, set, unset, reset) - `mcp.go` - MCP server commands (install, start, list, get) - `version.go` - Version command diff --git a/README.md b/README.md index ac527646..7f2827cb 100644 --- a/README.md +++ b/README.md @@ -110,7 +110,7 @@ Tiger CLI provides the following commands: - `delete` - Delete a service - `update-password` - Update service master password - `logs` - View service logs -- `tiger db` - Database operations +- `tiger db` - Database operations (each command below also accepts a read replica set ID in place of the service ID) - `connect` - Connect to a database with psql (in an interactive terminal, if the service has read replicas, offers to connect to one of them; use `--no-replica-prompt` to skip) - `connection-string` - Get connection string for a service - `test-connection` - Test database connectivity diff --git a/internal/tiger/cmd/db.go b/internal/tiger/cmd/db.go index 49a446c4..0562d8ca 100644 --- a/internal/tiger/cmd/db.go +++ b/internal/tiger/cmd/db.go @@ -56,6 +56,9 @@ The service ID can be provided as an argument or will use the default service from your configuration. The connection string includes all necessary parameters for establishing a database connection to the TimescaleDB/PostgreSQL service. +You can also pass a read replica set ID in place of the service ID to get a +connection string for that read replica. + By default, passwords are excluded from the connection string for security. Use --with-password to include the password directly in the connection string. @@ -92,29 +95,25 @@ Examples: return err } - service, err := getServiceDetailsFunc(cmd, cfg, args) + service, replica, err := resolveConnectionTarget(cmd, cfg, args) if err != nil { return err } - details, err := common.GetConnectionDetails(service, common.ConnectionDetailsOptions{ + details, err := buildConnectionDetailsForTarget(cmd, service, replica, common.ConnectionDetailsOptions{ Pooled: dbConnectionStringPooled, Role: dbConnectionStringRole, WithPassword: dbConnectionStringWithPassword, ReadOnly: dbConnectionStringReadOnly || cfg.ReadOnly, }) if err != nil { - return fmt.Errorf("failed to build connection string: %w", err) + return err } if dbConnectionStringWithPassword && details.Password == "" { return fmt.Errorf("password not available to include in connection string") } - if err := details.RequirePooler(dbConnectionStringPooled); err != nil { - return err - } - fmt.Fprintln(cmd.OutOrStdout(), details.String()) return nil }, @@ -163,11 +162,19 @@ primary. Use --no-replica-prompt to skip this prompt and always connect to the requested service. The prompt is automatically skipped when stdin is not a terminal (e.g. in scripts) or when the service has no read replicas. +You can also pass a read replica set ID directly in place of the service ID to +connect straight to that read replica without the prompt. +Read replicas are read-only, may lag the primary, and share the primary +service's credentials. + Examples: # Connect to default service tiger db connect tiger db psql + # Connect directly to a read replica by its ID + tiger db connect rep1234567 + # Connect without the read replica prompt tiger db connect svc-12345 --no-replica-prompt @@ -202,7 +209,7 @@ Examples: // Separate service ID from additional psql flags serviceArgs, psqlFlags := separateServiceAndPsqlArgs(cmd, args) - service, err := getServiceDetailsFunc(cmd, cfg, serviceArgs) + service, replica, err := resolveConnectionTarget(cmd, cfg, serviceArgs) if err != nil { return err } @@ -219,18 +226,25 @@ Examples: ReadOnly: dbConnectReadOnly || cfg.ReadOnly, } - // Optionally offer to connect to an existing read replica instead of - // the primary service. In non-interactive contexts, or when the - // service has no read replicas, this returns the primary's details - // without prompting. Pooler availability is validated here: a hard - // error for the primary, warn-and-fall-back for replicas. - details, err := resolveConnectTarget(cmd.Context(), cmd, cfg.Client, cfg.ProjectID, service, opts, dbConnectNoReplicaPrompt) - if err != nil { - return err - } - if details == nil { - // User cancelled the connection. - return nil + var details *common.ConnectionDetails + if replica != nil { + // A replica ID was named directly — connect straight to it. + details, err = buildConnectionDetailsForTarget(cmd, service, replica, opts) + if err != nil { + return err + } + fmt.Fprintf(cmd.ErrOrStderr(), "Connecting to read replica '%s'...\n", util.DerefStr(replica.Name)) + } else { + // A primary ID — offer the interactive replica menu (a no-op that + // returns the primary when non-interactive or no replicas exist). + details, err = resolveConnectTarget(cmd.Context(), cmd, cfg.Client, cfg.ProjectID, service, opts, dbConnectNoReplicaPrompt) + if err != nil { + return err + } + if details == nil { + // User cancelled the connection. + return nil + } } // Replicas share the primary's credentials, so password storage and @@ -262,6 +276,9 @@ The service ID can be provided as an argument or will use the default service from your configuration. This command tests if the database is accepting connections and returns appropriate exit codes following pg_isready conventions. +You can also pass a read replica set ID in place of the service ID to test +connectivity to that read replica. + Return Codes: 0: Server is accepting connections normally 1: Server is rejecting connections (e.g., during startup) @@ -292,22 +309,18 @@ Examples: return common.ExitWithCode(common.ExitInvalidParameters, err) } - service, err := getServiceDetailsFunc(cmd, cfg, args) + service, replica, err := resolveConnectionTarget(cmd, cfg, args) if err != nil { return common.ExitWithCode(common.ExitInvalidParameters, err) } // Build connection string for testing with password (if available) - details, err := common.GetConnectionDetails(service, common.ConnectionDetailsOptions{ + details, err := buildConnectionDetailsForTarget(cmd, service, replica, common.ConnectionDetailsOptions{ Pooled: dbTestConnectionPooled, Role: dbTestConnectionRole, WithPassword: true, }) if err != nil { - return common.ExitWithCode(common.ExitInvalidParameters, fmt.Errorf("failed to build connection string: %w", err)) - } - - if err := details.RequirePooler(dbTestConnectionPooled); err != nil { return common.ExitWithCode(common.ExitInvalidParameters, err) } @@ -813,8 +826,10 @@ indexes, triggers, and TimescaleDB hypertable and continuous aggregate metadata. The service ID can be provided as an argument or will use the default service -from your configuration. Only objects the connecting role can access are -returned. The connection is opened in Tiger Cloud's immutable read-only mode. +from your configuration. You can also pass a read replica set ID in place of the +service ID to introspect that read replica. Only objects the connecting role can +access are returned. The connection is opened in Tiger Cloud's immutable +read-only mode. By default only user-facing schemas and objects are shown. View and routine definitions and object comments are omitted unless requested, since they can be @@ -844,12 +859,14 @@ Examples: return err } - service, err := getServiceDetailsFunc(cmd, cfg, args) + service, replica, err := resolveConnectionTarget(cmd, cfg, args) if err != nil { return err } - schema, err := common.FetchServiceSchema(cmd.Context(), service, dbSchemaRole, dbSchemaPooled, common.SchemaOptions{ + warnReplicaPooler(cmd, replica, dbSchemaPooled) + + schema, err := common.FetchServiceSchema(cmd.Context(), service, replica, dbSchemaRole, dbSchemaPooled, common.SchemaOptions{ Schema: dbSchemaSchema, IncludeInternal: dbSchemaInternal, IncludeDefinitions: dbSchemaDefinitions, @@ -891,6 +908,66 @@ func buildDbCmd() *cobra.Command { return cmd } +// resolveConnectionTarget looks up the target named by args, which may be a +// primary service ID or a read replica set ID. For a replica, the returned +// service is its parent (for credentials) and replica is non-nil. This lets a +// replica ID work anywhere a service ID does across the db connection commands. +func resolveConnectionTarget(cmd *cobra.Command, cfg *common.Config, args []string) (api.Service, *api.ReadReplicaSet, error) { + // Service lookup is the common case (and what tests stub), so try it first. + service, err := getServiceDetailsFunc(cmd, cfg, args) + if err == nil { + return service, nil, nil + } + + // Not a service — the ID might be a read replica, reachable only by + // scanning its parent. On any failure, keep the original lookup error. + serviceID, idErr := getServiceID(cfg.Config, args) + if idErr != nil { + return api.Service{}, nil, err + } + + ctx, cancel := context.WithTimeout(cmd.Context(), 30*time.Second) + defer cancel() + + target, scanErr := common.FindReplicaByID(ctx, cfg.Client, cfg.ProjectID, serviceID) + if scanErr != nil { + return api.Service{}, nil, err + } + + return target.Service, target.Replica, nil +} + +// warnReplicaPooler prints the replica pooler-fallback warning to stderr, if +// any. It is a no-op for the primary (nil replica) or when nothing to warn. +func warnReplicaPooler(cmd *cobra.Command, replica *api.ReadReplicaSet, pooled bool) { + if warning := common.ReplicaPoolerWarning(replica, pooled); warning != "" { + fmt.Fprintf(cmd.ErrOrStderr(), "⚠️ Warning: %s\n", warning) + } +} + +// buildConnectionDetailsForTarget builds connection details for the primary or +// a read replica. A requested-but-unavailable pooler is a hard error for the +// primary, but a warn-and-fall-back-to-direct for a replica. +func buildConnectionDetailsForTarget(cmd *cobra.Command, service api.Service, replica *api.ReadReplicaSet, opts common.ConnectionDetailsOptions) (*common.ConnectionDetails, error) { + if replica != nil { + warnReplicaPooler(cmd, replica, opts.Pooled) + details, err := common.GetReplicaConnectionDetails(service, *replica, opts) + if err != nil { + return nil, fmt.Errorf("failed to build connection string: %w", err) + } + return details, nil + } + + details, err := common.GetConnectionDetails(service, opts) + if err != nil { + return nil, fmt.Errorf("failed to build connection string: %w", err) + } + if err := details.RequirePooler(opts.Pooled); err != nil { + return nil, err + } + return details, nil +} + // getServiceDetails is a helper that handles common service lookup logic and returns the service details func getServiceDetails(cmd *cobra.Command, cfg *common.Config, args []string) (api.Service, error) { // Determine service ID diff --git a/internal/tiger/cmd/read_replica.go b/internal/tiger/cmd/read_replica.go index 93ab5229..de255e47 100644 --- a/internal/tiger/cmd/read_replica.go +++ b/internal/tiger/cmd/read_replica.go @@ -28,17 +28,8 @@ func resolveConnectTarget( opts common.ConnectionDetailsOptions, noReplicaPrompt bool, ) (*common.ConnectionDetails, error) { - // returnPrimary builds the primary's details. Requesting --pooled when no - // pooler exists is a hard error. returnPrimary := func() (*common.ConnectionDetails, error) { - details, err := common.GetConnectionDetails(primary, opts) - if err != nil { - return nil, fmt.Errorf("failed to build connection string: %w", err) - } - if err := details.RequirePooler(opts.Pooled); err != nil { - return nil, err - } - return details, nil + return buildConnectionDetailsForTarget(cmd, primary, nil, opts) } // Only prompt on an interactive terminal. @@ -79,15 +70,7 @@ func resolveConnectTarget( } replica := choice.replica - - // --pooled is best-effort on replicas: warn and connect directly if the - // chosen replica has no pooler. - if opts.Pooled && !replicaHasPooler(replica) { - fmt.Fprintf(cmd.ErrOrStderr(), "⚠️ Warning: read replica '%s' has no connection pooler; connecting directly instead.\n", util.DerefStr(replica.Name)) - opts.Pooled = false - } - - details, err := common.GetReplicaConnectionDetails(primary, *replica, opts) + details, err := buildConnectionDetailsForTarget(cmd, primary, replica, opts) if err != nil { return nil, err } @@ -95,11 +78,6 @@ func resolveConnectTarget( return details, nil } -// replicaHasPooler reports whether a replica set exposes a pooler endpoint. -func replicaHasPooler(replica *api.ReadReplicaSet) bool { - return replica != nil && replica.ConnectionPooler != nil && replica.ConnectionPooler.Endpoint != nil -} - // fetchReplicaSets retrieves the read replica sets for a service. func fetchReplicaSets(ctx context.Context, client *api.ClientWithResponses, projectID, serviceID string) ([]api.ReadReplicaSet, error) { ctx, cancel := context.WithTimeout(ctx, 30*time.Second) diff --git a/internal/tiger/cmd/read_replica_test.go b/internal/tiger/cmd/read_replica_test.go index 9eefebdf..fe8e0929 100644 --- a/internal/tiger/cmd/read_replica_test.go +++ b/internal/tiger/cmd/read_replica_test.go @@ -91,27 +91,6 @@ func TestConnectTargetModel_KeySelection(t *testing.T) { } } -func TestReplicaHasPooler(t *testing.T) { - host := "h" - port := 6432 - - cases := []struct { - name string - replica *api.ReadReplicaSet - want bool - }{ - {"nil replica", nil, false}, - {"no pooler", &api.ReadReplicaSet{}, false}, - {"pooler without endpoint", &api.ReadReplicaSet{ConnectionPooler: &api.ConnectionPooler{}}, false}, - {"pooler with endpoint", &api.ReadReplicaSet{ConnectionPooler: &api.ConnectionPooler{Endpoint: &api.Endpoint{Host: &host, Port: &port}}}, true}, - } - for _, tc := range cases { - if got := replicaHasPooler(tc.replica); got != tc.want { - t.Errorf("%s: replicaHasPooler = %v, want %v", tc.name, got, tc.want) - } - } -} - // TestResolveConnectTarget_NoReplicasSkipsPrompt verifies that, with no // connectable replicas, resolveConnectTarget connects to the primary directly // instead of showing a single-option menu (which would block on TTY input in diff --git a/internal/tiger/cmd/replica_connect_test.go b/internal/tiger/cmd/replica_connect_test.go new file mode 100644 index 00000000..ff8adbcb --- /dev/null +++ b/internal/tiger/cmd/replica_connect_test.go @@ -0,0 +1,196 @@ +package cmd + +import ( + "bytes" + "context" + "encoding/json" + "fmt" + "io" + "net/http" + "net/http/httptest" + "strings" + "testing" + + "github.com/spf13/cobra" + + "github.com/timescale/tiger-cli/internal/tiger/api" + "github.com/timescale/tiger-cli/internal/tiger/common" + "github.com/timescale/tiger-cli/internal/tiger/config" + "github.com/timescale/tiger-cli/internal/tiger/util" +) + +// replicaConnectTestConfig builds a Config whose client is backed by an +// httptest server that 404s on getService (so resolution falls back to the +// scan) and returns the given services from the list endpoint. +func replicaConnectTestConfig(t *testing.T, list []api.Service) *common.Config { + t.Helper() + + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + if strings.HasSuffix(r.URL.Path, "/services") { + _ = json.NewEncoder(w).Encode(list) + return + } + // getService for a replica ID is a miss. + w.WriteHeader(http.StatusNotFound) + _ = json.NewEncoder(w).Encode(map[string]string{"message": "service not found"}) + })) + t.Cleanup(srv.Close) + + client, err := api.NewClientWithResponses(srv.URL) + if err != nil { + t.Fatalf("failed to build client: %v", err) + } + return &common.Config{Config: &config.Config{}, ProjectID: "proj1", Client: client} +} + +func replicaConnectTestService() api.Service { + rhost := "replica.example.com" + rport := 5432 + return api.Service{ + ServiceId: util.Ptr("svcprimary"), + ProjectId: util.Ptr("proj1"), + Name: util.Ptr("my-db"), + ReadReplicaSets: &[]api.ReadReplicaSet{{ + Id: util.Ptr("rep1234567"), + Name: util.Ptr("reporting-replica"), + Status: util.Ptr(api.ReadReplicaSetStatusActive), + Endpoint: &api.Endpoint{Host: &rhost, Port: &rport}, + }}, + } +} + +// TestResolveConnectionTarget_Primary: when the service lookup succeeds, the ID +// is a primary and no replica is returned. +func TestResolveConnectionTarget_Primary(t *testing.T) { + orig := getServiceDetailsFunc + getServiceDetailsFunc = func(cmd *cobra.Command, cfg *common.Config, args []string) (api.Service, error) { + return api.Service{ServiceId: util.Ptr("svcprimary")}, nil + } + defer func() { getServiceDetailsFunc = orig }() + + cfg := &common.Config{Config: &config.Config{}, ProjectID: "proj1"} + cmd := &cobra.Command{} + cmd.SetContext(context.Background()) + + service, replica, err := resolveConnectionTarget(cmd, cfg, []string{"svcprimary"}) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if replica != nil { + t.Fatalf("expected no replica for a primary ID, got %+v", replica) + } + if util.DerefStr(service.ServiceId) != "svcprimary" { + t.Errorf("expected primary svcprimary, got %q", util.DerefStr(service.ServiceId)) + } +} + +// TestResolveConnectionTarget_ReplicaFallback: when the service lookup fails, +// the ID is scanned against the project's read replicas, and the matching +// replica plus its parent service is returned. +func TestResolveConnectionTarget_ReplicaFallback(t *testing.T) { + orig := getServiceDetailsFunc + getServiceDetailsFunc = func(cmd *cobra.Command, cfg *common.Config, args []string) (api.Service, error) { + return api.Service{}, fmt.Errorf("service not found") + } + defer func() { getServiceDetailsFunc = orig }() + + cfg := replicaConnectTestConfig(t, []api.Service{replicaConnectTestService()}) + cmd := &cobra.Command{} + cmd.SetContext(context.Background()) + + service, replica, err := resolveConnectionTarget(cmd, cfg, []string{"rep1234567"}) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if replica == nil { + t.Fatal("expected a replica to be resolved, got nil") + } + if util.DerefStr(replica.Id) != "rep1234567" { + t.Errorf("expected replica rep1234567, got %q", util.DerefStr(replica.Id)) + } + // Parent service (used for credentials) is returned alongside the replica. + if util.DerefStr(service.ServiceId) != "svcprimary" { + t.Errorf("expected parent svcprimary, got %q", util.DerefStr(service.ServiceId)) + } +} + +// TestResolveConnectionTarget_UnknownReturnsOriginalError: an ID that is neither +// a service nor a replica surfaces the original service-lookup error. +func TestResolveConnectionTarget_UnknownReturnsOriginalError(t *testing.T) { + orig := getServiceDetailsFunc + getServiceDetailsFunc = func(cmd *cobra.Command, cfg *common.Config, args []string) (api.Service, error) { + return api.Service{}, fmt.Errorf("original lookup error") + } + defer func() { getServiceDetailsFunc = orig }() + + cfg := replicaConnectTestConfig(t, []api.Service{replicaConnectTestService()}) + cmd := &cobra.Command{} + cmd.SetContext(context.Background()) + + _, _, err := resolveConnectionTarget(cmd, cfg, []string{"missing999"}) + if err == nil { + t.Fatal("expected an error for an unknown ID, got nil") + } + if !strings.Contains(err.Error(), "original lookup error") { + t.Errorf("expected the original lookup error to be surfaced, got %v", err) + } +} + +// TestBuildConnectionDetailsForTarget_ReplicaPoolerFallback: requesting --pooled +// on a replica with no pooler warns and falls back to a direct connection. +func TestBuildConnectionDetailsForTarget_ReplicaPoolerFallback(t *testing.T) { + rhost := "replica.example.com" + rport := 5432 + replica := &api.ReadReplicaSet{ + Id: util.Ptr("rep1234567"), + Name: util.Ptr("reporting-replica"), + Endpoint: &api.Endpoint{Host: &rhost, Port: &rport}, + } + primary := api.Service{ServiceId: util.Ptr("svcprimary"), ProjectId: util.Ptr("proj1")} + + buf := &bytes.Buffer{} + cmd := &cobra.Command{} + cmd.SetErr(buf) + cmd.SetOut(io.Discard) + + details, err := buildConnectionDetailsForTarget(cmd, primary, replica, common.ConnectionDetailsOptions{ + Pooled: true, + Role: "tsdbadmin", + }) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if details.Host != rhost { + t.Errorf("expected replica host %q, got %q", rhost, details.Host) + } + if details.IsPooler { + t.Error("expected fallback to a direct (non-pooler) connection") + } + if !strings.Contains(buf.String(), "no connection pooler") { + t.Errorf("expected a pooler-fallback warning, got %q", buf.String()) + } +} + +// TestBuildConnectionDetailsForTarget_PrimaryRequiresPooler: requesting --pooled +// on a primary with no pooler is a hard error. +func TestBuildConnectionDetailsForTarget_PrimaryRequiresPooler(t *testing.T) { + host := "primary.example.com" + port := 5432 + primary := api.Service{ + ServiceId: util.Ptr("svcprimary"), + Endpoint: &api.Endpoint{Host: &host, Port: &port}, + } + + cmd := &cobra.Command{} + cmd.SetErr(io.Discard) + cmd.SetOut(io.Discard) + + _, err := buildConnectionDetailsForTarget(cmd, primary, nil, common.ConnectionDetailsOptions{ + Pooled: true, + Role: "tsdbadmin", + }) + if err == nil { + t.Fatal("expected an error when a pooler is unavailable for the primary, got nil") + } +} diff --git a/internal/tiger/common/connection.go b/internal/tiger/common/connection.go index ad993b3f..753e2fab 100644 --- a/internal/tiger/common/connection.go +++ b/internal/tiger/common/connection.go @@ -7,8 +7,24 @@ import ( "github.com/jackc/pgx/v5" "github.com/timescale/tiger-cli/internal/tiger/api" + "github.com/timescale/tiger-cli/internal/tiger/util" ) +// replicaHasPooler reports whether a read replica exposes a pooler endpoint. +func replicaHasPooler(replica *api.ReadReplicaSet) bool { + return replica != nil && replica.ConnectionPooler != nil && replica.ConnectionPooler.Endpoint != nil +} + +// ReplicaPoolerWarning returns the single-sourced warning shown when pooling was +// requested for a replica with no pooler (the connection then falls back to +// direct, handled by buildConnectionDetails). +func ReplicaPoolerWarning(replica *api.ReadReplicaSet, pooled bool) string { + if pooled && replica != nil && !replicaHasPooler(replica) { + return fmt.Sprintf("read replica %q has no connection pooler; connecting directly instead", util.DerefStr(replica.Name)) + } + return "" +} + // ConnectionDetailsOptions configures how the connection string is built type ConnectionDetailsOptions struct { // Pooled determines whether to use the pooler endpoint (if available) @@ -77,6 +93,30 @@ func ConnectToService(ctx context.Context, service api.Service, opts ConnectionD return nil, err } + return connectWithDetails(ctx, details, mode) +} + +// ConnectToTarget opens a pgx connection to the primary service, or to a read +// replica when replica is non-nil (credentials still resolve against the +// primary). A requested pooler that the replica lacks falls back to a direct +// connection; this function does not warn — callers surface that via +// common.ReplicaPoolerWarning. The caller owns the returned connection and must +// Close it. +func ConnectToTarget(ctx context.Context, primary api.Service, replica *api.ReadReplicaSet, opts ConnectionDetailsOptions, mode pgx.QueryExecMode) (*pgx.Conn, error) { + if replica == nil { + return ConnectToService(ctx, primary, opts, mode) + } + + details, err := GetReplicaConnectionDetails(primary, *replica, opts) + if err != nil { + return nil, fmt.Errorf("failed to build connection string: %w", err) + } + return connectWithDetails(ctx, details, mode) +} + +// connectWithDetails parses connection details and opens a pgx connection using +// the given query execution mode. +func connectWithDetails(ctx context.Context, details *ConnectionDetails, mode pgx.QueryExecMode) (*pgx.Conn, error) { connConfig, err := pgx.ParseConfig(details.String()) if err != nil { return nil, fmt.Errorf("failed to parse connection string: %w", err) diff --git a/internal/tiger/common/replica.go b/internal/tiger/common/replica.go new file mode 100644 index 00000000..002e2443 --- /dev/null +++ b/internal/tiger/common/replica.go @@ -0,0 +1,77 @@ +package common + +import ( + "context" + "fmt" + "net/http" + + "github.com/timescale/tiger-cli/internal/tiger/api" + "github.com/timescale/tiger-cli/internal/tiger/util" +) + +// ResolvedTarget is a connection ID resolved to either a primary service or one +// of its read replica sets. Service is always the parent (credentials and +// password resets operate on it, since replicas share the primary's +// credentials); Replica is non-nil when the ID named a read replica, whose +// endpoint connections should target. +type ResolvedTarget struct { + Service api.Service + Replica *api.ReadReplicaSet +} + +// ResolveServiceOrReplica resolves id to a primary service or a read replica of +// some service in the project. It tries a direct service lookup first; on +// failure it scans services for a replica with that ID (read replicas aren't +// addressable on their own). If neither matches, the original service-lookup +// error is returned. +func ResolveServiceOrReplica(ctx context.Context, client api.ClientWithResponsesInterface, projectID, id string) (*ResolvedTarget, error) { + resp, err := client.GetServiceWithResponse(ctx, projectID, id) + if err != nil { + return nil, fmt.Errorf("failed to fetch service details: %w", err) + } + if resp.StatusCode() == http.StatusOK { + if resp.JSON200 == nil { + return nil, fmt.Errorf("empty response from API") + } + return &ResolvedTarget{Service: *resp.JSON200}, nil + } + + // Not a service — maybe a read replica ID. + if target, scanErr := FindReplicaByID(ctx, client, projectID, id); scanErr == nil { + return target, nil + } + + // Neither; surface the original service-lookup error. + return nil, ExitWithErrorFromStatusCode(resp.StatusCode(), resp.JSON4XX) +} + +// FindReplicaByID scans the project's services and returns the parent service +// and read replica set whose ID matches id, or an error when no replica in the +// project matches. Use it to fall back to replica resolution once a direct +// service lookup has failed. +func FindReplicaByID(ctx context.Context, client api.ClientWithResponsesInterface, projectID, id string) (*ResolvedTarget, error) { + resp, err := client.GetServicesWithResponse(ctx, projectID) + if err != nil { + return nil, fmt.Errorf("failed to list services: %w", err) + } + if resp.StatusCode() != http.StatusOK { + return nil, ExitWithErrorFromStatusCode(resp.StatusCode(), resp.JSON4XX) + } + + if resp.JSON200 != nil { + services := *resp.JSON200 + for i := range services { + if services[i].ReadReplicaSets == nil { + continue + } + replicas := *services[i].ReadReplicaSets + for j := range replicas { + if util.DerefStr(replicas[j].Id) == id { + return &ResolvedTarget{Service: services[i], Replica: &replicas[j]}, nil + } + } + } + } + + return nil, fmt.Errorf("no service or read replica found with ID %q", id) +} diff --git a/internal/tiger/common/replica_test.go b/internal/tiger/common/replica_test.go new file mode 100644 index 00000000..650a364c --- /dev/null +++ b/internal/tiger/common/replica_test.go @@ -0,0 +1,167 @@ +package common + +import ( + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "strings" + "testing" + + "github.com/timescale/tiger-cli/internal/tiger/api" + "github.com/timescale/tiger-cli/internal/tiger/util" +) + +// replicaTestClient builds an API client backed by an httptest server that +// serves the getService endpoint from services (keyed by service ID, 404 when +// absent) and the getServices list endpoint from list. +func replicaTestClient(t *testing.T, services map[string]api.Service, list []api.Service) *api.ClientWithResponses { + t.Helper() + + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + + // List: /projects/{project_id}/services + if strings.HasSuffix(r.URL.Path, "/services") { + _ = json.NewEncoder(w).Encode(list) + return + } + + // Get: /projects/{project_id}/services/{service_id} + parts := strings.Split(strings.Trim(r.URL.Path, "/"), "/") + id := parts[len(parts)-1] + svc, ok := services[id] + if !ok { + w.WriteHeader(http.StatusNotFound) + _ = json.NewEncoder(w).Encode(map[string]string{"message": "service not found"}) + return + } + _ = json.NewEncoder(w).Encode(svc) + })) + t.Cleanup(srv.Close) + + client, err := api.NewClientWithResponses(srv.URL) + if err != nil { + t.Fatalf("failed to build client: %v", err) + } + return client +} + +func testReplicaService() api.Service { + rhost := "replica.example.com" + rport := 5432 + return api.Service{ + ServiceId: util.Ptr("svcprimary"), + ProjectId: util.Ptr("proj1"), + Name: util.Ptr("my-db"), + ReadReplicaSets: &[]api.ReadReplicaSet{ + { + Id: util.Ptr("rep1234567"), + Name: util.Ptr("reporting-replica"), + Status: util.Ptr(api.ReadReplicaSetStatusActive), + Endpoint: &api.Endpoint{Host: &rhost, Port: &rport}, + }, + }, + } +} + +func TestResolveServiceOrReplica_Primary(t *testing.T) { + primary := testReplicaService() + client := replicaTestClient(t, map[string]api.Service{"svcprimary": primary}, []api.Service{primary}) + + target, err := ResolveServiceOrReplica(context.Background(), client, "proj1", "svcprimary") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if target.Replica != nil { + t.Fatalf("expected a primary target, got a replica: %+v", target.Replica) + } + if util.DerefStr(target.Service.ServiceId) != "svcprimary" { + t.Errorf("expected primary svcprimary, got %q", util.DerefStr(target.Service.ServiceId)) + } +} + +func TestResolveServiceOrReplica_Replica(t *testing.T) { + primary := testReplicaService() + // The replica ID is not addressable as a service, so getService 404s and + // resolution must fall back to scanning the services list. + client := replicaTestClient(t, map[string]api.Service{"svcprimary": primary}, []api.Service{primary}) + + target, err := ResolveServiceOrReplica(context.Background(), client, "proj1", "rep1234567") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if target.Replica == nil { + t.Fatalf("expected a replica target, got primary only") + } + // Parent service is used for credentials. + if util.DerefStr(target.Service.ServiceId) != "svcprimary" { + t.Errorf("expected parent svcprimary, got %q", util.DerefStr(target.Service.ServiceId)) + } + // Replica carries the endpoint we should connect to. + if util.DerefStr(target.Replica.Id) != "rep1234567" { + t.Errorf("expected replica rep1234567, got %q", util.DerefStr(target.Replica.Id)) + } + if target.Replica.Endpoint == nil || util.DerefStr(target.Replica.Endpoint.Host) != "replica.example.com" { + t.Errorf("expected replica endpoint host, got %+v", target.Replica.Endpoint) + } +} + +func TestResolveServiceOrReplica_NotFound(t *testing.T) { + primary := testReplicaService() + client := replicaTestClient(t, map[string]api.Service{"svcprimary": primary}, []api.Service{primary}) + + // An ID that is neither a service nor a known replica surfaces an error. + _, err := ResolveServiceOrReplica(context.Background(), client, "proj1", "unknown999") + if err == nil { + t.Fatal("expected an error for an unknown ID, got nil") + } +} + +func TestReplicaPoolerWarning(t *testing.T) { + host := "h" + port := 6432 + withPooler := &api.ReadReplicaSet{ + Name: util.Ptr("rep-a"), + ConnectionPooler: &api.ConnectionPooler{Endpoint: &api.Endpoint{Host: &host, Port: &port}}, + } + noPooler := &api.ReadReplicaSet{Name: util.Ptr("rep-b")} + + cases := []struct { + name string + replica *api.ReadReplicaSet + pooled bool + wantWarning bool + }{ + {"nil replica", nil, true, false}, + {"not pooled, no pooler", noPooler, false, false}, + {"not pooled, has pooler", withPooler, false, false}, + {"pooled, has pooler", withPooler, true, false}, + {"pooled, no pooler warns", noPooler, true, true}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + warning := ReplicaPoolerWarning(tc.replica, tc.pooled) + if (warning != "") != tc.wantWarning { + t.Errorf("warning = %q, wantWarning = %v", warning, tc.wantWarning) + } + }) + } +} + +func TestFindReplicaByID(t *testing.T) { + primary := testReplicaService() + client := replicaTestClient(t, map[string]api.Service{"svcprimary": primary}, []api.Service{primary}) + + target, err := FindReplicaByID(context.Background(), client, "proj1", "rep1234567") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if target.Replica == nil || util.DerefStr(target.Replica.Id) != "rep1234567" { + t.Errorf("expected to find replica rep1234567, got %+v", target) + } + + if _, err := FindReplicaByID(context.Background(), client, "proj1", "missing999"); err == nil { + t.Error("expected an error when no replica matches, got nil") + } +} diff --git a/internal/tiger/common/schema_fetch.go b/internal/tiger/common/schema_fetch.go index 0ce78c1a..6b30c7cb 100644 --- a/internal/tiger/common/schema_fetch.go +++ b/internal/tiger/common/schema_fetch.go @@ -9,19 +9,20 @@ import ( "github.com/timescale/tiger-cli/internal/tiger/util" ) -// FetchServiceSchema opens a read-only connection to the service and -// introspects its schema. It is the shared entry point for the `tiger db -// schema` CLI command and the db_schema MCP tool. +// FetchServiceSchema opens a read-only connection to the service (or one of its +// read replicas, when replica is non-nil) and introspects its schema. It is the +// shared entry point for the `tiger db schema` CLI command and the db_schema +// MCP tool. // // The connection is forced read-only: introspection only issues SELECTs, so // this is always safe and guards against accidental writes. -func FetchServiceSchema(ctx context.Context, service api.Service, role string, pooled bool, opts SchemaOptions) (*DatabaseSchema, error) { +func FetchServiceSchema(ctx context.Context, service api.Service, replica *api.ReadReplicaSet, role string, pooled bool, opts SchemaOptions) (*DatabaseSchema, error) { if err := CheckServiceReady(service); err != nil { return nil, err } // Introspection runs parameterless statements, so the simple protocol fits. - conn, err := ConnectToService(ctx, service, ConnectionDetailsOptions{ + conn, err := ConnectToTarget(ctx, service, replica, ConnectionDetailsOptions{ Pooled: pooled, Role: role, WithPassword: true, diff --git a/internal/tiger/mcp/db_tools.go b/internal/tiger/mcp/db_tools.go index 62ea933f..9af7c4b7 100644 --- a/internal/tiger/mcp/db_tools.go +++ b/internal/tiger/mcp/db_tools.go @@ -4,7 +4,6 @@ import ( "context" "encoding/json" "fmt" - "net/http" "time" "github.com/google/jsonschema-go/jsonschema" @@ -62,7 +61,7 @@ type DBExecuteQueryInput struct { func (DBExecuteQueryInput) Schema() *jsonschema.Schema { schema := util.Must(jsonschema.For[DBExecuteQueryInput](nil)) - schema.Properties["service_id"].Description = "Unique identifier of the service (10-character alphanumeric string). Use service_list to find service IDs." + schema.Properties["service_id"].Description = "Unique identifier of the service (10-character alphanumeric string). Use service_list to find service IDs. A read replica set ID is also accepted here — passing one runs the query against that read replica (which is read-only and may lag the primary) instead of the primary service." schema.Properties["service_id"].Examples = []any{"e6ue9697jf", "u8me885b93"} schema.Properties["service_id"].Pattern = "^[a-z0-9]{10}$" @@ -108,6 +107,7 @@ type DBExecuteQueryOutput struct { ExecutionTime string `json:"execution_time"` Truncated bool `json:"truncated,omitempty"` Notice string `json:"notice,omitempty"` + Warning string `json:"warning,omitempty"` } func (DBExecuteQueryOutput) Schema() *jsonschema.Schema { @@ -143,6 +143,8 @@ func (DBExecuteQueryOutput) Schema() *jsonschema.Schema { schema.Properties["notice"].Description = "Present only when results were truncated. Actionable guidance for getting the needed data via SQL (aggregate, filter, paginate) instead of re-running the query." + schema.Properties["warning"].Description = "Present when connection pooling was requested for a read replica that has none; the query ran over a direct connection instead." + return schema } @@ -203,7 +205,7 @@ type DBSchemaInput struct { func (DBSchemaInput) Schema() *jsonschema.Schema { schema := util.Must(jsonschema.For[DBSchemaInput](nil)) - schema.Properties["service_id"].Description = "Unique identifier of the service (10-character alphanumeric string). Use service_list to find service IDs." + schema.Properties["service_id"].Description = "Unique identifier of the service (10-character alphanumeric string). Use service_list to find service IDs. A read replica set ID is also accepted here — passing one introspects that read replica instead of the primary service." schema.Properties["service_id"].Examples = []any{"e6ue9697jf", "u8me885b93"} schema.Properties["service_id"].Pattern = "^[a-z0-9]{10}$" @@ -233,6 +235,7 @@ func (DBSchemaInput) Schema() *jsonschema.Schema { // DBSchemaOutput represents output for db_schema type DBSchemaOutput struct { SchemaText string `json:"schema"` + Warning string `json:"warning,omitempty"` } func (DBSchemaOutput) Schema() *jsonschema.Schema { @@ -240,6 +243,8 @@ func (DBSchemaOutput) Schema() *jsonschema.Schema { schema.Properties["schema"].Description = "The database schema rendered as human-readable text, grouped under a SCHEMA header per namespace." + schema.Properties["warning"].Description = "Present when connection pooling was requested for a read replica that has none; the schema was read over a direct connection instead." + return schema } @@ -261,18 +266,16 @@ func (s *Server) handleDBSchema(ctx context.Context, req *mcp.CallToolRequest, i zap.Bool("pooled", input.Pooled), ) - serviceResp, err := cfg.Client.GetServiceWithResponse(ctx, cfg.ProjectID, input.ServiceID) + // service_id may name a service or one of its read replicas. + target, err := common.ResolveServiceOrReplica(ctx, cfg.Client, cfg.ProjectID, input.ServiceID) if err != nil { - return nil, DBSchemaOutput{}, fmt.Errorf("failed to get service details: %w", err) - } - if serviceResp.StatusCode() != http.StatusOK { - return nil, DBSchemaOutput{}, common.ExitWithErrorFromStatusCode(serviceResp.StatusCode(), serviceResp.JSON4XX) - } - if serviceResp.JSON200 == nil { - return nil, DBSchemaOutput{}, fmt.Errorf("empty response from API") + return nil, DBSchemaOutput{}, err } - schema, err := common.FetchServiceSchema(ctx, *serviceResp.JSON200, input.Role, input.Pooled, common.SchemaOptions{ + // A replica without a pooler connects directly; surface that as a warning. + warning := common.ReplicaPoolerWarning(target.Replica, input.Pooled) + + schema, err := common.FetchServiceSchema(ctx, target.Service, target.Replica, input.Role, input.Pooled, common.SchemaOptions{ Schema: input.SchemaName, IncludeInternal: input.Internal, IncludeDefinitions: input.Definitions, @@ -282,7 +285,7 @@ func (s *Server) handleDBSchema(ctx context.Context, req *mcp.CallToolRequest, i return nil, DBSchemaOutput{}, err } - return nil, DBSchemaOutput{SchemaText: common.FormatSchema(schema)}, nil + return nil, DBSchemaOutput{SchemaText: common.FormatSchema(schema), Warning: warning}, nil } // handleDBExecuteQuery handles the db_execute_query MCP tool @@ -305,21 +308,14 @@ func (s *Server) handleDBExecuteQuery(ctx context.Context, req *mcp.CallToolRequ zap.Bool("read_only", cfg.ReadOnly), ) - // Get service details to construct connection string - serviceResp, err := cfg.Client.GetServiceWithResponse(ctx, cfg.ProjectID, input.ServiceID) + // service_id may name a service or one of its read replicas. + target, err := common.ResolveServiceOrReplica(ctx, cfg.Client, cfg.ProjectID, input.ServiceID) if err != nil { - return nil, DBExecuteQueryOutput{}, fmt.Errorf("failed to get service details: %w", err) - } - - if serviceResp.StatusCode() != http.StatusOK { - return nil, DBExecuteQueryOutput{}, common.ExitWithErrorFromStatusCode(serviceResp.StatusCode(), serviceResp.JSON4XX) - } - - if serviceResp.JSON200 == nil { - return nil, DBExecuteQueryOutput{}, fmt.Errorf("empty response from API") + return nil, DBExecuteQueryOutput{}, err } - service := *serviceResp.JSON200 + // A replica without a pooler connects directly; surface that as a warning. + poolerWarning := common.ReplicaPoolerWarning(target.Replica, input.Pooled) // Create query context with timeout queryCtx, cancel := context.WithTimeout(ctx, timeout) @@ -340,7 +336,7 @@ func (s *Server) handleDBExecuteQuery(ctx context.Context, req *mcp.CallToolRequ } // Connect to database - conn, err := common.ConnectToService(queryCtx, service, common.ConnectionDetailsOptions{ + conn, err := common.ConnectToTarget(queryCtx, target.Service, target.Replica, common.ConnectionDetailsOptions{ Pooled: input.Pooled, Role: input.Role, WithPassword: true, @@ -412,6 +408,7 @@ func (s *Server) handleDBExecuteQuery(ctx context.Context, req *mcp.CallToolRequ output := DBExecuteQueryOutput{ ResultSets: resultSets, ExecutionTime: time.Since(startTime).String(), + Warning: poolerWarning, } if truncated { output.Truncated = true diff --git a/specs/spec.md b/specs/spec.md index 5bd9c951..e4e69713 100644 --- a/specs/spec.md +++ b/specs/spec.md @@ -568,8 +568,11 @@ The `save-password` command accepts passwords through three methods (in order of **Advanced psql Usage:** The `connect` and `psql` commands support passing additional flags directly to the psql client using the `--` separator. Any flags after `--` are passed through to psql unchanged, allowing full access to psql's functionality while maintaining tiger's connection and authentication handling. +**Read Replica ID as a Connection Target (all db connection commands):** +The `db connection-string`, `db connect` / `db psql`, `db test-connection`, and `db schema` commands accept a read replica set ID anywhere a service ID is accepted (as the positional argument or the default service). Because a read replica set ID is not addressable as a service (the API only exposes read replicas nested under their parent service), the CLI first tries to look up the ID as a service; if that fails, it scans the project's services for a read replica set with that ID. When it matches a replica, the command connects to that replica's endpoint while credentials resolve against the parent service. An ID that is neither a service nor a read replica surfaces the original service-not-found error. + **Read Replica Prompt (connect/psql):** -When `tiger db connect` / `tiger db psql` runs in an interactive terminal, it checks whether the target service has any read replicas (read replica sets). If the service has one or more active read replicas, it presents a menu offering to connect to the primary (default) or to one of the existing replicas. If the service has no read replicas, no menu is shown and the command connects to the primary directly. +When `tiger db connect` / `tiger db psql` runs in an interactive terminal and is given a *primary service* ID, it checks whether that service has any read replicas (read replica sets). If the service has one or more active read replicas, it presents a menu offering to connect to the primary (default) or to one of the existing replicas. If the service has no read replicas, no menu is shown and the command connects to the primary directly. A read replica shares the primary service's credentials, so stored passwords and password recovery work transparently against the replica. The prompt is automatically skipped when stdin is not a TTY (e.g. in scripts/automation), in which case the command connects to the requested service as before. Use `--no-replica-prompt` to skip the prompt even in an interactive terminal. diff --git a/specs/spec_mcp.md b/specs/spec_mcp.md index 094b4b0d..d9a8dfc0 100644 --- a/specs/spec_mcp.md +++ b/specs/spec_mcp.md @@ -344,7 +344,7 @@ Test database connectivity. Execute a SQL query on a service database. **Parameters:** -- `service_id` (string, required): Service ID +- `service_id` (string, required): Service ID. A read replica set ID is also accepted here — passing one runs the query against that read replica (which is read-only and may lag the primary) instead of the primary service. Credentials resolve against the parent service. - `query` (string, required): SQL query to execute - `parameters` (array, optional): Query parameters for parameterized queries. Values are substituted for $1, $2, etc. placeholders in the query. - `timeout_seconds` (number, optional): Query timeout in seconds (default: 30) @@ -378,6 +378,7 @@ Execute a SQL query on a service database. - Empty `rows` array for commands that don't return rows (INSERT, UPDATE, DELETE, DDL commands) - For parity with `tiger db connect` command, supports custom roles and connection pooling - `truncated` (per result set and top-level) and `notice` are present only when results were capped; see "Result limiting" above +- `warning` is present only when `pooled` was requested for a read replica that has no connection pooler; the query ran over a direct connection instead ### High-Availability Management From deea178e9e9a6cd6e2910781de6e94dabb43cfc9 Mon Sep 17 00:00:00 2001 From: Alexandra Primakina Date: Wed, 15 Jul 2026 12:06:18 +0200 Subject: [PATCH 2/8] fix(db): validate replica status and skip scan on non-404 lookups Read replica ID resolution now: - validates a matched replica is active and has an endpoint, returning a clear "not active" / "no endpoint" error instead of failing late at connect time. A new ErrReplicaNotFound sentinel distinguishes a real no-match (fall back to the service error) from a matched-but-unusable replica (surface it). - only scans the project's services when the service lookup failed with a 404; other failures (auth, network, 5xx) are surfaced as-is via the new common.IsNotFound helper, avoiding a wasted service-list call. Co-Authored-By: Claude Opus 4.8 --- internal/tiger/cmd/db.go | 18 ++++-- internal/tiger/cmd/replica_connect_test.go | 35 ++++++++++-- internal/tiger/common/errors.go | 7 +++ internal/tiger/common/replica.go | 44 +++++++++++---- internal/tiger/common/replica_test.go | 64 +++++++++++++++++++++- 5 files changed, 146 insertions(+), 22 deletions(-) diff --git a/internal/tiger/cmd/db.go b/internal/tiger/cmd/db.go index 0562d8ca..aef24a78 100644 --- a/internal/tiger/cmd/db.go +++ b/internal/tiger/cmd/db.go @@ -919,8 +919,12 @@ func resolveConnectionTarget(cmd *cobra.Command, cfg *common.Config, args []stri return service, nil, nil } - // Not a service — the ID might be a read replica, reachable only by - // scanning its parent. On any failure, keep the original lookup error. + // Only a not-found could instead be a read replica; other failures (auth, + // network, 5xx) aren't worth a full service-list scan. + if !common.IsNotFound(err) { + return api.Service{}, nil, err + } + serviceID, idErr := getServiceID(cfg.Config, args) if idErr != nil { return api.Service{}, nil, err @@ -930,11 +934,15 @@ func resolveConnectionTarget(cmd *cobra.Command, cfg *common.Config, args []stri defer cancel() target, scanErr := common.FindReplicaByID(ctx, cfg.Client, cfg.ProjectID, serviceID) - if scanErr != nil { + switch { + case scanErr == nil: + return target.Service, target.Replica, nil + case errors.Is(scanErr, common.ErrReplicaNotFound): + // Neither a service nor a replica: surface the original not-found error. return api.Service{}, nil, err + default: + return api.Service{}, nil, scanErr } - - return target.Service, target.Replica, nil } // warnReplicaPooler prints the replica pooler-fallback warning to stderr, if diff --git a/internal/tiger/cmd/replica_connect_test.go b/internal/tiger/cmd/replica_connect_test.go index ff8adbcb..93f0b0a1 100644 --- a/internal/tiger/cmd/replica_connect_test.go +++ b/internal/tiger/cmd/replica_connect_test.go @@ -91,7 +91,7 @@ func TestResolveConnectionTarget_Primary(t *testing.T) { func TestResolveConnectionTarget_ReplicaFallback(t *testing.T) { orig := getServiceDetailsFunc getServiceDetailsFunc = func(cmd *cobra.Command, cfg *common.Config, args []string) (api.Service, error) { - return api.Service{}, fmt.Errorf("service not found") + return api.Service{}, common.ExitWithErrorFromStatusCode(http.StatusNotFound, fmt.Errorf("service not found")) } defer func() { getServiceDetailsFunc = orig }() @@ -115,12 +115,13 @@ func TestResolveConnectionTarget_ReplicaFallback(t *testing.T) { } } -// TestResolveConnectionTarget_UnknownReturnsOriginalError: an ID that is neither -// a service nor a replica surfaces the original service-lookup error. +// TestResolveConnectionTarget_UnknownReturnsOriginalError: a not-found ID that +// matches neither a service nor a replica surfaces the original (service) +// lookup error rather than the replica-scan's not-found error. func TestResolveConnectionTarget_UnknownReturnsOriginalError(t *testing.T) { orig := getServiceDetailsFunc getServiceDetailsFunc = func(cmd *cobra.Command, cfg *common.Config, args []string) (api.Service, error) { - return api.Service{}, fmt.Errorf("original lookup error") + return api.Service{}, common.ExitWithErrorFromStatusCode(http.StatusNotFound, fmt.Errorf("original lookup error")) } defer func() { getServiceDetailsFunc = orig }() @@ -137,6 +138,32 @@ func TestResolveConnectionTarget_UnknownReturnsOriginalError(t *testing.T) { } } +// TestResolveConnectionTarget_NonNotFoundSkipsScan: a non-not-found service +// error (e.g. auth) is surfaced as-is without a replica scan, even though the +// ID would match a replica if scanned. +func TestResolveConnectionTarget_NonNotFoundSkipsScan(t *testing.T) { + orig := getServiceDetailsFunc + getServiceDetailsFunc = func(cmd *cobra.Command, cfg *common.Config, args []string) (api.Service, error) { + return api.Service{}, common.ExitWithErrorFromStatusCode(http.StatusUnauthorized, fmt.Errorf("auth failed")) + } + defer func() { getServiceDetailsFunc = orig }() + + cfg := replicaConnectTestConfig(t, []api.Service{replicaConnectTestService()}) + cmd := &cobra.Command{} + cmd.SetContext(context.Background()) + + _, replica, err := resolveConnectionTarget(cmd, cfg, []string{"rep1234567"}) + if err == nil { + t.Fatal("expected the original auth error, got nil") + } + if replica != nil { + t.Fatal("expected no replica scan for a non-not-found error") + } + if !strings.Contains(err.Error(), "auth failed") { + t.Errorf("expected the original auth error to be surfaced, got %v", err) + } +} + // TestBuildConnectionDetailsForTarget_ReplicaPoolerFallback: requesting --pooled // on a replica with no pooler warns and falls back to a direct connection. func TestBuildConnectionDetailsForTarget_ReplicaPoolerFallback(t *testing.T) { diff --git a/internal/tiger/common/errors.go b/internal/tiger/common/errors.go index 3a42fa57..24f5bdc0 100644 --- a/internal/tiger/common/errors.go +++ b/internal/tiger/common/errors.go @@ -57,6 +57,13 @@ func ExitWithCode(code int, err error) error { return ExitCodeError{code: code, err: err} } +// IsNotFound reports whether err represents an HTTP 404 "not found" API failure, +// as produced by ExitWithErrorFromStatusCode. +func IsNotFound(err error) bool { + var exitErr ExitCodeError + return errors.As(err, &exitErr) && exitErr.ExitCode() == ExitServiceNotFound +} + // ExitWithErrorFromStatusCode maps HTTP status codes to CLI exit codes func ExitWithErrorFromStatusCode(statusCode int, err error) error { if err == nil { diff --git a/internal/tiger/common/replica.go b/internal/tiger/common/replica.go index 002e2443..92c9b627 100644 --- a/internal/tiger/common/replica.go +++ b/internal/tiger/common/replica.go @@ -2,6 +2,7 @@ package common import ( "context" + "errors" "fmt" "net/http" @@ -19,6 +20,11 @@ type ResolvedTarget struct { Replica *api.ReadReplicaSet } +// ErrReplicaNotFound is returned by FindReplicaByID when no replica matches the +// ID — distinct from a match that can't be used or an API failure — so callers +// can fall back instead of surfacing it. +var ErrReplicaNotFound = errors.New("no matching read replica") + // ResolveServiceOrReplica resolves id to a primary service or a read replica of // some service in the project. It tries a direct service lookup first; on // failure it scans services for a replica with that ID (read replicas aren't @@ -36,19 +42,23 @@ func ResolveServiceOrReplica(ctx context.Context, client api.ClientWithResponses return &ResolvedTarget{Service: *resp.JSON200}, nil } - // Not a service — maybe a read replica ID. - if target, scanErr := FindReplicaByID(ctx, client, projectID, id); scanErr == nil { - return target, nil + // Only a not-found might instead be a read replica; other failures aren't + // worth scanning every service for. A matched-but-unusable replica surfaces + // its own error; a true no-match falls through to the service error below. + if resp.StatusCode() == http.StatusNotFound { + if target, scanErr := FindReplicaByID(ctx, client, projectID, id); !errors.Is(scanErr, ErrReplicaNotFound) { + return target, scanErr + } } - // Neither; surface the original service-lookup error. return nil, ExitWithErrorFromStatusCode(resp.StatusCode(), resp.JSON4XX) } -// FindReplicaByID scans the project's services and returns the parent service -// and read replica set whose ID matches id, or an error when no replica in the -// project matches. Use it to fall back to replica resolution once a direct -// service lookup has failed. +// FindReplicaByID scans the project's services for a read replica whose ID +// matches id and returns it with its parent service. A matched replica must be +// active and expose an endpoint; a match that isn't returns a descriptive +// error. It returns ErrReplicaNotFound when no replica matches. Use it to fall +// back to replica resolution once a direct service lookup has failed. func FindReplicaByID(ctx context.Context, client api.ClientWithResponsesInterface, projectID, id string) (*ResolvedTarget, error) { resp, err := client.GetServicesWithResponse(ctx, projectID) if err != nil { @@ -66,12 +76,24 @@ func FindReplicaByID(ctx context.Context, client api.ClientWithResponsesInterfac } replicas := *services[i].ReadReplicaSets for j := range replicas { - if util.DerefStr(replicas[j].Id) == id { - return &ResolvedTarget{Service: services[i], Replica: &replicas[j]}, nil + replica := &replicas[j] + if util.DerefStr(replica.Id) != id { + continue + } + if replica.Status == nil || *replica.Status != api.ReadReplicaSetStatusActive { + status := "unknown" + if replica.Status != nil { + status = string(*replica.Status) + } + return nil, fmt.Errorf("read replica %q is not active (status: %s)", id, status) + } + if replica.Endpoint == nil { + return nil, fmt.Errorf("read replica %q has no endpoint available", id) } + return &ResolvedTarget{Service: services[i], Replica: replica}, nil } } } - return nil, fmt.Errorf("no service or read replica found with ID %q", id) + return nil, fmt.Errorf("no service or read replica found with ID %q: %w", id, ErrReplicaNotFound) } diff --git a/internal/tiger/common/replica_test.go b/internal/tiger/common/replica_test.go index 650a364c..896593cd 100644 --- a/internal/tiger/common/replica_test.go +++ b/internal/tiger/common/replica_test.go @@ -3,6 +3,7 @@ package common import ( "context" "encoding/json" + "errors" "net/http" "net/http/httptest" "strings" @@ -161,7 +162,66 @@ func TestFindReplicaByID(t *testing.T) { t.Errorf("expected to find replica rep1234567, got %+v", target) } - if _, err := FindReplicaByID(context.Background(), client, "proj1", "missing999"); err == nil { - t.Error("expected an error when no replica matches, got nil") + _, err = FindReplicaByID(context.Background(), client, "proj1", "missing999") + if !errors.Is(err, ErrReplicaNotFound) { + t.Errorf("expected ErrReplicaNotFound when no replica matches, got %v", err) + } +} + +// TestFindReplicaByID_NotActive: a matched replica that isn't active returns a +// descriptive error (not ErrReplicaNotFound), so callers surface it. +func TestFindReplicaByID_NotActive(t *testing.T) { + primary := api.Service{ + ServiceId: util.Ptr("svcprimary"), + ProjectId: util.Ptr("proj1"), + ReadReplicaSets: &[]api.ReadReplicaSet{{ + Id: util.Ptr("rep1234567"), + Name: util.Ptr("still-creating"), + Status: util.Ptr(api.ReadReplicaSetStatusCreating), + }}, + } + client := replicaTestClient(t, map[string]api.Service{"svcprimary": primary}, []api.Service{primary}) + + _, err := FindReplicaByID(context.Background(), client, "proj1", "rep1234567") + if err == nil { + t.Fatal("expected an error for a non-active replica, got nil") + } + if errors.Is(err, ErrReplicaNotFound) { + t.Errorf("expected a descriptive not-active error, got ErrReplicaNotFound: %v", err) + } + if !strings.Contains(err.Error(), "not active") { + t.Errorf("expected a 'not active' error, got %v", err) + } +} + +// TestResolveServiceOrReplica_NonNotFoundSkipsScan: a non-404 service lookup +// error is surfaced without listing services to look for a replica. +func TestResolveServiceOrReplica_NonNotFoundSkipsScan(t *testing.T) { + primary := testReplicaService() + listed := false + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + if strings.HasSuffix(r.URL.Path, "/services") { + listed = true + _ = json.NewEncoder(w).Encode([]api.Service{primary}) + return + } + // GET service → permission denied (not a 404). + w.WriteHeader(http.StatusForbidden) + _ = json.NewEncoder(w).Encode(map[string]string{"message": "forbidden"}) + })) + t.Cleanup(srv.Close) + + client, err := api.NewClientWithResponses(srv.URL) + if err != nil { + t.Fatalf("failed to build client: %v", err) + } + + // "rep1234567" would match a replica if scanned, but a 403 must skip the scan. + if _, err := ResolveServiceOrReplica(context.Background(), client, "proj1", "rep1234567"); err == nil { + t.Fatal("expected the 403 error to be surfaced, got nil") + } + if listed { + t.Error("expected no service-list scan for a non-404 service error") } } From 9053d559272aeea01e103a1dc8483ce0314a52f5 Mon Sep 17 00:00:00 2001 From: Alexandra Primakina Date: Wed, 15 Jul 2026 13:20:02 +0200 Subject: [PATCH 3/8] refactor(db): resolve read replicas via forked_from instead of a list scan The API resolves a read replica ID through the normal service-get endpoint, returning the replica's own endpoint plus a forked_from link to its parent (is_standby, service_id). Use that instead of scanning the project's services: - common.ResolveConnectionTarget turns a fetched service into a ConnectionTarget{Connect, Credential, IsReplica}. A standby replica connects to its own endpoint but resolves credentials (and password reset) against the parent primary, fixing the "password authentication failed" error when connecting to a replica by ID. - Removes the ResolveServiceOrReplica/FindReplicaByID list-scan machinery, the IsNotFound helper, GetReplicaConnectionDetails, ConnectToService/ConnectToTarget. - Consolidates the pooler policy: ConnectionTarget.Details owns the build-and-RequirePooler rule; ReplicaPoolerWarning owns the replica guard; resolveConnectTarget handles both the by-ID replica and the interactive menu. - getServiceDetails delegates to the shared common.GetService. Co-Authored-By: Claude Opus 4.8 --- CLAUDE.md | 2 +- internal/tiger/cmd/db.go | 136 ++++-------- internal/tiger/cmd/read_replica.go | 85 ++++--- internal/tiger/cmd/read_replica_test.go | 3 +- internal/tiger/cmd/replica_connect_test.go | 160 ++++++-------- internal/tiger/common/connection.go | 75 +++---- internal/tiger/common/connection_test.go | 38 ++-- internal/tiger/common/errors.go | 7 - internal/tiger/common/replica.go | 144 ++++++------ internal/tiger/common/replica_test.go | 246 +++++++++------------ internal/tiger/common/schema_fetch.go | 15 +- internal/tiger/mcp/db_tools.go | 12 +- specs/spec.md | 2 +- 13 files changed, 392 insertions(+), 533 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 82d4cd94..ab054a83 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -273,7 +273,7 @@ Tiger CLI is a Go-based command-line interface for managing Tiger, the modern da - `service.go` - Service management commands (list, create, get, fork, start, stop, resize, delete, update-password, logs) - `db.go` - Database operation commands (connection-string, connect, test-connection) - `read_replica.go` - Read replica selection flow for `db connect`/`psql`: in an interactive terminal, when the service has one or more active read replicas (listed via the `/replicaSets` API), prompts to connect to the primary or one of the replicas. Skipped when stdin is not a TTY, when `--no-replica-prompt` is set, or when the service has no read replicas. - - Read replica ID as a connection target: the db connection commands (`db connection-string`, `db connect`/`psql`, `db test-connection`, `db schema`) and the `db_execute_query`/`db_schema` MCP tools accept a read replica set ID anywhere a service ID is accepted. Resolution is `common.ResolveServiceOrReplica` (MCP) / `resolveConnectionTarget` in `db.go` (CLI): try the ID as a service first, and on failure scan the project's services' embedded `read_replica_sets` for a matching ID (`common.FindReplicaByID`). A matched replica connects to the replica's endpoint while credentials/password reset resolve against the parent service. + - Read replica ID as a connection target: the db connection commands (`db connection-string`, `db connect`/`psql`, `db test-connection`, `db schema`) and the `db_execute_query`/`db_schema` MCP tools accept a read replica set ID anywhere a service ID is accepted. The API's `GetService` resolves both primary and read replica IDs, returning the replica's own endpoint plus a `forked_from` link (`is_standby: true`, parent `service_id`). `common.ResolveConnectionTarget` turns a fetched `api.Service` into a `common.ConnectionTarget{Connect, Credential, IsReplica}`: a standby replica connects to its own endpoint (`Connect`) but resolves credentials/password reset against the parent primary (`Credential`, fetched via `GetService`), since read replicas share the primary's credentials. MCP handlers call `common.ResolveConnectionTargetByID`; the CLI's `resolveConnectionTarget` fetches via `getServiceDetailsFunc` (the test seam) then calls `ResolveConnectionTarget`. The interactive `db connect` menu builds a target from a `/replicaSets` entry via `common.NewReplicaConnectionTarget`. - `config.go` - Configuration management commands (show, set, unset, reset) - `mcp.go` - MCP server commands (install, start, list, get) - `version.go` - Version command diff --git a/internal/tiger/cmd/db.go b/internal/tiger/cmd/db.go index aef24a78..905c016a 100644 --- a/internal/tiger/cmd/db.go +++ b/internal/tiger/cmd/db.go @@ -6,7 +6,6 @@ import ( "encoding/base64" "errors" "fmt" - "net/http" "os" "os/exec" "strings" @@ -95,12 +94,12 @@ Examples: return err } - service, replica, err := resolveConnectionTarget(cmd, cfg, args) + target, err := resolveConnectionTarget(cmd, cfg, args) if err != nil { return err } - details, err := buildConnectionDetailsForTarget(cmd, service, replica, common.ConnectionDetailsOptions{ + details, err := buildConnectionDetailsForTarget(cmd, target, common.ConnectionDetailsOptions{ Pooled: dbConnectionStringPooled, Role: dbConnectionStringRole, WithPassword: dbConnectionStringWithPassword, @@ -209,7 +208,7 @@ Examples: // Separate service ID from additional psql flags serviceArgs, psqlFlags := separateServiceAndPsqlArgs(cmd, args) - service, replica, err := resolveConnectionTarget(cmd, cfg, serviceArgs) + target, err := resolveConnectionTarget(cmd, cfg, serviceArgs) if err != nil { return err } @@ -226,30 +225,19 @@ Examples: ReadOnly: dbConnectReadOnly || cfg.ReadOnly, } - var details *common.ConnectionDetails - if replica != nil { - // A replica ID was named directly — connect straight to it. - details, err = buildConnectionDetailsForTarget(cmd, service, replica, opts) - if err != nil { - return err - } - fmt.Fprintf(cmd.ErrOrStderr(), "Connecting to read replica '%s'...\n", util.DerefStr(replica.Name)) - } else { - // A primary ID — offer the interactive replica menu (a no-op that - // returns the primary when non-interactive or no replicas exist). - details, err = resolveConnectTarget(cmd.Context(), cmd, cfg.Client, cfg.ProjectID, service, opts, dbConnectNoReplicaPrompt) - if err != nil { - return err - } - if details == nil { - // User cancelled the connection. - return nil - } + // Connects straight to a replica named by ID, or offers the interactive + // replica menu for a primary. Returns nil details if the user cancels. + details, err := resolveConnectTarget(cmd.Context(), cmd, cfg.Client, cfg.ProjectID, target, opts, dbConnectNoReplicaPrompt) + if err != nil { + return err + } + if details == nil { + return nil } - // Replicas share the primary's credentials, so password storage and - // recovery always operate on the primary service. - return connectWithPasswordMenu(cmd.Context(), cmd, cfg.Client, service, details, psqlPath, psqlFlags) + // Read replicas share the primary's credentials, so password storage + // and recovery always operate on the credential service. + return connectWithPasswordMenu(cmd.Context(), cmd, cfg.Client, target.Credential, details, psqlPath, psqlFlags) }, } @@ -309,13 +297,13 @@ Examples: return common.ExitWithCode(common.ExitInvalidParameters, err) } - service, replica, err := resolveConnectionTarget(cmd, cfg, args) + target, err := resolveConnectionTarget(cmd, cfg, args) if err != nil { return common.ExitWithCode(common.ExitInvalidParameters, err) } // Build connection string for testing with password (if available) - details, err := buildConnectionDetailsForTarget(cmd, service, replica, common.ConnectionDetailsOptions{ + details, err := buildConnectionDetailsForTarget(cmd, target, common.ConnectionDetailsOptions{ Pooled: dbTestConnectionPooled, Role: dbTestConnectionRole, WithPassword: true, @@ -859,14 +847,14 @@ Examples: return err } - service, replica, err := resolveConnectionTarget(cmd, cfg, args) + target, err := resolveConnectionTarget(cmd, cfg, args) if err != nil { return err } - warnReplicaPooler(cmd, replica, dbSchemaPooled) + warnReplicaPooler(cmd, target, dbSchemaPooled) - schema, err := common.FetchServiceSchema(cmd.Context(), service, replica, dbSchemaRole, dbSchemaPooled, common.SchemaOptions{ + schema, err := common.FetchServiceSchema(cmd.Context(), target, dbSchemaRole, dbSchemaPooled, common.SchemaOptions{ Schema: dbSchemaSchema, IncludeInternal: dbSchemaInternal, IncludeDefinitions: dbSchemaDefinitions, @@ -909,71 +897,34 @@ func buildDbCmd() *cobra.Command { } // resolveConnectionTarget looks up the target named by args, which may be a -// primary service ID or a read replica set ID. For a replica, the returned -// service is its parent (for credentials) and replica is non-nil. This lets a -// replica ID work anywhere a service ID does across the db connection commands. -func resolveConnectionTarget(cmd *cobra.Command, cfg *common.Config, args []string) (api.Service, *api.ReadReplicaSet, error) { - // Service lookup is the common case (and what tests stub), so try it first. +// primary service ID or a read replica set ID. This lets a replica ID work +// anywhere a service ID does across the db connection commands. +func resolveConnectionTarget(cmd *cobra.Command, cfg *common.Config, args []string) (*common.ConnectionTarget, error) { service, err := getServiceDetailsFunc(cmd, cfg, args) - if err == nil { - return service, nil, nil - } - - // Only a not-found could instead be a read replica; other failures (auth, - // network, 5xx) aren't worth a full service-list scan. - if !common.IsNotFound(err) { - return api.Service{}, nil, err - } - - serviceID, idErr := getServiceID(cfg.Config, args) - if idErr != nil { - return api.Service{}, nil, err + if err != nil { + return nil, err } + // The API resolves both primary and read replica IDs via GetService; a read + // replica comes back linked to its parent, whose credentials it shares. ctx, cancel := context.WithTimeout(cmd.Context(), 30*time.Second) defer cancel() - - target, scanErr := common.FindReplicaByID(ctx, cfg.Client, cfg.ProjectID, serviceID) - switch { - case scanErr == nil: - return target.Service, target.Replica, nil - case errors.Is(scanErr, common.ErrReplicaNotFound): - // Neither a service nor a replica: surface the original not-found error. - return api.Service{}, nil, err - default: - return api.Service{}, nil, scanErr - } + return common.ResolveConnectionTarget(ctx, cfg.Client, cfg.ProjectID, service) } // warnReplicaPooler prints the replica pooler-fallback warning to stderr, if -// any. It is a no-op for the primary (nil replica) or when nothing to warn. -func warnReplicaPooler(cmd *cobra.Command, replica *api.ReadReplicaSet, pooled bool) { - if warning := common.ReplicaPoolerWarning(replica, pooled); warning != "" { +// any. It is a no-op for a primary target or when there's nothing to warn. +func warnReplicaPooler(cmd *cobra.Command, target *common.ConnectionTarget, pooled bool) { + if warning := common.ReplicaPoolerWarning(target, pooled); warning != "" { fmt.Fprintf(cmd.ErrOrStderr(), "⚠️ Warning: %s\n", warning) } } -// buildConnectionDetailsForTarget builds connection details for the primary or -// a read replica. A requested-but-unavailable pooler is a hard error for the -// primary, but a warn-and-fall-back-to-direct for a replica. -func buildConnectionDetailsForTarget(cmd *cobra.Command, service api.Service, replica *api.ReadReplicaSet, opts common.ConnectionDetailsOptions) (*common.ConnectionDetails, error) { - if replica != nil { - warnReplicaPooler(cmd, replica, opts.Pooled) - details, err := common.GetReplicaConnectionDetails(service, *replica, opts) - if err != nil { - return nil, fmt.Errorf("failed to build connection string: %w", err) - } - return details, nil - } - - details, err := common.GetConnectionDetails(service, opts) - if err != nil { - return nil, fmt.Errorf("failed to build connection string: %w", err) - } - if err := details.RequirePooler(opts.Pooled); err != nil { - return nil, err - } - return details, nil +// buildConnectionDetailsForTarget builds connection details for a target, +// warning first when a replica falls back from a requested pooler. +func buildConnectionDetailsForTarget(cmd *cobra.Command, target *common.ConnectionTarget, opts common.ConnectionDetailsOptions) (*common.ConnectionDetails, error) { + warnReplicaPooler(cmd, target, opts.Pooled) + return target.Details(opts) } // getServiceDetails is a helper that handles common service lookup logic and returns the service details @@ -986,25 +937,14 @@ func getServiceDetails(cmd *cobra.Command, cfg *common.Config, args []string) (a cmd.SilenceUsage = true - // Fetch service details ctx, cancel := context.WithTimeout(cmd.Context(), 30*time.Second) defer cancel() - resp, err := cfg.Client.GetServiceWithResponse(ctx, cfg.ProjectID, serviceID) + service, err := common.GetService(ctx, cfg.Client, cfg.ProjectID, serviceID) if err != nil { - return api.Service{}, fmt.Errorf("failed to fetch service details: %w", err) - } - - // Handle API response - if resp.StatusCode() != http.StatusOK { - return api.Service{}, common.ExitWithErrorFromStatusCode(resp.StatusCode(), resp.JSON4XX) - } - - if resp.JSON200 == nil { - return api.Service{}, fmt.Errorf("empty response from API") + return api.Service{}, err } - - return *resp.JSON200, nil + return *service, nil } // ArgsLenAtDashProvider defines the interface for getting ArgsLenAtDash diff --git a/internal/tiger/cmd/read_replica.go b/internal/tiger/cmd/read_replica.go index de255e47..8c027938 100644 --- a/internal/tiger/cmd/read_replica.go +++ b/internal/tiger/cmd/read_replica.go @@ -15,67 +15,64 @@ import ( "github.com/timescale/tiger-cli/internal/tiger/util" ) -// resolveConnectTarget prompts the user to connect to a read replica instead of -// the primary, returning the chosen target (nil if cancelled). It returns the -// primary without prompting when there's nothing to choose: non-interactive -// stdin, prompting disabled, or no connectable replicas. +// resolveConnectTarget resolves the connection details for `tiger db connect`. +// A target that is already a read replica (named directly by ID) connects +// straight to it. A primary, on an interactive terminal, is offered a menu to +// connect to the primary or one of its read replicas. It returns nil details +// when the user cancels. The menu is skipped for non-interactive stdin, when +// prompting is disabled, or when the service has no connectable replicas. func resolveConnectTarget( ctx context.Context, cmd *cobra.Command, client *api.ClientWithResponses, projectID string, - primary api.Service, + target *common.ConnectionTarget, opts common.ConnectionDetailsOptions, noReplicaPrompt bool, ) (*common.ConnectionDetails, error) { - returnPrimary := func() (*common.ConnectionDetails, error) { - return buildConnectionDetailsForTarget(cmd, primary, nil, opts) - } - - // Only prompt on an interactive terminal. - if noReplicaPrompt || !checkStdinIsTTY() { - return returnPrimary() - } - - replicas, err := fetchReplicaSets(ctx, client, projectID, util.DerefStr(primary.ServiceId)) - if err != nil { - // Don't block the connection if we can't list replicas. - fmt.Fprintf(cmd.ErrOrStderr(), "Warning: could not list read replicas: %v\n", err) - return returnPrimary() - } - - // Only active replicas with an endpoint are connectable. - var connectable []api.ReadReplicaSet - for _, r := range replicas { - if r.Status != nil && *r.Status == api.ReadReplicaSetStatusActive && r.Endpoint != nil { - connectable = append(connectable, r) + // chosen is what we connect to; the menu below may replace it with a replica. + chosen := target + + // Offer the replica menu only for a primary on an interactive terminal. + if !target.IsReplica && !noReplicaPrompt && checkStdinIsTTY() { + primary := target.Connect + replicas, err := fetchReplicaSets(ctx, client, projectID, util.DerefStr(primary.ServiceId)) + if err != nil { + // Don't block the connection if we can't list replicas. + fmt.Fprintf(cmd.ErrOrStderr(), "Warning: could not list read replicas: %v\n", err) + } else if connectable := connectableReplicas(replicas); len(connectable) > 0 { + choice, err := selectConnectTargetOption(cmd.ErrOrStderr(), primary, connectable) + if err != nil { + return nil, err + } + switch choice.kind { + case targetCancel: + return nil, nil + case targetReplica: + chosen = common.NewReplicaConnectionTarget(primary, *choice.replica) + } } } - // Nothing to choose between, so skip the menu and use the primary. - if len(connectable) == 0 { - return returnPrimary() - } - - choice, err := selectConnectTargetOption(cmd.ErrOrStderr(), primary, connectable) + details, err := buildConnectionDetailsForTarget(cmd, chosen, opts) if err != nil { return nil, err } - - switch choice.kind { - case targetCancel: - return nil, nil - case targetPrimary: - return returnPrimary() + if chosen.IsReplica { + fmt.Fprintf(cmd.ErrOrStderr(), "Connecting to read replica '%s'...\n", util.DerefStr(chosen.Connect.Name)) } + return details, nil +} - replica := choice.replica - details, err := buildConnectionDetailsForTarget(cmd, primary, replica, opts) - if err != nil { - return nil, err +// connectableReplicas filters to active read replicas that expose an endpoint. +func connectableReplicas(replicas []api.ReadReplicaSet) []api.ReadReplicaSet { + var out []api.ReadReplicaSet + for _, r := range replicas { + if r.Status != nil && *r.Status == api.ReadReplicaSetStatusActive && r.Endpoint != nil { + out = append(out, r) + } } - fmt.Fprintf(cmd.ErrOrStderr(), "Connecting to read replica '%s'...\n", util.DerefStr(replica.Name)) - return details, nil + return out } // fetchReplicaSets retrieves the read replica sets for a service. diff --git a/internal/tiger/cmd/read_replica_test.go b/internal/tiger/cmd/read_replica_test.go index fe8e0929..b3dd88c5 100644 --- a/internal/tiger/cmd/read_replica_test.go +++ b/internal/tiger/cmd/read_replica_test.go @@ -125,7 +125,8 @@ func TestResolveConnectTarget_NoReplicasSkipsPrompt(t *testing.T) { cmd.SetOut(io.Discard) cmd.SetErr(io.Discard) - details, err := resolveConnectTarget(context.Background(), cmd, client, "proj-1", primary, + target := &common.ConnectionTarget{Connect: primary, Credential: primary} + details, err := resolveConnectTarget(context.Background(), cmd, client, "proj-1", target, common.ConnectionDetailsOptions{Role: "tsdbadmin"}, false /*noReplicaPrompt*/) if err != nil { t.Fatalf("unexpected error: %v", err) diff --git a/internal/tiger/cmd/replica_connect_test.go b/internal/tiger/cmd/replica_connect_test.go index 93f0b0a1..36052ce0 100644 --- a/internal/tiger/cmd/replica_connect_test.go +++ b/internal/tiger/cmd/replica_connect_test.go @@ -19,21 +19,21 @@ import ( "github.com/timescale/tiger-cli/internal/tiger/util" ) -// replicaConnectTestConfig builds a Config whose client is backed by an -// httptest server that 404s on getService (so resolution falls back to the -// scan) and returns the given services from the list endpoint. -func replicaConnectTestConfig(t *testing.T, list []api.Service) *common.Config { +// serviceClientConfig builds a Config whose client serves the getService +// endpoint from the given services keyed by ID (404 when absent). +func serviceClientConfig(t *testing.T, services map[string]api.Service) *common.Config { t.Helper() srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.Header().Set("Content-Type", "application/json") - if strings.HasSuffix(r.URL.Path, "/services") { - _ = json.NewEncoder(w).Encode(list) + parts := strings.Split(strings.Trim(r.URL.Path, "/"), "/") + svc, ok := services[parts[len(parts)-1]] + if !ok { + w.WriteHeader(http.StatusNotFound) + _ = json.NewEncoder(w).Encode(map[string]string{"message": "service not found"}) return } - // getService for a replica ID is a miss. - w.WriteHeader(http.StatusNotFound) - _ = json.NewEncoder(w).Encode(map[string]string{"message": "service not found"}) + _ = json.NewEncoder(w).Encode(svc) })) t.Cleanup(srv.Close) @@ -44,28 +44,39 @@ func replicaConnectTestConfig(t *testing.T, list []api.Service) *common.Config { return &common.Config{Config: &config.Config{}, ProjectID: "proj1", Client: client} } -func replicaConnectTestService() api.Service { - rhost := "replica.example.com" - rport := 5432 +func primarySvc() api.Service { + host := "svcprimary.example.com" + port := 5432 return api.Service{ ServiceId: util.Ptr("svcprimary"), ProjectId: util.Ptr("proj1"), Name: util.Ptr("my-db"), - ReadReplicaSets: &[]api.ReadReplicaSet{{ - Id: util.Ptr("rep1234567"), - Name: util.Ptr("reporting-replica"), - Status: util.Ptr(api.ReadReplicaSetStatusActive), - Endpoint: &api.Endpoint{Host: &rhost, Port: &rport}, - }}, + Endpoint: &api.Endpoint{Host: &host, Port: &port}, } } -// TestResolveConnectionTarget_Primary: when the service lookup succeeds, the ID -// is a primary and no replica is returned. +func standbySvc() api.Service { + host := "replica.example.com" + port := 5432 + return api.Service{ + ServiceId: util.Ptr("rep1234567"), + ProjectId: util.Ptr("proj1"), + Name: util.Ptr("reporting-replica"), + Endpoint: &api.Endpoint{Host: &host, Port: &port}, + ForkedFrom: &api.ForkSpec{ + IsStandby: util.Ptr(true), + ProjectId: util.Ptr("proj1"), + ServiceId: util.Ptr("svcprimary"), + }, + } +} + +// TestResolveConnectionTarget_Primary: a plain service resolves to a primary +// target (connect == credential, no parent fetch, so no client needed). func TestResolveConnectionTarget_Primary(t *testing.T) { orig := getServiceDetailsFunc getServiceDetailsFunc = func(cmd *cobra.Command, cfg *common.Config, args []string) (api.Service, error) { - return api.Service{ServiceId: util.Ptr("svcprimary")}, nil + return primarySvc(), nil } defer func() { getServiceDetailsFunc = orig }() @@ -73,94 +84,60 @@ func TestResolveConnectionTarget_Primary(t *testing.T) { cmd := &cobra.Command{} cmd.SetContext(context.Background()) - service, replica, err := resolveConnectionTarget(cmd, cfg, []string{"svcprimary"}) + target, err := resolveConnectionTarget(cmd, cfg, []string{"svcprimary"}) if err != nil { t.Fatalf("unexpected error: %v", err) } - if replica != nil { - t.Fatalf("expected no replica for a primary ID, got %+v", replica) + if target.IsReplica { + t.Fatal("expected a primary target") } - if util.DerefStr(service.ServiceId) != "svcprimary" { - t.Errorf("expected primary svcprimary, got %q", util.DerefStr(service.ServiceId)) + if util.DerefStr(target.Connect.ServiceId) != "svcprimary" { + t.Errorf("expected connect svcprimary, got %q", util.DerefStr(target.Connect.ServiceId)) } } -// TestResolveConnectionTarget_ReplicaFallback: when the service lookup fails, -// the ID is scanned against the project's read replicas, and the matching -// replica plus its parent service is returned. -func TestResolveConnectionTarget_ReplicaFallback(t *testing.T) { +// TestResolveConnectionTarget_Replica: a standby service connects to the replica +// but resolves credentials against the parent (fetched via the client). +func TestResolveConnectionTarget_Replica(t *testing.T) { orig := getServiceDetailsFunc getServiceDetailsFunc = func(cmd *cobra.Command, cfg *common.Config, args []string) (api.Service, error) { - return api.Service{}, common.ExitWithErrorFromStatusCode(http.StatusNotFound, fmt.Errorf("service not found")) + return standbySvc(), nil } defer func() { getServiceDetailsFunc = orig }() - cfg := replicaConnectTestConfig(t, []api.Service{replicaConnectTestService()}) + cfg := serviceClientConfig(t, map[string]api.Service{"svcprimary": primarySvc()}) cmd := &cobra.Command{} cmd.SetContext(context.Background()) - service, replica, err := resolveConnectionTarget(cmd, cfg, []string{"rep1234567"}) + target, err := resolveConnectionTarget(cmd, cfg, []string{"rep1234567"}) if err != nil { t.Fatalf("unexpected error: %v", err) } - if replica == nil { - t.Fatal("expected a replica to be resolved, got nil") - } - if util.DerefStr(replica.Id) != "rep1234567" { - t.Errorf("expected replica rep1234567, got %q", util.DerefStr(replica.Id)) - } - // Parent service (used for credentials) is returned alongside the replica. - if util.DerefStr(service.ServiceId) != "svcprimary" { - t.Errorf("expected parent svcprimary, got %q", util.DerefStr(service.ServiceId)) + if !target.IsReplica { + t.Fatal("expected a replica target") } -} - -// TestResolveConnectionTarget_UnknownReturnsOriginalError: a not-found ID that -// matches neither a service nor a replica surfaces the original (service) -// lookup error rather than the replica-scan's not-found error. -func TestResolveConnectionTarget_UnknownReturnsOriginalError(t *testing.T) { - orig := getServiceDetailsFunc - getServiceDetailsFunc = func(cmd *cobra.Command, cfg *common.Config, args []string) (api.Service, error) { - return api.Service{}, common.ExitWithErrorFromStatusCode(http.StatusNotFound, fmt.Errorf("original lookup error")) + if util.DerefStr(target.Connect.ServiceId) != "rep1234567" { + t.Errorf("expected connect rep1234567, got %q", util.DerefStr(target.Connect.ServiceId)) } - defer func() { getServiceDetailsFunc = orig }() - - cfg := replicaConnectTestConfig(t, []api.Service{replicaConnectTestService()}) - cmd := &cobra.Command{} - cmd.SetContext(context.Background()) - - _, _, err := resolveConnectionTarget(cmd, cfg, []string{"missing999"}) - if err == nil { - t.Fatal("expected an error for an unknown ID, got nil") - } - if !strings.Contains(err.Error(), "original lookup error") { - t.Errorf("expected the original lookup error to be surfaced, got %v", err) + if util.DerefStr(target.Credential.ServiceId) != "svcprimary" { + t.Errorf("expected credential svcprimary, got %q", util.DerefStr(target.Credential.ServiceId)) } } -// TestResolveConnectionTarget_NonNotFoundSkipsScan: a non-not-found service -// error (e.g. auth) is surfaced as-is without a replica scan, even though the -// ID would match a replica if scanned. -func TestResolveConnectionTarget_NonNotFoundSkipsScan(t *testing.T) { +// TestResolveConnectionTarget_LookupError: a service-lookup failure is surfaced. +func TestResolveConnectionTarget_LookupError(t *testing.T) { orig := getServiceDetailsFunc getServiceDetailsFunc = func(cmd *cobra.Command, cfg *common.Config, args []string) (api.Service, error) { - return api.Service{}, common.ExitWithErrorFromStatusCode(http.StatusUnauthorized, fmt.Errorf("auth failed")) + return api.Service{}, fmt.Errorf("lookup failed") } defer func() { getServiceDetailsFunc = orig }() - cfg := replicaConnectTestConfig(t, []api.Service{replicaConnectTestService()}) + cfg := &common.Config{Config: &config.Config{}, ProjectID: "proj1"} cmd := &cobra.Command{} cmd.SetContext(context.Background()) - _, replica, err := resolveConnectionTarget(cmd, cfg, []string{"rep1234567"}) - if err == nil { - t.Fatal("expected the original auth error, got nil") - } - if replica != nil { - t.Fatal("expected no replica scan for a non-not-found error") - } - if !strings.Contains(err.Error(), "auth failed") { - t.Errorf("expected the original auth error to be surfaced, got %v", err) + if _, err := resolveConnectionTarget(cmd, cfg, []string{"x"}); err == nil { + t.Fatal("expected an error, got nil") } } @@ -169,22 +146,22 @@ func TestResolveConnectionTarget_NonNotFoundSkipsScan(t *testing.T) { func TestBuildConnectionDetailsForTarget_ReplicaPoolerFallback(t *testing.T) { rhost := "replica.example.com" rport := 5432 - replica := &api.ReadReplicaSet{ - Id: util.Ptr("rep1234567"), - Name: util.Ptr("reporting-replica"), - Endpoint: &api.Endpoint{Host: &rhost, Port: &rport}, + target := &common.ConnectionTarget{ + Connect: api.Service{ + ServiceId: util.Ptr("rep1234567"), + Name: util.Ptr("reporting-replica"), + Endpoint: &api.Endpoint{Host: &rhost, Port: &rport}, + }, + Credential: api.Service{ServiceId: util.Ptr("svcprimary"), ProjectId: util.Ptr("proj1")}, + IsReplica: true, } - primary := api.Service{ServiceId: util.Ptr("svcprimary"), ProjectId: util.Ptr("proj1")} buf := &bytes.Buffer{} cmd := &cobra.Command{} cmd.SetErr(buf) cmd.SetOut(io.Discard) - details, err := buildConnectionDetailsForTarget(cmd, primary, replica, common.ConnectionDetailsOptions{ - Pooled: true, - Role: "tsdbadmin", - }) + details, err := buildConnectionDetailsForTarget(cmd, target, common.ConnectionDetailsOptions{Pooled: true, Role: "tsdbadmin"}) if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -204,20 +181,17 @@ func TestBuildConnectionDetailsForTarget_ReplicaPoolerFallback(t *testing.T) { func TestBuildConnectionDetailsForTarget_PrimaryRequiresPooler(t *testing.T) { host := "primary.example.com" port := 5432 - primary := api.Service{ + svc := api.Service{ ServiceId: util.Ptr("svcprimary"), Endpoint: &api.Endpoint{Host: &host, Port: &port}, } + target := &common.ConnectionTarget{Connect: svc, Credential: svc} cmd := &cobra.Command{} cmd.SetErr(io.Discard) cmd.SetOut(io.Discard) - _, err := buildConnectionDetailsForTarget(cmd, primary, nil, common.ConnectionDetailsOptions{ - Pooled: true, - Role: "tsdbadmin", - }) - if err == nil { + if _, err := buildConnectionDetailsForTarget(cmd, target, common.ConnectionDetailsOptions{Pooled: true, Role: "tsdbadmin"}); err == nil { t.Fatal("expected an error when a pooler is unavailable for the primary, got nil") } } diff --git a/internal/tiger/common/connection.go b/internal/tiger/common/connection.go index 753e2fab..bed0d958 100644 --- a/internal/tiger/common/connection.go +++ b/internal/tiger/common/connection.go @@ -10,19 +10,19 @@ import ( "github.com/timescale/tiger-cli/internal/tiger/util" ) -// replicaHasPooler reports whether a read replica exposes a pooler endpoint. -func replicaHasPooler(replica *api.ReadReplicaSet) bool { - return replica != nil && replica.ConnectionPooler != nil && replica.ConnectionPooler.Endpoint != nil +// hasPooler reports whether a connection pooler exposes an endpoint. +func hasPooler(pooler *api.ConnectionPooler) bool { + return pooler != nil && pooler.Endpoint != nil } -// ReplicaPoolerWarning returns the single-sourced warning shown when pooling was -// requested for a replica with no pooler (the connection then falls back to -// direct, handled by buildConnectionDetails). -func ReplicaPoolerWarning(replica *api.ReadReplicaSet, pooled bool) string { - if pooled && replica != nil && !replicaHasPooler(replica) { - return fmt.Sprintf("read replica %q has no connection pooler; connecting directly instead", util.DerefStr(replica.Name)) +// ReplicaPoolerWarning returns the warning to show when pooling was requested +// for a read replica with no pooler (the connection falls back to direct), or "" +// otherwise — including for a non-replica target, so callers need no IsReplica guard. +func ReplicaPoolerWarning(target *ConnectionTarget, pooled bool) string { + if !target.IsReplica || !pooled || hasPooler(target.Connect.ConnectionPooler) { + return "" } - return "" + return fmt.Sprintf("read replica %q has no connection pooler; connecting directly instead", util.DerefStr(target.Connect.Name)) } // ConnectionDetailsOptions configures how the connection string is built @@ -74,42 +74,27 @@ func (d *ConnectionDetails) RequirePooler(requested bool) error { const readOnlyConnectionOption = "options=-c%20tsdb_admin.read_only_connection%3Dtrue" func GetConnectionDetails(service api.Service, opts ConnectionDetailsOptions) (*ConnectionDetails, error) { - if service.Endpoint == nil { - return nil, fmt.Errorf("service endpoint not available") - } - return buildConnectionDetails(service.Endpoint, service.ConnectionPooler, service, opts) + return GetConnectionDetailsFor(service, service, opts) } -// ConnectToService resolves the service's connection details and opens a pgx -// connection using the given query execution mode. It is the shared -// service-to-connection path used by the query and schema tools. The caller -// owns the returned connection and must Close it. -func ConnectToService(ctx context.Context, service api.Service, opts ConnectionDetailsOptions, mode pgx.QueryExecMode) (*pgx.Conn, error) { - details, err := GetConnectionDetails(service, opts) - if err != nil { - return nil, fmt.Errorf("failed to build connection string: %w", err) - } - if err := details.RequirePooler(opts.Pooled); err != nil { - return nil, err +// GetConnectionDetailsFor builds connection details using connService for the +// endpoint/pooler and credService for the password lookup. For a primary the +// two are the same; for a read replica connService is the replica (its own +// endpoint) and credService is the parent primary whose credentials it shares. +func GetConnectionDetailsFor(connService, credService api.Service, opts ConnectionDetailsOptions) (*ConnectionDetails, error) { + if connService.Endpoint == nil { + return nil, fmt.Errorf("service endpoint not available") } - - return connectWithDetails(ctx, details, mode) + return buildConnectionDetails(connService.Endpoint, connService.ConnectionPooler, credService, opts) } -// ConnectToTarget opens a pgx connection to the primary service, or to a read -// replica when replica is non-nil (credentials still resolve against the -// primary). A requested pooler that the replica lacks falls back to a direct -// connection; this function does not warn — callers surface that via -// common.ReplicaPoolerWarning. The caller owns the returned connection and must -// Close it. -func ConnectToTarget(ctx context.Context, primary api.Service, replica *api.ReadReplicaSet, opts ConnectionDetailsOptions, mode pgx.QueryExecMode) (*pgx.Conn, error) { - if replica == nil { - return ConnectToService(ctx, primary, opts, mode) - } - - details, err := GetReplicaConnectionDetails(primary, *replica, opts) +// ConnectTarget opens a pgx connection to the target (see +// ConnectionTarget.Details for the pooler policy). The caller owns the returned +// connection and must Close it. +func ConnectTarget(ctx context.Context, target *ConnectionTarget, opts ConnectionDetailsOptions, mode pgx.QueryExecMode) (*pgx.Conn, error) { + details, err := target.Details(opts) if err != nil { - return nil, fmt.Errorf("failed to build connection string: %w", err) + return nil, err } return connectWithDetails(ctx, details, mode) } @@ -126,16 +111,6 @@ func connectWithDetails(ctx context.Context, details *ConnectionDetails, mode pg return pgx.ConnectConfig(ctx, connConfig) } -// GetReplicaConnectionDetails builds connection details for a read replica set. -// Host/port come from the replica's endpoint, but the password is looked up via -// the primary, since replicas share the primary's credentials. -func GetReplicaConnectionDetails(primary api.Service, replica api.ReadReplicaSet, opts ConnectionDetailsOptions) (*ConnectionDetails, error) { - if replica.Endpoint == nil { - return nil, fmt.Errorf("read replica endpoint not available") - } - return buildConnectionDetails(replica.Endpoint, replica.ConnectionPooler, primary, opts) -} - // buildConnectionDetails selects the endpoint (pooler when requested and // available, otherwise direct) and assembles the connection details. The // password, if requested, is looked up against passwordService. diff --git a/internal/tiger/common/connection_test.go b/internal/tiger/common/connection_test.go index 54711777..2db2f3ad 100644 --- a/internal/tiger/common/connection_test.go +++ b/internal/tiger/common/connection_test.go @@ -475,13 +475,15 @@ func TestBuildConnectionString_WithPassword_InvalidServiceEndpoint(t *testing.T) } } -func TestGetReplicaConnectionDetails(t *testing.T) { +func TestGetConnectionDetailsFor(t *testing.T) { primaryHost := "primary.example.com" replicaHost := "replica.example.com" poolerHost := "replica-pooler.example.com" port := 5432 poolerPort := 6432 + // credService supplies credentials only; endpoint selection is driven by + // connService. WithPassword is off here, so credService is not exercised. primary := api.Service{ ServiceId: util.Ptr("svc-primary"), ProjectId: util.Ptr("proj-1"), @@ -492,13 +494,13 @@ func TestGetReplicaConnectionDetails(t *testing.T) { } t.Run("direct endpoint", func(t *testing.T) { - replica := api.ReadReplicaSet{ - Id: util.Ptr("rep-1"), - Name: util.Ptr("my-replica"), - Endpoint: &api.Endpoint{Host: &replicaHost, Port: &port}, + conn := api.Service{ + ServiceId: util.Ptr("rep-1"), + Name: util.Ptr("my-replica"), + Endpoint: &api.Endpoint{Host: &replicaHost, Port: &port}, } - details, err := GetReplicaConnectionDetails(primary, replica, ConnectionDetailsOptions{Role: "tsdbadmin"}) + details, err := GetConnectionDetailsFor(conn, primary, ConnectionDetailsOptions{Role: "tsdbadmin"}) if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -517,16 +519,16 @@ func TestGetReplicaConnectionDetails(t *testing.T) { }) t.Run("pooled endpoint when available", func(t *testing.T) { - replica := api.ReadReplicaSet{ - Id: util.Ptr("rep-1"), - Name: util.Ptr("my-replica"), - Endpoint: &api.Endpoint{Host: &replicaHost, Port: &port}, + conn := api.Service{ + ServiceId: util.Ptr("rep-1"), + Name: util.Ptr("my-replica"), + Endpoint: &api.Endpoint{Host: &replicaHost, Port: &port}, ConnectionPooler: &api.ConnectionPooler{ Endpoint: &api.Endpoint{Host: &poolerHost, Port: &poolerPort}, }, } - details, err := GetReplicaConnectionDetails(primary, replica, ConnectionDetailsOptions{Role: "tsdbadmin", Pooled: true}) + details, err := GetConnectionDetailsFor(conn, primary, ConnectionDetailsOptions{Role: "tsdbadmin", Pooled: true}) if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -539,12 +541,12 @@ func TestGetReplicaConnectionDetails(t *testing.T) { }) t.Run("falls back to direct when pooler requested but unavailable", func(t *testing.T) { - replica := api.ReadReplicaSet{ - Id: util.Ptr("rep-1"), - Endpoint: &api.Endpoint{Host: &replicaHost, Port: &port}, + conn := api.Service{ + ServiceId: util.Ptr("rep-1"), + Endpoint: &api.Endpoint{Host: &replicaHost, Port: &port}, } - details, err := GetReplicaConnectionDetails(primary, replica, ConnectionDetailsOptions{Role: "tsdbadmin", Pooled: true}) + details, err := GetConnectionDetailsFor(conn, primary, ConnectionDetailsOptions{Role: "tsdbadmin", Pooled: true}) if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -557,9 +559,9 @@ func TestGetReplicaConnectionDetails(t *testing.T) { }) t.Run("error when endpoint missing", func(t *testing.T) { - replica := api.ReadReplicaSet{Id: util.Ptr("rep-1")} - if _, err := GetReplicaConnectionDetails(primary, replica, ConnectionDetailsOptions{Role: "tsdbadmin"}); err == nil { - t.Fatal("expected error for missing replica endpoint") + conn := api.Service{ServiceId: util.Ptr("rep-1")} + if _, err := GetConnectionDetailsFor(conn, primary, ConnectionDetailsOptions{Role: "tsdbadmin"}); err == nil { + t.Fatal("expected error for missing connection endpoint") } }) } diff --git a/internal/tiger/common/errors.go b/internal/tiger/common/errors.go index 24f5bdc0..3a42fa57 100644 --- a/internal/tiger/common/errors.go +++ b/internal/tiger/common/errors.go @@ -57,13 +57,6 @@ func ExitWithCode(code int, err error) error { return ExitCodeError{code: code, err: err} } -// IsNotFound reports whether err represents an HTTP 404 "not found" API failure, -// as produced by ExitWithErrorFromStatusCode. -func IsNotFound(err error) bool { - var exitErr ExitCodeError - return errors.As(err, &exitErr) && exitErr.ExitCode() == ExitServiceNotFound -} - // ExitWithErrorFromStatusCode maps HTTP status codes to CLI exit codes func ExitWithErrorFromStatusCode(statusCode int, err error) error { if err == nil { diff --git a/internal/tiger/common/replica.go b/internal/tiger/common/replica.go index 92c9b627..4eb15715 100644 --- a/internal/tiger/common/replica.go +++ b/internal/tiger/common/replica.go @@ -2,7 +2,6 @@ package common import ( "context" - "errors" "fmt" "net/http" @@ -10,90 +9,95 @@ import ( "github.com/timescale/tiger-cli/internal/tiger/util" ) -// ResolvedTarget is a connection ID resolved to either a primary service or one -// of its read replica sets. Service is always the parent (credentials and -// password resets operate on it, since replicas share the primary's -// credentials); Replica is non-nil when the ID named a read replica, whose -// endpoint connections should target. -type ResolvedTarget struct { - Service api.Service - Replica *api.ReadReplicaSet +// ConnectionTarget describes where to connect and whose credentials to use. +// Connect supplies the endpoint/pooler, Credential the password — the same +// service for a primary; for a read replica, Connect is the replica and +// Credential is the parent primary whose credentials it shares. +type ConnectionTarget struct { + Connect api.Service + Credential api.Service + IsReplica bool } -// ErrReplicaNotFound is returned by FindReplicaByID when no replica matches the -// ID — distinct from a match that can't be used or an API failure — so callers -// can fall back instead of surfacing it. -var ErrReplicaNotFound = errors.New("no matching read replica") +// Details builds connection details for the target — endpoint/pooler from +// Connect, password from Credential. A requested-but-unavailable pooler is a +// hard error for a primary but silently falls back to direct for a replica. +func (t *ConnectionTarget) Details(opts ConnectionDetailsOptions) (*ConnectionDetails, error) { + details, err := GetConnectionDetailsFor(t.Connect, t.Credential, opts) + if err != nil { + return nil, fmt.Errorf("failed to build connection string: %w", err) + } + if !t.IsReplica { + if err := details.RequirePooler(opts.Pooled); err != nil { + return nil, err + } + } + return details, nil +} -// ResolveServiceOrReplica resolves id to a primary service or a read replica of -// some service in the project. It tries a direct service lookup first; on -// failure it scans services for a replica with that ID (read replicas aren't -// addressable on their own). If neither matches, the original service-lookup -// error is returned. -func ResolveServiceOrReplica(ctx context.Context, client api.ClientWithResponsesInterface, projectID, id string) (*ResolvedTarget, error) { +// GetService fetches a single service by ID. The API resolves both primary +// service IDs and read replica set IDs here; a read replica comes back as a +// service whose endpoint is the replica's and whose ForkedFrom links to its +// parent. +func GetService(ctx context.Context, client api.ClientWithResponsesInterface, projectID, id string) (*api.Service, error) { resp, err := client.GetServiceWithResponse(ctx, projectID, id) if err != nil { return nil, fmt.Errorf("failed to fetch service details: %w", err) } - if resp.StatusCode() == http.StatusOK { - if resp.JSON200 == nil { - return nil, fmt.Errorf("empty response from API") - } - return &ResolvedTarget{Service: *resp.JSON200}, nil + if resp.StatusCode() != http.StatusOK { + return nil, ExitWithErrorFromStatusCode(resp.StatusCode(), resp.JSON4XX) + } + if resp.JSON200 == nil { + return nil, fmt.Errorf("empty response from API") } + return resp.JSON200, nil +} - // Only a not-found might instead be a read replica; other failures aren't - // worth scanning every service for. A matched-but-unusable replica surfaces - // its own error; a true no-match falls through to the service error below. - if resp.StatusCode() == http.StatusNotFound { - if target, scanErr := FindReplicaByID(ctx, client, projectID, id); !errors.Is(scanErr, ErrReplicaNotFound) { - return target, scanErr - } +// ResolveConnectionTarget turns a fetched service into a ConnectionTarget. When +// the service is a standby read replica (ForkedFrom.IsStandby), it connects to +// the replica but resolves credentials against the parent primary, which is +// fetched here. Everything else (primary, or an independent fork with its own +// credentials) connects with its own credentials. +func ResolveConnectionTarget(ctx context.Context, client api.ClientWithResponsesInterface, projectID string, service api.Service) (*ConnectionTarget, error) { + fork := service.ForkedFrom + if fork == nil || !util.Deref(fork.IsStandby) { + return &ConnectionTarget{Connect: service, Credential: service}, nil } - return nil, ExitWithErrorFromStatusCode(resp.StatusCode(), resp.JSON4XX) -} + parentID := util.DerefStr(fork.ServiceId) + if parentID == "" { + return &ConnectionTarget{Connect: service, Credential: service, IsReplica: true}, nil + } -// FindReplicaByID scans the project's services for a read replica whose ID -// matches id and returns it with its parent service. A matched replica must be -// active and expose an endpoint; a match that isn't returns a descriptive -// error. It returns ErrReplicaNotFound when no replica matches. Use it to fall -// back to replica resolution once a direct service lookup has failed. -func FindReplicaByID(ctx context.Context, client api.ClientWithResponsesInterface, projectID, id string) (*ResolvedTarget, error) { - resp, err := client.GetServicesWithResponse(ctx, projectID) + parent, err := GetService(ctx, client, projectID, parentID) if err != nil { - return nil, fmt.Errorf("failed to list services: %w", err) - } - if resp.StatusCode() != http.StatusOK { - return nil, ExitWithErrorFromStatusCode(resp.StatusCode(), resp.JSON4XX) + return nil, fmt.Errorf("failed to fetch parent service %q for read replica: %w", parentID, err) } + return &ConnectionTarget{Connect: service, Credential: *parent, IsReplica: true}, nil +} - if resp.JSON200 != nil { - services := *resp.JSON200 - for i := range services { - if services[i].ReadReplicaSets == nil { - continue - } - replicas := *services[i].ReadReplicaSets - for j := range replicas { - replica := &replicas[j] - if util.DerefStr(replica.Id) != id { - continue - } - if replica.Status == nil || *replica.Status != api.ReadReplicaSetStatusActive { - status := "unknown" - if replica.Status != nil { - status = string(*replica.Status) - } - return nil, fmt.Errorf("read replica %q is not active (status: %s)", id, status) - } - if replica.Endpoint == nil { - return nil, fmt.Errorf("read replica %q has no endpoint available", id) - } - return &ResolvedTarget{Service: services[i], Replica: replica}, nil - } - } +// ResolveConnectionTargetByID fetches a service (which may be a read replica) by +// ID and resolves its ConnectionTarget. +func ResolveConnectionTargetByID(ctx context.Context, client api.ClientWithResponsesInterface, projectID, id string) (*ConnectionTarget, error) { + service, err := GetService(ctx, client, projectID, id) + if err != nil { + return nil, err } + return ResolveConnectionTarget(ctx, client, projectID, *service) +} - return nil, fmt.Errorf("no service or read replica found with ID %q: %w", id, ErrReplicaNotFound) +// NewReplicaConnectionTarget builds a ConnectionTarget for connecting to one of +// a service's read replica sets (as listed via the /replicaSets endpoint). The +// replica supplies the endpoint; the primary supplies the credentials. +func NewReplicaConnectionTarget(primary api.Service, replica api.ReadReplicaSet) *ConnectionTarget { + return &ConnectionTarget{ + Connect: api.Service{ + ServiceId: replica.Id, + Name: replica.Name, + Endpoint: replica.Endpoint, + ConnectionPooler: replica.ConnectionPooler, + }, + Credential: primary, + IsReplica: true, + } } diff --git a/internal/tiger/common/replica_test.go b/internal/tiger/common/replica_test.go index 896593cd..a6c54488 100644 --- a/internal/tiger/common/replica_test.go +++ b/internal/tiger/common/replica_test.go @@ -3,7 +3,6 @@ package common import ( "context" "encoding/json" - "errors" "net/http" "net/http/httptest" "strings" @@ -13,22 +12,14 @@ import ( "github.com/timescale/tiger-cli/internal/tiger/util" ) -// replicaTestClient builds an API client backed by an httptest server that -// serves the getService endpoint from services (keyed by service ID, 404 when -// absent) and the getServices list endpoint from list. -func replicaTestClient(t *testing.T, services map[string]api.Service, list []api.Service) *api.ClientWithResponses { +// serviceTestClient builds an API client backed by an httptest server that +// serves the getService endpoint from services keyed by service ID (404 when +// absent). +func serviceTestClient(t *testing.T, services map[string]api.Service) *api.ClientWithResponses { t.Helper() srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.Header().Set("Content-Type", "application/json") - - // List: /projects/{project_id}/services - if strings.HasSuffix(r.URL.Path, "/services") { - _ = json.NewEncoder(w).Encode(list) - return - } - - // Get: /projects/{project_id}/services/{service_id} parts := strings.Split(strings.Trim(r.URL.Path, "/"), "/") id := parts[len(parts)-1] svc, ok := services[id] @@ -48,180 +39,163 @@ func replicaTestClient(t *testing.T, services map[string]api.Service, list []api return client } -func testReplicaService() api.Service { - rhost := "replica.example.com" - rport := 5432 +func primaryService() api.Service { + host := "svcprimary.example.com" + port := 5432 return api.Service{ ServiceId: util.Ptr("svcprimary"), ProjectId: util.Ptr("proj1"), Name: util.Ptr("my-db"), - ReadReplicaSets: &[]api.ReadReplicaSet{ - { - Id: util.Ptr("rep1234567"), - Name: util.Ptr("reporting-replica"), - Status: util.Ptr(api.ReadReplicaSetStatusActive), - Endpoint: &api.Endpoint{Host: &rhost, Port: &rport}, - }, + Endpoint: &api.Endpoint{Host: &host, Port: &port}, + } +} + +func replicaService() api.Service { + host := "replica.example.com" + port := 5432 + return api.Service{ + ServiceId: util.Ptr("rep1234567"), + ProjectId: util.Ptr("proj1"), + Name: util.Ptr("reporting-replica"), + Endpoint: &api.Endpoint{Host: &host, Port: &port}, + ForkedFrom: &api.ForkSpec{ + IsStandby: util.Ptr(true), + ProjectId: util.Ptr("proj1"), + ServiceId: util.Ptr("svcprimary"), }, } } -func TestResolveServiceOrReplica_Primary(t *testing.T) { - primary := testReplicaService() - client := replicaTestClient(t, map[string]api.Service{"svcprimary": primary}, []api.Service{primary}) +func TestResolveConnectionTarget_Primary(t *testing.T) { + primary := primaryService() + client := serviceTestClient(t, map[string]api.Service{"svcprimary": primary}) - target, err := ResolveServiceOrReplica(context.Background(), client, "proj1", "svcprimary") + target, err := ResolveConnectionTarget(context.Background(), client, "proj1", primary) if err != nil { t.Fatalf("unexpected error: %v", err) } - if target.Replica != nil { - t.Fatalf("expected a primary target, got a replica: %+v", target.Replica) + if target.IsReplica { + t.Fatal("expected a primary target") } - if util.DerefStr(target.Service.ServiceId) != "svcprimary" { - t.Errorf("expected primary svcprimary, got %q", util.DerefStr(target.Service.ServiceId)) + if util.DerefStr(target.Connect.ServiceId) != "svcprimary" || util.DerefStr(target.Credential.ServiceId) != "svcprimary" { + t.Errorf("expected connect and credential to be the primary, got %+v", target) } } -func TestResolveServiceOrReplica_Replica(t *testing.T) { - primary := testReplicaService() - // The replica ID is not addressable as a service, so getService 404s and - // resolution must fall back to scanning the services list. - client := replicaTestClient(t, map[string]api.Service{"svcprimary": primary}, []api.Service{primary}) +func TestResolveConnectionTarget_Replica(t *testing.T) { + primary := primaryService() + replica := replicaService() + client := serviceTestClient(t, map[string]api.Service{"svcprimary": primary}) - target, err := ResolveServiceOrReplica(context.Background(), client, "proj1", "rep1234567") + target, err := ResolveConnectionTarget(context.Background(), client, "proj1", replica) if err != nil { t.Fatalf("unexpected error: %v", err) } - if target.Replica == nil { - t.Fatalf("expected a replica target, got primary only") - } - // Parent service is used for credentials. - if util.DerefStr(target.Service.ServiceId) != "svcprimary" { - t.Errorf("expected parent svcprimary, got %q", util.DerefStr(target.Service.ServiceId)) + if !target.IsReplica { + t.Fatal("expected a replica target") } - // Replica carries the endpoint we should connect to. - if util.DerefStr(target.Replica.Id) != "rep1234567" { - t.Errorf("expected replica rep1234567, got %q", util.DerefStr(target.Replica.Id)) + // Connect to the replica's own endpoint. + if util.DerefStr(target.Connect.ServiceId) != "rep1234567" { + t.Errorf("expected connect service rep1234567, got %q", util.DerefStr(target.Connect.ServiceId)) } - if target.Replica.Endpoint == nil || util.DerefStr(target.Replica.Endpoint.Host) != "replica.example.com" { - t.Errorf("expected replica endpoint host, got %+v", target.Replica.Endpoint) + // Credentials resolve against the parent primary (fetched via GetService). + if util.DerefStr(target.Credential.ServiceId) != "svcprimary" { + t.Errorf("expected credential service svcprimary, got %q", util.DerefStr(target.Credential.ServiceId)) } } -func TestResolveServiceOrReplica_NotFound(t *testing.T) { - primary := testReplicaService() - client := replicaTestClient(t, map[string]api.Service{"svcprimary": primary}, []api.Service{primary}) +func TestResolveConnectionTarget_ReplicaParentFetchFails(t *testing.T) { + replica := replicaService() + // Parent svcprimary is absent from the server → parent fetch 404s. + client := serviceTestClient(t, map[string]api.Service{}) - // An ID that is neither a service nor a known replica surfaces an error. - _, err := ResolveServiceOrReplica(context.Background(), client, "proj1", "unknown999") - if err == nil { - t.Fatal("expected an error for an unknown ID, got nil") + if _, err := ResolveConnectionTarget(context.Background(), client, "proj1", replica); err == nil { + t.Fatal("expected an error when the parent service can't be fetched, got nil") } } -func TestReplicaPoolerWarning(t *testing.T) { - host := "h" - port := 6432 - withPooler := &api.ReadReplicaSet{ - Name: util.Ptr("rep-a"), - ConnectionPooler: &api.ConnectionPooler{Endpoint: &api.Endpoint{Host: &host, Port: &port}}, - } - noPooler := &api.ReadReplicaSet{Name: util.Ptr("rep-b")} +func TestResolveConnectionTargetByID(t *testing.T) { + primary := primaryService() + replica := replicaService() + client := serviceTestClient(t, map[string]api.Service{"svcprimary": primary, "rep1234567": replica}) - cases := []struct { - name string - replica *api.ReadReplicaSet - pooled bool - wantWarning bool - }{ - {"nil replica", nil, true, false}, - {"not pooled, no pooler", noPooler, false, false}, - {"not pooled, has pooler", withPooler, false, false}, - {"pooled, has pooler", withPooler, true, false}, - {"pooled, no pooler warns", noPooler, true, true}, + // Primary ID → primary target. + target, err := ResolveConnectionTargetByID(context.Background(), client, "proj1", "svcprimary") + if err != nil { + t.Fatalf("unexpected error: %v", err) } - for _, tc := range cases { - t.Run(tc.name, func(t *testing.T) { - warning := ReplicaPoolerWarning(tc.replica, tc.pooled) - if (warning != "") != tc.wantWarning { - t.Errorf("warning = %q, wantWarning = %v", warning, tc.wantWarning) - } - }) + if target.IsReplica { + t.Error("expected primary target for a primary ID") } -} -func TestFindReplicaByID(t *testing.T) { - primary := testReplicaService() - client := replicaTestClient(t, map[string]api.Service{"svcprimary": primary}, []api.Service{primary}) - - target, err := FindReplicaByID(context.Background(), client, "proj1", "rep1234567") + // Replica ID → replica target with parent credentials. + target, err = ResolveConnectionTargetByID(context.Background(), client, "proj1", "rep1234567") if err != nil { t.Fatalf("unexpected error: %v", err) } - if target.Replica == nil || util.DerefStr(target.Replica.Id) != "rep1234567" { - t.Errorf("expected to find replica rep1234567, got %+v", target) + if !target.IsReplica || util.DerefStr(target.Credential.ServiceId) != "svcprimary" { + t.Errorf("expected replica target with parent credentials, got %+v", target) } - _, err = FindReplicaByID(context.Background(), client, "proj1", "missing999") - if !errors.Is(err, ErrReplicaNotFound) { - t.Errorf("expected ErrReplicaNotFound when no replica matches, got %v", err) + // Unknown ID → error. + if _, err := ResolveConnectionTargetByID(context.Background(), client, "proj1", "missing9999"); err == nil { + t.Error("expected an error for an unknown ID, got nil") } } -// TestFindReplicaByID_NotActive: a matched replica that isn't active returns a -// descriptive error (not ErrReplicaNotFound), so callers surface it. -func TestFindReplicaByID_NotActive(t *testing.T) { - primary := api.Service{ - ServiceId: util.Ptr("svcprimary"), - ProjectId: util.Ptr("proj1"), - ReadReplicaSets: &[]api.ReadReplicaSet{{ - Id: util.Ptr("rep1234567"), - Name: util.Ptr("still-creating"), - Status: util.Ptr(api.ReadReplicaSetStatusCreating), - }}, +func TestNewReplicaConnectionTarget(t *testing.T) { + primary := primaryService() + host := "menu-replica.example.com" + port := 5432 + replica := api.ReadReplicaSet{ + Id: util.Ptr("rep7654321"), + Name: util.Ptr("menu-replica"), + Endpoint: &api.Endpoint{Host: &host, Port: &port}, } - client := replicaTestClient(t, map[string]api.Service{"svcprimary": primary}, []api.Service{primary}) - _, err := FindReplicaByID(context.Background(), client, "proj1", "rep1234567") - if err == nil { - t.Fatal("expected an error for a non-active replica, got nil") + target := NewReplicaConnectionTarget(primary, replica) + if !target.IsReplica { + t.Error("expected a replica target") } - if errors.Is(err, ErrReplicaNotFound) { - t.Errorf("expected a descriptive not-active error, got ErrReplicaNotFound: %v", err) + if util.DerefStr(target.Connect.ServiceId) != "rep7654321" || util.DerefStr(target.Connect.Endpoint.Host) != host { + t.Errorf("expected connect to carry the replica endpoint, got %+v", target.Connect) } - if !strings.Contains(err.Error(), "not active") { - t.Errorf("expected a 'not active' error, got %v", err) + if util.DerefStr(target.Credential.ServiceId) != "svcprimary" { + t.Errorf("expected credential to be the primary, got %q", util.DerefStr(target.Credential.ServiceId)) } } -// TestResolveServiceOrReplica_NonNotFoundSkipsScan: a non-404 service lookup -// error is surfaced without listing services to look for a replica. -func TestResolveServiceOrReplica_NonNotFoundSkipsScan(t *testing.T) { - primary := testReplicaService() - listed := false - srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - w.Header().Set("Content-Type", "application/json") - if strings.HasSuffix(r.URL.Path, "/services") { - listed = true - _ = json.NewEncoder(w).Encode([]api.Service{primary}) - return - } - // GET service → permission denied (not a 404). - w.WriteHeader(http.StatusForbidden) - _ = json.NewEncoder(w).Encode(map[string]string{"message": "forbidden"}) - })) - t.Cleanup(srv.Close) +func TestReplicaPoolerWarning(t *testing.T) { + host := "h" + port := 6432 + withPooler := api.Service{ + Name: util.Ptr("rep-a"), + ConnectionPooler: &api.ConnectionPooler{Endpoint: &api.Endpoint{Host: &host, Port: &port}}, + } + noPooler := api.Service{Name: util.Ptr("rep-b")} - client, err := api.NewClientWithResponses(srv.URL) - if err != nil { - t.Fatalf("failed to build client: %v", err) + replica := func(svc api.Service) *ConnectionTarget { + return &ConnectionTarget{Connect: svc, Credential: primaryService(), IsReplica: true} } - // "rep1234567" would match a replica if scanned, but a 403 must skip the scan. - if _, err := ResolveServiceOrReplica(context.Background(), client, "proj1", "rep1234567"); err == nil { - t.Fatal("expected the 403 error to be surfaced, got nil") + cases := []struct { + name string + target *ConnectionTarget + pooled bool + wantWarning bool + }{ + {"primary is never warned", &ConnectionTarget{Connect: noPooler, Credential: noPooler}, true, false}, + {"replica not pooled, no pooler", replica(noPooler), false, false}, + {"replica not pooled, has pooler", replica(withPooler), false, false}, + {"replica pooled, has pooler", replica(withPooler), true, false}, + {"replica pooled, no pooler warns", replica(noPooler), true, true}, } - if listed { - t.Error("expected no service-list scan for a non-404 service error") + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + warning := ReplicaPoolerWarning(tc.target, tc.pooled) + if (warning != "") != tc.wantWarning { + t.Errorf("warning = %q, wantWarning = %v", warning, tc.wantWarning) + } + }) } } diff --git a/internal/tiger/common/schema_fetch.go b/internal/tiger/common/schema_fetch.go index 6b30c7cb..f2ea9ad0 100644 --- a/internal/tiger/common/schema_fetch.go +++ b/internal/tiger/common/schema_fetch.go @@ -5,24 +5,23 @@ import ( "github.com/jackc/pgx/v5" - "github.com/timescale/tiger-cli/internal/tiger/api" "github.com/timescale/tiger-cli/internal/tiger/util" ) -// FetchServiceSchema opens a read-only connection to the service (or one of its -// read replicas, when replica is non-nil) and introspects its schema. It is the +// FetchServiceSchema opens a read-only connection to the target (a primary +// service or one of its read replicas) and introspects its schema. It is the // shared entry point for the `tiger db schema` CLI command and the db_schema // MCP tool. // // The connection is forced read-only: introspection only issues SELECTs, so // this is always safe and guards against accidental writes. -func FetchServiceSchema(ctx context.Context, service api.Service, replica *api.ReadReplicaSet, role string, pooled bool, opts SchemaOptions) (*DatabaseSchema, error) { - if err := CheckServiceReady(service); err != nil { +func FetchServiceSchema(ctx context.Context, target *ConnectionTarget, role string, pooled bool, opts SchemaOptions) (*DatabaseSchema, error) { + if err := CheckServiceReady(target.Connect); err != nil { return nil, err } // Introspection runs parameterless statements, so the simple protocol fits. - conn, err := ConnectToTarget(ctx, service, replica, ConnectionDetailsOptions{ + conn, err := ConnectTarget(ctx, target, ConnectionDetailsOptions{ Pooled: pooled, Role: role, WithPassword: true, @@ -34,8 +33,8 @@ func FetchServiceSchema(ctx context.Context, service api.Service, replica *api.R defer conn.Close(context.Background()) ident := SchemaIdent{ - ID: util.DerefStr(service.ServiceId), - Name: util.DerefStr(service.Name), + ID: util.DerefStr(target.Connect.ServiceId), + Name: util.DerefStr(target.Connect.Name), } return FetchSchemaFromConn(ctx, conn, ident, opts) } diff --git a/internal/tiger/mcp/db_tools.go b/internal/tiger/mcp/db_tools.go index 9af7c4b7..2d484768 100644 --- a/internal/tiger/mcp/db_tools.go +++ b/internal/tiger/mcp/db_tools.go @@ -267,15 +267,15 @@ func (s *Server) handleDBSchema(ctx context.Context, req *mcp.CallToolRequest, i ) // service_id may name a service or one of its read replicas. - target, err := common.ResolveServiceOrReplica(ctx, cfg.Client, cfg.ProjectID, input.ServiceID) + target, err := common.ResolveConnectionTargetByID(ctx, cfg.Client, cfg.ProjectID, input.ServiceID) if err != nil { return nil, DBSchemaOutput{}, err } // A replica without a pooler connects directly; surface that as a warning. - warning := common.ReplicaPoolerWarning(target.Replica, input.Pooled) + warning := common.ReplicaPoolerWarning(target, input.Pooled) - schema, err := common.FetchServiceSchema(ctx, target.Service, target.Replica, input.Role, input.Pooled, common.SchemaOptions{ + schema, err := common.FetchServiceSchema(ctx, target, input.Role, input.Pooled, common.SchemaOptions{ Schema: input.SchemaName, IncludeInternal: input.Internal, IncludeDefinitions: input.Definitions, @@ -309,13 +309,13 @@ func (s *Server) handleDBExecuteQuery(ctx context.Context, req *mcp.CallToolRequ ) // service_id may name a service or one of its read replicas. - target, err := common.ResolveServiceOrReplica(ctx, cfg.Client, cfg.ProjectID, input.ServiceID) + target, err := common.ResolveConnectionTargetByID(ctx, cfg.Client, cfg.ProjectID, input.ServiceID) if err != nil { return nil, DBExecuteQueryOutput{}, err } // A replica without a pooler connects directly; surface that as a warning. - poolerWarning := common.ReplicaPoolerWarning(target.Replica, input.Pooled) + poolerWarning := common.ReplicaPoolerWarning(target, input.Pooled) // Create query context with timeout queryCtx, cancel := context.WithTimeout(ctx, timeout) @@ -336,7 +336,7 @@ func (s *Server) handleDBExecuteQuery(ctx context.Context, req *mcp.CallToolRequ } // Connect to database - conn, err := common.ConnectToTarget(queryCtx, target.Service, target.Replica, common.ConnectionDetailsOptions{ + conn, err := common.ConnectTarget(queryCtx, target, common.ConnectionDetailsOptions{ Pooled: input.Pooled, Role: input.Role, WithPassword: true, diff --git a/specs/spec.md b/specs/spec.md index e4e69713..d69af598 100644 --- a/specs/spec.md +++ b/specs/spec.md @@ -569,7 +569,7 @@ The `save-password` command accepts passwords through three methods (in order of The `connect` and `psql` commands support passing additional flags directly to the psql client using the `--` separator. Any flags after `--` are passed through to psql unchanged, allowing full access to psql's functionality while maintaining tiger's connection and authentication handling. **Read Replica ID as a Connection Target (all db connection commands):** -The `db connection-string`, `db connect` / `db psql`, `db test-connection`, and `db schema` commands accept a read replica set ID anywhere a service ID is accepted (as the positional argument or the default service). Because a read replica set ID is not addressable as a service (the API only exposes read replicas nested under their parent service), the CLI first tries to look up the ID as a service; if that fails, it scans the project's services for a read replica set with that ID. When it matches a replica, the command connects to that replica's endpoint while credentials resolve against the parent service. An ID that is neither a service nor a read replica surfaces the original service-not-found error. +The `db connection-string`, `db connect` / `db psql`, `db test-connection`, and `db schema` commands accept a read replica set ID anywhere a service ID is accepted (as the positional argument or the default service). The API resolves a read replica ID through the same service-get endpoint, returning the replica's own connection endpoint along with a `forked_from` link identifying it as a standby of its parent primary. The command connects to the replica's endpoint but resolves credentials (and any password reset) against the parent primary, since read replicas share the primary's credentials. **Read Replica Prompt (connect/psql):** When `tiger db connect` / `tiger db psql` runs in an interactive terminal and is given a *primary service* ID, it checks whether that service has any read replicas (read replica sets). If the service has one or more active read replicas, it presents a menu offering to connect to the primary (default) or to one of the existing replicas. If the service has no read replicas, no menu is shown and the command connects to the primary directly. From 57e8469ee2488b69af216ec34898a6e2e6faeb2a Mon Sep 17 00:00:00 2001 From: Alexandra Primakina Date: Wed, 15 Jul 2026 13:46:12 +0200 Subject: [PATCH 4/8] docs(db): tighten read replica help text Drop the redundant "in place of the service ID" and doubled "read replica" wording from the db connection commands' help and the README, and remove the read-only/lag aside from `db connect` help (kept in the MCP tool description). Co-Authored-By: Claude Opus 4.8 --- README.md | 2 +- internal/tiger/cmd/db.go | 19 +++++++------------ internal/tiger/common/replica.go | 3 +-- 3 files changed, 9 insertions(+), 15 deletions(-) diff --git a/README.md b/README.md index 7f2827cb..8250d472 100644 --- a/README.md +++ b/README.md @@ -110,7 +110,7 @@ Tiger CLI provides the following commands: - `delete` - Delete a service - `update-password` - Update service master password - `logs` - View service logs -- `tiger db` - Database operations (each command below also accepts a read replica set ID in place of the service ID) +- `tiger db` - Database operations (each command below also accepts a read replica set ID) - `connect` - Connect to a database with psql (in an interactive terminal, if the service has read replicas, offers to connect to one of them; use `--no-replica-prompt` to skip) - `connection-string` - Get connection string for a service - `test-connection` - Test database connectivity diff --git a/internal/tiger/cmd/db.go b/internal/tiger/cmd/db.go index 905c016a..cf5177d5 100644 --- a/internal/tiger/cmd/db.go +++ b/internal/tiger/cmd/db.go @@ -55,8 +55,7 @@ The service ID can be provided as an argument or will use the default service from your configuration. The connection string includes all necessary parameters for establishing a database connection to the TimescaleDB/PostgreSQL service. -You can also pass a read replica set ID in place of the service ID to get a -connection string for that read replica. +You can also pass a read replica set ID to get a connection string for that replica. By default, passwords are excluded from the connection string for security. Use --with-password to include the password directly in the connection string. @@ -161,10 +160,8 @@ primary. Use --no-replica-prompt to skip this prompt and always connect to the requested service. The prompt is automatically skipped when stdin is not a terminal (e.g. in scripts) or when the service has no read replicas. -You can also pass a read replica set ID directly in place of the service ID to -connect straight to that read replica without the prompt. -Read replicas are read-only, may lag the primary, and share the primary -service's credentials. +You can also pass a read replica set ID to connect straight to that replica, +skipping the prompt. Read replicas share the primary's credentials. Examples: # Connect to default service @@ -264,8 +261,7 @@ The service ID can be provided as an argument or will use the default service from your configuration. This command tests if the database is accepting connections and returns appropriate exit codes following pg_isready conventions. -You can also pass a read replica set ID in place of the service ID to test -connectivity to that read replica. +You can also pass a read replica set ID to test connectivity to that replica. Return Codes: 0: Server is accepting connections normally @@ -814,10 +810,9 @@ indexes, triggers, and TimescaleDB hypertable and continuous aggregate metadata. The service ID can be provided as an argument or will use the default service -from your configuration. You can also pass a read replica set ID in place of the -service ID to introspect that read replica. Only objects the connecting role can -access are returned. The connection is opened in Tiger Cloud's immutable -read-only mode. +from your configuration. You can also pass a read replica set ID to introspect +that replica. Only objects the connecting role can access are returned. The +connection is opened in Tiger Cloud's immutable read-only mode. By default only user-facing schemas and objects are shown. View and routine definitions and object comments are omitted unless requested, since they can be diff --git a/internal/tiger/common/replica.go b/internal/tiger/common/replica.go index 4eb15715..94bd75e6 100644 --- a/internal/tiger/common/replica.go +++ b/internal/tiger/common/replica.go @@ -56,8 +56,7 @@ func GetService(ctx context.Context, client api.ClientWithResponsesInterface, pr // ResolveConnectionTarget turns a fetched service into a ConnectionTarget. When // the service is a standby read replica (ForkedFrom.IsStandby), it connects to // the replica but resolves credentials against the parent primary, which is -// fetched here. Everything else (primary, or an independent fork with its own -// credentials) connects with its own credentials. +// fetched here. func ResolveConnectionTarget(ctx context.Context, client api.ClientWithResponsesInterface, projectID string, service api.Service) (*ConnectionTarget, error) { fork := service.ForkedFrom if fork == nil || !util.Deref(fork.IsStandby) { From 94aadd70e0deeaf5bc71f1bf5c1c12e809ea19a5 Mon Sep 17 00:00:00 2001 From: Alexandra Primakina Date: Thu, 16 Jul 2026 14:56:03 +0200 Subject: [PATCH 5/8] fix(db): URL-encode connection userinfo; guard replica IDs on write commands MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Build the connection string via net/url so a role or password containing URL-special characters (e.g. a manually entered password) no longer produces an unparseable "invalid userinfo" string. - service update-password (CLI + MCP) and db create role reject a read replica ID, pointing at the primary — a read-only replica can't be written to, and replicas share the primary's credentials. Adds common.IsReadReplica. - db save-password redirects a replica ID to the parent primary, storing the password where the connect commands read it. Co-Authored-By: Claude Opus 4.8 --- CLAUDE.md | 2 +- internal/tiger/cmd/db.go | 19 +++- internal/tiger/cmd/db_test.go | 121 +++++++++++++++++++++++ internal/tiger/cmd/service.go | 9 ++ internal/tiger/cmd/service_test.go | 35 +++++++ internal/tiger/common/connection.go | 12 ++- internal/tiger/common/connection_test.go | 24 +++++ internal/tiger/common/replica.go | 16 +-- internal/tiger/mcp/service_tools.go | 40 +++++--- specs/spec.md | 2 +- specs/spec_mcp.md | 2 +- 11 files changed, 251 insertions(+), 31 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index ab054a83..7a29e28b 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -273,7 +273,7 @@ Tiger CLI is a Go-based command-line interface for managing Tiger, the modern da - `service.go` - Service management commands (list, create, get, fork, start, stop, resize, delete, update-password, logs) - `db.go` - Database operation commands (connection-string, connect, test-connection) - `read_replica.go` - Read replica selection flow for `db connect`/`psql`: in an interactive terminal, when the service has one or more active read replicas (listed via the `/replicaSets` API), prompts to connect to the primary or one of the replicas. Skipped when stdin is not a TTY, when `--no-replica-prompt` is set, or when the service has no read replicas. - - Read replica ID as a connection target: the db connection commands (`db connection-string`, `db connect`/`psql`, `db test-connection`, `db schema`) and the `db_execute_query`/`db_schema` MCP tools accept a read replica set ID anywhere a service ID is accepted. The API's `GetService` resolves both primary and read replica IDs, returning the replica's own endpoint plus a `forked_from` link (`is_standby: true`, parent `service_id`). `common.ResolveConnectionTarget` turns a fetched `api.Service` into a `common.ConnectionTarget{Connect, Credential, IsReplica}`: a standby replica connects to its own endpoint (`Connect`) but resolves credentials/password reset against the parent primary (`Credential`, fetched via `GetService`), since read replicas share the primary's credentials. MCP handlers call `common.ResolveConnectionTargetByID`; the CLI's `resolveConnectionTarget` fetches via `getServiceDetailsFunc` (the test seam) then calls `ResolveConnectionTarget`. The interactive `db connect` menu builds a target from a `/replicaSets` entry via `common.NewReplicaConnectionTarget`. + - Read replica ID as a connection target: the db connection commands (`db connection-string`, `db connect`/`psql`, `db test-connection`, `db schema`) and the `db_execute_query`/`db_schema` MCP tools accept a read replica set ID anywhere a service ID is accepted. The API's `GetService` resolves both primary and read replica IDs, returning the replica's own endpoint plus a `forked_from` link (`is_standby: true`, parent `service_id`). `common.ResolveConnectionTarget` turns a fetched `api.Service` into a `common.ConnectionTarget{Connect, Credential, IsReplica}`: a standby replica connects to its own endpoint (`Connect`) but resolves credentials/password reset against the parent primary (`Credential`, fetched via `GetService`), since read replicas share the primary's credentials. MCP handlers call `common.ResolveConnectionTargetByID`; the CLI's `resolveConnectionTarget` fetches via `getServiceDetailsFunc` (the test seam) then calls `ResolveConnectionTarget`. The interactive `db connect` menu builds a target from a `/replicaSets` entry via `common.NewReplicaConnectionTarget`. `common.IsReadReplica(service)` reports whether a fetched service is a standby. `service update-password` (CLI and MCP) and `db create role` use it to reject a replica ID (a read-only replica can't be written to). `db save-password` instead redirects to the parent (via `resolveConnectionTarget`), storing the password where the connect commands read it. - `config.go` - Configuration management commands (show, set, unset, reset) - `mcp.go` - MCP server commands (install, start, list, get) - `version.go` - Version command diff --git a/internal/tiger/cmd/db.go b/internal/tiger/cmd/db.go index cf5177d5..0cff4717 100644 --- a/internal/tiger/cmd/db.go +++ b/internal/tiger/cmd/db.go @@ -369,10 +369,14 @@ Examples: return err } - service, err := getServiceDetailsFunc(cmd, cfg, args) + // Resolve the target so a read replica id stores the password against + // its parent primary: replicas share the primary's credentials, and + // connect/test-connection look the password up against the primary. + target, err := resolveConnectionTarget(cmd, cfg, args) if err != nil { return err } + service := target.Credential // Determine password based on precedence: // 1. --password flag with value @@ -412,6 +416,10 @@ Examples: return fmt.Errorf("failed to save password: %w", err) } + if target.IsReplica { + fmt.Fprintf(cmd.ErrOrStderr(), "Read replicas share the primary's credentials; saving against primary %s.\n", + *service.ServiceId) + } fmt.Fprintf(cmd.ErrOrStderr(), "Password saved successfully for service %s (role: %s)\n", *service.ServiceId, dbSavePasswordRole) return nil @@ -650,7 +658,8 @@ func buildDbCreateRoleCmd() *cobra.Command { Long: `Create a new database role with optional read-only enforcement. The service ID can be provided as an argument or will use the default service -from your configuration. +from your configuration. A read replica ID is rejected, since replicas are +read-only; create the role on the primary instead. By default, a secure random password is auto-generated for the new role. You can: - Provide an explicit password with --password= @@ -724,6 +733,12 @@ PostgreSQL Configuration Parameters That May Be Set: return err } + // A read replica is read-only, so a role can't be created there. + if common.IsReadReplica(service) { + return fmt.Errorf("%q is a read replica; create the role on its primary service %q instead", + util.Deref(service.ServiceId), util.DerefStr(service.ForkedFrom.ServiceId)) + } + // Get password rolePassword, err := getPasswordForRole(passwordFlag) if err != nil { diff --git a/internal/tiger/cmd/db_test.go b/internal/tiger/cmd/db_test.go index 5f4a64b6..998754b3 100644 --- a/internal/tiger/cmd/db_test.go +++ b/internal/tiger/cmd/db_test.go @@ -3,7 +3,10 @@ package cmd import ( "bytes" "context" + "encoding/json" "fmt" + "net/http" + "net/http/httptest" "os" "strings" "testing" @@ -93,6 +96,38 @@ func executeDBCommand(ctx context.Context, args ...string) (string, error) { return buf.String(), err } +// db create role on a read replica ID is rejected (replicas are read-only). +func TestDBCreateRole_ReadReplicaRejected(t *testing.T) { + tmpDir := setupDBTest(t) + + if _, err := config.UseTestConfig(tmpDir, map[string]any{ + "api_url": "http://localhost:9999", + "project_id": "test-project-123", + }); err != nil { + t.Fatalf("Failed to save test config: %v", err) + } + mockTestPAT(t) + + standby := api.Service{ + ServiceId: util.Ptr("rep1234567"), + ProjectId: util.Ptr("test-project-123"), + ForkedFrom: &api.ForkSpec{IsStandby: util.Ptr(true), ServiceId: util.Ptr("svcprimary")}, + } + orig := getServiceDetailsFunc + getServiceDetailsFunc = func(cmd *cobra.Command, cfg *common.Config, args []string) (api.Service, error) { + return standby, nil + } + defer func() { getServiceDetailsFunc = orig }() + + _, err := executeDBCommand(t.Context(), "db", "create", "role", "rep1234567", "--name", "ai_analyst") + if err == nil { + t.Fatal("expected create role on a read replica to be rejected") + } + if !strings.Contains(err.Error(), "read replica") || !strings.Contains(err.Error(), "svcprimary") { + t.Errorf("expected guidance pointing at the primary service, got: %v", err) + } +} + func TestDBConnectionString_NoServiceID(t *testing.T) { tmpDir := setupDBTest(t) @@ -1009,6 +1044,92 @@ func TestDBSavePassword_ExplicitPassword(t *testing.T) { } } +// TestDBSavePassword_ReplicaResolvesToParent verifies that passing a read +// replica id stores the password against the parent primary, so it is found by +// the connect/test-connection read path. +func TestDBSavePassword_ReplicaResolvesToParent(t *testing.T) { + config.SetTestServiceName(t) + tmpDir := setupDBTest(t) + + originalStorage := viper.GetString("password_storage") + viper.Set("password_storage", "keyring") + defer viper.Set("password_storage", originalStorage) + + const projectID = "test-project-123" + port := 5432 + primaryHost := "svcprimary.example.com" + replicaHost := "replica.example.com" + primary := api.Service{ + ServiceId: util.Ptr("svcprimary"), + ProjectId: util.Ptr(projectID), + Endpoint: &api.Endpoint{Host: &primaryHost, Port: &port}, + } + replica := api.Service{ + ServiceId: util.Ptr("rep1234567"), + ProjectId: util.Ptr(projectID), + Endpoint: &api.Endpoint{Host: &replicaHost, Port: &port}, + ForkedFrom: &api.ForkSpec{ + IsStandby: util.Ptr(true), + ProjectId: util.Ptr(projectID), + ServiceId: util.Ptr("svcprimary"), + }, + } + + // Serve the parent primary lookup; the replica itself comes from the mocked + // getServiceDetailsFunc, so only the parent fetch reaches the API. + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + parts := strings.Split(strings.Trim(r.URL.Path, "/"), "/") + if parts[len(parts)-1] == "svcprimary" { + _ = json.NewEncoder(w).Encode(primary) + return + } + w.WriteHeader(http.StatusNotFound) + _ = json.NewEncoder(w).Encode(map[string]string{"message": "not found"}) + })) + defer srv.Close() + + _, err := config.UseTestConfig(tmpDir, map[string]any{ + "api_url": srv.URL, + "project_id": projectID, + "service_id": "rep1234567", + }) + if err != nil { + t.Fatalf("Failed to save test config: %v", err) + } + + mockTestPAT(t) + originalGetServiceDetails := getServiceDetailsFunc + getServiceDetailsFunc = func(cmd *cobra.Command, cfg *common.Config, args []string) (api.Service, error) { + return replica, nil + } + defer func() { getServiceDetailsFunc = originalGetServiceDetails }() + + const testPassword = "replica-parent-pw" + output, err := executeDBCommand(t.Context(), "db", "save-password", "rep1234567", "--password="+testPassword) + if err != nil { + t.Fatalf("Expected save-password to succeed, got error: %v", err) + } + if !strings.Contains(output, "svcprimary") { + t.Errorf("expected parent primary id in output, got: %s", output) + } + + storage := common.GetPasswordStorage() + // Stored against the parent primary, matching the connect read path. + got, err := storage.Get(primary, "tsdbadmin") + if err != nil { + t.Fatalf("expected password stored under primary, got error: %v", err) + } + defer storage.Remove(primary, "tsdbadmin") + if got != testPassword { + t.Errorf("expected %q under primary, got %q", testPassword, got) + } + // Not stored under the replica id. + if pw, err := storage.Get(replica, "tsdbadmin"); err == nil && pw != "" { + t.Errorf("expected no password under replica, got %q", pw) + } +} + func TestDBSavePassword_EnvironmentVariable(t *testing.T) { // Use a unique service name for this test to avoid conflicts config.SetTestServiceName(t) diff --git a/internal/tiger/cmd/service.go b/internal/tiger/cmd/service.go index bbcbb371..740053d4 100644 --- a/internal/tiger/cmd/service.go +++ b/internal/tiger/cmd/service.go @@ -431,6 +431,9 @@ The service ID can be provided as an argument or will use the default service from your configuration. This command updates the master password for the 'tsdbadmin' user used to authenticate to the database service. +A read replica ID is rejected — read replicas share the primary's credentials, +so update the password on the primary instead. + Examples: # Update password for default service, interactively prompts tiger service update-password @@ -500,6 +503,12 @@ Examples: } service := *serviceResp.JSON200 + // A read replica has no separate password to rotate. + if common.IsReadReplica(service) { + return fmt.Errorf("%q is a read replica; update the password on its primary service %q instead", + serviceID, util.DerefStr(service.ForkedFrom.ServiceId)) + } + statusOutput := cmd.ErrOrStderr() if autoGenerate { diff --git a/internal/tiger/cmd/service_test.go b/internal/tiger/cmd/service_test.go index b1cae243..6007a1db 100644 --- a/internal/tiger/cmd/service_test.go +++ b/internal/tiger/cmd/service_test.go @@ -5,6 +5,8 @@ import ( "context" "encoding/json" "errors" + "net/http" + "net/http/httptest" "os" "strings" "testing" @@ -16,6 +18,7 @@ import ( "github.com/timescale/tiger-cli/internal/tiger/api" "github.com/timescale/tiger-cli/internal/tiger/common" "github.com/timescale/tiger-cli/internal/tiger/config" + "github.com/timescale/tiger-cli/internal/tiger/util" ) func setupServiceTest(t *testing.T) string { @@ -968,6 +971,38 @@ func TestServiceUpdatePassword_NoAuth(t *testing.T) { } } +// update-password on a read replica ID is rejected, pointing at the primary. +func TestServiceUpdatePassword_ReadReplicaRejected(t *testing.T) { + tmpDir := setupServiceTest(t) + + // getService resolves the replica ID to a standby linked to its parent. + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(api.Service{ + ServiceId: util.Ptr("rep1234567"), + ProjectId: util.Ptr("test-project-123"), + ForkedFrom: &api.ForkSpec{IsStandby: util.Ptr(true), ServiceId: util.Ptr("svcprimary")}, + }) + })) + defer srv.Close() + + if _, err := config.UseTestConfig(tmpDir, map[string]any{ + "api_url": srv.URL, + "project_id": "test-project-123", + }); err != nil { + t.Fatalf("Failed to save test config: %v", err) + } + mockTestPAT(t) + + _, err, _ := executeServiceCommand(t.Context(), "service", "update-password", "rep1234567", "--new-password", "irrelevant") + if err == nil { + t.Fatal("expected update-password on a read replica to be rejected") + } + if !strings.Contains(err.Error(), "read replica") || !strings.Contains(err.Error(), "svcprimary") { + t.Errorf("expected guidance pointing at the primary service, got: %v", err) + } +} + func TestServiceUpdatePassword_EnvironmentVariable(t *testing.T) { tmpDir := setupServiceTest(t) diff --git a/internal/tiger/common/connection.go b/internal/tiger/common/connection.go index bed0d958..c9bf334e 100644 --- a/internal/tiger/common/connection.go +++ b/internal/tiger/common/connection.go @@ -3,6 +3,7 @@ package common import ( "context" "fmt" + "net/url" "github.com/jackc/pgx/v5" @@ -156,12 +157,13 @@ func (d *ConnectionDetails) String() string { query += "&" + readOnlyConnectionOption } - if d.Password == "" { - // Build connection string without password (default behavior) - return fmt.Sprintf("postgresql://%s@%s:%d/%s?%s", d.Role, d.Host, d.Port, d.Database, query) + // url.User* percent-encodes the role/password so URL-special characters (e.g. + // in a manually entered password) don't break connection-string parsing. + userinfo := url.User(d.Role) + if d.Password != "" { + userinfo = url.UserPassword(d.Role, d.Password) } - // Include password in connection string - return fmt.Sprintf("postgresql://%s:%s@%s:%d/%s?%s", d.Role, d.Password, d.Host, d.Port, d.Database, query) + return fmt.Sprintf("postgresql://%s@%s:%d/%s?%s", userinfo, d.Host, d.Port, d.Database, query) } // GetPassword fetches the password for the specified service from the diff --git a/internal/tiger/common/connection_test.go b/internal/tiger/common/connection_test.go index 2db2f3ad..c5c5d7fb 100644 --- a/internal/tiger/common/connection_test.go +++ b/internal/tiger/common/connection_test.go @@ -2,6 +2,7 @@ package common import ( "fmt" + "net/url" "strings" "testing" @@ -314,6 +315,29 @@ func TestBuildConnectionString_WithPassword_PgpassStorage(t *testing.T) { } } +// A password with URL-special characters must still produce a parseable URL. +func TestConnectionDetailsString_EncodesSpecialCharPassword(t *testing.T) { + details := &ConnectionDetails{ + Role: "tsdbadmin", + Password: "p@ss/w:rd? #[x]", + Host: "host.example.com", + Port: 5432, + Database: "tsdb", + } + + s := details.String() + u, err := url.Parse(s) + if err != nil { + t.Fatalf("connection string should be a parseable URL, got error: %v (%q)", err, s) + } + if got := u.User.Username(); got != details.Role { + t.Errorf("role did not round-trip: got %q, want %q", got, details.Role) + } + if pw, _ := u.User.Password(); pw != details.Password { + t.Errorf("password did not round-trip: got %q, want %q", pw, details.Password) + } +} + func TestBuildConnectionString_WithPassword_NoStorage(t *testing.T) { // Set no storage as the password storage method for this test originalStorage := viper.GetString("password_storage") diff --git a/internal/tiger/common/replica.go b/internal/tiger/common/replica.go index 94bd75e6..dfd6cf65 100644 --- a/internal/tiger/common/replica.go +++ b/internal/tiger/common/replica.go @@ -53,17 +53,21 @@ func GetService(ctx context.Context, client api.ClientWithResponsesInterface, pr return resp.JSON200, nil } +// IsReadReplica reports whether the service is a standby read replica (which +// shares its parent primary's credentials). +func IsReadReplica(service api.Service) bool { + return service.ForkedFrom != nil && util.Deref(service.ForkedFrom.IsStandby) +} + // ResolveConnectionTarget turns a fetched service into a ConnectionTarget. When -// the service is a standby read replica (ForkedFrom.IsStandby), it connects to -// the replica but resolves credentials against the parent primary, which is -// fetched here. +// the service is a standby read replica, it connects to the replica but resolves +// credentials against the parent primary, which is fetched here. func ResolveConnectionTarget(ctx context.Context, client api.ClientWithResponsesInterface, projectID string, service api.Service) (*ConnectionTarget, error) { - fork := service.ForkedFrom - if fork == nil || !util.Deref(fork.IsStandby) { + if !IsReadReplica(service) { return &ConnectionTarget{Connect: service, Credential: service}, nil } - parentID := util.DerefStr(fork.ServiceId) + parentID := util.DerefStr(service.ForkedFrom.ServiceId) if parentID == "" { return &ConnectionTarget{Connect: service, Credential: service, IsReplica: true}, nil } diff --git a/internal/tiger/mcp/service_tools.go b/internal/tiger/mcp/service_tools.go index 48ed9d93..9a6379db 100644 --- a/internal/tiger/mcp/service_tools.go +++ b/internal/tiger/mcp/service_tools.go @@ -1074,32 +1074,42 @@ func (s *Server) handleServiceUpdatePassword(ctx context.Context, req *mcp.CallT Password: input.Password, } - // Make API call to update password ctx, cancel := context.WithTimeout(ctx, 30*time.Second) defer cancel() + // Fetch first so we can reject read replicas and reuse the service for + // password storage below. + serviceResp, err := cfg.Client.GetServiceWithResponse(ctx, cfg.ProjectID, input.ServiceID) + if err != nil { + return nil, ServiceUpdatePasswordOutput{}, fmt.Errorf("failed to get service details: %w", err) + } + if serviceResp.StatusCode() != http.StatusOK { + return nil, ServiceUpdatePasswordOutput{}, common.ExitWithErrorFromStatusCode(serviceResp.StatusCode(), serviceResp.JSON4XX) + } + if serviceResp.JSON200 == nil { + return nil, ServiceUpdatePasswordOutput{}, fmt.Errorf("empty response from API") + } + service := *serviceResp.JSON200 + if common.IsReadReplica(service) { + return nil, ServiceUpdatePasswordOutput{}, fmt.Errorf("%q is a read replica; update the password on its primary service %q instead", + input.ServiceID, util.DerefStr(service.ForkedFrom.ServiceId)) + } + resp, err := cfg.Client.UpdatePasswordWithResponse(ctx, cfg.ProjectID, input.ServiceID, updateReq) if err != nil { return nil, ServiceUpdatePasswordOutput{}, fmt.Errorf("failed to update service password: %w", err) } - - // Handle API response if resp.StatusCode() != http.StatusOK && resp.StatusCode() != http.StatusNoContent { return nil, ServiceUpdatePasswordOutput{}, common.ExitWithErrorFromStatusCode(resp.StatusCode(), resp.JSON4XX) } - // Get service details for password storage (similar to CLI implementation) - var passwordStorage *common.PasswordStorageResult - serviceResp, err := cfg.Client.GetServiceWithResponse(ctx, cfg.ProjectID, input.ServiceID) - if err == nil && serviceResp.StatusCode() == http.StatusOK && serviceResp.JSON200 != nil { - // Save the new password using the shared util function - result, err := common.SavePasswordWithResult(api.Service(*serviceResp.JSON200), input.Password, "tsdbadmin") - passwordStorage = &result - if err != nil { - logging.Debug("MCP: Password storage failed", zap.Error(err)) - } else { - logging.Debug("MCP: Password saved successfully", zap.String("method", result.Method)) - } + // Save the new password using the service we already fetched. + result, saveErr := common.SavePasswordWithResult(service, input.Password, "tsdbadmin") + passwordStorage := &result + if saveErr != nil { + logging.Debug("MCP: Password storage failed", zap.Error(saveErr)) + } else { + logging.Debug("MCP: Password saved successfully", zap.String("method", result.Method)) } output := ServiceUpdatePasswordOutput{ diff --git a/specs/spec.md b/specs/spec.md index d69af598..60b8dcb7 100644 --- a/specs/spec.md +++ b/specs/spec.md @@ -215,7 +215,7 @@ Manage database services. - `detach-vpc`: Detach service from VPC - `enable-pooler`: Enable connection pooling - `disable-pooler`: Disable connection pooling -- `update-password`: Update service master password +- `update-password`: Update service master password (rejects a read replica ID; update the primary instead) - `set-default`: Set default service **Commands with Wait/Timeout Behavior:** diff --git a/specs/spec_mcp.md b/specs/spec_mcp.md index d9a8dfc0..4bd58cc7 100644 --- a/specs/spec_mcp.md +++ b/specs/spec_mcp.md @@ -313,7 +313,7 @@ Detach a service from a VPC. Update the master password for a service. **Parameters:** -- `service_id` (string, required): Service ID +- `service_id` (string, required): Service ID. A read replica ID is rejected; update the password on the primary instead. - `password` (string, required): New password for the service **Returns:** Operation status confirmation. From c32b197a5858f3eca1705ab8eb192a29959c439e Mon Sep 17 00:00:00 2001 From: Alexandra Primakina Date: Thu, 16 Jul 2026 15:23:50 +0200 Subject: [PATCH 6/8] refactor(db): rename ConnectionTarget fields per review feedback Rename ConnectionTarget.Connect -> ConnectionService and Credential -> CredentialService so the field names convey that Credential is the service whose credentials are used, not the credentials themselves. Trim the now- redundant doc comments the self-documenting names replace. Co-Authored-By: Claude Opus 4.8 --- internal/tiger/cmd/db.go | 4 +-- internal/tiger/cmd/read_replica.go | 4 +-- internal/tiger/cmd/read_replica_test.go | 2 +- internal/tiger/cmd/replica_connect_test.go | 20 ++++++------- internal/tiger/common/connection.go | 4 +-- internal/tiger/common/replica.go | 33 +++++++++++----------- internal/tiger/common/replica_test.go | 24 ++++++++-------- internal/tiger/common/schema_fetch.go | 6 ++-- 8 files changed, 48 insertions(+), 49 deletions(-) diff --git a/internal/tiger/cmd/db.go b/internal/tiger/cmd/db.go index 0cff4717..421ab598 100644 --- a/internal/tiger/cmd/db.go +++ b/internal/tiger/cmd/db.go @@ -234,7 +234,7 @@ Examples: // Read replicas share the primary's credentials, so password storage // and recovery always operate on the credential service. - return connectWithPasswordMenu(cmd.Context(), cmd, cfg.Client, target.Credential, details, psqlPath, psqlFlags) + return connectWithPasswordMenu(cmd.Context(), cmd, cfg.Client, target.CredentialService, details, psqlPath, psqlFlags) }, } @@ -376,7 +376,7 @@ Examples: if err != nil { return err } - service := target.Credential + service := target.CredentialService // Determine password based on precedence: // 1. --password flag with value diff --git a/internal/tiger/cmd/read_replica.go b/internal/tiger/cmd/read_replica.go index 8c027938..a5abbfba 100644 --- a/internal/tiger/cmd/read_replica.go +++ b/internal/tiger/cmd/read_replica.go @@ -35,7 +35,7 @@ func resolveConnectTarget( // Offer the replica menu only for a primary on an interactive terminal. if !target.IsReplica && !noReplicaPrompt && checkStdinIsTTY() { - primary := target.Connect + primary := target.ConnectionService replicas, err := fetchReplicaSets(ctx, client, projectID, util.DerefStr(primary.ServiceId)) if err != nil { // Don't block the connection if we can't list replicas. @@ -59,7 +59,7 @@ func resolveConnectTarget( return nil, err } if chosen.IsReplica { - fmt.Fprintf(cmd.ErrOrStderr(), "Connecting to read replica '%s'...\n", util.DerefStr(chosen.Connect.Name)) + fmt.Fprintf(cmd.ErrOrStderr(), "Connecting to read replica '%s'...\n", util.DerefStr(chosen.ConnectionService.Name)) } return details, nil } diff --git a/internal/tiger/cmd/read_replica_test.go b/internal/tiger/cmd/read_replica_test.go index b3dd88c5..1dbf327d 100644 --- a/internal/tiger/cmd/read_replica_test.go +++ b/internal/tiger/cmd/read_replica_test.go @@ -125,7 +125,7 @@ func TestResolveConnectTarget_NoReplicasSkipsPrompt(t *testing.T) { cmd.SetOut(io.Discard) cmd.SetErr(io.Discard) - target := &common.ConnectionTarget{Connect: primary, Credential: primary} + target := &common.ConnectionTarget{ConnectionService: primary, CredentialService: primary} details, err := resolveConnectTarget(context.Background(), cmd, client, "proj-1", target, common.ConnectionDetailsOptions{Role: "tsdbadmin"}, false /*noReplicaPrompt*/) if err != nil { diff --git a/internal/tiger/cmd/replica_connect_test.go b/internal/tiger/cmd/replica_connect_test.go index 36052ce0..281f60ba 100644 --- a/internal/tiger/cmd/replica_connect_test.go +++ b/internal/tiger/cmd/replica_connect_test.go @@ -91,8 +91,8 @@ func TestResolveConnectionTarget_Primary(t *testing.T) { if target.IsReplica { t.Fatal("expected a primary target") } - if util.DerefStr(target.Connect.ServiceId) != "svcprimary" { - t.Errorf("expected connect svcprimary, got %q", util.DerefStr(target.Connect.ServiceId)) + if util.DerefStr(target.ConnectionService.ServiceId) != "svcprimary" { + t.Errorf("expected connect svcprimary, got %q", util.DerefStr(target.ConnectionService.ServiceId)) } } @@ -116,11 +116,11 @@ func TestResolveConnectionTarget_Replica(t *testing.T) { if !target.IsReplica { t.Fatal("expected a replica target") } - if util.DerefStr(target.Connect.ServiceId) != "rep1234567" { - t.Errorf("expected connect rep1234567, got %q", util.DerefStr(target.Connect.ServiceId)) + if util.DerefStr(target.ConnectionService.ServiceId) != "rep1234567" { + t.Errorf("expected connect rep1234567, got %q", util.DerefStr(target.ConnectionService.ServiceId)) } - if util.DerefStr(target.Credential.ServiceId) != "svcprimary" { - t.Errorf("expected credential svcprimary, got %q", util.DerefStr(target.Credential.ServiceId)) + if util.DerefStr(target.CredentialService.ServiceId) != "svcprimary" { + t.Errorf("expected credential svcprimary, got %q", util.DerefStr(target.CredentialService.ServiceId)) } } @@ -147,13 +147,13 @@ func TestBuildConnectionDetailsForTarget_ReplicaPoolerFallback(t *testing.T) { rhost := "replica.example.com" rport := 5432 target := &common.ConnectionTarget{ - Connect: api.Service{ + ConnectionService: api.Service{ ServiceId: util.Ptr("rep1234567"), Name: util.Ptr("reporting-replica"), Endpoint: &api.Endpoint{Host: &rhost, Port: &rport}, }, - Credential: api.Service{ServiceId: util.Ptr("svcprimary"), ProjectId: util.Ptr("proj1")}, - IsReplica: true, + CredentialService: api.Service{ServiceId: util.Ptr("svcprimary"), ProjectId: util.Ptr("proj1")}, + IsReplica: true, } buf := &bytes.Buffer{} @@ -185,7 +185,7 @@ func TestBuildConnectionDetailsForTarget_PrimaryRequiresPooler(t *testing.T) { ServiceId: util.Ptr("svcprimary"), Endpoint: &api.Endpoint{Host: &host, Port: &port}, } - target := &common.ConnectionTarget{Connect: svc, Credential: svc} + target := &common.ConnectionTarget{ConnectionService: svc, CredentialService: svc} cmd := &cobra.Command{} cmd.SetErr(io.Discard) diff --git a/internal/tiger/common/connection.go b/internal/tiger/common/connection.go index c9bf334e..0c227b12 100644 --- a/internal/tiger/common/connection.go +++ b/internal/tiger/common/connection.go @@ -20,10 +20,10 @@ func hasPooler(pooler *api.ConnectionPooler) bool { // for a read replica with no pooler (the connection falls back to direct), or "" // otherwise — including for a non-replica target, so callers need no IsReplica guard. func ReplicaPoolerWarning(target *ConnectionTarget, pooled bool) string { - if !target.IsReplica || !pooled || hasPooler(target.Connect.ConnectionPooler) { + if !target.IsReplica || !pooled || hasPooler(target.ConnectionService.ConnectionPooler) { return "" } - return fmt.Sprintf("read replica %q has no connection pooler; connecting directly instead", util.DerefStr(target.Connect.Name)) + return fmt.Sprintf("read replica %q has no connection pooler; connecting directly instead", util.DerefStr(target.ConnectionService.Name)) } // ConnectionDetailsOptions configures how the connection string is built diff --git a/internal/tiger/common/replica.go b/internal/tiger/common/replica.go index dfd6cf65..f62de88f 100644 --- a/internal/tiger/common/replica.go +++ b/internal/tiger/common/replica.go @@ -9,21 +9,20 @@ import ( "github.com/timescale/tiger-cli/internal/tiger/util" ) -// ConnectionTarget describes where to connect and whose credentials to use. -// Connect supplies the endpoint/pooler, Credential the password — the same -// service for a primary; for a read replica, Connect is the replica and -// Credential is the parent primary whose credentials it shares. +// ConnectionTarget is the service to connect to plus the service whose +// credentials to use. They're the same for a primary; for a read replica the +// CredentialService is the parent primary, whose credentials it shares. type ConnectionTarget struct { - Connect api.Service - Credential api.Service - IsReplica bool + ConnectionService api.Service + CredentialService api.Service + IsReplica bool } -// Details builds connection details for the target — endpoint/pooler from -// Connect, password from Credential. A requested-but-unavailable pooler is a -// hard error for a primary but silently falls back to direct for a replica. +// Details builds the target's connection details. A requested-but-unavailable +// pooler is a hard error for a primary but silently falls back to direct for a +// replica. func (t *ConnectionTarget) Details(opts ConnectionDetailsOptions) (*ConnectionDetails, error) { - details, err := GetConnectionDetailsFor(t.Connect, t.Credential, opts) + details, err := GetConnectionDetailsFor(t.ConnectionService, t.CredentialService, opts) if err != nil { return nil, fmt.Errorf("failed to build connection string: %w", err) } @@ -64,19 +63,19 @@ func IsReadReplica(service api.Service) bool { // credentials against the parent primary, which is fetched here. func ResolveConnectionTarget(ctx context.Context, client api.ClientWithResponsesInterface, projectID string, service api.Service) (*ConnectionTarget, error) { if !IsReadReplica(service) { - return &ConnectionTarget{Connect: service, Credential: service}, nil + return &ConnectionTarget{ConnectionService: service, CredentialService: service}, nil } parentID := util.DerefStr(service.ForkedFrom.ServiceId) if parentID == "" { - return &ConnectionTarget{Connect: service, Credential: service, IsReplica: true}, nil + return &ConnectionTarget{ConnectionService: service, CredentialService: service, IsReplica: true}, nil } parent, err := GetService(ctx, client, projectID, parentID) if err != nil { return nil, fmt.Errorf("failed to fetch parent service %q for read replica: %w", parentID, err) } - return &ConnectionTarget{Connect: service, Credential: *parent, IsReplica: true}, nil + return &ConnectionTarget{ConnectionService: service, CredentialService: *parent, IsReplica: true}, nil } // ResolveConnectionTargetByID fetches a service (which may be a read replica) by @@ -94,13 +93,13 @@ func ResolveConnectionTargetByID(ctx context.Context, client api.ClientWithRespo // replica supplies the endpoint; the primary supplies the credentials. func NewReplicaConnectionTarget(primary api.Service, replica api.ReadReplicaSet) *ConnectionTarget { return &ConnectionTarget{ - Connect: api.Service{ + ConnectionService: api.Service{ ServiceId: replica.Id, Name: replica.Name, Endpoint: replica.Endpoint, ConnectionPooler: replica.ConnectionPooler, }, - Credential: primary, - IsReplica: true, + CredentialService: primary, + IsReplica: true, } } diff --git a/internal/tiger/common/replica_test.go b/internal/tiger/common/replica_test.go index a6c54488..a4294fe3 100644 --- a/internal/tiger/common/replica_test.go +++ b/internal/tiger/common/replica_test.go @@ -77,7 +77,7 @@ func TestResolveConnectionTarget_Primary(t *testing.T) { if target.IsReplica { t.Fatal("expected a primary target") } - if util.DerefStr(target.Connect.ServiceId) != "svcprimary" || util.DerefStr(target.Credential.ServiceId) != "svcprimary" { + if util.DerefStr(target.ConnectionService.ServiceId) != "svcprimary" || util.DerefStr(target.CredentialService.ServiceId) != "svcprimary" { t.Errorf("expected connect and credential to be the primary, got %+v", target) } } @@ -95,12 +95,12 @@ func TestResolveConnectionTarget_Replica(t *testing.T) { t.Fatal("expected a replica target") } // Connect to the replica's own endpoint. - if util.DerefStr(target.Connect.ServiceId) != "rep1234567" { - t.Errorf("expected connect service rep1234567, got %q", util.DerefStr(target.Connect.ServiceId)) + if util.DerefStr(target.ConnectionService.ServiceId) != "rep1234567" { + t.Errorf("expected connect service rep1234567, got %q", util.DerefStr(target.ConnectionService.ServiceId)) } // Credentials resolve against the parent primary (fetched via GetService). - if util.DerefStr(target.Credential.ServiceId) != "svcprimary" { - t.Errorf("expected credential service svcprimary, got %q", util.DerefStr(target.Credential.ServiceId)) + if util.DerefStr(target.CredentialService.ServiceId) != "svcprimary" { + t.Errorf("expected credential service svcprimary, got %q", util.DerefStr(target.CredentialService.ServiceId)) } } @@ -133,7 +133,7 @@ func TestResolveConnectionTargetByID(t *testing.T) { if err != nil { t.Fatalf("unexpected error: %v", err) } - if !target.IsReplica || util.DerefStr(target.Credential.ServiceId) != "svcprimary" { + if !target.IsReplica || util.DerefStr(target.CredentialService.ServiceId) != "svcprimary" { t.Errorf("expected replica target with parent credentials, got %+v", target) } @@ -157,11 +157,11 @@ func TestNewReplicaConnectionTarget(t *testing.T) { if !target.IsReplica { t.Error("expected a replica target") } - if util.DerefStr(target.Connect.ServiceId) != "rep7654321" || util.DerefStr(target.Connect.Endpoint.Host) != host { - t.Errorf("expected connect to carry the replica endpoint, got %+v", target.Connect) + if util.DerefStr(target.ConnectionService.ServiceId) != "rep7654321" || util.DerefStr(target.ConnectionService.Endpoint.Host) != host { + t.Errorf("expected connect to carry the replica endpoint, got %+v", target.ConnectionService) } - if util.DerefStr(target.Credential.ServiceId) != "svcprimary" { - t.Errorf("expected credential to be the primary, got %q", util.DerefStr(target.Credential.ServiceId)) + if util.DerefStr(target.CredentialService.ServiceId) != "svcprimary" { + t.Errorf("expected credential to be the primary, got %q", util.DerefStr(target.CredentialService.ServiceId)) } } @@ -175,7 +175,7 @@ func TestReplicaPoolerWarning(t *testing.T) { noPooler := api.Service{Name: util.Ptr("rep-b")} replica := func(svc api.Service) *ConnectionTarget { - return &ConnectionTarget{Connect: svc, Credential: primaryService(), IsReplica: true} + return &ConnectionTarget{ConnectionService: svc, CredentialService: primaryService(), IsReplica: true} } cases := []struct { @@ -184,7 +184,7 @@ func TestReplicaPoolerWarning(t *testing.T) { pooled bool wantWarning bool }{ - {"primary is never warned", &ConnectionTarget{Connect: noPooler, Credential: noPooler}, true, false}, + {"primary is never warned", &ConnectionTarget{ConnectionService: noPooler, CredentialService: noPooler}, true, false}, {"replica not pooled, no pooler", replica(noPooler), false, false}, {"replica not pooled, has pooler", replica(withPooler), false, false}, {"replica pooled, has pooler", replica(withPooler), true, false}, diff --git a/internal/tiger/common/schema_fetch.go b/internal/tiger/common/schema_fetch.go index f2ea9ad0..53c2fbc2 100644 --- a/internal/tiger/common/schema_fetch.go +++ b/internal/tiger/common/schema_fetch.go @@ -16,7 +16,7 @@ import ( // The connection is forced read-only: introspection only issues SELECTs, so // this is always safe and guards against accidental writes. func FetchServiceSchema(ctx context.Context, target *ConnectionTarget, role string, pooled bool, opts SchemaOptions) (*DatabaseSchema, error) { - if err := CheckServiceReady(target.Connect); err != nil { + if err := CheckServiceReady(target.ConnectionService); err != nil { return nil, err } @@ -33,8 +33,8 @@ func FetchServiceSchema(ctx context.Context, target *ConnectionTarget, role stri defer conn.Close(context.Background()) ident := SchemaIdent{ - ID: util.DerefStr(target.Connect.ServiceId), - Name: util.DerefStr(target.Connect.Name), + ID: util.DerefStr(target.ConnectionService.ServiceId), + Name: util.DerefStr(target.ConnectionService.Name), } return FetchSchemaFromConn(ctx, conn, ident, opts) } From 89993fbc3c58130ec2fa69d9d870c453902132f5 Mon Sep 17 00:00:00 2001 From: Alexandra Primakina Date: Thu, 16 Jul 2026 15:49:25 +0200 Subject: [PATCH 7/8] refactor(db): disambiguate connection-resolution function names Per review feedback, the near-identical resolveConnectionTarget / resolveConnectTarget read like a typo. Rename the args->ConnectionTarget lookup to lookupConnectionTarget and the interactive chooser (->ConnectionDetails) to selectConnection, and update their tests. common.ResolveConnectionTarget keeps its name. Co-Authored-By: Claude Opus 4.8 --- internal/tiger/cmd/db.go | 16 ++++++++-------- internal/tiger/cmd/read_replica.go | 12 +++++------- internal/tiger/cmd/read_replica_test.go | 8 ++++---- internal/tiger/cmd/replica_connect_test.go | 18 +++++++++--------- 4 files changed, 26 insertions(+), 28 deletions(-) diff --git a/internal/tiger/cmd/db.go b/internal/tiger/cmd/db.go index 421ab598..cf96b0c4 100644 --- a/internal/tiger/cmd/db.go +++ b/internal/tiger/cmd/db.go @@ -93,7 +93,7 @@ Examples: return err } - target, err := resolveConnectionTarget(cmd, cfg, args) + target, err := lookupConnectionTarget(cmd, cfg, args) if err != nil { return err } @@ -205,7 +205,7 @@ Examples: // Separate service ID from additional psql flags serviceArgs, psqlFlags := separateServiceAndPsqlArgs(cmd, args) - target, err := resolveConnectionTarget(cmd, cfg, serviceArgs) + target, err := lookupConnectionTarget(cmd, cfg, serviceArgs) if err != nil { return err } @@ -224,7 +224,7 @@ Examples: // Connects straight to a replica named by ID, or offers the interactive // replica menu for a primary. Returns nil details if the user cancels. - details, err := resolveConnectTarget(cmd.Context(), cmd, cfg.Client, cfg.ProjectID, target, opts, dbConnectNoReplicaPrompt) + details, err := selectConnection(cmd.Context(), cmd, cfg.Client, cfg.ProjectID, target, opts, dbConnectNoReplicaPrompt) if err != nil { return err } @@ -293,7 +293,7 @@ Examples: return common.ExitWithCode(common.ExitInvalidParameters, err) } - target, err := resolveConnectionTarget(cmd, cfg, args) + target, err := lookupConnectionTarget(cmd, cfg, args) if err != nil { return common.ExitWithCode(common.ExitInvalidParameters, err) } @@ -372,7 +372,7 @@ Examples: // Resolve the target so a read replica id stores the password against // its parent primary: replicas share the primary's credentials, and // connect/test-connection look the password up against the primary. - target, err := resolveConnectionTarget(cmd, cfg, args) + target, err := lookupConnectionTarget(cmd, cfg, args) if err != nil { return err } @@ -857,7 +857,7 @@ Examples: return err } - target, err := resolveConnectionTarget(cmd, cfg, args) + target, err := lookupConnectionTarget(cmd, cfg, args) if err != nil { return err } @@ -906,10 +906,10 @@ func buildDbCmd() *cobra.Command { return cmd } -// resolveConnectionTarget looks up the target named by args, which may be a +// lookupConnectionTarget looks up the target named by args, which may be a // primary service ID or a read replica set ID. This lets a replica ID work // anywhere a service ID does across the db connection commands. -func resolveConnectionTarget(cmd *cobra.Command, cfg *common.Config, args []string) (*common.ConnectionTarget, error) { +func lookupConnectionTarget(cmd *cobra.Command, cfg *common.Config, args []string) (*common.ConnectionTarget, error) { service, err := getServiceDetailsFunc(cmd, cfg, args) if err != nil { return nil, err diff --git a/internal/tiger/cmd/read_replica.go b/internal/tiger/cmd/read_replica.go index a5abbfba..44b014a3 100644 --- a/internal/tiger/cmd/read_replica.go +++ b/internal/tiger/cmd/read_replica.go @@ -15,13 +15,11 @@ import ( "github.com/timescale/tiger-cli/internal/tiger/util" ) -// resolveConnectTarget resolves the connection details for `tiger db connect`. -// A target that is already a read replica (named directly by ID) connects -// straight to it. A primary, on an interactive terminal, is offered a menu to -// connect to the primary or one of its read replicas. It returns nil details -// when the user cancels. The menu is skipped for non-interactive stdin, when -// prompting is disabled, or when the service has no connectable replicas. -func resolveConnectTarget( +// selectConnection returns the connection details for `tiger db connect`. A +// replica target connects straight through; a primary in an interactive +// terminal is offered a menu to pick the primary or one of its replicas (nil +// details means the user cancelled). +func selectConnection( ctx context.Context, cmd *cobra.Command, client *api.ClientWithResponses, diff --git a/internal/tiger/cmd/read_replica_test.go b/internal/tiger/cmd/read_replica_test.go index 1dbf327d..d063910a 100644 --- a/internal/tiger/cmd/read_replica_test.go +++ b/internal/tiger/cmd/read_replica_test.go @@ -91,11 +91,11 @@ func TestConnectTargetModel_KeySelection(t *testing.T) { } } -// TestResolveConnectTarget_NoReplicasSkipsPrompt verifies that, with no -// connectable replicas, resolveConnectTarget connects to the primary directly +// TestSelectConnection_NoReplicasSkipsPrompt verifies that, with no +// connectable replicas, selectConnection connects to the primary directly // instead of showing a single-option menu (which would block on TTY input in // this test). -func TestResolveConnectTarget_NoReplicasSkipsPrompt(t *testing.T) { +func TestSelectConnection_NoReplicasSkipsPrompt(t *testing.T) { server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.Header().Set("Content-Type", "application/json") w.WriteHeader(http.StatusOK) @@ -126,7 +126,7 @@ func TestResolveConnectTarget_NoReplicasSkipsPrompt(t *testing.T) { cmd.SetErr(io.Discard) target := &common.ConnectionTarget{ConnectionService: primary, CredentialService: primary} - details, err := resolveConnectTarget(context.Background(), cmd, client, "proj-1", target, + details, err := selectConnection(context.Background(), cmd, client, "proj-1", target, common.ConnectionDetailsOptions{Role: "tsdbadmin"}, false /*noReplicaPrompt*/) if err != nil { t.Fatalf("unexpected error: %v", err) diff --git a/internal/tiger/cmd/replica_connect_test.go b/internal/tiger/cmd/replica_connect_test.go index 281f60ba..87e88cdf 100644 --- a/internal/tiger/cmd/replica_connect_test.go +++ b/internal/tiger/cmd/replica_connect_test.go @@ -71,9 +71,9 @@ func standbySvc() api.Service { } } -// TestResolveConnectionTarget_Primary: a plain service resolves to a primary +// TestLookupConnectionTarget_Primary: a plain service resolves to a primary // target (connect == credential, no parent fetch, so no client needed). -func TestResolveConnectionTarget_Primary(t *testing.T) { +func TestLookupConnectionTarget_Primary(t *testing.T) { orig := getServiceDetailsFunc getServiceDetailsFunc = func(cmd *cobra.Command, cfg *common.Config, args []string) (api.Service, error) { return primarySvc(), nil @@ -84,7 +84,7 @@ func TestResolveConnectionTarget_Primary(t *testing.T) { cmd := &cobra.Command{} cmd.SetContext(context.Background()) - target, err := resolveConnectionTarget(cmd, cfg, []string{"svcprimary"}) + target, err := lookupConnectionTarget(cmd, cfg, []string{"svcprimary"}) if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -96,9 +96,9 @@ func TestResolveConnectionTarget_Primary(t *testing.T) { } } -// TestResolveConnectionTarget_Replica: a standby service connects to the replica +// TestLookupConnectionTarget_Replica: a standby service connects to the replica // but resolves credentials against the parent (fetched via the client). -func TestResolveConnectionTarget_Replica(t *testing.T) { +func TestLookupConnectionTarget_Replica(t *testing.T) { orig := getServiceDetailsFunc getServiceDetailsFunc = func(cmd *cobra.Command, cfg *common.Config, args []string) (api.Service, error) { return standbySvc(), nil @@ -109,7 +109,7 @@ func TestResolveConnectionTarget_Replica(t *testing.T) { cmd := &cobra.Command{} cmd.SetContext(context.Background()) - target, err := resolveConnectionTarget(cmd, cfg, []string{"rep1234567"}) + target, err := lookupConnectionTarget(cmd, cfg, []string{"rep1234567"}) if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -124,8 +124,8 @@ func TestResolveConnectionTarget_Replica(t *testing.T) { } } -// TestResolveConnectionTarget_LookupError: a service-lookup failure is surfaced. -func TestResolveConnectionTarget_LookupError(t *testing.T) { +// TestLookupConnectionTarget_LookupError: a service-lookup failure is surfaced. +func TestLookupConnectionTarget_LookupError(t *testing.T) { orig := getServiceDetailsFunc getServiceDetailsFunc = func(cmd *cobra.Command, cfg *common.Config, args []string) (api.Service, error) { return api.Service{}, fmt.Errorf("lookup failed") @@ -136,7 +136,7 @@ func TestResolveConnectionTarget_LookupError(t *testing.T) { cmd := &cobra.Command{} cmd.SetContext(context.Background()) - if _, err := resolveConnectionTarget(cmd, cfg, []string{"x"}); err == nil { + if _, err := lookupConnectionTarget(cmd, cfg, []string{"x"}); err == nil { t.Fatal("expected an error, got nil") } } From ec575557d7aac38a9a1b7774c6b30adf3c0fdfb3 Mon Sep 17 00:00:00 2001 From: Alexandra Primakina Date: Thu, 16 Jul 2026 17:03:22 +0200 Subject: [PATCH 8/8] docs: trim read replica descriptions in CLAUDE.md and spec.md Condense the overlong CLAUDE.md read-replica bullet to a short pointer and fix the stale ConnectionTarget field names (ConnectionService/CredentialService). Drop the forked_from/service-get implementation detail from spec.md, keeping the user-facing behavior. Co-Authored-By: Claude Opus 4.8 --- CLAUDE.md | 1 - internal/tiger/mcp/db_tools.go | 2 +- specs/spec.md | 2 +- specs/spec_mcp.md | 2 +- 4 files changed, 3 insertions(+), 4 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 7a29e28b..399800a7 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -273,7 +273,6 @@ Tiger CLI is a Go-based command-line interface for managing Tiger, the modern da - `service.go` - Service management commands (list, create, get, fork, start, stop, resize, delete, update-password, logs) - `db.go` - Database operation commands (connection-string, connect, test-connection) - `read_replica.go` - Read replica selection flow for `db connect`/`psql`: in an interactive terminal, when the service has one or more active read replicas (listed via the `/replicaSets` API), prompts to connect to the primary or one of the replicas. Skipped when stdin is not a TTY, when `--no-replica-prompt` is set, or when the service has no read replicas. - - Read replica ID as a connection target: the db connection commands (`db connection-string`, `db connect`/`psql`, `db test-connection`, `db schema`) and the `db_execute_query`/`db_schema` MCP tools accept a read replica set ID anywhere a service ID is accepted. The API's `GetService` resolves both primary and read replica IDs, returning the replica's own endpoint plus a `forked_from` link (`is_standby: true`, parent `service_id`). `common.ResolveConnectionTarget` turns a fetched `api.Service` into a `common.ConnectionTarget{Connect, Credential, IsReplica}`: a standby replica connects to its own endpoint (`Connect`) but resolves credentials/password reset against the parent primary (`Credential`, fetched via `GetService`), since read replicas share the primary's credentials. MCP handlers call `common.ResolveConnectionTargetByID`; the CLI's `resolveConnectionTarget` fetches via `getServiceDetailsFunc` (the test seam) then calls `ResolveConnectionTarget`. The interactive `db connect` menu builds a target from a `/replicaSets` entry via `common.NewReplicaConnectionTarget`. `common.IsReadReplica(service)` reports whether a fetched service is a standby. `service update-password` (CLI and MCP) and `db create role` use it to reject a replica ID (a read-only replica can't be written to). `db save-password` instead redirects to the parent (via `resolveConnectionTarget`), storing the password where the connect commands read it. - `config.go` - Configuration management commands (show, set, unset, reset) - `mcp.go` - MCP server commands (install, start, list, get) - `version.go` - Version command diff --git a/internal/tiger/mcp/db_tools.go b/internal/tiger/mcp/db_tools.go index 2d484768..c3ad5332 100644 --- a/internal/tiger/mcp/db_tools.go +++ b/internal/tiger/mcp/db_tools.go @@ -61,7 +61,7 @@ type DBExecuteQueryInput struct { func (DBExecuteQueryInput) Schema() *jsonschema.Schema { schema := util.Must(jsonschema.For[DBExecuteQueryInput](nil)) - schema.Properties["service_id"].Description = "Unique identifier of the service (10-character alphanumeric string). Use service_list to find service IDs. A read replica set ID is also accepted here — passing one runs the query against that read replica (which is read-only and may lag the primary) instead of the primary service." + schema.Properties["service_id"].Description = "Unique identifier of the service (10-character alphanumeric string). Use service_list to find service IDs. A read replica set ID is also accepted here — passing one runs the query against that read replica (which is read-only) instead of the primary service." schema.Properties["service_id"].Examples = []any{"e6ue9697jf", "u8me885b93"} schema.Properties["service_id"].Pattern = "^[a-z0-9]{10}$" diff --git a/specs/spec.md b/specs/spec.md index 60b8dcb7..b09373fe 100644 --- a/specs/spec.md +++ b/specs/spec.md @@ -569,7 +569,7 @@ The `save-password` command accepts passwords through three methods (in order of The `connect` and `psql` commands support passing additional flags directly to the psql client using the `--` separator. Any flags after `--` are passed through to psql unchanged, allowing full access to psql's functionality while maintaining tiger's connection and authentication handling. **Read Replica ID as a Connection Target (all db connection commands):** -The `db connection-string`, `db connect` / `db psql`, `db test-connection`, and `db schema` commands accept a read replica set ID anywhere a service ID is accepted (as the positional argument or the default service). The API resolves a read replica ID through the same service-get endpoint, returning the replica's own connection endpoint along with a `forked_from` link identifying it as a standby of its parent primary. The command connects to the replica's endpoint but resolves credentials (and any password reset) against the parent primary, since read replicas share the primary's credentials. +The `db connection-string`, `db connect` / `db psql`, `db test-connection`, and `db schema` commands accept a read replica set ID anywhere a service ID is accepted (as the positional argument or the default service). The command connects to the replica's endpoint but resolves credentials (and any password reset) against the parent primary, since read replicas share the primary's credentials. **Read Replica Prompt (connect/psql):** When `tiger db connect` / `tiger db psql` runs in an interactive terminal and is given a *primary service* ID, it checks whether that service has any read replicas (read replica sets). If the service has one or more active read replicas, it presents a menu offering to connect to the primary (default) or to one of the existing replicas. If the service has no read replicas, no menu is shown and the command connects to the primary directly. diff --git a/specs/spec_mcp.md b/specs/spec_mcp.md index 4bd58cc7..e748f574 100644 --- a/specs/spec_mcp.md +++ b/specs/spec_mcp.md @@ -344,7 +344,7 @@ Test database connectivity. Execute a SQL query on a service database. **Parameters:** -- `service_id` (string, required): Service ID. A read replica set ID is also accepted here — passing one runs the query against that read replica (which is read-only and may lag the primary) instead of the primary service. Credentials resolve against the parent service. +- `service_id` (string, required): Service ID. A read replica set ID is also accepted here — passing one runs the query against that read replica (which is read-only) instead of the primary service. Credentials resolve against the parent service. - `query` (string, required): SQL query to execute - `parameters` (array, optional): Query parameters for parameterized queries. Values are substituted for $1, $2, etc. placeholders in the query. - `timeout_seconds` (number, optional): Query timeout in seconds (default: 30)