From 0a95e1a70b44595c76d71dadba3fc480a3e74a3b Mon Sep 17 00:00:00 2001 From: Rohan Kumar Date: Tue, 27 May 2025 15:07:50 +0530 Subject: [PATCH 1/3] feat : add service annotations to DWR CRD Signed-off-by: Rohan Kumar --- .../controller/v1alpha1/devworkspacerouting_types.go | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/apis/controller/v1alpha1/devworkspacerouting_types.go b/apis/controller/v1alpha1/devworkspacerouting_types.go index 6db93e48a..8dd7fc1a6 100644 --- a/apis/controller/v1alpha1/devworkspacerouting_types.go +++ b/apis/controller/v1alpha1/devworkspacerouting_types.go @@ -31,6 +31,10 @@ type DevWorkspaceRoutingSpec struct { Endpoints map[string]EndpointList `json:"endpoints"` // Selector that should be used by created services to point to the devworkspace Pod PodSelector map[string]string `json:"podSelector"` + // Machines to services map + // +optional + // +kubebuilder:validation:Optional + Service map[string]Service `json:"services"` } type DevWorkspaceRoutingClass string @@ -108,6 +112,14 @@ type EndpointProtocol string // +kubebuilder:validation:XPreserveUnknownFields type Attributes map[string]apiext.JSON +type Service struct { + // Map of annotations to be added to the Kubernetes Service. + // +optional + // +patchMergeKey=name + // +patchStrategy=merge + Annotations map[string]string `json:"annotations,omitempty"` +} + type Endpoint struct { // +kubebuilder:validation:Pattern=^[a-z0-9]([-a-z0-9]*[a-z0-9])?$ // +kubebuilder:validation:MaxLength=63 From 724d74291f5f6076756f2209ef831e31322abc3c Mon Sep 17 00:00:00 2001 From: Rohan Kumar Date: Wed, 28 May 2025 11:50:37 +0530 Subject: [PATCH 2/3] chore : regenerate CRDs for DWR service annotations Signed-off-by: Rohan Kumar --- .../v1alpha1/zz_generated.deepcopy.go | 29 +++++++++++++++++++ ...oller.devfile.io_devworkspaceroutings.yaml | 11 +++++++ deploy/deployment/kubernetes/combined.yaml | 12 ++++++++ ...r.devfile.io.CustomResourceDefinition.yaml | 12 ++++++++ deploy/deployment/openshift/combined.yaml | 12 ++++++++ ...r.devfile.io.CustomResourceDefinition.yaml | 12 ++++++++ ...oller.devfile.io_devworkspaceroutings.yaml | 12 ++++++++ 7 files changed, 100 insertions(+) diff --git a/apis/controller/v1alpha1/zz_generated.deepcopy.go b/apis/controller/v1alpha1/zz_generated.deepcopy.go index f4e3367d1..07ee98fd1 100644 --- a/apis/controller/v1alpha1/zz_generated.deepcopy.go +++ b/apis/controller/v1alpha1/zz_generated.deepcopy.go @@ -240,6 +240,13 @@ func (in *DevWorkspaceRoutingSpec) DeepCopyInto(out *DevWorkspaceRoutingSpec) { (*out)[key] = val } } + if in.Service != nil { + in, out := &in.Service, &out.Service + *out = make(map[string]Service, len(*in)) + for key, val := range *in { + (*out)[key] = *val.DeepCopy() + } + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new DevWorkspaceRoutingSpec. @@ -609,6 +616,28 @@ func (in *RoutingConfig) DeepCopy() *RoutingConfig { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *Service) DeepCopyInto(out *Service) { + *out = *in + if in.Annotations != nil { + in, out := &in.Annotations, &out.Annotations + *out = make(map[string]string, len(*in)) + for key, val := range *in { + (*out)[key] = val + } + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new Service. +func (in *Service) DeepCopy() *Service { + if in == nil { + return nil + } + out := new(Service) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *ServiceAccountConfig) DeepCopyInto(out *ServiceAccountConfig) { *out = *in diff --git a/deploy/bundle/manifests/controller.devfile.io_devworkspaceroutings.yaml b/deploy/bundle/manifests/controller.devfile.io_devworkspaceroutings.yaml index b1a1bd29a..97c438b71 100644 --- a/deploy/bundle/manifests/controller.devfile.io_devworkspaceroutings.yaml +++ b/deploy/bundle/manifests/controller.devfile.io_devworkspaceroutings.yaml @@ -172,6 +172,17 @@ spec: routingClass: description: 'Class of the routing: this drives which DevWorkspaceRouting controller will manage this routing' type: string + services: + additionalProperties: + properties: + annotations: + additionalProperties: + type: string + description: Map of annotations to be added to the Kubernetes Service. + type: object + type: object + description: Machines to services map + type: object required: - devworkspaceId - endpoints diff --git a/deploy/deployment/kubernetes/combined.yaml b/deploy/deployment/kubernetes/combined.yaml index d47eba2e4..450ec7781 100644 --- a/deploy/deployment/kubernetes/combined.yaml +++ b/deploy/deployment/kubernetes/combined.yaml @@ -3337,6 +3337,18 @@ spec: description: 'Class of the routing: this drives which DevWorkspaceRouting controller will manage this routing' type: string + services: + additionalProperties: + properties: + annotations: + additionalProperties: + type: string + description: Map of annotations to be added to the Kubernetes + Service. + type: object + type: object + description: Machines to services map + type: object required: - devworkspaceId - endpoints diff --git a/deploy/deployment/kubernetes/objects/devworkspaceroutings.controller.devfile.io.CustomResourceDefinition.yaml b/deploy/deployment/kubernetes/objects/devworkspaceroutings.controller.devfile.io.CustomResourceDefinition.yaml index e7cb982df..18e1834af 100644 --- a/deploy/deployment/kubernetes/objects/devworkspaceroutings.controller.devfile.io.CustomResourceDefinition.yaml +++ b/deploy/deployment/kubernetes/objects/devworkspaceroutings.controller.devfile.io.CustomResourceDefinition.yaml @@ -175,6 +175,18 @@ spec: description: 'Class of the routing: this drives which DevWorkspaceRouting controller will manage this routing' type: string + services: + additionalProperties: + properties: + annotations: + additionalProperties: + type: string + description: Map of annotations to be added to the Kubernetes + Service. + type: object + type: object + description: Machines to services map + type: object required: - devworkspaceId - endpoints diff --git a/deploy/deployment/openshift/combined.yaml b/deploy/deployment/openshift/combined.yaml index 77ce338b3..611e28933 100644 --- a/deploy/deployment/openshift/combined.yaml +++ b/deploy/deployment/openshift/combined.yaml @@ -3337,6 +3337,18 @@ spec: description: 'Class of the routing: this drives which DevWorkspaceRouting controller will manage this routing' type: string + services: + additionalProperties: + properties: + annotations: + additionalProperties: + type: string + description: Map of annotations to be added to the Kubernetes + Service. + type: object + type: object + description: Machines to services map + type: object required: - devworkspaceId - endpoints diff --git a/deploy/deployment/openshift/objects/devworkspaceroutings.controller.devfile.io.CustomResourceDefinition.yaml b/deploy/deployment/openshift/objects/devworkspaceroutings.controller.devfile.io.CustomResourceDefinition.yaml index e7cb982df..18e1834af 100644 --- a/deploy/deployment/openshift/objects/devworkspaceroutings.controller.devfile.io.CustomResourceDefinition.yaml +++ b/deploy/deployment/openshift/objects/devworkspaceroutings.controller.devfile.io.CustomResourceDefinition.yaml @@ -175,6 +175,18 @@ spec: description: 'Class of the routing: this drives which DevWorkspaceRouting controller will manage this routing' type: string + services: + additionalProperties: + properties: + annotations: + additionalProperties: + type: string + description: Map of annotations to be added to the Kubernetes + Service. + type: object + type: object + description: Machines to services map + type: object required: - devworkspaceId - endpoints diff --git a/deploy/templates/crd/bases/controller.devfile.io_devworkspaceroutings.yaml b/deploy/templates/crd/bases/controller.devfile.io_devworkspaceroutings.yaml index 44effa2ec..2d86a48e3 100644 --- a/deploy/templates/crd/bases/controller.devfile.io_devworkspaceroutings.yaml +++ b/deploy/templates/crd/bases/controller.devfile.io_devworkspaceroutings.yaml @@ -172,6 +172,18 @@ spec: description: 'Class of the routing: this drives which DevWorkspaceRouting controller will manage this routing' type: string + services: + additionalProperties: + properties: + annotations: + additionalProperties: + type: string + description: Map of annotations to be added to the Kubernetes + Service. + type: object + type: object + description: Machines to services map + type: object required: - devworkspaceId - endpoints From 96c6a023341e4f10102837476bbcb052a11834c5 Mon Sep 17 00:00:00 2001 From: Rohan Kumar Date: Tue, 27 May 2025 22:49:42 +0530 Subject: [PATCH 3/3] feat : support service annotations in basic solver Signed-off-by: Rohan Kumar --- .../devworkspacerouting_controller_test.go | 1 + .../solvers/basic_solver.go | 34 ++- .../solvers/basic_solver_test.go | 281 ++++++++++++++++++ .../solvers/cluster_solver.go | 2 +- .../devworkspacerouting/solvers/common.go | 23 +- .../devworkspacerouting/util_test.go | 10 + docs/unsupported-devfile-api.adoc | 3 +- pkg/provision/workspace/routing.go | 10 + .../warning/add-all-unsupported-features.yaml | 4 +- webhook/workspace/handler/warning.go | 29 +- 10 files changed, 357 insertions(+), 40 deletions(-) create mode 100644 controllers/controller/devworkspacerouting/solvers/basic_solver_test.go diff --git a/controllers/controller/devworkspacerouting/devworkspacerouting_controller_test.go b/controllers/controller/devworkspacerouting/devworkspacerouting_controller_test.go index 6c8cf1a30..baeedad35 100644 --- a/controllers/controller/devworkspacerouting/devworkspacerouting_controller_test.go +++ b/controllers/controller/devworkspacerouting/devworkspacerouting_controller_test.go @@ -114,6 +114,7 @@ var _ = Describe("DevWorkspaceRouting Controller", func() { err := k8sClient.Get(ctx, serviceNamespacedName, createdService) return err == nil }, timeout, interval).Should(BeTrue(), "Service should exist in cluster") + Expect(createdService.ObjectMeta.Annotations).Should(HaveKeyWithValue(serviceAnnotationKey, serviceAnnotationValue), "Service should have annotation") Expect(createdService.Spec.Selector).Should(Equal(createdDWR.Spec.PodSelector), "Service should have pod selector from DevWorkspace metadata") Expect(createdService.Labels).Should(Equal(ExpectedLabels), "Service should contain DevWorkspace ID label") expectedOwnerReference := devWorkspaceRoutingOwnerRef(createdDWR) diff --git a/controllers/controller/devworkspacerouting/solvers/basic_solver.go b/controllers/controller/devworkspacerouting/solvers/basic_solver.go index 50df27fa1..5aa9596fc 100644 --- a/controllers/controller/devworkspacerouting/solvers/basic_solver.go +++ b/controllers/controller/devworkspacerouting/solvers/basic_solver.go @@ -43,12 +43,38 @@ var nginxIngressAnnotations = func(endpointName string, endpointAnnotations map[ return annotations } +func serviceAnnotations(sourceAnnotations map[string]string, isDiscoverable bool, serviceRoutingConfig controllerv1alpha1.Service) map[string]string { + annotations := make(map[string]string) + if sourceAnnotations != nil && len(sourceAnnotations) > 0 { + for k, v := range sourceAnnotations { + annotations[k] = v + } + } + if isDiscoverable { + annotations[constants.DevWorkspaceDiscoverableServiceAnnotation] = "true" + } + if serviceRoutingConfig.Annotations != nil && len(serviceRoutingConfig.Annotations) > 0 { + for k, v := range serviceRoutingConfig.Annotations { + annotations[k] = v + } + } + return annotations +} + // Basic solver exposes endpoints without any authentication // According to the current cluster there is different behavior: // Kubernetes: use Ingresses without TLS // OpenShift: use Routes with TLS enabled type BasicSolver struct{} +var routingSuffixSupplier = func() string { + return config.GetGlobalConfig().Routing.ClusterHostSuffix +} + +var isOpenShift = func() bool { + return infrastructure.IsOpenShift() +} + var _ RoutingSolver = (*BasicSolver)(nil) func (s *BasicSolver) FinalizerRequired(*controllerv1alpha1.DevWorkspaceRouting) bool { @@ -63,16 +89,16 @@ func (s *BasicSolver) GetSpecObjects(routing *controllerv1alpha1.DevWorkspaceRou routingObjects := RoutingObjects{} // TODO: Use workspace-scoped ClusterHostSuffix to allow overriding - routingSuffix := config.GetGlobalConfig().Routing.ClusterHostSuffix + routingSuffix := routingSuffixSupplier() if routingSuffix == "" { return routingObjects, &RoutingInvalid{"basic routing requires .config.routing.clusterHostSuffix to be set in operator config"} } spec := routing.Spec - services := getServicesForEndpoints(spec.Endpoints, workspaceMeta) - services = append(services, GetDiscoverableServicesForEndpoints(spec.Endpoints, workspaceMeta)...) + services := getServicesForEndpoints(spec, workspaceMeta) + services = append(services, GetDiscoverableServicesForEndpoints(spec, workspaceMeta)...) routingObjects.Services = services - if infrastructure.IsOpenShift() { + if isOpenShift() { routingObjects.Routes = getRoutesForSpec(routingSuffix, spec.Endpoints, workspaceMeta) } else { routingObjects.Ingresses = getIngressesForSpec(routingSuffix, spec.Endpoints, workspaceMeta) diff --git a/controllers/controller/devworkspacerouting/solvers/basic_solver_test.go b/controllers/controller/devworkspacerouting/solvers/basic_solver_test.go new file mode 100644 index 000000000..5b0b439b6 --- /dev/null +++ b/controllers/controller/devworkspacerouting/solvers/basic_solver_test.go @@ -0,0 +1,281 @@ +// Copyright (c) 2019-2025 Red Hat, Inc. +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package solvers + +import ( + "testing" + + "github.com/devfile/devworkspace-operator/apis/controller/v1alpha1" + "github.com/stretchr/testify/assert" + corev1 "k8s.io/api/core/v1" + networkingv1 "k8s.io/api/networking/v1" + apiext "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/intstr" +) + +func TestServiceAnnotations(t *testing.T) { + tests := []struct { + name string + sourceAnnotations map[string]string + isDiscoverable bool + serviceRoutingConfig v1alpha1.Service + expectedAnnotations map[string]string + }{ + { + name: "No annotations provided and discoverable disabled should return empty", + sourceAnnotations: nil, + isDiscoverable: false, + serviceRoutingConfig: v1alpha1.Service{ + Annotations: nil, + }, + expectedAnnotations: map[string]string{}, + }, + { + name: "Source annotations present and discoverable disabled should return source annotations", + sourceAnnotations: map[string]string{ + "key1": "value1", + "key2": "value2", + }, + isDiscoverable: false, + serviceRoutingConfig: v1alpha1.Service{ + Annotations: nil, + }, + expectedAnnotations: map[string]string{ + "key1": "value1", + "key2": "value2", + }, + }, + { + name: "Discoverable annotation enabled should return discoverable annotation", + sourceAnnotations: nil, + isDiscoverable: true, + serviceRoutingConfig: v1alpha1.Service{ + Annotations: nil, + }, + expectedAnnotations: map[string]string{ + "controller.devfile.io/discoverable-service": "true", + }, + }, + { + name: "DevWorkspaceRouting Service routing config annotations merged with source annotations", + sourceAnnotations: map[string]string{ + "key1": "value1", + }, + isDiscoverable: false, + serviceRoutingConfig: v1alpha1.Service{ + Annotations: map[string]string{ + "key3": "value3", + }, + }, + expectedAnnotations: map[string]string{ + "key1": "value1", + "key3": "value3", + }, + }, + { + name: "DevWorkspaceRouting Service routing config annotations merged with source annotations and discoverable annotation", + sourceAnnotations: map[string]string{ + "key1": "value1", + }, + isDiscoverable: true, + serviceRoutingConfig: v1alpha1.Service{ + Annotations: map[string]string{ + "key3": "value3", + }, + }, + expectedAnnotations: map[string]string{ + "controller.devfile.io/discoverable-service": "true", + "key1": "value1", + "key3": "value3", + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Given + When + result := serviceAnnotations(tt.sourceAnnotations, tt.isDiscoverable, tt.serviceRoutingConfig) + // Then + assert.Equal(t, tt.expectedAnnotations, result) + }) + } +} + +var devWorkspaceRouting = v1alpha1.DevWorkspaceRouting{ + Spec: v1alpha1.DevWorkspaceRoutingSpec{ + DevWorkspaceId: "workspaceb978dc9bd4ba428b", + RoutingClass: "basic", + Endpoints: map[string]v1alpha1.EndpointList{ + "component1": []v1alpha1.Endpoint{ + { + Name: "endpoint1", + TargetPort: 8080, + Exposure: "public", + Protocol: "http", + Secure: false, + Path: "/test", + Attributes: map[string]apiext.JSON{}, + Annotations: map[string]string{ + "endpoint-annotation-key1": "endpoint-annotation-value1", + }, + }, + }, + }, + PodSelector: map[string]string{ + "controller.devfile.io/devworkspace_id": "workspaceb978dc9bd4ba428b", + }, + Service: map[string]v1alpha1.Service{ + "component1": { + Annotations: map[string]string{ + "service-annotation-key": "service-annotation-value", + }, + }, + }, + }, +} + +func TestGetSpecObjects_WhenValidDWRProvidedAndOpenShiftUnavailable_ThenGenerateRoutingObjectsServiceAndIngress(t *testing.T) { + // Given + basicSolver := &BasicSolver{} + routingSuffixSupplier = func() string { + return "test.routing" + } + isOpenShift = func() bool { + return false + } + dwRouting := &devWorkspaceRouting + workspaceMeta := DevWorkspaceMetadata{ + DevWorkspaceId: "workspaceb978dc9bd4ba428b", + Namespace: "test", + PodSelector: map[string]string{ + "controller.devfile.io/devworkspace_id": "workspaceb978dc9bd4ba428b", + }, + } + + // When + routingObjects, err := basicSolver.GetSpecObjects(dwRouting, workspaceMeta) + + // Then + assert.NotNil(t, routingObjects) + assert.NoError(t, err) + assert.Len(t, routingObjects.Services, 1) + assert.Equal(t, corev1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: "workspaceb978dc9bd4ba428b-service", + Namespace: "test", + Labels: map[string]string{"controller.devfile.io/devworkspace_id": "workspaceb978dc9bd4ba428b"}, + Annotations: map[string]string{"service-annotation-key": "service-annotation-value"}, + }, + Spec: corev1.ServiceSpec{ + Type: corev1.ServiceTypeClusterIP, + Ports: []corev1.ServicePort{ + { + Name: "endpoint1", + Protocol: corev1.ProtocolTCP, + Port: 8080, + TargetPort: intstr.IntOrString{IntVal: 8080}, + }, + }, + Selector: map[string]string{"controller.devfile.io/devworkspace_id": "workspaceb978dc9bd4ba428b"}, + }, + }, routingObjects.Services[0]) + assert.Len(t, routingObjects.Ingresses, 1) + assert.Equal(t, metav1.ObjectMeta{ + Name: "workspaceb978dc9bd4ba428b-endpoint1", + Namespace: "test", + Labels: map[string]string{"controller.devfile.io/devworkspace_id": "workspaceb978dc9bd4ba428b"}, + Annotations: map[string]string{ + "controller.devfile.io/endpoint_name": "endpoint1", + "endpoint-annotation-key1": "endpoint-annotation-value1", + "nginx.ingress.kubernetes.io/rewrite-target": "/", + "nginx.ingress.kubernetes.io/ssl-redirect": "false", + }, + }, routingObjects.Ingresses[0].ObjectMeta) + assert.Len(t, routingObjects.Ingresses[0].Spec.Rules, 1) + assert.Equal(t, "workspaceb978dc9bd4ba428b-endpoint1-8080.test.routing", routingObjects.Ingresses[0].Spec.Rules[0].Host) + assert.Len(t, routingObjects.Ingresses[0].Spec.Rules[0].HTTP.Paths, 1) + assert.Equal(t, networkingv1.IngressBackend{ + Service: &networkingv1.IngressServiceBackend{ + Name: "workspaceb978dc9bd4ba428b-service", + Port: networkingv1.ServiceBackendPort{Number: int32(8080)}, + }, + }, routingObjects.Ingresses[0].Spec.Rules[0].HTTP.Paths[0].Backend) + assert.Len(t, routingObjects.Routes, 0) +} + +func TestGetSpecObjects_WhenValidDWRProvidedAndOpenShiftAvailable_ThenGenerateRoutingObjectsServiceAndRoute(t *testing.T) { + // Given + basicSolver := &BasicSolver{} + routingSuffixSupplier = func() string { + return "test.routing" + } + isOpenShift = func() bool { + return true + } + dwRouting := &devWorkspaceRouting + workspaceMeta := DevWorkspaceMetadata{ + DevWorkspaceId: "workspaceb978dc9bd4ba428b", + Namespace: "test", + PodSelector: map[string]string{ + "controller.devfile.io/devworkspace_id": "workspaceb978dc9bd4ba428b", + }, + } + + // When + routingObjects, err := basicSolver.GetSpecObjects(dwRouting, workspaceMeta) + + // Then + assert.NotNil(t, routingObjects) + assert.NoError(t, err) + assert.Len(t, routingObjects.Services, 1) + assert.Equal(t, corev1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: "workspaceb978dc9bd4ba428b-service", + Namespace: "test", + Labels: map[string]string{"controller.devfile.io/devworkspace_id": "workspaceb978dc9bd4ba428b"}, + Annotations: map[string]string{"service-annotation-key": "service-annotation-value"}, + }, + Spec: corev1.ServiceSpec{ + Type: corev1.ServiceTypeClusterIP, + Ports: []corev1.ServicePort{ + { + Name: "endpoint1", + Protocol: corev1.ProtocolTCP, + Port: 8080, + TargetPort: intstr.IntOrString{IntVal: 8080}, + }, + }, + Selector: map[string]string{"controller.devfile.io/devworkspace_id": "workspaceb978dc9bd4ba428b"}, + }, + }, routingObjects.Services[0]) + assert.Len(t, routingObjects.Ingresses, 0) + assert.Len(t, routingObjects.Routes, 1) + assert.Equal(t, metav1.ObjectMeta{ + Name: "workspaceb978dc9bd4ba428b-endpoint1", + Namespace: "test", + Labels: map[string]string{ + "controller.devfile.io/devworkspace_id": "workspaceb978dc9bd4ba428b", + }, + Annotations: map[string]string{ + "controller.devfile.io/endpoint_name": "endpoint1", + "endpoint-annotation-key1": "endpoint-annotation-value1", + "haproxy.router.openshift.io/rewrite-target": "/", + }, + }, routingObjects.Routes[0].ObjectMeta) + assert.Equal(t, "workspaceb978dc9bd4ba428b.test.routing", routingObjects.Routes[0].Spec.Host) + assert.Equal(t, "/endpoint1/", routingObjects.Routes[0].Spec.Path) + assert.Equal(t, "Service", routingObjects.Routes[0].Spec.To.Kind) + assert.Equal(t, "workspaceb978dc9bd4ba428b-service", routingObjects.Routes[0].Spec.To.Name) +} diff --git a/controllers/controller/devworkspacerouting/solvers/cluster_solver.go b/controllers/controller/devworkspacerouting/solvers/cluster_solver.go index 7c5462252..46c3f9d46 100644 --- a/controllers/controller/devworkspacerouting/solvers/cluster_solver.go +++ b/controllers/controller/devworkspacerouting/solvers/cluster_solver.go @@ -46,7 +46,7 @@ func (s *ClusterSolver) Finalize(*controllerv1alpha1.DevWorkspaceRouting) error func (s *ClusterSolver) GetSpecObjects(routing *controllerv1alpha1.DevWorkspaceRouting, workspaceMeta DevWorkspaceMetadata) (RoutingObjects, error) { spec := routing.Spec - services := getServicesForEndpoints(spec.Endpoints, workspaceMeta) + services := getServicesForEndpoints(spec, workspaceMeta) podAdditions := &controllerv1alpha1.PodAdditions{} if s.TLS { readOnlyMode := int32(420) diff --git a/controllers/controller/devworkspacerouting/solvers/common.go b/controllers/controller/devworkspacerouting/solvers/common.go index b55367ad8..2bf07f857 100644 --- a/controllers/controller/devworkspacerouting/solvers/common.go +++ b/controllers/controller/devworkspacerouting/solvers/common.go @@ -36,9 +36,10 @@ type DevWorkspaceMetadata struct { // GetDiscoverableServicesForEndpoints converts the endpoint list into a set of services, each corresponding to a single discoverable // endpoint from the list. Endpoints with the NoneEndpointExposure are ignored. -func GetDiscoverableServicesForEndpoints(endpoints map[string]controllerv1alpha1.EndpointList, meta DevWorkspaceMetadata) []corev1.Service { +func GetDiscoverableServicesForEndpoints(routingSpec controllerv1alpha1.DevWorkspaceRoutingSpec, meta DevWorkspaceMetadata) []corev1.Service { var services []corev1.Service - for _, machineEndpoints := range endpoints { + endpoints := routingSpec.Endpoints + for componentName, machineEndpoints := range endpoints { for _, endpoint := range machineEndpoints { if endpoint.Exposure == controllerv1alpha1.NoneEndpointExposure { continue @@ -61,9 +62,7 @@ func GetDiscoverableServicesForEndpoints(endpoints map[string]controllerv1alpha1 Labels: map[string]string{ constants.DevWorkspaceIDLabel: meta.DevWorkspaceId, }, - Annotations: map[string]string{ - constants.DevWorkspaceDiscoverableServiceAnnotation: "true", - }, + Annotations: serviceAnnotations(nil, true, routingSpec.Service[componentName]), }, Spec: corev1.ServiceSpec{ Ports: []corev1.ServicePort{servicePort}, @@ -79,8 +78,9 @@ func GetDiscoverableServicesForEndpoints(endpoints map[string]controllerv1alpha1 // GetServiceForEndpoints returns a single service that exposes all endpoints of given exposure types, possibly also including the discoverable types. // `nil` is returned if the service would expose no ports satisfying the provided criteria. -func GetServiceForEndpoints(endpoints map[string]controllerv1alpha1.EndpointList, meta DevWorkspaceMetadata, includeDiscoverable bool, exposureType ...controllerv1alpha1.EndpointExposure) *corev1.Service { +func GetServiceForEndpoints(routingSpec controllerv1alpha1.DevWorkspaceRoutingSpec, meta DevWorkspaceMetadata, includeDiscoverable bool, exposureType ...controllerv1alpha1.EndpointExposure) *corev1.Service { // "set" of ports that are still left for exposure + endpoints := routingSpec.Endpoints ports := map[int]bool{} for _, es := range endpoints { for _, endpoint := range es { @@ -95,8 +95,9 @@ func GetServiceForEndpoints(endpoints map[string]controllerv1alpha1.EndpointList } var exposedPorts []corev1.ServicePort + var annotations = make(map[string]string) - for _, es := range endpoints { + for componentName, es := range endpoints { for _, endpoint := range es { if !validExposures[endpoint.Exposure] { continue @@ -117,6 +118,7 @@ func GetServiceForEndpoints(endpoints map[string]controllerv1alpha1.EndpointList }) } } + annotations = serviceAnnotations(annotations, false, routingSpec.Service[componentName]) } if len(exposedPorts) == 0 { @@ -130,6 +132,7 @@ func GetServiceForEndpoints(endpoints map[string]controllerv1alpha1.EndpointList Labels: map[string]string{ constants.DevWorkspaceIDLabel: meta.DevWorkspaceId, }, + Annotations: annotations, }, Spec: corev1.ServiceSpec{ Selector: meta.PodSelector, @@ -139,12 +142,12 @@ func GetServiceForEndpoints(endpoints map[string]controllerv1alpha1.EndpointList } } -func getServicesForEndpoints(endpoints map[string]controllerv1alpha1.EndpointList, meta DevWorkspaceMetadata) []corev1.Service { - if len(endpoints) == 0 { +func getServicesForEndpoints(routingSpec controllerv1alpha1.DevWorkspaceRoutingSpec, meta DevWorkspaceMetadata) []corev1.Service { + if len(routingSpec.Endpoints) == 0 { return nil } - service := GetServiceForEndpoints(endpoints, meta, true, controllerv1alpha1.PublicEndpointExposure, controllerv1alpha1.InternalEndpointExposure) + service := GetServiceForEndpoints(routingSpec, meta, true, controllerv1alpha1.PublicEndpointExposure, controllerv1alpha1.InternalEndpointExposure) if service == nil { return nil } diff --git a/controllers/controller/devworkspacerouting/util_test.go b/controllers/controller/devworkspacerouting/util_test.go index d386f4635..26df82610 100644 --- a/controllers/controller/devworkspacerouting/util_test.go +++ b/controllers/controller/devworkspacerouting/util_test.go @@ -51,6 +51,9 @@ const ( endpointAnnotationKey = "endpoint-annotation-key" endpointAnnotationValue = "endpoint-annotation-value" + + serviceAnnotationKey = "service-annotation-key" + serviceAnnotationValue = "service-annotation-value" ) var ( @@ -96,6 +99,13 @@ func createDWR(workspaceID string, name string) *controllerv1alpha1.DevWorkspace DevWorkspaceId: workspaceID, RoutingClass: controllerv1alpha1.DevWorkspaceRoutingBasic, Endpoints: machineEndpointsMap, + Service: map[string]controllerv1alpha1.Service{ + testMachineName: { + Annotations: map[string]string{ + serviceAnnotationKey: serviceAnnotationValue, + }, + }, + }, PodSelector: map[string]string{ constants.DevWorkspaceIDLabel: workspaceID, }, diff --git a/docs/unsupported-devfile-api.adoc b/docs/unsupported-devfile-api.adoc index d23d5a3a0..85a4ce05b 100644 --- a/docs/unsupported-devfile-api.adoc +++ b/docs/unsupported-devfile-api.adoc @@ -1,11 +1,10 @@ -# Unsupported Devfile API += Unsupported Devfile API The following features of the Devfile API that are not yet supported by the DevWorkspace-Operator: [options="header"] |================================================================================================================================================================================================ | DevFile feature | Related issue -| `components.container.annotation.service` | https://github.com/devfile/devworkspace-operator/issues/799[Support setting annotations on services/endpoints from DevWorkspace] | `components.container.dedicatedPod` | | `components.image` | https://github.com/eclipse/che/issues/21186[Support Devfile v2 outer loop components of type image and kubernetes] | `components.custom` | diff --git a/pkg/provision/workspace/routing.go b/pkg/provision/workspace/routing.go index df14d89aa..0bbd667ce 100644 --- a/pkg/provision/workspace/routing.go +++ b/pkg/provision/workspace/routing.go @@ -77,6 +77,7 @@ func getSpecRouting( scheme *runtime.Scheme) (*v1alpha1.DevWorkspaceRouting, error) { endpoints := map[string]v1alpha1.EndpointList{} + serviceAnnotations := map[string]v1alpha1.Service{} for _, component := range workspace.Spec.Template.Components { if component.Container == nil { continue @@ -85,6 +86,14 @@ func getSpecRouting( if len(componentEndpoints) > 0 { endpoints[component.Name] = append(endpoints[component.Name], conversion.ConvertAllDevfileEndpoints(componentEndpoints)...) } + if component.Container.Annotation != nil { + componentService := component.Container.Annotation.Service + if len(componentService) > 0 { + serviceAnnotations[component.Name] = v1alpha1.Service{ + Annotations: componentService, + } + } + } } var annotations map[string]string @@ -119,6 +128,7 @@ func getSpecRouting( DevWorkspaceId: workspace.Status.DevWorkspaceId, RoutingClass: v1alpha1.DevWorkspaceRoutingClass(routingClass), Endpoints: endpoints, + Service: serviceAnnotations, PodSelector: map[string]string{ constants.DevWorkspaceIDLabel: workspace.Status.DevWorkspaceId, }, diff --git a/webhook/workspace/handler/testdata/warning/add-all-unsupported-features.yaml b/webhook/workspace/handler/testdata/warning/add-all-unsupported-features.yaml index 05c5614b0..f57093d3b 100644 --- a/webhook/workspace/handler/testdata/warning/add-all-unsupported-features.yaml +++ b/webhook/workspace/handler/testdata/warning/add-all-unsupported-features.yaml @@ -17,8 +17,6 @@ input: image: testing-image dedicatedPod: true annotation: - service: - key: value - name: projects volume: ephemeral: true @@ -43,5 +41,5 @@ input: - eventF output: - expectedWarning: "Unsupported Devfile features are present in this workspace. The following features will have no effect: components[].container.annotation.service, used by components: testing-container-1; components[].container.dedicatedPod, used by components: testing-container-1; components[].image, used by components: image-component; components[].custom, used by components: custom-component; events.postStop: eventD, eventE, eventF" + expectedWarning: "Unsupported Devfile features are present in this workspace. The following features will have no effect: components[].container.dedicatedPod, used by components: testing-container-1; components[].image, used by components: image-component; components[].custom, used by components: custom-component; events.postStop: eventD, eventE, eventF" newWarningsPresent: true diff --git a/webhook/workspace/handler/warning.go b/webhook/workspace/handler/warning.go index c3e37bfae..40f49e3a1 100644 --- a/webhook/workspace/handler/warning.go +++ b/webhook/workspace/handler/warning.go @@ -24,21 +24,19 @@ import ( ) type unsupportedWarnings struct { - serviceAnnotations map[string]bool - dedicatedPod map[string]bool - imageComponent map[string]bool - customComponent map[string]bool - eventPostStop map[string]bool + dedicatedPod map[string]bool + imageComponent map[string]bool + customComponent map[string]bool + eventPostStop map[string]bool } // Returns an initialized unsupportedWarnings struct func newUnsupportedWarnings() *unsupportedWarnings { return &unsupportedWarnings{ - serviceAnnotations: make(map[string]bool), - dedicatedPod: make(map[string]bool), - imageComponent: make(map[string]bool), - customComponent: make(map[string]bool), - eventPostStop: make(map[string]bool), + dedicatedPod: make(map[string]bool), + imageComponent: make(map[string]bool), + customComponent: make(map[string]bool), + eventPostStop: make(map[string]bool), } } @@ -46,9 +44,6 @@ func checkUnsupportedFeatures(devWorkspaceSpec dwv2.DevWorkspaceTemplateSpec) (w warnings = newUnsupportedWarnings() for _, component := range devWorkspaceSpec.Components { if component.Container != nil { - if component.Container.Annotation != nil && component.Container.Annotation.Service != nil { - warnings.serviceAnnotations[component.Name] = true - } if component.Container.DedicatedPod != nil && *component.Container.DedicatedPod { warnings.dedicatedPod[component.Name] = true } @@ -72,8 +67,7 @@ func checkUnsupportedFeatures(devWorkspaceSpec dwv2.DevWorkspaceTemplateSpec) (w } func unsupportedWarningsPresent(warnings *unsupportedWarnings) bool { - return len(warnings.serviceAnnotations) > 0 || - len(warnings.dedicatedPod) > 0 || + return len(warnings.dedicatedPod) > 0 || len(warnings.imageComponent) > 0 || len(warnings.customComponent) > 0 || len(warnings.eventPostStop) > 0 @@ -92,10 +86,6 @@ func formatUnsupportedFeaturesWarning(warnings *unsupportedWarnings) string { return warningNames } - if len(warnings.serviceAnnotations) > 0 { - serviceAnnotationsMsg := "components[].container.annotation.service, used by components: " + strings.Join(getWarningNames(warnings.serviceAnnotations), ", ") - msg = append(msg, serviceAnnotationsMsg) - } if len(warnings.dedicatedPod) > 0 { dedicatedPodMsg := "components[].container.dedicatedPod, used by components: " + strings.Join(getWarningNames(warnings.dedicatedPod), ", ") msg = append(msg, dedicatedPodMsg) @@ -132,7 +122,6 @@ func checkForAddedUnsupportedFeatures(oldWksp, newWksp *dwv2.DevWorkspace) *unsu return newWarningNames } - addedWarnings.serviceAnnotations = getAddedWarnings(oldWarnings.serviceAnnotations, newWarnings.serviceAnnotations) addedWarnings.dedicatedPod = getAddedWarnings(oldWarnings.dedicatedPod, newWarnings.dedicatedPod) addedWarnings.imageComponent = getAddedWarnings(oldWarnings.imageComponent, newWarnings.imageComponent) addedWarnings.customComponent = getAddedWarnings(oldWarnings.customComponent, newWarnings.customComponent)