From 7a8abc83bea27964d9ee40492140e1821cbec398 Mon Sep 17 00:00:00 2001 From: jeanluc Date: Sun, 10 Nov 2024 15:56:16 +0100 Subject: [PATCH] Fix compatibility with master cluster mode --- changelog/99.fixed.md | 1 + src/saltext/vault/runners/vault.py | 5 +- .../runners/test_vault_master_cluster.py | 170 ++++++++++++++++++ tests/unit/runners/vault/test_vault.py | 25 ++- 4 files changed, 199 insertions(+), 2 deletions(-) create mode 100644 changelog/99.fixed.md create mode 100644 tests/integration/runners/test_vault_master_cluster.py diff --git a/changelog/99.fixed.md b/changelog/99.fixed.md new file mode 100644 index 0000000..a7f78e9 --- /dev/null +++ b/changelog/99.fixed.md @@ -0,0 +1 @@ +Fixed compatibility with master cluster mode diff --git a/src/saltext/vault/runners/vault.py b/src/saltext/vault/runners/vault.py index ffd3a92..d44b41a 100644 --- a/src/saltext/vault/runners/vault.py +++ b/src/saltext/vault/runners/vault.py @@ -916,7 +916,10 @@ def _validate_signature(minion_id, signature, impersonated_by_master): Validate that either minion with id minion_id, or the master, signed the request """ - pki_dir = __opts__["pki_dir"] + if not impersonated_by_master and __opts__.get("cluster_id") is not None: + pki_dir = __opts__["cluster_pki_dir"] + else: + pki_dir = __opts__["pki_dir"] if impersonated_by_master: public_key = f"{pki_dir}/master.pub" else: diff --git a/tests/integration/runners/test_vault_master_cluster.py b/tests/integration/runners/test_vault_master_cluster.py new file mode 100644 index 0000000..48e6798 --- /dev/null +++ b/tests/integration/runners/test_vault_master_cluster.py @@ -0,0 +1,170 @@ +import logging +import subprocess + +import pytest +import salt.utils.platform + +pytest.importorskip("docker") + +pytestmark = [ + pytest.mark.slow_test, + pytest.mark.skip_if_binaries_missing("vault", "getent"), + pytest.mark.usefixtures("vault_container_version"), +] + +log = logging.getLogger(__name__) + + +@pytest.fixture(scope="module") +def vault_master_config(vault_port): + return { + "open_mode": True, + "ext_pillar": [{"vault": "secret/path/foo"}], + "peer_run": { + ".*": [ + "vault.get_config", + "vault.generate_new_token", + ], + }, + "vault": { + "auth": {"token": "testsecret"}, + "cache": { + "backend": "file", + }, + "issue": { + "type": "token", + "token": { + "params": { + "num_uses": 0, + } + }, + }, + "policies": { + "assign": [ + "salt_minion", + "salt_minion_{minion}", + "salt_role_{pillar[roles]}", + ], + "cache_time": 0, + }, + "server": { + "url": f"http://127.0.0.1:{vault_port}", + }, + }, + "minion_data_cache": True, + } + + +@pytest.fixture(scope="module") +def cluster_shared_path(tmp_path): + path = tmp_path / "cluster" + path.mkdir() + return path + + +@pytest.fixture(scope="module") +def cluster_pki_path(cluster_shared_path): + path = cluster_shared_path / "pki" + path.mkdir() + (path / "peers").mkdir() + return path + + +@pytest.fixture(scope="module") +def cluster_cache_path(cluster_shared_path): + path = cluster_shared_path / "cache" + path.mkdir() + return path + + +@pytest.fixture(scope="module") +def cluster_master_1(salt_factories, cluster_pki_path, cluster_cache_path): + config_defaults = { + "open_mode": True, + } + config_overrides = { + "interface": "127.0.0.1", + "cluster_id": "master_cluster", + "cluster_peers": [ + "127.0.0.2", + "127.0.0.3", + ], + "cluster_pki_dir": str(cluster_pki_path), + "cache_dir": str(cluster_cache_path), + } + factory = salt_factories.salt_master_daemon( + "127.0.0.1", + defaults=config_defaults, + overrides=config_overrides, + extra_cli_arguments_after_first_start_failure=["--log-level=info"], + ) + with factory.started(start_timeout=120): + yield factory + + +@pytest.fixture(scope="module") +def cluster_master_2(salt_factories, cluster_master_1, vault_master_config): + if salt.utils.platform.is_darwin() or salt.utils.platform.is_freebsd(): + subprocess.check_output(["ifconfig", "lo0", "alias", "127.0.0.2", "up"]) + + config_overrides = { + "interface": "127.0.0.2", + "cluster_id": "master_cluster", + "cluster_peers": [ + "127.0.0.1", + "127.0.0.3", + ], + "cluster_pki_dir": cluster_master_1.config["cluster_pki_dir"], + "cache_dir": cluster_master_1.config["cache_dir"], + } + + # Use the same ports for both masters, they are binding to different interfaces + for key in ( + "ret_port", + "publish_port", + ): + config_overrides[key] = cluster_master_1.config[key] + factory = salt_factories.salt_master_daemon( + "127.0.0.2", + defaults=vault_master_config, + overrides=config_overrides, + extra_cli_arguments_after_first_start_failure=["--log-level=info"], + ) + with factory.started(start_timeout=120): + yield factory + + +@pytest.fixture(scope="module") +def cluster_minion_1(cluster_master_1, vault_master_config): + port = cluster_master_1.config["ret_port"] + addr = cluster_master_1.config["interface"] + config_overrides = { + "master": f"{addr}:{port}", + } + factory = cluster_master_1.salt_minion_daemon( + "cluster-minion-1", + defaults=vault_master_config, + overrides=config_overrides, + extra_cli_arguments_after_first_start_failure=["--log-level=info"], + ) + with factory.started(start_timeout=120): + yield factory + + +@pytest.fixture +def salt_call_cli(cluster_minion_1): + return cluster_minion_1.salt_call_cli(timeout=120) + + +def test_minion_can_authenticate(salt_call_cli): + ret = salt_call_cli.run("vault.read_secret", "secret/path/foo") + assert ret.returncode == 0 + assert ret.data + assert ret.data.get("success") == "yeehaaw" + + +def test_minion_pillar_is_populated_as_expected(salt_call_cli): + ret = salt_call_cli.run("pillar.items") + assert ret.returncode == 0 + assert ret.data + assert ret.data.get("success") == "yeehaww" diff --git a/tests/unit/runners/vault/test_vault.py b/tests/unit/runners/vault/test_vault.py index 41aaf5f..eecd7dc 100644 --- a/tests/unit/runners/vault/test_vault.py +++ b/tests/unit/runners/vault/test_vault.py @@ -1,3 +1,4 @@ +from pathlib import Path from unittest.mock import ANY from unittest.mock import MagicMock from unittest.mock import Mock @@ -13,10 +14,11 @@ @pytest.fixture -def configure_loader_modules(): +def configure_loader_modules(master_opts): return { vault: { "__grains__": {"id": "test-master"}, + "__opts__": master_opts, } } @@ -1484,3 +1486,24 @@ def test_revoke_token_by_accessor(client): client.post.assert_called_once_with( "auth/token/revoke-accessor", payload={"accessor": "test-accessor"} ) + + +@pytest.mark.parametrize("cluster_id", (None, "test_cluster")) +@pytest.mark.parametrize("impersonated", (False, True)) +def test_validate_signature_pki_dir(cluster_id, impersonated, master_opts): + """ + Ensure we can validate minion signatures when running in + master cluster mode. + """ + master_opts["cluster_id"] = cluster_id + base = Path(master_opts["pki_dir"]) + if not impersonated and cluster_id: + base = base.parent / "cluster" / "pki" + master_opts["cluster_pki_dir"] = str(base) + if impersonated: + expected = base / "master.pub" + else: + expected = base / "minions" / "test-minion" + with patch("salt.crypt.verify_signature", autospec=True) as verify: + vault._validate_signature("test-minion", "", impersonated) + assert verify.call_args.args[0] == str(expected)