Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
69 commits
Select commit Hold shift + click to select a range
e1a14e0
Implement instance level predefined roles
shayancanonical May 6, 2025
23c1ea0
Merge branch '16/edge' into feature/16_predefined_instance_roles
shayancanonical May 6, 2025
a5c6442
Fix minor bug introduced while rebasing off of 16/edge
shayancanonical May 6, 2025
09c4a71
Add integration test for charmed_read and charmed_dml roles
shayancanonical May 6, 2025
d778bba
Revert all major changes except introduction of predefined roles
shayancanonical May 7, 2025
873132e
Sweep diff and minor bug fixes
shayancanonical May 7, 2025
0dc8d3d
Avoid creating set_user extension
shayancanonical May 7, 2025
1764971
Merge branch '16/edge' into feature/16_predefined_instance_roles
shayancanonical May 7, 2025
fead5cd
Port Carl's fix for broken unit tests
shayancanonical May 7, 2025
2c0f822
Merge branch '16/edge' into feature/16_predefined_instance_roles
shayancanonical May 7, 2025
c848782
Merge branch '16/edge' into feature/16_predefined_instance_roles
shayancanonical May 28, 2025
e49e999
Create set_up_predefined_catalog_roles_function
marceloneppel May 29, 2025
5724320
Fix linting and run function on database creation
marceloneppel May 29, 2025
8da9ab5
Add login hook function
marceloneppel May 29, 2025
e4bc8d2
Escalate relation users
marceloneppel May 29, 2025
8094e4f
Merge branch '16/edge' into feature/16_predefined_instance_roles
shayancanonical May 30, 2025
2f204a9
Add integration test
marceloneppel May 30, 2025
0dd22ed
Fix unit test
marceloneppel May 30, 2025
d453455
Merge remote-tracking branch 'origin/feature/16_predefined_instance_r…
marceloneppel May 30, 2025
2fc6e5c
Check for no write permissions for relation user
marceloneppel May 30, 2025
fea0148
Don't set up catalog roles if they already exist
marceloneppel May 30, 2025
1c5a911
Test database creation permission
marceloneppel May 30, 2025
6a083c5
Improve logs and move cleanup process to the beginning of the test
marceloneppel Jun 2, 2025
16d6783
Wait for relation to be removed and retrieve primary
marceloneppel Jun 2, 2025
fe5ce8f
Handle re-relation
marceloneppel Jun 2, 2025
43493d4
Add test for removing and re-adding relation
marceloneppel Jun 2, 2025
1005f7e
Test roles after database re-creation
marceloneppel Jun 2, 2025
c526616
Test table creation failure for charmed_databases_owner user
marceloneppel Jun 2, 2025
5de77d0
Deduplicate relations retrieval code
marceloneppel Jun 2, 2025
ff5dd37
Check that the relation user can escalate to the database owner user …
marceloneppel Jun 2, 2025
1d170ff
Check escalation back to charmed_databases_owner
marceloneppel Jun 3, 2025
44beefa
Test permissions on newly created database
marceloneppel Jun 3, 2025
883c4ba
Check database owner user permissions in the newly created database
marceloneppel Jun 3, 2025
4eb981b
Reduce duplicated code with check_connected_user helper function
marceloneppel Jun 3, 2025
8b85257
Reduce more duplicated code with check_connected_user helper function
marceloneppel Jun 3, 2025
e669da3
Bump library
marceloneppel Jun 3, 2025
17e7e4a
Fix test_charmed_read_role
marceloneppel Jun 3, 2025
f1d3293
Remove admin and postgres roles
marceloneppel Jun 3, 2025
3aba882
Create DBA role
marceloneppel Jun 4, 2025
e16567d
Bump postgresql charm lib for 16/edge to v1 due to backwards incompat…
shayancanonical Jun 4, 2025
2c15212
Remove admin role test
marceloneppel Jun 4, 2025
f711479
Add DBA user test
marceloneppel Jun 4, 2025
1fed145
Test DBA role in replica
marceloneppel Jun 4, 2025
c8efa82
Grant reset_user function to DBA role
marceloneppel Jun 4, 2025
6cd2a85
Test set_user function for unprivileged users
marceloneppel Jun 4, 2025
e42e220
Reduce duplicate code in check_connected_user helper function
marceloneppel Jun 4, 2025
fbebe06
Merge remote-tracking branch 'origin/feature/16_predefined_instance_r…
marceloneppel Jun 4, 2025
3ae3885
Merge remote-tracking branch 'origin/feature/16_predefined_dba_role' …
marceloneppel Jun 4, 2025
fb566b5
Merge remote-tracking branch 'origin/feature/16_predefined_roles_clea…
marceloneppel Jun 4, 2025
5f2328e
Fix charmed_databases_owner permissions
marceloneppel Jun 4, 2025
3deb576
Fix test_charmed_dba_role
marceloneppel Jun 4, 2025
c78e2ff
Merge branch '16/edge' into feature/16_predefined_instance_roles
shayancanonical Jun 5, 2025
526bf2d
Re-add mistakenly removed patch statements
shayancanonical Jun 5, 2025
fd41021
Merge remote-tracking branch 'origin/feature/16_predefined_instance_r…
marceloneppel Jun 5, 2025
fa1efa7
Merge remote-tracking branch 'origin/feature/16_predefined_dba_role' …
marceloneppel Jun 5, 2025
4084579
Merge remote-tracking branch 'origin/feature/16_predefined_roles_clea…
marceloneppel Jun 5, 2025
f3eceba
Merge remote-tracking branch 'origin/16/edge' into feature/16_predefi…
marceloneppel Jun 5, 2025
9721e91
Merge remote-tracking branch 'origin/feature/16_predefined_dba_role' …
marceloneppel Jun 5, 2025
7f98938
Merge remote-tracking branch 'origin/feature/16_predefined_roles_clea…
marceloneppel Jun 5, 2025
40c2051
Merge remote-tracking branch 'origin/16/edge' into feature/16_predefi…
marceloneppel Jun 6, 2025
d121c1f
Merge remote-tracking branch 'origin/feature/16_predefined_dba_role' …
marceloneppel Jun 6, 2025
71fb74f
Merge remote-tracking branch 'origin/feature/16_predefined_roles_clea…
marceloneppel Jun 6, 2025
dc5b01e
Reset connection to None before creating a new connection
marceloneppel Jun 6, 2025
262dc2b
Merge remote-tracking branch 'origin/feature/16_predefined_dba_role' …
marceloneppel Jun 6, 2025
0848dcf
Remove irrelevant test and increase timeout
marceloneppel Jun 6, 2025
32f4f56
Merge remote-tracking branch 'origin/feature/16_predefined_roles_clea…
marceloneppel Jun 6, 2025
24605d9
Merge remote-tracking branch 'origin/16/edge' into feature/16_predefi…
marceloneppel Jun 7, 2025
4f2f0e2
Merge remote-tracking branch 'origin/feature/16_predefined_roles_clea…
marceloneppel Jun 7, 2025
32663cb
Merge remote-tracking branch 'origin/16/edge' into feature/16_predefi…
marceloneppel Jun 8, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
275 changes: 172 additions & 103 deletions lib/charms/postgresql_k8s/v1/postgresql.py

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion src/charm.py
Original file line number Diff line number Diff line change
Expand Up @@ -1712,7 +1712,7 @@ def _start_primary(self, event: StartEvent) -> None: # noqa: C901
return

try:
self.postgresql.create_predefined_roles()
self.postgresql.create_predefined_instance_roles()
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just renaming to be clear on which types of predefined roles we are creating at this moment (i.e. the ones introduced by #881).

except PostgreSQLCreatePredefinedRolesError as e:
logger.exception(e)
self.unit.status = BlockedStatus("Failed to create pre-defined roles")
Expand Down
9 changes: 5 additions & 4 deletions src/relations/postgresql_provider.py
Original file line number Diff line number Diff line change
Expand Up @@ -110,12 +110,13 @@ def _on_database_requested(self, event: DatabaseRequestedEvent) -> None:
try:
# Creates the user and the database for this specific relation.
user = f"relation-{event.relation.id}"
password = new_password()
self.charm.postgresql.create_user(user, password, extra_user_roles=extra_user_roles)
plugins = self.charm.get_plugins()

self.charm.postgresql.create_database(
database, user, plugins=plugins, client_relations=self.charm.client_relations
self.charm.postgresql.create_database(database, plugins=plugins)

password = new_password()
self.charm.postgresql.create_user(
user, password, extra_user_roles=extra_user_roles, in_role=f"{database}_admin"
)
Comment on lines +115 to 120
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The order of the calls was changed here because the catalog/database level roles are created along with the database, and we need to pass the {database-name}_admin to the user creation method.


# Share the credentials with the application.
Expand Down
2 changes: 2 additions & 0 deletions templates/patroni.yml.j2
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@ bootstrap:
logging_collector: 'on'
wal_level: logical
shared_preload_libraries: 'timescaledb,pgaudit,set_user'
session_preload_libraries: 'login_hook'
set_user.block_log_statement: 'on'
set_user.exit_on_error: 'on'
set_user.superuser_allowlist: '+charmed_dba'
Expand Down Expand Up @@ -136,6 +137,7 @@ postgresql:
data_dir: {{ data_path }}
parameters:
shared_preload_libraries: 'timescaledb,pgaudit,set_user'
session_preload_libraries: 'login_hook'
set_user.block_log_statement: 'on'
set_user.exit_on_error: 'on'
set_user.superuser_allowlist: '+charmed_dba'
Expand Down
66 changes: 66 additions & 0 deletions tests/integration/helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@

import botocore
import psycopg2
import pytest
import requests
import yaml
from juju.model import Model
Expand Down Expand Up @@ -755,6 +756,63 @@ def check_connected_user(
assert False, "No result returned from the query"


async def check_roles_and_their_permissions(
ops_test: OpsTest, relation_endpoint: str, database_name: str
) -> None:
action = await ops_test.model.units[f"{DATA_INTEGRATOR_APP_NAME}/0"].run_action(
action_name="get-credentials"
)
result = await action.wait()
data_integrator_credentials = result.results
username = data_integrator_credentials[relation_endpoint]["username"]
uris = data_integrator_credentials[relation_endpoint]["uris"]
connection = None
try:
connection = psycopg2.connect(uris)
connection.autocommit = True
with connection.cursor() as cursor:
logger.info(
"Checking that the relation user is automatically escalated to the database owner user"
)
check_connected_user(cursor, username, f"{database_name}_owner")
logger.info("Creating a test table and inserting data")
cursor.execute("CREATE TABLE test_table (id INTEGER);")
logger.info("Inserting data into the test table")
cursor.execute("INSERT INTO test_table(id) VALUES(1);")
logger.info("Reading data from the test table")
cursor.execute("SELECT * FROM test_table;")
result = cursor.fetchall()
assert len(result) == 1, "The database owner user should be able to read the data"

logger.info("Checking that the database owner user can't create a database")
with pytest.raises(psycopg2.errors.InsufficientPrivilege):
cursor.execute(f"CREATE DATABASE {database_name}_2;")

logger.info("Checking that the relation user can't create a table")
cursor.execute("RESET ROLE;")
check_connected_user(cursor, username, username)
with pytest.raises(psycopg2.errors.InsufficientPrivilege):
cursor.execute("CREATE TABLE test_table_2 (id INTEGER);")
finally:
if connection is not None:
connection.close()

connection_string = f"host={data_integrator_credentials[relation_endpoint]['read-only-endpoints'].split(':')[0]} dbname={data_integrator_credentials[relation_endpoint]['database']} user={username} password={data_integrator_credentials[relation_endpoint]['password']}"
connection = None
try:
connection = psycopg2.connect(connection_string)
with connection.cursor() as cursor:
logger.info("Checking that the relation user can read data from the database")
check_connected_user(cursor, username, username, primary=False)
logger.info("Reading data from the test table")
cursor.execute("SELECT * FROM test_table;")
result = cursor.fetchall()
assert len(result) == 1, "The relation user should be able to read the data"
finally:
if connection is not None:
connection.close()


async def check_tls(ops_test: OpsTest, unit_name: str, enabled: bool) -> bool:
"""Returns whether TLS is enabled on the specific PostgreSQL instance.

Expand Down Expand Up @@ -918,6 +976,14 @@ async def primary_changed(ops_test: OpsTest, old_primary: str) -> bool:
return primary != old_primary


def relations(ops_test: OpsTest, provider_app: str, requirer_app: str) -> list:
return [
relation
for relation in ops_test.model.applications[provider_app].relations
if not relation.is_peer and relation.requires.application_name == requirer_app
]
Comment on lines +979 to +984
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Way to check if the relation still exists (and avoid situations where we removed it and it's still alive when trying to re-relate).



async def restart_machine(ops_test: OpsTest, unit_name: str) -> None:
"""Restart the machine where a unit run on.

Expand Down
22 changes: 1 addition & 21 deletions tests/integration/new_relations/test_new_relations_1.py
Original file line number Diff line number Diff line change
Expand Up @@ -225,26 +225,6 @@ async def test_filter_out_degraded_replicas(ops_test: OpsTest):
)


async def test_user_with_extra_roles(ops_test: OpsTest):
"""Test superuser actions and the request for more permissions."""
# Get the connection string to connect to the database.
connection_string = await build_connection_string(
ops_test, APPLICATION_APP_NAME, FIRST_DATABASE_RELATION_NAME
)

# Connect to the database.
connection = psycopg2.connect(connection_string)
connection.autocommit = True
cursor = connection.cursor()

# Test the user can create a database and another user.
cursor.execute("CREATE DATABASE another_database;")
cursor.execute("CREATE USER another_user WITH ENCRYPTED PASSWORD 'test-password';")

cursor.close()
connection.close()


async def test_two_applications_doesnt_share_the_same_relation_data(ops_test: OpsTest):
"""Test that two different application connect to the database with different credentials."""
# Set some variables to use in this test.
Expand Down Expand Up @@ -394,7 +374,7 @@ async def test_relation_data_is_updated_correctly_when_scaling(ops_test: OpsTest
# Add two more units.
await ops_test.model.applications[DATABASE_APP_NAME].add_units(2)
await ops_test.model.wait_for_idle(
apps=[DATABASE_APP_NAME], status="active", timeout=1500, wait_for_exact_units=4
apps=[DATABASE_APP_NAME], status="active", timeout=3000, wait_for_exact_units=4
)

assert_sync_standbys(
Expand Down
Loading
Loading