From 92623aafff4e8788f81bcd4b3b16e29ec62d59db Mon Sep 17 00:00:00 2001 From: "Gary T. Giesen" Date: Fri, 10 Jul 2026 13:35:12 -0400 Subject: [PATCH 1/2] Fix spurious "Output was trimmed" message in archive.extracted archive.extracted appended ". Output was trimmed to {} number of lines" unconditionally whenever files were extracted, even though trimming only happens when trim_output is set. With the default trim_output=False this produced the misleading comment "Output was trimmed to False number of lines" while nothing was trimmed. Guard the append with the same trim_output check that gates the actual trimming so the message only appears when output was really trimmed. Fixes #59570 --- changelog/59570.fixed.md | 1 + salt/states/archive.py | 7 +- tests/pytests/unit/states/test_archive.py | 116 ++++++++++++++++++++++ 3 files changed, 121 insertions(+), 3 deletions(-) create mode 100644 changelog/59570.fixed.md diff --git a/changelog/59570.fixed.md b/changelog/59570.fixed.md new file mode 100644 index 000000000000..49ec605833c5 --- /dev/null +++ b/changelog/59570.fixed.md @@ -0,0 +1 @@ +Fixed archive.extracted appending "Output was trimmed to False number of lines" when trim_output was left at its default and no output was actually trimmed. The message is now only added when trimming really occurs. diff --git a/salt/states/archive.py b/salt/states/archive.py index e99556f1eaf6..cf5611f4110d 100644 --- a/salt/states/archive.py +++ b/salt/states/archive.py @@ -1646,9 +1646,10 @@ def extracted( name, ) _add_explanation(ret, source_hash_trigger, contents_missing) - ret["comment"] += ". Output was trimmed to {} number of lines".format( - trim_output - ) + if trim_output: + ret["comment"] += ". Output was trimmed to {} number of lines".format( + trim_output + ) ret["result"] = True else: diff --git a/tests/pytests/unit/states/test_archive.py b/tests/pytests/unit/states/test_archive.py index b5350bc2dba5..7759f5a76282 100644 --- a/tests/pytests/unit/states/test_archive.py +++ b/tests/pytests/unit/states/test_archive.py @@ -290,6 +290,122 @@ def test_tar_bsdtar_with_trim_output(): assert ret["comment"].endswith("Output was trimmed to 1 number of lines") +def test_tar_bsdtar_without_trim_output_59570(): + """ + Direct-altitude regression test for #59570. + + When extraction actually happens but trim_output is left at its default + (the ``trim_output=False`` parameter default), no output is trimmed, so + the "Output was trimmed to ... number of lines" message must NOT be + appended. Previously it was appended unconditionally, producing the + nonsensical "Output was trimmed to False number of lines". + """ + bsdtar = MagicMock(return_value="tar (bsdtar)") + source = "/tmp/foo.tar.gz" + mock_false = MagicMock(return_value=False) + mock_true = MagicMock(return_value=True) + state_single_mock = MagicMock(return_value={"local": {"result": True}}) + run_all = MagicMock( + return_value={"retcode": 0, "stdout": "stdout", "stderr": "stderr"} + ) + mock_source_list = MagicMock(return_value=(source, None)) + list_mock = MagicMock( + return_value={ + "dirs": [], + "files": ["stderr"], + "links": [], + "top_level_dirs": [], + "top_level_files": ["stderr"], + "top_level_links": [], + } + ) + isfile_mock = MagicMock(side_effect=_isfile_side_effect) + + with patch.dict( + archive.__salt__, + { + "cmd.run": bsdtar, + "file.directory_exists": mock_false, + "file.file_exists": mock_false, + "state.single": state_single_mock, + "file.makedirs": mock_true, + "cmd.run_all": run_all, + "archive.list": list_mock, + "file.source_list": mock_source_list, + }, + ), patch.dict(archive.__states__, {"file.directory": mock_true}), patch.object( + os.path, "isfile", isfile_mock + ), patch( + "salt.utils.path.which", MagicMock(return_value=True) + ): + # trim_output intentionally omitted -> uses the default (False) + ret = archive.extracted( + os.path.join(os.sep + "tmp", "out"), + source, + options="xvzf", + enforce_toplevel=False, + keep_source=True, + ) + assert ret["result"] is True + assert ret["changes"]["extracted_files"] == ["stderr"] + assert "Output was trimmed" not in ret["comment"] + + +def test_tar_bsdtar_with_trim_output_zero(): + """ + Peripheral coverage for #59570: an explicit falsy trim_output (0) means + "do not trim", so the trimmed-output message must also be suppressed. + """ + bsdtar = MagicMock(return_value="tar (bsdtar)") + source = "/tmp/foo.tar.gz" + mock_false = MagicMock(return_value=False) + mock_true = MagicMock(return_value=True) + state_single_mock = MagicMock(return_value={"local": {"result": True}}) + run_all = MagicMock( + return_value={"retcode": 0, "stdout": "stdout", "stderr": "stderr"} + ) + mock_source_list = MagicMock(return_value=(source, None)) + list_mock = MagicMock( + return_value={ + "dirs": [], + "files": ["stderr"], + "links": [], + "top_level_dirs": [], + "top_level_files": ["stderr"], + "top_level_links": [], + } + ) + isfile_mock = MagicMock(side_effect=_isfile_side_effect) + + with patch.dict( + archive.__salt__, + { + "cmd.run": bsdtar, + "file.directory_exists": mock_false, + "file.file_exists": mock_false, + "state.single": state_single_mock, + "file.makedirs": mock_true, + "cmd.run_all": run_all, + "archive.list": list_mock, + "file.source_list": mock_source_list, + }, + ), patch.dict(archive.__states__, {"file.directory": mock_true}), patch.object( + os.path, "isfile", isfile_mock + ), patch( + "salt.utils.path.which", MagicMock(return_value=True) + ): + ret = archive.extracted( + os.path.join(os.sep + "tmp", "out"), + source, + options="xvzf", + enforce_toplevel=False, + keep_source=True, + trim_output=0, + ) + assert ret["changes"]["extracted_files"] == ["stderr"] + assert "Output was trimmed" not in ret["comment"] + + def test_extracted_when_if_missing_path_exists(): """ When if_missing exists, we should exit without making any changes. From cf912304c2f5a4361dddcbfba8f2c4bd6241a123 Mon Sep 17 00:00:00 2001 From: "Gary T. Giesen" Date: Sun, 12 Jul 2026 01:32:40 -0400 Subject: [PATCH 2/2] Use an f-string for the trimmed-output message Per review on #69770. --- salt/states/archive.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/salt/states/archive.py b/salt/states/archive.py index cf5611f4110d..000737871ef4 100644 --- a/salt/states/archive.py +++ b/salt/states/archive.py @@ -1647,9 +1647,8 @@ def extracted( ) _add_explanation(ret, source_hash_trigger, contents_missing) if trim_output: - ret["comment"] += ". Output was trimmed to {} number of lines".format( - trim_output - ) + trim_msg = f". Output was trimmed to {trim_output} number of lines" + ret["comment"] += trim_msg ret["result"] = True else: