diff --git a/lib/charms/mongodb/v0/mongodb_tls.py b/lib/charms/mongodb/v0/mongodb_tls.py index a609ddcc1..243a4b89a 100644 --- a/lib/charms/mongodb/v0/mongodb_tls.py +++ b/lib/charms/mongodb/v0/mongodb_tls.py @@ -24,6 +24,13 @@ from ops.framework import Object from ops.model import ActiveStatus, MaintenanceStatus, Unit +from config import Config + +APP_SCOPE = Config.Relations.APP_SCOPE +UNIT_SCOPE = Config.Relations.UNIT_SCOPE +Scopes = Config.Relations.Scopes + + # The unique Charmhub library identifier, never change it LIBID = "e02a50f0795e4dd292f58e93b4f493dd" @@ -33,39 +40,44 @@ # Increment this PATCH version before using `charmcraft publish-lib` or reset # to 0 if you are raising the major API version LIBPATCH = 5 - +LIBPATCH = 6 logger = logging.getLogger(__name__) -TLS_RELATION = "certificates" class MongoDBTLS(Object): """In this class we manage client database relations.""" - def __init__(self, charm, peer_relation, substrate="k8s"): + def __init__(self, charm, peer_relation, substrate): """Manager of MongoDB client relations.""" super().__init__(charm, "client-relations") self.charm = charm self.substrate = substrate self.peer_relation = peer_relation - self.certs = TLSCertificatesRequiresV1(self.charm, TLS_RELATION) + self.certs = TLSCertificatesRequiresV1(self.charm, Config.TLS.TLS_PEER_RELATION) self.framework.observe( self.charm.on.set_tls_private_key_action, self._on_set_tls_private_key ) self.framework.observe( - self.charm.on[TLS_RELATION].relation_joined, self._on_tls_relation_joined + self.charm.on[Config.TLS.TLS_PEER_RELATION].relation_joined, + self._on_tls_relation_joined, ) self.framework.observe( - self.charm.on[TLS_RELATION].relation_broken, self._on_tls_relation_broken + self.charm.on[Config.TLS.TLS_PEER_RELATION].relation_broken, + self._on_tls_relation_broken, ) self.framework.observe(self.certs.on.certificate_available, self._on_certificate_available) self.framework.observe(self.certs.on.certificate_expiring, self._on_certificate_expiring) + def is_tls_enabled(self, scope: Scopes): + """Returns a boolean indicating if TLS for a given `scope` is enabled.""" + return self.charm.get_secret(scope, Config.TLS.SECRET_CERT_LABEL) is not None + def _on_set_tls_private_key(self, event: ActionEvent) -> None: """Set the TLS private key, which will be used for requesting the certificate.""" logger.debug("Request to set TLS private key received.") try: - self._request_certificate("unit", event.params.get("external-key", None)) + self._request_certificate(UNIT_SCOPE, event.params.get("external-key", None)) if not self.charm.unit.is_leader(): event.log( @@ -73,12 +85,12 @@ def _on_set_tls_private_key(self, event: ActionEvent) -> None: ) return - self._request_certificate("app", event.params.get("internal-key", None)) + self._request_certificate(APP_SCOPE, event.params.get("internal-key", None)) logger.debug("Successfully set TLS private key.") except ValueError as e: event.fail(str(e)) - def _request_certificate(self, scope: str, param: Optional[str]): + def _request_certificate(self, scope: Scopes, param: Optional[str]): if param is None: key = generate_private_key() else: @@ -92,11 +104,11 @@ def _request_certificate(self, scope: str, param: Optional[str]): sans_ip=[str(self.charm.model.get_binding(self.peer_relation).network.bind_address)], ) - self.charm.set_secret(scope, "key", key.decode("utf-8")) - self.charm.set_secret(scope, "csr", csr.decode("utf-8")) - self.charm.set_secret(scope, "cert", None) + self.charm.set_secret(scope, Config.TLS.SECRET_KEY_LABEL, key.decode("utf-8")) + self.charm.set_secret(scope, Config.TLS.SECRET_CSR_LABEL, csr.decode("utf-8")) + self.charm.set_secret(scope, Config.TLS.SECRET_CERT_LABEL, None) - if self.charm.model.get_relation(TLS_RELATION): + if self.charm.model.get_relation(Config.TLS.TLS_PEER_RELATION): self.certs.request_certificate_creation(certificate_signing_request=csr) @staticmethod @@ -117,63 +129,59 @@ def _parse_tls_file(raw_content: str) -> bytes: def _on_tls_relation_joined(self, _: RelationJoinedEvent) -> None: """Request certificate when TLS relation joined.""" if self.charm.unit.is_leader(): - self._request_certificate("app", None) + self._request_certificate(APP_SCOPE, None) - self._request_certificate("unit", None) + self._request_certificate(UNIT_SCOPE, None) def _on_tls_relation_broken(self, event: RelationBrokenEvent) -> None: """Disable TLS when TLS relation broken.""" logger.debug("Disabling external TLS for unit: %s", self.charm.unit.name) - self.charm.set_secret("unit", "ca", None) - self.charm.set_secret("unit", "cert", None) - self.charm.set_secret("unit", "chain", None) + self.charm.set_secret(UNIT_SCOPE, Config.TLS.SECRET_CA_LABEL, None) + self.charm.set_secret(UNIT_SCOPE, Config.TLS.SECRET_CERT_LABEL, None) + self.charm.set_secret(UNIT_SCOPE, Config.TLS.SECRET_CHAIN_LABEL, None) + if self.charm.unit.is_leader(): logger.debug("Disabling internal TLS") - self.charm.set_secret("app", "ca", None) - self.charm.set_secret("app", "cert", None) - self.charm.set_secret("app", "chain", None) - if self.charm.get_secret("app", "cert"): + self.charm.set_secret(APP_SCOPE, Config.TLS.SECRET_CA_LABEL, None) + self.charm.set_secret(APP_SCOPE, Config.TLS.SECRET_CERT_LABEL, None) + self.charm.set_secret(APP_SCOPE, Config.TLS.SECRET_CHAIN_LABEL, None) + + if self.charm.get_secret(APP_SCOPE, Config.TLS.SECRET_CERT_LABEL): logger.debug( "Defer until the leader deletes the internal TLS certificate to avoid second restart." ) event.defer() return - logger.debug("Restarting mongod with TLS disabled.") - if self.substrate == "vm": - self.charm.unit.status = MaintenanceStatus("disabling TLS") - self.charm.restart_mongod_service() - self.charm.unit.status = ActiveStatus() - else: - self.charm.on_mongod_pebble_ready(event) + logger.info("Restarting mongod with TLS disabled.") + self.charm.unit.status = MaintenanceStatus("disabling TLS") + self.charm.delete_tls_certificate_from_workload() + self.charm.restart_mongod_service() + self.charm.unit.status = ActiveStatus() def _on_certificate_available(self, event: CertificateAvailableEvent) -> None: """Enable TLS when TLS certificate available.""" - if ( - event.certificate_signing_request.rstrip() - == self.charm.get_secret("unit", "csr").rstrip() - ): + unit_csr = self.charm.get_secret(UNIT_SCOPE, Config.TLS.SECRET_CSR_LABEL) + app_csr = self.charm.get_secret(APP_SCOPE, Config.TLS.SECRET_CSR_LABEL) + + if unit_csr and event.certificate_signing_request.rstrip() == unit_csr.rstrip(): logger.debug("The external TLS certificate available.") - scope = "unit" # external crs - elif ( - event.certificate_signing_request.rstrip() - == self.charm.get_secret("app", "csr").rstrip() - ): + scope = UNIT_SCOPE # external crs + elif app_csr and event.certificate_signing_request.rstrip() == app_csr.rstrip(): logger.debug("The internal TLS certificate available.") - scope = "app" # internal crs + scope = APP_SCOPE # internal crs else: - logger.error("An unknown certificate available.") + logger.error("An unknown certificate is available -- ignoring.") return - old_cert = self.charm.get_secret(scope, "cert") - renewal = old_cert and old_cert != event.certificate - - if scope == "unit" or (scope == "app" and self.charm.unit.is_leader()): + if scope == UNIT_SCOPE or (scope == APP_SCOPE and self.charm.unit.is_leader()): self.charm.set_secret( - scope, "chain", "\n".join(event.chain) if event.chain is not None else None + scope, + Config.TLS.SECRET_CHAIN_LABEL, + "\n".join(event.chain) if event.chain is not None else None, ) - self.charm.set_secret(scope, "cert", event.certificate) - self.charm.set_secret(scope, "ca", event.ca) + self.charm.set_secret(scope, Config.TLS.SECRET_CERT_LABEL, event.certificate) + self.charm.set_secret(scope, Config.TLS.SECRET_CA_LABEL, event.ca) if self._waiting_for_certs(): logger.debug( @@ -182,24 +190,20 @@ def _on_certificate_available(self, event: CertificateAvailableEvent) -> None: event.defer() return - if renewal and self.substrate == "k8s": - self.charm.unit.get_container("mongod").stop("mongod") + logger.info("Restarting mongod with TLS enabled.") - logger.debug("Restarting mongod with TLS enabled.") - if self.substrate == "vm": - self.charm._push_tls_certificate_to_workload() - self.charm.unit.status = MaintenanceStatus("enabling TLS") - self.charm.restart_mongod_service() - self.charm.unit.status = ActiveStatus() - else: - self.charm.on_mongod_pebble_ready(event) + self.charm.delete_tls_certificate_from_workload() + self.charm.push_tls_certificate_to_workload() + self.charm.unit.status = MaintenanceStatus("enabling TLS") + self.charm.restart_mongod_service() + self.charm.unit.status = ActiveStatus() def _waiting_for_certs(self): """Returns a boolean indicating whether additional certs are needed.""" - if not self.charm.get_secret("app", "cert"): + if not self.charm.get_secret(APP_SCOPE, Config.TLS.SECRET_CERT_LABEL): logger.debug("Waiting for application certificate.") return True - if not self.charm.get_secret("unit", "cert"): + if not self.charm.get_secret(UNIT_SCOPE, Config.TLS.SECRET_CERT_LABEL): logger.debug("Waiting for application certificate.") return True @@ -207,21 +211,27 @@ def _waiting_for_certs(self): def _on_certificate_expiring(self, event: CertificateExpiringEvent) -> None: """Request the new certificate when old certificate is expiring.""" - if event.certificate.rstrip() == self.charm.get_secret("unit", "cert").rstrip(): + if ( + event.certificate.rstrip() + == self.charm.get_secret(UNIT_SCOPE, Config.TLS.SECRET_CERT_LABEL).rstrip() + ): logger.debug("The external TLS certificate expiring.") - scope = "unit" # external cert - elif event.certificate.rstrip() == self.charm.get_secret("app", "cert").rstrip(): + scope = UNIT_SCOPE # external cert + elif ( + event.certificate.rstrip() + == self.charm.get_secret(APP_SCOPE, Config.TLS.SECRET_CERT_LABEL).rstrip() + ): logger.debug("The internal TLS certificate expiring.") if not self.charm.unit.is_leader(): return - scope = "app" # internal cert + scope = APP_SCOPE # internal cert else: logger.error("An unknown certificate expiring.") return logger.debug("Generating a new Certificate Signing Request.") - key = self.charm.get_secret(scope, "key").encode("utf-8") - old_csr = self.charm.get_secret(scope, "csr").encode("utf-8") + key = self.charm.get_secret(scope, Config.TLS.SECRET_KEY_LABEL).encode("utf-8") + old_csr = self.charm.get_secret(scope, Config.TLS.SECRET_CSR_LABEL).encode("utf-8") new_csr = generate_csr( private_key=key, subject=self.get_host(self.charm.unit), @@ -236,7 +246,7 @@ def _on_certificate_expiring(self, event: CertificateExpiringEvent) -> None: new_certificate_signing_request=new_csr, ) - self.charm.set_secret(scope, "csr", new_csr.decode("utf-8")) + self.charm.set_secret(scope, Config.TLS.SECRET_CSR_LABEL, new_csr.decode("utf-8")) def _get_sans(self) -> List[str]: """Create a list of DNS names for a MongoDB unit. @@ -252,19 +262,24 @@ def _get_sans(self) -> List[str]: str(self.charm.model.get_binding(self.peer_relation).network.bind_address), ] - def get_tls_files(self, scope: str) -> Tuple[Optional[str], Optional[str]]: + def get_tls_files(self, scope: Scopes) -> Tuple[Optional[str], Optional[str]]: """Prepare TLS files in special MongoDB way. MongoDB needs two files: — CA file should have a full chain. — PEM file should have private key and certificate without certificate chain. """ - ca = self.charm.get_secret(scope, "ca") - chain = self.charm.get_secret(scope, "chain") + if not self.is_tls_enabled(scope): + logging.debug(f"TLS disabled for {scope}") + return None, None + logging.debug(f"TLS *enabled* for {scope}, fetching data for CA and PEM files ") + + ca = self.charm.get_secret(scope, Config.TLS.SECRET_CA_LABEL) + chain = self.charm.get_secret(scope, Config.TLS.SECRET_CHAIN_LABEL) ca_file = chain if chain else ca - key = self.charm.get_secret(scope, "key") - cert = self.charm.get_secret(scope, "cert") + key = self.charm.get_secret(scope, Config.TLS.SECRET_KEY_LABEL) + cert = self.charm.get_secret(scope, Config.TLS.SECRET_CERT_LABEL) pem_file = key if cert: pem_file = key + "\n" + cert if key else cert @@ -276,4 +291,4 @@ def get_host(self, unit: Unit): if self.substrate == "vm": return self.charm._unit_ip(unit) else: - return self.charm.get_hostname_by_unit(unit.name) + return self.charm.get_hostname_for_unit(unit) diff --git a/src/charm.py b/src/charm.py index a6c1effcc..8b07daed0 100755 --- a/src/charm.py +++ b/src/charm.py @@ -4,6 +4,7 @@ # See LICENSE file for licensing details. import json import logging +import re import subprocess import time from typing import Dict, List, Optional, Set @@ -39,6 +40,7 @@ OperatorUser, ) from charms.operator_libs_linux.v1 import snap +from ops import JujuVersion from ops.charm import ( ActionEvent, CharmBase, @@ -47,6 +49,8 @@ RelationDepartedEvent, RelationEvent, RelationJoinedEvent, + SecretChangedEvent, + SecretRemoveEvent, StartEvent, StorageDetachingEvent, UpdateStatusEvent, @@ -57,17 +61,30 @@ BlockedStatus, MaintenanceStatus, Relation, + SecretNotFoundError, Unit, WaitingStatus, ) from tenacity import Retrying, before_log, retry, stop_after_attempt, wait_fixed from config import Config -from exceptions import AdminUserCreationError, ApplicationHostNotFoundError -from machine_helpers import push_file_to_unit, update_mongod_service +from exceptions import ( + AdminUserCreationError, + ApplicationHostNotFoundError, + SecretNotAddedError, +) +from machine_helpers import ( + push_file_to_unit, + remove_file_from_unit, + update_mongod_service, +) logger = logging.getLogger(__name__) +APP_SCOPE = Config.Relations.APP_SCOPE +UNIT_SCOPE = Config.Relations.UNIT_SCOPE +Scopes = Config.Relations.Scopes + class MongodbOperatorCharm(CharmBase): """Charm the service.""" @@ -98,6 +115,10 @@ def __init__(self, *args): self.framework.observe(self.on.get_password_action, self._on_get_password) self.framework.observe(self.on.set_password_action, self._on_set_password) + # secrets + self.framework.observe(self.on.secret_remove, self._on_secret_remove) + self.framework.observe(self.on.secret_changed, self._on_secret_changed) + # handle provider side of relations self.client_relations = MongoDBProvider(self, substrate=Config.SUBSTRATE) self.legacy_client_relations = MongoDBLegacyProvider(self) @@ -113,6 +134,8 @@ def __init__(self, *args): log_slots=Config.Monitoring.LOG_SLOTS, ) + self.secrets = {APP_SCOPE: {}, UNIT_SCOPE: {}} + # BEGIN: properties @property @@ -184,20 +207,18 @@ def backup_config(self) -> MongoDBConfiguration: @property def unit_peer_data(self) -> Dict: """Peer relation data object.""" - relation = self.model.get_relation(Config.Relations.PEERS) - if relation is None: + if not self._peers: return {} - return relation.data[self.unit] + return self._peers.data[self.unit] @property def app_peer_data(self) -> Dict: """Peer relation data object.""" - relation = self.model.get_relation(Config.Relations.PEERS) - if not relation: + if not self._peers: return {} - return relation.data[self.app] + return self._peers.data[self.app] @property def _peers(self) -> Optional[Relation]: @@ -223,6 +244,10 @@ def db_initialised(self, value): f"'db_initialised' must be a boolean value. Proivded: {value} is of type {type(value)}" ) + @property + def _juju_has_secrets(self) -> bool: + return JujuVersion.from_environ().has_secrets + # END: properties # BEGIN: charm event handlers @@ -267,7 +292,7 @@ def _on_start(self, event: StartEvent) -> None: """ # mongod requires keyFile and TLS certificates on the file system self._instatiate_keyfile(event) - self._push_tls_certificate_to_workload() + self.push_tls_certificate_to_workload() try: logger.debug("starting MongoDB.") @@ -381,7 +406,7 @@ def _on_relation_handler(self, event: RelationEvent) -> None: def _on_leader_elected(self, event: LeaderElectedEvent) -> None: """Generates necessary keyfile and updates replica hosts.""" - if not self.get_secret("app", "keyfile"): + if not self.get_secret(APP_SCOPE, Config.Secrets.SECRET_KEYFILE_NAME): self._generate_secrets() self._update_hosts(event) @@ -498,7 +523,9 @@ def _on_get_password(self, event: ActionEvent) -> None: if not username: return key_name = MongoDBUser.get_password_key_name_for_user(username) - event.set_results({Config.Actions.PASSWORD_PARAM_NAME: self.get_secret("app", key_name)}) + event.set_results( + {Config.Actions.PASSWORD_PARAM_NAME: self.get_secret(APP_SCOPE, key_name)} + ) def _on_set_password(self, event: ActionEvent) -> None: """Set the password for the admin user.""" @@ -520,6 +547,11 @@ def _on_set_password(self, event: ActionEvent) -> None: return new_password = event.params.get(Config.Actions.PASSWORD_PARAM_NAME, generate_password()) + if len(new_password) > Config.Secrets.MAX_PASSWORD_LENGTH: + event.fail( + f"Password cannot be longer than {Config.Secrets.MAX_PASSWORD_LENGTH} characters." + ) + return with MongoDBConnection(self.mongodb_config) as mongo: try: mongo.set_user_password(username, new_password) @@ -532,7 +564,9 @@ def _on_set_password(self, event: ActionEvent) -> None: event.fail(f"Failed changing the password: {e}") return - self.set_secret("app", MongoDBUser.get_password_key_name_for_user(username), new_password) + secret_id = self.set_secret( + APP_SCOPE, MongoDBUser.get_password_key_name_for_user(username), new_password + ) if username == BackupUser.get_username(): self._connect_pbm_agent() @@ -540,7 +574,35 @@ def _on_set_password(self, event: ActionEvent) -> None: if username == MonitorUser.get_username(): self._connect_mongodb_exporter() - event.set_results({Config.Actions.PASSWORD_PARAM_NAME: new_password}) + event.set_results( + {Config.Actions.PASSWORD_PARAM_NAME: new_password, "secret-id": secret_id} + ) + + def _on_secret_remove(self, event: SecretRemoveEvent): + # We are keeping this function empty on purpose until the issue with secrets + # is not fixed. The issue is: https://bugs.launchpad.net/juju/+bug/2023364 + logging.error( + f"_on_secret_remove: Secret {event._id} seems to have no observers, could be removed" + ) + + def _on_secret_changed(self, event: SecretChangedEvent): + if self._compare_secret_ids( + event.secret.id, self.app_peer_data.get(Config.Secrets.SECRET_INTERNAL_LABEL) + ): + scope = APP_SCOPE + elif self._compare_secret_ids( + event.secret.id, self.unit_peer_data.get(Config.Secrets.SECRET_INTERNAL_LABEL) + ): + scope = UNIT_SCOPE + else: + logging.debug("Secret %s changed, but it's unknown", event.secret.id) + return + logging.debug("Secret %s for scope %s changed, refreshing", event.secret.id, scope) + self._update_juju_secrets_cache(scope) + + # changed secrets means that the URIs used for PBM and mongodb_exporter are now out of date + self._connect_mongodb_exporter() + self._connect_pbm_agent() # END: charm event handlers @@ -574,7 +636,7 @@ def _init_operator_user(self) -> None: raise AdminUserCreationError logger.debug(f"{OperatorUser.get_username()} user created") - self._user_created(OperatorUser) + self._set_user_created(OperatorUser) @retry( stop=stop_after_attempt(3), @@ -584,7 +646,7 @@ def _init_operator_user(self) -> None: ) def _init_monitor_user(self): """Creates the monitor user on the MongoDB database.""" - if self._user_created(MonitorUser): + if self._is_user_created(MonitorUser): return with MongoDBConnection(self.mongodb_config) as mongo: @@ -594,7 +656,7 @@ def _init_monitor_user(self): ) logger.debug("creating the monitor user...") mongo.create_user(self.monitor_config) - self._user_created(MonitorUser) + self._set_user_created(MonitorUser) # leader should reconnect to exporter after creating the monitor user - since the snap # will have an authorisation error until the the user has been created and the daemon @@ -620,7 +682,7 @@ def _init_backup_user(self): ) logger.debug("creating the backup user...") mongo.create_user(self.backup_config) - self._user_created(BackupUser) + self._set_user_created(BackupUser) # END: users management @@ -628,20 +690,20 @@ def _init_backup_user(self): def _is_user_created(self, user: MongoDBUser) -> bool: return f"{user.get_username()}-user-created" in self.app_peer_data - def _user_created(self, user: MongoDBUser) -> None: + def _set_user_created(self, user: MongoDBUser) -> None: self.app_peer_data[f"{user.get_username()}-user-created"] = "True" def _get_mongodb_config_for_user( self, user: MongoDBUser, hosts: Set[str] ) -> MongoDBConfiguration: - external_ca, _ = self.tls.get_tls_files("unit") - internal_ca, _ = self.tls.get_tls_files("app") + external_ca, _ = self.tls.get_tls_files(UNIT_SCOPE) + internal_ca, _ = self.tls.get_tls_files(APP_SCOPE) return MongoDBConfiguration( replset=self.app.name, database=user.get_database_name(), username=user.get_username(), - password=self.get_secret("app", user.get_password_key_name()), + password=self.get_secret(APP_SCOPE, user.get_password_key_name()), hosts=hosts, roles=user.get_roles(), tls_external=external_ca is not None, @@ -661,8 +723,8 @@ def _get_user_or_fail_event(self, event: ActionEvent, default_username: str) -> def _check_or_set_user_password(self, user: MongoDBUser) -> None: key = user.get_password_key_name() - if not self.get_secret("app", key): - self.set_secret("app", key, generate_password()) + if not self.get_secret(APP_SCOPE, key): + self.set_secret(APP_SCOPE, key, generate_password()) def _generate_secrets(self) -> None: """Generate secrets and put them into peer relation. @@ -673,8 +735,8 @@ def _generate_secrets(self) -> None: self._check_or_set_user_password(OperatorUser) self._check_or_set_user_password(MonitorUser) - if not self.get_secret("app", "keyfile"): - self.set_secret("app", "keyfile", generate_keyfile()) + if not self.get_secret(APP_SCOPE, Config.Secrets.SECRET_KEYFILE_NAME): + self.set_secret(APP_SCOPE, Config.Secrets.SECRET_KEYFILE_NAME, generate_keyfile()) def _update_hosts(self, event: LeaderElectedEvent) -> None: """Update replica set hosts and remove any unremoved replicas from the config.""" @@ -762,7 +824,7 @@ def _install_snap_packages(self, packages: List[str]) -> None: def _instatiate_keyfile(self, event: StartEvent) -> None: # wait for keyFile to be created by leader unit - if not self.get_secret("app", "keyfile"): + if not self.get_secret(APP_SCOPE, Config.Secrets.SECRET_KEYFILE_NAME): logger.debug("waiting for leader unit to generate keyfile contents") event.defer() return @@ -771,12 +833,12 @@ def _instatiate_keyfile(self, event: StartEvent) -> None: push_file_to_unit( parent_dir=Config.MONGOD_CONF_DIR, file_name=KEY_FILE, - file_contents=self.get_secret("app", "keyfile"), + file_contents=self.get_secret(APP_SCOPE, Config.Secrets.SECRET_KEYFILE_NAME), ) - def _push_tls_certificate_to_workload(self) -> None: + def push_tls_certificate_to_workload(self) -> None: """Uploads certificate to the workload container.""" - external_ca, external_pem = self.tls.get_tls_files("unit") + external_ca, external_pem = self.tls.get_tls_files(UNIT_SCOPE) if external_ca is not None: push_file_to_unit( parent_dir=Config.MONGOD_CONF_DIR, @@ -791,7 +853,7 @@ def _push_tls_certificate_to_workload(self) -> None: file_contents=external_pem, ) - internal_ca, internal_pem = self.tls.get_tls_files("app") + internal_ca, internal_pem = self.tls.get_tls_files(APP_SCOPE) if internal_ca is not None: push_file_to_unit( parent_dir=Config.MONGOD_CONF_DIR, @@ -806,13 +868,26 @@ def _push_tls_certificate_to_workload(self) -> None: file_contents=internal_pem, ) + @staticmethod + def delete_tls_certificate_from_workload() -> None: + """Deletes certificate from VM.""" + logger.info("Deleting TLS certificate from VM") + + for file in [ + Config.TLS.EXT_CA_FILE, + Config.TLS.EXT_PEM_FILE, + Config.TLS.INT_CA_FILE, + Config.TLS.INT_PEM_FILE, + ]: + remove_file_from_unit(Config.MONGOD_CONF_DIR, file) + def _connect_mongodb_exporter(self) -> None: """Exposes the endpoint to mongodb_exporter.""" if not self.db_initialised: return # must wait for leader to set URI before connecting - if not self.get_secret("app", MonitorUser.get_password_key_name()): + if not self.get_secret(APP_SCOPE, MonitorUser.get_password_key_name()): return snap_cache = snap.SnapCache() @@ -826,7 +901,7 @@ def _connect_pbm_agent(self) -> None: return # must wait for leader to set URI before any attempts to update are made - if not self.get_secret("app", BackupUser.get_password_key_name()): + if not self.get_secret(APP_SCOPE, BackupUser.get_password_key_name()): return snap_cache = snap.SnapCache() @@ -912,21 +987,33 @@ def _unit_ip(self, unit: Unit) -> str: def get_secret(self, scope: str, key: str) -> Optional[str]: """Get secret from the secret storage.""" - if scope == "unit": + if self._juju_has_secrets: + return self._juju_secret_get(scope, key) + + if scope == UNIT_SCOPE: return self.unit_peer_data.get(key, None) - elif scope == "app": + elif scope == APP_SCOPE: return self.app_peer_data.get(key, None) else: raise RuntimeError("Unknown secret scope.") - def set_secret(self, scope: str, key: str, value: Optional[str]) -> None: - """Set secret in the secret storage.""" - if scope == "unit": + def set_secret(self, scope: str, key: str, value: Optional[str]) -> Optional[str]: + """Set secret in the secret storage. + + Juju versions > 3.0 use `juju secrets`, this function first checks + which secret store is being used before setting the secret. + """ + if self._juju_has_secrets: + if not value: + return self._juju_secret_remove(scope, key) + return self._juju_secret_set(scope, key, value) + + if scope == UNIT_SCOPE: if not value: del self.unit_peer_data[key] return self.unit_peer_data.update({key: str(value)}) - elif scope == "app": + elif scope == APP_SCOPE: if not value: del self.app_peer_data[key] return @@ -1006,9 +1093,153 @@ def restart_backup_service(self) -> None: charmed_mongodb_snap = snap_cache["charmed-mongodb"] charmed_mongodb_snap.restart(services=["pbm-agent"]) + def _scope_obj(self, scope: Scopes): + if scope == APP_SCOPE: + return self.app + if scope == UNIT_SCOPE: + return self.unit + + def _peer_data(self, scope: Scopes): + if not self._peers: + return {}.setdefault(scope, {}) + scope_obj = self._scope_obj(scope) + return self._peers.data[scope_obj] + + @staticmethod + def _compare_secret_ids(secret_id1: str, secret_id2: str) -> bool: + """Reliable comparison on secret equality. + + NOTE: Secret IDs may be of any of these forms: + - secret://9663a790-7828-4186-8b21-2624c58b6cfe/citb87nubg2s766pab40 + - secret:citb87nubg2s766pab40 + """ + if not secret_id1 or not secret_id2: + return False + + regex = re.compile(".*[^/][/:]") + + pure_id1 = regex.sub("", secret_id1) + pure_id2 = regex.sub("", secret_id2) + + if pure_id1 and pure_id2: + return pure_id1 == pure_id2 + return False + + def _juju_secret_set(self, scope: Scopes, key: str, value: str) -> str: + """Helper function setting Juju secret in Juju versions >3.0.""" + peer_data = self._peer_data(scope) + self._update_juju_secrets_cache(scope) + + secret = self.secrets[scope].get(Config.Secrets.SECRET_LABEL) + + # It's not the first secret for the scope, we can re-use the existing one + # that was fetched in the previous call, as fetching secrets from juju is + # slow + if secret: + secret_cache = self.secrets[scope][Config.Secrets.SECRET_CACHE_LABEL] + + if secret_cache.get(key) == value: + logging.debug(f"Key {scope}:{key} has this value defined already") + else: + secret_cache[key] = value + try: + secret.set_content(secret_cache) + logging.debug(f"Secret {scope}:{key} was {key} set") + except OSError as error: + logging.error( + f"Error in attempt to set '{key}' secret for scope '{scope}'. " + f"Existing keys were: {list(secret_cache.keys())}. {error}" + ) + + # We need to create a brand-new secret for this scope + else: + scope_obj = self._scope_obj(scope) + + secret = scope_obj.add_secret({key: value}) + if not secret: + raise SecretNotAddedError(f"Couldn't set secret {scope}:{key}") + + self.secrets[scope][Config.Secrets.SECRET_LABEL] = secret + self.secrets[scope][Config.Secrets.SECRET_CACHE_LABEL] = {key: value} + logging.debug(f"Secret {scope}:{key} published (as first). ID: {secret.id}") + peer_data.update({Config.Secrets.SECRET_INTERNAL_LABEL: secret.id}) + + return self.secrets[scope][Config.Secrets.SECRET_LABEL].id + + def _update_juju_secrets_cache(self, scope: Scopes) -> None: + """Helper function to retrieve all Juju secrets. + + This function is responsible for direct communication with the Juju Secret + store to retrieve the Mono Charm's single, unique Secret object's metadata, + and --on success-- its contents. + In parallel with retrieving secret information, it's immediately locally cached, + making sure that we have the snapshot of the secret for the lifetime of the event + (that's being processed) without additional fetch requests to the Juju Secret Store. + + (Note: metadata, i.e. the Secret object itself is cached as it may be necessary for + later operations, like updating contents.) + + The function is returning a boolean that reflects success or failure of the above. + """ + peer_data = self._peer_data(scope) + + if not peer_data.get(Config.Secrets.SECRET_INTERNAL_LABEL): + return + + if Config.Secrets.SECRET_CACHE_LABEL not in self.secrets[scope]: + try: + # NOTE: Secret contents are not yet available! + secret = self.model.get_secret(id=peer_data[Config.Secrets.SECRET_INTERNAL_LABEL]) + except SecretNotFoundError as e: + logging.debug( + f"No secret found for ID {peer_data[Config.Secrets.SECRET_INTERNAL_LABEL]}, {e}" + ) + return + + logging.debug(f"Secret {peer_data[Config.Secrets.SECRET_INTERNAL_LABEL]} downloaded") + + # We keep the secret object around -- needed when applying modifications + self.secrets[scope][Config.Secrets.SECRET_LABEL] = secret + + # We retrieve and cache actual secret data for the lifetime of the event scope + self.secrets[scope][Config.Secrets.SECRET_CACHE_LABEL] = secret.get_content() + + def _get_juju_secrets_cache(self, scope: Scopes): + return self.secrets[scope].get(Config.Secrets.SECRET_CACHE_LABEL) + + def _juju_secret_get(self, scope: Scopes, key: str) -> Optional[str]: + """Helper function to get Juju secret.""" + if not key: + return + + self._update_juju_secrets_cache(scope) + secret_cache = self._get_juju_secrets_cache(scope) + if secret_cache: + secret_data = secret_cache.get(key) + if secret_data and secret_data != Config.Secrets.SECRET_DELETED_LABEL: + logging.debug(f"Getting secret {scope}:{key}") + return secret_data + logging.debug(f"No value found for secret {scope}:{key}") + + def _juju_secret_remove(self, scope: Scopes, key: str) -> None: + """Remove a Juju 3.x secret.""" + self._update_juju_secrets_cache(scope) + + secret = self.secrets[scope].get(Config.Secrets.SECRET_LABEL) + if not secret: + logging.error(f"Secret {scope}:{key} wasn't deleted: no secrets are available") + return + + secret_cache = self.secrets[scope].get(Config.Secrets.SECRET_CACHE_LABEL) + if not secret_cache or key not in secret_cache: + logging.error(f"No secret {scope}:{key}") + return + + secret_cache[key] = Config.Secrets.SECRET_DELETED_LABEL + secret.set_content(secret_cache) + logging.debug(f"Secret {scope}:{key}") -# pbm_snap.restart(services=["pbm-agent"]) -# END: helper functions + # END: helper functions if __name__ == "__main__": diff --git a/src/config.py b/src/config.py index 8d8279f87..2d24f9f80 100644 --- a/src/config.py +++ b/src/config.py @@ -3,6 +3,9 @@ # See LICENSE file for licensing details. +from typing import Literal + + class Config: """Configuration for MongoDB Charm.""" @@ -40,9 +43,38 @@ class Monitoring: URI_PARAM_NAME = "monitor-uri" SERVICE_NAME = "mongodb-exporter" + class TLS: + """TLS related config for MongoDB Charm.""" + + EXT_PEM_FILE = "external-cert.pem" + EXT_CA_FILE = "external-ca.crt" + INT_PEM_FILE = "internal-cert.pem" + INT_CA_FILE = "internal-ca.crt" + KEY_FILE_NAME = "keyFile" + TLS_PEER_RELATION = "certificates" + + SECRET_CA_LABEL = "ca-secret" + SECRET_KEY_LABEL = "key-secret" + SECRET_CERT_LABEL = "cert-secret" + SECRET_CSR_LABEL = "csr-secret" + SECRET_CHAIN_LABEL = "chain-secret" + class Relations: """Relations related config for MongoDB Charm.""" NAME = "database" PEERS = "database-peers" OBSOLETE_RELATIONS_NAME = "obsolete" + APP_SCOPE = "app" + UNIT_SCOPE = "unit" + Scopes = Literal[APP_SCOPE, UNIT_SCOPE] + + class Secrets: + """Secrets related constants.""" + + SECRET_LABEL = "secret" + SECRET_CACHE_LABEL = "cache" + SECRET_KEYFILE_NAME = "keyfile" + SECRET_INTERNAL_LABEL = "internal-secret" + SECRET_DELETED_LABEL = "None" + MAX_PASSWORD_LENGTH = 4096 diff --git a/src/exceptions.py b/src/exceptions.py index cb78b3dbf..022182a4b 100644 --- a/src/exceptions.py +++ b/src/exceptions.py @@ -4,13 +4,37 @@ # See LICENSE file for licensing details. -class AdminUserCreationError(Exception): +class MongoError(Exception): + """Common parent for Mongo errors, allowing to catch them all at once.""" + + pass + + +class AdminUserCreationError(MongoError): """Raised when a commands to create an admin user on MongoDB fail.""" pass -class ApplicationHostNotFoundError(Exception): +class ApplicationHostNotFoundError(MongoError): """Raised when a queried host is not in the application peers or the current host.""" pass + + +class MongoSecretError(MongoError): + """Common parent for all Mongo Secret Exceptions.""" + + pass + + +class SecretNotAddedError(MongoSecretError): + """Raised when a Juju 3 secret couldn't be set or re-set.""" + + pass + + +class MissingSecretError(MongoSecretError): + """Could be raised when a Juju 3 mandatory secret couldn't be found.""" + + pass diff --git a/src/machine_helpers.py b/src/machine_helpers.py index eab2050e8..ba4b18fc8 100644 --- a/src/machine_helpers.py +++ b/src/machine_helpers.py @@ -50,9 +50,15 @@ def push_file_to_unit(parent_dir, file_name, file_contents) -> None: # MongoDB limitation; it is needed 400 rights for keyfile and we need 440 rights on tls certs # to be able to connect via MongoDB shell - if "keyFile" in file_name: + if Config.TLS.KEY_FILE_NAME in file_name: os.chmod(file_name, 0o400) else: os.chmod(file_name, 0o440) mongodb_user = pwd.getpwnam(MONGO_USER) os.chown(file_name, mongodb_user.pw_uid, ROOT_USER_GID) + + +def remove_file_from_unit(parent_dir, file_name) -> None: + """Remove file from vm unit.""" + if os.path.exists(f"{parent_dir}/{file_name}"): + os.remove(f"{parent_dir}/{file_name}") diff --git a/tests/conftest.py b/tests/conftest.py new file mode 100644 index 000000000..34259017c --- /dev/null +++ b/tests/conftest.py @@ -0,0 +1,44 @@ +# Copyright 2023 Canonical Ltd. +# See LICENSE file for licensing details. +from importlib.metadata import version +from unittest.mock import PropertyMock + +import pytest +from ops import JujuVersion +from pytest_mock import MockerFixture + + +@pytest.fixture(autouse=True) +def juju_has_secrets(mocker: MockerFixture): + """This fixture will force the usage of secrets whenever run on Juju 3.x. + + NOTE: This is needed, as normally JujuVersion is set to 0.0.0 in tests + (i.e. not the real juju version) + """ + if version("juju") < "3": + mocker.patch.object( + JujuVersion, "has_secrets", new_callable=PropertyMock + ).return_value = False + return False + else: + mocker.patch.object( + JujuVersion, "has_secrets", new_callable=PropertyMock + ).return_value = True + return True + + +@pytest.fixture +def only_with_juju_secrets(juju_has_secrets): + """Pretty way to skip Juju 3 tests.""" + if not juju_has_secrets: + pytest.skip("Secrets test only applies on Juju 3.x") + + +@pytest.fixture +def only_without_juju_secrets(juju_has_secrets): + """Pretty way to skip Juju 2-specific tests. + + Typically: to save CI time, when the same check were executed in a Juju 3-specific way already + """ + if juju_has_secrets: + pytest.skip("Skipping legacy secrets tests") diff --git a/tests/integration/ha_tests/helpers.py b/tests/integration/ha_tests/helpers.py index 2478945f8..fde1f58cc 100644 --- a/tests/integration/ha_tests/helpers.py +++ b/tests/integration/ha_tests/helpers.py @@ -487,7 +487,6 @@ async def kill_unit_process(ops_test: OpsTest, unit_name: str, kill_code: str): if len(ops_test.model.applications[app].units) < 2: await ops_test.model.applications[app].add_unit(count=1) await ops_test.model.wait_for_idle(apps=[app], status="active", timeout=1000) - kill_cmd = f"exec --unit {unit_name} -- pkill --signal {kill_code} -f {DB_PROCESS}" return_code, _, _ = await ops_test.juju(*kill_cmd.split()) diff --git a/tests/integration/helpers.py b/tests/integration/helpers.py index d97d06d9a..755f6e464 100644 --- a/tests/integration/helpers.py +++ b/tests/integration/helpers.py @@ -77,3 +77,26 @@ async def find_unit(ops_test: OpsTest, leader: bool, app=APP_NAME) -> ops.model. ret_unit = unit return ret_unit + + +async def get_leader_id(ops_test: OpsTest) -> int: + """Returns the unit number of the juju leader unit.""" + for unit in ops_test.model.applications[APP_NAME].units: + if await unit.is_leader_from_status(): + return int(unit.name.split("/")[1]) + return -1 + + +async def set_password( + ops_test: OpsTest, unit_id: int, username: str = "operator", password: str = "secret" +) -> str: + """Use the charm action to retrieve the password from provided unit. + + Returns: + String with the password stored on the peer relation databag. + """ + action = await ops_test.model.units.get(f"{APP_NAME}/{unit_id}").run_action( + "set-password", **{"username": username, "password": password} + ) + action = await action.wait() + return action.results diff --git a/tests/integration/metrics_tests/test_metrics.py b/tests/integration/metrics_tests/test_metrics.py index 282b55d63..090d8b570 100644 --- a/tests/integration/metrics_tests/test_metrics.py +++ b/tests/integration/metrics_tests/test_metrics.py @@ -59,7 +59,7 @@ async def test_endpoints_new_password(ops_test: OpsTest): action = await action.wait() # wait for non-leader units to receive relation changed event. time.sleep(3) - await ops_test.model.wait_for_idle() + await ops_test.model.wait_for_idle(apps=[app], status="active", idle_period=15) for unit in application.units: await verify_endpoints(ops_test, unit) diff --git a/tests/integration/test_charm.py b/tests/integration/test_charm.py index c7e1450fd..0f75d43ce 100644 --- a/tests/integration/test_charm.py +++ b/tests/integration/test_charm.py @@ -2,9 +2,11 @@ # Copyright 2023 Canonical Ltd. # See LICENSE file for licensing details. +import json import logging import os import time +from uuid import uuid4 import pytest from pymongo import MongoClient @@ -19,7 +21,9 @@ UNIT_IDS, count_primaries, find_unit, + get_leader_id, get_password, + set_password, unit_uri, ) @@ -176,13 +180,96 @@ async def test_monitor_user(ops_test: OpsTest) -> None: ] hosts = ",".join(replica_set_hosts) replica_set_uri = f"mongodb://monitor:{password}@{hosts}/admin?replicaSet=mongodb" - admin_mongod_cmd = f"charmed-mongodb.mongo '{replica_set_uri}' --eval 'rs.conf()'" check_monitor_cmd = f"exec --unit {unit.name} -- {admin_mongod_cmd}" return_code, _, _ = await ops_test.juju(*check_monitor_cmd.split()) assert return_code == 0, "command rs.conf() on monitor user does not work" +async def test_only_leader_can_set_while_all_can_read_password_secret(ops_test: OpsTest) -> None: + """Test verifies that only the leader can set a password, while all units can read it.""" + # Setting existing password + leader_id = await get_leader_id(ops_test) + non_leaders = list(UNIT_IDS) + non_leaders.remove(leader_id) + + password = "blablabla" + await set_password(ops_test, unit_id=non_leaders[0], username="monitor", password=password) + password1 = await get_password(ops_test, username="monitor") + assert password1 != password + + await set_password(ops_test, unit_id=leader_id, username="monitor", password=password) + for _ in UNIT_IDS: + password2 = await get_password(ops_test, username="monitor") + assert password2 == password + + +@pytest.mark.usefixtures("only_with_juju_secrets") +async def test_reset_and_get_password_secret_same_as_cli(ops_test: OpsTest) -> None: + """Test verifies that we can set and retrieve the correct password using Juju 3.x secrets.""" + new_password = str(uuid4()) + + # Resetting existing password + leader_id = await get_leader_id(ops_test) + result = await set_password( + ops_test, unit_id=leader_id, username="monitor", password=new_password + ) + + secret_id = result["secret-id"].split("/")[-1] + + # Getting back the pw programmatically + password = await get_password(ops_test, username="monitor") + + # Getting back the pw from juju CLI + complete_command = f"show-secret {secret_id} --reveal --format=json" + _, stdout, _ = await ops_test.juju(*complete_command.split()) + data = json.loads(stdout) + + assert password == new_password + assert data[secret_id]["content"]["Data"]["monitor-password"] == password + + +@pytest.mark.usefixtures("only_without_juju_secrets") +async def test_reset_and_get_password_no_secret(ops_test: OpsTest, mocker) -> None: + """Test verifies that we can set and retrieve the correct password using Juju 2.x.""" + new_password = str(uuid4()) + + # Re=setting existing password + leader_id = await get_leader_id(ops_test) + await set_password(ops_test, unit_id=leader_id, username="monitor", password=new_password) + + # Getting back the pw programmatically + password = await get_password(ops_test, username="monitor") + assert password == new_password + + +@pytest.mark.usefixtures("only_with_juju_secrets") +async def test_empty_password(ops_test: OpsTest) -> None: + """Test that the password can't be set to an empty string.""" + leader_id = await get_leader_id(ops_test) + + password1 = await get_password(ops_test, username="monitor") + await set_password(ops_test, unit_id=leader_id, username="monitor", password="") + password2 = await get_password(ops_test, username="monitor") + + # The password remained unchanged + assert password1 == password2 + + +@pytest.mark.usefixtures("only_with_juju_secrets") +async def test_no_password_change_on_invalid_password(ops_test: OpsTest) -> None: + """Test that in general, there is no change when password validation fails.""" + leader_id = await get_leader_id(ops_test) + password1 = await get_password(ops_test, username="monitor") + + # The password has to be minimum 3 characters + await set_password(ops_test, unit_id=leader_id, username="monitor", password="ca" * 1000000) + password2 = await get_password(ops_test, username="monitor") + + # The password didn't change + assert password1 == password2 + + async def test_exactly_one_primary_reported_by_juju(ops_test: OpsTest) -> None: """Tests that there is exactly one replica set primary unit reported by juju.""" diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py index 6160d1483..f11486c7e 100644 --- a/tests/unit/test_charm.py +++ b/tests/unit/test_charm.py @@ -38,6 +38,7 @@ def setUp(self, *unused): self.harness.begin() self.peer_rel_id = self.harness.add_relation("database-peers", "database-peers") + @patch("charm.MongodbOperatorCharm.get_secret") @patch_network_get(private_address="1.1.1.1") @patch("charm.MongoDBConnection") @patch("charm.MongodbOperatorCharm._init_operator_user") @@ -46,7 +47,7 @@ def setUp(self, *unused): @patch("charm.push_file_to_unit") @patch("builtins.open") def test_on_start_not_leader_doesnt_initialise_replica_set( - self, open, path, snap, _open_port_tcp, init_admin, connection + self, open, path, snap, _open_port_tcp, init_admin, connection, get_secret ): """Tests that a non leader unit does not initialise the replica set.""" # set snap data @@ -54,6 +55,7 @@ def test_on_start_not_leader_doesnt_initialise_replica_set( mock_mongodb_snap.present = True mock_mongodb_snap.start = mock.Mock() snap.return_value = {"charmed-mongodb": mock_mongodb_snap} + get_secret.return_value = "pass123" self.harness.charm.app_peer_data["monitor-password"] = "pass123" self.harness.set_leader(False) @@ -505,10 +507,12 @@ def test_update_status_additional_messages(self, _, pbm_status, connection, stat self.harness.charm.on.update_status.emit() self.assertEqual(self.harness.charm.unit.status, BlockedStatus("unknown")) + @patch("charm.MongodbOperatorCharm.get_secret") @patch_network_get(private_address="1.1.1.1") @patch("charm.MongoDBConnection") - def test_update_status_not_ready(self, connection): + def test_update_status_not_ready(self, connection, get_secret): """Tests that if mongod is not running on this unit it restarts it.""" + get_secret.return_value = "pass123" connection.return_value.__enter__.return_value.is_ready = False self.harness.charm.app_peer_data["db_initialised"] = "True" @@ -517,21 +521,25 @@ def test_update_status_not_ready(self, connection): self.harness.charm.unit.status, WaitingStatus("Waiting for MongoDB to start") ) + @patch("charm.MongodbOperatorCharm.get_secret") @patch_network_get(private_address="1.1.1.1") @patch("charm.MongoDBConnection") - def test_get_primary_current_unit_primary(self, connection): + def test_get_primary_current_unit_primary(self, connection, get_secret): """Tests get primary outputs correct primary when called on a primary replica.""" mock_event = mock.Mock() connection.return_value.__enter__.return_value.primary.return_value = "1.1.1.1" + get_secret.return_value = "pass123" self.harness.charm._on_get_primary_action(mock_event) mock_event.set_results.assert_called_with({"replica-set-primary": "mongodb/0"}) + @patch("charm.MongodbOperatorCharm.get_secret") @patch_network_get(private_address="1.1.1.1") @patch("charm.MongoDBConnection") - def test_get_primary_peer_unit_primary(self, connection): + def test_get_primary_peer_unit_primary(self, connection, get_secret): """Tests get primary outputs correct primary when called on a secondary replica.""" # add peer unit rel_id = self.harness.charm.model.get_relation("database-peers").id + get_secret.return_value = "pass123" self.harness.add_relation_unit(rel_id, "mongodb/1") self.harness.update_relation_data(rel_id, "mongodb/1", {"private-address": "2.2.2.2"}) @@ -543,13 +551,15 @@ def test_get_primary_peer_unit_primary(self, connection): self.harness.charm._on_get_primary_action(mock_event) mock_event.set_results.assert_called_with({"replica-set-primary": "mongodb/1"}) + @patch("charm.MongodbOperatorCharm.get_secret") @patch_network_get(private_address="1.1.1.1") @patch("charm.MongoDBConnection") - def test_primary_no_primary(self, connection): + def test_primary_no_primary(self, connection, get_secret): """Test that that the primary property can handle the case when there is no primary. Verifies that when there is no primary, the property _primary returns None. """ + get_secret.return_value = "pass123" # add peer unit rel_id = self.harness.charm.model.get_relation("database-peers").id self.harness.add_relation_unit(rel_id, "mongodb/1") @@ -562,11 +572,13 @@ def test_primary_no_primary(self, connection): primary = self.harness.charm._primary self.assertEqual(primary, None) + @patch("charm.MongodbOperatorCharm.get_secret") @patch_network_get(private_address="1.1.1.1") @patch("charm.MongoDBConnection") - def test_primary_failure(self, connection): + def test_primary_failure(self, connection, get_secret): """Tests that when getting the primary fails that no replica is reported as primary.""" # verify that we raise the correct exception + get_secret.return_value = "pass123" for exception in PYMONGO_EXCEPTIONS: connection.return_value.__enter__.return_value.primary.side_effect = exception self.assertEqual(self.harness.charm._primary, None) @@ -592,11 +604,13 @@ def test_storage_detaching_failure_does_not_defer(self, retry_stop, connection): self.harness.charm.on.mongodb_storage_detaching.emit(mock.Mock()) event.defer.assert_not_called() + @patch("charm.MongodbOperatorCharm.get_secret") @patch_network_get(private_address="1.1.1.1") @patch("charm.MongodbOperatorCharm._unit_ips") @patch("charm.MongoDBConnection") - def test_process_unremoved_units_handles_errors(self, connection, _unit_ips): + def test_process_unremoved_units_handles_errors(self, connection, _unit_ips, get_secret): """Test failures in process_unremoved_units are handled and not raised.""" + get_secret.return_value = "pass123" connection.return_value.__enter__.return_value.get_replset_members.return_value = { "1.1.1.1", "2.2.2.2", @@ -634,15 +648,14 @@ def test_set_password(self, pbm_status, connection): """Tests that a new admin password is generated and is returned to the user.""" self.harness.set_leader(True) pbm_status.return_value = ActiveStatus("pbm") - original_password = self.harness.charm.app_peer_data["operator-password"] + original_password = self.harness.charm.get_secret("app", "operator-password") action_event = mock.Mock() action_event.params = {} self.harness.charm._on_set_password(action_event) - new_password = self.harness.charm.app_peer_data["operator-password"] + new_password = self.harness.charm.get_secret("app", "operator-password") # verify app data is updated and results are reported to user self.assertNotEqual(original_password, new_password) - action_event.set_results.assert_called_with({"password": new_password}) @patch_network_get(private_address="1.1.1.1") @patch("charm.MongoDBConnection") @@ -654,11 +667,13 @@ def test_set_password_provided(self, pbm_status, connection): action_event = mock.Mock() action_event.params = {"password": "canonical123"} self.harness.charm._on_set_password(action_event) - new_password = self.harness.charm.app_peer_data["operator-password"] + new_password = self.harness.charm.get_secret("app", "operator-password") # verify app data is updated and results are reported to user self.assertEqual("canonical123", new_password) - action_event.set_results.assert_called_with({"password": "canonical123"}) + action_event.set_results.assert_called_with( + {"password": "canonical123", "secret-id": mock.ANY} + ) @patch_network_get(private_address="1.1.1.1") @patch("charm.MongoDBConnection") @@ -667,7 +682,7 @@ def test_set_password_failure(self, pbm_status, connection): """Tests failure to reset password does not update app data and failure is reported.""" self.harness.set_leader(True) pbm_status.return_value = ActiveStatus("pbm") - original_password = self.harness.charm.app_peer_data["operator-password"] + original_password = self.harness.charm.get_secret("app", "operator-password") action_event = mock.Mock() action_event.params = {} @@ -676,7 +691,7 @@ def test_set_password_failure(self, pbm_status, connection): exception ) self.harness.charm._on_set_password(action_event) - current_password = self.harness.charm.app_peer_data["operator-password"] + current_password = self.harness.charm.get_secret("app", "operator-password") # verify passwords are not updated. self.assertEqual(current_password, original_password) diff --git a/tests/unit/test_tls_lib.py b/tests/unit/test_tls_lib.py index a397e0ccc..4cf554a87 100644 --- a/tests/unit/test_tls_lib.py +++ b/tests/unit/test_tls_lib.py @@ -111,14 +111,17 @@ def test_tls_relation_broken_non_leader(self, restart_mongod_service): """Test non-leader removes only external cert & chain.""" # set initial certificate values rel_id = self.relate_to_tls_certificates_operator() - app_rsa_key = self.harness.charm.app_peer_data["key"] - app_csr = self.harness.charm.app_peer_data["csr"] + app_rsa_key = self.harness.charm.get_secret("app", "key-secret") + app_csr = self.harness.charm.get_secret("app", "csr-secret") self.harness.set_leader(False) self.harness.remove_relation(rel_id) - self.assertIsNone(self.harness.charm.unit_peer_data.get("ca", None)) - self.assertIsNone(self.harness.charm.unit_peer_data.get("cert", None)) - self.assertIsNone(self.harness.charm.unit_peer_data.get("chain", None)) + ca_secret = self.harness.charm.get_secret("unit", "ca-secret") + cert_secret = self.harness.charm.get_secret("unit", "cert-secret") + chain_secret = self.harness.charm.get_secret("unit", "chain-secret") + self.assertIsNone(ca_secret) + self.assertIsNone(cert_secret) + self.assertIsNone(chain_secret) # internal certificate should be maintained self.verify_internal_rsa_csr( @@ -138,12 +141,13 @@ def test_tls_relation_broken_leader(self, restart_mongod_service): self.harness.remove_relation(rel_id) # internal certificates and external certificates should be removed - self.assertIsNone(self.harness.charm.unit_peer_data.get("ca", None)) - self.assertIsNone(self.harness.charm.unit_peer_data.get("cert", None)) - self.assertIsNone(self.harness.charm.unit_peer_data.get("chain", None)) - self.assertIsNone(self.harness.charm.app_peer_data.get("ca", None)) - self.assertIsNone(self.harness.charm.app_peer_data.get("cert", None)) - self.assertIsNone(self.harness.charm.app_peer_data.get("chain", None)) + for scope in ["unit", "app"]: + ca_secret = self.harness.charm.get_secret(scope, "ca-secret") + cert_secret = self.harness.charm.get_secret(scope, "cert-secret") + chain_secret = self.harness.charm.get_secret(scope, "chain-secret") + self.assertIsNone(ca_secret) + self.assertIsNone(cert_secret) + self.assertIsNone(chain_secret) # units should be restarted after updating TLS settings restart_mongod_service.assert_called() @@ -153,14 +157,17 @@ def test_external_certificate_expiring(self): """Verifies that when an external certificate expires a csr is made.""" # assume relation exists with a current certificate self.relate_to_tls_certificates_operator() - self.harness.charm.unit_peer_data["cert"] = "unit-cert" + + self.harness.charm.set_secret("unit", "cert-secret", "unit-cert") # simulate current certificate expiring - old_csr = self.harness.charm.unit_peer_data["csr"] + old_csr = self.harness.charm.get_secret("unit", "csr-secret") + self.charm.tls.certs.on.certificate_expiring.emit(certificate="unit-cert", expiry=None) # verify a new csr was generated - new_csr = self.harness.charm.unit_peer_data["csr"] + + new_csr = self.harness.charm.get_secret("unit", "csr-secret") self.assertNotEqual(old_csr, new_csr) @patch_network_get(private_address="1.1.1.1") @@ -168,22 +175,23 @@ def test_internal_certificate_expiring(self): """Verifies that when an internal certificate expires a csr is made.""" # assume relation exists with a current certificate self.relate_to_tls_certificates_operator() - self.harness.charm.app_peer_data["cert"] = "app-cert" - self.harness.charm.unit_peer_data["cert"] = "unit-cert" + self.harness.charm.set_secret("app", "cert-secret", "app-cert") + self.harness.charm.set_secret("unit", "cert-secret", "unit-cert") # simulate current certificate expiring on non-leader self.harness.set_leader(False) - old_csr = self.harness.charm.app_peer_data["csr"] + + old_csr = self.harness.charm.get_secret("app", "csr-secret") self.charm.tls.certs.on.certificate_expiring.emit(certificate="app-cert", expiry=None) # the csr should not be changed by non-leader units - new_csr = self.harness.charm.app_peer_data["csr"] + new_csr = self.harness.charm.get_secret("app", "csr-secret") self.assertEqual(old_csr, new_csr) # verify a new csr was generated when leader receives expiry self.harness.set_leader(True) self.charm.tls.certs.on.certificate_expiring.emit(certificate="app-cert", expiry=None) - new_csr = self.harness.charm.app_peer_data["csr"] + new_csr = self.harness.charm.get_secret("app", "csr-secret") self.assertNotEqual(old_csr, new_csr) @patch_network_get(private_address="1.1.1.1") @@ -191,52 +199,59 @@ def test_unknown_certificate_expiring(self): """Verifies that when an unknown certificate expires nothing happens.""" # assume relation exists with a current certificate self.relate_to_tls_certificates_operator() - self.harness.charm.app_peer_data["cert"] = "app-cert" - self.harness.charm.unit_peer_data["cert"] = "unit-cert" + self.harness.charm.set_secret("app", "cert-secret", "app-cert") + self.harness.charm.set_secret("unit", "cert-secret", "unit-cert") # simulate unknown certificate expiring on leader - old_app_csr = self.harness.charm.app_peer_data["csr"] - old_unit_csr = self.harness.charm.unit_peer_data["csr"] + old_app_csr = self.harness.charm.get_secret("app", "csr-secret") + old_unit_csr = self.harness.charm.get_secret("unit", "csr-secret") + self.charm.tls.certs.on.certificate_expiring.emit(certificate="unknown-cert", expiry=None) - new_app_csr = self.harness.charm.app_peer_data["csr"] - new_unit_csr = self.harness.charm.unit_peer_data["csr"] + + new_app_csr = self.harness.charm.get_secret("app", "csr-secret") + new_unit_csr = self.harness.charm.get_secret("unit", "csr-secret") + self.assertEqual(old_app_csr, new_app_csr) self.assertEqual(old_unit_csr, new_unit_csr) @patch_network_get(private_address="1.1.1.1") - @patch("charm.MongodbOperatorCharm._push_tls_certificate_to_workload") + @patch("charm.MongodbOperatorCharm.push_tls_certificate_to_workload") @patch("charm.MongodbOperatorCharm.restart_mongod_service") def test_external_certificate_available(self, restart_mongod_service, _): """Tests behavior when external certificate is made available.""" # assume relation exists with a current certificate self.relate_to_tls_certificates_operator() - self.harness.charm.unit_peer_data["csr"] = "unit-crs" - self.harness.charm.unit_peer_data["cert"] = "unit-cert-old" - self.harness.charm.app_peer_data["cert"] = "app-cert" + self.harness.charm.set_secret("unit", "csr-secret", "csr-secret") + self.harness.charm.set_secret("unit", "cert-secret", "unit-cert-old") + self.harness.charm.set_secret("app", "cert-secret", "app-cert") self.charm.tls.certs.on.certificate_available.emit( - certificate_signing_request="unit-crs", + certificate_signing_request="csr-secret", chain=["unit-chain"], certificate="unit-cert", ca="unit-ca", ) - self.assertEqual(self.harness.charm.unit_peer_data["chain"], "unit-chain") - self.assertEqual(self.harness.charm.unit_peer_data["cert"], "unit-cert") - self.assertEqual(self.harness.charm.unit_peer_data["ca"], "unit-ca") + chain_secret = self.harness.charm.get_secret("unit", "chain-secret") + unit_secret = self.harness.charm.get_secret("unit", "cert-secret") + ca_secret = self.harness.charm.get_secret("unit", "ca-secret") + + self.assertEqual(chain_secret, "unit-chain") + self.assertEqual(unit_secret, "unit-cert") + self.assertEqual(ca_secret, "unit-ca") restart_mongod_service.assert_called() @patch_network_get(private_address="1.1.1.1") - @patch("charm.MongodbOperatorCharm._push_tls_certificate_to_workload") + @patch("charm.MongodbOperatorCharm.push_tls_certificate_to_workload") @patch("charm.MongodbOperatorCharm.restart_mongod_service") def test_internal_certificate_available(self, restart_mongod_service, _): """Tests behavior when internal certificate is made available.""" # assume relation exists with a current certificate self.relate_to_tls_certificates_operator() - self.harness.charm.app_peer_data["csr"] = "app-crs" - self.harness.charm.app_peer_data["cert"] = "app-cert-old" - self.harness.charm.unit_peer_data["cert"] = "unit-cert" + self.harness.charm.set_secret("app", "csr-secret", "app-crs") + self.harness.charm.set_secret("app", "cert-secret", "app-cert-old") + self.harness.charm.set_secret("unit", "cert-secret", "unit-cert") self.charm.tls.certs.on.certificate_available.emit( certificate_signing_request="app-crs", @@ -245,24 +260,28 @@ def test_internal_certificate_available(self, restart_mongod_service, _): ca="app-ca", ) - self.assertEqual(self.harness.charm.app_peer_data["chain"], "app-chain") - self.assertEqual(self.harness.charm.app_peer_data["cert"], "app-cert") - self.assertEqual(self.harness.charm.app_peer_data["ca"], "app-ca") + chain_secret = self.harness.charm.get_secret("app", "chain-secret") + unit_secret = self.harness.charm.get_secret("app", "cert-secret") + ca_secret = self.harness.charm.get_secret("app", "ca-secret") + + self.assertEqual(chain_secret, "app-chain") + self.assertEqual(unit_secret, "app-cert") + self.assertEqual(ca_secret, "app-ca") restart_mongod_service.assert_called() @patch_network_get(private_address="1.1.1.1") - @patch("charm.MongodbOperatorCharm._push_tls_certificate_to_workload") + @patch("charm.MongodbOperatorCharm.push_tls_certificate_to_workload") @patch("charm.MongodbOperatorCharm.restart_mongod_service") def test_unknown_certificate_available(self, restart_mongod_service, _): """Tests that when an unknown certificate is available, nothing is updated.""" # assume relation exists with a current certificate self.relate_to_tls_certificates_operator() - self.harness.charm.app_peer_data["chain"] = "app-chain-old" - self.harness.charm.app_peer_data["cert"] = "app-cert-old" - self.harness.charm.app_peer_data["csr"] = "app-crs-old" - self.harness.charm.app_peer_data["ca"] = "app-ca-old" - self.harness.charm.unit_peer_data["cert"] = "unit-cert" + self.harness.charm.set_secret("app", "chain-secret", "app-chain-old") + self.harness.charm.set_secret("app", "cert-secret", "app-cert-old") + self.harness.charm.set_secret("app", "csr-secret", "app-crs-old") + self.harness.charm.set_secret("app", "ca-secret", "app-ca-old") + self.harness.charm.set_secret("unit", "cert-secret", "unit-cert") self.charm.tls.certs.on.certificate_available.emit( certificate_signing_request="app-crs", @@ -271,9 +290,13 @@ def test_unknown_certificate_available(self, restart_mongod_service, _): ca="app-ca", ) - self.assertEqual(self.harness.charm.app_peer_data["chain"], "app-chain-old") - self.assertEqual(self.harness.charm.app_peer_data["cert"], "app-cert-old") - self.assertEqual(self.harness.charm.app_peer_data["ca"], "app-ca-old") + chain_secret = self.harness.charm.get_secret("app", "chain-secret") + unit_secret = self.harness.charm.get_secret("app", "cert-secret") + ca_secret = self.harness.charm.get_secret("app", "ca-secret") + + self.assertEqual(chain_secret, "app-chain-old") + self.assertEqual(unit_secret, "app-cert-old") + self.assertEqual(ca_secret, "app-ca-old") restart_mongod_service.assert_not_called() @@ -291,8 +314,9 @@ def verify_external_rsa_csr( Checks if rsa/csr were randomly generated or if they are a provided value. """ - unit_rsa_key = self.harness.charm.unit_peer_data.get("key", None) - unit_csr = self.harness.charm.unit_peer_data.get("csr", None) + unit_rsa_key = self.harness.charm.get_secret("unit", "key-secret") + unit_csr = self.harness.charm.get_secret("unit", "csr-secret") + if specific_rsa: self.assertEqual(unit_rsa_key, expected_rsa) else: @@ -310,8 +334,8 @@ def verify_internal_rsa_csr( Checks if rsa/csr were randomly generated or if they are a provided value. """ - app_rsa_key = self.harness.charm.app_peer_data.get("key", None) - app_csr = self.harness.charm.app_peer_data.get("csr", None) + app_rsa_key = self.harness.charm.get_secret("app", "key-secret") + app_csr = self.harness.charm.get_secret("app", "csr-secret") if specific_rsa: self.assertEqual(app_rsa_key, expected_rsa) else: diff --git a/tox.ini b/tox.ini index 416034267..bf5e7f657 100644 --- a/tox.ini +++ b/tox.ini @@ -57,6 +57,7 @@ description = Run unit tests deps = pytest requests + pytest-mock juju==3.2.0.1 coverage[toml] -r {tox_root}/requirements.txt @@ -75,6 +76,7 @@ deps = pytest juju==3.2.0.1 pytest-operator + pytest-mock -r {tox_root}/requirements.txt commands = pytest -v --tb native --log-cli-level=INFO -s --durations=0 {posargs} {[vars]tests_path}/integration/test_charm.py @@ -88,6 +90,7 @@ pass_env = deps = pytest juju==3.2.0.1 + pytest-mock pytest-operator -r {tox_root}/requirements.txt commands = @@ -117,6 +120,7 @@ pass_env = deps = pytest juju==3.2.0.1 + pytest-mock pytest-operator -r {tox_root}/requirements.txt commands = @@ -131,6 +135,7 @@ pass_env = deps = pytest juju==3.2.0.1 + pytest-mock pytest-operator -r {tox_root}/requirements.txt commands = @@ -150,6 +155,7 @@ pass_env = deps = pytest juju==3.2.0.1 + pytest-mock pytest-operator -r {tox_root}/requirements.txt commands = @@ -164,6 +170,7 @@ pass_env = deps = pytest juju==3.2.0.1 + pytest-mock pytest-operator -r {tox_root}/requirements.txt commands = @@ -179,6 +186,7 @@ pass_env = deps = pytest juju==3.2.0.1 + pytest-mock pytest-operator -r {tox_root}/requirements.txt commands =