diff --git a/changelog/68982.added.md b/changelog/68982.added.md new file mode 100644 index 000000000000..465609059cce --- /dev/null +++ b/changelog/68982.added.md @@ -0,0 +1,7 @@ +Added ``_color`` modifier for ``state_output``: setting ``state_output`` to +``full_color``, ``terse_color``, ``mixed_color``, ``changes_color``, or +``filter_color`` enables colorized unified diff output in the highstate +outputter. Added lines are green, removed lines are red, hunk headers +(``@@``) are cyan, file headers (``---`` / ``+++``) are bold red/green, and +context lines are gray. All other behavior is identical to the base mode +without ``_color``. diff --git a/doc/ref/cli/_includes/output-options.rst b/doc/ref/cli/_includes/output-options.rst index a70c48ac153a..3d3c0f1cd79a 100644 --- a/doc/ref/cli/_includes/output-options.rst +++ b/doc/ref/cli/_includes/output-options.rst @@ -44,8 +44,20 @@ Output Options .. option:: --state-output=STATE_OUTPUT, --state_output=STATE_OUTPUT Override the configured state_output value for minion - output. One of 'full', 'terse', 'mixed', 'changes' or - 'filter'. Default: 'none'. + output. One of ``full``, ``terse``, ``mixed``, ``changes`` or + ``filter``. Default: ``none``. + + Each mode accepts two optional suffixes: + + * ``_id`` — use the state ID as the display name instead of the state's + ``name`` value (e.g. ``full_id``). + * ``_color`` — colorize unified diffs in the changes section: added lines + green, removed lines red, hunk headers (``@@``) cyan, file headers + (``---`` / ``+++``) bold red/green, context lines gray (e.g. + ``full_color``). + + The two suffixes can be combined in either order, e.g. ``full_id_color`` + or ``full_color_id``. .. option:: --state-verbose=STATE_VERBOSE, --state_verbose=STATE_VERBOSE diff --git a/doc/ref/configuration/master.rst b/doc/ref/configuration/master.rst index 3cb473072f0c..b1d4844c6db8 100644 --- a/doc/ref/configuration/master.rst +++ b/doc/ref/configuration/master.rst @@ -3255,10 +3255,20 @@ The state_output setting controls which results will be output full multi line: ``full_id``, ``mixed_id``, ``changes_id`` and ``terse_id`` are also allowed; when set, the state ID will be used as name in the output. +Any of the above modes can be suffixed with ``_color`` (e.g. ``full_color``, +``mixed_color``) to enable colorized unified diff output in the changes +section. Added lines are shown in green, removed lines in red, hunk headers +in cyan, and context lines in gray. All other output behavior is identical to +the mode without the ``_color`` suffix. + .. code-block:: yaml state_output: full +.. code-block:: yaml + + state_output: full_color + .. conf_master:: state_output_diff ``state_output_diff`` diff --git a/doc/ref/configuration/minion.rst b/doc/ref/configuration/minion.rst index a4a9354697c3..74da50a0c5d4 100644 --- a/doc/ref/configuration/minion.rst +++ b/doc/ref/configuration/minion.rst @@ -2408,10 +2408,20 @@ The state_output setting controls which results will be output full multi line: ``full_id``, ``mixed_id``, ``changes_id`` and ``terse_id`` are also allowed; when set, the state ID will be used as name in the output. +Any of the above modes can be suffixed with ``_color`` (e.g. ``full_color``, +``mixed_color``) to enable colorized unified diff output in the changes +section. Added lines are shown in green, removed lines in red, hunk headers +in cyan, and context lines in gray. All other output behavior is identical to +the mode without the ``_color`` suffix. + .. code-block:: yaml state_output: full +.. code-block:: yaml + + state_output: full_color + .. conf_minion:: state_output_diff ``state_output_diff`` diff --git a/salt/output/highstate.py b/salt/output/highstate.py index dc00885753fa..69fb4ecc95df 100644 --- a/salt/output/highstate.py +++ b/salt/output/highstate.py @@ -44,13 +44,23 @@ These can be set as such from the command line, or in the Salt config as `state_output_exclude` or `state_output_terse`, respectively. - The output modes have one modifier: + The output modes have two modifiers that can be combined: ``full_id``, ``terse_id``, ``mixed_id``, ``changes_id`` and ``filter_id`` If ``_id`` is used, then the corresponding form will be used, but the value for ``name`` will be drawn from the state ID. This is useful for cases where the name value might be very long and hard to read. + ``full_color``, ``terse_color``, ``mixed_color``, ``changes_color`` and ``filter_color`` + If ``_color`` is used, unified diffs in the changes section will be + colorized: added lines in green, removed lines in red, hunk headers + (``@@``) in cyan, file headers (``---`` / ``+++``) in bold red/green, and + context lines in gray. All other output behavior is identical to the base + mode. + + The ``_id`` and ``_color`` modifiers can be combined, e.g. ``full_id_color`` + or ``full_color_id``. + state_tabular: If `state_output` uses the terse output, set this to `True` for an aligned output format. If you wish to use a custom format, this can be set to a @@ -136,6 +146,16 @@ log = logging.getLogger(__name__) +# Placeholder injected in place of each diff string before the nested outputter +# runs, then swapped back for a colorized diff. ``{}`` is filled with a unique +# index; the regex below matches every such placeholder in a single pass, +# capturing the leading indent (group 1) and the sentinel id (group 2). +_COLORDIFF_SENTINEL = "__COLORDIFF_{}__" +_COLORDIFF_RE = re.compile( + r"^( *)(?:\x1b\[[0-9;]*m)*(__COLORDIFF_\d+__)(?:\x1b\[[0-9;]*m)*$", + re.MULTILINE, +) + def _compress_ids(data): """ @@ -340,9 +360,15 @@ def _format_host(host, data, indent_level=1): """ host = salt.utils.data.decode(host) - colors = salt.utils.color.get_colors( - __opts__.get("color"), __opts__.get("color_theme") - ) + # There is no ``color`` key in the config defaults; ``get_printout`` + # resolves it to True/False (default True) before the outputter normally + # runs. On a direct call that resolution is skipped, so an unset value can + # reach here as None. Fall back to Salt's default of True so it matches + # the ``color is False`` check used in ``_render_diff``. + color_opt = __opts__.get("color") + if color_opt is None: + color_opt = True + colors = salt.utils.color.get_colors(color_opt, __opts__.get("color_theme")) tabular = __opts__.get("state_tabular", False) rcounts = {} rdurations = [] @@ -445,7 +471,7 @@ def _format_host(host, data, indent_level=1): schanged = True nchanges += 1 else: - schanged, ctext = _format_changes(ret["changes"]) + schanged, ctext = _format_changes(ret["changes"], colors) # if compressed, the changes are keyed by name if schanged and compressed_count > 1: nchanges += len(ret["changes"].get("compressed changes", {})) or 1 @@ -479,6 +505,9 @@ def _format_host(host, data, indent_level=1): tcolor = colors["LIGHT_YELLOW"] state_output = __opts__.get("state_output", "full").lower() + # Strip the _color modifier before all mode comparisons so that + # e.g. "full_color" behaves identically to "full" for layout purposes. + state_output = state_output.replace("_color", "") comps = tname.split("_|-") if state_output.endswith("_id"): @@ -742,22 +771,136 @@ def _counts(label, count): return "\n".join(hstrs), nchanges > 0 +def _render_diff(diff_str, indent, colors): + """ + Render a unified diff string with per-line ANSI colorization. + + Each line is colored according to its unified-diff role: + ``---`` (from-file header) -> LIGHT_RED (bold) + ``+++`` (to-file header) -> LIGHT_GREEN (bold) + ``@@`` (hunk header) -> CYAN + ``+`` (added line) -> GREEN + ``-`` (removed line) -> RED + context lines (leading space) -> LIGHT_GRAY + + The ``indent`` argument (an integer) is prepended as spaces to every line, + matching the nesting depth used by the surrounding nested outputter output. + The ``colors`` dict must be pre-built by the caller (e.g. from + ``salt.utils.color.get_colors``). Embedded terminal escape sequences in + the (untrusted) diff content are neutralized when ``strip_colors`` is set, + matching the ``nested`` outputter. + """ + prefix = " " * indent + + # Diff content is untrusted; neutralize any embedded escape sequences up + # front (ESC is not a line boundary, so this is equivalent to stripping + # each line) before we add the colorization escape codes. + if __opts__.get("strip_colors", True): + diff_str = salt.output.strip_esc_sequence(diff_str) + + if __opts__.get("color") is False: + return "\n".join(prefix + line for line in diff_str.splitlines()) + + GREEN = str(colors["GREEN"]) + ENDC = str(colors["ENDC"]) + RED = str(colors["RED"]) + CYAN = str(colors["CYAN"]) + WHITE = str(colors["LIGHT_GRAY"]) + LIGHT_RED = str(colors["LIGHT_RED"]) + LIGHT_GREEN = str(colors["LIGHT_GREEN"]) + + result = [] + for line in diff_str.splitlines(): + if line.startswith("---"): + color = LIGHT_RED + elif line.startswith("+++"): + color = LIGHT_GREEN + elif line.startswith("@@"): + color = CYAN + elif line.startswith("+"): + color = GREEN + elif line.startswith("-"): + color = RED + else: + color = WHITE + result.append(f"{prefix}{color}{line}{ENDC}") + return "\n".join(result) + + def _nested_changes(changes): """ - Print the changes data using the nested outputter + Print the changes data using the nested outputter. """ ret = "\n" ret += salt.output.out_format(changes, "nested", __opts__, nested_indent=14) return ret -def _format_changes(changes, orchestration=False): +def _nested_changes_colorized(changes, colors): + """ + Print the changes data with diff colorization (used when state_output + contains the ``_color`` modifier, e.g. ``full_color``). + + Replaces every ``diff`` string value at any nesting depth with a unique + sentinel, delegates all structural formatting to the native nested + outputter, then swaps each sentinel line back for a per-line colorized + diff. Note: ``colors`` must be pre-built by the caller. + """ + sentinels = {} # sentinel string -> raw diff string + + def _replace_diffs(obj): + if isinstance(obj, dict): + return { + k: ( + _assign_sentinel(v) + if k == "diff" and isinstance(v, str) + else _replace_diffs(v) + ) + for k, v in obj.items() + } + if isinstance(obj, list): + return [_replace_diffs(i) for i in obj] + return obj + + def _assign_sentinel(diff_str): + sentinel = _COLORDIFF_SENTINEL.format(len(sentinels)) + sentinels[sentinel] = diff_str + return sentinel + + nested_output = salt.output.out_format( + _replace_diffs(changes), "nested", __opts__, nested_indent=14 + ) + + if not sentinels: + return "\n" + nested_output + + # The nested outputter renders each sentinel as a single indented line + # (possibly wrapped in ANSI codes). ``_COLORDIFF_RE`` matches every + # sentinel in a single pass: group 1 is the runtime indent, group 2 the + # sentinel id used to look up its raw diff string. + def _replacer(match): + indent = len(match.group(1)) + diff_str = sentinels.get(match.group(2), "") + return _render_diff(diff_str, indent, colors) + + nested_output = _COLORDIFF_RE.sub(_replacer, nested_output) + + return "\n" + nested_output + + +def _format_changes(changes, colors, orchestration=False): """ - Format the changes dict based on what the data is + Format the changes dict based on what the data is. + + ``colors`` is the pre-built ANSI color mapping from the calling + ``_format_host`` invocation; passing it in avoids re-initializing the + color engine on every state block. """ if not changes: return False, "" + colorize = "_color" in __opts__.get("state_output", "").lower() + if orchestration: return True, _nested_changes(changes) @@ -774,7 +917,10 @@ def _format_changes(changes, orchestration=False): changed = changed or c else: changed = True - ctext = _nested_changes(changes) + if colorize: + ctext = _nested_changes_colorized(changes, colors) + else: + ctext = _nested_changes(changes) return changed, ctext diff --git a/tests/pytests/unit/output/test_highstate.py b/tests/pytests/unit/output/test_highstate.py index 7661a17aa350..0bd25ce716ac 100644 --- a/tests/pytests/unit/output/test_highstate.py +++ b/tests/pytests/unit/output/test_highstate.py @@ -1,5 +1,6 @@ import copy import logging +import re import pytest @@ -917,3 +918,271 @@ def test_nested_output(): assert " Succeeded: 2 (changed=1)" in ret assert " Failed: 0" in ret assert " Total states run: 2" in ret + + +# --------------------------------------------------------------------------- +# Tests for diff colorization +# --------------------------------------------------------------------------- + +# ANSI color codes used by salt.utils.color when color=True +_GREEN = "\x1b[0;32m" +_RED = "\x1b[0;31m" +_CYAN = "\x1b[0;36m" +_WHITE = "\x1b[0;37m" +_LIGHT_RED = "\x1b[0;1;31m" +_LIGHT_GREEN = "\x1b[0;1;32m" +_ENDC = "\x1b[0;0m" + + +def _strip_ansi(text): + """Remove all ANSI escape sequences from *text*.""" + return re.sub(r"\x1b\[[0-9;]+m", "", text) + + +_MOTD_STATE = { + "minion": { + "file_|-/etc/motd_|-/etc/motd_|-managed": { + "__id__": "/etc/motd", + "__run_num__": 0, + "__sls__": "motd", + "changes": { + "diff": ( + "--- /etc/motd\n" + "+++ /etc/motd\n" + "@@ -1,2 +1,2 @@\n" + " unchanged\n" + "-old line\n" + "+new line\n" + ) + }, + "comment": "File /etc/motd updated", + "duration": 10.0, + "name": "/etc/motd", + "result": True, + "start_time": "10:00:00.000000", + }, + } +} + + +def _assert_diff_colorized(rendered): + assert _RED in rendered + assert _GREEN in rendered + assert "-old line" in _strip_ansi(rendered) + assert "+new line" in _strip_ansi(rendered) + for line in rendered.splitlines(): + plain = _strip_ansi(line) + if "-old line" in plain or "+new line" in plain: + assert plain.startswith( + " " * 18 + ), f"Expected 18-space indent: {repr(plain)}" + # Diff file headers use the bold variants: --- (from) red, +++ (to) + # green. A real header is the marker followed by a space + path + # ("--- /path"), which distinguishes it from the outputter's own + # "----------" separator lines. + if plain.lstrip().startswith("--- "): + assert _LIGHT_RED in line + if plain.lstrip().startswith("+++ "): + assert _LIGHT_GREEN in line + + +def test_diff_in_full_color_output(minion_opts): + """file.managed diff has red removed and green added lines with full_color mode.""" + minion_opts.update( + { + "color": True, + "color_theme": None, + "state_verbose": True, + "state_output": "full_color", + } + ) + with patch.dict(highstate.__opts__, minion_opts): + _assert_diff_colorized(highstate.output(_MOTD_STATE)) + + +def test_diff_in_full_color_output_color_default(minion_opts): + """With color=None (default), full_color mode still colorizes diff lines.""" + minion_opts.update( + { + "color": None, + "color_theme": None, + "state_verbose": True, + "state_output": "full_color", + } + ) + with patch.dict(highstate.__opts__, minion_opts): + _assert_diff_colorized(highstate.output(_MOTD_STATE)) + + +def test_diff_not_colorized_in_plain_full_output(minion_opts): + """With state_output=full (no _color), diff is not colorized.""" + minion_opts.update( + { + "color": True, + "color_theme": None, + "state_verbose": True, + "state_output": "full", + } + ) + with patch.dict(highstate.__opts__, minion_opts): + rendered = highstate.output(_MOTD_STATE) + assert _RED not in rendered + + +@pytest.mark.parametrize("state_output", ["changes_color", "filter_color"]) +def test_diff_colorized_for_all_color_variants(minion_opts, state_output): + """Any XXX_color state_output variant colorizes the diff block.""" + minion_opts.update( + { + "color": True, + "color_theme": None, + "state_verbose": True, + "state_output": state_output, + } + ) + with patch.dict(highstate.__opts__, minion_opts): + _assert_diff_colorized(highstate.output(_MOTD_STATE)) + + +def test_diff_colorized_in_mixed_color_on_failure(minion_opts): + """mixed_color only shows full output for failures; a failed state's diff is colorized.""" + state_data = { + "minion": { + "file_|-/etc/motd_|-/etc/motd_|-managed": { + "__id__": "/etc/motd", + "__run_num__": 0, + "__sls__": "motd", + "changes": { + "diff": ( + "--- /etc/motd\n" + "+++ /etc/motd\n" + "@@ -1,2 +1,2 @@\n" + " unchanged\n" + "-old line\n" + "+new line\n" + ) + }, + "comment": "File /etc/motd failed to update", + "duration": 10.0, + "name": "/etc/motd", + "result": False, + "start_time": "10:00:00.000000", + }, + } + } + minion_opts.update( + { + "color": True, + "color_theme": None, + "state_verbose": True, + "state_output": "mixed_color", + } + ) + with patch.dict(highstate.__opts__, minion_opts): + rendered = highstate.output(state_data) + assert _RED in rendered + assert "-old line" in _strip_ansi(rendered) + + +def test_nested_diff_colorized_in_file_recurse(minion_opts): + """file.recurse changes nest diff under a file-path key; diff must still be colorized.""" + state_data = { + "minion": { + "file_|-/etc/test_|-/etc/test_|-recurse": { + "__id__": "recurse_test", + "__run_num__": 0, + "__sls__": "test", + "changes": { + "/etc/test/file.txt": { + "diff": ( + "--- /etc/test/file.txt\n" + "+++ /etc/test/file.txt\n" + "@@ -1,2 +1,2 @@\n" + " unchanged\n" + "-old line\n" + "+new line\n" + ) + }, + }, + "comment": "changes incoming", + "duration": 10.0, + "name": "/etc/test", + "result": None, + "start_time": "10:00:00.000000", + }, + } + } + minion_opts.update( + { + "color": True, + "color_theme": None, + "state_verbose": True, + "state_output": "full_color", + } + ) + with patch.dict(highstate.__opts__, minion_opts): + rendered = highstate.output(state_data) + assert _RED in rendered + assert _GREEN in rendered + assert "-old line" in _strip_ansi(rendered) + assert "+new line" in _strip_ansi(rendered) + + +def test_diff_not_colorized_without_color_modifier(minion_opts): + """Default state_output=full produces no diff colorization.""" + minion_opts.update({"color": True, "color_theme": None, "state_verbose": True}) + minion_opts.pop("state_output", None) + with patch.dict(highstate.__opts__, minion_opts): + rendered = highstate.output(_MOTD_STATE) + assert _RED not in rendered + + +def test_colorized_diff_strips_embedded_escape_sequences(minion_opts): + """ + Diff content is untrusted; embedded terminal escape sequences must be + neutralized in the colorized path just as the nested outputter does for + the plain path (strip_colors defaults to True). + """ + # ESC ]0;injected_title BEL is an OSC that would set the terminal title if + # printed. + injected = "\x1b]0;injected_title\x07" + state_data = { + "minion": { + "file_|-/etc/motd_|-/etc/motd_|-managed": { + "__id__": "/etc/motd", + "__run_num__": 0, + "__sls__": "motd", + "changes": { + "diff": ( + "--- /etc/motd\n" + "+++ /etc/motd\n" + "@@ -1,2 +1,2 @@\n" + " unchanged\n" + "-old line\n" + f"+new line{injected}\n" + ) + }, + "comment": "File /etc/motd updated", + "duration": 10.0, + "name": "/etc/motd", + "result": True, + "start_time": "10:00:00.000000", + }, + } + } + minion_opts.update( + { + "color": True, + "color_theme": None, + "state_verbose": True, + "strip_colors": True, + "state_output": "full_color", + } + ) + with patch.dict(highstate.__opts__, minion_opts): + rendered = highstate.output(state_data) + # The raw OSC injection must not survive into the output... + assert injected not in rendered + # ...but the visible text and colorization are preserved. + assert _GREEN in rendered + assert "+new line" in _strip_ansi(rendered)