From 362d6a3204e9a696db544b14d6b62eb13f2e4437 Mon Sep 17 00:00:00 2001 From: Alan Rominger Date: Fri, 4 Sep 2020 11:48:56 -0400 Subject: [PATCH] Add new option update_secrets to allow lazy or strict updating --- .../plugins/module_utils/tower_api.py | 35 ++++++++++++++++--- .../plugins/module_utils/tower_module.py | 1 - .../plugins/modules/tower_credential.py | 7 ++++ awx_collection/plugins/modules/tower_user.py | 16 ++++----- awx_collection/test/awx/test_completeness.py | 2 +- awx_collection/test/awx/test_credential.py | 12 +++++-- awx_collection/test/awx/test_user.py | 2 +- 7 files changed, 56 insertions(+), 19 deletions(-) diff --git a/awx_collection/plugins/module_utils/tower_api.py b/awx_collection/plugins/module_utils/tower_api.py index 24a14e5713..b23596d8fe 100644 --- a/awx_collection/plugins/module_utils/tower_api.py +++ b/awx_collection/plugins/module_utils/tower_api.py @@ -28,6 +28,7 @@ class TowerAPIModule(TowerModule): '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 +37,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 +484,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 +510,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): diff --git a/awx_collection/plugins/module_utils/tower_module.py b/awx_collection/plugins/module_utils/tower_module.py index 215e2db3c8..470ea80de3 100644 --- a/awx_collection/plugins/module_utils/tower_module.py +++ b/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 diff --git a/awx_collection/plugins/modules/tower_credential.py b/awx_collection/plugins/modules/tower_credential.py index f244ae812e..c8b3f89afa 100644 --- a/awx_collection/plugins/modules/tower_credential.py +++ b/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 diff --git a/awx_collection/plugins/modules/tower_user.py b/awx_collection/plugins/modules/tower_user.py index 68b2f65a48..8250a8176a 100644 --- a/awx_collection/plugins/modules/tower_user.py +++ b/awx_collection/plugins/modules/tower_user.py @@ -55,13 +55,12 @@ options: description: - Write-only field used to change the password. type: str - update_password: + update_secrets: description: - - C(always) will update passwords if they differ. - - C(on_create) will only set the password for newly created users. - type: str - choices: [ always, on_create ] - default: always + - C(true) will always password if user specifies password and 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. @@ -122,7 +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_password=dict(choices=['always', 'on_create'], default='always', no_log=False), + update_secrets=dict(type='bool', default=True, no_log=False), state=dict(choices=['present', 'absent'], default='present'), ) @@ -137,7 +136,6 @@ def main(): is_superuser = module.params.get('is_superuser') is_system_auditor = module.params.get('is_system_auditor') password = module.params.get('password') - update_password = module.params.get('update_password') state = module.params.get('state') # Attempt to look up the related items the user specified (these will fail the module if not found) @@ -163,7 +161,7 @@ def main(): new_fields['is_superuser'] = is_superuser if is_system_auditor: new_fields['is_system_auditor'] = is_system_auditor - if password and (not existing_item or update_password == 'always'): + if password: new_fields['password'] = password # If the state was present and we can let the module build or update the existing item, this will return on its own diff --git a/awx_collection/test/awx/test_completeness.py b/awx_collection/test/awx/test_completeness.py index ab8c1547f8..5dc232e2ed 100644 --- a/awx_collection/test/awx/test_completeness.py +++ b/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 diff --git a/awx_collection/test/awx/test_credential.py b/awx_collection/test/awx/test_credential.py index 713006a386..7c0070d9e0 100644 --- a/awx_collection/test/awx/test_credential.py +++ b/awx_collection/test/awx/test_credential.py @@ -154,7 +154,8 @@ 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', @@ -172,9 +173,14 @@ def test_secret_field_write_twice(run_module, organization, admin_user, cred_typ name='Galaxy Token for Steve', organization=organization.name, credential_type=cred_type.name, - inputs={'token': val2} + inputs={'token': val2}, + update_secrets=update_secrets ), admin_user) assert not result.get('failed', False), result.get('msg', result) Credential.objects.get(id=result['id']).inputs['token'] == val2 - assert result.get('changed'), result + print(result) + if update_secrets: + assert result.get('changed'), result + else: + assert result.get('changed') is False, result diff --git a/awx_collection/test/awx/test_user.py b/awx_collection/test/awx/test_user.py index 5babf6efc0..6c49f04121 100644 --- a/awx_collection/test/awx/test_user.py +++ b/awx_collection/test/awx/test_user.py @@ -52,7 +52,7 @@ def test_update_password_on_create(run_module, admin_user, mock_auth_stuff): result = run_module('tower_user', dict( username='Bob', password='pass4word', - update_password='on_create' + update_secrets=False ), admin_user) assert not result.get('failed', False), result.get('msg', result)