Make OpenShift routes work with missing hostname (#2074)

* Make OpenShift routes work with missing hostname

Signed-off-by: Pavol Loffay <[email protected]>

* Fix

Signed-off-by: Pavol Loffay <[email protected]>

* Fix

Signed-off-by: Pavol Loffay <[email protected]>

* Fix the changelog entry

Signed-off-by: Pavol Loffay <[email protected]>

* Update .chloggen/route-use-defaults.yaml

Co-authored-by: Israel Blancas <[email protected]>

---------

Signed-off-by: Pavol Loffay <[email protected]>
Co-authored-by: Ben B <[email protected]>
Co-authored-by: Israel Blancas <[email protected]>

.chloggen/route-use-defaults.yaml

@@ -0,0 +1,16 @@
+# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
+change_type: enhancement
+
+# The name of the component, or a single word describing the area of concern, (e.g. operator, target allocator, github action)
+component: operator
+
+# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
+note: Make OpenShift routes work with missing hostname
+
+# One or more tracking issues related to the change
+issues: [2074]
+
+# (Optional) One or more lines of additional information to render under the primary note.
+# These lines will be padded with 2 spaces and then inserted directly into the document.
+# Use pipe (|) for multiline entries.
+subtext: If the Ingress hostname is not specified OpenShift route hostname is set to `<port-name>-<otel-cr-name>-route-<otel-cr-namespace>-basedomain`.

internal/manifests/collector/route.go

@@ -66,11 +66,11 @@ func Routes(cfg config.Config, logger logr.Logger, otelcol v1alpha1.OpenTelemetr
 	routes := make([]*routev1.Route, len(ports))
 	for i, p := range ports {
 		portName := naming.PortName(p.Name, p.Port)
-		path := "/"
-		//  passthrough termination does not support paths
-		if otelcol.Spec.Ingress.Route.Termination == v1alpha1.TLSRouteTerminationTypePassthrough {
-			path = ""
+		host := ""
+		if otelcol.Spec.Ingress.Hostname != "" {
+			host = fmt.Sprintf("%s.%s", portName, otelcol.Spec.Ingress.Hostname)
 		}
+
 		routes[i] = &routev1.Route{
 			ObjectMeta: metav1.ObjectMeta{
 				Name:        naming.Route(otelcol.Name, p.Name),
@@ -83,8 +83,7 @@ func Routes(cfg config.Config, logger logr.Logger, otelcol v1alpha1.OpenTelemetr
 				},
 			},
 			Spec: routev1.RouteSpec{
-				Host: fmt.Sprintf("%s.%s", portName, otelcol.Spec.Ingress.Hostname),
-				Path: path,
+				Host: host,
 				To: routev1.RouteTargetReference{
 					Kind: "Service",
 					Name: naming.Service(otelcol.Name),

internal/manifests/collector/route_test.go

@@ -21,6 +21,7 @@ import (
 
 	routev1 "github.com/openshift/api/route/v1"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/util/intstr"
 
@@ -137,6 +138,47 @@ func TestDesiredRoutes(t *testing.T) {
 			},
 		}, got)
 	})
+	t.Run("hostname is set", func(t *testing.T) {
+		params, err := newParams("something:tag", testFileIngress)
+		if err != nil {
+			t.Fatal(err)
+		}
+
+		params.Instance.Namespace = "test"
+		params.Instance.Spec.Ingress = v1alpha1.Ingress{
+			Hostname: "example.com",
+			Type:     v1alpha1.IngressTypeRoute,
+			Route: v1alpha1.OpenShiftRoute{
+				Termination: v1alpha1.TLSRouteTerminationTypeInsecure,
+			},
+		}
+
+		routes := Routes(params.Config, params.Log, params.Instance)
+		require.Equal(t, 3, len(routes))
+		assert.Equal(t, "web.example.com", routes[0].Spec.Host)
+		assert.Equal(t, "otlp-grpc.example.com", routes[1].Spec.Host)
+		assert.Equal(t, "otlp-test-grpc.example.com", routes[2].Spec.Host)
+	})
+	t.Run("hostname is not set", func(t *testing.T) {
+		params, err := newParams("something:tag", testFileIngress)
+		if err != nil {
+			t.Fatal(err)
+		}
+
+		params.Instance.Namespace = "test"
+		params.Instance.Spec.Ingress = v1alpha1.Ingress{
+			Type: v1alpha1.IngressTypeRoute,
+			Route: v1alpha1.OpenShiftRoute{
+				Termination: v1alpha1.TLSRouteTerminationTypeInsecure,
+			},
+		}
+
+		routes := Routes(params.Config, params.Log, params.Instance)
+		require.Equal(t, 3, len(routes))
+		assert.Equal(t, "", routes[0].Spec.Host)
+		assert.Equal(t, "", routes[1].Spec.Host)
+		assert.Equal(t, "", routes[2].Spec.Host)
+	})
 }
 
 func TestRoutes(t *testing.T) {

tests/e2e-openshift/route/00-assert.yaml

@@ -22,11 +22,32 @@ metadata:
     kind: OpenTelemetryCollector
     name: simplest
 spec:
-  host: otlp-grpc.example.com
-  path: /
   port:
     targetPort: otlp-grpc
   to:
     kind: Service
     name: simplest-collector
   wildcardPolicy: None
+---
+apiVersion: route.openshift.io/v1
+kind: Route
+metadata:
+  annotations:
+    something.com: "true"
+  labels:
+    app.kubernetes.io/managed-by: opentelemetry-operator
+    app.kubernetes.io/name: otlp-http-simplest-route
+  name: otlp-http-simplest-route
+  ownerReferences:
+    - apiVersion: opentelemetry.io/v1alpha1
+      blockOwnerDeletion: true
+      controller: true
+      kind: OpenTelemetryCollector
+      name: simplest
+spec:
+  port:
+    targetPort: otlp-http
+  to:
+    kind: Service
+    name: simplest-collector
+  wildcardPolicy: None

tests/e2e-openshift/route/00-install.yaml

@@ -7,7 +7,6 @@ spec:
   mode: "deployment"
   ingress:
     type: route
-    hostname: "example.com"
     annotations:
       something.com: "true"
     route:
@@ -18,6 +17,7 @@ spec:
       otlp:
         protocols:
           grpc:
+          http:
 
     exporters:
       logging:

tests/e2e-openshift/route/01-report-empty-otlphttp-spans.yaml

@@ -0,0 +1,9 @@
+apiVersion: kuttl.dev/v1beta1
+kind: TestStep
+commands:
+  - script: |
+      #!/bin/bash
+      set -ex
+      # Export empty payload and check of collector accepted it with 2xx status code
+      otlp_http_host=$(kubectl get route otlp-http-simplest-route -n $NAMESPACE -o jsonpath='{.spec.host}')
+      for i in {1..40}; do curl --fail -ivX POST http://${otlp_http_host}:80/v1/traces -H "Content-Type: application/json" -d '{}' && break || sleep 1; done