diff --git a/workspaces/backend/api/helpers.go b/workspaces/backend/api/helpers.go index 7583ea70b..2469d82c2 100644 --- a/workspaces/backend/api/helpers.go +++ b/workspaces/backend/api/helpers.go @@ -111,3 +111,19 @@ func (a *App) LocationGetWorkspaceKind(name string) string { path := strings.Replace(WorkspaceKindsByNamePath, ":"+ResourceNamePathParam, name, 1) return path } + +// GetBooleanQueryParameter reads a query parameter that only accepts "true"/"false". +// If the parameter is absent, returns (false, nil). +// On invalid value, writes a 400 and returns an error. +func (a *App) GetBooleanQueryParameter(w http.ResponseWriter, r *http.Request, paramName string) (bool, error) { + val := r.URL.Query().Get(paramName) + if val == "" { + return false, nil // default if not provided + } + if val != "true" && val != "false" { + err := fmt.Errorf("invalid query parameter '%s' value '%s'. Must be 'true' or 'false'", paramName, val) + a.badRequestResponse(w, r, err) + return false, err + } + return val == "true", nil +} diff --git a/workspaces/backend/api/workspacekinds_handler.go b/workspaces/backend/api/workspacekinds_handler.go index b995a4002..452404302 100644 --- a/workspaces/backend/api/workspacekinds_handler.go +++ b/workspaces/backend/api/workspacekinds_handler.go @@ -153,9 +153,24 @@ func (a *App) GetWorkspaceKindsHandler(w http.ResponseWriter, r *http.Request, _ // @Failure 500 {object} ErrorEnvelope "Internal server error. An unexpected error occurred on the server." // @Router /workspacekinds [post] func (a *App) CreateWorkspaceKindHandler(w http.ResponseWriter, r *http.Request, _ httprouter.Params) { - - // validate the Content-Type header - if success := a.ValidateContentType(w, r, MediaTypeYaml); !success { + // Parse dry_run query parameter using helper + isDryRun, err := a.GetBooleanQueryParameter(w, r, "dry_run") + if err != nil { + return // 400 already written + } + hasDryRun := r.URL.Query().Get("dry_run") != "" + + // Content-Type validation + if !a.ValidateContentType(w, r, MediaTypeYaml) { + if hasDryRun { + // Special case: dry_run provided with wrong media type → 400 + a.badRequestResponse(w, r, + fmt.Errorf("dry_run is only supported when Content-Type: %s", MediaTypeYaml)) + } else { + // Normal case: wrong media type → 415 + a.unsupportedMediaTypeResponse(w, r, + fmt.Errorf("only %s is supported", MediaTypeYaml)) + } return } @@ -204,7 +219,7 @@ func (a *App) CreateWorkspaceKindHandler(w http.ResponseWriter, r *http.Request, } // ============================================================ - createdWorkspaceKind, err := a.repositories.WorkspaceKind.Create(r.Context(), workspaceKind) + createdWorkspaceKind, err := a.repositories.WorkspaceKind.Create(r.Context(), workspaceKind, isDryRun) if err != nil { if errors.Is(err, repository.ErrWorkspaceKindAlreadyExists) { a.conflictResponse(w, r, err) @@ -219,6 +234,13 @@ func (a *App) CreateWorkspaceKindHandler(w http.ResponseWriter, r *http.Request, return } + // Return appropriate response based on dry-run + if isDryRun { + responseEnvelope := &WorkspaceKindEnvelope{Data: *createdWorkspaceKind} + a.dataResponse(w, r, responseEnvelope) + return + } + // calculate the GET location for the created workspace kind (for the Location header) location := a.LocationGetWorkspaceKind(createdWorkspaceKind.Name) diff --git a/workspaces/backend/api/workspacekinds_handler_test.go b/workspaces/backend/api/workspacekinds_handler_test.go index 464659272..fff511216 100644 --- 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,219 @@ metadata: )) }) }) + + Context("when creating a WorkspaceKind (POST)", Serial, func() { + var newWorkspaceKindName string + var validYAML []byte + + BeforeEach(func() { + newWorkspaceKindName = "wsk-create-test" + + validYAML = []byte(fmt.Sprintf(` + apiVersion: kubeflow.org/v1beta1 + kind: WorkspaceKind + metadata: + name: %s + spec: + spawner: + displayName: "JupyterLab Notebook" + description: "A Workspace which runs JupyterLab in a Pod" + icon: + url: "https://jupyter.org/assets/favicons/apple-touch-icon.png" + logo: + url: "https://jupyter.org/assets/logos/jupyter/jupyter.png" + podTemplate: + serviceAccount: + name: default-editor + volumeMounts: + home: "/home/jovyan" + options: + imageConfig: + default: "jupyterlab_scipy_180" + values: + - id: "jupyterlab_scipy_180" + displayName: "JupyterLab SciPy 1.8.0" + description: "JupyterLab with SciPy 1.8.0" + spec: + image: "jupyter/scipy-notebook:2024.1.0" + podConfig: + default: "tiny_cpu" + values: + - id: "tiny_cpu" + displayName: "Tiny CPU" + description: "1 CPU core, 2GB RAM" + spec: + resources: + requests: + cpu: "100m" + memory: "512Mi" + limits: + cpu: "1" + memory: "2Gi" + `, newWorkspaceKindName)) + }) + + AfterEach(func() { + By("deleting the WorkspaceKind if it exists") + workspaceKind := &kubefloworgv1beta1.WorkspaceKind{ + ObjectMeta: metav1.ObjectMeta{ + Name: newWorkspaceKindName, + }, + } + _ = k8sClient.Delete(ctx, workspaceKind) + }) + + It("should create a WorkspaceKind successfully without dry-run", func() { + req, err := http.NewRequest(http.MethodPost, AllWorkspaceKindsPath, bytes.NewReader(validYAML)) + Expect(err).NotTo(HaveOccurred()) + req.Header.Set("Content-Type", MediaTypeYaml) + req.Header.Set(userIdHeader, adminUser) + + rr := httptest.NewRecorder() + a.CreateWorkspaceKindHandler(rr, req, httprouter.Params{}) + rs := rr.Result() + defer rs.Body.Close() + + Expect(rs.StatusCode).To(Equal(http.StatusCreated), descUnexpectedHTTPStatus, rr.Body.String()) + + location := rs.Header.Get("Location") + Expect(location).To(Equal(fmt.Sprintf("/api/v1/workspacekinds/%s", newWorkspaceKindName))) + + body, err := io.ReadAll(rs.Body) + Expect(err).NotTo(HaveOccurred()) + + var response WorkspaceKindCreateEnvelope + err = json.Unmarshal(body, &response) + Expect(err).NotTo(HaveOccurred()) + + created := &kubefloworgv1beta1.WorkspaceKind{} + err = k8sClient.Get(ctx, types.NamespacedName{Name: newWorkspaceKindName}, created) + Expect(err).NotTo(HaveOccurred()) + + expected := models.NewWorkspaceKindModelFromWorkspaceKind(created) + Expect(response.Data).To(BeComparableTo(expected)) + }) + + It("should validate WorkspaceKind with dry-run=true without persisting it", func() { + req, err := http.NewRequest(http.MethodPost, AllWorkspaceKindsPath+"?dry_run=true", bytes.NewReader(validYAML)) + Expect(err).NotTo(HaveOccurred()) + req.Header.Set("Content-Type", MediaTypeYaml) + req.Header.Set(userIdHeader, adminUser) + + rr := httptest.NewRecorder() + a.CreateWorkspaceKindHandler(rr, req, httprouter.Params{}) + rs := rr.Result() + defer rs.Body.Close() + + Expect(rs.StatusCode).To(Equal(http.StatusOK), descUnexpectedHTTPStatus, rr.Body.String()) + + // Ensure NOT persisted + notCreated := &kubefloworgv1beta1.WorkspaceKind{} + err = k8sClient.Get(ctx, types.NamespacedName{Name: newWorkspaceKindName}, notCreated) + Expect(err).To(HaveOccurred()) + Expect(apierrors.IsNotFound(err)).To(BeTrue()) + }) + + It("should return 400 for invalid YAML", func() { + invalidYAML := []byte("invalid: yaml: :") + + req, err := http.NewRequest(http.MethodPost, AllWorkspaceKindsPath, bytes.NewReader(invalidYAML)) + Expect(err).NotTo(HaveOccurred()) + req.Header.Set("Content-Type", MediaTypeYaml) + req.Header.Set(userIdHeader, adminUser) + + rr := httptest.NewRecorder() + a.CreateWorkspaceKindHandler(rr, req, httprouter.Params{}) + rs := rr.Result() + defer rs.Body.Close() + + Expect(rs.StatusCode).To(Equal(http.StatusBadRequest), descUnexpectedHTTPStatus, rr.Body.String()) + }) + + It("should return 415 for wrong content-type when dry-run not set", func() { + req, err := http.NewRequest(http.MethodPost, AllWorkspaceKindsPath, bytes.NewReader(validYAML)) + Expect(err).NotTo(HaveOccurred()) + req.Header.Set("Content-Type", MediaTypeJson) + req.Header.Set(userIdHeader, adminUser) + + rr := httptest.NewRecorder() + a.CreateWorkspaceKindHandler(rr, req, httprouter.Params{}) + rs := rr.Result() + defer rs.Body.Close() + + Expect(rs.StatusCode).To(Equal(http.StatusUnsupportedMediaType), descUnexpectedHTTPStatus, rr.Body.String()) + }) + + It("should return 400 for wrong content-type when dry-run is set", func() { + req, err := http.NewRequest(http.MethodPost, AllWorkspaceKindsPath+"?dry_run=true", bytes.NewReader(validYAML)) + Expect(err).NotTo(HaveOccurred()) + req.Header.Set("Content-Type", MediaTypeJson) + req.Header.Set(userIdHeader, adminUser) + + rr := httptest.NewRecorder() + a.CreateWorkspaceKindHandler(rr, req, httprouter.Params{}) + rs := rr.Result() + defer rs.Body.Close() + + Expect(rs.StatusCode).To(Equal(http.StatusBadRequest), descUnexpectedHTTPStatus, rr.Body.String()) + }) + + It("should return 400 for invalid dry-run value", func() { + req, err := http.NewRequest(http.MethodPost, AllWorkspaceKindsPath+"?dry_run=invalid", bytes.NewReader(validYAML)) + Expect(err).NotTo(HaveOccurred()) + req.Header.Set("Content-Type", MediaTypeYaml) + req.Header.Set(userIdHeader, adminUser) + + rr := httptest.NewRecorder() + a.CreateWorkspaceKindHandler(rr, req, httprouter.Params{}) + rs := rr.Result() + defer rs.Body.Close() + + Expect(rs.StatusCode).To(Equal(http.StatusBadRequest), descUnexpectedHTTPStatus, rr.Body.String()) + }) + + It("should surface Kubernetes validation errors when dry-run=true with invalid resource", func() { + invalidDryRunYAML := []byte(` + apiVersion: kubeflow.org/v1beta1 + kind: WorkspaceKind + metadata: + name: INVALID_NAME # invalid DNS-1123 (uppercase not allowed) + spec: + spawner: + displayName: "Bad Test" + description: "Invalid name test" + `) + + req, _ := http.NewRequest(http.MethodPost, AllWorkspaceKindsPath+"?dry_run=true", bytes.NewReader(invalidDryRunYAML)) + req.Header.Set("Content-Type", MediaTypeYaml) + req.Header.Set(userIdHeader, adminUser) + + rr := httptest.NewRecorder() + a.CreateWorkspaceKindHandler(rr, req, httprouter.Params{}) + rs := rr.Result() + defer rs.Body.Close() + + // Your handler maps apierrors.IsInvalid to 422 + Expect(rs.StatusCode).To(Equal(http.StatusUnprocessableEntity), descUnexpectedHTTPStatus, rr.Body.String()) + + body, _ := io.ReadAll(rs.Body) + + // Unmarshal to the envelope shape your backend uses + var env ErrorEnvelope + _ = json.Unmarshal(body, &env) + + // Ensure we received an error envelope + Expect(env.Error).NotTo(BeNil(), "expected error envelope in response") + + // Be lenient about exact schema of causes: assert the raw payload mentions metadata.name + Expect(strings.Contains(string(body), "metadata.name")). + To(BeTrue(), "expected validation details about metadata.name in error payload") + + // Ensure object not persisted + notCreated := &kubefloworgv1beta1.WorkspaceKind{} + getErr := k8sClient.Get(ctx, types.NamespacedName{Name: "INVALID_NAME"}, notCreated) + Expect(apierrors.IsNotFound(getErr)).To(BeTrue()) + }) + }) + }) diff --git a/workspaces/backend/internal/repositories/workspacekinds/repo.go b/workspaces/backend/internal/repositories/workspacekinds/repo.go index 02cd208b9..7621027dc 100644 --- a/workspaces/backend/internal/repositories/workspacekinds/repo.go +++ b/workspaces/backend/internal/repositories/workspacekinds/repo.go @@ -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" @@ -70,17 +71,17 @@ func (r *WorkspaceKindRepository) GetWorkspaceKinds(ctx context.Context) ([]mode return workspaceKindsModels, nil } -func (r *WorkspaceKindRepository) Create(ctx context.Context, workspaceKind *kubefloworgv1beta1.WorkspaceKind) (*models.WorkspaceKind, error) { - // create workspace kind - if err := r.client.Create(ctx, workspaceKind); err != nil { +func (r *WorkspaceKindRepository) Create(ctx context.Context, workspaceKind *kubefloworgv1beta1.WorkspaceKind, dryRun bool) (*models.WorkspaceKind, error) { + opts := &client.CreateOptions{} + if dryRun { + opts.DryRun = []string{metav1.DryRunAll} // server-side dry-run + } + + if err := r.client.Create(ctx, workspaceKind, opts); err != nil { if apierrors.IsAlreadyExists(err) { return nil, ErrWorkspaceKindAlreadyExists } - if apierrors.IsInvalid(err) { - // NOTE: we don't wrap this error so we can unpack it in the caller - // and extract the validation errors returned by the Kubernetes API server - return nil, err - } + // NOTE: don't wrap invalid errors, caller extracts validation causes return nil, err } @@ -88,6 +89,8 @@ func (r *WorkspaceKindRepository) Create(ctx context.Context, workspaceKind *kub // // TODO: this function should return the WorkspaceKindUpdate model, once the update WSK api is implemented // + + // Convert the created workspace to a WorkspaceKindUpdate model createdWorkspaceKindModel := models.NewWorkspaceKindModelFromWorkspaceKind(workspaceKind) return &createdWorkspaceKindModel, nil diff --git a/workspaces/frontend/package-lock.json b/workspaces/frontend/package-lock.json index c32e4493a..a429c1a7d 100644 --- a/workspaces/frontend/package-lock.json +++ b/workspaces/frontend/package-lock.json @@ -27666,4 +27666,4 @@ } } } -} +} \ No newline at end of file