Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Do not integrate an invalid relation #350

Merged
merged 2 commits into from
Oct 16, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading