From 90a3767c6847bc8782a0302c0fa58171811ad1e1 Mon Sep 17 00:00:00 2001 From: Alexey Igrychev Date: Thu, 31 Mar 2022 23:20:15 +0100 Subject: [PATCH] fix(cleanup): fail on getting manifests for some custom tag metadata Sluggification did not work correctly in some cases, not providing idempotence for slugged data. As a result, cleanup failed on getting manifests for custom tag metadata because nonexistent tags were constructed with sluggification. This PR fixes sluggification by supporting these cases. Signed-off-by: Alexey Igrychev --- pkg/slug/slug.go | 56 ++++++++++++++++++++++++++++++++++++++++++- pkg/slug/slug_test.go | 28 ++++++++++++++++------ 2 files changed, 76 insertions(+), 8 deletions(-) diff --git a/pkg/slug/slug.go b/pkg/slug/slug.go index 856751c26f..c6d2956c10 100644 --- a/pkg/slug/slug.go +++ b/pkg/slug/slug.go @@ -30,13 +30,64 @@ func Slug(data string) string { } func LimitedSlug(data string, slugMaxSize int) string { - if len(data) == 0 || slugify(data) == data && len(data) <= slugMaxSize { + if !shouldBeSlugged(data, slugMaxSize) { return data } return slug(data, slugMaxSize) } +func shouldBeSlugged(data string, slugMaxSize int) bool { + // nothing to slug + if len(data) == 0 { + return false + } + + // valid data + if slugify(data) == data && len(data) <= slugMaxSize { + return false + } + + // legacy: this code provides idempotence for slugged data with sequence of two hyphens in a specific place + // Remove this block in the following major releases. + { + // data length cannot be more than max size + if len(data) > slugMaxSize { + return true + } + + // data length cannot be equal or less than service part + servicePartSize := len(util.MurmurHash("ANY")) + len(slugSeparator) + if len(data) <= servicePartSize { + return true + } + + // data must contain only one sequence + if strings.Count(data, "--") != 1 { + return true + } + + // data must contain sequence in a certain place + { + firstHyphenInd := len(data) - servicePartSize - 1 + ind := strings.Index(data, "--") + if firstHyphenInd != ind { + return true + } + } + + // data without sequence must be valid + { + formattedData := strings.Replace(data, "--", "-", 1) + if slugify(formattedData) != formattedData { + return true + } + } + + return false + } +} + func Project(name string) string { if err := validateProject(name); err != nil { res := slugify(name) @@ -142,6 +193,9 @@ func slug(data string, maxSize int) string { var slugParts []string if sluggedData != "" { croppedSluggedData := cropSluggedData(sluggedData, murmurHash, maxSize) + + // legacy: this check cannot be fixed without breaking reproducibility. + // Fix by replacing HasPrefix on HasSuffix in the following major releases. if strings.HasPrefix(croppedSluggedData, "-") { slugParts = append(slugParts, croppedSluggedData[:len(croppedSluggedData)-1]) } else { diff --git a/pkg/slug/slug_test.go b/pkg/slug/slug_test.go index db0a840873..aad98c2ccf 100644 --- a/pkg/slug/slug_test.go +++ b/pkg/slug/slug_test.go @@ -10,10 +10,13 @@ import ( var servicePartSize = len(util.MurmurHash("stub")) + len(slugSeparator) func TestSlug(t *testing.T) { + legacyCaseWithTwoHyphensMaxSize := 48 + tests := []struct { - name string - data string - result string + name string + data string + maxSize *int + result string }{ { name: "empty", @@ -35,21 +38,32 @@ func TestSlug(t *testing.T) { data: strings.Repeat("x", DefaultSlugMaxSize+1), result: strings.Repeat("x", DefaultSlugMaxSize-servicePartSize) + "-27e2f02f", }, + { + name: "legacyCaseWithTwoHyphen", + data: "postgres-feature-31981-change-delivery-date-del-result", + maxSize: &legacyCaseWithTwoHyphensMaxSize, + result: "postgres-feature-31981-change-delivery--852739dc", + }, } for _, test := range tests { t.Run(test.name, func(t *testing.T) { - result := LimitedSlug(test.data, DefaultSlugMaxSize) + maxSize := DefaultSlugMaxSize + if test.maxSize != nil { + maxSize = *test.maxSize + } + + result := LimitedSlug(test.data, maxSize) if test.result != result { t.Errorf("\n[EXPECTED]: %s (%d)\n[GOT]: %s (%d)", test.result, len(test.result), result, len(result)) } - if len(result) > DefaultSlugMaxSize { - t.Errorf("Max size exceeded: [EXPECTED]: %d [GOT]: %d", DefaultSlugMaxSize, len(result)) + if len(result) > maxSize { + t.Errorf("Max size exceeded: [EXPECTED]: %d [GOT]: %d", maxSize, len(result)) } tRunIdempotence(t, test.name, test.data, func(s string) string { - return LimitedSlug(s, DefaultSlugMaxSize) + return LimitedSlug(s, maxSize) }) }) }