Added validation of unique name for service key

Author: FelisiaM

app/actions/service_credential_binding_key_create.rb

@@ -4,19 +4,35 @@ class ServiceCredentialBindingKeyCreate
       class UnprocessableCreate < StandardError
       end
 
-      def precursor(service_instance, volume_mount_services_enabled: false)
+      def precursor(service_instance, name)
+        validate!(service_instance)
+
+        binding_details = {
+          service_instance: service_instance,
+          name: name,
+          credentials: {}
+        }
+
+        ServiceKey.db.transaction do
+          ServiceKey.create(**binding_details)
+        end
+      rescue Sequel::ValidationFailed => e
+        raise UnprocessableCreate.new(e.message)
+      end
+
+      private
+
+      def validate!(service_instance)
+
         if service_instance.managed_instance?
           service_not_bindable! unless service_instance.service_plan.bindable?
           service_not_available! unless service_instance.service_plan.active?
-          volume_mount_not_enabled! if service_instance.volume_service? && !volume_mount_services_enabled
           operation_in_progress! if service_instance.operation_in_progress?
         else
           key_not_supported_for_user_provided_service!
         end
       end
 
-      private
-
       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

app/controllers/v3/service_credential_bindings_controller.rb

@@ -51,6 +51,7 @@ def create
     case message.type
     when 'app'
       app = VCAP::CloudController::AppModel.first(guid: message.app_guid)
+      # TODO create two unprocessable_app/si methods
       resource_not_accessible!('app', message.app_guid) unless can_access_resource?(app)
 
       unauthorized! unless can_write_to_space?(app.space)
@@ -71,7 +72,7 @@ def create
 
       V3::ServiceCredentialBindingKeyCreate.new.precursor(
         service_instance,
-        volume_mount_services_enabled: volume_services_enabled?
+        message.name
       )
 
       head :not_implemented

app/models/services/service_key.rb

@@ -33,7 +33,7 @@ def space
     def validate
       validates_presence :name
       validates_presence :service_instance
-      validates_unique [:name, :service_instance_id]
+      validates_unique [:name, :service_instance_id], message: Sequel.lit("The binding name is invalid. Key binding names must be unique. The service instance already has a key binding with name '#{name}'.")
 
       if service_instance
         MaxServiceKeysPolicy.new(

spec/request/service_credential_bindings_spec.rb

@@ -879,7 +879,9 @@ def check_filtered_bindings(*bindings)
         it 'returns 422 when the binding already exists' do
           api_call.call admin_headers
           expect(last_response.status).to eq(201).or eq(202)
+
           api_call.call admin_headers
+
           expect(last_response).to have_status_code(422)
           expect(parsed_response['errors']).to include(include({
             'detail' => include('The app is already bound to the service instance'),
@@ -1474,10 +1476,11 @@ def check_filtered_bindings(*bindings)
 
     context 'creating a credential binding as a key' do
       let(:service_instance) { VCAP::CloudController::ManagedServiceInstance.make(space: space, **service_instance_details) }
+      let(:binding_name) { 'some-key-name' }
       let(:create_body) {
         {
           type: 'key',
-          name: 'some-key-name',
+          name: binding_name,
           relationships: {
             service_instance: { data: { guid: service_instance_guid } },
           }
@@ -1580,6 +1583,32 @@ def check_filtered_bindings(*bindings)
           end
         end
 
+        context 'when a key with the same name exists for that SI' do
+          let(:service_cred_binding) {
+            VCAP::CloudController::ServiceKey.make(service_instance: service_instance)
+          }
+          let(:create_body) {
+            {
+              type: 'key',
+              name: service_cred_binding.name,
+              relationships: {
+                service_instance: { data: { guid: service_instance_guid } },
+              }
+            }.merge(request_extra)
+          }
+
+          it 'returns a 422' do
+            api_call.call space_dev_headers
+
+            expect(last_response).to have_status_code(422)
+            expect(parsed_response['errors']).to include(include({
+              'detail' => include("The binding name is invalid. Key binding names must be unique. The service instance already has a key binding with name '#{service_cred_binding.name}'."),
+              'title' => 'CF-UnprocessableEntity',
+              'code' => 10008,
+            }))
+          end
+        end
+
         context 'when the service instance is user-provided' do
           let(:service_instance) { VCAP::CloudController::UserProvidedServiceInstance.make(space: space, **service_instance_details) }