From 777ac3f4fbdc8e70d63776e66efc38f5f620619f Mon Sep 17 00:00:00 2001 From: Pavol Loffay Date: Fri, 28 Jul 2023 12:13:34 +0200 Subject: [PATCH] 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 --- .chloggen/1954.yaml | 16 ++++++ pkg/collector/daemonset.go | 4 +- pkg/collector/deployment.go | 2 +- pkg/collector/horizontalpodautoscaler.go | 8 +-- pkg/collector/parser/receiver.go | 19 +------ pkg/collector/parser/receiver_generic.go | 3 +- pkg/collector/parser/receiver_jaeger.go | 3 +- pkg/collector/parser/receiver_otlp.go | 7 +-- pkg/collector/parser/receiver_skywalking.go | 5 +- pkg/collector/parser/receiver_test.go | 3 +- pkg/collector/reconcile/config_replace.go | 4 +- pkg/collector/reconcile/configmap.go | 2 +- pkg/collector/reconcile/ingress.go | 9 ++-- pkg/collector/reconcile/ingress_test.go | 4 +- pkg/collector/reconcile/opentelemetry.go | 4 +- pkg/collector/reconcile/route.go | 10 ++-- pkg/collector/reconcile/route_test.go | 4 +- pkg/collector/reconcile/service.go | 12 ++--- pkg/collector/serviceaccount.go | 4 +- pkg/collector/statefulset.go | 4 +- pkg/collector/volume.go | 2 +- pkg/naming/main.go | 56 ++++++++++----------- pkg/naming/port.go | 30 +++++++++++ pkg/naming/port_test.go | 54 ++++++++++++++++++++ pkg/targetallocator/deployment.go | 2 +- pkg/targetallocator/serviceaccount.go | 4 +- pkg/targetallocator/volume.go | 2 +- 27 files changed, 180 insertions(+), 97 deletions(-) create mode 100755 .chloggen/1954.yaml create mode 100644 pkg/naming/port.go create mode 100644 pkg/naming/port_test.go diff --git a/.chloggen/1954.yaml b/.chloggen/1954.yaml new file mode 100755 index 0000000000..820aff91ba --- /dev/null +++ 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 index f68d7bb708..aee0975039 100644 --- 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 index 7fa91725aa..7f4d0c898f 100644 --- 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 index 3e721821ba..4771a841a7 100644 --- 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 index e5b4a3ae4f..04b0b350f3 100644 --- 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 index 21b4e2ce53..db73f864d4 100644 --- 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 index 428938c313..4d5868791f 100644 --- 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 index 2fa208fe94..1d2743940d 100644 --- 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 index bddd921146..8ef1ac8fc1 100644 --- 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 index 66c9bf083e..b160f7b537 100644 --- 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 index 37ab3d8131..481f60d97a 100644 --- 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 index 03ee284b6d..eb39776ca6 100644 --- 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 index fee7e8cd7f..bc1f6b7947 100644 --- 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 index 9c03dd5650..853de0c886 100644 --- 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 index a4bf95579c..af709ae641 100644 --- 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 index 50783611d7..7f3be7580b 100644 --- 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 index 4ea82bc913..565645041f 100644 --- 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", }, diff --git a/pkg/collector/reconcile/service.go b/pkg/collector/reconcile/service.go index 715f892d56..59955e0afd 100644 --- a/pkg/collector/reconcile/service.go +++ b/pkg/collector/reconcile/service.go @@ -74,7 +74,7 @@ func Services(ctx context.Context, params Params) error { } func desiredService(ctx context.Context, params Params) *corev1.Service { - name := naming.Service(params.Instance) + name := naming.Service(params.Instance.Name) labels := collector.Labels(params.Instance, name, []string{}) config, err := adapters.ConfigFromString(params.Instance.Spec.Config) @@ -121,7 +121,7 @@ func desiredService(ctx context.Context, params Params) *corev1.Service { return &corev1.Service{ ObjectMeta: metav1.ObjectMeta{ - Name: naming.Service(params.Instance), + Name: naming.Service(params.Instance.Name), Namespace: params.Instance.Namespace, Labels: labels, Annotations: params.Instance.Annotations, @@ -136,14 +136,14 @@ func desiredService(ctx context.Context, params Params) *corev1.Service { } func desiredTAService(params Params) corev1.Service { - name := naming.TAService(params.Instance) + name := naming.TAService(params.Instance.Name) labels := targetallocator.Labels(params.Instance, name) selector := targetallocator.Labels(params.Instance, name) return corev1.Service{ ObjectMeta: metav1.ObjectMeta{ - Name: naming.TAService(params.Instance), + Name: naming.TAService(params.Instance.Name), Namespace: params.Instance.Namespace, Labels: labels, }, @@ -164,7 +164,7 @@ func headless(ctx context.Context, params Params) *corev1.Service { return nil } - h.Name = naming.HeadlessService(params.Instance) + h.Name = naming.HeadlessService(params.Instance.Name) h.Labels[headlessLabel] = headlessExists // copy to avoid modifying params.Instance.Annotations @@ -181,7 +181,7 @@ func headless(ctx context.Context, params Params) *corev1.Service { } func monitoringService(ctx context.Context, params Params) *corev1.Service { - name := naming.MonitoringService(params.Instance) + name := naming.MonitoringService(params.Instance.Name) labels := collector.Labels(params.Instance, name, []string{}) c, err := adapters.ConfigFromString(params.Instance.Spec.Config) diff --git a/pkg/collector/serviceaccount.go b/pkg/collector/serviceaccount.go index ef3815f3bb..f4e41ed779 100644 --- a/pkg/collector/serviceaccount.go +++ b/pkg/collector/serviceaccount.go @@ -25,7 +25,7 @@ import ( // ServiceAccountName returns the name of the existing or self-provisioned service account to use for the given instance. func ServiceAccountName(instance v1alpha1.OpenTelemetryCollector) string { if len(instance.Spec.ServiceAccount) == 0 { - return naming.ServiceAccount(instance) + return naming.ServiceAccount(instance.Name) } return instance.Spec.ServiceAccount @@ -33,7 +33,7 @@ func ServiceAccountName(instance v1alpha1.OpenTelemetryCollector) string { // ServiceAccount returns the service account for the given instance. func ServiceAccount(otelcol v1alpha1.OpenTelemetryCollector) corev1.ServiceAccount { - name := naming.ServiceAccount(otelcol) + name := naming.ServiceAccount(otelcol.Name) labels := Labels(otelcol, name, []string{}) return corev1.ServiceAccount{ diff --git a/pkg/collector/statefulset.go b/pkg/collector/statefulset.go index cb81d6a426..8e686dc0ae 100644 --- a/pkg/collector/statefulset.go +++ b/pkg/collector/statefulset.go @@ -27,7 +27,7 @@ import ( // StatefulSet builds the statefulset for the given instance. func StatefulSet(cfg config.Config, logger logr.Logger, otelcol v1alpha1.OpenTelemetryCollector) appsv1.StatefulSet { - name := naming.Collector(otelcol) + name := naming.Collector(otelcol.Name) labels := Labels(otelcol, name, cfg.LabelsFilter()) annotations := Annotations(otelcol) @@ -41,7 +41,7 @@ func StatefulSet(cfg config.Config, logger logr.Logger, otelcol v1alpha1.OpenTel Annotations: annotations, }, Spec: appsv1.StatefulSetSpec{ - ServiceName: naming.Service(otelcol), + ServiceName: naming.Service(otelcol.Name), Selector: &metav1.LabelSelector{ MatchLabels: SelectorLabels(otelcol), }, diff --git a/pkg/collector/volume.go b/pkg/collector/volume.go index b6fb467fa2..cc524a068a 100644 --- a/pkg/collector/volume.go +++ b/pkg/collector/volume.go @@ -29,7 +29,7 @@ func Volumes(cfg config.Config, otelcol v1alpha1.OpenTelemetryCollector) []corev Name: naming.ConfigMapVolume(), VolumeSource: corev1.VolumeSource{ ConfigMap: &corev1.ConfigMapVolumeSource{ - LocalObjectReference: corev1.LocalObjectReference{Name: naming.ConfigMap(otelcol)}, + LocalObjectReference: corev1.LocalObjectReference{Name: naming.ConfigMap(otelcol.Name)}, Items: []corev1.KeyToPath{{ Key: cfg.CollectorConfigMapEntry(), Path: cfg.CollectorConfigMapEntry(), diff --git a/pkg/naming/main.go b/pkg/naming/main.go index d7117c41ce..889ac3d560 100644 --- a/pkg/naming/main.go +++ b/pkg/naming/main.go @@ -15,18 +15,14 @@ // Package naming is for determining the names for components (containers, services, ...). package naming -import ( - "github.com/open-telemetry/opentelemetry-operator/apis/v1alpha1" -) - // ConfigMap builds the name for the config map used in the OpenTelemetryCollector containers. -func ConfigMap(otelcol v1alpha1.OpenTelemetryCollector) string { - return DNSName(Truncate("%s-collector", 63, otelcol.Name)) +func ConfigMap(otelcol string) string { + return DNSName(Truncate("%s-collector", 63, otelcol)) } // TAConfigMap returns the name for the config map used in the TargetAllocator. -func TAConfigMap(otelcol v1alpha1.OpenTelemetryCollector) string { - return DNSName(Truncate("%s-targetallocator", 63, otelcol.Name)) +func TAConfigMap(otelcol string) string { + return DNSName(Truncate("%s-targetallocator", 63, otelcol)) } // ConfigMapVolume returns the name to use for the config map's volume in the pod. @@ -50,18 +46,18 @@ func TAContainer() string { } // Collector builds the collector (deployment/daemonset) name based on the instance. -func Collector(otelcol v1alpha1.OpenTelemetryCollector) string { - return DNSName(Truncate("%s-collector", 63, otelcol.Name)) +func Collector(otelcol string) string { + return DNSName(Truncate("%s-collector", 63, otelcol)) } // HorizontalPodAutoscaler builds the autoscaler name based on the instance. -func HorizontalPodAutoscaler(otelcol v1alpha1.OpenTelemetryCollector) string { - return DNSName(Truncate("%s-collector", 63, otelcol.Name)) +func HorizontalPodAutoscaler(otelcol string) string { + return DNSName(Truncate("%s-collector", 63, otelcol)) } // OpenTelemetryCollector builds the collector (deployment/daemonset) name based on the instance. -func OpenTelemetryCollector(otelcol v1alpha1.OpenTelemetryCollector) string { - return DNSName(Truncate("%s", 63, otelcol.Name)) +func OpenTelemetryCollector(otelcol string) string { + return DNSName(Truncate("%s", 63, otelcol)) } // OpenTelemetryCollectorName builds the collector (deployment/daemonset) name based on the instance. @@ -70,43 +66,43 @@ func OpenTelemetryCollectorName(otelcolName string) string { } // TargetAllocator returns the TargetAllocator deployment resource name. -func TargetAllocator(otelcol v1alpha1.OpenTelemetryCollector) string { - return DNSName(Truncate("%s-targetallocator", 63, otelcol.Name)) +func TargetAllocator(otelcol string) string { + return DNSName(Truncate("%s-targetallocator", 63, otelcol)) } // HeadlessService builds the name for the headless service based on the instance. -func HeadlessService(otelcol v1alpha1.OpenTelemetryCollector) string { +func HeadlessService(otelcol string) string { return DNSName(Truncate("%s-headless", 63, Service(otelcol))) } // MonitoringService builds the name for the monitoring service based on the instance. -func MonitoringService(otelcol v1alpha1.OpenTelemetryCollector) string { +func MonitoringService(otelcol string) string { return DNSName(Truncate("%s-monitoring", 63, Service(otelcol))) } // Service builds the service name based on the instance. -func Service(otelcol v1alpha1.OpenTelemetryCollector) string { - return DNSName(Truncate("%s-collector", 63, otelcol.Name)) +func Service(otelcol string) string { + return DNSName(Truncate("%s-collector", 63, otelcol)) } // Ingress builds the ingress name based on the instance. -func Ingress(otelcol v1alpha1.OpenTelemetryCollector) string { - return DNSName(Truncate("%s-ingress", 63, otelcol.Name)) +func Ingress(otelcol string) string { + return DNSName(Truncate("%s-ingress", 63, otelcol)) } // Route builds the route name based on the instance. -func Route(otelcol v1alpha1.OpenTelemetryCollector, prefix string) string { - return DNSName(Truncate("%s-%s-route", 63, prefix, otelcol.Name)) +func Route(otelcol string, prefix string) string { + return DNSName(Truncate("%s-%s-route", 63, prefix, otelcol)) } // TAService returns the name to use for the TargetAllocator service. -func TAService(otelcol v1alpha1.OpenTelemetryCollector) string { - return DNSName(Truncate("%s-targetallocator", 63, otelcol.Name)) +func TAService(otelcol string) string { + return DNSName(Truncate("%s-targetallocator", 63, otelcol)) } // ServiceAccount builds the service account name based on the instance. -func ServiceAccount(otelcol v1alpha1.OpenTelemetryCollector) string { - return DNSName(Truncate("%s-collector", 63, otelcol.Name)) +func ServiceAccount(otelcol string) string { + return DNSName(Truncate("%s-collector", 63, otelcol)) } // ServiceMonitor builds the service account name based on the instance. @@ -115,6 +111,6 @@ func ServiceMonitor(otelcol v1alpha1.OpenTelemetryCollector) string { } // TargetAllocatorServiceAccount returns the TargetAllocator service account resource name. -func TargetAllocatorServiceAccount(otelcol v1alpha1.OpenTelemetryCollector) string { - return DNSName(Truncate("%s-targetallocator", 63, otelcol.Name)) +func TargetAllocatorServiceAccount(otelcol string) string { + return DNSName(Truncate("%s-targetallocator", 63, otelcol)) } diff --git a/pkg/naming/port.go b/pkg/naming/port.go new file mode 100644 index 0000000000..f08fb2804d --- /dev/null +++ b/pkg/naming/port.go @@ -0,0 +1,30 @@ +package naming + +import ( + "fmt" + "regexp" + "strings" +) + +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])?$") +) + +// PortName defines the port name used in services, ingresses and routes. +// The port name in pods is validated differently it has to be 15 characters long. +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 +} diff --git a/pkg/naming/port_test.go b/pkg/naming/port_test.go new file mode 100644 index 0000000000..7512b5e82f --- /dev/null +++ b/pkg/naming/port_test.go @@ -0,0 +1,54 @@ +package naming + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func Test(t *testing.T) { + tests := []struct { + testName string + receiverName string + port int32 + expected string + }{ + { + testName: "too_long", + receiverName: "otlphttpotlphttpotlphttpotlphttpotlphttpotlphttpotlphttpotlphttpotlphttpotlphttpotlphttpotlphttp", + port: 4318, + expected: "port-4318", + }, + { + testName: "with underscore", + receiverName: "otlp_http", + port: 4318, + expected: "otlp-http", + }, + { + testName: "with slash", + receiverName: "otlp/http", + port: 4318, + expected: "otlp-http", + }, + { + testName: "not DNS", + receiverName: "otlp&&**http", + port: 4318, + expected: "port-4318", + }, + { + testName: "ok", + receiverName: "otlphttp", + port: 4318, + expected: "otlphttp", + }, + } + + for _, test := range tests { + t.Run(test.testName, func(t *testing.T) { + name := PortName(test.receiverName, test.port) + assert.Equal(t, test.expected, name) + }) + } +} diff --git a/pkg/targetallocator/deployment.go b/pkg/targetallocator/deployment.go index b3ab0a9abb..70d9507a31 100644 --- a/pkg/targetallocator/deployment.go +++ b/pkg/targetallocator/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.TargetAllocator(otelcol) + name := naming.TargetAllocator(otelcol.Name) labels := Labels(otelcol, name) annotations := Annotations(otelcol) diff --git a/pkg/targetallocator/serviceaccount.go b/pkg/targetallocator/serviceaccount.go index af904863b7..bc036c19e3 100644 --- a/pkg/targetallocator/serviceaccount.go +++ b/pkg/targetallocator/serviceaccount.go @@ -25,7 +25,7 @@ import ( // ServiceAccountName returns the name of the existing or self-provisioned service account to use for the given instance. func ServiceAccountName(instance v1alpha1.OpenTelemetryCollector) string { if len(instance.Spec.TargetAllocator.ServiceAccount) == 0 { - return naming.ServiceAccount(instance) + return naming.ServiceAccount(instance.Name) } return instance.Spec.TargetAllocator.ServiceAccount @@ -33,7 +33,7 @@ func ServiceAccountName(instance v1alpha1.OpenTelemetryCollector) string { // ServiceAccount returns the service account for the given instance. func ServiceAccount(otelcol v1alpha1.OpenTelemetryCollector) corev1.ServiceAccount { - name := naming.TargetAllocatorServiceAccount(otelcol) + name := naming.TargetAllocatorServiceAccount(otelcol.Name) labels := Labels(otelcol, name) return corev1.ServiceAccount{ diff --git a/pkg/targetallocator/volume.go b/pkg/targetallocator/volume.go index 7e1b6d1e06..47d2e38102 100644 --- a/pkg/targetallocator/volume.go +++ b/pkg/targetallocator/volume.go @@ -28,7 +28,7 @@ func Volumes(cfg config.Config, otelcol v1alpha1.OpenTelemetryCollector) []corev Name: naming.TAConfigMapVolume(), VolumeSource: corev1.VolumeSource{ ConfigMap: &corev1.ConfigMapVolumeSource{ - LocalObjectReference: corev1.LocalObjectReference{Name: naming.TAConfigMap(otelcol)}, + LocalObjectReference: corev1.LocalObjectReference{Name: naming.TAConfigMap(otelcol.Name)}, Items: []corev1.KeyToPath{ { Key: cfg.TargetAllocatorConfigMapEntry(),