From d256581203321ebb4bd205e1209c6fc77233b67f Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Tue, 10 Aug 2021 18:55:33 +0000 Subject: [PATCH 01/25] feat: Add update command to coder-cli This commit adds a new update subcommand that queries a Coder instance for its current version, fetches the corresponding version from GitHub releases if required, and updates the binary in-place. --- go.mod | 2 + go.sum | 3 + internal/cmd/cmd.go | 1 + internal/cmd/update.go | 309 ++++++++++++++++++++++++++++++++++++ internal/cmd/update_test.go | 215 +++++++++++++++++++++++++ internal/cmd/workspaces.go | 2 +- internal/version/version.go | 2 +- 7 files changed, 532 insertions(+), 2 deletions(-) create mode 100644 internal/cmd/update.go create mode 100644 internal/cmd/update_test.go diff --git a/go.mod b/go.mod index 09be0cc6..daeed5e7 100644 --- a/go.mod +++ b/go.mod @@ -5,6 +5,7 @@ go 1.14 require ( cdr.dev/slog v1.4.1 cdr.dev/wsep v0.0.0-20200728013649-82316a09813f + github.com/blang/semver/v4 v4.0.0 github.com/briandowns/spinner v1.16.0 github.com/cli/safeexec v1.0.0 github.com/fatih/color v1.12.0 @@ -23,6 +24,7 @@ require ( github.com/pion/webrtc/v3 v3.0.32 github.com/pkg/browser v0.0.0-20180916011732-0a3d74bf9ce4 github.com/rjeczalik/notify v0.9.2 + github.com/spf13/afero v1.6.0 github.com/spf13/cobra v1.2.1 github.com/stretchr/testify v1.7.0 golang.org/x/crypto v0.0.0-20210711020723-a769d52b0f97 diff --git a/go.sum b/go.sum index de738e5e..dae8f880 100644 --- a/go.sum +++ b/go.sum @@ -69,6 +69,8 @@ github.com/armon/go-metrics v0.0.0-20180917152333-f0300d1749da/go.mod h1:Q73ZrmV github.com/armon/go-radix v0.0.0-20180808171621-7fddfc383310/go.mod h1:ufUuZ+zHj4x4TnLV4JWEpy2hxWSpsRywHrMgIH9cCH8= github.com/bgentry/speakeasy v0.1.0/go.mod h1:+zsyZBPWlz7T6j88CTgSN5bM796AkVf0kBD4zp0CCIs= github.com/bketelsen/crypt v0.0.4/go.mod h1:aI6NrJ0pMGgvZKL1iVgXLnfIFJtfV+bKCoqOes/6LfM= +github.com/blang/semver/v4 v4.0.0 h1:1PFHFE6yCCTv8C1TeyNNarDzntLi7wMI5i/pzqYIsAM= +github.com/blang/semver/v4 v4.0.0/go.mod h1:IbckMUScFkM3pff0VJDNKRiT6TG/YpiHIM2yvyW5YoQ= github.com/briandowns/spinner v1.16.0 h1:DFmp6hEaIx2QXXuqSJmtfSBSAjRmpGiKG6ip2Wm/yOs= github.com/briandowns/spinner v1.16.0/go.mod h1:QOuQk7x+EaDASo80FEXwlwiA+j/PPIcX3FScO+3/ZPQ= github.com/census-instrumentation/opencensus-proto v0.2.1/go.mod h1:f6KPmirojxKA12rnyqOA5BBL4O983OfeGPqjHWSTneU= @@ -376,6 +378,7 @@ github.com/shurcooL/sanitized_anchor_name v1.0.0 h1:PdmoCO6wvbs+7yrJyMORt4/BmY5I github.com/shurcooL/sanitized_anchor_name v1.0.0/go.mod h1:1NzhyTcUVG4SuEtjjoZeVRXNmyL/1OwPU0+IJeTBvfc= github.com/smartystreets/assertions v0.0.0-20180927180507-b2de0cb4f26d/go.mod h1:OnSkiWE9lh6wB0YB77sQom3nweQdgAjqCqsofrRNTgc= github.com/smartystreets/goconvey v1.6.4/go.mod h1:syvi0/a8iFYH4r/RixwvyeAJjdLS9QV7WQ/tjFTllLA= +github.com/spf13/afero v1.6.0 h1:xoax2sJ2DT8S8xA2paPFjDCScCNeWsg75VG0DLRreiY= github.com/spf13/afero v1.6.0/go.mod h1:Ai8FlHk4v/PARR026UzYexafAt9roJ7LcLMAmO6Z93I= github.com/spf13/cast v1.3.1/go.mod h1:Qx5cxh0v+4UWYiBimWS+eyWzqEqokIECu5etghLkUJE= github.com/spf13/cobra v1.2.1 h1:+KmjbUw1hriSNMF55oPrkZcb27aECyrj8V2ytv7kWDw= diff --git a/internal/cmd/cmd.go b/internal/cmd/cmd.go index 26df6bc4..90911c07 100644 --- a/internal/cmd/cmd.go +++ b/internal/cmd/cmd.go @@ -38,6 +38,7 @@ func Make() *cobra.Command { tagsCmd(), tokensCmd(), tunnelCmd(), + updateCmd(), urlCmd(), usersCmd(), workspacesCmd(), diff --git a/internal/cmd/update.go b/internal/cmd/update.go new file mode 100644 index 00000000..dbc67db5 --- /dev/null +++ b/internal/cmd/update.go @@ -0,0 +1,309 @@ +package cmd + +import ( + "archive/tar" + "archive/zip" + "bufio" + "bytes" + "compress/gzip" + "context" + "fmt" + "io" + "net/http" + "net/url" + "os" + "path" + "runtime" + "strings" + "time" + + "cdr.dev/coder-cli/internal/config" + "cdr.dev/coder-cli/internal/version" + "cdr.dev/coder-cli/pkg/clog" + "golang.org/x/xerrors" + + "github.com/blang/semver/v4" + "github.com/manifoldco/promptui" + "github.com/spf13/afero" + "github.com/spf13/cobra" +) + +// updater updates coder-cli. +type updater struct { + confirmF func(label string) (string, error) + executablePath string + fs afero.Fs + httpClient getter + versionF func() string +} + +func updateCmd() *cobra.Command { + var ( + force bool + coderURL string + ) + + cmd := &cobra.Command{ + Use: "update", + Short: "Update coder binary", + Long: "Update coder to the version matching a given coder instance.", + RunE: func(cmd *cobra.Command, args []string) error { + ctx := cmd.Context() + httpClient := &http.Client{ + Timeout: 10 * time.Second, + } + + updater := &updater{ + httpClient: httpClient, + fs: afero.NewOsFs(), + confirmF: defaultConfirm, + versionF: func() string { return version.Version }, + executablePath: os.Args[0], + } + return updater.Run(ctx, force, coderURL) + }, + } + + cmd.Flags().BoolVar(&force, "force", false, "do not prompt for confirmation") + cmd.Flags().StringVar(&coderURL, "coder", "", "coder instance against which to match version") + + return cmd +} + +type getter interface { + Get(url string) (*http.Response, error) +} + +func (u *updater) Run(ctx context.Context, force bool, coderURLString string) error { + // TODO: check under following directories and warn if coder binary is under them: + // * homebrew prefix + // * coder assets root (env CODER_ASSETS_ROOT) + + currentBinaryStat, err := u.fs.Stat(u.executablePath) + if err != nil { + return clog.Fatal("preflight: cannot stat current binary", clog.Causef("%s", err)) + } + + if currentBinaryStat.Mode().Perm()&0222 == 0 { + return clog.Fatal("preflight: missing write permission on current binary") + } + + var coderURL *url.URL + if coderURLString == "" { + coderURL, err = getCoderConfigURL() + if err != nil { + return clog.Fatal( + "Unable to automatically determine coder URL", + clog.Causef(err.Error()), + clog.BlankLine, + clog.Tipf("use --coder to specify coder URL"), + ) + } + } else { + coderURL, err = url.Parse(coderURLString) + if err != nil { + return clog.Fatal("invalid coder URL", err.Error()) + } + } + + desiredVersion, err := getAPIVersionUnauthed(u.httpClient, *coderURL) + if err != nil { + return clog.Fatal("fetch api version", clog.Causef(err.Error())) + } + + clog.LogInfo(fmt.Sprintf("Coder instance at %q reports version %s", coderURL.String(), desiredVersion.String())) + clog.LogInfo(fmt.Sprintf("Current version of coder-cli is %s", version.Version)) + + if currentVersion, err := semver.Make(u.versionF()); err == nil { + if desiredVersion.Compare(currentVersion) == 0 { + clog.LogInfo("Up to date!") + return nil + } + } + + if !force { + label := fmt.Sprintf("Update coder-cli to version %s", desiredVersion.FinalizeVersion()) + if _, err := u.confirmF(label); err != nil { + return clog.Fatal("failed to confirm update", clog.Tipf(`use "--force" to update without confirmation`)) + } + } + + downloadURL := makeDownloadURL(desiredVersion.FinalizeVersion(), runtime.GOOS, runtime.GOARCH) + + var downloadBuf bytes.Buffer + memWriter := bufio.NewWriter(&downloadBuf) + + clog.LogInfo("fetching coder-cli from GitHub releases", downloadURL) + resp, err := u.httpClient.Get(downloadURL) + if err != nil { + return clog.Fatal(fmt.Sprintf("failed to fetch URL %s", downloadURL), clog.Causef(err.Error())) + } + + if resp.StatusCode != http.StatusOK { + return clog.Fatal("failed to fetch release", clog.Causef("URL %s returned status code %d", downloadURL, resp.StatusCode)) + } + + if _, err := io.Copy(memWriter, resp.Body); err != nil { + return clog.Fatal(fmt.Sprintf("failed to download %s", downloadURL), clog.Causef(err.Error())) + } + + _ = resp.Body.Close() + + if err := memWriter.Flush(); err != nil { + return clog.Fatal(fmt.Sprintf("failed to save %s", downloadURL), clog.Causef(err.Error())) + } + + // TODO: validate the checksum of the downloaded file. GitHub does not currently provide this information + // and we do not generate them yet. + updatedBinary, err := extractFromArchive("coder", downloadBuf.Bytes()) + if err != nil { + return clog.Fatal("failed to extract coder binary from archive", clog.Causef(err.Error())) + } + + // We assume the binary is named coder and write it to coder.new + updatedCoderBinaryPath := u.executablePath + ".new" + updatedBin, err := u.fs.OpenFile(updatedCoderBinaryPath, os.O_RDWR|os.O_CREATE|os.O_TRUNC, currentBinaryStat.Mode().Perm()) + if err != nil { + return clog.Fatal("failed to create file for updated coder binary", clog.Causef(err.Error())) + } + + fsWriter := bufio.NewWriter(updatedBin) + if _, err := io.Copy(fsWriter, bytes.NewReader(updatedBinary)); err != nil { + return clog.Fatal("failed to write updated coder binary to disk", clog.Causef(err.Error())) + } + + if err := fsWriter.Flush(); err != nil { + return clog.Fatal("failed to persist updated coder binary to disk", clog.Causef(err.Error())) + } + + if err = u.fs.Rename(updatedCoderBinaryPath, u.executablePath); err != nil { + return clog.Fatal("failed to update coder binary in-place", clog.Causef(err.Error())) + } + + clog.LogSuccess("Updated coder CLI to version " + desiredVersion.FinalizeVersion()) + return nil +} + +func defaultConfirm(label string) (string, error) { + p := promptui.Prompt{IsConfirm: true, Label: label} + return p.Run() +} + +func makeDownloadURL(version, ostype, archetype string) string { + const template = "https://github.com/cdr/coder-cli/releases/download/v%s/coder-cli-%s-%s.%s" + var ext string + switch ostype { + case "linux": + ext = "tar.gz" + default: + ext = ".zip" + } + return fmt.Sprintf(template, version, ostype, archetype, ext) +} + +func extractFromArchive(path string, archive []byte) ([]byte, error) { + contentType := http.DetectContentType(archive) + switch contentType { + case "application/zip": + return extractFromZip(path, archive) + case "application/x-gzip": + return extractFromTgz(path, archive) + default: + return nil, xerrors.Errorf("unknown archive type: %s", contentType) + } +} + +func extractFromZip(path string, archive []byte) ([]byte, error) { + zipReader, err := zip.NewReader(bytes.NewReader(archive), int64(len(archive))) + if err != nil { + return nil, xerrors.Errorf("failed to open zip archive") + } + + var zf *zip.File + for _, f := range zipReader.File { + if f.Name == path { + zf = f + break + } + } + if zf == nil { + return nil, xerrors.Errorf("could not find path %q in zip archive", path) + } + + rc, err := zf.Open() + if err != nil { + return nil, xerrors.Errorf("failed to extract path %q from archive", path) + } + defer rc.Close() + + var b bytes.Buffer + bw := bufio.NewWriter(&b) + if _, err := io.Copy(bw, rc); err != nil { + return nil, xerrors.Errorf("failed to copy path %q to from archive", path) + } + return b.Bytes(), nil +} + +func extractFromTgz(path string, archive []byte) ([]byte, error) { + zr, err := gzip.NewReader(bytes.NewReader(archive)) + if err != nil { + return nil, xerrors.Errorf("failed to gunzip archive") + } + + tr := tar.NewReader(zr) + + var b bytes.Buffer + bw := bufio.NewWriter(&b) + for { + hdr, err := tr.Next() + if err == io.EOF { + break + } + if err != nil { + return nil, xerrors.Errorf("failed to read tar archive: %w", err) + } + fi := hdr.FileInfo() + if fi.Name() == path && fi.Mode().IsRegular() { + _, _ = io.Copy(bw, tr) + break + } + } + + return b.Bytes(), nil +} + +// getCoderConfigURL reads the currently configured coder URL, returning an empty string if not configured. +func getCoderConfigURL() (*url.URL, error) { + urlString, err := config.URL.Read() + if err != nil { + return nil, err + } + configuredURL, err := url.Parse(strings.TrimSpace(urlString)) + if err != nil { + return nil, err + } + return configuredURL, nil +} + +// XXX: coder.Client requires an API key, but we may not be logged into the coder instance for which we +// want to determine the version. We don't need an API key to sniff the version header. +func getAPIVersionUnauthed(client getter, baseURL url.URL) (semver.Version, error) { + baseURL.Path = path.Join(baseURL.Path, "/api") + resp, err := client.Get(baseURL.String()) + if err != nil { + return semver.Version{}, xerrors.Errorf("get %s: %w", baseURL.String(), err) + } + defer resp.Body.Close() + + versionHdr := resp.Header.Get("coder-version") + if versionHdr == "" { + return semver.Version{}, xerrors.Errorf("URL %s response missing coder-version header", baseURL.String()) + } + + version, err := semver.Parse(versionHdr) + if err != nil { + return semver.Version{}, xerrors.Errorf("parsing coder-version header: %w", err) + } + + return version, nil +} diff --git a/internal/cmd/update_test.go b/internal/cmd/update_test.go new file mode 100644 index 00000000..c6762ef6 --- /dev/null +++ b/internal/cmd/update_test.go @@ -0,0 +1,215 @@ +package cmd + +import ( + "bytes" + "context" + "encoding/base64" + "io/fs" + "io/ioutil" + "net/http" + "os" + "strings" + "testing" + + "cdr.dev/slog/sloggers/slogtest/assert" + "github.com/manifoldco/promptui" + "github.com/spf13/afero" +) + +const ( + fakeExePath = "/coder" + fakeCoderURL = "https://my.cdr.dev/" + fakeNewVersion = "1.23.4-rc.5+678-gabcdef-12345678" + fakeOldVersion = "1.22.4-rc.5+678-gabcdef-12345678" + fakeReleaseURL = "https://github.com/cdr/coder-cli/releases/download/v1.23.4/coder-cli-linux-amd64.tar.gz" +) + +func Test_updater_run_noop(t *testing.T) { + fakefs := afero.NewMemMapFs() + httpClient := &fakeGetter{ + GetF: func(url string) (*http.Response, error) { + switch url { + case fakeCoderURL + "/api": + return fakeResponse([]byte{}, 401, "coder-version: "+fakeNewVersion), nil + default: + t.Errorf("unhandled url: %s", url) + t.FailNow() + return nil, nil // this will never happen + } + }, + } + ctx := context.Background() + u := &updater{ + confirmF: nil, // should not be required + executablePath: fakeExePath, + fs: fakefs, + httpClient: httpClient, + versionF: func() string { return fakeNewVersion }, + } + + // write fake executable + fakeFile(fakefs, fakeExePath, 0755, fakeNewVersion) + err := u.Run(ctx, false, fakeCoderURL) + assertFileContent(t, fakefs, fakeExePath, fakeNewVersion) + assert.Success(t, "update coder - noop", err) +} + +func Test_updater_run_changed(t *testing.T) { + fakefs := afero.NewMemMapFs() + httpClient := &fakeGetter{ + GetF: func(url string) (*http.Response, error) { + switch url { + case fakeCoderURL + "/api": + return fakeResponse([]byte{}, 401, "coder-version: "+fakeNewVersion), nil + case fakeReleaseURL: + return fakeResponse(fakeValidTgzBytes, 200), nil + default: + t.Errorf("unhandled url: %s", url) + t.FailNow() + return nil, nil // this will never happen + } + }, + } + ctx := context.Background() + u := &updater{ + confirmF: fakeConfirmYes, + executablePath: fakeExePath, + fs: fakefs, + httpClient: httpClient, + versionF: func() string { return fakeOldVersion }, + } + + // write fake executable + fakeFile(fakefs, fakeExePath, 0644, fakeOldVersion) + err := u.Run(ctx, false, fakeCoderURL) + assertFileContent(t, fakefs, fakeExePath, fakeNewVersion) + assert.Success(t, "update coder - new version", err) +} + +func Test_updater_run_changed_force(t *testing.T) { + fakefs := afero.NewMemMapFs() + fakeCoderURL := "https://my.cdr.dev/" + httpClient := &fakeGetter{ + GetF: func(url string) (*http.Response, error) { + switch url { + case fakeCoderURL + "/api": + return fakeResponse([]byte{}, 401, "coder-version: "+fakeNewVersion), nil + case fakeReleaseURL: + return fakeResponse(fakeValidTgzBytes, 200), nil + default: + t.Errorf("unhandled url: %s", url) + t.FailNow() + return nil, nil // this will never happen + } + }, + } + ctx := context.Background() + u := &updater{ + confirmF: nil, // should not be required + executablePath: fakeExePath, + fs: fakefs, + httpClient: httpClient, + versionF: func() string { return fakeOldVersion }, + } + + // write fake executable + fakeFile(fakefs, fakeExePath, 0644, fakeOldVersion) + err := u.Run(ctx, true, fakeCoderURL) + assertFileContent(t, fakefs, fakeExePath, fakeNewVersion) + assert.Success(t, "update coder - new version", err) +} + +func Test_updater_run_notconfirmed(t *testing.T) { + fakefs := afero.NewMemMapFs() + fakeCoderURL := "https://my.cdr.dev/" + httpClient := &fakeGetter{ + GetF: func(url string) (*http.Response, error) { + switch url { + case fakeCoderURL + "/api": + return fakeResponse([]byte{}, 401, "coder-version: "+fakeNewVersion), nil + default: + t.Errorf("unhandled url: %s", url) + t.FailNow() + return nil, nil // this will never happen + } + }, + } + ctx := context.Background() + u := &updater{ + confirmF: fakeConfirmNo, + executablePath: fakeExePath, + fs: fakefs, + httpClient: httpClient, + versionF: func() string { return fakeOldVersion }, + } + + // write fake executable + fakeFile(fakefs, fakeExePath, 0644, fakeOldVersion) + err := u.Run(ctx, false, fakeCoderURL) + assertFileContent(t, fakefs, fakeExePath, fakeOldVersion) + assert.ErrorContains(t, "update coder - new version", err, "failed to confirm update") +} + +type fakeGetter struct { + GetF func(url string) (*http.Response, error) +} + +func (f *fakeGetter) Get(url string) (*http.Response, error) { + return f.GetF(url) +} + +func fakeConfirmYes(_ string) (string, error) { + return "y", nil +} + +func fakeConfirmNo(_ string) (string, error) { + return "", promptui.ErrAbort +} + +func fakeResponse(body []byte, code int, headers ...string) *http.Response { + resp := &http.Response{} + resp.Body = ioutil.NopCloser(bytes.NewReader(body)) + resp.StatusCode = code + resp.Header = http.Header{} + + for _, e := range headers { + parts := strings.Split(e, ":") + k := strings.ToLower(strings.TrimSpace(parts[0])) + v := strings.ToLower(strings.TrimSpace(strings.Join(parts[1:], ":"))) + resp.Header.Set(k, v) + } + + return resp +} + +//nolint:unparam +func fakeFile(fs afero.Fs, name string, perm fs.FileMode, content string) { + f, err := fs.OpenFile(name, os.O_RDWR|os.O_CREATE|os.O_TRUNC, perm) + if err != nil { + panic(err) + } + defer f.Close() + + _, err = f.Write([]byte(content)) + if err != nil { + panic(err) + } +} + +//nolint:unparam +func assertFileContent(t *testing.T, fs afero.Fs, name string, content string) { + f, err := fs.OpenFile(name, os.O_RDONLY, 0) + assert.Success(t, "open file "+name, err) + defer f.Close() + + b, err := ioutil.ReadAll(f) + assert.Success(t, "read file "+name, err) + + assert.Equal(t, "assert content equal", content, string(b)) +} + +// this is a valid tgz file containing a single file named 'coder' with permissions 0751 +// containing the string "1.23.4-rc.5+678-gabcdef-12345678". +var fakeValidTgzBytes, _ = base64.StdEncoding.DecodeString(`H4sIAAAAAAAAA+3QsQ4CIRCEYR6F3oC7wIqvc3KnpQnq+3tGCwsTK3LN/zWTTDWZuG/XeeluJFlV +s1dqNfnOtyJOi4qllHOuTlSTqPMydNXH43afuvfu3w3jb9qExpRjCb1F2x3qMVymU5uXc9CUi63F +1vsAAAAAAAAAAAAAAAAAAL89AYuL424AKAAA`) diff --git a/internal/cmd/workspaces.go b/internal/cmd/workspaces.go index ff70b64d..d00d6e1d 100644 --- a/internal/cmd/workspaces.go +++ b/internal/cmd/workspaces.go @@ -210,7 +210,7 @@ func (*wsPinger) logSuccess(timeStr, msg string) { fmt.Printf("%s: %s\n", color.New(color.Bold, color.FgGreen).Sprint(timeStr), msg) } -// Only return fatal errors +// Only return fatal errors. func (w *wsPinger) ping(ctx context.Context) error { ctx, cancelFunc := context.WithTimeout(ctx, time.Second*15) defer cancelFunc() diff --git a/internal/version/version.go b/internal/version/version.go index ce1d5de9..8873f158 100644 --- a/internal/version/version.go +++ b/internal/version/version.go @@ -1,5 +1,5 @@ // Package version contains the compile-time injected version string and -// related utiliy methods. +// related utility methods. package version import ( From 972847ea3fbc648508908155b0116c405b8325f1 Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Fri, 13 Aug 2021 10:33:35 +0000 Subject: [PATCH 02/25] internal/cmd/update_test.go: refactor unit tests --- internal/cmd/update_test.go | 243 +++++++++++++++++++----------------- 1 file changed, 129 insertions(+), 114 deletions(-) diff --git a/internal/cmd/update_test.go b/internal/cmd/update_test.go index c6762ef6..5db3df09 100644 --- a/internal/cmd/update_test.go +++ b/internal/cmd/update_test.go @@ -24,139 +24,154 @@ const ( fakeReleaseURL = "https://github.com/cdr/coder-cli/releases/download/v1.23.4/coder-cli-linux-amd64.tar.gz" ) -func Test_updater_run_noop(t *testing.T) { - fakefs := afero.NewMemMapFs() - httpClient := &fakeGetter{ - GetF: func(url string) (*http.Response, error) { - switch url { - case fakeCoderURL + "/api": - return fakeResponse([]byte{}, 401, "coder-version: "+fakeNewVersion), nil - default: - t.Errorf("unhandled url: %s", url) - t.FailNow() - return nil, nil // this will never happen - } - }, - } - ctx := context.Background() - u := &updater{ - confirmF: nil, // should not be required - executablePath: fakeExePath, - fs: fakefs, - httpClient: httpClient, - versionF: func() string { return fakeNewVersion }, +func Test_updater_run(t *testing.T) { + t.Parallel() + + type params struct { + ConfirmF func(string) (string, error) + Ctx context.Context + ExecutablePath string + Fakefs afero.Fs + HttpClient *fakeGetter + VersionF func() string } - // write fake executable - fakeFile(fakefs, fakeExePath, 0755, fakeNewVersion) - err := u.Run(ctx, false, fakeCoderURL) - assertFileContent(t, fakefs, fakeExePath, fakeNewVersion) - assert.Success(t, "update coder - noop", err) -} + fromParams := func(p *params) *updater { + return &updater{ + confirmF: p.ConfirmF, + executablePath: p.ExecutablePath, + fs: p.Fakefs, + httpClient: p.HttpClient, + versionF: p.VersionF, + } + } -func Test_updater_run_changed(t *testing.T) { - fakefs := afero.NewMemMapFs() - httpClient := &fakeGetter{ - GetF: func(url string) (*http.Response, error) { - switch url { - case fakeCoderURL + "/api": - return fakeResponse([]byte{}, 401, "coder-version: "+fakeNewVersion), nil - case fakeReleaseURL: - return fakeResponse(fakeValidTgzBytes, 200), nil - default: - t.Errorf("unhandled url: %s", url) + run := func(t *testing.T, name string, fn func(t *testing.T, p *params)) { + t.Logf("running %s", name) + ctx := context.Background() + fakefs := afero.NewMemMapFs() + params := ¶ms{ + // This must be overridden inside run() + ConfirmF: func(string) (string, error) { + t.Errorf("unhandled ConfirmF") t.FailNow() - return nil, nil // this will never happen - } - }, - } - ctx := context.Background() - u := &updater{ - confirmF: fakeConfirmYes, - executablePath: fakeExePath, - fs: fakefs, - httpClient: httpClient, - versionF: func() string { return fakeOldVersion }, + return "", nil + }, + Ctx: ctx, + ExecutablePath: fakeExePath, + Fakefs: fakefs, + HttpClient: newFakeGetter(t), + // This must be overridden inside run() + VersionF: func() string { + t.Errorf("unhandled VersionF") + t.FailNow() + return "" + }, + } + + fn(t, params) } - // write fake executable - fakeFile(fakefs, fakeExePath, 0644, fakeOldVersion) - err := u.Run(ctx, false, fakeCoderURL) - assertFileContent(t, fakefs, fakeExePath, fakeNewVersion) - assert.Success(t, "update coder - new version", err) + run(t, "update coder - noop", func(t *testing.T, p *params) { + fakeFile(p.Fakefs, fakeExePath, 0755, fakeNewVersion) + p.HttpClient.M[fakeCoderURL+"/api"] = newFakeGetterResponse([]byte{}, 401, variadicS("coder-version: "+fakeNewVersion), nil) + p.VersionF = func() string { return fakeNewVersion } + u := fromParams(p) + err := u.Run(p.Ctx, false, fakeCoderURL) + assert.Success(t, "update coder - noop", err) + assertFileContent(t, p.Fakefs, fakeExePath, fakeNewVersion) + }) + + run(t, "update coder - old to new", func(t *testing.T, p *params) { + fakeFile(p.Fakefs, fakeExePath, 0755, fakeOldVersion) + p.HttpClient.M[fakeCoderURL+"/api"] = newFakeGetterResponse([]byte{}, 401, variadicS("coder-version: "+fakeNewVersion), nil) + p.HttpClient.M[fakeReleaseURL] = newFakeGetterResponse(fakeValidTgzBytes, 200, variadicS(), nil) + p.VersionF = func() string { return fakeOldVersion } + p.ConfirmF = func(string) (string, error) { return "", nil } + u := fromParams(p) + assertFileContent(t, p.Fakefs, fakeExePath, fakeOldVersion) + err := u.Run(p.Ctx, false, fakeCoderURL) + assert.Success(t, "update coder - old to new", err) + assertFileContent(t, p.Fakefs, fakeExePath, fakeNewVersion) + }) + + run(t, "update coder - old to new forced", func(t *testing.T, p *params) { + fakeFile(p.Fakefs, fakeExePath, 0755, fakeOldVersion) + p.HttpClient.M[fakeCoderURL+"/api"] = newFakeGetterResponse([]byte{}, 401, variadicS("coder-version: "+fakeNewVersion), nil) + p.HttpClient.M[fakeReleaseURL] = newFakeGetterResponse(fakeValidTgzBytes, 200, variadicS(), nil) + p.VersionF = func() string { return fakeOldVersion } + u := fromParams(p) + assertFileContent(t, p.Fakefs, fakeExePath, fakeOldVersion) + err := u.Run(p.Ctx, true, fakeCoderURL) + assert.Success(t, "update coder - old to new", err) + assertFileContent(t, p.Fakefs, fakeExePath, fakeNewVersion) + }) + + run(t, "update coder - user cancelled", func(t *testing.T, p *params) { + fakeFile(p.Fakefs, fakeExePath, 0755, fakeOldVersion) + p.HttpClient.M[fakeCoderURL+"/api"] = newFakeGetterResponse([]byte{}, 401, variadicS("coder-version: "+fakeNewVersion), nil) + p.VersionF = func() string { return fakeOldVersion } + p.ConfirmF = func(string) (string, error) { return "", promptui.ErrAbort } + u := fromParams(p) + assertFileContent(t, p.Fakefs, fakeExePath, fakeOldVersion) + err := u.Run(p.Ctx, false, fakeCoderURL) + assert.ErrorContains(t, "update coder - user cancelled", err, "failed to confirm update") + assertFileContent(t, p.Fakefs, fakeExePath, fakeOldVersion) + }) } -func Test_updater_run_changed_force(t *testing.T) { - fakefs := afero.NewMemMapFs() - fakeCoderURL := "https://my.cdr.dev/" - httpClient := &fakeGetter{ - GetF: func(url string) (*http.Response, error) { - switch url { - case fakeCoderURL + "/api": - return fakeResponse([]byte{}, 401, "coder-version: "+fakeNewVersion), nil - case fakeReleaseURL: - return fakeResponse(fakeValidTgzBytes, 200), nil - default: - t.Errorf("unhandled url: %s", url) - t.FailNow() - return nil, nil // this will never happen - } - }, +type fakeGetter struct { + M map[string]*fakeGetterResponse + T *testing.T +} + +func newFakeGetter(t *testing.T) *fakeGetter { + return &fakeGetter{ + M: make(map[string]*fakeGetterResponse), } - ctx := context.Background() - u := &updater{ - confirmF: nil, // should not be required - executablePath: fakeExePath, - fs: fakefs, - httpClient: httpClient, - versionF: func() string { return fakeOldVersion }, +} + +func (f *fakeGetter) Get(url string) (*http.Response, error) { + val, ok := f.M[url] + if !ok { + f.T.Errorf("unhandled url: %s", url) + f.T.FailNow() + return nil, nil // this will never happen } + return val.Resp, val.Err +} - // write fake executable - fakeFile(fakefs, fakeExePath, 0644, fakeOldVersion) - err := u.Run(ctx, true, fakeCoderURL) - assertFileContent(t, fakefs, fakeExePath, fakeNewVersion) - assert.Success(t, "update coder - new version", err) +type fakeGetterResponse struct { + Resp *http.Response + Err error } -func Test_updater_run_notconfirmed(t *testing.T) { - fakefs := afero.NewMemMapFs() - fakeCoderURL := "https://my.cdr.dev/" - httpClient := &fakeGetter{ - GetF: func(url string) (*http.Response, error) { - switch url { - case fakeCoderURL + "/api": - return fakeResponse([]byte{}, 401, "coder-version: "+fakeNewVersion), nil - default: - t.Errorf("unhandled url: %s", url) - t.FailNow() - return nil, nil // this will never happen - } - }, - } - ctx := context.Background() - u := &updater{ - confirmF: fakeConfirmNo, - executablePath: fakeExePath, - fs: fakefs, - httpClient: httpClient, - versionF: func() string { return fakeOldVersion }, +func newFakeGetterResponse(body []byte, code int, headers []string, err error) *fakeGetterResponse { + resp := &http.Response{} + resp.Body = ioutil.NopCloser(bytes.NewReader(body)) + resp.StatusCode = code + resp.Header = http.Header{} + + for _, e := range headers { + parts := strings.Split(e, ":") + k := strings.ToLower(strings.TrimSpace(parts[0])) + v := strings.ToLower(strings.TrimSpace(strings.Join(parts[1:], ":"))) + resp.Header.Set(k, v) } - // write fake executable - fakeFile(fakefs, fakeExePath, 0644, fakeOldVersion) - err := u.Run(ctx, false, fakeCoderURL) - assertFileContent(t, fakefs, fakeExePath, fakeOldVersion) - assert.ErrorContains(t, "update coder - new version", err, "failed to confirm update") + return &fakeGetterResponse{ + Resp: resp, + Err: err, + } } -type fakeGetter struct { - GetF func(url string) (*http.Response, error) +func variadicS(s ...string) []string { + return s } -func (f *fakeGetter) Get(url string) (*http.Response, error) { - return f.GetF(url) -} +// func (f *fakeGetter) Get(url string) (*http.Response, error) { +// return f.GetF(url) +// } func fakeConfirmYes(_ string) (string, error) { return "y", nil From 597afe1165b0e34df5ae55934cd5b1da6bd6727e Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Fri, 13 Aug 2021 10:39:49 +0000 Subject: [PATCH 03/25] fixup! internal/cmd/update_test.go: refactor unit tests --- internal/cmd/update_test.go | 44 ++++++++++++++----------------------- 1 file changed, 16 insertions(+), 28 deletions(-) diff --git a/internal/cmd/update_test.go b/internal/cmd/update_test.go index 5db3df09..1100950b 100644 --- a/internal/cmd/update_test.go +++ b/internal/cmd/update_test.go @@ -27,6 +27,7 @@ const ( func Test_updater_run(t *testing.T) { t.Parallel() + // params holds parameters for each test case type params struct { ConfirmF func(string) (string, error) Ctx context.Context @@ -36,6 +37,7 @@ func Test_updater_run(t *testing.T) { VersionF func() string } + // fromParams creates a new updater from params fromParams := func(p *params) *updater { return &updater{ confirmF: p.ConfirmF, @@ -73,21 +75,22 @@ func Test_updater_run(t *testing.T) { } run(t, "update coder - noop", func(t *testing.T, p *params) { - fakeFile(p.Fakefs, fakeExePath, 0755, fakeNewVersion) + fakeFile(t, p.Fakefs, fakeExePath, 0755, fakeNewVersion) p.HttpClient.M[fakeCoderURL+"/api"] = newFakeGetterResponse([]byte{}, 401, variadicS("coder-version: "+fakeNewVersion), nil) p.VersionF = func() string { return fakeNewVersion } u := fromParams(p) + assertFileContent(t, p.Fakefs, fakeExePath, fakeNewVersion) err := u.Run(p.Ctx, false, fakeCoderURL) assert.Success(t, "update coder - noop", err) assertFileContent(t, p.Fakefs, fakeExePath, fakeNewVersion) }) run(t, "update coder - old to new", func(t *testing.T, p *params) { - fakeFile(p.Fakefs, fakeExePath, 0755, fakeOldVersion) + fakeFile(t, p.Fakefs, fakeExePath, 0755, fakeOldVersion) p.HttpClient.M[fakeCoderURL+"/api"] = newFakeGetterResponse([]byte{}, 401, variadicS("coder-version: "+fakeNewVersion), nil) p.HttpClient.M[fakeReleaseURL] = newFakeGetterResponse(fakeValidTgzBytes, 200, variadicS(), nil) p.VersionF = func() string { return fakeOldVersion } - p.ConfirmF = func(string) (string, error) { return "", nil } + p.ConfirmF = fakeConfirmYes u := fromParams(p) assertFileContent(t, p.Fakefs, fakeExePath, fakeOldVersion) err := u.Run(p.Ctx, false, fakeCoderURL) @@ -96,22 +99,22 @@ func Test_updater_run(t *testing.T) { }) run(t, "update coder - old to new forced", func(t *testing.T, p *params) { - fakeFile(p.Fakefs, fakeExePath, 0755, fakeOldVersion) + fakeFile(t, p.Fakefs, fakeExePath, 0755, fakeOldVersion) p.HttpClient.M[fakeCoderURL+"/api"] = newFakeGetterResponse([]byte{}, 401, variadicS("coder-version: "+fakeNewVersion), nil) p.HttpClient.M[fakeReleaseURL] = newFakeGetterResponse(fakeValidTgzBytes, 200, variadicS(), nil) p.VersionF = func() string { return fakeOldVersion } u := fromParams(p) assertFileContent(t, p.Fakefs, fakeExePath, fakeOldVersion) err := u.Run(p.Ctx, true, fakeCoderURL) - assert.Success(t, "update coder - old to new", err) + assert.Success(t, "update coder - old to new forced", err) assertFileContent(t, p.Fakefs, fakeExePath, fakeNewVersion) }) run(t, "update coder - user cancelled", func(t *testing.T, p *params) { - fakeFile(p.Fakefs, fakeExePath, 0755, fakeOldVersion) + fakeFile(t, p.Fakefs, fakeExePath, 0755, fakeOldVersion) p.HttpClient.M[fakeCoderURL+"/api"] = newFakeGetterResponse([]byte{}, 401, variadicS("coder-version: "+fakeNewVersion), nil) p.VersionF = func() string { return fakeOldVersion } - p.ConfirmF = func(string) (string, error) { return "", promptui.ErrAbort } + p.ConfirmF = fakeConfirmNo u := fromParams(p) assertFileContent(t, p.Fakefs, fakeExePath, fakeOldVersion) err := u.Run(p.Ctx, false, fakeCoderURL) @@ -120,6 +123,7 @@ func Test_updater_run(t *testing.T) { }) } +// fakeGetter mocks HTTP requests type fakeGetter struct { M map[string]*fakeGetterResponse T *testing.T @@ -131,6 +135,7 @@ func newFakeGetter(t *testing.T) *fakeGetter { } } +// Get returns the configured response for url. If no response configured, test fails immediately. func (f *fakeGetter) Get(url string) (*http.Response, error) { val, ok := f.M[url] if !ok { @@ -146,6 +151,7 @@ type fakeGetterResponse struct { Err error } +// newFakeGetterResponse is a convenience function for mocking HTTP requests func newFakeGetterResponse(body []byte, code int, headers []string, err error) *fakeGetterResponse { resp := &http.Response{} resp.Body = ioutil.NopCloser(bytes.NewReader(body)) @@ -169,10 +175,6 @@ func variadicS(s ...string) []string { return s } -// func (f *fakeGetter) Get(url string) (*http.Response, error) { -// return f.GetF(url) -// } - func fakeConfirmYes(_ string) (string, error) { return "y", nil } @@ -181,24 +183,9 @@ func fakeConfirmNo(_ string) (string, error) { return "", promptui.ErrAbort } -func fakeResponse(body []byte, code int, headers ...string) *http.Response { - resp := &http.Response{} - resp.Body = ioutil.NopCloser(bytes.NewReader(body)) - resp.StatusCode = code - resp.Header = http.Header{} - - for _, e := range headers { - parts := strings.Split(e, ":") - k := strings.ToLower(strings.TrimSpace(parts[0])) - v := strings.ToLower(strings.TrimSpace(strings.Join(parts[1:], ":"))) - resp.Header.Set(k, v) - } - - return resp -} - //nolint:unparam -func fakeFile(fs afero.Fs, name string, perm fs.FileMode, content string) { +func fakeFile(t *testing.T, fs afero.Fs, name string, perm fs.FileMode, content string) { + t.Helper() f, err := fs.OpenFile(name, os.O_RDWR|os.O_CREATE|os.O_TRUNC, perm) if err != nil { panic(err) @@ -213,6 +200,7 @@ func fakeFile(fs afero.Fs, name string, perm fs.FileMode, content string) { //nolint:unparam func assertFileContent(t *testing.T, fs afero.Fs, name string, content string) { + t.Helper() f, err := fs.OpenFile(name, os.O_RDONLY, 0) assert.Success(t, "open file "+name, err) defer f.Close() From 1373e79f8d2dbe3979d350de2a7cb66b5878d2ba Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Fri, 13 Aug 2021 11:33:36 +0000 Subject: [PATCH 04/25] internal/cmd/update_test.go: more tests --- internal/cmd/update_test.go | 92 +++++++++++++++++++++++++++++++++++++ 1 file changed, 92 insertions(+) diff --git a/internal/cmd/update_test.go b/internal/cmd/update_test.go index 1100950b..8018aa8a 100644 --- a/internal/cmd/update_test.go +++ b/internal/cmd/update_test.go @@ -6,6 +6,7 @@ import ( "encoding/base64" "io/fs" "io/ioutil" + "net" "net/http" "os" "strings" @@ -121,6 +122,96 @@ func Test_updater_run(t *testing.T) { assert.ErrorContains(t, "update coder - user cancelled", err, "failed to confirm update") assertFileContent(t, p.Fakefs, fakeExePath, fakeOldVersion) }) + + run(t, "update coder - cannot stat", func(t *testing.T, p *params) { + u := fromParams(p) + err := u.Run(p.Ctx, false, fakeCoderURL) + assert.ErrorContains(t, "update coder - cannot stat", err, "cannot stat current binary") + }) + + run(t, "update coder - no permission", func(t *testing.T, p *params) { + fakeFile(t, p.Fakefs, fakeExePath, 0400, fakeOldVersion) + u := fromParams(p) + assertFileContent(t, p.Fakefs, fakeExePath, fakeOldVersion) + err := u.Run(p.Ctx, false, fakeCoderURL) + assert.ErrorContains(t, "update coder - no permission", err, "missing write permission") + assertFileContent(t, p.Fakefs, fakeExePath, fakeOldVersion) + }) + + run(t, "update coder - invalid url", func(t *testing.T, p *params) { + fakeFile(t, p.Fakefs, fakeExePath, 0755, fakeOldVersion) + p.VersionF = func() string { return fakeOldVersion } + u := fromParams(p) + assertFileContent(t, p.Fakefs, fakeExePath, fakeOldVersion) + err := u.Run(p.Ctx, false, "h$$p://invalid.url") + assert.ErrorContains(t, "update coder - invalid url", err, "invalid coder URL") + assertFileContent(t, p.Fakefs, fakeExePath, fakeOldVersion) + }) + + run(t, "update coder - fetch api version failure", func(t *testing.T, p *params) { + fakeFile(t, p.Fakefs, fakeExePath, 0755, fakeOldVersion) + p.HttpClient.M[fakeCoderURL+"/api"] = newFakeGetterResponse([]byte{}, 401, variadicS(), net.ErrClosed) + p.VersionF = func() string { return fakeOldVersion } + u := fromParams(p) + assertFileContent(t, p.Fakefs, fakeExePath, fakeOldVersion) + err := u.Run(p.Ctx, false, fakeCoderURL) + assert.ErrorContains(t, "update coder - fetch api version failure", err, "fetch api version") + assertFileContent(t, p.Fakefs, fakeExePath, fakeOldVersion) + }) + + run(t, "update coder - failed to fetch URL", func(t *testing.T, p *params) { + fakeFile(t, p.Fakefs, fakeExePath, 0755, fakeOldVersion) + p.HttpClient.M[fakeCoderURL+"/api"] = newFakeGetterResponse([]byte{}, 401, variadicS("coder-version: "+fakeNewVersion), nil) + p.HttpClient.M[fakeReleaseURL] = newFakeGetterResponse([]byte{}, 0, variadicS(), net.ErrClosed) + p.VersionF = func() string { return fakeOldVersion } + p.ConfirmF = fakeConfirmYes + u := fromParams(p) + assertFileContent(t, p.Fakefs, fakeExePath, fakeOldVersion) + err := u.Run(p.Ctx, false, fakeCoderURL) + assert.ErrorContains(t, "update coder - release URL 404", err, "failed to fetch URL") + assertFileContent(t, p.Fakefs, fakeExePath, fakeOldVersion) + }) + + run(t, "update coder - release URL 404", func(t *testing.T, p *params) { + fakeFile(t, p.Fakefs, fakeExePath, 0755, fakeOldVersion) + p.HttpClient.M[fakeCoderURL+"/api"] = newFakeGetterResponse([]byte{}, 401, variadicS("coder-version: "+fakeNewVersion), nil) + p.HttpClient.M[fakeReleaseURL] = newFakeGetterResponse([]byte{}, 404, variadicS(), nil) + p.VersionF = func() string { return fakeOldVersion } + p.ConfirmF = fakeConfirmYes + u := fromParams(p) + assertFileContent(t, p.Fakefs, fakeExePath, fakeOldVersion) + err := u.Run(p.Ctx, false, fakeCoderURL) + assert.ErrorContains(t, "update coder - release URL 404", err, "failed to fetch release") + assertFileContent(t, p.Fakefs, fakeExePath, fakeOldVersion) + }) + + run(t, "update coder - invalid archive", func(t *testing.T, p *params) { + fakeFile(t, p.Fakefs, fakeExePath, 0755, fakeOldVersion) + p.HttpClient.M[fakeCoderURL+"/api"] = newFakeGetterResponse([]byte{}, 401, variadicS("coder-version: "+fakeNewVersion), nil) + p.HttpClient.M[fakeReleaseURL] = newFakeGetterResponse([]byte{}, 200, variadicS(), nil) + p.VersionF = func() string { return fakeOldVersion } + p.ConfirmF = fakeConfirmYes + u := fromParams(p) + assertFileContent(t, p.Fakefs, fakeExePath, fakeOldVersion) + err := u.Run(p.Ctx, false, fakeCoderURL) + assert.ErrorContains(t, "update coder - invalid archive", err, "failed to extract coder binary from archive") + assertFileContent(t, p.Fakefs, fakeExePath, fakeOldVersion) + }) + + run(t, "update coder - read-only fs", func(t *testing.T, p *params) { + rwfs := p.Fakefs + p.Fakefs = afero.NewReadOnlyFs(rwfs) + fakeFile(t, rwfs, fakeExePath, 0755, fakeOldVersion) + p.HttpClient.M[fakeCoderURL+"/api"] = newFakeGetterResponse([]byte{}, 401, variadicS("coder-version: "+fakeNewVersion), nil) + p.HttpClient.M[fakeReleaseURL] = newFakeGetterResponse(fakeValidTgzBytes, 200, variadicS(), nil) + p.VersionF = func() string { return fakeOldVersion } + p.ConfirmF = fakeConfirmYes + u := fromParams(p) + assertFileContent(t, p.Fakefs, fakeExePath, fakeOldVersion) + err := u.Run(p.Ctx, false, fakeCoderURL) + assert.ErrorContains(t, "update coder - read-only fs", err, "failed to create file") + assertFileContent(t, p.Fakefs, fakeExePath, fakeOldVersion) + }) } // fakeGetter mocks HTTP requests @@ -132,6 +223,7 @@ type fakeGetter struct { func newFakeGetter(t *testing.T) *fakeGetter { return &fakeGetter{ M: make(map[string]*fakeGetterResponse), + T: t, } } From 513282af1c9b3ce47bdfea8446bc6579dfda9b1c Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Fri, 13 Aug 2021 11:42:28 +0000 Subject: [PATCH 05/25] internal/cmd/update_test.go: create dirs in memfs --- internal/cmd/update_test.go | 83 ++++++++++++++++++++----------------- 1 file changed, 44 insertions(+), 39 deletions(-) diff --git a/internal/cmd/update_test.go b/internal/cmd/update_test.go index 8018aa8a..263a81b8 100644 --- a/internal/cmd/update_test.go +++ b/internal/cmd/update_test.go @@ -9,6 +9,7 @@ import ( "net" "net/http" "os" + "path/filepath" "strings" "testing" @@ -18,11 +19,11 @@ import ( ) const ( - fakeExePath = "/coder" - fakeCoderURL = "https://my.cdr.dev/" - fakeNewVersion = "1.23.4-rc.5+678-gabcdef-12345678" - fakeOldVersion = "1.22.4-rc.5+678-gabcdef-12345678" - fakeReleaseURL = "https://github.com/cdr/coder-cli/releases/download/v1.23.4/coder-cli-linux-amd64.tar.gz" + fakeExePathLinux = "/home/user/bin/coder" + fakeCoderURL = "https://my.cdr.dev/" + fakeNewVersion = "1.23.4-rc.5+678-gabcdef-12345678" + fakeOldVersion = "1.22.4-rc.5+678-gabcdef-12345678" + fakeReleaseURL = "https://github.com/cdr/coder-cli/releases/download/v1.23.4/coder-cli-linux-amd64.tar.gz" ) func Test_updater_run(t *testing.T) { @@ -61,7 +62,7 @@ func Test_updater_run(t *testing.T) { return "", nil }, Ctx: ctx, - ExecutablePath: fakeExePath, + ExecutablePath: fakeExePathLinux, Fakefs: fakefs, HttpClient: newFakeGetter(t), // This must be overridden inside run() @@ -76,51 +77,51 @@ func Test_updater_run(t *testing.T) { } run(t, "update coder - noop", func(t *testing.T, p *params) { - fakeFile(t, p.Fakefs, fakeExePath, 0755, fakeNewVersion) + fakeFile(t, p.Fakefs, fakeExePathLinux, 0755, fakeNewVersion) p.HttpClient.M[fakeCoderURL+"/api"] = newFakeGetterResponse([]byte{}, 401, variadicS("coder-version: "+fakeNewVersion), nil) p.VersionF = func() string { return fakeNewVersion } u := fromParams(p) - assertFileContent(t, p.Fakefs, fakeExePath, fakeNewVersion) + assertFileContent(t, p.Fakefs, fakeExePathLinux, fakeNewVersion) err := u.Run(p.Ctx, false, fakeCoderURL) assert.Success(t, "update coder - noop", err) - assertFileContent(t, p.Fakefs, fakeExePath, fakeNewVersion) + assertFileContent(t, p.Fakefs, fakeExePathLinux, fakeNewVersion) }) run(t, "update coder - old to new", func(t *testing.T, p *params) { - fakeFile(t, p.Fakefs, fakeExePath, 0755, fakeOldVersion) + fakeFile(t, p.Fakefs, fakeExePathLinux, 0755, fakeOldVersion) p.HttpClient.M[fakeCoderURL+"/api"] = newFakeGetterResponse([]byte{}, 401, variadicS("coder-version: "+fakeNewVersion), nil) p.HttpClient.M[fakeReleaseURL] = newFakeGetterResponse(fakeValidTgzBytes, 200, variadicS(), nil) p.VersionF = func() string { return fakeOldVersion } p.ConfirmF = fakeConfirmYes u := fromParams(p) - assertFileContent(t, p.Fakefs, fakeExePath, fakeOldVersion) + assertFileContent(t, p.Fakefs, fakeExePathLinux, fakeOldVersion) err := u.Run(p.Ctx, false, fakeCoderURL) assert.Success(t, "update coder - old to new", err) - assertFileContent(t, p.Fakefs, fakeExePath, fakeNewVersion) + assertFileContent(t, p.Fakefs, fakeExePathLinux, fakeNewVersion) }) run(t, "update coder - old to new forced", func(t *testing.T, p *params) { - fakeFile(t, p.Fakefs, fakeExePath, 0755, fakeOldVersion) + fakeFile(t, p.Fakefs, fakeExePathLinux, 0755, fakeOldVersion) p.HttpClient.M[fakeCoderURL+"/api"] = newFakeGetterResponse([]byte{}, 401, variadicS("coder-version: "+fakeNewVersion), nil) p.HttpClient.M[fakeReleaseURL] = newFakeGetterResponse(fakeValidTgzBytes, 200, variadicS(), nil) p.VersionF = func() string { return fakeOldVersion } u := fromParams(p) - assertFileContent(t, p.Fakefs, fakeExePath, fakeOldVersion) + assertFileContent(t, p.Fakefs, fakeExePathLinux, fakeOldVersion) err := u.Run(p.Ctx, true, fakeCoderURL) assert.Success(t, "update coder - old to new forced", err) - assertFileContent(t, p.Fakefs, fakeExePath, fakeNewVersion) + assertFileContent(t, p.Fakefs, fakeExePathLinux, fakeNewVersion) }) run(t, "update coder - user cancelled", func(t *testing.T, p *params) { - fakeFile(t, p.Fakefs, fakeExePath, 0755, fakeOldVersion) + fakeFile(t, p.Fakefs, fakeExePathLinux, 0755, fakeOldVersion) p.HttpClient.M[fakeCoderURL+"/api"] = newFakeGetterResponse([]byte{}, 401, variadicS("coder-version: "+fakeNewVersion), nil) p.VersionF = func() string { return fakeOldVersion } p.ConfirmF = fakeConfirmNo u := fromParams(p) - assertFileContent(t, p.Fakefs, fakeExePath, fakeOldVersion) + assertFileContent(t, p.Fakefs, fakeExePathLinux, fakeOldVersion) err := u.Run(p.Ctx, false, fakeCoderURL) assert.ErrorContains(t, "update coder - user cancelled", err, "failed to confirm update") - assertFileContent(t, p.Fakefs, fakeExePath, fakeOldVersion) + assertFileContent(t, p.Fakefs, fakeExePathLinux, fakeOldVersion) }) run(t, "update coder - cannot stat", func(t *testing.T, p *params) { @@ -130,87 +131,87 @@ func Test_updater_run(t *testing.T) { }) run(t, "update coder - no permission", func(t *testing.T, p *params) { - fakeFile(t, p.Fakefs, fakeExePath, 0400, fakeOldVersion) + fakeFile(t, p.Fakefs, fakeExePathLinux, 0400, fakeOldVersion) u := fromParams(p) - assertFileContent(t, p.Fakefs, fakeExePath, fakeOldVersion) + assertFileContent(t, p.Fakefs, fakeExePathLinux, fakeOldVersion) err := u.Run(p.Ctx, false, fakeCoderURL) assert.ErrorContains(t, "update coder - no permission", err, "missing write permission") - assertFileContent(t, p.Fakefs, fakeExePath, fakeOldVersion) + assertFileContent(t, p.Fakefs, fakeExePathLinux, fakeOldVersion) }) run(t, "update coder - invalid url", func(t *testing.T, p *params) { - fakeFile(t, p.Fakefs, fakeExePath, 0755, fakeOldVersion) + fakeFile(t, p.Fakefs, fakeExePathLinux, 0755, fakeOldVersion) p.VersionF = func() string { return fakeOldVersion } u := fromParams(p) - assertFileContent(t, p.Fakefs, fakeExePath, fakeOldVersion) + assertFileContent(t, p.Fakefs, fakeExePathLinux, fakeOldVersion) err := u.Run(p.Ctx, false, "h$$p://invalid.url") assert.ErrorContains(t, "update coder - invalid url", err, "invalid coder URL") - assertFileContent(t, p.Fakefs, fakeExePath, fakeOldVersion) + assertFileContent(t, p.Fakefs, fakeExePathLinux, fakeOldVersion) }) run(t, "update coder - fetch api version failure", func(t *testing.T, p *params) { - fakeFile(t, p.Fakefs, fakeExePath, 0755, fakeOldVersion) + fakeFile(t, p.Fakefs, fakeExePathLinux, 0755, fakeOldVersion) p.HttpClient.M[fakeCoderURL+"/api"] = newFakeGetterResponse([]byte{}, 401, variadicS(), net.ErrClosed) p.VersionF = func() string { return fakeOldVersion } u := fromParams(p) - assertFileContent(t, p.Fakefs, fakeExePath, fakeOldVersion) + assertFileContent(t, p.Fakefs, fakeExePathLinux, fakeOldVersion) err := u.Run(p.Ctx, false, fakeCoderURL) assert.ErrorContains(t, "update coder - fetch api version failure", err, "fetch api version") - assertFileContent(t, p.Fakefs, fakeExePath, fakeOldVersion) + assertFileContent(t, p.Fakefs, fakeExePathLinux, fakeOldVersion) }) run(t, "update coder - failed to fetch URL", func(t *testing.T, p *params) { - fakeFile(t, p.Fakefs, fakeExePath, 0755, fakeOldVersion) + fakeFile(t, p.Fakefs, fakeExePathLinux, 0755, fakeOldVersion) p.HttpClient.M[fakeCoderURL+"/api"] = newFakeGetterResponse([]byte{}, 401, variadicS("coder-version: "+fakeNewVersion), nil) p.HttpClient.M[fakeReleaseURL] = newFakeGetterResponse([]byte{}, 0, variadicS(), net.ErrClosed) p.VersionF = func() string { return fakeOldVersion } p.ConfirmF = fakeConfirmYes u := fromParams(p) - assertFileContent(t, p.Fakefs, fakeExePath, fakeOldVersion) + assertFileContent(t, p.Fakefs, fakeExePathLinux, fakeOldVersion) err := u.Run(p.Ctx, false, fakeCoderURL) assert.ErrorContains(t, "update coder - release URL 404", err, "failed to fetch URL") - assertFileContent(t, p.Fakefs, fakeExePath, fakeOldVersion) + assertFileContent(t, p.Fakefs, fakeExePathLinux, fakeOldVersion) }) run(t, "update coder - release URL 404", func(t *testing.T, p *params) { - fakeFile(t, p.Fakefs, fakeExePath, 0755, fakeOldVersion) + fakeFile(t, p.Fakefs, fakeExePathLinux, 0755, fakeOldVersion) p.HttpClient.M[fakeCoderURL+"/api"] = newFakeGetterResponse([]byte{}, 401, variadicS("coder-version: "+fakeNewVersion), nil) p.HttpClient.M[fakeReleaseURL] = newFakeGetterResponse([]byte{}, 404, variadicS(), nil) p.VersionF = func() string { return fakeOldVersion } p.ConfirmF = fakeConfirmYes u := fromParams(p) - assertFileContent(t, p.Fakefs, fakeExePath, fakeOldVersion) + assertFileContent(t, p.Fakefs, fakeExePathLinux, fakeOldVersion) err := u.Run(p.Ctx, false, fakeCoderURL) assert.ErrorContains(t, "update coder - release URL 404", err, "failed to fetch release") - assertFileContent(t, p.Fakefs, fakeExePath, fakeOldVersion) + assertFileContent(t, p.Fakefs, fakeExePathLinux, fakeOldVersion) }) run(t, "update coder - invalid archive", func(t *testing.T, p *params) { - fakeFile(t, p.Fakefs, fakeExePath, 0755, fakeOldVersion) + fakeFile(t, p.Fakefs, fakeExePathLinux, 0755, fakeOldVersion) p.HttpClient.M[fakeCoderURL+"/api"] = newFakeGetterResponse([]byte{}, 401, variadicS("coder-version: "+fakeNewVersion), nil) p.HttpClient.M[fakeReleaseURL] = newFakeGetterResponse([]byte{}, 200, variadicS(), nil) p.VersionF = func() string { return fakeOldVersion } p.ConfirmF = fakeConfirmYes u := fromParams(p) - assertFileContent(t, p.Fakefs, fakeExePath, fakeOldVersion) + assertFileContent(t, p.Fakefs, fakeExePathLinux, fakeOldVersion) err := u.Run(p.Ctx, false, fakeCoderURL) assert.ErrorContains(t, "update coder - invalid archive", err, "failed to extract coder binary from archive") - assertFileContent(t, p.Fakefs, fakeExePath, fakeOldVersion) + assertFileContent(t, p.Fakefs, fakeExePathLinux, fakeOldVersion) }) run(t, "update coder - read-only fs", func(t *testing.T, p *params) { rwfs := p.Fakefs p.Fakefs = afero.NewReadOnlyFs(rwfs) - fakeFile(t, rwfs, fakeExePath, 0755, fakeOldVersion) + fakeFile(t, rwfs, fakeExePathLinux, 0755, fakeOldVersion) p.HttpClient.M[fakeCoderURL+"/api"] = newFakeGetterResponse([]byte{}, 401, variadicS("coder-version: "+fakeNewVersion), nil) p.HttpClient.M[fakeReleaseURL] = newFakeGetterResponse(fakeValidTgzBytes, 200, variadicS(), nil) p.VersionF = func() string { return fakeOldVersion } p.ConfirmF = fakeConfirmYes u := fromParams(p) - assertFileContent(t, p.Fakefs, fakeExePath, fakeOldVersion) + assertFileContent(t, p.Fakefs, fakeExePathLinux, fakeOldVersion) err := u.Run(p.Ctx, false, fakeCoderURL) assert.ErrorContains(t, "update coder - read-only fs", err, "failed to create file") - assertFileContent(t, p.Fakefs, fakeExePath, fakeOldVersion) + assertFileContent(t, p.Fakefs, fakeExePathLinux, fakeOldVersion) }) } @@ -278,6 +279,10 @@ func fakeConfirmNo(_ string) (string, error) { //nolint:unparam func fakeFile(t *testing.T, fs afero.Fs, name string, perm fs.FileMode, content string) { t.Helper() + err := fs.MkdirAll(filepath.Dir(name), 0750) + if err != nil { + panic(err) + } f, err := fs.OpenFile(name, os.O_RDWR|os.O_CREATE|os.O_TRUNC, perm) if err != nil { panic(err) From 5bf7f56f17d863fc9a7af7b1d5337263d258dfef Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Fri, 13 Aug 2021 12:09:16 +0000 Subject: [PATCH 06/25] internal/cmd/update_test.go: test for windows --- internal/cmd/update.go | 14 +++++++--- internal/cmd/update_test.go | 53 ++++++++++++++++++++++++++++++++----- 2 files changed, 58 insertions(+), 9 deletions(-) diff --git a/internal/cmd/update.go b/internal/cmd/update.go index dbc67db5..8f1865c1 100644 --- a/internal/cmd/update.go +++ b/internal/cmd/update.go @@ -34,6 +34,7 @@ type updater struct { executablePath string fs afero.Fs httpClient getter + osF func() string versionF func() string } @@ -54,11 +55,12 @@ func updateCmd() *cobra.Command { } updater := &updater{ + confirmF: defaultConfirm, + executablePath: os.Args[0], httpClient: httpClient, fs: afero.NewOsFs(), - confirmF: defaultConfirm, + osF: func() string { return runtime.GOOS }, versionF: func() string { return version.Version }, - executablePath: os.Args[0], } return updater.Run(ctx, force, coderURL) }, @@ -155,7 +157,13 @@ func (u *updater) Run(ctx context.Context, force bool, coderURLString string) er // TODO: validate the checksum of the downloaded file. GitHub does not currently provide this information // and we do not generate them yet. - updatedBinary, err := extractFromArchive("coder", downloadBuf.Bytes()) + var updatedBinaryName string + if u.osF() == "windows" { + updatedBinaryName = "coder.exe" + } else { + updatedBinaryName = "coder" + } + updatedBinary, err := extractFromArchive(updatedBinaryName, downloadBuf.Bytes()) if err != nil { return clog.Fatal("failed to extract coder binary from archive", clog.Causef(err.Error())) } diff --git a/internal/cmd/update_test.go b/internal/cmd/update_test.go index 263a81b8..1fb81ff5 100644 --- a/internal/cmd/update_test.go +++ b/internal/cmd/update_test.go @@ -19,11 +19,12 @@ import ( ) const ( - fakeExePathLinux = "/home/user/bin/coder" - fakeCoderURL = "https://my.cdr.dev/" - fakeNewVersion = "1.23.4-rc.5+678-gabcdef-12345678" - fakeOldVersion = "1.22.4-rc.5+678-gabcdef-12345678" - fakeReleaseURL = "https://github.com/cdr/coder-cli/releases/download/v1.23.4/coder-cli-linux-amd64.tar.gz" + fakeExePathLinux = "/home/user/bin/coder" + fakeExePathWindows = `C:\Users\user\bin\coder.exe` + fakeCoderURL = "https://my.cdr.dev/" + fakeNewVersion = "1.23.4-rc.5+678-gabcdef-12345678" + fakeOldVersion = "1.22.4-rc.5+678-gabcdef-12345678" + fakeReleaseURL = "https://github.com/cdr/coder-cli/releases/download/v1.23.4/coder-cli-linux-amd64.tar.gz" ) func Test_updater_run(t *testing.T) { @@ -36,6 +37,7 @@ func Test_updater_run(t *testing.T) { ExecutablePath string Fakefs afero.Fs HttpClient *fakeGetter + OsF func() string VersionF func() string } @@ -46,6 +48,7 @@ func Test_updater_run(t *testing.T) { executablePath: p.ExecutablePath, fs: p.Fakefs, httpClient: p.HttpClient, + osF: p.OsF, versionF: p.VersionF, } } @@ -65,6 +68,8 @@ func Test_updater_run(t *testing.T) { ExecutablePath: fakeExePathLinux, Fakefs: fakefs, HttpClient: newFakeGetter(t), + // Default to GOOS=linux + OsF: func() string { return "linux" }, // This must be overridden inside run() VersionF: func() string { t.Errorf("unhandled VersionF") @@ -100,6 +105,35 @@ func Test_updater_run(t *testing.T) { assertFileContent(t, p.Fakefs, fakeExePathLinux, fakeNewVersion) }) + run(t, "update coder - old to new - binary renamed", func(t *testing.T, p *params) { + p.ExecutablePath = "/home/user/bin/coder-cli" + fakeFile(t, p.Fakefs, p.ExecutablePath, 0755, fakeOldVersion) + p.HttpClient.M[fakeCoderURL+"/api"] = newFakeGetterResponse([]byte{}, 401, variadicS("coder-version: "+fakeNewVersion), nil) + p.HttpClient.M[fakeReleaseURL] = newFakeGetterResponse(fakeValidTgzBytes, 200, variadicS(), nil) + p.VersionF = func() string { return fakeOldVersion } + p.ConfirmF = fakeConfirmYes + u := fromParams(p) + assertFileContent(t, p.Fakefs, p.ExecutablePath, fakeOldVersion) + err := u.Run(p.Ctx, false, fakeCoderURL) + assert.Success(t, "update coder - old to new - binary renamed", err) + assertFileContent(t, p.Fakefs, p.ExecutablePath, fakeNewVersion) + }) + + run(t, "update coder - old to new - windows", func(t *testing.T, p *params) { + p.OsF = func() string { return "windows" } + p.ExecutablePath = fakeExePathWindows + fakeFile(t, p.Fakefs, fakeExePathWindows, 0755, fakeOldVersion) + p.HttpClient.M[fakeCoderURL+"/api"] = newFakeGetterResponse([]byte{}, 401, variadicS("coder-version: "+fakeNewVersion), nil) + p.HttpClient.M[fakeReleaseURL] = newFakeGetterResponse(fakeValidZipBytes, 200, variadicS(), nil) + p.VersionF = func() string { return fakeOldVersion } + p.ConfirmF = fakeConfirmYes + u := fromParams(p) + assertFileContent(t, p.Fakefs, fakeExePathWindows, fakeOldVersion) + err := u.Run(p.Ctx, false, fakeCoderURL) + assert.Success(t, "update coder - old to new - windows", err) + assertFileContent(t, p.Fakefs, fakeExePathWindows, fakeNewVersion) + }) + run(t, "update coder - old to new forced", func(t *testing.T, p *params) { fakeFile(t, p.Fakefs, fakeExePathLinux, 0755, fakeOldVersion) p.HttpClient.M[fakeCoderURL+"/api"] = newFakeGetterResponse([]byte{}, 401, variadicS("coder-version: "+fakeNewVersion), nil) @@ -308,8 +342,15 @@ func assertFileContent(t *testing.T, fs afero.Fs, name string, content string) { assert.Equal(t, "assert content equal", content, string(b)) } -// this is a valid tgz file containing a single file named 'coder' with permissions 0751 +// this is a valid tgz archive containing a single file named 'coder' with permissions 0751 // containing the string "1.23.4-rc.5+678-gabcdef-12345678". var fakeValidTgzBytes, _ = base64.StdEncoding.DecodeString(`H4sIAAAAAAAAA+3QsQ4CIRCEYR6F3oC7wIqvc3KnpQnq+3tGCwsTK3LN/zWTTDWZuG/XeeluJFlV s1dqNfnOtyJOi4qllHOuTlSTqPMydNXH43afuvfu3w3jb9qExpRjCb1F2x3qMVymU5uXc9CUi63F 1vsAAAAAAAAAAAAAAAAAAL89AYuL424AKAAA`) + +// this is a valid zip archive containing a single file named 'coder.exe' with permissions 0751 +// containing the string "1.23.4-rc.5+678-gabcdef-12345678". +var fakeValidZipBytes, _ = base64.StdEncoding.DecodeString(`UEsDBAoAAAAAAAtfDVNCHNDCIAAAACAAAAAJABwAY29kZXIuZXhlVVQJAAPmXRZh/10WYXV4CwAB +BOgDAAAE6AMAADEuMjMuNC1yYy41KzY3OC1nYWJjZGVmLTEyMzQ1Njc4UEsBAh4DCgAAAAAAC18N +U0Ic0MIgAAAAIAAAAAkAGAAAAAAAAQAAAO2BAAAAAGNvZGVyLmV4ZVVUBQAD5l0WYXV4CwABBOgD +AAAE6AMAAFBLBQYAAAAAAQABAE8AAABjAAAAAAA=`) From dabd178e3d06898bbe49cff183674d3a51226a02 Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Fri, 13 Aug 2021 12:40:42 +0000 Subject: [PATCH 07/25] fixup! internal/cmd/update_test.go: test for windows --- internal/cmd/update.go | 6 +++--- internal/cmd/update_test.go | 29 +++++++++++++++-------------- 2 files changed, 18 insertions(+), 17 deletions(-) diff --git a/internal/cmd/update.go b/internal/cmd/update.go index 8f1865c1..614f5907 100644 --- a/internal/cmd/update.go +++ b/internal/cmd/update.go @@ -130,7 +130,7 @@ func (u *updater) Run(ctx context.Context, force bool, coderURLString string) er } } - downloadURL := makeDownloadURL(desiredVersion.FinalizeVersion(), runtime.GOOS, runtime.GOARCH) + downloadURL := makeDownloadURL(desiredVersion, u.osF(), runtime.GOARCH) var downloadBuf bytes.Buffer memWriter := bufio.NewWriter(&downloadBuf) @@ -197,14 +197,14 @@ func defaultConfirm(label string) (string, error) { return p.Run() } -func makeDownloadURL(version, ostype, archetype string) string { +func makeDownloadURL(version *semver.Version, ostype, archtype string) string { const template = "https://github.com/cdr/coder-cli/releases/download/v%s/coder-cli-%s-%s.%s" var ext string switch ostype { case "linux": ext = "tar.gz" default: - ext = ".zip" + ext = "zip" } return fmt.Sprintf(template, version, ostype, archetype, ext) } diff --git a/internal/cmd/update_test.go b/internal/cmd/update_test.go index 1fb81ff5..f779131c 100644 --- a/internal/cmd/update_test.go +++ b/internal/cmd/update_test.go @@ -19,12 +19,13 @@ import ( ) const ( - fakeExePathLinux = "/home/user/bin/coder" - fakeExePathWindows = `C:\Users\user\bin\coder.exe` - fakeCoderURL = "https://my.cdr.dev/" - fakeNewVersion = "1.23.4-rc.5+678-gabcdef-12345678" - fakeOldVersion = "1.22.4-rc.5+678-gabcdef-12345678" - fakeReleaseURL = "https://github.com/cdr/coder-cli/releases/download/v1.23.4/coder-cli-linux-amd64.tar.gz" + fakeExePathLinux = "/home/user/bin/coder" + fakeExePathWindows = `C:\Users\user\bin\coder.exe` + fakeCoderURL = "https://my.cdr.dev/" + fakeNewVersion = "1.23.4-rc.5+678-gabcdef-12345678" + fakeOldVersion = "1.22.4-rc.5+678-gabcdef-12345678" + fakeReleaseURLLinux = "https://github.com/cdr/coder-cli/releases/download/v1.23.4/coder-cli-linux-amd64.tar.gz" + fakeReleaseURLWindows = "https://github.com/cdr/coder-cli/releases/download/v1.23.4/coder-cli-windows-amd64.zip" ) func Test_updater_run(t *testing.T) { @@ -95,7 +96,7 @@ func Test_updater_run(t *testing.T) { run(t, "update coder - old to new", func(t *testing.T, p *params) { fakeFile(t, p.Fakefs, fakeExePathLinux, 0755, fakeOldVersion) p.HttpClient.M[fakeCoderURL+"/api"] = newFakeGetterResponse([]byte{}, 401, variadicS("coder-version: "+fakeNewVersion), nil) - p.HttpClient.M[fakeReleaseURL] = newFakeGetterResponse(fakeValidTgzBytes, 200, variadicS(), nil) + p.HttpClient.M[fakeReleaseURLLinux] = newFakeGetterResponse(fakeValidTgzBytes, 200, variadicS(), nil) p.VersionF = func() string { return fakeOldVersion } p.ConfirmF = fakeConfirmYes u := fromParams(p) @@ -109,7 +110,7 @@ func Test_updater_run(t *testing.T) { p.ExecutablePath = "/home/user/bin/coder-cli" fakeFile(t, p.Fakefs, p.ExecutablePath, 0755, fakeOldVersion) p.HttpClient.M[fakeCoderURL+"/api"] = newFakeGetterResponse([]byte{}, 401, variadicS("coder-version: "+fakeNewVersion), nil) - p.HttpClient.M[fakeReleaseURL] = newFakeGetterResponse(fakeValidTgzBytes, 200, variadicS(), nil) + p.HttpClient.M[fakeReleaseURLLinux] = newFakeGetterResponse(fakeValidTgzBytes, 200, variadicS(), nil) p.VersionF = func() string { return fakeOldVersion } p.ConfirmF = fakeConfirmYes u := fromParams(p) @@ -124,7 +125,7 @@ func Test_updater_run(t *testing.T) { p.ExecutablePath = fakeExePathWindows fakeFile(t, p.Fakefs, fakeExePathWindows, 0755, fakeOldVersion) p.HttpClient.M[fakeCoderURL+"/api"] = newFakeGetterResponse([]byte{}, 401, variadicS("coder-version: "+fakeNewVersion), nil) - p.HttpClient.M[fakeReleaseURL] = newFakeGetterResponse(fakeValidZipBytes, 200, variadicS(), nil) + p.HttpClient.M[fakeReleaseURLWindows] = newFakeGetterResponse(fakeValidZipBytes, 200, variadicS(), nil) p.VersionF = func() string { return fakeOldVersion } p.ConfirmF = fakeConfirmYes u := fromParams(p) @@ -137,7 +138,7 @@ func Test_updater_run(t *testing.T) { run(t, "update coder - old to new forced", func(t *testing.T, p *params) { fakeFile(t, p.Fakefs, fakeExePathLinux, 0755, fakeOldVersion) p.HttpClient.M[fakeCoderURL+"/api"] = newFakeGetterResponse([]byte{}, 401, variadicS("coder-version: "+fakeNewVersion), nil) - p.HttpClient.M[fakeReleaseURL] = newFakeGetterResponse(fakeValidTgzBytes, 200, variadicS(), nil) + p.HttpClient.M[fakeReleaseURLLinux] = newFakeGetterResponse(fakeValidTgzBytes, 200, variadicS(), nil) p.VersionF = func() string { return fakeOldVersion } u := fromParams(p) assertFileContent(t, p.Fakefs, fakeExePathLinux, fakeOldVersion) @@ -197,7 +198,7 @@ func Test_updater_run(t *testing.T) { run(t, "update coder - failed to fetch URL", func(t *testing.T, p *params) { fakeFile(t, p.Fakefs, fakeExePathLinux, 0755, fakeOldVersion) p.HttpClient.M[fakeCoderURL+"/api"] = newFakeGetterResponse([]byte{}, 401, variadicS("coder-version: "+fakeNewVersion), nil) - p.HttpClient.M[fakeReleaseURL] = newFakeGetterResponse([]byte{}, 0, variadicS(), net.ErrClosed) + p.HttpClient.M[fakeReleaseURLLinux] = newFakeGetterResponse([]byte{}, 0, variadicS(), net.ErrClosed) p.VersionF = func() string { return fakeOldVersion } p.ConfirmF = fakeConfirmYes u := fromParams(p) @@ -210,7 +211,7 @@ func Test_updater_run(t *testing.T) { run(t, "update coder - release URL 404", func(t *testing.T, p *params) { fakeFile(t, p.Fakefs, fakeExePathLinux, 0755, fakeOldVersion) p.HttpClient.M[fakeCoderURL+"/api"] = newFakeGetterResponse([]byte{}, 401, variadicS("coder-version: "+fakeNewVersion), nil) - p.HttpClient.M[fakeReleaseURL] = newFakeGetterResponse([]byte{}, 404, variadicS(), nil) + p.HttpClient.M[fakeReleaseURLLinux] = newFakeGetterResponse([]byte{}, 404, variadicS(), nil) p.VersionF = func() string { return fakeOldVersion } p.ConfirmF = fakeConfirmYes u := fromParams(p) @@ -223,7 +224,7 @@ func Test_updater_run(t *testing.T) { run(t, "update coder - invalid archive", func(t *testing.T, p *params) { fakeFile(t, p.Fakefs, fakeExePathLinux, 0755, fakeOldVersion) p.HttpClient.M[fakeCoderURL+"/api"] = newFakeGetterResponse([]byte{}, 401, variadicS("coder-version: "+fakeNewVersion), nil) - p.HttpClient.M[fakeReleaseURL] = newFakeGetterResponse([]byte{}, 200, variadicS(), nil) + p.HttpClient.M[fakeReleaseURLLinux] = newFakeGetterResponse([]byte{}, 200, variadicS(), nil) p.VersionF = func() string { return fakeOldVersion } p.ConfirmF = fakeConfirmYes u := fromParams(p) @@ -238,7 +239,7 @@ func Test_updater_run(t *testing.T) { p.Fakefs = afero.NewReadOnlyFs(rwfs) fakeFile(t, rwfs, fakeExePathLinux, 0755, fakeOldVersion) p.HttpClient.M[fakeCoderURL+"/api"] = newFakeGetterResponse([]byte{}, 401, variadicS("coder-version: "+fakeNewVersion), nil) - p.HttpClient.M[fakeReleaseURL] = newFakeGetterResponse(fakeValidTgzBytes, 200, variadicS(), nil) + p.HttpClient.M[fakeReleaseURLLinux] = newFakeGetterResponse(fakeValidTgzBytes, 200, variadicS(), nil) p.VersionF = func() string { return fakeOldVersion } p.ConfirmF = fakeConfirmYes u := fromParams(p) From dcfeec1d5e7a97f5b9dbc1c68272cce9457f5909 Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Fri, 13 Aug 2021 12:57:46 +0000 Subject: [PATCH 08/25] internal/cmd/update.go: replace semver library --- go.mod | 2 +- go.sum | 4 ++-- internal/cmd/update.go | 31 +++++++++++++++++++++---------- internal/cmd/update_test.go | 21 ++++++++++++++++++--- 4 files changed, 42 insertions(+), 16 deletions(-) diff --git a/go.mod b/go.mod index daeed5e7..879292dc 100644 --- a/go.mod +++ b/go.mod @@ -5,7 +5,7 @@ go 1.14 require ( cdr.dev/slog v1.4.1 cdr.dev/wsep v0.0.0-20200728013649-82316a09813f - github.com/blang/semver/v4 v4.0.0 + github.com/Masterminds/semver/v3 v3.1.1 github.com/briandowns/spinner v1.16.0 github.com/cli/safeexec v1.0.0 github.com/fatih/color v1.12.0 diff --git a/go.sum b/go.sum index dae8f880..f1acd215 100644 --- a/go.sum +++ b/go.sum @@ -49,6 +49,8 @@ github.com/BurntSushi/toml v0.3.1/go.mod h1:xHWCNGjB5oqiDr8zfno3MHue2Ht5sIBksp03 github.com/BurntSushi/xgb v0.0.0-20160522181843-27f122750802/go.mod h1:IVnqGOEym/WlBOVXweHU+Q+/VP0lqqI8lqeDx9IjBqo= github.com/GeertJohan/go.incremental v1.0.0/go.mod h1:6fAjUhbVuX1KcMD3c8TEgVUqmo4seqhv0i0kdATSkM0= github.com/GeertJohan/go.rice v1.0.0/go.mod h1:eH6gbSOAUv07dQuZVnBmoDP8mgsM1rtixis4Tib9if0= +github.com/Masterminds/semver/v3 v3.1.1 h1:hLg3sBzpNErnxhQtUy/mmLR2I9foDujNK030IGemrRc= +github.com/Masterminds/semver/v3 v3.1.1/go.mod h1:VPu/7SZ7ePZ3QOrcuXROw5FAcLl4a0cBrbBpGY/8hQs= github.com/akavel/rsrc v0.8.0/go.mod h1:uLoCtb9J+EyAqh+26kdrTgmzRBFPGOolLWKpdxkKq+c= github.com/alecthomas/assert v0.0.0-20170929043011-405dbfeb8e38 h1:smF2tmSOzy2Mm+0dGI2AIUHY+w0BUc+4tn40djz7+6U= github.com/alecthomas/assert v0.0.0-20170929043011-405dbfeb8e38/go.mod h1:r7bzyVFMNntcxPZXK3/+KdruV1H5KSlyVY0gc+NgInI= @@ -69,8 +71,6 @@ github.com/armon/go-metrics v0.0.0-20180917152333-f0300d1749da/go.mod h1:Q73ZrmV github.com/armon/go-radix v0.0.0-20180808171621-7fddfc383310/go.mod h1:ufUuZ+zHj4x4TnLV4JWEpy2hxWSpsRywHrMgIH9cCH8= github.com/bgentry/speakeasy v0.1.0/go.mod h1:+zsyZBPWlz7T6j88CTgSN5bM796AkVf0kBD4zp0CCIs= github.com/bketelsen/crypt v0.0.4/go.mod h1:aI6NrJ0pMGgvZKL1iVgXLnfIFJtfV+bKCoqOes/6LfM= -github.com/blang/semver/v4 v4.0.0 h1:1PFHFE6yCCTv8C1TeyNNarDzntLi7wMI5i/pzqYIsAM= -github.com/blang/semver/v4 v4.0.0/go.mod h1:IbckMUScFkM3pff0VJDNKRiT6TG/YpiHIM2yvyW5YoQ= github.com/briandowns/spinner v1.16.0 h1:DFmp6hEaIx2QXXuqSJmtfSBSAjRmpGiKG6ip2Wm/yOs= github.com/briandowns/spinner v1.16.0/go.mod h1:QOuQk7x+EaDASo80FEXwlwiA+j/PPIcX3FScO+3/ZPQ= github.com/census-instrumentation/opencensus-proto v0.2.1/go.mod h1:f6KPmirojxKA12rnyqOA5BBL4O983OfeGPqjHWSTneU= diff --git a/internal/cmd/update.go b/internal/cmd/update.go index 614f5907..5333cd8b 100644 --- a/internal/cmd/update.go +++ b/internal/cmd/update.go @@ -22,7 +22,7 @@ import ( "cdr.dev/coder-cli/pkg/clog" "golang.org/x/xerrors" - "github.com/blang/semver/v4" + "github.com/Masterminds/semver/v3" "github.com/manifoldco/promptui" "github.com/spf13/afero" "github.com/spf13/cobra" @@ -116,7 +116,7 @@ func (u *updater) Run(ctx context.Context, force bool, coderURLString string) er clog.LogInfo(fmt.Sprintf("Coder instance at %q reports version %s", coderURL.String(), desiredVersion.String())) clog.LogInfo(fmt.Sprintf("Current version of coder-cli is %s", version.Version)) - if currentVersion, err := semver.Make(u.versionF()); err == nil { + if currentVersion, err := semver.StrictNewVersion(u.versionF()); err == nil { if desiredVersion.Compare(currentVersion) == 0 { clog.LogInfo("Up to date!") return nil @@ -124,7 +124,7 @@ func (u *updater) Run(ctx context.Context, force bool, coderURLString string) er } if !force { - label := fmt.Sprintf("Update coder-cli to version %s", desiredVersion.FinalizeVersion()) + label := fmt.Sprintf("Do you want to download version %s instead", desiredVersion) if _, err := u.confirmF(label); err != nil { return clog.Fatal("failed to confirm update", clog.Tipf(`use "--force" to update without confirmation`)) } @@ -188,7 +188,7 @@ func (u *updater) Run(ctx context.Context, force bool, coderURLString string) er return clog.Fatal("failed to update coder binary in-place", clog.Causef(err.Error())) } - clog.LogSuccess("Updated coder CLI to version " + desiredVersion.FinalizeVersion()) + clog.LogSuccess("Updated coder CLI to version " + desiredVersion.String()) return nil } @@ -206,7 +206,18 @@ func makeDownloadURL(version *semver.Version, ostype, archtype string) string { default: ext = "zip" } - return fmt.Sprintf(template, version, ostype, archetype, ext) + var b bytes.Buffer + fmt.Fprintf(&b, "%d", version.Major()) + fmt.Fprint(&b, ".") + fmt.Fprintf(&b, "%d", version.Minor()) + fmt.Fprint(&b, ".") + fmt.Fprintf(&b, "%d", version.Patch()) + if version.Prerelease() != "" { + fmt.Fprint(&b, "-") + fmt.Fprint(&b, version.Prerelease()) + } + + return fmt.Sprintf(template, b.String(), ostype, archtype, ext) } func extractFromArchive(path string, archive []byte) ([]byte, error) { @@ -295,22 +306,22 @@ func getCoderConfigURL() (*url.URL, error) { // XXX: coder.Client requires an API key, but we may not be logged into the coder instance for which we // want to determine the version. We don't need an API key to sniff the version header. -func getAPIVersionUnauthed(client getter, baseURL url.URL) (semver.Version, error) { +func getAPIVersionUnauthed(client getter, baseURL url.URL) (*semver.Version, error) { baseURL.Path = path.Join(baseURL.Path, "/api") resp, err := client.Get(baseURL.String()) if err != nil { - return semver.Version{}, xerrors.Errorf("get %s: %w", baseURL.String(), err) + return nil, xerrors.Errorf("get %s: %w", baseURL.String(), err) } defer resp.Body.Close() versionHdr := resp.Header.Get("coder-version") if versionHdr == "" { - return semver.Version{}, xerrors.Errorf("URL %s response missing coder-version header", baseURL.String()) + return nil, xerrors.Errorf("URL %s response missing coder-version header", baseURL.String()) } - version, err := semver.Parse(versionHdr) + version, err := semver.StrictNewVersion(versionHdr) if err != nil { - return semver.Version{}, xerrors.Errorf("parsing coder-version header: %w", err) + return nil, xerrors.Errorf("parsing coder-version header: %w", err) } return version, nil diff --git a/internal/cmd/update_test.go b/internal/cmd/update_test.go index f779131c..a65804bd 100644 --- a/internal/cmd/update_test.go +++ b/internal/cmd/update_test.go @@ -24,8 +24,8 @@ const ( fakeCoderURL = "https://my.cdr.dev/" fakeNewVersion = "1.23.4-rc.5+678-gabcdef-12345678" fakeOldVersion = "1.22.4-rc.5+678-gabcdef-12345678" - fakeReleaseURLLinux = "https://github.com/cdr/coder-cli/releases/download/v1.23.4/coder-cli-linux-amd64.tar.gz" - fakeReleaseURLWindows = "https://github.com/cdr/coder-cli/releases/download/v1.23.4/coder-cli-windows-amd64.zip" + fakeReleaseURLLinux = "https://github.com/cdr/coder-cli/releases/download/v1.23.4-rc.5/coder-cli-linux-amd64.tar.gz" + fakeReleaseURLWindows = "https://github.com/cdr/coder-cli/releases/download/v1.23.4-rc.5/coder-cli-windows-amd64.zip" ) func Test_updater_run(t *testing.T) { @@ -221,7 +221,7 @@ func Test_updater_run(t *testing.T) { assertFileContent(t, p.Fakefs, fakeExePathLinux, fakeOldVersion) }) - run(t, "update coder - invalid archive", func(t *testing.T, p *params) { + run(t, "update coder - invalid tgz archive", func(t *testing.T, p *params) { fakeFile(t, p.Fakefs, fakeExePathLinux, 0755, fakeOldVersion) p.HttpClient.M[fakeCoderURL+"/api"] = newFakeGetterResponse([]byte{}, 401, variadicS("coder-version: "+fakeNewVersion), nil) p.HttpClient.M[fakeReleaseURLLinux] = newFakeGetterResponse([]byte{}, 200, variadicS(), nil) @@ -234,6 +234,21 @@ func Test_updater_run(t *testing.T) { assertFileContent(t, p.Fakefs, fakeExePathLinux, fakeOldVersion) }) + run(t, "update coder - invalid zip archive", func(t *testing.T, p *params) { + p.OsF = func() string { return "windows" } + p.ExecutablePath = fakeExePathWindows + fakeFile(t, p.Fakefs, fakeExePathWindows, 0755, fakeOldVersion) + p.HttpClient.M[fakeCoderURL+"/api"] = newFakeGetterResponse([]byte{}, 401, variadicS("coder-version: "+fakeNewVersion), nil) + p.HttpClient.M[fakeReleaseURLWindows] = newFakeGetterResponse([]byte{}, 200, variadicS(), nil) + p.VersionF = func() string { return fakeOldVersion } + p.ConfirmF = fakeConfirmYes + u := fromParams(p) + assertFileContent(t, p.Fakefs, p.ExecutablePath, fakeOldVersion) + err := u.Run(p.Ctx, false, fakeCoderURL) + assert.ErrorContains(t, "update coder - invalid archive", err, "failed to extract coder binary from archive") + assertFileContent(t, p.Fakefs, p.ExecutablePath, fakeOldVersion) + }) + run(t, "update coder - read-only fs", func(t *testing.T, p *params) { rwfs := p.Fakefs p.Fakefs = afero.NewReadOnlyFs(rwfs) From 0801cfc695d713bc6816eceae92a7059fe0b1a19 Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Fri, 13 Aug 2021 13:23:25 +0000 Subject: [PATCH 09/25] internal/cmd/update.go: use /api/private/version instead of sniffing HTTP headers --- internal/cmd/update.go | 22 ++++++++++++++++------ internal/cmd/update_test.go | 32 ++++++++++++++++++++------------ 2 files changed, 36 insertions(+), 18 deletions(-) diff --git a/internal/cmd/update.go b/internal/cmd/update.go index 5333cd8b..e46ae680 100644 --- a/internal/cmd/update.go +++ b/internal/cmd/update.go @@ -7,8 +7,10 @@ import ( "bytes" "compress/gzip" "context" + "encoding/json" "fmt" "io" + "io/ioutil" "net/http" "net/url" "os" @@ -307,21 +309,29 @@ func getCoderConfigURL() (*url.URL, error) { // XXX: coder.Client requires an API key, but we may not be logged into the coder instance for which we // want to determine the version. We don't need an API key to sniff the version header. func getAPIVersionUnauthed(client getter, baseURL url.URL) (*semver.Version, error) { - baseURL.Path = path.Join(baseURL.Path, "/api") + baseURL.Path = path.Join(baseURL.Path, "/api/private/version") resp, err := client.Get(baseURL.String()) if err != nil { return nil, xerrors.Errorf("get %s: %w", baseURL.String(), err) } defer resp.Body.Close() - versionHdr := resp.Header.Get("coder-version") - if versionHdr == "" { - return nil, xerrors.Errorf("URL %s response missing coder-version header", baseURL.String()) + ver := struct { + Version string `json:"version"` + }{} + + body, err := ioutil.ReadAll(resp.Body) + if err != nil { + return nil, xerrors.Errorf("read response body: %w", err) + } + + if err := json.Unmarshal(body, &ver); err != nil { + return nil, xerrors.Errorf("parse version response: %w", err) } - version, err := semver.StrictNewVersion(versionHdr) + version, err := semver.StrictNewVersion(ver.Version) if err != nil { - return nil, xerrors.Errorf("parsing coder-version header: %w", err) + return nil, xerrors.Errorf("parsing coder version: %w", err) } return version, nil diff --git a/internal/cmd/update_test.go b/internal/cmd/update_test.go index a65804bd..3c0a3749 100644 --- a/internal/cmd/update_test.go +++ b/internal/cmd/update_test.go @@ -4,6 +4,7 @@ import ( "bytes" "context" "encoding/base64" + "fmt" "io/fs" "io/ioutil" "net" @@ -28,6 +29,12 @@ const ( fakeReleaseURLWindows = "https://github.com/cdr/coder-cli/releases/download/v1.23.4-rc.5/coder-cli-windows-amd64.zip" ) +var ( + apiPrivateVersionURL = fakeCoderURL + "/api/private/version" + fakeNewVersionJson = fmt.Sprintf(`{"version":%q}`, fakeNewVersion) + fakeOldVersionJson = fmt.Sprintf(`{"version":%q}`, fakeOldVersion) +) + func Test_updater_run(t *testing.T) { t.Parallel() @@ -84,7 +91,7 @@ func Test_updater_run(t *testing.T) { run(t, "update coder - noop", func(t *testing.T, p *params) { fakeFile(t, p.Fakefs, fakeExePathLinux, 0755, fakeNewVersion) - p.HttpClient.M[fakeCoderURL+"/api"] = newFakeGetterResponse([]byte{}, 401, variadicS("coder-version: "+fakeNewVersion), nil) + p.HttpClient.M[apiPrivateVersionURL] = newFakeGetterResponse([]byte(fakeNewVersionJson), 200, variadicS(), nil) p.VersionF = func() string { return fakeNewVersion } u := fromParams(p) assertFileContent(t, p.Fakefs, fakeExePathLinux, fakeNewVersion) @@ -95,7 +102,7 @@ func Test_updater_run(t *testing.T) { run(t, "update coder - old to new", func(t *testing.T, p *params) { fakeFile(t, p.Fakefs, fakeExePathLinux, 0755, fakeOldVersion) - p.HttpClient.M[fakeCoderURL+"/api"] = newFakeGetterResponse([]byte{}, 401, variadicS("coder-version: "+fakeNewVersion), nil) + p.HttpClient.M[apiPrivateVersionURL] = newFakeGetterResponse([]byte(fakeNewVersionJson), 200, variadicS(), nil) p.HttpClient.M[fakeReleaseURLLinux] = newFakeGetterResponse(fakeValidTgzBytes, 200, variadicS(), nil) p.VersionF = func() string { return fakeOldVersion } p.ConfirmF = fakeConfirmYes @@ -109,7 +116,7 @@ func Test_updater_run(t *testing.T) { run(t, "update coder - old to new - binary renamed", func(t *testing.T, p *params) { p.ExecutablePath = "/home/user/bin/coder-cli" fakeFile(t, p.Fakefs, p.ExecutablePath, 0755, fakeOldVersion) - p.HttpClient.M[fakeCoderURL+"/api"] = newFakeGetterResponse([]byte{}, 401, variadicS("coder-version: "+fakeNewVersion), nil) + p.HttpClient.M[apiPrivateVersionURL] = newFakeGetterResponse([]byte(fakeNewVersionJson), 200, variadicS(), nil) p.HttpClient.M[fakeReleaseURLLinux] = newFakeGetterResponse(fakeValidTgzBytes, 200, variadicS(), nil) p.VersionF = func() string { return fakeOldVersion } p.ConfirmF = fakeConfirmYes @@ -124,7 +131,7 @@ func Test_updater_run(t *testing.T) { p.OsF = func() string { return "windows" } p.ExecutablePath = fakeExePathWindows fakeFile(t, p.Fakefs, fakeExePathWindows, 0755, fakeOldVersion) - p.HttpClient.M[fakeCoderURL+"/api"] = newFakeGetterResponse([]byte{}, 401, variadicS("coder-version: "+fakeNewVersion), nil) + p.HttpClient.M[apiPrivateVersionURL] = newFakeGetterResponse([]byte(fakeNewVersionJson), 200, variadicS(), nil) p.HttpClient.M[fakeReleaseURLWindows] = newFakeGetterResponse(fakeValidZipBytes, 200, variadicS(), nil) p.VersionF = func() string { return fakeOldVersion } p.ConfirmF = fakeConfirmYes @@ -137,7 +144,7 @@ func Test_updater_run(t *testing.T) { run(t, "update coder - old to new forced", func(t *testing.T, p *params) { fakeFile(t, p.Fakefs, fakeExePathLinux, 0755, fakeOldVersion) - p.HttpClient.M[fakeCoderURL+"/api"] = newFakeGetterResponse([]byte{}, 401, variadicS("coder-version: "+fakeNewVersion), nil) + p.HttpClient.M[apiPrivateVersionURL] = newFakeGetterResponse([]byte(fakeNewVersionJson), 200, variadicS(), nil) p.HttpClient.M[fakeReleaseURLLinux] = newFakeGetterResponse(fakeValidTgzBytes, 200, variadicS(), nil) p.VersionF = func() string { return fakeOldVersion } u := fromParams(p) @@ -149,7 +156,7 @@ func Test_updater_run(t *testing.T) { run(t, "update coder - user cancelled", func(t *testing.T, p *params) { fakeFile(t, p.Fakefs, fakeExePathLinux, 0755, fakeOldVersion) - p.HttpClient.M[fakeCoderURL+"/api"] = newFakeGetterResponse([]byte{}, 401, variadicS("coder-version: "+fakeNewVersion), nil) + p.HttpClient.M[apiPrivateVersionURL] = newFakeGetterResponse([]byte(fakeNewVersionJson), 200, variadicS(), nil) p.VersionF = func() string { return fakeOldVersion } p.ConfirmF = fakeConfirmNo u := fromParams(p) @@ -186,7 +193,7 @@ func Test_updater_run(t *testing.T) { run(t, "update coder - fetch api version failure", func(t *testing.T, p *params) { fakeFile(t, p.Fakefs, fakeExePathLinux, 0755, fakeOldVersion) - p.HttpClient.M[fakeCoderURL+"/api"] = newFakeGetterResponse([]byte{}, 401, variadicS(), net.ErrClosed) + p.HttpClient.M[apiPrivateVersionURL] = newFakeGetterResponse([]byte{}, 401, variadicS(), net.ErrClosed) p.VersionF = func() string { return fakeOldVersion } u := fromParams(p) assertFileContent(t, p.Fakefs, fakeExePathLinux, fakeOldVersion) @@ -197,7 +204,7 @@ func Test_updater_run(t *testing.T) { run(t, "update coder - failed to fetch URL", func(t *testing.T, p *params) { fakeFile(t, p.Fakefs, fakeExePathLinux, 0755, fakeOldVersion) - p.HttpClient.M[fakeCoderURL+"/api"] = newFakeGetterResponse([]byte{}, 401, variadicS("coder-version: "+fakeNewVersion), nil) + p.HttpClient.M[apiPrivateVersionURL] = newFakeGetterResponse([]byte(fakeNewVersionJson), 200, variadicS(), nil) p.HttpClient.M[fakeReleaseURLLinux] = newFakeGetterResponse([]byte{}, 0, variadicS(), net.ErrClosed) p.VersionF = func() string { return fakeOldVersion } p.ConfirmF = fakeConfirmYes @@ -210,7 +217,7 @@ func Test_updater_run(t *testing.T) { run(t, "update coder - release URL 404", func(t *testing.T, p *params) { fakeFile(t, p.Fakefs, fakeExePathLinux, 0755, fakeOldVersion) - p.HttpClient.M[fakeCoderURL+"/api"] = newFakeGetterResponse([]byte{}, 401, variadicS("coder-version: "+fakeNewVersion), nil) + p.HttpClient.M[apiPrivateVersionURL] = newFakeGetterResponse([]byte(fakeNewVersionJson), 200, variadicS(), nil) p.HttpClient.M[fakeReleaseURLLinux] = newFakeGetterResponse([]byte{}, 404, variadicS(), nil) p.VersionF = func() string { return fakeOldVersion } p.ConfirmF = fakeConfirmYes @@ -223,7 +230,7 @@ func Test_updater_run(t *testing.T) { run(t, "update coder - invalid tgz archive", func(t *testing.T, p *params) { fakeFile(t, p.Fakefs, fakeExePathLinux, 0755, fakeOldVersion) - p.HttpClient.M[fakeCoderURL+"/api"] = newFakeGetterResponse([]byte{}, 401, variadicS("coder-version: "+fakeNewVersion), nil) + p.HttpClient.M[apiPrivateVersionURL] = newFakeGetterResponse([]byte(fakeNewVersionJson), 200, variadicS(), nil) p.HttpClient.M[fakeReleaseURLLinux] = newFakeGetterResponse([]byte{}, 200, variadicS(), nil) p.VersionF = func() string { return fakeOldVersion } p.ConfirmF = fakeConfirmYes @@ -238,7 +245,7 @@ func Test_updater_run(t *testing.T) { p.OsF = func() string { return "windows" } p.ExecutablePath = fakeExePathWindows fakeFile(t, p.Fakefs, fakeExePathWindows, 0755, fakeOldVersion) - p.HttpClient.M[fakeCoderURL+"/api"] = newFakeGetterResponse([]byte{}, 401, variadicS("coder-version: "+fakeNewVersion), nil) + p.HttpClient.M[apiPrivateVersionURL] = newFakeGetterResponse([]byte(fakeNewVersionJson), 200, variadicS(), nil) p.HttpClient.M[fakeReleaseURLWindows] = newFakeGetterResponse([]byte{}, 200, variadicS(), nil) p.VersionF = func() string { return fakeOldVersion } p.ConfirmF = fakeConfirmYes @@ -253,7 +260,7 @@ func Test_updater_run(t *testing.T) { rwfs := p.Fakefs p.Fakefs = afero.NewReadOnlyFs(rwfs) fakeFile(t, rwfs, fakeExePathLinux, 0755, fakeOldVersion) - p.HttpClient.M[fakeCoderURL+"/api"] = newFakeGetterResponse([]byte{}, 401, variadicS("coder-version: "+fakeNewVersion), nil) + p.HttpClient.M[apiPrivateVersionURL] = newFakeGetterResponse([]byte(fakeNewVersionJson), 200, variadicS(), nil) p.HttpClient.M[fakeReleaseURLLinux] = newFakeGetterResponse(fakeValidTgzBytes, 200, variadicS(), nil) p.VersionF = func() string { return fakeOldVersion } p.ConfirmF = fakeConfirmYes @@ -280,6 +287,7 @@ func newFakeGetter(t *testing.T) *fakeGetter { // Get returns the configured response for url. If no response configured, test fails immediately. func (f *fakeGetter) Get(url string) (*http.Response, error) { + f.T.Helper() val, ok := f.M[url] if !ok { f.T.Errorf("unhandled url: %s", url) From f6ce76f6496ea0b0613bbdcc1e17bab3827dd0ad Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Fri, 13 Aug 2021 13:25:21 +0000 Subject: [PATCH 10/25] gendocs --- docs/coder.md | 1 + docs/coder_update.md | 30 ++++++++++++++++++++++++++++++ 2 files changed, 31 insertions(+) create mode 100644 docs/coder_update.md diff --git a/docs/coder.md b/docs/coder.md index 17e7fa7f..513efb42 100644 --- a/docs/coder.md +++ b/docs/coder.md @@ -20,6 +20,7 @@ coder provides a CLI for working with an existing Coder installation * [coder ssh](coder_ssh.md) - Enter a shell of execute a command over SSH into a Coder workspace * [coder sync](coder_sync.md) - Establish a one way directory sync to a Coder workspace * [coder tokens](coder_tokens.md) - manage Coder API tokens for the active user +* [coder update](coder_update.md) - Update coder binary * [coder urls](coder_urls.md) - Interact with workspace DevURLs * [coder users](coder_users.md) - Interact with Coder user accounts * [coder workspaces](coder_workspaces.md) - Interact with Coder workspaces diff --git a/docs/coder_update.md b/docs/coder_update.md new file mode 100644 index 00000000..30266285 --- /dev/null +++ b/docs/coder_update.md @@ -0,0 +1,30 @@ +## coder update + +Update coder binary + +### Synopsis + +Update coder to the version matching a given coder instance. + +``` +coder update [flags] +``` + +### Options + +``` + --coder string coder instance against which to match version + --force do not prompt for confirmation + -h, --help help for update +``` + +### Options inherited from parent commands + +``` + -v, --verbose show verbose output +``` + +### SEE ALSO + +* [coder](coder.md) - coder provides a CLI for working with an existing Coder installation + From 637108406e507d8ce091cf101f9515d5c6fa376a Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Fri, 13 Aug 2021 13:56:40 +0000 Subject: [PATCH 11/25] internal/cmd/update.go: use os.Executable() instead of os.Args[0] --- internal/cmd/update.go | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/internal/cmd/update.go b/internal/cmd/update.go index e46ae680..775f4378 100644 --- a/internal/cmd/update.go +++ b/internal/cmd/update.go @@ -56,9 +56,14 @@ func updateCmd() *cobra.Command { Timeout: 10 * time.Second, } + currExe, err := os.Executable() + if err != nil { + return clog.Fatal("init: get current executable", clog.Causef(err.Error())) + } + updater := &updater{ confirmF: defaultConfirm, - executablePath: os.Args[0], + executablePath: currExe, httpClient: httpClient, fs: afero.NewOsFs(), osF: func() string { return runtime.GOOS }, @@ -85,7 +90,7 @@ func (u *updater) Run(ctx context.Context, force bool, coderURLString string) er currentBinaryStat, err := u.fs.Stat(u.executablePath) if err != nil { - return clog.Fatal("preflight: cannot stat current binary", clog.Causef("%s", err)) + return clog.Fatal("preflight: cannot stat current binary", clog.Causef(err.Error())) } if currentBinaryStat.Mode().Perm()&0222 == 0 { From bdb998ec39962d8b833b6526a4ae3aac2cdc6c5c Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Fri, 13 Aug 2021 15:20:21 +0000 Subject: [PATCH 12/25] internal/cmd/update.go: check path prefixes --- internal/cmd/update.go | 60 ++++++++++++++++++++++++++++--- internal/cmd/update_test.go | 72 ++++++++++++++++++++++++++++++++++++- 2 files changed, 127 insertions(+), 5 deletions(-) diff --git a/internal/cmd/update.go b/internal/cmd/update.go index 775f4378..cb667dbf 100644 --- a/internal/cmd/update.go +++ b/internal/cmd/update.go @@ -14,7 +14,9 @@ import ( "net/http" "net/url" "os" + "os/exec" "path" + "path/filepath" "runtime" "strings" "time" @@ -32,7 +34,8 @@ import ( // updater updates coder-cli. type updater struct { - confirmF func(label string) (string, error) + confirmF func(string) (string, error) + execF func(context.Context, string, ...string) ([]byte, error) executablePath string fs afero.Fs httpClient getter @@ -63,6 +66,7 @@ func updateCmd() *cobra.Command { updater := &updater{ confirmF: defaultConfirm, + execF: defaultExec, executablePath: currExe, httpClient: httpClient, fs: afero.NewOsFs(), @@ -84,9 +88,27 @@ type getter interface { } func (u *updater) Run(ctx context.Context, force bool, coderURLString string) error { - // TODO: check under following directories and warn if coder binary is under them: + // Check under following directories and warn if coder binary is under them: + // * C:\Windows\ // * homebrew prefix - // * coder assets root (env CODER_ASSETS_ROOT) + // * coder assets root (/var/tmp/coder) + var pathBlockList = []string{ + `C:\Windows\`, + `/var/tmp/coder`, + } + brewPrefixCmd, err := u.execF(ctx, "brew", "--prefix") + if err == nil { // ignore errors if homebrew not installed + pathBlockList = append(pathBlockList, strings.TrimSpace(string(brewPrefixCmd))) + } + + for _, prefix := range pathBlockList { + if HasFilePathPrefix(u.executablePath, prefix) { + return clog.Fatal( + "cowardly refusing to update coder binary", + clog.BlankLine, + clog.Causef("executable path %q is under blocklisted prefix %q", u.executablePath, prefix)) + } + } currentBinaryStat, err := u.fs.Stat(u.executablePath) if err != nil { @@ -312,7 +334,7 @@ func getCoderConfigURL() (*url.URL, error) { } // XXX: coder.Client requires an API key, but we may not be logged into the coder instance for which we -// want to determine the version. We don't need an API key to sniff the version header. +// want to determine the version. We don't need an API key to hit /api/private/version though. func getAPIVersionUnauthed(client getter, baseURL url.URL) (*semver.Version, error) { baseURL.Path = path.Join(baseURL.Path, "/api/private/version") resp, err := client.Get(baseURL.String()) @@ -341,3 +363,33 @@ func getAPIVersionUnauthed(client getter, baseURL url.URL) (*semver.Version, err return version, nil } + +// HasFilePathPrefix reports whether the filesystem path s +// begins with the elements in prefix. +// Lifted from github.com/golang/go/blob/master/src/cmd/internal/str/path.go +func HasFilePathPrefix(s, prefix string) bool { + sv := strings.ToUpper(filepath.VolumeName(s)) + pv := strings.ToUpper(filepath.VolumeName(prefix)) + s = s[len(sv):] + prefix = prefix[len(pv):] + switch { + default: + return false + case sv != pv: + return false + case len(s) == len(prefix): + return s == prefix + case prefix == "": + return true + case len(s) > len(prefix): + if prefix[len(prefix)-1] == filepath.Separator { + return strings.HasPrefix(s, prefix) + } + return s[len(prefix)] == filepath.Separator && s[:len(prefix)] == prefix + } +} + +// defaultExec wraps exec.CommandContext +func defaultExec(ctx context.Context, cmd string, args ...string) ([]byte, error) { + return exec.CommandContext(ctx, cmd, args...).CombinedOutput() +} diff --git a/internal/cmd/update_test.go b/internal/cmd/update_test.go index 3c0a3749..5587c5a6 100644 --- a/internal/cmd/update_test.go +++ b/internal/cmd/update_test.go @@ -11,6 +11,7 @@ import ( "net/http" "os" "path/filepath" + "runtime" "strings" "testing" @@ -32,7 +33,6 @@ const ( var ( apiPrivateVersionURL = fakeCoderURL + "/api/private/version" fakeNewVersionJson = fmt.Sprintf(`{"version":%q}`, fakeNewVersion) - fakeOldVersionJson = fmt.Sprintf(`{"version":%q}`, fakeOldVersion) ) func Test_updater_run(t *testing.T) { @@ -42,6 +42,7 @@ func Test_updater_run(t *testing.T) { type params struct { ConfirmF func(string) (string, error) Ctx context.Context + Execer *fakeExecer ExecutablePath string Fakefs afero.Fs HttpClient *fakeGetter @@ -53,6 +54,7 @@ func Test_updater_run(t *testing.T) { fromParams := func(p *params) *updater { return &updater{ confirmF: p.ConfirmF, + execF: p.Execer.ExecF, executablePath: p.ExecutablePath, fs: p.Fakefs, httpClient: p.HttpClient, @@ -65,6 +67,8 @@ func Test_updater_run(t *testing.T) { t.Logf("running %s", name) ctx := context.Background() fakefs := afero.NewMemMapFs() + execer := newFakeExecer(t) + execer.M["brew --prefix"] = fakeExecerResult{[]byte{}, os.ErrNotExist} params := ¶ms{ // This must be overridden inside run() ConfirmF: func(string) (string, error) { @@ -72,6 +76,7 @@ func Test_updater_run(t *testing.T) { t.FailNow() return "", nil }, + Execer: execer, Ctx: ctx, ExecutablePath: fakeExePathLinux, Fakefs: fakefs, @@ -270,6 +275,43 @@ func Test_updater_run(t *testing.T) { assert.ErrorContains(t, "update coder - read-only fs", err, "failed to create file") assertFileContent(t, p.Fakefs, fakeExePathLinux, fakeOldVersion) }) + + if runtime.GOOS == "windows" { + run(t, "update coder - path blocklist - windows", func(t *testing.T, p *params) { + p.ExecutablePath = `C:\Windows\system32\coder.exe` + u := fromParams(p) + err := u.Run(p.Ctx, false, fakeCoderURL) + assert.ErrorContains(t, "update coder - path blocklist - windows", err, "cowardly refusing to update coder binary") + }) + } else { + run(t, "update coder - path blocklist - coder assets dir", func(t *testing.T, p *params) { + p.ExecutablePath = `/var/tmp/coder/coder` + u := fromParams(p) + err := u.Run(p.Ctx, false, fakeCoderURL) + assert.ErrorContains(t, "update coder - path blocklist - windows", err, "cowardly refusing to update coder binary") + }) + run(t, "update coder - path blocklist - old homebrew prefix", func(t *testing.T, p *params) { + p.Execer.M["brew --prefix"] = fakeExecerResult{[]byte("/usr/local"), nil} + p.ExecutablePath = `/usr/local/bin/coder` + u := fromParams(p) + err := u.Run(p.Ctx, false, fakeCoderURL) + assert.ErrorContains(t, "update coder - path blocklist - old homebrew prefix", err, "cowardly refusing to update coder binary") + }) + run(t, "update coder - path blocklist - new homebrew prefix", func(t *testing.T, p *params) { + p.Execer.M["brew --prefix"] = fakeExecerResult{[]byte("/opt/homebrew"), nil} + p.ExecutablePath = `/opt/homebrew/bin/coder` + u := fromParams(p) + err := u.Run(p.Ctx, false, fakeCoderURL) + assert.ErrorContains(t, "update coder - path blocklist - new homebrew prefix", err, "cowardly refusing to update coder binary") + }) + run(t, "update coder - path blocklist - linuxbrew", func(t *testing.T, p *params) { + p.Execer.M["brew --prefix"] = fakeExecerResult{[]byte("/home/user/.linuxbrew"), nil} + p.ExecutablePath = `/home/user/.linuxbrew/bin/coder` + u := fromParams(p) + err := u.Run(p.Ctx, false, fakeCoderURL) + assert.ErrorContains(t, "update coder - path blocklist - linuxbrew", err, "cowardly refusing to update coder binary") + }) + } } // fakeGetter mocks HTTP requests @@ -378,3 +420,31 @@ var fakeValidZipBytes, _ = base64.StdEncoding.DecodeString(`UEsDBAoAAAAAAAtfDVNC BOgDAAAE6AMAADEuMjMuNC1yYy41KzY3OC1nYWJjZGVmLTEyMzQ1Njc4UEsBAh4DCgAAAAAAC18N U0Ic0MIgAAAAIAAAAAkAGAAAAAAAAQAAAO2BAAAAAGNvZGVyLmV4ZVVUBQAD5l0WYXV4CwABBOgD AAAE6AMAAFBLBQYAAAAAAQABAE8AAABjAAAAAAA=`) + +type fakeExecer struct { + M map[string]fakeExecerResult + T *testing.T +} + +func (f *fakeExecer) ExecF(_ context.Context, cmd string, args ...string) ([]byte, error) { + cmdAndArgs := strings.Join(append([]string{cmd}, args...), " ") + val, ok := f.M[cmdAndArgs] + if !ok { + f.T.Errorf("unhandled cmd %q", cmd) + f.T.FailNow() + return nil, nil // will never happen + } + return val.Output, val.Err +} + +func newFakeExecer(t *testing.T) *fakeExecer { + return &fakeExecer{ + M: make(map[string]fakeExecerResult), + T: t, + } +} + +type fakeExecerResult struct { + Output []byte + Err error +} From 306686ce4e02c90b1fc983cb4dbaaa19bfd373ed Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Fri, 13 Aug 2021 15:28:25 +0000 Subject: [PATCH 13/25] lint --- internal/cmd/update.go | 8 ++--- internal/cmd/update_test.go | 66 ++++++++++++++++++------------------- 2 files changed, 37 insertions(+), 37 deletions(-) diff --git a/internal/cmd/update.go b/internal/cmd/update.go index cb667dbf..1027f85e 100644 --- a/internal/cmd/update.go +++ b/internal/cmd/update.go @@ -226,7 +226,7 @@ func defaultConfirm(label string) (string, error) { return p.Run() } -func makeDownloadURL(version *semver.Version, ostype, archtype string) string { +func makeDownloadURL(version *semver.Version, ostype, arch string) string { const template = "https://github.com/cdr/coder-cli/releases/download/v%s/coder-cli-%s-%s.%s" var ext string switch ostype { @@ -246,7 +246,7 @@ func makeDownloadURL(version *semver.Version, ostype, archtype string) string { fmt.Fprint(&b, version.Prerelease()) } - return fmt.Sprintf(template, b.String(), ostype, archtype, ext) + return fmt.Sprintf(template, b.String(), ostype, arch, ext) } func extractFromArchive(path string, archive []byte) ([]byte, error) { @@ -366,7 +366,7 @@ func getAPIVersionUnauthed(client getter, baseURL url.URL) (*semver.Version, err // HasFilePathPrefix reports whether the filesystem path s // begins with the elements in prefix. -// Lifted from github.com/golang/go/blob/master/src/cmd/internal/str/path.go +// Lifted from github.com/golang/go/blob/master/src/cmd/internal/str/path.go. func HasFilePathPrefix(s, prefix string) bool { sv := strings.ToUpper(filepath.VolumeName(s)) pv := strings.ToUpper(filepath.VolumeName(prefix)) @@ -389,7 +389,7 @@ func HasFilePathPrefix(s, prefix string) bool { } } -// defaultExec wraps exec.CommandContext +// defaultExec wraps exec.CommandContext. func defaultExec(ctx context.Context, cmd string, args ...string) ([]byte, error) { return exec.CommandContext(ctx, cmd, args...).CombinedOutput() } diff --git a/internal/cmd/update_test.go b/internal/cmd/update_test.go index 5587c5a6..648a8055 100644 --- a/internal/cmd/update_test.go +++ b/internal/cmd/update_test.go @@ -28,11 +28,13 @@ const ( fakeOldVersion = "1.22.4-rc.5+678-gabcdef-12345678" fakeReleaseURLLinux = "https://github.com/cdr/coder-cli/releases/download/v1.23.4-rc.5/coder-cli-linux-amd64.tar.gz" fakeReleaseURLWindows = "https://github.com/cdr/coder-cli/releases/download/v1.23.4-rc.5/coder-cli-windows-amd64.zip" + goosWindows = "windows" + goosLinux = "linux" ) var ( apiPrivateVersionURL = fakeCoderURL + "/api/private/version" - fakeNewVersionJson = fmt.Sprintf(`{"version":%q}`, fakeNewVersion) + fakeNewVersionJSON = fmt.Sprintf(`{"version":%q}`, fakeNewVersion) ) func Test_updater_run(t *testing.T) { @@ -45,7 +47,7 @@ func Test_updater_run(t *testing.T) { Execer *fakeExecer ExecutablePath string Fakefs afero.Fs - HttpClient *fakeGetter + HTTPClient *fakeGetter OsF func() string VersionF func() string } @@ -57,7 +59,7 @@ func Test_updater_run(t *testing.T) { execF: p.Execer.ExecF, executablePath: p.ExecutablePath, fs: p.Fakefs, - httpClient: p.HttpClient, + httpClient: p.HTTPClient, osF: p.OsF, versionF: p.VersionF, } @@ -80,9 +82,9 @@ func Test_updater_run(t *testing.T) { Ctx: ctx, ExecutablePath: fakeExePathLinux, Fakefs: fakefs, - HttpClient: newFakeGetter(t), + HTTPClient: newFakeGetter(t), // Default to GOOS=linux - OsF: func() string { return "linux" }, + OsF: func() string { return goosLinux }, // This must be overridden inside run() VersionF: func() string { t.Errorf("unhandled VersionF") @@ -96,7 +98,7 @@ func Test_updater_run(t *testing.T) { run(t, "update coder - noop", func(t *testing.T, p *params) { fakeFile(t, p.Fakefs, fakeExePathLinux, 0755, fakeNewVersion) - p.HttpClient.M[apiPrivateVersionURL] = newFakeGetterResponse([]byte(fakeNewVersionJson), 200, variadicS(), nil) + p.HTTPClient.M[apiPrivateVersionURL] = newFakeGetterResponse([]byte(fakeNewVersionJSON), 200, variadicS(), nil) p.VersionF = func() string { return fakeNewVersion } u := fromParams(p) assertFileContent(t, p.Fakefs, fakeExePathLinux, fakeNewVersion) @@ -107,8 +109,8 @@ func Test_updater_run(t *testing.T) { run(t, "update coder - old to new", func(t *testing.T, p *params) { fakeFile(t, p.Fakefs, fakeExePathLinux, 0755, fakeOldVersion) - p.HttpClient.M[apiPrivateVersionURL] = newFakeGetterResponse([]byte(fakeNewVersionJson), 200, variadicS(), nil) - p.HttpClient.M[fakeReleaseURLLinux] = newFakeGetterResponse(fakeValidTgzBytes, 200, variadicS(), nil) + p.HTTPClient.M[apiPrivateVersionURL] = newFakeGetterResponse([]byte(fakeNewVersionJSON), 200, variadicS(), nil) + p.HTTPClient.M[fakeReleaseURLLinux] = newFakeGetterResponse(fakeValidTgzBytes, 200, variadicS(), nil) p.VersionF = func() string { return fakeOldVersion } p.ConfirmF = fakeConfirmYes u := fromParams(p) @@ -121,8 +123,8 @@ func Test_updater_run(t *testing.T) { run(t, "update coder - old to new - binary renamed", func(t *testing.T, p *params) { p.ExecutablePath = "/home/user/bin/coder-cli" fakeFile(t, p.Fakefs, p.ExecutablePath, 0755, fakeOldVersion) - p.HttpClient.M[apiPrivateVersionURL] = newFakeGetterResponse([]byte(fakeNewVersionJson), 200, variadicS(), nil) - p.HttpClient.M[fakeReleaseURLLinux] = newFakeGetterResponse(fakeValidTgzBytes, 200, variadicS(), nil) + p.HTTPClient.M[apiPrivateVersionURL] = newFakeGetterResponse([]byte(fakeNewVersionJSON), 200, variadicS(), nil) + p.HTTPClient.M[fakeReleaseURLLinux] = newFakeGetterResponse(fakeValidTgzBytes, 200, variadicS(), nil) p.VersionF = func() string { return fakeOldVersion } p.ConfirmF = fakeConfirmYes u := fromParams(p) @@ -133,11 +135,11 @@ func Test_updater_run(t *testing.T) { }) run(t, "update coder - old to new - windows", func(t *testing.T, p *params) { - p.OsF = func() string { return "windows" } + p.OsF = func() string { return goosWindows } p.ExecutablePath = fakeExePathWindows fakeFile(t, p.Fakefs, fakeExePathWindows, 0755, fakeOldVersion) - p.HttpClient.M[apiPrivateVersionURL] = newFakeGetterResponse([]byte(fakeNewVersionJson), 200, variadicS(), nil) - p.HttpClient.M[fakeReleaseURLWindows] = newFakeGetterResponse(fakeValidZipBytes, 200, variadicS(), nil) + p.HTTPClient.M[apiPrivateVersionURL] = newFakeGetterResponse([]byte(fakeNewVersionJSON), 200, variadicS(), nil) + p.HTTPClient.M[fakeReleaseURLWindows] = newFakeGetterResponse(fakeValidZipBytes, 200, variadicS(), nil) p.VersionF = func() string { return fakeOldVersion } p.ConfirmF = fakeConfirmYes u := fromParams(p) @@ -149,8 +151,8 @@ func Test_updater_run(t *testing.T) { run(t, "update coder - old to new forced", func(t *testing.T, p *params) { fakeFile(t, p.Fakefs, fakeExePathLinux, 0755, fakeOldVersion) - p.HttpClient.M[apiPrivateVersionURL] = newFakeGetterResponse([]byte(fakeNewVersionJson), 200, variadicS(), nil) - p.HttpClient.M[fakeReleaseURLLinux] = newFakeGetterResponse(fakeValidTgzBytes, 200, variadicS(), nil) + p.HTTPClient.M[apiPrivateVersionURL] = newFakeGetterResponse([]byte(fakeNewVersionJSON), 200, variadicS(), nil) + p.HTTPClient.M[fakeReleaseURLLinux] = newFakeGetterResponse(fakeValidTgzBytes, 200, variadicS(), nil) p.VersionF = func() string { return fakeOldVersion } u := fromParams(p) assertFileContent(t, p.Fakefs, fakeExePathLinux, fakeOldVersion) @@ -161,7 +163,7 @@ func Test_updater_run(t *testing.T) { run(t, "update coder - user cancelled", func(t *testing.T, p *params) { fakeFile(t, p.Fakefs, fakeExePathLinux, 0755, fakeOldVersion) - p.HttpClient.M[apiPrivateVersionURL] = newFakeGetterResponse([]byte(fakeNewVersionJson), 200, variadicS(), nil) + p.HTTPClient.M[apiPrivateVersionURL] = newFakeGetterResponse([]byte(fakeNewVersionJSON), 200, variadicS(), nil) p.VersionF = func() string { return fakeOldVersion } p.ConfirmF = fakeConfirmNo u := fromParams(p) @@ -198,7 +200,7 @@ func Test_updater_run(t *testing.T) { run(t, "update coder - fetch api version failure", func(t *testing.T, p *params) { fakeFile(t, p.Fakefs, fakeExePathLinux, 0755, fakeOldVersion) - p.HttpClient.M[apiPrivateVersionURL] = newFakeGetterResponse([]byte{}, 401, variadicS(), net.ErrClosed) + p.HTTPClient.M[apiPrivateVersionURL] = newFakeGetterResponse([]byte{}, 401, variadicS(), net.ErrClosed) p.VersionF = func() string { return fakeOldVersion } u := fromParams(p) assertFileContent(t, p.Fakefs, fakeExePathLinux, fakeOldVersion) @@ -209,8 +211,8 @@ func Test_updater_run(t *testing.T) { run(t, "update coder - failed to fetch URL", func(t *testing.T, p *params) { fakeFile(t, p.Fakefs, fakeExePathLinux, 0755, fakeOldVersion) - p.HttpClient.M[apiPrivateVersionURL] = newFakeGetterResponse([]byte(fakeNewVersionJson), 200, variadicS(), nil) - p.HttpClient.M[fakeReleaseURLLinux] = newFakeGetterResponse([]byte{}, 0, variadicS(), net.ErrClosed) + p.HTTPClient.M[apiPrivateVersionURL] = newFakeGetterResponse([]byte(fakeNewVersionJSON), 200, variadicS(), nil) + p.HTTPClient.M[fakeReleaseURLLinux] = newFakeGetterResponse([]byte{}, 0, variadicS(), net.ErrClosed) p.VersionF = func() string { return fakeOldVersion } p.ConfirmF = fakeConfirmYes u := fromParams(p) @@ -222,8 +224,8 @@ func Test_updater_run(t *testing.T) { run(t, "update coder - release URL 404", func(t *testing.T, p *params) { fakeFile(t, p.Fakefs, fakeExePathLinux, 0755, fakeOldVersion) - p.HttpClient.M[apiPrivateVersionURL] = newFakeGetterResponse([]byte(fakeNewVersionJson), 200, variadicS(), nil) - p.HttpClient.M[fakeReleaseURLLinux] = newFakeGetterResponse([]byte{}, 404, variadicS(), nil) + p.HTTPClient.M[apiPrivateVersionURL] = newFakeGetterResponse([]byte(fakeNewVersionJSON), 200, variadicS(), nil) + p.HTTPClient.M[fakeReleaseURLLinux] = newFakeGetterResponse([]byte{}, 404, variadicS(), nil) p.VersionF = func() string { return fakeOldVersion } p.ConfirmF = fakeConfirmYes u := fromParams(p) @@ -235,8 +237,8 @@ func Test_updater_run(t *testing.T) { run(t, "update coder - invalid tgz archive", func(t *testing.T, p *params) { fakeFile(t, p.Fakefs, fakeExePathLinux, 0755, fakeOldVersion) - p.HttpClient.M[apiPrivateVersionURL] = newFakeGetterResponse([]byte(fakeNewVersionJson), 200, variadicS(), nil) - p.HttpClient.M[fakeReleaseURLLinux] = newFakeGetterResponse([]byte{}, 200, variadicS(), nil) + p.HTTPClient.M[apiPrivateVersionURL] = newFakeGetterResponse([]byte(fakeNewVersionJSON), 200, variadicS(), nil) + p.HTTPClient.M[fakeReleaseURLLinux] = newFakeGetterResponse([]byte{}, 200, variadicS(), nil) p.VersionF = func() string { return fakeOldVersion } p.ConfirmF = fakeConfirmYes u := fromParams(p) @@ -247,11 +249,11 @@ func Test_updater_run(t *testing.T) { }) run(t, "update coder - invalid zip archive", func(t *testing.T, p *params) { - p.OsF = func() string { return "windows" } + p.OsF = func() string { return goosWindows } p.ExecutablePath = fakeExePathWindows fakeFile(t, p.Fakefs, fakeExePathWindows, 0755, fakeOldVersion) - p.HttpClient.M[apiPrivateVersionURL] = newFakeGetterResponse([]byte(fakeNewVersionJson), 200, variadicS(), nil) - p.HttpClient.M[fakeReleaseURLWindows] = newFakeGetterResponse([]byte{}, 200, variadicS(), nil) + p.HTTPClient.M[apiPrivateVersionURL] = newFakeGetterResponse([]byte(fakeNewVersionJSON), 200, variadicS(), nil) + p.HTTPClient.M[fakeReleaseURLWindows] = newFakeGetterResponse([]byte{}, 200, variadicS(), nil) p.VersionF = func() string { return fakeOldVersion } p.ConfirmF = fakeConfirmYes u := fromParams(p) @@ -265,8 +267,8 @@ func Test_updater_run(t *testing.T) { rwfs := p.Fakefs p.Fakefs = afero.NewReadOnlyFs(rwfs) fakeFile(t, rwfs, fakeExePathLinux, 0755, fakeOldVersion) - p.HttpClient.M[apiPrivateVersionURL] = newFakeGetterResponse([]byte(fakeNewVersionJson), 200, variadicS(), nil) - p.HttpClient.M[fakeReleaseURLLinux] = newFakeGetterResponse(fakeValidTgzBytes, 200, variadicS(), nil) + p.HTTPClient.M[apiPrivateVersionURL] = newFakeGetterResponse([]byte(fakeNewVersionJSON), 200, variadicS(), nil) + p.HTTPClient.M[fakeReleaseURLLinux] = newFakeGetterResponse(fakeValidTgzBytes, 200, variadicS(), nil) p.VersionF = func() string { return fakeOldVersion } p.ConfirmF = fakeConfirmYes u := fromParams(p) @@ -276,7 +278,7 @@ func Test_updater_run(t *testing.T) { assertFileContent(t, p.Fakefs, fakeExePathLinux, fakeOldVersion) }) - if runtime.GOOS == "windows" { + if runtime.GOOS == goosWindows { run(t, "update coder - path blocklist - windows", func(t *testing.T, p *params) { p.ExecutablePath = `C:\Windows\system32\coder.exe` u := fromParams(p) @@ -314,7 +316,7 @@ func Test_updater_run(t *testing.T) { } } -// fakeGetter mocks HTTP requests +// fakeGetter mocks HTTP requests. type fakeGetter struct { M map[string]*fakeGetterResponse T *testing.T @@ -344,7 +346,7 @@ type fakeGetterResponse struct { Err error } -// newFakeGetterResponse is a convenience function for mocking HTTP requests +// newFakeGetterResponse is a convenience function for mocking HTTP requests. func newFakeGetterResponse(body []byte, code int, headers []string, err error) *fakeGetterResponse { resp := &http.Response{} resp.Body = ioutil.NopCloser(bytes.NewReader(body)) @@ -376,7 +378,6 @@ func fakeConfirmNo(_ string) (string, error) { return "", promptui.ErrAbort } -//nolint:unparam func fakeFile(t *testing.T, fs afero.Fs, name string, perm fs.FileMode, content string) { t.Helper() err := fs.MkdirAll(filepath.Dir(name), 0750) @@ -395,7 +396,6 @@ func fakeFile(t *testing.T, fs afero.Fs, name string, perm fs.FileMode, content } } -//nolint:unparam func assertFileContent(t *testing.T, fs afero.Fs, name string, content string) { t.Helper() f, err := fs.OpenFile(name, os.O_RDONLY, 0) From 60a75a1c2cb9e6db669098ccf99f26b9a3022f51 Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Mon, 16 Aug 2021 11:18:11 +0000 Subject: [PATCH 14/25] internal/cmd/update_test.go: assertCLIError helper function for clog.CLIError --- internal/cmd/update_test.go | 60 ++++++++++++++++++++++++++----------- 1 file changed, 42 insertions(+), 18 deletions(-) diff --git a/internal/cmd/update_test.go b/internal/cmd/update_test.go index 648a8055..f2f4c21a 100644 --- a/internal/cmd/update_test.go +++ b/internal/cmd/update_test.go @@ -7,7 +7,6 @@ import ( "fmt" "io/fs" "io/ioutil" - "net" "net/http" "os" "path/filepath" @@ -15,9 +14,11 @@ import ( "strings" "testing" + "cdr.dev/coder-cli/pkg/clog" "cdr.dev/slog/sloggers/slogtest/assert" "github.com/manifoldco/promptui" "github.com/spf13/afero" + "golang.org/x/xerrors" ) const ( @@ -34,7 +35,9 @@ const ( var ( apiPrivateVersionURL = fakeCoderURL + "/api/private/version" + fakeError = xerrors.New("fake error for testing") fakeNewVersionJSON = fmt.Sprintf(`{"version":%q}`, fakeNewVersion) + fakeOldVersionJSON = fmt.Sprintf(`{"version":%q}`, fakeOldVersion) ) func Test_updater_run(t *testing.T) { @@ -169,14 +172,14 @@ func Test_updater_run(t *testing.T) { u := fromParams(p) assertFileContent(t, p.Fakefs, fakeExePathLinux, fakeOldVersion) err := u.Run(p.Ctx, false, fakeCoderURL) - assert.ErrorContains(t, "update coder - user cancelled", err, "failed to confirm update") + assertCLIError(t, "update coder - user cancelled", err, "failed to confirm update", "") assertFileContent(t, p.Fakefs, fakeExePathLinux, fakeOldVersion) }) run(t, "update coder - cannot stat", func(t *testing.T, p *params) { u := fromParams(p) err := u.Run(p.Ctx, false, fakeCoderURL) - assert.ErrorContains(t, "update coder - cannot stat", err, "cannot stat current binary") + assertCLIError(t, "update coder - cannot stat", err, "cannot stat current binary", os.ErrNotExist.Error()) }) run(t, "update coder - no permission", func(t *testing.T, p *params) { @@ -184,7 +187,7 @@ func Test_updater_run(t *testing.T) { u := fromParams(p) assertFileContent(t, p.Fakefs, fakeExePathLinux, fakeOldVersion) err := u.Run(p.Ctx, false, fakeCoderURL) - assert.ErrorContains(t, "update coder - no permission", err, "missing write permission") + assertCLIError(t, "update coder - no permission", err, "missing write permission", "") assertFileContent(t, p.Fakefs, fakeExePathLinux, fakeOldVersion) }) @@ -194,31 +197,31 @@ func Test_updater_run(t *testing.T) { u := fromParams(p) assertFileContent(t, p.Fakefs, fakeExePathLinux, fakeOldVersion) err := u.Run(p.Ctx, false, "h$$p://invalid.url") - assert.ErrorContains(t, "update coder - invalid url", err, "invalid coder URL") + assertCLIError(t, "update coder - invalid url", err, "invalid coder URL", "first path segment in URL cannot contain colon") assertFileContent(t, p.Fakefs, fakeExePathLinux, fakeOldVersion) }) run(t, "update coder - fetch api version failure", func(t *testing.T, p *params) { fakeFile(t, p.Fakefs, fakeExePathLinux, 0755, fakeOldVersion) - p.HTTPClient.M[apiPrivateVersionURL] = newFakeGetterResponse([]byte{}, 401, variadicS(), net.ErrClosed) + p.HTTPClient.M[apiPrivateVersionURL] = newFakeGetterResponse([]byte{}, 401, variadicS(), fakeError) p.VersionF = func() string { return fakeOldVersion } u := fromParams(p) assertFileContent(t, p.Fakefs, fakeExePathLinux, fakeOldVersion) err := u.Run(p.Ctx, false, fakeCoderURL) - assert.ErrorContains(t, "update coder - fetch api version failure", err, "fetch api version") + assertCLIError(t, "update coder - fetch api version failure", err, "fetch api version", fakeError.Error()) assertFileContent(t, p.Fakefs, fakeExePathLinux, fakeOldVersion) }) run(t, "update coder - failed to fetch URL", func(t *testing.T, p *params) { fakeFile(t, p.Fakefs, fakeExePathLinux, 0755, fakeOldVersion) p.HTTPClient.M[apiPrivateVersionURL] = newFakeGetterResponse([]byte(fakeNewVersionJSON), 200, variadicS(), nil) - p.HTTPClient.M[fakeReleaseURLLinux] = newFakeGetterResponse([]byte{}, 0, variadicS(), net.ErrClosed) + p.HTTPClient.M[fakeReleaseURLLinux] = newFakeGetterResponse([]byte{}, 0, variadicS(), fakeError) p.VersionF = func() string { return fakeOldVersion } p.ConfirmF = fakeConfirmYes u := fromParams(p) assertFileContent(t, p.Fakefs, fakeExePathLinux, fakeOldVersion) err := u.Run(p.Ctx, false, fakeCoderURL) - assert.ErrorContains(t, "update coder - release URL 404", err, "failed to fetch URL") + assertCLIError(t, "update coder - failed to fetch URL", err, "failed to fetch URL", fakeError.Error()) assertFileContent(t, p.Fakefs, fakeExePathLinux, fakeOldVersion) }) @@ -231,7 +234,7 @@ func Test_updater_run(t *testing.T) { u := fromParams(p) assertFileContent(t, p.Fakefs, fakeExePathLinux, fakeOldVersion) err := u.Run(p.Ctx, false, fakeCoderURL) - assert.ErrorContains(t, "update coder - release URL 404", err, "failed to fetch release") + assertCLIError(t, "update coder - release URL 404", err, "failed to fetch release", "status code 404") assertFileContent(t, p.Fakefs, fakeExePathLinux, fakeOldVersion) }) @@ -244,7 +247,7 @@ func Test_updater_run(t *testing.T) { u := fromParams(p) assertFileContent(t, p.Fakefs, fakeExePathLinux, fakeOldVersion) err := u.Run(p.Ctx, false, fakeCoderURL) - assert.ErrorContains(t, "update coder - invalid archive", err, "failed to extract coder binary from archive") + assertCLIError(t, "update coder - invalid tgz archive", err, "failed to extract coder binary from archive", "unknown archive type") assertFileContent(t, p.Fakefs, fakeExePathLinux, fakeOldVersion) }) @@ -259,7 +262,7 @@ func Test_updater_run(t *testing.T) { u := fromParams(p) assertFileContent(t, p.Fakefs, p.ExecutablePath, fakeOldVersion) err := u.Run(p.Ctx, false, fakeCoderURL) - assert.ErrorContains(t, "update coder - invalid archive", err, "failed to extract coder binary from archive") + assertCLIError(t, "update coder - invalid zip archive", err, "failed to extract coder binary from archive", "unknown archive type") assertFileContent(t, p.Fakefs, p.ExecutablePath, fakeOldVersion) }) @@ -274,7 +277,7 @@ func Test_updater_run(t *testing.T) { u := fromParams(p) assertFileContent(t, p.Fakefs, fakeExePathLinux, fakeOldVersion) err := u.Run(p.Ctx, false, fakeCoderURL) - assert.ErrorContains(t, "update coder - read-only fs", err, "failed to create file") + assertCLIError(t, "update coder - read-only fs", err, "failed to create file", "") assertFileContent(t, p.Fakefs, fakeExePathLinux, fakeOldVersion) }) @@ -283,35 +286,35 @@ func Test_updater_run(t *testing.T) { p.ExecutablePath = `C:\Windows\system32\coder.exe` u := fromParams(p) err := u.Run(p.Ctx, false, fakeCoderURL) - assert.ErrorContains(t, "update coder - path blocklist - windows", err, "cowardly refusing to update coder binary") + assertCLIError(t, "update coder - path blocklist - windows", err, "cowardly refusing to update coder binary", "blocklisted prefix") }) } else { run(t, "update coder - path blocklist - coder assets dir", func(t *testing.T, p *params) { p.ExecutablePath = `/var/tmp/coder/coder` u := fromParams(p) err := u.Run(p.Ctx, false, fakeCoderURL) - assert.ErrorContains(t, "update coder - path blocklist - windows", err, "cowardly refusing to update coder binary") + assertCLIError(t, "update coder - path blocklist - windows", err, "cowardly refusing to update coder binary", "blocklisted prefix") }) run(t, "update coder - path blocklist - old homebrew prefix", func(t *testing.T, p *params) { p.Execer.M["brew --prefix"] = fakeExecerResult{[]byte("/usr/local"), nil} p.ExecutablePath = `/usr/local/bin/coder` u := fromParams(p) err := u.Run(p.Ctx, false, fakeCoderURL) - assert.ErrorContains(t, "update coder - path blocklist - old homebrew prefix", err, "cowardly refusing to update coder binary") + assertCLIError(t, "update coder - path blocklist - old homebrew prefix", err, "cowardly refusing to update coder binary", "blocklisted prefix") }) run(t, "update coder - path blocklist - new homebrew prefix", func(t *testing.T, p *params) { p.Execer.M["brew --prefix"] = fakeExecerResult{[]byte("/opt/homebrew"), nil} p.ExecutablePath = `/opt/homebrew/bin/coder` u := fromParams(p) err := u.Run(p.Ctx, false, fakeCoderURL) - assert.ErrorContains(t, "update coder - path blocklist - new homebrew prefix", err, "cowardly refusing to update coder binary") + assertCLIError(t, "update coder - path blocklist - new homebrew prefix", err, "cowardly refusing to update coder binary", "blocklisted prefix") }) run(t, "update coder - path blocklist - linuxbrew", func(t *testing.T, p *params) { p.Execer.M["brew --prefix"] = fakeExecerResult{[]byte("/home/user/.linuxbrew"), nil} p.ExecutablePath = `/home/user/.linuxbrew/bin/coder` u := fromParams(p) err := u.Run(p.Ctx, false, fakeCoderURL) - assert.ErrorContains(t, "update coder - path blocklist - linuxbrew", err, "cowardly refusing to update coder binary") + assertCLIError(t, "update coder - path blocklist - linuxbrew", err, "cowardly refusing to update coder binary", "blocklisted prefix") }) } } @@ -408,6 +411,27 @@ func assertFileContent(t *testing.T, fs afero.Fs, name string, content string) { assert.Equal(t, "assert content equal", content, string(b)) } +func assertCLIError(t *testing.T, name string, err error, expectedHeader, expectedLines string) { + t.Helper() + cliError, ok := err.(clog.CLIError) + if !ok { + t.Errorf("%s: assert cli error: %+v is not a cli error", name, err) + } + + if !strings.Contains(err.Error(), expectedHeader) { + t.Errorf("%s: assert cli error: expected header %q to contain %q", name, err.Error(), expectedHeader) + } + + if expectedLines == "" { + return + } + + fullLines := strings.Join(cliError.Lines, "\n") + if !strings.Contains(fullLines, expectedLines) { + t.Errorf("%s: assert cli error: expected %q to contain %q", name, fullLines, expectedLines) + } +} + // this is a valid tgz archive containing a single file named 'coder' with permissions 0751 // containing the string "1.23.4-rc.5+678-gabcdef-12345678". var fakeValidTgzBytes, _ = base64.StdEncoding.DecodeString(`H4sIAAAAAAAAA+3QsQ4CIRCEYR6F3oC7wIqvc3KnpQnq+3tGCwsTK3LN/zWTTDWZuG/XeeluJFlV From 26984a21d2860d4320922e803cdd46cc4efdc1df Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Mon, 16 Aug 2021 11:33:22 +0000 Subject: [PATCH 15/25] internal/cmd/update.go: allow explicitly specifying version --- docs/coder_update.md | 7 ++-- internal/cmd/update.go | 83 ++++++++++++++++++++++++------------- internal/cmd/update_test.go | 67 ++++++++++++++++++++---------- 3 files changed, 103 insertions(+), 54 deletions(-) diff --git a/docs/coder_update.md b/docs/coder_update.md index 30266285..8bcc9fae 100644 --- a/docs/coder_update.md +++ b/docs/coder_update.md @@ -13,9 +13,10 @@ coder update [flags] ### Options ``` - --coder string coder instance against which to match version - --force do not prompt for confirmation - -h, --help help for update + --coder string query this coder instance for the matching version + --force do not prompt for confirmation + -h, --help help for update + --version string explicitly specify which version to fetch and install ``` ### Options inherited from parent commands diff --git a/internal/cmd/update.go b/internal/cmd/update.go index 1027f85e..be8949f9 100644 --- a/internal/cmd/update.go +++ b/internal/cmd/update.go @@ -45,8 +45,9 @@ type updater struct { func updateCmd() *cobra.Command { var ( - force bool - coderURL string + force bool + coderURL string + versionArg string ) cmd := &cobra.Command{ @@ -73,12 +74,13 @@ func updateCmd() *cobra.Command { osF: func() string { return runtime.GOOS }, versionF: func() string { return version.Version }, } - return updater.Run(ctx, force, coderURL) + return updater.Run(ctx, force, coderURL, versionArg) }, } cmd.Flags().BoolVar(&force, "force", false, "do not prompt for confirmation") - cmd.Flags().StringVar(&coderURL, "coder", "", "coder instance against which to match version") + cmd.Flags().StringVar(&coderURL, "coder", "", "query this coder instance for the matching version") + cmd.Flags().StringVar(&versionArg, "version", "", "explicitly specify which version to fetch and install") return cmd } @@ -87,7 +89,7 @@ type getter interface { Get(url string) (*http.Response, error) } -func (u *updater) Run(ctx context.Context, force bool, coderURLString string) error { +func (u *updater) Run(ctx context.Context, force bool, coderURLArg string, versionArg string) error { // Check under following directories and warn if coder binary is under them: // * C:\Windows\ // * homebrew prefix @@ -119,34 +121,18 @@ func (u *updater) Run(ctx context.Context, force bool, coderURLString string) er return clog.Fatal("preflight: missing write permission on current binary") } - var coderURL *url.URL - if coderURLString == "" { - coderURL, err = getCoderConfigURL() - if err != nil { - return clog.Fatal( - "Unable to automatically determine coder URL", - clog.Causef(err.Error()), - clog.BlankLine, - clog.Tipf("use --coder to specify coder URL"), - ) - } - } else { - coderURL, err = url.Parse(coderURLString) - if err != nil { - return clog.Fatal("invalid coder URL", err.Error()) - } - } + clog.LogInfo(fmt.Sprintf("Current version of coder-cli is %s", version.Version)) - desiredVersion, err := getAPIVersionUnauthed(u.httpClient, *coderURL) + desiredVersion, err := getDesiredVersion(u.httpClient, coderURLArg, versionArg) if err != nil { - return clog.Fatal("fetch api version", clog.Causef(err.Error())) + return clog.Fatal("failed to determine desired version of coder", clog.Causef(err.Error())) } - clog.LogInfo(fmt.Sprintf("Coder instance at %q reports version %s", coderURL.String(), desiredVersion.String())) - clog.LogInfo(fmt.Sprintf("Current version of coder-cli is %s", version.Version)) - - if currentVersion, err := semver.StrictNewVersion(u.versionF()); err == nil { - if desiredVersion.Compare(currentVersion) == 0 { + currentVersion, err := semver.StrictNewVersion(u.versionF()) + if err != nil { + clog.LogWarn("failed to determine current version of coder-cli", clog.Causef(err.Error())) + } else { + if currentVersion.Compare(desiredVersion) == 0 { clog.LogInfo("Up to date!") return nil } @@ -221,6 +207,45 @@ func (u *updater) Run(ctx context.Context, force bool, coderURLString string) er return nil } +func getDesiredVersion(httpClient getter, coderURLArg string, versionArg string) (*semver.Version, error) { + var coderURL *url.URL + var desiredVersion *semver.Version + var err error + + if coderURLArg != "" && versionArg != "" { + clog.LogWarn(fmt.Sprintf("ignoring the version reported by %q", coderURLArg), clog.Causef("--version flag was specified explicitly")) + } + + if versionArg != "" { + desiredVersion, err = semver.StrictNewVersion(versionArg) + if err != nil { + return &semver.Version{}, xerrors.Errorf("parse desired version arg: %w", err) + } + return desiredVersion, nil + } + + if coderURLArg == "" { + coderURL, err = getCoderConfigURL() + if err != nil { + return &semver.Version{}, xerrors.Errorf("get coder url: %w", err) + } + } else { + coderURL, err = url.Parse(coderURLArg) + if err != nil { + return &semver.Version{}, xerrors.Errorf("parse coder url arg: %w", err) + } + } + + desiredVersion, err = getAPIVersionUnauthed(httpClient, *coderURL) + if err != nil { + return &semver.Version{}, xerrors.Errorf("query coder version: %w", err) + } + + clog.LogInfo(fmt.Sprintf("Coder instance at %q reports version %s", coderURL.String(), desiredVersion.String())) + + return desiredVersion, nil +} + func defaultConfirm(label string) (string, error) { p := promptui.Prompt{IsConfirm: true, Label: label} return p.Run() diff --git a/internal/cmd/update_test.go b/internal/cmd/update_test.go index f2f4c21a..710b50fc 100644 --- a/internal/cmd/update_test.go +++ b/internal/cmd/update_test.go @@ -105,11 +105,24 @@ func Test_updater_run(t *testing.T) { p.VersionF = func() string { return fakeNewVersion } u := fromParams(p) assertFileContent(t, p.Fakefs, fakeExePathLinux, fakeNewVersion) - err := u.Run(p.Ctx, false, fakeCoderURL) + err := u.Run(p.Ctx, false, fakeCoderURL, "") assert.Success(t, "update coder - noop", err) assertFileContent(t, p.Fakefs, fakeExePathLinux, fakeNewVersion) }) + run(t, "update coder - explicit version specified", func(t *testing.T, p *params) { + fakeFile(t, p.Fakefs, fakeExePathLinux, 0755, fakeOldVersion) + p.HTTPClient.M[apiPrivateVersionURL] = newFakeGetterResponse([]byte(fakeOldVersionJSON), 200, variadicS(), nil) + p.HTTPClient.M[fakeReleaseURLLinux] = newFakeGetterResponse(fakeValidTgzBytes, 200, variadicS(), nil) + p.VersionF = func() string { return fakeOldVersion } + p.ConfirmF = fakeConfirmYes + u := fromParams(p) + assertFileContent(t, p.Fakefs, fakeExePathLinux, fakeOldVersion) + err := u.Run(p.Ctx, false, fakeCoderURL, fakeNewVersion) + assert.Success(t, "update coder - explicit version specified", err) + assertFileContent(t, p.Fakefs, fakeExePathLinux, fakeNewVersion) + }) + run(t, "update coder - old to new", func(t *testing.T, p *params) { fakeFile(t, p.Fakefs, fakeExePathLinux, 0755, fakeOldVersion) p.HTTPClient.M[apiPrivateVersionURL] = newFakeGetterResponse([]byte(fakeNewVersionJSON), 200, variadicS(), nil) @@ -118,7 +131,7 @@ func Test_updater_run(t *testing.T) { p.ConfirmF = fakeConfirmYes u := fromParams(p) assertFileContent(t, p.Fakefs, fakeExePathLinux, fakeOldVersion) - err := u.Run(p.Ctx, false, fakeCoderURL) + err := u.Run(p.Ctx, false, fakeCoderURL, "") assert.Success(t, "update coder - old to new", err) assertFileContent(t, p.Fakefs, fakeExePathLinux, fakeNewVersion) }) @@ -132,7 +145,7 @@ func Test_updater_run(t *testing.T) { p.ConfirmF = fakeConfirmYes u := fromParams(p) assertFileContent(t, p.Fakefs, p.ExecutablePath, fakeOldVersion) - err := u.Run(p.Ctx, false, fakeCoderURL) + err := u.Run(p.Ctx, false, fakeCoderURL, "") assert.Success(t, "update coder - old to new - binary renamed", err) assertFileContent(t, p.Fakefs, p.ExecutablePath, fakeNewVersion) }) @@ -147,7 +160,7 @@ func Test_updater_run(t *testing.T) { p.ConfirmF = fakeConfirmYes u := fromParams(p) assertFileContent(t, p.Fakefs, fakeExePathWindows, fakeOldVersion) - err := u.Run(p.Ctx, false, fakeCoderURL) + err := u.Run(p.Ctx, false, fakeCoderURL, "") assert.Success(t, "update coder - old to new - windows", err) assertFileContent(t, p.Fakefs, fakeExePathWindows, fakeNewVersion) }) @@ -159,7 +172,7 @@ func Test_updater_run(t *testing.T) { p.VersionF = func() string { return fakeOldVersion } u := fromParams(p) assertFileContent(t, p.Fakefs, fakeExePathLinux, fakeOldVersion) - err := u.Run(p.Ctx, true, fakeCoderURL) + err := u.Run(p.Ctx, true, fakeCoderURL, "") assert.Success(t, "update coder - old to new forced", err) assertFileContent(t, p.Fakefs, fakeExePathLinux, fakeNewVersion) }) @@ -171,14 +184,14 @@ func Test_updater_run(t *testing.T) { p.ConfirmF = fakeConfirmNo u := fromParams(p) assertFileContent(t, p.Fakefs, fakeExePathLinux, fakeOldVersion) - err := u.Run(p.Ctx, false, fakeCoderURL) + err := u.Run(p.Ctx, false, fakeCoderURL, "") assertCLIError(t, "update coder - user cancelled", err, "failed to confirm update", "") assertFileContent(t, p.Fakefs, fakeExePathLinux, fakeOldVersion) }) run(t, "update coder - cannot stat", func(t *testing.T, p *params) { u := fromParams(p) - err := u.Run(p.Ctx, false, fakeCoderURL) + err := u.Run(p.Ctx, false, fakeCoderURL, "") assertCLIError(t, "update coder - cannot stat", err, "cannot stat current binary", os.ErrNotExist.Error()) }) @@ -186,18 +199,28 @@ func Test_updater_run(t *testing.T) { fakeFile(t, p.Fakefs, fakeExePathLinux, 0400, fakeOldVersion) u := fromParams(p) assertFileContent(t, p.Fakefs, fakeExePathLinux, fakeOldVersion) - err := u.Run(p.Ctx, false, fakeCoderURL) + err := u.Run(p.Ctx, false, fakeCoderURL, "") assertCLIError(t, "update coder - no permission", err, "missing write permission", "") assertFileContent(t, p.Fakefs, fakeExePathLinux, fakeOldVersion) }) + run(t, "update coder - invalid version arg", func(t *testing.T, p *params) { + fakeFile(t, p.Fakefs, fakeExePathLinux, 0755, fakeOldVersion) + p.VersionF = func() string { return fakeOldVersion } + u := fromParams(p) + assertFileContent(t, p.Fakefs, fakeExePathLinux, fakeOldVersion) + err := u.Run(p.Ctx, false, fakeCoderURL, "Invalid Semantic Version") + assertCLIError(t, "update coder - invalid version arg", err, "failed to determine desired version of coder", "Invalid Semantic Version") + assertFileContent(t, p.Fakefs, fakeExePathLinux, fakeOldVersion) + }) + run(t, "update coder - invalid url", func(t *testing.T, p *params) { fakeFile(t, p.Fakefs, fakeExePathLinux, 0755, fakeOldVersion) p.VersionF = func() string { return fakeOldVersion } u := fromParams(p) assertFileContent(t, p.Fakefs, fakeExePathLinux, fakeOldVersion) - err := u.Run(p.Ctx, false, "h$$p://invalid.url") - assertCLIError(t, "update coder - invalid url", err, "invalid coder URL", "first path segment in URL cannot contain colon") + err := u.Run(p.Ctx, false, "h$$p://invalid.url", "") + assertCLIError(t, "update coder - invalid url", err, "failed to determine desired version of coder", "first path segment in URL cannot contain colon") assertFileContent(t, p.Fakefs, fakeExePathLinux, fakeOldVersion) }) @@ -207,8 +230,8 @@ func Test_updater_run(t *testing.T) { p.VersionF = func() string { return fakeOldVersion } u := fromParams(p) assertFileContent(t, p.Fakefs, fakeExePathLinux, fakeOldVersion) - err := u.Run(p.Ctx, false, fakeCoderURL) - assertCLIError(t, "update coder - fetch api version failure", err, "fetch api version", fakeError.Error()) + err := u.Run(p.Ctx, false, fakeCoderURL, "") + assertCLIError(t, "update coder - fetch api version failure", err, "failed to determine desired version of coder", fakeError.Error()) assertFileContent(t, p.Fakefs, fakeExePathLinux, fakeOldVersion) }) @@ -220,7 +243,7 @@ func Test_updater_run(t *testing.T) { p.ConfirmF = fakeConfirmYes u := fromParams(p) assertFileContent(t, p.Fakefs, fakeExePathLinux, fakeOldVersion) - err := u.Run(p.Ctx, false, fakeCoderURL) + err := u.Run(p.Ctx, false, fakeCoderURL, "") assertCLIError(t, "update coder - failed to fetch URL", err, "failed to fetch URL", fakeError.Error()) assertFileContent(t, p.Fakefs, fakeExePathLinux, fakeOldVersion) }) @@ -233,7 +256,7 @@ func Test_updater_run(t *testing.T) { p.ConfirmF = fakeConfirmYes u := fromParams(p) assertFileContent(t, p.Fakefs, fakeExePathLinux, fakeOldVersion) - err := u.Run(p.Ctx, false, fakeCoderURL) + err := u.Run(p.Ctx, false, fakeCoderURL, "") assertCLIError(t, "update coder - release URL 404", err, "failed to fetch release", "status code 404") assertFileContent(t, p.Fakefs, fakeExePathLinux, fakeOldVersion) }) @@ -246,7 +269,7 @@ func Test_updater_run(t *testing.T) { p.ConfirmF = fakeConfirmYes u := fromParams(p) assertFileContent(t, p.Fakefs, fakeExePathLinux, fakeOldVersion) - err := u.Run(p.Ctx, false, fakeCoderURL) + err := u.Run(p.Ctx, false, fakeCoderURL, "") assertCLIError(t, "update coder - invalid tgz archive", err, "failed to extract coder binary from archive", "unknown archive type") assertFileContent(t, p.Fakefs, fakeExePathLinux, fakeOldVersion) }) @@ -261,7 +284,7 @@ func Test_updater_run(t *testing.T) { p.ConfirmF = fakeConfirmYes u := fromParams(p) assertFileContent(t, p.Fakefs, p.ExecutablePath, fakeOldVersion) - err := u.Run(p.Ctx, false, fakeCoderURL) + err := u.Run(p.Ctx, false, fakeCoderURL, "") assertCLIError(t, "update coder - invalid zip archive", err, "failed to extract coder binary from archive", "unknown archive type") assertFileContent(t, p.Fakefs, p.ExecutablePath, fakeOldVersion) }) @@ -276,7 +299,7 @@ func Test_updater_run(t *testing.T) { p.ConfirmF = fakeConfirmYes u := fromParams(p) assertFileContent(t, p.Fakefs, fakeExePathLinux, fakeOldVersion) - err := u.Run(p.Ctx, false, fakeCoderURL) + err := u.Run(p.Ctx, false, fakeCoderURL, "") assertCLIError(t, "update coder - read-only fs", err, "failed to create file", "") assertFileContent(t, p.Fakefs, fakeExePathLinux, fakeOldVersion) }) @@ -285,35 +308,35 @@ func Test_updater_run(t *testing.T) { run(t, "update coder - path blocklist - windows", func(t *testing.T, p *params) { p.ExecutablePath = `C:\Windows\system32\coder.exe` u := fromParams(p) - err := u.Run(p.Ctx, false, fakeCoderURL) + err := u.Run(p.Ctx, false, fakeCoderURL, "") assertCLIError(t, "update coder - path blocklist - windows", err, "cowardly refusing to update coder binary", "blocklisted prefix") }) } else { run(t, "update coder - path blocklist - coder assets dir", func(t *testing.T, p *params) { p.ExecutablePath = `/var/tmp/coder/coder` u := fromParams(p) - err := u.Run(p.Ctx, false, fakeCoderURL) + err := u.Run(p.Ctx, false, fakeCoderURL, "") assertCLIError(t, "update coder - path blocklist - windows", err, "cowardly refusing to update coder binary", "blocklisted prefix") }) run(t, "update coder - path blocklist - old homebrew prefix", func(t *testing.T, p *params) { p.Execer.M["brew --prefix"] = fakeExecerResult{[]byte("/usr/local"), nil} p.ExecutablePath = `/usr/local/bin/coder` u := fromParams(p) - err := u.Run(p.Ctx, false, fakeCoderURL) + err := u.Run(p.Ctx, false, fakeCoderURL, "") assertCLIError(t, "update coder - path blocklist - old homebrew prefix", err, "cowardly refusing to update coder binary", "blocklisted prefix") }) run(t, "update coder - path blocklist - new homebrew prefix", func(t *testing.T, p *params) { p.Execer.M["brew --prefix"] = fakeExecerResult{[]byte("/opt/homebrew"), nil} p.ExecutablePath = `/opt/homebrew/bin/coder` u := fromParams(p) - err := u.Run(p.Ctx, false, fakeCoderURL) + err := u.Run(p.Ctx, false, fakeCoderURL, "") assertCLIError(t, "update coder - path blocklist - new homebrew prefix", err, "cowardly refusing to update coder binary", "blocklisted prefix") }) run(t, "update coder - path blocklist - linuxbrew", func(t *testing.T, p *params) { p.Execer.M["brew --prefix"] = fakeExecerResult{[]byte("/home/user/.linuxbrew"), nil} p.ExecutablePath = `/home/user/.linuxbrew/bin/coder` u := fromParams(p) - err := u.Run(p.Ctx, false, fakeCoderURL) + err := u.Run(p.Ctx, false, fakeCoderURL, "") assertCLIError(t, "update coder - path blocklist - linuxbrew", err, "cowardly refusing to update coder binary", "blocklisted prefix") }) } From bcaac7b1c5a10ac6bda3f69f9c8fc356a4157a7f Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Mon, 16 Aug 2021 12:17:58 +0000 Subject: [PATCH 16/25] internal/cmd/update.go: validate we can exec new binary --- internal/cmd/update.go | 13 +++++++++++++ internal/cmd/update_test.go | 38 ++++++++++++++++++++++++++++--------- 2 files changed, 42 insertions(+), 9 deletions(-) diff --git a/internal/cmd/update.go b/internal/cmd/update.go index be8949f9..3c285227 100644 --- a/internal/cmd/update.go +++ b/internal/cmd/update.go @@ -199,6 +199,19 @@ func (u *updater) Run(ctx context.Context, force bool, coderURLArg string, versi return clog.Fatal("failed to persist updated coder binary to disk", clog.Causef(err.Error())) } + _ = updatedBin.Close() + + // validate that we can execute the new binary before overwriting + updatedVersionOutput, err := u.execF(ctx, updatedCoderBinaryPath, "--version") + if err != nil { + return clog.Fatal("failed to check version of updated coder binary", + clog.BlankLine, + fmt.Sprintf("output: %q", string(updatedVersionOutput)), + clog.Causef(err.Error())) + } + + clog.LogInfo(fmt.Sprintf("updated binary reports %s", bytes.TrimSpace(updatedVersionOutput))) + if err = u.fs.Rename(updatedCoderBinaryPath, u.executablePath); err != nil { return clog.Fatal("failed to update coder binary in-place", clog.Causef(err.Error())) } diff --git a/internal/cmd/update_test.go b/internal/cmd/update_test.go index 710b50fc..6b58fc48 100644 --- a/internal/cmd/update_test.go +++ b/internal/cmd/update_test.go @@ -22,15 +22,16 @@ import ( ) const ( - fakeExePathLinux = "/home/user/bin/coder" - fakeExePathWindows = `C:\Users\user\bin\coder.exe` - fakeCoderURL = "https://my.cdr.dev/" - fakeNewVersion = "1.23.4-rc.5+678-gabcdef-12345678" - fakeOldVersion = "1.22.4-rc.5+678-gabcdef-12345678" - fakeReleaseURLLinux = "https://github.com/cdr/coder-cli/releases/download/v1.23.4-rc.5/coder-cli-linux-amd64.tar.gz" - fakeReleaseURLWindows = "https://github.com/cdr/coder-cli/releases/download/v1.23.4-rc.5/coder-cli-windows-amd64.zip" - goosWindows = "windows" - goosLinux = "linux" + fakeExePathLinux = "/home/user/bin/coder" + fakeUpdatedExePathLinux = "/home/user/bin/coder.new" + fakeExePathWindows = `C:\Users\user\bin\coder.exe` + fakeCoderURL = "https://my.cdr.dev/" + fakeNewVersion = "1.23.4-rc.5+678-gabcdef-12345678" + fakeOldVersion = "1.22.4-rc.5+678-gabcdef-12345678" + fakeReleaseURLLinux = "https://github.com/cdr/coder-cli/releases/download/v1.23.4-rc.5/coder-cli-linux-amd64.tar.gz" + fakeReleaseURLWindows = "https://github.com/cdr/coder-cli/releases/download/v1.23.4-rc.5/coder-cli-windows-amd64.zip" + goosWindows = "windows" + goosLinux = "linux" ) var ( @@ -116,6 +117,7 @@ func Test_updater_run(t *testing.T) { p.HTTPClient.M[fakeReleaseURLLinux] = newFakeGetterResponse(fakeValidTgzBytes, 200, variadicS(), nil) p.VersionF = func() string { return fakeOldVersion } p.ConfirmF = fakeConfirmYes + p.Execer.M[p.ExecutablePath+".new --version"] = fakeExecerResult{[]byte(fakeNewVersion), nil} u := fromParams(p) assertFileContent(t, p.Fakefs, fakeExePathLinux, fakeOldVersion) err := u.Run(p.Ctx, false, fakeCoderURL, fakeNewVersion) @@ -129,6 +131,7 @@ func Test_updater_run(t *testing.T) { p.HTTPClient.M[fakeReleaseURLLinux] = newFakeGetterResponse(fakeValidTgzBytes, 200, variadicS(), nil) p.VersionF = func() string { return fakeOldVersion } p.ConfirmF = fakeConfirmYes + p.Execer.M[p.ExecutablePath+".new --version"] = fakeExecerResult{[]byte(fakeNewVersion), nil} u := fromParams(p) assertFileContent(t, p.Fakefs, fakeExePathLinux, fakeOldVersion) err := u.Run(p.Ctx, false, fakeCoderURL, "") @@ -143,6 +146,7 @@ func Test_updater_run(t *testing.T) { p.HTTPClient.M[fakeReleaseURLLinux] = newFakeGetterResponse(fakeValidTgzBytes, 200, variadicS(), nil) p.VersionF = func() string { return fakeOldVersion } p.ConfirmF = fakeConfirmYes + p.Execer.M[p.ExecutablePath+".new --version"] = fakeExecerResult{[]byte(fakeNewVersion), nil} u := fromParams(p) assertFileContent(t, p.Fakefs, p.ExecutablePath, fakeOldVersion) err := u.Run(p.Ctx, false, fakeCoderURL, "") @@ -158,6 +162,7 @@ func Test_updater_run(t *testing.T) { p.HTTPClient.M[fakeReleaseURLWindows] = newFakeGetterResponse(fakeValidZipBytes, 200, variadicS(), nil) p.VersionF = func() string { return fakeOldVersion } p.ConfirmF = fakeConfirmYes + p.Execer.M[p.ExecutablePath+".new --version"] = fakeExecerResult{[]byte(fakeNewVersion), nil} u := fromParams(p) assertFileContent(t, p.Fakefs, fakeExePathWindows, fakeOldVersion) err := u.Run(p.Ctx, false, fakeCoderURL, "") @@ -170,6 +175,7 @@ func Test_updater_run(t *testing.T) { p.HTTPClient.M[apiPrivateVersionURL] = newFakeGetterResponse([]byte(fakeNewVersionJSON), 200, variadicS(), nil) p.HTTPClient.M[fakeReleaseURLLinux] = newFakeGetterResponse(fakeValidTgzBytes, 200, variadicS(), nil) p.VersionF = func() string { return fakeOldVersion } + p.Execer.M[p.ExecutablePath+".new --version"] = fakeExecerResult{[]byte(fakeNewVersion), nil} u := fromParams(p) assertFileContent(t, p.Fakefs, fakeExePathLinux, fakeOldVersion) err := u.Run(p.Ctx, true, fakeCoderURL, "") @@ -304,6 +310,20 @@ func Test_updater_run(t *testing.T) { assertFileContent(t, p.Fakefs, fakeExePathLinux, fakeOldVersion) }) + run(t, "update coder - cannot exec new binary", func(t *testing.T, p *params) { + fakeFile(t, p.Fakefs, fakeExePathLinux, 0755, fakeOldVersion) + p.HTTPClient.M[apiPrivateVersionURL] = newFakeGetterResponse([]byte(fakeNewVersionJSON), 200, variadicS(), nil) + p.HTTPClient.M[fakeReleaseURLLinux] = newFakeGetterResponse(fakeValidTgzBytes, 200, variadicS(), nil) + p.VersionF = func() string { return fakeOldVersion } + p.ConfirmF = fakeConfirmYes + p.Execer.M[p.ExecutablePath+".new --version"] = fakeExecerResult{nil, fakeError} + u := fromParams(p) + assertFileContent(t, p.Fakefs, fakeExePathLinux, fakeOldVersion) + err := u.Run(p.Ctx, false, fakeCoderURL, "") + assertCLIError(t, "update coder - cannot exec new binary", err, "failed to check version of updated coder binary", "") + assertFileContent(t, p.Fakefs, fakeExePathLinux, fakeOldVersion) + }) + if runtime.GOOS == goosWindows { run(t, "update coder - path blocklist - windows", func(t *testing.T, p *params) { p.ExecutablePath = `C:\Windows\system32\coder.exe` From 20028766da50bc5a4acfc1d5cfab4bef966dc2c0 Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Mon, 16 Aug 2021 12:39:30 +0000 Subject: [PATCH 17/25] fixup! internal/cmd/update.go: validate we can exec new binary --- internal/cmd/update.go | 8 +++----- internal/cmd/update_test.go | 19 +++++++++---------- 2 files changed, 12 insertions(+), 15 deletions(-) diff --git a/internal/cmd/update.go b/internal/cmd/update.go index 3c285227..5e77c2a6 100644 --- a/internal/cmd/update.go +++ b/internal/cmd/update.go @@ -131,11 +131,9 @@ func (u *updater) Run(ctx context.Context, force bool, coderURLArg string, versi currentVersion, err := semver.StrictNewVersion(u.versionF()) if err != nil { clog.LogWarn("failed to determine current version of coder-cli", clog.Causef(err.Error())) - } else { - if currentVersion.Compare(desiredVersion) == 0 { - clog.LogInfo("Up to date!") - return nil - } + } else if currentVersion.Compare(desiredVersion) == 0 { + clog.LogInfo("Up to date!") + return nil } if !force { diff --git a/internal/cmd/update_test.go b/internal/cmd/update_test.go index 6b58fc48..d261b737 100644 --- a/internal/cmd/update_test.go +++ b/internal/cmd/update_test.go @@ -22,16 +22,15 @@ import ( ) const ( - fakeExePathLinux = "/home/user/bin/coder" - fakeUpdatedExePathLinux = "/home/user/bin/coder.new" - fakeExePathWindows = `C:\Users\user\bin\coder.exe` - fakeCoderURL = "https://my.cdr.dev/" - fakeNewVersion = "1.23.4-rc.5+678-gabcdef-12345678" - fakeOldVersion = "1.22.4-rc.5+678-gabcdef-12345678" - fakeReleaseURLLinux = "https://github.com/cdr/coder-cli/releases/download/v1.23.4-rc.5/coder-cli-linux-amd64.tar.gz" - fakeReleaseURLWindows = "https://github.com/cdr/coder-cli/releases/download/v1.23.4-rc.5/coder-cli-windows-amd64.zip" - goosWindows = "windows" - goosLinux = "linux" + fakeExePathLinux = "/home/user/bin/coder" + fakeExePathWindows = `C:\Users\user\bin\coder.exe` + fakeCoderURL = "https://my.cdr.dev/" + fakeNewVersion = "1.23.4-rc.5+678-gabcdef-12345678" + fakeOldVersion = "1.22.4-rc.5+678-gabcdef-12345678" + fakeReleaseURLLinux = "https://github.com/cdr/coder-cli/releases/download/v1.23.4-rc.5/coder-cli-linux-amd64.tar.gz" + fakeReleaseURLWindows = "https://github.com/cdr/coder-cli/releases/download/v1.23.4-rc.5/coder-cli-windows-amd64.zip" + goosWindows = "windows" + goosLinux = "linux" ) var ( From 3ba1bed61dca1f5a736df082eb32145fb13db2dd Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Mon, 16 Aug 2021 13:55:53 +0000 Subject: [PATCH 18/25] internal/cmd/update.go: query github releases api for assets --- internal/cmd/update.go | 57 ++++++++++++++++++++++------ internal/cmd/update_test.go | 74 +++++++++++++++++++++++++------------ 2 files changed, 96 insertions(+), 35 deletions(-) diff --git a/internal/cmd/update.go b/internal/cmd/update.go index 5e77c2a6..21d598a7 100644 --- a/internal/cmd/update.go +++ b/internal/cmd/update.go @@ -32,6 +32,12 @@ import ( "github.com/spf13/cobra" ) +const ( + goosWindows = "windows" + goosLinux = "linux" + apiPrivateVersion = "/api/private/version" +) + // updater updates coder-cli. type updater struct { confirmF func(string) (string, error) @@ -143,7 +149,10 @@ func (u *updater) Run(ctx context.Context, force bool, coderURLArg string, versi } } - downloadURL := makeDownloadURL(desiredVersion, u.osF(), runtime.GOARCH) + downloadURL, err := queryGithubAssetURL(u.httpClient, desiredVersion, u.osF()) + if err != nil { + return clog.Fatal("failed to query github assets url", clog.Causef(err.Error())) + } var downloadBuf bytes.Buffer memWriter := bufio.NewWriter(&downloadBuf) @@ -262,15 +271,7 @@ func defaultConfirm(label string) (string, error) { return p.Run() } -func makeDownloadURL(version *semver.Version, ostype, arch string) string { - const template = "https://github.com/cdr/coder-cli/releases/download/v%s/coder-cli-%s-%s.%s" - var ext string - switch ostype { - case "linux": - ext = "tar.gz" - default: - ext = "zip" - } +func queryGithubAssetURL(httpClient getter, version *semver.Version, ostype string) (string, error) { var b bytes.Buffer fmt.Fprintf(&b, "%d", version.Major()) fmt.Fprint(&b, ".") @@ -282,7 +283,41 @@ func makeDownloadURL(version *semver.Version, ostype, arch string) string { fmt.Fprint(&b, version.Prerelease()) } - return fmt.Sprintf(template, b.String(), ostype, arch, ext) + urlString := fmt.Sprintf("https://api.github.com/repos/cdr/coder-cli/releases/tags/v%s", b.String()) + clog.LogInfo("query github releases", fmt.Sprintf("url: %q", urlString)) + + type asset struct { + BrowserDownloadURL string `json:"browser_download_url"` + Name string `json:"name"` + } + type release struct { + Assets []asset `json:"assets"` + } + var r release + + resp, err := httpClient.Get(urlString) + if err != nil { + return "", xerrors.Errorf("query github release url %s: %w", urlString, err) + } + defer resp.Body.Close() + + err = json.NewDecoder(resp.Body).Decode(&r) + if err != nil { + return "", xerrors.Errorf("unmarshal github releases api response: %w", err) + } + + var assetURLStr string + for _, a := range r.Assets { + if strings.HasPrefix(a.Name, "coder-cli-"+ostype) { + assetURLStr = a.BrowserDownloadURL + } + } + + if assetURLStr == "" { + return "", xerrors.Errorf("could not find release for ostype %s", ostype) + } + + return assetURLStr, nil } func extractFromArchive(path string, archive []byte) ([]byte, error) { diff --git a/internal/cmd/update_test.go b/internal/cmd/update_test.go index d261b737..7cd9781a 100644 --- a/internal/cmd/update_test.go +++ b/internal/cmd/update_test.go @@ -22,22 +22,24 @@ import ( ) const ( - fakeExePathLinux = "/home/user/bin/coder" - fakeExePathWindows = `C:\Users\user\bin\coder.exe` - fakeCoderURL = "https://my.cdr.dev/" - fakeNewVersion = "1.23.4-rc.5+678-gabcdef-12345678" - fakeOldVersion = "1.22.4-rc.5+678-gabcdef-12345678" - fakeReleaseURLLinux = "https://github.com/cdr/coder-cli/releases/download/v1.23.4-rc.5/coder-cli-linux-amd64.tar.gz" - fakeReleaseURLWindows = "https://github.com/cdr/coder-cli/releases/download/v1.23.4-rc.5/coder-cli-windows-amd64.zip" - goosWindows = "windows" - goosLinux = "linux" + fakeExePathLinux = "/home/user/bin/coder" + fakeExePathWindows = `C:\Users\user\bin\coder.exe` + fakeCoderURL = "https://my.cdr.dev/" + fakeNewVersion = "1.23.4-rc.5+678-gabcdef-12345678" + fakeOldVersion = "1.22.4-rc.5+678-gabcdef-12345678" + filenameLinux = "coder-cli-linux-amd64.tar.gz" + filenameWindows = "coder-cli-windows.zip" + fakeGithubReleaseURL = "https://api.github.com/repos/cdr/coder-cli/releases/tags/v1.23.4-rc.5" ) var ( - apiPrivateVersionURL = fakeCoderURL + "/api/private/version" - fakeError = xerrors.New("fake error for testing") - fakeNewVersionJSON = fmt.Sprintf(`{"version":%q}`, fakeNewVersion) - fakeOldVersionJSON = fmt.Sprintf(`{"version":%q}`, fakeOldVersion) + apiPrivateVersionURL = fakeCoderURL + apiPrivateVersion + fakeError = xerrors.New("fake error for testing") + fakeNewVersionJSON = fmt.Sprintf(`{"version":%q}`, fakeNewVersion) + fakeOldVersionJSON = fmt.Sprintf(`{"version":%q}`, fakeOldVersion) + fakeAssetURLLinux = "https://github.com/cdr/coder-cli/releases/download/v1.23.4-rc.5/" + filenameLinux + fakeAssetURLWindows = "https://github.com/cdr/coder-cli/releases/download/v1.23.4-rc.5/" + filenameWindows + fakeGithubReleaseJSON = fmt.Sprintf(`{"assets":[{"name":%q,"browser_download_url":%q},{"name":%q,"browser_download_url":%q}]}`, filenameLinux, fakeAssetURLLinux, filenameWindows, fakeAssetURLWindows) ) func Test_updater_run(t *testing.T) { @@ -113,7 +115,8 @@ func Test_updater_run(t *testing.T) { run(t, "update coder - explicit version specified", func(t *testing.T, p *params) { fakeFile(t, p.Fakefs, fakeExePathLinux, 0755, fakeOldVersion) p.HTTPClient.M[apiPrivateVersionURL] = newFakeGetterResponse([]byte(fakeOldVersionJSON), 200, variadicS(), nil) - p.HTTPClient.M[fakeReleaseURLLinux] = newFakeGetterResponse(fakeValidTgzBytes, 200, variadicS(), nil) + p.HTTPClient.M[fakeGithubReleaseURL] = newFakeGetterResponse([]byte(fakeGithubReleaseJSON), 200, variadicS(), nil) + p.HTTPClient.M[fakeAssetURLLinux] = newFakeGetterResponse(fakeValidTgzBytes, 200, variadicS(), nil) p.VersionF = func() string { return fakeOldVersion } p.ConfirmF = fakeConfirmYes p.Execer.M[p.ExecutablePath+".new --version"] = fakeExecerResult{[]byte(fakeNewVersion), nil} @@ -127,7 +130,8 @@ func Test_updater_run(t *testing.T) { run(t, "update coder - old to new", func(t *testing.T, p *params) { fakeFile(t, p.Fakefs, fakeExePathLinux, 0755, fakeOldVersion) p.HTTPClient.M[apiPrivateVersionURL] = newFakeGetterResponse([]byte(fakeNewVersionJSON), 200, variadicS(), nil) - p.HTTPClient.M[fakeReleaseURLLinux] = newFakeGetterResponse(fakeValidTgzBytes, 200, variadicS(), nil) + p.HTTPClient.M[fakeGithubReleaseURL] = newFakeGetterResponse([]byte(fakeGithubReleaseJSON), 200, variadicS(), nil) + p.HTTPClient.M[fakeAssetURLLinux] = newFakeGetterResponse(fakeValidTgzBytes, 200, variadicS(), nil) p.VersionF = func() string { return fakeOldVersion } p.ConfirmF = fakeConfirmYes p.Execer.M[p.ExecutablePath+".new --version"] = fakeExecerResult{[]byte(fakeNewVersion), nil} @@ -142,7 +146,8 @@ func Test_updater_run(t *testing.T) { p.ExecutablePath = "/home/user/bin/coder-cli" fakeFile(t, p.Fakefs, p.ExecutablePath, 0755, fakeOldVersion) p.HTTPClient.M[apiPrivateVersionURL] = newFakeGetterResponse([]byte(fakeNewVersionJSON), 200, variadicS(), nil) - p.HTTPClient.M[fakeReleaseURLLinux] = newFakeGetterResponse(fakeValidTgzBytes, 200, variadicS(), nil) + p.HTTPClient.M[fakeGithubReleaseURL] = newFakeGetterResponse([]byte(fakeGithubReleaseJSON), 200, variadicS(), nil) + p.HTTPClient.M[fakeAssetURLLinux] = newFakeGetterResponse(fakeValidTgzBytes, 200, variadicS(), nil) p.VersionF = func() string { return fakeOldVersion } p.ConfirmF = fakeConfirmYes p.Execer.M[p.ExecutablePath+".new --version"] = fakeExecerResult{[]byte(fakeNewVersion), nil} @@ -158,7 +163,8 @@ func Test_updater_run(t *testing.T) { p.ExecutablePath = fakeExePathWindows fakeFile(t, p.Fakefs, fakeExePathWindows, 0755, fakeOldVersion) p.HTTPClient.M[apiPrivateVersionURL] = newFakeGetterResponse([]byte(fakeNewVersionJSON), 200, variadicS(), nil) - p.HTTPClient.M[fakeReleaseURLWindows] = newFakeGetterResponse(fakeValidZipBytes, 200, variadicS(), nil) + p.HTTPClient.M[fakeGithubReleaseURL] = newFakeGetterResponse([]byte(fakeGithubReleaseJSON), 200, variadicS(), nil) + p.HTTPClient.M[fakeAssetURLWindows] = newFakeGetterResponse(fakeValidZipBytes, 200, variadicS(), nil) p.VersionF = func() string { return fakeOldVersion } p.ConfirmF = fakeConfirmYes p.Execer.M[p.ExecutablePath+".new --version"] = fakeExecerResult{[]byte(fakeNewVersion), nil} @@ -172,7 +178,8 @@ func Test_updater_run(t *testing.T) { run(t, "update coder - old to new forced", func(t *testing.T, p *params) { fakeFile(t, p.Fakefs, fakeExePathLinux, 0755, fakeOldVersion) p.HTTPClient.M[apiPrivateVersionURL] = newFakeGetterResponse([]byte(fakeNewVersionJSON), 200, variadicS(), nil) - p.HTTPClient.M[fakeReleaseURLLinux] = newFakeGetterResponse(fakeValidTgzBytes, 200, variadicS(), nil) + p.HTTPClient.M[fakeGithubReleaseURL] = newFakeGetterResponse([]byte(fakeGithubReleaseJSON), 200, variadicS(), nil) + p.HTTPClient.M[fakeAssetURLLinux] = newFakeGetterResponse(fakeValidTgzBytes, 200, variadicS(), nil) p.VersionF = func() string { return fakeOldVersion } p.Execer.M[p.ExecutablePath+".new --version"] = fakeExecerResult{[]byte(fakeNewVersion), nil} u := fromParams(p) @@ -240,10 +247,24 @@ func Test_updater_run(t *testing.T) { assertFileContent(t, p.Fakefs, fakeExePathLinux, fakeOldVersion) }) + run(t, "update coder - failed to query github releases", func(t *testing.T, p *params) { + fakeFile(t, p.Fakefs, fakeExePathLinux, 0755, fakeOldVersion) + p.HTTPClient.M[apiPrivateVersionURL] = newFakeGetterResponse([]byte(fakeNewVersionJSON), 200, variadicS(), nil) + p.HTTPClient.M[fakeGithubReleaseURL] = newFakeGetterResponse([]byte{}, 0, variadicS(), fakeError) + p.VersionF = func() string { return fakeOldVersion } + p.ConfirmF = fakeConfirmYes + u := fromParams(p) + assertFileContent(t, p.Fakefs, fakeExePathLinux, fakeOldVersion) + err := u.Run(p.Ctx, false, fakeCoderURL, "") + assertCLIError(t, "update coder - failed to query github releases", err, "failed to query github assets", fakeError.Error()) + assertFileContent(t, p.Fakefs, fakeExePathLinux, fakeOldVersion) + }) + run(t, "update coder - failed to fetch URL", func(t *testing.T, p *params) { fakeFile(t, p.Fakefs, fakeExePathLinux, 0755, fakeOldVersion) p.HTTPClient.M[apiPrivateVersionURL] = newFakeGetterResponse([]byte(fakeNewVersionJSON), 200, variadicS(), nil) - p.HTTPClient.M[fakeReleaseURLLinux] = newFakeGetterResponse([]byte{}, 0, variadicS(), fakeError) + p.HTTPClient.M[fakeGithubReleaseURL] = newFakeGetterResponse([]byte(fakeGithubReleaseJSON), 200, variadicS(), nil) + p.HTTPClient.M[fakeAssetURLLinux] = newFakeGetterResponse([]byte{}, 0, variadicS(), fakeError) p.VersionF = func() string { return fakeOldVersion } p.ConfirmF = fakeConfirmYes u := fromParams(p) @@ -256,7 +277,8 @@ func Test_updater_run(t *testing.T) { run(t, "update coder - release URL 404", func(t *testing.T, p *params) { fakeFile(t, p.Fakefs, fakeExePathLinux, 0755, fakeOldVersion) p.HTTPClient.M[apiPrivateVersionURL] = newFakeGetterResponse([]byte(fakeNewVersionJSON), 200, variadicS(), nil) - p.HTTPClient.M[fakeReleaseURLLinux] = newFakeGetterResponse([]byte{}, 404, variadicS(), nil) + p.HTTPClient.M[fakeGithubReleaseURL] = newFakeGetterResponse([]byte(fakeGithubReleaseJSON), 200, variadicS(), nil) + p.HTTPClient.M[fakeAssetURLLinux] = newFakeGetterResponse([]byte{}, 404, variadicS(), nil) p.VersionF = func() string { return fakeOldVersion } p.ConfirmF = fakeConfirmYes u := fromParams(p) @@ -269,7 +291,8 @@ func Test_updater_run(t *testing.T) { run(t, "update coder - invalid tgz archive", func(t *testing.T, p *params) { fakeFile(t, p.Fakefs, fakeExePathLinux, 0755, fakeOldVersion) p.HTTPClient.M[apiPrivateVersionURL] = newFakeGetterResponse([]byte(fakeNewVersionJSON), 200, variadicS(), nil) - p.HTTPClient.M[fakeReleaseURLLinux] = newFakeGetterResponse([]byte{}, 200, variadicS(), nil) + p.HTTPClient.M[fakeGithubReleaseURL] = newFakeGetterResponse([]byte(fakeGithubReleaseJSON), 200, variadicS(), nil) + p.HTTPClient.M[fakeAssetURLLinux] = newFakeGetterResponse([]byte{}, 200, variadicS(), nil) p.VersionF = func() string { return fakeOldVersion } p.ConfirmF = fakeConfirmYes u := fromParams(p) @@ -284,7 +307,8 @@ func Test_updater_run(t *testing.T) { p.ExecutablePath = fakeExePathWindows fakeFile(t, p.Fakefs, fakeExePathWindows, 0755, fakeOldVersion) p.HTTPClient.M[apiPrivateVersionURL] = newFakeGetterResponse([]byte(fakeNewVersionJSON), 200, variadicS(), nil) - p.HTTPClient.M[fakeReleaseURLWindows] = newFakeGetterResponse([]byte{}, 200, variadicS(), nil) + p.HTTPClient.M[fakeGithubReleaseURL] = newFakeGetterResponse([]byte(fakeGithubReleaseJSON), 200, variadicS(), nil) + p.HTTPClient.M[fakeAssetURLWindows] = newFakeGetterResponse([]byte{}, 200, variadicS(), nil) p.VersionF = func() string { return fakeOldVersion } p.ConfirmF = fakeConfirmYes u := fromParams(p) @@ -299,7 +323,8 @@ func Test_updater_run(t *testing.T) { p.Fakefs = afero.NewReadOnlyFs(rwfs) fakeFile(t, rwfs, fakeExePathLinux, 0755, fakeOldVersion) p.HTTPClient.M[apiPrivateVersionURL] = newFakeGetterResponse([]byte(fakeNewVersionJSON), 200, variadicS(), nil) - p.HTTPClient.M[fakeReleaseURLLinux] = newFakeGetterResponse(fakeValidTgzBytes, 200, variadicS(), nil) + p.HTTPClient.M[fakeGithubReleaseURL] = newFakeGetterResponse([]byte(fakeGithubReleaseJSON), 200, variadicS(), nil) + p.HTTPClient.M[fakeAssetURLLinux] = newFakeGetterResponse(fakeValidTgzBytes, 200, variadicS(), nil) p.VersionF = func() string { return fakeOldVersion } p.ConfirmF = fakeConfirmYes u := fromParams(p) @@ -312,7 +337,8 @@ func Test_updater_run(t *testing.T) { run(t, "update coder - cannot exec new binary", func(t *testing.T, p *params) { fakeFile(t, p.Fakefs, fakeExePathLinux, 0755, fakeOldVersion) p.HTTPClient.M[apiPrivateVersionURL] = newFakeGetterResponse([]byte(fakeNewVersionJSON), 200, variadicS(), nil) - p.HTTPClient.M[fakeReleaseURLLinux] = newFakeGetterResponse(fakeValidTgzBytes, 200, variadicS(), nil) + p.HTTPClient.M[fakeGithubReleaseURL] = newFakeGetterResponse([]byte(fakeGithubReleaseJSON), 200, variadicS(), nil) + p.HTTPClient.M[fakeAssetURLLinux] = newFakeGetterResponse(fakeValidTgzBytes, 200, variadicS(), nil) p.VersionF = func() string { return fakeOldVersion } p.ConfirmF = fakeConfirmYes p.Execer.M[p.ExecutablePath+".new --version"] = fakeExecerResult{nil, fakeError} From 6238053fe2ff46114d60c41f653d9e56c40f6e18 Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Tue, 17 Aug 2021 09:49:28 +0000 Subject: [PATCH 19/25] internal/cmd/update.go: handle copy error from archive --- internal/cmd/update.go | 7 +++++-- internal/cmd/update_test.go | 2 +- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/internal/cmd/update.go b/internal/cmd/update.go index 21d598a7..966ec1b5 100644 --- a/internal/cmd/update.go +++ b/internal/cmd/update.go @@ -145,7 +145,7 @@ func (u *updater) Run(ctx context.Context, force bool, coderURLArg string, versi if !force { label := fmt.Sprintf("Do you want to download version %s instead", desiredVersion) if _, err := u.confirmF(label); err != nil { - return clog.Fatal("failed to confirm update", clog.Tipf(`use "--force" to update without confirmation`)) + return clog.Fatal("user cancelled operation", clog.Tipf(`use "--force" to update without confirmation`)) } } @@ -383,7 +383,10 @@ func extractFromTgz(path string, archive []byte) ([]byte, error) { } fi := hdr.FileInfo() if fi.Name() == path && fi.Mode().IsRegular() { - _, _ = io.Copy(bw, tr) + _, err = io.Copy(bw, tr) + if err != nil { + return nil, xerrors.Errorf("failed to read file %q from archive", fi.Name()) + } break } } diff --git a/internal/cmd/update_test.go b/internal/cmd/update_test.go index 7cd9781a..696b8725 100644 --- a/internal/cmd/update_test.go +++ b/internal/cmd/update_test.go @@ -197,7 +197,7 @@ func Test_updater_run(t *testing.T) { u := fromParams(p) assertFileContent(t, p.Fakefs, fakeExePathLinux, fakeOldVersion) err := u.Run(p.Ctx, false, fakeCoderURL, "") - assertCLIError(t, "update coder - user cancelled", err, "failed to confirm update", "") + assertCLIError(t, "update coder - user cancelled", err, "user cancelled operation", "") assertFileContent(t, p.Fakefs, fakeExePathLinux, fakeOldVersion) }) From 24df4f71e43ac091da75887839e679a07c153fb9 Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Tue, 17 Aug 2021 09:49:59 +0000 Subject: [PATCH 20/25] internal/cmd/update.go: handle windows-specific behaviours --- internal/cmd/update.go | 34 +++++++++++++++++++++++++++------- internal/cmd/update_test.go | 16 +++++++++++++++- 2 files changed, 42 insertions(+), 8 deletions(-) diff --git a/internal/cmd/update.go b/internal/cmd/update.go index 966ec1b5..2eda584e 100644 --- a/internal/cmd/update.go +++ b/internal/cmd/update.go @@ -208,22 +208,42 @@ func (u *updater) Run(ctx context.Context, force bool, coderURLArg string, versi _ = updatedBin.Close() + if err := u.doUpdate(ctx, updatedCoderBinaryPath); err != nil { + return clog.Fatal("failed to update coder binary", clog.Causef(err.Error())) + } + + clog.LogSuccess("Updated coder CLI to version " + desiredVersion.String()) + return nil +} + +func (u *updater) doUpdate(ctx context.Context, updatedCoderBinaryPath string) error { + var err error + // TODO(cian): on Windows, we must do two things differnetly: + // 1) Calling the updated binary fails due to the xterminal.MakeOutputRaw call in main; skipping this check on Windows. + // 2) We must rename the currently running binary before renaming the new binary + if u.osF() == goosWindows { + err = u.fs.Rename(u.executablePath, updatedCoderBinaryPath+".old") + if err != nil { + return xerrors.Errorf("windows: rename current coder binary: %w", err) + } + err = u.fs.Rename(updatedCoderBinaryPath, u.executablePath) + if err != nil { + return xerrors.Errorf("windows: rename updated coder binary: %w", err) + } + return nil + } + // validate that we can execute the new binary before overwriting updatedVersionOutput, err := u.execF(ctx, updatedCoderBinaryPath, "--version") if err != nil { - return clog.Fatal("failed to check version of updated coder binary", - clog.BlankLine, - fmt.Sprintf("output: %q", string(updatedVersionOutput)), - clog.Causef(err.Error())) + return xerrors.Errorf("check version of updated coder binary: %w", err) } - clog.LogInfo(fmt.Sprintf("updated binary reports %s", bytes.TrimSpace(updatedVersionOutput))) if err = u.fs.Rename(updatedCoderBinaryPath, u.executablePath); err != nil { - return clog.Fatal("failed to update coder binary in-place", clog.Causef(err.Error())) + return xerrors.Errorf("update coder binary in-place: %w", err) } - clog.LogSuccess("Updated coder CLI to version " + desiredVersion.String()) return nil } diff --git a/internal/cmd/update_test.go b/internal/cmd/update_test.go index 696b8725..2a41e9b0 100644 --- a/internal/cmd/update_test.go +++ b/internal/cmd/update_test.go @@ -345,11 +345,25 @@ func Test_updater_run(t *testing.T) { u := fromParams(p) assertFileContent(t, p.Fakefs, fakeExePathLinux, fakeOldVersion) err := u.Run(p.Ctx, false, fakeCoderURL, "") - assertCLIError(t, "update coder - cannot exec new binary", err, "failed to check version of updated coder binary", "") + assertCLIError(t, "update coder - cannot exec new binary", err, "failed to update coder binary", fakeError.Error()) assertFileContent(t, p.Fakefs, fakeExePathLinux, fakeOldVersion) }) if runtime.GOOS == goosWindows { + run(t, "update coder - windows", func(t *testing.T, p *params) { + fakeFile(t, p.Fakefs, fakeExePathWindows, 0755, fakeOldVersion) + p.HTTPClient.M[apiPrivateVersionURL] = newFakeGetterResponse([]byte(fakeNewVersionJSON), 200, variadicS(), nil) + p.HTTPClient.M[fakeGithubReleaseURL] = newFakeGetterResponse([]byte(fakeGithubReleaseJSON), 200, variadicS(), nil) + p.HTTPClient.M[fakeAssetURLLinux] = newFakeGetterResponse(fakeValidTgzBytes, 200, variadicS(), nil) + p.VersionF = func() string { return fakeOldVersion } + p.ConfirmF = fakeConfirmYes + p.OsF = func() string { return goosWindows } + u := fromParams(p) + assertFileContent(t, p.Fakefs, fakeExePathWindows, fakeOldVersion) + err := u.Run(p.Ctx, false, fakeCoderURL, "") + assertCLIError(t, "update coder - cannot exec new binary", err, "failed to update coder binary", fakeError.Error()) + assertFileContent(t, p.Fakefs, fakeExePathWindows, fakeOldVersion) + }) run(t, "update coder - path blocklist - windows", func(t *testing.T, p *params) { p.ExecutablePath = `C:\Windows\system32\coder.exe` u := fromParams(p) From 21dd8361c960c71a59c209658e6630347fc80fb3 Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Tue, 17 Aug 2021 09:57:40 +0000 Subject: [PATCH 21/25] fixup! internal/cmd/update.go: handle windows-specific behaviours --- internal/cmd/update_test.go | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/internal/cmd/update_test.go b/internal/cmd/update_test.go index 2a41e9b0..f7ade169 100644 --- a/internal/cmd/update_test.go +++ b/internal/cmd/update_test.go @@ -350,20 +350,6 @@ func Test_updater_run(t *testing.T) { }) if runtime.GOOS == goosWindows { - run(t, "update coder - windows", func(t *testing.T, p *params) { - fakeFile(t, p.Fakefs, fakeExePathWindows, 0755, fakeOldVersion) - p.HTTPClient.M[apiPrivateVersionURL] = newFakeGetterResponse([]byte(fakeNewVersionJSON), 200, variadicS(), nil) - p.HTTPClient.M[fakeGithubReleaseURL] = newFakeGetterResponse([]byte(fakeGithubReleaseJSON), 200, variadicS(), nil) - p.HTTPClient.M[fakeAssetURLLinux] = newFakeGetterResponse(fakeValidTgzBytes, 200, variadicS(), nil) - p.VersionF = func() string { return fakeOldVersion } - p.ConfirmF = fakeConfirmYes - p.OsF = func() string { return goosWindows } - u := fromParams(p) - assertFileContent(t, p.Fakefs, fakeExePathWindows, fakeOldVersion) - err := u.Run(p.Ctx, false, fakeCoderURL, "") - assertCLIError(t, "update coder - cannot exec new binary", err, "failed to update coder binary", fakeError.Error()) - assertFileContent(t, p.Fakefs, fakeExePathWindows, fakeOldVersion) - }) run(t, "update coder - path blocklist - windows", func(t *testing.T, p *params) { p.ExecutablePath = `C:\Windows\system32\coder.exe` u := fromParams(p) From e064c47bbecde4ed270ce8420ebcbc968216d2b4 Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Tue, 17 Aug 2021 10:31:59 +0000 Subject: [PATCH 22/25] fixup! internal/cmd/update.go: handle windows-specific behaviours --- internal/cmd/update.go | 2 +- internal/cmd/update_test.go | 58 +++++++++++++++++++------------------ 2 files changed, 31 insertions(+), 29 deletions(-) diff --git a/internal/cmd/update.go b/internal/cmd/update.go index 2eda584e..d9a4edf6 100644 --- a/internal/cmd/update.go +++ b/internal/cmd/update.go @@ -218,7 +218,7 @@ func (u *updater) Run(ctx context.Context, force bool, coderURLArg string, versi func (u *updater) doUpdate(ctx context.Context, updatedCoderBinaryPath string) error { var err error - // TODO(cian): on Windows, we must do two things differnetly: + // TODO(cian): on Windows, we must do two things differently: // 1) Calling the updated binary fails due to the xterminal.MakeOutputRaw call in main; skipping this check on Windows. // 2) We must rename the currently running binary before renaming the new binary if u.osF() == goosWindows { diff --git a/internal/cmd/update_test.go b/internal/cmd/update_test.go index f7ade169..e246f666 100644 --- a/internal/cmd/update_test.go +++ b/internal/cmd/update_test.go @@ -71,34 +71,36 @@ func Test_updater_run(t *testing.T) { } run := func(t *testing.T, name string, fn func(t *testing.T, p *params)) { - t.Logf("running %s", name) - ctx := context.Background() - fakefs := afero.NewMemMapFs() - execer := newFakeExecer(t) - execer.M["brew --prefix"] = fakeExecerResult{[]byte{}, os.ErrNotExist} - params := ¶ms{ - // This must be overridden inside run() - ConfirmF: func(string) (string, error) { - t.Errorf("unhandled ConfirmF") - t.FailNow() - return "", nil - }, - Execer: execer, - Ctx: ctx, - ExecutablePath: fakeExePathLinux, - Fakefs: fakefs, - HTTPClient: newFakeGetter(t), - // Default to GOOS=linux - OsF: func() string { return goosLinux }, - // This must be overridden inside run() - VersionF: func() string { - t.Errorf("unhandled VersionF") - t.FailNow() - return "" - }, - } - - fn(t, params) + t.Run(name, func(t *testing.T) { + t.Logf("running %s", name) + ctx := context.Background() + fakefs := afero.NewMemMapFs() + execer := newFakeExecer(t) + execer.M["brew --prefix"] = fakeExecerResult{[]byte{}, os.ErrNotExist} + params := ¶ms{ + // This must be overridden inside run() + ConfirmF: func(string) (string, error) { + t.Errorf("unhandled ConfirmF") + t.FailNow() + return "", nil + }, + Execer: execer, + Ctx: ctx, + ExecutablePath: fakeExePathLinux, + Fakefs: fakefs, + HTTPClient: newFakeGetter(t), + // Default to GOOS=linux + OsF: func() string { return goosLinux }, + // This must be overridden inside run() + VersionF: func() string { + t.Errorf("unhandled VersionF") + t.FailNow() + return "" + }, + } + + fn(t, params) + }) } run(t, "update coder - noop", func(t *testing.T, p *params) { From eefa2f3e299bf5432bce386a04425275a92b5980 Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Tue, 17 Aug 2021 11:43:20 +0100 Subject: [PATCH 23/25] gofmt --- internal/cmd/update.go | 3 ++- internal/cmd/update_test.go | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/internal/cmd/update.go b/internal/cmd/update.go index d9a4edf6..015b807e 100644 --- a/internal/cmd/update.go +++ b/internal/cmd/update.go @@ -21,10 +21,11 @@ import ( "strings" "time" + "golang.org/x/xerrors" + "cdr.dev/coder-cli/internal/config" "cdr.dev/coder-cli/internal/version" "cdr.dev/coder-cli/pkg/clog" - "golang.org/x/xerrors" "github.com/Masterminds/semver/v3" "github.com/manifoldco/promptui" diff --git a/internal/cmd/update_test.go b/internal/cmd/update_test.go index e246f666..c751e2c4 100644 --- a/internal/cmd/update_test.go +++ b/internal/cmd/update_test.go @@ -14,11 +14,12 @@ import ( "strings" "testing" - "cdr.dev/coder-cli/pkg/clog" "cdr.dev/slog/sloggers/slogtest/assert" "github.com/manifoldco/promptui" "github.com/spf13/afero" "golang.org/x/xerrors" + + "cdr.dev/coder-cli/pkg/clog" ) const ( From 0e63d2f0593e0a8eb26f8d6468b4ddbceed9418b Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Tue, 17 Aug 2021 10:56:22 +0000 Subject: [PATCH 24/25] internal/cmd/users_test.go: assert CICD user instead of admin --- internal/cmd/update.go | 4 +--- internal/cmd/users_test.go | 8 ++++---- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/internal/cmd/update.go b/internal/cmd/update.go index 015b807e..c4ba2536 100644 --- a/internal/cmd/update.go +++ b/internal/cmd/update.go @@ -180,11 +180,9 @@ func (u *updater) Run(ctx context.Context, force bool, coderURLArg string, versi // TODO: validate the checksum of the downloaded file. GitHub does not currently provide this information // and we do not generate them yet. - var updatedBinaryName string + updatedBinaryName := "coder" if u.osF() == "windows" { updatedBinaryName = "coder.exe" - } else { - updatedBinaryName = "coder" } updatedBinary, err := extractFromArchive(updatedBinaryName, downloadBuf.Bytes()) if err != nil { diff --git a/internal/cmd/users_test.go b/internal/cmd/users_test.go index a82f4607..81fe10f7 100644 --- a/internal/cmd/users_test.go +++ b/internal/cmd/users_test.go @@ -16,17 +16,17 @@ func Test_users(t *testing.T) { res := execute(t, nil, "users", "ls", "--output=json") res.success(t) res.stdoutUnmarshals(t, &users) - assertAdmin(t, users) + assertCICD(t, users) res = execute(t, nil, "users", "ls", "--output=human") res.success(t) } -func assertAdmin(t *testing.T, users []coder.User) { +func assertCICD(t *testing.T, users []coder.User) { for _, u := range users { - if u.Username == "admin" { + if u.Username == "cicd" { return } } - slogtest.Fatal(t, "did not find admin user", slog.F("users", users)) + slogtest.Fatal(t, "did not find cicd user", slog.F("users", users)) } From 60944f09659c126e78d3cfd7a2513ecd97cf3e06 Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Tue, 17 Aug 2021 10:57:21 +0000 Subject: [PATCH 25/25] Revert "internal/cmd/users_test.go: assert CICD user instead of admin" This reverts commit 0e63d2f0593e0a8eb26f8d6468b4ddbceed9418b. --- internal/cmd/update.go | 4 +++- internal/cmd/users_test.go | 8 ++++---- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/internal/cmd/update.go b/internal/cmd/update.go index c4ba2536..015b807e 100644 --- a/internal/cmd/update.go +++ b/internal/cmd/update.go @@ -180,9 +180,11 @@ func (u *updater) Run(ctx context.Context, force bool, coderURLArg string, versi // TODO: validate the checksum of the downloaded file. GitHub does not currently provide this information // and we do not generate them yet. - updatedBinaryName := "coder" + var updatedBinaryName string if u.osF() == "windows" { updatedBinaryName = "coder.exe" + } else { + updatedBinaryName = "coder" } updatedBinary, err := extractFromArchive(updatedBinaryName, downloadBuf.Bytes()) if err != nil { diff --git a/internal/cmd/users_test.go b/internal/cmd/users_test.go index 81fe10f7..a82f4607 100644 --- a/internal/cmd/users_test.go +++ b/internal/cmd/users_test.go @@ -16,17 +16,17 @@ func Test_users(t *testing.T) { res := execute(t, nil, "users", "ls", "--output=json") res.success(t) res.stdoutUnmarshals(t, &users) - assertCICD(t, users) + assertAdmin(t, users) res = execute(t, nil, "users", "ls", "--output=human") res.success(t) } -func assertCICD(t *testing.T, users []coder.User) { +func assertAdmin(t *testing.T, users []coder.User) { for _, u := range users { - if u.Username == "cicd" { + if u.Username == "admin" { return } } - slogtest.Fatal(t, "did not find cicd user", slog.F("users", users)) + slogtest.Fatal(t, "did not find admin user", slog.F("users", users)) }