Make sure port name in ingress and route match service

Port name in service/ingress/route can be longer than 15 characters. The port name 15 characters is enforced in ports on pods. Signed-off-by: Pavol Loffay <[email protected]>
open-telemetry · Jul 28, 2023 · 777ac3f · 777ac3f
1 parent 42ff92f
commit 777ac3f
Show file tree

Hide file tree

Showing 27 changed files with 180 additions and 97 deletions.
diff --git a/.chloggen/1954.yaml b/.chloggen/1954.yaml
@@ -0,0 +1,16 @@
+# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
+change_type: bug_fix
+
+# 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: Fix port name matching between ingress/route and service
+
+# One or more tracking issues related to the change
+issues: [1954]
+
+# (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:
diff --git a/pkg/collector/daemonset.go b/pkg/collector/daemonset.go
@@ -27,14 +27,14 @@ import (
 
 // DaemonSet builds the deployment for the given instance.
 func DaemonSet(cfg config.Config, logger logr.Logger, otelcol v1alpha1.OpenTelemetryCollector) appsv1.DaemonSet {
-	name := naming.Collector(otelcol)
+	name := naming.Collector(otelcol.Name)
 	labels := Labels(otelcol, name, cfg.LabelsFilter())
 
 	annotations := Annotations(otelcol)
 	podAnnotations := PodAnnotations(otelcol)
 	return appsv1.DaemonSet{
 		ObjectMeta: metav1.ObjectMeta{
-			Name:        naming.Collector(otelcol),
+			Name:        naming.Collector(otelcol.Name),
 			Namespace:   otelcol.Namespace,
 			Labels:      labels,
 			Annotations: annotations,

diff --git a/pkg/collector/deployment.go b/pkg/collector/deployment.go
@@ -27,7 +27,7 @@ import (
 
 // Deployment builds the deployment for the given instance.
 func Deployment(cfg config.Config, logger logr.Logger, otelcol v1alpha1.OpenTelemetryCollector) appsv1.Deployment {
-	name := naming.Collector(otelcol)
+	name := naming.Collector(otelcol.Name)
 	labels := Labels(otelcol, name, cfg.LabelsFilter())
 
 	annotations := Annotations(otelcol)

diff --git a/pkg/collector/horizontalpodautoscaler.go b/pkg/collector/horizontalpodautoscaler.go
@@ -31,13 +31,13 @@ import (
 func HorizontalPodAutoscaler(cfg config.Config, logger logr.Logger, otelcol v1alpha1.OpenTelemetryCollector) client.Object {
 	autoscalingVersion := cfg.AutoscalingVersion()
 
-	name := naming.Collector(otelcol)
+	name := naming.Collector(otelcol.Name)
 	labels := Labels(otelcol, name, cfg.LabelsFilter())
 	annotations := Annotations(otelcol)
 	var result client.Object
 
 	objectMeta := metav1.ObjectMeta{
-		Name:        naming.HorizontalPodAutoscaler(otelcol),
+		Name:        naming.HorizontalPodAutoscaler(otelcol.Name),
 		Namespace:   otelcol.Namespace,
 		Labels:      labels,
 		Annotations: annotations,
@@ -96,7 +96,7 @@ func HorizontalPodAutoscaler(cfg config.Config, logger logr.Logger, otelcol v1al
 				ScaleTargetRef: autoscalingv2beta2.CrossVersionObjectReference{
 					APIVersion: v1alpha1.GroupVersion.String(),
 					Kind:       "OpenTelemetryCollector",
-					Name:       naming.OpenTelemetryCollector(otelcol),
+					Name:       naming.OpenTelemetryCollector(otelcol.Name),
 				},
 				MinReplicas: otelcol.Spec.Autoscaler.MinReplicas,
 				MaxReplicas: *otelcol.Spec.Autoscaler.MaxReplicas,
@@ -153,7 +153,7 @@ func HorizontalPodAutoscaler(cfg config.Config, logger logr.Logger, otelcol v1al
 				ScaleTargetRef: autoscalingv2.CrossVersionObjectReference{
 					APIVersion: v1alpha1.GroupVersion.String(),
 					Kind:       "OpenTelemetryCollector",
-					Name:       naming.OpenTelemetryCollector(otelcol),
+					Name:       naming.OpenTelemetryCollector(otelcol.Name),
 				},
 				MinReplicas: otelcol.Spec.Autoscaler.MinReplicas,
 				MaxReplicas: *otelcol.Spec.Autoscaler.MaxReplicas,

diff --git a/pkg/collector/parser/receiver.go b/pkg/collector/parser/receiver.go
@@ -18,6 +18,7 @@ package parser
 import (
 	"errors"
 	"fmt"
+	"github.com/open-telemetry/opentelemetry-operator/pkg/naming"
 	"regexp"
 	"strconv"
 	"strings"
@@ -159,7 +160,7 @@ func singlePortFromConfigEndpoint(logger logr.Logger, name string, config map[in
 		}
 
 		return &corev1.ServicePort{
-			Name: portName(name, port),
+			Name: naming.PortName(name, port),
 			Port: port,
 		}
 	default:
@@ -178,22 +179,6 @@ func getAddressFromConfig(logger logr.Logger, name, key string, config map[inter
 	return endpoint
 }
 
-func portName(receiverName string, port int32) string {
-	if len(receiverName) > 63 {
-		return fmt.Sprintf("port-%d", port)
-	}
-
-	candidate := strings.ReplaceAll(receiverName, "/", "-")
-	candidate = strings.ReplaceAll(candidate, "_", "-")
-
-	if !dnsLabelValidation.MatchString(candidate) {
-		return fmt.Sprintf("port-%d", port)
-	}
-
-	// matches the pattern and has less than 63 chars -- the candidate name is good to go!
-	return candidate
-}
-
 func portFromEndpoint(endpoint string) (int32, error) {
 	var err error
 	var port int64

diff --git a/pkg/collector/parser/receiver_generic.go b/pkg/collector/parser/receiver_generic.go
@@ -16,6 +16,7 @@ package parser
 
 import (
 	"github.com/go-logr/logr"
+	"github.com/open-telemetry/opentelemetry-operator/pkg/naming"
 	corev1 "k8s.io/api/core/v1"
 )
 
@@ -59,7 +60,7 @@ func (g *GenericReceiver) Ports() ([]corev1.ServicePort, error) {
 	if g.defaultPort > 0 {
 		return []corev1.ServicePort{{
 			Port:        g.defaultPort,
-			Name:        portName(g.name, g.defaultPort),
+			Name:        naming.PortName(g.name, g.defaultPort),
 			Protocol:    g.defaultProtocol,
 			AppProtocol: g.defaultAppProtocol,
 		}}, nil

diff --git a/pkg/collector/parser/receiver_jaeger.go b/pkg/collector/parser/receiver_jaeger.go
@@ -16,6 +16,7 @@ package parser
 
 import (
 	"fmt"
+	"github.com/open-telemetry/opentelemetry-operator/pkg/naming"
 
 	"github.com/go-logr/logr"
 	corev1 "k8s.io/api/core/v1"
@@ -104,7 +105,7 @@ func (j *JaegerReceiverParser) Ports() ([]corev1.ServicePort, error) {
 			// if not, we use the default port
 			if protocolPort == nil {
 				protocolPort = &corev1.ServicePort{
-					Name: portName(nameWithProtocol, protocol.defaultPort),
+					Name: naming.PortName(nameWithProtocol, protocol.defaultPort),
 					Port: protocol.defaultPort,
 				}
 			}

diff --git a/pkg/collector/parser/receiver_otlp.go b/pkg/collector/parser/receiver_otlp.go
@@ -16,6 +16,7 @@ package parser
 
 import (
 	"fmt"
+	"github.com/open-telemetry/opentelemetry-operator/pkg/naming"
 
 	"github.com/go-logr/logr"
 	corev1 "k8s.io/api/core/v1"
@@ -72,7 +73,7 @@ func (o *OTLPReceiverParser) Ports() ([]corev1.ServicePort, error) {
 			name: grpc,
 			defaultPorts: []corev1.ServicePort{
 				{
-					Name:        portName(fmt.Sprintf("%s-grpc", o.name), defaultOTLPGRPCPort),
+					Name:        naming.PortName(fmt.Sprintf("%s-grpc", o.name), defaultOTLPGRPCPort),
 					Port:        defaultOTLPGRPCPort,
 					TargetPort:  intstr.FromInt(int(defaultOTLPGRPCPort)),
 					AppProtocol: &grpc,
@@ -83,13 +84,13 @@ func (o *OTLPReceiverParser) Ports() ([]corev1.ServicePort, error) {
 			name: http,
 			defaultPorts: []corev1.ServicePort{
 				{
-					Name:        portName(fmt.Sprintf("%s-http", o.name), defaultOTLPHTTPPort),
+					Name:        naming.PortName(fmt.Sprintf("%s-http", o.name), defaultOTLPHTTPPort),
 					Port:        defaultOTLPHTTPPort,
 					TargetPort:  intstr.FromInt(int(defaultOTLPHTTPPort)),
 					AppProtocol: &http,
 				},
 				{
-					Name:        portName(fmt.Sprintf("%s-http-legacy", o.name), defaultOTLPHTTPLegacyPort),
+					Name:        naming.PortName(fmt.Sprintf("%s-http-legacy", o.name), defaultOTLPHTTPLegacyPort),
 					Port:        defaultOTLPHTTPLegacyPort,
 					TargetPort:  intstr.FromInt(int(defaultOTLPHTTPPort)), // we target the official port, not the legacy
 					AppProtocol: &http,

diff --git a/pkg/collector/parser/receiver_skywalking.go b/pkg/collector/parser/receiver_skywalking.go
@@ -16,6 +16,7 @@ package parser
 
 import (
 	"fmt"
+	"github.com/open-telemetry/opentelemetry-operator/pkg/naming"
 
 	"github.com/go-logr/logr"
 	corev1 "k8s.io/api/core/v1"
@@ -66,7 +67,7 @@ func (o *SkywalkingReceiverParser) Ports() ([]corev1.ServicePort, error) {
 			name: grpc,
 			defaultPorts: []corev1.ServicePort{
 				{
-					Name:        portName(fmt.Sprintf("%s-grpc", o.name), defaultSkywalkingGRPCPort),
+					Name:        naming.PortName(fmt.Sprintf("%s-grpc", o.name), defaultSkywalkingGRPCPort),
 					Port:        defaultSkywalkingGRPCPort,
 					TargetPort:  intstr.FromInt(int(defaultSkywalkingGRPCPort)),
 					AppProtocol: &grpc,
@@ -77,7 +78,7 @@ func (o *SkywalkingReceiverParser) Ports() ([]corev1.ServicePort, error) {
 			name: http,
 			defaultPorts: []corev1.ServicePort{
 				{
-					Name:        portName(fmt.Sprintf("%s-http", o.name), defaultSkywalkingHTTPPort),
+					Name:        naming.PortName(fmt.Sprintf("%s-http", o.name), defaultSkywalkingHTTPPort),
 					Port:        defaultSkywalkingHTTPPort,
 					TargetPort:  intstr.FromInt(int(defaultSkywalkingHTTPPort)),
 					AppProtocol: &http,

diff --git a/pkg/collector/parser/receiver_test.go b/pkg/collector/parser/receiver_test.go
@@ -15,6 +15,7 @@
 package parser
 
 import (
+	"github.com/open-telemetry/opentelemetry-operator/pkg/naming"
 	"testing"
 
 	"github.com/go-logr/logr"
@@ -38,7 +39,7 @@ func TestReceiverPortNames(t *testing.T) {
 		{"name starting with invalid char", "-my-receiver", "port-123", 123},
 	} {
 		t.Run(tt.desc, func(t *testing.T) {
-			assert.Equal(t, tt.expected, portName(tt.candidate, int32(tt.port)))
+			assert.Equal(t, tt.expected, naming.PortName(tt.candidate, int32(tt.port)))
 		})
 	}
 }

diff --git a/pkg/collector/reconcile/config_replace.go b/pkg/collector/reconcile/config_replace.go
@@ -66,7 +66,7 @@ func ReplaceConfig(instance v1alpha1.OpenTelemetryCollector) (string, error) {
 	if featuregate.EnableTargetAllocatorRewrite.IsEnabled() {
 		// To avoid issues caused by Prometheus validation logic, which fails regex validation when it encounters
 		// $$ in the prom config, we update the YAML file directly without marshaling and unmarshalling.
-		updPromCfgMap, getCfgPromErr := ta.AddTAConfigToPromConfig(promCfgMap, naming.TAService(instance))
+		updPromCfgMap, getCfgPromErr := ta.AddTAConfigToPromConfig(promCfgMap, naming.TAService(instance.Name))
 		if getCfgPromErr != nil {
 			return "", getCfgPromErr
 		}
@@ -84,7 +84,7 @@ func ReplaceConfig(instance v1alpha1.OpenTelemetryCollector) (string, error) {
 
 	// To avoid issues caused by Prometheus validation logic, which fails regex validation when it encounters
 	// $$ in the prom config, we update the YAML file directly without marshaling and unmarshalling.
-	updPromCfgMap, err := ta.AddHTTPSDConfigToPromConfig(promCfgMap, naming.TAService(instance))
+	updPromCfgMap, err := ta.AddHTTPSDConfigToPromConfig(promCfgMap, naming.TAService(instance.Name))
 	if err != nil {
 		return "", err
 	}

diff --git a/pkg/collector/reconcile/configmap.go b/pkg/collector/reconcile/configmap.go
@@ -61,7 +61,7 @@ func ConfigMaps(ctx context.Context, params Params) error {
 }
 
 func desiredConfigMap(_ context.Context, params Params) corev1.ConfigMap {
-	name := naming.ConfigMap(params.Instance)
+	name := naming.ConfigMap(params.Instance.Name)
 	labels := collector.Labels(params.Instance, name, []string{})
 
 	config, err := ReplaceConfig(params.Instance)

diff --git a/pkg/collector/reconcile/ingress.go b/pkg/collector/reconcile/ingress.go
@@ -56,10 +56,9 @@ func desiredIngresses(_ context.Context, params Params) *networkingv1.Ingress {
 			PathType: &pathType,
 			Backend: networkingv1.IngressBackend{
 				Service: &networkingv1.IngressServiceBackend{
-					Name: naming.Service(params.Instance),
+					Name: naming.Service(params.Instance.Name),
 					Port: networkingv1.ServiceBackendPort{
-						// Valid names must be non-empty and no more than 15 characters long.
-						Name: naming.Truncate(p.Name, 15),
+						Name: naming.PortName(p.Name, p.Port),
 					},
 				},
 			},
@@ -68,11 +67,11 @@ func desiredIngresses(_ context.Context, params Params) *networkingv1.Ingress {
 
 	return &networkingv1.Ingress{
 		ObjectMeta: metav1.ObjectMeta{
-			Name:        naming.Ingress(params.Instance),
+			Name:        naming.Ingress(params.Instance.Name),
 			Namespace:   params.Instance.Namespace,
 			Annotations: params.Instance.Spec.Ingress.Annotations,
 			Labels: map[string]string{
-				"app.kubernetes.io/name":       naming.Ingress(params.Instance),
+				"app.kubernetes.io/name":       naming.Ingress(params.Instance.Name),
 				"app.kubernetes.io/instance":   fmt.Sprintf("%s.%s", params.Instance.Namespace, params.Instance.Name),
 				"app.kubernetes.io/managed-by": "opentelemetry-operator",
 			},

diff --git a/pkg/collector/reconcile/ingress_test.go b/pkg/collector/reconcile/ingress_test.go
@@ -115,11 +115,11 @@ func TestDesiredIngresses(t *testing.T) {
 
 		assert.NotEqual(t, &networkingv1.Ingress{
 			ObjectMeta: metav1.ObjectMeta{
-				Name:        naming.Ingress(params.Instance),
+				Name:        naming.Ingress(params.Instance.Name),
 				Namespace:   ns,
 				Annotations: params.Instance.Spec.Ingress.Annotations,
 				Labels: map[string]string{
-					"app.kubernetes.io/name":       naming.Ingress(params.Instance),
+					"app.kubernetes.io/name":       naming.Ingress(params.Instance.Name),
 					"app.kubernetes.io/instance":   fmt.Sprintf("%s.%s", params.Instance.Namespace, params.Instance.Name),
 					"app.kubernetes.io/managed-by": "opentelemetry-operator",
 				},

diff --git a/pkg/collector/reconcile/opentelemetry.go b/pkg/collector/reconcile/opentelemetry.go
@@ -65,7 +65,7 @@ func updateScaleSubResourceStatus(ctx context.Context, cli client.Client, change
 		return nil
 	}
 
-	name := naming.Collector(*changed)
+	name := naming.Collector(changed.Name)
 
 	// Set the scale selector
 	labels := collector.Labels(*changed, name, []string{})
@@ -78,7 +78,7 @@ func updateScaleSubResourceStatus(ctx context.Context, cli client.Client, change
 	// Set the scale replicas
 	objKey := client.ObjectKey{
 		Namespace: changed.GetNamespace(),
-		Name:      naming.Collector(*changed),
+		Name:      naming.Collector(changed.Name),
 	}
 
 	var replicas int32

diff --git a/pkg/collector/reconcile/route.go b/pkg/collector/reconcile/route.go
@@ -17,7 +17,6 @@ package reconcile
 import (
 	"context"
 	"fmt"
-
 	routev1 "github.com/openshift/api/route/v1"
 	k8serrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -61,11 +60,11 @@ func desiredRoutes(_ context.Context, params Params) []routev1.Route {
 	for i, p := range ports {
 		routes[i] = routev1.Route{
 			ObjectMeta: metav1.ObjectMeta{
-				Name:        naming.Route(params.Instance, p.Name),
+				Name:        naming.Route(params.Instance.Name, p.Name),
 				Namespace:   params.Instance.Namespace,
 				Annotations: params.Instance.Spec.Ingress.Annotations,
 				Labels: map[string]string{
-					"app.kubernetes.io/name":       naming.Route(params.Instance, p.Name),
+					"app.kubernetes.io/name":       naming.Route(params.Instance.Name, p.Name),
 					"app.kubernetes.io/instance":   fmt.Sprintf("%s.%s", params.Instance.Namespace, params.Instance.Name),
 					"app.kubernetes.io/managed-by": "opentelemetry-operator",
 				},
@@ -75,11 +74,10 @@ func desiredRoutes(_ context.Context, params Params) []routev1.Route {
 				Path: "/" + p.Name,
 				To: routev1.RouteTargetReference{
 					Kind: "Service",
-					Name: naming.Service(params.Instance),
+					Name: naming.Service(params.Instance.Name),
 				},
 				Port: &routev1.RoutePort{
-					// Valid names must be non-empty and no more than 15 characters long.
-					TargetPort: intstr.FromString(naming.Truncate(p.Name, 15)),
+					TargetPort: intstr.FromString(naming.PortName(p.Name, p.Port)),
 				},
 				WildcardPolicy: routev1.WildcardPolicyNone,
 				TLS:            tlsCfg,

diff --git a/pkg/collector/reconcile/route_test.go b/pkg/collector/reconcile/route_test.go
@@ -115,11 +115,11 @@ func TestDesiredRoutes(t *testing.T) {
 
 		assert.NotEqual(t, &routev1.Route{
 			ObjectMeta: metav1.ObjectMeta{
-				Name:        naming.Route(params.Instance, ""),
+				Name:        naming.Route(params.Instance.Name, ""),
 				Namespace:   ns,
 				Annotations: params.Instance.Spec.Ingress.Annotations,
 				Labels: map[string]string{
-					"app.kubernetes.io/name":       naming.Route(params.Instance, ""),
+					"app.kubernetes.io/name":       naming.Route(params.Instance.Name, ""),
 					"app.kubernetes.io/instance":   fmt.Sprintf("%s.%s", params.Instance.Namespace, params.Instance.Name),
 					"app.kubernetes.io/managed-by": "opentelemetry-operator",
 				},