Do not require certificate for vespa prod commands

8 files changed, 64 insertions, 53 deletions

diff --git a/client/go/cmd/curl.go b/client/go/cmd/curl.go
index 044318583a4..e06942196a6 100644
--- a/client/go/cmd/curl.go
+++ b/client/go/cmd/curl.go
@@ -34,7 +34,7 @@ $ vespa curl -- -v --data-urlencode "yql=select * from music where album contain
 		SilenceUsage:      true,
 		Args:              cobra.MinimumNArgs(1),
 		RunE: func(cmd *cobra.Command, args []string) error {
-			target, err := cli.target("", "")
+			target, err := cli.target(targetOptions{})
 			if err != nil {
 				return err
 			}
diff --git a/client/go/cmd/deploy.go b/client/go/cmd/deploy.go
index 2260e5aaaa1..cf2176b0c69 100644
--- a/client/go/cmd/deploy.go
+++ b/client/go/cmd/deploy.go
@@ -47,14 +47,11 @@ $ vespa deploy -t cloud -z perf.aws-us-east-1c`,
 			if err != nil {
 				return err
 			}
-			target, err := cli.target(zoneArg, logLevelArg)
-			if err != nil {
-				return err
-			}
-			opts, err := cli.createDeploymentOptions(pkg, target)
+			target, err := cli.target(targetOptions{zone: zoneArg, logLevel: logLevelArg})
 			if err != nil {
 				return err
 			}
+			opts := cli.createDeploymentOptions(pkg, target)
 
 			var result vespa.PrepareResult
 			err = util.Spinner(cli.Stderr, "Uploading application package ...", func() error {
@@ -100,14 +97,11 @@ func newPrepareCmd(cli *CLI) *cobra.Command {
 			if err != nil {
 				return fmt.Errorf("could not find application package: %w", err)
 			}
-			target, err := cli.target("", "")
-			if err != nil {
-				return err
-			}
-			opts, err := cli.createDeploymentOptions(pkg, target)
+			target, err := cli.target(targetOptions{})
 			if err != nil {
 				return err
 			}
+			opts := cli.createDeploymentOptions(pkg, target)
 			var result vespa.PrepareResult
 			err = util.Spinner(cli.Stderr, "Uploading application package ...", func() error {
 				result, err = vespa.Prepare(opts)
@@ -141,14 +135,11 @@ func newActivateCmd(cli *CLI) *cobra.Command {
 			if err != nil {
 				return fmt.Errorf("could not read session id: %w", err)
 			}
-			target, err := cli.target("", "")
-			if err != nil {
-				return err
-			}
-			opts, err := cli.createDeploymentOptions(pkg, target)
+			target, err := cli.target(targetOptions{})
 			if err != nil {
 				return err
 			}
+			opts := cli.createDeploymentOptions(pkg, target)
 			err = vespa.Activate(sessionID, opts)
 			if err != nil {
 				return err
diff --git a/client/go/cmd/log.go b/client/go/cmd/log.go
index 829f2c4c1f1..dffad8896fc 100644
--- a/client/go/cmd/log.go
+++ b/client/go/cmd/log.go
@@ -34,7 +34,7 @@ $ vespa log --follow`,
 		SilenceUsage:      true,
 		Args:              cobra.MaximumNArgs(1),
 		RunE: func(cmd *cobra.Command, args []string) error {
-			target, err := cli.target("", levelArg)
+			target, err := cli.target(targetOptions{logLevel: levelArg})
 			if err != nil {
 				return err
 			}
diff --git a/client/go/cmd/prod.go b/client/go/cmd/prod.go
index a8480af34e5..9108cb7a222 100644
--- a/client/go/cmd/prod.go
+++ b/client/go/cmd/prod.go
@@ -72,7 +72,7 @@ https://cloud.vespa.ai/en/reference/deployment`,
 			if err != nil {
 				return fmt.Errorf("a services.xml declaring your cluster(s) must exist: %w", err)
 			}
-			target, err := cli.target("", "")
+			target, err := cli.target(targetOptions{noCertificate: true})
 			if err != nil {
 				return err
 			}
@@ -128,7 +128,7 @@ https://cloud.vespa.ai/en/automated-deployments`,
 		Example: `$ mvn package # when adding custom Java components
 $ vespa prod submit`,
 		RunE: func(cmd *cobra.Command, args []string) error {
-			target, err := cli.target("", "")
+			target, err := cli.target(targetOptions{noCertificate: true})
 			if err != nil {
 				return err
 			}
@@ -155,10 +155,7 @@ $ vespa prod submit`,
 			if !cli.isCI() {
 				cli.printWarning("We recommend doing this only from a CD job", "See https://cloud.vespa.ai/en/getting-to-production")
 			}
-			opts, err := cli.createDeploymentOptions(pkg, target)
-			if err != nil {
-				return err
-			}
+			opts := cli.createDeploymentOptions(pkg, target)
 			if err := vespa.Submit(opts); err != nil {
 				return fmt.Errorf("could not submit application for deployment: %w", err)
 			} else {
diff --git a/client/go/cmd/prod_test.go b/client/go/cmd/prod_test.go
index 38dd1a773e3..06883dfcdcf 100644
--- a/client/go/cmd/prod_test.go
+++ b/client/go/cmd/prod_test.go
@@ -10,8 +10,10 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 	"github.com/vespa-engine/vespa/client/go/mock"
 	"github.com/vespa-engine/vespa/client/go/util"
+	"github.com/vespa-engine/vespa/client/go/vespa"
 )
 
 func TestProdInit(t *testing.T) {
@@ -154,11 +156,24 @@ func TestProdSubmit(t *testing.T) {
 
 	cli, stdout, _ := newTestCLI(t, "CI=true")
 	cli.httpClient = httpClient
-	assert.Nil(t, cli.Run("config", "set", "application", "t1.a1.i1"))
+	app := vespa.ApplicationID{Tenant: "t1", Application: "a1", Instance: "i1"}
+	assert.Nil(t, cli.Run("config", "set", "application", app.String()))
 	assert.Nil(t, cli.Run("config", "set", "target", "cloud"))
 	assert.Nil(t, cli.Run("auth", "api-key"))
 	assert.Nil(t, cli.Run("auth", "cert", pkgDir))
 
+	// Remove certificate as it's not required for submission (but it must be part of the application package)
+	if path, err := cli.config.privateKeyPath(app); err == nil {
+		os.RemoveAll(path)
+	} else {
+		require.Nil(t, err)
+	}
+	if path, err := cli.config.certificatePath(app); err == nil {
+		os.RemoveAll(path)
+	} else {
+		require.Nil(t, err)
+	}
+
 	// Zipping requires relative paths, so must let command run from pkgDir, then reset cwd for subsequent tests.
 	if cwd, err := os.Getwd(); err != nil {
 		t.Fatal(err)
diff --git a/client/go/cmd/root.go b/client/go/cmd/root.go
index d4e071ba244..2dbc0e89e0f 100644
--- a/client/go/cmd/root.go
+++ b/client/go/cmd/root.go
@@ -73,6 +73,15 @@ type ErrCLI struct {
 	error
 }
 
+type targetOptions struct {
+	// zone declares the zone use when using this target. If empty, a default zone for the system is chosen.
+	zone string
+	// logLevel sets the log level to use for this target. If empty, it defaults to "info".
+	logLevel string
+	// noCertificate declares that no client certificate should be required when using this target.
+	noCertificate bool
+}
+
 // errHint creates a new CLI error, with optional hints that will be printed after the error
 func errHint(err error, hints ...string) ErrCLI { return ErrCLI{Status: 1, hints: hints, error: err} }
 
@@ -255,10 +264,9 @@ func (c *CLI) printWarning(msg string, hints ...string) {
 	}
 }
 
-// target creates a target according the configuration of this CLI. If zone is empty, the default zone for the system is
-// used. If logLevel is empty, it defaults to "info".
-func (c *CLI) target(zone, logLevel string) (vespa.Target, error) {
-	target, err := c.createTarget(zone, logLevel)
+// target creates a target according the configuration of this CLI and given opts.
+func (c *CLI) target(opts targetOptions) (vespa.Target, error) {
+	target, err := c.createTarget(opts)
 	if err != nil {
 		return nil, err
 	}
@@ -270,7 +278,7 @@ func (c *CLI) target(zone, logLevel string) (vespa.Target, error) {
 	return target, nil
 }
 
-func (c *CLI) createTarget(zone, logLevel string) (vespa.Target, error) {
+func (c *CLI) createTarget(opts targetOptions) (vespa.Target, error) {
 	targetType, err := c.config.targetType()
 	if err != nil {
 		return nil, err
@@ -282,17 +290,17 @@ func (c *CLI) createTarget(zone, logLevel string) (vespa.Target, error) {
 	case vespa.TargetLocal:
 		return vespa.LocalTarget(c.httpClient), nil
 	case vespa.TargetCloud, vespa.TargetHosted:
-		return c.createCloudTarget(targetType, zone, logLevel)
+		return c.createCloudTarget(targetType, opts)
 	}
 	return nil, errHint(fmt.Errorf("invalid target: %s", targetType), "Valid targets are 'local', 'cloud', 'hosted' or an URL")
 }
 
-func (c *CLI) createCloudTarget(targetType, zone, logLevel string) (vespa.Target, error) {
+func (c *CLI) createCloudTarget(targetType string, opts targetOptions) (vespa.Target, error) {
 	system, err := c.system(targetType)
 	if err != nil {
 		return nil, err
 	}
-	deployment, err := c.config.deploymentIn(zone, system)
+	deployment, err := c.config.deploymentIn(opts.zone, system)
 	if err != nil {
 		return nil, err
 	}
@@ -315,14 +323,17 @@ func (c *CLI) createCloudTarget(targetType, zone, logLevel string) (vespa.Target
 			}
 		}
 		authConfigPath = c.config.authConfigPath()
-		kp, err := c.config.x509KeyPair(deployment.Application)
-		if err != nil {
-			return nil, errHint(err, "Deployment to cloud requires a certificate. Try 'vespa auth cert'")
-		}
-		deploymentTLSOptions = vespa.TLSOptions{
-			KeyPair:         kp.KeyPair,
-			CertificateFile: kp.CertificateFile,
-			PrivateKeyFile:  kp.PrivateKeyFile,
+		deploymentTLSOptions = vespa.TLSOptions{}
+		if !opts.noCertificate {
+			kp, err := c.config.x509KeyPair(deployment.Application)
+			if err != nil {
+				return nil, errHint(err, "Deployment to cloud requires a certificate. Try 'vespa auth cert'")
+			}
+			deploymentTLSOptions = vespa.TLSOptions{
+				KeyPair:         kp.KeyPair,
+				CertificateFile: kp.CertificateFile,
+				PrivateKeyFile:  kp.PrivateKeyFile,
+			}
 		}
 	case vespa.TargetHosted:
 		kp, err := athenzKeyPair()
@@ -349,6 +360,7 @@ func (c *CLI) createCloudTarget(targetType, zone, logLevel string) (vespa.Target
 		TLSOptions:  deploymentTLSOptions,
 		ClusterURLs: endpoints,
 	}
+	logLevel := opts.logLevel
 	if logLevel == "" {
 		logLevel = "info"
 	}
@@ -378,7 +390,7 @@ func (c *CLI) system(targetType string) (vespa.System, error) {
 // wait period configured in this CLI. The parameter sessionOrRunID specifies either the session ID (local target) or
 // run ID (cloud target) to wait for.
 func (c *CLI) service(name string, sessionOrRunID int64, cluster string) (*vespa.Service, error) {
-	t, err := c.target("", "")
+	t, err := c.target(targetOptions{})
 	if err != nil {
 		return nil, err
 	}
@@ -393,17 +405,13 @@ func (c *CLI) service(name string, sessionOrRunID int64, cluster string) (*vespa
 	return s, nil
 }
 
-func (c *CLI) createDeploymentOptions(pkg vespa.ApplicationPackage, target vespa.Target) (vespa.DeploymentOptions, error) {
-	opts := vespa.DeploymentOptions{ApplicationPackage: pkg, Target: target}
-	if opts.IsCloud() {
-		if target.Type() == vespa.TargetCloud && !opts.ApplicationPackage.HasCertificate() {
-			hint := "Try 'vespa auth cert'"
-			return vespa.DeploymentOptions{}, errHint(fmt.Errorf("missing certificate in application package"), "Applications in Vespa Cloud require a certificate", hint)
-		}
+func (c *CLI) createDeploymentOptions(pkg vespa.ApplicationPackage, target vespa.Target) vespa.DeploymentOptions {
+	return vespa.DeploymentOptions{
+		ApplicationPackage: pkg,
+		Target:             target,
+		Timeout:            time.Duration(c.flags.waitSecs) * time.Second,
+		HTTPClient:         c.httpClient,
 	}
-	opts.Timeout = time.Duration(c.flags.waitSecs) * time.Second
-	opts.HTTPClient = c.httpClient
-	return opts, nil
 }
 
 // isCI returns true if running inside a continuous integration environment.
diff --git a/client/go/cmd/status.go b/client/go/cmd/status.go
index 2796999957d..1eacff56354 100644
--- a/client/go/cmd/status.go
+++ b/client/go/cmd/status.go
@@ -72,7 +72,7 @@ func newStatusDeployCmd(cli *CLI) *cobra.Command {
 }
 
 func printServiceStatus(cli *CLI, name string) error {
-	t, err := cli.target("", "")
+	t, err := cli.target(targetOptions{})
 	if err != nil {
 		return err
 	}
diff --git a/client/go/cmd/test.go b/client/go/cmd/test.go
index 2eb5d6fc9b5..69dc61e7e0e 100644
--- a/client/go/cmd/test.go
+++ b/client/go/cmd/test.go
@@ -479,7 +479,7 @@ type testContext struct {
 
 func (t *testContext) target() (vespa.Target, error) {
 	if t.lazyTarget == nil {
-		target, err := t.cli.target(t.zone, "")
+		target, err := t.cli.target(targetOptions{zone: t.zone})
 		if err != nil {
 			return nil, err
 		}