Fix missing ansible.builtin FQCNs in hardcoded action names #71824

felixfontein · 2020-09-19T07:13:26Z

SUMMARY

Fixes #71817, fixes #71818, fixes #72319, and a bunch of related problems. This is important since the docs try to always use the FQCN version of a action/module.

This is not the best way to fix this, since a collection might redirect an action to ansible.builtin.xxx; when invoked that way, the problems still happen. That is probably esoteric enough that it can be ignored (at least for now) :)

ISSUE TYPE

Bugfix Pull Request

COMPONENT NAME

core

felixfontein · 2020-09-19T15:40:18Z

/rebuild_failed

felixfontein · 2020-09-19T17:14:47Z

/rebuild_failed

felixfontein · 2020-09-19T18:33:33Z

/rebuild_failed

felixfontein · 2020-09-19T19:09:49Z

/rebuild_failed

bcoca · 2020-09-22T15:59:20Z

lib/ansible/cli/adhoc.py

@@ -69,7 +69,8 @@ def _play_ds(self, pattern, async_val, poll):
        mytask = {'action': {'module': context.CLIARGS['module_name'], 'args': parse_kv(context.CLIARGS['module_args'], check_raw=check_raw)}}

        # avoid adding to tasks that don't support it, unless set, then give user an error
-        if context.CLIARGS['module_name'] not in ('include_role', 'include_tasks') and any(frozenset((async_val, poll))):
+        include_actions = ('include_role', 'include_tasks', 'ansible.builtin.include_role', 'ansible.builtin.include_tasks')


i would just create a function that appends `ansible.builtin.' prefixed options to provided list

I already added such a function ;) I'm now using it everywhere. For efficiency, how about converting all the (now) dynamic function calls to new constants in (f.ex.) constants.py?

SimonHeimberg · 2020-09-30T09:53:16Z

Does this also check the following? (#71878)

- ansible.builtin.command:
    cmd: ls

I do not find it, but this does not mean it is not there.

felixfontein · 2020-09-30T10:00:24Z

@SimonHeimberg I just checked, with this PR your example also works (and with devel it does not).

bcoca · 2020-10-02T22:02:34Z

lib/ansible/constants.py

@@ -59,6 +59,15 @@ def __getitem__(self, y):
        return self._value[y]


+def _add_builtin_fqcn(names):


function is fine, but this might not be best location, as constants.py has a heavy cost on import for those that already dont use it.

I first thought about lib/ansible/utils/collection_loader/init.py, but that comes with its own import tree. How about lib/ansible/utils/fqcn.py (a new file)? Or lib/ansible/utils/helpers.py?

either works for me, you can even cache responses with a module level var

bcoca

a couple of small caveats, otherwise the code looks good.

felixfontein · 2020-10-02T22:13:35Z

@bcoca what about:

For efficiency, how about converting all the (now) dynamic function calls to new constants in (f.ex.) constants.py?

ci_complete

felixfontein · 2020-11-01T20:32:19Z

@bcoca I did the renaming. I had to adjust the renaming scheme a bit, since I noticed I used the same name twice. I hope the new one is fine as well :)

(The tests that currently fail also fail in other PRs, so they should be unrelated.)

felixfontein · 2020-11-02T16:32:58Z

/rebuild_failed

felixfontein · 2020-11-02T16:53:02Z

/rebuild

felixfontein · 2020-11-03T13:57:21Z

@bcoca thanks a lot for reviewing!

…71824) * Make sure hard-coded action names also check for FQCN. * Use _add_internal_fqcn() to avoid hardcoded lists and typoes. (cherry picked from commit da60525)

…71824) * Make sure hard-coded action names also check for FQCN. * Use _add_internal_fqcn() to avoid hardcoded lists and typoes. (cherry picked from commit da60525) ci_complete

ansibot added needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. and removed core_review In order to be merged, this PR must follow the core review workflow. labels Sep 19, 2020

ansibot added core_review In order to be merged, this PR must follow the core review workflow. and removed needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. labels Sep 19, 2020

samdoran removed the needs_triage Needs a first human triage before being processed. label Sep 22, 2020

bcoca reviewed Sep 22, 2020

View reviewed changes

bcoca reviewed Oct 2, 2020

View reviewed changes

bcoca requested changes Oct 2, 2020

View reviewed changes

ansibot added needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. and removed core_review In order to be merged, this PR must follow the core review workflow. labels Oct 2, 2020

Undo line removal.

c8e0dc5

felixfontein added 2 commits November 1, 2020 20:40

Fix collision.

3c3120e

Fix and adjust names.

b7ab5f2

ci_complete

ansibot added needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. and removed core_review In order to be merged, this PR must follow the core review workflow. labels Nov 1, 2020

bcoca approved these changes Nov 2, 2020

View reviewed changes

felixfontein closed this Nov 2, 2020

felixfontein reopened this Nov 2, 2020

ansibot added core_review In order to be merged, this PR must follow the core review workflow. and removed needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. labels Nov 2, 2020

bcoca merged commit da60525 into ansible:devel Nov 3, 2020

felixfontein deleted the fix-missing-fqcns branch November 3, 2020 13:52

felixfontein mentioned this pull request Nov 3, 2020

[2.10] Fix missing ansible.builtin FQCNs in hardcoded action names #72457

Merged

felixfontein mentioned this pull request Nov 3, 2020

[2.9] Fix missing ansible.builtin FQCNs in hardcoded action names #72458

Merged

felixfontein mentioned this pull request Nov 4, 2020

Replace some more FQCNs #72471

Merged

xanmanning mentioned this pull request Nov 9, 2020

Consider moving to a file based config PyratLabs/ansible-role-k3s#53

Closed

rlenferink mentioned this pull request Nov 15, 2020

ansible.builtin.shell: Unsupported parameters for (ansible.legacy.command) module #72627

Closed

mkrizek mentioned this pull request Nov 27, 2020

ansible ERROR when following "Introduction to ad-hoc commands" in User Guide #72724

Closed

ansible locked and limited conversation to collaborators Dec 1, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix missing ansible.builtin FQCNs in hardcoded action names #71824

Fix missing ansible.builtin FQCNs in hardcoded action names #71824

felixfontein commented Sep 19, 2020 •

edited by s-hertel

Loading

felixfontein commented Sep 19, 2020

felixfontein commented Sep 19, 2020

felixfontein commented Sep 19, 2020

felixfontein commented Sep 19, 2020

bcoca Sep 22, 2020

felixfontein Sep 30, 2020

SimonHeimberg commented Sep 30, 2020 •

edited

Loading

felixfontein commented Sep 30, 2020

bcoca Oct 2, 2020

felixfontein Oct 2, 2020

bcoca Oct 6, 2020

bcoca left a comment

felixfontein commented Oct 2, 2020

felixfontein commented Nov 1, 2020

felixfontein commented Nov 2, 2020

felixfontein commented Nov 2, 2020

felixfontein commented Nov 3, 2020

		@@ -59,6 +59,15 @@ def __getitem__(self, y):
		return self._value[y]


		def _add_builtin_fqcn(names):

Fix missing ansible.builtin FQCNs in hardcoded action names #71824

Fix missing ansible.builtin FQCNs in hardcoded action names #71824

Conversation

felixfontein commented Sep 19, 2020 • edited by s-hertel Loading

SUMMARY

ISSUE TYPE

COMPONENT NAME

felixfontein commented Sep 19, 2020

felixfontein commented Sep 19, 2020

felixfontein commented Sep 19, 2020

felixfontein commented Sep 19, 2020

bcoca Sep 22, 2020

Choose a reason for hiding this comment

felixfontein Sep 30, 2020

Choose a reason for hiding this comment

SimonHeimberg commented Sep 30, 2020 • edited Loading

felixfontein commented Sep 30, 2020

bcoca Oct 2, 2020

Choose a reason for hiding this comment

felixfontein Oct 2, 2020

Choose a reason for hiding this comment

bcoca Oct 6, 2020

Choose a reason for hiding this comment

bcoca left a comment

Choose a reason for hiding this comment

felixfontein commented Oct 2, 2020

felixfontein commented Nov 1, 2020

felixfontein commented Nov 2, 2020

felixfontein commented Nov 2, 2020

felixfontein commented Nov 3, 2020

felixfontein commented Sep 19, 2020 •

edited by s-hertel

Loading

SimonHeimberg commented Sep 30, 2020 •

edited

Loading