feat(ws): backend api to create wsk with YAML

[asaadbalum]

This PR adds the POST /api/v1/workspacekinds endpoint for creating a WorkspaceKind from a raw YAML upload.

The implementation adds a Create() method to the repository and the CreateWorkspaceKindHandler to process the request. The corresponding integration tests verify the success path using a static file payload and also cover key failure cases.

closes: #278

[andyatmiami]

Tested happy path via the Swagger UI generated from this PR and observed a WorkspaceKind was successfully created.

➜ manifests/ $ kubectl get workspacekinds/rstudio
NAME      WORKSPACES   DEPRECATED   HIDDEN
rstudio   0            false        false

[andyatmiami]

Error propagation in the case of the workspace kind already existing is not working... when I "spam" the Swagger UI to create the same workspace kind repeatedly - its returning a 500 error - which is not expected behavior

You can refer to the create workspace function (amongst other places) to see how we properly identify an error from a repository function and ensure its encoded appropriately for return to client here:

• https://github.com/kubeflow/notebooks/blob/notebooks-v2/workspaces/backend/api/workspaces_handler.go#L240-L252

[asaadbalum]

I've updated the CreateWorkspaceKindHandler to specifically check for the ErrWorkspaceKindAlreadyExists from the repository. It now correctly returns a 409 Conflict in this case.

[andyatmiami]

I'm not sure we want to use failedValidationResponse here... as it returns a 422 error..

Here is a case where I supplied an obviously invalid request body and got a 422 back:

(1) 422 is NOT listed as a possible HTTP response status code in our Swagger annotations
(2) I believe a 400 is a better/more natural status code to begin with here (but maybe I am old/out of touch/wrong?)

[asaadbalum]

400 is more appropriate here. I've changed the handler to use badRequestResponse for the missing name validation, so it now correctly returns a 400 Bad Request.

[thesuperzapper]

I disagree, and think the validation 422 is better because we include specific field references with where the error occurred.

[andyatmiami]

/lgtm

Thanks for promptly addressing all feedback.

I have verified the following scenarios:

• [happy path] can create workspacekind from YAML via API
• [error path] invalid content type return 415
• [error path] providing non-yaml input returns sensible error
• [error path] providing invalid workspacekind manifest returns sensible error
• [error path] providing wrong kind of (otherwise valid) manifest provides sensible error
• [error path] trying to create a resource whose name already exists returns sensible error

[thesuperzapper]

@asaadbalum thanks for this, I worked with @andyatmiami to push 50d6f9b which factors the logic out a bit and fixes some of the tests, I have also reverted to using application/yaml as the media type, as this is what upstream Kubernetes uses.

Feel free to review and indicate when you are ready to merge.

[andyatmiami]

/lgtm

re-validated same test cases I did previously and can confirm behavior is as intended.

[thesuperzapper]

/approve

[google-oss-prow]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: thesuperzapper

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [thesuperzapper]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[thesuperzapper]

/ok-to-test