fix: only create config managers if needed to avoid syncing on every …

…cmd (#2091) fixes #2086 - Recently we added a feature for cf.Manager to sync providers (1Password & ASM) - We should not create these managers if they are not used, otherwise we will prompt the user for 1Password access unnecessarily - Previously we were creating config + secret managers in `main.go` for every command, even though it is only needed for these commands: - `ftl serve` - `ftl dev` - `ftl config ...` if there is no controller running - `ftl secret ...` if there is no controller running
TBD54566975 · Jul 17, 2024 · 1e695b0 · 1e695b0
1 parent e9ef97a
commit 1e695b0
Show file tree

Hide file tree

Showing 9 changed files with 192 additions and 58 deletions.
diff --git a/backend/controller/admin/client.go b/backend/controller/admin/client.go
@@ -3,6 +3,7 @@ package admin
 import (
 	"context"
 	"errors"
+	"fmt"
 	"net"
 	"net/url"
 
@@ -41,22 +42,22 @@ type Client interface {
 	SecretUnset(ctx context.Context, req *connect.Request[ftlv1.UnsetSecretRequest]) (*connect.Response[ftlv1.UnsetSecretResponse], error)
 }
 
-// NewClient takes the service client and endpoint flag received by the cmd interface
-// and returns an appropriate interface for the cmd library to use.
+// ShouldUseLocalClient returns whether a local admin client should be used based on the admin service client and the endpoint.
 //
-// If the controller is not present AND endpoint is local, then inject a purely-local
-// implementation of the interface so that the user does not need to spin up a controller
-// just to run the `ftl config/secret` commands. Otherwise, return back the gRPC client.
-func NewClient(ctx context.Context, adminClient ftlv1connect.AdminServiceClient, endpoint *url.URL) (Client, error) {
+// If the controller is not present AND endpoint is local, then a local client should be used
+// so that the user does not need to spin up a controller just to run the `ftl config/secret` commands.
+//
+// If true is returned, use NewLocalClient() to create a local client after setting up config and secret managers for the context.
+func ShouldUseLocalClient(ctx context.Context, adminClient ftlv1connect.AdminServiceClient, endpoint *url.URL) (bool, error) {
 	isLocal, err := isEndpointLocal(endpoint)
 	if err != nil {
-		return adminClient, err
+		return false, err
 	}
 	_, err = adminClient.Ping(ctx, connect.NewRequest(&ftlv1.PingRequest{}))
 	if isConnectUnavailableError(err) && isLocal {
-		return newLocalClient(ctx), nil
+		return true, nil
 	}
-	return adminClient, nil
+	return false, nil
 }
 
 func isConnectUnavailableError(err error) bool {
@@ -71,7 +72,7 @@ func isEndpointLocal(endpoint *url.URL) (bool, error) {
 	h := endpoint.Hostname()
 	ips, err := net.LookupIP(h)
 	if err != nil {
-		return false, err
+		return false, fmt.Errorf("failed to look up own IP: %w", err)
 	}
 	for _, netip := range ips {
 		if netip.IsLoopback() {

diff --git a/backend/controller/admin/local_client.go b/backend/controller/admin/local_client.go
@@ -7,7 +7,7 @@ import (
 
 	"github.com/TBD54566975/ftl/backend/schema"
 	"github.com/TBD54566975/ftl/buildengine"
-	"github.com/TBD54566975/ftl/common/configuration"
+	cf "github.com/TBD54566975/ftl/common/configuration"
 	"github.com/TBD54566975/ftl/common/projectconfig"
 	"github.com/alecthomas/types/optional"
 )
@@ -24,9 +24,8 @@ type diskSchemaRetriever struct {
 	deployRoot optional.Option[string]
 }
 
-func newLocalClient(ctx context.Context) *localClient {
-	cm := configuration.ConfigFromContext(ctx)
-	sm := configuration.SecretsFromContext(ctx)
+// NewLocalClient creates a admin client that reads and writes from the provided config and secret managers
+func NewLocalClient(cm *cf.Manager[cf.Configuration], sm *cf.Manager[cf.Secrets]) Client {
 	return &localClient{NewAdminService(cm, sm, &diskSchemaRetriever{})}
 }
 

diff --git a/cmd/ftl/cmd_config.go b/cmd/ftl/cmd_config.go
@@ -13,7 +13,11 @@ import (
 
 	"github.com/TBD54566975/ftl/backend/controller/admin"
 	ftlv1 "github.com/TBD54566975/ftl/backend/protos/xyz/block/ftl/v1"
+	"github.com/TBD54566975/ftl/backend/protos/xyz/block/ftl/v1/ftlv1connect"
 	cf "github.com/TBD54566975/ftl/common/configuration"
+	"github.com/TBD54566975/ftl/common/projectconfig"
+	"github.com/TBD54566975/ftl/internal/log"
+	"github.com/TBD54566975/ftl/internal/rpc"
 )
 
 type configCmd struct {
@@ -60,7 +64,38 @@ type configListCmd struct {
 	Module string `optional:"" arg:"" placeholder:"MODULE" help:"List configuration only in this module."`
 }
 
-func (s *configListCmd) Run(ctx context.Context, adminClient admin.Client) error {
+func setUpAdminClient(ctx context.Context, config projectconfig.Config) (ctxOut context.Context, client admin.Client, err error) {
+	adminServiceClient := rpc.Dial(ftlv1connect.NewAdminServiceClient, cli.Endpoint.String(), log.Error)
+	shouldUseLocalClient, err := admin.ShouldUseLocalClient(ctx, adminServiceClient, cli.Endpoint)
+	if err != nil {
+		return ctx, client, fmt.Errorf("could not create admin client: %w", err)
+	}
+	if shouldUseLocalClient {
+		// create config and secret managers
+		cr := cf.ProjectConfigResolver[cf.Configuration]{Config: config.Path}
+		cm, err := cf.NewConfigurationManager(ctx, cr)
+		if err != nil {
+			return ctx, client, fmt.Errorf("could not create config manager: %w", err)
+		}
+		ctx = cf.ContextWithConfig(ctx, cm)
+
+		sr := cf.ProjectConfigResolver[cf.Secrets]{Config: config.Path}
+		sm, err := cf.NewSecretsManager(ctx, sr, cli.Vault, config.Path)
+		if err != nil {
+			return ctx, client, fmt.Errorf("could not create secrets manager: %w", err)
+		}
+		ctx = cf.ContextWithSecrets(ctx, sm)
+
+		return ctx, admin.NewLocalClient(cm, sm), nil
+	}
+	return ctx, adminServiceClient, nil
+}
+
+func (s *configListCmd) Run(ctx context.Context, projConfig projectconfig.Config) error {
+	ctx, adminClient, err := setUpAdminClient(ctx, projConfig)
+	if err != nil {
+		return err
+	}
 	resp, err := adminClient.ConfigList(ctx, connect.NewRequest(&ftlv1.ListConfigRequest{
 		Module:        &s.Module,
 		IncludeValues: &s.Values,
@@ -90,7 +125,11 @@ Returns a JSON-encoded configuration value.
 `
 }
 
-func (s *configGetCmd) Run(ctx context.Context, adminClient admin.Client) error {
+func (s *configGetCmd) Run(ctx context.Context, projConfig projectconfig.Config) error {
+	ctx, adminClient, err := setUpAdminClient(ctx, projConfig)
+	if err != nil {
+		return err
+	}
 	resp, err := adminClient.ConfigGet(ctx, connect.NewRequest(&ftlv1.GetConfigRequest{
 		Ref: configRefFromRef(s.Ref),
 	}))
@@ -107,8 +146,11 @@ type configSetCmd struct {
 	Value *string `arg:"" placeholder:"VALUE" help:"Configuration value (read from stdin if omitted)." optional:""`
 }
 
-func (s *configSetCmd) Run(ctx context.Context, scmd *configCmd, adminClient admin.Client) error {
-	var err error
+func (s *configSetCmd) Run(ctx context.Context, scmd *configCmd, projConfig projectconfig.Config) error {
+	ctx, adminClient, err := setUpAdminClient(ctx, projConfig)
+	if err != nil {
+		return err
+	}
 	var config []byte
 	if s.Value != nil {
 		config = []byte(*s.Value)
@@ -151,14 +193,18 @@ type configUnsetCmd struct {
 	Ref cf.Ref `arg:"" help:"Configuration reference in the form [<module>.]<name>."`
 }
 
-func (s *configUnsetCmd) Run(ctx context.Context, scmd *configCmd, adminClient admin.Client) error {
+func (s *configUnsetCmd) Run(ctx context.Context, scmd *configCmd, projConfig projectconfig.Config) error {
+	ctx, adminClient, err := setUpAdminClient(ctx, projConfig)
+	if err != nil {
+		return err
+	}
 	req := &ftlv1.UnsetConfigRequest{
 		Ref: configRefFromRef(s.Ref),
 	}
 	if provider, ok := scmd.provider().Get(); ok {
 		req.Provider = &provider
 	}
-	_, err := adminClient.ConfigUnset(ctx, connect.NewRequest(req))
+	_, err = adminClient.ConfigUnset(ctx, connect.NewRequest(req))
 	if err != nil {
 		return err
 	}
@@ -175,7 +221,11 @@ Imports configuration values from a JSON object.
 `
 }
 
-func (s *configImportCmd) Run(ctx context.Context, cmd *configCmd, adminClient admin.Client) error {
+func (s *configImportCmd) Run(ctx context.Context, cmd *configCmd, projConfig projectconfig.Config) error {
+	ctx, adminClient, err := setUpAdminClient(ctx, projConfig)
+	if err != nil {
+		return err
+	}
 	input, err := io.ReadAll(s.Input)
 	if err != nil {
 		return fmt.Errorf("failed to read input: %w", err)
@@ -218,7 +268,11 @@ Outputs configuration values in a JSON object. A provider can be used to filter
 `
 }
 
-func (s *configExportCmd) Run(ctx context.Context, cmd *configCmd, adminClient admin.Client) error {
+func (s *configExportCmd) Run(ctx context.Context, cmd *configCmd, projConfig projectconfig.Config) error {
+	ctx, adminClient, err := setUpAdminClient(ctx, projConfig)
+	if err != nil {
+		return err
+	}
 	req := &ftlv1.ListConfigRequest{
 		IncludeValues: optional.Some(true).Ptr(),
 	}

diff --git a/cmd/ftl/cmd_secret.go b/cmd/ftl/cmd_secret.go
@@ -13,9 +13,9 @@ import (
 	"github.com/mattn/go-isatty"
 	"golang.org/x/term"
 
-	"github.com/TBD54566975/ftl/backend/controller/admin"
 	ftlv1 "github.com/TBD54566975/ftl/backend/protos/xyz/block/ftl/v1"
 	cf "github.com/TBD54566975/ftl/common/configuration"
+	"github.com/TBD54566975/ftl/common/projectconfig"
 )
 
 type secretCmd struct {
@@ -63,7 +63,11 @@ type secretListCmd struct {
 	Module string `optional:"" arg:"" placeholder:"MODULE" help:"List secrets only in this module."`
 }
 
-func (s *secretListCmd) Run(ctx context.Context, adminClient admin.Client) error {
+func (s *secretListCmd) Run(ctx context.Context, projConfig projectconfig.Config) error {
+	ctx, adminClient, err := setUpAdminClient(ctx, projConfig)
+	if err != nil {
+		return err
+	}
 	resp, err := adminClient.SecretsList(ctx, connect.NewRequest(&ftlv1.ListSecretsRequest{
 		Module:        &s.Module,
 		IncludeValues: &s.Values,
@@ -92,7 +96,11 @@ Returns a JSON-encoded secret value.
 `
 }
 
-func (s *secretGetCmd) Run(ctx context.Context, adminClient admin.Client) error {
+func (s *secretGetCmd) Run(ctx context.Context, projConfig projectconfig.Config) error {
+	ctx, adminClient, err := setUpAdminClient(ctx, projConfig)
+	if err != nil {
+		return err
+	}
 	resp, err := adminClient.SecretGet(ctx, connect.NewRequest(&ftlv1.GetSecretRequest{
 		Ref: configRefFromRef(s.Ref),
 	}))
@@ -108,9 +116,12 @@ type secretSetCmd struct {
 	Ref  cf.Ref `arg:"" help:"Secret reference in the form [<module>.]<name>."`
 }
 
-func (s *secretSetCmd) Run(ctx context.Context, scmd *secretCmd, adminClient admin.Client) error {
+func (s *secretSetCmd) Run(ctx context.Context, scmd *secretCmd, projConfig projectconfig.Config) error {
 	// Prompt for a secret if stdin is a terminal, otherwise read from stdin.
-	var err error
+	ctx, adminClient, err := setUpAdminClient(ctx, projConfig)
+	if err != nil {
+		return err
+	}
 	var secret []byte
 	if isatty.IsTerminal(0) {
 		fmt.Print("Secret: ")
@@ -158,14 +169,18 @@ type secretUnsetCmd struct {
 	Ref cf.Ref `arg:"" help:"Secret reference in the form [<module>.]<name>."`
 }
 
-func (s *secretUnsetCmd) Run(ctx context.Context, scmd *secretCmd, adminClient admin.Client) error {
+func (s *secretUnsetCmd) Run(ctx context.Context, scmd *secretCmd, projConfig projectconfig.Config) error {
+	ctx, adminClient, err := setUpAdminClient(ctx, projConfig)
+	if err != nil {
+		return err
+	}
 	req := &ftlv1.UnsetSecretRequest{
 		Ref: configRefFromRef(s.Ref),
 	}
 	if provider, ok := scmd.provider().Get(); ok {
 		req.Provider = &provider
 	}
-	_, err := adminClient.SecretUnset(ctx, connect.NewRequest(req))
+	_, err = adminClient.SecretUnset(ctx, connect.NewRequest(req))
 	if err != nil {
 		return err
 	}
@@ -182,7 +197,11 @@ Imports secrets from a JSON object.
 `
 }
 
-func (s *secretImportCmd) Run(ctx context.Context, scmd *secretCmd, adminClient admin.Client) error {
+func (s *secretImportCmd) Run(ctx context.Context, scmd *secretCmd, projConfig projectconfig.Config) error {
+	ctx, adminClient, err := setUpAdminClient(ctx, projConfig)
+	if err != nil {
+		return err
+	}
 	input, err := io.ReadAll(s.Input)
 	if err != nil {
 		return fmt.Errorf("failed to read input: %w", err)
@@ -225,7 +244,11 @@ Outputs secrets in a JSON object. A provider can be used to filter which secrets
 `
 }
 
-func (s *secretExportCmd) Run(ctx context.Context, scmd *secretCmd, adminClient admin.Client) error {
+func (s *secretExportCmd) Run(ctx context.Context, scmd *secretCmd, projConfig projectconfig.Config) error {
+	ctx, adminClient, err := setUpAdminClient(ctx, projConfig)
+	if err != nil {
+		return err
+	}
 	req := &ftlv1.ListSecretsRequest{
 		IncludeValues: optional.Some(true).Ptr(),
 	}

diff --git a/cmd/ftl/cmd_serve.go b/cmd/ftl/cmd_serve.go
@@ -23,6 +23,7 @@ import (
 	"github.com/TBD54566975/ftl/backend/controller/sql/databasetesting"
 	ftlv1 "github.com/TBD54566975/ftl/backend/protos/xyz/block/ftl/v1"
 	"github.com/TBD54566975/ftl/backend/protos/xyz/block/ftl/v1/ftlv1connect"
+	cf "github.com/TBD54566975/ftl/common/configuration"
 	"github.com/TBD54566975/ftl/common/projectconfig"
 	"github.com/TBD54566975/ftl/internal/bind"
 	"github.com/TBD54566975/ftl/internal/container"
@@ -122,6 +123,22 @@ func (s *serveCmd) run(ctx context.Context, projConfig projectconfig.Config, ini
 		scope := fmt.Sprintf("controller%d", i)
 		controllerCtx := log.ContextWithLogger(ctx, logger.Scope(scope))
 
+		// create config manager for controller
+		cr := cf.ProjectConfigResolver[cf.Configuration]{Config: projConfig.Path}
+		cm, err := cf.NewConfigurationManager(controllerCtx, cr)
+		if err != nil {
+			return fmt.Errorf("could not create config manager: %w", err)
+		}
+		controllerCtx = cf.ContextWithConfig(controllerCtx, cm)
+
+		// create secrets manager for controller
+		sr := cf.ProjectConfigResolver[cf.Secrets]{Config: projConfig.Path}
+		sm, err := cf.NewSecretsManager(controllerCtx, sr, cli.Vault, projConfig.Path)
+		if err != nil {
+			return fmt.Errorf("could not create secrets manager: %w", err)
+		}
+		controllerCtx = cf.ContextWithSecrets(controllerCtx, sm)
+
 		wg.Go(func() error {
 			if err := controller.Start(controllerCtx, config, runnerScaling); err != nil {
 				logger.Errorf(err, "controller%d failed: %v", i, err)

diff --git a/cmd/ftl/integration_test.go b/cmd/ftl/integration_test.go
@@ -33,6 +33,66 @@ func TestBox(t *testing.T) {
 	)
 }
 
+func TestConfigsWithController(t *testing.T) {
+	Run(t, "", configActions(t)...)
+}
+
+func TestConfigsWithoutController(t *testing.T) {
+	RunWithoutController(t, "", configActions(t)...)
+}
+
+func configActions(t *testing.T) []Action {
+	t.Helper()
+
+	return []Action{
+		// test setting value without --json flag
+		Exec("ftl", "config", "set", "test.one", "hello world", "--inline"),
+		ExecWithExpectedOutput("\"hello world\"\n", "ftl", "config", "get", "test.one"),
+		// test updating value with --json flag
+		Exec("ftl", "config", "set", "test.one", `"hello world 2"`, "--json", "--inline"),
+		ExecWithExpectedOutput("\"hello world 2\"\n", "ftl", "config", "get", "test.one"),
+		// test deleting value
+		Exec("ftl", "config", "unset", "test.one", "--inline"),
+		ExpectError(
+			ExecWithOutput("ftl", []string{"config", "get", "test.one"}, func(output string) {}),
+			"failed to get from config manager: not found",
+		),
+	}
+}
+
+func TestSecretsWithController(t *testing.T) {
+	Run(t, "", secretActions(t)...)
+}
+
+func TestSecretsWithoutController(t *testing.T) {
+	RunWithoutController(t, "", secretActions(t)...)
+}
+
+func secretActions(t *testing.T) []Action {
+	t.Helper()
+
+	// can not easily use Exec() to enter secure text, using secret import instead
+	secretsPath1, err := filepath.Abs("testdata/secrets1.json")
+	assert.NoError(t, err)
+	secretsPath2, err := filepath.Abs("testdata/secrets2.json")
+	assert.NoError(t, err)
+
+	return []Action{
+		// test setting secret without --json flag
+		Exec("ftl", "secret", "import", "--inline", secretsPath1),
+		ExecWithExpectedOutput("\"hello world\"\n", "ftl", "secret", "get", "test.one"),
+		// test updating secret
+		Exec("ftl", "secret", "import", "--inline", secretsPath2),
+		ExecWithExpectedOutput("\"hello world 2\"\n", "ftl", "secret", "get", "test.one"),
+		// test deleting secret
+		Exec("ftl", "secret", "unset", "test.one", "--inline"),
+		ExpectError(
+			ExecWithOutput("ftl", []string{"secret", "get", "test.one"}, func(output string) {}),
+			"failed to get from secret manager: not found",
+		),
+	}
+}
+
 func TestSecretImportExport(t *testing.T) {
 	testImportExport(t, "secret")
 }