feat: gracefully skip explicit deploy when token lacks config:write scope
- commit
- c9f8ad6f2ad2237ce15dda4b0b923aa4d62f3415
- author
- brice · · 2026-03-19 04:52 UTC
- parent
- 34cd6d6
- signature
- unsigned
2 files · +38 -3
M internal/adapters/in/cli/push.go +18 -3
| @@ -3,7 +3,9 @@ package cli | |||
| 3 | 3 | import ( | |
| 4 | 4 | "bufio" | |
| 5 | 5 | "context" | |
| 6 | + | "errors" | |
| 6 | 7 | "fmt" | |
| 8 | + | "net/http" | |
| 7 | 9 | "os" | |
| 8 | 10 | "os/exec" | |
| 9 | 11 | "path/filepath" | |
| @@ -202,10 +204,16 @@ func runPush(ctx context.Context, req pushRequest) error { | |||
| 202 | 204 | ||
| 203 | 205 | // Signal the server to suppress event-based deploys for this image. | |
| 204 | 206 | // The CLI will trigger an explicit deploy after push completes. | |
| 207 | + | skipExplicitDeploy := req.NoDeploy | |
| 205 | 208 | if !req.NoDeploy { | |
| 206 | 209 | if err := handle.plane.DeployIntent(ctx, imageName); err != nil { | |
| 207 | - | // Non-fatal: worst case we get a redundant deploy via event | |
| 208 | - | fmt.Fprintf(os.Stderr, "warning: failed to register deploy intent: %v\n", err) | |
| 210 | + | if isInsufficientScope(err) { | |
| 211 | + | fmt.Fprintln(os.Stderr, "info: deploy intent skipped (insufficient scope), server will auto-deploy on image receive") | |
| 212 | + | skipExplicitDeploy = true | |
| 213 | + | } else { | |
| 214 | + | // Non-fatal: worst case we get a redundant deploy via event | |
| 215 | + | fmt.Fprintf(os.Stderr, "warning: failed to register deploy intent: %v\n", err) | |
| 216 | + | } | |
| 209 | 217 | } | |
| 210 | 218 | } | |
| 211 | 219 | ||
| @@ -228,13 +236,20 @@ func runPush(ctx context.Context, req pushRequest) error { | |||
| 228 | 236 | ||
| 229 | 237 | fmt.Println(styles.RenderSuccess("Push complete")) | |
| 230 | 238 | ||
| 231 | - | if !req.NoDeploy { | |
| 239 | + | if !skipExplicitDeploy { | |
| 232 | 240 | return deployAfterPush(ctx, handle.plane, pushDomain, req.NoConfirm) | |
| 233 | 241 | } | |
| 234 | 242 | ||
| 235 | 243 | return nil | |
| 236 | 244 | } | |
| 237 | 245 | ||
| 246 | + | // isInsufficientScope returns true if the error is an HTTP 403 Forbidden response, | |
| 247 | + | // meaning the token lacks the required admin scope. | |
| 248 | + | func isInsufficientScope(err error) bool { | |
| 249 | + | var httpErr *remote.HTTPError | |
| 250 | + | return errors.As(err, &httpErr) && httpErr.StatusCode == http.StatusForbidden | |
| 251 | + | } | |
| 252 | + | ||
| 238 | 253 | // resolveFromDomain resolves image info from a domain name (legacy mode). | |
| 239 | 254 | func resolveFromDomain(ctx context.Context, cp ControlPlane, pushDomain string) (registry, imageName, resolvedDomain string, err error) { | |
| 240 | 255 | route, err := cp.GetRoute(ctx, pushDomain) | |
| @@ -3,7 +3,9 @@ package cli | |||||
| 3 | import ( | 3 | import ( | ||
| 4 | "bufio" | 4 | "bufio" | ||
| 5 | "context" | 5 | "context" | ||
| 6 | + | "errors" | |||
| 6 | "fmt" | 7 | "fmt" | ||
| 8 | + | "net/http" | |||
| 7 | "os" | 9 | "os" | ||
| 8 | "os/exec" | 10 | "os/exec" | ||
| 9 | "path/filepath" | 11 | "path/filepath" | ||
| @@ -202,10 +204,16 @@ func runPush(ctx context.Context, req pushRequest) error { | |||||
| 202 | 204 | ||||
| 203 | // Signal the server to suppress event-based deploys for this image. | 205 | // Signal the server to suppress event-based deploys for this image. | ||
| 204 | // The CLI will trigger an explicit deploy after push completes. | 206 | // The CLI will trigger an explicit deploy after push completes. | ||
| 207 | + | skipExplicitDeploy := req.NoDeploy | |||
| 205 | if !req.NoDeploy { | 208 | if !req.NoDeploy { | ||
| 206 | if err := handle.plane.DeployIntent(ctx, imageName); err != nil { | 209 | if err := handle.plane.DeployIntent(ctx, imageName); err != nil { | ||
| 207 | - | // Non-fatal: worst case we get a redundant deploy via event | 210 | + | if isInsufficientScope(err) { |
| 208 | - | fmt.Fprintf(os.Stderr, "warning: failed to register deploy intent: %v\n", err) | 211 | + | fmt.Fprintln(os.Stderr, "info: deploy intent skipped (insufficient scope), server will auto-deploy on image receive") |
| 212 | + | skipExplicitDeploy = true | |||
| 213 | + | } else { | |||
| 214 | + | // Non-fatal: worst case we get a redundant deploy via event | |||
| 215 | + | fmt.Fprintf(os.Stderr, "warning: failed to register deploy intent: %v\n", err) | |||
| 216 | + | } | |||
| 209 | } | 217 | } | ||
| 210 | } | 218 | } | ||
| 211 | 219 | ||||
| @@ -228,13 +236,20 @@ func runPush(ctx context.Context, req pushRequest) error { | |||||
| 228 | 236 | ||||
| 229 | fmt.Println(styles.RenderSuccess("Push complete")) | 237 | fmt.Println(styles.RenderSuccess("Push complete")) | ||
| 230 | 238 | ||||
| 231 | - | if !req.NoDeploy { | 239 | + | if !skipExplicitDeploy { |
| 232 | return deployAfterPush(ctx, handle.plane, pushDomain, req.NoConfirm) | 240 | return deployAfterPush(ctx, handle.plane, pushDomain, req.NoConfirm) | ||
| 233 | } | 241 | } | ||
| 234 | 242 | ||||
| 235 | return nil | 243 | return nil | ||
| 236 | } | 244 | } | ||
| 237 | 245 | ||||
| 246 | + | // isInsufficientScope returns true if the error is an HTTP 403 Forbidden response, | |||
| 247 | + | // meaning the token lacks the required admin scope. | |||
| 248 | + | func isInsufficientScope(err error) bool { | |||
| 249 | + | var httpErr *remote.HTTPError | |||
| 250 | + | return errors.As(err, &httpErr) && httpErr.StatusCode == http.StatusForbidden | |||
| 251 | + | } | |||
| 252 | + | ||||
| 238 | // resolveFromDomain resolves image info from a domain name (legacy mode). | 253 | // resolveFromDomain resolves image info from a domain name (legacy mode). | ||
| 239 | func resolveFromDomain(ctx context.Context, cp ControlPlane, pushDomain string) (registry, imageName, resolvedDomain string, err error) { | 254 | func resolveFromDomain(ctx context.Context, cp ControlPlane, pushDomain string) (registry, imageName, resolvedDomain string, err error) { | ||
| 240 | route, err := cp.GetRoute(ctx, pushDomain) | 255 | route, err := cp.GetRoute(ctx, pushDomain) | ||
M internal/adapters/in/cli/push_test.go +20 -0
| @@ -2,6 +2,7 @@ package cli | |||
| 2 | 2 | ||
| 3 | 3 | import ( | |
| 4 | 4 | "context" | |
| 5 | + | "fmt" | |
| 5 | 6 | "io" | |
| 6 | 7 | "os" | |
| 7 | 8 | "os/exec" | |
| @@ -595,3 +596,22 @@ func TestResolveFromImage_NoRouteSuggestsBootstrap(t *testing.T) { | |||
| 595 | 596 | assert.Contains(t, err.Error(), `no route configured for image "myapp"`) | |
| 596 | 597 | assert.Contains(t, err.Error(), "gordon bootstrap") | |
| 597 | 598 | } | |
| 599 | + | ||
| 600 | + | func TestIsInsufficientScope(t *testing.T) { | |
| 601 | + | tests := []struct { | |
| 602 | + | name string | |
| 603 | + | err error | |
| 604 | + | want bool | |
| 605 | + | }{ | |
| 606 | + | {"nil error", nil, false}, | |
| 607 | + | {"HTTPError 403", &remote.HTTPError{StatusCode: 403, Status: "403 Forbidden", Body: "insufficient scope"}, true}, | |
| 608 | + | {"wrapped HTTPError 403", fmt.Errorf("deploy intent: %w", &remote.HTTPError{StatusCode: 403, Status: "403 Forbidden", Body: "scope"}), true}, | |
| 609 | + | {"HTTPError 500", &remote.HTTPError{StatusCode: 500, Status: "500 Internal Server Error", Body: "broke"}, false}, | |
| 610 | + | {"plain error", fmt.Errorf("connection refused"), false}, | |
| 611 | + | } | |
| 612 | + | for _, tt := range tests { | |
| 613 | + | t.Run(tt.name, func(t *testing.T) { | |
| 614 | + | assert.Equal(t, tt.want, isInsufficientScope(tt.err)) | |
| 615 | + | }) | |
| 616 | + | } | |
| 617 | + | } | |
| @@ -2,6 +2,7 @@ package cli | |||||
| 2 | 2 | ||||
| 3 | import ( | 3 | import ( | ||
| 4 | "context" | 4 | "context" | ||
| 5 | + | "fmt" | |||
| 5 | "io" | 6 | "io" | ||
| 6 | "os" | 7 | "os" | ||
| 7 | "os/exec" | 8 | "os/exec" | ||
| @@ -595,3 +596,22 @@ func TestResolveFromImage_NoRouteSuggestsBootstrap(t *testing.T) { | |||||
| 595 | assert.Contains(t, err.Error(), `no route configured for image "myapp"`) | 596 | assert.Contains(t, err.Error(), `no route configured for image "myapp"`) | ||
| 596 | assert.Contains(t, err.Error(), "gordon bootstrap") | 597 | assert.Contains(t, err.Error(), "gordon bootstrap") | ||
| 597 | } | 598 | } | ||
| 599 | + | ||||
| 600 | + | func TestIsInsufficientScope(t *testing.T) { | |||
| 601 | + | tests := []struct { | |||
| 602 | + | name string | |||
| 603 | + | err error | |||
| 604 | + | want bool | |||
| 605 | + | }{ | |||
| 606 | + | {"nil error", nil, false}, | |||
| 607 | + | {"HTTPError 403", &remote.HTTPError{StatusCode: 403, Status: "403 Forbidden", Body: "insufficient scope"}, true}, | |||
| 608 | + | {"wrapped HTTPError 403", fmt.Errorf("deploy intent: %w", &remote.HTTPError{StatusCode: 403, Status: "403 Forbidden", Body: "scope"}), true}, | |||
| 609 | + | {"HTTPError 500", &remote.HTTPError{StatusCode: 500, Status: "500 Internal Server Error", Body: "broke"}, false}, | |||
| 610 | + | {"plain error", fmt.Errorf("connection refused"), false}, | |||
| 611 | + | } | |||
| 612 | + | for _, tt := range tests { | |||
| 613 | + | t.Run(tt.name, func(t *testing.T) { | |||
| 614 | + | assert.Equal(t, tt.want, isInsufficientScope(tt.err)) | |||
| 615 | + | }) | |||
| 616 | + | } | |||
| 617 | + | } | |||