From 4d7a8240705c09e44d783c2e8e6fb6adee6d985a Mon Sep 17 00:00:00 2001 From: David Moreau Simard Date: Mon, 22 Jan 2018 17:27:53 -0500 Subject: [PATCH] Resolve Ansible variable precedence issue with include_vars This patch effectively works around an issue we're seeing with Ansible's var precedence with the "include -- with_first_found" pattern. TL;DR of what is happening is that a role, configure-unbound, has a "vars" directory which is meant to be used by an "include_vars -- with_first_found" task. However, since we have a "vars" directory at the playbook level, this directory has precedence over the one in the role. This makes it so the var files from the playbook directory are included instead of the one in the role. We wanted to address that by using {{ role_path }} within roles, but it turns out that Zuul's protection mechanisms is actually interfering [1]. Work around this for the time being. Since this is all not very straightforward, it ought to be documented properly somewhere -- this is in the README.rst file for the time being. Let's figure out a better place later. [1]: http://eavesdrop.openstack.org/irclogs/%23zuul/%23zuul.2018-01-15.log.html#t2018-01-15T22:44:13 Change-Id: Ia79a793200fcb89161783ff641b85106936084b5 --- roles/configure-unbound/tasks/main.yaml | 6 +++--- tests/multi-node-firewall-persistence.yaml | 10 +++++---- .../Debian.yaml | 0 .../README.rst | 21 +++++++++++++++++++ .../RedHat.yaml | 0 .../Suse.yaml | 0 .../Ubuntu_trusty.yaml | 0 .../default.yaml | 0 8 files changed, 30 insertions(+), 7 deletions(-) rename tests/{vars => multinode_firewall_persistence_vars}/Debian.yaml (100%) create mode 100644 tests/multinode_firewall_persistence_vars/README.rst rename tests/{vars => multinode_firewall_persistence_vars}/RedHat.yaml (100%) rename tests/{vars => multinode_firewall_persistence_vars}/Suse.yaml (100%) rename tests/{vars => multinode_firewall_persistence_vars}/Ubuntu_trusty.yaml (100%) rename tests/{vars => multinode_firewall_persistence_vars}/default.yaml (100%) diff --git a/roles/configure-unbound/tasks/main.yaml b/roles/configure-unbound/tasks/main.yaml index 64be2b8c..d8f6ef7e 100644 --- a/roles/configure-unbound/tasks/main.yaml +++ b/roles/configure-unbound/tasks/main.yaml @@ -41,9 +41,9 @@ - name: Include OS-specific variables include_vars: "{{ item }}" with_first_found: - - "{{ role_path }}/vars/{{ ansible_distribution }}.yaml" - - "{{ role_path }}/vars/{{ ansible_os_family }}.yaml" - - "{{ role_path }}/vars/default.yaml" + - "{{ ansible_distribution }}.yaml" + - "{{ ansible_os_family }}.yaml" + - "default.yaml" - name: Ensure Unbound conf.d directory exists become: yes diff --git a/tests/multi-node-firewall-persistence.yaml b/tests/multi-node-firewall-persistence.yaml index 3f646df7..cbbd4860 100644 --- a/tests/multi-node-firewall-persistence.yaml +++ b/tests/multi-node-firewall-persistence.yaml @@ -7,13 +7,15 @@ # it again -- we're testing here that both are persisted properly. - { role: multi-node-bridge, bridge_authorize_internal_traffic: true } post_tasks: + # NOTE (dmsimard): Using with_first_found and include_vars can yield + # unexpected results, see multinode_firewall_persistence_vars/README.rst - name: Include OS-specific variables include_vars: "{{ item }}" with_first_found: - - "{{ ansible_distribution }}_{{ ansible_distribution_release }}.yaml" - - "{{ ansible_distribution }}.yaml" - - "{{ ansible_os_family }}.yaml" - - "default.yaml" + - "multinode_firewall_persistence_vars/{{ ansible_distribution }}_{{ ansible_distribution_release }}.yaml" + - "multinode_firewall_persistence_vars/{{ ansible_distribution }}.yaml" + - "multinode_firewall_persistence_vars/{{ ansible_os_family }}.yaml" + - "multinode_firewall_persistence_vars/default.yaml" - name: Flush iptables rules become: yes diff --git a/tests/vars/Debian.yaml b/tests/multinode_firewall_persistence_vars/Debian.yaml similarity index 100% rename from tests/vars/Debian.yaml rename to tests/multinode_firewall_persistence_vars/Debian.yaml diff --git a/tests/multinode_firewall_persistence_vars/README.rst b/tests/multinode_firewall_persistence_vars/README.rst new file mode 100644 index 00000000..06af5c8a --- /dev/null +++ b/tests/multinode_firewall_persistence_vars/README.rst @@ -0,0 +1,21 @@ +multinode_firewall_persistence_vars +=================================== + +This directory is meant to contain distribution specific variables used in +integration tests for the ``multinode_firewall_persistence`` role. + +The behavior of the ``with_first_found`` lookup used with the ``include_vars`` +module will make it search for the ``vars`` directory in the "usual" order of +precedence which means if there is a ``vars`` directory inside the playbook +directory, it will search there first. + +This can result in one of two issues: + +1. If you try to prepend ``{{ role_path }}`` to workaround this issue with the + variable file paths, Zuul will deny the lookup if you are running an + untrusted playbook because the role was prepared in a trusted location and + Ansible is trying to search outside the work root as a result. +2. The variables included are the wrong ones -- the ones from + ``playbooks/vars`` are loaded instead of ``path/to//vars`` + +This is why this directory is called ``multinode_firewall_persistence_vars``. diff --git a/tests/vars/RedHat.yaml b/tests/multinode_firewall_persistence_vars/RedHat.yaml similarity index 100% rename from tests/vars/RedHat.yaml rename to tests/multinode_firewall_persistence_vars/RedHat.yaml diff --git a/tests/vars/Suse.yaml b/tests/multinode_firewall_persistence_vars/Suse.yaml similarity index 100% rename from tests/vars/Suse.yaml rename to tests/multinode_firewall_persistence_vars/Suse.yaml diff --git a/tests/vars/Ubuntu_trusty.yaml b/tests/multinode_firewall_persistence_vars/Ubuntu_trusty.yaml similarity index 100% rename from tests/vars/Ubuntu_trusty.yaml rename to tests/multinode_firewall_persistence_vars/Ubuntu_trusty.yaml diff --git a/tests/vars/default.yaml b/tests/multinode_firewall_persistence_vars/default.yaml similarity index 100% rename from tests/vars/default.yaml rename to tests/multinode_firewall_persistence_vars/default.yaml