Return frozen empty stand-in from #lifecycle_data on missing row (#5263)

The buildpack_lifecycle_data / cnb_lifecycle_data row can be destroyed
independently of the owning app, build, or droplet. When that happens,
calling #lifecycle_data returns nil and callers not guarding further
method invocations hit NoMethodError (e.g. build_presenter.rb on
lifecycle_data.to_hash), resulting in an HTTP 500 response.

Fall back to a frozen empty BuildpackLifecycleDataModel /
CNBLifecycleDataModel instance so callers see a well-shaped object
whose to_hash returns '{ buildpacks: [], stack: nil }'. The freeze
prevents any accidental attempt to persist the stand-in.

Also freeze the docker-branch stand-in for the same reason and
consistency.

Document the possibility of an empty data payload in the buildpack
and cnb sections of the v3 lifecycle doc.

Author: philippthun

app/models/runtime/app_model.rb

@@ -109,10 +109,13 @@ def lifecycle_type
     end
 
     def lifecycle_data
-      return buildpack_lifecycle_data if lifecycle_type == BuildpackLifecycleDataModel::LIFECYCLE_TYPE
-      return cnb_lifecycle_data if lifecycle_type == CNBLifecycleDataModel::LIFECYCLE_TYPE
+      # The lifecycle_data row can be destroyed independently of this
+      # app; fall back to a frozen empty instance so callers never see
+      # nil (and can't accidentally persist the stand-in).
+      return buildpack_lifecycle_data || BuildpackLifecycleDataModel.new.freeze if lifecycle_type == BuildpackLifecycleDataModel::LIFECYCLE_TYPE
+      return cnb_lifecycle_data || CNBLifecycleDataModel.new.freeze if lifecycle_type == CNBLifecycleDataModel::LIFECYCLE_TYPE
 
-      DockerLifecycleDataModel.new
+      DockerLifecycleDataModel.new.freeze
     end
 
     def current_package

app/models/runtime/build_model.rb

@@ -82,10 +82,13 @@ def cnb_lifecycle?
     end
 
     def lifecycle_data
-      return buildpack_lifecycle_data if lifecycle_type == BuildpackLifecycleDataModel::LIFECYCLE_TYPE
-      return cnb_lifecycle_data if lifecycle_type == CNBLifecycleDataModel::LIFECYCLE_TYPE
+      # The lifecycle_data row can be destroyed independently of this
+      # build; fall back to a frozen empty instance so callers never
+      # see nil (and can't accidentally persist the stand-in).
+      return buildpack_lifecycle_data || BuildpackLifecycleDataModel.new.freeze if lifecycle_type == BuildpackLifecycleDataModel::LIFECYCLE_TYPE
+      return cnb_lifecycle_data || CNBLifecycleDataModel.new.freeze if lifecycle_type == CNBLifecycleDataModel::LIFECYCLE_TYPE
 
-      DockerLifecycleDataModel.new
+      DockerLifecycleDataModel.new.freeze
     end
 
     def staged?

app/models/runtime/droplet_model.rb

@@ -187,10 +187,13 @@ def lifecycle_type
     end
 
     def lifecycle_data
-      return buildpack_lifecycle_data if lifecycle_type == BuildpackLifecycleDataModel::LIFECYCLE_TYPE
-      return cnb_lifecycle_data if lifecycle_type == CNBLifecycleDataModel::LIFECYCLE_TYPE
+      # The lifecycle_data row can be destroyed independently of this
+      # droplet; fall back to a frozen empty instance so callers never
+      # see nil (and can't accidentally persist the stand-in).
+      return buildpack_lifecycle_data || BuildpackLifecycleDataModel.new.freeze if lifecycle_type == BuildpackLifecycleDataModel::LIFECYCLE_TYPE
+      return cnb_lifecycle_data || CNBLifecycleDataModel.new.freeze if lifecycle_type == CNBLifecycleDataModel::LIFECYCLE_TYPE
 
-      DockerLifecycleDataModel.new
+      DockerLifecycleDataModel.new.freeze
     end
 
     def in_final_state?

docs/v3/source/includes/concepts/_lifecycles.md.erb

@@ -42,6 +42,9 @@ Name | Type | Description
 **data.buildpacks** | _list of strings_ | A list of the names of buildpacks, URLs from which they may be downloaded, or `null` to auto-detect a suitable buildpack during staging
 **data.stack** | _string_ | The root filesystem to use with the buildpack, for example `cflinuxfs4`
 
+For records whose historical lifecycle data is no longer available,
+**data.buildpacks** may be an empty list and **data.stack** may be `null`.
+
 ### Cloud Native Buildpacks Lifecycle *(experimental)*
 
 ```
@@ -83,6 +86,9 @@ Name | Type | Description
 **data.credentials** | _object_ | Credentials used to download the configured buildpacks. This can either contain username/password  or a token.
 **data.stack** | _string_ | The root filesystem to use with the buildpack, for example `cflinuxfs4`
 
+For records whose historical lifecycle data is no longer available,
+**data.buildpacks** may be an empty list and **data.stack** may be `null`.
+
 ### Docker lifecycle
 
 ```

spec/unit/models/runtime/app_model_spec.rb

@@ -362,6 +362,52 @@ module VCAP::CloudController
           expect(app_model.buildpack_lifecycle_data).to be_nil
           expect(app_model.cnb_lifecycle_data).to be_nil
         end
+
+        it 'returns a frozen instance so callers cannot persist the stand-in' do
+          expect(app_model.lifecycle_data).to be_frozen
+        end
+      end
+
+      context 'when lifecycle_type is buildpack but the associated row is missing' do
+        let(:app_model) { create(:app_model) }
+
+        before do
+          app_model.buildpack_lifecycle_data.destroy
+          app_model.reload
+        end
+
+        it 'returns an empty buildpack lifecycle data stand-in' do
+          expect(app_model.lifecycle_data).to be_a(BuildpackLifecycleDataModel)
+        end
+
+        it 'returns a frozen instance so callers cannot persist the stand-in' do
+          expect(app_model.lifecycle_data).to be_frozen
+        end
+
+        it 'produces an empty to_hash' do
+          expect(app_model.lifecycle_data.to_hash).to eq(buildpacks: [], stack: nil)
+        end
+      end
+
+      context 'when lifecycle_type is cnb but the associated row is missing' do
+        let(:app_model) { create(:app_model, :cnb) }
+
+        before do
+          app_model.cnb_lifecycle_data.destroy
+          app_model.reload
+        end
+
+        it 'returns an empty cnb lifecycle data stand-in' do
+          expect(app_model.lifecycle_data).to be_a(CNBLifecycleDataModel)
+        end
+
+        it 'returns a frozen instance so callers cannot persist the stand-in' do
+          expect(app_model.lifecycle_data).to be_frozen
+        end
+
+        it 'produces an empty to_hash' do
+          expect(app_model.lifecycle_data.to_hash).to eq(buildpacks: [], stack: nil)
+        end
       end
     end
 

spec/unit/models/runtime/build_model_spec.rb

@@ -146,6 +146,52 @@ module VCAP::CloudController
           expect(build_model.buildpack_lifecycle_data).to be_nil
           expect(build_model.cnb_lifecycle_data).to be_nil
         end
+
+        it 'returns a frozen instance so callers cannot persist the stand-in' do
+          expect(build_model.lifecycle_data).to be_frozen
+        end
+      end
+
+      context 'when lifecycle_type is buildpack but the associated row is missing' do
+        let(:build_model) { create(:build_model, app: nil) }
+
+        before do
+          build_model.buildpack_lifecycle_data.destroy
+          build_model.reload
+        end
+
+        it 'returns an empty buildpack lifecycle data stand-in' do
+          expect(build_model.lifecycle_data).to be_a(BuildpackLifecycleDataModel)
+        end
+
+        it 'returns a frozen instance so callers cannot persist the stand-in' do
+          expect(build_model.lifecycle_data).to be_frozen
+        end
+
+        it 'produces an empty to_hash' do
+          expect(build_model.lifecycle_data.to_hash).to eq(buildpacks: [], stack: nil)
+        end
+      end
+
+      context 'when lifecycle_type is cnb but the associated row is missing' do
+        let(:build_model) { create(:build_model, :cnb, app: nil) }
+
+        before do
+          build_model.cnb_lifecycle_data.destroy
+          build_model.reload
+        end
+
+        it 'returns an empty cnb lifecycle data stand-in' do
+          expect(build_model.lifecycle_data).to be_a(CNBLifecycleDataModel)
+        end
+
+        it 'returns a frozen instance so callers cannot persist the stand-in' do
+          expect(build_model.lifecycle_data).to be_frozen
+        end
+
+        it 'produces an empty to_hash' do
+          expect(build_model.lifecycle_data.to_hash).to eq(buildpacks: [], stack: nil)
+        end
       end
     end
 

spec/unit/models/runtime/droplet_model_spec.rb

@@ -165,6 +165,52 @@ module VCAP::CloudController
           expect(droplet_model.buildpack_lifecycle_data).to be_nil
           expect(droplet_model.cnb_lifecycle_data).to be_nil
         end
+
+        it 'returns a frozen instance so callers cannot persist the stand-in' do
+          expect(droplet_model.lifecycle_data).to be_frozen
+        end
+      end
+
+      context 'when lifecycle_type is buildpack but the associated row is missing' do
+        let(:droplet_model) { create(:droplet_model, app: nil) }
+
+        before do
+          droplet_model.buildpack_lifecycle_data.destroy
+          droplet_model.reload
+        end
+
+        it 'returns an empty buildpack lifecycle data stand-in' do
+          expect(droplet_model.lifecycle_data).to be_a(BuildpackLifecycleDataModel)
+        end
+
+        it 'returns a frozen instance so callers cannot persist the stand-in' do
+          expect(droplet_model.lifecycle_data).to be_frozen
+        end
+
+        it 'produces an empty to_hash' do
+          expect(droplet_model.lifecycle_data.to_hash).to eq(buildpacks: [], stack: nil)
+        end
+      end
+
+      context 'when lifecycle_type is cnb but the associated row is missing' do
+        let(:droplet_model) { create(:droplet_model, :cnb, app: nil) }
+
+        before do
+          droplet_model.cnb_lifecycle_data.destroy
+          droplet_model.reload
+        end
+
+        it 'returns an empty cnb lifecycle data stand-in' do
+          expect(droplet_model.lifecycle_data).to be_a(CNBLifecycleDataModel)
+        end
+
+        it 'returns a frozen instance so callers cannot persist the stand-in' do
+          expect(droplet_model.lifecycle_data).to be_frozen
+        end
+
+        it 'produces an empty to_hash' do
+          expect(droplet_model.lifecycle_data.to_hash).to eq(buildpacks: [], stack: nil)
+        end
       end
     end