From 79fdc2190a799e59bdc2c9c9c0f0937bde924cf0 Mon Sep 17 00:00:00 2001
From: Abhijeet Kasurde <akasurde@redhat.com>
Date: Mon, 8 Jul 2019 18:56:00 +0530
Subject: [PATCH] role: Fix role's hash_params (#55263)

* role: Fix role's hash_params

Fix based upon work done by alikins.

Fixes: #20596

Signed-off-by: Abhijeet Kasurde <akasurde@redhat.com>

* review comments

Signed-off-by: Abhijeet Kasurde <akasurde@redhat.com>
---
 .../fragments/20596-role-param_fix.yaml       |  2 +
 lib/ansible/playbook/role/__init__.py         |  4 +-
 test/units/playbook/role/test_role.py         | 46 +++++++++++++++++++
 3 files changed, 50 insertions(+), 2 deletions(-)
 create mode 100644 changelogs/fragments/20596-role-param_fix.yaml

diff --git a/changelogs/fragments/20596-role-param_fix.yaml b/changelogs/fragments/20596-role-param_fix.yaml
new file mode 100644
index 00000000000..eb02c151569
--- /dev/null
+++ b/changelogs/fragments/20596-role-param_fix.yaml
@@ -0,0 +1,2 @@
+bugfixes:
+- Fixed role's hash_params behavior to not union (https://github.com/ansible/ansible/issues/20596).
diff --git a/lib/ansible/playbook/role/__init__.py b/lib/ansible/playbook/role/__init__.py
index bb304fa755f..9230f48a30a 100644
--- a/lib/ansible/playbook/role/__init__.py
+++ b/lib/ansible/playbook/role/__init__.py
@@ -68,7 +68,7 @@ def hash_params(params):
                 new_params = set()
                 for k, v in params.items():
                     # Hash each entry individually
-                    new_params.update((k, hash_params(v)))
+                    new_params.add((k, hash_params(v)))
                 new_params = frozenset(new_params)
 
         elif isinstance(params, (Set, Sequence)):
@@ -79,7 +79,7 @@ def hash_params(params):
                 new_params = set()
                 for v in params:
                     # Hash each entry individually
-                    new_params.update(hash_params(v))
+                    new_params.add(hash_params(v))
                 new_params = frozenset(new_params)
         else:
             # This is just a guess.
diff --git a/test/units/playbook/role/test_role.py b/test/units/playbook/role/test_role.py
index b6386a99e61..f94037e9115 100644
--- a/test/units/playbook/role/test_role.py
+++ b/test/units/playbook/role/test_role.py
@@ -118,6 +118,52 @@ class TestHashParams(unittest.TestCase):
 
         self.assertRaises(TypeError, hash_params, params)
 
+    def test_param_dict_dupe_values(self):
+        params1 = {'foo': False}
+        params2 = {'bar': False}
+
+        res1 = hash_params(params1)
+        res2 = hash_params(params2)
+
+        hash1 = hash(res1)
+        hash2 = hash(res2)
+        self.assertNotEqual(res1, res2)
+        self.assertNotEqual(hash1, hash2)
+
+    def test_param_dupe(self):
+        params1 = {
+            # 'from_files': {},
+            'tags': [],
+            u'testvalue': False,
+            u'testvalue2': True,
+            # 'when': []
+        }
+        params2 = {
+            # 'from_files': {},
+            'tags': [],
+            u'testvalue': True,
+            u'testvalue2': False,
+            # 'when': []
+        }
+        res1 = hash_params(params1)
+        res2 = hash_params(params2)
+
+        self.assertNotEqual(hash(res1), hash(res2))
+        self.assertNotEqual(res1, res2)
+
+        foo = {}
+        foo[res1] = 'params1'
+        foo[res2] = 'params2'
+
+        self.assertEqual(len(foo), 2)
+
+        del foo[res2]
+        self.assertEqual(len(foo), 1)
+
+        for key in foo:
+            self.assertTrue(key in foo)
+            self.assertIn(key, foo)
+
 
 class TestRole(unittest.TestCase):