From 46766e34006a007b56eedcfdba49ee2b53dbcee8 Mon Sep 17 00:00:00 2001 From: David Murphy Date: Tue, 23 Jul 2024 09:41:11 -0600 Subject: [PATCH 1/2] Rework of accidently closed PR 65792 --- changelog/65027.fixed.md | 1 + salt/fileclient.py | 7 +- salt/minion.py | 19 +++++ salt/utils/extmods.py | 5 +- .../pytests/integration/cli/test_salt_call.py | 83 +++++++++++++++++++ 5 files changed, 112 insertions(+), 3 deletions(-) create mode 100644 changelog/65027.fixed.md diff --git a/changelog/65027.fixed.md b/changelog/65027.fixed.md new file mode 100644 index 000000000000..a2692b4a83db --- /dev/null +++ b/changelog/65027.fixed.md @@ -0,0 +1 @@ +Ensure sync from _grains occurs before attempting pillar compilation in case custom grain used in pillar file and salt-minion in masterless mode diff --git a/salt/fileclient.py b/salt/fileclient.py index d3694a316570..9054b8671521 100644 --- a/salt/fileclient.py +++ b/salt/fileclient.py @@ -46,12 +46,15 @@ MAX_FILENAME_LENGTH = 255 -def get_file_client(opts, pillar=False): +def get_file_client(opts, pillar=False, force_local=False): """ Read in the ``file_client`` option and return the correct type of file server """ - client = opts.get("file_client", "remote") + if force_local: + client = "local" + else: + client = opts.get("file_client", "remote") if pillar and client == "local": client = "pillar" diff --git a/salt/minion.py b/salt/minion.py index 67e2e34620fb..b722721433a0 100644 --- a/salt/minion.py +++ b/salt/minion.py @@ -114,6 +114,24 @@ # 6. Handle publications +def _sync_grains(opts): + # if local client (masterless minion), need sync of custom grains + # as they may be used in pillar compilation + # in addition, with masterless minion some opts may not be filled + # at this point of syncing,for example sometimes does not contain + # extmod_whitelist and extmod_blacklist hence set those to defaults, + # empty dict, if not part of opts, as ref'd in + # salt.utils.extmod sync function + if "local" == opts.get("file_client", "remote"): + if opts.get("extmod_whitelist", None) is None: + opts["extmod_whitelist"] = {} + + if opts.get("extmod_blacklist", None) is None: + opts["extmod_blacklist"] = {} + + salt.utils.extmods.sync(opts, "grains", force_local=True) + + def resolve_dns(opts, fallback=True): """ Resolves the master_ip and master_uri options @@ -926,6 +944,7 @@ def __init__(self, opts, context=None): # Late setup of the opts grains, so we can log from the grains module import salt.loader + _sync_grains(opts) opts["grains"] = salt.loader.grains(opts) super().__init__(opts) diff --git a/salt/utils/extmods.py b/salt/utils/extmods.py index f1b8a8264483..8601cfeedbc3 100644 --- a/salt/utils/extmods.py +++ b/salt/utils/extmods.py @@ -39,6 +39,7 @@ def sync( saltenv=None, extmod_whitelist=None, extmod_blacklist=None, + force_local=False, ): """ Sync custom modules into the extension_modules directory @@ -82,7 +83,9 @@ def sync( "Cannot create cache module directory %s. Check permissions.", mod_dir, ) - with salt.fileclient.get_file_client(opts) as fileclient: + with salt.fileclient.get_file_client( + opts, pillar=False, force_local=force_local + ) as fileclient: for sub_env in saltenv: log.info("Syncing %s for environment '%s'", form, sub_env) cache = [] diff --git a/tests/pytests/integration/cli/test_salt_call.py b/tests/pytests/integration/cli/test_salt_call.py index f927f499c858..de5a3a5cab48 100644 --- a/tests/pytests/integration/cli/test_salt_call.py +++ b/tests/pytests/integration/cli/test_salt_call.py @@ -531,3 +531,86 @@ def test_cve_2024_37088(salt_master_alt, salt_call_alt, caplog): assert ret.returncode == 1 assert ret.data is None assert "Got a bad pillar from master, type str, expecting dict" in caplog.text + + +def test_state_highstate_custom_grains_masterless_mode( + salt_master, salt_minion_factory +): + """ + This test ensure that custom grains in salt://_grains are loaded before pillar compilation + to ensure that any use of custom grains in pillar files are available when in masterless mode, + this implies that a sync of grains occurs before loading the regular + /etc/salt/grains or configuration file grains, as well as the usual grains. + Note: cannot use salt_minion and salt_call_cli, since these will be loaded before + the pillar and custom_grains files are written, hence using salt_minion_factory. + """ + pillar_top_sls = """ + base: + '*': + - defaults + """ + + pillar_defaults_sls = """ + mypillar: "{{ grains['custom_grain'] }}" + """ + + salt_top_sls = """ + base: + '*': + - test + """ + + salt_test_sls = """ + "donothing": + test.nop: [] + """ + + salt_custom_grains_py = """ + def main(): + return {'custom_grain': 'test_value'} + """ + + assert salt_master.is_running() + with salt_minion_factory.started(): + salt_minion = salt_minion_factory + salt_call_cli = salt_minion_factory.salt_call_cli() + with salt_minion.pillar_tree.base.temp_file( + "top.sls", pillar_top_sls + ), salt_minion.pillar_tree.base.temp_file( + "defaults.sls", pillar_defaults_sls + ), salt_minion.state_tree.base.temp_file( + "top.sls", salt_top_sls + ), salt_minion.state_tree.base.temp_file( + "test.sls", salt_test_sls + ), salt_minion.state_tree.base.temp_file( + "_grains/custom_grain.py", salt_custom_grains_py + ): + ## need to try masterless mode + ret = salt_call_cli.run("--local", "state.highstate") + assert ret.returncode == 0 + ret = salt_call_cli.run("pillar.items") + assert ret.returncode == 0 + assert ret.data + pillar_items = ret.data + assert "mypillar" in pillar_items + assert pillar_items["mypillar"] == "test_value" + + ## need to try with master mode + ret = salt_call_cli.run("state.highstate") + assert ret.returncode == 0 + ret = salt_call_cli.run("pillar.items") + assert ret.returncode == 0 + assert ret.data + pillar_items = ret.data + assert "mypillar" not in pillar_items + + +def test_salt_call_versions(salt_call_cli, caplog): + """ + Call test.versions without '--local' to test grains + are sync'd without any missing keys in opts + """ + with caplog.at_level(logging.DEBUG): + ret = salt_call_cli.run("test.versions") + assert ret.returncode == 0 + assert "Failed to sync grains module: 'master_uri'" not in caplog.messages From 0f2a8b40fac1744260aff9b3e3c78e560df3f9f4 Mon Sep 17 00:00:00 2001 From: David Murphy Date: Thu, 15 Aug 2024 13:23:13 -0600 Subject: [PATCH 2/2] Added debugging statements --- salt/fileclient.py | 11 ++++++++++- salt/minion.py | 2 ++ salt/utils/extmods.py | 7 +++++++ tests/pytests/integration/cli/test_salt_call.py | 7 +++++++ 4 files changed, 26 insertions(+), 1 deletion(-) diff --git a/salt/fileclient.py b/salt/fileclient.py index 9054b8671521..937d83114e12 100644 --- a/salt/fileclient.py +++ b/salt/fileclient.py @@ -51,6 +51,10 @@ def get_file_client(opts, pillar=False, force_local=False): Read in the ``file_client`` option and return the correct type of file server """ + print( + f"DGM get_file_client entry, opts '{opts}', pillar '{pillar}', force_local '{force_local}'", + flush=True, + ) if force_local: client = "local" else: @@ -58,9 +62,14 @@ def get_file_client(opts, pillar=False, force_local=False): if pillar and client == "local": client = "pillar" - return {"remote": RemoteClient, "local": FSClient, "pillar": PillarClient}.get( + # DGM return {"remote": RemoteClient, "local": FSClient, "pillar": PillarClient}.get( + # DGM client, RemoteClient + # DGM )(opts) + dgm_ret = {"remote": RemoteClient, "local": FSClient, "pillar": PillarClient}.get( client, RemoteClient )(opts) + print(f"DGM get_file_client exit, ret '{dgm_ret}'", flush=True) + return dgm_ret def decode_dict_keys_to_str(src): diff --git a/salt/minion.py b/salt/minion.py index b722721433a0..775bbfbcf07c 100644 --- a/salt/minion.py +++ b/salt/minion.py @@ -122,6 +122,8 @@ def _sync_grains(opts): # extmod_whitelist and extmod_blacklist hence set those to defaults, # empty dict, if not part of opts, as ref'd in # salt.utils.extmod sync function + dgm_fc_opts = opts.get("file_client", "remote") + print(f"DGM _sync_grains entry file_client settings '{dgm_fc_opts}'", flush=True) if "local" == opts.get("file_client", "remote"): if opts.get("extmod_whitelist", None) is None: opts["extmod_whitelist"] = {} diff --git a/salt/utils/extmods.py b/salt/utils/extmods.py index 8601cfeedbc3..9bb748f68428 100644 --- a/salt/utils/extmods.py +++ b/salt/utils/extmods.py @@ -44,6 +44,11 @@ def sync( """ Sync custom modules into the extension_modules directory """ + print( + f"DGM syc entry, opts '{opts}', form '{form}', saltenv '{saltenv}', extmod_whitelist '{extmod_whitelist}', extmod_blacklist '{extmod_blacklist}', force_local '{force_local}'", + flush=True, + ) + if saltenv is None: saltenv = ["base"] @@ -156,4 +161,6 @@ def sync( shutil.rmtree(emptydir, ignore_errors=True) except Exception as exc: # pylint: disable=broad-except log.error("Failed to sync %s module: %s", form, exc) + + print(f"DGM syc exit, ret '{ret}', touched '{touched}', opts '{opts}'", flush=True) return ret, touched diff --git a/tests/pytests/integration/cli/test_salt_call.py b/tests/pytests/integration/cli/test_salt_call.py index de5a3a5cab48..92d4056098e3 100644 --- a/tests/pytests/integration/cli/test_salt_call.py +++ b/tests/pytests/integration/cli/test_salt_call.py @@ -586,9 +586,16 @@ def main(): "_grains/custom_grain.py", salt_custom_grains_py ): ## need to try masterless mode + opts = salt_minion.config.copy() + opts["file_client"] = "local" + ret = salt_call_cli.run("--local", "state.highstate") assert ret.returncode == 0 ret = salt_call_cli.run("pillar.items") + print( + f"DGM test_state_highstate_custom_grains_masterless_mode, ret '{ret}'", + flush=True, + ) assert ret.returncode == 0 assert ret.data pillar_items = ret.data