Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Determine if passed arguments are playbooks or not #2912

Merged
merged 1 commit into from
Jan 19, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions .config/dictionary.txt
Original file line number Diff line number Diff line change
Expand Up @@ -270,6 +270,8 @@ robertdebock
rolepath
roundtrip
ruamel
rulebook
rulebooks
ruledirs
rulesdir
rulesdirs
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/tox.yml
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ jobs:
WSLENV: FORCE_COLOR:PYTEST_REQPASS:TOXENV:GITHUB_STEP_SUMMARY
# Number of expected test passes, safety measure for accidental skip of
# tests. Update value if you add/remove tests.
PYTEST_REQPASS: 755
PYTEST_REQPASS: 767

steps:
- name: Activate WSL1
Expand Down
Empty file.
3 changes: 3 additions & 0 deletions examples/other/guess-1.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
---
- name: Minimal yaml to determine it as a playbook
hosts: localhost
21 changes: 21 additions & 0 deletions examples/playbooks/rulebook.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
---
# That file is not a valid playbook but it is a valid rulebook that was
# mistakenly put under a playbook directory.
- name: Demo rules with kafka as source
hosts: localhost
sources:
- name: kafka
kafka:
topic: eda
host: localhost
port: 9092
group_id: testing
rules:
- name:
condition: event.i is defined
action:
debug:
- name:
condition: event.stop == true
action:
shutdown:
Empty file added examples/roles/foo.yml
Empty file.
19 changes: 19 additions & 0 deletions examples/rulebooks/rulebook.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
---
- name: Demo rules with kafka as source
hosts: localhost
sources:
- name: kafka
kafka:
topic: eda
host: localhost
port: 9092
group_id: testing
rules:
- name:
condition: event.i is defined
action:
debug:
- name:
condition: event.stop == true
action:
shutdown:
1 change: 1 addition & 0 deletions src/ansiblelint/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@
{"galaxy": "**/galaxy.yml"}, # Galaxy collection meta
{"reno": "**/releasenotes/*/*.{yaml,yml}"}, # reno release notes
{"tasks": "**/tasks/**/*.{yaml,yml}"},
{"rulebook": "**/rulebooks/*.{yml,yaml"},
{"playbook": "**/playbooks/*.{yml,yaml}"},
{"playbook": "**/*playbook*.{yml,yaml}"},
{"role": "**/roles/*/"},
Expand Down
1 change: 1 addition & 0 deletions src/ansiblelint/constants.py
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ def main():

FileType = Literal[
"playbook",
"rulebook",
"meta", # role meta
"meta-runtime",
"tasks", # includes pre_tasks, post_tasks
Expand Down
18 changes: 18 additions & 0 deletions src/ansiblelint/file_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -241,6 +241,24 @@ def __init__(
self.base_kind = base_kind or kind_from_path(self.path, base=True)
self.abspath = self.path.expanduser().absolute()

if self.kind == "yaml":
self._guess_kind()

def _guess_kind(self) -> None:
if self.kind == "yaml":
if isinstance(self.data, list) and "hosts" in self.data[0]:
if "rules" not in self.data[0]:
self.kind = "playbook"
else:
self.kind = "rulebook"
# we we failed to guess the more specific kind, we warn user
if self.kind == "yaml":
_logger.debug(
"Passed '%s' positional argument was identified as generic '%s' file kind.",
self.name,
self.kind,
)

def __getitem__(self, key: Any) -> Any:
"""Provide compatibility subscriptable support."""
if key == "path":
Expand Down
2 changes: 0 additions & 2 deletions src/ansiblelint/rules/name.py
Original file line number Diff line number Diff line change
Expand Up @@ -117,14 +117,12 @@ def _check_name(
return results
else:
effective_name = name[len(prefix) :]
# breakpoint()

if (
effective_name[0].isalpha()
and effective_name[0].islower()
and not effective_name[0].isupper()
):
# breakpoint()
results.append(
self.create_matcherror(
message="All names should start with an uppercase letter.",
Expand Down
1 change: 0 additions & 1 deletion src/ansiblelint/schemas/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,6 @@ def refresh_schemas(min_age_seconds: int = 3600 * 24) -> int:
_logger.debug("Checking for updated schemas...")

changed = 0
# breakpoint()
for kind, data in JSON_SCHEMAS.items():
url = data["url"]
if "#" in url:
Expand Down
8 changes: 0 additions & 8 deletions src/ansiblelint/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -846,14 +846,6 @@ def get_lintables(
if args:
for arg in args:
lintable = Lintable(arg)
if lintable.kind in ("yaml", None):
_logger.warning(
"Overriding detected file kind '%s' with 'playbook' "
"for given positional argument: %s",
lintable.kind,
arg,
)
lintable = Lintable(arg, kind="playbook")
lintables.append(lintable)
else:

Expand Down
2 changes: 1 addition & 1 deletion test/local-content/test-collection.yml
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,6 @@
- name: Use module from local collection
testns.test_collection.test_module_2:
- name: Use filter from local collection
assert:
ansible.builtin.assert:
that:
- 1 | testns.test_collection.test_filter(2) == '1:2'
Empty file.
Empty file.
Empty file.
125 changes: 77 additions & 48 deletions test/test_file_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -144,60 +144,94 @@ def test_discover_lintables_umlaut(monkeypatch: MonkeyPatch) -> None:
@pytest.mark.parametrize(
("path", "kind"),
(
("tasks/run_test_playbook.yml", "tasks"),
("foo/playbook.yml", "playbook"),
("playbooks/foo.yml", "playbook"),
("playbooks/roles/foo.yml", "yaml"),
pytest.param("tasks/run_test_playbook.yml", "tasks", id="0"),
pytest.param("foo/playbook.yml", "playbook", id="1"),
pytest.param("playbooks/foo.yml", "playbook", id="2"),
pytest.param("examples/roles/foo.yml", "yaml", id="3"),
# the only yml file that is not a playbook inside molecule/ folders
(".config/molecule/config.yml", "yaml"), # molecule shared config
(
"roles/foo/molecule/scenario1/base.yml",
"yaml",
pytest.param(
"examples/.config/molecule/config.yml", "yaml", id="4"
), # molecule shared config
pytest.param(
"test/schemas/test/molecule/cluster/base.yml", "yaml", id="5"
), # molecule scenario base config
(
"roles/foo/molecule/scenario1/molecule.yml",
"yaml",
pytest.param(
"test/schemas/test/molecule/cluster/molecule.yml", "yaml", id="6"
), # molecule scenario config
("roles/foo/molecule/scenario2/foobar.yml", "playbook"), # custom playbook name
(
"roles/foo/molecule/scenario3/converge.yml",
"playbook",
pytest.param(
"test/schemas/test/molecule/cluster/foobar.yml", "playbook", id="7"
), # custom playbook name
pytest.param(
"test/schemas/test/molecule/cluster/converge.yml", "playbook", id="8"
), # common playbook name
(
"roles/foo/molecule/scenario3/requirements.yml",
"requirements",
pytest.param(
"roles/foo/molecule/scenario3/requirements.yml", "requirements", id="9"
), # requirements
(
"roles/foo/molecule/scenario3/collections.yml",
"requirements",
pytest.param(
"roles/foo/molecule/scenario3/collections.yml", "requirements", id="10"
), # requirements
("roles/foo/meta/argument_specs.yml", "arg_specs"), # role argument specs
pytest.param(
"roles/foo/meta/argument_specs.yml", "arg_specs", id="11"
), # role argument specs
# tasks files:
("tasks/directory with spaces/main.yml", "tasks"), # tasks
("tasks/requirements.yml", "tasks"), # tasks
pytest.param("tasks/directory with spaces/main.yml", "tasks", id="12"), # tasks
pytest.param("tasks/requirements.yml", "tasks", id="13"), # tasks
# requirements (we do not support includes yet)
("requirements.yml", "requirements"), # collection requirements
("roles/foo/meta/requirements.yml", "requirements"), # inside role requirements
pytest.param(
"requirements.yml", "requirements", id="14"
), # collection requirements
pytest.param(
"roles/foo/meta/requirements.yml", "requirements", id="15"
), # inside role requirements
# Undeterminable files:
("test/fixtures/unknown-type.yml", "yaml"),
("releasenotes/notes/run-playbooks-refactor.yaml", "reno"), # reno
("examples/host_vars/localhost.yml", "vars"),
("examples/group_vars/all.yml", "vars"),
("examples/playbooks/vars/other.yml", "vars"),
("examples/playbooks/vars/subfolder/settings.yml", "vars"), # deep vars
("molecule/scenario/collections.yml", "requirements"), # deprecated 2.8 format
(
"../roles/geerlingguy.mysql/tasks/configure.yml",
"tasks",
pytest.param("test/fixtures/unknown-type.yml", "yaml", id="16"),
pytest.param(
"releasenotes/notes/run-playbooks-refactor.yaml", "reno", id="17"
), # reno
pytest.param("examples/host_vars/localhost.yml", "vars", id="18"),
pytest.param("examples/group_vars/all.yml", "vars", id="19"),
pytest.param("examples/playbooks/vars/other.yml", "vars", id="20"),
pytest.param(
"examples/playbooks/vars/subfolder/settings.yml", "vars", id="21"
), # deep vars
pytest.param(
"molecule/scenario/collections.yml", "requirements", id="22"
), # deprecated 2.8 format
pytest.param(
"../roles/geerlingguy.mysql/tasks/configure.yml", "tasks", id="23"
), # relative path involved
("galaxy.yml", "galaxy"),
("foo.j2.yml", "jinja2"),
("foo.yml.j2", "jinja2"),
("foo.j2.yaml", "jinja2"),
("foo.yaml.j2", "jinja2"),
pytest.param("galaxy.yml", "galaxy", id="24"),
pytest.param("foo.j2.yml", "jinja2", id="25"),
pytest.param("foo.yml.j2", "jinja2", id="26"),
pytest.param("foo.j2.yaml", "jinja2", id="27"),
pytest.param("foo.yaml.j2", "jinja2", id="28"),
pytest.param(
"examples/playbooks/rulebook.yml", "playbook", id="29"
), # playbooks folder should determine kind
pytest.param(
"examples/rulebooks/rulebook.yml", "rulebook", id="30"
), # content should determine it as a rulebook
pytest.param(
"examples/yamllint/valid.yml", "yaml", id="31"
), # empty yaml is valid yaml, not assuming anything else
pytest.param(
"examples/other/guess-1.yml", "playbook", id="32"
), # content should determine is as a play
pytest.param(
"examples/playbooks/tasks/passing_task.yml", "tasks", id="33"
), # content should determine is tasks
pytest.param("examples/collection/galaxy.yml", "galaxy", id="34"),
pytest.param("examples/meta/runtime.yml", "meta-runtime", id="35"),
pytest.param("examples/meta/changelogs/changelog.yaml", "changelog", id="36"),
pytest.param("examples/inventory/inventory.yml", "inventory", id="37"),
pytest.param("examples/inventory/production.yml", "inventory", id="38"),
pytest.param("examples/playbooks/vars/empty_vars.yml", "vars", id="39"),
pytest.param(
"examples/playbooks/vars/subfolder/settings.yaml", "vars", id="40"
),
),
)
def test_default_kinds(monkeypatch: MonkeyPatch, path: str, kind: FileType) -> None:
def test_kinds(monkeypatch: MonkeyPatch, path: str, kind: FileType) -> None:
"""Verify auto-detection logic based on DEFAULT_KINDS."""
options = cli.get_config([])

Expand All @@ -212,9 +246,6 @@ def mockreturn(options: Namespace) -> dict[str, Any]:

monkeypatch.setattr(file_utils, "discover_lintables", mockreturn)
result = file_utils.discover_lintables(options)
# get_lintable could return additional files and we only care to see
# that the given file is among the returned list.
assert lintable_detected.name in result
assert lintable_detected.kind == result[lintable_expected.name]


Expand Down Expand Up @@ -310,9 +341,7 @@ def test_lintable_with_new_file(tmp_path: Path) -> None:
"""Validate ``Lintable.updated`` for a new file."""
lintable = Lintable(tmp_path / "new.yaml")

with pytest.raises(FileNotFoundError):
_ = lintable.content

lintable.content = BASIC_PLAYBOOK
lintable.content = BASIC_PLAYBOOK
assert lintable.content == BASIC_PLAYBOOK

Expand Down
6 changes: 3 additions & 3 deletions test/test_formatter.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ def test_format_coloured_string() -> None:
message="message",
linenumber=1,
details=DETAILS,
filename=Lintable("filename.yml"),
filename=Lintable("filename.yml", content=""),
rule=rule,
)
formatter.format(match)
Expand All @@ -50,7 +50,7 @@ def test_unicode_format_string() -> None:
message="\U0001f427",
linenumber=1,
details=DETAILS,
filename=Lintable("filename.yml"),
filename=Lintable("filename.yml", content=""),
rule=rule,
)
formatter.format(match)
Expand All @@ -62,7 +62,7 @@ def test_dict_format_line() -> None:
message="xyz",
linenumber=1,
details={"hello": "world"}, # type: ignore
filename=Lintable("filename.yml"),
filename=Lintable("filename.yml", content=""),
rule=rule,
)
formatter.format(match)
6 changes: 3 additions & 3 deletions test/test_formatter_json.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ def setup_class(self) -> None:
message="message",
linenumber=1,
details="hello",
filename=Lintable("filename.yml"),
filename=Lintable("filename.yml", content=""),
rule=self.rule,
)
)
Expand All @@ -41,7 +41,7 @@ def setup_class(self) -> None:
message="message",
linenumber=2,
details="hello",
filename=Lintable("filename.yml"),
filename=Lintable("filename.yml", content=""),
rule=self.rule,
)
)
Expand Down Expand Up @@ -103,7 +103,7 @@ def test_validate_codeclimate_schema_with_positions(self) -> None:
linenumber=1,
column=42,
details="hello",
filename=Lintable("filename.yml"),
filename=Lintable("filename.yml", content=""),
rule=self.rule,
)
]
Expand Down
4 changes: 2 additions & 2 deletions test/test_formatter_sarif.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ def setup_class(self) -> None:
linenumber=1,
column=10,
details="hello",
filename=Lintable("filename.yml"),
filename=Lintable("filename.yml", content=""),
rule=self.rule,
tag="yaml[test]",
)
Expand All @@ -46,7 +46,7 @@ def setup_class(self) -> None:
message="message",
linenumber=2,
details="hello",
filename=Lintable("filename.yml"),
filename=Lintable("filename.yml", content=""),
rule=self.rule,
tag="yaml[test]",
)
Expand Down
4 changes: 2 additions & 2 deletions test/test_matcherrror.py
Original file line number Diff line number Diff line change
Expand Up @@ -77,8 +77,8 @@ def test_matcherror_invalid() -> None:
(MatchError("z"), MatchError("a")),
# filenames takes priority in sorting
(
MatchError("a", filename=Lintable("b")),
MatchError("a", filename=Lintable("a")),
MatchError("a", filename=Lintable("b", content="")),
MatchError("a", filename=Lintable("a", content="")),
),
# rule id partial-become > rule id no-changed-when
(
Expand Down