Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: avoid reloading ident map when not needed #4648

Merged
merged 4 commits into from
May 28, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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)
}
leonardoce marked this conversation as resolved.
Show resolved Hide resolved
_, 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