feat : Add support to add Service annotations from DevWorkspaceRouting configuration (#1293)

CHANGELOG.md

@@ -1,5 +1,18 @@
 # DevWorkspace Operator Changelog
 
+
+# Unreleased
+## Bug Fixes & Improvements
+- Remove `kube-rbac-proxy` sidecar metric proxy container from Operator deployment [#1343](https://github.com/devfile/devworkspace-operator/issues/1343)
+
+## Features
+- Add support for adding Service annotations from DevWorkspace component configuration to actual Service created by DevWorkspace operator [#1293](https://github.com/devfile/devworkspace-operator/issues/1293)
+
+## **Breaking Changes**
+- There are minor changes in signatures of these methods in `github.com/devfile/devworkspace-operator/controllers/controller/devworkspacerouting/solvers` package
+  - `GetDiscoverableServicesForEndpoints` now expects `controllerv1alpha1.DevWorkspaceRoutingSpec` as first argument, instead of `map[string]controllerv1alpha1.EndpointList`
+  - `GetServiceForEndpoints` now expects `controllerv1alpha1.DevWorkspaceRoutingSpec` as first argument, instead of `map[string]controllerv1alpha1.EndpointList`
+
 # v0.35.1
 ## Bug Fixes & Improvements
 - Reverted [#1269](https://github.com/devfile/devworkspace-operator/issues/1269) due to [#1453](https://github.com/devfile/devworkspace-operator/issues/1453)

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

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)

controllers/controller/devworkspacerouting/solvers/basic_solver.go

@@ -43,12 +43,31 @@ var nginxIngressAnnotations = func(endpointName string, endpointAnnotations map[
 	return annotations
 }
 
+func mergeServiceAnnotations(sourceAnnotations map[string]string, 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 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 _ RoutingSolver = (*BasicSolver)(nil)
 
 func (s *BasicSolver) FinalizerRequired(*controllerv1alpha1.DevWorkspaceRouting) bool {
@@ -63,14 +82,14 @@ 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() {
 		routingObjects.Routes = getRoutesForSpec(routingSuffix, spec.Endpoints, workspaceMeta)

controllers/controller/devworkspacerouting/solvers/basic_solver_test.go

@@ -0,0 +1,247 @@
+// 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/pkg/infrastructure"
+
+	"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
+		serviceRoutingConfig v1alpha1.Service
+		expectedAnnotations  map[string]string
+	}{
+		{
+			name:              "No annotations provided should return empty",
+			sourceAnnotations: nil,
+			serviceRoutingConfig: v1alpha1.Service{
+				Annotations: nil,
+			},
+			expectedAnnotations: map[string]string{},
+		},
+		{
+			name: "Source annotations present should return source annotations",
+			sourceAnnotations: map[string]string{
+				"key1": "value1",
+				"key2": "value2",
+			},
+			serviceRoutingConfig: v1alpha1.Service{
+				Annotations: nil,
+			},
+			expectedAnnotations: map[string]string{
+				"key1": "value1",
+				"key2": "value2",
+			},
+		},
+		{
+			name: "DevWorkspaceRouting Service routing config annotations merged with source annotations",
+			sourceAnnotations: map[string]string{
+				"key1": "value1",
+			},
+			serviceRoutingConfig: v1alpha1.Service{
+				Annotations: map[string]string{
+					"key3": "value3",
+				},
+			},
+			expectedAnnotations: map[string]string{
+				"key1": "value1",
+				"key3": "value3",
+			},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			// Given + When
+			result := mergeServiceAnnotations(tt.sourceAnnotations, 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"
+	}
+	infrastructure.InitializeForTesting(infrastructure.Kubernetes)
+	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"
+	}
+	infrastructure.InitializeForTesting(infrastructure.OpenShiftv4)
+	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)
+}