Removes stack_warnings from the models.

rashedkvm · rashedkvm · commit 99e4e940f2a0 · 2026-01-23T17:43:07.000-05:00
Actions own their warnings and Controllers set  warnings → headers

Signed-off-by: Rashed Kamal &lt;rashed.kamal@broadcom.com&gt;
diff --git a/app/actions/app_create.rb b/app/actions/app_create.rb
@@ -9,14 +9,15 @@ class AppCreate
 
     class InvalidApp < StandardError; end
 
+    attr_reader :warnings
+
     def initialize(user_audit_info)
       @user_audit_info = user_audit_info
       @logger = Steno.logger('cc.action.app_create')
     end
 
     def create(message, lifecycle)
       app = nil
-      warnings = []
       AppModel.db.transaction do
         app = AppModel.create(
           name: message.name,
@@ -25,7 +26,7 @@ def create(message, lifecycle)
         )
 
         lifecycle.create_lifecycle_data_model(app)
-        warnings = validate_stack_state(app, lifecycle)
+        @warnings = validate_stack_state(app, lifecycle)
         validate_buildpacks_are_ready(app)
 
         MetadataUpdate.update(app, message)
@@ -45,8 +46,6 @@ def create(message, lifecycle)
         )
       end
 
-      app.instance_variable_set(:@stack_warnings, warnings)
-
       app
     rescue Sequel::ValidationFailed => e
       v3_api_error!(:UniquenessError, e.message) if e.errors.on(%i[space_guid name])
diff --git a/app/actions/app_update.rb b/app/actions/app_update.rb
@@ -7,6 +7,8 @@ class AppUpdate
     class DropletNotFound < StandardError; end
     class InvalidApp < StandardError; end
 
+    attr_reader :warnings
+
     def initialize(user_audit_info, manifest_triggered: false, runners: nil)
       @user_audit_info = user_audit_info
       @logger = Steno.logger('cc.action.app_update')
@@ -53,8 +55,7 @@ def update(app, message, lifecycle)
         end
       end
 
-      warnings = validate_stack_state(app, message, lifecycle)
-      app.instance_variable_set(:@stack_warnings, warnings)
+      @warnings = validate_stack_state(app, message, lifecycle)
 
       app
     rescue Sequel::ValidationFailed,
diff --git a/app/actions/build_create.rb b/app/actions/build_create.rb
@@ -25,7 +25,7 @@ class LogRateLimitOrgQuotaExceeded < BuildError
     class StagingInProgress < BuildError
     end
 
-    attr_reader :staging_response
+    attr_reader :staging_response, :warnings
 
     def initialize(user_audit_info: UserAuditInfo.from_context(SecurityContext),
                    memory_limit_calculator: QuotaValidatingStagingMemoryCalculator.new,
@@ -41,7 +41,7 @@ def initialize(user_audit_info: UserAuditInfo.from_context(SecurityContext),
 
     def create_and_stage(package:, lifecycle:, metadata: nil, start_after_staging: false)
       logger.info("creating build for package #{package.guid}")
-      warnings = validate_stack_state!(lifecycle, package.app)
+      @warnings = validate_stack_state!(lifecycle, package.app)
       staging_in_progress! if package.app.staging_in_progress?
       raise InvalidPackage.new('Cannot stage package whose state is not ready.') if package.state != PackageModel::READY_STATE
 
@@ -61,7 +61,6 @@ def create_and_stage(package:, lifecycle:, metadata: nil, start_after_staging: f
         created_by_user_name: @user_audit_info.user_name,
         created_by_user_email: @user_audit_info.user_email
       )
-      build.instance_variable_set(:@stack_warnings, warnings)
 
       BuildModel.db.transaction do
         build.save
diff --git a/app/actions/v2/app_create.rb b/app/actions/v2/app_create.rb
@@ -1,6 +1,8 @@
 module VCAP::CloudController
   module V2
     class AppCreate
+      attr_reader :warnings
+
       def initialize(access_validator:)
         @access_validator = access_validator
       end
@@ -46,8 +48,7 @@ def create(request_attrs)
           @access_validator.validate_access(:create, process, request_attrs)
         end
 
-        warnings = validate_stack_state(request_attrs)
-        process.instance_variable_set(:@stack_warnings, warnings)
+        @warnings = validate_stack_state(request_attrs)
 
         process
       end
diff --git a/app/actions/v2/app_stage.rb b/app/actions/v2/app_stage.rb
@@ -29,7 +29,7 @@ def stage(process)
           start_after_staging: true
         )
 
-        @warnings = build.instance_variable_get(:@stack_warnings) || []
+        @warnings = build_creator.warnings || []
 
         TelemetryLogger.v2_emit(
           'create-build',
diff --git a/app/controllers/runtime/apps_controller.rb b/app/controllers/runtime/apps_controller.rb
@@ -358,7 +358,7 @@ def create
       creator = V2::AppCreate.new(access_validator: self)
       process = creator.create(request_attrs)
 
-      process.stack_warnings&.each { |warning| add_warning(warning) }
+      creator.warnings&.each { |warning| add_warning(warning) }
 
       @app_event_repository.record_app_create(
         process,
diff --git a/app/controllers/v3/apps_controller.rb b/app/controllers/v3/apps_controller.rb
@@ -98,7 +98,8 @@ def create
     FeatureFlag.raise_unless_enabled!(:diego_cnb) if lifecycle.type == VCAP::CloudController::Lifecycles::CNB
     unprocessable!(lifecycle.errors.full_messages) unless lifecycle.valid?
 
-    app = AppCreate.new(user_audit_info).create(message, lifecycle)
+    app_creator = AppCreate.new(user_audit_info)
+    app = app_creator.create(message, lifecycle)
     TelemetryLogger.v3_emit(
       'create-app',
       {
@@ -107,7 +108,7 @@ def create
       }
     )
 
-    add_warning_headers(app.stack_warnings) if app.stack_warnings&.any?
+    add_warning_headers(app_creator.warnings) if app_creator.warnings&.any?
 
     render status: :created, json: Presenters::V3::AppPresenter.new(app)
   rescue AppCreate::InvalidApp => e
@@ -127,7 +128,8 @@ def update
     lifecycle = AppLifecycleProvider.provide_for_update(message, app)
     unprocessable!(lifecycle.errors.full_messages) unless lifecycle.valid?
 
-    app = AppUpdate.new(user_audit_info).update(app, message, lifecycle)
+    app_updater = AppUpdate.new(user_audit_info)
+    app = app_updater.update(app, message, lifecycle)
     TelemetryLogger.v3_emit(
       'update-app',
       {
@@ -136,7 +138,7 @@ def update
       }
     )
 
-    add_warning_headers(app.stack_warnings) if app.stack_warnings&.any?
+    add_warning_headers(app_updater.warnings) if app_updater.warnings&.any?
 
     render status: :ok, json: Presenters::V3::AppPresenter.new(app)
   rescue AppUpdate::DropletNotFound
diff --git a/app/controllers/v3/builds_controller.rb b/app/controllers/v3/builds_controller.rb
@@ -49,7 +49,8 @@ def create
     FeatureFlag.raise_unless_enabled!(:diego_cnb) if lifecycle.type == VCAP::CloudController::Lifecycles::CNB
     unprocessable!(lifecycle.errors.full_messages) unless lifecycle.valid?
 
-    build = BuildCreate.new.create_and_stage(package: package, lifecycle: lifecycle, metadata: message.metadata)
+    build_creator = BuildCreate.new
+    build = build_creator.create_and_stage(package: package, lifecycle: lifecycle, metadata: message.metadata)
 
     TelemetryLogger.v3_emit(
       'create-build',
@@ -65,7 +66,7 @@ def create
       }
     )
 
-    add_warning_headers(build.stack_warnings) if build.stack_warnings&.any?
+    add_warning_headers(build_creator.warnings) if build_creator.warnings&.any?
 
     render status: :created, json: Presenters::V3::BuildPresenter.new(build)
   rescue BuildCreate::InvalidPackage => e
diff --git a/app/models/runtime/app_model.rb b/app/models/runtime/app_model.rb
@@ -11,8 +11,6 @@ class AppModel < Sequel::Model(:apps)
     DEFAULT_CONTAINER_USER = 'vcap'.freeze
     DEFAULT_DOCKER_CONTAINER_USER = 'root'.freeze
 
-    attr_reader :stack_warnings
-
     many_to_many :routes, join_table: :route_mappings, left_key: :app_guid, left_primary_key: :guid, right_primary_key: :guid, right_key: :route_guid
     one_to_many :route_mappings, class: 'VCAP::CloudController::RouteMappingModel', key: :app_guid, primary_key: :guid
     one_to_many :service_bindings, key: :app_guid, primary_key: :guid
diff --git a/app/models/runtime/build_model.rb b/app/models/runtime/build_model.rb
@@ -17,8 +17,6 @@ class BuildModel < Sequel::Model(:builds)
                                 CNBGenericBuildFailed CNBDownloadBuildpackFailed CNBDetectFailed
                                 CNBBuildFailed CNBExportFailed CNBLaunchFailed CNBRestoreFailed].map(&:freeze).freeze
 
-    attr_reader :stack_warnings
-
     many_to_one :app,
                 class: 'VCAP::CloudController::AppModel',
                 key: :app_guid,
diff --git a/app/models/runtime/process_model.rb b/app/models/runtime/process_model.rb
@@ -22,8 +22,6 @@ class ProcessModel < Sequel::Model(:processes) # rubocop:disable Metrics/ClassLe
 
     extend IntegerArraySerializer
 
-    attr_reader :stack_warnings
-
     def after_initialize
       self.instances        ||= db_schema[:instances][:default].to_i
       self.memory           ||= Config.config.get(:default_app_memory)
diff --git a/app/presenters/v3/app_presenter.rb b/app/presenters/v3/app_presenter.rb
@@ -45,8 +45,6 @@ def to_hash
             links: build_links
           }
 
-          hash[:warnings] = app_warnings if app_warnings
-
           @decorators.reduce(hash) { |memo, d| d.decorate(memo, [app]) }
         end
 
@@ -76,12 +74,6 @@ def build_links
 
           links.delete_if { |_, v| v.nil? }
         end
-
-        def app_warnings
-          return nil unless app.stack_warnings&.any?
-
-          app.stack_warnings.map { |warning| { detail: warning } }
-        end
       end
     end
   end
diff --git a/app/presenters/v3/build_presenter.rb b/app/presenters/v3/build_presenter.rb
@@ -15,7 +15,7 @@ def associated_resources
         end
 
         def to_hash
-          hash = {
+          {
             guid: build.guid,
             created_at: build.created_at,
             updated_at: build.updated_at,
@@ -42,10 +42,6 @@ def to_hash
             },
             links: build_links
           }
-
-          hash[:warnings] = build_warnings if build_warnings
-
-          hash
         end
 
         private
@@ -65,12 +61,6 @@ def error
           e.presence
         end
 
-        def build_warnings
-          return nil unless build.stack_warnings&.any?
-
-          build.stack_warnings.map { |warning| { detail: warning } }
-        end
-
         def build_links
           {
             self: { href: url_builder.build_url(path: "/v3/builds/#{build.guid}") },
diff --git a/spec/request/apps_spec.rb b/spec/request/apps_spec.rb
@@ -415,12 +415,11 @@
           }
         end
 
-        it 'creates the app with warnings in response body' do
+        it 'creates the app without warnings in response body' do
           post '/v3/apps', create_request.to_json, user_header
 
           expect(last_response.status).to eq(201)
-          expect(parsed_response['warnings']).to be_present
-          expect(parsed_response['warnings'].first['detail']).to include('DEPRECATED')
+          expect(parsed_response).not_to have_key('warnings')
         end
 
         it 'includes warnings in X-Cf-Warnings header' do
diff --git a/spec/request/builds_spec.rb b/spec/request/builds_spec.rb
@@ -299,10 +299,7 @@
 
           expect(last_response.status).to eq(201)
           expect(parsed_response['state']).to eq('STAGING')
-          expect(parsed_response['warnings']).to be_present
-          expect(parsed_response['warnings'][0]['detail']).to include('DEPRECATED')
-          expect(parsed_response['warnings'][0]['detail']).to include('cflinuxfs3')
-          expect(parsed_response['warnings'][0]['detail']).to include('WARNING')
+          expect(parsed_response).not_to have_key('warnings')
         end
 
         it 'includes warning in response headers' do
@@ -328,9 +325,7 @@
 
           expect(last_response.status).to eq(201)
           expect(parsed_response['state']).to eq('STAGING')
-          expect(parsed_response['warnings']).to be_present
-          expect(parsed_response['warnings'][0]['detail']).to include('DEPRECATED')
-          expect(parsed_response['warnings'][0]['detail']).to include('cflinuxfs3')
+          expect(parsed_response).not_to have_key('warnings')
           expect(app_model.builds_dataset.count).to eq(2)
         end
 
diff --git a/spec/unit/actions/app_create_spec.rb b/spec/unit/actions/app_create_spec.rb
@@ -255,8 +255,8 @@ module VCAP::CloudController
           it 'creates the app with warnings' do
             app = app_create.create(message, lifecycle)
             expect(app.id).not_to be_nil
-            expect(app.stack_warnings).to be_present
-            expect(app.stack_warnings.first).to include('DEPRECATED')
+            expect(app_create.warnings).to be_present
+            expect(app_create.warnings.first).to include('DEPRECATED')
           end
         end
 
@@ -266,7 +266,7 @@ module VCAP::CloudController
           it 'creates the app without warnings' do
             app = app_create.create(message, lifecycle)
             expect(app.id).not_to be_nil
-            expect(app.stack_warnings).to be_empty
+            expect(app_create.warnings).to be_empty
           end
         end
 
@@ -277,7 +277,7 @@ module VCAP::CloudController
           it 'skips stack validation' do
             app = app_create.create(message, lifecycle)
             expect(app.id).not_to be_nil
-            expect(app.stack_warnings).to be_empty
+            expect(app_create.warnings).to be_empty
           end
         end
       end
diff --git a/spec/unit/actions/app_update_spec.rb b/spec/unit/actions/app_update_spec.rb
@@ -407,27 +407,27 @@ module VCAP::CloudController
           before { new_stack.update(state: StackStates::STACK_DEPRECATED) }
 
           it 'updates the app with warnings' do
-            app = app_update.update(app_model, message, lifecycle)
-            expect(app.stack_warnings).to be_present
-            expect(app.stack_warnings.first).to include('DEPRECATED')
+            app_update.update(app_model, message, lifecycle)
+            expect(app_update.warnings).to be_present
+            expect(app_update.warnings.first).to include('DEPRECATED')
           end
         end
 
         context 'when stack is ACTIVE' do
           before { new_stack.update(state: StackStates::STACK_ACTIVE) }
 
           it 'updates the app without warnings' do
-            app = app_update.update(app_model, message, lifecycle)
-            expect(app.stack_warnings).to be_empty
+            app_update.update(app_model, message, lifecycle)
+            expect(app_update.warnings).to be_empty
           end
         end
 
         context 'when stack is not being changed' do
           let(:message) { AppUpdateMessage.new({ name: 'new-name' }) }
 
           it 'does not validate stack state' do
-            app = app_update.update(app_model, message, lifecycle)
-            expect(app.stack_warnings).to be_empty
+            app_update.update(app_model, message, lifecycle)
+            expect(app_update.warnings).to be_empty
           end
         end
       end
diff --git a/spec/unit/actions/v2/app_create_spec.rb b/spec/unit/actions/v2/app_create_spec.rb
@@ -278,8 +278,8 @@ module VCAP::CloudController
           it 'creates the app with warnings' do
             process = app_create.create(request_attrs)
             expect(process.id).not_to be_nil
-            expect(process.stack_warnings).to be_present
-            expect(process.stack_warnings.first).to include('DEPRECATED')
+            expect(app_create.warnings).to be_present
+            expect(app_create.warnings.first).to include('DEPRECATED')
           end
         end
 
@@ -298,7 +298,7 @@ module VCAP::CloudController
           it 'creates the app without warnings' do
             process = app_create.create(request_attrs)
             expect(process.id).not_to be_nil
-            expect(process.stack_warnings).to be_empty
+            expect(app_create.warnings).to be_empty
           end
         end
 
@@ -316,7 +316,7 @@ module VCAP::CloudController
           it 'skips stack validation' do
             process = app_create.create(request_attrs)
             expect(process.id).not_to be_nil
-            expect(process.stack_warnings).to be_empty
+            expect(app_create.warnings).to be_empty
           end
         end
       end
diff --git a/spec/unit/actions/v2/app_stage_spec.rb b/spec/unit/actions/v2/app_stage_spec.rb
@@ -16,7 +16,7 @@ module V2
 
         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_create) { instance_double(BuildCreate, create_and_stage_without_event: build, staging_response: 'staging-response') }
+        let(:build_create) { instance_double(BuildCreate, create_and_stage_without_event: build, staging_response: 'staging-response', warnings: []) }
 
         before do
           allow(BuildCreate).to receive(:new).with(memory_limit_calculator: an_instance_of(NonQuotaValidatingStagingMemoryCalculator)).and_return(build_create)
diff --git a/spec/unit/presenters/v3/app_presenter_spec.rb b/spec/unit/presenters/v3/app_presenter_spec.rb
diff --git a/spec/unit/presenters/v3/build_presenter_spec.rb b/spec/unit/presenters/v3/build_presenter_spec.rb

Original file line number	Diff line number	Diff line change
`@@ -29,7 +29,7 @@ def stage(process)`
`29`	`29`	`start_after_staging: true`
`30`	`30`	`)`
`31`	`31`
`32`		`- @warnings = build.instance_variable_get(:@stack_warnings) \|\| []`
	`32`	`+ @warnings = build_creator.warnings \|\| []`
`33`	`33`
`34`	`34`	`TelemetryLogger.v2_emit(`
`35`	`35`	`'create-build',`
Original file line number	Diff line number	Diff line change
`@@ -98,7 +98,8 @@ def create`
`98`	`98`	`FeatureFlag.raise_unless_enabled!(:diego_cnb) if lifecycle.type == VCAP::CloudController::Lifecycles::CNB`
`99`	`99`	`unprocessable!(lifecycle.errors.full_messages) unless lifecycle.valid?`
`100`	`100`
`101`		`- app = AppCreate.new(user_audit_info).create(message, lifecycle)`
	`101`	`+ app_creator = AppCreate.new(user_audit_info)`
	`102`	`+ app = app_creator.create(message, lifecycle)`
`102`	`103`	`TelemetryLogger.v3_emit(`
`103`	`104`	`'create-app',`
`104`	`105`	`{`
`@@ -107,7 +108,7 @@ def create`
`107`	`108`	`}`
`108`	`109`	`)`
`109`	`110`
`110`		`- add_warning_headers(app.stack_warnings) if app.stack_warnings&.any?`
	`111`	`+ add_warning_headers(app_creator.warnings) if app_creator.warnings&.any?`
`111`	`112`
`112`	`113`	`render status: :created, json: Presenters::V3::AppPresenter.new(app)`
`113`	`114`	`rescue AppCreate::InvalidApp => e`
`@@ -127,7 +128,8 @@ def update`
`127`	`128`	`lifecycle = AppLifecycleProvider.provide_for_update(message, app)`
`128`	`129`	`unprocessable!(lifecycle.errors.full_messages) unless lifecycle.valid?`
`129`	`130`
`130`		`- app = AppUpdate.new(user_audit_info).update(app, message, lifecycle)`
	`131`	`+ app_updater = AppUpdate.new(user_audit_info)`
	`132`	`+ app = app_updater.update(app, message, lifecycle)`
`131`	`133`	`TelemetryLogger.v3_emit(`
`132`	`134`	`'update-app',`
`133`	`135`	`{`
`@@ -136,7 +138,7 @@ def update`
`136`	`138`	`}`
`137`	`139`	`)`
`138`	`140`
`139`		`- add_warning_headers(app.stack_warnings) if app.stack_warnings&.any?`
	`141`	`+ add_warning_headers(app_updater.warnings) if app_updater.warnings&.any?`
`140`	`142`
`141`	`143`	`render status: :ok, json: Presenters::V3::AppPresenter.new(app)`
`142`	`144`	`rescue AppUpdate::DropletNotFound`
Original file line number	Diff line number	Diff line change
`@@ -49,7 +49,8 @@ def create`
`49`	`49`	`FeatureFlag.raise_unless_enabled!(:diego_cnb) if lifecycle.type == VCAP::CloudController::Lifecycles::CNB`
`50`	`50`	`unprocessable!(lifecycle.errors.full_messages) unless lifecycle.valid?`
`51`	`51`
`52`		`- build = BuildCreate.new.create_and_stage(package: package, lifecycle: lifecycle, metadata: message.metadata)`
	`52`	`+ build_creator = BuildCreate.new`
	`53`	`+ build = build_creator.create_and_stage(package: package, lifecycle: lifecycle, metadata: message.metadata)`
`53`	`54`
`54`	`55`	`TelemetryLogger.v3_emit(`
`55`	`56`	`'create-build',`
`@@ -65,7 +66,7 @@ def create`
`65`	`66`	`}`
`66`	`67`	`)`
`67`	`68`
`68`		`- add_warning_headers(build.stack_warnings) if build.stack_warnings&.any?`
	`69`	`+ add_warning_headers(build_creator.warnings) if build_creator.warnings&.any?`
`69`	`70`
`70`	`71`	`render status: :created, json: Presenters::V3::BuildPresenter.new(build)`
`71`	`72`	`rescue BuildCreate::InvalidPackage => e`