From 3d4f3a3a2828aaf6e2d367930482ea4c666b03ae Mon Sep 17 00:00:00 2001
From: Albin Vass <albin.vass@gmail.com>
Date: Wed, 13 May 2020 09:19:03 +0200
Subject: [PATCH] Add linting rule to enforce no-same-owner policy
Change-Id: I92c66a21be95935d11fc8e9887d9d91c645d28d4
---
.rules/ZuulJobsNoSameOwner.py | 81 +++++++++++++++++++
doc/source/policy.rst | 34 +++++---
.../faulty/roles/block/tasks/main.yaml | 4 +
.../faulty/roles/nested-block/tasks/main.yaml | 5 ++
.../faulty/roles/synchronize/tasks/main.yaml | 3 +
.../roles/unarchive-bz2/tasks/main.yaml | 3 +
.../roles/unarchive-delegated/tasks/main.yaml | 4 +
.../faulty/roles/unarchive-gz/tasks/main.yaml | 3 +
.../roles/unarchive-tar/tasks/main.yaml | 3 +
.../faulty/roles/unarchive-xz/tasks/main.yaml | 3 +
.../unarchive-zip-same-owner/tasks/main.yaml | 6 ++
.../roles/unarchive-zip/tasks/main.yaml | 5 ++
.../synchronize-delegated/tasks/main.yaml | 4 +
.../synchronize-no-same-owner/tasks/main.yaml | 5 ++
.../unarchive-no-same-owner/tasks/main.yaml | 5 ++
.../unarchive-remote-src/tasks/main.yaml | 4 +
.../tasks/main.yaml | 3 +
17 files changed, 163 insertions(+), 12 deletions(-)
create mode 100644 .rules/ZuulJobsNoSameOwner.py
create mode 100644 test-playbooks/ansible-lint-rules/ZUULJOBS0002/faulty/roles/block/tasks/main.yaml
create mode 100644 test-playbooks/ansible-lint-rules/ZUULJOBS0002/faulty/roles/nested-block/tasks/main.yaml
create mode 100644 test-playbooks/ansible-lint-rules/ZUULJOBS0002/faulty/roles/synchronize/tasks/main.yaml
create mode 100644 test-playbooks/ansible-lint-rules/ZUULJOBS0002/faulty/roles/unarchive-bz2/tasks/main.yaml
create mode 100644 test-playbooks/ansible-lint-rules/ZUULJOBS0002/faulty/roles/unarchive-delegated/tasks/main.yaml
create mode 100644 test-playbooks/ansible-lint-rules/ZUULJOBS0002/faulty/roles/unarchive-gz/tasks/main.yaml
create mode 100644 test-playbooks/ansible-lint-rules/ZUULJOBS0002/faulty/roles/unarchive-tar/tasks/main.yaml
create mode 100644 test-playbooks/ansible-lint-rules/ZUULJOBS0002/faulty/roles/unarchive-xz/tasks/main.yaml
create mode 100644 test-playbooks/ansible-lint-rules/ZUULJOBS0002/faulty/roles/unarchive-zip-same-owner/tasks/main.yaml
create mode 100644 test-playbooks/ansible-lint-rules/ZUULJOBS0002/faulty/roles/unarchive-zip/tasks/main.yaml
create mode 100644 test-playbooks/ansible-lint-rules/ZUULJOBS0002/valid/roles/synchronize-delegated/tasks/main.yaml
create mode 100644 test-playbooks/ansible-lint-rules/ZUULJOBS0002/valid/roles/synchronize-no-same-owner/tasks/main.yaml
create mode 100644 test-playbooks/ansible-lint-rules/ZUULJOBS0002/valid/roles/unarchive-no-same-owner/tasks/main.yaml
create mode 100644 test-playbooks/ansible-lint-rules/ZUULJOBS0002/valid/roles/unarchive-remote-src/tasks/main.yaml
create mode 100644 test-playbooks/ansible-lint-rules/ZUULJOBS0002/valid/roles/unarchive-unknown-file-ending/tasks/main.yaml
diff --git a/.rules/ZuulJobsNoSameOwner.py b/.rules/ZuulJobsNoSameOwner.py
new file mode 100644
index 000000000..bbe05a23a
--- /dev/null
+++ b/.rules/ZuulJobsNoSameOwner.py
@@ -0,0 +1,81 @@
+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
diff --git a/doc/source/policy.rst b/doc/source/policy.rst
index 184ae403c..5cd809cc6 100644
--- a/doc/source/policy.rst
+++ b/doc/source/policy.rst
@@ -219,20 +219,30 @@ group should not be preserved when transfering files between them.
For example when using the synchronize module set owner and group
to ``false``::
- synchronize:
- dest: /tmp/log.txt
- src: /tmp/log.txt
- owner: false
- group: false
+ - name: valid
+ synchronize:
+ dest: /tmp/log.txt
+ src: /tmp/log.txt
+ owner: false
+ group: false
-And when using the unarchive module add ``--no-same-owner`` to
-extra-ops::
+When using the unarchive module add ``--no-same-owner`` to extra_opts
+when handling tarballs and do not use ``-X`` when handling zipfiles::
+
+ - name: valid
+ unarchive:
+ dest: ~/example
+ src: /tmp/example.tar.gz
+ extra_opts:
+ - '--no-same-owner'
+
+ - name: faulty
+ unarchive:
+ dest: ~/example
+ src: /tmp/example.zip
+ extra_opts:
+ - '-X'
- unarchive:
- dest: ~/example
- src: /tmp/example.tar.gz
- extra_ops:
- - '--no-same-owner'
Testing
-------
diff --git a/test-playbooks/ansible-lint-rules/ZUULJOBS0002/faulty/roles/block/tasks/main.yaml b/test-playbooks/ansible-lint-rules/ZUULJOBS0002/faulty/roles/block/tasks/main.yaml
new file mode 100644
index 000000000..fc5d65f19
--- /dev/null
+++ b/test-playbooks/ansible-lint-rules/ZUULJOBS0002/faulty/roles/block/tasks/main.yaml
@@ -0,0 +1,4 @@
+- block:
+ - synchronize:
+ src: dummy
+ dest: dummy
diff --git a/test-playbooks/ansible-lint-rules/ZUULJOBS0002/faulty/roles/nested-block/tasks/main.yaml b/test-playbooks/ansible-lint-rules/ZUULJOBS0002/faulty/roles/nested-block/tasks/main.yaml
new file mode 100644
index 000000000..bf28591b9
--- /dev/null
+++ b/test-playbooks/ansible-lint-rules/ZUULJOBS0002/faulty/roles/nested-block/tasks/main.yaml
@@ -0,0 +1,5 @@
+- block:
+ - block:
+ - synchronize:
+ src: dummy
+ dest: dummy
diff --git a/test-playbooks/ansible-lint-rules/ZUULJOBS0002/faulty/roles/synchronize/tasks/main.yaml b/test-playbooks/ansible-lint-rules/ZUULJOBS0002/faulty/roles/synchronize/tasks/main.yaml
new file mode 100644
index 000000000..7a2b8091f
--- /dev/null
+++ b/test-playbooks/ansible-lint-rules/ZUULJOBS0002/faulty/roles/synchronize/tasks/main.yaml
@@ -0,0 +1,3 @@
+- synchronize:
+ src: dummy
+ dest: dummy
diff --git a/test-playbooks/ansible-lint-rules/ZUULJOBS0002/faulty/roles/unarchive-bz2/tasks/main.yaml b/test-playbooks/ansible-lint-rules/ZUULJOBS0002/faulty/roles/unarchive-bz2/tasks/main.yaml
new file mode 100644
index 000000000..fbd47081a
--- /dev/null
+++ b/test-playbooks/ansible-lint-rules/ZUULJOBS0002/faulty/roles/unarchive-bz2/tasks/main.yaml
@@ -0,0 +1,3 @@
+- unarchive:
+ src: "{{ file }}.tar.bz2"
+ dest: "dummy"
diff --git a/test-playbooks/ansible-lint-rules/ZUULJOBS0002/faulty/roles/unarchive-delegated/tasks/main.yaml b/test-playbooks/ansible-lint-rules/ZUULJOBS0002/faulty/roles/unarchive-delegated/tasks/main.yaml
new file mode 100644
index 000000000..bb4379055
--- /dev/null
+++ b/test-playbooks/ansible-lint-rules/ZUULJOBS0002/faulty/roles/unarchive-delegated/tasks/main.yaml
@@ -0,0 +1,4 @@
+- unarchive:
+ src: "{{ file }}.tar.bz2"
+ dest: "dummy"
+ delegate_to: localhost
diff --git a/test-playbooks/ansible-lint-rules/ZUULJOBS0002/faulty/roles/unarchive-gz/tasks/main.yaml b/test-playbooks/ansible-lint-rules/ZUULJOBS0002/faulty/roles/unarchive-gz/tasks/main.yaml
new file mode 100644
index 000000000..289e85675
--- /dev/null
+++ b/test-playbooks/ansible-lint-rules/ZUULJOBS0002/faulty/roles/unarchive-gz/tasks/main.yaml
@@ -0,0 +1,3 @@
+- unarchive:
+ src: "{{ file }}.tar.gz"
+ dest: "dummy"
diff --git a/test-playbooks/ansible-lint-rules/ZUULJOBS0002/faulty/roles/unarchive-tar/tasks/main.yaml b/test-playbooks/ansible-lint-rules/ZUULJOBS0002/faulty/roles/unarchive-tar/tasks/main.yaml
new file mode 100644
index 000000000..72759bb2e
--- /dev/null
+++ b/test-playbooks/ansible-lint-rules/ZUULJOBS0002/faulty/roles/unarchive-tar/tasks/main.yaml
@@ -0,0 +1,3 @@
+- unarchive:
+ src: "{{ file }}.tar"
+ dest: "dummy"
diff --git a/test-playbooks/ansible-lint-rules/ZUULJOBS0002/faulty/roles/unarchive-xz/tasks/main.yaml b/test-playbooks/ansible-lint-rules/ZUULJOBS0002/faulty/roles/unarchive-xz/tasks/main.yaml
new file mode 100644
index 000000000..bc9e54a32
--- /dev/null
+++ b/test-playbooks/ansible-lint-rules/ZUULJOBS0002/faulty/roles/unarchive-xz/tasks/main.yaml
@@ -0,0 +1,3 @@
+- unarchive:
+ src: "{{ file }}.tar.xz"
+ dest: "dummy"
diff --git a/test-playbooks/ansible-lint-rules/ZUULJOBS0002/faulty/roles/unarchive-zip-same-owner/tasks/main.yaml b/test-playbooks/ansible-lint-rules/ZUULJOBS0002/faulty/roles/unarchive-zip-same-owner/tasks/main.yaml
new file mode 100644
index 000000000..72a08bb6b
--- /dev/null
+++ b/test-playbooks/ansible-lint-rules/ZUULJOBS0002/faulty/roles/unarchive-zip-same-owner/tasks/main.yaml
@@ -0,0 +1,6 @@
+- unarchive:
+ src: "{{ file }}.zip"
+ dest: dummy
+ extra_opts:
+ - '-X'
+
diff --git a/test-playbooks/ansible-lint-rules/ZUULJOBS0002/faulty/roles/unarchive-zip/tasks/main.yaml b/test-playbooks/ansible-lint-rules/ZUULJOBS0002/faulty/roles/unarchive-zip/tasks/main.yaml
new file mode 100644
index 000000000..42fbeb7d1
--- /dev/null
+++ b/test-playbooks/ansible-lint-rules/ZUULJOBS0002/faulty/roles/unarchive-zip/tasks/main.yaml
@@ -0,0 +1,5 @@
+- unarchive:
+ src: "{{ file }}.zip"
+ dest: dummy
+ extra_opts:
+ - '-X'
diff --git a/test-playbooks/ansible-lint-rules/ZUULJOBS0002/valid/roles/synchronize-delegated/tasks/main.yaml b/test-playbooks/ansible-lint-rules/ZUULJOBS0002/valid/roles/synchronize-delegated/tasks/main.yaml
new file mode 100644
index 000000000..a1abdd62b
--- /dev/null
+++ b/test-playbooks/ansible-lint-rules/ZUULJOBS0002/valid/roles/synchronize-delegated/tasks/main.yaml
@@ -0,0 +1,4 @@
+- synchronize:
+ src: dummy
+ dest: dummy
+ delegate_to: localhost
diff --git a/test-playbooks/ansible-lint-rules/ZUULJOBS0002/valid/roles/synchronize-no-same-owner/tasks/main.yaml b/test-playbooks/ansible-lint-rules/ZUULJOBS0002/valid/roles/synchronize-no-same-owner/tasks/main.yaml
new file mode 100644
index 000000000..9f0fc7671
--- /dev/null
+++ b/test-playbooks/ansible-lint-rules/ZUULJOBS0002/valid/roles/synchronize-no-same-owner/tasks/main.yaml
@@ -0,0 +1,5 @@
+- synchronize:
+ src: dummy
+ dest: dummy
+ owner: no
+ group: no
diff --git a/test-playbooks/ansible-lint-rules/ZUULJOBS0002/valid/roles/unarchive-no-same-owner/tasks/main.yaml b/test-playbooks/ansible-lint-rules/ZUULJOBS0002/valid/roles/unarchive-no-same-owner/tasks/main.yaml
new file mode 100644
index 000000000..b42e89666
--- /dev/null
+++ b/test-playbooks/ansible-lint-rules/ZUULJOBS0002/valid/roles/unarchive-no-same-owner/tasks/main.yaml
@@ -0,0 +1,5 @@
+- unarchive:
+ src: "{{ file }}.tar.gz"
+ dest: dummy
+ extra_opts:
+ - '--no-same-owner'
diff --git a/test-playbooks/ansible-lint-rules/ZUULJOBS0002/valid/roles/unarchive-remote-src/tasks/main.yaml b/test-playbooks/ansible-lint-rules/ZUULJOBS0002/valid/roles/unarchive-remote-src/tasks/main.yaml
new file mode 100644
index 000000000..556217250
--- /dev/null
+++ b/test-playbooks/ansible-lint-rules/ZUULJOBS0002/valid/roles/unarchive-remote-src/tasks/main.yaml
@@ -0,0 +1,4 @@
+- unarchive:
+ src: "{{ file }}.tar.xz"
+ dest: "dummy"
+ remote_src: true
diff --git a/test-playbooks/ansible-lint-rules/ZUULJOBS0002/valid/roles/unarchive-unknown-file-ending/tasks/main.yaml b/test-playbooks/ansible-lint-rules/ZUULJOBS0002/valid/roles/unarchive-unknown-file-ending/tasks/main.yaml
new file mode 100644
index 000000000..f237d57d7
--- /dev/null
+++ b/test-playbooks/ansible-lint-rules/ZUULJOBS0002/valid/roles/unarchive-unknown-file-ending/tasks/main.yaml
@@ -0,0 +1,3 @@
+- unarchive:
+ src: "{{ file }}"
+ dest: "dummy"