diff --git a/.chloggen/1954.yaml b/.chloggen/1954.yaml new file mode 100755 index 0000000000..67c2595901 --- /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. 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 index 307f1b84d7..3bfe7ac26e 100644 --- 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 index 264d9a6ebe..0e4f3428d6 100644 --- 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 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..c25ff2f73d 100644 --- 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 index 21b4e2ce53..dbf9bcc27c 100644 --- 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 index 428938c313..f36fe6aeb7 100644 --- 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 index 827c988c4a..9bcdb4cb08 100644 --- 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 index a31d6c4afb..0cd8228623 100644 --- 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 index bddd921146..3397999118 100644 --- 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 index 66c9bf083e..411929ad22 100644 --- 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 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..09f6ff8381 100644 --- 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 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/reconcile/servicemonitor.go b/pkg/collector/reconcile/servicemonitor.go index 7da2f275be..942e16586c 100644 --- a/pkg/collector/reconcile/servicemonitor.go +++ b/pkg/collector/reconcile/servicemonitor.go @@ -58,9 +58,9 @@ func desiredServiceMonitors(_ context.Context, params Params) []monitoringv1.Ser { ObjectMeta: metav1.ObjectMeta{ Namespace: col.Namespace, - Name: naming.ServiceMonitor(col), + Name: naming.ServiceMonitor(col.Name), Labels: map[string]string{ - "app.kubernetes.io/name": naming.ServiceMonitor(params.Instance), + "app.kubernetes.io/name": naming.ServiceMonitor(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/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..a45f990712 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,51 +66,51 @@ 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. -func ServiceMonitor(otelcol v1alpha1.OpenTelemetryCollector) string { - return DNSName(Truncate("%s-collector", 63, otelcol.Name)) +func ServiceMonitor(otelcol string) string { + return DNSName(Truncate("%s-collector", 63, otelcol)) } // 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..f4ba37acab --- /dev/null +++ b/pkg/naming/port.go @@ -0,0 +1,44 @@ +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +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 pod and ingress spec has to be maximum 15 characters long. +func PortName(receiverName string, port int32) string { + if len(receiverName) > 15 { + 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 15 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..172956b4cd --- /dev/null +++ b/pkg/naming/port_test.go @@ -0,0 +1,68 @@ +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +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/naming/ports.go b/pkg/naming/ports.go deleted file mode 100644 index 108f8b75c1..0000000000 --- a/pkg/naming/ports.go +++ /dev/null @@ -1,40 +0,0 @@ -// Copyright The OpenTelemetry Authors -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package naming - -import ( - "strings" - "unicode/utf8" -) - -// PortName returns a dns-safe string for the given name. -// Any char that is not [a-z0-9] is replaced by "-" or "a". -// Replacement character "a" is used only at the beginning or at the end of the name. -// The function does not change length of the string. -func PortName(name string) string { - var d []rune - - for i, x := range strings.ToLower(name) { - if regex.Match([]byte(string(x))) { - d = append(d, x) - } else if i == 0 || i == utf8.RuneCountInString(name)-1 { - d = append(d, 'a') - } else { - d = append(d, '-') - } - } - - return string(d) -} diff --git a/pkg/naming/ports_test.go b/pkg/naming/ports_test.go deleted file mode 100644 index 9e433c95f3..0000000000 --- a/pkg/naming/ports_test.go +++ /dev/null @@ -1,43 +0,0 @@ -// Copyright The OpenTelemetry Authors -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package naming - -import ( - "regexp" - "testing" - - "github.com/stretchr/testify/assert" -) - -func TestPortName(t *testing.T) { - var tests = []struct { - in string - out string - }{ - {"simplest", "simplest"}, - {"prometheus/dev", "prometheus-dev"}, - {"prometheus.dev", "prometheus-dev"}, - {"prometheus-", "prometheusa"}, - {"-prometheus", "aprometheus"}, - } - rule, err := regexp.Compile(`^[a-z0-9]([-a-z0-9]*[a-z0-9])?$`) - assert.NoError(t, err) - - for _, tt := range tests { - assert.Equal(t, tt.out, PortName(tt.in)) - matched := rule.Match([]byte(tt.out)) - assert.True(t, matched, "%v is not a valid name", tt.out) - } -} diff --git a/pkg/targetallocator/configmap.go b/pkg/targetallocator/configmap.go index 210bd1c8be..30dea41541 100644 --- a/pkg/targetallocator/configmap.go +++ b/pkg/targetallocator/configmap.go @@ -32,7 +32,7 @@ const ( ) func ConfigMap(instance v1alpha1.OpenTelemetryCollector) (corev1.ConfigMap, error) { - name := naming.TAConfigMap(instance) + name := naming.TAConfigMap(instance.Name) version := strings.Split(instance.Spec.Image, ":") labels := Labels(instance, name) if len(version) > 1 { 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(),