Merge pull request #420 from authzed/fix-usage-with-no-context

Fix CLI usage with no pre-set context/config

internal/client/client.go

@@ -54,7 +54,7 @@ func newClientForCurrentContext(cmd *cobra.Command) (Client, error) {
 }
 
 func newClientForContext(cmd *cobra.Command, contextName string, secretStore storage.SecretStore) (*authzed.Client, error) {
-	currentToken, err := storage.GetToken(contextName, secretStore)
+	currentToken, err := storage.GetTokenIfExists(contextName, secretStore)
 	if err != nil {
 		return nil, err
 	}
@@ -74,6 +74,14 @@ func newClientForContext(cmd *cobra.Command, contextName string, secretStore sto
 
 // GetCurrentTokenWithCLIOverride returns the current token, but overridden by any parameter specified via CLI args
 func GetCurrentTokenWithCLIOverride(cmd *cobra.Command, configStore storage.ConfigStore, secretStore storage.SecretStore) (storage.Token, error) {
+	// Handle the no-config case separately
+	configExists, err := configStore.Exists()
+	if err != nil {
+		return storage.Token{}, err
+	}
+	if !configExists {
+		return GetTokenWithCLIOverride(cmd, storage.Token{})
+	}
 	token, err := storage.CurrentToken(
 		configStore,
 		secretStore,

internal/client/client_test.go

@@ -2,6 +2,7 @@ package client_test
 
 import (
 	"os"
+	"path"
 	"testing"
 
 	"github.com/authzed/zed/internal/client"
@@ -12,10 +13,11 @@ import (
 )
 
 func TestGetTokenWithCLIOverride(t *testing.T) {
+	require := require.New(t)
 	testCert, err := os.CreateTemp("", "")
-	require.NoError(t, err)
+	require.NoError(err)
 	_, err = testCert.Write([]byte("hi"))
-	require.NoError(t, err)
+	require.NoError(err)
 	cmd := zedtesting.CreateTestCobraCommandWithFlagValue(t,
 		zedtesting.StringFlag{FlagName: "token", FlagValue: "t1", Changed: true},
 		zedtesting.StringFlag{FlagName: "certificate-path", FlagValue: testCert.Name(), Changed: true},
@@ -29,13 +31,13 @@ func TestGetTokenWithCLIOverride(t *testing.T) {
 
 	// cli args take precedence when defined
 	to, err := client.GetTokenWithCLIOverride(cmd, storage.Token{})
-	require.NoError(t, err)
-	require.True(t, to.AnyValue())
-	require.Equal(t, "t1", to.APIToken)
-	require.Equal(t, "e1", to.Endpoint)
-	require.Equal(t, []byte("hi"), to.CACert)
-	require.Equal(t, &bTrue, to.Insecure)
-	require.Equal(t, &bTrue, to.NoVerifyCA)
+	require.NoError(err)
+	require.True(to.AnyValue())
+	require.Equal("t1", to.APIToken)
+	require.Equal("e1", to.Endpoint)
+	require.Equal([]byte("hi"), to.CACert)
+	require.Equal(&bTrue, to.Insecure)
+	require.Equal(&bTrue, to.NoVerifyCA)
 
 	// storage token takes precedence when defined
 	cmd = zedtesting.CreateTestCobraCommandWithFlagValue(t,
@@ -52,11 +54,66 @@ func TestGetTokenWithCLIOverride(t *testing.T) {
 		Insecure:   &bFalse,
 		NoVerifyCA: &bFalse,
 	})
-	require.NoError(t, err)
-	require.True(t, to.AnyValue())
-	require.Equal(t, "t2", to.APIToken)
-	require.Equal(t, "e2", to.Endpoint)
-	require.Equal(t, []byte("bye"), to.CACert)
-	require.Equal(t, &bFalse, to.Insecure)
-	require.Equal(t, &bFalse, to.NoVerifyCA)
+	require.NoError(err)
+	require.True(to.AnyValue())
+	require.Equal("t2", to.APIToken)
+	require.Equal("e2", to.Endpoint)
+	require.Equal([]byte("bye"), to.CACert)
+	require.Equal(&bFalse, to.Insecure)
+	require.Equal(&bFalse, to.NoVerifyCA)
+}
+
+func TestGetCurrentTokenWithCLIOverrideWithoutConfigFile(t *testing.T) {
+	// When we refactored the token setting logic, we broke the workflow where zed is used without a saved
+	// configuration. This asserts that that workflow works.
+	require := require.New(t)
+	cmd := zedtesting.CreateTestCobraCommandWithFlagValue(t,
+		zedtesting.StringFlag{FlagName: "token", FlagValue: "t1", Changed: true},
+		zedtesting.StringFlag{FlagName: "endpoint", FlagValue: "e1", Changed: true},
+		zedtesting.StringFlag{FlagName: "certificate-path", FlagValue: "", Changed: false},
+		zedtesting.BoolFlag{FlagName: "insecure", FlagValue: true, Changed: true},
+	)
+
+	configStore := &storage.JSONConfigStore{ConfigPath: "/not/a/valid/path"}
+	secretStore := &storage.KeychainSecretStore{ConfigPath: "/not/a/valid/path"}
+	token, err := client.GetCurrentTokenWithCLIOverride(cmd, configStore, secretStore)
+
+	// cli args take precedence when defined
+	require.NoError(err)
+	require.True(token.AnyValue())
+	require.Equal("t1", token.APIToken)
+	require.Equal("e1", token.Endpoint)
+	require.NotNil(token.Insecure)
+	require.True(*token.Insecure)
+}
+
+func TestGetCurrentTokenWithCLIOverrideWithoutSecretFile(t *testing.T) {
+	// When we refactored the token setting logic, we broke the workflow where zed is used without a saved
+	// context. This asserts that that workflow works.
+	require := require.New(t)
+	cmd := zedtesting.CreateTestCobraCommandWithFlagValue(t,
+		zedtesting.StringFlag{FlagName: "token", FlagValue: "t1", Changed: true},
+		zedtesting.StringFlag{FlagName: "endpoint", FlagValue: "e1", Changed: true},
+		zedtesting.StringFlag{FlagName: "certificate-path", FlagValue: "", Changed: false},
+		zedtesting.BoolFlag{FlagName: "insecure", FlagValue: true, Changed: true},
+	)
+
+	bTrue := true
+
+	tmpDir, err := os.MkdirTemp("", "")
+	require.NoError(err)
+	configPath := path.Join(tmpDir, "config.json")
+	err = os.WriteFile(configPath, []byte("{}"), 0o600)
+	require.NoError(err)
+
+	configStore := &storage.JSONConfigStore{ConfigPath: tmpDir}
+	secretStore := &storage.KeychainSecretStore{ConfigPath: "/not/a/valid/path"}
+	token, err := client.GetCurrentTokenWithCLIOverride(cmd, configStore, secretStore)
+
+	// cli args take precedence when defined
+	require.NoError(err)
+	require.True(token.AnyValue())
+	require.Equal("t1", token.APIToken)
+	require.Equal("e1", token.Endpoint)
+	require.Equal(&bTrue, token.Insecure)
 }

internal/storage/config.go

@@ -3,6 +3,7 @@ package storage
 import (
 	"encoding/json"
 	"errors"
+	"io/fs"
 	"os"
 	"path/filepath"
 	"runtime"
@@ -15,6 +16,9 @@ const configFileName = "config.json"
 // ErrConfigNotFound is returned if there is no Config in a ConfigStore.
 var ErrConfigNotFound = errors.New("config did not exist")
 
+// ErrTokenNotFound is returned if there is no Token in a ConfigStore.
+var ErrTokenNotFound = errors.New("token does not exist")
+
 // Config represents the contents of a zed configuration file.
 type Config struct {
 	Version      string
@@ -25,6 +29,7 @@ type Config struct {
 type ConfigStore interface {
 	Get() (Config, error)
 	Put(Config) error
+	Exists() (bool, error)
 }
 
 // TokenWithOverride returns a Token that retrieves its values from the reference Token, and has its values overridden
@@ -69,23 +74,28 @@ func TokenWithOverride(overrideToken Token, referenceToken Token) (Token, error)
 
 // CurrentToken is a convenient way to obtain the CurrentToken field from the
 // current Config.
-func CurrentToken(cs ConfigStore, ss SecretStore) (Token, error) {
+func CurrentToken(cs ConfigStore, ss SecretStore) (token Token, err error) {
 	cfg, err := cs.Get()
 	if err != nil {
 		return Token{}, err
 	}
 
-	return GetToken(cfg.CurrentToken, ss)
+	return GetTokenIfExists(cfg.CurrentToken, ss)
 }
 
 // SetCurrentToken is a convenient way to set the CurrentToken field in a
 // the current config.
 func SetCurrentToken(name string, cs ConfigStore, ss SecretStore) error {
 	// Ensure the token exists
-	if _, err := GetToken(name, ss); err != nil {
+	exists, err := TokenExists(name, ss)
+	if err != nil {
 		return err
 	}
 
+	if !exists {
+		return ErrTokenNotFound
+	}
+
 	cfg, err := cs.Get()
 	if err != nil {
 		if errors.Is(err, ErrConfigNotFound) {
@@ -110,10 +120,9 @@ var _ ConfigStore = JSONConfigStore{}
 // Get parses a Config from the filesystem.
 func (s JSONConfigStore) Get() (Config, error) {
 	cfgBytes, err := os.ReadFile(filepath.Join(s.ConfigPath, configFileName))
-	if err != nil {
-		if os.IsNotExist(err) {
-			return Config{}, ErrConfigNotFound
-		}
+	if errors.Is(err, fs.ErrNotExist) {
+		return Config{}, ErrConfigNotFound
+	} else if err != nil {
 		return Config{}, err
 	}
 
@@ -139,6 +148,15 @@ func (s JSONConfigStore) Put(cfg Config) error {
 	return atomicWriteFile(filepath.Join(s.ConfigPath, configFileName), cfgBytes, 0o774)
 }
 
+func (s JSONConfigStore) Exists() (bool, error) {
+	if _, err := os.Stat(filepath.Join(s.ConfigPath, configFileName)); errors.Is(err, fs.ErrNotExist) {
+		return false, nil
+	} else if err != nil {
+		return false, err
+	}
+	return true, nil
+}
+
 // atomicWriteFile writes data to filename+some suffix, then renames it into
 // filename.
 //

internal/storage/secrets.go

@@ -14,9 +14,6 @@ import (
 	"github.com/authzed/zed/internal/console"
 )
 
-// ErrTokenNotFound is returned if there is no Token in a ConfigStore.
-var ErrTokenNotFound = errors.New("token does not exist")
-
 type Token struct {
 	Name       string
 	Endpoint   string
@@ -72,7 +69,8 @@ type SecretStore interface {
 	Put(s Secrets) error
 }
 
-func GetToken(name string, ss SecretStore) (Token, error) {
+// Returns an empty token if no token exists.
+func GetTokenIfExists(name string, ss SecretStore) (Token, error) {
 	secrets, err := ss.Get()
 	if err != nil {
 		return Token{}, err
@@ -84,7 +82,22 @@ func GetToken(name string, ss SecretStore) (Token, error) {
 		}
 	}
 
-	return Token{}, ErrTokenNotFound
+	return Token{}, nil
+}
+
+func TokenExists(name string, ss SecretStore) (bool, error) {
+	secrets, err := ss.Get()
+	if err != nil {
+		return false, err
+	}
+
+	for _, token := range secrets.Tokens {
+		if name == token.Name {
+			return true, nil
+		}
+	}
+
+	return false, nil
 }
 
 func PutToken(t Token, ss SecretStore) error {