From 57e119ba68693df6c2e0b9465b1df38f04bc50d5 Mon Sep 17 00:00:00 2001 From: Mario Manno Date: Fri, 21 Jul 2023 18:02:41 +0200 Subject: [PATCH] Prevent non-hex chars in release name hash MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When a release name must be truncated to comply with Helm's maximum release name length, Fleet truncates the name and builds a new release name as `-`. In cases where the last character of `` is a dash, a previous commit would include the character immediately following the dash and place it before the hash, which might cause confusion. Instead, this commit does not include that character in such cases and increases the hash length by 1, so that characters after the last dash in the release name remain hexadecimal. Avoid increasing hex length if truncated string ends with - In such cases, the separator is simply omitted. The resulting string will be one character shorter than the limit, which should not cause any issues. On the other hand, the hash will contain only hexadecimal digits, as it previously did. Since double dashes are not part of the result, this preserves consistency across repeated calls, and hence between bundle ID and Helm release name, thereby preventing unexpected deletion and re-creation of bundles. Co-authored-by: Corentin Néau --- pkg/name/name.go | 23 +++++++++++++++++++---- pkg/name/name_test.go | 3 +++ 2 files changed, 22 insertions(+), 4 deletions(-) diff --git a/pkg/name/name.go b/pkg/name/name.go index 3fb5cb6aae..15b4db6e76 100644 --- a/pkg/name/name.go +++ b/pkg/name/name.go @@ -20,8 +20,12 @@ var ( ) // Limit the length of a string to count characters. If the string's length is -// greater than count, it will be truncated and a hash will be appended to the -// end. +// greater than count, it will be truncated and a separator will be appended to +// the end, followed by a hash. +// If the last character of the truncated string is the separator, then the +// separator itself is omitted. This prevents the result from containing two +// consecutive separators. In such a case, the result will be [count -1] +// characters long. // If count is too small to include the shortened hash the string is simply // truncated. func Limit(s string, count int) string { @@ -29,10 +33,21 @@ func Limit(s string, count int) string { return s } - if count <= 6 { + const hexLen int = 5 + separator := "-" + + if count <= hexLen+len(separator) { return s[:count] } - return fmt.Sprintf("%s-%s", s[:count-6], Hex(s, 5)) + + nbCharsBeforeTrim := count - hexLen - len(separator) + + // If the last character of the truncated string is the separator, include it instead of the separator. + if string(s[nbCharsBeforeTrim-1]) == separator { + separator = "" + } + + return fmt.Sprintf("%s%s%s", s[:nbCharsBeforeTrim], separator, Hex(s, hexLen)) } // Hex returns a hex-encoded hash of the string and truncates it to length. diff --git a/pkg/name/name_test.go b/pkg/name/name_test.go index a0a7708413..f7967ac98d 100644 --- a/pkg/name/name_test.go +++ b/pkg/name/name_test.go @@ -31,6 +31,7 @@ var _ = Describe("Name", func() { {arg: "12345678", n: 8, result: "12345678"}, {arg: "12345678", n: 7, result: "1-25d55"}, {arg: "123456789", n: 8, result: "12-25f9e"}, + {arg: "1-3456789", n: 8, result: "1-9b657"}, // no double dash in the result } It("matches expected results", func() { @@ -51,6 +52,8 @@ var _ = Describe("Name", func() { {arg: "long-name-test-0.App_ ", result: "long-name-test-0-app-5bf6b3fb"}, {arg: "long-name-test--App_-_12.factor", result: "long-name-test-app-12-factor-0efbac37"}, {arg: "bundle.name.example.com", result: "bundle-name-example-com-645ef821"}, + // no double dash in the result + {arg: str53[0:46] + "-1234567", result: "1234567890123456789012345678901234567890123456-d0bce"}, } It("matches expected results", func() {