Adds warning when changing default stack state

Polishing based on review comments

Signed-off-by: Rashed Kamal <[email protected]>

Author: rashedkvm

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

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) }

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

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

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,

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"}'
 
 ```
 

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

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

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