[last PR stuff] + Add warning if configs specified in 2 params (#5)

* Lean on API validation for tower_inventory_source arg errors

used for
 - validating needed credential is given
 - missing source_project for scm sources

* Add warning when config is specified in 2 places

Fix up unit tests, address multiple comments re: backwards compatibility, redundant methods, etc.

Update new_name and variables parameters, update unit tests

awx_collection/test/awx/conftest.py

@@ -47,7 +47,19 @@ def sanitize_dict(din):
 
 
 @pytest.fixture
-def run_module(request):
+def collection_import():
+    """These tests run assuming that the awx_collection folder is inserted
+    into the PATH before-hand. But all imports internally to the collection
+    go through this fixture so that can be changed if needed.
+    For instance, we could switch to fully-qualified import paths.
+    """
+    def rf(path):
+        return importlib.import_module(path)
+    return rf
+
+
+@pytest.fixture
+def run_module(request, collection_import):
     def rf(module_name, module_params, request_user):
 
         def new_request(self, method, url, **kwargs):
@@ -97,7 +109,7 @@ def run_module(request):
         # Note that a proper Ansiballz explosion of the modules will have an import path like:
         # ansible_collections.awx.awx.plugins.modules.{}
         # We should consider supporting that in the future
-        resource_module = importlib.import_module('plugins.modules.{0}'.format(module_name))
+        resource_module = collection_import('plugins.modules.{0}'.format(module_name))
 
         if not isinstance(module_params, dict):
             raise RuntimeError('Module params must be dict, got {0}'.format(type(module_params)))

awx_collection/test/awx/test_credential.py

@@ -78,10 +78,8 @@ def test_create_custom_credential_type(run_module, admin_user):
 
     ct = CredentialType.objects.get(name='Nexus')
     result.pop('invocation')
-    result.pop('name')
     assert result == {
-        "credential_type": "Nexus",
-        "state": "present",
+        "name": "Nexus",
         "id": ct.pk,
         "changed": True,
     }

awx_collection/test/awx/test_group.py

@@ -10,18 +10,19 @@ from awx.main.models import Organization, Inventory, Group
 def test_create_group(run_module, admin_user):
     org = Organization.objects.create(name='test-org')
     inv = Inventory.objects.create(name='test-inv', organization=org)
+    variables = {"ansible_network_os": "iosxr"}
 
     result = run_module('tower_group', dict(
         name='Test Group',
         inventory='test-inv',
-        variables='ansible_network_os: iosxr',
+        variables=variables,
         state='present'
     ), admin_user)
     assert result.get('changed'), result
 
     group = Group.objects.get(name='Test Group')
     assert group.inventory == inv
-    assert group.variables == 'ansible_network_os: iosxr'
+    assert group.variables == '{"ansible_network_os": "iosxr"}'
 
     result.pop('invocation')
     assert result == {
@@ -39,13 +40,11 @@ def test_tower_group_idempotent(run_module, admin_user):
     group = Group.objects.create(
         name='Test Group',
         inventory=inv,
-        variables='ansible_network_os: iosxr'
     )
 
     result = run_module('tower_group', dict(
         name='Test Group',
         inventory='test-inv',
-        variables='ansible_network_os: iosxr',
         state='present'
     ), admin_user)
 

awx_collection/test/awx/test_inventory_source.py

@@ -10,25 +10,29 @@ from awx.main.models import Organization, Inventory, InventorySource, Project
 def base_inventory():
     org = Organization.objects.create(name='test-org')
     inv = Inventory.objects.create(name='test-inv', organization=org)
-    Project.objects.create(
-        name='test-proj',
-        organization=org,
-        scm_type='git',
-        scm_url='https://github.com/ansible/test-playbooks.git',
-    )
     return inv
 
 
+@pytest.fixture
+def project(base_inventory):
+    return Project.objects.create(
+        name='test-proj',
+        organization=base_inventory.organization,
+        scm_type='git',
+        scm_url='https://github.com/ansible/test-playbooks.git',
+    )
+
+
 @pytest.mark.django_db
-def test_inventory_source_create(run_module, admin_user, base_inventory):
+def test_inventory_source_create(run_module, admin_user, base_inventory, project):
     source_path = '/var/lib/awx/example_source_path/'
     result = run_module('tower_inventory_source', dict(
         name='foo',
-        inventory='test-inv',
+        inventory=base_inventory.name,
         state='present',
         source='scm',
         source_path=source_path,
-        source_project='test-proj'
+        source_project=project.name
     ), admin_user)
     assert result.pop('changed', None), result
 
@@ -46,6 +50,7 @@ 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',
@@ -92,16 +97,16 @@ def test_create_inventory_source_multiple_orgs(run_module, admin_user):
 
 
 @pytest.mark.django_db
-def test_create_inventory_source_with_venv(run_module, admin_user, base_inventory, mocker):
+def test_create_inventory_source_with_venv(run_module, admin_user, base_inventory, mocker, project):
     path = '/var/lib/awx/venv/custom-venv/foobar13489435/'
     source_path = '/var/lib/awx/example_source_path/'
     with mocker.patch('awx.main.models.mixins.get_custom_venv_choices', return_value=[path]):
         result = run_module('tower_inventory_source', dict(
             name='foo',
-            inventory='test-inv',
+            inventory=base_inventory.name,
             state='present',
             source='scm',
-            source_project='test-proj',
+            source_project=project.name,
             custom_virtualenv=path,
             source_path=source_path
         ), admin_user)
@@ -115,7 +120,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):
+def test_custom_venv_no_op(run_module, admin_user, base_inventory, mocker, project):
     """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
@@ -125,7 +130,7 @@ def test_custom_venv_no_op(run_module, admin_user, base_inventory, mocker):
     inv_src = InventorySource.objects.create(
         name='foo',
         inventory=base_inventory,
-        source_project=Project.objects.get(name='test-proj'),
+        source_project=project,
         source='scm',
         custom_virtualenv='/venv/foobar/'
     )
@@ -134,13 +139,93 @@ def test_custom_venv_no_op(run_module, admin_user, base_inventory, mocker):
         result = run_module('tower_inventory_source', dict(
             name='foo',
             description='this is the changed description',
-            inventory='test-inv',
+            inventory=base_inventory.name,
             source='scm',  # is required, but behavior is arguable
             state='present',
-            source_project='test-proj',
+            source_project=project.name,
             source_path=source_path
         ), admin_user)
     assert result.pop('changed', None), result
     inv_src.refresh_from_db()
     assert inv_src.custom_virtualenv == '/venv/foobar/'
     assert inv_src.description == 'this is the changed description'
+
+
+# Tests related to source-specific parameters
+#
+# We want to let the API return issues with "this doesn't support that", etc.
+#
+# GUI OPTIONS:
+# - - - - - - - manual:	file:	scm:	ec2:	gce	azure_rm	vmware	sat	cloudforms	openstack	rhv	tower	custom
+# credential		?	?	o	o	r	r		r	r	r		r		r	r	o
+# source_project	?	?	r	-	-	-		-	-	-		-		-	-	-
+# source_path		?	?	r	-	-	-		-	-	-		-		-	-	-
+# verbosity			?	?	o	o	o	o		o	o	o		o		o	o	o
+# overwrite			?	?	o	o	o	o		o	o	o		o		o	o	o
+# overwrite_vars	?	?	o	o	o	o		o	o	o		o		o	o	o
+# update_on_launch	?	?	o	o	o	o		o	o	o		o		o	o	o
+# UoPL          	?	?	o	-	-	-		-	-	-		-		-	-	-
+# source_regions	?	?	-	o	o	o		-	-	-		-		-	-	-
+# instance_filters	?	?	-	o	-	-		o	-	-		-		-	o	-
+# group_by			?	?	-	o	-	-		o	-	-		-		-	-	-
+# source_vars*		?	?	-	o	-	o		o	o	o		o		-	-	-
+# environmet vars*	?	?	o	-	-	-		-	-	-		-		-	-	o
+# source_script		?	?	-	-	-	-		-	-	-		-		-	-	r
+#
+# UoPL - update_on_project_launch
+# * - source_vars are labeled environment_vars on project and custom sources
+
+
+@pytest.mark.django_db
+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,
+        source='azure_rm',
+        state='present'
+    ), admin_user)
+    assert result.pop('failed', None) is True, result
+
+    assert 'Credential is required for a cloud source' in result.get('msg', '')
+
+
+@pytest.mark.django_db
+def test_source_project_not_for_cloud(run_module, admin_user, base_inventory, project):
+    result = run_module('tower_inventory_source', dict(
+        name='Test ec2 Inventory Source',
+        inventory=base_inventory.name,
+        source='ec2',
+        state='present',
+        source_project=project.name
+    ), admin_user)
+    assert result.pop('failed', None) is True, result
+
+    assert 'Cannot set source_project if not SCM type' in result.get('msg', '')
+
+
+@pytest.mark.django_db
+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,
+        source='ec2',
+        state='present',
+        source_path='where/am/I'
+    ), admin_user)
+    assert result.pop('failed', None) is True, result
+
+    assert 'Cannot set source_path if not SCM type' in result.get('msg', '')
+
+
+@pytest.mark.django_db
+def test_scm_source_needs_project(run_module, admin_user, base_inventory):
+    result = run_module('tower_inventory_source', dict(
+        name='SCM inventory without project',
+        inventory=base_inventory.name,
+        state='present',
+        source='scm',
+        source_path='/var/lib/awx/example_source_path/'
+    ), admin_user)
+    assert result.pop('failed', None), result
+
+    assert 'Project required for scm type sources' in result.get('msg', '')

awx_collection/test/awx/test_module_utils.py

@@ -0,0 +1,35 @@
+from __future__ import (absolute_import, division, print_function)
+__metaclass__ = type
+
+import sys
+
+from unittest import mock
+
+import json
+
+
+def test_duplicate_config(collection_import):
+    # imports done here because of PATH issues unique to this test suite
+    TowerModule = collection_import('plugins.module_utils.tower_api').TowerModule
+    data = {
+        'name': 'zigzoom',
+        'zig': 'zoom',
+        'tower_username': 'bob',
+        'tower_config_file': 'my_config'
+    }
+    cli_data = {'ANSIBLE_MODULE_ARGS': data}
+    testargs = ['module_file.py', json.dumps(cli_data)]
+    with mock.patch('ansible.module_utils.basic.AnsibleModule.warn') as mock_warn:
+        with mock.patch.object(sys, 'argv', testargs):
+            with mock.patch.object(TowerModule, 'load_config') as mock_load:
+                argument_spec = dict(
+                    name=dict(required=True),
+                    zig=dict(type='str'),
+                )
+                TowerModule(argument_spec=argument_spec)
+            mock_load.mock_calls[-1] == mock.call('my_config')
+    mock_warn.assert_called_once_with(
+        'The parameter(s) tower_username were provided at the same time as '
+        'tower_config_file. Precedence may be unstable, '
+        'we suggest either using config file or params.'
+    )