From c3c7077f60d42029be1be05f8bb60b5affbd0d0d Mon Sep 17 00:00:00 2001 From: Denise Li Date: Tue, 14 May 2024 19:29:46 -0400 Subject: [PATCH] feat: use secrets instead of env vars for DSNs (#1483) Fixes https://github.com/TBD54566975/ftl/issues/1427 database/ftl-project.toml was written using: ``` ftl secret set --inline database.FTL_DSN_DATABASE_TESTDB --config integration/testdata/go/database/ftl-project.toml ``` This PR only works thanks to this earlier PR: https://github.com/TBD54566975/ftl/pull/1472 --- backend/controller/controller.go | 2 +- go-runtime/ftl/ftltest/ftltest.go | 9 +-- integration/actions_test.go | 5 -- integration/harness_test.go | 6 +- integration/integration_test.go | 24 ++++---- .../testdata/go/database/database_test.go | 2 + .../testdata/go/database/ftl-project.toml | 13 ++++ internal/modulecontext/from_environment.go | 60 ------------------- internal/modulecontext/from_secrets.go | 55 +++++++++++++++++ ...vironment_test.go => from_secrets_test.go} | 8 ++- 10 files changed, 95 insertions(+), 89 deletions(-) create mode 100644 integration/testdata/go/database/ftl-project.toml delete mode 100644 internal/modulecontext/from_environment.go create mode 100644 internal/modulecontext/from_secrets.go rename internal/modulecontext/{from_environment_test.go => from_secrets_test.go} (75%) diff --git a/backend/controller/controller.go b/backend/controller/controller.go index ccd65c5ef2..76e9efebc5 100644 --- a/backend/controller/controller.go +++ b/backend/controller/controller.go @@ -668,7 +668,7 @@ func (s *Service) GetModuleContext(ctx context.Context, req *connect.Request[ftl if err != nil { return nil, connect.NewError(connect.CodeInternal, fmt.Errorf("could not get secrets: %w", err)) } - databases, err := modulecontext.DatabasesFromEnvironment(ctx, name) + databases, err := modulecontext.DatabasesFromSecrets(ctx, name, secrets) if err != nil { return nil, connect.NewError(connect.CodeInternal, fmt.Errorf("could not get databases: %w", err)) } diff --git a/go-runtime/ftl/ftltest/ftltest.go b/go-runtime/ftl/ftltest/ftltest.go index d12b656221..a49a65cb0f 100644 --- a/go-runtime/ftl/ftltest/ftltest.go +++ b/go-runtime/ftl/ftltest/ftltest.go @@ -37,15 +37,10 @@ func Context(options ...Option) context.Context { ctx := log.ContextWithNewDefaultLogger(context.Background()) name := ftl.Module() - databases, err := modulecontext.DatabasesFromEnvironment(ctx, name) - if err != nil { - panic(fmt.Sprintf("error setting up module context from environment: %v", err)) - } - state := &OptionsState{ configs: make(map[string][]byte), secrets: make(map[string][]byte), - databases: databases, + databases: make(map[string]modulecontext.Database), mockVerbs: make(map[schema.RefKey]modulecontext.Verb), } for _, option := range options { @@ -183,7 +178,7 @@ func WithSecret[T ftl.SecretType](secret ftl.SecretValue[T], value T) Option { // ) func WithDatabase(dbHandle ftl.Database) Option { return func(ctx context.Context, state *OptionsState) error { - originalDSN, err := modulecontext.GetDSNFromEnvar(ftl.Module(), dbHandle.Name) + originalDSN, err := modulecontext.GetDSNFromSecret(ftl.Module(), dbHandle.Name, state.secrets) if err != nil { return err } diff --git a/integration/actions_test.go b/integration/actions_test.go index b2e2de0de5..054250a73b 100644 --- a/integration/actions_test.go +++ b/integration/actions_test.go @@ -26,7 +26,6 @@ import ( schemapb "github.com/TBD54566975/ftl/backend/protos/xyz/block/ftl/v1/schema" ftlexec "github.com/TBD54566975/ftl/internal/exec" "github.com/TBD54566975/ftl/internal/log" - "github.com/TBD54566975/ftl/internal/modulecontext" "github.com/TBD54566975/scaffolder" ) @@ -271,10 +270,6 @@ func createDBAction(module, dbName string, isTest bool) action { } func createDB(t testing.TB, module, dbName string, isTestDb bool) { - // envars do not include test suffix - t.Setenv(modulecontext.DSNEnvarName(module, dbName), - fmt.Sprintf("postgres://postgres:secret@localhost:54320/%s?sslmode=disable", dbName)) - // insert test suffix if needed when actually setting up db if isTestDb { dbName += "_test" diff --git a/integration/harness_test.go b/integration/harness_test.go index c4413eb575..48c486ddb7 100644 --- a/integration/harness_test.go +++ b/integration/harness_test.go @@ -41,7 +41,7 @@ func infof(format string, args ...any) { var buildOnce sync.Once // run an integration test. -func run(t *testing.T, actions ...action) { +func run(t *testing.T, ftlConfigPath string, actions ...action) { tmpDir := t.TempDir() cwd, err := os.Getwd() @@ -49,6 +49,10 @@ func run(t *testing.T, actions ...action) { rootDir := internal.GitRoot("") + if ftlConfigPath != "" { + t.Setenv("FTL_CONFIG", filepath.Join(tmpDir, ftlConfigPath)) + } + // Build FTL binary logger := log.Configure(&logWriter{logger: t}, log.Config{Level: log.Debug}) ctx := log.ContextWithLogger(context.Background(), logger) diff --git a/integration/integration_test.go b/integration/integration_test.go index 84f0c49e1b..39cf727237 100644 --- a/integration/integration_test.go +++ b/integration/integration_test.go @@ -30,7 +30,7 @@ func TestCron(t *testing.T) { t.Cleanup(func() { _ = os.Remove(tmpFile) }) - run(t, + run(t, "", copyModule("cron"), deploy("cron"), func(t testing.TB, ic testContext) error { @@ -41,7 +41,7 @@ func TestCron(t *testing.T) { } func TestLifecycle(t *testing.T) { - run(t, + run(t, "", exec("ftl", "init", "go", ".", "echo"), deploy("echo"), call("echo", "echo", obj{"name": "Bob"}, func(response obj) error { @@ -54,7 +54,7 @@ func TestLifecycle(t *testing.T) { } func TestInterModuleCall(t *testing.T) { - run(t, + run(t, "", copyModule("echo"), copyModule("time"), deploy("time"), @@ -73,7 +73,7 @@ func TestInterModuleCall(t *testing.T) { } func TestNonExportedDecls(t *testing.T) { - run(t, + run(t, "", copyModule("time"), deploy("time"), copyModule("echo"), @@ -84,10 +84,10 @@ func TestNonExportedDecls(t *testing.T) { } func TestDatabase(t *testing.T) { - createDB(t, "database", "testdb", false) - run(t, + run(t, "database/ftl-project.toml", // deploy real module against "testdb" copyModule("database"), + createDBAction("database", "testdb", false), deploy("database"), call("database", "insert", obj{"data": "hello"}, func(response obj) error { return nil }), queryRow("testdb", "SELECT data FROM requests", "hello"), @@ -100,7 +100,7 @@ func TestDatabase(t *testing.T) { } func TestSchemaGenerate(t *testing.T) { - run(t, + run(t, "", copyDir("../schema-generate", "schema-generate"), mkdir("build/schema-generate"), exec("ftl", "schema", "generate", "schema-generate", "build/schema-generate"), @@ -109,7 +109,7 @@ func TestSchemaGenerate(t *testing.T) { } func TestHttpEncodeOmitempty(t *testing.T) { - run(t, + run(t, "", copyModule("omitempty"), deploy("omitempty"), httpCall(http.MethodGet, "/get", jsonData(t, obj{}), func(resp *httpResponse) error { @@ -124,7 +124,7 @@ func TestHttpEncodeOmitempty(t *testing.T) { } func TestHttpIngress(t *testing.T) { - run(t, + run(t, "", copyModule("httpingress"), deploy("httpingress"), httpCall(http.MethodGet, "/users/123/posts/456", jsonData(t, obj{}), func(resp *httpResponse) error { @@ -243,14 +243,14 @@ func TestHttpIngress(t *testing.T) { } func TestRuntimeReflection(t *testing.T) { - run(t, + run(t, "", copyModule("runtimereflection"), testModule("runtimereflection"), ) } func TestModuleUnitTests(t *testing.T) { - run(t, + run(t, "", copyModule("time"), copyModule("wrapped"), copyModule("verbtypes"), @@ -261,7 +261,7 @@ func TestModuleUnitTests(t *testing.T) { } func TestLease(t *testing.T) { - run(t, + run(t, "", copyModule("leases"), deploy("leases"), func(t testing.TB, ic testContext) error { diff --git a/integration/testdata/go/database/database_test.go b/integration/testdata/go/database/database_test.go index 57a386e2aa..db71701e27 100644 --- a/integration/testdata/go/database/database_test.go +++ b/integration/testdata/go/database/database_test.go @@ -10,6 +10,7 @@ import ( func TestDatabase(t *testing.T) { ctx := ftltest.Context( + ftltest.WithProjectFiles("ftl-project.toml"), ftltest.WithDatabase(db), ) @@ -20,6 +21,7 @@ func TestDatabase(t *testing.T) { assert.Equal(t, "unit test 1", list[0]) ctx = ftltest.Context( + ftltest.WithProjectFiles("ftl-project.toml"), ftltest.WithDatabase(db), ) diff --git a/integration/testdata/go/database/ftl-project.toml b/integration/testdata/go/database/ftl-project.toml new file mode 100644 index 0000000000..092eda798f --- /dev/null +++ b/integration/testdata/go/database/ftl-project.toml @@ -0,0 +1,13 @@ +ftl-min-version = "" + +[global] + +[modules] + [modules.database] + [modules.database.secrets] + FTL_DSN_DATABASE_TESTDB = "inline://InBvc3RncmVzOi8vcG9zdGdyZXM6c2VjcmV0QGxvY2FsaG9zdDo1NDMyMC90ZXN0ZGI_c3NsbW9kZT1kaXNhYmxlIg" + +[executables] + ftl = "" + +[commands] diff --git a/internal/modulecontext/from_environment.go b/internal/modulecontext/from_environment.go deleted file mode 100644 index 54ec7b963d..0000000000 --- a/internal/modulecontext/from_environment.go +++ /dev/null @@ -1,60 +0,0 @@ -package modulecontext - -import ( - "context" - "fmt" - "os" - "strings" -) - -// DatabasesFromEnvironment finds DSNs in environment variables and creates a map of databases. -// -// Environment variables should be in the format FTL_POSTGRES_DSN___ -func DatabasesFromEnvironment(ctx context.Context, module string) (map[string]Database, error) { - databases := map[string]Database{} - for _, entry := range os.Environ() { - if !strings.HasPrefix(entry, "FTL_POSTGRES_DSN_") { - continue - } - parts := strings.SplitN(entry, "=", 2) - if len(parts) != 2 { - return nil, fmt.Errorf("invalid DSN environment variable: %s", entry) - } - key := parts[0] - value := parts[1] - // FTL_POSTGRES_DSN_MODULE_DBNAME - parts = strings.Split(key, "_") - if len(parts) != 5 { - return nil, fmt.Errorf("invalid DSN environment variable: %s", entry) - } - moduleName := parts[3] - dbName := parts[4] - if !strings.EqualFold(moduleName, module) { - continue - } - dbName = strings.ToLower(dbName) - db, err := NewDatabase(DBTypePostgres, value) - if err != nil { - return nil, fmt.Errorf("could not create database %q with DSN %q: %w", dbName, value, err) - } - databases[dbName] = db - } - return databases, nil -} - -// DSNEnvarName returns the name of the environment variable that is expected to hold the DSN for a database. -// -// The format is FTL_POSTGRES_DSN__ -func DSNEnvarName(module, name string) string { - return fmt.Sprintf("FTL_POSTGRES_DSN_%s_%s", strings.ToUpper(module), strings.ToUpper(name)) -} - -// GetDSNFromEnvar returns the DSN for a database from an environment variable. -func GetDSNFromEnvar(module, name string) (string, error) { - envarName := DSNEnvarName(module, name) - dsn, ok := os.LookupEnv(envarName) - if !ok { - return "", fmt.Errorf("missing DSN for database %s: expected to find it at the environment variable %s", name, envarName) - } - return dsn, nil -} diff --git a/internal/modulecontext/from_secrets.go b/internal/modulecontext/from_secrets.go new file mode 100644 index 0000000000..4c84b57482 --- /dev/null +++ b/internal/modulecontext/from_secrets.go @@ -0,0 +1,55 @@ +package modulecontext + +import ( + "context" + "fmt" + "strings" +) + +// DatabasesFromSecrets finds DSNs in environment variables and creates a map of databases. +// +// Environment variables should be in the format FTL_POSTGRES_DSN___ +func DatabasesFromSecrets(ctx context.Context, module string, secrets map[string][]byte) (map[string]Database, error) { + databases := map[string]Database{} + for sName, maybeDSN := range secrets { + if !strings.HasPrefix(sName, "FTL_DSN_") { + continue + } + // FTL_DSN__ + parts := strings.Split(sName, "_") + if len(parts) != 4 { + return nil, fmt.Errorf("invalid DSN secret key %q should have format FTL_DSN_MODULE_DBNAME", sName) + } + moduleName := strings.ToLower(parts[2]) + dbName := strings.ToLower(parts[3]) + if !strings.EqualFold(moduleName, module) { + continue + } + dsnStr := string(maybeDSN) + dsn := dsnStr[1 : len(dsnStr)-1] // chop leading + trailing quotes + db, err := NewDatabase(DBTypePostgres, dsn) + if err != nil { + return nil, fmt.Errorf("could not create database %q with DSN %q: %w", dbName, maybeDSN, err) + } + databases[dbName] = db + } + return databases, nil +} + +// DSNSecretKey returns the key for the secret that is expected to hold the DSN for a database. +// +// The format is FTL_DSN__ +func DSNSecretKey(module, name string) string { + return fmt.Sprintf("FTL_DSN_%s_%s", strings.ToUpper(module), strings.ToUpper(name)) +} + +// GetDSNFromSecret returns the DSN for a database from the relevant secret +func GetDSNFromSecret(module, name string, secrets map[string][]byte) (string, error) { + key := DSNSecretKey(module, name) + dsn, ok := secrets[key] + if !ok { + return "", fmt.Errorf("secrets map %v is missing DSN with key %q", secrets, key) + } + dsnStr := string(dsn) + return dsnStr[1 : len(dsnStr)-1], nil // chop leading + trailing quotes +} diff --git a/internal/modulecontext/from_environment_test.go b/internal/modulecontext/from_secrets_test.go similarity index 75% rename from internal/modulecontext/from_environment_test.go rename to internal/modulecontext/from_secrets_test.go index 2bd84fc7c6..971b99b09b 100644 --- a/internal/modulecontext/from_environment_test.go +++ b/internal/modulecontext/from_secrets_test.go @@ -9,11 +9,13 @@ import ( "github.com/alecthomas/assert/v2" ) -func TestFromEnvironment(t *testing.T) { +func TestFromSecrets(t *testing.T) { ctx := log.ContextWithNewDefaultLogger(context.Background()) - t.Setenv(DSNEnvarName("echo", "echo"), "postgres://echo:echo@localhost:5432/echo") - databases, err := DatabasesFromEnvironment(ctx, "echo") + secrets := map[string][]byte{ + "FTL_DSN_ECHO_ECHO": []byte("\"postgres://echo:echo@localhost:5432/echo\""), + } + databases, err := DatabasesFromSecrets(ctx, "echo", secrets) assert.NoError(t, err) response := NewBuilder("echo").AddDatabases(databases).Build().ToProto()