fix(cli): tighten deploy review follow-ups
- commit
- f231d9cb2ef1350f6f4e21ab1141bf92c4501241
- author
- brice · · 2026-04-06 14:01 UTC
- parent
- 913d8fc
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
7 files · +124 -31
M internal/adapters/in/cli/deploy.go +38 -10
| @@ -3,11 +3,11 @@ package cli | |||
| 3 | 3 | import ( | |
| 4 | 4 | "context" | |
| 5 | 5 | "fmt" | |
| 6 | + | "io" | |
| 6 | 7 | ||
| 7 | 8 | "github.com/spf13/cobra" | |
| 8 | 9 | ||
| 9 | 10 | "github.com/bnema/gordon/internal/adapters/in/cli/remote" | |
| 10 | - | "github.com/bnema/gordon/internal/adapters/in/cli/ui/styles" | |
| 11 | 11 | "github.com/bnema/gordon/internal/app" | |
| 12 | 12 | ) | |
| 13 | 13 | ||
| @@ -17,7 +17,8 @@ type deployer interface { | |||
| 17 | 17 | ||
| 18 | 18 | // newDeployCmd creates the deploy command. | |
| 19 | 19 | func newDeployCmd() *cobra.Command { | |
| 20 | - | return &cobra.Command{ | |
| 20 | + | var jsonOut bool | |
| 21 | + | cmd := &cobra.Command{ | |
| 21 | 22 | Use: "deploy <domain>", | |
| 22 | 23 | Short: "Manually deploy or redeploy a route", | |
| 23 | 24 | Long: `Triggers a deployment for the specified route domain. | |
| @@ -38,20 +39,23 @@ Examples: | |||
| 38 | 39 | } | |
| 39 | 40 | defer handle.close() | |
| 40 | 41 | ||
| 41 | - | return runDeploy(cmd.Context(), handle.plane, handle.isRemote, args[0]) | |
| 42 | + | return runDeploy(cmd.Context(), handle.plane, handle.isRemote, args[0], cmd.OutOrStdout(), jsonOut) | |
| 42 | 43 | }, | |
| 43 | 44 | } | |
| 45 | + | cmd.Flags().BoolVar(&jsonOut, "json", false, "Output as JSON") | |
| 46 | + | return cmd | |
| 44 | 47 | } | |
| 45 | 48 | ||
| 46 | - | func runDeploy(ctx context.Context, deployer deployer, isRemote bool, deployDomain string) error { | |
| 49 | + | func runDeploy(ctx context.Context, deployer deployer, isRemote bool, deployDomain string, out io.Writer, jsonOut bool) error { | |
| 47 | 50 | result, err := deployer.Deploy(ctx, deployDomain) | |
| 48 | 51 | if err != nil { | |
| 49 | 52 | if isRemote && shouldFallbackToLocal(err) { | |
| 50 | 53 | domain, localErr := app.SendDeploySignal(deployDomain) | |
| 51 | 54 | if localErr == nil { | |
| 52 | - | fmt.Println(styles.RenderWarning(fmt.Sprintf("Remote deploy failed (%v), used local signal fallback", err))) | |
| 53 | - | fmt.Println(styles.RenderSuccess(fmt.Sprintf("Deploy signal sent for domain: %s", domain))) | |
| 54 | - | return nil | |
| 55 | + | if writeErr := cliWriteLine(out, cliRenderWarning(fmt.Sprintf("Remote deploy failed (%v), used local signal fallback", err))); writeErr != nil { | |
| 56 | + | return writeErr | |
| 57 | + | } | |
| 58 | + | return cliWriteLine(out, cliRenderSuccess(fmt.Sprintf("Deploy signal sent for domain: %s", domain))) | |
| 55 | 59 | } | |
| 56 | 60 | return fmt.Errorf("failed to deploy: remote error: %w; local fallback also failed: %v", err, localErr) | |
| 57 | 61 | } | |
| @@ -59,10 +63,34 @@ func runDeploy(ctx context.Context, deployer deployer, isRemote bool, deployDoma | |||
| 59 | 63 | return formatDeployFailure(err) | |
| 60 | 64 | } | |
| 61 | 65 | ||
| 66 | + | if jsonOut { | |
| 67 | + | return writeJSON(out, result) | |
| 68 | + | } | |
| 69 | + | ||
| 70 | + | messageDomain := deployDomain | |
| 71 | + | if result.Domain != "" { | |
| 72 | + | messageDomain = result.Domain | |
| 73 | + | } | |
| 74 | + | ||
| 62 | 75 | containerID := result.ContainerID | |
| 63 | - | if len(containerID) > 12 { | |
| 76 | + | if containerID != "" && len(containerID) > 12 { | |
| 64 | 77 | containerID = containerID[:12] | |
| 65 | 78 | } | |
| 66 | - | fmt.Println(styles.RenderSuccess(fmt.Sprintf("Deployed %s (container: %s)", deployDomain, containerID))) | |
| 67 | - | return nil | |
| 79 | + | ||
| 80 | + | switch result.Status { | |
| 81 | + | case "deployed": | |
| 82 | + | msg := fmt.Sprintf("Deployed %s", messageDomain) | |
| 83 | + | if containerID != "" { | |
| 84 | + | msg += fmt.Sprintf(" (container: %s)", containerID) | |
| 85 | + | } | |
| 86 | + | return cliWriteLine(out, cliRenderSuccess(msg)) | |
| 87 | + | case "queued": | |
| 88 | + | return cliWriteLine(out, cliRenderSuccess(fmt.Sprintf("Deploy queued for domain: %s", messageDomain))) | |
| 89 | + | default: | |
| 90 | + | msg := fmt.Sprintf("Deploy status for %s: %s", messageDomain, result.Status) | |
| 91 | + | if containerID != "" { | |
| 92 | + | msg += fmt.Sprintf(" (container: %s)", containerID) | |
| 93 | + | } | |
| 94 | + | return cliWriteLine(out, cliRenderInfo(msg)) | |
| 95 | + | } | |
| 68 | 96 | } | |
| @@ -3,11 +3,11 @@ package cli | |||||
| 3 | import ( | 3 | import ( | ||
| 4 | "context" | 4 | "context" | ||
| 5 | "fmt" | 5 | "fmt" | ||
| 6 | + | "io" | |||
| 6 | 7 | ||||
| 7 | "github.com/spf13/cobra" | 8 | "github.com/spf13/cobra" | ||
| 8 | 9 | ||||
| 9 | "github.com/bnema/gordon/internal/adapters/in/cli/remote" | 10 | "github.com/bnema/gordon/internal/adapters/in/cli/remote" | ||
| 10 | - | "github.com/bnema/gordon/internal/adapters/in/cli/ui/styles" | |||
| 11 | "github.com/bnema/gordon/internal/app" | 11 | "github.com/bnema/gordon/internal/app" | ||
| 12 | ) | 12 | ) | ||
| 13 | 13 | ||||
| @@ -17,7 +17,8 @@ type deployer interface { | |||||
| 17 | 17 | ||||
| 18 | // newDeployCmd creates the deploy command. | 18 | // newDeployCmd creates the deploy command. | ||
| 19 | func newDeployCmd() *cobra.Command { | 19 | func newDeployCmd() *cobra.Command { | ||
| 20 | - | return &cobra.Command{ | 20 | + | var jsonOut bool |
| 21 | + | cmd := &cobra.Command{ | |||
| 21 | Use: "deploy <domain>", | 22 | Use: "deploy <domain>", | ||
| 22 | Short: "Manually deploy or redeploy a route", | 23 | Short: "Manually deploy or redeploy a route", | ||
| 23 | Long: `Triggers a deployment for the specified route domain. | 24 | Long: `Triggers a deployment for the specified route domain. | ||
| @@ -38,20 +39,23 @@ Examples: | |||||
| 38 | } | 39 | } | ||
| 39 | defer handle.close() | 40 | defer handle.close() | ||
| 40 | 41 | ||||
| 41 | - | return runDeploy(cmd.Context(), handle.plane, handle.isRemote, args[0]) | 42 | + | return runDeploy(cmd.Context(), handle.plane, handle.isRemote, args[0], cmd.OutOrStdout(), jsonOut) |
| 42 | }, | 43 | }, | ||
| 43 | } | 44 | } | ||
| 45 | + | cmd.Flags().BoolVar(&jsonOut, "json", false, "Output as JSON") | |||
| 46 | + | return cmd | |||
| 44 | } | 47 | } | ||
| 45 | 48 | ||||
| 46 | - | func runDeploy(ctx context.Context, deployer deployer, isRemote bool, deployDomain string) error { | 49 | + | func runDeploy(ctx context.Context, deployer deployer, isRemote bool, deployDomain string, out io.Writer, jsonOut bool) error { |
| 47 | result, err := deployer.Deploy(ctx, deployDomain) | 50 | result, err := deployer.Deploy(ctx, deployDomain) | ||
| 48 | if err != nil { | 51 | if err != nil { | ||
| 49 | if isRemote && shouldFallbackToLocal(err) { | 52 | if isRemote && shouldFallbackToLocal(err) { | ||
| 50 | domain, localErr := app.SendDeploySignal(deployDomain) | 53 | domain, localErr := app.SendDeploySignal(deployDomain) | ||
| 51 | if localErr == nil { | 54 | if localErr == nil { | ||
| 52 | - | fmt.Println(styles.RenderWarning(fmt.Sprintf("Remote deploy failed (%v), used local signal fallback", err))) | 55 | + | if writeErr := cliWriteLine(out, cliRenderWarning(fmt.Sprintf("Remote deploy failed (%v), used local signal fallback", err))); writeErr != nil { |
| 53 | - | fmt.Println(styles.RenderSuccess(fmt.Sprintf("Deploy signal sent for domain: %s", domain))) | 56 | + | return writeErr |
| 54 | - | return nil | 57 | + | } |
| 58 | + | return cliWriteLine(out, cliRenderSuccess(fmt.Sprintf("Deploy signal sent for domain: %s", domain))) | |||
| 55 | } | 59 | } | ||
| 56 | return fmt.Errorf("failed to deploy: remote error: %w; local fallback also failed: %v", err, localErr) | 60 | return fmt.Errorf("failed to deploy: remote error: %w; local fallback also failed: %v", err, localErr) | ||
| 57 | } | 61 | } | ||
| @@ -59,10 +63,34 @@ func runDeploy(ctx context.Context, deployer deployer, isRemote bool, deployDoma | |||||
| 59 | return formatDeployFailure(err) | 63 | return formatDeployFailure(err) | ||
| 60 | } | 64 | } | ||
| 61 | 65 | ||||
| 66 | + | if jsonOut { | |||
| 67 | + | return writeJSON(out, result) | |||
| 68 | + | } | |||
| 69 | + | ||||
| 70 | + | messageDomain := deployDomain | |||
| 71 | + | if result.Domain != "" { | |||
| 72 | + | messageDomain = result.Domain | |||
| 73 | + | } | |||
| 74 | + | ||||
| 62 | containerID := result.ContainerID | 75 | containerID := result.ContainerID | ||
| 63 | - | if len(containerID) > 12 { | 76 | + | if containerID != "" && len(containerID) > 12 { |
| 64 | containerID = containerID[:12] | 77 | containerID = containerID[:12] | ||
| 65 | } | 78 | } | ||
| 66 | - | fmt.Println(styles.RenderSuccess(fmt.Sprintf("Deployed %s (container: %s)", deployDomain, containerID))) | 79 | + | |
| 67 | - | return nil | 80 | + | switch result.Status { |
| 81 | + | case "deployed": | |||
| 82 | + | msg := fmt.Sprintf("Deployed %s", messageDomain) | |||
| 83 | + | if containerID != "" { | |||
| 84 | + | msg += fmt.Sprintf(" (container: %s)", containerID) | |||
| 85 | + | } | |||
| 86 | + | return cliWriteLine(out, cliRenderSuccess(msg)) | |||
| 87 | + | case "queued": | |||
| 88 | + | return cliWriteLine(out, cliRenderSuccess(fmt.Sprintf("Deploy queued for domain: %s", messageDomain))) | |||
| 89 | + | default: | |||
| 90 | + | msg := fmt.Sprintf("Deploy status for %s: %s", messageDomain, result.Status) | |||
| 91 | + | if containerID != "" { | |||
| 92 | + | msg += fmt.Sprintf(" (container: %s)", containerID) | |||
| 93 | + | } | |||
| 94 | + | return cliWriteLine(out, cliRenderInfo(msg)) | |||
| 95 | + | } | |||
| 68 | } | 96 | } | ||
M internal/adapters/in/cli/deploy_error.go +28 -3
| @@ -3,12 +3,16 @@ package cli | |||
| 3 | 3 | import ( | |
| 4 | 4 | "errors" | |
| 5 | 5 | "fmt" | |
| 6 | + | "regexp" | |
| 6 | 7 | "strings" | |
| 8 | + | "unicode" | |
| 7 | 9 | ||
| 8 | 10 | "github.com/bnema/gordon/internal/adapters/in/cli/remote" | |
| 9 | 11 | "github.com/bnema/gordon/internal/domain" | |
| 10 | 12 | ) | |
| 11 | 13 | ||
| 14 | + | var ansiEscapePattern = regexp.MustCompile(`\x1b\[[0-9;?]*[ -/]*[@-~]`) | |
| 15 | + | ||
| 12 | 16 | func formatDeployFailure(err error) error { | |
| 13 | 17 | var deployErr *domain.DeployFailureError | |
| 14 | 18 | if errors.As(err, &deployErr) { | |
| @@ -44,13 +48,34 @@ func renderDeployFailure(summary, cause, hint string, logs []string) string { | |||
| 44 | 48 | b.WriteString("\nHint: ") | |
| 45 | 49 | b.WriteString(hint) | |
| 46 | 50 | } | |
| 47 | - | if len(logs) > 0 { | |
| 51 | + | cleanedLogs := make([]string, 0, len(logs)) | |
| 52 | + | for _, line := range logs { | |
| 53 | + | if cleanedLine := sanitizeDeployLogLine(line); cleanedLine != "" { | |
| 54 | + | cleanedLogs = append(cleanedLogs, cleanedLine) | |
| 55 | + | } | |
| 56 | + | } | |
| 57 | + | if len(cleanedLogs) > 0 { | |
| 48 | 58 | b.WriteString("\n\nRecent container logs:") | |
| 49 | - | for _, line := range logs { | |
| 59 | + | for _, cleanedLine := range cleanedLogs { | |
| 50 | 60 | b.WriteString("\n ") | |
| 51 | - | b.WriteString(line) | |
| 61 | + | b.WriteString(cleanedLine) | |
| 52 | 62 | } | |
| 53 | 63 | } | |
| 54 | 64 | ||
| 55 | 65 | return b.String() | |
| 56 | 66 | } | |
| 67 | + | ||
| 68 | + | func sanitizeDeployLogLine(line string) string { | |
| 69 | + | line = ansiEscapePattern.ReplaceAllString(line, "") | |
| 70 | + | line = strings.Map(func(r rune) rune { | |
| 71 | + | switch { | |
| 72 | + | case r == '\t': | |
| 73 | + | return r | |
| 74 | + | case !unicode.IsPrint(r): | |
| 75 | + | return -1 | |
| 76 | + | default: | |
| 77 | + | return r | |
| 78 | + | } | |
| 79 | + | }, line) | |
| 80 | + | return strings.TrimSpace(line) | |
| 81 | + | } | |
| @@ -3,12 +3,16 @@ package cli | |||||
| 3 | import ( | 3 | import ( | ||
| 4 | "errors" | 4 | "errors" | ||
| 5 | "fmt" | 5 | "fmt" | ||
| 6 | + | "regexp" | |||
| 6 | "strings" | 7 | "strings" | ||
| 8 | + | "unicode" | |||
| 7 | 9 | ||||
| 8 | "github.com/bnema/gordon/internal/adapters/in/cli/remote" | 10 | "github.com/bnema/gordon/internal/adapters/in/cli/remote" | ||
| 9 | "github.com/bnema/gordon/internal/domain" | 11 | "github.com/bnema/gordon/internal/domain" | ||
| 10 | ) | 12 | ) | ||
| 11 | 13 | ||||
| 14 | + | var ansiEscapePattern = regexp.MustCompile(`\x1b\[[0-9;?]*[ -/]*[@-~]`) | |||
| 15 | + | ||||
| 12 | func formatDeployFailure(err error) error { | 16 | func formatDeployFailure(err error) error { | ||
| 13 | var deployErr *domain.DeployFailureError | 17 | var deployErr *domain.DeployFailureError | ||
| 14 | if errors.As(err, &deployErr) { | 18 | if errors.As(err, &deployErr) { | ||
| @@ -44,13 +48,34 @@ func renderDeployFailure(summary, cause, hint string, logs []string) string { | |||||
| 44 | b.WriteString("\nHint: ") | 48 | b.WriteString("\nHint: ") | ||
| 45 | b.WriteString(hint) | 49 | b.WriteString(hint) | ||
| 46 | } | 50 | } | ||
| 47 | - | if len(logs) > 0 { | 51 | + | cleanedLogs := make([]string, 0, len(logs)) |
| 52 | + | for _, line := range logs { | |||
| 53 | + | if cleanedLine := sanitizeDeployLogLine(line); cleanedLine != "" { | |||
| 54 | + | cleanedLogs = append(cleanedLogs, cleanedLine) | |||
| 55 | + | } | |||
| 56 | + | } | |||
| 57 | + | if len(cleanedLogs) > 0 { | |||
| 48 | b.WriteString("\n\nRecent container logs:") | 58 | b.WriteString("\n\nRecent container logs:") | ||
| 49 | - | for _, line := range logs { | 59 | + | for _, cleanedLine := range cleanedLogs { |
| 50 | b.WriteString("\n ") | 60 | b.WriteString("\n ") | ||
| 51 | - | b.WriteString(line) | 61 | + | b.WriteString(cleanedLine) |
| 52 | } | 62 | } | ||
| 53 | } | 63 | } | ||
| 54 | 64 | ||||
| 55 | return b.String() | 65 | return b.String() | ||
| 56 | } | 66 | } | ||
| 67 | + | ||||
| 68 | + | func sanitizeDeployLogLine(line string) string { | |||
| 69 | + | line = ansiEscapePattern.ReplaceAllString(line, "") | |||
| 70 | + | line = strings.Map(func(r rune) rune { | |||
| 71 | + | switch { | |||
| 72 | + | case r == '\t': | |||
| 73 | + | return r | |||
| 74 | + | case !unicode.IsPrint(r): | |||
| 75 | + | return -1 | |||
| 76 | + | default: | |||
| 77 | + | return r | |||
| 78 | + | } | |||
| 79 | + | }, line) | |||
| 80 | + | return strings.TrimSpace(line) | |||
| 81 | + | } | |||
M internal/adapters/in/cli/deploy_error_test.go +11 -0
| @@ -51,3 +51,14 @@ func TestFormatDeployFailure_GenericFallback(t *testing.T) { | |||
| 51 | 51 | assert.Equal(t, "failed to deploy: boom", err.Error()) | |
| 52 | 52 | assert.ErrorIs(t, err, root) | |
| 53 | 53 | } | |
| 54 | + | ||
| 55 | + | func TestFormatDeployFailure_SkipsEmptySanitizedLogsSection(t *testing.T) { | |
| 56 | + | err := formatDeployFailure(&domain.DeployFailureError{ | |
| 57 | + | Summary: "failed to deploy", | |
| 58 | + | Cause: "health check failed", | |
| 59 | + | Logs: []string{"\x1b[31m\x1b[0m", "\n\r\t"}, | |
| 60 | + | }) | |
| 61 | + | ||
| 62 | + | require.Error(t, err) | |
| 63 | + | assert.Equal(t, "failed to deploy\nCause: health check failed", err.Error()) | |
| 64 | + | } | |
| @@ -51,3 +51,14 @@ func TestFormatDeployFailure_GenericFallback(t *testing.T) { | |||||
| 51 | assert.Equal(t, "failed to deploy: boom", err.Error()) | 51 | assert.Equal(t, "failed to deploy: boom", err.Error()) | ||
| 52 | assert.ErrorIs(t, err, root) | 52 | assert.ErrorIs(t, err, root) | ||
| 53 | } | 53 | } | ||
| 54 | + | ||||
| 55 | + | func TestFormatDeployFailure_SkipsEmptySanitizedLogsSection(t *testing.T) { | |||
| 56 | + | err := formatDeployFailure(&domain.DeployFailureError{ | |||
| 57 | + | Summary: "failed to deploy", | |||
| 58 | + | Cause: "health check failed", | |||
| 59 | + | Logs: []string{"\x1b[31m\x1b[0m", "\n\r\t"}, | |||
| 60 | + | }) | |||
| 61 | + | ||||
| 62 | + | require.Error(t, err) | |||
| 63 | + | assert.Equal(t, "failed to deploy\nCause: health check failed", err.Error()) | |||
| 64 | + | } | |||
M internal/adapters/in/cli/deploy_test.go +15 -1
| @@ -1,6 +1,7 @@ | |||
| 1 | 1 | package cli | |
| 2 | 2 | ||
| 3 | 3 | import ( | |
| 4 | + | "bytes" | |
| 4 | 5 | "context" | |
| 5 | 6 | "testing" | |
| 6 | 7 | ||
| @@ -26,8 +27,21 @@ func TestRunDeploy_LocalTypedErrorUsesDeployFormatter(t *testing.T) { | |||
| 26 | 27 | Cause: "health check failed", | |
| 27 | 28 | Hint: "check DATABASE_URL", | |
| 28 | 29 | Logs: []string{"booting app", "connection refused"}, | |
| 29 | - | }}, false, "app.example.com") | |
| 30 | + | }}, false, "app.example.com", &bytes.Buffer{}, false) | |
| 30 | 31 | ||
| 31 | 32 | require.Error(t, err) | |
| 32 | 33 | assert.Equal(t, "failed to deploy\nCause: health check failed\nHint: check DATABASE_URL\n\nRecent container logs:\n booting app\n connection refused", err.Error()) | |
| 33 | 34 | } | |
| 35 | + | ||
| 36 | + | func TestRunDeploy_UsesResultDomainWhenPresent(t *testing.T) { | |
| 37 | + | var out bytes.Buffer | |
| 38 | + | err := runDeploy(context.Background(), &deployTestDeployer{result: &remote.DeployResult{ | |
| 39 | + | Status: "deployed", | |
| 40 | + | Domain: "actual.example.com", | |
| 41 | + | ContainerID: "1234567890abcdef", | |
| 42 | + | }}, false, "requested.example.com", &out, false) | |
| 43 | + | ||
| 44 | + | require.NoError(t, err) | |
| 45 | + | assert.Contains(t, out.String(), "Deployed actual.example.com (container: 1234567890ab)") | |
| 46 | + | assert.NotContains(t, out.String(), "requested.example.com") | |
| 47 | + | } | |
| @@ -1,6 +1,7 @@ | |||||
| 1 | package cli | 1 | package cli | ||
| 2 | 2 | ||||
| 3 | import ( | 3 | import ( | ||
| 4 | + | "bytes" | |||
| 4 | "context" | 5 | "context" | ||
| 5 | "testing" | 6 | "testing" | ||
| 6 | 7 | ||||
| @@ -26,8 +27,21 @@ func TestRunDeploy_LocalTypedErrorUsesDeployFormatter(t *testing.T) { | |||||
| 26 | Cause: "health check failed", | 27 | Cause: "health check failed", | ||
| 27 | Hint: "check DATABASE_URL", | 28 | Hint: "check DATABASE_URL", | ||
| 28 | Logs: []string{"booting app", "connection refused"}, | 29 | Logs: []string{"booting app", "connection refused"}, | ||
| 29 | - | }}, false, "app.example.com") | 30 | + | }}, false, "app.example.com", &bytes.Buffer{}, false) |
| 30 | 31 | ||||
| 31 | require.Error(t, err) | 32 | require.Error(t, err) | ||
| 32 | assert.Equal(t, "failed to deploy\nCause: health check failed\nHint: check DATABASE_URL\n\nRecent container logs:\n booting app\n connection refused", err.Error()) | 33 | assert.Equal(t, "failed to deploy\nCause: health check failed\nHint: check DATABASE_URL\n\nRecent container logs:\n booting app\n connection refused", err.Error()) | ||
| 33 | } | 34 | } | ||
| 35 | + | ||||
| 36 | + | func TestRunDeploy_UsesResultDomainWhenPresent(t *testing.T) { | |||
| 37 | + | var out bytes.Buffer | |||
| 38 | + | err := runDeploy(context.Background(), &deployTestDeployer{result: &remote.DeployResult{ | |||
| 39 | + | Status: "deployed", | |||
| 40 | + | Domain: "actual.example.com", | |||
| 41 | + | ContainerID: "1234567890abcdef", | |||
| 42 | + | }}, false, "requested.example.com", &out, false) | |||
| 43 | + | ||||
| 44 | + | require.NoError(t, err) | |||
| 45 | + | assert.Contains(t, out.String(), "Deployed actual.example.com (container: 1234567890ab)") | |||
| 46 | + | assert.NotContains(t, out.String(), "requested.example.com") | |||
| 47 | + | } | |||
M internal/adapters/in/cli/remote/client_test.go +1 -0
| @@ -346,6 +346,7 @@ func TestParseErrorResponse_DeployFailureFields(t *testing.T) { | |||
| 346 | 346 | require.True(t, errors.As(err, &httpErr)) | |
| 347 | 347 | assert.Equal(t, 500, httpErr.StatusCode) | |
| 348 | 348 | assert.Equal(t, "container exited during startup", httpErr.Body) | |
| 349 | + | assert.True(t, httpErr.Structured) | |
| 349 | 350 | assert.Equal(t, "health check failed", httpErr.Cause) | |
| 350 | 351 | assert.Equal(t, "check DATABASE_URL", httpErr.Hint) | |
| 351 | 352 | assert.Equal(t, []string{"booting app", "connection refused"}, httpErr.Logs) | |
| @@ -346,6 +346,7 @@ func TestParseErrorResponse_DeployFailureFields(t *testing.T) { | |||||
| 346 | require.True(t, errors.As(err, &httpErr)) | 346 | require.True(t, errors.As(err, &httpErr)) | ||
| 347 | assert.Equal(t, 500, httpErr.StatusCode) | 347 | assert.Equal(t, 500, httpErr.StatusCode) | ||
| 348 | assert.Equal(t, "container exited during startup", httpErr.Body) | 348 | assert.Equal(t, "container exited during startup", httpErr.Body) | ||
| 349 | + | assert.True(t, httpErr.Structured) | |||
| 349 | assert.Equal(t, "health check failed", httpErr.Cause) | 350 | assert.Equal(t, "health check failed", httpErr.Cause) | ||
| 350 | assert.Equal(t, "check DATABASE_URL", httpErr.Hint) | 351 | assert.Equal(t, "check DATABASE_URL", httpErr.Hint) | ||
| 351 | assert.Equal(t, []string{"booting app", "connection refused"}, httpErr.Logs) | 352 | assert.Equal(t, []string{"booting app", "connection refused"}, httpErr.Logs) | ||
M internal/adapters/in/http/admin/handler_test.go +29 -16
| @@ -6,6 +6,7 @@ import ( | |||
| 6 | 6 | "encoding/json" | |
| 7 | 7 | "errors" | |
| 8 | 8 | "fmt" | |
| 9 | + | "io" | |
| 9 | 10 | "net/http" | |
| 10 | 11 | "net/http/httptest" | |
| 11 | 12 | "net/url" | |
| @@ -1369,19 +1370,25 @@ func TestHandler_Deploy_StructuredDeployFailure(t *testing.T) { | |||
| 1369 | 1370 | d.ContainerSvc = containerSvc | |
| 1370 | 1371 | d.SecretSvc = secretSvc | |
| 1371 | 1372 | }) | |
| 1373 | + | s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | |
| 1374 | + | handler.ServeHTTP(w, r.WithContext(ctxWithScopes("admin:config:write"))) | |
| 1375 | + | })) | |
| 1376 | + | defer s.Close() | |
| 1372 | 1377 | ||
| 1373 | 1378 | configSvc.EXPECT().GetRoute(mock.Anything, "app.example.com").Return(route, nil).Once() | |
| 1374 | 1379 | containerSvc.EXPECT().Deploy(mock.Anything, *route).Return(nil, deployErr).Once() | |
| 1375 | 1380 | ||
| 1376 | - | req := httptest.NewRequest("POST", "/admin/deploy/app.example.com", nil) | |
| 1377 | - | req = req.WithContext(ctxWithScopes("admin:config:write")) | |
| 1378 | - | rec := httptest.NewRecorder() | |
| 1379 | - | ||
| 1380 | - | handler.ServeHTTP(rec, req) | |
| 1381 | + | req, err := http.NewRequestWithContext(context.Background(), http.MethodPost, s.URL+"/admin/deploy/app.example.com", nil) | |
| 1382 | + | assert.NoError(t, err) | |
| 1383 | + | resp, err := s.Client().Do(req) | |
| 1384 | + | assert.NoError(t, err) | |
| 1385 | + | defer resp.Body.Close() | |
| 1386 | + | body, err := io.ReadAll(resp.Body) | |
| 1387 | + | assert.NoError(t, err) | |
| 1381 | 1388 | ||
| 1382 | - | assert.Equal(t, http.StatusInternalServerError, rec.Code) | |
| 1383 | - | assert.JSONEq(t, `{"error":"failed to deploy","cause":"image pull failed","hint":"push the image before retrying","logs":["pulling manifest","manifest unknown"]}`, rec.Body.String()) | |
| 1384 | - | assert.NotContains(t, rec.Body.String(), "internal registry detail") | |
| 1389 | + | assert.Equal(t, http.StatusInternalServerError, resp.StatusCode) | |
| 1390 | + | assert.JSONEq(t, `{"error":"failed to deploy","cause":"image pull failed","hint":"push the image before retrying","logs":["pulling manifest","manifest unknown"]}`, string(body)) | |
| 1391 | + | assert.NotContains(t, string(body), "internal registry detail") | |
| 1385 | 1392 | } | |
| 1386 | 1393 | ||
| 1387 | 1394 | func TestHandler_Deploy_GenericErrorStillUsesLegacyBody(t *testing.T) { | |
| @@ -1398,19 +1405,25 @@ func TestHandler_Deploy_GenericErrorStillUsesLegacyBody(t *testing.T) { | |||
| 1398 | 1405 | d.ContainerSvc = containerSvc | |
| 1399 | 1406 | d.SecretSvc = secretSvc | |
| 1400 | 1407 | }) | |
| 1408 | + | s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | |
| 1409 | + | handler.ServeHTTP(w, r.WithContext(ctxWithScopes("admin:config:write"))) | |
| 1410 | + | })) | |
| 1411 | + | defer s.Close() | |
| 1401 | 1412 | ||
| 1402 | 1413 | configSvc.EXPECT().GetRoute(mock.Anything, "app.example.com").Return(route, nil).Once() | |
| 1403 | 1414 | containerSvc.EXPECT().Deploy(mock.Anything, *route).Return(nil, deployErr).Once() | |
| 1404 | 1415 | ||
| 1405 | - | req := httptest.NewRequest("POST", "/admin/deploy/app.example.com", nil) | |
| 1406 | - | req = req.WithContext(ctxWithScopes("admin:config:write")) | |
| 1407 | - | rec := httptest.NewRecorder() | |
| 1408 | - | ||
| 1409 | - | handler.ServeHTTP(rec, req) | |
| 1416 | + | req, err := http.NewRequestWithContext(context.Background(), http.MethodPost, s.URL+"/admin/deploy/app.example.com", nil) | |
| 1417 | + | assert.NoError(t, err) | |
| 1418 | + | resp, err := s.Client().Do(req) | |
| 1419 | + | assert.NoError(t, err) | |
| 1420 | + | defer resp.Body.Close() | |
| 1421 | + | body, err := io.ReadAll(resp.Body) | |
| 1422 | + | assert.NoError(t, err) | |
| 1410 | 1423 | ||
| 1411 | - | assert.Equal(t, http.StatusInternalServerError, rec.Code) | |
| 1412 | - | assert.JSONEq(t, `{"error":"failed to deploy container"}`, rec.Body.String()) | |
| 1413 | - | assert.NotContains(t, rec.Body.String(), "internal registry detail") | |
| 1424 | + | assert.Equal(t, http.StatusInternalServerError, resp.StatusCode) | |
| 1425 | + | assert.JSONEq(t, `{"error":"failed to deploy container"}`, string(body)) | |
| 1426 | + | assert.NotContains(t, string(body), "internal registry detail") | |
| 1414 | 1427 | } | |
| 1415 | 1428 | ||
| 1416 | 1429 | func TestHandler_Restart_ContainerNotFound(t *testing.T) { | |
| @@ -6,6 +6,7 @@ import ( | |||||
| 6 | "encoding/json" | 6 | "encoding/json" | ||
| 7 | "errors" | 7 | "errors" | ||
| 8 | "fmt" | 8 | "fmt" | ||
| 9 | + | "io" | |||
| 9 | "net/http" | 10 | "net/http" | ||
| 10 | "net/http/httptest" | 11 | "net/http/httptest" | ||
| 11 | "net/url" | 12 | "net/url" | ||
| @@ -1369,19 +1370,25 @@ func TestHandler_Deploy_StructuredDeployFailure(t *testing.T) { | |||||
| 1369 | d.ContainerSvc = containerSvc | 1370 | d.ContainerSvc = containerSvc | ||
| 1370 | d.SecretSvc = secretSvc | 1371 | d.SecretSvc = secretSvc | ||
| 1371 | }) | 1372 | }) | ||
| 1373 | + | s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | |||
| 1374 | + | handler.ServeHTTP(w, r.WithContext(ctxWithScopes("admin:config:write"))) | |||
| 1375 | + | })) | |||
| 1376 | + | defer s.Close() | |||
| 1372 | 1377 | ||||
| 1373 | configSvc.EXPECT().GetRoute(mock.Anything, "app.example.com").Return(route, nil).Once() | 1378 | configSvc.EXPECT().GetRoute(mock.Anything, "app.example.com").Return(route, nil).Once() | ||
| 1374 | containerSvc.EXPECT().Deploy(mock.Anything, *route).Return(nil, deployErr).Once() | 1379 | containerSvc.EXPECT().Deploy(mock.Anything, *route).Return(nil, deployErr).Once() | ||
| 1375 | 1380 | ||||
| 1376 | - | req := httptest.NewRequest("POST", "/admin/deploy/app.example.com", nil) | 1381 | + | req, err := http.NewRequestWithContext(context.Background(), http.MethodPost, s.URL+"/admin/deploy/app.example.com", nil) |
| 1377 | - | req = req.WithContext(ctxWithScopes("admin:config:write")) | 1382 | + | assert.NoError(t, err) |
| 1378 | - | rec := httptest.NewRecorder() | 1383 | + | resp, err := s.Client().Do(req) |
| 1379 | - | 1384 | + | assert.NoError(t, err) | |
| 1380 | - | handler.ServeHTTP(rec, req) | 1385 | + | defer resp.Body.Close() |
| 1386 | + | body, err := io.ReadAll(resp.Body) | |||
| 1387 | + | assert.NoError(t, err) | |||
| 1381 | 1388 | ||||
| 1382 | - | assert.Equal(t, http.StatusInternalServerError, rec.Code) | 1389 | + | assert.Equal(t, http.StatusInternalServerError, resp.StatusCode) |
| 1383 | - | assert.JSONEq(t, `{"error":"failed to deploy","cause":"image pull failed","hint":"push the image before retrying","logs":["pulling manifest","manifest unknown"]}`, rec.Body.String()) | 1390 | + | assert.JSONEq(t, `{"error":"failed to deploy","cause":"image pull failed","hint":"push the image before retrying","logs":["pulling manifest","manifest unknown"]}`, string(body)) |
| 1384 | - | assert.NotContains(t, rec.Body.String(), "internal registry detail") | 1391 | + | assert.NotContains(t, string(body), "internal registry detail") |
| 1385 | } | 1392 | } | ||
| 1386 | 1393 | ||||
| 1387 | func TestHandler_Deploy_GenericErrorStillUsesLegacyBody(t *testing.T) { | 1394 | func TestHandler_Deploy_GenericErrorStillUsesLegacyBody(t *testing.T) { | ||
| @@ -1398,19 +1405,25 @@ func TestHandler_Deploy_GenericErrorStillUsesLegacyBody(t *testing.T) { | |||||
| 1398 | d.ContainerSvc = containerSvc | 1405 | d.ContainerSvc = containerSvc | ||
| 1399 | d.SecretSvc = secretSvc | 1406 | d.SecretSvc = secretSvc | ||
| 1400 | }) | 1407 | }) | ||
| 1408 | + | s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | |||
| 1409 | + | handler.ServeHTTP(w, r.WithContext(ctxWithScopes("admin:config:write"))) | |||
| 1410 | + | })) | |||
| 1411 | + | defer s.Close() | |||
| 1401 | 1412 | ||||
| 1402 | configSvc.EXPECT().GetRoute(mock.Anything, "app.example.com").Return(route, nil).Once() | 1413 | configSvc.EXPECT().GetRoute(mock.Anything, "app.example.com").Return(route, nil).Once() | ||
| 1403 | containerSvc.EXPECT().Deploy(mock.Anything, *route).Return(nil, deployErr).Once() | 1414 | containerSvc.EXPECT().Deploy(mock.Anything, *route).Return(nil, deployErr).Once() | ||
| 1404 | 1415 | ||||
| 1405 | - | req := httptest.NewRequest("POST", "/admin/deploy/app.example.com", nil) | 1416 | + | req, err := http.NewRequestWithContext(context.Background(), http.MethodPost, s.URL+"/admin/deploy/app.example.com", nil) |
| 1406 | - | req = req.WithContext(ctxWithScopes("admin:config:write")) | 1417 | + | assert.NoError(t, err) |
| 1407 | - | rec := httptest.NewRecorder() | 1418 | + | resp, err := s.Client().Do(req) |
| 1408 | - | 1419 | + | assert.NoError(t, err) | |
| 1409 | - | handler.ServeHTTP(rec, req) | 1420 | + | defer resp.Body.Close() |
| 1421 | + | body, err := io.ReadAll(resp.Body) | |||
| 1422 | + | assert.NoError(t, err) | |||
| 1410 | 1423 | ||||
| 1411 | - | assert.Equal(t, http.StatusInternalServerError, rec.Code) | 1424 | + | assert.Equal(t, http.StatusInternalServerError, resp.StatusCode) |
| 1412 | - | assert.JSONEq(t, `{"error":"failed to deploy container"}`, rec.Body.String()) | 1425 | + | assert.JSONEq(t, `{"error":"failed to deploy container"}`, string(body)) |
| 1413 | - | assert.NotContains(t, rec.Body.String(), "internal registry detail") | 1426 | + | assert.NotContains(t, string(body), "internal registry detail") |
| 1414 | } | 1427 | } | ||
| 1415 | 1428 | ||||
| 1416 | func TestHandler_Restart_ContainerNotFound(t *testing.T) { | 1429 | func TestHandler_Restart_ContainerNotFound(t *testing.T) { | ||
M internal/usecase/container/service.go +2 -1
| @@ -320,7 +320,8 @@ func (s *Service) Deploy(ctx context.Context, route domain.Route) (*domain.Conta | |||
| 320 | 320 | resources, err := s.prepareDeployResources(ctx, route, existing) | |
| 321 | 321 | if err != nil { | |
| 322 | 322 | if deployErr := s.wrapPullDeployFailure(route, existing, err); deployErr != nil { | |
| 323 | - | return nil, deployErr | |
| 323 | + | err = deployErr | |
| 324 | + | return nil, err | |
| 324 | 325 | } | |
| 325 | 326 | return nil, err | |
| 326 | 327 | } | |
| @@ -320,7 +320,8 @@ func (s *Service) Deploy(ctx context.Context, route domain.Route) (*domain.Conta | |||||
| 320 | resources, err := s.prepareDeployResources(ctx, route, existing) | 320 | resources, err := s.prepareDeployResources(ctx, route, existing) | ||
| 321 | if err != nil { | 321 | if err != nil { | ||
| 322 | if deployErr := s.wrapPullDeployFailure(route, existing, err); deployErr != nil { | 322 | if deployErr := s.wrapPullDeployFailure(route, existing, err); deployErr != nil { | ||
| 323 | - | return nil, deployErr | 323 | + | err = deployErr |
| 324 | + | return nil, err | |||
| 324 | } | 325 | } | ||
| 325 | return nil, err | 326 | return nil, err | ||
| 326 | } | 327 | } | ||