Make sure port name in ingress and route match service (open-telemetr…

…y#1970) * 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]> * Fix Signed-off-by: Pavol Loffay <[email protected]> * Fix Signed-off-by: Pavol Loffay <[email protected]> --------- Signed-off-by: Pavol Loffay <[email protected]>
ItielOlenick · Aug 2, 2023 · 3e36666 · 3e36666
1 parent 4c5d52f
commit 3e36666
Show file tree

Hide file tree

Showing 34 changed files with 226 additions and 196 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. All ports are truncated to 15 characters. If the port name is longer it is changed to port-%d pattern.
+
+# 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/adapters/config_to_ports_test.go b/pkg/collector/adapters/config_to_ports_test.go
@@ -96,11 +96,11 @@ func TestExtractPortsFromConfig(t *testing.T) {
 
 	expectedPorts := []corev1.ServicePort{
 		{Name: "examplereceiver", Port: 12345},
-		{Name: "examplereceiver-settings", Port: 12346},
-		{Name: "jaeger-custom-thrift-http", AppProtocol: &httpAppProtocol, Protocol: "TCP", Port: 15268, TargetPort: targetPortZero},
+		{Name: "port-12346", Port: 12346},
+		{Name: "port-15268", AppProtocol: &httpAppProtocol, Protocol: "TCP", Port: 15268, TargetPort: targetPortZero},
 		{Name: "jaeger-grpc", AppProtocol: &grpcAppProtocol, Protocol: "TCP", Port: 14250},
-		{Name: "jaeger-thrift-binary", Protocol: "UDP", Port: 6833},
-		{Name: "jaeger-thrift-compact", Protocol: "UDP", Port: 6831},
+		{Name: "port-6833", Protocol: "UDP", Port: 6833},
+		{Name: "port-6831", Protocol: "UDP", Port: 6831},
 		{Name: "otlp-2-grpc", AppProtocol: &grpcAppProtocol, Protocol: "TCP", Port: 55555},
 		{Name: "otlp-grpc", AppProtocol: &grpcAppProtocol, Port: 4317, TargetPort: targetPort4317},
 		{Name: "otlp-http", AppProtocol: &httpAppProtocol, Port: 4318, TargetPort: targetPort4318},

diff --git a/pkg/collector/container.go b/pkg/collector/container.go
@@ -232,7 +232,7 @@ func getPrometheusExporterPorts(c map[interface{}]interface{}) ([]corev1.Contain
 				}
 				ports = append(ports,
 					corev1.ContainerPort{
-						Name:          naming.PortName(exporterName),
+						Name:          naming.PortName(exporterName, containerPort),
 						ContainerPort: containerPort,
 						Protocol:      corev1.ProtocolTCP,
 					},

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
@@ -25,11 +25,8 @@ import (
 	"github.com/go-logr/logr"
 	corev1 "k8s.io/api/core/v1"
 	v1 "k8s.io/api/core/v1"
-)
 
-var (
-	// DNS_LABEL constraints: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#dns-label-names
-	dnsLabelValidation = regexp.MustCompile("^(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])?$")
+	"github.com/open-telemetry/opentelemetry-operator/pkg/naming"
 )
 
 // ReceiverParser is an interface that should be implemented by all receiver parsers.
@@ -159,7 +156,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 +175,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
@@ -17,6 +17,8 @@ package parser
 import (
 	"github.com/go-logr/logr"
 	corev1 "k8s.io/api/core/v1"
+
+	"github.com/open-telemetry/opentelemetry-operator/pkg/naming"
 )
 
 const parserNameGeneric = "__generic"
@@ -59,7 +61,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
@@ -19,6 +19,8 @@ import (
 
 	"github.com/go-logr/logr"
 	corev1 "k8s.io/api/core/v1"
+
+	"github.com/open-telemetry/opentelemetry-operator/pkg/naming"
 )
 
 var _ ReceiverParser = &JaegerReceiverParser{}
@@ -104,7 +106,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_jaeger_test.go b/pkg/collector/parser/receiver_jaeger_test.go
@@ -88,10 +88,10 @@ func TestJaegerExposeDefaultPorts(t *testing.T) {
 		portNumber        int32
 		seen              bool
 	}{
-		"jaeger-grpc":           {portNumber: 14250, transportProtocol: corev1.ProtocolTCP},
-		"jaeger-thrift-http":    {portNumber: 14268, transportProtocol: corev1.ProtocolTCP},
-		"jaeger-thrift-compact": {portNumber: 6831, transportProtocol: corev1.ProtocolUDP},
-		"jaeger-thrift-binary":  {portNumber: 6832, transportProtocol: corev1.ProtocolUDP},
+		"jaeger-grpc": {portNumber: 14250, transportProtocol: corev1.ProtocolTCP},
+		"port-14268":  {portNumber: 14268, transportProtocol: corev1.ProtocolTCP},
+		"port-6831":   {portNumber: 6831, transportProtocol: corev1.ProtocolUDP},
+		"port-6832":   {portNumber: 6832, transportProtocol: corev1.ProtocolUDP},
 	}
 
 	// test

diff --git a/pkg/collector/parser/receiver_otlp.go b/pkg/collector/parser/receiver_otlp.go
@@ -20,6 +20,8 @@ import (
 	"github.com/go-logr/logr"
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/util/intstr"
+
+	"github.com/open-telemetry/opentelemetry-operator/pkg/naming"
 )
 
 var _ ReceiverParser = &OTLPReceiverParser{}
@@ -71,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,
@@ -82,7 +84,7 @@ 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,

diff --git a/pkg/collector/parser/receiver_skywalking.go b/pkg/collector/parser/receiver_skywalking.go
@@ -20,6 +20,8 @@ import (
 	"github.com/go-logr/logr"
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/util/intstr"
+
+	"github.com/open-telemetry/opentelemetry-operator/pkg/naming"
 )
 
 var _ ReceiverParser = &SkywalkingReceiverParser{}
@@ -66,7 +68,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 +79,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
@@ -21,6 +21,8 @@ import (
 	"github.com/stretchr/testify/assert"
 	corev1 "k8s.io/api/core/v1"
 	logf "sigs.k8s.io/controller-runtime/pkg/log"
+
+	"github.com/open-telemetry/opentelemetry-operator/pkg/naming"
 )
 
 var logger = logf.Log.WithName("unit-tests")
@@ -38,7 +40,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
@@ -61,11 +61,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 +75,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",
 				},