[v17] Fix incorrect incompatibility warning in tsh

lib/client/api.go

@@ -4206,46 +4206,16 @@ func (tc *TeleportClient) Ping(ctx context.Context) (*webclient.PingResponse, er
 
 	// Verify server->client and client->server compatibility.
 	if tc.CheckVersions {
-		if !utils.MeetsMinVersion(teleport.Version, pr.MinClientVersion) {
-			fmt.Fprintf(tc.Stderr, `
-WARNING
-Detected potentially incompatible client and server versions.
-Minimum client version supported by the server is %v but you are using %v.
-Please upgrade tsh to %v or newer or use the --skip-version-check flag to bypass this check.
-Future versions of tsh will fail when incompatible versions are detected.
-
-`,
-				pr.MinClientVersion, teleport.Version, pr.MinClientVersion)
-		}
-
-		if !utils.MeetsMaxVersion(teleport.Version, pr.ServerVersion) {
-			serverVersionWithWildcards, err := utils.MajorSemverWithWildcards(pr.ServerVersion)
-			if err != nil {
-				return nil, trace.Wrap(err)
-			}
-			fmt.Fprintf(tc.Stderr, `
-WARNING
-Detected potentially incompatible client and server versions.
-Maximum client version supported by the server is %v but you are using %v.
-Please downgrade tsh to %v or use the --skip-version-check flag to bypass this check.
-Future versions of tsh will fail when incompatible versions are detected.
-
-`,
-				serverVersionWithWildcards, teleport.Version, serverVersionWithWildcards)
+		warning, err := getClientIncompatibilityWarning(versions{
+			MinClient: pr.MinClientVersion,
+			Client:    teleport.Version,
+			Server:    pr.ServerVersion,
+		})
+		if err != nil {
+			return nil, trace.Wrap(err)
 		}
-
-		// Recent `tsh mfa` changes require at least Teleport v15.
-		const minServerVersion = "15.0.0-aa" // "-aa" matches all development versions
-		if !utils.MeetsMinVersion(pr.ServerVersion, minServerVersion) {
-			fmt.Fprintf(tc.Stderr, `
-WARNING
-Detected incompatible client and server versions.
-Minimum server version supported by tsh is %v but your server is using %v.
-Please use a tsh version that matches your server.
-You may use the --skip-version-check flag to bypass this check.
-
-`,
-				minServerVersion, pr.ServerVersion)
+		if warning != "" {
+			fmt.Fprint(tc.Stderr, warning)
 		}
 	}
 
@@ -4268,6 +4238,53 @@ You may use the --skip-version-check flag to bypass this check.
 	return pr, nil
 }
 
+type versions struct {
+	MinClient string
+	Client    string
+	Server    string
+}
+
+func getClientIncompatibilityWarning(versions versions) (string, error) {
+	if !utils.MeetsMinVersion(versions.Client, versions.MinClient) {
+		return fmt.Sprintf(`
+WARNING
+Detected potentially incompatible client and server versions.
+Minimum client version supported by the server is %v but you are using %v.
+Please upgrade tsh to %v or newer or use the --skip-version-check flag to bypass this check.
+Future versions of tsh will fail when incompatible versions are detected.
+
+`,
+			versions.MinClient, versions.Client, versions.MinClient), nil
+	}
+
+	clientMajorVersion, err := utils.MajorSemver(versions.Client)
+	if err != nil {
+		return "", trace.Wrap(err)
+	}
+	serverMajorVersion, err := utils.MajorSemver(versions.Server)
+	if err != nil {
+		return "", trace.Wrap(err)
+	}
+
+	if !utils.MeetsMaxVersion(clientMajorVersion, serverMajorVersion) {
+		serverVersionWithWildcards, err := utils.MajorSemverWithWildcards(serverMajorVersion)
+		if err != nil {
+			return "", trace.Wrap(err)
+		}
+		return fmt.Sprintf(`
+WARNING
+Detected potentially incompatible client and server versions.
+Maximum client version supported by the server is %v but you are using %v.
+Please downgrade tsh to %v or use the --skip-version-check flag to bypass this check.
+Future versions of tsh will fail when incompatible versions are detected.
+
+`,
+			serverVersionWithWildcards, versions.Client, serverVersionWithWildcards), nil
+	}
+
+	return "", nil
+}
+
 // GetCurrentSignatureAlgorithmSuite returns the current signature algorithm
 // suite configured in the local cluster. It matches [cryptosuites.GetSuiteFunc].
 func (tc *TeleportClient) GetCurrentSignatureAlgorithmSuite(ctx context.Context) (types.SignatureAlgorithmSuite, error) {

lib/client/api_test.go

@@ -29,6 +29,7 @@ import (
 	"testing"
 	"time"
 
+	"github.com/coreos/go-semver/semver"
 	"github.com/google/go-cmp/cmp"
 	"github.com/gravitational/trace"
 	"github.com/stretchr/testify/assert"
@@ -1390,3 +1391,72 @@ func TestNonRetryableError(t *testing.T) {
 	assert.True(t, trace.IsAccessDenied(err))
 	assert.Equal(t, orgError, err.Unwrap())
 }
+
+func TestWarningAboutIncompatibleClientVersion(t *testing.T) {
+	tests := []struct {
+		name            string
+		clientVersion   string
+		serverVersion   string
+		expectedWarning string
+	}{
+		{
+			name:          "client on a higher major version than server triggers a warning",
+			clientVersion: "17.0.0",
+			serverVersion: "16.0.0",
+			expectedWarning: `
+WARNING
+Detected potentially incompatible client and server versions.
+Maximum client version supported by the server is 16.x.x but you are using 17.0.0.
+Please downgrade tsh to 16.x.x or use the --skip-version-check flag to bypass this check.
+Future versions of tsh will fail when incompatible versions are detected.
+
+`,
+		},
+		{
+			name:          "client on a too low major version compared to server triggers a warning",
+			clientVersion: "16.4.0",
+			serverVersion: "18.0.0",
+			expectedWarning: `
+WARNING
+Detected potentially incompatible client and server versions.
+Minimum client version supported by the server is 17.0.0 but you are using 16.4.0.
+Please upgrade tsh to 17.0.0 or newer or use the --skip-version-check flag to bypass this check.
+Future versions of tsh will fail when incompatible versions are detected.
+
+`,
+		},
+		{
+			name:            "client on a higher minor version than server does not trigger a warning",
+			clientVersion:   "17.1.0",
+			serverVersion:   "17.0.0",
+			expectedWarning: "",
+		},
+		{
+			name:            "client on a lower major version than server does not trigger a warning",
+			clientVersion:   "17.0.0",
+			serverVersion:   "18.0.0",
+			expectedWarning: "",
+		},
+		{
+			name:            "client and server on the same version do not trigger a warning",
+			clientVersion:   "18.0.0",
+			serverVersion:   "18.0.0",
+			expectedWarning: "",
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			minClientVersion, err := semver.NewVersion(test.serverVersion)
+			require.NoError(t, err)
+			minClientVersion.Major = minClientVersion.Major - 1
+			warning, err := getClientIncompatibilityWarning(versions{
+				MinClient: minClientVersion.String(),
+				Client:    test.clientVersion,
+				Server:    test.serverVersion,
+			})
+			require.NoError(t, err)
+			require.Equal(t, test.expectedWarning, warning)
+		})
+	}
+}