Skip to content

Commit 836ed36

Browse files
rohanKanojiarohanKanojia
committed
feat : support service annotations in basic solver
Signed-off-by: Rohan Kumar <[email protected]>
1 parent 7af0176 commit 836ed36

File tree

10 files changed

+316
-38
lines changed

10 files changed

+316
-38
lines changed

‎controllers/controller/devworkspacerouting/devworkspacerouting_controller_test.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,7 @@ var _ = Describe("DevWorkspaceRouting Controller", func() {
114114
err := k8sClient.Get(ctx, serviceNamespacedName, createdService)
115115
return err == nil
116116
}, timeout, interval).Should(BeTrue(), "Service should exist in cluster")
117+
Expect(createdService.ObjectMeta.Annotations).Should(HaveKeyWithValue(serviceAnnotationKey, serviceAnnotationValue), "Service should have annotation")
117118
Expect(createdService.Spec.Selector).Should(Equal(createdDWR.Spec.PodSelector), "Service should have pod selector from DevWorkspace metadata")
118119
Expect(createdService.Labels).Should(Equal(ExpectedLabels), "Service should contain DevWorkspace ID label")
119120
expectedOwnerReference := devWorkspaceRoutingOwnerRef(createdDWR)

‎controllers/controller/devworkspacerouting/solvers/basic_solver.go

Lines changed: 22 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -43,12 +43,31 @@ var nginxIngressAnnotations = func(endpointName string, endpointAnnotations map[
4343
return annotations
4444
}
4545

46+
func mergeServiceAnnotations(sourceAnnotations map[string]string, serviceRoutingConfig controllerv1alpha1.Service) map[string]string {
47+
annotations := make(map[string]string)
48+
if sourceAnnotations != nil && len(sourceAnnotations) > 0 {
49+
for k, v := range sourceAnnotations {
50+
annotations[k] = v
51+
}
52+
}
53+
if serviceRoutingConfig.Annotations != nil && len(serviceRoutingConfig.Annotations) > 0 {
54+
for k, v := range serviceRoutingConfig.Annotations {
55+
annotations[k] = v
56+
}
57+
}
58+
return annotations
59+
}
60+
4661
// Basic solver exposes endpoints without any authentication
4762
// According to the current cluster there is different behavior:
4863
// Kubernetes: use Ingresses without TLS
4964
// OpenShift: use Routes with TLS enabled
5065
type BasicSolver struct{}
5166

67+
var routingSuffixSupplier = func() string {
68+
return config.GetGlobalConfig().Routing.ClusterHostSuffix
69+
}
70+
5271
var _ RoutingSolver = (*BasicSolver)(nil)
5372

5473
func (s *BasicSolver) FinalizerRequired(*controllerv1alpha1.DevWorkspaceRouting) bool {
@@ -63,14 +82,14 @@ func (s *BasicSolver) GetSpecObjects(routing *controllerv1alpha1.DevWorkspaceRou
6382
routingObjects := RoutingObjects{}
6483

6584
// TODO: Use workspace-scoped ClusterHostSuffix to allow overriding
66-
routingSuffix := config.GetGlobalConfig().Routing.ClusterHostSuffix
85+
routingSuffix := routingSuffixSupplier()
6786
if routingSuffix == "" {
6887
return routingObjects, &RoutingInvalid{"basic routing requires .config.routing.clusterHostSuffix to be set in operator config"}
6988
}
7089

7190
spec := routing.Spec
72-
services := getServicesForEndpoints(spec.Endpoints, workspaceMeta)
73-
services = append(services, GetDiscoverableServicesForEndpoints(spec.Endpoints, workspaceMeta)...)
91+
services := getServicesForEndpoints(spec, workspaceMeta)
92+
services = append(services, GetDiscoverableServicesForEndpoints(spec, workspaceMeta)...)
7493
routingObjects.Services = services
7594
if infrastructure.IsOpenShift() {
7695
routingObjects.Routes = getRoutesForSpec(routingSuffix, spec.Endpoints, workspaceMeta)

‎controllers/controller/devworkspacerouting/solvers/basic_solver_test.go

Lines changed: 247 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,247 @@
1+
// Copyright (c) 2019-2025 Red Hat, Inc.
2+
// Licensed under the Apache License, Version 2.0 (the "License");
3+
// you may not use this file except in compliance with the License.
4+
// You may obtain a copy of the License at
5+
//
6+
// http://www.apache.org/licenses/LICENSE-2.0
7+
//
8+
// Unless required by applicable law or agreed to in writing, software
9+
// distributed under the License is distributed on an "AS IS" BASIS,
10+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
11+
// See the License for the specific language governing permissions and
12+
// limitations under the License.
13+
14+
package solvers
15+
16+
import (
17+
"testing"
18+
19+
"github.com/devfile/devworkspace-operator/pkg/infrastructure"
20+
21+
"github.com/devfile/devworkspace-operator/apis/controller/v1alpha1"
22+
"github.com/stretchr/testify/assert"
23+
corev1 "k8s.io/api/core/v1"
24+
networkingv1 "k8s.io/api/networking/v1"
25+
apiext "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
26+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
27+
"k8s.io/apimachinery/pkg/util/intstr"
28+
)
29+
30+
func TestServiceAnnotations(t *testing.T) {
31+
tests := []struct {
32+
name string
33+
sourceAnnotations map[string]string
34+
serviceRoutingConfig v1alpha1.Service
35+
expectedAnnotations map[string]string
36+
}{
37+
{
38+
name: "No annotations provided should return empty",
39+
sourceAnnotations: nil,
40+
serviceRoutingConfig: v1alpha1.Service{
41+
Annotations: nil,
42+
},
43+
expectedAnnotations: map[string]string{},
44+
},
45+
{
46+
name: "Source annotations present should return source annotations",
47+
sourceAnnotations: map[string]string{
48+
"key1": "value1",
49+
"key2": "value2",
50+
},
51+
serviceRoutingConfig: v1alpha1.Service{
52+
Annotations: nil,
53+
},
54+
expectedAnnotations: map[string]string{
55+
"key1": "value1",
56+
"key2": "value2",
57+
},
58+
},
59+
{
60+
name: "DevWorkspaceRouting Service routing config annotations merged with source annotations",
61+
sourceAnnotations: map[string]string{
62+
"key1": "value1",
63+
},
64+
serviceRoutingConfig: v1alpha1.Service{
65+
Annotations: map[string]string{
66+
"key3": "value3",
67+
},
68+
},
69+
expectedAnnotations: map[string]string{
70+
"key1": "value1",
71+
"key3": "value3",
72+
},
73+
},
74+
}
75+
76+
for _, tt := range tests {
77+
t.Run(tt.name, func(t *testing.T) {
78+
// Given + When
79+
result := mergeServiceAnnotations(tt.sourceAnnotations, tt.serviceRoutingConfig)
80+
// Then
81+
assert.Equal(t, tt.expectedAnnotations, result)
82+
})
83+
}
84+
}
85+
86+
var devWorkspaceRouting = v1alpha1.DevWorkspaceRouting{
87+
Spec: v1alpha1.DevWorkspaceRoutingSpec{
88+
DevWorkspaceId: "workspaceb978dc9bd4ba428b",
89+
RoutingClass: "basic",
90+
Endpoints: map[string]v1alpha1.EndpointList{
91+
"component1": []v1alpha1.Endpoint{
92+
{
93+
Name: "endpoint1",
94+
TargetPort: 8080,
95+
Exposure: "public",
96+
Protocol: "http",
97+
Secure: false,
98+
Path: "/test",
99+
Attributes: map[string]apiext.JSON{},
100+
Annotations: map[string]string{
101+
"endpoint-annotation-key1": "endpoint-annotation-value1",
102+
},
103+
},
104+
},
105+
},
106+
PodSelector: map[string]string{
107+
"controller.devfile.io/devworkspace_id": "workspaceb978dc9bd4ba428b",
108+
},
109+
Service: map[string]v1alpha1.Service{
110+
"component1": {
111+
Annotations: map[string]string{
112+
"service-annotation-key": "service-annotation-value",
113+
},
114+
},
115+
},
116+
},
117+
}
118+
119+
func TestGetSpecObjects_WhenValidDWRProvidedAndOpenShiftUnavailable_ThenGenerateRoutingObjectsServiceAndIngress(t *testing.T) {
120+
// Given
121+
basicSolver := &BasicSolver{}
122+
routingSuffixSupplier = func() string {
123+
return "test.routing"
124+
}
125+
infrastructure.InitializeForTesting(infrastructure.Kubernetes)
126+
dwRouting := &devWorkspaceRouting
127+
workspaceMeta := DevWorkspaceMetadata{
128+
DevWorkspaceId: "workspaceb978dc9bd4ba428b",
129+
Namespace: "test",
130+
PodSelector: map[string]string{
131+
"controller.devfile.io/devworkspace_id": "workspaceb978dc9bd4ba428b",
132+
},
133+
}
134+
135+
// When
136+
routingObjects, err := basicSolver.GetSpecObjects(dwRouting, workspaceMeta)
137+
138+
// Then
139+
assert.NotNil(t, routingObjects)
140+
assert.NoError(t, err)
141+
assert.Len(t, routingObjects.Services, 1)
142+
assert.Equal(t, corev1.Service{
143+
ObjectMeta: metav1.ObjectMeta{
144+
Name: "workspaceb978dc9bd4ba428b-service",
145+
Namespace: "test",
146+
Labels: map[string]string{"controller.devfile.io/devworkspace_id": "workspaceb978dc9bd4ba428b"},
147+
Annotations: map[string]string{"service-annotation-key": "service-annotation-value"},
148+
},
149+
Spec: corev1.ServiceSpec{
150+
Type: corev1.ServiceTypeClusterIP,
151+
Ports: []corev1.ServicePort{
152+
{
153+
Name: "endpoint1",
154+
Protocol: corev1.ProtocolTCP,
155+
Port: 8080,
156+
TargetPort: intstr.IntOrString{IntVal: 8080},
157+
},
158+
},
159+
Selector: map[string]string{"controller.devfile.io/devworkspace_id": "workspaceb978dc9bd4ba428b"},
160+
},
161+
}, routingObjects.Services[0])
162+
assert.Len(t, routingObjects.Ingresses, 1)
163+
assert.Equal(t, metav1.ObjectMeta{
164+
Name: "workspaceb978dc9bd4ba428b-endpoint1",
165+
Namespace: "test",
166+
Labels: map[string]string{"controller.devfile.io/devworkspace_id": "workspaceb978dc9bd4ba428b"},
167+
Annotations: map[string]string{
168+
"controller.devfile.io/endpoint_name": "endpoint1",
169+
"endpoint-annotation-key1": "endpoint-annotation-value1",
170+
"nginx.ingress.kubernetes.io/rewrite-target": "/",
171+
"nginx.ingress.kubernetes.io/ssl-redirect": "false",
172+
},
173+
}, routingObjects.Ingresses[0].ObjectMeta)
174+
assert.Len(t, routingObjects.Ingresses[0].Spec.Rules, 1)
175+
assert.Equal(t, "workspaceb978dc9bd4ba428b-endpoint1-8080.test.routing", routingObjects.Ingresses[0].Spec.Rules[0].Host)
176+
assert.Len(t, routingObjects.Ingresses[0].Spec.Rules[0].HTTP.Paths, 1)
177+
assert.Equal(t, networkingv1.IngressBackend{
178+
Service: &networkingv1.IngressServiceBackend{
179+
Name: "workspaceb978dc9bd4ba428b-service",
180+
Port: networkingv1.ServiceBackendPort{Number: int32(8080)},
181+
},
182+
}, routingObjects.Ingresses[0].Spec.Rules[0].HTTP.Paths[0].Backend)
183+
assert.Len(t, routingObjects.Routes, 0)
184+
}
185+
186+
func TestGetSpecObjects_WhenValidDWRProvidedAndOpenShiftAvailable_ThenGenerateRoutingObjectsServiceAndRoute(t *testing.T) {
187+
// Given
188+
basicSolver := &BasicSolver{}
189+
routingSuffixSupplier = func() string {
190+
return "test.routing"
191+
}
192+
infrastructure.InitializeForTesting(infrastructure.OpenShiftv4)
193+
dwRouting := &devWorkspaceRouting
194+
workspaceMeta := DevWorkspaceMetadata{
195+
DevWorkspaceId: "workspaceb978dc9bd4ba428b",
196+
Namespace: "test",
197+
PodSelector: map[string]string{
198+
"controller.devfile.io/devworkspace_id": "workspaceb978dc9bd4ba428b",
199+
},
200+
}
201+
202+
// When
203+
routingObjects, err := basicSolver.GetSpecObjects(dwRouting, workspaceMeta)
204+
205+
// Then
206+
assert.NotNil(t, routingObjects)
207+
assert.NoError(t, err)
208+
assert.Len(t, routingObjects.Services, 1)
209+
assert.Equal(t, corev1.Service{
210+
ObjectMeta: metav1.ObjectMeta{
211+
Name: "workspaceb978dc9bd4ba428b-service",
212+
Namespace: "test",
213+
Labels: map[string]string{"controller.devfile.io/devworkspace_id": "workspaceb978dc9bd4ba428b"},
214+
Annotations: map[string]string{"service-annotation-key": "service-annotation-value"},
215+
},
216+
Spec: corev1.ServiceSpec{
217+
Type: corev1.ServiceTypeClusterIP,
218+
Ports: []corev1.ServicePort{
219+
{
220+
Name: "endpoint1",
221+
Protocol: corev1.ProtocolTCP,
222+
Port: 8080,
223+
TargetPort: intstr.IntOrString{IntVal: 8080},
224+
},
225+
},
226+
Selector: map[string]string{"controller.devfile.io/devworkspace_id": "workspaceb978dc9bd4ba428b"},
227+
},
228+
}, routingObjects.Services[0])
229+
assert.Len(t, routingObjects.Ingresses, 0)
230+
assert.Len(t, routingObjects.Routes, 1)
231+
assert.Equal(t, metav1.ObjectMeta{
232+
Name: "workspaceb978dc9bd4ba428b-endpoint1",
233+
Namespace: "test",
234+
Labels: map[string]string{
235+
"controller.devfile.io/devworkspace_id": "workspaceb978dc9bd4ba428b",
236+
},
237+
Annotations: map[string]string{
238+
"controller.devfile.io/endpoint_name": "endpoint1",
239+
"endpoint-annotation-key1": "endpoint-annotation-value1",
240+
"haproxy.router.openshift.io/rewrite-target": "/",
241+
},
242+
}, routingObjects.Routes[0].ObjectMeta)
243+
assert.Equal(t, "workspaceb978dc9bd4ba428b.test.routing", routingObjects.Routes[0].Spec.Host)
244+
assert.Equal(t, "/endpoint1/", routingObjects.Routes[0].Spec.Path)
245+
assert.Equal(t, "Service", routingObjects.Routes[0].Spec.To.Kind)
246+
assert.Equal(t, "workspaceb978dc9bd4ba428b-service", routingObjects.Routes[0].Spec.To.Name)
247+
}

‎controllers/controller/devworkspacerouting/solvers/cluster_solver.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ func (s *ClusterSolver) Finalize(*controllerv1alpha1.DevWorkspaceRouting) error
4646

4747
func (s *ClusterSolver) GetSpecObjects(routing *controllerv1alpha1.DevWorkspaceRouting, workspaceMeta DevWorkspaceMetadata) (RoutingObjects, error) {
4848
spec := routing.Spec
49-
services := getServicesForEndpoints(spec.Endpoints, workspaceMeta)
49+
services := getServicesForEndpoints(spec, workspaceMeta)
5050
podAdditions := &controllerv1alpha1.PodAdditions{}
5151
if s.TLS {
5252
readOnlyMode := int32(420)

‎controllers/controller/devworkspacerouting/solvers/common.go

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -36,9 +36,10 @@ type DevWorkspaceMetadata struct {
3636

3737
// GetDiscoverableServicesForEndpoints converts the endpoint list into a set of services, each corresponding to a single discoverable
3838
// endpoint from the list. Endpoints with the NoneEndpointExposure are ignored.
39-
func GetDiscoverableServicesForEndpoints(endpoints map[string]controllerv1alpha1.EndpointList, meta DevWorkspaceMetadata) []corev1.Service {
39+
func GetDiscoverableServicesForEndpoints(routingSpec controllerv1alpha1.DevWorkspaceRoutingSpec, meta DevWorkspaceMetadata) []corev1.Service {
4040
var services []corev1.Service
41-
for _, machineEndpoints := range endpoints {
41+
endpoints := routingSpec.Endpoints
42+
for componentName, machineEndpoints := range endpoints {
4243
for _, endpoint := range machineEndpoints {
4344
if endpoint.Exposure == controllerv1alpha1.NoneEndpointExposure {
4445
continue
@@ -61,9 +62,9 @@ func GetDiscoverableServicesForEndpoints(endpoints map[string]controllerv1alpha1
6162
Labels: map[string]string{
6263
constants.DevWorkspaceIDLabel: meta.DevWorkspaceId,
6364
},
64-
Annotations: map[string]string{
65+
Annotations: mergeServiceAnnotations(map[string]string{
6566
constants.DevWorkspaceDiscoverableServiceAnnotation: "true",
66-
},
67+
}, routingSpec.Service[componentName]),
6768
},
6869
Spec: corev1.ServiceSpec{
6970
Ports: []corev1.ServicePort{servicePort},
@@ -79,8 +80,9 @@ func GetDiscoverableServicesForEndpoints(endpoints map[string]controllerv1alpha1
7980

8081
// GetServiceForEndpoints returns a single service that exposes all endpoints of given exposure types, possibly also including the discoverable types.
8182
// `nil` is returned if the service would expose no ports satisfying the provided criteria.
82-
func GetServiceForEndpoints(endpoints map[string]controllerv1alpha1.EndpointList, meta DevWorkspaceMetadata, includeDiscoverable bool, exposureType ...controllerv1alpha1.EndpointExposure) *corev1.Service {
83+
func GetServiceForEndpoints(routingSpec controllerv1alpha1.DevWorkspaceRoutingSpec, meta DevWorkspaceMetadata, includeDiscoverable bool, exposureType ...controllerv1alpha1.EndpointExposure) *corev1.Service {
8384
// "set" of ports that are still left for exposure
85+
endpoints := routingSpec.Endpoints
8486
ports := map[int]bool{}
8587
for _, es := range endpoints {
8688
for _, endpoint := range es {
@@ -95,8 +97,9 @@ func GetServiceForEndpoints(endpoints map[string]controllerv1alpha1.EndpointList
9597
}
9698

9799
var exposedPorts []corev1.ServicePort
100+
var annotations = make(map[string]string)
98101

99-
for _, es := range endpoints {
102+
for componentName, es := range endpoints {
100103
for _, endpoint := range es {
101104
if !validExposures[endpoint.Exposure] {
102105
continue
@@ -117,6 +120,7 @@ func GetServiceForEndpoints(endpoints map[string]controllerv1alpha1.EndpointList
117120
})
118121
}
119122
}
123+
annotations = mergeServiceAnnotations(annotations, routingSpec.Service[componentName])
120124
}
121125

122126
if len(exposedPorts) == 0 {
@@ -130,6 +134,7 @@ func GetServiceForEndpoints(endpoints map[string]controllerv1alpha1.EndpointList
130134
Labels: map[string]string{
131135
constants.DevWorkspaceIDLabel: meta.DevWorkspaceId,
132136
},
137+
Annotations: annotations,
133138
},
134139
Spec: corev1.ServiceSpec{
135140
Selector: meta.PodSelector,
@@ -139,12 +144,12 @@ func GetServiceForEndpoints(endpoints map[string]controllerv1alpha1.EndpointList
139144
}
140145
}
141146

142-
func getServicesForEndpoints(endpoints map[string]controllerv1alpha1.EndpointList, meta DevWorkspaceMetadata) []corev1.Service {
143-
if len(endpoints) == 0 {
147+
func getServicesForEndpoints(routingSpec controllerv1alpha1.DevWorkspaceRoutingSpec, meta DevWorkspaceMetadata) []corev1.Service {
148+
if len(routingSpec.Endpoints) == 0 {
144149
return nil
145150
}
146151

147-
service := GetServiceForEndpoints(endpoints, meta, true, controllerv1alpha1.PublicEndpointExposure, controllerv1alpha1.InternalEndpointExposure)
152+
service := GetServiceForEndpoints(routingSpec, meta, true, controllerv1alpha1.PublicEndpointExposure, controllerv1alpha1.InternalEndpointExposure)
148153
if service == nil {
149154
return nil
150155
}

0 commit comments

Comments
 (0)