[DPE-6752] Implement instance level predefined roles (#881)

* Implement instance level predefined roles

* Fix minor bug introduced while rebasing off of 16/edge

* Add integration test for charmed_read and charmed_dml roles

* Revert all major changes except introduction of predefined roles

* Sweep diff and minor bug fixes

* Avoid creating set_user extension

* Port Carl's fix for broken unit tests

* Bump postgresql charm lib for 16/edge to v1 due to backwards incompatible changes

* Re-add mistakenly removed patch statements

Author: shayancanonical

lib/charms/postgresql_k8s/v0/postgresql.py

@@ -31,11 +31,11 @@
 LIBID = "24ee217a54e840a598ff21a079c3e678"
 
 # Increment this major API version when introducing breaking changes
-LIBAPI = 0
+LIBAPI = 1
 
 # Increment this PATCH version before using `charmcraft publish-lib` or reset
 # to 0 if you are raising the major API version
-LIBPATCH = 52
+LIBPATCH = 0
 
 # Groups to distinguish HBA access
 ACCESS_GROUP_IDENTITY = "identity_access"
@@ -49,6 +49,11 @@
     ACCESS_GROUP_RELATION,
 ]
 
+ROLE_STATS = "charmed_stats"
+ROLE_READ = "charmed_read"
+ROLE_DML = "charmed_dml"
+ROLE_BACKUP = "charmed_backup"
+
 # Groups to distinguish database permissions
 PERMISSIONS_GROUP_ADMIN = "admin"
 
@@ -125,6 +130,15 @@ class PostgreSQLUpdateUserPasswordError(Exception):
     """Exception raised when updating a user password fails."""
 
 
+class PostgreSQLCreatePredefinedRolesError(Exception):
+    """Exception raised when creating predefined roles."""
+
+
+class PostgreSQLGrantDatabasePrivilegesToUserError(Exception):
+    """Exception raised when granting database privileges to user."""
+
+
+
 class PostgreSQL:
     """Class to encapsulate all operations related to interacting with PostgreSQL instance."""
 
@@ -326,6 +340,55 @@ def create_user(
             logger.error(f"Failed to create user: {e}")
             raise PostgreSQLCreateUserError() from e
 
+
+    def create_predefined_roles(self) -> None:
+        """Create predefined roles."""
+        role_to_queries = {
+            ROLE_STATS: [
+                f"CREATE ROLE {ROLE_STATS} NOSUPERUSER NOCREATEDB NOCREATEROLE NOREPLICATION NOLOGIN IN ROLE pg_monitor",
+            ],
+            ROLE_READ: [
+                f"CREATE ROLE {ROLE_READ} NOSUPERUSER NOCREATEDB NOCREATEROLE NOREPLICATION NOLOGIN IN ROLE pg_read_all_data",
+            ],
+            ROLE_DML: [
+                f"CREATE ROLE {ROLE_DML} NOSUPERUSER NOCREATEDB NOCREATEROLE NOREPLICATION NOLOGIN IN ROLE pg_write_all_data",
+            ],
+            ROLE_BACKUP: [
+                f"CREATE ROLE {ROLE_BACKUP} NOSUPERUSER NOCREATEDB NOCREATEROLE NOREPLICATION NOLOGIN IN ROLE pg_checkpoint",
+                f"GRANT {ROLE_STATS} TO {ROLE_BACKUP}",
+                f"GRANT execute ON FUNCTION pg_backup_start TO {ROLE_BACKUP}",
+                f"GRANT execute ON FUNCTION pg_backup_stop TO {ROLE_BACKUP}",
+                f"GRANT execute ON FUNCTION pg_create_restore_point TO {ROLE_BACKUP}",
+                f"GRANT execute ON FUNCTION pg_switch_wal TO {ROLE_BACKUP}",
+            ],
+        }
+
+        _, existing_roles = self.list_valid_privileges_and_roles()
+
+        try:
+            with self._connect_to_database() as connection, connection.cursor() as cursor:
+                for role, queries in role_to_queries.items():
+                    if role in existing_roles:
+                        logger.debug(f"Role {role} already exists")
+                        continue
+
+                    logger.info(f"Creating predefined role {role}")
+
+                    for query in queries:
+                        cursor.execute(SQL(query))
+        except psycopg2.Error as e:
+            logger.error(f"Failed to create predefined roles: {e}")
+            raise PostgreSQLCreatePredefinedRolesError() from e
+
+    def grant_database_privileges_to_user(self, user: str, database: str, privileges: list[str]) -> None:
+        """Grant the specified priviliges on the provided database for the user."""
+        try:
+            with self._connect_to_database() as connection, connection.cursor() as cursor:
+                cursor.execute(SQL("GRANT {} ON DATABASE {} TO {};").format(Identifier(", ".join(privileges)), Identifier(database), Identifier(user)))
+        except psycopg2.Error as e:
+            logger.error(f"Faield to grant privileges to user: {e}")
+            raise PostgreSQLGrantDatabasePrivilegesToUserError() from e
+
     def delete_user(self, user: str) -> None:
         """Deletes a database user.
 
@@ -727,7 +790,7 @@ def set_up_database(self, temp_location: Optional[str] = None) -> None:
                     )
                 self.create_user(
                     PERMISSIONS_GROUP_ADMIN,
-                    extra_user_roles=["pg_read_all_data", "pg_write_all_data"],
+                    extra_user_roles=[ROLE_READ, ROLE_DML],
                 )
                 cursor.execute("GRANT CONNECT ON DATABASE postgres TO admin;")
         except psycopg2.Error as e:

poetry.lock

@@ -6,6 +6,6 @@ name = "charmed-postgresql"
 
 [snap.revisions]
 # amd64
-x86_64 = "170"
+x86_64 = "182"
 # arm64
-aarch64 = "169"
+aarch64 = "181"

refresh_versions.toml

@@ -27,7 +27,6 @@
 from jinja2 import Template
 from ops.charm import ActionEvent, HookEvent
 from ops.framework import Object
-from ops.jujuversion import JujuVersion
 from ops.model import ActiveStatus, MaintenanceStatus
 from tenacity import RetryError, Retrying, stop_after_attempt, wait_fixed
 
@@ -893,12 +892,11 @@ def _on_create_backup_action(self, event) -> None:
 
         # Test uploading metadata to S3 to test credentials before backup.
         datetime_backup_requested = datetime.now().strftime("%Y-%m-%dT%H:%M:%SZ")
-        juju_version = JujuVersion.from_environ()
         metadata = f"""Date Backup Requested: {datetime_backup_requested}
 Model Name: {self.model.name}
 Application Name: {self.model.app.name}
 Unit Name: {self.charm.unit.name}
-Juju Version: {juju_version!s}
+Juju Version: {self.charm.model.juju_version!s}
 """
         if not self._upload_content_to_s3(
             metadata,

src/backups.py

@@ -33,10 +33,14 @@
     ACCESS_GROUP_IDENTITY,
     ACCESS_GROUPS,
     REQUIRED_PLUGINS,
+    ROLE_BACKUP,
+    ROLE_STATS,
     PostgreSQL,
+    PostgreSQLCreatePredefinedRolesError,
     PostgreSQLCreateUserError,
     PostgreSQLEnableDisableExtensionError,
     PostgreSQLGetCurrentTimelineError,
+    PostgreSQLGrantDatabasePrivilegesToUserError,
     PostgreSQLListUsersError,
     PostgreSQLUpdateUserPasswordError,
 )
@@ -264,8 +268,7 @@ def __init__(self, *args):
             deleted_label=SECRET_DELETED_LABEL,
         )
 
-        run_cmd = "/usr/bin/juju-exec"
-        self._observer = ClusterTopologyObserver(self, run_cmd)
+        self._observer = ClusterTopologyObserver(self, "/usr/bin/juju-exec")
         self._rotate_logs = RotateLogs(self)
         self.framework.observe(self.on.cluster_topology_change, self._on_cluster_topology_change)
         self.framework.observe(self.on.install, self._on_install)
@@ -1682,7 +1685,7 @@ def _setup_ldap_sync(self, postgres_snap: snap.Snap | None = None) -> None:
         logger.debug("Starting LDAP sync service")
         postgres_snap.restart(services=["ldap-sync"])
 
-    def _start_primary(self, event: StartEvent) -> None:
+    def _start_primary(self, event: StartEvent) -> None:  # noqa: C901
         """Bootstrap the cluster."""
         # Set some information needed by Patroni to bootstrap the cluster.
         if not self._patroni.bootstrap_cluster():
@@ -1696,6 +1699,25 @@ def _start_primary(self, event: StartEvent) -> None:
             event.defer()
             return
 
+        if not self._can_connect_to_postgresql:
+            logger.debug("Deferring on_start: awaiting for database to start")
+            self.unit.status = WaitingStatus("awaiting for database to start")
+            event.defer()
+            return
+
+        if not self.primary_endpoint:
+            logger.debug("Deferrring on_start: awaitng start of the primary")
+            self.unit.status = WaitingStatus("awaiting start of the primary")
+            event.defer()
+            return
+
+        try:
+            self.postgresql.create_predefined_roles()
+        except PostgreSQLCreatePredefinedRolesError as e:
+            logger.exception(e)
+            self.unit.status = BlockedStatus("Failed to create pre-defined roles")
+            return
+
         # Create the default postgres database user that is needed for some
         # applications (not charms) like Landscape Server.
         try:
@@ -1704,16 +1726,25 @@ def _start_primary(self, event: StartEvent) -> None:
             users = self.postgresql.list_users()
             if "postgres" not in users:
                 self.postgresql.create_user("postgres", new_password(), admin=True)
-                # Create the backup user.
+            # Create the backup user.
             if BACKUP_USER not in users:
-                self.postgresql.create_user(BACKUP_USER, new_password(), admin=True)
+                self.postgresql.create_user(
+                    BACKUP_USER, new_password(), extra_user_roles=[ROLE_BACKUP]
+                )
+                self.postgresql.grant_database_privileges_to_user(
+                    BACKUP_USER, "postgres", ["connect"]
+                )
             if MONITORING_USER not in users:
                 # Create the monitoring user.
                 self.postgresql.create_user(
                     MONITORING_USER,
                     self.get_secret(APP_SCOPE, MONITORING_PASSWORD_KEY),
-                    extra_user_roles=["pg_monitor"],
+                    extra_user_roles=[ROLE_STATS],
                 )
+        except PostgreSQLGrantDatabasePrivilegesToUserError as e:
+            logger.exception(e)
+            self.unit.status = BlockedStatus("Failed to grant database privileges to user")
+            return
         except PostgreSQLCreateUserError as e:
             logger.exception(e)
             self.set_unit_status(BlockedStatus("Failed to create postgres user"))
@@ -1796,13 +1827,14 @@ def _update_admin_password(self, admin_secret_id: str) -> None:
             return
 
         try:
+            updateable_users = [*SYSTEM_USERS, BACKUP_USER]
             # get the secret content and check each user configured there
             # only SYSTEM_USERS with changed passwords are processed, all others ignored
             updated_passwords = self.get_secret_from_id(secret_id=admin_secret_id)
             for user, password in list(updated_passwords.items()):
-                if user not in SYSTEM_USERS:
+                if user not in updateable_users:
                     logger.error(
-                        f"Can only update system users: {', '.join(SYSTEM_USERS)} not {user}"
+                        f"Can only update system users: {', '.join(updateable_users)} not {user}"
                     )
                     updated_passwords.pop(user)
                     continue

src/charm.py

@@ -27,7 +27,7 @@
 PATRONI_SERVICE_NAME = "snap.charmed-postgresql.patroni.service"
 PATRONI_SERVICE_DEFAULT_PATH = f"/etc/systemd/system/{PATRONI_SERVICE_NAME}"
 # List of system usernames needed for correct work of the charm/workload.
-SYSTEM_USERS = [BACKUP_USER, REPLICATION_USER, REWIND_USER, USER, MONITORING_USER]
+SYSTEM_USERS = [REPLICATION_USER, REWIND_USER, USER, MONITORING_USER]
 
 # Snap constants.
 PGBACKREST_EXECUTABLE = "charmed-postgresql.pgbackrest"

src/constants.py

@@ -143,7 +143,7 @@ def _on_database_requested(self, event: DatabaseRequestedEvent) -> None:
                 BlockedStatus(
                     e.message
                     if issubclass(type(e), PostgreSQLCreateUserError) and e.message is not None
-                    else f"Failed to initialize {self.relation_name} relation"
+                    else f"Failed to initialize relation {self.relation_name}"
                 )
             )
 
@@ -295,6 +295,11 @@ def _update_unit_status(self, relation: Relation) -> None:
             and self.charm.unit.status.message == INVALID_EXTRA_USER_ROLE_BLOCKING_MESSAGE
         ) and not self.check_for_invalid_extra_user_roles(relation.id):
             self.charm.set_unit_status(ActiveStatus())
+        if (
+            self.charm.is_blocked
+            and "Failed to initialize relation" in self.charm.unit.status.message
+        ):
+            self.charm.set_unit_status(ActiveStatus())
 
         self._update_unit_status_on_blocking_endpoint_simultaneously()
 

src/relations/postgresql_provider.py

@@ -1078,11 +1078,24 @@ async def get_detached_storages(ops_test: OpsTest) -> list[str]:
 
 async def check_password_auth(ops_test: OpsTest, unit_name: str) -> bool:
     """Checks if "operator" password is valid for current postgresql db."""
-    stdout = await run_command_on_unit(
-        ops_test,
+    complete_command = [
+        "exec",
+        "--unit",
         unit_name,
-        """grep -E 'password authentication failed for user' /var/snap/charmed-postgresql/common/var/log/postgresql/postgresql*""",
-    )
+        "--",
+        "grep",
+        "-E",
+        "'password'",
+        "/var/snap/charmed-postgresql/common/var/log/postgresql/postgresql*",
+    ]
+    return_code, stdout, _ = await ops_test.juju(*complete_command)
+    if return_code != 0:
+        raise Exception(
+            "Expected command %s to succeed. Instead it failed: %s with code: ",
+            complete_command,
+            stdout,
+            return_code,
+        )
     return 'password authentication failed for user "operator"' not in stdout