Merge pull request cloudfoundry#1984 from cloudfoundry/fix-service-key-k8s-175562836

make credential_client_id optional when creating service keys

[#175562836](https://www.pivotaltracker.com/story/show/175562836)

Author: tcdowney

lib/cloud_controller/config_schemas/api_schema.rb

@@ -361,11 +361,10 @@ class ApiSchema < VCAP::Config
               registry_tag_base: String,
             }
           },
+          optional(:cc_service_key_client_name) => String,
+          optional(:cc_service_key_client_secret) => String,
 
           **VCAP::Config::Dsl.omit_on_k8s(
-            cc_service_key_client_name: String,
-            cc_service_key_client_secret: String,
-
             diego: {
               bbs: {
                 url: String,

lib/services/service_brokers/v2/client.rb

@@ -71,10 +71,11 @@ def create_service_key(key, arbitrary_parameters: {})
       body              = {
         service_id:    key.service.broker_provided_id,
         plan_id:       key.service_plan.broker_provided_id,
-        bind_resource: { credential_client_id: @config.get(:cc_service_key_client_name) },
+        bind_resource: {},
         context:       context_hash(key.service_instance),
       }
 
+      body[:bind_resource][:credential_client_id] = @config.get(:cc_service_key_client_name) if @config.get(:cc_service_key_client_name).present?
       body[:parameters] = arbitrary_parameters if arbitrary_parameters.present?
 
       begin
@@ -95,12 +96,11 @@ def create_service_key(key, arbitrary_parameters: {})
 
     def bind(binding, arbitrary_parameters: {}, accepts_incomplete: false)
       path = service_binding_resource_path(binding.guid, binding.service_instance.guid, accepts_incomplete: accepts_incomplete)
-      key_required_parameters = { credential_client_id: @config.get(:cc_service_key_client_name) } if binding.is_a?(VCAP::CloudController::ServiceKey)
       body = {
         service_id:    binding.service.broker_provided_id,
         plan_id:       binding.service_plan.broker_provided_id,
         app_guid:      binding.try(:app_guid),
-        bind_resource: binding.required_parameters || key_required_parameters,
+        bind_resource: binding_required_parameters(binding),
         context:       context_hash(binding.service_instance)
       }
       body = body.reject { |_, v| v.nil? }
@@ -319,6 +319,14 @@ def fetch_service_binding(service_binding)
 
     private
 
+    def binding_required_parameters(binding)
+      if binding.is_a?(VCAP::CloudController::ServiceKey) && @config.get(:cc_service_key_client_name).present?
+        service_key_parameters = { credential_client_id: @config.get(:cc_service_key_client_name) }
+      end
+
+      binding.required_parameters || service_key_parameters || {}
+    end
+
     def context_hash_with_instance_name(service_instance, name: service_instance.name)
       context_hash(service_instance).merge(instance_name: name)
     end

spec/unit/lib/services/service_brokers/v2/client_spec.rb

@@ -976,6 +976,32 @@ module VCAP::Services::ServiceBrokers::V2
           )
       end
 
+      context 'when cc_service_key_client is configured' do
+        it 'includes the optional credential_client_id parameter' do
+          client.create_service_key(key)
+
+          expect(http_client).to have_received(:put).
+            with(anything,
+              hash_including({ bind_resource: { credential_client_id: cc_service_key_client_name } })
+            )
+        end
+      end
+
+      context 'when cc_service_key_client is NOT present' do
+        before do
+          TestConfig.override(cc_service_key_client_name: nil)
+        end
+
+        it 'does NOT include the optional credential_client_id parameter' do
+          client.create_service_key(key)
+
+          expect(http_client).to have_received(:put).
+            with(anything,
+              hash_excluding({ bind_resource: { credential_client_id: anything } })
+            )
+        end
+      end
+
       it 'sets the credentials on the key' do
         attributes = client.create_service_key(key)
         key.set(attributes)
@@ -1224,13 +1250,30 @@ module VCAP::Services::ServiceBrokers::V2
       context 'when the binding is of type key' do
         let(:binding) { VCAP::CloudController::ServiceKey.make }
 
-        it 'sends the right bind_resource' do
-          client.bind(binding)
+        context 'when cc_service_key_client is configured' do
+          it 'includes the optional credential_client_id parameter' do
+            client.bind(binding)
 
-          expect(http_client).to have_received(:put).
-            with(anything,
-              hash_including({ bind_resource: { credential_client_id: 'cc_service_key_client' } })
-            )
+            expect(http_client).to have_received(:put).
+              with(anything,
+                hash_including({ bind_resource: { credential_client_id: 'cc_service_key_client' } })
+              )
+          end
+        end
+
+        context 'when cc_service_key_client is NOT present' do
+          before do
+            TestConfig.override(cc_service_key_client_name: nil)
+          end
+
+          it 'does NOT include the optional credential_client_id parameter' do
+            client.bind(binding)
+
+            expect(http_client).to have_received(:put).
+              with(anything,
+                hash_excluding({ bind_resource: { credential_client_id: anything } })
+              )
+          end
         end
 
         it 'does not send the app_guid in the request' do