diff --git a/lib/charms/vault_k8s/v0/vault_kv.py b/lib/charms/vault_k8s/v0/vault_kv.py index 3a9c2ad9..37da63ec 100644 --- a/lib/charms/vault_k8s/v0/vault_kv.py +++ b/lib/charms/vault_k8s/v0/vault_kv.py @@ -135,7 +135,7 @@ def get_nonce(self): # Increment this PATCH version before using `charmcraft publish-lib` or reset # to 0 if you are raising the major API version -LIBPATCH = 10 +LIBPATCH = 11 PYDEPS = ["pydantic", "pytest-interface-tester"] @@ -259,6 +259,25 @@ class VaultKvGoneAwayEvent(ops.EventBase): pass +class VaultKvClientDetachedEvent(ops.EventBase): + """VaultKvClientDetachedEvent Event.""" + + def __init__(self, handle: ops.Handle, unit_name: str): + super().__init__(handle) + self.unit_name = unit_name + + def snapshot(self) -> Dict[str, Any]: + """Return snapshot data that should be persisted.""" + return { + "unit_name": self.unit_name, + } + + def restore(self, snapshot: Dict[str, Any]) -> None: + """Restore the event from a snapshot.""" + super().restore(snapshot) + self.unit_name = snapshot["unit_name"] + + class NewVaultKvClientAttachedEvent(ops.EventBase): """New vault kv client attached event.""" @@ -306,7 +325,7 @@ class VaultKvProviderEvents(ops.ObjectEvents): """List of events that the Vault Kv provider charm can leverage.""" new_vault_kv_client_attached = ops.EventSource(NewVaultKvClientAttachedEvent) - gone_away = ops.EventSource(VaultKvGoneAwayEvent) + vault_kv_client_detached = ops.EventSource(VaultKvClientDetachedEvent) class VaultKvProvides(ops.Object): @@ -327,8 +346,8 @@ def __init__( self._on_relation_changed, ) self.framework.observe( - self.charm.on[relation_name].relation_broken, - self._on_vault_kv_relation_broken, + self.charm.on[relation_name].relation_departed, + self._on_vault_kv_relation_departed, ) def _on_relation_changed(self, event: ops.RelationChangedEvent): @@ -356,9 +375,10 @@ def _on_relation_changed(self, event: ops.RelationChangedEvent): nonce=event.relation.data[unit]["nonce"], ) - def _on_vault_kv_relation_broken(self, event: ops.RelationBrokenEvent): - """Handle relation broken.""" - self.on.gone_away.emit() + def _on_vault_kv_relation_departed(self, event: ops.RelationDepartedEvent): + """Handle relation departed.""" + if event.departing_unit: + self.on.vault_kv_client_detached.emit(unit_name=event.departing_unit.name) def set_vault_url(self, relation: ops.Relation, vault_url: str): """Set the vault_url on the relation.""" diff --git a/src/charm.py b/src/charm.py index ccf936de..45030895 100755 --- a/src/charm.py +++ b/src/charm.py @@ -39,7 +39,11 @@ Vault, VaultClientError, ) -from charms.vault_k8s.v0.vault_kv import NewVaultKvClientAttachedEvent, VaultKvProvides +from charms.vault_k8s.v0.vault_kv import ( + NewVaultKvClientAttachedEvent, + VaultKvClientDetachedEvent, + VaultKvProvides, +) from charms.vault_k8s.v0.vault_s3 import S3, S3Error from charms.vault_k8s.v0.vault_tls import File, VaultCertsError, VaultTLSManager from container import Container @@ -187,6 +191,9 @@ def __init__(self, *args): self.framework.observe( self.vault_kv.on.new_vault_kv_client_attached, self._on_new_vault_kv_client_attached ) + self.framework.observe( + self.vault_kv.on.vault_kv_client_detached, self._on_vault_kv_client_detached + ) self.framework.observe( self.vault_autounseal_provides.on.vault_autounseal_requirer_relation_broken, self._on_vault_autounseal_requirer_relation_broken, @@ -466,6 +473,10 @@ def _on_new_vault_kv_client_attached(self, event: NewVaultKvClientAttachedEvent) nonce=event.nonce, ) + def _on_vault_kv_client_detached(self, event: VaultKvClientDetachedEvent): + label = self._get_vault_kv_secret_label(unit_name=event.unit_name) + self._remove_juju_secret_by_label(label=label) + def _configure_pki_secrets_engine(self) -> None: """Configure the PKI secrets engine.""" if not self.unit.is_leader(): @@ -889,7 +900,7 @@ def _on_restore_backup_action(self, event: ActionEvent) -> None: ca_cert_path=self.tls.get_tls_file_path_in_charm(File.CA), ) if role_id and secret_id and not vault.authenticate(AppRole(role_id, secret_id)): - self._remove_approle_auth_secret() + self._remove_juju_secret_by_label(VAULT_CHARM_APPROLE_SECRET_LABEL) except Exception as e: logger.error("Failed to remove old approle secret: %s", e) event.fail(message="Failed to remove old approle secret.") @@ -970,30 +981,38 @@ def _ensure_unit_credentials( token_ttl="1h", token_max_ttl="1h", ) + juju_secret_label = self._get_vault_kv_secret_label(unit_name=unit_name) secret = self._create_or_update_kv_secret( vault, + nonce, relation, role_id, role_name, egress_subnets, + juju_secret_label, ) self.vault_kv.set_unit_credentials(relation, nonce, secret) def _create_or_update_kv_secret( self, vault: Vault, + nonce: str, relation: Relation, role_id: str, role_name: str, egress_subnets: List[str], + label: str, ) -> Secret: """Create or update a KV secret for a unit. Fetch secret id from peer relation, if it exists, update the secret, otherwise create it. """ - label = KV_SECRET_PREFIX + role_name - secret_id = self._get_vault_kv_secret_in_peer_relation(label) + # TODO bug: https://bugs.launchpad.net/juju/+bug/2075153 + # Until the referenced bug is fixed we must pass the secret ID here + # not to lose the secret://modeluuid:secretID format + current_credentials = self.vault_kv.get_credentials(relation) + secret_id = current_credentials.get(nonce, None) if secret_id is None: return self._create_kv_secret( vault, relation, role_id, role_name, egress_subnets, label @@ -1020,7 +1039,6 @@ def _create_kv_secret( ) if secret.id is None: raise RuntimeError(f"Unexpected error, just created secret {label!r} has no id") - self._set_vault_kv_secret_in_peer_relation(label, secret.id) secret.grant(relation) return secret @@ -1058,27 +1076,6 @@ def _remove_stale_nonce(self, relation: Relation, nonce: str) -> None: if nonce not in set(credential_nonces): self.vault_kv.remove_unit_credentials(relation, nonce=nonce) - def _get_vault_kv_secrets_in_peer_relation(self) -> Dict[str, str]: - """Return the vault kv secrets from the peer relation.""" - if not self._is_peer_relation_created(): - raise RuntimeError("Peer relation not created") - relation = self.model.get_relation(PEER_RELATION_NAME) - secrets = json.loads(relation.data[self.app].get("vault-kv-secrets", "{}")) # type: ignore[union-attr] # noqa: E501 - return secrets - - def _get_vault_kv_secret_in_peer_relation(self, label: str) -> Optional[str]: - """Return the vault kv secret id associated to input label from peer relation.""" - return self._get_vault_kv_secrets_in_peer_relation().get(label) - - def _set_vault_kv_secret_in_peer_relation(self, label: str, secret_id: str): - """Set the vault kv secret in the peer relation.""" - if not self._is_peer_relation_created(): - raise RuntimeError("Peer relation not created") - secrets = self._get_vault_kv_secrets_in_peer_relation() - secrets[label] = secret_id - relation = self.model.get_relation(PEER_RELATION_NAME) - relation.data[self.app].update({"vault-kv-secrets": json.dumps(secrets, sort_keys=True)}) # type: ignore[union-attr] # noqa: E501 - def _delete_vault_data(self) -> None: """Delete Vault's data.""" try: @@ -1299,14 +1296,18 @@ def _get_approle_auth_secret(self) -> Tuple[Optional[str], Optional[str]]: ) return role_id, secret_id - def _remove_approle_auth_secret(self) -> None: - """Remove the approle secret if it exists.""" + def _remove_juju_secret_by_label(self, label: str): + """Remove the specified secret if it exists.""" try: - juju_secret = self.model.get_secret(label=VAULT_CHARM_APPROLE_SECRET_LABEL) + juju_secret = self.model.get_secret(label=label) juju_secret.remove_all_revisions() except SecretNotFoundError: return + def _get_vault_kv_secret_label(self, unit_name: str): + unit_name_dash = unit_name.replace("/", "-") + return f"{KV_SECRET_PREFIX}{unit_name_dash}" + def _get_missing_s3_parameters(self) -> List[str]: """Return the list of missing S3 parameters. diff --git a/tests/unit/lib/charms/vault_k8s/v0/test_vault_kv.py b/tests/unit/lib/charms/vault_k8s/v0/test_vault_kv.py index 5ec9de4d..23db78e8 100644 --- a/tests/unit/lib/charms/vault_k8s/v0/test_vault_kv.py +++ b/tests/unit/lib/charms/vault_k8s/v0/test_vault_kv.py @@ -10,6 +10,7 @@ from charms.vault_k8s.v0.vault_kv import ( KVRequest, NewVaultKvClientAttachedEvent, + VaultKvClientDetachedEvent, VaultKvConnectedEvent, VaultKvGoneAwayEvent, VaultKvProvides, @@ -20,16 +21,18 @@ from ops import testing from ops.charm import CharmBase +VAULT_KV_RELATION_NAME = "vault-kv" + class VaultKvProviderCharm(CharmBase): metadata_yaml = textwrap.dedent( - """ + f""" name: vault-kv-provider containers: vault: resource: vault-image provides: - vault-kv: + {VAULT_KV_RELATION_NAME}: interface: vault-kv """ ) @@ -40,20 +43,26 @@ def __init__(self, *args): self.framework.observe( self.interface.on.new_vault_kv_client_attached, self._on_new_vault_kv_client_attached ) + self.framework.observe( + self.interface.on.vault_kv_client_detached, self._on_vault_kv_client_detached + ) def _on_new_vault_kv_client_attached(self, event: NewVaultKvClientAttachedEvent): pass + def _on_vault_kv_client_detached(self, event: VaultKvClientDetachedEvent): + pass + class VaultKvRequirerCharm(CharmBase): metadata_yaml = textwrap.dedent( - """ + f""" name: vault-kv-requirer containers: my-app: resource: my-app-image requires: - vault-kv: + {VAULT_KV_RELATION_NAME}: interface: vault-kv """ ) @@ -62,7 +71,7 @@ def __init__(self, *args): super().__init__(*args) self.interface = VaultKvRequires( self, - relation_name="vault-kv", + relation_name=VAULT_KV_RELATION_NAME, mount_suffix="dummy", ) self.framework.observe(self.interface.on.connected, self._on_connected) @@ -90,7 +99,7 @@ def setUp(self): def setup_relation(self, remote_app: str = "vault-kv-requires", leader: bool = True) -> tuple: """Set up a relation between the charm and a remote app with 1 unit.""" remote_unit = remote_app + "/0" - rel_name = "vault-kv" + rel_name = VAULT_KV_RELATION_NAME self.harness.set_leader(leader) rel_id = self.harness.add_relation(rel_name, remote_app) relation = self.harness.model.get_relation(rel_name, rel_id) @@ -414,6 +423,17 @@ def test_given_2_requests_when_get_kv_requests_then_requests_are_returned(self): assert expected_kv_request_1 in kv_requests assert expected_kv_request_2 in kv_requests + @patch("test_vault_kv.VaultKvProviderCharm._on_vault_kv_client_detached") + def test_given_vault_kv_relation_when_relation_departed_then_vault_kv_client_detached_event_fired( + self, _on_vault_kv_client_detached + ): + _, remote_unit, _, rel_id = self.setup_relation() + self.harness.remove_relation(rel_id) + args, _ = _on_vault_kv_client_detached.call_args + event = args[0] + assert isinstance(event, VaultKvClientDetachedEvent) + assert event.unit_name == remote_unit + class TestVaultKvRequires(unittest.TestCase): def setUp(self): @@ -426,7 +446,7 @@ def setUp(self): def setup_relation(self, leader: bool = True) -> tuple: remote_app = "vault-kv-provides" remote_unit = remote_app + "/0" - rel_name = "vault-kv" + rel_name = VAULT_KV_RELATION_NAME self.harness.set_leader(leader) rel_id = self.harness.add_relation(rel_name, remote_app) relation = self.harness.model.get_relation(rel_name, rel_id) diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py index 00e758b4..57b3e7b0 100644 --- a/tests/unit/test_charm.py +++ b/tests/unit/test_charm.py @@ -1506,6 +1506,43 @@ def test_given_prerequisites_are_met_when_new_vault_kv_client_attached_then_kv_m SecretsBackend.KV_V2, "charm-vault-kv-requirer-suffix" ) + @patch("ops.model.Secret.remove_all_revisions") + def test_given_vault_kv_client_when_client_detached_then_kv_secret_is_removed( + self, + patch_remove_secret, + ): + self.mock_vault.configure_mock( + **{ + "configure_approle.return_value": "12345678", + "generate_role_secret_id.return_value": "11111111", + }, + ) + self.mock_vault_tls_manager.pull_tls_file_from_workload.return_value = "test cert" + + self.harness.add_relation(relation_name="vault-peers", remote_app="vault") + self.harness.set_leader(is_leader=True) + self.harness.set_can_connect(container=self.container_name, val=True) + event = Mock() + event.params = {"relation_name": "relation", "relation_id": "99"} + self._set_approle_secret( + role_id="root token content", + secret_id="whatever secret id", + ) + rel_id, _ = self.setup_vault_kv_relation() + event = Mock() + event.relation_name = VAULT_KV_RELATION_NAME + event.relation_id = rel_id + event.app_name = VAULT_KV_REQUIRER_APPLICATION_NAME + event.unit_name = f"{VAULT_KV_REQUIRER_APPLICATION_NAME}/0" + event.mount_suffix = "suffix" + event.egress_subnets = ["2.2.2.0/24"] + event.nonce = "123123" + self.harness.charm._on_new_vault_kv_client_attached(event) + kv_client_detached_event = Mock() + kv_client_detached_event.unit_name = f"{VAULT_KV_REQUIRER_APPLICATION_NAME}/0" + self.harness.charm._on_vault_kv_client_detached(kv_client_detached_event) + patch_remove_secret.assert_called() + # Test PKI @patch("charm.get_common_name_from_certificate", new=Mock) @patch(f"{TLS_CERTIFICATES_LIB_PATH}.TLSCertificatesRequiresV3.request_certificate_creation")