Skip to content

Commit

Permalink
More fixes
Browse files Browse the repository at this point in the history
  • Loading branch information
ssbarnea committed Sep 28, 2022
1 parent be0382c commit df153ce
Show file tree
Hide file tree
Showing 10 changed files with 32 additions and 26 deletions.
2 changes: 1 addition & 1 deletion examples/playbooks/custom_module.yml
Original file line number Diff line number Diff line change
Expand Up @@ -5,5 +5,5 @@
tags:
- "{{ foo }}"
tasks:
- name: Run custom module # noqa: fqcn[module]
- name: Run custom module # noqa: fqcn[action]
fake_module: {}
4 changes: 2 additions & 2 deletions examples/playbooks/nomatchestest.transformed.yml
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
hosts: whatever
tasks:
- name: Hello world
action: debug msg="Hello!" # noqa: fqcn[builtin]
action: debug msg="Hello!" # noqa: fqcn[action-internal]

- name: This should be fine too
action: file state=touch dest=./wherever mode=0600 # noqa: fqcn[builtin]
action: file state=touch dest=./wherever mode=0600 # noqa: fqcn[action-internal]
4 changes: 2 additions & 2 deletions examples/playbooks/nomatchestest.yml
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
hosts: whatever
tasks:
- name: Hello world
action: debug msg="Hello!" # noqa: fqcn[builtin]
action: debug msg="Hello!" # noqa: fqcn[action-internal]

- name: This should be fine too
action: file state=touch dest=./wherever mode=0600 # noqa: fqcn[builtin]
action: file state=touch dest=./wherever mode=0600 # noqa: fqcn[action-internal]
2 changes: 1 addition & 1 deletion examples/playbooks/rule-risky-file-permissions-fail.yml
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@
hosts: all
tasks:
- name: Permissions missing
# noqa: fqcn[builtin]
# noqa: fqcn[action-internal]
get_url:
url: http://foo
dest: foo
Expand Down
2 changes: 1 addition & 1 deletion src/ansiblelint/constants.py
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ def main():
"hg-latest": "latest[hg]",
"no-jinja-nesting": "jinja[invalid]",
"no-loop-var-prefix": "loop-var-prefix",
"fqcn-builtins": "fqcn[builtin]",
"fqcn-builtins": "fqcn[action-internal]",
}

PLAYBOOK_TASK_KEYWORDS = [
Expand Down
28 changes: 16 additions & 12 deletions src/ansiblelint/rules/fqcn.py
Original file line number Diff line number Diff line change
Expand Up @@ -101,21 +101,22 @@ def matchtask(
if module in builtins:
result.append(
self.create_matcherror(
message="Use FQCN for builtin actions.",
message=f"Use FQCN for builtin module actions ({module}).",
details=f"Use `ansible.builtin.{module}` or `ansible.legacy.{module}` instead.",
filename=file,
linenumber=task["__line__"],
tag="fqcn[builtin]",
tag="fqcn[action-internal]",
)
)
# Add here implementation for fqcn[action-redirect]
elif module != "block/always/rescue" and module.count(".") != 2:
result.append(
self.create_matcherror(
message="Use FQCN for actions, such `namespace.collection_name.action`.",
message=f"Use FQCN for module actions, such `<namespace>.<collection>.{module}`.",
details=f"Action `{module}` is not FQCN.",
filename=file,
linenumber=task["__line__"],
tag="fqcn[module]",
tag="fqcn[action]",
)
)
return result
Expand All @@ -132,15 +133,15 @@ def matchtask(
- hosts: localhost
tasks:
- name: Shell (fqcn)
ansible.builtin.shell: echo This rule should not get matched by the fqcn[builtin] rule
ansible.builtin.shell: echo This rule should not get matched by the fqcn rule
"""

FAIL_PLAY = """
- hosts: localhost
tasks:
- name: Shell (fqcn[builtin])
shell: echo This rule should get matched by the fqcn[builtin] rule
- name: Shell (fqcn[module])
- name: Shell (fqcn[action-internal])
shell: echo This rule should get matched by the fqcn rule
- name: Shell (fqcn[action])
ini_file:
path: /tmp/test.ini
"""
Expand All @@ -152,10 +153,13 @@ def test_fqcn_builtin_fail(rule_runner: RunFromText) -> None:
"""Test rule matches."""
results = rule_runner.run_playbook(FAIL_PLAY)
assert len(results) == 2
assert results[0].tag == "fqcn[builtin]"
assert "Use FQCN for builtin actions" in results[0].message
assert results[1].tag == "fqcn[module]"
assert "Use FQCN for module calls" in results[1].message
assert results[0].tag == "fqcn[action-internal]"
assert "Use FQCN for builtin module actions" in results[0].message
assert results[1].tag == "fqcn[action]"
assert (
"Use FQCN for module actions, such `<namespace>.<collection>"
in results[1].message
)

@pytest.mark.parametrize(
"rule_runner", (FQCNBuiltinsRule,), indirect=["rule_runner"]
Expand Down
4 changes: 3 additions & 1 deletion src/ansiblelint/schemas/ansible-lint-config.json
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,9 @@
"deprecated-module",
"empty-string-compare",
"fqcn",
"fqcn[builtin]",
"fqcn[action-internal]",
"fqcn[action-redirect]",
"fqcn[action]",
"latest",
"galaxy-collection-version",
"ignore-errors",
Expand Down
4 changes: 1 addition & 3 deletions test/test_eco.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,11 +41,9 @@ def test_eco(repo: str) -> None:
)
# run ansible lint and paths from user home in order to produce
# consistent results regardless on its location.

# we exclude `fqcn[builtin]` until repository owners fix it.
for step in ["before", "after"]:

args = ["-f", "pep8", "-x", "fqcn[builtin]"]
args = ["-f", "pep8"]
executable = (
"ansible-lint"
if step == "after"
Expand Down
4 changes: 2 additions & 2 deletions test/test_file_path_evaluation.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@
"""\
---
- name: From subtask 2 do something
debug: # <-- expected to raise fqcn[builtin]
debug: # <-- expected to raise fqcn[action-internal]
msg: |
Something...
"""
Expand Down Expand Up @@ -88,7 +88,7 @@
"""\
---
- name: From subtask 2 do something
debug: # <-- expected to raise fqcn[builtin]
debug: # <-- expected to raise fqcn[action-internal]
msg: |
Something...
"""
Expand Down
4 changes: 3 additions & 1 deletion test/test_import_include_role.py
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,9 @@ def test_import_role2(
) -> None:
"""Test that include_role digs deeper than import_role."""
runner = Runner(
playbook_path, rules=default_rules_collection, skip_list=["fqcn[builtin]"]
playbook_path,
rules=default_rules_collection,
skip_list=["fqcn[action-internal]"],
)
results = runner.run()
for message in messages:
Expand Down

0 comments on commit df153ce

Please sign in to comment.