Skip to content

feat(ws): Implement DELETE endpoint for WorkspaceKind #480

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: notebooks-v2
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 9 additions & 1 deletion workspaces/backend/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ make run
If you want to use a different port:

```shell
make run PORT=8000
make run PORT=8000
```

### Endpoints
Expand All @@ -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 |
Expand Down Expand Up @@ -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/workspacekinds/{name}
curl -X DELETE localhost:4000/api/v1/workspacekinds/jupyterlab
```
1 change: 1 addition & 0 deletions workspaces/backend/api/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
60 changes: 60 additions & 0 deletions workspaces/backend/api/workspacekinds_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
172 changes: 172 additions & 0 deletions workspaces/backend/api/workspacekinds_handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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())
})
})
})
18 changes: 18 additions & 0 deletions workspaces/backend/internal/repositories/workspacekinds/repo.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (

kubefloworgv1beta1 "github.com/kubeflow/notebooks/workspaces/controller/api/v1beta1"
apierrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"sigs.k8s.io/controller-runtime/pkg/client"

models "github.com/kubeflow/notebooks/workspaces/backend/internal/models/workspacekinds"
Expand Down Expand Up @@ -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
}
64 changes: 64 additions & 0 deletions workspaces/backend/openapi/docs.go
Original file line number Diff line number Diff line change
Expand Up @@ -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": {
Expand Down
Loading