From 26449343f97eafeda1ac6430eb63f3b05e1ef4c3 Mon Sep 17 00:00:00 2001 From: Guillaume Belanger Date: Thu, 31 Aug 2023 08:13:00 -0400 Subject: [PATCH 01/14] Adds raft storage --- metadata.yaml | 4 ++-- src/charm.py | 57 +++++++++++++++++++++++++++++++++++++-------------- 2 files changed, 44 insertions(+), 17 deletions(-) diff --git a/metadata.yaml b/metadata.yaml index ead2144e..8681267f 100644 --- a/metadata.yaml +++ b/metadata.yaml @@ -18,8 +18,8 @@ containers: vault: resource: vault-image mounts: - - storage: vault-storage - location: /srv + - storage: vault-raft + location: /vault/raft resources: vault-image: diff --git a/src/charm.py b/src/charm.py index a7dfb77e..c43fd739 100755 --- a/src/charm.py +++ b/src/charm.py @@ -15,7 +15,7 @@ KubernetesServicePatch, ServicePort, ) -from ops.charm import CharmBase, ConfigChangedEvent, InstallEvent +from ops.charm import CharmBase, ConfigChangedEvent, InstallEvent, RelationJoinedEvent from ops.main import main from ops.model import ActiveStatus, MaintenanceStatus, ModelError, WaitingStatus from ops.pebble import Layer @@ -24,7 +24,7 @@ logger = logging.getLogger(__name__) -VAULT_STORAGE_PATH = "/srv" +VAULT_RAFT_DATA_PATH = "/vault/raft" PEER_RELATION_NAME = "vault-peers" @@ -45,6 +45,9 @@ def __init__(self, *args): charm=self, ports=[ServicePort(name="vault", port=self.VAULT_PORT)], ) + self.framework.observe( + self.on[PEER_RELATION_NAME].relation_created, self._on_peer_relation_created + ) def _on_install(self, event: InstallEvent): if not self.unit.is_leader(): @@ -80,8 +83,6 @@ def _on_config_changed(self, event: ConfigChangedEvent) -> None: Returns: None """ - if not self.unit.is_leader(): - return if not self._container.can_connect(): self.unit.status = WaitingStatus("Waiting to be able to connect to vault unit") event.defer() @@ -96,7 +97,6 @@ def _on_config_changed(self, event: ConfigChangedEvent) -> None: event.defer() return self._set_pebble_plan() - self._patch_storage_ownership() vault = Vault(url=self._api_address) vault.set_token(token=root_token) vault.wait_for_api_available() @@ -104,6 +104,10 @@ def _on_config_changed(self, event: ConfigChangedEvent) -> None: vault.unseal(unseal_keys=unseal_keys) self.unit.status = ActiveStatus() + def _on_peer_relation_created(self, event: RelationJoinedEvent) -> None: + """Handle relation-joined event for the replicas relation.""" + self._set_peer_relation_unit_address() + @property def _api_address(self) -> str: return f"http://{self._bind_address}:{self.VAULT_PORT}" @@ -199,15 +203,15 @@ def _vault_layer(self) -> Layer: Returns: Layer: Pebble Layer """ - backends = {"file": {"path": VAULT_STORAGE_PATH}} vault_config = { - "backend": backends, + "ui": True, + "storage": {"raft": self._get_raft_config()}, "listener": {"tcp": {"tls_disable": True, "address": f"[::]:{self.VAULT_PORT}"}}, "default_lease_ttl": self.model.config["default_lease_ttl"], "max_lease_ttl": self.model.config["max_lease_ttl"], "disable_mlock": True, "cluster_addr": f"http://{self._bind_address}:{self.VAULT_CLUSTER_PORT}", - "api_addr": f"http://{self._bind_address}:{self.VAULT_PORT}", + "api_addr": self._api_address, } return Layer( @@ -229,14 +233,37 @@ def _vault_layer(self) -> Layer: } ) - def _patch_storage_ownership(self) -> None: - """Fix up storage permissions (broken on AWS and GCP otherwise)'. + def _set_peer_relation_unit_address(self) -> None: + peer_relation = self.model.get_relation(PEER_RELATION_NAME) + peer_relation.data[self.unit].update({"unit-address": self._api_address}) - Returns: - None - """ - command = ["chown", "100:1000", VAULT_STORAGE_PATH] - self._container.exec(command=command) + def _get_peer_relation_unit_addresses(self) -> List[str]: + peer_relation = self.model.get_relation(PEER_RELATION_NAME) + if not peer_relation: + return [] + return [peer_relation.data[peer]["unit-address"] for peer in peer_relation.units] + + def _other_peer_unit_addresses(self) -> List[str]: + return [ + unit_address for unit_address in self._get_peer_relation_unit_addresses() if + unit_address != self._api_address + ] + + def _get_raft_config(self) -> dict: + retry_join = [ + {"leader_api_addr": unit_address} for unit_address in self._other_peer_unit_addresses() + ] + raft_config = { + "path": VAULT_RAFT_DATA_PATH, + "node_id": self._node_id, + } + if retry_join: + raft_config["retry_join"] = retry_join + return raft_config + + @property + def _node_id(self): + return f"{self.model.name}-{self.unit.name}" if __name__ == "__main__": # pragma: no cover From c27aa7e56d3d4f4610845a2e000354283f3e63a0 Mon Sep 17 00:00:00 2001 From: Guillaume Belanger Date: Wed, 6 Sep 2023 17:52:18 -0400 Subject: [PATCH 02/14] implements raft backend --- README.md | 2 +- metadata.yaml | 2 +- src/charm.py | 87 +++++++- src/vault.py | 38 ++-- tests/integration/test_integration.py | 23 ++ tests/unit/test_charm.py | 297 +++++++++++++++++++++++--- tests/unit/test_vault.py | 64 +++--- 7 files changed, 412 insertions(+), 101 deletions(-) diff --git a/README.md b/README.md index c51eb113..9cf7b09e 100644 --- a/README.md +++ b/README.md @@ -15,7 +15,7 @@ for the related charm, enabling the related charm to manage its own TLS keys loc Deploy the charm: ```bash -juju deploy vault-k8s --trust +juju deploy vault-k8s -n 5 --trust ``` ### Retrieve Vault's Root token diff --git a/metadata.yaml b/metadata.yaml index 8681267f..067efc63 100644 --- a/metadata.yaml +++ b/metadata.yaml @@ -28,7 +28,7 @@ resources: upstream-source: vault:1.13.3 storage: - vault-storage: + vault-raft: type: filesystem minimum-size: 10G diff --git a/src/charm.py b/src/charm.py index c43fd739..7d054d8e 100755 --- a/src/charm.py +++ b/src/charm.py @@ -15,9 +15,21 @@ KubernetesServicePatch, ServicePort, ) -from ops.charm import CharmBase, ConfigChangedEvent, InstallEvent, RelationJoinedEvent +from ops.charm import ( + CharmBase, + ConfigChangedEvent, + InstallEvent, + RelationJoinedEvent, + RemoveEvent, +) from ops.main import main -from ops.model import ActiveStatus, MaintenanceStatus, ModelError, WaitingStatus +from ops.model import ( + ActiveStatus, + MaintenanceStatus, + ModelError, + SecretNotFoundError, + WaitingStatus, +) from ops.pebble import Layer from vault import Vault @@ -38,16 +50,17 @@ def __init__(self, *args): super().__init__(*args) self._service_name = self._container_name = "vault" self._container = self.unit.get_container(self._container_name) - self.framework.observe(self.on.install, self._on_install) - self.framework.observe(self.on.vault_pebble_ready, self._on_config_changed) - self.framework.observe(self.on.config_changed, self._on_config_changed) self.service_patcher = KubernetesServicePatch( charm=self, ports=[ServicePort(name="vault", port=self.VAULT_PORT)], ) + self.framework.observe(self.on.install, self._on_install) + self.framework.observe(self.on.vault_pebble_ready, self._on_config_changed) + self.framework.observe(self.on.config_changed, self._on_config_changed) self.framework.observe( self.on[PEER_RELATION_NAME].relation_created, self._on_peer_relation_created ) + self.framework.observe(self.on.remove, self._on_remove) def _on_install(self, event: InstallEvent): if not self.unit.is_leader(): @@ -67,7 +80,10 @@ def _on_install(self, event: InstallEvent): self.unit.status = MaintenanceStatus("Initializing vault") self._set_pebble_plan() vault = Vault(url=self._api_address) - vault.wait_for_api_available() + if not vault.is_api_available(): + self.unit.status = WaitingStatus("Waiting for vault to be available") + event.defer() + return root_token, unseal_keys = vault.initialize() self._set_initialization_secret_in_peer_relation(root_token, unseal_keys) vault.set_token(token=root_token) @@ -96,18 +112,56 @@ def _on_config_changed(self, event: ConfigChangedEvent) -> None: self.unit.status = WaitingStatus("Waiting for vault initialization secret") event.defer() return + if not self.unit.is_leader() and len(self._other_peer_unit_addresses()) == 0: + self.unit.status = WaitingStatus("Waiting for other units to provide their addresses") + event.defer() + return + self.unit.status = MaintenanceStatus("Preparing vault") self._set_pebble_plan() vault = Vault(url=self._api_address) vault.set_token(token=root_token) - vault.wait_for_api_available() + if not vault.is_api_available(): + self.unit.status = WaitingStatus("Waiting for vault to be available") + event.defer() + return + if not vault.is_initialized(): + self.unit.status = WaitingStatus("Waiting for vault to be initialized") + event.defer() + return if vault.is_sealed(): vault.unseal(unseal_keys=unseal_keys) + self._set_peer_relation_unit_address() self.unit.status = ActiveStatus() def _on_peer_relation_created(self, event: RelationJoinedEvent) -> None: """Handle relation-joined event for the replicas relation.""" self._set_peer_relation_unit_address() + def _on_remove(self, event: RemoveEvent): + """Handler triggered when the charm is removed. + + Removes the vault service and the raft data and removes the node from the raft cluster. + """ + if not self._container.can_connect(): + return + root_token, unseal_keys = self._get_initialization_secret_from_peer_relation() + if root_token: + vault = Vault(url=self._api_address) + vault.set_token(token=root_token) + if vault.is_api_available() and vault.node_in_raft_peers(node_id=self._node_id): + vault.remove_raft_node(node_id=self._node_id) + if self._vault_service_is_running(): + self._container.stop(self._service_name) + self._container.remove_path(path=f"{VAULT_RAFT_DATA_PATH}/*", recursive=True) + + def _vault_service_is_running(self) -> bool: + """Check if the vault service is running.""" + try: + self._container.get_service(service_name=self._service_name) + except ModelError: + return False + return True + @property def _api_address(self) -> str: return f"http://{self._bind_address}:{self.VAULT_PORT}" @@ -147,7 +201,10 @@ def _get_initialization_secret_from_peer_relation( ) if not juju_secret_id: return None, None - juju_secret = self.model.get_secret(id=juju_secret_id) + try: + juju_secret = self.model.get_secret(id=juju_secret_id) + except SecretNotFoundError: + return None, None content = juju_secret.get_content() return content["roottoken"], json.loads(content["unsealkeys"]) @@ -162,6 +219,7 @@ def _set_pebble_plan(self) -> None: if plan.services != layer.services: self._container.add_layer(self._container_name, layer, combine=True) self._container.replan() + logger.info("Pebble layer added") @property def _bind_address(self) -> Optional[str]: @@ -235,18 +293,25 @@ def _vault_layer(self) -> Layer: def _set_peer_relation_unit_address(self) -> None: peer_relation = self.model.get_relation(PEER_RELATION_NAME) + if not peer_relation: + raise RuntimeError("Peer relation not created") peer_relation.data[self.unit].update({"unit-address": self._api_address}) def _get_peer_relation_unit_addresses(self) -> List[str]: peer_relation = self.model.get_relation(PEER_RELATION_NAME) + unit_addresses = [] if not peer_relation: return [] - return [peer_relation.data[peer]["unit-address"] for peer in peer_relation.units] + for peer in peer_relation.units: + if "unit-address" in peer_relation.data[peer]: + unit_addresses.append(peer_relation.data[peer]["unit-address"]) + return unit_addresses def _other_peer_unit_addresses(self) -> List[str]: return [ - unit_address for unit_address in self._get_peer_relation_unit_addresses() if - unit_address != self._api_address + unit_address + for unit_address in self._get_peer_relation_unit_addresses() + if unit_address != self._api_address ] def _get_raft_config(self) -> dict: diff --git a/src/vault.py b/src/vault.py index 4af07e14..b89c7449 100644 --- a/src/vault.py +++ b/src/vault.py @@ -5,7 +5,6 @@ """Contains all the specificities to communicate with Vault through its API.""" import logging -from time import sleep, time from typing import List, Tuple import hvac # type: ignore[import] @@ -48,8 +47,13 @@ def initialize( initialize_response = self._client.sys.initialize( secret_shares=secret_shares, secret_threshold=secret_threshold ) + logger.info("Vault is initialized") return initialize_response["root_token"], initialize_response["keys"] + def is_initialized(self) -> bool: + """Returns whether Vault is initialized.""" + return self._client.sys.is_initialized() + def is_sealed(self) -> bool: """Returns whether Vault is sealed.""" return self._client.sys.is_sealed() @@ -58,24 +62,7 @@ def unseal(self, unseal_keys: List[str]) -> None: """Unseal Vault.""" for unseal_key in unseal_keys: self._client.sys.submit_unseal_key(unseal_key) - - def wait_to_be_ready(self, timeout: int = 30): - """Wait for vault to be ready.""" - start_time = time() - while time() - start_time < timeout: - if self.is_ready(): - return - sleep(2) - raise TimeoutError("Timed out waiting for vault to be ready") - - def wait_for_api_available(self, timeout: int = 30) -> None: - """Wait for vault to be available.""" - start_time = time() - while time() - start_time < timeout: - if self.is_api_available(): - return - sleep(2) - raise TimeoutError("Timed out waiting for vault to be available") + logger.info("Vault is unsealed") def is_api_available(self) -> bool: """Returns whether Vault is available.""" @@ -88,3 +75,16 @@ def is_api_available(self) -> bool: def set_token(self, token: str) -> None: """Sets the Vault token for authentication.""" self._client.token = token + + def remove_raft_node(self, node_id: str) -> None: + """Remove raft peer.""" + self._client.sys.remove_raft_node(server_id=node_id) + logger.info(f"Removed raft node {node_id}") + + def node_in_raft_peers(self, node_id: str) -> bool: + """Check if node is in raft peers.""" + raft_config = self._client.sys.read_raft_config() + for peer in raft_config["data"]["config"]["servers"]: + if peer["node_id"] == node_id: + return True + return False diff --git a/tests/integration/test_integration.py b/tests/integration/test_integration.py index fc8f26b7..1c804c87 100644 --- a/tests/integration/test_integration.py +++ b/tests/integration/test_integration.py @@ -33,6 +33,7 @@ async def deploy_charm(ops_test: OpsTest, charm: Path) -> None: application_name=APPLICATION_NAME, trust=True, series="jammy", + num_units=5, ) @pytest.mark.abort_on_fail @@ -54,3 +55,25 @@ async def test_given_default_config_when_deploy_then_status_is_active( await ops_test.model.wait_for_idle( # type: ignore[union-attr] apps=[APPLICATION_NAME], status="active", timeout=1000 ) + + @pytest.mark.abort_on_fail + async def test_given_application_is_deployed_when_scale_up_then_status_is_active( + self, + ops_test: OpsTest, + ): + await ops_test.model.applications[APPLICATION_NAME].scale(6) + + await ops_test.model.wait_for_idle( # type: ignore[union-attr] + apps=[APPLICATION_NAME], status="active", timeout=1000 + ) + + @pytest.mark.abort_on_fail + async def test_given_application_is_deployed_when_scale_down_then_status_is_active( + self, + ops_test: OpsTest, + ): + await ops_test.model.applications[APPLICATION_NAME].scale(3) + + await ops_test.model.wait_for_idle( # type: ignore[union-attr] + apps=[APPLICATION_NAME], status="active", timeout=1000 + ) diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py index 903ceae0..1b6f9855 100644 --- a/tests/unit/test_charm.py +++ b/tests/unit/test_charm.py @@ -1,5 +1,5 @@ #!/usr/bin/env python3 -# Copyright 2021 Canonical Ltd. +# Copyright 2023 Canonical Ltd. # See LICENSE file for licensing details. import json @@ -29,8 +29,10 @@ class TestCharm(unittest.TestCase): lambda charm, ports: None, ) def setUp(self): + self.model_name = "whatever" self.harness = testing.Harness(VaultCharm) self.addCleanup(self.harness.cleanup) + self.harness.set_model_name(name=self.model_name) self.harness.begin() self.container_name = "vault" self.app_name = "vault-k8s" @@ -55,6 +57,15 @@ def _set_initialization_secret_in_peer_relation( key_values=key_values, ) + def _set_other_unit_api_address_in_peer_relation(self, relation_id: int, unit_name: str): + """Set the other unit api address in the peer relation.""" + key_values = {"unit-address": "http://5.2.1.9:8200"} + self.harness.update_relation_data( + app_or_unit=unit_name, + relation_id=relation_id, + key_values=key_values, + ) + def test_given_cant_connect_to_workload_when_install_then_status_is_waiting(self): self.harness.set_leader(is_leader=True) self.harness.set_can_connect(container=self.container_name, val=False) @@ -96,12 +107,12 @@ def test_given_bind_address_not_available_when_install_then_status_is_waiting( @patch("vault.Vault.unseal") @patch("vault.Vault.set_token") @patch("vault.Vault.initialize") - @patch("vault.Vault.wait_for_api_available") + @patch("vault.Vault.is_api_available") @patch("ops.model.Model.get_binding") def test_given_binding_address_when_install_then_vault_is_initialized( self, patch_get_binding, - patch_vault_wait_for_api_available, + patch_is_api_available, patch_vault_initialize, patch_vault_set_token, patch_vault_unseal, @@ -110,13 +121,13 @@ def test_given_binding_address_when_install_then_vault_is_initialized( unseal_keys = ["unseal key 1"] bind_address = "1.2.1.2" patch_get_binding.return_value = MockBinding(bind_address=bind_address) + patch_is_api_available.return_value = True self.harness.set_leader(is_leader=True) self.harness.set_can_connect(container=self.container_name, val=True) patch_vault_initialize.return_value = root_token, unseal_keys self._set_peer_relation() self.harness.charm.on.install.emit() - patch_vault_wait_for_api_available.assert_called_once() patch_vault_initialize.assert_called_once() patch_vault_set_token.assert_called_once_with(token=root_token) patch_vault_unseal.assert_called_once_with(unseal_keys=unseal_keys) @@ -124,18 +135,31 @@ def test_given_binding_address_when_install_then_vault_is_initialized( @patch("vault.Vault.unseal", new=Mock) @patch("vault.Vault.set_token", new=Mock) @patch("vault.Vault.initialize") - @patch("vault.Vault.wait_for_api_available", new=Mock) + @patch("vault.Vault.is_api_available") @patch("ops.model.Model.get_binding") def test_given_binding_address_when_install_then_pebble_is_planned( - self, patch_get_binding, patch_vault_initialize + self, patch_get_binding, patch_is_api_available, patch_vault_initialize ): bind_address = "1.2.1.2" patch_get_binding.return_value = MockBinding(bind_address=bind_address) + patch_is_api_available.return_value = True self.harness.set_leader(is_leader=True) self.harness.set_can_connect(container=self.container_name, val=True) patch_vault_initialize.return_value = "root token content", "unseal key content" self._set_peer_relation() + expected_vault_config = { + "ui": True, + "storage": { + "raft": {"path": "/vault/raft", "node_id": f"{self.model_name}-{self.app_name}/0"} + }, + "listener": {"tcp": {"tls_disable": True, "address": "[::]:8200"}}, + "default_lease_ttl": "168h", + "max_lease_ttl": "720h", + "disable_mlock": True, + "cluster_addr": f"http://{bind_address}:8201", + "api_addr": f"http://{bind_address}:8200", + } expected_plan = { "services": { "vault": { @@ -144,14 +168,7 @@ def test_given_binding_address_when_install_then_pebble_is_planned( "command": "/usr/local/bin/docker-entrypoint.sh server", "startup": "enabled", "environment": { - "VAULT_LOCAL_CONFIG": '{"backend": {"file": {"path": "/srv"}}, ' - '"listener": {"tcp": {' - '"tls_disable": true, "address": "[::]:8200"}}, ' - '"default_lease_ttl": "168h", "max_lease_ttl": "720h", ' - '"disable_mlock": true, ' - f'"cluster_addr": "http://{bind_address}:8201", ' - f'"api_addr": "http://{bind_address}:8200"' - "}", + "VAULT_LOCAL_CONFIG": json.dumps(expected_vault_config), "VAULT_API_ADDR": "http://[::]:8200", }, } @@ -165,12 +182,13 @@ def test_given_binding_address_when_install_then_pebble_is_planned( @patch("vault.Vault.unseal", new=Mock) @patch("vault.Vault.set_token", new=Mock) @patch("vault.Vault.initialize") - @patch("vault.Vault.wait_for_api_available", new=Mock) + @patch("vault.Vault.is_api_available") @patch("ops.model.Model.get_binding") def test_given_binding_address_when_install_then_status_is_active( - self, patch_get_binding, patch_vault_initialize + self, patch_get_binding, patch_is_api_available, patch_vault_initialize ): patch_get_binding.return_value = MockBinding(bind_address="1.2.1.2") + patch_is_api_available.return_value = True self.harness.set_leader(is_leader=True) self.harness.set_can_connect(container=self.container_name, val=True) patch_vault_initialize.return_value = "root token content", "unseal key content" @@ -180,6 +198,24 @@ def test_given_binding_address_when_install_then_status_is_active( self.assertEqual(self.harness.charm.unit.status, ActiveStatus()) + @patch("vault.Vault.is_api_available") + @patch("ops.model.Model.get_binding") + def test_given_vault_not_available_when_install_then_status_is_waiting( + self, patch_get_binding, patch_is_api_available + ): + self._set_peer_relation() + self.harness.set_leader(is_leader=True) + self.harness.set_can_connect(container=self.container_name, val=True) + patch_get_binding.return_value = MockBinding(bind_address="1.2.1.2") + patch_is_api_available.return_value = False + + self.harness.charm.on.install.emit() + + self.assertEqual( + self.harness.charm.unit.status, + WaitingStatus("Waiting for vault to be available"), + ) + def test_given_cant_connect_to_workload_when_config_changed_then_status_is_waiting(self): self.harness.set_leader(is_leader=True) self.harness.set_can_connect(container=self.container_name, val=False) @@ -216,15 +252,41 @@ def test_given_initialization_secret_not_stored_when_config_changed_then_status_ WaitingStatus("Waiting for vault initialization secret"), ) + def test_given_other_unit_addresses_not_available_when_config_changed_then_status_is_waiting( + self, + ): + self.harness.set_leader(is_leader=False) + self.harness.set_can_connect(container=self.container_name, val=True) + peer_relation_id = self._set_peer_relation() + self._set_initialization_secret_in_peer_relation( + relation_id=peer_relation_id, + root_token="root token content", + unseal_keys=["unseal key content"], + ) + + self.harness.charm.on.config_changed.emit() + + self.assertEqual( + self.harness.charm.unit.status, + WaitingStatus("Waiting for other units to provide their addresses"), + ) + @patch("ops.model.Model.get_binding") @patch("vault.Vault.is_sealed") - @patch("vault.Vault.wait_for_api_available", new=Mock) + @patch("vault.Vault.is_initialized") + @patch("vault.Vault.is_api_available") @patch("ops.model.Container.exec", new=Mock) def test_given_initialization_secret_is_stored_when_config_changed_then_pebble_plan_is_applied( - self, patch_vault_is_sealed, patch_get_binding + self, + patch_is_api_available, + patch_is_initialized, + patch_vault_is_sealed, + patch_get_binding, ): bind_address = "1.2.3.4" patch_vault_is_sealed.return_value = False + patch_is_api_available.return_value = True + patch_is_initialized.return_value = True self.harness.set_leader(is_leader=True) self.harness.set_can_connect(container=self.container_name, val=True) peer_relation_id = self._set_peer_relation() @@ -237,6 +299,18 @@ def test_given_initialization_secret_is_stored_when_config_changed_then_pebble_p self.harness.charm.on.config_changed.emit() + expected_vault_config = { + "ui": True, + "storage": { + "raft": {"path": "/vault/raft", "node_id": f"{self.model_name}-{self.app_name}/0"} + }, + "listener": {"tcp": {"tls_disable": True, "address": "[::]:8200"}}, + "default_lease_ttl": "168h", + "max_lease_ttl": "720h", + "disable_mlock": True, + "cluster_addr": f"http://{bind_address}:8201", + "api_addr": f"http://{bind_address}:8200", + } expected_plan = { "services": { "vault": { @@ -245,14 +319,7 @@ def test_given_initialization_secret_is_stored_when_config_changed_then_pebble_p "command": "/usr/local/bin/docker-entrypoint.sh server", "startup": "enabled", "environment": { - "VAULT_LOCAL_CONFIG": '{"backend": {"file": {"path": "/srv"}}, ' - '"listener": {"tcp": {' - '"tls_disable": true, "address": "[::]:8200"}}, ' - '"default_lease_ttl": "168h", "max_lease_ttl": "720h", ' - '"disable_mlock": true, ' - f'"cluster_addr": "http://{bind_address}:8201", ' - f'"api_addr": "http://{bind_address}:8200"' - "}", + "VAULT_LOCAL_CONFIG": json.dumps(expected_vault_config), "VAULT_API_ADDR": "http://[::]:8200", }, } @@ -265,12 +332,19 @@ def test_given_initialization_secret_is_stored_when_config_changed_then_pebble_p @patch("ops.model.Model.get_binding") @patch("vault.Vault.is_sealed") - @patch("vault.Vault.wait_for_api_available", new=Mock) + @patch("vault.Vault.is_initialized") + @patch("vault.Vault.is_api_available") @patch("ops.model.Container.exec", new=Mock) def test_given_initialization_secret_is_stored_when_config_changed_then_status_is_active( - self, patch_vault_is_sealed, patch_get_binding + self, + patch_is_api_available, + patch_is_initialized, + patch_vault_is_sealed, + patch_get_binding, ): patch_vault_is_sealed.return_value = False + patch_is_api_available.return_value = True + patch_is_initialized.return_value = True self.harness.set_leader(is_leader=True) self.harness.set_can_connect(container=self.container_name, val=True) peer_relation_id = self._set_peer_relation() @@ -288,11 +362,19 @@ def test_given_initialization_secret_is_stored_when_config_changed_then_status_i @patch("ops.model.Model.get_binding") @patch("vault.Vault.unseal") @patch("vault.Vault.is_sealed") - @patch("vault.Vault.wait_for_api_available", new=Mock) + @patch("vault.Vault.is_initialized") + @patch("vault.Vault.is_api_available") @patch("ops.model.Container.exec", new=Mock) def test_given_vault_is_sealed_when_config_changed_then_vault_is_unsealed( - self, patch_vault_is_sealed, patch_vault_unseal, patch_get_binding + self, + patch_is_api_available, + patch_is_initialized, + patch_vault_is_sealed, + patch_vault_unseal, + patch_get_binding, ): + patch_is_api_available.return_value = True + patch_is_initialized.return_value = True unseal_keys = ["unseal key content"] patch_vault_is_sealed.return_value = True self.harness.set_leader(is_leader=True) @@ -308,3 +390,158 @@ def test_given_vault_is_sealed_when_config_changed_then_vault_is_unsealed( self.harness.charm.on.config_changed.emit() patch_vault_unseal.assert_called_once_with(unseal_keys=unseal_keys) + + @patch("ops.model.Model.get_binding") + @patch("vault.Vault.unseal", new=Mock) + @patch("vault.Vault.is_sealed", new=Mock) + @patch("vault.Vault.is_initialized", new=Mock) + @patch("vault.Vault.is_api_available") + @patch("ops.model.Container.exec", new=Mock) + def test_given_vault_api_not_available_when_config_changed_then_status_is_waiting( + self, patch_is_api_available, patch_get_binding + ): + self.harness.set_can_connect(container=self.container_name, val=True) + patch_is_api_available.return_value = False + self.harness.set_leader(is_leader=False) + peer_relation_id = self._set_peer_relation() + other_unit_name = f"{self.app_name}/1" + self.harness.add_relation_unit( + relation_id=peer_relation_id, remote_unit_name=other_unit_name + ) + self._set_initialization_secret_in_peer_relation( + relation_id=peer_relation_id, + root_token="root token content", + unseal_keys=["unseal_keys"], + ) + self._set_other_unit_api_address_in_peer_relation( + relation_id=peer_relation_id, + unit_name=other_unit_name, + ) + patch_get_binding.return_value = MockBinding(bind_address="1.2.3.4") + + self.harness.charm.on.config_changed.emit() + + self.assertEqual( + self.harness.charm.unit.status, + WaitingStatus("Waiting for vault to be available"), + ) + + @patch("ops.model.Model.get_binding") + @patch("vault.Vault.unseal", new=Mock) + @patch("vault.Vault.is_sealed", new=Mock) + @patch("vault.Vault.is_initialized") + @patch("vault.Vault.is_api_available") + @patch("ops.model.Container.exec", new=Mock) + def test_given_vault_is_not_initialized_when_config_changed_then_status_is_waiting( + self, patch_is_api_available, patch_is_initialized, patch_get_binding + ): + self.harness.set_can_connect(container=self.container_name, val=True) + patch_is_api_available.return_value = True + patch_is_initialized.return_value = False + self.harness.set_leader(is_leader=False) + peer_relation_id = self._set_peer_relation() + other_unit_name = f"{self.app_name}/1" + self.harness.add_relation_unit( + relation_id=peer_relation_id, remote_unit_name=other_unit_name + ) + self._set_initialization_secret_in_peer_relation( + relation_id=peer_relation_id, + root_token="root token content", + unseal_keys=["unseal_keys"], + ) + self._set_other_unit_api_address_in_peer_relation( + relation_id=peer_relation_id, + unit_name=other_unit_name, + ) + patch_get_binding.return_value = MockBinding(bind_address="1.2.3.4") + + self.harness.charm.on.config_changed.emit() + + self.assertEqual( + self.harness.charm.unit.status, + WaitingStatus("Waiting for vault to be initialized"), + ) + + @patch("ops.model.Container.remove_path") + def test_given_can_connect_when_on_remove_then_raft_storage_path_is_deleted( + self, patch_remove_path + ): + self.harness.set_can_connect(container=self.container_name, val=True) + + self.harness.charm.on.remove.emit() + + patch_remove_path.assert_called_with(path="/vault/raft/*", recursive=True) + + @patch("vault.Vault.is_api_available") + @patch("vault.Vault.node_in_raft_peers") + @patch("vault.Vault.remove_raft_node") + @patch("ops.model.Container.remove_path", new=Mock) + def test_given_node_in_raft_when_on_remove_then_node_is_removed_from_raft( + self, + patch_remove_raft_node, + patch_node_in_raft_peers, + patch_is_api_available, + ): + self.harness.set_can_connect(container=self.container_name, val=True) + patch_is_api_available.return_value = True + patch_node_in_raft_peers.return_value = True + peer_relation_id = self._set_peer_relation() + self._set_initialization_secret_in_peer_relation( + relation_id=peer_relation_id, + root_token="root token content", + unseal_keys=["unseal_keys"], + ) + + self.harness.charm.on.remove.emit() + + patch_remove_raft_node.assert_called_with(node_id=f"{self.model_name}-{self.app_name}/0") + + @patch("vault.Vault.is_api_available") + @patch("vault.Vault.node_in_raft_peers") + @patch("vault.Vault.remove_raft_node") + @patch("ops.model.Container.remove_path", new=Mock) + def test_given_node_not_in_raft_when_on_remove_then_node_is_not_removed_from_raft( + self, + patch_remove_raft_node, + patch_node_in_raft_peers, + patch_is_api_available, + ): + self.harness.set_can_connect(container=self.container_name, val=True) + patch_is_api_available.return_value = True + patch_node_in_raft_peers.return_value = False + peer_relation_id = self._set_peer_relation() + self._set_initialization_secret_in_peer_relation( + relation_id=peer_relation_id, + root_token="root token content", + unseal_keys=["unseal_keys"], + ) + + self.harness.charm.on.remove.emit() + + patch_remove_raft_node.assert_not_called() + + @patch("vault.Vault.is_api_available") + @patch("vault.Vault.node_in_raft_peers") + @patch("vault.Vault.remove_raft_node", new=Mock) + @patch("ops.model.Container.get_service", new=Mock) + @patch("ops.model.Container.stop") + @patch("ops.model.Container.remove_path", new=Mock) + def test_given_service_is_running_when_on_remove_then_service_is_stopped( + self, + patch_stop_service, + patch_node_in_raft_peers, + patch_is_api_available, + ): + self.harness.set_can_connect(container=self.container_name, val=True) + patch_is_api_available.return_value = True + patch_node_in_raft_peers.return_value = False + peer_relation_id = self._set_peer_relation() + self._set_initialization_secret_in_peer_relation( + relation_id=peer_relation_id, + root_token="root token content", + unseal_keys=["unseal_keys"], + ) + + self.harness.charm.on.remove.emit() + + patch_stop_service.assert_called_with("vault") diff --git a/tests/unit/test_vault.py b/tests/unit/test_vault.py index c8e014ac..0e9c1ca9 100644 --- a/tests/unit/test_vault.py +++ b/tests/unit/test_vault.py @@ -3,7 +3,7 @@ # See LICENSE file for licensing details. import unittest -from unittest.mock import Mock, call, patch +from unittest.mock import call, patch import requests @@ -95,44 +95,6 @@ def test_given_n_unseal_keys_when_unseal_then_unseal_called_n_times( [call(unseal_key) for unseal_key in unseal_keys] ) - @patch("vault.Vault.is_ready") - def test_given_is_ready_when_wait_to_be_ready_then_returns(self, patch_is_ready): - patch_is_ready.return_value = True - - vault = Vault(url="http://whatever-url") - - vault.wait_to_be_ready(timeout=1) - - @patch("vault.Vault.is_ready") - @patch("vault.sleep", new=Mock) - def test_given_is_not_ready_when_wait_to_be_ready_then_timeout_error(self, patch_is_ready): - patch_is_ready.return_value = False - vault = Vault(url="http://whatever-url") - - with self.assertRaises(TimeoutError): - vault.wait_to_be_ready(timeout=1) - - @patch("vault.Vault.is_api_available") - def test_given_api_available_when_wait_for_api_available_then_returns( - self, patch_is_api_available - ): - patch_is_api_available.return_value = True - vault = Vault(url="http://whatever-url") - - vault.wait_for_api_available(timeout=1) - - @patch("vault.sleep", new=Mock) - @patch("vault.Vault.is_api_available") - def test_given_api_not_available_when_wait_for_api_available_then_timeouterror( - self, - patch_is_api_available, - ): - patch_is_api_available.return_value = False - vault = Vault(url="http://whatever-url") - - with self.assertRaises(TimeoutError): - vault.wait_for_api_available(timeout=1) - @patch("hvac.api.system_backend.health.Health.read_health_status") def test_given_connection_error_when_is_api_available_then_return_false( self, patch_health_status @@ -148,3 +110,27 @@ def test_given_api_returns_when_is_api_available_then_return_true(self, patch_he vault = Vault(url="http://whatever-url") self.assertTrue(vault.is_api_available()) + + @patch("hvac.api.system_backend.raft.Raft.read_raft_config") + def test_given_node_in_peer_list_when_node_in_raft_peers_then_returns_true( + self, patch_health_status + ): + node_id = "whatever node id" + vault = Vault(url="http://whatever-url") + patch_health_status.return_value = { + "data": {"config": {"servers": [{"node_id": node_id}]}} + } + + self.assertTrue(vault.node_in_raft_peers(node_id=node_id)) + + @patch("hvac.api.system_backend.raft.Raft.read_raft_config") + def test_given_node_not_in_peer_list_when_node_in_raft_peers_then_returns_false( + self, patch_health_status + ): + node_id = "whatever node id" + vault = Vault(url="http://whatever-url") + patch_health_status.return_value = { + "data": {"config": {"servers": [{"node_id": "not our node"}]}} + } + + self.assertFalse(vault.node_in_raft_peers(node_id=node_id)) From 629c8413d180613bb601d01f4ca180ec53a122a3 Mon Sep 17 00:00:00 2001 From: Guillaume Belanger Date: Wed, 6 Sep 2023 17:55:49 -0400 Subject: [PATCH 03/14] Fixes static analysis warning --- tests/integration/test_integration.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/integration/test_integration.py b/tests/integration/test_integration.py index 1c804c87..1fab28f0 100644 --- a/tests/integration/test_integration.py +++ b/tests/integration/test_integration.py @@ -61,7 +61,7 @@ async def test_given_application_is_deployed_when_scale_up_then_status_is_active self, ops_test: OpsTest, ): - await ops_test.model.applications[APPLICATION_NAME].scale(6) + await ops_test.model.applications[APPLICATION_NAME].scale(6) # type: ignore[union-attr] await ops_test.model.wait_for_idle( # type: ignore[union-attr] apps=[APPLICATION_NAME], status="active", timeout=1000 @@ -72,7 +72,7 @@ async def test_given_application_is_deployed_when_scale_down_then_status_is_acti self, ops_test: OpsTest, ): - await ops_test.model.applications[APPLICATION_NAME].scale(3) + await ops_test.model.applications[APPLICATION_NAME].scale(3) # type: ignore[union-attr] await ops_test.model.wait_for_idle( # type: ignore[union-attr] apps=[APPLICATION_NAME], status="active", timeout=1000 From 551ddefe36681e375e5c5c5340bfa704f577e895 Mon Sep 17 00:00:00 2001 From: guillaume Date: Fri, 8 Sep 2023 08:49:41 -0400 Subject: [PATCH 04/14] Improves integration tests --- test-requirements.txt | 18 +++++++++++++ tests/integration/test_integration.py | 4 ++- tests/unit/test_vault.py | 2 +- tox.ini | 39 ++++----------------------- 4 files changed, 27 insertions(+), 36 deletions(-) create mode 100644 test-requirements.txt diff --git a/test-requirements.txt b/test-requirements.txt new file mode 100644 index 00000000..847fe9ad --- /dev/null +++ b/test-requirements.txt @@ -0,0 +1,18 @@ +black +coverage[toml] +flake8 +flake8-docstrings +flake8-copyright +flake8-builtins +hvac +isort +juju +mypy +pep8-naming +pyproject-flake8 +pytest +pytest-operator +types-PyYAML +types-setuptools +types-toml +types-requests diff --git a/tests/integration/test_integration.py b/tests/integration/test_integration.py index 1fab28f0..9c8338f6 100644 --- a/tests/integration/test_integration.py +++ b/tests/integration/test_integration.py @@ -60,8 +60,9 @@ async def test_given_default_config_when_deploy_then_status_is_active( async def test_given_application_is_deployed_when_scale_up_then_status_is_active( self, ops_test: OpsTest, + build_and_deploy, ): - await ops_test.model.applications[APPLICATION_NAME].scale(6) # type: ignore[union-attr] + await ops_test.model.applications[APPLICATION_NAME].scale(7) # type: ignore[union-attr] await ops_test.model.wait_for_idle( # type: ignore[union-attr] apps=[APPLICATION_NAME], status="active", timeout=1000 @@ -71,6 +72,7 @@ async def test_given_application_is_deployed_when_scale_up_then_status_is_active async def test_given_application_is_deployed_when_scale_down_then_status_is_active( self, ops_test: OpsTest, + build_and_deploy, ): await ops_test.model.applications[APPLICATION_NAME].scale(3) # type: ignore[union-attr] diff --git a/tests/unit/test_vault.py b/tests/unit/test_vault.py index 0e9c1ca9..3752405d 100644 --- a/tests/unit/test_vault.py +++ b/tests/unit/test_vault.py @@ -1,5 +1,5 @@ #!/usr/bin/env python3 -# Copyright 2021 Canonical Ltd. +# Copyright 2023 Canonical Ltd. # See LICENSE file for licensing details. import unittest diff --git a/tox.ini b/tox.ini index a9c13278..f45ca7f6 100644 --- a/tox.ini +++ b/tox.ini @@ -1,4 +1,4 @@ -# Copyright 2021 Canonical Ltd. +# Copyright 2023 Canonical Ltd. # See LICENSE file for licensing details. [tox] skipsdist=True @@ -15,32 +15,22 @@ all_path = {[vars]src_path} {[vars]unit_test_path} {[vars]integration_test_path} setenv = PYTHONPATH = {toxinidir}:{toxinidir}/lib:{[vars]src_path} PYTHONBREAKPOINT=ipdb.set_trace +deps = + -r{toxinidir}/requirements.txt + -r{toxinidir}/test-requirements.txt passenv = PYTHONPATH - HOME CHARM_BUILD_DIR MODEL_SETTINGS [testenv:fmt] description = Apply coding style standards to code -deps = - black - isort commands = isort {[vars]all_path} black {[vars]all_path} [testenv:lint] description = Check code against coding style standards -deps = - black - flake8 == 4.0.1 - flake8-docstrings - flake8-copyright - flake8-builtins - pyproject-flake8 - pep8-naming - isort commands = pflake8 {[vars]all_path} isort --check-only --diff {[vars]all_path} @@ -48,16 +38,6 @@ commands = [testenv:static] description = Run static analysis checks -deps = - -r{toxinidir}/requirements.txt - mypy - types-PyYAML - pytest - pytest-operator - juju - types-setuptools - types-toml - types-requests setenv = PYTHONPATH = "" commands = @@ -65,20 +45,11 @@ commands = [testenv:unit] description = Run unit tests -deps = - pytest - coverage[toml] - -r{toxinidir}/requirements.txt commands = coverage run --source={[vars]src_path} -m pytest {[vars]unit_test_path} -v --tb native -s {posargs} coverage report [testenv:integration] description = Run integration tests -deps = - juju - pytest - pytest-operator - -r{toxinidir}/requirements.txt commands = - pytest --asyncio-mode=auto -v --tb native {[vars]integration_test_path} --log-cli-level=INFO -s {posargs} --destructive-mode + pytest --asyncio-mode=auto -v --tb native {[vars]integration_test_path} --log-cli-level=INFO -s {posargs} From cf31d4f9158d1e610610a19c1cd3c9d114304433 Mon Sep 17 00:00:00 2001 From: guillaume Date: Fri, 8 Sep 2023 08:59:20 -0400 Subject: [PATCH 05/14] Improves docstring --- README.md | 7 ++----- src/charm.py | 12 +++++++++--- 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/README.md b/README.md index 9cf7b09e..20ffdedb 100644 --- a/README.md +++ b/README.md @@ -3,11 +3,8 @@ This charm deploys [Vault][vault-upstream], a tool for securely managing secrets used in modern computing (e.g. passwords, certificates, API keys). -In addition to deploying and initializing Vault, this charm provides a relation -for other charms to request that Vault's Certificate Authority (CA) sign a certificate -for the related charm, enabling the related charm to manage its own TLS keys locally. - -> **Note**: This charm does not support high-availability / scaling . +In addition to deploying and initializing Vault, this charm supports high availability mode using +the Raft backend. ## Usage diff --git a/src/charm.py b/src/charm.py index 7d054d8e..f23a5b03 100755 --- a/src/charm.py +++ b/src/charm.py @@ -9,7 +9,7 @@ import json import logging -from typing import List, Optional, Tuple +from typing import List, Optional, Tuple, Dict, Any from charms.observability_libs.v1.kubernetes_service_patch import ( KubernetesServicePatch, @@ -63,6 +63,7 @@ def __init__(self, *args): self.framework.observe(self.on.remove, self._on_remove) def _on_install(self, event: InstallEvent): + """Handler triggered when the charm is installed.""" if not self.unit.is_leader(): return if not self._container.can_connect(): @@ -164,6 +165,7 @@ def _vault_service_is_running(self) -> bool: @property def _api_address(self) -> str: + """Returns the API address.""" return f"http://{self._bind_address}:{self.VAULT_PORT}" def _set_initialization_secret_in_peer_relation( @@ -292,12 +294,14 @@ def _vault_layer(self) -> Layer: ) def _set_peer_relation_unit_address(self) -> None: + """Set the unit address in the peer relation.""" peer_relation = self.model.get_relation(PEER_RELATION_NAME) if not peer_relation: raise RuntimeError("Peer relation not created") peer_relation.data[self.unit].update({"unit-address": self._api_address}) def _get_peer_relation_unit_addresses(self) -> List[str]: + """Returns list of peer unit addresses.""" peer_relation = self.model.get_relation(PEER_RELATION_NAME) unit_addresses = [] if not peer_relation: @@ -308,13 +312,15 @@ def _get_peer_relation_unit_addresses(self) -> List[str]: return unit_addresses def _other_peer_unit_addresses(self) -> List[str]: + """Returns list of other peer unit addresses.""" return [ unit_address for unit_address in self._get_peer_relation_unit_addresses() if unit_address != self._api_address ] - def _get_raft_config(self) -> dict: + def _get_raft_config(self) -> Dict[str, Any]: + """Returns raft config for vault.""" retry_join = [ {"leader_api_addr": unit_address} for unit_address in self._other_peer_unit_addresses() ] @@ -327,7 +333,7 @@ def _get_raft_config(self) -> dict: return raft_config @property - def _node_id(self): + def _node_id(self) -> str: return f"{self.model.name}-{self.unit.name}" From 41609b6c4ffc8837188a7825c6a694628ffde211 Mon Sep 17 00:00:00 2001 From: guillaume Date: Fri, 8 Sep 2023 09:23:23 -0400 Subject: [PATCH 06/14] Improves docstring --- src/charm.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/charm.py b/src/charm.py index f23a5b03..c697b5ee 100755 --- a/src/charm.py +++ b/src/charm.py @@ -9,7 +9,7 @@ import json import logging -from typing import List, Optional, Tuple, Dict, Any +from typing import Any, Dict, List, Optional, Tuple from charms.observability_libs.v1.kubernetes_service_patch import ( KubernetesServicePatch, From c30a0cb1cf3a666955b392f13e3776d8e1df8899 Mon Sep 17 00:00:00 2001 From: guillaume Date: Fri, 8 Sep 2023 10:10:57 -0400 Subject: [PATCH 07/14] Fixes static analysis warning --- src/charm.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/charm.py b/src/charm.py index c697b5ee..3dc2239f 100755 --- a/src/charm.py +++ b/src/charm.py @@ -324,7 +324,7 @@ def _get_raft_config(self) -> Dict[str, Any]: retry_join = [ {"leader_api_addr": unit_address} for unit_address in self._other_peer_unit_addresses() ] - raft_config = { + raft_config: Dict[str, Any] = { "path": VAULT_RAFT_DATA_PATH, "node_id": self._node_id, } From af7daaa36a2193a66f44f99cd7f583e50374caf5 Mon Sep 17 00:00:00 2001 From: guillaume Date: Fri, 8 Sep 2023 10:33:46 -0400 Subject: [PATCH 08/14] Improves docstring --- src/charm.py | 47 +++++++++++++++++++++++++++++++++++++---------- 1 file changed, 37 insertions(+), 10 deletions(-) diff --git a/src/charm.py b/src/charm.py index 3dc2239f..c89c578c 100755 --- a/src/charm.py +++ b/src/charm.py @@ -63,7 +63,10 @@ def __init__(self, *args): self.framework.observe(self.on.remove, self._on_remove) def _on_install(self, event: InstallEvent): - """Handler triggered when the charm is installed.""" + """Handler triggered when the charm is installed. + + Sets pebble plan, initializes vault, and unseals vault. + """ if not self.unit.is_leader(): return if not self._container.can_connect(): @@ -94,11 +97,8 @@ def _on_install(self, event: InstallEvent): def _on_config_changed(self, event: ConfigChangedEvent) -> None: """Handler triggered whenever there is a config-changed event. - Args: - event: Juju event - - Returns: - None + Configures pebble layer, sets the unit address in the peer relation, starts the vault + service, and unseals Vault. """ if not self._container.can_connect(): self.unit.status = WaitingStatus("Waiting to be able to connect to vault unit") @@ -165,7 +165,10 @@ def _vault_service_is_running(self) -> bool: @property def _api_address(self) -> str: - """Returns the API address.""" + """Returns the API address. + + Example: "http://1.2.3.4:8200" + """ return f"http://{self._bind_address}:{self.VAULT_PORT}" def _set_initialization_secret_in_peer_relation( @@ -246,7 +249,9 @@ def _vault_layer(self) -> Layer: """Returns pebble layer to start Vault. Vault config options: - backend: Configures the storage backend where Vault data is stored. + ui: Enables the built-in static web UI. + storage: Configures the storage backend, which represents the location for the + durable storage of Vault's information. listener: Configures how Vault is listening for API requests. default_lease_ttl: Specifies the default lease duration for Vault's tokens and secrets. max_lease_ttl: Specifies the maximum possible lease duration for Vault's tokens and @@ -312,7 +317,10 @@ def _get_peer_relation_unit_addresses(self) -> List[str]: return unit_addresses def _other_peer_unit_addresses(self) -> List[str]: - """Returns list of other peer unit addresses.""" + """Returns list of other peer unit addresses. + + We exclude our own unit address from the list. + """ return [ unit_address for unit_address in self._get_peer_relation_unit_addresses() @@ -320,7 +328,22 @@ def _other_peer_unit_addresses(self) -> List[str]: ] def _get_raft_config(self) -> Dict[str, Any]: - """Returns raft config for vault.""" + """Returns raft config for vault. + + Example of raft config: + { + "path": "/vault/raft", + "node_id": "vault-k8s-0", + "retry_join": [ + { + "leader_api_addr": "http://1.2.3.4:8200" + }, + { + "leader_api_addr": "http://1.2.3.4:8200" + } + ] + } + """ retry_join = [ {"leader_api_addr": unit_address} for unit_address in self._other_peer_unit_addresses() ] @@ -334,6 +357,10 @@ def _get_raft_config(self) -> Dict[str, Any]: @property def _node_id(self) -> str: + """Returns node id for vault. + + Example of node id: "vault-k8s-0" + """ return f"{self.model.name}-{self.unit.name}" From c3ff9f84d6a68ab4f8126256b8533cec19771917 Mon Sep 17 00:00:00 2001 From: guillaume Date: Fri, 8 Sep 2023 10:48:56 -0400 Subject: [PATCH 09/14] Removes unneeded method --- src/charm.py | 2 +- src/vault.py | 11 -------- tests/unit/test_vault.py | 54 ---------------------------------------- 3 files changed, 1 insertion(+), 66 deletions(-) diff --git a/src/charm.py b/src/charm.py index c89c578c..0ad22290 100755 --- a/src/charm.py +++ b/src/charm.py @@ -339,7 +339,7 @@ def _get_raft_config(self) -> Dict[str, Any]: "leader_api_addr": "http://1.2.3.4:8200" }, { - "leader_api_addr": "http://1.2.3.4:8200" + "leader_api_addr": "http://5.6.7.8:8200" } ] } diff --git a/src/vault.py b/src/vault.py index b89c7449..3c95f617 100644 --- a/src/vault.py +++ b/src/vault.py @@ -25,17 +25,6 @@ class Vault: def __init__(self, url: str): self._client = hvac.Client(url=url) - def is_ready(self) -> bool: - """Returns whether Vault is ready for interaction.""" - if not self._client.sys.is_initialized(): - return False - if self.is_sealed(): - return False - health_status = self._client.sys.read_health_status() - if health_status.status_code != 200: - return False - return True - def initialize( self, secret_shares: int = 1, secret_threshold: int = 1 ) -> Tuple[str, List[str]]: diff --git a/tests/unit/test_vault.py b/tests/unit/test_vault.py index 3752405d..13fc867d 100644 --- a/tests/unit/test_vault.py +++ b/tests/unit/test_vault.py @@ -11,60 +11,6 @@ class TestVault(unittest.TestCase): - @patch("hvac.api.system_backend.Init.is_initialized") - def test_given_vault_not_initialized_when_is_ready_then_return_false( - self, patch_is_initialized - ): - patch_is_initialized.return_value = False - - vault = Vault(url="http://whatever-url") - - self.assertFalse(vault.is_ready()) - - @patch("hvac.api.system_backend.seal.Seal.is_sealed") - @patch("hvac.api.system_backend.Init.is_initialized") - def test_given_vault_is_sealed_when_is_ready_then_return_false( - self, patch_is_initialized, patch_is_sealed - ): - patch_is_initialized.return_value = True - patch_is_sealed.return_value = True - - vault = Vault(url="http://whatever-url") - - self.assertFalse(vault.is_ready()) - - @patch("hvac.api.system_backend.health.Health.read_health_status") - @patch("hvac.api.system_backend.seal.Seal.is_sealed") - @patch("hvac.api.system_backend.init.Init.is_initialized") - def test_given_vault_health_returns_40x_when_is_ready_then_return_false( - self, patch_is_initialized, patch_is_sealed, patch_read_health_status - ): - patch_is_initialized.return_value = True - patch_is_sealed.return_value = False - health_status_response = requests.Response() - health_status_response.status_code = 404 - patch_read_health_status.return_value = health_status_response - - vault = Vault(url="http://whatever-url") - - self.assertFalse(vault.is_ready()) - - @patch("hvac.api.system_backend.health.Health.read_health_status") - @patch("hvac.api.system_backend.seal.Seal.is_sealed") - @patch("hvac.api.system_backend.init.Init.is_initialized") - def test_given_vault_health_returns_200_when_is_ready_then_return_true( - self, patch_is_initialized, patch_is_sealed, patch_read_health_status - ): - patch_is_initialized.return_value = True - patch_is_sealed.return_value = False - health_status_response = requests.Response() - health_status_response.status_code = 200 - patch_read_health_status.return_value = health_status_response - - vault = Vault(url="http://whatever-url") - - self.assertTrue(vault.is_ready()) - @patch("hvac.api.system_backend.init.Init.initialize") def test_given_shares_and_threshold_when_initialize_then_root_token_and_unseal_key_returned( self, patch_initialize From 92574ce2cfa1a185d553373fa10a9a00ebcf5cf6 Mon Sep 17 00:00:00 2001 From: Guillaume Belanger Date: Mon, 11 Sep 2023 17:33:38 -0400 Subject: [PATCH 10/14] Update src/vault.py Co-authored-by: Ghislain Bourgeois --- src/vault.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/vault.py b/src/vault.py index 3c95f617..84cba843 100644 --- a/src/vault.py +++ b/src/vault.py @@ -68,7 +68,7 @@ def set_token(self, token: str) -> None: def remove_raft_node(self, node_id: str) -> None: """Remove raft peer.""" self._client.sys.remove_raft_node(server_id=node_id) - logger.info(f"Removed raft node {node_id}") + logger.info("Removed raft node %s", node_id) def node_in_raft_peers(self, node_id: str) -> bool: """Check if node is in raft peers.""" From 6b1417b13c3da8a18a587d6da16f00d07f499981 Mon Sep 17 00:00:00 2001 From: Guillaume Belanger Date: Mon, 11 Sep 2023 17:45:21 -0400 Subject: [PATCH 11/14] Waits for all units to be deployed in integration tests --- tests/integration/test_integration.py | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/tests/integration/test_integration.py b/tests/integration/test_integration.py index 9c8338f6..1735bd5a 100644 --- a/tests/integration/test_integration.py +++ b/tests/integration/test_integration.py @@ -53,7 +53,10 @@ async def test_given_default_config_when_deploy_then_status_is_active( self, ops_test: OpsTest, build_and_deploy ): await ops_test.model.wait_for_idle( # type: ignore[union-attr] - apps=[APPLICATION_NAME], status="active", timeout=1000 + apps=[APPLICATION_NAME], + status="active", + timeout=1000, + wait_for_exact_units=5, ) @pytest.mark.abort_on_fail @@ -62,10 +65,14 @@ async def test_given_application_is_deployed_when_scale_up_then_status_is_active ops_test: OpsTest, build_and_deploy, ): - await ops_test.model.applications[APPLICATION_NAME].scale(7) # type: ignore[union-attr] + num_units = 7 + await ops_test.model.applications[APPLICATION_NAME].scale(num_units) # type: ignore[union-attr] await ops_test.model.wait_for_idle( # type: ignore[union-attr] - apps=[APPLICATION_NAME], status="active", timeout=1000 + apps=[APPLICATION_NAME], + status="active", + timeout=1000, + wait_for_exact_units=num_units, ) @pytest.mark.abort_on_fail @@ -74,8 +81,12 @@ async def test_given_application_is_deployed_when_scale_down_then_status_is_acti ops_test: OpsTest, build_and_deploy, ): - await ops_test.model.applications[APPLICATION_NAME].scale(3) # type: ignore[union-attr] + num_units = 3 + await ops_test.model.applications[APPLICATION_NAME].scale(num_units) # type: ignore[union-attr] await ops_test.model.wait_for_idle( # type: ignore[union-attr] - apps=[APPLICATION_NAME], status="active", timeout=1000 + apps=[APPLICATION_NAME], + status="active", + timeout=1000, + wait_for_exact_units=num_units, ) From de1da658f7b78adc82cd29d54a7107a0a64bf527 Mon Sep 17 00:00:00 2001 From: Guillaume Belanger Date: Tue, 12 Sep 2023 07:59:48 -0400 Subject: [PATCH 12/14] Uses api address instead of unit-address because it's more specific to vault (with the scheme and port) --- src/charm.py | 42 ++++++++++++++++++++++------------------ tests/unit/test_charm.py | 35 ++++++++------------------------- 2 files changed, 31 insertions(+), 46 deletions(-) diff --git a/src/charm.py b/src/charm.py index 0ad22290..e5f10e35 100755 --- a/src/charm.py +++ b/src/charm.py @@ -11,6 +11,7 @@ import logging from typing import Any, Dict, List, Optional, Tuple +import requests from charms.observability_libs.v1.kubernetes_service_patch import ( KubernetesServicePatch, ServicePort, @@ -92,7 +93,6 @@ def _on_install(self, event: InstallEvent): self._set_initialization_secret_in_peer_relation(root_token, unseal_keys) vault.set_token(token=root_token) vault.unseal(unseal_keys=unseal_keys) - self.unit.status = ActiveStatus() def _on_config_changed(self, event: ConfigChangedEvent) -> None: """Handler triggered whenever there is a config-changed event. @@ -113,7 +113,7 @@ def _on_config_changed(self, event: ConfigChangedEvent) -> None: self.unit.status = WaitingStatus("Waiting for vault initialization secret") event.defer() return - if not self.unit.is_leader() and len(self._other_peer_unit_addresses()) == 0: + if not self.unit.is_leader() and len(self._other_peer_node_api_addresses()) == 0: self.unit.status = WaitingStatus("Waiting for other units to provide their addresses") event.defer() return @@ -131,12 +131,12 @@ def _on_config_changed(self, event: ConfigChangedEvent) -> None: return if vault.is_sealed(): vault.unseal(unseal_keys=unseal_keys) - self._set_peer_relation_unit_address() + self._set_peer_relation_node_api_address() self.unit.status = ActiveStatus() def _on_peer_relation_created(self, event: RelationJoinedEvent) -> None: """Handle relation-joined event for the replicas relation.""" - self._set_peer_relation_unit_address() + self._set_peer_relation_node_api_address() def _on_remove(self, event: RemoveEvent): """Handler triggered when the charm is removed. @@ -149,8 +149,11 @@ def _on_remove(self, event: RemoveEvent): if root_token: vault = Vault(url=self._api_address) vault.set_token(token=root_token) - if vault.is_api_available() and vault.node_in_raft_peers(node_id=self._node_id): - vault.remove_raft_node(node_id=self._node_id) + try: + if vault.is_api_available() and vault.node_in_raft_peers(node_id=self._node_id): + vault.remove_raft_node(node_id=self._node_id) + except (requests.exceptions.ConnectionError, requests.exceptions.TooManyRedirects): + pass if self._vault_service_is_running(): self._container.stop(self._service_name) self._container.remove_path(path=f"{VAULT_RAFT_DATA_PATH}/*", recursive=True) @@ -275,7 +278,7 @@ def _vault_layer(self) -> Layer: "default_lease_ttl": self.model.config["default_lease_ttl"], "max_lease_ttl": self.model.config["max_lease_ttl"], "disable_mlock": True, - "cluster_addr": f"http://{self._bind_address}:{self.VAULT_CLUSTER_PORT}", + "cluster_addr": f"https://{self._bind_address}:{self.VAULT_CLUSTER_PORT}", "api_addr": self._api_address, } @@ -298,33 +301,33 @@ def _vault_layer(self) -> Layer: } ) - def _set_peer_relation_unit_address(self) -> None: + def _set_peer_relation_node_api_address(self) -> None: """Set the unit address in the peer relation.""" peer_relation = self.model.get_relation(PEER_RELATION_NAME) if not peer_relation: raise RuntimeError("Peer relation not created") - peer_relation.data[self.unit].update({"unit-address": self._api_address}) + peer_relation.data[self.unit].update({"node_api_address": self._api_address}) - def _get_peer_relation_unit_addresses(self) -> List[str]: + def _get_peer_relation_node_api_addresses(self) -> List[str]: """Returns list of peer unit addresses.""" peer_relation = self.model.get_relation(PEER_RELATION_NAME) - unit_addresses = [] + node_api_addresses = [] if not peer_relation: return [] for peer in peer_relation.units: - if "unit-address" in peer_relation.data[peer]: - unit_addresses.append(peer_relation.data[peer]["unit-address"]) - return unit_addresses + if "node_api_address" in peer_relation.data[peer]: + node_api_addresses.append(peer_relation.data[peer]["node_api_address"]) + return node_api_addresses - def _other_peer_unit_addresses(self) -> List[str]: + def _other_peer_node_api_addresses(self) -> List[str]: """Returns list of other peer unit addresses. We exclude our own unit address from the list. """ return [ - unit_address - for unit_address in self._get_peer_relation_unit_addresses() - if unit_address != self._api_address + node_api_address + for node_api_address in self._get_peer_relation_node_api_addresses() + if node_api_address != self._api_address ] def _get_raft_config(self) -> Dict[str, Any]: @@ -345,7 +348,8 @@ def _get_raft_config(self) -> Dict[str, Any]: } """ retry_join = [ - {"leader_api_addr": unit_address} for unit_address in self._other_peer_unit_addresses() + {"leader_api_addr": node_api_address} + for node_api_address in self._other_peer_node_api_addresses() ] raft_config: Dict[str, Any] = { "path": VAULT_RAFT_DATA_PATH, diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py index 1b6f9855..e3a584c7 100644 --- a/tests/unit/test_charm.py +++ b/tests/unit/test_charm.py @@ -57,9 +57,9 @@ def _set_initialization_secret_in_peer_relation( key_values=key_values, ) - def _set_other_unit_api_address_in_peer_relation(self, relation_id: int, unit_name: str): - """Set the other unit api address in the peer relation.""" - key_values = {"unit-address": "http://5.2.1.9:8200"} + def _set_other_node_api_address_in_peer_relation(self, relation_id: int, unit_name: str): + """Set the other node api address in the peer relation.""" + key_values = {"node_api_address": "http://5.2.1.9:8200"} self.harness.update_relation_data( app_or_unit=unit_name, relation_id=relation_id, @@ -157,7 +157,7 @@ def test_given_binding_address_when_install_then_pebble_is_planned( "default_lease_ttl": "168h", "max_lease_ttl": "720h", "disable_mlock": True, - "cluster_addr": f"http://{bind_address}:8201", + "cluster_addr": f"https://{bind_address}:8201", "api_addr": f"http://{bind_address}:8200", } expected_plan = { @@ -179,25 +179,6 @@ def test_given_binding_address_when_install_then_pebble_is_planned( updated_plan = self.harness.get_container_pebble_plan("vault").to_dict() self.assertEqual(updated_plan, expected_plan) - @patch("vault.Vault.unseal", new=Mock) - @patch("vault.Vault.set_token", new=Mock) - @patch("vault.Vault.initialize") - @patch("vault.Vault.is_api_available") - @patch("ops.model.Model.get_binding") - def test_given_binding_address_when_install_then_status_is_active( - self, patch_get_binding, patch_is_api_available, patch_vault_initialize - ): - patch_get_binding.return_value = MockBinding(bind_address="1.2.1.2") - patch_is_api_available.return_value = True - self.harness.set_leader(is_leader=True) - self.harness.set_can_connect(container=self.container_name, val=True) - patch_vault_initialize.return_value = "root token content", "unseal key content" - self._set_peer_relation() - - self.harness.charm.on.install.emit() - - self.assertEqual(self.harness.charm.unit.status, ActiveStatus()) - @patch("vault.Vault.is_api_available") @patch("ops.model.Model.get_binding") def test_given_vault_not_available_when_install_then_status_is_waiting( @@ -252,7 +233,7 @@ def test_given_initialization_secret_not_stored_when_config_changed_then_status_ WaitingStatus("Waiting for vault initialization secret"), ) - def test_given_other_unit_addresses_not_available_when_config_changed_then_status_is_waiting( + def test_given_other_node_api_addresses_not_available_when_config_changed_then_status_is_waiting( # noqa: E501 self, ): self.harness.set_leader(is_leader=False) @@ -308,7 +289,7 @@ def test_given_initialization_secret_is_stored_when_config_changed_then_pebble_p "default_lease_ttl": "168h", "max_lease_ttl": "720h", "disable_mlock": True, - "cluster_addr": f"http://{bind_address}:8201", + "cluster_addr": f"https://{bind_address}:8201", "api_addr": f"http://{bind_address}:8200", } expected_plan = { @@ -413,7 +394,7 @@ def test_given_vault_api_not_available_when_config_changed_then_status_is_waitin root_token="root token content", unseal_keys=["unseal_keys"], ) - self._set_other_unit_api_address_in_peer_relation( + self._set_other_node_api_address_in_peer_relation( relation_id=peer_relation_id, unit_name=other_unit_name, ) @@ -449,7 +430,7 @@ def test_given_vault_is_not_initialized_when_config_changed_then_status_is_waiti root_token="root token content", unseal_keys=["unseal_keys"], ) - self._set_other_unit_api_address_in_peer_relation( + self._set_other_node_api_address_in_peer_relation( relation_id=peer_relation_id, unit_name=other_unit_name, ) From 83924ea1fedbd163c3c7fe279fbee586a89b97ff Mon Sep 17 00:00:00 2001 From: Guillaume Belanger Date: Tue, 12 Sep 2023 11:02:35 -0400 Subject: [PATCH 13/14] Removes try/except around vault.node_in_raft_peers --- src/charm.py | 8 ++------ tests/integration/test_integration.py | 4 ++-- 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/src/charm.py b/src/charm.py index e5f10e35..e3d28c45 100755 --- a/src/charm.py +++ b/src/charm.py @@ -11,7 +11,6 @@ import logging from typing import Any, Dict, List, Optional, Tuple -import requests from charms.observability_libs.v1.kubernetes_service_patch import ( KubernetesServicePatch, ServicePort, @@ -149,11 +148,8 @@ def _on_remove(self, event: RemoveEvent): if root_token: vault = Vault(url=self._api_address) vault.set_token(token=root_token) - try: - if vault.is_api_available() and vault.node_in_raft_peers(node_id=self._node_id): - vault.remove_raft_node(node_id=self._node_id) - except (requests.exceptions.ConnectionError, requests.exceptions.TooManyRedirects): - pass + if vault.is_api_available() and vault.node_in_raft_peers(node_id=self._node_id): + vault.remove_raft_node(node_id=self._node_id) if self._vault_service_is_running(): self._container.stop(self._service_name) self._container.remove_path(path=f"{VAULT_RAFT_DATA_PATH}/*", recursive=True) diff --git a/tests/integration/test_integration.py b/tests/integration/test_integration.py index 1735bd5a..2273d097 100644 --- a/tests/integration/test_integration.py +++ b/tests/integration/test_integration.py @@ -66,7 +66,7 @@ async def test_given_application_is_deployed_when_scale_up_then_status_is_active build_and_deploy, ): num_units = 7 - await ops_test.model.applications[APPLICATION_NAME].scale(num_units) # type: ignore[union-attr] + await ops_test.model.applications[APPLICATION_NAME].scale(num_units) # type: ignore[union-attr] # noqa: E501 await ops_test.model.wait_for_idle( # type: ignore[union-attr] apps=[APPLICATION_NAME], @@ -82,7 +82,7 @@ async def test_given_application_is_deployed_when_scale_down_then_status_is_acti build_and_deploy, ): num_units = 3 - await ops_test.model.applications[APPLICATION_NAME].scale(num_units) # type: ignore[union-attr] + await ops_test.model.applications[APPLICATION_NAME].scale(num_units) # type: ignore[union-attr] # noqa: E501 await ops_test.model.wait_for_idle( # type: ignore[union-attr] apps=[APPLICATION_NAME], From cbb8f5fde4593d42bf761377021de1d64b96dcc9 Mon Sep 17 00:00:00 2001 From: Guillaume Belanger Date: Tue, 12 Sep 2023 13:25:47 -0400 Subject: [PATCH 14/14] Deletes appropriate file on install and remove events --- README.md | 2 ++ src/charm.py | 40 ++++++++++++++++++++++++++++-------- src/vault.py | 7 ++++++- tests/unit/test_charm.py | 44 ++++++++++++++++++++++++++++++---------- tests/unit/test_vault.py | 30 +++++++++++++++++++++++---- 5 files changed, 98 insertions(+), 25 deletions(-) diff --git a/README.md b/README.md index 20ffdedb..31eb5bfc 100644 --- a/README.md +++ b/README.md @@ -15,6 +15,8 @@ Deploy the charm: juju deploy vault-k8s -n 5 --trust ``` +We recommend deploying Vault with an odd number of units. + ### Retrieve Vault's Root token Retrieve the Juju secrets list: diff --git a/src/charm.py b/src/charm.py index e3d28c45..900044ca 100755 --- a/src/charm.py +++ b/src/charm.py @@ -30,13 +30,13 @@ SecretNotFoundError, WaitingStatus, ) -from ops.pebble import Layer +from ops.pebble import ChangeError, Layer, PathError from vault import Vault logger = logging.getLogger(__name__) -VAULT_RAFT_DATA_PATH = "/vault/raft" +VAULT_STORAGE_PATH = "/vault/raft" PEER_RELATION_NAME = "vault-peers" @@ -67,12 +67,13 @@ def _on_install(self, event: InstallEvent): Sets pebble plan, initializes vault, and unseals vault. """ - if not self.unit.is_leader(): - return if not self._container.can_connect(): self.unit.status = WaitingStatus("Waiting to be able to connect to vault unit") event.defer() return + self._delete_vault_data() + if not self.unit.is_leader(): + return if not self._is_peer_relation_created(): self.unit.status = WaitingStatus("Waiting for peer relation to be created") event.defer() @@ -145,14 +146,35 @@ def _on_remove(self, event: RemoveEvent): if not self._container.can_connect(): return root_token, unseal_keys = self._get_initialization_secret_from_peer_relation() - if root_token: + if self._bind_address and root_token: vault = Vault(url=self._api_address) vault.set_token(token=root_token) - if vault.is_api_available() and vault.node_in_raft_peers(node_id=self._node_id): + if ( + vault.is_api_available() + and vault.is_node_in_raft_peers(node_id=self._node_id) + and vault.get_num_raft_peers() > 1 + ): vault.remove_raft_node(node_id=self._node_id) if self._vault_service_is_running(): - self._container.stop(self._service_name) - self._container.remove_path(path=f"{VAULT_RAFT_DATA_PATH}/*", recursive=True) + try: + self._container.stop(self._service_name) + except ChangeError: + logger.warning("Failed to stop Vault service") + pass + self._delete_vault_data() + + def _delete_vault_data(self) -> None: + """Delete Vault's data.""" + try: + self._container.remove_path(path=f"{VAULT_STORAGE_PATH}/vault.db") + logger.info("Removed Vault's main database") + except PathError: + logger.info("No Vault database to remove") + try: + self._container.remove_path(path=f"{VAULT_STORAGE_PATH}/raft/raft.db") + logger.info("Removed Vault's Raft database") + except PathError: + logger.info("No Vault raft database to remove") def _vault_service_is_running(self) -> bool: """Check if the vault service is running.""" @@ -348,7 +370,7 @@ def _get_raft_config(self) -> Dict[str, Any]: for node_api_address in self._other_peer_node_api_addresses() ] raft_config: Dict[str, Any] = { - "path": VAULT_RAFT_DATA_PATH, + "path": VAULT_STORAGE_PATH, "node_id": self._node_id, } if retry_join: diff --git a/src/vault.py b/src/vault.py index 84cba843..1606eaae 100644 --- a/src/vault.py +++ b/src/vault.py @@ -70,10 +70,15 @@ def remove_raft_node(self, node_id: str) -> None: self._client.sys.remove_raft_node(server_id=node_id) logger.info("Removed raft node %s", node_id) - def node_in_raft_peers(self, node_id: str) -> bool: + def is_node_in_raft_peers(self, node_id: str) -> bool: """Check if node is in raft peers.""" raft_config = self._client.sys.read_raft_config() for peer in raft_config["data"]["config"]["servers"]: if peer["node_id"] == node_id: return True return False + + def get_num_raft_peers(self) -> int: + """Returns the number of raft peers.""" + raft_config = self._client.sys.read_raft_config() + return len(raft_config["data"]["config"]["servers"]) diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py index e3a584c7..31d52273 100644 --- a/tests/unit/test_charm.py +++ b/tests/unit/test_charm.py @@ -5,7 +5,7 @@ import json import unittest from typing import List -from unittest.mock import Mock, patch +from unittest.mock import Mock, call, patch from ops import testing from ops.model import ActiveStatus, ModelError, WaitingStatus @@ -66,6 +66,19 @@ def _set_other_node_api_address_in_peer_relation(self, relation_id: int, unit_na key_values=key_values, ) + @patch("ops.model.Container.remove_path") + def test_given_can_connect_to_workload_when_install_then_existing_data_is_removed( + self, patch_remove_path + ): + self.harness.set_leader(is_leader=False) + self.harness.set_can_connect(container=self.container_name, val=True) + + self.harness.charm.on.install.emit() + + patch_remove_path.assert_has_calls( + calls=[call(path="/vault/raft/vault.db"), call(path="/vault/raft/raft/raft.db")] + ) + def test_given_cant_connect_to_workload_when_install_then_status_is_waiting(self): self.harness.set_leader(is_leader=True) self.harness.set_can_connect(container=self.container_name, val=False) @@ -451,21 +464,30 @@ def test_given_can_connect_when_on_remove_then_raft_storage_path_is_deleted( self.harness.charm.on.remove.emit() - patch_remove_path.assert_called_with(path="/vault/raft/*", recursive=True) + patch_remove_path.assert_has_calls( + calls=[call(path="/vault/raft/vault.db"), call(path="/vault/raft/raft/raft.db")] + ) + @patch("ops.model.Model.get_binding") + @patch("vault.Vault.get_num_raft_peers") @patch("vault.Vault.is_api_available") - @patch("vault.Vault.node_in_raft_peers") + @patch("vault.Vault.is_node_in_raft_peers") @patch("vault.Vault.remove_raft_node") @patch("ops.model.Container.remove_path", new=Mock) def test_given_node_in_raft_when_on_remove_then_node_is_removed_from_raft( self, patch_remove_raft_node, - patch_node_in_raft_peers, + patch_is_node_in_raft_peers, patch_is_api_available, + patch_get_num_raft_peers, + patch_get_binding, ): + patch_get_num_raft_peers.return_value = 2 + bind_address = "1.2.3.4" + patch_get_binding.return_value = MockBinding(bind_address=bind_address) self.harness.set_can_connect(container=self.container_name, val=True) patch_is_api_available.return_value = True - patch_node_in_raft_peers.return_value = True + patch_is_node_in_raft_peers.return_value = True peer_relation_id = self._set_peer_relation() self._set_initialization_secret_in_peer_relation( relation_id=peer_relation_id, @@ -478,18 +500,18 @@ def test_given_node_in_raft_when_on_remove_then_node_is_removed_from_raft( patch_remove_raft_node.assert_called_with(node_id=f"{self.model_name}-{self.app_name}/0") @patch("vault.Vault.is_api_available") - @patch("vault.Vault.node_in_raft_peers") + @patch("vault.Vault.is_node_in_raft_peers") @patch("vault.Vault.remove_raft_node") @patch("ops.model.Container.remove_path", new=Mock) def test_given_node_not_in_raft_when_on_remove_then_node_is_not_removed_from_raft( self, patch_remove_raft_node, - patch_node_in_raft_peers, + patch_is_node_in_raft_peers, patch_is_api_available, ): self.harness.set_can_connect(container=self.container_name, val=True) patch_is_api_available.return_value = True - patch_node_in_raft_peers.return_value = False + patch_is_node_in_raft_peers.return_value = False peer_relation_id = self._set_peer_relation() self._set_initialization_secret_in_peer_relation( relation_id=peer_relation_id, @@ -502,7 +524,7 @@ def test_given_node_not_in_raft_when_on_remove_then_node_is_not_removed_from_raf patch_remove_raft_node.assert_not_called() @patch("vault.Vault.is_api_available") - @patch("vault.Vault.node_in_raft_peers") + @patch("vault.Vault.is_node_in_raft_peers") @patch("vault.Vault.remove_raft_node", new=Mock) @patch("ops.model.Container.get_service", new=Mock) @patch("ops.model.Container.stop") @@ -510,12 +532,12 @@ def test_given_node_not_in_raft_when_on_remove_then_node_is_not_removed_from_raf def test_given_service_is_running_when_on_remove_then_service_is_stopped( self, patch_stop_service, - patch_node_in_raft_peers, + patch_is_node_in_raft_peers, patch_is_api_available, ): self.harness.set_can_connect(container=self.container_name, val=True) patch_is_api_available.return_value = True - patch_node_in_raft_peers.return_value = False + patch_is_node_in_raft_peers.return_value = False peer_relation_id = self._set_peer_relation() self._set_initialization_secret_in_peer_relation( relation_id=peer_relation_id, diff --git a/tests/unit/test_vault.py b/tests/unit/test_vault.py index 13fc867d..dee32aee 100644 --- a/tests/unit/test_vault.py +++ b/tests/unit/test_vault.py @@ -58,7 +58,7 @@ def test_given_api_returns_when_is_api_available_then_return_true(self, patch_he self.assertTrue(vault.is_api_available()) @patch("hvac.api.system_backend.raft.Raft.read_raft_config") - def test_given_node_in_peer_list_when_node_in_raft_peers_then_returns_true( + def test_given_node_in_peer_list_when_is_node_in_raft_peers_then_returns_true( self, patch_health_status ): node_id = "whatever node id" @@ -67,10 +67,10 @@ def test_given_node_in_peer_list_when_node_in_raft_peers_then_returns_true( "data": {"config": {"servers": [{"node_id": node_id}]}} } - self.assertTrue(vault.node_in_raft_peers(node_id=node_id)) + self.assertTrue(vault.is_node_in_raft_peers(node_id=node_id)) @patch("hvac.api.system_backend.raft.Raft.read_raft_config") - def test_given_node_not_in_peer_list_when_node_in_raft_peers_then_returns_false( + def test_given_node_not_in_peer_list_when_is_node_in_raft_peers_then_returns_false( self, patch_health_status ): node_id = "whatever node id" @@ -79,4 +79,26 @@ def test_given_node_not_in_peer_list_when_node_in_raft_peers_then_returns_false( "data": {"config": {"servers": [{"node_id": "not our node"}]}} } - self.assertFalse(vault.node_in_raft_peers(node_id=node_id)) + self.assertFalse(vault.is_node_in_raft_peers(node_id=node_id)) + + @patch("hvac.api.system_backend.raft.Raft.read_raft_config") + def test_given_1_node_in_raft_cluster_when_get_num_raft_peers_then_returns_1( + self, patch_health_status + ): + patch_health_status.return_value = { + "data": { + "config": { + "servers": [ + {"node_id": "node 1"}, + {"node_id": "node 2"}, + {"node_id": "node 3"}, + ] + } + } + } + + vault = Vault(url="http://whatever-url") + + vault.get_num_raft_peers() + + self.assertEqual(3, vault.get_num_raft_peers())