Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[RFC0030 - 3] File-based service bindings #4026

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion app/controllers/runtime/apps_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ def read_env(guid)
staging_env_json: EnvironmentVariableGroup.staging.environment_json,
running_env_json: EnvironmentVariableGroup.running.environment_json,
environment_json: process.app.environment_variables,
system_env_json: SystemEnvPresenter.new(process.service_bindings).system_env,
system_env_json: SystemEnvPresenter.new(process).system_env,
application_env_json: { 'VCAP_APPLICATION' => vcap_application }
}, mode: :compat)
]
Expand Down
2 changes: 2 additions & 0 deletions app/models/runtime/process_model.rb
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,8 @@ def revisions_enabled?
app.revisions_enabled
end

delegate :file_based_service_bindings_enabled, to: :app

def package_hash
# this caches latest_package for performance reasons
package = latest_package
Expand Down
7 changes: 5 additions & 2 deletions app/presenters/system_environment/system_env_presenter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,14 @@
require 'presenters/system_environment/service_binding_presenter'

class SystemEnvPresenter
def initialize(service_bindings)
@service_bindings = service_bindings
def initialize(app_or_process)
@file_based_service_bindings_enabled = app_or_process.file_based_service_bindings_enabled
@service_bindings = app_or_process.service_bindings
end

def system_env
return { SERVICE_BINDING_ROOT: '/etc/cf-service-bindings' } if @file_based_service_bindings_enabled

{ VCAP_SERVICES: service_binding_env_variables }
end

Expand Down
2 changes: 1 addition & 1 deletion app/presenters/v3/app_env_presenter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ def to_hash
environment_variables: app.environment_variables,
staging_env_json: EnvironmentVariableGroup.staging.environment_json,
running_env_json: EnvironmentVariableGroup.running.environment_json,
system_env_json: redact_hash(SystemEnvPresenter.new(app.service_bindings).system_env),
system_env_json: redact_hash(SystemEnvPresenter.new(app).system_env),
application_env_json: vcap_application
}
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ def build(app, space, lifecycle, memory_limit, staging_disk_in_mb, vars_from_mes
'MEMORY_LIMIT' => "#{memory_limit}m"
}
).
merge(SystemEnvPresenter.new(app.service_bindings).system_env.stringify_keys)
merge(SystemEnvPresenter.new(app).system_env.stringify_keys)
end
end
end
4 changes: 3 additions & 1 deletion lib/cloud_controller/diego/app_recipe_builder.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
require 'cloud_controller/diego/cnb/desired_lrp_builder'
require 'cloud_controller/diego/process_guid'
require 'cloud_controller/diego/ssh_key'
require 'cloud_controller/diego/service_binding_files_builder'
require 'credhub/config_helpers'
require 'models/helpers/health_check_types'
require 'cloud_controller/diego/main_lrp_action_builder'
Expand Down Expand Up @@ -100,7 +101,8 @@ def app_lrp_arguments
organizational_unit: ["organization:#{process.organization.guid}", "space:#{process.space.guid}", "app:#{process.app_guid}"]
),
image_username: process.desired_droplet.docker_receipt_username,
image_password: process.desired_droplet.docker_receipt_password
image_password: process.desired_droplet.docker_receipt_password,
volume_mounted_files: ServiceBindingFilesBuilder.build(process)
}.compact
end

Expand Down
2 changes: 1 addition & 1 deletion lib/cloud_controller/diego/environment.rb
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ def common_json_and_merge(&blk)
@initial_env.
merge(process.environment_json || {}).
merge(blk.call).
merge(SystemEnvPresenter.new(process.service_bindings).system_env)
merge(SystemEnvPresenter.new(process).system_env)

diego_env = diego_env.merge(DATABASE_URL: process.database_uri) if process.database_uri

Expand Down
86 changes: 86 additions & 0 deletions lib/cloud_controller/diego/service_binding_files_builder.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
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)
@file_based_service_bindings_enabled = app_or_process.file_based_service_bindings_enabled
@service_bindings = app_or_process.service_bindings
end

def build
return nil unless @file_based_service_bindings_enabled

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 valid_name?(name)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you explain in the error message which names are valid?

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.to_s, v) }

# add the rest of the hash; already existing credential keys are overwritten
# VCAP_SERVICES attribute names are transformed (e.g. binding_guid -> binding-guid)
sb_hash.each { |k, v| total_bytesize += add_file(service_binding_files, name, transform_vcap_services_attribute(k.to_s), 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::File 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 valid_name?(key)

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::File.new(path:, content:)
path.bytesize + content.bytesize
end

def valid_name?(name)
name.match?(/^[a-z0-9\-.]{1,253}$/)
end

def transform_vcap_services_attribute(name)
if %w[binding_guid binding_name instance_guid instance_name syslog_drain_url volume_mounts].include?(name)
name.tr('_', '-')
else
name
end
end
end
end
end
2 changes: 1 addition & 1 deletion lib/cloud_controller/diego/task_environment.rb
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ def build
initial_envs.
merge(app_env).
merge('VCAP_APPLICATION' => vcap_application, 'MEMORY_LIMIT' => "#{task.memory_in_mb}m").
merge(SystemEnvPresenter.new(app.service_bindings).system_env.stringify_keys)
merge(SystemEnvPresenter.new(app).system_env.stringify_keys)

task_env = task_env.merge('VCAP_PLATFORM_OPTIONS' => credhub_url) if credhub_url.present? && cred_interpolation_enabled?

Expand Down
7 changes: 5 additions & 2 deletions lib/cloud_controller/diego/task_recipe_builder.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
require 'cloud_controller/diego/bbs_environment_builder'
require 'cloud_controller/diego/task_completion_callback_generator'
require 'cloud_controller/diego/task_cpu_weight_calculator'
require 'cloud_controller/diego/service_binding_files_builder'

module VCAP::CloudController
module Diego
Expand Down Expand Up @@ -52,7 +53,8 @@ def build_app_task(config, task)
]
),
image_username: task.droplet.docker_receipt_username,
image_password: task.droplet.docker_receipt_password
image_password: task.droplet.docker_receipt_password,
volume_mounted_files: ServiceBindingFilesBuilder.build(task.app)
}.compact)
end

Expand Down Expand Up @@ -90,7 +92,8 @@ def build_staging_task(config, staging_details)
]
),
image_username: staging_details.package.docker_username,
image_password: staging_details.package.docker_password
image_password: staging_details.package.docker_password,
volume_mounted_files: ServiceBindingFilesBuilder.build(staging_details.package.app)
}.compact)
end

Expand Down
2 changes: 2 additions & 0 deletions lib/diego/bbs/models/actions_pb.rb

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions lib/diego/bbs/models/desired_lrp_pb.rb

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

19 changes: 19 additions & 0 deletions lib/diego/bbs/models/file_pb.rb

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions lib/diego/bbs/models/task_pb.rb

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

14 changes: 14 additions & 0 deletions spec/request/apps_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1601,6 +1601,20 @@
end

it_behaves_like 'permissions for single object endpoint', ALL_PERMISSIONS

context 'when file-based service bindings are enabled' do
let(:app_model_response_object) do
r = super()
r[:system_env_json] = { SERVICE_BINDING_ROOT: '/etc/cf-service-bindings' }
r
end

before do
app_model.update(file_based_service_bindings_enabled: true)
end

it_behaves_like 'permissions for single object endpoint', ALL_PERMISSIONS
end
end

context 'when VCAP_SERVICES contains potentially sensitive information' do
Expand Down
23 changes: 23 additions & 0 deletions spec/unit/lib/cloud_controller/diego/app_recipe_builder_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,23 @@ module Diego
expect(lrp.trusted_system_certificates_path).to eq(RUNNING_TRUSTED_SYSTEM_CERT_PATH)
expect(lrp.PlacementTags).to eq(['placement-tag'])
expect(lrp.certificate_properties).to eq(expected_certificate_properties)

expect(lrp.volume_mounted_files).to be_empty
end
end

shared_examples 'file-based service bindings' do
context 'when file-based service bindings are enabled' do
before do
app = process.app
app.update(file_based_service_bindings_enabled: true)
VCAP::CloudController::ServiceBinding.make(service_instance: ManagedServiceInstance.make(space: app.space), app: app)
end

it 'includes volume mounted files' do
lrp = builder.build_app_lrp
expect(lrp.volume_mounted_files).not_to be_empty
end
end
end

Expand Down Expand Up @@ -914,6 +931,8 @@ module Diego
expect(lrp2.action).to eq(expected_action)
end
end

include_examples 'file-based service bindings'
end

context 'when the lifecycle_type is "cnb"' do
Expand Down Expand Up @@ -1005,6 +1024,8 @@ module Diego
}))
end
end

include_examples 'file-based service bindings'
end

context 'when the lifecycle_type is "docker"' do
Expand Down Expand Up @@ -1348,6 +1369,8 @@ module Diego
}))
end
end

include_examples 'file-based service bindings'
end
end

Expand Down
2 changes: 1 addition & 1 deletion spec/unit/lib/cloud_controller/diego/environment_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ module VCAP::CloudController::Diego
encoded_vcap_application_json = vcap_app.to_json

vcap_services_key = :VCAP_SERVICES
system_env = SystemEnvPresenter.new(process.service_bindings).system_env
system_env = SystemEnvPresenter.new(process).system_env
expect(system_env).to have_key(vcap_services_key)

encoded_vcap_services_json = system_env[vcap_services_key].to_json
Expand Down
Loading
Loading