From 4a89749dd58d8921091dd504ebc7c87b29d8b417 Mon Sep 17 00:00:00 2001 From: Brian Coca Date: Wed, 16 Jan 2019 13:39:03 -0500 Subject: [PATCH] Prevent duplicate role insertion into roles: (#50552) * Corner case in which import_role would add another instance of a role with the same signature into roles: when it already existed there. roles: - name: a tasks: - import_role: name=a would execute role 'a' 3 times instead of the intended 2 (x2 in roles: phase +1 in tasks:) * added tests (cherry picked from commit eca7c3c8c763db0da4bb56c17d17f0a5be0f56d8) --- changelogs/fragments/fix_ir_dupes.yml | 2 ++ lib/ansible/playbook/role/__init__.py | 3 +++ test/integration/targets/roles/aliases | 1 + .../targets/roles/allowed_dupes.yml | 18 ++++++++++++++++++ test/integration/targets/roles/no_dupes.yml | 19 +++++++++++++++++++ .../targets/roles/roles/a/tasks/main.yml | 1 + .../targets/roles/roles/b/meta/main.yml | 2 ++ .../targets/roles/roles/b/tasks/main.yml | 1 + .../targets/roles/roles/c/meta/main.yml | 2 ++ .../targets/roles/roles/c/tasks/main.yml | 1 + test/integration/targets/roles/runme.sh | 14 ++++++++++++++ 11 files changed, 64 insertions(+) create mode 100644 changelogs/fragments/fix_ir_dupes.yml create mode 100644 test/integration/targets/roles/aliases create mode 100644 test/integration/targets/roles/allowed_dupes.yml create mode 100644 test/integration/targets/roles/no_dupes.yml create mode 100644 test/integration/targets/roles/roles/a/tasks/main.yml create mode 100644 test/integration/targets/roles/roles/b/meta/main.yml create mode 100644 test/integration/targets/roles/roles/b/tasks/main.yml create mode 100644 test/integration/targets/roles/roles/c/meta/main.yml create mode 100644 test/integration/targets/roles/roles/c/tasks/main.yml create mode 100755 test/integration/targets/roles/runme.sh diff --git a/changelogs/fragments/fix_ir_dupes.yml b/changelogs/fragments/fix_ir_dupes.yml new file mode 100644 index 00000000000..a27b7376b20 --- /dev/null +++ b/changelogs/fragments/fix_ir_dupes.yml @@ -0,0 +1,2 @@ +bugfixes: + - prevent import_role from inserting dupe into `roles:` execution when duplicate signature role already exists in the section. diff --git a/lib/ansible/playbook/role/__init__.py b/lib/ansible/playbook/role/__init__.py index b3b04976f61..94fba1c2e1a 100644 --- a/lib/ansible/playbook/role/__init__.py +++ b/lib/ansible/playbook/role/__init__.py @@ -149,6 +149,9 @@ class Role(Base, Become, Conditional, Taggable): params['from_files'] = from_files if role_include.vars: params['vars'] = role_include.vars + + params['from_include'] = from_include + hashed_params = hash_params(params) if role_include.role in play.ROLE_CACHE: for (entry, role_obj) in iteritems(play.ROLE_CACHE[role_include.role]): diff --git a/test/integration/targets/roles/aliases b/test/integration/targets/roles/aliases new file mode 100644 index 00000000000..b59832142f2 --- /dev/null +++ b/test/integration/targets/roles/aliases @@ -0,0 +1 @@ +shippable/posix/group3 diff --git a/test/integration/targets/roles/allowed_dupes.yml b/test/integration/targets/roles/allowed_dupes.yml new file mode 100644 index 00000000000..998950b3142 --- /dev/null +++ b/test/integration/targets/roles/allowed_dupes.yml @@ -0,0 +1,18 @@ +- name: test that import_role adds one (just one) execution of the role + hosts: localhost + gather_facts: false + tags: ['importrole'] + roles: + - name: a + tasks: + - name: import role ignores dupe rule + import_role: name=a + +- name: test that include_role adds one (just one) execution of the role + hosts: localhost + gather_facts: false + tags: ['includerole'] + roles: + - name: a + tasks: + - include_role: name=a diff --git a/test/integration/targets/roles/no_dupes.yml b/test/integration/targets/roles/no_dupes.yml new file mode 100644 index 00000000000..0ac9ff94b44 --- /dev/null +++ b/test/integration/targets/roles/no_dupes.yml @@ -0,0 +1,19 @@ +- name: play should only show 1 invocation of a, as dependencies in this play are deduped + hosts: testhost + gather_facts: false + tags: [ 'inroles' ] + roles: + - role: a + - role: b + - role: c + +- name: play should only show 1 invocation of a, as dependencies in this play are deduped even outside of roles + hosts: testhost + gather_facts: false + tags: [ 'acrossroles' ] + roles: + - role: a + - role: b + tasks: + - name: execute role c which depends on a + import_role: name=c diff --git a/test/integration/targets/roles/roles/a/tasks/main.yml b/test/integration/targets/roles/roles/a/tasks/main.yml new file mode 100644 index 00000000000..7fb1b487efb --- /dev/null +++ b/test/integration/targets/roles/roles/a/tasks/main.yml @@ -0,0 +1 @@ +- debug: msg=A diff --git a/test/integration/targets/roles/roles/b/meta/main.yml b/test/integration/targets/roles/roles/b/meta/main.yml new file mode 100644 index 00000000000..f95ffe651b8 --- /dev/null +++ b/test/integration/targets/roles/roles/b/meta/main.yml @@ -0,0 +1,2 @@ +dependencies: + - name: a diff --git a/test/integration/targets/roles/roles/b/tasks/main.yml b/test/integration/targets/roles/roles/b/tasks/main.yml new file mode 100644 index 00000000000..57c135248e5 --- /dev/null +++ b/test/integration/targets/roles/roles/b/tasks/main.yml @@ -0,0 +1 @@ +- debug: msg=B diff --git a/test/integration/targets/roles/roles/c/meta/main.yml b/test/integration/targets/roles/roles/c/meta/main.yml new file mode 100644 index 00000000000..04bd23be836 --- /dev/null +++ b/test/integration/targets/roles/roles/c/meta/main.yml @@ -0,0 +1,2 @@ +dependencies: + - name: a diff --git a/test/integration/targets/roles/roles/c/tasks/main.yml b/test/integration/targets/roles/roles/c/tasks/main.yml new file mode 100644 index 00000000000..190c429bc93 --- /dev/null +++ b/test/integration/targets/roles/roles/c/tasks/main.yml @@ -0,0 +1 @@ +- debug: msg=C diff --git a/test/integration/targets/roles/runme.sh b/test/integration/targets/roles/runme.sh new file mode 100755 index 00000000000..fe99ea10b63 --- /dev/null +++ b/test/integration/targets/roles/runme.sh @@ -0,0 +1,14 @@ +#!/usr/bin/env bash + +set -eux + +# test no dupes when dependencies in b and c point to a in roles: +[ "$(ansible-playbook no_dupes.yml -i ../../inventory --tags inroles "$@" | grep -c '"msg": "A"')" = "1" ] +[ "$(ansible-playbook no_dupes.yml -i ../../inventory --tags acrossroles "$@" | grep -c '"msg": "A"')" = "1" ] + +# but still dupe across plays +[ "$(ansible-playbook no_dupes.yml -i ../../inventory "$@" | grep -c '"msg": "A"')" = "2" ] + +# include/import can execute another instance of role +[ "$(ansible-playbook allowed_dupes.yml -i ../../inventory --tags importrole "$@" | grep -c '"msg": "A"')" = "2" ] +[ "$(ansible-playbook allowed_dupes.yml -i ../../inventory --tags includerole "$@" | grep -c '"msg": "A"')" = "2" ]