fix(cli) narrow retry to gateway errors and warn on insecure TLS
- commit
- 2b588606410eef052b591d62e818045be7db4491
- author
- bnema · · 2026-02-06 20:43 UTC
- parent
- fce0810
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
2 files · +12 -5
M internal/adapters/in/cli/remote/client.go +9 -2
| @@ -11,6 +11,7 @@ import ( | |||
| 11 | 11 | "io" | |
| 12 | 12 | "net/http" | |
| 13 | 13 | "net/url" | |
| 14 | + | "os" | |
| 14 | 15 | "strings" | |
| 15 | 16 | "time" | |
| 16 | 17 | ||
| @@ -88,6 +89,8 @@ func (c *Client) applyTLSConfig() { | |||
| 88 | 89 | return | |
| 89 | 90 | } | |
| 90 | 91 | ||
| 92 | + | fmt.Fprintf(os.Stderr, "WARNING: TLS certificate verification disabled for %s\n", c.baseURL) | |
| 93 | + | ||
| 91 | 94 | var transport *http.Transport | |
| 92 | 95 | if existing, ok := c.httpClient.Transport.(*http.Transport); ok { | |
| 93 | 96 | transport = existing.Clone() | |
| @@ -96,7 +99,9 @@ func (c *Client) applyTLSConfig() { | |||
| 96 | 99 | } | |
| 97 | 100 | ||
| 98 | 101 | if transport.TLSClientConfig == nil { | |
| 99 | - | transport.TLSClientConfig = &tls.Config{} | |
| 102 | + | transport.TLSClientConfig = &tls.Config{MinVersion: tls.VersionTLS12} | |
| 103 | + | } else { | |
| 104 | + | transport.TLSClientConfig.MinVersion = tls.VersionTLS12 | |
| 100 | 105 | } | |
| 101 | 106 | //nolint:gosec // Explicit CLI opt-in via --insecure for self-signed/private cert deployments. | |
| 102 | 107 | transport.TLSClientConfig.InsecureSkipVerify = true | |
| @@ -164,7 +169,9 @@ func isRetryableRequestError(err error) bool { | |||
| 164 | 169 | } | |
| 165 | 170 | ||
| 166 | 171 | func isRetryableStatus(status int) bool { | |
| 167 | - | return status >= 500 && status <= 599 | |
| 172 | + | // Only retry gateway errors; retrying 500 on non-idempotent POSTs | |
| 173 | + | // (deploy, restart, reload) can cause duplicate state changes. | |
| 174 | + | return status == 502 || status == 503 || status == 504 | |
| 168 | 175 | } | |
| 169 | 176 | ||
| 170 | 177 | func retryDelay(attempt int) time.Duration { | |
| @@ -11,6 +11,7 @@ import ( | |||||
| 11 | "io" | 11 | "io" | ||
| 12 | "net/http" | 12 | "net/http" | ||
| 13 | "net/url" | 13 | "net/url" | ||
| 14 | + | "os" | |||
| 14 | "strings" | 15 | "strings" | ||
| 15 | "time" | 16 | "time" | ||
| 16 | 17 | ||||
| @@ -88,6 +89,8 @@ func (c *Client) applyTLSConfig() { | |||||
| 88 | return | 89 | return | ||
| 89 | } | 90 | } | ||
| 90 | 91 | ||||
| 92 | + | fmt.Fprintf(os.Stderr, "WARNING: TLS certificate verification disabled for %s\n", c.baseURL) | |||
| 93 | + | ||||
| 91 | var transport *http.Transport | 94 | var transport *http.Transport | ||
| 92 | if existing, ok := c.httpClient.Transport.(*http.Transport); ok { | 95 | if existing, ok := c.httpClient.Transport.(*http.Transport); ok { | ||
| 93 | transport = existing.Clone() | 96 | transport = existing.Clone() | ||
| @@ -96,7 +99,9 @@ func (c *Client) applyTLSConfig() { | |||||
| 96 | } | 99 | } | ||
| 97 | 100 | ||||
| 98 | if transport.TLSClientConfig == nil { | 101 | if transport.TLSClientConfig == nil { | ||
| 99 | - | transport.TLSClientConfig = &tls.Config{} | 102 | + | transport.TLSClientConfig = &tls.Config{MinVersion: tls.VersionTLS12} |
| 103 | + | } else { | |||
| 104 | + | transport.TLSClientConfig.MinVersion = tls.VersionTLS12 | |||
| 100 | } | 105 | } | ||
| 101 | //nolint:gosec // Explicit CLI opt-in via --insecure for self-signed/private cert deployments. | 106 | //nolint:gosec // Explicit CLI opt-in via --insecure for self-signed/private cert deployments. | ||
| 102 | transport.TLSClientConfig.InsecureSkipVerify = true | 107 | transport.TLSClientConfig.InsecureSkipVerify = true | ||
| @@ -164,7 +169,9 @@ func isRetryableRequestError(err error) bool { | |||||
| 164 | } | 169 | } | ||
| 165 | 170 | ||||
| 166 | func isRetryableStatus(status int) bool { | 171 | func isRetryableStatus(status int) bool { | ||
| 167 | - | return status >= 500 && status <= 599 | 172 | + | // Only retry gateway errors; retrying 500 on non-idempotent POSTs |
| 173 | + | // (deploy, restart, reload) can cause duplicate state changes. | |||
| 174 | + | return status == 502 || status == 503 || status == 504 | |||
| 168 | } | 175 | } | ||
| 169 | 176 | ||||
| 170 | func retryDelay(attempt int) time.Duration { | 177 | func retryDelay(attempt int) time.Duration { | ||
M internal/adapters/in/cli/remote/client_test.go +3 -3
| @@ -82,15 +82,15 @@ func TestClientDeployReturnsErrorAfterRetryExhaustion(t *testing.T) { | |||
| 82 | 82 | require.Equal(t, "/admin/deploy/test.example.com", r.URL.Path) | |
| 83 | 83 | require.Equal(t, http.MethodPost, r.Method) | |
| 84 | 84 | atomic.AddInt32(&attempts, 1) | |
| 85 | - | w.WriteHeader(http.StatusInternalServerError) | |
| 86 | - | _, _ = w.Write([]byte(`{"error":"failed to deploy container"}`)) | |
| 85 | + | w.WriteHeader(http.StatusBadGateway) | |
| 86 | + | _, _ = w.Write([]byte(`{"error":"upstream unavailable"}`)) | |
| 87 | 87 | })) | |
| 88 | 88 | defer srv.Close() | |
| 89 | 89 | ||
| 90 | 90 | client := NewClient(srv.URL) | |
| 91 | 91 | _, err := client.Deploy(context.Background(), "test.example.com") | |
| 92 | 92 | require.Error(t, err) | |
| 93 | - | assert.Contains(t, err.Error(), "500 Internal Server Error: failed to deploy container") | |
| 93 | + | assert.Contains(t, err.Error(), "502 Bad Gateway: upstream unavailable") | |
| 94 | 94 | assert.EqualValues(t, retryMaxAttempts, atomic.LoadInt32(&attempts)) | |
| 95 | 95 | } | |
| 96 | 96 | ||
| @@ -82,15 +82,15 @@ func TestClientDeployReturnsErrorAfterRetryExhaustion(t *testing.T) { | |||||
| 82 | require.Equal(t, "/admin/deploy/test.example.com", r.URL.Path) | 82 | require.Equal(t, "/admin/deploy/test.example.com", r.URL.Path) | ||
| 83 | require.Equal(t, http.MethodPost, r.Method) | 83 | require.Equal(t, http.MethodPost, r.Method) | ||
| 84 | atomic.AddInt32(&attempts, 1) | 84 | atomic.AddInt32(&attempts, 1) | ||
| 85 | - | w.WriteHeader(http.StatusInternalServerError) | 85 | + | w.WriteHeader(http.StatusBadGateway) |
| 86 | - | _, _ = w.Write([]byte(`{"error":"failed to deploy container"}`)) | 86 | + | _, _ = w.Write([]byte(`{"error":"upstream unavailable"}`)) |
| 87 | })) | 87 | })) | ||
| 88 | defer srv.Close() | 88 | defer srv.Close() | ||
| 89 | 89 | ||||
| 90 | client := NewClient(srv.URL) | 90 | client := NewClient(srv.URL) | ||
| 91 | _, err := client.Deploy(context.Background(), "test.example.com") | 91 | _, err := client.Deploy(context.Background(), "test.example.com") | ||
| 92 | require.Error(t, err) | 92 | require.Error(t, err) | ||
| 93 | - | assert.Contains(t, err.Error(), "500 Internal Server Error: failed to deploy container") | 93 | + | assert.Contains(t, err.Error(), "502 Bad Gateway: upstream unavailable") |
| 94 | assert.EqualValues(t, retryMaxAttempts, atomic.LoadInt32(&attempts)) | 94 | assert.EqualValues(t, retryMaxAttempts, atomic.LoadInt32(&attempts)) | ||
| 95 | } | 95 | } | ||
| 96 | 96 | ||||