Fixes & features

CHANGELOG.md

@@ -1,5 +1,22 @@
 # Changelog
 
+## 3.4.0
+
+### Features
+
+- Add timeout support for reconcile loops
+- postgresqluserrole: Add support for extra connection url parameters
+
+### Fixes
+
+- postgresqldatabase: Avoid closing connections multiple time in row and close them when needed
+- postgresqldatabase: Avoid alters on tables & types when owner is already the right one
+- postgresqldatabase: Improve deletion by checking if roles or database are existing in engine before trying to delete them. This will avoid dead lock scenarios
+
+### Refactor
+
+- Use context on all SQL calls
+
 ## 3.3.0
 
 ### Features

api/postgresql/v1alpha1/postgresqluserrole_types.go

@@ -56,6 +56,8 @@ type PostgresqlUserRolePrivilege struct {
 	// +kubebuilder:validation:Required
 	// +kubebuilder:validation:MinLength=1
 	GeneratedSecretName string `json:"generatedSecretName"`
+	// Extra connection URL Parameters
+	ExtraConnectionURLParameters map[string]string `json:"extraConnectionUrlParameters,omitempty"`
 }
 
 type PostgresqlUserRoleAttributes struct {

cmd/main.go

@@ -64,13 +64,14 @@ func init() {
 } //nolint: wsl // Needed by operator
 
 func main() {
-	var metricsAddr, probeAddr, resyncPeriodStr string
+	var metricsAddr, probeAddr, resyncPeriodStr, reconcileTimeoutStr string
 
 	var enableLeaderElection bool
 
 	flag.StringVar(&metricsAddr, "metrics-bind-address", ":8080", "The address the metric endpoint binds to.")
 	flag.StringVar(&probeAddr, "health-probe-bind-address", ":8081", "The address the probe endpoint binds to.")
 	flag.StringVar(&resyncPeriodStr, "resync-period", "30s", "The resync period to reload all resources for auto-heal procedures.")
+	flag.StringVar(&reconcileTimeoutStr, "reconcile-timeout", "5s", "The reconcile max timeout.")
 	flag.BoolVar(&enableLeaderElection, "leader-elect", false,
 		"Enable leader election for controller manager. "+
 			"Enabling this will ensure there is only one active controller manager.")
@@ -90,6 +91,13 @@ func main() {
 		setupLog.Error(err, "unable to parse resync period")
 		os.Exit(1)
 	}
+	// Parse duration
+	reconcileTimeout, err := time.ParseDuration(reconcileTimeoutStr)
+	// Check error
+	if err != nil {
+		setupLog.Error(err, "unable to parse reconcile timeout")
+		os.Exit(1)
+	}
 	// Log
 	setupLog.Info(fmt.Sprintf("Starting manager with %s resync period", resyncPeriodStr))
 
@@ -132,6 +140,7 @@ func main() {
 		),
 		ControllerRuntimeDetailedErrorTotal: controllerRuntimeDetailedErrorTotal,
 		ControllerName:                      "postgresqlengineconfiguration",
+		ReconcileTimeout:                    reconcileTimeout,
 	}).SetupWithManager(mgr); err != nil {
 		setupLog.Error(err, "unable to create controller", "controller", "PostgresqlEngineConfiguration")
 		os.Exit(1)
@@ -151,6 +160,7 @@ func main() {
 		),
 		ControllerRuntimeDetailedErrorTotal: controllerRuntimeDetailedErrorTotal,
 		ControllerName:                      "postgresqldatabase",
+		ReconcileTimeout:                    reconcileTimeout,
 	}).SetupWithManager(mgr); err != nil {
 		setupLog.Error(err, "unable to create controller", "controller", "PostgresqlDatabase")
 		os.Exit(1)
@@ -170,6 +180,7 @@ func main() {
 		),
 		ControllerRuntimeDetailedErrorTotal: controllerRuntimeDetailedErrorTotal,
 		ControllerName:                      "postgresqluserrole",
+		ReconcileTimeout:                    reconcileTimeout,
 	}).SetupWithManager(mgr); err != nil {
 		setupLog.Error(err, "unable to create controller", "controller", "PostgresqlUserRole")
 		os.Exit(1)
@@ -189,6 +200,7 @@ func main() {
 		),
 		ControllerRuntimeDetailedErrorTotal: controllerRuntimeDetailedErrorTotal,
 		ControllerName:                      "postgresqlpublication",
+		ReconcileTimeout:                    reconcileTimeout,
 	}).SetupWithManager(mgr); err != nil {
 		setupLog.Error(err, "unable to create controller", "controller", "PostgresqlPublication")
 		os.Exit(1)

config/crd/bases/postgresql.easymile.com_postgresqluserroles.yaml

@@ -93,6 +93,11 @@ spec:
                       required:
                       - name
                       type: object
+                    extraConnectionUrlParameters:
+                      additionalProperties:
+                        type: string
+                      description: Extra connection URL Parameters
+                      type: object
                     generatedSecretName:
                       description: Generated secret name prefix
                       minLength: 1

config/samples/userrole/managed-simple-rotation.yaml

@@ -20,3 +20,7 @@ spec:
         name: simple
       # Generated secret name with information for the selected database
       generatedSecretName: managed-simple-rotation
+      # Extra connection URL Parameters
+      extraConnectionUrlParameters:
+        {}
+        # param1: value1

config/samples/userrole/managed-simple.yaml

@@ -18,6 +18,10 @@ spec:
         name: simple
       # Generated secret name with information for the selected database
       generatedSecretName: managed-simple
+      # Extra connection URL Parameters
+      extraConnectionUrlParameters:
+        {}
+        # param1: value1
   # Role attributes
   # Note: Only attributes that aren't conflicting with operator are supported.
   roleAttributes:

config/samples/userrole/provided-simple.yaml

@@ -16,6 +16,10 @@ spec:
         name: simple
       # Generated secret name with information for the selected database
       generatedSecretName: simple1
+      # Extra connection URL Parameters
+      extraConnectionUrlParameters:
+        {}
+        # param1: value1
   # Import secret that will contain "USERNAME" and "PASSWORD" for provided mode
   importSecretName: provided-simple
   # Role attributes

docs/crds/PostgresqlUserRole.md

@@ -37,12 +37,13 @@ All these names are available for `kubectl`:
 
 ### PostgresqlUserRolePrivilege
 
-| Field               | Description                                                                                                                                                                               | Scheme            | Required |
-| ------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ----------------- | -------- |
-| privilege           | User privilege on database. Enumeration is `OWNER`, `WRITER`, `READER`.                                                                                                                   | String            | true     |
-| connectionType      | Connection type to be used for secret generation (Can be set to BOUNCER if wanted and supported by engine configuration). Enumeration is `PRIMARY`, `BOUNCER`. Default value is `PRIMARY` | String            | false    |
-| database            | [PostgresqlDatabase](./PostgresqlDatabase.md) object reference                                                                                                                            | [CRLink](#crlink) | true     |
-| generatedSecretName | Generated secret name used for secret generation.                                                                                                                                         | String            | true     |
+| Field                        | Description                                                                                                                                                                               | Scheme              | Required |
+| ---------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------------------- | -------- |
+| privilege                    | User privilege on database. Enumeration is `OWNER`, `WRITER`, `READER`.                                                                                                                   | String              | true     |
+| connectionType               | Connection type to be used for secret generation (Can be set to BOUNCER if wanted and supported by engine configuration). Enumeration is `PRIMARY`, `BOUNCER`. Default value is `PRIMARY` | String              | false    |
+| database                     | [PostgresqlDatabase](./PostgresqlDatabase.md) object reference                                                                                                                            | [CRLink](#crlink)   | true     |
+| generatedSecretName          | Generated secret name used for secret generation.                                                                                                                                         | String              | true     |
+| extraConnectionUrlParameters | Extra connection url parameters that will be added into `POSTGRES_URL_ARGS` and `ARGS` fields in generated secret                                                                         | `map[string]string` | false    |
 
 ### PostgresqlUserRoleAttributes
 
@@ -96,6 +97,9 @@ spec:
         name: simple
       # Generated secret name with information for the selected database
       generatedSecretName: simple1
+      # Extra connection URL Parameters
+      extraConnectionUrlParameters: {}
+      #   param1: value1
   # Import secret that will contain "USERNAME" and "PASSWORD" for provided mode
   importSecretName: provided-simple
   # Role attributes

internal/controller/postgresql/postgres/aws.go

@@ -1,6 +1,7 @@
 package postgres
 
 import (
+	"context"
 	"fmt"
 
 	"github.com/lib/pq"
@@ -21,32 +22,32 @@ func newAWSPG(postgres *pg) PG {
 	}
 }
 
-func (c *awspg) AlterDefaultLoginRole(role, setRole string) error {
+func (c *awspg) AlterDefaultLoginRole(ctx context.Context, role, setRole string) error {
 	// On AWS RDS the postgres user isn't really superuser so he doesn't have permissions
 	// to ALTER USER unless he belongs to both roles
-	err := c.GrantRole(role, c.user, false)
+	err := c.GrantRole(ctx, role, c.user, false)
 	if err != nil {
 		return err
 	}
 
 	defer func() {
-		err := c.RevokeRole(role, c.user)
+		err := c.RevokeRole(ctx, role, c.user)
 		// Check error
 		if err != nil {
 			c.log.Error(err, "error in revoke role")
 		}
 	}()
 
-	return c.pg.AlterDefaultLoginRole(role, setRole)
+	return c.pg.AlterDefaultLoginRole(ctx, role, setRole)
 }
 
-func (c *awspg) CreateDB(dbname, role string) error {
+func (c *awspg) CreateDB(ctx context.Context, dbname, role string) error {
 	err := c.connect(c.defaultDatabase)
 	if err != nil {
 		return err
 	}
 
-	_, err = c.db.Exec(fmt.Sprintf(CreateDBWithoutOwnerSQLTemplate, dbname))
+	_, err = c.db.ExecContext(ctx, fmt.Sprintf(CreateDBWithoutOwnerSQLTemplate, dbname))
 	if err != nil {
 		// eat DUPLICATE DATABASE ERROR
 		// Try to cast error
@@ -56,18 +57,18 @@ func (c *awspg) CreateDB(dbname, role string) error {
 		}
 	}
 
-	_, err = c.db.Exec(fmt.Sprintf(AlterDBOwnerSQLTemplate, dbname, role))
+	_, err = c.db.ExecContext(ctx, fmt.Sprintf(AlterDBOwnerSQLTemplate, dbname, role))
 	if err != nil {
 		return err
 	}
 
 	return nil
 }
 
-func (c *awspg) DropRoleAndDropAndChangeOwnedBy(role, newOwner, database string) error {
+func (c *awspg) DropRoleAndDropAndChangeOwnedBy(ctx context.Context, role, newOwner, database string) error {
 	// On AWS RDS the postgres user isn't really superuser so he doesn't have permissions
 	// to REASSIGN OWNED BY unless he belongs to both roles
-	err := c.GrantRole(role, c.user, false)
+	err := c.GrantRole(ctx, role, c.user, false)
 	// Check error
 	if err != nil {
 		// Try to cast error
@@ -85,7 +86,7 @@ func (c *awspg) DropRoleAndDropAndChangeOwnedBy(role, newOwner, database string)
 		}
 	}
 
-	err = c.GrantRole(newOwner, c.user, false)
+	err = c.GrantRole(ctx, newOwner, c.user, false)
 	// Check error
 	if err != nil {
 		// Try to cast error
@@ -107,19 +108,19 @@ func (c *awspg) DropRoleAndDropAndChangeOwnedBy(role, newOwner, database string)
 	}
 
 	defer func() {
-		err := c.RevokeRole(newOwner, c.user)
+		err := c.RevokeRole(ctx, newOwner, c.user)
 		if err != nil {
 			c.log.Error(err, "error in revoke role")
 		}
 	}()
 
-	return c.pg.DropRoleAndDropAndChangeOwnedBy(role, newOwner, database)
+	return c.pg.DropRoleAndDropAndChangeOwnedBy(ctx, role, newOwner, database)
 }
 
-func (c *awspg) ChangeAndDropOwnedBy(role, newOwner, database string) error {
+func (c *awspg) ChangeAndDropOwnedBy(ctx context.Context, role, newOwner, database string) error {
 	// On AWS RDS the postgres user isn't really superuser so he doesn't have permissions
 	// to REASSIGN OWNED BY unless he belongs to both roles
-	err := c.GrantRole(role, c.user, false)
+	err := c.GrantRole(ctx, role, c.user, false)
 	// Check error
 	if err != nil {
 		// Try to cast error
@@ -137,7 +138,7 @@ func (c *awspg) ChangeAndDropOwnedBy(role, newOwner, database string) error {
 		}
 	}
 
-	err = c.GrantRole(newOwner, c.user, false)
+	err = c.GrantRole(ctx, newOwner, c.user, false)
 	// Check error
 	if err != nil {
 		// Try to cast error
@@ -159,11 +160,11 @@ func (c *awspg) ChangeAndDropOwnedBy(role, newOwner, database string) error {
 	}
 
 	defer func() {
-		err := c.RevokeRole(newOwner, c.user)
+		err := c.RevokeRole(ctx, newOwner, c.user)
 		if err != nil {
 			c.log.Error(err, "error in revoke role")
 		}
 	}()
 
-	return c.pg.ChangeAndDropOwnedBy(role, newOwner, database)
+	return c.pg.ChangeAndDropOwnedBy(ctx, role, newOwner, database)
 }

internal/controller/postgresql/postgres/azure.go

@@ -1,6 +1,7 @@
 package postgres
 
 import (
+	"context"
 	"fmt"
 	"strings"
 )
@@ -26,8 +27,8 @@ func newAzurePG(postgres *pg) PG {
 	}
 }
 
-func (azpg *azurepg) CreateUserRole(role, password string, attributes *RoleAttributes) (string, error) {
-	returnedRole, err := azpg.pg.CreateUserRole(role, password, attributes)
+func (azpg *azurepg) CreateUserRole(ctx context.Context, role, password string, attributes *RoleAttributes) (string, error) {
+	returnedRole, err := azpg.pg.CreateUserRole(ctx, role, password, attributes)
 	if err != nil {
 		return "", err
 	}
@@ -44,12 +45,12 @@ func (azpg *azurepg) GetRoleForLogin(login string) string {
 	return login
 }
 
-func (azpg *azurepg) CreateDB(dbname, role string) error {
+func (azpg *azurepg) CreateDB(ctx context.Context, dbname, role string) error {
 	// Have to add the master role to the group role before we can transfer the database owner
-	err := azpg.GrantRole(role, azpg.GetRoleForLogin(azpg.user), false)
+	err := azpg.GrantRole(ctx, role, azpg.GetRoleForLogin(azpg.user), false)
 	if err != nil {
 		return err
 	}
 
-	return azpg.pg.CreateDB(dbname, role)
+	return azpg.pg.CreateDB(ctx, dbname, role)
 }