Skip to content

Commit

Permalink
Replace git-latest and hg-latest with latest rule
Browse files Browse the repository at this point in the history
Combines two similar rules into a more generic one that can be used
for similar purposed.
  • Loading branch information
ssbarnea committed Aug 31, 2022
1 parent 727735c commit 58964c9
Show file tree
Hide file tree
Showing 14 changed files with 127 additions and 147 deletions.
4 changes: 2 additions & 2 deletions .ansible-lint
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ use_default_rules: true
# This makes linter to fully ignore rules/tags listed below
skip_list:
- skip_this_tag
- git-latest
- latest[git]

# Any rule that has the 'opt-in' tag will not be loaded unless its 'id' is
# mentioned in the enable_list:
Expand All @@ -57,7 +57,7 @@ enable_list:
# This makes the linter display but not fail for rules/tags listed below:
warn_list:
- skip_this_tag
- git-latest
- latest[git]
- experimental # experimental is included in the implicit list
# - role-name
# - yaml[document-start] # you can also use sub-rule matches
Expand Down
4 changes: 2 additions & 2 deletions examples/playbooks/noqa.yml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
---
- hosts: localhost
tasks:
- name: This would typically fire git-latest and partial-become
become_user: alice # noqa git-latest partial-become
- name: This would typically fire latest[git] and partial-become
become_user: alice # noqa latest[git] partial-become
git: src=/path/to/git/repo dest=checkout
20 changes: 10 additions & 10 deletions examples/playbooks/skiptasks.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,10 @@
- hosts: all

tasks:
- name: Test git-latest
- name: Test latest[git]
action: ansible.builtin.git

- name: Test hg-latest
- name: Test latest[hg]
action: ansible.builtin.hg

- name: Test command-instead-of-module
Expand All @@ -15,12 +15,12 @@
- name: Test deprecated-command-syntax
ansible.builtin.command: creates=B chmod 644 A

- name: Test git-latest (skip)
- name: Test latest[git] (skip)
action: ansible.builtin.git
tags:
- skip_ansible_lint

- name: Test hg-latest (skip)
- name: Test latest[hg] (skip)
action: ansible.builtin.hg
tags:
- skip_ansible_lint
Expand All @@ -35,30 +35,30 @@
tags:
- skip_ansible_lint

- name: Test git-latest (don't warn)
- name: Test latest[git] (don't warn)
ansible.builtin.command: git log
args:
warn: false
changed_when: false

- name: Test hg-latest (don't warn)
- name: Test latest[hg] (don't warn)
ansible.builtin.command: chmod 644 A
args:
warn: false
creates: B

- name: Test hg-latest (warn)
- name: Test latest[hg] (warn)
ansible.builtin.command: chmod 644 A
args:
warn: true
creates: B

- name: Test git-latest (don't warn single line)
- name: Test latest[git] (don't warn single line)
ansible.builtin.command: warn=False chdir=/tmp/blah git log
changed_when: false

- name: Test hg-latest (don't warn single line)
- name: Test latest[hg] (don't warn single line)
ansible.builtin.command: warn=no creates=B chmod 644 A

- name: Test hg-latest (warn single line)
- name: Test latest[hg] (warn single line)
ansible.builtin.command: warn=yes creates=B chmod 644 A
8 changes: 4 additions & 4 deletions examples/playbooks/test_skip_inside_yaml.yml
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,15 @@
tags:
- skip_ansible_lint
tasks:
- name: Test hg-latest
- name: Test latest[hg]
action: ansible.builtin.hg
- name: Test hg-latest (skipped) # noqa hg-latest
- name: Test latest[hg] (skipped) # noqa latest[hg]
action: ansible.builtin.hg

- name: Test git-latest and partial-become
- name: Test latest[git] and partial-become
become_user: alice
action: ansible.builtin.git
- name: Test git-latest and partial-become (skipped) # noqa git-latest partial-become
- name: Test latest[git] and partial-become (skipped) # noqa latest[git] partial-become
become_user: alice
action: ansible.builtin.git

Expand Down
6 changes: 4 additions & 2 deletions src/ansiblelint/constants.py
Original file line number Diff line number Diff line change
Expand Up @@ -85,8 +85,8 @@ def main():
"304": "inline-env-var",
"305": "command-instead-of-shell",
"306": "risky-shell-pipe",
"401": "git-latest",
"402": "hg-latest",
"401": "latest[git]",
"402": "latest[hg]",
"403": "package-latest",
"404": "no-relative-paths",
"501": "partial-become",
Expand All @@ -103,6 +103,8 @@ def main():
"911": "syntax-check",
"var-spacing": "jinja[spacing]",
"unnamed-task": "name[missing]",
"git-latest": "latest[git]",
"hg-latest": "latest[hg]",
}

PLAYBOOK_TASK_KEYWORDS = [
Expand Down
3 changes: 1 addition & 2 deletions src/ansiblelint/data/profiles.yml
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,7 @@ safety:
a non-determinant outcomes and cause security issues in some cases.
extends: moderate
rules:
git-latest:
hg-latest:
latest:
package-latest:
risky-file-permissions:
risky-octal:
Expand Down
52 changes: 0 additions & 52 deletions src/ansiblelint/rules/git_latest.py

This file was deleted.

52 changes: 0 additions & 52 deletions src/ansiblelint/rules/hg_latest.py

This file was deleted.

43 changes: 43 additions & 0 deletions src/ansiblelint/rules/latest.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
# latest

The `latest` rule checks that module arguments like those used for source
control checkout do not have arguments that might generate different results
based on context.

This more generic rule replaced two older rules named `git-latest` and
`hg-latest`.

We are aware that there are genuine cases where getting the tip of the main
branch is not accidental. For these cases, just add a comment such as
`# noqa: latest` to the same line to prevent it from triggering.

## Possible errors messages:

- `latest[git]`
- `latest[hg]`

## Problematic code

```yaml
---
- name: Example for `latest` rule
hosts: localhost
tasks:
- name: Risky use of git module
ansible.builtin.git:
repo: "https://foosball.example.org/path/to/repo.git"
version: HEAD # <-- HEAD value is triggering the rule
```

## Correct code

```yaml
---
- name: Example for `latest` rule
hosts: localhost
tasks:
- name: Safe use of git module
ansible.builtin.git:
repo: "https://foosball.example.org/path/to/repo.git"
version: abcd1234... # <-- that is safe
```
41 changes: 41 additions & 0 deletions src/ansiblelint/rules/latest.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
"""Implementation of latest rule."""
from __future__ import annotations

from typing import TYPE_CHECKING, Any

from ansiblelint.errors import MatchError
from ansiblelint.rules import AnsibleLintRule

if TYPE_CHECKING:
from typing import Optional

from ansiblelint.file_utils import Lintable


class LatestRule(AnsibleLintRule):
"""Result of the command may vary on subsequent runs."""

id = "latest"
description = (
"All version control checkouts must point to "
"an explicit commit or tag, not just ``latest``"
)
severity = "MEDIUM"
tags = ["idempotency"]
version_added = "v6.5.2"

def matchtask(
self, task: dict[str, Any], file: Lintable | None = None
) -> bool | str | MatchError:
"""Check if module args are safe."""
if (
task["action"]["__ansible_module__"] == "git"
and task["action"].get("version", "HEAD") == "HEAD"
):
return self.create_matcherror(tag="latest[git]")
if (
task["action"]["__ansible_module__"] == "hg"
and task["action"].get("revision", "default") == "default"
):
return self.create_matcherror(tag="latest[hg]")
return False
3 changes: 1 addition & 2 deletions src/ansiblelint/schemas/ansible-lint-config.json
Original file line number Diff line number Diff line change
Expand Up @@ -94,8 +94,7 @@
"deprecated-module",
"empty-string-compare",
"fqcn-builtins",
"git-latest",
"hg-latest",
"latest",
"ignore-errors",
"inline-env-var",
"internal-error",
Expand Down
8 changes: 4 additions & 4 deletions test/test_skip_inside_yaml.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,23 +13,23 @@

ROLE_TASKS_WITH_BLOCK = """\
---
- name: Bad git 1 # noqa git-latest
- name: Bad git 1 # noqa latest[git]
action: ansible.builtin.git a=b c=d
- name: Bad git 2
action: ansible.builtin.git a=b c=d
- name: Block with rescue and always section
block:
- name: Bad git 3 # noqa git-latest
- name: Bad git 3 # noqa latest[git]
action: ansible.builtin.git a=b c=d
- name: Bad git 4
action: ansible.builtin.git a=b c=d
rescue:
- name: Bad git 5 # noqa git-latest
- name: Bad git 5 # noqa latest[git]
action: ansible.builtin.git a=b c=d
- name: Bad git 6
action: ansible.builtin.git a=b c=d
always:
- name: Bad git 7 # noqa git-latest
- name: Bad git 7 # noqa latest[git]
action: ansible.builtin.git a=b c=d
- name: Bad git 8
action: ansible.builtin.git a=b c=d
Expand Down

0 comments on commit 58964c9

Please sign in to comment.