From 8cbe82e039817cd0faa03222e30c3c5757cfd416 Mon Sep 17 00:00:00 2001 From: Jamie Bliss Date: Sat, 1 Jun 2019 20:25:27 -0400 Subject: [PATCH 01/35] Modular Systems: Document saltenvs and modules --- doc/topics/development/modules/index.rst | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/doc/topics/development/modules/index.rst b/doc/topics/development/modules/index.rst index e95a485759c..e200ece8b82 100644 --- a/doc/topics/development/modules/index.rst +++ b/doc/topics/development/modules/index.rst @@ -55,9 +55,6 @@ prepended by underscore, such as: Modules must be synced before they can be used. This can happen a few ways, discussed below. -.. note: - Using saltenvs besides ``base`` may not work in all contexts. - Sync Via States ~~~~~~~~~~~~~~~ @@ -78,6 +75,15 @@ or specific dynamic modules. The ``saltutil.sync_*`` :py:mod:`runner functions ` can be used to sync modules to minions and the master, respectively. +About saltenvs +~~~~~~~~~~~~~~ + +In either of the above cases, which saltenvs are synced from are based on the topfiles. (Unless specified manually.) + +The minion syncs modules from saltenvs that mention the minion in the saltenv's topfile. + +That is, for each saltenv on the file server, if a minion is mentioned in that saltenv's topfile, the minion will load modules from that saltenv. + The extmods Directory --------------------- From b5cec62be33aa1db6affd64fa6f2f7ce087a3f3a Mon Sep 17 00:00:00 2001 From: Jamie Bliss Date: Sat, 1 Jun 2019 20:44:43 -0400 Subject: [PATCH 02/35] Rephrase and restructure better. --- doc/topics/development/modules/index.rst | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/doc/topics/development/modules/index.rst b/doc/topics/development/modules/index.rst index e200ece8b82..3cd41b317e5 100644 --- a/doc/topics/development/modules/index.rst +++ b/doc/topics/development/modules/index.rst @@ -64,7 +64,7 @@ dynamic modules when states are run. To disable this behavior set :conf_minion:`autoload_dynamic_modules` to ``False`` in the minion config. When dynamic modules are autoloaded via states, only the modules defined in the -same saltenvs as the states currently being run. +same saltenv as the states currently being run are synced. Sync Via the saltutil Module ~~~~~~~~~~~~~~~~~~~~~~~~~~~~ @@ -75,10 +75,7 @@ or specific dynamic modules. The ``saltutil.sync_*`` :py:mod:`runner functions ` can be used to sync modules to minions and the master, respectively. -About saltenvs -~~~~~~~~~~~~~~ - -In either of the above cases, which saltenvs are synced from are based on the topfiles. (Unless specified manually.) +Which saltenvs that the minion syncs from is based on the topfiles. (Unless specified manually.) The minion syncs modules from saltenvs that mention the minion in the saltenv's topfile. From f68568d6c3519ba6f8a62819e649f163c0c9ccb7 Mon Sep 17 00:00:00 2001 From: Jamie Bliss Date: Wed, 26 Jun 2019 16:24:01 -0400 Subject: [PATCH 03/35] Use @mchugh19's suggestion --- doc/topics/development/modules/index.rst | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/doc/topics/development/modules/index.rst b/doc/topics/development/modules/index.rst index 3cd41b317e5..97d43ef4da2 100644 --- a/doc/topics/development/modules/index.rst +++ b/doc/topics/development/modules/index.rst @@ -75,11 +75,7 @@ or specific dynamic modules. The ``saltutil.sync_*`` :py:mod:`runner functions ` can be used to sync modules to minions and the master, respectively. -Which saltenvs that the minion syncs from is based on the topfiles. (Unless specified manually.) - -The minion syncs modules from saltenvs that mention the minion in the saltenv's topfile. - -That is, for each saltenv on the file server, if a minion is mentioned in that saltenv's topfile, the minion will load modules from that saltenv. +If saltenv environments are used (through the :ref:`top file `, the :conf_minion:`environment` option of the minion configuration file, or as an argument on the command line) modules will be synced from the applied environments. The extmods Directory From 67703832e681c1fb2a8934a7f341f844d5d11c84 Mon Sep 17 00:00:00 2001 From: "Daniel A. Wozniak" Date: Mon, 18 Sep 2023 15:26:52 -0700 Subject: [PATCH 04/35] 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 05/35] 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 06/35] 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 07/35] 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 08/35] 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 09/35] 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 10/35] 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 11/35] 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 12/35] 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 13/35] 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 14/35] 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 15/35] 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 16/35] 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 17/35] 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 18/35] 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 From de12fc5ba522283d3054cce3511ecbe7612c570d Mon Sep 17 00:00:00 2001 From: "Daniel A. Wozniak" Date: Sat, 9 Dec 2023 23:54:34 -0700 Subject: [PATCH 19/35] Add 3007.x branch to releases --- cicd/shared-gh-workflows-context.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/cicd/shared-gh-workflows-context.yml b/cicd/shared-gh-workflows-context.yml index f30b579e934..5a62fde66c7 100644 --- a/cicd/shared-gh-workflows-context.yml +++ b/cicd/shared-gh-workflows-context.yml @@ -3,3 +3,4 @@ python_version: "3.10.13" relenv_version: "0.14.2" release-branches: - "3006.x" + - "3007.x" From 14d89b6ae38cee56e44f4c689748b19e0f97df5c Mon Sep 17 00:00:00 2001 From: Elias Probst Date: Fri, 27 Nov 2020 21:20:07 +0000 Subject: [PATCH 20/35] modules.system: document platform support for "reboot witnessed" functions The `system.{set,get}_reboot_required_witnessed` functions only work and make sense on NI Linux RT systems. Highlight this in their documentation. --- salt/modules/system.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/salt/modules/system.py b/salt/modules/system.py index 5d1be5f6fd2..784892c337a 100644 --- a/salt/modules/system.py +++ b/salt/modules/system.py @@ -651,6 +651,10 @@ NILRT_REBOOT_WITNESS_PATH = "/var/volatile/tmp/salt/reboot_witnessed" @depends("_is_nilrt_family") def set_reboot_required_witnessed(): """ + .. note:: + + This only applies to Minions running on NI Linux RT + This function is used to remember that an event indicating that a reboot is required was witnessed. This function writes to a temporary filesystem so the event gets cleared upon reboot. @@ -681,6 +685,10 @@ def set_reboot_required_witnessed(): @depends("_is_nilrt_family") def get_reboot_required_witnessed(): """ + .. note:: + + This only applies to Minions running on NI Linux RT + Determine if at any time during the current boot session the salt minion witnessed an event indicating that a reboot is required. From e681745c09c30a164c0604a9c14d7f8f3d0b5b97 Mon Sep 17 00:00:00 2001 From: Elias Probst Date: Sat, 28 Nov 2020 22:39:22 +0000 Subject: [PATCH 21/35] modules.system: improve documentation consistency Make better/more consistent use of rST features, such as: - use "verbatim" for inline code/variables - reference module functions using `:mod:` - fix syntax (e.g. missing empty line before list) Furthermore, improve the wording (prevent personal pronouns), spelling (e.g. use upper-cased acronyms such as `CLI` or `POSIX`) and grammar. --- salt/modules/system.py | 147 +++++++++++++++++++++-------------------- 1 file changed, 76 insertions(+), 71 deletions(-) diff --git a/salt/modules/system.py b/salt/modules/system.py index 784892c337a..18c062878b4 100644 --- a/salt/modules/system.py +++ b/salt/modules/system.py @@ -3,13 +3,13 @@ Support for reboot, shutdown, etc on POSIX-like systems. .. note:: - If you have configured a wrapper such as ``molly-guard`` to - intercept *interactive* shutdown commands, be aware that calling - ``system.halt``, ``system.poweroff``, ``system.reboot``, and - ``system.shutdown`` with ``salt-call`` will hang indefinitely - while the wrapper script waits for user input. Calling them with - ``salt`` will work as expected. - + If a wrapper such as ``molly-guard`` to intercept *interactive* shutdown + commands is configured, calling :mod:`system.halt `, + :mod:`system.poweroff `, + :mod:`system.reboot `, and + :mod:`system.shutdown ` with ``salt-call`` will + hang indefinitely while the wrapper script waits for user input. Calling them + with ``salt`` will work as expected. """ import os.path @@ -28,7 +28,7 @@ __virtualname__ = "system" def __virtual__(): """ Only supported on POSIX-like systems - Windows, Solaris, and Mac have their own modules + Windows, Solaris, and OS X have their own modules """ if salt.utils.platform.is_windows(): return (False, "This module is not available on Windows") @@ -137,7 +137,7 @@ def _date_bin_set_datetime(new_date): """ set the system date/time using the date command - Note using a strictly posix-compliant date binary we can only set the date + Note using a strictly POSIX-compliant date binary we can only set the date up to the minute. """ cmd = ["date"] @@ -176,7 +176,7 @@ def _date_bin_set_datetime(new_date): def has_settable_hwclock(): """ - Returns True if the system has a hardware clock capable of being + Returns ``True`` if the system has a hardware clock capable of being set from software. CLI Example: @@ -211,13 +211,13 @@ def _swclock_to_hwclock(): def _try_parse_datetime(time_str, fmts): """ - Attempts to parse the input time_str as a date. + Attempts to parse the input ``time_str`` as a date. :param str time_str: A string representing the time :param list fmts: A list of date format strings. - :return: Returns a datetime object if parsed properly. Otherwise None - :rtype datetime: + :return: Returns a datetime object if parsed properly. Otherwise ``None`` + :rtype: datetime """ result = None for fmt in fmts: @@ -231,9 +231,9 @@ def _try_parse_datetime(time_str, fmts): def _offset_to_min(utc_offset): """ - Helper function that converts the utc offset string into number of minutes - offset. Input is in form "[+-]?HHMM". Example valid inputs are "+0500" - "-0300" and "0800". These would return -300, 180, 480 respectively. + Helper function that converts the UTC offset string into number of minutes + offset. Input is in form ``[+-]?HHMM``. Example valid inputs are ``+0500`` + ``-0300`` and ``0800``. These would return ``-300``, ``180``, ``480`` respectively. """ match = re.match(r"^([+-])?(\d\d)(\d\d)$", utc_offset) if not match: @@ -250,7 +250,7 @@ def _get_offset_time(utc_offset): """ Will return the current time adjusted using the input timezone offset. - :rtype datetime: + :rtype: datetime """ if utc_offset is not None: minutes = _offset_to_min(utc_offset) @@ -266,12 +266,12 @@ def get_system_time(utc_offset=None): """ Get the system time. - :param str utc_offset: The utc offset in 4 digit (+0600) format with an - optional sign (+/-). Will default to None which will use the local - timezone. To set the time based off of UTC use "'+0000'". Note: if + :param str utc_offset: The UTC offset in 4 digit (e.g. ``+0600``) format with an + optional sign (``+``/``-``). Will default to ``None`` which will use the local + timezone. To set the time based off of UTC use ``+0000``. Note: If being passed through the command line will need to be quoted twice to - allow negative offsets. - :return: Returns the system time in HH:MM:SS AM/PM format. + allow negative offsets (e.g. ``"'+0000'"``). + :return: Returns the system time in ``HH:MM:SS AM/PM`` format. :rtype: str CLI Example: @@ -290,22 +290,23 @@ def set_system_time(newtime, utc_offset=None): :param str newtime: The time to set. Can be any of the following formats. - - HH:MM:SS AM/PM - - HH:MM AM/PM - - HH:MM:SS (24 hour) - - HH:MM (24 hour) - Note that the salt command line parser parses the date/time - before we obtain the argument (preventing us from doing utc) + - ``HH:MM:SS AM/PM`` + - ``HH:MM AM/PM`` + - ``HH:MM:SS`` (24 hour) + - ``HH:MM`` (24 hour) + + Note that the Salt command line parser parses the date/time + before we obtain the argument (preventing us from doing UTC) Therefore the argument must be passed in as a string. - Meaning you may have to quote the text twice from the command line. + Meaning the text might have to be quoted twice on the command line. - :param str utc_offset: The utc offset in 4 digit (+0600) format with an - optional sign (+/-). Will default to None which will use the local - timezone. To set the time based off of UTC use "'+0000'". Note: if + :param str utc_offset: The UTC offset in 4 digit (``+0600``) format with an + optional sign (``+``/``-``). Will default to ``None`` which will use the local + timezone. To set the time based off of UTC use ``+0000``. Note: If being passed through the command line will need to be quoted twice to - allow negative offsets. - :return: Returns True if successful. Otherwise False. + allow negative offsets (e.g. ``"'+0000'"``) + :return: Returns ``True`` if successful. Otherwise ``False``. :rtype: bool CLI Example: @@ -331,12 +332,12 @@ def get_system_date_time(utc_offset=None): """ Get the system date/time. - :param str utc_offset: The utc offset in 4 digit (+0600) format with an - optional sign (+/-). Will default to None which will use the local - timezone. To set the time based off of UTC use "'+0000'". Note: if + :param str utc_offset: The UTC offset in 4 digit (``+0600``) format with an + optional sign (``+``/``-``). Will default to ``None`` which will use the local + timezone. To set the time based off of UTC use ``+0000``. Note: If being passed through the command line will need to be quoted twice to - allow negative offsets. - :return: Returns the system time in YYYY-MM-DD hh:mm:ss format. + allow negative offsets (e.g. ``"'+0000'"``). + :return: Returns the system time in ``YYYY-MM-DD hh:mm:ss`` format. :rtype: str CLI Example: @@ -361,25 +362,26 @@ def set_system_date_time( """ Set the system date and time. Each argument is an element of the date, but not required. If an element is not passed, the current system value for - that element will be used. For example, if you don't pass the year, the - current system year will be used. (Used by set_system_date and - set_system_time) + that element will be used. For example, if the year is not passed, the + current system year will be used. (Used by + :mod:`system.set_system_date ` and + :mod:`system.set_system_time `) Updates hardware clock, if present, in addition to software (kernel) clock. - :param int years: Years digit, ie: 2015 - :param int months: Months digit: 1 - 12 - :param int days: Days digit: 1 - 31 - :param int hours: Hours digit: 0 - 23 - :param int minutes: Minutes digit: 0 - 59 - :param int seconds: Seconds digit: 0 - 59 - :param str utc_offset: The utc offset in 4 digit (+0600) format with an - optional sign (+/-). Will default to None which will use the local - timezone. To set the time based off of UTC use "'+0000'". Note: if + :param int years: Years digit, e.g.: ``2015`` + :param int months: Months digit: ``1``-``12`` + :param int days: Days digit: ``1``-``31`` + :param int hours: Hours digit: ``0``-``23`` + :param int minutes: Minutes digit: ``0``-``59`` + :param int seconds: Seconds digit: ``0``-``59`` + :param str utc_offset: The UTC offset in 4 digit (``+0600``) format with an + optional sign (``+``/``-``). Will default to ``None`` which will use the local + timezone. To set the time based off of UTC use ``+0000``. Note: If being passed through the command line will need to be quoted twice to - allow negative offsets. - :return: True if successful. Otherwise False. + allow negative offsets (e.g. ``"'+0000'"``). + :return: ``True`` if successful. Otherwise ``False``. :rtype: bool CLI Example: @@ -427,11 +429,11 @@ def get_system_date(utc_offset=None): """ Get the system date - :param str utc_offset: The utc offset in 4 digit (+0600) format with an - optional sign (+/-). Will default to None which will use the local - timezone. To set the time based off of UTC use "'+0000'". Note: if + :param str utc_offset: The UTC offset in 4 digit (``+0600``) format with an + optional sign (``+``/``-``). Will default to ``None`` which will use the local + timezone. To set the time based off of UTC use ``+0000``. Note: If being passed through the command line will need to be quoted twice to - allow negative offsets. + allow negative offsets (e.g. ``"'+0000'"``). :return: Returns the system date. :rtype: str @@ -447,17 +449,17 @@ def get_system_date(utc_offset=None): def set_system_date(newdate, utc_offset=None): """ - Set the system date. Use format for the date. + Set the system date. Use ```` format for the date. :param str newdate: The date to set. Can be any of the following formats: - - YYYY-MM-DD - - MM-DD-YYYY - - MM-DD-YY - - MM/DD/YYYY - - MM/DD/YY - - YYYY/MM/DD + - ``YYYY-MM-DD`` + - ``MM-DD-YYYY`` + - ``MM-DD-YY`` + - ``MM/DD/YYYY`` + - ``MM/DD/YY`` + - ``YYYY/MM/DD`` CLI Example: @@ -505,7 +507,7 @@ class _FixedOffset(tzinfo): def _strip_quotes(str_q): """ - Helper function to strip off the ' or " off of a string + Helper function to strip off the ``'`` or ``"`` off of a string """ if str_q[0] == str_q[-1] and str_q.startswith(("'", '"')): return str_q[1:-1] @@ -514,11 +516,12 @@ def _strip_quotes(str_q): def get_computer_desc(): """ - Get PRETTY_HOSTNAME value stored in /etc/machine-info + Get ``PRETTY_HOSTNAME`` value stored in ``/etc/machine-info`` If this file doesn't exist or the variable doesn't exist - return False. + return ``False``. - :return: Value of PRETTY_HOSTNAME if this does not exist False. + :return: Value of ``PRETTY_HOSTNAME`` in ``/etc/machine-info``. + If file/variable does not exist ``False``. :rtype: str CLI Example: @@ -560,12 +563,12 @@ def get_computer_desc(): def set_computer_desc(desc): """ - Set PRETTY_HOSTNAME value stored in /etc/machine-info + Set ``PRETTY_HOSTNAME`` value stored in ``/etc/machine-info`` This will create the file if it does not exist. If - it is unable to create or modify this file returns False. + it is unable to create or modify this file, ``False`` is returned. :param str desc: The computer description - :return: False on failure. True if successful. + :return: ``False`` on failure. ``True`` if successful. CLI Example: @@ -662,6 +665,8 @@ def set_reboot_required_witnessed(): Returns: bool: ``True`` if successful, otherwise ``False`` + CLI Example: + .. code-block:: bash salt '*' system.set_reboot_required_witnessed From 913c0f75662dc1ba0359479fce5054fed07fd0ac Mon Sep 17 00:00:00 2001 From: piterpunk Date: Thu, 20 May 2021 20:30:10 -0300 Subject: [PATCH 22/35] Removed an unused assignment in file.patch --- changelog/57204.fixed | 1 + salt/states/file.py | 2 -- 2 files changed, 1 insertion(+), 2 deletions(-) create mode 100644 changelog/57204.fixed diff --git a/changelog/57204.fixed b/changelog/57204.fixed new file mode 100644 index 00000000000..038b6642852 --- /dev/null +++ b/changelog/57204.fixed @@ -0,0 +1 @@ +Removed an unused assignment in file.patch diff --git a/salt/states/file.py b/salt/states/file.py index d41895e1515..bce54092a0f 100644 --- a/salt/states/file.py +++ b/salt/states/file.py @@ -7165,8 +7165,6 @@ def patch( ) return ret - options = sanitized_options - try: source_match = __salt__["file.source_list"](source, source_hash, __env__)[0] except CommandExecutionError as exc: From 84b8993543f13bdbf3ecaa3d7ef162dcbcafa23e Mon Sep 17 00:00:00 2001 From: mreider Date: Thu, 7 Dec 2023 11:34:10 -0500 Subject: [PATCH 23/35] add documentation for facl defaults, in acl state --- salt/states/linux_acl.py | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/salt/states/linux_acl.py b/salt/states/linux_acl.py index 55157457486..b854ffb818b 100644 --- a/salt/states/linux_acl.py +++ b/salt/states/linux_acl.py @@ -14,6 +14,17 @@ Ensure a Linux ACL is present - acl_name: damian - perms: rwx +Ensure a Linux ACL is present as a default for all new objects + +.. code-block:: yaml + + root: + acl.present: + - name: /root + - acl_type: "default:user" + - acl_name: damian + - perms: rwx + Ensure a Linux ACL does not exist .. code-block:: yaml From 341f844c6ded20525b2099cc7d01f91ade6abb01 Mon Sep 17 00:00:00 2001 From: Ari Maniatis Date: Thu, 21 Jan 2021 16:08:06 +1100 Subject: [PATCH 24/35] Better list of FreeBSD ami for EC2 The old list was many many years out of date. --- doc/topics/cloud/aws.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/topics/cloud/aws.rst b/doc/topics/cloud/aws.rst index 76833f41f53..6c043228899 100644 --- a/doc/topics/cloud/aws.rst +++ b/doc/topics/cloud/aws.rst @@ -758,7 +758,7 @@ them have never been used, much less tested, by the Salt Stack team. * `FreeBSD`__ -.. __: http://www.daemonology.net/freebsd-on-ec2/ +.. __: https://aws.amazon.com/marketplace/search/results?filters=vendor_id&vendor_id=92bb514d-02bc-49fd-9727-c474863f63da * `Fedora`__ From 9e95e3b2bf53a150d3684fe087f34ceb941187f7 Mon Sep 17 00:00:00 2001 From: "Daniel A. Wozniak" Date: Sun, 10 Dec 2023 14:55:32 -0700 Subject: [PATCH 25/35] Fix changelog entry --- changelog/{57204.fixed => 57204.fixed.md} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename changelog/{57204.fixed => 57204.fixed.md} (100%) diff --git a/changelog/57204.fixed b/changelog/57204.fixed.md similarity index 100% rename from changelog/57204.fixed rename to changelog/57204.fixed.md From 54cf836d58939d8c507d14e11d230f7fe4640ba7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Konrad=20Moso=C5=84?= Date: Sun, 2 May 2021 18:36:51 +0200 Subject: [PATCH 26/35] Cloud/DigitalOcean: Don't throw error when deleting instance, fixes #58190 --- salt/cloud/clouds/digitalocean.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/salt/cloud/clouds/digitalocean.py b/salt/cloud/clouds/digitalocean.py index 91c1ef71fa1..487179f6de0 100644 --- a/salt/cloud/clouds/digitalocean.py +++ b/salt/cloud/clouds/digitalocean.py @@ -985,7 +985,7 @@ def destroy_dns_records(fqdn): records = response["domain_records"] if records: - record_ids = [r["id"] for r in records if r["name"].decode() == hostname] + record_ids = [r["id"] for r in records if r["name"] == hostname] log.debug("deleting DNS record IDs: %s", record_ids) for id_ in record_ids: try: From 0b423e443fd2111a319c60b236a2a856edc129ef Mon Sep 17 00:00:00 2001 From: Ari Maniatis Date: Thu, 19 Aug 2021 16:41:50 +1000 Subject: [PATCH 27/35] The script attribute is not userdata --- doc/topics/cloud/aws.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/topics/cloud/aws.rst b/doc/topics/cloud/aws.rst index 6c043228899..b2225639d04 100644 --- a/doc/topics/cloud/aws.rst +++ b/doc/topics/cloud/aws.rst @@ -289,7 +289,7 @@ Set up an initial profile at ``/etc/salt/cloud.profiles``: image: ami-a73264ce size: m1.xlarge ssh_username: ec2-user - script: /etc/salt/cloud.deploy.d/user_data.sh + script: /etc/salt/cloud.deploy.d/my_bootstrap.sh network_interfaces: - DeviceIndex: 0 PrivateIpAddresses: From 30690637b5636565592d4ec3210cc615098fc3c1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Krzysztof=20Paw=C5=82owski?= Date: Fri, 14 Apr 2023 17:35:38 +0200 Subject: [PATCH 28/35] Return error if patch file passed to state file.patch is malformed If patch file provided for file.patch state is malformed then state returns `Patch was already applied` but patch is not applied. ID: patch_example Function: file.patch Name: /tmp/example Result: True Comment: Patch was already applied Started: 12:20:50.953163 Duration: 61.558 ms Changes: It is better to return error in such case. ID: patch_example Function: file.patch Name: /tmp/example Result: False Comment: /usr/bin/patch: **** malformed patch at line 7: Started: 12:33:44.915605 Duration: 59.202 ms Changes: --- changelog/59806.fixed.md | 1 + salt/states/file.py | 4 ++++ tests/pytests/integration/states/test_file.py | 3 ++- 3 files changed, 7 insertions(+), 1 deletion(-) create mode 100644 changelog/59806.fixed.md diff --git a/changelog/59806.fixed.md b/changelog/59806.fixed.md new file mode 100644 index 00000000000..2cca505c2bf --- /dev/null +++ b/changelog/59806.fixed.md @@ -0,0 +1 @@ +Return error if patch file passed to state file.patch is malformed. diff --git a/salt/states/file.py b/salt/states/file.py index bce54092a0f..d591ae9d8dc 100644 --- a/salt/states/file.py +++ b/salt/states/file.py @@ -7263,6 +7263,10 @@ def patch( pre_check = _patch(patch_file, patch_opts) if pre_check["retcode"] != 0: + if not os.path.exists(patch_rejects) or os.path.getsize(patch_rejects) == 0: + ret["comment"] = pre_check["stderr"] + ret["result"] = False + return ret # Try to reverse-apply hunks from rejects file using a dry-run. # If this returns a retcode of 0, we know that the patch was # already applied. Rejects are written from the base of the diff --git a/tests/pytests/integration/states/test_file.py b/tests/pytests/integration/states/test_file.py index a84d2f4797e..cd64fc918b6 100644 --- a/tests/pytests/integration/states/test_file.py +++ b/tests/pytests/integration/states/test_file.py @@ -929,6 +929,7 @@ def test_patch_directory_template( - source: {all_patch_template} - template: "jinja" - context: {context} + - strip: 1 """.format( base_dir=tmp_path, all_patch_template=all_patch_template, context=context ) @@ -945,7 +946,7 @@ def test_patch_directory_template( # Check to make sure the patch was applied okay state_run = next(iter(ret.data.values())) assert state_run["result"] is True - assert state_run["comment"] == "Patch was already applied" + assert state_run["comment"] == "Patch successfully applied" # Re-run the state, should succeed and there should be a message about # a partially-applied hunk. From b9b0c2462be09ef62419688da78ba075925be86d Mon Sep 17 00:00:00 2001 From: Benjamin Drung Date: Wed, 13 Oct 2021 17:43:43 +0200 Subject: [PATCH 29/35] doc: Exclude documentation_options.js from default theme `documentation_options.js` from the default theme sets the option `URL_ROOT` to: ``` document.getElementById("documentation_options").getAttribute('data-url_root') ``` This requires that the script element for `documentation_options.js` includes the tag `id="documentation_options"` and sets the `data-url_root` tag. Otherwise evaluating `URL_ROOT` will fail and building the documentation during the Debian package build will fail: ``` dh_sphinxdoc: error: DOCUMENTATION_OPTIONS does not define URL_ROOT ``` The variable `DOCUMENTATION_OPTIONS` is directly set `layout.html` and therefore `documentation_options.js` does not need to be included. So just exclude it. Signed-off-by: Benjamin Drung --- doc/_themes/saltstack/layout.html | 1 + 1 file changed, 1 insertion(+) diff --git a/doc/_themes/saltstack/layout.html b/doc/_themes/saltstack/layout.html index 7589881f71f..8b2426c4d66 100644 --- a/doc/_themes/saltstack/layout.html +++ b/doc/_themes/saltstack/layout.html @@ -21,6 +21,7 @@ {# Remove old version of jQuery #} {% set js_blacklist = [ + '_static/documentation_options.js', '_static/jquery.js', ] %} From 21bdb366e58b83159e38ea825119b15d6d1f5148 Mon Sep 17 00:00:00 2001 From: code-review-doctor Date: Sun, 24 Apr 2022 17:25:18 +0100 Subject: [PATCH 30/35] Fix issue probably-meant-fstring found at https://codereview.doctor --- salt/modules/mount.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/salt/modules/mount.py b/salt/modules/mount.py index 869c6f2016d..d2937877bc8 100644 --- a/salt/modules/mount.py +++ b/salt/modules/mount.py @@ -1924,7 +1924,7 @@ def set_filesystems( except OSError: raise CommandExecutionError("File not writable {}".format(config)) except Exception as exc: - raise CommandExecutionError("set_filesystems error exception {exc}") + raise CommandExecutionError(f"set_filesystems error exception {exc}") return ret @@ -1973,7 +1973,7 @@ def rm_filesystems(name, device, config="/etc/filesystems"): except OSError as exc: raise CommandExecutionError("Couldn't write to {}: {}".format(config, exc)) except Exception as exc: - raise CommandExecutionError("rm_filesystems error exception {exc}") + raise CommandExecutionError(f"rm_filesystems error exception {exc}") return modified From f933e66a771c00f5b2d15c41ae2da92e7fcd2c04 Mon Sep 17 00:00:00 2001 From: Thomas Merkel Date: Thu, 7 Jul 2022 15:09:05 +0200 Subject: [PATCH 31/35] doc: gitfs_remotes provide additional ordering information The user should be informed that a state in a file will overrule a state in an directory for gitfs_remotes because the remotes are merged together to one structure. An extra example should illustrate that it. --- doc/topics/tutorials/gitfs.rst | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/doc/topics/tutorials/gitfs.rst b/doc/topics/tutorials/gitfs.rst index 0c85b61445b..5e6a375663c 100644 --- a/doc/topics/tutorials/gitfs.rst +++ b/doc/topics/tutorials/gitfs.rst @@ -276,10 +276,12 @@ And each repository contains some files: edit/vim.sls edit/vimrc nginx/init.sls + shell/init.sls second.git: edit/dev_vimrc haproxy/init.sls + shell.sls third: haproxy/haproxy.conf @@ -296,6 +298,12 @@ is executed. For example: * A request for the file :strong:`salt://haproxy/haproxy.conf` will be served from the :strong:`file:///root/third` repo. +Also a requested state file overrules a directories with an `init.sls`-file. +For example: + +* A request for :strong:`state.apply shell` will be served from the + :strong:`https://github.com/example/second.git` git repo. + .. note:: This example is purposefully contrived to illustrate the behavior of the From 788b922d3491203aaa1c30cd32eb7dd525680739 Mon Sep 17 00:00:00 2001 From: Thomas Merkel Date: Sun, 4 Sep 2022 21:04:25 +0200 Subject: [PATCH 32/35] Update doc/topics/tutorials/gitfs.rst Co-authored-by: Caleb Beard <53276404+MKLeb@users.noreply.github.com> --- doc/topics/tutorials/gitfs.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/topics/tutorials/gitfs.rst b/doc/topics/tutorials/gitfs.rst index 5e6a375663c..acc7051c3f3 100644 --- a/doc/topics/tutorials/gitfs.rst +++ b/doc/topics/tutorials/gitfs.rst @@ -298,7 +298,7 @@ is executed. For example: * A request for the file :strong:`salt://haproxy/haproxy.conf` will be served from the :strong:`file:///root/third` repo. -Also a requested state file overrules a directories with an `init.sls`-file. +Also a requested state file overrules a directory with an `init.sls`-file. For example: * A request for :strong:`state.apply shell` will be served from the From 7a366444ab2db5b102e16859eb172161f2a5378c Mon Sep 17 00:00:00 2001 From: "Daniel A. Wozniak" Date: Sun, 10 Dec 2023 18:22:06 -0700 Subject: [PATCH 33/35] Fix pre-commit --- doc/topics/tutorials/gitfs.rst | 1 + salt/modules/mount.py | 110 ++++++++++++++++----------------- 2 files changed, 54 insertions(+), 57 deletions(-) diff --git a/doc/topics/tutorials/gitfs.rst b/doc/topics/tutorials/gitfs.rst index acc7051c3f3..a149896802c 100644 --- a/doc/topics/tutorials/gitfs.rst +++ b/doc/topics/tutorials/gitfs.rst @@ -1,5 +1,6 @@ .. _tutorial-gitfs: + ================================== Git Fileserver Backend Walkthrough ================================== diff --git a/salt/modules/mount.py b/salt/modules/mount.py index d2937877bc8..f396912ba24 100644 --- a/salt/modules/mount.py +++ b/salt/modules/mount.py @@ -197,7 +197,7 @@ def _active_mounts_openbsd(ret): comps = re.sub(r"\s+", " ", line).split() parens = re.findall(r"\((.*?)\)", line, re.DOTALL) if len(parens) > 1: - nod = __salt__["cmd.run_stdout"]("ls -l {}".format(comps[0])) + nod = __salt__["cmd.run_stdout"](f"ls -l {comps[0]}") nod = " ".join(nod.split()).split(" ") ret[comps[3]] = { "device": comps[0], @@ -306,7 +306,7 @@ class _fstab_entry: @classmethod def dict_from_line(cls, line, keys=fstab_keys): if len(keys) != 6: - raise ValueError("Invalid key array: {}".format(keys)) + raise ValueError(f"Invalid key array: {keys}") if line.startswith("#"): raise cls.ParseError("Comment!") @@ -523,12 +523,12 @@ class _FileSystemsEntry: @classmethod def dict_from_lines(cls, lines, keys=filesystems_keys): if len(lines) < 2: - raise ValueError("Invalid number of lines: {}".format(lines)) + raise ValueError(f"Invalid number of lines: {lines}") if not keys: # if empty force default filesystems_keys keys = _FileSystemsEntry.filesystems_keys elif len(keys) < 6: - raise ValueError("Invalid key name array: {}".format(keys)) + raise ValueError(f"Invalid key name array: {keys}") blk_lines = lines orddict = OrderedDict() @@ -546,9 +546,7 @@ class _FileSystemsEntry: if key_name in keys: orddict[key_name] = comps[1].strip() else: - raise ValueError( - "Invalid name for use in filesystems: {}".format(key_name) - ) + raise ValueError(f"Invalid name for use in filesystems: {key_name}") return orddict @@ -575,7 +573,7 @@ class _FileSystemsEntry: strg_out = entry["name"] + ":" + os.linesep for k, v in entry.items(): if "name" not in k: - strg_out += "\t{}\t\t= {}".format(k, v) + os.linesep + strg_out += f"\t{k}\t\t= {v}" + os.linesep strg_out += os.linesep return str(strg_out) @@ -586,7 +584,7 @@ class _FileSystemsEntry: list_out.append(str(entry["name"] + ":" + os.linesep)) for k, v in entry.items(): if "name" not in k: - list_out.append(str("\t{}\t\t= {}".format(k, v) + os.linesep)) + list_out.append(str(f"\t{k}\t\t= {v}" + os.linesep)) list_out.append(str(os.linesep)) return list_out @@ -793,7 +791,7 @@ def set_fstab( test=False, match_on="auto", not_change=False, - **kwargs + **kwargs, ): """ Verify that this mount is represented in the fstab, change the mount @@ -869,12 +867,12 @@ def set_fstab( filterFn = lambda key: key not in _fstab_entry.fstab_keys invalid_keys = filter(filterFn, match_on) - msg = 'Unrecognized keys in match_on: "{}"'.format(invalid_keys) + msg = f'Unrecognized keys in match_on: "{invalid_keys}"' raise CommandExecutionError(msg) # parse file, use ret to cache status if not os.path.isfile(config): - raise CommandExecutionError('Bad config file "{}"'.format(config)) + raise CommandExecutionError(f'Bad config file "{config}"') try: with salt.utils.files.fopen(config, "r") as ifile: @@ -930,7 +928,7 @@ def set_vfstab( test=False, match_on="auto", not_change=False, - **kwargs + **kwargs, ): """ .. versionadded:: 2016.3.2 @@ -999,12 +997,12 @@ def set_vfstab( filterFn = lambda key: key not in _vfstab_entry.vfstab_keys invalid_keys = filter(filterFn, match_on) - msg = 'Unrecognized keys in match_on: "{}"'.format(invalid_keys) + msg = f'Unrecognized keys in match_on: "{invalid_keys}"' raise CommandExecutionError(msg) # parse file, use ret to cache status if not os.path.isfile(config): - raise CommandExecutionError('Bad config file "{}"'.format(config)) + raise CommandExecutionError(f'Bad config file "{config}"') try: with salt.utils.files.fopen(config, "r") as ifile: @@ -1117,7 +1115,7 @@ def set_automaster( config="/etc/auto_salt", test=False, not_change=False, - **kwargs + **kwargs, ): """ Verify that this mount is represented in the auto_salt, change the mount @@ -1139,11 +1137,11 @@ def set_automaster( if not os.path.isfile(config): __salt__["file.touch"](config) - __salt__["file.append"](automaster_file, "/-\t\t\t{}".format(config)) + __salt__["file.append"](automaster_file, f"/-\t\t\t{config}") - name = "/..{}".format(name) - device_fmt = "{}:{}".format(fstype, device) - type_opts = "-fstype={},{}".format(fstype, opts) + name = f"/..{name}" + device_fmt = f"{fstype}:{device}" + type_opts = f"-fstype={fstype},{opts}" if fstype == "smbfs": device_fmt = device_fmt.replace(fstype, "") @@ -1185,7 +1183,7 @@ def set_automaster( "auto_master entry for mount point %s needs to be updated", name, ) - newline = "{}\t{}\t{}\n".format(name, type_opts, device_fmt) + newline = f"{name}\t{type_opts}\t{device_fmt}\n" lines.append(newline) else: lines.append(line) @@ -1212,14 +1210,14 @@ def set_automaster( else: if not salt.utils.args.test_mode(test=test, **kwargs): # The entry is new, add it to the end of the fstab - newline = "{}\t{}\t{}\n".format(name, type_opts, device_fmt) + newline = f"{name}\t{type_opts}\t{device_fmt}\n" lines.append(newline) try: with salt.utils.files.fopen(config, "wb") as ofile: # The line was changed, commit it! ofile.writelines(salt.utils.data.encode(lines)) except OSError: - raise CommandExecutionError("File not writable {}".format(config)) + raise CommandExecutionError(f"File not writable {config}") return "new" @@ -1280,7 +1278,7 @@ def mount( if not mnt: return False first = next(iter(mnt.keys())) - __context__["img.mnt_{}".format(first)] = mnt + __context__[f"img.mnt_{first}"] = mnt return first return False @@ -1297,24 +1295,24 @@ def mount( args = "" if opts is not None: lopts = ",".join(opts) - args = "-o {}".format(lopts) + args = f"-o {lopts}" if fstype: # use of fstype on AIX differs from typical Linux use of -t # functionality AIX uses -v vfsname, -t fstype mounts all with # fstype in /etc/filesystems if "AIX" in __grains__["os"]: - args += " -v {}".format(fstype) + args += f" -v {fstype}" elif "solaris" in __grains__["os"].lower(): - args += " -F {}".format(fstype) + args += f" -F {fstype}" else: - args += " -t {}".format(fstype) + args += f" -t {fstype}" cmd = "mount " if device: - cmd += "{} '{}' '{}' ".format(args, device, name) + cmd += f"{args} '{device}' '{name}' " else: - cmd += "'{}' ".format(name) + cmd += f"'{name}' " out = __salt__["cmd.run_all"](cmd, runas=user, python_shell=False) if out["retcode"]: return out["stderr"] @@ -1360,23 +1358,23 @@ def remount(name, device, mkmnt=False, fstype="", opts="defaults", user=None): args = "" if opts: lopts = ",".join(opts) - args = "-o {}".format(lopts) + args = f"-o {lopts}" if fstype: # use of fstype on AIX differs from typical Linux use of # -t functionality AIX uses -v vfsname, -t fstype mounts # all with fstype in /etc/filesystems if "AIX" in __grains__["os"]: - args += " -v {}".format(fstype) + args += f" -v {fstype}" elif "solaris" in __grains__["os"].lower(): - args += " -F {}".format(fstype) + args += f" -F {fstype}" else: - args += " -t {}".format(fstype) + args += f" -t {fstype}" if __grains__["os"] not in ["OpenBSD", "MacOS", "Darwin"] or force_mount: - cmd = "mount {} '{}' '{}' ".format(args, device, name) + cmd = f"mount {args} '{device}' '{name}' " else: - cmd = "mount -u {} '{}' '{}' ".format(args, device, name) + cmd = f"mount -u {args} '{device}' '{name}' " out = __salt__["cmd.run_all"](cmd, runas=user, python_shell=False) if out["retcode"]: return out["stderr"] @@ -1407,18 +1405,18 @@ def umount(name, device=None, user=None, util="mount"): elif util == "qemu_nbd": # This functionality used to live in img.umount_image if "qemu_nbd.clear" in __salt__: - if "img.mnt_{}".format(name) in __context__: - __salt__["qemu_nbd.clear"](__context__["img.mnt_{}".format(name)]) + if f"img.mnt_{name}" in __context__: + __salt__["qemu_nbd.clear"](__context__[f"img.mnt_{name}"]) return mnts = active() if name not in mnts: - return "{} does not have anything mounted".format(name) + return f"{name} does not have anything mounted" if not device: - cmd = "umount '{}'".format(name) + cmd = f"umount '{name}'" else: - cmd = "umount '{}'".format(device) + cmd = f"umount '{device}'" out = __salt__["cmd.run_all"](cmd, runas=user, python_shell=False) if out["retcode"]: return out["stderr"] @@ -1443,7 +1441,7 @@ def is_fuse_exec(cmd): elif not salt.utils.path.which("ldd"): raise CommandNotFoundError("ldd") - out = __salt__["cmd.run"]("ldd {}".format(cmd_path), python_shell=False) + out = __salt__["cmd.run"](f"ldd {cmd_path}", python_shell=False) return "libfuse" in out @@ -1535,13 +1533,13 @@ def swapon(name, priority=None): if __grains__["kernel"] == "SunOS": if __grains__["virtual"] != "zone": - __salt__["cmd.run"]("swap -a '{}'".format(name), python_shell=False) + __salt__["cmd.run"](f"swap -a '{name}'", python_shell=False) else: return False else: - cmd = "swapon '{}'".format(name) + cmd = f"swapon '{name}'" if priority and "AIX" not in __grains__["kernel"]: - cmd += " -p {}".format(priority) + cmd += f" -p {priority}" __salt__["cmd.run"](cmd, python_shell=False) on_ = swaps() @@ -1569,13 +1567,13 @@ def swapoff(name): if name in on_: if __grains__["kernel"] == "SunOS": if __grains__["virtual"] != "zone": - __salt__["cmd.run"]("swap -a '{}'".format(name), python_shell=False) + __salt__["cmd.run"](f"swap -a '{name}'", python_shell=False) else: return False elif __grains__["os"] != "OpenBSD": - __salt__["cmd.run"]("swapoff '{}'".format(name), python_shell=False) + __salt__["cmd.run"](f"swapoff '{name}'", python_shell=False) else: - __salt__["cmd.run"]("swapctl -d '{}'".format(name), python_shell=False) + __salt__["cmd.run"](f"swapctl -d '{name}'", python_shell=False) on_ = swaps() if name in on_: return False @@ -1779,7 +1777,7 @@ def set_filesystems( test=False, match_on="auto", not_change=False, - **kwargs + **kwargs, ): """ .. versionadded:: 2018.3.3 @@ -1880,13 +1878,11 @@ def set_filesystems( except KeyError: filterFn = lambda key: key not in _FileSystemsEntry.compatibility_keys invalid_keys = filter(filterFn, match_on) - raise CommandExecutionError( - 'Unrecognized keys in match_on: "{}"'.format(invalid_keys) - ) + raise CommandExecutionError(f'Unrecognized keys in match_on: "{invalid_keys}"') # parse file, use ret to cache status if not os.path.isfile(config): - raise CommandExecutionError('Bad config file "{}"'.format(config)) + raise CommandExecutionError(f'Bad config file "{config}"') # read in block of filesystem, block starts with '/' till empty line try: @@ -1904,7 +1900,7 @@ def set_filesystems( view_lines.append(fsys_view) except OSError as exc: - raise CommandExecutionError("Couldn't read from {}: {}".format(config, exc)) + raise CommandExecutionError(f"Couldn't read from {config}: {exc}") # add line if not present or changed if ret is None: @@ -1922,7 +1918,7 @@ def set_filesystems( ofile.writelines(salt.utils.data.encode(list_strgs)) except OSError: - raise CommandExecutionError("File not writable {}".format(config)) + raise CommandExecutionError(f"File not writable {config}") except Exception as exc: raise CommandExecutionError(f"set_filesystems error exception {exc}") @@ -1961,7 +1957,7 @@ def rm_filesystems(name, device, config="/etc/filesystems"): view_lines.append(fsys_view) except OSError as exc: - raise CommandExecutionError("Couldn't read from {}: {}".format(config, exc)) + raise CommandExecutionError(f"Couldn't read from {config}: {exc}") if modified: try: @@ -1971,7 +1967,7 @@ def rm_filesystems(name, device, config="/etc/filesystems"): list_strgs = _FileSystemsEntry.dict_to_list_lines(entry) ofile.writelines(salt.utils.data.encode(list_strgs)) except OSError as exc: - raise CommandExecutionError("Couldn't write to {}: {}".format(config, exc)) + raise CommandExecutionError(f"Couldn't write to {config}: {exc}") except Exception as exc: raise CommandExecutionError(f"rm_filesystems error exception {exc}") From c51c7e856ada9fb8691c906580e52b380dd563c1 Mon Sep 17 00:00:00 2001 From: "Daniel A. Wozniak" Date: Sun, 10 Dec 2023 15:01:22 -0700 Subject: [PATCH 34/35] Update documentation of encoding kwarg --- salt/states/postgres_database.py | 29 +++++++++++++++++------------ 1 file changed, 17 insertions(+), 12 deletions(-) diff --git a/salt/states/postgres_database.py b/salt/states/postgres_database.py index 7e162d6e7ff..eaeec035273 100644 --- a/salt/states/postgres_database.py +++ b/salt/states/postgres_database.py @@ -51,7 +51,12 @@ def present( Default tablespace for the database encoding - The character encoding scheme to be used in this database + The character encoding scheme to be used in this database. The encoding + has to be defined in the following format (without hyphen). + + .. code-block:: yaml + + - encoding: UTF8 lc_collate The LC_COLLATE setting to be used in this database @@ -89,7 +94,7 @@ def present( "name": name, "changes": {}, "result": True, - "comment": "Database {} is already present".format(name), + "comment": f"Database {name} is already present", } db_args = { @@ -138,11 +143,11 @@ def present( if __opts__["test"]: ret["result"] = None if name not in dbs: - ret["comment"] = "Database {} is set to be created".format(name) + ret["comment"] = f"Database {name} is set to be created" else: ret[ "comment" - ] = "Database {} exists, but parameters need to be changed".format(name) + ] = f"Database {name} exists, but parameters need to be changed" return ret if name not in dbs and __salt__["postgres.db_create"]( name, @@ -152,20 +157,20 @@ def present( lc_ctype=lc_ctype, owner=owner, template=template, - **db_args + **db_args, ): - ret["comment"] = "The database {} has been created".format(name) + ret["comment"] = f"The database {name} has been created" ret["changes"][name] = "Present" elif name in dbs and __salt__["postgres.db_alter"]( name, tablespace=tablespace, owner=owner, owner_recurse=owner_recurse, **db_args ): - ret["comment"] = "Parameters for database {} have been changed".format(name) + ret["comment"] = f"Parameters for database {name} have been changed" ret["changes"][name] = "Parameters changed" elif name in dbs: - ret["comment"] = "Failed to change parameters for database {}".format(name) + ret["comment"] = f"Failed to change parameters for database {name}" ret["result"] = False else: - ret["comment"] = "Failed to create database {}".format(name) + ret["comment"] = f"Failed to create database {name}" ret["result"] = False return ret @@ -217,13 +222,13 @@ def absent( if __salt__["postgres.db_exists"](name, **db_args): if __opts__["test"]: ret["result"] = None - ret["comment"] = "Database {} is set to be removed".format(name) + ret["comment"] = f"Database {name} is set to be removed" return ret if __salt__["postgres.db_remove"](name, **db_args): - ret["comment"] = "Database {} has been removed".format(name) + ret["comment"] = f"Database {name} has been removed" ret["changes"][name] = "Absent" return ret # fallback - ret["comment"] = "Database {} is not present, so it cannot be removed".format(name) + ret["comment"] = f"Database {name} is not present, so it cannot be removed" return ret From cf23adeac24a740c850fded812765378ee10b0c0 Mon Sep 17 00:00:00 2001 From: Max Arnold Date: Mon, 11 Dec 2023 08:44:29 +0700 Subject: [PATCH 35/35] Fix key_str usage example --- salt/wheel/key.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/salt/wheel/key.py b/salt/wheel/key.py index 44a1b7ab626..66bee797220 100644 --- a/salt/wheel/key.py +++ b/salt/wheel/key.py @@ -271,7 +271,7 @@ def key_str(match): .. code-block:: python - >>> wheel.cmd('key.key_str', ['minion1']) + >>> wheel.cmd('key.print', ['minion1']) {'minions': {'minion1': '-----BEGIN PUBLIC KEY-----\nMIIBIjANBgkqhkiG9w0B ... TWugEQpPt\niQIDAQAB\n-----END PUBLIC KEY-----'}}