Skip to content

Commit

Permalink
[DPE-5564] Don't set secrets until db is set (#373)
Browse files Browse the repository at this point in the history
* Don't set secrets until db is set

* Hard set test rel id
  • Loading branch information
dragomirp authored Sep 30, 2024
1 parent 3d01053 commit eb2b8d1
Show file tree
Hide file tree
Showing 4 changed files with 32 additions and 4 deletions.
21 changes: 20 additions & 1 deletion lib/charms/data_platform_libs/v0/data_interfaces.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"]

Expand Down Expand Up @@ -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
##############################################################################
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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)

Expand Down Expand Up @@ -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)

Expand Down Expand Up @@ -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)

Expand Down
2 changes: 1 addition & 1 deletion src/charm.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,8 @@
Relation,
StartEvent,
WaitingStatus,
main,
)
from ops.main import main

from config import CharmConfig
from constants import (
Expand Down
2 changes: 2 additions & 0 deletions src/relations/pgbouncer_provider.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
11 changes: 9 additions & 2 deletions tests/unit/relations/test_pgbouncer_provider.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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
Expand Down Expand Up @@ -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)
Expand Down

0 comments on commit eb2b8d1

Please sign in to comment.