From fd24918ba86f68c74a43c4c984de99814f9a187d Mon Sep 17 00:00:00 2001 From: John Westcott IV Date: Tue, 31 Mar 2020 00:07:46 -0400 Subject: [PATCH 1/8] Initial conversion of tower_credential --- awx_collection/README.md | 1 + .../plugins/module_utils/tower_api.py | 36 +- .../plugins/modules/tower_credential.py | 316 ++++++++---------- .../targets/tower_credential/tasks/main.yml | 211 ++++++++++-- 4 files changed, 351 insertions(+), 213 deletions(-) diff --git a/awx_collection/README.md b/awx_collection/README.md index 380f46549e..70d682660e 100644 --- a/awx_collection/README.md +++ b/awx_collection/README.md @@ -52,6 +52,7 @@ The following notes are changes that may require changes to playbooks: - Some return values (e.g., `credential_type`) have been removed. Use of `id` is recommended. - `tower_job_template` no longer supports the deprecated `extra_vars_path` parameter, please use `extra_vars` with the lookup plugin to replace this functionality. - The `notification_configuration` parameter of `tower_notification` has changed from a string to a dict. Please use the `lookup` plugin to read an existing file into a dict. + - `tower_credential` no longer supports passing a file name to ssh_key_data. ## Running Unit Tests diff --git a/awx_collection/plugins/module_utils/tower_api.py b/awx_collection/plugins/module_utils/tower_api.py index 4d51a8d337..f4ec7d4af2 100644 --- a/awx_collection/plugins/module_utils/tower_api.py +++ b/awx_collection/plugins/module_utils/tower_api.py @@ -265,7 +265,14 @@ class TowerModule(AnsibleModule): def resolve_name_to_id(self, endpoint, name_or_id): # Try to resolve the object by name - response = self.get_endpoint(endpoint, **{'data': {'name': name_or_id}}) + name_field = 'name' + if endpoint == 'users': + name_field = 'username' + + response = self.get_endpoint(endpoint, **{'data': {name_field: name_or_id}}) + if response['status_code'] == 400: + self.fail_json(msg="Unable to try and resolve {0} for {1} : {2}".format(endpoint, name_or_id, response['json']['detail'])) + if response['json']['count'] == 1: return response['json']['results'][0]['id'] elif response['json']['count'] == 0: @@ -567,6 +574,23 @@ class TowerModule(AnsibleModule): else: self.exit_json(**self.json_output) + # We need to be able to recursevly step through fields in the case of inputs for credentials. + # They are dicts and we can't just compare them at the top level because the dict returned from tower_cli will have fields like $encrypted$ + def compare_fields(self, new_item, existing_item): + needs_update = False + for field in new_item: + existing_field = existing_item.get(field, None) + new_field = new_item.get(field, None) + if type(existing_field) == dict and type(new_field) == dict: + needs_update = needs_update or self.compare_fields(new_field, existing_field) + # If the two items don't match and we are not comparing '' to None + # In the case of extra_vars in a job template, we have to pass in {} for nothing ('' is not valid) + # But when its returned from the API, instead of {} we get back ''. + elif existing_field != new_field and not (existing_field in (None, '') and new_field in ('', {})): + # Something doesn't match so let's update it + needs_update = True + return needs_update + def update_if_needed(self, existing_item, new_item, on_update=None, associations=None): # This will exit from the module on its own # If the method successfully updates an item and on_update param is defined, @@ -594,15 +618,7 @@ class TowerModule(AnsibleModule): self.fail_json(msg="Unable to process update of item due to missing data {0}".format(ke)) # Check to see if anything within the item requires the item to be updated - needs_update = False - for field in new_item: - existing_field = existing_item.get(field, None) - new_field = new_item.get(field, None) - # If the two items don't match and we are not comparing '' to None - if existing_field != new_field and not (existing_field in (None, '') and new_field == ''): - # Something doesn't match so let's update it - needs_update = True - break + needs_update = self.compare_fields(new_item, existing_item) # If we decided the item needs to be updated, update it self.json_output['id'] = item_id diff --git a/awx_collection/plugins/modules/tower_credential.py b/awx_collection/plugins/modules/tower_credential.py index e7b412b6c7..cdda10867a 100644 --- a/awx_collection/plugins/modules/tower_credential.py +++ b/awx_collection/plugins/modules/tower_credential.py @@ -28,37 +28,24 @@ options: - The name to use for the credential. required: True type: str + new_name: + description: + - Setting this option will change the existing name (looked up via the name field. + required: True + type: str description: description: - The description to use for the credential. type: str - user: - description: - - User that should own this credential. - type: str - team: - description: - - Team that should own this credential. - type: str - project: - description: - - Project that should use this credential. - type: str organization: description: - Organization that should own the credential. - required: True - type: str - kind: - description: - - Type of credential being added. - - The ssh choice refers to a Tower Machine credential. required: False type: str - choices: ["ssh", "vault", "net", "scm", "aws", "vmware", "satellite6", "cloudforms", "gce", "azure_rm", "openstack", "rhv", "insights", "tower"] credential_type: description: - Name of credential type. + - Will be prefered over kind required: False version_added: "2.10" type: str @@ -67,91 +54,132 @@ options: - >- Credential inputs where the keys are var names used in templating. 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) required: False version_added: "2.9" type: dict + user: + description: + - User that should own this credential. + type: str + team: + description: + - Team that should own this credential. + type: str + + kind: + description: + - Type of credential being added. + - The ssh choice refers to a Tower Machine credential. + - Deprecated, please use credential_type + required: False + type: str + choices: ["ssh", "vault", "net", "scm", "aws", "vmware", "satellite6", "cloudforms", "gce", "azure_rm", "openstack", "rhv", "insights", "tower"] host: description: - Host for this credential. + - Deprecated, will be removed in a future release type: str username: description: - Username for this credential. ``access_key`` for AWS. + - Deprecated, please use inputs type: str password: description: - Password for this credential. ``secret_key`` for AWS. ``api_key`` for RAX. - Use "ASK" and launch in Tower to be prompted. + - Deprecated, please use inputs + type: str + project: + description: + - Project that should use this credential for GCP. + - Deprecated, will be removed in a future release type: str ssh_key_data: description: - SSH private key content. To extract the content from a file path, use the lookup function (see examples). + - Deprecated, please use inputs required: False type: str ssh_key_unlock: description: - Unlock password for ssh_key. - Use "ASK" and launch in Tower to be prompted. + - Deprecated, please use inputs type: str authorize: description: - Should use authorize for net type. + - Deprecated, please use inputs type: bool default: 'no' authorize_password: description: - Password for net credentials that require authorize. + - Deprecated, please use inputs type: str client: description: - Client or application ID for azure_rm type. + - Deprecated, please use inputs type: str security_token: description: - STS token for aws type. + - Deprecated, please use inputs version_added: "2.6" type: str secret: description: - Secret token for azure_rm type. + - Deprecated, please use inputs type: str subscription: description: - Subscription ID for azure_rm type. + - Deprecated, please use inputs type: str tenant: description: - Tenant ID for azure_rm type. + - Deprecated, please use inputs type: str domain: description: - Domain for openstack type. + - Deprecated, please use inputs type: str become_method: description: - Become method to use for privilege escalation. - Some examples are "None", "sudo", "su", "pbrun" - Due to become plugins, these can be arbitrary + - Deprecated, please use inputs type: str become_username: description: - Become username. - Use "ASK" and launch in Tower to be prompted. + - Deprecated, please use inputs type: str become_password: description: - Become password. - Use "ASK" and launch in Tower to be prompted. + - Deprecated, please use inputs type: str vault_password: description: - Vault password. - Use "ASK" and launch in Tower to be prompted. + - Deprecated, please use inputs type: str vault_id: description: - Vault identifier. - This parameter is only valid if C(kind) is specified as C(vault). + - Deprecated, please use inputs type: str version_added: "2.8" state: @@ -160,21 +188,23 @@ options: choices: ["present", "absent"] default: "present" type: str - -requirements: -- ansible-tower-cli >= 3.0.2 - + tower_oauthtoken: + description: + - The Tower OAuth token to use. + required: False + type: str + version_added: "3.7" extends_documentation_fragment: awx.awx.auth ''' EXAMPLES = ''' -- name: Add tower credential +- name: Add tower machine credential tower_credential: name: Team Name description: Team Description organization: test-org - kind: ssh + credential_type: Machine state: present tower_config_file: "~/tower_cli.cfg" @@ -183,11 +213,12 @@ EXAMPLES = ''' name: SCM Credential organization: Default state: present - kind: scm - username: joe - password: secret - ssh_key_data: "{{ lookup('file', '/tmp/id_rsa') }}" - ssh_key_unlock: "passphrase" + credential_type: Source Control + inputs: + username: joe + password: secret + ssh_key_data: "{{ lookup('file', '/tmp/id_rsa') }}" + ssh_key_unlock: "passphrase" - name: Fetch private key slurp: @@ -196,12 +227,10 @@ EXAMPLES = ''' - name: Add Credential Into Tower tower_credential: name: Workshop Credential - ssh_key_data: "{{ aws_ssh_key['content'] | b64decode }}" - kind: ssh + credential_type: Machine organization: Default - tower_username: admin - tower_password: ansible - tower_host: https://localhost + inputs: + ssh_key_data: "{{ aws_ssh_key['content'] | b64decode }}" run_once: true delegate_to: localhost @@ -215,23 +244,11 @@ EXAMPLES = ''' tower_host: https://localhost ''' -import os - -from ansible.module_utils._text import to_text -from ..module_utils.ansible_tower import TowerModule, tower_auth_config, tower_check_mode - -try: - import tower_cli - import tower_cli.exceptions as exc - - from tower_cli.conf import settings -except ImportError: - pass - +from ..module_utils.tower_api import TowerModule KIND_CHOICES = { 'ssh': 'Machine', - 'vault': 'Ansible Vault', + 'vault': 'Vault', 'net': 'Network', 'scm': 'Source Control', 'aws': 'Amazon Web Services', @@ -257,162 +274,117 @@ OLD_INPUT_NAMES = ( ) -def credential_type_for_kind(params): - credential_type_res = tower_cli.get_resource('credential_type') - kind = params.get('kind') - arguments = {'managed_by_tower': True} - if kind == 'ssh': - if params.get('vault_password'): - arguments['kind'] = 'vault' - else: - arguments['kind'] = 'ssh' - elif kind in ('net', 'scm', 'insights', 'vault'): - arguments['kind'] = kind - elif kind in KIND_CHOICES: - arguments.update(dict( - kind='cloud', - name=KIND_CHOICES[kind] - )) - return credential_type_res.get(**arguments) - - def main(): - + # Any additional arguments that are not fields of the item can be added here argument_spec = dict( name=dict(required=True), - user=dict(), - team=dict(), - kind=dict(choices=list(KIND_CHOICES.keys())), + new_name=dict(), + description=dict(), + organization=dict(), credential_type=dict(), inputs=dict(type='dict'), + user=dict(), + team=dict(), + # These are for backwards compatability + kind=dict(choices=list(KIND_CHOICES.keys())), host=dict(), username=dict(), password=dict(no_log=True), - ssh_key_data=dict(no_log=True, type='str'), + project=dict(), + ssh_key_data=dict(no_log=True), ssh_key_unlock=dict(no_log=True), - authorize=dict(type='bool', default=False), + authorize=dict(type='bool'), authorize_password=dict(no_log=True), client=dict(), security_token=dict(), - secret=dict(), - tenant=dict(), + secret=dict(no_log=True), subscription=dict(), + tenant=dict(), domain=dict(), become_method=dict(), become_username=dict(), become_password=dict(no_log=True), vault_password=dict(no_log=True), - description=dict(), - organization=dict(required=True), - project=dict(), - state=dict(choices=['present', 'absent'], default='present'), vault_id=dict(), + # End backwards compatability + state=dict(choices=['present', 'absent'], default='present'), ) - mutually_exclusive = [ - ('kind', 'credential_type') - ] - for input_name in OLD_INPUT_NAMES: - mutually_exclusive.append(('inputs', input_name)) - - module = TowerModule(argument_spec=argument_spec, supports_check_mode=True, - mutually_exclusive=mutually_exclusive) + # Create a module for ourselves + module = TowerModule(argument_spec=argument_spec, supports_check_mode=True, required_one_of=[['kind', 'credential_type']]) + # Extract our parameters name = module.params.get('name') + new_name = module.params.get('new_name') + description = module.params.get('description') organization = module.params.get('organization') + credential_type = module.params.get('credential_type') + inputs = module.params.get('inputs') + user = module.params.get('user') + team = module.params.get('team') + # The legacy arguments are put into a hash down below + kind = module.params.get('kind') + # End backwards compatability state = module.params.get('state') - json_output = {'credential': name, 'state': state} + # Attempt to look up the related items the user specified (these will fail the module if not found) + if organization: + org_id = module.resolve_name_to_id('organizations', organization) + if user: + user_id = module.resolve_name_to_id('users', user) + if team: + team_id = module.resolve_name_to_id('teams', team) - tower_auth = tower_auth_config(module) - with settings.runtime_values(**tower_auth): - tower_check_mode(module) - credential = tower_cli.get_resource('credential') - try: - params = {} - params['create_on_missing'] = True - params['name'] = name + if kind: + module.deprecate(msg='The kind parameter has been depricated, please use credential_type instead', version="3.6") - if organization: - org_res = tower_cli.get_resource('organization') - org = org_res.get(name=organization) - params['organization'] = org['id'] + cred_type_id = module.resolve_name_to_id('credential_types', credential_type if credential_type else KIND_CHOICES[kind]) - try: - tower_cli.get_resource('credential_type') - except (ImportError, AttributeError): - # /api/v1/ backwards compat - # older versions of tower-cli don't *have* a credential_type - # resource - params['kind'] = module.params.get('kind') - else: - if module.params.get('credential_type'): - credential_type_res = tower_cli.get_resource('credential_type') - try: - credential_type = credential_type_res.get(name=module.params['credential_type']) - except (exc.NotFound) as excinfo: - module.fail_json(msg=( - 'Failed to update credential, credential_type not found: {0}' - ).format(excinfo), changed=False) - params['credential_type'] = credential_type['id'] + # Attempt to look up the object based on the provided name and inventory ID + credential = module.get_one('credentials', **{ + 'data': { + 'name': name, + 'credential_type': cred_type_id, + } + }) - if module.params.get('inputs'): - params['inputs'] = module.params.get('inputs') + # Create credential input from legacy inputs + credential_inputs = {} + for legacy_input in OLD_INPUT_NAMES: + if module.params.get(legacy_input) is not None: + module.deprecate(msg='{0} parameter has been depricated, please use inputs instead'.format(legacy_input), version="3.6") + credential_inputs[legacy_input] = module.params.get(legacy_input) + if inputs: + credential_inputs.update(inputs) - elif module.params.get('kind'): - credential_type = credential_type_for_kind(module.params) - params['credential_type'] = credential_type['id'] - else: - module.fail_json(msg='must either specify credential_type or kind', changed=False) + # Create the data that gets sent for create and update + credential_fields = { + 'name': new_name if new_name else name, + 'credential_type': cred_type_id, + 'inputs': credential_inputs, + } + if description: + credential_fields['description'] = description + if organization: + credential_fields['organization'] = org_id - if module.params.get('description'): - params['description'] = module.params.get('description') + # If we don't already have a credential (and we are creating one) we can add user/team + # The API does not appear to do anything with these after creation anyway + # NOTE: We can't just add these on a modification because they are never returned from a GET so it would always cause a changed=True + if not credential: + if user: + credential_fields['user'] = user_id + if team: + credential_fields['team'] = team_id - if module.params.get('user'): - user_res = tower_cli.get_resource('user') - user = user_res.get(username=module.params.get('user')) - params['user'] = user['id'] - - if module.params.get('team'): - team_res = tower_cli.get_resource('team') - team = team_res.get(name=module.params.get('team')) - params['team'] = team['id'] - - if module.params.get('ssh_key_data'): - data = module.params.get('ssh_key_data') - if os.path.exists(data): - module.deprecate( - msg='ssh_key_data should be a string, not a path to a file.', - version="2.12" - ) - if os.path.isdir(data): - module.fail_json(msg='attempted to read contents of directory: %s' % data) - with open(data, 'rb') as f: - module.params['ssh_key_data'] = to_text(f.read()) - else: - module.params['ssh_key_data'] = data - - if module.params.get('vault_id', None) and module.params.get('kind') != 'vault': - module.fail_json(msg="Parameter 'vault_id' is only valid if parameter 'kind' is specified as 'vault'") - - for key in OLD_INPUT_NAMES: - if 'kind' in params: - params[key] = module.params.get(key) - elif module.params.get(key): - params.setdefault('inputs', {})[key] = module.params.get(key) - - if state == 'present': - result = credential.modify(**params) - json_output['id'] = result['id'] - elif state == 'absent': - result = credential.delete(**params) - except (exc.NotFound) as excinfo: - module.fail_json(msg='Failed to update credential, organization not found: {0}'.format(excinfo), changed=False) - except (exc.ConnectionError, exc.BadRequest, exc.AuthError) as excinfo: - module.fail_json(msg='Failed to update credential: {0}'.format(excinfo), changed=False) - - json_output['changed'] = result['changed'] - module.exit_json(**json_output) + if state == 'absent': + # If the state was absent we can let the module delete it if needed, the module will handle exiting from this + module.delete_if_needed(credential) + elif state == 'present': + # If the state was present we can let the module build or update the existing group, this will return on its own + module.create_or_update_if_needed( + credential, credential_fields, endpoint='credentials', item_type='credential' + ) if __name__ == '__main__': diff --git a/awx_collection/tests/integration/targets/tower_credential/tasks/main.yml b/awx_collection/tests/integration/targets/tower_credential/tasks/main.yml index d30dc67ff6..42bf108d30 100644 --- a/awx_collection/tests/integration/targets/tower_credential/tasks/main.yml +++ b/awx_collection/tests/integration/targets/tower_credential/tasks/main.yml @@ -1,24 +1,29 @@ --- +- name: Generate a random string for test + set_fact: + test_id: "{{ lookup('password', '/dev/null chars=ascii_letters length=16') }}" + when: test_id is not defined + - name: Generate names set_fact: - ssh_cred_name1: "AWX-Collection-tests-tower_credential-ssh-cred1-{{ lookup('password', '/dev/null chars=ascii_letters length=16') }}" - ssh_cred_name2: "AWX-Collection-tests-tower_credential-ssh-cred2-{{ lookup('password', '/dev/null chars=ascii_letters length=16') }}" - ssh_cred_name3: "AWX-Collection-tests-tower_credential-ssh-cred-lookup-source-{{ lookup('password', '/dev/null chars=ascii_letters length=16') }}" - ssh_cred_name4: "AWX-Collection-tests-tower_credential-ssh-cred-file-source-{{ lookup('password', '/dev/null chars=ascii_letters length=16') }}" - vault_cred_name1: "AWX-Collection-tests-tower_credential-vault-cred1-{{ lookup('password', '/dev/null chars=ascii_letters length=16') }}" - vault_cred_name2: "AWX-Collection-tests-tower_credential-vault-ssh-cred1-{{ lookup('password', '/dev/null chars=ascii_letters length=16') }}" - net_cred_name1: "AWX-Collection-tests-tower_credential-net-cred1-{{ lookup('password', '/dev/null chars=ascii_letters length=16') }}" - scm_cred_name1: "AWX-Collection-tests-tower_credential-scm-cred1-{{ lookup('password', '/dev/null chars=ascii_letters length=16') }}" - aws_cred_name1: "AWX-Collection-tests-tower_credential-aws-cred1-{{ lookup('password', '/dev/null chars=ascii_letters length=16') }}" - vmware_cred_name1: "AWX-Collection-tests-tower_credential-vmware-cred1-{{ lookup('password', '/dev/null chars=ascii_letters length=16') }}" - sat6_cred_name1: "AWX-Collection-tests-tower_credential-sat6-cred1-{{ lookup('password', '/dev/null chars=ascii_letters length=16') }}" - cf_cred_name1: "AWX-Collection-tests-tower_credential-cf-cred1-{{ lookup('password', '/dev/null chars=ascii_letters length=16') }}" - gce_cred_name1: "AWX-Collection-tests-tower_credential-gce-cred1-{{ lookup('password', '/dev/null chars=ascii_letters length=16') }}" - azurerm_cred_name1: "AWX-Collection-tests-tower_credential-azurerm-cred1-{{ lookup('password', '/dev/null chars=ascii_letters length=16') }}" - openstack_cred_name1: "AWX-Collection-tests-tower_credential-openstack-cred1-{{ lookup('password', '/dev/null chars=ascii_letters length=16') }}" - rhv_cred_name1: "AWX-Collection-tests-tower_credential-rhv-cred1-{{ lookup('password', '/dev/null chars=ascii_letters length=16') }}" - insights_cred_name1: "AWX-Collection-tests-tower_credential-insights-cred1-{{ lookup('password', '/dev/null chars=ascii_letters length=16') }}" - tower_cred_name1: "AWX-Collection-tests-tower_credential-tower-cred1-{{ lookup('password', '/dev/null chars=ascii_letters length=16') }}" + ssh_cred_name1: "AWX-Collection-tests-tower_credential-ssh-cred1-{{ test_id }}" + ssh_cred_name2: "AWX-Collection-tests-tower_credential-ssh-cred2-{{ test_id }}" + ssh_cred_name3: "AWX-Collection-tests-tower_credential-ssh-cred-lookup-source-{{ test_id }}" + ssh_cred_name4: "AWX-Collection-tests-tower_credential-ssh-cred-file-source-{{ test_id }}" + vault_cred_name1: "AWX-Collection-tests-tower_credential-vault-cred1-{{ test_id }}" + vault_cred_name2: "AWX-Collection-tests-tower_credential-vault-ssh-cred1-{{ test_id }}" + net_cred_name1: "AWX-Collection-tests-tower_credential-net-cred1-{{ test_id }}" + scm_cred_name1: "AWX-Collection-tests-tower_credential-scm-cred1-{{ test_id }}" + aws_cred_name1: "AWX-Collection-tests-tower_credential-aws-cred1-{{ test_id }}" + vmware_cred_name1: "AWX-Collection-tests-tower_credential-vmware-cred1-{{ test_id }}" + sat6_cred_name1: "AWX-Collection-tests-tower_credential-sat6-cred1-{{ test_id }}" + cf_cred_name1: "AWX-Collection-tests-tower_credential-cf-cred1-{{ test_id }}" + gce_cred_name1: "AWX-Collection-tests-tower_credential-gce-cred1-{{ test_id }}" + azurerm_cred_name1: "AWX-Collection-tests-tower_credential-azurerm-cred1-{{ test_id }}" + openstack_cred_name1: "AWX-Collection-tests-tower_credential-openstack-cred1-{{ test_id }}" + rhv_cred_name1: "AWX-Collection-tests-tower_credential-rhv-cred1-{{ test_id }}" + insights_cred_name1: "AWX-Collection-tests-tower_credential-insights-cred1-{{ test_id }}" + tower_cred_name1: "AWX-Collection-tests-tower_credential-tower-cred1-{{ test_id }}" - name: create a tempdir for an SSH key local_action: shell mktemp -d @@ -31,7 +36,42 @@ set_fact: ssh_key_data: "{{ lookup('file', tempdir.stdout + '/id_rsa') }}" -- name: Create a User-specific credential +- name: Test deprication warnings + tower_credential: + name: "{{ ssh_cred_name1 }}" + organization: Default + user: admin + kind: ssh + authorize: False + authorize_password: 'test' + client: 'test' + security_token: 'test' + secret: 'test' + tenant: 'test' + subscription: 'test' + domain: 'test' + become_method: 'test' + become_username: 'test' + become_password: 'test' + vault_password: 'test' + project: 'test' + host: 'test' + username: 'test' + password: 'test' + ssh_key_data: 'test' + vault_id: 'test' + ssh_key_unlock: 'test' + state: absent + ignore_errors: True + register: result + +- assert: + that: + - "'deprecations' in result" + # The 20 comes from the length of OLD_INPUT_NAMES + 1 for kind + - result['deprecations'] | length() == 20 + +- name: Create a User-specific credential (old school) tower_credential: name: "{{ ssh_cred_name1 }}" organization: Default @@ -44,6 +84,44 @@ that: - "result is changed" +- name: Re-create the User-specific credential (new school) + tower_credential: + name: "{{ ssh_cred_name1 }}" + organization: Default + user: admin + credential_type: 'Machine' + state: present + register: result + +- assert: + that: + - "result is not changed" + +- name: Delete a User-specific credential + tower_credential: + name: "{{ ssh_cred_name1 }}" + organization: Default + user: admin + state: absent + kind: ssh + register: result + +- assert: + that: + - "result is changed" + +- name: Create the User-specific credential tied to a user, no org + tower_credential: + name: "{{ ssh_cred_name1 }}" + user: admin + credential_type: 'Machine' + state: present + register: result + +- assert: + that: + - "result is changed" + - name: Delete a User-specific credential tower_credential: name: "{{ ssh_cred_name1 }}" @@ -57,7 +135,7 @@ that: - "result is changed" -- name: Create a valid SSH credential +- name: Create a valid SSH credential (old school) tower_credential: name: "{{ ssh_cred_name2 }}" organization: Default @@ -77,7 +155,48 @@ that: - "result is changed" -- name: Create a valid SSH credential from lookup source +- name: Create a valid SSH credential (new school) + tower_credential: + name: "{{ ssh_cred_name2 }}" + organization: Default + state: present + credential_type: Machine + description: An example SSH credential + inputs: + username: joe + password: secret + become_method: sudo + become_username: superuser + become_password: supersecret + ssh_key_data: "{{ ssh_key_data }}" + ssh_key_unlock: "passphrase" + register: result + +# This will be changed because we are setting ssh_key_data and ssh_key_unlock. +# These will come out as $encrypted$ which will always compare false to the values. +- assert: + that: + - result is changed + +- name: Create a valid SSH credential (new school) (no change) + tower_credential: + name: "{{ ssh_cred_name2 }}" + organization: Default + state: present + credential_type: Machine + description: An example SSH credential + inputs: + username: joe + become_method: sudo + become_username: superuser + register: result + +# This should no longer be changed because we aren't passing any secure fields +- assert: + that: + - result is not changed + +- name: Create a valid SSH credential from lookup source (old school) tower_credential: name: "{{ ssh_cred_name3 }}" organization: Default @@ -97,7 +216,29 @@ that: - "result is changed" -- name: Create a valid SSH credential from file source +- name: Create a valid SSH credential from lookup source (new school) + tower_credential: + name: "{{ ssh_cred_name3 }}" + organization: Default + state: present + credential_type: Machine + description: An example SSH credential from lookup source + inputs: + username: joe + password: secret + become_method: sudo + become_username: superuser + become_password: supersecret + ssh_key_data: "{{ lookup('file', tempdir.stdout + '/id_rsa') }}" + ssh_key_unlock: "passphrase" + register: result + +# This will be changed because we are passing in ssh_key_data and password +- assert: + that: + - result is changed + +- name: Fail to create an SSH credential from a file source (old school format) tower_credential: name: "{{ ssh_cred_name4 }}" organization: Default @@ -112,12 +253,13 @@ ssh_key_data: "{{ tempdir.stdout }}/id_rsa" ssh_key_unlock: "passphrase" register: result + ignore_errors: True - assert: that: - - "result is changed" - - "result is not failed" - - "'ssh_key_data should be a string, not a path to a file.' in result.deprecations[0].msg" + - result is failed + - "'Unable to create credential {{ ssh_cred_name4 }}' in result.msg" + - "'Invalid certificate or key' in result.msg" - name: Create an invalid SSH credential (passphrase required) tower_credential: @@ -148,7 +290,7 @@ - assert: that: - "result is failed" - - "'The requested object could not be found' in result.msg" + - "'The organizations Missing Organization was not found on the Tower server' in result.msg" - name: Delete an SSH credential tower_credential: @@ -182,9 +324,10 @@ kind: ssh register: result +# This one was never really created so it shouldn't be deleted - assert: that: - - "result is changed" + - "result is not changed" - name: Create a valid Vault credential tower_credential: @@ -201,7 +344,7 @@ - "result is changed" # We should decide when to delete this test -- name: Create a valid Vault credential w/ kind=ssh (deprecated) +- name: Create a valid Vault credential w/ kind=ssh (deprecated, will now fail) tower_credential: name: "{{ vault_cred_name2 }}" organization: Default @@ -210,10 +353,14 @@ description: An example Vault credential vault_password: secret-vault register: result + ignore_errors: True - assert: that: - - "result is changed" + - result is failed + - "'Unable to create credential {{ vault_cred_name2 }}' in result.msg" + - "'Additional properties are not allowed' in result.msg" + - "'\\'vault_password\\' was unexpected' in result.msg" - name: Delete a Vault credential tower_credential: @@ -235,9 +382,10 @@ kind: vault register: result +# The creation of vault_cred_name2 never worked so we shouldn't actually need to delete it - assert: that: - - "result is changed" + - "result is not changed" - name: Create a valid Network credential tower_credential: @@ -594,4 +742,5 @@ - assert: that: - - "result.msg =='Failed to update credential, organization not found: The requested object could not be found.'" + - result is failed + - "result.msg =='The organizations test-non-existing-org was not found on the Tower server'" From 3f64768ba8aa3a5b114f6999e9c8bd907c170788 Mon Sep 17 00:00:00 2001 From: AlanCoding Date: Tue, 31 Mar 2020 11:24:15 -0400 Subject: [PATCH 2/8] loosen some credential test assertions --- awx_collection/test/awx/test_credential.py | 274 ++++++++++----------- 1 file changed, 128 insertions(+), 146 deletions(-) diff --git a/awx_collection/test/awx/test_credential.py b/awx_collection/test/awx/test_credential.py index 63ca46f0ff..31ba9326ff 100644 --- a/awx_collection/test/awx/test_credential.py +++ b/awx_collection/test/awx/test_credential.py @@ -6,143 +6,8 @@ import pytest from awx.main.models import Credential, CredentialType, Organization -@pytest.mark.django_db -def test_create_machine_credential(run_module, admin_user): - Organization.objects.create(name='test-org') - # create the ssh credential type - ct = CredentialType.defaults['ssh']() - ct.save() - # Example from docs - result = run_module('tower_credential', dict( - name='Test Machine Credential', - organization='test-org', - kind='ssh', - state='present' - ), admin_user) - assert result.get('changed'), result - - cred = Credential.objects.get(name='Test Machine Credential') - assert cred.credential_type == ct - result.pop('invocation') - assert result == { - "credential": "Test Machine Credential", - "state": "present", - "id": cred.pk, - "changed": True - } - - -@pytest.mark.django_db -def test_create_vault_credential(run_module, admin_user): - # https://github.com/ansible/ansible/issues/61324 - Organization.objects.create(name='test-org') - ct = CredentialType.defaults['vault']() - ct.save() - - result = run_module('tower_credential', dict( - name='Test Vault Credential', - organization='test-org', - kind='vault', - vault_id='bar', - vault_password='foobar', - state='present' - ), admin_user) - assert result.get('changed'), result - - cred = Credential.objects.get(name='Test Vault Credential') - assert cred.credential_type == ct - assert 'vault_id' in cred.inputs - assert 'vault_password' in cred.inputs - result.pop('invocation') - assert result == { - "credential": "Test Vault Credential", - "state": "present", - "id": cred.pk, - "changed": True - } - - -@pytest.mark.django_db -def test_create_custom_credential_type(run_module, admin_user): - # Example from docs - result = run_module('tower_credential_type', dict( - name='Nexus', - description='Credentials type for Nexus', - kind='cloud', - inputs={"fields": [{"id": "server", "type": "string", "default": "", "label": ""}], "required": []}, - injectors={'extra_vars': {'nexus_credential': 'test'}}, - state='present', - validate_certs='false' - ), admin_user) - assert result.get('changed'), result - - ct = CredentialType.objects.get(name='Nexus') - result.pop('invocation') - assert result == { - "name": "Nexus", - "id": ct.pk, - "changed": True, - } - assert ct.inputs == {"fields": [{"id": "server", "type": "string", "default": "", "label": ""}], "required": []} - assert ct.injectors == {'extra_vars': {'nexus_credential': 'test'}} - - -@pytest.mark.django_db -def test_kind_ct_exclusivity(run_module, admin_user): - result = run_module('tower_credential', dict( - name='A credential', - organization='test-org', - kind='ssh', - credential_type='foobar', # cannot specify if kind is also specified - state='present' - ), admin_user) - - result.pop('invocation') - assert result == { - 'failed': True, - 'msg': 'parameters are mutually exclusive: kind|credential_type' - } - - -@pytest.mark.django_db -def test_input_exclusivity(run_module, admin_user): - result = run_module('tower_credential', dict( - name='A credential', - organization='test-org', - kind='ssh', - inputs={'token': '7rEZK38DJl58A7RxA6EC7lLvUHbBQ1'}, - security_token='7rEZK38DJl58A7RxA6EC7lLvUHbBQ1', - state='present' - ), admin_user) - - result.pop('invocation') - assert result == { - 'failed': True, - 'msg': 'parameters are mutually exclusive: inputs|security_token' - } - - -@pytest.mark.django_db -def test_missing_credential_type(run_module, admin_user): - Organization.objects.create(name='test-org') - result = run_module('tower_credential', dict( - name='A credential', - organization='test-org', - credential_type='foobar', - state='present' - ), admin_user) - - result.pop('invocation') - assert result == { - "changed": False, - "failed": True, - 'msg': 'Failed to update credential, credential_type not found: The requested object could not be found.' - } - - -@pytest.mark.django_db -def test_make_use_of_custom_credential_type(run_module, admin_user): - Organization.objects.create(name='test-org') +@pytest.fixture +def cred_type(): # Make a credential type which will be used by the credential ct = CredentialType.objects.create( name='Ansible Galaxy Token', @@ -163,23 +28,140 @@ def test_make_use_of_custom_credential_type(run_module, admin_user): } } ) + return ct + + +@pytest.mark.django_db +def test_create_machine_credential(run_module, admin_user, organization): + Organization.objects.create(name='test-org') + # create the ssh credential type + ct = CredentialType.defaults['ssh']() + ct.save() + # Example from docs + result = run_module('tower_credential', dict( + name='Test Machine Credential', + organization=organization.name, + kind='ssh', + state='present' + ), admin_user) + assert not result.get('failed', False), result.get('msg', result) + assert result.get('changed'), result + + cred = Credential.objects.get(name='Test Machine Credential') + assert cred.credential_type == ct + + assert result['name'] == "Test Machine Credential" + assert result['id'] == cred.pk + + +@pytest.mark.django_db +def test_create_vault_credential(run_module, admin_user, organization): + # https://github.com/ansible/ansible/issues/61324 + Organization.objects.create(name='test-org') + ct = CredentialType.defaults['vault']() + ct.save() + + result = run_module('tower_credential', dict( + name='Test Vault Credential', + organization=organization.name, + kind='vault', + vault_id='bar', + vault_password='foobar', + state='present' + ), admin_user) + assert not result.get('failed', False), result.get('msg', result) + assert result.get('changed'), result + + cred = Credential.objects.get(name='Test Vault Credential') + assert cred.credential_type == ct + assert 'vault_id' in cred.inputs + assert 'vault_password' in cred.inputs + + assert result['name'] == "Test Vault Credential" + assert result['id'] == cred.pk + + +@pytest.mark.django_db +def test_create_custom_credential_type(run_module, admin_user): + # Example from docs + result = run_module('tower_credential_type', dict( + name='Nexus', + description='Credentials type for Nexus', + kind='cloud', + inputs={"fields": [{"id": "server", "type": "string", "default": "", "label": ""}], "required": []}, + injectors={'extra_vars': {'nexus_credential': 'test'}}, + state='present', + validate_certs='false' + ), admin_user) + assert not result.get('failed', False), result.get('msg', result) + assert result.get('changed'), result + + ct = CredentialType.objects.get(name='Nexus') + + assert result['name'] == 'Nexus' + assert result['id'] == ct.pk + + assert ct.inputs == {"fields": [{"id": "server", "type": "string", "default": "", "label": ""}], "required": []} + assert ct.injectors == {'extra_vars': {'nexus_credential': 'test'}} + + +@pytest.mark.django_db +def test_kind_ct_exclusivity(run_module, admin_user, organization, cred_type, silence_deprecation): + result = run_module('tower_credential', dict( + name='A credential', + organization=organization.name, + kind='ssh', + credential_type=cred_type.name, + state='present' + ), admin_user) + assert result.get('failed', False), result.get('msg', result) + assert result['msg'] == 'parameters are mutually exclusive: kind|credential_type' + + +@pytest.mark.django_db +def test_input_exclusivity(run_module, admin_user, organization): + result = run_module('tower_credential', dict( + name='A credential', + organization=organization.name, + kind='ssh', + inputs={'token': '7rEZK38DJl58A7RxA6EC7lLvUHbBQ1'}, + security_token='7rEZK38DJl58A7RxA6EC7lLvUHbBQ1', + state='present' + ), admin_user) + assert result.get('failed', False), result + assert result['msg'] == 'parameters are mutually exclusive: inputs|security_token' + + +@pytest.mark.django_db +def test_missing_credential_type(run_module, admin_user, organization): + Organization.objects.create(name='test-org') + result = run_module('tower_credential', dict( + name='A credential', + organization=organization.name, + credential_type='foobar', + state='present' + ), admin_user) + assert result.get('failed', False), result + assert 'foobar was not found on the Tower server' in result['msg'] + + +@pytest.mark.django_db +def test_make_use_of_custom_credential_type(run_module, admin_user, cred_type): + Organization.objects.create(name='test-org') + result = run_module('tower_credential', dict( name='Galaxy Token for Steve', organization='test-org', - credential_type='Ansible Galaxy Token', + credential_type=cred_type.name, inputs={'token': '7rEZK38DJl58A7RxA6EC7lLvUHbBQ1'}, state='present' ), admin_user) cred = Credential.objects.get(name='Galaxy Token for Steve') - assert cred.credential_type_id == ct.id + assert cred.credential_type_id == cred_type.id assert list(cred.inputs.keys()) == ['token'] assert cred.inputs['token'].startswith('$encrypted$') assert len(cred.inputs['token']) >= len('$encrypted$') + len('7rEZK38DJl58A7RxA6EC7lLvUHbBQ1') - result.pop('invocation') - assert result == { - "credential": "Galaxy Token for Steve", - "state": "present", - "id": cred.pk, - "changed": True - } + + assert result['name'] == "Galaxy Token for Steve" + assert result['id'] == cred.pk From 5c9ff512485a833ea110c855697613ef2e922ec6 Mon Sep 17 00:00:00 2001 From: John Westcott IV Date: Tue, 31 Mar 2020 12:49:13 -0400 Subject: [PATCH 3/8] Change compare_fields to static method --- awx_collection/plugins/module_utils/tower_api.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/awx_collection/plugins/module_utils/tower_api.py b/awx_collection/plugins/module_utils/tower_api.py index f4ec7d4af2..758e2dccc5 100644 --- a/awx_collection/plugins/module_utils/tower_api.py +++ b/awx_collection/plugins/module_utils/tower_api.py @@ -575,14 +575,15 @@ class TowerModule(AnsibleModule): self.exit_json(**self.json_output) # We need to be able to recursevly step through fields in the case of inputs for credentials. - # They are dicts and we can't just compare them at the top level because the dict returned from tower_cli will have fields like $encrypted$ - def compare_fields(self, new_item, existing_item): + # They are dicts and we can't just compare them at the top level because the dict returned from the tower api will have fields like $encrypted$ + @staticmethod + def compare_fields(new_item, existing_item): needs_update = False for field in new_item: existing_field = existing_item.get(field, None) new_field = new_item.get(field, None) if type(existing_field) == dict and type(new_field) == dict: - needs_update = needs_update or self.compare_fields(new_field, existing_field) + needs_update = needs_update or TowerModule.compare_fields(new_field, existing_field) # If the two items don't match and we are not comparing '' to None # In the case of extra_vars in a job template, we have to pass in {} for nothing ('' is not valid) # But when its returned from the API, instead of {} we get back ''. @@ -618,7 +619,7 @@ class TowerModule(AnsibleModule): self.fail_json(msg="Unable to process update of item due to missing data {0}".format(ke)) # Check to see if anything within the item requires the item to be updated - needs_update = self.compare_fields(new_item, existing_item) + needs_update = TowerModule.compare_fields(new_item, existing_item) # If we decided the item needs to be updated, update it self.json_output['id'] = item_id From 8b881d195d93ac416a5796066f8823713472dc0b Mon Sep 17 00:00:00 2001 From: John Westcott IV Date: Tue, 31 Mar 2020 12:50:34 -0400 Subject: [PATCH 4/8] Change lookup to include organization --- .../plugins/modules/tower_credential.py | 17 +++++++++-------- .../targets/tower_credential/tasks/main.yml | 9 ++++----- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/awx_collection/plugins/modules/tower_credential.py b/awx_collection/plugins/modules/tower_credential.py index cdda10867a..3aa2343788 100644 --- a/awx_collection/plugins/modules/tower_credential.py +++ b/awx_collection/plugins/modules/tower_credential.py @@ -45,7 +45,7 @@ options: credential_type: description: - Name of credential type. - - Will be prefered over kind + - Will be preferred over kind required: False version_added: "2.10" type: str @@ -340,13 +340,14 @@ def main(): cred_type_id = module.resolve_name_to_id('credential_types', credential_type if credential_type else KIND_CHOICES[kind]) - # Attempt to look up the object based on the provided name and inventory ID - credential = module.get_one('credentials', **{ - 'data': { - 'name': name, - 'credential_type': cred_type_id, - } - }) + # Attempt to look up the object based on the provided name, credential type and optional organization + lookup_data = { + 'name': name, + 'credential_type': cred_type_id, + } + if organization: + lookup_data['organization'] = org_id + credential = module.get_one('credentials', **{'data': lookup_data}) # Create credential input from legacy inputs credential_inputs = {} diff --git a/awx_collection/tests/integration/targets/tower_credential/tasks/main.yml b/awx_collection/tests/integration/targets/tower_credential/tasks/main.yml index 42bf108d30..aef37c3ff1 100644 --- a/awx_collection/tests/integration/targets/tower_credential/tasks/main.yml +++ b/awx_collection/tests/integration/targets/tower_credential/tasks/main.yml @@ -42,7 +42,7 @@ organization: Default user: admin kind: ssh - authorize: False + authorize: false authorize_password: 'test' client: 'test' security_token: 'test' @@ -62,7 +62,7 @@ vault_id: 'test' ssh_key_unlock: 'test' state: absent - ignore_errors: True + ignore_errors: true register: result - assert: @@ -125,7 +125,6 @@ - name: Delete a User-specific credential tower_credential: name: "{{ ssh_cred_name1 }}" - organization: Default user: admin state: absent kind: ssh @@ -253,7 +252,7 @@ ssh_key_data: "{{ tempdir.stdout }}/id_rsa" ssh_key_unlock: "passphrase" register: result - ignore_errors: True + ignore_errors: true - assert: that: @@ -353,7 +352,7 @@ description: An example Vault credential vault_password: secret-vault register: result - ignore_errors: True + ignore_errors: true - assert: that: From 6d08e2151111f3c7adee6871e115a196d87117f6 Mon Sep 17 00:00:00 2001 From: John Westcott IV Date: Wed, 1 Apr 2020 11:10:45 -0400 Subject: [PATCH 5/8] Resolving comment and updating tests --- .../plugins/module_utils/tower_api.py | 8 +++++- awx_collection/test/awx/test_credential.py | 26 ++++++++++++------- 2 files changed, 23 insertions(+), 11 deletions(-) diff --git a/awx_collection/plugins/module_utils/tower_api.py b/awx_collection/plugins/module_utils/tower_api.py index 758e2dccc5..a7bbc93857 100644 --- a/awx_collection/plugins/module_utils/tower_api.py +++ b/awx_collection/plugins/module_utils/tower_api.py @@ -575,7 +575,13 @@ class TowerModule(AnsibleModule): self.exit_json(**self.json_output) # We need to be able to recursevly step through fields in the case of inputs for credentials. - # They are dicts and we can't just compare them at the top level because the dict returned from the tower api will have fields like $encrypted$ + # They are dicts and we can't just compare them at the top level because the dict from the tower api may have more fields that we have. + # For example, say someone did: + # - tower_credential: + # name: 'a cred' + # username: 'John' + # Our new dict would be like { 'username': 'new_name' } + # But the existing cred from tower might come back as: { 'username': 'new_name', 'password': '$encrypted$', 'field2': 'something else' } @staticmethod def compare_fields(new_item, existing_item): needs_update = False diff --git a/awx_collection/test/awx/test_credential.py b/awx_collection/test/awx/test_credential.py index 31ba9326ff..c0bcaee755 100644 --- a/awx_collection/test/awx/test_credential.py +++ b/awx_collection/test/awx/test_credential.py @@ -106,7 +106,7 @@ def test_create_custom_credential_type(run_module, admin_user): @pytest.mark.django_db -def test_kind_ct_exclusivity(run_module, admin_user, organization, cred_type, silence_deprecation): +def test_ct_precedence_over_kind(run_module, admin_user, organization, cred_type, silence_deprecation): result = run_module('tower_credential', dict( name='A credential', organization=organization.name, @@ -114,22 +114,28 @@ def test_kind_ct_exclusivity(run_module, admin_user, organization, cred_type, si credential_type=cred_type.name, state='present' ), admin_user) - assert result.get('failed', False), result.get('msg', result) - assert result['msg'] == 'parameters are mutually exclusive: kind|credential_type' + assert not result.get('failed', False), result.get('msg', result) + + cred = Credential.objects.get(name='A credential') + + assert cred.credential_type == cred_type.name @pytest.mark.django_db -def test_input_exclusivity(run_module, admin_user, organization): +def test_input_overrides_old_fields(run_module, admin_user, organization): result = run_module('tower_credential', dict( - name='A credential', + name='A Vault credential', organization=organization.name, - kind='ssh', - inputs={'token': '7rEZK38DJl58A7RxA6EC7lLvUHbBQ1'}, - security_token='7rEZK38DJl58A7RxA6EC7lLvUHbBQ1', + kind='Vault', + inputs={'vault_id': 'asdf'}, + vault_id='1234', state='present' ), admin_user) - assert result.get('failed', False), result - assert result['msg'] == 'parameters are mutually exclusive: inputs|security_token' + assert not result.get('failed', False), result + + cred = Credential.objects.get(name='A Vault credential') + + assert cred.inputs.vault_id == 'asdf' @pytest.mark.django_db From f5cf7c204fce324153c93193803af5e0b861941f Mon Sep 17 00:00:00 2001 From: beeankha Date: Wed, 1 Apr 2020 16:08:05 -0400 Subject: [PATCH 6/8] Update unit test, edit credential module to pass sanity tests --- .../plugins/modules/tower_credential.py | 6 +++--- awx_collection/test/awx/test_credential.py | 15 +++++++++------ .../targets/tower_credential/tasks/main.yml | 2 +- 3 files changed, 13 insertions(+), 10 deletions(-) diff --git a/awx_collection/plugins/modules/tower_credential.py b/awx_collection/plugins/modules/tower_credential.py index 3aa2343788..1cdc222de8 100644 --- a/awx_collection/plugins/modules/tower_credential.py +++ b/awx_collection/plugins/modules/tower_credential.py @@ -31,7 +31,7 @@ options: new_name: description: - Setting this option will change the existing name (looked up via the name field. - required: True + required: False type: str description: description: @@ -336,7 +336,7 @@ def main(): team_id = module.resolve_name_to_id('teams', team) if kind: - module.deprecate(msg='The kind parameter has been depricated, please use credential_type instead', version="3.6") + module.deprecate(msg='The kind parameter has been deprecated, please use credential_type instead', version="3.6") cred_type_id = module.resolve_name_to_id('credential_types', credential_type if credential_type else KIND_CHOICES[kind]) @@ -353,7 +353,7 @@ def main(): credential_inputs = {} for legacy_input in OLD_INPUT_NAMES: if module.params.get(legacy_input) is not None: - module.deprecate(msg='{0} parameter has been depricated, please use inputs instead'.format(legacy_input), version="3.6") + module.deprecate(msg='{0} parameter has been deprecated, please use inputs instead'.format(legacy_input), version="3.6") credential_inputs[legacy_input] = module.params.get(legacy_input) if inputs: credential_inputs.update(inputs) diff --git a/awx_collection/test/awx/test_credential.py b/awx_collection/test/awx/test_credential.py index c0bcaee755..3707426755 100644 --- a/awx_collection/test/awx/test_credential.py +++ b/awx_collection/test/awx/test_credential.py @@ -118,24 +118,27 @@ def test_ct_precedence_over_kind(run_module, admin_user, organization, cred_type cred = Credential.objects.get(name='A credential') - assert cred.credential_type == cred_type.name + assert cred.credential_type == cred_type @pytest.mark.django_db def test_input_overrides_old_fields(run_module, admin_user, organization): + # create the vault credential type + ct = CredentialType.defaults['vault']() + ct.save() result = run_module('tower_credential', dict( name='A Vault credential', organization=organization.name, - kind='Vault', - inputs={'vault_id': 'asdf'}, + kind='vault', vault_id='1234', - state='present' + inputs={'vault_id': 'asdf'}, + state='present', ), admin_user) - assert not result.get('failed', False), result + assert not result.get('failed', False), result.get('msg', result) cred = Credential.objects.get(name='A Vault credential') - assert cred.inputs.vault_id == 'asdf' + assert cred.inputs['vault_id'] == 'asdf' @pytest.mark.django_db diff --git a/awx_collection/tests/integration/targets/tower_credential/tasks/main.yml b/awx_collection/tests/integration/targets/tower_credential/tasks/main.yml index aef37c3ff1..b7e7172eba 100644 --- a/awx_collection/tests/integration/targets/tower_credential/tasks/main.yml +++ b/awx_collection/tests/integration/targets/tower_credential/tasks/main.yml @@ -36,7 +36,7 @@ set_fact: ssh_key_data: "{{ lookup('file', tempdir.stdout + '/id_rsa') }}" -- name: Test deprication warnings +- name: Test deprecation warnings tower_credential: name: "{{ ssh_cred_name1 }}" organization: Default From c1bb62cc3691131005b9847d2252e7717a85eddd Mon Sep 17 00:00:00 2001 From: John Westcott IV Date: Fri, 3 Apr 2020 12:47:28 -0400 Subject: [PATCH 7/8] Removing recursive check, allowwing old pattern to commence --- .../plugins/module_utils/tower_api.py | 34 +++++----------- .../plugins/modules/tower_credential.py | 40 +++++++++++++++++++ .../targets/tower_credential/tasks/main.yml | 4 +- 3 files changed, 51 insertions(+), 27 deletions(-) diff --git a/awx_collection/plugins/module_utils/tower_api.py b/awx_collection/plugins/module_utils/tower_api.py index a7bbc93857..b7145e15f4 100644 --- a/awx_collection/plugins/module_utils/tower_api.py +++ b/awx_collection/plugins/module_utils/tower_api.py @@ -574,30 +574,6 @@ class TowerModule(AnsibleModule): else: self.exit_json(**self.json_output) - # We need to be able to recursevly step through fields in the case of inputs for credentials. - # They are dicts and we can't just compare them at the top level because the dict from the tower api may have more fields that we have. - # For example, say someone did: - # - tower_credential: - # name: 'a cred' - # username: 'John' - # Our new dict would be like { 'username': 'new_name' } - # But the existing cred from tower might come back as: { 'username': 'new_name', 'password': '$encrypted$', 'field2': 'something else' } - @staticmethod - def compare_fields(new_item, existing_item): - needs_update = False - for field in new_item: - existing_field = existing_item.get(field, None) - new_field = new_item.get(field, None) - if type(existing_field) == dict and type(new_field) == dict: - needs_update = needs_update or TowerModule.compare_fields(new_field, existing_field) - # If the two items don't match and we are not comparing '' to None - # In the case of extra_vars in a job template, we have to pass in {} for nothing ('' is not valid) - # But when its returned from the API, instead of {} we get back ''. - elif existing_field != new_field and not (existing_field in (None, '') and new_field in ('', {})): - # Something doesn't match so let's update it - needs_update = True - return needs_update - def update_if_needed(self, existing_item, new_item, on_update=None, associations=None): # This will exit from the module on its own # If the method successfully updates an item and on_update param is defined, @@ -625,7 +601,15 @@ class TowerModule(AnsibleModule): self.fail_json(msg="Unable to process update of item due to missing data {0}".format(ke)) # Check to see if anything within the item requires the item to be updated - needs_update = TowerModule.compare_fields(new_item, existing_item) + needs_update = False + for field in new_item: + existing_field = existing_item.get(field, None) + new_field = new_item.get(field, None) + # If the two items don't match and we are not comparing '' to None + if existing_field != new_field and not (existing_field in (None, '') and new_field == ''): + # Something doesn't match so let's update it + needs_update = True + break # If we decided the item needs to be updated, update it self.json_output['id'] = item_id diff --git a/awx_collection/plugins/modules/tower_credential.py b/awx_collection/plugins/modules/tower_credential.py index 1cdc222de8..c57efc87d8 100644 --- a/awx_collection/plugins/modules/tower_credential.py +++ b/awx_collection/plugins/modules/tower_credential.py @@ -194,7 +194,12 @@ options: required: False type: str version_added: "3.7" + extends_documentation_fragment: awx.awx.auth + +notes: + - Values `inputs` and the other deprecated fields (such as `tenant`) are replacements of existing values. + See the last 4 examples for details. ''' @@ -224,6 +229,7 @@ EXAMPLES = ''' slurp: src: '$HOME/.ssh/aws-private.pem' register: aws_ssh_key + - name: Add Credential Into Tower tower_credential: name: Workshop Credential @@ -242,6 +248,40 @@ EXAMPLES = ''' tower_username: admin tower_password: ansible tower_host: https://localhost + +- name: Create a Vaiult credential (example for notes) + tower_credential: + name: Example password + credential_type: Vault + organization: Default + inputs: + vault_password: 'hello' + vault_id: 'My ID' + +- name: Bad password update (will replace vault_id) + tower_credential: + name: Example password + credential_type: Vault + organization: Default + inputs: + vault_password: 'new_password' + +- name: Another bad password update (will replace vault_id) + tower_credential: + name: Example password + credential_type: Vault + organization: Default + vault_password: 'new_password' + +- name: A safe way to update a password and keep vault_id + tower_credential: + name: Example password + credential_type: Vault + organization: Default + inputs: + vault_password: 'new_password' + vault_id: 'My ID' + ''' from ..module_utils.tower_api import TowerModule diff --git a/awx_collection/tests/integration/targets/tower_credential/tasks/main.yml b/awx_collection/tests/integration/targets/tower_credential/tasks/main.yml index b7e7172eba..55d98396b5 100644 --- a/awx_collection/tests/integration/targets/tower_credential/tasks/main.yml +++ b/awx_collection/tests/integration/targets/tower_credential/tasks/main.yml @@ -177,7 +177,7 @@ that: - result is changed -- name: Create a valid SSH credential (new school) (no change) +- name: Create a valid SSH credential (new school) tower_credential: name: "{{ ssh_cred_name2 }}" organization: Default @@ -193,7 +193,7 @@ # This should no longer be changed because we aren't passing any secure fields - assert: that: - - result is not changed + - result is changed - name: Create a valid SSH credential from lookup source (old school) tower_credential: From b0f68d97dafeeddfa7532582fef11c5a1a575817 Mon Sep 17 00:00:00 2001 From: beeankha Date: Mon, 6 Apr 2020 12:38:46 -0400 Subject: [PATCH 8/8] Update comment in test playbook: --- .../tests/integration/targets/tower_credential/tasks/main.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/awx_collection/tests/integration/targets/tower_credential/tasks/main.yml b/awx_collection/tests/integration/targets/tower_credential/tasks/main.yml index 55d98396b5..e55383c56a 100644 --- a/awx_collection/tests/integration/targets/tower_credential/tasks/main.yml +++ b/awx_collection/tests/integration/targets/tower_credential/tasks/main.yml @@ -190,7 +190,7 @@ become_username: superuser register: result -# This should no longer be changed because we aren't passing any secure fields +# This shows as "changed" because these listed inputs replace the existing inputs from the previous task - assert: that: - result is changed