Add pull request support to SCM generator

This commit adds pull request support to SCM generator so the generator
can create ArgoCD apps for PRs as well.

Fixes argoproj#466

Signed-off-by: Fardin Khanjani <[email protected]>

api/v1alpha1/applicationset_types.go

@@ -314,6 +314,8 @@ type SCMProviderGeneratorGithub struct {
 	TokenRef *SecretRef `json:"tokenRef,omitempty"`
 	// Scan all branches instead of just the default branch.
 	AllBranches bool `json:"allBranches,omitempty"`
+	// Scan all pull requests
+	AllPullRequests bool `json:"allPullRequests,omitempty"`
 }
 
 // SCMProviderGeneratorGitlab defines a connection info specific to Gitlab.

manifests/crds/argoproj.io_applicationsets.yaml

@@ -4800,6 +4800,8 @@ spec:
                                     properties:
                                       allBranches:
                                         type: boolean
+                                      allPullRequests:
+                                        type: boolean
                                       api:
                                         type: string
                                       organization:
@@ -5707,6 +5709,8 @@ spec:
                           properties:
                             allBranches:
                               type: boolean
+                            allPullRequests:
+                              type: boolean
                             api:
                               type: string
                             organization:

manifests/install-with-argo-cd.yaml

@@ -6617,6 +6617,8 @@ spec:
                                     properties:
                                       allBranches:
                                         type: boolean
+                                      allPullRequests:
+                                        type: boolean
                                       api:
                                         type: string
                                       organization:
@@ -7524,6 +7526,8 @@ spec:
                           properties:
                             allBranches:
                               type: boolean
+                            allPullRequests:
+                              type: boolean
                             api:
                               type: string
                             organization:

manifests/install.yaml

@@ -4799,6 +4799,8 @@ spec:
                                     properties:
                                       allBranches:
                                         type: boolean
+                                      allPullRequests:
+                                        type: boolean
                                       api:
                                         type: string
                                       organization:
@@ -5706,6 +5708,8 @@ spec:
                           properties:
                             allBranches:
                               type: boolean
+                            allPullRequests:
+                              type: boolean
                             api:
                               type: string
                             organization:

pkg/generators/scm_provider.go

@@ -64,7 +64,7 @@ func (g *SCMProviderGenerator) GenerateParams(appSetGenerator *argoprojiov1alpha
 		if err != nil {
 			return nil, fmt.Errorf("error fetching Github token: %v", err)
 		}
-		provider, err = scm_provider.NewGithubProvider(ctx, providerConfig.Github.Organization, token, providerConfig.Github.API, providerConfig.Github.AllBranches)
+		provider, err = scm_provider.NewGithubProvider(ctx, providerConfig.Github.Organization, token, providerConfig.Github.API, providerConfig.Github.AllBranches, providerConfig.Github.AllPullRequests)
 		if err != nil {
 			return nil, fmt.Errorf("error initializing Github service: %v", err)
 		}
@@ -92,7 +92,7 @@ func (g *SCMProviderGenerator) GenerateParams(appSetGenerator *argoprojiov1alpha
 			"organization": repo.Organization,
 			"repository":   repo.Repository,
 			"url":          repo.URL,
-			"branch":       repo.Branch,
+			"revision":     repo.Revision,
 			"sha":          repo.SHA,
 			"labels":       strings.Join(repo.Labels, ","),
 		})

pkg/generators/scm_provider_test.go

@@ -86,15 +86,15 @@ func TestSCMProviderGenerateParams(t *testing.T) {
 				Organization: "myorg",
 				Repository:   "repo1",
 				URL:          "[email protected]:myorg/repo1.git",
-				Branch:       "main",
+				Revision:     "main",
 				SHA:          "abcd1234",
 				Labels:       []string{"prod", "staging"},
 			},
 			{
 				Organization: "myorg",
 				Repository:   "repo2",
 				URL:          "[email protected]:myorg/repo2.git",
-				Branch:       "main",
+				Revision:     "main",
 				SHA:          "00000000",
 			},
 		},
@@ -108,7 +108,7 @@ func TestSCMProviderGenerateParams(t *testing.T) {
 	assert.Equal(t, "myorg", params[0]["organization"])
 	assert.Equal(t, "repo1", params[0]["repository"])
 	assert.Equal(t, "[email protected]:myorg/repo1.git", params[0]["url"])
-	assert.Equal(t, "main", params[0]["branch"])
+	assert.Equal(t, "main", params[0]["revision"])
 	assert.Equal(t, "abcd1234", params[0]["sha"])
 	assert.Equal(t, "prod,staging", params[0]["labels"])
 	assert.Equal(t, "repo2", params[1]["repository"])

pkg/services/scm_provider/github.go

@@ -4,20 +4,22 @@ import (
 	"context"
 	"fmt"
 	"os"
+	"strconv"
 
 	"github.com/google/go-github/v35/github"
 	"golang.org/x/oauth2"
 )
 
 type GithubProvider struct {
-	client       *github.Client
-	organization string
-	allBranches  bool
+	client          *github.Client
+	organization    string
+	allBranches     bool
+	allPullRequests bool
 }
 
 var _ SCMProviderService = &GithubProvider{}
 
-func NewGithubProvider(ctx context.Context, organization string, token string, url string, allBranches bool) (*GithubProvider, error) {
+func NewGithubProvider(ctx context.Context, organization string, token string, url string, allBranches bool, allPullRequests bool) (*GithubProvider, error) {
 	var ts oauth2.TokenSource
 	// Undocumented environment variable to set a default token, to be used in testing to dodge anonymous rate limits.
 	if token == "" {
@@ -39,7 +41,8 @@ func NewGithubProvider(ctx context.Context, organization string, token string, u
 			return nil, err
 		}
 	}
-	return &GithubProvider{client: client, organization: organization, allBranches: allBranches}, nil
+
+	return &GithubProvider{client: client, organization: organization, allBranches: allBranches, allPullRequests: allPullRequests}, nil
 }
 
 func (g *GithubProvider) ListRepos(ctx context.Context, cloneProtocol string) ([]*Repository, error) {
@@ -74,11 +77,29 @@ func (g *GithubProvider) ListRepos(ctx context.Context, cloneProtocol string) ([
 					Organization: githubRepo.Owner.GetLogin(),
 					Repository:   githubRepo.GetName(),
 					URL:          url,
-					Branch:       branch.GetName(),
+					Revision:     branch.GetName(),
 					SHA:          branch.GetCommit().GetSHA(),
 					Labels:       githubRepo.Topics,
 				})
 			}
+
+			if g.allPullRequests {
+				pullRequests, err := g.listPullRequests(ctx, githubRepo)
+				if err != nil {
+					return nil, fmt.Errorf("error listing pull requests for %s/%s: %v", githubRepo.Owner.GetLogin(), githubRepo.GetName(), err)
+				}
+
+				for _, pr := range pullRequests {
+					repos = append(repos, &Repository{
+						Organization: githubRepo.Owner.GetLogin(),
+						Repository:   githubRepo.GetName(),
+						URL:          url,
+						Revision:     strconv.FormatInt(int64(pr.GetNumber()), 10), // PR number is an int
+						SHA:          pr.GetHead().GetSHA(),
+						Labels:       githubRepo.Topics,
+					})
+				}
+			}
 		}
 		if resp.NextPage == 0 {
 			break
@@ -90,7 +111,7 @@ func (g *GithubProvider) ListRepos(ctx context.Context, cloneProtocol string) ([
 
 func (g *GithubProvider) RepoHasPath(ctx context.Context, repo *Repository, path string) (bool, error) {
 	_, _, resp, err := g.client.Repositories.GetContents(ctx, repo.Organization, repo.Repository, path, &github.RepositoryContentGetOptions{
-		Ref: repo.Branch,
+		Ref: repo.SHA,
 	})
 	// 404s are not an error here, just a normal false.
 	if resp != nil && resp.StatusCode == 404 {
@@ -132,3 +153,29 @@ func (g *GithubProvider) listBranches(ctx context.Context, repo *github.Reposito
 	}
 	return branches, nil
 }
+
+func (g *GithubProvider) listPullRequests(ctx context.Context, repo *github.Repository) ([]github.PullRequest, error) {
+	opt := &github.PullRequestListOptions{
+		ListOptions: github.ListOptions{PerPage: 100},
+	}
+
+	pullRequests := []github.PullRequest{}
+
+	for {
+		allPullRequests, resp, err := g.client.PullRequests.List(ctx, repo.Owner.GetLogin(), repo.GetName(), opt)
+		if err != nil {
+			return nil, err
+		}
+
+		for _, pr := range allPullRequests {
+			pullRequests = append(pullRequests, *pr)
+		}
+
+		if resp.NextPage == 0 {
+			break
+		}
+		opt.Page = resp.NextPage
+	}
+
+	return pullRequests, nil
+}

pkg/services/scm_provider/github_test.go

@@ -35,9 +35,9 @@ func checkRateLimit(t *testing.T, err error) {
 
 func TestGithubListRepos(t *testing.T) {
 	cases := []struct {
-		name, proto, url      string
-		hasError, allBranches bool
-		branches              []string
+		name, proto, url                       string
+		hasError, allBranches, allPullRequests bool
+		branches                               []string
 	}{
 		{
 			name:     "blank protocol",
@@ -65,11 +65,17 @@ func TestGithubListRepos(t *testing.T) {
 			url:         "[email protected]:argoproj/applicationset.git",
 			branches:    []string{"master", "release-0.1.0"},
 		},
+		{
+			name:            "all branches",
+			allPullRequests: false,
+			url:             "[email protected]:argoproj/applicationset.git",
+			branches:        []string{"master", "release-0.1.0"},
+		},
 	}
 
 	for _, c := range cases {
 		t.Run(c.name, func(t *testing.T) {
-			provider, _ := NewGithubProvider(context.Background(), "argoproj", "", "", c.allBranches)
+			provider, _ := NewGithubProvider(context.Background(), "argoproj", "", "", c.allBranches, c.allPullRequests)
 			rawRepos, err := provider.ListRepos(context.Background(), c.proto)
 			if c.hasError {
 				assert.NotNil(t, err)
@@ -82,7 +88,7 @@ func TestGithubListRepos(t *testing.T) {
 				for _, r := range rawRepos {
 					if r.Repository == "applicationset" {
 						repos = append(repos, r)
-						branches = append(branches, r.Branch)
+						branches = append(branches, r.Revision)
 					}
 				}
 				assert.NotEmpty(t, repos)
@@ -96,11 +102,11 @@ func TestGithubListRepos(t *testing.T) {
 }
 
 func TestGithubHasPath(t *testing.T) {
-	host, _ := NewGithubProvider(context.Background(), "argoproj", "", "", false)
+	host, _ := NewGithubProvider(context.Background(), "argoproj", "", "", false, false)
 	repo := &Repository{
 		Organization: "argoproj",
 		Repository:   "applicationset",
-		Branch:       "master",
+		Revision:     "master",
 	}
 	ok, err := host.RepoHasPath(context.Background(), repo, "pkg/")
 	checkRateLimit(t, err)

pkg/services/scm_provider/gitlab.go

@@ -72,7 +72,7 @@ func (g *GitlabProvider) ListRepos(ctx context.Context, cloneProtocol string) ([
 					Organization: gitlabRepo.Namespace.FullPath,
 					Repository:   gitlabRepo.Path,
 					URL:          url,
-					Branch:       branch.Name,
+					Revision:     branch.Name,
 					SHA:          branch.Commit.ID,
 					Labels:       gitlabRepo.TagList,
 				})
@@ -93,7 +93,7 @@ func (g *GitlabProvider) RepoHasPath(_ context.Context, repo *Repository, path s
 	}
 	_, resp, err := g.client.Repositories.ListTree(p.ID, &gitlab.ListTreeOptions{
 		Path: &path,
-		Ref:  &repo.Branch,
+		Ref:  &repo.Revision,
 	})
 	if err != nil {
 		return false, err

pkg/services/scm_provider/gitlab_test.go

@@ -56,7 +56,7 @@ func TestGitlabListRepos(t *testing.T) {
 				for _, r := range rawRepos {
 					if r.Repository == "argocd" {
 						repos = append(repos, r)
-						branches = append(branches, r.Branch)
+						branches = append(branches, r.Revision)
 					}
 				}
 				assert.NotEmpty(t, repos)
@@ -74,7 +74,7 @@ func TestGitlabHasPath(t *testing.T) {
 	repo := &Repository{
 		Organization: "test-argocd-proton",
 		Repository:   "argocd",
-		Branch:       "master",
+		Revision:     "master",
 	}
 	ok, err := host.RepoHasPath(context.Background(), repo, "argocd")
 	assert.Nil(t, err)

pkg/services/scm_provider/types.go

@@ -10,7 +10,7 @@ type Repository struct {
 	Organization string
 	Repository   string
 	URL          string
-	Branch       string
+	Revision     string // Would love suggestions on better alternatives to "Revision". Something that can represent both a branch and a PR
 	SHA          string
 	Labels       []string
 }

pkg/services/scm_provider/utils.go

@@ -44,7 +44,7 @@ func matchFilter(ctx context.Context, provider SCMProviderService, repo *Reposit
 		return false, nil
 	}
 
-	if filter.BranchMatch != nil && !filter.BranchMatch.MatchString(repo.Branch) {
+	if filter.BranchMatch != nil && !filter.BranchMatch.MatchString(repo.Revision) {
 		return false, nil
 	}
 

pkg/services/scm_provider/utils_test.go

@@ -134,23 +134,23 @@ func TestFilterBranchMatch(t *testing.T) {
 		Repos: []*Repository{
 			{
 				Repository: "one",
-				Branch:     "one",
+				Revision:   "one",
 			},
 			{
 				Repository: "one",
-				Branch:     "two",
+				Revision:   "two",
 			},
 			{
 				Repository: "two",
-				Branch:     "one",
+				Revision:   "one",
 			},
 			{
 				Repository: "three",
-				Branch:     "one",
+				Revision:   "one",
 			},
 			{
 				Repository: "three",
-				Branch:     "two",
+				Revision:   "two",
 			},
 		},
 	}
@@ -163,9 +163,9 @@ func TestFilterBranchMatch(t *testing.T) {
 	assert.Nil(t, err)
 	assert.Len(t, repos, 2)
 	assert.Equal(t, "one", repos[0].Repository)
-	assert.Equal(t, "two", repos[0].Branch)
+	assert.Equal(t, "two", repos[0].Revision)
 	assert.Equal(t, "three", repos[1].Repository)
-	assert.Equal(t, "two", repos[1].Branch)
+	assert.Equal(t, "two", repos[1].Revision)
 }
 
 func TestMultiFilterAnd(t *testing.T) {