From f22adca6f70bc094d2849b945360e62b04583d15 Mon Sep 17 00:00:00 2001 From: Ryan Petrello Date: Mon, 26 Aug 2019 18:08:01 -0400 Subject: [PATCH] improve parsing of JSON inputs see: https://github.com/ansible/awx/issues/4573 see: https://github.com/ansible/awx/issues/2371 --- awx/api/metadata.py | 30 ++++++++++++++++++++++++++- awx/main/models/__init__.py | 3 ++- awx/main/models/activity_stream.py | 3 ++- awx/main/models/base.py | 11 ++++++++++ awx/main/models/inventory.py | 14 ++++++------- awx/main/models/jobs.py | 6 +++--- awx/main/models/workflow.py | 7 ++++--- awxkit/awxkit/cli/options.py | 33 +++++++++++++++++++++++++++--- 8 files changed, 88 insertions(+), 19 deletions(-) diff --git a/awx/api/metadata.py b/awx/api/metadata.py index cc44e6d0e9..94b5ff4ccd 100644 --- a/awx/api/metadata.py +++ b/awx/api/metadata.py @@ -5,6 +5,8 @@ from collections import OrderedDict # Django from django.core.exceptions import PermissionDenied +from django.db.models.fields import PositiveIntegerField, BooleanField +from django.db.models.fields.related import ForeignKey from django.http import Http404 from django.utils.encoding import force_text, smart_text from django.utils.translation import ugettext_lazy as _ @@ -14,9 +16,11 @@ from rest_framework import exceptions from rest_framework import metadata from rest_framework import serializers from rest_framework.relations import RelatedField, ManyRelatedField +from rest_framework.fields import JSONField as DRFJSONField from rest_framework.request import clone_request # AWX +from awx.main.fields import JSONField from awx.main.models import InventorySource, NotificationTemplate @@ -68,6 +72,8 @@ class Metadata(metadata.SimpleMetadata): else: for model_field in serializer.Meta.model._meta.fields: if field.field_name == model_field.name: + if getattr(model_field, '__accepts_json__', None): + field_info['type'] = 'json' field_info['filterable'] = True break else: @@ -115,14 +121,36 @@ class Metadata(metadata.SimpleMetadata): field_info[notification_type_name] = notification_type_class.init_parameters # Update type of fields returned... + model_field = None + if serializer and hasattr(serializer, 'Meta') and hasattr(serializer.Meta, 'model'): + try: + model_field = serializer.Meta.model._meta.get_field(field.field_name) + except Exception: + pass if field.field_name == 'type': field_info['type'] = 'choice' - elif field.field_name == 'url': + elif field.field_name in ('url', 'custom_virtualenv', 'token'): field_info['type'] = 'string' elif field.field_name in ('related', 'summary_fields'): field_info['type'] = 'object' + elif isinstance(field, PositiveIntegerField): + field_info['type'] = 'integer' elif field.field_name in ('created', 'modified'): field_info['type'] = 'datetime' + elif ( + RelatedField in field.__class__.__bases__ or + isinstance(model_field, ForeignKey) + ): + field_info['type'] = 'id' + elif ( + isinstance(field, JSONField) or + isinstance(model_field, JSONField) or + isinstance(field, DRFJSONField) or + isinstance(getattr(field, 'model_field', None), JSONField) + ): + field_info['type'] = 'json' + elif isinstance(model_field, BooleanField): + field_info['type'] = 'boolean' return field_info diff --git a/awx/main/models/__init__.py b/awx/main/models/__init__.py index 51e62f55a3..f822701a29 100644 --- a/awx/main/models/__init__.py +++ b/awx/main/models/__init__.py @@ -7,7 +7,8 @@ from django.db.models.signals import pre_delete # noqa # AWX from awx.main.models.base import ( # noqa - BaseModel, PrimordialModel, prevent_search, CLOUD_INVENTORY_SOURCES, VERBOSITY_CHOICES + BaseModel, PrimordialModel, prevent_search, accepts_json, + CLOUD_INVENTORY_SOURCES, VERBOSITY_CHOICES ) from awx.main.models.unified_jobs import ( # noqa UnifiedJob, UnifiedJobTemplate, StdoutMaxBytesExceeded diff --git a/awx/main/models/activity_stream.py b/awx/main/models/activity_stream.py index bcc2ab20ef..20042c82df 100644 --- a/awx/main/models/activity_stream.py +++ b/awx/main/models/activity_stream.py @@ -4,6 +4,7 @@ # Tower from awx.api.versioning import reverse from awx.main.fields import JSONField +from awx.main.models.base import accepts_json # Django from django.db import models @@ -34,7 +35,7 @@ class ActivityStream(models.Model): actor = models.ForeignKey('auth.User', null=True, on_delete=models.SET_NULL, related_name='activity_stream') operation = models.CharField(max_length=13, choices=OPERATION_CHOICES) timestamp = models.DateTimeField(auto_now_add=True) - changes = models.TextField(blank=True) + changes = accepts_json(models.TextField(blank=True)) deleted_actor = JSONField(null=True) action_node = models.CharField( blank=True, diff --git a/awx/main/models/base.py b/awx/main/models/base.py index 9925dc6049..70fa92cf0a 100644 --- a/awx/main/models/base.py +++ b/awx/main/models/base.py @@ -408,3 +408,14 @@ def prevent_search(relation): """ setattr(relation, '__prevent_search__', True) return relation + + +def accepts_json(relation): + """ + Used to mark a model field as allowing JSON e.g,. JobTemplate.extra_vars + This is *mostly* used as a way to provide type hints for certain fields + so that HTTP OPTIONS reports the type data we need for the CLI to allow + JSON/YAML input. + """ + setattr(relation, '__accepts_json__', True) + return relation diff --git a/awx/main/models/inventory.py b/awx/main/models/inventory.py index c3e447e10a..8e5f1733ee 100644 --- a/awx/main/models/inventory.py +++ b/awx/main/models/inventory.py @@ -45,7 +45,7 @@ from awx.main.models.base import ( CommonModelNameNotUnique, VarsDictProperty, CLOUD_INVENTORY_SOURCES, - prevent_search + prevent_search, accepts_json ) from awx.main.models.events import InventoryUpdateEvent from awx.main.models.unified_jobs import UnifiedJob, UnifiedJobTemplate @@ -93,11 +93,11 @@ class Inventory(CommonModelNameNotUnique, ResourceMixin, RelatedJobsMixin): on_delete=models.SET_NULL, null=True, ) - variables = models.TextField( + variables = accepts_json(models.TextField( blank=True, default='', help_text=_('Inventory variables in JSON or YAML format.'), - ) + )) has_active_failures = models.BooleanField( default=False, editable=False, @@ -608,11 +608,11 @@ class Host(CommonModelNameNotUnique, RelatedJobsMixin): default='', help_text=_('The value used by the remote inventory source to uniquely identify the host'), ) - variables = models.TextField( + variables = accepts_json(models.TextField( blank=True, default='', help_text=_('Host variables in JSON or YAML format.'), - ) + )) last_job = models.ForeignKey( 'Job', related_name='hosts_as_last_job+', @@ -796,11 +796,11 @@ class Group(CommonModelNameNotUnique, RelatedJobsMixin): related_name='children', blank=True, ) - variables = models.TextField( + variables = accepts_json(models.TextField( blank=True, default='', help_text=_('Group variables in JSON or YAML format.'), - ) + )) hosts = models.ManyToManyField( 'Host', related_name='groups', diff --git a/awx/main/models/jobs.py b/awx/main/models/jobs.py index f3fc42bd46..f600d9f3f5 100644 --- a/awx/main/models/jobs.py +++ b/awx/main/models/jobs.py @@ -27,7 +27,7 @@ from rest_framework.exceptions import ParseError from awx.api.versioning import reverse from awx.main.models.base import ( BaseModel, CreatedModifiedModel, - prevent_search, + prevent_search, accepts_json, JOB_TYPE_CHOICES, VERBOSITY_CHOICES, VarsDictProperty ) @@ -116,10 +116,10 @@ class JobOptions(BaseModel): blank=True, default=0, ) - extra_vars = prevent_search(models.TextField( + extra_vars = prevent_search(accepts_json(models.TextField( blank=True, default='', - )) + ))) job_tags = models.CharField( max_length=1024, blank=True, diff --git a/awx/main/models/workflow.py b/awx/main/models/workflow.py index d413f05666..9f958886ab 100644 --- a/awx/main/models/workflow.py +++ b/awx/main/models/workflow.py @@ -13,7 +13,8 @@ from django.core.exceptions import ObjectDoesNotExist # AWX from awx.api.versioning import reverse -from awx.main.models import prevent_search, UnifiedJobTemplate, UnifiedJob +from awx.main.models import (prevent_search, accepts_json, UnifiedJobTemplate, + UnifiedJob) from awx.main.models.notifications import ( NotificationTemplate, JobNotificationMixin @@ -291,10 +292,10 @@ class WorkflowJobOptions(BaseModel): class Meta: abstract = True - extra_vars = prevent_search(models.TextField( + extra_vars = accepts_json(prevent_search(models.TextField( blank=True, default='', - )) + ))) allow_simultaneous = models.BooleanField( default=False ) diff --git a/awxkit/awxkit/cli/options.py b/awxkit/awxkit/cli/options.py index 9fc2cf05a9..17b8a93c44 100644 --- a/awxkit/awxkit/cli/options.py +++ b/awxkit/awxkit/cli/options.py @@ -1,3 +1,7 @@ +import argparse +import json +import yaml + from distutils.util import strtobool from .custom import CustomAction @@ -84,6 +88,18 @@ class ResourceOptionsParser(object): if method == 'list' and param.get('filterable') is False: continue + def json_or_yaml(v): + try: + return json.loads(v) + except Exception: + try: + return yaml.safe_load(v) + except Exception: + raise argparse.ArgumentTypeError("{} is not valid JSON or YAML".format(v)) + + def jsonstr(v): + return json.dumps(json_or_yaml(v)) + kwargs = { 'help': help_text, 'required': required, @@ -92,14 +108,16 @@ class ResourceOptionsParser(object): 'field': int, 'integer': int, 'boolean': strtobool, - 'field': int, # foreign key + 'id': int, # foreign key + 'json': json_or_yaml, }.get(param['type'], str), } meta_map = { 'string': 'TEXT', 'integer': 'INTEGER', 'boolean': 'BOOLEAN', - 'field': 'ID', # foreign key + 'id': 'ID', # foreign key + 'json': 'JSON/YAML', } if param.get('choices', []): kwargs['choices'] = [c[0] for c in param['choices']] @@ -112,7 +130,7 @@ class ResourceOptionsParser(object): elif param['type'] in meta_map: kwargs['metavar'] = meta_map[param['type']] - if param['type'] == 'field': + if param['type'] == 'id' and not kwargs.get('help'): kwargs['help'] = 'the ID of the associated {}'.format(k) # SPECIAL CUSTOM LOGIC GOES HERE :'( @@ -126,6 +144,15 @@ class ResourceOptionsParser(object): if self.resource == 'job_templates' and method == 'create' and k in ('project', 'playbook'): kwargs['required'] = required = True + # unlike *other* actual JSON fields in the API, inventory and JT + # variables *actually* want json.dumps() strings (ugh) + # see: https://github.com/ansible/awx/issues/2371 + if ( + (self.resource in ('job_templates', 'workflow_job_templates') and k == 'extra_vars') or + (self.resource in ('inventory', 'groups', 'hosts') and k == 'variables') + ): + kwargs['type'] = jsonstr + if required: required_group.add_argument( '--{}'.format(k),