API server version check

Makefile

@@ -117,7 +117,7 @@ ifeq ("$(OS)","windows")
 BINARIES=$(BUILDDIR)/tsh
 endif
 
-VERSRC = version.go gitref.go
+VERSRC = version.go gitref.go api/version.go
 
 KUBECONFIG ?=
 TEST_KUBE ?=

api/api.go

@@ -0,0 +1,17 @@
+package api
+
+import (
+	"fmt"
+
+	"github.com/coreos/go-semver/semver"
+)
+
+// MinServerVersion is the minimum server version required by the client.
+var MinServerVersion string
+
+func init() {
+	// Per https://github.com/gravitational/teleport/blob/master/rfd/0012-teleport-versioning.md,
+	// only one major version backwards is supported for servers.
+	ver := semver.New(Version)
+	MinServerVersion = fmt.Sprintf("%d.0.0", ver.Major-1)
+}

api/client/client.go

@@ -26,6 +26,7 @@ import (
 	"sync/atomic"
 	"time"
 
+	"github.com/gravitational/teleport/api"
 	"github.com/gravitational/teleport/api/client/proto"
 	"github.com/gravitational/teleport/api/client/webclient"
 	"github.com/gravitational/teleport/api/constants"
@@ -168,6 +169,11 @@ func connect(ctx context.Context, cfg Config) (*Client, error) {
 				return
 			}
 
+			if err := clt.checkServerCompatibility(ctx); err != nil {
+				sendError(trace.Wrap(err))
+				return
+			}
+
 			// If cltChan is empty, send clt.
 			select {
 			case cltChan <- clt:
@@ -303,6 +309,26 @@ func (c *Client) grpcDialer() func(ctx context.Context, addr string) (net.Conn,
 	}
 }
 
+// checkServerCompatibility is used to check if the server and client versions are compatible.
+func (c *Client) checkServerCompatibility(ctx context.Context) error {
+	if c.c.SkipVersionCheck == true {
+		return nil
+	}
+	resp, err := c.Ping(ctx)
+	if err != nil {
+		return trace.Wrap(err)
+	}
+
+	if err := utils.CheckVersions(resp.ServerVersion, api.MinServerVersion); err != nil {
+		if trace.IsBadParameter(err) {
+			return trace.Wrap(err, "the client requires a server version of %v or higher. Pass SkipVersionCheck=true into the client constructor to bypass this version check.", api.MinServerVersion)
+		}
+		return trace.Wrap(err)
+	}
+
+	return nil
+}
+
 // Config contains configuration of the client
 type Config struct {
 	// Addrs is a list of teleport auth/proxy server addresses to dial.
@@ -330,6 +356,8 @@ type Config struct {
 	// requires this field to be set. If the web proxy was provided with
 	// signed TLS certificates, this field should not be set.
 	InsecureAddressDiscovery bool
+	// Skips checking the server version against the minimum supported server version.
+	SkipVersionCheck bool
 }
 
 // CheckAndSetDefaults checks and sets default config values.

api/go.mod

@@ -3,6 +3,7 @@ module github.com/gravitational/teleport/api
 go 1.15
 
 require (
+	github.com/coreos/go-semver v0.3.0
 	github.com/gogo/protobuf v1.3.1
 	github.com/golang/protobuf v1.4.2
 	github.com/google/go-cmp v0.5.4

api/go.sum

@@ -2,6 +2,8 @@ cloud.google.com/go v0.26.0/go.mod h1:aQUYkXzVsufM+DwF1aE+0xfcU+56JwCaLick0ClmMT
 github.com/BurntSushi/toml v0.3.1/go.mod h1:xHWCNGjB5oqiDr8zfno3MHue2Ht5sIBksp03qcyfWMU=
 github.com/census-instrumentation/opencensus-proto v0.2.1/go.mod h1:f6KPmirojxKA12rnyqOA5BBL4O983OfeGPqjHWSTneU=
 github.com/client9/misspell v0.3.4/go.mod h1:qj6jICC3Q7zFZvVWo7KLAzC3yx5G7kyvSDkc90ppPyw=
+github.com/coreos/go-semver v0.3.0 h1:wkHLiw0WNATZnSG7epLsujiMCgPAc9xhjJ4tgnAxmfM=
+github.com/coreos/go-semver v0.3.0/go.mod h1:nnelYz7RCh+5ahJtPPxZlU+153eP4D4r3EedlOD2RNk=
 github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c=
 github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
 github.com/envoyproxy/go-control-plane v0.9.1-0.20191026205805-5f8ba28d4473/go.mod h1:YTl/9mNaCwkRvm6d1a2C3ymFceY/DCBVvsKhRF0iEA4=

api/utils/utils_test.go

@@ -0,0 +1,57 @@
+/*
+Copyright 2015-2019 Gravitational, Inc.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package utils_test
+
+import (
+	"testing"
+
+	"github.com/gravitational/teleport/api/utils"
+
+	"github.com/gravitational/trace"
+	"github.com/stretchr/testify/require"
+)
+
+func assertIsBadParameter(r require.TestingT, err error, i ...interface{}) {
+	require.True(r, trace.IsBadParameter(err))
+}
+
+// TestVersions tests versions compatibility checking
+func TestVersions(t *testing.T) {
+	t.Parallel()
+	testCases := []struct {
+		desc      string
+		client    string
+		minClient string
+		assertErr require.ErrorAssertionFunc
+	}{
+		{desc: "client older than min version", client: "1.0.0", minClient: "1.1.0", assertErr: assertIsBadParameter},
+		{desc: "client same as min version", client: "1.0.0", minClient: "1.0.0"},
+		{desc: "client newer than min version", client: "1.1.0", minClient: "1.0.0"},
+		{desc: "pre-releases clients are ok", client: "1.1.0-alpha.1", minClient: "1.0.0"},
+		{desc: "older pre-releases are no ok", client: "1.1.0-alpha.1", minClient: "1.1.0", assertErr: assertIsBadParameter},
+	}
+	for _, tt := range testCases {
+		t.Run(tt.desc, func(t *testing.T) {
+			err := utils.CheckVersions(tt.client, tt.minClient)
+			if tt.assertErr != nil {
+				tt.assertErr(t, err)
+			} else {
+				require.NoError(t, err)
+			}
+		})
+	}
+}

lib/utils/ver.go → api/utils/ver.go

@@ -22,24 +22,21 @@ import (
 	"github.com/coreos/go-semver/semver"
 )
 
-// CheckVersions compares client and server versions and makes sure that the
-// client version is greater than or equal to the minimum version supported
-// by the server.
-func CheckVersions(clientVersion, minClientVersion string) error {
-	clientSemver, err := semver.NewVersion(clientVersion)
+// CheckVersions compares a version with a minimum version supported.
+// This is used for comparing server-client compatibility in both direction.
+func CheckVersions(currentVersion, minVersion string) error {
+	currentSemver, err := semver.NewVersion(currentVersion)
 	if err != nil {
-		return trace.Wrap(err, "unsupported version format, need semver format: %q, e.g 1.0.0", clientVersion)
+		return trace.Wrap(err, "unsupported version format, need semver format: %q, e.g 1.0.0", currentVersion)
 	}
 
-	minClientSemver, err := semver.NewVersion(minClientVersion)
+	minSemver, err := semver.NewVersion(minVersion)
 	if err != nil {
-		return trace.Wrap(err, "unsupported version format, need semver format: %q, e.g 1.0.0", minClientVersion)
+		return trace.Wrap(err, "unsupported version format, need semver format: %q, e.g 1.0.0", minVersion)
 	}
 
-	if clientSemver.Compare(*minClientSemver) < 0 {
-		return trace.BadParameter(`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.`, minClientVersion, clientVersion, minClientVersion)
+	if currentSemver.Compare(*minSemver) < 0 {
+		return trace.BadParameter("incompatible versions", currentVersion, minVersion)
 	}
 
 	return nil

api/version.go

@@ -0,0 +1,11 @@
+/* DO NOT EDIT THIS FILE. IT IS GENERATED BY 'make api/version.go'*/
+
+package api
+
+const (
+	Version = "7.0.0-dev"
+)
+
+// Gitref variable is automatically set to the output of git-describe
+// during the build process
+var Gitref string

lib/client/api.go

@@ -52,6 +52,7 @@ import (
 	"github.com/gravitational/teleport/api/profile"
 	"github.com/gravitational/teleport/api/types"
 	"github.com/gravitational/teleport/api/types/wrappers"
+	apiutils "github.com/gravitational/teleport/api/utils"
 	"github.com/gravitational/teleport/api/utils/keypaths"
 	"github.com/gravitational/teleport/lib/auth"
 	"github.com/gravitational/teleport/lib/defaults"
@@ -2340,9 +2341,11 @@ func (tc *TeleportClient) Ping(ctx context.Context) (*webclient.PingResponse, er
 
 	// If version checking was requested and the server advertises a minimum version.
 	if tc.CheckVersions && pr.MinClientVersion != "" {
-		if err := utils.CheckVersions(teleport.Version, pr.MinClientVersion); err != nil {
-			fmt.Printf("\nWARNING: %v\n", err)
-			fmt.Printf("Future versions of tsh will fail when incompatible versions are detected.\n\n")
+		if err := apiutils.CheckVersions(teleport.Version, pr.MinClientVersion); err != nil && trace.IsBadParameter(err) {
+			fmt.Printf("\nWARNING\nDetected potentially incompatible client and server versions.\n")
+			fmt.Printf("Minimum client version supported by the server is %v but you are using %v.\n", pr.MinClientVersion, teleport.Version)
+			fmt.Printf("Please upgrade tsh to %v or newer or use the --skip-version-check flag to bypass this check.\n", pr.MinClientVersion)
+			fmt.Printf("Future versions of tsh will fail when incompatible versions are detected\n\n")
 		}
 	}
 

lib/utils/utils_test.go

@@ -147,31 +147,6 @@ func (s *UtilsSuite) TestMiscFunctions(c *check.C) {
 	c.Assert(RemoveFromSlice([]string{"a", "a", "b"}, "a"), check.DeepEquals, []string{"b"})
 }
 
-// TestVersions tests versions compatibility checking
-func (s *UtilsSuite) TestVersions(c *check.C) {
-	testCases := []struct {
-		info      string
-		client    string
-		minClient string
-		err       error
-	}{
-		{info: "client older than min version", client: "1.0.0", minClient: "1.1.0", err: trace.BadParameter("")},
-		{info: "client same as min version", client: "1.0.0", minClient: "1.0.0"},
-		{info: "client newer than min version", client: "1.1.0", minClient: "1.0.0"},
-		{info: "pre-releases clients are ok", client: "1.1.0-alpha.1", minClient: "1.0.0"},
-		{info: "older pre-releases are no ok", client: "1.1.0-alpha.1", minClient: "1.1.0", err: trace.BadParameter("")},
-	}
-	for i, testCase := range testCases {
-		comment := check.Commentf("test case %v %q", i, testCase.info)
-		err := CheckVersions(testCase.client, testCase.minClient)
-		if testCase.err == nil {
-			c.Assert(err, check.IsNil, comment)
-		} else {
-			c.Assert(err, check.FitsTypeOf, testCase.err, comment)
-		}
-	}
-}
-
 // TestClickableURL tests clickable URL conversions
 func (s *UtilsSuite) TestClickableURL(c *check.C) {
 	testCases := []struct {

version.go

@@ -1,4 +1,4 @@
-/* DO NOT EDIT THIS FILE. IT IS GENERATED BY 'make setver'*/
+/* DO NOT EDIT THIS FILE. IT IS GENERATED BY 'make version.go'*/
 
 package teleport
 

version.mk

@@ -1,15 +1,23 @@
 GITREF=`git describe --dirty --long --tags`
 
 # $(VERSION_GO) will be written to version.go
-VERSION_GO="/* DO NOT EDIT THIS FILE. IT IS GENERATED BY 'make setver'*/\n\n\
+VERSION_GO="/* DO NOT EDIT THIS FILE. IT IS GENERATED BY 'make version.go'*/\n\n\
 package teleport\n\
 const( Version = \"$(VERSION)\" )\n\
 // Gitref variable is automatically set to the output of "git-describe" \n\
 // during the build process\n\
 var Gitref string\n"
 
+# $(API_VERSION_GO) will be written to api/version.go
+API_VERSION_GO="/* DO NOT EDIT THIS FILE. IT IS GENERATED BY 'make api/version.go'*/\n\n\
+package api\n\
+const( Version = \"$(VERSION)\" )\n\
+// Gitref variable is automatically set to the output of "git-describe" \n\
+// during the build process\n\
+var Gitref string\n"
+
 # $(GIT_GO) will be written to gitref.go
-GITREF_GO="/* DO NOT EDIT THIS FILE. IT IS GENERATED BY make */ \n\n\
+GITREF_GO="/* DO NOT EDIT THIS FILE. IT IS GENERATED BY `make gitref.go` */ \n\n\
 package teleport\n\
 func init() { Gitref = \"$(GITREF)\"}  "
 
@@ -19,6 +27,7 @@ func init() { Gitref = \"$(GITREF)\"}  "
 .PHONY:setver
 setver: helm-version
 	@printf $(VERSION_GO) | gofmt > version.go
+	@printf $(API_VERSION_GO) | gofmt > ./api/version.go
 	@printf $(GITREF_GO) | gofmt > gitref.go
 
 # helm-version automatically updates the versions of Helm charts to match the version set in the Makefile,