Merge pull request #19290 from vespa-engine/mpolden/sample-apps-cache

Cache sample apps

10 files changed, 126 insertions, 69 deletions

diff --git a/client/go/cmd/api_key_test.go b/client/go/cmd/api_key_test.go
index 2497568604f..1deb628c21e 100644
--- a/client/go/cmd/api_key_test.go
+++ b/client/go/cmd/api_key_test.go
@@ -4,20 +4,20 @@
 package cmd
 
 import (
-	"strings"
+	"path/filepath"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
 )
 
 func TestAPIKey(t *testing.T) {
-	homeDir := t.TempDir()
-	keyFile := homeDir + "/.vespa/t1.api-key.pem"
+	homeDir := filepath.Join(t.TempDir(), ".vespa")
+	keyFile := filepath.Join(homeDir, "t1.api-key.pem")
 
 	out, _ := execute(command{args: []string{"api-key", "-a", "t1.a1.i1"}, homeDir: homeDir}, t, nil)
-	assert.True(t, strings.HasPrefix(out, "Success: API private key written to "+keyFile+"\n"))
+	assert.Contains(t, out, "Success: API private key written to "+keyFile+"\n")
 
 	out, _ = execute(command{args: []string{"api-key", "-a", "t1.a1.i1"}, homeDir: homeDir}, t, nil)
-	assert.True(t, strings.HasPrefix(out, "Error: File "+keyFile+" already exists\nHint: Use -f to overwrite it\n"))
-	assert.True(t, strings.Contains(out, "This is your public key"))
+	assert.Contains(t, out, "Error: File "+keyFile+" already exists\nHint: Use -f to overwrite it\n")
+	assert.Contains(t, out, "This is your public key")
 }
diff --git a/client/go/cmd/cert_test.go b/client/go/cmd/cert_test.go
index d93def2fa70..cd5f88764b9 100644
--- a/client/go/cmd/cert_test.go
+++ b/client/go/cmd/cert_test.go
@@ -14,7 +14,7 @@ import (
 )
 
 func TestCert(t *testing.T) {
-	homeDir := t.TempDir()
+	homeDir := filepath.Join(t.TempDir(), ".vespa")
 	pkgDir := mockApplicationPackage(t, false)
 	out, _ := execute(command{args: []string{"cert", "-a", "t1.a1.i1", pkgDir}, homeDir: homeDir}, t, nil)
 
@@ -23,8 +23,8 @@ func TestCert(t *testing.T) {
 
 	appDir := filepath.Join(pkgDir, "src", "main", "application")
 	pkgCertificate := filepath.Join(appDir, "security", "clients.pem")
-	certificate := filepath.Join(homeDir, ".vespa", app.String(), "data-plane-public-cert.pem")
-	privateKey := filepath.Join(homeDir, ".vespa", app.String(), "data-plane-private-key.pem")
+	certificate := filepath.Join(homeDir, app.String(), "data-plane-public-cert.pem")
+	privateKey := filepath.Join(homeDir, app.String(), "data-plane-private-key.pem")
 
 	assert.Equal(t, fmt.Sprintf("Success: Certificate written to %s\nSuccess: Certificate written to %s\nSuccess: Private key written to %s\n", pkgCertificate, certificate, privateKey), out)
 
@@ -33,7 +33,7 @@ func TestCert(t *testing.T) {
 }
 
 func TestCertCompressedPackage(t *testing.T) {
-	homeDir := t.TempDir()
+	homeDir := filepath.Join(t.TempDir(), ".vespa")
 	pkgDir := mockApplicationPackage(t, true)
 	zipFile := filepath.Join(pkgDir, "target", "application.zip")
 	err := os.MkdirAll(filepath.Dir(zipFile), 0755)
diff --git a/client/go/cmd/clone.go b/client/go/cmd/clone.go
index 9503a81debf..508ad49438f 100644
--- a/client/go/cmd/clone.go
+++ b/client/go/cmd/clone.go
@@ -6,11 +6,10 @@ package cmd
 
 import (
 	"archive/zip"
+	"errors"
 	"io"
-	"io/ioutil"
 	"log"
 	"net/http"
-	"net/url"
 	"os"
 	"path/filepath"
 	"strings"
@@ -20,22 +19,29 @@ import (
 	"github.com/vespa-engine/vespa/client/go/util"
 )
 
-// Set this to test without downloading this file from github
-var existingSampleAppsZip string
+const sampleAppsCacheTTL = time.Hour * 168 // 1 week
+
 var listApps bool
+var forceClone bool
 
 func init() {
 	rootCmd.AddCommand(cloneCmd)
 	cloneCmd.Flags().BoolVarP(&listApps, "list", "l", false, "List available sample applications")
+	cloneCmd.Flags().BoolVarP(&forceClone, "force", "f", false, "Ignore cache and force downloading the latest sample application from GitHub")
 }
 
 var cloneCmd = &cobra.Command{
-	// TODO: "application" and "list" subcommands?
 	Use:   "clone sample-application-path target-directory",
 	Short: "Create files and directory structure for a new Vespa application from a sample application",
-	Long: `Creates an application package file structure.
+	Long: `Create files and directory structure for a new Vespa application
+from a sample application.
+
+Sample applications are downloaded from
+https://github.com/vespa-engine/sample-apps.
 
-The application package is copied from a sample application in https://github.com/vespa-engine/sample-apps`,
+By default sample applications are cached in the user's cache directory. This
+directory can be overriden by setting the VESPA_CLI_CACHE_DIR environment
+variable.`,
 	Example:           "$ vespa clone vespa-cloud/album-recommendation my-app",
 	DisableAutoGenTag: true,
 	Run: func(cmd *cobra.Command, args []string) {
@@ -60,12 +66,7 @@ The application package is copied from a sample application in https://github.co
 
 func cloneApplication(source string, name string) {
 	zipFile := getSampleAppsZip()
-	if zipFile == nil {
-		return
-	}
-	if existingSampleAppsZip == "" { // Indicates we created a temp file now
-		defer os.Remove(zipFile.Name())
-	}
+	defer zipFile.Close()
 
 	zipReader, zipOpenError := zip.OpenReader(zipFile.Name())
 	if zipOpenError != nil {
@@ -101,45 +102,67 @@ func cloneApplication(source string, name string) {
 	}
 }
 
+func openOutputFile() (*os.File, error) {
+	cacheDir, err := vespaCliCacheDir()
+	if err != nil {
+		return nil, err
+	}
+	cacheFile := filepath.Join(cacheDir, "sample-apps-master.zip")
+	return os.OpenFile(cacheFile, os.O_RDWR|os.O_CREATE, 0755)
+}
+
+func useCache(cacheFile *os.File) (bool, error) {
+	if forceClone {
+		return false, nil
+	}
+	stat, err := cacheFile.Stat()
+	if errors.Is(err, os.ErrNotExist) {
+		return false, nil
+	} else if err != nil {
+		return false, err
+	}
+	expiry := stat.ModTime().Add(sampleAppsCacheTTL)
+	return stat.Size() > 0 && time.Now().Before(expiry), nil
+}
+
 func getSampleAppsZip() *os.File {
-	if existingSampleAppsZip != "" {
-		existing, openExistingError := os.Open(existingSampleAppsZip)
-		if openExistingError != nil {
-			printErr(openExistingError, "Could not open existing sample apps zip file '", color.Cyan(existingSampleAppsZip), "'")
-		}
-		return existing
+	f, err := openOutputFile()
+	if err != nil {
+		fatalErr(err, "Could not determine location of cache file")
+		return nil
+	}
+	useCache, err := useCache(f)
+	if err != nil {
+		fatalErr(err, "Could not determine cache status", "Try ignoring the cache with the -f flag")
+		return nil
+	}
+	if useCache {
+		log.Print(color.Yellow("Using cached sample apps ..."))
+		return f
 	}
 
-	// TODO: Cache it?
 	log.Print(color.Yellow("Downloading sample apps ...")) // TODO: Spawn thread to indicate progress
-	zipUrl, _ := url.Parse("https://github.com/vespa-engine/sample-apps/archive/refs/heads/master.zip")
-	request := &http.Request{
-		URL:    zipUrl,
-		Method: "GET",
+	request, err := http.NewRequest("GET", "https://github.com/vespa-engine/sample-apps/archive/refs/heads/master.zip", nil)
+	if err != nil {
+		fatalErr(err, "Invalid URL")
+		return nil
 	}
-	response, reqErr := util.HttpDo(request, time.Minute*60, "GitHub")
-	if reqErr != nil {
-		printErr(reqErr, "Could not download sample apps from GitHub")
+	response, err := util.HttpDo(request, time.Minute*60, "GitHub")
+	if err != nil {
+		fatalErr(err, "Could not download sample apps from GitHub")
 		return nil
 	}
 	defer response.Body.Close()
 	if response.StatusCode != 200 {
-		printErr(nil, "Could not download sample apps from GitHub: ", response.StatusCode)
+		fatalErr(nil, "Could not download sample apps from GitHub: ", response.StatusCode)
 		return nil
 	}
 
-	destination, tempFileError := ioutil.TempFile("", "prefix")
-	if tempFileError != nil {
-		printErr(tempFileError, "Could not create a temporary file to hold sample apps")
-	}
-	// destination, _ := os.Create("./" + name + "/sample-apps.zip")
-	// defer destination.Close()
-	_, err := io.Copy(destination, response.Body)
-	if err != nil {
-		printErr(err, "Could not download sample apps from GitHub")
+	if _, err := io.Copy(f, response.Body); err != nil {
+		fatalErr(err, "Could not write sample apps to file: ", f.Name())
 		return nil
 	}
-	return destination
+	return f
 }
 
 func copy(f *zip.File, destinationDir string, zipEntryPrefix string) error {
diff --git a/client/go/cmd/clone_test.go b/client/go/cmd/clone_test.go
index 5027c5bf972..6cf11dd4d40 100644
--- a/client/go/cmd/clone_test.go
+++ b/client/go/cmd/clone_test.go
@@ -8,6 +8,7 @@ import (
 	"os"
 	"path/filepath"
 	"testing"
+	"time"
 
 	"github.com/stretchr/testify/assert"
 	"github.com/vespa-engine/vespa/client/go/util"
@@ -18,10 +19,14 @@ func TestClone(t *testing.T) {
 }
 
 func assertCreated(sampleAppName string, app string, t *testing.T) {
-	existingSampleAppsZip = "testdata/sample-apps-master.zip"
-	standardOut := executeCommand(t, &mockHttpClient{}, []string{"clone", sampleAppName, app}, []string{})
+	testFile := filepath.Join("testdata", "sample-apps-master.zip")
+	now := time.Now()
+	if err := os.Chtimes(testFile, now, now); err != nil { // Ensure test file is considered new enough by cache mechanism
+		t.Fatal(err)
+	}
+	out, _ := execute(command{cacheDir: filepath.Dir(testFile), args: []string{"clone", sampleAppName, app}}, t, nil)
 	defer os.RemoveAll(app)
-	assert.Equal(t, "Created "+app+"\n", standardOut)
+	assert.Equal(t, "Using cached sample apps ...\nCreated "+app+"\n", out)
 	assert.True(t, util.PathExists(filepath.Join(app, "README.md")))
 	assert.True(t, util.PathExists(filepath.Join(app, "src", "main", "application")))
 	assert.True(t, util.IsDirectory(filepath.Join(app, "src", "main", "application")))
diff --git a/client/go/cmd/command_tester.go b/client/go/cmd/command_tester.go
index f455ffa9957..71f821521df 100644
--- a/client/go/cmd/command_tester.go
+++ b/client/go/cmd/command_tester.go
@@ -22,6 +22,7 @@ import (
 
 type command struct {
 	homeDir  string
+	cacheDir string
 	args     []string
 	moreArgs []string
 }
@@ -31,12 +32,16 @@ func execute(cmd command, t *testing.T, client *mockHttpClient) (string, string)
 		util.ActiveHttpClient = client
 	}
 
-	// Set config dir. Use a separate one per test if none is specified
+	// Set Vespa CLI directories. Use a separate one per test if none is specified
 	if cmd.homeDir == "" {
-		cmd.homeDir = t.TempDir()
+		cmd.homeDir = filepath.Join(t.TempDir(), ".vespa")
 		viper.Reset()
 	}
-	os.Setenv("VESPA_CLI_HOME", filepath.Join(cmd.homeDir, ".vespa"))
+	if cmd.cacheDir == "" {
+		cmd.cacheDir = filepath.Join(t.TempDir(), ".cache", "vespa")
+	}
+	os.Setenv("VESPA_CLI_HOME", cmd.homeDir)
+	os.Setenv("VESPA_CLI_CACHE_DIR", cmd.cacheDir)
 
 	// Reset flags to their default value - persistent flags in Cobra persists over tests
 	rootCmd.Flags().VisitAll(func(f *pflag.Flag) {
diff --git a/client/go/cmd/config.go b/client/go/cmd/config.go
index c88225338c7..863f247bd7c 100644
--- a/client/go/cmd/config.go
+++ b/client/go/cmd/config.go
@@ -108,16 +108,8 @@ type Config struct {
 }
 
 func LoadConfig() (*Config, error) {
-	home := os.Getenv("VESPA_CLI_HOME")
-	if home == "" {
-		var err error
-		home, err = os.UserHomeDir()
-		if err != nil {
-			return nil, err
-		}
-		home = filepath.Join(home, ".vespa")
-	}
-	if err := os.MkdirAll(home, 0700); err != nil {
+	home, err := vespaCliHome()
+	if err != nil {
 		return nil, err
 	}
 	c := &Config{Home: home, createDirs: true}
diff --git a/client/go/cmd/config_test.go b/client/go/cmd/config_test.go
index cf50f561f0f..25ba7cc0655 100644
--- a/client/go/cmd/config_test.go
+++ b/client/go/cmd/config_test.go
@@ -1,13 +1,14 @@
 package cmd
 
 import (
+	"path/filepath"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
 )
 
 func TestConfig(t *testing.T) {
-	homeDir := t.TempDir()
+	homeDir := filepath.Join(t.TempDir(), ".vespa")
 	assertConfigCommand(t, "invalid option or value: \"foo\": \"bar\"\n", homeDir, "config", "set", "foo", "bar")
 	assertConfigCommand(t, "foo = <unset>\n", homeDir, "config", "get", "foo")
 	assertConfigCommand(t, "target = local\n", homeDir, "config", "get", "target")
diff --git a/client/go/cmd/curl_test.go b/client/go/cmd/curl_test.go
index 340eacd0bd3..e67ef560c41 100644
--- a/client/go/cmd/curl_test.go
+++ b/client/go/cmd/curl_test.go
@@ -10,13 +10,13 @@ import (
 )
 
 func TestCurl(t *testing.T) {
-	homeDir := t.TempDir()
+	homeDir := filepath.Join(t.TempDir(), ".vespa")
 	httpClient := &mockHttpClient{}
 	convergeServices(httpClient)
 	out, _ := execute(command{homeDir: homeDir, args: []string{"curl", "-n", "-a", "t1.a1.i1", "--", "-v", "--data-urlencode", "arg=with space", "/search"}}, t, httpClient)
 
 	expected := fmt.Sprintf("curl --key %s --cert %s -v --data-urlencode 'arg=with space' https://127.0.0.1:8080/search\n",
-		filepath.Join(homeDir, ".vespa", "t1.a1.i1", "data-plane-private-key.pem"),
-		filepath.Join(homeDir, ".vespa", "t1.a1.i1", "data-plane-public-cert.pem"))
+		filepath.Join(homeDir, "t1.a1.i1", "data-plane-private-key.pem"),
+		filepath.Join(homeDir, "t1.a1.i1", "data-plane-public-cert.pem"))
 	assert.Equal(t, expected, out)
 }
diff --git a/client/go/cmd/deploy_test.go b/client/go/cmd/deploy_test.go
index 443f7e8846f..9614806b968 100644
--- a/client/go/cmd/deploy_test.go
+++ b/client/go/cmd/deploy_test.go
@@ -130,7 +130,7 @@ func assertActivate(applicationPackage string, arguments []string, t *testing.T)
 	if err := cfg.WriteSessionID(vespa.DefaultApplication, 42); err != nil {
 		t.Fatal(err)
 	}
-	out, _ := execute(command{args: arguments, homeDir: homeDir}, t, client)
+	out, _ := execute(command{args: arguments, homeDir: cfg.Home}, t, client)
 	assert.Equal(t,
 		"Success: Activated "+applicationPackage+" with session 42\n",
 		out)
diff --git a/client/go/cmd/helpers.go b/client/go/cmd/helpers.go
index 6cd7aa38936..09eef495018 100644
--- a/client/go/cmd/helpers.go
+++ b/client/go/cmd/helpers.go
@@ -10,6 +10,7 @@ import (
 	"io/ioutil"
 	"log"
 	"os"
+	"path/filepath"
 	"strings"
 	"time"
 
@@ -48,6 +49,36 @@ func printSuccess(msg ...interface{}) {
 	log.Print(color.Green("Success: "), fmt.Sprint(msg...))
 }
 
+func vespaCliHome() (string, error) {
+	home := os.Getenv("VESPA_CLI_HOME")
+	if home == "" {
+		userHome, err := os.UserHomeDir()
+		if err != nil {
+			return "", err
+		}
+		home = filepath.Join(userHome, ".vespa")
+	}
+	if err := os.MkdirAll(home, 0700); err != nil {
+		return "", err
+	}
+	return home, nil
+}
+
+func vespaCliCacheDir() (string, error) {
+	cacheDir := os.Getenv("VESPA_CLI_CACHE_DIR")
+	if cacheDir == "" {
+		userCacheDir, err := os.UserCacheDir()
+		if err != nil {
+			return "", err
+		}
+		cacheDir = filepath.Join(userCacheDir, "vespa")
+	}
+	if err := os.MkdirAll(cacheDir, 0755); err != nil {
+		return "", err
+	}
+	return cacheDir, nil
+}
+
 func deploymentFromArgs() vespa.Deployment {
 	zone, err := vespa.ZoneFromString(zoneArg)
 	if err != nil {