From 18c95bf706230ba9e3ace70646daa1eeaa085853 Mon Sep 17 00:00:00 2001 From: Ryan Petrello Date: Mon, 9 Apr 2018 11:26:20 -0400 Subject: [PATCH] add exception handling to deprecated v1 credential support see: https://github.com/ansible/tower/issues/1268 --- awx/api/fields.py | 19 +++++++++++ awx/api/serializers.py | 33 +++++-------------- .../test_deprecated_credential_assignment.py | 32 ++++++++++++++++++ 3 files changed, 60 insertions(+), 24 deletions(-) diff --git a/awx/api/fields.py b/awx/api/fields.py index 6a1ddb6018..5276ef4dec 100644 --- a/awx/api/fields.py +++ b/awx/api/fields.py @@ -3,12 +3,14 @@ # Django from django.utils.translation import ugettext_lazy as _ +from django.core.exceptions import ObjectDoesNotExist # Django REST Framework from rest_framework import serializers # AWX from awx.conf import fields +from awx.main.models import Credential __all__ = ['BooleanNullField', 'CharNullField', 'ChoiceNullField', 'VerbatimField'] @@ -87,3 +89,20 @@ class OAuth2ProviderField(fields.DictField): if invalid_flags: self.fail('invalid_key_names', invalid_key_names=', '.join(list(invalid_flags))) return data + + +class DeprecatedCredentialField(serializers.IntegerField): + + def __init__(self, **kwargs): + kwargs['allow_null'] = True + kwargs['default'] = None + kwargs['min_value'] = 1 + kwargs['help_text'] = 'This resource has been deprecated and will be removed in a future release' + super(DeprecatedCredentialField, self).__init__(**kwargs) + + def to_internal_value(self, pk): + try: + Credential.objects.get(pk=pk) + except ObjectDoesNotExist: + raise serializers.ValidationError(_('Credential {} does not exist').format(pk)) + return pk diff --git a/awx/api/serializers.py b/awx/api/serializers.py index 0ac84bea4a..bd17ff1126 100644 --- a/awx/api/serializers.py +++ b/awx/api/serializers.py @@ -56,12 +56,11 @@ from awx.main.validators import vars_validate_or_raise from awx.conf.license import feature_enabled from awx.api.versioning import reverse, get_request_version -from awx.api.fields import BooleanNullField, CharNullField, ChoiceNullField, VerbatimField +from awx.api.fields import (BooleanNullField, CharNullField, ChoiceNullField, + VerbatimField, DeprecatedCredentialField) logger = logging.getLogger('awx.api.serializers') -DEPRECATED = 'This resource has been deprecated and will be removed in a future release' - # Fields that should be summarized regardless of object type. DEFAULT_SUMMARY_FIELDS = ('id', 'name', 'description')# , 'created_by', 'modified_by')#, 'type') @@ -1957,9 +1956,7 @@ class CustomInventoryScriptSerializer(BaseSerializer): class InventorySourceOptionsSerializer(BaseSerializer): - credential = models.PositiveIntegerField( - blank=True, null=True, default=None, - help_text='This resource has been deprecated and will be removed in a future release') + credential = DeprecatedCredentialField() class Meta: fields = ('*', 'source', 'source_path', 'source_script', 'source_vars', 'credential', @@ -2818,15 +2815,11 @@ class V1JobOptionsSerializer(BaseSerializer): model = Credential fields = ('*', 'cloud_credential', 'network_credential') - V1_FIELDS = { - 'cloud_credential': models.PositiveIntegerField(blank=True, null=True, default=None, help_text=DEPRECATED), - 'network_credential': models.PositiveIntegerField(blank=True, null=True, default=None, help_text=DEPRECATED), - } + V1_FIELDS = ('cloud_credential', 'network_credential',) def build_field(self, field_name, info, model_class, nested_depth): if field_name in self.V1_FIELDS: - return self.build_standard_field(field_name, - self.V1_FIELDS[field_name]) + return (DeprecatedCredentialField, {}) return super(V1JobOptionsSerializer, self).build_field(field_name, info, model_class, nested_depth) @@ -2837,15 +2830,11 @@ class LegacyCredentialFields(BaseSerializer): model = Credential fields = ('*', 'credential', 'vault_credential') - LEGACY_FIELDS = { - 'credential': models.PositiveIntegerField(blank=True, null=True, default=None, help_text=DEPRECATED), - 'vault_credential': models.PositiveIntegerField(blank=True, null=True, default=None, help_text=DEPRECATED), - } + LEGACY_FIELDS = ('credential', 'vault_credential',) def build_field(self, field_name, info, model_class, nested_depth): if field_name in self.LEGACY_FIELDS: - return self.build_standard_field(field_name, - self.LEGACY_FIELDS[field_name]) + return (DeprecatedCredentialField, {}) return super(LegacyCredentialFields, self).build_field(field_name, info, model_class, nested_depth) @@ -3718,9 +3707,7 @@ class LaunchConfigurationBaseSerializer(BaseSerializer): class WorkflowJobTemplateNodeSerializer(LaunchConfigurationBaseSerializer): - credential = models.PositiveIntegerField( - blank=True, null=True, default=None, - help_text='This resource has been deprecated and will be removed in a future release') + credential = DeprecatedCredentialField() success_nodes = serializers.PrimaryKeyRelatedField(many=True, read_only=True) failure_nodes = serializers.PrimaryKeyRelatedField(many=True, read_only=True) always_nodes = serializers.PrimaryKeyRelatedField(many=True, read_only=True) @@ -3811,9 +3798,7 @@ class WorkflowJobTemplateNodeSerializer(LaunchConfigurationBaseSerializer): class WorkflowJobNodeSerializer(LaunchConfigurationBaseSerializer): - credential = models.PositiveIntegerField( - blank=True, null=True, default=None, - help_text='This resource has been deprecated and will be removed in a future release') + credential = DeprecatedCredentialField() success_nodes = serializers.PrimaryKeyRelatedField(many=True, read_only=True) failure_nodes = serializers.PrimaryKeyRelatedField(many=True, read_only=True) always_nodes = serializers.PrimaryKeyRelatedField(many=True, read_only=True) diff --git a/awx/main/tests/functional/api/test_deprecated_credential_assignment.py b/awx/main/tests/functional/api/test_deprecated_credential_assignment.py index e9090630e2..f6466affd7 100644 --- a/awx/main/tests/functional/api/test_deprecated_credential_assignment.py +++ b/awx/main/tests/functional/api/test_deprecated_credential_assignment.py @@ -1,3 +1,4 @@ +import json import mock import pytest @@ -5,6 +6,14 @@ from awx.main.models import Credential, Job from awx.api.versioning import reverse +@pytest.fixture +def ec2_source(inventory, project): + with mock.patch('awx.main.models.unified_jobs.UnifiedJobTemplate.update'): + return inventory.inventory_sources.create( + name='some_source', update_on_project_update=True, source='ec2', + source_project=project, scm_last_revision=project.scm_revision) + + @pytest.fixture def job_template(job_template, project, inventory): job_template.playbook = 'helloworld.yml' @@ -34,6 +43,14 @@ def test_ssh_credential_access(get, job_template, admin, machine_credential): assert resp.data['summary_fields']['credential']['kind'] == 'ssh' +@pytest.mark.django_db +@pytest.mark.parametrize('key', ('credential', 'vault_credential', 'cloud_credential', 'network_credential')) +def test_invalid_credential_update(get, patch, job_template, admin, key): + url = reverse('api:job_template_detail', kwargs={'pk': job_template.pk, 'version': 'v1'}) + resp = patch(url, {key: 999999}, admin, expect=400) + assert 'Credential 999999 does not exist' in json.loads(resp.content)[key] + + @pytest.mark.django_db def test_ssh_credential_update(get, patch, job_template, admin, machine_credential): url = reverse('api:job_template_detail', kwargs={'pk': job_template.pk}) @@ -362,3 +379,18 @@ def test_rbac_default_credential_usage(get, post, job_template, alice, machine_c new_cred.use_role.members.add(alice) url = reverse('api:job_template_launch', kwargs={'pk': job_template.pk}) post(url, {'credential': new_cred.pk}, alice, expect=201) + + +@pytest.mark.django_db +def test_inventory_source_deprecated_credential(get, patch, admin, ec2_source, credential): + url = reverse('api:inventory_source_detail', kwargs={'pk': ec2_source.pk}) + patch(url, {'credential': credential.pk}, admin, expect=200) + resp = get(url, admin, expect=200) + assert json.loads(resp.content)['credential'] == credential.pk + + +@pytest.mark.django_db +def test_inventory_source_invalid_deprecated_credential(patch, admin, ec2_source, credential): + url = reverse('api:inventory_source_detail', kwargs={'pk': ec2_source.pk}) + resp = patch(url, {'credential': 999999}, admin, expect=400) + assert 'Credential 999999 does not exist' in resp.content