From 586003b430e89e331695943798e13b3453e49e41 Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Thu, 30 Sep 2021 10:03:42 +0000 Subject: [PATCH 1/2] fix: tolerate leading v in semantic versions from coder version API --- internal/cmd/update.go | 4 ++-- internal/cmd/update_test.go | 47 +++++++++++++++++++++++++++++++++++++ 2 files changed, 49 insertions(+), 2 deletions(-) diff --git a/internal/cmd/update.go b/internal/cmd/update.go index d321ab7e..bcdeabbf 100644 --- a/internal/cmd/update.go +++ b/internal/cmd/update.go @@ -136,7 +136,7 @@ func (u *updater) Run(ctx context.Context, force bool, coderURLArg string, versi return clog.Fatal("failed to determine desired version of coder", clog.Causef(err.Error())) } - currentVersion, err := semver.StrictNewVersion(u.versionF()) + currentVersion, err := semver.NewVersion(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 { @@ -452,7 +452,7 @@ func getAPIVersionUnauthed(client getter, baseURL url.URL) (*semver.Version, err return nil, xerrors.Errorf("parse version response: %w", err) } - version, err := semver.StrictNewVersion(ver.Version) + version, err := semver.NewVersion(ver.Version) if err != nil { return nil, xerrors.Errorf("parsing coder version: %w", err) } diff --git a/internal/cmd/update_test.go b/internal/cmd/update_test.go index fc85b709..6f0a63c1 100644 --- a/internal/cmd/update_test.go +++ b/internal/cmd/update_test.go @@ -106,6 +106,19 @@ func Test_updater_run(t *testing.T) { } run(t, "update coder - noop", func(t *testing.T, p *params) { + fakeNewVersion := "v" + fakeNewVersion + fakeFile(t, p.Fakefs, fakeExePathLinux, 0755, fakeNewVersion) + 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) + err := u.Run(p.Ctx, false, fakeCoderURL, "") + assert.Success(t, "update coder - noop", err) + assertFileContent(t, p.Fakefs, fakeExePathLinux, fakeNewVersion) + }) + + run(t, "update coder - should be noop but versions have leading v", func(t *testing.T, p *params) { + fakeNewVersion := "v" + fakeNewVersion fakeFile(t, p.Fakefs, fakeExePathLinux, 0755, fakeNewVersion) p.HTTPClient.M[apiPrivateVersionURL] = newFakeGetterResponse([]byte(fakeNewVersionJSON), 200, variadicS(), nil) p.VersionF = func() string { return fakeNewVersion } @@ -131,6 +144,23 @@ func Test_updater_run(t *testing.T) { assertFileContent(t, p.Fakefs, fakeExePathLinux, fakeNewVersion) }) + run(t, "update coder - explicit version - leading v", func(t *testing.T, p *params) { + fakeNewVersion := "v" + fakeNewVersion + fakeOldVersion := "v" + fakeOldVersion + fakeFile(t, p.Fakefs, fakeExePathLinux, 0755, fakeOldVersion) + p.HTTPClient.M[apiPrivateVersionURL] = newFakeGetterResponse([]byte(fakeOldVersionJSON), 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} + 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, strings.TrimPrefix(fakeNewVersion, "v")) // TODO: stop hard-coding this + }) + 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) @@ -146,6 +176,23 @@ func Test_updater_run(t *testing.T) { assertFileContent(t, p.Fakefs, fakeExePathLinux, fakeNewVersion) }) + run(t, "update coder - old to new - leading v", func(t *testing.T, p *params) { + fakeNewVersion := "v" + fakeNewVersion + fakeOldVersion := "v" + fakeOldVersion + fakeFile(t, p.Fakefs, fakeExePathLinux, 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.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, "") + assert.Success(t, "update coder - old to new", err) + assertFileContent(t, p.Fakefs, fakeExePathLinux, strings.TrimPrefix(fakeNewVersion, "v")) // TODO: stop hard-coding this + }) + 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) From b60088f1fcda5861c8bb4da6ac20adb926f10bcf Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Thu, 30 Sep 2021 10:11:27 +0000 Subject: [PATCH 2/2] fix: quote reported version strings and only print Major.Minor.Patch --- internal/cmd/update.go | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/internal/cmd/update.go b/internal/cmd/update.go index bcdeabbf..c9b94d80 100644 --- a/internal/cmd/update.go +++ b/internal/cmd/update.go @@ -129,7 +129,7 @@ func (u *updater) Run(ctx context.Context, force bool, coderURLArg string, versi return clog.Fatal("preflight: missing write permission on current binary") } - clog.LogInfo(fmt.Sprintf("Current version of coder-cli is %s", version.Version)) + clog.LogInfo(fmt.Sprintf("Current version of coder-cli is %q", version.Version)) desiredVersion, err := getDesiredVersion(u.httpClient, coderURLArg, versionArg) if err != nil { @@ -145,7 +145,11 @@ 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) + label := fmt.Sprintf("Do you want to download version %d.%d.%d instead", + desiredVersion.Major(), + desiredVersion.Minor(), + desiredVersion.Patch(), + ) if _, err := u.confirmF(label); err != nil { return clog.Fatal("user cancelled operation", clog.Tipf(`use "--force" to update without confirmation`)) } @@ -240,7 +244,7 @@ func (u *updater) doUpdate(ctx context.Context, updatedCoderBinaryPath string) e if err != nil { return xerrors.Errorf("check version of updated coder binary: %w", err) } - clog.LogInfo(fmt.Sprintf("updated binary reports %s", bytes.TrimSpace(updatedVersionOutput))) + clog.LogInfo(fmt.Sprintf("updated binary reports %q", bytes.TrimSpace(updatedVersionOutput))) if err = u.fs.Rename(updatedCoderBinaryPath, u.executablePath); err != nil { return xerrors.Errorf("update coder binary in-place: %w", err) @@ -283,7 +287,7 @@ func getDesiredVersion(httpClient getter, coderURLArg string, versionArg string) 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())) + clog.LogInfo(fmt.Sprintf("Coder instance at %q reports version %q", coderURL.String(), desiredVersion.String())) return desiredVersion, nil }