pr 2: workspace update api

thesuperzapper · andyatmiami · commit 5d06b7186cc4 · 2025-11-21T22:34:35.000-05:00
Signed-off-by: Mathew Wicks &lt;5735406+thesuperzapper@users.noreply.github.com&gt;

Signed-off-by: Andy Stoneberg &lt;astonebe@redhat.com&gt;
diff --git a/workspaces/backend/api/app.go b/workspaces/backend/api/app.go
@@ -117,6 +117,7 @@ func (a *App) Routes() http.Handler {
 	router.GET(WorkspacesByNamespacePath, a.GetWorkspacesByNamespaceHandler)
 	router.GET(WorkspacesByNamePath, a.GetWorkspaceHandler)
 	router.POST(WorkspacesByNamespacePath, a.CreateWorkspaceHandler)
+	router.PATCH(WorkspacesByNamePath, a.UpdateWorkspaceHandler)
 	router.DELETE(WorkspacesByNamePath, a.DeleteWorkspaceHandler)
 	router.POST(PauseWorkspacePath, a.PauseActionWorkspaceHandler)
 
diff --git a/workspaces/backend/api/workspaces_handler.go b/workspaces/backend/api/workspaces_handler.go
@@ -35,6 +35,8 @@ import (
 
 type WorkspaceCreateEnvelope Envelope[*models.WorkspaceCreate]
 
+type WorkspaceUpdateEnvelope Envelope[*models.WorkspaceUpdate]
+
 type WorkspaceListEnvelope Envelope[[]models.Workspace]
 
 type WorkspaceEnvelope Envelope[models.Workspace]
@@ -292,6 +294,135 @@ func (a *App) CreateWorkspaceHandler(w http.ResponseWriter, r *http.Request, ps
 	a.createdResponse(w, r, responseEnvelope, location)
 }
 
+// UpdateWorkspaceHandler updates an existing workspace.
+//
+//	@Summary		Update workspace
+//	@Description	Updates an existing workspace
+//	@Tags			workspaces
+//	@ID				updateWorkspace
+//	@Accept			json
+//	@Produce		json
+//	@Param			namespace	path		string					true	"Namespace of the workspace"	extensions(x-example=kubeflow-user-example-com)
+//	@Param			name		path		string					true	"Name of the workspace"			extensions(x-example=my-workspace)
+//	@Param			body		body		WorkspaceCreateEnvelope	true	"Workspace creation configuration"
+//	@Success		200			{object}	WorkspaceEnvelope		"Workspace updated successfully"
+//	@Failure		400			{object}	ErrorEnvelope			"Bad Request."
+//	@Failure		401			{object}	ErrorEnvelope			"Unauthorized. Authentication is required."
+//	@Failure		403			{object}	ErrorEnvelope			"Forbidden. User does not have permission to create workspace."
+//	@Failure		409			{object}	ErrorEnvelope			"Conflict. Current workspace generation is newer than provided."
+//	@Failure		413			{object}	ErrorEnvelope			"Request Entity Too Large. The request body is too large."
+//	@Failure		415			{object}	ErrorEnvelope			"Unsupported Media Type. Content-Type header is not correct."
+//	@Failure		422			{object}	ErrorEnvelope			"Unprocessable Entity. Validation error."
+//	@Failure		500			{object}	ErrorEnvelope			"Internal server error. An unexpected error occurred on the server."
+//	@Router			/workspaces/{namespace}/{name} [patch]
+func (a *App) UpdateWorkspaceHandler(w http.ResponseWriter, r *http.Request, ps httprouter.Params) {
+	namespace := ps.ByName(NamespacePathParam)
+	workspaceName := ps.ByName(ResourceNamePathParam)
+
+	// validate path parameters
+	var valErrs field.ErrorList
+	valErrs = append(valErrs, helper.ValidateKubernetesNamespaceName(field.NewPath(NamespacePathParam), namespace)...)
+	valErrs = append(valErrs, helper.ValidateWorkspaceName(field.NewPath(ResourceNamePathParam), workspaceName)...)
+	if len(valErrs) > 0 {
+		a.failedValidationResponse(w, r, errMsgPathParamsInvalid, valErrs, nil)
+		return
+	}
+
+	//
+	// TODO: require the caller to provide the generation base that they are expecting to update.
+	//       either include the generation as a field in the request body, or a header.
+	//
+
+	//
+	// TODO: update the "get" workspace (not list) to return the current generation of the workspace.
+	//       this probably means swapping the GET for Workspace with return a WorkspaceUpdate rather than a Workspace
+	//        - it also means that we will be treating GET as only used for retrieving the current state of the workspace,
+	//          before updating it, whereas LIST is used for getting the details of all workspaces (e.g. the table view in the UI).
+	//
+
+	// validate the Content-Type header
+	if success := a.ValidateContentType(w, r, "application/json"); !success {
+		return
+	}
+
+	// decode the request body
+	bodyEnvelope := &WorkspaceUpdateEnvelope{}
+	err := a.DecodeJSON(r, bodyEnvelope)
+	if err != nil {
+		if a.IsMaxBytesError(err) {
+			a.requestEntityTooLargeResponse(w, r, err)
+			return
+		}
+		//
+		// TODO: handle UnmarshalTypeError and return 422,
+		//       decode the paths which were failed to decode (included in the error)
+		//       and also do this in the other handlers which decode json
+		//
+		a.badRequestResponse(w, r, fmt.Errorf("error decoding request body: %w", err))
+		return
+	}
+
+	// validate the request body
+	dataPath := field.NewPath("data")
+	if bodyEnvelope.Data == nil {
+		valErrs = field.ErrorList{field.Required(dataPath, "data is required")}
+		a.failedValidationResponse(w, r, errMsgRequestBodyInvalid, valErrs, nil)
+		return
+	}
+	valErrs = bodyEnvelope.Data.Validate(dataPath)
+	if len(valErrs) > 0 {
+		a.failedValidationResponse(w, r, errMsgRequestBodyInvalid, valErrs, nil)
+		return
+	}
+
+	workspaceUpdate := bodyEnvelope.Data
+
+	// =========================== AUTH ===========================
+	authPolicies := []*auth.ResourcePolicy{
+		auth.NewResourcePolicy(
+			auth.ResourceVerbUpdate,
+			&kubefloworgv1beta1.Workspace{
+				ObjectMeta: metav1.ObjectMeta{
+					Namespace: namespace,
+					Name:      workspaceName,
+				},
+			},
+		),
+	}
+	if success := a.requireAuth(w, r, authPolicies); !success {
+		return
+	}
+	// ============================================================
+
+	updatedWorkspace, err := a.repositories.Workspace.UpdateWorkspace(r.Context(), workspaceUpdate, namespace, workspaceName)
+	if err != nil {
+		if errors.Is(err, repository.ErrWorkspaceNotFound) {
+			a.notFoundResponse(w, r)
+			return
+		}
+		//
+		// TODO: there is still a race condition once we actually try and patch/update the workspace,
+		//       unless we use optimistic locking with the generation field
+		//      (or resourceVersion, with the risk of unexpected 500 errors to the caller).
+		//
+		if errors.Is(err, repository.ErrWorkspaceGenerationConflict) {
+			causes := helper.StatusCausesFromAPIStatus(err)
+			a.conflictResponse(w, r, err, causes)
+			return
+		}
+		if apierrors.IsInvalid(err) {
+			causes := helper.StatusCausesFromAPIStatus(err)
+			a.failedValidationResponse(w, r, errMsgKubernetesValidation, nil, causes)
+			return
+		}
+		a.serverErrorResponse(w, r, fmt.Errorf("error updating workspace: %w", err))
+		return
+	}
+
+	responseEnvelope := &WorkspaceUpdateEnvelope{Data: updatedWorkspace}
+	a.dataResponse(w, r, responseEnvelope)
+}
+
 // DeleteWorkspaceHandler deletes a specific workspace by namespace and name.
 //
 //	@Summary		Delete workspace
diff --git a/workspaces/backend/internal/models/workspaces/funcs_write.go b/workspaces/backend/internal/models/workspaces/funcs_write.go
@@ -67,3 +67,48 @@ func NewWorkspaceCreateModelFromWorkspace(ws *kubefloworgv1beta1.Workspace) *Wor
 
 	return workspaceCreateModel
 }
+
+// NewWorkspaceUpdateModelFromWorkspace creates WorkspaceUpdate model from a Workspace object.
+func NewWorkspaceUpdateModelFromWorkspace(ws *kubefloworgv1beta1.Workspace) *WorkspaceUpdate {
+	podLabels := make(map[string]string)
+	podAnnotations := make(map[string]string)
+	if ws.Spec.PodTemplate.PodMetadata != nil {
+		// NOTE: we copy the maps to avoid creating a reference to the original maps.
+		for k, v := range ws.Spec.PodTemplate.PodMetadata.Labels {
+			podLabels[k] = v
+		}
+		for k, v := range ws.Spec.PodTemplate.PodMetadata.Annotations {
+			podAnnotations[k] = v
+		}
+	}
+
+	dataVolumes := make([]PodVolumeMount, len(ws.Spec.PodTemplate.Volumes.Data))
+	for i, v := range ws.Spec.PodTemplate.Volumes.Data {
+		dataVolumes[i] = PodVolumeMount{
+			PVCName:   v.PVCName,
+			MountPath: v.MountPath,
+			ReadOnly:  ptr.Deref(v.ReadOnly, false),
+		}
+	}
+
+	workspaceUpdateModel := &WorkspaceUpdate{
+		Paused:       ptr.Deref(ws.Spec.Paused, false),
+		DeferUpdates: ptr.Deref(ws.Spec.DeferUpdates, false),
+		PodTemplate: PodTemplateMutate{
+			PodMetadata: PodMetadataMutate{
+				Labels:      podLabels,
+				Annotations: podAnnotations,
+			},
+			Volumes: PodVolumesMutate{
+				Home: ws.Spec.PodTemplate.Volumes.Home,
+				Data: dataVolumes,
+			},
+			Options: PodTemplateOptionsMutate{
+				ImageConfig: ws.Spec.PodTemplate.Options.ImageConfig,
+				PodConfig:   ws.Spec.PodTemplate.Options.PodConfig,
+			},
+		},
+	}
+
+	return workspaceUpdateModel
+}
diff --git a/workspaces/backend/internal/models/workspaces/types_write.go b/workspaces/backend/internal/models/workspaces/types_write.go
@@ -51,6 +51,31 @@ func (w *WorkspaceCreate) Validate(prefix *field.Path) []*field.Error {
 	return errs
 }
 
+// WorkspaceUpdate is used to update an existing workspace.
+// NOTE: we only do basic validation, more complex validation is done by the controller when attempting to update the workspace.
+type WorkspaceUpdate struct {
+	//
+	// TODO: it probably makes sense to return the generation here,
+	//       so both the GET and PATCH requests include it, so we can detect update conflicts.
+	//
+	// ...
+
+	Paused       bool              `json:"paused"`       // TODO: remove `paused` once we have an "actions" api for pausing workspaces
+	DeferUpdates bool              `json:"deferUpdates"` // TODO: remove `deferUpdates` once the controller is no longer applying redirects
+	PodTemplate  PodTemplateMutate `json:"podTemplate"`
+}
+
+// Validate validates the WorkspaceUpdate struct.
+func (w *WorkspaceUpdate) Validate(prefix *field.Path) []*field.Error {
+	var errs []*field.Error
+
+	// validate pod template
+	podTemplatePath := prefix.Child("podTemplate")
+	errs = append(errs, w.PodTemplate.Validate(podTemplatePath)...)
+
+	return errs
+}
+
 type PodTemplateMutate struct {
 	PodMetadata PodMetadataMutate        `json:"podMetadata"`
 	Volumes     PodVolumesMutate         `json:"volumes"`
diff --git a/workspaces/backend/internal/repositories/workspaces/repo.go b/workspaces/backend/internal/repositories/workspaces/repo.go
@@ -33,9 +33,10 @@ import (
 )
 
 var (
-	ErrWorkspaceNotFound      = fmt.Errorf("workspace not found")
-	ErrWorkspaceAlreadyExists = fmt.Errorf("workspace already exists")
-	ErrWorkspaceInvalidState  = fmt.Errorf("workspace is in an invalid state for this operation")
+	ErrWorkspaceNotFound           = fmt.Errorf("workspace not found")
+	ErrWorkspaceAlreadyExists      = fmt.Errorf("workspace already exists")
+	ErrWorkspaceInvalidState       = fmt.Errorf("workspace is in an invalid state for this operation")
+	ErrWorkspaceGenerationConflict = fmt.Errorf("current workspace generation does not match request")
 )
 
 type WorkspaceRepository struct {
@@ -202,6 +203,20 @@ func (r *WorkspaceRepository) CreateWorkspace(ctx context.Context, workspaceCrea
 	return createdWorkspaceModel, nil
 }
 
+func (r *WorkspaceRepository) UpdateWorkspace(ctx context.Context, workspaceUpdate *models.WorkspaceUpdate, namespace, workspaceName string) (*models.WorkspaceUpdate, error) {
+	//
+	// TODO: implement update logic,
+	//
+	// TODO: conflict detection based on generation
+	//       make sure to raise `ErrWorkspaceGenerationConflict` which is handled by the caller
+	//       (?? does this mean we should avoid the cache when getting the current workspace)
+	//
+	//
+	// TODO: ensure we raise ErrWorkspaceNotFound if the workspace does not exist (and handle by calkler)
+	//
+	return nil, fmt.Errorf("update workspace not implemented yet")
+}
+
 func (r *WorkspaceRepository) DeleteWorkspace(ctx context.Context, namespace, workspaceName string) error {
 	workspace := &kubefloworgv1beta1.Workspace{
 		ObjectMeta: metav1.ObjectMeta{
