Password rotation (canonical#26)

Author: marceloneppel

actions.yaml

@@ -3,6 +3,22 @@
 
 get-primary:
   description: Get the unit with is the primary/leader in the replication.
-get-operator-password:
-  description: Get the operator user password used by charm.
-    It is internal charm user, SHOULD NOT be used by applications.
+get-password:
+  description: Change the system user's password, which is used by charm.
+    It is for internal charm users and SHOULD NOT be used by applications.
+  params:
+    username:
+      type: string
+      description: The username, the default value 'operator'.
+        Possible values - operator, replication.
+set-password:
+  description: Change the system user's password, which is used by charm.
+    It is for internal charm users and SHOULD NOT be used by applications.
+  params:
+    username:
+      type: string
+      description: The username, the default value 'operator'.
+        Possible values - operator, replication.
+    password:
+      type: string
+      description: The password will be auto-generated if this option is not specified.

lib/charms/postgresql_k8s/v0/postgresql.py

@@ -31,7 +31,7 @@
 
 # Increment this PATCH version before using `charmcraft publish-lib` or reset
 # to 0 if you are raising the major API version
-LIBPATCH = 1
+LIBPATCH = 2
 
 
 logger = logging.getLogger(__name__)
@@ -53,6 +53,10 @@ class PostgreSQLGetPostgreSQLVersionError(Exception):
     """Exception raised when retrieving PostgreSQL version fails."""
 
 
+class PostgreSQLUpdateUserPasswordError(Exception):
+    """Exception raised when updating a user password fails."""
+
+
 class PostgreSQL:
     """Class to encapsulate all operations related to interacting with PostgreSQL instance."""
 
@@ -173,3 +177,27 @@ def get_postgresql_version(self) -> str:
         except psycopg2.Error as e:
             logger.error(f"Failed to get PostgreSQL version: {e}")
             raise PostgreSQLGetPostgreSQLVersionError()
+
+    def update_user_password(self, username: str, password: str) -> None:
+        """Update a user password.
+
+        Args:
+            username: the user to update the password.
+            password: the new password for the user.
+
+        Raises:
+            PostgreSQLUpdateUserPasswordError if the password couldn't be changed.
+        """
+        try:
+            with self._connect_to_database() as connection, connection.cursor() as cursor:
+                cursor.execute(
+                    sql.SQL("ALTER USER {} WITH ENCRYPTED PASSWORD '" + password + "';").format(
+                        sql.Identifier(username)
+                    )
+                )
+        except psycopg2.Error as e:
+            logger.error(f"Failed to update user password: {e}")
+            raise PostgreSQLUpdateUserPasswordError()
+        finally:
+            if connection is not None:
+                connection.close()

src/charm.py

@@ -7,7 +7,10 @@
 import logging
 from typing import Dict, List, Optional
 
-from charms.postgresql_k8s.v0.postgresql import PostgreSQL
+from charms.postgresql_k8s.v0.postgresql import (
+    PostgreSQL,
+    PostgreSQLUpdateUserPasswordError,
+)
 from lightkube import ApiError, Client, codecs
 from lightkube.resources.core_v1 import Endpoints, Pod, Service
 from ops.charm import (
@@ -30,7 +33,14 @@
 from requests import ConnectionError
 from tenacity import RetryError
 
-from constants import PEER, REPLICATION_PASSWORD_KEY, USER, USER_PASSWORD_KEY
+from constants import (
+    PEER,
+    REPLICATION_PASSWORD_KEY,
+    REPLICATION_USER,
+    SYSTEM_USERS,
+    USER,
+    USER_PASSWORD_KEY,
+)
 from patroni import NotReadyError, Patroni
 from relations.db import DbProvides
 from relations.postgresql_provider import PostgreSQLProvider
@@ -59,9 +69,8 @@ def __init__(self, *args):
         self.framework.observe(self.on.postgresql_pebble_ready, self._on_postgresql_pebble_ready)
         self.framework.observe(self.on.stop, self._on_stop)
         self.framework.observe(self.on.upgrade_charm, self._on_upgrade_charm)
-        self.framework.observe(
-            self.on.get_operator_password_action, self._on_get_operator_password
-        )
+        self.framework.observe(self.on.get_password_action, self._on_get_password)
+        self.framework.observe(self.on.set_password_action, self._on_set_password)
         self.framework.observe(self.on.get_primary_action, self._on_get_primary)
         self.framework.observe(self.on.update_status, self._on_update_status)
         self._storage_path = self.meta.storages["pgdata"].location
@@ -118,7 +127,7 @@ def postgresql(self) -> PostgreSQL:
         return PostgreSQL(
             host=self.primary_endpoint,
             user=USER,
-            password=self._get_operator_password(),
+            password=self._get_secret("app", f"{USER}-password"),
             database="postgres",
         )
 
@@ -426,9 +435,74 @@ def _create_resources(self) -> None:
                 self.unit.status = BlockedStatus(f"failed to create services {e}")
                 return
 
-    def _on_get_operator_password(self, event: ActionEvent) -> None:
-        """Returns the password for the operator user as an action response."""
-        event.set_results({USER_PASSWORD_KEY: self._get_operator_password()})
+    def _on_get_password(self, event: ActionEvent) -> None:
+        """Returns the password for a user as an action response.
+
+        If no user is provided, the password of the operator user is returned.
+        """
+        username = event.params.get("username", USER)
+        if username not in SYSTEM_USERS:
+            event.fail(
+                f"The action can be run only for users used by the charm or Patroni:"
+                f" {', '.join(SYSTEM_USERS)} not {username}"
+            )
+            return
+        event.set_results(
+            {f"{username}-password": self._get_secret("app", f"{username}-password")}
+        )
+
+    def _on_set_password(self, event: ActionEvent) -> None:
+        """Set the password for the specified user."""
+        # Only leader can write the new password into peer relation.
+        if not self.unit.is_leader():
+            event.fail("The action can be run only on leader unit")
+            return
+
+        username = event.params.get("username", USER)
+        if username not in SYSTEM_USERS:
+            event.fail(
+                f"The action can be run only for users used by the charm:"
+                f" {', '.join(SYSTEM_USERS)} not {username}"
+            )
+            return
+
+        password = new_password()
+        if "password" in event.params:
+            password = event.params["password"]
+
+        if password == self._get_secret("app", f"{username}-password"):
+            event.log("The old and new passwords are equal.")
+            event.set_results({f"{username}-password": password})
+            return
+
+        # Ensure all members are ready before trying to reload Patroni
+        # configuration to avoid errors (like the API not responding in
+        # one instance because PostgreSQL and/or Patroni are not ready).
+        if not self._patroni.are_all_members_ready():
+            event.fail(
+                "Failed changing the password: Not all members healthy or finished initial sync."
+            )
+            return
+
+        # Update the password in the PostgreSQL instance.
+        try:
+            self.postgresql.update_user_password(username, password)
+        except PostgreSQLUpdateUserPasswordError as e:
+            logger.exception(e)
+            event.fail(
+                "Failed changing the password: Not all members healthy or finished initial sync."
+            )
+            return
+
+        # Update the password in the secret store.
+        self._set_secret("app", f"{username}-password", password)
+
+        # Update and reload Patroni configuration in this unit to use the new password.
+        # Other units Patroni configuration will be reloaded in the peer relation changed event.
+        self._patroni.render_patroni_yml_file()
+        self._patroni.reload_patroni_configuration()
+
+        event.set_results({f"{username}-password": password})
 
     def _on_get_primary(self, event: ActionEvent) -> None:
         """Get primary instance."""
@@ -495,6 +569,8 @@ def _patroni(self):
             self._namespace,
             self.app.planned_units(),
             self._storage_path,
+            self._get_secret("app", USER_PASSWORD_KEY),
+            self._get_secret("app", REPLICATION_PASSWORD_KEY),
         )
 
     @property
@@ -555,10 +631,8 @@ def _postgresql_layer(self) -> Layer:
                         "PATRONI_KUBERNETES_USE_ENDPOINTS": "true",
                         "PATRONI_NAME": pod_name,
                         "PATRONI_SCOPE": f"patroni-{self._name}",
-                        "PATRONI_REPLICATION_USERNAME": "replication",
-                        "PATRONI_REPLICATION_PASSWORD": self._replication_password,
+                        "PATRONI_REPLICATION_USERNAME": REPLICATION_USER,
                         "PATRONI_SUPERUSER_USERNAME": USER,
-                        "PATRONI_SUPERUSER_PASSWORD": self._get_operator_password(),
                     },
                 }
             },
@@ -575,15 +649,6 @@ def _peers(self) -> Relation:
         """
         return self.model.get_relation(PEER)
 
-    def _get_operator_password(self) -> str:
-        """Get operator user password."""
-        return self._get_secret("app", USER_PASSWORD_KEY)
-
-    @property
-    def _replication_password(self) -> str:
-        """Get replication user password."""
-        return self._get_secret("app", REPLICATION_PASSWORD_KEY)
-
     def _unit_name_to_pod_name(self, unit_name: str) -> str:
         """Converts unit name to pod name.
 

src/constants.py

@@ -5,6 +5,9 @@
 
 DATABASE_PORT = "5432"
 PEER = "database-peers"
+REPLICATION_USER = "replication"
 REPLICATION_PASSWORD_KEY = "replication-password"
 USER = "operator"
 USER_PASSWORD_KEY = "operator-password"
+# List of system usernames needed for correct work of the charm/workload.
+SYSTEM_USERS = [REPLICATION_USER, USER]

src/patroni.py

@@ -32,13 +32,22 @@ class Patroni:
     """This class handles the communication with Patroni API and configuration files."""
 
     def __init__(
-        self, endpoint: str, endpoints: List[str], namespace: str, planned_units, storage_path: str
+        self,
+        endpoint: str,
+        endpoints: List[str],
+        namespace: str,
+        planned_units,
+        storage_path: str,
+        superuser_password: str,
+        replication_password: str,
     ):
         self._endpoint = endpoint
         self._endpoints = endpoints
         self._namespace = namespace
         self._storage_path = storage_path
         self._planned_units = planned_units
+        self._superuser_password = superuser_password
+        self._replication_password = replication_password
 
     def get_primary(self, unit_name_pattern=False) -> str:
         """Get primary instance.
@@ -137,6 +146,8 @@ def render_patroni_yml_file(self) -> None:
             endpoints=self._endpoints,
             namespace=self._namespace,
             storage_path=self._storage_path,
+            superuser_password=self._superuser_password,
+            replication_password=self._replication_password,
         )
         self._render_file(f"{self._storage_path}/patroni.yml", rendered, 0o644)
 

templates/patroni.yml.j2

@@ -30,4 +30,9 @@ postgresql:
   {%- for endpoint in endpoints %}
   - host replication replication {{ endpoint }}.{{ namespace }}.svc.cluster.local md5
   {%- endfor %}
+  authentication:
+    replication:
+      password: {{ replication_password }}
+    superuser:
+      password: {{ superuser_password }}
 use_endpoints: true