Skip to content

Commit c7b0383

Browse files
Fix permissions comparison (#17)
Signed-off-by: Marcelo Henrique Neppel <[email protected]>
1 parent 2fef585 commit c7b0383

File tree

2 files changed

+46
-9
lines changed

2 files changed

+46
-9
lines changed

single_kernel_postgresql/utils/postgresql.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1079,7 +1079,7 @@ def set_up_database(self, temp_location: Optional[str] = None) -> None:
10791079
temp_location_stats = os.stat(temp_location)
10801080
if (
10811081
pwd.getpwuid(temp_location_stats.st_uid).pw_name != SNAP_USER
1082-
or temp_location_stats.st_mode != POSTGRESQL_STORAGE_PERMISSIONS
1082+
or int(temp_location_stats.st_mode & 0o777) != POSTGRESQL_STORAGE_PERMISSIONS
10831083
):
10841084
change_owner(temp_location)
10851085
os.chmod(temp_location, POSTGRESQL_STORAGE_PERMISSIONS)

tests/unit/test_postgresql.py

Lines changed: 45 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -340,8 +340,8 @@ def test_set_up_database_with_temp_tablespace_and_missing_owner_role(harness):
340340
patch("single_kernel_postgresql.utils.postgresql.os.stat") as _stat,
341341
patch("single_kernel_postgresql.utils.postgresql.pwd.getpwuid") as _getpwuid,
342342
):
343-
# Simulate a temp location owned by wrong user/permissions to trigger fixup
344-
stat_result = type("stat_result", (), {"st_uid": 0, "st_mode": 0o755})
343+
# Simulate a temp location owned by wrong user/permissions to trigger fixup (33188 means 0o644)
344+
stat_result = type("stat_result", (), {"st_uid": 0, "st_mode": 33188})
345345
_stat.return_value = stat_result
346346
_getpwuid.return_value.pw_name = "root"
347347

@@ -402,10 +402,8 @@ def test_set_up_database_owner_mismatch_triggers_rename_and_fix(harness):
402402
patch("single_kernel_postgresql.utils.postgresql.pwd.getpwuid") as _getpwuid,
403403
patch("single_kernel_postgresql.utils.postgresql.datetime") as _dt,
404404
):
405-
# Owner differs, permissions are correct
406-
stat_result = type(
407-
"stat_result", (), {"st_uid": 0, "st_mode": POSTGRESQL_STORAGE_PERMISSIONS}
408-
)
405+
# Owner differs, permissions are correct (16832 means 0o700)
406+
stat_result = type("stat_result", (), {"st_uid": 0, "st_mode": 16832})
409407
_stat.return_value = stat_result
410408
_getpwuid.return_value.pw_name = "root"
411409

@@ -440,8 +438,8 @@ def test_set_up_database_permissions_mismatch_triggers_rename_and_fix(harness):
440438
patch("single_kernel_postgresql.utils.postgresql.pwd.getpwuid") as _getpwuid,
441439
patch("single_kernel_postgresql.utils.postgresql.datetime") as _dt,
442440
):
443-
# Owner matches SNAP_USER, permissions differ
444-
stat_result = type("stat_result", (), {"st_uid": 0, "st_mode": 0o755})
441+
# Owner matches SNAP_USER, permissions differ (33188 means 0o644)
442+
stat_result = type("stat_result", (), {"st_uid": 0, "st_mode": 33188})
445443
_stat.return_value = stat_result
446444
_getpwuid.return_value.pw_name = SNAP_USER
447445

@@ -716,3 +714,42 @@ def test_create_user():
716714

717715
with pytest.raises(PostgreSQLCreateUserError):
718716
pg.create_user("username", "password")
717+
718+
719+
def test_set_up_database_owner_and_permissions_match_no_rename_or_fix(harness):
720+
with (
721+
patch(
722+
"single_kernel_postgresql.utils.postgresql.PostgreSQL._connect_to_database"
723+
) as _connect_to_database,
724+
patch("single_kernel_postgresql.utils.postgresql.PostgreSQL.set_up_login_hook_function"),
725+
patch(
726+
"single_kernel_postgresql.utils.postgresql.PostgreSQL.set_up_predefined_catalog_roles_function"
727+
),
728+
patch("single_kernel_postgresql.utils.postgresql.change_owner") as _change_owner,
729+
patch("single_kernel_postgresql.utils.postgresql.os.chmod") as _chmod,
730+
patch("single_kernel_postgresql.utils.postgresql.os.stat") as _stat,
731+
patch("single_kernel_postgresql.utils.postgresql.pwd.getpwuid") as _getpwuid,
732+
):
733+
# Owner matches SNAP_USER and permissions are correct (16832 means 0o700)
734+
stat_result = type("stat_result", (), {"st_uid": 0, "st_mode": 16832})
735+
_stat.return_value = stat_result
736+
_getpwuid.return_value.pw_name = SNAP_USER
737+
738+
execute_direct = _connect_to_database.return_value.cursor.return_value.execute
739+
fetchone_direct = _connect_to_database.return_value.cursor.return_value.fetchone
740+
# No mismatch, so the existence check returns True and no creation/rename occurs
741+
fetchone_direct.return_value = True
742+
743+
harness.charm.postgresql.set_up_database(temp_location="/var/lib/postgresql/tmp")
744+
745+
# No permission/owner fix should be performed
746+
_change_owner.assert_not_called()
747+
_chmod.assert_not_called()
748+
749+
# It should check for temp tablespace existence
750+
execute_direct.assert_any_call("SELECT TRUE FROM pg_tablespace WHERE spcname='temp';")
751+
752+
# Ensure that no rename was attempted
753+
for c in execute_direct.call_args_list:
754+
if c.args:
755+
assert "ALTER TABLESPACE temp RENAME TO" not in c.args[0]

0 commit comments

Comments
 (0)