feat: Slugified the branch name in PR generators (argoproj#9462)

* Slugified the branch name in PR generators Add branchSlug attr to not cause a breaking change Signed-off-by: Aymen Ben Tanfous <aymen.bentanfous@cimpress.com> * Fixed pull_request_test Signed-off-by: Aymen Ben Tanfous <aymen.bentanfous@cimpress.com> * Adding 'branch_slug' as an output of the generator Signed-off-by: Aymen Ben Tanfous <aymen.bentanfous@cimpress.com> * Updated the doc Signed-off-by: Aymen Ben Tanfous <aymen.bentanfous@cimpress.com> * Fixed test Signed-off-by: Aymen Ben Tanfous <aymen.bentanfous@cimpress.com> Co-authored-by: Aymen Ben Tanfous <aymen.bentanfous@cimpress.com>
Aym3nTN · Sep 15, 2022 · 3552cde · 3552cde
1 parent b992ce8
commit 3552cde
Show file tree

Hide file tree

Showing 6 changed files with 54 additions and 6 deletions.
diff --git a/applicationset/generators/pull_request.go b/applicationset/generators/pull_request.go
@@ -12,6 +12,7 @@ import (
 	"github.com/argoproj/argo-cd/v2/applicationset/services/pull_request"
 	pullrequest "github.com/argoproj/argo-cd/v2/applicationset/services/pull_request"
 	argoprojiov1alpha1 "github.com/argoproj/argo-cd/v2/pkg/apis/applicationset/v1alpha1"
+	"github.com/gosimple/slug"
 )
 
 var _ Generator = (*PullRequestGenerator)(nil)
@@ -67,11 +68,24 @@ func (g *PullRequestGenerator) GenerateParams(appSetGenerator *argoprojiov1alpha
 		return nil, fmt.Errorf("error listing repos: %v", err)
 	}
 	params := make([]map[string]string, 0, len(pulls))
+
+	// In order to follow the DNS label standard as defined in RFC 1123,
+	// we need to limit the 'branch' to 50 to give room to append/suffix-ing it
+	// with 13 more characters. Also, there is the need to clean it as recommended
+	// here https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#dns-label-names
+	slug.MaxLength = 50
+
+	// Converting underscores to dashes
+	slug.CustomSub = map[string]string{
+		"_": "-",
+	}
+
 	for _, pull := range pulls {
 		params = append(params, map[string]string{
-			"number":   strconv.Itoa(pull.Number),
-			"branch":   pull.Branch,
-			"head_sha": pull.HeadSHA,
+			"number":      strconv.Itoa(pull.Number),
+			"branch":      pull.Branch,
+			"branch_slug": slug.Make(pull.Branch),
+			"head_sha":    pull.HeadSHA,
 		})
 	}
 	return params, nil

diff --git a/applicationset/generators/pull_request_test.go b/applicationset/generators/pull_request_test.go
@@ -37,9 +37,34 @@ func TestPullRequestGithubGenerateParams(t *testing.T) {
 			},
 			expected: []map[string]string{
 				{
-					"number":   "1",
-					"branch":   "branch1",
-					"head_sha": "089d92cbf9ff857a39e6feccd32798ca700fb958",
+					"number":      "1",
+					"branch":      "branch1",
+					"branch_slug": "branch1",
+					"head_sha":    "089d92cbf9ff857a39e6feccd32798ca700fb958",
+				},
+			},
+			expectedErr: nil,
+		},
+		{
+			selectFunc: func(context.Context, *argoprojiov1alpha1.PullRequestGenerator, *argoprojiov1alpha1.ApplicationSet) (pullrequest.PullRequestService, error) {
+				return pullrequest.NewFakeService(
+					ctx,
+					[]*pullrequest.PullRequest{
+						&pullrequest.PullRequest{
+							Number:  2,
+							Branch:  "feat/areally+long_pull_request_name_to_test_argo_slugification_and_branch_name_shortening_feature",
+							HeadSHA: "9b34ff5bd418e57d58891eb0aa0728043ca1e8be",
+						},
+					},
+					nil,
+				)
+			},
+			expected: []map[string]string{
+				{
+					"number":      "2",
+					"branch":      "feat/areally+long_pull_request_name_to_test_argo_slugification_and_branch_name_shortening_feature",
+					"branch_slug": "feat-areally-long-pull-request-name-to-test-argo",
+					"head_sha":    "9b34ff5bd418e57d58891eb0aa0728043ca1e8be",
 				},
 			},
 			expectedErr: nil,
@@ -64,6 +89,7 @@ func TestPullRequestGithubGenerateParams(t *testing.T) {
 		generatorConfig := argoprojiov1alpha1.ApplicationSetGenerator{
 			PullRequest: &argoprojiov1alpha1.PullRequestGenerator{},
 		}
+
 		got, gotErr := gen.GenerateParams(&generatorConfig, nil)
 		assert.Equal(t, c.expectedErr, gotErr)
 		assert.ElementsMatch(t, c.expected, got)

diff --git a/docs/operator-manual/applicationset/Generators-Pull-Request.md b/docs/operator-manual/applicationset/Generators-Pull-Request.md
@@ -263,6 +263,7 @@ spec:
 
 * `number`: The ID number of the pull request.
 * `branch`: The name of the branch of the pull request head.
+* `branch_slug`: The branch name will be cleaned to be conform to the DNS label standard as defined in [RFC 1123](https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#dns-label-names), and truncated to 50 characters to give room to append/suffix-ing it with 13 more characters. 
 * `head_sha`: This is the SHA of the head of the pull request.
 
 ## Webhook Configuration

diff --git a/go.mod b/go.mod
@@ -230,6 +230,7 @@ require (
 )
 
 require (
+	github.com/gosimple/slug v1.12.0
 	go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc v0.31.0
 	go.opentelemetry.io/otel v1.6.3
 	go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc v1.6.3
@@ -240,6 +241,7 @@ require (
 	github.com/PagerDuty/go-pagerduty v1.5.0 // indirect
 	github.com/cenkalti/backoff/v4 v4.1.2 // indirect
 	github.com/go-logr/stdr v1.2.2 // indirect
+	github.com/gosimple/unidecode v1.0.1 // indirect
 	github.com/grpc-ecosystem/grpc-gateway/v2 v2.7.0 // indirect
 	github.com/jmespath/go-jmespath v0.4.0 // indirect
 	go.opentelemetry.io/otel/exporters/otlp/internal/retry v1.6.3 // indirect

diff --git a/go.sum b/go.sum
@@ -594,6 +594,10 @@ github.com/gorilla/mux v1.7.4/go.mod h1:DVbg23sWSpFRCP0SfiEN6jmj59UnW/n46BH5rLB7
 github.com/gorilla/mux v1.8.0/go.mod h1:DVbg23sWSpFRCP0SfiEN6jmj59UnW/n46BH5rLB71So=
 github.com/gorilla/websocket v1.4.2 h1:+/TMaTYc4QFitKJxsQ7Yye35DkWvkdLcvGKqM+x0Ufc=
 github.com/gorilla/websocket v1.4.2/go.mod h1:YR8l580nyteQvAITg2hZ9XVh4b55+EU/adAjf1fMHhE=
+github.com/gosimple/slug v1.12.0 h1:xzuhj7G7cGtd34NXnW/yF0l+AGNfWqwgh/IXgFy7dnc=
+github.com/gosimple/slug v1.12.0/go.mod h1:UiRaFH+GEilHstLUmcBgWcI42viBN7mAb818JrYOeFQ=
+github.com/gosimple/unidecode v1.0.1 h1:hZzFTMMqSswvf0LBJZCZgThIZrpDHFXux9KeGmn6T/o=
+github.com/gosimple/unidecode v1.0.1/go.mod h1:CP0Cr1Y1kogOtx0bJblKzsVWrqYaqfNOnHzpgWw4Awc=
 github.com/gregdel/pushover v1.1.0 h1:dwHyvrcpZCOS9V1fAnKPaGRRI5OC55cVaKhMybqNsKQ=
 github.com/gregdel/pushover v1.1.0/go.mod h1:EcaO66Nn1StkpEm1iKtBTV3d2A16SoMsVER1PthX7to=
 github.com/gregjones/httpcache v0.0.0-20180305231024-9cad4c3443a7/go.mod h1:FecbI9+v66THATjSRHfNgh1IVFe/9kFxbXtjV0ctIMA=

diff --git a/pkg/apis/applicationset/v1alpha1/applicationset_types.go b/pkg/apis/applicationset/v1alpha1/applicationset_types.go
@@ -397,6 +397,7 @@ type PullRequestGenerator struct {
 	BitbucketServer *PullRequestGeneratorBitbucketServer `json:"bitbucketServer,omitempty"`
 	// Filters for which pull requests should be considered.
 	Filters []PullRequestGeneratorFilter `json:"filters,omitempty"`
+
 	// Standard parameters.
 	RequeueAfterSeconds *int64                 `json:"requeueAfterSeconds,omitempty"`
 	Template            ApplicationSetTemplate `json:"template,omitempty"`