From 4a0399e62f2c438e823047c2b490017830f95891 Mon Sep 17 00:00:00 2001 From: Marcelo Henrique Neppel Date: Tue, 16 Aug 2022 11:49:41 -0300 Subject: [PATCH 01/18] Change charm database user --- actions.yaml | 5 +-- lib/charms/postgresql_k8s/v0/postgresql.py | 2 +- src/charm.py | 31 ++++++++++--------- src/cluster.py | 5 ++- src/constants.py | 1 + templates/patroni.yml.j2 | 2 +- tests/integration/helpers.py | 16 +++++----- tests/integration/test_charm.py | 8 ++--- tests/unit/test_charm.py | 36 +++++++++++----------- 9 files changed, 56 insertions(+), 50 deletions(-) diff --git a/actions.yaml b/actions.yaml index 059e17b2ba..f465bd1a91 100644 --- a/actions.yaml +++ b/actions.yaml @@ -3,5 +3,6 @@ get-primary: description: Get the unit which is the primary/leader in the replication. -get-initial-password: - description: Get the initial postgres user password for the database. +get-operator-password: + description: Get the operator user password used by charm. + It is internal charm user, SHOULD NOT be used by applications. diff --git a/lib/charms/postgresql_k8s/v0/postgresql.py b/lib/charms/postgresql_k8s/v0/postgresql.py index d96edfb960..f901204051 100644 --- a/lib/charms/postgresql_k8s/v0/postgresql.py +++ b/lib/charms/postgresql_k8s/v0/postgresql.py @@ -155,7 +155,7 @@ def delete_user(self, user: str) -> None: with self._connect_to_database( database ) as connection, connection.cursor() as cursor: - cursor.execute(sql.SQL("REASSIGN OWNED BY {} TO postgres;").format(sql.Identifier(user))) + cursor.execute(sql.SQL("REASSIGN OWNED BY {} TO operator;").format(sql.Identifier(user))) cursor.execute(sql.SQL("DROP OWNED BY {};").format(sql.Identifier(user))) # Delete the user. diff --git a/src/charm.py b/src/charm.py index 076ac5f177..d171dfa050 100755 --- a/src/charm.py +++ b/src/charm.py @@ -36,7 +36,7 @@ RemoveRaftMemberFailedError, SwitchoverFailedError, ) -from constants import PEER +from constants import PEER, USER from relations.postgresql_provider import PostgreSQLProvider from utils import new_password @@ -60,7 +60,9 @@ def __init__(self, *args): self.framework.observe(self.on[PEER].relation_departed, self._on_peer_relation_departed) self.framework.observe(self.on.pgdata_storage_detaching, self._on_pgdata_storage_detaching) self.framework.observe(self.on.start, self._on_start) - self.framework.observe(self.on.get_initial_password_action, self._on_get_initial_password) + self.framework.observe( + self.on.get_operator_password_action, self._on_get_operator_password + ) self.framework.observe(self.on.update_status, self._on_update_status) self._cluster_name = self.app.name self._member_name = self.unit.name.replace("/", "-") @@ -73,8 +75,8 @@ def postgresql(self) -> PostgreSQL: """Returns an instance of the object used to interact with the database.""" return PostgreSQL( host=self.primary_endpoint, - user="postgres", - password=self._get_postgres_password(), + user=USER, + password=self._get_operator_password(), database="postgres", ) @@ -324,7 +326,7 @@ def _patroni(self) -> Patroni: self._member_name, self.app.planned_units(), self._peer_members_ips, - self._get_postgres_password(), + self._get_operator_password(), self._replication_password, ) @@ -459,7 +461,7 @@ def _on_leader_elected(self, event: LeaderElectedEvent) -> None: """Handle the leader-elected event.""" data = self._peers.data[self.app] # The leader sets the needed password on peer relation databag if they weren't set before. - data.setdefault("postgres-password", new_password()) + data.setdefault("operator-password", new_password()) data.setdefault("replication-password", new_password()) # Update the list of the current PostgreSQL hosts when a new leader is elected. @@ -500,11 +502,10 @@ def _on_start(self, event) -> None: if self._has_blocked_status: return - postgres_password = self._get_postgres_password() - replication_password = self._get_postgres_password() + postgres_password = self._get_operator_password() # If the leader was not elected (and the needed passwords were not generated yet), # the cluster cannot be bootstrapped yet. - if not postgres_password or not replication_password: + if not postgres_password or not self._replication_password: logger.info("leader not elected and/or passwords not yet generated") self.unit.status = WaitingStatus("awaiting passwords generation") event.defer() @@ -538,9 +539,9 @@ def _on_start(self, event) -> None: self._peers.data[self.app]["cluster_initialised"] = "True" self.unit.status = ActiveStatus() - def _on_get_initial_password(self, event: ActionEvent) -> None: - """Returns the password for the postgres user as an action response.""" - event.set_results({"postgres-password": self._get_postgres_password()}) + def _on_get_operator_password(self, event: ActionEvent) -> None: + """Returns the password for the operator user as an action response.""" + event.set_results({"operator-password": self._get_operator_password()}) def _on_update_status(self, _) -> None: """Update endpoints of the postgres client relation and update users list.""" @@ -552,15 +553,15 @@ def _has_blocked_status(self) -> bool: """Returns whether the unit is in a blocked state.""" return isinstance(self.unit.status, BlockedStatus) - def _get_postgres_password(self) -> str: - """Get postgres user password. + def _get_operator_password(self) -> str: + """Get operator user password. Returns: The password from the peer relation or None if the password has not yet been set by the leader. """ data = self._peers.data[self.app] - return data.get("postgres-password") + return data.get("operator-password") @property def _replication_password(self) -> str: diff --git a/src/cluster.py b/src/cluster.py index 272de20e9e..03ece3d92f 100644 --- a/src/cluster.py +++ b/src/cluster.py @@ -28,6 +28,8 @@ wait_fixed, ) +from constants import USER + logger = logging.getLogger(__name__) PATRONI_SERVICE = "patroni" @@ -70,7 +72,7 @@ def __init__( member_name: name of the member inside the cluster peers_ips: IP addresses of the peer units planned_units: number of units planned for the cluster - superuser_password: password for the postgres user + superuser_password: password for the operator user replication_password: password for the user used in the replication """ self.unit_ip = unit_ip @@ -259,6 +261,7 @@ def _render_patroni_yml_file(self, replica: bool = False) -> None: scope=self.cluster_name, self_ip=self.unit_ip, replica=replica, + superuser=USER, superuser_password=self.superuser_password, replication_password=self.replication_password, version=self._get_postgresql_version(), diff --git a/src/constants.py b/src/constants.py index 1ce25738e5..a3427b8c2d 100644 --- a/src/constants.py +++ b/src/constants.py @@ -7,3 +7,4 @@ DATABASE_PORT = "5432" PEER = "database-peers" ALL_CLIENT_RELATIONS = [DATABASE] +USER = "operator" diff --git a/templates/patroni.yml.j2 b/templates/patroni.yml.j2 index 375f7932d8..0ced378216 100644 --- a/templates/patroni.yml.j2 +++ b/templates/patroni.yml.j2 @@ -62,5 +62,5 @@ postgresql: username: replication password: {{ replication_password }} superuser: - username: postgres + username: {{ superuser }} password: {{ superuser_password }} diff --git a/tests/integration/helpers.py b/tests/integration/helpers.py index 7dc731b299..8d84503160 100644 --- a/tests/integration/helpers.py +++ b/tests/integration/helpers.py @@ -57,13 +57,13 @@ def db_connect(host: str, password: str) -> psycopg2.extensions.connection: Args: host: the IP of the postgres host - password: postgres password + password: operator user password Returns: - psycopg2 connection object linked to postgres db, under "postgres" user. + psycopg2 connection object linked to postgres db, under "operator" user. """ return psycopg2.connect( - f"dbname='postgres' user='postgres' host='{host}' password='{password}' connect_timeout=10" + f"dbname='postgres' user='operator' host='{host}' password='{password}' connect_timeout=10" ) @@ -114,20 +114,20 @@ def get_application_units_ips(ops_test: OpsTest, application_name: str) -> List[ return [unit.public_address for unit in ops_test.model.applications[application_name].units] -async def get_postgres_password(ops_test: OpsTest, unit_name: str) -> str: - """Retrieve the postgres user password using the action. +async def get_operator_password(ops_test: OpsTest, unit_name: str) -> str: + """Retrieve the operator user password using the action. Args: ops_test: ops_test instance. unit_name: the name of the unit. Returns: - the postgres user password. + the operator user password. """ unit = ops_test.model.units.get(unit_name) - action = await unit.run_action("get-initial-password") + action = await unit.run_action("get-operator-password") result = await action.wait() - return result.results["postgres-password"] + return result.results["operator-password"] async def get_primary(ops_test: OpsTest, unit_name: str) -> str: diff --git a/tests/integration/test_charm.py b/tests/integration/test_charm.py index 9a7d1620db..9d32a78044 100644 --- a/tests/integration/test_charm.py +++ b/tests/integration/test_charm.py @@ -19,7 +19,7 @@ convert_records_to_dict, db_connect, find_unit, - get_postgres_password, + get_operator_password, get_primary, get_unit_address, scale_application, @@ -85,12 +85,12 @@ async def test_settings_are_correct(ops_test: OpsTest, series: str, unit_id: int # Set a composite application name in order to test in more than one series at the same time. application_name = build_application_name(series) - # Retrieving the postgres user password using the action. + # Retrieving the operator user password using the action. action = await ops_test.model.units.get(f"{application_name}/{unit_id}").run_action( "get-initial-password" ) action = await action.wait() - password = action.results["postgres-password"] + password = action.results["operator-password"] # Connect to PostgreSQL. host = get_unit_address(ops_test, f"{application_name}/{unit_id}") @@ -222,7 +222,7 @@ async def test_persist_data_through_primary_deletion(ops_test: OpsTest, series: application_name = build_application_name(series) any_unit_name = ops_test.model.applications[application_name].units[0].name primary = await get_primary(ops_test, any_unit_name) - password = await get_postgres_password(ops_test, primary) + password = await get_operator_password(ops_test, primary) # Write data to primary IP. host = get_unit_address(ops_test, primary) diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py index 2e5fea00d2..5e8b8a700e 100644 --- a/tests/unit/test_charm.py +++ b/tests/unit/test_charm.py @@ -118,12 +118,12 @@ def test_on_leader_elected( self, _update_cluster_members, _primary_endpoint, _update_endpoints ): # Assert that there is no password in the peer relation. - self.assertIsNone(self.charm._peers.data[self.charm.app].get("postgres-password", None)) + self.assertIsNone(self.charm._peers.data[self.charm.app].get("operator-password", None)) # Check that a new password was generated on leader election. _primary_endpoint.return_value = "1.1.1.1" self.harness.set_leader() - password = self.charm._peers.data[self.charm.app].get("postgres-password", None) + password = self.charm._peers.data[self.charm.app].get("operator-password", None) _update_cluster_members.assert_called_once() _update_endpoints.assert_not_called() self.assertIsNotNone(password) @@ -136,7 +136,7 @@ def test_on_leader_elected( self.harness.set_leader(False) self.harness.set_leader() self.assertEqual( - self.charm._peers.data[self.charm.app].get("postgres-password", None), password + self.charm._peers.data[self.charm.app].get("operator-password", None), password ) _update_endpoints.assert_called_once() self.assertFalse(isinstance(self.harness.model.unit.status, BlockedStatus)) @@ -154,10 +154,10 @@ def test_on_leader_elected( @patch("charm.Patroni.member_started") @patch("charm.Patroni.bootstrap_cluster") @patch("charm.PostgresqlOperatorCharm._replication_password") - @patch("charm.PostgresqlOperatorCharm._get_postgres_password") + @patch("charm.PostgresqlOperatorCharm._get_operator_password") def test_on_start( self, - _get_postgres_password, + _get_operator_password, _replication_password, _bootstrap_cluster, _member_started, @@ -165,13 +165,13 @@ def test_on_start( __, ): # Test before the passwords are generated. - _get_postgres_password.return_value = None + _get_operator_password.return_value = None self.charm.on.start.emit() _bootstrap_cluster.assert_not_called() self.assertTrue(isinstance(self.harness.model.unit.status, WaitingStatus)) # Mock the passwords. - _get_postgres_password.return_value = "fake-postgres-password" + _get_operator_password.return_value = "fake-operator-password" _replication_password.return_value = "fake-replication-password" # Mock cluster start success values. @@ -193,9 +193,9 @@ def test_on_start( @patch("charm.Patroni.bootstrap_cluster") @patch("charm.PostgresqlOperatorCharm._replication_password") - @patch("charm.PostgresqlOperatorCharm._get_postgres_password") + @patch("charm.PostgresqlOperatorCharm._get_operator_password") def test_on_start_after_blocked_state( - self, _get_postgres_password, _replication_password, _bootstrap_cluster + self, _get_operator_password, _replication_password, _bootstrap_cluster ): # Set an initial blocked status (like after the install hook was triggered). initial_status = BlockedStatus("fake message") @@ -203,30 +203,30 @@ def test_on_start_after_blocked_state( # Test for a failed cluster bootstrapping. self.charm.on.start.emit() - _get_postgres_password.assert_not_called() + _get_operator_password.assert_not_called() _replication_password.assert_not_called() _bootstrap_cluster.assert_not_called() # Assert the status didn't change. self.assertEqual(self.harness.model.unit.status, initial_status) - @patch("charm.PostgresqlOperatorCharm._get_postgres_password") - def test_on_get_postgres_password(self, _get_postgres_password): + @patch("charm.PostgresqlOperatorCharm._get_operator_password") + def test_on_get_operator_password(self, _get_operator_password): mock_event = Mock() - _get_postgres_password.return_value = "test-password" + _get_operator_password.return_value = "test-password" self.charm._on_get_initial_password(mock_event) - _get_postgres_password.assert_called_once() - mock_event.set_results.assert_called_once_with({"postgres-password": "test-password"}) + _get_operator_password.assert_called_once() + mock_event.set_results.assert_called_once_with({"operator-password": "test-password"}) @patch("charm.PostgreSQLProvider.update_endpoints") @patch("charm.Patroni.update_cluster_members") @patch_network_get(private_address="1.1.1.1") - def test_get_postgres_password(self, _, __): + def test_get_operator_password(self, _, __): # Test for a None password. - self.assertIsNone(self.charm._get_postgres_password()) + self.assertIsNone(self.charm._get_operator_password()) # Then test for a non empty password after leader election and peer data set. self.harness.set_leader() - password = self.charm._get_postgres_password() + password = self.charm._get_operator_password() self.assertIsNotNone(password) self.assertNotEqual(password, "") From 37c06399e1a2ce55e7fa52687053c8b04cb756fc Mon Sep 17 00:00:00 2001 From: Marcelo Henrique Neppel Date: Tue, 16 Aug 2022 13:17:17 -0300 Subject: [PATCH 02/18] Fix unit tests --- tests/unit/test_charm.py | 2 +- tests/unit/test_cluster.py | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py index 5e8b8a700e..2f873dcd7c 100644 --- a/tests/unit/test_charm.py +++ b/tests/unit/test_charm.py @@ -213,7 +213,7 @@ def test_on_start_after_blocked_state( def test_on_get_operator_password(self, _get_operator_password): mock_event = Mock() _get_operator_password.return_value = "test-password" - self.charm._on_get_initial_password(mock_event) + self.charm._on_get_operator_password(mock_event) _get_operator_password.assert_called_once() mock_event.set_results.assert_called_once_with({"operator-password": "test-password"}) diff --git a/tests/unit/test_cluster.py b/tests/unit/test_cluster.py index 566eb607f8..2f5009d572 100644 --- a/tests/unit/test_cluster.py +++ b/tests/unit/test_cluster.py @@ -110,6 +110,7 @@ def test_render_patroni_yml_file(self, _, _render_file): peers_ips=self.peers_ips, scope=scope, self_ip=self.patroni.unit_ip, + superuser="operator", superuser_password=superuser_password, replication_password=replication_password, version=self.patroni._get_postgresql_version(), From d05d991b83aed9d7860ec19beed1671753f60279 Mon Sep 17 00:00:00 2001 From: Marcelo Henrique Neppel Date: Tue, 16 Aug 2022 15:07:37 -0300 Subject: [PATCH 03/18] Fix integration test call --- tests/integration/test_charm.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/integration/test_charm.py b/tests/integration/test_charm.py index 9d32a78044..e503097523 100644 --- a/tests/integration/test_charm.py +++ b/tests/integration/test_charm.py @@ -87,7 +87,7 @@ async def test_settings_are_correct(ops_test: OpsTest, series: str, unit_id: int # Retrieving the operator user password using the action. action = await ops_test.model.units.get(f"{application_name}/{unit_id}").run_action( - "get-initial-password" + "get-operator-password" ) action = await action.wait() password = action.results["operator-password"] From aa61f60cd017cdb28e4238493747beb754227581 Mon Sep 17 00:00:00 2001 From: Marcelo Henrique Neppel Date: Wed, 17 Aug 2022 14:30:14 -0300 Subject: [PATCH 04/18] Fix user name in library --- lib/charms/postgresql_k8s/v0/postgresql.py | 4 +++- tests/integration/helpers.py | 4 ++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/lib/charms/postgresql_k8s/v0/postgresql.py b/lib/charms/postgresql_k8s/v0/postgresql.py index f901204051..524d5f8f63 100644 --- a/lib/charms/postgresql_k8s/v0/postgresql.py +++ b/lib/charms/postgresql_k8s/v0/postgresql.py @@ -155,7 +155,9 @@ def delete_user(self, user: str) -> None: with self._connect_to_database( database ) as connection, connection.cursor() as cursor: - cursor.execute(sql.SQL("REASSIGN OWNED BY {} TO operator;").format(sql.Identifier(user))) + cursor.execute(sql.SQL("REASSIGN OWNED BY {} TO {};").format( + sql.Identifier(user), sql.Identifier(self.user) + )) cursor.execute(sql.SQL("DROP OWNED BY {};").format(sql.Identifier(user))) # Delete the user. diff --git a/tests/integration/helpers.py b/tests/integration/helpers.py index 9bafa94f88..fee9125cb5 100644 --- a/tests/integration/helpers.py +++ b/tests/integration/helpers.py @@ -68,7 +68,7 @@ async def check_database_users_existence( """ unit = ops_test.model.applications[DATABASE_APP_NAME].units[0] unit_address = await unit.get_public_address() - password = await get_postgres_password(ops_test, unit.name) + password = await get_operator_password(ops_test, unit.name) # Retrieve all users in the database. users_in_db = await execute_query_on_unit( @@ -94,7 +94,7 @@ async def check_databases_creation(ops_test: OpsTest, databases: List[str]) -> N databases: List of database names that should have been created """ unit = ops_test.model.applications[DATABASE_APP_NAME].units[0] - password = await get_postgres_password(ops_test, unit.name) + password = await get_operator_password(ops_test, unit.name) for unit in ops_test.model.applications[DATABASE_APP_NAME].units: unit_address = await unit.get_public_address() From 991de5f19792a06d02730e3dfcbebc8ea5ce0281 Mon Sep 17 00:00:00 2001 From: Marcelo Henrique Neppel Date: Wed, 17 Aug 2022 16:09:51 -0300 Subject: [PATCH 05/18] Fix user --- tests/integration/helpers.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/integration/helpers.py b/tests/integration/helpers.py index fee9125cb5..baf6a42537 100644 --- a/tests/integration/helpers.py +++ b/tests/integration/helpers.py @@ -290,7 +290,7 @@ async def execute_query_on_unit( A list of rows that were potentially returned from the query. """ with psycopg2.connect( - f"dbname='{database}' user='postgres' host='{unit_address}' password='{password}' connect_timeout=10" + f"dbname='{database}' user='operator' host='{unit_address}' password='{password}' connect_timeout=10" ) as connection, connection.cursor() as cursor: cursor.execute(query) output = list(itertools.chain(*cursor.fetchall())) From db18ba634d6febeab74f88c3acb338d55bb3501a Mon Sep 17 00:00:00 2001 From: Marcelo Henrique Neppel Date: Thu, 25 Aug 2022 09:35:35 -0300 Subject: [PATCH 06/18] Add default postgres user creation --- src/charm.py | 11 ++++++++++- tests/unit/test_charm.py | 20 ++++++++++++++++++-- 2 files changed, 28 insertions(+), 3 deletions(-) diff --git a/src/charm.py b/src/charm.py index b7883d9086..d24ce1e570 100755 --- a/src/charm.py +++ b/src/charm.py @@ -10,7 +10,7 @@ from typing import List, Optional, Set from charms.operator_libs_linux.v0 import apt -from charms.postgresql_k8s.v0.postgresql import PostgreSQL +from charms.postgresql_k8s.v0.postgresql import PostgreSQL, PostgreSQLCreateUserError from ops.charm import ( ActionEvent, CharmBase, @@ -555,6 +555,15 @@ def _on_start(self, event) -> None: event.defer() return + # Create the default postgres database user that is needed for some + # applications (not charms) like Landscape Server. + try: + self.postgresql.create_user("postgres", new_password(), admin=True) + except PostgreSQLCreateUserError as e: + logger.exception(e) + self.unit.status = BlockedStatus("Failed to create postgres user") + return + self.postgresql_client_relation.oversee_users() # Set the flag to enable the replicas to start the Patroni service. diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py index e7aee50e5d..ea7b323e1c 100644 --- a/tests/unit/test_charm.py +++ b/tests/unit/test_charm.py @@ -6,6 +6,7 @@ from unittest.mock import Mock, PropertyMock, call, mock_open, patch from charms.operator_libs_linux.v0 import apt +from charms.postgresql_k8s.v0.postgresql import PostgreSQLCreateUserError from ops.model import ActiveStatus, BlockedStatus, WaitingStatus from ops.testing import Harness @@ -150,6 +151,7 @@ def test_on_leader_elected( @patch_network_get(private_address="1.1.1.1") @patch("charm.PostgreSQLProvider.oversee_users") + @patch("charm.PostgresqlOperatorCharm.postgresql") @patch("charm.PostgreSQLProvider.update_endpoints") @patch("charm.Patroni.update_cluster_members") @patch("charm.Patroni.member_started") @@ -164,6 +166,7 @@ def test_on_start( _member_started, _, __, + _postgresql, _oversee_users, ): # Test before the passwords are generated. @@ -176,8 +179,9 @@ def test_on_start( _get_operator_password.return_value = "fake-operator-password" _replication_password.return_value = "fake-replication-password" - # Mock cluster start success values. - _bootstrap_cluster.side_effect = [False, True] + # Mock cluster start and postgres user creation success values. + _bootstrap_cluster.side_effect = [False, True, True] + _postgresql.create_user.side_effect = [PostgreSQLCreateUserError, None] # Test for a failed cluster bootstrapping. # TODO: test replicas start (DPE-494). @@ -190,8 +194,20 @@ def test_on_start( # Set an initial waiting status (like after the install hook was triggered). self.harness.model.unit.status = WaitingStatus("fake message") + # Test the event of an error happening when trying to create the default postgres user. + self.charm.on.start.emit() + _postgresql.create_user.assert_called_once() + _oversee_users.assert_not_called() + self.assertTrue(isinstance(self.harness.model.unit.status, BlockedStatus)) + + # Set an initial waiting status again (like after the install hook was triggered). + self.harness.model.unit.status = WaitingStatus("fake message") + # Then test the event of a correct cluster bootstrapping. self.charm.on.start.emit() + self.assertEqual( + _postgresql.create_user.call_count, 2 + ) # Considering the previous failed call. _oversee_users.assert_called_once() self.assertTrue(isinstance(self.harness.model.unit.status, ActiveStatus)) From 61946dab2e656bec4ad5a7d1a5c3c73dd7204700 Mon Sep 17 00:00:00 2001 From: Marcelo Henrique Neppel Date: Thu, 25 Aug 2022 10:14:00 -0300 Subject: [PATCH 07/18] Change action name --- actions.yaml | 2 +- src/charm.py | 16 +++++++--------- tests/integration/helpers.py | 8 ++++---- tests/integration/test_charm.py | 11 ++++------- tests/unit/test_charm.py | 30 +++++++++++++++--------------- 5 files changed, 31 insertions(+), 36 deletions(-) diff --git a/actions.yaml b/actions.yaml index f465bd1a91..7f02b9e9e4 100644 --- a/actions.yaml +++ b/actions.yaml @@ -3,6 +3,6 @@ get-primary: description: Get the unit which is the primary/leader in the replication. -get-operator-password: +get-password: description: Get the operator user password used by charm. It is internal charm user, SHOULD NOT be used by applications. diff --git a/src/charm.py b/src/charm.py index d24ce1e570..3bc82ce0d4 100755 --- a/src/charm.py +++ b/src/charm.py @@ -63,9 +63,7 @@ def __init__(self, *args): self.framework.observe(self.on[PEER].relation_departed, self._on_peer_relation_departed) self.framework.observe(self.on.pgdata_storage_detaching, self._on_pgdata_storage_detaching) self.framework.observe(self.on.start, self._on_start) - self.framework.observe( - self.on.get_operator_password_action, self._on_get_operator_password - ) + self.framework.observe(self.on.get_password_action, self._on_get_password) self.framework.observe(self.on.update_status, self._on_update_status) self._cluster_name = self.app.name self._member_name = self.unit.name.replace("/", "-") @@ -81,7 +79,7 @@ def postgresql(self) -> PostgreSQL: return PostgreSQL( host=self.primary_endpoint, user=USER, - password=self._get_operator_password(), + password=self._get_password(), database="postgres", ) @@ -337,7 +335,7 @@ def _patroni(self) -> Patroni: self._member_name, self.app.planned_units(), self._peer_members_ips, - self._get_operator_password(), + self._get_password(), self._replication_password, ) @@ -524,7 +522,7 @@ def _on_start(self, event) -> None: if self._has_blocked_status: return - postgres_password = self._get_operator_password() + postgres_password = self._get_password() # If the leader was not elected (and the needed passwords were not generated yet), # the cluster cannot be bootstrapped yet. if not postgres_password or not self._replication_password: @@ -570,9 +568,9 @@ def _on_start(self, event) -> None: self._peers.data[self.app]["cluster_initialised"] = "True" self.unit.status = ActiveStatus() - def _on_get_operator_password(self, event: ActionEvent) -> None: + def _on_get_password(self, event: ActionEvent) -> None: """Returns the password for the operator user as an action response.""" - event.set_results({"operator-password": self._get_operator_password()}) + event.set_results({"operator-password": self._get_password()}) def _on_update_status(self, _) -> None: """Update endpoints of the postgres client relation and update users list.""" @@ -586,7 +584,7 @@ def _has_blocked_status(self) -> bool: """Returns whether the unit is in a blocked state.""" return isinstance(self.unit.status, BlockedStatus) - def _get_operator_password(self) -> str: + def _get_password(self) -> str: """Get operator user password. Returns: diff --git a/tests/integration/helpers.py b/tests/integration/helpers.py index baf6a42537..91ce885196 100644 --- a/tests/integration/helpers.py +++ b/tests/integration/helpers.py @@ -68,7 +68,7 @@ async def check_database_users_existence( """ unit = ops_test.model.applications[DATABASE_APP_NAME].units[0] unit_address = await unit.get_public_address() - password = await get_operator_password(ops_test, unit.name) + password = await get_password(ops_test, unit.name) # Retrieve all users in the database. users_in_db = await execute_query_on_unit( @@ -94,7 +94,7 @@ async def check_databases_creation(ops_test: OpsTest, databases: List[str]) -> N databases: List of database names that should have been created """ unit = ops_test.model.applications[DATABASE_APP_NAME].units[0] - password = await get_operator_password(ops_test, unit.name) + password = await get_password(ops_test, unit.name) for unit in ops_test.model.applications[DATABASE_APP_NAME].units: unit_address = await unit.get_public_address() @@ -344,7 +344,7 @@ def get_application_units_ips(ops_test: OpsTest, application_name: str) -> List[ return [unit.public_address for unit in ops_test.model.applications[application_name].units] -async def get_operator_password(ops_test: OpsTest, unit_name: str) -> str: +async def get_password(ops_test: OpsTest, unit_name: str) -> str: """Retrieve the operator user password using the action. Args: @@ -355,7 +355,7 @@ async def get_operator_password(ops_test: OpsTest, unit_name: str) -> str: the operator user password. """ unit = ops_test.model.units.get(unit_name) - action = await unit.run_action("get-operator-password") + action = await unit.run_action("get-password") result = await action.wait() return result.results["operator-password"] diff --git a/tests/integration/test_charm.py b/tests/integration/test_charm.py index a689f297aa..09e0a16e3b 100644 --- a/tests/integration/test_charm.py +++ b/tests/integration/test_charm.py @@ -19,7 +19,7 @@ convert_records_to_dict, db_connect, find_unit, - get_operator_password, + get_password, get_primary, get_unit_address, scale_application, @@ -79,11 +79,8 @@ async def test_settings_are_correct(ops_test: OpsTest, series: str, unit_id: int application_name = build_application_name(series) # Retrieving the operator user password using the action. - action = await ops_test.model.units.get(f"{application_name}/{unit_id}").run_action( - "get-operator-password" - ) - action = await action.wait() - password = action.results["operator-password"] + any_unit_name = ops_test.model.applications[application_name].units[0].name + password = await get_password(ops_test, any_unit_name) # Connect to PostgreSQL. host = get_unit_address(ops_test, f"{application_name}/{unit_id}") @@ -215,7 +212,7 @@ async def test_persist_data_through_primary_deletion(ops_test: OpsTest, series: application_name = build_application_name(series) any_unit_name = ops_test.model.applications[application_name].units[0].name primary = await get_primary(ops_test, any_unit_name) - password = await get_operator_password(ops_test, primary) + password = await get_password(ops_test, primary) # Write data to primary IP. host = get_unit_address(ops_test, primary) diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py index ea7b323e1c..5e69bf0a8e 100644 --- a/tests/unit/test_charm.py +++ b/tests/unit/test_charm.py @@ -157,10 +157,10 @@ def test_on_leader_elected( @patch("charm.Patroni.member_started") @patch("charm.Patroni.bootstrap_cluster") @patch("charm.PostgresqlOperatorCharm._replication_password") - @patch("charm.PostgresqlOperatorCharm._get_operator_password") + @patch("charm.PostgresqlOperatorCharm._get_password") def test_on_start( self, - _get_operator_password, + _get_password, _replication_password, _bootstrap_cluster, _member_started, @@ -170,13 +170,13 @@ def test_on_start( _oversee_users, ): # Test before the passwords are generated. - _get_operator_password.return_value = None + _get_password.return_value = None self.charm.on.start.emit() _bootstrap_cluster.assert_not_called() self.assertTrue(isinstance(self.harness.model.unit.status, WaitingStatus)) # Mock the passwords. - _get_operator_password.return_value = "fake-operator-password" + _get_password.return_value = "fake-operator-password" _replication_password.return_value = "fake-replication-password" # Mock cluster start and postgres user creation success values. @@ -213,9 +213,9 @@ def test_on_start( @patch("charm.Patroni.bootstrap_cluster") @patch("charm.PostgresqlOperatorCharm._replication_password") - @patch("charm.PostgresqlOperatorCharm._get_operator_password") + @patch("charm.PostgresqlOperatorCharm._get_password") def test_on_start_after_blocked_state( - self, _get_operator_password, _replication_password, _bootstrap_cluster + self, _get_password, _replication_password, _bootstrap_cluster ): # Set an initial blocked status (like after the install hook was triggered). initial_status = BlockedStatus("fake message") @@ -223,30 +223,30 @@ def test_on_start_after_blocked_state( # Test for a failed cluster bootstrapping. self.charm.on.start.emit() - _get_operator_password.assert_not_called() + _get_password.assert_not_called() _replication_password.assert_not_called() _bootstrap_cluster.assert_not_called() # Assert the status didn't change. self.assertEqual(self.harness.model.unit.status, initial_status) - @patch("charm.PostgresqlOperatorCharm._get_operator_password") - def test_on_get_operator_password(self, _get_operator_password): + @patch("charm.PostgresqlOperatorCharm._get_password") + def test_on_get_password(self, _get_password): mock_event = Mock() - _get_operator_password.return_value = "test-password" - self.charm._on_get_operator_password(mock_event) - _get_operator_password.assert_called_once() + _get_password.return_value = "test-password" + self.charm._on_get_password(mock_event) + _get_password.assert_called_once() mock_event.set_results.assert_called_once_with({"operator-password": "test-password"}) @patch("charm.PostgreSQLProvider.update_endpoints") @patch("charm.Patroni.update_cluster_members") @patch_network_get(private_address="1.1.1.1") - def test_get_operator_password(self, _, __): + def test_get_password(self, _, __): # Test for a None password. - self.assertIsNone(self.charm._get_operator_password()) + self.assertIsNone(self.charm._get_password()) # Then test for a non empty password after leader election and peer data set. self.harness.set_leader() - password = self.charm._get_operator_password() + password = self.charm._get_password() self.assertIsNotNone(password) self.assertNotEqual(password, "") From 5f01c6e0ec5d3133aa8dcb992dfe40bbc7fb0e9f Mon Sep 17 00:00:00 2001 From: Marcelo Henrique Neppel Date: Thu, 25 Aug 2022 14:44:59 -0300 Subject: [PATCH 08/18] Rework secrets management --- src/charm.py | 63 +++++++++++++++++++++++++++++++++------- src/constants.py | 2 ++ tests/unit/test_charm.py | 42 ++++++++++++++++++++++++++- 3 files changed, 95 insertions(+), 12 deletions(-) diff --git a/src/charm.py b/src/charm.py index 3bc82ce0d4..b09a6f87b1 100755 --- a/src/charm.py +++ b/src/charm.py @@ -7,7 +7,7 @@ import logging import os import subprocess -from typing import List, Optional, Set +from typing import Dict, List, Optional, Set from charms.operator_libs_linux.v0 import apt from charms.postgresql_k8s.v0.postgresql import PostgreSQL, PostgreSQLCreateUserError @@ -37,7 +37,7 @@ RemoveRaftMemberFailedError, SwitchoverFailedError, ) -from constants import PEER, USER +from constants import PEER, REPLICATION_PASSWORD_KEY, USER, USER_PASSWORD_KEY from relations.db import DbProvides from relations.postgresql_provider import PostgreSQLProvider from utils import new_password @@ -73,6 +73,48 @@ def __init__(self, *args): self.legacy_db_relation = DbProvides(self, admin=False) self.legacy_db_admin_relation = DbProvides(self, admin=True) + @property + def app_peer_data(self) -> Dict: + """Application peer relation data object.""" + relation = self.model.get_relation(PEER) + if relation is None: + return {} + + return relation.data[self.app] + + @property + def unit_peer_data(self) -> Dict: + """Unit peer relation data object.""" + relation = self.model.get_relation(PEER) + if relation is None: + return {} + + return relation.data[self.unit] + + def _get_secret(self, scope: str, key: str) -> Optional[str]: + """Get secret from the secret storage.""" + if scope == "unit": + return self.unit_peer_data.get(key, None) + elif scope == "app": + return self.app_peer_data.get(key, None) + else: + raise RuntimeError("Unknown secret scope.") + + def _set_secret(self, scope: str, key: str, value: Optional[str]) -> None: + """Get secret from the secret storage.""" + if scope == "unit": + if not value: + del self.unit_peer_data[key] + return + self.unit_peer_data.update({key: value}) + elif scope == "app": + if not value: + del self.app_peer_data[key] + return + self.app_peer_data.update({key: value}) + else: + raise RuntimeError("Unknown secret scope.") + @property def postgresql(self) -> PostgreSQL: """Returns an instance of the object used to interact with the database.""" @@ -468,10 +510,11 @@ def _inhibit_default_cluster_creation(self) -> None: def _on_leader_elected(self, event: LeaderElectedEvent) -> None: """Handle the leader-elected event.""" - data = self._peers.data[self.app] - # The leader sets the needed password on peer relation databag if they weren't set before. - data.setdefault("operator-password", new_password()) - data.setdefault("replication-password", new_password()) + # The leader sets the needed passwords if they weren't set before. + if self._get_secret("app", USER_PASSWORD_KEY) is None: + self._set_secret("app", USER_PASSWORD_KEY, new_password()) + if self._get_secret("app", REPLICATION_PASSWORD_KEY) is None: + self._set_secret("app", REPLICATION_PASSWORD_KEY, new_password()) # Update the list of the current PostgreSQL hosts when a new leader is elected. # Add this unit to the list of cluster members @@ -570,7 +613,7 @@ def _on_start(self, event) -> None: def _on_get_password(self, event: ActionEvent) -> None: """Returns the password for the operator user as an action response.""" - event.set_results({"operator-password": self._get_password()}) + event.set_results({USER_PASSWORD_KEY: self._get_password()}) def _on_update_status(self, _) -> None: """Update endpoints of the postgres client relation and update users list.""" @@ -591,8 +634,7 @@ def _get_password(self) -> str: The password from the peer relation or None if the password has not yet been set by the leader. """ - data = self._peers.data[self.app] - return data.get("operator-password") + return self._get_secret("app", USER_PASSWORD_KEY) @property def _replication_password(self) -> str: @@ -602,8 +644,7 @@ def _replication_password(self) -> str: The password from the peer relation or None if the password has not yet been set by the leader. """ - data = self._peers.data[self.app] - return data.get("replication-password") + return self._get_secret("app", REPLICATION_PASSWORD_KEY) def _install_apt_packages(self, _, packages: List[str]) -> None: """Simple wrapper around 'apt-get install -y. diff --git a/src/constants.py b/src/constants.py index 1dea2ef46c..8fcc127f29 100644 --- a/src/constants.py +++ b/src/constants.py @@ -9,4 +9,6 @@ LEGACY_DB_ADMIN = "db-admin" PEER = "database-peers" ALL_CLIENT_RELATIONS = [DATABASE, LEGACY_DB, LEGACY_DB_ADMIN] +REPLICATION_PASSWORD_KEY = "replication-password" USER = "operator" +USER_PASSWORD_KEY = "operator-password" diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py index 5e69bf0a8e..41fd7ac032 100644 --- a/tests/unit/test_charm.py +++ b/tests/unit/test_charm.py @@ -27,7 +27,7 @@ def setUp(self): self.addCleanup(self.harness.cleanup) self.harness.begin() self.charm = self.harness.charm - self.harness.add_relation(self._peer_relation, self.charm.app.name) + self.rel_id = self.harness.add_relation(self._peer_relation, self.charm.app.name) @patch_network_get(private_address="1.1.1.1") @patch("charm.PostgresqlOperatorCharm._install_pip_packages") @@ -305,3 +305,43 @@ def test_install_pip_packages(self, _call): # Then, test for an error. with self.assertRaises(subprocess.SubprocessError): self.charm._install_pip_packages(packages) + + @patch_network_get(private_address="1.1.1.1") + @patch("charm.PostgresqlOperatorCharm._on_leader_elected") + def test_get_secret(self, _): + self.harness.set_leader() + + # Test application scope. + assert self.charm._get_secret("app", "password") is None + self.harness.update_relation_data( + self.rel_id, self.charm.app.name, {"password": "test-password"} + ) + assert self.charm._get_secret("app", "password") == "test-password" + + # Test unit scope. + assert self.charm._get_secret("unit", "password") is None + self.harness.update_relation_data( + self.rel_id, self.charm.unit.name, {"password": "test-password"} + ) + assert self.charm._get_secret("unit", "password") == "test-password" + + @patch_network_get(private_address="1.1.1.1") + @patch("charm.PostgresqlOperatorCharm._on_leader_elected") + def test_set_secret(self, _): + self.harness.set_leader() + + # Test application scope. + assert "password" not in self.harness.get_relation_data(self.rel_id, self.charm.app.name) + self.charm._set_secret("app", "password", "test-password") + assert ( + self.harness.get_relation_data(self.rel_id, self.charm.app.name)["password"] + == "test-password" + ) + + # Test unit scope. + assert "password" not in self.harness.get_relation_data(self.rel_id, self.charm.unit.name) + self.charm._set_secret("unit", "password", "test-password") + assert ( + self.harness.get_relation_data(self.rel_id, self.charm.unit.name)["password"] + == "test-password" + ) From aa14718c194ad10600b553565866a8a4c7fb4f05 Mon Sep 17 00:00:00 2001 From: Marcelo Henrique Neppel Date: Fri, 26 Aug 2022 10:05:16 -0300 Subject: [PATCH 09/18] Add password rotation logic --- actions.yaml | 17 ++- lib/charms/postgresql_k8s/v0/postgresql.py | 28 ++++- src/charm.py | 86 ++++++++++++++- src/cluster.py | 10 +- src/constants.py | 3 + tests/integration/helpers.py | 58 +++++++++- tests/integration/test_password_rotation.py | 68 ++++++++++++ tests/unit/test_charm.py | 115 +++++++++++++++++--- tests/unit/test_cluster.py | 2 +- 9 files changed, 353 insertions(+), 34 deletions(-) create mode 100644 tests/integration/test_password_rotation.py diff --git a/actions.yaml b/actions.yaml index 7f02b9e9e4..af012a3fe2 100644 --- a/actions.yaml +++ b/actions.yaml @@ -4,5 +4,18 @@ get-primary: description: Get the unit which is the primary/leader in the replication. get-password: - description: Get the operator user password used by charm. - It is internal charm user, SHOULD NOT be used by applications. + description: Change the system user's password, which is used by charm. + It is for internal charm users and SHOULD NOT be used by applications. + params: + username: + type: string + description: The username, the default value 'operator'. + Possible values - operator, replication. +set-password: + description: Change the system user's password, which is used by charm. + It is for internal charm users and SHOULD NOT be used by applications. + params: + username: + type: string + description: The username, the default value 'operator'. + Possible values - operator, replication. diff --git a/lib/charms/postgresql_k8s/v0/postgresql.py b/lib/charms/postgresql_k8s/v0/postgresql.py index 524d5f8f63..6e87561a5e 100644 --- a/lib/charms/postgresql_k8s/v0/postgresql.py +++ b/lib/charms/postgresql_k8s/v0/postgresql.py @@ -32,7 +32,7 @@ # Increment this PATCH version before using `charmcraft publish-lib` or reset # to 0 if you are raising the major API version -LIBPATCH = 1 +LIBPATCH = 2 logger = logging.getLogger(__name__) @@ -58,6 +58,10 @@ class PostgreSQLListUsersError(Exception): """Exception raised when retrieving PostgreSQL users list fails.""" +class PostgreSQLUpdateUserPasswordError(Exception): + """Exception raised when updating a user password fails.""" + + class PostgreSQL: """Class to encapsulate all operations related to interacting with PostgreSQL instance.""" @@ -196,3 +200,25 @@ def list_users(self) -> Set[str]: except psycopg2.Error as e: logger.error(f"Failed to list PostgreSQL database users: {e}") raise PostgreSQLListUsersError() + + def update_user_password(self, username: str, password: str) -> None: + """Update a user password. + Args: + username: the user to update the password. + password: the new password for the user. + Raises: + PostgreSQLUpdateUserPasswordError if the password couldn't be changed. + """ + try: + with self._connect_to_database() as connection, connection.cursor() as cursor: + cursor.execute( + sql.SQL("ALTER USER {} WITH ENCRYPTED PASSWORD '" + password + "';").format( + sql.Identifier(username) + ) + ) + except psycopg2.Error as e: + logger.error(f"Failed to update user password: {e}") + raise PostgreSQLUpdateUserPasswordError() + finally: + if connection is not None: + connection.close() diff --git a/src/charm.py b/src/charm.py index b09a6f87b1..31b93b6e15 100755 --- a/src/charm.py +++ b/src/charm.py @@ -10,7 +10,11 @@ from typing import Dict, List, Optional, Set from charms.operator_libs_linux.v0 import apt -from charms.postgresql_k8s.v0.postgresql import PostgreSQL, PostgreSQLCreateUserError +from charms.postgresql_k8s.v0.postgresql import ( + PostgreSQL, + PostgreSQLCreateUserError, + PostgreSQLUpdateUserPasswordError, +) from ops.charm import ( ActionEvent, CharmBase, @@ -37,7 +41,13 @@ RemoveRaftMemberFailedError, SwitchoverFailedError, ) -from constants import PEER, REPLICATION_PASSWORD_KEY, USER, USER_PASSWORD_KEY +from constants import ( + PEER, + REPLICATION_PASSWORD_KEY, + SYSTEM_USERS, + USER, + USER_PASSWORD_KEY, +) from relations.db import DbProvides from relations.postgresql_provider import PostgreSQLProvider from utils import new_password @@ -64,6 +74,7 @@ def __init__(self, *args): self.framework.observe(self.on.pgdata_storage_detaching, self._on_pgdata_storage_detaching) self.framework.observe(self.on.start, self._on_start) self.framework.observe(self.on.get_password_action, self._on_get_password) + self.framework.observe(self.on.set_password_action, self._on_set_password) self.framework.observe(self.on.update_status, self._on_update_status) self._cluster_name = self.app.name self._member_name = self.unit.name.replace("/", "-") @@ -121,7 +132,7 @@ def postgresql(self) -> PostgreSQL: return PostgreSQL( host=self.primary_endpoint, user=USER, - password=self._get_password(), + password=self._get_secret("app", f"{USER}-password"), database="postgres", ) @@ -612,8 +623,73 @@ def _on_start(self, event) -> None: self.unit.status = ActiveStatus() def _on_get_password(self, event: ActionEvent) -> None: - """Returns the password for the operator user as an action response.""" - event.set_results({USER_PASSWORD_KEY: self._get_password()}) + """Returns the password for a user as an action response. + + If no user is provided, the password of the operator user is returned. + """ + username = event.params.get("username", USER) + if username not in SYSTEM_USERS: + event.fail( + f"The action can be run only for users used by the charm or Patroni:" + f" {', '.join(SYSTEM_USERS)} not {username}" + ) + return + event.set_results( + {f"{username}-password": self._get_secret("app", f"{username}-password")} + ) + + def _on_set_password(self, event: ActionEvent) -> None: + """Set the password for the specified user.""" + # Only leader can write the new password into peer relation. + if not self.unit.is_leader(): + event.fail("The action can be run only on leader unit") + return + + username = event.params.get("username", USER) + if username not in SYSTEM_USERS: + event.fail( + f"The action can be run only for users used by the charm:" + f" {', '.join(SYSTEM_USERS)} not {username}" + ) + return + + password = new_password() + if "password" in event.params: + password = event.params["password"] + + if password == self._get_secret("app", f"{username}-password"): + event.log("The old and new passwords are equal.") + event.set_results({f"{username}-password": password}) + return + + # Ensure all members are ready before trying to reload Patroni + # configuration to avoid errors (like the API not responding in + # one instance because PostgreSQL and/or Patroni are not ready). + if not self._patroni.are_all_members_ready(): + event.fail( + "Failed changing the password: Not all members healthy or finished initial sync." + ) + return + + # Update the password in the PostgreSQL instance. + try: + self.postgresql.update_user_password(username, password) + except PostgreSQLUpdateUserPasswordError as e: + logger.exception(e) + event.fail( + "Failed changing the password: Not all members healthy or finished initial sync." + ) + return + + # Update the password in the secret store. + self._set_secret("app", f"{username}-password", password) + + # Update and reload Patroni configuration in this unit to use the new password. + # Other units Patroni configuration will be reloaded in the peer relation changed event. + self._patroni.render_patroni_yml_file() + self._patroni.reload_patroni_configuration() + + event.set_results({f"{username}-password": password}) def _on_update_status(self, _) -> None: """Update endpoints of the postgres client relation and update users list.""" diff --git a/src/cluster.py b/src/cluster.py index 03ece3d92f..9c28b19119 100644 --- a/src/cluster.py +++ b/src/cluster.py @@ -98,7 +98,7 @@ def configure_patroni_on_unit(self, replica: bool = False): (defaults to False, which configures the unit as a leader) """ self._change_owner(self.storage_path) - self._render_patroni_yml_file(replica) + self.render_patroni_yml_file(replica) self._render_patroni_service_file() # Reload systemd services before trying to start Patroni. daemon_reload() @@ -248,7 +248,7 @@ def _render_patroni_service_file(self) -> None: rendered = template.render(conf_path=self.storage_path) self._render_file("/etc/systemd/system/patroni.service", rendered, 0o644) - def _render_patroni_yml_file(self, replica: bool = False) -> None: + def render_patroni_yml_file(self, replica: bool = False) -> None: """Render the Patroni configuration file.""" # Open the template patroni.yml file. with open("templates/patroni.yml.j2", "r") as file: @@ -320,10 +320,10 @@ def primary_changed(self, old_primary: str) -> bool: def update_cluster_members(self) -> None: """Update the list of members of the cluster.""" # Update the members in the Patroni configuration. - self._render_patroni_yml_file() + self.render_patroni_yml_file() if service_running(PATRONI_SERVICE): - self._reload_patroni_configuration() + self.reload_patroni_configuration() def remove_raft_member(self, member_ip: str) -> None: """Remove a member from the raft cluster. @@ -353,6 +353,6 @@ def remove_raft_member(self, member_ip: str) -> None: raise RemoveRaftMemberFailedError() @retry(stop=stop_after_attempt(3), wait=wait_exponential(multiplier=1, min=2, max=10)) - def _reload_patroni_configuration(self): + def reload_patroni_configuration(self): """Reload Patroni configuration after it was changed.""" requests.post(f"http://{self.unit_ip}:8008/reload") diff --git a/src/constants.py b/src/constants.py index 8fcc127f29..1554176034 100644 --- a/src/constants.py +++ b/src/constants.py @@ -9,6 +9,9 @@ LEGACY_DB_ADMIN = "db-admin" PEER = "database-peers" ALL_CLIENT_RELATIONS = [DATABASE, LEGACY_DB, LEGACY_DB_ADMIN] +REPLICATION_USER = "replication" REPLICATION_PASSWORD_KEY = "replication-password" USER = "operator" USER_PASSWORD_KEY = "operator-password" +# List of system usernames needed for correct work of the charm/workload. +SYSTEM_USERS = [REPLICATION_USER, USER] diff --git a/tests/integration/helpers.py b/tests/integration/helpers.py index 91ce885196..881aa1c4cb 100644 --- a/tests/integration/helpers.py +++ b/tests/integration/helpers.py @@ -4,6 +4,7 @@ import itertools import tempfile import zipfile +from datetime import datetime from pathlib import Path from typing import List @@ -12,7 +13,13 @@ import yaml from juju.unit import Unit from pytest_operator.plugin import OpsTest -from tenacity import Retrying, stop_after_attempt, wait_exponential +from tenacity import ( + Retrying, + retry, + retry_if_result, + stop_after_attempt, + wait_exponential, +) METADATA = yaml.safe_load(Path("./metadata.yaml").read_text()) DATABASE_APP_NAME = METADATA["name"] @@ -118,6 +125,30 @@ async def check_databases_creation(ops_test: OpsTest, databases: List[str]) -> N assert len(output) +@retry( + retry=retry_if_result(lambda x: not x), + stop=stop_after_attempt(10), + wait=wait_exponential(multiplier=1, min=2, max=30), +) +def check_patroni(ops_test: OpsTest, unit_name: str, restart_time: float) -> bool: + """Check if Patroni is running correctly on a specific unit. + + Args: + ops_test: The ops test framework instance + unit_name: The name of the unit + restart_time: Point in time before the unit was restarted. + + Returns: + whether Patroni is running correctly. + """ + unit_ip = get_unit_address(ops_test, unit_name) + health_info = requests.get(f"http://{unit_ip}:8008/health").json() + postmaster_start_time = datetime.strptime( + health_info["postmaster_start_time"], "%Y-%m-%d %H:%M:%S.%f%z" + ).timestamp() + return postmaster_start_time > restart_time and health_info["state"] == "running" + + def build_application_name(series: str) -> str: """Return a composite application name combining application name and series.""" return f"{DATABASE_APP_NAME}-{series}" @@ -344,15 +375,15 @@ def get_application_units_ips(ops_test: OpsTest, application_name: str) -> List[ return [unit.public_address for unit in ops_test.model.applications[application_name].units] -async def get_password(ops_test: OpsTest, unit_name: str) -> str: - """Retrieve the operator user password using the action. +async def get_password(ops_test: OpsTest, unit_name: str, username: str = "operator") -> str: + """Retrieve a user password using the action. Args: ops_test: ops_test instance. unit_name: the name of the unit. Returns: - the operator user password. + the user password. """ unit = ops_test.model.units.get(unit_name) action = await unit.run_action("get-password") @@ -409,6 +440,25 @@ async def scale_application(ops_test: OpsTest, application_name: str, count: int ) +def restart_patroni(ops_test: OpsTest, unit_name: str) -> None: + """Restart Patroni on a specific unit. + + Args: + ops_test: The ops test framework instance + unit_name: The name of the unit + """ + unit_ip = get_unit_address(ops_test, unit_name) + requests.post(f"http://{unit_ip}:8008/restart") + + +async def set_password(ops_test: OpsTest, unit_name: str, username: str = "operator"): + """Retrieve a user password using the action.""" + unit = ops_test.model.units.get(unit_name) + action = await unit.run_action("set-password", **{"username": username}) + result = await action.wait() + return result.results + + def switchover(ops_test: OpsTest, current_primary: str, candidate: str = None) -> None: """Trigger a switchover. diff --git a/tests/integration/test_password_rotation.py b/tests/integration/test_password_rotation.py new file mode 100644 index 0000000000..2f4e8ae4b5 --- /dev/null +++ b/tests/integration/test_password_rotation.py @@ -0,0 +1,68 @@ +#!/usr/bin/env python3 +# Copyright 2022 Canonical Ltd. +# See LICENSE file for licensing details. +import time + +import pytest +from pytest_operator.plugin import OpsTest + +from tests.helpers import METADATA +from tests.integration.helpers import ( + check_patroni, + get_password, + restart_patroni, + set_password, +) + +APP_NAME = METADATA["name"] + + +@pytest.mark.skip_if_deployed +@pytest.mark.abort_on_fail +async def test_deploy_active(ops_test: OpsTest): + """Build the charm and deploy it.""" + charm = await ops_test.build_charm(".") + async with ops_test.fast_forward(): + await ops_test.model.deploy( + charm, resources={"patroni": "patroni.tar.gz"}, application_name=APP_NAME, num_units=3 + ) + await ops_test.juju("attach-resource", APP_NAME, "patroni=patroni.tar.gz") + await ops_test.model.wait_for_idle(apps=[APP_NAME], status="active", timeout=1000) + + +async def test_password_rotation(ops_test: OpsTest): + """Test password rotation action.""" + # Get the initial passwords set for the system users. + any_unit_name = ops_test.model.applications[APP_NAME].units[0].name + superuser_password = await get_password(ops_test, any_unit_name) + replication_password = await get_password(ops_test, any_unit_name, "replication") + + # Get the leader unit name (because passwords can only be set through it). + leader = None + for unit in ops_test.model.applications[APP_NAME].units: + if await unit.is_leader_from_status(): + leader = unit.name + break + + # Change both passwords. + result = await set_password(ops_test, unit_name=leader) + assert "operator-password" in result.keys() + await ops_test.model.wait_for_idle(apps=[APP_NAME], status="active", timeout=1000) + + result = await set_password(ops_test, unit_name=leader, username="replication") + assert "replication-password" in result.keys() + await ops_test.model.wait_for_idle(apps=[APP_NAME], status="active", timeout=1000) + + new_superuser_password = await get_password(ops_test, any_unit_name) + new_replication_password = await get_password(ops_test, any_unit_name, "replication") + + assert superuser_password != new_superuser_password + assert replication_password != new_replication_password + + # Restart Patroni on any non-leader unit and check that + # Patroni and PostgreSQL continue to work. + restart_time = time.time() + for unit in ops_test.model.applications[APP_NAME].units: + if not await unit.is_leader_from_status(): + restart_patroni(ops_test, unit.name) + assert check_patroni(ops_test, unit.name, restart_time) diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py index 41fd7ac032..3e8e429c60 100644 --- a/tests/unit/test_charm.py +++ b/tests/unit/test_charm.py @@ -3,10 +3,13 @@ import os import subprocess import unittest -from unittest.mock import Mock, PropertyMock, call, mock_open, patch +from unittest.mock import MagicMock, Mock, PropertyMock, call, mock_open, patch from charms.operator_libs_linux.v0 import apt -from charms.postgresql_k8s.v0.postgresql import PostgreSQLCreateUserError +from charms.postgresql_k8s.v0.postgresql import ( + PostgreSQLCreateUserError, + PostgreSQLUpdateUserPasswordError, +) from ops.model import ActiveStatus, BlockedStatus, WaitingStatus from ops.testing import Harness @@ -229,26 +232,106 @@ def test_on_start_after_blocked_state( # Assert the status didn't change. self.assertEqual(self.harness.model.unit.status, initial_status) - @patch("charm.PostgresqlOperatorCharm._get_password") - def test_on_get_password(self, _get_password): - mock_event = Mock() - _get_password.return_value = "test-password" + def test_on_get_password(self): + # Create a mock event and set passwords in peer relation data. + mock_event = MagicMock(params={}) + self.harness.update_relation_data( + self.rel_id, + self.charm.app.name, + { + "operator-password": "test-password", + "replication-password": "replication-test-password", + }, + ) + + # Test providing an invalid username. + mock_event.params["username"] = "user" + self.charm._on_get_password(mock_event) + mock_event.fail.assert_called_once() + mock_event.set_results.assert_not_called() + + # Test without providing the username option. + mock_event.reset_mock() + del mock_event.params["username"] self.charm._on_get_password(mock_event) - _get_password.assert_called_once() mock_event.set_results.assert_called_once_with({"operator-password": "test-password"}) - @patch("charm.PostgreSQLProvider.update_endpoints") - @patch("charm.Patroni.update_cluster_members") + # Also test providing the username option. + mock_event.reset_mock() + mock_event.params["username"] = "replication" + self.charm._on_get_password(mock_event) + mock_event.set_results.assert_called_once_with( + {"replication-password": "replication-test-password"} + ) + @patch_network_get(private_address="1.1.1.1") - def test_get_password(self, _, __): - # Test for a None password. - self.assertIsNone(self.charm._get_password()) + @patch("charm.Patroni.reload_patroni_configuration") + @patch("charm.Patroni.render_patroni_yml_file") + @patch("charm.PostgresqlOperatorCharm._set_secret") + @patch("charm.PostgresqlOperatorCharm.postgresql") + @patch("charm.Patroni.are_all_members_ready") + @patch("charm.PostgresqlOperatorCharm._on_leader_elected") + def test_on_set_password( + self, + _, + _are_all_members_ready, + _postgresql, + _set_secret, + _render_patroni_yml_file, + _reload_patroni_configuration, + ): + # Create a mock event. + mock_event = MagicMock(params={}) - # Then test for a non empty password after leader election and peer data set. + # Set some values for the other mocks. + _are_all_members_ready.side_effect = [False, True, True, True, True] + _postgresql.update_user_password = PropertyMock( + side_effect=[PostgreSQLUpdateUserPasswordError, None, None, None] + ) + + # Test trying to set a password through a non leader unit. + self.charm._on_set_password(mock_event) + mock_event.fail.assert_called_once() + _set_secret.assert_not_called() + + # Test providing an invalid username. self.harness.set_leader() - password = self.charm._get_password() - self.assertIsNotNone(password) - self.assertNotEqual(password, "") + mock_event.reset_mock() + mock_event.params["username"] = "user" + self.charm._on_set_password(mock_event) + mock_event.fail.assert_called_once() + _set_secret.assert_not_called() + + # Test without providing the username option but without all cluster members ready. + mock_event.reset_mock() + del mock_event.params["username"] + self.charm._on_set_password(mock_event) + mock_event.fail.assert_called_once() + _set_secret.assert_not_called() + + # Test for an error updating when updating the user password in the database. + mock_event.reset_mock() + self.charm._on_set_password(mock_event) + mock_event.fail.assert_called_once() + _set_secret.assert_not_called() + + # Test without providing the username option. + self.charm._on_set_password(mock_event) + self.assertEqual(_set_secret.call_args_list[0][0][1], "operator-password") + + # Also test providing the username option. + _set_secret.reset_mock() + mock_event.params["username"] = "replication" + self.charm._on_set_password(mock_event) + self.assertEqual(_set_secret.call_args_list[0][0][1], "replication-password") + + # And test providing both the username and password options. + _set_secret.reset_mock() + mock_event.params["password"] = "replication-test-password" + self.charm._on_set_password(mock_event) + _set_secret.assert_called_once_with( + "app", "replication-password", "replication-test-password" + ) @patch("charms.operator_libs_linux.v0.apt.add_package") @patch("charms.operator_libs_linux.v0.apt.update") diff --git a/tests/unit/test_cluster.py b/tests/unit/test_cluster.py index 2f5009d572..6947cae694 100644 --- a/tests/unit/test_cluster.py +++ b/tests/unit/test_cluster.py @@ -123,7 +123,7 @@ def test_render_patroni_yml_file(self, _, _render_file): # Patch the `open` method with our mock. with patch("builtins.open", mock, create=True): # Call the method. - self.patroni._render_patroni_yml_file() + self.patroni.render_patroni_yml_file() # Check the template is opened read-only in the call to open. self.assertEqual(mock.call_args_list[0][0], ("templates/patroni.yml.j2", "r")) From 9e7fdfd7f85ed8ac9e84cb6500470ac168ac10a3 Mon Sep 17 00:00:00 2001 From: Marcelo Henrique Neppel Date: Fri, 26 Aug 2022 10:52:11 -0300 Subject: [PATCH 10/18] Add user to the action parameters --- tests/integration/helpers.py | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/tests/integration/helpers.py b/tests/integration/helpers.py index 881aa1c4cb..08cebe8c74 100644 --- a/tests/integration/helpers.py +++ b/tests/integration/helpers.py @@ -381,14 +381,15 @@ async def get_password(ops_test: OpsTest, unit_name: str, username: str = "opera Args: ops_test: ops_test instance. unit_name: the name of the unit. + username: the user to get the password. Returns: the user password. """ unit = ops_test.model.units.get(unit_name) - action = await unit.run_action("get-password") + action = await unit.run_action("get-password", **{"username": username}) result = await action.wait() - return result.results["operator-password"] + return result.results[f"{username}-password"] async def get_primary(ops_test: OpsTest, unit_name: str) -> str: @@ -452,7 +453,16 @@ def restart_patroni(ops_test: OpsTest, unit_name: str) -> None: async def set_password(ops_test: OpsTest, unit_name: str, username: str = "operator"): - """Retrieve a user password using the action.""" + """Retrieve a user password using the action. + + Args: + ops_test: ops_test instance. + unit_name: the name of the unit. + username: the user to set the password. + + Returns: + the results from the action. + """ unit = ops_test.model.units.get(unit_name) action = await unit.run_action("set-password", **{"username": username}) result = await action.wait() From 12a4a7b85ac5b7dbb056eaaad017211bf6d50174 Mon Sep 17 00:00:00 2001 From: Marcelo Henrique Neppel Date: Sat, 27 Aug 2022 09:24:40 -0300 Subject: [PATCH 11/18] Add separate environments for different tests --- .github/workflows/ci.yaml | 60 +++++++++++- .github/workflows/release.yaml | 64 ++++++++++++- .../new_relations/test_new_relations.py | 10 ++ tests/integration/test_charm.py | 6 ++ tests/integration/test_db.py | 2 + tests/integration/test_db_admin.py | 2 + tox.ini | 91 ++++++++++++++++++- 7 files changed, 226 insertions(+), 9 deletions(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index a759f2c515..ab8c30e558 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -13,6 +13,7 @@ jobs: run: python3 -m pip install tox - name: Run linters run: tox -e lint + unit-test: name: Unit tests runs-on: ubuntu-latest @@ -25,8 +26,61 @@ jobs: run: tox -e unit - name: Upload Coverage to Codecov uses: codecov/codecov-action@v2 - integration-test-lxd: - name: Integration tests (lxd) + + integration-test-lxd-charm: + name: Integration tests for charm deployment (lxd) + runs-on: ubuntu-latest + steps: + - name: Checkout + uses: actions/checkout@v2 + - name: Setup operator environment + uses: charmed-kubernetes/actions-operator@main + with: + provider: lxd + - name: Run integration tests + run: tox -e charm-integration + + integration-test-lxd-database-relation: + name: Integration tests for database relation (lxd) + runs-on: ubuntu-latest + steps: + - name: Checkout + uses: actions/checkout@v2 + - name: Setup operator environment + uses: charmed-kubernetes/actions-operator@main + with: + provider: lxd + - name: Run integration tests + run: tox -e database-relation-integration + + integration-test-lxd-db-relation: + name: Integration tests for db relation (lxd) + runs-on: ubuntu-latest + steps: + - name: Checkout + uses: actions/checkout@v2 + - name: Setup operator environment + uses: charmed-kubernetes/actions-operator@main + with: + provider: lxd + - name: Run integration tests + run: tox -e db-relation-integration + + integration-test-lxd-db-admin-relation: + name: Integration tests for db-admin relation (lxd) + runs-on: ubuntu-latest + steps: + - name: Checkout + uses: actions/checkout@v2 + - name: Setup operator environment + uses: charmed-kubernetes/actions-operator@main + with: + provider: lxd + - name: Run integration tests + run: tox -e db-admin-relation-integration + + integration-test-lxd-password-rotation: + name: Integration tests for password rotation (lxd) runs-on: ubuntu-latest steps: - name: Checkout @@ -36,4 +90,4 @@ jobs: with: provider: lxd - name: Run integration tests - run: tox -e integration + run: tox -e password-rotation-integration diff --git a/.github/workflows/release.yaml b/.github/workflows/release.yaml index 80f700bba0..9c00b80780 100644 --- a/.github/workflows/release.yaml +++ b/.github/workflows/release.yaml @@ -42,8 +42,8 @@ jobs: - name: Run tests run: tox -e unit - integration-test-lxd: - name: Integration tests for (lxd) + integration-test-charm: + name: Integration tests for charm deployment runs-on: ubuntu-latest steps: - name: Checkout @@ -53,7 +53,59 @@ jobs: with: provider: lxd - name: Run integration tests - run: tox -e integration + run: tox -e charm-integration + + integration-test-database-relation: + name: Integration tests for database relation + runs-on: ubuntu-latest + steps: + - name: Checkout + uses: actions/checkout@v2 + - name: Setup operator environment + uses: charmed-kubernetes/actions-operator@main + with: + provider: lxd + - name: Run integration tests + run: tox -e database-relation-integration + + integration-test-db-relation: + name: Integration tests for db relation + runs-on: ubuntu-latest + steps: + - name: Checkout + uses: actions/checkout@v2 + - name: Setup operator environment + uses: charmed-kubernetes/actions-operator@main + with: + provider: lxd + - name: Run integration tests + run: tox -e db-relation-integration + + integration-test-db-admin-relation: + name: Integration tests for db-admin relation + runs-on: ubuntu-latest + steps: + - name: Checkout + uses: actions/checkout@v2 + - name: Setup operator environment + uses: charmed-kubernetes/actions-operator@main + with: + provider: lxd + - name: Run integration tests + run: tox -e db-admin-relation-integration + + integration-test-password-rotation: + name: Integration tests for password rotation + runs-on: ubuntu-latest + steps: + - name: Checkout + uses: actions/checkout@v2 + - name: Setup operator environment + uses: charmed-kubernetes/actions-operator@main + with: + provider: lxd + - name: Run integration tests + run: tox -e password-rotation-integration release-to-charmhub: name: Release to CharmHub @@ -61,7 +113,11 @@ jobs: - lib-check - lint - unit-test - - integration-test-lxd + - integration-test-charm + - integration-test-database-relation + - integration-test-db-relation + - integration-test-db-admin-relation + - integration-test-password-rotation runs-on: ubuntu-latest steps: - name: Checkout diff --git a/tests/integration/new_relations/test_new_relations.py b/tests/integration/new_relations/test_new_relations.py index dc473f90b4..5bdc1dc51b 100644 --- a/tests/integration/new_relations/test_new_relations.py +++ b/tests/integration/new_relations/test_new_relations.py @@ -31,6 +31,7 @@ @pytest.mark.abort_on_fail +@pytest.mark.database_relation async def test_deploy_charms(ops_test: OpsTest, application_charm, database_charm): """Deploy both charms (application and database) to use in the tests.""" # Deploy both charms (multiple units for each application to test that later they correctly @@ -65,6 +66,7 @@ async def test_deploy_charms(ops_test: OpsTest, application_charm, database_char await ops_test.model.wait_for_idle(apps=APP_NAMES, status="active", timeout=3000) +@pytest.mark.database_relation async def test_no_read_only_endpoint_in_standalone_cluster(ops_test: OpsTest): """Test that there is no read-only endpoint in a standalone cluster.""" async with ops_test.fast_forward(): @@ -91,6 +93,7 @@ async def test_no_read_only_endpoint_in_standalone_cluster(ops_test: OpsTest): ) +@pytest.mark.database_relation async def test_read_only_endpoint_in_scaled_up_cluster(ops_test: OpsTest): """Test that there is read-only endpoint in a scaled up cluster.""" async with ops_test.fast_forward(): @@ -109,6 +112,7 @@ async def test_read_only_endpoint_in_scaled_up_cluster(ops_test: OpsTest): @pytest.mark.abort_on_fail +@pytest.mark.database_relation async def test_database_relation_with_charm_libraries(ops_test: OpsTest): """Test basic functionality of database relation interface.""" # Get the connection string to connect to the database using the read/write endpoint. @@ -156,6 +160,7 @@ async def test_database_relation_with_charm_libraries(ops_test: OpsTest): cursor.execute("DROP TABLE test;") +@pytest.mark.database_relation 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. @@ -176,6 +181,7 @@ async def test_user_with_extra_roles(ops_test: OpsTest): connection.close() +@pytest.mark.database_relation async def test_two_applications_doesnt_share_the_same_relation_data( ops_test: OpsTest, application_charm ): @@ -210,6 +216,7 @@ async def test_two_applications_doesnt_share_the_same_relation_data( assert application_connection_string != another_application_connection_string +@pytest.mark.database_relation async def test_an_application_can_connect_to_multiple_database_clusters( ops_test: OpsTest, database_charm ): @@ -242,6 +249,7 @@ async def test_an_application_can_connect_to_multiple_database_clusters( assert application_connection_string != another_application_connection_string +@pytest.mark.database_relation async def test_an_application_can_connect_to_multiple_aliased_database_clusters( ops_test: OpsTest, database_charm ): @@ -277,6 +285,7 @@ async def test_an_application_can_connect_to_multiple_aliased_database_clusters( assert application_connection_string != another_application_connection_string +@pytest.mark.database_relation async def test_an_application_can_request_multiple_databases(ops_test: OpsTest, application_charm): """Test that an application can request additional databases using the same interface.""" # Relate the charms using another relation and wait for them exchanging some connection data. @@ -297,6 +306,7 @@ async def test_an_application_can_request_multiple_databases(ops_test: OpsTest, assert first_database_connection_string != second_database_connection_string +@pytest.mark.database_relation async def test_relation_data_is_updated_correctly_when_scaling(ops_test: OpsTest): """Test that relation data, like connection data, is updated correctly when scaling.""" # Retrieve the list of current database unit names. diff --git a/tests/integration/test_charm.py b/tests/integration/test_charm.py index 64e828982d..a01c749eca 100644 --- a/tests/integration/test_charm.py +++ b/tests/integration/test_charm.py @@ -32,7 +32,9 @@ UNIT_IDS = [0, 1, 2] +@pytest.mark.skip_if_deployed @pytest.mark.abort_on_fail +@pytest.mark.charm @pytest.mark.parametrize("series", SERIES) async def test_deploy(ops_test: OpsTest, charm: str, series: str): """Deploy the charm-under-test. @@ -58,6 +60,7 @@ async def test_deploy(ops_test: OpsTest, charm: str, series: str): @pytest.mark.abort_on_fail +@pytest.mark.charm @pytest.mark.parametrize("series", SERIES) @pytest.mark.parametrize("unit_id", UNIT_IDS) async def test_database_is_up(ops_test: OpsTest, series: str, unit_id: int): @@ -71,6 +74,7 @@ async def test_database_is_up(ops_test: OpsTest, series: str, unit_id: int): assert result.status_code == 200 +@pytest.mark.charm @pytest.mark.parametrize("series", SERIES) @pytest.mark.parametrize("unit_id", UNIT_IDS) async def test_settings_are_correct(ops_test: OpsTest, series: str, unit_id: int): @@ -123,6 +127,7 @@ async def test_settings_are_correct(ops_test: OpsTest, series: str, unit_id: int assert settings["maximum_lag_on_failover"] == 1048576 +@pytest.mark.charm @pytest.mark.parametrize("series", SERIES) async def test_scale_down_and_up(ops_test: OpsTest, series: str): """Test data is replicated to new units after a scale up.""" @@ -208,6 +213,7 @@ async def test_scale_down_and_up(ops_test: OpsTest, series: str): await scale_application(ops_test, application_name, initial_scale) +@pytest.mark.charm @pytest.mark.parametrize("series", SERIES) async def test_persist_data_through_primary_deletion(ops_test: OpsTest, series: str): """Test data persists through a primary deletion.""" diff --git a/tests/integration/test_db.py b/tests/integration/test_db.py index 456f33b35e..f92d82ad2a 100644 --- a/tests/integration/test_db.py +++ b/tests/integration/test_db.py @@ -24,6 +24,7 @@ RELATION_NAME = "db" +@pytest.mark.db_relation async def test_mailman3_core_db(ops_test: OpsTest, charm: str) -> None: """Deploy Mailman3 Core to test the 'db' relation.""" async with ops_test.fast_forward(): @@ -92,6 +93,7 @@ async def test_mailman3_core_db(ops_test: OpsTest, charm: str) -> None: assert domain_name not in [domain.mail_host for domain in client.domains] +@pytest.mark.db_relation async def test_relation_data_is_updated_correctly_when_scaling(ops_test: OpsTest): """Test that relation data, like connection data, is updated correctly when scaling.""" # Retrieve the list of current database unit names. diff --git a/tests/integration/test_db_admin.py b/tests/integration/test_db_admin.py index 51c5e8ac50..1964537424 100644 --- a/tests/integration/test_db_admin.py +++ b/tests/integration/test_db_admin.py @@ -5,6 +5,7 @@ import json import logging +import pytest as pytest from landscape_api.base import run_query from pytest_operator.plugin import OpsTest @@ -24,6 +25,7 @@ DATABASE_UNITS = 3 +@pytest.mark.db_admin_relation async def test_landscape_scalable_bundle_db(ops_test: OpsTest, charm: str) -> None: """Deploy Landscape Scalable Bundle to test the 'db-admin' relation.""" config = { diff --git a/tox.ini b/tox.ini index 5d07a7aa2e..0a52161e5f 100644 --- a/tox.ini +++ b/tox.ini @@ -65,8 +65,96 @@ commands = coverage report coverage xml +[testenv:charm-integration] +description = Run charm integration tests +deps = + pytest + juju==2.9.11 # juju 3.0.0 has issues with retrieving action results + pytest-operator + psycopg2-binary + -r{toxinidir}/requirements.txt +commands = + # Download patroni resource to use in the charm deployment. + sh -c 'stat patroni.tar.gz > /dev/null 2>&1 || curl "https://github.com/zalando/patroni/archive/refs/tags/v2.1.3.tar.gz" -L -s > patroni.tar.gz' + pytest -v --tb native --ignore={[vars]tst_path}unit --log-cli-level=INFO -s {posargs} -m charm + # Remove the downloaded resource. + sh -c 'rm -f patroni.tar.gz' +whitelist_externals = + sh + +[testenv:database-relation-integration] +description = Run database relation integration tests +deps = + pytest + juju==2.9.11 # juju 3.0.0 has issues with retrieving action results + pytest-operator + psycopg2-binary + -r{toxinidir}/requirements.txt +commands = + # Download patroni resource to use in the charm deployment. + sh -c 'stat patroni.tar.gz > /dev/null 2>&1 || curl "https://github.com/zalando/patroni/archive/refs/tags/v2.1.3.tar.gz" -L -s > patroni.tar.gz' + pytest -v --tb native --ignore={[vars]tst_path}unit --log-cli-level=INFO -s {posargs} -m database_relation + # Remove the downloaded resource. + sh -c 'rm -f patroni.tar.gz' +whitelist_externals = + sh + +[testenv:db-relation-integration] +description = Run db relation integration tests +deps = + pytest + juju==2.9.11 # juju 3.0.0 has issues with retrieving action results + mailmanclient + pytest-operator + psycopg2-binary + -r{toxinidir}/requirements.txt +commands = + # Download patroni resource to use in the charm deployment. + sh -c 'stat patroni.tar.gz > /dev/null 2>&1 || curl "https://github.com/zalando/patroni/archive/refs/tags/v2.1.3.tar.gz" -L -s > patroni.tar.gz' + pytest -v --tb native --ignore={[vars]tst_path}unit --log-cli-level=INFO -s {posargs}-m db_relation + # Remove the downloaded resource. + sh -c 'rm -f patroni.tar.gz' +whitelist_externals = + sh + +[testenv:db-admin-relation-integration] +description = Run db-admin relation integration tests +deps = + pytest + juju==2.9.11 # juju 3.0.0 has issues with retrieving action results + landscape-api-py3 + pytest-operator + psycopg2-binary + -r{toxinidir}/requirements.txt +commands = + # Download patroni resource to use in the charm deployment. + sh -c 'stat patroni.tar.gz > /dev/null 2>&1 || curl "https://github.com/zalando/patroni/archive/refs/tags/v2.1.3.tar.gz" -L -s > patroni.tar.gz' + pytest -v --tb native --ignore={[vars]tst_path}unit --log-cli-level=INFO -s {posargs} -m db_admin_relation + # Remove the downloaded resource. + sh -c 'rm -f patroni.tar.gz' +whitelist_externals = + sh + +[testenv:password-rotation-integration] +description = Run password rotation integration tests +deps = + pytest + juju==2.9.11 # juju 3.0.0 has issues with retrieving action results + landscape-api-py3 + pytest-operator + psycopg2-binary + -r{toxinidir}/requirements.txt +commands = + # Download patroni resource to use in the charm deployment. + sh -c 'stat patroni.tar.gz > /dev/null 2>&1 || curl "https://github.com/zalando/patroni/archive/refs/tags/v2.1.3.tar.gz" -L -s > patroni.tar.gz' + pytest -v --tb native --ignore={[vars]tst_path}unit --log-cli-level=INFO -s {posargs} -m password_rotation + # Remove the downloaded resource. + sh -c 'rm -f patroni.tar.gz' +whitelist_externals = + sh + [testenv:integration] -description = Run integration tests +description = Run all integration tests deps = pytest juju==2.9.11 # juju 3.0.0 has issues with retrieving action results @@ -74,7 +162,6 @@ deps = mailmanclient pytest-operator psycopg2-binary - requests -r{toxinidir}/requirements.txt commands = # Download patroni resource to use in the charm deployment. From bc723d1b3b27b1f403a83c2d7252224e5578184d Mon Sep 17 00:00:00 2001 From: Marcelo Henrique Neppel Date: Sat, 27 Aug 2022 09:34:16 -0300 Subject: [PATCH 12/18] Add all dependencies --- tox.ini | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/tox.ini b/tox.ini index 0a52161e5f..6c123cda2a 100644 --- a/tox.ini +++ b/tox.ini @@ -70,6 +70,8 @@ description = Run charm integration tests deps = pytest juju==2.9.11 # juju 3.0.0 has issues with retrieving action results + landscape-api-py3 + mailmanclient pytest-operator psycopg2-binary -r{toxinidir}/requirements.txt @@ -87,6 +89,8 @@ description = Run database relation integration tests deps = pytest juju==2.9.11 # juju 3.0.0 has issues with retrieving action results + landscape-api-py3 + mailmanclient pytest-operator psycopg2-binary -r{toxinidir}/requirements.txt @@ -104,6 +108,7 @@ description = Run db relation integration tests deps = pytest juju==2.9.11 # juju 3.0.0 has issues with retrieving action results + landscape-api-py3 mailmanclient pytest-operator psycopg2-binary @@ -111,7 +116,7 @@ deps = commands = # Download patroni resource to use in the charm deployment. sh -c 'stat patroni.tar.gz > /dev/null 2>&1 || curl "https://github.com/zalando/patroni/archive/refs/tags/v2.1.3.tar.gz" -L -s > patroni.tar.gz' - pytest -v --tb native --ignore={[vars]tst_path}unit --log-cli-level=INFO -s {posargs}-m db_relation + pytest -v --tb native --ignore={[vars]tst_path}unit --log-cli-level=INFO -s {posargs} -m db_relation # Remove the downloaded resource. sh -c 'rm -f patroni.tar.gz' whitelist_externals = @@ -123,6 +128,7 @@ deps = pytest juju==2.9.11 # juju 3.0.0 has issues with retrieving action results landscape-api-py3 + mailmanclient pytest-operator psycopg2-binary -r{toxinidir}/requirements.txt @@ -141,6 +147,7 @@ deps = pytest juju==2.9.11 # juju 3.0.0 has issues with retrieving action results landscape-api-py3 + mailmanclient pytest-operator psycopg2-binary -r{toxinidir}/requirements.txt From 2d90e0339b20a8e67828dfc77a58cbc0eda125ab Mon Sep 17 00:00:00 2001 From: Marcelo Henrique Neppel Date: Sat, 27 Aug 2022 10:14:57 -0300 Subject: [PATCH 13/18] Add pytest marks --- tests/integration/test_password_rotation.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/integration/test_password_rotation.py b/tests/integration/test_password_rotation.py index 2f4e8ae4b5..f0811e10a1 100644 --- a/tests/integration/test_password_rotation.py +++ b/tests/integration/test_password_rotation.py @@ -17,8 +17,9 @@ APP_NAME = METADATA["name"] -@pytest.mark.skip_if_deployed @pytest.mark.abort_on_fail +@pytest.mark.password_rotation +@pytest.mark.skip_if_deployed async def test_deploy_active(ops_test: OpsTest): """Build the charm and deploy it.""" charm = await ops_test.build_charm(".") @@ -30,6 +31,7 @@ async def test_deploy_active(ops_test: OpsTest): await ops_test.model.wait_for_idle(apps=[APP_NAME], status="active", timeout=1000) +@pytest.mark.password_rotation async def test_password_rotation(ops_test: OpsTest): """Test password rotation action.""" # Get the initial passwords set for the system users. From 3f259d5903639f228ea0f7c7b1333ea666993aec Mon Sep 17 00:00:00 2001 From: Marcelo Henrique Neppel Date: Fri, 2 Sep 2022 08:38:14 -0300 Subject: [PATCH 14/18] Fix action description --- actions.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/actions.yaml b/actions.yaml index 9a0c23a845..54a0da8f89 100644 --- a/actions.yaml +++ b/actions.yaml @@ -4,7 +4,7 @@ get-primary: description: Get the unit which is the primary/leader in the replication. get-password: - description: Change the system user's password, which is used by charm. + description: Get the system user's password, which is used by charm. It is for internal charm users and SHOULD NOT be used by applications. params: username: From 56ff42a60fc49b5643fd6a4c42591956e69d057f Mon Sep 17 00:00:00 2001 From: Marcelo Henrique Neppel Date: Fri, 2 Sep 2022 08:40:51 -0300 Subject: [PATCH 15/18] Fix method docstring --- lib/charms/postgresql_k8s/v0/postgresql.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/charms/postgresql_k8s/v0/postgresql.py b/lib/charms/postgresql_k8s/v0/postgresql.py index 6e87561a5e..10b65132c6 100644 --- a/lib/charms/postgresql_k8s/v0/postgresql.py +++ b/lib/charms/postgresql_k8s/v0/postgresql.py @@ -203,9 +203,11 @@ def list_users(self) -> Set[str]: def update_user_password(self, username: str, password: str) -> None: """Update a user password. + Args: username: the user to update the password. password: the new password for the user. + Raises: PostgreSQLUpdateUserPasswordError if the password couldn't be changed. """ From 62cc60470877cdc751d0bcb0b7c6d21f4bf9d12c Mon Sep 17 00:00:00 2001 From: Marcelo Henrique Neppel Date: Fri, 2 Sep 2022 08:48:46 -0300 Subject: [PATCH 16/18] Fix pytest mark --- tests/integration/test_password_rotation.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/integration/test_password_rotation.py b/tests/integration/test_password_rotation.py index 5181e7f510..0dd04ef0da 100644 --- a/tests/integration/test_password_rotation.py +++ b/tests/integration/test_password_rotation.py @@ -18,7 +18,7 @@ @pytest.mark.abort_on_fail -@pytest.mark.password_rotation +@pytest.mark.password_rotation_tests @pytest.mark.skip_if_deployed async def test_deploy_active(ops_test: OpsTest): """Build the charm and deploy it.""" @@ -31,7 +31,7 @@ async def test_deploy_active(ops_test: OpsTest): await ops_test.model.wait_for_idle(apps=[APP_NAME], status="active", timeout=1000) -@pytest.mark.password_rotation +@pytest.mark.password_rotation_tests async def test_password_rotation(ops_test: OpsTest): """Test password rotation action.""" # Get the initial passwords set for the system users. From 63004b41a0ad05185940657dde1961ef99d83d8f Mon Sep 17 00:00:00 2001 From: Marcelo Henrique Neppel Date: Fri, 2 Sep 2022 18:39:22 -0300 Subject: [PATCH 17/18] Fix pytest markers --- .../new_relations/test_new_relations.py | 21 +++++++++---------- tests/integration/test_charm.py | 12 +++++------ tests/integration/test_db.py | 4 ++-- tests/integration/test_db_admin.py | 2 +- tests/integration/test_password_rotation.py | 4 ++-- tox.ini | 10 ++++----- 6 files changed, 26 insertions(+), 27 deletions(-) diff --git a/tests/integration/new_relations/test_new_relations.py b/tests/integration/new_relations/test_new_relations.py index 5bdc1dc51b..93a9b090ff 100644 --- a/tests/integration/new_relations/test_new_relations.py +++ b/tests/integration/new_relations/test_new_relations.py @@ -31,7 +31,7 @@ @pytest.mark.abort_on_fail -@pytest.mark.database_relation +@pytest.mark.database_relation_tests async def test_deploy_charms(ops_test: OpsTest, application_charm, database_charm): """Deploy both charms (application and database) to use in the tests.""" # Deploy both charms (multiple units for each application to test that later they correctly @@ -66,7 +66,7 @@ async def test_deploy_charms(ops_test: OpsTest, application_charm, database_char await ops_test.model.wait_for_idle(apps=APP_NAMES, status="active", timeout=3000) -@pytest.mark.database_relation +@pytest.mark.database_relation_tests async def test_no_read_only_endpoint_in_standalone_cluster(ops_test: OpsTest): """Test that there is no read-only endpoint in a standalone cluster.""" async with ops_test.fast_forward(): @@ -93,7 +93,7 @@ async def test_no_read_only_endpoint_in_standalone_cluster(ops_test: OpsTest): ) -@pytest.mark.database_relation +@pytest.mark.database_relation_tests async def test_read_only_endpoint_in_scaled_up_cluster(ops_test: OpsTest): """Test that there is read-only endpoint in a scaled up cluster.""" async with ops_test.fast_forward(): @@ -111,8 +111,7 @@ async def test_read_only_endpoint_in_scaled_up_cluster(ops_test: OpsTest): ) -@pytest.mark.abort_on_fail -@pytest.mark.database_relation +@pytest.mark.database_relation_tests async def test_database_relation_with_charm_libraries(ops_test: OpsTest): """Test basic functionality of database relation interface.""" # Get the connection string to connect to the database using the read/write endpoint. @@ -160,7 +159,7 @@ async def test_database_relation_with_charm_libraries(ops_test: OpsTest): cursor.execute("DROP TABLE test;") -@pytest.mark.database_relation +@pytest.mark.database_relation_tests 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. @@ -181,7 +180,7 @@ async def test_user_with_extra_roles(ops_test: OpsTest): connection.close() -@pytest.mark.database_relation +@pytest.mark.database_relation_tests async def test_two_applications_doesnt_share_the_same_relation_data( ops_test: OpsTest, application_charm ): @@ -216,7 +215,7 @@ async def test_two_applications_doesnt_share_the_same_relation_data( assert application_connection_string != another_application_connection_string -@pytest.mark.database_relation +@pytest.mark.database_relation_tests async def test_an_application_can_connect_to_multiple_database_clusters( ops_test: OpsTest, database_charm ): @@ -249,7 +248,7 @@ async def test_an_application_can_connect_to_multiple_database_clusters( assert application_connection_string != another_application_connection_string -@pytest.mark.database_relation +@pytest.mark.database_relation_tests async def test_an_application_can_connect_to_multiple_aliased_database_clusters( ops_test: OpsTest, database_charm ): @@ -285,7 +284,7 @@ async def test_an_application_can_connect_to_multiple_aliased_database_clusters( assert application_connection_string != another_application_connection_string -@pytest.mark.database_relation +@pytest.mark.database_relation_tests async def test_an_application_can_request_multiple_databases(ops_test: OpsTest, application_charm): """Test that an application can request additional databases using the same interface.""" # Relate the charms using another relation and wait for them exchanging some connection data. @@ -306,7 +305,7 @@ async def test_an_application_can_request_multiple_databases(ops_test: OpsTest, assert first_database_connection_string != second_database_connection_string -@pytest.mark.database_relation +@pytest.mark.database_relation_tests async def test_relation_data_is_updated_correctly_when_scaling(ops_test: OpsTest): """Test that relation data, like connection data, is updated correctly when scaling.""" # Retrieve the list of current database unit names. diff --git a/tests/integration/test_charm.py b/tests/integration/test_charm.py index 9e9f067f0f..d8c6531bdc 100644 --- a/tests/integration/test_charm.py +++ b/tests/integration/test_charm.py @@ -32,10 +32,10 @@ UNIT_IDS = [0, 1, 2] -@pytest.mark.skip_if_deployed @pytest.mark.abort_on_fail -@pytest.mark.charm +@pytest.mark.charm_tests @pytest.mark.parametrize("series", SERIES) +@pytest.mark.skip_if_deployed async def test_deploy(ops_test: OpsTest, charm: str, series: str): """Deploy the charm-under-test. @@ -60,7 +60,7 @@ async def test_deploy(ops_test: OpsTest, charm: str, series: str): @pytest.mark.abort_on_fail -@pytest.mark.charm +@pytest.mark.charm_tests @pytest.mark.parametrize("series", SERIES) @pytest.mark.parametrize("unit_id", UNIT_IDS) async def test_database_is_up(ops_test: OpsTest, series: str, unit_id: int): @@ -74,7 +74,7 @@ async def test_database_is_up(ops_test: OpsTest, series: str, unit_id: int): assert result.status_code == 200 -@pytest.mark.charm +@pytest.mark.charm_tests @pytest.mark.parametrize("series", SERIES) @pytest.mark.parametrize("unit_id", UNIT_IDS) async def test_settings_are_correct(ops_test: OpsTest, series: str, unit_id: int): @@ -124,7 +124,7 @@ async def test_settings_are_correct(ops_test: OpsTest, series: str, unit_id: int assert settings["maximum_lag_on_failover"] == 1048576 -@pytest.mark.charm +@pytest.mark.charm_tests @pytest.mark.parametrize("series", SERIES) async def test_scale_down_and_up(ops_test: OpsTest, series: str): """Test data is replicated to new units after a scale up.""" @@ -210,7 +210,7 @@ async def test_scale_down_and_up(ops_test: OpsTest, series: str): await scale_application(ops_test, application_name, initial_scale) -@pytest.mark.charm +@pytest.mark.charm_tests @pytest.mark.parametrize("series", SERIES) async def test_persist_data_through_primary_deletion(ops_test: OpsTest, series: str): """Test data persists through a primary deletion.""" diff --git a/tests/integration/test_db.py b/tests/integration/test_db.py index f92d82ad2a..7212d53086 100644 --- a/tests/integration/test_db.py +++ b/tests/integration/test_db.py @@ -24,7 +24,7 @@ RELATION_NAME = "db" -@pytest.mark.db_relation +@pytest.mark.db_relation_tests async def test_mailman3_core_db(ops_test: OpsTest, charm: str) -> None: """Deploy Mailman3 Core to test the 'db' relation.""" async with ops_test.fast_forward(): @@ -93,7 +93,7 @@ async def test_mailman3_core_db(ops_test: OpsTest, charm: str) -> None: assert domain_name not in [domain.mail_host for domain in client.domains] -@pytest.mark.db_relation +@pytest.mark.db_relation_tests async def test_relation_data_is_updated_correctly_when_scaling(ops_test: OpsTest): """Test that relation data, like connection data, is updated correctly when scaling.""" # Retrieve the list of current database unit names. diff --git a/tests/integration/test_db_admin.py b/tests/integration/test_db_admin.py index 1964537424..d8b29fb2ca 100644 --- a/tests/integration/test_db_admin.py +++ b/tests/integration/test_db_admin.py @@ -25,7 +25,7 @@ DATABASE_UNITS = 3 -@pytest.mark.db_admin_relation +@pytest.mark.db_admin_relation_tests async def test_landscape_scalable_bundle_db(ops_test: OpsTest, charm: str) -> None: """Deploy Landscape Scalable Bundle to test the 'db-admin' relation.""" config = { diff --git a/tests/integration/test_password_rotation.py b/tests/integration/test_password_rotation.py index f0811e10a1..ef3e84555e 100644 --- a/tests/integration/test_password_rotation.py +++ b/tests/integration/test_password_rotation.py @@ -18,7 +18,7 @@ @pytest.mark.abort_on_fail -@pytest.mark.password_rotation +@pytest.mark.password_rotation_tests @pytest.mark.skip_if_deployed async def test_deploy_active(ops_test: OpsTest): """Build the charm and deploy it.""" @@ -31,7 +31,7 @@ async def test_deploy_active(ops_test: OpsTest): await ops_test.model.wait_for_idle(apps=[APP_NAME], status="active", timeout=1000) -@pytest.mark.password_rotation +@pytest.mark.password_rotation_tests async def test_password_rotation(ops_test: OpsTest): """Test password rotation action.""" # Get the initial passwords set for the system users. diff --git a/tox.ini b/tox.ini index 6c123cda2a..c1d1bd1f97 100644 --- a/tox.ini +++ b/tox.ini @@ -78,7 +78,7 @@ deps = commands = # Download patroni resource to use in the charm deployment. sh -c 'stat patroni.tar.gz > /dev/null 2>&1 || curl "https://github.com/zalando/patroni/archive/refs/tags/v2.1.3.tar.gz" -L -s > patroni.tar.gz' - pytest -v --tb native --ignore={[vars]tst_path}unit --log-cli-level=INFO -s {posargs} -m charm + pytest -v --tb native --ignore={[vars]tst_path}unit --log-cli-level=INFO -s {posargs} -m charm_tests # Remove the downloaded resource. sh -c 'rm -f patroni.tar.gz' whitelist_externals = @@ -97,7 +97,7 @@ deps = commands = # Download patroni resource to use in the charm deployment. sh -c 'stat patroni.tar.gz > /dev/null 2>&1 || curl "https://github.com/zalando/patroni/archive/refs/tags/v2.1.3.tar.gz" -L -s > patroni.tar.gz' - pytest -v --tb native --ignore={[vars]tst_path}unit --log-cli-level=INFO -s {posargs} -m database_relation + pytest -v --tb native --ignore={[vars]tst_path}unit --log-cli-level=INFO -s {posargs} -m database_relation_tests # Remove the downloaded resource. sh -c 'rm -f patroni.tar.gz' whitelist_externals = @@ -116,7 +116,7 @@ deps = commands = # Download patroni resource to use in the charm deployment. sh -c 'stat patroni.tar.gz > /dev/null 2>&1 || curl "https://github.com/zalando/patroni/archive/refs/tags/v2.1.3.tar.gz" -L -s > patroni.tar.gz' - pytest -v --tb native --ignore={[vars]tst_path}unit --log-cli-level=INFO -s {posargs} -m db_relation + pytest -v --tb native --ignore={[vars]tst_path}unit --log-cli-level=INFO -s {posargs} -m db_relation_tests # Remove the downloaded resource. sh -c 'rm -f patroni.tar.gz' whitelist_externals = @@ -135,7 +135,7 @@ deps = commands = # Download patroni resource to use in the charm deployment. sh -c 'stat patroni.tar.gz > /dev/null 2>&1 || curl "https://github.com/zalando/patroni/archive/refs/tags/v2.1.3.tar.gz" -L -s > patroni.tar.gz' - pytest -v --tb native --ignore={[vars]tst_path}unit --log-cli-level=INFO -s {posargs} -m db_admin_relation + pytest -v --tb native --ignore={[vars]tst_path}unit --log-cli-level=INFO -s {posargs} -m db_admin_relation_tests # Remove the downloaded resource. sh -c 'rm -f patroni.tar.gz' whitelist_externals = @@ -154,7 +154,7 @@ deps = commands = # Download patroni resource to use in the charm deployment. sh -c 'stat patroni.tar.gz > /dev/null 2>&1 || curl "https://github.com/zalando/patroni/archive/refs/tags/v2.1.3.tar.gz" -L -s > patroni.tar.gz' - pytest -v --tb native --ignore={[vars]tst_path}unit --log-cli-level=INFO -s {posargs} -m password_rotation + pytest -v --tb native --ignore={[vars]tst_path}unit --log-cli-level=INFO -s {posargs} -m password_rotation_tests # Remove the downloaded resource. sh -c 'rm -f patroni.tar.gz' whitelist_externals = From 42a5b13116a59f2034d967b9d8d32fbc46be2113 Mon Sep 17 00:00:00 2001 From: Marcelo Henrique Neppel Date: Sat, 3 Sep 2022 10:16:46 -0300 Subject: [PATCH 18/18] Register pytest markers --- pyproject.toml | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/pyproject.toml b/pyproject.toml index f974d2ae79..d873e96cb4 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -12,6 +12,13 @@ show_missing = true minversion = "6.0" log_cli_level = "INFO" asyncio_mode = "auto" +markers = [ + "charm_tests", + "database_relation_tests", + "db_relation_tests", + "db_admin_relation_tests", + "password_rotation_tests", +] # Formatting tools configuration [tool.black]