Validate Postres config (#156)

* Validate config before applying

* Cleanup

* Add extension verification

* Fleshing things out

* lint fixes

Author: davissp14

internal/api/handle_admin.go

@@ -150,15 +150,14 @@ func handleUpdatePostgresSettings(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
+	// Logistical PG setting validations.
+	requestedChanges, err = node.PGConfig.Validate(r.Context(), conn, requestedChanges)
+	if err != nil {
+		renderErr(w, err)
+		return
+	}
+
 	for k, v := range requestedChanges {
-		exists, err := admin.SettingExists(r.Context(), conn, k)
-		if err != nil {
-			renderErr(w, err)
-		}
-		if !exists {
-			renderErr(w, fmt.Errorf("invalid config option: %s", k))
-			return
-		}
 		cfg[k] = v
 	}
 

internal/flypg/admin/admin.go

@@ -337,6 +337,15 @@ func SettingExists(ctx context.Context, pg *pgx.Conn, setting string) (bool, err
 	return out, nil
 }
 
+func ExtensionAvailable(ctx context.Context, pg *pgx.Conn, extension string) (bool, error) {
+	sql := fmt.Sprintf("SELECT EXISTS(SELECT 1 FROM pg_available_extensions WHERE name='%s')", extension)
+	var out bool
+	if err := pg.QueryRow(ctx, sql).Scan(&out); err != nil {
+		return false, err
+	}
+	return out, nil
+}
+
 func SettingRequiresRestart(ctx context.Context, pg *pgx.Conn, setting string) (bool, error) {
 	sql := fmt.Sprintf("SELECT pending_restart FROM pg_settings WHERE name='%s'", setting)
 	row := pg.QueryRow(ctx, sql)
@@ -370,3 +379,33 @@ func GetSetting(ctx context.Context, pg *pgx.Conn, setting string) (*PGSetting,
 	}
 	return &out, nil
 }
+
+func ValidatePGSettings(ctx context.Context, conn *pgx.Conn, requested map[string]interface{}) error {
+	for k, v := range requested {
+		exists, err := SettingExists(ctx, conn, k)
+		if err != nil {
+			return fmt.Errorf("failed to verify setting: %s", err)
+		}
+		if !exists {
+			return fmt.Errorf("setting %v is not a valid config option", k)
+		}
+
+		// Verify specified extensions are installed
+		if k == "shared_preload_libraries" {
+			extensions := strings.Trim(v.(string), "'")
+			extSlice := strings.Split(extensions, ",")
+			for _, e := range extSlice {
+				available, err := ExtensionAvailable(ctx, conn, e)
+				if err != nil {
+					return fmt.Errorf("failed to verify pg extension %s: %s", e, err)
+				}
+
+				if !available {
+					return fmt.Errorf("extension %s has not been installed within this image", e)
+				}
+			}
+		}
+	}
+
+	return nil
+}

internal/flypg/pg.go

@@ -3,6 +3,7 @@ package flypg
 import (
 	"context"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"io"
 	"log"
@@ -167,7 +168,7 @@ func (c *PGConfig) SetDefaults() error {
 		"wal_level":                "replica",
 		"wal_log_hints":            true,
 		"hot_standby":              true,
-		"archive_mode":             true,
+		"archive_mode":             "on",
 		"archive_command":          "'/bin/true'",
 		"shared_preload_libraries": fmt.Sprintf("'%s'", strings.Join(sharedPreloadLibraries, ",")),
 	}
@@ -273,6 +274,75 @@ func (c *PGConfig) writePasswordFile(pwd string) error {
 	return nil
 }
 
+func (c *PGConfig) Validate(ctx context.Context, conn *pgx.Conn, requested ConfigMap) (ConfigMap, error) {
+	if err := admin.ValidatePGSettings(ctx, conn, requested); err != nil {
+		return requested, err
+	}
+
+	return c.validateCompatibility(requested)
+}
+
+func (c *PGConfig) validateCompatibility(requested ConfigMap) (ConfigMap, error) {
+	current, err := c.CurrentConfig()
+	if err != nil {
+		return requested, fmt.Errorf("failed to resolve current config: %s", err)
+	}
+
+	// Shared preload libraries
+	if v, ok := requested["shared_preload_libraries"]; ok {
+		val := v.(string)
+
+		// Remove any formatting that may be applied
+		val = strings.Trim(val, "'")
+		val = strings.TrimSpace(val)
+		if val == "" {
+			return requested, errors.New("`shared_preload_libraries` must contain the `repmgr` extension")
+		}
+
+		// Confirm repmgr is specified
+		repmgrPresent := false
+		entries := strings.Split(val, ",")
+		for _, entry := range entries {
+			if entry == "repmgr" {
+				repmgrPresent = true
+				break
+			}
+		}
+
+		if !repmgrPresent {
+			return requested, errors.New("`shared_preload_libraries` must contain the `repmgr` extension")
+		}
+
+		// Reconstruct value with proper formatting
+		requested["shared_preload_libraries"] = fmt.Sprintf("'%s'", val)
+	}
+
+	// Wal-level
+	if v, ok := requested["wal_level"]; ok {
+		value := v.(string)
+
+		// Postgres will not boot properly if minimal is set with archive_mode enabled.
+		if value == "minimal" {
+			valid := false
+			if val, ok := requested["archive_mode"]; ok {
+				if val == "off" {
+					valid = true
+				}
+			}
+
+			if !valid && current["archive_mode"] == "off" {
+				valid = true
+			}
+
+			if !valid {
+				return requested, errors.New("archive_mode must be set to `off` before wal_level can be set to `minimal`")
+			}
+		}
+	}
+
+	return requested, nil
+}
+
 type HBAEntry struct {
 	Type     string
 	Database string

internal/flypg/pg_test.go

@@ -255,6 +255,122 @@ func setup(t *testing.T) error {
 	return nil
 }
 
+func TestValidateCompatibility(t *testing.T) {
+	if err := setup(t); err != nil {
+		t.Fatal(err)
+	}
+	defer cleanup()
+
+	pgConf := &PGConfig{
+		DataDir:                pgTestDirectory,
+		Port:                   5433,
+		ConfigFilePath:         pgConfigFilePath,
+		InternalConfigFilePath: pgInternalConfigFilePath,
+		UserConfigFilePath:     pgUserConfigFilePath,
+
+		passwordFilePath: pgPasswordFilePath,
+		repmgrUsername:   "repmgr",
+		repmgrDatabase:   "repgmr",
+	}
+
+	if err := stubPGConfigFile(); err != nil {
+		t.Fatal(err)
+	}
+
+	store, _ := state.NewStore()
+	if err := pgConf.initialize(store); err != nil {
+		t.Fatal(err)
+	}
+	t.Run("SharedPreloadLibraries", func(t *testing.T) {
+		valid := ConfigMap{
+			"shared_preload_libraries": "repmgr",
+		}
+		conf, err := pgConf.validateCompatibility(valid)
+		if err != nil {
+			t.Fatal(err)
+		}
+		if conf["shared_preload_libraries"].(string) != "'repmgr'" {
+			t.Fatal("expected preload library string to be wrapped in single quotes")
+		}
+
+		valid = ConfigMap{
+			"shared_preload_libraries": "'repmgr'",
+		}
+		conf, err = pgConf.validateCompatibility(valid)
+		if err != nil {
+			t.Fatal(err)
+		}
+		if conf["shared_preload_libraries"].(string) != "'repmgr'" {
+			t.Fatal("expected preload library string to be wrapped in single quotes")
+		}
+
+		valid = ConfigMap{
+			"shared_preload_libraries": "repmgr,timescaledb",
+		}
+		conf, err = pgConf.validateCompatibility(valid)
+		if err != nil {
+			t.Fatal(err)
+		}
+		if conf["shared_preload_libraries"].(string) != "'repmgr,timescaledb'" {
+			t.Fatal("expected preload library string to be wrapped in single quotes")
+		}
+
+		valid = ConfigMap{
+			"shared_preload_libraries": "",
+		}
+		if _, err := pgConf.validateCompatibility(valid); err == nil {
+			t.Fatal("expected validation to fail when empty")
+		}
+
+		valid = ConfigMap{
+			"shared_preload_libraries": "timescaledb",
+		}
+		if _, err := pgConf.validateCompatibility(valid); err == nil {
+			t.Fatal("expected validation to fail when repmgr is missing")
+		}
+
+	})
+
+	t.Run("WalLevel", func(t *testing.T) {
+		valid := ConfigMap{
+			"wal_level": "replica",
+		}
+		if _, err := pgConf.validateCompatibility(valid); err != nil {
+			t.Fatal(err)
+		}
+
+		valid = ConfigMap{
+			"wal_level": "logical",
+		}
+		if _, err := pgConf.validateCompatibility(valid); err != nil {
+			t.Fatal(err)
+		}
+
+		valid = ConfigMap{
+			"wal_level":    "minimal",
+			"archive_mode": "off",
+		}
+		if _, err := pgConf.validateCompatibility(valid); err != nil {
+			t.Fatal(err)
+		}
+
+		invalid := ConfigMap{
+			"wal_level": "minimal",
+		}
+
+		if _, err := pgConf.validateCompatibility(invalid); err == nil {
+			t.Fatal("expected wal_level minimal to fail with archiving enabled")
+		}
+
+		invalid = ConfigMap{
+			"wal_level": "logical",
+		}
+		if _, err := pgConf.validateCompatibility(invalid); err != nil {
+			t.Fatal(err)
+		}
+	})
+}
+
 func stubPGConfigFile() error {
 	file, err := os.Create(pgConfigFilePath)
 	if err != nil {