ensure the default lifecycle is set for buildpack model

Co-authored-by: Johannes Dillmann <[email protected]>

Author: 2 people

app/actions/buildpack_update.rb

@@ -27,7 +27,10 @@ def validation_error!(error, message)
       error!(%(Stack '#{message.stack}' does not exist)) if error.errors.on(:stack)&.include?(:buildpack_stack_does_not_exist)
       error!(%(Buildpack stack cannot be changed)) if error.errors.on(:stack)&.include?(:buildpack_cant_change_stacks)
       error!(%(Buildpack with name '#{error.model.name}' and an unassigned stack already exists)) if error.errors.on(:stack)&.include?(:unique)
-      error!(%(Buildpack with name '#{error.model.name}' and stack '#{error.model.stack}' already exists)) if error.errors.on(%i[name stack])&.include?(:unique)
+
+      if error.errors.on(%i[name stack lifecycle])&.include?(:unique)
+        error!(%(Buildpack with name '#{error.model.name}', stack '#{error.model.stack}' and lifecycle '#{error.model.lifecycle}' already exists))
+      end
 
       error!(error.message)
     end

app/jobs/runtime/buildpack_installer_factory.rb

@@ -37,7 +37,7 @@ def plan(buildpack_name, manifest_fields)
 
           planned_jobs = []
 
-          found_buildpacks = Buildpack.where(name: buildpack_name, lifecycle: manifest_fields[0][:lifecycle]).all
+          found_buildpacks = Buildpack.where(name: buildpack_name, lifecycle: manifest_fields[0][:options][:lifecycle]).all
           ensure_no_attempt_to_upgrade_a_stackless_locked_buildpack(buildpack_name, found_buildpacks, manifest_fields)
 
           manifest_fields.each do |buildpack_fields|

app/models/runtime/buildpack.rb

@@ -3,6 +3,7 @@
 module VCAP::CloudController
   class Buildpack < Sequel::Model
     plugin :list, scope: :lifecycle
+    plugin :after_initialize
 
     export_attributes :name, :stack, :position, :enabled, :locked, :filename, :lifecycle
     import_attributes :name, :stack, :position, :enabled, :locked, :filename, :lifecycle, :key
@@ -12,6 +13,10 @@ class Buildpack < Sequel::Model
       READY_STATE = 'READY'.freeze
     ].map(&:freeze).freeze
 
+    def after_initialize
+      self.lifecycle ||= Lifecycles::BUILDPACK
+    end
+
     one_to_many :labels, class: 'VCAP::CloudController::BuildpackLabelModel', key: :resource_guid, primary_key: :guid
     one_to_many :annotations, class: 'VCAP::CloudController::BuildpackAnnotationModel', key: :resource_guid, primary_key: :guid
 
@@ -39,12 +44,13 @@ def self.at_last_position
     end
 
     def validate
-      validates_unique %i[name stack]
+      validates_unique %i[name stack lifecycle]
       validates_format(/\A(\w|-)+\z/, :name, message: 'can only contain alphanumeric characters')
 
       validate_stack_existence
       validate_stack_change
       validate_multiple_nil_stacks
+      validate_lifecycle_change
     end
 
     def locked?
@@ -87,6 +93,12 @@ def validate_stack_change
       errors.add(:stack, :buildpack_cant_change_stacks) if column_changes.key?(:stack)
     end
 
+    def validate_lifecycle_change
+      return if initial_value(:lifecycle).nil?
+
+      errors.add(:lifecycle, :buildpack_cant_change_lifecycle) if column_changes.key?(:lifecycle)
+    end
+
     def validate_stack_existence
       return unless stack
 

spec/unit/actions/app_apply_manifest_spec.rb

@@ -119,6 +119,7 @@ module VCAP::CloudController
             end
 
             describe 'using cnb type' do
+              let(:message) { AppManifestMessage.create_from_yml({ name: 'blah', buildpack: buildpack.name, lifecycle: 'cnb' }) }
               let(:app) { AppModel.make(:cnb) }
 
               it 'calls AppUpdate with the correct arguments' do

spec/unit/actions/buildpack_update_spec.rb

@@ -120,13 +120,13 @@ module VCAP::CloudController
           end
         end
 
-        it 'raises a human-friendly error when name and stack conflict' do
+        it 'raises a human-friendly error when name, stack and lifecycle conflict' do
           expect(buildpack1.stack).to eq buildpack2.stack
           message = BuildpackUpdateMessage.new(name: buildpack1.name)
 
           expect do
             BuildpackUpdate.new.update(buildpack2, message)
-          end.to raise_error(BuildpackUpdate::Error, "Buildpack with name '#{buildpack1.name}' and stack '#{buildpack1.stack}' already exists")
+          end.to raise_error(BuildpackUpdate::Error, "Buildpack with name '#{buildpack1.name}', stack '#{buildpack1.stack}' and lifecycle '#{buildpack1.lifecycle}' already exists")
         end
 
         it 're-raises when there is an unknown error' do

spec/unit/jobs/runtime/buildpack_installer_factory_spec.rb

@@ -6,7 +6,7 @@ module Jobs::Runtime
       describe '#plan' do
         let(:name) { 'the-buildpack' }
         let(:file) { 'the-file' }
-        let(:opts) { { enabled: true, locked: false, position: 1 } }
+        let(:opts) { { enabled: true, locked: false, position: 1, lifecycle: 'buildpack' } }
         let(:factory) { BuildpackInstallerFactory.new }
         let(:jobs) { factory.plan(name, buildpack_fields) }
 

spec/unit/lib/cloud_controller/diego/lifecycles/buildpack_lifecycle_spec.rb

@@ -169,7 +169,7 @@ module VCAP::CloudController
       it 'returns the expected value' do
         expect(buildpack_lifecycle.buildpack_infos).to eq(stubbed_data[:buildpack_infos])
 
-        expect(BuildpackLifecycleFetcher).to have_received(:fetch).with(%w[cool-buildpack rad-buildpack], Stack.default.name)
+        expect(BuildpackLifecycleFetcher).to have_received(:fetch).with(%w[cool-buildpack rad-buildpack], Stack.default.name, 'buildpack')
       end
     end
 

spec/unit/lib/cloud_controller/diego/lifecycles/cnb_lifecycle_spec.rb

@@ -171,15 +171,18 @@ module VCAP::CloudController
       let(:stubbed_data) { { stack: app.lifecycle_data.stack, buildpack_infos: [instance_double(BuildpackInfo)] } }
       let(:request_data) do
         {
-          buildpacks: %w[docker://cool-buildpack docker://rad-buildpack]
+          buildpacks: %w[cool-buildpack docker://rad-buildpack]
         }
       end
 
+      before do
+        allow(BuildpackLifecycleFetcher).to receive(:fetch).and_return(stubbed_data)
+      end
+
       it 'returns the expected value' do
-        expect(cnb_lifecycle.buildpack_infos).to have(2).items
+        expect(cnb_lifecycle.buildpack_infos).to eq(stubbed_data[:buildpack_infos])
 
-        expect(cnb_lifecycle.buildpack_infos[0].buildpack_url).to eq('docker://cool-buildpack')
-        expect(cnb_lifecycle.buildpack_infos[1].buildpack_url).to eq('docker://rad-buildpack')
+        expect(BuildpackLifecycleFetcher).to have_received(:fetch).with(%w[cool-buildpack docker://rad-buildpack], app.lifecycle_data.stack, 'cnb')
       end
     end
   end

spec/unit/models/runtime/buildpack_spec.rb

@@ -9,7 +9,7 @@ def ordered_buildpacks
     it { is_expected.to have_timestamp_columns }
 
     describe 'Validations' do
-      it { is_expected.to validate_uniqueness %i[name stack] }
+      it { is_expected.to validate_uniqueness %i[name stack lifecycle] }
 
       describe 'stack' do
         let(:stack) {  Stack.make(name: 'happy') }
@@ -74,11 +74,21 @@ def ordered_buildpacks
           end
         end
       end
+
+      describe 'lifecycle' do
+        it 'cannot be changed once it is set' do
+          buildpack = Buildpack.create(name: 'test', lifecycle: 'cnb')
+          buildpack.lifecycle = 'buildpack'
+
+          expect(buildpack).not_to be_valid
+          expect(buildpack.errors.on(:lifecycle)).to include(:buildpack_cant_change_lifecycle)
+        end
+      end
     end
 
     describe 'Serialization' do
-      it { is_expected.to export_attributes :name, :stack, :position, :enabled, :locked, :filename }
-      it { is_expected.to import_attributes :name, :stack, :position, :enabled, :locked, :filename, :key }
+      it { is_expected.to export_attributes :name, :stack, :position, :enabled, :locked, :filename, :lifecycle }
+      it { is_expected.to import_attributes :name, :stack, :position, :enabled, :locked, :filename, :lifecycle, :key }
 
       it 'does not string mung(e)?' do
         expect(Buildpack.new(name: "my_custom_buildpack\r\n").to_json).to eq '"my_custom_buildpack\r\n"'