Skip to content

Commit 1749686

Browse files
sreyasreya
committed
pr comments
1 parent 790cfc3 commit 1749686

File tree

3 files changed

+35
-23
lines changed

3 files changed

+35
-23
lines changed

‎cli/login.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import (
1818

1919
"github.com/coder/pretty"
2020

21+
"github.com/coder/coder/v2/buildinfo"
2122
"github.com/coder/coder/v2/cli/clibase"
2223
"github.com/coder/coder/v2/cli/cliui"
2324
"github.com/coder/coder/v2/coderd/userpassword"
@@ -175,7 +176,7 @@ func (r *RootCmd) login() *clibase.Cmd {
175176
// Try to check the version of the server prior to logging in.
176177
// It may be useful to warn the user if they are trying to login
177178
// on a very old client.
178-
err = r.checkVersions(inv, client, false)
179+
err = r.checkVersions(inv, client, buildinfo.Version())
179180
if err != nil {
180181
// Checking versions isn't a fatal error so we print a warning
181182
// and proceed.

‎cli/root.go

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -602,7 +602,7 @@ func (r *RootCmd) PrintWarnings(client *codersdk.Client) clibase.MiddlewareFunc
602602
warningErr = make(chan error)
603603
)
604604
go func() {
605-
versionErr <- r.checkVersions(inv, client, false)
605+
versionErr <- r.checkVersions(inv, client, buildinfo.Version())
606606
close(versionErr)
607607
}()
608608

@@ -817,16 +817,15 @@ func formatExamples(examples ...example) string {
817817
// is detected. forceCheck is a test flag and should always be false in production.
818818
//
819819
//nolint:revive
820-
func (r *RootCmd) checkVersions(i *clibase.Invocation, client *codersdk.Client, forceCheck bool) error {
820+
func (r *RootCmd) checkVersions(i *clibase.Invocation, client *codersdk.Client, clientVersion string) error {
821821
if r.noVersionCheck {
822822
return nil
823823
}
824824

825825
ctx, cancel := context.WithTimeout(i.Context(), 10*time.Second)
826826
defer cancel()
827827

828-
clientVersion := buildinfo.Version()
829-
info, err := client.BuildInfo(ctx)
828+
serverInfo, err := client.BuildInfo(ctx)
830829
// Avoid printing errors that are connection-related.
831830
if isConnectionError(err) {
832831
return nil
@@ -835,12 +834,11 @@ func (r *RootCmd) checkVersions(i *clibase.Invocation, client *codersdk.Client,
835834
return xerrors.Errorf("build info: %w", err)
836835
}
837836

838-
fmtWarningText := "version mismatch: client %s, server %s\n%s"
837+
if !buildinfo.VersionsMatch(clientVersion, serverInfo.Version) {
838+
fmtWarningText := "version mismatch: client %s, server %s\n%s"
839+
fmtWarn := pretty.Sprint(cliui.DefaultStyles.Warn, fmtWarningText)
840+
warning := fmt.Sprintf(fmtWarn, clientVersion, strings.TrimPrefix(serverInfo.CanonicalVersion(), "v"), serverInfo.UpgradeMessage)
839841

840-
fmtWarn := pretty.Sprint(cliui.DefaultStyles.Warn, fmtWarningText)
841-
warning := fmt.Sprintf(fmtWarn, info.Version, strings.TrimPrefix(info.CanonicalVersion(), "v"), info.UpgradeMessage)
842-
843-
if !buildinfo.VersionsMatch(clientVersion, info.Version) || forceCheck {
844842
_, _ = fmt.Fprint(i.Stderr, warning)
845843
_, _ = fmt.Fprintln(i.Stderr)
846844
}

‎cli/root_internal_test.go

Lines changed: 26 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -3,16 +3,21 @@ package cli
33
import (
44
"bytes"
55
"fmt"
6+
"net/http"
7+
"net/http/httptest"
8+
"net/url"
69
"os"
710
"runtime"
811
"testing"
912

1013
"github.com/stretchr/testify/require"
1114
"go.uber.org/goleak"
1215

13-
"github.com/coder/coder/v2/cli/clibase"
16+
"github.com/coder/coder/v2/buildinfo"
1417
"github.com/coder/coder/v2/cli/cliui"
15-
"github.com/coder/coder/v2/coderd/coderdtest"
18+
"github.com/coder/coder/v2/coderd"
19+
"github.com/coder/coder/v2/coderd/httpapi"
20+
"github.com/coder/coder/v2/codersdk"
1621
"github.com/coder/pretty"
1722
)
1823

@@ -100,18 +105,25 @@ func Test_checkVersions(t *testing.T) {
100105

101106
var (
102107
expectedUpgradeMessage = "My custom upgrade message"
103-
dv = coderdtest.DeploymentValues(t)
104108
)
105-
dv.CLIUpgradeMessage = clibase.String(expectedUpgradeMessage)
106109

107-
ownerClient := coderdtest.New(t, &coderdtest.Options{
108-
DeploymentValues: dv,
109-
})
110-
owner := coderdtest.CreateFirstUser(t, ownerClient)
110+
srv := httptest.NewServer(http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) {
111+
httpapi.Write(r.Context(), rw, http.StatusOK, codersdk.BuildInfoResponse{
112+
ExternalURL: buildinfo.ExternalURL(),
113+
// Provide a version that will not match
114+
Version: "v1.0.0",
115+
AgentAPIVersion: coderd.AgentAPIVersionREST,
116+
// does not matter what the url is
117+
DashboardURL: "https://example.com",
118+
WorkspaceProxy: false,
119+
UpgradeMessage: expectedUpgradeMessage,
120+
})
121+
}))
122+
defer srv.Close()
123+
surl, err := url.Parse(srv.URL)
124+
require.NoError(t, err)
111125

112-
// Create an unprivileged user to ensure the message can be printed
113-
// to any Coder user.
114-
memberClient, _ := coderdtest.CreateAnotherUser(t, ownerClient, owner.OrganizationID)
126+
client := codersdk.New(surl)
115127

116128
r := &RootCmd{}
117129

@@ -122,10 +134,11 @@ func Test_checkVersions(t *testing.T) {
122134
inv := cmd.Invoke()
123135
inv.Stderr = &buf
124136

125-
err = r.checkVersions(inv, memberClient, true)
137+
err = r.checkVersions(inv, client, "v2.0.0")
126138
require.NoError(t, err)
127139

128-
expectedOutput := fmt.Sprintln(pretty.Sprint(cliui.DefaultStyles.Warn, expectedUpgradeMessage))
140+
fmtOutput := fmt.Sprintf("version mismatch: client v2.0.0, server 1.0.0\n%s", expectedUpgradeMessage)
141+
expectedOutput := fmt.Sprintln(pretty.Sprint(cliui.DefaultStyles.Warn, fmtOutput))
129142
require.Equal(t, expectedOutput, buf.String())
130143
})
131144
}

0 commit comments

Comments
 (0)