Skip to content

Commit

Permalink
Temporary avoid auto-fixing YAML files not owned by ansible (#3837)
Browse files Browse the repository at this point in the history
  • Loading branch information
ssbarnea authored Oct 12, 2023
1 parent 3c90628 commit c81e65f
Show file tree
Hide file tree
Showing 6 changed files with 73 additions and 10 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/tox.yml
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ jobs:
env:
# Number of expected test passes, safety measure for accidental skip of
# tests. Update value if you add/remove tests.
PYTEST_REQPASS: 831
PYTEST_REQPASS: 832
steps:
- uses: actions/checkout@v4
with:
Expand Down
9 changes: 9 additions & 0 deletions examples/.github/workflows/sample.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
---
name: ack
on: # <-- ansible-lint should not complain or touch about this 'on'
pull_request_target:
types: [opened]

jobs:
ack:
uses: ansible/devtools/.github/workflows/ack.yml@main
15 changes: 15 additions & 0 deletions src/ansiblelint/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,21 @@
{"text/python": "**/*.py"},
]

# File kinds that are recognized by ansible, used internally to force use of
# YAML 1.1 instead of 1.2 due to ansible-core dependency on pyyaml.
ANSIBLE_OWNED_KINDS = {
"handlers",
"galaxy",
"meta",
"meta-runtime",
"playbook",
"requirements",
"role-arg-spec",
"rulebook",
"tasks",
"vars",
}

PROFILES = yaml_from_file(Path(__file__).parent / "data" / "profiles.yml")

LOOP_VAR_PREFIX = "^(__|{role}_)"
Expand Down
6 changes: 5 additions & 1 deletion src/ansiblelint/file_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
import wcmatch.wcmatch
from yaml.error import YAMLError

from ansiblelint.config import BASE_KINDS, Options, options
from ansiblelint.config import ANSIBLE_OWNED_KINDS, BASE_KINDS, Options, options
from ansiblelint.constants import CONFIG_FILENAMES, FileType, States

if TYPE_CHECKING:
Expand Down Expand Up @@ -386,6 +386,10 @@ def __repr__(self) -> str:
"""Return user friendly representation of a lintable."""
return f"{self.name} ({self.kind})"

def is_owned_by_ansible(self) -> bool:
"""Return true for YAML files that are managed by Ansible."""
return self.kind in ANSIBLE_OWNED_KINDS

@property
def data(self) -> Any:
"""Return loaded data representation for current file, if possible."""
Expand Down
3 changes: 3 additions & 0 deletions src/ansiblelint/transformer.py
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,9 @@ def run(self) -> None:
if self.write_set != {"none"}:
self._do_transforms(file, ruamel_data or data, file_is_yaml, matches)

if not file.is_owned_by_ansible():
return

if file_is_yaml:
# noinspection PyUnboundLocalVariable
file.content = yaml.dumps(ruamel_data)
Expand Down
48 changes: 40 additions & 8 deletions test/test_transformer.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@

import ansiblelint.__main__ as main
from ansiblelint.app import App
from ansiblelint.file_utils import Lintable

# noinspection PyProtectedMember
from ansiblelint.runner import LintResult, get_matches
Expand All @@ -34,119 +35,150 @@ def fixture_runner_result(


@pytest.mark.parametrize(
("playbook_str", "matches_count", "transformed"),
("playbook_str", "matches_count", "transformed", "is_owned_by_ansible"),
(
# reuse TestRunner::test_runner test cases to ensure transformer does not mangle matches
pytest.param(
"examples/playbooks/nomatchestest.yml",
0,
False,
True,
id="nomatchestest",
),
pytest.param("examples/playbooks/unicode.yml", 1, False, id="unicode"),
pytest.param("examples/playbooks/unicode.yml", 1, False, True, id="unicode"),
pytest.param(
"examples/playbooks/lots_of_warnings.yml",
993,
False,
True,
id="lots_of_warnings",
),
pytest.param("examples/playbooks/become.yml", 0, False, id="become"),
pytest.param("examples/playbooks/become.yml", 0, False, True, id="become"),
pytest.param(
"examples/playbooks/contains_secrets.yml",
0,
False,
True,
id="contains_secrets",
),
pytest.param(
"examples/playbooks/vars/empty_vars.yml",
0,
False,
True,
id="empty_vars",
),
pytest.param("examples/playbooks/vars/strings.yml", 0, True, id="strings"),
pytest.param("examples/playbooks/vars/empty.yml", 1, False, id="empty"),
pytest.param("examples/playbooks/name-case.yml", 1, True, id="name_case"),
pytest.param("examples/playbooks/fqcn.yml", 3, True, id="fqcn"),
pytest.param(
"examples/playbooks/vars/strings.yml",
0,
True,
True,
id="strings",
),
pytest.param("examples/playbooks/vars/empty.yml", 1, False, True, id="empty"),
pytest.param("examples/playbooks/name-case.yml", 1, True, True, id="name_case"),
pytest.param("examples/playbooks/fqcn.yml", 3, True, True, id="fqcn"),
pytest.param(
"examples/playbooks/multi_yaml_doc.yml",
1,
False,
True,
id="multi_yaml_doc",
),
pytest.param(
"examples/playbooks/transform_command_instead_of_shell.yml",
3,
True,
True,
id="cmd_instead_of_shell",
),
pytest.param(
"examples/playbooks/transform-deprecated-local-action.yml",
1,
True,
True,
id="dep_local_action",
),
pytest.param(
"examples/playbooks/transform-block-indentation-indicator.yml",
0,
True,
True,
id="multiline_msg_with_indent_indicator",
),
pytest.param(
"examples/playbooks/transform-jinja.yml",
7,
True,
True,
id="jinja_spacing",
),
pytest.param(
"examples/playbooks/transform-no-jinja-when.yml",
3,
True,
True,
id="no_jinja_when",
),
pytest.param(
"examples/playbooks/vars/transform_nested_data.yml",
3,
True,
True,
id="nested",
),
pytest.param(
"examples/playbooks/transform-key-order.yml",
6,
True,
True,
id="key_order_transform",
),
pytest.param(
"examples/playbooks/transform-no-free-form.yml",
2,
True,
True,
id="no_free_form_transform",
),
pytest.param(
"examples/playbooks/transform-partial-become.yml",
4,
True,
True,
id="partial_become",
),
pytest.param(
"examples/playbooks/transform-key-order-play.yml",
1,
True,
True,
id="key_order_play_transform",
),
pytest.param(
"examples/.github/workflows/sample.yml",
0,
False,
False,
id="github-workflow",
),
),
)
@mock.patch.dict(os.environ, {"ANSIBLE_LINT_WRITE_TMP": "1"}, clear=True)
def test_transformer(
def test_transformer( # pylint: disable=too-many-arguments
config_options: Options,
playbook_str: str,
runner_result: LintResult,
transformed: bool,
is_owned_by_ansible: bool,
matches_count: int,
) -> None:
"""Test that transformer can go through any corner cases.
Based on TestRunner::test_runner
"""
# test ability to detect is_owned_by_ansible
assert Lintable(playbook_str).is_owned_by_ansible() == is_owned_by_ansible
playbook = Path(playbook_str)
config_options.write_list = ["all"]
transformer = Transformer(result=runner_result, options=config_options)
Expand Down

0 comments on commit c81e65f

Please sign in to comment.