Skip to content

Commit d216094

Browse files
committed
[PoC] Implement Custom Stacks RFC
1 parent cf40b64 commit d216094

File tree

13 files changed

+188
-16
lines changed

13 files changed

+188
-16
lines changed

app/fetchers/buildpack_lifecycle_fetcher.rb

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,15 @@ module VCAP::CloudController
44
class BuildpackLifecycleFetcher
55
class << self
66
def fetch(buildpack_names, stack_name, lifecycle=Config.config.get(:default_app_lifecycle))
7+
# Handle custom stacks (docker:// URLs) - they don't exist in the database
8+
if stack_name.is_a?(String) && stack_name.include?('docker://')
9+
stack = stack_name
10+
else
11+
stack = Stack.find(name: stack_name)
12+
end
13+
714
{
8-
stack: Stack.find(name: stack_name),
15+
stack: stack,
916
buildpack_infos: ordered_buildpacks(buildpack_names, stack_name, lifecycle)
1017
}
1118
end

app/messages/buildpack_lifecycle_data_message.rb

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
require 'uri'
2+
13
module VCAP::CloudController
24
class BuildpackLifecycleDataMessage < BaseMessage
35
register_allowed_keys %i[buildpacks stack credentials]
@@ -19,6 +21,7 @@ class BuildpackLifecycleDataMessage < BaseMessage
1921

2022
validate :buildpacks_content
2123
validate :credentials_content
24+
validate :custom_stack_requires_custom_buildpack
2225

2326
def buildpacks_content
2427
return unless buildpacks.is_a?(Array)
@@ -40,7 +43,34 @@ def buildpacks_content
4043
def credentials_content
4144
return unless credentials.is_a?(Hash)
4245

43-
errors.add(:credentials, 'credentials value must be a hash') if credentials.any? { |_, v| !v.is_a?(Hash) }
46+
credentials.each do |registry, creds|
47+
unless creds.is_a?(Hash)
48+
errors.add(:credentials, "for registry '#{registry}' must be a hash")
49+
next
50+
end
51+
52+
has_username = creds.key?('username') || creds.key?(:username)
53+
has_password = creds.key?('password') || creds.key?(:password)
54+
errors.add(:base, "credentials for #{registry} must include 'username' and 'password'") unless has_username && has_password
55+
end
56+
end
57+
58+
def custom_stack_requires_custom_buildpack
59+
return unless stack.is_a?(String) && stack.include?('docker://')
60+
return unless FeatureFlag.enabled?(:diego_custom_stacks)
61+
return unless buildpacks.is_a?(Array)
62+
63+
buildpacks.each do |buildpack_name|
64+
# If buildpack is a URL, it's custom
65+
next if buildpack_name&.match?(URI::DEFAULT_PARSER.make_regexp)
66+
67+
# Check if it's a system buildpack
68+
system_buildpack = Buildpack.find(name: buildpack_name)
69+
if system_buildpack
70+
errors.add(:base, 'Buildpack must be a custom buildpack when using a custom stack')
71+
break
72+
end
73+
end
4474
end
4575
end
4676
end

app/models/runtime/cnb_lifecycle_data_model.rb

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -68,13 +68,10 @@ def using_custom_buildpack?
6868
end
6969

7070
def to_hash
71-
hash = {
71+
{
7272
buildpacks: buildpacks.map { |buildpack| CloudController::UrlSecretObfuscator.obfuscate(buildpack) },
7373
stack: stack
7474
}
75-
hash[:credentials] = Presenters::Censorship::REDACTED_CREDENTIAL unless credentials.nil?
76-
77-
hash
7875
end
7976

8077
def validate

app/models/runtime/feature_flag.rb

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ class UndefinedFeatureFlagError < StandardError
1414
service_instance_creation: true,
1515
diego_docker: false,
1616
diego_cnb: false,
17+
diego_custom_stacks: false,
1718
set_roles_by_username: true,
1819
unset_roles_by_username: true,
1920
task_creation: true,

lib/cloud_controller/diego/lifecycles/buildpack_lifecycle_data_validator.rb

Lines changed: 29 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,14 @@ class BuildpackLifecycleDataValidator
88

99
validate :buildpacks_are_uri_or_nil
1010
validate :stack_exists_in_db
11+
validate :custom_stack_requires_custom_buildpack
12+
13+
def custom_stack_requires_custom_buildpack
14+
return unless stack.is_a?(String) && stack.include?('docker://')
15+
return if buildpack_infos.all?(&:custom?)
16+
17+
errors.add(:buildpack, 'must be a custom buildpack when using a custom stack')
18+
end
1119

1220
def buildpacks_are_uri_or_nil
1321
buildpack_infos.each do |buildpack_info|
@@ -16,15 +24,34 @@ def buildpacks_are_uri_or_nil
1624
next if buildpack_info.buildpack_url
1725

1826
if stack
19-
errors.add(:buildpack, %("#{buildpack_info.buildpack}" for stack "#{stack.name}" must be an existing admin buildpack or a valid git URI))
27+
stack_name = stack.is_a?(String) ? stack : stack.name
28+
errors.add(:buildpack, %("#{buildpack_info.buildpack}" for stack "#{stack_name}" must be an existing admin buildpack or a valid git URI))
2029
else
2130
errors.add(:buildpack, %("#{buildpack_info.buildpack}" must be an existing admin buildpack or a valid git URI))
2231
end
2332
end
2433
end
2534

2635
def stack_exists_in_db
27-
errors.add(:stack, 'must be an existing stack') if stack.nil?
36+
# Explicitly check for nil first
37+
if stack.nil?
38+
errors.add(:stack, 'must be an existing stack')
39+
return
40+
end
41+
42+
# Handle custom stacks (docker:// URLs passed as strings)
43+
if stack.is_a?(String) && stack.include?('docker://') && FeatureFlag.enabled?(:diego_custom_stacks)
44+
return
45+
end
46+
47+
# Handle existing stack objects or string names
48+
if stack.is_a?(String)
49+
# For string stack names, verify they exist in the database
50+
unless VCAP::CloudController::Stack.where(name: stack).any?
51+
errors.add(:stack, 'must be an existing stack')
52+
end
53+
end
54+
# If stack is an object (not nil, not string), assume it's valid
2855
end
2956
end
3057
end

lib/cloud_controller/diego/lifecycles/lifecycle_base.rb

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,13 @@ def initialize(package, staging_message)
1717
delegate :valid?, :errors, to: :validator
1818

1919
def staging_stack
20-
requested_stack || app_stack || VCAP::CloudController::Stack.default.name
20+
stack = requested_stack || app_stack || VCAP::CloudController::Stack.default.name
21+
FeatureFlag.raise_unless_enabled!(:diego_custom_stacks) if stack.is_a?(String) && stack.include?('docker://')
22+
stack
23+
end
24+
25+
def credentials
26+
staging_message.lifecycle_data[:credentials]
2127
end
2228

2329
private

lib/cloud_controller/diego/task_recipe_builder.rb

Lines changed: 26 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -91,12 +91,36 @@ def build_staging_task(config, staging_details)
9191
"app:#{staging_details.package.app_guid}"
9292
]
9393
),
94-
image_username: staging_details.package.docker_username,
95-
image_password: staging_details.package.docker_password,
94+
image_username: image_username(staging_details),
95+
image_password: image_password(staging_details),
9696
volume_mounted_files: ServiceBindingFilesBuilder.build(staging_details.package.app)
9797
}.compact)
9898
end
9999

100+
def image_username(staging_details)
101+
return staging_details.package.docker_username if staging_details.package.docker_username.present?
102+
return unless staging_details.lifecycle.respond_to?(:credentials) && staging_details.lifecycle.credentials.present?
103+
104+
cred = get_credentials_for_stack(staging_details)
105+
cred ? cred['username'] : nil
106+
end
107+
108+
def image_password(staging_details)
109+
return staging_details.package.docker_password if staging_details.package.docker_password.present?
110+
return unless staging_details.lifecycle.respond_to?(:credentials) && staging_details.lifecycle.credentials.present?
111+
112+
cred = get_credentials_for_stack(staging_details)
113+
cred ? cred['password'] : nil
114+
end
115+
116+
def get_credentials_for_stack(staging_details)
117+
return nil unless staging_details.lifecycle.staging_stack.include?('docker://')
118+
119+
stack_uri = URI.parse(staging_details.lifecycle.staging_stack)
120+
host = stack_uri.host
121+
staging_details.lifecycle.credentials[host]
122+
end
123+
100124
private
101125

102126
def metric_tags(source)

spec/api/documentation/feature_flags_api_spec.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@
1818
client.get '/v2/config/feature_flags', {}, headers
1919

2020
expect(status).to eq(200)
21-
expect(parsed_response.length).to eq(18)
21+
expect(parsed_response.length).to eq(19)
2222
expect(parsed_response).to include(
2323
{
2424
'name' => 'user_org_creation',

spec/unit/actions/app_update_spec.rb

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -259,6 +259,23 @@ module VCAP::CloudController
259259
end
260260
end
261261

262+
context 'when the lifecycle is valid' do
263+
let(:message) do
264+
AppUpdateMessage.new({
265+
lifecycle: {
266+
type: 'buildpack',
267+
data: { buildpacks: ['http://new-buildpack.url', 'ruby'], stack: stack.name }
268+
}
269+
})
270+
end
271+
272+
it 'does not raise an error' do
273+
expect do
274+
app_update.update(app_model, message, lifecycle)
275+
end.not_to raise_error
276+
end
277+
end
278+
262279
context 'when changing the lifecycle type' do
263280
let(:message) do
264281
AppUpdateMessage.new({

spec/unit/controllers/v3/apps_controller_spec.rb

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -468,7 +468,7 @@
468468
{
469469
name: 'some-name',
470470
relationships: { space: { data: { guid: space.guid } } },
471-
lifecycle: { type: 'cnb', data: { buildpacks: ['http://example.com'], credentials: { registry: { user: 'password' } } } }
471+
lifecycle: { type: 'cnb', data: { buildpacks: ['http://example.com'], stack: 'cflinuxfs4', credentials: { 'docker.io' => { 'username' => 'user', 'password' => 'password' } } } }
472472
}
473473
end
474474

@@ -479,7 +479,7 @@
479479
lifecycle_data = response_body['lifecycle']['data']
480480

481481
expect(response).to have_http_status :created
482-
expect(lifecycle_data).to eq({ 'buildpacks' => ['http://example.com'], 'stack' => 'default-stack-name', 'credentials' => '***' })
482+
expect(lifecycle_data).to eq({ 'buildpacks' => ['http://example.com'], 'stack' => 'cflinuxfs4' })
483483
end
484484
end
485485
end

0 commit comments

Comments
 (0)