ServiceAccounts: return 404 when deleting a non‑existent service account (#106618)

ServiceAccounts: return `404` when deleting a non‑existent service account

pkg/services/serviceaccounts/api/api.go

@@ -223,6 +223,7 @@ func (api *ServiceAccountsAPI) validateRole(r *org.RoleType, orgRole org.RoleTyp
 // 400: badRequestError
 // 401: unauthorisedError
 // 403: forbiddenError
+// 404: notFoundError
 // 500: internalServerError
 func (api *ServiceAccountsAPI) DeleteServiceAccount(ctx *contextmodel.ReqContext) response.Response {
 	saID, err := strconv.ParseInt(web.Params(ctx.Req)[":serviceAccountId"], 10, 64)
@@ -231,7 +232,7 @@ func (api *ServiceAccountsAPI) DeleteServiceAccount(ctx *contextmodel.ReqContext
 	}
 	err = api.service.DeleteServiceAccount(ctx.Req.Context(), ctx.GetOrgID(), saID)
 	if err != nil {
-		return response.Error(http.StatusInternalServerError, "Service account deletion error", err)
+		return response.ErrOrFallback(http.StatusInternalServerError, "Service account deletion error", err)
 	}
 	return response.Success("Service account deleted")
 }

pkg/services/serviceaccounts/api/api_test.go

@@ -102,6 +102,7 @@ func TestServiceAccountsAPI_DeleteServiceAccount(t *testing.T) {
 		desc         string
 		id           int64
 		permissions  []accesscontrol.Permission
+		expectedErr  error
 		expectedCode int
 	}
 
@@ -118,11 +119,21 @@ func TestServiceAccountsAPI_DeleteServiceAccount(t *testing.T) {
 			permissions:  []accesscontrol.Permission{{Action: serviceaccounts.ActionDelete, Scope: "serviceaccounts:id:1"}},
 			expectedCode: http.StatusForbidden,
 		},
+		{
+			desc:         "should return a 404 error if the service account doesn't exist",
+			id:           1,
+			permissions:  []accesscontrol.Permission{{Action: serviceaccounts.ActionDelete, Scope: "serviceaccounts:id:1"}},
+			expectedErr:  serviceaccounts.ErrServiceAccountNotFound.Errorf("service account with id 1 not found"),
+			expectedCode: http.StatusNotFound,
+		},
 	}
 
 	for _, tt := range tests {
 		t.Run(tt.desc, func(t *testing.T) {
-			server := setupTests(t)
+			server := setupTests(t, func(a *ServiceAccountsAPI) {
+				a.service = &satests.FakeServiceAccountService{ExpectedErr: tt.expectedErr}
+			})
+
 			req := server.NewRequest(http.MethodDelete, fmt.Sprintf("/api/serviceaccounts/%d", tt.id), nil)
 			webtest.RequestWithSignedInUser(req, &user.SignedInUser{OrgID: 1, Permissions: map[int64]map[string][]string{1: accesscontrol.GroupScopesByActionContext(context.Background(), tt.permissions)}})
 			res, err := server.Send(req)

public/api-merged.json

@@ -8877,6 +8877,9 @@
           "403": {
             "$ref": "#/responses/forbiddenError"
           },
+          "404": {
+            "$ref": "#/responses/notFoundError"
+          },
           "500": {
             "$ref": "#/responses/internalServerError"
           }

public/openapi3.json

@@ -23226,6 +23226,9 @@
           "403": {
             "$ref": "#/components/responses/forbiddenError"
           },
+          "404": {
+            "$ref": "#/components/responses/notFoundError"
+          },
           "500": {
             "$ref": "#/components/responses/internalServerError"
           }