diff --git a/workspaces/backend/api/suite_test.go b/workspaces/backend/api/suite_test.go index 9c0c77fef..d468a29cf 100644 --- a/workspaces/backend/api/suite_test.go +++ b/workspaces/backend/api/suite_test.go @@ -245,12 +245,17 @@ func NewExampleWorkspaceKind(name string) *kubefloworgv1beta1.WorkspaceKind { VolumeMounts: kubefloworgv1beta1.WorkspaceKindVolumeMounts{ Home: "/home/jovyan", }, - HTTPProxy: &kubefloworgv1beta1.HTTPProxy{ - RemovePathPrefix: ptr.To(false), - RequestHeaders: &kubefloworgv1beta1.IstioHeaderOperations{ - Set: map[string]string{"X-RStudio-Root-Path": "{{ .PathPrefix }}"}, - Add: map[string]string{}, - Remove: []string{}, + Ports: []kubefloworgv1beta1.WorkspaceKindPort{ + { + PortId: "jupyterlab", + HTTPProxy: &kubefloworgv1beta1.HTTPProxy{ + RemovePathPrefix: ptr.To(false), + RequestHeaders: &kubefloworgv1beta1.IstioHeaderOperations{ + Set: map[string]string{}, + Add: map[string]string{}, + Remove: []string{}, + }, + }, }, }, ExtraEnv: []v1.EnvVar{ diff --git a/workspaces/controller/api/v1beta1/workspacekind_types.go b/workspaces/controller/api/v1beta1/workspacekind_types.go index 7e7952a02..a109500f8 100644 --- a/workspaces/controller/api/v1beta1/workspacekind_types.go +++ b/workspaces/controller/api/v1beta1/workspacekind_types.go @@ -29,6 +29,16 @@ import ( =============================================================================== */ +// PortId represents a port identifier +// - this is NOT used as the Container or Service port name, but as part of the HTTP path +// - this is used to reference the port in the `imageconfig` ports.[].id +// - this is also used to reference the port in the podtemplate ports.[].portId +// +// +kubebuilder:validation:MinLength:=1 +// +kubebuilder:validation:MaxLength:=32 +// +kubebuilder:validation:Pattern:=^[a-z0-9][a-z0-9_-]*[a-z0-9]$ +type PortId string + // WorkspaceKindSpec defines the desired state of WorkspaceKind type WorkspaceKindSpec struct { @@ -115,9 +125,9 @@ type WorkspaceKindPodTemplate struct { // volume mount paths VolumeMounts WorkspaceKindVolumeMounts `json:"volumeMounts"` - // http proxy configs (MUTABLE) + // ports that the container listens on // +kubebuilder:validation:Optional - HTTPProxy *HTTPProxy `json:"httpProxy,omitempty"` + Ports []WorkspaceKindPort `json:"ports,omitempty"` // environment variables for Workspace Pods (MUTABLE) // - the following go template functions are available: @@ -151,6 +161,17 @@ type WorkspaceKindPodTemplate struct { Options WorkspaceKindPodOptions `json:"options"` } +type WorkspaceKindPort struct { + // the id of the port + // - identifier for the port in `imageconfig` ports.[].id + // +kubebuilder:example="jupyterlab" + PortId PortId `json:"portId"` + + // the http proxy config for the port (MUTABLE) + // +kubebuilder:validation:Optional + HTTPProxy *HTTPProxy `json:"httpProxy,omitempty"` +} + type WorkspaceKindPodMetadata struct { // labels to be applied to the Pod resource // +kubebuilder:validation:Optional @@ -339,11 +360,8 @@ type ImageConfigSpec struct { type ImagePort struct { // the id of the port // - this is NOT used as the Container or Service port name, but as part of the HTTP path - // +kubebuilder:validation:MinLength:=1 - // +kubebuilder:validation:MaxLength:=32 - // +kubebuilder:validation:Pattern:=^[a-z0-9][a-z0-9_-]*[a-z0-9]$ // +kubebuilder:example="jupyterlab" - Id string `json:"id"` + Id PortId `json:"id"` // the port number // +kubebuilder:validation:Minimum:=1 diff --git a/workspaces/controller/api/v1beta1/zz_generated.deepcopy.go b/workspaces/controller/api/v1beta1/zz_generated.deepcopy.go index 568c3f809..216c4c2d4 100644 --- a/workspaces/controller/api/v1beta1/zz_generated.deepcopy.go +++ b/workspaces/controller/api/v1beta1/zz_generated.deepcopy.go @@ -701,10 +701,12 @@ func (in *WorkspaceKindPodTemplate) DeepCopyInto(out *WorkspaceKindPodTemplate) (*in).DeepCopyInto(*out) } out.VolumeMounts = in.VolumeMounts - if in.HTTPProxy != nil { - in, out := &in.HTTPProxy, &out.HTTPProxy - *out = new(HTTPProxy) - (*in).DeepCopyInto(*out) + if in.Ports != nil { + in, out := &in.Ports, &out.Ports + *out = make([]WorkspaceKindPort, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } } if in.ExtraEnv != nil { in, out := &in.ExtraEnv, &out.ExtraEnv @@ -750,6 +752,26 @@ func (in *WorkspaceKindPodTemplate) DeepCopy() *WorkspaceKindPodTemplate { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *WorkspaceKindPort) DeepCopyInto(out *WorkspaceKindPort) { + *out = *in + if in.HTTPProxy != nil { + in, out := &in.HTTPProxy, &out.HTTPProxy + *out = new(HTTPProxy) + (*in).DeepCopyInto(*out) + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new WorkspaceKindPort. +func (in *WorkspaceKindPort) DeepCopy() *WorkspaceKindPort { + if in == nil { + return nil + } + out := new(WorkspaceKindPort) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *WorkspaceKindProbes) DeepCopyInto(out *WorkspaceKindProbes) { *out = *in diff --git a/workspaces/controller/config/crd/bases/kubeflow.org_workspacekinds.yaml b/workspaces/controller/config/crd/bases/kubeflow.org_workspacekinds.yaml index 9fea44636..2fb97491c 100644 --- a/workspaces/controller/config/crd/bases/kubeflow.org_workspacekinds.yaml +++ b/workspaces/controller/config/crd/bases/kubeflow.org_workspacekinds.yaml @@ -2275,51 +2275,6 @@ spec: x-kubernetes-list-map-keys: - name x-kubernetes-list-type: map - httpProxy: - description: http proxy configs (MUTABLE) - properties: - removePathPrefix: - default: false - description: |- - if the path prefix is stripped from incoming HTTP requests - - if true, the '/workspace/{profile_name}/{workspace_name}/' path prefix - is stripped from incoming requests, the application sees the request - as if it was made to '/...' - - this only works if the application serves RELATIVE URLs for its assets - type: boolean - requestHeaders: - description: |- - header manipulation rules for incoming HTTP requests - - sets the `spec.http[].headers.request` of the Istio VirtualService - https://istio.io/latest/docs/reference/config/networking/virtual-service/#Headers-HeaderOperations - - the following string templates are available: - - `.PathPrefix`: the path prefix of the Workspace (e.g. '/workspace/{profile_name}/{workspace_name}/') - properties: - add: - additionalProperties: - type: string - description: append the given values to the headers specified - by keys (will create a comma-separated list of values) - example: - My-Header: value-to-append - type: object - remove: - description: remove the specified headers - example: - - Header-To-Remove - items: - type: string - type: array - set: - additionalProperties: - type: string - description: overwrite the headers specified by key with - the given values - example: - X-RStudio-Root-Path: '{{ .PathPrefix }}' - type: object - type: object - type: object options: description: options are the user-selectable fields, they determine the PodSpec of the Workspace @@ -3729,6 +3684,69 @@ spec: description: labels to be applied to the Pod resource type: object type: object + ports: + description: ports that the container listens on + items: + properties: + httpProxy: + description: the http proxy config for the port (MUTABLE) + properties: + removePathPrefix: + default: false + description: |- + if the path prefix is stripped from incoming HTTP requests + - if true, the '/workspace/{profile_name}/{workspace_name}/' path prefix + is stripped from incoming requests, the application sees the request + as if it was made to '/...' + - this only works if the application serves RELATIVE URLs for its assets + type: boolean + requestHeaders: + description: |- + header manipulation rules for incoming HTTP requests + - sets the `spec.http[].headers.request` of the Istio VirtualService + https://istio.io/latest/docs/reference/config/networking/virtual-service/#Headers-HeaderOperations + - the following string templates are available: + - `.PathPrefix`: the path prefix of the Workspace (e.g. '/workspace/{profile_name}/{workspace_name}/') + properties: + add: + additionalProperties: + type: string + description: append the given values to the headers + specified by keys (will create a comma-separated + list of values) + example: + My-Header: value-to-append + type: object + remove: + description: remove the specified headers + example: + - Header-To-Remove + items: + type: string + type: array + set: + additionalProperties: + type: string + description: overwrite the headers specified by + key with the given values + example: + X-RStudio-Root-Path: '{{ .PathPrefix }}' + type: object + type: object + type: object + portId: + description: |- + the id of the port + - identifier for the port in `imageconfig` ports.[].id + example: jupyterlab + maxLength: 32 + minLength: 1 + pattern: ^[a-z0-9][a-z0-9_-]*[a-z0-9]$ + type: string + required: + - portId + type: object + type: array probes: description: standard probes to determine Container health (MUTABLE) properties: diff --git a/workspaces/controller/config/samples/jupyterlab_v1beta1_workspacekind.yaml b/workspaces/controller/config/samples/jupyterlab_v1beta1_workspacekind.yaml index d7bb858b2..f2689e7b3 100644 --- a/workspaces/controller/config/samples/jupyterlab_v1beta1_workspacekind.yaml +++ b/workspaces/controller/config/samples/jupyterlab_v1beta1_workspacekind.yaml @@ -135,28 +135,36 @@ spec: ## home: "/home/jovyan" - ## http proxy configs (MUTABLE) - ## - httpProxy: - - ## if the path prefix is stripped from incoming HTTP requests - ## - if true, the '/workspace/{profile_name}/{workspace_name}/' path prefix - ## is stripped from incoming requests, the application sees the request - ## as if it was made to '/...' - ## - this only works if the application serves RELATIVE URLs for its assets - ## - removePathPrefix: false + ## port configs (MUTABLE) + ports: + + ## the list of ports that the Workspace exposes + ## configs apply to a single port + ## portId is the identifier for the port in `imageconfig` ports.[].id + - portId: "jupyterlab" + + ## http proxy configs (MUTABLE) + ## only "HTTP" protocol ports are supported + httpProxy: + + ## if the path prefix is stripped from incoming HTTP requests + ## - if true, the '/workspace/{profile_name}/{workspace_name}/' path prefix + ## is stripped from incoming requests, the application sees the request + ## as if it was made to '/...' + ## - this only works if the application serves RELATIVE URLs for its assets + ## + removePathPrefix: false - ## header manipulation rules for incoming HTTP requests - ## - sets the `spec.http[].headers.request` of the Istio VirtualService - ## https://istio.io/latest/docs/reference/config/networking/virtual-service/#Headers-HeaderOperations - ## - the following string templates are available: - ## - `.PathPrefix`: the path prefix of the Workspace (e.g. '/workspace/{profile_name}/{workspace_name}/') - ## - requestHeaders: {} - #set: { "X-RStudio-Root-Path": "{{ .PathPrefix }}" } # for RStudio - #add: {} - #remove: [] + ## header manipulation rules for incoming HTTP requests + ## - sets the `spec.http[].headers.request` of the Istio VirtualService + ## https://istio.io/latest/docs/reference/config/networking/virtual-service/#Headers-HeaderOperations + ## - the following string templates are available: + ## - `.PathPrefix`: the path prefix of the Workspace (e.g. '/workspace/{profile_name}/{workspace_name}/') + ## + requestHeaders: {} + #set: { "X-RStudio-Root-Path": "{{ .PathPrefix }}" } # for RStudio + #add: {} + #remove: [] ## environment variables for Workspace Pods (MUTABLE) ## - spec for EnvVar: diff --git a/workspaces/controller/internal/controller/suite_test.go b/workspaces/controller/internal/controller/suite_test.go index fbc5a5918..279e10836 100644 --- a/workspaces/controller/internal/controller/suite_test.go +++ b/workspaces/controller/internal/controller/suite_test.go @@ -220,12 +220,17 @@ func NewExampleWorkspaceKind1(name string) *kubefloworgv1beta1.WorkspaceKind { VolumeMounts: kubefloworgv1beta1.WorkspaceKindVolumeMounts{ Home: "/home/jovyan", }, - HTTPProxy: &kubefloworgv1beta1.HTTPProxy{ - RemovePathPrefix: ptr.To(false), - RequestHeaders: &kubefloworgv1beta1.IstioHeaderOperations{ - Set: map[string]string{"X-RStudio-Root-Path": "{{ .PathPrefix }}"}, - Add: map[string]string{}, - Remove: []string{}, + Ports: []kubefloworgv1beta1.WorkspaceKindPort{ + { + PortId: "jupyterlab", + HTTPProxy: &kubefloworgv1beta1.HTTPProxy{ + RemovePathPrefix: ptr.To(false), + RequestHeaders: &kubefloworgv1beta1.IstioHeaderOperations{ + Set: map[string]string{}, + Add: map[string]string{}, + Remove: []string{}, + }, + }, }, }, ExtraEnv: []v1.EnvVar{ diff --git a/workspaces/controller/internal/controller/workspace_controller.go b/workspaces/controller/internal/controller/workspace_controller.go index 66c13b174..adefc85f1 100644 --- a/workspaces/controller/internal/controller/workspace_controller.go +++ b/workspaces/controller/internal/controller/workspace_controller.go @@ -650,7 +650,7 @@ func generateStatefulSet(workspace *kubefloworgv1beta1.Workspace, workspaceKind seenPorts[port.Port] = true // NOTE: we construct this map for use in the go string templates - containerPortsIdMap[port.Id] = port + containerPortsIdMap[string(port.Id)] = port } // generate container env diff --git a/workspaces/controller/internal/webhook/suite_test.go b/workspaces/controller/internal/webhook/suite_test.go index 2eee78390..5fb6cf44f 100644 --- a/workspaces/controller/internal/webhook/suite_test.go +++ b/workspaces/controller/internal/webhook/suite_test.go @@ -208,12 +208,20 @@ func NewExampleWorkspaceKind(name string) *kubefloworgv1beta1.WorkspaceKind { VolumeMounts: kubefloworgv1beta1.WorkspaceKindVolumeMounts{ Home: "/home/jovyan", }, - HTTPProxy: &kubefloworgv1beta1.HTTPProxy{ - RemovePathPrefix: ptr.To(false), - RequestHeaders: &kubefloworgv1beta1.IstioHeaderOperations{ - Set: map[string]string{"X-RStudio-Root-Path": "{{ .PathPrefix }}"}, - Add: map[string]string{}, - Remove: []string{}, + Ports: []kubefloworgv1beta1.WorkspaceKindPort{ + { + PortId: "jupyterlab", + HTTPProxy: &kubefloworgv1beta1.HTTPProxy{ + RemovePathPrefix: ptr.To(false), + RequestHeaders: &kubefloworgv1beta1.IstioHeaderOperations{ + Set: map[string]string{}, + Add: map[string]string{}, + Remove: []string{}, + }, + }, + }, + { + PortId: "my_port", }, }, ExtraEnv: []v1.EnvVar{ @@ -613,6 +621,38 @@ func NewExampleWorkspaceKindWithDuplicatePorts(name string) *kubefloworgv1beta1. return workspaceKind } +// NewExampleWorkspaceKindWithEmptyPortsArrayInPodTemplate returns a WorkspaceKind with an empty ports array in podTemplate.ports. +func NewExampleWorkspaceKindWithEmptyPortsArrayInPodTemplate(name string) *kubefloworgv1beta1.WorkspaceKind { + workspaceKind := NewExampleWorkspaceKind(name) + workspaceKind.Spec.PodTemplate.Ports = []kubefloworgv1beta1.WorkspaceKindPort{} + return workspaceKind +} + +// NewExampleWorkspaceKindWithDuplicatePortsInPodTemplate returns a WorkspaceKind with duplicate ports in podTemplate.ports. +func NewExampleWorkspaceKindWithDuplicatePortsInPodTemplate(name string) *kubefloworgv1beta1.WorkspaceKind { + workspaceKind := NewExampleWorkspaceKind(name) + workspaceKind.Spec.PodTemplate.Ports = []kubefloworgv1beta1.WorkspaceKindPort{ + { + PortId: "jupyterlab", + }, + { + PortId: "jupyterlab", + }, + } + return workspaceKind +} + +// NewExampleWorkspaceKindWithNonExistentPortIdInImageConfig returns a WorkspaceKind with a non-existent portId in imageConfig.ports. +func NewExampleWorkspaceKindWithNonExistentPortIdInImageConfig(name string) *kubefloworgv1beta1.WorkspaceKind { + workspaceKind := NewExampleWorkspaceKind(name) + workspaceKind.Spec.PodTemplate.Ports = []kubefloworgv1beta1.WorkspaceKindPort{ + { + PortId: "non-existent-port-id", + }, + } + return workspaceKind +} + // NewExampleWorkspaceKindWithInvalidExtraEnvValue returns a WorkspaceKind with an invalid extraEnv value. func NewExampleWorkspaceKindWithInvalidExtraEnvValue(name string) *kubefloworgv1beta1.WorkspaceKind { workspaceKind := NewExampleWorkspaceKind(name) diff --git a/workspaces/controller/internal/webhook/workspacekind_webhook.go b/workspaces/controller/internal/webhook/workspacekind_webhook.go index e7fd49a38..f58556b56 100644 --- a/workspaces/controller/internal/webhook/workspacekind_webhook.go +++ b/workspaces/controller/internal/webhook/workspacekind_webhook.go @@ -78,6 +78,9 @@ func (v *WorkspaceKindValidator) ValidateCreate(ctx context.Context, obj runtime // validate the extra environment variables allErrs = append(allErrs, validateExtraEnv(workspaceKind)...) + // validate the ports configuration + allErrs = append(allErrs, validatePorts(workspaceKind)...) + // generate helper maps for imageConfig values imageConfigIdMap := make(map[string]kubefloworgv1beta1.ImageConfigValue) imageConfigRedirectMap := make(map[string]string) @@ -156,6 +159,11 @@ func (v *WorkspaceKindValidator) ValidateUpdate(ctx context.Context, oldObj, new allErrs = append(allErrs, validateExtraEnv(newWorkspaceKind)...) } + // validate the ports configuration + if !equality.Semantic.DeepEqual(newWorkspaceKind.Spec.PodTemplate.Ports, oldWorkspaceKind.Spec.PodTemplate.Ports) { + allErrs = append(allErrs, validatePorts(newWorkspaceKind)...) + } + // calculate changes to imageConfig values var shouldValidateImageConfigRedirects = false toValidateImageConfigIds := make(map[string]bool) @@ -516,6 +524,61 @@ func (v *WorkspaceKindValidator) validatePodTemplatePodMetadata(workspaceKind *k return errs } +// validatePorts validates the ports in podTemplate.ports of WorkspaceKind +func validatePorts(workspaceKind *kubefloworgv1beta1.WorkspaceKind) []*field.Error { + var errs []*field.Error + + ports := workspaceKind.Spec.PodTemplate.Ports + portsPath := field.NewPath("spec", "podTemplate", "ports") + + // if no ports are defined, we can skip validation + if len(ports) == 0 { + return errs + } + + // collect all referenced port IDs from imageConfig values + referencedPortIds := make(map[string]bool) + for _, imageConfigValue := range workspaceKind.Spec.PodTemplate.Options.ImageConfig.Values { + for _, imagePort := range imageConfigValue.Spec.Ports { + referencedPortIds[string(imagePort.Id)] = true + } + } + + // track seen port IDs to ensure uniqueness + seenPortIds := make(map[string]bool) + + // validate each port in the ports array + for i, port := range ports { + portId := port.PortId + portPath := portsPath.Index(i) + portIdPath := portPath.Child("portId") + + // validate that portId is not empty (should be caught by CRD validation, but be safe) + if portId == "" { + errs = append(errs, field.Required(portIdPath, "portId is required")) + continue + } + + // validate that each port has a unique portId + if seenPortIds[string(portId)] { + errs = append(errs, field.Duplicate(portIdPath, portId)) + } else { + seenPortIds[string(portId)] = true + } + + // validate that the portId references a valid port from imageConfig + if !referencedPortIds[string(portId)] { + errs = append(errs, field.Invalid(portIdPath, portId, + fmt.Sprintf("portId %q does not match any port defined in imageConfig values", portId))) + } + + // httpProxy is optional, so no validation needed for nil + // The structure validation is handled by the CRD schema + } + + return errs +} + // validateExtraEnv validates the extra environment variables in a WorkspaceKind func validateExtraEnv(workspaceKind *kubefloworgv1beta1.WorkspaceKind) []*field.Error { var errs []*field.Error @@ -575,7 +638,7 @@ func validateImageConfigValue(imageConfigValue *kubefloworgv1beta1.ImageConfigVa // validate the ports seenPorts := make(map[int32]bool) for _, port := range imageConfigValue.Spec.Ports { - portId := port.Id + portId := string(port.Id) portNumber := port.Port if _, exists := seenPorts[portNumber]; exists { portPath := imageConfigValuePath.Child("spec", "ports").Key(portId).Child("port") diff --git a/workspaces/controller/internal/webhook/workspacekind_webhook_test.go b/workspaces/controller/internal/webhook/workspacekind_webhook_test.go index dd2c321ae..5e52768ad 100644 --- a/workspaces/controller/internal/webhook/workspacekind_webhook_test.go +++ b/workspaces/controller/internal/webhook/workspacekind_webhook_test.go @@ -98,6 +98,21 @@ var _ = Describe("WorkspaceKind Webhook", func() { workspaceKind: NewExampleWorkspaceKindWithDuplicatePorts("wsk-webhook-create--image-config-duplicate-ports"), shouldSucceed: false, }, + { + description: "should reject creation with empty ports array in podTemplate", + workspaceKind: NewExampleWorkspaceKindWithEmptyPortsArrayInPodTemplate("wsk-webhook-create--pod-template-empty-ports-array"), + shouldSucceed: true, + }, + { + description: "should reject creation with duplicate ports in podTemplate.ports", + workspaceKind: NewExampleWorkspaceKindWithDuplicatePortsInPodTemplate("wsk-webhook-create--pod-template-duplicate-portids"), + shouldSucceed: false, + }, + { + description: "should reject creation with non-existent portId in imageConfig.ports", + workspaceKind: NewExampleWorkspaceKindWithNonExistentPortIdInImageConfig("wsk-webhook-create--image-config-non-existent-portid"), + shouldSucceed: false, + }, { description: "should reject creation if extraEnv[].value is not a valid Go template", workspaceKind: NewExampleWorkspaceKindWithInvalidExtraEnvValue("wsk-webhook-create--extra-invalid-env-value"), @@ -508,6 +523,26 @@ var _ = Describe("WorkspaceKind Webhook", func() { return ContainSubstring("port %d is defined more than once", duplicatePortNumber) }, }, + { + description: "should reject updating a portId in podTemplate.ports to a duplicate portId", + shouldSucceed: false, + + workspaceKind: NewExampleWorkspaceKind(workspaceKindName), + modifyKindFn: func(wsk *kubefloworgv1beta1.WorkspaceKind) gomegaTypes.GomegaMatcher { + wsk.Spec.PodTemplate.Ports[1].PortId = "jupyterlab" + return ContainSubstring("Duplicate value: %q", "jupyterlab") + }, + }, + { + description: "should reject updating a portId in podTemplate.ports to a non-existent portId in imageConfig.ports", + shouldSucceed: false, + + workspaceKind: NewExampleWorkspaceKind(workspaceKindName), + modifyKindFn: func(wsk *kubefloworgv1beta1.WorkspaceKind) gomegaTypes.GomegaMatcher { + wsk.Spec.PodTemplate.Ports[0].PortId = "non-existent-port-id" + return ContainSubstring("portId %q does not match any port defined in imageConfig values", "non-existent-port-id") + }, + }, { description: "should reject updating a podMetadata.labels key to an invalid value", shouldSucceed: false,