From 6ba667db516ae7694e62bbe3996ffec232aac83e Mon Sep 17 00:00:00 2001 From: Leonardo Cecchi Date: Tue, 28 May 2024 13:27:56 +0200 Subject: [PATCH] fix: avoid reloading ident map when not needed (#4648) 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 Signed-off-by: Marco Nenciarini Signed-off-by: Jaime Silvela Co-authored-by: Marco Nenciarini Co-authored-by: Jaime Silvela (cherry picked from commit 6b3039ebb513f430bca1b9666a657da2faf30dab) --- .../controller/instance_controller.go | 2 +- pkg/management/postgres/configuration.go | 22 +++++++++++------- pkg/management/postgres/ident.go | 17 -------------- pkg/management/postgres/instance.go | 4 +++- pkg/management/postgres/restore.go | 23 ++++++++++--------- 5 files changed, 30 insertions(+), 38 deletions(-) diff --git a/internal/management/controller/instance_controller.go b/internal/management/controller/instance_controller.go index 2f20de01a7..4016264fe2 100644 --- a/internal/management/controller/instance_controller.go +++ b/internal/management/controller/instance_controller.go @@ -296,7 +296,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 } diff --git a/pkg/management/postgres/configuration.go b/pkg/management/postgres/configuration.go index 638f5eddae..8cc34e6310 100644 --- a/pkg/management/postgres/configuration.go +++ b/pkg/management/postgres/configuration.go @@ -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 } diff --git a/pkg/management/postgres/ident.go b/pkg/management/postgres/ident.go index d080215d2d..4b3d07a032 100644 --- a/pkg/management/postgres/ident.go +++ b/pkg/management/postgres/ident.go @@ -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. // diff --git a/pkg/management/postgres/instance.go b/pkg/management/postgres/instance.go index 3b07ddbacd..ce8c54ac9b 100644 --- a/pkg/management/postgres/instance.go +++ b/pkg/management/postgres/instance.go @@ -295,7 +295,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 diff --git a/pkg/management/postgres/restore.go b/pkg/management/postgres/restore.go index e9ff25b86c..f962d968cb 100644 --- a/pkg/management/postgres/restore.go +++ b/pkg/management/postgres/restore.go @@ -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 @@ -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 @@ -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