Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Service broker create instance schemas #834

Merged
merged 5 commits into from
Jun 30, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions app/models/services/service_plan.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
export_attributes :name, :free, :description, :service_guid, :extra, :unique_id, :public, :bindable, :active, :create_instance_schema

export_attributes_from_methods bindable: :bindable?

import_attributes :name, :free, :description, :service_guid, :extra, :unique_id, :public, :bindable
import_attributes :name, :free, :description, :service_guid, :extra, :unique_id, :public, :bindable, :create_instance_schema

strip_attributes :name

Expand Down
1 change: 1 addition & 0 deletions app/presenters/v2/presenter_provider.rb
Original file line number Diff line number Diff line change
Expand Up @@ -32,3 +32,4 @@ def self.presenters
require_relative 'service_instance_presenter'
require_relative 'space_presenter'
require_relative 'organization_presenter'
require_relative 'service_plan_presenter'
46 changes: 46 additions & 0 deletions app/presenters/v2/service_plan_presenter.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
module CloudController
module Presenters
module V2
class ServicePlanPresenter < BasePresenter
extend PresenterProvider

present_for_class 'VCAP::CloudController::ServicePlan'

def entity_hash(controller, plan, opts, depth, parents, orphans=nil)
entity = DefaultPresenter.new.entity_hash(controller, plan, opts, depth, parents, orphans)

schemas = present_schemas(plan)
entity.merge!(schemas)
entity.delete('create_instance_schema')

entity
end

private

def present_schemas(plan)
create_instance_schema = parse_schema(plan.create_instance_schema)
{
'schemas' => {
'service_instance' => {
'create' => {
'parameters' => create_instance_schema
}
}
}
}
end

def parse_schema(schema)
return {} unless schema

begin
JSON.parse(schema)
rescue JSON::ParserError
{}
end
end
end
end
end
end
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
Sequel.migration do
change do
add_column :service_plans, :create_instance_schema, :text, null: true
end
end
3 changes: 2 additions & 1 deletion lib/services/service_brokers/service_manager.rb
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,8 @@ def update_or_create_plans(catalog)
free: catalog_plan.free,
bindable: catalog_plan.bindable,
active: true,
extra: catalog_plan.metadata ? catalog_plan.metadata.to_json : nil
extra: catalog_plan.metadata ? catalog_plan.metadata.to_json : nil,
create_instance_schema: catalog_plan.schemas.create_instance ? catalog_plan.schemas.create_instance.to_json : nil
})
@services_event_repository.with_service_plan_event(plan) do
plan.save(changed: true)
Expand Down
1 change: 1 addition & 0 deletions lib/services/service_brokers/v2.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ module VCAP::Services::ServiceBrokers::V2 end
require 'services/service_brokers/v2/catalog'
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/http_client'
require 'services/service_brokers/v2/client'
require 'services/service_brokers/v2/orphan_mitigator'
Expand Down
10 changes: 8 additions & 2 deletions lib/services/service_brokers/v2/catalog_plan.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,7 @@ module VCAP::Services::ServiceBrokers::V2
class CatalogPlan
include CatalogValidationHelper

attr_reader :broker_provided_id, :name, :description, :metadata, :catalog_service, :errors, :free, :bindable

attr_reader :broker_provided_id, :name, :description, :metadata, :catalog_service, :errors, :free, :bindable, :schemas
def initialize(catalog_service, attrs)
@catalog_service = catalog_service
@broker_provided_id = attrs['id']
Expand All @@ -13,6 +12,7 @@ def initialize(catalog_service, attrs)
@errors = VCAP::Services::ValidationErrors.new
@free = attrs['free'].nil? ? true : attrs['free']
@bindable = attrs['bindable']
@schemas = CatalogSchemas.new(attrs['schemas'])
end

def cc_plan
Expand All @@ -22,6 +22,7 @@ def cc_plan
def valid?
return @valid if defined? @valid
validate!
validate_schemas!
@valid = errors.empty?
end

Expand All @@ -38,6 +39,10 @@ def validate!
validate_bool!(:bindable, bindable) if bindable
end

def validate_schemas!
errors.add_nested(schemas, schemas.errors) unless schemas.valid?
end

def human_readable_attr_name(name)
{
broker_provided_id: 'Plan id',
Expand All @@ -46,6 +51,7 @@ def human_readable_attr_name(name)
metadata: 'Plan metadata',
free: 'Plan free',
bindable: 'Plan bindable',
schemas: 'Plan schemas',
}.fetch(name) { raise NotImplementedError }
end
end
Expand Down
38 changes: 38 additions & 0 deletions lib/services/service_brokers/v2/catalog_schemas.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
module VCAP::Services::ServiceBrokers::V2
class CatalogSchemas
attr_reader :errors, :create_instance

def initialize(attrs)
@errors = VCAP::Services::ValidationErrors.new
validate_and_populate_create_instance(attrs)
end

def valid?
errors.empty?
end

private

def validate_and_populate_create_instance(attrs)
return unless attrs
unless attrs.is_a? Hash
errors.add("Schemas must be a hash, but has value #{attrs.inspect}")
return
end

path = []
['service_instance', 'create', 'parameters'].each do |key|
path += [key]
attrs = attrs[key]
return nil unless attrs

unless attrs.is_a? Hash
errors.add("Schemas #{path.join('.')} must be a hash, but has value #{attrs.inspect}")
return nil
end
end

@create_instance = attrs
end
end
end
18 changes: 15 additions & 3 deletions lib/services/service_brokers/validation_errors_formatter.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
module VCAP::Services::ServiceBrokers
class ValidationErrorsFormatter
INDENT = ' '.freeze

def format(validation_errors)
message = "\n"
validation_errors.messages.each { |e| message += "#{e}\n" }
Expand All @@ -9,19 +10,30 @@ def format(validation_errors)

message += "Service #{service.name}\n"
service_errors.messages.each do |error|
message += "#{INDENT}#{error}\n"
message += indent + "#{error}\n"
end

service_errors.nested_errors.each do |plan, plan_errors|
next if plan_errors.empty?

message += "#{INDENT}Plan #{plan.name}\n"
message += indent + "Plan #{plan.name}\n"
plan_errors.messages.each do |error|
message += "#{INDENT}#{INDENT}#{error}\n"
message += indent(2) + "#{error}\n"
end

plan_errors.nested_errors.each do |schema, schema_errors|
message += indent(2) + "Schemas\n"
schema_errors.messages.each do |error|
message += indent(3) + "#{error}\n"
end
end
end
end
message
end

def indent(num=1)
INDENT.to_s * num
end
end
end
68 changes: 68 additions & 0 deletions spec/acceptance/broker_api_compatibility/broker_api_v2.13_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
require 'spec_helper'

RSpec.describe 'Service Broker API integration' do
describe 'v2.13' do
include VCAP::CloudController::BrokerApiHelper

let(:create_instance_schema) { {} }
let(:schemas) {
{
'service_instance' => {
'create' => {
'parameters' => create_instance_schema
}
}
}
}

let(:catalog) { default_catalog(plan_schemas: schemas) }

before do
setup_cc
setup_broker(catalog)
@broker = VCAP::CloudController::ServiceBroker.find guid: @broker_guid
end

context 'when a broker catalog defines plan schemas' do
let(:create_instance_schema) {
{
'$schema' => 'http://example.com/schema',
'type' => 'object'
}
}

it 'is responds with the schema for a service plan entry' do
get("/v2/service_plans/#{@plan_guid}",
{}.to_json,
json_headers(admin_headers))

parsed_body = MultiJson.load(last_response.body)
expect(parsed_body['entity']['schemas']).to eq schemas
end
end

context 'when the broker catalog defines a plan without schemas' do
it 'responds with an empty schema' do
get("/v2/service_plans/#{@large_plan_guid}",
{}.to_json,
json_headers(admin_headers))

parsed_body = MultiJson.load(last_response.body)
expect(parsed_body['entity']['schemas']).to eq({ 'service_instance' => { 'create' => { 'parameters' => {} } } })
end
end

context 'when the broker catalog has an invalid schema' do
before do
update_broker(default_catalog(plan_schemas: { 'service_instance' => { 'create' => true } }))
end

it 'returns an error' do
parsed_body = MultiJson.load(last_response.body)

expect(parsed_body['code']).to eq(270012)
expect(parsed_body['description']).to include('Schemas service_instance.create must be a hash, but has value true')
end
end
end
end
2 changes: 1 addition & 1 deletion spec/api/api_version_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
require 'vcap/digester'

RSpec.describe 'Stable API warning system', api_version_check: true do
API_FOLDER_CHECKSUM = '9256f850ec5715e833d756dbc84d15a7d118bac8'.freeze
API_FOLDER_CHECKSUM = '768d0cd5ab9b32809f63c7e6ede60ff101ca431e'.freeze

it 'tells the developer if the API specs change' do
api_folder = File.expand_path('..', __FILE__)
Expand Down
38 changes: 20 additions & 18 deletions spec/api/documentation/events_api_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -613,15 +613,16 @@

example 'List Service Plan Create Events' do
new_plan = VCAP::CloudController::ServicePlan.new(
guid: 'guid',
name: 'plan-name',
service: test_service,
description: 'A plan',
unique_id: 'guid',
free: true,
public: true,
active: true,
bindable: true
guid: 'guid',
name: 'plan-name',
service: test_service,
description: 'A plan',
unique_id: 'guid',
free: true,
public: true,
active: true,
bindable: true,
create_instance_schema: '{}'
)
service_event_repository.with_service_plan_event(new_plan) do
new_plan.save
Expand All @@ -638,15 +639,16 @@
actee_name: new_plan.name,
space_guid: '',
metadata: {
'name' => new_plan.name,
'free' => new_plan.free,
'description' => new_plan.description,
'service_guid' => new_plan.service.guid,
'extra' => new_plan.extra,
'unique_id' => new_plan.unique_id,
'public' => new_plan.public,
'active' => new_plan.active,
'bindable' => new_plan.bindable
'name' => new_plan.name,
'free' => new_plan.free,
'description' => new_plan.description,
'service_guid' => new_plan.service.guid,
'extra' => new_plan.extra,
'unique_id' => new_plan.unique_id,
'public' => new_plan.public,
'active' => new_plan.active,
'bindable' => new_plan.bindable,
'create_instance_schema' => new_plan.create_instance_schema
}
}
end
Expand Down
8 changes: 5 additions & 3 deletions spec/api/documentation/service_plans_api_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,16 +11,18 @@
field :guid, 'The guid of the service plan', required: false
field :public, 'A boolean describing that the plan is visible to the all users', required: false, default: true

standard_model_list(:service_plans, VCAP::CloudController::ServicePlansController)
standard_model_get(:service_plans, nested_attributes: [:service])
expected_attributes = VCAP::CloudController::ServicePlan.new.export_attrs - [:create_instance_schema] + [:schemas]

standard_model_list(:service_plans, VCAP::CloudController::ServicePlansController, export_attributes: expected_attributes)
standard_model_get(:service_plans, export_attributes: expected_attributes)
standard_model_delete(:service_plans)

put '/v2/service_plans' do
example 'Updating a Service Plan' do
client.put "/v2/service_plans/#{guid}", fields_json(public: false), headers
expect(status).to eq(201)

standard_entity_response parsed_response, :service_plans
standard_entity_response parsed_response, :service_plans, expected_attributes: expected_attributes
end
end
end
Expand Down
3 changes: 2 additions & 1 deletion spec/api/documentation/services_api_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,8 @@
VCAP::CloudController::ServicePlan.make(service: service)
end

standard_model_list :service_plan, VCAP::CloudController::ServicePlansController, outer_model: :service
expected_attributes = VCAP::CloudController::ServicePlan.new.export_attrs - [:create_instance_schema] + [:schemas]
standard_model_list :service_plan, VCAP::CloudController::ServicePlansController, { outer_model: :service, export_attributes: expected_attributes }
end
end
end
8 changes: 5 additions & 3 deletions spec/support/broker_api_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ def stub_catalog_fetch(broker_response_status=200, catalog=nil)
body: catalog.to_json)
end

def default_catalog(plan_updateable: false, requires: [])
def default_catalog(plan_updateable: false, requires: [], plan_schemas: {})
{
services: [
{
Expand All @@ -41,8 +41,10 @@ def default_catalog(plan_updateable: false, requires: [])
{
id: 'plan1-guid-here',
name: 'small',
description: 'A small shared database with 100mb storage quota and 10 connections'
}, {
description: 'A small shared database with 100mb storage quota and 10 connections',
schemas: plan_schemas
},
{
id: 'plan2-guid-here',
name: 'large',
description: 'A large dedicated database with 10GB storage quota, 512MB of RAM, and 100 connections'
Expand Down
Loading