From a1fa9243bc3a05df9049dfd3874201b8257c4f1e Mon Sep 17 00:00:00 2001 From: Ryan Petrello Date: Mon, 1 May 2017 15:42:56 -0400 Subject: [PATCH] split `machine` CredentialType into two distinct (ssh and vault) kinds --- awx/api/filters.py | 40 +++++++++++++------ awx/api/serializers.py | 12 +----- .../0040_v320_add_credentialtype_model.py | 2 +- awx/main/models/ad_hoc_commands.py | 2 +- awx/main/models/credential.py | 34 ++++++++-------- awx/main/models/jobs.py | 2 +- .../tests/functional/api/test_credential.py | 32 +++++++++++++++ 7 files changed, 81 insertions(+), 43 deletions(-) diff --git a/awx/api/filters.py b/awx/api/filters.py index 3fc44ce921..aa617910c4 100644 --- a/awx/api/filters.py +++ b/awx/api/filters.py @@ -6,7 +6,7 @@ import re import json # Django -from django.core.exceptions import FieldError, ValidationError, ObjectDoesNotExist +from django.core.exceptions import FieldError, ValidationError from django.db import models from django.db.models import Q from django.db.models.fields import FieldDoesNotExist @@ -174,18 +174,6 @@ class FieldLookupBackend(BaseFilterBackend): except UnicodeEncodeError: raise ValueError("%r is not an allowed field name. Must be ascii encodable." % lookup) - # Make legacy v1 Credential fields work for backwards compatability - # TODO: remove after API v1 deprecation period - if model._meta.object_name == 'Credential' and lookup == 'kind': - try: - type_ = CredentialType.from_v1_kind(value) - if type_ is None: - raise ParseError(_('cannot filter on kind %s') % value) - value = type_.pk - lookup = 'credential_type' - except ObjectDoesNotExist as e: - raise ParseError(_('cannot filter on kind %s') % value) - field, new_lookup = self.get_field_from_lookup(model, lookup) # Type names are stored without underscores internally, but are presented and @@ -277,6 +265,32 @@ class FieldLookupBackend(BaseFilterBackend): key = key[5:] q_not = True + # Make legacy v1 Credential fields work for backwards compatability + # TODO: remove after API v1 deprecation period + # + # convert v1 `Credential.kind` queries to `Credential.credential_type__pk` + if queryset.model._meta.object_name == 'Credential' and key == 'kind': + key = key.replace('kind', 'credential_type') + + if 'ssh' in values: + # In 3.2, SSH and Vault became separate credential types, but in the v1 API, + # they're both still "kind=ssh" + # under the hood, convert `/api/v1/credentials/?kind=ssh` to + # `/api/v1/credentials/?or__credential_type=&or__credential_type=` + values = set(values) + values.add('vault') + values = list(values) + q_or = True + + for i, kind in enumerate(values): + if kind == 'vault': + type_ = CredentialType.objects.get(kind=kind) + else: + type_ = CredentialType.from_v1_kind(kind) + if type_ is None: + raise ParseError(_('cannot filter on kind %s') % kind) + values[i] = type_.pk + # Convert value(s) to python and add to the appropriate list. for value in values: if q_int: diff --git a/awx/api/serializers.py b/awx/api/serializers.py index 1086dd8784..8d974ad41f 100644 --- a/awx/api/serializers.py +++ b/awx/api/serializers.py @@ -318,15 +318,6 @@ class BaseSerializer(serializers.ModelSerializer): fval = getattr(fkval, field, None) - # TODO: remove when API v1 is removed - if all([ - self.version == 1, - 'credential' in fk, - field == 'kind', - fval == 'machine' - ]): - fval = 'ssh' - if fval is None and field == 'type': if isinstance(fkval, PolymorphicModel): fkval = fkval.get_real_instance() @@ -1883,9 +1874,8 @@ class CredentialSerializer(BaseSerializer): # TODO: remove when API v1 is removed if self.version == 1: - if value.get('kind') == 'machine': + if value.get('kind') == 'vault': value['kind'] = 'ssh' - for field in V1Credential.PASSWORD_FIELDS: if field in value and force_text(value[field]).startswith('$encrypted$'): value[field] = '$encrypted$' diff --git a/awx/main/migrations/0040_v320_add_credentialtype_model.py b/awx/main/migrations/0040_v320_add_credentialtype_model.py index 96c34246f2..fa335d8e3c 100644 --- a/awx/main/migrations/0040_v320_add_credentialtype_model.py +++ b/awx/main/migrations/0040_v320_add_credentialtype_model.py @@ -23,7 +23,7 @@ class Migration(migrations.Migration): ('modified', models.DateTimeField(default=None, editable=False)), ('description', models.TextField(default=b'', blank=True)), ('name', models.CharField(max_length=512)), - ('kind', models.CharField(max_length=32, choices=[(b'machine', 'Machine'), (b'net', 'Network'), (b'scm', 'Source Control'), (b'cloud', 'Cloud')])), + ('kind', models.CharField(max_length=32, choices=[(b'ssh', 'SSH'), (b'vault', 'Vault'), (b'net', 'Network'), (b'scm', 'Source Control'), (b'cloud', 'Cloud')])), ('managed_by_tower', models.BooleanField(default=False, editable=False)), ('inputs', awx.main.fields.CredentialTypeInputField(default={}, blank=True)), ('injectors', awx.main.fields.CredentialTypeInjectorField(default={}, blank=True)), diff --git a/awx/main/models/ad_hoc_commands.py b/awx/main/models/ad_hoc_commands.py index 4dba6fb48a..57fb071c53 100644 --- a/awx/main/models/ad_hoc_commands.py +++ b/awx/main/models/ad_hoc_commands.py @@ -99,7 +99,7 @@ class AdHocCommand(UnifiedJob, JobNotificationMixin): def clean_credential(self): cred = self.credential - if cred and cred.kind != 'machine': + if cred and cred.kind != 'ssh': raise ValidationError( _('You must provide a machine / SSH credential.'), ) diff --git a/awx/main/models/credential.py b/awx/main/models/credential.py index 7303465bce..929b80883c 100644 --- a/awx/main/models/credential.py +++ b/awx/main/models/credential.py @@ -317,7 +317,7 @@ class Credential(PasswordFieldsModel, CommonModelNameNotUnique, ResourceMixin): # @property def needs_ssh_password(self): - return self.kind == 'machine' and self.password == 'ASK' + return self.kind == 'ssh' and self.password == 'ASK' @property def has_encrypted_ssh_key_data(self): @@ -336,17 +336,17 @@ class Credential(PasswordFieldsModel, CommonModelNameNotUnique, ResourceMixin): @property def needs_ssh_key_unlock(self): - if self.kind == 'machine' and self.ssh_key_unlock in ('ASK', ''): + if self.kind == 'ssh' and self.ssh_key_unlock in ('ASK', ''): return self.has_encrypted_ssh_key_data return False @property def needs_become_password(self): - return self.kind == 'machine' and self.become_password == 'ASK' + return self.kind == 'ssh' and self.become_password == 'ASK' @property def needs_vault_password(self): - return self.kind == 'machine' and self.vault_password == 'ASK' + return self.kind == 'vault' and self.vault_password == 'ASK' @property def passwords_needed(self): @@ -396,7 +396,7 @@ class Credential(PasswordFieldsModel, CommonModelNameNotUnique, ResourceMixin): raise ValidationError(_('Credential cannot be assigned to both a user and team.')) def _password_field_allows_ask(self, field): - return bool(self.kind == 'machine' and field != 'ssh_key_data') + return field in self.credential_type.askable_fields def save(self, *args, **kwargs): inputs_before = {} @@ -472,7 +472,8 @@ class CredentialType(CommonModelNameNotUnique): unique_together = (('name', 'kind'),) KIND_CHOICES = ( - ('machine', _('Machine')), + ('ssh', _('SSH')), + ('vault', _('Vault')), ('net', _('Network')), ('scm', _('Source Control')), ('cloud', _('Cloud')) @@ -513,6 +514,13 @@ class CredentialType(CommonModelNameNotUnique): if field.get('secret', False) is True ] + @property + def askable_fields(self): + return [ + field['id'] for field in self.inputs.get('fields', []) + if field.get('ask_at_runtime', False) is True + ] + @classmethod def default(cls, f): func = functools.partial(f, cls) @@ -534,15 +542,9 @@ class CredentialType(CommonModelNameNotUnique): requirements = {} if kind == 'ssh': if 'vault_password' in data: - requirements.update(dict( - kind='machine', - name='Vault' - )) + requirements['kind'] = 'vault' else: - requirements.update(dict( - kind='machine', - name='SSH' - )) + requirements['kind'] = 'ssh' elif kind in ('net', 'scm'): requirements['kind'] = kind elif kind in kind_choices: @@ -643,7 +645,7 @@ class CredentialType(CommonModelNameNotUnique): @CredentialType.default def ssh(cls): return cls( - kind='machine', + kind='ssh', name='SSH', managed_by_tower=True, inputs={ @@ -724,7 +726,7 @@ def scm(cls): @CredentialType.default def vault(cls): return cls( - kind='machine', + kind='vault', name='Vault', managed_by_tower=True, inputs={ diff --git a/awx/main/models/jobs.py b/awx/main/models/jobs.py index c1ebb69309..c15ab663a3 100644 --- a/awx/main/models/jobs.py +++ b/awx/main/models/jobs.py @@ -168,7 +168,7 @@ class JobOptions(BaseModel): def clean_credential(self): cred = self.credential - if cred and cred.kind != 'machine': + if cred and cred.kind != 'ssh': raise ValidationError( _('You must provide a machine / SSH credential.'), ) diff --git a/awx/main/tests/functional/api/test_credential.py b/awx/main/tests/functional/api/test_credential.py index 1c809b8e3f..6f12a98a7d 100644 --- a/awx/main/tests/functional/api/test_credential.py +++ b/awx/main/tests/functional/api/test_credential.py @@ -33,6 +33,38 @@ def test_filter_by_v1_kind(get, admin, organization, kind, total): assert response.data['count'] == total +@pytest.mark.django_db +def test_filter_by_v1_kind_with_vault(get, admin, organization): + CredentialType.setup_tower_managed_defaults() + cred = Credential( + credential_type=CredentialType.objects.get(kind='ssh'), + name='Best credential ever', + organization=organization, + inputs={ + 'username': u'jim', + 'password': u'secret' + } + ) + cred.save() + cred = Credential( + credential_type=CredentialType.objects.get(kind='vault'), + name='Best credential ever', + organization=organization, + inputs={ + 'vault_password': u'vault!' + } + ) + cred.save() + + response = get( + reverse('api:credential_list', kwargs={'version': 'v1'}), + admin, + QUERY_STRING='kind=ssh' + ) + assert response.status_code == 200 + assert response.data['count'] == 2 + + @pytest.mark.django_db def test_custom_credentials_not_in_v1_api_list(get, admin, organization): """