Skip to content

Commit

Permalink
chore: avoid using PGPASSFILE environment variable (#3522)
Browse files Browse the repository at this point in the history
This patch removed the PGPASSFILE environment variable since
jackc/pgx is correctly handling the `passfile` connection string option.

See also #2386

Signed-off-by: Leonardo Cecchi <leonardo.cecchi@enterprisedb.com>
Signed-off-by: Armando Ruocco <armando.ruocco@enterprisedb.com>
Co-authored-by: Armando Ruocco <armando.ruocco@enterprisedb.com>
  • Loading branch information
leonardoce and armru committed Dec 12, 2023
1 parent 7c8ca6a commit d6e427b
Show file tree
Hide file tree
Showing 5 changed files with 17 additions and 40 deletions.
10 changes: 1 addition & 9 deletions internal/cmd/manager/instance/pgbasebackup/cmd.go
Expand Up @@ -122,20 +122,12 @@ func (env *CloneInfo) bootstrapUsingPgbasebackup(ctx context.Context) error {
return fmt.Errorf("missing external cluster")
}

connectionString, pgpass, err := external.ConfigureConnectionToServer(
connectionString, err := external.ConfigureConnectionToServer(
ctx, env.client, env.info.Namespace, &server)
if err != nil {
return err
}

// Unfortunately lib/pq doesn't support the passfile
// connection option so we must rely on an environment
// variable.
if pgpass != "" {
if err = os.Setenv("PGPASSFILE", pgpass); err != nil {
return err
}
}
err = postgres.ClonePgData(connectionString, env.info.PgData, env.info.PgWal)
if err != nil {
return err
Expand Down
23 changes: 12 additions & 11 deletions pkg/management/external/external.go
Expand Up @@ -29,14 +29,13 @@ import (

// ConfigureConnectionToServer creates a connection string to the external
// server, using the configuration inside the cluster and dumping the secret when
// needed. This function will return a connection string, the name of the pgpass file
// to be used, and an error state
// needed in a custom passfile.
// Returns a connection string or any error encountered
func ConfigureConnectionToServer(
ctx context.Context, client ctrl.Client,
namespace string, server *apiv1.ExternalCluster,
) (string, string, error) {
) (string, error) {
connectionParameters := make(map[string]string, len(server.ConnectionParameters))
pgpassfile := ""

for key, value := range server.ConnectionParameters {
connectionParameters[key] = value
Expand All @@ -45,7 +44,7 @@ func ConfigureConnectionToServer(
if server.SSLCert != nil {
name, err := DumpSecretKeyRefToFile(ctx, client, namespace, server.Name, server.SSLCert)
if err != nil {
return "", "", err
return "", err
}

connectionParameters["sslcert"] = name
Expand All @@ -54,7 +53,7 @@ func ConfigureConnectionToServer(
if server.SSLKey != nil {
name, err := DumpSecretKeyRefToFile(ctx, client, namespace, server.Name, server.SSLKey)
if err != nil {
return "", "", err
return "", err
}

connectionParameters["sslkey"] = name
Expand All @@ -63,7 +62,7 @@ func ConfigureConnectionToServer(
if server.SSLRootCert != nil {
name, err := DumpSecretKeyRefToFile(ctx, client, namespace, server.Name, server.SSLRootCert)
if err != nil {
return "", "", err
return "", err
}

connectionParameters["sslrootcert"] = name
Expand All @@ -72,14 +71,16 @@ func ConfigureConnectionToServer(
if server.Password != nil {
password, err := ReadSecretKeyRef(ctx, client, namespace, server.Password)
if err != nil {
return "", "", err
return "", err
}

pgpassfile, err = CreatePgPassFile(server.Name, password)
pgpassfile, err := CreatePgPassFile(server.Name, password)
if err != nil {
return "", "", err
return "", err
}

connectionParameters["passfile"] = pgpassfile
}

return configfile.CreateConnectionString(connectionParameters), pgpassfile, nil
return configfile.CreateConnectionString(connectionParameters), nil
}
12 changes: 1 addition & 11 deletions pkg/management/postgres/initdb.go
Expand Up @@ -23,7 +23,6 @@ import (
"context"
"database/sql"
"fmt"
"os"
"os/exec"
"path"
"path/filepath"
Expand Down Expand Up @@ -433,7 +432,7 @@ func getConnectionPoolerForExternalCluster(
modifiedExternalCluster := externalCluster.DeepCopy()
delete(modifiedExternalCluster.ConnectionParameters, "dbname")

sourceDBConnectionString, pgpass, err := external.ConfigureConnectionToServer(
sourceDBConnectionString, err := external.ConfigureConnectionToServer(
ctx,
client,
namespaceOfNewCluster,
Expand All @@ -443,14 +442,5 @@ func getConnectionPoolerForExternalCluster(
return nil, err
}

// Unfortunately lib/pq doesn't support the passfile
// connection option so we must rely on an environment
// variable.
if pgpass != "" {
if err = os.Setenv("PGPASSFILE", pgpass); err != nil {
return nil, err
}
}

return pool.NewPostgresqlConnectionPool(sourceDBConnectionString), nil
}
8 changes: 1 addition & 7 deletions pkg/management/postgres/instance_replica.go
Expand Up @@ -74,17 +74,11 @@ func (instance *Instance) writeReplicaConfigurationForDesignatedPrimary(
return false, fmt.Errorf("missing external cluster")
}

connectionString, pgpassfile, err := external.ConfigureConnectionToServer(
connectionString, err := external.ConfigureConnectionToServer(
ctx, cli, instance.Namespace, &server)
if err != nil {
return false, err
}

if pgpassfile != "" {
connectionString = fmt.Sprintf("%v passfile=%v",
connectionString,
pgpassfile)
}

return UpdateReplicaConfiguration(instance.PgData, connectionString, "")
}
4 changes: 2 additions & 2 deletions pkg/management/postgres/restore.go
Expand Up @@ -139,7 +139,7 @@ func (info InitInfo) RestoreSnapshot(ctx context.Context, cli client.Client, imm
return fmt.Errorf("missing external cluster: %v", cluster.Spec.ReplicaCluster.Source)
}

connectionString, _, err := external.ConfigureConnectionToServer(
connectionString, err := external.ConfigureConnectionToServer(
ctx, cli, info.Namespace, &server)
if err != nil {
return err
Expand Down Expand Up @@ -272,7 +272,7 @@ func (info InitInfo) Restore(ctx context.Context) error {
return fmt.Errorf("missing external cluster: %v", cluster.Spec.ReplicaCluster.Source)
}

connectionString, _, err := external.ConfigureConnectionToServer(
connectionString, err := external.ConfigureConnectionToServer(
ctx, typedClient, info.Namespace, &server)
if err != nil {
return err
Expand Down

0 comments on commit d6e427b

Please sign in to comment.