From eeca99864087b41fbff6eceed7b6878545722b37 Mon Sep 17 00:00:00 2001 From: vilmibm Date: Wed, 8 Sep 2021 17:27:06 -0500 Subject: [PATCH] binary extension support in gh extension install --- pkg/cmd/extension/command.go | 48 ++++++++++++- pkg/cmd/extension/command_test.go | 53 +++++++++++++- pkg/cmd/extension/http.go | 113 ++++++++++++++++++++++++++++++ pkg/cmd/extension/manager.go | 82 +++++++++++++++++++++- pkg/cmd/extension/manager_test.go | 59 +++++++++++++++- pkg/extensions/extension.go | 6 +- pkg/extensions/manager_mock.go | 93 ++++++++++++++++++------ 7 files changed, 425 insertions(+), 29 deletions(-) create mode 100644 pkg/cmd/extension/http.go diff --git a/pkg/cmd/extension/command.go b/pkg/cmd/extension/command.go index 3d5215829..f83637681 100644 --- a/pkg/cmd/extension/command.go +++ b/pkg/cmd/extension/command.go @@ -3,10 +3,13 @@ package extension import ( "errors" "fmt" + "net/http" "os" "strings" + "time" "github.com/MakeNowJust/heredoc" + "github.com/cli/cli/v2/api" "github.com/cli/cli/v2/git" "github.com/cli/cli/v2/internal/ghrepo" "github.com/cli/cli/v2/pkg/cmdutil" @@ -102,15 +105,38 @@ func NewCmdExtension(f *cmdutil.Factory) *cobra.Command { return err } if err := checkValidExtension(cmd.Root(), m, repo.RepoName()); err != nil { + // TODO i feel like this should check for a gh-foo script return err } + client, err := f.HttpClient() + if err != nil { + return fmt.Errorf("could not make http client: %w", err) + } + client = api.NewCachedClient(client, time.Second*30) + + isBin, err := isBinExtension(client, repo) + if err != nil { + return fmt.Errorf("could not check for binary extension: %w", err) + } + if isBin { + return m.InstallBin(client, repo) + } + + hs, err := hasScript(client, repo) + if err != nil { + return err + } + if !hs { + return errors.New("extension is uninstallable: missing executable") + } + cfg, err := f.Config() if err != nil { return err } protocol, _ := cfg.Get(repo.RepoHost(), "git_protocol") - return m.Install(ghrepo.FormatRemoteURL(repo, protocol), io.Out, io.ErrOut) + return m.InstallGit(ghrepo.FormatRemoteURL(repo, protocol), io.Out, io.ErrOut) }, }, func() *cobra.Command { @@ -220,6 +246,26 @@ func checkValidExtension(rootCmd *cobra.Command, m extensions.ExtensionManager, return nil } +func isBinExtension(client *http.Client, repo ghrepo.Interface) (isBin bool, err error) { + hs, err := hasScript(client, repo) + if err != nil || hs { + return + } + + _, err = fetchLatestRelease(client, repo) + if err != nil { + httpErr, ok := err.(api.HTTPError) + if ok && httpErr.StatusCode == 404 { + err = nil + return + } + return + } + + isBin = true + return +} + func normalizeExtensionSelector(n string) string { if idx := strings.IndexRune(n, '/'); idx >= 0 { n = n[idx+1:] diff --git a/pkg/cmd/extension/command_test.go b/pkg/cmd/extension/command_test.go index 9dc46b3ba..635fcba17 100644 --- a/pkg/cmd/extension/command_test.go +++ b/pkg/cmd/extension/command_test.go @@ -3,14 +3,17 @@ package extension import ( "io" "io/ioutil" + "net/http" "os" "strings" "testing" "github.com/MakeNowJust/heredoc" "github.com/cli/cli/v2/internal/config" + "github.com/cli/cli/v2/internal/ghrepo" "github.com/cli/cli/v2/pkg/cmdutil" "github.com/cli/cli/v2/pkg/extensions" + "github.com/cli/cli/v2/pkg/httpmock" "github.com/cli/cli/v2/pkg/iostreams" "github.com/spf13/cobra" "github.com/stretchr/testify/assert" @@ -25,6 +28,7 @@ func TestNewCmdExtension(t *testing.T) { tests := []struct { name string args []string + httpStubs func(*httpmock.Registry) managerStubs func(em *extensions.ExtensionManagerMock) func(*testing.T) isTTY bool wantErr bool @@ -33,17 +37,22 @@ func TestNewCmdExtension(t *testing.T) { wantStderr string }{ { - name: "install an extension", + name: "install a git extension", args: []string{"install", "owner/gh-some-ext"}, + httpStubs: func(reg *httpmock.Registry) { + reg.Register( + httpmock.REST("GET", "repos/owner/gh-some-ext/contents/gh-some-ext"), + httpmock.StringResponse("a script")) + }, managerStubs: func(em *extensions.ExtensionManagerMock) func(*testing.T) { em.ListFunc = func(bool) []extensions.Extension { return []extensions.Extension{} } - em.InstallFunc = func(s string, out, errOut io.Writer) error { + em.InstallGitFunc = func(s string, out, errOut io.Writer) error { return nil } return func(t *testing.T) { - installCalls := em.InstallCalls() + installCalls := em.InstallGitCalls() assert.Equal(t, 1, len(installCalls)) assert.Equal(t, "https://github.com/owner/gh-some-ext.git", installCalls[0].URL) listCalls := em.ListCalls() @@ -51,6 +60,33 @@ func TestNewCmdExtension(t *testing.T) { } }, }, + { + name: "install a binary extension", + args: []string{"install", "owner/gh-bin-ext"}, + httpStubs: func(reg *httpmock.Registry) { + reg.Register( + httpmock.REST("GET", "repos/owner/gh-bin-ext/contents/gh-bin-ext"), + httpmock.StatusStringResponse(404, "no")) + reg.Register( + httpmock.REST("GET", "repos/owner/gh-bin-ext/releases/latest"), + httpmock.StringResponse("{}")) + }, + managerStubs: func(em *extensions.ExtensionManagerMock) func(*testing.T) { + em.ListFunc = func(bool) []extensions.Extension { + return []extensions.Extension{} + } + em.InstallBinFunc = func(_ *http.Client, _ ghrepo.Interface) error { + return nil + } + return func(t *testing.T) { + installCalls := em.InstallBinCalls() + assert.Equal(t, 1, len(installCalls)) + assert.Equal(t, "gh-bin-ext", installCalls[0].Repo.RepoName()) + listCalls := em.ListCalls() + assert.Equal(t, 1, len(listCalls)) + } + }, + }, { name: "install an extension with same name as existing extension", args: []string{"install", "owner/gh-existing-ext"}, @@ -281,12 +317,23 @@ func TestNewCmdExtension(t *testing.T) { assertFunc = tt.managerStubs(em) } + reg := httpmock.Registry{} + defer reg.Verify(t) + client := http.Client{Transport: ®} + + if tt.httpStubs != nil { + tt.httpStubs(®) + } + f := cmdutil.Factory{ Config: func() (config.Config, error) { return config.NewBlankConfig(), nil }, IOStreams: ios, ExtensionManager: em, + HttpClient: func() (*http.Client, error) { + return &client, nil + }, } cmd := NewCmdExtension(&f) diff --git a/pkg/cmd/extension/http.go b/pkg/cmd/extension/http.go new file mode 100644 index 000000000..de6f61e4b --- /dev/null +++ b/pkg/cmd/extension/http.go @@ -0,0 +1,113 @@ +package extension + +import ( + "encoding/json" + "fmt" + "io" + "io/ioutil" + "net/http" + "os" + + "github.com/cli/cli/v2/api" + "github.com/cli/cli/v2/internal/ghinstance" + "github.com/cli/cli/v2/internal/ghrepo" +) + +func hasScript(httpClient *http.Client, repo ghrepo.Interface) (hs bool, err error) { + path := fmt.Sprintf("repos/%s/%s/contents/%s", + repo.RepoOwner(), repo.RepoName(), repo.RepoName()) + url := ghinstance.RESTPrefix(repo.RepoHost()) + path + req, err := http.NewRequest("GET", url, nil) + if err != nil { + return + } + + resp, err := httpClient.Do(req) + if err != nil { + return + } + defer resp.Body.Close() + + if resp.StatusCode == 404 { + return + } + + if resp.StatusCode > 299 { + err = api.HandleHTTPError(resp) + return + } + + hs = true + return +} + +type releaseAsset struct { + Name string + APIURL string `json:"url"` +} + +type release struct { + Assets []releaseAsset +} + +// downloadAsset downloads a single asset to the given file path. +func downloadAsset(httpClient *http.Client, asset releaseAsset, destPath string) error { + req, err := http.NewRequest("GET", asset.APIURL, nil) + if err != nil { + return err + } + + req.Header.Set("Accept", "application/octet-stream") + + resp, err := httpClient.Do(req) + if err != nil { + return err + } + defer resp.Body.Close() + + if resp.StatusCode > 299 { + return api.HandleHTTPError(resp) + } + + f, err := os.OpenFile(destPath, os.O_WRONLY|os.O_CREATE|os.O_EXCL, 0755) + if err != nil { + return err + } + defer f.Close() + + _, err = io.Copy(f, resp.Body) + return err +} + +// fetchLatestRelease finds the latest published release for a repository. +func fetchLatestRelease(httpClient *http.Client, baseRepo ghrepo.Interface) (*release, error) { + path := fmt.Sprintf("repos/%s/%s/releases/latest", baseRepo.RepoOwner(), baseRepo.RepoName()) + url := ghinstance.RESTPrefix(baseRepo.RepoHost()) + path + req, err := http.NewRequest("GET", url, nil) + if err != nil { + return nil, err + } + + resp, err := httpClient.Do(req) + if err != nil { + return nil, err + } + defer resp.Body.Close() + + if resp.StatusCode > 299 { + return nil, api.HandleHTTPError(resp) + } + + b, err := ioutil.ReadAll(resp.Body) + if err != nil { + return nil, err + } + + var r release + err = json.Unmarshal(b, &r) + if err != nil { + return nil, err + } + + return &r, nil +} diff --git a/pkg/cmd/extension/manager.go b/pkg/cmd/extension/manager.go index b488704e5..8ffbc422e 100644 --- a/pkg/cmd/extension/manager.go +++ b/pkg/cmd/extension/manager.go @@ -6,6 +6,7 @@ import ( "fmt" "io" "io/ioutil" + "net/http" "os" "os/exec" "path" @@ -15,9 +16,11 @@ import ( "github.com/MakeNowJust/heredoc" "github.com/cli/cli/v2/internal/config" + "github.com/cli/cli/v2/internal/ghrepo" "github.com/cli/cli/v2/pkg/extensions" "github.com/cli/cli/v2/pkg/findsh" "github.com/cli/safeexec" + "gopkg.in/yaml.v3" ) type Manager struct { @@ -25,6 +28,7 @@ type Manager struct { lookPath func(string) (string, error) findSh func() (string, error) newCommand func(string, ...string) *exec.Cmd + platform func() string } func NewManager() *Manager { @@ -33,6 +37,9 @@ func NewManager() *Manager { lookPath: safeexec.LookPath, findSh: findsh.Find, newCommand: exec.Command, + platform: func() string { + return fmt.Sprintf("%s-%s", runtime.GOOS, runtime.GOARCH) + }, } } @@ -172,7 +179,80 @@ func (m *Manager) InstallLocal(dir string) error { return makeSymlink(dir, targetLink) } -func (m *Manager) Install(cloneURL string, stdout, stderr io.Writer) error { +type BinManifest struct { + Owner string + Name string + Host string + // TODO I may end up not using this; just thinking ahead to local installs + Path string +} + +func (m *Manager) InstallBin(client *http.Client, repo ghrepo.Interface) error { + var r *release + r, err := fetchLatestRelease(client, repo) + if err != nil { + return err + } + + suffix := m.platform() + var asset *releaseAsset + for _, a := range r.Assets { + if strings.HasSuffix(a.Name, suffix) { + asset = &a + break + } + } + + if asset == nil { + return fmt.Errorf("%s unsupported for %s. Open an issue: `gh issue create -R%s/%s -t'Support %s'`", + repo.RepoName(), + suffix, repo.RepoOwner(), repo.RepoName(), suffix) + } + + name := repo.RepoName() + targetDir := filepath.Join(m.installDir(), name) + // TODO clean this up if function errs? + err = os.MkdirAll(targetDir, 0755) + if err != nil { + return fmt.Errorf("failed to create installation directory: %w", err) + } + + binPath := filepath.Join(targetDir, name) + + err = downloadAsset(client, *asset, binPath) + if err != nil { + return fmt.Errorf("failed to download asset %s: %w", asset.Name, err) + } + + manifest := BinManifest{ + Name: name, + Owner: repo.RepoOwner(), + Host: repo.RepoHost(), + Path: binPath, + } + + bs, err := yaml.Marshal(manifest) + if err != nil { + return fmt.Errorf("failed to serialize manifest: %w", err) + } + + manifestPath := filepath.Join(targetDir, "manifest.yml") + + f, err := os.OpenFile(manifestPath, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0600) + if err != nil { + return fmt.Errorf("failed to open manifest for writing: %w", err) + } + defer f.Close() + + _, err = f.Write(bs) + if err != nil { + return fmt.Errorf("failed write manifest file: %w", err) + } + + return nil +} + +func (m *Manager) InstallGit(cloneURL string, stdout, stderr io.Writer) error { exe, err := m.lookPath("git") if err != nil { return err diff --git a/pkg/cmd/extension/manager_test.go b/pkg/cmd/extension/manager_test.go index 2fd458bf1..c03d3fa62 100644 --- a/pkg/cmd/extension/manager_test.go +++ b/pkg/cmd/extension/manager_test.go @@ -4,6 +4,7 @@ import ( "bytes" "fmt" "io/ioutil" + "net/http" "os" "os/exec" "path/filepath" @@ -11,7 +12,10 @@ import ( "testing" "github.com/MakeNowJust/heredoc" + "github.com/cli/cli/v2/internal/ghrepo" + "github.com/cli/cli/v2/pkg/httpmock" "github.com/stretchr/testify/assert" + "gopkg.in/yaml.v3" ) func TestHelperProcess(t *testing.T) { @@ -39,6 +43,9 @@ func newTestManager(dir string) *Manager { cmd.Env = []string{"GH_WANT_HELPER_PROCESS=1"} return cmd }, + platform: func() string { + return "amiga-arm64" + }, } } @@ -190,18 +197,66 @@ func TestManager_Upgrade_NoExtensions(t *testing.T) { assert.Equal(t, "", stderr.String()) } -func TestManager_Install(t *testing.T) { +func TestManager_InstallGit(t *testing.T) { tempDir := t.TempDir() m := newTestManager(tempDir) stdout := &bytes.Buffer{} stderr := &bytes.Buffer{} - err := m.Install("https://github.com/owner/gh-some-ext.git", stdout, stderr) + err := m.InstallGit("https://github.com/owner/gh-some-ext.git", stdout, stderr) assert.NoError(t, err) assert.Equal(t, fmt.Sprintf("[git clone https://github.com/owner/gh-some-ext.git %s]\n", filepath.Join(tempDir, "extensions", "gh-some-ext")), stdout.String()) assert.Equal(t, "", stderr.String()) } +func TestManager_InstallBin(t *testing.T) { + repo := ghrepo.NewWithHost("owner", "gh-bin-ext", "example.com") + + reg := httpmock.Registry{} + defer reg.Verify(t) + client := http.Client{Transport: ®} + + reg.Register( + httpmock.REST("GET", "api/v3/repos/owner/gh-bin-ext/releases/latest"), + httpmock.JSONResponse( + release{ + Assets: []releaseAsset{ + { + Name: "gh-bin-ext-amiga-arm64", + APIURL: "https://example.com/release/cool", + }, + }, + })) + reg.Register( + httpmock.REST("GET", "release/cool"), + httpmock.StringResponse("FAKE BINARY")) + + tempDir := t.TempDir() + m := newTestManager(tempDir) + + err := m.InstallBin(&client, repo) + assert.NoError(t, err) + + manifest, err := os.ReadFile(filepath.Join(tempDir, "extensions/gh-bin-ext/manifest.yml")) + assert.NoError(t, err) + + var bm BinManifest + err = yaml.Unmarshal(manifest, &bm) + assert.NoError(t, err) + + assert.Equal(t, BinManifest{ + Name: "gh-bin-ext", + Owner: "owner", + Host: "example.com", + Path: filepath.Join(tempDir, "extensions/gh-bin-ext/gh-bin-ext"), + }, bm) + + fakeBin, err := os.ReadFile(filepath.Join(tempDir, "extensions/gh-bin-ext/gh-bin-ext")) + assert.NoError(t, err) + + assert.Equal(t, "FAKE BINARY", string(fakeBin)) +} + func TestManager_Create(t *testing.T) { tempDir := t.TempDir() oldWd, _ := os.Getwd() diff --git a/pkg/extensions/extension.go b/pkg/extensions/extension.go index 54379ec7f..6102c1e9a 100644 --- a/pkg/extensions/extension.go +++ b/pkg/extensions/extension.go @@ -2,6 +2,9 @@ package extensions import ( "io" + "net/http" + + "github.com/cli/cli/v2/internal/ghrepo" ) //go:generate moq -rm -out extension_mock.go . Extension @@ -16,7 +19,8 @@ type Extension interface { //go:generate moq -rm -out manager_mock.go . ExtensionManager type ExtensionManager interface { List(includeMetadata bool) []Extension - Install(url string, stdout, stderr io.Writer) error + InstallGit(url string, stdout, stderr io.Writer) error + InstallBin(client *http.Client, repo ghrepo.Interface) error InstallLocal(dir string) error Upgrade(name string, force bool, stdout, stderr io.Writer) error Remove(name string) error diff --git a/pkg/extensions/manager_mock.go b/pkg/extensions/manager_mock.go index 157262698..97596e70d 100644 --- a/pkg/extensions/manager_mock.go +++ b/pkg/extensions/manager_mock.go @@ -4,7 +4,9 @@ package extensions import ( + "github.com/cli/cli/v2/internal/ghrepo" "io" + "net/http" "sync" ) @@ -24,8 +26,11 @@ var _ ExtensionManager = &ExtensionManagerMock{} // DispatchFunc: func(args []string, stdin io.Reader, stdout io.Writer, stderr io.Writer) (bool, error) { // panic("mock out the Dispatch method") // }, -// InstallFunc: func(url string, stdout io.Writer, stderr io.Writer) error { -// panic("mock out the Install method") +// InstallBinFunc: func(client *http.Client, repo ghrepo.Interface) error { +// panic("mock out the InstallBin method") +// }, +// InstallGitFunc: func(url string, stdout io.Writer, stderr io.Writer) error { +// panic("mock out the InstallGit method") // }, // InstallLocalFunc: func(dir string) error { // panic("mock out the InstallLocal method") @@ -52,8 +57,11 @@ type ExtensionManagerMock struct { // DispatchFunc mocks the Dispatch method. DispatchFunc func(args []string, stdin io.Reader, stdout io.Writer, stderr io.Writer) (bool, error) - // InstallFunc mocks the Install method. - InstallFunc func(url string, stdout io.Writer, stderr io.Writer) error + // InstallBinFunc mocks the InstallBin method. + InstallBinFunc func(client *http.Client, repo ghrepo.Interface) error + + // InstallGitFunc mocks the InstallGit method. + InstallGitFunc func(url string, stdout io.Writer, stderr io.Writer) error // InstallLocalFunc mocks the InstallLocal method. InstallLocalFunc func(dir string) error @@ -85,8 +93,15 @@ type ExtensionManagerMock struct { // Stderr is the stderr argument value. Stderr io.Writer } - // Install holds details about calls to the Install method. - Install []struct { + // InstallBin holds details about calls to the InstallBin method. + InstallBin []struct { + // Client is the client argument value. + Client *http.Client + // Repo is the repo argument value. + Repo ghrepo.Interface + } + // InstallGit holds details about calls to the InstallGit method. + InstallGit []struct { // URL is the url argument value. URL string // Stdout is the stdout argument value. @@ -123,7 +138,8 @@ type ExtensionManagerMock struct { } lockCreate sync.RWMutex lockDispatch sync.RWMutex - lockInstall sync.RWMutex + lockInstallBin sync.RWMutex + lockInstallGit sync.RWMutex lockInstallLocal sync.RWMutex lockList sync.RWMutex lockRemove sync.RWMutex @@ -204,10 +220,45 @@ func (mock *ExtensionManagerMock) DispatchCalls() []struct { return calls } -// Install calls InstallFunc. -func (mock *ExtensionManagerMock) Install(url string, stdout io.Writer, stderr io.Writer) error { - if mock.InstallFunc == nil { - panic("ExtensionManagerMock.InstallFunc: method is nil but ExtensionManager.Install was just called") +// InstallBin calls InstallBinFunc. +func (mock *ExtensionManagerMock) InstallBin(client *http.Client, repo ghrepo.Interface) error { + if mock.InstallBinFunc == nil { + panic("ExtensionManagerMock.InstallBinFunc: method is nil but ExtensionManager.InstallBin was just called") + } + callInfo := struct { + Client *http.Client + Repo ghrepo.Interface + }{ + Client: client, + Repo: repo, + } + mock.lockInstallBin.Lock() + mock.calls.InstallBin = append(mock.calls.InstallBin, callInfo) + mock.lockInstallBin.Unlock() + return mock.InstallBinFunc(client, repo) +} + +// InstallBinCalls gets all the calls that were made to InstallBin. +// Check the length with: +// len(mockedExtensionManager.InstallBinCalls()) +func (mock *ExtensionManagerMock) InstallBinCalls() []struct { + Client *http.Client + Repo ghrepo.Interface +} { + var calls []struct { + Client *http.Client + Repo ghrepo.Interface + } + mock.lockInstallBin.RLock() + calls = mock.calls.InstallBin + mock.lockInstallBin.RUnlock() + return calls +} + +// InstallGit calls InstallGitFunc. +func (mock *ExtensionManagerMock) InstallGit(url string, stdout io.Writer, stderr io.Writer) error { + if mock.InstallGitFunc == nil { + panic("ExtensionManagerMock.InstallGitFunc: method is nil but ExtensionManager.InstallGit was just called") } callInfo := struct { URL string @@ -218,16 +269,16 @@ func (mock *ExtensionManagerMock) Install(url string, stdout io.Writer, stderr i Stdout: stdout, Stderr: stderr, } - mock.lockInstall.Lock() - mock.calls.Install = append(mock.calls.Install, callInfo) - mock.lockInstall.Unlock() - return mock.InstallFunc(url, stdout, stderr) + mock.lockInstallGit.Lock() + mock.calls.InstallGit = append(mock.calls.InstallGit, callInfo) + mock.lockInstallGit.Unlock() + return mock.InstallGitFunc(url, stdout, stderr) } -// InstallCalls gets all the calls that were made to Install. +// InstallGitCalls gets all the calls that were made to InstallGit. // Check the length with: -// len(mockedExtensionManager.InstallCalls()) -func (mock *ExtensionManagerMock) InstallCalls() []struct { +// len(mockedExtensionManager.InstallGitCalls()) +func (mock *ExtensionManagerMock) InstallGitCalls() []struct { URL string Stdout io.Writer Stderr io.Writer @@ -237,9 +288,9 @@ func (mock *ExtensionManagerMock) InstallCalls() []struct { Stdout io.Writer Stderr io.Writer } - mock.lockInstall.RLock() - calls = mock.calls.Install - mock.lockInstall.RUnlock() + mock.lockInstallGit.RLock() + calls = mock.calls.InstallGit + mock.lockInstallGit.RUnlock() return calls }