Skip to content
This repository has been archived by the owner on Jul 15, 2024. It is now read-only.

feat: Add pull request support to SCM generator #469

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions api/v1alpha1/applicationset_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"`
Comment on lines 315 to +318
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There should be documentation on what to expect the generator to return if both allBranches and allPullRequests are true.

}

// SCMProviderGeneratorGitlab defines a connection info specific to Gitlab.
Expand All @@ -328,6 +330,8 @@ type SCMProviderGeneratorGitlab 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"`
}

// SCMProviderGeneratorFilter is a single repository filter.
Expand All @@ -342,6 +346,10 @@ type SCMProviderGeneratorFilter struct {
LabelMatch *string `json:"labelMatch,omitempty"`
// A regex which must match the branch name.
BranchMatch *string `json:"branchMatch,omitempty"`
// A regex which must match the branch name.
PullRequestBranchMatch *string `json:"pullRequestTitleMatch,omitempty"`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should there be two filters, one to match source branches, and the other to match target branches?

Copy link
Author

@fardin01 fardin01 Mar 4, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's an interesting point! I can't recall ever seeing a source branch being merged into a non-default target branch (main, master etc.). In addition, usually, all source branches get merged into one single target branch (otherwise you got a very messy repo 😄 ) which makes filtering the target branch pointless. So IMHO this feature would only support a "corner case". What do you think?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider this use case: I want to spin up test environments for each PR against main. Developer A opens a PR from feature/whatever to main. Developer B has suggestions but instead of writing them all out, just opens a PR from feature/whatever-review to feature/whatever.

I think this is common enough to justify giving the AppSet the designer to avoid creating an App for that second PR.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But if it ends up being a lot of work, we could certainly leave it for a future enhancement.

// A regex which must match at least one pull request label.
PullRequestLabelMatch *string `json:"pullRequestLabelMatch,omitempty"`
}

// PullRequestGenerator defines a generator that scrapes a PullRequest API to find candidate pull requests.
Expand Down
10 changes: 10 additions & 0 deletions api/v1alpha1/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@ require (
github.com/opencontainers/go-digest v1.0.0 // indirect
github.com/patrickmn/go-cache v2.1.0+incompatible // indirect
github.com/peterbourgon/diskv v2.0.1+incompatible // indirect
github.com/pkg/errors v0.9.1 // indirect
github.com/pmezard/go-difflib v1.0.0 // indirect
github.com/prometheus/client_golang v1.11.0 // indirect
github.com/prometheus/client_model v0.2.0 // indirect
Expand Down
24 changes: 24 additions & 0 deletions manifests/crds/argoproj.io_applicationsets.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -2778,6 +2778,10 @@ spec:
items:
type: string
type: array
pullRequestLabelMatch:
type: string
pullRequestTitleMatch:
type: string
repositoryMatch:
type: string
type: object
Expand All @@ -2786,6 +2790,8 @@ spec:
properties:
allBranches:
type: boolean
allPullRequests:
type: boolean
api:
type: string
organization:
Expand All @@ -2807,6 +2813,8 @@ spec:
properties:
allBranches:
type: boolean
allPullRequests:
type: boolean
api:
type: string
group:
Expand Down Expand Up @@ -4920,6 +4928,10 @@ spec:
items:
type: string
type: array
pullRequestLabelMatch:
type: string
pullRequestTitleMatch:
type: string
repositoryMatch:
type: string
type: object
Expand All @@ -4928,6 +4940,8 @@ spec:
properties:
allBranches:
type: boolean
allPullRequests:
type: boolean
api:
type: string
organization:
Expand All @@ -4949,6 +4963,8 @@ spec:
properties:
allBranches:
type: boolean
allPullRequests:
type: boolean
api:
type: string
group:
Expand Down Expand Up @@ -5851,6 +5867,10 @@ spec:
items:
type: string
type: array
pullRequestLabelMatch:
type: string
pullRequestTitleMatch:
type: string
repositoryMatch:
type: string
type: object
Expand All @@ -5859,6 +5879,8 @@ spec:
properties:
allBranches:
type: boolean
allPullRequests:
type: boolean
api:
type: string
organization:
Expand All @@ -5880,6 +5902,8 @@ spec:
properties:
allBranches:
type: boolean
allPullRequests:
type: boolean
api:
type: string
group:
Expand Down
Loading