From b80b4fe93dd805b7e47d534985f63b85490855d9 Mon Sep 17 00:00:00 2001 From: Philipp Thun Date: Mon, 20 Apr 2026 15:28:35 +0200 Subject: [PATCH 1/3] Remove unused kpack lifecycle code Kpack is dead code - the API validator rejects lifecycle_type 'kpack', no LifecycleProvider support exists, and the infrastructure was never completed. This removes the model, associations, blueprints, and tests. DB migrations for kpack_lifecycle_data table are intentionally retained for a future cleanup. --- app/models.rb | 1 - app/models/runtime/app_model.rb | 6 -- app/models/runtime/build_model.rb | 6 +- app/models/runtime/droplet_model.rb | 5 -- .../runtime/kpack_lifecycle_data_model.rb | 54 ------------- spec/request/builds_spec.rb | 77 ------------------- spec/request/droplets_spec.rb | 2 +- spec/support/fakes/blueprints.rb | 43 ----------- spec/unit/actions/build_delete_spec.rb | 10 --- spec/unit/actions/build_update_spec.rb | 46 ----------- spec/unit/actions/droplet_update_spec.rb | 2 +- .../messages/build_update_message_spec.rb | 4 +- spec/unit/models/runtime/build_model_spec.rb | 10 --- .../unit/models/runtime/droplet_model_spec.rb | 20 +---- .../kpack_lifecycle_data_model_spec.rb | 33 -------- 15 files changed, 6 insertions(+), 313 deletions(-) delete mode 100644 app/models/runtime/kpack_lifecycle_data_model.rb delete mode 100644 spec/unit/models/runtime/kpack_lifecycle_data_model_spec.rb diff --git a/app/models.rb b/app/models.rb index 93e1594b38d..d6f1418c79e 100644 --- a/app/models.rb +++ b/app/models.rb @@ -8,7 +8,6 @@ require 'models/runtime/droplet_model' require 'models/runtime/buildpack_lifecycle_data_model' require 'models/runtime/buildpack_lifecycle_buildpack_model' -require 'models/runtime/kpack_lifecycle_data_model' require 'models/runtime/cnb_lifecycle_data_model' require 'models/runtime/docker_lifecycle_data_model' require 'models/runtime/task_model' diff --git a/app/models/runtime/app_model.rb b/app/models/runtime/app_model.rb index b02e1550c56..f2a35d17599 100644 --- a/app/models/runtime/app_model.rb +++ b/app/models/runtime/app_model.rb @@ -52,11 +52,6 @@ class AppModel < Sequel::Model(:apps) key: :app_guid, primary_key: :guid - one_to_one :kpack_lifecycle_data, - class: 'VCAP::CloudController::KpackLifecycleDataModel', - key: :app_guid, - primary_key: :guid - one_to_one :cnb_lifecycle_data, class: 'VCAP::CloudController::CNBLifecycleDataModel', key: :app_guid, @@ -66,7 +61,6 @@ class AppModel < Sequel::Model(:apps) serializes_via_json :environment_variables add_association_dependencies buildpack_lifecycle_data: :destroy - add_association_dependencies kpack_lifecycle_data: :destroy add_association_dependencies cnb_lifecycle_data: :destroy add_association_dependencies labels: :destroy add_association_dependencies annotations: :destroy diff --git a/app/models/runtime/build_model.rb b/app/models/runtime/build_model.rb index abc5310866d..617b0b52b21 100644 --- a/app/models/runtime/build_model.rb +++ b/app/models/runtime/build_model.rb @@ -35,10 +35,6 @@ class BuildModel < Sequel::Model(:builds) class: 'VCAP::CloudController::BuildpackLifecycleDataModel', key: :build_guid, primary_key: :guid - one_to_one :kpack_lifecycle_data, - class: 'VCAP::CloudController::KpackLifecycleDataModel', - key: :build_guid, - primary_key: :guid one_to_one :cnb_lifecycle_data, class: 'VCAP::CloudController::CNBLifecycleDataModel', key: :build_guid, @@ -49,7 +45,7 @@ class BuildModel < Sequel::Model(:builds) one_to_many :labels, class: 'VCAP::CloudController::BuildLabelModel', key: :resource_guid, primary_key: :guid one_to_many :annotations, class: 'VCAP::CloudController::BuildAnnotationModel', key: :resource_guid, primary_key: :guid - add_association_dependencies buildpack_lifecycle_data: :destroy, kpack_lifecycle_data: :destroy, cnb_lifecycle_data: :destroy + add_association_dependencies buildpack_lifecycle_data: :destroy, cnb_lifecycle_data: :destroy add_association_dependencies labels: :destroy add_association_dependencies annotations: :destroy diff --git a/app/models/runtime/droplet_model.rb b/app/models/runtime/droplet_model.rb index 9716d1c8585..b4ddf9bf7f1 100644 --- a/app/models/runtime/droplet_model.rb +++ b/app/models/runtime/droplet_model.rb @@ -33,10 +33,6 @@ class DropletModel < Sequel::Model(:droplets) class: 'VCAP::CloudController::BuildpackLifecycleDataModel', key: :droplet_guid, primary_key: :guid - one_to_one :kpack_lifecycle_data, - class: 'VCAP::CloudController::KpackLifecycleDataModel', - key: :droplet_guid, - primary_key: :guid one_to_one :cnb_lifecycle_data, class: 'VCAP::CloudController::CNBLifecycleDataModel', key: :droplet_guid, @@ -45,7 +41,6 @@ class DropletModel < Sequel::Model(:droplets) one_to_many :annotations, class: 'VCAP::CloudController::DropletAnnotationModel', key: :resource_guid, primary_key: :guid add_association_dependencies buildpack_lifecycle_data: :destroy - add_association_dependencies kpack_lifecycle_data: :destroy add_association_dependencies cnb_lifecycle_data: :destroy add_association_dependencies labels: :destroy add_association_dependencies annotations: :destroy diff --git a/app/models/runtime/kpack_lifecycle_data_model.rb b/app/models/runtime/kpack_lifecycle_data_model.rb deleted file mode 100644 index a60403d1bb3..00000000000 --- a/app/models/runtime/kpack_lifecycle_data_model.rb +++ /dev/null @@ -1,54 +0,0 @@ -require 'cloud_controller/diego/lifecycles/lifecycles' - -module VCAP::CloudController - class KpackLifecycleDataModel < Sequel::Model(:kpack_lifecycle_data) - include Serializer - - many_to_one :app, - class: '::VCAP::CloudController::AppModel', - key: :app_guid, - primary_key: :guid, - without_guid_generation: true - - many_to_one :droplet, - class: '::VCAP::CloudController::DropletModel', - key: :droplet_guid, - primary_key: :guid, - without_guid_generation: true - - many_to_one :build, - class: '::VCAP::CloudController::BuildModel', - key: :build_guid, - primary_key: :guid, - without_guid_generation: true - - # if this gets any thicker, we should model it properly in its own table - serializes_via_json :buildpacks - - def using_custom_buildpack? - false - end - - def buildpack_models - [] - end - - def first_custom_buildpack_url - nil - end - - # def buildpacks=(new_buildpacks) end - - def to_hash - { - buildpacks: - } - end - - def stack - nil - end - - def stack=(new_value) end - end -end diff --git a/spec/request/builds_spec.rb b/spec/request/builds_spec.rb index 5a88e6b7795..89b3fa2bc70 100644 --- a/spec/request/builds_spec.rb +++ b/spec/request/builds_spec.rb @@ -748,83 +748,6 @@ it_behaves_like 'permissions for single object endpoint', ALL_PERMISSIONS end end - - context 'updating state' do - let(:build_model) do - VCAP::CloudController::BuildModel.make(:kpack, package: package_model, - state: VCAP::CloudController::BuildModel::STAGING_STATE, app: app_model) - end - let(:request) do - { - state: 'STAGED', - lifecycle: { - type: 'kpack', - data: { - image: 'some-fake-image:tag' - } - } - } - end - - it 'allows admins to update the state' do - patch "/v3/builds/#{build_model.guid}", request.to_json, admin_headers - expect(last_response.status).to eq(200), last_response.body - expect(build_model.reload.state).to eq('STAGED') - parsed_response = Oj.load(last_response.body) - expect(parsed_response['state']).to eq('STAGED') - end - - context 'when the cloud_controller.update_build_state scope is present' do - context 'when a build was successfully completed' do - it 'updates the state to STAGED' do - patch "/v3/builds/#{build_model.guid}", request.to_json, build_state_updater_headers - parsed_response = Oj.load(last_response.body) - expect(last_response.status).to eq(200) - - expect(build_model.reload.state).to eq('STAGED') - expect(parsed_response['state']).to eq('STAGED') - end - - it 'creates a droplet with the appropriate image reference' do - patch "/v3/builds/#{build_model.guid}", request.to_json, build_state_updater_headers - - expect(build_model.reload.droplet.docker_receipt_image).to eq('some-fake-image:tag') - expect(build_model.reload.droplet.state).to eq('STAGED') - end - end - - context 'when a build failed to complete' do - let(:request) do - { - state: 'FAILED', - error: 'failed to stage build' - } - end - - it 'returns 200' do - patch "/v3/builds/#{build_model.guid}", request.to_json, build_state_updater_headers - expect(last_response.status).to eq(200), last_response.body - end - end - end - - context 'when the cloud_controller.update_build_state scope is NOT present' do - it '403s' do - patch "/v3/builds/#{build_model.guid}", { state: 'STAGED' }.to_json, developer_headers - expect(last_response.status).to eq(403), last_response.body - end - end - - context 'when the the developer is looking in the wrong space' do - let(:wrong_developer) { make_developer_for_space(VCAP::CloudController::Space.make) } - let(:wrong_developer_headers) { headers_for(wrong_developer, user_name: user_name, email: 'bob@loblaw.com') } - - it '404s' do - patch "/v3/builds/#{build_model.guid}", { state: 'STAGED' }.to_json, wrong_developer_headers - expect(last_response.status).to eq(404), last_response.body - end - end - end end end end diff --git a/spec/request/droplets_spec.rb b/spec/request/droplets_spec.rb index fb60d1d9df6..0db30447dbe 100644 --- a/spec/request/droplets_spec.rb +++ b/spec/request/droplets_spec.rb @@ -1924,7 +1924,7 @@ let(:rebased_image_reference) { 'rebased-image-reference' } let!(:og_docker_droplet) do VCAP::CloudController::DropletModel.make( - :kpack, + :docker, state: VCAP::CloudController::DropletModel::STAGED_STATE, app_guid: app_model.guid, package_guid: package_model.guid, diff --git a/spec/support/fakes/blueprints.rb b/spec/support/fakes/blueprints.rb index 86a528857b0..480787a3e66 100644 --- a/spec/support/fakes/blueprints.rb +++ b/spec/support/fakes/blueprints.rb @@ -55,13 +55,6 @@ module VCAP::CloudController buildpack_cache_sha256_checksum { Sham.guid } end - AppModel.blueprint(:kpack) do - name { Sham.name } - space { Space.make } - buildpack_lifecycle_data { nil.tap { |_| object.save } } - kpack_lifecycle_data { KpackLifecycleDataModel.make(app: object.save) } - end - AppModel.blueprint(:cnb) do name { Sham.name } space { Space.make } @@ -89,15 +82,6 @@ module VCAP::CloudController buildpack_lifecycle_data { nil.tap { |_| object.save } } end - BuildModel.blueprint(:kpack) do - guid { Sham.guid } - state { VCAP::CloudController::DropletModel::STAGING_STATE } - app { AppModel.make(droplet: DropletModel.make) } - kpack_lifecycle_data { KpackLifecycleDataModel.make(build: object.save) } - package { PackageModel.make(app:) } - droplet { DropletModel.make(:docker, build: object.save) } - end - BuildModel.blueprint(:buildpack) do guid { Sham.guid } state { VCAP::CloudController::DropletModel::STAGING_STATE } @@ -164,7 +148,6 @@ module VCAP::CloudController app { AppModel.make(:cnb, droplet: object.save) } cnb_lifecycle_data { CNBLifecycleDataModel.make(droplet: object.save) } buildpack_lifecycle_data { nil.tap { |_| object.save } } - kpack_lifecycle_data { nil.tap { |_| object.save } } end DropletModel.blueprint(:docker) do @@ -174,18 +157,6 @@ module VCAP::CloudController state { VCAP::CloudController::DropletModel::STAGED_STATE } app { AppModel.make(droplet: object.save) } buildpack_lifecycle_data { nil.tap { |_| object.save } } - kpack_lifecycle_data { nil.tap { |_| object.save } } - end - - DropletModel.blueprint(:kpack) do - guid { Sham.guid } - droplet_hash { nil } - sha256_checksum { nil } - docker_receipt_image { nil } - app { AppModel.make(:kpack, droplet: object.save) } - state { VCAP::CloudController::DropletModel::STAGED_STATE } - buildpack_lifecycle_data { nil.tap { |_| object.save } } - kpack_lifecycle_data { KpackLifecycleDataModel.make(droplet: object.save) } end DeploymentModel.blueprint do @@ -532,15 +503,6 @@ module VCAP::CloudController metadata { {} } end - ProcessModel.blueprint(:kpack) do - app { AppModel.make(:kpack, droplet: DropletModel.make(:kpack)) } - diego { true } - instances { 1 } - type { Sham.name } - metadata { {} } - state { 'STARTED' } - end - ProcessModel.blueprint(:nonmatching_guid) do instances { 1 } type { 'web' } @@ -710,11 +672,6 @@ module VCAP::CloudController build { BuildModel.make } end - KpackLifecycleDataModel.blueprint do - build { BuildModel.make } - buildpacks { [] } - end - BuildpackLifecycleBuildpackModel.blueprint do admin_buildpack_name { Buildpack.make(name: 'ruby').name } buildpack_url { nil } diff --git a/spec/unit/actions/build_delete_spec.rb b/spec/unit/actions/build_delete_spec.rb index f676d4c4d19..d086a2adc15 100644 --- a/spec/unit/actions/build_delete_spec.rb +++ b/spec/unit/actions/build_delete_spec.rb @@ -60,16 +60,6 @@ module VCAP::CloudController end.to change(BuildpackLifecycleDataModel, :count).by(-2).and change(BuildpackLifecycleBuildpackModel, :count).by(-2) [lifecycle_data1, lifecycle_data2, lifecycle_buildpack1, lifecycle_buildpack2].each { |l| expect(l).not_to exist } end - - it 'deletes associated kpack lifecycle data' do - lifecycle1 = KpackLifecycleDataModel.make(build: build1) - lifecycle2 = KpackLifecycleDataModel.make(build: build2) - - expect do - build_delete.delete_for_app(app.guid) - end.to change(KpackLifecycleDataModel, :count).by(-2) - [lifecycle1, lifecycle2].each { |l| expect(l).not_to exist } - end end end end diff --git a/spec/unit/actions/build_update_spec.rb b/spec/unit/actions/build_update_spec.rb index 12a91bf99e5..c8fb38e8a04 100644 --- a/spec/unit/actions/build_update_spec.rb +++ b/spec/unit/actions/build_update_spec.rb @@ -125,52 +125,6 @@ module VCAP::CloudController end end end - - context 'when updating state' do - let(:build) { BuildModel.make(:kpack) } - - context 'when a build was successfully completed' do - let(:body) do - { - state: 'STAGED', - lifecycle: { - type: 'kpack', - data: { - image: 'some-fake-image:tag', - processTypes: { - foo: 'foo start', - bar: 'bar start' - } - } - } - } - end - - it 'updates the build state as STAGED and updates the droplet with correct metadata' do - build_update.update(build, message) - - expect(build.state).to eq('STAGED') - expect(build.droplet.state).to eq('STAGED') - expect(build.droplet.docker_receipt_image).to eq('some-fake-image:tag') - expect(build.droplet.process_types).to eq({ 'foo' => 'foo start', 'bar' => 'bar start' }) - end - end - - context 'when the state is FAILED' do - let(:body) do - { - state: 'FAILED', - error: 'failed to stage build' - } - end - - it 'updates the state to FAILED' do - build_update.update(build, message) - expect(build.state).to eq 'FAILED' - expect(build.error_description).to include 'failed to stage build' - end - end - end end end end diff --git a/spec/unit/actions/droplet_update_spec.rb b/spec/unit/actions/droplet_update_spec.rb index 0737cab6106..ea7c0c35ddd 100644 --- a/spec/unit/actions/droplet_update_spec.rb +++ b/spec/unit/actions/droplet_update_spec.rb @@ -57,7 +57,7 @@ module VCAP::CloudController context 'image updates' do context 'when the droplet is not STAGED' do - let!(:droplet) { DropletModel.make(:kpack, state: VCAP::CloudController::DropletModel::STAGING_STATE, app: nil) } + let!(:droplet) { DropletModel.make(:docker, state: VCAP::CloudController::DropletModel::STAGING_STATE, app: nil) } let(:message) do VCAP::CloudController::DropletUpdateMessage.new({ diff --git a/spec/unit/messages/build_update_message_spec.rb b/spec/unit/messages/build_update_message_spec.rb index a9c5c49a9a0..718897a2186 100644 --- a/spec/unit/messages/build_update_message_spec.rb +++ b/spec/unit/messages/build_update_message_spec.rb @@ -46,7 +46,7 @@ module VCAP::CloudController state: 'STAGED', error: 'error', lifecycle: { - type: 'kpack', + type: 'docker', data: { image: 'some-image:tag' } @@ -65,7 +65,7 @@ module VCAP::CloudController { state: 'STAGED', lifecycle: { - type: 'kpack', + type: 'docker', data: { image: 'some-image:tag' } diff --git a/spec/unit/models/runtime/build_model_spec.rb b/spec/unit/models/runtime/build_model_spec.rb index 1655ab6b195..ad40897a054 100644 --- a/spec/unit/models/runtime/build_model_spec.rb +++ b/spec/unit/models/runtime/build_model_spec.rb @@ -173,16 +173,6 @@ module VCAP::CloudController and change(BuildpackLifecycleBuildpackModel, :count).by(-2) end end - - context 'kpack dependencies' do - let!(:lifecycle_data) { KpackLifecycleDataModel.make(build: build_model) } - - it 'deletes the dependent kpack_lifecycle_data_models when a build is deleted' do - expect do - build_model.destroy - end.to change(KpackLifecycleDataModel, :count).by(-1) - end - end end describe '#staged?' do diff --git a/spec/unit/models/runtime/droplet_model_spec.rb b/spec/unit/models/runtime/droplet_model_spec.rb index 7da70f7f4af..554ae2ee192 100644 --- a/spec/unit/models/runtime/droplet_model_spec.rb +++ b/spec/unit/models/runtime/droplet_model_spec.rb @@ -140,26 +140,8 @@ module VCAP::CloudController end end - context 'when there is kpack_lifecycle_data associated to the droplet' do - let(:droplet_model) { DropletModel.make(:kpack, app: nil) } - let!(:lifecycle_data) do - KpackLifecycleDataModel.make(droplet: droplet_model) - end - - before do - droplet_model.kpack_lifecycle_data = lifecycle_data - droplet_model.save - end - - it 'deletes the dependent kpack_lifecycle_data_models when a droplet is deleted' do - expect do - droplet_model.destroy - end.to change(KpackLifecycleDataModel, :count).by(-1) - end - end - context 'when there is cnb_lifecycle_data associated to the droplet' do - let(:droplet_model) { DropletModel.make(:kpack, app: nil) } + let(:droplet_model) { DropletModel.make(:cnb, app: nil) } let!(:lifecycle_data) do CNBLifecycleDataModel.make( droplet: droplet_model, diff --git a/spec/unit/models/runtime/kpack_lifecycle_data_model_spec.rb b/spec/unit/models/runtime/kpack_lifecycle_data_model_spec.rb deleted file mode 100644 index dd908b58af0..00000000000 --- a/spec/unit/models/runtime/kpack_lifecycle_data_model_spec.rb +++ /dev/null @@ -1,33 +0,0 @@ -require 'spec_helper' - -module VCAP::CloudController - RSpec.describe KpackLifecycleDataModel do - subject(:lifecycle_data) { KpackLifecycleDataModel.new } - - describe '#to_hash' do - let(:expected_lifecycle_data) do - { buildpacks: buildpacks || [] } - end - let(:buildpacks) { [buildpack] } - let(:buildpack) { 'ruby' } - - before do - Buildpack.make(name: 'ruby') - lifecycle_data.buildpacks = buildpacks - lifecycle_data.save - end - - it 'returns the lifecycle data as a hash' do - expect(lifecycle_data.to_hash).to eq expected_lifecycle_data - end - - context 'when the user has not specified a buildpack' do - let(:buildpacks) { [] } - - it 'returns the lifecycle data as a hash' do - expect(lifecycle_data.to_hash).to eq expected_lifecycle_data - end - end - end - end -end From 07233e85581d7d9c67b050e66f9c5c67d301c732 Mon Sep 17 00:00:00 2001 From: Philipp Thun Date: Tue, 28 Apr 2026 12:03:31 +0200 Subject: [PATCH 2/3] Add rubocop cop to prevent model usage in migration specs Migration specs roll back the schema before running, so model classes may reference columns that don't exist yet. Using raw Sequel operations ensures specs are independent of the current model code, as documented in spec/migrations/Readme.md. - Add Migration/NoModelInSpecs cop that flags any method call on a *Model constant receiver in spec/migrations/ - Replace model .make calls with raw db[:table].insert in four migration specs: sidecars, revision_sidecars, sidecar_process_types, and revision_process_commands --- .rubocop_cc.yml | 7 ++ spec/linters/migration/no_model_in_specs.rb | 25 +++++++ .../migration/no_model_in_specs_spec.rb | 74 +++++++++++++++++++ ...onstraint_to_sidecar_process_types_spec.rb | 37 ++++++---- ...ue_constraint_to_revision_sidecars_spec.rb | 19 +++-- ..._add_unique_constraint_to_sidecars_spec.rb | 16 ++-- ...raint_to_revision_process_commands_spec.rb | 18 +++-- 7 files changed, 158 insertions(+), 38 deletions(-) create mode 100644 spec/linters/migration/no_model_in_specs.rb create mode 100644 spec/linters/migration/no_model_in_specs_spec.rb diff --git a/.rubocop_cc.yml b/.rubocop_cc.yml index bc55373b36f..5e68caa7463 100644 --- a/.rubocop_cc.yml +++ b/.rubocop_cc.yml @@ -11,6 +11,7 @@ plugins: require: - ./spec/linters/migration/add_constraint_name.rb - ./spec/linters/migration/include_string_size.rb + - ./spec/linters/migration/no_model_in_specs.rb - ./spec/linters/migration/require_primary_key.rb - ./spec/linters/migration/too_many_migration_runs.rb - ./spec/linters/match_requires_with_includes.rb @@ -67,6 +68,12 @@ Migration/IncludeStringSize: # Exclude for old Migrations Exclude: - !ruby/regexp /db/migrations/201([0-9]).+\.rb$/ - !ruby/regexp /db/migrations/202([0-2]).+\.rb$/ +Migration/NoModelInSpecs: + Include: + - spec/migrations/**/* + Exclude: + # Uses custom tmp_migrations_dir so schema is never rolled back + - spec/migrations/20190712210940_backfill_status_for_deployments_spec.rb Migration/RequirePrimaryKey: # Exclude for old Migrations Include: - db/migrations/**/* diff --git a/spec/linters/migration/no_model_in_specs.rb b/spec/linters/migration/no_model_in_specs.rb new file mode 100644 index 00000000000..4b8fb31f182 --- /dev/null +++ b/spec/linters/migration/no_model_in_specs.rb @@ -0,0 +1,25 @@ +module RuboCop + module Cop + module Migration + class NoModelInSpecs < RuboCop::Cop::Base + MSG = 'Do not use model classes in migration specs. ' \ + 'Use raw Sequel operations (e.g. db[:table].insert) instead. ' \ + 'See spec/migrations/Readme.md for details.'.freeze + + def on_send(node) + add_offense(node) if model_receiver?(node.receiver) + end + + private + + def model_receiver?(receiver) + return false unless receiver + return false unless receiver.const_type? + + name = receiver.const_name.to_s + name.end_with?('Model') && name != 'Sequel::Model' + end + end + end + end +end diff --git a/spec/linters/migration/no_model_in_specs_spec.rb b/spec/linters/migration/no_model_in_specs_spec.rb new file mode 100644 index 00000000000..d77c864e674 --- /dev/null +++ b/spec/linters/migration/no_model_in_specs_spec.rb @@ -0,0 +1,74 @@ +require 'spec_helper' +require 'rubocop' +require 'rubocop/rspec/cop_helper' +require 'rubocop/config' +require 'linters/migration/no_model_in_specs' + +RSpec.describe RuboCop::Cop::Migration::NoModelInSpecs do + include CopHelper + + subject(:cop) { described_class.new(RuboCop::Config.new({})) } + + let(:message) do + 'Do not use model classes in migration specs. ' \ + 'Use raw Sequel operations (e.g. db[:table].insert) instead. ' \ + 'See spec/migrations/Readme.md for details.' + end + + it 'registers an offense for Model.make' do + result = inspect_source(<<~RUBY) + VCAP::CloudController::AppModel.make + RUBY + + expect(result.size).to eq(1) + expect(result.map(&:message)).to eq([message]) + end + + it 'registers an offense for Model.make!' do + result = inspect_source(<<~RUBY) + VCAP::CloudController::AppModel.make! + RUBY + + expect(result.size).to eq(1) + end + + it 'registers an offense for Model.create' do + result = inspect_source(<<~RUBY) + VCAP::CloudController::DeploymentModel.create(guid: 'test') + RUBY + + expect(result.size).to eq(1) + end + + it 'registers an offense for Model.where' do + result = inspect_source(<<~RUBY) + VCAP::CloudController::AppModel.where(guid: 'test').first + RUBY + + expect(result.size).to eq(1) + end + + it 'does not register an offense for raw Sequel inserts' do + result = inspect_source(<<~RUBY) + db[:apps].insert(guid: 'test', name: 'test-app') + RUBY + + expect(result.size).to eq(0) + end + + it 'does not register an offense for Sequel::Model' do + result = inspect_source(<<~RUBY) + Sequel::Model.db + RUBY + + expect(result.size).to eq(0) + end + + it 'does not register an offense for non-Model constants' do + result = inspect_source(<<~RUBY) + SecureRandom.uuid + RUBY + + expect(result.size).to eq(0) + end +end diff --git a/spec/migrations/20251030100000_add_unique_constraint_to_sidecar_process_types_spec.rb b/spec/migrations/20251030100000_add_unique_constraint_to_sidecar_process_types_spec.rb index 51f9ea639dc..1186fe84586 100644 --- a/spec/migrations/20251030100000_add_unique_constraint_to_sidecar_process_types_spec.rb +++ b/spec/migrations/20251030100000_add_unique_constraint_to_sidecar_process_types_spec.rb @@ -6,22 +6,27 @@ let(:migration_filename) { '20251030100000_add_unique_constraint_to_sidecar_process_types.rb' } end - let!(:app) { VCAP::CloudController::AppModel.make } - let!(:sidecar) { VCAP::CloudController::SidecarModel.make(app:) } - let!(:revision) { VCAP::CloudController::RevisionModel.make(app:) } - let!(:revision_sidecar) { VCAP::CloudController::RevisionSidecarModel.make(revision:) } + let(:app_guid) { SecureRandom.uuid } + let(:sidecar_guid) { SecureRandom.uuid } + let(:revision_guid) { SecureRandom.uuid } + let(:revision_sidecar_guid) { SecureRandom.uuid } it 'removes duplicates, adds unique constraints, and is reversible' do + db[:apps].insert(guid: app_guid, name: 'test-app') + db[:sidecars].insert(guid: sidecar_guid, name: 'test-sidecar', command: 'command', app_guid: app_guid) + db[:revisions].insert(guid: revision_guid, app_guid: app_guid, description: 'some description') + db[:revision_sidecars].insert(guid: revision_sidecar_guid, name: 'test-sidecar', command: 'command', revision_guid: revision_guid) + # ========================================================================================= # SETUP: Create duplicate entries for both tables to test the de-duplication logic. # ========================================================================================= - db[:sidecar_process_types].insert(sidecar_guid: sidecar.guid, type: 'web', app_guid: app.guid, guid: SecureRandom.uuid) - db[:sidecar_process_types].insert(sidecar_guid: sidecar.guid, type: 'web', app_guid: app.guid, guid: SecureRandom.uuid) - expect(db[:sidecar_process_types].where(sidecar_guid: sidecar.guid, type: 'web').count).to eq(2) + db[:sidecar_process_types].insert(sidecar_guid: sidecar_guid, type: 'web', app_guid: app_guid, guid: SecureRandom.uuid) + db[:sidecar_process_types].insert(sidecar_guid: sidecar_guid, type: 'web', app_guid: app_guid, guid: SecureRandom.uuid) + expect(db[:sidecar_process_types].where(sidecar_guid: sidecar_guid, type: 'web').count).to eq(2) - db[:revision_sidecar_process_types].insert(revision_sidecar_guid: revision_sidecar.guid, type: 'worker', guid: SecureRandom.uuid) - db[:revision_sidecar_process_types].insert(revision_sidecar_guid: revision_sidecar.guid, type: 'worker', guid: SecureRandom.uuid) - expect(db[:revision_sidecar_process_types].where(revision_sidecar_guid: revision_sidecar.guid, type: 'worker').count).to eq(2) + db[:revision_sidecar_process_types].insert(revision_sidecar_guid: revision_sidecar_guid, type: 'worker', guid: SecureRandom.uuid) + db[:revision_sidecar_process_types].insert(revision_sidecar_guid: revision_sidecar_guid, type: 'worker', guid: SecureRandom.uuid) + expect(db[:revision_sidecar_process_types].where(revision_sidecar_guid: revision_sidecar_guid, type: 'worker').count).to eq(2) # ========================================================================================= # UP MIGRATION: Run the migration to apply the unique constraints. @@ -31,13 +36,13 @@ # ========================================================================================= # ASSERT UP MIGRATION: Verify that duplicates are removed and constraints are enforced. # ========================================================================================= - expect(db[:sidecar_process_types].where(sidecar_guid: sidecar.guid, type: 'web').count).to eq(1) + expect(db[:sidecar_process_types].where(sidecar_guid: sidecar_guid, type: 'web').count).to eq(1) expect(db.indexes(:sidecar_process_types)).to include(:sidecar_process_types_sidecar_guid_type_index) - expect { db[:sidecar_process_types].insert(sidecar_guid: sidecar.guid, type: 'web', app_guid: app.guid, guid: SecureRandom.uuid) }.to raise_error(Sequel::UniqueConstraintViolation) + expect { db[:sidecar_process_types].insert(sidecar_guid: sidecar_guid, type: 'web', app_guid: app_guid, guid: SecureRandom.uuid) }.to raise_error(Sequel::UniqueConstraintViolation) - expect(db[:revision_sidecar_process_types].where(revision_sidecar_guid: revision_sidecar.guid, type: 'worker').count).to eq(1) + expect(db[:revision_sidecar_process_types].where(revision_sidecar_guid: revision_sidecar_guid, type: 'worker').count).to eq(1) expect(db.indexes(:revision_sidecar_process_types)).to include(:revision_sidecar_process_types_revision_sidecar_guid_type_index) - expect { db[:revision_sidecar_process_types].insert(revision_sidecar_guid: revision_sidecar.guid, type: 'worker', guid: SecureRandom.uuid) }.to raise_error(Sequel::UniqueConstraintViolation) + expect { db[:revision_sidecar_process_types].insert(revision_sidecar_guid: revision_sidecar_guid, type: 'worker', guid: SecureRandom.uuid) }.to raise_error(Sequel::UniqueConstraintViolation) # ========================================================================================= # TEST IDEMPOTENCY: Running the migration again should not cause any errors. @@ -53,9 +58,9 @@ # ASSERT DOWN MIGRATION: Verify that constraints are removed and duplicates can be re-inserted. # ========================================================================================= expect(db.indexes(:sidecar_process_types)).not_to include(:sidecar_process_types_sidecar_guid_type_index) - expect { db[:sidecar_process_types].insert(sidecar_guid: sidecar.guid, type: 'web', app_guid: app.guid, guid: SecureRandom.uuid) }.not_to raise_error + expect { db[:sidecar_process_types].insert(sidecar_guid: sidecar_guid, type: 'web', app_guid: app_guid, guid: SecureRandom.uuid) }.not_to raise_error expect(db.indexes(:revision_sidecar_process_types)).not_to include(:revision_sidecar_process_types_revision_sidecar_guid_type_index) - expect { db[:revision_sidecar_process_types].insert(revision_sidecar_guid: revision_sidecar.guid, type: 'worker', guid: SecureRandom.uuid) }.not_to raise_error + expect { db[:revision_sidecar_process_types].insert(revision_sidecar_guid: revision_sidecar_guid, type: 'worker', guid: SecureRandom.uuid) }.not_to raise_error end end diff --git a/spec/migrations/20260320141005_add_unique_constraint_to_revision_sidecars_spec.rb b/spec/migrations/20260320141005_add_unique_constraint_to_revision_sidecars_spec.rb index 6dd325426d9..cf427949f09 100644 --- a/spec/migrations/20260320141005_add_unique_constraint_to_revision_sidecars_spec.rb +++ b/spec/migrations/20260320141005_add_unique_constraint_to_revision_sidecars_spec.rb @@ -5,22 +5,25 @@ let(:migration_filename) { '20260320141005_add_unique_constraint_to_revision_sidecars.rb' } end - let!(:app) { VCAP::CloudController::AppModel.make } - let!(:revision) { VCAP::CloudController::RevisionModel.make(:app) } + let(:app_guid) { SecureRandom.uuid } + let(:revision_guid) { SecureRandom.uuid } it 'remove duplicates, add constraint and revert migration' do + db[:apps].insert(guid: app_guid, name: 'test-app') + db[:revisions].insert(guid: revision_guid, app_guid: app_guid, description: 'some description') + # create duplicate entries - db[:revision_sidecars].insert(guid: SecureRandom.uuid, name: 'app', command: 'command', revision_guid: revision.guid) - db[:revision_sidecars].insert(guid: SecureRandom.uuid, name: 'app', command: 'command', revision_guid: revision.guid) - expect(db[:revision_sidecars].where(name: 'app', revision_guid: revision.guid).count).to eq(2) + db[:revision_sidecars].insert(guid: SecureRandom.uuid, name: 'app', command: 'command', revision_guid: revision_guid) + db[:revision_sidecars].insert(guid: SecureRandom.uuid, name: 'app', command: 'command', revision_guid: revision_guid) + expect(db[:revision_sidecars].where(name: 'app', revision_guid: revision_guid).count).to eq(2) # run the migration Sequel::Migrator.run(db, migrations_path, target: current_migration_index, allow_missing_migration_files: true) # verify duplicates are removed and constraint is enforced - expect(db[:revision_sidecars].where(name: 'app', revision_guid: revision.guid).count).to eq(1) + expect(db[:revision_sidecars].where(name: 'app', revision_guid: revision_guid).count).to eq(1) expect(db.indexes(:revision_sidecars)).to include(:revision_sidecars_revision_guid_name_index) - expect { db[:revision_sidecars].insert(guid: SecureRandom.uuid, name: 'app', command: 'command', revision_guid: revision.guid) }.to raise_error(Sequel::UniqueConstraintViolation) + expect { db[:revision_sidecars].insert(guid: SecureRandom.uuid, name: 'app', command: 'command', revision_guid: revision_guid) }.to raise_error(Sequel::UniqueConstraintViolation) # running the migration again should not cause any errors expect { Sequel::Migrator.run(db, migrations_path, target: current_migration_index, allow_missing_migration_files: true) }.not_to raise_error @@ -30,6 +33,6 @@ # verify constraint is removed and duplicates can be re-inserted expect(db.indexes(:revision_sidecars)).not_to include(:revision_sidecars_revision_guid_name_index) - expect { db[:revision_sidecars].insert(guid: SecureRandom.uuid, name: 'app', command: 'command', revision_guid: revision.guid) }.not_to raise_error + expect { db[:revision_sidecars].insert(guid: SecureRandom.uuid, name: 'app', command: 'command', revision_guid: revision_guid) }.not_to raise_error end end diff --git a/spec/migrations/20260323092954_add_unique_constraint_to_sidecars_spec.rb b/spec/migrations/20260323092954_add_unique_constraint_to_sidecars_spec.rb index ca979bc041f..cbc84da70db 100644 --- a/spec/migrations/20260323092954_add_unique_constraint_to_sidecars_spec.rb +++ b/spec/migrations/20260323092954_add_unique_constraint_to_sidecars_spec.rb @@ -5,21 +5,23 @@ let(:migration_filename) { '20260323092954_add_unique_constraint_to_sidecars.rb' } end - let!(:app) { VCAP::CloudController::AppModel.make } + let(:app_guid) { SecureRandom.uuid } it 'remove duplicates, add constraint and revert migration' do + db[:apps].insert(guid: app_guid, name: 'test-app') + # create duplicate entries - db[:sidecars].insert(guid: SecureRandom.uuid, name: 'app', command: 'command', app_guid: app.guid) - db[:sidecars].insert(guid: SecureRandom.uuid, name: 'app', command: 'command', app_guid: app.guid) - expect(db[:sidecars].where(name: 'app', app_guid: app.guid).count).to eq(2) + db[:sidecars].insert(guid: SecureRandom.uuid, name: 'app', command: 'command', app_guid: app_guid) + db[:sidecars].insert(guid: SecureRandom.uuid, name: 'app', command: 'command', app_guid: app_guid) + expect(db[:sidecars].where(name: 'app', app_guid: app_guid).count).to eq(2) # run the migration Sequel::Migrator.run(db, migrations_path, target: current_migration_index, allow_missing_migration_files: true) # verify duplicates are removed and constraint is enforced - expect(db[:sidecars].where(name: 'app', app_guid: app.guid).count).to eq(1) + expect(db[:sidecars].where(name: 'app', app_guid: app_guid).count).to eq(1) expect(db.indexes(:sidecars)).to include(:sidecars_app_guid_name_index) - expect { db[:sidecars].insert(guid: SecureRandom.uuid, name: 'app', command: 'command', app_guid: app.guid) }.to raise_error(Sequel::UniqueConstraintViolation) + expect { db[:sidecars].insert(guid: SecureRandom.uuid, name: 'app', command: 'command', app_guid: app_guid) }.to raise_error(Sequel::UniqueConstraintViolation) # running the migration again should not cause any errors expect { Sequel::Migrator.run(db, migrations_path, target: current_migration_index, allow_missing_migration_files: true) }.not_to raise_error @@ -29,6 +31,6 @@ # verify constraint is removed and duplicates can be re-inserted expect(db.indexes(:sidecars)).not_to include(:sidecars_app_guid_name_index) - expect { db[:sidecars].insert(guid: SecureRandom.uuid, name: 'app', command: 'command', app_guid: app.guid) }.not_to raise_error + expect { db[:sidecars].insert(guid: SecureRandom.uuid, name: 'app', command: 'command', app_guid: app_guid) }.not_to raise_error end end diff --git a/spec/migrations/20260323144429_add_unique_constraint_to_revision_process_commands_spec.rb b/spec/migrations/20260323144429_add_unique_constraint_to_revision_process_commands_spec.rb index f4a59565e90..ae49122ee66 100644 --- a/spec/migrations/20260323144429_add_unique_constraint_to_revision_process_commands_spec.rb +++ b/spec/migrations/20260323144429_add_unique_constraint_to_revision_process_commands_spec.rb @@ -6,22 +6,26 @@ let(:migration_filename) { '20260323144429_add_unique_constraint_to_revision_process_commands.rb' } end - let!(:revision) { VCAP::CloudController::RevisionModel.make } + let(:app_guid) { SecureRandom.uuid } + let(:revision_guid) { SecureRandom.uuid } it 'removes duplicates, adds constraint and reverts migration' do + db[:apps].insert(guid: app_guid, name: 'test-app') + db[:revisions].insert(guid: revision_guid, app_guid: app_guid, description: 'some description') + # create duplicate entries - db[:revision_process_commands].insert(guid: SecureRandom.uuid, revision_guid: revision.guid, process_type: 'worker') - db[:revision_process_commands].insert(guid: SecureRandom.uuid, revision_guid: revision.guid, process_type: 'worker') - expect(db[:revision_process_commands].where(revision_guid: revision.guid, process_type: 'worker').count).to eq(2) + db[:revision_process_commands].insert(guid: SecureRandom.uuid, revision_guid: revision_guid, process_type: 'worker') + db[:revision_process_commands].insert(guid: SecureRandom.uuid, revision_guid: revision_guid, process_type: 'worker') + expect(db[:revision_process_commands].where(revision_guid: revision_guid, process_type: 'worker').count).to eq(2) # run the migration Sequel::Migrator.run(db, migrations_path, target: current_migration_index, allow_missing_migration_files: true) # verify duplicates are removed and constraint is enforced - expect(db[:revision_process_commands].where(revision_guid: revision.guid, process_type: 'worker').count).to eq(1) + expect(db[:revision_process_commands].where(revision_guid: revision_guid, process_type: 'worker').count).to eq(1) expect(db.indexes(:revision_process_commands)).to include(:revision_process_commands_revision_guid_process_type_index) expect do - db[:revision_process_commands].insert(guid: SecureRandom.uuid, revision_guid: revision.guid, process_type: 'worker') + db[:revision_process_commands].insert(guid: SecureRandom.uuid, revision_guid: revision_guid, process_type: 'worker') end.to raise_error(Sequel::UniqueConstraintViolation) # running the migration again should not cause any errors @@ -33,7 +37,7 @@ # verify constraint is removed and duplicates can be re-inserted expect(db.indexes(:revision_process_commands)).not_to include(:revision_process_commands_revision_guid_process_type_index) expect do - db[:revision_process_commands].insert(guid: SecureRandom.uuid, revision_guid: revision.guid, process_type: 'worker') + db[:revision_process_commands].insert(guid: SecureRandom.uuid, revision_guid: revision_guid, process_type: 'worker') end.not_to raise_error end end From ecf03f889ece92a3016a86baf8d306b0e30266f5 Mon Sep 17 00:00:00 2001 From: Philipp Thun Date: Tue, 28 Apr 2026 13:10:35 +0200 Subject: [PATCH 3/3] Add lifecycle_type column to apps, droplets, and builds Store lifecycle_type directly on the model instead of deriving it from associated lifecycle data records. This simplifies queries and makes the lifecycle type explicit. - Add lifecycle_type column to apps, droplets, and builds tables - Set lifecycle_type in create actions (AppCreate, DropletCreate) - Add before_create hooks to builds/droplets to inherit from app - Add model validation for lifecycle_type values - Assign lifecycle_data back to model in lifecycle classes to ensure the in-memory association is set (avoids Sequel caching issues) - Add lifecycle type validation to DropletCopy - Update blueprints to set lifecycle_type explicitly - Fix shared example updated_at filtering to use allow_manual_update and explicit update instead of update_on_create: false - Replace inline timestamp workarounds with shared example usage in apps, droplets, and revisions request specs --- app/actions/app_create.rb | 3 +- app/actions/droplet_copy.rb | 1 + app/actions/droplet_create.rb | 6 +- app/actions/v2/app_create.rb | 3 +- app/models/runtime/app_model.rb | 9 +- app/models/runtime/build_model.rb | 29 +++- app/models/runtime/droplet_model.rb | 15 +- ...260428120000_add_lifecycle_type_to_apps.rb | 29 ++++ ...28120100_add_lifecycle_type_to_droplets.rb | 29 ++++ ...0428120200_add_lifecycle_type_to_builds.rb | 29 ++++ .../lifecycles/app_buildpack_lifecycle.rb | 2 +- .../diego/lifecycles/app_cnb_lifecycle.rb | 2 +- .../diego/lifecycles/buildpack_lifecycle.rb | 2 +- .../diego/lifecycles/cnb_lifecycle.rb | 2 +- ...8120000_add_lifecycle_type_to_apps_spec.rb | 35 +++++ ...100_add_lifecycle_type_to_droplets_spec.rb | 35 +++++ ...20200_add_lifecycle_type_to_builds_spec.rb | 35 +++++ spec/request/apps_spec.rb | 51 ++----- spec/request/droplets_spec.rb | 64 +------- spec/request/revisions_spec.rb | 115 +------------- spec/request_spec_shared_examples.rb | 19 ++- spec/support/fakes/blueprints.rb | 45 ++++-- spec/unit/actions/app_apply_manifest_spec.rb | 4 +- spec/unit/actions/app_create_spec.rb | 1 - spec/unit/actions/app_start_spec.rb | 11 +- spec/unit/actions/build_delete_spec.rb | 8 +- spec/unit/actions/droplet_copy_spec.rb | 11 ++ spec/unit/actions/droplet_create_spec.rb | 35 +++-- spec/unit/actions/droplet_update_spec.rb | 2 +- spec/unit/actions/v2/app_stage_spec.rb | 2 +- spec/unit/actions/v2/app_update_spec.rb | 2 +- .../staging_completion_controller_spec.rb | 4 +- .../runtime/apps_controller_spec.rb | 2 +- .../controllers/v3/apps_controller_spec.rb | 4 +- .../v3/droplets_controller_spec.rb | 2 +- spec/unit/fetchers/app_list_fetcher_spec.rb | 12 +- .../jobs/v3/buildpack_cache_upload_spec.rb | 2 +- spec/unit/jobs/v3/droplet_bits_copier_spec.rb | 4 +- .../diego/app_recipe_builder_spec.rb | 10 +- .../buildpack/staging_action_builder_spec.rb | 2 +- .../diego/cnb/staging_action_builder_spec.rb | 2 +- .../docker/staging_completion_handler_spec.rb | 7 +- .../app_buildpack_lifecycle_spec.rb | 2 +- .../lifecycles/app_cnb_lifecycle_spec.rb | 2 +- .../lifecycles/app_lifecycle_provider_spec.rb | 2 +- .../lifecycles/buildpack_lifecycle_spec.rb | 2 +- .../lifecycles/lifecycle_provider_spec.rb | 2 +- .../lib/cloud_controller/diego/stager_spec.rb | 5 +- .../diego/task_environment_spec.rb | 2 +- .../diego/task_recipe_builder_spec.rb | 4 +- spec/unit/models/runtime/app_model_spec.rb | 74 +++++---- spec/unit/models/runtime/build_model_spec.rb | 144 ++++++++---------- .../unit/models/runtime/droplet_model_spec.rb | 127 +++++++-------- spec/unit/models/runtime/task_model_spec.rb | 4 +- .../presenters/v3/build_presenter_spec.rb | 14 +- .../presenters/v3/droplet_presenter_spec.rb | 3 +- .../repositories/app_event_repository_spec.rb | 2 +- .../app_usage_event_repository_spec.rb | 2 - 58 files changed, 563 insertions(+), 515 deletions(-) create mode 100644 db/migrations/20260428120000_add_lifecycle_type_to_apps.rb create mode 100644 db/migrations/20260428120100_add_lifecycle_type_to_droplets.rb create mode 100644 db/migrations/20260428120200_add_lifecycle_type_to_builds.rb create mode 100644 spec/migrations/20260428120000_add_lifecycle_type_to_apps_spec.rb create mode 100644 spec/migrations/20260428120100_add_lifecycle_type_to_droplets_spec.rb create mode 100644 spec/migrations/20260428120200_add_lifecycle_type_to_builds_spec.rb diff --git a/app/actions/app_create.rb b/app/actions/app_create.rb index 4ea914fb42c..245ded8bb0c 100644 --- a/app/actions/app_create.rb +++ b/app/actions/app_create.rb @@ -22,7 +22,8 @@ def create(message, lifecycle) app = AppModel.create( name: message.name, space_guid: message.space_guid, - environment_variables: message.environment_variables + environment_variables: message.environment_variables, + lifecycle_type: lifecycle.type ) lifecycle.create_lifecycle_data_model(app) diff --git a/app/actions/droplet_copy.rb b/app/actions/droplet_copy.rb index 668ba04fdd1..e00eea261f9 100644 --- a/app/actions/droplet_copy.rb +++ b/app/actions/droplet_copy.rb @@ -18,6 +18,7 @@ def initialize(source_droplet) def copy(destination_app, user_audit_info) raise InvalidCopyError.new('source droplet is not staged') unless @source_droplet.staged? + raise InvalidCopyError.new('source and destination lifecycle types do not match') unless @source_droplet.lifecycle_type == destination_app.lifecycle_type new_droplet = DropletModel.new(state: DropletModel::COPYING_STATE, app: destination_app) diff --git a/app/actions/droplet_create.rb b/app/actions/droplet_create.rb index 603560051ad..f82d40da82a 100644 --- a/app/actions/droplet_create.rb +++ b/app/actions/droplet_create.rb @@ -12,7 +12,7 @@ def create(app, message, user_audit_info) end droplet = DropletModel.new( - app_guid: app.guid, + app: app, state: DropletModel::AWAITING_UPLOAD_STATE, process_types: message.process_types || DEFAULT_PROCESS_TYPES, execution_metadata: '' @@ -20,7 +20,7 @@ def create(app, message, user_audit_info) DropletModel.db.transaction do droplet.save - VCAP::CloudController::BuildpackLifecycleDataModel.create( + droplet.buildpack_lifecycle_data = VCAP::CloudController::BuildpackLifecycleDataModel.create( droplet: ) end @@ -84,7 +84,7 @@ def find_or_create_buildpack_droplet(build) def droplet_from_build(build) DropletModel.new( - app_guid: build.app_guid, + app: build.app, package_guid: build.package_guid, state: DropletModel::STAGING_STATE, build: build diff --git a/app/actions/v2/app_create.rb b/app/actions/v2/app_create.rb index 0e7d7f34632..706e4524388 100644 --- a/app/actions/v2/app_create.rb +++ b/app/actions/v2/app_create.rb @@ -15,7 +15,8 @@ def create(request_attrs) name: request_attrs['name'], space_guid: request_attrs['space_guid'], environment_variables: request_attrs['environment_json'], - enable_ssh: request_attrs['enable_ssh'] + enable_ssh: request_attrs['enable_ssh'], + lifecycle_type: (request_attrs.key?('docker_image') ? DockerLifecycleDataModel::LIFECYCLE_TYPE : BuildpackLifecycleDataModel::LIFECYCLE_TYPE) ) validate_lifecycle!(request_attrs) diff --git a/app/models/runtime/app_model.rb b/app/models/runtime/app_model.rb index f2a35d17599..e56aa1d7b27 100644 --- a/app/models/runtime/app_model.rb +++ b/app/models/runtime/app_model.rb @@ -93,9 +93,14 @@ def validate validates_format APP_NAME_REGEX, :name validate_environment_variables validate_droplet_is_staged + + validates_includes %w[buildpack cnb docker], :lifecycle_type end def lifecycle_type + return self[:lifecycle_type] if self[:lifecycle_type].present? + + # Fallback for records without lifecycle_type column value return BuildpackLifecycleDataModel::LIFECYCLE_TYPE if buildpack_lifecycle_data return CNBLifecycleDataModel::LIFECYCLE_TYPE if cnb_lifecycle_data @@ -103,8 +108,8 @@ def lifecycle_type end def lifecycle_data - return buildpack_lifecycle_data if buildpack_lifecycle_data - return cnb_lifecycle_data if cnb_lifecycle_data + return buildpack_lifecycle_data if lifecycle_type == BuildpackLifecycleDataModel::LIFECYCLE_TYPE + return cnb_lifecycle_data if lifecycle_type == CNBLifecycleDataModel::LIFECYCLE_TYPE DockerLifecycleDataModel.new end diff --git a/app/models/runtime/build_model.rb b/app/models/runtime/build_model.rb index 617b0b52b21..8171d76592b 100644 --- a/app/models/runtime/build_model.rb +++ b/app/models/runtime/build_model.rb @@ -50,24 +50,39 @@ class BuildModel < Sequel::Model(:builds) add_association_dependencies labels: :destroy add_association_dependencies annotations: :destroy + def validate + super + validates_includes %w[buildpack cnb docker], :lifecycle_type + end + + def before_create + # Inherit lifecycle_type from associated app if not explicitly set + self[:lifecycle_type] = app&.lifecycle_type if self[:lifecycle_type].blank? + + super + end + def lifecycle_type - return Lifecycles::BUILDPACK if buildpack_lifecycle_data - return Lifecycles::CNB if cnb_lifecycle_data + return self[:lifecycle_type] if self[:lifecycle_type].present? + + # Fallback for records without lifecycle_type column value + return BuildpackLifecycleDataModel::LIFECYCLE_TYPE if buildpack_lifecycle_data + return CNBLifecycleDataModel::LIFECYCLE_TYPE if cnb_lifecycle_data - Lifecycles::DOCKER + DockerLifecycleDataModel::LIFECYCLE_TYPE end def buildpack_lifecycle? - lifecycle_type == Lifecycles::BUILDPACK + lifecycle_type == BuildpackLifecycleDataModel::LIFECYCLE_TYPE end def cnb_lifecycle? - lifecycle_type == Lifecycles::CNB + lifecycle_type == CNBLifecycleDataModel::LIFECYCLE_TYPE end def lifecycle_data - return buildpack_lifecycle_data if buildpack_lifecycle_data - return cnb_lifecycle_data if cnb_lifecycle_data + return buildpack_lifecycle_data if lifecycle_type == BuildpackLifecycleDataModel::LIFECYCLE_TYPE + return cnb_lifecycle_data if lifecycle_type == CNBLifecycleDataModel::LIFECYCLE_TYPE DockerLifecycleDataModel.new end diff --git a/app/models/runtime/droplet_model.rb b/app/models/runtime/droplet_model.rb index b4ddf9bf7f1..2f0e56c5ce9 100644 --- a/app/models/runtime/droplet_model.rb +++ b/app/models/runtime/droplet_model.rb @@ -49,6 +49,13 @@ class DropletModel < Sequel::Model(:droplets) serializes_via_json :process_types serializes_via_json :sidecars + def before_create + # Inherit lifecycle_type from associated app if not explicitly set + self[:lifecycle_type] = app&.lifecycle_type if self[:lifecycle_type].blank? + + super + end + def around_destroy yield rescue Sequel::ForeignKeyConstraintViolation => e @@ -65,6 +72,7 @@ def error def validate super validates_includes DROPLET_STATES, :state, allow_missing: true + validates_includes %w[buildpack cnb docker], :lifecycle_type end def set_buildpack_receipt(buildpack_key:, detect_output:, requested_buildpack:, buildpack_url: nil) @@ -168,6 +176,9 @@ def fail_to_stage!(reason='StagingError', details='staging failed') end def lifecycle_type + return self[:lifecycle_type] if self[:lifecycle_type].present? + + # Fallback for records without lifecycle_type column value return BuildpackLifecycleDataModel::LIFECYCLE_TYPE if buildpack_lifecycle_data return CNBLifecycleDataModel::LIFECYCLE_TYPE if cnb_lifecycle_data @@ -175,8 +186,8 @@ def lifecycle_type end def lifecycle_data - return buildpack_lifecycle_data if buildpack_lifecycle_data - return cnb_lifecycle_data if cnb_lifecycle_data + return buildpack_lifecycle_data if lifecycle_type == BuildpackLifecycleDataModel::LIFECYCLE_TYPE + return cnb_lifecycle_data if lifecycle_type == CNBLifecycleDataModel::LIFECYCLE_TYPE DockerLifecycleDataModel.new end diff --git a/db/migrations/20260428120000_add_lifecycle_type_to_apps.rb b/db/migrations/20260428120000_add_lifecycle_type_to_apps.rb new file mode 100644 index 00000000000..393403e9fec --- /dev/null +++ b/db/migrations/20260428120000_add_lifecycle_type_to_apps.rb @@ -0,0 +1,29 @@ +Sequel.migration do + no_transaction # to use the 'concurrently' option + + up do + if database_type == :postgres + add_column :apps, :lifecycle_type, String, null: true, size: 255, if_not_exists: true + VCAP::Migration.with_concurrent_timeout(self) do + add_index :apps, :lifecycle_type, name: :apps_lifecycle_type_index, concurrently: true, if_not_exists: true + end + else + # MySQL + add_column :apps, :lifecycle_type, String, null: true, size: 255 unless schema(:apps).map(&:first).include?(:lifecycle_type) + add_index :apps, :lifecycle_type, name: :apps_lifecycle_type_index, concurrently: false unless indexes(:apps).include?(:apps_lifecycle_type_index) + end + end + + down do + if database_type == :postgres + VCAP::Migration.with_concurrent_timeout(self) do + drop_index :apps, :lifecycle_type, name: :apps_lifecycle_type_index, concurrently: true, if_exists: true + end + drop_column :apps, :lifecycle_type, if_exists: true + else + # MySQL + drop_index :apps, :lifecycle_type, name: :apps_lifecycle_type_index, concurrently: false if indexes(:apps).include?(:apps_lifecycle_type_index) + drop_column :apps, :lifecycle_type if schema(:apps).map(&:first).include?(:lifecycle_type) + end + end +end diff --git a/db/migrations/20260428120100_add_lifecycle_type_to_droplets.rb b/db/migrations/20260428120100_add_lifecycle_type_to_droplets.rb new file mode 100644 index 00000000000..e94a843002c --- /dev/null +++ b/db/migrations/20260428120100_add_lifecycle_type_to_droplets.rb @@ -0,0 +1,29 @@ +Sequel.migration do + no_transaction # to use the 'concurrently' option + + up do + if database_type == :postgres + add_column :droplets, :lifecycle_type, String, null: true, size: 255, if_not_exists: true + VCAP::Migration.with_concurrent_timeout(self) do + add_index :droplets, :lifecycle_type, name: :droplets_lifecycle_type_index, concurrently: true, if_not_exists: true + end + else + # MySQL + add_column :droplets, :lifecycle_type, String, null: true, size: 255 unless schema(:droplets).map(&:first).include?(:lifecycle_type) + add_index :droplets, :lifecycle_type, name: :droplets_lifecycle_type_index, concurrently: false unless indexes(:droplets).include?(:droplets_lifecycle_type_index) + end + end + + down do + if database_type == :postgres + VCAP::Migration.with_concurrent_timeout(self) do + drop_index :droplets, :lifecycle_type, name: :droplets_lifecycle_type_index, concurrently: true, if_exists: true + end + drop_column :droplets, :lifecycle_type, if_exists: true + else + # MySQL + drop_index :droplets, :lifecycle_type, name: :droplets_lifecycle_type_index, concurrently: false if indexes(:droplets).include?(:droplets_lifecycle_type_index) + drop_column :droplets, :lifecycle_type if schema(:droplets).map(&:first).include?(:lifecycle_type) + end + end +end diff --git a/db/migrations/20260428120200_add_lifecycle_type_to_builds.rb b/db/migrations/20260428120200_add_lifecycle_type_to_builds.rb new file mode 100644 index 00000000000..2f8d9a55f66 --- /dev/null +++ b/db/migrations/20260428120200_add_lifecycle_type_to_builds.rb @@ -0,0 +1,29 @@ +Sequel.migration do + no_transaction # to use the 'concurrently' option + + up do + if database_type == :postgres + add_column :builds, :lifecycle_type, String, null: true, size: 255, if_not_exists: true + VCAP::Migration.with_concurrent_timeout(self) do + add_index :builds, :lifecycle_type, name: :builds_lifecycle_type_index, concurrently: true, if_not_exists: true + end + else + # MySQL + add_column :builds, :lifecycle_type, String, null: true, size: 255 unless schema(:builds).map(&:first).include?(:lifecycle_type) + add_index :builds, :lifecycle_type, name: :builds_lifecycle_type_index, concurrently: false unless indexes(:builds).include?(:builds_lifecycle_type_index) + end + end + + down do + if database_type == :postgres + VCAP::Migration.with_concurrent_timeout(self) do + drop_index :builds, :lifecycle_type, name: :builds_lifecycle_type_index, concurrently: true, if_exists: true + end + drop_column :builds, :lifecycle_type, if_exists: true + else + # MySQL + drop_index :builds, :lifecycle_type, name: :builds_lifecycle_type_index, concurrently: false if indexes(:builds).include?(:builds_lifecycle_type_index) + drop_column :builds, :lifecycle_type if schema(:builds).map(&:first).include?(:lifecycle_type) + end + end +end diff --git a/lib/cloud_controller/diego/lifecycles/app_buildpack_lifecycle.rb b/lib/cloud_controller/diego/lifecycles/app_buildpack_lifecycle.rb index 419c9fc9c6c..994b46d9981 100644 --- a/lib/cloud_controller/diego/lifecycles/app_buildpack_lifecycle.rb +++ b/lib/cloud_controller/diego/lifecycles/app_buildpack_lifecycle.rb @@ -18,7 +18,7 @@ def initialize(message) delegate :valid?, :errors, to: :validator def create_lifecycle_data_model(app) - BuildpackLifecycleDataModel.create( + app.buildpack_lifecycle_data = BuildpackLifecycleDataModel.create( buildpacks:, stack:, app: diff --git a/lib/cloud_controller/diego/lifecycles/app_cnb_lifecycle.rb b/lib/cloud_controller/diego/lifecycles/app_cnb_lifecycle.rb index a4eae0f65dd..b056a704e21 100644 --- a/lib/cloud_controller/diego/lifecycles/app_cnb_lifecycle.rb +++ b/lib/cloud_controller/diego/lifecycles/app_cnb_lifecycle.rb @@ -18,7 +18,7 @@ def initialize(message) delegate :valid?, :errors, to: :validator def create_lifecycle_data_model(app) - CNBLifecycleDataModel.create( + app.cnb_lifecycle_data = CNBLifecycleDataModel.create( buildpacks:, stack:, credentials:, diff --git a/lib/cloud_controller/diego/lifecycles/buildpack_lifecycle.rb b/lib/cloud_controller/diego/lifecycles/buildpack_lifecycle.rb index 25efa5cda17..5744c020a57 100644 --- a/lib/cloud_controller/diego/lifecycles/buildpack_lifecycle.rb +++ b/lib/cloud_controller/diego/lifecycles/buildpack_lifecycle.rb @@ -10,7 +10,7 @@ def type end def create_lifecycle_data_model(build) - VCAP::CloudController::BuildpackLifecycleDataModel.create( + build.buildpack_lifecycle_data = VCAP::CloudController::BuildpackLifecycleDataModel.create( buildpacks: Array(buildpacks_to_use), stack: staging_stack, build: build diff --git a/lib/cloud_controller/diego/lifecycles/cnb_lifecycle.rb b/lib/cloud_controller/diego/lifecycles/cnb_lifecycle.rb index 45941e6ebc0..da797f81c93 100644 --- a/lib/cloud_controller/diego/lifecycles/cnb_lifecycle.rb +++ b/lib/cloud_controller/diego/lifecycles/cnb_lifecycle.rb @@ -10,7 +10,7 @@ def type end def create_lifecycle_data_model(build) - VCAP::CloudController::CNBLifecycleDataModel.create( + build.cnb_lifecycle_data = VCAP::CloudController::CNBLifecycleDataModel.create( buildpacks: Array(buildpacks_to_use), stack: staging_stack, build: build, diff --git a/spec/migrations/20260428120000_add_lifecycle_type_to_apps_spec.rb b/spec/migrations/20260428120000_add_lifecycle_type_to_apps_spec.rb new file mode 100644 index 00000000000..c806949c85e --- /dev/null +++ b/spec/migrations/20260428120000_add_lifecycle_type_to_apps_spec.rb @@ -0,0 +1,35 @@ +require 'spec_helper' +require 'migrations/helpers/migration_shared_context' + +RSpec.describe 'migration to add lifecycle_type to apps', isolation: :truncation, type: :migration do + include_context 'migration' do + let(:migration_filename) { '20260428120000_add_lifecycle_type_to_apps.rb' } + end + + it 'adds and removes the lifecycle_type column and index (idempotent)' do + expect(db.schema(:apps).map(&:first)).not_to include(:lifecycle_type) + expect(db.indexes(:apps)).not_to have_key(:apps_lifecycle_type_index) + + # up + Sequel::Migrator.run(db, migrations_path, target: current_migration_index, allow_missing_migration_files: true) + + expect(db.schema(:apps).map(&:first)).to include(:lifecycle_type) + expect(db.indexes(:apps)).to have_key(:apps_lifecycle_type_index) + expect(db.indexes(:apps)[:apps_lifecycle_type_index][:columns]).to eq([:lifecycle_type]) + + lifecycle_type_column = db.schema(:apps).find { |col| col[0] == :lifecycle_type } + expect(lifecycle_type_column[1][:allow_null]).to be true + + # up is idempotent + expect { Sequel::Migrator.run(db, migrations_path, target: current_migration_index, allow_missing_migration_files: true) }.not_to raise_error + + # down + Sequel::Migrator.run(db, migrations_path, target: current_migration_index - 1, allow_missing_migration_files: true) + + expect(db.schema(:apps).map(&:first)).not_to include(:lifecycle_type) + expect(db.indexes(:apps)).not_to have_key(:apps_lifecycle_type_index) + + # down is idempotent + expect { Sequel::Migrator.run(db, migrations_path, target: current_migration_index - 1, allow_missing_migration_files: true) }.not_to raise_error + end +end diff --git a/spec/migrations/20260428120100_add_lifecycle_type_to_droplets_spec.rb b/spec/migrations/20260428120100_add_lifecycle_type_to_droplets_spec.rb new file mode 100644 index 00000000000..1b944ac2548 --- /dev/null +++ b/spec/migrations/20260428120100_add_lifecycle_type_to_droplets_spec.rb @@ -0,0 +1,35 @@ +require 'spec_helper' +require 'migrations/helpers/migration_shared_context' + +RSpec.describe 'migration to add lifecycle_type to droplets', isolation: :truncation, type: :migration do + include_context 'migration' do + let(:migration_filename) { '20260428120100_add_lifecycle_type_to_droplets.rb' } + end + + it 'adds and removes the lifecycle_type column and index (idempotent)' do + expect(db.schema(:droplets).map(&:first)).not_to include(:lifecycle_type) + expect(db.indexes(:droplets)).not_to have_key(:droplets_lifecycle_type_index) + + # up + Sequel::Migrator.run(db, migrations_path, target: current_migration_index, allow_missing_migration_files: true) + + expect(db.schema(:droplets).map(&:first)).to include(:lifecycle_type) + expect(db.indexes(:droplets)).to have_key(:droplets_lifecycle_type_index) + expect(db.indexes(:droplets)[:droplets_lifecycle_type_index][:columns]).to eq([:lifecycle_type]) + + lifecycle_type_column = db.schema(:droplets).find { |col| col[0] == :lifecycle_type } + expect(lifecycle_type_column[1][:allow_null]).to be true + + # up is idempotent + expect { Sequel::Migrator.run(db, migrations_path, target: current_migration_index, allow_missing_migration_files: true) }.not_to raise_error + + # down + Sequel::Migrator.run(db, migrations_path, target: current_migration_index - 1, allow_missing_migration_files: true) + + expect(db.schema(:droplets).map(&:first)).not_to include(:lifecycle_type) + expect(db.indexes(:droplets)).not_to have_key(:droplets_lifecycle_type_index) + + # down is idempotent + expect { Sequel::Migrator.run(db, migrations_path, target: current_migration_index - 1, allow_missing_migration_files: true) }.not_to raise_error + end +end diff --git a/spec/migrations/20260428120200_add_lifecycle_type_to_builds_spec.rb b/spec/migrations/20260428120200_add_lifecycle_type_to_builds_spec.rb new file mode 100644 index 00000000000..a0834c24784 --- /dev/null +++ b/spec/migrations/20260428120200_add_lifecycle_type_to_builds_spec.rb @@ -0,0 +1,35 @@ +require 'spec_helper' +require 'migrations/helpers/migration_shared_context' + +RSpec.describe 'migration to add lifecycle_type to builds', isolation: :truncation, type: :migration do + include_context 'migration' do + let(:migration_filename) { '20260428120200_add_lifecycle_type_to_builds.rb' } + end + + it 'adds and removes the lifecycle_type column and index (idempotent)' do + expect(db.schema(:builds).map(&:first)).not_to include(:lifecycle_type) + expect(db.indexes(:builds)).not_to have_key(:builds_lifecycle_type_index) + + # up + Sequel::Migrator.run(db, migrations_path, target: current_migration_index, allow_missing_migration_files: true) + + expect(db.schema(:builds).map(&:first)).to include(:lifecycle_type) + expect(db.indexes(:builds)).to have_key(:builds_lifecycle_type_index) + expect(db.indexes(:builds)[:builds_lifecycle_type_index][:columns]).to eq([:lifecycle_type]) + + lifecycle_type_column = db.schema(:builds).find { |col| col[0] == :lifecycle_type } + expect(lifecycle_type_column[1][:allow_null]).to be true + + # up is idempotent + expect { Sequel::Migrator.run(db, migrations_path, target: current_migration_index, allow_missing_migration_files: true) }.not_to raise_error + + # down + Sequel::Migrator.run(db, migrations_path, target: current_migration_index - 1, allow_missing_migration_files: true) + + expect(db.schema(:builds).map(&:first)).not_to include(:lifecycle_type) + expect(db.indexes(:builds)).not_to have_key(:builds_lifecycle_type_index) + + # down is idempotent + expect { Sequel::Migrator.run(db, migrations_path, target: current_migration_index - 1, allow_missing_migration_files: true) }.not_to raise_error + end +end diff --git a/spec/request/apps_spec.rb b/spec/request/apps_spec.rb index 64fcef98a77..7e90a7d013c 100644 --- a/spec/request/apps_spec.rb +++ b/spec/request/apps_spec.rb @@ -780,35 +780,12 @@ end end - context 'filtering by timestamps' do - before do - VCAP::CloudController::AppModel.plugin :timestamps, update_on_create: false - end - - # .make updates the resource after creating it, over writing our passed in updated_at timestamp - # Therefore we cannot use shared_examples as the updated_at will not be as written - let!(:resource_1) { VCAP::CloudController::AppModel.create(name: '1', created_at: '2020-05-26T18:47:01Z', updated_at: '2020-05-26T18:47:01Z', space: space) } - let!(:resource_2) { VCAP::CloudController::AppModel.create(name: '2', created_at: '2020-05-26T18:47:02Z', updated_at: '2020-05-26T18:47:02Z', space: space) } - let!(:resource_3) { VCAP::CloudController::AppModel.create(name: '3', created_at: '2020-05-26T18:47:03Z', updated_at: '2020-05-26T18:47:03Z', space: space) } - let!(:resource_4) { VCAP::CloudController::AppModel.create(name: '4', created_at: '2020-05-26T18:47:04Z', updated_at: '2020-05-26T18:47:04Z', space: space) } - - after do - VCAP::CloudController::AppModel.plugin :timestamps, update_on_create: true - end - - it 'filters by the created at' do - get "/v3/apps?created_ats[lt]=#{resource_3.created_at.iso8601}", nil, admin_header - - expect(last_response).to have_status_code(200) - expect(parsed_response['resources'].pluck('guid')).to contain_exactly(resource_1.guid, resource_2.guid) - end - - it 'filters ny the updated_at' do - get "/v3/apps?updated_ats[lt]=#{resource_3.updated_at.iso8601}", nil, admin_header - - expect(last_response).to have_status_code(200) - expect(parsed_response['resources'].pluck('guid')).to contain_exactly(resource_1.guid, resource_2.guid) + it_behaves_like 'list_endpoint_with_common_filters' do + let(:resource_klass) { VCAP::CloudController::AppModel } + let(:api_call) do + ->(headers, filters) { get "/v3/apps?#{filters}", nil, headers } end + let(:headers) { admin_header } end context 'faceted search' do @@ -1388,7 +1365,6 @@ let!(:stack) { VCAP::CloudController::Stack.make(name: 'stack-name') } let!(:app_model) do VCAP::CloudController::AppModel.make( - :buildpack, name: 'my_app', guid: 'app1_guid', space: space, @@ -1652,7 +1628,6 @@ let(:api_call) { ->(user_headers) { get "/v3/apps/#{app_model.guid}/env", nil, user_headers } } let!(:app_model) do VCAP::CloudController::AppModel.make( - :buildpack, name: 'my_app', guid: 'app1_guid', space: space, @@ -2061,7 +2036,6 @@ let(:api_call) { ->(user_headers) { get "/v3/apps/#{app_model.guid}/ssh_enabled", nil, user_headers } } let!(:app_model) do VCAP::CloudController::AppModel.make( - :buildpack, name: 'my_app', guid: 'app1_guid', space: space @@ -2167,7 +2141,6 @@ describe 'PATCH /v3/apps/:guid' do let(:app_model) do VCAP::CloudController::AppModel.make( - :buildpack, name: 'original_name', space: space, environment_variables: { 'ORIGINAL' => 'ENVAR' }, @@ -2403,7 +2376,6 @@ let(:stack) { VCAP::CloudController::Stack.make(name: 'stack-name') } let(:app_model) do VCAP::CloudController::AppModel.make( - :buildpack, name: 'app-name', space: space, desired_state: 'STOPPED' @@ -2413,7 +2385,6 @@ context 'app lifecycle is buildpack' do let!(:droplet) do VCAP::CloudController::DropletModel.make( - :buildpack, app: app_model, state: VCAP::CloudController::DropletModel::STAGED_STATE ) @@ -2526,7 +2497,6 @@ let!(:process_model) { VCAP::CloudController::ProcessModel.make(app: app_model, log_rate_limit: log_rate_limit) } let(:app_model) do VCAP::CloudController::AppModel.make( - :buildpack, name: 'app-name', space: space, desired_state: 'STOPPED' @@ -2747,16 +2717,16 @@ let(:stack) { VCAP::CloudController::Stack.make(name: 'stack-name') } let(:app_model) do VCAP::CloudController::AppModel.make( - :buildpack, name: 'app-name', space: space, desired_state: 'STARTED' ) end let!(:droplet) do - VCAP::CloudController::DropletModel.make(:buildpack, - app: app_model, - state: VCAP::CloudController::DropletModel::STAGED_STATE) + VCAP::CloudController::DropletModel.make( + app: app_model, + state: VCAP::CloudController::DropletModel::STAGED_STATE + ) end before do @@ -2912,7 +2882,6 @@ let(:stack) { VCAP::CloudController::Stack.make(name: 'stack-name') } let(:app_model) do VCAP::CloudController::AppModel.make( - :buildpack, name: 'app-name', space: space, desired_state: 'STARTED' @@ -2922,7 +2891,6 @@ context 'app lifecycle is buildpack' do let!(:droplet) do VCAP::CloudController::DropletModel.make( - :buildpack, app: app_model, state: VCAP::CloudController::DropletModel::STAGED_STATE ) @@ -3153,7 +3121,6 @@ let(:stack) { VCAP::CloudController::Stack.make(name: 'stack-name') } let(:app_model) do VCAP::CloudController::AppModel.make( - :buildpack, name: 'my_app', space: space, desired_state: 'STOPPED' diff --git a/spec/request/droplets_spec.rb b/spec/request/droplets_spec.rb index 0db30447dbe..adb72532112 100644 --- a/spec/request/droplets_spec.rb +++ b/spec/request/droplets_spec.rb @@ -833,7 +833,7 @@ end context 'when a droplet does not have a buildpack lifecycle' do - let!(:droplet_without_lifecycle) { VCAP::CloudController::DropletModel.make(:buildpack, package_guid: VCAP::CloudController::PackageModel.make.guid) } + let!(:droplet_without_lifecycle) { VCAP::CloudController::DropletModel.make(package_guid: VCAP::CloudController::PackageModel.make.guid) } it 'is excluded' do get '/v3/droplets', nil, developer_headers @@ -977,71 +977,21 @@ end end - context 'filtering by timestamps' do + it_behaves_like 'list_endpoint_with_common_filters' do before do - VCAP::CloudController::DropletModel.plugin :timestamps, update_on_create: false - - # Delete all the existing DropletModels so they don't overlap in timestamp with our queries VCAP::CloudController::DropletModel.dataset.delete end - # .make updates the resource after creating it, over writing our passed in updated_at timestamp - # Therefore we cannot use shared_examples as the updated_at will not be as written - let!(:resource_1) do - VCAP::CloudController::DropletModel.create( - state: VCAP::CloudController::DropletModel::STAGED_STATE, - created_at: '2020-05-26T18:47:01Z', - updated_at: '2020-05-26T18:47:01Z', - app: app_model - ) - end - let!(:resource_2) do - VCAP::CloudController::DropletModel.create( - state: VCAP::CloudController::DropletModel::STAGED_STATE, - created_at: '2020-05-26T18:47:02Z', - updated_at: '2020-05-26T18:47:02Z', - app: app_model - ) - end - let!(:resource_3) do - VCAP::CloudController::DropletModel.create( - state: VCAP::CloudController::DropletModel::STAGED_STATE, - created_at: '2020-05-26T18:47:03Z', - updated_at: '2020-05-26T18:47:03Z', - app: app_model - ) - end - let!(:resource_4) do - VCAP::CloudController::DropletModel.create( - state: VCAP::CloudController::DropletModel::STAGED_STATE, - created_at: '2020-05-26T18:47:04Z', - updated_at: '2020-05-26T18:47:04Z', - app: app_model - ) - end - - after do - VCAP::CloudController::DropletModel.plugin :timestamps, update_on_create: true - end - - it 'filters by the created at' do - get "/v3/droplets?created_ats[lt]=#{resource_3.created_at.iso8601}", nil, admin_headers - - expect(last_response).to have_status_code(200) - expect(parsed_response['resources'].pluck('guid')).to contain_exactly(resource_1.guid, resource_2.guid) - end - - it 'filters by the updated_at' do - get "/v3/droplets?updated_ats[lt]=#{resource_3.updated_at.iso8601}", nil, admin_headers - - expect(last_response).to have_status_code(200) - expect(parsed_response['resources'].pluck('guid')).to contain_exactly(resource_1.guid, resource_2.guid) + let(:resource_klass) { VCAP::CloudController::DropletModel } + let(:api_call) do + ->(headers, filters) { get "/v3/droplets?#{filters}", nil, headers } end + let(:headers) { admin_headers } end end describe 'DELETE /v3/droplets/:guid' do - let!(:droplet) { VCAP::CloudController::DropletModel.make(:buildpack, app_guid: app_model.guid) } + let!(:droplet) { VCAP::CloudController::DropletModel.make(app_guid: app_model.guid) } let(:api_call) { ->(user_headers) { delete "/v3/droplets/#{droplet.guid}", nil, user_headers } } let(:db_check) do diff --git a/spec/request/revisions_spec.rb b/spec/request/revisions_spec.rb index d3ace2cf840..58b9f7e5c85 100644 --- a/spec/request/revisions_spec.rb +++ b/spec/request/revisions_spec.rb @@ -323,116 +323,13 @@ end end - context 'filtering by guids' do - before do - VCAP::CloudController::RevisionModel.plugin :timestamps, update_on_create: false - end - - let(:droplet) { VCAP::CloudController::DropletModel.make(app: app_model) } - # .make updates the resource after creating it, over writing our passed in updated_at timestamp - # Therefore we cannot use shared_examples as the updated_at will not be as written - let!(:resource_1) do - VCAP::CloudController::RevisionModel.create( - created_at: '2020-05-26T18:47:01Z', - updated_at: '2020-05-26T18:47:01Z', - droplet: droplet, - description: '', - app: app_model - ) - end - let!(:resource_2) do - VCAP::CloudController::RevisionModel.create( - created_at: '2020-05-26T18:47:02Z', - updated_at: '2020-05-26T18:47:02Z', - droplet: droplet, - description: '', - app: app_model - ) - end - let!(:resource_3) do - VCAP::CloudController::RevisionModel.create( - created_at: '2020-05-26T18:47:03Z', - updated_at: '2020-05-26T18:47:03Z', - droplet: droplet, - description: '', - app: app_model - ) - end - - after do - VCAP::CloudController::RevisionModel.plugin :timestamps, update_on_create: true - end - - it 'filters by the created at' do - get "/v3/apps/#{app_model.guid}/revisions?guids=#{resource_1.guid},#{resource_2.guid}", nil, admin_headers - - expect(last_response).to have_status_code(200) - expect(parsed_response['resources'].pluck('guid')).to contain_exactly(resource_1.guid, resource_2.guid) - end - end - - context 'filtering by timestamps' do - before do - VCAP::CloudController::RevisionModel.plugin :timestamps, update_on_create: false - end - - let(:droplet) { VCAP::CloudController::DropletModel.make(app: app_model) } - # .make updates the resource after creating it, over writing our passed in updated_at timestamp - # Therefore we cannot use shared_examples as the updated_at will not be as written - let!(:resource_1) do - VCAP::CloudController::RevisionModel.create( - created_at: '2020-05-26T18:47:01Z', - updated_at: '2020-05-26T18:47:01Z', - droplet: droplet, - description: '', - app: app_model - ) - end - let!(:resource_2) do - VCAP::CloudController::RevisionModel.create( - created_at: '2020-05-26T18:47:02Z', - updated_at: '2020-05-26T18:47:02Z', - droplet: droplet, - description: '', - app: app_model - ) - end - let!(:resource_3) do - VCAP::CloudController::RevisionModel.create( - created_at: '2020-05-26T18:47:03Z', - updated_at: '2020-05-26T18:47:03Z', - droplet: droplet, - description: '', - app: app_model - ) - end - let!(:resource_4) do - VCAP::CloudController::RevisionModel.create( - created_at: '2020-05-26T18:47:04Z', - updated_at: '2020-05-26T18:47:04Z', - droplet: droplet, - description: '', - app: app_model - ) - end - - after do - VCAP::CloudController::RevisionModel.plugin :timestamps, update_on_create: true - end - - it 'filters by the created at' do - get "/v3/apps/#{app_model.guid}/revisions?created_ats[lt]=#{resource_3.created_at.iso8601}", nil, admin_headers - - expect(last_response).to have_status_code(200) - expect(parsed_response['resources'].pluck('guid')).to contain_exactly(resource_1.guid, resource_2.guid) - end - - it 'filters by the updated_at' do - get "/v3/apps/#{app_model.guid}/revisions?updated_ats[lt]=#{resource_3.updated_at.iso8601}", nil, admin_headers - - expect(last_response).to have_status_code(200) - expect(parsed_response['resources'].pluck('guid')).to contain_exactly(resource_1.guid, resource_2.guid) + it_behaves_like 'list_endpoint_with_common_filters' do + let(:resource_klass) { VCAP::CloudController::RevisionModel } + let(:additional_resource_params) { { app: app_model } } + let(:api_call) do + ->(headers, filters) { get "/v3/apps/#{app_model.guid}/revisions?#{filters}", nil, headers } end + let(:headers) { admin_headers } end end end diff --git a/spec/request_spec_shared_examples.rb b/spec/request_spec_shared_examples.rb index 7281736e92e..ef9606182ba 100644 --- a/spec/request_spec_shared_examples.rb +++ b/spec/request_spec_shared_examples.rb @@ -305,19 +305,22 @@ def errors_without_test_mode_info(parsed_response) end context 'filtering timestamps on update' do - # before must occur before the let! otherwise the resources will be created with - # update_on_create: true + # Some blueprints call object.save multiple times (e.g. to create associated lifecycle_data), + # which triggers the timestamps plugin's before_update hook and overwrites updated_at. + # Setting allow_manual_update: true and using an explicit update after make ensures the + # updated_at value is preserved. Note: plugin :timestamps resets all options to defaults, + # so update_on_create: true must be passed explicitly to preserve it. before do - resource_klass.plugin :timestamps, update_on_create: false + resource_klass.plugin :timestamps, update_on_create: true, allow_manual_update: true end - let!(:resource_1) { resource_klass.make(guid: '1', updated_at: '2020-05-26T18:47:01Z', **additional_resource_params) } - let!(:resource_2) { resource_klass.make(guid: '2', updated_at: '2020-05-26T18:47:02Z', **additional_resource_params) } - let!(:resource_3) { resource_klass.make(guid: '3', updated_at: '2020-05-26T18:47:03Z', **additional_resource_params) } - let!(:resource_4) { resource_klass.make(guid: '4', updated_at: '2020-05-26T18:47:04Z', **additional_resource_params) } + let!(:resource_1) { resource_klass.make(guid: '1', **additional_resource_params).tap { |r| r.update(updated_at: '2020-05-26T18:47:01Z') } } + let!(:resource_2) { resource_klass.make(guid: '2', **additional_resource_params).tap { |r| r.update(updated_at: '2020-05-26T18:47:02Z') } } + let!(:resource_3) { resource_klass.make(guid: '3', **additional_resource_params).tap { |r| r.update(updated_at: '2020-05-26T18:47:03Z') } } + let!(:resource_4) { resource_klass.make(guid: '4', **additional_resource_params).tap { |r| r.update(updated_at: '2020-05-26T18:47:04Z') } } after do - resource_klass.plugin :timestamps, update_on_create: true + resource_klass.plugin :timestamps, update_on_create: true, allow_manual_update: false end it 'filters' do diff --git a/spec/support/fakes/blueprints.rb b/spec/support/fakes/blueprints.rb index 480787a3e66..d3fd5cdb52a 100644 --- a/spec/support/fakes/blueprints.rb +++ b/spec/support/fakes/blueprints.rb @@ -47,7 +47,9 @@ module VCAP::CloudController AppModel.blueprint do name { Sham.name } space { Space.make } + lifecycle_type { BuildpackLifecycleDataModel::LIFECYCLE_TYPE } buildpack_lifecycle_data { BuildpackLifecycleDataModel.make(app: object.save) } + cnb_lifecycle_data { nil.tap { |_| object.save } } end AppModel.blueprint(:all_fields) do @@ -58,6 +60,7 @@ module VCAP::CloudController AppModel.blueprint(:cnb) do name { Sham.name } space { Space.make } + lifecycle_type { CNBLifecycleDataModel::LIFECYCLE_TYPE } buildpack_lifecycle_data { nil.tap { |_| object.save } } cnb_lifecycle_data { CNBLifecycleDataModel.make(app: object.save) } end @@ -65,28 +68,36 @@ module VCAP::CloudController AppModel.blueprint(:docker) do name { Sham.name } space { Space.make } + lifecycle_type { DockerLifecycleDataModel::LIFECYCLE_TYPE } buildpack_lifecycle_data { nil.tap { |_| object.save } } cnb_lifecycle_data { nil.tap { |_| object.save } } end BuildModel.blueprint do guid { Sham.guid } - app { AppModel.make } state { VCAP::CloudController::BuildModel::STAGED_STATE } + lifecycle_type { BuildpackLifecycleDataModel::LIFECYCLE_TYPE } + buildpack_lifecycle_data { BuildpackLifecycleDataModel.make(build: object.save) } + cnb_lifecycle_data { nil.tap { |_| object.save } } + app { AppModel.make } end BuildModel.blueprint(:docker) do guid { Sham.guid } - state { VCAP::CloudController::DropletModel::STAGING_STATE } - app { AppModel.make(droplet: DropletModel.make) } + state { VCAP::CloudController::BuildModel::STAGING_STATE } + lifecycle_type { DockerLifecycleDataModel::LIFECYCLE_TYPE } buildpack_lifecycle_data { nil.tap { |_| object.save } } + cnb_lifecycle_data { nil.tap { |_| object.save } } + app { AppModel.make(:docker) } end - BuildModel.blueprint(:buildpack) do + BuildModel.blueprint(:cnb) do guid { Sham.guid } - state { VCAP::CloudController::DropletModel::STAGING_STATE } - app { AppModel.make } - buildpack_lifecycle_data { BuildpackLifecycleDataModel.make(build: object.save) } + state { VCAP::CloudController::BuildModel::STAGING_STATE } + lifecycle_type { CNBLifecycleDataModel::LIFECYCLE_TYPE } + buildpack_lifecycle_data { nil.tap { |_| object.save } } + cnb_lifecycle_data { CNBLifecycleDataModel.make(build: object.save) } + app { AppModel.make(:cnb) } end PackageModel.blueprint do @@ -117,10 +128,12 @@ module VCAP::CloudController guid { Sham.guid } process_types { { 'web' => '$HOME/boot.sh' } } state { VCAP::CloudController::DropletModel::STAGED_STATE } - app { AppModel.make(droplet: object.save) } droplet_hash { Sham.guid } sha256_checksum { Sham.guid } + lifecycle_type { BuildpackLifecycleDataModel::LIFECYCLE_TYPE } buildpack_lifecycle_data { BuildpackLifecycleDataModel.make(droplet: object.save) } + cnb_lifecycle_data { nil.tap { |_| object.save } } + app { AppModel.make(droplet: object.save) } end DropletModel.blueprint(:all_fields) do @@ -141,22 +154,26 @@ module VCAP::CloudController DropletModel.blueprint(:cnb) do guid { Sham.guid } - droplet_hash { Sham.guid } - sha256_checksum { Sham.guid } process_types { { 'web' => '$HOME/boot.sh' } } state { VCAP::CloudController::DropletModel::STAGED_STATE } - app { AppModel.make(:cnb, droplet: object.save) } - cnb_lifecycle_data { CNBLifecycleDataModel.make(droplet: object.save) } + droplet_hash { Sham.guid } + sha256_checksum { Sham.guid } + lifecycle_type { CNBLifecycleDataModel::LIFECYCLE_TYPE } buildpack_lifecycle_data { nil.tap { |_| object.save } } + cnb_lifecycle_data { CNBLifecycleDataModel.make(droplet: object.save) } + app { AppModel.make(:cnb, droplet: object.save) } end DropletModel.blueprint(:docker) do guid { Sham.guid } + process_types { nil } + state { VCAP::CloudController::DropletModel::STAGED_STATE } droplet_hash { nil } sha256_checksum { nil } - state { VCAP::CloudController::DropletModel::STAGED_STATE } - app { AppModel.make(droplet: object.save) } + lifecycle_type { DockerLifecycleDataModel::LIFECYCLE_TYPE } buildpack_lifecycle_data { nil.tap { |_| object.save } } + cnb_lifecycle_data { nil.tap { |_| object.save } } + app { AppModel.make(:docker, droplet: object.save) } end DeploymentModel.blueprint do diff --git a/spec/unit/actions/app_apply_manifest_spec.rb b/spec/unit/actions/app_apply_manifest_spec.rb index dd69d6caa44..b749d280497 100644 --- a/spec/unit/actions/app_apply_manifest_spec.rb +++ b/spec/unit/actions/app_apply_manifest_spec.rb @@ -184,7 +184,7 @@ module VCAP::CloudController context 'when the default differs from what is already set on the app' do let(:message) { AppManifestMessage.create_from_yml({ name: 'blah', buildpack: buildpack.name }) } - let(:app) { AppModel.make(:buildpack) } + let(:app) { AppModel.make } it 'preserves the apps lifecycle' do app_apply_manifest.apply(app.guid, message) @@ -200,7 +200,7 @@ module VCAP::CloudController let(:buildpack) { VCAP::CloudController::Buildpack.make } let(:message) { AppManifestMessage.create_from_yml({ name: 'blah' }) } let(:app_update_message) { message.app_update_message } - let(:app) { AppModel.make(:buildpack) } + let(:app) { AppModel.make } before do TestConfig.override(default_app_lifecycle: 'buildpack') diff --git a/spec/unit/actions/app_create_spec.rb b/spec/unit/actions/app_create_spec.rb index 8730f187d4a..3501bfa0fe1 100644 --- a/spec/unit/actions/app_create_spec.rb +++ b/spec/unit/actions/app_create_spec.rb @@ -41,7 +41,6 @@ module VCAP::CloudController context 'when the request is valid' do before do expect(message).to be_valid - allow(lifecycle).to receive(:create_lifecycle_data_model) end it 'creates an app' do diff --git a/spec/unit/actions/app_start_spec.rb b/spec/unit/actions/app_start_spec.rb index bc6c7a9b6f4..9b97c225110 100644 --- a/spec/unit/actions/app_start_spec.rb +++ b/spec/unit/actions/app_start_spec.rb @@ -43,9 +43,10 @@ module VCAP::CloudController context 'when the app has a buildpack lifecycle' do let(:app) do - AppModel.make(:buildpack, - desired_state: 'STOPPED', - environment_variables: environment_variables) + AppModel.make( + desired_state: 'STOPPED', + environment_variables: environment_variables + ) end let!(:droplet) { DropletModel.make(app:) } let!(:process1) { ProcessModel.make(:process, state: 'STOPPED', app: app) } @@ -113,7 +114,7 @@ module VCAP::CloudController end context 'when the app has two associated droplets' do - let(:app) { AppModel.make(:buildpack) } + let(:app) { AppModel.make } let!(:web_process) { ProcessModel.make(app: app, type: 'web', revision: revisionA) } let!(:worker_process) { ProcessModel.make(app: app, type: 'worker', revision: revisionA) } let(:package) { PackageModel.make(app: app, state: PackageModel::READY_STATE) } @@ -198,7 +199,7 @@ module VCAP::CloudController end describe '#start_without_event' do - let(:app) { AppModel.make(:buildpack, desired_state: 'STOPPED') } + let(:app) { AppModel.make(desired_state: 'STOPPED') } it 'sets the desired state on the app' do AppStart.start_without_event(app) diff --git a/spec/unit/actions/build_delete_spec.rb b/spec/unit/actions/build_delete_spec.rb index d086a2adc15..e5fedf43f63 100644 --- a/spec/unit/actions/build_delete_spec.rb +++ b/spec/unit/actions/build_delete_spec.rb @@ -46,19 +46,17 @@ module VCAP::CloudController end it 'deletes associated buildpack lifecycle data/buildpack' do - lifecycle_data1 = BuildpackLifecycleDataModel.make(build: build1) - lifecycle_data2 = BuildpackLifecycleDataModel.make(build: build2) lifecycle_buildpack1 = BuildpackLifecycleBuildpackModel.make( - buildpack_lifecycle_data: lifecycle_data1, admin_buildpack_name: nil, buildpack_url: 'http://example.com/buildpack1' + buildpack_lifecycle_data: build1.buildpack_lifecycle_data, admin_buildpack_name: nil, buildpack_url: 'http://example.com/buildpack1' ) lifecycle_buildpack2 = BuildpackLifecycleBuildpackModel.make( - buildpack_lifecycle_data: lifecycle_data2, admin_buildpack_name: nil, buildpack_url: 'http://example.com/buildpack2' + buildpack_lifecycle_data: build2.buildpack_lifecycle_data, admin_buildpack_name: nil, buildpack_url: 'http://example.com/buildpack2' ) expect do build_delete.delete_for_app(app.guid) end.to change(BuildpackLifecycleDataModel, :count).by(-2).and change(BuildpackLifecycleBuildpackModel, :count).by(-2) - [lifecycle_data1, lifecycle_data2, lifecycle_buildpack1, lifecycle_buildpack2].each { |l| expect(l).not_to exist } + [build1.buildpack_lifecycle_data, build2.buildpack_lifecycle_data, lifecycle_buildpack1, lifecycle_buildpack2].each { |l| expect(l).not_to exist } end end end diff --git a/spec/unit/actions/droplet_copy_spec.rb b/spec/unit/actions/droplet_copy_spec.rb index d8e1b2741cf..5498b3aeb8c 100644 --- a/spec/unit/actions/droplet_copy_spec.rb +++ b/spec/unit/actions/droplet_copy_spec.rb @@ -72,6 +72,16 @@ module VCAP::CloudController end end + context 'when source and destination lifecycle types do not match' do + let!(:target_app) { VCAP::CloudController::AppModel.make(:docker, name: 'target-app-name') } + + it 'raises' do + expect do + droplet_copy.copy(target_app, user_audit_info) + end.to raise_error(/source and destination lifecycle types do not match/) + end + end + context 'when lifecycle is buildpack' do it 'creates a buildpack_lifecycle_data record for the new droplet' do expect do @@ -101,6 +111,7 @@ module VCAP::CloudController context 'when lifecycle is docker' do let(:lifecycle_type) { :docker } + let!(:target_app) { VCAP::CloudController::AppModel.make(:docker, name: 'target-app-name') } before do source_droplet.update(docker_receipt_image: 'urvashi/reddy') diff --git a/spec/unit/actions/droplet_create_spec.rb b/spec/unit/actions/droplet_create_spec.rb index ff40470b743..6c007f33d8c 100644 --- a/spec/unit/actions/droplet_create_spec.rb +++ b/spec/unit/actions/droplet_create_spec.rb @@ -160,8 +160,6 @@ module VCAP::CloudController describe '#find_or_create_buildpack_droplet' do context 'buildpack lifecycle' do - let!(:buildpack_lifecycle_data) { BuildpackLifecycleDataModel.make(build:) } - it 'sets it on the droplet' do expect do droplet_create.find_or_create_buildpack_droplet(build) @@ -174,8 +172,8 @@ module VCAP::CloudController expect(droplet.package).to eq(package) expect(droplet.build).to eq(build) - buildpack_lifecycle_data.reload - expect(buildpack_lifecycle_data.droplet).to eq(droplet) + build.buildpack_lifecycle_data.reload + expect(build.buildpack_lifecycle_data.droplet).to eq(droplet) event = Event.last expect(event.type).to eq('audit.app.droplet.create') @@ -206,8 +204,8 @@ module VCAP::CloudController expect(droplet.package).to eq(package) expect(droplet.build).to eq(build) - buildpack_lifecycle_data.reload - expect(buildpack_lifecycle_data.droplet).to eq(droplet) + build.buildpack_lifecycle_data.reload + expect(build.buildpack_lifecycle_data.droplet).to eq(droplet) end it 'is idempotent on repeated calls (no duplicate droplet, same GUID, no extra event)' do @@ -221,8 +219,8 @@ module VCAP::CloudController # Ensure still only one droplet for this build expect(DropletModel.where(build_guid: build.guid).count).to eq(1) - buildpack_lifecycle_data.reload - expect(buildpack_lifecycle_data.droplet).to eq(first) + build.buildpack_lifecycle_data.reload + expect(build.buildpack_lifecycle_data.droplet).to eq(first) end it 'returns the pre-existing droplet when one already exists for the build' do @@ -232,7 +230,7 @@ module VCAP::CloudController build: build, state: DropletModel::STAGING_STATE ) - buildpack_lifecycle_data.update(droplet: existing) + build.buildpack_lifecycle_data.update(droplet: existing) expect do returned = droplet_create.find_or_create_buildpack_droplet(build) @@ -273,7 +271,15 @@ module VCAP::CloudController end context 'cnb lifecycle' do - let!(:cnb_lifecycle_data) { CNBLifecycleDataModel.make(build:) } + let(:app) { AppModel.make(:cnb) } + let(:build) do + BuildModel.make(:cnb, + app: app, + package: package, + created_by_user_guid: 'schneider', + created_by_user_email: 'bob@loblaw.com', + created_by_user_name: 'bobert') + end it 'sets it on the droplet' do expect do @@ -287,8 +293,8 @@ module VCAP::CloudController expect(droplet.package).to eq(package) expect(droplet.build).to eq(build) - cnb_lifecycle_data.reload - expect(cnb_lifecycle_data.droplet).to eq(droplet) + build.cnb_lifecycle_data.reload + expect(build.cnb_lifecycle_data.droplet).to eq(droplet) event = Event.last expect(event.type).to eq('audit.app.droplet.create') @@ -310,10 +316,7 @@ module VCAP::CloudController context 'when the build does not contain created_by fields' do let(:build) do - BuildModel.make( - app:, - package: - ) + BuildModel.make(:cnb, app:, package:) end it 'sets the actor to UNKNOWN' do diff --git a/spec/unit/actions/droplet_update_spec.rb b/spec/unit/actions/droplet_update_spec.rb index ea7c0c35ddd..f828173a24a 100644 --- a/spec/unit/actions/droplet_update_spec.rb +++ b/spec/unit/actions/droplet_update_spec.rb @@ -74,7 +74,7 @@ module VCAP::CloudController end context 'when the droplet type is buildpack' do - let!(:droplet) { DropletModel.make(:buildpack, app: nil) } + let!(:droplet) { DropletModel.make(app: nil) } let(:message) do VCAP::CloudController::DropletUpdateMessage.new({ diff --git a/spec/unit/actions/v2/app_stage_spec.rb b/spec/unit/actions/v2/app_stage_spec.rb index 430c3261dc1..c93277ba5f4 100644 --- a/spec/unit/actions/v2/app_stage_spec.rb +++ b/spec/unit/actions/v2/app_stage_spec.rb @@ -15,7 +15,7 @@ module V2 let(:app) { AppModel.make } let(:package) { PackageModel.make(app: app, state: PackageModel::READY_STATE) } - let(:build) { BuildModel.make(:buildpack, app: app, package: package, created_by_user_guid: 'user-guid') } + let(:build) { BuildModel.make(app: app, package: package, created_by_user_guid: 'user-guid') } let(:build_create) { instance_double(BuildCreate, create_and_stage_without_event: build, staging_response: 'staging-response', warnings: []) } before do diff --git a/spec/unit/actions/v2/app_update_spec.rb b/spec/unit/actions/v2/app_update_spec.rb index a413f2855f5..6e783cde4fc 100644 --- a/spec/unit/actions/v2/app_update_spec.rb +++ b/spec/unit/actions/v2/app_update_spec.rb @@ -416,7 +416,7 @@ module VCAP::CloudController end context 'when changing from buildpack to docker' do - let(:process) { ProcessModel.make(app: AppModel.make(:buildpack)) } + let(:process) { ProcessModel.make(app: AppModel.make) } let(:app) { process.app } it 'raises an error' do diff --git a/spec/unit/controllers/internal/staging_completion_controller_spec.rb b/spec/unit/controllers/internal/staging_completion_controller_spec.rb index af46ba8ed13..eb338221466 100644 --- a/spec/unit/controllers/internal/staging_completion_controller_spec.rb +++ b/spec/unit/controllers/internal/staging_completion_controller_spec.rb @@ -60,6 +60,8 @@ module VCAP::CloudController let(:staging_guid) { droplet.guid } context 'when it is a docker app' do + let(:staged_app) { AppModel.make(:docker) } + let(:package) { PackageModel.make(:docker, state: 'READY', app_guid: staged_app.guid) } let(:droplet) { DropletModel.make(:docker, package_guid: package.guid, app_guid: staged_app.guid, state: DropletModel::STAGING_STATE) } it 'calls the stager with a build created from the droplet and the response' do @@ -207,7 +209,6 @@ module VCAP::CloudController let(:package) { PackageModel.make(state: 'READY', app_guid: staged_app.guid) } let!(:droplet) { DropletModel.make } let(:build) { BuildModel.make(package_guid: package.guid, app: staged_app) } - let!(:lifecycle_data) { BuildpackLifecycleDataModel.make(buildpacks: [buildpack_name], stack: 'cflinuxfs4', build: build) } let(:staging_guid) { build.guid } let(:buildpacks) do [ @@ -226,6 +227,7 @@ module VCAP::CloudController before do build.droplet = droplet + build.buildpack_lifecycle_data.update(buildpacks: [buildpack_name], stack: 'cflinuxfs4') end it 'calls the stager with the build and response' do diff --git a/spec/unit/controllers/runtime/apps_controller_spec.rb b/spec/unit/controllers/runtime/apps_controller_spec.rb index d3231f87a60..83f33b9dac4 100644 --- a/spec/unit/controllers/runtime/apps_controller_spec.rb +++ b/spec/unit/controllers/runtime/apps_controller_spec.rb @@ -1015,7 +1015,7 @@ def update_app end context 'when changing from buildpack to docker' do - let(:process) { ProcessModel.make(app: AppModel.make(:buildpack)) } + let(:process) { ProcessModel.make(app: AppModel.make) } it 'raises an error' do put "/v2/apps/#{process.app.guid}", Oj.dump({ docker_image: 'repo/great-image' }) diff --git a/spec/unit/controllers/v3/apps_controller_spec.rb b/spec/unit/controllers/v3/apps_controller_spec.rb index beee5e6ed5b..f74f841b593 100644 --- a/spec/unit/controllers/v3/apps_controller_spec.rb +++ b/spec/unit/controllers/v3/apps_controller_spec.rb @@ -574,7 +574,7 @@ end describe '#update' do - let(:app_model) { VCAP::CloudController::AppModel.make(:buildpack) } + let(:app_model) { VCAP::CloudController::AppModel.make } let(:space) { app_model.space } let(:org) { space.organization } @@ -1089,7 +1089,7 @@ describe '#start' do let(:app_model) { VCAP::CloudController::AppModel.make(droplet_guid: droplet.guid) } - let(:droplet) { VCAP::CloudController::DropletModel.make(:buildpack, state: VCAP::CloudController::DropletModel::STAGED_STATE) } + let(:droplet) { VCAP::CloudController::DropletModel.make(state: VCAP::CloudController::DropletModel::STAGED_STATE) } let(:space) { app_model.space } let(:org) { space.organization } let(:user) { set_current_user(VCAP::CloudController::User.make) } diff --git a/spec/unit/controllers/v3/droplets_controller_spec.rb b/spec/unit/controllers/v3/droplets_controller_spec.rb index c197eda0b75..766d9263bf2 100644 --- a/spec/unit/controllers/v3/droplets_controller_spec.rb +++ b/spec/unit/controllers/v3/droplets_controller_spec.rb @@ -27,7 +27,7 @@ let(:source_app_guid) { VCAP::CloudController::AppModel.make(space_guid: source_space.guid).guid } let(:target_app_guid) { target_app.guid } let(:state) { 'STAGED' } - let!(:source_droplet) { VCAP::CloudController::DropletModel.make(:buildpack, state: state, app_guid: source_app_guid) } + let!(:source_droplet) { VCAP::CloudController::DropletModel.make(state: state, app_guid: source_app_guid) } let(:source_droplet_guid) { source_droplet.guid } let(:request_body) do { diff --git a/spec/unit/fetchers/app_list_fetcher_spec.rb b/spec/unit/fetchers/app_list_fetcher_spec.rb index 856f352abbc..62be83c9695 100644 --- a/spec/unit/fetchers/app_list_fetcher_spec.rb +++ b/spec/unit/fetchers/app_list_fetcher_spec.rb @@ -204,16 +204,16 @@ module VCAP::CloudController context 'filtering timestamps' do before do - AppModel.plugin :timestamps, update_on_create: false + AppModel.plugin :timestamps, update_on_create: true, allow_manual_update: true end - let!(:resource_1) { AppModel.create(name: '1', created_at: '2020-05-26T18:47:01Z', updated_at: '2020-05-26T18:47:01Z', space: space) } - let!(:resource_2) { AppModel.create(name: '2', created_at: '2020-05-26T18:47:02Z', updated_at: '2020-05-26T18:47:02Z', space: space) } - let!(:resource_3) { AppModel.create(name: '3', created_at: '2020-05-26T18:47:03Z', updated_at: '2020-05-26T18:47:03Z', space: space) } - let!(:resource_4) { AppModel.create(name: '4', created_at: '2020-05-26T18:47:04Z', updated_at: '2020-05-26T18:47:04Z', space: space) } + let!(:resource_1) { AppModel.make(name: '1', created_at: '2020-05-26T18:47:01Z', space: space).tap { |r| r.update(updated_at: '2020-05-26T18:47:01Z') } } + let!(:resource_2) { AppModel.make(name: '2', created_at: '2020-05-26T18:47:02Z', space: space).tap { |r| r.update(updated_at: '2020-05-26T18:47:02Z') } } + let!(:resource_3) { AppModel.make(name: '3', created_at: '2020-05-26T18:47:03Z', space: space).tap { |r| r.update(updated_at: '2020-05-26T18:47:03Z') } } + let!(:resource_4) { AppModel.make(name: '4', created_at: '2020-05-26T18:47:04Z', space: space).tap { |r| r.update(updated_at: '2020-05-26T18:47:04Z') } } after do - AppModel.plugin :timestamps, update_on_create: true + AppModel.plugin :timestamps, update_on_create: true, allow_manual_update: false end context 'filtering on created_at' do diff --git a/spec/unit/jobs/v3/buildpack_cache_upload_spec.rb b/spec/unit/jobs/v3/buildpack_cache_upload_spec.rb index ec0a7947a6c..722daec0ea1 100644 --- a/spec/unit/jobs/v3/buildpack_cache_upload_spec.rb +++ b/spec/unit/jobs/v3/buildpack_cache_upload_spec.rb @@ -5,7 +5,7 @@ module Jobs::V3 RSpec.describe BuildpackCacheUpload, job_context: :api do subject(:job) { BuildpackCacheUpload.new(local_path: local_file.path, app_guid: app.guid, stack_name: 'some-stack') } - let(:app) { AppModel.make(:buildpack) } + let(:app) { AppModel.make } let(:file_content) { 'some_file_content' } let(:local_file) do diff --git a/spec/unit/jobs/v3/droplet_bits_copier_spec.rb b/spec/unit/jobs/v3/droplet_bits_copier_spec.rb index f647632a2e8..f4e4ec0616b 100644 --- a/spec/unit/jobs/v3/droplet_bits_copier_spec.rb +++ b/spec/unit/jobs/v3/droplet_bits_copier_spec.rb @@ -11,8 +11,8 @@ module Jobs::V3 CloudController::Blobstore::FogClient.new(connection_config: { provider: 'Local', local_root: blobstore_dir }, directory_key: 'droplet') end - let(:source_droplet) { DropletModel.make(:buildpack, droplet_hash: 'abcdef1234', sha256_checksum: '4321fedcba', state: DropletModel::STAGED_STATE, app: nil) } - let(:destination_droplet) { DropletModel.make(:buildpack, droplet_hash: nil, sha256_checksum: nil, state: DropletModel::STAGING_STATE, app: nil) } + let(:source_droplet) { DropletModel.make(droplet_hash: 'abcdef1234', sha256_checksum: '4321fedcba', state: DropletModel::STAGED_STATE, app: nil) } + let(:destination_droplet) { DropletModel.make(droplet_hash: nil, sha256_checksum: nil, state: DropletModel::STAGING_STATE, app: nil) } before do Fog.unmock! diff --git a/spec/unit/lib/cloud_controller/diego/app_recipe_builder_spec.rb b/spec/unit/lib/cloud_controller/diego/app_recipe_builder_spec.rb index 84da2e95051..267efcdd9d3 100644 --- a/spec/unit/lib/cloud_controller/diego/app_recipe_builder_spec.rb +++ b/spec/unit/lib/cloud_controller/diego/app_recipe_builder_spec.rb @@ -1019,13 +1019,9 @@ module Diego let(:ports) { '8080' } before do - VCAP::CloudController::BuildpackLifecycleDataModel.make( - app: app_model, - buildpacks: nil, - stack: 'potato-stack' - ) + app_model.cnb_lifecycle_data.update(stack: 'potato-stack') - allow(VCAP::CloudController::Diego::Buildpack::DesiredLrpBuilder).to receive(:new).and_return(desired_lrp_builder) + allow(VCAP::CloudController::Diego::CNB::DesiredLrpBuilder).to receive(:new).and_return(desired_lrp_builder) end it_behaves_like 'creating a desired lrp' @@ -1414,7 +1410,7 @@ module Diego describe '#build_app_lrp_update' do let(:config) { Config.new({}) } - let(:app_model) { AppModel.make(:buildpack, guid: 'app-guid', droplet: DropletModel.make(state: 'STAGED')) } + let(:app_model) { AppModel.make(guid: 'app-guid', droplet: DropletModel.make(state: 'STAGED')) } let(:process) do process = ProcessModel.make(:process, instances: 7, app: app_model) process.this.update(updated_at: Time.at(2)) diff --git a/spec/unit/lib/cloud_controller/diego/buildpack/staging_action_builder_spec.rb b/spec/unit/lib/cloud_controller/diego/buildpack/staging_action_builder_spec.rb index 78e60c95f6d..03b611d1058 100644 --- a/spec/unit/lib/cloud_controller/diego/buildpack/staging_action_builder_spec.rb +++ b/spec/unit/lib/cloud_controller/diego/buildpack/staging_action_builder_spec.rb @@ -6,7 +6,7 @@ module Buildpack RSpec.describe StagingActionBuilder do subject(:builder) { StagingActionBuilder.new(config, staging_details, lifecycle_data) } - let(:droplet) { DropletModel.make(:buildpack) } + let(:droplet) { DropletModel.make } let(:enable_declarative_asset_downloads) { false } let(:legacy_md5_buildpack_paths_enabled) { false } let(:config) do diff --git a/spec/unit/lib/cloud_controller/diego/cnb/staging_action_builder_spec.rb b/spec/unit/lib/cloud_controller/diego/cnb/staging_action_builder_spec.rb index 6fe185b74dd..d3242223388 100644 --- a/spec/unit/lib/cloud_controller/diego/cnb/staging_action_builder_spec.rb +++ b/spec/unit/lib/cloud_controller/diego/cnb/staging_action_builder_spec.rb @@ -7,7 +7,7 @@ module CNB RSpec.describe StagingActionBuilder do subject(:builder) { StagingActionBuilder.new(config, staging_details, lifecycle_data) } - let(:droplet) { DropletModel.make(:buildpack) } + let(:droplet) { DropletModel.make } let(:enable_declarative_asset_downloads) { false } let(:legacy_md5_buildpack_paths_enabled) { false } let(:config) do diff --git a/spec/unit/lib/cloud_controller/diego/docker/staging_completion_handler_spec.rb b/spec/unit/lib/cloud_controller/diego/docker/staging_completion_handler_spec.rb index ac82223d250..2dc05254508 100644 --- a/spec/unit/lib/cloud_controller/diego/docker/staging_completion_handler_spec.rb +++ b/spec/unit/lib/cloud_controller/diego/docker/staging_completion_handler_spec.rb @@ -6,14 +6,15 @@ module Diego module Docker RSpec.describe StagingCompletionHandler do let(:logger) { instance_double(Steno::Logger, info: nil, error: nil, warn: nil) } - let(:app) { AppModel.make } - let(:package) { PackageModel.make(app:) } - let!(:build) { BuildModel.make(app: app, package: package, state: BuildModel::STAGING_STATE) } + let(:app) { AppModel.make(:docker) } + let(:package) { PackageModel.make(:docker, app:) } + let!(:build) { BuildModel.make(:docker, app: app, package: package, state: BuildModel::STAGING_STATE) } let(:runners) { instance_double(Runners) } subject(:handler) { StagingCompletionHandler.new(build, runners) } before do + FeatureFlag.create(name: 'diego_docker', enabled: true) allow(Steno).to receive(:logger).and_return(logger) allow(VCAP::AppLogEmitter).to receive(:emit_error) set_current_user_as_admin(user: User.make(guid: '1234'), email: 'joe@joe.com', user_name: 'briggs') 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 e57986cb339..f8b4d0529bd 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 @@ -86,7 +86,7 @@ module VCAP::CloudController end describe '#update_lifecycle_data_model' do - let(:app) { AppModel.make(:buildpack) } + let(:app) { AppModel.make } let!(:ruby_buildpack) { Buildpack.make(name: 'ruby_buildpack') } let(:lifecycle_request_data) { { buildpacks: ['http://oj.com', 'ruby_buildpack'], stack: 'sweetness' } } diff --git a/spec/unit/lib/cloud_controller/diego/lifecycles/app_cnb_lifecycle_spec.rb b/spec/unit/lib/cloud_controller/diego/lifecycles/app_cnb_lifecycle_spec.rb index 27e28307c53..776238e5423 100644 --- a/spec/unit/lib/cloud_controller/diego/lifecycles/app_cnb_lifecycle_spec.rb +++ b/spec/unit/lib/cloud_controller/diego/lifecycles/app_cnb_lifecycle_spec.rb @@ -82,7 +82,7 @@ module VCAP::CloudController end describe '#update_lifecycle_data_model' do - let(:app) { AppModel.make(:buildpack) } + let(:app) { AppModel.make } let(:lifecycle_request_data) { { buildpacks: ['http://oj.com', 'http://acme.com'], stack: 'sweetness' } } it 'updates the CNBLifecycleDataModel' do diff --git a/spec/unit/lib/cloud_controller/diego/lifecycles/app_lifecycle_provider_spec.rb b/spec/unit/lib/cloud_controller/diego/lifecycles/app_lifecycle_provider_spec.rb index 31b809e0c1b..21b9462aa9e 100644 --- a/spec/unit/lib/cloud_controller/diego/lifecycles/app_lifecycle_provider_spec.rb +++ b/spec/unit/lib/cloud_controller/diego/lifecycles/app_lifecycle_provider_spec.rb @@ -86,7 +86,7 @@ module VCAP::CloudController let(:request) { {} } context 'the app is buildpack type' do - let(:app) { AppModel.make(:buildpack) } + let(:app) { AppModel.make } it 'returns a AppBuildpackLifecycle' do expect(AppLifecycleProvider.provide_for_update(message, app)).to be_a(AppBuildpackLifecycle) diff --git a/spec/unit/lib/cloud_controller/diego/lifecycles/buildpack_lifecycle_spec.rb b/spec/unit/lib/cloud_controller/diego/lifecycles/buildpack_lifecycle_spec.rb index 44a4e27508d..7868b03de04 100644 --- a/spec/unit/lib/cloud_controller/diego/lifecycles/buildpack_lifecycle_spec.rb +++ b/spec/unit/lib/cloud_controller/diego/lifecycles/buildpack_lifecycle_spec.rb @@ -40,7 +40,7 @@ module VCAP::CloudController end context 'when the user does not specify buildpacks' do - let(:app) { AppModel.make(:buildpack, name: 'some-app', space: Space.make) } + let(:app) { AppModel.make(name: 'some-app', space: Space.make) } let(:request_data) { {} } context 'when the app has buildpacks' do diff --git a/spec/unit/lib/cloud_controller/diego/lifecycles/lifecycle_provider_spec.rb b/spec/unit/lib/cloud_controller/diego/lifecycles/lifecycle_provider_spec.rb index 8de0674418d..df26827758b 100644 --- a/spec/unit/lib/cloud_controller/diego/lifecycles/lifecycle_provider_spec.rb +++ b/spec/unit/lib/cloud_controller/diego/lifecycles/lifecycle_provider_spec.rb @@ -38,7 +38,7 @@ module VCAP::CloudController let(:package) { PackageModel.make(app_guid: app.guid) } context 'when the app defaults to buildpack' do - let(:app) { AppModel.make(:buildpack) } + let(:app) { AppModel.make } it 'returns a BuildpackLifecycle' do expect(LifecycleProvider.provide(package, message)).to be_a(BuildpackLifecycle) diff --git a/spec/unit/lib/cloud_controller/diego/stager_spec.rb b/spec/unit/lib/cloud_controller/diego/stager_spec.rb index 7299313ab71..75a42539f0c 100644 --- a/spec/unit/lib/cloud_controller/diego/stager_spec.rb +++ b/spec/unit/lib/cloud_controller/diego/stager_spec.rb @@ -122,7 +122,6 @@ module Diego context 'buildpack' do let(:build) { BuildModel.make } - let!(:lifecycle_data_model) { BuildpackLifecycleDataModel.make(build:) } it 'delegates to a buildpack staging completion handler' do stager.staging_complete(build, staging_response) @@ -133,7 +132,6 @@ module Diego context 'docker' do let(:build) { BuildModel.make(:docker) } - let!(:lifecycle_data_model) { nil } it 'delegates to a docker staging completion handler' do stager.staging_complete(build, staging_response) @@ -143,8 +141,7 @@ module Diego end context 'cnb' do - let(:build) { BuildModel.make } - let!(:lifecycle_data_model) { CNBLifecycleDataModel.make(build:) } + let(:build) { BuildModel.make(:cnb) } it 'delegates to a cnb staging completion handler' do stager.staging_complete(build, staging_response) diff --git a/spec/unit/lib/cloud_controller/diego/task_environment_spec.rb b/spec/unit/lib/cloud_controller/diego/task_environment_spec.rb index b69606e91bc..fb336b0d777 100644 --- a/spec/unit/lib/cloud_controller/diego/task_environment_spec.rb +++ b/spec/unit/lib/cloud_controller/diego/task_environment_spec.rb @@ -79,7 +79,7 @@ module VCAP::CloudController::Diego end context 'when the task is for a buildpack app' do - let(:app) { VCAP::CloudController::AppModel.make(:buildpack) } + let(:app) { VCAP::CloudController::AppModel.make } it 'sets the LANG environment variable' do constructed_envs = TaskEnvironment.new(app, task, space).build diff --git a/spec/unit/lib/cloud_controller/diego/task_recipe_builder_spec.rb b/spec/unit/lib/cloud_controller/diego/task_recipe_builder_spec.rb index e0457530a29..1ca13d4650b 100644 --- a/spec/unit/lib/cloud_controller/diego/task_recipe_builder_spec.rb +++ b/spec/unit/lib/cloud_controller/diego/task_recipe_builder_spec.rb @@ -163,7 +163,7 @@ module Diego end context 'with a buildpack backend' do - let(:droplet) { DropletModel.make(:buildpack, package:, app:) } + let(:droplet) { DropletModel.make(package:, app:) } let(:buildpack_staging_action) { ::Diego::Bbs::Models::RunAction.new } let(:lifecycle_environment_variables) { [::Diego::Bbs::Models::EnvironmentVariable.new(name: 'the-buildpack-env-var', value: 'the-buildpack-value')] } @@ -500,7 +500,7 @@ module Diego end context 'with a buildpack backend' do - let(:droplet) { DropletModel.make(:buildpack, app:) } + let(:droplet) { DropletModel.make(app:) } let(:task_action_builder) do instance_double( diff --git a/spec/unit/models/runtime/app_model_spec.rb b/spec/unit/models/runtime/app_model_spec.rb index e7682319ecf..934b6e1707c 100644 --- a/spec/unit/models/runtime/app_model_spec.rb +++ b/spec/unit/models/runtime/app_model_spec.rb @@ -284,33 +284,47 @@ module VCAP::CloudController end.to raise_error(Sequel::ValidationFailed, /droplet presence/) end end + + it 'validates lifecycle_type is one of buildpack, cnb, or docker' do + expect do + AppModel.make(lifecycle_type: 'invalid') + end.to raise_error(Sequel::ValidationFailed, /lifecycle_type/) + end + + it 'accepts valid lifecycle_type values' do + %w[buildpack cnb docker].each do |type| + expect do + AppModel.make(lifecycle_type: type) + end.not_to raise_error + end + end end describe '#lifecycle_type' do - context 'the model contains buildpack_lifecycle_data' do + before do + # Remove lifecycle type to test fallback mechanism based on associated lifecycle data + app_model.update(lifecycle_type: '') + end + + context 'when there is buildpack_lifecycle_data associated to the app' do before { BuildpackLifecycleDataModel.make(app: app_model) } - it 'returns the string "buildpack" if buildpack_lifecycle_data is on the model' do + it 'returns the string "buildpack"' do app_model.reload expect(app_model.lifecycle_type).to eq('buildpack') end end - context 'the model contains cnb_lifecycle_data' do + context 'when there is cnb_lifecycle_data associated to the app' do before { CNBLifecycleDataModel.make(app: app_model) } - it 'returns the string "cnb" if cnb_lifecycle_data is on the model' do + it 'returns the string "cnb"' do app_model.reload expect(app_model.lifecycle_type).to eq('cnb') end end - context 'the model does not contain any lifecycle_data' do - before do - app_model.buildpack_lifecycle_data = nil - app_model.save - end - + context 'when there is no lifecycle data associated to the app' do it 'returns the string "docker"' do expect(app_model.lifecycle_type).to eq('docker') end @@ -319,36 +333,44 @@ module VCAP::CloudController describe '#lifecycle_data' do context 'buildpack_lifecycle_data' do - let!(:buildpack_lifecycle_data) { BuildpackLifecycleDataModel.make(app: app_model) } + let!(:app_model) { AppModel.make } - it 'returns buildpack_lifecycle_data if it is on the model' do - expect(app_model.reload.lifecycle_data).to eq(buildpack_lifecycle_data) + it 'returns a buildpack lifecycle data model' do + expect(app_model.lifecycle_data).to be_a(BuildpackLifecycleDataModel) + expect(app_model.lifecycle_data).to eq(app_model.buildpack_lifecycle_data) + expect(app_model.cnb_lifecycle_data).to be_nil end - it 'is a persistable hash' do - expect(app_model.reload.lifecycle_data.buildpacks).to eq(buildpack_lifecycle_data.buildpacks) - expect(app_model.reload.lifecycle_data.stack).to eq(buildpack_lifecycle_data.stack) + it 'deletes the dependent buildpack_lifecycle_data model when an app is deleted' do + expect do + app_model.destroy + end.to change(BuildpackLifecycleDataModel, :count).by(-1) end end context 'cnb_lifecycle_data' do - let!(:cnb_lifecycle_data) { CNBLifecycleDataModel.make(app: app_model) } + let!(:app_model) { AppModel.make(:cnb) } - it 'returns cnb_lifecycle_data if it is on the model' do - expect(app_model.reload.lifecycle_data).to eq(cnb_lifecycle_data) + it 'returns a cnb lifecycle data model' do + expect(app_model.lifecycle_data).to be_a(CNBLifecycleDataModel) + expect(app_model.lifecycle_data).to eq(app_model.cnb_lifecycle_data) + expect(app_model.buildpack_lifecycle_data).to be_nil end - it 'is a persistable hash' do - expect(app_model.reload.cnb_lifecycle_data.buildpacks).to eq(cnb_lifecycle_data.buildpacks) - expect(app_model.reload.cnb_lifecycle_data.stack).to eq(cnb_lifecycle_data.stack) + it 'deletes the dependent cnb_lifecycle_data when an app is deleted' do + expect do + app_model.destroy + end.to change(CNBLifecycleDataModel, :count).by(-1) end end - context 'lifecycle_data is nil' do - let(:non_buildpack_app_model) { AppModel.create(name: 'non-buildpack', space: space) } + context 'neither buildpack_lifecycle_data, nor cnb_lifecycle_data' do + let(:app_model) { AppModel.make(:docker) } - it 'returns a docker data model' do - expect(non_buildpack_app_model.lifecycle_data).to be_a(DockerLifecycleDataModel) + it 'returns a docker lifecycle data model' do + expect(app_model.lifecycle_data).to be_a(DockerLifecycleDataModel) + expect(app_model.buildpack_lifecycle_data).to be_nil + expect(app_model.cnb_lifecycle_data).to be_nil end end end diff --git a/spec/unit/models/runtime/build_model_spec.rb b/spec/unit/models/runtime/build_model_spec.rb index ad40897a054..8fd7faeca19 100644 --- a/spec/unit/models/runtime/build_model_spec.rb +++ b/spec/unit/models/runtime/build_model_spec.rb @@ -49,128 +49,106 @@ module VCAP::CloudController end describe '#lifecycle_type' do - context 'buildpack_lifecycle_data' do - let!(:buildpack_lifecycle_data) do - BuildpackLifecycleDataModel.make( - build: build_model, - buildpacks: ['http://some-buildpack.com', 'http://another-buildpack.net'] - ) - end + before do + # Remove lifecycle type to test fallback mechanism based on associated lifecycle data + build_model.update(lifecycle_type: '') + end - before do - build_model.buildpack_lifecycle_data = buildpack_lifecycle_data - build_model.save - end + context 'when there is buildpack_lifecycle_data associated to the build' do + let(:build_model) { BuildModel.make(app: nil) } it 'returns the string "buildpack"' do expect(build_model.lifecycle_type).to eq('buildpack') end end - context 'cnb_lifecycle_data' do - let!(:cnb_lifecycle_data) do - CNBLifecycleDataModel.make( - build: build_model, - buildpacks: ['http://some-buildpack.com', 'http://another-buildpack.net'] - ) - end - - before do - build_model.cnb_lifecycle_data = cnb_lifecycle_data - build_model.save - end + context 'when there is cnb_lifecycle_data associated to the build' do + let(:build_model) { BuildModel.make(:cnb, app: nil) } it 'returns the string "cnb"' do expect(build_model.lifecycle_type).to eq('cnb') end end - context 'no lifecycle_data' do - it 'returns the string "docker"' do - build_model.buildpack_lifecycle_data = nil - build_model.save + context 'when there is no lifecycle data associated to the build' do + let(:build_model) { BuildModel.make(:docker, app: nil) } + it 'returns the string "docker"' do expect(build_model.lifecycle_type).to eq('docker') end end end - describe '#lifecycle_data' do - context 'buildpack_lifecycle_data' do - let!(:buildpack_lifecycle_data) do - BuildpackLifecycleDataModel.make( - build: build_model, - buildpacks: ['http://some-buildpack.com', 'http://another-buildpack.net'] - ) - end + describe '#before_create' do + it 'inherits lifecycle_type from app if not set' do + app = AppModel.make(:cnb) + build = BuildModel.create(app: app, state: BuildModel::STAGING_STATE) + expect(build[:lifecycle_type]).to eq('cnb') + end - before do - build_model.buildpack_lifecycle_data = buildpack_lifecycle_data - build_model.save - end + it 'uses explicit lifecycle_type if set' do + app = AppModel.make + build = BuildModel.create(app: app, state: BuildModel::STAGING_STATE, lifecycle_type: 'docker') + expect(build[:lifecycle_type]).to eq('docker') + end + end - it 'returns buildpack_lifecycle_data' do - expect(build_model.lifecycle_data).to eq(buildpack_lifecycle_data) - end + describe 'validations' do + it 'validates lifecycle_type is one of buildpack, cnb, or docker' do + expect do + BuildModel.make(lifecycle_type: 'invalid') + end.to raise_error(Sequel::ValidationFailed, /lifecycle_type/) + end - it 'is a persistable hash' do - expect(build_model.reload.lifecycle_data.buildpacks).to eq(buildpack_lifecycle_data.buildpacks) - expect(build_model.reload.lifecycle_data.stack).to eq(buildpack_lifecycle_data.stack) + it 'accepts valid lifecycle_type values' do + %w[buildpack cnb docker].each do |type| + expect do + BuildModel.make(lifecycle_type: type) + end.not_to raise_error end end + end - context 'cnb_lifecycle_data' do - let!(:cnb_lifecycle_data) do - CNBLifecycleDataModel.make( - build: build_model, - buildpacks: ['http://some-buildpack.com', 'http://another-buildpack.net'] - ) - end + describe '#lifecycle_data' do + context 'buildpack_lifecycle_data' do + let!(:build_model) { BuildModel.make(app: nil) } - before do - build_model.cnb_lifecycle_data = cnb_lifecycle_data - build_model.save + it 'returns a buildpack lifecycle data model' do + expect(build_model.lifecycle_data).to be_a(BuildpackLifecycleDataModel) + expect(build_model.lifecycle_data).to eq(build_model.buildpack_lifecycle_data) + expect(build_model.cnb_lifecycle_data).to be_nil end - it 'returns cnb_lifecycle_data' do - expect(build_model.lifecycle_data).to eq(cnb_lifecycle_data) + it 'deletes the dependent buildpack_lifecycle_data model when a build is deleted' do + expect do + build_model.destroy + end.to change(BuildpackLifecycleDataModel, :count).by(-1) end + end + + context 'cnb_lifecycle_data' do + let!(:build_model) { BuildModel.make(:cnb, app: nil) } - it 'is a persistable hash' do - expect(build_model.reload.cnb_lifecycle_data.buildpacks).to eq(cnb_lifecycle_data.buildpacks) - expect(build_model.reload.cnb_lifecycle_data.stack).to eq(cnb_lifecycle_data.stack) + it 'returns a cnb lifecycle data model' do + expect(build_model.lifecycle_data).to be_a(CNBLifecycleDataModel) + expect(build_model.lifecycle_data).to eq(build_model.cnb_lifecycle_data) + expect(build_model.buildpack_lifecycle_data).to be_nil end - it 'deletes the dependent cnb_lifecycle_data_models when a build is deleted' do + it 'deletes the dependent cnb_lifecycle_data when a build is deleted' do expect do build_model.destroy - end.to change(CNBLifecycleDataModel, :count).by(-1). - and change(BuildpackLifecycleBuildpackModel, :count).by(-2) + end.to change(CNBLifecycleDataModel, :count).by(-1) end end - context 'no lifecycle_data' do - it 'returns a docker lifecycle model' do - build_model.buildpack_lifecycle_data = nil - build_model.save + context 'neither buildpack_lifecycle_data, nor cnb_lifecycle_data' do + let(:build_model) { BuildModel.make(:docker, app: nil) } + it 'returns a docker lifecycle data model' do expect(build_model.lifecycle_data).to be_a(DockerLifecycleDataModel) - end - end - - context 'buildpack dependencies' do - let!(:buildpack_lifecycle_data) do - BuildpackLifecycleDataModel.make( - build: build_model, - buildpacks: ['http://some-buildpack.com', 'http://another-buildpack.net'] - ) - end - - it 'deletes the dependent buildpack_lifecycle_data_models when a build is deleted' do - expect do - build_model.destroy - end.to change(BuildpackLifecycleDataModel, :count).by(-1). - and change(BuildpackLifecycleBuildpackModel, :count).by(-2) + expect(build_model.buildpack_lifecycle_data).to be_nil + expect(build_model.cnb_lifecycle_data).to be_nil end end end diff --git a/spec/unit/models/runtime/droplet_model_spec.rb b/spec/unit/models/runtime/droplet_model_spec.rb index 554ae2ee192..032b551c267 100644 --- a/spec/unit/models/runtime/droplet_model_spec.rb +++ b/spec/unit/models/runtime/droplet_model_spec.rb @@ -64,15 +64,13 @@ module VCAP::CloudController end describe '#lifecycle_type' do - context 'when there is buildpack_lifecycle_data associated to the droplet' do - let(:droplet_model) { DropletModel.make(:buildpack, app: nil) } - let!(:lifecycle_data) { BuildpackLifecycleDataModel.make(droplet: droplet_model) } + before do + # Remove lifecycle type to test fallback mechanism based on associated lifecycle data + droplet_model.update(lifecycle_type: '') + end - before do - droplet_model.buildpack_lifecycle_data = lifecycle_data - droplet_model.cnb_lifecycle_data = nil - droplet_model.save - end + context 'when there is buildpack_lifecycle_data associated to the droplet' do + let(:droplet_model) { DropletModel.make(app: nil) } it 'returns the string "buildpack"' do expect(droplet_model.lifecycle_type).to eq('buildpack') @@ -80,14 +78,7 @@ module VCAP::CloudController end context 'when there is cnb_lifecycle_data associated to the droplet' do - let(:droplet_model) { DropletModel.make(:buildpack, app: nil) } - let!(:lifecycle_data) { CNBLifecycleDataModel.make(droplet: droplet_model) } - - before do - droplet_model.cnb_lifecycle_data = lifecycle_data - droplet_model.buildpack_lifecycle_data = nil - droplet_model.save - end + let(:droplet_model) { DropletModel.make(:cnb, app: nil) } it 'returns the string "cnb"' do expect(droplet_model.lifecycle_type).to eq('cnb') @@ -97,90 +88,82 @@ module VCAP::CloudController context 'when there is no lifecycle data associated to the droplet' do let(:droplet_model) { DropletModel.make(:docker, app: nil) } - before do - droplet_model.buildpack_lifecycle_data = nil - droplet_model.save - end - it 'returns the string "docker"' do expect(droplet_model.lifecycle_type).to eq('docker') end end end - describe '#lifecycle_data' do - context 'when there is buildpack_lifecycle_data associated to the droplet' do - let(:droplet_model) { DropletModel.make(:buildpack, app: nil) } - let!(:lifecycle_data) do - BuildpackLifecycleDataModel.make( - droplet: droplet_model, - buildpacks: ['http://some-buildpack.com', 'http://another-buildpack.net'] - ) - end - - before do - droplet_model.buildpack_lifecycle_data = lifecycle_data - droplet_model.save - end + describe '#before_create' do + it 'inherits lifecycle_type from app if not set' do + app = AppModel.make(:cnb) + droplet = DropletModel.create(app: app, state: DropletModel::STAGING_STATE) + expect(droplet[:lifecycle_type]).to eq('cnb') + end - it 'returns buildpack_lifecycle_data if it is on the model' do - expect(droplet_model.lifecycle_data).to eq(lifecycle_data) - end + it 'uses explicit lifecycle_type if set' do + app = AppModel.make + droplet = DropletModel.create(app: app, state: DropletModel::STAGING_STATE, lifecycle_type: 'docker') + expect(droplet[:lifecycle_type]).to eq('docker') + end + end - it 'is a persistable hash' do - expect(droplet_model.reload.lifecycle_data.buildpacks).to eq(lifecycle_data.buildpacks) - expect(droplet_model.reload.lifecycle_data.stack).to eq(lifecycle_data.stack) - end + describe 'validations' do + it 'validates lifecycle_type is one of buildpack, cnb, or docker' do + expect do + DropletModel.make(lifecycle_type: 'invalid') + end.to raise_error(Sequel::ValidationFailed, /lifecycle_type/) + end - it 'deletes the dependent buildpack_lifecycle_data_models when a droplet is deleted' do + it 'accepts valid lifecycle_type values' do + %w[buildpack cnb docker].each do |type| expect do - droplet_model.destroy - end.to change(BuildpackLifecycleDataModel, :count).by(-1). - and change(BuildpackLifecycleBuildpackModel, :count).by(-2) + DropletModel.make(lifecycle_type: type) + end.not_to raise_error end end + end - context 'when there is cnb_lifecycle_data associated to the droplet' do - let(:droplet_model) { DropletModel.make(:cnb, app: nil) } - let!(:lifecycle_data) do - CNBLifecycleDataModel.make( - droplet: droplet_model, - buildpacks: ['http://some-buildpack.com', 'http://another-buildpack.net'] - ) - end + describe '#lifecycle_data' do + context 'buildpack_lifecycle_data' do + let!(:droplet_model) { DropletModel.make(app: nil) } - before do - droplet_model.cnb_lifecycle_data = lifecycle_data - droplet_model.save + it 'returns a buildpack lifecycle data model' do + expect(droplet_model.lifecycle_data).to be_a(BuildpackLifecycleDataModel) + expect(droplet_model.lifecycle_data).to eq(droplet_model.buildpack_lifecycle_data) + expect(droplet_model.cnb_lifecycle_data).to be_nil end - it 'returns cnb_lifecycle_data if it is on the model' do - expect(droplet_model.lifecycle_data).to eq(lifecycle_data) + it 'deletes the dependent buildpack_lifecycle_data model when a droplet is deleted' do + expect do + droplet_model.destroy + end.to change(BuildpackLifecycleDataModel, :count).by(-1) end + end - it 'is a persistable hash' do - expect(droplet_model.reload.cnb_lifecycle_data.buildpacks).to eq(lifecycle_data.buildpacks) - expect(droplet_model.reload.cnb_lifecycle_data.stack).to eq(lifecycle_data.stack) + context 'cnb_lifecycle_data' do + let!(:droplet_model) { DropletModel.make(:cnb, app: nil) } + + it 'returns a cnb lifecycle data model' do + expect(droplet_model.lifecycle_data).to be_a(CNBLifecycleDataModel) + expect(droplet_model.lifecycle_data).to eq(droplet_model.cnb_lifecycle_data) + expect(droplet_model.buildpack_lifecycle_data).to be_nil end - it 'deletes the dependent cnb_lifecycle_data_models when a droplet is deleted' do + it 'deletes the dependent cnb_lifecycle_data when a droplet is deleted' do expect do droplet_model.destroy - end.to change(CNBLifecycleDataModel, :count).by(-1). - and change(BuildpackLifecycleBuildpackModel, :count).by(-2) + end.to change(CNBLifecycleDataModel, :count).by(-1) end end - context 'when there is no lifecycle data associated to the droplet' do + context 'neither buildpack_lifecycle_data, nor cnb_lifecycle_data' do let(:droplet_model) { DropletModel.make(:docker, app: nil) } - before do - droplet_model.buildpack_lifecycle_data = nil - droplet_model.save - end - - it 'returns a docker lifecycle model' do + it 'returns a docker lifecycle data model' do expect(droplet_model.lifecycle_data).to be_a(DockerLifecycleDataModel) + expect(droplet_model.buildpack_lifecycle_data).to be_nil + expect(droplet_model.cnb_lifecycle_data).to be_nil end end end diff --git a/spec/unit/models/runtime/task_model_spec.rb b/spec/unit/models/runtime/task_model_spec.rb index 993f3619bb5..fe1e5790c45 100644 --- a/spec/unit/models/runtime/task_model_spec.rb +++ b/spec/unit/models/runtime/task_model_spec.rb @@ -297,7 +297,7 @@ module VCAP::CloudController context 'when there is a droplet and it does not have the docker lifecycle' do let(:parent_app) { AppModel.make(:docker) } let(:task) { TaskModel.make(app: parent_app, droplet: droplet, state: TaskModel::SUCCEEDED_STATE) } - let(:droplet) { DropletModel.make(:buildpack, app: parent_app) } + let(:droplet) { DropletModel.make(app: parent_app) } it 'returns false' do expect(task.docker?).to be(false) @@ -334,7 +334,7 @@ module VCAP::CloudController context 'when there is a droplet and it does not have the cnb lifecycle' do let(:parent_app) { AppModel.make(:cnb) } let(:task) { TaskModel.make(app: parent_app, droplet: droplet, state: TaskModel::SUCCEEDED_STATE) } - let(:droplet) { DropletModel.make(:buildpack, app: parent_app) } + let(:droplet) { DropletModel.make(app: parent_app) } it 'returns false' do expect(task.cnb?).to be(false) diff --git a/spec/unit/presenters/v3/build_presenter_spec.rb b/spec/unit/presenters/v3/build_presenter_spec.rb index 5d0bb07a6bb..2a75502559a 100644 --- a/spec/unit/presenters/v3/build_presenter_spec.rb +++ b/spec/unit/presenters/v3/build_presenter_spec.rb @@ -21,14 +21,15 @@ module VCAP::CloudController::Presenters::V3 created_by_user_email: 'this user emailed in' ) end - let!(:lifecycle_data) do - VCAP::CloudController::BuildpackLifecycleDataModel.make(buildpacks:, stack:, build:) - end describe '#to_hash' do let(:result) { BuildPresenter.new(build).to_hash } context 'buildpack lifecycle' do + before do + build.buildpack_lifecycle_data.update(buildpacks:, stack:) + end + it 'presents the build as a hash' do links = { self: { href: "#{link_prefix}/v3/builds/#{build.guid}" }, @@ -80,9 +81,9 @@ module VCAP::CloudController::Presenters::V3 end context 'docker lifecycle' do - before do - build.buildpack_lifecycle_data = nil - end + let(:app) { VCAP::CloudController::AppModel.make(:docker) } + let(:package) { VCAP::CloudController::PackageModel.make(:docker, app:) } + let(:build) { VCAP::CloudController::BuildModel.make(:docker, app:, package:) } it 'presents the build as a hash' do links = { @@ -110,7 +111,6 @@ module VCAP::CloudController::Presenters::V3 context 'when the droplet has finished staging' do let(:droplet) do VCAP::CloudController::DropletModel.make( - :buildpack, state: VCAP::CloudController::DropletModel::STAGED_STATE, package_guid: package.guid, app: app, diff --git a/spec/unit/presenters/v3/droplet_presenter_spec.rb b/spec/unit/presenters/v3/droplet_presenter_spec.rb index a649e7416da..fb22a505fae 100644 --- a/spec/unit/presenters/v3/droplet_presenter_spec.rb +++ b/spec/unit/presenters/v3/droplet_presenter_spec.rb @@ -5,7 +5,6 @@ module VCAP::CloudController::Presenters::V3 RSpec.describe DropletPresenter do let(:droplet) do VCAP::CloudController::DropletModel.make( - :buildpack, state: VCAP::CloudController::DropletModel::STAGED_STATE, error_id: 'FAILED', error_description: 'things went all sorts of bad', @@ -115,7 +114,7 @@ module VCAP::CloudController::Presenters::V3 describe 'links' do context 'when there is no package guid' do - let(:droplet) { VCAP::CloudController::DropletModel.make(:buildpack, package_guid: nil) } + let(:droplet) { VCAP::CloudController::DropletModel.make(package_guid: nil) } it 'links to nil' do expect(result[:links][:package]).to be_nil diff --git a/spec/unit/repositories/app_event_repository_spec.rb b/spec/unit/repositories/app_event_repository_spec.rb index f1d12ea10fc..32b40b7177d 100644 --- a/spec/unit/repositories/app_event_repository_spec.rb +++ b/spec/unit/repositories/app_event_repository_spec.rb @@ -532,7 +532,7 @@ module Repositories context 'with a v3 app' do describe '#record_app_create' do - let(:app) { AppModel.make(:buildpack) } + let(:app) { AppModel.make } let(:request_attrs) do { 'name' => 'new', diff --git a/spec/unit/repositories/app_usage_event_repository_spec.rb b/spec/unit/repositories/app_usage_event_repository_spec.rb index 02a3307a626..2d05ef77b0f 100644 --- a/spec/unit/repositories/app_usage_event_repository_spec.rb +++ b/spec/unit/repositories/app_usage_event_repository_spec.rb @@ -395,7 +395,6 @@ module Repositories context 'when the build has BOTH an associated droplet and lifecycle data' do let!(:build) do BuildModel.make( - :buildpack, guid: 'build-1', package_guid: package.guid, app_guid: app_model.guid @@ -403,7 +402,6 @@ module Repositories end let!(:droplet) do DropletModel.make( - :buildpack, buildpack_receipt_buildpack: 'a-buildpack', buildpack_receipt_buildpack_guid: 'a-buildpack-guid', build: build