Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions workspaces/backend/api/app.go
Copy link
Contributor

@andyatmiami andyatmiami Feb 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should discuss if ListValues is a "good enough" prefix vs. something (sadly wayy more verbose and) specific.

  • ex: PodTemplateOptionsListValues

Hard to know right now - but if/as we add non PodTemplate data structures in the future - its possible ListValues is misleadingly vague.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@andyatmiami Yes I agree, this will be more detailed even though it is a bit verbose.

Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ const (
// workspacekinds
AllWorkspaceKindsPath = PathPrefix + "/workspacekinds"
WorkspaceKindsByNamePath = AllWorkspaceKindsPath + "/:" + ResourceNamePathParam
ListValuesPath = WorkspaceKindsByNamePath + "/podtemplate/options/listvalues"

// namespaces
AllNamespacesPath = PathPrefix + "/namespaces"
Expand Down Expand Up @@ -136,6 +137,7 @@ func (a *App) Routes() http.Handler {
router.GET(AllWorkspaceKindsPath, a.GetWorkspaceKindsHandler)
router.GET(WorkspaceKindsByNamePath, a.GetWorkspaceKindHandler)
router.POST(AllWorkspaceKindsPath, a.CreateWorkspaceKindHandler)
router.POST(ListValuesPath, a.ListValuesHandler)

// swagger
router.GET(SwaggerPath, a.GetSwaggerHandler)
Expand Down
69 changes: 69 additions & 0 deletions workspaces/backend/api/workspacekinds_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,8 @@ type WorkspaceKindListEnvelope Envelope[[]models.WorkspaceKind]

type WorkspaceKindEnvelope Envelope[models.WorkspaceKind]

type ListValuesEnvelope Envelope[models.ListValuesResponse]

// GetWorkspaceKindHandler retrieves a specific workspace kind by name.
//
// @Summary Get workspace kind
Expand Down Expand Up @@ -237,3 +239,70 @@ func (a *App) CreateWorkspaceKindHandler(w http.ResponseWriter, r *http.Request,
responseEnvelope := &WorkspaceKindCreateEnvelope{Data: createdWorkspaceKind}
a.createdResponse(w, r, responseEnvelope, location)
}

// ListValuesHandler returns filtered imageConfig and podConfig options for a WorkspaceKind.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Based on other PR review comments I have made - I think we need to update annotations to reflect other 4xx status codes that can be returned:

 //	@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."

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in #972

//
// @Summary List values for workspace kind options
// @Description Returns filtered imageConfig and podConfig options based on the provided context. This endpoint is used by the workspace creation wizard to show compatible options.
// @Tags workspacekinds
// @ID listValues
// @Accept json
// @Produce json
// @Param name path string true "Name of the workspace kind" extensions(x-example=jupyterlab)
// @Param body body models.ListValuesRequest true "Request body with optional context filters"
// @Success 200 {object} ListValuesEnvelope "Successful operation. Returns filtered options with rule_effects."
// @Failure 400 {object} ErrorEnvelope "Bad Request. Invalid workspace kind name or request body."
// @Failure 401 {object} ErrorEnvelope "Unauthorized. Authentication is required."
// @Failure 403 {object} ErrorEnvelope "Forbidden. User does not have permission to access the workspace kind."
// @Failure 404 {object} ErrorEnvelope "Not Found. Workspace kind does not exist."
// @Failure 500 {object} ErrorEnvelope "Internal server error. An unexpected error occurred on the server."
// @Router /workspacekinds/{name}/podtemplate/options/listvalues [post]
func (a *App) ListValuesHandler(w http.ResponseWriter, r *http.Request, ps httprouter.Params) {
name := ps.ByName(ResourceNamePathParam)

// validate path parameters
var valErrs field.ErrorList
valErrs = append(valErrs, helper.ValidateWorkspaceKindName(field.NewPath(ResourceNamePathParam), name)...)
if len(valErrs) > 0 {
a.failedValidationResponse(w, r, errMsgPathParamsInvalid, valErrs, nil)
return
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We want to be consistent with the other API endpoints in notebooks-v2.

After path param validation - given we are parsing a request body - we need need to validate content-type here:

	// validate the Content-Type header
	if success := a.ValidateContentType(w, r, MediaTypeJson); !success {
		return
	}

you can see this pattern in effect (for instance) here:

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in #972

// parse request body
var requestBody models.ListValuesRequest
if err := a.DecodeJSON(r, &requestBody); err != nil {
a.badRequestResponse(w, r, err)
return
}
Comment on lines +271 to +276
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After decoding the request payload - we want to make sure we are validating the contents. Existing codebase strives for a clear/repeatable pattern in how this is accomplished:

  1. Handler Pattern (e.g., secrets_handler.go, workspaces_handler.go)
  // 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
  }
  1. Model Validate Pattern (e.g., workspaces/types_write.go)
  func (w *WorkspaceCreate) Validate(prefix *field.Path) []*field.Error {
      var errs []*field.Error

      // validate the workspace name
      namePath := prefix.Child("name")
      errs = append(errs, helper.ValidateWorkspaceName(namePath, w.Name)...)

      // validate nested struct
      podTemplatePath := prefix.Child("podTemplate")
      errs = append(errs, w.PodTemplate.Validate(podTemplatePath)...)

      return errs
  }
  • ℹ️ As your request payload simply acts as filters - your model definitions defined in a types.go is the proper way to be structured!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would follow/adhere to existing patterns when calling DecodeJSON

For instance, in secrets_handler.go (and/or most files) - you see this pattern:

	// parse request body
	bodyEnvelope := &SecretCreateEnvelope{}
	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
	}

Lets structure similarly in this PR.

Copy link

@akrem-chabchoub akrem-chabchoub Mar 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in #972 (WIP for comments below)


// =========================== AUTH ===========================
authPolicies := []*auth.ResourcePolicy{
auth.NewResourcePolicy(
auth.ResourceVerbGet,
&kubefloworgv1beta1.WorkspaceKind{
ObjectMeta: metav1.ObjectMeta{Name: name},
},
),
}
if success := a.requireAuth(w, r, authPolicies); !success {
return
}
// ============================================================

// get the workspace kind
workspaceKind, err := a.repositories.WorkspaceKind.GetWorkspaceKind(r.Context(), name)
if err != nil {
if errors.Is(err, repository.ErrWorkspaceKindNotFound) {
a.notFoundResponse(w, r)
return
}
a.serverErrorResponse(w, r, err)
return
}

// build the response with rule_effects and context filtering
response := models.BuildListValuesResponse(workspaceKind, requestBody.Data.Context)

responseEnvelope := &ListValuesEnvelope{Data: response}
a.dataResponse(w, r, responseEnvelope)
}
72 changes: 72 additions & 0 deletions workspaces/backend/internal/models/workspacekinds/funcs.go
Original file line number Diff line number Diff line change
Expand Up @@ -175,3 +175,75 @@ func buildOptionRedirect(redirect *kubefloworgv1beta1.OptionRedirect) *OptionRed
Message: message,
}
}

// BuildListValuesResponse transforms a WorkspaceKind into a ListValuesResponse
// by adding rule_effects to each option value and applying context filters
func BuildListValuesResponse(wsk WorkspaceKind, context *ListValuesContext) ListValuesResponse {
imageValues := buildImageConfigValuesWithRules(wsk.PodTemplate.Options.ImageConfig, context)
podValues := buildPodConfigValuesWithRules(wsk.PodTemplate.Options.PodConfig, context)

return ListValuesResponse{
ImageConfig: ImageConfigWithRules{
Default: wsk.PodTemplate.Options.ImageConfig.Default,
Values: imageValues,
},
PodConfig: PodConfigWithRules{
Default: wsk.PodTemplate.Options.PodConfig.Default,
Values: podValues,
},
}
}

func buildImageConfigValuesWithRules(imageConfig ImageConfig, context *ListValuesContext) []ImageConfigValueWithRules {
values := []ImageConfigValueWithRules{}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
values := []ImageConfigValueWithRules{}
// Pre-allocate to avoid repeated allocations and copies
values := make([]ImageConfigValueWithRules, 0, len(imageConfig.Values))

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in #972


for _, v := range imageConfig.Values {
// Filter by context if imageConfig.id is specified
if context != nil && context.ImageConfig != nil {
if v.Id != context.ImageConfig.Id {
continue // skip this value
}
}

// Transform and add rule_effects
values = append(values, ImageConfigValueWithRules{
Id: v.Id,
DisplayName: v.DisplayName,
Description: v.Description,
Labels: v.Labels,
Hidden: v.Hidden,
Redirect: v.Redirect,
ClusterMetrics: v.ClusterMetrics,
RuleEffects: RuleEffects{UiHide: false}, // Always false for stub
})
}

return values
}

func buildPodConfigValuesWithRules(podConfig PodConfig, context *ListValuesContext) []PodConfigValueWithRules {
values := []PodConfigValueWithRules{}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
values := []PodConfigValueWithRules{}
// Pre-allocate to avoid repeated allocations and copies
values := make([]PodConfigValueWithRules, 0, len(podConfig.Values))

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in #972


for _, v := range podConfig.Values {
// Filter by context if podConfig.id is specified
if context != nil && context.PodConfig != nil {
if v.Id != context.PodConfig.Id {
continue // skip this value
}
}

// Transform and add rule_effects
values = append(values, PodConfigValueWithRules{
Id: v.Id,
DisplayName: v.DisplayName,
Description: v.Description,
Labels: v.Labels,
Hidden: v.Hidden,
Redirect: v.Redirect,
ClusterMetrics: v.ClusterMetrics,
RuleEffects: RuleEffects{UiHide: false}, // Always false for stub
})
}

return values
}
67 changes: 67 additions & 0 deletions workspaces/backend/internal/models/workspacekinds/types.go
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This it TOTALLY MY FAULT and I humbly apologize for the oversight, but we use camelCase in our backend API - and I have no idea why I forgot this when originally drafting up the API spec 😢

The following kebab_case attributes should be moved to camelCase:

  • rule_effects -> ruleEffects
  • ui_hide -> uiHide

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in #972

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given existing pattern we are starting to form (with SecretListItem and WorkspaceListItem - how do you feel about the following naming conventions?

Use <Entity>ListItem pattern for semantic alignment:

  // Consistent with SecretListItem, WorkspaceListItem
  type ImageConfigValueListItem struct {
      ImageConfigValue
      RuleEffects RuleEffects `json:"ruleEffects"`
  }

  type PodConfigValueListItem struct {
      PodConfigValue
      RuleEffects RuleEffects `json:"ruleEffects"`
  }

  // Response structs
  type ImageConfigListResult struct {
      Default string                      `json:"default"`
      Values  []ImageConfigValueListItem  `json:"values"`
  }

  type PodConfigListResult struct {
      Default string                    `json:"default"`
      Values  []PodConfigValueListItem  `json:"values"`
  }

  type ListValuesResponse struct {
      ImageConfig ImageConfigListResult `json:"imageConfig"`
      PodConfig   PodConfigListResult   `json:"podConfig"`
  }

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in #972

Original file line number Diff line number Diff line change
Expand Up @@ -109,3 +109,70 @@ const (
RedirectMessageLevelWarning RedirectMessageLevel = "Warning"
RedirectMessageLevelDanger RedirectMessageLevel = "Danger"
)

type ListValuesRequest struct {
Data ListValuesRequestData `json:"data"`
}

type ListValuesRequestData struct {
Context *ListValuesContext `json:"context,omitempty"`
}

Comment on lines +112 to +120
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
type ListValuesRequest struct {
Data ListValuesRequestData `json:"data"`
}
type ListValuesRequestData struct {
Context *ListValuesContext `json:"context,omitempty"`
}
// REMOVE this - it duplicates Envelope
// type ListValuesRequest struct {
// Data ListValuesRequestData `json:"data"`
// }
// KEEP this - it has the "context" field
type ListValuesRequest struct {
Context *ListValuesContext `json:"context,omitempty"`
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Please note that I changed the remaining model here from ListValuesRequestData to ListValuesRequest

In api/workspacekinds_handler.go:

  // Use the standard Envelope pattern wrapping ListValuesRequest
  type ListValuesRequestEnvelope Envelope[*models.ListValuesRequest]

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in #972

type ListValuesContext struct {
Namespace *ContextNamespace `json:"namespace,omitempty"`
PodConfig *ContextPodConfig `json:"podConfig,omitempty"`
ImageConfig *ContextImageConfig `json:"imageConfig,omitempty"`
}

type ContextNamespace struct {
Name string `json:"name"`
}

type ContextPodConfig struct {
Id string `json:"id"`
}

type ContextImageConfig struct {
Id string `json:"id"`
}

type ListValuesResponse struct {
ImageConfig ImageConfigWithRules `json:"imageConfig"`
PodConfig PodConfigWithRules `json:"podConfig"`
}

type ImageConfigWithRules struct {
Default string `json:"default"`
Values []ImageConfigValueWithRules `json:"values"`
}

type ImageConfigValueWithRules struct {
Id string `json:"id"`
DisplayName string `json:"displayName"`
Description string `json:"description"`
Labels []OptionLabel `json:"labels"`
Hidden bool `json:"hidden"`
Redirect *OptionRedirect `json:"redirect,omitempty"`
ClusterMetrics clusterMetrics `json:"clusterMetrics,omitempty"`
RuleEffects RuleEffects `json:"rule_effects"`
}

type PodConfigWithRules struct {
Default string `json:"default"`
Values []PodConfigValueWithRules `json:"values"`
}

type PodConfigValueWithRules struct {
Id string `json:"id"`
DisplayName string `json:"displayName"`
Description string `json:"description"`
Labels []OptionLabel `json:"labels"`
Hidden bool `json:"hidden"`
Redirect *OptionRedirect `json:"redirect,omitempty"`
ClusterMetrics clusterMetrics `json:"clusterMetrics,omitempty"`
RuleEffects RuleEffects `json:"rule_effects"`
}

type RuleEffects struct {
UiHide bool `json:"ui_hide"`
}
Loading
Loading