Skip to content

Commit c4a118b

Browse files
Loosen file name validation for kubernetes service bindings (#4525)
- The ServiceBinding spec has been updated to explicitly mention that underscores are valid in credential keys (https://github.com/servicebinding/spec/blob/main/README.md#workload-projection). Updating the implementation to match the change in the spec Signed-off-by: Tom Kennedy <[email protected]>
1 parent 87dc55c commit c4a118b

File tree

2 files changed

+19
-3
lines changed

2 files changed

+19
-3
lines changed

lib/cloud_controller/diego/service_binding_files_builder.rb

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ def build_service_binding_k8s
3737
@service_bindings.select(&:create_succeeded?).each do |service_binding|
3838
sb_hash = ServiceBindingPresenter.new(service_binding, include_instance: true).to_hash
3939
name = sb_hash[:name]
40-
raise IncompatibleBindings.new("Invalid binding name: '#{name}'. Name must match #{binding_naming_convention.inspect}") unless valid_name?(name)
40+
raise IncompatibleBindings.new("Invalid binding name: '#{name}'. Name must match #{binding_naming_convention.inspect}") unless valid_binding_name?(name)
4141
raise IncompatibleBindings.new("Duplicate binding name: #{name}") if names.add?(name).nil?
4242

4343
# add the credentials first
@@ -72,13 +72,17 @@ def binding_naming_convention
7272
/^[a-z0-9\-.]{1,253}$/
7373
end
7474

75+
def file_naming_convention
76+
/^[a-z0-9\-._]{1,253}$/
77+
end
78+
7579
# - adds a Diego::Bbs::Models::File object to the service_binding_files hash
7680
# - binding name is used as the directory name, key is used as the file name
7781
# - returns the bytesize of the path and content
7882
# - skips (and returns 0) if the value is nil or an empty array or hash
7983
# - serializes the value to JSON if it is a non-string object
8084
def add_file(service_binding_files, name, key, value)
81-
raise IncompatibleBindings.new("Invalid file name: #{key}") unless valid_name?(key)
85+
raise IncompatibleBindings.new("Invalid file name: #{key}") unless valid_file_name?(key)
8286

8387
path = "#{name}/#{key}"
8488
content = if value.nil?
@@ -95,10 +99,14 @@ def add_file(service_binding_files, name, key, value)
9599
path.bytesize + content.bytesize
96100
end
97101

98-
def valid_name?(name)
102+
def valid_binding_name?(name)
99103
name.match?(binding_naming_convention)
100104
end
101105

106+
def valid_file_name?(name)
107+
name.match?(file_naming_convention)
108+
end
109+
102110
def transform_vcap_services_attribute(name)
103111
if %w[binding_guid binding_name instance_guid instance_name syslog_drain_url volume_mounts].include?(name)
104112
name.tr('_', '-')

spec/unit/lib/cloud_controller/diego/service_binding_files_builder_spec.rb

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -274,6 +274,14 @@ module VCAP::CloudController::Diego
274274
expect { service_binding_files }.to raise_error(ServiceBindingFilesBuilder::IncompatibleBindings, 'Invalid file name: ../secret')
275275
end
276276
end
277+
278+
context 'when credential keys contain underscores' do
279+
let(:credentials) { { some_secret: 'hidden' } }
280+
281+
it 'does not return an error' do
282+
expect { service_binding_files }.not_to raise_error
283+
end
284+
end
277285
end
278286
end
279287

0 commit comments

Comments
 (0)