Merge pull request #6183 from ryanpetrello/multicredential_job

Various custom credential type/multi-credential selection tidbits

awx/api/filters.py

@@ -265,6 +265,11 @@ class FieldLookupBackend(BaseFilterBackend):
                     key = key[5:]
                     q_not = True
 
+                # Make legacy v1 Job/Template fields work for backwards compatability
+                # TODO: remove after API v1 deprecation period
+                if queryset.model._meta.object_name in ('JobTemplate', 'Job') and key in ('cloud_credential', 'network_credential'):
+                    key = 'extra_credentials'
+
                 # Make legacy v1 Credential fields work for backwards compatability
                 # TODO: remove after API v1 deprecation period
                 #

awx/api/views.py

@@ -2704,6 +2704,12 @@ class JobTemplateExtraCredentialsList(SubListCreateAttachDetachAPIView):
     new_in_api_v2 = True
 
     def is_valid_relation(self, parent, sub, created=False):
+        current_extra_types = [
+            cred.credential_type.pk for cred in parent.extra_credentials.all()
+        ]
+        if sub.credential_type.pk in current_extra_types:
+            return {'error': _('Cannot assign multiple %s credentials.' % sub.credential_type.name)}
+
         if sub.credential_type.kind not in ('net', 'cloud'):
             return {'error': _('Extra credentials must be network or cloud.')}
         return super(JobTemplateExtraCredentialsList, self).is_valid_relation(parent, sub, created)
@@ -3470,7 +3476,7 @@ class JobDetail(RetrieveUpdateDestroyAPIView):
         return super(JobDetail, self).destroy(request, *args, **kwargs)
 
 
-class JobExtraCredentialsList(SubListCreateAttachDetachAPIView):
+class JobExtraCredentialsList(SubListAPIView):
 
     model = Credential
     serializer_class = CredentialSerializer
@@ -3479,11 +3485,6 @@ class JobExtraCredentialsList(SubListCreateAttachDetachAPIView):
     new_in_320 = True
     new_in_api_v2 = True
 
-    def is_valid_relation(self, parent, sub, created=False):
-        if sub.credential_type.kind not in ('net', 'cloud'):
-            return {'error': _('Extra credentials must be network or cloud.')}
-        return super(JobExtraCredentialsList, self).is_valid_relation(parent, sub, created)
-
 
 class JobLabelList(SubListAPIView):
 

awx/main/fields.py

@@ -363,6 +363,8 @@ class JSONSchemaField(JSONBField):
         super(JSONSchemaField, self).validate(value, model_instance)
         errors = []
         for error in Draft4Validator(self.schema(model_instance)).iter_errors(value):
+            if error.validator == 'pattern' and 'error' in error.schema:
+                error.message = error.schema['error'] % error.instance
             errors.append(error)
 
         if errors:
@@ -471,7 +473,11 @@ class CredentialTypeInputField(JSONSchemaField):
                                 'items': {'type': 'string'},
                                 'uniqueItems': True
                             },
-                            'id': {'type': 'string'},
+                            'id': {
+                                'type': 'string',
+                                'pattern': '^[a-zA-Z_]+[a-zA-Z0-9_]*$',
+                                'error': '%s is an invalid variable name',
+                            },
                             'label': {'type': 'string'},
                             'help_text': {'type': 'string'},
                             'multiline': {'type': 'boolean'},
@@ -490,14 +496,24 @@ class CredentialTypeInputField(JSONSchemaField):
             value, model_instance
         )
 
+        ids = {}
         for field in value.get('fields', []):
-            if field.get('id') == 'tower':
+            id_ = field.get('id')
+            if id_ == 'tower':
                 raise django_exceptions.ValidationError(
                     _('"tower" is a reserved field name'),
                     code='invalid',
                     params={'value': value},
                 )
 
+            if id_ in ids:
+                raise django_exceptions.ValidationError(
+                    _('field IDs must be unique (%s)' % id_),
+                    code='invalid',
+                    params={'value': value},
+                )
+            ids[id_] = True
+
 
 
 class CredentialTypeInjectorField(JSONSchemaField):

awx/main/models/jobs.py

@@ -181,7 +181,7 @@ class JobOptions(BaseModel):
 
     @property
     def all_credentials(self):
-        credentials = self.extra_credentials.all()
+        credentials = list(self.extra_credentials.all())
         if self.vault_credential:
             credentials.insert(0, self.vault_credential)
         if self.credential:

awx/main/tests/functional/api/test_job.py

@@ -5,52 +5,7 @@ from awx.api.versioning import reverse
 
 # TODO: test this with RBAC and lower-priveleged users
 @pytest.mark.django_db
-def test_extra_credential_creation(get, post, organization_factory, job_template_factory, credentialtype_aws):
+def test_extra_credentials(get, organization_factory, job_template_factory, credential):
-    objs = organization_factory("org", superusers=['admin'])
-    jt = job_template_factory("jt", organization=objs.organization,
-                              inventory='test_inv', project='test_proj').job_template
-    job = jt.create_unified_job()
-
-    url = reverse('api:job_extra_credentials_list', kwargs={'version': 'v2', 'pk': job.pk})
-    response = post(url, {
-        'name': 'My Cred',
-        'credential_type': credentialtype_aws.pk,
-        'inputs': {
-            'username': 'bob',
-            'password': 'secret',
-        }
-    }, objs.superusers.admin)
-    assert response.status_code == 201
-
-    response = get(url, user=objs.superusers.admin)
-    assert response.data.get('count') == 1
-
-
-# TODO: test this with RBAC and lower-priveleged users
-@pytest.mark.django_db
-def test_attach_extra_credential(get, post, organization_factory, job_template_factory, credential):
-    objs = organization_factory("org", superusers=['admin'])
-    jt = job_template_factory("jt", organization=objs.organization,
-                              inventory='test_inv', project='test_proj').job_template
-    job = jt.create_unified_job()
-
-    url = reverse('api:job_extra_credentials_list', kwargs={'version': 'v2', 'pk': job.pk})
-    response = get(url, user=objs.superusers.admin)
-    assert response.data.get('count') == 0
-
-    response = post(url, {
-        'associate': True,
-        'id': credential.id,
-    }, objs.superusers.admin)
-    assert response.status_code == 204
-
-    response = get(url, user=objs.superusers.admin)
-    assert response.data.get('count') == 1
-
-
-# TODO: test this with RBAC and lower-priveleged users
-@pytest.mark.django_db
-def test_detach_extra_credential(get, post, organization_factory, job_template_factory, credential):
     objs = organization_factory("org", superusers=['admin'])
     jt = job_template_factory("jt", organization=objs.organization,
                               inventory='test_inv', project='test_proj').job_template
@@ -61,28 +16,3 @@ def test_detach_extra_credential(get, post, organization_factory, job_template_f
     url = reverse('api:job_extra_credentials_list', kwargs={'version': 'v2', 'pk': job.pk})
     response = get(url, user=objs.superusers.admin)
     assert response.data.get('count') == 1
-
-    response = post(url, {
-        'disassociate': True,
-        'id': credential.id,
-    }, objs.superusers.admin)
-    assert response.status_code == 204
-
-    response = get(url, user=objs.superusers.admin)
-    assert response.data.get('count') == 0
-
-
-@pytest.mark.django_db
-def test_attach_extra_credential_wrong_kind_xfail(get, post, organization_factory, job_template_factory, machine_credential):
-    """Extra credentials only allow net + cloud credentials"""
-    objs = organization_factory("org", superusers=['admin'])
-    jt = job_template_factory("jt", organization=objs.organization,
-                              inventory='test_inv', project='test_proj').job_template
-    job = jt.create_unified_job()
-
-    url = reverse('api:job_extra_credentials_list', kwargs={'version': 'v2', 'pk': job.pk})
-    response = post(url, {
-        'associate': True,
-        'id': machine_credential.id,
-    }, objs.superusers.admin)
-    assert response.status_code == 400

awx/main/tests/functional/api/test_job_template.py

@@ -58,6 +58,42 @@ def test_extra_credential_creation(get, post, organization_factory, job_template
     assert response.data.get('count') == 1
 
 
+@pytest.mark.django_db
+def test_extra_credential_unique_type_xfail(get, post, organization_factory, job_template_factory, credentialtype_aws):
+    objs = organization_factory("org", superusers=['admin'])
+    jt = job_template_factory("jt", organization=objs.organization,
+                              inventory='test_inv', project='test_proj').job_template
+
+    url = reverse('api:job_template_extra_credentials_list', kwargs={'version': 'v2', 'pk': jt.pk})
+    response = post(url, {
+        'name': 'My Cred',
+        'credential_type': credentialtype_aws.pk,
+        'inputs': {
+            'username': 'bob',
+            'password': 'secret',
+        }
+    }, objs.superusers.admin)
+    assert response.status_code == 201
+
+    response = get(url, user=objs.superusers.admin)
+    assert response.data.get('count') == 1
+
+    # this request should fail because you can't assign the same type (aws)
+    # twice
+    response = post(url, {
+        'name': 'My Cred',
+        'credential_type': credentialtype_aws.pk,
+        'inputs': {
+            'username': 'joe',
+            'password': 'another-secret',
+        }
+    }, objs.superusers.admin)
+    assert response.status_code == 400
+
+    response = get(url, user=objs.superusers.admin)
+    assert response.data.get('count') == 1
+
+
 # TODO: test this with RBAC and lower-priveleged users
 @pytest.mark.django_db
 def test_attach_extra_credential(get, post, organization_factory, job_template_factory, credential):
@@ -130,7 +166,7 @@ def test_v1_extra_credentials_detail(get, organization_factory, job_template_fac
 
 
 @pytest.mark.django_db
-def test_v1_set_extra_credentials(get, patch, organization_factory, job_template_factory, credential, net_credential):
+def test_v1_set_extra_credentials_assignment(get, patch, organization_factory, job_template_factory, credential, net_credential):
     objs = organization_factory("org", superusers=['admin'])
     jt = job_template_factory("jt", organization=objs.organization,
                               inventory='test_inv', project='test_proj').job_template
@@ -163,6 +199,28 @@ def test_v1_set_extra_credentials(get, patch, organization_factory, job_template
     assert response.data.get('network_credential') is None
 
 
+@pytest.mark.django_db
+def test_filter_by_v1(get, organization_factory, job_template_factory, credential, net_credential):
+    objs = organization_factory("org", superusers=['admin'])
+    jt = job_template_factory("jt", organization=objs.organization,
+                              inventory='test_inv', project='test_proj').job_template
+    jt.extra_credentials.add(credential)
+    jt.extra_credentials.add(net_credential)
+    jt.save()
+
+    for query in (
+        ('cloud_credential', str(credential.pk)),
+        ('network_credential', str(net_credential.pk))
+    ):
+        url = reverse('api:job_template_list', kwargs={'version': 'v1'})
+        response = get(
+            url,
+            user=objs.superusers.admin,
+            QUERY_STRING='='.join(query)
+        )
+        assert response.data.get('count') == 1
+
+
 @pytest.mark.django_db
 @pytest.mark.parametrize(
     "grant_project, grant_credential, grant_inventory, expect", [

awx/main/tests/functional/test_credential.py

@@ -58,6 +58,8 @@ def test_cloud_kind_uniqueness():
     ({'fields': [{'id': 'username', 'label': 'Username', 'type': 'string'}]}, True),
     ({'fields': [{'id': 'username', 'label': 'Username', 'help_text': 1}]}, False),
     ({'fields': [{'id': 'username', 'label': 'Username', 'help_text': 'Help Text'}]}, True),  # noqa
+    ({'fields': [{'id': 'username', 'label': 'Username'}, {'id': 'username', 'label': 'Username 2'}]}, False),  # noqa
+    ({'fields': [{'id': '$invalid$', 'label': 'Invalid'}]}, False),  # noqa
     ({'fields': [{'id': 'password', 'label': 'Password', 'type': 'number'}]}, True),
     ({'fields': [{'id': 'ssh_key', 'label': 'SSH Key', 'type': 'ssh_private_key'}]}, True),  # noqa
     ({'fields': [{'id': 'other', 'label': 'Other', 'type': 'boolean'}]}, False),