Skip to content

Commit 61d200f

Browse files
feat(api): add Kubernetes-layer dry-run validation for WorkspaceKind creation
This update enhances the WorkspaceKind POST API to properly perform dry-run validation at the Kubernetes API layer. When `dry_run=true` is specified, the request now invokes controller-runtime’s Create API with dry-run semantics, allowing validation errors to be surfaced directly from Kubernetes (e.g., schema or webhook errors). Changes include: - Added CreateOptions with DryRunAll for dry-run requests - Unified create logic for both dry-run and normal requests - Improved error propagation and response mapping - Updated tests for dry-run and standard create flows Signed-off-by: Bhakti Narvekar <[email protected]>
1 parent 12c994f commit 61d200f

File tree

4 files changed

+264
-4
lines changed

4 files changed

+264
-4
lines changed

workspaces/backend/api/helpers.go

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -111,3 +111,19 @@ func (a *App) LocationGetWorkspaceKind(name string) string {
111111
path := strings.Replace(WorkspaceKindsByNamePath, ":"+ResourceNamePathParam, name, 1)
112112
return path
113113
}
114+
115+
// GetBooleanQueryParameter reads a query parameter that only accepts "true"/"false".
116+
// If the parameter is absent, returns (false, nil).
117+
// On invalid value, writes a 400 and returns an error.
118+
func (a *App) GetBooleanQueryParameter(w http.ResponseWriter, r *http.Request, paramName string) (bool, error) {
119+
val := r.URL.Query().Get(paramName)
120+
if val == "" {
121+
return false, nil // default if not provided
122+
}
123+
if val != "true" && val != "false" {
124+
err := fmt.Errorf("invalid query parameter '%s' value '%s'. Must be 'true' or 'false'", paramName, val)
125+
a.badRequestResponse(w, r, err)
126+
return false, err
127+
}
128+
return val == "true", nil
129+
}

workspaces/backend/api/workspacekinds_handler.go

Lines changed: 25 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -154,8 +154,24 @@ func (a *App) GetWorkspaceKindsHandler(w http.ResponseWriter, r *http.Request, _
154154
// @Router /workspacekinds [post]
155155
func (a *App) CreateWorkspaceKindHandler(w http.ResponseWriter, r *http.Request, _ httprouter.Params) {
156156

157+
// Parse dry_run query parameter using helper
158+
isDryRun, err := a.GetBooleanQueryParameter(w, r, "dry_run")
159+
if err != nil {
160+
return
161+
}
162+
hasDryRun := r.URL.Query().Get("dry_run") != ""
163+
157164
// validate the Content-Type header
158-
if success := a.ValidateContentType(w, r, MediaTypeYaml); !success {
165+
if !a.ValidateContentType(w, r, MediaTypeYaml) {
166+
if hasDryRun {
167+
// Special case: dry_run provided with wrong media type → 400
168+
a.badRequestResponse(w, r,
169+
fmt.Errorf("dry_run is only supported when Content-Type: %s", MediaTypeYaml))
170+
} else {
171+
// Normal case: wrong media type → 415
172+
a.unsupportedMediaTypeResponse(w, r,
173+
fmt.Errorf("only %s is supported", MediaTypeYaml))
174+
}
159175
return
160176
}
161177

@@ -204,7 +220,7 @@ func (a *App) CreateWorkspaceKindHandler(w http.ResponseWriter, r *http.Request,
204220
}
205221
// ============================================================
206222

207-
createdWorkspaceKind, err := a.repositories.WorkspaceKind.Create(r.Context(), workspaceKind)
223+
createdWorkspaceKind, err := a.repositories.WorkspaceKind.Create(r.Context(), workspaceKind, isDryRun)
208224
if err != nil {
209225
if errors.Is(err, repository.ErrWorkspaceKindAlreadyExists) {
210226
a.conflictResponse(w, r, err)
@@ -219,6 +235,13 @@ func (a *App) CreateWorkspaceKindHandler(w http.ResponseWriter, r *http.Request,
219235
return
220236
}
221237

238+
// Return appropriate response based on dry-run
239+
if isDryRun {
240+
responseEnvelope := &WorkspaceKindEnvelope{Data: *createdWorkspaceKind}
241+
a.dataResponse(w, r, responseEnvelope)
242+
return
243+
}
244+
222245
// calculate the GET location for the created workspace kind (for the Location header)
223246
location := a.LocationGetWorkspaceKind(createdWorkspaceKind.Name)
224247

workspaces/backend/api/workspacekinds_handler_test.go

Lines changed: 215 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ import (
3030
. "github.com/onsi/ginkgo/v2"
3131
. "github.com/onsi/gomega"
3232
corev1 "k8s.io/api/core/v1"
33+
apierrors "k8s.io/apimachinery/pkg/api/errors"
3334
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
3435
"k8s.io/apimachinery/pkg/types"
3536
"k8s.io/apimachinery/pkg/util/validation/field"
@@ -510,4 +511,218 @@ metadata:
510511
))
511512
})
512513
})
514+
515+
Context("when creating a WorkspaceKind (POST)", Serial, func() {
516+
var newWorkspaceKindName string
517+
var validYAML []byte
518+
519+
BeforeEach(func() {
520+
newWorkspaceKindName = "wsk-create-test"
521+
522+
validYAML = []byte(fmt.Sprintf(`
523+
apiVersion: kubeflow.org/v1beta1
524+
kind: WorkspaceKind
525+
metadata:
526+
name: %s
527+
spec:
528+
spawner:
529+
displayName: "JupyterLab Notebook"
530+
description: "A Workspace which runs JupyterLab in a Pod"
531+
icon:
532+
url: "https://jupyter.org/assets/favicons/apple-touch-icon.png"
533+
logo:
534+
url: "https://jupyter.org/assets/logos/jupyter/jupyter.png"
535+
podTemplate:
536+
serviceAccount:
537+
name: default-editor
538+
volumeMounts:
539+
home: "/home/jovyan"
540+
options:
541+
imageConfig:
542+
default: "jupyterlab_scipy_180"
543+
values:
544+
- id: "jupyterlab_scipy_180"
545+
displayName: "JupyterLab SciPy 1.8.0"
546+
description: "JupyterLab with SciPy 1.8.0"
547+
spec:
548+
image: "jupyter/scipy-notebook:2024.1.0"
549+
podConfig:
550+
default: "tiny_cpu"
551+
values:
552+
- id: "tiny_cpu"
553+
displayName: "Tiny CPU"
554+
description: "1 CPU core, 2GB RAM"
555+
spec:
556+
resources:
557+
requests:
558+
cpu: "100m"
559+
memory: "512Mi"
560+
limits:
561+
cpu: "1"
562+
memory: "2Gi"
563+
`, newWorkspaceKindName))
564+
})
565+
566+
AfterEach(func() {
567+
By("deleting the WorkspaceKind if it exists")
568+
workspaceKind := &kubefloworgv1beta1.WorkspaceKind{
569+
ObjectMeta: metav1.ObjectMeta{
570+
Name: newWorkspaceKindName,
571+
},
572+
}
573+
_ = k8sClient.Delete(ctx, workspaceKind)
574+
})
575+
576+
It("should create a WorkspaceKind successfully without dry-run", func() {
577+
req, err := http.NewRequest(http.MethodPost, AllWorkspaceKindsPath, bytes.NewReader(validYAML))
578+
Expect(err).NotTo(HaveOccurred())
579+
req.Header.Set("Content-Type", MediaTypeYaml)
580+
req.Header.Set(userIdHeader, adminUser)
581+
582+
rr := httptest.NewRecorder()
583+
a.CreateWorkspaceKindHandler(rr, req, httprouter.Params{})
584+
rs := rr.Result()
585+
defer rs.Body.Close()
586+
587+
Expect(rs.StatusCode).To(Equal(http.StatusCreated), descUnexpectedHTTPStatus, rr.Body.String())
588+
589+
location := rs.Header.Get("Location")
590+
Expect(location).To(Equal(fmt.Sprintf("/api/v1/workspacekinds/%s", newWorkspaceKindName)))
591+
592+
body, err := io.ReadAll(rs.Body)
593+
Expect(err).NotTo(HaveOccurred())
594+
595+
var response WorkspaceKindCreateEnvelope
596+
err = json.Unmarshal(body, &response)
597+
Expect(err).NotTo(HaveOccurred())
598+
599+
created := &kubefloworgv1beta1.WorkspaceKind{}
600+
err = k8sClient.Get(ctx, types.NamespacedName{Name: newWorkspaceKindName}, created)
601+
Expect(err).NotTo(HaveOccurred())
602+
603+
expected := models.NewWorkspaceKindModelFromWorkspaceKind(created)
604+
Expect(response.Data).To(BeComparableTo(expected))
605+
})
606+
607+
It("should validate WorkspaceKind with dry-run=true without persisting it", func() {
608+
req, err := http.NewRequest(http.MethodPost, AllWorkspaceKindsPath+"?dry_run=true", bytes.NewReader(validYAML))
609+
Expect(err).NotTo(HaveOccurred())
610+
req.Header.Set("Content-Type", MediaTypeYaml)
611+
req.Header.Set(userIdHeader, adminUser)
612+
613+
rr := httptest.NewRecorder()
614+
a.CreateWorkspaceKindHandler(rr, req, httprouter.Params{})
615+
rs := rr.Result()
616+
defer rs.Body.Close()
617+
618+
Expect(rs.StatusCode).To(Equal(http.StatusOK), descUnexpectedHTTPStatus, rr.Body.String())
619+
620+
// Ensure NOT persisted
621+
notCreated := &kubefloworgv1beta1.WorkspaceKind{}
622+
err = k8sClient.Get(ctx, types.NamespacedName{Name: newWorkspaceKindName}, notCreated)
623+
Expect(err).To(HaveOccurred())
624+
Expect(apierrors.IsNotFound(err)).To(BeTrue())
625+
})
626+
627+
It("should return 400 for invalid YAML", func() {
628+
invalidYAML := []byte("invalid: yaml: :")
629+
630+
req, err := http.NewRequest(http.MethodPost, AllWorkspaceKindsPath, bytes.NewReader(invalidYAML))
631+
Expect(err).NotTo(HaveOccurred())
632+
req.Header.Set("Content-Type", MediaTypeYaml)
633+
req.Header.Set(userIdHeader, adminUser)
634+
635+
rr := httptest.NewRecorder()
636+
a.CreateWorkspaceKindHandler(rr, req, httprouter.Params{})
637+
rs := rr.Result()
638+
defer rs.Body.Close()
639+
640+
Expect(rs.StatusCode).To(Equal(http.StatusBadRequest), descUnexpectedHTTPStatus, rr.Body.String())
641+
})
642+
643+
It("should return 415 for wrong content-type when dry-run not set", func() {
644+
req, err := http.NewRequest(http.MethodPost, AllWorkspaceKindsPath, bytes.NewReader(validYAML))
645+
Expect(err).NotTo(HaveOccurred())
646+
req.Header.Set("Content-Type", MediaTypeJson)
647+
req.Header.Set(userIdHeader, adminUser)
648+
649+
rr := httptest.NewRecorder()
650+
a.CreateWorkspaceKindHandler(rr, req, httprouter.Params{})
651+
rs := rr.Result()
652+
defer rs.Body.Close()
653+
654+
Expect(rs.StatusCode).To(Equal(http.StatusUnsupportedMediaType), descUnexpectedHTTPStatus, rr.Body.String())
655+
})
656+
657+
It("should return 400 for wrong content-type when dry-run is set", func() {
658+
req, err := http.NewRequest(http.MethodPost, AllWorkspaceKindsPath+"?dry_run=true", bytes.NewReader(validYAML))
659+
Expect(err).NotTo(HaveOccurred())
660+
req.Header.Set("Content-Type", MediaTypeJson)
661+
req.Header.Set(userIdHeader, adminUser)
662+
663+
rr := httptest.NewRecorder()
664+
a.CreateWorkspaceKindHandler(rr, req, httprouter.Params{})
665+
rs := rr.Result()
666+
defer rs.Body.Close()
667+
668+
Expect(rs.StatusCode).To(Equal(http.StatusBadRequest), descUnexpectedHTTPStatus, rr.Body.String())
669+
})
670+
671+
It("should return 400 for invalid dry-run value", func() {
672+
req, err := http.NewRequest(http.MethodPost, AllWorkspaceKindsPath+"?dry_run=invalid", bytes.NewReader(validYAML))
673+
Expect(err).NotTo(HaveOccurred())
674+
req.Header.Set("Content-Type", MediaTypeYaml)
675+
req.Header.Set(userIdHeader, adminUser)
676+
677+
rr := httptest.NewRecorder()
678+
a.CreateWorkspaceKindHandler(rr, req, httprouter.Params{})
679+
rs := rr.Result()
680+
defer rs.Body.Close()
681+
682+
Expect(rs.StatusCode).To(Equal(http.StatusBadRequest), descUnexpectedHTTPStatus, rr.Body.String())
683+
})
684+
685+
It("should surface Kubernetes validation errors when dry-run=true with invalid resource", func() {
686+
invalidDryRunYAML := []byte(`
687+
apiVersion: kubeflow.org/v1beta1
688+
kind: WorkspaceKind
689+
metadata:
690+
name: INVALID_NAME # invalid DNS-1123 (uppercase not allowed)
691+
spec:
692+
spawner:
693+
displayName: "Bad Test"
694+
description: "Invalid name test"
695+
`)
696+
697+
req, _ := http.NewRequest(http.MethodPost, AllWorkspaceKindsPath+"?dry_run=true", bytes.NewReader(invalidDryRunYAML))
698+
req.Header.Set("Content-Type", MediaTypeYaml)
699+
req.Header.Set(userIdHeader, adminUser)
700+
701+
rr := httptest.NewRecorder()
702+
a.CreateWorkspaceKindHandler(rr, req, httprouter.Params{})
703+
rs := rr.Result()
704+
defer rs.Body.Close()
705+
706+
// Your handler maps apierrors.IsInvalid to 422
707+
Expect(rs.StatusCode).To(Equal(http.StatusUnprocessableEntity), descUnexpectedHTTPStatus, rr.Body.String())
708+
709+
body, _ := io.ReadAll(rs.Body)
710+
711+
// Unmarshal to the envelope shape your backend uses
712+
var env ErrorEnvelope
713+
_ = json.Unmarshal(body, &env)
714+
715+
// Ensure we received an error envelope
716+
Expect(env.Error).NotTo(BeNil(), "expected error envelope in response")
717+
718+
// Be lenient about exact schema of causes: assert the raw payload mentions metadata.name
719+
Expect(strings.Contains(string(body), "metadata.name")).
720+
To(BeTrue(), "expected validation details about metadata.name in error payload")
721+
722+
// Ensure object not persisted
723+
notCreated := &kubefloworgv1beta1.WorkspaceKind{}
724+
getErr := k8sClient.Get(ctx, types.NamespacedName{Name: "INVALID_NAME"}, notCreated)
725+
Expect(apierrors.IsNotFound(getErr)).To(BeTrue())
726+
})
727+
})
513728
})

workspaces/backend/internal/repositories/workspacekinds/repo.go

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import (
2222

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

2728
models "github.com/kubeflow/notebooks/workspaces/backend/internal/models/workspacekinds"
@@ -70,9 +71,14 @@ func (r *WorkspaceKindRepository) GetWorkspaceKinds(ctx context.Context) ([]mode
7071
return workspaceKindsModels, nil
7172
}
7273

73-
func (r *WorkspaceKindRepository) Create(ctx context.Context, workspaceKind *kubefloworgv1beta1.WorkspaceKind) (*models.WorkspaceKind, error) {
74+
func (r *WorkspaceKindRepository) Create(ctx context.Context, workspaceKind *kubefloworgv1beta1.WorkspaceKind, dryRun bool) (*models.WorkspaceKind, error) {
75+
opts := &client.CreateOptions{}
76+
if dryRun {
77+
opts.DryRun = []string{metav1.DryRunAll} // server-side dry-run
78+
}
79+
7480
// create workspace kind
75-
if err := r.client.Create(ctx, workspaceKind); err != nil {
81+
if err := r.client.Create(ctx, workspaceKind, opts); err != nil {
7682
if apierrors.IsAlreadyExists(err) {
7783
return nil, ErrWorkspaceKindAlreadyExists
7884
}

0 commit comments

Comments
 (0)