Fix compatibility with master cluster mode

changelog/99.fixed.md

@@ -0,0 +1 @@
+Fixed compatibility with master cluster mode

src/saltext/vault/runners/vault.py

@@ -10,6 +10,7 @@
 import logging
 import os
 from collections.abc import Mapping
+from pathlib import Path
 
 import salt.cache
 import salt.crypt
@@ -916,15 +917,18 @@ 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 = Path(__opts__["cluster_pki_dir"])
+    else:
+        pki_dir = Path(__opts__["pki_dir"])
     if impersonated_by_master:
-        public_key = f"{pki_dir}/master.pub"
+        public_key = pki_dir / "master.pub"
     else:
-        public_key = f"{pki_dir}/minions/{minion_id}"
+        public_key = pki_dir / "minions" / minion_id
 
     log.trace("Validating signature for %s", minion_id)
     signature = base64.b64decode(signature)
-    if not salt.crypt.verify_signature(public_key, minion_id, signature):
+    if not salt.crypt.verify_signature(str(public_key), minion_id, signature):
         raise salt.exceptions.AuthenticationError(
             f"Could not validate token request from {minion_id}"
         )

tests/integration/runners/conftest.py

@@ -0,0 +1,13 @@
+import pytest
+
+from tests.support.vault import vault_delete_secret
+from tests.support.vault import vault_write_secret
+
+
+@pytest.fixture(scope="class")
+def vault_testing_values(vault_container_version):  # pylint: disable=unused-argument
+    vault_write_secret("secret/path/foo", success="yeehaaw")
+    try:
+        yield
+    finally:
+        vault_delete_secret("secret/path/foo")

tests/integration/runners/test_vault.py

@@ -20,7 +20,6 @@
 
 log = logging.getLogger(__name__)
 
-
 pytestmark = [
     pytest.mark.slow_test,
     pytest.mark.skip_if_binaries_missing("vault", "getent"),
@@ -526,15 +525,6 @@ def vault_pillar_values_approle(vault_salt_minion):
         vault_delete_secret("salt/roles/foo")
 
 
-@pytest.fixture(scope="class")
-def vault_testing_values(vault_container_version):  # pylint: disable=unused-argument
-    vault_write_secret("secret/path/foo", success="yeehaaw")
-    try:
-        yield
-    finally:
-        vault_delete_secret("secret/path/foo")
-
-
 @pytest.fixture
 def minion_conn_cachedir(vault_salt_call_cli):
     ret = vault_salt_call_cli.run("config.get", "cachedir")

tests/integration/runners/test_vault_master_cluster.py

@@ -0,0 +1,169 @@
+import logging
+import subprocess
+
+import pytest
+import salt.utils.platform
+import salt.version
+
+pytest.importorskip("docker")
+
+log = logging.getLogger(__name__)
+salt_version = int(salt.version.__version__.split(".")[0])
+
+pytestmark = [
+    pytest.mark.slow_test,
+    pytest.mark.skip_if_binaries_missing("vault", "getent"),
+    pytest.mark.skipif(salt_version < 3007, reason="Master cluster requires Salt 3007+"),
+    pytest.mark.usefixtures("vault_container_version", "vault_testing_values"),
+    pytest.mark.parametrize("vault_container_version", ("latest",), indirect=True),
+]
+
+
+@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_factory):
+    return tmp_path_factory.mktemp("cluster")
+
+
+@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, vault_master_config):
+    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=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_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") == "yeehaaw"

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)