Merge pull request #22158 from vespa-engine/revert-22138-mpolden/ci-api-key-filev7.575.35

Revert "Allow reading API key from default path when Auth0 is not configured"

3 files changed, 34 insertions, 37 deletions

diff --git a/client/go/cmd/config.go b/client/go/cmd/config.go
index 02477aecf28..726676ce476 100644
--- a/client/go/cmd/config.go
+++ b/client/go/cmd/config.go
@@ -20,6 +20,7 @@ import (
 	"github.com/spf13/pflag"
 	"github.com/vespa-engine/vespa/client/go/auth/auth0"
 	"github.com/vespa-engine/vespa/client/go/config"
+	"github.com/vespa-engine/vespa/client/go/util"
 	"github.com/vespa-engine/vespa/client/go/vespa"
 )
 
@@ -432,21 +433,30 @@ func (c *Config) authConfigPath() string {
 	return filepath.Join(c.homeDir, "auth.json")
 }
 
-func (c *Config) readAPIKey(cli *CLI, system vespa.System, tenantName string) ([]byte, error) {
+func (c *Config) readAPIKey(tenantName string) ([]byte, error) {
 	if override, ok := c.apiKeyFromEnv(); ok {
 		return override, nil
 	}
-	if path, ok := c.apiKeyFileFromEnv(); ok {
-		return os.ReadFile(path)
+	return os.ReadFile(c.apiKeyPath(tenantName))
+}
+
+// useAPIKey returns true if an API key should be used when authenticating with system.
+func (c *Config) useAPIKey(cli *CLI, system vespa.System, tenantName string) bool {
+	if _, ok := c.apiKeyFromEnv(); ok {
+		return true
+	}
+	if _, ok := c.apiKeyFileFromEnv(); ok {
+		return true
 	}
 	if !cli.isCI() {
+		// Fall back to API key, if present and Auth0 has not been configured
 		client, err := auth0.New(c.authConfigPath(), system.Name, system.URL)
-		if err == nil && client.HasCredentials() {
-			return nil, nil // use Auth0
+		if err != nil || !client.HasCredentials() {
+			cli.printWarning("Regular authentication is preferred over API key in a non-CI context", "Authenticate with 'vespa auth login'")
+			return util.PathExists(c.apiKeyPath(tenantName))
 		}
-		cli.printWarning("Authenticating with API key. This is discouraged in non-CI environments", "Authenticate with 'vespa auth login'")
 	}
-	return os.ReadFile(c.apiKeyPath(tenantName))
+	return false
 }
 
 func (c *Config) readSessionID(app vespa.ApplicationID) (int64, error) {
diff --git a/client/go/cmd/config_test.go b/client/go/cmd/config_test.go
index 9f41ef46914..f89e752f82d 100644
--- a/client/go/cmd/config_test.go
+++ b/client/go/cmd/config_test.go
@@ -145,33 +145,17 @@ func assertConfigCommandErr(t *testing.T, configHome, expected string, args ...s
 	assert.NotNil(t, assertConfigCommandStdErr(t, configHome, expected, args...))
 }
 
-func TestReadAPIKey(t *testing.T) {
+func TestUseAPIKey(t *testing.T) {
 	cli, _, _ := newTestCLI(t)
-	key, err := cli.config.readAPIKey(cli, vespa.PublicSystem, "t1")
-	assert.Nil(t, key)
-	require.NotNil(t, err)
+	assert.False(t, cli.config.useAPIKey(cli, vespa.PublicSystem, "t1"))
 
-	// From default path when it exists
-	require.Nil(t, os.WriteFile(filepath.Join(cli.config.homeDir, "t1.api-key.pem"), []byte("foo"), 0600))
-	key, err = cli.config.readAPIKey(cli, vespa.PublicSystem, "t1")
-	require.Nil(t, err)
-	assert.Equal(t, []byte("foo"), key)
-
-	// From file specified in environment
-	keyFile := filepath.Join(t.TempDir(), "key")
-	require.Nil(t, os.WriteFile(keyFile, []byte("bar"), 0600))
-	cli, _, _ = newTestCLI(t, "VESPA_CLI_API_KEY_FILE="+keyFile)
-	key, err = cli.config.readAPIKey(cli, vespa.PublicSystem, "t1")
-	require.Nil(t, err)
-	assert.Equal(t, []byte("bar"), key)
+	cli, _, _ = newTestCLI(t, "VESPA_CLI_API_KEY_FILE=/tmp/foo")
+	assert.True(t, cli.config.useAPIKey(cli, vespa.PublicSystem, "t1"))
 
-	// From key specified in environment
-	cli, _, _ = newTestCLI(t, "VESPA_CLI_API_KEY=baz")
-	key, err = cli.config.readAPIKey(cli, vespa.PublicSystem, "t1")
-	require.Nil(t, err)
-	assert.Equal(t, []byte("baz"), key)
+	cli, _, _ = newTestCLI(t, "VESPA_CLI_API_KEY=foo")
+	assert.True(t, cli.config.useAPIKey(cli, vespa.PublicSystem, "t1"))
 
-	// Auth0 is preferred when configured
+	// Prefer Auth0, if configured
 	authContent := `
 {
     "version": 1,
@@ -188,9 +172,10 @@ func TestReadAPIKey(t *testing.T) {
 		}
 	}
 }`
-	cli, _, _ = newTestCLI(t)
-	require.Nil(t, os.WriteFile(filepath.Join(cli.config.homeDir, "auth.json"), []byte(authContent), 0600))
-	key, err = cli.config.readAPIKey(cli, vespa.PublicSystem, "t1")
+	cli, _, _ = newTestCLI(t, "VESPA_CLI_CLOUD_SYSTEM=public")
+	_, err := os.Create(filepath.Join(cli.config.homeDir, "t2.api-key.pem"))
 	require.Nil(t, err)
-	assert.Equal(t, []byte(nil), key)
+	assert.True(t, cli.config.useAPIKey(cli, vespa.PublicSystem, "t2"))
+	require.Nil(t, os.WriteFile(filepath.Join(cli.config.homeDir, "auth.json"), []byte(authContent), 0600))
+	assert.False(t, cli.config.useAPIKey(cli, vespa.PublicSystem, "t2"))
 }
diff --git a/client/go/cmd/root.go b/client/go/cmd/root.go
index 0557248cedf..e88398a7fde 100644
--- a/client/go/cmd/root.go
+++ b/client/go/cmd/root.go
@@ -324,9 +324,11 @@ func (c *CLI) createCloudTarget(targetType string, opts targetOptions) (vespa.Ta
 	)
 	switch targetType {
 	case vespa.TargetCloud:
-		apiKey, err = c.config.readAPIKey(c, system, deployment.Application.Tenant)
-		if err != nil {
-			return nil, err
+		if c.config.useAPIKey(c, system, deployment.Application.Tenant) {
+			apiKey, err = c.config.readAPIKey(deployment.Application.Tenant)
+			if err != nil {
+				return nil, err
+			}
 		}
 		authConfigPath = c.config.authConfigPath()
 		deploymentTLSOptions = vespa.TLSOptions{}