Skip to content

Commit ab80ff4

Browse files
committed
[PoC] Implement Custom Stacks RFC
1 parent 0e29a82 commit ab80ff4

File tree

11 files changed

+156
-9
lines changed

11 files changed

+156
-9
lines changed

app/messages/app_manifest_message.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,7 @@ def sidecar_create_messages
104104
end
105105

106106
def app_update_message
107-
@app_update_message ||= AppUpdateMessage.new(app_lifecycle_hash)
107+
@app_update_message ||= AppUpdateMessage.new(lifecycle: lifecycle_message)
108108
end
109109

110110
def app_update_environment_variables_message

app/messages/buildpack_lifecycle_data_message.rb

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,14 @@ def buildpacks_content
4040
def credentials_content
4141
return unless credentials.is_a?(Hash)
4242

43-
errors.add(:credentials, 'credentials value must be a hash') if credentials.any? { |_, v| !v.is_a?(Hash) }
43+
credentials.each do |registry, creds|
44+
unless creds.is_a?(Hash)
45+
errors.add(:credentials, "for registry '#{registry}' must be a hash")
46+
next
47+
end
48+
49+
errors.add(:credentials, "for registry '#{registry}' must include 'username' and 'password'") unless creds.key?('username') && creds.key?('password')
50+
end
4451
end
4552
end
4653
end

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: 13 additions & 1 deletion
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|
@@ -24,7 +32,11 @@ def buildpacks_are_uri_or_nil
2432
end
2533

2634
def stack_exists_in_db
27-
errors.add(:stack, 'must be an existing stack') if stack.nil?
35+
return if stack.nil?
36+
return if stack.is_a?(String) && stack.include?('docker://') && FeatureFlag.enabled?(:diego_custom_stacks)
37+
return if VCAP::CloudController::Stack.where(name: stack).any?
38+
39+
errors.add(:stack, 'must be an existing stack')
2840
end
2941
end
3042
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['user'] : 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: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -259,6 +259,40 @@ 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+
279+
context 'when the lifecycle is valid' do
280+
let(:message) do
281+
AppUpdateMessage.new({
282+
lifecycle: {
283+
type: 'buildpack',
284+
data: { buildpacks: ['http://new-buildpack.url', 'ruby'], stack: stack.name }
285+
}
286+
})
287+
end
288+
289+
it 'does not raise an error' do
290+
expect do
291+
app_update.update(app_model, message, lifecycle)
292+
end.not_to raise_error
293+
end
294+
end
295+
262296
context 'when changing the lifecycle type' do
263297
let(:message) do
264298
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

spec/unit/messages/build_create_message_spec.rb

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,62 @@ module VCAP::CloudController
126126
expect(message).not_to be_valid
127127
expect(message.errors[:lifecycle]).to include('Buildpacks can only contain strings')
128128
end
129+
130+
context 'credentials' do
131+
it 'is valid with valid credentials' do
132+
params[:lifecycle] = {
133+
type: 'buildpack',
134+
data: {
135+
stack: 'cflinuxfs4',
136+
credentials: {
137+
'docker.io' => {
138+
'username' => 'user',
139+
'password' => 'password'
140+
}
141+
}
142+
}
143+
}
144+
message = BuildCreateMessage.new(params)
145+
expect(message).to be_valid
146+
end
147+
148+
it 'is invalid with invalid credentials' do
149+
params[:lifecycle] = {
150+
type: 'buildpack',
151+
data: {
152+
stack: 'cflinuxfs4',
153+
credentials: {
154+
'docker.io' => {
155+
'username' => 'user'
156+
}
157+
}
158+
}
159+
}
160+
message = BuildCreateMessage.new(params)
161+
expect(message).not_to be_valid
162+
expect(message.errors[:lifecycle]).to include("credentials for docker.io must include 'username' and 'password'")
163+
end
164+
end
165+
166+
context 'when a custom stack is used with a system buildpack' do
167+
before do
168+
VCAP::CloudController::FeatureFlag.make(name: 'diego_custom_stacks', enabled: true)
169+
VCAP::CloudController::Buildpack.make(name: 'ruby_buildpack')
170+
end
171+
172+
it 'is not valid' do
173+
params[:lifecycle] = {
174+
type: 'buildpack',
175+
data: {
176+
stack: 'docker://cloudfoundry/cflinuxfs4',
177+
buildpacks: ['ruby_buildpack']
178+
}
179+
}
180+
message = BuildCreateMessage.new(params)
181+
expect(message).not_to be_valid
182+
expect(message.errors[:lifecycle]).to include('Buildpack must be a custom buildpack when using a custom stack')
183+
end
184+
end
129185
end
130186

131187
describe 'docker lifecycle' do

0 commit comments

Comments
 (0)