diff --git a/README.md b/README.md index d0a18b8c..78591097 100644 --- a/README.md +++ b/README.md @@ -105,6 +105,13 @@ Create an API key from the [Kernel dashboard](https://dashboard.onkernel.com). - `--log-level ` - Set log level (trace, debug, info, warn, error, fatal, print) - `--project ` - Scope requests to a project by ID or exact name (or set `KERNEL_PROJECT`). Project-scoped OAuth tokens cannot switch projects. +### Delete errors + +Resource delete commands return a nonzero exit status and show the API error when a request +fails, including HTTP 404 for an invalid project or a missing resource. A 404 is not treated +as successful deletion. This also applies to resource lookups performed before confirmation. +Successful API responses and user-cancelled confirmations still exit successfully. + ## JSON Output Many commands support JSON output for scripting and automation. Use `--output json` or `-o json` to get machine-readable output: diff --git a/cmd/api_keys.go b/cmd/api_keys.go index 33a3344a..a8aeb401 100644 --- a/cmd/api_keys.go +++ b/cmd/api_keys.go @@ -322,9 +322,6 @@ func (c APIKeysCmd) Delete(ctx context.Context, in APIKeysDeleteInput) error { } if err := c.apiKeys.Delete(ctx, in.ID); err != nil { - if util.IsNotFound(err) { - return fmt.Errorf("API key %q not found", in.ID) - } return util.CleanedUpSdkError{Err: err} } diff --git a/cmd/api_keys_test.go b/cmd/api_keys_test.go index 23d12167..9b782c9d 100644 --- a/cmd/api_keys_test.go +++ b/cmd/api_keys_test.go @@ -260,7 +260,9 @@ func TestAPIKeysDeleteReturnsNotFoundError(t *testing.T) { err := c.Delete(context.Background(), APIKeysDeleteInput{ID: "missing_key", SkipConfirm: true}) require.Error(t, err) - assert.Contains(t, err.Error(), `API key "missing_key" not found`) + var apiErr *kernel.Error + require.ErrorAs(t, err, &apiErr) + assert.Equal(t, http.StatusNotFound, apiErr.StatusCode) } func TestAPIKeysDeleteReturnsAPIError(t *testing.T) { diff --git a/cmd/auth_connections.go b/cmd/auth_connections.go index dc2cb263..775faf0d 100644 --- a/cmd/auth_connections.go +++ b/cmd/auth_connections.go @@ -739,10 +739,6 @@ func (c AuthConnectionCmd) Delete(ctx context.Context, in AuthConnectionDeleteIn } if err := c.svc.Delete(ctx, in.ID); err != nil { - if util.IsNotFound(err) { - pterm.Info.Printf("Managed auth '%s' not found\n", in.ID) - return nil - } return util.CleanedUpSdkError{Err: err} } pterm.Success.Printf("Deleted managed auth: %s\n", in.ID) diff --git a/cmd/browsers.go b/cmd/browsers.go index caf2cfe2..7bee9f7b 100644 --- a/cmd/browsers.go +++ b/cmd/browsers.go @@ -747,11 +747,10 @@ func buildBrowserTableData(sessionID, cdpURL, liveViewURL string, profile kernel } func (b BrowsersCmd) Delete(ctx context.Context, in BrowsersDeleteInput) error { - // Treat not found as a success (idempotent delete) - if err := b.browsers.DeleteByID(ctx, in.Identifier); err != nil && !util.IsNotFound(err) { + if err := b.browsers.DeleteByID(ctx, in.Identifier); err != nil { return util.CleanedUpSdkError{Err: err} } - pterm.Success.Printf("Successfully deleted (or already absent) browser: %s\n", in.Identifier) + pterm.Success.Printf("Successfully deleted browser: %s\n", in.Identifier) return nil } diff --git a/cmd/browsers_test.go b/cmd/browsers_test.go index a039f2fd..cedf0670 100644 --- a/cmd/browsers_test.go +++ b/cmd/browsers_test.go @@ -1009,7 +1009,7 @@ func TestBrowsersDelete_Success(t *testing.T) { _ = b.Delete(context.Background(), BrowsersDeleteInput{Identifier: "any"}) out := outBuf.String() - assert.Contains(t, out, "Successfully deleted (or already absent) browser: any") + assert.Contains(t, out, "Successfully deleted browser: any") } func TestBrowsersDelete_Failure(t *testing.T) { diff --git a/cmd/credential_providers.go b/cmd/credential_providers.go index 42f5b67b..c1b54032 100644 --- a/cmd/credential_providers.go +++ b/cmd/credential_providers.go @@ -284,10 +284,6 @@ func (c CredentialProvidersCmd) Delete(ctx context.Context, in CredentialProvide } if err := c.providers.Delete(ctx, in.ID); err != nil { - if util.IsNotFound(err) { - pterm.Info.Printf("Credential provider '%s' not found\n", in.ID) - return nil - } return util.CleanedUpSdkError{Err: err} } pterm.Success.Printf("Deleted credential provider: %s\n", in.ID) diff --git a/cmd/credentials.go b/cmd/credentials.go index bf7b764c..b4272025 100644 --- a/cmd/credentials.go +++ b/cmd/credentials.go @@ -305,10 +305,6 @@ func (c CredentialsCmd) Delete(ctx context.Context, in CredentialsDeleteInput) e } if err := c.credentials.Delete(ctx, in.Identifier); err != nil { - if util.IsNotFound(err) { - pterm.Info.Printf("Credential '%s' not found\n", in.Identifier) - return nil - } return util.CleanedUpSdkError{Err: err} } pterm.Success.Printf("Deleted credential: %s\n", in.Identifier) diff --git a/cmd/delete_errors_test.go b/cmd/delete_errors_test.go new file mode 100644 index 00000000..58272360 --- /dev/null +++ b/cmd/delete_errors_test.go @@ -0,0 +1,109 @@ +package cmd + +import ( + "context" + "fmt" + "io" + "net/http" + "net/http/httptest" + "testing" + + "github.com/kernel/cli/pkg/util" + kernel "github.com/kernel/kernel-go-sdk" + "github.com/kernel/kernel-go-sdk/option" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestDeleteAPIErrors(t *testing.T) { + commands := []struct { + name string + lookup bool + run func(context.Context, kernel.Client) error + }{ + {"browsers", false, func(ctx context.Context, client kernel.Client) error { + return (BrowsersCmd{browsers: &client.Browsers}).Delete(ctx, BrowsersDeleteInput{Identifier: "resource"}) + }}, + {"profiles lookup", true, func(ctx context.Context, client kernel.Client) error { + return (ProfilesCmd{profiles: &client.Profiles}).Delete(ctx, ProfilesDeleteInput{Identifier: "resource", SkipConfirm: true}) + }}, + {"profiles delete", false, func(ctx context.Context, client kernel.Client) error { + return (ProfilesCmd{profiles: &client.Profiles}).Delete(ctx, ProfilesDeleteInput{Identifier: "resource", SkipConfirm: true}) + }}, + {"extensions", false, func(ctx context.Context, client kernel.Client) error { + return (ExtensionsCmd{extensions: &client.Extensions}).Delete(ctx, ExtensionsDeleteInput{Identifier: "resource", SkipConfirm: true}) + }}, + {"credentials", false, func(ctx context.Context, client kernel.Client) error { + return (CredentialsCmd{credentials: &client.Credentials}).Delete(ctx, CredentialsDeleteInput{Identifier: "resource", SkipConfirm: true}) + }}, + {"credential providers", false, func(ctx context.Context, client kernel.Client) error { + return (CredentialProvidersCmd{providers: &client.CredentialProviders}).Delete(ctx, CredentialProvidersDeleteInput{ID: "resource", SkipConfirm: true}) + }}, + {"managed auth", false, func(ctx context.Context, client kernel.Client) error { + return (AuthConnectionCmd{svc: &client.Auth.Connections}).Delete(ctx, AuthConnectionDeleteInput{ID: "resource", SkipConfirm: true}) + }}, + {"telemetry destinations", false, func(ctx context.Context, client kernel.Client) error { + return (TelemetryDestinationsCmd{destinations: &client.Telemetry.Destinations}).Delete(ctx, TelemetryDestinationsDeleteInput{Identifier: "resource", SkipConfirm: true}) + }}, + {"api keys", false, func(ctx context.Context, client kernel.Client) error { + return (APIKeysCmd{apiKeys: &client.APIKeys}).Delete(ctx, APIKeysDeleteInput{ID: "resource", SkipConfirm: true}) + }}, + } + responses := []struct { + status int + body, message string + }{ + {404, `{"code":"project_not_found","message":"Project not found or inactive"}`, "project_not_found: Project not found or inactive"}, + {404, `{"code":"not_found","message":"Resource not found"}`, "not_found: Resource not found"}, + {403, `{"code":"forbidden","message":"Access denied"}`, "forbidden: Access denied"}, + {409, `{"code":"conflict","message":"Resource is still in use"}`, "conflict: Resource is still in use"}, + {500, `{"code":"internal_error","message":"Deletion failed"}`, "internal_error: Deletion failed"}, + {204, "", ""}, + } + for _, command := range commands { + for _, response := range responses { + if command.lookup && response.status == http.StatusNoContent { + continue + } + t.Run(fmt.Sprintf("%s/%d/%s", command.name, response.status, response.message), func(t *testing.T) { + buf := capturePtermOutput(t) + calls := 0 + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + calls++ + assert.Equal(t, "selected-project", r.Header.Get("X-Kernel-Project")) + w.Header().Set("Content-Type", "application/json") + if r.Method == http.MethodGet && !command.lookup { + assert.Equal(t, "profiles delete", command.name) + _, _ = io.WriteString(w, `{"id":"resource","name":"resource"}`) + return + } + if command.lookup { + assert.Equal(t, http.MethodGet, r.Method) + } else { + assert.Equal(t, http.MethodDelete, r.Method) + } + w.WriteHeader(response.status) + _, _ = io.WriteString(w, response.body) + })) + defer server.Close() + client := kernel.NewClient(option.WithBaseURL(server.URL), option.WithAPIKey("test"), option.WithProject("selected-project"), option.WithMaxRetries(0)) + err := command.run(context.Background(), client) + if response.status == http.StatusNoContent { + require.NoError(t, err) + assert.Regexp(t, "[Dd]eleted", buf.String()) + } else { + var apiErr *kernel.Error + require.ErrorAs(t, err, &apiErr) + assert.Equal(t, response.status, apiErr.StatusCode) + assert.Equal(t, response.message, util.CleanedUpSdkError{Err: err}.Error()) + assert.Empty(t, buf.String()) + } + expectedCalls := 1 + if command.name == "profiles delete" { + expectedCalls = 2 + } + assert.Equal(t, expectedCalls, calls) + }) + } + } +} diff --git a/cmd/extensions.go b/cmd/extensions.go index 17b78dad..b3ddc111 100644 --- a/cmd/extensions.go +++ b/cmd/extensions.go @@ -202,10 +202,6 @@ func (e ExtensionsCmd) Delete(ctx context.Context, in ExtensionsDeleteInput) err } if err := e.extensions.Delete(ctx, in.Identifier); err != nil { - if util.IsNotFound(err) { - pterm.Info.Printf("Extension '%s' not found\n", in.Identifier) - return nil - } return util.CleanedUpSdkError{Err: err} } pterm.Success.Printf("Deleted extension: %s\n", in.Identifier) diff --git a/cmd/extensions_test.go b/cmd/extensions_test.go index d11e0f0f..246817d6 100644 --- a/cmd/extensions_test.go +++ b/cmd/extensions_test.go @@ -160,8 +160,9 @@ func TestExtensionsDelete_NotFound(t *testing.T) { return &kernel.Error{StatusCode: http.StatusNotFound} }} e := ExtensionsCmd{extensions: fake} - _ = e.Delete(context.Background(), ExtensionsDeleteInput{Identifier: "missing", SkipConfirm: true}) - assert.Contains(t, buf.String(), "not found") + err := e.Delete(context.Background(), ExtensionsDeleteInput{Identifier: "missing", SkipConfirm: true}) + assert.Error(t, err) + assert.Empty(t, buf.String()) } func TestExtensionsDownload_MissingOutput(t *testing.T) { diff --git a/cmd/profiles.go b/cmd/profiles.go index 9cfa5e71..a0bc7243 100644 --- a/cmd/profiles.go +++ b/cmd/profiles.go @@ -229,13 +229,8 @@ func (p ProfilesCmd) Create(ctx context.Context, in ProfilesCreateInput) error { } func (p ProfilesCmd) Delete(ctx context.Context, in ProfilesDeleteInput) error { - // Resolve using Get first; treat not found as success with a message item, err := p.profiles.Get(ctx, in.Identifier) if err != nil { - if util.IsNotFound(err) { - pterm.Info.Printf("Profile '%s' not found\n", in.Identifier) - return nil - } return util.CleanedUpSdkError{Err: err} } if item == nil || item.ID == "" { @@ -258,10 +253,6 @@ func (p ProfilesCmd) Delete(ctx context.Context, in ProfilesDeleteInput) error { } if err := p.profiles.Delete(ctx, in.Identifier); err != nil { - if util.IsNotFound(err) { - pterm.Info.Printf("Profile '%s' not found\n", in.Identifier) - return nil - } return util.CleanedUpSdkError{Err: err} } pterm.Success.Printf("Deleted profile: %s\n", in.Identifier) diff --git a/cmd/profiles_test.go b/cmd/profiles_test.go index d8af4798..fa240b95 100644 --- a/cmd/profiles_test.go +++ b/cmd/profiles_test.go @@ -198,8 +198,9 @@ func TestProfilesDelete_ConfirmNotFound(t *testing.T) { return nil, &kernel.Error{StatusCode: http.StatusNotFound} }} p := ProfilesCmd{profiles: fake} - _ = p.Delete(context.Background(), ProfilesDeleteInput{Identifier: "missing"}) - assert.Contains(t, buf.String(), "not found") + err := p.Delete(context.Background(), ProfilesDeleteInput{Identifier: "missing"}) + assert.Error(t, err) + assert.Empty(t, buf.String()) } func TestProfilesDelete_SkipConfirm(t *testing.T) { diff --git a/cmd/proxies/delete.go b/cmd/proxies/delete.go index 45d6fe96..a6e08cb5 100644 --- a/cmd/proxies/delete.go +++ b/cmd/proxies/delete.go @@ -20,11 +20,7 @@ func (p ProxyCmd) Delete(ctx context.Context, in ProxyDeleteInput) error { // Try to get the proxy details for better confirmation message proxy, err := p.proxies.Get(ctx, in.ID) if err != nil { - // If we can't get the proxy, just use the ID - if !util.IsNotFound(err) { - return util.CleanedUpSdkError{Err: err} - } - proxy = nil + return util.CleanedUpSdkError{Err: err} } var confirmMsg string @@ -48,10 +44,6 @@ func (p ProxyCmd) Delete(ctx context.Context, in ProxyDeleteInput) error { err := p.proxies.Delete(ctx, in.ID) if err != nil { - if util.IsNotFound(err) { - pterm.Warning.Printf("Proxy '%s' not found\n", in.ID) - return nil - } return util.CleanedUpSdkError{Err: err} } diff --git a/cmd/proxies/delete_test.go b/cmd/proxies/delete_test.go index 8edce639..df78f216 100644 --- a/cmd/proxies/delete_test.go +++ b/cmd/proxies/delete_test.go @@ -3,12 +3,18 @@ package proxies import ( "context" "errors" + "fmt" + "io" "net/http" + "net/http/httptest" "testing" + "github.com/kernel/cli/pkg/interactive" + "github.com/kernel/cli/pkg/util" "github.com/kernel/kernel-go-sdk" "github.com/kernel/kernel-go-sdk/option" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestProxyDelete_SkipConfirm_Success(t *testing.T) { @@ -49,10 +55,46 @@ func TestProxyDelete_SkipConfirm_NotFound(t *testing.T) { SkipConfirm: true, }) - assert.NoError(t, err) // Not found returns nil - output := buf.String() + assert.Error(t, err) + assert.NotContains(t, buf.String(), "Successfully deleted") +} - assert.Contains(t, output, "Proxy 'not-found' not found") +func TestProxyDeleteAPIErrors(t *testing.T) { + for _, skip := range []bool{false, true} { + for _, response := range []struct { + status int + code string + }{ + {404, "project_not_found"}, {404, "not_found"}, {403, "forbidden"}, {409, "conflict"}, {500, "internal_error"}, + } { + t.Run(fmt.Sprintf("skip=%t/%s", skip, response.code), func(t *testing.T) { + buf := captureOutput(t) + calls := 0 + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + calls++ + assert.Equal(t, "selected-project", r.Header.Get("X-Kernel-Project")) + if skip { + assert.Equal(t, http.MethodDelete, r.Method) + } else { + assert.Equal(t, http.MethodGet, r.Method) + } + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(response.status) + _, _ = io.WriteString(w, fmt.Sprintf(`{"code":%q,"message":"API error details"}`, response.code)) + })) + defer server.Close() + client := kernel.NewClient(option.WithBaseURL(server.URL), option.WithAPIKey("test"), option.WithProject("selected-project"), option.WithMaxRetries(0)) + p := ProxyCmd{proxies: &client.Proxies, prompter: interactive.NewPrompterWithTerminal(true)} + err := p.Delete(context.Background(), ProxyDeleteInput{ID: "resource", SkipConfirm: skip}) + var apiErr *kernel.Error + require.ErrorAs(t, err, &apiErr) + assert.Equal(t, response.status, apiErr.StatusCode) + assert.Equal(t, response.code+": API error details", util.CleanedUpSdkError{Err: err}.Error()) + assert.NotContains(t, buf.String(), "Successfully deleted") + assert.Equal(t, 1, calls) + }) + } + } } func TestProxyDelete_SkipConfirm_APIError(t *testing.T) { diff --git a/cmd/telemetry_destinations.go b/cmd/telemetry_destinations.go index f8b4d417..8b66c50c 100644 --- a/cmd/telemetry_destinations.go +++ b/cmd/telemetry_destinations.go @@ -305,10 +305,6 @@ func (c TelemetryDestinationsCmd) Delete(ctx context.Context, in TelemetryDestin } if err := c.destinations.Delete(ctx, in.Identifier); err != nil { - if util.IsNotFound(err) { - pterm.Info.Printf("OTLP destination '%s' not found\n", in.Identifier) - return nil - } // A 409 here means the destination is still referenced; the API's own // message names what still holds it, so it is surfaced as-is. return util.CleanedUpSdkError{Err: err} diff --git a/cmd/telemetry_destinations_test.go b/cmd/telemetry_destinations_test.go index 47e41020..59af1e31 100644 --- a/cmd/telemetry_destinations_test.go +++ b/cmd/telemetry_destinations_test.go @@ -260,8 +260,8 @@ func TestTelemetryDestinationsDelete_NotFound(t *testing.T) { return &kernel.Error{StatusCode: http.StatusNotFound} }} c := TelemetryDestinationsCmd{destinations: fake} - require.NoError(t, c.Delete(context.Background(), TelemetryDestinationsDeleteInput{Identifier: "nope", SkipConfirm: true})) - assert.Contains(t, buf.String(), "not found") + require.Error(t, c.Delete(context.Background(), TelemetryDestinationsDeleteInput{Identifier: "nope", SkipConfirm: true})) + assert.Empty(t, buf.String()) } func TestTelemetryDestinationsDelete_NonInteractiveWithoutYes(t *testing.T) {