fix: tolerate leading v in semantic versions from coder version API (#449)

* fix: tolerate leading v in semantic versions from coder version API
* fix: quote reported version strings and only print Major.Minor.Patch

Author: johnstcn

internal/cmd/update.go

@@ -129,14 +129,14 @@ 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 {
 		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 {
@@ -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
 }
@@ -452,7 +456,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)
 	}

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)