[CLI-1828] Show detailed error when attempting to delete a cluster with active connectors (#1363)

sgagniere · Brian Strauch · web-flow · commit 9e818032f3af · 2022-07-20T14:56:32.000-07:00
* Added detailed error response to CatchKafkaNotFoundError even when the HTTP status code is 403. Also added new suggestion to check for active connectors.

* Modified corresponding golden files.

* Missed a line in one of the golden files in previous commit

* Modified the last suggestion to apply specifically to deletion

* Modified CatchKafkaNotFoundError to only suggest checking for active connectors in the case of a delete request

* Compact suggestion choosing

Co-authored-by: Brian Strauch &lt;bstrauch@confluent.io&gt;

* Cleaned up suggestion message

* Added back missing newline

* Updated resourceNotFoundErrMsg to say 'resource not found' instead of 'Forbidden'

* Updated more functions in catcher.go to also return v2 error detail in case of a 403 response, and added an integration test to test CatchServiceAccountNotFoundError

* Added integration test for deleting an environment that doesn't exist

* un-capitalize service account error

Co-authored-by: Brian Strauch &lt;bstrauch@confluent.io&gt;

* un-capitalize environment error

Co-authored-by: Brian Strauch &lt;bstrauch@confluent.io&gt;

* Removed redundant line and updated some golden files

* Error message code formatting

Co-authored-by: Brian Strauch &lt;bstrauch@confluent.io&gt;

* Update internal/pkg/errors/catcher.go

* fix integration tests

Co-authored-by: Brian Strauch &lt;bstrauch@confluent.io&gt;
diff --git a/internal/pkg/errors/catcher.go b/internal/pkg/errors/catcher.go
@@ -183,7 +183,7 @@ func CatchEnvironmentNotFoundError(err error, r *http.Response) error {
 	}
 
 	if r != nil && r.StatusCode == http.StatusForbidden {
-		return NewWrapErrorWithSuggestions(err, "Environment not found or access forbidden", EnvNotFoundSuggestions)
+		return NewWrapErrorWithSuggestions(CatchV2ErrorDetailWithResponse(err, r), "environment not found or access forbidden", EnvNotFoundSuggestions)
 	}
 
 	return CatchV2ErrorDetailWithResponse(err, r)
@@ -198,7 +198,11 @@ func CatchKafkaNotFoundError(err error, clusterId string, r *http.Response) erro
 	}
 
 	if r != nil && r.StatusCode == http.StatusForbidden {
-		return NewWrapErrorWithSuggestions(err, "Kafka cluster not found or access forbidden", ChooseRightEnvironmentSuggestions)
+		suggestions := ChooseRightEnvironmentSuggestions
+		if r.Request.Method == http.MethodDelete {
+			suggestions = KafkaClusterDeletingSuggestions
+		}
+		return NewWrapErrorWithSuggestions(CatchV2ErrorDetailWithResponse(err, r), "Kafka cluster not found or access forbidden", suggestions)
 	}
 
 	return CatchV2ErrorDetailWithResponse(err, r)
@@ -223,7 +227,7 @@ func CatchClusterConfigurationNotValidError(err error, r *http.Response) error {
 
 func CatchApiKeyForbiddenAccessError(err error, operation string, r *http.Response) error {
 	if r != nil && r.StatusCode == http.StatusForbidden || strings.Contains(err.Error(), "Unknown API key") {
-		return NewWrapErrorWithSuggestions(err, fmt.Sprintf("error %s api key", operation), APIKeyNotFoundSuggestions)
+		return NewWrapErrorWithSuggestions(CatchV2ErrorDetailWithResponse(err, r), fmt.Sprintf("error %s API key", operation), APIKeyNotFoundSuggestions)
 	}
 	return CatchV2ErrorDetailWithResponse(err, r)
 }
@@ -268,7 +272,7 @@ func CatchServiceAccountNotFoundError(err error, r *http.Response, serviceAccoun
 			errorMsg := fmt.Sprintf(ServiceAccountNotFoundErrorMsg, serviceAccountId)
 			return NewErrorWithSuggestions(errorMsg, ServiceAccountNotFoundSuggestions)
 		case http.StatusForbidden:
-			return NewWrapErrorWithSuggestions(err, "Service account not found or access forbidden", ServiceAccountNotFoundSuggestions)
+			return NewWrapErrorWithSuggestions(CatchV2ErrorDetailWithResponse(err, r), "service account not found or access forbidden", ServiceAccountNotFoundSuggestions)
 		}
 	}
 
diff --git a/internal/pkg/errors/error_message.go b/internal/pkg/errors/error_message.go
@@ -153,6 +153,8 @@ const (
 	KafkaClusterExpandingErrorMsg                 = "Your cluster is expanding. Please wait for that operation to complete before updating again."
 	KafkaClusterShrinkingErrorMsg                 = "Your cluster is shrinking. Please wait for that operation to complete before updating again."
 	KafkaClusterDeletingErrorMsg                  = "Your cluster is in the process of being deleted. Cannot initiate cluster resize."
+	KafkaClusterDeletingSuggestions               = ChooseRightEnvironmentSuggestions + "\n" +
+		"Ensure the cluster is not associated with any active Connect clusters."
 	ChooseRightEnvironmentSuggestions             = "Ensure the cluster ID you entered is valid.\n" +
 		"Ensure the cluster you are specifying belongs to the currently selected environment with `confluent kafka cluster list`, `confluent environment list`, and `confluent environment use`."
 	UnknownTopicErrorMsg      = `unknown topic "%s"`
diff --git a/test/environment_test.go b/test/environment_test.go
@@ -15,6 +15,7 @@ func (s *CLITestSuite) TestEnvironment() {
 		{args: "environment create saucayyy -o json", fixture: "environment/9.golden"},
 		{args: "environment create saucayyy -o yaml", fixture: "environment/10.golden"},
 		{args: "environment delete not-595", fixture: "environment/11.golden"},
+		{args: "environment delete env-dne", fixture: "environment/12.golden", wantErrCode: 1},
 	}
 
 	resetConfiguration(s.T())
diff --git a/test/fixtures/output/api-key/47.golden b/test/fixtures/output/api-key/47.golden
@@ -1,4 +1,4 @@
-Error: error getting api key: Unknown API key UNKNOWN
+Error: error getting API key: resource not found: Unknown API key UNKNOWN
 
 Suggestions:
     Ensure the API key exists and has not been deleted, or create a new API key via `confluent api-key create`.
diff --git a/test/fixtures/output/api-key/50.golden b/test/fixtures/output/api-key/50.golden
@@ -1,4 +1,4 @@
-Error: unable to set active API key: Forbidden: Unknown API key UNKNOWN
+Error: unable to set active API key: resource not found: Unknown API key UNKNOWN
 
 Suggestions:
     If you did not create this API key with the CLI or created it on another computer, you must first store the API key and secret locally with `confluent api-key store UNKNOWN <secret>`.
diff --git a/test/fixtures/output/environment/12.golden b/test/fixtures/output/environment/12.golden
@@ -0,0 +1,4 @@
+Error: environment not found or access forbidden: 403 Forbidden
+
+Suggestions:
+    List available environments with `confluent environment list`.
diff --git a/test/fixtures/output/iam/service-account/service-account-not-found.golden b/test/fixtures/output/iam/service-account/service-account-not-found.golden
@@ -0,0 +1,4 @@
+Error: service account not found or access forbidden: resource not found: 403 Forbidden
+
+Suggestions:
+    List service accounts with `confluent service-account list`.
diff --git a/test/fixtures/output/iam/user/delete-dne.golden b/test/fixtures/output/iam/user/delete-dne.golden
@@ -1 +1 @@
-Error: failed to delete user "u-1": Forbidden: 403 Forbidden
+Error: failed to delete user "u-1": resource not found: 403 Forbidden
diff --git a/test/fixtures/output/iam/user/update-dne.golden b/test/fixtures/output/iam/user/update-dne.golden
@@ -1 +1 @@
-Error: failed to update user "u-1": Forbidden: 403 Forbidden
+Error: failed to update user "u-1": resource not found: 403 Forbidden
diff --git a/test/fixtures/output/kafka/kafka-delete-unknown-error.golden b/test/fixtures/output/kafka/kafka-delete-unknown-error.golden
@@ -1,5 +1,6 @@
-Error: Kafka cluster not found or access forbidden: 403 Forbidden
+Error: Kafka cluster not found or access forbidden: resource not found: 403 Forbidden
 
 Suggestions:
     Ensure the cluster ID you entered is valid.
     Ensure the cluster you are specifying belongs to the currently selected environment with `confluent kafka cluster list`, `confluent environment list`, and `confluent environment use`.
+    Ensure the cluster is not associated with any active Connect clusters.
diff --git a/test/fixtures/output/login/delete-unknown-key.golden b/test/fixtures/output/login/delete-unknown-key.golden
@@ -1,4 +1,4 @@
-Error: error getting api key: Unknown API key UNKNOWN
+Error: error getting API key: resource not found: Unknown API key UNKNOWN
 
 Suggestions:
     Ensure the API key exists and has not been deleted, or create a new API key via `confluent api-key create`.
diff --git a/test/iam_test.go b/test/iam_test.go
@@ -176,6 +176,7 @@ func (s *CLITestSuite) TestIAMServiceAccount() {
 		{args: "iam service-account describe sa-12345 -o json", fixture: "iam/service-account/describe-json.golden"},
 		{args: "iam service-account describe sa-12345 -o yaml", fixture: "iam/service-account/describe-yaml.golden"},
 		{args: "iam service-account describe sa-12345", fixture: "iam/service-account/describe.golden"},
+		{args: "iam service-account describe sa-6789", fixture: "iam/service-account/service-account-not-found.golden", wantErrCode: 1},
 		{args: "iam service-account update sa-12345 --description new-description", fixture: "iam/service-account/update.golden"},
 		{args: "iam service-account update sa-12345 --description new-description-2", fixture: "iam/service-account/update-2.golden"},
 		{args: "iam service-account delete sa-12345", fixture: "iam/service-account/delete.golden"},
diff --git a/test/test-server/iam_handlers.go b/test/test-server/iam_handlers.go
@@ -173,13 +173,19 @@ func handleIamServiceAccount(t *testing.T) http.HandlerFunc {
 		id := mux.Vars(r)["id"]
 		switch r.Method {
 		case http.MethodGet:
-			serviceAccount := iamv2.IamV2ServiceAccount{
-				Id:          iamv2.PtrString(serviceAccountResourceID),
-				DisplayName: iamv2.PtrString("service_account"),
-				Description: iamv2.PtrString("at your service."),
+			switch id {
+			case "sa-6789":
+				err := writeResourceNotFoundError(w)
+				require.NoError(t, err)
+			default:
+				serviceAccount := iamv2.IamV2ServiceAccount{
+					Id:          iamv2.PtrString(serviceAccountResourceID),
+					DisplayName: iamv2.PtrString("service_account"),
+					Description: iamv2.PtrString("at your service."),
+				}
+				err := json.NewEncoder(w).Encode(serviceAccount)
+				require.NoError(t, err)
 			}
-			err := json.NewEncoder(w).Encode(serviceAccount)
-			require.NoError(t, err)
 		case http.MethodPatch:
 			var req iamv2.IamV2ServiceAccount
 			err := json.NewDecoder(r.Body).Decode(&req)
diff --git a/test/test-server/org_handlers.go b/test/test-server/org_handlers.go
@@ -36,7 +36,7 @@ func handleOrgEnvironment(t *testing.T) http.HandlerFunc {
 			}
 		} else {
 			// env not found
-			w.WriteHeader(http.StatusNotFound)
+			w.WriteHeader(http.StatusForbidden)
 		}
 	}
 }
diff --git a/test/test-server/utils.go b/test/test-server/utils.go
@@ -18,7 +18,7 @@ import (
 
 var (
 	serviceAccountInvalidErrMsg = `{"errors":[{"status":"403","detail":"service account is not valid"}]}`
-	resourceNotFoundErrMsg      = `{"errors":[{"status":"403","detail":"Forbidden"}]}`
+	resourceNotFoundErrMsg      = `{"errors":[{"status":"403","detail":"resource not found"}]}`
 	v1ResourceNotFoundErrMsg    = `{"error":{"code":403,"message":"resource not found","nested_errors":{},"details":[],"stack":null},"cluster":null}`
 )
 

Original file line number	Diff line number	Diff line change
`@@ -183,7 +183,7 @@ func CatchEnvironmentNotFoundError(err error, r *http.Response) error {`
`183`	`183`	`}`
`184`	`184`
`185`	`185`	`if r != nil && r.StatusCode == http.StatusForbidden {`
`186`		`- return NewWrapErrorWithSuggestions(err, "Environment not found or access forbidden", EnvNotFoundSuggestions)`
	`186`	`+ return NewWrapErrorWithSuggestions(CatchV2ErrorDetailWithResponse(err, r), "environment not found or access forbidden", EnvNotFoundSuggestions)`
`187`	`187`	`}`
`188`	`188`
`189`	`189`	`return CatchV2ErrorDetailWithResponse(err, r)`
`@@ -198,7 +198,11 @@ func CatchKafkaNotFoundError(err error, clusterId string, r *http.Response) erro`
`198`	`198`	`}`
`199`	`199`
`200`	`200`	`if r != nil && r.StatusCode == http.StatusForbidden {`
`201`		`- return NewWrapErrorWithSuggestions(err, "Kafka cluster not found or access forbidden", ChooseRightEnvironmentSuggestions)`
	`201`	`+ suggestions := ChooseRightEnvironmentSuggestions`
	`202`	`+ if r.Request.Method == http.MethodDelete {`
	`203`	`+ suggestions = KafkaClusterDeletingSuggestions`
	`204`	`+ }`
	`205`	`+ return NewWrapErrorWithSuggestions(CatchV2ErrorDetailWithResponse(err, r), "Kafka cluster not found or access forbidden", suggestions)`
`202`	`206`	`}`
`203`	`207`
`204`	`208`	`return CatchV2ErrorDetailWithResponse(err, r)`
`@@ -223,7 +227,7 @@ func CatchClusterConfigurationNotValidError(err error, r *http.Response) error {`
`223`	`227`
`224`	`228`	`func CatchApiKeyForbiddenAccessError(err error, operation string, r *http.Response) error {`
`225`	`229`	`if r != nil && r.StatusCode == http.StatusForbidden \|\| strings.Contains(err.Error(), "Unknown API key") {`
`226`		`- return NewWrapErrorWithSuggestions(err, fmt.Sprintf("error %s api key", operation), APIKeyNotFoundSuggestions)`
	`230`	`+ return NewWrapErrorWithSuggestions(CatchV2ErrorDetailWithResponse(err, r), fmt.Sprintf("error %s API key", operation), APIKeyNotFoundSuggestions)`
`227`	`231`	`}`
`228`	`232`	`return CatchV2ErrorDetailWithResponse(err, r)`
`229`	`233`	`}`
`@@ -268,7 +272,7 @@ func CatchServiceAccountNotFoundError(err error, r *http.Response, serviceAccoun`
`268`	`272`	`errorMsg := fmt.Sprintf(ServiceAccountNotFoundErrorMsg, serviceAccountId)`
`269`	`273`	`return NewErrorWithSuggestions(errorMsg, ServiceAccountNotFoundSuggestions)`
`270`	`274`	`case http.StatusForbidden:`
`271`		`- return NewWrapErrorWithSuggestions(err, "Service account not found or access forbidden", ServiceAccountNotFoundSuggestions)`
	`275`	`+ return NewWrapErrorWithSuggestions(CatchV2ErrorDetailWithResponse(err, r), "service account not found or access forbidden", ServiceAccountNotFoundSuggestions)`
`272`	`276`	`}`
`273`	`277`	`}`
`274`	`278`
Original file line number	Diff line number	Diff line change
`@@ -15,6 +15,7 @@ func (s *CLITestSuite) TestEnvironment() {`
`15`	`15`	`{args: "environment create saucayyy -o json", fixture: "environment/9.golden"},`
`16`	`16`	`{args: "environment create saucayyy -o yaml", fixture: "environment/10.golden"},`
`17`	`17`	`{args: "environment delete not-595", fixture: "environment/11.golden"},`
	`18`	`+ {args: "environment delete env-dne", fixture: "environment/12.golden", wantErrCode: 1},`
`18`	`19`	`}`
`19`	`20`
`20`	`21`	`resetConfiguration(s.T())`
Original file line number	Diff line number	Diff line change
`@@ -1 +1 @@`
`1`		`-Error: failed to delete user "u-1": Forbidden: 403 Forbidden`
	`1`	`+Error: failed to delete user "u-1": resource not found: 403 Forbidden`
Original file line number	Diff line number	Diff line change
`@@ -1 +1 @@`
`1`		`-Error: failed to update user "u-1": Forbidden: 403 Forbidden`
	`1`	`+Error: failed to update user "u-1": resource not found: 403 Forbidden`