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

@@ -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

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)
+    org_id = module.resolve_name_to_id('organizations', organization)
-    inventory_source = module.get_one('inventory_sources', **{
+    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
     )

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 = {}
+    org_id = module.resolve_name_to_id('organizations', organization)
-    organization_id = None
+    inventory_object = module.get_one('inventories', **{
-    organization = module.params.get('organization')
+        'data': {
-    if organization:
+            'name': inventory,
-        organization_id = module.get_one_by_name_or_id('organizations', organization)
+            'organization': org_id,
+        }
+    })
 
-    # Attempt to look up the inventory the user specified (these will fail the module if not found)
+    if not inventory_object:
-    inventory_object = module.get_one_by_name_or_id('inventories', inventory)
+        module.fail_json(msg='The specified inventory was not found.')
-    # 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)
+    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(
+    if wait:
-        url=inventory_source_update_results['json']['url'],
+        inventory_source_update_results = module.wait_on_url(
-        object_name=inventory_object,
+            url=inventory_source_update_results['json']['url'],
-        object_type='inventory_update',
+            object_name=inventory_object,
-        timeout=timeout, interval=interval
+            object_type='inventory_update',
-    )
+            timeout=timeout, interval=interval
+        )
 
     module.exit_json(**{
         'changed': True,

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',

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:

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 }}"
+        inventory_source: "{{ item.name }}"
-        organization: "{{ created_org.id }}"
+        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:

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