diff --git a/changelog/fragments/fix-trim-format-dns1123-functions.yaml b/changelog/fragments/fix-trim-format-dns1123-functions.yaml new file mode 100644 index 00000000000..2c2896ea2a4 --- /dev/null +++ b/changelog/fragments/fix-trim-format-dns1123-functions.yaml @@ -0,0 +1,8 @@ +entries: + - description: > + Fixed invalid object names generated for long package names passed + to `run packagemanifests` & `run bundle`. + + kind: "bugfix" + + breaking: false diff --git a/internal/olm/operator/registry/configmap/deployment_test.go b/internal/olm/operator/registry/configmap/deployment_test.go index 5b73cdf9760..a97e1a47428 100644 --- a/internal/olm/operator/registry/configmap/deployment_test.go +++ b/internal/olm/operator/registry/configmap/deployment_test.go @@ -28,9 +28,9 @@ var _ = Describe("Deployment", func() { Describe("getRegistryServerName", func() { It("should return the formatted servername", func() { - Expect(getRegistryServerName("pkgName")).Should(Equal("pkgName-registry-server")) + Expect(getRegistryServerName("pkgName")).Should(Equal("pkgname-registry-server")) // This checks if all the special characters are handled correctly - Expect(getRegistryServerName("$abc.foo$@(&#(&!*)@&#")).Should(Equal("-abc-foo--registry-server")) + Expect(getRegistryServerName("$abc.foo$@(&#(&!*)@&#")).Should(Equal("abc-foo-registry-server")) }) }) @@ -39,7 +39,7 @@ var _ = Describe("Deployment", func() { labels := map[string]string{ "owner": "operator-sdk", "package-name": "$abc.foo$@(&#(&!*)@&#", - "server-name": "-abc-foo--registry-server", + "server-name": "abc-foo-registry-server", } Expect(getRegistryDeploymentLabels("$abc.foo$@(&#(&!*)@&#")).Should(Equal(labels)) diff --git a/internal/util/k8sutil/k8sutil.go b/internal/util/k8sutil/k8sutil.go index 9fe23a6c865..297fffa80a2 100644 --- a/internal/util/k8sutil/k8sutil.go +++ b/internal/util/k8sutil/k8sutil.go @@ -93,7 +93,11 @@ var dns1123LabelRegexp = regexp.MustCompile("[^a-zA-Z0-9]+") // replacing all non-compliant UTF-8 characters with "-". func FormatOperatorNameDNS1123(name string) string { if len(validation.IsDNS1123Label(name)) != 0 { - return dns1123LabelRegexp.ReplaceAllString(name, "-") + // Use - for any of the non-matching characters + n := dns1123LabelRegexp.ReplaceAllString(name, "-") + + // Now let's remove any leading or trailing - + return strings.ToLower(strings.Trim(n, "-")) } return name } @@ -102,7 +106,7 @@ func FormatOperatorNameDNS1123(name string) string { // by removing characters from the beginning of label such that len(label) <= 63. func TrimDNS1123Label(label string) string { if len(label) > validation.DNS1123LabelMaxLength { - return label[len(label)-validation.DNS1123LabelMaxLength:] + return strings.Trim(label[len(label)-validation.DNS1123LabelMaxLength:], "-") } return label } diff --git a/internal/util/k8sutil/k8sutil_test.go b/internal/util/k8sutil/k8sutil_test.go index 9fcdcbaadc6..21b6690885d 100644 --- a/internal/util/k8sutil/k8sutil_test.go +++ b/internal/util/k8sutil/k8sutil_test.go @@ -240,3 +240,95 @@ func TestSupportsOwnerReference(t *testing.T) { }) } } + +func TestTrimDNS1123Label(t *testing.T) { + type testcase struct { + name string + label string + expected string + } + testcases := []testcase{ + { + name: "return valid truncated values", + label: "quay-io-raffaelespazzoli-proactive-node-scaling-operator-bundle-latest", + expected: "raffaelespazzoli-proactive-node-scaling-operator-bundle-latest", + }, + { + name: "valid labels with proper length are noops", + label: "raffaelespazzoli-proactive-node-scaling-operator-bundle-latest", + expected: "raffaelespazzoli-proactive-node-scaling-operator-bundle-latest", + }, + { + name: "short invalid labels are left alone", + label: "-$*@*#fixed-invalid(__$)@+==-name-#$($", + expected: "-$*@*#fixed-invalid(__$)@+==-name-#$($", + }, + } + for _, tc := range testcases { + t.Run(tc.name, func(t *testing.T) { + result := TrimDNS1123Label(tc.label) + assert.Equal(t, tc.expected, result) + }) + } +} + +func TestFormatOperatorNameDNS1123(t *testing.T) { + type testcase struct { + name string + label string + expected string + } + testcases := []testcase{ + { + name: "should not start with -", + label: "-doesnot-start-with-hyphen", + expected: "doesnot-start-with-hyphen", + }, + { + name: "should not start with non-alphanumeric", + label: "$@*#(@does-notstart-garbage", + expected: "does-notstart-garbage", + }, + { + name: "should not have non-alphanumeric", + label: "sample-1234$@*#(@does-notstart-garbage", + expected: "sample-1234-does-notstart-garbage", + }, + { + name: "should not end with non-alphanumeric", + label: "sample-1234-does-notstart-garbage#$*@#*($_!-_@(", + expected: "sample-1234-does-notstart-garbage", + }, + { + name: "should not start or end with hyphen", + label: "-does-not-start-or-end-with-hyphen---", + expected: "does-not-start-or-end-with-hyphen", + }, + { + name: "empty string is a noop", + label: "", + expected: "", + }, + { + name: "string of invalid characters results in empty string", + label: "@#@#)$*!!_$#*$*!@", + expected: "", + }, + { + name: "valid long names are not trimmed", + label: "quay-io-raffaelespazzoli-proactive-node-scaling-operator-bundle-latest", + expected: "quay-io-raffaelespazzoli-proactive-node-scaling-operator-bundle-latest", + }, + { + name: "should not contain capital letters", + label: "QUAY-IO-gobble-gobBLE", + expected: "quay-io-gobble-gobble", + }, + } + for _, tc := range testcases { + t.Run(tc.name, func(t *testing.T) { + result := FormatOperatorNameDNS1123(tc.label) + assert.Equal(t, tc.expected, result) + }) + } +}