Validate that app buildpack is ready

The new validation method on AppModel caused a few problems regarding
the buildpack_lifecycle_data relationshiop (and the lifecycle_data)
method, since it loads the relationship at creation/validation time.
Previously the relationship was lazy loaded when it was first
referenced.

Co-authored-by: Matt Royal <mroyal@pivotal.io>
Co-authored-by: Jaskanwal Pawar <jpawar@pivotal.io>
Co-authored-by: Piyali Banerjee <pbanerjee@pivotal.io>

[#173038631]

Author: matt-royal

app/models/runtime/app_model.rb

@@ -72,6 +72,7 @@ def validate
       super
       validates_presence :name
       validates_format APP_NAME_REGEX, :name
+      validate_buildpacks_are_ready
       validate_environment_variables
       validate_droplet_is_staged
 
@@ -160,6 +161,20 @@ def validate_environment_variables
         validate_each(self, :environment_variables, environment_variables)
     end
 
+    def validate_buildpacks_are_ready
+      return unless self.buildpack_lifecycle_data
+
+      self.buildpack_lifecycle_data.buildpack_lifecycle_buildpacks.each do |blb|
+        unless blb.custom?
+          buildpack = Buildpack.find(name: blb.admin_buildpack_name)
+
+          if buildpack && buildpack.state != Buildpack::READY_STATE
+            errors.add(:buildpack, 'must be in ready state')
+          end
+        end
+      end
+    end
+
     def validate_droplet_is_staged
       if droplet && droplet.state != DropletModel::STAGED_STATE
         errors.add(:droplet, 'must be in staged state')

lib/cloud_controller/diego/lifecycles/app_buildpack_lifecycle.rb

@@ -17,7 +17,7 @@ def initialize(message)
     delegate :valid?, :errors, to: :validator
 
     def create_lifecycle_data_model(app)
-      BuildpackLifecycleDataModel.create(
+      app.buildpack_lifecycle_data = BuildpackLifecycleDataModel.create(
         buildpacks: buildpacks,
         stack:     stack,
         app:       app

lib/cloud_controller/diego/lifecycles/app_kpack_lifecycle.rb

@@ -3,7 +3,7 @@ class AppKpackLifecycle
     def initialize(*_message); end
 
     def create_lifecycle_data_model(app)
-      KpackLifecycleDataModel.create(app: app)
+      app.kpack_lifecycle_data = KpackLifecycleDataModel.create(app: app)
     end
 
     def update_lifecycle_data_model(_); end

spec/unit/actions/app_find_or_create_skeleton_spec.rb

@@ -29,7 +29,7 @@ module VCAP::CloudController
 
           expect(app.name).to eq(name)
           expect(app.space).to eq(space)
-          expect(app.lifecycle_type).to eq(Lifecycles::BUILDPACK)
+          expect(app.reload.lifecycle_type).to eq(Lifecycles::BUILDPACK)
         end
       end
 

spec/unit/models/runtime/app_model_spec.rb

@@ -264,13 +264,33 @@ module VCAP::CloudController
           expect(app_model).to be_valid
         end
       end
+
+      describe 'buildpacks' do
+        let(:ready_buildpack) { Buildpack.make(filename: 'some-file') }
+        let(:unready_buildpack) { Buildpack.make(filename: nil) }
+        let!(:buildpack_lifecycle_data) { BuildpackLifecycleDataModel.make(app: app_model, buildpacks: [ready_buildpack.name, unready_buildpack.name]) }
+
+        it 'does not allow buildpacks that are not READY' do
+          expect {
+            app_model.buildpack_lifecycle_data = buildpack_lifecycle_data
+            app_model.save
+          }.to raise_error(Sequel::ValidationFailed, /must be in ready state/)
+        end
+
+        it 'is valid with buildpacks that are READY' do
+          buildpack_lifecycle_data.buildpacks = [ready_buildpack.name]
+          app_model.buildpack_lifecycle_data = buildpack_lifecycle_data
+          expect(app_model).to be_valid
+        end
+      end
     end
 
     describe '#lifecycle_type' do
       context 'the model contains buildpack_lifecycle_data' do
         before { BuildpackLifecycleDataModel.make(app: app_model) }
 
         it 'returns the string "buildpack" if buildpack_lifecycle_data is on the model' do
+          app_model.reload
           expect(app_model.lifecycle_type).to eq('buildpack')
         end
       end
@@ -301,7 +321,7 @@ module VCAP::CloudController
         let!(:buildpack_lifecycle_data) { BuildpackLifecycleDataModel.make(app: app_model) }
 
         it 'returns buildpack_lifecycle_data if it is on the model' do
-          expect(app_model.lifecycle_data).to eq(buildpack_lifecycle_data)
+          expect(app_model.reload.lifecycle_data).to eq(buildpack_lifecycle_data)
         end
 
         it 'is a persistable hash' do