Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

branch_slug in ApplicationSet PR generator truncates too far #15163

Closed
whitelancer opened this issue Aug 22, 2023 · 2 comments
Closed

branch_slug in ApplicationSet PR generator truncates too far #15163

whitelancer opened this issue Aug 22, 2023 · 2 comments
Labels
component:application-sets Bulk application management related enhancement New feature or request

Comments

@whitelancer
Copy link

whitelancer commented Aug 22, 2023

Hi there, I've noticed an issue with the branch_slug in the PR ApplicationSet generator 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

And a bigger issue with a branch that only contains one slash (or hyphen):

	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

The simplest fix would be to disable smart truncate in the go library. I realize this may cause problems with existing params, but it's impossible to generate unique branch slugs when using smart truncate.

For instance, these all give back feature as the slug.

slug.EnableSmartTruncate = true
slug.MaxLength = 50
fmt.Println(slug.Make("feature/reallylongnamethatwillbetruncatedbecauseitistoolong"))
fmt.Println(slug.Make("feature/isanotherreallylongnamethatwillbetruncatedbadly"))
fmt.Println(slug.Make("feature/shouldbedifferentbutitalsoisnottruncatedcorrectly"))
@whitelancer whitelancer added the bug Something isn't working label Aug 22, 2023
@crenshaw-dev
Copy link
Member

Cross-linking my answer from another thread: #10622 (comment)

@crenshaw-dev crenshaw-dev added enhancement New feature or request component:application-sets Bulk application management related and removed bug Something isn't working labels Aug 22, 2023
@Aym3nTN
Copy link
Contributor

Aym3nTN commented Nov 16, 2023

This MR #15188 should've closed this one

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component:application-sets Bulk application management related enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants