Merge pull request #1233 from AlanCoding/no_turning_back

Raise 400 error on removal of credential on launch

awx/api/serializers.py

@@ -3734,15 +3734,30 @@ class JobLaunchSerializer(BaseSerializer):
         distinct_cred_kinds = []
         for cred in accepted.get('credentials', []):
             if cred.unique_hash() in distinct_cred_kinds:
-                errors['credentials'] = _('Cannot assign multiple %s credentials.' % cred.credential_type.name)
+                errors.setdefault('credentials', []).append(_(
+                    'Cannot assign multiple {} credentials.'
+                ).format(cred.unique_hash(display=True)))
             distinct_cred_kinds.append(cred.unique_hash())
 
+        # Prohibit removing credentials from the JT list (unsupported for now)
+        template_credentials = template.credentials.all()
+        if 'credentials' in attrs:
+            removed_creds = set(template_credentials) - set(attrs['credentials'])
+            provided_mapping = Credential.unique_dict(attrs['credentials'])
+            for cred in removed_creds:
+                if cred.unique_hash() in provided_mapping.keys():
+                    continue  # User replaced credential with new of same type
+                errors.setdefault('credentials', []).append(_(
+                    'Removing {} credential at launch time without replacement is not supported. '
+                    'Provided list lacked credential(s): {}.'
+                ).format(cred.unique_hash(display=True), ', '.join([str(c) for c in removed_creds])))
+
         # verify that credentials (either provided or existing) don't
         # require launch-time passwords that have not been provided
         if 'credentials' in accepted:
             launch_credentials = accepted['credentials']
         else:
-            launch_credentials = template.credentials.all()
+            launch_credentials = template_credentials
         passwords = attrs.get('credential_passwords', {})  # get from original attrs
         passwords_lacking = []
         for cred in launch_credentials:

awx/api/views.py

@@ -2884,6 +2884,7 @@ class JobTemplateLaunch(RetrieveAPIView):
         ):
             # make a list of the current credentials
             existing_credentials = obj.credentials.all()
+            template_credentials = list(existing_credentials)  # save copy of existing
             new_credentials = []
             for key, conditional in (
                 ('credential', lambda cred: cred.credential_type.kind != 'ssh'),
@@ -2910,6 +2911,11 @@ class JobTemplateLaunch(RetrieveAPIView):
             # combine the list of "new" and the filtered list of "old"
             new_credentials.extend([cred.pk for cred in existing_credentials])
             if new_credentials:
+                # If provided list doesn't contain the pre-existing credentials
+                # defined on the template, add them back here
+                for cred_obj in template_credentials:
+                    if cred_obj.pk not in new_credentials:
+                        new_credentials.append(cred_obj.pk)
                 modern_data['credentials'] = new_credentials
 
         # credential passwords were historically provided as top-level attributes

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

@@ -283,7 +283,7 @@ def test_job_launch_JT_with_validation(machine_credential, credential, deploy_jo
     deploy_jobtemplate.ask_variables_on_launch = True
     deploy_jobtemplate.save()
 
-    kv = dict(extra_vars={"job_launch_var": 4}, credentials=[machine_credential.pk])
+    kv = dict(extra_vars={"job_launch_var": 4}, credentials=[machine_credential.pk, credential.pk])
     serializer = JobLaunchSerializer(data=kv, context={'template': deploy_jobtemplate})
     validated = serializer.is_valid()
     assert validated, serializer.errors
@@ -338,16 +338,17 @@ def test_job_launch_JT_enforces_unique_credentials_kinds(machine_credential, cre
 
 
 @pytest.mark.django_db
-def test_job_launch_with_empty_creds(machine_credential, vault_credential, deploy_jobtemplate):
+def test_job_launch_with_empty_creds(machine_credential, vault_credential, deploy_jobtemplate, credential):
     deploy_jobtemplate.ask_credential_on_launch = True
     deploy_jobtemplate.credentials.add(machine_credential)
     deploy_jobtemplate.credentials.add(vault_credential)
-    kv = dict(credentials=[])
+    # `credentials` list is strictly those already present on deploy_jobtemplate
+    kv = dict(credentials=[credential.pk, machine_credential.pk, vault_credential.pk])
     serializer = JobLaunchSerializer(data=kv, context={'template': deploy_jobtemplate})
     validated = serializer.is_valid()
-    assert validated
+    assert validated, serializer.errors
 
-    prompted_fields, ignored_fields, errors = deploy_jobtemplate._accept_or_ignore_job_kwargs(**kv)
+    prompted_fields, ignored_fields, errors = deploy_jobtemplate._accept_or_ignore_job_kwargs(**serializer.validated_data)
     job_obj = deploy_jobtemplate.create_unified_job(**prompted_fields)
     assert job_obj.credential is deploy_jobtemplate.credential
     assert job_obj.vault_credential is deploy_jobtemplate.vault_credential

docs/prompting.md

@@ -42,18 +42,19 @@ spec to exist and `survey_enabled` to be true). On the other hand,
 if `ask_variables_on_launch` is true, users can provide any variables in
 extra_vars.
 
-Prompting enablement for several types of credentials is controlled by a single
+_(supported, but deprecated)_ Prompting enablement for several types of
+credentials is controlled by a single
 field. On launch, multiple types of credentials can be provided in their respective fields
 inside of `credential`, `vault_credential`, and `extra_credentials`. Providing
 credentials that require password input from the user on launch is
 allowed, and the password must be provided along-side the credential, of course.
 
-If the job is being spawned using a saved launch configuration, however,
-all non-machine credential types are managed by a many-to-many relationship
+If the job is being spawned using a saved launch configuration,
+all credential types are managed by a many-to-many relationship
 called `credentials` relative to the launch configuration object.
-When the job is spawned, the credentials in that relationship will be
-sorted into the job's many-to-many credential fields according to their
-type (cloud vs. vault).
+The credentials in this relationship will either add to the job template's
+credential list, or replace a credential in the job template's list if it
+is the same type.
 
 ### Manual use of Prompts
 
@@ -67,14 +68,22 @@ actions in the API.
  - POST to `/api/v2/system_job_templates/N/launch/`
    - can accept certain fields, with no user configuration
 
+When launching manually, certain restrictions apply to the use of credentials
+ - if providing any of `credential`, `vault_credential`, and `extra_credentials`
+   this becomes the "legacy" method, and imposes additional restrictions on
+   relaunch, and is mutually exclusive with the use of `credentials` field
+ - if providing `credentials`, existing credentials on the job template may
+   only be removed if replaced by another credential of the same type
+   this is so that relaunch will use the up-to-date credential on the template
+   if it has been edited since the prior launch
+
 #### Data Rules for Prompts
 
 For the POST action to launch, data for "prompts" are provided as top-level
 keys in the request data. There is a special-case to allow a list to be
 provided for `credentials`, which is otherwise not possible in AWX API design.
-The list of credentials will either add extra credentials, or replace
-existing credentials in the job template if a provided credential is of
-the same type.
+The list of credentials provided in the POST data will become the list
+for the spawned job.
 
 Values of `null` are not allowed, if the field is not being over-ridden,
 the key should not be given in the payload. A 400 should be returned if
@@ -88,7 +97,7 @@ POST to `/api/v2/job_templates/N/launch/` with data:
 {
   "job_type": "check",
   "limit": "",
-  "credentials": [1, 2, 4],
+  "credentials": [1, 2, 4, 5],
   "extra_vars": {}
 }
 ```
@@ -117,10 +126,13 @@ be combined with the job template extra_vars dictionary, with the
 request data taking precedence.
 
 Provided credentials will replace any job template credentials of the same
-exclusive type, but combine with any others. In the example, the job template
+exclusive type. In the example, the job template
 credential 3 was replaced with the provided credential 1, because a job
 may only use 1 gce credential because these two credentials define the
 same environment variables and configuration file.
+If the job had not provided the credential 1, a 400 error would have been
+returned because the job must contain the same types of credentials as its
+job template.
 
 ### Saved Launch-time Configurations
 
@@ -135,6 +147,11 @@ In the case of workflow nodes and schedules, the prompted fields are saved
 directly on the model. Those models include Workflow Job Template Nodes,
 Workflow Job Nodes (a copy of the first), and Schedules.
 
+The many-to-many `credentials` field differs from other fields because
+they are managed through a sub-endpoint relative to the node or schedule.
+This relationship contains the _additional_ credentials to apply when
+it spawns a job.
+
 Jobs, themselves, have a configuration object stored in a related model,
 and only used to prepare the correct launch-time configuration for subsequent
 re-launch and re-scheduling of the job. To see these prompts for a particular