diff --git a/lib/charms/data_platform_libs/v0/data_interfaces.py b/lib/charms/data_platform_libs/v0/data_interfaces.py index 714eace4..a266a748 100644 --- a/lib/charms/data_platform_libs/v0/data_interfaces.py +++ b/lib/charms/data_platform_libs/v0/data_interfaces.py @@ -320,7 +320,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 = 24 +LIBPATCH = 28 PYDEPS = ["ops>=2.0.0"] @@ -347,16 +347,6 @@ class SecretGroup(Enum): EXTRA = "extra" -# Local map to associate mappings with secrets potentially as a group -SECRET_LABEL_MAP = { - "username": SecretGroup.USER, - "password": SecretGroup.USER, - "uris": SecretGroup.USER, - "tls": SecretGroup.TLS, - "tls-ca": SecretGroup.TLS, -} - - class DataInterfacesError(Exception): """Common ancestor for DataInterfaces related exceptions.""" @@ -432,15 +422,15 @@ def diff(event: RelationChangedEvent, bucket: Union[Unit, Application]) -> Diff: ) # These are the keys that were added to the databag and triggered this event. - added = new_data.keys() - old_data.keys() # pyright: ignore [reportGeneralTypeIssues] + added = new_data.keys() - old_data.keys() # pyright: ignore [reportAssignmentType] # These are the keys that were removed from the databag and triggered this event. - deleted = old_data.keys() - new_data.keys() # pyright: ignore [reportGeneralTypeIssues] + deleted = old_data.keys() - new_data.keys() # pyright: ignore [reportAssignmentType] # These are the keys that already existed in the databag, # but had their values changed. changed = { key - for key in old_data.keys() & new_data.keys() # pyright: ignore [reportGeneralTypeIssues] - if old_data[key] != new_data[key] # pyright: ignore [reportGeneralTypeIssues] + for key in old_data.keys() & new_data.keys() # pyright: ignore [reportAssignmentType] + if old_data[key] != new_data[key] # pyright: ignore [reportAssignmentType] } # Convert the new_data to a serializable format and save it for a next diff check. set_encoded_field(event.relation, bucket, "data", new_data) @@ -453,7 +443,7 @@ def leader_only(f): """Decorator to ensure that only leader can perform given operation.""" def wrapper(self, *args, **kwargs): - if not self.local_unit.is_leader(): + if self.component == self.local_app and not self.local_unit.is_leader(): logger.error( "This operation (%s()) can only be performed by the leader unit", f.__name__ ) @@ -487,12 +477,19 @@ class CachedSecret: The data structure is precisely re-using/simulating as in the actual Secret Storage """ - def __init__(self, charm: CharmBase, label: str, secret_uri: Optional[str] = None): + def __init__( + self, + charm: CharmBase, + component: Union[Application, Unit], + label: str, + secret_uri: Optional[str] = None, + ): self._secret_meta = None self._secret_content = {} self._secret_uri = secret_uri self.label = label self.charm = charm + self.component = component def add_secret(self, content: Dict[str, str], relation: Relation) -> Secret: """Create a new secret.""" @@ -501,8 +498,10 @@ def add_secret(self, content: Dict[str, str], relation: Relation) -> Secret: "Secret is already defined with uri %s", self._secret_uri ) - secret = self.charm.app.add_secret(content, label=self.label) - secret.grant(relation) + secret = self.component.add_secret(content, label=self.label) + if relation.app != self.charm.app: + # If it's not a peer relation, grant is to be applied + secret.grant(relation) self._secret_uri = secret.id self._secret_meta = secret return self._secret_meta @@ -531,8 +530,13 @@ def get_content(self) -> Dict[str, str]: except (ValueError, ModelError) as err: # https://bugs.launchpad.net/juju/+bug/2042596 # Only triggered when 'refresh' is set - msg = "ERROR either URI or label should be used for getting an owned secret but not both" - if isinstance(err, ModelError) and msg not in str(err): + known_model_errors = [ + "ERROR either URI or label should be used for getting an owned secret but not both", + "ERROR secret owner cannot use --refresh", + ] + if isinstance(err, ModelError) and not any( + msg in str(err) for msg in known_model_errors + ): raise # Due to: ValueError: Secret owner cannot use refresh=True self._secret_content = self.meta.get_content() @@ -558,14 +562,15 @@ def get_info(self) -> Optional[SecretInfo]: class SecretCache: """A data structure storing CachedSecret objects.""" - def __init__(self, charm): + def __init__(self, charm: CharmBase, component: Union[Application, Unit]): self.charm = charm + self.component = component self._secrets: Dict[str, CachedSecret] = {} def get(self, label: str, uri: Optional[str] = None) -> Optional[CachedSecret]: """Getting a secret from Juju Secret store or cache.""" if not self._secrets.get(label): - secret = CachedSecret(self.charm, label, uri) + secret = CachedSecret(self.charm, self.component, label, uri) if secret.meta: self._secrets[label] = secret return self._secrets.get(label) @@ -575,7 +580,7 @@ def add(self, label: str, content: Dict[str, str], relation: Relation) -> Cached if self._secrets.get(label): raise SecretAlreadyExistsError(f"Secret {label} already exists") - secret = CachedSecret(self.charm, label) + secret = CachedSecret(self.charm, self.component, label) secret.add_secret(content, relation) self._secrets[label] = secret return self._secrets[label] @@ -587,6 +592,17 @@ def add(self, label: str, content: Dict[str, str], relation: Relation) -> Cached class DataRelation(Object, ABC): """Base relation data mainpulation (abstract) class.""" + SCOPE = Scope.APP + + # Local map to associate mappings with secrets potentially as a group + SECRET_LABEL_MAP = { + "username": SecretGroup.USER, + "password": SecretGroup.USER, + "uris": SecretGroup.USER, + "tls": SecretGroup.TLS, + "tls-ca": SecretGroup.TLS, + } + def __init__(self, charm: CharmBase, relation_name: str) -> None: super().__init__(charm, relation_name) self.charm = charm @@ -598,7 +614,8 @@ def __init__(self, charm: CharmBase, relation_name: str) -> None: self._on_relation_changed_event, ) self._jujuversion = None - self.secrets = SecretCache(self.charm) + self.component = self.local_app if self.SCOPE == Scope.APP else self.local_unit + self.secrets = SecretCache(self.charm, self.component) @property def relations(self) -> List[Relation]: @@ -677,8 +694,7 @@ def _generate_secret_label( """Generate unique group_mappings for secrets within a relation context.""" return f"{relation_name}.{relation_id}.{group_mapping.value}.secret" - @staticmethod - def _generate_secret_field_name(group_mapping: SecretGroup) -> str: + def _generate_secret_field_name(self, group_mapping: SecretGroup) -> str: """Generate unique group_mappings for secrets within a relation context.""" return f"{PROV_SECRET_PREFIX}{group_mapping.value}" @@ -705,8 +721,8 @@ def _relation_from_secret_label(self, secret_label: str) -> Optional[Relation]: except ModelError: return - @staticmethod - def _group_secret_fields(secret_fields: List[str]) -> Dict[SecretGroup, List[str]]: + @classmethod + def _group_secret_fields(cls, secret_fields: List[str]) -> Dict[SecretGroup, List[str]]: """Helper function to arrange secret mappings under their group. NOTE: All unrecognized items end up in the 'extra' secret bucket. @@ -714,7 +730,7 @@ def _group_secret_fields(secret_fields: List[str]) -> Dict[SecretGroup, List[str """ secret_fieldnames_grouped = {} for key in secret_fields: - if group := SECRET_LABEL_MAP.get(key): + if group := cls.SECRET_LABEL_MAP.get(key): secret_fieldnames_grouped.setdefault(group, []).append(key) else: secret_fieldnames_grouped.setdefault(SecretGroup.EXTRA, []).append(key) @@ -736,22 +752,22 @@ def _get_group_secret_contents( return {k: v for k, v in secret_data.items() if k in secret_fields} return {} - @staticmethod + @classmethod def _content_for_secret_group( - content: Dict[str, str], secret_fields: Set[str], group_mapping: SecretGroup + cls, content: Dict[str, str], secret_fields: Set[str], group_mapping: SecretGroup ) -> Dict[str, str]: """Select : pairs from input, that belong to this particular Secret group.""" if group_mapping == SecretGroup.EXTRA: return { k: v for k, v in content.items() - if k in secret_fields and k not in SECRET_LABEL_MAP.keys() + if k in secret_fields and k not in cls.SECRET_LABEL_MAP.keys() } return { k: v for k, v in content.items() - if k in secret_fields and SECRET_LABEL_MAP.get(k) == group_mapping + if k in secret_fields and cls.SECRET_LABEL_MAP.get(k) == group_mapping } @juju_secrets_only @@ -784,7 +800,7 @@ def _process_secret_fields( fallback_to_databag = ( req_secret_fields and self.local_unit.is_leader() - and set(req_secret_fields) & set(relation.data[self.local_app]) + and set(req_secret_fields) & set(relation.data[self.component]) ) normal_fields = set(impacted_rel_fields) @@ -807,7 +823,7 @@ def _process_secret_fields( return (result, normal_fields) def _fetch_relation_data_without_secrets( - self, app: Application, relation: Relation, fields: Optional[List[str]] + self, component: Union[Application, Unit], relation: Relation, fields: Optional[List[str]] ) -> Dict[str, str]: """Fetching databag contents when no secrets are involved. @@ -816,17 +832,19 @@ def _fetch_relation_data_without_secrets( This is used typically when the Provides side wants to read the Requires side's data, or when the Requires side may want to read its own data. """ - if app not in relation.data or not relation.data[app]: + if component not in relation.data or not relation.data[component]: return {} if fields: - return {k: relation.data[app][k] for k in fields if k in relation.data[app]} + return { + k: relation.data[component][k] for k in fields if k in relation.data[component] + } else: - return dict(relation.data[app]) + return dict(relation.data[component]) def _fetch_relation_data_with_secrets( self, - app: Application, + component: Union[Application, Unit], req_secret_fields: Optional[List[str]], relation: Relation, fields: Optional[List[str]] = None, @@ -842,10 +860,10 @@ def _fetch_relation_data_with_secrets( normal_fields = [] if not fields: - if app not in relation.data or not relation.data[app]: + if component not in relation.data or not relation.data[component]: return {} - all_fields = list(relation.data[app].keys()) + all_fields = list(relation.data[component].keys()) normal_fields = [field for field in all_fields if not self._is_secret_field(field)] # There must have been secrets there @@ -862,38 +880,35 @@ def _fetch_relation_data_with_secrets( # (Typically when Juju3 Requires meets Juju2 Provides) if normal_fields: result.update( - self._fetch_relation_data_without_secrets(app, relation, list(normal_fields)) + self._fetch_relation_data_without_secrets(component, relation, list(normal_fields)) ) return result def _update_relation_data_without_secrets( - self, app: Application, relation: Relation, data: Dict[str, str] + self, component: Union[Application, Unit], relation: Relation, data: Dict[str, str] ) -> None: """Updating databag contents when no secrets are involved.""" - if app not in relation.data or relation.data[app] is None: + if component not in relation.data or relation.data[component] is None: return - if any(self._is_secret_field(key) for key in data.keys()): - raise SecretsIllegalUpdateError("Can't update secret {key}.") - if relation: - relation.data[app].update(data) + relation.data[component].update(data) def _delete_relation_data_without_secrets( - self, app: Application, relation: Relation, fields: List[str] + self, component: Union[Application, Unit], relation: Relation, fields: List[str] ) -> None: """Remove databag fields 'fields' from Relation.""" - if app not in relation.data or not relation.data[app]: + if component not in relation.data or relation.data[component] is None: return for field in fields: try: - relation.data[app].pop(field) + relation.data[component].pop(field) except KeyError: - logger.debug( - "Non-existing field was attempted to be removed from the databag %s, %s", - str(relation.id), + logger.error( + "Non-existing field '%s' was attempted to be removed from the databag (relation ID: %s)", str(field), + str(relation.id), ) pass @@ -954,7 +969,6 @@ def fetch_relation_field( .get(field) ) - @leader_only def fetch_my_relation_data( self, relation_ids: Optional[List[int]] = None, @@ -983,7 +997,6 @@ def fetch_my_relation_data( data[relation.id] = self._fetch_my_specific_relation_data(relation, fields) return data - @leader_only def fetch_my_relation_field( self, relation_id: int, field: str, relation_name: Optional[str] = None ) -> Optional[str]: @@ -1035,27 +1048,38 @@ def _diff(self, event: RelationChangedEvent) -> Diff: @juju_secrets_only def _add_relation_secret( - self, relation: Relation, content: Dict[str, str], group_mapping: SecretGroup + self, + relation: Relation, + group_mapping: SecretGroup, + secret_fields: Set[str], + data: Dict[str, str], + uri_to_databag=True, ) -> bool: """Add a new Juju Secret that will be registered in the relation databag.""" secret_field = self._generate_secret_field_name(group_mapping) - if relation.data[self.local_app].get(secret_field): + if uri_to_databag and relation.data[self.component].get(secret_field): logging.error("Secret for relation %s already exists, not adding again", relation.id) return False + content = self._content_for_secret_group(data, secret_fields, group_mapping) + label = self._generate_secret_label(self.relation_name, relation.id, group_mapping) secret = self.secrets.add(label, content, relation) # According to lint we may not have a Secret ID - if secret.meta and secret.meta.id: - relation.data[self.local_app][secret_field] = secret.meta.id + if uri_to_databag and secret.meta and secret.meta.id: + relation.data[self.component][secret_field] = secret.meta.id # Return the content that was added return True @juju_secrets_only def _update_relation_secret( - self, relation: Relation, content: Dict[str, str], group_mapping: SecretGroup + self, + relation: Relation, + group_mapping: SecretGroup, + secret_fields: Set[str], + data: Dict[str, str], ) -> bool: """Update the contents of an existing Juju Secret, referred in the relation databag.""" secret = self._get_relation_secret(relation.id, group_mapping) @@ -1064,6 +1088,8 @@ def _update_relation_secret( logging.error("Can't update secret for relation %s", relation.id) return False + content = self._content_for_secret_group(data, secret_fields, group_mapping) + old_content = secret.get_content() full_content = copy.deepcopy(old_content) full_content.update(content) @@ -1078,13 +1104,13 @@ def _add_or_update_relation_secrets( group: SecretGroup, secret_fields: Set[str], data: Dict[str, str], + uri_to_databag=True, ) -> bool: """Update contents for Secret group. If the Secret doesn't exist, create it.""" - secret_content = self._content_for_secret_group(data, secret_fields, group) if self._get_relation_secret(relation.id, group): - return self._update_relation_secret(relation, secret_content, group) + return self._update_relation_secret(relation, group, secret_fields, data) else: - return self._add_relation_secret(relation, secret_content, group) + return self._add_relation_secret(relation, group, secret_fields, data, uri_to_databag) @juju_secrets_only def _delete_relation_secret( @@ -1116,7 +1142,7 @@ def _delete_relation_secret( if not new_content: field = self._generate_secret_field_name(group) try: - relation.data[self.local_app].pop(field) + relation.data[self.component].pop(field) except KeyError: pass @@ -1233,6 +1259,11 @@ def set_tls_ca(self, relation_id: int, tls_ca: str) -> None: """ self.update_relation_data(relation_id, {"tls-ca": tls_ca}) + # Public functions -- inherited + + fetch_my_relation_data = leader_only(DataRelation.fetch_my_relation_data) + fetch_my_relation_field = leader_only(DataRelation.fetch_my_relation_field) + class DataRequires(DataRelation): """Requires-side of the relation.""" @@ -1297,7 +1328,7 @@ def _register_secret_to_relation( label = self._generate_secret_label(relation_name, relation_id, group) # Fetchin the Secret's meta information ensuring that it's locally getting registered with - CachedSecret(self.charm, label, secret_id).meta + CachedSecret(self.charm, self.component, label, secret_id).meta def _register_secrets_to_relation(self, relation: Relation, params_name_list: List[str]): """Make sure that secrets of the provided list are locally 'registered' from the databag. @@ -1426,6 +1457,220 @@ def _delete_relation_data(self, relation: Relation, fields: List[str]) -> None: """ return self._delete_relation_data_without_secrets(self.local_app, relation, fields) + # Public functions -- inherited + + fetch_my_relation_data = leader_only(DataRelation.fetch_my_relation_data) + fetch_my_relation_field = leader_only(DataRelation.fetch_my_relation_field) + + +# Base DataPeer + + +class DataPeer(DataRequires, DataProvides): + """Represents peer relations.""" + + SECRET_FIELDS = ["operator-password"] + SECRET_FIELD_NAME = "internal_secret" + SECRET_LABEL_MAP = {} + + def __init__( + self, + charm, + relation_name: str, + extra_user_roles: Optional[str] = None, + additional_secret_fields: Optional[List[str]] = [], + secret_field_name: Optional[str] = None, + deleted_label: Optional[str] = None, + ): + """Manager of base client relations.""" + DataRequires.__init__( + self, charm, relation_name, extra_user_roles, additional_secret_fields + ) + self.secret_field_name = secret_field_name if secret_field_name else self.SECRET_FIELD_NAME + self.deleted_label = deleted_label + + @property + def scope(self) -> Optional[Scope]: + """Turn component information into Scope.""" + if isinstance(self.component, Application): + return Scope.APP + if isinstance(self.component, Unit): + return Scope.UNIT + + def _on_relation_changed_event(self, event: RelationChangedEvent) -> None: + """Event emitted when the relation has changed.""" + pass + + def _on_secret_changed_event(self, event: SecretChangedEvent) -> None: + """Event emitted when the secret has changed.""" + pass + + def _generate_secret_label( + self, relation_name: str, relation_id: int, group_mapping: SecretGroup + ) -> str: + members = [self.charm.app.name] + if self.scope: + members.append(self.scope.value) + return f"{'.'.join(members)}" + + def _generate_secret_field_name(self, group_mapping: SecretGroup = SecretGroup.EXTRA) -> str: + """Generate unique group_mappings for secrets within a relation context.""" + return f"{self.secret_field_name}" + + @juju_secrets_only + def _get_relation_secret( + self, + relation_id: int, + group_mapping: SecretGroup = SecretGroup.EXTRA, + relation_name: Optional[str] = None, + ) -> Optional[CachedSecret]: + """Retrieve a Juju Secret specifically for peer relations. + + In case this code may be executed within a rolling upgrade, and we may need to + migrate secrets from the databag to labels, we make sure to stick the correct + label on the secret, and clean up the local databag. + """ + if not relation_name: + relation_name = self.relation_name + + relation = self.charm.model.get_relation(relation_name, relation_id) + if not relation: + return + + label = self._generate_secret_label(relation_name, relation_id, group_mapping) + secret_uri = relation.data[self.component].get(self._generate_secret_field_name(), None) + + # Fetching the secret with fallback to URI (in case label is not yet known) + # Label would we "stuck" on the secret in case it is found + secret = self.secrets.get(label, secret_uri) + + # Either app scope secret with leader executing, or unit scope secret + leader_or_unit_scope = self.component != self.local_app or self.local_unit.is_leader() + if secret_uri and secret and leader_or_unit_scope: + # Databag reference to the secret URI can be removed, now that it's labelled + relation.data[self.component].pop(self._generate_secret_field_name(), None) + return secret + + def _get_group_secret_contents( + self, + relation: Relation, + group: SecretGroup, + secret_fields: Optional[Union[Set[str], List[str]]] = None, + ) -> Dict[str, str]: + """Helper function to retrieve collective, requested contents of a secret.""" + result = super()._get_group_secret_contents(relation, group, secret_fields) + if not self.deleted_label: + return result + return {key: result[key] for key in result if result[key] != self.deleted_label} + + def _remove_secret_from_databag(self, relation, fields: List[str]) -> None: + """For Rolling Upgrades -- when moving from databag to secrets usage. + + Practically what happens here is to remove stuff from the databag that is + to be stored in secrets. + """ + if not self.secret_fields: + return + + secret_fields_passed = set(self.secret_fields) & set(fields) + for field in secret_fields_passed: + if self._fetch_relation_data_without_secrets(self.component, relation, [field]): + self._delete_relation_data_without_secrets(self.component, relation, [field]) + + def _fetch_specific_relation_data( + self, relation: Relation, fields: Optional[List[str]] + ) -> Dict[str, str]: + """Fetch data available (directily or indirectly -- i.e. secrets) from the relation.""" + return self._fetch_relation_data_with_secrets( + self.component, self.secret_fields, relation, fields + ) + + def _fetch_my_specific_relation_data( + self, relation: Relation, fields: Optional[List[str]] + ) -> Dict[str, str]: + """Fetch data available (directily or indirectly -- i.e. secrets) from the relation for owner/this_app.""" + return self._fetch_relation_data_with_secrets( + self.component, self.secret_fields, relation, fields + ) + + def _update_relation_data(self, relation: Relation, data: Dict[str, str]) -> None: + """Update data available (directily or indirectly -- i.e. secrets) from the relation for owner/this_app.""" + self._remove_secret_from_databag(relation, list(data.keys())) + _, normal_fields = self._process_secret_fields( + relation, + self.secret_fields, + list(data), + self._add_or_update_relation_secrets, + data=data, + uri_to_databag=False, + ) + + normal_content = {k: v for k, v in data.items() if k in normal_fields} + self._update_relation_data_without_secrets(self.component, relation, normal_content) + + def _delete_relation_data(self, relation: Relation, fields: List[str]) -> None: + """Delete data available (directily or indirectly -- i.e. secrets) from the relation for owner/this_app.""" + if self.secret_fields and self.deleted_label: + current_data = self.fetch_my_relation_data([relation.id], fields) + if current_data is not None: + # Check if the secret we wanna delete actually exists + # Given the "deleted label", here we can't rely on the default mechanism (i.e. 'key not found') + if non_existent := (set(fields) & set(self.secret_fields)) - set( + current_data.get(relation.id, []) + ): + logger.error( + "Non-existing secret %s was attempted to be removed.", + ", ".join(non_existent), + ) + + _, normal_fields = self._process_secret_fields( + relation, + self.secret_fields, + fields, + self._update_relation_secret, + data={field: self.deleted_label for field in fields}, + ) + else: + _, normal_fields = self._process_secret_fields( + relation, self.secret_fields, fields, self._delete_relation_secret, fields=fields + ) + self._delete_relation_data_without_secrets(self.component, relation, list(normal_fields)) + + def fetch_relation_data( + self, + relation_ids: Optional[List[int]] = None, + fields: Optional[List[str]] = None, + relation_name: Optional[str] = None, + ) -> Dict[int, Dict[str, str]]: + """This method makes no sense for a Peer Relation.""" + raise NotImplementedError( + "Peer Relation only supports 'self-side' fetch methods: " + "fetch_my_relation_data() and fetch_my_relation_field()" + ) + + def fetch_relation_field( + self, relation_id: int, field: str, relation_name: Optional[str] = None + ) -> Optional[str]: + """This method makes no sense for a Peer Relation.""" + raise NotImplementedError( + "Peer Relation only supports 'self-side' fetch methods: " + "fetch_my_relation_data() and fetch_my_relation_field()" + ) + + # Public functions -- inherited + + fetch_my_relation_data = DataRelation.fetch_my_relation_data + fetch_my_relation_field = DataRelation.fetch_my_relation_field + + +class DataPeerUnit(DataPeer): + """Unit databag representation.""" + + SCOPE = Scope.UNIT + + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + # General events @@ -1442,12 +1687,8 @@ def extra_user_roles(self) -> Optional[str]: return self.relation.data[self.relation.app].get("extra-user-roles") -class AuthenticationEvent(RelationEvent): - """Base class for authentication fields for events. - - The amount of logic added here is not ideal -- but this was the only way to preserve - the interface when moving to Juju Secrets - """ +class RelationEventWithSecret(RelationEvent): + """Base class for Relation Events that need to handle secrets.""" @property def _secrets(self) -> dict: @@ -1459,18 +1700,6 @@ def _secrets(self) -> dict: self._cached_secrets = {} return self._cached_secrets - @property - def _jujuversion(self) -> JujuVersion: - """Caching jujuversion to avoid a Juju call on each field evaluation. - - DON'T USE the encapsulated helper variable outside of this function - """ - if not hasattr(self, "_cached_jujuversion"): - self._cached_jujuversion = None - if not self._cached_jujuversion: - self._cached_jujuversion = JujuVersion.from_environ() - return self._cached_jujuversion - def _get_secret(self, group) -> Optional[Dict[str, str]]: """Retrieveing secrets.""" if not self.app: @@ -1486,7 +1715,15 @@ def _get_secret(self, group) -> Optional[Dict[str, str]]: @property def secrets_enabled(self): """Is this Juju version allowing for Secrets usage?""" - return self._jujuversion.has_secrets + return JujuVersion.from_environ().has_secrets + + +class AuthenticationEvent(RelationEventWithSecret): + """Base class for authentication fields for events. + + The amount of logic added here is not ideal -- but this was the only way to preserve + the interface when moving to Juju Secrets + """ @property def username(self) -> Optional[str]: @@ -1559,6 +1796,17 @@ def database(self) -> Optional[str]: class DatabaseRequestedEvent(DatabaseProvidesEvent, ExtraRoleEvent): """Event emitted when a new database is requested for use on this relation.""" + @property + def external_node_connectivity(self) -> bool: + """Returns the requested external_node_connectivity field.""" + if not self.relation.app: + return False + + return ( + self.relation.data[self.relation.app].get("external-node-connectivity", "false") + == "true" + ) + class DatabaseProvidesEvents(CharmEvents): """Database events. @@ -1569,7 +1817,7 @@ class DatabaseProvidesEvents(CharmEvents): database_requested = EventSource(DatabaseRequestedEvent) -class DatabaseRequiresEvent(RelationEvent): +class DatabaseRequiresEvent(RelationEventWithSecret): """Base class for database events.""" @property @@ -1624,6 +1872,11 @@ def uris(self) -> Optional[str]: if not self.relation.app: return None + if self.secrets_enabled: + secret = self._get_secret("user") + if secret: + return secret.get("uris") + return self.relation.data[self.relation.app].get("uris") @property @@ -1667,7 +1920,7 @@ class DatabaseRequiresEvents(CharmEvents): class DatabaseProvides(DataProvides): """Provider-side of the database relations.""" - on = DatabaseProvidesEvents() # pyright: ignore [reportGeneralTypeIssues] + on = DatabaseProvidesEvents() # pyright: ignore [reportAssignmentType] def __init__(self, charm: CharmBase, relation_name: str) -> None: super().__init__(charm, relation_name) @@ -1762,7 +2015,7 @@ def set_version(self, relation_id: int, version: str) -> None: class DatabaseRequires(DataRequires): """Requires-side of the database relation.""" - on = DatabaseRequiresEvents() # pyright: ignore [reportGeneralTypeIssues] + on = DatabaseRequiresEvents() # pyright: ignore [reportAssignmentType] def __init__( self, @@ -1772,11 +2025,13 @@ def __init__( extra_user_roles: Optional[str] = None, relations_aliases: Optional[List[str]] = None, additional_secret_fields: Optional[List[str]] = [], + external_node_connectivity: bool = False, ): """Manager of database client relations.""" super().__init__(charm, relation_name, extra_user_roles, additional_secret_fields) self.database = database_name self.relations_aliases = relations_aliases + self.external_node_connectivity = external_node_connectivity # Define custom event names for each alias. if relations_aliases: @@ -1927,16 +2182,16 @@ def _on_relation_created_event(self, event: RelationCreatedEvent) -> None: if not self.local_unit.is_leader(): return + event_data = {"database": self.database} + if self.extra_user_roles: - self.update_relation_data( - event.relation.id, - { - "database": self.database, - "extra-user-roles": self.extra_user_roles, - }, - ) - else: - self.update_relation_data(event.relation.id, {"database": self.database}) + event_data["extra-user-roles"] = self.extra_user_roles + + # set external-node-connectivity field + if self.external_node_connectivity: + event_data["external-node-connectivity"] = "true" + + self.update_relation_data(event.relation.id, event_data) def _on_relation_changed_event(self, event: RelationChangedEvent) -> None: """Event emitted when the database relation has changed.""" @@ -2091,7 +2346,7 @@ class KafkaRequiresEvents(CharmEvents): class KafkaProvides(DataProvides): """Provider-side of the Kafka relation.""" - on = KafkaProvidesEvents() # pyright: ignore [reportGeneralTypeIssues] + on = KafkaProvidesEvents() # pyright: ignore [reportAssignmentType] def __init__(self, charm: CharmBase, relation_name: str) -> None: super().__init__(charm, relation_name) @@ -2152,7 +2407,7 @@ def set_zookeeper_uris(self, relation_id: int, zookeeper_uris: str) -> None: class KafkaRequires(DataRequires): """Requires-side of the Kafka relation.""" - on = KafkaRequiresEvents() # pyright: ignore [reportGeneralTypeIssues] + on = KafkaRequiresEvents() # pyright: ignore [reportAssignmentType] def __init__( self, @@ -2289,7 +2544,7 @@ class OpenSearchRequiresEvents(CharmEvents): class OpenSearchProvides(DataProvides): """Provider-side of the OpenSearch relation.""" - on = OpenSearchProvidesEvents() # pyright: ignore[reportGeneralTypeIssues] + on = OpenSearchProvidesEvents() # pyright: ignore[reportAssignmentType] def __init__(self, charm: CharmBase, relation_name: str) -> None: super().__init__(charm, relation_name) @@ -2342,7 +2597,7 @@ def set_version(self, relation_id: int, version: str) -> None: class OpenSearchRequires(DataRequires): """Requires-side of the OpenSearch relation.""" - on = OpenSearchRequiresEvents() # pyright: ignore[reportGeneralTypeIssues] + on = OpenSearchRequiresEvents() # pyright: ignore[reportAssignmentType] def __init__( self, diff --git a/lib/charms/mongodb/v0/config_server_interface.py b/lib/charms/mongodb/v0/config_server_interface.py index e9b3ab0c..9e05e6a1 100644 --- a/lib/charms/mongodb/v0/config_server_interface.py +++ b/lib/charms/mongodb/v0/config_server_interface.py @@ -16,7 +16,7 @@ from charms.mongodb.v1.mongos import MongosConnection from ops.charm import CharmBase, EventBase, RelationBrokenEvent from ops.framework import Object -from ops.model import ActiveStatus, MaintenanceStatus, WaitingStatus +from ops.model import ActiveStatus, BlockedStatus, MaintenanceStatus, WaitingStatus from config import Config @@ -35,7 +35,7 @@ # Increment this PATCH version before using `charmcraft publish-lib` or reset # to 0 if you are raising the major API version -LIBPATCH = 4 +LIBPATCH = 7 class ClusterProvider(Object): @@ -64,18 +64,29 @@ def __init__( def pass_hook_checks(self, event: EventBase) -> bool: """Runs the pre-hooks checks for ClusterProvider, returns True if all pass.""" - if not self.charm.is_role(Config.Role.CONFIG_SERVER): + if not self.charm.db_initialised: + logger.info("Deferring %s. db is not initialised.", type(event)) + event.defer() + return False + + if not self.is_valid_mongos_integration(): logger.info( - "Skipping %s. ShardingProvider is only be executed by config-server", type(event) + "Skipping %s. ClusterProvider is only be executed by config-server", type(event) ) return False if not self.charm.unit.is_leader(): return False - if not self.charm.db_initialised: - logger.info("Deferring %s. db is not initialised.", type(event)) - event.defer() + return True + + def is_valid_mongos_integration(self) -> bool: + """Returns true if the integration to mongos is valid.""" + is_integrated_to_mongos = len( + self.charm.model.relations[Config.Relations.CLUSTER_RELATIONS_NAME] + ) + + if not self.charm.is_role(Config.Role.CONFIG_SERVER) and is_integrated_to_mongos: return False return True @@ -83,6 +94,10 @@ def pass_hook_checks(self, event: EventBase) -> bool: def _on_relation_changed(self, event) -> None: """Handles providing mongos with KeyFile and hosts.""" if not self.pass_hook_checks(event): + if not self.is_valid_mongos_integration(): + self.charm.unit.status = BlockedStatus( + "Relation to mongos not supported, config role must be config-server" + ) logger.info("Skipping relation joined event: hook checks did not pass") return @@ -208,7 +223,6 @@ def _on_relation_changed(self, event) -> None: event.relation.id, CONFIG_SERVER_DB_KEY ) if not key_file_contents or not config_server_db: - event.defer() self.charm.unit.status = WaitingStatus("Waiting for secrets from config-server") return @@ -261,7 +275,13 @@ def _on_relation_broken(self, event: RelationBrokenEvent) -> None: def is_mongos_running(self) -> bool: """Returns true if mongos service is running.""" - with MongosConnection(None, f"mongodb://{MONGOS_SOCKET_URI_FMT}") as mongo: + connection_uri = f"mongodb://{self.charm.get_mongos_host()}" + + # when running internally, connections through Unix Domain sockets do not need port. + if self.charm.is_external_client: + connection_uri = connection_uri + f":{Config.MONGOS_PORT}" + + with MongosConnection(None, connection_uri) as mongo: return mongo.is_ready def update_config_server_db(self, config_server_db) -> bool: @@ -271,7 +291,10 @@ def update_config_server_db(self, config_server_db) -> bool: mongos_config = self.charm.mongos_config mongos_start_args = get_mongos_args( - mongos_config, snap_install=True, config_server_db=config_server_db + mongos_config, + snap_install=True, + config_server_db=config_server_db, + external_connectivity=self.charm.is_external_client, ) add_args_to_env("MONGOS_ARGS", mongos_start_args) return True diff --git a/lib/charms/mongodb/v1/helpers.py b/lib/charms/mongodb/v1/helpers.py index ea2dd736..9038198d 100644 --- a/lib/charms/mongodb/v1/helpers.py +++ b/lib/charms/mongodb/v1/helpers.py @@ -1,4 +1,5 @@ """Simple functions, which can be used in both K8s and VM charms.""" + # Copyright 2023 Canonical Ltd. # See LICENSE file for licensing details. import json @@ -29,7 +30,7 @@ # Increment this PATCH version before using `charmcraft publish-lib` or reset # to 0 if you are raising the major API version -LIBPATCH = 1 +LIBPATCH = 4 # path to store mongodb ketFile KEY_FILE = "keyFile" @@ -44,11 +45,22 @@ MONGO_SHELL = "charmed-mongodb.mongosh" DATA_DIR = "/var/lib/mongodb" +LOG_DIR = "/var/log/mongodb" +LOG_TO_SYSLOG = True CONF_DIR = "/etc/mongod" MONGODB_LOG_FILENAME = "mongodb.log" logger = logging.getLogger(__name__) +def _get_logging_options(snap_install: bool) -> str: + # TODO sending logs to syslog until we have a separate mount point for logs + if LOG_TO_SYSLOG: + return "" + # in k8s the default logging options that are used for the vm charm are ignored and logs are + # the output of the container. To enable logging to a file it must be set explicitly + return f"--logpath={LOG_DIR}/{MONGODB_LOG_FILENAME}" if snap_install else "" + + # noinspection GrazieInspection def get_create_user_cmd(config: MongoDBConfiguration, mongo_path=MONGO_SHELL) -> List[str]: """Creates initial admin user for MongoDB. @@ -84,6 +96,7 @@ def get_mongos_args( config, snap_install: bool = False, config_server_db: str = None, + external_connectivity: bool = True, ) -> str: """Returns the arguments used for starting mongos on a config-server side application. @@ -92,9 +105,9 @@ def get_mongos_args( """ # suborinate charm which provides its own config_server_db, should only use unix domain socket binding_ips = ( - f"--bind_ip {MONGODB_COMMON_DIR}/var/mongodb-27018.sock" - if config_server_db - else "--bind_ip_all" + "--bind_ip_all" + if external_connectivity + else f"--bind_ip {MONGODB_COMMON_DIR}/var/mongodb-27018.sock" ) # mongos running on the config server communicates through localhost @@ -130,9 +143,7 @@ def get_mongod_args( """ full_data_dir = f"{MONGODB_COMMON_DIR}{DATA_DIR}" if snap_install else DATA_DIR full_conf_dir = f"{MONGODB_SNAP_DATA_DIR}{CONF_DIR}" if snap_install else CONF_DIR - # in k8s the default logging options that are used for the vm charm are ignored and logs are - # the output of the container. To enable logging to a file it must be set explicitly - logging_options = "" if snap_install else f"--logpath={full_data_dir}/{MONGODB_LOG_FILENAME}" + logging_options = _get_logging_options(snap_install) cmd = [ # bind to localhost and external interfaces "--bind_ip_all", @@ -143,6 +154,8 @@ def get_mongod_args( # for simplicity we run the mongod daemon on shards, configsvrs, and replicas on the same # port f"--port={Config.MONGODB_PORT}", + "--auditDestination=syslog", # TODO sending logs to syslog until we have a separate mount point for logs + f"--auditFormat={Config.AuditLog.FORMAT}", logging_options, ] if auth: @@ -164,6 +177,7 @@ def get_mongod_args( f"--tlsCertificateKeyFile={full_conf_dir}/{TLS_EXT_PEM_FILE}", # allow non-TLS connections "--tlsMode=preferTLS", + "--tlsDisabledProtocols=TLS1_0,TLS1_1", ] ) diff --git a/lib/charms/mongos/v0/mongos_client_interface.py b/lib/charms/mongos/v0/mongos_client_interface.py index edaa1627..c455dacc 100644 --- a/lib/charms/mongos/v0/mongos_client_interface.py +++ b/lib/charms/mongos/v0/mongos_client_interface.py @@ -11,7 +11,7 @@ from ops.charm import CharmBase from charms.data_platform_libs.v0.data_interfaces import ( DatabaseProvides, - DatabaseRequires, + DatabaseRequestedEvent, ) from charms.mongodb.v1.mongos import MongosConfiguration @@ -20,6 +20,7 @@ DATABASE_KEY = "database" USER_ROLES_KEY = "extra-user-roles" MONGOS_RELATION_NAME = "mongos_proxy" +EXTERNAL_CONNECTIVITY_TAG = "external-node-connectivity" # TODO - the below LIBID, LIBAPI, and LIBPATCH are not valid and were made manually. These will be # created automatically once the charm has been published. The charm has not yet been published @@ -34,7 +35,7 @@ # Increment this PATCH version before using `charmcraft publish-lib` or reset # to 0 if you are raising the major API version -LIBPATCH = 1 +LIBPATCH = 2 """Library to manage the relation for the application between mongos and the deployed application. In short, this relation ensure that: @@ -92,9 +93,20 @@ def _on_relation_changed(self, event) -> None: if not self.charm.unit.is_leader(): return - relation_data = event.relation.data[event.app] - new_database_name = relation_data.get(DATABASE_KEY, self.charm.database) - new_extra_user_roles = relation_data.get(USER_ROLES_KEY, self.charm.extra_user_roles) + new_database_name = ( + self.database_provides.fetch_relation_field(event.relation.id, DATABASE_KEY) + or self.charm.database + ) + new_extra_user_roles = ( + self.database_provides.fetch_relation_field(event.relation.id, USER_ROLES_KEY) + or self.charm.extra_user_roles + ) + external_connectivity = ( + self.database_provides.fetch_relation_field( + event.relation.id, EXTERNAL_CONNECTIVITY_TAG + ) + == "true" + ) if new_database_name != self.charm.database: self.charm.set_database(new_database_name) @@ -105,6 +117,10 @@ def _on_relation_changed(self, event) -> None: self.charm.set_user_roles(new_extra_user_roles) + self.charm.set_external_connectivity(external_connectivity) + if external_connectivity: + self.charm.open_mongos_port() + def remove_connection_info(self) -> None: """Sends the URI to the related parent application""" logger.info("Removing connection information from host application.") @@ -123,31 +139,3 @@ def update_connection_info(self, config: MongosConfiguration) -> None: relation.id, config.uri, ) - - -class MongosRequirer(Object): - """Manage relations between the mongos router and the application on the application side.""" - - def __init__( - self, - charm: CharmBase, - database_name: str, - extra_user_roles: str, - relation_name: str = MONGOS_RELATION_NAME, - ) -> None: - """Constructor for MongosRequirer object.""" - self.relation_name = relation_name - self.charm = charm - - if not database_name: - database_name = f"{self.charm.app}" - - self.database_requires = DatabaseRequires( - self.charm, - relation_name=self.relation_name, - database_name=database_name, - extra_user_roles=extra_user_roles, - ) - - super().__init__(charm, self.relation_name) - # TODO Future PRs handle relation broken diff --git a/metadata.yaml b/metadata.yaml index cc8101e2..b33dc49f 100644 --- a/metadata.yaml +++ b/metadata.yaml @@ -24,6 +24,7 @@ requires: mongos_proxy: interface: mongos_client scope: container + limit: 1 cluster: interface: config-server limit: 1 diff --git a/src/charm.py b/src/charm.py index 841d2de0..8b107e16 100755 --- a/src/charm.py +++ b/src/charm.py @@ -9,9 +9,10 @@ from charms.operator_libs_linux.v1 import snap from pathlib import Path +from typing import Set, List, Optional, Dict + from charms.mongodb.v0.mongodb_secrets import SecretCache from charms.mongos.v0.mongos_client_interface import MongosProvider -from typing import Set, List, Optional, Dict from charms.mongodb.v0.mongodb_secrets import generate_secret_label from charms.mongodb.v1.mongos import MongosConfiguration from charms.mongodb.v0.config_server_interface import ClusterRequirer @@ -39,6 +40,7 @@ CONFIG_ARG = "--configdb" USER_ROLES_TAG = "extra-user-roles" DATABASE_TAG = "database" +EXTERNAL_CONNECTIVITY_TAG = "external-connectivity" class MongosOperatorCharm(ops.CharmBase): @@ -286,6 +288,12 @@ def set_database(self, database: str) -> None: config_server_rel.id, {DATABASE_TAG: database} ) + def set_external_connectivity(self, external_connectivity: bool) -> None: + """Sets the connectivity type for mongos.""" + self.app_peer_data[EXTERNAL_CONNECTIVITY_TAG] = json.dumps( + external_connectivity + ) + def check_relation_broken_or_scale_down(self, event: RelationDepartedEvent) -> None: """Checks relation departed event is the result of removed relation or scale down. @@ -338,14 +346,37 @@ def proceed_on_broken_event(self, event) -> bool: return True + def get_mongos_host(self) -> str: + """Returns the host for mongos as a str. + + The host for mongos can be either the Unix Domain Socket or an IP address depending on how + the client wishes to connect to mongos (inside Juju or outside). + """ + if self.is_external_client: + return self._unit_ip + return Config.MONGOS_SOCKET_URI_FMT + @staticmethod def _generate_relation_departed_key(rel_id: int) -> str: """Generates the relation departed key for a specified relation id.""" return f"relation_{rel_id}_departed" + def open_mongos_port(self) -> None: + """Opens the mongos port for TCP connections.""" + self.unit.open_port("tcp", Config.MONGOS_PORT) + # END: helper functions # BEGIN: properties + @property + def _unit_ip(self) -> str: + """Returns the ip address of the unit.""" + return str(self.model.get_binding(Config.Relations.PEERS).network.bind_address) + + @property + def is_external_client(self) -> Optional[str]: + """Returns the database requested by the hosting application of the subordinate charm.""" + return json.loads(self.app_peer_data.get(EXTERNAL_CONNECTIVITY_TAG)) @property def database(self) -> Optional[str]: @@ -370,12 +401,8 @@ def extra_user_roles(self) -> Set[str]: @property def mongos_config(self) -> MongosConfiguration: """Generates a MongoDBConfiguration object for mongos in the deployment of MongoDB.""" - # TODO future PR - use ip addresses for hosts for data-integrator as that charm will not - # communicate to mongos via the Unix Domain Socket. - hosts = [Config.MONGOS_SOCKET_URI_FMT] - # mongos using Unix Domain Socket to communicate do not use port, Future PR - use port - # when suborinate charm of data-integrator. - port = None + hosts = [self.get_mongos_host()] + port = Config.MONGOS_PORT if self.is_external_client else None return MongosConfiguration( database=self.database, @@ -397,15 +424,6 @@ def _peers(self) -> Optional[Relation]: """ return self.model.get_relation(Config.Relations.PEERS) - @property - def _peers(self) -> Optional[Relation]: - """Fetch the peer relation. - - Returns: - An `ops.model.Relation` object representing the peer relation. - """ - return self.model.get_relation(Config.Relations.PEERS) - @property def unit_peer_data(self) -> Dict: """Unit peer relation data object.""" diff --git a/tests/integration/application/lib/charms/mongos/v0/mongos_client_interface.py b/tests/integration/application/lib/charms/mongos/v0/mongos_client_interface.py deleted file mode 100644 index a08dc608..00000000 --- a/tests/integration/application/lib/charms/mongos/v0/mongos_client_interface.py +++ /dev/null @@ -1,139 +0,0 @@ -# Copyright 2023 Canonical Ltd. -# See LICENSE file for licensing details. - -"""In this class, we manage relations between config-servers and shards. - -This class handles the sharing of secrets between sharded components, adding shards, and removing -shards. -""" -import logging -from ops.framework import Object -from ops.charm import CharmBase -from charms.data_platform_libs.v0.data_interfaces import ( - DatabaseProvides, - DatabaseRequires, -) - - -logger = logging.getLogger(__name__) -DATABASE_KEY = "database" -USER_ROLES_KEY = "extra-user-roles" -MONGOS_RELATION_NAME = "mongos_proxy" - -# TODO - the below LIBID, LIBAPI, and LIBPATCH are not valid and were made manually. These will be -# created automatically once the charm has been published. The charm has not yet been published -# due to: -# https://discourse.charmhub.io/t/request-ownership-of-reserved-mongos-charm/12735 - -# The unique Charmhub library identifier, never change it -LIBID = "58ad1ccca4974932ba22b97781b9b2a0" - -# Increment this major API version when introducing breaking changes -LIBAPI = 0 - -# Increment this PATCH version before using `charmcraft publish-lib` or reset -# to 0 if you are raising the major API version -LIBPATCH = 1 - -"""Library to manage the relation for the application between mongos and the deployed application. -In short, this relation ensure that: -1. mongos receives the specified database and users roles needed by the host application -2. the host application receives the generated username, password and uri for connecting to the -sharded cluster. - -This library contains the Requires and Provides classes for handling the relation between an -application and mongos. The mongos application relies on the MongosProvider class and the deployed -application uses the MongoDBRequires class. - -The following is an example of how to use the MongoDBRequires class to specify the roles and -database name: - -```python -from charms.mongos.v0.mongos_client_interface import MongosRequirer - - -class ApplicationCharm(CharmBase): - - def __init__(self, *args): - super().__init__(*args) - - # relation events for mongos client - self._mongos_client = MongosRequirer( - self, - database_name="my-test-db", - extra_user_roles="admin", - ) -``` - -To receive the username, password, and uri: -# TODO this is to be implemented in a future PR -""" - - -class MongosProvider(Object): - """Manage relations between the mongos router and the application on the mongos side.""" - - def __init__( - self, charm: CharmBase, relation_name: str = MONGOS_RELATION_NAME - ) -> None: - """Constructor for MongosProvider object.""" - self.relation_name = relation_name - self.charm = charm - self.database_provides = DatabaseProvides( - self.charm, relation_name=self.relation_name - ) - - super().__init__(charm, self.relation_name) - self.framework.observe( - charm.on[self.relation_name].relation_changed, self._on_relation_changed - ) - - # TODO Future PRs handle relation broken - - def _on_relation_changed(self, event) -> None: - """Handles updating the database and extra user roles.""" - if not self.charm.unit.is_leader(): - return - - relation_data = event.relation.data[event.app] - new_database_name = relation_data.get(DATABASE_KEY, self.charm.database) - new_extra_user_roles = relation_data.get( - USER_ROLES_KEY, self.charm.extra_user_roles - ) - - if new_database_name != self.charm.database: - self.charm.set_database(new_database_name) - - if new_extra_user_roles != self.charm.extra_user_roles: - if isinstance(new_extra_user_roles, str): - new_extra_user_roles = [new_extra_user_roles] - - self.charm.set_user_roles(new_extra_user_roles) - - -class MongosRequirer(Object): - """Manage relations between the mongos router and the application on the application side.""" - - def __init__( - self, - charm: CharmBase, - database_name: str, - extra_user_roles: str, - relation_name: str = MONGOS_RELATION_NAME, - ) -> None: - """Constructor for MongosRequirer object.""" - self.relation_name = relation_name - self.charm = charm - - if not database_name: - database_name = f"{self.charm.app}" - - self.database_requires = DatabaseRequires( - self.charm, - relation_name=self.relation_name, - database_name=database_name, - extra_user_roles=extra_user_roles, - ) - - super().__init__(charm, self.relation_name) - # TODO Future PRs handle relation broken diff --git a/tests/integration/application/metadata.yaml b/tests/integration/application/metadata.yaml index 94b06adf..6b2fd3a9 100644 --- a/tests/integration/application/metadata.yaml +++ b/tests/integration/application/metadata.yaml @@ -10,5 +10,5 @@ series: - jammy provides: - mongos_proxy: + mongos: interface: mongos_client diff --git a/tests/integration/application/src/charm.py b/tests/integration/application/src/charm.py index 81958f32..1f2deea7 100755 --- a/tests/integration/application/src/charm.py +++ b/tests/integration/application/src/charm.py @@ -15,7 +15,7 @@ from ops.model import ActiveStatus -from charms.mongos.v0.mongos_client_interface import MongosRequirer +from charms.data_platform_libs.v0.data_interfaces import DatabaseRequires logger = logging.getLogger(__name__) @@ -32,8 +32,9 @@ def __init__(self, *args): self.framework.observe(self.on.start, self._on_start) # relation events for mongos client - self._mongos_client = MongosRequirer( + self.database = DatabaseRequires( self, + relation_name="mongos", database_name="my-test-db", extra_user_roles=EXTRA_USER_ROLES, ) diff --git a/tests/integration/external_relations/__init__.py b/tests/integration/external_relations/__init__.py new file mode 100644 index 00000000..db3bfe1a --- /dev/null +++ b/tests/integration/external_relations/__init__.py @@ -0,0 +1,2 @@ +# Copyright 2023 Canonical Ltd. +# See LICENSE file for licensing details. diff --git a/tests/integration/external_relations/test_charm.py b/tests/integration/external_relations/test_charm.py new file mode 100644 index 00000000..29ad6834 --- /dev/null +++ b/tests/integration/external_relations/test_charm.py @@ -0,0 +1,121 @@ +#!/usr/bin/env python3 +# Copyright 2023 Canonical Ltd. +# See LICENSE file for licensing details. +import pytest +from pytest_operator.plugin import OpsTest +from ..helpers import check_mongos + + +DATA_INTEGRATOR_APP_NAME = "data-integrator" +MONGOS_APP_NAME = "mongos" +CLUSTER_REL_NAME = "cluster" +MONGODB_CHARM_NAME = "mongodb" + +CONFIG_SERVER_APP_NAME = "config-server" +SHARD_APP_NAME = "shard" +SHARD_REL_NAME = "sharding" +CONFIG_SERVER_REL_NAME = "config-server" + + +@pytest.mark.group(1) +@pytest.mark.abort_on_fail +async def test_build_and_deploy(ops_test: OpsTest) -> None: + """Build and deploy a sharded cluster.""" + + mongos_charm = await ops_test.build_charm(".") + await ops_test.model.deploy( + DATA_INTEGRATOR_APP_NAME, + channel="latest/edge", + ) + await ops_test.model.deploy( + mongos_charm, + num_units=0, + application_name=MONGOS_APP_NAME, + ) + await ops_test.model.deploy( + MONGODB_CHARM_NAME, + application_name=CONFIG_SERVER_APP_NAME, + channel="6/edge", + revision=142, + config={"role": "config-server"}, + ) + await ops_test.model.deploy( + MONGODB_CHARM_NAME, + application_name=SHARD_APP_NAME, + channel="6/edge", + revision=142, + config={"role": "shard"}, + ) + + await ops_test.model.wait_for_idle( + apps=[DATA_INTEGRATOR_APP_NAME, SHARD_APP_NAME, CONFIG_SERVER_APP_NAME], + idle_period=10, + raise_on_blocked=False, + ) + + +@pytest.mark.group(1) +@pytest.mark.abort_on_fail +async def test_mongos_starts_with_config_server(ops_test: OpsTest) -> None: + """Verify mongos is running and can be accessed externally via IP-address.""" + # mongos cannot start until it has a host application + await ops_test.model.applications[DATA_INTEGRATOR_APP_NAME].set_config( + { + "database-name": "test-database", + } + ) + + await ops_test.model.add_relation(DATA_INTEGRATOR_APP_NAME, MONGOS_APP_NAME) + await ops_test.model.wait_for_idle( + apps=[MONGOS_APP_NAME], + status="blocked", + idle_period=10, + ) + + # prepare sharded cluster + await ops_test.model.wait_for_idle( + apps=[CONFIG_SERVER_APP_NAME, SHARD_APP_NAME], + idle_period=10, + raise_on_blocked=False, + ) + await ops_test.model.integrate( + f"{SHARD_APP_NAME}:{SHARD_REL_NAME}", + f"{CONFIG_SERVER_APP_NAME}:{CONFIG_SERVER_REL_NAME}", + ) + await ops_test.model.wait_for_idle( + apps=[CONFIG_SERVER_APP_NAME, SHARD_APP_NAME], + idle_period=20, + raise_on_blocked=False, + ) + + # connect sharded cluster to mongos + await ops_test.model.integrate( + f"{MONGOS_APP_NAME}:{CLUSTER_REL_NAME}", + f"{CONFIG_SERVER_APP_NAME}:{CLUSTER_REL_NAME}", + ) + await ops_test.model.wait_for_idle( + apps=[CONFIG_SERVER_APP_NAME, SHARD_APP_NAME, MONGOS_APP_NAME], + idle_period=20, + status="active", + ) + + mongos_unit = ops_test.model.applications[MONGOS_APP_NAME].units[0] + mongos_running = await check_mongos( + ops_test, mongos_unit, auth=False, external=True + ) + assert mongos_running, "Mongos is not currently running." + + +@pytest.mark.group(1) +@pytest.mark.abort_on_fail +async def test_mongos_has_user(ops_test: OpsTest) -> None: + """Verify mongos has user and is able to connect externally via IP-address.""" + mongos_unit = ops_test.model.applications[MONGOS_APP_NAME].units[0] + mongos_running = await check_mongos( + ops_test, + mongos_unit, + app_name=DATA_INTEGRATOR_APP_NAME, + auth=True, + external=True, + ) + assert mongos_running, "Mongos is not currently running." diff --git a/tests/integration/helpers.py b/tests/integration/helpers.py index 56112557..09687a67 100644 --- a/tests/integration/helpers.py +++ b/tests/integration/helpers.py @@ -11,18 +11,29 @@ async def generate_mongos_command( - ops_test: OpsTest, auth: bool, uri: str = None + ops_test: OpsTest, + auth: bool, + app_name: Optional[str], + uri: str = None, + external: bool = False, ) -> str: """Generates a command which verifies mongos is running.""" - mongodb_uri = uri or await generate_mongos_uri(ops_test, auth) + mongodb_uri = uri or await generate_mongos_uri(ops_test, auth, app_name, external) return f"{MONGO_SHELL} '{mongodb_uri}' --eval '{PING_CMD}'" async def check_mongos( - ops_test: OpsTest, unit: ops.model.Unit, auth: bool, uri: str = None + ops_test: OpsTest, + unit: ops.model.Unit, + auth: bool, + app_name: Optional[str] = None, + uri: str = None, + external: bool = False, ) -> bool: """Returns whether mongos is running on the provided unit.""" - mongos_check = await generate_mongos_command(ops_test, auth, uri) + mongos_check = await generate_mongos_command( + ops_test, auth, app_name, uri, external + ) # since mongos is communicating only via the unix domain socket, we cannot connect to it via # traditional pymongo methods @@ -31,13 +42,15 @@ async def check_mongos( return return_code == 0 -async def run_mongos_command(ops_test: OpsTest, unit: ops.model.Unit, mongos_cmd: str): +async def run_mongos_command( + ops_test: OpsTest, unit: ops.model.Unit, mongos_cmd: str, app_name: str +): """Runs the provided mongos command. The mongos charm uses the unix domain socket to communicate, and therefore we cannot run MongoDB commands from outside the unit and we must use `juju exec` instead. """ - mongodb_uri = await generate_mongos_uri(ops_test, auth=True) + mongodb_uri = await generate_mongos_uri(ops_test, auth=True, app_name=app_name) check_cmd = [ "exec", @@ -53,13 +66,23 @@ async def run_mongos_command(ops_test: OpsTest, unit: ops.model.Unit, mongos_cmd return (return_code, std_output, std_err) -async def generate_mongos_uri(ops_test: OpsTest, auth: bool) -> str: +async def generate_mongos_uri( + ops_test: OpsTest, + auth: bool, + app_name: Optional[str] = None, + external: bool = False, +) -> str: """Generates a URI for accessing mongos.""" + host = ( + MONGOS_SOCKET + if not external + else f"{await get_ip_address(ops_test, app_name=MONGOS_APP_NAME)}:27018" + ) if not auth: - return f"mongodb://{MONGOS_SOCKET}" + return f"mongodb://{host}" secret_uri = await get_application_relation_data( - ops_test, "application", "mongos_proxy", "secret-user" + ops_test, app_name, "mongos", "secret-user" ) secret_data = await get_secret_data(ops_test, secret_uri) @@ -102,7 +125,6 @@ async def get_application_relation_data( """ unit = ops_test.model.applications[application_name].units[0] raw_data = (await ops_test.juju("show-unit", unit.name))[1] - if not raw_data: raise ValueError(f"no unit info could be grabbed for { unit.name}") data = yaml.safe_load(raw_data) @@ -111,7 +133,6 @@ async def get_application_relation_data( relation_data = [ v for v in data[unit.name]["relation-info"] if v["endpoint"] == relation_name ] - if relation_id: # Filter the data based on the relation id. relation_data = [v for v in relation_data if v["relation-id"] == relation_id] @@ -130,3 +151,9 @@ async def get_application_relation_data( ) return relation_data[0]["application-data"].get(key) + + +async def get_ip_address(ops_test, app_name=MONGOS_APP_NAME) -> str: + """Returns an IP address of the fist unit of a provided application.""" + app_unit = ops_test.model.applications[app_name].units[0] + return await app_unit.get_public_address() diff --git a/tests/integration/test_charm.py b/tests/integration/test_charm.py index 5f926539..3fbbe0f0 100644 --- a/tests/integration/test_charm.py +++ b/tests/integration/test_charm.py @@ -120,7 +120,9 @@ async def test_mongos_starts_with_config_server(ops_test: OpsTest) -> None: async def test_mongos_has_user(ops_test: OpsTest) -> None: # prepare sharded cluster mongos_unit = ops_test.model.applications[MONGOS_APP_NAME].units[0] - mongos_running = await check_mongos(ops_test, mongos_unit, auth=True) + mongos_running = await check_mongos( + ops_test, mongos_unit, app_name=APPLICATION_APP_NAME, auth=True + ) assert mongos_running, "Mongos is not currently running." @@ -147,7 +149,9 @@ async def test_mongos_updates_config_db(ops_test: OpsTest) -> None: # prepare sharded cluster mongos_unit = ops_test.model.applications[MONGOS_APP_NAME].units[0] - mongos_running = await check_mongos(ops_test, mongos_unit, auth=True) + mongos_running = await check_mongos( + ops_test, mongos_unit, app_name=APPLICATION_APP_NAME, auth=True + ) assert mongos_running, "Mongos is not currently running." @@ -156,13 +160,23 @@ async def test_mongos_updates_config_db(ops_test: OpsTest) -> None: async def test_user_with_extra_roles(ops_test: OpsTest) -> None: cmd = f'db.createUser({{user: "{TEST_USER_NAME}", pwd: "{TEST_USER_PWD}", roles: [{{role: "readWrite", db: "{TEST_DB_NAME}"}}]}});' mongos_unit = ops_test.model.applications[MONGOS_APP_NAME].units[0] - return_code, _, std_err = await run_mongos_command(ops_test, mongos_unit, cmd) + return_code, _, std_err = await run_mongos_command( + ops_test, mongos_unit, cmd, app_name=APPLICATION_APP_NAME + ) assert ( return_code == 0 ), f"mongos user does not have correct permissions to create new user, error: {std_err}" - test_user_uri = f"mongodb://{TEST_USER_NAME}:{TEST_USER_PWD}@{MONGOS_SOCKET}/{TEST_DB_NAME}" - mongos_running = await check_mongos(ops_test, mongos_unit, auth=True, uri=test_user_uri) + test_user_uri = ( + f"mongodb://{TEST_USER_NAME}:{TEST_USER_PWD}@{MONGOS_SOCKET}/{TEST_DB_NAME}" + ) + mongos_running = await check_mongos( + ops_test, + mongos_unit, + app_name=APPLICATION_APP_NAME, + auth=True, + uri=test_user_uri, + ) assert mongos_running, "User created is not accessible." @@ -179,7 +193,9 @@ async def test_mongos_can_scale(ops_test: OpsTest) -> None: ) for mongos_unit in ops_test.model.applications[MONGOS_APP_NAME].units: - mongos_running = await check_mongos(ops_test, mongos_unit, auth=True) + mongos_running = await check_mongos( + ops_test, mongos_unit, app_name=APPLICATION_APP_NAME, auth=True + ) assert mongos_running, "Mongos is not currently running." # destroy the unit we were initially connected to @@ -194,7 +210,9 @@ async def test_mongos_can_scale(ops_test: OpsTest) -> None: # prepare sharded cluster mongos_unit = ops_test.model.applications[MONGOS_APP_NAME].units[0] - mongos_running = await check_mongos(ops_test, mongos_unit, auth=True) + mongos_running = await check_mongos( + ops_test, mongos_unit, app_name=APPLICATION_APP_NAME, auth=True + ) assert mongos_running, "Mongos is not currently running." @@ -219,13 +237,17 @@ async def test_mongos_stops_without_config_server(ops_test: OpsTest) -> None: ) mongos_unit = ops_test.model.applications[MONGOS_APP_NAME].units[0] - mongos_running = await check_mongos(ops_test, mongos_unit, auth=False) + mongos_running = await check_mongos( + ops_test, mongos_unit, app_name=APPLICATION_APP_NAME, auth=False + ) assert not mongos_running, "Mongos is running without a config server." secrets = await get_application_relation_data( - ops_test, "application", "mongos_proxy", "secret-user" + ops_test, "application", "mongos", "secret-user" ) - assert secrets is None, "mongos still has connection info without being connected to cluster." + assert ( + secrets is None + ), "mongos still has connection info without being connected to cluster." # verify that Charmed MongoDB is blocked waiting for config-server await ops_test.model.wait_for_idle( diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py index bcfe643a..04061e4b 100644 --- a/tests/unit/test_charm.py +++ b/tests/unit/test_charm.py @@ -11,6 +11,7 @@ from parameterized import parameterized from unittest import mock + from charms.operator_libs_linux.v1 import snap from ops.model import BlockedStatus, WaitingStatus from ops.testing import Harness @@ -22,6 +23,9 @@ from charms.data_platform_libs.v0.data_interfaces import DatabaseRequiresEvents CLUSTER_ALIAS = "cluster" +MONGOS_SOCKET_URI_FMT = ( + "%2Fvar%2Fsnap%2Fcharmed-mongodb%2Fcommon%2Fvar%2Fmongodb-27018.sock" +) class TestCharm(unittest.TestCase): @@ -143,3 +147,14 @@ def test_status_shows_mongos_waiting(self): self.harness.add_relation("cluster", "config-server") self.harness.charm.on.update_status.emit() self.assertTrue(isinstance(self.harness.charm.unit.status, WaitingStatus)) + + @patch_network_get(private_address="1.1.1.1") + def test_mongos_host(self): + """TBD.""" + self.harness.charm.app_peer_data["external-connectivity"] = json.dumps(False) + mongos_host = self.harness.charm.get_mongos_host() + self.assertEqual(mongos_host, MONGOS_SOCKET_URI_FMT) + + self.harness.charm.app_peer_data["external-connectivity"] = json.dumps(True) + mongos_host = self.harness.charm.get_mongos_host() + self.assertEqual(mongos_host, "1.1.1.1") diff --git a/tests/unit/test_config_server_lib.py b/tests/unit/test_config_server_lib.py index d24c38f2..bb85146d 100644 --- a/tests/unit/test_config_server_lib.py +++ b/tests/unit/test_config_server_lib.py @@ -48,14 +48,18 @@ def setUp(self): @patch("ops.framework.EventBase.defer") @patch("charm.ClusterRequirer.update_keyfile") def test_on_relation_changed_waits_keyfile(self, update_keyfile, defer): - """Tests that relation changed waits for keyfile.""" + """Tests that relation changed does not wait for keyfile. + + When mongos is incorrectly integrated with a non-config server (ie shard), it can end up + waiting forever for a keyfile + """ # fails due to being run on non-config-server - relation_id = self.harness.add_relation("cluster", "config-server") - self.harness.add_relation_unit(relation_id, "config-server/0") - self.harness.update_relation_data(relation_id, "config-server/0", PEER_ADDR) + relation_id = self.harness.add_relation("cluster", "shard") + self.harness.add_relation_unit(relation_id, "shard/0") + self.harness.update_relation_data(relation_id, "shard/0", PEER_ADDR) update_keyfile.assert_not_called() - defer.assert_called() + defer.assert_not_called() @patch("charm.MongosOperatorCharm.push_file_to_unit") @patch("charm.MongosOperatorCharm.get_keyfile_contents") diff --git a/tests/unit/test_mongos_client_lib.py b/tests/unit/test_mongos_client_lib.py new file mode 100644 index 00000000..a5581719 --- /dev/null +++ b/tests/unit/test_mongos_client_lib.py @@ -0,0 +1,65 @@ +# Copyright 2023 Canonical Ltd. +# See LICENSE file for licensing details. +import unittest + +from unittest.mock import patch + +from ops.testing import Harness + +from charm import MongosOperatorCharm + +from .helpers import patch_network_get + +from charms.data_platform_libs.v0.data_interfaces import DatabaseRequiresEvents + +PEER_ADDR = {"private-address": "127.4.5.6"} +REL_DATA = { + "database": "database", + "extra-user-roles": "admin", +} +EXTERNAL_CONNECTIVITY_TAG = "external-node-connectivity" + +MONGOS_VAR = "MONGOS_ARGS=--configdb config-server-db/host:port" + +CLUSTER_ALIAS = "cluster" + + +class TestMongosInterface(unittest.TestCase): + @patch_network_get(private_address="1.1.1.1") + def setUp(self): + try: + # runs before each test to delete the custom events created for the aliases. This is + # needed because the events are created again in the next test, which causes an error + # related to duplicated events. + delattr(DatabaseRequiresEvents, f"{CLUSTER_ALIAS}_database_created") + delattr(DatabaseRequiresEvents, f"{CLUSTER_ALIAS}_endpoints_changed") + delattr( + DatabaseRequiresEvents, f"{CLUSTER_ALIAS}_read_only_endpoints_changed" + ) + except AttributeError: + # Ignore the events not existing before the first test. + pass + + self.harness = Harness(MongosOperatorCharm) + self.harness.begin() + self.harness.add_relation("router-peers", "router-peers") + self.harness.set_leader(True) + self.charm = self.harness.charm + self.addCleanup(self.harness.cleanup) + + @patch("charm.MongosOperatorCharm.open_mongos_port") + def test_mongos_opens_port_external(self, open_mongos_port): + """Tests that relation changed does not wait for keyfile. + + When mongos is incorrectly integrated with a non-config server (ie shard), it can end up + waiting forever for a keyfile + """ + # fails due to being run on non-config-server + relation_id = self.harness.add_relation("mongos_proxy", "host-charm") + self.harness.add_relation_unit(relation_id, "host-charm/0") + self.harness.update_relation_data(relation_id, "host-charm", REL_DATA) + open_mongos_port.assert_not_called() + + REL_DATA[EXTERNAL_CONNECTIVITY_TAG] = "true" + self.harness.update_relation_data(relation_id, "host-charm", REL_DATA) + open_mongos_port.assert_called()