diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 7f360d8c3..9007809d1 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -70,7 +70,7 @@ Testing high availability on a production cluster can be done with: tox run -e ha-integration -- --model= ``` -Note if you'd like to test storage re-use in ha-testing, your storage must not be of the type `rootfs`. `rootfs` storage is tied to the machine lifecycle and does not stick around after unit removal. `rootfs` storage is used by default with `tox run -e ha-integration`. To test ha-testing for storage re-use: +Note if you'd like to test storage reuse in ha-testing, your storage must not be of the type `rootfs`. `rootfs` storage is tied to the machine lifecycle and does not stick around after unit removal. `rootfs` storage is used by default with `tox run -e ha-integration`. To test ha-testing for storage reuse: ```shell juju create-storage-pool mongodb-ebs ebs volume-type=standard # create a storage pool juju deploy ./*charm --storage mongodb=mongodb-ebs,7G,1 # deploy 1 or more units of application with said storage pool diff --git a/lib/charms/mongodb/v0/helpers.py b/lib/charms/mongodb/v0/helpers.py index e5acc9d3d..26fcbf868 100644 --- a/lib/charms/mongodb/v0/helpers.py +++ b/lib/charms/mongodb/v0/helpers.py @@ -210,7 +210,7 @@ def process_pbm_error(error_string: Optional[_StrOrBytes]) -> str: message = "couldn't configure s3 backup option" if not error_string: return message - if type(error_string) == bytes: + if isinstance(error_string, bytes): error_string = error_string.decode("utf-8") if "status code: 403" in error_string: # type: ignore message = "s3 credentials are incorrect." diff --git a/lib/charms/mongodb/v0/mongodb.py b/lib/charms/mongodb/v0/mongodb.py index 25a65d7c0..1ff8bcc8e 100644 --- a/lib/charms/mongodb/v0/mongodb.py +++ b/lib/charms/mongodb/v0/mongodb.py @@ -308,7 +308,7 @@ def create_role(self, role_name: str, privileges: dict, roles: dict = []): Args: role_name: name of the role to be added. - privileges: privledges to be associated with the role. + privileges: privileges to be associated with the role. roles: List of roles from which this role inherits privileges. """ try: diff --git a/lib/charms/mongodb/v0/mongodb_backups.py b/lib/charms/mongodb/v0/mongodb_backups.py index d919a2710..48f1be0eb 100644 --- a/lib/charms/mongodb/v0/mongodb_backups.py +++ b/lib/charms/mongodb/v0/mongodb_backups.py @@ -505,7 +505,7 @@ def _try_to_restore(self, backup_id: str) -> None: If PBM is resyncing, the function will retry to create backup (up to BACKUP_RESTORE_MAX_ATTEMPTS times) with BACKUP_RESTORE_ATTEMPT_COOLDOWN - time between attepts. + time between attempts. If PMB returen any other error, the function will raise RestoreError. """ @@ -523,7 +523,7 @@ def _try_to_restore(self, backup_id: str) -> None: restore_cmd = restore_cmd + remapping_args.split(" ") self.charm.run_pbm_command(restore_cmd) except (subprocess.CalledProcessError, ExecError) as e: - if type(e) == subprocess.CalledProcessError: + if isinstance(e, subprocess.CalledProcessError): error_message = e.output.decode("utf-8") else: error_message = str(e.stderr) @@ -541,7 +541,7 @@ def _try_to_backup(self): If PBM is resyncing, the function will retry to create backup (up to BACKUP_RESTORE_MAX_ATTEMPTS times) - with BACKUP_RESTORE_ATTEMPT_COOLDOWN time between attepts. + with BACKUP_RESTORE_ATTEMPT_COOLDOWN time between attempts. If PMB returen any other error, the function will raise BackupError. """ @@ -560,7 +560,7 @@ def _try_to_backup(self): ) return backup_id_match.group("backup_id") if backup_id_match else "N/A" except (subprocess.CalledProcessError, ExecError) as e: - if type(e) == subprocess.CalledProcessError: + if isinstance(e, subprocess.CalledProcessError): error_message = e.output.decode("utf-8") else: error_message = str(e.stderr) @@ -636,13 +636,13 @@ def _get_backup_restore_operation_result(self, current_pbm_status, previous_pbm_ to contain the operation type (backup/restore) and the backup id. """ if ( - type(current_pbm_status) == type(previous_pbm_status) + type(current_pbm_status) is type(previous_pbm_status) and current_pbm_status.message == previous_pbm_status.message ): return f"Operation is still in progress: '{current_pbm_status.message}'" if ( - type(previous_pbm_status) == MaintenanceStatus + isinstance(previous_pbm_status, MaintenanceStatus) and "backup id:" in previous_pbm_status.message ): backup_id = previous_pbm_status.message.split("backup id:")[-1].strip() diff --git a/lib/charms/mongodb/v0/mongodb_secrets.py b/lib/charms/mongodb/v0/mongodb_secrets.py new file mode 100644 index 000000000..804c0f071 --- /dev/null +++ b/lib/charms/mongodb/v0/mongodb_secrets.py @@ -0,0 +1,137 @@ +"""Secrets related helper classes/functions.""" +# Copyright 2023 Canonical Ltd. +# See LICENSE file for licensing details. + +from typing import Dict, Optional + +from ops import Secret, SecretInfo +from ops.charm import CharmBase +from ops.model import SecretNotFoundError + +from config import Config +from exceptions import SecretAlreadyExistsError + +# The unique Charmhub library identifier, never change it + +# The unique Charmhub library identifier, never change it +LIBID = "87456e41c7594240b92b783a648592b5" + +# 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 + +APP_SCOPE = Config.Relations.APP_SCOPE +UNIT_SCOPE = Config.Relations.UNIT_SCOPE +Scopes = Config.Relations.Scopes + + +def generate_secret_label(charm: CharmBase, scope: Scopes) -> str: + """Generate unique group_mappings for secrets within a relation context. + + Defined as a standalone function, as the choice on secret labels definition belongs to the + Application Logic. To be kept separate from classes below, which are simply to provide a + (smart) abstraction layer above Juju Secrets. + """ + members = [charm.app.name, scope] + return f"{'.'.join(members)}" + + +# Secret cache + + +class CachedSecret: + """Abstraction layer above direct Juju access with caching. + + The data structure is precisely re-using/simulating Juju Secrets behavior, while + also making sure not to fetch a secret multiple times within the same event scope. + """ + + def __init__(self, charm: CharmBase, 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 + + def add_secret(self, content: Dict[str, str], scope: Scopes) -> Secret: + """Create a new secret.""" + if self._secret_uri: + raise SecretAlreadyExistsError( + "Secret is already defined with uri %s", self._secret_uri + ) + + if scope == Config.Relations.APP_SCOPE: + secret = self.charm.app.add_secret(content, label=self.label) + else: + secret = self.charm.unit.add_secret(content, label=self.label) + self._secret_uri = secret.id + self._secret_meta = secret + return self._secret_meta + + @property + def meta(self) -> Optional[Secret]: + """Getting cached secret meta-information.""" + if self._secret_meta: + return self._secret_meta + + if not (self._secret_uri or self.label): + return + + try: + self._secret_meta = self.charm.model.get_secret(label=self.label) + except SecretNotFoundError: + if self._secret_uri: + self._secret_meta = self.charm.model.get_secret( + id=self._secret_uri, label=self.label + ) + return self._secret_meta + + def get_content(self) -> Dict[str, str]: + """Getting cached secret content.""" + if not self._secret_content: + if self.meta: + self._secret_content = self.meta.get_content() + return self._secret_content + + def set_content(self, content: Dict[str, str]) -> None: + """Setting cached secret content.""" + if self.meta: + self.meta.set_content(content) + self._secret_content = content + + def get_info(self) -> Optional[SecretInfo]: + """Wrapper function for get the corresponding call on the Secret object if any.""" + if self.meta: + return self.meta.get_info() + + +class SecretCache: + """A data structure storing CachedSecret objects.""" + + def __init__(self, charm): + self.charm = charm + 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) + if secret.meta: + self._secrets[label] = secret + return self._secrets.get(label) + + def add(self, label: str, content: Dict[str, str], scope: Scopes) -> CachedSecret: + """Adding a secret to Juju Secret.""" + if self._secrets.get(label): + raise SecretAlreadyExistsError(f"Secret {label} already exists") + + secret = CachedSecret(self.charm, label) + secret.add_secret(content, scope) + self._secrets[label] = secret + return self._secrets[label] + + +# END: Secret cache diff --git a/requirements.txt b/requirements.txt index ff9e7f2be..e892ba369 100644 --- a/requirements.txt +++ b/requirements.txt @@ -15,4 +15,5 @@ pyrsistent==0.19.3 pyyaml==6.0.1 zipp==3.11.0 pyOpenSSL==22.1.0 -typing-extensions==4.5.0 \ No newline at end of file +typing-extensions==4.5.0 +parameterized==0.9.0 diff --git a/src/charm.py b/src/charm.py index 2a116682f..cc349f48e 100755 --- a/src/charm.py +++ b/src/charm.py @@ -4,7 +4,6 @@ # See LICENSE file for licensing details. import json import logging -import re import subprocess import time from typing import Dict, List, Optional, Set @@ -30,6 +29,7 @@ ) from charms.mongodb.v0.mongodb_backups import S3_RELATION, MongoDBBackups from charms.mongodb.v0.mongodb_provider import MongoDBProvider +from charms.mongodb.v0.mongodb_secrets import SecretCache, generate_secret_label from charms.mongodb.v0.mongodb_tls import MongoDBTLS from charms.mongodb.v0.mongodb_vm_legacy_provider import MongoDBLegacyProvider from charms.mongodb.v0.users import ( @@ -61,18 +61,13 @@ 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, - SecretNotAddedError, -) +from exceptions import AdminUserCreationError, ApplicationHostNotFoundError from machine_helpers import ( push_file_to_unit, remove_file_from_unit, @@ -134,7 +129,7 @@ def __init__(self, *args): log_slots=Config.Monitoring.LOG_SLOTS, ) - self.secrets = {APP_SCOPE: {}, UNIT_SCOPE: {}} + self.secrets = SecretCache(self) # BEGIN: properties @@ -586,19 +581,27 @@ def _on_secret_remove(self, event: SecretRemoveEvent): ) 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) - ): + """Handles secrets changes event. + + When user run set-password action, juju leader changes the password inside the database + and inside the secret object. This action runs the restart for monitoring tool and + for backup tool on non-leader units to keep them working with MongoDB. The same workflow + occurs on TLS certs change. + """ + label = None + if generate_secret_label(self, Config.Relations.APP_SCOPE) == event.secret.label: + label = generate_secret_label(self, Config.Relations.APP_SCOPE) scope = APP_SCOPE - elif self._compare_secret_ids( - event.secret.id, self.unit_peer_data.get(Config.Secrets.SECRET_INTERNAL_LABEL) - ): + elif generate_secret_label(self, Config.Relations.UNIT_SCOPE) == event.secret.label: + label = generate_secret_label(self, Config.Relations.UNIT_SCOPE) 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) + + # Refreshing cache + self.secrets.get(label) # changed secrets means that the URIs used for PBM and mongodb_exporter are now out of date self._connect_mongodb_exporter() @@ -987,15 +990,14 @@ def _unit_ip(self, unit: Unit) -> str: def get_secret(self, scope: str, key: str) -> Optional[str]: """Get secret from the secret storage.""" - if self._juju_has_secrets: - return self._juju_secret_get(scope, key) + label = generate_secret_label(self, scope) + secret = self.secrets.get(label) + if not secret: + return - if scope == UNIT_SCOPE: - return self.unit_peer_data.get(key, None) - elif scope == APP_SCOPE: - return self.app_peer_data.get(key, None) - else: - raise RuntimeError("Unknown secret scope.") + value = secret.get_content().get(key) + if value != Config.Secrets.SECRET_DELETED_LABEL: + return value def set_secret(self, scope: str, key: str, value: Optional[str]) -> Optional[str]: """Set secret in the secret storage. @@ -1003,23 +1005,35 @@ def set_secret(self, scope: str, key: str, value: Optional[str]) -> Optional[str 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 not value: + return self.remove_secret(scope, key) - 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_SCOPE: - if not value: - del self.app_peer_data[key] - return - self.app_peer_data.update({key: str(value)}) + label = generate_secret_label(self, scope) + secret = self.secrets.get(label) + if not secret: + self.secrets.add(label, {key: value}, scope) else: - raise RuntimeError("Unknown secret scope.") + content = secret.get_content() + content.update({key: value}) + secret.set_content(content) + return label + + def remove_secret(self, scope, key) -> None: + """Removing a secret.""" + label = generate_secret_label(self, scope) + secret = self.secrets.get(label) + + if not secret: + return + + content = secret.get_content() + + if not content.get(key) or content[key] == Config.Secrets.SECRET_DELETED_LABEL: + logger.error(f"Non-existing secret {scope}:{key} was attempted to be removed.") + return + + content[key] = Config.Secrets.SECRET_DELETED_LABEL + secret.set_content(content) def restart_mongod_service(self, auth=None): """Restarts the mongod service with its associated configuration.""" @@ -1105,140 +1119,6 @@ def _peer_data(self, scope: Scopes): 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}") - # END: helper functions diff --git a/src/exceptions.py b/src/exceptions.py index 022182a4b..bb4e24c16 100644 --- a/src/exceptions.py +++ b/src/exceptions.py @@ -38,3 +38,9 @@ class MissingSecretError(MongoSecretError): """Could be raised when a Juju 3 mandatory secret couldn't be found.""" pass + + +class SecretAlreadyExistsError(MongoSecretError): + """A secret that we want to create already exists.""" + + pass diff --git a/tests/integration/ha_tests/test_ha.py b/tests/integration/ha_tests/test_ha.py index 13d0d84a2..600f5bafc 100644 --- a/tests/integration/ha_tests/test_ha.py +++ b/tests/integration/ha_tests/test_ha.py @@ -96,7 +96,7 @@ async def test_storage_re_use(ops_test, continuous_writes): app = await helpers.app_name(ops_test) if helpers.storage_type(ops_test, app) == "rootfs": pytest.skip( - "re-use of storage can only be used on deployments with persistent storage not on rootfs deployments" + "reuse of storage can only be used on deployments with persistent storage not on rootfs deployments" ) # removing the only replica can be disastrous @@ -501,7 +501,7 @@ async def test_full_cluster_crash(ops_test: OpsTest, continuous_writes, reset_re ) # This test serves to verify behavior when all replicas are down at the same time that when - # they come back online they operate as expected. This check verifies that we meet the criterea + # they come back online they operate as expected. This check verifies that we meet the criteria # of all replicas being down at the same time. assert await helpers.all_db_processes_down(ops_test), "Not all units down at the same time." @@ -549,7 +549,7 @@ async def test_full_cluster_restart(ops_test: OpsTest, continuous_writes, reset_ ) # This test serves to verify behavior when all replicas are down at the same time that when - # they come back online they operate as expected. This check verifies that we meet the criterea + # they come back online they operate as expected. This check verifies that we meet the criteria # of all replicas being down at the same time. assert await helpers.all_db_processes_down(ops_test), "Not all units down at the same time." diff --git a/tests/integration/test_charm.py b/tests/integration/test_charm.py index 0f75d43ce..edeadac87 100644 --- a/tests/integration/test_charm.py +++ b/tests/integration/test_charm.py @@ -220,6 +220,16 @@ async def test_reset_and_get_password_secret_same_as_cli(ops_test: OpsTest) -> N # Getting back the pw programmatically password = await get_password(ops_test, username="monitor") + # + # No way to retrieve a secet by label for now (https://bugs.launchpad.net/juju/+bug/2037104) + # Therefore we take advantage of the fact, that we only have ONE single secret a this point + # So we take the single member of the list + # NOTE: This would BREAK if for instance units had secrets at the start... + # + complete_command = "list-secrets" + _, stdout, _ = await ops_test.juju(*complete_command.split()) + secret_id = stdout.split("\n")[1].split(" ")[0] + # 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()) diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py index f11486c7e..c3ac1a86b 100644 --- a/tests/unit/test_charm.py +++ b/tests/unit/test_charm.py @@ -1,13 +1,17 @@ # Copyright 2023 Canonical Ltd. # See LICENSE file for licensing details. +import logging +import re import unittest from unittest import mock -from unittest.mock import call, patch +from unittest.mock import MagicMock, call, patch +import pytest from charms.operator_libs_linux.v1 import snap from ops.model import ActiveStatus, BlockedStatus, MaintenanceStatus, WaitingStatus from ops.testing import Harness +from parameterized import parameterized from pymongo.errors import ConfigurationError, ConnectionFailure, OperationFailure from tenacity import stop_after_attempt @@ -38,6 +42,15 @@ def setUp(self, *unused): self.harness.begin() self.peer_rel_id = self.harness.add_relation("database-peers", "database-peers") + @pytest.fixture + def use_caplog(self, caplog): + self._caplog = caplog + + def _setup_secrets(self): + self.harness.set_leader(True) + self.harness.charm._generate_secrets() + self.harness.set_leader(False) + @patch("charm.MongodbOperatorCharm.get_secret") @patch_network_get(private_address="1.1.1.1") @patch("charm.MongoDBConnection") @@ -641,61 +654,230 @@ def test_start_init_user_after_second_call(self, run, config): self.harness.charm._init_operator_user() run.assert_called_once() + def test_get_password(self): + self._setup_secrets() + assert isinstance(self.harness.charm.get_secret("app", "monitor-password"), str) + self.harness.charm.get_secret("app", "non-existing-secret") is None + + self.harness.charm.set_secret("unit", "somekey", "bla") + assert isinstance(self.harness.charm.get_secret("unit", "somekey"), str) + self.harness.charm.get_secret("unit", "non-existing-secret") is None + + def test_set_reset_existing_password_app(self): + """NOTE: currently ops.testing seems to allow for non-leader to set secrets too!""" + self._setup_secrets() + + # Getting current password + self.harness.charm.set_secret("app", "monitor-password", "bla") + assert self.harness.charm.get_secret("app", "monitor-password") == "bla" + + self.harness.charm.set_secret("app", "monitor-password", "blablabla") + assert self.harness.charm.get_secret("app", "monitor-password") == "blablabla" + + @parameterized.expand([("app"), ("unit")]) + def test_set_secret_returning_secret_id(self, scope): + secret_id = self.harness.charm.set_secret(scope, "somekey", "bla") + assert re.match(f"mongodb.{scope}", secret_id) + + @parameterized.expand([("app"), ("unit")]) + def test_set_reset_new_secret(self, scope): + """NOTE: currently ops.testing seems to allow for non-leader to set secrets too!""" + # Getting current password + self.harness.charm.set_secret(scope, "new-secret", "bla") + assert self.harness.charm.get_secret(scope, "new-secret") == "bla" + + # Reset new secret + self.harness.charm.set_secret(scope, "new-secret", "blablabla") + assert self.harness.charm.get_secret(scope, "new-secret") == "blablabla" + + # Set another new secret + self.harness.charm.set_secret(scope, "new-secret2", "blablabla") + assert self.harness.charm.get_secret(scope, "new-secret2") == "blablabla" + + @parameterized.expand([("app"), ("unit")]) + def test_invalid_secret(self, scope): + with self.assertRaises(TypeError): + self.harness.charm.set_secret("unit", "somekey", 1) + + self.harness.charm.set_secret("unit", "somekey", "") + assert self.harness.charm.get_secret(scope, "somekey") is None + + @pytest.mark.usefixtures("use_caplog") + def test_delete_password(self): + """NOTE: currently ops.testing seems to allow for non-leader to remove secrets too!""" + self._setup_secrets() + + assert self.harness.charm.get_secret("app", "monitor-password") + self.harness.charm.remove_secret("app", "monitor-password") + assert self.harness.charm.get_secret("app", "monitor-password") is None + + assert self.harness.charm.set_secret("unit", "somekey", "somesecret") + self.harness.charm.remove_secret("unit", "somekey") + assert self.harness.charm.get_secret("unit", "somekey") is None + + with self._caplog.at_level(logging.ERROR): + self.harness.charm.remove_secret("app", "monitor-password") + assert ( + "Non-existing secret app:monitor-password was attempted to be removed." + in self._caplog.text + ) + + self.harness.charm.remove_secret("unit", "somekey") + assert ( + "Non-existing secret unit:somekey was attempted to be removed." + in self._caplog.text + ) + + self.harness.charm.remove_secret("app", "non-existing-secret") + assert ( + "Non-existing secret app:non-existing-secret was attempted to be removed." + in self._caplog.text + ) + + self.harness.charm.remove_secret("unit", "non-existing-secret") + assert ( + "Non-existing secret unit:non-existing-secret was attempted to be removed." + in self._caplog.text + ) + + @parameterized.expand([("app"), ("unit")]) + @patch("charm.MongodbOperatorCharm._connect_mongodb_exporter") + def test_on_secret_changed(self, scope, connect_exporter): + """NOTE: currently ops.testing seems to allow for non-leader to set secrets too!""" + secret_label = self.harness.charm.set_secret(scope, "new-secret", "bla") + secret = self.harness.charm.model.get_secret(label=secret_label) + + event = mock.Mock() + event.secret = secret + secret_label = self.harness.charm._on_secret_changed(event) + connect_exporter.assert_called() + + @parameterized.expand([("app"), ("unit")]) + @pytest.mark.usefixtures("use_caplog") + @patch("charm.MongodbOperatorCharm._connect_mongodb_exporter") + def test_on_other_secret_changed(self, scope, connect_exporter): + """NOTE: currently ops.testing seems to allow for non-leader to set secrets too!""" + # "Hack": creating a secret outside of the normal MongodbOperatorCharm.set_secret workflow + scope_obj = self.harness.charm._scope_obj(scope) + secret = scope_obj.add_secret({"key": "value"}) + + event = mock.Mock() + event.secret = secret + + with self._caplog.at_level(logging.DEBUG): + self.harness.charm._on_secret_changed(event) + assert f"Secret {secret.id} changed, but it's unknown" in self._caplog.text + + connect_exporter.assert_not_called() + @patch_network_get(private_address="1.1.1.1") @patch("charm.MongoDBConnection") - @patch("charm.MongoDBBackups._get_pbm_status") - def test_set_password(self, pbm_status, connection): - """Tests that a new admin password is generated and is returned to the user.""" + @patch("charm.MongodbOperatorCharm._connect_mongodb_exporter") + def test_connect_to_mongo_exporter_on_set_password(self, connect_exporter, connection): + """Test _connect_mongodb_exporter is called when the password is set for 'montior' user.""" + # container = self.harness.model.unit.get_container("mongod") + # self.harness.set_can_connect(container, True) + # self.harness.charm.on.mongod_pebble_ready.emit(container) self.harness.set_leader(True) - pbm_status.return_value = ActiveStatus("pbm") - original_password = self.harness.charm.get_secret("app", "operator-password") + action_event = mock.Mock() - action_event.params = {} + action_event.params = {"username": "monitor"} self.harness.charm._on_set_password(action_event) - 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) + connect_exporter.assert_called() @patch_network_get(private_address="1.1.1.1") - @patch("charm.MongoDBConnection") @patch("charm.MongoDBBackups._get_pbm_status") - def test_set_password_provided(self, pbm_status, connection): - """Tests that a given password is set as the new mongodb password.""" + @patch("charm.MongodbOperatorCharm.has_backup_service") + @patch("charm.MongoDBConnection") + @patch("charm.MongodbOperatorCharm._connect_mongodb_exporter") + def test_event_set_password_secrets( + self, connect_exporter, connection, has_backup_service, get_pbm_status + ): + """Test _connect_mongodb_exporter is called when the password is set for 'montior' user. + + Furthermore: in Juju 3.x we want to use secrets + """ + pw = "bla" + has_backup_service.return_value = True + get_pbm_status.return_value = ActiveStatus() self.harness.set_leader(True) - pbm_status.return_value = ActiveStatus("pbm") + action_event = mock.Mock() - action_event.params = {"password": "canonical123"} + action_event.set_results = MagicMock() + action_event.params = {"username": "monitor", "password": pw} self.harness.charm._on_set_password(action_event) - new_password = self.harness.charm.get_secret("app", "operator-password") + connect_exporter.assert_called() - # 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", "secret-id": mock.ANY} - ) + action_event.set_results.assert_called() + args_pw_set = action_event.set_results.call_args.args[0] + assert "secret-id" in args_pw_set + + action_event.params = {"username": "monitor"} + self.harness.charm._on_get_password(action_event) + args_pw = action_event.set_results.call_args.args[0] + assert "password" in args_pw + assert args_pw["password"] == pw @patch_network_get(private_address="1.1.1.1") - @patch("charm.MongoDBConnection") @patch("charm.MongoDBBackups._get_pbm_status") - def test_set_password_failure(self, pbm_status, connection): - """Tests failure to reset password does not update app data and failure is reported.""" + @patch("charm.MongodbOperatorCharm.has_backup_service") + @patch("charm.MongoDBConnection") + @patch("charm.MongodbOperatorCharm._connect_mongodb_exporter") + def test_event_auto_reset_password_secrets_when_no_pw_value_shipped( + self, connect_exporter, connection, has_backup_service, get_pbm_status + ): + """Test _connect_mongodb_exporter is called when the password is set for 'montior' user. + + Furthermore: in Juju 3.x we want to use secrets + """ + has_backup_service.return_value = True + get_pbm_status.return_value = ActiveStatus() + self._setup_secrets() self.harness.set_leader(True) - pbm_status.return_value = ActiveStatus("pbm") - original_password = self.harness.charm.get_secret("app", "operator-password") + action_event = mock.Mock() - action_event.params = {} + action_event.set_results = MagicMock() - for exception in [PYMONGO_EXCEPTIONS, NotReadyError]: - connection.return_value.__enter__.return_value.set_user_password.side_effect = ( - exception - ) - self.harness.charm._on_set_password(action_event) - current_password = self.harness.charm.get_secret("app", "operator-password") + # Getting current password + action_event.params = {"username": "monitor"} + self.harness.charm._on_get_password(action_event) + args_pw = action_event.set_results.call_args.args[0] + assert "password" in args_pw + pw1 = args_pw["password"] - # verify passwords are not updated. - self.assertEqual(current_password, original_password) - action_event.fail.assert_called() + # No password value was shipped + action_event.params = {"username": "monitor"} + self.harness.charm._on_set_password(action_event) + connect_exporter.assert_called() + + # New password was generated + action_event.params = {"username": "monitor"} + self.harness.charm._on_get_password(action_event) + args_pw = action_event.set_results.call_args.args[0] + assert "password" in args_pw + pw2 = args_pw["password"] + + # a new password was created + assert pw1 != pw2 + + @patch("charm.MongoDBConnection") + @patch("charm.MongodbOperatorCharm._connect_mongodb_exporter") + def test_event_any_unit_can_get_password_secrets(self, connect_exporter, connection): + """Test _connect_mongodb_exporter is called when the password is set for 'montior' user. + + Furthermore: in Juju 3.x we want to use secrets + """ + self._setup_secrets() + + action_event = mock.Mock() + action_event.set_results = MagicMock() + + # Getting current password + action_event.params = {"username": "monitor"} + self.harness.charm._on_get_password(action_event) + args_pw = action_event.set_results.call_args.args[0] + assert "password" in args_pw + assert args_pw["password"] @patch_network_get(private_address="1.1.1.1") @patch("charm.MongoDBBackups._get_pbm_status") diff --git a/tox.ini b/tox.ini index bf5e7f657..c89fdca5b 100644 --- a/tox.ini +++ b/tox.ini @@ -60,6 +60,7 @@ deps = pytest-mock juju==3.2.0.1 coverage[toml] + parameterized -r {tox_root}/requirements.txt commands = coverage run --source={[vars]src_path} \ @@ -195,4 +196,4 @@ commands = [testenv:cleanup_juju_models] description = Cleanup Juju models commands = - python {[vars]tests_path}/integration/cleanup_resources.py \ No newline at end of file + python {[vars]tests_path}/integration/cleanup_resources.py