Merge pull request #865 from pivotal-cf-experimental/bsus-pr

Service broker update instance schemas

app/models/services/service_plan.rb

@@ -6,11 +6,11 @@ class ServicePlan < Sequel::Model
 
     add_association_dependencies service_plan_visibilities: :destroy
 
-    export_attributes :name, :free, :description, :service_guid, :extra, :unique_id, :public, :bindable, :active, :create_instance_schema
+    export_attributes :name, :free, :description, :service_guid, :extra, :unique_id, :public, :bindable, :active, :create_instance_schema, :update_instance_schema
 
     export_attributes_from_methods bindable: :bindable?
 
-    import_attributes :name, :free, :description, :service_guid, :extra, :unique_id, :public, :bindable, :create_instance_schema
+    import_attributes :name, :free, :description, :service_guid, :extra, :unique_id, :public, :bindable, :create_instance_schema, :update_instance_schema
 
     strip_attributes :name
 

app/presenters/v2/service_plan_presenter.rb

@@ -12,6 +12,7 @@ def entity_hash(controller, plan, opts, depth, parents, orphans=nil)
           schemas = present_schemas(plan)
           entity.merge!(schemas)
           entity.delete('create_instance_schema')
+          entity.delete('update_instance_schema')
 
           entity
         end
@@ -20,11 +21,15 @@ def entity_hash(controller, plan, opts, depth, parents, orphans=nil)
 
         def present_schemas(plan)
           create_instance_schema = parse_schema(plan.create_instance_schema)
+          update_instance_schema = parse_schema(plan.update_instance_schema)
           {
             'schemas' => {
               'service_instance' => {
                 'create' => {
                   'parameters' => create_instance_schema
+                },
+                'update' => {
+                    'parameters' => update_instance_schema
                 }
               }
             }

db/migrations/20170712153045_add_instance_update_schema_to_service_plans.rb

@@ -0,0 +1,5 @@
+Sequel.migration do
+  change do
+    add_column :service_plans, :update_instance_schema, :text, null: true
+  end
+end

lib/services/service_brokers/service_manager.rb

@@ -65,7 +65,8 @@ def update_or_create_plans(catalog)
           bindable:    catalog_plan.bindable,
           active:      true,
           extra:       catalog_plan.metadata.try(:to_json),
-          create_instance_schema: catalog_plan.schemas.create_instance.try(:to_json)
+          create_instance_schema: catalog_plan.schemas.create_instance.try(:to_json),
+          update_instance_schema: catalog_plan.schemas.update_instance.try(:to_json)
         })
         @services_event_repository.with_service_plan_event(plan) do
           plan.save(changed: true)

lib/services/service_brokers/v2.rb

@@ -5,6 +5,7 @@ module VCAP::Services::ServiceBrokers::V2 end
 require 'services/service_brokers/v2/catalog_service'
 require 'services/service_brokers/v2/catalog_plan'
 require 'services/service_brokers/v2/catalog_schemas'
+require 'services/service_brokers/v2/schema'
 require 'services/service_brokers/v2/http_client'
 require 'services/service_brokers/v2/client'
 require 'services/service_brokers/v2/orphan_mitigator'

lib/services/service_brokers/v2/catalog_schemas.rb

@@ -1,109 +1,78 @@
-require 'json-schema'
-
 module VCAP::Services::ServiceBrokers::V2
-  MAX_SCHEMA_SIZE = 65_536
   class CatalogSchemas
-    attr_reader :errors, :create_instance
+    attr_reader :errors, :create_instance, :update_instance
 
-    def initialize(schema)
+    def initialize(schemas)
       @errors = VCAP::Services::ValidationErrors.new
-      validate_and_populate_create_instance(schema)
-    end
-
-    def valid?
-      errors.empty?
-    end
-
-    private
-
-    def validate_and_populate_create_instance(schemas)
       return unless schemas
-      unless schemas.is_a? Hash
-        errors.add("Schemas must be a hash, but has value #{schemas.inspect}")
-        return
-      end
 
-      path = []
-      ['service_instance', 'create', 'parameters'].each do |key|
-        path += [key]
-        schemas = schemas[key]
-        return nil unless schemas
+      return unless validate_structure(schemas, [])
+      service_instance_path = ['service_instance']
+      return unless validate_structure(schemas, service_instance_path)
 
-        unless schemas.is_a? Hash
-          errors.add("Schemas #{path.join('.')} must be a hash, but has value #{schemas.inspect}")
-          return nil
-        end
-      end
+      create_schema = get_method('create', schemas)
+      @create_instance = Schema.new(create_schema) if create_schema
 
-      create_instance_schema = schemas
-      create_instance_path = path.join('.')
+      update_schema = get_method('update', schemas)
+      @update_instance = Schema.new(update_schema) if update_schema
+    end
 
-      validate_schema(create_instance_path, create_instance_schema)
-      return unless errors.empty?
+    def valid?
+      return false unless errors.empty?
 
-      @create_instance = create_instance_schema
-    end
+      if create_instance && !create_instance.validate
+        add_schema_validation_errors(create_instance.errors, 'service_instance.create.parameters')
+      end
 
-    def validate_schema(path, schema)
-      schema_validations.each do |validation|
-        break if errors.present?
-        send(validation, path, schema)
+      if update_instance && !update_instance.validate
+        add_schema_validation_errors(update_instance.errors, 'service_instance.update.parameters')
       end
-    end
 
-    def schema_validations
-      [
-        :validate_schema_size,
-        :validate_metaschema,
-        :validate_no_external_references,
-        :validate_schema_type
-      ]
+      errors.empty?
     end
 
-    def validate_schema_type(path, schema)
-      add_schema_error_msg(path, 'must have field "type", with value "object"') if schema['type'] != 'object'
-    end
+    private
 
-    def validate_schema_size(path, schema)
-      errors.add("Schema #{path} is larger than 64KB") if schema.to_json.length > MAX_SCHEMA_SIZE
-    end
+    def validate_structure(schemas, path)
+      unless schemas.is_a? Hash
+        add_schema_type_error_msg(path, schemas)
+        return false
+      end
+      schema = path.reduce(schemas) { |current, key|
+        return false unless current.key?(key)
+        current.fetch(key)
+      }
+      return false unless schema
+
+      unless schema.is_a? Hash
+        add_schema_type_error_msg(path, schema)
+        return false
+      end
 
-    def validate_metaschema(path, schema)
-      JSON::Validator.schema_reader = JSON::Schema::Reader.new(accept_uri: false, accept_file: false)
-      file = File.read(JSON::Validator.validator_for_name('draft4').metaschema)
+      true
+    end
 
-      metaschema = JSON.parse(file)
+    def get_method(method, schema)
+      path = ['service_instance', method]
+      return unless validate_structure(schema, path)
 
-      begin
-        errors = JSON::Validator.fully_validate(metaschema, schema)
-      rescue => e
-        add_schema_error_msg(path, e)
-        return nil
-      end
+      path = ['service_instance', method, 'parameters']
+      return unless validate_structure(schema, path)
 
-      errors.each do |error|
-        add_schema_error_msg(path, "Must conform to JSON Schema Draft 04: #{error}")
-      end
+      schema['service_instance'][method]['parameters']
     end
 
-    def validate_no_external_references(path, schema)
-      JSON::Validator.schema_reader = JSON::Schema::Reader.new(accept_uri: false, accept_file: false)
-
-      begin
-        JSON::Validator.validate!(schema, {})
-      rescue JSON::Schema::SchemaError => e
-        add_schema_error_msg(path, "Custom meta schemas are not supported: #{e}")
-      rescue JSON::Schema::ReadRefused => e
-        add_schema_error_msg(path, "No external references are allowed: #{e}")
-      rescue JSON::Schema::ValidationError
-        # We don't care if our input fails validation on broker schema
-      rescue => e
-        add_schema_error_msg(path, e)
+    def add_schema_validation_errors(schema_errors, path)
+      schema_errors.messages.each do |_, error_list|
+        error_list.each do |error_msg|
+          errors.add("Schema #{path} is not valid. #{error_msg}")
+        end
       end
     end
 
-    def add_schema_error_msg(path, err)
-      errors.add("Schema #{path} is not valid. #{err}")
+    def add_schema_type_error_msg(path, value)
+      path = path.empty? ? '' : " #{path.join('.')}"
+      errors.add("Schemas#{path} must be a hash, but has value #{value.inspect}")
     end
   end
 end

lib/services/service_brokers/v2/schema.rb

@@ -0,0 +1,67 @@
+require 'json-schema'
+
+module VCAP::Services::ServiceBrokers::V2
+  class Schema
+    include ActiveModel::Validations
+
+    MAX_SCHEMA_SIZE = 65_536
+
+    validates :to_json, length: { maximum: MAX_SCHEMA_SIZE, message: 'Must not be larger than 64KB' }
+    validate :validate_schema_type, :validate_metaschema, :validate_no_external_references
+
+    def initialize(schema)
+      @schema = schema
+    end
+
+    def to_json
+      @schema.to_json
+    end
+
+    private
+
+    def validate_schema_type
+      return unless errors.blank?
+      add_schema_error_msg('must have field "type", with value "object"') if @schema['type'] != 'object'
+    end
+
+    def validate_metaschema
+      return unless errors.blank?
+      JSON::Validator.schema_reader = JSON::Schema::Reader.new(accept_uri: false, accept_file: false)
+      file = File.read(JSON::Validator.validator_for_name('draft4').metaschema)
+
+      metaschema = JSON.parse(file)
+
+      begin
+        errors = JSON::Validator.fully_validate(metaschema, @schema, errors_as_objects: true)
+      rescue => e
+        add_schema_error_msg(e)
+        return nil
+      end
+
+      errors.each do |error|
+        add_schema_error_msg("Must conform to JSON Schema Draft 04: #{error[:message]}")
+      end
+    end
+
+    def validate_no_external_references
+      return unless errors.blank?
+      JSON::Validator.schema_reader = JSON::Schema::Reader.new(accept_uri: false, accept_file: false)
+
+      begin
+        JSON::Validator.validate!(@schema, {})
+      rescue JSON::Schema::SchemaError
+        add_schema_error_msg('Custom meta schemas are not supported.')
+      rescue JSON::Schema::ReadRefused => e
+        add_schema_error_msg("No external references are allowed: #{e}")
+      rescue JSON::Schema::ValidationError
+        # We don't care if our input fails validation on broker schema
+      rescue => e
+        add_schema_error_msg(e)
+      end
+    end
+
+    def add_schema_error_msg(err)
+      errors.add(:base, err.to_s)
+    end
+  end
+end