fix(appset): allow for shorthand git refs in git generators #15427 (#…

…19783) * fix(appset): allow for shorthand git refs in git generators Signed-off-by: rumstead <[email protected]> * Retrigger CI pipeline Signed-off-by: rumstead <[email protected]> * attempt to fix goimports Signed-off-by: rumstead <[email protected]> * attempt to fix goimports Signed-off-by: rumstead <[email protected]> * remove redundant test Signed-off-by: rumstead <[email protected]> --------- Signed-off-by: rumstead <[email protected]>
argoproj · Sep 5, 2024 · 01874d6 · 01874d6
1 parent aa2bafd
commit 01874d6
Show file tree

Hide file tree

Showing 6 changed files with 308 additions and 38 deletions.
diff --git a/applicationset/webhook/testdata/github-commit-event-feature-branch.json b/applicationset/webhook/testdata/github-commit-event-feature-branch.json
@@ -0,0 +1,186 @@
+{
+    "ref": "refs/heads/env/dev",
+    "before": "d5c1ffa8e294bc18c639bfb4e0df499251034414",
+    "after": "63738bb582c8b540af7bcfc18f87c575c3ed66e0",
+    "created": false,
+    "deleted": false,
+    "forced": true,
+    "base_ref": null,
+    "compare": "https://github.com/org/repo/compare/d5c1ffa8e294...63738bb582c8",
+    "commits": [
+      {
+        "id": "63738bb582c8b540af7bcfc18f87c575c3ed66e0",
+        "tree_id": "64897da445207e409ad05af93b1f349ad0a4ee19",
+        "distinct": true,
+        "message": "Add staging-argocd-demo environment",
+        "timestamp": "2018-05-04T15:40:02-07:00",
+        "url": "https://github.com/org/repo/commit/63738bb582c8b540af7bcfc18f87c575c3ed66e0",
+        "author": {
+          "name": "Jesse Suen",
+          "email": "[email protected]",
+          "username": "org"
+        },
+        "committer": {
+          "name": "Jesse Suen",
+          "email": "[email protected]",
+          "username": "org"
+        },
+        "added": [
+          "ksapps/test-app/environments/staging-argocd-demo/main.jsonnet",
+          "ksapps/test-app/environments/staging-argocd-demo/params.libsonnet"
+        ],
+        "removed": [
+
+        ],
+        "modified": [
+          "ksapps/test-app/app.yaml"
+        ]
+      }
+    ],
+    "head_commit": {
+      "id": "63738bb582c8b540af7bcfc18f87c575c3ed66e0",
+      "tree_id": "64897da445207e409ad05af93b1f349ad0a4ee19",
+      "distinct": true,
+      "message": "Add staging-argocd-demo environment",
+      "timestamp": "2018-05-04T15:40:02-07:00",
+      "url": "https://github.com/org/repo/commit/63738bb582c8b540af7bcfc18f87c575c3ed66e0",
+      "author": {
+        "name": "Jesse Suen",
+        "email": "[email protected]",
+        "username": "org"
+      },
+      "committer": {
+        "name": "Jesse Suen",
+        "email": "[email protected]",
+        "username": "org"
+      },
+      "added": [
+        "ksapps/test-app/environments/staging-argocd-demo/main.jsonnet",
+        "ksapps/test-app/environments/staging-argocd-demo/params.libsonnet"
+      ],
+      "removed": [
+
+      ],
+      "modified": [
+        "ksapps/test-app/app.yaml"
+      ]
+    },
+    "repository": {
+      "id": 123060978,
+      "name": "repo",
+      "full_name": "org/repo",
+      "owner": {
+        "name": "org",
+        "email": "[email protected]",
+        "login": "org",
+        "id": 12677113,
+        "avatar_url": "https://avatars0.githubusercontent.com/u/12677113?v=4",
+        "gravatar_id": "",
+        "url": "https://api.github.com/users/org",
+        "html_url": "https://github.com/org",
+        "followers_url": "https://api.github.com/users/org/followers",
+        "following_url": "https://api.github.com/users/org/following{/other_user}",
+        "gists_url": "https://api.github.com/users/org/gists{/gist_id}",
+        "starred_url": "https://api.github.com/users/org/starred{/owner}{/repo}",
+        "subscriptions_url": "https://api.github.com/users/org/subscriptions",
+        "organizations_url": "https://api.github.com/users/org/orgs",
+        "repos_url": "https://api.github.com/users/org/repos",
+        "events_url": "https://api.github.com/users/org/events{/privacy}",
+        "received_events_url": "https://api.github.com/users/org/received_events",
+        "type": "User",
+        "site_admin": false
+      },
+      "private": false,
+      "html_url": "https://github.com/org/repo",
+      "description": "Test Repository",
+      "fork": false,
+      "url": "https://github.com/org/repo",
+      "forks_url": "https://api.github.com/repos/org/repo/forks",
+      "keys_url": "https://api.github.com/repos/org/repo/keys{/key_id}",
+      "collaborators_url": "https://api.github.com/repos/org/repo/collaborators{/collaborator}",
+      "teams_url": "https://api.github.com/repos/org/repo/teams",
+      "hooks_url": "https://api.github.com/repos/org/repo/hooks",
+      "issue_events_url": "https://api.github.com/repos/org/repo/issues/events{/number}",
+      "events_url": "https://api.github.com/repos/org/repo/events",
+      "assignees_url": "https://api.github.com/repos/org/repo/assignees{/user}",
+      "branches_url": "https://api.github.com/repos/org/repo/branches{/branch}",
+      "tags_url": "https://api.github.com/repos/org/repo/tags",
+      "blobs_url": "https://api.github.com/repos/org/repo/git/blobs{/sha}",
+      "git_tags_url": "https://api.github.com/repos/org/repo/git/tags{/sha}",
+      "git_refs_url": "https://api.github.com/repos/org/repo/git/refs{/sha}",
+      "trees_url": "https://api.github.com/repos/org/repo/git/trees{/sha}",
+      "statuses_url": "https://api.github.com/repos/org/repo/statuses/{sha}",
+      "languages_url": "https://api.github.com/repos/org/repo/languages",
+      "stargazers_url": "https://api.github.com/repos/org/repo/stargazers",
+      "contributors_url": "https://api.github.com/repos/org/repo/contributors",
+      "subscribers_url": "https://api.github.com/repos/org/repo/subscribers",
+      "subscription_url": "https://api.github.com/repos/org/repo/subscription",
+      "commits_url": "https://api.github.com/repos/org/repo/commits{/sha}",
+      "git_commits_url": "https://api.github.com/repos/org/repo/git/commits{/sha}",
+      "comments_url": "https://api.github.com/repos/org/repo/comments{/number}",
+      "issue_comment_url": "https://api.github.com/repos/org/repo/issues/comments{/number}",
+      "contents_url": "https://api.github.com/repos/org/repo/contents/{+path}",
+      "compare_url": "https://api.github.com/repos/org/repo/compare/{base}...{head}",
+      "merges_url": "https://api.github.com/repos/org/repo/merges",
+      "archive_url": "https://api.github.com/repos/org/repo/{archive_format}{/ref}",
+      "downloads_url": "https://api.github.com/repos/org/repo/downloads",
+      "issues_url": "https://api.github.com/repos/org/repo/issues{/number}",
+      "pulls_url": "https://api.github.com/repos/org/repo/pulls{/number}",
+      "milestones_url": "https://api.github.com/repos/org/repo/milestones{/number}",
+      "notifications_url": "https://api.github.com/repos/org/repo/notifications{?since,all,participating}",
+      "labels_url": "https://api.github.com/repos/org/repo/labels{/name}",
+      "releases_url": "https://api.github.com/repos/org/repo/releases{/id}",
+      "deployments_url": "https://api.github.com/repos/org/repo/deployments",
+      "created_at": 1519698615,
+      "updated_at": "2018-05-04T22:37:55Z",
+      "pushed_at": 1525473610,
+      "git_url": "git://github.com/org/repo.git",
+      "ssh_url": "[email protected]:org/repo.git",
+      "clone_url": "https://github.com/org/repo.git",
+      "svn_url": "https://github.com/org/repo",
+      "homepage": null,
+      "size": 538,
+      "stargazers_count": 0,
+      "watchers_count": 0,
+      "language": null,
+      "has_issues": true,
+      "has_projects": true,
+      "has_downloads": true,
+      "has_wiki": true,
+      "has_pages": false,
+      "forks_count": 1,
+      "mirror_url": null,
+      "archived": false,
+      "open_issues_count": 0,
+      "license": null,
+      "forks": 1,
+      "open_issues": 0,
+      "watchers": 0,
+      "default_branch": "master",
+      "stargazers": 0,
+      "master_branch": "master"
+    },
+    "pusher": {
+      "name": "org",
+      "email": "[email protected]"
+    },
+    "sender": {
+      "login": "org",
+      "id": 12677113,
+      "avatar_url": "https://avatars0.githubusercontent.com/u/12677113?v=4",
+      "gravatar_id": "",
+      "url": "https://api.github.com/users/org",
+      "html_url": "https://github.com/org",
+      "followers_url": "https://api.github.com/users/org/followers",
+      "following_url": "https://api.github.com/users/org/following{/other_user}",
+      "gists_url": "https://api.github.com/users/org/gists{/gist_id}",
+      "starred_url": "https://api.github.com/users/org/starred{/owner}{/repo}",
+      "subscriptions_url": "https://api.github.com/users/org/subscriptions",
+      "organizations_url": "https://api.github.com/users/org/orgs",
+      "repos_url": "https://api.github.com/users/org/repos",
+      "events_url": "https://api.github.com/users/org/events{/privacy}",
+      "received_events_url": "https://api.github.com/users/org/received_events",
+      "type": "User",
+      "site_admin": false
+    }
+  }
diff --git a/applicationset/webhook/webhook.go b/applicationset/webhook/webhook.go
@@ -19,6 +19,7 @@ import (
 	"github.com/argoproj/argo-cd/v2/common"
 	"github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1"
 	argosettings "github.com/argoproj/argo-cd/v2/util/settings"
+	"github.com/argoproj/argo-cd/v2/util/webhook"
 
 	"github.com/go-playground/webhooks/v6/azuredevops"
 	"github.com/go-playground/webhooks/v6/github"
@@ -190,11 +191,6 @@ func (h *WebhookHandler) Handler(w http.ResponseWriter, r *http.Request) {
 	}
 }
 
-func parseRevision(ref string) string {
-	refParts := strings.SplitN(ref, "/", 3)
-	return refParts[len(refParts)-1]
-}
-
 func getGitGeneratorInfo(payload interface{}) *gitGeneratorInfo {
 	var (
 		webURL      string
@@ -204,16 +200,16 @@ func getGitGeneratorInfo(payload interface{}) *gitGeneratorInfo {
 	switch payload := payload.(type) {
 	case github.PushPayload:
 		webURL = payload.Repository.HTMLURL
-		revision = parseRevision(payload.Ref)
+		revision = webhook.ParseRevision(payload.Ref)
 		touchedHead = payload.Repository.DefaultBranch == revision
 	case gitlab.PushEventPayload:
 		webURL = payload.Project.WebURL
-		revision = parseRevision(payload.Ref)
+		revision = webhook.ParseRevision(payload.Ref)
 		touchedHead = payload.Project.DefaultBranch == revision
 	case azuredevops.GitPushEvent:
 		// See: https://learn.microsoft.com/en-us/azure/devops/service-hooks/events?view=azure-devops#git.push
 		webURL = payload.Resource.Repository.RemoteURL
-		revision = parseRevision(payload.Resource.RefUpdates[0].Name)
+		revision = webhook.ParseRevision(payload.Resource.RefUpdates[0].Name)
 		touchedHead = payload.Resource.RefUpdates[0].Name == payload.Resource.Repository.DefaultBranch
 		// unfortunately, Azure DevOps doesn't provide a list of changed files
 	default:
@@ -373,12 +369,12 @@ func shouldRefreshPluginGenerator(gen *v1alpha1.PluginGenerator) bool {
 }
 
 func genRevisionHasChanged(gen *v1alpha1.GitGenerator, revision string, touchedHead bool) bool {
-	targetRev := parseRevision(gen.Revision)
+	targetRev := webhook.ParseRevision(gen.Revision)
 	if targetRev == "HEAD" || targetRev == "" { // revision is head
 		return touchedHead
 	}
 
-	return targetRev == revision
+	return targetRev == revision || gen.Revision == revision
 }
 
 func gitGeneratorUsesURL(gen *v1alpha1.GitGenerator, webURL string, repoRegexp *regexp.Regexp) bool {

diff --git a/applicationset/webhook/webhook_test.go b/applicationset/webhook/webhook_test.go
@@ -67,6 +67,15 @@ func TestWebhookHandler(t *testing.T) {
 			expectedStatusCode: http.StatusOK,
 			expectedRefresh:    true,
 		},
+		{
+			desc:               "WebHook from a GitHub repository via Commit shorthand",
+			headerKey:          "X-GitHub-Event",
+			headerValue:        "push",
+			payloadFile:        "github-commit-event-feature-branch.json",
+			effectedAppSets:    []string{"github-shorthand", "matrix-pull-request-github-plugin", "plugin"},
+			expectedStatusCode: http.StatusOK,
+			expectedRefresh:    true,
+		},
 		{
 			desc:               "WebHook from a GitHub repository via Commit to branch",
 			headerKey:          "X-GitHub-Event",
@@ -192,6 +201,7 @@ func TestWebhookHandler(t *testing.T) {
 				fakeAppWithGitGenerator("git-github", namespace, "https://github.com/org/repo"),
 				fakeAppWithGitGenerator("git-gitlab", namespace, "https://gitlab/group/name"),
 				fakeAppWithGitGenerator("git-azure-devops", namespace, "https://dev.azure.com/fabrikam-fiber-inc/DefaultCollection/_git/Fabrikam-Fiber-Git"),
+				fakeAppWithGitGeneratorWithRevision("github-shorthand", namespace, "https://github.com/org/repo", "env/dev"),
 				fakeAppWithGithubPullRequestGenerator("pull-request-github", namespace, "CodErTOcat", "Hello-World"),
 				fakeAppWithGitlabPullRequestGenerator("pull-request-gitlab", namespace, "100500"),
 				fakeAppWithAzureDevOpsPullRequestGenerator("pull-request-azure-devops", namespace, "DefaultCollection", "Fabrikam"),
@@ -302,14 +312,62 @@ func mockGenerators() map[string]generators.Generator {
 }
 
 func TestGenRevisionHasChanged(t *testing.T) {
-	assert.True(t, genRevisionHasChanged(&v1alpha1.GitGenerator{}, "master", true))
-	assert.False(t, genRevisionHasChanged(&v1alpha1.GitGenerator{}, "master", false))
-
-	assert.True(t, genRevisionHasChanged(&v1alpha1.GitGenerator{Revision: "dev"}, "dev", true))
-	assert.False(t, genRevisionHasChanged(&v1alpha1.GitGenerator{Revision: "dev"}, "master", false))
-
-	assert.True(t, genRevisionHasChanged(&v1alpha1.GitGenerator{Revision: "refs/heads/dev"}, "dev", true))
-	assert.False(t, genRevisionHasChanged(&v1alpha1.GitGenerator{Revision: "refs/heads/dev"}, "master", false))
+	type args struct {
+		gen         *v1alpha1.GitGenerator
+		revision    string
+		touchedHead bool
+	}
+	tests := []struct {
+		name string
+		args args
+		want bool
+	}{
+		{name: "touchedHead", args: args{
+			gen:         &v1alpha1.GitGenerator{},
+			revision:    "main",
+			touchedHead: true,
+		}, want: true},
+		{name: "didntTouchHead", args: args{
+			gen:         &v1alpha1.GitGenerator{},
+			revision:    "main",
+			touchedHead: false,
+		}, want: false},
+		{name: "foundEqualShort", args: args{
+			gen:         &v1alpha1.GitGenerator{Revision: "dev"},
+			revision:    "dev",
+			touchedHead: true,
+		}, want: true},
+		{name: "foundEqualLongGen", args: args{
+			gen:         &v1alpha1.GitGenerator{Revision: "refs/heads/dev"},
+			revision:    "dev",
+			touchedHead: true,
+		}, want: true},
+		{name: "foundNotEqualLongGen", args: args{
+			gen:         &v1alpha1.GitGenerator{Revision: "refs/heads/dev"},
+			revision:    "main",
+			touchedHead: true,
+		}, want: false},
+		{name: "foundNotEqualShort", args: args{
+			gen:         &v1alpha1.GitGenerator{Revision: "dev"},
+			revision:    "main",
+			touchedHead: false,
+		}, want: false},
+		{name: "foundEqualTag", args: args{
+			gen:         &v1alpha1.GitGenerator{Revision: "v3.14.1"},
+			revision:    "v3.14.1",
+			touchedHead: false,
+		}, want: true},
+		{name: "foundEqualTagLongGen", args: args{
+			gen:         &v1alpha1.GitGenerator{Revision: "refs/tags/v3.14.1"},
+			revision:    "v3.14.1",
+			touchedHead: false,
+		}, want: true},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			assert.Equalf(t, tt.want, genRevisionHasChanged(tt.args.gen, tt.args.revision, tt.args.touchedHead), "genRevisionHasChanged(%v, %v, %v)", tt.args.gen, tt.args.revision, tt.args.touchedHead)
+		})
+	}
 }
 
 func fakeAppWithGitGenerator(name, namespace, repo string) *v1alpha1.ApplicationSet {
@@ -331,6 +389,12 @@ func fakeAppWithGitGenerator(name, namespace, repo string) *v1alpha1.Application
 	}
 }
 
+func fakeAppWithGitGeneratorWithRevision(name, namespace, repo, revision string) *v1alpha1.ApplicationSet {
+	appSet := fakeAppWithGitGenerator(name, namespace, repo)
+	appSet.Spec.Generators[0].Git.Revision = revision
+	return appSet
+}
+
 func fakeAppWithGitlabPullRequestGenerator(name, namespace, projectId string) *v1alpha1.ApplicationSet {
 	return &v1alpha1.ApplicationSet{
 		ObjectMeta: metav1.ObjectMeta{
@@ -711,7 +775,7 @@ func fakeAppWithMatrixAndPullRequestGeneratorWithPluginGenerator(name, namespace
 func newFakeClient(ns string) *kubefake.Clientset {
 	s := runtime.NewScheme()
 	s.AddKnownTypes(v1alpha1.SchemeGroupVersion, &v1alpha1.ApplicationSet{})
-	return kubefake.NewSimpleClientset(&corev1.ConfigMap{ObjectMeta: metav1.ObjectMeta{Name: "argocd-cm", Namespace: ns, Labels: map[string]string{
+	return kubefake.NewClientset(&corev1.ConfigMap{ObjectMeta: metav1.ObjectMeta{Name: "argocd-cm", Namespace: ns, Labels: map[string]string{
 		"app.kubernetes.io/part-of": "argocd",
 	}}}, &corev1.Secret{
 		ObjectMeta: metav1.ObjectMeta{

diff --git a/cmd/argocd-applicationset-controller/commands/applicationset_controller.go b/cmd/argocd-applicationset-controller/commands/applicationset_controller.go
@@ -270,7 +270,7 @@ func startWebhookServer(webhookHandler *webhook.WebhookHandler, webhookAddr stri
 	mux := http.NewServeMux()
 	mux.HandleFunc("/api/webhook", webhookHandler.Handler)
 	go func() {
-		log.Info("Starting webhook server")
+		log.Infof("Starting webhook server %s", webhookAddr)
 		err := http.ListenAndServe(webhookAddr, mux)
 		if err != nil {
 			log.Error(err, "failed to start webhook server")