diff --git a/CLAUDE.md b/CLAUDE.md index c82f9ae0..3b216156 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -137,7 +137,7 @@ Further conventions: ### Service Refs -A command that identifies a service takes a **ref** — its ID, a read replica set ID, or its name — and resolves it through the API, which also returns the whole resource. Never try to tell an ID from a name in the CLI. `tiger service` and `tiger db` commands resolve through the `ref` filter on the API's service list (`resolveService`). `tiger branch` commands use the API's resolve endpoint, with its `type` parameter set so that a ref can only name a branch (`common.ResolveBranch`, called by `resolveBranch`). The service and branch ref helpers sit side by side in `ref_helper.go`. +A command that identifies a service takes a **ref** — its ID, a read replica set ID, or its name — and resolves it through the API, which also returns the whole resource. Never try to tell an ID from a name in the CLI. `tiger service` commands resolve through the `ref` filter on the API's service list (`resolveService`). `tiger branch` and `tiger db` commands use the API's resolve endpoint, whose `type` parameter decides what a ref can name: only branches for `tiger branch` (`common.ResolveBranch`, called by `resolveBranch`), and anything they can connect to — a service, a read replica set, or a branch — for `tiger db` (`common.ResolveConnectionTarget`, called by `resolveConnectionTarget` in `db.go`). The service and branch ref helpers sit side by side in `ref_helper.go`. - A command that changes the service it resolves uses `resolveServiceForWrite`, which carries the read-only gate; everything else uses `resolveService`. - Help text shows `[name-or-id]` and says the service "can be given by ID or name". Shell completion offers names, falling back to IDs when what is typed matches an ID instead. diff --git a/internal/cmd/db.go b/internal/cmd/db.go index 523a2929..354de19c 100644 --- a/internal/cmd/db.go +++ b/internal/cmd/db.go @@ -1,12 +1,14 @@ package cmd import ( + "context" "errors" "fmt" "github.com/jackc/pgx/v5/pgconn" "github.com/spf13/cobra" + "github.com/timescale/tiger-cli/internal/api" "github.com/timescale/tiger-cli/internal/common" ) @@ -29,26 +31,63 @@ func buildDbCmd(app *common.App) *cobra.Command { return cmd } +// branchRefHelp is the paragraph a db command's help gains for branches, saying +// what passing one does. Branches are a preview, so it is empty without the +// experimental gate. +func branchRefHelp(experimental bool, text string) string { + if !experimental { + return "" + } + return "\n\n" + text +} + // handleDatabaseError adds guidance to the failures whose fix isn't evident from // the message: the readiness sentinels, and a Postgres authentication failure, // which otherwise arrives as a bare pgx error saying nothing about the password // it was rejected for. Every other error passes through unchanged. func handleDatabaseError(err error, target *common.ConnectionTarget) error { + // A branch is managed through `tiger branch`, everything else through + // `tiger service`. The group also names the target in the readiness + // errors, whose sentinels always say "service". + group := "service" + if target.Kind == common.TargetBranch { + group = "branch" + } + switch { case errors.Is(err, common.ErrPaused): - return fmt.Errorf("%w — start it with 'tiger service start %s'", common.ErrPaused, target.ID) + return fmt.Errorf("%s is paused — start it with 'tiger %s start %s'", group, group, target.ID) case errors.Is(err, common.ErrNotReady): - return fmt.Errorf("%w — check its status with 'tiger service get %s' and try again", common.ErrNotReady, target.ID) + return fmt.Errorf("%s is not ready — check its status with 'tiger %s get %s' and try again", group, group, target.ID) case isPostgresAuthenticationError(err): // A read replica shares its primary's credentials, so the password // belongs to the primary rather than the replica connected to. serviceID := target.Credentials.ID - return fmt.Errorf("%w\n\nThe stored password is missing or invalid. Save the current one with 'tiger db save-password %s', or reset it with 'tiger service update-password %s'", - err, serviceID, serviceID) + return fmt.Errorf("%w\n\nThe stored password is missing or invalid. Save the current one with 'tiger db save-password %s', or reset it with 'tiger %s update-password %s'", + err, serviceID, group, serviceID) } return err } +// resolveConnectionTarget resolves the ref a db command was given to what it +// connects to: a service, a read replica set, or (behind the experimental +// gate) a branch, by ID or name. +func resolveConnectionTarget(ctx context.Context, client api.ClientWithResponsesInterface, projectID string, ref serviceRef, experimental bool) (*common.ConnectionTarget, error) { + hint := "Run 'tiger service list' to find the ID you want" + if experimental { + hint = "Run 'tiger service list' or 'tiger branch list' to find the ID you want" + } + + target, err := common.ResolveConnectionTarget(ctx, client, projectID, ref.ref, experimental) + if err != nil { + return nil, withAmbiguousRefHint(err, hint) + } + if err := checkDefaultIsID(target.ID, ref); err != nil { + return nil, err + } + return target, nil +} + // isPostgresAuthenticationError checks if the error is a PostgreSQL authentication failure func isPostgresAuthenticationError(err error) bool { // Check for PostgreSQL error code 28P01 (invalid_password) or 28000 (invalid_authorization_specification) diff --git a/internal/cmd/db_create_role.go b/internal/cmd/db_create_role.go index 81264a8c..9c5f13cd 100644 --- a/internal/cmd/db_create_role.go +++ b/internal/cmd/db_create_role.go @@ -29,7 +29,7 @@ func buildDbCreateRoleCmd(app *common.App) *cobra.Command { The service can be given by ID or name as an argument, or will use the default service from your configuration. A read replica ID is rejected, since replicas -are read-only; create the role on the primary instead. +are read-only; create the role on the primary instead.` + branchRefHelp(app.Experimental, "You can also pass a branch, by ID or name, to create the role on that branch.") + ` By default, a secure random password is auto-generated for the new role. You can: - Provide an explicit password with --password= @@ -100,15 +100,23 @@ PostgreSQL Configuration Parameters That May Be Set: } // Gated here rather than letting the server reject the CREATE ROLE. - service, err := resolveServiceForWrite(cmd.Context(), cfg, client, projectID, serviceRef) + // Refuse the blanket case before resolving, as resolveServiceForWrite + // does, then gate on the target's own tag. + if cfg.ReadOnly.BlocksAll() { + return common.ErrReadOnly + } + target, err := resolveConnectionTarget(cmd.Context(), client, projectID, serviceRef, app.Experimental) if err != nil { return err } + if err := common.CheckReadOnly(cfg, target.Environment); err != nil { + return err + } // A read replica is read-only, so a role can't be created there. - if common.IsReadReplica(*service) { + if target.Kind == common.TargetReadReplica { return fmt.Errorf("'%s' is a read replica; create the role on its primary service '%s' instead", - service.ServiceID, util.DerefStr(service.ForkedFrom.ServiceID)) + target.ID, target.Credentials.ID) } // Get password @@ -118,7 +126,7 @@ PostgreSQL Configuration Parameters That May Be Set: } // Build connection string - details, err := common.GetConnectionDetails(cfg, common.NewServiceConnectionTarget(*service), common.ConnectionDetailsOptions{ + details, err := common.GetConnectionDetails(cfg, target, common.ConnectionDetailsOptions{ Pooled: false, Role: "tsdbadmin", // Use admin role to create new roles WithPassword: true, @@ -143,7 +151,7 @@ PostgreSQL Configuration Parameters That May Be Set: } // Save password to storage with the new role name - result, err := common.SavePasswordWithResult(cfg, common.ServicePasswordTarget(*service), rolePassword, name) + result, err := common.SavePasswordWithResult(cfg, target.Credentials.PasswordTarget, rolePassword, name) if err != nil { cmd.PrintErrf("Warning: %s\n", result.Message) } else if !result.Success { diff --git a/internal/cmd/db_create_role_test.go b/internal/cmd/db_create_role_test.go index e8046ccc..6c5c034d 100644 --- a/internal/cmd/db_create_role_test.go +++ b/internal/cmd/db_create_role_test.go @@ -51,7 +51,7 @@ func TestDbCreateRoleCmd(t *testing.T) { name: "read-only prod refuses PROD service", args: []string{"db", "create", "role", "svc-12345", "--name", "ai_analyst"}, opts: []runOption{withConfig(map[string]any{"read_only": "prod"})}, - mock: expectTaggedService("PROD"), + mock: expectTaggedTarget("PROD"), wantErr: `this operation is not allowed on services tagged PROD while read_only is set to "prod"`, }, { @@ -62,7 +62,7 @@ func TestDbCreateRoleCmd(t *testing.T) { opts: []runOption{withConfig(map[string]any{"read_only": "prod"})}, mock: func(m *mocks.MockClientWithResponsesInterface) { tag := "DEV" - expectResolveRef(m, "svc-12345", sampleService(func(s *api.Service) { + expectResolveTarget(m, "svc-12345", sampleService(func(s *api.Service) { s.Metadata = &api.ServiceMetadata{Environment: &tag} s.Endpoint = nil })) @@ -76,7 +76,7 @@ func TestDbCreateRoleCmd(t *testing.T) { args: []string{"db", "create", "role", "--name", "ai_analyst"}, opts: []runOption{withConfig(map[string]any{"service_id": "svc-12345"})}, mock: func(m *mocks.MockClientWithResponsesInterface) { - expectResolveRef(m, "svc-12345", sampleService(func(s *api.Service) { + expectResolveTarget(m, "svc-12345", sampleService(func(s *api.Service) { s.Endpoint = nil })) }, @@ -86,24 +86,24 @@ func TestDbCreateRoleCmd(t *testing.T) { name: "network error", args: []string{"db", "create", "role", "svc-12345", "--name", "ai_analyst"}, mock: func(m *mocks.MockClientWithResponsesInterface) { - expectResolveRefError(m, "svc-12345", errors.New("connection refused")) + expectResolveTargetError(m, "svc-12345", errors.New("connection refused")) }, - wantErr: `failed to resolve service 'svc-12345': connection refused`, + wantErr: `failed to resolve service or read replica set 'svc-12345': connection refused`, }, { name: "service not found", args: []string{"db", "create", "role", "svc-12345", "--name", "ai_analyst"}, mock: func(m *mocks.MockClientWithResponsesInterface) { - expectResolveRefNotFound(m, "svc-12345") + expectResolveTargetNotFound(m, "svc-12345") }, - wantErr: "service 'svc-12345' not found", + wantErr: "service or read replica set 'svc-12345' not found", checks: []checkFunc{checkExitCode(common.ExitServiceNotFound)}, }, { name: "ambiguous name refused", args: []string{"db", "create", "role", "my-api-db", "--name", "ai_analyst"}, mock: func(m *mocks.MockClientWithResponsesInterface) { - expectResolveRefStatus(m, "my-api-db", http.StatusBadRequest, &api.Error{Message: new("ambiguous service name matches multiple services")}) + expectResolveTargetStatus(m, "my-api-db", http.StatusBadRequest, &api.ClientError{Message: new("ambiguous service name matches multiple services")}) }, wantErr: "ambiguous service name matches multiple services\nRun 'tiger service list' to find the ID you want", checks: []checkFunc{checkExitCode(common.ExitInvalidParameters)}, @@ -112,7 +112,7 @@ func TestDbCreateRoleCmd(t *testing.T) { name: "nil response body", args: []string{"db", "create", "role", "svc-12345", "--name", "ai_analyst"}, mock: func(m *mocks.MockClientWithResponsesInterface) { - expectResolveRefStatus(m, "svc-12345", http.StatusOK, nil) + expectResolveTargetStatus(m, "svc-12345", http.StatusOK, nil) }, wantErr: "empty response from API", }, @@ -120,7 +120,8 @@ func TestDbCreateRoleCmd(t *testing.T) { name: "read replica rejected", args: []string{"db", "create", "role", "rep-67890", "--name", "ai_analyst"}, mock: func(m *mocks.MockClientWithResponsesInterface) { - expectResolveRef(m, "rep-67890", sampleReplica()) + expectResolveTarget(m, "rep-67890", sampleReplica()) + expectGetService(m, "svc-12345", sampleService()) }, wantErr: "'rep-67890' is a read replica; create the role on its primary service 'svc-12345' instead", }, @@ -131,7 +132,7 @@ func TestDbCreateRoleCmd(t *testing.T) { args: []string{"db", "create", "role", "svc-12345", "--name", "ai_analyst"}, opts: []runOption{withGenerateSecurePassword(32, "", errors.New("entropy unavailable"))}, mock: func(m *mocks.MockClientWithResponsesInterface) { - expectResolveRef(m, "svc-12345", sampleService()) + expectResolveTarget(m, "svc-12345", sampleService()) }, wantErr: "failed to determine password: entropy unavailable", }, @@ -139,12 +140,21 @@ func TestDbCreateRoleCmd(t *testing.T) { name: "endpoint not available", args: []string{"db", "create", "role", "svc-12345", "--name", "ai_analyst"}, mock: func(m *mocks.MockClientWithResponsesInterface) { - expectResolveRef(m, "svc-12345", sampleService(func(s *api.Service) { + expectResolveTarget(m, "svc-12345", sampleService(func(s *api.Service) { s.Endpoint = nil })) }, wantErr: "failed to build connection string: service endpoint not available", }, + { + // Branches are a preview, so the help mentions them only behind + // the gate. The generated docs/cli reference covers the ungated + // help. + name: "help mentions branches when experimental", + args: []string{"db", "create", "role", "--help"}, + opts: []runOption{withExperimental()}, + wantStdout: matchHelp("You can also pass a branch, by ID or name, to create the role on that branch."), + }, }) } diff --git a/internal/cmd/db_extensions_list.go b/internal/cmd/db_extensions_list.go index b3390024..a38257ef 100644 --- a/internal/cmd/db_extensions_list.go +++ b/internal/cmd/db_extensions_list.go @@ -23,7 +23,7 @@ extension's version, the schema it lives in, and its description. The service can be given by ID or name as an argument, or will use the default service from your configuration. You can also pass a read replica set ID to list the extensions on that replica. The connection is opened in Tiger Cloud's -immutable read-only mode.`, +immutable read-only mode.` + branchRefHelp(app.Experimental, "You can also pass a branch, by ID or name, to list the extensions on that\nbranch."), Example: ` # List the extensions installed in the default service tiger db extensions list @@ -46,12 +46,7 @@ immutable read-only mode.`, return err } - service, err := resolveService(cmd.Context(), client, projectID, serviceRef) - if err != nil { - return err - } - - target, err := common.NewConnectionTarget(cmd.Context(), client, projectID, *service) + target, err := resolveConnectionTarget(cmd.Context(), client, projectID, serviceRef, app.Experimental) if err != nil { return err } diff --git a/internal/cmd/db_extensions_list_test.go b/internal/cmd/db_extensions_list_test.go index f95f726e..bb86af06 100644 --- a/internal/cmd/db_extensions_list_test.go +++ b/internal/cmd/db_extensions_list_test.go @@ -38,7 +38,7 @@ func withFetchExtensions(wantTarget *common.ConnectionTarget, wantRole string, e func TestDbExtensionsListCmd(t *testing.T) { setupGetWithStatus := func(status api.DeployStatus) func(m *mocks.MockClientWithResponsesInterface) { return func(m *mocks.MockClientWithResponsesInterface) { - expectResolveRef(m, "svc-12345", sampleService(func(s *api.Service) { + expectResolveTarget(m, "svc-12345", sampleService(func(s *api.Service) { s.Status = status })) } @@ -53,11 +53,9 @@ func TestDbExtensionsListCmd(t *testing.T) { primaryTarget := common.NewServiceConnectionTarget(primary) replica := sampleReplica() // A replica connects to its own endpoint with its primary's credentials. - replicaTarget := common.NewServiceConnectionTarget(replica) - replicaTarget.Kind = common.TargetReadReplica - replicaTarget.Credentials = common.ServiceCredentials(primary) + replicaTarget := common.NewReplicaConnectionTarget(replica, common.ServiceCredentials(primary)) setupGetReplica := func(m *mocks.MockClientWithResponsesInterface) { - expectResolveRef(m, "rep-67890", replica) + expectResolveTarget(m, "rep-67890", replica) expectGetService(m, "svc-12345", primary) } @@ -90,24 +88,24 @@ func TestDbExtensionsListCmd(t *testing.T) { name: "network error", args: []string{"db", "extensions", "list", "svc-12345"}, mock: func(m *mocks.MockClientWithResponsesInterface) { - expectResolveRefError(m, "svc-12345", errors.New("connection refused")) + expectResolveTargetError(m, "svc-12345", errors.New("connection refused")) }, - wantErr: `failed to resolve service 'svc-12345': connection refused`, + wantErr: `failed to resolve service or read replica set 'svc-12345': connection refused`, }, { name: "service not found", args: []string{"db", "extensions", "list", "svc-12345"}, mock: func(m *mocks.MockClientWithResponsesInterface) { - expectResolveRefNotFound(m, "svc-12345") + expectResolveTargetNotFound(m, "svc-12345") }, - wantErr: "service 'svc-12345' not found", + wantErr: "service or read replica set 'svc-12345' not found", checks: []checkFunc{checkExitCode(common.ExitServiceNotFound)}, }, { name: "nil response body", args: []string{"db", "extensions", "list", "svc-12345"}, mock: func(m *mocks.MockClientWithResponsesInterface) { - expectResolveRefStatus(m, "svc-12345", http.StatusOK, nil) + expectResolveTargetStatus(m, "svc-12345", http.StatusOK, nil) }, wantErr: "empty response from API", }, @@ -115,7 +113,7 @@ func TestDbExtensionsListCmd(t *testing.T) { name: "read replica parent lookup fails", args: []string{"db", "extensions", "list", "rep-67890"}, mock: func(m *mocks.MockClientWithResponsesInterface) { - expectResolveRef(m, "rep-67890", replica) + expectResolveTarget(m, "rep-67890", replica) m.EXPECT().GetServiceWithResponse(validCtx, testProjectID, "svc-12345"). Return(nil, errors.New("connection refused")) }, @@ -133,6 +131,27 @@ func TestDbExtensionsListCmd(t *testing.T) { mock: setupGetWithStatus(api.DeployStatusQUEUED), wantErr: notReadyMsg("svc-12345"), }, + { + // Branches are a preview, so without the gate a ref naming one is + // reported as not found, without mentioning branches. + name: "branch not found without experimental", + args: []string{"db", "extensions", "list", "test-branch"}, + mock: func(m *mocks.MockClientWithResponsesInterface) { + expectResolveTarget(m, "test-branch", sampleBranch()) + }, + wantErr: "service or read replica set 'test-branch' not found", + checks: []checkFunc{checkExitCode(common.ExitServiceNotFound)}, + }, + { + // A branch is started through `tiger branch`, not `tiger service`. + name: "branch paused", + args: []string{"db", "extensions", "list", "br-12345"}, + opts: []runOption{withExperimental()}, + mock: func(m *mocks.MockClientWithResponsesInterface) { + expectResolveTarget(m, "br-12345", sampleBranch(branchStatus(api.DeployStatusPAUSED))) + }, + wantErr: "branch is paused — start it with 'tiger branch start br-12345'", + }, { name: "fetch fails", args: []string{"db", "extensions", "list", "svc-12345"}, @@ -228,6 +247,14 @@ func TestDbExtensionsListCmd(t *testing.T) { opts: []runOption{withFetchExtensions(replicaTarget, "tsdbadmin", []common.Extension{}, nil)}, wantStdout: "[]\n", }, + { + // A branch connects with its own credentials, not its parent's. + name: "lists the extensions of a branch", + args: []string{"db", "extensions", "list", "br-12345", "-o", "json"}, + mock: func(m *mocks.MockClientWithResponsesInterface) { expectResolveTarget(m, "br-12345", sampleBranch()) }, + opts: []runOption{withExperimental(), withFetchExtensions(common.NewBranchConnectionTarget(sampleBranch()), "tsdbadmin", []common.Extension{}, nil)}, + wantStdout: "[]\n", + }, { name: "passes the role down to the fetch", args: []string{"db", "extensions", "list", "svc-12345", "--role", "reader", "-o", "json"}, @@ -235,5 +262,14 @@ func TestDbExtensionsListCmd(t *testing.T) { opts: []runOption{withFetchExtensions(primaryTarget, "reader", []common.Extension{}, nil)}, wantStdout: "[]\n", }, + { + // Branches are a preview, so the help mentions them only behind + // the gate. The generated docs/cli reference covers the ungated + // help. + name: "help mentions branches when experimental", + args: []string{"db", "extensions", "list", "--help"}, + opts: []runOption{withExperimental()}, + wantStdout: matchHelp("You can also pass a branch, by ID or name, to list the extensions on that\nbranch."), + }, }) } diff --git a/internal/cmd/db_ping.go b/internal/cmd/db_ping.go index 0556fb88..86cd14e0 100644 --- a/internal/cmd/db_ping.go +++ b/internal/cmd/db_ping.go @@ -29,7 +29,7 @@ The service can be given by ID or name 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. +You can also pass a read replica set ID to test connectivity to that replica.` + branchRefHelp(app.Experimental, "You can also pass a branch, by ID or name, to test connectivity to that branch.") + ` Return Codes: 0: Server is accepting connections normally @@ -64,12 +64,7 @@ Return Codes: return common.ExitWithCode(common.ExitInvalidParameters, err) } - service, err := resolveService(cmd.Context(), client, projectID, serviceRef) - if err != nil { - return common.ExitWithCode(common.ExitInvalidParameters, err) - } - - target, err := common.NewConnectionTarget(cmd.Context(), client, projectID, *service) + target, err := resolveConnectionTarget(cmd.Context(), client, projectID, serviceRef, app.Experimental) if err != nil { return common.ExitWithCode(common.ExitInvalidParameters, err) } diff --git a/internal/cmd/db_ping_test.go b/internal/cmd/db_ping_test.go index d3d08665..3edc7ed5 100644 --- a/internal/cmd/db_ping_test.go +++ b/internal/cmd/db_ping_test.go @@ -19,7 +19,7 @@ import ( func TestDbPingCmd(t *testing.T) { setupGet := func(m *mocks.MockClientWithResponsesInterface) { - expectResolveRef(m, "svc-12345", sampleService()) + expectResolveTarget(m, "svc-12345", sampleService()) } runCmdTests(t, []cmdTest{ @@ -51,9 +51,9 @@ func TestDbPingCmd(t *testing.T) { args: []string{"db", "ping"}, opts: []runOption{withConfig(map[string]any{"service_id": "svc-12345"})}, mock: func(m *mocks.MockClientWithResponsesInterface) { - expectResolveRefError(m, "svc-12345", errors.New("connection refused")) + expectResolveTargetError(m, "svc-12345", errors.New("connection refused")) }, - wantErr: `failed to resolve service 'svc-12345': connection refused`, + wantErr: `failed to resolve service or read replica set 'svc-12345': connection refused`, checks: []checkFunc{checkExitCode(common.ExitInvalidParameters)}, }, { @@ -65,25 +65,25 @@ func TestDbPingCmd(t *testing.T) { name: "network error", args: []string{"db", "ping", "svc-12345"}, mock: func(m *mocks.MockClientWithResponsesInterface) { - expectResolveRefError(m, "svc-12345", errors.New("connection refused")) + expectResolveTargetError(m, "svc-12345", errors.New("connection refused")) }, - wantErr: `failed to resolve service 'svc-12345': connection refused`, + wantErr: `failed to resolve service or read replica set 'svc-12345': connection refused`, checks: []checkFunc{checkExitCode(common.ExitInvalidParameters)}, }, { name: "service not found", args: []string{"db", "ping", "svc-12345"}, mock: func(m *mocks.MockClientWithResponsesInterface) { - expectResolveRefNotFound(m, "svc-12345") + expectResolveTargetNotFound(m, "svc-12345") }, - wantErr: "service 'svc-12345' not found", + wantErr: "service or read replica set 'svc-12345' not found", checks: []checkFunc{checkExitCode(common.ExitInvalidParameters)}, }, { name: "ambiguous name refused", args: []string{"db", "ping", "my-api-db"}, mock: func(m *mocks.MockClientWithResponsesInterface) { - expectResolveRefStatus(m, "my-api-db", http.StatusBadRequest, &api.Error{Message: new("ambiguous service name matches multiple services")}) + expectResolveTargetStatus(m, "my-api-db", http.StatusBadRequest, &api.ClientError{Message: new("ambiguous service name matches multiple services")}) }, wantErr: "ambiguous service name matches multiple services\nRun 'tiger service list' to find the ID you want", checks: []checkFunc{checkExitCode(common.ExitInvalidParameters)}, @@ -92,7 +92,7 @@ func TestDbPingCmd(t *testing.T) { name: "nil response body", args: []string{"db", "ping", "svc-12345"}, mock: func(m *mocks.MockClientWithResponsesInterface) { - expectResolveRefStatus(m, "svc-12345", http.StatusOK, nil) + expectResolveTargetStatus(m, "svc-12345", http.StatusOK, nil) }, wantErr: "empty response from API", checks: []checkFunc{checkExitCode(common.ExitInvalidParameters)}, @@ -117,7 +117,7 @@ func TestDbPingCmd(t *testing.T) { name: "unreachable server", args: []string{"db", "ping", "svc-12345"}, mock: func(m *mocks.MockClientWithResponsesInterface) { - expectResolveRef(m, "svc-12345", sampleService(func(s *api.Service) { + expectResolveTarget(m, "svc-12345", sampleService(func(s *api.Service) { s.Endpoint = &api.Endpoint{Host: new("127.0.0.1"), Port: new(1)} })) }, @@ -136,7 +136,7 @@ func TestDbPingCmd(t *testing.T) { name: "connection timeout", args: []string{"db", "ping", "svc-12345", "--timeout", "250ms"}, mock: func(m *mocks.MockClientWithResponsesInterface) { - expectResolveRef(m, "svc-12345", sampleService(func(s *api.Service) { + expectResolveTarget(m, "svc-12345", sampleService(func(s *api.Service) { s.Endpoint = &api.Endpoint{Host: new("192.0.2.1"), Port: new(5432)} })) }, @@ -148,6 +148,15 @@ func TestDbPingCmd(t *testing.T) { wantStderr: "Connection timeout after 250ms\n", checks: []checkFunc{checkExitCode(common.ExitTimeout)}, }, + { + // Branches are a preview, so the help mentions them only behind + // the gate. The generated docs/cli reference covers the ungated + // help. + name: "help mentions branches when experimental", + args: []string{"db", "ping", "--help"}, + opts: []runOption{withExperimental()}, + wantStdout: matchHelp("You can also pass a branch, by ID or name, to test connectivity to that branch."), + }, }) } diff --git a/internal/cmd/db_psql.go b/internal/cmd/db_psql.go index 0c34dbe7..559a9e6f 100644 --- a/internal/cmd/db_psql.go +++ b/internal/cmd/db_psql.go @@ -56,7 +56,7 @@ 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.`, +skipping the prompt. Read replicas share the primary's credentials.` + branchRefHelp(app.Experimental, "You can also pass a branch, by ID or name, to connect to that branch. A branch\nhas its own credentials and no read replicas."), Example: ` # Connect to default service tiger db psql @@ -98,12 +98,7 @@ skipping the prompt. Read replicas share the primary's credentials.`, return err } - service, err := resolveService(cmd.Context(), client, projectID, serviceRef) - if err != nil { - return err - } - - target, err := common.NewConnectionTarget(cmd.Context(), client, projectID, *service) + target, err := resolveConnectionTarget(cmd.Context(), client, projectID, serviceRef, app.Experimental) if err != nil { return err } @@ -184,6 +179,7 @@ func selectConnection( chosen := target // Offer the replica menu only for a primary on an interactive terminal. + // Branches have no read replicas. if target.Kind == common.TargetService && !noReplicaPrompt && util.IsTerminal(cmd.InOrStdin()) && util.IsTerminal(cmd.ErrOrStderr()) { replicas, err := fetchReplicaSets(ctx, client, projectID, target.ID) if err != nil { diff --git a/internal/cmd/db_psql_test.go b/internal/cmd/db_psql_test.go index 69925a17..8fdfaf17 100644 --- a/internal/cmd/db_psql_test.go +++ b/internal/cmd/db_psql_test.go @@ -59,32 +59,32 @@ func TestDbPsqlCmd(t *testing.T) { args: []string{"db", "psql", "--", "-c", "SELECT 1;"}, opts: []runOption{withConfig(map[string]any{"service_id": "svc-12345"})}, mock: func(m *mocks.MockClientWithResponsesInterface) { - expectResolveRefError(m, "svc-12345", errors.New("connection refused")) + expectResolveTargetError(m, "svc-12345", errors.New("connection refused")) }, - wantErr: `failed to resolve service 'svc-12345': connection refused`, + wantErr: `failed to resolve service or read replica set 'svc-12345': connection refused`, }, { name: "service ID before -- separator", args: []string{"db", "psql", "svc-12345", "--", "-c", "SELECT 1;"}, mock: func(m *mocks.MockClientWithResponsesInterface) { - expectResolveRefError(m, "svc-12345", errors.New("connection refused")) + expectResolveTargetError(m, "svc-12345", errors.New("connection refused")) }, - wantErr: `failed to resolve service 'svc-12345': connection refused`, + wantErr: `failed to resolve service or read replica set 'svc-12345': connection refused`, }, { name: "service not found", args: []string{"db", "psql", "svc-12345"}, mock: func(m *mocks.MockClientWithResponsesInterface) { - expectResolveRefNotFound(m, "svc-12345") + expectResolveTargetNotFound(m, "svc-12345") }, - wantErr: "service 'svc-12345' not found", + wantErr: "service or read replica set 'svc-12345' not found", checks: []checkFunc{checkExitCode(common.ExitServiceNotFound)}, }, { name: "ambiguous name refused", args: []string{"db", "psql", "my-api-db"}, mock: func(m *mocks.MockClientWithResponsesInterface) { - expectResolveRefStatus(m, "my-api-db", http.StatusBadRequest, &api.Error{Message: new("ambiguous service name matches multiple services")}) + expectResolveTargetStatus(m, "my-api-db", http.StatusBadRequest, &api.ClientError{Message: new("ambiguous service name matches multiple services")}) }, wantErr: "ambiguous service name matches multiple services\nRun 'tiger service list' to find the ID you want", checks: []checkFunc{checkExitCode(common.ExitInvalidParameters)}, @@ -93,7 +93,7 @@ func TestDbPsqlCmd(t *testing.T) { name: "nil response body", args: []string{"db", "psql", "svc-12345"}, mock: func(m *mocks.MockClientWithResponsesInterface) { - expectResolveRefStatus(m, "svc-12345", http.StatusOK, nil) + expectResolveTargetStatus(m, "svc-12345", http.StatusOK, nil) }, wantErr: "empty response from API", }, @@ -101,7 +101,7 @@ func TestDbPsqlCmd(t *testing.T) { name: "parent fetch fails for read replica", args: []string{"db", "psql", "rep-67890"}, mock: func(m *mocks.MockClientWithResponsesInterface) { - expectResolveRef(m, "rep-67890", sampleReplica()) + expectResolveTarget(m, "rep-67890", sampleReplica()) m.EXPECT().GetServiceWithResponse(validCtx, testProjectID, "svc-12345"). Return(nil, errors.New("connection refused")) }, @@ -112,7 +112,7 @@ func TestDbPsqlCmd(t *testing.T) { args: []string{"db", "psql", "svc-12345"}, opts: []runOption{withEnv("PATH", "/nonexistent")}, mock: func(m *mocks.MockClientWithResponsesInterface) { - expectResolveRef(m, "svc-12345", sampleService()) + expectResolveTarget(m, "svc-12345", sampleService()) }, wantErr: "psql not found. Install the PostgreSQL client tools", }, @@ -123,7 +123,7 @@ func TestDbPsqlCmd(t *testing.T) { args: []string{"db", "psql", "svc-12345"}, opts: []runOption{withEnv("PATH", psqlDir)}, mock: func(m *mocks.MockClientWithResponsesInterface) { - expectResolveRef(m, "svc-12345", sampleService(noEndpoint)) + expectResolveTarget(m, "svc-12345", sampleService(noEndpoint)) }, wantErr: "failed to build connection string: service endpoint not available", }, @@ -132,7 +132,7 @@ func TestDbPsqlCmd(t *testing.T) { args: []string{"db", "psql", "svc-12345", "--no-replica-prompt"}, opts: []runOption{withIsTerminal(true), withEnv("PATH", psqlDir)}, mock: func(m *mocks.MockClientWithResponsesInterface) { - expectResolveRef(m, "svc-12345", sampleService(noEndpoint)) + expectResolveTarget(m, "svc-12345", sampleService(noEndpoint)) }, wantErr: "failed to build connection string: service endpoint not available", }, @@ -141,7 +141,7 @@ func TestDbPsqlCmd(t *testing.T) { args: []string{"db", "psql", "rep-67890"}, opts: []runOption{withIsTerminal(true), withEnv("PATH", psqlDir)}, mock: func(m *mocks.MockClientWithResponsesInterface) { - expectResolveRef(m, "rep-67890", sampleReplica(noEndpoint)) + expectResolveTarget(m, "rep-67890", sampleReplica(func(r *api.ReadReplicaSet) { r.Endpoint = nil })) expectGetService(m, "svc-12345", sampleService()) }, wantErr: "failed to build connection string: service endpoint not available", @@ -151,7 +151,7 @@ func TestDbPsqlCmd(t *testing.T) { args: []string{"db", "psql", "svc-12345"}, opts: []runOption{withIsTerminal(true), withEnv("PATH", psqlDir)}, mock: func(m *mocks.MockClientWithResponsesInterface) { - expectResolveRef(m, "svc-12345", sampleService(noEndpoint)) + expectResolveTarget(m, "svc-12345", sampleService(noEndpoint)) m.EXPECT().GetReplicaSetsWithResponse(validCtx, testProjectID, "svc-12345"). Return(nil, errors.New("connection refused")) }, @@ -163,7 +163,7 @@ func TestDbPsqlCmd(t *testing.T) { args: []string{"db", "psql", "svc-12345"}, opts: []runOption{withIsTerminal(true), withEnv("PATH", psqlDir)}, mock: func(m *mocks.MockClientWithResponsesInterface) { - expectResolveRef(m, "svc-12345", sampleService(noEndpoint)) + expectResolveTarget(m, "svc-12345", sampleService(noEndpoint)) m.EXPECT().GetReplicaSetsWithResponse(validCtx, testProjectID, "svc-12345"). Return(&api.GetReplicaSetsResponse{ HTTPResponse: httpResponse(http.StatusOK), @@ -177,7 +177,7 @@ func TestDbPsqlCmd(t *testing.T) { args: []string{"db", "psql", "svc-12345"}, opts: []runOption{withIsTerminal(true), withEnv("PATH", psqlDir)}, mock: func(m *mocks.MockClientWithResponsesInterface) { - expectResolveRef(m, "svc-12345", sampleService(noEndpoint)) + expectResolveTarget(m, "svc-12345", sampleService(noEndpoint)) replicas := []api.ReadReplicaSet{ { ID: "rep-1", @@ -203,7 +203,7 @@ func TestDbPsqlCmd(t *testing.T) { args: []string{"db", "psql", "svc-12345", "--pooled"}, opts: []runOption{withEnv("PATH", psqlDir)}, mock: func(m *mocks.MockClientWithResponsesInterface) { - expectResolveRef(m, "svc-12345", sampleService()) + expectResolveTarget(m, "svc-12345", sampleService()) }, wantErr: "connection pooler not available for this service", }, @@ -216,7 +216,7 @@ func TestDbPsqlCmd(t *testing.T) { name: "non-auth connection error surfaces directly", args: []string{"db", "psql", "svc-12345"}, mock: func(m *mocks.MockClientWithResponsesInterface) { - expectResolveRef(m, "svc-12345", sampleService(func(s *api.Service) { + expectResolveTarget(m, "svc-12345", sampleService(func(s *api.Service) { s.Endpoint = &api.Endpoint{Host: new("127.0.0.1"), Port: new(1)} })) }, @@ -228,6 +228,15 @@ func TestDbPsqlCmd(t *testing.T) { }), wantStderr: matchPrefix("Warning: could not retrieve stored password: secret not found in keyring\n"), }, + { + // Branches are a preview, so the help mentions them only behind + // the gate. The generated docs/cli reference covers the ungated + // help. + name: "help mentions branches when experimental", + args: []string{"db", "psql", "--help"}, + opts: []runOption{withExperimental()}, + wantStdout: matchHelp("You can also pass a branch, by ID or name, to connect to that branch."), + }, }) } diff --git a/internal/cmd/db_query.go b/internal/cmd/db_query.go index 8ffcdb3c..8319904d 100644 --- a/internal/cmd/db_query.go +++ b/internal/cmd/db_query.go @@ -34,7 +34,7 @@ local psql installation. The service can be given by ID or name as an argument, or will use the default service from your configuration. You can also pass a read replica set ID to -query that replica. +query that replica.` + branchRefHelp(app.Experimental, "You can also pass a branch, by ID or name, to query that branch.") + ` The query comes from --command, from the SQL file named by --file, or, if neither is given, from stdin. @@ -95,12 +95,7 @@ read-only while leaving DEV services writable.`, return err } - service, err := resolveService(cmd.Context(), client, projectID, serviceRef) - if err != nil { - return err - } - - target, err := common.NewConnectionTarget(cmd.Context(), client, projectID, *service) + target, err := resolveConnectionTarget(cmd.Context(), client, projectID, serviceRef, app.Experimental) if err != nil { return err } diff --git a/internal/cmd/db_query_test.go b/internal/cmd/db_query_test.go index ea8ad05e..636368ec 100644 --- a/internal/cmd/db_query_test.go +++ b/internal/cmd/db_query_test.go @@ -73,7 +73,7 @@ func selectResult() *common.QueryResult { func TestDbQueryCmd(t *testing.T) { setupGetService := func(m *mocks.MockClientWithResponsesInterface) { - expectResolveRef(m, "svc-12345", sampleService()) + expectResolveTarget(m, "svc-12345", sampleService()) } sqlDir := t.TempDir() @@ -165,7 +165,7 @@ func TestDbQueryCmd(t *testing.T) { { name: "read_only prod, PROD service", args: []string{"db", "query", "svc-12345", "-c", "SELECT 1"}, - mock: expectTaggedService("PROD"), + mock: expectTaggedTarget("PROD"), opts: []runOption{ withConfig(map[string]any{"read_only": "prod"}), withExecuteQuery(common.ExecuteQueryArgs{ @@ -179,7 +179,7 @@ func TestDbQueryCmd(t *testing.T) { // prod mode leaves DEV services writable. name: "read_only prod, DEV service", args: []string{"db", "query", "svc-12345", "-c", "SELECT 1"}, - mock: expectTaggedService("DEV"), + mock: expectTaggedTarget("DEV"), opts: []runOption{ withConfig(map[string]any{"read_only": "prod"}), withExecuteQuery(common.ExecuteQueryArgs{ @@ -192,24 +192,24 @@ func TestDbQueryCmd(t *testing.T) { name: "network error", args: []string{"db", "query", "svc-12345", "-c", "SELECT 1"}, mock: func(m *mocks.MockClientWithResponsesInterface) { - expectResolveRefError(m, "svc-12345", errors.New("connection refused")) + expectResolveTargetError(m, "svc-12345", errors.New("connection refused")) }, - wantErr: `failed to resolve service 'svc-12345': connection refused`, + wantErr: `failed to resolve service or read replica set 'svc-12345': connection refused`, }, { name: "service not found", args: []string{"db", "query", "svc-12345", "-c", "SELECT 1"}, mock: func(m *mocks.MockClientWithResponsesInterface) { - expectResolveRefNotFound(m, "svc-12345") + expectResolveTargetNotFound(m, "svc-12345") }, - wantErr: "service 'svc-12345' not found", + wantErr: "service or read replica set 'svc-12345' not found", checks: []checkFunc{checkExitCode(common.ExitServiceNotFound)}, }, { name: "ambiguous name refused", args: []string{"db", "query", "my-api-db", "-c", "SELECT 1"}, mock: func(m *mocks.MockClientWithResponsesInterface) { - expectResolveRefStatus(m, "my-api-db", http.StatusBadRequest, &api.Error{Message: new("ambiguous service name matches multiple services")}) + expectResolveTargetStatus(m, "my-api-db", http.StatusBadRequest, &api.ClientError{Message: new("ambiguous service name matches multiple services")}) }, wantErr: "ambiguous service name matches multiple services\nRun 'tiger service list' to find the ID you want", checks: []checkFunc{checkExitCode(common.ExitInvalidParameters)}, @@ -224,6 +224,27 @@ func TestDbQueryCmd(t *testing.T) { }, nil, common.ErrPaused)}, wantErr: pausedMsg("svc-12345"), }, + { + // A branch is started through `tiger branch`, not `tiger service`. + name: "branch paused", + args: []string{"db", "query", "br-12345", "-c", "SELECT 1"}, + opts: []runOption{withExperimental(), withExecuteQuery(common.ExecuteQueryArgs{ + Query: "SELECT 1", + Role: "tsdbadmin", + }, nil, common.ErrPaused)}, + mock: func(m *mocks.MockClientWithResponsesInterface) { expectResolveTarget(m, "br-12345", sampleBranch()) }, + wantErr: "branch is paused — start it with 'tiger branch start br-12345'", + }, + { + name: "branch not ready", + args: []string{"db", "query", "br-12345", "-c", "SELECT 1"}, + opts: []runOption{withExperimental(), withExecuteQuery(common.ExecuteQueryArgs{ + Query: "SELECT 1", + Role: "tsdbadmin", + }, nil, common.ErrNotReady)}, + mock: func(m *mocks.MockClientWithResponsesInterface) { expectResolveTarget(m, "br-12345", sampleBranch()) }, + wantErr: "branch is not ready — check its status with 'tiger branch get br-12345' and try again", + }, { // A bare pgx auth failure says nothing about which password is // wrong or how to fix it, so the command appends the recovery @@ -508,5 +529,14 @@ result_sets: }, &common.QueryResult{}, nil), }, }, + { + // Branches are a preview, so the help mentions them only behind + // the gate. The generated docs/cli reference covers the ungated + // help. + name: "help mentions branches when experimental", + args: []string{"db", "query", "--help"}, + opts: []runOption{withExperimental()}, + wantStdout: matchHelp("You can also pass a branch, by ID or name, to query that branch."), + }, }) } diff --git a/internal/cmd/db_save_password.go b/internal/cmd/db_save_password.go index 82d34525..252f3648 100644 --- a/internal/cmd/db_save_password.go +++ b/internal/cmd/db_save_password.go @@ -26,7 +26,7 @@ service from your configuration. The password can be provided via: 3. Interactive prompt (if neither provided) The password will be saved according to your --password-storage setting -(keyring, pgpass, or none).`, +(keyring, pgpass, or none).` + branchRefHelp(app.Experimental, "You can also pass a branch, by ID or name, to save the password for that branch."), Example: ` # Save password with explicit value (highest precedence) tiger db save-password svc-12345 --password=your-password @@ -56,15 +56,10 @@ The password will be saved according to your --password-storage setting return err } - resolved, err := resolveService(cmd.Context(), client, projectID, serviceRef) - if err != nil { - return err - } - // Resolve the target so a read replica stores the password against // its parent primary: replicas share the primary's credentials, and // psql/ping look the password up against the primary. - target, err := common.NewConnectionTarget(cmd.Context(), client, projectID, *resolved) + target, err := resolveConnectionTarget(cmd.Context(), client, projectID, serviceRef, app.Experimental) if err != nil { return err } @@ -114,7 +109,11 @@ The password will be saved according to your --password-storage setting if target.Kind == common.TargetReadReplica { cmd.PrintErrf("Read replicas share the primary's credentials; saving against primary %s.\n", label) } - cmd.PrintErrf("Password saved for service %s, role %s\n", label, role) + kind := "service" + if target.Kind == common.TargetBranch { + kind = "branch" + } + cmd.PrintErrf("Password saved for %s %s, role %s\n", kind, label, role) return nil }, } diff --git a/internal/cmd/db_save_password_test.go b/internal/cmd/db_save_password_test.go index 206420c6..661ede93 100644 --- a/internal/cmd/db_save_password_test.go +++ b/internal/cmd/db_save_password_test.go @@ -14,7 +14,7 @@ import ( func TestDbSavePasswordCmd(t *testing.T) { setupGetService := func(m *mocks.MockClientWithResponsesInterface) { - expectResolveRef(m, "svc-12345", sampleService()) + expectResolveTarget(m, "svc-12345", sampleService()) } // checkKeyringPassword asserts the mock keyring holds want for role. @@ -53,24 +53,24 @@ func TestDbSavePasswordCmd(t *testing.T) { name: "network error fetching service", args: []string{"db", "save-password", "svc-12345", "--password=pw"}, mock: func(m *mocks.MockClientWithResponsesInterface) { - expectResolveRefError(m, "svc-12345", errors.New("connection refused")) + expectResolveTargetError(m, "svc-12345", errors.New("connection refused")) }, - wantErr: `failed to resolve service 'svc-12345': connection refused`, + wantErr: `failed to resolve service or read replica set 'svc-12345': connection refused`, }, { name: "service not found", args: []string{"db", "save-password", "svc-12345", "--password=pw"}, mock: func(m *mocks.MockClientWithResponsesInterface) { - expectResolveRefNotFound(m, "svc-12345") + expectResolveTargetNotFound(m, "svc-12345") }, - wantErr: "service 'svc-12345' not found", + wantErr: "service or read replica set 'svc-12345' not found", checks: []checkFunc{checkExitCode(common.ExitServiceNotFound)}, }, { name: "ambiguous name refused", args: []string{"db", "save-password", "my-api-db", "--password=pw"}, mock: func(m *mocks.MockClientWithResponsesInterface) { - expectResolveRefStatus(m, "my-api-db", http.StatusBadRequest, &api.Error{Message: new("ambiguous service name matches multiple services")}) + expectResolveTargetStatus(m, "my-api-db", http.StatusBadRequest, &api.ClientError{Message: new("ambiguous service name matches multiple services")}) }, wantErr: "ambiguous service name matches multiple services\nRun 'tiger service list' to find the ID you want", checks: []checkFunc{checkExitCode(common.ExitInvalidParameters)}, @@ -79,7 +79,7 @@ func TestDbSavePasswordCmd(t *testing.T) { name: "nil response body", args: []string{"db", "save-password", "svc-12345", "--password=pw"}, mock: func(m *mocks.MockClientWithResponsesInterface) { - expectResolveRefStatus(m, "svc-12345", http.StatusOK, nil) + expectResolveTargetStatus(m, "svc-12345", http.StatusOK, nil) }, wantErr: "empty response from API", }, @@ -203,17 +203,37 @@ func TestDbSavePasswordCmd(t *testing.T) { name: "replica ID saves against parent primary", args: []string{"db", "save-password", "rep-67890", "--password=replica-pw"}, mock: func(m *mocks.MockClientWithResponsesInterface) { - expectResolveRef(m, "rep-67890", sampleReplica()) + expectResolveTarget(m, "rep-67890", sampleReplica()) expectGetService(m, "svc-12345", sampleService()) }, wantStderr: "Read replicas share the primary's credentials; saving against primary 'test-service' (svc-12345).\nPassword saved for service 'test-service' (svc-12345), role tsdbadmin\n", checks: []checkFunc{func(t *testing.T, result cmdResult) { // Stored against the parent primary, matching the connect read path. checkKeyringPassword("tsdbadmin", "replica-pw")(t, result) - if pw, err := (&common.KeyringStorage{}).Get(common.ServicePasswordTarget(sampleReplica()), "tsdbadmin"); err == nil { + if pw, err := (&common.KeyringStorage{}).Get(common.PasswordTarget{ID: "rep-67890", ProjectID: testProjectID}, "tsdbadmin"); err == nil { t.Errorf("expected no password stored under the replica ID, got %q", pw) } }}, }, + { + // A branch has its own password, saved under its own ID. + name: "branch saves against itself", + args: []string{"db", "save-password", "test-branch", "--password=branch-pw"}, + opts: []runOption{withExperimental()}, + mock: func(m *mocks.MockClientWithResponsesInterface) { + expectResolveTarget(m, "test-branch", sampleBranch()) + }, + wantStderr: "Password saved for branch 'test-branch' (br-12345), role tsdbadmin\n", + checks: []checkFunc{checkStoredPassword("br-12345", "branch-pw")}, + }, + { + // Branches are a preview, so the help mentions them only behind + // the gate. The generated docs/cli reference covers the ungated + // help. + name: "help mentions branches when experimental", + args: []string{"db", "save-password", "--help"}, + opts: []runOption{withExperimental()}, + wantStdout: matchHelp("You can also pass a branch, by ID or name, to save the password for that branch."), + }, }) } diff --git a/internal/cmd/db_schema.go b/internal/cmd/db_schema.go index 8ff27407..14552725 100644 --- a/internal/cmd/db_schema.go +++ b/internal/cmd/db_schema.go @@ -25,7 +25,7 @@ metadata. The service can be given by ID or name as an argument, or will use the default service 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. +returned. The connection is opened in Tiger Cloud's immutable read-only mode.` + branchRefHelp(app.Experimental, "You can also pass a branch, by ID or name, to introspect that branch.") + ` 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 @@ -58,12 +58,7 @@ large and may embed implementation details.`, return err } - service, err := resolveService(cmd.Context(), client, projectID, serviceRef) - if err != nil { - return err - } - - target, err := common.NewConnectionTarget(cmd.Context(), client, projectID, *service) + target, err := resolveConnectionTarget(cmd.Context(), client, projectID, serviceRef, app.Experimental) if err != nil { return err } diff --git a/internal/cmd/db_schema_test.go b/internal/cmd/db_schema_test.go index 4b12f1a2..7f2589a6 100644 --- a/internal/cmd/db_schema_test.go +++ b/internal/cmd/db_schema_test.go @@ -36,7 +36,7 @@ func withFetchServiceSchema(want common.FetchServiceSchemaArgs, schema *common.D func TestDbSchemaCmd(t *testing.T) { setupGetWithStatus := func(status api.DeployStatus) func(m *mocks.MockClientWithResponsesInterface) { return func(m *mocks.MockClientWithResponsesInterface) { - expectResolveRef(m, "svc-12345", sampleService(func(s *api.Service) { + expectResolveTarget(m, "svc-12345", sampleService(func(s *api.Service) { s.Status = status })) } @@ -82,24 +82,24 @@ func TestDbSchemaCmd(t *testing.T) { name: "network error", args: []string{"db", "schema", "svc-12345"}, mock: func(m *mocks.MockClientWithResponsesInterface) { - expectResolveRefError(m, "svc-12345", errors.New("connection refused")) + expectResolveTargetError(m, "svc-12345", errors.New("connection refused")) }, - wantErr: `failed to resolve service 'svc-12345': connection refused`, + wantErr: `failed to resolve service or read replica set 'svc-12345': connection refused`, }, { name: "service not found", args: []string{"db", "schema", "svc-12345"}, mock: func(m *mocks.MockClientWithResponsesInterface) { - expectResolveRefNotFound(m, "svc-12345") + expectResolveTargetNotFound(m, "svc-12345") }, - wantErr: "service 'svc-12345' not found", + wantErr: "service or read replica set 'svc-12345' not found", checks: []checkFunc{checkExitCode(common.ExitServiceNotFound)}, }, { name: "ambiguous name refused", args: []string{"db", "schema", "my-api-db"}, mock: func(m *mocks.MockClientWithResponsesInterface) { - expectResolveRefStatus(m, "my-api-db", http.StatusBadRequest, &api.Error{Message: new("ambiguous service name matches multiple services")}) + expectResolveTargetStatus(m, "my-api-db", http.StatusBadRequest, &api.ClientError{Message: new("ambiguous service name matches multiple services")}) }, wantErr: "ambiguous service name matches multiple services\nRun 'tiger service list' to find the ID you want", checks: []checkFunc{checkExitCode(common.ExitInvalidParameters)}, @@ -108,7 +108,7 @@ func TestDbSchemaCmd(t *testing.T) { name: "nil response body", args: []string{"db", "schema", "svc-12345"}, mock: func(m *mocks.MockClientWithResponsesInterface) { - expectResolveRefStatus(m, "svc-12345", http.StatusOK, nil) + expectResolveTargetStatus(m, "svc-12345", http.StatusOK, nil) }, wantErr: "empty response from API", }, @@ -142,8 +142,8 @@ func TestDbSchemaCmd(t *testing.T) { name: "replica pooled without pooler warns before readiness check", args: []string{"db", "schema", "rep-67890", "--pooled"}, mock: func(m *mocks.MockClientWithResponsesInterface) { - expectResolveRef(m, "rep-67890", sampleReplica(func(s *api.Service) { - s.Status = api.DeployStatusQUEUED + expectResolveTarget(m, "rep-67890", sampleReplica(func(r *api.ReadReplicaSet) { + r.Status = api.ReadReplicaSetStatusCreating })) expectGetService(m, "svc-12345", sampleService()) }, @@ -179,7 +179,7 @@ func TestDbSchemaCmd(t *testing.T) { name: "prints the schema of a read replica", args: []string{"db", "schema", "rep-67890", "--pooled"}, mock: func(m *mocks.MockClientWithResponsesInterface) { - expectResolveRef(m, "rep-67890", sampleReplica()) + expectResolveTarget(m, "rep-67890", sampleReplica()) expectGetService(m, "svc-12345", sampleService()) }, opts: []runOption{withFetchServiceSchema(common.FetchServiceSchemaArgs{Role: "tsdbadmin", Pooled: true}, schema, nil)}, @@ -194,5 +194,14 @@ func TestDbSchemaCmd(t *testing.T) { opts: []runOption{withFetchServiceSchema(common.FetchServiceSchemaArgs{Role: "tsdbadmin"}, nil, errors.New("failed to connect to database: no route to host"))}, wantErr: "failed to connect to database: no route to host", }, + { + // Branches are a preview, so the help mentions them only behind + // the gate. The generated docs/cli reference covers the ungated + // help. + name: "help mentions branches when experimental", + args: []string{"db", "schema", "--help"}, + opts: []runOption{withExperimental()}, + wantStdout: matchHelp("You can also pass a branch, by ID or name, to introspect that branch."), + }, }) } diff --git a/internal/cmd/db_test.go b/internal/cmd/db_test.go index 9b00dfd3..e1376e4f 100644 --- a/internal/cmd/db_test.go +++ b/internal/cmd/db_test.go @@ -11,26 +11,54 @@ import ( "github.com/timescale/tiger-cli/internal/api/mocks" ) -// sampleReplica returns a standby read replica of sampleService, shaped the way -// GetService returns it when given a read replica set ID. -func sampleReplica(overrides ...func(*api.Service)) api.Service { - svc := sampleService(func(s *api.Service) { - s.ServiceID = "rep-67890" - s.Name = "replica-service" - s.Endpoint = &api.Endpoint{ +// sampleReplica returns an active read replica set of sampleService, as the +// resolve endpoint returns it. +func sampleReplica(overrides ...func(*api.ReadReplicaSet)) api.ReadReplicaSet { + replica := api.ReadReplicaSet{ + ID: "rep-67890", + ServiceID: "svc-12345", + Name: "replica-service", + Status: api.ReadReplicaSetStatusActive, + Nodes: 1, + Endpoint: &api.Endpoint{ Host: new("rep-67890.project.tsdb.cloud.timescale.com"), Port: new(5432), - } - s.ForkedFrom = &api.ForkSpec{ - IsStandby: new(true), - ProjectID: new(testProjectID), - ServiceID: new("svc-12345"), - } - }) + }, + } for _, o := range overrides { - o(&svc) + o(&replica) + } + return replica +} + +// expectResolveTarget registers the unrestricted resolve a `tiger db` command +// makes for what it connects to, answered with v (see expectResolve). +func expectResolveTarget(m *mocks.MockClientWithResponsesInterface, ref string, v any) { + expectResolve(m, ref, nil, v) +} + +// expectTaggedTarget is expectTaggedService for a `tiger db` command: the +// resolution of svc-12345 onto a service carrying the given environment tag. +func expectTaggedTarget(tag string) func(*mocks.MockClientWithResponsesInterface) { + return func(m *mocks.MockClientWithResponsesInterface) { + expectResolveTarget(m, "svc-12345", sampleService(envTag(tag))) } - return svc +} + +// expectResolveTargetNotFound registers a ref that matches nothing. +func expectResolveTargetNotFound(m *mocks.MockClientWithResponsesInterface, ref string) { + expectResolveStatus(m, ref, nil, http.StatusNotFound, &api.ClientError{Message: new("resource not found")}) +} + +// expectResolveTargetError registers a resolution that never reaches the API. +func expectResolveTargetError(m *mocks.MockClientWithResponsesInterface, ref string, err error) { + expectResolveError(m, ref, nil, err) +} + +// expectResolveTargetStatus registers a resolution answered with status. A +// nil apiErr stands in for a response with no body. +func expectResolveTargetStatus(m *mocks.MockClientWithResponsesInterface, ref string, status int, apiErr *api.ClientError) { + expectResolveStatus(m, ref, nil, status, apiErr) } // pausedMsg and notReadyMsg build the readiness errors handleDatabaseError diff --git a/internal/cmd/db_uri.go b/internal/cmd/db_uri.go index d7e22e8d..2f660af3 100644 --- a/internal/cmd/db_uri.go +++ b/internal/cmd/db_uri.go @@ -25,7 +25,7 @@ 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. +You can also pass a read replica set ID to get a connection string for that replica.` + branchRefHelp(app.Experimental, "You can also pass a branch, by ID or name, to get a connection string for that branch.") + ` By default, passwords are excluded from the connection string for security. Use --with-password to include the password directly in the connection string. @@ -67,12 +67,7 @@ services writable.`, return err } - service, err := resolveService(cmd.Context(), client, projectID, serviceRef) - if err != nil { - return err - } - - target, err := common.NewConnectionTarget(cmd.Context(), client, projectID, *service) + target, err := resolveConnectionTarget(cmd.Context(), client, projectID, serviceRef, app.Experimental) if err != nil { return err } diff --git a/internal/cmd/db_uri_test.go b/internal/cmd/db_uri_test.go index a74f7889..e1a9143a 100644 --- a/internal/cmd/db_uri_test.go +++ b/internal/cmd/db_uri_test.go @@ -15,13 +15,14 @@ func TestDbURICmd(t *testing.T) { directURI = "postgresql://tsdbadmin@svc-12345.project.tsdb.cloud.timescale.com:5432/tsdb?sslmode=require\n" readOnlyURI = "postgresql://tsdbadmin@svc-12345.project.tsdb.cloud.timescale.com:5432/tsdb?sslmode=require&options=-c%20tsdb_admin.read_only_connection%3Dtrue\n" replicaURI = "postgresql://tsdbadmin@rep-67890.project.tsdb.cloud.timescale.com:5432/tsdb?sslmode=require\n" + branchURI = "postgresql://tsdbadmin@br-12345.test-project-123.branch.tsdb.cloud.timescale.com:36956/tsdb?sslmode=require\n" ) setupGet := func(m *mocks.MockClientWithResponsesInterface) { - expectResolveRef(m, "svc-12345", sampleService()) + expectResolveTarget(m, "svc-12345", sampleService()) } setupGetReplica := func(m *mocks.MockClientWithResponsesInterface) { - expectResolveRef(m, "rep-67890", sampleReplica()) + expectResolveTarget(m, "rep-67890", sampleReplica()) expectGetService(m, "svc-12345", sampleService()) } withPooler := func(s *api.Service) { @@ -50,33 +51,70 @@ func TestDbURICmd(t *testing.T) { name: "network error", args: []string{"db", "uri", "svc-12345"}, mock: func(m *mocks.MockClientWithResponsesInterface) { - expectResolveRefError(m, "svc-12345", errors.New("connection refused")) + expectResolveTargetError(m, "svc-12345", errors.New("connection refused")) }, - wantErr: `failed to resolve service 'svc-12345': connection refused`, + wantErr: `failed to resolve service or read replica set 'svc-12345': connection refused`, }, { name: "service not found", args: []string{"db", "uri", "svc-12345"}, mock: func(m *mocks.MockClientWithResponsesInterface) { - expectResolveRefNotFound(m, "svc-12345") + expectResolveTargetNotFound(m, "svc-12345") }, - wantErr: "service 'svc-12345' not found", + wantErr: "service or read replica set 'svc-12345' not found", checks: []checkFunc{checkExitCode(common.ExitServiceNotFound)}, }, { name: "ambiguous name refused", args: []string{"db", "uri", "my-api-db"}, mock: func(m *mocks.MockClientWithResponsesInterface) { - expectResolveRefStatus(m, "my-api-db", http.StatusBadRequest, &api.Error{Message: new("ambiguous service name matches multiple services")}) + expectResolveTargetStatus(m, "my-api-db", http.StatusBadRequest, &api.ClientError{Message: new("ambiguous service name matches multiple services")}) }, wantErr: "ambiguous service name matches multiple services\nRun 'tiger service list' to find the ID you want", checks: []checkFunc{checkExitCode(common.ExitInvalidParameters)}, }, + { + // Branches are a preview, so the hint names `tiger branch list` + // only behind the gate. + name: "ambiguous name refused when experimental", + args: []string{"db", "uri", "my-api-db"}, + opts: []runOption{withExperimental()}, + mock: func(m *mocks.MockClientWithResponsesInterface) { + expectResolveTargetStatus(m, "my-api-db", http.StatusBadRequest, &api.ClientError{Message: new("ambiguous ref matches multiple resources")}) + }, + wantErr: "ambiguous ref matches multiple resources\nRun 'tiger service list' or 'tiger branch list' to find the ID you want", + checks: []checkFunc{checkExitCode(common.ExitInvalidParameters)}, + }, + { + // Branches are a preview, so without the gate a ref naming one is + // reported as not found, without mentioning branches. + name: "branch not found without experimental", + args: []string{"db", "uri", "test-branch"}, + mock: func(m *mocks.MockClientWithResponsesInterface) { + expectResolveTarget(m, "test-branch", sampleBranch()) + }, + wantErr: "service or read replica set 'test-branch' not found", + checks: []checkFunc{checkExitCode(common.ExitServiceNotFound)}, + }, + { + name: "branch by name when experimental", + args: []string{"db", "uri", "test-branch"}, + opts: []runOption{withExperimental()}, + mock: func(m *mocks.MockClientWithResponsesInterface) { expectResolveTarget(m, "test-branch", sampleBranch()) }, + wantStdout: branchURI, + }, + { + name: "branch has no pooler", + args: []string{"db", "uri", "br-12345", "--pooled"}, + opts: []runOption{withExperimental()}, + mock: func(m *mocks.MockClientWithResponsesInterface) { expectResolveTarget(m, "br-12345", sampleBranch()) }, + wantErr: "connection pooler not available for this service", + }, { name: "nil response body", args: []string{"db", "uri", "svc-12345"}, mock: func(m *mocks.MockClientWithResponsesInterface) { - expectResolveRefStatus(m, "svc-12345", http.StatusOK, nil) + expectResolveTargetStatus(m, "svc-12345", http.StatusOK, nil) }, wantErr: "empty response from API", }, @@ -109,7 +147,7 @@ func TestDbURICmd(t *testing.T) { name: "pooled with pooler available", args: []string{"db", "uri", "svc-12345", "--pooled"}, mock: func(m *mocks.MockClientWithResponsesInterface) { - expectResolveRef(m, "svc-12345", sampleService(withPooler)) + expectResolveTarget(m, "svc-12345", sampleService(withPooler)) }, wantStdout: "postgresql://tsdbadmin@pooler.svc-12345.project.tsdb.cloud.timescale.com:6432/tsdb?sslmode=require\n", }, @@ -148,14 +186,14 @@ func TestDbURICmd(t *testing.T) { { name: "config prod, PROD service", args: []string{"db", "uri", "svc-12345"}, - mock: expectTaggedService("PROD"), + mock: expectTaggedTarget("PROD"), opts: []runOption{withConfig(map[string]any{"read_only": "prod"})}, wantStdout: readOnlyURI, }, { name: "config prod, DEV service", args: []string{"db", "uri", "svc-12345"}, - mock: expectTaggedService("DEV"), + mock: expectTaggedTarget("DEV"), opts: []runOption{withConfig(map[string]any{"read_only": "prod"})}, wantStdout: directURI, }, @@ -169,7 +207,7 @@ func TestDbURICmd(t *testing.T) { { name: "flag on beats config prod", args: []string{"db", "uri", "svc-12345", "--read-only"}, - mock: expectTaggedService("DEV"), + mock: expectTaggedTarget("DEV"), opts: []runOption{withConfig(map[string]any{"read_only": "prod"})}, wantStdout: readOnlyURI, }, @@ -183,8 +221,8 @@ func TestDbURICmd(t *testing.T) { name: "replica pooled with pooler uses replica pooler", args: []string{"db", "uri", "rep-67890", "--pooled"}, mock: func(m *mocks.MockClientWithResponsesInterface) { - expectResolveRef(m, "rep-67890", sampleReplica(func(s *api.Service) { - s.ConnectionPooler = &api.ConnectionPooler{ + expectResolveTarget(m, "rep-67890", sampleReplica(func(r *api.ReadReplicaSet) { + r.ConnectionPooler = &api.ConnectionPooler{ Endpoint: &api.Endpoint{ Host: new("pooler.rep-67890.project.tsdb.cloud.timescale.com"), Port: new(6432), @@ -206,7 +244,7 @@ func TestDbURICmd(t *testing.T) { name: "replica parent fetch error", args: []string{"db", "uri", "rep-67890"}, mock: func(m *mocks.MockClientWithResponsesInterface) { - expectResolveRef(m, "rep-67890", sampleReplica()) + expectResolveTarget(m, "rep-67890", sampleReplica()) m.EXPECT().GetServiceWithResponse(validCtx, testProjectID, "svc-12345"). Return(nil, errors.New("connection refused")) }, @@ -226,5 +264,22 @@ func TestDbURICmd(t *testing.T) { opts: []runOption{withStoredPassword(common.ServicePasswordTarget(sampleService()), "primary-pw")}, wantStdout: "postgresql://tsdbadmin:primary-pw@rep-67890.project.tsdb.cloud.timescale.com:5432/tsdb?sslmode=require\n", }, + { + // A branch has its own password, saved under its own ID. + name: "branch uses its own credentials", + args: []string{"db", "uri", "br-12345", "--with-password"}, + opts: []runOption{withExperimental(), withStoredPassword(common.BranchPasswordTarget(sampleBranch()), "branch-pw"), withStoredPassword(common.ServicePasswordTarget(sampleService()), "parent-pw")}, + mock: func(m *mocks.MockClientWithResponsesInterface) { expectResolveTarget(m, "br-12345", sampleBranch()) }, + wantStdout: "postgresql://tsdbadmin:branch-pw@br-12345.test-project-123.branch.tsdb.cloud.timescale.com:36956/tsdb?sslmode=require\n", + }, + { + // Branches are a preview, so the help mentions them only behind + // the gate. The generated docs/cli reference covers the ungated + // help. + name: "help mentions branches when experimental", + args: []string{"db", "uri", "--help"}, + opts: []runOption{withExperimental()}, + wantStdout: matchHelp("You can also pass a branch, by ID or name, to get a connection string for that branch."), + }, }) } diff --git a/internal/cmd/ref_helper.go b/internal/cmd/ref_helper.go index ab31aa07..9c93590b 100644 --- a/internal/cmd/ref_helper.go +++ b/internal/cmd/ref_helper.go @@ -71,7 +71,13 @@ func defaultServiceSource(cmd *cobra.Command) string { // resolveService resolves a ref to the service it names, through the ref // filter on the service list: a match is a one-item list, and no match is an -// empty one rather than a 404. +// empty one rather than a 404. A read replica set ID matches too, and comes +// back as a service whose ForkedFrom links to its parent. +// +// TODO: move to the API's resolve endpoint, like the branch and db +// commands, once read replica sets have their own commands. Its SERVICE type +// doesn't match replica sets, and until those commands exist this lookup is +// the only way to reach one. func resolveService(ctx context.Context, client api.ClientWithResponsesInterface, projectID string, ref serviceRef) (*api.Service, error) { resp, err := client.GetServicesWithResponse(ctx, projectID, &api.GetServicesParams{Ref: &ref.ref}) if err != nil { @@ -103,23 +109,23 @@ func resolveService(ctx context.Context, client api.ClientWithResponsesInterface } service := services[0] - if err := checkDefaultIsID(service, ref); err != nil { + if err := checkDefaultIsID(service.ServiceID, ref); err != nil { return nil, err } return &service, nil } // checkDefaultIsID refuses a configured default that turned out to be a name -// (see serviceRef for why). A ref equal to the service's own ID matched by ID -// or replica set ID; anything else matched by name. Checking after the lookup -// rather than before is what lets the error name the ID to store instead. -func checkDefaultIsID(service api.Service, ref serviceRef) error { - if ref.source == "" || service.ServiceID == ref.ref { +// (see serviceRef for why). A ref equal to the resolved ID matched by ID; +// anything else matched by name. Checking after the lookup rather than before +// is what lets the error name the ID to store instead. +func checkDefaultIsID(id string, ref serviceRef) error { + if ref.source == "" || id == ref.ref { return nil } return common.ExitWithCode(common.ExitInvalidParameters, fmt.Errorf( "the default service must be an ID, but %s is set to the name '%s'. Set it to %s, or pass the name as an argument", - ref.source, ref.ref, service.ServiceID)) + ref.source, ref.ref, id)) } // serviceLabel identifies a service in status output by name and ID together. diff --git a/internal/cmd/service.go b/internal/cmd/service.go index 9e19e627..35164d55 100644 --- a/internal/cmd/service.go +++ b/internal/cmd/service.go @@ -79,6 +79,9 @@ func cpuMemoryHelp(shared, experimental bool) string { // OutputService represents a service with computed fields for output type OutputService struct { api.Service + // BranchCount is set only by service get, behind the experimental gate, + // for a service that can have branches. + BranchCount *int `json:"branch_count,omitempty"` common.ConnectionDetails ConnectionString string `json:"connection_string,omitempty"` ConsoleURL string `json:"console_url,omitempty"` @@ -93,6 +96,8 @@ type serviceOutputOptions struct { WithPassword bool // RequirePassword fails the output when WithPassword finds no password. RequirePassword bool + // BranchCount is the service's branch count, if it was counted. + BranchCount *int } // outputService outputs a single service in the configured output format. @@ -178,6 +183,10 @@ func outputServiceTable(output io.Writer, service OutputService) error { } } + if service.BranchCount != nil { + table.Append("Branches", fmt.Sprintf("%d", *service.BranchCount)) + } + // Endpoint information if service.Endpoint != nil && service.Endpoint.Host != nil && service.Endpoint.Port != nil { table.Append("Direct Endpoint", fmt.Sprintf("%s:%d", *service.Endpoint.Host, *service.Endpoint.Port)) @@ -215,7 +224,8 @@ func outputServiceTable(output io.Writer, service OutputService) error { // opts.Experimental is set. func prepareServiceForOutput(cmd *cobra.Command, cfg *config.Config, service api.Service, opts serviceOutputOptions) OutputService { outputSvc := OutputService{ - Service: service, + Service: service, + BranchCount: opts.BranchCount, } outputSvc.Service.InitialPassword = nil if !opts.Experimental { diff --git a/internal/cmd/service_delete.go b/internal/cmd/service_delete.go index 65357961..b1cb0430 100644 --- a/internal/cmd/service_delete.go +++ b/internal/cmd/service_delete.go @@ -16,11 +16,7 @@ import ( func buildServiceDeleteCmd(app *common.App) *cobra.Command { var confirm bool - cmd := &cobra.Command{ - Use: "delete ", - Aliases: []string{"rm"}, - Short: "Delete a database service", - Long: `Delete a database service permanently. + long := `Delete a database service permanently. The service can be given by ID or name, but must be given explicitly: there is no fallback to the default service. @@ -28,8 +24,23 @@ no fallback to the default service. This operation is irreversible. By default, you will be prompted to type the service ID to confirm deletion — the ID, not the name — unless you use the --confirm flag. +` + // Branches are a preview, so only the experimental help mentions them. The + // command lists them regardless: they're deleted either way. + if app.Experimental { + long += ` +Deleting a service also deletes all of its branches. They are listed in the +confirmation prompt and again once the service is deleted. +` + } + long += ` +Note for AI agents: Always confirm with the user before performing this destructive operation.` -Note for AI agents: Always confirm with the user before performing this destructive operation.`, + cmd := &cobra.Command{ + Use: "delete ", + Aliases: []string{"rm"}, + Short: "Delete a database service", + Long: long, Example: ` # Delete a service (with confirmation prompt) tiger service delete svc-12345 @@ -57,10 +68,17 @@ Note for AI agents: Always confirm with the user before performing this destruct return err } + if !confirm && (!util.IsTerminal(cmd.InOrStdin()) || !util.IsTerminal(cmd.ErrOrStderr())) { + return fmt.Errorf("TTY not detected - cannot prompt for confirmation. Use --confirm to skip the prompt") + } + + branches := listBranchesToDelete(cmd, client, projectID, *service) + // Prompt for confirmation unless --confirm is used if !confirm { - if !util.IsTerminal(cmd.InOrStdin()) || !util.IsTerminal(cmd.ErrOrStderr()) { - return fmt.Errorf("TTY not detected - cannot prompt for confirmation. Use --confirm to skip the prompt") + if len(branches) > 0 { + cmd.PrintErrf("This service has %s, which will also be deleted:\n", common.FormatBranchCount(len(branches))) + printBranchLabels(cmd, branches) } // Show both forms, but take only the ID: a name can move to a // different service through a rename, so it is the wrong @@ -92,7 +110,12 @@ Note for AI agents: Always confirm with the user before performing this destruct return common.ExitWithErrorFromStatusCode(resp.StatusCode(), resp.JSON4XX) } - cmd.PrintErrf("Service %s deleted.\n", serviceLabel(*service)) + if len(branches) == 0 { + cmd.PrintErrf("Service %s deleted.\n", serviceLabel(*service)) + return nil + } + cmd.PrintErrf("Service %s deleted, along with its %s:\n", serviceLabel(*service), common.FormatBranchCount(len(branches))) + printBranchLabels(cmd, branches) return nil }, } @@ -101,3 +124,28 @@ Note for AI agents: Always confirm with the user before performing this destruct return cmd } + +// listBranchesToDelete returns the branches that deleting service takes with +// it, so the command can show them before and after the delete. The API +// deletes them without asking, so this isn't gated on the experimental env +// var: whoever has branches gets told. A failed listing is a warning rather +// than an error, so the delete can still go ahead. +func listBranchesToDelete(cmd *cobra.Command, client api.ClientWithResponsesInterface, projectID string, service api.Service) []api.Branch { + if !common.CanHaveBranches(service) { + return nil + } + branches, err := common.ListBranches(cmd.Context(), client, projectID, service.ServiceID) + if err != nil { + cmd.PrintErrf("Warning: failed to list branches: %v\n", err) + cmd.PrintErrln("Any branches of this service will also be deleted.") + return nil + } + return branches +} + +// printBranchLabels prints one indented line per branch. +func printBranchLabels(cmd *cobra.Command, branches []api.Branch) { + for _, branch := range branches { + cmd.PrintErrf(" %s\n", branchLabel(branch)) + } +} diff --git a/internal/cmd/service_delete_test.go b/internal/cmd/service_delete_test.go index 3b6981d2..ceed40e5 100644 --- a/internal/cmd/service_delete_test.go +++ b/internal/cmd/service_delete_test.go @@ -23,6 +23,23 @@ func TestServiceDeleteCmd(t *testing.T) { } } + // setupFluidDelete is setupDelete for a fluid service, which can have + // branches, so the prompt lists them first. + setupFluidDelete := func(branches ...api.Branch) func(m *mocks.MockClientWithResponsesInterface) { + return func(m *mocks.MockClientWithResponsesInterface) { + setupDelete("svc-12345", fluidPlatform)(m) + m.EXPECT().GetBranchesWithResponse(validCtx, testProjectID, "svc-12345", nil). + Return(&api.GetBranchesResponse{ + HTTPResponse: httpResponse(http.StatusOK), + JSON200: &branches, + }, nil) + } + } + otherBranch := sampleBranch(func(b *api.Branch) { + b.ID = "br-67890" + b.Name = "other-branch" + }) + confirmPrompt := "Are you sure you want to delete service 'test-service' (svc-12345)? This operation cannot be undone.\n" + "Type the service ID 'svc-12345' to confirm: " @@ -119,6 +136,66 @@ func TestServiceDeleteCmd(t *testing.T) { mock: setupDelete("test-service"), wantStderr: confirmPrompt + "Service 'test-service' (svc-12345) deleted.\n", }, + { + // The API deletes a service's branches along with it, so they're + // listed before the prompt and after the delete, even without the + // experimental gate. + name: "confirmation lists the branches deleted with the service", + args: []string{"service", "delete", "svc-12345"}, + opts: []runOption{withIsTerminal(true), withStdin("svc-12345\n")}, + mock: setupFluidDelete(sampleBranch(), otherBranch), + wantStderr: "This service has 2 branches, which will also be deleted:\n" + + " 'test-branch' (br-12345)\n" + + " 'other-branch' (br-67890)\n" + + confirmPrompt + + "Service 'test-service' (svc-12345) deleted, along with its 2 branches:\n" + + " 'test-branch' (br-12345)\n" + + " 'other-branch' (br-67890)\n", + }, + { + name: "confirmation lists a single branch", + args: []string{"service", "delete", "svc-12345"}, + opts: []runOption{withIsTerminal(true), withStdin("svc-12345\n")}, + mock: setupFluidDelete(sampleBranch()), + wantStderr: "This service has 1 branch, which will also be deleted:\n" + + " 'test-branch' (br-12345)\n" + + confirmPrompt + + "Service 'test-service' (svc-12345) deleted, along with its 1 branch:\n" + + " 'test-branch' (br-12345)\n", + }, + { + name: "fluid service without branches lists none", + args: []string{"service", "delete", "svc-12345"}, + opts: []runOption{withIsTerminal(true), withStdin("svc-12345\n")}, + mock: setupFluidDelete(), + wantStderr: confirmPrompt + "Service 'test-service' (svc-12345) deleted.\n", + }, + { + // The listing only informs the decision, so a failure still lets + // the user decide. + name: "branch listing fails", + args: []string{"service", "delete", "svc-12345"}, + opts: []runOption{withIsTerminal(true), withStdin("svc-12345\n")}, + mock: func(m *mocks.MockClientWithResponsesInterface) { + setupDelete("svc-12345", fluidPlatform)(m) + m.EXPECT().GetBranchesWithResponse(validCtx, testProjectID, "svc-12345", nil). + Return(nil, errors.New("connection refused")) + }, + wantStderr: "Warning: failed to list branches: failed to list branches: connection refused\n" + + "Any branches of this service will also be deleted.\n" + + confirmPrompt + + "Service 'test-service' (svc-12345) deleted.\n", + }, + { + // Without a prompt, the branches are still listed once the service + // is deleted. + name: "confirm flag still lists the deleted branches", + args: []string{"service", "delete", "svc-12345", "--confirm"}, + mock: setupFluidDelete(sampleBranch(), otherBranch), + wantStderr: "Service 'test-service' (svc-12345) deleted, along with its 2 branches:\n" + + " 'test-branch' (br-12345)\n" + + " 'other-branch' (br-67890)\n", + }, { name: "confirm flag skips prompt", args: []string{"service", "delete", "svc-12345", "--confirm"}, @@ -155,5 +232,18 @@ func TestServiceDeleteCmd(t *testing.T) { mock: setupDelete("svc-12345"), wantStderr: "Service 'test-service' (svc-12345) deleted.\n", }, + { + // Branches are a preview, so the help mentions them only behind + // the gate. + name: "help mentions branches when experimental", + args: []string{"service", "delete", "--help"}, + opts: []runOption{withExperimental()}, + wantStdout: matchHelp("Deleting a service also deletes all of its branches."), + }, + { + name: "help omits branches by default", + args: []string{"service", "delete", "--help"}, + wantStdout: matchHelp("Delete a database service permanently.", "branch"), + }, }) } diff --git a/internal/cmd/service_get.go b/internal/cmd/service_get.go index dd705ad3..126b35d2 100644 --- a/internal/cmd/service_get.go +++ b/internal/cmd/service_get.go @@ -53,11 +53,23 @@ resource usage.`, } service := *resolved + // Branches are a preview, so they're counted only behind the gate. + // The rest of the output doesn't depend on the count. + var branchCount *int + if app.Experimental && common.CanHaveBranches(service) { + if count, err := common.CountBranches(cmd.Context(), client, projectID, service.ServiceID); err != nil { + cmd.PrintErrf("Warning: failed to count branches: %v\n", err) + } else { + branchCount = &count + } + } + // Output service in requested format return outputService(cmd, cfg, service, serviceOutputOptions{ Experimental: app.Experimental, WithPassword: withPassword, RequirePassword: true, + BranchCount: branchCount, }) }, } diff --git a/internal/cmd/service_get_test.go b/internal/cmd/service_get_test.go index a6beb46d..815e780c 100644 --- a/internal/cmd/service_get_test.go +++ b/internal/cmd/service_get_test.go @@ -45,6 +45,18 @@ func TestServiceGetCmd(t *testing.T) { } } + // setupGetFluid also lists the fluid service's two branches, which only + // service get does, behind the experimental gate. + setupGetFluid := func(m *mocks.MockClientWithResponsesInterface) { + expectResolveRef(m, "svc-12345", fluidService) + branches := []api.Branch{sampleBranch(), sampleBranch()} + m.EXPECT().GetBranchesWithResponse(validCtx, testProjectID, "svc-12345", nil). + Return(&api.GetBranchesResponse{ + HTTPResponse: httpResponse(http.StatusOK), + JSON200: &branches, + }, nil) + } + runCmdTests(t, []cmdTest{ { name: "not logged in", @@ -300,12 +312,12 @@ status: READY `, }, { - // The platform and IOPS are preview fields: the output cases above - // carry them from sampleService and drop them, while these show them - // with the gate. - name: "experimental table output shows platform and IOPS", + // The platform, IOPS, and branch count are preview fields: the + // output cases above carry a platform and IOPS from sampleService + // and drop them, while these show all three with the gate. + name: "experimental table output shows platform, IOPS, and branches", args: []string{"service", "get", "svc-12345"}, - mock: setupGet(fluidService), + mock: setupGetFluid, opts: []runOption{experimental}, wantStdout: `┌───────────────────┬─────────────────────────────────────────────────────────────────────────────────────────────┐ │ PROPERTY │ VALUE │ @@ -319,6 +331,7 @@ status: READY │ CPU │ 1 cores (1000m) │ │ Memory │ 4 GB │ │ IOPS │ 6000 │ +│ Branches │ 2 │ │ Direct Endpoint │ svc-12345.project.tsdb.cloud.timescale.com:5432 │ │ Created │ 2025-01-15 10:30:00 UTC │ │ Connection String │ postgresql://tsdbadmin@svc-12345.project.tsdb.cloud.timescale.com:5432/tsdb?sslmode=require │ @@ -327,9 +340,9 @@ status: READY `, }, { - name: "experimental json output includes platform and IOPS", + name: "experimental json output includes platform, IOPS, and branches", args: []string{"service", "get", "svc-12345", "-o", "json"}, - mock: setupGet(fluidService), + mock: setupGetFluid, opts: []runOption{experimental}, wantStdout: `{ "created": "2025-01-15T10:30:00Z", @@ -354,6 +367,7 @@ status: READY "service_id": "svc-12345", "service_type": "TIMESCALEDB", "status": "READY", + "branch_count": 2, "role": "tsdbadmin", "host": "svc-12345.project.tsdb.cloud.timescale.com", "port": 5432, @@ -364,11 +378,12 @@ status: READY `, }, { - name: "experimental yaml output includes platform and IOPS", + name: "experimental yaml output includes platform, IOPS, and branches", args: []string{"service", "get", "svc-12345", "-o", "yaml"}, - mock: setupGet(fluidService), + mock: setupGetFluid, opts: []runOption{experimental}, - wantStdout: `connection_string: postgresql://tsdbadmin@svc-12345.project.tsdb.cloud.timescale.com:5432/tsdb?sslmode=require + wantStdout: `branch_count: 2 +connection_string: postgresql://tsdbadmin@svc-12345.project.tsdb.cloud.timescale.com:5432/tsdb?sslmode=require console_url: https://console.cloud.tigerdata.com/dashboard/services/svc-12345 created: "2025-01-15T10:30:00Z" database: tsdb @@ -393,6 +408,35 @@ service_type: TIMESCALEDB status: READY `, }, + { + // Only fluid services can be branched, so a standard one isn't + // counted. The mock fails the test on any branch listing. + name: "experimental standard service lists no branches", + args: []string{"service", "get", "svc-12345", "-o", "env"}, + mock: setupGet(sampleService()), + opts: []runOption{experimental}, + wantStdout: "PGHOST=svc-12345.project.tsdb.cloud.timescale.com\nPGPORT=5432\nPGDATABASE=tsdb\nPGUSER=tsdbadmin\n", + }, + { + name: "fluid service lists no branches without the gate", + args: []string{"service", "get", "svc-12345", "-o", "env"}, + mock: setupGet(fluidService), + wantStdout: "PGHOST=svc-12345.project.tsdb.cloud.timescale.com\nPGPORT=5432\nPGDATABASE=tsdb\nPGUSER=tsdbadmin\n", + }, + { + // The count is the only thing the branch listing adds, so the + // service is still shown without it. + name: "branch count fails", + args: []string{"service", "get", "svc-12345", "-o", "env"}, + mock: func(m *mocks.MockClientWithResponsesInterface) { + expectResolveRef(m, "svc-12345", fluidService) + m.EXPECT().GetBranchesWithResponse(validCtx, testProjectID, "svc-12345", nil). + Return(nil, errors.New("connection refused")) + }, + opts: []runOption{experimental}, + wantStdout: "PGHOST=svc-12345.project.tsdb.cloud.timescale.com\nPGPORT=5432\nPGDATABASE=tsdb\nPGUSER=tsdbadmin\n", + wantStderr: "Warning: failed to count branches: failed to list branches: connection refused\n", + }, { name: "env output", args: []string{"service", "get", "svc-12345", "-o", "env"}, diff --git a/internal/common/branch.go b/internal/common/branch.go index d02590e4..e53fbfd2 100644 --- a/internal/common/branch.go +++ b/internal/common/branch.go @@ -7,6 +7,7 @@ import ( "net/http" "github.com/timescale/tiger-cli/internal/api" + "github.com/timescale/tiger-cli/internal/util" ) // BranchEnvironmentTag is the tag every branch is judged by. Branches have no @@ -26,6 +27,13 @@ func ResolveBranch(ctx context.Context, client api.ClientWithResponsesInterface, return resolved.Branch, nil } +// CanHaveBranches reports whether service can have branches, so callers can +// skip listing them for services that never do. Only fluid services can be +// branched. +func CanHaveBranches(service api.Service) bool { + return util.Deref(service.Platform) == api.ServicePlatformFLUID +} + // ListBranches returns serviceID's branches. func ListBranches(ctx context.Context, client api.ClientWithResponsesInterface, projectID, serviceID string) ([]api.Branch, error) { resp, err := client.GetBranchesWithResponse(ctx, projectID, serviceID, nil) @@ -40,3 +48,21 @@ func ListBranches(ctx context.Context, client api.ClientWithResponsesInterface, } return *resp.JSON200, nil } + +// CountBranches returns how many branches serviceID has. +func CountBranches(ctx context.Context, client api.ClientWithResponsesInterface, projectID, serviceID string) (int, error) { + branches, err := ListBranches(ctx, client, projectID, serviceID) + if err != nil { + return 0, err + } + return len(branches), nil +} + +// FormatBranchCount phrases a number of branches, as "1 branch" or +// "2 branches". +func FormatBranchCount(n int) string { + if n == 1 { + return "1 branch" + } + return fmt.Sprintf("%d branches", n) +} diff --git a/internal/common/connection_target.go b/internal/common/connection_target.go index 0a8a37c3..52ed313c 100644 --- a/internal/common/connection_target.go +++ b/internal/common/connection_target.go @@ -159,6 +159,44 @@ func (t *ConnectionTarget) Details(cfg *config.Config, opts ConnectionDetailsOpt return details, nil } +// ResolveConnectionTarget resolves ref, an ID or a name, to the service, +// branch, or read replica set it names and works out whose credentials it +// uses. Branches are a preview, so unless allowBranches is set a ref naming +// one is reported as not found. +func ResolveConnectionTarget(ctx context.Context, client api.ClientWithResponsesInterface, projectID, ref string, allowBranches bool) (*ConnectionTarget, error) { + kind := connectionTargetKind(allowBranches) + resolved, err := resolve(ctx, client, projectID, ref, nil, kind) + if err != nil { + return nil, err + } + + switch { + case resolved.Branch != nil: + if !allowBranches { + return nil, ExitWithCode(ExitServiceNotFound, fmt.Errorf("%s '%s' not found", kind, ref)) + } + return NewBranchConnectionTarget(*resolved.Branch), nil + case resolved.ReadReplicaSet != nil: + replica := *resolved.ReadReplicaSet + parent, err := GetService(ctx, client, projectID, replica.ServiceID) + if err != nil { + return nil, fmt.Errorf("failed to fetch parent service %q for read replica: %w", replica.ServiceID, err) + } + return NewReplicaConnectionTarget(replica, ServiceCredentials(*parent)), nil + default: + return NewServiceConnectionTarget(*resolved.Service), nil + } +} + +// connectionTargetKind names what ResolveConnectionTarget looks for, in its +// errors. Branches are left out unless they are allowed. +func connectionTargetKind(allowBranches bool) string { + if allowBranches { + return "service, branch, or read replica set" + } + return "service or read replica set" +} + // ResolveConnectionTargetByID fetches the service named by id — which may be a // primary service ID or a read replica set ID, both of which GetService // resolves — and works out whose credentials it uses. Callers that already