From eb2b8d1fdcecd49653924d58e464faabb3bb2282 Mon Sep 17 00:00:00 2001 From: Dragomir Penev <6687393+dragomirp@users.noreply.github.com> Date: Mon, 30 Sep 2024 19:29:49 +0300 Subject: [PATCH] [DPE-5564] Don't set secrets until db is set (#373) * Don't set secrets until db is set * Hard set test rel id --- .../data_platform_libs/v0/data_interfaces.py | 21 ++++++++++++++++++- src/charm.py | 2 +- src/relations/pgbouncer_provider.py | 2 ++ .../unit/relations/test_pgbouncer_provider.py | 11 ++++++++-- 4 files changed, 32 insertions(+), 4 deletions(-) diff --git a/lib/charms/data_platform_libs/v0/data_interfaces.py b/lib/charms/data_platform_libs/v0/data_interfaces.py index aaed2e528..3bc2dd850 100644 --- a/lib/charms/data_platform_libs/v0/data_interfaces.py +++ b/lib/charms/data_platform_libs/v0/data_interfaces.py @@ -331,7 +331,7 @@ def _on_topic_requested(self, event: TopicRequestedEvent): # Increment this PATCH version before using `charmcraft publish-lib` or reset # to 0 if you are raising the major API version -LIBPATCH = 39 +LIBPATCH = 40 PYDEPS = ["ops>=2.0.0"] @@ -391,6 +391,10 @@ class IllegalOperationError(DataInterfacesError): """To be used when an operation is not allowed to be performed.""" +class PrematureDataAccessError(DataInterfacesError): + """To be raised when the Relation Data may be accessed (written) before protocol init complete.""" + + ############################################################################## # Global helpers / utilities ############################################################################## @@ -1453,6 +1457,8 @@ def _on_relation_changed_event(self, event: RelationChangedEvent) -> None: class ProviderData(Data): """Base provides-side of the data products relation.""" + RESOURCE_FIELD = "database" + def __init__( self, model: Model, @@ -1618,6 +1624,15 @@ def _fetch_my_specific_relation_data( def _update_relation_data(self, relation: Relation, data: Dict[str, str]) -> None: """Set values for fields not caring whether it's a secret or not.""" req_secret_fields = [] + + keys = set(data.keys()) + if self.fetch_relation_field(relation.id, self.RESOURCE_FIELD) is None and ( + keys - {"endpoints", "read-only-endpoints", "replset"} + ): + raise PrematureDataAccessError( + "Premature access to relation data, update is forbidden before the connection is initialized." + ) + if relation.app: req_secret_fields = get_encoded_list(relation, relation.app, REQ_SECRET_FIELDS) @@ -3290,6 +3305,8 @@ class KafkaRequiresEvents(CharmEvents): class KafkaProviderData(ProviderData): """Provider-side of the Kafka relation.""" + RESOURCE_FIELD = "topic" + def __init__(self, model: Model, relation_name: str) -> None: super().__init__(model, relation_name) @@ -3539,6 +3556,8 @@ class OpenSearchRequiresEvents(CharmEvents): class OpenSearchProvidesData(ProviderData): """Provider-side of the OpenSearch relation.""" + RESOURCE_FIELD = "index" + def __init__(self, model: Model, relation_name: str) -> None: super().__init__(model, relation_name) diff --git a/src/charm.py b/src/charm.py index 17c068656..7915fc012 100755 --- a/src/charm.py +++ b/src/charm.py @@ -34,8 +34,8 @@ Relation, StartEvent, WaitingStatus, + main, ) -from ops.main import main from config import CharmConfig from constants import ( diff --git a/src/relations/pgbouncer_provider.py b/src/relations/pgbouncer_provider.py index ba456661a..cd21ffccb 100644 --- a/src/relations/pgbouncer_provider.py +++ b/src/relations/pgbouncer_provider.py @@ -238,6 +238,8 @@ def update_connection_info(self, relation): database = rel_data.get("database") user = f"relation_id_{relation.id}" password = self.database_provides.fetch_my_relation_field(relation.id, "password") + if not database or not password: + return host = "localhost" uri_host = host diff --git a/tests/unit/relations/test_pgbouncer_provider.py b/tests/unit/relations/test_pgbouncer_provider.py index eb5795b18..9dacd3a2d 100644 --- a/tests/unit/relations/test_pgbouncer_provider.py +++ b/tests/unit/relations/test_pgbouncer_provider.py @@ -65,12 +65,17 @@ def setUp(self): @patch("charms.data_platform_libs.v0.data_interfaces.DatabaseProvides.set_credentials") @patch("charms.data_platform_libs.v0.data_interfaces.DatabaseProvides.set_endpoints") @patch("charms.data_platform_libs.v0.data_interfaces.DatabaseProvides.set_version") + @patch( + "charms.data_platform_libs.v0.data_interfaces.DatabaseProvides.fetch_my_relation_field", + return_value="test_pass", + ) @patch("charm.PgBouncerCharm.set_relation_databases") @patch("charm.PgBouncerCharm.generate_relation_databases") def test_on_database_requested( self, _gen_rel_dbs, _set_rel_dbs, + _dbp_fetch_my_relation_field, _dbp_set_version, _dbp_set_endpoints, _dbp_set_credentials, @@ -88,11 +93,13 @@ def test_on_database_requested( _gen_rel_dbs.return_value = {} event = MagicMock() - rel_id = event.relation.id = 1 + rel_id = event.relation.id = self.client_rel_id database = event.database = "test-db" event.extra_user_roles = "SUPERUSER" event.external_node_connectivity = False user = f"relation_id_{rel_id}" + with self.harness.hooks_disabled(): + self.harness.update_relation_data(rel_id, "application", {"database": "test-db"}) # check we exit immediately if backend doesn't exist. _check_backend.return_value = False @@ -126,7 +133,7 @@ def test_on_database_requested( rel_id, f"localhost:{self.charm.config['listen_port']}" ) _set_rel_dbs.assert_called_once_with({ - "1": {"name": "test-db", "legacy": False}, + str(rel_id): {"name": "test-db", "legacy": False}, "*": {"name": "*", "auth_dbname": "test-db"}, }) _render_pgb_config.assert_called_once_with(reload_pgbouncer=True)