Skip to content

Commit

Permalink
fix: avoid reloading ident map when not needed (#4648)
Browse files Browse the repository at this point in the history
Before this patch, the instance manager could write the ident map using
two different formats, even if the content was semantically the same.

This was triggering an unneeded configuration reload after the
postmaster was started up.

Fixes: #4626

Signed-off-by: Leonardo Cecchi <leonardo.cecchi@enterprisedb.com>
Signed-off-by: Marco Nenciarini <marco.nenciarini@enterprisedb.com>
Signed-off-by: Jaime Silvela <jaime.silvela@enterprisedb.com>
Co-authored-by: Marco Nenciarini <marco.nenciarini@enterprisedb.com>
Co-authored-by: Jaime Silvela <jaime.silvela@enterprisedb.com>
  • Loading branch information
3 people committed May 28, 2024
1 parent 546f424 commit 6b3039e
Show file tree
Hide file tree
Showing 5 changed files with 30 additions and 38 deletions.
2 changes: 1 addition & 1 deletion internal/management/controller/instance_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -298,7 +298,7 @@ func (r *InstanceReconciler) refreshConfigurationFiles(
return false, err
}

reloadIdent, err := r.instance.RefreshPGIdent(cluster)
reloadIdent, err := r.instance.RefreshPGIdent(cluster.Spec.PostgresConfiguration.PgIdent)
if err != nil {
return false, err
}
Expand Down
22 changes: 14 additions & 8 deletions pkg/management/postgres/configuration.go
Original file line number Diff line number Diff line change
Expand Up @@ -198,16 +198,22 @@ func quoteHbaLiteral(literal string) string {
return fmt.Sprintf(`"%s"`, literal)
}

// GeneratePostgresqlIdent generates the pg_ident.conf content
func (instance *Instance) GeneratePostgresqlIdent(cluster *apiv1.Cluster) (string, error) {
return postgres.CreateIdentRules(cluster.Spec.PostgresConfiguration.PgIdent,
getCurrentUserOrDefaultToInsecureMapping())
// generatePostgresqlIdent generates the pg_ident.conf content given
// a set of additional pg_ident lines that is usually taken from the
// Cluster configuration
func (instance *Instance) generatePostgresqlIdent(additionalLines []string) (string, error) {
return postgres.CreateIdentRules(
additionalLines,
getCurrentUserOrDefaultToInsecureMapping(),
)
}

// RefreshPGIdent generates and writes down the pg_ident.conf file
func (instance *Instance) RefreshPGIdent(cluster *apiv1.Cluster) (postgresIdentChanged bool, err error) {
// Generate pg_hba.conf file
pgIdentContent, err := instance.GeneratePostgresqlIdent(cluster)
// RefreshPGIdent generates and writes down the pg_ident.conf file given
// a set of additional pg_ident lines that is usually taken from the
// Cluster configuration
func (instance *Instance) RefreshPGIdent(additionalLines []string) (postgresIdentChanged bool, err error) {
// Generate pg_ident.conf file
pgIdentContent, err := instance.generatePostgresqlIdent(additionalLines)
if err != nil {
return false, nil
}
Expand Down
17 changes: 0 additions & 17 deletions pkg/management/postgres/ident.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,28 +17,11 @@ limitations under the License.
package postgres

import (
"fmt"
"os/user"
"path/filepath"

"github.com/cloudnative-pg/cloudnative-pg/pkg/fileutils"
"github.com/cloudnative-pg/cloudnative-pg/pkg/management/log"
"github.com/cloudnative-pg/cloudnative-pg/pkg/management/postgres/constants"
)

// WritePostgresUserMaps creates a pg_ident.conf file containing only one map called "local" that
// maps the current user to "postgres" user.
func WritePostgresUserMaps(pgData string) error {
username := getCurrentUserOrDefaultToInsecureMapping()
_, err := fileutils.WriteStringToFile(filepath.Join(pgData, constants.PostgresqlIdentFile),
fmt.Sprintf("local %s postgres\n", username))
if err != nil {
return err
}

return nil
}

// getCurrentUserOrDefaultToInsecureMapping retrieves the current system user's username.
// If the retrieval fails, it falls back to an insecure mapping using the root ("/") as the default username.
//
Expand Down
4 changes: 3 additions & 1 deletion pkg/management/postgres/instance.go
Original file line number Diff line number Diff line change
Expand Up @@ -299,7 +299,9 @@ func (instance *Instance) VerifyPgDataCoherence(ctx context.Context) error {
return err
}

return WritePostgresUserMaps(instance.PgData)
// creates a bare pg_ident.conf that only grants local access
_, err := instance.RefreshPGIdent(nil)
return err
}

// InstanceCommand are commands for the goroutine managing postgres
Expand Down
23 changes: 12 additions & 11 deletions pkg/management/postgres/restore.go
Original file line number Diff line number Diff line change
Expand Up @@ -715,36 +715,36 @@ func (info InitInfo) WriteInitialPostgresqlConf(cluster *apiv1.Cluster) error {

_, err = temporaryInstance.RefreshPGHBA(cluster, "")
if err != nil {
return fmt.Errorf("while reading configuration files from ConfigMap: %w", err)
return fmt.Errorf("while generating pg_hba.conf: %w", err)
}
_, err = temporaryInstance.RefreshPGIdent(cluster)
_, err = temporaryInstance.RefreshPGIdent(cluster.Spec.PostgresConfiguration.PgIdent)
if err != nil {
return fmt.Errorf("while reading configuration files from ConfigMap: %w", err)
return fmt.Errorf("while generating pg_ident.conf: %w", err)
}
_, err = temporaryInstance.RefreshConfigurationFilesFromCluster(cluster, false)
if err != nil {
return fmt.Errorf("while reading configuration files from ConfigMap: %w", err)
return fmt.Errorf("while generating Postgres configuration: %w", err)
}

err = fileutils.CopyFile(
path.Join(temporaryInitInfo.PgData, "postgresql.conf"),
path.Join(info.PgData, "postgresql.conf"))
if err != nil {
return fmt.Errorf("while creating postgresql.conf: %w", err)
return fmt.Errorf("while installing postgresql.conf: %w", err)
}

err = fileutils.CopyFile(
path.Join(temporaryInitInfo.PgData, constants.PostgresqlCustomConfigurationFile),
path.Join(info.PgData, constants.PostgresqlCustomConfigurationFile))
if err != nil {
return fmt.Errorf("while creating custom.conf: %w", err)
return fmt.Errorf("while installing %v: %w", constants.PostgresqlCustomConfigurationFile, err)
}

err = fileutils.CopyFile(
path.Join(temporaryInitInfo.PgData, constants.PostgresqlOverrideConfigurationFile),
path.Join(info.PgData, constants.PostgresqlOverrideConfigurationFile))
if err != nil {
return fmt.Errorf("while creating %v: %w", constants.PostgresqlOverrideConfigurationFile, err)
return fmt.Errorf("while installing %v: %w", constants.PostgresqlOverrideConfigurationFile, err)
}

// Disable SSL as we still don't have the required certificates
Expand All @@ -758,8 +758,8 @@ func (info InitInfo) WriteInitialPostgresqlConf(cluster *apiv1.Cluster) error {
return err
}

// WriteRestoreHbaConf writes a pg_hba.conf allowing access without password from localhost.
// this is needed to set the PostgreSQL password after the postgres server is started and active
// WriteRestoreHbaConf writes basic pg_hba.conf and pg_ident.conf allowing access without password from localhost.
// This is needed to set the PostgreSQL password after the postgres server is started and active
func (info InitInfo) WriteRestoreHbaConf() error {
// We allow every access from localhost, and this is needed to correctly restore
// the database
Expand All @@ -770,8 +770,9 @@ func (info InitInfo) WriteRestoreHbaConf() error {
return err
}

// Create the local map referred in the HBA configuration
return WritePostgresUserMaps(info.PgData)
// Create only the local map referred in the HBA configuration
_, err = info.GetInstance().RefreshPGIdent(nil)
return err
}

// ConfigureInstanceAfterRestore changes the superuser password
Expand Down

0 comments on commit 6b3039e

Please sign in to comment.