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

Update key-order check to check all keys #2222

Closed
wants to merge 6 commits into from
Closed
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
3 changes: 3 additions & 0 deletions src/ansiblelint/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@
}

PROFILES = yaml_from_file(Path(__file__).parent / "data" / "profiles.yml")
KEY_ORDER = yaml_from_file(Path(__file__).parent / "data" / "key-order.yml")

options = Namespace(
cache_dir=None,
Expand Down Expand Up @@ -125,6 +126,8 @@
extra_vars=None,
enable_list=[],
skip_action_validation=True,
key_order=KEY_ORDER["default"],
custom_key_order=None,
rules={}, # Placeholder to set and keep configurations for each rule.
)

Expand Down
75 changes: 75 additions & 0 deletions src/ansiblelint/data/key-order.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
---
default:
- name
- action
enhanced:
- name
- become
- become_user
- when
- notify
- delegate_to
- no_log
- action
- args
- loop
- loop_control
- with
- vars
- tags
shared:
- name
- become
- become_exe
- become_flags
- become_method
- become_user
- remote_user
- changed_when
- failed_when
- check_mode
- any_errors_fatal
- when
- notify
- collections
- connection
- debugger
- delay
- delegate_facts
- delegate_to
- diff
- environment
- ignore_errors
- ignore_unreachable
- local_action
- module_defaults
- no_log
- poll
- port
- async
- timeout
- retries
- until
- register
- run_once
- throttle
- action
- args
- loop
- loop_control
- with
- with_dict
- with_fileglob
- with_filetree
- with_first_found
- with_indexed_items
- with_ini
- with_inventory_hostnames
- with_items
- with_lines
- with_random_choice
- with_sequence
- with_subelements
- with_together
- vars
- tags
64 changes: 57 additions & 7 deletions src/ansiblelint/rules/key_order.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,12 @@
"""All tasks should be have name come first."""
"""Ensure specific order of keys in mappings."""
from __future__ import annotations

import sys
from collections import OrderedDict as odict
from operator import itemgetter
from typing import Any

from ansiblelint.config import options
from ansiblelint.file_utils import Lintable
from ansiblelint.rules import AnsibleLintRule
from ansiblelint.testing import RunFromText
Expand All @@ -13,20 +16,47 @@ class KeyOrderRule(AnsibleLintRule):
"""Ensure specific order of keys in mappings."""

id = "key-order"
description = """\
Keys should be in the specified order. In the default configuration, it only enforces name first. Checking the order of all keys can be anabled by setting 'key_order' in the config
"""
shortdesc = __doc__
severity = "LOW"
tags = ["formatting", "experimental"]
version_added = "v6.2.0"
needs_raw_task = True

# skipped rules is not a key
removed_keys = ["skipped_rules"]
possible_keys = options.key_order
if options.custom_key_order:
possible_keys = options.custom_key_order

ordered_expected_keys = odict((key, idx) for idx, key in enumerate(possible_keys))

def matchtask(
self, task: dict[str, Any], file: Lintable | None = None
) -> bool | str:
raw_task = task["__raw_task__"]
if "name" in raw_task:
attribute_list = [*raw_task]
if bool(attribute_list[0] != "name"):
return "'name' key is not first"
keys = task["__raw_task__"].keys()

# get the expected order in from the lookup table
actual_order = odict()
for attr in keys:
if not attr.startswith("__") and (attr not in self.removed_keys):
pos = self.ordered_expected_keys.get(attr)
if pos is None:
pos = self.ordered_expected_keys.get("action")
actual_order[attr] = pos

sorted_actual_order = odict(
sorted(
actual_order.items(),
key=itemgetter(1),
)
)

if bool(sorted_actual_order != actual_order):
text = ",".join(sorted_actual_order.keys())
return f"Keys are not in order. Expected order '{text}'"
return False


Expand Down Expand Up @@ -58,6 +88,12 @@ def matchtask(
- register: test
shell: echo hello
name: Register first
- tags: hello
no_log: true
name: Echo hello with tags
become: true
delegate_to: localhost
shell: Echo hello with tags
"""

PLAY_SUCCESS = """---
Expand All @@ -70,6 +106,20 @@ def matchtask(
msg: "Debug without a name"
- name: Flush handlers
meta: flush_handlers
- name: task with no_log on top
no_log: true # noqa key-order
shell: echo hello
- name: Echo hello with tags
become: true
delegate_to: localhost
no_log: true
shell: echo hello with tags
tags: hello
- name: Loopy
command: echo {{ item }}
loop:
- 1
- 2
- no_log: true # noqa key-order
shell: echo hello
name: Task with no_log on top
Expand All @@ -85,4 +135,4 @@ def test_task_name_has_name_first_rule_pass(rule_runner: RunFromText) -> None:
def test_task_name_has_name_first_rule_fail(rule_runner: RunFromText) -> None:
"""Test rule matches."""
results = rule_runner.run_playbook(PLAY_FAIL)
assert len(results) == 6
assert len(results) == 7