diff --git a/docs/docs/35-references/10-promotion-steps.md b/docs/docs/35-references/10-promotion-steps.md index 38fdc2bd3..4bb1b8e81 100644 --- a/docs/docs/35-references/10-promotion-steps.md +++ b/docs/docs/35-references/10-promotion-steps.md @@ -1021,6 +1021,7 @@ requests. | `targetBranch` | `string` | N | The branch to which the changes should be merged. | | `createTargetBranch` | `boolean` | N | Indicates whether a new, empty orphaned branch should be created and pushed to the remote if the target branch does not already exist there. Default is `false`. | | `title` | `string` | N | The title for the pull request. Kargo generates a title based on the commit messages if it is not explicitly specified. | +| `labels` | `[]string` | N | Labels to add to the pull request. | #### `git-open-pr` Example diff --git a/go.mod b/go.mod index af4b52c71..16c9517cf 100644 --- a/go.mod +++ b/go.mod @@ -122,6 +122,7 @@ require ( github.com/russross/blackfriday/v2 v2.1.0 // indirect github.com/shopspring/decimal v1.4.0 // indirect github.com/spf13/cast v1.7.0 // indirect + github.com/stretchr/objx v0.5.2 // indirect github.com/valyala/bytebufferpool v1.0.0 // indirect github.com/x448/float16 v0.8.4 // indirect github.com/xeipuuv/gojsonpointer v0.0.0-20190905194746-02993c407bfb // indirect diff --git a/internal/directives/git_pr_opener.go b/internal/directives/git_pr_opener.go index bcb080dd1..45d53b429 100644 --- a/internal/directives/git_pr_opener.go +++ b/internal/directives/git_pr_opener.go @@ -218,6 +218,7 @@ func (g *gitPROpener) runPromotionStep( Base: cfg.TargetBranch, Title: title, Description: description, + Labels: cfg.Labels, }, ); err != nil { return PromotionStepResult{Status: kargoapi.PromotionPhaseErrored}, diff --git a/internal/directives/schemas/git-open-pr-config.json b/internal/directives/schemas/git-open-pr-config.json index 6438c32fe..ba574a65f 100644 --- a/internal/directives/schemas/git-open-pr-config.json +++ b/internal/directives/schemas/git-open-pr-config.json @@ -43,6 +43,15 @@ "type": "string", "description": "The title for the pull request. Kargo generates a title based on the commit messages if it is not explicitly specified.", "minLength": 1 + }, + "labels": { + "type": "array", + "description": "Labels to add to the pull request.", + "items": { + "type": "string", + "description": "A pull request label", + "minLength": 1 + } } }, "oneOf": [ diff --git a/internal/directives/zz_config_types.go b/internal/directives/zz_config_types.go index d97c0a2eb..c4a3053b1 100644 --- a/internal/directives/zz_config_types.go +++ b/internal/directives/zz_config_types.go @@ -187,6 +187,8 @@ type GitOpenPRConfig struct { CreateTargetBranch bool `json:"createTargetBranch,omitempty"` // Indicates whether to skip TLS verification when cloning the repository. Default is false. InsecureSkipTLSVerify bool `json:"insecureSkipTLSVerify,omitempty"` + // Labels to add to the pull request. + Labels []string `json:"labels,omitempty"` // The name of the Git provider to use. Currently only 'github', 'gitlab' and 'azure' are // supported. Kargo will try to infer the provider if it is not explicitly specified. Provider *Provider `json:"provider,omitempty"` diff --git a/internal/gitprovider/azure/azure.go b/internal/gitprovider/azure/azure.go index dcf97bad5..c33e0f26f 100644 --- a/internal/gitprovider/azure/azure.go +++ b/internal/gitprovider/azure/azure.go @@ -7,6 +7,7 @@ import ( "strings" "github.com/microsoft/azure-devops-go-api/azuredevops/v7" + adocore "github.com/microsoft/azure-devops-go-api/azuredevops/v7/core" adogit "github.com/microsoft/azure-devops-go-api/azuredevops/v7/git" "k8s.io/utils/ptr" @@ -94,6 +95,12 @@ func (p *provider) CreatePullRequest( return nil, fmt.Errorf("error getting repository %q: %w", p.repo, err) } repoID := ptr.To(repository.Id.String()) + labels := make([]adocore.WebApiTagDefinition, 0, len(opts.Labels)) + for _, label := range opts.Labels { + labels = append(labels, adocore.WebApiTagDefinition{ + Name: &label, + }) + } sourceRefName := ptr.To(fmt.Sprintf("refs/heads/%s", opts.Head)) targetRefName := ptr.To(fmt.Sprintf("refs/heads/%s", opts.Base)) adoPR, err := gitClient.CreatePullRequest(ctx, adogit.CreatePullRequestArgs{ @@ -102,6 +109,7 @@ func (p *provider) CreatePullRequest( GitPullRequestToCreate: &adogit.GitPullRequest{ Title: &opts.Title, Description: &opts.Description, + Labels: &labels, SourceRefName: sourceRefName, TargetRefName: targetRefName, }, diff --git a/internal/gitprovider/github/github.go b/internal/gitprovider/github/github.go index 7964027e4..84f634704 100644 --- a/internal/gitprovider/github/github.go +++ b/internal/gitprovider/github/github.go @@ -41,11 +41,42 @@ func init() { gitprovider.Register(ProviderName, registration) } +type githubClient interface { + CreatePullRequest( + ctx context.Context, + owner string, + repo string, + pull *github.NewPullRequest, + ) (*github.PullRequest, *github.Response, error) + + ListPullRequests( + ctx context.Context, + owner string, + repo string, + opts *github.PullRequestListOptions, + ) ([]*github.PullRequest, *github.Response, error) + + GetPullRequests( + ctx context.Context, + owner string, + repo string, + number int, + ) (*github.PullRequest, *github.Response, error) + + AddLabelsToIssue( + ctx context.Context, + owner string, + repo string, + number int, + labels []string, + ) ([]*github.Label, *github.Response, error) +} + // provider is a GitHub implementation of gitprovider.Interface. type provider struct { // nolint: revive owner string repo string - client *github.Client + client githubClient } // NewProvider returns a GitHub-based implementation of gitprovider.Interface. @@ -81,10 +112,51 @@ func NewProvider( return &provider{ owner: owner, repo: repo, - client: client, + client: &githubClientWrapper{client}, }, nil } +type githubClientWrapper struct { + client *github.Client +} + +func (g githubClientWrapper) CreatePullRequest( + ctx context.Context, + owner string, + repo string, + pull *github.NewPullRequest, +) (*github.PullRequest, *github.Response, error) { + return g.client.PullRequests.Create(ctx, owner, repo, pull) +} + +func (g githubClientWrapper) ListPullRequests( + ctx context.Context, + owner string, + repo string, + opts *github.PullRequestListOptions, +) ([]*github.PullRequest, *github.Response, error) { + return g.client.PullRequests.List(ctx, owner, repo, opts) +} + +func (g githubClientWrapper) GetPullRequests( + ctx context.Context, + owner string, + repo string, + number int, +) (*github.PullRequest, *github.Response, error) { + return g.client.PullRequests.Get(ctx, owner, repo, number) +} + +func (g githubClientWrapper) AddLabelsToIssue( + ctx context.Context, + owner string, + repo string, + number int, + labels []string, +) ([]*github.Label, *github.Response, error) { + return g.client.Issues.AddLabelsToIssue(ctx, owner, repo, number, labels) +} + // CreatePullRequest implements gitprovider.Interface. func (p *provider) CreatePullRequest( ctx context.Context, @@ -93,7 +165,7 @@ func (p *provider) CreatePullRequest( if opts == nil { opts = &gitprovider.CreatePullRequestOpts{} } - ghPR, _, err := p.client.PullRequests.Create(ctx, + ghPR, _, err := p.client.CreatePullRequest(ctx, p.owner, p.repo, &github.NewPullRequest{ @@ -111,6 +183,17 @@ func (p *provider) CreatePullRequest( return nil, fmt.Errorf("unexpected nil pull request") } pr := convertGithubPR(*ghPR) + if len(opts.Labels) > 0 { + _, _, err = p.client.AddLabelsToIssue(ctx, + p.owner, + p.repo, + int(pr.Number), + opts.Labels, + ) + } + if err != nil { + return nil, err + } return &pr, nil } @@ -119,7 +202,7 @@ func (p *provider) GetPullRequest( ctx context.Context, id int64, ) (*gitprovider.PullRequest, error) { - ghPR, _, err := p.client.PullRequests.Get(ctx, p.owner, p.repo, int(id)) + ghPR, _, err := p.client.GetPullRequests(ctx, p.owner, p.repo, int(id)) if err != nil { return nil, err } @@ -160,7 +243,7 @@ func (p *provider) ListPullRequests( } prs := []gitprovider.PullRequest{} for { - ghPRs, res, err := p.client.PullRequests.List(ctx, p.owner, p.repo, &listOpts) + ghPRs, res, err := p.client.ListPullRequests(ctx, p.owner, p.repo, &listOpts) if err != nil { return nil, err } diff --git a/internal/gitprovider/github/github_test.go b/internal/gitprovider/github/github_test.go index f35753b90..46e937ff6 100644 --- a/internal/gitprovider/github/github_test.go +++ b/internal/gitprovider/github/github_test.go @@ -1,11 +1,19 @@ package github import ( + "context" "testing" + "github.com/google/go-github/v56/github" + "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" + + "github.com/akuity/kargo/internal/gitprovider" ) +const testRepoOwner = "akuity" +const testRepoName = "kargo" + func TestParseGitHubURL(t *testing.T) { testCases := []struct { url string @@ -57,3 +65,283 @@ func TestParseGitHubURL(t *testing.T) { }) } } + +type mockGithubClient struct { + mock.Mock + pr *github.PullRequest + owner string + repo string + newPr *github.NewPullRequest + labels []string + listOpts *github.PullRequestListOptions +} + +func (m *mockGithubClient) ListPullRequests( + ctx context.Context, + owner string, + repo string, + opts *github.PullRequestListOptions, +) ([]*github.PullRequest, *github.Response, error) { + args := m.Called(ctx, owner, repo, opts) + m.owner = owner + m.repo = repo + m.listOpts = opts + prs, ok := args.Get(0).([]*github.PullRequest) + if !ok { + return nil, nil, args.Error(2) + } + resp, ok := args.Get(1).(*github.Response) + if !ok { + return prs, nil, args.Error(2) + } + return prs, resp, args.Error(2) +} + +func (m *mockGithubClient) GetPullRequests( + ctx context.Context, + owner string, + repo string, + number int, +) (*github.PullRequest, *github.Response, error) { + args := m.Called(ctx, owner, repo, number) + m.owner = owner + m.repo = repo + pr, ok := args.Get(0).(*github.PullRequest) + if !ok { + return nil, nil, args.Error(2) + } + resp, ok := args.Get(1).(*github.Response) + if !ok { + return pr, nil, args.Error(2) + } + return pr, resp, args.Error(2) +} + +func (m *mockGithubClient) AddLabelsToIssue( + ctx context.Context, + owner string, + repo string, + number int, + labels []string, +) ([]*github.Label, *github.Response, error) { + args := m.Called(ctx, owner, repo, number, labels) + m.labels = labels + labelsResp, ok := args.Get(0).([]*github.Label) + if !ok { + return nil, nil, args.Error(2) + } + resp, ok := args.Get(1).(*github.Response) + if !ok { + return labelsResp, nil, args.Error(2) + } + return labelsResp, resp, args.Error(2) +} +func (m *mockGithubClient) CreatePullRequest( + ctx context.Context, + owner string, + repo string, + pull *github.NewPullRequest, +) (*github.PullRequest, *github.Response, error) { + args := m.Called(ctx, owner, repo, pull) + m.owner = owner + m.repo = repo + m.newPr = pull + + pr, ok := args.Get(0).(*github.PullRequest) + if !ok { + return nil, nil, args.Error(2) + } + resp, ok := args.Get(1).(*github.Response) + if !ok { + return pr, nil, args.Error(2) + } + return pr, resp, args.Error(2) +} + +func TestCreatePullRequestWithLabels(t *testing.T) { + opts := gitprovider.CreatePullRequestOpts{ + Head: "feature-branch", + Base: "main", + Title: "title", + Description: "desc", + Labels: []string{"label1", "label2"}, + } + + // set up mock + mockClient := &mockGithubClient{ + pr: &github.PullRequest{ + Number: github.Int(42), + MergeCommitSHA: github.String("sha"), + State: github.String("open"), + URL: github.String("url"), + }, + } + mockClient. + On("CreatePullRequest", context.Background(), testRepoOwner, testRepoName, mock.Anything). + Return( + &github.PullRequest{ + Number: mockClient.pr.Number, + Head: &github.PullRequestBranch{ + Ref: github.String(opts.Head), + }, + Base: &github.PullRequestBranch{ + Ref: github.String(opts.Base), + }, + Title: github.String(opts.Title), + Body: github.String(opts.Description), + MergeCommitSHA: mockClient.pr.MergeCommitSHA, + State: mockClient.pr.State, + HTMLURL: mockClient.pr.URL, + }, + &github.Response{}, + nil, + ) + mockClient. + On("AddLabelsToIssue", context.Background(), testRepoOwner, testRepoName, *mockClient.pr.Number, mock.Anything). + Return( + []*github.Label{}, + &github.Response{}, + nil, + ) + + // call the code we are testing + g := provider{ + owner: testRepoOwner, + repo: testRepoName, + client: mockClient, + } + pr, err := g.CreatePullRequest(context.Background(), &opts) + + // assert that the expectations were met + mockClient.AssertExpectations(t) + + // other assertions + require.NoError(t, err) + require.Equal(t, testRepoOwner, mockClient.owner) + require.Equal(t, testRepoName, mockClient.repo) + require.Equal(t, opts.Head, *mockClient.newPr.Head) + require.Equal(t, opts.Base, *mockClient.newPr.Base) + require.Equal(t, opts.Title, *mockClient.newPr.Title, + "Expected title in new PR request to match title from options") + require.Equal(t, opts.Description, *mockClient.newPr.Body, + "Expected body in new PR request to match description from options") + require.ElementsMatch(t, opts.Labels, mockClient.labels, + "Expected labels passed to GitHub client to match labels from options") + + require.Equal(t, int64(*mockClient.pr.Number), pr.Number, + "Expected PR number in returned object to match what was returned by GitHub") + require.Equal(t, *mockClient.pr.MergeCommitSHA, pr.MergeCommitSHA) + require.Equal(t, *mockClient.pr.URL, pr.URL) + require.True(t, pr.Open) +} + +func TestGetPullRequest(t *testing.T) { + // set up mock + mockClient := &mockGithubClient{ + pr: &github.PullRequest{ + Number: github.Int(42), + MergeCommitSHA: github.String("sha"), + State: github.String("open"), + URL: github.String("url"), + }, + } + mockClient. + On("GetPullRequests", context.Background(), testRepoOwner, testRepoName, *mockClient.pr.Number). + Return( + &github.PullRequest{ + Number: mockClient.pr.Number, + Head: &github.PullRequestBranch{ + Ref: github.String("head"), + }, + MergeCommitSHA: mockClient.pr.MergeCommitSHA, + State: mockClient.pr.State, + HTMLURL: mockClient.pr.URL, + }, + &github.Response{}, + nil, + ) + + // call the code we are testing + g := provider{ + owner: testRepoOwner, + repo: testRepoName, + client: mockClient, + } + pr, err := g.GetPullRequest(context.Background(), 42) + + // assert that the expectations were met + mockClient.AssertExpectations(t) + + // other assertions + require.NoError(t, err) + require.Equal(t, testRepoOwner, mockClient.owner) + require.Equal(t, testRepoName, mockClient.repo) + require.Equal(t, int64(*mockClient.pr.Number), pr.Number, + "Expected PR number in returned object to match what was returned by GitHub") + require.Equal(t, *mockClient.pr.MergeCommitSHA, pr.MergeCommitSHA) + require.Equal(t, *mockClient.pr.URL, pr.URL) + require.True(t, pr.Open) +} + +func TestListPullRequests(t *testing.T) { + opts := gitprovider.ListPullRequestOptions{ + State: gitprovider.PullRequestStateAny, + HeadBranch: "head", + BaseBranch: "base", + } + + // set up mock + mockClient := &mockGithubClient{ + pr: &github.PullRequest{ + Number: github.Int(42), + MergeCommitSHA: github.String("sha"), + State: github.String("open"), + URL: github.String("url"), + }, + } + mockClient. + On("ListPullRequests", context.Background(), testRepoOwner, testRepoName, &github.PullRequestListOptions{ + State: "all", + Head: opts.HeadBranch, + Base: opts.BaseBranch, + Sort: "", + Direction: "", + ListOptions: github.ListOptions{ + Page: 0, + PerPage: 100, + }, + }). + Return( + []*github.PullRequest{{ + Number: mockClient.pr.Number, + Head: &github.PullRequestBranch{ + Ref: github.String("head"), + }, + MergeCommitSHA: mockClient.pr.MergeCommitSHA, + State: mockClient.pr.State, + HTMLURL: mockClient.pr.URL, + }}, + &github.Response{}, + nil, + ) + + // call the code we are testing + g := provider{ + owner: testRepoOwner, + repo: testRepoName, + client: mockClient, + } + + prs, err := g.ListPullRequests(context.Background(), &opts) + require.NoError(t, err) + + require.Equal(t, testRepoOwner, mockClient.owner) + require.Equal(t, testRepoName, mockClient.repo) + require.Equal(t, opts.HeadBranch, mockClient.listOpts.Head) + require.Equal(t, opts.BaseBranch, mockClient.listOpts.Base) + + require.Equal(t, int64(*mockClient.pr.Number), prs[0].Number) + require.Equal(t, *mockClient.pr.MergeCommitSHA, prs[0].MergeCommitSHA) + require.Equal(t, *mockClient.pr.URL, prs[0].URL) + require.True(t, prs[0].Open) +} diff --git a/internal/gitprovider/gitlab/gitlab.go b/internal/gitprovider/gitlab/gitlab.go index 20270c5a1..9a59c8cfd 100644 --- a/internal/gitprovider/gitlab/gitlab.go +++ b/internal/gitprovider/gitlab/gitlab.go @@ -119,6 +119,7 @@ func (p *provider) CreatePullRequest( glMR, _, err := p.client.CreateMergeRequest(p.projectName, &gitlab.CreateMergeRequestOptions{ Title: &opts.Title, Description: &opts.Description, + Labels: (*gitlab.LabelOptions)(&opts.Labels), SourceBranch: &opts.Head, TargetBranch: &opts.Base, RemoveSourceBranch: gitlab.Ptr(true), diff --git a/internal/gitprovider/gitprovider.go b/internal/gitprovider/gitprovider.go index c83dd8091..da5e8f466 100644 --- a/internal/gitprovider/gitprovider.go +++ b/internal/gitprovider/gitprovider.go @@ -64,6 +64,8 @@ type CreatePullRequestOpts struct { Head string // Base is the name of the target branch. Base string + // Labels is an array of strings that should be added as labels to the pull request. + Labels []string } // ListPullRequestOptions encapsulates the options used when listing pull diff --git a/ui/src/gen/directives/git-open-pr-config.json b/ui/src/gen/directives/git-open-pr-config.json index 8a29fd2b7..76bf4d8f6 100644 --- a/ui/src/gen/directives/git-open-pr-config.json +++ b/ui/src/gen/directives/git-open-pr-config.json @@ -46,6 +46,15 @@ "type": "string", "description": "The title for the pull request. Kargo generates a title based on the commit messages if it is not explicitly specified.", "minLength": 1 + }, + "labels": { + "type": "array", + "description": "Labels to add to the pull request.", + "items": { + "type": "string", + "description": "A pull request label", + "minLength": 1 + } } } } \ No newline at end of file