Skip to content

Commit

Permalink
Add builder for service binding files
Browse files Browse the repository at this point in the history 
- validate binding names and (credential) keys
- check for duplicate binding names
- check the total bytesize, maximum allowed size is 1MB
- files are added in the following order:
   1. credential keys
   2. VCAP_SERVICES attributes
   3. 'type' and 'provider'
- in case a credential key equals a VCAP_SERVICES attributes or 'type'
  or 'provider', it will be overwritten
- file content is serialized as JSON (non-string objects)
  • Loading branch information
@philippthun
philippthun committed Oct 18, 2024
1 parent bfa798b commit 4879337
Show file tree
Hide file tree
Showing 2 changed files with 343 additions and 0 deletions.
70 changes: 70 additions & 0 deletions lib/cloud_controller/diego/service_binding_files_builder.rb
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
module VCAP::CloudController
module Diego
class ServiceBindingFilesBuilder
class IncompatibleBindings < StandardError; end

MAX_ALLOWED_BYTESIZE = 1_000_000

def self.build(app_or_process)
new(app_or_process).build
end

def initialize(app_or_process)
@service_bindings = app_or_process.service_bindings
end

def build
service_binding_files = {}
names = Set.new # to check for duplicate binding names
total_bytesize = 0 # to check the total bytesize

@service_bindings.select(&:create_succeeded?).each do |service_binding|
sb_hash = ServiceBindingPresenter.new(service_binding, include_instance: true).to_hash
name = sb_hash[:name]
raise IncompatibleBindings.new("Invalid binding name: #{name}") unless name.match?(/^[a-z0-9\-.]{1,253}$/)
raise IncompatibleBindings.new("Duplicate binding name: #{name}") if names.add?(name).nil?

# add the credentials first
sb_hash.delete(:credentials)&.each { |k, v| total_bytesize += add_file(service_binding_files, name, k, v) }

# add the rest of the hash; already existing credential keys are overwritten
sb_hash.each { |k, v| total_bytesize += add_file(service_binding_files, name, k, v) }

# add the type and provider
label = sb_hash[:label]
total_bytesize += add_file(service_binding_files, name, 'type', label)
total_bytesize += add_file(service_binding_files, name, 'provider', label)
end

raise IncompatibleBindings.new("Bindings exceed the maximum allowed bytesize of #{MAX_ALLOWED_BYTESIZE}: #{total_bytesize}") if total_bytesize > MAX_ALLOWED_BYTESIZE

service_binding_files.values
end

private

# - adds a Diego::Bbs::Models::Files object to the service_binding_files hash
# - binding name is used as the directory name, key is used as the file name
# - returns the bytesize of the path and content
# - skips (and returns 0) if the value is nil or an empty array or hash
# - serializes the value to JSON if it is a non-string object
def add_file(service_binding_files, name, key, value)
raise IncompatibleBindings.new("Invalid file name: #{key}") unless key.match?(/^[a-z0-9\-._]{1,253}$/)

path = "#{name}/#{key}"
content = if value.nil?
return 0
elsif value.is_a?(String)
value
else
return 0 if (value.is_a?(Array) || value.is_a?(Hash)) && value.empty?

Oj.dump(value, mode: :compat)
end

service_binding_files[path] = ::Diego::Bbs::Models::Files.new(name: path, value: content)
path.bytesize + content.bytesize
end
end
end
end
273 changes: 273 additions & 0 deletions spec/unit/lib/cloud_controller/diego/service_binding_files_builder_spec.rb
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,273 @@
require 'spec_helper'
require 'cloud_controller/diego/service_binding_files_builder'

module VCAP::CloudController::Diego
RSpec.shared_examples 'mapping of type and provider' do |label|
it 'sets type and provider to the service label' do
expect(service_binding_files.find { |f| f.name == "#{directory}/type" }).to have_attributes(value: label || 'service-name')
expect(service_binding_files.find { |f| f.name == "#{directory}/provider" }).to have_attributes(value: label || 'service-name')
expect(service_binding_files.find { |f| f.name == "#{directory}/label" }).to have_attributes(value: label || 'service-name')
end
end

RSpec.shared_examples 'mapping of binding metadata' do |name|
it 'maps service binding metadata attributes to files' do
expect(service_binding_files.find { |f| f.name == "#{directory}/binding_guid" }).to have_attributes(value: binding.guid)
expect(service_binding_files.find { |f| f.name == "#{directory}/name" }).to have_attributes(value: name || 'binding-name')
expect(service_binding_files.find { |f| f.name == "#{directory}/binding_name" }).to have_attributes(value: 'binding-name') if name.nil?
end
end

RSpec.shared_examples 'mapping of instance metadata' do |instance_name|
it 'maps service instance metadata attributes to files' do
expect(service_binding_files.find { |f| f.name == "#{directory}/instance_guid" }).to have_attributes(value: instance.guid)
expect(service_binding_files.find { |f| f.name == "#{directory}/instance_name" }).to have_attributes(value: instance_name || 'instance-name')
end
end

RSpec.shared_examples 'mapping of plan metadata' do
it 'maps service plan metadata attributes to files' do
expect(service_binding_files.find { |f| f.name == "#{directory}/plan" }).to have_attributes(value: 'plan-name')
end
end

RSpec.shared_examples 'mapping of tags' do |tags|
it 'maps (service tags merged with) instance tags to a file' do
expect(service_binding_files.find do |f|
f.name == "#{directory}/tags"
end).to have_attributes(value: tags || '["a-service-tag","another-service-tag","an-instance-tag","another-instance-tag"]')
end
end

RSpec.shared_examples 'mapping of credentials' do |credential_files|
it 'maps service binding credentials to individual files' do
expected_credential_files = credential_files || {
string: 'a string',
number: '42',
boolean: 'true',
array: '["one","two","three"]',
hash: '{"key":"value"}'
}
expected_credential_files.each do |name, content|
expect(service_binding_files.find { |f| f.name == "#{directory}/#{name}" }).to have_attributes(value: content)
end
end
end

RSpec.shared_examples 'expected files' do |files|
it 'does not include other files' do
other_files = service_binding_files.reject do |file|
match = file.name.match(%r{^#{directory}/(.+)$})
!match.nil? && !files.delete(match[1]).nil?
end

expect(files).to be_empty
expect(other_files).to be_empty
end
end

RSpec.describe ServiceBindingFilesBuilder do
let(:service) { VCAP::CloudController::Service.make(label: 'service-name', tags: %w[a-service-tag another-service-tag]) }
let(:plan) { VCAP::CloudController::ServicePlan.make(name: 'plan-name', service: service) }
let(:instance) { VCAP::CloudController::ManagedServiceInstance.make(name: 'instance-name', tags: %w[an-instance-tag another-instance-tag], service_plan: plan) }
let(:binding_name) { 'binding-name' }
let(:credentials) do
{
string: 'a string',
number: 42,
boolean: true,
array: %w[one two three],
hash: {
key: 'value'
}
}
end
let(:syslog_drain_url) { nil }
let(:volume_mounts) { nil }
let(:binding) do
VCAP::CloudController::ServiceBinding.make(
name: binding_name,
credentials: credentials,
service_instance: instance,
syslog_drain_url: syslog_drain_url,
volume_mounts: volume_mounts
)
end
let(:app) { binding.app }
let(:directory) { 'binding-name' }

describe '#build' do
subject(:build) { ServiceBindingFilesBuilder.build(app) }

it 'returns an array of Diego::Bbs::Models::Files' do
expect(build).to be_an(Array)
expect(build).not_to be_empty
expect(build).to all(be_a(Diego::Bbs::Models::Files))
end

describe 'mapping rules for service binding files' do
subject(:service_binding_files) { build }

it 'puts all files into a directory named after the service binding' do
expect(service_binding_files).not_to be_empty
expect(service_binding_files).to all(have_attributes(name: match(%r{^binding-name/.+$})))
end

include_examples 'mapping of type and provider'
include_examples 'mapping of binding metadata'
include_examples 'mapping of instance metadata'
include_examples 'mapping of plan metadata'
include_examples 'mapping of tags'
include_examples 'mapping of credentials'

it 'omits null or empty array attributes' do
expect(service_binding_files).not_to include(have_attributes(name: 'binding-name/syslog_drain_url'))
expect(service_binding_files).not_to include(have_attributes(name: 'binding-name/volume_mounts'))
end

include_examples 'expected files', %w[type provider label binding_guid name binding_name instance_guid instance_name plan tags string number boolean array hash]

context 'when binding_name is nil' do
let(:binding_name) { nil }
let(:directory) { 'instance-name' }

include_examples 'mapping of type and provider'
include_examples 'mapping of binding metadata', 'instance-name'
include_examples 'mapping of instance metadata'
include_examples 'mapping of plan metadata'
include_examples 'mapping of tags'
include_examples 'mapping of credentials'

include_examples 'expected files', %w[type provider label binding_guid name instance_guid instance_name plan tags string number boolean array hash]
end

context 'when syslog_drain_url is set' do
let(:syslog_drain_url) { 'https://syslog.drain' }

it 'maps the attribute to a file' do
expect(service_binding_files.find { |f| f.name == 'binding-name/syslog_drain_url' }).to have_attributes(value: 'https://syslog.drain')
end

include_examples 'mapping of type and provider'
include_examples 'mapping of binding metadata'
include_examples 'mapping of instance metadata'
include_examples 'mapping of plan metadata'
include_examples 'mapping of tags'
include_examples 'mapping of credentials'

include_examples 'expected files',
%w[type provider label binding_guid name binding_name instance_guid instance_name plan tags string number boolean array hash syslog_drain_url]
end

context 'when volume_mounts is set' do
let(:volume_mounts) do
[{
container_dir: 'dir1',
device_type: 'type1',
mode: 'mode1',
foo: 'bar'
}, {
container_dir: 'dir2',
device_type: 'type2',
mode: 'mode2',
foo: 'baz'
}]
end

it 'maps the attribute to a file' do
expect(service_binding_files.find do |f|
f.name == 'binding-name/volume_mounts'
end).to have_attributes(value: '[{"container_dir":"dir1","device_type":"type1","mode":"mode1"},{"container_dir":"dir2","device_type":"type2","mode":"mode2"}]')
end

include_examples 'mapping of type and provider'
include_examples 'mapping of binding metadata'
include_examples 'mapping of instance metadata'
include_examples 'mapping of plan metadata'
include_examples 'mapping of tags'
include_examples 'mapping of credentials'

include_examples 'expected files',
%w[type provider label binding_guid name binding_name instance_guid instance_name plan tags string number boolean array hash volume_mounts]
end

context 'when the instance is user-provided' do
let(:instance) { VCAP::CloudController::UserProvidedServiceInstance.make(name: 'upsi', tags: %w[an-upsi-tag another-upsi-tag]) }

include_examples 'mapping of type and provider', 'user-provided'
include_examples 'mapping of binding metadata'
include_examples 'mapping of instance metadata', 'upsi'
include_examples 'mapping of tags', '["an-upsi-tag","another-upsi-tag"]'
include_examples 'mapping of credentials'

include_examples 'expected files', %w[type provider label binding_guid name binding_name instance_guid instance_name tags string number boolean array hash]
end

context 'when there are duplicate keys at different levels' do
let(:credentials) { { type: 'duplicate-type', name: 'duplicate-name', credentials: { password: 'secret' } } }

include_examples 'mapping of type and provider' # no 'duplicate-type'
include_examples 'mapping of binding metadata' # no 'duplicate-name'
include_examples 'mapping of instance metadata'
include_examples 'mapping of plan metadata'
include_examples 'mapping of tags'
include_examples 'mapping of credentials', { credentials: '{"password":"secret"}' }

include_examples 'expected files', %w[type provider label binding_guid name binding_name instance_guid instance_name plan tags credentials]
end

context 'when there are duplicate binding names' do
let(:binding_name) { 'duplicate-name' }

before do
VCAP::CloudController::ServiceBinding.make(app: app,
service_instance: VCAP::CloudController::UserProvidedServiceInstance.make(
space: app.space, name: 'duplicate-name'
))
end

it 'raises an exception' do
expect { service_binding_files }.to raise_error(ServiceBindingFilesBuilder::IncompatibleBindings, 'Duplicate binding name: duplicate-name')
end
end

context 'when binding names violate the Service Binding Specification for Kubernetes' do
let(:binding_name) { 'binding_name' }

it 'raises an exception' do
expect { service_binding_files }.to raise_error(ServiceBindingFilesBuilder::IncompatibleBindings, 'Invalid binding name: binding_name')
end
end

context 'when the bindings exceed the maximum allowed bytesize' do
let(:xxl_credentials) do
c = {}
value = 'v' * 1000
1000.times do |i|
c["key#{i}"] = value
end
c
end

before do
allow_any_instance_of(ServiceBindingPresenter).to receive(:to_hash).and_wrap_original do |original|
original.call.merge(credentials: xxl_credentials)
end
end

it 'raises an exception' do
expect { service_binding_files }.to raise_error(ServiceBindingFilesBuilder::IncompatibleBindings, /^Bindings exceed the maximum allowed bytesize of 1000000: \d+/)
end
end

context 'when credential keys violate the Service Binding Specification for Kubernetes for binding entry file names' do
let(:credentials) { { '../secret': 'hidden' } }

it 'raises an exception' do
expect { service_binding_files }.to raise_error(ServiceBindingFilesBuilder::IncompatibleBindings, 'Invalid file name: ../secret')
end
end
end
end
end
end

0 comments on commit 4879337

Please sign in to comment.