From 7764f1c1a53cf0efc17adb04744125f1f773c6e4 Mon Sep 17 00:00:00 2001 From: beeankha Date: Wed, 2 Sep 2020 22:05:25 -0400 Subject: [PATCH] Fix inv source org lookup, adjust unit tests (since org is now required for inv source module), update README, edit typos in test-related README --- awx/main/tests/factories/README.md | 4 +- .../plugins/modules/tower_inventory_source.py | 36 +++++++++------ .../modules/tower_inventory_source_update.py | 46 ++++++++++++------- .../test/awx/test_inventory_source.py | 40 ++++++---------- .../tower_inventory_source/tasks/main.yml | 2 + .../tasks/main.yml | 13 +++--- .../template_galaxy/templates/README.md.j2 | 1 + 7 files changed, 75 insertions(+), 67 deletions(-) diff --git a/awx/main/tests/factories/README.md b/awx/main/tests/factories/README.md index c451c02598..916c996cfa 100644 --- a/awx/main/tests/factories/README.md +++ b/awx/main/tests/factories/README.md @@ -52,11 +52,11 @@ patterns -------- `mk` functions are single object fixtures. They should create only a single object with the minimum deps. -They should also accept a `persited` flag, if they must be persisted to work, they raise an error if persisted=False +They should also accept a `persisted` flag, if they must be persisted to work, they raise an error if persisted=False `generate` and `apply` functions are helpers that build up the various parts of a `create` functions objects. These should be useful for more than one create function to use and should explicitly accept all of the values needed -to execute. These functions should also be robust and have very speciifc error reporting about constraints and/or +to execute. These functions should also be robust and have very specific error reporting about constraints and/or bad values. `create` functions compose many of the `mk` and `generate` functions to make different object diff --git a/awx_collection/plugins/modules/tower_inventory_source.py b/awx_collection/plugins/modules/tower_inventory_source.py index eb316fd9ca..7c7c506826 100644 --- a/awx_collection/plugins/modules/tower_inventory_source.py +++ b/awx_collection/plugins/modules/tower_inventory_source.py @@ -131,6 +131,7 @@ options: organization: description: - Name of the inventory source's inventory's organization. + required: True type: str extends_documentation_fragment: awx.awx.auth ''' @@ -173,7 +174,7 @@ def main(): enabled_value=dict(), host_filter=dict(), credential=dict(), - organization=dict(), + organization=dict(required=True), overwrite=dict(type='bool'), overwrite_vars=dict(type='bool'), custom_virtualenv=dict(), @@ -196,29 +197,34 @@ def main(): name = module.params.get('name') new_name = module.params.get('new_name') inventory = module.params.get('inventory') + organization = module.params.get('organization') source_script = module.params.get('source_script') credential = module.params.get('credential') source_project = module.params.get('source_project') state = module.params.get('state') - new_fields = {} - organization_id = None - organization = module.params.get('organization') - if organization: - organization_id = module.get_one_by_name_or_id('organizations', organization) - # Attempt to look up inventory source based on the provided name and inventory ID - inventory_id = module.resolve_name_to_id('inventories', inventory) - inventory_source = module.get_one('inventory_sources', **{ + org_id = module.resolve_name_to_id('organizations', organization) + inventory_object = module.get_one('inventories', **{ + 'data': { + 'name': inventory, + 'organization': org_id, + } + }) + + if not inventory_object: + module.fail_json(msg='The specified inventory was not found.') + + inventory_source_object = module.get_one('inventory_sources', **{ 'data': { 'name': name, - 'inventory': inventory_id, + 'inventory': inventory_object['id'], } }) 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(inventory_source) + module.delete_if_needed(inventory_source_object) # Attempt to look up associated field items the user specified. association_fields = {} @@ -244,7 +250,7 @@ def main(): # Create the data that gets sent for create and update inventory_source_fields = { 'name': new_name if new_name else name, - 'inventory': inventory_id, + 'inventory': inventory_object['id'], } # Attempt to look up the related items the user specified (these will fail the module if not found) @@ -273,12 +279,12 @@ def main(): inventory_source_fields['source_vars'] = dumps(inventory_source_fields['source_vars']) # Sanity check on arguments - if state == 'present' and not inventory_source and not inventory_source_fields['source']: + if state == 'present' and not inventory_source_object and not inventory_source_fields['source']: module.fail_json(msg="If creating a new inventory source, the source param must be present") - # If the state was present we can let the module build or update the existing inventory_source, this will return on its own + # If the state was present we can let the module build or update the existing inventory_source_object, this will return on its own module.create_or_update_if_needed( - inventory_source, inventory_source_fields, + inventory_source_object, inventory_source_fields, endpoint='inventory_sources', item_type='inventory source', associations=association_fields ) diff --git a/awx_collection/plugins/modules/tower_inventory_source_update.py b/awx_collection/plugins/modules/tower_inventory_source_update.py index c8ac301ef3..6f9cdf6de1 100644 --- a/awx_collection/plugins/modules/tower_inventory_source_update.py +++ b/awx_collection/plugins/modules/tower_inventory_source_update.py @@ -36,7 +36,7 @@ options: description: - Name of the inventory source's inventory's organization. type: str - required: False + required: True wait: description: - Wait for the job to complete. @@ -91,7 +91,7 @@ def main(): argument_spec = dict( inventory=dict(required=True), inventory_source=dict(required=True), - organization=dict(), + organization=dict(required=True), wait=dict(default=False, type='bool'), interval=dict(default=1.0, type='float'), timeout=dict(default=None, type='int'), @@ -103,20 +103,31 @@ def main(): # Extract our parameters inventory = module.params.get('inventory') inventory_source = module.params.get('inventory_source') + organization = module.params.get('organization') wait = module.params.get('wait') interval = module.params.get('interval') timeout = module.params.get('timeout') - new_fields = {} - organization_id = None - organization = module.params.get('organization') - if organization: - organization_id = module.get_one_by_name_or_id('organizations', organization) + org_id = module.resolve_name_to_id('organizations', organization) + inventory_object = module.get_one('inventories', **{ + 'data': { + 'name': inventory, + 'organization': org_id, + } + }) - # Attempt to look up the inventory the user specified (these will fail the module if not found) - inventory_object = module.get_one_by_name_or_id('inventories', inventory) - # Return all inventory sources related to the specified inventory - inventory_source_object = module.get_one_by_name_or_id(inventory_object['related']['inventory_sources'], inventory_source) + if not inventory_object: + module.fail_json(msg='The specified inventory was not found.') + + inventory_source_object = module.get_one('inventory_sources', **{ + 'data': { + 'name': inventory_source, + 'inventory': inventory_object['id'], + } + }) + + if not inventory_source_object: + module.fail_json(msg='The specified inventory source was not found.') # Sync the inventory source(s) inventory_source_update_results = module.post_endpoint(inventory_source_object['related']['update'], **{'data': {}}) @@ -124,12 +135,13 @@ def main(): if inventory_source_update_results['status_code'] != 202: module.fail_json(msg="Failed to update inventory source, see response for details", **{'response': inventory_source_update_results}) - inventory_source_update_results = module.wait_on_url( - url=inventory_source_update_results['json']['url'], - object_name=inventory_object, - object_type='inventory_update', - timeout=timeout, interval=interval - ) + if wait: + inventory_source_update_results = module.wait_on_url( + url=inventory_source_update_results['json']['url'], + object_name=inventory_object, + object_type='inventory_update', + timeout=timeout, interval=interval + ) module.exit_json(**{ 'changed': True, diff --git a/awx_collection/test/awx/test_inventory_source.py b/awx_collection/test/awx/test_inventory_source.py index b27653fb94..b1b362efad 100644 --- a/awx_collection/test/awx/test_inventory_source.py +++ b/awx_collection/test/awx/test_inventory_source.py @@ -24,11 +24,12 @@ def project(base_inventory): @pytest.mark.django_db -def test_inventory_source_create(run_module, admin_user, base_inventory, project): +def test_inventory_source_create(run_module, admin_user, base_inventory, project, organization): source_path = '/var/lib/awx/example_source_path/' result = run_module('tower_inventory_source', dict( name='foo', inventory=base_inventory.name, + organization='test-org', state='present', source='scm', source_path=source_path, @@ -45,30 +46,6 @@ def test_inventory_source_create(run_module, admin_user, base_inventory, project } -@pytest.mark.django_db -def test_create_inventory_source_implied_org(run_module, admin_user): - org = Organization.objects.create(name='test-org') - inv = Inventory.objects.create(name='test-inv', organization=org) - - # Credential is not required for ec2 source, because of IAM roles - result = run_module('tower_inventory_source', dict( - name='Test Inventory Source', - inventory='test-inv', - source='ec2', - state='present' - ), admin_user) - assert result.pop('changed', None), result - - inv_src = InventorySource.objects.get(name='Test Inventory Source') - assert inv_src.inventory == inv - - result.pop('invocation') - assert result == { - "name": "Test Inventory Source", - "id": inv_src.id, - } - - @pytest.mark.django_db def test_create_inventory_source_multiple_orgs(run_module, admin_user): org = Organization.objects.create(name='test-org') @@ -80,7 +57,8 @@ def test_create_inventory_source_multiple_orgs(run_module, admin_user): result = run_module('tower_inventory_source', dict( name='Test Inventory Source', - inventory=inv2.id, + inventory=inv2.name, + organization='test-org-number-two', source='ec2', state='present' ), admin_user) @@ -104,6 +82,7 @@ def test_create_inventory_source_with_venv(run_module, admin_user, base_inventor result = run_module('tower_inventory_source', dict( name='foo', inventory=base_inventory.name, + organization='test-org', state='present', source='scm', source_project=project.name, @@ -120,7 +99,7 @@ def test_create_inventory_source_with_venv(run_module, admin_user, base_inventor @pytest.mark.django_db -def test_custom_venv_no_op(run_module, admin_user, base_inventory, mocker, project): +def test_custom_venv_no_op(run_module, admin_user, base_inventory, mocker, project, organization): """If the inventory source is modified, then it should not blank fields unrelated to the params that the user passed. This enforces assumptions about the behavior of the AnsibleModule @@ -140,6 +119,7 @@ def test_custom_venv_no_op(run_module, admin_user, base_inventory, mocker, proje name='foo', description='this is the changed description', inventory=base_inventory.name, + organization='test-org', source='scm', # is required, but behavior is arguable state='present', source_project=project.name, @@ -156,6 +136,7 @@ def test_falsy_value(run_module, admin_user, base_inventory): result = run_module('tower_inventory_source', dict( name='falsy-test', inventory=base_inventory.name, + organization='test-org', source='ec2', update_on_launch=True ), admin_user) @@ -168,6 +149,7 @@ def test_falsy_value(run_module, admin_user, base_inventory): result = run_module('tower_inventory_source', dict( name='falsy-test', inventory=base_inventory.name, + organization='test-org', # source='ec2', update_on_launch=False ), admin_user) @@ -203,6 +185,7 @@ def test_missing_required_credential(run_module, admin_user, base_inventory): result = run_module('tower_inventory_source', dict( name='Test Azure Source', inventory=base_inventory.name, + organization='test-org', source='azure_rm', state='present' ), admin_user) @@ -216,6 +199,7 @@ def test_source_project_not_for_cloud(run_module, admin_user, base_inventory, pr result = run_module('tower_inventory_source', dict( name='Test ec2 Inventory Source', inventory=base_inventory.name, + organization='test-org', source='ec2', state='present', source_project=project.name @@ -230,6 +214,7 @@ def test_source_path_not_for_cloud(run_module, admin_user, base_inventory): result = run_module('tower_inventory_source', dict( name='Test ec2 Inventory Source', inventory=base_inventory.name, + organization='test-org', source='ec2', state='present', source_path='where/am/I' @@ -243,6 +228,7 @@ def test_source_path_not_for_cloud(run_module, admin_user, base_inventory): def test_scm_source_needs_project(run_module, admin_user, base_inventory): result = run_module('tower_inventory_source', dict( name='SCM inventory without project', + organization='test-org', inventory=base_inventory.name, state='present', source='scm', diff --git a/awx_collection/tests/integration/targets/tower_inventory_source/tasks/main.yml b/awx_collection/tests/integration/targets/tower_inventory_source/tasks/main.yml index fb5f33c3ca..738d432711 100644 --- a/awx_collection/tests/integration/targets/tower_inventory_source/tasks/main.yml +++ b/awx_collection/tests/integration/targets/tower_inventory_source/tasks/main.yml @@ -31,6 +31,7 @@ credential: "{{ openstack_cred }}" overwrite: true update_on_launch: true + organization: Default source_vars: private: false source: openstack @@ -47,6 +48,7 @@ credential: "Does Not Exit" source_project: "Does Not Exist" source_script: "Does Not Exist" + organization: Default state: absent - assert: diff --git a/awx_collection/tests/integration/targets/tower_inventory_source_update/tasks/main.yml b/awx_collection/tests/integration/targets/tower_inventory_source_update/tasks/main.yml index fd6cd48f2f..01b29a0cf0 100644 --- a/awx_collection/tests/integration/targets/tower_inventory_source_update/tasks/main.yml +++ b/awx_collection/tests/integration/targets/tower_inventory_source_update/tasks/main.yml @@ -32,12 +32,11 @@ name: "{{ inv_name }}" organization: "{{ org_name }}" state: present - register: created_inventory - name: Create another inventory w/ same name tower_inventory: name: "{{ inv_name }}" - organization: "{{ org_name }}" + organization: Default state: present register: created_inventory @@ -64,7 +63,7 @@ - name: Test Inventory Source Update tower_inventory_source_update: inventory: "{{ inv_name }}" - inventory_source: "{{ inv_source1 }}" + inventory_source: "{{ inv_source2 }}" organization: Default register: result @@ -75,10 +74,12 @@ - name: Test Inventory Source Update for All Sources tower_inventory_source_update: inventory: "{{ inv_name }}" - inventory_source: "{{ item }}" - organization: "{{ created_org.id }}" + inventory_source: "{{ item.name }}" + organization: Default wait: true - loop: "{{ query('awx.awx.tower_api', 'inventory_sources', query_params={ 'inventory': created_inventory.id }, return_ids=True ) }}" + loop: "{{ query('awx.awx.tower_api', 'inventory_sources', query_params={ 'inventory': created_inventory.id }, expect_objects=True, return_objects=True) }}" + loop_control: + label: "{{ item.name }}" register: result - assert: diff --git a/awx_collection/tools/roles/template_galaxy/templates/README.md.j2 b/awx_collection/tools/roles/template_galaxy/templates/README.md.j2 index 8a5743d34f..e1de06ec97 100644 --- a/awx_collection/tools/roles/template_galaxy/templates/README.md.j2 +++ b/awx_collection/tools/roles/template_galaxy/templates/README.md.j2 @@ -104,6 +104,7 @@ The following notes are changes that may require changes to playbooks: - The `notification_configuration` parameter of `tower_notification_template` 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. - The HipChat `notification_type` has been removed and can no longer be created using the `tower_notification_template` module. + - The `tower_inventory_source` module now requires the `organization` parameter in order to specify the inventory source's inventory's organization. {% if collection_package | lower() == "awx" %} ## Running Unit Tests