Use new schema syntax and recursively set default values. Includes unit tests.

expandybird/expansion/schema_validation.py

@@ -26,13 +26,12 @@ PROPERTIES = "properties"
 # This validator will set default values in properties.
 # This does not return a complete set of errors; use only for setting defaults.
 # Pass this object a schema to get a validator for that schema.
-DEFAULT_VALIDATOR = schema_validation_utils.OnlyValidateProperties(
-    schema_validation_utils.ExtendWithDefault(jsonschema.Draft4Validator))
+DEFAULT_SETTER = schema_validation_utils.ExtendWithDefault(
+    jsonschema.Draft4Validator)
 
-# This is a regular validator, use after using the DEFAULT_VALIDATOR
+# This is a regular validator, use after using the DEFAULT_SETTER
 # Pass this object a schema to get a validator for that schema.
-VALIDATOR = schema_validation_utils.OnlyValidateProperties(
-    jsonschema.Draft4Validator)
+VALIDATOR = jsonschema.Draft4Validator
 
 # This is a validator using the default Draft4 metaschema,
 # use it to validate user schemas.
@@ -61,29 +60,61 @@ IMPORT_SCHEMA_VALIDATOR = jsonschema.Draft4Validator(
     yaml.safe_load(IMPORT_SCHEMA))
 
 
+def _ValidateSchema(schema, validating_imports, schema_name, template_name):
+  """Validate that the passed in schema file is correctly formatted.
+
+  Args:
+    schema: contents of the schema file
+    validating_imports: boolean, if we should validate the 'imports'
+        section of the schema
+    schema_name: name of the schema file to validate
+    template_name: name of the template whose properties are being validated
+
+  Raises:
+    ValidationErrors: A list of ValidationError errors that occured when
+        validating the schema file
+  """
+  schema_errors = []
+
+  # Validate the syntax of the optional "imports:" section of the schema
+  if validating_imports:
+    schema_errors.extend(IMPORT_SCHEMA_VALIDATOR.iter_errors(schema))
+
+  # Validate the syntax of the jsonSchema section of the schema
+  try:
+    schema_errors.extend(SCHEMA_VALIDATOR.iter_errors(schema))
+  except jsonschema.RefResolutionError as e:
+    # Calls to iter_errors could throw a RefResolution exception
+    raise ValidationErrors(schema_name, template_name,
+                           [e], is_schema_error=True)
+
+  if schema_errors:
+    raise ValidationErrors(schema_name, template_name,
+                           schema_errors, is_schema_error=True)
+
+
 def Validate(properties, schema_name, template_name, imports):
   """Given a set of properties, validates it against the given schema.
 
   Args:
     properties: dict, the properties to be validated
     schema_name: name of the schema file to validate
-    template_name: name of the template whose's properties are being validated
-    imports: map from string to string, the map of imported files names
-        and contents
+    template_name: name of the template whose properties are being validated
+    imports: the map of imported files names to file contents
 
   Returns:
     Dict containing the validated properties, with defaults filled in
 
   Raises:
     ValidationErrors: A list of ValidationError errors that occurred when
-        validating the properties and schema
+        validating the properties and schema,
+        or if the schema file was not found
   """
   if schema_name not in imports:
     raise ValidationErrors(schema_name, template_name,
-                           ["Could not find schema file '"
-                            + schema_name + "'."])
-  else:
-    raw_schema = imports[schema_name]
+                           ["Could not find schema file '%s'." % schema_name])
+
+  raw_schema = imports[schema_name]
 
   if properties is None:
     properties = {}
@@ -91,33 +122,14 @@ def Validate(properties, schema_name, template_name, imports):
   schema = yaml.safe_load(raw_schema)
 
   # If the schema is empty, do nothing.
-  if schema is None:
+  if not schema:
     return properties
 
-  schema_errors = []
   validating_imports = IMPORTS in schema and schema[IMPORTS]
 
-  # Validate the syntax of the optional "imports:" section of the schema
-  if validating_imports:
-    schema_errors.extend(list(IMPORT_SCHEMA_VALIDATOR.iter_errors(schema)))
-
-  # Validate the syntax of the optional "properties:" section of the schema
-  if PROPERTIES in schema and schema[PROPERTIES]:
-    try:
-      schema_errors.extend(
-          list(SCHEMA_VALIDATOR.iter_errors(schema[PROPERTIES])))
-    except jsonschema.RefResolutionError as e:
-      # Calls to iter_errors could throw a RefResolution exception
-      raise ValidationErrors(schema_name, template_name,
-                             list(e), is_schema_error=True)
-
-  if schema_errors:
-    raise ValidationErrors(schema_name, template_name,
-                           schema_errors, is_schema_error=True)
+   # If this doesn't raise any exceptions, we can assume we have a valid schema
+  _ValidateSchema(schema, validating_imports, schema_name, template_name)
 
-  ######
-  # Assume we have a valid schema
-  ######
   errors = []
 
   # Validate that all files specified as "imports:" were included
@@ -131,24 +143,25 @@ def Validate(properties, schema_name, template_name, imports):
         import_name = import_object["path"]
 
       if import_name not in imports:
-        errors.append(("File '" + import_name + "' requested in schema '"
-                       + schema_name + "' but not included with imports."))
+        errors.append(("File '%s' requested in schema '%s' "
+                       "but not included with imports."
+                       % (import_name, schema_name)))
 
   try:
-    # This code block uses DEFAULT_VALIDATOR and VALIDATOR for two very
+    # This code block uses DEFAULT_SETTER and VALIDATOR for two very
     # different purposes.
-    # DEFAULT_VALIDATOR is based on JSONSchema 4, but uses modified validators:
+    # DEFAULT_SETTER is based on JSONSchema 4, but uses modified validators:
     # - The 'required' validator does nothing
     # - The 'properties' validator sets default values on user properties
     # With these changes, the validator does not report errors correctly.
     #
     # So, we do error reporting in two steps:
-    # 1) Use DEFAULT_VALIDATOR to set default values in the user's properties
+    # 1) Use DEFAULT_SETTER to set default values in the user's properties
     # 2) Use the unmodified VALIDATOR to report all of the errors
 
     # Calling iter_errors mutates properties in place, adding default values.
     # You must call list()! This is a generator, not a function!
-    list(DEFAULT_VALIDATOR(schema).iter_errors(properties))
+    list(DEFAULT_SETTER(schema).iter_errors(properties))
 
     # Now that we have default values, validate the properties
     errors.extend(list(VALIDATOR(schema).iter_errors(properties)))
@@ -158,7 +171,7 @@ def Validate(properties, schema_name, template_name, imports):
   except jsonschema.RefResolutionError as e:
     # Calls to iter_errors could throw a RefResolution exception
     raise ValidationErrors(schema_name, template_name,
-                           list(e), is_schema_error=True)
+                           [e], is_schema_error=True)
   except TypeError as e:
     raise ValidationErrors(
         schema_name, template_name,
@@ -191,7 +204,7 @@ class ValidationErrors(Exception):
       message = "Invalid properties for '%s':\n" % self.template_name
 
     for error in self.errors:
-      if type(error) is jsonschema.exceptions.ValidationError:
+      if isinstance(error, jsonschema.exceptions.ValidationError):
         error_message = error.message
         location = list(error.path)
         if location and len(location):

expandybird/expansion/schema_validation_utils.py

@@ -15,35 +15,10 @@
 
 import jsonschema
 
-DEFAULT = "default"
-PROPERTIES = "properties"
-REF = "$ref"
-REQUIRED = "required"
-
-
-def OnlyValidateProperties(validator_class):
-  """Takes a validator and makes it process only the 'properties' top level.
-
-  Args:
-    validator_class: A class to add a new validator to
-
-  Returns:
-    A validator_class that will validate properties against things
-    under the top level "properties" field
-  """
-
-  def PropertiesValidator(unused_validator, inputs, instance, schema):
-    if inputs is None:
-      inputs = {}
-    for error in validator_class(schema).iter_errors(instance, inputs):
-      yield error
-
-  # This makes sure the only keyword jsonschema will validate is 'properties'
-  new_validators = ClearValidatorMap(validator_class.VALIDATORS)
-  new_validators.update({PROPERTIES: PropertiesValidator})
-
-  return jsonschema.validators.extend(
-      validator_class, new_validators)
+DEFAULT = 'default'
+PROPERTIES = 'properties'
+REF = '$ref'
+REQUIRED = 'required'
 
 
 def ExtendWithDefault(validator_class):
@@ -55,33 +30,33 @@ def ExtendWithDefault(validator_class):
   Returns:
     A validator_class that will set default values and ignore required fields
   """
+  validate_properties = validator_class.VALIDATORS['properties']
 
-  def SetDefaultsInProperties(validator, properties, instance, unused_schema):
-    if properties is None:
-      properties = {}
-    SetDefaults(validator, properties, instance)
+  def SetDefaultsInProperties(validator, user_schema, user_properties,
+                              parent_schema):
+    SetDefaults(validator, user_schema or {}, user_properties, parent_schema,
+                validate_properties)
 
   return jsonschema.validators.extend(
       validator_class, {PROPERTIES: SetDefaultsInProperties,
                         REQUIRED: IgnoreKeyword})
 
 
-def SetDefaults(validator, properties, instance):
+def SetDefaults(validator, user_schema, user_properties, parent_schema,
+                validate_properties):
   """Populate the default values of properties.
 
   Args:
-    validator: A generator that validates the "properties" keyword
-    properties: User properties on which to set defaults
-    instance: Piece of user schema containing "properties"
+    validator: A generator that validates the "properties" keyword of the schema
+    user_schema: Schema which might define defaults, might be a nested part of
+      the entire schema file.
+    user_properties: User provided values which we are setting defaults on
+    parent_schema: Schema object that contains the schema being evaluated on
+      this pass, user_schema.
+    validate_properties: Validator function, called recursively.
   """
-  if not properties:
-    return
-
-  for dm_property, subschema in properties.iteritems():
-    # If the property already has a value, we don't need it's default
-    if dm_property in instance:
-      return
 
+  for schema_property, subschema in user_schema.iteritems():
     # The ordering of these conditions assumes that '$ref' blocks override
     # all other schema info, which is what the jsonschema library assumes.
 
@@ -89,17 +64,21 @@ def SetDefaults(validator, properties, instance):
     # see if that reference defines a 'default' value
     if REF in subschema:
       out = ResolveReferencedDefault(validator, subschema[REF])
-      instance.setdefault(dm_property, out)
+      user_properties.setdefault(schema_property, out)
     # Otherwise, see if the subschema has a 'default' value
     elif DEFAULT in subschema:
-      instance.setdefault(dm_property, subschema[DEFAULT])
+      user_properties.setdefault(schema_property, subschema[DEFAULT])
+
+  # Recursively apply defaults. This is a generator, so we must wrap with list()
+  list(validate_properties(validator, user_schema,
+                           user_properties, parent_schema))
 
 
 def ResolveReferencedDefault(validator, ref):
   """Resolves a reference, and returns any default value it defines.
 
   Args:
-    validator: A generator the validates the "$ref" keyword
+    validator: A generator that validates the "$ref" keyword
     ref: The target of the "$ref" keyword
 
   Returns:
@@ -110,14 +89,6 @@ def ResolveReferencedDefault(validator, ref):
       return resolved[DEFAULT]
 
 
-def ClearValidatorMap(validators):
-  """Remaps all JsonSchema validators to make them do nothing."""
-  ignore_validators = {}
-  for keyword in validators:
-    ignore_validators.update({keyword: IgnoreKeyword})
-  return ignore_validators
-
-
 def IgnoreKeyword(
     unused_validator, unused_required, unused_instance, unused_schema):
   """Validator for JsonSchema that does nothing."""

expandybird/test/schemas/bad.jinja.schema

+info:
+  title: Schema with a lots of errors in it
+
+imports:
+
+properties:
+  exclusiveMin:
+    type: integer
+    exclusiveMinimum: 0

expandybird/test/schemas/default_ref.jinja.schema

+info:
+  title: Schema with a property that has a referenced default value
+
+imports:
+
+properties:
+  number:
+    $ref: '#/level/mult'
+
+level:
+  mult:
+    type: integer
+    multipleOf: 1
+    default: 1

expandybird/test/schemas/defaults.jinja.schema

+info:
+  title: Schema with properties that have default values
+
+imports:
+
+properties:
+  one:
+    type: integer
+    default: 1
+  alpha:
+    type: string
+    default: alpha

expandybird/test/schemas/defaults.py.schema

+info:
+  title: Schema with properties that have default values
+
+imports:
+
+properties:
+  one:
+    type: integer
+    default: 1
+  alpha:
+    type: string
+    default: alpha

expandybird/test/schemas/invalid_default.jinja.schema

+info:
+  title: Schema with a required integer property that has a default string value
+
+imports:
+
+required:
+  - number
+properties:
+  number:
+    type: integer
+    default: string

expandybird/test/schemas/invalid_reference.py.schema

+info:
+  title: Schema with references to something that doesnt exist
+
+imports:
+
+properties:
+  odd:
+    type: integer
+    not:
+      $ref: '#/wheeeeeee'

expandybird/test/schemas/invalid_reference_schema.py.schema

+info:
+  title: Schema with references to something that doesnt exist
+
+imports:
+
+properties:
+  odd:
+    $ref: '#/wheeeeeee'

expandybird/test/schemas/metadata.py.schema

+info:
+  title: Schema with properties that have extra metadata
+
+imports:
+
+properties:
+  one:
+    type: integer
+    default: 1
+    metadata:
+      gcloud: is great!
+      compute: is awesome
+  alpha:
+    type: string
+    default: alpha
+    metadata:
+      - you
+      - can
+      - do
+      - anything

expandybird/test/schemas/missing_quote.py.schema

+info:
+  title: Schema with references
+
+imports:
+
+properties:
+  number:
+    $ref: #/number
+
+number:
+    type: integer

expandybird/test/schemas/nested_defaults.py.schema

+info:
+  title: VM with Disks
+  author: Kubernetes
+  description: Creates a single vm, then attaches disks to it.
+
+required:
+- zone
+
+properties:
+  zone:
+    type: string
+    description: GCP zone
+    default: us-central1-a
+  disks:
+    type: array
+    items:
+      type: object
+      required:
+      - name
+      properties:
+        name:
+          type: string
+          description: Suffix for this disk
+        sizeGb:
+          type: integer
+          default: 100
+        diskType:
+          type: string
+          enum:
+          - pd-standard
+          - pd-ssd
+          default: pd-standard
+      additionalProperties: false

expandybird/test/schemas/numbers.py.schema

+info:
+  title: Schema with a lots of number properties and restrictions
+
+imports:
+
+properties:
+  minimum0:
+    type: integer
+    minimum: 0
+  exclusiveMin0:
+    type: integer
+    minimum: 0
+    exclusiveMinimum: true
+  maximum10:
+    type: integer
+    maximum: 10
+  exclusiveMax10:
+    type: integer
+    maximum: 10
+    exclusiveMaximum: true
+  even:
+    type: integer
+    multipleOf: 2
+  odd:
+    type: integer
+    not:
+      multipleOf: 2

expandybird/test/schemas/ref_nested_defaults.py.schema

+info:
+  title: VM with Disks
+  author: Kubernetes
+  description: Creates a single vm, then attaches disks to it.
+
+required:
+- zone
+
+properties:
+  zone:
+    type: string
+    description: GCP zone
+    default: us-central1-a
+  disks:
+    type: array
+    items:
+      $ref: '#/disk'
+
+disk:
+  type: object
+  required:
+  - name
+  properties:
+    name:
+      type: string
+      description: Suffix for this disk
+    sizeGb:
+      type: integer
+      default: 100
+    diskType:
+      type: string
+      enum:
+      - pd-standard
+      - pd-ssd
+      default: pd-standard
+  additionalProperties: false

expandybird/test/schemas/reference.jinja.schema

+info:
+  title: Schema with references
+
+imports:
+
+properties:
+  odd:
+    type: integer
+    not:
+      $ref: '#/even'
+
+
+even:
+    multipleOf: 2

expandybird/test/schemas/req_default_ref.py.schema

+info:
+  title: Schema with a required property that has a referenced default value
+
+imports:
+
+required:
+  - number
+properties:
+  number:
+    $ref: '#/default_val'
+
+default_val:
+  type: integer
+  default: my_name

expandybird/test/schemas/required.jinja.schema

+info:
+  title: Schema with a required property
+
+imports:
+
+required:
+  - name
+properties:
+  name:
+    type: string

expandybird/test/schemas/required_default.jinja.schema

+info:
+  title: Schema with a required property that has a default value
+
+imports:
+
+required:
+  - name
+properties:
+  name:
+    type: string
+    default: my_name