Merge pull request #21362 from vespa-engine/mpolden/cli-compat

Warn on API incompatibility in Vespa CLI

10 files changed, 127 insertions, 15 deletions

diff --git a/client/go/cmd/command_tester.go b/client/go/cmd/command_tester.go
index 22f50b52cee..82682b3b355 100644
--- a/client/go/cmd/command_tester.go
+++ b/client/go/cmd/command_tester.go
@@ -65,6 +65,7 @@ func execute(cmd command, t *testing.T, client *mockHttpClient) (string, string)
 	rootCmd.Flags().VisitAll(resetFlag)
 	queryCmd.Flags().VisitAll(resetFlag)
 	documentCmd.Flags().VisitAll(resetFlag)
+	logCmd.Flags().VisitAll(resetFlag)
 
 	// Capture stdout and execute command
 	var capturedOut bytes.Buffer
diff --git a/client/go/cmd/helpers.go b/client/go/cmd/helpers.go
index 6cddfd09f54..905e51dda4f 100644
--- a/client/go/cmd/helpers.go
+++ b/client/go/cmd/helpers.go
@@ -13,20 +13,18 @@ import (
 	"strings"
 	"time"
 
+	"github.com/vespa-engine/vespa/client/go/build"
+	"github.com/vespa-engine/vespa/client/go/version"
 	"github.com/vespa-engine/vespa/client/go/vespa"
 )
 
 func printErrHint(err error, hints ...string) {
-	printErr(err)
+	fmt.Fprintln(stderr, color.Red("Error:"), err)
 	for _, hint := range hints {
 		fmt.Fprintln(stderr, color.Cyan("Hint:"), hint)
 	}
 }
 
-func printErr(err error) {
-	fmt.Fprintln(stderr, color.Red("Error:"), err)
-}
-
 func printSuccess(msg ...interface{}) {
 	log.Print(color.Green("Success: "), fmt.Sprint(msg...))
 }
@@ -151,6 +149,21 @@ func getApiURL() string {
 }
 
 func getTarget() (vespa.Target, error) {
+	clientVersion, err := version.Parse(build.Version)
+	if err != nil {
+		return nil, err
+	}
+	target, err := createTarget()
+	if err != nil {
+		return nil, err
+	}
+	if err := target.CheckVersion(clientVersion); err != nil {
+		printErrHint(err, "This is not a fatal error, but this version may not work as expected", "Try 'vespa version' to check for a new version")
+	}
+	return target, nil
+}
+
+func createTarget() (vespa.Target, error) {
 	targetType, err := getTargetType()
 	if err != nil {
 		return nil, err
diff --git a/client/go/cmd/log_test.go b/client/go/cmd/log_test.go
index 4b32927ca17..9d895d1f244 100644
--- a/client/go/cmd/log_test.go
+++ b/client/go/cmd/log_test.go
@@ -6,6 +6,7 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/assert"
+	"github.com/vespa-engine/vespa/client/go/build"
 )
 
 func TestLog(t *testing.T) {
@@ -26,3 +27,21 @@ func TestLog(t *testing.T) {
 	_, errOut := execute(command{homeDir: homeDir, args: []string{"log", "--from", "2021-09-27T13:12:49Z", "--to", "2021-09-27T13:15:00", "1h"}}, t, httpClient)
 	assert.Equal(t, "Error: invalid period: cannot combine --from/--to with relative value: 1h\n", errOut)
 }
+
+func TestLogOldClient(t *testing.T) {
+	buildVersion := build.Version
+	build.Version = "7.0.0"
+	homeDir := filepath.Join(t.TempDir(), ".vespa")
+	pkgDir := mockApplicationPackage(t, false)
+	httpClient := &mockHttpClient{}
+	httpClient.NextResponse(200, `{"minVersion": "8.0.0"}`)
+	execute(command{homeDir: homeDir, args: []string{"config", "set", "application", "t1.a1.i1"}}, t, httpClient)
+	execute(command{homeDir: homeDir, args: []string{"config", "set", "target", "cloud"}}, t, httpClient)
+	execute(command{homeDir: homeDir, args: []string{"api-key"}}, t, httpClient)
+	execute(command{homeDir: homeDir, args: []string{"cert", pkgDir}}, t, httpClient)
+	out, errOut := execute(command{homeDir: homeDir, args: []string{"log"}}, t, httpClient)
+	assert.Equal(t, "", out)
+	expected := "Error: client version 7.0.0 is less than the minimum supported version: 8.0.0\nHint: This is not a fatal error, but this version may not work as expected\nHint: Try 'vespa version' to check for a new version\n"
+	assert.Equal(t, expected, errOut)
+	build.Version = buildVersion
+}
diff --git a/client/go/cmd/prod.go b/client/go/cmd/prod.go
index 89f401a356e..8c40eb969bf 100644
--- a/client/go/cmd/prod.go
+++ b/client/go/cmd/prod.go
@@ -362,7 +362,7 @@ func prompt(r *bufio.Reader, question, defaultAnswer string, validator func(inpu
 		}
 
 		if err := validator(input); err != nil {
-			printErr(err)
+			printErrHint(err)
 			fmt.Fprintln(stderr)
 			input = ""
 		}
diff --git a/client/go/cmd/root.go b/client/go/cmd/root.go
index e25c5cb2d0d..0f4bef5595f 100644
--- a/client/go/cmd/root.go
+++ b/client/go/cmd/root.go
@@ -134,7 +134,7 @@ func Execute() error {
 				printErrHint(cliErr, cliErr.hints...)
 			}
 		} else {
-			printErr(err)
+			printErrHint(err)
 		}
 	}
 	return err
diff --git a/client/go/version/version.go b/client/go/version/version.go
index 00e26d25135..b27529fa5e1 100644
--- a/client/go/version/version.go
+++ b/client/go/version/version.go
@@ -15,6 +15,11 @@ type Version struct {
 	Label string
 }
 
+// IsZero returns whether v is the zero version, 0.0.0.
+func (v Version) IsZero() bool {
+	return v.Major == 0 && v.Minor == 0 && v.Patch == 0
+}
+
 func (v Version) String() string {
 	var sb strings.Builder
 	sb.WriteString(strconv.Itoa(v.Major))
diff --git a/client/go/version/version_test.go b/client/go/version/version_test.go
index 759b1c1d0c1..1caf99a1dd4 100644
--- a/client/go/version/version_test.go
+++ b/client/go/version/version_test.go
@@ -25,6 +25,20 @@ func TestParse(t *testing.T) {
 	assert.Equal(t, "1.2.3-foo", v.String())
 }
 
+func TestIsZero(t *testing.T) {
+	v, err := Parse("0.0.0")
+	assert.Nil(t, err)
+	assert.True(t, v.IsZero())
+
+	v, err = Parse("0.0.0-foo")
+	assert.Nil(t, err)
+	assert.True(t, v.IsZero())
+
+	v, err = Parse("1.2.3")
+	assert.Nil(t, err)
+	assert.False(t, v.IsZero())
+}
+
 func TestCompare(t *testing.T) {
 	assertComparison(t, "1.2.3", '>', "1.0.0")
 	assertComparison(t, "1.0.0", '<', "1.2.3")
diff --git a/client/go/vespa/deploy.go b/client/go/vespa/deploy.go
index 6ef4995e181..cac03067a69 100644
--- a/client/go/vespa/deploy.go
+++ b/client/go/vespa/deploy.go
@@ -333,7 +333,7 @@ func Submit(opts DeploymentOpts) error {
 	request.Header.Set("Content-Type", writer.FormDataContentType())
 	serviceDescription := "Submit service"
 	sigKeyId := opts.Deployment.Application.SerializedForm()
-	if err := opts.Target.PrepareApiRequest(request, sigKeyId); err != nil {
+	if err := opts.Target.SignRequest(request, sigKeyId); err != nil {
 		return err
 	}
 	response, err := util.HttpDo(request, time.Minute*10, sigKeyId)
@@ -371,7 +371,7 @@ func uploadApplicationPackage(url *url.URL, opts DeploymentOpts) (int64, error)
 	}
 	serviceDescription := "Deploy service"
 	sigKeyId := opts.Deployment.Application.SerializedForm()
-	if err := opts.Target.PrepareApiRequest(request, sigKeyId); err != nil {
+	if err := opts.Target.SignRequest(request, sigKeyId); err != nil {
 		return 0, err
 	}
 
diff --git a/client/go/vespa/target.go b/client/go/vespa/target.go
index f50716a5a3a..ae4934d788d 100644
--- a/client/go/vespa/target.go
+++ b/client/go/vespa/target.go
@@ -20,6 +20,7 @@ import (
 
 	"github.com/vespa-engine/vespa/client/go/auth0"
 	"github.com/vespa-engine/vespa/client/go/util"
+	"github.com/vespa-engine/vespa/client/go/version"
 )
 
 const (
@@ -52,7 +53,11 @@ type Target interface {
 	// PrintLog writes the logs of this deployment using given options to control output.
 	PrintLog(options LogOptions) error
 
-	PrepareApiRequest(req *http.Request, sigKeyId string) error
+	// SignRequest signs request with given keyID as required by the implementation of this target.
+	SignRequest(request *http.Request, keyID string) error
+
+	// CheckVersion verifies whether clientVersion is compatible with this target.
+	CheckVersion(clientVersion version.Version) error
 }
 
 // TLSOptions configures the certificate to use for service requests.
@@ -85,7 +90,9 @@ type customTarget struct {
 	baseURL    string
 }
 
-func (t *customTarget) PrepareApiRequest(req *http.Request, sigKeyId string) error { return nil }
+func (t *customTarget) SignRequest(req *http.Request, sigKeyId string) error { return nil }
+
+func (t *customTarget) CheckVersion(version version.Version) error { return nil }
 
 // Do sends request to this service. Any required authentication happens automatically.
 func (s *Service) Do(request *http.Request, timeout time.Duration) (*http.Response, error) {
@@ -265,7 +272,7 @@ func (t *cloudTarget) Service(name string, timeout time.Duration, runID int64, c
 	return nil, fmt.Errorf("unknown service: %s", name)
 }
 
-func (t *cloudTarget) PrepareApiRequest(req *http.Request, sigKeyId string) error {
+func (t *cloudTarget) SignRequest(req *http.Request, sigKeyId string) error {
 	if Auth0AccessTokenEnabled() {
 		if t.cloudAuth == "access-token" {
 			if err := t.addAuth0AccessToken(req); err != nil {
@@ -289,6 +296,36 @@ func (t *cloudTarget) PrepareApiRequest(req *http.Request, sigKeyId string) erro
 	return nil
 }
 
+func (t *cloudTarget) CheckVersion(clientVersion version.Version) error {
+	if clientVersion.IsZero() { // development version is always fine
+		return nil
+	}
+	req, err := http.NewRequest("GET", fmt.Sprintf("%s/cli/v1/", t.apiURL), nil)
+	if err != nil {
+		return err
+	}
+	response, err := util.HttpDo(req, 10*time.Second, "")
+	if err != nil {
+		return err
+	}
+	defer response.Body.Close()
+	var cliResponse struct {
+		MinVersion string `json:"minVersion"`
+	}
+	dec := json.NewDecoder(response.Body)
+	if err := dec.Decode(&cliResponse); err != nil {
+		return err
+	}
+	minVersion, err := version.Parse(cliResponse.MinVersion)
+	if err != nil {
+		return err
+	}
+	if clientVersion.Less(minVersion) {
+		return fmt.Errorf("client version %s is less than the minimum supported version: %s", clientVersion, minVersion)
+	}
+	return nil
+}
+
 func (t *cloudTarget) addAuth0AccessToken(request *http.Request) error {
 	a, err := auth0.GetAuth0(t.authConfigPath, t.systemName, t.apiURL)
 	if err != nil {
@@ -324,7 +361,7 @@ func (t *cloudTarget) PrintLog(options LogOptions) error {
 			q.Set("to", strconv.FormatInt(toMillis, 10))
 		}
 		req.URL.RawQuery = q.Encode()
-		t.PrepareApiRequest(req, t.deployment.Application.SerializedForm())
+		t.SignRequest(req, t.deployment.Application.SerializedForm())
 		return req
 	}
 	logFunc := func(status int, response []byte) (bool, error) {
@@ -380,7 +417,7 @@ func (t *cloudTarget) waitForRun(runID int64, timeout time.Duration) error {
 		q := req.URL.Query()
 		q.Set("after", strconv.FormatInt(lastID, 10))
 		req.URL.RawQuery = q.Encode()
-		if err := t.PrepareApiRequest(req, t.deployment.Application.SerializedForm()); err != nil {
+		if err := t.SignRequest(req, t.deployment.Application.SerializedForm()); err != nil {
 			panic(err)
 		}
 		return req
@@ -439,7 +476,7 @@ func (t *cloudTarget) discoverEndpoints(timeout time.Duration) error {
 	if err != nil {
 		return err
 	}
-	if err := t.PrepareApiRequest(req, t.deployment.Application.SerializedForm()); err != nil {
+	if err := t.SignRequest(req, t.deployment.Application.SerializedForm()); err != nil {
 		return err
 	}
 	urlsByCluster := make(map[string]string)
diff --git a/client/go/vespa/target_test.go b/client/go/vespa/target_test.go
index 0cfe9f1962c..5aa20b22465 100644
--- a/client/go/vespa/target_test.go
+++ b/client/go/vespa/target_test.go
@@ -13,6 +13,7 @@ import (
 	"time"
 
 	"github.com/stretchr/testify/assert"
+	"github.com/vespa-engine/vespa/client/go/version"
 )
 
 type mockVespaApi struct {
@@ -22,6 +23,9 @@ type mockVespaApi struct {
 
 func (v *mockVespaApi) mockVespaHandler(w http.ResponseWriter, req *http.Request) {
 	switch req.URL.Path {
+	case "/cli/v1/":
+		response := `{"minVersion":"8.0.0"}`
+		w.Write([]byte(response))
 	case "/application/v4/tenant/t1/application/a1/instance/i1/environment/dev/region/us-north-1":
 		response := "{}"
 		if v.deploymentConverged {
@@ -137,6 +141,25 @@ func TestLog(t *testing.T) {
 	assert.Equal(t, expected, buf.String())
 }
 
+func TestCheckVersion(t *testing.T) {
+	vc := mockVespaApi{}
+	srv := httptest.NewServer(http.HandlerFunc(vc.mockVespaHandler))
+	defer srv.Close()
+
+	target := createCloudTarget(t, srv.URL, ioutil.Discard)
+	assert.Nil(t, target.CheckVersion(mustVersion("8.0.0")))
+	assert.Nil(t, target.CheckVersion(mustVersion("8.1.0")))
+	assert.NotNil(t, target.CheckVersion(mustVersion("7.0.0")))
+}
+
+func mustVersion(s string) version.Version {
+	v, err := version.Parse(s)
+	if err != nil {
+		panic(err)
+	}
+	return v
+}
+
 func createCloudTarget(t *testing.T, url string, logWriter io.Writer) Target {
 	kp, err := CreateKeyPair()
 	assert.Nil(t, err)