From 048b253289a3cdd7aafae14cb606f2a733cfbd58 Mon Sep 17 00:00:00 2001 From: mmatache <89529881+mmatache@users.noreply.github.com> Date: Wed, 8 Dec 2021 14:22:34 +0200 Subject: [PATCH] Limit names and labels to 63 characters (#609) * added naming formaters from jaeger project * updated naming to use name formatters * update labels to conform with kubernetes rules * fixed linting issues * added Jaeger copyright * updated Jaeger copyright --- pkg/collector/labels.go | 5 +- pkg/naming/dns.go | 49 ++++++++++++++++ pkg/naming/dns_test.go | 50 ++++++++++++++++ pkg/naming/main.go | 20 +++---- pkg/naming/triming.go | 75 ++++++++++++++++++++++++ pkg/naming/triming_test.go | 104 ++++++++++++++++++++++++++++++++++ pkg/targetallocator/labels.go | 5 +- 7 files changed, 291 insertions(+), 17 deletions(-) create mode 100644 pkg/naming/dns.go create mode 100644 pkg/naming/dns_test.go create mode 100644 pkg/naming/triming.go create mode 100644 pkg/naming/triming_test.go diff --git a/pkg/collector/labels.go b/pkg/collector/labels.go index 7f4867a3f3..06fc02bb7c 100644 --- a/pkg/collector/labels.go +++ b/pkg/collector/labels.go @@ -15,9 +15,8 @@ package collector import ( - "fmt" - "github.com/open-telemetry/opentelemetry-operator/apis/v1alpha1" + "github.com/open-telemetry/opentelemetry-operator/pkg/naming" ) // Labels return the common labels to all objects that are part of a managed OpenTelemetryCollector. @@ -31,7 +30,7 @@ func Labels(instance v1alpha1.OpenTelemetryCollector) map[string]string { } base["app.kubernetes.io/managed-by"] = "opentelemetry-operator" - base["app.kubernetes.io/instance"] = fmt.Sprintf("%s.%s", instance.Namespace, instance.Name) + base["app.kubernetes.io/instance"] = naming.Truncate("%s.%s", 63, instance.Namespace, instance.Name) base["app.kubernetes.io/part-of"] = "opentelemetry" base["app.kubernetes.io/component"] = "opentelemetry-collector" diff --git a/pkg/naming/dns.go b/pkg/naming/dns.go new file mode 100644 index 0000000000..4f78d9ebe6 --- /dev/null +++ b/pkg/naming/dns.go @@ -0,0 +1,49 @@ +// 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. + +// Additional copyrights: +// Copyright The Jaeger Authors + +package naming + +import ( + "regexp" + "strings" + "unicode/utf8" +) + +var regex = regexp.MustCompile(`[a-z0-9]`) + +// DNSName 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. +// source: https://github.com/jaegertracing/jaeger-operator/blob/91e3b69ee5c8761bbda9d3cf431400a73fc1112a/pkg/util/dns_name.go#L15 +func DNSName(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/dns_test.go b/pkg/naming/dns_test.go new file mode 100644 index 0000000000..ad53e9b08d --- /dev/null +++ b/pkg/naming/dns_test.go @@ -0,0 +1,50 @@ +// 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. + +// Additional copyrights: +// Copyright The Jaeger Authors + +package naming + +import ( + "regexp" + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestDnsName(t *testing.T) { + var tests = []struct { + in string + out string + }{ + {"simplest", "simplest"}, + {"instance.with.dots-collector-headless", "instance-with-dots-collector-headless"}, + {"TestQueryDottedServiceName.With.Dots", "testquerydottedservicename-with-dots"}, + {"Service🦄", "servicea"}, + {"📈Stock-Tracker", "astock-tracker"}, + {"-📈Stock-Tracker", "a-stock-tracker"}, + {"📈", "a"}, + {"foo-", "fooa"}, + {"-foo", "afoo"}, + } + 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, DNSName(tt.in)) + matched := rule.Match([]byte(tt.out)) + assert.True(t, matched, "%v is not a valid name", tt.out) + } +} diff --git a/pkg/naming/main.go b/pkg/naming/main.go index 380bc53d54..d81b9130c5 100644 --- a/pkg/naming/main.go +++ b/pkg/naming/main.go @@ -16,19 +16,17 @@ package naming import ( - "fmt" - "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 fmt.Sprintf("%s-collector", otelcol.Name) + return DNSName(Truncate("%s-collector", 63, otelcol.Name)) } // TAConfigMap returns the name for the config map used in the TargetAllocator. func TAConfigMap(otelcol v1alpha1.OpenTelemetryCollector) string { - return fmt.Sprintf("%s-targetallocator", otelcol.Name) + return DNSName(Truncate("%s-targetallocator", 63, otelcol.Name)) } // ConfigMapVolume returns the name to use for the config map's volume in the pod. @@ -53,35 +51,35 @@ func TAContainer() string { // Collector builds the collector (deployment/daemonset) name based on the instance. func Collector(otelcol v1alpha1.OpenTelemetryCollector) string { - return fmt.Sprintf("%s-collector", otelcol.Name) + return DNSName(Truncate("%s-collector", 63, otelcol.Name)) } // TargetAllocator returns the TargetAllocator deployment resource name. func TargetAllocator(otelcol v1alpha1.OpenTelemetryCollector) string { - return fmt.Sprintf("%s-targetallocator", otelcol.Name) + return DNSName(Truncate("%s-targetallocator", 63, otelcol.Name)) } // HeadlessService builds the name for the headless service based on the instance. func HeadlessService(otelcol v1alpha1.OpenTelemetryCollector) string { - return fmt.Sprintf("%s-headless", Service(otelcol)) + 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 { - return fmt.Sprintf("%s-monitoring", Service(otelcol)) + return DNSName(Truncate("%s-monitoring", 63, Service(otelcol))) } // Service builds the service name based on the instance. func Service(otelcol v1alpha1.OpenTelemetryCollector) string { - return fmt.Sprintf("%s-collector", otelcol.Name) + return DNSName(Truncate("%s-collector", 63, otelcol.Name)) } // TAService returns the name to use for the TargetAllocator service. func TAService(otelcol v1alpha1.OpenTelemetryCollector) string { - return fmt.Sprintf("%s-targetallocator", otelcol.Name) + return DNSName(Truncate("%s-targetallocator", 63, otelcol.Name)) } // ServiceAccount builds the service account name based on the instance. func ServiceAccount(otelcol v1alpha1.OpenTelemetryCollector) string { - return fmt.Sprintf("%s-collector", otelcol.Name) + return DNSName(Truncate("%s-collector", 63, otelcol.Name)) } diff --git a/pkg/naming/triming.go b/pkg/naming/triming.go new file mode 100644 index 0000000000..1520254780 --- /dev/null +++ b/pkg/naming/triming.go @@ -0,0 +1,75 @@ +// 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. + +// Additional copyrights: +// Copyright The Jaeger Authors + +package naming + +import ( + "fmt" + "regexp" +) + +var regexpEndReplace, regexpBeginReplace *regexp.Regexp + +func init() { + regexpEndReplace, _ = regexp.Compile("[^A-Za-z0-9]+$") + regexpBeginReplace, _ = regexp.Compile("^[^A-Za-z0-9]+") +} + +// Truncate will shorten the length of the instance name so that it contains at most max chars when combined with the fixed part +// If the fixed part is already bigger than the max, this function is noop. +// source: https://github.com/jaegertracing/jaeger-operator/blob/91e3b69ee5c8761bbda9d3cf431400a73fc1112a/pkg/util/truncate.go#L17 +func Truncate(format string, max int, values ...interface{}) string { + var truncated []interface{} + result := fmt.Sprintf(format, values...) + + if excess := len(result) - max; excess > 0 { + // we try to reduce the first string we find + for _, value := range values { + if excess == 0 { + continue + } + + if s, ok := value.(string); ok { + if len(s) > excess { + value = s[:len(s)-excess] + excess = 0 + } else { + value = "" // skip this value entirely + excess = excess - len(s) + } + } + + truncated = append(truncated, value) + } + + result = fmt.Sprintf(format, truncated...) + } + + // if at this point, the result is still bigger than max, apply a hard cap: + if len(result) > max { + return result[:max] + } + + return trimNonAlphaNumeric(result) +} + +// trimNonAlphaNumeric remove all non-alphanumeric values from start and end of the string +// source: https://github.com/jaegertracing/jaeger-operator/blob/91e3b69ee5c8761bbda9d3cf431400a73fc1112a/pkg/util/truncate.go#L53 +func trimNonAlphaNumeric(text string) string { + newText := regexpEndReplace.ReplaceAllString(text, "") + return regexpBeginReplace.ReplaceAllString(newText, "") +} diff --git a/pkg/naming/triming_test.go b/pkg/naming/triming_test.go new file mode 100644 index 0000000000..90fccd7fdb --- /dev/null +++ b/pkg/naming/triming_test.go @@ -0,0 +1,104 @@ +// 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. + +// Additional copyrights: +// Copyright The Jaeger Authors + +package naming + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestTruncate(t *testing.T) { + for _, tt := range []struct { + format string + max int + values []interface{} + expected string + cap string + }{ + { + format: "%s-collector", + max: 63, + values: []interface{}{"simplest"}, + expected: "simplest-collector", + cap: "the standard case", + }, + { + format: "d0c1e62-4d96-11ea-b174-c85b7644b6b5-5d0c1e62-4d96-11ea-b174-c85b7644b6b5", + max: 63, + values: []interface{}{}, + expected: "d0c1e62-4d96-11ea-b174-c85b7644b6b5-5d0c1e62-4d96-11ea-b174-c85", + cap: "first N case", + }, + { + format: "%s-collector", + max: 63, + values: []interface{}{"d0c1e62-4d96-11ea-b174-c85b7644b6b5-5d0c1e62-4d96-11ea-b174-c85b7644b6b5"}, + expected: "d0c1e62-4d96-11ea-b174-c85b7644b6b5-5d0c1e62-4d96-11e-collector", + cap: "instance + fixed within bounds", + }, + { + format: "%s-%s-collector", + max: 63, + values: []interface{}{"d0c1e62", "4d96-11ea-b174-c85b7644b6b5-5d0c1e62-4d96-11ea-b174-c85b7644b6b5"}, + expected: "4d96-11ea-b174-c85b7644b6b5-5d0c1e62-4d96-11ea-b174--collector", + cap: "first value gets dropped, second truncated", + }, + { + format: "%d-%s-collector", + max: 63, + values: []interface{}{42, "d0c1e62-4d96-11ea-b174-c85b7644b6b5-5d0c1e62-4d96-11ea-b174-c85b7644b6b5"}, + expected: "42-d0c1e62-4d96-11ea-b174-c85b7644b6b5-5d0c1e62-4d96--collector", + cap: "first value gets passed, second truncated", + }, + } { + assert.Equal(t, tt.expected, Truncate(tt.format, tt.max, tt.values...)) + } +} + +func TestTrimNonAlphaNumeric(t *testing.T) { + tests := []struct { + input string + expected string + }{ + { + input: "-%$#ThisIsALabel", + expected: "ThisIsALabel", + }, + + { + input: "label-invalid--_truncated-.", + expected: "label-invalid--_truncated", + }, + + { + input: "--(label-invalid--_truncated-#.1.", + expected: "label-invalid--_truncated-#.1", + }, + + { + input: "12ValidLabel3", + expected: "12ValidLabel3", + }, + } + + for _, test := range tests { + output := trimNonAlphaNumeric(test.input) + assert.Equal(t, test.expected, output) + } +} diff --git a/pkg/targetallocator/labels.go b/pkg/targetallocator/labels.go index 96720215f6..5cd4f5373a 100644 --- a/pkg/targetallocator/labels.go +++ b/pkg/targetallocator/labels.go @@ -15,9 +15,8 @@ package targetallocator import ( - "fmt" - "github.com/open-telemetry/opentelemetry-operator/apis/v1alpha1" + "github.com/open-telemetry/opentelemetry-operator/pkg/naming" ) // Labels return the common labels to all TargetAllocator objects that are part of a managed OpenTelemetryCollector. @@ -31,7 +30,7 @@ func Labels(instance v1alpha1.OpenTelemetryCollector) map[string]string { } base["app.kubernetes.io/managed-by"] = "opentelemetry-operator" - base["app.kubernetes.io/instance"] = fmt.Sprintf("%s.%s", instance.Namespace, instance.Name) + base["app.kubernetes.io/instance"] = naming.Truncate("%s.%s", 63, instance.Namespace, instance.Name) base["app.kubernetes.io/part-of"] = "opentelemetry" base["app.kubernetes.io/component"] = "opentelemetry-targetallocator"