Merge pull request #8087 from AlanCoding/update_secrets

Add new option update_secrets to allow lazy or strict updating

Reviewed-by: https://github.com/apps/softwarefactory-project-zuul

awx_collection/plugins/module_utils/tower_api.py

@@ -22,12 +22,12 @@ class TowerAPIModule(TowerModule):
         'tower': 'Red Hat Ansible Tower',
     }
     session = None
-    cookie_jar = CookieJar()
     IDENTITY_FIELDS = {
         'users': 'username',
         'workflow_job_template_nodes': 'identifier',
         'instances': 'hostname'
     }
+    ENCRYPTED_STRING = "$encrypted$"
 
     def __init__(self, argument_spec, direct_params=None, error_callback=None, warn_callback=None, **kwargs):
         kwargs['supports_check_mode'] = True
@@ -36,6 +36,11 @@ class TowerAPIModule(TowerModule):
                                              error_callback=error_callback, warn_callback=warn_callback, **kwargs)
         self.session = Request(cookies=CookieJar(), validate_certs=self.verify_ssl)
 
+        if 'update_secrets' in self.params:
+            self.update_secrets = self.params.pop('update_secrets')
+        else:
+            self.update_secrets = True
+
     @staticmethod
     def param_to_endpoint(name):
         exceptions = {
@@ -478,6 +483,25 @@ class TowerAPIModule(TowerModule):
             return True
         return False
 
+    @staticmethod
+    def fields_could_be_same(old_field, new_field):
+        """Treating $encrypted$ as a wild card,
+        return False if the two values are KNOWN to be different
+        return True if the two values are the same, or could potentially be the same,
+        depending on the unknown $encrypted$ value or sub-values
+        """
+        if isinstance(old_field, dict) and isinstance(new_field, dict):
+            if set(old_field.keys()) != set(new_field.keys()):
+                return False
+            for key in new_field.keys():
+                if not TowerAPIModule.fields_could_be_same(old_field[key], new_field[key]):
+                    return False
+            return True  # all sub-fields are either equal or could be equal
+        else:
+            if old_field == TowerAPIModule.ENCRYPTED_STRING:
+                return True
+            return bool(new_field == old_field)
+
     def objects_could_be_different(self, old, new, field_set=None, warning=False):
         if field_set is None:
             field_set = set(fd for fd in new.keys() if fd not in ('modified', 'related', 'summary_fields'))
@@ -485,11 +509,13 @@ class TowerAPIModule(TowerModule):
             new_field = new.get(field, None)
             old_field = old.get(field, None)
             if old_field != new_field:
-                return True  # Something doesn't match
+                if self.update_secrets or (not self.fields_could_be_same(old_field, new_field)):
+                    return True  # Something doesn't match, or something might not match
             elif self.has_encrypted_values(new_field) or field not in new:
-                # case of 'field not in new' - user password write-only field that API will not display
-                self._encrypted_changed_warning(field, old, warning=warning)
-                return True
+                if self.update_secrets or (not self.fields_could_be_same(old_field, new_field)):
+                    # case of 'field not in new' - user password write-only field that API will not display
+                    self._encrypted_changed_warning(field, old, warning=warning)
+                    return True
         return False
 
     def update_if_needed(self, existing_item, new_item, on_update=None, associations=None):

awx_collection/plugins/module_utils/tower_module.py

@@ -52,7 +52,6 @@ class TowerModule(AnsibleModule):
     oauth_token_id = None
     authenticated = False
     config_name = 'tower_cli.cfg'
-    ENCRYPTED_STRING = "$encrypted$"
     version_checked = False
     error_callback = None
     warn_callback = None

awx_collection/plugins/modules/tower_credential.py

@@ -52,6 +52,12 @@ options:
           Refer to the Ansible Tower documentation for example syntax.
         - Any fields in this dict will take prescedence over any fields mentioned below (i.e. host, username, etc)
       type: dict
+    update_secrets:
+      description:
+        - C(true) will always update encrypted values.
+        - C(false) will only updated encrypted values if a change is absolutely known to be needed.
+      type: bool
+      default: true
     user:
       description:
         - User that should own this credential.
@@ -308,6 +314,7 @@ def main():
         organization=dict(),
         credential_type=dict(),
         inputs=dict(type='dict', no_log=True),
+        update_secrets=dict(type='bool', default=True, no_log=False),
         user=dict(),
         team=dict(),
         # These are for backwards compatability

awx_collection/plugins/modules/tower_user.py

@@ -55,6 +55,12 @@ options:
       description:
         - Write-only field used to change the password.
       type: str
+    update_secrets:
+      description:
+        - C(true) will always change password if user specifies password, even if API gives $encrypted$ for password.
+        - C(false) will only set the password if other values change too.
+      type: bool
+      default: true
     state:
       description:
         - Desired state of the resource.
@@ -115,6 +121,7 @@ def main():
         is_superuser=dict(type='bool', default=False, aliases=['superuser']),
         is_system_auditor=dict(type='bool', default=False, aliases=['auditor']),
         password=dict(no_log=True),
+        update_secrets=dict(type='bool', default=True, no_log=False),
         state=dict(choices=['present', 'absent'], default='present'),
     )
 

awx_collection/test/awx/test_completeness.py

@@ -30,7 +30,7 @@ no_endpoint_for_module = [
 
 # Global module parameters we can ignore
 ignore_parameters = [
-    'state', 'new_name',
+    'state', 'new_name', 'update_secrets'
 ]
 
 # Some modules take additional parameters that do not appear in the API

awx_collection/test/awx/test_credential.py

@@ -154,27 +154,25 @@ def test_make_use_of_custom_credential_type(run_module, organization, admin_user
 
 
 @pytest.mark.django_db
-def test_secret_field_write_twice(run_module, organization, admin_user, cred_type):
+@pytest.mark.parametrize('update_secrets', [True, False])
+def test_secret_field_write_twice(run_module, organization, admin_user, cred_type, update_secrets):
     val1 = '7rEZK38DJl58A7RxA6EC7lLvUHbBQ1'
-    result = run_module('tower_credential', dict(
-        name='Galaxy Token for Steve',
-        organization=organization.name,
-        credential_type=cred_type.name,
-        inputs={'token': val1}
-    ), admin_user)
-    assert not result.get('failed', False), result.get('msg', result)
-
-    Credential.objects.get(id=result['id']).inputs['token'] == val1
-
     val2 = '7rEZ238DJl5837rxA6xxxlLvUHbBQ1'
+    for val in (val1, val2):
+        result = run_module('tower_credential', dict(
+            name='Galaxy Token for Steve',
+            organization=organization.name,
+            credential_type=cred_type.name,
+            inputs={'token': val},
+            update_secrets=update_secrets
+        ), admin_user)
+        assert not result.get('failed', False), result.get('msg', result)
 
-    result = run_module('tower_credential', dict(
-        name='Galaxy Token for Steve',
-        organization=organization.name,
-        credential_type=cred_type.name,
-        inputs={'token': val2}
-    ), admin_user)
-    assert not result.get('failed', False), result.get('msg', result)
+        if update_secrets:
+            assert Credential.objects.get(id=result['id']).get_input('token') == val
 
-    Credential.objects.get(id=result['id']).inputs['token'] == val2
-    assert result.get('changed'), result
+    if update_secrets:
+        assert result.get('changed'), result
+    else:
+        assert result.get('changed') is False, result
+        assert Credential.objects.get(id=result['id']).get_input('token') == val1

awx_collection/test/awx/test_user.py

@@ -44,3 +44,16 @@ def test_password_no_op_warning(run_module, admin_user, mock_auth_stuff, silence
     silence_warning.assert_called_once_with(
         "The field password of user {0} has encrypted data and "
         "may inaccurately report task is changed.".format(result['id']))
+
+
+@pytest.mark.django_db
+def test_update_password_on_create(run_module, admin_user, mock_auth_stuff):
+    for i in range(2):
+        result = run_module('tower_user', dict(
+            username='Bob',
+            password='pass4word',
+            update_secrets=False
+        ), admin_user)
+        assert not result.get('failed', False), result.get('msg', result)
+
+    assert not result.get('changed')