Skip to content

Commit 637c044

Browse files
committed
Allow specifying buildpacks with CNBs
1 parent 1f77564 commit 637c044

File tree

6 files changed

+153
-33
lines changed

6 files changed

+153
-33
lines changed

app/actions/app_apply_manifest.rb

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,7 @@ def apply(app_guid, message)
5959

6060
app_update_message = message.app_update_message
6161
lifecycle = AppLifecycleProvider.provide_for_update(app_update_message, app)
62+
6263
AppUpdate.new(@user_audit_info, manifest_triggered: true).update(app, app_update_message, lifecycle)
6364

6465
update_routes(app, message)

app/controllers/v3/space_manifests_controller.rb

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ def apply_manifest
1616
can_write_space(space)
1717

1818
messages = parsed_app_manifests.map { |app_manifest| AppManifestMessage.create_from_yml(app_manifest) }
19+
1920
errors = messages.each_with_index.flat_map { |message, i| errors_for_message(message, i) }
2021
compound_error!(errors) unless errors.empty?
2122

app/messages/app_manifest_message.rb

Lines changed: 22 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -123,16 +123,32 @@ def audit_hash
123123
end
124124

125125
def app_lifecycle_hash
126-
lifecycle_data = if requested?(:lifecycle) && @lifecycle == 'cnb'
127-
cnb_lifecycle_data
126+
lifecycle_type = if requested?(:lifecycle) && @lifecycle == 'cnb'
127+
Lifecycles::CNB
128+
elsif requested?(:lifecycle) && @lifecycle == 'buildpack'
129+
Lifecycles::BUILDPACK
128130
elsif requested?(:docker)
129-
docker_lifecycle_data
130-
elsif requested?(:buildpacks) || requested?(:buildpack) || requested?(:stack)
131-
buildpacks_lifecycle_data
131+
Lifecycles::DOCKER
132132
end
133133

134+
data = {
135+
buildpacks: requested_buildpacks,
136+
stack: @stack,
137+
credentials: @cnb_credentials
138+
}.compact
139+
140+
if lifecycle_type == Lifecycles::DOCKER
141+
lifecycle = {
142+
type: Lifecycles::DOCKER
143+
}
144+
elsif lifecycle_type || data.present?
145+
lifecycle = {}
146+
lifecycle[:type] = lifecycle_type if lifecycle_type.present?
147+
lifecycle[:data] = data if data.present?
148+
end
149+
134150
{
135-
lifecycle: lifecycle_data,
151+
lifecycle: lifecycle,
136152
metadata: requested?(:metadata) ? metadata : nil
137153
}.compact
138154
end
@@ -279,31 +295,6 @@ def service_bindings_attribute_mapping
279295
mapping
280296
end
281297

282-
def docker_lifecycle_data
283-
{ type: Lifecycles::DOCKER }
284-
end
285-
286-
def cnb_lifecycle_data
287-
{
288-
type: Lifecycles::CNB,
289-
data: {
290-
buildpacks: requested_buildpacks,
291-
stack: @stack,
292-
credentials: @cnb_credentials
293-
}.compact
294-
}
295-
end
296-
297-
def buildpacks_lifecycle_data
298-
{
299-
type: Lifecycles::BUILDPACK,
300-
data: {
301-
buildpacks: requested_buildpacks,
302-
stack: @stack
303-
}.compact
304-
}
305-
end
306-
307298
def should_autodetect?(buildpack)
308299
buildpack == 'default' || buildpack == 'null' || buildpack.nil?
309300
end

spec/unit/actions/app_apply_manifest_spec.rb

Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -147,6 +147,98 @@ module VCAP::CloudController
147147
end
148148
end
149149

150+
context 'cnb apps' do
151+
let(:buildpack) { VCAP::CloudController::Buildpack.make }
152+
let(:message) { AppManifestMessage.create_from_yml({ name: 'blah' }) }
153+
let(:app_update_message) { message.app_update_message }
154+
let(:app) { AppModel.make(:cnb) }
155+
156+
before do
157+
TestConfig.override(default_app_lifecycle: 'cnb')
158+
end
159+
160+
context 'when the default_app_lifecycle is set and the the lifecycle is not specified' do
161+
it 'preserves the lifecycle' do
162+
app_apply_manifest.apply(app.guid, message)
163+
expect(AppUpdate).to have_received(:new).with(user_audit_info, manifest_triggered: true)
164+
expect(app_update).to have_received(:update).
165+
with(app, app_update_message, instance_of(AppCNBLifecycle))
166+
expect(app.reload.lifecycle_type).to eq('cnb')
167+
end
168+
end
169+
170+
context 'when buildpack is specified' do
171+
let(:message) { AppManifestMessage.create_from_yml({ name: 'blah', buildpack: buildpack.name }) }
172+
173+
it 'preserves the lifecycle' do
174+
app_apply_manifest.apply(app.guid, message)
175+
expect(AppUpdate).to have_received(:new).with(user_audit_info, manifest_triggered: true)
176+
expect(app_update).to have_received(:update).
177+
with(app, app_update_message, instance_of(AppCNBLifecycle))
178+
expect(app.reload.lifecycle_type).to eq('cnb')
179+
end
180+
end
181+
182+
context 'when the default differs from what is already set on the app' do
183+
let(:message) { AppManifestMessage.create_from_yml({ name: 'blah', buildpack: buildpack.name }) }
184+
let(:app) { AppModel.make(:buildpack) }
185+
186+
it 'preserves the apps lifecycle' do
187+
app_apply_manifest.apply(app.guid, message)
188+
expect(AppUpdate).to have_received(:new).with(user_audit_info, manifest_triggered: true)
189+
expect(app_update).to have_received(:update).
190+
with(app, app_update_message, instance_of(AppBuildpackLifecycle))
191+
expect(app.reload.lifecycle_type).to eq('buildpack')
192+
end
193+
end
194+
end
195+
196+
context 'buildpack apps' do
197+
let(:buildpack) { VCAP::CloudController::Buildpack.make }
198+
let(:message) { AppManifestMessage.create_from_yml({ name: 'blah' }) }
199+
let(:app_update_message) { message.app_update_message }
200+
let(:app) { AppModel.make(:buildpack) }
201+
202+
before do
203+
TestConfig.override(default_app_lifecycle: 'buildpack')
204+
end
205+
206+
context 'when the default_app_lifecycle is set and the the lifecycle is not specified' do
207+
it 'preserves the lifecycle' do
208+
app_apply_manifest.apply(app.guid, message)
209+
expect(AppUpdate).to have_received(:new).with(user_audit_info, manifest_triggered: true)
210+
expect(app_update).to have_received(:update).
211+
with(app, app_update_message, instance_of(AppBuildpackLifecycle))
212+
expect(app.reload.lifecycle_type).to eq('buildpack')
213+
end
214+
end
215+
216+
context 'when buildpack is specified' do
217+
let(:message) { AppManifestMessage.create_from_yml({ name: 'blah', buildpack: buildpack.name }) }
218+
219+
it 'preserves the lifecycle' do
220+
app_apply_manifest.apply(app.guid, message)
221+
expect(AppUpdate).to have_received(:new).with(user_audit_info, manifest_triggered: true)
222+
expect(app_update).to have_received(:update).
223+
with(app, app_update_message, instance_of(AppBuildpackLifecycle))
224+
expect(app.reload.lifecycle_type).to eq('buildpack')
225+
end
226+
end
227+
228+
context 'when the default differs from what is already set on the app' do
229+
let(:message) { AppManifestMessage.create_from_yml({ name: 'blah', buildpack: buildpack.name }) }
230+
let(:app) { AppModel.make(:cnb) }
231+
232+
it 'preserves the apps lifecycle' do
233+
app_apply_manifest.apply(app.guid, message)
234+
expect(AppUpdate).to have_received(:new).with(user_audit_info, manifest_triggered: true)
235+
expect(app_update).to have_received(:update).
236+
with(app, app_update_message, instance_of(AppCNBLifecycle))
237+
expect(app.reload.lifecycle_type).to eq('cnb')
238+
end
239+
end
240+
end
241+
150242
describe 'updating stack' do
151243
let(:message) { AppManifestMessage.create_from_yml({ name: 'stack-test', stack: 'cflinuxfs4' }) }
152244
let(:app_update_message) { message.app_update_message }

spec/unit/jobs/deserialization_spec.rb

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -178,7 +178,6 @@ module Jobs
178178
- :lifecycle
179179
extra_keys: []
180180
lifecycle:
181-
:type: buildpack
182181
:data:
183182
:buildpacks:
184183
- ruby

spec/unit/messages/app_manifest_message_spec.rb

Lines changed: 37 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1064,6 +1064,7 @@ module VCAP::CloudController
10641064
message = AppManifestMessage.create_from_yml(params_from_yaml)
10651065

10661066
expect(message).not_to be_valid
1067+
10671068
expect(message.errors).to have(1).items
10681069
expect(message.errors.full_messages).to include('Cannot specify both buildpack(s) and docker keys')
10691070
end
@@ -1995,7 +1996,6 @@ module VCAP::CloudController
19951996

19961997
it 'does not forward missing attributes to the AppUpdateMessage' do
19971998
message = AppManifestMessage.create_from_yml(parsed_yaml)
1998-
19991999
expect(message.app_update_message.requested?(:lifecycle)).to be false
20002000
end
20012001
end
@@ -2105,6 +2105,42 @@ module VCAP::CloudController
21052105
end
21062106
end
21072107

2108+
context 'when buildpack is the default_app_lifecycle but lifecycle is not set' do
2109+
before do
2110+
TestConfig.override(default_app_lifecycle: 'buildpack')
2111+
end
2112+
2113+
let(:parsed_yaml) { { name: 'cnb', buildpacks: %w[nodejs java], stack: stack.name } }
2114+
2115+
it 'sets the app lifecycle to nil' do
2116+
message = AppManifestMessage.create_from_yml(parsed_yaml)
2117+
2118+
expect(message).to be_valid
2119+
expect(message.app_update_message.lifecycle_type).to eq(nil)
2120+
expect(message.app_update_message.buildpack_data.buildpacks).to eq(%w[nodejs java])
2121+
expect(message.app_update_message.buildpack_data.stack).to eq(stack.name)
2122+
expect(message.app_update_message.buildpack_data.credentials).to be_nil
2123+
end
2124+
end
2125+
2126+
context 'when cnb is the default_app_lifecycle but lifecycle is not set' do
2127+
before do
2128+
TestConfig.override(default_app_lifecycle: 'cnb')
2129+
end
2130+
2131+
let(:parsed_yaml) { { name: 'cnb', buildpacks: %w[nodejs java], stack: stack.name } }
2132+
2133+
it 'sets the app lifecycle to cnb' do
2134+
message = AppManifestMessage.create_from_yml(parsed_yaml)
2135+
2136+
expect(message).to be_valid
2137+
expect(message.app_update_message.lifecycle_type).to eq(nil)
2138+
expect(message.app_update_message.buildpack_data.buildpacks).to eq(%w[nodejs java])
2139+
expect(message.app_update_message.buildpack_data.stack).to eq(stack.name)
2140+
expect(message.app_update_message.buildpack_data.credentials).to be_nil
2141+
end
2142+
end
2143+
21082144
context 'when cnb is disabled' do
21092145
before do
21102146
FeatureFlag.make(name: 'diego_cnb', enabled: false, error_message: 'I am a banana')

0 commit comments

Comments
 (0)