Skip to content

Commit 577307e

Browse files
[DPE-7500] Create catalog/database level roles (#921)
* 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 * Create set_up_predefined_catalog_roles_function Signed-off-by: Marcelo Henrique Neppel <[email protected]> * Fix linting and run function on database creation Signed-off-by: Marcelo Henrique Neppel <[email protected]> * Add login hook function Signed-off-by: Marcelo Henrique Neppel <[email protected]> * Escalate relation users Signed-off-by: Marcelo Henrique Neppel <[email protected]> * Add integration test Signed-off-by: Marcelo Henrique Neppel <[email protected]> * Fix unit test Signed-off-by: Marcelo Henrique Neppel <[email protected]> * Check for no write permissions for relation user Signed-off-by: Marcelo Henrique Neppel <[email protected]> * Don't set up catalog roles if they already exist Signed-off-by: Marcelo Henrique Neppel <[email protected]> * Test database creation permission Signed-off-by: Marcelo Henrique Neppel <[email protected]> * Improve logs and move cleanup process to the beginning of the test Signed-off-by: Marcelo Henrique Neppel <[email protected]> * Wait for relation to be removed and retrieve primary Signed-off-by: Marcelo Henrique Neppel <[email protected]> * Handle re-relation Signed-off-by: Marcelo Henrique Neppel <[email protected]> * Add test for removing and re-adding relation Signed-off-by: Marcelo Henrique Neppel <[email protected]> * Test roles after database re-creation Signed-off-by: Marcelo Henrique Neppel <[email protected]> * Test table creation failure for charmed_databases_owner user Signed-off-by: Marcelo Henrique Neppel <[email protected]> * Deduplicate relations retrieval code Signed-off-by: Marcelo Henrique Neppel <[email protected]> * Check that the relation user can escalate to the database owner user and create a table Signed-off-by: Marcelo Henrique Neppel <[email protected]> * Check escalation back to charmed_databases_owner Signed-off-by: Marcelo Henrique Neppel <[email protected]> * Test permissions on newly created database Signed-off-by: Marcelo Henrique Neppel <[email protected]> * Check database owner user permissions in the newly created database Signed-off-by: Marcelo Henrique Neppel <[email protected]> * Reduce duplicated code with check_connected_user helper function Signed-off-by: Marcelo Henrique Neppel <[email protected]> * Reduce more duplicated code with check_connected_user helper function Signed-off-by: Marcelo Henrique Neppel <[email protected]> * Bump library Signed-off-by: Marcelo Henrique Neppel <[email protected]> * Fix test_charmed_read_role Signed-off-by: Marcelo Henrique Neppel <[email protected]> * Remove admin and postgres roles Signed-off-by: Marcelo Henrique Neppel <[email protected]> * Create DBA role Signed-off-by: Marcelo Henrique Neppel <[email protected]> * Bump postgresql charm lib for 16/edge to v1 due to backwards incompatible changes * Remove admin role test Signed-off-by: Marcelo Henrique Neppel <[email protected]> * Add DBA user test Signed-off-by: Marcelo Henrique Neppel <[email protected]> * Test DBA role in replica Signed-off-by: Marcelo Henrique Neppel <[email protected]> * Grant reset_user function to DBA role Signed-off-by: Marcelo Henrique Neppel <[email protected]> * Test set_user function for unprivileged users Signed-off-by: Marcelo Henrique Neppel <[email protected]> * Reduce duplicate code in check_connected_user helper function Signed-off-by: Marcelo Henrique Neppel <[email protected]> * Fix charmed_databases_owner permissions Signed-off-by: Marcelo Henrique Neppel <[email protected]> * Fix test_charmed_dba_role Signed-off-by: Marcelo Henrique Neppel <[email protected]> * Re-add mistakenly removed patch statements * Reset connection to None before creating a new connection Signed-off-by: Marcelo Henrique Neppel <[email protected]> * Remove irrelevant test and increase timeout Signed-off-by: Marcelo Henrique Neppel <[email protected]> --------- Signed-off-by: Marcelo Henrique Neppel <[email protected]> Co-authored-by: Shayan Patel <[email protected]>
1 parent 59e6ff2 commit 577307e

File tree

11 files changed

+522
-135
lines changed

11 files changed

+522
-135
lines changed

lib/charms/postgresql_k8s/v1/postgresql.py

Lines changed: 172 additions & 103 deletions
Large diffs are not rendered by default.

src/charm.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1712,7 +1712,7 @@ def _start_primary(self, event: StartEvent) -> None: # noqa: C901
17121712
return
17131713

17141714
try:
1715-
self.postgresql.create_predefined_roles()
1715+
self.postgresql.create_predefined_instance_roles()
17161716
except PostgreSQLCreatePredefinedRolesError as e:
17171717
logger.exception(e)
17181718
self.unit.status = BlockedStatus("Failed to create pre-defined roles")

src/relations/postgresql_provider.py

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -110,12 +110,13 @@ def _on_database_requested(self, event: DatabaseRequestedEvent) -> None:
110110
try:
111111
# Creates the user and the database for this specific relation.
112112
user = f"relation-{event.relation.id}"
113-
password = new_password()
114-
self.charm.postgresql.create_user(user, password, extra_user_roles=extra_user_roles)
115113
plugins = self.charm.get_plugins()
116114

117-
self.charm.postgresql.create_database(
118-
database, user, plugins=plugins, client_relations=self.charm.client_relations
115+
self.charm.postgresql.create_database(database, plugins=plugins)
116+
117+
password = new_password()
118+
self.charm.postgresql.create_user(
119+
user, password, extra_user_roles=extra_user_roles, in_role=f"{database}_admin"
119120
)
120121

121122
# Share the credentials with the application.

templates/patroni.yml.j2

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,7 @@ bootstrap:
9797
logging_collector: 'on'
9898
wal_level: logical
9999
shared_preload_libraries: 'timescaledb,pgaudit,set_user'
100+
session_preload_libraries: 'login_hook'
100101
set_user.block_log_statement: 'on'
101102
set_user.exit_on_error: 'on'
102103
set_user.superuser_allowlist: '+charmed_dba'
@@ -136,6 +137,7 @@ postgresql:
136137
data_dir: {{ data_path }}
137138
parameters:
138139
shared_preload_libraries: 'timescaledb,pgaudit,set_user'
140+
session_preload_libraries: 'login_hook'
139141
set_user.block_log_statement: 'on'
140142
set_user.exit_on_error: 'on'
141143
set_user.superuser_allowlist: '+charmed_dba'

tests/integration/helpers.py

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414

1515
import botocore
1616
import psycopg2
17+
import pytest
1718
import requests
1819
import yaml
1920
from juju.model import Model
@@ -755,6 +756,63 @@ def check_connected_user(
755756
assert False, "No result returned from the query"
756757

757758

759+
async def check_roles_and_their_permissions(
760+
ops_test: OpsTest, relation_endpoint: str, database_name: str
761+
) -> None:
762+
action = await ops_test.model.units[f"{DATA_INTEGRATOR_APP_NAME}/0"].run_action(
763+
action_name="get-credentials"
764+
)
765+
result = await action.wait()
766+
data_integrator_credentials = result.results
767+
username = data_integrator_credentials[relation_endpoint]["username"]
768+
uris = data_integrator_credentials[relation_endpoint]["uris"]
769+
connection = None
770+
try:
771+
connection = psycopg2.connect(uris)
772+
connection.autocommit = True
773+
with connection.cursor() as cursor:
774+
logger.info(
775+
"Checking that the relation user is automatically escalated to the database owner user"
776+
)
777+
check_connected_user(cursor, username, f"{database_name}_owner")
778+
logger.info("Creating a test table and inserting data")
779+
cursor.execute("CREATE TABLE test_table (id INTEGER);")
780+
logger.info("Inserting data into the test table")
781+
cursor.execute("INSERT INTO test_table(id) VALUES(1);")
782+
logger.info("Reading data from the test table")
783+
cursor.execute("SELECT * FROM test_table;")
784+
result = cursor.fetchall()
785+
assert len(result) == 1, "The database owner user should be able to read the data"
786+
787+
logger.info("Checking that the database owner user can't create a database")
788+
with pytest.raises(psycopg2.errors.InsufficientPrivilege):
789+
cursor.execute(f"CREATE DATABASE {database_name}_2;")
790+
791+
logger.info("Checking that the relation user can't create a table")
792+
cursor.execute("RESET ROLE;")
793+
check_connected_user(cursor, username, username)
794+
with pytest.raises(psycopg2.errors.InsufficientPrivilege):
795+
cursor.execute("CREATE TABLE test_table_2 (id INTEGER);")
796+
finally:
797+
if connection is not None:
798+
connection.close()
799+
800+
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']}"
801+
connection = None
802+
try:
803+
connection = psycopg2.connect(connection_string)
804+
with connection.cursor() as cursor:
805+
logger.info("Checking that the relation user can read data from the database")
806+
check_connected_user(cursor, username, username, primary=False)
807+
logger.info("Reading data from the test table")
808+
cursor.execute("SELECT * FROM test_table;")
809+
result = cursor.fetchall()
810+
assert len(result) == 1, "The relation user should be able to read the data"
811+
finally:
812+
if connection is not None:
813+
connection.close()
814+
815+
758816
async def check_tls(ops_test: OpsTest, unit_name: str, enabled: bool) -> bool:
759817
"""Returns whether TLS is enabled on the specific PostgreSQL instance.
760818
@@ -918,6 +976,14 @@ async def primary_changed(ops_test: OpsTest, old_primary: str) -> bool:
918976
return primary != old_primary
919977

920978

979+
def relations(ops_test: OpsTest, provider_app: str, requirer_app: str) -> list:
980+
return [
981+
relation
982+
for relation in ops_test.model.applications[provider_app].relations
983+
if not relation.is_peer and relation.requires.application_name == requirer_app
984+
]
985+
986+
921987
async def restart_machine(ops_test: OpsTest, unit_name: str) -> None:
922988
"""Restart the machine where a unit run on.
923989

tests/integration/new_relations/test_new_relations_1.py

Lines changed: 1 addition & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -225,26 +225,6 @@ async def test_filter_out_degraded_replicas(ops_test: OpsTest):
225225
)
226226

227227

228-
async def test_user_with_extra_roles(ops_test: OpsTest):
229-
"""Test superuser actions and the request for more permissions."""
230-
# Get the connection string to connect to the database.
231-
connection_string = await build_connection_string(
232-
ops_test, APPLICATION_APP_NAME, FIRST_DATABASE_RELATION_NAME
233-
)
234-
235-
# Connect to the database.
236-
connection = psycopg2.connect(connection_string)
237-
connection.autocommit = True
238-
cursor = connection.cursor()
239-
240-
# Test the user can create a database and another user.
241-
cursor.execute("CREATE DATABASE another_database;")
242-
cursor.execute("CREATE USER another_user WITH ENCRYPTED PASSWORD 'test-password';")
243-
244-
cursor.close()
245-
connection.close()
246-
247-
248228
async def test_two_applications_doesnt_share_the_same_relation_data(ops_test: OpsTest):
249229
"""Test that two different application connect to the database with different credentials."""
250230
# Set some variables to use in this test.
@@ -394,7 +374,7 @@ async def test_relation_data_is_updated_correctly_when_scaling(ops_test: OpsTest
394374
# Add two more units.
395375
await ops_test.model.applications[DATABASE_APP_NAME].add_units(2)
396376
await ops_test.model.wait_for_idle(
397-
apps=[DATABASE_APP_NAME], status="active", timeout=1500, wait_for_exact_units=4
377+
apps=[DATABASE_APP_NAME], status="active", timeout=3000, wait_for_exact_units=4
398378
)
399379

400380
assert_sync_standbys(

0 commit comments

Comments
 (0)