From 26d2e98d97919f22758066fb024005ed5e20960e Mon Sep 17 00:00:00 2001
From: Martin Polden <mpolden@mpolden.no>
Date: Tue, 15 Mar 2022 09:36:17 +0100
Subject: Revert "Revert "Use entity tag to expire sample apps cache""

---
 client/go/auth/zts/zts_test.go   |   4 +-
 client/go/cmd/api_key.go         |   2 +-
 client/go/cmd/clone.go           | 199 ++++++++++++++++++++++++++++-----------
 client/go/cmd/clone_list_test.go |   8 +-
 client/go/cmd/clone_test.go      |  67 +++++++++----
 client/go/cmd/config.go          |   6 ++
 client/go/cmd/deploy_test.go     |   6 +-
 client/go/cmd/document_test.go   |   6 +-
 client/go/cmd/log_test.go        |   6 +-
 client/go/cmd/prod.go            |   2 +-
 client/go/cmd/prod_test.go       |   4 +-
 client/go/cmd/query_test.go      |   8 +-
 client/go/cmd/root.go            |  12 +--
 client/go/cmd/test_test.go       |  10 +-
 client/go/cmd/version_test.go    |   4 +-
 client/go/mock/http.go           |  30 +++---
 16 files changed, 257 insertions(+), 117 deletions(-)

(limited to 'client')

diff --git a/client/go/auth/zts/zts_test.go b/client/go/auth/zts/zts_test.go
index 0eec085aadb..504f29e9808 100644
--- a/client/go/auth/zts/zts_test.go
+++ b/client/go/auth/zts/zts_test.go
@@ -13,12 +13,12 @@ func TestAccessToken(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	httpClient.NextResponse(400, `{"message": "bad request"}`)
+	httpClient.NextResponseString(400, `{"message": "bad request"}`)
 	_, err = client.AccessToken("vespa.vespa", tls.Certificate{})
 	if err == nil {
 		t.Fatal("want error for non-ok response status")
 	}
-	httpClient.NextResponse(200, `{"access_token": "foo bar"}`)
+	httpClient.NextResponseString(200, `{"access_token": "foo bar"}`)
 	token, err := client.AccessToken("vespa.vespa", tls.Certificate{})
 	if err != nil {
 		t.Fatal(err)
diff --git a/client/go/cmd/api_key.go b/client/go/cmd/api_key.go
index cacfe3fd180..1050b040aae 100644
--- a/client/go/cmd/api_key.go
+++ b/client/go/cmd/api_key.go
@@ -70,7 +70,7 @@ func doApiKey(cli *CLI, overwriteKey bool, args []string) error {
 	apiKeyFile := cli.config.apiKeyPath(app.Tenant)
 	if util.PathExists(apiKeyFile) && !overwriteKey {
 		err := fmt.Errorf("refusing to overwrite %s", apiKeyFile)
-		cli.printErrHint(err, "Use -f to overwrite it")
+		cli.printErr(err, "Use -f to overwrite it")
 		printPublicKey(system, apiKeyFile, app.Tenant)
 		return ErrCLI{error: err, quiet: true}
 	}
diff --git a/client/go/cmd/clone.go b/client/go/cmd/clone.go
index 0829e064af9..03a55d839d6 100644
--- a/client/go/cmd/clone.go
+++ b/client/go/cmd/clone.go
@@ -6,14 +6,13 @@ package cmd
 
 import (
 	"archive/zip"
-	"errors"
 	"fmt"
 	"io"
-	"io/ioutil"
 	"log"
 	"net/http"
 	"os"
 	"path/filepath"
+	"sort"
 	"strings"
 	"time"
 
@@ -21,10 +20,12 @@ import (
 	"github.com/spf13/cobra"
 )
 
+const sampleAppsNamePrefix = "sample-apps-master"
+
 func newCloneCmd(cli *CLI) *cobra.Command {
 	var (
-		listApps   bool
-		forceClone bool
+		listApps bool
+		noCache  bool
 	)
 	cmd := &cobra.Command{
 		Use:   "clone sample-application-path target-directory",
@@ -55,24 +56,37 @@ variable.`,
 			if len(args) != 2 {
 				return fmt.Errorf("expected exactly 2 arguments, got %d", len(args))
 			}
-			return cloneApplication(cli, args[0], args[1], forceClone)
+			cloner := &cloner{cli: cli, noCache: noCache}
+			return cloner.Clone(args[0], args[1])
 		},
 	}
 	cmd.Flags().BoolVarP(&listApps, "list", "l", false, "List available sample applications")
-	cmd.Flags().BoolVarP(&forceClone, "force", "f", false, "Ignore cache and force downloading the latest sample application from GitHub")
+	cmd.Flags().BoolVarP(&noCache, "force", "f", false, "Ignore cache and force downloading the latest sample application from GitHub")
 	return cmd
 }
 
-func cloneApplication(cli *CLI, applicationName string, applicationDir string, force bool) error {
-	zipFile, err := openSampleAppsZip(force, cli)
+type cloner struct {
+	cli     *CLI
+	noCache bool
+}
+
+type zipFile struct {
+	path    string
+	etag    string
+	modTime time.Time
+}
+
+// Clone copies the application identified by applicationName into given path. If the cached copy of sample applications
+// has expired (as determined by its entity tag), a current copy will be downloaded from GitHub automatically.
+func (c *cloner) Clone(applicationName, path string) error {
+	zipPath, err := c.zipPath()
 	if err != nil {
 		return err
 	}
-	defer zipFile.Close()
 
-	r, err := zip.OpenReader(zipFile.Name())
+	r, err := zip.OpenReader(zipPath)
 	if err != nil {
-		return fmt.Errorf("could not open sample apps zip '%s': %w", color.CyanString(zipFile.Name()), err)
+		return fmt.Errorf("could not open sample apps zip '%s': %w", color.CyanString(zipPath), err)
 	}
 	defer r.Close()
 
@@ -81,91 +95,168 @@ func cloneApplication(cli *CLI, applicationName string, applicationDir string, f
 		dirPrefix := "sample-apps-master/" + applicationName + "/"
 		if strings.HasPrefix(f.Name, dirPrefix) {
 			if !found { // Create destination directory lazily when source is found
-				createErr := os.Mkdir(applicationDir, 0755)
+				createErr := os.Mkdir(path, 0755)
 				if createErr != nil {
-					return fmt.Errorf("could not create directory '%s': %w", color.CyanString(applicationDir), createErr)
+					return fmt.Errorf("could not create directory '%s': %w", color.CyanString(path), createErr)
 				}
 			}
 			found = true
 
-			if err := copy(f, applicationDir, dirPrefix); err != nil {
+			if err := copy(f, path, dirPrefix); err != nil {
 				return fmt.Errorf("could not copy zip entry '%s': %w", color.CyanString(f.Name), err)
 			}
 		}
 	}
+
 	if !found {
 		return errHint(fmt.Errorf("could not find source application '%s'", color.CyanString(applicationName)), "Use -f to ignore the cache")
 	} else {
-		log.Print("Created ", color.CyanString(applicationDir))
+		log.Print("Created ", color.CyanString(path))
 	}
 	return nil
 }
 
-func useCache(force bool, stat os.FileInfo) (bool, error) {
-	if force {
-		return false, nil
+// zipPath returns the path to the latest sample application ZIP file.
+func (c *cloner) zipPath() (string, error) {
+	zipFiles, err := c.listZipFiles()
+	if err != nil {
+		return "", nil
+	}
+	cacheCandidates := zipFiles
+	if c.noCache {
+		cacheCandidates = nil
 	}
-	expiry := stat.ModTime().Add(time.Hour * 168) // 1 week
-	return stat.Size() > 0 && time.Now().Before(expiry), nil
+	zipPath, cacheHit, err := c.downloadZip(cacheCandidates)
+	if err != nil {
+		if cacheHit {
+			c.cli.printWarning(err)
+		} else {
+			return "", err
+		}
+	}
+	if cacheHit {
+		log.Print(color.YellowString("Using cached sample apps ..."))
+	}
+	// Remove obsolete files
+	for _, zf := range zipFiles {
+		if zf.path != zipPath {
+			os.Remove(zf.path)
+		}
+	}
+	return zipPath, nil
 }
 
-func fetchSampleAppsZip(destination string, cli *CLI) error {
-	f, err := ioutil.TempFile(filepath.Dir(destination), "sample-apps")
+// listZipFiles list all sample apps ZIP files found in cacheDir.
+func (c *cloner) listZipFiles() ([]zipFile, error) {
+	dirEntries, err := os.ReadDir(c.cli.config.cacheDir)
 	if err != nil {
-		return fmt.Errorf("could not create temporary file: %w", err)
+		return nil, err
+	}
+	var zipFiles []zipFile
+	for _, entry := range dirEntries {
+		ext := filepath.Ext(entry.Name())
+		if ext != ".zip" {
+			continue
+		}
+		if !strings.HasPrefix(entry.Name(), sampleAppsNamePrefix) {
+			continue
+		}
+		fi, err := entry.Info()
+		if err != nil {
+			return nil, err
+		}
+		name := fi.Name()
+		etag := ""
+		parts := strings.Split(name, "_")
+		if len(parts) == 2 {
+			etag = strings.TrimSuffix(parts[1], ext)
+		}
+		zipFiles = append(zipFiles, zipFile{
+			path:    filepath.Join(c.cli.config.cacheDir, name),
+			etag:    etag,
+			modTime: fi.ModTime(),
+		})
+	}
+	return zipFiles, nil
+}
+
+// downloadZip conditionally downloads the latest sample apps ZIP file. If any of the ZIP files among cacheFiles are
+// usable, downloading is skipped.
+func (c *cloner) downloadZip(cachedFiles []zipFile) (string, bool, error) {
+	zipPath := ""
+	etag := ""
+	sort.Slice(cachedFiles, func(i, j int) bool { return cachedFiles[i].modTime.Before(cachedFiles[j].modTime) })
+	if len(cachedFiles) > 0 {
+		latest := cachedFiles[len(cachedFiles)-1]
+		zipPath = latest.path
+		etag = latest.etag
 	}
-	defer f.Close()
-	return cli.spinner(cli.Stderr, color.YellowString("Downloading sample apps ..."), func() error {
+	// The latest cached file, if any, is considered a hit until we have downloaded a fresh one. This allows us to use
+	// the cached copy if GitHub is unavailable.
+	cacheHit := zipPath != ""
+	err := c.cli.spinner(c.cli.Stderr, color.YellowString("Downloading sample apps ..."), func() error {
 		request, err := http.NewRequest("GET", "https://github.com/vespa-engine/sample-apps/archive/refs/heads/master.zip", nil)
 		if err != nil {
 			return fmt.Errorf("invalid url: %w", err)
 		}
-		response, err := cli.httpClient.Do(request, time.Minute*60)
+		if etag != "" {
+			request.Header = make(http.Header)
+			request.Header.Set("if-none-match", fmt.Sprintf(`W/"%s"`, etag))
+		}
+		response, err := c.cli.httpClient.Do(request, time.Minute*60)
 		if err != nil {
 			return fmt.Errorf("could not download sample apps: %w", err)
 		}
 		defer response.Body.Close()
+		if response.StatusCode == http.StatusNotModified { // entity tag matched so our cached copy is current
+			return nil
+		}
 		if response.StatusCode != http.StatusOK {
 			return fmt.Errorf("could not download sample apps: github returned status %d", response.StatusCode)
 		}
-		if _, err := io.Copy(f, response.Body); err != nil {
-			return fmt.Errorf("could not write sample apps to file: %s: %w", f.Name(), err)
-		}
-		f.Close()
-		if err := os.Rename(f.Name(), destination); err != nil {
-			return fmt.Errorf("could not move sample apps to cache path")
+		etag = trimEntityTagID(response.Header.Get("etag"))
+		newPath, err := c.writeZip(response.Body, etag)
+		if err != nil {
+			return err
 		}
+		zipPath = newPath
+		cacheHit = false
 		return nil
 	})
+	return zipPath, cacheHit, err
 }
 
-func openSampleAppsZip(forceClone bool, cli *CLI) (*os.File, error) {
-	cacheDir, err := vespaCliCacheDir(cli.Environment)
+// writeZip atomically writes the contents of reader zipReader to a file in the CLI cache directory.
+func (c *cloner) writeZip(zipReader io.Reader, etag string) (string, error) {
+	f, err := os.CreateTemp(c.cli.config.cacheDir, "sample-apps-tmp-")
 	if err != nil {
-		return nil, err
-	}
-	path := filepath.Join(cacheDir, "sample-apps-master.zip")
-	cacheExists := true
-	stat, err := os.Stat(path)
-	if errors.Is(err, os.ErrNotExist) {
-		cacheExists = false
-	} else if err != nil {
-		return nil, fmt.Errorf("could not stat existing cache file: %w", err)
+		return "", fmt.Errorf("could not create temporary file: %w", err)
 	}
-	if cacheExists {
-		useCache, err := useCache(forceClone, stat)
-		if err != nil {
-			return nil, errHint(fmt.Errorf("could not determine cache status: %w", err), "Try ignoring the cache with the -f flag")
-		}
-		if useCache {
-			log.Print(color.YellowString("Using cached sample apps ..."))
-			return os.Open(path)
+	cleanTemp := true
+	defer func() {
+		f.Close()
+		if cleanTemp {
+			os.Remove(f.Name())
 		}
+	}()
+	if _, err := io.Copy(f, zipReader); err != nil {
+		return "", fmt.Errorf("could not write sample apps to file: %s: %w", f.Name(), err)
+	}
+	f.Close()
+	path := filepath.Join(c.cli.config.cacheDir, sampleAppsNamePrefix)
+	if etag != "" {
+		path += "_" + etag
 	}
-	if err := fetchSampleAppsZip(path, cli); err != nil {
-		return nil, fmt.Errorf("could not fetch sample apps: %w", err)
+	path += ".zip"
+	if err := os.Rename(f.Name(), path); err != nil {
+		return "", fmt.Errorf("could not move sample apps to %s", path)
 	}
-	return os.Open(path)
+	cleanTemp = false
+	return path, nil
+}
+
+func trimEntityTagID(s string) string {
+	return strings.TrimSuffix(strings.TrimPrefix(s, `W/"`), `"`)
 }
 
 func copy(f *zip.File, destinationDir string, zipEntryPrefix string) error {
diff --git a/client/go/cmd/clone_list_test.go b/client/go/cmd/clone_list_test.go
index 73aa70f12f2..2ed2449af7d 100644
--- a/client/go/cmd/clone_list_test.go
+++ b/client/go/cmd/clone_list_test.go
@@ -12,10 +12,10 @@ import (
 
 func TestListSampleApps(t *testing.T) {
 	c := &mock.HTTPClient{}
-	c.NextResponse(200, readTestData(t, "sample-apps-contents.json"))
-	c.NextResponse(200, readTestData(t, "sample-apps-news.json"))
-	c.NextResponse(200, readTestData(t, "sample-apps-operations.json"))
-	c.NextResponse(200, readTestData(t, "sample-apps-vespa-cloud.json"))
+	c.NextResponseString(200, readTestData(t, "sample-apps-contents.json"))
+	c.NextResponseString(200, readTestData(t, "sample-apps-news.json"))
+	c.NextResponseString(200, readTestData(t, "sample-apps-operations.json"))
+	c.NextResponseString(200, readTestData(t, "sample-apps-vespa-cloud.json"))
 
 	apps, err := listSampleApps(c)
 	assert.Nil(t, err)
diff --git a/client/go/cmd/clone_test.go b/client/go/cmd/clone_test.go
index 9587a1435d3..1971e0032a4 100644
--- a/client/go/cmd/clone_test.go
+++ b/client/go/cmd/clone_test.go
@@ -5,9 +5,10 @@
 package cmd
 
 import (
-	"io/ioutil"
+	"net/http"
 	"os"
 	"path/filepath"
+	"strings"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -21,28 +22,60 @@ func TestClone(t *testing.T) {
 }
 
 func assertCreated(sampleAppName string, app string, t *testing.T) {
-	appCached := app + "-cache"
+	tempDir := t.TempDir()
+	app1 := filepath.Join(tempDir, "app1")
 	defer os.RemoveAll(app)
-	defer os.RemoveAll(appCached)
 
 	httpClient := &mock.HTTPClient{}
-	testdata, err := ioutil.ReadFile(filepath.Join("testdata", "sample-apps-master.zip"))
+	cli, stdout, stderr := newTestCLI(t)
+	cli.httpClient = httpClient
+	testdata, err := os.ReadFile(filepath.Join("testdata", "sample-apps-master.zip"))
 	require.Nil(t, err)
+
+	// Initial cloning. GitHub includes the ETag header, but we don't require it
 	httpClient.NextResponseBytes(200, testdata)
+	require.Nil(t, cli.Run("clone", sampleAppName, app1))
+	assert.Equal(t, "Created "+app1+"\n", stdout.String())
+	assertFiles(t, app1)
 
-	cli, stdout, _ := newTestCLI(t)
-	cli.httpClient = httpClient
-	err = cli.Run("clone", sampleAppName, app)
-	assert.Nil(t, err)
+	// Clone with cache hit
+	httpClient.NextStatus(http.StatusNotModified)
+	stdout.Reset()
+	app2 := filepath.Join(tempDir, "app2")
+	require.Nil(t, cli.Run("clone", sampleAppName, app2))
+	assert.Equal(t, "Using cached sample apps ...\nCreated "+app2+"\n", stdout.String())
+	assertFiles(t, app2)
 
-	assert.Equal(t, "Created "+app+"\n", stdout.String())
-	assertFiles(t, app)
+	// Clone while ignoring cache
+	headers := make(http.Header)
+	headers.Set("etag", `W/"id1"`)
+	httpClient.NextResponse(mock.HTTPResponse{Status: 200, Body: testdata, Header: headers})
+	stdout.Reset()
+	app3 := filepath.Join(tempDir, "app3")
+	require.Nil(t, cli.Run("clone", "-f", sampleAppName, app3))
+	assert.Equal(t, "Created "+app3+"\n", stdout.String())
+	assertFiles(t, app3)
 
+	// Cloning falls back to cached copy if GitHub is unavailable
+	httpClient.NextStatus(500)
 	stdout.Reset()
-	err = cli.Run("clone", sampleAppName, appCached)
-	assert.Nil(t, err)
-	assert.Equal(t, "Using cached sample apps ...\nCreated "+appCached+"\n", stdout.String())
-	assertFiles(t, appCached)
+	app4 := filepath.Join(tempDir, "app4")
+	require.Nil(t, cli.Run("clone", "-f=false", sampleAppName, app4))
+	assert.Equal(t, "Warning: could not download sample apps: github returned status 500\n", stderr.String())
+	assert.Equal(t, "Using cached sample apps ...\nCreated "+app4+"\n", stdout.String())
+	assertFiles(t, app4)
+
+	// The only cached file is the latest one
+	dirEntries, err := os.ReadDir(cli.config.cacheDir)
+	require.Nil(t, err)
+	var zipFiles []string
+	for _, de := range dirEntries {
+		name := de.Name()
+		if strings.HasPrefix(name, sampleAppsNamePrefix) {
+			zipFiles = append(zipFiles, name)
+		}
+	}
+	assert.Equal(t, []string{"sample-apps-master_id1.zip"}, zipFiles)
 }
 
 func assertFiles(t *testing.T, app string) {
@@ -50,10 +83,12 @@ func assertFiles(t *testing.T, app string) {
 	assert.True(t, util.PathExists(filepath.Join(app, "src", "main", "application")))
 	assert.True(t, util.IsDirectory(filepath.Join(app, "src", "main", "application")))
 
-	servicesStat, _ := os.Stat(filepath.Join(app, "src", "main", "application", "services.xml"))
+	servicesStat, err := os.Stat(filepath.Join(app, "src", "main", "application", "services.xml"))
+	require.Nil(t, err)
 	servicesSize := int64(1772)
 	assert.Equal(t, servicesSize, servicesStat.Size())
 
-	scriptStat, _ := os.Stat(filepath.Join(app, "bin", "convert-msmarco.sh"))
+	scriptStat, err := os.Stat(filepath.Join(app, "bin", "convert-msmarco.sh"))
+	require.Nil(t, err)
 	assert.Equal(t, os.FileMode(0755), scriptStat.Mode())
 }
diff --git a/client/go/cmd/config.go b/client/go/cmd/config.go
index 75bd9959280..b75ae046534 100644
--- a/client/go/cmd/config.go
+++ b/client/go/cmd/config.go
@@ -95,6 +95,7 @@ $ vespa config get target`,
 
 type Config struct {
 	homeDir     string
+	cacheDir    string
 	environment map[string]string
 	bindings    ConfigBindings
 	createDirs  bool
@@ -131,8 +132,13 @@ func loadConfig(environment map[string]string, bindings ConfigBindings) (*Config
 	if err != nil {
 		return nil, fmt.Errorf("could not detect config directory: %w", err)
 	}
+	cacheDir, err := vespaCliCacheDir(environment)
+	if err != nil {
+		return nil, fmt.Errorf("could not detect cache directory: %w", err)
+	}
 	c := &Config{
 		homeDir:     home,
+		cacheDir:    cacheDir,
 		environment: environment,
 		bindings:    bindings,
 		createDirs:  true,
diff --git a/client/go/cmd/deploy_test.go b/client/go/cmd/deploy_test.go
index e2d6da5399d..c46322953c0 100644
--- a/client/go/cmd/deploy_test.go
+++ b/client/go/cmd/deploy_test.go
@@ -119,7 +119,7 @@ func assertDeploy(applicationPackage string, arguments []string, t *testing.T) {
 
 func assertPrepare(applicationPackage string, arguments []string, t *testing.T) {
 	client := &mock.HTTPClient{}
-	client.NextResponse(200, `{"session-id":"42"}`)
+	client.NextResponseString(200, `{"session-id":"42"}`)
 	cli, stdout, _ := newTestCLI(t)
 	cli.httpClient = client
 	assert.Nil(t, cli.Run(arguments...))
@@ -170,7 +170,7 @@ func assertDeployRequestMade(target string, client *mock.HTTPClient, t *testing.
 
 func assertApplicationPackageError(t *testing.T, cmd string, status int, expectedMessage string, returnBody string) {
 	client := &mock.HTTPClient{}
-	client.NextResponse(status, returnBody)
+	client.NextResponseString(status, returnBody)
 	cli, _, stderr := newTestCLI(t)
 	cli.httpClient = client
 	assert.NotNil(t, cli.Run(cmd, "testdata/applications/withTarget/target/application.zip"))
@@ -181,7 +181,7 @@ func assertApplicationPackageError(t *testing.T, cmd string, status int, expecte
 
 func assertDeployServerError(t *testing.T, status int, errorMessage string) {
 	client := &mock.HTTPClient{}
-	client.NextResponse(status, errorMessage)
+	client.NextResponseString(status, errorMessage)
 	cli, _, stderr := newTestCLI(t)
 	cli.httpClient = client
 	assert.NotNil(t, cli.Run("deploy", "testdata/applications/withTarget/target/application.zip"))
diff --git a/client/go/cmd/document_test.go b/client/go/cmd/document_test.go
index 77b7d68d666..b6f3cbcf6ca 100644
--- a/client/go/cmd/document_test.go
+++ b/client/go/cmd/document_test.go
@@ -131,7 +131,7 @@ func assertDocumentGet(arguments []string, documentId string, t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	client.NextResponse(200, "{\"fields\":{\"foo\":\"bar\"}}")
+	client.NextResponseString(200, "{\"fields\":{\"foo\":\"bar\"}}")
 	cli, stdout, _ := newTestCLI(t)
 	cli.httpClient = client
 	assert.Nil(t, cli.Run(arguments...))
@@ -150,7 +150,7 @@ func assertDocumentGet(arguments []string, documentId string, t *testing.T) {
 
 func assertDocumentError(t *testing.T, status int, errorMessage string) {
 	client := &mock.HTTPClient{}
-	client.NextResponse(status, errorMessage)
+	client.NextResponseString(status, errorMessage)
 	cli, _, stderr := newTestCLI(t)
 	cli.httpClient = client
 	assert.NotNil(t, cli.Run("document", "put",
@@ -163,7 +163,7 @@ func assertDocumentError(t *testing.T, status int, errorMessage string) {
 
 func assertDocumentServerError(t *testing.T, status int, errorMessage string) {
 	client := &mock.HTTPClient{}
-	client.NextResponse(status, errorMessage)
+	client.NextResponseString(status, errorMessage)
 	cli, _, stderr := newTestCLI(t)
 	cli.httpClient = client
 	assert.NotNil(t, cli.Run("document", "put",
diff --git a/client/go/cmd/log_test.go b/client/go/cmd/log_test.go
index fb08474dd4d..d3e7b630869 100644
--- a/client/go/cmd/log_test.go
+++ b/client/go/cmd/log_test.go
@@ -12,7 +12,7 @@ import (
 func TestLog(t *testing.T) {
 	pkgDir := mockApplicationPackage(t, false)
 	httpClient := &mock.HTTPClient{}
-	httpClient.NextResponse(200, `1632738690.905535	host1a.dev.aws-us-east-1c	806/53	logserver-container	Container.com.yahoo.container.jdisc.ConfiguredApplication	info	Switching to the latest deployed set of configurations and components. Application config generation: 52532`)
+	httpClient.NextResponseString(200, `1632738690.905535	host1a.dev.aws-us-east-1c	806/53	logserver-container	Container.com.yahoo.container.jdisc.ConfiguredApplication	info	Switching to the latest deployed set of configurations and components. Application config generation: 52532`)
 	cli, stdout, stderr := newTestCLI(t)
 	cli.httpClient = httpClient
 
@@ -36,8 +36,8 @@ func TestLogOldClient(t *testing.T) {
 
 	pkgDir := mockApplicationPackage(t, false)
 	httpClient := &mock.HTTPClient{}
-	httpClient.NextResponse(200, `{"minVersion": "8.0.0"}`)
-	httpClient.NextResponse(200, `1632738690.905535	host1a.dev.aws-us-east-1c	806/53	logserver-container	Container.com.yahoo.container.jdisc.ConfiguredApplication	info	Switching to the latest deployed set of configurations and components. Application config generation: 52532`)
+	httpClient.NextResponseString(200, `{"minVersion": "8.0.0"}`)
+	httpClient.NextResponseString(200, `1632738690.905535	host1a.dev.aws-us-east-1c	806/53	logserver-container	Container.com.yahoo.container.jdisc.ConfiguredApplication	info	Switching to the latest deployed set of configurations and components. Application config generation: 52532`)
 	cli.httpClient = httpClient
 
 	assert.Nil(t, cli.Run("config", "set", "application", "t1.a1.i1"))
diff --git a/client/go/cmd/prod.go b/client/go/cmd/prod.go
index 9108cb7a222..6764d4a85f2 100644
--- a/client/go/cmd/prod.go
+++ b/client/go/cmd/prod.go
@@ -359,7 +359,7 @@ func prompt(cli *CLI, stdin *bufio.Reader, question, defaultAnswer string, valid
 		}
 
 		if err := validator(input); err != nil {
-			cli.printErrHint(err)
+			cli.printErr(err)
 			fmt.Fprintln(cli.Stderr)
 			input = ""
 		}
diff --git a/client/go/cmd/prod_test.go b/client/go/cmd/prod_test.go
index 06883dfcdcf..0441d334fa1 100644
--- a/client/go/cmd/prod_test.go
+++ b/client/go/cmd/prod_test.go
@@ -152,7 +152,7 @@ func TestProdSubmit(t *testing.T) {
 	createApplication(t, pkgDir, false)
 
 	httpClient := &mock.HTTPClient{}
-	httpClient.NextResponse(200, `ok`)
+	httpClient.NextResponseString(200, `ok`)
 
 	cli, stdout, _ := newTestCLI(t, "CI=true")
 	cli.httpClient = httpClient
@@ -195,7 +195,7 @@ func TestProdSubmitWithJava(t *testing.T) {
 	createApplication(t, pkgDir, true)
 
 	httpClient := &mock.HTTPClient{}
-	httpClient.NextResponse(200, `ok`)
+	httpClient.NextResponseString(200, `ok`)
 	cli, stdout, _ := newTestCLI(t, "CI=true")
 	cli.httpClient = httpClient
 	assert.Nil(t, cli.Run("config", "set", "application", "t1.a1.i1"))
diff --git a/client/go/cmd/query_test.go b/client/go/cmd/query_test.go
index a9348f8ddeb..ab03058d079 100644
--- a/client/go/cmd/query_test.go
+++ b/client/go/cmd/query_test.go
@@ -21,7 +21,7 @@ func TestQuery(t *testing.T) {
 
 func TestQueryVerbose(t *testing.T) {
 	client := &mock.HTTPClient{}
-	client.NextResponse(200, "{\"query\":\"result\"}")
+	client.NextResponseString(200, "{\"query\":\"result\"}")
 
 	cli, stdout, stderr := newTestCLI(t)
 	cli.httpClient = client
@@ -59,7 +59,7 @@ func TestServerError(t *testing.T) {
 
 func assertQuery(t *testing.T, expectedQuery string, query ...string) {
 	client := &mock.HTTPClient{}
-	client.NextResponse(200, "{\"query\":\"result\"}")
+	client.NextResponseString(200, "{\"query\":\"result\"}")
 	cli, stdout, _ := newTestCLI(t)
 	cli.httpClient = client
 
@@ -76,7 +76,7 @@ func assertQuery(t *testing.T, expectedQuery string, query ...string) {
 
 func assertQueryError(t *testing.T, status int, errorMessage string) {
 	client := &mock.HTTPClient{}
-	client.NextResponse(status, errorMessage)
+	client.NextResponseString(status, errorMessage)
 	cli, _, stderr := newTestCLI(t)
 	cli.httpClient = client
 	assert.NotNil(t, cli.Run("query", "yql=select from sources * where title contains 'foo'"))
@@ -88,7 +88,7 @@ func assertQueryError(t *testing.T, status int, errorMessage string) {
 
 func assertQueryServiceError(t *testing.T, status int, errorMessage string) {
 	client := &mock.HTTPClient{}
-	client.NextResponse(status, errorMessage)
+	client.NextResponseString(status, errorMessage)
 	cli, _, stderr := newTestCLI(t)
 	cli.httpClient = client
 	assert.NotNil(t, cli.Run("query", "yql=select from sources * where title contains 'foo'"))
diff --git a/client/go/cmd/root.go b/client/go/cmd/root.go
index 49cab75be53..f5cda8377dc 100644
--- a/client/go/cmd/root.go
+++ b/client/go/cmd/root.go
@@ -261,7 +261,7 @@ func (c *CLI) configureCommands() {
 	rootCmd.AddCommand(newVersionCmd(c))            // version
 }
 
-func (c *CLI) printErrHint(err error, hints ...string) {
+func (c *CLI) printErr(err error, hints ...string) {
 	fmt.Fprintln(c.Stderr, color.RedString("Error:"), err)
 	for _, hint := range hints {
 		fmt.Fprintln(c.Stderr, color.CyanString("Hint:"), hint)
@@ -269,10 +269,10 @@ func (c *CLI) printErrHint(err error, hints ...string) {
 }
 
 func (c *CLI) printSuccess(msg ...interface{}) {
-	log.Print(color.GreenString("Success: "), fmt.Sprint(msg...))
+	fmt.Fprintln(c.Stdout, color.GreenString("Success:"), fmt.Sprint(msg...))
 }
 
-func (c *CLI) printWarning(msg string, hints ...string) {
+func (c *CLI) printWarning(msg interface{}, hints ...string) {
 	fmt.Fprintln(c.Stderr, color.YellowString("Warning:"), msg)
 	for _, hint := range hints {
 		fmt.Fprintln(c.Stderr, color.CyanString("Hint:"), hint)
@@ -287,7 +287,7 @@ func (c *CLI) target(opts targetOptions) (vespa.Target, error) {
 	}
 	if !c.isCloudCI() { // Vespa Cloud always runs an up-to-date version
 		if err := target.CheckVersion(c.version); err != nil {
-			c.printWarning(err.Error(), "This version may not work as expected", "Try 'vespa version' to check for a new version")
+			c.printWarning(err, "This version may not work as expected", "Try 'vespa version' to check for a new version")
 		}
 	}
 	return target, nil
@@ -467,10 +467,10 @@ func (c *CLI) Run(args ...string) error {
 	if err != nil {
 		if cliErr, ok := err.(ErrCLI); ok {
 			if !cliErr.quiet {
-				c.printErrHint(cliErr, cliErr.hints...)
+				c.printErr(cliErr, cliErr.hints...)
 			}
 		} else {
-			c.printErrHint(err)
+			c.printErr(err)
 		}
 	}
 	return err
diff --git a/client/go/cmd/test_test.go b/client/go/cmd/test_test.go
index c3538c7cb1a..b6bc6a826d8 100644
--- a/client/go/cmd/test_test.go
+++ b/client/go/cmd/test_test.go
@@ -25,7 +25,7 @@ func TestSuite(t *testing.T) {
 	client.NextStatus(200)
 	client.NextStatus(200)
 	for i := 0; i < 11; i++ {
-		client.NextResponse(200, string(searchResponse))
+		client.NextResponseString(200, string(searchResponse))
 	}
 
 	expectedBytes, _ := ioutil.ReadFile("testdata/tests/expected-suite.out")
@@ -96,8 +96,8 @@ func TestSingleTest(t *testing.T) {
 	searchResponse, _ := ioutil.ReadFile("testdata/tests/response.json")
 	client.NextStatus(200)
 	client.NextStatus(200)
-	client.NextResponse(200, string(searchResponse))
-	client.NextResponse(200, string(searchResponse))
+	client.NextResponseString(200, string(searchResponse))
+	client.NextResponseString(200, string(searchResponse))
 	cli, stdout, stderr := newTestCLI(t)
 	cli.httpClient = client
 
@@ -136,8 +136,8 @@ func TestSingleTestWithCloudAndEndpoints(t *testing.T) {
 	require.Nil(t, err)
 	client.NextStatus(200)
 	client.NextStatus(200)
-	client.NextResponse(200, string(searchResponse))
-	client.NextResponse(200, string(searchResponse))
+	client.NextResponseString(200, string(searchResponse))
+	client.NextResponseString(200, string(searchResponse))
 
 	assert.Nil(t, cli.Run("test", "testdata/tests/system-test/test.json", "-t", "cloud", "-a", "t.a.i"))
 	expectedBytes, err := ioutil.ReadFile("testdata/tests/expected.out")
diff --git a/client/go/cmd/version_test.go b/client/go/cmd/version_test.go
index b78c66c9e7f..4354a2098c7 100644
--- a/client/go/cmd/version_test.go
+++ b/client/go/cmd/version_test.go
@@ -10,7 +10,7 @@ import (
 
 func TestVersion(t *testing.T) {
 	c := &mock.HTTPClient{}
-	c.NextResponse(200, `[{"tag_name": "v1.2.3", "published_at": "2021-09-10T12:00:00Z"}]`)
+	c.NextResponseString(200, `[{"tag_name": "v1.2.3", "published_at": "2021-09-10T12:00:00Z"}]`)
 
 	sp := &mock.Exec{}
 	cli, stdout, stderr := newTestCLI(t)
@@ -27,7 +27,7 @@ func TestVersion(t *testing.T) {
 
 func TestVersionCheckHomebrew(t *testing.T) {
 	c := &mock.HTTPClient{}
-	c.NextResponse(200, `[{"tag_name": "v1.2.3", "published_at": "2021-09-10T12:00:00Z"}]`)
+	c.NextResponseString(200, `[{"tag_name": "v1.2.3", "published_at": "2021-09-10T12:00:00Z"}]`)
 
 	sp := &mock.Exec{ProgramPath: "/usr/local/bin/vespa", CombinedOutput: "/usr/local"}
 	cli, stdout, stderr := newTestCLI(t)
diff --git a/client/go/mock/http.go b/client/go/mock/http.go
index f2fcf9c5960..770c6f7ebd9 100644
--- a/client/go/mock/http.go
+++ b/client/go/mock/http.go
@@ -11,7 +11,7 @@ import (
 
 type HTTPClient struct {
 	// The responses to return for future requests. Once a response is consumed, it's removed from this slice
-	nextResponses []httpResponse
+	nextResponses []HTTPResponse
 
 	// LastRequest is the last HTTP request made through this
 	LastRequest *http.Request
@@ -20,34 +20,42 @@ type HTTPClient struct {
 	Requests []*http.Request
 }
 
-type httpResponse struct {
-	status int
-	body   []byte
+type HTTPResponse struct {
+	Status int
+	Body   []byte
+	Header http.Header
 }
 
 func (c *HTTPClient) NextStatus(status int) { c.NextResponseBytes(status, nil) }
 
-func (c *HTTPClient) NextResponse(status int, body string) {
+func (c *HTTPClient) NextResponseString(status int, body string) {
 	c.NextResponseBytes(status, []byte(body))
 }
 
 func (c *HTTPClient) NextResponseBytes(status int, body []byte) {
-	c.nextResponses = append(c.nextResponses, httpResponse{status: status, body: body})
+	c.nextResponses = append(c.nextResponses, HTTPResponse{Status: status, Body: body})
+}
+
+func (c *HTTPClient) NextResponse(response HTTPResponse) {
+	c.nextResponses = append(c.nextResponses, response)
 }
 
 func (c *HTTPClient) Do(request *http.Request, timeout time.Duration) (*http.Response, error) {
-	response := httpResponse{status: 200}
+	response := HTTPResponse{Status: 200}
 	if len(c.nextResponses) > 0 {
 		response = c.nextResponses[0]
 		c.nextResponses = c.nextResponses[1:]
 	}
 	c.LastRequest = request
 	c.Requests = append(c.Requests, request)
+	if response.Header == nil {
+		response.Header = make(http.Header)
+	}
 	return &http.Response{
-			Status:     "Status " + strconv.Itoa(response.status),
-			StatusCode: response.status,
-			Body:       ioutil.NopCloser(bytes.NewBuffer(response.body)),
-			Header:     make(http.Header),
+			Status:     "Status " + strconv.Itoa(response.Status),
+			StatusCode: response.Status,
+			Body:       ioutil.NopCloser(bytes.NewBuffer(response.Body)),
+			Header:     response.Header,
 		},
 		nil
 }
-- 
cgit v1.2.3