From e7ef131467d54acd4e70fbba625aea7a7beddaac Mon Sep 17 00:00:00 2001 From: Rashed Kamal Date: Wed, 19 Nov 2025 09:08:08 -0500 Subject: [PATCH 01/14] Stack Management - DB Migration Signed-off-by: Rashed Kamal --- .../20251117123719_add_state_to_stacks.rb | 13 +++ ...20251117123719_add_state_to_stacks_spec.rb | 91 +++++++++++++++++++ 2 files changed, 104 insertions(+) create mode 100644 db/migrations/20251117123719_add_state_to_stacks.rb create mode 100644 spec/migrations/20251117123719_add_state_to_stacks_spec.rb diff --git a/db/migrations/20251117123719_add_state_to_stacks.rb b/db/migrations/20251117123719_add_state_to_stacks.rb new file mode 100644 index 00000000000..29879bb882c --- /dev/null +++ b/db/migrations/20251117123719_add_state_to_stacks.rb @@ -0,0 +1,13 @@ +Sequel.migration do + up do + alter_table :stacks do + add_column :state, String, null: false, default: 'ACTIVE', size: 255 unless @db.schema(:stacks).map(&:first).include?(:state) + end + end + + down do + alter_table :stacks do + drop_column :state if @db.schema(:stacks).map(&:first).include?(:state) + end + end +end diff --git a/spec/migrations/20251117123719_add_state_to_stacks_spec.rb b/spec/migrations/20251117123719_add_state_to_stacks_spec.rb new file mode 100644 index 00000000000..6d7fcc88b03 --- /dev/null +++ b/spec/migrations/20251117123719_add_state_to_stacks_spec.rb @@ -0,0 +1,91 @@ +require 'spec_helper' +require 'migrations/helpers/migration_shared_context' + +RSpec.describe 'migration to add state column to stacks table', isolation: :truncation, type: :migration do + include_context 'migration' do + let(:migration_filename) { '20251117123719_add_state_to_stacks.rb' } + end + + describe 'stacks table' do + subject(:run_migration) { Sequel::Migrator.run(db, migrations_path, target: current_migration_index, allow_missing_migration_files: true) } + + describe 'up' do + it 'adds a column `state`' do + expect(db[:stacks].columns).not_to include(:state) + run_migration + expect(db[:stacks].columns).to include(:state) + end + + it 'sets the default value of existing stacks to ACTIVE' do + db[:stacks].insert(guid: SecureRandom.uuid, name: 'existing-stack', description: 'An existing stack') + run_migration + expect(db[:stacks].first(name: 'existing-stack')[:state]).to eq('ACTIVE') + end + + it 'sets the default value of new stacks to ACTIVE' do + run_migration + db[:stacks].insert(guid: SecureRandom.uuid, name: 'new-stack', description: 'A new stack') + expect(db[:stacks].first(name: 'new-stack')[:state]).to eq('ACTIVE') + end + + it 'forbids null values' do + run_migration + expect do + db[:stacks].insert(guid: SecureRandom.uuid, name: 'null-state-stack', description: 'A stack with null state', state: nil) + end.to raise_error(Sequel::NotNullConstraintViolation) + end + + it 'allows valid state values' do + run_migration + %w[ACTIVE DEPRECATED RESTRICTED DISABLED].each do |state| + expect do + db[:stacks].insert(guid:SecureRandom.uuid, name: "stack-#{state.downcase}", description: "A #{state} stack", state: state) + end.not_to raise_error + expect(db[:stacks].first(name: "stack-#{state.downcase}")[:state]).to eq(state) + end + end + + context 'when the column already exists' do + before do + db.alter_table :stacks do + add_column :state, String, null: false, default: 'ACTIVE', size: 255 unless @db.schema(:stacks).map(&:first).include?(:state) + end + end + + it 'does not fail' do + expect(db[:stacks].columns).to include(:state) + expect { run_migration }.not_to raise_error + expect(db[:stacks].columns).to include(:state) + end + end + end + + describe 'down' do + subject(:run_rollback) { Sequel::Migrator.run(db, migrations_path, target: current_migration_index - 1, allow_missing_migration_files: true) } + + before do + run_migration + end + + it 'removes the `state` column' do + expect(db[:stacks].columns).to include(:state) + run_rollback + expect(db[:stacks].columns).not_to include(:state) + end + + context 'when the column does not exist' do + before do + db.alter_table :stacks do + drop_column :state if @db.schema(:stacks).map(&:first).include?(:state) + end + end + + it 'does not fail' do + expect(db[:stacks].columns).not_to include(:state) + expect { run_rollback }.not_to raise_error + expect(db[:stacks].columns).not_to include(:state) + end + end + end + end +end From 71e206f794c07a39fdbe2390e95c310535a172fb Mon Sep 17 00:00:00 2001 From: Rashed Kamal Date: Thu, 20 Nov 2025 18:21:37 -0500 Subject: [PATCH 02/14] Stack Management - Model update and Unit Test Signed-off-by: Rashed Kamal --- app/models/runtime/stack.rb | 36 ++++++++++++++++++++++++++ spec/unit/models/runtime/stack_spec.rb | 25 +++++++++++++++++- 2 files changed, 60 insertions(+), 1 deletion(-) diff --git a/app/models/runtime/stack.rb b/app/models/runtime/stack.rb index 0637e612e1c..cd9038a540b 100644 --- a/app/models/runtime/stack.rb +++ b/app/models/runtime/stack.rb @@ -10,6 +10,18 @@ class MissingDefaultStackError < StandardError class AppsStillPresentError < StandardError end + STACK_ACTIVE = 'ACTIVE'.freeze + STACK_RESTRICTED = 'RESTRICTED'.freeze + STACK_DEPRECATED = 'DEPRECATED'.freeze + STACK_DISABLED = 'DISABLED'.freeze + + VALID_STATES = [ + STACK_ACTIVE, + STACK_RESTRICTED, + STACK_DEPRECATED, + STACK_DISABLED + ].freeze + # NOTE: that "apps" here returns processes for v2 meta-reasons many_to_many :apps, class: 'VCAP::CloudController::ProcessModel', @@ -43,6 +55,7 @@ def around_save def validate validates_presence :name validates_unique :name + validates_includes VALID_STATES, :state, allow_nil: true end def before_destroy @@ -98,5 +111,28 @@ def self.populate_from_hash(hash) create(hash.slice('name', 'description', 'build_rootfs_image', 'run_rootfs_image')) end end + def active? + state == STACK_ACTIVE + end + + def deprecated? + state == STACK_DEPRECATED + end + + def restricted? + state == STACK_RESTRICTED + end + + def disabled? + state == STACK_DISABLED + end + + def can_stage_new_app? + !restricted? && !disabled? + end + + def can_restage_apps? + !disabled? + end end end diff --git a/spec/unit/models/runtime/stack_spec.rb b/spec/unit/models/runtime/stack_spec.rb index 48987111025..f4aa8480595 100644 --- a/spec/unit/models/runtime/stack_spec.rb +++ b/spec/unit/models/runtime/stack_spec.rb @@ -26,11 +26,34 @@ module VCAP::CloudController it { is_expected.to validate_presence :name } it { is_expected.to validate_uniqueness :name } it { is_expected.to strip_whitespace :name } + + describe 'state validation' do + it 'accepts valid states' do + stack = Stack.make + Stack::VALID_STATES.each do |valid_state| + stack.state = valid_state + expect(stack).to be_valid + end + end + + it 'rejects invalid states' do + stack = Stack.make + stack.state = 'INVALID' + expect(stack).not_to be_valid + expect(stack.errors[:state]).to include(:includes) + end + + it 'allows nil state' do + stack = Stack.make + stack.state = nil + expect(stack).to be_valid + end + end end describe 'Serialization' do it { is_expected.to export_attributes :name, :description, :build_rootfs_image, :run_rootfs_image } - it { is_expected.to import_attributes :name, :description, :build_rootfs_image, :run_rootfs_image } + it { is_expected.to import_attributes :name, :description, :build_rootfs_image, :run_rootfs_image} end describe '.configure' do From 835939b295cfee11782e209a95bd35ad13d4122c Mon Sep 17 00:00:00 2001 From: Rashed Kamal Date: Fri, 21 Nov 2025 17:59:40 -0500 Subject: [PATCH 03/14] Expose stack state through V3 API endpoints only **V2 API endpoints must not expose or accept state parameter.** Signed-off-by: Rashed Kamal --- app/actions/stack_create.rb | 3 +- app/actions/stack_update.rb | 1 + app/messages/stack_create_message.rb | 10 +- app/messages/stack_update_message.rb | 4 +- app/models/helpers/stack_states.rb | 17 ++ app/models/runtime/stack.rb | 24 +-- app/presenters/v3/stack_presenter.rb | 1 + ...20251117123719_add_state_to_stacks_spec.rb | 2 +- spec/request/stacks_spec.rb | 11 + spec/request/stacks_state_spec.rb | 201 ++++++++++++++++++ .../messages/stack_create_message_spec.rb | 32 +++ .../messages/stack_update_message_spec.rb | 57 +++++ spec/unit/models/runtime/stack_spec.rb | 4 +- 13 files changed, 344 insertions(+), 23 deletions(-) create mode 100644 app/models/helpers/stack_states.rb create mode 100644 spec/request/stacks_state_spec.rb create mode 100644 spec/unit/messages/stack_update_message_spec.rb diff --git a/app/actions/stack_create.rb b/app/actions/stack_create.rb index 55e77bf78f7..142d780da42 100644 --- a/app/actions/stack_create.rb +++ b/app/actions/stack_create.rb @@ -12,7 +12,8 @@ def initialize(user_audit_info) def create(message) stack = VCAP::CloudController::Stack.create( name: message.name, - description: message.description + description: message.description, + state: message.state ) MetadataUpdate.update(stack, message) diff --git a/app/actions/stack_update.rb b/app/actions/stack_update.rb index 1980cff17b0..ec42149c64a 100644 --- a/app/actions/stack_update.rb +++ b/app/actions/stack_update.rb @@ -12,6 +12,7 @@ def initialize(user_audit_info) def update(stack, message) stack.db.transaction do + stack.update(state: message.state) if message.requested?(:state) MetadataUpdate.update(stack, message) Repositories::StackEventRepository.new.record_stack_update(stack, @user_audit_info, message.audit_hash) end diff --git a/app/messages/stack_create_message.rb b/app/messages/stack_create_message.rb index 082dfb4b565..7afea27773f 100644 --- a/app/messages/stack_create_message.rb +++ b/app/messages/stack_create_message.rb @@ -1,10 +1,18 @@ require 'messages/metadata_base_message' +require 'models/helpers/stack_states' module VCAP::CloudController class StackCreateMessage < MetadataBaseMessage - register_allowed_keys %i[name description] + register_allowed_keys %i[name description state] validates :name, presence: true, length: { maximum: 250 } validates :description, length: { maximum: 250 } + validates :state, inclusion: { in: StackStates::VALID_STATES, message: "must be one of #{StackStates::VALID_STATES.join(', ')}" }, allow_nil: true + + def state + return @state if defined?(@state) + + @state = requested?(:state) ? super : StackStates::DEFAULT_STATE + end end end diff --git a/app/messages/stack_update_message.rb b/app/messages/stack_update_message.rb index c9fce2392fb..4050b942b5e 100644 --- a/app/messages/stack_update_message.rb +++ b/app/messages/stack_update_message.rb @@ -1,9 +1,11 @@ require 'messages/metadata_base_message' +require 'models/helpers/stack_states' module VCAP::CloudController class StackUpdateMessage < MetadataBaseMessage - register_allowed_keys [] + register_allowed_keys [:state] validates_with NoAdditionalKeysValidator + validates :state, inclusion: { in: StackStates::VALID_STATES, message: "must be one of #{StackStates::VALID_STATES.join(', ')}" }, allow_nil: true end end diff --git a/app/models/helpers/stack_states.rb b/app/models/helpers/stack_states.rb new file mode 100644 index 00000000000..097e7c8e819 --- /dev/null +++ b/app/models/helpers/stack_states.rb @@ -0,0 +1,17 @@ +module VCAP::CloudController + class StackStates + STACK_ACTIVE = 'ACTIVE'.freeze + STACK_RESTRICTED = 'RESTRICTED'.freeze + STACK_DEPRECATED = 'DEPRECATED'.freeze + STACK_DISABLED = 'DISABLED'.freeze + + DEFAULT_STATE = STACK_ACTIVE + + VALID_STATES = [ + STACK_ACTIVE, + STACK_RESTRICTED, + STACK_DEPRECATED, + STACK_DISABLED + ].freeze + end +end diff --git a/app/models/runtime/stack.rb b/app/models/runtime/stack.rb index cd9038a540b..98db03ed298 100644 --- a/app/models/runtime/stack.rb +++ b/app/models/runtime/stack.rb @@ -1,5 +1,6 @@ require 'models/helpers/process_types' require 'models/helpers/stack_config_file' +require 'models/helpers/stack_states' module VCAP::CloudController class Stack < Sequel::Model @@ -10,18 +11,6 @@ class MissingDefaultStackError < StandardError class AppsStillPresentError < StandardError end - STACK_ACTIVE = 'ACTIVE'.freeze - STACK_RESTRICTED = 'RESTRICTED'.freeze - STACK_DEPRECATED = 'DEPRECATED'.freeze - STACK_DISABLED = 'DISABLED'.freeze - - VALID_STATES = [ - STACK_ACTIVE, - STACK_RESTRICTED, - STACK_DEPRECATED, - STACK_DISABLED - ].freeze - # NOTE: that "apps" here returns processes for v2 meta-reasons many_to_many :apps, class: 'VCAP::CloudController::ProcessModel', @@ -55,7 +44,7 @@ def around_save def validate validates_presence :name validates_unique :name - validates_includes VALID_STATES, :state, allow_nil: true + validates_includes StackStates::VALID_STATES, :state, allow_nil: true end def before_destroy @@ -111,20 +100,21 @@ def self.populate_from_hash(hash) create(hash.slice('name', 'description', 'build_rootfs_image', 'run_rootfs_image')) end end + def active? - state == STACK_ACTIVE + state == StackStates::STACK_ACTIVE end def deprecated? - state == STACK_DEPRECATED + state == StackStates::STACK_DEPRECATED end def restricted? - state == STACK_RESTRICTED + state == StackStates::STACK_RESTRICTED end def disabled? - state == STACK_DISABLED + state == StackStates::STACK_DISABLED end def can_stage_new_app? diff --git a/app/presenters/v3/stack_presenter.rb b/app/presenters/v3/stack_presenter.rb index eaff5313bf0..a053eb69f80 100644 --- a/app/presenters/v3/stack_presenter.rb +++ b/app/presenters/v3/stack_presenter.rb @@ -12,6 +12,7 @@ def to_hash updated_at: stack.updated_at, name: stack.name, description: stack.description, + state: stack.state, run_rootfs_image: stack.run_rootfs_image, build_rootfs_image: stack.build_rootfs_image, default: stack.default?, diff --git a/spec/migrations/20251117123719_add_state_to_stacks_spec.rb b/spec/migrations/20251117123719_add_state_to_stacks_spec.rb index 6d7fcc88b03..ce3a56afcae 100644 --- a/spec/migrations/20251117123719_add_state_to_stacks_spec.rb +++ b/spec/migrations/20251117123719_add_state_to_stacks_spec.rb @@ -39,7 +39,7 @@ run_migration %w[ACTIVE DEPRECATED RESTRICTED DISABLED].each do |state| expect do - db[:stacks].insert(guid:SecureRandom.uuid, name: "stack-#{state.downcase}", description: "A #{state} stack", state: state) + db[:stacks].insert(guid: SecureRandom.uuid, name: "stack-#{state.downcase}", description: "A #{state} stack", state: state) end.not_to raise_error expect(db[:stacks].first(name: "stack-#{state.downcase}")[:state]).to eq(state) end diff --git a/spec/request/stacks_spec.rb b/spec/request/stacks_spec.rb index fd64bcd017e..f348e1bdfab 100644 --- a/spec/request/stacks_spec.rb +++ b/spec/request/stacks_spec.rb @@ -26,6 +26,7 @@ 'run_rootfs_image' => stack1.run_rootfs_image, 'build_rootfs_image' => stack1.build_rootfs_image, 'guid' => stack1.guid, + 'state' => 'ACTIVE', 'default' => false, 'metadata' => { 'labels' => {}, 'annotations' => {} }, 'created_at' => iso8601, @@ -42,6 +43,7 @@ 'run_rootfs_image' => stack2.run_rootfs_image, 'build_rootfs_image' => stack2.build_rootfs_image, 'guid' => stack2.guid, + 'state' => 'ACTIVE', 'default' => true, 'metadata' => { 'labels' => {}, 'annotations' => {} }, 'created_at' => iso8601, @@ -122,6 +124,7 @@ 'run_rootfs_image' => stack1.run_rootfs_image, 'build_rootfs_image' => stack1.build_rootfs_image, 'guid' => stack1.guid, + 'state' => 'ACTIVE', 'default' => false, 'metadata' => { 'labels' => {}, 'annotations' => {} }, 'created_at' => iso8601, @@ -138,6 +141,7 @@ 'run_rootfs_image' => stack2.run_rootfs_image, 'build_rootfs_image' => stack2.build_rootfs_image, 'guid' => stack2.guid, + 'state' => 'ACTIVE', 'default' => true, 'metadata' => { 'labels' => {}, 'annotations' => {} }, 'created_at' => iso8601, @@ -177,6 +181,7 @@ 'run_rootfs_image' => stack1.run_rootfs_image, 'build_rootfs_image' => stack1.build_rootfs_image, 'guid' => stack1.guid, + 'state' => 'ACTIVE', 'default' => false, 'metadata' => { 'labels' => {}, 'annotations' => {} }, 'created_at' => iso8601, @@ -193,6 +198,7 @@ 'run_rootfs_image' => stack3.run_rootfs_image, 'build_rootfs_image' => stack3.build_rootfs_image, 'guid' => stack3.guid, + 'state' => 'ACTIVE', 'default' => false, 'metadata' => { 'labels' => {}, 'annotations' => {} }, 'created_at' => iso8601, @@ -232,6 +238,7 @@ 'run_rootfs_image' => stack2.run_rootfs_image, 'build_rootfs_image' => stack2.build_rootfs_image, 'guid' => stack2.guid, + 'state' => 'ACTIVE', 'default' => true, 'metadata' => { 'labels' => {}, 'annotations' => {} }, 'created_at' => iso8601, @@ -287,6 +294,7 @@ 'run_rootfs_image' => stack1.run_rootfs_image, 'build_rootfs_image' => stack1.build_rootfs_image, 'guid' => stack1.guid, + 'state' => 'ACTIVE', 'default' => false, 'metadata' => { 'labels' => { @@ -323,6 +331,7 @@ 'run_rootfs_image' => stack.run_rootfs_image, 'build_rootfs_image' => stack.build_rootfs_image, 'guid' => stack.guid, + 'state' => 'ACTIVE', 'default' => false, 'metadata' => { 'labels' => {}, 'annotations' => {} }, 'created_at' => iso8601, @@ -670,6 +679,7 @@ } }, 'guid' => created_stack.guid, + 'state' => 'ACTIVE', 'created_at' => iso8601, 'updated_at' => iso8601, 'links' => { @@ -734,6 +744,7 @@ } }, 'guid' => stack.guid, + 'state' => 'ACTIVE', 'created_at' => iso8601, 'updated_at' => iso8601, 'links' => { diff --git a/spec/request/stacks_state_spec.rb b/spec/request/stacks_state_spec.rb new file mode 100644 index 00000000000..d7ba3fd67a7 --- /dev/null +++ b/spec/request/stacks_state_spec.rb @@ -0,0 +1,201 @@ +require 'spec_helper' +require 'request_spec_shared_examples' + +RSpec.describe 'Stacks State Management' do + let(:stack_config_file) { File.join(Paths::FIXTURES, 'config/stacks.yml') } + let(:user) { make_user(admin: true) } + let(:headers) { admin_headers_for(user) } + + before { VCAP::CloudController::Stack.configure(stack_config_file) } + + describe 'POST /v3/stacks with state' do + context 'when creating stack with explicit state' do + %w[ACTIVE DEPRECATED RESTRICTED DISABLED].each do |state| + it "creates stack with #{state} state" do + request_body = { + name: "stack-#{state.downcase}", + description: 'test stack', + state: state + }.to_json + + post '/v3/stacks', request_body, headers + + expect(last_response.status).to eq(201) + expect(parsed_response['state']).to eq(state) + expect(parsed_response['name']).to eq("stack-#{state.downcase}") + end + end + end + + context 'when creating stack without state' do + it 'defaults to ACTIVE' do + request_body = { + name: 'default-state-stack', + description: 'test stack' + }.to_json + + post '/v3/stacks', request_body, headers + + expect(last_response.status).to eq(201) + expect(parsed_response['state']).to eq('ACTIVE') + end + end + + context 'when creating stack with invalid state' do + it 'returns validation error' do + request_body = { + name: 'invalid-stack', + state: 'INVALID_STATE' + }.to_json + + post '/v3/stacks', request_body, headers + + expect(last_response.status).to eq(422) + expect(parsed_response['errors'].first['detail']).to include('must be one of ACTIVE, RESTRICTED, DEPRECATED, DISABLED') + end + end + + context 'as non-admin user' do + let(:non_admin_user) { make_user } + let(:non_admin_headers) { headers_for(non_admin_user) } + + it 'is unauthorized' do + request_body = { + name: 'test-stack', + state: 'ACTIVE' + }.to_json + + post '/v3/stacks', request_body, non_admin_headers + + expect(last_response.status).to eq(403) + end + end + end + + describe 'PATCH /v3/stacks/:guid with state' do + let!(:stack) { VCAP::CloudController::Stack.make(name: 'test-stack', state: 'ACTIVE') } + + context 'when updating state through lifecycle' do + it 'transitions from ACTIVE to DEPRECATED' do + request_body = { state: 'DEPRECATED' }.to_json + + patch "/v3/stacks/#{stack.guid}", request_body, headers + + expect(last_response.status).to eq(200) + expect(parsed_response['state']).to eq('DEPRECATED') + + stack.reload + expect(stack.state).to eq('DEPRECATED') + end + + it 'transitions from DEPRECATED to RESTRICTED' do + stack.update(state: 'DEPRECATED') + + request_body = { state: 'RESTRICTED' }.to_json + + patch "/v3/stacks/#{stack.guid}", request_body, headers + + expect(last_response.status).to eq(200) + expect(parsed_response['state']).to eq('RESTRICTED') + end + + it 'transitions from RESTRICTED to DISABLED' do + stack.update(state: 'RESTRICTED') + + request_body = { state: 'DISABLED' }.to_json + + patch "/v3/stacks/#{stack.guid}", request_body, headers + + expect(last_response.status).to eq(200) + expect(parsed_response['state']).to eq('DISABLED') + end + + it 'allows transition back to ACTIVE' do + stack.update(state: 'DEPRECATED') + + request_body = { state: 'ACTIVE' }.to_json + + patch "/v3/stacks/#{stack.guid}", request_body, headers + + expect(last_response.status).to eq(200) + expect(parsed_response['state']).to eq('ACTIVE') + end + end + + context 'when updating with invalid state' do + it 'returns validation error' do + request_body = { state: 'BOGUS_STATE' }.to_json + + patch "/v3/stacks/#{stack.guid}", request_body, headers + + expect(last_response.status).to eq(422) + expect(parsed_response['errors'].first['detail']).to include('must be one of ACTIVE, RESTRICTED, DEPRECATED, DISABLED') + end + end + + context 'when updating metadata without changing state' do + it 'preserves existing state' do + stack.update(state: 'DEPRECATED') + + request_body = { + metadata: { + labels: { test: 'label' } + } + }.to_json + + patch "/v3/stacks/#{stack.guid}", request_body, headers + + expect(last_response.status).to eq(200) + expect(parsed_response['state']).to eq('DEPRECATED') + expect(parsed_response['metadata']['labels']['test']).to eq('label') + end + end + + context 'as non-admin user' do + let(:non_admin_user) { make_user } + let(:non_admin_headers) { headers_for(non_admin_user) } + + it 'is unauthorized' do + request_body = { state: 'DEPRECATED' }.to_json + + patch "/v3/stacks/#{stack.guid}", request_body, non_admin_headers + + expect(last_response.status).to eq(403) + end + end + end + + describe 'GET /v3/stacks/:guid' do + let!(:deprecated_stack) { VCAP::CloudController::Stack.make(state: 'DEPRECATED') } + let(:reader_user) { make_user } + let(:reader_headers) { headers_for(reader_user) } + + it 'returns state field for all users' do + get "/v3/stacks/#{deprecated_stack.guid}", nil, reader_headers + + expect(last_response.status).to eq(200) + expect(parsed_response['state']).to eq('DEPRECATED') + end + end + + describe 'GET /v3/stacks' do + before { VCAP::CloudController::Stack.dataset.destroy } + + let!(:active_stack) { VCAP::CloudController::Stack.make(name: 'active', state: 'ACTIVE') } + let!(:deprecated_stack) { VCAP::CloudController::Stack.make(name: 'deprecated', state: 'DEPRECATED') } + let!(:restricted_stack) { VCAP::CloudController::Stack.make(name: 'restricted', state: 'RESTRICTED') } + let!(:disabled_stack) { VCAP::CloudController::Stack.make(name: 'disabled', state: 'DISABLED') } + + let(:reader_user) { make_user } + let(:reader_headers) { headers_for(reader_user) } + + it 'includes state for all stacks' do + get '/v3/stacks', nil, reader_headers + + expect(last_response.status).to eq(200) + + resources = parsed_response['resources'] + expect(resources.pluck('state')).to contain_exactly('ACTIVE', 'DEPRECATED', 'RESTRICTED', 'DISABLED') + end + end +end diff --git a/spec/unit/messages/stack_create_message_spec.rb b/spec/unit/messages/stack_create_message_spec.rb index d854ecf0b86..df7ffc8b950 100644 --- a/spec/unit/messages/stack_create_message_spec.rb +++ b/spec/unit/messages/stack_create_message_spec.rb @@ -81,5 +81,37 @@ end end end + + describe 'state' do + context 'when it is not provided' do + let(:params) { valid_params } + + it 'defaults to ACTIVE' do + expect(subject.state).to eq('ACTIVE') + end + end + + context 'when it is a valid state' do + %w[ACTIVE DEPRECATED RESTRICTED DISABLED].each do |valid_state| + context "when state is #{valid_state}" do + let(:params) { valid_params.merge({ state: valid_state }) } + + it 'is valid' do + expect(subject).to be_valid + expect(subject.state).to eq(valid_state) + end + end + end + end + + context 'when it is an invalid state' do + let(:params) { valid_params.merge({ state: 'INVALID_STATE' }) } + + it 'returns an error' do + expect(subject).not_to be_valid + expect(subject.errors[:state]).to include('must be one of ACTIVE, RESTRICTED, DEPRECATED, DISABLED') + end + end + end end end diff --git a/spec/unit/messages/stack_update_message_spec.rb b/spec/unit/messages/stack_update_message_spec.rb new file mode 100644 index 00000000000..f6363cb681c --- /dev/null +++ b/spec/unit/messages/stack_update_message_spec.rb @@ -0,0 +1,57 @@ +require 'lightweight_spec_helper' +require 'messages/stack_update_message' + +RSpec.describe VCAP::CloudController::StackUpdateMessage do + describe 'validations' do + subject { described_class.new(params) } + + let(:valid_params) do + { + metadata: { + labels: { + potato: 'mashed' + }, + annotations: { + happy: 'annotation' + } + } + } + end + + it 'is valid with metadata only' do + expect(described_class.new(valid_params)).to be_valid + end + + describe 'state' do + context 'when it is a valid state' do + %w[ACTIVE DEPRECATED RESTRICTED DISABLED].each do |valid_state| + context "when state is #{valid_state}" do + let(:params) { valid_params.merge({ state: valid_state }) } + + it 'is valid' do + expect(subject).to be_valid + expect(subject.state).to eq(valid_state) + end + end + end + end + + context 'when it is an invalid state' do + let(:params) { valid_params.merge({ state: 'INVALID_STATE' }) } + + it 'returns an error' do + expect(subject).not_to be_valid + expect(subject.errors[:state]).to include('must be one of ACTIVE, RESTRICTED, DEPRECATED, DISABLED') + end + end + + context 'when it is not provided' do + let(:params) { valid_params } + + it 'is valid' do + expect(subject).to be_valid + end + end + end + end +end diff --git a/spec/unit/models/runtime/stack_spec.rb b/spec/unit/models/runtime/stack_spec.rb index f4aa8480595..ac43503f088 100644 --- a/spec/unit/models/runtime/stack_spec.rb +++ b/spec/unit/models/runtime/stack_spec.rb @@ -30,7 +30,7 @@ module VCAP::CloudController describe 'state validation' do it 'accepts valid states' do stack = Stack.make - Stack::VALID_STATES.each do |valid_state| + StackStates::VALID_STATES.each do |valid_state| stack.state = valid_state expect(stack).to be_valid end @@ -53,7 +53,7 @@ module VCAP::CloudController describe 'Serialization' do it { is_expected.to export_attributes :name, :description, :build_rootfs_image, :run_rootfs_image } - it { is_expected.to import_attributes :name, :description, :build_rootfs_image, :run_rootfs_image} + it { is_expected.to import_attributes :name, :description, :build_rootfs_image, :run_rootfs_image } end describe '.configure' do From d55b71ab95e42e98982ac18b8b5ebad3070ebfa4 Mon Sep 17 00:00:00 2001 From: Rashed Kamal Date: Thu, 4 Dec 2025 13:36:59 -0500 Subject: [PATCH 04/14] Add Stack States in API documentation Signed-off-by: Rashed Kamal --- .../source/includes/api_resources/_stacks.erb | 26 +++++++++++++++++++ .../includes/resources/stacks/_object.md.erb | 1 + .../includes/resources/stacks/_update.md.erb | 5 ++-- 3 files changed, 30 insertions(+), 2 deletions(-) diff --git a/docs/v3/source/includes/api_resources/_stacks.erb b/docs/v3/source/includes/api_resources/_stacks.erb index 2e91570c628..4abc6868cd7 100644 --- a/docs/v3/source/includes/api_resources/_stacks.erb +++ b/docs/v3/source/includes/api_resources/_stacks.erb @@ -5,6 +5,7 @@ "updated_at": "2018-11-09T22:43:28Z", "name": "my-stack", "description": "Here is my stack!", + "state": "ACTIVE", "build_rootfs_image": "my-stack", "run_rootfs_image": "my-stack", "default": true, @@ -45,6 +46,7 @@ "build_rootfs_image": "my-stack-1-build", "run_rootfs_image": "my-stack-1-run", "description": "This is my first stack!", + "state": "ACTIVE", "default": true, "metadata": { "labels": {}, @@ -64,6 +66,7 @@ "description": "This is my second stack!", "build_rootfs_image": "my-stack-2-build", "run_rootfs_image": "my-stack-2-run", + "state": "DEPRECATED", "default": false, "metadata": { "labels": {}, @@ -79,3 +82,26 @@ } <% end %> + +<% content_for :single_stack_disabled do | metadata={} | %> +{ + "guid": "11c916c9-c2f9-440e-8e73-102e79c4704d", + "created_at": "2018-11-09T22:43:28Z", + "updated_at": "2018-11-09T22:43:28Z", + "name": "my-stack", + "description": "Here is my stack!", + "state": "ACTIVE", + "build_rootfs_image": "my-stack", + "run_rootfs_image": "my-stack", + "default": true, + "metadata": { + "labels": <%= metadata.fetch(:labels, {}).to_json(space: ' ', object_nl: ' ')%>, + "annotations": <%= metadata.fetch(:annotations, {}).to_json(space: ' ', object_nl: ' ')%> + }, + "links": { + "self": { + "href": "https://api.example.com/v3/stacks/11c916c9-c2f9-440e-8e73-102e79c4704d" + } + } +} +<% end %> diff --git a/docs/v3/source/includes/resources/stacks/_object.md.erb b/docs/v3/source/includes/resources/stacks/_object.md.erb index 09e7280a9f9..cb20ae480b4 100644 --- a/docs/v3/source/includes/resources/stacks/_object.md.erb +++ b/docs/v3/source/includes/resources/stacks/_object.md.erb @@ -14,6 +14,7 @@ Name | Type | Description **updated_at** | _[timestamp](#timestamps)_ | The time with zone when the object was last updated **name** | _string_ | The name of the stack **description** | _string_ | The description of the stack +**state** | string | The state of the stack; valid states are: `ACTIVE`, `RESTRICTED`, `DEPRECATED`, `DISABLED` **build_rootfs_image** | _string | The name of the stack image associated with staging/building Apps. If a stack does not have unique images, this will be the same as the stack name. **run_rootfs_image** | _string | The name of the stack image associated with running Apps + Tasks. If a stack does not have unique images, this will be the same as the stack name. **default** | _boolean_ | Whether the stack is configured to be the default stack for new applications. diff --git a/docs/v3/source/includes/resources/stacks/_update.md.erb b/docs/v3/source/includes/resources/stacks/_update.md.erb index fcca207f005..567fc51c000 100644 --- a/docs/v3/source/includes/resources/stacks/_update.md.erb +++ b/docs/v3/source/includes/resources/stacks/_update.md.erb @@ -9,7 +9,7 @@ curl "https://api.example.org/v3/stacks/[guid]" \ -X PATCH \ -H "Authorization: bearer [token]" \ -H "Content-Type: application/json" \ - -d '{ "metadata": { "labels": { "key": "value" }, "annotations": {"note": "detailed information"}}}' + -d '{ "metadata": { "labels": { "key": "value" }, "annotations": {"note": "detailed information"}, "state": "DISABLED" }}' ``` @@ -21,7 +21,7 @@ Example Response HTTP/1.1 200 OK Content-Type: application/json -<%= yield_content :single_stack, labels: { "key" => "value" }, "annotations": {"note" => "detailed information"} %> +<%= yield_content :single_stack_disabled, labels: { "key" => "value" }, "annotations": {"note" => "detailed information"} %> ``` #### Definition @@ -33,6 +33,7 @@ Name | Type | Description ---- | ---- | ----------- **metadata.labels** | [_label object_](#labels) | Labels applied to the stack **metadata.annotations** | [_annotation object_](#annotations) | Annotations applied to the stack +**state** | string | The state of the stack; valid states are: `ACTIVE`, `RESTRICTED`, `DEPRECATED`, `DISABLED` #### Permitted roles | From f0333344b2b689e8cb153ae59a69ebf791ce1d45 Mon Sep 17 00:00:00 2001 From: Rashed Kamal Date: Thu, 11 Dec 2025 10:38:41 -0500 Subject: [PATCH 05/14] Additional validation and tests for v3/stacks API Signed-off-by: Rashed Kamal --- app/messages/stack_create_message.rb | 6 +++++- app/messages/stack_update_message.rb | 6 +++++- spec/request/stacks_state_spec.rb | 14 ++++++++++++++ spec/unit/messages/stack_create_message_spec.rb | 9 +++++++++ spec/unit/messages/stack_update_message_spec.rb | 9 +++++++++ 5 files changed, 42 insertions(+), 2 deletions(-) diff --git a/app/messages/stack_create_message.rb b/app/messages/stack_create_message.rb index 7afea27773f..c9f8a91a32c 100644 --- a/app/messages/stack_create_message.rb +++ b/app/messages/stack_create_message.rb @@ -7,7 +7,11 @@ class StackCreateMessage < MetadataBaseMessage validates :name, presence: true, length: { maximum: 250 } validates :description, length: { maximum: 250 } - validates :state, inclusion: { in: StackStates::VALID_STATES, message: "must be one of #{StackStates::VALID_STATES.join(', ')}" }, allow_nil: true + validates :state, inclusion: { in: StackStates::VALID_STATES, message: "must be one of #{StackStates::VALID_STATES.join(', ')}" }, allow_nil: false, if: :state_requested? + + def state_requested? + requested?(:state) + end def state return @state if defined?(@state) diff --git a/app/messages/stack_update_message.rb b/app/messages/stack_update_message.rb index 4050b942b5e..ecd98baf693 100644 --- a/app/messages/stack_update_message.rb +++ b/app/messages/stack_update_message.rb @@ -6,6 +6,10 @@ class StackUpdateMessage < MetadataBaseMessage register_allowed_keys [:state] validates_with NoAdditionalKeysValidator - validates :state, inclusion: { in: StackStates::VALID_STATES, message: "must be one of #{StackStates::VALID_STATES.join(', ')}" }, allow_nil: true + validates :state, inclusion: { in: StackStates::VALID_STATES, message: "must be one of #{StackStates::VALID_STATES.join(', ')}" }, allow_nil: false, if: :state_requested? + + def state_requested? + requested?(:state) + end end end diff --git a/spec/request/stacks_state_spec.rb b/spec/request/stacks_state_spec.rb index d7ba3fd67a7..07b3257ec1e 100644 --- a/spec/request/stacks_state_spec.rb +++ b/spec/request/stacks_state_spec.rb @@ -55,6 +55,20 @@ end end + context 'when creating stack with null state' do + it 'returns validation error' do + request_body = { + name: 'stack-null-state', + state: nil + }.to_json + + post '/v3/stacks', request_body, headers + + expect(last_response.status).to eq(422) + expect(parsed_response['errors'].first['detail']).to include('must be one of ACTIVE, RESTRICTED, DEPRECATED, DISABLED') + end + end + context 'as non-admin user' do let(:non_admin_user) { make_user } let(:non_admin_headers) { headers_for(non_admin_user) } diff --git a/spec/unit/messages/stack_create_message_spec.rb b/spec/unit/messages/stack_create_message_spec.rb index df7ffc8b950..a8383b86c3c 100644 --- a/spec/unit/messages/stack_create_message_spec.rb +++ b/spec/unit/messages/stack_create_message_spec.rb @@ -112,6 +112,15 @@ expect(subject.errors[:state]).to include('must be one of ACTIVE, RESTRICTED, DEPRECATED, DISABLED') end end + + context 'when it is explicitly null' do + let(:params) { valid_params.merge({ state: nil }) } + + it 'returns an error' do + expect(subject).not_to be_valid + expect(subject.errors[:state]).to include('must be one of ACTIVE, RESTRICTED, DEPRECATED, DISABLED') + end + end end end end diff --git a/spec/unit/messages/stack_update_message_spec.rb b/spec/unit/messages/stack_update_message_spec.rb index f6363cb681c..eae64b1caac 100644 --- a/spec/unit/messages/stack_update_message_spec.rb +++ b/spec/unit/messages/stack_update_message_spec.rb @@ -52,6 +52,15 @@ expect(subject).to be_valid end end + + context 'when it is explicitly null' do + let(:params) { valid_params.merge({ state: nil }) } + + it 'returns an error' do + expect(subject).not_to be_valid + expect(subject.errors[:state]).to include('must be one of ACTIVE, RESTRICTED, DEPRECATED, DISABLED') + end + end end end end From 85e114bd7c2ea5c79ceef8730ae3afad669cc407 Mon Sep 17 00:00:00 2001 From: Rashed Kamal Date: Sat, 13 Dec 2025 12:33:08 -0500 Subject: [PATCH 06/14] Standalone Stack State Validator - State Logic class Signed-off-by: Rashed Kamal --- lib/cloud_controller.rb | 2 + lib/cloud_controller/stack_state_validator.rb | 28 +++ .../stack_state_validator_spec.rb | 238 ++++++++++++++++++ 3 files changed, 268 insertions(+) create mode 100644 lib/cloud_controller/stack_state_validator.rb create mode 100644 spec/unit/lib/cloud_controller/stack_state_validator_spec.rb diff --git a/lib/cloud_controller.rb b/lib/cloud_controller.rb index 5875c33462a..ba8297fdc47 100644 --- a/lib/cloud_controller.rb +++ b/lib/cloud_controller.rb @@ -117,3 +117,5 @@ module VCAP::CloudController; end require 'cloud_controller/errands/rotate_database_key' require 'services' + +require 'cloud_controller/stack_state_validator' diff --git a/lib/cloud_controller/stack_state_validator.rb b/lib/cloud_controller/stack_state_validator.rb new file mode 100644 index 00000000000..d8f874e6c2f --- /dev/null +++ b/lib/cloud_controller/stack_state_validator.rb @@ -0,0 +1,28 @@ +module VCAP::CloudController + class StackStateValidator + class StackValidationError < StandardError; end + class DisabledStackError < StackValidationError; end + class RestrictedStackError < StackValidationError; end + def self.validate_for_new_app!(stack) + return [] if stack.active? + + raise DisabledStackError.new("Stack '#{stack.name}' is disabled and cannot be used for staging new applications. #{stack.description}") if stack.disabled? + + raise RestrictedStackError.new("Stack '#{stack.name}' is restricted and cannot be used for staging new applications. #{stack.description}") if stack.restricted? + + stack.deprecated? ? [build_deprecation_warning(stack)] : [] + end + + def self.validate_for_restaging!(stack) + return [] if stack.active? || stack.restricted? + + raise DisabledStackError.new("Stack '#{stack.name}' is disabled and cannot be used for staging new applications. #{stack.description}") if stack.disabled? + + stack.deprecated? ? [build_deprecation_warning(stack)] : [] + end + + def self.build_deprecation_warning(stack) + "Stack '#{stack.name}' is deprecated and will be removed in the future. #{stack.description}" + end + end +end diff --git a/spec/unit/lib/cloud_controller/stack_state_validator_spec.rb b/spec/unit/lib/cloud_controller/stack_state_validator_spec.rb new file mode 100644 index 00000000000..f6222220576 --- /dev/null +++ b/spec/unit/lib/cloud_controller/stack_state_validator_spec.rb @@ -0,0 +1,238 @@ +require 'spec_helper' + +module VCAP::CloudController + RSpec.describe StackStateValidator do + describe '.validate_for_new_app!' do + context 'when stack is Active' do + let(:stack) { Stack.make(state: StackStates::STACK_ACTIVE, description: 'My ACTIVE stack') } + + it 'returns empty warnings' do + result = StackStateValidator.validate_for_new_app!(stack) + expect(result).to eq([]) + end + + it 'does not raise an error' do + expect { StackStateValidator.validate_for_new_app!(stack) }.not_to raise_error + end + end + + context 'when stack is DEPRECATED' do + let(:stack) { Stack.make(state: StackStates::STACK_DEPRECATED, description: 'My DEPRECATED stack') } + + it 'returns a warning message' do + result = StackStateValidator.validate_for_new_app!(stack) + expect(result).to be_an(Array) + expect(result.size).to eq(1) + expect(result.first).to include("Stack '#{stack.name}' is deprecated and will be removed in the future. #{stack.description}") + end + + it 'returns a warning message with stack name' do + result = StackStateValidator.validate_for_new_app!(stack) + warning = result.first + expect(warning).to include(stack.name) + expect(warning).to include(stack.description) + expect(warning).to include('deprecated') + end + + it 'does not raise an error' do + expect { StackStateValidator.validate_for_new_app!(stack) }.not_to raise_error + end + end + + context 'when stack is RESTRICTED' do + let(:stack) { Stack.make(state: StackStates::STACK_RESTRICTED, description: 'My RESTRICTED stack') } + + it 'raise RestrictedStackError' do + expect do + StackStateValidator.validate_for_new_app!(stack) + end.to raise_error(StackStateValidator::RestrictedStackError, /Stack '#{stack.name}' is restricted and cannot be used for staging new applications./) + end + + it 'includes stack name in error message' do + expect do + StackStateValidator.validate_for_new_app!(stack) + end.to raise_error(StackStateValidator::RestrictedStackError, /#{stack.name}/) + end + + it 'includes stack description in error message' do + expect do + StackStateValidator.validate_for_new_app!(stack) + end.to raise_error(StackStateValidator::RestrictedStackError, /#{stack.description}/) + end + + it 'raises RestrictedStackError which is a StackStateValidator::Error' do + expect do + StackStateValidator.validate_for_new_app!(stack) + end.to raise_error(StackStateValidator::StackValidationError) + end + end + + context 'when stack is DISABLED' do + let(:stack) { Stack.make(state: StackStates::STACK_DISABLED, description: 'My DEPRECATED stack') } + + it 'returns a disabled error message' do + expect do + StackStateValidator.validate_for_new_app!(stack) + end.to raise_error(StackStateValidator::DisabledStackError, /Stack '#{stack.name}' is disabled and cannot be used for staging new applications./) + end + + it 'includes stack name in error message' do + expect do + StackStateValidator.validate_for_new_app!(stack) + end.to raise_error(StackStateValidator::DisabledStackError, /#{stack.name}/) + end + + it 'includes stack description in error message' do + expect do + StackStateValidator.validate_for_new_app!(stack) + end.to raise_error(StackStateValidator::DisabledStackError, /#{stack.description}/) + end + end + end + + describe '.validate_for_restaging_app!' do + context 'when stack is Active' do + let(:stack) { Stack.make(state: StackStates::STACK_ACTIVE, description: 'My ACTIVE stack') } + + it 'for restaging returns empty warnings' do + result = StackStateValidator.validate_for_restaging!(stack) + expect(result).to eq([]) + end + + it 'does not raise an error' do + expect { StackStateValidator.validate_for_new_app!(stack) }.not_to raise_error + end + end + + context 'when stack is DEPRECATED' do + let(:stack) { Stack.make(state: StackStates::STACK_DEPRECATED, description: 'My DEPRECATED stack') } + + it 'returns a warning message' do + result = StackStateValidator.validate_for_restaging!(stack) + expect(result).to be_an(Array) + expect(result.size).to eq(1) + expect(result.first).to include("Stack '#{stack.name}' is deprecated and will be removed in the future. #{stack.description}") + end + + it 'returns a warning message with stack name' do + result = StackStateValidator.validate_for_restaging!(stack) + warning = result.first + expect(warning).to include(stack.name) + expect(warning).to include(stack.description) + expect(warning).to include('deprecated') + end + + it 'does not raise an error' do + expect { StackStateValidator.validate_for_restaging!(stack) }.not_to raise_error + end + end + + context 'when stack is RESTRICTED' do + let(:stack) { Stack.make(state: StackStates::STACK_RESTRICTED, description: 'My RESTRICTED stack') } + + it 'returns empty warnings' do + result = StackStateValidator.validate_for_restaging!(stack) + expect(result).to eq([]) + end + + it 'does not raise an error' do + expect { StackStateValidator.validate_for_restaging!(stack) }.not_to raise_error + end + end + + context 'when stack is DISABLED' do + let(:stack) { Stack.make(state: StackStates::STACK_DISABLED, description: 'My DEPRECATED stack') } + + it 'returns a disabled error message' do + expect do + StackStateValidator.validate_for_restaging!(stack) + end.to raise_error(StackStateValidator::DisabledStackError, /Stack '#{stack.name}' is disabled and cannot be used for staging new applications./) + end + + it 'includes stack name in error message' do + expect do + StackStateValidator.validate_for_restaging!(stack) + end.to raise_error(StackStateValidator::DisabledStackError, /#{stack.name}/) + end + + it 'includes stack description in error message' do + expect do + StackStateValidator.validate_for_restaging!(stack) + end.to raise_error(StackStateValidator::DisabledStackError, /#{stack.description}/) + end + end + end + + describe '.build_deprecation_warning' do + let(:stack) { Stack.make(name: 'cflinuxfs3', description: 'End of life December 2025') } + + it 'returns formatted warning string' do + warning = StackStateValidator.build_deprecation_warning(stack) + expect(warning).to be_a(String) + expect(warning).to include('cflinuxfs3') + expect(warning).to include('deprecated') + expect(warning).to include('End of life December 2025') + end + + it 'includes stack name when description is empty' do + stack.description = '' + warning = StackStateValidator.build_deprecation_warning(stack) + expect(warning).to include('cflinuxfs3') + end + + it 'handles nil description' do + stack.description = nil + warning = StackStateValidator.build_deprecation_warning(stack) + expect(warning).to include('cflinuxfs3') + end + end + + describe 'state behavior matrix' do + let(:active_stack) { Stack.make(state: StackStates::STACK_ACTIVE) } + let(:deprecated_stack) { Stack.make(state: StackStates::STACK_DEPRECATED) } + let(:restricted_stack) { Stack.make(state: StackStates::STACK_RESTRICTED) } + let(:disabled_stack) { Stack.make(state: StackStates::STACK_DISABLED) } + + describe 'new app creation' do + it 'allows ACTIVE without warnings' do + result = StackStateValidator.validate_for_new_app!(active_stack) + expect(result).to be_empty + end + + it 'allows DEPRECATED with warnings' do + result = StackStateValidator.validate_for_new_app!(deprecated_stack) + expect(result).not_to be_empty + end + + it 'rejects RESTRICTED' do + expect { StackStateValidator.validate_for_new_app!(restricted_stack) }.to raise_error(StackStateValidator::RestrictedStackError) + end + + it 'rejects DISABLED' do + expect { StackStateValidator.validate_for_new_app!(disabled_stack) }.to raise_error(StackStateValidator::DisabledStackError) + end + end + + describe 'restaging' do + it 'allows ACTIVE without warnings' do + result = StackStateValidator.validate_for_restaging!(active_stack) + expect(result).to be_empty + end + + it 'allows DEPRECATED with warnings' do + result = StackStateValidator.validate_for_restaging!(deprecated_stack) + expect(result).not_to be_empty + end + + it 'allows RESTRICTED without warnings' do + result = StackStateValidator.validate_for_restaging!(restricted_stack) + expect(result).to be_empty + end + + it 'rejects DISABLED' do + expect { StackStateValidator.validate_for_restaging!(disabled_stack) }.to raise_error(StackStateValidator::DisabledStackError) + end + end + end + end +end From a0d23bfee33905cdf3a243825caf82f1d278602b Mon Sep 17 00:00:00 2001 From: Rashed Kamal Date: Tue, 16 Dec 2025 09:31:53 -0500 Subject: [PATCH 07/14] Stack state validation and integration into app creation and build workflows. Signed-off-by: Rashed Kamal --- app/actions/build_create.rb | 26 ++++ app/models/runtime/build_model.rb | 2 + app/presenters/v3/build_presenter.rb | 7 + errors/v2.yml | 5 + spec/request/apps_spec.rb | 2 + spec/request/builds_spec.rb | 128 ++++++++++++++++++ spec/unit/actions/build_create_spec.rb | 78 +++++++++++ .../presenters/v3/build_presenter_spec.rb | 27 ++++ 8 files changed, 275 insertions(+) diff --git a/app/actions/build_create.rb b/app/actions/build_create.rb index 80c6bd79fa4..1bdde8cc14a 100644 --- a/app/actions/build_create.rb +++ b/app/actions/build_create.rb @@ -41,6 +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) 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 @@ -60,6 +61,7 @@ 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 @@ -179,5 +181,29 @@ def stagers def staging_in_progress! raise StagingInProgress end + + def validate_stack_state!(lifecycle, app) + return [] if lifecycle.type == Lifecycles::DOCKER + + stack = Stack.find(name: lifecycle.staging_stack) + return [] unless stack + + warnings = if first_build_for_app?(app) + StackStateValidator.validate_for_new_app!(stack) + else + StackStateValidator.validate_for_restaging!(stack) + end + warnings.each { |warning| logger.warn(warning) } + warnings + rescue StackStateValidator::DisabledStackError, StackStateValidator::RestrictedStackError => e + raise CloudController::Errors::ApiError.new_from_details( + 'StackValidationFailed', + e.message + ) + end + + def first_build_for_app?(app) + app.builds_dataset.count.zero? + end end end diff --git a/app/models/runtime/build_model.rb b/app/models/runtime/build_model.rb index abc5310866d..8dac1c16ef1 100644 --- a/app/models/runtime/build_model.rb +++ b/app/models/runtime/build_model.rb @@ -17,6 +17,8 @@ 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/presenters/v3/build_presenter.rb b/app/presenters/v3/build_presenter.rb index 3579faf4364..fa4f861afe7 100644 --- a/app/presenters/v3/build_presenter.rb +++ b/app/presenters/v3/build_presenter.rb @@ -30,6 +30,7 @@ def to_hash }, package: { guid: build.package_guid }, droplet: droplet, + warnings: build_warnings, created_by: { guid: build.created_by_user_guid, name: build.created_by_user_name, @@ -61,6 +62,12 @@ 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/errors/v2.yml b/errors/v2.yml index cf4bf28bea2..1f0d12418cb 100644 --- a/errors/v2.yml +++ b/errors/v2.yml @@ -863,6 +863,11 @@ http_code: 404 message: "The stack could not be found: %s" +250004: + name: StackValidationFailed + http_code: 422 + message: "%s" + 260001: name: ServicePlanVisibilityInvalid http_code: 400 diff --git a/spec/request/apps_spec.rb b/spec/request/apps_spec.rb index 063a2452f7a..14d3101c773 100644 --- a/spec/request/apps_spec.rb +++ b/spec/request/apps_spec.rb @@ -1900,6 +1900,7 @@ 'droplet' => { 'guid' => droplet.guid }, + 'warnings' => nil, 'relationships' => { 'app' => { 'data' => { 'guid' => app_model.guid } } }, 'metadata' => { 'labels' => {}, 'annotations' => {} }, 'links' => { @@ -1929,6 +1930,7 @@ 'droplet' => { 'guid' => second_droplet.guid }, + 'warnings' => nil, 'relationships' => { 'app' => { 'data' => { 'guid' => app_model.guid } } }, 'metadata' => { 'labels' => {}, 'annotations' => {} }, 'links' => { diff --git a/spec/request/builds_spec.rb b/spec/request/builds_spec.rb index 0a72800ec9c..17fa8c30d08 100644 --- a/spec/request/builds_spec.rb +++ b/spec/request/builds_spec.rb @@ -92,6 +92,7 @@ 'guid' => package.guid }, 'droplet' => nil, + 'warnings' => nil, 'relationships' => { 'app' => { 'data' => { 'guid' => app_model.guid } } }, 'links' => { 'self' => { @@ -199,6 +200,130 @@ end end end + + context 'when stack is DISABLED' do + let(:disabled_stack) { VCAP::CloudController::Stack.make(name: 'cflinuxfs3', state: 'DISABLED', description: 'cflinuxfs3 stack is now disabled') } + let(:create_request) do + { + lifecycle: { + type: 'buildpack', + data: { + buildpacks: ['https://github.com/myorg/awesome-buildpack'], + stack: disabled_stack.name + } + }, + package: { + guid: package.guid + } + } + end + + it 'returns 422 and does not create the build' do + post '/v3/builds', create_request.to_json, developer_headers + + expect(last_response.status).to eq(422) + expect(parsed_response['errors'].first['detail']).to include('disabled') + expect(parsed_response['errors'].first['detail']).to include('cannot be used for staging new applications') + expect(VCAP::CloudController::BuildModel.count).to eq(0) + end + end + + context 'when stack is RESTRICTED' do + let(:restricted_stack) { VCAP::CloudController::Stack.make(name: 'cflinuxfs3', state: 'RESTRICTED', description: 'No new apps') } + let(:create_request) do + { + lifecycle: { + type: 'buildpack', + data: { + buildpacks: ['http://github.com/myorg/awesome-buildpack'], + stack: restricted_stack.name + } + }, + package: { + guid: package.guid + } + } + end + + context 'first build for app' do + it 'returns 422 and does not create build' do + expect(app_model.builds_dataset.count).to eq(0) + + post '/v3/builds', create_request.to_json, developer_headers + + expect(last_response.status).to eq(422) + expect(parsed_response['errors'].first['detail']).to include('cannot be used for staging new applications') + expect(VCAP::CloudController::BuildModel.count).to eq(0) + end + end + + context 'app has previous builds' do + before do + VCAP::CloudController::BuildModel.make(app: app_model, state: VCAP::CloudController::BuildModel::STAGED_STATE) + end + + it 'returns 201 and creates build' do + expect(app_model.builds_dataset.count).to eq(1) + + post '/v3/builds', create_request.to_json, developer_headers + + expect(last_response.status).to eq(201) + expect(parsed_response['state']).to eq('STAGING') + expect(app_model.builds_dataset.count).to eq(2) + end + end + end + + context 'when stack is DEPRECATED' do + let(:deprecated_stack) { VCAP::CloudController::Stack.make(name: 'cflinuxfs3', state: 'DEPRECATED', description: 'cflinuxfs3 stack is deprecated. Please migrate your application to cflinuxfs4') } + let(:create_request) do + { + lifecycle: { + type: 'buildpack', + data: { + buildpacks: ['http://github.com/myorg/awesome-buildpack'], + stack: deprecated_stack.name + } + }, + package: { + guid: package.guid + } + } + end + + context 'first build for app' do + it 'returns 201 and does not create the build' do + expect(app_model.builds_dataset.count).to eq(0) + + post '/v3/builds', create_request.to_json, developer_headers + + 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 stack is deprecated') + end + end + + context 'app has previous builds' do + before do + VCAP::CloudController::BuildModel.make(app: app_model, state: VCAP::CloudController::BuildModel::STAGED_STATE) + end + + it 'returns 201 and does not create the build' do + expect(app_model.builds_dataset.count).to eq(1) + + post '/v3/builds', create_request.to_json, developer_headers + + 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 stack is deprecated') + expect(app_model.builds_dataset.count).to eq(2) + end + end + end end describe 'GET /v3/builds' do @@ -349,6 +474,7 @@ 'droplet' => { 'guid' => droplet.guid }, + 'warnings' => nil, 'relationships' => { 'app' => { 'data' => { 'guid' => app_model.guid } } }, 'metadata' => { 'labels' => {}, 'annotations' => {} }, 'links' => { @@ -378,6 +504,7 @@ 'droplet' => { 'guid' => second_droplet.guid }, + 'warnings' => nil, 'relationships' => { 'app' => { 'data' => { 'guid' => app_model.guid } } }, 'metadata' => { 'labels' => {}, 'annotations' => {} }, 'links' => { @@ -480,6 +607,7 @@ 'droplet' => { 'guid' => droplet.guid }, + 'warnings' => nil, 'metadata' => { 'labels' => {}, 'annotations' => {} }, 'relationships' => { 'app' => { 'data' => { 'guid' => app_model.guid } } }, 'links' => { diff --git a/spec/unit/actions/build_create_spec.rb b/spec/unit/actions/build_create_spec.rb index 8b38b4c287c..d0969a88bfb 100644 --- a/spec/unit/actions/build_create_spec.rb +++ b/spec/unit/actions/build_create_spec.rb @@ -452,6 +452,84 @@ module VCAP::CloudController end end + context 'when stack is DISABLED' do + let(:disabled_stack) { Stack.make(name: 'cflinuxfs3', state: 'DISABLED', description: 'Migrate to cflinuxfs4') } + let(:lifecycle_data) do + { + stack: disabled_stack.name, + buildpacks: [buildpack_git_url] + } + end + + it 'raises StackValidationFailed error for staging' do + expect do + action.create_and_stage(package:, lifecycle:) + end.to raise_error(CloudController::Errors::ApiError) do |error| + expect(error.name).to eq('StackValidationFailed') + expect(error.message).to include('disabled') + expect(error.message).to include('cannot be used for staging new applications') + end + end + + it 'does not create any DB records' do + expect do + action.create_and_stage(package:, lifecycle:) + rescue StandardError + nil + end.not_to(change { [BuildModel.count, BuildpackLifecycleDataModel.count, AppUsageEvent.count, Event.count] }) + end + end + + context 'when stack is RESTRICTED' do + let(:restricted_stack) { Stack.make(name: 'cflinuxfs3', state: 'RESTRICTED', description: 'No new apps') } + let(:lifecycle_data) do + { + stack: restricted_stack.name, + buildpacks: [buildpack_git_url] + } + end + + context 'build for new app' do + it 'raises StackValidationFailed error' do + expect(app.builds_dataset.count).to eq(0) + expect do + action.create_and_stage(package:, lifecycle:) + end.to raise_error(CloudController::Errors::ApiError) do |error| + expect(error.name).to eq('StackValidationFailed') + expect(error.message).to include('annot be used for staging new applications') + end + end + + it 'does not create any DB records' do + expect do + action.create_and_stage(package:, lifecycle:) + rescue StandardError + nil + end.not_to(change { [BuildModel.count, BuildpackLifecycleDataModel.count, AppUsageEvent.count, Event.count] }) + end + end + + context 'app has previous builds' do + before do + BuildModel.make(app: app, state: BuildModel::STAGED_STATE) + end + + it 'allows restaging' do + expect(app.builds_dataset.count).to eq(1) + expect do + action.create_and_stage(package:, lifecycle:) + end.not_to raise_error + end + + it 'creates build successfully' do + build = action.create_and_stage(package:, lifecycle:) + expect(build.id).not_to be_nil + expect(build.state).to eq(BuildModel::STAGING_STATE) + expect(app.builds_dataset.count).to eq(2) + end + end + end + context 'when there is already a staging in progress for the app' do it 'raises a StagingInProgress exception' do BuildModel.make(state: BuildModel::STAGING_STATE, app: app) diff --git a/spec/unit/presenters/v3/build_presenter_spec.rb b/spec/unit/presenters/v3/build_presenter_spec.rb index e47de781c56..658103e1e8b 100644 --- a/spec/unit/presenters/v3/build_presenter_spec.rb +++ b/spec/unit/presenters/v3/build_presenter_spec.rb @@ -158,6 +158,33 @@ module VCAP::CloudController::Presenters::V3 expect(result[:package][:guid]).to eq(@package_guid) end end + + context 'when stack has warnings' do + before do + build.instance_variable_set(:@stack_warnings, ['Stack cflinuxfs3 is deprecated. EOL Dec 2025']) + end + + it 'includes warnings in response' do + expect(result[:warnings]).to be_present + expect(result[:warnings]).to eq([{ detail: 'Stack cflinuxfs3 is deprecated. EOL Dec 2025' }]) + end + end + + context 'when stack has no warnings' do + before do + build.instance_variable_set(:@stack_warnings, []) + end + + it 'returns nil for warnings' do + expect(result[:warnings]).to be_nil + end + end + + context 'when stack warnings is nil' do + it 'returns nil for warnings' do + expect(result[:warnings]).to be_nil + end + end end end end From 91b49ce82ca44059ae3e219bef3abca23e49ee8c Mon Sep 17 00:00:00 2001 From: Rashed Kamal Date: Wed, 17 Dec 2025 20:01:48 -0500 Subject: [PATCH 08/14] V2 API stack validation and tests Signed-off-by: Rashed Kamal --- app/actions/v2/app_stage.rb | 6 + app/actions/v2/app_update.rb | 7 +- app/controllers/runtime/apps_controller.rb | 1 + .../runtime/restages_controller.rb | 5 +- spec/request/v2/apps_spec.rb | 184 ++++++++++++++++++ spec/unit/actions/v2/app_stage_spec.rb | 94 +++++++++ spec/unit/actions/v2/app_update_spec.rb | 6 +- .../runtime/apps_controller_spec.rb | 2 +- .../runtime/restages_controller_spec.rb | 2 +- 9 files changed, 300 insertions(+), 7 deletions(-) diff --git a/app/actions/v2/app_stage.rb b/app/actions/v2/app_stage.rb index 5e30f4b5d3b..968aedfecbf 100644 --- a/app/actions/v2/app_stage.rb +++ b/app/actions/v2/app_stage.rb @@ -1,8 +1,11 @@ module VCAP::CloudController module V2 class AppStage + attr_reader :warnings + def initialize(stagers:) @stagers = stagers + @warnings = [] end def stage(process) @@ -25,6 +28,9 @@ def stage(process) lifecycle: lifecycle, start_after_staging: true ) + + @warnings = build.instance_variable_get(:@stack_warnings) || [] + TelemetryLogger.v2_emit( 'create-build', { diff --git a/app/actions/v2/app_update.rb b/app/actions/v2/app_update.rb index e07279f4f4e..c5b32b756cb 100644 --- a/app/actions/v2/app_update.rb +++ b/app/actions/v2/app_update.rb @@ -4,9 +4,12 @@ module VCAP::CloudController module V2 class AppUpdate + attr_reader :warnings + def initialize(access_validator:, stagers:) @access_validator = access_validator @stagers = stagers + @warnings = [] end def update(app, process, request_attrs) @@ -116,7 +119,9 @@ def prepare_to_stage(app) end def stage(process) - V2::AppStage.new(stagers: @stagers).stage(process) + app_stage = V2::AppStage.new(stagers: @stagers) + app_stage.stage(process) + @warnings = app_stage.warnings end def start_or_stop(app, request_attrs) diff --git a/app/controllers/runtime/apps_controller.rb b/app/controllers/runtime/apps_controller.rb index 5f3a514389c..22157ecd01b 100644 --- a/app/controllers/runtime/apps_controller.rb +++ b/app/controllers/runtime/apps_controller.rb @@ -294,6 +294,7 @@ def update(guid) updater = V2::AppUpdate.new(access_validator: self, stagers: @stagers) updater.update(app, process, request_attrs) + updater.warnings.each { |warning| add_warning(warning) } after_update(process) diff --git a/app/controllers/runtime/restages_controller.rb b/app/controllers/runtime/restages_controller.rb index 189d479cafd..b73e3920a9c 100644 --- a/app/controllers/runtime/restages_controller.rb +++ b/app/controllers/runtime/restages_controller.rb @@ -35,7 +35,10 @@ def restage(guid) process.app.update(droplet_guid: nil) AppStart.start_without_event(process.app, create_revision: false) end - V2::AppStage.new(stagers: @stagers).stage(process) + # V2::AppStage.new(stagers: @stagers).stage(process) + app_stage = V2::AppStage.new(stagers: @stagers) + app_stage.stage(process) + app_stage.warnings.each { |warning| add_warning(warning) } @app_event_repository.record_app_restage(process, UserAuditInfo.from_context(SecurityContext)) diff --git a/spec/request/v2/apps_spec.rb b/spec/request/v2/apps_spec.rb index 36a1536a3a3..056d73e134f 100644 --- a/spec/request/v2/apps_spec.rb +++ b/spec/request/v2/apps_spec.rb @@ -926,6 +926,58 @@ end end + context 'stack state validation on app update with staging' do + let(:stager) { instance_double(VCAP::CloudController::Diego::Stager, stage: nil) } + + before do + allow_any_instance_of(VCAP::CloudController::Stagers).to receive(:validate_process) + allow_any_instance_of(VCAP::CloudController::Stagers).to receive(:stager_for_build).and_return(stager) + VCAP::CloudController::Buildpack.make + VCAP::CloudController::PackageModel.make(app: process.app, state: VCAP::CloudController::PackageModel::READY_STATE) + end + + context 'when stack is DISABLED' do + let(:disabled_stack) { VCAP::CloudController::Stack.make(name: 'cflinuxfs2', state: 'DISABLED', description: 'Migrate to cflinuxfs4') } + let(:update_params) { Oj.dump({ state: 'STARTED' }) } + + before do + process.app.buildpack_lifecycle_data.update(stack: disabled_stack.name) + process.update(state: 'STOPPED') + end + + it 'returns 422 with stack validation error when starting app' do + put "/v2/apps/#{process.guid}", update_params, headers_for(user) + + expect(last_response.status).to eq(422) + parsed_response = Oj.load(last_response.body) + expect(parsed_response['error_code']).to eq('CF-StackValidationFailed') + expect(parsed_response['description']).to include('disabled') + expect(parsed_response['description']).to include('cflinuxfs2') + end + end + + context 'when stack is DEPRECATED' do + let(:deprecated_stack) { VCAP::CloudController::Stack.make(name: 'cflinuxfs3', state: 'DEPRECATED', description: 'EOL Dec 2025') } + let(:update_params) { Oj.dump({ state: 'STARTED' }) } + + before do + process.app.buildpack_lifecycle_data.update(stack: deprecated_stack.name) + process.app.update(droplet_guid: nil) + process.update(state: 'STOPPED') + end + + it 'allows starting app with deprecation warning' do + put "/v2/apps/#{process.guid}", update_params, headers_for(user) + + expect(last_response.status).to eq(201) + expect(last_response.headers['X-Cf-Warnings']).to be_present + decoded_warning = CGI.unescape(last_response.headers['X-Cf-Warnings']) + expect(decoded_warning).to include('deprecated') + expect(decoded_warning).to include('EOL Dec 2025') + end + end + end + context 'when process memory is being decreased and the new memory allocation is lower than memory of associated sidecars' do let!(:process) do VCAP::CloudController::ProcessModelFactory.make( @@ -1563,6 +1615,138 @@ def make_actual_lrp(instance_guid:, index:, state:, error:, since:) end end end + + context 'stack state validation' do + let(:process) { VCAP::CloudController::ProcessModelFactory.make(name: 'maria', space: space, diego: true) } + let(:stager) { instance_double(VCAP::CloudController::Diego::Stager, stage: nil) } + + before do + allow_any_instance_of(VCAP::CloudController::Stagers).to receive(:validate_process) + allow_any_instance_of(VCAP::CloudController::Stagers).to receive(:stager_for_build).and_return(stager) + VCAP::CloudController::Buildpack.make + end + + context 'when stack is DISABLED' do + let(:disabled_stack) { VCAP::CloudController::Stack.make(name: 'cflinuxfs2', state: 'DISABLED', description: 'Migrate to cflinuxfs4') } + + before do + process.app.buildpack_lifecycle_data.update(stack: disabled_stack.name) + end + + it 'returns 422 with stack validation error' do + post "/v2/apps/#{process.guid}/restage", nil, headers_for(user) + + expect(last_response.status).to eq(422) + parsed_response = Oj.load(last_response.body) + expect(parsed_response['error_code']).to eq('CF-StackValidationFailed') + expect(parsed_response['description']).to include('disabled') + expect(parsed_response['description']).to include('cannot be used for staging') + expect(parsed_response['description']).to include('cflinuxfs2') + expect(parsed_response['description']).to include('Migrate to cflinuxfs4') + end + + it 'does not expose stack state field in error response' do + post "/v2/apps/#{process.guid}/restage", nil, headers_for(user) + + parsed_response = Oj.load(last_response.body) + expect(parsed_response['entity']).to be_nil + expect(parsed_response['error_code']).to eq('CF-StackValidationFailed') + end + end + + context 'when stack is RESTRICTED' do + let(:restricted_stack) { VCAP::CloudController::Stack.make(name: 'cflinuxfs3', state: 'RESTRICTED', description: 'No new apps') } + + before do + process.app.buildpack_lifecycle_data.update(stack: restricted_stack.name) + end + + context 'for first build' do + before do + process.app.builds_dataset.destroy + end + + it 'returns 422 with stack validation error' do + expect(process.app.builds_dataset.count).to eq(0) + + post "/v2/apps/#{process.guid}/restage", nil, headers_for(user) + + expect(last_response.status).to eq(422) + parsed_response = Oj.load(last_response.body) + expect(parsed_response['error_code']).to eq('CF-StackValidationFailed') + expect(parsed_response['description']).to include('cannot be used for staging new applications') + expect(parsed_response['description']).to include('cflinuxfs3') + end + end + + context 'for restaging existing app' do + before do + VCAP::CloudController::BuildModel.make(app: process.app, state: 'STAGED') + end + + it 'allows restaging without errors' do + post "/v2/apps/#{process.guid}/restage", nil, headers_for(user) + + expect(last_response.status).to eq(201) + end + + it 'does not include warnings header' do + post "/v2/apps/#{process.guid}/restage", nil, headers_for(user) + + expect(last_response.headers['X-Cf-Warnings']).to be_nil + end + end + end + + context 'when stack is DEPRECATED' do + let(:deprecated_stack) { VCAP::CloudController::Stack.make(name: 'cflinuxfs3', state: 'DEPRECATED', description: 'EOL Dec 2025') } + + before do + process.app.buildpack_lifecycle_data.update(stack: deprecated_stack.name) + end + + it 'allows restaging with success' do + post "/v2/apps/#{process.guid}/restage", nil, headers_for(user) + + expect(last_response.status).to eq(201) + end + + it 'includes deprecation warning in X-Cf-Warnings header' do + post "/v2/apps/#{process.guid}/restage", nil, headers_for(user) + + expect(last_response.headers['X-Cf-Warnings']).to be_present + decoded_warning = CGI.unescape(last_response.headers['X-Cf-Warnings']) + expect(decoded_warning).to include('deprecated') + expect(decoded_warning).to include('cflinuxfs3') + expect(decoded_warning).to include('EOL Dec 2025') + end + + it 'does not expose stack state field in response body' do + post "/v2/apps/#{process.guid}/restage", nil, headers_for(user) + + parsed_response = Oj.load(last_response.body) + # The response includes process 'state' (STARTED/STOPPED) which is different from stack 'state' + # We verify stack state is not exposed by checking it's not in the stack-related fields + expect(parsed_response['entity']['stack_guid']).to be_present + expect(parsed_response.to_s).not_to match(/DEPRECATED/) + end + end + + context 'when stack is ACTIVE' do + let(:active_stack) { VCAP::CloudController::Stack.make(name: 'cflinuxfs5', state: 'ACTIVE') } + + before do + process.app.buildpack_lifecycle_data.update(stack: active_stack.name) + end + + it 'allows restaging without warnings' do + post "/v2/apps/#{process.guid}/restage", nil, headers_for(user) + + expect(last_response.status).to eq(201) + expect(last_response.headers['X-Cf-Warnings']).to be_nil + end + end + end end describe 'PUT /v2/apps/:guid/bits' do diff --git a/spec/unit/actions/v2/app_stage_spec.rb b/spec/unit/actions/v2/app_stage_spec.rb index 023584d0d5c..0f13b042460 100644 --- a/spec/unit/actions/v2/app_stage_spec.rb +++ b/spec/unit/actions/v2/app_stage_spec.rb @@ -215,6 +215,100 @@ module V2 end end end + + context 'stack state warnings' do + let(:process) { ProcessModelFactory.make } + let(:user_audit_info) do + UserAuditInfo.new(user_email: 'test@example.com', user_name: 'test-user', user_guid: 'test-guid') + end + + before do + allow(UserAuditInfo).to receive(:from_context).and_return(user_audit_info) + allow(BuildCreate).to receive(:new).and_call_original + allow_any_instance_of(Diego::Stager).to receive(:stage).and_return 'staging-complete' + end + + context 'when stack is DEPRECATED' do + let(:deprecated_stack) { Stack.make(name: 'cflinuxfs3', state: 'DEPRECATED', description: 'EOL Dec 2025') } + + before do + process.app.buildpack_lifecycle_data.update(stack: deprecated_stack.name) + end + + it 'captures warnings from BuildCreate' do + action.stage(process) + + expect(action.warnings).not_to be_empty + expect(action.warnings.first).to include('deprecated') + expect(action.warnings.first).to include('cflinuxfs3') + expect(action.warnings.first).to include('EOL Dec 2025') + end + end + + context 'when stack is ACTIVE' do + let(:active_stack) { Stack.make(name: 'cflinuxfs5', state: 'ACTIVE') } + + before do + process.app.buildpack_lifecycle_data.update(stack: active_stack.name) + end + + it 'has no warnings' do + action.stage(process) + + expect(action.warnings).to be_empty + end + end + + context 'when stack is DISABLED' do + let(:disabled_stack) { Stack.make(name: 'cflinuxfs2', state: 'DISABLED', description: 'Migrate to cflinuxfs4') } + + before do + process.app.buildpack_lifecycle_data.update(stack: disabled_stack.name) + end + + it 'raises StackValidationFailed error' do + expect { action.stage(process) }.to raise_error(CloudController::Errors::ApiError) do |error| + expect(error.name).to eq('StackValidationFailed') + expect(error.message).to include('disabled') + expect(error.message).to include('cannot be used for staging') + end + end + end + + context 'when stack is RESTRICTED' do + let(:restricted_stack) { Stack.make(name: 'cflinuxfs3-restricted', state: 'RESTRICTED', description: 'No new apps') } + + before do + process.app.buildpack_lifecycle_data.update(stack: restricted_stack.name) + end + + context 'for first build' do + before do + process.app.builds_dataset.destroy + end + + it 'raises StackValidationFailed error' do + expect(process.app.builds_dataset.count).to eq(0) + + expect { action.stage(process) }.to raise_error(CloudController::Errors::ApiError) do |error| + expect(error.name).to eq('StackValidationFailed') + expect(error.message).to include('cannot be used for staging new applications') + end + end + end + + context 'for restaging existing app' do + before do + BuildModel.make(app: process.app, state: 'STAGED') + end + + it 'allows staging without warnings' do + expect { action.stage(process) }.not_to raise_error + expect(action.warnings).to be_empty + end + end + end + end end end end diff --git a/spec/unit/actions/v2/app_update_spec.rb b/spec/unit/actions/v2/app_update_spec.rb index 7be122232af..778e1bd62fd 100644 --- a/spec/unit/actions/v2/app_update_spec.rb +++ b/spec/unit/actions/v2/app_update_spec.rb @@ -432,7 +432,7 @@ module VCAP::CloudController describe 'updating docker_image' do let(:process) { ProcessModelFactory.make(app: AppModel.make(:docker), docker_image: 'repo/original-image') } let!(:original_package) { process.latest_package } - let(:app_stage) { instance_double(V2::AppStage, stage: nil) } + let(:app_stage) { instance_double(V2::AppStage, stage: nil, warnings: []) } before do FeatureFlag.create(name: 'diego_docker', enabled: true) @@ -507,7 +507,7 @@ module VCAP::CloudController describe 'updating docker_credentials' do let(:process) { ProcessModelFactory.make(app: AppModel.make(:docker), docker_image: 'repo/original-image') } let!(:original_package) { process.latest_package } - let(:app_stage) { instance_double(V2::AppStage, stage: nil) } + let(:app_stage) { instance_double(V2::AppStage, stage: nil, warnings: []) } before do FeatureFlag.create(name: 'diego_docker', enabled: true) @@ -545,7 +545,7 @@ module VCAP::CloudController end describe 'staging' do - let(:app_stage) { instance_double(V2::AppStage, stage: nil) } + let(:app_stage) { instance_double(V2::AppStage, stage: nil, warnings: []) } let(:process) { ProcessModelFactory.make(state: 'STARTED') } let(:app) { process.app } diff --git a/spec/unit/controllers/runtime/apps_controller_spec.rb b/spec/unit/controllers/runtime/apps_controller_spec.rb index 300f736f34b..d3231f87a60 100644 --- a/spec/unit/controllers/runtime/apps_controller_spec.rb +++ b/spec/unit/controllers/runtime/apps_controller_spec.rb @@ -1094,7 +1094,7 @@ def update_app end describe 'staging' do - let(:app_stage) { instance_double(V2::AppStage, stage: nil) } + let(:app_stage) { instance_double(V2::AppStage, stage: nil, warnings: []) } let(:process) { ProcessModelFactory.make } before do diff --git a/spec/unit/controllers/runtime/restages_controller_spec.rb b/spec/unit/controllers/runtime/restages_controller_spec.rb index 854d3d6aaa9..12ca41e2f93 100644 --- a/spec/unit/controllers/runtime/restages_controller_spec.rb +++ b/spec/unit/controllers/runtime/restages_controller_spec.rb @@ -11,7 +11,7 @@ module VCAP::CloudController describe 'POST /v2/apps/:id/restage' do subject(:restage_request) { post "/v2/apps/#{process.app.guid}/restage", {} } let!(:process) { ProcessModelFactory.make } - let(:app_stage) { instance_double(V2::AppStage, stage: nil) } + let(:app_stage) { instance_double(V2::AppStage, stage: nil, warnings: []) } before do allow(V2::AppStage).to receive(:new).and_return(app_stage) From a3605f9b967b134ee690fd749b660557a6f1ea75 Mon Sep 17 00:00:00 2001 From: Rashed Kamal Date: Wed, 7 Jan 2026 16:41:28 -0500 Subject: [PATCH 09/14] Add deprecation warning in build create Signed-off-by: Rashed Kamal --- app/controllers/v3/builds_controller.rb | 2 ++ spec/request/builds_spec.rb | 20 ++++++++++++++++++++ 2 files changed, 22 insertions(+) diff --git a/app/controllers/v3/builds_controller.rb b/app/controllers/v3/builds_controller.rb index 239bd1371ca..b8598285828 100644 --- a/app/controllers/v3/builds_controller.rb +++ b/app/controllers/v3/builds_controller.rb @@ -65,6 +65,8 @@ def create } ) + add_warning_headers(build.stack_warnings) if build.stack_warnings&.any? + render status: :created, json: Presenters::V3::BuildPresenter.new(build) rescue BuildCreate::InvalidPackage => e bad_request!(e.message) diff --git a/spec/request/builds_spec.rb b/spec/request/builds_spec.rb index 17fa8c30d08..ccfcb40be26 100644 --- a/spec/request/builds_spec.rb +++ b/spec/request/builds_spec.rb @@ -303,6 +303,16 @@ expect(parsed_response['warnings'][0]['detail']).to include('deprecated') expect(parsed_response['warnings'][0]['detail']).to include('cflinuxfs3 stack is deprecated') end + + it 'includes warming in response headers' do + post '/v3/builds', create_request.to_json, developer_headers + + expect(last_response.status).to eq(201) + expect(last_response.headers['X-Cf-Warnings']).to be_present + warning = CGI.unescape(last_response.headers['X-Cf-Warnings']) + expect(warning).to include('deprecated') + expect(warning).to include('cflinuxfs3 stack is deprecated') + end end context 'app has previous builds' do @@ -322,6 +332,16 @@ expect(parsed_response['warnings'][0]['detail']).to include('cflinuxfs3 stack is deprecated') expect(app_model.builds_dataset.count).to eq(2) end + + it 'includes warming in response headers' do + post '/v3/builds', create_request.to_json, developer_headers + + expect(last_response.status).to eq(201) + expect(last_response.headers['X-Cf-Warnings']).to be_present + warning = CGI.unescape(last_response.headers['X-Cf-Warnings']) + expect(warning).to include('deprecated') + expect(warning).to include('cflinuxfs3 stack is deprecated') + end end end end From 243cba7983aee8f3d16c4723321c1c982bdaeded Mon Sep 17 00:00:00 2001 From: Rashed Kamal Date: Thu, 8 Jan 2026 15:34:35 -0500 Subject: [PATCH 10/14] Update stck state validation error and warning messages Signed-off-by: Rashed Kamal --- lib/cloud_controller/stack_state_validator.rb | 18 +++++--- spec/request/builds_spec.rb | 28 ++++++------ spec/request/v2/apps_spec.rb | 14 +++--- spec/unit/actions/build_create_spec.rb | 7 +-- spec/unit/actions/v2/app_stage_spec.rb | 8 ++-- .../stack_state_validator_spec.rb | 43 +++++-------------- 6 files changed, 51 insertions(+), 67 deletions(-) diff --git a/lib/cloud_controller/stack_state_validator.rb b/lib/cloud_controller/stack_state_validator.rb index d8f874e6c2f..3408f3b5295 100644 --- a/lib/cloud_controller/stack_state_validator.rb +++ b/lib/cloud_controller/stack_state_validator.rb @@ -6,23 +6,27 @@ class RestrictedStackError < StackValidationError; end def self.validate_for_new_app!(stack) return [] if stack.active? - raise DisabledStackError.new("Stack '#{stack.name}' is disabled and cannot be used for staging new applications. #{stack.description}") if stack.disabled? + raise DisabledStackError.new(build_stack_error(stack, StackStates::STACK_DISABLED)) if stack.disabled? - raise RestrictedStackError.new("Stack '#{stack.name}' is restricted and cannot be used for staging new applications. #{stack.description}") if stack.restricted? + raise RestrictedStackError.new(build_stack_error(stack, StackStates::STACK_RESTRICTED)) if stack.restricted? - stack.deprecated? ? [build_deprecation_warning(stack)] : [] + stack.deprecated? ? [build_deprecation_warning(stack, StackStates::STACK_DEPRECATED)] : [] end def self.validate_for_restaging!(stack) return [] if stack.active? || stack.restricted? - raise DisabledStackError.new("Stack '#{stack.name}' is disabled and cannot be used for staging new applications. #{stack.description}") if stack.disabled? + raise DisabledStackError.new(build_stack_error(stack, StackStates::STACK_DISABLED)) if stack.disabled? - stack.deprecated? ? [build_deprecation_warning(stack)] : [] + stack.deprecated? ? [build_deprecation_warning(stack, StackStates::STACK_DEPRECATED)] : [] end - def self.build_deprecation_warning(stack) - "Stack '#{stack.name}' is deprecated and will be removed in the future. #{stack.description}" + def self.build_stack_error(stack, state) + "ERROR: Staging failed. The stack '#{stack.name}' is '#{state}' and cannot be used for staging." + end + + def self.build_deprecation_warning(stack, state) + "WARNING: The stack '#{stack.name}' is '#{state}' and will be removed in the future." end end end diff --git a/spec/request/builds_spec.rb b/spec/request/builds_spec.rb index ccfcb40be26..077ea1de486 100644 --- a/spec/request/builds_spec.rb +++ b/spec/request/builds_spec.rb @@ -222,8 +222,8 @@ post '/v3/builds', create_request.to_json, developer_headers expect(last_response.status).to eq(422) - expect(parsed_response['errors'].first['detail']).to include('disabled') - expect(parsed_response['errors'].first['detail']).to include('cannot be used for staging new applications') + expect(parsed_response['errors'].first['detail']).to include('DISABLED') + expect(parsed_response['errors'].first['detail']).to include('cannot be used for staging') expect(VCAP::CloudController::BuildModel.count).to eq(0) end end @@ -252,7 +252,8 @@ post '/v3/builds', create_request.to_json, developer_headers expect(last_response.status).to eq(422) - expect(parsed_response['errors'].first['detail']).to include('cannot be used for staging new applications') + expect(parsed_response['errors'].first['detail']).to include('cannot be used for staging') + expect(parsed_response['errors'].first['detail']).to include('RESTRICTED') expect(VCAP::CloudController::BuildModel.count).to eq(0) end end @@ -300,18 +301,19 @@ 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 stack is deprecated') + 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') end - it 'includes warming in response headers' do + it 'includes warning in response headers' do post '/v3/builds', create_request.to_json, developer_headers expect(last_response.status).to eq(201) expect(last_response.headers['X-Cf-Warnings']).to be_present warning = CGI.unescape(last_response.headers['X-Cf-Warnings']) - expect(warning).to include('deprecated') - expect(warning).to include('cflinuxfs3 stack is deprecated') + expect(warning).to include('DEPRECATED') + expect(warning).to include('cflinuxfs3') end end @@ -328,19 +330,19 @@ 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 stack is deprecated') + expect(parsed_response['warnings'][0]['detail']).to include('DEPRECATED') + expect(parsed_response['warnings'][0]['detail']).to include('cflinuxfs3') expect(app_model.builds_dataset.count).to eq(2) end - it 'includes warming in response headers' do + it 'includes warning in response headers' do post '/v3/builds', create_request.to_json, developer_headers expect(last_response.status).to eq(201) expect(last_response.headers['X-Cf-Warnings']).to be_present warning = CGI.unescape(last_response.headers['X-Cf-Warnings']) - expect(warning).to include('deprecated') - expect(warning).to include('cflinuxfs3 stack is deprecated') + expect(warning).to include('DEPRECATED') + expect(warning).to include('cflinuxfs3') end end end diff --git a/spec/request/v2/apps_spec.rb b/spec/request/v2/apps_spec.rb index 056d73e134f..a951fb30a18 100644 --- a/spec/request/v2/apps_spec.rb +++ b/spec/request/v2/apps_spec.rb @@ -951,7 +951,7 @@ expect(last_response.status).to eq(422) parsed_response = Oj.load(last_response.body) expect(parsed_response['error_code']).to eq('CF-StackValidationFailed') - expect(parsed_response['description']).to include('disabled') + expect(parsed_response['description']).to include('DISABLED') expect(parsed_response['description']).to include('cflinuxfs2') end end @@ -972,8 +972,8 @@ expect(last_response.status).to eq(201) expect(last_response.headers['X-Cf-Warnings']).to be_present decoded_warning = CGI.unescape(last_response.headers['X-Cf-Warnings']) - expect(decoded_warning).to include('deprecated') - expect(decoded_warning).to include('EOL Dec 2025') + expect(decoded_warning).to include('DEPRECATED') + expect(decoded_warning).to include('cflinuxfs3') end end end @@ -1639,10 +1639,9 @@ def make_actual_lrp(instance_guid:, index:, state:, error:, since:) expect(last_response.status).to eq(422) parsed_response = Oj.load(last_response.body) expect(parsed_response['error_code']).to eq('CF-StackValidationFailed') - expect(parsed_response['description']).to include('disabled') + expect(parsed_response['description']).to include('DISABLED') expect(parsed_response['description']).to include('cannot be used for staging') expect(parsed_response['description']).to include('cflinuxfs2') - expect(parsed_response['description']).to include('Migrate to cflinuxfs4') end it 'does not expose stack state field in error response' do @@ -1674,7 +1673,7 @@ def make_actual_lrp(instance_guid:, index:, state:, error:, since:) expect(last_response.status).to eq(422) parsed_response = Oj.load(last_response.body) expect(parsed_response['error_code']).to eq('CF-StackValidationFailed') - expect(parsed_response['description']).to include('cannot be used for staging new applications') + expect(parsed_response['description']).to include('cannot be used for staging') expect(parsed_response['description']).to include('cflinuxfs3') end end @@ -1716,9 +1715,8 @@ def make_actual_lrp(instance_guid:, index:, state:, error:, since:) expect(last_response.headers['X-Cf-Warnings']).to be_present decoded_warning = CGI.unescape(last_response.headers['X-Cf-Warnings']) - expect(decoded_warning).to include('deprecated') + expect(decoded_warning).to include('DEPRECATED') expect(decoded_warning).to include('cflinuxfs3') - expect(decoded_warning).to include('EOL Dec 2025') end it 'does not expose stack state field in response body' do diff --git a/spec/unit/actions/build_create_spec.rb b/spec/unit/actions/build_create_spec.rb index d0969a88bfb..58eca6992f1 100644 --- a/spec/unit/actions/build_create_spec.rb +++ b/spec/unit/actions/build_create_spec.rb @@ -466,8 +466,8 @@ module VCAP::CloudController action.create_and_stage(package:, lifecycle:) end.to raise_error(CloudController::Errors::ApiError) do |error| expect(error.name).to eq('StackValidationFailed') - expect(error.message).to include('disabled') - expect(error.message).to include('cannot be used for staging new applications') + expect(error.message).to include('DISABLED') + expect(error.message).to include('cannot be used for staging') end end @@ -496,7 +496,8 @@ module VCAP::CloudController action.create_and_stage(package:, lifecycle:) end.to raise_error(CloudController::Errors::ApiError) do |error| expect(error.name).to eq('StackValidationFailed') - expect(error.message).to include('annot be used for staging new applications') + expect(error.message).to include('annot be used for staging') + expect(error.message).to include('RESTRICTED') end end diff --git a/spec/unit/actions/v2/app_stage_spec.rb b/spec/unit/actions/v2/app_stage_spec.rb index 0f13b042460..0e819b1fc4e 100644 --- a/spec/unit/actions/v2/app_stage_spec.rb +++ b/spec/unit/actions/v2/app_stage_spec.rb @@ -239,9 +239,8 @@ module V2 action.stage(process) expect(action.warnings).not_to be_empty - expect(action.warnings.first).to include('deprecated') + expect(action.warnings.first).to include('DEPRECATED') expect(action.warnings.first).to include('cflinuxfs3') - expect(action.warnings.first).to include('EOL Dec 2025') end end @@ -269,7 +268,7 @@ module V2 it 'raises StackValidationFailed error' do expect { action.stage(process) }.to raise_error(CloudController::Errors::ApiError) do |error| expect(error.name).to eq('StackValidationFailed') - expect(error.message).to include('disabled') + expect(error.message).to include('DISABLED') expect(error.message).to include('cannot be used for staging') end end @@ -292,7 +291,8 @@ module V2 expect { action.stage(process) }.to raise_error(CloudController::Errors::ApiError) do |error| expect(error.name).to eq('StackValidationFailed') - expect(error.message).to include('cannot be used for staging new applications') + expect(error.message).to include('cannot be used for staging') + expect(error.message).to include('RESTRICTED') end end end diff --git a/spec/unit/lib/cloud_controller/stack_state_validator_spec.rb b/spec/unit/lib/cloud_controller/stack_state_validator_spec.rb index f6222220576..7cafc67bfc8 100644 --- a/spec/unit/lib/cloud_controller/stack_state_validator_spec.rb +++ b/spec/unit/lib/cloud_controller/stack_state_validator_spec.rb @@ -23,15 +23,14 @@ module VCAP::CloudController result = StackStateValidator.validate_for_new_app!(stack) expect(result).to be_an(Array) expect(result.size).to eq(1) - expect(result.first).to include("Stack '#{stack.name}' is deprecated and will be removed in the future. #{stack.description}") + expect(result.first).to include("stack '#{stack.name}' is '#{StackStates::STACK_DEPRECATED}' and will be removed in the future") end it 'returns a warning message with stack name' do result = StackStateValidator.validate_for_new_app!(stack) warning = result.first expect(warning).to include(stack.name) - expect(warning).to include(stack.description) - expect(warning).to include('deprecated') + expect(warning).to include('DEPRECATED') end it 'does not raise an error' do @@ -45,7 +44,7 @@ module VCAP::CloudController it 'raise RestrictedStackError' do expect do StackStateValidator.validate_for_new_app!(stack) - end.to raise_error(StackStateValidator::RestrictedStackError, /Stack '#{stack.name}' is restricted and cannot be used for staging new applications./) + end.to raise_error(StackStateValidator::RestrictedStackError, /The stack '#{stack.name}' is '#{StackStates::STACK_RESTRICTED}' and cannot be used for staging./) end it 'includes stack name in error message' do @@ -54,12 +53,6 @@ module VCAP::CloudController end.to raise_error(StackStateValidator::RestrictedStackError, /#{stack.name}/) end - it 'includes stack description in error message' do - expect do - StackStateValidator.validate_for_new_app!(stack) - end.to raise_error(StackStateValidator::RestrictedStackError, /#{stack.description}/) - end - it 'raises RestrictedStackError which is a StackStateValidator::Error' do expect do StackStateValidator.validate_for_new_app!(stack) @@ -73,7 +66,7 @@ module VCAP::CloudController it 'returns a disabled error message' do expect do StackStateValidator.validate_for_new_app!(stack) - end.to raise_error(StackStateValidator::DisabledStackError, /Stack '#{stack.name}' is disabled and cannot be used for staging new applications./) + end.to raise_error(StackStateValidator::DisabledStackError, /The stack '#{stack.name}' is '#{StackStates::STACK_DISABLED}' and cannot be used for staging./) end it 'includes stack name in error message' do @@ -81,12 +74,6 @@ module VCAP::CloudController StackStateValidator.validate_for_new_app!(stack) end.to raise_error(StackStateValidator::DisabledStackError, /#{stack.name}/) end - - it 'includes stack description in error message' do - expect do - StackStateValidator.validate_for_new_app!(stack) - end.to raise_error(StackStateValidator::DisabledStackError, /#{stack.description}/) - end end end @@ -111,15 +98,14 @@ module VCAP::CloudController result = StackStateValidator.validate_for_restaging!(stack) expect(result).to be_an(Array) expect(result.size).to eq(1) - expect(result.first).to include("Stack '#{stack.name}' is deprecated and will be removed in the future. #{stack.description}") + expect(result.first).to include("stack '#{stack.name}' is '#{StackStates::STACK_DEPRECATED}' and will be removed in the future") end it 'returns a warning message with stack name' do result = StackStateValidator.validate_for_restaging!(stack) warning = result.first expect(warning).to include(stack.name) - expect(warning).to include(stack.description) - expect(warning).to include('deprecated') + expect(warning).to include('DEPRECATED') end it 'does not raise an error' do @@ -146,7 +132,7 @@ module VCAP::CloudController it 'returns a disabled error message' do expect do StackStateValidator.validate_for_restaging!(stack) - end.to raise_error(StackStateValidator::DisabledStackError, /Stack '#{stack.name}' is disabled and cannot be used for staging new applications./) + end.to raise_error(StackStateValidator::DisabledStackError, /The stack '#{stack.name}' is '#{StackStates::STACK_DISABLED}' and cannot be used for staging./) end it 'includes stack name in error message' do @@ -154,12 +140,6 @@ module VCAP::CloudController StackStateValidator.validate_for_restaging!(stack) end.to raise_error(StackStateValidator::DisabledStackError, /#{stack.name}/) end - - it 'includes stack description in error message' do - expect do - StackStateValidator.validate_for_restaging!(stack) - end.to raise_error(StackStateValidator::DisabledStackError, /#{stack.description}/) - end end end @@ -167,22 +147,21 @@ module VCAP::CloudController let(:stack) { Stack.make(name: 'cflinuxfs3', description: 'End of life December 2025') } it 'returns formatted warning string' do - warning = StackStateValidator.build_deprecation_warning(stack) + warning = StackStateValidator.build_deprecation_warning(stack, StackStates::STACK_DEPRECATED) expect(warning).to be_a(String) expect(warning).to include('cflinuxfs3') - expect(warning).to include('deprecated') - expect(warning).to include('End of life December 2025') + expect(warning).to include('DEPRECATED') end it 'includes stack name when description is empty' do stack.description = '' - warning = StackStateValidator.build_deprecation_warning(stack) + warning = StackStateValidator.build_deprecation_warning(stack, StackStates::STACK_DEPRECATED) expect(warning).to include('cflinuxfs3') end it 'handles nil description' do stack.description = nil - warning = StackStateValidator.build_deprecation_warning(stack) + warning = StackStateValidator.build_deprecation_warning(stack, StackStates::STACK_DEPRECATED) expect(warning).to include('cflinuxfs3') end end From d98ba5f4690e04ba1d10ac3245633bbe7cc0ea42 Mon Sep 17 00:00:00 2001 From: Rashed Kamal Date: Mon, 12 Jan 2026 12:59:19 -0500 Subject: [PATCH 11/14] Stack state validation when creating and updating apps via V2 and V3 APIs Sets warnings in the response hash when there are warnings Signed-off-by: Rashed Kamal --- app/actions/app_create.rb | 18 +++ app/actions/app_update.rb | 21 +++- app/actions/v2/app_create.rb | 15 +++ app/actions/v2/app_update.rb | 18 +++ app/controllers/runtime/apps_controller.rb | 2 + app/controllers/v3/apps_controller.rb | 4 + app/models/runtime/app_model.rb | 2 + app/models/runtime/process_model.rb | 2 + app/presenters/v3/app_presenter.rb | 8 ++ app/presenters/v3/build_presenter.rb | 7 +- spec/request/apps_spec.rb | 92 +++++++++++++++- spec/request/builds_spec.rb | 4 - spec/request/v2/apps_spec.rb | 74 +++++++++++++ spec/unit/actions/app_create_spec.rb | 65 +++++++++++ spec/unit/actions/app_update_spec.rb | 74 +++++++++++++ spec/unit/actions/v2/app_create_spec.rb | 103 ++++++++++++++++++ spec/unit/actions/v2/app_update_spec.rb | 81 ++++++++++++++ spec/unit/presenters/v3/app_presenter_spec.rb | 26 +++++ .../presenters/v3/build_presenter_spec.rb | 10 +- 19 files changed, 612 insertions(+), 14 deletions(-) diff --git a/app/actions/app_create.rb b/app/actions/app_create.rb index 36c48d8e4ac..d7d04e6929b 100644 --- a/app/actions/app_create.rb +++ b/app/actions/app_create.rb @@ -16,6 +16,7 @@ def initialize(user_audit_info) def create(message, lifecycle) app = nil + warnings = [] AppModel.db.transaction do app = AppModel.create( name: message.name, @@ -24,6 +25,7 @@ def create(message, lifecycle) ) lifecycle.create_lifecycle_data_model(app) + warnings = validate_stack_state(app, lifecycle) validate_buildpacks_are_ready(app) MetadataUpdate.update(app, message) @@ -43,10 +45,14 @@ 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]) + raise InvalidApp.new(e.message) + rescue StackStateValidator::DisabledStackError, StackStateValidator::RestrictedStackError => e raise InvalidApp.new(e.message) end @@ -74,5 +80,17 @@ def validate_buildpacks_are_ready(app) end end end + + def validate_stack_state(app, lifecycle) + return [] if lifecycle.type == Lifecycles::DOCKER + + stack_name = app.lifecycle_data.try(:stack) + return [] unless stack_name + + stack = Stack.find(name: stack_name) + return [] unless stack + + StackStateValidator.validate_for_new_app!(stack) + end end end diff --git a/app/actions/app_update.rb b/app/actions/app_update.rb index 7cb9985cbaa..988472c6e87 100644 --- a/app/actions/app_update.rb +++ b/app/actions/app_update.rb @@ -53,8 +53,13 @@ def update(app, message, lifecycle) end end + warnings = validate_stack_state(app, message, lifecycle) + app.instance_variable_set(:@stack_warnings, warnings) + app - rescue Sequel::ValidationFailed => e + rescue Sequel::ValidationFailed, + StackStateValidator::DisabledStackError, + StackStateValidator::RestrictedStackError => e raise InvalidApp.new(e.message) end @@ -81,5 +86,19 @@ def validate_not_changing_lifecycle_type!(app, lifecycle) def existing_environment_variables_for(app) app.environment_variables.nil? ? {} : app.environment_variables.symbolize_keys end + + def validate_stack_state(app, message, lifecycle) + return [] if lifecycle.type == Lifecycles::DOCKER + return [] unless message.requested?(:lifecycle) && message.buildpack_data.requested?(:stack) + + stack = Stack.find(name: message.buildpack_data.stack) + return [] unless stack + + if app.builds_dataset.count.zero? + StackStateValidator.validate_for_new_app!(stack) + else + StackStateValidator.validate_for_restaging!(stack) + end + end end end diff --git a/app/actions/v2/app_create.rb b/app/actions/v2/app_create.rb index de6bdeac15d..079dae3d063 100644 --- a/app/actions/v2/app_create.rb +++ b/app/actions/v2/app_create.rb @@ -46,6 +46,9 @@ 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) + process end @@ -106,6 +109,18 @@ def validate_package_exists!(process, request_attrs) raise CloudController::Errors::ApiError.new_from_details('AppPackageInvalid', 'bits have not been uploaded') end + + def validate_stack_state(request_attrs) + return [] if request_attrs.key?('docker_image') + + stack_name = get_stack_name(request_attrs['stack_guid']) + stack = Stack.find(name: stack_name) + return [] unless stack + + StackStateValidator.validate_for_new_app!(stack) + rescue StackStateValidator::DisabledStackError, StackStateValidator::RestrictedStackError => e + raise CloudController::Errors::ApiError.new_from_details('StackValidationFailed', e.message) + end end end end diff --git a/app/actions/v2/app_update.rb b/app/actions/v2/app_update.rb index c5b32b756cb..bbfa7972cc2 100644 --- a/app/actions/v2/app_update.rb +++ b/app/actions/v2/app_update.rb @@ -39,6 +39,7 @@ def update(app, process, request_attrs) prepare_to_stage(app) if staging_necessary?(process, request_attrs) end + validate_stack_state(app, request_attrs) stage(process) if staging_necessary?(process, request_attrs) end @@ -184,6 +185,23 @@ def staging_necessary?(process, request_attrs) def v2_api_staging_disabled? !!VCAP::CloudController::Config.config.get(:temporary_disable_v2_staging) end + + def validate_stack_state(app, request_attrs) + return unless request_attrs.key?('stack_guid') + return if request_attrs.key?('docker_image') + + stack = Stack.find(guid: request_attrs['stack_guid']) + return unless stack + + stack_warnings = if app.builds_dataset.count.zero? + StackStateValidator.validate_for_new_app!(stack) + else + StackStateValidator.validate_for_restaging!(stack) + end + @warnings.concat(stack_warnings) + rescue StackStateValidator::DisabledStackError, StackStateValidator::RestrictedStackError => e + raise CloudController::Errors::ApiError.new_from_details('StackValidationFailed', e.message) + end end end end diff --git a/app/controllers/runtime/apps_controller.rb b/app/controllers/runtime/apps_controller.rb index 22157ecd01b..1ec42d1fbd9 100644 --- a/app/controllers/runtime/apps_controller.rb +++ b/app/controllers/runtime/apps_controller.rb @@ -358,6 +358,8 @@ def create creator = V2::AppCreate.new(access_validator: self) process = creator.create(request_attrs) + process.stack_warnings&.each { |warning| add_warning(warning) } + @app_event_repository.record_app_create( process, process.space, diff --git a/app/controllers/v3/apps_controller.rb b/app/controllers/v3/apps_controller.rb index 76c2f32f48c..21d21a05950 100644 --- a/app/controllers/v3/apps_controller.rb +++ b/app/controllers/v3/apps_controller.rb @@ -107,6 +107,8 @@ def create } ) + add_warning_headers(app.stack_warnings) if app.stack_warnings&.any? + render status: :created, json: Presenters::V3::AppPresenter.new(app) rescue AppCreate::InvalidApp => e unprocessable!(e.message) @@ -134,6 +136,8 @@ def update } ) + add_warning_headers(app.stack_warnings) if app.stack_warnings&.any? + render status: :ok, json: Presenters::V3::AppPresenter.new(app) rescue AppUpdate::DropletNotFound droplet_not_found! diff --git a/app/models/runtime/app_model.rb b/app/models/runtime/app_model.rb index b47733f7fb3..e30790aad2b 100644 --- a/app/models/runtime/app_model.rb +++ b/app/models/runtime/app_model.rb @@ -11,6 +11,8 @@ 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/process_model.rb b/app/models/runtime/process_model.rb index b31700b07fe..c500d0b6498 100644 --- a/app/models/runtime/process_model.rb +++ b/app/models/runtime/process_model.rb @@ -22,6 +22,8 @@ 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 index 35bb5eda738..82352dda92a 100644 --- a/app/presenters/v3/app_presenter.rb +++ b/app/presenters/v3/app_presenter.rb @@ -45,6 +45,8 @@ def to_hash links: build_links } + hash[:warnings] = app_warnings if app_warnings + @decorators.reduce(hash) { |memo, d| d.decorate(memo, [app]) } end @@ -74,6 +76,12 @@ 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 index fa4f861afe7..6d27c25bd03 100644 --- 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, @@ -30,7 +30,6 @@ def to_hash }, package: { guid: build.package_guid }, droplet: droplet, - warnings: build_warnings, created_by: { guid: build.created_by_user_guid, name: build.created_by_user_name, @@ -43,6 +42,10 @@ def to_hash }, links: build_links } + + hash[:warnings] = build_warnings if build_warnings + + hash end private diff --git a/spec/request/apps_spec.rb b/spec/request/apps_spec.rb index 14d3101c773..fec0db0851e 100644 --- a/spec/request/apps_spec.rb +++ b/spec/request/apps_spec.rb @@ -362,6 +362,96 @@ end end end + + context 'stack state validation' do + before do + space.organization.add_user(user) + space.add_developer(user) + end + + context 'when stack is DISABLED' do + let(:disabled_stack) { VCAP::CloudController::Stack.make(name: 'disabled-stack', state: 'DISABLED') } + let(:create_request) do + { + name: 'my_app', + lifecycle: { type: 'buildpack', data: { stack: disabled_stack.name } }, + relationships: { space: { data: { guid: space.guid } } } + } + end + + it 'returns 422 with error message' do + post '/v3/apps', create_request.to_json, user_header + + expect(last_response.status).to eq(422) + expect(parsed_response['errors'].first['detail']).to include('DISABLED') + end + end + + context 'when stack is RESTRICTED' do + let(:restricted_stack) { VCAP::CloudController::Stack.make(name: 'restricted-stack', state: 'RESTRICTED') } + let(:create_request) do + { + name: 'my_app', + lifecycle: { type: 'buildpack', data: { stack: restricted_stack.name } }, + relationships: { space: { data: { guid: space.guid } } } + } + end + + it 'returns 422 with error message for new apps' do + post '/v3/apps', create_request.to_json, user_header + + expect(last_response.status).to eq(422) + expect(parsed_response['errors'].first['detail']).to include('RESTRICTED') + end + end + + context 'when stack is DEPRECATED' do + let(:deprecated_stack) { VCAP::CloudController::Stack.make(name: 'deprecated-stack', state: 'DEPRECATED') } + let(:create_request) do + { + name: 'my_app', + lifecycle: { type: 'buildpack', data: { stack: deprecated_stack.name } }, + relationships: { space: { data: { guid: space.guid } } } + } + end + + it 'creates the app with 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') + end + + it 'includes warnings in X-Cf-Warnings header' do + post '/v3/apps', create_request.to_json, user_header + + expect(last_response.status).to eq(201) + expect(last_response.headers['X-Cf-Warnings']).to be_present + decoded_warning = CGI.unescape(last_response.headers['X-Cf-Warnings']) + expect(decoded_warning).to include('DEPRECATED') + end + end + + context 'when stack is ACTIVE' do + let(:active_stack) { VCAP::CloudController::Stack.make(name: 'active-stack', state: 'ACTIVE') } + let(:create_request) do + { + name: 'my_app', + lifecycle: { type: 'buildpack', data: { stack: active_stack.name } }, + relationships: { space: { data: { guid: space.guid } } } + } + end + + it 'creates the app without warnings' do + post '/v3/apps', create_request.to_json, user_header + + expect(last_response.status).to eq(201) + expect(parsed_response).not_to have_key('warnings') + expect(last_response.headers['X-Cf-Warnings']).to be_nil + end + end + end end describe 'GET /v3/apps' do @@ -1900,7 +1990,6 @@ 'droplet' => { 'guid' => droplet.guid }, - 'warnings' => nil, 'relationships' => { 'app' => { 'data' => { 'guid' => app_model.guid } } }, 'metadata' => { 'labels' => {}, 'annotations' => {} }, 'links' => { @@ -1930,7 +2019,6 @@ 'droplet' => { 'guid' => second_droplet.guid }, - 'warnings' => nil, 'relationships' => { 'app' => { 'data' => { 'guid' => app_model.guid } } }, 'metadata' => { 'labels' => {}, 'annotations' => {} }, 'links' => { diff --git a/spec/request/builds_spec.rb b/spec/request/builds_spec.rb index 077ea1de486..2f16cbe5a3a 100644 --- a/spec/request/builds_spec.rb +++ b/spec/request/builds_spec.rb @@ -92,7 +92,6 @@ 'guid' => package.guid }, 'droplet' => nil, - 'warnings' => nil, 'relationships' => { 'app' => { 'data' => { 'guid' => app_model.guid } } }, 'links' => { 'self' => { @@ -496,7 +495,6 @@ 'droplet' => { 'guid' => droplet.guid }, - 'warnings' => nil, 'relationships' => { 'app' => { 'data' => { 'guid' => app_model.guid } } }, 'metadata' => { 'labels' => {}, 'annotations' => {} }, 'links' => { @@ -526,7 +524,6 @@ 'droplet' => { 'guid' => second_droplet.guid }, - 'warnings' => nil, 'relationships' => { 'app' => { 'data' => { 'guid' => app_model.guid } } }, 'metadata' => { 'labels' => {}, 'annotations' => {} }, 'links' => { @@ -629,7 +626,6 @@ 'droplet' => { 'guid' => droplet.guid }, - 'warnings' => nil, 'metadata' => { 'labels' => {}, 'annotations' => {} }, 'relationships' => { 'app' => { 'data' => { 'guid' => app_model.guid } } }, 'links' => { diff --git a/spec/request/v2/apps_spec.rb b/spec/request/v2/apps_spec.rb index a951fb30a18..4e5c147e7a3 100644 --- a/spec/request/v2/apps_spec.rb +++ b/spec/request/v2/apps_spec.rb @@ -689,6 +689,80 @@ ) end end + + describe 'stack state validation' do + context 'when stack is DISABLED' do + let(:disabled_stack) { VCAP::CloudController::Stack.make(name: 'disabled-stack', state: 'DISABLED') } + + it 'returns 422 with error message' do + post_params = Oj.dump({ + name: 'maria', + space_guid: space.guid, + stack_guid: disabled_stack.guid + }) + + post '/v2/apps', post_params, headers_for(user) + + expect(last_response.status).to eq(422) + expect(parsed_response['error_code']).to eq('CF-StackValidationFailed') + expect(parsed_response['description']).to include('DISABLED') + end + end + + context 'when stack is RESTRICTED' do + let(:restricted_stack) { VCAP::CloudController::Stack.make(name: 'restricted-stack', state: 'RESTRICTED') } + + it 'returns 422 with error message for new apps' do + post_params = Oj.dump({ + name: 'maria', + space_guid: space.guid, + stack_guid: restricted_stack.guid + }) + + post '/v2/apps', post_params, headers_for(user) + + expect(last_response.status).to eq(422) + expect(parsed_response['error_code']).to eq('CF-StackValidationFailed') + expect(parsed_response['description']).to include('RESTRICTED') + end + end + + context 'when stack is DEPRECATED' do + let(:deprecated_stack) { VCAP::CloudController::Stack.make(name: 'deprecated-stack', state: 'DEPRECATED') } + + it 'creates the app with warnings in X-Cf-Warnings header' do + post_params = Oj.dump({ + name: 'maria', + space_guid: space.guid, + stack_guid: deprecated_stack.guid + }) + + post '/v2/apps', post_params, headers_for(user) + + expect(last_response.status).to eq(201) + expect(last_response.headers['X-Cf-Warnings']).to be_present + decoded_warning = CGI.unescape(last_response.headers['X-Cf-Warnings']) + expect(decoded_warning).to include('DEPRECATED') + end + end + + context 'when stack is ACTIVE' do + let(:active_stack) { VCAP::CloudController::Stack.make(name: 'active-stack', state: 'ACTIVE') } + + it 'creates the app without warnings' do + post_params = Oj.dump({ + name: 'maria', + space_guid: space.guid, + stack_guid: active_stack.guid + }) + + post '/v2/apps', post_params, headers_for(user) + + expect(last_response.status).to eq(201) + expect(last_response.headers['X-Cf-Warnings']).to be_nil + end + end + end end describe 'PUT /v2/apps/:guid' do diff --git a/spec/unit/actions/app_create_spec.rb b/spec/unit/actions/app_create_spec.rb index a35f68958a2..c5fddd068a6 100644 --- a/spec/unit/actions/app_create_spec.rb +++ b/spec/unit/actions/app_create_spec.rb @@ -216,6 +216,71 @@ module VCAP::CloudController end.to raise_error(CloudController::Errors::V3::ApiError) end end + + describe 'stack state validation' do + let(:test_stack) { Stack.make(name: 'test-stack-for-validation') } + let(:lifecycle_request) { { type: 'buildpack', data: { buildpacks: [buildpack_identifier], stack: test_stack.name } } } + + context 'when stack is DISABLED' do + before { test_stack.update(state: StackStates::STACK_DISABLED) } + + it 'raises InvalidApp error' do + expect do + app_create.create(message, lifecycle) + end.to raise_error(AppCreate::InvalidApp, /DISABLED/) + end + + it 'does not create an app' do + expect do + app_create.create(message, lifecycle) + rescue StandardError + nil + end.not_to(change(AppModel, :count)) + end + end + + context 'when stack is RESTRICTED' do + before { test_stack.update(state: StackStates::STACK_RESTRICTED) } + + it 'raises InvalidApp error for new apps' do + expect do + app_create.create(message, lifecycle) + end.to raise_error(AppCreate::InvalidApp, /RESTRICTED/) + end + end + + context 'when stack is DEPRECATED' do + before { test_stack.update(state: StackStates::STACK_DEPRECATED) } + + 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') + end + end + + context 'when stack is ACTIVE' do + before { test_stack.update(state: StackStates::STACK_ACTIVE) } + + 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 + end + end + + context 'when lifecycle is docker' do + let(:lifecycle_request) { { type: 'docker' } } + let(:lifecycle) { AppDockerLifecycle.new(message) } + + 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 + end + end + end end end end diff --git a/spec/unit/actions/app_update_spec.rb b/spec/unit/actions/app_update_spec.rb index 2b2f9cc6a63..7fcfee8bf89 100644 --- a/spec/unit/actions/app_update_spec.rb +++ b/spec/unit/actions/app_update_spec.rb @@ -357,6 +357,80 @@ module VCAP::CloudController expect { app_update.update(app_model, message, lifecycle) }.to raise_error(AppUpdate::InvalidApp) end end + + describe 'stack state validation on stack change' do + let(:new_stack) { Stack.make(name: 'new-stack') } + let(:message) do + AppUpdateMessage.new({ + lifecycle: { + type: 'buildpack', + data: { stack: new_stack.name } + } + }) + end + + context 'when stack is DISABLED' do + before { new_stack.update(state: StackStates::STACK_DISABLED) } + + it 'raises InvalidApp error' do + expect do + app_update.update(app_model, message, lifecycle) + end.to raise_error(AppUpdate::InvalidApp, /DISABLED/) + end + end + + context 'when stack is RESTRICTED' do + before { new_stack.update(state: StackStates::STACK_RESTRICTED) } + + context 'when app has no builds' do + it 'raises InvalidApp error' do + expect(app_model.builds_dataset.count).to eq(0) + expect do + app_update.update(app_model, message, lifecycle) + end.to raise_error(AppUpdate::InvalidApp, /RESTRICTED/) + end + end + + context 'when app has previous builds' do + before { BuildModel.make(app: app_model, state: BuildModel::STAGED_STATE) } + + it 'allows the update' do + expect(app_model.builds_dataset.count).to eq(1) + expect do + app_update.update(app_model, message, lifecycle) + end.not_to raise_error + end + end + end + + context 'when stack is DEPRECATED' do + 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') + 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 + 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 + end + end + end end end end diff --git a/spec/unit/actions/v2/app_create_spec.rb b/spec/unit/actions/v2/app_create_spec.rb index a54a5e49bc2..063023c000e 100644 --- a/spec/unit/actions/v2/app_create_spec.rb +++ b/spec/unit/actions/v2/app_create_spec.rb @@ -217,6 +217,109 @@ module VCAP::CloudController expect(process.app.lifecycle_data.buildpacks).to eq([]) end end + + describe 'stack state validation' do + context 'when stack is DISABLED' do + let(:disabled_stack) { Stack.make(name: 'disabled-stack', state: StackStates::STACK_DISABLED) } + let(:request_attrs) do + { + 'name' => 'maria', + 'space_guid' => space.guid, + 'state' => 'STOPPED', + 'health_check_type' => 'port', + 'stack_guid' => disabled_stack.guid + } + end + + it 'raises StackValidationFailed error' do + expect do + app_create.create(request_attrs) + end.to raise_error(CloudController::Errors::ApiError) do |error| + expect(error.name).to eq('StackValidationFailed') + expect(error.message).to include('DISABLED') + end + end + end + + context 'when stack is RESTRICTED' do + let(:restricted_stack) { Stack.make(name: 'restricted-stack', state: StackStates::STACK_RESTRICTED) } + let(:request_attrs) do + { + 'name' => 'maria', + 'space_guid' => space.guid, + 'state' => 'STOPPED', + 'health_check_type' => 'port', + 'stack_guid' => restricted_stack.guid + } + end + + it 'raises StackValidationFailed error for new apps' do + expect do + app_create.create(request_attrs) + end.to raise_error(CloudController::Errors::ApiError) do |error| + expect(error.name).to eq('StackValidationFailed') + expect(error.message).to include('RESTRICTED') + end + end + end + + context 'when stack is DEPRECATED' do + let(:deprecated_stack) { Stack.make(name: 'deprecated-stack', state: StackStates::STACK_DEPRECATED) } + let(:request_attrs) do + { + 'name' => 'maria', + 'space_guid' => space.guid, + 'state' => 'STOPPED', + 'health_check_type' => 'port', + 'stack_guid' => deprecated_stack.guid + } + end + + 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') + end + end + + context 'when stack is ACTIVE' do + let(:active_stack) { Stack.make(name: 'active-stack', state: StackStates::STACK_ACTIVE) } + let(:request_attrs) do + { + 'name' => 'maria', + 'space_guid' => space.guid, + 'state' => 'STOPPED', + 'health_check_type' => 'port', + 'stack_guid' => active_stack.guid + } + end + + 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 + end + end + + context 'when app is docker-based' do + let(:request_attrs) do + { + 'name' => 'maria', + 'space_guid' => space.guid, + 'state' => 'STOPPED', + 'health_check_type' => 'port', + 'docker_image' => 'some-image:latest' + } + end + + 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 + end + end + end end end end diff --git a/spec/unit/actions/v2/app_update_spec.rb b/spec/unit/actions/v2/app_update_spec.rb index 778e1bd62fd..a413f2855f5 100644 --- a/spec/unit/actions/v2/app_update_spec.rb +++ b/spec/unit/actions/v2/app_update_spec.rb @@ -715,6 +715,87 @@ module VCAP::CloudController end end end + + describe 'stack state validation on stack change' do + let(:process) { ProcessModel.make } + let(:app) { process.app } + + context 'when stack is DISABLED' do + let(:disabled_stack) { Stack.make(name: 'disabled-stack', state: StackStates::STACK_DISABLED) } + + it 'raises StackValidationFailed error' do + expect do + app_update.update(app, process, { 'stack_guid' => disabled_stack.guid }) + end.to raise_error(CloudController::Errors::ApiError) do |error| + expect(error.name).to eq('StackValidationFailed') + expect(error.message).to include('DISABLED') + end + end + end + + context 'when stack is RESTRICTED' do + let(:restricted_stack) { Stack.make(name: 'restricted-stack', state: StackStates::STACK_RESTRICTED) } + + context 'when app has no builds' do + it 'raises StackValidationFailed error' do + expect(app.builds_dataset.count).to eq(0) + expect do + app_update.update(app, process, { 'stack_guid' => restricted_stack.guid }) + end.to raise_error(CloudController::Errors::ApiError) do |error| + expect(error.name).to eq('StackValidationFailed') + expect(error.message).to include('RESTRICTED') + end + end + end + + context 'when app has previous builds' do + before { BuildModel.make(app: app, state: BuildModel::STAGED_STATE) } + + it 'allows the update' do + expect(app.builds_dataset.count).to eq(1) + expect do + app_update.update(app, process, { 'stack_guid' => restricted_stack.guid }) + end.not_to raise_error + end + end + end + + context 'when stack is DEPRECATED' do + let(:deprecated_stack) { Stack.make(name: 'deprecated-stack', state: StackStates::STACK_DEPRECATED) } + + it 'updates the app with warnings' do + app_update.update(app, process, { 'stack_guid' => deprecated_stack.guid }) + expect(app_update.warnings).to be_present + expect(app_update.warnings.first).to include('DEPRECATED') + end + end + + context 'when stack is ACTIVE' do + let(:active_stack) { Stack.make(name: 'active-stack', state: StackStates::STACK_ACTIVE) } + + it 'updates the app without warnings' do + app_update.update(app, process, { 'stack_guid' => active_stack.guid }) + expect(app_update.warnings).to be_empty + end + end + + context 'when stack is not being changed' do + it 'does not validate stack state' do + app_update.update(app, process, { 'name' => 'new-name' }) + expect(app_update.warnings).to be_empty + end + end + + context 'when app is docker-based' do + let(:docker_process) { ProcessModelFactory.make(app: AppModel.make(:docker), docker_image: 'repo/original-image') } + let(:docker_app) { docker_process.app } + + it 'skips stack validation when updating docker image' do + app_update.update(docker_app, docker_process, { 'docker_image' => 'new-image:latest' }) + expect(app_update.warnings).to be_empty + end + end + end end end end diff --git a/spec/unit/presenters/v3/app_presenter_spec.rb b/spec/unit/presenters/v3/app_presenter_spec.rb index db3dad95550..e9a03e381cd 100644 --- a/spec/unit/presenters/v3/app_presenter_spec.rb +++ b/spec/unit/presenters/v3/app_presenter_spec.rb @@ -112,6 +112,32 @@ def decorate(hash, apps) expect(result[:included][:bananas]).to contain_exactly('Davis is bananas') end end + + context 'when there are stack warnings' do + before do + app.instance_variable_set(:@stack_warnings, ["WARNING: The stack 'cflinuxfs3' is 'DEPRECATED' and will be removed in the future."]) + end + + it 'includes warnings in the presented app' do + expect(result[:warnings]).to eq([{ detail: "WARNING: The stack 'cflinuxfs3' is 'DEPRECATED' and will be removed in the future." }]) + end + end + + context 'when there are no stack state validation warnings' do + it 'does not include warnings key' do + expect(result).not_to have_key(:warnings) + end + end + + context 'when stack_warnings is empty' do + before do + app.instance_variable_set(:@stack_warnings, []) + end + + it 'does not include warnings key' do + expect(result).not_to have_key(:warnings) + end + end end end end diff --git a/spec/unit/presenters/v3/build_presenter_spec.rb b/spec/unit/presenters/v3/build_presenter_spec.rb index 658103e1e8b..7f5bcb2032b 100644 --- a/spec/unit/presenters/v3/build_presenter_spec.rb +++ b/spec/unit/presenters/v3/build_presenter_spec.rb @@ -175,14 +175,14 @@ module VCAP::CloudController::Presenters::V3 build.instance_variable_set(:@stack_warnings, []) end - it 'returns nil for warnings' do - expect(result[:warnings]).to be_nil + it 'does not include warnings key' do + expect(result).not_to have_key(:warnings) end end - context 'when stack warnings is nil' do - it 'returns nil for warnings' do - expect(result[:warnings]).to be_nil + context 'when no stack warning present' do + it 'does not include warnings key' do + expect(result).not_to have_key(:warnings) end end end From 54e547f17c8a6ced9af747cf5613f5b59b292fae Mon Sep 17 00:00:00 2001 From: Rashed Kamal Date: Fri, 23 Jan 2026 17:28:22 -0500 Subject: [PATCH 12/14] =?UTF-8?q?Removes=20stack=5Fwarnings=20from=20the?= =?UTF-8?q?=20models.=20Actions=20own=20their=20warnings=20and=20Controlle?= =?UTF-8?q?rs=20set=20=20warnings=20=E2=86=92=20headers?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Rashed Kamal --- app/actions/app_create.rb | 7 +++-- app/actions/app_update.rb | 5 ++-- app/actions/build_create.rb | 5 ++-- app/actions/v2/app_create.rb | 5 ++-- app/actions/v2/app_stage.rb | 2 +- app/controllers/runtime/apps_controller.rb | 2 +- app/controllers/v3/apps_controller.rb | 10 ++++--- app/controllers/v3/builds_controller.rb | 5 ++-- app/models/runtime/app_model.rb | 2 -- app/models/runtime/build_model.rb | 2 -- app/models/runtime/process_model.rb | 2 -- app/presenters/v3/app_presenter.rb | 8 ------ app/presenters/v3/build_presenter.rb | 12 +-------- spec/request/apps_spec.rb | 5 ++-- spec/request/builds_spec.rb | 9 ++----- spec/unit/actions/app_create_spec.rb | 8 +++--- spec/unit/actions/app_update_spec.rb | 14 +++++----- spec/unit/actions/v2/app_create_spec.rb | 8 +++--- spec/unit/actions/v2/app_stage_spec.rb | 2 +- spec/unit/presenters/v3/app_presenter_spec.rb | 26 ------------------- .../presenters/v3/build_presenter_spec.rb | 21 --------------- 21 files changed, 43 insertions(+), 117 deletions(-) diff --git a/app/actions/app_create.rb b/app/actions/app_create.rb index d7d04e6929b..4ea914fb42c 100644 --- a/app/actions/app_create.rb +++ b/app/actions/app_create.rb @@ -9,6 +9,8 @@ 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') @@ -16,7 +18,6 @@ def initialize(user_audit_info) 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 index 988472c6e87..a5ac3ead884 100644 --- 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 index 1bdde8cc14a..b2765cc18b3 100644 --- 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 index 079dae3d063..0e7d7f34632 100644 --- 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 index 968aedfecbf..1990fb9c698 100644 --- 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 index 1ec42d1fbd9..d57f39b5775 100644 --- 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 index 21d21a05950..3ed4e2a8a9d 100644 --- 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 index b8598285828..c2072481d5b 100644 --- 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 index e30790aad2b..b47733f7fb3 100644 --- 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 index 8dac1c16ef1..abc5310866d 100644 --- 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 index c500d0b6498..b31700b07fe 100644 --- 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 index 82352dda92a..35bb5eda738 100644 --- 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 index 6d27c25bd03..3579faf4364 100644 --- 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 index fec0db0851e..64fcef98a77 100644 --- 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 index 2f16cbe5a3a..d63b7edce7e 100644 --- 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 index c5fddd068a6..da951d624ce 100644 --- 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 index 7fcfee8bf89..777a4a647dd 100644 --- a/spec/unit/actions/app_update_spec.rb +++ b/spec/unit/actions/app_update_spec.rb @@ -407,9 +407,9 @@ 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 @@ -417,8 +417,8 @@ module VCAP::CloudController 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 @@ -426,8 +426,8 @@ module VCAP::CloudController 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 index 063023c000e..0225cb8f867 100644 --- 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 index 0e819b1fc4e..362e7418d72 100644 --- 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 index e9a03e381cd..db3dad95550 100644 --- a/spec/unit/presenters/v3/app_presenter_spec.rb +++ b/spec/unit/presenters/v3/app_presenter_spec.rb @@ -112,32 +112,6 @@ def decorate(hash, apps) expect(result[:included][:bananas]).to contain_exactly('Davis is bananas') end end - - context 'when there are stack warnings' do - before do - app.instance_variable_set(:@stack_warnings, ["WARNING: The stack 'cflinuxfs3' is 'DEPRECATED' and will be removed in the future."]) - end - - it 'includes warnings in the presented app' do - expect(result[:warnings]).to eq([{ detail: "WARNING: The stack 'cflinuxfs3' is 'DEPRECATED' and will be removed in the future." }]) - end - end - - context 'when there are no stack state validation warnings' do - it 'does not include warnings key' do - expect(result).not_to have_key(:warnings) - end - end - - context 'when stack_warnings is empty' do - before do - app.instance_variable_set(:@stack_warnings, []) - end - - it 'does not include warnings key' do - expect(result).not_to have_key(:warnings) - end - end end end end diff --git a/spec/unit/presenters/v3/build_presenter_spec.rb b/spec/unit/presenters/v3/build_presenter_spec.rb index 7f5bcb2032b..5d0bb07a6bb 100644 --- a/spec/unit/presenters/v3/build_presenter_spec.rb +++ b/spec/unit/presenters/v3/build_presenter_spec.rb @@ -159,27 +159,6 @@ module VCAP::CloudController::Presenters::V3 end end - context 'when stack has warnings' do - before do - build.instance_variable_set(:@stack_warnings, ['Stack cflinuxfs3 is deprecated. EOL Dec 2025']) - end - - it 'includes warnings in response' do - expect(result[:warnings]).to be_present - expect(result[:warnings]).to eq([{ detail: 'Stack cflinuxfs3 is deprecated. EOL Dec 2025' }]) - end - end - - context 'when stack has no warnings' do - before do - build.instance_variable_set(:@stack_warnings, []) - end - - it 'does not include warnings key' do - expect(result).not_to have_key(:warnings) - end - end - context 'when no stack warning present' do it 'does not include warnings key' do expect(result).not_to have_key(:warnings) From 569c2bd9a1a7d8ce56fbf78d10470bae0db97f39 Mon Sep 17 00:00:00 2001 From: Rashed Kamal Date: Mon, 26 Jan 2026 16:42:26 -0500 Subject: [PATCH 13/14] Describe stack states in API doc Signed-off-by: Rashed Kamal --- .../source/includes/resources/stacks/_create.md.erb | 2 ++ docs/v3/source/includes/resources/stacks/_header.md | 12 ++++++++++++ 2 files changed, 14 insertions(+) diff --git a/docs/v3/source/includes/resources/stacks/_create.md.erb b/docs/v3/source/includes/resources/stacks/_create.md.erb index 275f77802b4..5d73609de96 100644 --- a/docs/v3/source/includes/resources/stacks/_create.md.erb +++ b/docs/v3/source/includes/resources/stacks/_create.md.erb @@ -12,6 +12,7 @@ curl "https://api.example.org/v3/stacks" \ -d '{ "name": "my-stack", "description": "Here is my stack!", + "state": "ACTIVE", }' ``` @@ -42,6 +43,7 @@ Name | Type | Description | Default **description** | _string_ | Description of the stack; must no longer than 250 characters | `null` **metadata.labels** | [_label object_](#labels) | Labels applied to the stack **metadata.annotations** | [_annotation object_](#annotations) | Annotations applied to the stack +**state** | string | The state of the stack; valid states are: `ACTIVE`, `RESTRICTED`, `DEPRECATED`, `DISABLED` #### Permitted roles | diff --git a/docs/v3/source/includes/resources/stacks/_header.md b/docs/v3/source/includes/resources/stacks/_header.md index 5aa27db3ee1..4bfe9b4bb2d 100644 --- a/docs/v3/source/includes/resources/stacks/_header.md +++ b/docs/v3/source/includes/resources/stacks/_header.md @@ -12,3 +12,15 @@ An application will automatically use buildpacks associated with the application Stacks are not used for apps with a [Docker lifecycle](#docker-lifecycle). +Operators control stack availability through state management. The following states determine how a stack can be used: + +*ACTIVE*: Default state. The stack is fully available for all operations. + +*DEPRECATED*: The stack is nearing end-of-life. It remains fully functional, +but users should migrate to an ACTIVE stack. + +*RESTRICTED*: A transitional state typically applied before deprecation or disabling. +New application creation is blocked; existing deployments continue to operate normally. + +*DISABLED*: The stack has reached end-of-life. New application creation and restaging are prohibited. +Running applications remain available. From 2b79df5a329758806aee9a00acba8e950634d6fa Mon Sep 17 00:00:00 2001 From: Rashed Kamal Date: Wed, 28 Jan 2026 12:56:52 -0500 Subject: [PATCH 14/14] Adds warning when changing default stack state Polishing based on review comments Signed-off-by: Rashed Kamal --- app/actions/stack_update.rb | 2 +- .../runtime/restages_controller.rb | 2 +- app/controllers/v3/stacks_controller.rb | 2 + app/models/runtime/stack.rb | 10 +---- .../source/includes/api_resources/_stacks.erb | 2 +- .../includes/resources/stacks/_update.md.erb | 2 +- spec/request/builds_spec.rb | 4 +- .../controllers/v3/stacks_controller_spec.rb | 45 +++++++++++++++++++ spec/unit/models/runtime/stack_spec.rb | 4 +- 9 files changed, 56 insertions(+), 17 deletions(-) diff --git a/app/actions/stack_update.rb b/app/actions/stack_update.rb index ec42149c64a..41fb244724c 100644 --- a/app/actions/stack_update.rb +++ b/app/actions/stack_update.rb @@ -16,7 +16,7 @@ def update(stack, message) MetadataUpdate.update(stack, message) Repositories::StackEventRepository.new.record_stack_update(stack, @user_audit_info, message.audit_hash) end - @logger.info("Finished updating metadata on stack #{stack.guid}") + @logger.info("Finished updating stack #{stack.guid}") stack rescue Sequel::ValidationFailed => e diff --git a/app/controllers/runtime/restages_controller.rb b/app/controllers/runtime/restages_controller.rb index b73e3920a9c..746fd67c37c 100644 --- a/app/controllers/runtime/restages_controller.rb +++ b/app/controllers/runtime/restages_controller.rb @@ -35,7 +35,7 @@ def restage(guid) process.app.update(droplet_guid: nil) AppStart.start_without_event(process.app, create_revision: false) end - # V2::AppStage.new(stagers: @stagers).stage(process) + app_stage = V2::AppStage.new(stagers: @stagers) app_stage.stage(process) app_stage.warnings.each { |warning| add_warning(warning) } diff --git a/app/controllers/v3/stacks_controller.rb b/app/controllers/v3/stacks_controller.rb index 29da3d29db1..940d5576ac4 100644 --- a/app/controllers/v3/stacks_controller.rb +++ b/app/controllers/v3/stacks_controller.rb @@ -52,7 +52,9 @@ def update message = StackUpdateMessage.new(hashed_params[:body]) unprocessable!(message.errors.full_messages) unless message.valid? + changing_default_stack_state = stack.default? && message.requested?(:state) stack = StackUpdate.new(user_audit_info).update(stack, message) + add_warning_headers(["Changing state on the default stack '#{stack.name}' may affect new application deployments."]) if changing_default_stack_state render status: :ok, json: Presenters::V3::StackPresenter.new(stack) end diff --git a/app/models/runtime/stack.rb b/app/models/runtime/stack.rb index 98db03ed298..c2fb2d81222 100644 --- a/app/models/runtime/stack.rb +++ b/app/models/runtime/stack.rb @@ -44,7 +44,7 @@ def around_save def validate validates_presence :name validates_unique :name - validates_includes StackStates::VALID_STATES, :state, allow_nil: true + validates_includes StackStates::VALID_STATES, :state, allow_missing: true end def before_destroy @@ -116,13 +116,5 @@ def restricted? def disabled? state == StackStates::STACK_DISABLED end - - def can_stage_new_app? - !restricted? && !disabled? - end - - def can_restage_apps? - !disabled? - end end end diff --git a/docs/v3/source/includes/api_resources/_stacks.erb b/docs/v3/source/includes/api_resources/_stacks.erb index 4abc6868cd7..34946fe29cc 100644 --- a/docs/v3/source/includes/api_resources/_stacks.erb +++ b/docs/v3/source/includes/api_resources/_stacks.erb @@ -90,7 +90,7 @@ "updated_at": "2018-11-09T22:43:28Z", "name": "my-stack", "description": "Here is my stack!", - "state": "ACTIVE", + "state": "DISABLED", "build_rootfs_image": "my-stack", "run_rootfs_image": "my-stack", "default": true, diff --git a/docs/v3/source/includes/resources/stacks/_update.md.erb b/docs/v3/source/includes/resources/stacks/_update.md.erb index 567fc51c000..6a79a0451ea 100644 --- a/docs/v3/source/includes/resources/stacks/_update.md.erb +++ b/docs/v3/source/includes/resources/stacks/_update.md.erb @@ -9,7 +9,7 @@ curl "https://api.example.org/v3/stacks/[guid]" \ -X PATCH \ -H "Authorization: bearer [token]" \ -H "Content-Type: application/json" \ - -d '{ "metadata": { "labels": { "key": "value" }, "annotations": {"note": "detailed information"}, "state": "DISABLED" }}' + -d '{ "metadata": { "labels": { "key": "value" }, "annotations": {"note": "detailed information"}},"state":"ACTIVE"}' ``` diff --git a/spec/request/builds_spec.rb b/spec/request/builds_spec.rb index d63b7edce7e..5a88e6b7795 100644 --- a/spec/request/builds_spec.rb +++ b/spec/request/builds_spec.rb @@ -292,7 +292,7 @@ end context 'first build for app' do - it 'returns 201 and does not create the build' do + it 'returns 201 and stages the app' do expect(app_model.builds_dataset.count).to eq(0) post '/v3/builds', create_request.to_json, developer_headers @@ -318,7 +318,7 @@ VCAP::CloudController::BuildModel.make(app: app_model, state: VCAP::CloudController::BuildModel::STAGED_STATE) end - it 'returns 201 and does not create the build' do + it 'returns 201 and stages the app' do expect(app_model.builds_dataset.count).to eq(1) post '/v3/builds', create_request.to_json, developer_headers diff --git a/spec/unit/controllers/v3/stacks_controller_spec.rb b/spec/unit/controllers/v3/stacks_controller_spec.rb index 59fd7a9934f..3207abc02fe 100644 --- a/spec/unit/controllers/v3/stacks_controller_spec.rb +++ b/spec/unit/controllers/v3/stacks_controller_spec.rb @@ -513,6 +513,51 @@ end end + describe 'default stack state change warning' do + let(:stack_config_file) { File.join(Paths::FIXTURES, 'config/stacks.yml') } + let(:default_stack_name) { 'default-stack-name' } + + before do + VCAP::CloudController::Stack.dataset.destroy + VCAP::CloudController::Stack.configure(stack_config_file) + set_current_user_as_admin + end + + context 'when changing state on the default stack' do + let!(:default_stack) { VCAP::CloudController::Stack.make(name: default_stack_name) } + + it 'returns a warning header with the stack name' do + patch :update, params: { guid: default_stack.guid, state: 'DEPRECATED' }, as: :json + + expect(response).to have_http_status(:ok) + expect(response).to have_warning_message("Changing state on the default stack '#{default_stack_name}' may affect new application deployments.") + end + end + + context 'when changing state on a non-default stack' do + let!(:default_stack) { VCAP::CloudController::Stack.make(name: default_stack_name) } + let!(:other_stack) { VCAP::CloudController::Stack.make(name: 'other-stack') } + + it 'does not return a warning header' do + patch :update, params: { guid: other_stack.guid, state: 'DEPRECATED' }, as: :json + + expect(response).to have_http_status(:ok) + expect(response.headers['X-Cf-Warnings']).to be_nil + end + end + + context 'when updating metadata on the default stack without changing state' do + let!(:default_stack) { VCAP::CloudController::Stack.make(name: default_stack_name) } + + it 'does not return a warning header' do + patch :update, params: { guid: default_stack.guid, metadata: { labels: { foo: 'bar' } } }, as: :json + + expect(response).to have_http_status(:ok) + expect(response.headers['X-Cf-Warnings']).to be_nil + end + end + end + describe 'authorization' do it_behaves_like 'permissions endpoint' do let(:roles_to_http_responses) do diff --git a/spec/unit/models/runtime/stack_spec.rb b/spec/unit/models/runtime/stack_spec.rb index ac43503f088..ec3fe336ee6 100644 --- a/spec/unit/models/runtime/stack_spec.rb +++ b/spec/unit/models/runtime/stack_spec.rb @@ -43,10 +43,10 @@ module VCAP::CloudController expect(stack.errors[:state]).to include(:includes) end - it 'allows nil state' do + it 'does not allow nil state' do stack = Stack.make stack.state = nil - expect(stack).to be_valid + expect(stack).not_to be_valid end end end