Password rotation

actions.yaml

@@ -4,5 +4,21 @@
 get-primary:
   description: Get the unit which is the primary/leader in the replication.
 get-password:
-  description: Get the operator user password used by charm.
-    It is internal charm user, SHOULD NOT be used by applications.
+  description: Get 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

@@ -32,7 +32,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__)
@@ -58,6 +58,10 @@ class PostgreSQLListUsersError(Exception):
     """Exception raised when retrieving PostgreSQL users list 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."""
 
@@ -196,3 +200,27 @@ def list_users(self) -> Set[str]:
         except psycopg2.Error as e:
             logger.error(f"Failed to list PostgreSQL database users: {e}")
             raise PostgreSQLListUsersError()
+
+    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

@@ -10,7 +10,11 @@
 from typing import Dict, List, Optional, Set
 
 from charms.operator_libs_linux.v0 import apt
-from charms.postgresql_k8s.v0.postgresql import PostgreSQL, PostgreSQLCreateUserError
+from charms.postgresql_k8s.v0.postgresql import (
+    PostgreSQL,
+    PostgreSQLCreateUserError,
+    PostgreSQLUpdateUserPasswordError,
+)
 from ops.charm import (
     ActionEvent,
     CharmBase,
@@ -37,7 +41,13 @@
     RemoveRaftMemberFailedError,
     SwitchoverFailedError,
 )
-from constants import PEER, REPLICATION_PASSWORD_KEY, USER, USER_PASSWORD_KEY
+from constants import (
+    PEER,
+    REPLICATION_PASSWORD_KEY,
+    SYSTEM_USERS,
+    USER,
+    USER_PASSWORD_KEY,
+)
 from relations.db import DbProvides
 from relations.postgresql_provider import PostgreSQLProvider
 from utils import new_password
@@ -64,6 +74,7 @@ def __init__(self, *args):
         self.framework.observe(self.on.pgdata_storage_detaching, self._on_pgdata_storage_detaching)
         self.framework.observe(self.on.start, self._on_start)
         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.update_status, self._on_update_status)
         self._cluster_name = self.app.name
         self._member_name = self.unit.name.replace("/", "-")
@@ -121,7 +132,7 @@ def postgresql(self) -> PostgreSQL:
         return PostgreSQL(
             host=self.primary_endpoint,
             user=USER,
-            password=self._get_password(),
+            password=self._get_secret("app", f"{USER}-password"),
             database="postgres",
         )
 
@@ -612,8 +623,71 @@ def _on_start(self, event) -> None:
         self.unit.status = ActiveStatus()
 
     def _on_get_password(self, event: ActionEvent) -> None:
-        """Returns the password for the operator user as an action response."""
-        event.set_results({USER_PASSWORD_KEY: self._get_password()})
+        """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 = event.params.get("password", new_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_update_status(self, _) -> None:
         """Update endpoints of the postgres client relation and update users list."""

src/cluster.py

@@ -98,7 +98,7 @@ def configure_patroni_on_unit(self, replica: bool = False):
             (defaults to False, which configures the unit as a leader)
         """
         self._change_owner(self.storage_path)
-        self._render_patroni_yml_file(replica)
+        self.render_patroni_yml_file(replica)
         self._render_patroni_service_file()
         # Reload systemd services before trying to start Patroni.
         daemon_reload()
@@ -248,7 +248,7 @@ def _render_patroni_service_file(self) -> None:
         rendered = template.render(conf_path=self.storage_path)
         self._render_file("/etc/systemd/system/patroni.service", rendered, 0o644)
 
-    def _render_patroni_yml_file(self, replica: bool = False) -> None:
+    def render_patroni_yml_file(self, replica: bool = False) -> None:
         """Render the Patroni configuration file."""
         # Open the template patroni.yml file.
         with open("templates/patroni.yml.j2", "r") as file:
@@ -320,10 +320,10 @@ def primary_changed(self, old_primary: str) -> bool:
     def update_cluster_members(self) -> None:
         """Update the list of members of the cluster."""
         # Update the members in the Patroni configuration.
-        self._render_patroni_yml_file()
+        self.render_patroni_yml_file()
 
         if service_running(PATRONI_SERVICE):
-            self._reload_patroni_configuration()
+            self.reload_patroni_configuration()
 
     def remove_raft_member(self, member_ip: str) -> None:
         """Remove a member from the raft cluster.
@@ -353,6 +353,6 @@ def remove_raft_member(self, member_ip: str) -> None:
             raise RemoveRaftMemberFailedError()
 
     @retry(stop=stop_after_attempt(3), wait=wait_exponential(multiplier=1, min=2, max=10))
-    def _reload_patroni_configuration(self):
+    def reload_patroni_configuration(self):
         """Reload Patroni configuration after it was changed."""
         requests.post(f"http://{self.unit_ip}:8008/reload")

src/constants.py

@@ -9,6 +9,9 @@
 LEGACY_DB_ADMIN = "db-admin"
 PEER = "database-peers"
 ALL_CLIENT_RELATIONS = [DATABASE, LEGACY_DB, LEGACY_DB_ADMIN]
+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]

tests/integration/helpers.py

@@ -4,6 +4,7 @@
 import itertools
 import tempfile
 import zipfile
+from datetime import datetime
 from pathlib import Path
 from typing import List
 
@@ -12,7 +13,13 @@
 import yaml
 from juju.unit import Unit
 from pytest_operator.plugin import OpsTest
-from tenacity import Retrying, stop_after_attempt, wait_exponential
+from tenacity import (
+    Retrying,
+    retry,
+    retry_if_result,
+    stop_after_attempt,
+    wait_exponential,
+)
 
 METADATA = yaml.safe_load(Path("./metadata.yaml").read_text())
 DATABASE_APP_NAME = METADATA["name"]
@@ -118,6 +125,30 @@ async def check_databases_creation(ops_test: OpsTest, databases: List[str]) -> N
             assert len(output)
 
 
+@retry(
+    retry=retry_if_result(lambda x: not x),
+    stop=stop_after_attempt(10),
+    wait=wait_exponential(multiplier=1, min=2, max=30),
+)
+def check_patroni(ops_test: OpsTest, unit_name: str, restart_time: float) -> bool:
+    """Check if Patroni is running correctly on a specific unit.
+
+    Args:
+        ops_test: The ops test framework instance
+        unit_name: The name of the unit
+        restart_time: Point in time before the unit was restarted.
+
+    Returns:
+        whether Patroni is running correctly.
+    """
+    unit_ip = get_unit_address(ops_test, unit_name)
+    health_info = requests.get(f"http://{unit_ip}:8008/health").json()
+    postmaster_start_time = datetime.strptime(
+        health_info["postmaster_start_time"], "%Y-%m-%d %H:%M:%S.%f%z"
+    ).timestamp()
+    return postmaster_start_time > restart_time and health_info["state"] == "running"
+
+
 def build_application_name(series: str) -> str:
     """Return a composite application name combining application name and series."""
     return f"{DATABASE_APP_NAME}-{series}"
@@ -344,20 +375,21 @@ def get_application_units_ips(ops_test: OpsTest, application_name: str) -> List[
     return [unit.public_address for unit in ops_test.model.applications[application_name].units]
 
 
-async def get_password(ops_test: OpsTest, unit_name: str) -> str:
-    """Retrieve the operator user password using the action.
+async def get_password(ops_test: OpsTest, unit_name: str, username: str = "operator") -> str:
+    """Retrieve a user password using the action.
 
     Args:
         ops_test: ops_test instance.
         unit_name: the name of the unit.
+        username: the user to get the password.
 
     Returns:
-        the operator user password.
+        the user password.
     """
     unit = ops_test.model.units.get(unit_name)
-    action = await unit.run_action("get-password")
+    action = await unit.run_action("get-password", **{"username": username})
     result = await action.wait()
-    return result.results["operator-password"]
+    return result.results[f"{username}-password"]
 
 
 async def get_primary(ops_test: OpsTest, unit_name: str) -> str:
@@ -409,6 +441,41 @@ async def scale_application(ops_test: OpsTest, application_name: str, count: int
     )
 
 
+def restart_patroni(ops_test: OpsTest, unit_name: str) -> None:
+    """Restart Patroni on a specific unit.
+
+    Args:
+        ops_test: The ops test framework instance
+        unit_name: The name of the unit
+    """
+    unit_ip = get_unit_address(ops_test, unit_name)
+    requests.post(f"http://{unit_ip}:8008/restart")
+
+
+async def set_password(
+    ops_test: OpsTest, unit_name: str, username: str = "operator", password: str = None
+):
+    """Set a user password using the action.
+
+    Args:
+        ops_test: ops_test instance.
+        unit_name: the name of the unit.
+        username: the user to set the password.
+        password: optional password to use
+            instead of auto-generating
+
+    Returns:
+        the results from the action.
+    """
+    unit = ops_test.model.units.get(unit_name)
+    parameters = {"username": username}
+    if password is not None:
+        parameters["password"] = password
+    action = await unit.run_action("set-password", **parameters)
+    result = await action.wait()
+    return result.results
+
+
 def switchover(ops_test: OpsTest, current_primary: str, candidate: str = None) -> None:
     """Trigger a switchover.