Skip to content

Commit aaee79b

Browse files
catchyzhengcatchyzheng
authored
feat(internal/librarian): add push-config flag and corresponding functionality (#868)
feat(internal/librarian): add `push-config` flag and its functionality. When push-config and github token both set and exist, we do the following: 1. add all changes to staging. 2. commit them. 3. create pull request and push to the repo. Fixes: #805
1 parent 10e3218 commit aaee79b

File tree

10 files changed

+839
-15
lines changed

10 files changed

+839
-15
lines changed

‎internal/github/github.go‎

Lines changed: 58 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,10 +20,12 @@ import (
2020
"context"
2121
"fmt"
2222
"io"
23+
"log/slog"
2324
"net/http"
2425
"strings"
2526

2627
"github.com/google/go-github/v69/github"
28+
"github.com/googleapis/librarian/internal/gitrepo"
2729
)
2830

2931
// PullRequest is a type alias for the go-github type.
@@ -81,9 +83,9 @@ type PullRequestMetadata struct {
8183
Number int
8284
}
8385

84-
// ParseUrl parses a GitHub URL (anything to do with a repository) to determine
86+
// ParseURL parses a GitHub URL (anything to do with a repository) to determine
8587
// the GitHub repo details (owner and name).
86-
func ParseUrl(remoteUrl string) (*Repository, error) {
88+
func ParseURL(remoteUrl string) (*Repository, error) {
8789
if !strings.HasPrefix(remoteUrl, "https://github.com/") {
8890
return nil, fmt.Errorf("remote '%s' is not a GitHub remote", remoteUrl)
8991
}
@@ -108,3 +110,57 @@ func (c *Client) GetRawContent(ctx context.Context, path, ref string) ([]byte, e
108110
defer body.Close()
109111
return io.ReadAll(body)
110112
}
113+
114+
// CreatePullRequest creates a pull request in the remote repo.
115+
// At the moment this requires a single remote to be configured,
116+
// which must have a GitHub HTTPS URL. We assume a base branch of "main".
117+
func (c *Client) CreatePullRequest(ctx context.Context, repo *Repository, remoteBranch, title, body string) (*PullRequestMetadata, error) {
118+
if body == "" {
119+
body = "Regenerated all changed APIs. See individual commits for details."
120+
}
121+
newPR := &github.NewPullRequest{
122+
Title: &title,
123+
Head: &remoteBranch,
124+
Base: github.Ptr("main"),
125+
Body: github.Ptr(body),
126+
MaintainerCanModify: github.Ptr(true),
127+
}
128+
pr, _, err := c.PullRequests.Create(ctx, repo.Owner, repo.Name, newPR)
129+
if err != nil {
130+
return nil, err
131+
}
132+
133+
slog.Info("PR created", "url", pr.GetHTMLURL())
134+
pullRequestMetadata := &PullRequestMetadata{Repo: repo, Number: pr.GetNumber()}
135+
return pullRequestMetadata, nil
136+
}
137+
138+
// FetchGitHubRepoFromRemote parses the GitHub repo name from the remote for this repository.
139+
// There must only be a single remote with a GitHub URL (as the first URL), in order to provide an
140+
// unambiguous result.
141+
// Remotes without any URLs, or where the first URL does not start with https://github.com/ are ignored.
142+
func FetchGitHubRepoFromRemote(repo *gitrepo.Repository) (*Repository, error) {
143+
remotes, err := repo.Remotes()
144+
if err != nil {
145+
return nil, err
146+
}
147+
var gitHubRemoteNames = []string{}
148+
gitHubURL := ""
149+
for _, remote := range remotes {
150+
urls := remote.Config().URLs
151+
if len(urls) > 0 && strings.HasPrefix(urls[0], "https://github.com/") {
152+
gitHubRemoteNames = append(gitHubRemoteNames, remote.Config().Name)
153+
gitHubURL = urls[0]
154+
}
155+
}
156+
157+
if len(gitHubRemoteNames) == 0 {
158+
return nil, fmt.Errorf("no GitHub remotes found")
159+
}
160+
161+
if len(gitHubRemoteNames) > 1 {
162+
n := strings.Join(gitHubRemoteNames, ", ")
163+
return nil, fmt.Errorf("can only determine the GitHub repo with a single matching remote; GitHub remotes in repo: %s", n)
164+
}
165+
return ParseURL(gitHubURL)
166+
}

‎internal/github/github_test.go‎

Lines changed: 223 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,14 +16,19 @@ package github
1616

1717
import (
1818
"context"
19+
"encoding/json"
1920
"fmt"
2021
"net/http"
2122
"net/http/httptest"
2223
"net/url"
2324
"strings"
2425
"testing"
2526

27+
"github.com/go-git/go-git/v5"
28+
gogitConfig "github.com/go-git/go-git/v5/config"
2629
"github.com/google/go-cmp/cmp"
30+
"github.com/google/go-github/v69/github"
31+
"github.com/googleapis/librarian/internal/gitrepo"
2732
)
2833

2934
func TestToken(t *testing.T) {
@@ -122,6 +127,120 @@ func TestGetRawContent(t *testing.T) {
122127
}
123128
}
124129

130+
// newTestGitRepo creates a new git repository in a temporary directory with the given remotes.
131+
func newTestGitRepo(t *testing.T, remotes map[string][]string) *gitrepo.Repository {
132+
t.Helper()
133+
dir := t.TempDir()
134+
135+
r, err := git.PlainInit(dir, false)
136+
if err != nil {
137+
t.Fatalf("git.PlainInit failed: %v", err)
138+
}
139+
140+
for name, urls := range remotes {
141+
_, err := r.CreateRemote(&gogitConfig.RemoteConfig{
142+
Name: name,
143+
URLs: urls,
144+
})
145+
if err != nil {
146+
t.Fatalf("CreateRemote failed: %v", err)
147+
}
148+
}
149+
150+
repo, err := gitrepo.NewRepository(&gitrepo.RepositoryOptions{Dir: dir})
151+
if err != nil {
152+
t.Fatalf("gitrepo.NewRepository failed: %v", err)
153+
}
154+
return repo
155+
}
156+
157+
func TestFetchGitHubRepoFromRemote(t *testing.T) {
158+
t.Parallel()
159+
for _, test := range []struct {
160+
name string
161+
remotes map[string][]string
162+
wantRepo *Repository
163+
wantErr bool
164+
wantErrSubstr string
165+
}{
166+
{
167+
name: "Single GitHub remote",
168+
remotes: map[string][]string{
169+
"origin": {"https://github.com/owner/repo.git"},
170+
},
171+
wantRepo: &Repository{Owner: "owner", Name: "repo"},
172+
},
173+
{
174+
name: "No remotes",
175+
remotes: map[string][]string{},
176+
wantErr: true,
177+
wantErrSubstr: "no GitHub remotes found",
178+
},
179+
{
180+
name: "No GitHub remotes",
181+
remotes: map[string][]string{
182+
"origin": {"https://gitlab.com/owner/repo.git"},
183+
},
184+
wantErr: true,
185+
wantErrSubstr: "no GitHub remotes found",
186+
},
187+
{
188+
name: "Multiple remotes, one is GitHub",
189+
remotes: map[string][]string{
190+
"gitlab": {"https://gitlab.com/owner/repo.git"},
191+
"upstream": {"https://github.com/gh-owner/gh-repo.git"},
192+
},
193+
wantRepo: &Repository{Owner: "gh-owner", Name: "gh-repo"},
194+
},
195+
{
196+
name: "Multiple GitHub remotes",
197+
remotes: map[string][]string{
198+
"origin": {"https://github.com/owner/repo.git"},
199+
"upstream": {"https://github.com/owner2/repo2.git"},
200+
},
201+
wantErr: true,
202+
wantErrSubstr: "can only determine the GitHub repo with a single matching remote",
203+
},
204+
{
205+
name: "Remote with multiple URLs, first is not GitHub",
206+
remotes: map[string][]string{
207+
"origin": {"https://gitlab.com/owner/repo.git", "https://github.com/owner/repo.git"},
208+
},
209+
wantErr: true,
210+
wantErrSubstr: "no GitHub remotes found",
211+
},
212+
{
213+
name: "Remote with multiple URLs, first is GitHub",
214+
remotes: map[string][]string{
215+
"origin": {"https://github.com/owner/repo.git", "https://gitlab.com/owner/repo.git"},
216+
},
217+
wantRepo: &Repository{Owner: "owner", Name: "repo"},
218+
},
219+
} {
220+
t.Run(test.name, func(t *testing.T) {
221+
t.Parallel()
222+
repo := newTestGitRepo(t, test.remotes)
223+
224+
got, err := FetchGitHubRepoFromRemote(repo)
225+
226+
if test.wantErr {
227+
if err == nil {
228+
t.Errorf("FetchGitHubRepoFromRemote() err = nil, want error containing %q", test.wantErrSubstr)
229+
} else if !strings.Contains(err.Error(), test.wantErrSubstr) {
230+
t.Errorf("FetchGitHubRepoFromRemote() err = %v, want error containing %q", err, test.wantErrSubstr)
231+
}
232+
} else {
233+
if err != nil {
234+
t.Errorf("FetchGitHubRepoFromRemote() err = %v, want nil", err)
235+
}
236+
if diff := cmp.Diff(test.wantRepo, got); diff != "" {
237+
t.Errorf("FetchGitHubRepoFromRemote() repo mismatch (-want +got): %s", diff)
238+
}
239+
}
240+
})
241+
}
242+
}
243+
125244
func TestParseUrl(t *testing.T) {
126245
t.Parallel()
127246
for _, test := range []struct {
@@ -158,7 +277,7 @@ func TestParseUrl(t *testing.T) {
158277
} {
159278
t.Run(test.name, func(t *testing.T) {
160279
t.Parallel()
161-
repo, err := ParseUrl(test.remoteUrl)
280+
repo, err := ParseURL(test.remoteUrl)
162281

163282
if test.wantErr {
164283
if err == nil {
@@ -177,3 +296,106 @@ func TestParseUrl(t *testing.T) {
177296
})
178297
}
179298
}
299+
300+
func TestCreatePullRequest(t *testing.T) {
301+
t.Parallel()
302+
for _, test := range []struct {
303+
name string
304+
remoteBranch string
305+
title string
306+
body string
307+
handler http.HandlerFunc
308+
wantMetadata *PullRequestMetadata
309+
wantErr bool
310+
wantErrSubstr string
311+
}{
312+
{
313+
name: "Success with provided body",
314+
remoteBranch: "feature-branch",
315+
title: "New Feature",
316+
body: "This is a new feature.",
317+
handler: func(w http.ResponseWriter, r *http.Request) {
318+
if r.Method != http.MethodPost {
319+
t.Errorf("unexpected method: got %s, want %s", r.Method, http.MethodPost)
320+
}
321+
if r.URL.Path != "/repos/owner/repo/pulls" {
322+
t.Errorf("unexpected path: got %s, want %s", r.URL.Path, "/repos/owner/repo/pulls")
323+
}
324+
var newPR github.NewPullRequest
325+
if err := json.NewDecoder(r.Body).Decode(&newPR); err != nil {
326+
t.Fatalf("failed to decode request body: %v", err)
327+
}
328+
if *newPR.Title != "New Feature" {
329+
t.Errorf("unexpected title: got %q, want %q", *newPR.Title, "New Feature")
330+
}
331+
if *newPR.Body != "This is a new feature." {
332+
t.Errorf("unexpected body: got %q, want %q", *newPR.Body, "This is a new feature.")
333+
}
334+
if *newPR.Head != "feature-branch" {
335+
t.Errorf("unexpected head: got %q, want %q", *newPR.Head, "feature-branch")
336+
}
337+
if *newPR.Base != "main" {
338+
t.Errorf("unexpected base: got %q, want %q", *newPR.Base, "main")
339+
}
340+
fmt.Fprint(w, `{"number": 1, "html_url": "https://github.com/owner/repo/pull/1"}`)
341+
},
342+
wantMetadata: &PullRequestMetadata{Repo: &Repository{Owner: "owner", Name: "repo"}, Number: 1},
343+
},
344+
{
345+
name: "Success with empty body",
346+
remoteBranch: "another-branch",
347+
title: "Another PR",
348+
body: "",
349+
handler: func(w http.ResponseWriter, r *http.Request) {
350+
var newPR github.NewPullRequest
351+
if err := json.NewDecoder(r.Body).Decode(&newPR); err != nil {
352+
t.Fatalf("failed to decode request body: %v", err)
353+
}
354+
expectedBody := "Regenerated all changed APIs. See individual commits for details."
355+
if *newPR.Body != expectedBody {
356+
t.Errorf("unexpected body: got %q, want %q", *newPR.Body, expectedBody)
357+
}
358+
fmt.Fprint(w, `{"number": 1, "html_url": "https://github.com/owner/repo/pull/1"}`)
359+
},
360+
wantMetadata: &PullRequestMetadata{Repo: &Repository{Owner: "owner", Name: "repo"}, Number: 1},
361+
},
362+
{
363+
name: "GitHub API error",
364+
remoteBranch: "error-branch",
365+
title: "Error PR",
366+
body: "This will fail.",
367+
handler: func(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusInternalServerError) },
368+
wantErr: true,
369+
wantErrSubstr: "500",
370+
},
371+
} {
372+
t.Run(test.name, func(t *testing.T) {
373+
t.Parallel()
374+
server := httptest.NewServer(test.handler)
375+
defer server.Close()
376+
377+
repo := &Repository{Owner: "owner", Name: "repo"}
378+
client, err := newClientWithHTTP("fake-token", repo, server.Client())
379+
if err != nil {
380+
t.Fatalf("newClientWithHTTP() error = %v", err)
381+
}
382+
client.BaseURL, _ = url.Parse(server.URL + "/")
383+
384+
metadata, err := client.CreatePullRequest(context.Background(), repo, test.remoteBranch, test.title, test.body)
385+
386+
if test.wantErr {
387+
if err == nil {
388+
t.Errorf("CreatePullRequest() err = nil, want error containing %q", test.wantErrSubstr)
389+
} else if !strings.Contains(err.Error(), test.wantErrSubstr) {
390+
t.Errorf("CreatePullRequest() err = %v, want error containing %q", err, test.wantErrSubstr)
391+
}
392+
} else if err != nil {
393+
t.Errorf("CreatePullRequest() err = %v, want nil", err)
394+
}
395+
396+
if diff := cmp.Diff(test.wantMetadata, metadata); diff != "" {
397+
t.Errorf("CreatePullRequest() metadata mismatch (-want +got):\n%s", diff)
398+
}
399+
})
400+
}
401+
}

0 commit comments

Comments
 (0)