From eafe5fc7393e4c69d987589473cc1c8a4ef56f2c Mon Sep 17 00:00:00 2001 From: Matt Clay Date: Fri, 30 May 2025 11:36:21 -0700 Subject: [PATCH] Add missing warning methods and args (#85225) --- .../fragments/module_utils_warnings.yml | 5 +++ .../_protomatter/plugins/filter/unmask.py | 2 +- lib/ansible/executor/task_executor.py | 2 +- lib/ansible/module_utils/_internal/_errors.py | 2 + lib/ansible/module_utils/basic.py | 32 +++++++++++++-- lib/ansible/module_utils/common/warnings.py | 41 ++++++++++++++++++- lib/ansible/utils/display.py | 2 +- test/units/conftest.py | 2 + .../common/warnings/test_deprecate.py | 30 ++++++++++++-- .../common/warnings/test_error_as_warning.py | 34 +++++++++++++++ .../module_utils/common/warnings/test_warn.py | 16 +++++++- test/units/utils/display/test_warning.py | 23 +++++++++++ 12 files changed, 179 insertions(+), 12 deletions(-) create mode 100644 changelogs/fragments/module_utils_warnings.yml create mode 100644 test/units/module_utils/common/warnings/test_error_as_warning.py diff --git a/changelogs/fragments/module_utils_warnings.yml b/changelogs/fragments/module_utils_warnings.yml new file mode 100644 index 00000000000..0679481448b --- /dev/null +++ b/changelogs/fragments/module_utils_warnings.yml @@ -0,0 +1,5 @@ +minor_changes: + - module_utils - Add optional ``help_text`` argument to ``AnsibleModule.warn``. + - module_utils - Add ``AnsibleModule.error_as_warning``. + - module_utils - Add ``ansible.module_utils.common.warnings.error_as_warning``. + - display - Add ``help_text`` and ``obj`` to ``Display.error_as_warning``. diff --git a/lib/ansible/_internal/ansible_collections/ansible/_protomatter/plugins/filter/unmask.py b/lib/ansible/_internal/ansible_collections/ansible/_protomatter/plugins/filter/unmask.py index 8a07bc79393..076a6ff7499 100644 --- a/lib/ansible/_internal/ansible_collections/ansible/_protomatter/plugins/filter/unmask.py +++ b/lib/ansible/_internal/ansible_collections/ansible/_protomatter/plugins/filter/unmask.py @@ -12,7 +12,7 @@ from ansible.errors import AnsibleError def unmask(value: object, type_names: str | list[str]) -> object: """ - Internal filter to suppress automatic type transformation in Jinja (e.g., WarningMessageDetail, DeprecationMessageDetail, ErrorDetail). + Internal filter to suppress automatic type transformation in Jinja (e.g., WarningSummary, DeprecationSummary, ErrorSummary). Lazy collection caching is in play - the first attempt to access a value in a given lazy container must be with unmasking in place, or the transformed value will already be cached. """ diff --git a/lib/ansible/executor/task_executor.py b/lib/ansible/executor/task_executor.py index 3055fe809ab..9504aa815fd 100644 --- a/lib/ansible/executor/task_executor.py +++ b/lib/ansible/executor/task_executor.py @@ -806,7 +806,7 @@ class TaskExecutor: if isinstance(deprecations, list): for deprecation in deprecations: if not isinstance(deprecation, _messages.DeprecationSummary): - # translate non-DeprecationMessageDetail message dicts + # translate non-DeprecationSummary message dicts try: if (collection_name := deprecation.pop('collection_name', ...)) is not ...: # deprecated: description='enable the deprecation message for collection_name' core_version='2.23' diff --git a/lib/ansible/module_utils/_internal/_errors.py b/lib/ansible/module_utils/_internal/_errors.py index 294a74210f0..48a5dcadde1 100644 --- a/lib/ansible/module_utils/_internal/_errors.py +++ b/lib/ansible/module_utils/_internal/_errors.py @@ -13,6 +13,8 @@ from . import _messages MSG_REASON_DIRECT_CAUSE: _t.Final[str] = '<<< caused by >>>' MSG_REASON_HANDLING_CAUSE: _t.Final[str] = '<<< while handling >>>' +TRACEBACK_REASON_EXCEPTION_DIRECT_WARNING: _t.Final[str] = 'The above exception was the direct cause of the following warning:' + class EventFactory: """Factory for creating `Event` instances from `BaseException` instances on targets.""" diff --git a/lib/ansible/module_utils/basic.py b/lib/ansible/module_utils/basic.py index 2d9688d6a79..655be7784ef 100644 --- a/lib/ansible/module_utils/basic.py +++ b/lib/ansible/module_utils/basic.py @@ -164,6 +164,7 @@ from ansible.module_utils.common._utils import get_all_subclasses as _get_all_su from ansible.module_utils.parsing.convert_bool import BOOLEANS, BOOLEANS_FALSE, BOOLEANS_TRUE, boolean from ansible.module_utils.common.warnings import ( deprecate, + error_as_warning, get_deprecations, get_warnings, warn, @@ -504,9 +505,34 @@ class AnsibleModule(object): return self._tmpdir - def warn(self, warning): - warn(warning) - self.log('[WARNING] %s' % warning) + def warn( + self, + warning: str, + *, + help_text: str | None = None, + ) -> None: + _skip_stackwalk = True + + warn( + warning=warning, + help_text=help_text, + ) + + def error_as_warning( + self, + msg: str | None, + exception: BaseException, + *, + help_text: str | None = None, + ) -> None: + """Display an exception as a warning.""" + _skip_stackwalk = True + + error_as_warning( + msg=msg, + exception=exception, + help_text=help_text, + ) def deprecate( self, diff --git a/lib/ansible/module_utils/common/warnings.py b/lib/ansible/module_utils/common/warnings.py index 1352f9189c8..f4a6406e4a9 100644 --- a/lib/ansible/module_utils/common/warnings.py +++ b/lib/ansible/module_utils/common/warnings.py @@ -6,7 +6,7 @@ from __future__ import annotations as _annotations import typing as _t -from ansible.module_utils._internal import _traceback, _deprecator, _event_utils, _messages +from ansible.module_utils._internal import _traceback, _deprecator, _event_utils, _messages, _errors from ansible.module_utils import _internal @@ -40,6 +40,45 @@ def warn( _global_warnings[warning] = None +def error_as_warning( + msg: str | None, + exception: BaseException, + *, + help_text: str | None = None, + obj: object = None, +) -> None: + """Display an exception as a warning.""" + _skip_stackwalk = True + + if _internal.is_controller: + _display = _internal.import_controller_module('ansible.utils.display').Display() + _display.error_as_warning( + msg=msg, + exception=exception, + help_text=help_text, + obj=obj, + ) + + return + + event = _errors.EventFactory.from_exception(exception, _traceback.is_traceback_enabled(_traceback.TracebackEvent.WARNING)) + + warning = _messages.WarningSummary( + event=_messages.Event( + msg=msg, + help_text=help_text, + formatted_traceback=_traceback.maybe_capture_traceback(msg, _traceback.TracebackEvent.WARNING), + chain=_messages.EventChain( + msg_reason=_errors.MSG_REASON_DIRECT_CAUSE, + traceback_reason=_errors.TRACEBACK_REASON_EXCEPTION_DIRECT_WARNING, + event=event, + ), + ), + ) + + _global_warnings[warning] = None + + def deprecate( msg: str, version: str | None = None, diff --git a/lib/ansible/utils/display.py b/lib/ansible/utils/display.py index 086101bdee1..dbd9d2e4160 100644 --- a/lib/ansible/utils/display.py +++ b/lib/ansible/utils/display.py @@ -903,7 +903,7 @@ class Display(metaclass=Singleton): formatted_traceback=_traceback.maybe_capture_traceback(msg, _traceback.TracebackEvent.WARNING), chain=_messages.EventChain( msg_reason=_errors.MSG_REASON_DIRECT_CAUSE, - traceback_reason="The above exception was the direct cause of the following warning:", + traceback_reason=_errors.TRACEBACK_REASON_EXCEPTION_DIRECT_WARNING, event=event, ), ) diff --git a/test/units/conftest.py b/test/units/conftest.py index fa07c76f2a5..35fac71d2b9 100644 --- a/test/units/conftest.py +++ b/test/units/conftest.py @@ -27,6 +27,7 @@ else: from .controller_only_conftest import * # pylint: disable=wildcard-import,unused-wildcard-import from ansible.module_utils import _internal as _module_utils_internal +from ansible.module_utils._internal import _traceback as _module_utils_internal_traceback def pytest_configure(config: pytest.Config): @@ -83,3 +84,4 @@ def pytest_collection_finish(session: pytest.Session): def as_target(mocker: pytest_mock.MockerFixture) -> None: """Force execution in the context of a target host instead of the controller.""" mocker.patch.object(_module_utils_internal, 'is_controller', False) + mocker.patch.object(_module_utils_internal_traceback, '_is_traceback_enabled', _module_utils_internal_traceback._is_module_traceback_enabled) diff --git a/test/units/module_utils/common/warnings/test_deprecate.py b/test/units/module_utils/common/warnings/test_deprecate.py index 8566bec7880..ba900a2220b 100644 --- a/test/units/module_utils/common/warnings/test_deprecate.py +++ b/test/units/module_utils/common/warnings/test_deprecate.py @@ -10,19 +10,43 @@ import typing as t import pytest -from ansible.module_utils._internal import _traceback +from ansible.module_utils._internal import _traceback, _messages, _deprecator +from ansible.module_utils.basic import AnsibleModule from ansible.module_utils.common import warnings from ansible.module_utils.common.warnings import deprecate +from ansible.module_utils.testing import patch_module_args from units.mock.module import ModuleEnvMocker -pytestmark = pytest.mark.usefixtures("module_env_mocker") +pytestmark = pytest.mark.usefixtures("as_target", "module_env_mocker") + + +def test_deprecate() -> None: + deprecate('Deprecation message') + assert warnings.get_deprecation_messages() == (dict(msg='Deprecation message', collection_name=None, version=None),) + assert warnings.get_deprecations() == [_messages.DeprecationSummary( + event=_messages.Event(msg='Deprecation message'), + deprecator=_deprecator.INDETERMINATE_DEPRECATOR, + )] + + +def test_deprecate_via_module() -> None: + with patch_module_args(): + am = AnsibleModule(argument_spec={}) + + am.deprecate('Deprecation message') + + assert warnings.get_deprecation_messages() == (dict(msg='Deprecation message', collection_name=None, version=None),) + assert warnings.get_deprecations() == [_messages.DeprecationSummary( + event=_messages.Event(msg='Deprecation message'), + deprecator=_deprecator.INDETERMINATE_DEPRECATOR, + )] def test_dedupe_with_traceback(module_env_mocker: ModuleEnvMocker) -> None: module_env_mocker.set_traceback_config([_traceback.TracebackEvent.DEPRECATED]) deprecate_args: dict[str, t.Any] = dict(msg="same", version="1.2.3", collection_name="blar.blar") - # DeprecationMessageDetail dataclass object hash is the dedupe key; presence of differing tracebacks or SourceContexts affects de-dupe + # DeprecationSummary dataclass object hash is the dedupe key; presence of differing tracebacks or SourceContexts affects de-dupe for _i in range(0, 10): # same location, same traceback- should be collapsed to one message diff --git a/test/units/module_utils/common/warnings/test_error_as_warning.py b/test/units/module_utils/common/warnings/test_error_as_warning.py new file mode 100644 index 00000000000..6afaed3f4dc --- /dev/null +++ b/test/units/module_utils/common/warnings/test_error_as_warning.py @@ -0,0 +1,34 @@ +from __future__ import annotations + +import pytest + +from ansible.module_utils.basic import AnsibleModule +from ansible.module_utils.common import warnings + +from ansible.module_utils.common.warnings import error_as_warning +from ansible.module_utils.testing import patch_module_args + +pytestmark = pytest.mark.usefixtures("as_target", "module_env_mocker") + + +def test_error_as_warning() -> None: + try: + raise Exception('hello') + except Exception as ex: + error_as_warning('Warning message', ex) + + assert warnings.get_warning_messages() == ('Warning message: hello',) + assert len(warnings.get_warnings()) == 1 + + +def test_error_as_warning_via_module() -> None: + with patch_module_args(): + am = AnsibleModule(argument_spec={}) + + try: + raise Exception('hello') + except Exception as ex: + am.error_as_warning('Warning message', ex) + + assert warnings.get_warning_messages() == ('Warning message: hello',) + assert len(warnings.get_warnings()) == 1 diff --git a/test/units/module_utils/common/warnings/test_warn.py b/test/units/module_utils/common/warnings/test_warn.py index f62ad361966..6da10d6b465 100644 --- a/test/units/module_utils/common/warnings/test_warn.py +++ b/test/units/module_utils/common/warnings/test_warn.py @@ -8,12 +8,14 @@ import pytest import typing as t from ansible.module_utils._internal import _traceback, _messages +from ansible.module_utils.basic import AnsibleModule from ansible.module_utils.common import warnings from ansible.module_utils.common.warnings import warn +from ansible.module_utils.testing import patch_module_args from units.mock.module import ModuleEnvMocker -pytestmark = pytest.mark.usefixtures("module_env_mocker") +pytestmark = pytest.mark.usefixtures("as_target", "module_env_mocker") def test_warn(): @@ -22,6 +24,16 @@ def test_warn(): assert warnings.get_warnings() == [_messages.WarningSummary(event=_messages.Event(msg='Warning message'))] +def test_warn_via_module() -> None: + with patch_module_args(): + am = AnsibleModule(argument_spec={}) + + am.warn('Warning message') + + assert warnings.get_warning_messages() == ('Warning message',) + assert warnings.get_warnings() == [_messages.WarningSummary(event=_messages.Event(msg='Warning message'))] + + def test_multiple_warnings(): messages = [ 'First warning', @@ -40,7 +52,7 @@ def test_dedupe_with_traceback(module_env_mocker: ModuleEnvMocker) -> None: module_env_mocker.set_traceback_config([_traceback.TracebackEvent.WARNING]) msg = "a warning message" - # WarningMessageDetail dataclass object hash is the dedupe key; presence of differing tracebacks or SourceContexts affects de-dupe + # WarningSummary dataclass object hash is the dedupe key; presence of differing tracebacks or SourceContexts affects de-dupe for _i in range(0, 10): warn(msg) # same location, same traceback- should be collapsed to one message diff --git a/test/units/utils/display/test_warning.py b/test/units/utils/display/test_warning.py index 3fe545c4455..f06f85ccf08 100644 --- a/test/units/utils/display/test_warning.py +++ b/test/units/utils/display/test_warning.py @@ -7,6 +7,29 @@ from __future__ import annotations import pytest from ansible.utils.display import Display +from ansible.module_utils import basic +from units.test_utils.controller.display import emits_warnings + + +def test_module_utils_warn() -> None: + """Verify that `module_utils.basic.warn` on the controller is routed to `Display.warning`.""" + with emits_warnings(warning_pattern="hello"): + basic.warn("hello") + + +def test_module_utils_error_as_warning() -> None: + """Verify that `module_utils.basic.error_as_warning` on the controller is routed to `Display.error_as_warning`.""" + with emits_warnings(warning_pattern="hello.*world"): + try: + raise Exception("world") + except Exception as ex: + basic.error_as_warning("hello", ex) + + +def test_module_utils_deprecate() -> None: + """Verify that `module_utils.basic.deprecate` on the controller is routed to `Display.deprecated`.""" + with emits_warnings(deprecation_pattern="hello"): + basic.deprecate("hello", version='9999.9') @pytest.fixture