[pull] trunk from cli:trunk

api/queries_repo.go

@@ -364,6 +364,7 @@ func GitHubRepo(client *Client, repo ghrepo.Interface) (*Repository, error) {
  databaseId
  name
  owner { login }
+ sshUrl
  hasIssuesEnabled
  description
  hasWikiEnabled

api/queries_repo_test.go

@@ -42,6 +42,7 @@ func TestGitHubRepo_success(t *testing.T) {
  "databaseId": 1234,
  "name": "REPO",
  "owner": {"login": "OWNER"},
+ "sshUrl": "org-1234@github.com:OWNER/REPO.git",
  "hasIssuesEnabled": true,
  "description": "a cool repo",
  "hasWikiEnabled": true,
@@ -61,6 +62,7 @@ func TestGitHubRepo_success(t *testing.T) {
  DatabaseID: 1234,
  Name: "REPO",
  Owner: RepositoryOwner{Login: "OWNER"},
+ SSHURL: "org-1234@github.com:OWNER/REPO.git",
  HasIssuesEnabled: true,
  Description: "a cool repo",
  HasWikiEnabled: true,
@@ -85,6 +87,7 @@ func TestGitHubRepo_withParent(t *testing.T) {
  "id": "REPOID",
  "name": "REPO",
  "owner": {"login": "OWNER"},
+ "sshUrl": "git@github.com:OWNER/REPO.git",
  "hasIssuesEnabled": true,
  "description": "",
  "hasWikiEnabled": false,
@@ -94,6 +97,7 @@ func TestGitHubRepo_withParent(t *testing.T) {
  "id": "PARENTID",
  "name": "PARENT-REPO",
  "owner": {"login": "PARENT-OWNER"},
+ "sshUrl": "org-5678@github.com:PARENT-OWNER/PARENT-REPO.git",
  "hasIssuesEnabled": true,
  "description": "parent repo",
  "hasWikiEnabled": true,
@@ -112,6 +116,7 @@ func TestGitHubRepo_withParent(t *testing.T) {
  ID: "PARENTID",
  Name: "PARENT-REPO",
  Owner: RepositoryOwner{Login: "PARENT-OWNER"},
+ SSHURL: "org-5678@github.com:PARENT-OWNER/PARENT-REPO.git",
  HasIssuesEnabled: true,
  Description: "parent repo",
  HasWikiEnabled: true,
@@ -123,6 +128,7 @@ func TestGitHubRepo_withParent(t *testing.T) {
  ID: "REPOID",
  Name: "REPO",
  Owner: RepositoryOwner{Login: "OWNER"},
+ SSHURL: "git@github.com:OWNER/REPO.git",
  HasIssuesEnabled: true,
  ViewerPermission: "READ",
  DefaultBranchRef: BranchRef{Name: "main"},
@@ -134,21 +140,22 @@ func TestGitHubRepo_withParent(t *testing.T) {
 	assert.False(t, repo.ViewerCanTriage())
 }
 
-// TestBaseRepoQueriesSelectDatabaseID guards the 3 queries that build the base
-// repository. Each selection is written by hand, and a field left out of one
-// of them is silently zero rather than an error, so the selection is asserted
-// against the request instead of the response.
-func TestBaseRepoQueriesSelectDatabaseID(t *testing.T) {
+// TestBaseRepoQuerySelections guards fields that must be explicitly requested.
+// A field left out of a query is silently zero rather than an error, so the
+// selection is asserted against the request instead of the response.
+func TestBaseRepoQuerySelections(t *testing.T) {
 	tests := []struct {
  name string
  matcher httpmock.Matcher
  body string
+ fields []string
  call func(*Client) error
 	}{
  {
  name: "GitHubRepo",
  matcher: httpmock.GraphQL(`query RepositoryInfo\b`),
  body: `{ "data": { "repository": { "id": "REPOID", "databaseId": 1234 } } }`,
+ fields: []string{"databaseId", "sshUrl"},
  call: func(client *Client) error {
  _, err := GitHubRepo(client, ghrepo.New("OWNER", "REPO"))
  return err
@@ -158,6 +165,7 @@ func TestBaseRepoQueriesSelectDatabaseID(t *testing.T) {
  name: "RepoNetwork",
  matcher: httpmock.GraphQL(`query RepositoryNetwork\b`),
  body: `{ "data": { "repo_000": { "id": "REPOID", "databaseId": 1234 } } }`,
+ fields: []string{"databaseId"},
  call: func(client *Client) error {
  _, err := RepoNetwork(client, []ghrepo.Interface{ghrepo.New("OWNER", "REPO")})
  return err
@@ -167,6 +175,7 @@ func TestBaseRepoQueriesSelectDatabaseID(t *testing.T) {
  name: "IssueRepoInfo",
  matcher: httpmock.GraphQL(`query IssueRepositoryInfo\b`),
  body: `{ "data": { "repository": { "id": "REPOID", "databaseId": 1234 } } }`,
+ fields: []string{"databaseId"},
  call: func(client *Client) error {
  _, err := IssueRepoInfo(client, ghrepo.New("OWNER", "REPO"))
  return err
@@ -185,7 +194,9 @@ func TestBaseRepoQueriesSelectDatabaseID(t *testing.T) {
  }))
 
  require.NoError(t, tt.call(newTestClient(httpReg)))
- assert.Contains(t, query, "databaseId")
+ for _, field := range tt.fields {
+ assert.Contains(t, query, field)
+ }
  })
 	}
 }

pkg/cmd/repo/clone/clone.go

@@ -175,7 +175,10 @@ func cloneRun(opts *CloneOptions) error {
 	if err != nil {
  return err
 	}
-	canonicalCloneURL := ghrepo.FormatRemoteURL(canonicalRepo, protocol)
+	canonicalCloneURL, err := remoteURL(canonicalRepo, protocol)
+	if err != nil {
+ return err
+	}
 
 	// If repo HasWikiEnabled and wantsWiki is true then create a new clone URL
 	if wantsWiki {
@@ -203,7 +206,10 @@ func cloneRun(opts *CloneOptions) error {
  }
  } else {
  protocol := cfg.GitProtocol(canonicalRepo.Parent.RepoHost()).Value
- upstreamURL := ghrepo.FormatRemoteURL(canonicalRepo.Parent, protocol)
+ upstreamURL, err := remoteURL(canonicalRepo.Parent, protocol)
+ if err != nil {
+ return err
+ }
 
  upstreamName := opts.UpstreamName
  if opts.UpstreamName == "@owner" {
@@ -236,6 +242,16 @@ func cloneRun(opts *CloneOptions) error {
 	return nil
 }
 
+func remoteURL(repo *api.Repository, protocol string) (string, error) {
+	if protocol != "ssh" {
+ return ghrepo.FormatRemoteURL(repo, protocol), nil
+	}
+	if repo.SSHURL == "" {
+ return "", fmt.Errorf("invalid API response: repository %s is missing sshUrl", ghrepo.FullName(repo))
+	}
+	return repo.SSHURL, nil
+}
+
 // simplifyURL strips given URL of extra parts like extra path segments (i.e.,
 // anything beyond `/owner/repo`), query strings, or fragments. This function
 // never returns an error.

pkg/cmd/repo/clone/clone_test.go

@@ -112,14 +112,18 @@ func TestNewCmdClone(t *testing.T) {
 }
 
 func runCloneCommand(httpClient *http.Client, cli string) (*test.CmdOut, error) {
+	return runCloneCommandWithConfig(httpClient, config.NewMockConfig(), cli)
+}
+
+func runCloneCommandWithConfig(httpClient *http.Client, cfg gh.Config, cli string) (*test.CmdOut, error) {
 	ios, stdin, stdout, stderr := iostreams.Test()
 	fac := &cmdutil.Factory{
  IOStreams: ios,
  HttpClient: func() (*http.Client, error) {
  return httpClient, nil
  },
  Config: func() (gh.Config, error) {
- return config.NewMockConfig(), nil
+ return cfg, nil
  },
  GitClient: &git.Client{
  GhPath: "some/path/gh",
@@ -151,9 +155,11 @@ func runCloneCommand(httpClient *http.Client, cli string) (*test.CmdOut, error)
 
 func Test_RepoClone(t *testing.T) {
 	tests := []struct {
- name string
- args string
- want string
+ name string
+ args string
+ sshURL string
+ gitProtocol string
+ want string
 	}{
  {
  name: "shorthand",
@@ -186,9 +192,23 @@ func Test_RepoClone(t *testing.T) {
  want: "git clone https://github.com/OWNER/REPO.git",
  },
  {
- name: "SSH URL",
- args: "git@github.com:OWNER/REPO.git",
- want: "git clone git@github.com:OWNER/REPO.git",
+ name: "SSH URL",
+ args: "git@github.com:OWNER/REPO.git",
+ sshURL: "git@github.com:OWNER/REPO.git",
+ want: "git clone git@github.com:OWNER/REPO.git",
+ },
+ {
+ name: "GitHub.com SSH certificate authority URL",
+ args: "OWNER/REPO",
+ sshURL: "org-1234@github.com:OWNER/REPO.git",
+ gitProtocol: "ssh",
+ want: "git clone org-1234@github.com:OWNER/REPO.git",
+ },
+ {
+ name: "GHE.com SSH certificate authority URL",
+ args: "test-prodweu01@test-prodweu01.ghe.com:OWNER/REPO.git",
+ sshURL: "test-prodweu01_1234@test-prodweu01.ghe.com:OWNER/REPO.git",
+ want: "git clone test-prodweu01_1234@test-prodweu01.ghe.com:OWNER/REPO.git",
  },
  {
  name: "Non-canonical capitalization",
@@ -200,6 +220,13 @@ func Test_RepoClone(t *testing.T) {
  args: "Owner/Repo.wiki",
  want: "git clone https://github.com/OWNER/REPO.wiki.git",
  },
+ {
+ name: "clone wiki with SSH certificate authority URL",
+ args: "Owner/Repo.wiki",
+ sshURL: "org-1234@github.com:OWNER/REPO.git",
+ gitProtocol: "ssh",
+ want: "git clone org-1234@github.com:OWNER/REPO.wiki.git",
+ },
  {
  name: "wiki URL",
  args: "https://github.com/owner/repo.wiki",
@@ -223,6 +250,7 @@ func Test_RepoClone(t *testing.T) {
  "owner": {
  "login": "OWNER"
  },
+ "sshUrl": "`+tt.sshURL+`",
  "hasWikiEnabled": true
  } } }
  `))
@@ -233,7 +261,11 @@ func Test_RepoClone(t *testing.T) {
  defer restore(t)
  cs.Register(tt.want, 0, "")
 
- output, err := runCloneCommand(httpClient, tt.args)
+ cfg := config.NewMockConfig()
+ if tt.gitProtocol != "" {
+ cfg.Set("", "git_protocol", tt.gitProtocol)
+ }
+ output, err := runCloneCommandWithConfig(httpClient, cfg, tt.args)
  if err != nil {
  t.Fatalf("error running command `repo clone`: %v", err)
  }
@@ -244,6 +276,25 @@ func Test_RepoClone(t *testing.T) {
 	}
 }
 
+func Test_RepoClone_missingSSHURL(t *testing.T) {
+	reg := &httpmock.Registry{}
+	defer reg.Verify(t)
+	reg.Register(
+ httpmock.GraphQL(`query RepositoryInfo\b`),
+ httpmock.StringResponse(`
+ { "data": { "repository": {
+ "name": "REPO",
+ "owner": {"login": "OWNER"}
+ } } }
+ `))
+
+	cfg := config.NewMockConfig()
+	cfg.Set("", "git_protocol", "ssh")
+
+	_, err := runCloneCommandWithConfig(&http.Client{Transport: reg}, cfg, "OWNER/REPO")
+	require.EqualError(t, err, "invalid API response: repository OWNER/REPO is missing sshUrl")
+}
+
 func Test_RepoClone_hasParent(t *testing.T) {
 	reg := &httpmock.Registry{}
 	defer reg.Verify(t)
@@ -284,6 +335,41 @@ func Test_RepoClone_hasParent(t *testing.T) {
 	}
 }
 
+func Test_RepoClone_hasParentWithSSHCertificateAuthorityURLs(t *testing.T) {
+	reg := &httpmock.Registry{}
+	defer reg.Verify(t)
+	reg.Register(
+ httpmock.GraphQL(`query RepositoryInfo\b`),
+ httpmock.StringResponse(`
+ { "data": { "repository": {
+ "name": "REPO",
+ "owner": {"login": "OWNER"},
+ "sshUrl": "org-1234@github.com:OWNER/REPO.git",
+ "parent": {
+ "name": "ORIG",
+ "owner": {"login": "hubot"},
+ "sshUrl": "org-5678@github.com:hubot/ORIG.git",
+ "defaultBranchRef": {"name": "trunk"}
+ }
+ } } }
+ `))
+
+	cfg := config.NewMockConfig()
+	cfg.Set("", "git_protocol", "ssh")
+	httpClient := &http.Client{Transport: reg}
+
+	cs, cmdTeardown := run.Stub()
+	defer cmdTeardown(t)
+	cs.Register(`git clone org-1234@github.com:OWNER/REPO.git`, 0, "")
+	cs.Register(`git -C REPO remote add -t trunk upstream org-5678@github.com:hubot/ORIG.git`, 0, "")
+	cs.Register(`git -C REPO fetch upstream`, 0, "")
+	cs.Register(`git -C REPO remote set-branches upstream *`, 0, "")
+	cs.Register(`git -C REPO config --add remote.upstream.gh-resolved base`, 0, "")
+
+	_, err := runCloneCommandWithConfig(httpClient, cfg, "OWNER/REPO")
+	require.NoError(t, err)
+}
+
 func Test_RepoClone_hasParent_upstreamRemoteName(t *testing.T) {
 	reg := &httpmock.Registry{}
 	defer reg.Verify(t)