As a v3 API user, my POST requests to /v3/service_brokers are asynchr…

…onous (#1440)

This commit covers most of the happy paths and some unhappy ones.
User story comes right up soon for more interesting unhappy paths.

[#168083026]
[finishes #168476345]

Co-authored-by: Felisia Martini <fmartini@pivotal.io>
Co-authored-by: Winna Bridgewater <wbridgewater@pivotal.io>
Co-authored-by: Oleksii Fedorov <ofedorov@pivotal.io>
Co-authored-by: Derik Evangelista <devangelista@pivotal.io>
Co-authored-by: Joao Lebre <jlebre@pivotal.io>

app/actions/service_broker_create.rb

@@ -1,8 +1,13 @@
+require 'jobs/v3/synchronize_broker_catalog_job'
+
 module VCAP::CloudController
   module V3
     class ServiceBrokerCreate
-      class InvalidServiceBroker < StandardError; end
-      class SpaceNotFound < StandardError; end
+      class InvalidServiceBroker < StandardError
+      end
+
+      class SpaceNotFound < StandardError
+      end
 
       def initialize(service_event_repository, service_manager)
         @service_event_repository = service_event_repository
@@ -15,29 +20,24 @@ def create(message)
           broker_url: message.url,
           auth_username: message.credentials_data.username,
           auth_password: message.credentials_data.password,
+          space_guid: message.relationships_message.space_guid
         }
+        broker = ServiceBroker.create(params)
 
-        if message.space_guid
-          params[:space_id] = Space.first(guid: message.space_guid).id
-        end
-
-        broker = VCAP::CloudController::ServiceBroker.new(params)
-
-        registration = VCAP::Services::ServiceBrokers::ServiceBrokerRegistration.new(
-          broker,
-          service_manager,
-          service_event_repository,
-          route_services_enabled?,
-          volume_services_enabled?,
+        broker.update(
+          service_broker_state: VCAP::CloudController::ServiceBrokerState.new(
+            state: VCAP::CloudController::ServiceBrokerStateEnum::SYNCHRONIZING
+          )
         )
 
-        unless registration.create
-          raise InvalidServiceBroker.new(broker.errors.full_messages.join(','))
-        end
+        service_event_repository.record_broker_event(:create, broker, params)
 
-        {
-          warnings: registration.warnings
-        }
+        synchronization_job = SynchronizeBrokerCatalogJob.new(broker.guid)
+        pollable_job = Jobs::Enqueuer.new(synchronization_job, queue: 'cc-generic').enqueue_pollable
+
+        { pollable_job: pollable_job }
+      rescue Sequel::ValidationFailed => e
+        raise InvalidServiceBroker.new(e.errors.full_messages.join(','))
       end
 
       private

app/controllers/services/service_brokers_controller.rb

@@ -1,5 +1,5 @@
 require 'presenters/api/service_broker_presenter'
-require 'actions/services/service_broker_create'
+require 'actions/v2/services/service_broker_create'
 require 'actions/services/service_broker_update'
 
 module VCAP::CloudController

app/controllers/v3/service_brokers_controller.rb

@@ -47,14 +47,13 @@ def create
       unauthorized! unless permission_queryer.can_write_global_service_broker?
     end
 
-    service_event_repository = VCAP::CloudController::Repositories::ServiceEventRepository::WithBrokerActor.new
+    service_event_repository = VCAP::CloudController::Repositories::ServiceEventRepository::WithUserActor.new(user_audit_info)
     service_manager = VCAP::Services::ServiceBrokers::ServiceManager.new(service_event_repository)
     service_broker_create = VCAP::CloudController::V3::ServiceBrokerCreate.new(service_event_repository, service_manager)
-
     result = service_broker_create.create(message)
 
-    add_warning_headers(result[:warnings])
-    render status: :created, json: {}
+    url_builder = VCAP::CloudController::Presenters::ApiUrlBuilder.new
+    head :accepted, 'Location' => url_builder.build_url(path: "/v3/jobs/#{result[:pollable_job].guid}")
   rescue VCAP::CloudController::V3::ServiceBrokerCreate::InvalidServiceBroker => e
     unprocessable!(e.message)
   end

app/jobs/v3/synchronize_broker_catalog_job.rb

@@ -0,0 +1,108 @@
+module VCAP::CloudController
+  module V3
+    class SynchronizeBrokerCatalogJob < VCAP::CloudController::Jobs::CCJob
+      def initialize(broker_guid)
+        @broker_guid = broker_guid
+      end
+
+      def perform
+        Perform.new(broker_guid).perform
+      end
+
+      def job_name_in_configuration
+        :synchronize_service_broker_catalog
+      end
+
+      def max_attempts
+        1
+      end
+
+      def resource_type
+        'service_brokers'
+      end
+
+      def resource_guid
+        broker_guid
+      end
+
+      def display_name
+        'service_broker.catalog.synchronize'
+      end
+
+      private
+
+      attr_reader :broker_guid
+
+      class Perform
+        def initialize(broker_guid)
+          @broker_guid = broker_guid
+          @broker = ServiceBroker.find(guid: broker_guid)
+          @formatter = VCAP::Services::ServiceBrokers::ValidationErrorsFormatter.new
+          @service_event_repository = VCAP::CloudController::Repositories::ServiceEventRepository::WithBrokerActor.new
+          @client_manager = VCAP::Services::SSO::DashboardClientManager.new(broker, service_event_repository)
+          @broker_client = VCAP::Services::ServiceClientProvider.provide(broker: broker)
+          @service_manager = VCAP::Services::ServiceBrokers::ServiceManager.new(service_event_repository)
+        end
+
+        def perform
+          ensure_state_present
+
+          catalog = VCAP::Services::ServiceBrokers::V2::Catalog.new(broker, broker_client.catalog)
+
+          raise fail_with_invalid_catalog(catalog.validation_errors) unless catalog.valid?
+          raise fail_with_incompatible_catalog(catalog.incompatibility_errors) unless catalog.compatible?
+
+          unless client_manager.synchronize_clients_with_catalog(catalog)
+            # TODO: raise some humanized exceptions if it failed
+          end
+
+          service_manager.sync_services_and_plans(catalog)
+
+          # TODO: if service_manager.has_warnings?
+          # TODO: if client_manager.has_warnings?
+
+          available_state
+        rescue
+          failed_state
+          raise
+        end
+
+        private
+
+        attr_reader :broker_guid, :broker, :broker_client,
+            :formatter, :client_manager, :service_event_repository,
+            :service_manager
+
+        def ensure_state_present
+          if broker.service_broker_state.nil?
+            broker.service_broker_state = ServiceBrokerState.new(
+              state: ServiceBrokerStateEnum::SYNCHRONIZING
+            )
+          end
+        end
+
+        def failed_state
+          broker.service_broker_state.update(
+            state: ServiceBrokerStateEnum::SYNCHRONIZATION_FAILED
+          )
+        end
+
+        def available_state
+          broker.service_broker_state.update(
+            state: ServiceBrokerStateEnum::AVAILABLE
+          )
+        end
+
+        def fail_with_invalid_catalog(errors)
+          full_message = formatter.format(errors)
+          raise CloudController::Errors::ApiError.new_from_details('ServiceBrokerCatalogInvalid', full_message)
+        end
+
+        def fail_with_incompatible_catalog(errors)
+          full_message = formatter.format(errors)
+          raise CloudController::Errors::ApiError.new_from_details('ServiceBrokerCatalogIncompatible', full_message)
+        end
+      end
+    end
+  end
+end

app/models.rb

@@ -110,6 +110,8 @@
 require 'models/services/service_binding_operation'
 require 'models/services/user_provided_service_instance'
 require 'models/services/service_broker'
+require 'models/services/service_broker_state'
+require 'models/services/service_broker_state_enum'
 require 'models/services/service_plan'
 require 'models/services/service_plan_visibility'
 require 'models/services/service_usage_event'

app/models/services/service_broker.rb

@@ -3,6 +3,7 @@ class ServiceBroker < Sequel::Model
     one_to_many :services
     one_to_many :service_dashboard_client
     many_to_one :space
+    one_to_one :service_broker_state
 
     import_attributes :name, :broker_url, :auth_username, :auth_password
     export_attributes :name, :broker_url, :auth_username, :space_guid

app/models/services/service_broker_state.rb

@@ -0,0 +1,6 @@
+module VCAP::CloudController
+  class ServiceBrokerState < Sequel::Model
+    import_attributes :state, :service_broker_guid
+    export_attributes :guid, :state, :service_broker_guid
+  end
+end

app/models/services/service_broker_state_enum.rb

@@ -0,0 +1,7 @@
+module VCAP::CloudController
+  class ServiceBrokerStateEnum
+    SYNCHRONIZING = 'SYNCHRONIZING'.freeze
+    SYNCHRONIZATION_FAILED = 'SYNCHRONIZATION_FAILED'.freeze
+    AVAILABLE = 'AVAILABLE'.freeze
+  end
+end

app/presenters/v3/base_presenter.rb

@@ -1,3 +1,5 @@
+require 'presenters/helpers/censorship'
+
 module VCAP::CloudController
   module Presenters
     module V3

app/presenters/v3/service_broker_presenter.rb

@@ -1,6 +1,8 @@
 require 'presenters/v3/base_presenter'
 require 'models/helpers/metadata_helpers'
 require 'presenters/mixins/metadata_presentation_helpers'
+require 'models/services/service_broker_state_enum'
+require 'presenters/api_url_builder'
 
 module VCAP::CloudController
   module Presenters
@@ -11,6 +13,8 @@ def to_hash
             guid: broker.guid,
             name: broker.name,
             url: broker.broker_url,
+            available: status == 'available',
+            status: status,
             created_at: broker.created_at,
             updated_at: broker.updated_at,
             relationships: build_relationships,
@@ -24,6 +28,24 @@ def broker
           @resource
         end
 
+        def status
+          state = broker.service_broker_state&.state
+
+          if state == VCAP::CloudController::ServiceBrokerStateEnum::SYNCHRONIZING
+            return 'synchronization in progress'
+          end
+
+          if state == VCAP::CloudController::ServiceBrokerStateEnum::SYNCHRONIZATION_FAILED
+            return 'synchronization failed'
+          end
+
+          if state == VCAP::CloudController::ServiceBrokerStateEnum::AVAILABLE
+            return 'available'
+          end
+
+          'unknown'
+        end
+
         def build_relationships
           if broker.space_guid.nil?
             {}

app/repositories/service_event_repository.rb

@@ -169,7 +169,7 @@ def with_parameters_redacted(params)
 
         def metadata_for_broker_params(params)
           request_hash = {}
-          [:name, :broker_url, :auth_username].each do |key|
+          [:name, :broker_url, :auth_username, :space_guid].each do |key|
             request_hash[key] = params[key] unless params[key].nil?
           end
           request_hash[:auth_password] = Presenters::Censorship::REDACTED if params.key? :auth_password

db/migrations/20190905131313_create_service_broker_states.rb

@@ -0,0 +1,13 @@
+Sequel.migration do
+  change do
+    create_table(:service_broker_states) do
+      VCAP::Migration.common(self)
+
+      String :state, size: 50, null: false
+
+      Integer :service_broker_id, null: false
+      foreign_key :service_broker_id, :service_brokers, name: :fk_service_brokers_id
+      index [:service_broker_id], name: :fk_service_brokers_id_index
+    end
+  end
+end

lib/cloud_controller/diego/buildpack/staging_action_builder.rb

@@ -1,4 +1,5 @@
 require 'credhub/config_helpers'
+require 'diego/action_builder'
 
 module VCAP::CloudController
   module Diego

lib/services/service_brokers/v2/catalog.rb

@@ -1,12 +1,15 @@
 module VCAP::Services::ServiceBrokers::V2
   class Catalog
-    attr_reader :service_broker, :services, :plans, :errors
+    attr_reader :service_broker, :services, :plans, :errors, :incompatibility_errors
+
+    alias_method :validation_errors, :errors
 
     def initialize(service_broker, catalog_hash)
       @service_broker = service_broker
-      @services       = []
-      @plans          = []
-      @errors         = VCAP::Services::ValidationErrors.new
+      @services = []
+      @plans = []
+      @errors = VCAP::Services::ValidationErrors.new
+      @incompatibility_errors = VCAP::Services::ValidationErrors.new
 
       catalog_hash.fetch('services', []).each do |service_attrs|
         service = CatalogService.new(service_broker, service_attrs)
@@ -20,7 +23,20 @@ def valid?
       validate_all_service_ids_are_unique
       validate_all_service_names_are_unique
       validate_all_service_dashboard_clients_are_unique
-      errors.empty?
+      validation_errors.empty?
+    end
+
+    def compatible?
+      services.each { |service|
+        if service.route_service? && route_services_disabled?
+          incompatibility_errors.add("Service #{service.name} is declared to be a route service but support for route services is disabled.")
+        end
+
+        if service.volume_mount_service? && volume_services_disabled?
+          incompatibility_errors.add("Service #{service.name} is declared to be a volume mount service but support for volume mount services is disabled.")
+        end
+      }
+      incompatibility_errors.empty?
     end
 
     private
@@ -29,7 +45,7 @@ def validate_all_service_dashboard_clients_are_unique
       dashboard_clients = valid_dashboard_clients(services)
       dashboard_client_ids = valid_dashboard_client_ids(dashboard_clients)
       if has_duplicates?(dashboard_client_ids)
-        errors.add('Service dashboard_client id must be unique')
+        validation_errors.add('Service dashboard_client id must be unique')
       end
     end
 
@@ -43,13 +59,13 @@ def valid_dashboard_clients(services)
 
     def validate_all_service_ids_are_unique
       if has_duplicates?(services.map(&:broker_provided_id).compact)
-        errors.add('Service ids must be unique')
+        validation_errors.add('Service ids must be unique')
       end
     end
 
     def validate_all_service_names_are_unique
       if has_duplicates?(services.map(&:name))
-        errors.add('Service names must be unique within a broker')
+        validation_errors.add('Service names must be unique within a broker')
       end
     end
 
@@ -59,10 +75,18 @@ def has_duplicates?(array)
 
     def validate_services
       services.each do |service|
-        errors.add_nested(service, service.errors) unless service.valid?
+        validation_errors.add_nested(service, service.errors) unless service.valid?
       end
 
-      errors.add('Service broker must provide at least one service') if services.empty?
+      validation_errors.add('Service broker must provide at least one service') if services.empty?
+    end
+
+    def volume_services_disabled?
+      !VCAP::CloudController::Config.config.get(:volume_services_enabled)
+    end
+
+    def route_services_disabled?
+      !VCAP::CloudController::Config.config.get(:route_services_enabled)
     end
   end
 end