diff --git a/lib/cloud_controller/diego/lifecycles/app_base_lifecycle.rb b/lib/cloud_controller/diego/lifecycles/app_base_lifecycle.rb index 1c2cffdf67d..e94fd6308b4 100644 --- a/lib/cloud_controller/diego/lifecycles/app_base_lifecycle.rb +++ b/lib/cloud_controller/diego/lifecycles/app_base_lifecycle.rb @@ -7,7 +7,8 @@ class AppBaseLifecycle def initialize(message, app=nil) @message = message - db_result = BuildpackLifecycleFetcher.fetch(effective_buildpacks(app), stack, type) + validation_data = lifecycle_data_for_validation(app) + db_result = BuildpackLifecycleFetcher.fetch(validation_data[:buildpacks], validation_data[:stack], type) @validator = validator_class.new( buildpack_infos: db_result[:buildpack_infos], @@ -37,8 +38,22 @@ def validator_class BuildpackLifecycleDataValidator end - def effective_buildpacks(app) - message.buildpack_data.requested?(:buildpacks) ? buildpacks : (app&.lifecycle_data&.buildpacks || []) + def lifecycle_data_for_validation(app) + return { buildpacks:, stack: } unless app && validating_stored_buildpacks? + + stored_data = app.lifecycle_data + + { + buildpacks: stored_data.buildpacks || [], + stack: message.buildpack_data.requested?(:stack) ? stack : (stored_data.stack || stack) + } + end + + def validating_stored_buildpacks? + # Stored URLs are needed to detect embedded userinfo when credentials are added separately. + message.buildpack_data.requested?(:credentials) && + message.buildpack_data.credentials.present? && + !message.buildpack_data.requested?(:buildpacks) end def effective_credentials(app) diff --git a/spec/unit/lib/cloud_controller/diego/lifecycles/app_buildpack_lifecycle_spec.rb b/spec/unit/lib/cloud_controller/diego/lifecycles/app_buildpack_lifecycle_spec.rb index 5bcabd74861..49e06cf0fa0 100644 --- a/spec/unit/lib/cloud_controller/diego/lifecycles/app_buildpack_lifecycle_spec.rb +++ b/spec/unit/lib/cloud_controller/diego/lifecycles/app_buildpack_lifecycle_spec.rb @@ -134,6 +134,57 @@ module VCAP::CloudController end end + describe 'validation of stored lifecycle data' do + let(:stored_stack) { create(:stack, name: 'some-stack') } + let(:app) { create(:app_model, :buildpack) } + let!(:stored_buildpack) { create(:buildpack, name: 'some-buildpack', stack: stored_stack.name) } + + before do + app.lifecycle_data.update(buildpacks: ['some-buildpack'], stack: stored_stack.name) + end + + context 'when the request does not include lifecycle data' do + let(:message) { AppUpdateMessage.new(lifecycle: { type: 'buildpack', data: {} }) } + + it 'does not validate stored buildpacks' do + expect(BuildpackLifecycleFetcher).to receive(:fetch).with([], Stack.default.name, Lifecycles::BUILDPACK).and_call_original + + lifecycle = AppBuildpackLifecycle.new(message, app) + + expect(lifecycle).to be_valid + end + end + + context 'when the request clears lifecycle credentials' do + let(:message) { AppUpdateMessage.new(lifecycle: { type: 'buildpack', data: { credentials: nil } }) } + + it 'does not validate stored buildpacks' do + expect(BuildpackLifecycleFetcher).to receive(:fetch).with([], Stack.default.name, Lifecycles::BUILDPACK).and_call_original + + lifecycle = AppBuildpackLifecycle.new(message, app) + + expect(lifecycle).to be_valid + end + end + + context 'when credentials are updated without buildpacks' do + let(:message) do + AppUpdateMessage.new(lifecycle: { + type: 'buildpack', + data: { credentials: { 'buildpacks.example.org' => { username: 'user', password: 'password' } } } + }) + end + + it 'validates the stored buildpacks against the stored stack' do + expect(BuildpackLifecycleFetcher).to receive(:fetch).with(['some-buildpack'], stored_stack.name, Lifecycles::BUILDPACK).and_call_original + + lifecycle = AppBuildpackLifecycle.new(message, app) + + expect(lifecycle).to be_valid + end + end + end + describe '#create_lifecycle_data_model with credentials' do context 'when credentials are provided' do let(:creds) { { 'git.example.com' => { 'username' => 'u', 'password' => 'p' } } }