diff --git a/README.md b/README.md index ac527646..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 +- `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 49a446c4..cf96b0c4 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" @@ -56,6 +55,8 @@ 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 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. @@ -92,29 +93,25 @@ Examples: return err } - service, err := getServiceDetailsFunc(cmd, cfg, args) + target, err := lookupConnectionTarget(cmd, cfg, args) if err != nil { return err } - details, err := common.GetConnectionDetails(service, common.ConnectionDetailsOptions{ + details, err := buildConnectionDetailsForTarget(cmd, target, 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 +160,17 @@ 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 to connect straight to that replica, +skipping the prompt. Read replicas share the primary'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 +205,7 @@ Examples: // Separate service ID from additional psql flags serviceArgs, psqlFlags := separateServiceAndPsqlArgs(cmd, args) - service, err := getServiceDetailsFunc(cmd, cfg, serviceArgs) + target, err := lookupConnectionTarget(cmd, cfg, serviceArgs) if err != nil { return err } @@ -219,23 +222,19 @@ 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) + // 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 := selectConnection(cmd.Context(), cmd, cfg.Client, cfg.ProjectID, target, 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 - // 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.CredentialService, details, psqlPath, psqlFlags) }, } @@ -262,6 +261,8 @@ 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 to test connectivity to that replica. + Return Codes: 0: Server is accepting connections normally 1: Server is rejecting connections (e.g., during startup) @@ -292,22 +293,18 @@ Examples: return common.ExitWithCode(common.ExitInvalidParameters, err) } - service, err := getServiceDetailsFunc(cmd, cfg, args) + target, err := lookupConnectionTarget(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, target, 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) } @@ -372,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 := lookupConnectionTarget(cmd, cfg, args) if err != nil { return err } + service := target.CredentialService // Determine password based on precedence: // 1. --password flag with value @@ -415,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 @@ -653,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= @@ -727,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 { @@ -813,8 +825,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. 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 @@ -844,12 +857,14 @@ Examples: return err } - service, err := getServiceDetailsFunc(cmd, cfg, args) + target, err := lookupConnectionTarget(cmd, cfg, args) if err != nil { return err } - schema, err := common.FetchServiceSchema(cmd.Context(), service, dbSchemaRole, dbSchemaPooled, common.SchemaOptions{ + warnReplicaPooler(cmd, target, dbSchemaPooled) + + schema, err := common.FetchServiceSchema(cmd.Context(), target, dbSchemaRole, dbSchemaPooled, common.SchemaOptions{ Schema: dbSchemaSchema, IncludeInternal: dbSchemaInternal, IncludeDefinitions: dbSchemaDefinitions, @@ -891,6 +906,37 @@ func buildDbCmd() *cobra.Command { return cmd } +// 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 lookupConnectionTarget(cmd *cobra.Command, cfg *common.Config, args []string) (*common.ConnectionTarget, error) { + service, err := getServiceDetailsFunc(cmd, cfg, args) + 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() + 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 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 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 func getServiceDetails(cmd *cobra.Command, cfg *common.Config, args []string) (api.Service, error) { // Determine service ID @@ -901,25 +947,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/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/read_replica.go b/internal/tiger/cmd/read_replica.go index 93ab5229..44b014a3 100644 --- a/internal/tiger/cmd/read_replica.go +++ b/internal/tiger/cmd/read_replica.go @@ -15,89 +15,62 @@ 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. -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, projectID string, - primary api.Service, + target *common.ConnectionTarget, 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) + // 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.ConnectionService + replicas, err := fetchReplicaSets(ctx, client, projectID, util.DerefStr(primary.ServiceId)) 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 + // 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) + } } - return details, nil } - // 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) - } - } - - // 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() - } - - 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) - if err != nil { - return nil, err + if chosen.IsReplica { + fmt.Fprintf(cmd.ErrOrStderr(), "Connecting to read replica '%s'...\n", util.DerefStr(chosen.ConnectionService.Name)) } - fmt.Fprintf(cmd.ErrOrStderr(), "Connecting to read replica '%s'...\n", util.DerefStr(replica.Name)) 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 +// 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) + } + } + 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 9eefebdf..d063910a 100644 --- a/internal/tiger/cmd/read_replica_test.go +++ b/internal/tiger/cmd/read_replica_test.go @@ -91,32 +91,11 @@ 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 +// 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) @@ -146,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{ConnectionService: primary, CredentialService: primary} + 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 new file mode 100644 index 00000000..87e88cdf --- /dev/null +++ b/internal/tiger/cmd/replica_connect_test.go @@ -0,0 +1,197 @@ +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" +) + +// 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") + 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 + } + _ = 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 &common.Config{Config: &config.Config{}, ProjectID: "proj1", Client: client} +} + +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"), + Endpoint: &api.Endpoint{Host: &host, Port: &port}, + } +} + +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"), + }, + } +} + +// TestLookupConnectionTarget_Primary: a plain service resolves to a primary +// target (connect == credential, no parent fetch, so no client needed). +func TestLookupConnectionTarget_Primary(t *testing.T) { + orig := getServiceDetailsFunc + getServiceDetailsFunc = func(cmd *cobra.Command, cfg *common.Config, args []string) (api.Service, error) { + return primarySvc(), nil + } + defer func() { getServiceDetailsFunc = orig }() + + cfg := &common.Config{Config: &config.Config{}, ProjectID: "proj1"} + cmd := &cobra.Command{} + cmd.SetContext(context.Background()) + + target, err := lookupConnectionTarget(cmd, cfg, []string{"svcprimary"}) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if target.IsReplica { + t.Fatal("expected a primary target") + } + if util.DerefStr(target.ConnectionService.ServiceId) != "svcprimary" { + t.Errorf("expected connect svcprimary, got %q", util.DerefStr(target.ConnectionService.ServiceId)) + } +} + +// TestLookupConnectionTarget_Replica: a standby service connects to the replica +// but resolves credentials against the parent (fetched via the client). +func TestLookupConnectionTarget_Replica(t *testing.T) { + orig := getServiceDetailsFunc + getServiceDetailsFunc = func(cmd *cobra.Command, cfg *common.Config, args []string) (api.Service, error) { + return standbySvc(), nil + } + defer func() { getServiceDetailsFunc = orig }() + + cfg := serviceClientConfig(t, map[string]api.Service{"svcprimary": primarySvc()}) + cmd := &cobra.Command{} + cmd.SetContext(context.Background()) + + target, err := lookupConnectionTarget(cmd, cfg, []string{"rep1234567"}) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if !target.IsReplica { + t.Fatal("expected a replica target") + } + if util.DerefStr(target.ConnectionService.ServiceId) != "rep1234567" { + t.Errorf("expected connect rep1234567, got %q", util.DerefStr(target.ConnectionService.ServiceId)) + } + if util.DerefStr(target.CredentialService.ServiceId) != "svcprimary" { + t.Errorf("expected credential svcprimary, got %q", util.DerefStr(target.CredentialService.ServiceId)) + } +} + +// 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") + } + defer func() { getServiceDetailsFunc = orig }() + + cfg := &common.Config{Config: &config.Config{}, ProjectID: "proj1"} + cmd := &cobra.Command{} + cmd.SetContext(context.Background()) + + if _, err := lookupConnectionTarget(cmd, cfg, []string{"x"}); err == nil { + t.Fatal("expected an error, got nil") + } +} + +// 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 + target := &common.ConnectionTarget{ + ConnectionService: api.Service{ + ServiceId: util.Ptr("rep1234567"), + Name: util.Ptr("reporting-replica"), + Endpoint: &api.Endpoint{Host: &rhost, Port: &rport}, + }, + CredentialService: api.Service{ServiceId: util.Ptr("svcprimary"), ProjectId: util.Ptr("proj1")}, + IsReplica: true, + } + + buf := &bytes.Buffer{} + cmd := &cobra.Command{} + cmd.SetErr(buf) + cmd.SetOut(io.Discard) + + details, err := buildConnectionDetailsForTarget(cmd, target, 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 + svc := api.Service{ + ServiceId: util.Ptr("svcprimary"), + Endpoint: &api.Endpoint{Host: &host, Port: &port}, + } + target := &common.ConnectionTarget{ConnectionService: svc, CredentialService: svc} + + cmd := &cobra.Command{} + cmd.SetErr(io.Discard) + cmd.SetOut(io.Discard) + + 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/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 ad993b3f..0c227b12 100644 --- a/internal/tiger/common/connection.go +++ b/internal/tiger/common/connection.go @@ -3,12 +3,29 @@ package common import ( "context" "fmt" + "net/url" "github.com/jackc/pgx/v5" "github.com/timescale/tiger-cli/internal/tiger/api" + "github.com/timescale/tiger-cli/internal/tiger/util" ) +// hasPooler reports whether a connection pooler exposes an endpoint. +func hasPooler(pooler *api.ConnectionPooler) bool { + return pooler != nil && pooler.Endpoint != nil +} + +// 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.ConnectionService.ConnectionPooler) { + return "" + } + 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 type ConnectionDetailsOptions struct { // Pooled determines whether to use the pooler endpoint (if available) @@ -58,25 +75,34 @@ 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 GetConnectionDetailsFor(service, service, opts) +} + +// 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 buildConnectionDetails(service.Endpoint, service.ConnectionPooler, service, opts) + return buildConnectionDetails(connService.Endpoint, connService.ConnectionPooler, credService, 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) +// 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) - } - if err := details.RequirePooler(opts.Pooled); err != nil { return nil, 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) @@ -86,16 +112,6 @@ func ConnectToService(ctx context.Context, service api.Service, opts ConnectionD 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. @@ -141,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 54711777..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") @@ -475,13 +499,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 +518,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 +543,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 +565,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 +583,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/replica.go b/internal/tiger/common/replica.go new file mode 100644 index 00000000..f62de88f --- /dev/null +++ b/internal/tiger/common/replica.go @@ -0,0 +1,105 @@ +package common + +import ( + "context" + "fmt" + "net/http" + + "github.com/timescale/tiger-cli/internal/tiger/api" + "github.com/timescale/tiger-cli/internal/tiger/util" +) + +// 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 { + ConnectionService api.Service + CredentialService api.Service + IsReplica bool +} + +// 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.ConnectionService, t.CredentialService, 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 +} + +// 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 { + return nil, ExitWithErrorFromStatusCode(resp.StatusCode(), resp.JSON4XX) + } + if resp.JSON200 == nil { + return nil, fmt.Errorf("empty response from API") + } + 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, 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) { + if !IsReadReplica(service) { + return &ConnectionTarget{ConnectionService: service, CredentialService: service}, nil + } + + parentID := util.DerefStr(service.ForkedFrom.ServiceId) + if parentID == "" { + 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{ConnectionService: service, CredentialService: *parent, IsReplica: true}, 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) +} + +// 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{ + ConnectionService: api.Service{ + ServiceId: replica.Id, + Name: replica.Name, + Endpoint: replica.Endpoint, + ConnectionPooler: replica.ConnectionPooler, + }, + CredentialService: primary, + IsReplica: true, + } +} diff --git a/internal/tiger/common/replica_test.go b/internal/tiger/common/replica_test.go new file mode 100644 index 00000000..a4294fe3 --- /dev/null +++ b/internal/tiger/common/replica_test.go @@ -0,0 +1,201 @@ +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" +) + +// 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") + 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 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"), + 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 TestResolveConnectionTarget_Primary(t *testing.T) { + primary := primaryService() + client := serviceTestClient(t, map[string]api.Service{"svcprimary": primary}) + + target, err := ResolveConnectionTarget(context.Background(), client, "proj1", primary) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if target.IsReplica { + t.Fatal("expected a primary target") + } + 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) + } +} + +func TestResolveConnectionTarget_Replica(t *testing.T) { + primary := primaryService() + replica := replicaService() + client := serviceTestClient(t, map[string]api.Service{"svcprimary": primary}) + + target, err := ResolveConnectionTarget(context.Background(), client, "proj1", replica) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if !target.IsReplica { + t.Fatal("expected a replica target") + } + // Connect to the replica's own endpoint. + 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.CredentialService.ServiceId) != "svcprimary" { + t.Errorf("expected credential service svcprimary, got %q", util.DerefStr(target.CredentialService.ServiceId)) + } +} + +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{}) + + 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 TestResolveConnectionTargetByID(t *testing.T) { + primary := primaryService() + replica := replicaService() + client := serviceTestClient(t, map[string]api.Service{"svcprimary": primary, "rep1234567": replica}) + + // Primary ID → primary target. + target, err := ResolveConnectionTargetByID(context.Background(), client, "proj1", "svcprimary") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if target.IsReplica { + t.Error("expected primary target for a primary ID") + } + + // 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.IsReplica || util.DerefStr(target.CredentialService.ServiceId) != "svcprimary" { + t.Errorf("expected replica target with parent credentials, got %+v", target) + } + + // Unknown ID → error. + if _, err := ResolveConnectionTargetByID(context.Background(), client, "proj1", "missing9999"); err == nil { + t.Error("expected an error for an unknown ID, got nil") + } +} + +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}, + } + + target := NewReplicaConnectionTarget(primary, replica) + if !target.IsReplica { + t.Error("expected a replica target") + } + 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.CredentialService.ServiceId) != "svcprimary" { + t.Errorf("expected credential to be the primary, got %q", util.DerefStr(target.CredentialService.ServiceId)) + } +} + +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")} + + replica := func(svc api.Service) *ConnectionTarget { + return &ConnectionTarget{ConnectionService: svc, CredentialService: primaryService(), IsReplica: true} + } + + cases := []struct { + name string + target *ConnectionTarget + pooled bool + wantWarning bool + }{ + {"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}, + {"replica pooled, no pooler warns", replica(noPooler), true, true}, + } + 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 0ce78c1a..53c2fbc2 100644 --- a/internal/tiger/common/schema_fetch.go +++ b/internal/tiger/common/schema_fetch.go @@ -5,23 +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 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 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, 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.ConnectionService); err != nil { return nil, err } // Introspection runs parameterless statements, so the simple protocol fits. - conn, err := ConnectToService(ctx, service, ConnectionDetailsOptions{ + conn, err := ConnectTarget(ctx, target, ConnectionDetailsOptions{ Pooled: pooled, Role: role, WithPassword: true, @@ -33,8 +33,8 @@ func FetchServiceSchema(ctx context.Context, service api.Service, role string, p defer conn.Close(context.Background()) ident := SchemaIdent{ - ID: util.DerefStr(service.ServiceId), - Name: util.DerefStr(service.Name), + ID: util.DerefStr(target.ConnectionService.ServiceId), + Name: util.DerefStr(target.ConnectionService.Name), } return FetchSchemaFromConn(ctx, conn, ident, opts) } diff --git a/internal/tiger/mcp/db_tools.go b/internal/tiger/mcp/db_tools.go index 62ea933f..c3ad5332 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) 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.ResolveConnectionTargetByID(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, input.Pooled) + + schema, err := common.FetchServiceSchema(ctx, target, 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.ResolveConnectionTargetByID(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, 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.ConnectTarget(queryCtx, target, 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/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 5bd9c951..b09373fe 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:** @@ -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). 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, 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..e748f574 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. @@ -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) 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