fix: refactor manager.go to remove MutableProviders and accept provid…

…er key when writing

cmd/ftl/cmd_config.go

@@ -17,6 +17,9 @@ type configCmd struct {
 	Get   configGetCmd   `cmd:"" help:"Get a configuration value."`
 	Set   configSetCmd   `cmd:"" help:"Set a configuration value."`
 	Unset configUnsetCmd `cmd:"" help:"Unset a configuration value."`
+
+	Envar  bool `help:"Print configuration as environment variables." group:"Provider:" xor:"configwriter"`
+	Inline bool `help:"Write values inline in the configuration file." group:"Provider:" xor:"configwriter"`
 }
 
 func (s *configCmd) Help() string {
@@ -109,8 +112,11 @@ func (s *configSetCmd) Run(ctx context.Context, scmd *configCmd, cr cf.Resolver[
 		return err
 	}
 
-	if err := sm.Mutable(); err != nil {
-		return err
+	var providerKey string
+	if scmd.Inline {
+		providerKey = "inline"
+	} else if scmd.Envar {
+		providerKey = "envar"
 	}
 
 	var config []byte
@@ -131,7 +137,7 @@ func (s *configSetCmd) Run(ctx context.Context, scmd *configCmd, cr cf.Resolver[
 	} else {
 		configValue = string(config)
 	}
-	return sm.Set(ctx, s.Ref, configValue)
+	return sm.Set(ctx, providerKey, s.Ref, configValue)
 }
 
 type configUnsetCmd struct {
@@ -143,5 +149,13 @@ func (s *configUnsetCmd) Run(ctx context.Context, scmd *configCmd, cr cf.Resolve
 	if err != nil {
 		return err
 	}
-	return sm.Unset(ctx, s.Ref)
+
+	var providerKey string
+	if scmd.Inline {
+		providerKey = "inline"
+	} else if scmd.Envar {
+		providerKey = "envar"
+	}
+
+	return sm.Unset(ctx, providerKey, s.Ref)
 }

cmd/ftl/cmd_secret.go

@@ -20,6 +20,11 @@ type secretCmd struct {
 	Get   secretGetCmd   `cmd:"" help:"Get a secret."`
 	Set   secretSetCmd   `cmd:"" help:"Set a secret."`
 	Unset secretUnsetCmd `cmd:"" help:"Unset a secret."`
+
+	Envar    bool   `help:"Print configuration as environment variables." group:"Provider:" xor:"secretwriter"`
+	Inline   bool   `help:"Write values inline in the configuration file." group:"Provider:" xor:"secretwriter"`
+	Keychain bool   `help:"Write to the system keychain." group:"Provider:" xor:"secretwriter"`
+	Vault    string `name:"op" help:"Store a secret in this 1Password vault. The name of the 1Password item will be the <ref> and the secret will be stored in the password field." group:"Provider:" xor:"secretwriter" placeholder:"VAULT"`
 }
 
 func (s *secretCmd) Help() string {
@@ -114,8 +119,15 @@ func (s *secretSetCmd) Run(ctx context.Context, scmd *secretCmd, sr cf.Resolver[
 		return err
 	}
 
-	if err := sm.Mutable(); err != nil {
-		return err
+	var providerKey string
+	if scmd.Envar {
+		providerKey = "envar"
+	} else if scmd.Inline {
+		providerKey = "inline"
+	} else if scmd.Keychain {
+		providerKey = "keychain"
+	} else if scmd.Vault != "" {
+		providerKey = "op"
 	}
 
 	// Prompt for a secret if stdin is a terminal, otherwise read from stdin.
@@ -142,7 +154,7 @@ func (s *secretSetCmd) Run(ctx context.Context, scmd *secretCmd, sr cf.Resolver[
 	} else {
 		secretValue = string(secret)
 	}
-	return sm.Set(ctx, s.Ref, secretValue)
+	return sm.Set(ctx, providerKey, s.Ref, secretValue)
 }
 
 type secretUnsetCmd struct {
@@ -154,5 +166,17 @@ func (s *secretUnsetCmd) Run(ctx context.Context, scmd *secretCmd, sr cf.Resolve
 	if err != nil {
 		return err
 	}
-	return sm.Unset(ctx, s.Ref)
+
+	var providerKey string
+	if scmd.Envar {
+		providerKey = "envar"
+	} else if scmd.Inline {
+		providerKey = "inline"
+	} else if scmd.Keychain {
+		providerKey = "keychain"
+	} else if scmd.Vault != "" {
+		providerKey = "op"
+	}
+
+	return sm.Unset(ctx, providerKey, s.Ref)
 }

common/configuration/1password_provider.go

@@ -19,11 +19,10 @@ import (
 // OnePasswordProvider is a configuration provider that reads passwords from
 // 1Password vaults via the "op" command line tool.
 type OnePasswordProvider struct {
-	Vault string `name:"op" help:"Store a secret in this 1Password vault. The name of the 1Password item will be the <ref> and the secret will be stored in the password field." group:"Provider:" xor:"configwriter" placeholder:"VAULT"`
+	// TODO(saf): this was set via CLI, now needs to be set via an arg.
+	Vault string ""
 }
 
-var _ MutableProvider[Secrets] = OnePasswordProvider{}
-
 func (OnePasswordProvider) Role() Secrets                               { return Secrets{} }
 func (o OnePasswordProvider) Key() string                               { return "op" }
 func (o OnePasswordProvider) Delete(ctx context.Context, ref Ref) error { return nil }
@@ -91,8 +90,6 @@ func (o OnePasswordProvider) Store(ctx context.Context, ref Ref, value []byte) (
 	return url, nil
 }
 
-func (o OnePasswordProvider) Writer() bool { return o.Vault != "" }
-
 func checkOpBinary() error {
 	_, err := exec.LookPath("op")
 	if err != nil {

common/configuration/api.go

@@ -89,26 +89,6 @@ type Provider[R Role] interface {
 	Role() R
 	Key() string
 	Load(ctx context.Context, ref Ref, key *url.URL) ([]byte, error)
-}
-
-// A MutableProvider is a Provider that can update configuration.
-type MutableProvider[R Role] interface {
-	Provider[R]
-	// Writer returns true if this provider should be used to store configuration.
-	//
-	// Only one provider should return true.
-	//
-	// To be usable from the CLI, each provider must be a Kong-compatible struct
-	// containing a flag that this method should return. For example:
-	//
-	// 	type InlineProvider struct {
-	// 		Inline bool `help:"Write values inline." group:"Provider:" xor:"configwriter"`
-	// 	}
-	//
-	//	func (i InlineProvider) Writer() bool { return i.Inline }
-	//
-	// The "xor" tag is used to ensure that only one writer is selected.
-	Writer() bool
 	// Store a configuration value and return its key.
 	Store(ctx context.Context, ref Ref, value []byte) (*url.URL, error)
 	// Delete a configuration value.

common/configuration/db_config_provider.go

@@ -19,8 +19,6 @@ type DBConfigProviderDAL interface {
 	UnsetModuleConfiguration(ctx context.Context, module optional.Option[string], name string) error
 }
 
-var _ MutableProvider[Configuration] = DBConfigProvider{}
-
 func NewDBConfigProvider(dal DBConfigProviderDAL) DBConfigProvider {
 	return DBConfigProvider{
 		dal: dal,

common/configuration/envar_provider.go

@@ -10,11 +10,7 @@ import (
 
 // EnvarProvider is a configuration provider that reads secrets or configuration
 // from environment variables.
-type EnvarProvider[R Role] struct {
-	Envar bool `help:"Print configuration as environment variables." xor:"configwriter" group:"Provider:"`
-}
-
-var _ MutableProvider[Configuration] = EnvarProvider[Configuration]{}
+type EnvarProvider[R Role] struct{}
 
 func (EnvarProvider[R]) Role() R     { var r R; return r }
 func (EnvarProvider[R]) Key() string { return "envar" }
@@ -40,7 +36,7 @@ func (e EnvarProvider[R]) Store(ctx context.Context, ref Ref, value []byte) (*ur
 	return &url.URL{Scheme: "envar", Host: ref.Name}, nil
 }
 
-func (e EnvarProvider[R]) Writer() bool { return e.Envar }
+func (e EnvarProvider[R]) Writer() bool { return true }
 
 func (e EnvarProvider[R]) key(ref Ref) string {
 	key := e.prefix()

common/configuration/inline_provider.go

@@ -8,16 +8,12 @@ import (
 )
 
 // InlineProvider is a configuration provider that stores configuration in its key.
-type InlineProvider[R Role] struct {
-	Inline bool `help:"Write values inline in the configuration file." group:"Provider:" xor:"configwriter"`
-}
-
-var _ MutableProvider[Configuration] = InlineProvider[Configuration]{}
+type InlineProvider[R Role] struct{}
 
 func (InlineProvider[R]) Role() R     { var r R; return r }
 func (InlineProvider[R]) Key() string { return "inline" }
 
-func (i InlineProvider[R]) Writer() bool { return i.Inline }
+func (i InlineProvider[R]) Writer() bool { return true }
 
 func (InlineProvider[R]) Load(ctx context.Context, ref Ref, key *url.URL) ([]byte, error) {
 	data, err := base64.RawURLEncoding.DecodeString(key.Host)

common/configuration/keychain_provider.go

@@ -10,16 +10,12 @@ import (
 	keyring "github.com/zalando/go-keyring"
 )
 
-type KeychainProvider struct {
-	Keychain bool `help:"Write to the system keychain." group:"Provider:" xor:"configwriter"`
-}
-
-var _ MutableProvider[Secrets] = KeychainProvider{}
+type KeychainProvider struct{}
 
 func (KeychainProvider) Role() Secrets { return Secrets{} }
 func (k KeychainProvider) Key() string { return "keychain" }
 
-func (k KeychainProvider) Writer() bool { return k.Keychain }
+func (k KeychainProvider) Writer() bool { return true }
 
 func (k KeychainProvider) Load(ctx context.Context, ref Ref, key *url.URL) ([]byte, error) {
 	value, err := keyring.Get(k.serviceName(ref), key.Host)

common/configuration/manager.go

@@ -28,7 +28,6 @@ func (Configuration) String() string { return "configuration" }
 // the Resolver and Provider interfaces.
 type Manager[R Role] struct {
 	providers map[string]Provider[R]
-	writer    MutableProvider[R]
 	resolver  Resolver[R]
 }
 
@@ -60,32 +59,11 @@ func New[R Role](ctx context.Context, resolver Resolver[R], providers []Provider
 	}
 	for _, p := range providers {
 		m.providers[p.Key()] = p
-		if mutable, ok := p.(MutableProvider[R]); ok && mutable.Writer() {
-			if m.writer != nil {
-				return nil, fmt.Errorf("multiple writers %s and %s", m.writer.Key(), p.Key())
-			}
-			m.writer = mutable
-		}
 	}
 	m.resolver = resolver
 	return m, nil
 }
 
-// Mutable returns an error if the configuration manager doesn't have a
-// writeable provider configured.
-func (m *Manager[R]) Mutable() error {
-	if m.writer != nil {
-		return nil
-	}
-	writers := []string{}
-	for _, p := range m.providers {
-		if mutable, ok := p.(MutableProvider[R]); ok {
-			writers = append(writers, "--"+mutable.Key())
-		}
-	}
-	return fmt.Errorf("no writeable configuration provider available, specify one of %s", strings.Join(writers, ", "))
-}
-
 // getData returns a data value for a configuration from the active providers.
 // The data can be unmarshalled from JSON.
 func (m *Manager[R]) getData(ctx context.Context, ref Ref) ([]byte, error) {
@@ -123,15 +101,16 @@ func (m *Manager[R]) Get(ctx context.Context, ref Ref, value any) error {
 }
 
 // Set a configuration value.
-func (m *Manager[R]) Set(ctx context.Context, ref Ref, value any) error {
-	if err := m.Mutable(); err != nil {
-		return err
+func (m *Manager[R]) Set(ctx context.Context, pkey string, ref Ref, value any) error {
+	provider, ok := m.providers[pkey]
+	if !ok {
+		return fmt.Errorf("no provider for key %q", pkey)
 	}
 	data, err := json.Marshal(value)
 	if err != nil {
 		return err
 	}
-	key, err := m.writer.Store(ctx, ref, data)
+	key, err := provider.Store(ctx, ref, data)
 	if err != nil {
 		return err
 	}
@@ -170,13 +149,13 @@ func (m *Manager[R]) MapForModule(ctx context.Context, module string) (map[strin
 }
 
 // Unset a configuration value in all providers.
-func (m *Manager[R]) Unset(ctx context.Context, ref Ref) error {
-	for _, provider := range m.providers {
-		if mutable, ok := provider.(MutableProvider[R]); ok {
-			if err := mutable.Delete(ctx, ref); err != nil && !errors.Is(err, ErrNotFound) {
-				return err
-			}
-		}
+func (m *Manager[R]) Unset(ctx context.Context, pkey string, ref Ref) error {
+	provider, ok := m.providers[pkey]
+	if !ok {
+		return fmt.Errorf("no provider for key %q", pkey)
+	}
+	if err := provider.Delete(ctx, ref); err != nil && !errors.Is(err, ErrNotFound) {
+		return err
 	}
 	return m.resolver.Unset(ctx, ref)
 }

common/configuration/manager_test.go

@@ -21,7 +21,7 @@ func TestManager(t *testing.T) {
 	ctx := log.ContextWithNewDefaultLogger(context.Background())
 
 	t.Run("Secrets", func(t *testing.T) {
-		kcp := KeychainProvider{Keychain: true}
+		kcp := KeychainProvider{}
 		_, err := kcp.Store(ctx, Ref{Name: "mutable"}, []byte("hello"))
 		assert.NoError(t, err)
 		cf, err := New(ctx,
@@ -32,7 +32,7 @@ func TestManager(t *testing.T) {
 				kcp,
 			})
 		assert.NoError(t, err)
-		testManager(t, ctx, cf, "FTL_SECRET_YmF6", []Entry{
+		testManager(t, ctx, cf, "keychain", "FTL_SECRET_YmF6", []Entry{
 			{Ref: Ref{Name: "baz"}, Accessor: URL("envar://baz")},
 			{Ref: Ref{Name: "foo"}, Accessor: URL("inline://ImJhciI")},
 			{Ref: Ref{Name: "mutable"}, Accessor: URL("keychain://mutable")},
@@ -43,10 +43,10 @@ func TestManager(t *testing.T) {
 			ProjectConfigResolver[Configuration]{Config: []string{config}},
 			[]Provider[Configuration]{
 				EnvarProvider[Configuration]{},
-				InlineProvider[Configuration]{Inline: true}, // Writer
+				InlineProvider[Configuration]{},
 			})
 		assert.NoError(t, err)
-		testManager(t, ctx, cf, "FTL_CONFIG_YmF6", []Entry{
+		testManager(t, ctx, cf, "inline", "FTL_CONFIG_YmF6", []Entry{
 			{Ref: Ref{Name: "baz"}, Accessor: URL("envar://baz")},
 			{Ref: Ref{Name: "foo"}, Accessor: URL("inline://ImJhciI")},
 			{Ref: Ref{Name: "mutable"}, Accessor: URL("inline://ImhlbGxvIg")},
@@ -62,9 +62,7 @@ func TestMapPriority(t *testing.T) {
 	cm, err := New(ctx,
 		ProjectConfigResolver[Configuration]{Config: []string{config}},
 		[]Provider[Configuration]{
-			InlineProvider[Configuration]{
-				Inline: true,
-			},
+			InlineProvider[Configuration]{},
 		})
 	assert.NoError(t, err)
 	moduleName := "test"
@@ -79,12 +77,12 @@ func TestMapPriority(t *testing.T) {
 		globalStrValue := "GlobalHelloWorld"
 		if i%2 == 0 {
 			// sometimes try setting the module config first
-			assert.NoError(t, cm.Set(ctx, Ref{Module: optional.Some(moduleName), Name: key}, strValue))
-			assert.NoError(t, cm.Set(ctx, Ref{Module: optional.None[string](), Name: key}, globalStrValue))
+			assert.NoError(t, cm.Set(ctx, "inline", Ref{Module: optional.Some(moduleName), Name: key}, strValue))
+			assert.NoError(t, cm.Set(ctx, "inline", Ref{Module: optional.None[string](), Name: key}, globalStrValue))
 		} else {
 			// other times try setting the global config first
-			assert.NoError(t, cm.Set(ctx, Ref{Module: optional.None[string](), Name: key}, globalStrValue))
-			assert.NoError(t, cm.Set(ctx, Ref{Module: optional.Some(moduleName), Name: key}, strValue))
+			assert.NoError(t, cm.Set(ctx, "inline", Ref{Module: optional.None[string](), Name: key}, globalStrValue))
+			assert.NoError(t, cm.Set(ctx, "inline", Ref{Module: optional.Some(moduleName), Name: key}, strValue))
 		}
 	}
 	result, err := cm.MapForModule(ctx, moduleName)
@@ -118,6 +116,7 @@ func testManager[R Role](
 	t *testing.T,
 	ctx context.Context,
 	cf *Manager[R],
+	providerKey string,
 	envarName string,
 	expectedListing []Entry,
 ) {
@@ -147,7 +146,7 @@ func testManager[R Role](
 	assert.IsError(t, err, ErrNotFound)
 
 	// Change value.
-	err = cf.Set(ctx, Ref{Name: "mutable"}, "hello")
+	err = cf.Set(ctx, providerKey, Ref{Name: "mutable"}, "hello")
 	assert.NoError(t, err)
 
 	err = cf.Get(ctx, Ref{Name: "mutable"}, &fooValue)
@@ -159,7 +158,7 @@ func testManager[R Role](
 	assert.Equal(t, expectedListing, actualListing)
 
 	// Delete value
-	err = cf.Unset(ctx, Ref{Name: "foo"})
+	err = cf.Unset(ctx, "envar", Ref{Name: "foo"})
 	assert.NoError(t, err)
 	err = cf.Get(ctx, Ref{Name: "foo"}, &fooValue)
 	assert.IsError(t, err, ErrNotFound)