Fix: Sticking to DNS label standard in branch_slug (Gitlab PR Generators)

[Aym3nTN]

Summary

When using the "Build" job from Gitlab auto-devops, the image name includes the slugified version of the branch ( using this variable CI_COMMIT_REF_SLUG) which is limited to 63 characters. So when trying to use the branch_slug key to building the path of the image, Argo won't find it because in my old PR #9462 I hardcoded the slug max length to 50.

Motivation

Same motivation as #9462
In order to follow the DNS label standard as defined in RFC 1123, manifests' name metadata should not exceed 63 and must consist of lower case alphanumeric characters, '-' or '.', and must start and end with an alphanumeric character.

Docs: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#dns-label-names

Proposal

This time after falling into the limit trap, I suggest sticking to DN label standard and keep the limit as 63 characters.

[cleverhu]

I'd like to fix this, can this issue been assigend to me ?

[cleverhu]

@Aym3nTN I pulled a request for this, I don't know is it ok. PTAL.

[Aym3nTN]

Have you tested the case when 63 characters cuts a word in 2?
I tested the gosimple/slug library but it doesn't cover that case, so I created a PR gosimple/slug#75 in that repo to remediate to that use case and waiting for the maintainer to have a look at it

[cleverhu]

你测试过 63 个字符将一个单词分成 2 的情况吗？ 我测试了 gosimple/slug 库，但它没有涵盖这种情况，因此我在该 repo 中创建了一个 PR gosimple/slug#75来修复该用例并等待维护者查看它

I tested for it, it passed!

[Aym3nTN]

Run a test with this branch name feature/areally+longer_pull_request_name_to_test_argo_slugifications_and_branch_name_shortening_feature the expected slug should be feature/areally+longer_pull_request_name_to_test_argo_slugifica

[cleverhu]

Run a test with this branch name feature/areally+longer_pull_request_name_to_test_argo_slugifications_and_branch_name_shortening_feature the expected slug should be feature/areally+longer_pull_request_name_to_test_argo_slugifica

You mean that there will be a word divided into two at the end. I misunderstood your meaning, and this may not be fixed.

[Aym3nTN]

@cleverhu I can take it from if you'd like, I might have a solution for it, alright?

[crenshaw-dev]

@Aym3nTN I think I'd like to avoid changing the branch_slug parameter for two reasons:

1. people might rely on the current sluggification for certain use cases - even if they don't, changing the behavior in a patch release would be Surprising
2. I'd like to encourage folks to switch to using go-templated ApplicationSets, because it will help avoid future backwards-compatibility issues

You could augment the ApplicationSet functions map with a wrapper for the slug function. You'd add the function here:

argo-cd/applicationset/utils/utils.go

Line 30 in 0a02a63

sprigFuncMap["normalize"] = SanitizeName

[cleverhu]

@cleverhu I can take it from if you'd like, I might have a solution for it, alright?

Of course. I may not be familiar with this area. Please deal with it.

[whitelancer]

Hi there, I've noticed another issue with the branch_slug that I'm hoping we can address. Since the library is using the Go slug library ("github.com/gosimple/slug") the default behavior is to have EnableSmartTruncate enabled/on. This creates a lot of problems when trying to generate unique branch names from long branches, such as:

	slug.MaxLength = 50
	urlSlug := slug.Make("feature/123reallylong/name/herethatmustbe/truncated/soon/orelseitwillblow/up")
	fmt.Println(urlSlug)

feature-123reallylong-name-herethatmustbe

	urlSlug = slug.Make("feature/123reallylongnameherethatmustbetruncatedsoonorelseitwillblowup")
	fmt.Println(urlSlug)

feature

When trying to use branch_slug to generate a container image name, this becomes a major problem especially in the second example. Here is what I expect and is more repeatable for other tools:

	slug.EnableSmartTruncate = false
	slug.MaxLength = 50
	urlSlug := slug.Make("feature/123reallylong/name/herethatmustbe/truncated/soon/orelseitwillblow/up")
	fmt.Println(urlSlug)

feature-123reallylong-name-herethatmustbe-truncate

	slug.EnableSmartTruncate = false
	urlSlug = slug.Make("feature/123reallylongnameherethatmustbetruncatedsoonorelseitwillblowup")
	fmt.Println(urlSlug)

feature-123reallylongnameherethatmustbetruncatedso

Also, in sticking with the original issue, I'm also trying to use that CI_COMMIT_REF_SLUG variable and would like to see the slug.MaxLength set to 63. If nothing else, make it customizable please!

[crenshaw-dev]

@whitelancer I think the best solution is to augment the ApplicationSet functions map with a parameterized wrapper for the slug function.

[whitelancer]

@crenshaw-dev Is there anyway for us to do that, as the user of the app? Or is that only if Argo's codebase gets modified to fix/change this?

[crenshaw-dev]

This will require a feature PR. Basically, just like we added a SanitizeName function, someone will need to add a Slug function:

argo-cd/applicationset/utils/utils.go

Line 30 in 0a02a63

sprigFuncMap["normalize"] = SanitizeName

[Aym3nTN]

I have just created a PR for this #15188 , please have a look at it and let me know