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

11 files changed, 64 insertions, 0 deletions

diff --git a/changelogs/fragments/fix_ir_dupes.yml b/changelogs/fragments/fix_ir_dupes.yml
new file mode 100644
index 0000000000..a27b7376b2
--- /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 b32e26b100..e7f87d25f6 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 0000000000..b59832142f
--- /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 0000000000..998950b314
--- /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 0000000000..0ac9ff94b4
--- /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 0000000000..7fb1b487ef
--- /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 0000000000..f95ffe651b
--- /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 0000000000..57c135248e
--- /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 0000000000..04bd23be83
--- /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 0000000000..190c429bc9
--- /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 0000000000..fe99ea10b6
--- /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" ]