From f2d7c0af424036482817cf6efc9407d200cac1ed Mon Sep 17 00:00:00 2001
From: Harald Musum <musum@verizonmedia.com>
Date: Tue, 15 Mar 2022 08:02:57 +0100
Subject: 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, 117 insertions(+), 257 deletions(-)

(limited to 'client')

diff --git a/client/go/auth/zts/zts_test.go b/client/go/auth/zts/zts_test.go
index 504f29e9808..0eec085aadb 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.NextResponseString(400, `{"message": "bad request"}`)
+	httpClient.NextResponse(400, `{"message": "bad request"}`)
 	_, err = client.AccessToken("vespa.vespa", tls.Certificate{})
 	if err == nil {
 		t.Fatal("want error for non-ok response status")
 	}
-	httpClient.NextResponseString(200, `{"access_token": "foo bar"}`)
+	httpClient.NextResponse(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 1050b040aae..cacfe3fd180 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.printErr(err, "Use -f to overwrite it")
+		cli.printErrHint(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 03a55d839d6..0829e064af9 100644
--- a/client/go/cmd/clone.go
+++ b/client/go/cmd/clone.go
@@ -6,13 +6,14 @@ package cmd
 
 import (
 	"archive/zip"
+	"errors"
 	"fmt"
 	"io"
+	"io/ioutil"
 	"log"
 	"net/http"
 	"os"
 	"path/filepath"
-	"sort"
 	"strings"
 	"time"
 
@@ -20,12 +21,10 @@ import (
 	"github.com/spf13/cobra"
 )
 
-const sampleAppsNamePrefix = "sample-apps-master"
-
 func newCloneCmd(cli *CLI) *cobra.Command {
 	var (
-		listApps bool
-		noCache  bool
+		listApps   bool
+		forceClone bool
 	)
 	cmd := &cobra.Command{
 		Use:   "clone sample-application-path target-directory",
@@ -56,37 +55,24 @@ variable.`,
 			if len(args) != 2 {
 				return fmt.Errorf("expected exactly 2 arguments, got %d", len(args))
 			}
-			cloner := &cloner{cli: cli, noCache: noCache}
-			return cloner.Clone(args[0], args[1])
+			return cloneApplication(cli, args[0], args[1], forceClone)
 		},
 	}
 	cmd.Flags().BoolVarP(&listApps, "list", "l", false, "List available sample applications")
-	cmd.Flags().BoolVarP(&noCache, "force", "f", false, "Ignore cache and force downloading the latest sample application from GitHub")
+	cmd.Flags().BoolVarP(&forceClone, "force", "f", false, "Ignore cache and force downloading the latest sample application from GitHub")
 	return cmd
 }
 
-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()
+func cloneApplication(cli *CLI, applicationName string, applicationDir string, force bool) error {
+	zipFile, err := openSampleAppsZip(force, cli)
 	if err != nil {
 		return err
 	}
+	defer zipFile.Close()
 
-	r, err := zip.OpenReader(zipPath)
+	r, err := zip.OpenReader(zipFile.Name())
 	if err != nil {
-		return fmt.Errorf("could not open sample apps zip '%s': %w", color.CyanString(zipPath), err)
+		return fmt.Errorf("could not open sample apps zip '%s': %w", color.CyanString(zipFile.Name()), err)
 	}
 	defer r.Close()
 
@@ -95,168 +81,91 @@ func (c *cloner) Clone(applicationName, path string) error {
 		dirPrefix := "sample-apps-master/" + applicationName + "/"
 		if strings.HasPrefix(f.Name, dirPrefix) {
 			if !found { // Create destination directory lazily when source is found
-				createErr := os.Mkdir(path, 0755)
+				createErr := os.Mkdir(applicationDir, 0755)
 				if createErr != nil {
-					return fmt.Errorf("could not create directory '%s': %w", color.CyanString(path), createErr)
+					return fmt.Errorf("could not create directory '%s': %w", color.CyanString(applicationDir), createErr)
 				}
 			}
 			found = true
 
-			if err := copy(f, path, dirPrefix); err != nil {
+			if err := copy(f, applicationDir, 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(path))
+		log.Print("Created ", color.CyanString(applicationDir))
 	}
 	return 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
+func useCache(force bool, stat os.FileInfo) (bool, error) {
+	if force {
+		return false, 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
+	expiry := stat.ModTime().Add(time.Hour * 168) // 1 week
+	return stat.Size() > 0 && time.Now().Before(expiry), nil
 }
 
-// listZipFiles list all sample apps ZIP files found in cacheDir.
-func (c *cloner) listZipFiles() ([]zipFile, error) {
-	dirEntries, err := os.ReadDir(c.cli.config.cacheDir)
+func fetchSampleAppsZip(destination string, cli *CLI) error {
+	f, err := ioutil.TempFile(filepath.Dir(destination), "sample-apps")
 	if err != nil {
-		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
+		return fmt.Errorf("could not create temporary file: %w", err)
 	}
-	// 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 {
+	defer f.Close()
+	return cli.spinner(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)
 		}
-		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)
+		response, err := 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)
 		}
-		etag = trimEntityTagID(response.Header.Get("etag"))
-		newPath, err := c.writeZip(response.Body, etag)
-		if err != nil {
-			return err
+		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")
 		}
-		zipPath = newPath
-		cacheHit = false
 		return nil
 	})
-	return zipPath, cacheHit, err
 }
 
-// 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-")
+func openSampleAppsZip(forceClone bool, cli *CLI) (*os.File, error) {
+	cacheDir, err := vespaCliCacheDir(cli.Environment)
 	if err != nil {
-		return "", fmt.Errorf("could not create temporary file: %w", err)
+		return nil, err
 	}
-	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)
+	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)
 	}
-	f.Close()
-	path := filepath.Join(c.cli.config.cacheDir, sampleAppsNamePrefix)
-	if etag != "" {
-		path += "_" + etag
+	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)
+		}
 	}
-	path += ".zip"
-	if err := os.Rename(f.Name(), path); err != nil {
-		return "", fmt.Errorf("could not move sample apps to %s", path)
+	if err := fetchSampleAppsZip(path, cli); err != nil {
+		return nil, fmt.Errorf("could not fetch sample apps: %w", err)
 	}
-	cleanTemp = false
-	return path, nil
-}
-
-func trimEntityTagID(s string) string {
-	return strings.TrimSuffix(strings.TrimPrefix(s, `W/"`), `"`)
+	return os.Open(path)
 }
 
 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 2ed2449af7d..73aa70f12f2 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.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"))
+	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"))
 
 	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 1971e0032a4..9587a1435d3 100644
--- a/client/go/cmd/clone_test.go
+++ b/client/go/cmd/clone_test.go
@@ -5,10 +5,9 @@
 package cmd
 
 import (
-	"net/http"
+	"io/ioutil"
 	"os"
 	"path/filepath"
-	"strings"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -22,60 +21,28 @@ func TestClone(t *testing.T) {
 }
 
 func assertCreated(sampleAppName string, app string, t *testing.T) {
-	tempDir := t.TempDir()
-	app1 := filepath.Join(tempDir, "app1")
+	appCached := app + "-cache"
 	defer os.RemoveAll(app)
+	defer os.RemoveAll(appCached)
 
 	httpClient := &mock.HTTPClient{}
-	cli, stdout, stderr := newTestCLI(t)
-	cli.httpClient = httpClient
-	testdata, err := os.ReadFile(filepath.Join("testdata", "sample-apps-master.zip"))
+	testdata, err := ioutil.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)
 
-	// 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)
+	cli, stdout, _ := newTestCLI(t)
+	cli.httpClient = httpClient
+	err = cli.Run("clone", sampleAppName, app)
+	assert.Nil(t, err)
 
-	// 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)
+	assert.Equal(t, "Created "+app+"\n", stdout.String())
+	assertFiles(t, app)
 
-	// Cloning falls back to cached copy if GitHub is unavailable
-	httpClient.NextStatus(500)
 	stdout.Reset()
-	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)
+	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)
 }
 
 func assertFiles(t *testing.T, app string) {
@@ -83,12 +50,10 @@ 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, err := os.Stat(filepath.Join(app, "src", "main", "application", "services.xml"))
-	require.Nil(t, err)
+	servicesStat, _ := os.Stat(filepath.Join(app, "src", "main", "application", "services.xml"))
 	servicesSize := int64(1772)
 	assert.Equal(t, servicesSize, servicesStat.Size())
 
-	scriptStat, err := os.Stat(filepath.Join(app, "bin", "convert-msmarco.sh"))
-	require.Nil(t, err)
+	scriptStat, _ := os.Stat(filepath.Join(app, "bin", "convert-msmarco.sh"))
 	assert.Equal(t, os.FileMode(0755), scriptStat.Mode())
 }
diff --git a/client/go/cmd/config.go b/client/go/cmd/config.go
index b75ae046534..75bd9959280 100644
--- a/client/go/cmd/config.go
+++ b/client/go/cmd/config.go
@@ -95,7 +95,6 @@ $ vespa config get target`,
 
 type Config struct {
 	homeDir     string
-	cacheDir    string
 	environment map[string]string
 	bindings    ConfigBindings
 	createDirs  bool
@@ -132,13 +131,8 @@ 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 c46322953c0..e2d6da5399d 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.NextResponseString(200, `{"session-id":"42"}`)
+	client.NextResponse(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.NextResponseString(status, returnBody)
+	client.NextResponse(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.NextResponseString(status, errorMessage)
+	client.NextResponse(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 b6f3cbcf6ca..77b7d68d666 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.NextResponseString(200, "{\"fields\":{\"foo\":\"bar\"}}")
+	client.NextResponse(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.NextResponseString(status, errorMessage)
+	client.NextResponse(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.NextResponseString(status, errorMessage)
+	client.NextResponse(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 d3e7b630869..fb08474dd4d 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.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`)
+	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`)
 	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.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`)
+	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`)
 	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 6764d4a85f2..9108cb7a222 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.printErr(err)
+			cli.printErrHint(err)
 			fmt.Fprintln(cli.Stderr)
 			input = ""
 		}
diff --git a/client/go/cmd/prod_test.go b/client/go/cmd/prod_test.go
index 0441d334fa1..06883dfcdcf 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.NextResponseString(200, `ok`)
+	httpClient.NextResponse(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.NextResponseString(200, `ok`)
+	httpClient.NextResponse(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 ab03058d079..a9348f8ddeb 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.NextResponseString(200, "{\"query\":\"result\"}")
+	client.NextResponse(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.NextResponseString(200, "{\"query\":\"result\"}")
+	client.NextResponse(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.NextResponseString(status, errorMessage)
+	client.NextResponse(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.NextResponseString(status, errorMessage)
+	client.NextResponse(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 f5cda8377dc..49cab75be53 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) printErr(err error, hints ...string) {
+func (c *CLI) printErrHint(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) printErr(err error, hints ...string) {
 }
 
 func (c *CLI) printSuccess(msg ...interface{}) {
-	fmt.Fprintln(c.Stdout, color.GreenString("Success:"), fmt.Sprint(msg...))
+	log.Print(color.GreenString("Success: "), fmt.Sprint(msg...))
 }
 
-func (c *CLI) printWarning(msg interface{}, hints ...string) {
+func (c *CLI) printWarning(msg string, 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, "This version may not work as expected", "Try 'vespa version' to check for a new version")
+			c.printWarning(err.Error(), "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.printErr(cliErr, cliErr.hints...)
+				c.printErrHint(cliErr, cliErr.hints...)
 			}
 		} else {
-			c.printErr(err)
+			c.printErrHint(err)
 		}
 	}
 	return err
diff --git a/client/go/cmd/test_test.go b/client/go/cmd/test_test.go
index b6bc6a826d8..c3538c7cb1a 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.NextResponseString(200, string(searchResponse))
+		client.NextResponse(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.NextResponseString(200, string(searchResponse))
-	client.NextResponseString(200, string(searchResponse))
+	client.NextResponse(200, string(searchResponse))
+	client.NextResponse(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.NextResponseString(200, string(searchResponse))
-	client.NextResponseString(200, string(searchResponse))
+	client.NextResponse(200, string(searchResponse))
+	client.NextResponse(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 4354a2098c7..b78c66c9e7f 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.NextResponseString(200, `[{"tag_name": "v1.2.3", "published_at": "2021-09-10T12:00:00Z"}]`)
+	c.NextResponse(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.NextResponseString(200, `[{"tag_name": "v1.2.3", "published_at": "2021-09-10T12:00:00Z"}]`)
+	c.NextResponse(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 770c6f7ebd9..f2fcf9c5960 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,42 +20,34 @@ type HTTPClient struct {
 	Requests []*http.Request
 }
 
-type HTTPResponse struct {
-	Status int
-	Body   []byte
-	Header http.Header
+type httpResponse struct {
+	status int
+	body   []byte
 }
 
 func (c *HTTPClient) NextStatus(status int) { c.NextResponseBytes(status, nil) }
 
-func (c *HTTPClient) NextResponseString(status int, body string) {
+func (c *HTTPClient) NextResponse(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})
-}
-
-func (c *HTTPClient) NextResponse(response HTTPResponse) {
-	c.nextResponses = append(c.nextResponses, response)
+	c.nextResponses = append(c.nextResponses, httpResponse{status: status, body: body})
 }
 
 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:     response.Header,
+			Status:     "Status " + strconv.Itoa(response.status),
+			StatusCode: response.status,
+			Body:       ioutil.NopCloser(bytes.NewBuffer(response.body)),
+			Header:     make(http.Header),
 		},
 		nil
 }
-- 
cgit v1.2.3