Skip to content

Commit

Permalink
fix: Do not integrate an invalid relation (#509)
Browse files Browse the repository at this point in the history
  • Loading branch information
Gu1nness authored Oct 15, 2024
1 parent 6ee2ad3 commit 33eb4b4
Show file tree
Hide file tree
Showing 2 changed files with 45 additions and 4 deletions.
23 changes: 20 additions & 3 deletions lib/charms/mongodb/v1/mongodb_provider.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,13 @@
from charms.data_platform_libs.v0.data_interfaces import DatabaseProvides
from charms.mongodb.v0.mongo import MongoConfiguration, MongoConnection
from charms.mongodb.v1.helpers import generate_password
from ops.charm import CharmBase, EventBase, RelationBrokenEvent, RelationChangedEvent
from ops.charm import (
CharmBase,
EventBase,
RelationBrokenEvent,
RelationChangedEvent,
RelationEvent,
)
from ops.framework import Object
from ops.model import Relation
from pymongo.errors import PyMongoError
Expand All @@ -31,7 +37,7 @@

# Increment this PATCH version before using `charmcraft publish-lib` or reset
# to 0 if you are raising the major API version
LIBPATCH = 14
LIBPATCH = 15

logger = logging.getLogger(__name__)
REL_NAME = "database"
Expand Down Expand Up @@ -88,6 +94,11 @@ def pass_sanity_hook_checks(self) -> bool:
if not self.charm.db_initialised:
return False

# Warning: the sanity_hook_checks can pass when the call is
# issued by a config-sever because the config-server is allowed to manage the users
# in MongoDB. This is not well named and does not protect integration of a config-server
# to a client application. The mongos charm however doesn't care
# because it supports only one integration that uses MongoDBProvider.
if not self.charm.is_role(Config.Role.MONGOS) and not self.charm.is_relation_feasible(
self.get_relation_name()
):
Expand All @@ -99,8 +110,14 @@ def pass_sanity_hook_checks(self) -> bool:

return True

def pass_hook_checks(self, event: EventBase) -> bool:
def pass_hook_checks(self, event: RelationEvent) -> bool:
"""Runs the pre-hooks checks for MongoDBProvider, returns True if all pass."""
# First, ensure that the relation is valid, useless to do anything else otherwise
if not self.charm.is_role(Config.Role.MONGOS) and not self.charm.is_relation_feasible(
event.relation.name
):
return False

if not self.pass_sanity_hook_checks():
return False

Expand Down
26 changes: 25 additions & 1 deletion tests/unit/test_mongodb_provider.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
from unittest import mock
from unittest.mock import patch

from ops import BlockedStatus
from ops.charm import RelationEvent
from ops.testing import Harness
from parameterized import parameterized
Expand Down Expand Up @@ -36,9 +37,32 @@ def setUp(self, *unused):
self.charm = self.harness.charm
self.addCleanup(self.harness.cleanup)

@parameterized.expand([["config-server"], ["shard"]])
@patch("charms.mongodb.v0.set_status.get_charm_revision")
@patch("charm.MongoDBProvider.oversee_users")
def test_relation_event_relation_not_feasible(self, role: str, oversee_users, *unused):
"""Tests that relating with a wrong role sets a blocked status."""

def is_config_server_role(role_name: str):
return role_name == role

self.harness.charm.is_role = is_config_server_role

relation_id = self.harness.add_relation("database", "consumer")
self.harness.add_relation_unit(relation_id, "consumer/0")
self.harness.update_relation_data(relation_id, "consumer/0", PEER_ADDR)

assert self.harness.charm.unit.status == BlockedStatus(
"Sharding roles do not support database interface."
)
oversee_users.assert_not_called()

@patch("charms.mongodb.v0.set_status.get_charm_revision")
@patch("charm.CrossAppVersionChecker.is_local_charm")
@patch("charm.CrossAppVersionChecker.is_integrated_to_locally_built_charm")
@patch("ops.framework.EventBase.defer")
@patch("charm.MongoDBProvider.oversee_users")
def test_relation_event_db_not_initialised(self, oversee_users, defer):
def test_relation_event_db_not_initialised(self, oversee_users, defer, *unused):
"""Tests no database relations are handled until the database is initialised.
Users should not be "overseen" until the database has been initialised, no matter the
Expand Down

0 comments on commit 33eb4b4

Please sign in to comment.