From 67703832e681c1fb2a8934a7f341f844d5d11c84 Mon Sep 17 00:00:00 2001 From: "Daniel A. Wozniak" Date: Mon, 18 Sep 2023 15:26:52 -0700 Subject: [PATCH 01/15] Cluster aes session rotation test --- tests/pytests/integration/cluster/__init__.py | 0 tests/pytests/integration/cluster/conftest.py | 150 +++++++++++++++++ .../cluster/test_basic_cluster.py | 0 tests/pytests/scenarios/cluster/conftest.py | 152 ++---------------- .../pytests/scenarios/cluster/test_cluster.py | 57 +++++++ 5 files changed, 218 insertions(+), 141 deletions(-) create mode 100644 tests/pytests/integration/cluster/__init__.py create mode 100644 tests/pytests/integration/cluster/conftest.py rename tests/pytests/{scenarios => integration}/cluster/test_basic_cluster.py (100%) create mode 100644 tests/pytests/scenarios/cluster/test_cluster.py diff --git a/tests/pytests/integration/cluster/__init__.py b/tests/pytests/integration/cluster/__init__.py new file mode 100644 index 00000000000..e69de29bb2d diff --git a/tests/pytests/integration/cluster/conftest.py b/tests/pytests/integration/cluster/conftest.py new file mode 100644 index 00000000000..34431a65416 --- /dev/null +++ b/tests/pytests/integration/cluster/conftest.py @@ -0,0 +1,150 @@ +import logging +import subprocess + +import pytest + +import salt.utils.platform + +log = logging.getLogger(__name__) + + +@pytest.fixture +def cluster_shared_path(tmp_path): + path = tmp_path / "cluster" + path.mkdir() + return path + + +@pytest.fixture +def cluster_pki_path(cluster_shared_path): + path = cluster_shared_path / "pki" + path.mkdir() + (path / "peers").mkdir() + return path + + +@pytest.fixture +def cluster_cache_path(cluster_shared_path): + path = cluster_shared_path / "cache" + path.mkdir() + return path + + +@pytest.fixture +def cluster_master_1(request, salt_factories, cluster_pki_path, cluster_cache_path): + config_defaults = { + "open_mode": True, + "transport": request.config.getoption("--transport"), + } + 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 +def cluster_master_2(salt_factories, cluster_master_1): + if salt.utils.platform.is_darwin() or salt.utils.platform.is_freebsd(): + subprocess.check_output(["ifconfig", "lo0", "alias", "127.0.0.2", "up"]) + + config_defaults = { + "open_mode": True, + "transport": cluster_master_1.config["transport"], + } + 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=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 +def cluster_master_3(salt_factories, cluster_master_1): + if salt.utils.platform.is_darwin() or salt.utils.platform.is_freebsd(): + subprocess.check_output(["ifconfig", "lo0", "alias", "127.0.0.3", "up"]) + + config_defaults = { + "open_mode": True, + "transport": cluster_master_1.config["transport"], + } + config_overrides = { + "interface": "127.0.0.3", + "cluster_id": "master_cluster", + "cluster_peers": [ + "127.0.0.1", + "127.0.0.2", + ], + "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.3", + 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 +def cluster_minion_1(cluster_master_1): + config_defaults = { + "transport": cluster_master_1.config["transport"], + } + + port = cluster_master_1.config["ret_port"] + addr = cluster_master_1.config["interface"] + config_overrides = { + "master": f"{addr}:{port}", + "test.foo": "baz", + } + factory = cluster_master_1.salt_minion_daemon( + "cluster-minion-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 diff --git a/tests/pytests/scenarios/cluster/test_basic_cluster.py b/tests/pytests/integration/cluster/test_basic_cluster.py similarity index 100% rename from tests/pytests/scenarios/cluster/test_basic_cluster.py rename to tests/pytests/integration/cluster/test_basic_cluster.py diff --git a/tests/pytests/scenarios/cluster/conftest.py b/tests/pytests/scenarios/cluster/conftest.py index 34431a65416..eb19b7c94cf 100644 --- a/tests/pytests/scenarios/cluster/conftest.py +++ b/tests/pytests/scenarios/cluster/conftest.py @@ -5,146 +5,16 @@ import pytest import salt.utils.platform + +from tests.pytests.integration.cluster.conftest import ( + cluster_shared_path, + cluster_pki_path, + cluster_cache_path, + cluster_master_1, + cluster_master_2, + cluster_master_3, + cluster_minion_1, +) + log = logging.getLogger(__name__) - -@pytest.fixture -def cluster_shared_path(tmp_path): - path = tmp_path / "cluster" - path.mkdir() - return path - - -@pytest.fixture -def cluster_pki_path(cluster_shared_path): - path = cluster_shared_path / "pki" - path.mkdir() - (path / "peers").mkdir() - return path - - -@pytest.fixture -def cluster_cache_path(cluster_shared_path): - path = cluster_shared_path / "cache" - path.mkdir() - return path - - -@pytest.fixture -def cluster_master_1(request, salt_factories, cluster_pki_path, cluster_cache_path): - config_defaults = { - "open_mode": True, - "transport": request.config.getoption("--transport"), - } - 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 -def cluster_master_2(salt_factories, cluster_master_1): - if salt.utils.platform.is_darwin() or salt.utils.platform.is_freebsd(): - subprocess.check_output(["ifconfig", "lo0", "alias", "127.0.0.2", "up"]) - - config_defaults = { - "open_mode": True, - "transport": cluster_master_1.config["transport"], - } - 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=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 -def cluster_master_3(salt_factories, cluster_master_1): - if salt.utils.platform.is_darwin() or salt.utils.platform.is_freebsd(): - subprocess.check_output(["ifconfig", "lo0", "alias", "127.0.0.3", "up"]) - - config_defaults = { - "open_mode": True, - "transport": cluster_master_1.config["transport"], - } - config_overrides = { - "interface": "127.0.0.3", - "cluster_id": "master_cluster", - "cluster_peers": [ - "127.0.0.1", - "127.0.0.2", - ], - "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.3", - 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 -def cluster_minion_1(cluster_master_1): - config_defaults = { - "transport": cluster_master_1.config["transport"], - } - - port = cluster_master_1.config["ret_port"] - addr = cluster_master_1.config["interface"] - config_overrides = { - "master": f"{addr}:{port}", - "test.foo": "baz", - } - factory = cluster_master_1.salt_minion_daemon( - "cluster-minion-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 diff --git a/tests/pytests/scenarios/cluster/test_cluster.py b/tests/pytests/scenarios/cluster/test_cluster.py new file mode 100644 index 00000000000..e19377578d9 --- /dev/null +++ b/tests/pytests/scenarios/cluster/test_cluster.py @@ -0,0 +1,57 @@ +import pathlib +import time +import os + +import salt.crypt + +def test_cluster_key_rotation( + cluster_master_1, cluster_master_2, cluster_master_3, cluster_minion_1, + cluster_cache_path, +): + cli = cluster_master_2.salt_cli(timeout=120) + ret = cli.run("test.ping", minion_tgt="cluster-minion-1") + assert ret.data is True + + # Validate the aes session key for all masters match + keys = set() + for master in (cluster_master_1, cluster_master_2, cluster_master_3,): + config = cluster_minion_1.config.copy() + config["master_uri"] = f"tcp://{master.config['interface']}:{master.config['ret_port']}" + auth = salt.crypt.SAuth(config) + auth.authenticate() + assert "aes" in auth._creds + keys.add(auth._creds["aes"]) + + assert len(keys) == 1 + orig_aes = keys.pop() + + dfpath = pathlib.Path(cluster_master_1.config["cachedir"]) / ".dfn" + assert not dfpath.exists() + salt.crypt.dropfile( + cluster_master_1.config["cachedir"], + user=os.getlogin(), + master_id=cluster_master_1.config["id"], + ) + assert dfpath.exists() + timeout = 2 * cluster_master_1.config["loop_interval"] + start = time.monotonic() + while True: + if not dfpath.exists(): + break + if time.monotonic() - start > timeout: + assert False, f"Drop file never removed {dfpath}" + + keys = set() + + # Validate the aes session key for all masters match + for master in (cluster_master_1, cluster_master_2, cluster_master_3,): + config = cluster_minion_1.config.copy() + config["master_uri"] = f"tcp://{master.config['interface']}:{master.config['ret_port']}" + auth = salt.crypt.SAuth(config) + auth.authenticate() + assert "aes" in auth._creds + keys.add(auth._creds["aes"]) + + assert len(keys) == 1 + # Validate the aes session key actually changed + assert orig_aes != keys.pop() From c3e99eda876f94d6b250f3c38d07a9b394e3abdd Mon Sep 17 00:00:00 2001 From: "Daniel A. Wozniak" Date: Mon, 18 Sep 2023 17:11:27 -0700 Subject: [PATCH 02/15] Fix key rotation --- salt/channel/server.py | 2 - salt/crypt.py | 27 ++++--- salt/master.py | 77 +++++++++++++++---- .../pytests/scenarios/cluster/test_cluster.py | 1 + 4 files changed, 77 insertions(+), 30 deletions(-) diff --git a/salt/channel/server.py b/salt/channel/server.py index afdc0282a88..5042aca8506 100644 --- a/salt/channel/server.py +++ b/salt/channel/server.py @@ -1045,9 +1045,7 @@ class MasterPubServerChannel: """ try: tag, data = salt.utils.event.SaltEvent.unpack(payload) - log.error("recieved event from peer %s %r", tag, data) if tag.startswith("cluster/peer"): - log.error("Got peer join %r", data) peer = data["peer_id"] aes = data["peers"][self.opts["id"]]["aes"] sig = data["peers"][self.opts["id"]]["sig"] diff --git a/salt/crypt.py b/salt/crypt.py index de342b84beb..522e8ca6fe4 100644 --- a/salt/crypt.py +++ b/salt/crypt.py @@ -16,6 +16,7 @@ import pathlib import random import stat import sys +import tempfile import time import traceback import uuid @@ -1620,19 +1621,21 @@ class Crypticle: return b64key.replace("\n", "") @classmethod - def read_or_generate_key(cls, path, key_size=192, remove=False): - if remove: - os.remove(path) + def write_key(cls, path, key_size=192): + directory = pathlib.Path(path).parent + fd, tmp = tempfile.mkstemp(dir=directory, prefix="aes") with salt.utils.files.set_umask(0o177): - try: - with salt.utils.files.fopen(path, "r") as fp: - return fp.read() - except FileNotFoundError: - pass - key = cls.generate_key_string(key_size) - with salt.utils.files.fopen(path, "w") as fp: - fp.write(key) - return key + with salt.utils.files.fopen(tmp, "w") as fp: + fp.write(cls.generate_key_string(key_size)) + os.rename(tmp, path) + + @classmethod + def read_key(cls, path): + try: + with salt.utils.files.fopen(path, "r") as fp: + return fp.read() + except FileNotFoundError: + pass @classmethod def extract_keys(cls, key_string, key_size): diff --git a/salt/master.py b/salt/master.py index ad88f57c495..8990917e949 100644 --- a/salt/master.py +++ b/salt/master.py @@ -142,7 +142,6 @@ class SMaster: def rotate_secrets( cls, opts=None, event=None, use_lock=True, owner=False, publisher=None ): - log.info("Rotating master AES key") if opts is None: opts = {} @@ -173,6 +172,39 @@ class SMaster: log.debug("Pinging all connected minions due to key rotation") salt.utils.master.ping_all_connected_minions(opts) + @classmethod + def rotate_cluster_secret( + cls, opts=None, event=None, use_lock=True, owner=False, publisher=None + ): + log.debug("Rotating cluster AES key") + if opts is None: + opts = {} + + if use_lock: + with cls.secrets["cluster_aes"]["secret"].get_lock(): + cls.secrets["cluster_aes"][ + "secret" + ].value = salt.utils.stringutils.to_bytes( + cls.secrets["cluster_aes"]["reload"](remove=owner) + ) + else: + cls.secrets["cluster_aes"][ + "secret" + ].value = salt.utils.stringutils.to_bytes( + cls.secrets["cluster_aes"]["reload"](remove=owner) + ) + + if event: + event.fire_event({f"rotate_cluster_aes_key": True}, tag="rotate_cluster_aes_key") + + if publisher: + publisher.send_aes_key_event() + + if opts.get("ping_on_rotate"): + # Ping all minions to get them to pick up the new key + log.debug("Pinging all connected minions due to key rotation") + salt.utils.master.ping_all_connected_minions(opts) + class Maintenance(salt.utils.process.SignalHandlingProcess): """ @@ -358,7 +390,7 @@ class Maintenance(salt.utils.process.SignalHandlingProcess): if to_rotate: if self.opts.get("cluster_id", None): - SMaster.rotate_secrets( + SMaster.rotate_cluster_secret( self.opts, self.event, owner=True, publisher=self.ipc_publisher ) else: @@ -714,6 +746,20 @@ class Master(SMaster): log.critical("Master failed pre flight checks, exiting\n") sys.exit(salt.defaults.exitcodes.EX_GENERIC) + def read_or_generate_key(self, remove=False, fs_wait=0.1): + """ + Used to manage a cluster aes session key file. + """ + path = os.path.join(self.opts["cluster_pki_dir"], ".aes") + if remove: + os.remove(path) + key = salt.crypt.Crypticle.read_key(path) + if key: + return key + salt.crypt.Crypticle.write_key(path) + time.sleep(fs_wait) + return salt.crypt.Crypticle.read_key(path) + def start(self): """ Turn on the master server components @@ -731,22 +777,17 @@ class Master(SMaster): # signal handlers with salt.utils.process.default_signals(signal.SIGINT, signal.SIGTERM): if self.opts["cluster_id"]: - keypath = os.path.join(self.opts["cluster_pki_dir"], ".aes") - cluster_keygen = functools.partial( - salt.crypt.Crypticle.read_or_generate_key, - keypath, - ) # Setup the secrets here because the PubServerChannel may need # them as well. SMaster.secrets["cluster_aes"] = { "secret": multiprocessing.Array( - ctypes.c_char, salt.utils.stringutils.to_bytes(cluster_keygen()) + ctypes.c_char, salt.utils.stringutils.to_bytes(self.read_or_generate_key()) ), "serial": multiprocessing.Value( ctypes.c_longlong, lock=False, # We'll use the lock from 'secret' ), - "reload": cluster_keygen, + "reload": self.read_or_generate_key, } SMaster.secrets["aes"] = { @@ -779,7 +820,7 @@ class Master(SMaster): ipc_publisher.pre_fork(self.process_manager) self.process_manager.add_process( EventMonitor, - args=[self.opts], + args=[self.opts, ipc_publisher], name="EventMonitor", ) @@ -814,7 +855,7 @@ class Master(SMaster): if self.opts.get("event_return"): log.info("Creating master event return process") self.process_manager.add_process( - salt.utils.event.EventReturn, args=(self.opts,), name="EventReturn" + salt.utils.event.EventReturn, args=(self.opts), name="EventReturn" ) ext_procs = self.opts.get("ext_processes", []) @@ -908,19 +949,19 @@ class EventMonitor(salt.utils.process.SignalHandlingProcess): - Handle key rotate events. """ - def __init__(self, opts, channels=None, name="EventMonitor"): + def __init__(self, opts, ipc_publisher, channels=None, name="EventMonitor"): super().__init__(name=name) self.opts = opts if channels is None: channels = [] self.channels = channels + self.ipc_publisher = ipc_publisher async def handle_event(self, package): """ Event handler for publish forwarder """ tag, data = salt.utils.event.SaltEvent.unpack(package) - log.debug("Event monitor got event %s %r", tag, data) if tag.startswith("salt/job") and tag.endswith("/publish"): peer_id = data.pop("__peer_id", None) if peer_id: @@ -937,9 +978,13 @@ class EventMonitor(salt.utils.process.SignalHandlingProcess): for chan in self.channels: tasks.append(asyncio.create_task(chan.publish(data))) await asyncio.gather(*tasks) - elif tag == "rotate_aes_key": - log.debug("Event monitor recieved rotate aes key event, rotating key.") - SMaster.rotate_secrets(self.opts, owner=False) + elif tag == "rotate_cluster_aes_key": + peer_id = data.pop("__peer_id", None) + if peer_id: + log.debug("Rotating AES session key") + SMaster.rotate_cluster_secret(self.opts, owner=False, publisher=self.ipc_publisher) + else: + log.trace("Ignore tag %s", tag) def run(self): io_loop = tornado.ioloop.IOLoop() diff --git a/tests/pytests/scenarios/cluster/test_cluster.py b/tests/pytests/scenarios/cluster/test_cluster.py index e19377578d9..d3d39fa3227 100644 --- a/tests/pytests/scenarios/cluster/test_cluster.py +++ b/tests/pytests/scenarios/cluster/test_cluster.py @@ -25,6 +25,7 @@ def test_cluster_key_rotation( assert len(keys) == 1 orig_aes = keys.pop() + # Create a drop file and wait for the master to do a key rotation. dfpath = pathlib.Path(cluster_master_1.config["cachedir"]) / ".dfn" assert not dfpath.exists() salt.crypt.dropfile( From 77d79ddfe8b56cf72b65adaa19a6efc2767df2c6 Mon Sep 17 00:00:00 2001 From: "Daniel A. Wozniak" Date: Mon, 18 Sep 2023 17:50:06 -0700 Subject: [PATCH 03/15] fix pre commit in tests --- tests/pytests/scenarios/cluster/conftest.py | 7 ++--- .../pytests/scenarios/cluster/test_cluster.py | 28 +++++++++++++++---- 2 files changed, 24 insertions(+), 11 deletions(-) diff --git a/tests/pytests/scenarios/cluster/conftest.py b/tests/pytests/scenarios/cluster/conftest.py index eb19b7c94cf..be0c3800094 100644 --- a/tests/pytests/scenarios/cluster/conftest.py +++ b/tests/pytests/scenarios/cluster/conftest.py @@ -4,17 +4,14 @@ import subprocess import pytest import salt.utils.platform - - from tests.pytests.integration.cluster.conftest import ( - cluster_shared_path, - cluster_pki_path, cluster_cache_path, cluster_master_1, cluster_master_2, cluster_master_3, cluster_minion_1, + cluster_pki_path, + cluster_shared_path, ) log = logging.getLogger(__name__) - diff --git a/tests/pytests/scenarios/cluster/test_cluster.py b/tests/pytests/scenarios/cluster/test_cluster.py index d3d39fa3227..10d9b0ad218 100644 --- a/tests/pytests/scenarios/cluster/test_cluster.py +++ b/tests/pytests/scenarios/cluster/test_cluster.py @@ -1,11 +1,15 @@ +import os import pathlib import time -import os import salt.crypt + def test_cluster_key_rotation( - cluster_master_1, cluster_master_2, cluster_master_3, cluster_minion_1, + cluster_master_1, + cluster_master_2, + cluster_master_3, + cluster_minion_1, cluster_cache_path, ): cli = cluster_master_2.salt_cli(timeout=120) @@ -14,9 +18,15 @@ def test_cluster_key_rotation( # Validate the aes session key for all masters match keys = set() - for master in (cluster_master_1, cluster_master_2, cluster_master_3,): + for master in ( + cluster_master_1, + cluster_master_2, + cluster_master_3, + ): config = cluster_minion_1.config.copy() - config["master_uri"] = f"tcp://{master.config['interface']}:{master.config['ret_port']}" + config[ + "master_uri" + ] = f"tcp://{master.config['interface']}:{master.config['ret_port']}" auth = salt.crypt.SAuth(config) auth.authenticate() assert "aes" in auth._creds @@ -45,9 +55,15 @@ def test_cluster_key_rotation( keys = set() # Validate the aes session key for all masters match - for master in (cluster_master_1, cluster_master_2, cluster_master_3,): + for master in ( + cluster_master_1, + cluster_master_2, + cluster_master_3, + ): config = cluster_minion_1.config.copy() - config["master_uri"] = f"tcp://{master.config['interface']}:{master.config['ret_port']}" + config[ + "master_uri" + ] = f"tcp://{master.config['interface']}:{master.config['ret_port']}" auth = salt.crypt.SAuth(config) auth.authenticate() assert "aes" in auth._creds From 8c77c94bf6a0a654863e8d5c61f7fa2452fdfc3e Mon Sep 17 00:00:00 2001 From: "Daniel A. Wozniak" Date: Mon, 18 Sep 2023 17:50:43 -0700 Subject: [PATCH 04/15] Fix pre-commit --- salt/master.py | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/salt/master.py b/salt/master.py index 8990917e949..711e6c57980 100644 --- a/salt/master.py +++ b/salt/master.py @@ -6,7 +6,6 @@ import asyncio import collections import copy import ctypes -import functools import logging import multiprocessing import os @@ -195,7 +194,9 @@ class SMaster: ) if event: - event.fire_event({f"rotate_cluster_aes_key": True}, tag="rotate_cluster_aes_key") + event.fire_event( + {f"rotate_cluster_aes_key": True}, tag="rotate_cluster_aes_key" + ) if publisher: publisher.send_aes_key_event() @@ -781,7 +782,8 @@ class Master(SMaster): # them as well. SMaster.secrets["cluster_aes"] = { "secret": multiprocessing.Array( - ctypes.c_char, salt.utils.stringutils.to_bytes(self.read_or_generate_key()) + ctypes.c_char, + salt.utils.stringutils.to_bytes(self.read_or_generate_key()), ), "serial": multiprocessing.Value( ctypes.c_longlong, @@ -982,7 +984,9 @@ class EventMonitor(salt.utils.process.SignalHandlingProcess): peer_id = data.pop("__peer_id", None) if peer_id: log.debug("Rotating AES session key") - SMaster.rotate_cluster_secret(self.opts, owner=False, publisher=self.ipc_publisher) + SMaster.rotate_cluster_secret( + self.opts, owner=False, publisher=self.ipc_publisher + ) else: log.trace("Ignore tag %s", tag) From a50f5b8ff63822a674e5c43dd34e597f626bc246 Mon Sep 17 00:00:00 2001 From: "Daniel A. Wozniak" Date: Wed, 20 Sep 2023 14:07:56 -0700 Subject: [PATCH 05/15] Fix tests --- tests/pytests/unit/test_crypt.py | 12 ++++++------ tests/pytests/unit/test_master.py | 2 +- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/tests/pytests/unit/test_crypt.py b/tests/pytests/unit/test_crypt.py index 6b5b054fb5a..72f68161885 100644 --- a/tests/pytests/unit/test_crypt.py +++ b/tests/pytests/unit/test_crypt.py @@ -284,12 +284,12 @@ def test_verify_signature_bad_sig(tmp_path): def test_read_or_generate_key_string(tmp_path): keyfile = tmp_path / ".aes" assert not keyfile.exists() - first_key = salt.crypt.Crypticle.read_or_generate_key(keyfile) - assert keyfile.exists() - second_key = salt.crypt.Crypticle.read_or_generate_key(keyfile) - assert first_key == second_key - third_key = salt.crypt.Crypticle.read_or_generate_key(keyfile, remove=True) - assert second_key != third_key + first_key = salt.crypt.Crypticle.read_key(keyfile) + assert first_key is None + assert not keyfile.exists() + salt.crypt.Crypticle.write_key(keyfile) + second_key = salt.crypt.Crypticle.read_key(keyfile) + assert second_key is not None def test_dropfile_contents(tmp_path, master_opts): diff --git a/tests/pytests/unit/test_master.py b/tests/pytests/unit/test_master.py index 198c63d8602..7a309e7d14f 100644 --- a/tests/pytests/unit/test_master.py +++ b/tests/pytests/unit/test_master.py @@ -990,7 +990,7 @@ def test_key_rotate_no_master_match(maintenance): def test_key_dfn_wait(cluster_maintenance): now = time.monotonic() key = pathlib.Path(cluster_maintenance.opts["cluster_pki_dir"]) / ".aes" - salt.crypt.Crypticle.read_or_generate_key(str(key)) + salt.crypt.Crypticle.write_key(str(key)) rotate_time = time.monotonic() - (cluster_maintenance.opts["publish_session"] + 1) os.utime(str(key), (rotate_time, rotate_time)) From 34bcdf4e329beb7862fbde73ad12fe8b079e15be Mon Sep 17 00:00:00 2001 From: "Daniel A. Wozniak" Date: Wed, 20 Sep 2023 14:28:45 -0700 Subject: [PATCH 06/15] Fix master startup bug --- salt/master.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/salt/master.py b/salt/master.py index 711e6c57980..04da9d54032 100644 --- a/salt/master.py +++ b/salt/master.py @@ -857,7 +857,7 @@ class Master(SMaster): if self.opts.get("event_return"): log.info("Creating master event return process") self.process_manager.add_process( - salt.utils.event.EventReturn, args=(self.opts), name="EventReturn" + salt.utils.event.EventReturn, args=(self.opts,), name="EventReturn" ) ext_procs = self.opts.get("ext_processes", []) From 172a1f9f35420aee878765ef04cc43a68577a999 Mon Sep 17 00:00:00 2001 From: "Daniel A. Wozniak" Date: Thu, 21 Sep 2023 14:09:50 -0700 Subject: [PATCH 07/15] Fix linter --- tests/pytests/scenarios/cluster/conftest.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/pytests/scenarios/cluster/conftest.py b/tests/pytests/scenarios/cluster/conftest.py index be0c3800094..d49f3341251 100644 --- a/tests/pytests/scenarios/cluster/conftest.py +++ b/tests/pytests/scenarios/cluster/conftest.py @@ -1,9 +1,6 @@ import logging -import subprocess -import pytest - -import salt.utils.platform +# pylint: disable=unused-import from tests.pytests.integration.cluster.conftest import ( cluster_cache_path, cluster_master_1, @@ -14,4 +11,7 @@ from tests.pytests.integration.cluster.conftest import ( cluster_shared_path, ) +# pylint: enable=unused-import + + log = logging.getLogger(__name__) From 4447b362d294ef187c4fbb8edcea2e2f05e7da4f Mon Sep 17 00:00:00 2001 From: "Daniel A. Wozniak" Date: Wed, 8 Nov 2023 14:53:58 -0700 Subject: [PATCH 08/15] Fix key tests on windows --- salt/crypt.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/salt/crypt.py b/salt/crypt.py index 522e8ca6fe4..ce3a5a83c67 100644 --- a/salt/crypt.py +++ b/salt/crypt.py @@ -1623,8 +1623,9 @@ class Crypticle: @classmethod def write_key(cls, path, key_size=192): directory = pathlib.Path(path).parent - fd, tmp = tempfile.mkstemp(dir=directory, prefix="aes") with salt.utils.files.set_umask(0o177): + fd, tmp = tempfile.mkstemp(dir=directory, prefix="aes") + os.close(fd) with salt.utils.files.fopen(tmp, "w") as fp: fp.write(cls.generate_key_string(key_size)) os.rename(tmp, path) From f66d495334164213d9d19f871573e0201b58d2bc Mon Sep 17 00:00:00 2001 From: "Daniel A. Wozniak" Date: Tue, 21 Nov 2023 14:55:18 -0700 Subject: [PATCH 09/15] Bump cache seed --- .github/workflows/ci.yml | 2 +- .github/workflows/nightly.yml | 2 +- .github/workflows/release.yml | 2 +- .github/workflows/scheduled.yml | 2 +- .github/workflows/staging.yml | 2 +- .github/workflows/templates/layout.yml.jinja | 2 +- 6 files changed, 6 insertions(+), 6 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index a0b9499f07f..090dbbe488e 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -16,7 +16,7 @@ on: env: COLUMNS: 190 - CACHE_SEED: SEED-1 # Bump the number to invalidate all caches + CACHE_SEED: SEED-4 # Bump the number to invalidate all caches RELENV_DATA: "${{ github.workspace }}/.relenv" permissions: diff --git a/.github/workflows/nightly.yml b/.github/workflows/nightly.yml index ec836062e62..f9ccf0a2dca 100644 --- a/.github/workflows/nightly.yml +++ b/.github/workflows/nightly.yml @@ -22,7 +22,7 @@ on: env: COLUMNS: 190 - CACHE_SEED: SEED-1 # Bump the number to invalidate all caches + CACHE_SEED: SEED-4 # Bump the number to invalidate all caches RELENV_DATA: "${{ github.workspace }}/.relenv" permissions: diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 5512ab1cb48..a641b760db0 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -21,7 +21,7 @@ on: env: COLUMNS: 190 - CACHE_SEED: SEED-1 # Bump the number to invalidate all caches + CACHE_SEED: SEED-4 # Bump the number to invalidate all caches RELENV_DATA: "${{ github.workspace }}/.relenv" permissions: diff --git a/.github/workflows/scheduled.yml b/.github/workflows/scheduled.yml index 1d68e7b5d72..71b8e334236 100644 --- a/.github/workflows/scheduled.yml +++ b/.github/workflows/scheduled.yml @@ -12,7 +12,7 @@ on: env: COLUMNS: 190 - CACHE_SEED: SEED-1 # Bump the number to invalidate all caches + CACHE_SEED: SEED-4 # Bump the number to invalidate all caches RELENV_DATA: "${{ github.workspace }}/.relenv" permissions: diff --git a/.github/workflows/staging.yml b/.github/workflows/staging.yml index 947b4347f0f..96f98137d33 100644 --- a/.github/workflows/staging.yml +++ b/.github/workflows/staging.yml @@ -37,7 +37,7 @@ on: env: COLUMNS: 190 - CACHE_SEED: SEED-1 # Bump the number to invalidate all caches + CACHE_SEED: SEED-4 # Bump the number to invalidate all caches RELENV_DATA: "${{ github.workspace }}/.relenv" permissions: diff --git a/.github/workflows/templates/layout.yml.jinja b/.github/workflows/templates/layout.yml.jinja index 779ccdb1274..eeb53920faf 100644 --- a/.github/workflows/templates/layout.yml.jinja +++ b/.github/workflows/templates/layout.yml.jinja @@ -34,7 +34,7 @@ on: env: COLUMNS: 190 - CACHE_SEED: SEED-1 # Bump the number to invalidate all caches + CACHE_SEED: SEED-4 # Bump the number to invalidate all caches RELENV_DATA: "${{ github.workspace }}/.relenv" <%- endblock env %> From cb9dd368eeb9af4d8dfb1316b00a1091d5090e56 Mon Sep 17 00:00:00 2001 From: "Daniel A. Wozniak" Date: Tue, 21 Nov 2023 15:16:56 -0700 Subject: [PATCH 10/15] Add license to setup.cfg --- setup.cfg | 3 +++ 1 file changed, 3 insertions(+) diff --git a/setup.cfg b/setup.cfg index 2f452d87695..a635fdb242f 100644 --- a/setup.cfg +++ b/setup.cfg @@ -1,3 +1,6 @@ +[metadata] +license_files = LICENSE + [sdist] owner = root group = root From 369b35c6709bb33f7954f9015c35bc96218c0d25 Mon Sep 17 00:00:00 2001 From: "Daniel A. Wozniak" Date: Tue, 21 Nov 2023 15:26:25 -0700 Subject: [PATCH 11/15] Add license to pyproject.toml instead of setup.cfg --- pyproject.toml | 3 +++ setup.cfg | 3 --- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 8afba8f0f4c..eb300235cb3 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,3 +1,6 @@ +[project] +license = {text = "Apache-2.0"} + [tool.black] exclude= """ /( diff --git a/setup.cfg b/setup.cfg index a635fdb242f..2f452d87695 100644 --- a/setup.cfg +++ b/setup.cfg @@ -1,6 +1,3 @@ -[metadata] -license_files = LICENSE - [sdist] owner = root group = root From 070798fdc5b6218dc7b3074bc6118d74c34b5b28 Mon Sep 17 00:00:00 2001 From: Pedro Algarvio Date: Tue, 21 Nov 2023 20:19:32 +0000 Subject: [PATCH 12/15] Make sure `PIP_CONSTRAINT` is also set when building RPM's from source Signed-off-by: Pedro Algarvio --- .github/workflows/ci.yml | 2 +- .github/workflows/nightly.yml | 2 +- .github/workflows/release.yml | 2 +- .github/workflows/scheduled.yml | 2 +- .github/workflows/staging.yml | 2 +- .github/workflows/templates/layout.yml.jinja | 2 +- 6 files changed, 6 insertions(+), 6 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 090dbbe488e..a0b9499f07f 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -16,7 +16,7 @@ on: env: COLUMNS: 190 - CACHE_SEED: SEED-4 # Bump the number to invalidate all caches + CACHE_SEED: SEED-1 # Bump the number to invalidate all caches RELENV_DATA: "${{ github.workspace }}/.relenv" permissions: diff --git a/.github/workflows/nightly.yml b/.github/workflows/nightly.yml index f9ccf0a2dca..ec836062e62 100644 --- a/.github/workflows/nightly.yml +++ b/.github/workflows/nightly.yml @@ -22,7 +22,7 @@ on: env: COLUMNS: 190 - CACHE_SEED: SEED-4 # Bump the number to invalidate all caches + CACHE_SEED: SEED-1 # Bump the number to invalidate all caches RELENV_DATA: "${{ github.workspace }}/.relenv" permissions: diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index a641b760db0..5512ab1cb48 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -21,7 +21,7 @@ on: env: COLUMNS: 190 - CACHE_SEED: SEED-4 # Bump the number to invalidate all caches + CACHE_SEED: SEED-1 # Bump the number to invalidate all caches RELENV_DATA: "${{ github.workspace }}/.relenv" permissions: diff --git a/.github/workflows/scheduled.yml b/.github/workflows/scheduled.yml index 71b8e334236..1d68e7b5d72 100644 --- a/.github/workflows/scheduled.yml +++ b/.github/workflows/scheduled.yml @@ -12,7 +12,7 @@ on: env: COLUMNS: 190 - CACHE_SEED: SEED-4 # Bump the number to invalidate all caches + CACHE_SEED: SEED-1 # Bump the number to invalidate all caches RELENV_DATA: "${{ github.workspace }}/.relenv" permissions: diff --git a/.github/workflows/staging.yml b/.github/workflows/staging.yml index 96f98137d33..947b4347f0f 100644 --- a/.github/workflows/staging.yml +++ b/.github/workflows/staging.yml @@ -37,7 +37,7 @@ on: env: COLUMNS: 190 - CACHE_SEED: SEED-4 # Bump the number to invalidate all caches + CACHE_SEED: SEED-1 # Bump the number to invalidate all caches RELENV_DATA: "${{ github.workspace }}/.relenv" permissions: diff --git a/.github/workflows/templates/layout.yml.jinja b/.github/workflows/templates/layout.yml.jinja index eeb53920faf..779ccdb1274 100644 --- a/.github/workflows/templates/layout.yml.jinja +++ b/.github/workflows/templates/layout.yml.jinja @@ -34,7 +34,7 @@ on: env: COLUMNS: 190 - CACHE_SEED: SEED-4 # Bump the number to invalidate all caches + CACHE_SEED: SEED-1 # Bump the number to invalidate all caches RELENV_DATA: "${{ github.workspace }}/.relenv" <%- endblock env %> From 885ddef0e0e3c7a4c40f8282963b7db046e1fcd9 Mon Sep 17 00:00:00 2001 From: "Daniel A. Wozniak" Date: Tue, 21 Nov 2023 23:07:04 -0700 Subject: [PATCH 13/15] revert un-wanted change --- pyproject.toml | 3 --- 1 file changed, 3 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index eb300235cb3..8afba8f0f4c 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,3 @@ -[project] -license = {text = "Apache-2.0"} - [tool.black] exclude= """ /( From 23d6ca83137bb86a3f835f63d84d00bc01abe29b Mon Sep 17 00:00:00 2001 From: "Daniel A. Wozniak" Date: Wed, 22 Nov 2023 02:56:59 -0700 Subject: [PATCH 14/15] Bump seed --- .github/workflows/ci.yml | 2 +- .github/workflows/nightly.yml | 2 +- .github/workflows/release.yml | 2 +- .github/workflows/scheduled.yml | 2 +- .github/workflows/staging.yml | 2 +- .github/workflows/templates/layout.yml.jinja | 2 +- 6 files changed, 6 insertions(+), 6 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index a0b9499f07f..86002b2eaf7 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -16,7 +16,7 @@ on: env: COLUMNS: 190 - CACHE_SEED: SEED-1 # Bump the number to invalidate all caches + CACHE_SEED: SEED-2 # Bump the number to invalidate all caches RELENV_DATA: "${{ github.workspace }}/.relenv" permissions: diff --git a/.github/workflows/nightly.yml b/.github/workflows/nightly.yml index ec836062e62..39c17c4948e 100644 --- a/.github/workflows/nightly.yml +++ b/.github/workflows/nightly.yml @@ -22,7 +22,7 @@ on: env: COLUMNS: 190 - CACHE_SEED: SEED-1 # Bump the number to invalidate all caches + CACHE_SEED: SEED-2 # Bump the number to invalidate all caches RELENV_DATA: "${{ github.workspace }}/.relenv" permissions: diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 5512ab1cb48..b671bc362a7 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -21,7 +21,7 @@ on: env: COLUMNS: 190 - CACHE_SEED: SEED-1 # Bump the number to invalidate all caches + CACHE_SEED: SEED-2 # Bump the number to invalidate all caches RELENV_DATA: "${{ github.workspace }}/.relenv" permissions: diff --git a/.github/workflows/scheduled.yml b/.github/workflows/scheduled.yml index 1d68e7b5d72..5d28eca9f60 100644 --- a/.github/workflows/scheduled.yml +++ b/.github/workflows/scheduled.yml @@ -12,7 +12,7 @@ on: env: COLUMNS: 190 - CACHE_SEED: SEED-1 # Bump the number to invalidate all caches + CACHE_SEED: SEED-2 # Bump the number to invalidate all caches RELENV_DATA: "${{ github.workspace }}/.relenv" permissions: diff --git a/.github/workflows/staging.yml b/.github/workflows/staging.yml index 947b4347f0f..0e6d820a74e 100644 --- a/.github/workflows/staging.yml +++ b/.github/workflows/staging.yml @@ -37,7 +37,7 @@ on: env: COLUMNS: 190 - CACHE_SEED: SEED-1 # Bump the number to invalidate all caches + CACHE_SEED: SEED-2 # Bump the number to invalidate all caches RELENV_DATA: "${{ github.workspace }}/.relenv" permissions: diff --git a/.github/workflows/templates/layout.yml.jinja b/.github/workflows/templates/layout.yml.jinja index 779ccdb1274..65398626219 100644 --- a/.github/workflows/templates/layout.yml.jinja +++ b/.github/workflows/templates/layout.yml.jinja @@ -34,7 +34,7 @@ on: env: COLUMNS: 190 - CACHE_SEED: SEED-1 # Bump the number to invalidate all caches + CACHE_SEED: SEED-2 # Bump the number to invalidate all caches RELENV_DATA: "${{ github.workspace }}/.relenv" <%- endblock env %> From f582cb23655e120c6512e84360dda18f9f54f439 Mon Sep 17 00:00:00 2001 From: "Daniel A. Wozniak" Date: Sat, 9 Dec 2023 15:39:10 -0700 Subject: [PATCH 15/15] Skip unsless test on windows --- tests/pytests/unit/modules/test_saltutil.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/pytests/unit/modules/test_saltutil.py b/tests/pytests/unit/modules/test_saltutil.py index 42986c464e1..7306c343405 100644 --- a/tests/pytests/unit/modules/test_saltutil.py +++ b/tests/pytests/unit/modules/test_saltutil.py @@ -177,7 +177,10 @@ def test_refresh_matchers(): assert ret is False +@pytest.mark.skip_on_windows def test_refresh_modules_async_false(): + # XXX: This test adds coverage but what is it really testing? Seems we'd be + # better off with at least a functional test here. kwargs = {"async": False} ret = saltutil.refresh_modules(**kwargs) assert ret is False