From e0e65465860defd0eec7b58b987e59fb59d74f64 Mon Sep 17 00:00:00 2001 From: MKLeb Date: Thu, 12 Oct 2023 19:52:31 -0400 Subject: [PATCH] Add more coverage for the saltfile mixin and ensure passed CLI options take priority --- changelog/65358.fixed.md | 1 + salt/utils/parsers.py | 2 + .../unit/utils/parsers/test_saltfile_mixin.py | 141 ++++++++++++++---- 3 files changed, 119 insertions(+), 25 deletions(-) create mode 100644 changelog/65358.fixed.md diff --git a/changelog/65358.fixed.md b/changelog/65358.fixed.md new file mode 100644 index 00000000000..9a9acc31b4d --- /dev/null +++ b/changelog/65358.fixed.md @@ -0,0 +1 @@ +Ensure CLI options take priority over Saltfile options diff --git a/salt/utils/parsers.py b/salt/utils/parsers.py index 06858c6122f..f3ba1948d89 100644 --- a/salt/utils/parsers.py +++ b/salt/utils/parsers.py @@ -454,6 +454,7 @@ class SaltfileMixIn(metaclass=MixInMeta): if value != default: # The user passed an argument, we won't override it with the # one from Saltfile, if any + cli_config.pop(option.dest) continue # We reached this far! Set the Saltfile value on the option @@ -477,6 +478,7 @@ class SaltfileMixIn(metaclass=MixInMeta): if value != default: # The user passed an argument, we won't override it with # the one from Saltfile, if any + cli_config.pop(option.dest) continue setattr(self.options, option.dest, cli_config[option.dest]) diff --git a/tests/pytests/unit/utils/parsers/test_saltfile_mixin.py b/tests/pytests/unit/utils/parsers/test_saltfile_mixin.py index 5ea20aad5ed..fa99f26c081 100644 --- a/tests/pytests/unit/utils/parsers/test_saltfile_mixin.py +++ b/tests/pytests/unit/utils/parsers/test_saltfile_mixin.py @@ -7,6 +7,7 @@ import shutil import pytest +import salt.exceptions import salt.utils.parsers from tests.support.helpers import patched_environ from tests.support.mock import patch @@ -52,6 +53,11 @@ class MockSaltfileParser( default=None, help="Write the output to the specified file.", ) + group.add_option( + "--version-arg", + action="version", + help="Option to test no dest", + ) @pytest.fixture @@ -59,14 +65,6 @@ def parser(): return MockSaltfileParser() -# @pytest.fixture -# def parser(): -# # Mock this because we don't need it and it causes an error -# # if there is more than one test being run in this file -# with patch.object(salt.utils.parsers.LogLevelMixIn, "_LogLevelMixIn__setup_logging_routines"): -# yield salt.utils.parsers.SaltCallOptionParser() - - @pytest.fixture def saltfile(tmp_path): fp = tmp_path / "Saltfile" @@ -74,42 +72,53 @@ def saltfile(tmp_path): return fp -@pytest.fixture -def base_opts(): - # return ["--local", "test.ping"] - return [] - - -def test_saltfile_in_environment(parser, saltfile, base_opts): +def test_saltfile_in_environment(parser, saltfile): """ Test setting the SALT_SALTFILE environment variable """ with patched_environ(SALT_SALTFILE=str(saltfile)): - parser.parse_args(base_opts) + parser.parse_args([]) assert parser.options.saltfile == str(saltfile) -def test_saltfile_option(parser, saltfile, base_opts): +def test_saltfile_option(parser, saltfile): """ - Test setting the SALT_SALTFILE environment variable + Test setting the saltfile via the CLI """ - parser.parse_args(base_opts + ["--saltfile", str(saltfile)]) + parser.parse_args(["--saltfile", str(saltfile)]) assert parser.options.saltfile == str(saltfile) -def test_saltfile_cwd(parser, saltfile, base_opts, tmp_path): +def test_bad_saltfile_option(parser, saltfile, tmp_path): """ - Test setting the SALT_SALTFILE environment variable + Test setting a bad saltfile via the CLI + """ + with pytest.raises(SystemExit): + parser.parse_args(["--saltfile", str(tmp_path / "fake_dir")]) + + +def test_saltfile_cwd(parser, saltfile, tmp_path): + """ + Test using a saltfile in the cwd """ with patch("os.getcwd", return_value=str(tmp_path)) as cwd_mock: - parser.parse_args(base_opts) + parser.parse_args([]) assert parser.options.saltfile == str(saltfile) cwd_mock.assert_called_once() -def test_saltfile_user_home(parser, saltfile, base_opts, tmp_path): +def test_saltfile_cwd_doesnt_exist(parser, saltfile, tmp_path): """ - Test setting the SALT_SALTFILE environment variable + Test using a saltfile in the cwd that doesn't exist + """ + with patch("os.getcwd", return_value=str(tmp_path / "fake_dir")) as cwd_mock: + parser.parse_args([]) + assert parser.options.saltfile is None + + +def test_saltfile_user_home(parser, saltfile, tmp_path): + """ + Test using a saltfile in ~/.salt/ """ fake_dir = tmp_path / "fake_dir" fake_dir.mkdir() @@ -119,7 +128,89 @@ def test_saltfile_user_home(parser, saltfile, base_opts, tmp_path): salt_subdir.mkdir() dest = str(salt_subdir / "Saltfile") shutil.copy(str(saltfile), dest) - parser.parse_args(base_opts) + parser.parse_args([]) assert parser.options.saltfile == dest cwd_mock.assert_called_once() eu_mock.assert_called_with("~") + + +def test_bad_saltfile(parser, saltfile): + """ + Test a saltfile with bad configuration + """ + contents = """ + bad "yaml": + - this is: bad yaml + - bad yaml=data: + - {"bad": yaml, "data": "yaml"} + """ + saltfile.write_text(contents) + # It raises two errors, let's catch them both + with pytest.raises(SystemExit): + with pytest.raises(salt.exceptions.SaltConfigurationError): + parser.parse_args(["--saltfile", str(saltfile)]) + + +def test_saltfile_without_prog_name(parser, saltfile): + """ + Test a saltfile with valid yaml but without the program name in it + """ + contents = "good: yaml" + saltfile.write_text(contents) + # This should just run cleanly + parser.parse_args(["--saltfile", str(saltfile)]) + + +def test_saltfile(parser, saltfile): + """ + Test a valid saltfile + """ + contents = """ + __main__.py: + log_level: debug + output: json + """ + saltfile.write_text(contents) + parser.parse_args(["--saltfile", str(saltfile)]) + print(parser.option_list) + assert parser.options.log_level == "debug" + assert parser.options.output == "json" + + +def test_saltfile_unusual_option(parser, saltfile): + """ + Test a valid saltfile + """ + contents = """ + __main__.py: + go: birds + """ + saltfile.write_text(contents) + parser.parse_args(["--saltfile", str(saltfile)]) + assert parser.options.go == "birds" + + +def test_saltfile_cli_override(parser, saltfile): + """ + Test a valid saltfile + """ + contents = """ + __main__.py: + log_level: debug + output: json + output_file: /fake/file + """ + saltfile.write_text(contents) + parser.parse_args( + [ + "--saltfile", + str(saltfile), + "--log-level", + "info", + "--out-file", + "/still/fake/file", + ] + ) + assert parser.options.log_level == "info" + assert parser.options.output == "json" + assert parser.options.output_file == "/still/fake/file"