Skip to content

Commit

Permalink
fix: Do not integrate an invalid relation (#350)
Browse files Browse the repository at this point in the history
  • Loading branch information
Gu1nness authored Oct 16, 2024
1 parent f02e1fe commit 5f86ac0
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 4 deletions.
24 changes: 21 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 = 13
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 Expand Up @@ -372,6 +389,7 @@ def _get_config(
mongo_args["port"] = Config.MONGOS_PORT
if self.substrate == Config.Substrate.K8S:
mongo_args["hosts"] = self.charm.get_mongos_hosts_for_client()
mongo_args["port"] = self.charm.get_mongos_port()
else:
mongo_args["replset"] = self.charm.app.name

Expand Down
5 changes: 4 additions & 1 deletion tests/unit/test_mongodb_provider.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,12 @@ def setUp(self, *unused):
self.charm = self.harness.charm
self.addCleanup(self.harness.cleanup)

@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 5f86ac0

Please sign in to comment.