From 063dcf6e32ead83bf653c7b0e2a8872df87a7675 Mon Sep 17 00:00:00 2001 From: avallete Date: Wed, 23 Oct 2024 18:26:01 +0200 Subject: [PATCH 1/8] feat(cli): allow local postgres.conf configuration Closes: #2611 --- internal/db/start/start.go | 18 +++++++---- pkg/config/db.go | 61 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 73 insertions(+), 6 deletions(-) diff --git a/internal/db/start/start.go b/internal/db/start/start.go index 826d46c86..a300f5594 100644 --- a/internal/db/start/start.go +++ b/internal/db/start/start.go @@ -56,7 +56,6 @@ func NewContainerConfig() container.Config { env := []string{ "POSTGRES_PASSWORD=" + utils.Config.Db.Password, "POSTGRES_HOST=/var/run/postgresql", - "POSTGRES_INITDB_ARGS=--lc-ctype=C.UTF-8", "JWT_SECRET=" + utils.Config.Auth.JwtSecret, fmt.Sprintf("JWT_EXP=%d", utils.Config.Auth.JwtExpiry), } @@ -81,13 +80,18 @@ func NewContainerConfig() container.Config { Timeout: 2 * time.Second, Retries: 3, }, - Entrypoint: []string{"sh", "-c", `cat <<'EOF' > /etc/postgresql.schema.sql && cat <<'EOF' > /etc/postgresql-custom/pgsodium_root.key && docker-entrypoint.sh postgres -D /etc/postgresql + Entrypoint: []string{"sh", "-c", ` +cat <<'EOF' > /etc/postgresql.schema.sql && \ +cat <<'EOF' > /etc/postgresql-custom/pgsodium_root.key && \ +cat <<'EOF' >> /etc/postgresql/postgresql.conf && \ +docker-entrypoint.sh postgres -D /etc/postgresql ` + initialSchema + ` ` + _supabaseSchema + ` EOF ` + utils.Config.Db.RootKey + ` EOF -`}, +` + utils.Config.Db.Settings.ToPostgresConfig() + ` +EOF`}, } if utils.Config.Db.MajorVersion >= 14 { config.Cmd = []string{"postgres", @@ -124,11 +128,13 @@ func StartDatabase(ctx context.Context, fsys afero.Fs, w io.Writer, options ...f } if utils.Config.Db.MajorVersion <= 14 { config.Entrypoint = []string{"sh", "-c", ` - cat <<'EOF' > /docker-entrypoint-initdb.d/supabase_schema.sql +cat <<'EOF' > /docker-entrypoint-initdb.d/supabase_schema.sql && \ +cat <<'EOF' >> /etc/postgresql/postgresql.conf && \ +docker-entrypoint.sh postgres -D /etc/postgresql ` + _supabaseSchema + ` EOF - docker-entrypoint.sh postgres -D /etc/postgresql - `} +` + utils.Config.Db.Settings.ToPostgresConfig() + ` +EOF`} hostConfig.Tmpfs = map[string]string{"/docker-entrypoint-initdb.d": ""} } // Creating volume will not override existing volume, so we must inspect explicitly diff --git a/pkg/config/db.go b/pkg/config/db.go index 89e5bfd24..6620c301f 100644 --- a/pkg/config/db.go +++ b/pkg/config/db.go @@ -1,6 +1,10 @@ package config import ( + "fmt" + "reflect" + "strings" + "github.com/google/go-cmp/cmp" v1API "github.com/supabase/cli/pkg/api" "github.com/supabase/cli/pkg/cast" @@ -146,6 +150,63 @@ func (a *settings) fromRemoteConfig(remoteConfig v1API.PostgresConfigResponse) s return result } +func (a *settings) ToPostgresConfig() string { + // create a valid string to append to /etc/postgresql/postgresql.conf + var config strings.Builder + + // Helper function to add a setting if it's not nil + addSetting := func(name string, value interface{}) { + if value != nil { + switch v := value.(type) { + case *string: + if v != nil { + fmt.Fprintf(&config, "%s = '%s'\n", name, *v) + } + case *int: + if v != nil { + fmt.Fprintf(&config, "%s = %d\n", name, *v) + } + case *SessionReplicationRole: + if v != nil { + fmt.Fprintf(&config, "%s = '%s'\n", name, string(*v)) + } + default: + // For other types, only add if it's a non-nil pointer + if reflect.ValueOf(value).Kind() == reflect.Ptr && !reflect.ValueOf(value).IsNil() { + fmt.Fprintf(&config, "%s = %v\n", name, reflect.ValueOf(value).Elem().Interface()) + } + } + } + } + + // Add all the settings + addSetting("effective_cache_size", a.EffectiveCacheSize) + addSetting("logical_decoding_work_mem", a.LogicalDecodingWorkMem) + addSetting("maintenance_work_mem", a.MaintenanceWorkMem) + addSetting("max_connections", a.MaxConnections) + addSetting("max_locks_per_transaction", a.MaxLocksPerTransaction) + addSetting("max_parallel_maintenance_workers", a.MaxParallelMaintenanceWorkers) + addSetting("max_parallel_workers", a.MaxParallelWorkers) + addSetting("max_parallel_workers_per_gather", a.MaxParallelWorkersPerGather) + addSetting("max_replication_slots", a.MaxReplicationSlots) + addSetting("max_slot_wal_keep_size", a.MaxSlotWalKeepSize) + addSetting("max_standby_archive_delay", a.MaxStandbyArchiveDelay) + addSetting("max_standby_streaming_delay", a.MaxStandbyStreamingDelay) + addSetting("max_wal_senders", a.MaxWalSenders) + addSetting("max_wal_size", a.MaxWalSize) + addSetting("max_worker_processes", a.MaxWorkerProcesses) + if a.SessionReplicationRole != nil { + addSetting("session_replication_role", string(*a.SessionReplicationRole)) + } + addSetting("shared_buffers", a.SharedBuffers) + addSetting("statement_timeout", a.StatementTimeout) + addSetting("wal_keep_size", a.WalKeepSize) + addSetting("wal_sender_timeout", a.WalSenderTimeout) + addSetting("work_mem", a.WorkMem) + + return config.String() +} + func (a *settings) DiffWithRemote(remoteConfig v1API.PostgresConfigResponse) ([]byte, error) { // Convert the config values into easily comparable remoteConfig values currentValue, err := ToTomlBytes(a) From b8c121d036448ae256514e16ee6833b730d5080f Mon Sep 17 00:00:00 2001 From: avallete Date: Wed, 23 Oct 2024 18:32:03 +0200 Subject: [PATCH 2/8] chore: test ToPostgresConfig --- pkg/config/db.go | 6 ++---- pkg/config/db_test.go | 38 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 40 insertions(+), 4 deletions(-) diff --git a/pkg/config/db.go b/pkg/config/db.go index 6620c301f..3bcf80a00 100644 --- a/pkg/config/db.go +++ b/pkg/config/db.go @@ -150,8 +150,8 @@ func (a *settings) fromRemoteConfig(remoteConfig v1API.PostgresConfigResponse) s return result } +// create a valid string to append to /etc/postgresql/postgresql.conf func (a *settings) ToPostgresConfig() string { - // create a valid string to append to /etc/postgresql/postgresql.conf var config strings.Builder // Helper function to add a setting if it's not nil @@ -195,9 +195,7 @@ func (a *settings) ToPostgresConfig() string { addSetting("max_wal_senders", a.MaxWalSenders) addSetting("max_wal_size", a.MaxWalSize) addSetting("max_worker_processes", a.MaxWorkerProcesses) - if a.SessionReplicationRole != nil { - addSetting("session_replication_role", string(*a.SessionReplicationRole)) - } + addSetting("session_replication_role", a.SessionReplicationRole) addSetting("shared_buffers", a.SharedBuffers) addSetting("statement_timeout", a.StatementTimeout) addSetting("wal_keep_size", a.WalKeepSize) diff --git a/pkg/config/db_test.go b/pkg/config/db_test.go index e7c573475..283fbb15a 100644 --- a/pkg/config/db_test.go +++ b/pkg/config/db_test.go @@ -153,3 +153,41 @@ func TestDbSettingsDiffWithRemote(t *testing.T) { assert.Contains(t, string(diff), "-shared_buffers = \"1GB\"") }) } + +func TestSettingsToPostgresConfig(t *testing.T) { + t.Run("Only set values should appear", func(t *testing.T) { + settings := settings{ + MaxConnections: cast.Ptr(uint(100)), + MaxLocksPerTransaction: cast.Ptr(uint(64)), + SharedBuffers: cast.Ptr("128MB"), + WorkMem: cast.Ptr("4MB"), + } + got := settings.ToPostgresConfig() + + assert.Contains(t, got, "max_connections = 100") + assert.Contains(t, got, "max_locks_per_transaction = 64") + assert.Contains(t, got, "shared_buffers = '128MB'") + assert.Contains(t, got, "work_mem = '4MB'") + + assert.NotContains(t, got, "effective_cache_size") + assert.NotContains(t, got, "maintenance_work_mem") + assert.NotContains(t, got, "max_parallel_workers") + }) + + t.Run("SessionReplicationRole should be handled correctly", func(t *testing.T) { + settings := settings{ + SessionReplicationRole: cast.Ptr(SessionReplicationRoleOrigin), + } + got := settings.ToPostgresConfig() + + assert.Contains(t, got, "session_replication_role = 'origin'") + }) + + t.Run("Empty settings should result in empty string", func(t *testing.T) { + settings := settings{} + got := settings.ToPostgresConfig() + + assert.Empty(t, got) + assert.NotContains(t, got, "=") + }) +} From f47fb1b0e52333807e5bf9fa376f717c5bdc91b8 Mon Sep 17 00:00:00 2001 From: avallete Date: Wed, 23 Oct 2024 18:33:59 +0200 Subject: [PATCH 3/8] chore: add start runtime test --- internal/db/start/start_test.go | 47 +++++++++++++++++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git a/internal/db/start/start_test.go b/internal/db/start/start_test.go index 96d97da8c..4c29a0a4b 100644 --- a/internal/db/start/start_test.go +++ b/internal/db/start/start_test.go @@ -17,6 +17,7 @@ import ( "github.com/supabase/cli/internal/testing/apitest" "github.com/supabase/cli/internal/testing/fstest" "github.com/supabase/cli/internal/utils" + "github.com/supabase/cli/pkg/cast" "github.com/supabase/cli/pkg/pgtest" ) @@ -308,3 +309,49 @@ func TestSetupDatabase(t *testing.T) { assert.Empty(t, apitest.ListUnmatchedRequests()) }) } +func TestStartDatabaseWithCustomSettings(t *testing.T) { + t.Run("starts database with custom MaxConnections", func(t *testing.T) { + // Setup + utils.Config.Db.MajorVersion = 15 + utils.DbId = "supabase_db_test" + utils.ConfigId = "supabase_config_test" + utils.Config.Db.Port = 5432 + utils.Config.Db.Settings.MaxConnections = cast.Ptr(uint(200)) + + // Setup in-memory fs + fsys := afero.NewMemMapFs() + + // Setup mock docker + require.NoError(t, apitest.MockDocker(utils.Docker)) + defer gock.OffAll() + gock.New(utils.Docker.DaemonHost()). + Get("/v" + utils.Docker.ClientVersion() + "/volumes/" + utils.DbId). + Reply(http.StatusNotFound). + JSON(volume.Volume{}) + apitest.MockDockerStart(utils.Docker, utils.GetRegistryImageUrl(utils.Config.Db.Image), utils.DbId) + gock.New(utils.Docker.DaemonHost()). + Get("/v" + utils.Docker.ClientVersion() + "/containers/" + utils.DbId + "/json"). + Reply(http.StatusOK). + JSON(types.ContainerJSON{ContainerJSONBase: &types.ContainerJSONBase{ + State: &types.ContainerState{ + Running: true, + Health: &types.Health{Status: types.Healthy}, + }, + }}) + + // Setup mock postgres + conn := pgtest.NewConn() + defer conn.Close(t) + + // Run test + err := StartDatabase(context.Background(), fsys, io.Discard, conn.Intercept) + + // Check error + assert.NoError(t, err) + assert.Empty(t, apitest.ListUnmatchedRequests()) + + // Check if the custom MaxConnections setting was applied + config := NewContainerConfig() + assert.Contains(t, config.Entrypoint[2], "max_connections = 200") + }) +} From 0cf4aca332868bfe0489e13b8f7164902ef9398d Mon Sep 17 00:00:00 2001 From: avallete Date: Wed, 23 Oct 2024 18:41:51 +0200 Subject: [PATCH 4/8] chore: use less max_connections --- internal/db/start/start_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/db/start/start_test.go b/internal/db/start/start_test.go index 4c29a0a4b..f2105a2d1 100644 --- a/internal/db/start/start_test.go +++ b/internal/db/start/start_test.go @@ -316,7 +316,7 @@ func TestStartDatabaseWithCustomSettings(t *testing.T) { utils.DbId = "supabase_db_test" utils.ConfigId = "supabase_config_test" utils.Config.Db.Port = 5432 - utils.Config.Db.Settings.MaxConnections = cast.Ptr(uint(200)) + utils.Config.Db.Settings.MaxConnections = cast.Ptr(uint(50)) // Setup in-memory fs fsys := afero.NewMemMapFs() @@ -352,6 +352,6 @@ func TestStartDatabaseWithCustomSettings(t *testing.T) { // Check if the custom MaxConnections setting was applied config := NewContainerConfig() - assert.Contains(t, config.Entrypoint[2], "max_connections = 200") + assert.Contains(t, config.Entrypoint[2], "max_connections = 50") }) } From de420f3044ea89994162aea89953559299968ba5 Mon Sep 17 00:00:00 2001 From: avallete Date: Thu, 24 Oct 2024 09:42:14 +0200 Subject: [PATCH 5/8] fix: db start tests --- internal/db/start/start_test.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/internal/db/start/start_test.go b/internal/db/start/start_test.go index f2105a2d1..475562f2f 100644 --- a/internal/db/start/start_test.go +++ b/internal/db/start/start_test.go @@ -339,6 +339,12 @@ func TestStartDatabaseWithCustomSettings(t *testing.T) { }, }}) + apitest.MockDockerStart(utils.Docker, utils.GetRegistryImageUrl(utils.Config.Realtime.Image), "test-realtime") + require.NoError(t, apitest.MockDockerLogs(utils.Docker, "test-realtime", "")) + apitest.MockDockerStart(utils.Docker, utils.GetRegistryImageUrl(utils.Config.Storage.Image), "test-storage") + require.NoError(t, apitest.MockDockerLogs(utils.Docker, "test-storage", "")) + apitest.MockDockerStart(utils.Docker, utils.GetRegistryImageUrl(utils.Config.Auth.Image), "test-auth") + require.NoError(t, apitest.MockDockerLogs(utils.Docker, "test-auth", "")) // Setup mock postgres conn := pgtest.NewConn() defer conn.Close(t) From f126854348aad71d6430cde372fd103f664398d3 Mon Sep 17 00:00:00 2001 From: avallete Date: Thu, 24 Oct 2024 13:49:34 +0200 Subject: [PATCH 6/8] chore: add prefix to postgres config --- pkg/config/db.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/config/db.go b/pkg/config/db.go index 3bcf80a00..e56dd5907 100644 --- a/pkg/config/db.go +++ b/pkg/config/db.go @@ -153,6 +153,7 @@ func (a *settings) fromRemoteConfig(remoteConfig v1API.PostgresConfigResponse) s // create a valid string to append to /etc/postgresql/postgresql.conf func (a *settings) ToPostgresConfig() string { var config strings.Builder + fmt.Fprint(&config, "\n#supabase [db.settings] configuration\n") // Helper function to add a setting if it's not nil addSetting := func(name string, value interface{}) { From 15369added372a48e51e92823fd4279f76ca0b6a Mon Sep 17 00:00:00 2001 From: avallete Date: Thu, 24 Oct 2024 14:06:12 +0200 Subject: [PATCH 7/8] fix: test --- pkg/config/db_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/config/db_test.go b/pkg/config/db_test.go index 283fbb15a..70424e956 100644 --- a/pkg/config/db_test.go +++ b/pkg/config/db_test.go @@ -187,7 +187,7 @@ func TestSettingsToPostgresConfig(t *testing.T) { settings := settings{} got := settings.ToPostgresConfig() - assert.Empty(t, got) + assert.Equal(t, got, "\n#supabase [db.settings] configuration\n") assert.NotContains(t, got, "=") }) } From ac9f39c4a366cb8b5286df3f2acc26aa6c3b534b Mon Sep 17 00:00:00 2001 From: Qiao Han Date: Thu, 24 Oct 2024 20:59:18 +0800 Subject: [PATCH 8/8] chore: serialise postgres conf as toml --- pkg/config/db.go | 63 ++++++------------------------------------- pkg/config/db_test.go | 2 +- 2 files changed, 9 insertions(+), 56 deletions(-) diff --git a/pkg/config/db.go b/pkg/config/db.go index e56dd5907..e7c5f820b 100644 --- a/pkg/config/db.go +++ b/pkg/config/db.go @@ -1,9 +1,7 @@ package config import ( - "fmt" - "reflect" - "strings" + "bytes" "github.com/google/go-cmp/cmp" v1API "github.com/supabase/cli/pkg/api" @@ -150,60 +148,15 @@ func (a *settings) fromRemoteConfig(remoteConfig v1API.PostgresConfigResponse) s return result } +const pgConfHeader = "\n# supabase [db.settings] configuration\n" + // create a valid string to append to /etc/postgresql/postgresql.conf func (a *settings) ToPostgresConfig() string { - var config strings.Builder - fmt.Fprint(&config, "\n#supabase [db.settings] configuration\n") - - // Helper function to add a setting if it's not nil - addSetting := func(name string, value interface{}) { - if value != nil { - switch v := value.(type) { - case *string: - if v != nil { - fmt.Fprintf(&config, "%s = '%s'\n", name, *v) - } - case *int: - if v != nil { - fmt.Fprintf(&config, "%s = %d\n", name, *v) - } - case *SessionReplicationRole: - if v != nil { - fmt.Fprintf(&config, "%s = '%s'\n", name, string(*v)) - } - default: - // For other types, only add if it's a non-nil pointer - if reflect.ValueOf(value).Kind() == reflect.Ptr && !reflect.ValueOf(value).IsNil() { - fmt.Fprintf(&config, "%s = %v\n", name, reflect.ValueOf(value).Elem().Interface()) - } - } - } - } - - // Add all the settings - addSetting("effective_cache_size", a.EffectiveCacheSize) - addSetting("logical_decoding_work_mem", a.LogicalDecodingWorkMem) - addSetting("maintenance_work_mem", a.MaintenanceWorkMem) - addSetting("max_connections", a.MaxConnections) - addSetting("max_locks_per_transaction", a.MaxLocksPerTransaction) - addSetting("max_parallel_maintenance_workers", a.MaxParallelMaintenanceWorkers) - addSetting("max_parallel_workers", a.MaxParallelWorkers) - addSetting("max_parallel_workers_per_gather", a.MaxParallelWorkersPerGather) - addSetting("max_replication_slots", a.MaxReplicationSlots) - addSetting("max_slot_wal_keep_size", a.MaxSlotWalKeepSize) - addSetting("max_standby_archive_delay", a.MaxStandbyArchiveDelay) - addSetting("max_standby_streaming_delay", a.MaxStandbyStreamingDelay) - addSetting("max_wal_senders", a.MaxWalSenders) - addSetting("max_wal_size", a.MaxWalSize) - addSetting("max_worker_processes", a.MaxWorkerProcesses) - addSetting("session_replication_role", a.SessionReplicationRole) - addSetting("shared_buffers", a.SharedBuffers) - addSetting("statement_timeout", a.StatementTimeout) - addSetting("wal_keep_size", a.WalKeepSize) - addSetting("wal_sender_timeout", a.WalSenderTimeout) - addSetting("work_mem", a.WorkMem) - - return config.String() + // Assuming postgres settings is always a flat struct, we can serialise + // using toml, then replace double quotes with single. + data, _ := ToTomlBytes(*a) + body := bytes.ReplaceAll(data, []byte{'"'}, []byte{'\''}) + return pgConfHeader + string(body) } func (a *settings) DiffWithRemote(remoteConfig v1API.PostgresConfigResponse) ([]byte, error) { diff --git a/pkg/config/db_test.go b/pkg/config/db_test.go index 70424e956..8d70ec21b 100644 --- a/pkg/config/db_test.go +++ b/pkg/config/db_test.go @@ -187,7 +187,7 @@ func TestSettingsToPostgresConfig(t *testing.T) { settings := settings{} got := settings.ToPostgresConfig() - assert.Equal(t, got, "\n#supabase [db.settings] configuration\n") + assert.Equal(t, got, "\n# supabase [db.settings] configuration\n") assert.NotContains(t, got, "=") }) }