Skip to content

Commit cfb2956

Browse files
[DPE-7584] Fix temp tablespace directory permissions (#10)
* Fix temp tablespace directory permissions Signed-off-by: Marcelo Henrique Neppel <[email protected]> * Move permissions value to a literal Signed-off-by: Marcelo Henrique Neppel <[email protected]> * Move snap user to a literal Signed-off-by: Marcelo Henrique Neppel <[email protected]> * Bumb version to 16.0.0 Signed-off-by: Marcelo Henrique Neppel <[email protected]> --------- Signed-off-by: Marcelo Henrique Neppel <[email protected]>
1 parent 3c33016 commit cfb2956

File tree

7 files changed

+184
-3
lines changed

7 files changed

+184
-3
lines changed

pyproject.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
[project]
55
name = "postgresql-charms-single-kernel"
66
description = "Shared and reusable code for PostgreSQL-related charms"
7-
version = "0.0.1"
7+
version = "16.0.0"
88
readme = "README.md"
99
license = "Apache-2.0"
1010
authors = [

single_kernel_postgresql/config/literals.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,9 @@
55
This module should contain the literals used in the charms (paths, enums, etc).
66
"""
77

8+
# Permissions.
9+
POSTGRESQL_STORAGE_PERMISSIONS = 0o700
10+
811
# Relations.
912
PEER = "database-peers"
1013

@@ -13,5 +16,6 @@
1316
MONITORING_USER = "monitoring"
1417
REPLICATION_USER = "replication"
1518
REWIND_USER = "rewind"
19+
SNAP_USER = "_daemon_"
1620
USER = "operator"
1721
SYSTEM_USERS = [MONITORING_USER, REPLICATION_USER, REWIND_USER, USER]
Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
# Copyright 2025 Canonical Ltd.
2+
# See LICENSE file for licensing details.
3+
"""Filesystem utilities."""
4+
5+
import os
6+
import pwd
7+
8+
from ..config.literals import SNAP_USER
9+
10+
11+
def change_owner(path: str) -> None:
12+
"""Change the ownership of a file or a directory to the snap user.
13+
14+
Args:
15+
path: path to a file or directory.
16+
"""
17+
# Get the uid/gid for the snap user.
18+
user_database = pwd.getpwnam(SNAP_USER)
19+
# Set the correct ownership for the file or directory.
20+
os.chown(path, uid=user_database.pw_uid, gid=user_database.pw_gid)

single_kernel_postgresql/utils/postgresql.py

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,14 +20,16 @@
2020
"""
2121

2222
import logging
23+
import os
2324
from collections import OrderedDict
2425
from typing import Dict, List, Optional, Set, Tuple
2526

2627
import psycopg2
2728
from ops import ConfigData
2829
from psycopg2.sql import SQL, Identifier, Literal
2930

30-
from ..config.literals import BACKUP_USER, SYSTEM_USERS
31+
from ..config.literals import BACKUP_USER, POSTGRESQL_STORAGE_PERMISSIONS, SYSTEM_USERS
32+
from .filesystem import change_owner
3133

3234
# Groups to distinguish HBA access
3335
ACCESS_GROUP_IDENTITY = "identity_access"
@@ -1071,6 +1073,10 @@ def set_up_database(self, temp_location: Optional[str] = None) -> None:
10711073
cursor = connection.cursor()
10721074

10731075
if temp_location is not None:
1076+
# Fix permissions on the temporary tablespace location when a reboot happens and tmpfs is being used.
1077+
change_owner(temp_location)
1078+
os.chmod(temp_location, POSTGRESQL_STORAGE_PERMISSIONS)
1079+
10741080
cursor.execute("SELECT TRUE FROM pg_tablespace WHERE spcname='temp';")
10751081
if cursor.fetchone() is None:
10761082
cursor.execute(f"CREATE TABLESPACE temp LOCATION '{temp_location}';")

tests/unit/test_filesystem.py

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
# Copyright 2025 Canonical Ltd.
2+
# See LICENSE file for licensing details.
3+
from tempfile import NamedTemporaryFile
4+
from unittest.mock import MagicMock, patch
5+
6+
import pytest
7+
from single_kernel_postgresql.utils.filesystem import change_owner
8+
9+
10+
def test_change_owner_calls_pwd_and_os_chown_with_daemon_user():
11+
with (
12+
patch("single_kernel_postgresql.utils.filesystem.pwd.getpwnam") as getpwnam,
13+
patch("single_kernel_postgresql.utils.filesystem.os.chown") as chown,
14+
NamedTemporaryFile(delete=True) as tmp,
15+
):
16+
# Simulate pwd entry
17+
pw_entry = MagicMock()
18+
pw_entry.pw_uid = 1234
19+
pw_entry.pw_gid = 4321
20+
getpwnam.return_value = pw_entry
21+
22+
change_owner(tmp.name)
23+
24+
getpwnam.assert_called_once_with("_daemon_")
25+
chown.assert_called_once_with(tmp.name, uid=1234, gid=4321)
26+
27+
28+
def test_change_owner_raises_when_user_missing():
29+
# When the _daemon_ user is not present, pwd.getpwnam raises KeyError
30+
with (
31+
patch("single_kernel_postgresql.utils.filesystem.pwd.getpwnam", side_effect=KeyError),
32+
pytest.raises(KeyError),
33+
NamedTemporaryFile(delete=True) as tmp,
34+
):
35+
change_owner(tmp.name)
36+
37+
38+
def test_change_owner_bubbles_up_os_error():
39+
# Ensure we surface OSError coming from os.chown
40+
with (
41+
patch("single_kernel_postgresql.utils.filesystem.pwd.getpwnam") as getpwnam,
42+
patch("single_kernel_postgresql.utils.filesystem.os.chown", side_effect=OSError("denied")),
43+
NamedTemporaryFile(delete=True) as tmp,
44+
):
45+
entry = MagicMock()
46+
entry.pw_uid = 1
47+
entry.pw_gid = 1
48+
getpwnam.return_value = entry
49+
with pytest.raises(OSError):
50+
change_owner(tmp.name)

tests/unit/test_postgresql.py

Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,9 +11,11 @@
1111
from single_kernel_postgresql.utils.postgresql import (
1212
ACCESS_GROUP_INTERNAL,
1313
ACCESS_GROUPS,
14+
ROLE_DATABASES_OWNER,
1415
PostgreSQL,
1516
PostgreSQLCreateDatabaseError,
1617
PostgreSQLCreateUserError,
18+
PostgreSQLDatabasesSetupError,
1719
PostgreSQLGetLastArchivedWALError,
1820
PostgreSQLUndefinedHostError,
1921
PostgreSQLUndefinedPasswordError,
@@ -315,6 +317,105 @@ def test_validate_group_map(harness):
315317
assert harness.charm.postgresql.validate_group_map("ldap_group ldap_test_group") is False
316318

317319

320+
def test_set_up_database_with_temp_tablespace_and_missing_owner_role(harness):
321+
with (
322+
patch(
323+
"single_kernel_postgresql.utils.postgresql.PostgreSQL._connect_to_database"
324+
) as _connect_to_database,
325+
patch("single_kernel_postgresql.utils.postgresql.PostgreSQL.set_up_login_hook_function"),
326+
patch(
327+
"single_kernel_postgresql.utils.postgresql.PostgreSQL.set_up_predefined_catalog_roles_function"
328+
),
329+
patch("single_kernel_postgresql.utils.postgresql.PostgreSQL.create_user") as _create_user,
330+
patch("single_kernel_postgresql.utils.postgresql.change_owner") as _change_owner,
331+
patch("single_kernel_postgresql.utils.postgresql.os.chmod") as _chmod,
332+
):
333+
# First connection (non-context) for temp tablespace
334+
execute_direct = _connect_to_database.return_value.cursor.return_value.execute
335+
fetchone_direct = _connect_to_database.return_value.cursor.return_value.fetchone
336+
fetchone_direct.return_value = None
337+
338+
# Second and third connections are context-managed
339+
execute_cm = _connect_to_database.return_value.__enter__.return_value.cursor.return_value.__enter__.return_value.execute
340+
fetchone_cm = _connect_to_database.return_value.__enter__.return_value.cursor.return_value.__enter__.return_value.fetchone
341+
fetchone_cm.return_value = None # owner role missing
342+
343+
harness.charm.postgresql.set_up_database(temp_location="/var/lib/postgresql/tmp")
344+
345+
# Ensure permission fixes applied
346+
_change_owner.assert_called_once_with("/var/lib/postgresql/tmp")
347+
_chmod.assert_called_once_with("/var/lib/postgresql/tmp", 0o700)
348+
349+
# Validate temp tablespace operations
350+
execute_direct.assert_has_calls([
351+
call("SELECT TRUE FROM pg_tablespace WHERE spcname='temp';"),
352+
call("CREATE TABLESPACE temp LOCATION '/var/lib/postgresql/tmp';"),
353+
call("GRANT CREATE ON TABLESPACE temp TO public;"),
354+
])
355+
356+
# create_user called for missing owner role
357+
_create_user.assert_called_once_with(
358+
ROLE_DATABASES_OWNER, can_create_database=True, extra_user_roles=["charmed_dml"]
359+
)
360+
361+
# Final revokes and grants
362+
system_users = harness.charm.postgresql.system_users
363+
expected = [
364+
call("REVOKE ALL PRIVILEGES ON DATABASE postgres FROM PUBLIC;"),
365+
call("REVOKE CREATE ON SCHEMA public FROM PUBLIC;"),
366+
*[
367+
call(SQL("GRANT ALL PRIVILEGES ON DATABASE postgres TO {};").format(Identifier(u)))
368+
for u in system_users
369+
],
370+
]
371+
execute_cm.assert_has_calls(expected, any_order=False)
372+
373+
374+
def test_set_up_database_no_temp_and_existing_owner_role(harness):
375+
with (
376+
patch(
377+
"single_kernel_postgresql.utils.postgresql.PostgreSQL._connect_to_database"
378+
) as _connect_to_database,
379+
patch("single_kernel_postgresql.utils.postgresql.PostgreSQL.set_up_login_hook_function"),
380+
patch(
381+
"single_kernel_postgresql.utils.postgresql.PostgreSQL.set_up_predefined_catalog_roles_function"
382+
),
383+
patch("single_kernel_postgresql.utils.postgresql.PostgreSQL.create_user") as _create_user,
384+
):
385+
# owner role exists
386+
fetchone = _connect_to_database.return_value.__enter__.return_value.cursor.return_value.__enter__.return_value.fetchone
387+
fetchone.return_value = True
388+
389+
harness.charm.postgresql.set_up_database()
390+
391+
_create_user.assert_not_called()
392+
393+
execute = _connect_to_database.return_value.__enter__.return_value.cursor.return_value.__enter__.return_value.execute
394+
system_users = harness.charm.postgresql.system_users
395+
execute.assert_has_calls([
396+
call("REVOKE ALL PRIVILEGES ON DATABASE postgres FROM PUBLIC;"),
397+
call("REVOKE CREATE ON SCHEMA public FROM PUBLIC;"),
398+
*[
399+
call(SQL("GRANT ALL PRIVILEGES ON DATABASE postgres TO {};").format(Identifier(u)))
400+
for u in system_users
401+
],
402+
])
403+
404+
405+
def test_set_up_database_raises_wrapped_error(harness):
406+
with (
407+
patch(
408+
"single_kernel_postgresql.utils.postgresql.PostgreSQL._connect_to_database"
409+
) as _connect_to_database,
410+
patch("single_kernel_postgresql.utils.postgresql.change_owner"),
411+
patch("single_kernel_postgresql.utils.postgresql.os.chmod"),
412+
):
413+
execute_direct = _connect_to_database.return_value.cursor.return_value.execute
414+
execute_direct.side_effect = psycopg2.Error
415+
with pytest.raises(PostgreSQLDatabasesSetupError):
416+
harness.charm.postgresql.set_up_database(temp_location="/tmp")
417+
418+
318419
def test_connect_to_database():
319420
# Error on no host
320421
pg = PostgreSQL(None, None, "operator", None, "postgres", None)

uv.lock

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)