Skip to content

Commit

Permalink
fix: enforce postgres.auto.conf to rw before run rewind (#3728)
Browse files Browse the repository at this point in the history
pg_rewind needs to be able to write all the files in the PostgreSQL
data directory. For this reason, we always set `postgresql.auto.conf` mode to
600 before running it.

After the PostgreSQL data directory is ready to be used, we revert the
permission to be coherent with what the user specified in the
`enableAlterSystem` configuration parameter.

Closes: #3698

Signed-off-by: Tao Li <tao.li@enterprisedb.com>
Signed-off-by: Marco Nenciarini <marco.nenciarini@enterprisedb.com>
Signed-off-by: Leonardo Cecchi <leonardo.cecchi@enterprisedb.com>
Co-authored-by: Marco Nenciarini <marco.nenciarini@enterprisedb.com>
Co-authored-by: Leonardo Cecchi <leonardo.cecchi@enterprisedb.com>
(cherry picked from commit 9ff7942)
  • Loading branch information
litaocdl committed Jan 29, 2024
1 parent cdcd5f4 commit 1557b60
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 7 deletions.
7 changes: 4 additions & 3 deletions internal/management/controller/instance_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -133,9 +133,6 @@ func (r *InstanceReconciler) Reconcile(
}
reloadNeeded = reloadNeeded || reloadConfigNeeded

// Reconcile postgresql.auto.conf file
r.reconcileAutoConf(ctx, cluster)

// here we execute initialization tasks that need to be executed only on the first reconciliation loop
if !r.firstReconcileDone.Load() {
if err = r.initialize(ctx, cluster); err != nil {
Expand All @@ -144,6 +141,10 @@ func (r *InstanceReconciler) Reconcile(
r.firstReconcileDone.Store(true)
}

// Reconcile postgresql.auto.conf file. This operation must be after the initialize() call
// because it could interfere with pg_rewind execution
r.reconcileAutoConf(ctx, cluster)

// Reconcile cluster role without DB
reloadClusterRoleConfig, err := r.reconcileClusterRoleWithoutDB(ctx, cluster)
if err != nil {
Expand Down
9 changes: 9 additions & 0 deletions internal/management/controller/instance_startup.go
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,15 @@ func (r *InstanceReconciler) verifyPgDataCoherenceForPrimary(ctx context.Context
return err
}

// Set permission of postgres.auto.conf to 0600 to allow pg_rewind to write to it
// the mode will be later reset by the reconciliation again, skip the error as
// rewind may be not needed
err = r.instance.SetPostgreSQLAutoConfWritable(true)
if err != nil {
contextLogger.Error(
err, "Error while changing mode of the postgresql.auto.conf file before pg_rewind, skipped")
}

// pg_rewind could require a clean shutdown of the old primary to
// work. Unfortunately, if the old primary is already clean starting
// it up may make it advance in respect to the new one.
Expand Down
13 changes: 9 additions & 4 deletions pkg/management/postgres/instance.go
Original file line number Diff line number Diff line change
Expand Up @@ -197,14 +197,19 @@ type Instance struct {
tablespaceSynchronizerChan chan map[string]apiv1.TablespaceConfiguration
}

// SetAlterSystemEnabled allows or deny writes to the
// `postgresql.auto.conf` file in PGDATA, allowing and denying the
// usage of the ALTER SYSTEM SQL command
// SetAlterSystemEnabled allows or deny the usage of the
// ALTER SYSTEM SQL command
func (instance *Instance) SetAlterSystemEnabled(enabled bool) error {
return instance.SetPostgreSQLAutoConfWritable(enabled)
}

// SetPostgreSQLAutoConfWritable allows or deny writes to the
// `postgresql.auto.conf` file in PGDATA
func (instance *Instance) SetPostgreSQLAutoConfWritable(writeable bool) error {
autoConfFileName := path.Join(instance.PgData, "postgresql.auto.conf")

mode := fs.FileMode(0o600)
if !enabled {
if !writeable {
mode = 0o400
}
return os.Chmod(autoConfFileName, mode)
Expand Down

0 comments on commit 1557b60

Please sign in to comment.