v3(services): create service key binding

* v3(services): Add operations table for service keys [#174147679](https://www.pivotaltracker.com/story/show/174147679) * v3(services): add service key binding actor [#174147679](https://www.pivotaltracker.com/story/show/174147679) * v3(services): add last operation to service keys [#174147679](https://www.pivotaltracker.com/story/show/174147679) * v3(services): create key action binds and polls last operation [#174147679](https://www.pivotaltracker.com/story/show/174147679) * v3(services): service key controller and client implementation This commit reuses the same bind method in the client for service keys, service credential and service route bindings. Before this, service keys had their own create_service_key client method, which is still available for v2 [#174147679](https://www.pivotaltracker.com/story/show/174147679) * v3(services): configure resource urls and logging keys used for service key bindings [#174147679](https://www.pivotaltracker.com/story/show/174147679) * v3(tests): extract common tests for ServiceBindings and ServiceKey creation [#174147679](https://www.pivotaltracker.com/story/show/174147679) * v3(style): fix rubocop issues [#174147679](https://www.pivotaltracker.com/story/show/174147679) * v3(tests): fix service key bindings tests [#174147679](https://www.pivotaltracker.com/story/show/174147679) * v3(services): refactor service bindings creation [#174147679](https://www.pivotaltracker.com/story/show/174147679) * v3(services): async key bindings tests [#174163869](https://www.pivotaltracker.com/story/show/174163869) * v3(services): create key changes from PR [#174163869](https://www.pivotaltracker.com/story/show/174163869) * v3(services): Reorder signature for enqueue_bind_job Co-authored-by: Felisia Martini <[email protected]>
univ0298 · Dec 2, 2020 · 4dc219d · 4dc219d
1 parent e8e660f
commit 4dc219d
Show file tree

Hide file tree

Showing 33 changed files with 1,213 additions and 712 deletions.
diff --git a/app/actions/service_credential_binding_app_create.rb b/app/actions/service_credential_binding_app_create.rb
@@ -45,30 +45,6 @@ def precursor(service_instance, app: nil, name: nil, volume_mount_services_enabl
 
       private
 
-      def complete_binding_and_save(binding, binding_details, last_operation)
-        binding.save_with_attributes_and_new_operation(
-          binding_details.symbolize_keys.slice(*PERMITTED_BINDING_ATTRIBUTES),
-          {
-            type: 'create',
-            state: last_operation[:state],
-            description: last_operation[:description]
-          }
-        )
-        event_repository.record_create(binding, @user_audit_info, @audit_hash, manifest_triggered: false)
-      end
-
-      def save_incomplete_binding(binding, broker_operation)
-        binding.save_with_attributes_and_new_operation(
-          {},
-          {
-            type: 'create',
-            state: 'in progress',
-            broker_provided_operation: broker_operation
-          }
-        )
-        event_repository.record_start_create(binding, @user_audit_info, @audit_hash, manifest_triggered: false)
-      end
-
       def validate!(service_instance, app, volume_mount_services_enabled)
         app_is_required! unless app.present?
         space_mismatch! unless all_space_guids(service_instance).include? app.space.guid
@@ -81,6 +57,10 @@ def validate!(service_instance, app, volume_mount_services_enabled)
         end
       end
 
+      def permitted_binding_attributes
+        PERMITTED_BINDING_ATTRIBUTES
+      end
+
       def all_space_guids(service_instance)
         (service_instance.shared_spaces + [service_instance.space]).map(&:guid)
       end

diff --git a/app/actions/service_credential_binding_key_create.rb b/app/actions/service_credential_binding_key_create.rb
@@ -2,12 +2,20 @@
 
 module VCAP::CloudController
   module V3
-    class ServiceCredentialBindingKeyCreate
+    class ServiceCredentialBindingKeyCreate < V3::ServiceBindingCreate
       include ServiceCredentialBindingCreateMixin
 
       class UnprocessableCreate < StandardError
       end
 
+      def initialize(user_audit_info, audit_hash)
+        super()
+        @user_audit_info = user_audit_info
+        @audit_hash = audit_hash
+      end
+
+      PERMITTED_BINDING_ATTRIBUTES = [:credentials].freeze
+
       def precursor(service_instance, name)
         validate!(service_instance)
 
@@ -17,8 +25,13 @@ def precursor(service_instance, name)
           credentials: {}
         }
 
-        ServiceKey.db.transaction do
-          ServiceKey.create(**binding_details)
+        ServiceKey.new(**binding_details).tap do |b|
+          b.save_with_new_operation(
+            {
+              type: 'create',
+              state: 'in progress',
+            }
+          )
         end
       rescue Sequel::ValidationFailed => e
         key_validation_error!(
@@ -40,6 +53,15 @@ def validate!(service_instance)
         end
       end
 
+      def permitted_binding_attributes
+        PERMITTED_BINDING_ATTRIBUTES
+      end
+
+      def event_repository
+        @event_repository ||= Repositories::ServiceGenericBindingEventRepository.new(
+          Repositories::ServiceGenericBindingEventRepository::SERVICE_KEY_CREDENTIAL_BINDING)
+      end
+
       def key_not_supported_for_user_provided_service!
         raise UnprocessableCreate.new("Service credential bindings of type 'key' are not supported for user-provided service instances.")
       end

diff --git a/app/actions/service_route_binding_create.rb b/app/actions/service_route_binding_create.rb
@@ -11,6 +11,8 @@ def initialize(user_audit_info, audit_hash)
         @audit_hash = audit_hash
       end
 
+      PERMITTED_BINDING_ATTRIBUTES = [:route_service_url].freeze
+
       def precursor(service_instance, route, message:)
         validate!(service_instance, route)
 
@@ -51,44 +53,12 @@ def validate!(service_instance, route)
         operation_in_progress! if service_instance.operation_in_progress?
       end
 
-      def complete_binding_and_save(binding, binding_details, last_operation)
-        binding.save_with_attributes_and_new_operation(
-          {
-            route_service_url: binding_details[:route_service_url]
-          },
-          {
-            type: 'create',
-            state: last_operation[:state],
-            description: last_operation[:description],
-          }
-        )
-
-        binding.notify_diego
-
-        event_repository.record_create(
-          binding,
-          @user_audit_info,
-          @audit_hash,
-          manifest_triggered: false
-        )
+      def permitted_binding_attributes
+        PERMITTED_BINDING_ATTRIBUTES
       end
 
-      def save_incomplete_binding(precursor, operation)
-        precursor.save_with_attributes_and_new_operation(
-          {},
-          {
-            type: 'create',
-            state: 'in progress',
-            broker_provided_operation: operation
-          }
-        )
-
-        event_repository.record_start_create(
-          precursor,
-          @user_audit_info,
-          @audit_hash,
-          manifest_triggered: false
-        )
+      def post_bind_action(binding)
+        binding.notify_diego
       end
 
       def operation_in_progress!

diff --git a/app/actions/v3/service_binding_create.rb b/app/actions/v3/service_binding_create.rb
@@ -64,6 +64,25 @@ def save_failed_state(binding, e)
         )
       end
 
+      def complete_binding_and_save(binding, binding_details, last_operation)
+        binding.save_with_attributes_and_new_operation(
+          binding_details.symbolize_keys.slice(*permitted_binding_attributes),
+          {
+            type: 'create',
+            state: last_operation[:state],
+            description: last_operation[:description],
+          }
+        )
+
+        post_bind_action(binding)
+
+        event_repository.record_create(
+          binding,
+          @user_audit_info,
+          @audit_hash
+        )
+      end
+
       def save_last_operation(binding, details)
         binding.save_with_attributes_and_new_operation(
           {},
@@ -76,6 +95,20 @@ def save_last_operation(binding, details)
         )
       end
 
+      def save_incomplete_binding(binding, broker_operation)
+        binding.save_with_attributes_and_new_operation(
+          {},
+          {
+            type: 'create',
+            state: 'in progress',
+            broker_provided_operation: broker_operation
+          }
+        )
+        event_repository.record_start_create(binding, @user_audit_info, @audit_hash)
+      end
+
+      def post_bind_action(binding); end
+
       def bindings_retrievable?(binding)
         binding.service_instance.service.bindings_retrievable
       end

diff --git a/app/controllers/v3/service_credential_bindings_controller.rb b/app/controllers/v3/service_credential_bindings_controller.rb
@@ -120,13 +120,11 @@ def parameters
   private
 
   def create_key_binding(message, service_instance)
-    V3::ServiceCredentialBindingKeyCreate.new.precursor(
-      service_instance,
-      message.name
-    )
+    action = V3::ServiceCredentialBindingKeyCreate.new(user_audit_info, message.audit_hash)
+    binding = action.precursor(service_instance, message.name)
 
-    head :not_implemented
-    return
+    pollable_job_guid = enqueue_bind_job(:key, binding.guid, message)
+    head :accepted, 'Location' => url_builder.build_url(path: "/v3/jobs/#{pollable_job_guid}")
   end
 
   def build_create_message(params)
@@ -161,17 +159,17 @@ def create_app_binding(message, service_instance, app)
 
     case service_instance
     when ManagedServiceInstance
-      pollable_job_guid = enqueue_bind_job(binding.guid, message)
+      pollable_job_guid = enqueue_bind_job(:credential, binding.guid, message)
       head :accepted, 'Location' => url_builder.build_url(path: "/v3/jobs/#{pollable_job_guid}")
     when UserProvidedServiceInstance
       action.bind(binding)
       render status: :created, json: Presenters::V3::ServiceCredentialBindingPresenter.new(binding).to_hash
     end
   end
 
-  def enqueue_bind_job(binding_guid, message)
+  def enqueue_bind_job(type, binding_guid, message)
     bind_job = VCAP::CloudController::V3::CreateBindingAsyncJob.new(
-      :credential,
+      type,
       binding_guid,
       user_audit_info: user_audit_info,
       audit_hash: message.audit_hash,

diff --git a/app/jobs/v3/create_service_binding_job_factory.rb b/app/jobs/v3/create_service_binding_job_factory.rb
@@ -1,5 +1,9 @@
 require 'jobs/v3/create_service_route_binding_job_actor'
 require 'jobs/v3/create_service_credential_binding_job_actor'
+require 'jobs/v3/create_service_key_binding_job_actor'
+require 'actions/service_credential_binding_app_create'
+require 'actions/service_credential_binding_key_create'
+require 'actions/service_route_binding_create'
 
 module VCAP::CloudController
   module V3
@@ -13,6 +17,8 @@ def self.for(type)
           CreateServiceRouteBindingJobActor.new
         when :credential
           CreateServiceCredentialBindingJobActor.new
+        when :key
+          CreateServiceKeyBindingJobActor.new
         else
           raise InvalidType
         end
@@ -24,6 +30,8 @@ def self.action(type, user_audit_info, audit_hash)
           V3::ServiceRouteBindingCreate.new(user_audit_info, audit_hash)
         when :credential
           V3::ServiceCredentialBindingAppCreate.new(user_audit_info, audit_hash)
+        when :key
+          V3::ServiceCredentialBindingKeyCreate.new(user_audit_info, audit_hash)
         else
           raise InvalidType
         end

diff --git a/app/jobs/v3/create_service_credential_binding_job_actor.rb b/app/jobs/v3/create_service_credential_binding_job_actor.rb
@@ -1,7 +1,3 @@
-require 'jobs/reoccurring_job'
-require 'actions/service_credential_binding_app_create'
-require 'cloud_controller/errors/api_error'
-
 module VCAP::CloudController
   module V3
     class CreateServiceCredentialBindingJobActor

diff --git a/app/jobs/v3/create_service_key_binding_job_actor.rb b/app/jobs/v3/create_service_key_binding_job_actor.rb
@@ -0,0 +1,17 @@
+module VCAP::CloudController
+  module V3
+    class CreateServiceKeyBindingJobActor
+      def display_name
+        'service_keys.create'
+      end
+
+      def resource_type
+        'service_credential_binding'
+      end
+
+      def get_resource(resource_id)
+        ServiceKey.first(guid: resource_id)
+      end
+    end
+  end
+end
diff --git a/app/models.rb b/app/models.rb
@@ -132,6 +132,7 @@
 require 'models/services/service_plan_label_model'
 require 'models/services/service_usage_event'
 require 'models/services/service_key'
+require 'models/services/service_key_operation'
 require 'models/services/service_credential_binding_view'
 
 require 'models/request_count'

diff --git a/app/models/runtime/pollable_job_model.rb b/app/models/runtime/pollable_job_model.rb
@@ -27,7 +27,7 @@ def resource_exists?
               when 'service_route_binding'
                 RouteBinding
               when 'service_credential_binding'
-                ServiceBinding
+                ServiceCredentialBinding::View
               else
                 Sequel::Model(ActiveSupport::Inflector.pluralize(resource_type).to_sym)
               end

diff --git a/app/models/services/service_binding.rb b/app/models/services/service_binding.rb
@@ -133,15 +133,11 @@ def is_created?
     end
 
     def terminal_state?
-      !service_binding_operation || (['succeeded', 'failed'].include? service_binding_operation.state)
+      !service_binding_operation || (%w(succeeded failed).include? service_binding_operation.state)
     end
 
     def operation_in_progress?
-      if service_binding_operation && service_binding_operation.state == 'in progress'
-        return true
-      end
-
-      false
+      !!service_binding_operation && service_binding_operation.state == 'in progress'
     end
 
     def save_with_attributes_and_new_operation(attributes, operation)

diff --git a/app/models/services/service_key.rb b/app/models/services/service_key.rb
@@ -4,6 +4,8 @@ class InvalidAppAndServiceRelation < StandardError; end
 
     many_to_one :service_instance
 
+    one_to_one :service_key_operation
+
     export_attributes :name, :service_instance_guid, :credentials
 
     import_attributes :name, :service_instance_guid, :credentials
@@ -79,8 +81,44 @@ def logger
       @logger ||= Steno.logger('cc.models.service_key')
     end
 
+    def last_operation
+      service_key_operation
+    end
+
+    def terminal_state?
+      !service_key_operation || (%w(succeeded failed).include? service_key_operation.state)
+    end
+
+    # TODO: can these methods be a mixin somehow
     def operation_in_progress?
-      false
+      !!service_key_operation && service_key_operation.state == 'in progress'
+    end
+
+    def required_parameters
+      nil
+    end
+
+    def save_with_attributes_and_new_operation(attributes, operation)
+      save_with_new_operation(operation, attributes: attributes)
+      self
+    end
+
+    def save_with_new_operation(last_operation, attributes: {})
+      ServiceKey.db.transaction do
+        self.lock!
+        set(attributes.except(:parameters, :route_services_url, :endpoints))
+        save_changes
+
+        if self.last_operation
+          self.last_operation.destroy
+        end
+
+        # it is important to create the service key operation with the service key
+        # instead of doing self.service_key_operation = x
+        # because mysql will deadlock when requests happen concurrently otherwise.
+        ServiceKeyOperation.create(last_operation.merge(service_key_id: self.id))
+        self.service_key_operation(reload: true)
+      end
     end
   end
 end
diff --git a/app/models/services/service_key_operation.rb b/app/models/services/service_key_operation.rb
@@ -0,0 +1,10 @@
+module VCAP::CloudController
+  class ServiceKeyOperation < Sequel::Model
+    export_attributes :state, :description, :type, :updated_at, :created_at
+
+    def update_attributes(attrs)
+      self.set attrs
+      self.save
+    end
+  end
+end