fix(service-proxy): enhance Service Proxy URL formatting

cmd/service-proxy/proxy.go

@@ -132,11 +132,14 @@ func (pm *ProxyManager) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.R
 	for _, plugin := range plugins {
 		for url, svc := range plugin.Status.ExposedServices {
 			u := *k8sAPIURL // copy URL struct
-			proto := "http"
-			if svc.Protocol != nil {
-				proto = *svc.Protocol
+
+			if svc.Protocol != nil && *svc.Protocol == "https" {
+				// For HTTPS, format should be: https:<service_name>:<port>
+				u.Path = fmt.Sprintf("/api/v1/namespaces/%s/services/https:%s:%d/proxy", svc.Namespace, svc.Name, svc.Port)
+			} else {
+				// For HTTP, format should be: <service_name>:<port>
+				u.Path = fmt.Sprintf("/api/v1/namespaces/%s/services/%s:%d/proxy", svc.Namespace, svc.Name, svc.Port)
 			}
-			u.Path = fmt.Sprintf("/api/v1/namespaces/%s/services/%s:%s:%d/proxy", svc.Namespace, proto, svc.Name, svc.Port)
 			cls.routes[url] = route{url: &u, namespace: svc.Namespace, serviceName: svc.Name}
 		}
 	}

cmd/service-proxy/proxy_test.go

@@ -5,7 +5,6 @@ package main
 
 import (
 	"context"
-	"fmt"
 	"net/http"
 	"net/http/httputil"
 	"net/url"
@@ -92,39 +91,61 @@ func TestRewrite(t *testing.T) {
 	}
 }
 
-// TestReconcile tests the reconcile function of the proxy manager.
-// It injects a client from  sigs.k8s.io/controller-runtime/pkg/client/fake into the proxy manager and
-// sets up a cluster and a plugin with an exposed service in the fake client.
-// The test checks if the route is properly added to the cluster.
-func TestReconcile(t *testing.T) {
-	pm := NewProxyManager()
-	pm.client = fake.NewClientBuilder().WithScheme(test.GreenhouseV1Alpha1Scheme()).WithObjects(
-		&greenhousev1alpha1.Plugin{
-			ObjectMeta: metav1.ObjectMeta{
-				Name:      "plugin1",
-				Namespace: "namespace",
-			},
-			Spec: greenhousev1alpha1.PluginSpec{
-				ClusterName: "cluster-1",
-			},
-			Status: greenhousev1alpha1.PluginStatus{
-				ExposedServices: map[string]greenhousev1alpha1.Service{
-					"https://cluster-1--1234567.org.basedomain": {
+func TestURLGenerationWithProtocols(t *testing.T) {
+	// Test cases to cover different protocol scenarios
+	tests := []struct {
+		name            string
+		protocol        *string
+		expectedURLPath string
+	}{
+		{
+			name:            "default_no_protocol",
+			protocol:        nil,
+			expectedURLPath: "/api/v1/namespaces/namespace/services/test:8080/proxy",
+		},
+		{
+			name:            "explicit_http_protocol",
+			protocol:        pointer("http"),
+			expectedURLPath: "/api/v1/namespaces/namespace/services/test:8080/proxy",
+		},
+		{
+			name:            "explicit_https_protocol",
+			protocol:        pointer("https"),
+			expectedURLPath: "/api/v1/namespaces/namespace/services/https:test:8080/proxy",
+		},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			pm := NewProxyManager()
+			pm.client = fake.NewClientBuilder().WithScheme(test.GreenhouseV1Alpha1Scheme()).WithObjects(
+				&greenhousev1alpha1.Plugin{
+					ObjectMeta: metav1.ObjectMeta{
+						Name:      "plugin1",
 						Namespace: "namespace",
-						Name:      "test",
-						Port:      8080,
+					},
+					Spec: greenhousev1alpha1.PluginSpec{
+						ClusterName: "cluster-1",
+					},
+					Status: greenhousev1alpha1.PluginStatus{
+						ExposedServices: map[string]greenhousev1alpha1.Service{
+							"https://cluster-1--1234567.org.basedomain": {
+								Namespace: "namespace",
+								Name:      "test",
+								Port:      8080,
+								Protocol:  tc.protocol,
+							},
+						},
 					},
 				},
-			},
-		},
-		&v1.Secret{
-			ObjectMeta: metav1.ObjectMeta{
-				Name:      "cluster-1",
-				Namespace: "namespace",
-			},
-			Type: "greenhouse.sap/kubeconfig",
-			Data: map[string][]byte{
-				greenhouseapis.GreenHouseKubeConfigKey: []byte(`
+				&v1.Secret{
+					ObjectMeta: metav1.ObjectMeta{
+						Name:      "cluster-1",
+						Namespace: "namespace",
+					},
+					Type: "greenhouse.sap/kubeconfig",
+					Data: map[string][]byte{
+						greenhouseapis.GreenHouseKubeConfigKey: []byte(`
 kind: Config
 apiVersion: v1
 clusters:
@@ -140,22 +161,31 @@ current-context: context1
 users:
 - name: user1
 `),
-			},
-		}).Build()
-	ctx := context.Background()
-	_, err := pm.Reconcile(ctx, ctrl.Request{NamespacedName: types.NamespacedName{Name: "cluster-1", Namespace: "namespace"}})
+					},
+				}).Build()
 
-	if err != nil {
-		t.Errorf("expected no error, got: %s", err)
-	}
+			ctx := context.Background()
+			_, err := pm.Reconcile(ctx, ctrl.Request{NamespacedName: types.NamespacedName{Name: "cluster-1", Namespace: "namespace"}})
 
-	route, ok := pm.clusters["cluster-1"].routes["https://cluster-1--1234567.org.basedomain"]
-	if !ok {
-		t.Fatal("expected route to be added")
-	}
-	targetURL := route.url
-	expectedURL := fmt.Sprintf("%s/api/v1/namespaces/%s/services/%s:%s:%s/proxy", "https://apiserver.test", "namespace", "http", "test", "8080")
-	if targetURL.String() != expectedURL {
-		t.Errorf("expected url %s, got %s", expectedURL, targetURL.String())
+			if err != nil {
+				t.Errorf("expected no error, got: %s", err)
+			}
+
+			route, ok := pm.clusters["cluster-1"].routes["https://cluster-1--1234567.org.basedomain"]
+			if !ok {
+				t.Fatal("expected route to be added")
+			}
+
+			targetURL := route.url
+			expectedURL := "https://apiserver.test" + tc.expectedURLPath
+			if targetURL.String() != expectedURL {
+				t.Errorf("expected url %s, got %s", expectedURL, targetURL.String())
+			}
+		})
 	}
 }
+
+// helper function to create string pointer
+func pointer(s string) *string {
+	return &s
+}