diff --git a/pyproject.toml b/pyproject.toml index c9bcbd9..bfe4124 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -4,7 +4,7 @@ [project] name = "postgresql-charms-single-kernel" description = "Shared and reusable code for PostgreSQL-related charms" -version = "16.0.0" +version = "16.0.1" readme = "README.md" license = "Apache-2.0" authors = [ diff --git a/single_kernel_postgresql/utils/postgresql.py b/single_kernel_postgresql/utils/postgresql.py index 2ad421c..d0f88cd 100644 --- a/single_kernel_postgresql/utils/postgresql.py +++ b/single_kernel_postgresql/utils/postgresql.py @@ -21,14 +21,16 @@ import logging import os +import pwd from collections import OrderedDict +from datetime import datetime, timezone from typing import Dict, List, Optional, Set, Tuple import psycopg2 from ops import ConfigData from psycopg2.sql import SQL, Identifier, Literal -from ..config.literals import BACKUP_USER, POSTGRESQL_STORAGE_PERMISSIONS, SYSTEM_USERS +from ..config.literals import BACKUP_USER, POSTGRESQL_STORAGE_PERMISSIONS, SNAP_USER, SYSTEM_USERS from .filesystem import change_owner # Groups to distinguish HBA access @@ -1074,9 +1076,31 @@ def set_up_database(self, temp_location: Optional[str] = None) -> None: if temp_location is not None: # Fix permissions on the temporary tablespace location when a reboot happens and tmpfs is being used. - change_owner(temp_location) - os.chmod(temp_location, POSTGRESQL_STORAGE_PERMISSIONS) + temp_location_stats = os.stat(temp_location) + if ( + pwd.getpwuid(temp_location_stats.st_uid).pw_name != SNAP_USER + or temp_location_stats.st_mode != POSTGRESQL_STORAGE_PERMISSIONS + ): + change_owner(temp_location) + os.chmod(temp_location, POSTGRESQL_STORAGE_PERMISSIONS) + # Rename existing temp tablespace if it exists, instead of dropping it. + cursor.execute("SELECT TRUE FROM pg_tablespace WHERE spcname='temp';") + if cursor.fetchone() is not None: + new_name = f"temp_{datetime.now(timezone.utc).strftime('%Y%m%d%H%M%S')}" + cursor.execute(f"ALTER TABLESPACE temp RENAME TO {new_name};") + + # List temp tablespaces with suffix for operator follow-up cleanup and log them. + cursor.execute( + "SELECT spcname FROM pg_tablespace WHERE spcname LIKE 'temp_%';" + ) + temp_tbls = sorted([row[0] for row in cursor.fetchall()]) + logger.info( + "There are %d temp tablespaces that should be checked and removed: %s", + len(temp_tbls), + ", ".join(temp_tbls), + ) + # Ensure a fresh temp tablespace exists at the expected location. cursor.execute("SELECT TRUE FROM pg_tablespace WHERE spcname='temp';") if cursor.fetchone() is None: cursor.execute(f"CREATE TABLESPACE temp LOCATION '{temp_location}';") diff --git a/tests/unit/test_postgresql.py b/tests/unit/test_postgresql.py index 340c3e1..09cf0e5 100644 --- a/tests/unit/test_postgresql.py +++ b/tests/unit/test_postgresql.py @@ -1,17 +1,24 @@ # Copyright 2025 Canonical Ltd. # See LICENSE file for licensing details. +# ruff: noqa: I001 from unittest.mock import call, patch, sentinel +from datetime import datetime, timezone, UTC import psycopg2 import pytest from ops.testing import Harness -from psycopg2.sql import SQL, Composed, Identifier, Literal +from psycopg2.sql import Composed, Identifier, Literal, SQL + from single_kernel_postgresql.abstract_charm import AbstractPostgreSQLCharm -from single_kernel_postgresql.config.literals import PEER, SYSTEM_USERS +from single_kernel_postgresql.config.literals import ( + PEER, + POSTGRESQL_STORAGE_PERMISSIONS, + SNAP_USER, + SYSTEM_USERS, +) from single_kernel_postgresql.utils.postgresql import ( - ACCESS_GROUP_INTERNAL, ACCESS_GROUPS, - ROLE_DATABASES_OWNER, + ACCESS_GROUP_INTERNAL, PostgreSQL, PostgreSQLCreateDatabaseError, PostgreSQLCreateUserError, @@ -19,6 +26,7 @@ PostgreSQLGetLastArchivedWALError, PostgreSQLUndefinedHostError, PostgreSQLUndefinedPasswordError, + ROLE_DATABASES_OWNER, ) @@ -329,7 +337,14 @@ def test_set_up_database_with_temp_tablespace_and_missing_owner_role(harness): patch("single_kernel_postgresql.utils.postgresql.PostgreSQL.create_user") as _create_user, patch("single_kernel_postgresql.utils.postgresql.change_owner") as _change_owner, patch("single_kernel_postgresql.utils.postgresql.os.chmod") as _chmod, + patch("single_kernel_postgresql.utils.postgresql.os.stat") as _stat, + patch("single_kernel_postgresql.utils.postgresql.pwd.getpwuid") as _getpwuid, ): + # Simulate a temp location owned by wrong user/permissions to trigger fixup + stat_result = type("stat_result", (), {"st_uid": 0, "st_mode": 0o755}) + _stat.return_value = stat_result + _getpwuid.return_value.pw_name = "root" + # First connection (non-context) for temp tablespace execute_direct = _connect_to_database.return_value.cursor.return_value.execute fetchone_direct = _connect_to_database.return_value.cursor.return_value.fetchone @@ -346,8 +361,9 @@ def test_set_up_database_with_temp_tablespace_and_missing_owner_role(harness): _change_owner.assert_called_once_with("/var/lib/postgresql/tmp") _chmod.assert_called_once_with("/var/lib/postgresql/tmp", 0o700) - # Validate temp tablespace operations + # Validate temp tablespace operations: check existence and create/grant when missing execute_direct.assert_has_calls([ + call("SELECT TRUE FROM pg_tablespace WHERE spcname='temp';"), call("SELECT TRUE FROM pg_tablespace WHERE spcname='temp';"), call("CREATE TABLESPACE temp LOCATION '/var/lib/postgresql/tmp';"), call("GRANT CREATE ON TABLESPACE temp TO public;"), @@ -371,6 +387,80 @@ def test_set_up_database_with_temp_tablespace_and_missing_owner_role(harness): execute_cm.assert_has_calls(expected, any_order=False) +def test_set_up_database_owner_mismatch_triggers_rename_and_fix(harness): + with ( + patch( + "single_kernel_postgresql.utils.postgresql.PostgreSQL._connect_to_database" + ) as _connect_to_database, + patch("single_kernel_postgresql.utils.postgresql.PostgreSQL.set_up_login_hook_function"), + patch( + "single_kernel_postgresql.utils.postgresql.PostgreSQL.set_up_predefined_catalog_roles_function" + ), + patch("single_kernel_postgresql.utils.postgresql.change_owner") as _change_owner, + patch("single_kernel_postgresql.utils.postgresql.os.chmod") as _chmod, + patch("single_kernel_postgresql.utils.postgresql.os.stat") as _stat, + patch("single_kernel_postgresql.utils.postgresql.pwd.getpwuid") as _getpwuid, + patch("single_kernel_postgresql.utils.postgresql.datetime") as _dt, + ): + # Owner differs, permissions are correct + stat_result = type( + "stat_result", (), {"st_uid": 0, "st_mode": POSTGRESQL_STORAGE_PERMISSIONS} + ) + _stat.return_value = stat_result + _getpwuid.return_value.pw_name = "root" + + # Mock datetime.now(timezone.utc) to a fixed timestamp + _dt.now.return_value = datetime(2025, 1, 1, 1, 2, 3, tzinfo=UTC) + _dt.timezone = timezone # ensure timezone.utc is available in the patch target + + execute_direct = _connect_to_database.return_value.cursor.return_value.execute + fetchone_direct = _connect_to_database.return_value.cursor.return_value.fetchone + fetchone_direct.side_effect = [True, None] + + harness.charm.postgresql.set_up_database(temp_location="/var/lib/postgresql/tmp") + + _change_owner.assert_called_once_with("/var/lib/postgresql/tmp") + _chmod.assert_called_once_with("/var/lib/postgresql/tmp", POSTGRESQL_STORAGE_PERMISSIONS) + execute_direct.assert_any_call("SELECT TRUE FROM pg_tablespace WHERE spcname='temp';") + execute_direct.assert_any_call("ALTER TABLESPACE temp RENAME TO temp_20250101010203;") + + +def test_set_up_database_permissions_mismatch_triggers_rename_and_fix(harness): + with ( + patch( + "single_kernel_postgresql.utils.postgresql.PostgreSQL._connect_to_database" + ) as _connect_to_database, + patch("single_kernel_postgresql.utils.postgresql.PostgreSQL.set_up_login_hook_function"), + patch( + "single_kernel_postgresql.utils.postgresql.PostgreSQL.set_up_predefined_catalog_roles_function" + ), + patch("single_kernel_postgresql.utils.postgresql.change_owner") as _change_owner, + patch("single_kernel_postgresql.utils.postgresql.os.chmod") as _chmod, + patch("single_kernel_postgresql.utils.postgresql.os.stat") as _stat, + patch("single_kernel_postgresql.utils.postgresql.pwd.getpwuid") as _getpwuid, + patch("single_kernel_postgresql.utils.postgresql.datetime") as _dt, + ): + # Owner matches SNAP_USER, permissions differ + stat_result = type("stat_result", (), {"st_uid": 0, "st_mode": 0o755}) + _stat.return_value = stat_result + _getpwuid.return_value.pw_name = SNAP_USER + + # Mock datetime.now(timezone.utc) to a fixed timestamp + _dt.now.return_value = datetime(2025, 1, 1, 1, 2, 3, tzinfo=UTC) + _dt.timezone = timezone + + execute_direct = _connect_to_database.return_value.cursor.return_value.execute + fetchone_direct = _connect_to_database.return_value.cursor.return_value.fetchone + fetchone_direct.side_effect = [True, None] + + harness.charm.postgresql.set_up_database(temp_location="/var/lib/postgresql/tmp") + + _change_owner.assert_called_once_with("/var/lib/postgresql/tmp") + _chmod.assert_called_once_with("/var/lib/postgresql/tmp", POSTGRESQL_STORAGE_PERMISSIONS) + execute_direct.assert_any_call("SELECT TRUE FROM pg_tablespace WHERE spcname='temp';") + execute_direct.assert_any_call("ALTER TABLESPACE temp RENAME TO temp_20250101010203;") + + def test_set_up_database_no_temp_and_existing_owner_role(harness): with ( patch( diff --git a/uv.lock b/uv.lock index 3f2527c..470ee73 100644 --- a/uv.lock +++ b/uv.lock @@ -268,7 +268,7 @@ wheels = [ [[package]] name = "postgresql-charms-single-kernel" -version = "16.0.0" +version = "16.0.1" source = { virtual = "." } [package.dev-dependencies]