feat(ws): Implement DELETE endpoint for WorkspaceKind

andyatmiami · andyatmiami · commit 49f77ed1132c · 2025-07-11T16:14:59.000-04:00
- Added the ability to delete a specific WorkspaceKind via a new DELETE API endpoint.
    - `DELETE /workspacekinds/{name}`
- Updated the README to include the new endpoint details and added corresponding tests to ensure proper functionality and error handling for various scenarios, including validation and authorization checks.
- Swagger files regenerated

The implementation is more or less a "carbon copy" of how it is implemented for Workspaces _with the exception_ of the need to handle the `Conflict` that can be returned by the admission webhook.  While there is basic handling of this condition in the PR - I'm not necessarily happy with it.

First off, there is an error message prefix inserted by kubernetes itself:

```
curl -X 'DELETE'   'http://localhost:4001/api/v1/workspacekinds/jupyterlab'   -H 'accept: application/json'
{
	"error": {
		"code": "409",
		"message": "admission webhook \"vworkspacekind.kb.io\" denied the request: Operation cannot be fulfilled on WorkspaceKind.kubeflow.org \"jupyterlab\": WorkspaceKind is used by 2 workspace(s)"
	}
}
```

The `admission webhook \"vworkspacekind.kb.io\" denied the request:` is appended on the error message returned from `workspacekind_webhook.go` and not easily able to be omitted (without falling back to brittle string parsing).  I personally don't think that is appropriate details to put in an error message that could be surfaced in the UI.  And while a `Conflict` error today is the only surfaced from the workspacekind webhook code if a workspace is using it - I'm not confident that condition would hold in the future... and there is no other information I can reliably act on to distinguish this particular cause (to simply provide a more succinct error message in `repo.go`)

Furthermore, our testing framework does not currently run with webhooks - so I have deliberately **NOT** added a test case for this behavior.

Signed-off-by: Andy Stoneberg &lt;astonebe@redhat.com&gt;
diff --git a/workspaces/backend/README.md b/workspaces/backend/README.md
@@ -27,7 +27,7 @@ make run
 If you want to use a different port:
 
 ```shell
-make run PORT=8000 
+make run PORT=8000
 ```
 
 ### Endpoints
@@ -47,6 +47,7 @@ make run PORT=8000
 | GET /api/v1/workspacekinds                   | workspacekinds_handler | Get all WorkspaceKind                   |
 | POST /api/v1/workspacekinds                  | TBD                    | Create a WorkspaceKind                  |
 | GET /api/v1/workspacekinds/{name}            | workspacekinds_handler | Get a WorkspaceKind entity              |
+| DELETE /api/v1/workspacekinds/{name}         | workspacekinds_handler | Delete a WorkspaceKind entity           |
 | PATCH /api/v1/workspacekinds/{name}          | TBD                    | Patch a WorkspaceKind entity            |
 | PUT /api/v1/workspacekinds/{name}            | TBD                    | Update a WorkspaceKind entity           |
 | DELETE /api/v1/workspacekinds/{name}         | TBD                    | Delete a WorkspaceKind entity           |
@@ -148,3 +149,10 @@ Get a WorkspaceKind:
 # GET /api/v1/workspacekinds/{name}
 curl -i localhost:4000/api/v1/workspacekinds/jupyterlab
 ```
+
+Delete a WorkspaceKind:
+
+```shell
+# DELETE /api/v1/workspaces/{namespace}/{name}
+curl -X DELETE localhost:4000/api/v1/workspacekinds/jupyterlab
+```
diff --git a/workspaces/backend/api/app.go b/workspaces/backend/api/app.go
@@ -121,6 +121,7 @@ func (a *App) Routes() http.Handler {
 	router.GET(AllWorkspaceKindsPath, a.GetWorkspaceKindsHandler)
 	router.GET(WorkspaceKindsByNamePath, a.GetWorkspaceKindHandler)
 	router.POST(AllWorkspaceKindsPath, a.CreateWorkspaceKindHandler)
+	router.DELETE(WorkspaceKindsByNamePath, a.DeleteWorkspaceKindHandler)
 
 	// swagger
 	router.GET(SwaggerPath, a.GetSwaggerHandler)
diff --git a/workspaces/backend/api/workspacekinds_handler.go b/workspaces/backend/api/workspacekinds_handler.go
@@ -131,6 +131,66 @@ func (a *App) GetWorkspaceKindsHandler(w http.ResponseWriter, r *http.Request, _
 	a.dataResponse(w, r, responseEnvelope)
 }
 
+// DeleteWorkspaceKindHandler deletes a specific workspace kind by name.
+//
+//	@Summary		Delete workspace kind
+//	@Description	Deletes a specific workspace kind identified by its name.
+//	@Tags			workspacekinds
+//	@Accept			json
+//	@Produce		json
+//	@Param			name	path		string			true	"Name of the workspace kind"	extensions(x-example=jupyterlab)
+//	@Success		204		{object}	nil				"Workspace kind deleted successfully"
+//	@Failure		400		{object}	ErrorEnvelope	"Bad Request. Invalid workspace kind name format."
+//	@Failure		401		{object}	ErrorEnvelope	"Unauthorized. Authentication is required."
+//	@Failure		403		{object}	ErrorEnvelope	"Forbidden. User does not have permission to delete the workspace kind."
+//	@Failure		404		{object}	ErrorEnvelope	"Not Found. Workspace kind does not exist."
+//	@Failure		409		{object}	ErrorEnvelope	"Conflict. Workspace kind is in use by one or more workspaces."
+//	@Failure		500		{object}	ErrorEnvelope	"Internal server error. An unexpected error occurred on the server."
+//	@Router			/workspacekinds/{name} [delete]
+func (a *App) DeleteWorkspaceKindHandler(w http.ResponseWriter, r *http.Request, ps httprouter.Params) {
+	name := ps.ByName(ResourceNamePathParam)
+
+	// validate path parameters
+	var valErrs field.ErrorList
+	valErrs = append(valErrs, helper.ValidateFieldIsDNS1123Subdomain(field.NewPath(ResourceNamePathParam), name)...)
+	if len(valErrs) > 0 {
+		a.failedValidationResponse(w, r, errMsgPathParamsInvalid, valErrs, nil)
+		return
+	}
+
+	// =========================== AUTH ===========================
+	authPolicies := []*auth.ResourcePolicy{
+		auth.NewResourcePolicy(
+			auth.ResourceVerbDelete,
+			&kubefloworgv1beta1.WorkspaceKind{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: name,
+				},
+			},
+		),
+	}
+	if success := a.requireAuth(w, r, authPolicies); !success {
+		return
+	}
+	// ============================================================
+
+	err := a.repositories.WorkspaceKind.DeleteWorkspaceKind(r.Context(), name)
+	if err != nil {
+		if errors.Is(err, repository.ErrWorkspaceKindNotFound) {
+			a.notFoundResponse(w, r)
+			return
+		}
+		if apierrors.IsConflict(err) {
+			a.conflictResponse(w, r, err)
+			return
+		}
+		a.serverErrorResponse(w, r, err)
+		return
+	}
+
+	a.deletedResponse(w, r)
+}
+
 // CreateWorkspaceKindHandler creates a new workspace kind.
 //
 //	@Summary		Create workspace kind
diff --git a/workspaces/backend/api/workspacekinds_handler_test.go b/workspaces/backend/api/workspacekinds_handler_test.go
@@ -30,6 +30,7 @@ import (
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 	corev1 "k8s.io/api/core/v1"
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/apimachinery/pkg/util/validation/field"
@@ -507,4 +508,175 @@ metadata:
 			))
 		})
 	})
+
+	// NOTE: these tests create and delete resources on the cluster, so cannot be run in parallel.
+	//       therefore, we run them using the `Serial` Ginkgo decorator.
+	Context("when deleting a WorkspaceKind", Serial, func() {
+
+		var (
+			workspaceKindName     string
+			workspaceKindKey      types.NamespacedName
+			workspaceKindToDelete *kubefloworgv1beta1.WorkspaceKind
+		)
+
+		BeforeEach(func() {
+			uniqueName := fmt.Sprintf("wsk-delete-test-%d", GinkgoRandomSeed())
+			workspaceKindName = fmt.Sprintf("workspacekind-%s", uniqueName)
+			workspaceKindKey = types.NamespacedName{Name: workspaceKindName}
+
+			By("creating the WorkspaceKind to delete")
+			workspaceKindToDelete = NewExampleWorkspaceKind(workspaceKindName)
+			Expect(k8sClient.Create(ctx, workspaceKindToDelete)).To(Succeed())
+		})
+
+		AfterEach(func() {
+			By("cleaning up the WorkspaceKind")
+			workspaceKind := &kubefloworgv1beta1.WorkspaceKind{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: workspaceKindName,
+				},
+			}
+			_ = k8sClient.Delete(ctx, workspaceKind)
+		})
+
+		It("should successfully delete an existing WorkspaceKind", func() {
+			By("creating the HTTP request to delete the WorkspaceKind")
+			path := strings.Replace(WorkspaceKindsByNamePath, ":"+ResourceNamePathParam, workspaceKindName, 1)
+			req, err := http.NewRequest(http.MethodDelete, path, http.NoBody)
+			Expect(err).NotTo(HaveOccurred())
+
+			By("setting the auth headers")
+			req.Header.Set(userIdHeader, adminUser)
+
+			By("executing DeleteWorkspaceKindHandler")
+			rr := httptest.NewRecorder()
+			ps := httprouter.Params{
+				httprouter.Param{
+					Key:   ResourceNamePathParam,
+					Value: workspaceKindName,
+				},
+			}
+			a.DeleteWorkspaceKindHandler(rr, req, ps)
+			rs := rr.Result()
+			defer rs.Body.Close()
+
+			By("verifying the HTTP response status code")
+			Expect(rs.StatusCode).To(Equal(http.StatusNoContent), descUnexpectedHTTPStatus, rr.Body.String())
+
+			By("ensuring the WorkspaceKind has been deleted")
+			deletedWorkspaceKind := &kubefloworgv1beta1.WorkspaceKind{}
+			err = k8sClient.Get(ctx, workspaceKindKey, deletedWorkspaceKind)
+			Expect(err).To(HaveOccurred())
+			Expect(apierrors.IsNotFound(err)).To(BeTrue())
+		})
+
+		It("should return 404 when trying to delete a non-existent WorkspaceKind", func() {
+			nonExistentName := "non-existent-workspacekind"
+
+			By("creating the HTTP request to delete a non-existent WorkspaceKind")
+			path := strings.Replace(WorkspaceKindsByNamePath, ":"+ResourceNamePathParam, nonExistentName, 1)
+			req, err := http.NewRequest(http.MethodDelete, path, http.NoBody)
+			Expect(err).NotTo(HaveOccurred())
+
+			By("setting the auth headers")
+			req.Header.Set(userIdHeader, adminUser)
+
+			By("executing DeleteWorkspaceKindHandler")
+			rr := httptest.NewRecorder()
+			ps := httprouter.Params{
+				httprouter.Param{
+					Key:   ResourceNamePathParam,
+					Value: nonExistentName,
+				},
+			}
+			a.DeleteWorkspaceKindHandler(rr, req, ps)
+			rs := rr.Result()
+			defer rs.Body.Close()
+
+			By("verifying the HTTP response status code")
+			Expect(rs.StatusCode).To(Equal(http.StatusNotFound), descUnexpectedHTTPStatus, rr.Body.String())
+		})
+
+		It("should return 422 when workspace kind name has invalid format", func() {
+			invalidName := "InvalidNameWithUppercase"
+
+			By("creating the HTTP request with invalid workspace kind name")
+			path := strings.Replace(WorkspaceKindsByNamePath, ":"+ResourceNamePathParam, invalidName, 1)
+			req, err := http.NewRequest(http.MethodDelete, path, http.NoBody)
+			Expect(err).NotTo(HaveOccurred())
+
+			By("setting the auth headers")
+			req.Header.Set(userIdHeader, adminUser)
+
+			By("executing DeleteWorkspaceKindHandler")
+			rr := httptest.NewRecorder()
+			ps := httprouter.Params{
+				httprouter.Param{
+					Key:   ResourceNamePathParam,
+					Value: invalidName,
+				},
+			}
+			a.DeleteWorkspaceKindHandler(rr, req, ps)
+			rs := rr.Result()
+			defer rs.Body.Close()
+
+			By("verifying the HTTP response status code")
+			Expect(rs.StatusCode).To(Equal(http.StatusUnprocessableEntity), descUnexpectedHTTPStatus, rr.Body.String())
+
+			By("decoding the error response")
+			var response ErrorEnvelope
+			err = json.Unmarshal(rr.Body.Bytes(), &response)
+			Expect(err).NotTo(HaveOccurred())
+
+			By("verifying the error message indicates validation failure")
+			Expect(response.Error.Message).To(ContainSubstring("path parameters were invalid"))
+		})
+
+		It("should return 401 when no authentication is provided", func() {
+			By("creating the HTTP request without auth headers")
+			path := strings.Replace(WorkspaceKindsByNamePath, ":"+ResourceNamePathParam, workspaceKindName, 1)
+			req, err := http.NewRequest(http.MethodDelete, path, http.NoBody)
+			Expect(err).NotTo(HaveOccurred())
+
+			By("executing DeleteWorkspaceKindHandler without auth")
+			rr := httptest.NewRecorder()
+			ps := httprouter.Params{
+				httprouter.Param{
+					Key:   ResourceNamePathParam,
+					Value: workspaceKindName,
+				},
+			}
+			a.DeleteWorkspaceKindHandler(rr, req, ps)
+			rs := rr.Result()
+			defer rs.Body.Close()
+
+			By("verifying the HTTP response status code")
+			Expect(rs.StatusCode).To(Equal(http.StatusUnauthorized), descUnexpectedHTTPStatus, rr.Body.String())
+		})
+
+		It("should return 403 when user lacks permission to delete workspace kind", func() {
+			By("creating the HTTP request with non-admin user")
+			path := strings.Replace(WorkspaceKindsByNamePath, ":"+ResourceNamePathParam, workspaceKindName, 1)
+			req, err := http.NewRequest(http.MethodDelete, path, http.NoBody)
+			Expect(err).NotTo(HaveOccurred())
+
+			By("setting the auth headers with non-admin user")
+			req.Header.Set(userIdHeader, "non-admin-user")
+
+			By("executing DeleteWorkspaceKindHandler")
+			rr := httptest.NewRecorder()
+			ps := httprouter.Params{
+				httprouter.Param{
+					Key:   ResourceNamePathParam,
+					Value: workspaceKindName,
+				},
+			}
+			a.DeleteWorkspaceKindHandler(rr, req, ps)
+			rs := rr.Result()
+			defer rs.Body.Close()
+
+			By("verifying the HTTP response status code")
+			Expect(rs.StatusCode).To(Equal(http.StatusForbidden), descUnexpectedHTTPStatus, rr.Body.String())
+		})
+	})
 })
diff --git a/workspaces/backend/internal/repositories/workspacekinds/repo.go b/workspaces/backend/internal/repositories/workspacekinds/repo.go
@@ -25,6 +25,7 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
 	models "github.com/kubeflow/notebooks/workspaces/backend/internal/models/workspacekinds"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
 var ErrWorkspaceKindNotFound = errors.New("workspace kind not found")
@@ -92,3 +93,20 @@ func (r *WorkspaceKindRepository) Create(ctx context.Context, workspaceKind *kub
 
 	return &createdWorkspaceKindModel, nil
 }
+
+func (r *WorkspaceKindRepository) DeleteWorkspaceKind(ctx context.Context, name string) error {
+	workspaceKind := &kubefloworgv1beta1.WorkspaceKind{
+		ObjectMeta: metav1.ObjectMeta{
+			Name: name,
+		},
+	}
+
+	if err := r.client.Delete(ctx, workspaceKind); err != nil {
+		if apierrors.IsNotFound(err) {
+			return ErrWorkspaceKindNotFound
+		}
+		return err
+	}
+
+	return nil
+}
diff --git a/workspaces/backend/openapi/docs.go b/workspaces/backend/openapi/docs.go
@@ -265,6 +265,70 @@ const docTemplate = `{
                         }
                     }
                 }
+            },
+            "delete": {
+                "description": "Deletes a specific workspace kind identified by its name.",
+                "consumes": [
+                    "application/json"
+                ],
+                "produces": [
+                    "application/json"
+                ],
+                "tags": [
+                    "workspacekinds"
+                ],
+                "summary": "Delete workspace kind",
+                "parameters": [
+                    {
+                        "type": "string",
+                        "x-example": "jupyterlab",
+                        "description": "Name of the workspace kind",
+                        "name": "name",
+                        "in": "path",
+                        "required": true
+                    }
+                ],
+                "responses": {
+                    "204": {
+                        "description": "Workspace kind deleted successfully"
+                    },
+                    "400": {
+                        "description": "Bad Request. Invalid workspace kind name format.",
+                        "schema": {
+                            "$ref": "#/definitions/api.ErrorEnvelope"
+                        }
+                    },
+                    "401": {
+                        "description": "Unauthorized. Authentication is required.",
+                        "schema": {
+                            "$ref": "#/definitions/api.ErrorEnvelope"
+                        }
+                    },
+                    "403": {
+                        "description": "Forbidden. User does not have permission to delete the workspace kind.",
+                        "schema": {
+                            "$ref": "#/definitions/api.ErrorEnvelope"
+                        }
+                    },
+                    "404": {
+                        "description": "Not Found. Workspace kind does not exist.",
+                        "schema": {
+                            "$ref": "#/definitions/api.ErrorEnvelope"
+                        }
+                    },
+                    "409": {
+                        "description": "Conflict. Workspace kind is in use by one or more workspaces.",
+                        "schema": {
+                            "$ref": "#/definitions/api.ErrorEnvelope"
+                        }
+                    },
+                    "500": {
+                        "description": "Internal server error. An unexpected error occurred on the server.",
+                        "schema": {
+                            "$ref": "#/definitions/api.ErrorEnvelope"
+                        }
+                    }
+                }
             }
         },
         "/workspaces": {
diff --git a/workspaces/backend/openapi/swagger.json b/workspaces/backend/openapi/swagger.json
