Use unique loop vars to avoid conflicts

We have to be careful about avoiding outer loop loop_var conflicts in
ansible. Because the zuul-jobs roles are meant to be reconsumed
elsewhere we should not use 'item' loopvars and instead set them to
something a bit more unique.

We use a zj_ prefix to try and be unique to this repo and document this
convention.

Change-Id: I20b9327a914890e9eafcb2b36dc8c23fb472bc8f
This commit is contained in:
Clark Boylan 2020-02-01 14:40:51 -08:00
parent e323dc117b
commit 4ed66807a0
9 changed files with 50 additions and 14 deletions

View File

@ -153,6 +153,30 @@ If relevant, the specific steps where the privilege escalation occurs should be
documented so that they can be reproduced when configuring hosts. If possible, documented so that they can be reproduced when configuring hosts. If possible,
they should be grouped in a separate playbook that can be applied to hosts manually. they should be grouped in a separate playbook that can be applied to hosts manually.
Ansible Loops in Roles
**********************
Because the Ansible roles contained in this repo are expected to be
pretty universally applicable in Zuul systems we must write them
defensively to work around some Ansible behaviors. In particular
nesting Ansible loops using the default `loop_var` of `item` is not
safe.
Roles in this repo should override the default `loop_var` in loops
and use a variable name prefixed with `zj_` to make them more unique.
The idea is this will avoid conflicts with the calling level which
may use `include_role` in a loop creating a `loop_var` conflict.
For example::
command: echo {{ zj_number }}
loop:
- one
- two
- three
loop_control:
loop_var: zj_number
Installing Dependencies in Roles Installing Dependencies in Roles
******************************** ********************************

View File

@ -13,8 +13,8 @@
# We can't use the default 'item' because roles may be used in # We can't use the default 'item' because roles may be used in
# higher level loops and 'item' could conflict in that case. # higher level loops and 'item' could conflict in that case.
loop_control: loop_control:
loop_var: loop_container_name loop_var: zj_container_name
shell: "{{ container_command }} logs {{ loop_container_name }} &> {{ ansible_user_dir }}/zuul-output/logs/{{ container_command }}/{{ loop_container_name }}.txt" shell: "{{ container_command }} logs {{ zj_container_name }} &> {{ ansible_user_dir }}/zuul-output/logs/{{ container_command }}/{{ zj_container_name }}.txt"
args: args:
executable: /bin/bash executable: /bin/bash
ignore_errors: true ignore_errors: true

View File

@ -1,10 +1,12 @@
- name: Verify repository names - name: Verify repository names
when: | when: |
docker_credentials.repository is defined docker_credentials.repository is defined
and not item.repository | regex_search(docker_credentials.repository) and not zj_image.repository | regex_search(docker_credentials.repository)
loop: "{{ docker_images }}" loop: "{{ docker_images }}"
loop_control:
loop_var: zj_image
fail: fail:
msg: "{{ item.repository }} not permitted by {{ docker_credentials.repository }}" msg: "{{ zj_image.repository }} not permitted by {{ docker_credentials.repository }}"
# This is used by the delete tasks # This is used by the delete tasks
- name: Get dockerhub JWT token - name: Get dockerhub JWT token
no_log: true no_log: true

View File

@ -84,13 +84,15 @@
- name: Pull artifacts from intermediate registry - name: Pull artifacts from intermediate registry
command: >- command: >-
skopeo --insecure-policy copy skopeo --insecure-policy copy
{{ item.url }} {{ zj_zuul_artifact.url }}
docker://127.0.0.1:{{ socat_port }}/{{ item.metadata.repository | regex_replace('^docker\.io/(.*)', '\1') }}:{{ item.metadata.tag }} docker://127.0.0.1:{{ socat_port }}/{{ zj_zuul_artifact.metadata.repository | regex_replace('^docker\.io/(.*)', '\1') }}:{{ zj_zuul_artifact.metadata.tag }}
retries: 3 retries: 3
register: result register: result
until: result is success until: result is success
when: "'metadata' in item and item.metadata.type | default('') == 'container_image'" when: "'metadata' in zj_zuul_artifact and zj_zuul_artifact.metadata.type | default('') == 'container_image'"
loop: "{{ zuul.artifacts | default([]) }}" loop: "{{ zuul.artifacts | default([]) }}"
loop_control:
loop_var: zj_zuul_artifact
always: always:
- name: Remove docker user config - name: Remove docker user config
command: "shred ~/.docker/config.json" command: "shred ~/.docker/config.json"

View File

@ -19,10 +19,12 @@
- name: Ensure registry volume directories exists - name: Ensure registry volume directories exists
file: file:
state: directory state: directory
path: "{{ buildset_registry_root }}/{{ item }}" path: "{{ buildset_registry_root }}/{{ zj_dir }}"
loop: loop:
- tls - tls
- conf - conf
loop_control:
loop_var: zj_dir
- name: Generate registry secrets - name: Generate registry secrets
set_fact: set_fact:
registry_password: "{{ lookup('password', '/dev/null') }}" registry_password: "{{ lookup('password', '/dev/null') }}"

View File

@ -1,10 +1,12 @@
- name: Verify repository names - name: Verify repository names
when: | when: |
docker_credentials.repository is defined docker_credentials.repository is defined
and not item.repository | regex_search(docker_credentials.repository) and not zj_image.repository | regex_search(docker_credentials.repository)
loop: "{{ docker_images }}" loop: "{{ docker_images }}"
loop_control:
loop_var: zj_image
fail: fail:
msg: "{{ item.repository }} not permitted by {{ docker_credentials.repository }}" msg: "{{ zj_image.repository }} not permitted by {{ docker_credentials.repository }}"
- name: Log in to dockerhub - name: Log in to dockerhub
command: "docker login -u {{ docker_credentials.username }} -p {{ docker_credentials.password }}" command: "docker login -u {{ docker_credentials.username }} -p {{ docker_credentials.password }}"
no_log: true no_log: true

View File

@ -1,8 +1,8 @@
- name: Upload tag to dockerhub - name: Upload tag to dockerhub
command: "docker push {{ image.repository }}:change_{{ zuul.change }}_{{ image_tag }}" command: "docker push {{ image.repository }}:change_{{ zuul.change }}_{{ zj_image_tag }}"
loop: "{{ image.tags | default(['latest']) }}" loop: "{{ image.tags | default(['latest']) }}"
loop_control: loop_control:
loop_var: image_tag loop_var: zj_image_tag
register: result register: result
until: result.rc == 0 until: result.rc == 0
retries: 3 retries: 3

View File

@ -8,5 +8,7 @@
- name: Build zone.db file list - name: Build zone.db file list
set_fact: set_fact:
zone_db_files: '{{ zone_db_files + [ [item.path.split("/")[-2], item.path] ] }}' zone_db_files: '{{ zone_db_files + [ [zj_zone_db_found_file.path.split("/")[-2], zj_zone_db_found_file.path] ] }}'
loop: "{{ zone_db_found_files['files'] }}" loop: "{{ zone_db_found_files['files'] }}"
loop_control:
loop_var: zj_zone_db_found_file

View File

@ -9,5 +9,7 @@
when: not zone_db_files when: not zone_db_files
- name: 'Run checkzone' - name: 'Run checkzone'
command: '/usr/sbin/named-checkzone {{ item[0] }} {{ item[1] }}' command: '/usr/sbin/named-checkzone {{ zj_zone_db_file[0] }} {{ zj_zone_db_file[1] }}'
loop: "{{ zone_db_files }}" loop: "{{ zone_db_files }}"
loop_control:
loop_var: zj_zone_db_file