kubeflow / kubeflow/notebooks

[TASK] WorkspaceKind `POST`/`PUT` APIs allow frontend to validate YAML payload without persisting changes

Open
#417 7 comments 0 reactions 0 assignees View on GitHub
area/backend area/v2 kind/plan-task
Dominant language
No language data
Stars
84
Forks
149
Avg merge
5d 15h
Merged PRs (30d)
29

Description

🛑 Blocked by:
- #278
- #416

## Description

Initially, the `frontend` will have the user submit raw YAML WorkspaceKind manifests for the `POST` (create) and `PUT` (update) endpoints of the `/workspacekinds` API suite. As that YAML can be quite large (particularly as the number of options for `podConfig` and `imageConfig` grows) - we want users to have a sense of confidence prior to them submitting the payload to apply to the cluster.

Inspired by k8s, we want to add support for a `?dry_run=true|false` query parameter to the following endpoints:
- `POST api/v1/workspacekinds`
- `PUT api/v1/workspacekinds/{name}`

The default value for `dry_run` (in the event it is not specified) is `false`. When `dry_run=false`, it is essentially a no-op and the "normal" behavior to create/update the `WorkspaceKind` proceeds as usual.

When `dry_run=true`, the following constraints apply:
- under no circumstances are changes introduced to k8s as a result of the API call
- a `dry_run` is performed at the kubernetes layer (see: [CreateOptions](https://github.com/kubernetes-sigs/controller-runtime/blob/v0.21.0/pkg/client/options.go#L211) , [UpdateOptions](https://github.com/kubernetes-sigs/controller-runtime/blob/v0.21.0/pkg/client/options.go#L731) )
- if no error(s) occur as a result of the `dry_run`:
- for `POST`, the `application/json` data representation of the `WorkspaceKind` defined in the YAML is returned
- `Content-Type: application/json` header returned on response
- for `PUT`, the `application/json` data representation of the `WorkspaceKind` **persistend on the cluster** is returned
- `Content-Type: application/json` header returned on response
- ℹ while this behavior is at face value misaligned with `POST` behavior, we believe returning the currently persisted object in this case has more _future upside_ to potentially allow UI to emphasize detected changes
- if error(s) occur as a result of the `dry_run`, those errors are returned in the response payload using the standard Error API schema

`?dry_run` query parameter should only be honored if `Content-Type: application/yaml` provided on the request. In the event `dry_run` query parameter is provided, and a valid `Content-Type` that is not equal to Content-Type: application/yaml` is on the request - a `400` HTTP response code should be returned.
- This likely is irrelevant at this point as `Content-Type: application/yaml` is the only supported `Content-Type` (and a request providing any other header value would/should/could first fail with a `415`)

💡 Given the obvious overlap in functionality across `POST` and `PUT`, the decision was made to raise this issue under a single ticket. We can certainly split this work into issues for each endpoint - but then need to be mindful that they are consistent and leveraging shared code upon. merging.

## Acceptance Criteria
- Calling `POST api/v1/workspacekinds?dry_run=false` behaves identically to `POST api/v1/workspacekinds`
- i.e. the `WorkspaceKind` is created (if valid) or returns error from attempting to apply to cluster
- Calling `PUT api/v1/workspacekinds/{name}?dry_run=false` behaves identically to `PUT api/v1/workspacekinds`
- i.e. the `WorkspaceKind` is updated (if valid) or returns error from attempting to apply to cluster
- Calling `POST api/v1/workspacekinds?dry_run=true` on a valid manifest payload returns an `application/json` response following same schema as `GET api/v1/workspacekinds/{name}` when `Content-Type: application/json` header present
- the `application/json` response is a representation of the `WorkspaceKind` as defined in the request payload
- Calling `PUT api/v1/workspacekinds/{name}?dry_run=true` on a valid manifest payload returns an `application/json` response following same schema as `GET api/v1/workspacekinds/{name}` when `Content-Type: application/json` header present
- the `application/json` response is a representation of the `WorkspaceKind` as its currently persisted on the cluster
- Calling `POST api/v1/workspacekinds?dry_run=true` on an invalid manifest payload returns an `application/json` response following Error schema used in Notebooks 2.0
- all errors returned from attempting to `dry_run` via the controller-runtime client should be encoded in response
- Calling `POST api/v1/workspacekinds?dry_run=true` on an invalid manifest payload returns an `application/json` response following Error schema used in Notebooks 2.0
- all errors returned from attempting to `dry_run` via the controller-runtime client should be encoded in response
- Providing any value to the `dry_run` query parameter argument that is not equal to one of (`true`, `false`) should return a `400` error

Contributor guide

Open the contributing guide

Research direction

Start at the POST and PUT /api/v1/workspacekinds endpoint entry points and read the controller-runtime CreateOptions and UpdateOptions documentation linked in the issue. Add strict dry_run parsing, YAML content-type validation, Kubernetes dry-run behavior, and JSON or standard Error API responses; verify the listed acceptance criteria without persisting changes when dry_run=true.

Written by the indexing model from the issue text.

Assessment

Tech stack
kubernetes, yaml
Domain
api, backend, infrastructure
Issue type
Feature
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
45/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.