Skip to content

Commit

Permalink
[DPE-5306] Data Interafces v40 (#615)
Browse files Browse the repository at this point in the history
* Data Interafces v40

* Adjustments on unittests

* Fix on charm code not to publish before relation is fully established

* Don't set uri if no database or password

* Unit test

---------

Co-authored-by: Dragomir Penev <[email protected]>
  • Loading branch information
juditnovak and dragomirp authored Oct 10, 2024
1 parent 7ef0f2a commit a3221f8
Show file tree
Hide file tree
Showing 3 changed files with 42 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."""

DATABASE_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.DATABASE_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."""

DATABASE_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."""

DATABASE_FIELD = "index"

def __init__(self, model: Model, relation_name: str) -> None:
super().__init__(model, relation_name)

Expand Down
2 changes: 2 additions & 0 deletions src/relations/postgresql_provider.py
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,8 @@ def update_endpoints(self, event: DatabaseRequestedEvent = None) -> None:
user = f"relation-{relation_id}"
database = rel_data[relation_id].get("database")
password = secret_data.get(relation_id, {}).get("password")
if not database or not password:
continue

# Set the read/write endpoint.
self.database_provides.set_endpoints(
Expand Down
23 changes: 20 additions & 3 deletions tests/unit/test_postgresql_provider.py
Original file line number Diff line number Diff line change
Expand Up @@ -281,18 +281,35 @@ def test_update_endpoints_without_event(harness):
"relations.postgresql_provider.DatabaseProvides.fetch_my_relation_data"
) as _fetch_my_relation_data,
):
_fetch_my_relation_data.return_value.get().get.return_value = "test_password"

# Mock the members_ips list to simulate different scenarios
# (with and without a replica).
_members_ips.side_effect = [{"1.1.1.1", "2.2.2.2"}, {"1.1.1.1"}]
_members_ips.side_effect = [{"1.1.1.1", "2.2.2.2"}, {"1.1.1.1", "2.2.2.2"}, {"1.1.1.1"}]
rel_id = harness.model.get_relation(RELATION_NAME).id

# Don't set data if no password
_fetch_my_relation_data.return_value.get().get.return_value = None

harness.charm.postgresql_client_relation.update_endpoints()
assert harness.get_relation_data(rel_id, harness.charm.app.name) == {}

_fetch_my_relation_data.reset_mock()
_fetch_my_relation_data.return_value.get().get.return_value = "test_password"

# Add two different relations.
rel_id = harness.add_relation(RELATION_NAME, "application")
another_rel_id = harness.add_relation(RELATION_NAME, "application")

relation_ids = [rel.id for rel in harness.charm.model.relations[RELATION_NAME]]
other_rel_ids = set(relation_ids) - set({rel_id, another_rel_id})

with harness.hooks_disabled():
for relation_id in other_rel_ids:
harness.update_relation_data(
relation_id,
"application",
{"database": "some_db", "extra-user-roles": ""},
)

harness.update_relation_data(
rel_id,
"application",
Expand Down

0 comments on commit a3221f8

Please sign in to comment.