Upgrade ansible-lint to 5.0

- bumps ansible-lint to 5.0
- updates our custom rules to make them compatible with 5.0
- replace custom module mocking with native ansible-lint ones
- remove custom call of ansible-playbook --syntax-check as now this
  is done by ansible-lint
- assured molecule vars are hosted under a vars/ folder in order to
  avoid confusing linter detection.
- replaced custom rule for loop var names in role as now this this an
  optional core feature of the linter (see config)
- replaced custom rule no-same-owner with opt-in one (see config)

Change-Id: I233fae8c9036d295968a97ee80e07fde8846c633
This commit is contained in:
Sorin Sbarnea 2021-01-08 08:57:26 +00:00
parent 78f18243f1
commit 0eaa5cf59a
54 changed files with 30 additions and 436 deletions

View File

@ -3,11 +3,16 @@ exclude_paths:
parseable: true
quiet: false
skip_list:
- '106' # Role name does not match ``^[a-z][a-z0-9_]+$`` pattern
- '204' # Lines should be no longer than 160 chars
- '301' # Commands should not change things if nothing needs doing
- '701' # No 'galaxy_info' found
rulesdir:
- ./.rules/
- meta-no-info # No 'galaxy_info' found
- no-changed-when # Commands should not change things if nothing needs doing
- no-tabs # Most files should not contain tabs
- role-name # Role name does not match ``^[a-z][a-z0-9_]+$`` pattern
use_default_rules: true
verbosity: 1
mock_modules:
- zuul_console
- zuul_return
loop_var_prefix: zj_
# Enable rules that are disabled by default:
enable_list:
- no-same-owner

View File

@ -1,55 +0,0 @@
from ansiblelint import AnsibleLintRule
class ZuulJobsNamespaceLoopVar(AnsibleLintRule):
id = 'ZUULJOBS0001'
shortdesc = 'Loop vars should have zj_ prefix'
description = """
Check for tasks that does not follow
the policy of namespacing loop variables with zj_ prefix.
See: \
https://zuul-ci.org/docs/zuul-jobs/policy.html\
#loops-in-roles
"""
tags = {'zuul-jobs-namespace-loop-var'}
def matchplay(self, file, task):
results = []
if file.get('type') not in ('tasks', 'handlers'):
return results
results.extend(self.handle_play(task))
return results
def handle_play(self, task):
results = []
if 'block' in task:
results.extend(self.handle_playlist(task['block']))
else:
results.extend(self.handle_task(task))
return results
def handle_playlist(self, playlist):
results = []
for play in playlist:
results.extend(self.handle_play(play))
return results
def handle_task(self, task):
results = []
has_loop = 'loop' in task
for key in task.keys():
if key.startswith('with_'):
has_loop = True
if has_loop:
if 'loop_control' not in task:
results.append(("", self.shortdesc))
elif 'loop_var' not in task.get('loop_control'):
results.append(("", self.shortdesc))
elif not task.get('loop_control')\
.get('loop_var').startswith('zj_'):
results.append(("", self.shortdesc))
return results

View File

@ -1,81 +0,0 @@
import re
from ansiblelint import AnsibleLintRule
class ZuulJobsNoSameOwner(AnsibleLintRule):
id = 'ZUULJOBS0002'
shortdesc = 'Owner should not be kept between executor and remote'
description = """
Since there is no way to guarantee that the user and or group on the remote
node also exist on the executor and vice versa, owner and group should not
be preserved when transfering files between them.
See:
https://zuul-ci.org/docs/zuul-jobs/policy.html\
#preservation-of-owner-between-executor-and-remote
"""
tags = {'zuul-jobs-no-same-owner'}
def matchplay(self, file, play):
results = []
if file.get('type') not in ('tasks',
'handlers',
'playbooks'):
return results
results.extend(self.handle_play(play))
return results
def handle_play(self, task):
results = []
if 'block' in task:
results.extend(self.handle_playlist(task['block']))
else:
results.extend(self.handle_task(task))
return results
def handle_playlist(self, playlist):
results = []
for play in playlist:
results.extend(self.handle_play(play))
return results
def handle_task(self, task):
results = []
if 'synchronize' in task:
if self.handle_synchronize(task):
results.append(("", self.shortdesc))
elif 'unarchive' in task:
if self.handle_unarchive(task):
results.append(("", self.shortdesc))
return results
def handle_synchronize(self, task):
if task.get('delegate_to') is not None:
return False
synchronize = task['synchronize']
archive = synchronize.get('archive', True)
if synchronize.get('owner', archive) or\
synchronize.get('group', archive):
return True
return False
def handle_unarchive(self, task):
unarchive = task['unarchive']
delegate_to = task.get('delegate_to')
if delegate_to == 'localhost' or\
delegate_to != 'localhost' and 'remote_src' not in unarchive:
if unarchive['src'].endswith('zip'):
if '-X' in unarchive.get('extra_opts', []):
return True
if re.search(r'.*\.tar(\.(gz|bz2|xz))?$', unarchive['src']):
if '--no-same-owner' not in unarchive.get('extra_opts', []):
return True
return False

View File

@ -1,5 +1,6 @@
# linters have different requirements than test ones, some would
# conflict, like ansible version required by ansible-lint.
ansible>=2.9,<2.11 # required by ansible-lint
flake8
yamllint>=1.23.0
ansible-lint>=4.3.7,<5
ansible-lint>=5.0.3rc1,<5.1

View File

@ -19,7 +19,7 @@
register: bazelisk_downloaded
# This will apply to further plays and playbooks
- name: Set bazelisk_executable fact
- name: Set bazelisk_executable fact # noqa no-handler
set_fact:
bazelisk_executable: "{{ bazelisk_target }}"
cacheable: true

View File

@ -25,7 +25,7 @@
loop_var: zj_item
register: _add_apt_repos
- name: Update APT cache
- name: Update APT cache # noqa no-handler
become: true
apt:
update_cache: yes

View File

@ -1,5 +1,5 @@
- name: Install Rust
shell: | # noqa 303
shell: | # noqa command-instead-of-module
set -o pipefail
curl -sSf https://sh.rustup.rs | sh -s -- -y --no-modify-path --default-toolchain {{ ensure_rust_rustup_toolchain }}
environment:

View File

@ -1,5 +1,5 @@
- name: Check if unzip is installed
command: unzip -v # noqa 303
command: unzip -v # noqa command-instead-of-module
failed_when: false
register: _unzip_probe

View File

@ -8,7 +8,7 @@
group: no
when: not zuul_use_fetch_output
- name: Copy sphinx build html # noqa 208
- name: Copy sphinx build html # noqa risky-file-permissions
copy:
dest: "{{ zuul_output_dir }}/logs/"
src: "{{ sphinx_output_src }}"

View File

@ -62,7 +62,7 @@
envlist: "{{ tox_all_environments.stdout_lines }}"
when: tox_all_environments.stdout_lines is defined
- name: Copy tox logs # noqa 208
- name: Copy tox logs # noqa risky-file-permissions
copy:
dest: "{{ zuul_output_dir }}/logs/tox/"
src: "{{ zuul_work_dir }}/.tox/{{ zj_testenv }}/log/"

View File

@ -9,7 +9,7 @@
# try to delete the next file.
- name: Remove sudo access for zuul user.
become: yes
command: rm -rf /etc/sudoers.d/zuul /etc/sudoers.d/90-cloud-init-users # noqa 302
command: rm -rf /etc/sudoers.d/zuul /etc/sudoers.d/90-cloud-init-users # noqa deprecated-command-syntax
when: zuul_is_sudoer.rc == 0
- name: Prove that general sudo access is actually revoked.

View File

@ -89,7 +89,7 @@
- name: Get path to main.tf relative to the repo root
when: terraform_command == "plan"
register: main_file_location
command: "git ls-files --full-name main.tf" # noqa 303
command: "git ls-files --full-name main.tf" # noqa command-instead-of-module
args:
chdir: "{{ zuul_work_dir }}"

View File

@ -42,7 +42,7 @@
dest: "{{ ca_dir }}/{{ buildset_registry_alias }}.crt"
mode: 0644
register: _tls_ca
- name: Update CA certs
- name: Update CA certs # noqa: no-handler
command: "{{ ca_command }}"
become: true
when: _tls_ca is changed

View File

@ -1,5 +0,0 @@
- debug:
msg: "I should fail: {{ item }}"
loop:
- 1
- 2

View File

@ -1,5 +0,0 @@
- debug:
msg: "I should fail: {{ item }}"
with_list:
- 1
- 2

View File

@ -1,6 +0,0 @@
- block:
- debug:
var: item
loop:
- 1
- 2

View File

@ -1,6 +0,0 @@
- block:
- debug:
var: item
with_items:
- 1
- 2

View File

@ -1,7 +0,0 @@
- block:
- block:
- debug:
var: item
loop:
- 1
- 2

View File

@ -1,4 +0,0 @@
- include: "{{ item }}.yaml"
with_list:
- 1
- 2

View File

@ -1,5 +0,0 @@
- debug:
msg: "I should error: {{ item }} "
loop:
- 1
- 2

View File

@ -1,5 +0,0 @@
- debug:
msg: "I should error: {{ item }} "
with_list:
- 1
- 2

View File

@ -1,8 +0,0 @@
- debug:
msg: "I should pass: {{ zj_item }}"
loop:
- 1
- 2
loop_control:
loop_var: zj_item

View File

@ -1,8 +0,0 @@
- debug:
msg: "I should pass: {{ zj_item }}"
with_list:
- 1
- 2
loop_control:
loop_var: zj_item

View File

@ -1,8 +0,0 @@
- block:
- debug:
msg: zj_item
loop:
- 1
- 2
loop_control:
loop_var: zj_item

View File

@ -1,8 +0,0 @@
- block:
- debug:
msg: zj_item
with_items:
- 1
- 2
loop_control:
loop_var: zj_item

View File

@ -1,5 +0,0 @@
- debug: # noqa ZUULJOBS0001
var: item
loop:
- 1
- 2

View File

@ -1,7 +0,0 @@
- include: "{{ zj_item }}.yaml"
loop:
- 1
- 2
loop_control:
loop_var: zj_item

View File

@ -1,6 +0,0 @@
- include: "{{ zj_item }}.yaml"
with_list:
- 1
- 2
loop_control:
loop_var: zj_item

View File

@ -1,8 +0,0 @@
- debug:
msg: "I should pass: {{ zj_item }} "
loop:
- 1
- 2
loop_control:
loop_var: zj_item

View File

@ -1,7 +0,0 @@
- debug:
msg: "I should pass: {{ zj_item }} "
with_list:
- 1
- 2
loop_control:
loop_var: zj_item

View File

@ -1,4 +0,0 @@
- block:
- synchronize:
src: dummy
dest: dummy

View File

@ -1,5 +0,0 @@
- block:
- block:
- synchronize:
src: dummy
dest: dummy

View File

@ -1,3 +0,0 @@
- synchronize:
src: dummy
dest: dummy

View File

@ -1,3 +0,0 @@
- unarchive:
src: "{{ file }}.tar.bz2"
dest: "dummy"

View File

@ -1,4 +0,0 @@
- unarchive:
src: "{{ file }}.tar.bz2"
dest: "dummy"
delegate_to: localhost

View File

@ -1,3 +0,0 @@
- unarchive:
src: "{{ file }}.tar.gz"
dest: "dummy"

View File

@ -1,3 +0,0 @@
- unarchive:
src: "{{ file }}.tar"
dest: "dummy"

View File

@ -1,3 +0,0 @@
- unarchive:
src: "{{ file }}.tar.xz"
dest: "dummy"

View File

@ -1,6 +0,0 @@
- unarchive:
src: "{{ file }}.zip"
dest: dummy
extra_opts:
- '-X'

View File

@ -1,5 +0,0 @@
- unarchive:
src: "{{ file }}.zip"
dest: dummy
extra_opts:
- '-X'

View File

@ -1,4 +0,0 @@
- synchronize:
src: dummy
dest: dummy
delegate_to: localhost

View File

@ -1,5 +0,0 @@
- synchronize:
src: dummy
dest: dummy
owner: no
group: no

View File

@ -1,5 +0,0 @@
- unarchive:
src: "{{ file }}.tar.gz"
dest: dummy
extra_opts:
- '--no-same-owner'

View File

@ -1,4 +0,0 @@
- unarchive:
src: "{{ file }}.tar.xz"
dest: "dummy"
remote_src: true

View File

@ -1,3 +0,0 @@
- unarchive:
src: "{{ file }}"
dest: "dummy"

View File

@ -1,5 +0,0 @@
parseable: true
quiet: false
rulesdir:
- ../../.rules/
verbosity: 1

View File

@ -1,61 +0,0 @@
- hosts: all
roles:
- ensure-pip
- ensure-virtualenv
tasks:
- name: Create tempdir for ansible-lint venv
tempfile:
state: directory
register: ansible_lint_tempdir
- name: Install linters-requirements.txt
pip:
requirements: "{{ ansible_user_dir }}/{{ zuul.project.src_dir }}/linters-requirements.txt"
virtualenv: "{{ ansible_lint_tempdir.path }}"
- name: Make sure ansible-lint is installed
command: "{{ ansible_lint_tempdir.path }}/bin/ansible-lint --version"
- name: Get faulty playbooks and roles
command: >-
find test-playbooks/ansible-lint-rules/
-mindepth 4
-maxdepth 4
-wholename '*ZUULJOBS*/faulty/*/*'
args:
chdir: "{{ ansible_user_dir }}/{{ zuul.project.src_dir }}"
register: faulty_ansible_items
- name: Get valid playbooks and roles
command: >-
find test-playbooks/ansible-lint-rules/
-mindepth 4
-maxdepth 4
-wholename '*ZUULJOBS*/valid/roles/*'
args:
chdir: "{{ ansible_user_dir }}/{{ zuul.project.src_dir }}"
register: valid_ansible_items
- name: Make sure faulty roles fail linting
command: >-
{{ ansible_lint_tempdir.path }}/bin/ansible-lint
-c test-playbooks/ansible-lint-rules/ansible-lint.yaml
-t {{ item | regex_replace('.*/(ZUULJOBS.*?)/.*', '\1') }}
{{ item }}
args:
chdir: "{{ ansible_user_dir }}/{{ zuul.project.src_dir }}"
register: ansible_lint
failed_when: ansible_lint.rc == 0
loop: "{{ faulty_ansible_items.stdout_lines }}"
- name: Make sure valid roles pass linting
command: >-
{{ ansible_lint_tempdir.path }}/bin/ansible-lint
-c test-playbooks/ansible-lint-rules/ansible-lint.yaml
-t {{ item | regex_replace('.*/(ZUULJOBS.*?)/.*', '\1') }}
{{ item }}
args:
chdir: "{{ ansible_user_dir }}/{{ zuul.project.src_dir }}"
register: ansible_lint
failed_when: ansible_lint.rc != 0
loop: "{{ valid_ansible_items.stdout_lines }}"

View File

@ -19,7 +19,6 @@
tox_envlist: docs,linters
tox_environment:
ANSIBLE_ROLES_PATH: "{{ ansible_user_dir }}/{{ zuul.project.src_dir }}/roles"
ANSIBLE_LIBRARY: "{{ ansible_user_dir }}/{{ zuul.project.src_dir }}/tests/fake-ansible"
- name: Create tempfile to verify testenvs ran
tempfile:

View File

@ -1,6 +0,0 @@
# This is a fake zuul_return to make ansible-lint happy
from ansible.module_utils.basic import AnsibleModule
def main():
return AnsibleModule()

View File

@ -1,12 +0,0 @@
# This is a fake zuul_return to make ansible-lint happy
from ansible.module_utils.basic import AnsibleModule
def main():
return AnsibleModule(
argument_spec=dict(
data=dict(default=None),
path=dict(default=None, type=str),
file=dict(default=None, type=str),
)
)

View File

@ -38,8 +38,9 @@ passenv =
# to export ANSIBLE_ROLES_PATH pointing to the currect repos.
# see openstack-zuul-jobs-linters job for more information.
ANSIBLE_ROLES_PATH
TERM
setenv =
ANSIBLE_LIBRARY= {toxinidir}/tests/fake-ansible
GIT_PAGER: cat
whitelist_externals = bash
deps =
-r{toxinidir}/linters-requirements.txt
@ -50,8 +51,6 @@ commands =
python -m ansiblelint --version
python -m ansiblelint {env:ANSIBLELINT_OPTS:--progressive}
# Ansible Syntax Check
bash -c "find playbooks -type f -regex '.*.ya?ml' ! -regex '.*vars\/.*' -exec \
ansible-playbook --syntax-check -i {toxinidir}/tests/inventory \{\} + > /dev/null"
{toxinidir}/tools/check_jobs_documented.py
{toxinidir}/tools/update-test-platforms.py
bash -c "(( $(find playbooks -name *.yml | wc -l) == 0)) || \{ echo 'Use .yaml'; exit 1; \}"

View File

@ -1,18 +0,0 @@
- job:
name: zuul-jobs-test-ansible-lint-rules
description: |
Test custom ansible-lint rules in zuul-jobs
run: test-playbooks/ansible-lint-rules/run.yaml
files:
- ^\.rules/.*
- ^test-playbooks/ansible-lint-rules/.*
- ^\.ansible-lint
- project:
check:
jobs: &id001
- zuul-jobs-test-ansible-lint-rules
gate:
jobs: *id001
periodic-weekly:
jobs: *id001