Skip to content

Commit bded5dc

Browse files
catchyzhengzhumin8
andauthored
feat: publish-release automation only triggers CloudBuild if a repository contains a merged pull request with label release:pending (#1789)
fix: #1786 --------- Co-authored-by: Min Zhu <[email protected]>
1 parent 73c1bc7 commit bded5dc

File tree

4 files changed

+187
-3
lines changed

4 files changed

+187
-3
lines changed

internal/automation/trigger.go

Lines changed: 25 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,10 +20,12 @@ import (
2020
"fmt"
2121
"iter"
2222
"log/slog"
23+
"os"
2324

2425
cloudbuild "cloud.google.com/go/cloudbuild/apiv1/v2"
2526
"cloud.google.com/go/cloudbuild/apiv1/v2/cloudbuildpb"
2627
"github.com/googleapis/gax-go/v2"
28+
"github.com/googleapis/librarian/internal/github"
2729
)
2830

2931
var triggerNameByCommandName = map[string]string{
@@ -34,6 +36,11 @@ var triggerNameByCommandName = map[string]string{
3436

3537
const region = "global"
3638

39+
// GitHubClient handles communication with the GitHub API.
40+
type GitHubClient interface {
41+
FindMergedPullRequestsWithPendingReleaseLabel(ctx context.Context, owner, repo string) ([]*github.PullRequest, error)
42+
}
43+
3744
type wrappedCloudBuildClient struct {
3845
client *cloudbuild.Client
3946
}
@@ -64,10 +71,14 @@ func RunCommand(ctx context.Context, command string, projectId string, push bool
6471
wrappedClient := &wrappedCloudBuildClient{
6572
client: c,
6673
}
67-
return runCommandWithClient(ctx, wrappedClient, command, projectId, push)
74+
ghClient, err := github.NewClient(os.Getenv("LIBRARIAN_GITHUB_TOKEN"), &github.Repository{})
75+
if err != nil {
76+
return fmt.Errorf("error creating github client: %w", err)
77+
}
78+
return runCommandWithClient(ctx, wrappedClient, ghClient, command, projectId, push)
6879
}
6980

70-
func runCommandWithClient(ctx context.Context, client CloudBuildClient, command string, projectId string, push bool) error {
81+
func runCommandWithClient(ctx context.Context, client CloudBuildClient, ghClient GitHubClient, command string, projectId string, push bool) error {
7182
// validate command is allowed
7283
triggerName := triggerNameByCommandName[command]
7384
if triggerName == "" {
@@ -91,6 +102,18 @@ func runCommandWithClient(ctx context.Context, client CloudBuildClient, command
91102
"_GITHUB_TOKEN_SECRET_NAME": repository.SecretName,
92103
"_PUSH": fmt.Sprintf("%v", push),
93104
}
105+
if command == "publish-release" {
106+
prs, err := ghClient.FindMergedPullRequestsWithPendingReleaseLabel(ctx, "googleapis", repository.Name)
107+
if err != nil {
108+
slog.Error("Error finding merged pull requests for publish-release", slog.Any("err", err), slog.String("repository", repository.Name))
109+
errs = append(errs, err)
110+
continue
111+
}
112+
if len(prs) == 0 {
113+
slog.Info("No pull requests with label 'release:pending' found. Skipping 'publish-release' trigger.", slog.String("repository", repository.Name))
114+
continue
115+
}
116+
}
94117
err = runCloudBuildTriggerByName(ctx, client, projectId, region, triggerName, substitutions)
95118
if err != nil {
96119
slog.Error("Error triggering cloudbuild", slog.Any("err", err))

internal/automation/trigger_test.go

Lines changed: 56 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,18 @@ import (
2020
"testing"
2121

2222
"cloud.google.com/go/cloudbuild/apiv1/v2/cloudbuildpb"
23+
"github.com/google/go-github/v69/github"
2324
)
2425

26+
type mockGitHubClient struct {
27+
prs []*github.PullRequest
28+
err error
29+
}
30+
31+
func (m *mockGitHubClient) FindMergedPullRequestsWithPendingReleaseLabel(ctx context.Context, owner, repo string) ([]*github.PullRequest, error) {
32+
return m.prs, m.err
33+
}
34+
2535
func TestRunCommandWithClient(t *testing.T) {
2636
for _, test := range []struct {
2737
name string
@@ -31,6 +41,8 @@ func TestRunCommandWithClient(t *testing.T) {
3141
runError error
3242
wantErr bool
3343
buildTriggers []*cloudbuildpb.BuildTrigger
44+
ghPRs []*github.PullRequest
45+
ghError error
3446
}{
3547
{
3648
name: "runs generate trigger",
@@ -97,14 +109,57 @@ func TestRunCommandWithClient(t *testing.T) {
97109
},
98110
},
99111
},
112+
{
113+
name: "runs publish-release trigger",
114+
command: "publish-release",
115+
push: true,
116+
wantErr: false,
117+
buildTriggers: []*cloudbuildpb.BuildTrigger{
118+
{
119+
Name: "publish-release",
120+
Id: "publish-release-trigger-id",
121+
},
122+
},
123+
ghPRs: []*github.PullRequest{{}},
124+
},
125+
{
126+
name: "skips publish-release with no PRs",
127+
command: "publish-release",
128+
push: true,
129+
wantErr: false,
130+
buildTriggers: []*cloudbuildpb.BuildTrigger{
131+
{
132+
Name: "publish-release",
133+
Id: "publish-release-trigger-id",
134+
},
135+
},
136+
ghPRs: []*github.PullRequest{},
137+
},
138+
{
139+
name: "error finding PRs for publish-release",
140+
command: "publish-release",
141+
push: true,
142+
wantErr: true,
143+
buildTriggers: []*cloudbuildpb.BuildTrigger{
144+
{
145+
Name: "publish-release",
146+
Id: "publish-release-trigger-id",
147+
},
148+
},
149+
ghError: fmt.Errorf("github error"),
150+
},
100151
} {
101152
t.Run(test.name, func(t *testing.T) {
102153
ctx := context.Background()
103154
client := &mockCloudBuildClient{
104155
runError: test.runError,
105156
buildTriggers: test.buildTriggers,
106157
}
107-
err := runCommandWithClient(ctx, client, test.command, "some-project", test.push)
158+
ghClient := &mockGitHubClient{
159+
prs: test.ghPRs,
160+
err: test.ghError,
161+
}
162+
err := runCommandWithClient(ctx, client, ghClient, test.command, "some-project", test.push)
108163
if test.wantErr && err == nil {
109164
t.Errorf("expected error, but did not return one")
110165
} else if !test.wantErr && err != nil {

internal/github/github.go

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -244,3 +244,40 @@ func (c *Client) CreateIssueComment(ctx context.Context, number int, comment str
244244
})
245245
return err
246246
}
247+
248+
// hasLabel checks if a pull request has a given label.
249+
func hasLabel(pr *PullRequest, labelName string) bool {
250+
for _, l := range pr.Labels {
251+
if l.GetName() == labelName {
252+
return true
253+
}
254+
}
255+
return false
256+
}
257+
258+
// FindMergedPullRequestsWithPendingReleaseLabel finds all merged pull requests with the "release:pending" label.
259+
func (c *Client) FindMergedPullRequestsWithPendingReleaseLabel(ctx context.Context, owner, repo string) ([]*PullRequest, error) {
260+
var allPRs []*PullRequest
261+
opt := &github.PullRequestListOptions{
262+
State: "closed",
263+
ListOptions: github.ListOptions{
264+
PerPage: 100,
265+
},
266+
}
267+
for {
268+
prs, resp, err := c.PullRequests.List(ctx, owner, repo, opt)
269+
if err != nil {
270+
return nil, err
271+
}
272+
for _, pr := range prs {
273+
if pr.GetMerged() && hasLabel(pr, "release:pending") {
274+
allPRs = append(allPRs, pr)
275+
}
276+
}
277+
if resp.NextPage == 0 {
278+
break
279+
}
280+
opt.Page = resp.NextPage
281+
}
282+
return allPRs, nil
283+
}

internal/github/github_test.go

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -934,3 +934,72 @@ func TestCreateIssueComment(t *testing.T) {
934934
})
935935
}
936936
}
937+
938+
func TestFindMergedPullRequestsWithPendingReleaseLabel(t *testing.T) {
939+
t.Parallel()
940+
for _, test := range []struct {
941+
name string
942+
handler http.HandlerFunc
943+
wantPRs []*PullRequest
944+
wantErr bool
945+
wantErrSubstr string
946+
}{
947+
{
948+
name: "Success with single page",
949+
handler: func(w http.ResponseWriter, r *http.Request) {
950+
if r.URL.Query().Get("state") != "closed" {
951+
t.Errorf("unexpected state: got %q", r.URL.Query().Get("state"))
952+
}
953+
pr1 := github.PullRequest{Number: github.Ptr(1), Labels: []*github.Label{{Name: github.Ptr("release:pending")}}}
954+
pr2 := github.PullRequest{Number: github.Ptr(2), Labels: []*github.Label{{Name: github.Ptr("other-label")}}}
955+
pr3 := github.PullRequest{Number: github.Ptr(4), Merged: github.Ptr(true), Labels: []*github.Label{{Name: github.Ptr("release:pending")}}}
956+
prs := []*github.PullRequest{&pr1, &pr2, &pr3}
957+
b, err := json.Marshal(prs)
958+
if err != nil {
959+
t.Fatalf("json.Marshal() failed: %v", err)
960+
}
961+
fmt.Fprint(w, string(b))
962+
},
963+
wantPRs: []*PullRequest{
964+
{Number: github.Ptr(4), Merged: github.Ptr(true), Labels: []*github.Label{{Name: github.Ptr("release:pending")}}},
965+
},
966+
},
967+
{
968+
name: "API error",
969+
handler: func(w http.ResponseWriter, r *http.Request) {
970+
w.WriteHeader(http.StatusInternalServerError)
971+
},
972+
wantErr: true,
973+
wantErrSubstr: "500",
974+
},
975+
} {
976+
t.Run(test.name, func(t *testing.T) {
977+
t.Parallel()
978+
server := httptest.NewServer(test.handler)
979+
defer server.Close()
980+
981+
repo := &Repository{Owner: "owner", Name: "repo"}
982+
client, err := newClientWithHTTP("fake-token", repo, server.Client())
983+
if err != nil {
984+
t.Fatalf("newClientWithHTTP() error = %v", err)
985+
}
986+
client.BaseURL, _ = url.Parse(server.URL + "/")
987+
988+
prs, err := client.FindMergedPullRequestsWithPendingReleaseLabel(context.Background(), "owner", "repo")
989+
990+
if test.wantErr {
991+
if err == nil {
992+
t.Errorf("FindMergedPullRequestsWithPendingReleaseLabel() err = nil, want error containing %q", test.wantErrSubstr)
993+
} else if !strings.Contains(err.Error(), test.wantErrSubstr) {
994+
t.Errorf("FindMergedPullRequestsWithPendingReleaseLabel() err = %v, want error containing %q", err, test.wantErrSubstr)
995+
}
996+
} else if err != nil {
997+
t.Errorf("FindMergedPullRequestsWithPendingReleaseLabel() err = %v, want nil", err)
998+
}
999+
1000+
if diff := cmp.Diff(test.wantPRs, prs); diff != "" {
1001+
t.Errorf("FindMergedPullRequestsWithPendingReleaseLabel() prs mismatch (-want +got):\n%s", diff)
1002+
}
1003+
})
1004+
}
1005+
}

0 commit comments

Comments
 (0)