From df074046ace21388cc2ceab53f8d2ef98fe186ed Mon Sep 17 00:00:00 2001 From: nate smith Date: Thu, 10 Oct 2019 15:39:39 -0500 Subject: [PATCH 01/22] add context.go and move over getToken --- api/client.go | 22 ++-------------------- context/context.go | 24 ++++++++++++++++++++++++ 2 files changed, 26 insertions(+), 20 deletions(-) create mode 100644 context/context.go diff --git a/api/client.go b/api/client.go index fc6895ca1..8e431be1a 100644 --- a/api/client.go +++ b/api/client.go @@ -7,9 +7,8 @@ import ( "io/ioutil" "net/http" "os" - "os/user" - "regexp" + "github.com/github/gh-cli/context" "github.com/github/gh-cli/version" ) @@ -57,7 +56,7 @@ func graphQL(query string, variables map[string]string, v interface{}) error { return err } - token, err := getToken() + token, err := context.GetToken() if err != nil { return err } @@ -136,20 +135,3 @@ func debugResponse(resp *http.Response, body string) { fmt.Printf("DEBUG: GraphQL response:\n%+v\n\n%s\n\n", resp, body) } - -// TODO: Everything below this line will be removed when Nate's context work is complete -func getToken() (string, error) { - usr, err := user.Current() - if err != nil { - return "", err - } - - content, err := ioutil.ReadFile(usr.HomeDir + "/.config/hub") - if err != nil { - return "", err - } - - r := regexp.MustCompile(`oauth_token: (\w+)`) - token := r.FindStringSubmatch(string(content)) - return token[1], nil -} diff --git a/context/context.go b/context/context.go new file mode 100644 index 000000000..ab830deab --- /dev/null +++ b/context/context.go @@ -0,0 +1,24 @@ +package context + +import ( + "io/ioutil" + "os/user" + "regexp" +) + +// GetToken returns the authentication token as stored in the config file for the user running gh-cli +func GetToken() (string, error) { + usr, err := user.Current() + if err != nil { + return "", err + } + + content, err := ioutil.ReadFile(usr.HomeDir + "/.config/hub") + if err != nil { + return "", err + } + + r := regexp.MustCompile(`oauth_token: (\w+)`) + token := r.FindStringSubmatch(string(content)) + return token[1], nil +} From 6e6b18c50a3a1c8f806bcecc80a89833a8697705 Mon Sep 17 00:00:00 2001 From: nate smith Date: Thu, 10 Oct 2019 15:43:25 -0500 Subject: [PATCH 02/22] move more stuff into context.go --- api/queries.go | 35 ++++++++++++----------------------- context/context.go | 21 +++++++++++++++++++++ 2 files changed, 33 insertions(+), 23 deletions(-) diff --git a/api/queries.go b/api/queries.go index aa26c0be0..72b9056e0 100644 --- a/api/queries.go +++ b/api/queries.go @@ -2,9 +2,8 @@ package api import ( "fmt" - "strings" - "github.com/github/gh-cli/git" + "github.com/github/gh-cli/context" "github.com/github/gh-cli/github" ) @@ -84,10 +83,17 @@ func PullRequests() (PullRequestsPayload, error) { project := project() owner := project.Owner repo := project.Name - currentBranch := currentBranch() + currentBranch, cberr := context.CurrentBranch() + if cberr != nil { + return PullRequestsPayload{}, cberr + } + currentUsername, err := context.CurrentUsername() + if err != nil { + return PullRequestsPayload{}, err + } - viewerQuery := fmt.Sprintf("repo:%s/%s state:open is:pr author:%s", owner, repo, currentUsername()) - reviewerQuery := fmt.Sprintf("repo:%s/%s state:open review-requested:%s", owner, repo, currentUsername()) + viewerQuery := fmt.Sprintf("repo:%s/%s state:open is:pr author:%s", owner, repo, currentUsername) + reviewerQuery := fmt.Sprintf("repo:%s/%s state:open review-requested:%s", owner, repo, currentUsername) variables := map[string]string{ "viewerQuery": viewerQuery, @@ -98,7 +104,7 @@ func PullRequests() (PullRequestsPayload, error) { } var resp response - err := graphQL(query, variables, &resp) + err = graphQL(query, variables, &resp) if err != nil { return PullRequestsPayload{}, err } @@ -142,20 +148,3 @@ func project() github.Project { panic("Could not get the project. What is a project? I don't know, it's kind of like a git repository I think?") } - -func currentBranch() string { - currentBranch, err := git.Head() - if err != nil { - panic(err) - } - - return strings.Replace(currentBranch, "refs/heads/", "", 1) -} - -func currentUsername() string { - host, err := github.CurrentConfig().DefaultHost() - if err != nil { - panic(err) - } - return host.User -} diff --git a/context/context.go b/context/context.go index ab830deab..85af0d8f9 100644 --- a/context/context.go +++ b/context/context.go @@ -4,6 +4,10 @@ import ( "io/ioutil" "os/user" "regexp" + "strings" + + "github.com/github/gh-cli/git" + "github.com/github/gh-cli/github" ) // GetToken returns the authentication token as stored in the config file for the user running gh-cli @@ -22,3 +26,20 @@ func GetToken() (string, error) { token := r.FindStringSubmatch(string(content)) return token[1], nil } + +func CurrentUsername() (string, error) { + host, err := github.CurrentConfig().DefaultHost() + if err != nil { + return "", nil + } + return host.User, nil +} + +func CurrentBranch() (string, error) { + currentBranch, err := git.Head() + if err != nil { + return "", err + } + + return strings.Replace(currentBranch, "refs/heads/", "", 1), nil +} From 09d58b923db0bc1255fbbaedf61e1f61ca63b34a Mon Sep 17 00:00:00 2001 From: nate smith Date: Thu, 10 Oct 2019 16:02:23 -0500 Subject: [PATCH 03/22] add GitHubRepository --- api/queries.go | 27 ++++--------- context/ghrepo.go | 100 ++++++++++++++++++++++++++++++++++++++++++++++ github/hosts.go | 2 +- github/project.go | 2 +- github/remote.go | 16 ++++++++ 5 files changed, 125 insertions(+), 22 deletions(-) create mode 100644 context/ghrepo.go diff --git a/api/queries.go b/api/queries.go index 72b9056e0..51fe5d700 100644 --- a/api/queries.go +++ b/api/queries.go @@ -4,7 +4,6 @@ import ( "fmt" "github.com/github/gh-cli/context" - "github.com/github/gh-cli/github" ) type PullRequestsPayload struct { @@ -80,9 +79,13 @@ func PullRequests() (PullRequestsPayload, error) { } ` - project := project() - owner := project.Owner - repo := project.Name + ghRepo, rerr := context.CurrentGitHubRepository() + if rerr != nil { + return PullRequestsPayload{}, nil + } + + owner := ghRepo.Owner + repo := ghRepo.Name currentBranch, cberr := context.CurrentBranch() if cberr != nil { return PullRequestsPayload{}, cberr @@ -132,19 +135,3 @@ func PullRequests() (PullRequestsPayload, error) { return payload, nil } - -// TODO: Everything below this line will be removed when Nate's context work is complete -func project() github.Project { - remotes, error := github.Remotes() - if error != nil { - panic(error) - } - - for _, remote := range remotes { - if project, error := remote.Project(); error == nil { - return *project - } - } - - panic("Could not get the project. What is a project? I don't know, it's kind of like a git repository I think?") -} diff --git a/context/ghrepo.go b/context/ghrepo.go new file mode 100644 index 000000000..a64425761 --- /dev/null +++ b/context/ghrepo.go @@ -0,0 +1,100 @@ +package context + +import ( + "fmt" + "net/url" + "os" + "strings" + + "github.com/github/gh-cli/github" +) + +type GitHubRepository struct { + Name string + Owner string + Host string + Protocol string +} + +func CurrentGitHubRepository() (*GitHubRepository, error) { + + var repoURL *url.URL + var err error + if repoFromEnv := os.Getenv("GH_REPO"); repoFromEnv != "" { + repoURL, err = url.Parse(fmt.Sprintf("https://github.com/%s.git", repoFromEnv)) + if err != nil { + return nil, err + } + } else { + remote, rerr := github.GuessRemote() + + if rerr != nil { + return nil, rerr + } + repoURL = remote.URL + } + + urlError := fmt.Errorf("invalid GitHub URL: %s", repoURL) + if !github.KnownGitHubHostsInclude(repoURL.Host) { + return nil, urlError + } + + parts := strings.SplitN(repoURL.Path, "/", 4) + if len(parts) <= 2 { + return nil, urlError + } + + name := strings.TrimSuffix(parts[2], ".git") + owner := parts[1] + host := repoURL.Host + protocol := repoURL.Scheme + + if strings.Contains(owner, "/") { + result := strings.SplitN(owner, "/", 2) + owner = result[0] + if name == "" { + name = result[1] + } + } else if strings.Contains(name, "/") { + result := strings.SplitN(name, "/", 2) + if owner == "" { + owner = result[0] + } + name = result[1] + } + + if host == "" { + host = github.DefaultGitHubHost() + } + if host == "ssh.github.com" { + host = github.GitHubHost + } + + if protocol != "http" && protocol != "https" { + protocol = "" + } + if protocol == "" { + h := github.CurrentConfig().Find(host) + if h != nil { + protocol = h.Protocol + } + } + if protocol == "" { + protocol = "https" + } + + if owner == "" { + h := github.CurrentConfig().Find(host) + if h != nil { + owner = h.User + } + } + + return &GitHubRepository{ + Name: name, + Owner: owner, + Host: host, + Protocol: protocol, + }, nil + +} diff --git a/github/hosts.go b/github/hosts.go index 5cf889c53..a74d9810c 100644 --- a/github/hosts.go +++ b/github/hosts.go @@ -22,7 +22,7 @@ func (e *GithubHostError) Error() string { return fmt.Sprintf("Invalid GitHub URL: %s", e.url) } -func knownGitHubHostsInclude(host string) bool { +func KnownGitHubHostsInclude(host string) bool { for _, hh := range knownGitHubHosts() { if hh == host { return true diff --git a/github/project.go b/github/project.go index 142532f2d..6e9fa3c41 100644 --- a/github/project.go +++ b/github/project.go @@ -47,7 +47,7 @@ func (p *Project) GitURL(name, owner string, isSSH bool) string { } func NewProjectFromURL(url *url.URL) (p *Project, err error) { - if !knownGitHubHostsInclude(url.Host) { + if !KnownGitHubHostsInclude(url.Host) { err = &GithubHostError{url} return } diff --git a/github/remote.go b/github/remote.go index 523b54d7f..80e4eaa55 100644 --- a/github/remote.go +++ b/github/remote.go @@ -99,3 +99,19 @@ func newRemote(name string, urlMap map[string]string) (Remote, error) { return r, nil } + +// GuessRemote attempts to select and return the remote a user likely wants to target when dealing with GitHub repositories. +func GuessRemote() (Remote, error) { + + remotes, err := Remotes() + if err != nil { + return Remote{}, err + } + + if len(remotes) == 0 { + return Remote{}, fmt.Errorf("unable to guess remote") + } + + // lol + return remotes[0], nil +} From 81218a0c7d3436c7f05e3f6cf60224d8be7463a3 Mon Sep 17 00:00:00 2001 From: nate smith Date: Thu, 10 Oct 2019 16:06:40 -0500 Subject: [PATCH 04/22] remove some dead code --- github/client.go | 39 --------------------------------------- github/remote.go | 8 -------- 2 files changed, 47 deletions(-) diff --git a/github/client.go b/github/client.go index e1855e15c..bd1272e90 100644 --- a/github/client.go +++ b/github/client.go @@ -35,45 +35,6 @@ type Client struct { cachedClient *simpleClient } -func (client *Client) FetchPullRequests(project *Project, filterParams map[string]interface{}, limit int, filter func(*PullRequest) bool) (pulls []PullRequest, err error) { - api, err := client.simpleApi() - if err != nil { - return - } - - path := fmt.Sprintf("repos/%s/%s/pulls?per_page=%d", project.Owner, project.Name, perPage(limit, 100)) - if filterParams != nil { - path = addQuery(path, filterParams) - } - - pulls = []PullRequest{} - var res *simpleResponse - - for path != "" { - res, err = api.GetFile(path, draftsType) - if err = checkStatus(200, "fetching pull requests", res, err); err != nil { - return - } - path = res.Link("next") - - pullsPage := []PullRequest{} - if err = res.Unmarshal(&pullsPage); err != nil { - return - } - for _, pr := range pullsPage { - if filter == nil || filter(&pr) { - pulls = append(pulls, pr) - if limit > 0 && len(pulls) == limit { - path = "" - break - } - } - } - } - - return -} - func (client *Client) PullRequest(project *Project, id string) (pr *PullRequest, err error) { api, err := client.simpleApi() if err != nil { diff --git a/github/remote.go b/github/remote.go index 80e4eaa55..e51266d93 100644 --- a/github/remote.go +++ b/github/remote.go @@ -23,14 +23,6 @@ func (remote *Remote) String() string { return remote.Name } -func (remote *Remote) Project() (*Project, error) { - p, err := NewProjectFromURL(remote.URL) - if _, ok := err.(*GithubHostError); ok { - return NewProjectFromURL(remote.PushURL) - } - return p, err -} - func Remotes() (remotes []Remote, err error) { re := regexp.MustCompile(`(.+)\s+(.+)\s+\((push|fetch)\)`) From 1737f1685bc47e2bf36532f2a74379686d0e8f66 Mon Sep 17 00:00:00 2001 From: nate smith Date: Thu, 10 Oct 2019 17:06:06 -0500 Subject: [PATCH 05/22] remove more dead code --- github/client.go | 256 ----------------------------------------------- 1 file changed, 256 deletions(-) diff --git a/github/client.go b/github/client.go index bd1272e90..6b90da1a6 100644 --- a/github/client.go +++ b/github/client.go @@ -1,16 +1,12 @@ package github import ( - "bytes" - "encoding/json" "fmt" - "io" "net/http" "net/url" "os" "os/exec" "strings" - "time" "github.com/github/gh-cli/version" ) @@ -35,226 +31,10 @@ type Client struct { cachedClient *simpleClient } -func (client *Client) PullRequest(project *Project, id string) (pr *PullRequest, err error) { - api, err := client.simpleApi() - if err != nil { - return - } - - res, err := api.Get(fmt.Sprintf("repos/%s/%s/pulls/%s", project.Owner, project.Name, id)) - if err = checkStatus(200, "getting pull request", res, err); err != nil { - return - } - - pr = &PullRequest{} - err = res.Unmarshal(pr) - - return -} - -func (client *Client) CreatePullRequest(project *Project, params map[string]interface{}) (pr *PullRequest, err error) { - api, err := client.simpleApi() - if err != nil { - return - } - - res, err := api.PostJSONPreview(fmt.Sprintf("repos/%s/%s/pulls", project.Owner, project.Name), params, draftsType) - if err = checkStatus(201, "creating pull request", res, err); err != nil { - if res != nil && res.StatusCode == 404 { - projectUrl := strings.SplitN(project.WebURL("", "", ""), "://", 2)[1] - err = fmt.Errorf("%s\nAre you sure that %s exists?", err, projectUrl) - } - return - } - - pr = &PullRequest{} - err = res.Unmarshal(pr) - - return -} - -func (client *Client) UpdatePullRequest(pr *PullRequest, params map[string]interface{}) (updatedPullRequest *PullRequest, err error) { - api, err := client.simpleApi() - if err != nil { - return - } - - res, err := api.PatchJSON(pr.ApiUrl, params) - if err = checkStatus(200, "updating pull request", res, err); err != nil { - return - } - - updatedPullRequest = &PullRequest{} - err = res.Unmarshal(updatedPullRequest) - return -} - -func (client *Client) RequestReview(project *Project, prNumber int, params map[string]interface{}) (err error) { - api, err := client.simpleApi() - if err != nil { - return - } - - res, err := api.PostJSON(fmt.Sprintf("repos/%s/%s/pulls/%d/requested_reviewers", project.Owner, project.Name, prNumber), params) - if err = checkStatus(201, "requesting reviewer", res, err); err != nil { - return - } - - res.Body.Close() - return -} - -func (client *Client) Repository(project *Project) (repo *Repository, err error) { - api, err := client.simpleApi() - if err != nil { - return - } - - res, err := api.Get(fmt.Sprintf("repos/%s/%s", project.Owner, project.Name)) - if err = checkStatus(200, "getting repository info", res, err); err != nil { - return - } - - repo = &Repository{} - err = res.Unmarshal(&repo) - return -} - -type Repository struct { - Name string `json:"name"` - FullName string `json:"full_name"` - Parent *Repository `json:"parent"` - Owner *User `json:"owner"` - Private bool `json:"private"` - HasWiki bool `json:"has_wiki"` - Permissions *RepositoryPermissions `json:"permissions"` - HtmlUrl string `json:"html_url"` - DefaultBranch string `json:"default_branch"` -} - -type RepositoryPermissions struct { - Admin bool `json:"admin"` - Push bool `json:"push"` - Pull bool `json:"pull"` -} - -type Issue struct { - Number int `json:"number"` - State string `json:"state"` - Title string `json:"title"` - Body string `json:"body"` - User *User `json:"user"` - - PullRequest *PullRequest `json:"pull_request"` - Head *PullRequestSpec `json:"head"` - Base *PullRequestSpec `json:"base"` - - MergeCommitSha string `json:"merge_commit_sha"` - MaintainerCanModify bool `json:"maintainer_can_modify"` - Draft bool `json:"draft"` - - Comments int `json:"comments"` - Labels []IssueLabel `json:"labels"` - Assignees []User `json:"assignees"` - Milestone *Milestone `json:"milestone"` - CreatedAt time.Time `json:"created_at"` - UpdatedAt time.Time `json:"updated_at"` - MergedAt time.Time `json:"merged_at"` - - RequestedReviewers []User `json:"requested_reviewers"` - RequestedTeams []Team `json:"requested_teams"` - - ApiUrl string `json:"url"` - HtmlUrl string `json:"html_url"` - - ClosedBy *User `json:"closed_by"` -} - -type PullRequest Issue - -type PullRequestSpec struct { - Label string `json:"label"` - Ref string `json:"ref"` - Sha string `json:"sha"` - Repo *Repository `json:"repo"` -} - -func (pr *PullRequest) IsSameRepo() bool { - return pr.Head != nil && pr.Head.Repo != nil && - pr.Head.Repo.Name == pr.Base.Repo.Name && - pr.Head.Repo.Owner.Login == pr.Base.Repo.Owner.Login -} - -func (pr *PullRequest) HasRequestedReviewer(name string) bool { - for _, user := range pr.RequestedReviewers { - if strings.EqualFold(user.Login, name) { - return true - } - } - return false -} - -func (pr *PullRequest) HasRequestedTeam(name string) bool { - for _, team := range pr.RequestedTeams { - if strings.EqualFold(team.Slug, name) { - return true - } - } - return false -} - -type IssueLabel struct { - Name string `json:"name"` - Color string `json:"color"` -} - type User struct { Login string `json:"login"` } -type Team struct { - Name string `json:"name"` - Slug string `json:"slug"` -} - -type Milestone struct { - Number int `json:"number"` - Title string `json:"title"` -} - -func (client *Client) GenericAPIRequest(method, path string, data interface{}, headers map[string]string, ttl int) (*simpleResponse, error) { - api, err := client.simpleApi() - if err != nil { - return nil, err - } - api.CacheTTL = ttl - - var body io.Reader - switch d := data.(type) { - case map[string]interface{}: - if method == "GET" { - path = addQuery(path, d) - } else if len(d) > 0 { - json, err := json.Marshal(d) - if err != nil { - return nil, err - } - body = bytes.NewBuffer(json) - } - case io.Reader: - body = d - } - - return api.performRequest(method, path, body, func(req *http.Request) { - if body != nil { - req.Header.Set("Content-Type", "application/json; charset=utf-8") - } - for key, value := range headers { - req.Header.Set(key, value) - } - }) -} - func (client *Client) CurrentUser() (user *User, err error) { api, err := client.simpleApi() if err != nil { @@ -508,39 +288,3 @@ func authTokenNote(num int) (string, error) { return fmt.Sprintf("hub for %s@%s", n, h), nil } - -func perPage(limit, max int) int { - if limit > 0 { - limit = limit + (limit / 2) - if limit < max { - return limit - } - } - return max -} - -func addQuery(path string, params map[string]interface{}) string { - if len(params) == 0 { - return path - } - - query := url.Values{} - for key, value := range params { - switch v := value.(type) { - case string: - query.Add(key, v) - case nil: - query.Add(key, "") - case int: - query.Add(key, fmt.Sprintf("%d", v)) - case bool: - query.Add(key, fmt.Sprintf("%v", v)) - } - } - - sep := "?" - if strings.Contains(path, sep) { - sep = "&" - } - return path + sep + query.Encode() -} From 9000548b1cb631d7a804c57bb822b820d7efd0b9 Mon Sep 17 00:00:00 2001 From: nate smith Date: Thu, 10 Oct 2019 17:08:04 -0500 Subject: [PATCH 06/22] remove project in favor of ghr --- github/project.go | 119 ---------------------------------------------- 1 file changed, 119 deletions(-) delete mode 100644 github/project.go diff --git a/github/project.go b/github/project.go deleted file mode 100644 index 6e9fa3c41..000000000 --- a/github/project.go +++ /dev/null @@ -1,119 +0,0 @@ -package github - -import ( - "fmt" - "net/url" - "strings" - - "github.com/github/gh-cli/utils" -) - -type Project struct { - Name string - Owner string - Host string - Protocol string -} - -func (p Project) String() string { - return fmt.Sprintf("%s/%s", p.Owner, p.Name) -} - -func (p *Project) SameAs(other *Project) bool { - return strings.ToLower(p.Owner) == strings.ToLower(other.Owner) && - strings.ToLower(p.Name) == strings.ToLower(other.Name) && - strings.ToLower(p.Host) == strings.ToLower(other.Host) -} - -func (p *Project) WebURL(name, owner, path string) string { - if owner == "" { - owner = p.Owner - } - if name == "" { - name = p.Name - } - - ownerWithName := fmt.Sprintf("%s/%s", owner, name) - url := fmt.Sprintf("%s://%s", p.Protocol, utils.ConcatPaths(p.Host, ownerWithName)) - if path != "" { - url = utils.ConcatPaths(url, path) - } - - return url -} - -func (p *Project) GitURL(name, owner string, isSSH bool) string { - return p.WebURL(name, owner, "") + ".git" -} - -func NewProjectFromURL(url *url.URL) (p *Project, err error) { - if !KnownGitHubHostsInclude(url.Host) { - err = &GithubHostError{url} - return - } - - parts := strings.SplitN(url.Path, "/", 4) - if len(parts) <= 2 { - err = fmt.Errorf("Invalid GitHub URL: %s", url) - return - } - - name := strings.TrimSuffix(parts[2], ".git") - p = newProject(parts[1], name, url.Host, url.Scheme) - - return -} - -func NewProject(owner, name, host string) *Project { - return newProject(owner, name, host, "") -} - -func newProject(owner, name, host, protocol string) *Project { - if strings.Contains(owner, "/") { - result := strings.SplitN(owner, "/", 2) - owner = result[0] - if name == "" { - name = result[1] - } - } else if strings.Contains(name, "/") { - result := strings.SplitN(name, "/", 2) - if owner == "" { - owner = result[0] - } - name = result[1] - } - - if host == "" { - host = DefaultGitHubHost() - } - if host == "ssh.github.com" { - host = GitHubHost - } - - if protocol != "http" && protocol != "https" { - protocol = "" - } - if protocol == "" { - h := CurrentConfig().Find(host) - if h != nil { - protocol = h.Protocol - } - } - if protocol == "" { - protocol = "https" - } - - if owner == "" { - h := CurrentConfig().Find(host) - if h != nil { - owner = h.User - } - } - - return &Project{ - Name: name, - Owner: owner, - Host: host, - Protocol: protocol, - } -} From d7ee9a0ade66779cba65076fadcc738f1353a1de Mon Sep 17 00:00:00 2001 From: nate smith Date: Thu, 10 Oct 2019 17:42:52 -0500 Subject: [PATCH 07/22] move remote to context package --- context/ghrepo.go | 2 +- {github => context}/remote.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) rename {github => context}/remote.go (99%) diff --git a/context/ghrepo.go b/context/ghrepo.go index a64425761..5ea9a45a6 100644 --- a/context/ghrepo.go +++ b/context/ghrepo.go @@ -26,7 +26,7 @@ func CurrentGitHubRepository() (*GitHubRepository, error) { return nil, err } } else { - remote, rerr := github.GuessRemote() + remote, rerr := GuessRemote() if rerr != nil { return nil, rerr diff --git a/github/remote.go b/context/remote.go similarity index 99% rename from github/remote.go rename to context/remote.go index e51266d93..a47f382b8 100644 --- a/github/remote.go +++ b/context/remote.go @@ -1,4 +1,4 @@ -package github +package context import ( "fmt" From b94b448f08932abe4d1d2c157750c0ba202251e7 Mon Sep 17 00:00:00 2001 From: nate smith Date: Thu, 10 Oct 2019 17:53:53 -0500 Subject: [PATCH 08/22] remove unused method --- github/config.go | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/github/config.go b/github/config.go index 9a5221e50..420f56c19 100644 --- a/github/config.go +++ b/github/config.go @@ -344,18 +344,6 @@ func (c *Config) DefaultHost() (host *Host, err error) { return } -func (c *Config) DefaultHostNoPrompt() (*Host, error) { - if GitHubHostEnv != "" { - return c.PromptForHost(GitHubHostEnv) - } else if len(c.Hosts) > 0 { - host := c.Hosts[0] - // HACK: forces host to inherit GITHUB_TOKEN if applicable - return c.PromptForHost(host.Host) - } else { - return c.PromptForHost(GitHubHost) - } -} - // CheckWriteable checks if config file is writeable. This should // be called before asking for credentials and only if current // operation needs to update the file. See issue #1314 for details. From 6ef29819c7bdcfd69715482bfeb4f17c54418994 Mon Sep 17 00:00:00 2001 From: nate smith Date: Fri, 11 Oct 2019 10:53:29 -0500 Subject: [PATCH 09/22] towards moving config into context --- {github => context}/config.go | 6 +-- {github => context}/config_decoder.go | 2 +- {github => context}/config_encoder.go | 2 +- {github => context}/config_service.go | 2 +- context/context.go | 7 ++- context/ghrepo.go | 14 +++--- github/hosts.go | 64 --------------------------- 7 files changed, 17 insertions(+), 80 deletions(-) rename {github => context}/config.go (98%) rename {github => context}/config_decoder.go (98%) rename {github => context}/config_encoder.go (98%) rename {github => context}/config_service.go (97%) delete mode 100644 github/hosts.go diff --git a/github/config.go b/context/config.go similarity index 98% rename from github/config.go rename to context/config.go index 420f56c19..5ff7fd186 100644 --- a/github/config.go +++ b/context/config.go @@ -1,4 +1,4 @@ -package github +package context import ( "bufio" @@ -41,7 +41,7 @@ func (c *Config) PromptForHost(host string) (h *Host, err error) { token := c.DetectToken() tokenFromEnv := token != "" - if host != GitHubHost { + if host != GitHubHostname() { if _, e := url.Parse("https://" + host); e != nil { err = fmt.Errorf("invalid hostname: %q", host) return @@ -338,7 +338,7 @@ func (c *Config) DefaultHost() (host *Host, err error) { // HACK: forces host to inherit GITHUB_TOKEN if applicable host, err = c.PromptForHost(host.Host) } else { - host, err = c.PromptForHost(DefaultGitHubHost()) + host, err = c.PromptForHost(GitHubHostname()) } return diff --git a/github/config_decoder.go b/context/config_decoder.go similarity index 98% rename from github/config_decoder.go rename to context/config_decoder.go index 7e467b761..8a703c0f8 100644 --- a/github/config_decoder.go +++ b/context/config_decoder.go @@ -1,4 +1,4 @@ -package github +package context import ( "io" diff --git a/github/config_encoder.go b/context/config_encoder.go similarity index 98% rename from github/config_encoder.go rename to context/config_encoder.go index 87609d38d..4b11f9b73 100644 --- a/github/config_encoder.go +++ b/context/config_encoder.go @@ -1,4 +1,4 @@ -package github +package context import ( "io" diff --git a/github/config_service.go b/context/config_service.go similarity index 97% rename from github/config_service.go rename to context/config_service.go index 3aaa6915f..121517e2c 100644 --- a/github/config_service.go +++ b/context/config_service.go @@ -1,4 +1,4 @@ -package github +package context import ( "os" diff --git a/context/context.go b/context/context.go index 85af0d8f9..68695ddcf 100644 --- a/context/context.go +++ b/context/context.go @@ -7,7 +7,6 @@ import ( "strings" "github.com/github/gh-cli/git" - "github.com/github/gh-cli/github" ) // GetToken returns the authentication token as stored in the config file for the user running gh-cli @@ -28,7 +27,7 @@ func GetToken() (string, error) { } func CurrentUsername() (string, error) { - host, err := github.CurrentConfig().DefaultHost() + host, err := CurrentConfig().DefaultHost() if err != nil { return "", nil } @@ -43,3 +42,7 @@ func CurrentBranch() (string, error) { return strings.Replace(currentBranch, "refs/heads/", "", 1), nil } + +func GitHubHostname() string { + return "github.com" +} diff --git a/context/ghrepo.go b/context/ghrepo.go index 5ea9a45a6..bfb8b8f84 100644 --- a/context/ghrepo.go +++ b/context/ghrepo.go @@ -5,8 +5,6 @@ import ( "net/url" "os" "strings" - - "github.com/github/gh-cli/github" ) type GitHubRepository struct { @@ -21,7 +19,7 @@ func CurrentGitHubRepository() (*GitHubRepository, error) { var repoURL *url.URL var err error if repoFromEnv := os.Getenv("GH_REPO"); repoFromEnv != "" { - repoURL, err = url.Parse(fmt.Sprintf("https://github.com/%s.git", repoFromEnv)) + repoURL, err = url.Parse(fmt.Sprintf("https://%s/%s.git", GitHubHostname(), repoFromEnv)) if err != nil { return nil, err } @@ -35,7 +33,7 @@ func CurrentGitHubRepository() (*GitHubRepository, error) { } urlError := fmt.Errorf("invalid GitHub URL: %s", repoURL) - if !github.KnownGitHubHostsInclude(repoURL.Host) { + if repoURL.Host != GitHubHostname() && repoURL.Host != fmt.Sprintf("ssh.%s", GitHubHostname()) { return nil, urlError } @@ -64,17 +62,17 @@ func CurrentGitHubRepository() (*GitHubRepository, error) { } if host == "" { - host = github.DefaultGitHubHost() + host = GitHubHostname() } if host == "ssh.github.com" { - host = github.GitHubHost + host = GitHubHostname() } if protocol != "http" && protocol != "https" { protocol = "" } if protocol == "" { - h := github.CurrentConfig().Find(host) + h := CurrentConfig().Find(host) if h != nil { protocol = h.Protocol } @@ -84,7 +82,7 @@ func CurrentGitHubRepository() (*GitHubRepository, error) { } if owner == "" { - h := github.CurrentConfig().Find(host) + h := CurrentConfig().Find(host) if h != nil { owner = h.User } diff --git a/github/hosts.go b/github/hosts.go deleted file mode 100644 index a74d9810c..000000000 --- a/github/hosts.go +++ /dev/null @@ -1,64 +0,0 @@ -package github - -import ( - "fmt" - "net/url" - "os" - "strings" - - "github.com/github/gh-cli/git" -) - -var ( - GitHubHostEnv = os.Getenv("GITHUB_HOST") - cachedHosts []string -) - -type GithubHostError struct { - url *url.URL -} - -func (e *GithubHostError) Error() string { - return fmt.Sprintf("Invalid GitHub URL: %s", e.url) -} - -func KnownGitHubHostsInclude(host string) bool { - for _, hh := range knownGitHubHosts() { - if hh == host { - return true - } - } - - return false -} - -func knownGitHubHosts() []string { - if cachedHosts != nil { - return cachedHosts - } - - hosts := []string{} - defaultHost := DefaultGitHubHost() - hosts = append(hosts, defaultHost) - hosts = append(hosts, "ssh.github.com") - - ghHosts, _ := git.ConfigAll("hub.host") - for _, ghHost := range ghHosts { - ghHost = strings.TrimSpace(ghHost) - if ghHost != "" { - hosts = append(hosts, ghHost) - } - } - - cachedHosts = hosts - return hosts -} - -func DefaultGitHubHost() string { - defaultHost := GitHubHostEnv - if defaultHost == "" { - defaultHost = GitHubHost - } - - return defaultHost -} From cccb6832c36b8b80a3ee6c438c956023fe10c2da Mon Sep 17 00:00:00 2001 From: nate smith Date: Mon, 14 Oct 2019 11:02:04 -0500 Subject: [PATCH 10/22] Revert "towards moving config into context" This reverts commit 7427716ea81fbe4bc6645767d1aca3b9beb05d46. --- context/context.go | 7 +-- context/ghrepo.go | 14 +++--- {context => github}/config.go | 6 +-- {context => github}/config_decoder.go | 2 +- {context => github}/config_encoder.go | 2 +- {context => github}/config_service.go | 2 +- github/hosts.go | 64 +++++++++++++++++++++++++++ 7 files changed, 80 insertions(+), 17 deletions(-) rename {context => github}/config.go (98%) rename {context => github}/config_decoder.go (98%) rename {context => github}/config_encoder.go (98%) rename {context => github}/config_service.go (97%) create mode 100644 github/hosts.go diff --git a/context/context.go b/context/context.go index 68695ddcf..85af0d8f9 100644 --- a/context/context.go +++ b/context/context.go @@ -7,6 +7,7 @@ import ( "strings" "github.com/github/gh-cli/git" + "github.com/github/gh-cli/github" ) // GetToken returns the authentication token as stored in the config file for the user running gh-cli @@ -27,7 +28,7 @@ func GetToken() (string, error) { } func CurrentUsername() (string, error) { - host, err := CurrentConfig().DefaultHost() + host, err := github.CurrentConfig().DefaultHost() if err != nil { return "", nil } @@ -42,7 +43,3 @@ func CurrentBranch() (string, error) { return strings.Replace(currentBranch, "refs/heads/", "", 1), nil } - -func GitHubHostname() string { - return "github.com" -} diff --git a/context/ghrepo.go b/context/ghrepo.go index bfb8b8f84..5ea9a45a6 100644 --- a/context/ghrepo.go +++ b/context/ghrepo.go @@ -5,6 +5,8 @@ import ( "net/url" "os" "strings" + + "github.com/github/gh-cli/github" ) type GitHubRepository struct { @@ -19,7 +21,7 @@ func CurrentGitHubRepository() (*GitHubRepository, error) { var repoURL *url.URL var err error if repoFromEnv := os.Getenv("GH_REPO"); repoFromEnv != "" { - repoURL, err = url.Parse(fmt.Sprintf("https://%s/%s.git", GitHubHostname(), repoFromEnv)) + repoURL, err = url.Parse(fmt.Sprintf("https://github.com/%s.git", repoFromEnv)) if err != nil { return nil, err } @@ -33,7 +35,7 @@ func CurrentGitHubRepository() (*GitHubRepository, error) { } urlError := fmt.Errorf("invalid GitHub URL: %s", repoURL) - if repoURL.Host != GitHubHostname() && repoURL.Host != fmt.Sprintf("ssh.%s", GitHubHostname()) { + if !github.KnownGitHubHostsInclude(repoURL.Host) { return nil, urlError } @@ -62,17 +64,17 @@ func CurrentGitHubRepository() (*GitHubRepository, error) { } if host == "" { - host = GitHubHostname() + host = github.DefaultGitHubHost() } if host == "ssh.github.com" { - host = GitHubHostname() + host = github.GitHubHost } if protocol != "http" && protocol != "https" { protocol = "" } if protocol == "" { - h := CurrentConfig().Find(host) + h := github.CurrentConfig().Find(host) if h != nil { protocol = h.Protocol } @@ -82,7 +84,7 @@ func CurrentGitHubRepository() (*GitHubRepository, error) { } if owner == "" { - h := CurrentConfig().Find(host) + h := github.CurrentConfig().Find(host) if h != nil { owner = h.User } diff --git a/context/config.go b/github/config.go similarity index 98% rename from context/config.go rename to github/config.go index 5ff7fd186..420f56c19 100644 --- a/context/config.go +++ b/github/config.go @@ -1,4 +1,4 @@ -package context +package github import ( "bufio" @@ -41,7 +41,7 @@ func (c *Config) PromptForHost(host string) (h *Host, err error) { token := c.DetectToken() tokenFromEnv := token != "" - if host != GitHubHostname() { + if host != GitHubHost { if _, e := url.Parse("https://" + host); e != nil { err = fmt.Errorf("invalid hostname: %q", host) return @@ -338,7 +338,7 @@ func (c *Config) DefaultHost() (host *Host, err error) { // HACK: forces host to inherit GITHUB_TOKEN if applicable host, err = c.PromptForHost(host.Host) } else { - host, err = c.PromptForHost(GitHubHostname()) + host, err = c.PromptForHost(DefaultGitHubHost()) } return diff --git a/context/config_decoder.go b/github/config_decoder.go similarity index 98% rename from context/config_decoder.go rename to github/config_decoder.go index 8a703c0f8..7e467b761 100644 --- a/context/config_decoder.go +++ b/github/config_decoder.go @@ -1,4 +1,4 @@ -package context +package github import ( "io" diff --git a/context/config_encoder.go b/github/config_encoder.go similarity index 98% rename from context/config_encoder.go rename to github/config_encoder.go index 4b11f9b73..87609d38d 100644 --- a/context/config_encoder.go +++ b/github/config_encoder.go @@ -1,4 +1,4 @@ -package context +package github import ( "io" diff --git a/context/config_service.go b/github/config_service.go similarity index 97% rename from context/config_service.go rename to github/config_service.go index 121517e2c..3aaa6915f 100644 --- a/context/config_service.go +++ b/github/config_service.go @@ -1,4 +1,4 @@ -package context +package github import ( "os" diff --git a/github/hosts.go b/github/hosts.go new file mode 100644 index 000000000..a74d9810c --- /dev/null +++ b/github/hosts.go @@ -0,0 +1,64 @@ +package github + +import ( + "fmt" + "net/url" + "os" + "strings" + + "github.com/github/gh-cli/git" +) + +var ( + GitHubHostEnv = os.Getenv("GITHUB_HOST") + cachedHosts []string +) + +type GithubHostError struct { + url *url.URL +} + +func (e *GithubHostError) Error() string { + return fmt.Sprintf("Invalid GitHub URL: %s", e.url) +} + +func KnownGitHubHostsInclude(host string) bool { + for _, hh := range knownGitHubHosts() { + if hh == host { + return true + } + } + + return false +} + +func knownGitHubHosts() []string { + if cachedHosts != nil { + return cachedHosts + } + + hosts := []string{} + defaultHost := DefaultGitHubHost() + hosts = append(hosts, defaultHost) + hosts = append(hosts, "ssh.github.com") + + ghHosts, _ := git.ConfigAll("hub.host") + for _, ghHost := range ghHosts { + ghHost = strings.TrimSpace(ghHost) + if ghHost != "" { + hosts = append(hosts, ghHost) + } + } + + cachedHosts = hosts + return hosts +} + +func DefaultGitHubHost() string { + defaultHost := GitHubHostEnv + if defaultHost == "" { + defaultHost = GitHubHost + } + + return defaultHost +} From 0600c8c68c78942678dc2d8094fa3ad088d42408 Mon Sep 17 00:00:00 2001 From: nate smith Date: Tue, 15 Oct 2019 13:36:38 -0500 Subject: [PATCH 11/22] switch to context struct --- api/client.go | 3 ++- api/queries.go | 20 ++++++++----------- context/context.go | 49 ++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 59 insertions(+), 13 deletions(-) diff --git a/api/client.go b/api/client.go index 8e431be1a..a7ea0636c 100644 --- a/api/client.go +++ b/api/client.go @@ -56,10 +56,11 @@ func graphQL(query string, variables map[string]string, v interface{}) error { return err } - token, err := context.GetToken() + ctx, err := context.GetContext() if err != nil { return err } + token := ctx.Token req.Header.Set("Authorization", "token "+token) req.Header.Set("Content-Type", "application/json; charset=utf-8") diff --git a/api/queries.go b/api/queries.go index 51fe5d700..a72b75189 100644 --- a/api/queries.go +++ b/api/queries.go @@ -79,22 +79,18 @@ func PullRequests() (PullRequestsPayload, error) { } ` - ghRepo, rerr := context.CurrentGitHubRepository() - if rerr != nil { - return PullRequestsPayload{}, nil - } - - owner := ghRepo.Owner - repo := ghRepo.Name - currentBranch, cberr := context.CurrentBranch() - if cberr != nil { - return PullRequestsPayload{}, cberr - } - currentUsername, err := context.CurrentUsername() + ctx, err := context.GetContext() if err != nil { return PullRequestsPayload{}, err } + ghRepo := ctx.GHRepo + currentBranch := ctx.Branch + currentUsername := ctx.Username + + owner := ghRepo.Owner + repo := ghRepo.Name + viewerQuery := fmt.Sprintf("repo:%s/%s state:open is:pr author:%s", owner, repo, currentUsername) reviewerQuery := fmt.Sprintf("repo:%s/%s state:open review-requested:%s", owner, repo, currentUsername) diff --git a/context/context.go b/context/context.go index 85af0d8f9..ea9234a39 100644 --- a/context/context.go +++ b/context/context.go @@ -1,6 +1,7 @@ package context import ( + "fmt" "io/ioutil" "os/user" "regexp" @@ -10,6 +11,13 @@ import ( "github.com/github/gh-cli/github" ) +type Context struct { + Token string + Username string + Branch string + GHRepo *GitHubRepository +} + // GetToken returns the authentication token as stored in the config file for the user running gh-cli func GetToken() (string, error) { usr, err := user.Current() @@ -43,3 +51,44 @@ func CurrentBranch() (string, error) { return strings.Replace(currentBranch, "refs/heads/", "", 1), nil } + +func GetContext() (*Context, error) { + errors := []error{} + token, terr := GetToken() + if terr != nil { + errors = append(errors, terr) + } + + username, uerr := CurrentUsername() + if uerr != nil { + errors = append(errors, uerr) + } + + branch, berr := CurrentBranch() + if berr != nil { + errors = append(errors, berr) + } + + ghrepo, ghrerr := CurrentGitHubRepository() + if ghrerr != nil { + errors = append(errors, ghrerr) + } + + var err error = nil + + if len(errors) > 0 { + errStrings := []string{} + for _, e := range errors { + errStrings = append(errStrings, e.Error()) + } + + err = fmt.Errorf(strings.Join(errStrings, ", ")) + } + + return &Context{ + Token: token, + Username: username, + Branch: branch, + GHRepo: ghrepo, + }, err +} From 8016d808842fdf4974510df6db95fd1f6a25c3c1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Thu, 17 Oct 2019 01:13:15 +0200 Subject: [PATCH 12/22] Create overridable Context interface --- Makefile | 5 ++ api/client.go | 3 +- api/queries.go | 22 +++-- command/pr.go | 63 ++++---------- command/pr_test.go | 8 +- command/root.go | 26 ++++++ context/blank_context.go | 63 ++++++++++++++ context/config_file.go | 42 ++++++++++ context/config_file_test.go | 24 ++++++ context/context.go | 161 ++++++++++++++++++++++-------------- context/ghrepo.go | 100 ---------------------- context/remote.go | 113 ++++++++++++------------- context/remote_test.go | 18 ++++ 13 files changed, 367 insertions(+), 281 deletions(-) create mode 100644 context/blank_context.go create mode 100644 context/config_file.go create mode 100644 context/config_file_test.go delete mode 100644 context/ghrepo.go create mode 100644 context/remote_test.go diff --git a/Makefile b/Makefile index 93cfc0436..988c9cad5 100644 --- a/Makefile +++ b/Makefile @@ -5,3 +5,8 @@ BUILD_FILES = $(shell go list -f '{{range .GoFiles}}{{$$.Dir}}/{{.}}\ bin/gh: $(BUILD_FILES) go build -o "$@" + +test: + go test ./... + +.PHONY: test \ No newline at end of file diff --git a/api/client.go b/api/client.go index a351f564a..cafbbb507 100644 --- a/api/client.go +++ b/api/client.go @@ -56,11 +56,10 @@ var GraphQL = func(query string, variables map[string]string, data interface{}) return err } - ctx, err := context.GetContext() + token, err := context.Current().AuthToken() if err != nil { return err } - token := ctx.Token req.Header.Set("Authorization", "token "+token) req.Header.Set("Content-Type", "application/json; charset=utf-8") diff --git a/api/queries.go b/api/queries.go index 187d2c860..b9140670a 100644 --- a/api/queries.go +++ b/api/queries.go @@ -19,7 +19,7 @@ type PullRequest struct { HeadRefName string } -func PullRequests() (PullRequestsPayload, error) { +func PullRequests() (*PullRequestsPayload, error) { type edges struct { Edges []struct { Node PullRequest @@ -79,14 +79,18 @@ func PullRequests() (PullRequestsPayload, error) { } ` - ctx, err := context.GetContext() + ghRepo, err := context.Current().BaseRepo() if err != nil { - return PullRequestsPayload{}, err + return nil, err + } + currentBranch, err := context.Current().Branch() + if err != nil { + return nil, err + } + currentUsername, err := context.Current().AuthLogin() + if err != nil { + return nil, err } - - ghRepo := ctx.GHRepo - currentBranch := ctx.Branch - currentUsername := ctx.Username owner := ghRepo.Owner repo := ghRepo.Name @@ -105,7 +109,7 @@ func PullRequests() (PullRequestsPayload, error) { var resp response err = GraphQL(query, variables, &resp) if err != nil { - return PullRequestsPayload{}, err + return nil, err } var viewerCreated []PullRequest @@ -129,5 +133,5 @@ func PullRequests() (PullRequestsPayload, error) { currentPR, } - return payload, nil + return &payload, nil } diff --git a/command/pr.go b/command/pr.go index 596d9e988..c4377c0e0 100644 --- a/command/pr.go +++ b/command/pr.go @@ -2,15 +2,11 @@ package command import ( "fmt" - "net/url" - "os" "os/exec" "strconv" - "strings" "github.com/github/gh-cli/api" - "github.com/github/gh-cli/git" - "github.com/github/gh-cli/github" + "github.com/github/gh-cli/context" "github.com/github/gh-cli/utils" "github.com/spf13/cobra" ) @@ -52,7 +48,11 @@ func prList(cmd *cobra.Command, args []string) error { if prPayload.CurrentPR != nil { printPrs(*prPayload.CurrentPR) } else { - message := fmt.Sprintf(" There is no pull request associated with %s", utils.Cyan("["+currentBranch()+"]")) + currentBranch, err := context.Current().Branch() + if err != nil { + return err + } + message := fmt.Sprintf(" There is no pull request associated with %s", utils.Cyan("["+currentBranch+"]")) printMessage(message) } fmt.Println() @@ -77,19 +77,26 @@ func prList(cmd *cobra.Command, args []string) error { } func prView(cmd *cobra.Command, args []string) error { - project := project() + baseRepo, err := context.Current().BaseRepo() + if err != nil { + return err + } var openURL string if len(args) > 0 { if prNumber, err := strconv.Atoi(args[0]); err == nil { - openURL = project.WebURL("", "", fmt.Sprintf("pull/%d", prNumber)) + // TODO: move URL generation into GitHubRepository + openURL = fmt.Sprintf("https://github.com/%s/%s/pull/%d", baseRepo.Owner, baseRepo.Name, prNumber) } else { return fmt.Errorf("invalid pull request number: '%s'", args[0]) } } else { prPayload, err := api.PullRequests() if err != nil || prPayload.CurrentPR == nil { - branch := currentBranch() + branch, err := context.Current().Branch() + if err != nil { + return err + } return fmt.Errorf("The [%s] branch has no open PRs", branch) } openURL = prPayload.CurrentPR.URL @@ -130,41 +137,3 @@ func openInBrowser(url string) error { endingArgs := append(launcher[1:], url) return exec.Command(launcher[0], endingArgs...).Run() } - -// The functions below should be replaced at some point by the context package -// 🧨🧨🧨🧨🧨🧨🧨🧨🧨🧨🧨🧨🧨🧨🧨🧨🧨🧨🧨🧨🧨🧨🧨🧨🧨🧨🧨🧨🧨🧨🧨🧨🧨🧨 -func currentBranch() string { - currentBranch, err := git.Head() - if err != nil { - panic(err) - } - - return strings.Replace(currentBranch, "refs/heads/", "", 1) -} - -func project() github.Project { - if repoFromEnv := os.Getenv("GH_REPO"); repoFromEnv != "" { - repoURL, err := url.Parse(fmt.Sprintf("https://github.com/%s.git", repoFromEnv)) - if err != nil { - panic(err) - } - project, err := github.NewProjectFromURL(repoURL) - if err != nil { - panic(err) - } - return *project - } - - remotes, err := github.Remotes() - if err != nil { - panic(err) - } - - for _, remote := range remotes { - if project, err := remote.Project(); err == nil { - return *project - } - } - - panic("Could not get the project. What is a project? I don't know, it's kind of like a git repository I think?") -} diff --git a/command/pr_test.go b/command/pr_test.go index ced88b3b8..fa1538304 100644 --- a/command/pr_test.go +++ b/command/pr_test.go @@ -4,16 +4,18 @@ import ( "regexp" "testing" + "github.com/github/gh-cli/context" "github.com/github/gh-cli/test" ) func TestPRList(t *testing.T) { + ctx := context.InitBlankContext() + ctx.SetBaseRepo("github/FAKE-GITHUB-REPO-NAME") + ctx.SetBranch("master") + teardown := test.MockGraphQLResponse("test/fixtures/pr.json") defer teardown() - gitRepo := test.UseTempGitRepo() - defer gitRepo.TearDown() - output, err := test.RunCommand(RootCmd, "pr list") if err != nil { t.Errorf("error running command `pr list`: %v", err) diff --git a/command/root.go b/command/root.go index a644189bf..f4c227cb5 100644 --- a/command/root.go +++ b/command/root.go @@ -2,15 +2,41 @@ package command import ( "fmt" + "os" + "github.com/github/gh-cli/context" "github.com/spf13/cobra" ) +var ( + currentRepo string + currentBranch string +) + +func init() { + RootCmd.PersistentFlags().StringVarP(¤tRepo, "repo", "R", "", "current GitHub repository") + RootCmd.PersistentFlags().StringVarP(¤tBranch, "current-branch", "B", "", "current git branch") +} + +func initContext() { + ctx := context.InitDefaultContext() + ctx.SetBranch(currentBranch) + repo := currentRepo + if repo == "" { + repo = os.Getenv("GH_REPO") + } + ctx.SetBaseRepo(repo) +} + +// RootCmd is the entry point of command-line execution var RootCmd = &cobra.Command{ Use: "gh", Short: "GitHub CLI", Long: `Do things with GitHub from your terminal`, Args: cobra.MinimumNArgs(1), + PersistentPreRun: func(cmd *cobra.Command, args []string) { + initContext() + }, Run: func(cmd *cobra.Command, args []string) { fmt.Println("root") }, diff --git a/context/blank_context.go b/context/blank_context.go new file mode 100644 index 000000000..166026598 --- /dev/null +++ b/context/blank_context.go @@ -0,0 +1,63 @@ +package context + +import ( + "fmt" + "strings" +) + +// InitBlankContext initializes a blank context for testing +func InitBlankContext() Context { + currentContext = &blankContext{ + authToken: "OTOKEN", + authLogin: "monalisa", + } + return currentContext +} + +// A Context implementation that queries the filesystem +type blankContext struct { + authToken string + authLogin string + branch string + baseRepo *GitHubRepository +} + +func (c *blankContext) AuthToken() (string, error) { + return c.authToken, nil +} + +func (c *blankContext) AuthLogin() (string, error) { + return c.authLogin, nil +} + +func (c *blankContext) Branch() (string, error) { + if c.branch == "" { + return "", fmt.Errorf("branch was not initialized") + } + return c.branch, nil +} + +func (c *blankContext) SetBranch(b string) { + c.branch = b +} + +func (c *blankContext) Remotes() (Remotes, error) { + return Remotes{}, nil +} + +func (c *blankContext) BaseRepo() (*GitHubRepository, error) { + if c.baseRepo == nil { + return nil, fmt.Errorf("base repo was not initialized") + } + return c.baseRepo, nil +} + +func (c *blankContext) SetBaseRepo(nwo string) { + parts := strings.SplitN(nwo, "/", 2) + if len(parts) == 2 { + c.baseRepo = &GitHubRepository{ + Owner: parts[0], + Name: parts[1], + } + } +} diff --git a/context/config_file.go b/context/config_file.go new file mode 100644 index 000000000..0d5c02ddb --- /dev/null +++ b/context/config_file.go @@ -0,0 +1,42 @@ +package context + +import ( + "io" + "io/ioutil" + "os" + + "gopkg.in/yaml.v3" +) + +type configEntry struct { + User string + Token string `yaml:"oauth_token"` +} + +func parseConfigFile(fn string) (*configEntry, error) { + f, err := os.Open(fn) + if err != nil { + return nil, err + } + defer f.Close() + return parseConfig(f) +} + +func parseConfig(r io.Reader) (*configEntry, error) { + data, err := ioutil.ReadAll(r) + if err != nil { + return nil, err + } + var config yaml.Node + err = yaml.Unmarshal(data, &config) + if err != nil { + return nil, err + } + var entries []configEntry + // TODO: this will panic if the config is malformed + err = config.Content[0].Content[1].Decode(&entries) + if err != nil { + return nil, err + } + return &entries[0], nil +} diff --git a/context/config_file_test.go b/context/config_file_test.go new file mode 100644 index 000000000..e519401c8 --- /dev/null +++ b/context/config_file_test.go @@ -0,0 +1,24 @@ +package context + +import ( + "strings" + "testing" +) + +func Test_parseConfig(t *testing.T) { + c := strings.NewReader(`--- +github.com: +- user: monalisa + oauth_token: OTOKEN +`) + entry, err := parseConfig(c) + if err != nil { + t.Error(err) + } + if entry.User != "monalisa" { + t.Errorf("got User: %q", entry.User) + } + if entry.Token != "OTOKEN" { + t.Errorf("got User: %q", entry.Token) + } +} diff --git a/context/context.go b/context/context.go index ea9234a39..be5f59187 100644 --- a/context/context.go +++ b/context/context.go @@ -1,94 +1,135 @@ package context import ( - "fmt" - "io/ioutil" - "os/user" - "regexp" "strings" "github.com/github/gh-cli/git" - "github.com/github/gh-cli/github" + "github.com/mitchellh/go-homedir" ) -type Context struct { - Token string - Username string - Branch string - GHRepo *GitHubRepository +// Context represents the interface for querying information about the current environment +type Context interface { + AuthToken() (string, error) + AuthLogin() (string, error) + Branch() (string, error) + SetBranch(string) + Remotes() (Remotes, error) + BaseRepo() (*GitHubRepository, error) + SetBaseRepo(string) } -// GetToken returns the authentication token as stored in the config file for the user running gh-cli -func GetToken() (string, error) { - usr, err := user.Current() +var currentContext Context + +// Current returns the currently initialized Context instance +func Current() Context { + return currentContext +} + +// InitDefaultContext initializes the default filesystem context +func InitDefaultContext() Context { + ctx := &fsContext{} + if currentContext == nil { + currentContext = ctx + } + return ctx +} + +// A Context implementation that queries the filesystem +type fsContext struct { + config *configEntry + remotes Remotes + branch string + baseRepo *GitHubRepository +} + +func (c *fsContext) configFile() string { + dir, _ := homedir.Expand("~/.config/hub") + return dir +} + +func (c *fsContext) getConfig() (*configEntry, error) { + if c.config == nil { + entry, err := parseConfigFile(c.configFile()) + if err != nil { + return nil, err + } + c.config = entry + } + return c.config, nil +} + +func (c *fsContext) AuthToken() (string, error) { + config, err := c.getConfig() if err != nil { return "", err } + return config.Token, nil +} - content, err := ioutil.ReadFile(usr.HomeDir + "/.config/hub") +func (c *fsContext) AuthLogin() (string, error) { + config, err := c.getConfig() if err != nil { return "", err } - - r := regexp.MustCompile(`oauth_token: (\w+)`) - token := r.FindStringSubmatch(string(content)) - return token[1], nil + return config.User, nil } -func CurrentUsername() (string, error) { - host, err := github.CurrentConfig().DefaultHost() - if err != nil { - return "", nil +func (c *fsContext) Branch() (string, error) { + if c.branch != "" { + return c.branch, nil } - return host.User, nil -} -func CurrentBranch() (string, error) { currentBranch, err := git.Head() if err != nil { return "", err } - return strings.Replace(currentBranch, "refs/heads/", "", 1), nil + c.branch = strings.Replace(currentBranch, "refs/heads/", "", 1) + return c.branch, nil } -func GetContext() (*Context, error) { - errors := []error{} - token, terr := GetToken() - if terr != nil { - errors = append(errors, terr) - } +func (c *fsContext) SetBranch(b string) { + c.branch = b +} - username, uerr := CurrentUsername() - if uerr != nil { - errors = append(errors, uerr) - } - - branch, berr := CurrentBranch() - if berr != nil { - errors = append(errors, berr) - } - - ghrepo, ghrerr := CurrentGitHubRepository() - if ghrerr != nil { - errors = append(errors, ghrerr) - } - - var err error = nil - - if len(errors) > 0 { - errStrings := []string{} - for _, e := range errors { - errStrings = append(errStrings, e.Error()) +func (c *fsContext) Remotes() (Remotes, error) { + if c.remotes == nil { + rem, err := parseRemotes() + if err != nil { + return nil, err } + c.remotes = rem + } + return c.remotes, nil +} - err = fmt.Errorf(strings.Join(errStrings, ", ")) +func (c *fsContext) BaseRepo() (*GitHubRepository, error) { + if c.baseRepo != nil { + return c.baseRepo, nil } - return &Context{ - Token: token, - Username: username, - Branch: branch, - GHRepo: ghrepo, - }, err + remotes, err := c.Remotes() + if err != nil { + return nil, err + } + rem, err := remotes.FindByName("upstream", "github", "origin", "*") + if err != nil { + return nil, err + } + + c.baseRepo = &GitHubRepository{ + Owner: rem.Owner, + Name: rem.Repo, + } + return c.baseRepo, nil +} + +func (c *fsContext) SetBaseRepo(nwo string) { + parts := strings.SplitN(nwo, "/", 2) + if len(parts) == 2 { + c.baseRepo = &GitHubRepository{ + Owner: parts[0], + Name: parts[1], + } + } } diff --git a/context/ghrepo.go b/context/ghrepo.go deleted file mode 100644 index 5ea9a45a6..000000000 --- a/context/ghrepo.go +++ /dev/null @@ -1,100 +0,0 @@ -package context - -import ( - "fmt" - "net/url" - "os" - "strings" - - "github.com/github/gh-cli/github" -) - -type GitHubRepository struct { - Name string - Owner string - Host string - Protocol string -} - -func CurrentGitHubRepository() (*GitHubRepository, error) { - - var repoURL *url.URL - var err error - if repoFromEnv := os.Getenv("GH_REPO"); repoFromEnv != "" { - repoURL, err = url.Parse(fmt.Sprintf("https://github.com/%s.git", repoFromEnv)) - if err != nil { - return nil, err - } - } else { - remote, rerr := GuessRemote() - - if rerr != nil { - return nil, rerr - } - repoURL = remote.URL - } - - urlError := fmt.Errorf("invalid GitHub URL: %s", repoURL) - if !github.KnownGitHubHostsInclude(repoURL.Host) { - return nil, urlError - } - - parts := strings.SplitN(repoURL.Path, "/", 4) - if len(parts) <= 2 { - return nil, urlError - } - - name := strings.TrimSuffix(parts[2], ".git") - owner := parts[1] - host := repoURL.Host - protocol := repoURL.Scheme - - if strings.Contains(owner, "/") { - result := strings.SplitN(owner, "/", 2) - owner = result[0] - if name == "" { - name = result[1] - } - } else if strings.Contains(name, "/") { - result := strings.SplitN(name, "/", 2) - if owner == "" { - owner = result[0] - } - name = result[1] - } - - if host == "" { - host = github.DefaultGitHubHost() - } - if host == "ssh.github.com" { - host = github.GitHubHost - } - - if protocol != "http" && protocol != "https" { - protocol = "" - } - if protocol == "" { - h := github.CurrentConfig().Find(host) - if h != nil { - protocol = h.Protocol - } - } - if protocol == "" { - protocol = "https" - } - - if owner == "" { - h := github.CurrentConfig().Find(host) - if h != nil { - owner = h.User - } - } - - return &GitHubRepository{ - Name: name, - Owner: owner, - Host: host, - Protocol: protocol, - }, nil - -} diff --git a/context/remote.go b/context/remote.go index a47f382b8..d729bcbae 100644 --- a/context/remote.go +++ b/context/remote.go @@ -2,39 +2,56 @@ package context import ( "fmt" - "net/url" "regexp" "strings" "github.com/github/gh-cli/git" ) -var ( - OriginNamesInLookupOrder = []string{"upstream", "github", "origin"} -) +const defaultHostname = "github.com" +// Remotes represents a set of git remotes +type Remotes []*Remote + +// FindByName returns the first Remote whose name matches the list +func (r Remotes) FindByName(names ...string) (*Remote, error) { + for _, name := range names { + for _, rem := range r { + if rem.Name == name || name == "*" { + return rem, nil + } + } + } + return nil, fmt.Errorf("no GitHub remotes found") +} + +// Remote represents a git remote mapped to a GitHub repository type Remote struct { - Name string - URL *url.URL - PushURL *url.URL + Name string + Owner string + Repo string } -func (remote *Remote) String() string { - return remote.Name +func (r *Remote) String() string { + return r.Name } -func Remotes() (remotes []Remote, err error) { +// GitHubRepository represents a GitHub respository +type GitHubRepository struct { + Name string + Owner string +} + +func parseRemotes() (remotes Remotes, err error) { re := regexp.MustCompile(`(.+)\s+(.+)\s+\((push|fetch)\)`) - rs, err := git.Remotes() + gitRemotes, err := git.Remotes() if err != nil { - err = fmt.Errorf("Can't load git remote") return } - // build the remotes map remotesMap := make(map[string]map[string]string) - for _, r := range rs { + for _, r := range gitRemotes { if re.MatchString(r) { match := re.FindStringSubmatch(r) name := strings.TrimSpace(match[1]) @@ -49,61 +66,37 @@ func Remotes() (remotes []Remote, err error) { } } - // construct remotes in priority order - names := OriginNamesInLookupOrder - for _, name := range names { - if u, ok := remotesMap[name]; ok { - r, err := newRemote(name, u) - if err == nil { - remotes = append(remotes, r) - delete(remotesMap, name) - } + for name, urlMap := range remotesMap { + repo, err := repoFromURL(urlMap["fetch"]) + if err != nil { + repo, err = repoFromURL(urlMap["push"]) } - } - - // the rest of the remotes - for n, u := range remotesMap { - r, err := newRemote(n, u) if err == nil { - remotes = append(remotes, r) + remotes = append(remotes, &Remote{ + Name: name, + Owner: repo.Owner, + Repo: repo.Name, + }) } } return } -func newRemote(name string, urlMap map[string]string) (Remote, error) { - r := Remote{} - - fetchURL, ferr := git.ParseURL(urlMap["fetch"]) - pushURL, perr := git.ParseURL(urlMap["push"]) - if ferr != nil && perr != nil { - return r, fmt.Errorf("No valid remote URLs") - } - - r.Name = name - if ferr == nil { - r.URL = fetchURL - } - if perr == nil { - r.PushURL = pushURL - } - - return r, nil -} - -// GuessRemote attempts to select and return the remote a user likely wants to target when dealing with GitHub repositories. -func GuessRemote() (Remote, error) { - - remotes, err := Remotes() +func repoFromURL(u string) (*GitHubRepository, error) { + url, err := git.ParseURL(u) if err != nil { - return Remote{}, err + return nil, err } - - if len(remotes) == 0 { - return Remote{}, fmt.Errorf("unable to guess remote") + if url.Hostname() != defaultHostname { + return nil, fmt.Errorf("invalid hostname: %s", url.Hostname()) } - - // lol - return remotes[0], nil + parts := strings.SplitN(strings.TrimPrefix(url.Path, "/"), "/", 3) + if len(parts) < 2 { + return nil, fmt.Errorf("invalid path: %s", url.Path) + } + return &GitHubRepository{ + Owner: parts[0], + Name: strings.TrimSuffix(parts[1], ".git"), + }, nil } diff --git a/context/remote_test.go b/context/remote_test.go new file mode 100644 index 000000000..7c23a398f --- /dev/null +++ b/context/remote_test.go @@ -0,0 +1,18 @@ +package context + +import ( + "testing" +) + +func Test_repoFromURL(t *testing.T) { + r, err := repoFromURL("http://github.com/monalisa/octo-cat.git") + if err != nil { + t.Error(err) + } + if r.Owner != "monalisa" { + t.Errorf("got Owner: %q", r.Owner) + } + if r.Name != "octo-cat" { + t.Errorf("got Name: %q", r.Name) + } +} From a8aa5feb02d7f39e7d85a700d1b50fac1bbf21ac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Thu, 17 Oct 2019 01:47:26 +0200 Subject: [PATCH 13/22] Test remote parsing --- context/context.go | 6 +++++- context/remote.go | 7 +------ context/remote_test.go | 38 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 44 insertions(+), 7 deletions(-) diff --git a/context/context.go b/context/context.go index be5f59187..cff1f28f3 100644 --- a/context/context.go +++ b/context/context.go @@ -94,7 +94,11 @@ func (c *fsContext) SetBranch(b string) { func (c *fsContext) Remotes() (Remotes, error) { if c.remotes == nil { - rem, err := parseRemotes() + gitRemotes, err := git.Remotes() + if err != nil { + return nil, err + } + rem, err := parseRemotes(gitRemotes) if err != nil { return nil, err } diff --git a/context/remote.go b/context/remote.go index d729bcbae..caa7ee10d 100644 --- a/context/remote.go +++ b/context/remote.go @@ -42,14 +42,9 @@ type GitHubRepository struct { Owner string } -func parseRemotes() (remotes Remotes, err error) { +func parseRemotes(gitRemotes []string) (remotes Remotes, err error) { re := regexp.MustCompile(`(.+)\s+(.+)\s+\((push|fetch)\)`) - gitRemotes, err := git.Remotes() - if err != nil { - return - } - remotesMap := make(map[string]map[string]string) for _, r := range gitRemotes { if re.MatchString(r) { diff --git a/context/remote_test.go b/context/remote_test.go index 7c23a398f..d9a6b30f6 100644 --- a/context/remote_test.go +++ b/context/remote_test.go @@ -16,3 +16,41 @@ func Test_repoFromURL(t *testing.T) { t.Errorf("got Name: %q", r.Name) } } + +func Test_parseRemotes(t *testing.T) { + remoteList := []string{ + "mona\tgit@github.com:monalisa/myfork.git (fetch)", + "origin\thttps://github.com/monalisa/octo-cat.git (fetch)", + "origin\thttps://github.com/monalisa/octo-cat-push.git (push)", + "upstream\thttps://example.com/nowhere.git (fetch)", + "upstream\thttps://github.com/hubot/tools (push)", + } + r, err := parseRemotes(remoteList) + if err != nil { + t.Error(err) + } + + mona, err := r.FindByName("mona") + if err != nil { + t.Error(err) + } + if mona.Owner != "monalisa" || mona.Repo != "myfork" { + t.Errorf("got %s/%s", mona.Owner, mona.Repo) + } + + origin, err := r.FindByName("origin") + if err != nil { + t.Error(err) + } + if origin.Owner != "monalisa" || origin.Repo != "octo-cat" { + t.Errorf("got %s/%s", origin.Owner, origin.Repo) + } + + upstream, err := r.FindByName("upstream") + if err != nil { + t.Error(err) + } + if upstream.Owner != "hubot" || upstream.Repo != "tools" { + t.Errorf("got %s/%s", upstream.Owner, upstream.Repo) + } +} From 63f40263672c18a236e594e61bf84e9631ddaa82 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Thu, 17 Oct 2019 01:52:08 +0200 Subject: [PATCH 14/22] :fire: github package --- github/client.go | 290 ----------------------- github/config.go | 397 ------------------------------- github/config_decoder.go | 61 ----- github/config_encoder.go | 52 ----- github/config_service.go | 43 ---- github/hosts.go | 64 ----- github/http.go | 490 --------------------------------------- test/helpers.go | 3 - 8 files changed, 1400 deletions(-) delete mode 100644 github/client.go delete mode 100644 github/config.go delete mode 100644 github/config_decoder.go delete mode 100644 github/config_encoder.go delete mode 100644 github/config_service.go delete mode 100644 github/hosts.go delete mode 100644 github/http.go diff --git a/github/client.go b/github/client.go deleted file mode 100644 index 6b90da1a6..000000000 --- a/github/client.go +++ /dev/null @@ -1,290 +0,0 @@ -package github - -import ( - "fmt" - "net/http" - "net/url" - "os" - "os/exec" - "strings" - - "github.com/github/gh-cli/version" -) - -const ( - GitHubHost string = "github.com" - OAuthAppURL string = "https://github.com/github/gh-cli" -) - -var userAgent = "GitHub CLI " + version.Version - -func NewClient(h string) *Client { - return NewClientWithHost(&Host{Host: h}) -} - -func NewClientWithHost(host *Host) *Client { - return &Client{Host: host} -} - -type Client struct { - Host *Host - cachedClient *simpleClient -} - -type User struct { - Login string `json:"login"` -} - -func (client *Client) CurrentUser() (user *User, err error) { - api, err := client.simpleApi() - if err != nil { - return - } - - res, err := api.Get("user") - if err = checkStatus(200, "getting current user", res, err); err != nil { - return - } - - user = &User{} - err = res.Unmarshal(user) - return -} - -type AuthorizationEntry struct { - Token string `json:"token"` -} - -func isToken(api *simpleClient, password string) bool { - api.PrepareRequest = func(req *http.Request) { - req.Header.Set("Authorization", "token "+password) - } - - res, _ := api.Get("user") - if res != nil && res.StatusCode == 200 { - return true - } - return false -} - -func (client *Client) FindOrCreateToken(user, password, twoFactorCode string) (token string, err error) { - api := client.apiClient() - - if len(password) >= 40 && isToken(api, password) { - return password, nil - } - - params := map[string]interface{}{ - "scopes": []string{"repo"}, - "note_url": OAuthAppURL, - } - - api.PrepareRequest = func(req *http.Request) { - req.SetBasicAuth(user, password) - if twoFactorCode != "" { - req.Header.Set("X-GitHub-OTP", twoFactorCode) - } - } - - count := 1 - maxTries := 9 - for { - params["note"], err = authTokenNote(count) - if err != nil { - return - } - - res, postErr := api.PostJSON("authorizations", params) - if postErr != nil { - err = postErr - break - } - - if res.StatusCode == 201 { - auth := &AuthorizationEntry{} - if err = res.Unmarshal(auth); err != nil { - return - } - token = auth.Token - break - } else if res.StatusCode == 422 && count < maxTries { - count++ - } else { - errInfo, e := res.ErrorInfo() - if e == nil { - err = errInfo - } else { - err = e - } - return - } - } - - return -} - -func (client *Client) ensureAccessToken() error { - if client.Host.AccessToken == "" { - host, err := CurrentConfig().PromptForHost(client.Host.Host) - if err != nil { - return err - } - client.Host = host - } - return nil -} - -func (client *Client) simpleApi() (c *simpleClient, err error) { - err = client.ensureAccessToken() - if err != nil { - return - } - - if client.cachedClient != nil { - c = client.cachedClient - return - } - - c = client.apiClient() - c.PrepareRequest = func(req *http.Request) { - clientDomain := normalizeHost(client.Host.Host) - if strings.HasPrefix(clientDomain, "api.github.") { - clientDomain = strings.TrimPrefix(clientDomain, "api.") - } - requestHost := strings.ToLower(req.URL.Host) - if requestHost == clientDomain || strings.HasSuffix(requestHost, "."+clientDomain) { - req.Header.Set("Authorization", "token "+client.Host.AccessToken) - } - } - - client.cachedClient = c - return -} - -func (client *Client) apiClient() *simpleClient { - unixSocket := os.ExpandEnv(client.Host.UnixSocket) - httpClient := newHttpClient(os.Getenv("HUB_TEST_HOST"), os.Getenv("HUB_VERBOSE") != "", unixSocket) - apiRoot := client.absolute(normalizeHost(client.Host.Host)) - if !strings.HasPrefix(apiRoot.Host, "api.github.") { - apiRoot.Path = "/api/v3/" - } - - return &simpleClient{ - httpClient: httpClient, - rootUrl: apiRoot, - } -} - -func (client *Client) absolute(host string) *url.URL { - u, err := url.Parse("https://" + host + "/") - if err != nil { - panic(err) - } else if client.Host != nil && client.Host.Protocol != "" { - u.Scheme = client.Host.Protocol - } - return u -} - -func normalizeHost(host string) string { - if host == "" { - return GitHubHost - } else if strings.EqualFold(host, GitHubHost) { - return "api.github.com" - } else if strings.EqualFold(host, "github.localhost") { - return "api.github.localhost" - } else { - return strings.ToLower(host) - } -} - -func checkStatus(expectedStatus int, action string, response *simpleResponse, err error) error { - if err != nil { - return fmt.Errorf("Error %s: %s", action, err.Error()) - } else if response.StatusCode != expectedStatus { - errInfo, err := response.ErrorInfo() - if err == nil { - return FormatError(action, errInfo) - } else { - return fmt.Errorf("Error %s: %s (HTTP %d)", action, err.Error(), response.StatusCode) - } - } else { - return nil - } -} - -func FormatError(action string, err error) (ee error) { - switch e := err.(type) { - default: - ee = err - case *errorInfo: - statusCode := e.Response.StatusCode - var reason string - if s := strings.SplitN(e.Response.Status, " ", 2); len(s) >= 2 { - reason = strings.TrimSpace(s[1]) - } - - errStr := fmt.Sprintf("Error %s: %s (HTTP %d)", action, reason, statusCode) - - var errorSentences []string - for _, err := range e.Errors { - switch err.Code { - case "custom": - errorSentences = append(errorSentences, err.Message) - case "missing_field": - errorSentences = append(errorSentences, fmt.Sprintf("Missing field: \"%s\"", err.Field)) - case "already_exists": - errorSentences = append(errorSentences, fmt.Sprintf("Duplicate value for \"%s\"", err.Field)) - case "invalid": - errorSentences = append(errorSentences, fmt.Sprintf("Invalid value for \"%s\"", err.Field)) - case "unauthorized": - errorSentences = append(errorSentences, fmt.Sprintf("Not allowed to change field \"%s\"", err.Field)) - } - } - - var errorMessage string - if len(errorSentences) > 0 { - errorMessage = strings.Join(errorSentences, "\n") - } else { - errorMessage = e.Message - if action == "getting current user" && e.Message == "Resource not accessible by integration" { - errorMessage = errorMessage + "\nYou must specify GITHUB_USER via environment variable." - } - } - - if errorMessage != "" { - errStr = fmt.Sprintf("%s\n%s", errStr, errorMessage) - } - - ee = fmt.Errorf(errStr) - } - - return -} - -func authTokenNote(num int) (string, error) { - n := os.Getenv("USER") - - if n == "" { - n = os.Getenv("USERNAME") - } - - if n == "" { - whoami := exec.Command("whoami") - whoamiOut, err := whoami.Output() - if err != nil { - return "", err - } - n = strings.TrimSpace(string(whoamiOut)) - } - - h, err := os.Hostname() - if err != nil { - return "", err - } - - if num > 1 { - return fmt.Sprintf("hub for %s@%s %d", n, h, num), nil - } - - return fmt.Sprintf("hub for %s@%s", n, h), nil -} diff --git a/github/config.go b/github/config.go deleted file mode 100644 index 420f56c19..000000000 --- a/github/config.go +++ /dev/null @@ -1,397 +0,0 @@ -package github - -import ( - "bufio" - "fmt" - "io/ioutil" - "net/url" - "os" - "os/signal" - "path/filepath" - "strconv" - "strings" - "syscall" - - "github.com/github/gh-cli/ui" - "github.com/github/gh-cli/utils" - "github.com/mitchellh/go-homedir" - "golang.org/x/crypto/ssh/terminal" -) - -type yamlHost struct { - User string `yaml:"user"` - OAuthToken string `yaml:"oauth_token"` - Protocol string `yaml:"protocol"` - UnixSocket string `yaml:"unix_socket,omitempty"` -} - -type Host struct { - Host string `toml:"host"` - User string `toml:"user"` - AccessToken string `toml:"access_token"` - Protocol string `toml:"protocol"` - UnixSocket string `toml:"unix_socket,omitempty"` -} - -type Config struct { - Hosts []*Host `toml:"hosts"` -} - -func (c *Config) PromptForHost(host string) (h *Host, err error) { - token := c.DetectToken() - tokenFromEnv := token != "" - - if host != GitHubHost { - if _, e := url.Parse("https://" + host); e != nil { - err = fmt.Errorf("invalid hostname: %q", host) - return - } - } - - h = c.Find(host) - if h != nil { - if h.User == "" { - utils.Check(CheckWriteable(configsFile())) - // User is missing from the config: this is a broken config probably - // because it was created with an old (broken) version of hub. Let's fix - // it now. See issue #1007 for details. - user := c.PromptForUser(host) - if user == "" { - utils.Check(fmt.Errorf("missing user")) - } - h.User = user - err := newConfigService().Save(configsFile(), c) - utils.Check(err) - } - if tokenFromEnv { - h.AccessToken = token - } else { - return - } - } else { - h = &Host{ - Host: host, - AccessToken: token, - Protocol: "https", - } - c.Hosts = append(c.Hosts, h) - } - - client := NewClientWithHost(h) - - if !tokenFromEnv { - utils.Check(CheckWriteable(configsFile())) - err = c.authorizeClient(client, host) - if err != nil { - return - } - } - - userFromEnv := os.Getenv("GITHUB_USER") - repoFromEnv := os.Getenv("GITHUB_REPOSITORY") - if userFromEnv == "" && repoFromEnv != "" { - repoParts := strings.SplitN(repoFromEnv, "/", 2) - if len(repoParts) > 0 { - userFromEnv = repoParts[0] - } - } - if tokenFromEnv && userFromEnv != "" { - h.User = userFromEnv - } else { - var currentUser *User - currentUser, err = client.CurrentUser() - if err != nil { - return - } - h.User = currentUser.Login - } - - if !tokenFromEnv { - err = newConfigService().Save(configsFile(), c) - } - - return -} - -func (c *Config) authorizeClient(client *Client, host string) (err error) { - user := c.PromptForUser(host) - pass := c.PromptForPassword(host, user) - - var code, token string - for { - token, err = client.FindOrCreateToken(user, pass, code) - if err == nil { - break - } - - if ae, ok := err.(*errorInfo); ok && strings.HasPrefix(ae.Response.Header.Get("X-GitHub-OTP"), "required;") { - if code != "" { - ui.Errorln("warning: invalid two-factor code") - } - code = c.PromptForOTP() - } else { - break - } - } - - if err == nil { - client.Host.AccessToken = token - } - - return -} - -func (c *Config) DetectToken() string { - return os.Getenv("GITHUB_TOKEN") -} - -func (c *Config) PromptForUser(host string) (user string) { - user = os.Getenv("GITHUB_USER") - if user != "" { - return - } - - ui.Printf("%s username: ", host) - user = c.scanLine() - - return -} - -func (c *Config) PromptForPassword(host, user string) (pass string) { - pass = os.Getenv("GITHUB_PASSWORD") - if pass != "" { - return - } - - ui.Printf("%s password for %s (never stored): ", host, user) - if ui.IsTerminal(os.Stdin) { - if password, err := getPassword(); err == nil { - pass = password - } - } else { - pass = c.scanLine() - } - - return -} - -func (c *Config) PromptForOTP() string { - fmt.Print("two-factor authentication code: ") - return c.scanLine() -} - -func (c *Config) scanLine() string { - var line string - scanner := bufio.NewScanner(os.Stdin) - if scanner.Scan() { - line = scanner.Text() - } - utils.Check(scanner.Err()) - - return line -} - -func getPassword() (string, error) { - stdin := int(syscall.Stdin) - initialTermState, err := terminal.GetState(stdin) - if err != nil { - return "", err - } - - c := make(chan os.Signal) - signal.Notify(c, os.Interrupt, syscall.SIGTERM) - go func() { - s := <-c - terminal.Restore(stdin, initialTermState) - switch sig := s.(type) { - case syscall.Signal: - if int(sig) == 2 { - fmt.Println("^C") - } - } - os.Exit(1) - }() - - passBytes, err := terminal.ReadPassword(stdin) - if err != nil { - return "", err - } - - signal.Stop(c) - fmt.Print("\n") - return string(passBytes), nil -} - -func (c *Config) Find(host string) *Host { - for _, h := range c.Hosts { - if h.Host == host { - return h - } - } - - return nil -} - -func (c *Config) selectHost() *Host { - options := len(c.Hosts) - - if options == 1 { - return c.Hosts[0] - } - - prompt := "Select host:\n" - for idx, host := range c.Hosts { - prompt += fmt.Sprintf(" %d. %s\n", idx+1, host.Host) - } - prompt += fmt.Sprint("> ") - - ui.Printf(prompt) - index := c.scanLine() - i, err := strconv.Atoi(index) - if err != nil || i < 1 || i > options { - utils.Check(fmt.Errorf("Error: must enter a number [1-%d]", options)) - } - - return c.Hosts[i-1] -} - -var defaultConfigsFile string - -func configsFile() string { - if configFromEnv := os.Getenv("HUB_CONFIG"); configFromEnv != "" { - return configFromEnv - } - if defaultConfigsFile == "" { - var err error - defaultConfigsFile, err = determineConfigLocation() - utils.Check(err) - } - return defaultConfigsFile -} - -func homeConfig() (string, error) { - if home, err := homedir.Dir(); err != nil { - return "", err - } else { - return filepath.Join(home, ".config"), nil - } -} - -func determineConfigLocation() (string, error) { - var err error - - xdgHome := os.Getenv("XDG_CONFIG_HOME") - configDir := xdgHome - if configDir == "" { - if configDir, err = homeConfig(); err != nil { - return "", err - } - } - - xdgDirs := os.Getenv("XDG_CONFIG_DIRS") - if xdgDirs == "" { - xdgDirs = "/etc/xdg" - } - searchDirs := append([]string{configDir}, strings.Split(xdgDirs, ":")...) - - for _, dir := range searchDirs { - filename := filepath.Join(dir, "hub") - if _, err := os.Stat(filename); err == nil { - return filename, nil - } - } - - configFile := filepath.Join(configDir, "hub") - - if configDir == xdgHome { - if homeDir, _ := homeConfig(); homeDir != "" { - legacyConfig := filepath.Join(homeDir, "hub") - if _, err = os.Stat(legacyConfig); err == nil { - ui.Errorf("Notice: config file found but not respected at: %s\n", legacyConfig) - ui.Errorf("You might want to move it to `%s' to avoid re-authenticating.\n", configFile) - } - } - } - - return configFile, nil -} - -var currentConfig *Config -var configLoadedFrom = "" - -func CurrentConfig() *Config { - filename := configsFile() - if configLoadedFrom != filename { - currentConfig = &Config{} - newConfigService().Load(filename, currentConfig) - configLoadedFrom = filename - } - - return currentConfig -} - -func (c *Config) DefaultHost() (host *Host, err error) { - if GitHubHostEnv != "" { - host, err = c.PromptForHost(GitHubHostEnv) - } else if len(c.Hosts) > 0 { - host = c.selectHost() - // HACK: forces host to inherit GITHUB_TOKEN if applicable - host, err = c.PromptForHost(host.Host) - } else { - host, err = c.PromptForHost(DefaultGitHubHost()) - } - - return -} - -// CheckWriteable checks if config file is writeable. This should -// be called before asking for credentials and only if current -// operation needs to update the file. See issue #1314 for details. -func CheckWriteable(filename string) error { - // Check if file exists already. if it doesn't, we will delete it after - // checking for writeabilty - fileExistsAlready := false - - if _, err := os.Stat(filename); err == nil { - fileExistsAlready = true - } - - err := os.MkdirAll(filepath.Dir(filename), 0771) - if err != nil { - return err - } - - w, err := os.OpenFile(filename, os.O_WRONLY|os.O_CREATE|os.O_APPEND, 0600) - if err != nil { - return err - } - w.Close() - - if !fileExistsAlready { - err := os.Remove(filename) - if err != nil { - return err - } - } - return nil -} - -// Public for testing purpose -func CreateTestConfigs(user, token string) *Config { - f, _ := ioutil.TempFile("", "test-config") - os.Setenv("HUB_CONFIG", f.Name()) - - host := &Host{ - User: "jingweno", - AccessToken: "123", - Host: GitHubHost, - } - - c := &Config{Hosts: []*Host{host}} - err := newConfigService().Save(f.Name(), c) - if err != nil { - panic(err) - } - - return c -} diff --git a/github/config_decoder.go b/github/config_decoder.go deleted file mode 100644 index 7e467b761..000000000 --- a/github/config_decoder.go +++ /dev/null @@ -1,61 +0,0 @@ -package github - -import ( - "io" - "io/ioutil" - - "github.com/BurntSushi/toml" - "gopkg.in/yaml.v2" -) - -type configDecoder interface { - Decode(r io.Reader, c *Config) error -} - -type tomlConfigDecoder struct { -} - -func (t *tomlConfigDecoder) Decode(r io.Reader, c *Config) error { - _, err := toml.DecodeReader(r, c) - return err -} - -type yamlConfigDecoder struct { -} - -func (y *yamlConfigDecoder) Decode(r io.Reader, c *Config) error { - d, err := ioutil.ReadAll(r) - if err != nil { - return err - } - - yc := yaml.MapSlice{} - err = yaml.Unmarshal(d, &yc) - - if err != nil { - return err - } - - for _, hostEntry := range yc { - v := hostEntry.Value.([]interface{}) - if len(v) < 1 { - continue - } - host := &Host{Host: hostEntry.Key.(string)} - for _, prop := range v[0].(yaml.MapSlice) { - switch prop.Key.(string) { - case "user": - host.User = prop.Value.(string) - case "oauth_token": - host.AccessToken = prop.Value.(string) - case "protocol": - host.Protocol = prop.Value.(string) - case "unix_socket": - host.UnixSocket = prop.Value.(string) - } - } - c.Hosts = append(c.Hosts, host) - } - - return nil -} diff --git a/github/config_encoder.go b/github/config_encoder.go deleted file mode 100644 index 87609d38d..000000000 --- a/github/config_encoder.go +++ /dev/null @@ -1,52 +0,0 @@ -package github - -import ( - "io" - - "github.com/BurntSushi/toml" - "gopkg.in/yaml.v2" -) - -type configEncoder interface { - Encode(w io.Writer, c *Config) error -} - -type tomlConfigEncoder struct { -} - -func (t *tomlConfigEncoder) Encode(w io.Writer, c *Config) error { - enc := toml.NewEncoder(w) - return enc.Encode(c) -} - -type yamlConfigEncoder struct { -} - -func (y *yamlConfigEncoder) Encode(w io.Writer, c *Config) error { - yc := yaml.MapSlice{} - for _, h := range c.Hosts { - yc = append(yc, yaml.MapItem{ - Key: h.Host, - Value: []yamlHost{ - { - User: h.User, - OAuthToken: h.AccessToken, - Protocol: h.Protocol, - UnixSocket: h.UnixSocket, - }, - }, - }) - } - - d, err := yaml.Marshal(yc) - if err != nil { - return err - } - - n, err := w.Write(d) - if err == nil && n < len(d) { - err = io.ErrShortWrite - } - - return err -} diff --git a/github/config_service.go b/github/config_service.go deleted file mode 100644 index 3aaa6915f..000000000 --- a/github/config_service.go +++ /dev/null @@ -1,43 +0,0 @@ -package github - -import ( - "os" - "path/filepath" -) - -func newConfigService() *configService { - return &configService{ - Encoder: &yamlConfigEncoder{}, - Decoder: &yamlConfigDecoder{}, - } -} - -type configService struct { - Encoder configEncoder - Decoder configDecoder -} - -func (s *configService) Save(filename string, c *Config) error { - err := os.MkdirAll(filepath.Dir(filename), 0771) - if err != nil { - return err - } - - w, err := os.OpenFile(filename, os.O_RDWR|os.O_CREATE|os.O_TRUNC, 0600) - if err != nil { - return err - } - defer w.Close() - - return s.Encoder.Encode(w, c) -} - -func (s *configService) Load(filename string, c *Config) error { - r, err := os.Open(filename) - if err != nil { - return err - } - defer r.Close() - - return s.Decoder.Decode(r, c) -} diff --git a/github/hosts.go b/github/hosts.go deleted file mode 100644 index a74d9810c..000000000 --- a/github/hosts.go +++ /dev/null @@ -1,64 +0,0 @@ -package github - -import ( - "fmt" - "net/url" - "os" - "strings" - - "github.com/github/gh-cli/git" -) - -var ( - GitHubHostEnv = os.Getenv("GITHUB_HOST") - cachedHosts []string -) - -type GithubHostError struct { - url *url.URL -} - -func (e *GithubHostError) Error() string { - return fmt.Sprintf("Invalid GitHub URL: %s", e.url) -} - -func KnownGitHubHostsInclude(host string) bool { - for _, hh := range knownGitHubHosts() { - if hh == host { - return true - } - } - - return false -} - -func knownGitHubHosts() []string { - if cachedHosts != nil { - return cachedHosts - } - - hosts := []string{} - defaultHost := DefaultGitHubHost() - hosts = append(hosts, defaultHost) - hosts = append(hosts, "ssh.github.com") - - ghHosts, _ := git.ConfigAll("hub.host") - for _, ghHost := range ghHosts { - ghHost = strings.TrimSpace(ghHost) - if ghHost != "" { - hosts = append(hosts, ghHost) - } - } - - cachedHosts = hosts - return hosts -} - -func DefaultGitHubHost() string { - defaultHost := GitHubHostEnv - if defaultHost == "" { - defaultHost = GitHubHost - } - - return defaultHost -} diff --git a/github/http.go b/github/http.go deleted file mode 100644 index 25abcb351..000000000 --- a/github/http.go +++ /dev/null @@ -1,490 +0,0 @@ -package github - -import ( - "bytes" - "context" - "crypto/md5" - "encoding/json" - "fmt" - "io" - "io/ioutil" - "net" - "net/http" - "net/url" - "os" - "path" - "path/filepath" - "regexp" - "sort" - "strconv" - "strings" - "time" - - "github.com/github/gh-cli/ui" - "github.com/github/gh-cli/utils" -) - -const apiPayloadVersion = "application/vnd.github.v3+json;charset=utf-8" -const patchMediaType = "application/vnd.github.v3.patch;charset=utf-8" -const textMediaType = "text/plain;charset=utf-8" -const checksType = "application/vnd.github.antiope-preview+json;charset=utf-8" -const draftsType = "application/vnd.github.shadow-cat-preview+json;charset=utf-8" -const cacheVersion = 2 - -var inspectHeaders = []string{ - "Authorization", - "X-GitHub-OTP", - "Location", - "Link", - "Accept", -} - -type verboseTransport struct { - Transport *http.Transport - Verbose bool - OverrideURL *url.URL - Out io.Writer - Colorized bool -} - -func (t *verboseTransport) RoundTrip(req *http.Request) (resp *http.Response, err error) { - if t.Verbose { - t.dumpRequest(req) - } - - if t.OverrideURL != nil { - port := "80" - if s := strings.Split(req.URL.Host, ":"); len(s) > 1 { - port = s[1] - } - - req = cloneRequest(req) - req.Header.Set("X-Original-Scheme", req.URL.Scheme) - req.Header.Set("X-Original-Port", port) - req.Host = req.URL.Host - req.URL.Scheme = t.OverrideURL.Scheme - req.URL.Host = t.OverrideURL.Host - } - - resp, err = t.Transport.RoundTrip(req) - - if err == nil && t.Verbose { - t.dumpResponse(resp) - } - - return -} - -func (t *verboseTransport) dumpRequest(req *http.Request) { - info := fmt.Sprintf("> %s %s://%s%s", req.Method, req.URL.Scheme, req.URL.Host, req.URL.RequestURI()) - t.verbosePrintln(info) - t.dumpHeaders(req.Header, ">") - body := t.dumpBody(req.Body) - if body != nil { - // reset body since it's been read - req.Body = body - } -} - -func (t *verboseTransport) dumpResponse(resp *http.Response) { - info := fmt.Sprintf("< HTTP %d", resp.StatusCode) - t.verbosePrintln(info) - t.dumpHeaders(resp.Header, "<") - body := t.dumpBody(resp.Body) - if body != nil { - // reset body since it's been read - resp.Body = body - } -} - -func (t *verboseTransport) dumpHeaders(header http.Header, indent string) { - for _, listed := range inspectHeaders { - for name, vv := range header { - if !strings.EqualFold(name, listed) { - continue - } - for _, v := range vv { - if v != "" { - r := regexp.MustCompile("(?i)^(basic|token) (.+)") - if r.MatchString(v) { - v = r.ReplaceAllString(v, "$1 [REDACTED]") - } - - info := fmt.Sprintf("%s %s: %s", indent, name, v) - t.verbosePrintln(info) - } - } - } - } -} - -func (t *verboseTransport) dumpBody(body io.ReadCloser) io.ReadCloser { - if body == nil { - return nil - } - - defer body.Close() - buf := new(bytes.Buffer) - _, err := io.Copy(buf, body) - utils.Check(err) - - if buf.Len() > 0 { - t.verbosePrintln(buf.String()) - } - - return ioutil.NopCloser(buf) -} - -func (t *verboseTransport) verbosePrintln(msg string) { - if t.Colorized { - msg = fmt.Sprintf("\033[36m%s\033[0m", msg) - } - - fmt.Fprintln(t.Out, msg) -} - -func newHttpClient(testHost string, verbose bool, unixSocket string) *http.Client { - var testURL *url.URL - if testHost != "" { - testURL, _ = url.Parse(testHost) - } - var httpTransport *http.Transport - if unixSocket != "" { - dialFunc := func(network, addr string) (net.Conn, error) { - return net.Dial("unix", unixSocket) - } - dialContext := func(_ context.Context, _, _ string) (net.Conn, error) { - return net.Dial("unix", unixSocket) - } - httpTransport = &http.Transport{ - DialContext: dialContext, - DialTLS: dialFunc, - ResponseHeaderTimeout: 30 * time.Second, - ExpectContinueTimeout: 10 * time.Second, - TLSHandshakeTimeout: 10 * time.Second, - } - } else { - httpTransport = &http.Transport{ - DialContext: (&net.Dialer{ - Timeout: 30 * time.Second, - KeepAlive: 30 * time.Second, - }).DialContext, - TLSHandshakeTimeout: 10 * time.Second, - } - } - tr := &verboseTransport{ - Transport: httpTransport, - Verbose: verbose, - OverrideURL: testURL, - Out: ui.Stderr, - Colorized: ui.IsTerminal(os.Stderr), - } - - return &http.Client{ - Transport: tr, - } -} - -func cloneRequest(req *http.Request) *http.Request { - dup := new(http.Request) - *dup = *req - dup.URL, _ = url.Parse(req.URL.String()) - dup.Header = make(http.Header) - for k, s := range req.Header { - dup.Header[k] = s - } - return dup -} - -type simpleClient struct { - httpClient *http.Client - rootUrl *url.URL - PrepareRequest func(*http.Request) - CacheTTL int -} - -func (c *simpleClient) performRequest(method, path string, body io.Reader, configure func(*http.Request)) (*simpleResponse, error) { - url, err := url.Parse(path) - if err == nil { - url = c.rootUrl.ResolveReference(url) - return c.performRequestUrl(method, url, body, configure) - } else { - return nil, err - } -} - -func (c *simpleClient) performRequestUrl(method string, url *url.URL, body io.Reader, configure func(*http.Request)) (res *simpleResponse, err error) { - req, err := http.NewRequest(method, url.String(), body) - if err != nil { - return - } - if c.PrepareRequest != nil { - c.PrepareRequest(req) - } - req.Header.Set("User-Agent", userAgent) - req.Header.Set("Accept", apiPayloadVersion) - - if configure != nil { - configure(req) - } - - key := cacheKey(req) - if cachedResponse := c.cacheRead(key, req); cachedResponse != nil { - res = &simpleResponse{cachedResponse} - return - } - - httpResponse, err := c.httpClient.Do(req) - if err != nil { - return - } - - c.cacheWrite(key, httpResponse) - res = &simpleResponse{httpResponse} - - return -} - -func isGraphQL(req *http.Request) bool { - return req.URL.Path == "/graphql" -} - -func canCache(req *http.Request) bool { - return strings.EqualFold(req.Method, "GET") || isGraphQL(req) -} - -func (c *simpleClient) cacheRead(key string, req *http.Request) (res *http.Response) { - if c.CacheTTL > 0 && canCache(req) { - f := cacheFile(key) - cacheInfo, err := os.Stat(f) - if err != nil { - return - } - if time.Since(cacheInfo.ModTime()).Seconds() > float64(c.CacheTTL) { - return - } - cf, err := os.Open(f) - if err != nil { - return - } - defer cf.Close() - - cb, err := ioutil.ReadAll(cf) - if err != nil { - return - } - parts := strings.SplitN(string(cb), "\r\n\r\n", 2) - if len(parts) < 2 { - return - } - - res = &http.Response{ - Body: ioutil.NopCloser(bytes.NewBufferString(parts[1])), - Header: http.Header{}, - } - headerLines := strings.Split(parts[0], "\r\n") - if len(headerLines) < 1 { - return - } - if proto := strings.SplitN(headerLines[0], " ", 3); len(proto) >= 3 { - res.Proto = proto[0] - res.Status = fmt.Sprintf("%s %s", proto[1], proto[2]) - if code, _ := strconv.Atoi(proto[1]); code > 0 { - res.StatusCode = code - } - } - for _, line := range headerLines[1:] { - kv := strings.SplitN(line, ":", 2) - if len(kv) >= 2 { - res.Header.Add(kv[0], strings.TrimLeft(kv[1], " ")) - } - } - } - return -} - -func (c *simpleClient) cacheWrite(key string, res *http.Response) { - if c.CacheTTL > 0 && canCache(res.Request) && res.StatusCode < 500 && res.StatusCode != 403 { - bodyCopy := &bytes.Buffer{} - bodyReplacement := readCloserCallback{ - Reader: io.TeeReader(res.Body, bodyCopy), - Closer: res.Body, - Callback: func() { - f := cacheFile(key) - err := os.MkdirAll(filepath.Dir(f), 0771) - if err != nil { - return - } - cf, err := os.OpenFile(f, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0600) - if err != nil { - return - } - defer cf.Close() - fmt.Fprintf(cf, "%s %s\r\n", res.Proto, res.Status) - res.Header.Write(cf) - fmt.Fprintf(cf, "\r\n") - io.Copy(cf, bodyCopy) - }, - } - res.Body = &bodyReplacement - } -} - -type readCloserCallback struct { - Callback func() - Closer io.Closer - io.Reader -} - -func (rc *readCloserCallback) Close() error { - err := rc.Closer.Close() - if err == nil { - rc.Callback() - } - return err -} - -func cacheKey(req *http.Request) string { - path := strings.Replace(req.URL.EscapedPath(), "/", "-", -1) - if len(path) > 1 { - path = strings.TrimPrefix(path, "-") - } - host := req.Host - if host == "" { - host = req.URL.Host - } - hash := md5.New() - fmt.Fprintf(hash, "%d:", cacheVersion) - io.WriteString(hash, req.Header.Get("Accept")) - io.WriteString(hash, req.Header.Get("Authorization")) - queryParts := strings.Split(req.URL.RawQuery, "&") - sort.Strings(queryParts) - for _, q := range queryParts { - fmt.Fprintf(hash, "%s&", q) - } - if isGraphQL(req) && req.Body != nil { - if b, err := ioutil.ReadAll(req.Body); err == nil { - req.Body = ioutil.NopCloser(bytes.NewBuffer(b)) - hash.Write(b) - } - } - return fmt.Sprintf("%s/%s_%x", host, path, hash.Sum(nil)) -} - -func cacheFile(key string) string { - return path.Join(os.TempDir(), "hub", "api", key) -} - -func (c *simpleClient) jsonRequest(method, path string, body interface{}, configure func(*http.Request)) (*simpleResponse, error) { - json, err := json.Marshal(body) - if err != nil { - return nil, err - } - buf := bytes.NewBuffer(json) - - return c.performRequest(method, path, buf, func(req *http.Request) { - req.Header.Set("Content-Type", "application/json; charset=utf-8") - if configure != nil { - configure(req) - } - }) -} - -func (c *simpleClient) Get(path string) (*simpleResponse, error) { - return c.performRequest("GET", path, nil, nil) -} - -func (c *simpleClient) GetFile(path string, mimeType string) (*simpleResponse, error) { - return c.performRequest("GET", path, nil, func(req *http.Request) { - req.Header.Set("Accept", mimeType) - }) -} - -func (c *simpleClient) PostJSON(path string, payload interface{}) (*simpleResponse, error) { - return c.jsonRequest("POST", path, payload, nil) -} - -func (c *simpleClient) PostJSONPreview(path string, payload interface{}, mimeType string) (*simpleResponse, error) { - return c.jsonRequest("POST", path, payload, func(req *http.Request) { - req.Header.Set("Accept", mimeType) - }) -} - -func (c *simpleClient) PatchJSON(path string, payload interface{}) (*simpleResponse, error) { - return c.jsonRequest("PATCH", path, payload, nil) -} - -type simpleResponse struct { - *http.Response -} - -type errorInfo struct { - Message string `json:"message"` - Errors []fieldError `json:"errors"` - Response *http.Response -} -type errorInfoSimple struct { - Message string `json:"message"` - Errors []string `json:"errors"` -} -type fieldError struct { - Resource string `json:"resource"` - Message string `json:"message"` - Code string `json:"code"` - Field string `json:"field"` -} - -func (e *errorInfo) Error() string { - return e.Message -} - -func (res *simpleResponse) Unmarshal(dest interface{}) (err error) { - defer res.Body.Close() - - body, err := ioutil.ReadAll(res.Body) - if err != nil { - return - } - - return json.Unmarshal(body, dest) -} - -func (res *simpleResponse) ErrorInfo() (msg *errorInfo, err error) { - defer res.Body.Close() - - body, err := ioutil.ReadAll(res.Body) - if err != nil { - return - } - - msg = &errorInfo{} - err = json.Unmarshal(body, msg) - if err != nil { - msgSimple := &errorInfoSimple{} - if err = json.Unmarshal(body, msgSimple); err == nil { - msg.Message = msgSimple.Message - for _, errMsg := range msgSimple.Errors { - msg.Errors = append(msg.Errors, fieldError{ - Code: "custom", - Message: errMsg, - }) - } - } - } - if err == nil { - msg.Response = res.Response - } - - return -} - -func (res *simpleResponse) Link(name string) string { - linkVal := res.Header.Get("Link") - re := regexp.MustCompile(`<([^>]+)>; rel="([^"]+)"`) - for _, match := range re.FindAllStringSubmatch(linkVal, -1) { - if match[2] == name { - return match[1] - } - } - return "" -} diff --git a/test/helpers.go b/test/helpers.go index ce2de88df..90b16778a 100644 --- a/test/helpers.go +++ b/test/helpers.go @@ -10,7 +10,6 @@ import ( "strings" "github.com/github/gh-cli/api" - "github.com/github/gh-cli/github" "github.com/spf13/cobra" ) @@ -20,8 +19,6 @@ type TempGitRepo struct { } func UseTempGitRepo() *TempGitRepo { - github.CreateTestConfigs("mario", "i-love-peach") - pwd, _ := os.Getwd() oldEnv := make(map[string]string) overrideEnv := func(name, value string) { From 25c90ed265acdd48e343219c56325f668811ea5e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Thu, 17 Oct 2019 01:54:12 +0200 Subject: [PATCH 15/22] go mod tidy --- go.mod | 4 +--- go.sum | 8 ++------ 2 files changed, 3 insertions(+), 9 deletions(-) diff --git a/go.mod b/go.mod index dca837862..b80ee1d9a 100644 --- a/go.mod +++ b/go.mod @@ -3,13 +3,11 @@ module github.com/github/gh-cli go 1.13 require ( - github.com/BurntSushi/toml v0.3.1 github.com/gookit/color v1.2.0 github.com/kballard/go-shellquote v0.0.0-20180428030007-95032a82bc51 github.com/mattn/go-colorable v0.1.2 github.com/mattn/go-isatty v0.0.9 github.com/mitchellh/go-homedir v1.1.0 github.com/spf13/cobra v0.0.5 - golang.org/x/crypto v0.0.0-20190926180335-cea2066c6411 - gopkg.in/yaml.v2 v2.2.2 + gopkg.in/yaml.v3 v3.0.0-20191010095647-fc94e3f71652 ) diff --git a/go.sum b/go.sum index 4086387eb..1a7c223fc 100644 --- a/go.sum +++ b/go.sum @@ -44,14 +44,8 @@ github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UV github.com/ugorji/go/codec v0.0.0-20181204163529-d75b2dcb6bc8/go.mod h1:VFNgLljTbGfSG7qAOspJ7OScBnGdDN/yBr0sguwnwf0= github.com/xordataexchange/crypt v0.0.3-0.20170626215501-b2862e3d0a77/go.mod h1:aYKd//L2LvnjZzWKhF00oedf4jCCReLcmhLdhm1A27Q= golang.org/x/crypto v0.0.0-20181203042331-505ab145d0a9/go.mod h1:6SG95UA2DQfeDnfUPMdvaQW0Q7yPrPDi9nlGo2tz2b4= -golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w= -golang.org/x/crypto v0.0.0-20190926180335-cea2066c6411 h1:kuW9k4QvBJpRjC3rxEytsfIYPs8oGY3Jw7iR36h0FIY= -golang.org/x/crypto v0.0.0-20190926180335-cea2066c6411/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI= -golang.org/x/net v0.0.0-20190404232315-eb5bcb51f2a3/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg= golang.org/x/sys v0.0.0-20181205085412-a5c9d58dba9a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= -golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= golang.org/x/sys v0.0.0-20190222072716-a9d3bda3a223/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= -golang.org/x/sys v0.0.0-20190412213103-97732733099d/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20190813064441-fde4db37ae7a h1:aYOabOQFp6Vj6W1F80affTUvO9UxmJRx8K0gsfABByQ= golang.org/x/sys v0.0.0-20190813064441-fde4db37ae7a/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= @@ -59,3 +53,5 @@ gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+ gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/yaml.v2 v2.2.2 h1:ZCJp+EgiOT7lHqUV2J862kp8Qj64Jo6az82+3Td9dZw= gopkg.in/yaml.v2 v2.2.2/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= +gopkg.in/yaml.v3 v3.0.0-20191010095647-fc94e3f71652 h1:VKvJ/mQ4BgCjZUDggYFxTe0qv9jPMHsZPD4Xt91Y5H4= +gopkg.in/yaml.v3 v3.0.0-20191010095647-fc94e3f71652/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= From c09eb742c32295080a48788149984c63f32e73f4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Thu, 17 Oct 2019 13:46:35 +0200 Subject: [PATCH 16/22] Keep original order of remotes --- context/remote.go | 5 ++++- context/remote_test.go | 18 ++++++------------ 2 files changed, 10 insertions(+), 13 deletions(-) diff --git a/context/remote.go b/context/remote.go index caa7ee10d..b17d4771d 100644 --- a/context/remote.go +++ b/context/remote.go @@ -45,6 +45,7 @@ type GitHubRepository struct { func parseRemotes(gitRemotes []string) (remotes Remotes, err error) { re := regexp.MustCompile(`(.+)\s+(.+)\s+\((push|fetch)\)`) + names := []string{} remotesMap := make(map[string]map[string]string) for _, r := range gitRemotes { if re.MatchString(r) { @@ -56,12 +57,14 @@ func parseRemotes(gitRemotes []string) (remotes Remotes, err error) { if !ok { utm = make(map[string]string) remotesMap[name] = utm + names = append(names, name) } utm[urlType] = url } } - for name, urlMap := range remotesMap { + for _, name := range names { + urlMap := remotesMap[name] repo, err := repoFromURL(urlMap["fetch"]) if err != nil { repo, err = repoFromURL(urlMap["push"]) diff --git a/context/remote_test.go b/context/remote_test.go index d9a6b30f6..47c1f63f8 100644 --- a/context/remote_test.go +++ b/context/remote_test.go @@ -29,27 +29,21 @@ func Test_parseRemotes(t *testing.T) { if err != nil { t.Error(err) } - - mona, err := r.FindByName("mona") - if err != nil { - t.Error(err) + if len(r) != 3 { + t.Errorf("found %d remotes", len(r)) } + + mona := r[0] if mona.Owner != "monalisa" || mona.Repo != "myfork" { t.Errorf("got %s/%s", mona.Owner, mona.Repo) } - origin, err := r.FindByName("origin") - if err != nil { - t.Error(err) - } + origin := r[1] if origin.Owner != "monalisa" || origin.Repo != "octo-cat" { t.Errorf("got %s/%s", origin.Owner, origin.Repo) } - upstream, err := r.FindByName("upstream") - if err != nil { - t.Error(err) - } + upstream := r[2] if upstream.Owner != "hubot" || upstream.Repo != "tools" { t.Errorf("got %s/%s", upstream.Owner, upstream.Repo) } From 74c637fec8ab39e5ed992cebd6d223fe85ee4be9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Thu, 17 Oct 2019 14:34:55 +0200 Subject: [PATCH 17/22] Improve config file parsing --- context/config_file.go | 20 +++++++++++----- context/config_file_test.go | 48 ++++++++++++++++++++++++++++++------- 2 files changed, 53 insertions(+), 15 deletions(-) diff --git a/context/config_file.go b/context/config_file.go index 0d5c02ddb..f0a6f4b81 100644 --- a/context/config_file.go +++ b/context/config_file.go @@ -1,6 +1,7 @@ package context import ( + "fmt" "io" "io/ioutil" "os" @@ -32,11 +33,18 @@ func parseConfig(r io.Reader) (*configEntry, error) { if err != nil { return nil, err } - var entries []configEntry - // TODO: this will panic if the config is malformed - err = config.Content[0].Content[1].Decode(&entries) - if err != nil { - return nil, err + if len(config.Content) < 1 { + return nil, fmt.Errorf("malformed config") } - return &entries[0], nil + for i := 0; i < len(config.Content[0].Content)-1; i = i + 2 { + if config.Content[0].Content[i].Value == defaultHostname { + var entries []configEntry + err = config.Content[0].Content[i+1].Decode(&entries) + if err != nil { + return nil, err + } + return &entries[0], nil + } + } + return nil, fmt.Errorf("could not find config entry for %q", defaultHostname) } diff --git a/context/config_file_test.go b/context/config_file_test.go index e519401c8..7163e6b2a 100644 --- a/context/config_file_test.go +++ b/context/config_file_test.go @@ -1,24 +1,54 @@ package context import ( + "errors" + "reflect" "strings" "testing" ) +func eq(t *testing.T, got interface{}, expected interface{}) { + if !reflect.DeepEqual(got, expected) { + t.Errorf("expected: %v, got: %v", expected, got) + } +} + func Test_parseConfig(t *testing.T) { c := strings.NewReader(`--- github.com: - user: monalisa oauth_token: OTOKEN + protocol: https +- user: wronguser + oauth_token: NOTTHIS `) entry, err := parseConfig(c) - if err != nil { - t.Error(err) - } - if entry.User != "monalisa" { - t.Errorf("got User: %q", entry.User) - } - if entry.Token != "OTOKEN" { - t.Errorf("got User: %q", entry.Token) - } + eq(t, err, nil) + eq(t, entry.User, "monalisa") + eq(t, entry.Token, "OTOKEN") +} + +func Test_parseConfig_multipleHosts(t *testing.T) { + c := strings.NewReader(`--- +example.com: +- user: wronguser + oauth_token: NOTTHIS +github.com: +- user: monalisa + oauth_token: OTOKEN +`) + entry, err := parseConfig(c) + eq(t, err, nil) + eq(t, entry.User, "monalisa") + eq(t, entry.Token, "OTOKEN") +} + +func Test_parseConfig_notFound(t *testing.T) { + c := strings.NewReader(`--- +example.com: +- user: wronguser + oauth_token: NOTTHIS +`) + _, err := parseConfig(c) + eq(t, err, errors.New(`could not find config entry for "github.com"`)) } From 79e8766d8f26243f5c49d9dae5562d9122f351af Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Thu, 17 Oct 2019 14:44:53 +0200 Subject: [PATCH 18/22] Use `eq` in more tests --- context/context.go | 6 +----- context/remote.go | 2 +- context/remote_test.go | 37 +++++++------------------------------ 3 files changed, 9 insertions(+), 36 deletions(-) diff --git a/context/context.go b/context/context.go index cff1f28f3..4e72e4e2f 100644 --- a/context/context.go +++ b/context/context.go @@ -98,11 +98,7 @@ func (c *fsContext) Remotes() (Remotes, error) { if err != nil { return nil, err } - rem, err := parseRemotes(gitRemotes) - if err != nil { - return nil, err - } - c.remotes = rem + c.remotes = parseRemotes(gitRemotes) } return c.remotes, nil } diff --git a/context/remote.go b/context/remote.go index b17d4771d..8ffcef567 100644 --- a/context/remote.go +++ b/context/remote.go @@ -42,7 +42,7 @@ type GitHubRepository struct { Owner string } -func parseRemotes(gitRemotes []string) (remotes Remotes, err error) { +func parseRemotes(gitRemotes []string) (remotes Remotes) { re := regexp.MustCompile(`(.+)\s+(.+)\s+\((push|fetch)\)`) names := []string{} diff --git a/context/remote_test.go b/context/remote_test.go index 47c1f63f8..661d142e8 100644 --- a/context/remote_test.go +++ b/context/remote_test.go @@ -6,15 +6,8 @@ import ( func Test_repoFromURL(t *testing.T) { r, err := repoFromURL("http://github.com/monalisa/octo-cat.git") - if err != nil { - t.Error(err) - } - if r.Owner != "monalisa" { - t.Errorf("got Owner: %q", r.Owner) - } - if r.Name != "octo-cat" { - t.Errorf("got Name: %q", r.Name) - } + eq(t, err, nil) + eq(t, r, &GitHubRepository{Owner: "monalisa", Name: "octo-cat"}) } func Test_parseRemotes(t *testing.T) { @@ -25,26 +18,10 @@ func Test_parseRemotes(t *testing.T) { "upstream\thttps://example.com/nowhere.git (fetch)", "upstream\thttps://github.com/hubot/tools (push)", } - r, err := parseRemotes(remoteList) - if err != nil { - t.Error(err) - } - if len(r) != 3 { - t.Errorf("found %d remotes", len(r)) - } + r := parseRemotes(remoteList) + eq(t, len(r), 3) - mona := r[0] - if mona.Owner != "monalisa" || mona.Repo != "myfork" { - t.Errorf("got %s/%s", mona.Owner, mona.Repo) - } - - origin := r[1] - if origin.Owner != "monalisa" || origin.Repo != "octo-cat" { - t.Errorf("got %s/%s", origin.Owner, origin.Repo) - } - - upstream := r[2] - if upstream.Owner != "hubot" || upstream.Repo != "tools" { - t.Errorf("got %s/%s", upstream.Owner, upstream.Repo) - } + eq(t, r[0], &Remote{Name: "mona", Owner: "monalisa", Repo: "myfork"}) + eq(t, r[1], &Remote{Name: "origin", Owner: "monalisa", Repo: "octo-cat"}) + eq(t, r[2], &Remote{Name: "upstream", Owner: "hubot", Repo: "tools"}) } From 344906bf0333d5a8b656a960bc3b68f99b5f710d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Thu, 17 Oct 2019 15:49:50 +0200 Subject: [PATCH 19/22] Test SSH config parser --- command/root.go | 2 ++ git/ssh_config.go | 59 +++++++++++++++++++++--------------------- git/ssh_config_test.go | 27 +++++++++++++++++++ git/url.go | 30 +++++++++++---------- 4 files changed, 75 insertions(+), 43 deletions(-) create mode 100644 git/ssh_config_test.go diff --git a/command/root.go b/command/root.go index f4c227cb5..1df3a2b0d 100644 --- a/command/root.go +++ b/command/root.go @@ -5,6 +5,7 @@ import ( "os" "github.com/github/gh-cli/context" + "github.com/github/gh-cli/git" "github.com/spf13/cobra" ) @@ -26,6 +27,7 @@ func initContext() { repo = os.Getenv("GH_REPO") } ctx.SetBaseRepo(repo) + git.InitSSHAliasMap(nil) } // RootCmd is the entry point of command-line execution diff --git a/git/ssh_config.go b/git/ssh_config.go index 6749d90cc..47d46ac46 100644 --- a/git/ssh_config.go +++ b/git/ssh_config.go @@ -2,6 +2,7 @@ package git import ( "bufio" + "io" "os" "path/filepath" "regexp" @@ -10,13 +11,19 @@ import ( "github.com/mitchellh/go-homedir" ) -const ( - hostReStr = "(?i)^[ \t]*(host|hostname)[ \t]+(.+)$" +var ( + sshHostRE, + sshTokenRE *regexp.Regexp ) -type SSHConfig map[string]string +func init() { + sshHostRE = regexp.MustCompile("(?i)^[ \t]*(host|hostname)[ \t]+(.+)$") + sshTokenRE = regexp.MustCompile(`%[%h]`) +} -func newSSHConfigReader() *SSHConfigReader { +type sshAliasMap map[string]string + +func sshParseFiles() sshAliasMap { configFiles := []string{ "/etc/ssh_config", "/etc/ssh/ssh_config", @@ -25,38 +32,33 @@ func newSSHConfigReader() *SSHConfigReader { userConfig := filepath.Join(homedir, ".ssh", "config") configFiles = append([]string{userConfig}, configFiles...) } - return &SSHConfigReader{ - Files: configFiles, + + openFiles := []io.Reader{} + for _, file := range configFiles { + f, err := os.Open(file) + if err != nil { + continue + } + defer f.Close() + openFiles = append(openFiles, f) } + return sshParse(openFiles...) } -type SSHConfigReader struct { - Files []string -} - -func (r *SSHConfigReader) Read() SSHConfig { - config := make(SSHConfig) - hostRe := regexp.MustCompile(hostReStr) - - for _, filename := range r.Files { - r.readFile(config, hostRe, filename) +func sshParse(r ...io.Reader) sshAliasMap { + config := sshAliasMap{} + for _, file := range r { + sshParseConfig(config, file) } - return config } -func (r *SSHConfigReader) readFile(c SSHConfig, re *regexp.Regexp, f string) error { - file, err := os.Open(f) - if err != nil { - return err - } - defer file.Close() - +func sshParseConfig(c sshAliasMap, file io.Reader) error { hosts := []string{"*"} scanner := bufio.NewScanner(file) for scanner.Scan() { line := scanner.Text() - match := re.FindStringSubmatch(line) + match := sshHostRE.FindStringSubmatch(line) if match == nil { continue } @@ -67,7 +69,7 @@ func (r *SSHConfigReader) readFile(c SSHConfig, re *regexp.Regexp, f string) err } else { for _, host := range hosts { for _, name := range names { - c[host] = expandTokens(name, host) + c[host] = sshExpandTokens(name, host) } } } @@ -76,9 +78,8 @@ func (r *SSHConfigReader) readFile(c SSHConfig, re *regexp.Regexp, f string) err return scanner.Err() } -func expandTokens(text, host string) string { - re := regexp.MustCompile(`%[%h]`) - return re.ReplaceAllStringFunc(text, func(match string) string { +func sshExpandTokens(text, host string) string { + return sshTokenRE.ReplaceAllStringFunc(text, func(match string) string { switch match { case "%h": return host diff --git a/git/ssh_config_test.go b/git/ssh_config_test.go new file mode 100644 index 000000000..12de53bd8 --- /dev/null +++ b/git/ssh_config_test.go @@ -0,0 +1,27 @@ +package git + +import ( + "reflect" + "strings" + "testing" +) + +// TODO: extract assertion helpers into a shared package +func eq(t *testing.T, got interface{}, expected interface{}) { + if !reflect.DeepEqual(got, expected) { + t.Errorf("expected: %v, got: %v", expected, got) + } +} + +func Test_sshParse(t *testing.T) { + m := sshParse(strings.NewReader(` + Host foo bar + HostName example.com + `), strings.NewReader(` + Host bar baz + hostname %%%h.net%% + `)) + eq(t, m["foo"], "example.com") + eq(t, m["bar"], "%bar.net%") + eq(t, m["nonexist"], "") +} diff --git a/git/url.go b/git/url.go index f3f4adf99..792d75350 100644 --- a/git/url.go +++ b/git/url.go @@ -7,15 +7,12 @@ import ( ) var ( - cachedSSHConfig SSHConfig + cachedSSHConfig sshAliasMap protocolRe = regexp.MustCompile("^[a-zA-Z_+-]+://") ) -type URLParser struct { - SSHConfig SSHConfig -} - -func (p *URLParser) Parse(rawURL string) (u *url.URL, err error) { +// ParseURL normalizes git remote urls +func ParseURL(rawURL string) (u *url.URL, err error) { if !protocolRe.MatchString(rawURL) && strings.Contains(rawURL, ":") && // not a Windows path @@ -44,7 +41,10 @@ func (p *URLParser) Parse(rawURL string) (u *url.URL, err error) { u.Host = u.Host[0:idx] } - sshHost := p.SSHConfig[u.Host] + if cachedSSHConfig == nil { + return + } + sshHost := cachedSSHConfig[u.Host] // ignore replacing host that fixes for limited network // https://help.github.com/articles/using-ssh-over-the-https-port ignoredHost := u.Host == "github.com" && sshHost == "ssh.github.com" @@ -55,12 +55,14 @@ func (p *URLParser) Parse(rawURL string) (u *url.URL, err error) { return } -func ParseURL(rawURL string) (u *url.URL, err error) { - if cachedSSHConfig == nil { - cachedSSHConfig = newSSHConfigReader().Read() +// InitSSHAliasMap prepares globally cached SSH hostname alias mappings +func InitSSHAliasMap(m map[string]string) { + if m == nil { + cachedSSHConfig = sshParseFiles() + return + } + cachedSSHConfig = sshAliasMap{} + for k, v := range m { + cachedSSHConfig[k] = v } - - p := &URLParser{cachedSSHConfig} - - return p.Parse(rawURL) } From 183db995357839cab601f389b8b14a8864ac7c8f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Thu, 17 Oct 2019 15:58:26 +0200 Subject: [PATCH 20/22] Ensure remote URL parsing tests don't read user SSH config files --- command/root.go | 6 +----- context/remote_test.go | 21 +++++++++++++++++++++ 2 files changed, 22 insertions(+), 5 deletions(-) diff --git a/command/root.go b/command/root.go index 1df3a2b0d..ff4a3e706 100644 --- a/command/root.go +++ b/command/root.go @@ -17,9 +17,7 @@ var ( func init() { RootCmd.PersistentFlags().StringVarP(¤tRepo, "repo", "R", "", "current GitHub repository") RootCmd.PersistentFlags().StringVarP(¤tBranch, "current-branch", "B", "", "current git branch") -} -func initContext() { ctx := context.InitDefaultContext() ctx.SetBranch(currentBranch) repo := currentRepo @@ -27,6 +25,7 @@ func initContext() { repo = os.Getenv("GH_REPO") } ctx.SetBaseRepo(repo) + git.InitSSHAliasMap(nil) } @@ -36,9 +35,6 @@ var RootCmd = &cobra.Command{ Short: "GitHub CLI", Long: `Do things with GitHub from your terminal`, Args: cobra.MinimumNArgs(1), - PersistentPreRun: func(cmd *cobra.Command, args []string) { - initContext() - }, Run: func(cmd *cobra.Command, args []string) { fmt.Println("root") }, diff --git a/context/remote_test.go b/context/remote_test.go index 661d142e8..096af07e2 100644 --- a/context/remote_test.go +++ b/context/remote_test.go @@ -2,15 +2,36 @@ package context import ( "testing" + + "github.com/github/gh-cli/git" ) func Test_repoFromURL(t *testing.T) { + git.InitSSHAliasMap(nil) + r, err := repoFromURL("http://github.com/monalisa/octo-cat.git") eq(t, err, nil) eq(t, r, &GitHubRepository{Owner: "monalisa", Name: "octo-cat"}) } +func Test_repoFromURL_SSH(t *testing.T) { + git.InitSSHAliasMap(map[string]string{ + "gh": "github.com", + "github.com": "ssh.github.com", + }) + + r, err := repoFromURL("git@gh:monalisa/octo-cat") + eq(t, err, nil) + eq(t, r, &GitHubRepository{Owner: "monalisa", Name: "octo-cat"}) + + r, err = repoFromURL("git@github.com:monalisa/octo-cat") + eq(t, err, nil) + eq(t, r, &GitHubRepository{Owner: "monalisa", Name: "octo-cat"}) +} + func Test_parseRemotes(t *testing.T) { + git.InitSSHAliasMap(nil) + remoteList := []string{ "mona\tgit@github.com:monalisa/myfork.git (fetch)", "origin\thttps://github.com/monalisa/octo-cat.git (fetch)", From 51c70dd918b1569ee0dadbb6eca7b57e904d85b3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Thu, 17 Oct 2019 16:05:25 +0200 Subject: [PATCH 21/22] Add test for invalid remote URLs --- context/remote_test.go | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/context/remote_test.go b/context/remote_test.go index 096af07e2..bae779b62 100644 --- a/context/remote_test.go +++ b/context/remote_test.go @@ -1,6 +1,7 @@ package context import ( + "errors" "testing" "github.com/github/gh-cli/git" @@ -14,6 +15,16 @@ func Test_repoFromURL(t *testing.T) { eq(t, r, &GitHubRepository{Owner: "monalisa", Name: "octo-cat"}) } +func Test_repoFromURL_invalid(t *testing.T) { + git.InitSSHAliasMap(nil) + + _, err := repoFromURL("https://example.com/one/two") + eq(t, err, errors.New(`invalid hostname: example.com`)) + + _, err = repoFromURL("/path/to/disk") + eq(t, err, errors.New(`invalid hostname: `)) +} + func Test_repoFromURL_SSH(t *testing.T) { git.InitSSHAliasMap(map[string]string{ "gh": "github.com", From d334d56f09ae4e6e4f5fa9c3358ecff390f09b6f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Thu, 17 Oct 2019 16:10:45 +0200 Subject: [PATCH 22/22] Test FindByName --- context/remote_test.go | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/context/remote_test.go b/context/remote_test.go index bae779b62..70b49c4e5 100644 --- a/context/remote_test.go +++ b/context/remote_test.go @@ -57,3 +57,22 @@ func Test_parseRemotes(t *testing.T) { eq(t, r[1], &Remote{Name: "origin", Owner: "monalisa", Repo: "octo-cat"}) eq(t, r[2], &Remote{Name: "upstream", Owner: "hubot", Repo: "tools"}) } + +func Test_Remotes_FindByName(t *testing.T) { + list := Remotes{ + &Remote{Name: "mona", Owner: "monalisa", Repo: "myfork"}, + &Remote{Name: "origin", Owner: "monalisa", Repo: "octo-cat"}, + &Remote{Name: "upstream", Owner: "hubot", Repo: "tools"}, + } + + r, err := list.FindByName("upstream", "origin") + eq(t, err, nil) + eq(t, r.Name, "upstream") + + r, err = list.FindByName("nonexist", "*") + eq(t, err, nil) + eq(t, r.Name, "mona") + + _, err = list.FindByName("nonexist") + eq(t, err, errors.New(`no GitHub remotes found`)) +}