From 936efa4df3e5921b1c6bd79b8ab4b563b0336ee5 Mon Sep 17 00:00:00 2001 From: Bobbie Soedirgo Date: Wed, 3 Aug 2022 14:00:18 +0800 Subject: [PATCH 1/2] fix(config): interpolate env on load always Reverts https://github.com/supabase/cli/commit/f8d5613279d060f14883512200915b2ca340a1be Don't quite recall why we only interpolate on `supabase start`. --- internal/start/start.go | 3 -- internal/utils/config.go | 82 ++++++++++++++++------------------- internal/utils/config_test.go | 4 +- 3 files changed, 38 insertions(+), 51 deletions(-) diff --git a/internal/start/start.go b/internal/start/start.go index 4be3111e10..70cea41614 100644 --- a/internal/start/start.go +++ b/internal/start/start.go @@ -38,9 +38,6 @@ func Run() error { if err := utils.LoadConfig(); err != nil { return err } - if err := utils.InterpolateEnvInConfig(); err != nil { - return err - } if err := utils.AssertSupabaseStartIsRunning(); err == nil { return errors.New(utils.Aqua("supabase start") + " is already running. Try running " + utils.Aqua("supabase stop") + " first.") } diff --git a/internal/utils/config.go b/internal/utils/config.go index 26d8d9384b..c6b71b988e 100644 --- a/internal/utils/config.go +++ b/internal/utils/config.go @@ -228,56 +228,48 @@ func LoadConfigFS(fsys afero.Fs) error { ClientId: "", Secret: "", } - } - } - } - - return nil -} - -func InterpolateEnvInConfig() error { - maybeLoadEnv := func(s string) (string, error) { - matches := regexp.MustCompile(`^env\((.*)\)$`).FindStringSubmatch(s) - if len(matches) == 0 { - return s, nil - } - - envName := matches[1] - value := os.Getenv(envName) - if value == "" { - return "", errors.New(`Error evaluating "env(` + envName + `)": environment variable ` + envName + " is unset.") - } - - return value, nil - } + } else if Config.Auth.External[ext].Enabled { + maybeLoadEnv := func(s string) (string, error) { + matches := regexp.MustCompile(`^env\((.*)\)$`).FindStringSubmatch(s) + if len(matches) == 0 { + return s, nil + } + + envName := matches[1] + value := os.Getenv(envName) + if value == "" { + return "", errors.New(`Error evaluating "env(` + envName + `)": environment variable ` + envName + " is unset.") + } + + return value, nil + } - for _, ext := range authExternalProviders { - if Config.Auth.External[ext].Enabled { - var clientId, secret string + var clientId, secret string - if Config.Auth.External[ext].ClientId == "" { - return fmt.Errorf("Missing required field in config: auth.external.%s.client_id", ext) - } else { - v, err := maybeLoadEnv(Config.Auth.External[ext].ClientId) - if err != nil { - return err + if Config.Auth.External[ext].ClientId == "" { + return fmt.Errorf("Missing required field in config: auth.external.%s.client_id", ext) + } else { + v, err := maybeLoadEnv(Config.Auth.External[ext].ClientId) + if err != nil { + return err + } + clientId = v } - clientId = v - } - if Config.Auth.External[ext].Secret == "" { - return fmt.Errorf("Missing required field in config: auth.external.%s.secret", ext) - } else { - v, err := maybeLoadEnv(Config.Auth.External[ext].Secret) - if err != nil { - return err + if Config.Auth.External[ext].Secret == "" { + return fmt.Errorf("Missing required field in config: auth.external.%s.secret", ext) + } else { + v, err := maybeLoadEnv(Config.Auth.External[ext].Secret) + if err != nil { + return err + } + secret = v } - secret = v - } - Config.Auth.External[ext] = provider{ - Enabled: true, - ClientId: clientId, - Secret: secret, + Config.Auth.External[ext] = provider{ + Enabled: true, + ClientId: clientId, + Secret: secret, + } } } } diff --git a/internal/utils/config_test.go b/internal/utils/config_test.go index cffc80bfc8..df1a92e2b1 100644 --- a/internal/utils/config_test.go +++ b/internal/utils/config_test.go @@ -21,7 +21,6 @@ func TestConfigParsing(t *testing.T) { t.Setenv("AZURE_CLIENT_ID", "hello") t.Setenv("AZURE_SECRET", "this is cool") assert.NoError(t, LoadConfigFS(fsys)) - assert.NoError(t, InterpolateEnvInConfig()) assert.Equal(t, "hello", Config.Auth.External["azure"].ClientId) assert.Equal(t, "this is cool", Config.Auth.External["azure"].Secret) @@ -30,7 +29,6 @@ func TestConfigParsing(t *testing.T) { t.Run("config file with environment variables fails when unset", func(t *testing.T) { fsys := afero.NewMemMapFs() assert.NoError(t, WriteConfig(fsys, true)) - assert.NoError(t, LoadConfigFS(fsys)) - assert.Error(t, InterpolateEnvInConfig()) + assert.Error(t, LoadConfigFS(fsys)) }) } From 9530b05cd2b812b3bfcb2d430255c2ac4a275dac Mon Sep 17 00:00:00 2001 From: Bobbie Soedirgo Date: Wed, 3 Aug 2022 14:02:10 +0800 Subject: [PATCH 2/2] test(db/remote/set): use WriteConfig w/ test=false Otherwise it gives: === RUN TestDbRemoteSetCommand/sets_the_remote_database_url set_test.go:33: Error Trace: /Users/soedirgo/repos/cli/internal/db/remote/set/set_test.go:33 Error: Received unexpected error: Error evaluating "env(AZURE_CLIENT_ID)": environment variable AZURE_CLIENT_ID is unset. Test: TestDbRemoteSetCommand/sets_the_remote_database_url FAIL github.com/supabase/cli/internal/db/remote/set 104.862s from interpolating config env. --- internal/db/remote/set/set_test.go | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/internal/db/remote/set/set_test.go b/internal/db/remote/set/set_test.go index e5633c811d..5475f567b1 100644 --- a/internal/db/remote/set/set_test.go +++ b/internal/db/remote/set/set_test.go @@ -17,7 +17,7 @@ func TestDbRemoteSetCommand(t *testing.T) { t.Run("sets the remote database url", func(t *testing.T) { // Setup in-memory fs fsys := afero.NewMemMapFs() - require.NoError(t, utils.WriteConfig(fsys, true)) + require.NoError(t, utils.WriteConfig(fsys, false)) // Setup initial migration version := "20220727064247" _, err := fsys.Create("supabase/migrations/" + version + "_init.sql") @@ -36,7 +36,7 @@ func TestDbRemoteSetCommand(t *testing.T) { t.Run("creates migrations table if absent", func(t *testing.T) { // Setup in-memory fs fsys := afero.NewMemMapFs() - require.NoError(t, utils.WriteConfig(fsys, true)) + require.NoError(t, utils.WriteConfig(fsys, false)) // Setup mock postgres conn := pgtest.NewConn() defer conn.Close(t) @@ -61,7 +61,7 @@ func TestDbRemoteSetCommand(t *testing.T) { t.Run("throws error on invalid postgres url", func(t *testing.T) { // Setup in-memory fs fsys := afero.NewMemMapFs() - require.NoError(t, utils.WriteConfig(fsys, true)) + require.NoError(t, utils.WriteConfig(fsys, false)) // Run test assert.Error(t, Run("invalid", fsys)) }) @@ -69,7 +69,7 @@ func TestDbRemoteSetCommand(t *testing.T) { t.Run("throws error on failture to connect", func(t *testing.T) { // Setup in-memory fs fsys := afero.NewMemMapFs() - require.NoError(t, utils.WriteConfig(fsys, true)) + require.NoError(t, utils.WriteConfig(fsys, false)) // Run test assert.Error(t, Run(postgresUrl, fsys)) }) @@ -77,7 +77,7 @@ func TestDbRemoteSetCommand(t *testing.T) { t.Run("throws error on missing server version", func(t *testing.T) { // Setup in-memory fs fsys := afero.NewMemMapFs() - require.NoError(t, utils.WriteConfig(fsys, true)) + require.NoError(t, utils.WriteConfig(fsys, false)) // Setup mock postgres conn := pgtest.NewWithStatus(map[string]string{ "standard_conforming_strings": "on", @@ -90,7 +90,7 @@ func TestDbRemoteSetCommand(t *testing.T) { t.Run("throws error on unsupported server version", func(t *testing.T) { // Setup in-memory fs fsys := afero.NewMemMapFs() - require.NoError(t, utils.WriteConfig(fsys, true)) + require.NoError(t, utils.WriteConfig(fsys, false)) // Setup mock postgres conn := pgtest.NewWithStatus(map[string]string{ "standard_conforming_strings": "on", @@ -104,7 +104,7 @@ func TestDbRemoteSetCommand(t *testing.T) { t.Run("throws error on failure to create table", func(t *testing.T) { // Setup in-memory fs fsys := afero.NewMemMapFs() - require.NoError(t, utils.WriteConfig(fsys, true)) + require.NoError(t, utils.WriteConfig(fsys, false)) // Setup mock postgres conn := pgtest.NewConn() defer conn.Close(t) @@ -119,7 +119,7 @@ func TestDbRemoteSetCommand(t *testing.T) { t.Run("throws error on failure to list migrations", func(t *testing.T) { // Setup in-memory fs fsys := afero.NewMemMapFs() - require.NoError(t, utils.WriteConfig(fsys, true)) + require.NoError(t, utils.WriteConfig(fsys, false)) // Setup mock postgres conn := pgtest.NewConn() defer conn.Close(t) @@ -134,7 +134,7 @@ func TestDbRemoteSetCommand(t *testing.T) { t.Run("throws error on migration mismatch", func(t *testing.T) { // Setup in-memory fs fsys := afero.NewMemMapFs() - require.NoError(t, utils.WriteConfig(fsys, true)) + require.NoError(t, utils.WriteConfig(fsys, false)) // Setup mock postgres conn := pgtest.NewConn() defer conn.Close(t) @@ -149,7 +149,7 @@ func TestDbRemoteSetCommand(t *testing.T) { t.Run("throws error on malformed file name", func(t *testing.T) { // Setup in-memory fs fsys := afero.NewMemMapFs() - require.NoError(t, utils.WriteConfig(fsys, true)) + require.NoError(t, utils.WriteConfig(fsys, false)) // Setup initial migration version := "20220727064247" _, err := fsys.Create("supabase/migrations/" + version + ".sql") @@ -168,7 +168,7 @@ func TestDbRemoteSetCommand(t *testing.T) { t.Run("throws error on failure to create directory", func(t *testing.T) { // Setup in-memory fs fsys := afero.NewMemMapFs() - require.NoError(t, utils.WriteConfig(fsys, true)) + require.NoError(t, utils.WriteConfig(fsys, false)) // Setup mock postgres conn := pgtest.NewConn() defer conn.Close(t)