Skip to content
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
9 changes: 8 additions & 1 deletion app/fetchers/buildpack_lifecycle_fetcher.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,15 @@ module VCAP::CloudController
class BuildpackLifecycleFetcher
class << self
def fetch(buildpack_names, stack_name, lifecycle=Config.config.get(:default_app_lifecycle))
# Handle custom stacks (docker:// URLs) - they don't exist in the database
if stack_name.is_a?(String) && stack_name.include?('docker://')
stack = stack_name
else
stack = Stack.find(name: stack_name)
end

{
stack: Stack.find(name: stack_name),
stack: stack,
buildpack_infos: ordered_buildpacks(buildpack_names, stack_name, lifecycle)
}
end
Expand Down
32 changes: 31 additions & 1 deletion app/messages/buildpack_lifecycle_data_message.rb
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
require 'uri'

module VCAP::CloudController
class BuildpackLifecycleDataMessage < BaseMessage
register_allowed_keys %i[buildpacks stack credentials]
Expand All @@ -19,6 +21,7 @@ class BuildpackLifecycleDataMessage < BaseMessage

validate :buildpacks_content
validate :credentials_content
validate :custom_stack_requires_custom_buildpack

def buildpacks_content
return unless buildpacks.is_a?(Array)
Expand All @@ -40,7 +43,34 @@ def buildpacks_content
def credentials_content
return unless credentials.is_a?(Hash)

errors.add(:credentials, 'credentials value must be a hash') if credentials.any? { |_, v| !v.is_a?(Hash) }
credentials.each do |registry, creds|
unless creds.is_a?(Hash)
errors.add(:credentials, "for registry '#{registry}' must be a hash")
next
end

has_username = creds.key?('username') || creds.key?(:username)
has_password = creds.key?('password') || creds.key?(:password)
errors.add(:base, "credentials for #{registry} must include 'username' and 'password'") unless has_username && has_password
end
end

def custom_stack_requires_custom_buildpack
return unless stack.is_a?(String) && stack.include?('docker://')
return unless FeatureFlag.enabled?(:diego_custom_stacks)
return unless buildpacks.is_a?(Array)

buildpacks.each do |buildpack_name|
# If buildpack is a URL, it's custom
next if buildpack_name&.match?(URI::DEFAULT_PARSER.make_regexp)

# Check if it's a system buildpack
system_buildpack = Buildpack.find(name: buildpack_name)
if system_buildpack
errors.add(:base, 'Buildpack must be a custom buildpack when using a custom stack')
break
end
end
end
end
end
2 changes: 1 addition & 1 deletion app/messages/validators.rb
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,7 @@ def validate(record)
lifecycle_data_message = lifecycle_data_message_class.new(record.lifecycle_data)
return if lifecycle_data_message.valid?

lifecycle_data_message.errors.full_messages.each do |message|
lifecycle_data_message.errors.each do |attribute, message|
record.errors.add(:lifecycle, message:)
end
end
Expand Down
5 changes: 1 addition & 4 deletions app/models/runtime/cnb_lifecycle_data_model.rb
Original file line number Diff line number Diff line change
Expand Up @@ -68,13 +68,10 @@ def using_custom_buildpack?
end

def to_hash
hash = {
{
buildpacks: buildpacks.map { |buildpack| CloudController::UrlSecretObfuscator.obfuscate(buildpack) },
stack: stack
}
hash[:credentials] = Presenters::Censorship::REDACTED_CREDENTIAL unless credentials.nil?

hash
end

def validate
Expand Down
1 change: 1 addition & 0 deletions app/models/runtime/feature_flag.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ class UndefinedFeatureFlagError < StandardError
service_instance_creation: true,
diego_docker: false,
diego_cnb: false,
diego_custom_stacks: false,
set_roles_by_username: true,
unset_roles_by_username: true,
task_creation: true,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,14 @@ class BuildpackLifecycleDataValidator

validate :buildpacks_are_uri_or_nil
validate :stack_exists_in_db
validate :custom_stack_requires_custom_buildpack

def custom_stack_requires_custom_buildpack
return unless stack.is_a?(String) && stack.include?('docker://')
return if buildpack_infos.all?(&:custom?)

errors.add(:buildpack, 'must be a custom buildpack when using a custom stack')
end

def buildpacks_are_uri_or_nil
buildpack_infos.each do |buildpack_info|
Expand All @@ -16,15 +24,34 @@ def buildpacks_are_uri_or_nil
next if buildpack_info.buildpack_url

if stack
errors.add(:buildpack, %("#{buildpack_info.buildpack}" for stack "#{stack.name}" must be an existing admin buildpack or a valid git URI))
stack_name = stack.is_a?(String) ? stack : stack.name
errors.add(:buildpack, %("#{buildpack_info.buildpack}" for stack "#{stack_name}" must be an existing admin buildpack or a valid git URI))
else
errors.add(:buildpack, %("#{buildpack_info.buildpack}" must be an existing admin buildpack or a valid git URI))
end
end
end

def stack_exists_in_db
errors.add(:stack, 'must be an existing stack') if stack.nil?
# Explicitly check for nil first
if stack.nil?
errors.add(:stack, 'must be an existing stack')
return
end

# Handle custom stacks (docker:// URLs passed as strings)
if stack.is_a?(String) && stack.include?('docker://') && FeatureFlag.enabled?(:diego_custom_stacks)
return
end

# Handle existing stack objects or string names
if stack.is_a?(String)
# For string stack names, verify they exist in the database
unless VCAP::CloudController::Stack.where(name: stack).any?
errors.add(:stack, 'must be an existing stack')
end
end
# If stack is an object (not nil, not string), assume it's valid
end
end
end
8 changes: 7 additions & 1 deletion lib/cloud_controller/diego/lifecycles/lifecycle_base.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,13 @@ def initialize(package, staging_message)
delegate :valid?, :errors, to: :validator

def staging_stack
requested_stack || app_stack || VCAP::CloudController::Stack.default.name
stack = requested_stack || app_stack || VCAP::CloudController::Stack.default.name
FeatureFlag.raise_unless_enabled!(:diego_custom_stacks) if stack.is_a?(String) && stack.include?('docker://')
stack
end

def credentials
staging_message.lifecycle_data[:credentials]
end

private
Expand Down
28 changes: 26 additions & 2 deletions lib/cloud_controller/diego/task_recipe_builder.rb
Original file line number Diff line number Diff line change
Expand Up @@ -91,12 +91,36 @@ def build_staging_task(config, staging_details)
"app:#{staging_details.package.app_guid}"
]
),
image_username: staging_details.package.docker_username,
image_password: staging_details.package.docker_password,
image_username: image_username(staging_details),
image_password: image_password(staging_details),
volume_mounted_files: ServiceBindingFilesBuilder.build(staging_details.package.app)
}.compact)
end

def image_username(staging_details)
return staging_details.package.docker_username if staging_details.package.docker_username.present?
return unless staging_details.lifecycle.respond_to?(:credentials) && staging_details.lifecycle.credentials.present?

cred = get_credentials_for_stack(staging_details)
cred ? cred['username'] : nil
end

def image_password(staging_details)
return staging_details.package.docker_password if staging_details.package.docker_password.present?
return unless staging_details.lifecycle.respond_to?(:credentials) && staging_details.lifecycle.credentials.present?

cred = get_credentials_for_stack(staging_details)
cred ? cred['password'] : nil
end

def get_credentials_for_stack(staging_details)
return nil unless staging_details.lifecycle.staging_stack.include?('docker://')

stack_uri = URI.parse(staging_details.lifecycle.staging_stack)
host = stack_uri.host
staging_details.lifecycle.credentials[host]
end

private

def metric_tags(source)
Expand Down
2 changes: 1 addition & 1 deletion spec/api/documentation/feature_flags_api_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
client.get '/v2/config/feature_flags', {}, headers

expect(status).to eq(200)
expect(parsed_response.length).to eq(18)
expect(parsed_response.length).to eq(19)
expect(parsed_response).to include(
{
'name' => 'user_org_creation',
Expand Down
17 changes: 17 additions & 0 deletions spec/unit/actions/app_update_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -259,6 +259,23 @@ module VCAP::CloudController
end
end

context 'when the lifecycle is valid' do
let(:message) do
AppUpdateMessage.new({
lifecycle: {
type: 'buildpack',
data: { buildpacks: ['http://new-buildpack.url', 'ruby'], stack: stack.name }
}
})
end

it 'does not raise an error' do
expect do
app_update.update(app_model, message, lifecycle)
end.not_to raise_error
end
end

context 'when changing the lifecycle type' do
let(:message) do
AppUpdateMessage.new({
Expand Down
4 changes: 2 additions & 2 deletions spec/unit/controllers/v3/apps_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -468,7 +468,7 @@
{
name: 'some-name',
relationships: { space: { data: { guid: space.guid } } },
lifecycle: { type: 'cnb', data: { buildpacks: ['http://example.com'], credentials: { registry: { user: 'password' } } } }
lifecycle: { type: 'cnb', data: { buildpacks: ['http://example.com'], stack: 'cflinuxfs4', credentials: { 'docker.io' => { 'username' => 'user', 'password' => 'password' } } } }
}
end

Expand All @@ -479,7 +479,7 @@
lifecycle_data = response_body['lifecycle']['data']

expect(response).to have_http_status :created
expect(lifecycle_data).to eq({ 'buildpacks' => ['http://example.com'], 'stack' => 'default-stack-name', 'credentials' => '***' })
expect(lifecycle_data).to eq({ 'buildpacks' => ['http://example.com'], 'stack' => 'cflinuxfs4' })
end
end
end
Expand Down
4 changes: 2 additions & 2 deletions spec/unit/messages/app_manifest_message_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2095,7 +2095,7 @@ module VCAP::CloudController
end

context 'when cnb_credentials key is specified' do
let(:parsed_yaml) { { name: 'cnb', lifecycle: 'cnb', buildpacks: %w[nodejs java], stack: stack.name, cnb_credentials: { registry: { username: 'password' } } } }
let(:parsed_yaml) { { name: 'cnb', lifecycle: 'cnb', buildpacks: %w[nodejs java], stack: stack.name, cnb_credentials: { registry: { username: 'user', password: 'password' } } } }

it 'adds credentials to the lifecycle_data' do
message = AppManifestMessage.create_from_yml(parsed_yaml)
Expand All @@ -2104,7 +2104,7 @@ module VCAP::CloudController
expect(message.app_update_message.lifecycle_type).to eq(Lifecycles::CNB)
expect(message.app_update_message.buildpack_data.buildpacks).to eq(%w[nodejs java])
expect(message.app_update_message.buildpack_data.stack).to eq(stack.name)
expect(message.app_update_message.buildpack_data.credentials).to eq({ registry: { username: 'password' } })
expect(message.app_update_message.buildpack_data.credentials).to eq({ registry: { username: 'user', password: 'password' } })
end
end
end
Expand Down
56 changes: 56 additions & 0 deletions spec/unit/messages/build_create_message_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,62 @@ module VCAP::CloudController
expect(message).not_to be_valid
expect(message.errors[:lifecycle]).to include('Buildpacks can only contain strings')
end

context 'credentials' do
it 'is valid with valid credentials' do
params[:lifecycle] = {
type: 'buildpack',
data: {
stack: 'cflinuxfs4',
credentials: {
'docker.io' => {
'username' => 'user',
'password' => 'password'
}
}
}
}
message = BuildCreateMessage.new(params)
expect(message).to be_valid
end

it 'is invalid with invalid credentials' do
params[:lifecycle] = {
type: 'buildpack',
data: {
stack: 'cflinuxfs4',
credentials: {
'docker.io' => {
'username' => 'user'
}
}
}
}
message = BuildCreateMessage.new(params)
expect(message).not_to be_valid
expect(message.errors[:lifecycle]).to include("credentials for docker.io must include 'username' and 'password'")
end
end

context 'when a custom stack is used with a system buildpack' do
before do
VCAP::CloudController::FeatureFlag.make(name: 'diego_custom_stacks', enabled: true)
VCAP::CloudController::Buildpack.make(name: 'ruby_buildpack')
end

it 'is not valid' do
params[:lifecycle] = {
type: 'buildpack',
data: {
stack: 'docker://cloudfoundry/cflinuxfs4',
buildpacks: ['ruby_buildpack']
}
}
message = BuildCreateMessage.new(params)
expect(message).not_to be_valid
expect(message.errors[:lifecycle]).to include('Buildpack must be a custom buildpack when using a custom stack')
end
end
end

describe 'docker lifecycle' do
Expand Down
7 changes: 7 additions & 0 deletions spec/unit/models/runtime/feature_flag_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,13 @@ module VCAP::CloudController
end
end
end

context 'when the diego_custom_stacks feature flag is not overridden' do
it 'returns the default value' do
expect(FeatureFlag.enabled?(:diego_custom_stacks)).to be(false)
expect(FeatureFlag.disabled?(:diego_custom_stacks)).to be(true)
end
end
end

describe '.raise_unless_enabled!' do
Expand Down
Loading