Prevent org and space quota log rate limit changes

- If an existing deployment is upgraded to gain log rate limiting support then existing application processes will default to -1 (unlimited). - If an org or space quota is subsequently updated to include a finite log rate limit then processes in the affected orgs or spaces will be unable to restart without having their log rate limit changed. - Block org and space quota finite log rate limit updates until processes under the affected orgs or spaces have been updated with a finite log rate limit. Specify the names of the first two orgs or spaces and a total count in the error. - Block assignment of org or space quotas with finite log rate limits to orgs or spaces that contain processes with unlimited log rate limits. Co-authored-by: Carson Long <[email protected]>
cloudfoundry · Sep 12, 2022 · a3356ff · a3356ff
1 parent 5b358a0
commit a3356ff
Show file tree

Hide file tree

Showing 11 changed files with 308 additions and 5 deletions.
diff --git a/app/actions/organization_quota_apply.rb b/app/actions/organization_quota_apply.rb
@@ -4,8 +4,20 @@ class Error < ::StandardError
     end
 
     def apply(org_quota, message)
+      orgs = valid_orgs(message.organization_guids)
+
+      if org_quota.log_rate_limit != QuotaDefinition::UNLIMITED
+        affected_processes = Organization.where(Sequel[:organizations][:id] => orgs.map(&:id)).
+                             join(:spaces, organization_id: :id).
+                             join(:apps, space_guid: :guid).
+                             join(:processes, app_guid: :guid)
+
+        unless affected_processes.where(log_rate_limit: ProcessModel::UNLIMITED_LOG_RATE).empty?
+          error!('Current usage exceeds new quota values. The org(s) being assigned this quota contain apps running with an unlimited log rate limit.')
+        end
+      end
+
       QuotaDefinition.db.transaction do
-        orgs = valid_orgs(message.organization_guids)
         orgs.each { |org| org_quota.add_organization(org) }
       end
     rescue Sequel::ValidationFailed => e

diff --git a/app/actions/organization_quotas_update.rb b/app/actions/organization_quotas_update.rb
@@ -2,8 +2,18 @@ module VCAP::CloudController
   class OrganizationQuotasUpdate
     class Error < ::StandardError
     end
+
+    MAX_ORGS_TO_LIST_ON_FAILURE = 2
+
     # rubocop:disable Metrics/CyclomaticComplexity
     def self.update(quota, message)
+      if log_rate_limit(message) != QuotaDefinition::UNLIMITED
+        orgs = orgs_with_unlimited_processes(quota)
+        if orgs.any?
+          unlimited_processes_exist_error!(orgs)
+        end
+      end
+
       quota.db.transaction do
         quota.lock!
 
@@ -84,5 +94,34 @@ def self.total_routes(message)
     def self.total_private_domains(message)
       default_if_nil(message.total_domains, QuotaDefinition::UNLIMITED)
     end
+
+    def self.orgs_with_unlimited_processes(quota)
+      quota.organizations_dataset.
+        distinct.
+        select(Sequel[:organizations][:name]).
+        join(:spaces, organization_id: :id).
+        join(:apps, space_guid: :guid).
+        join(:processes, app_guid: :guid).
+        where(log_rate_limit: QuotaDefinition::UNLIMITED).
+        order(:name).
+        all
+    end
+
+    def self.unlimited_processes_exist_error!(orgs)
+      named_orgs = orgs.take(MAX_ORGS_TO_LIST_ON_FAILURE).map(&:name).join("', '")
+      message = if orgs.size == 1
+                  "Org '#{named_orgs}' assigned this quota contains"
+                elsif orgs.size > MAX_ORGS_TO_LIST_ON_FAILURE
+                  "Orgs '#{named_orgs}' and #{orgs.drop(MAX_ORGS_TO_LIST_ON_FAILURE).size}" \
+                  ' other orgs assigned this quota contain'
+                else
+                  "Orgs '#{named_orgs}' assigned this quota contain"
+                end + ' apps running with an unlimited log rate limit.'
+
+      raise Error.new("Current usage exceeds new quota values. #{message}")
+    end
+
+    private_class_method :orgs_with_unlimited_processes,
+                         :unlimited_processes_exist_error!
   end
 end
diff --git a/app/actions/space_quota_apply.rb b/app/actions/space_quota_apply.rb
@@ -6,6 +6,16 @@ class Error < ::StandardError
     def apply(space_quota, message, visible_space_guids: [], all_spaces_visible: false)
       spaces = valid_spaces(message.space_guids, visible_space_guids, all_spaces_visible, space_quota.organization_id)
 
+      if space_quota.log_rate_limit != QuotaDefinition::UNLIMITED
+        affected_processes = Space.where(Sequel[:spaces][:id] => spaces.map(&:id)).
+                             join(:apps, space_guid: :guid).
+                             join(:processes, app_guid: :guid)
+
+        unless affected_processes.where(log_rate_limit: ProcessModel::UNLIMITED_LOG_RATE).empty?
+          error!('Current usage exceeds new quota values. The space(s) being assigned this quota contain apps running with an unlimited log rate limit.')
+        end
+      end
+
       SpaceQuotaDefinition.db.transaction do
         spaces.each { |space| space_quota.add_space(space) }
       end

diff --git a/app/actions/space_quota_update.rb b/app/actions/space_quota_update.rb
@@ -3,8 +3,17 @@ class SpaceQuotaUpdate
     class Error < ::StandardError
     end
 
+    MAX_SPACES_TO_LIST_ON_FAILURE = 2
+
     # rubocop:disable Metrics/CyclomaticComplexity
     def self.update(quota, message)
+      if log_rate_limit(message) != QuotaDefinition::UNLIMITED
+        spaces = spaces_with_unlimited_processes(quota)
+        if spaces.any?
+          unlimited_processes_exist_error!(spaces)
+        end
+      end
+
       quota.db.transaction do
         quota.lock!
 
@@ -79,5 +88,33 @@ def self.total_reserved_route_ports(message)
     def self.total_routes(message)
       default_if_nil(message.total_routes, SpaceQuotaDefinition::UNLIMITED)
     end
+
+    def self.spaces_with_unlimited_processes(quota)
+      quota.spaces_dataset.
+        distinct.
+        select(Sequel[:spaces][:name]).
+        join(:apps, space_guid: :guid).
+        join(:processes, app_guid: :guid).
+        where(log_rate_limit: QuotaDefinition::UNLIMITED).
+        order(:name).
+        all
+    end
+
+    def self.unlimited_processes_exist_error!(spaces)
+      named_spaces = spaces.take(MAX_SPACES_TO_LIST_ON_FAILURE).map(&:name).join("', '")
+      message = if spaces.size == 1
+                  "Space '#{named_spaces}' assigned this quota contains"
+                elsif spaces.size > MAX_SPACES_TO_LIST_ON_FAILURE
+                  "Spaces '#{named_spaces}' and #{spaces.drop(MAX_SPACES_TO_LIST_ON_FAILURE).size}" \
+                  ' other spaces assigned this quota contain'
+                else
+                  "Spaces '#{named_spaces}' assigned this quota contain"
+                end + ' apps running with an unlimited log rate limit.'
+
+      raise Error.new("Current usage exceeds new quota values. #{message}")
+    end
+
+    private_class_method :spaces_with_unlimited_processes,
+                         :unlimited_processes_exist_error!
   end
 end
diff --git a/app/models/runtime/process_model.rb b/app/models/runtime/process_model.rb
@@ -34,6 +34,7 @@ def after_initialize
     NO_APP_PORT_SPECIFIED = -1
     DEFAULT_HTTP_PORT     = 8080
     DEFAULT_PORTS         = [DEFAULT_HTTP_PORT].freeze
+    UNLIMITED_LOG_RATE    = -1
 
     many_to_one :app, class: 'VCAP::CloudController::AppModel', key: :app_guid, primary_key: :guid, without_guid_generation: true
     many_to_one :revision, class: 'VCAP::CloudController::RevisionModel', key: :revision_guid, primary_key: :guid, without_guid_generation: true

diff --git a/spec/request/organization_quotas_spec.rb b/spec/request/organization_quotas_spec.rb
@@ -462,6 +462,20 @@ module VCAP::CloudController
           expect(last_response).to include_error_message("Organization Quota '#{organization_quota.name}' already exists.")
         end
       end
+
+      context 'when trying to set a log rate limit and there are apps with unlimited log rates' do
+        let!(:app_model) { VCAP::CloudController::AppModel.make(name: 'name1', space: space) }
+        let!(:process_model) { VCAP::CloudController::ProcessModel.make(app: app_model, log_rate_limit: -1) }
+
+        it 'returns 422' do
+          patch "/v3/organization_quotas/#{organization_quota.guid}", params.to_json, admin_header
+
+          expect(last_response).to have_status_code(422)
+          expect(last_response).to include_error_message(
+            'Current usage exceeds new quota values. ' \
+            "Org '#{org.name}' assigned this quota contains apps running with an unlimited log rate limit.")
+        end
+      end
     end
 
     describe 'POST /v3/organization_quotas/:guid/relationships/organizations' do
@@ -524,6 +538,19 @@ module VCAP::CloudController
           expect(parsed_response['errors'][0]['detail']).to eq('Invalid data type: Data[0] guid should be a string.')
         end
       end
+
+      context 'when the quota has a finite log rate limit and there are apps with unlimited log rates' do
+        let(:org_quota) { VCAP::CloudController::QuotaDefinition.make(log_rate_limit: 100) }
+        let!(:app_model) { VCAP::CloudController::AppModel.make(name: 'name1', space: space) }
+        let!(:process_model) { VCAP::CloudController::ProcessModel.make(app: app_model, log_rate_limit: -1) }
+
+        it 'returns 422' do
+          post "/v3/organization_quotas/#{org_quota.guid}/relationships/organizations", params.to_json, admin_header
+          expect(last_response).to have_status_code(422)
+          expect(last_response).to include_error_message(
+            'Current usage exceeds new quota values. The org(s) being assigned this quota contain apps running with an unlimited log rate limit.')
+        end
+      end
     end
 
     describe 'DELETE /v3/organization_quotas/:guid/' do

diff --git a/spec/request/space_quotas_spec.rb b/spec/request/space_quotas_spec.rb
@@ -260,6 +260,18 @@ module VCAP::CloudController
           expect(last_response).to include_error_message("Unknown field(s): 'wat'")
         end
       end
+      context 'when trying to set a log rate limit and there are apps with unlimited log rates' do
+        let!(:app_model) { VCAP::CloudController::AppModel.make(name: 'name1', space: space) }
+        let!(:process_model) { VCAP::CloudController::ProcessModel.make(app: app_model, log_rate_limit: -1) }
+
+        it 'returns 422' do
+          patch "/v3/space_quotas/#{space_quota.guid}", params.to_json, admin_header
+
+          expect(last_response).to have_status_code(422)
+          expect(last_response).to include_error_message(
+            "Current usage exceeds new quota values. Space '#{space.name}' assigned this quota contains apps running with an unlimited log rate limit.")
+        end
+      end
     end
 
     describe 'GET /v3/space_quotas' do
@@ -810,6 +822,19 @@ module VCAP::CloudController
           expect(parsed_response['errors'][0]['detail']).to eq('Invalid data type: Data[1] guid should be a string.')
         end
       end
+      context 'when the quota has a finite log rate limit and there are apps with unlimited log rates' do
+        let(:space_quota) { VCAP::CloudController::SpaceQuotaDefinition.make(guid: 'space-quota-guid', organization: org, log_rate_limit: 100) }
+        let!(:other_space) { VCAP::CloudController::Space.make(guid: 'other-space-guid', organization: org, space_quota_definition: space_quota) }
+        let!(:app_model) { VCAP::CloudController::AppModel.make(name: 'name1', space: other_space) }
+        let!(:process_model) { VCAP::CloudController::ProcessModel.make(app: app_model, log_rate_limit: -1) }
+
+        it 'returns 422' do
+          post "/v3/space_quotas/#{space_quota.guid}/relationships/spaces", params.to_json, admin_header
+          expect(last_response).to have_status_code(422)
+          expect(last_response).to include_error_message(
+            'Current usage exceeds new quota values. The space(s) being assigned this quota contain apps running with an unlimited log rate limit.')
+        end
+      end
     end
 
     describe 'DELETE /v3/space_quotas/:guid/relationships/spaces' do

diff --git a/spec/unit/actions/organization_quota_apply_spec.rb b/spec/unit/actions/organization_quota_apply_spec.rb
@@ -4,7 +4,7 @@
 
 module VCAP::CloudController
   RSpec.describe OrganizationQuotaApply do
-    describe '#create' do
+    describe '#apply' do
       subject { OrganizationQuotaApply.new }
 
       let(:org) { VCAP::CloudController::Organization.make }
@@ -53,6 +53,21 @@ module VCAP::CloudController
           }.to raise_error(OrganizationQuotaApply::Error, "Organizations with guids [\"#{invalid_org_guid}\"] do not exist")
         end
       end
+
+      context 'when trying to set a log rate limit and there are apps with unlimited log rates' do
+        let(:space) { VCAP::CloudController::Space.make(guid: 'space-guid', organization: org) }
+        let(:app_model) { VCAP::CloudController::AppModel.make(name: 'name1', space: space) }
+        let!(:process_model) { VCAP::CloudController::ProcessModel.make(app: app_model, log_rate_limit: -1) }
+        let(:org_quota) { VCAP::CloudController::QuotaDefinition.make(log_rate_limit: 2000) }
+
+        it 'raises an error' do
+          expect {
+            subject.apply(org_quota, message)
+          }.to raise_error(OrganizationQuotaApply::Error,
+            'Current usage exceeds new quota values. ' \
+            'The org(s) being assigned this quota contain apps running with an unlimited log rate limit.')
+        end
+      end
     end
   end
 end
diff --git a/spec/unit/actions/organization_quotas_update_spec.rb b/spec/unit/actions/organization_quotas_update_spec.rb
@@ -103,6 +103,67 @@ module VCAP::CloudController
             end
           end
         end
+
+        context 'when there are affected processes that have an unlimited log rate limit' do
+          def create_orgs_with_unlimited_log_rate_process(count)
+            count.downto(1) do |i|
+              org = VCAP::CloudController::Organization.make(guid: "org-guid-#{i}", name: "org-name-#{i}", quota_definition: org_quota)
+              space = VCAP::CloudController::Space.make(guid: "space-guid-#{i}", organization: org)
+              app_model = VCAP::CloudController::AppModel.make(name: "app-#{i}", space: space)
+              VCAP::CloudController::ProcessModel.make(app: app_model, log_rate_limit: -1)
+            end
+          end
+
+          context 'and they are only in a single org' do
+            before do
+              create_orgs_with_unlimited_log_rate_process(1)
+            end
+            it 'errors with a message telling the user the affected org' do
+              expect do
+                OrganizationQuotasUpdate.update(org_quota, message)
+              end.to raise_error(OrganizationQuotasUpdate::Error, "Current usage exceeds new quota values. Org 'org-name-1' " \
+                                 'assigned this quota contains apps running with an unlimited log rate limit.')
+            end
+          end
+          context 'and they are in two orgs' do
+            before do
+              create_orgs_with_unlimited_log_rate_process(2)
+            end
+            it 'errors with a message telling the user the affected orgs' do
+              expect do
+                OrganizationQuotasUpdate.update(org_quota, message)
+              end.to raise_error(OrganizationQuotasUpdate::Error, "Current usage exceeds new quota values. Orgs 'org-name-1', 'org-name-2' " \
+                                 'assigned this quota contain apps running with an unlimited log rate limit.')
+            end
+          end
+
+          context 'and they are spread across five orgs' do
+            before do
+              create_orgs_with_unlimited_log_rate_process(5)
+            end
+            it 'errors with a message telling the user some of the affected orgs and a total count' do
+              expect do
+                OrganizationQuotasUpdate.update(org_quota, message)
+              end.to raise_error(OrganizationQuotasUpdate::Error, "Current usage exceeds new quota values. Orgs 'org-name-1', 'org-name-2' and 3 other orgs " \
+                                 'assigned this quota contain apps running with an unlimited log rate limit.')
+            end
+          end
+
+          context 'and there is more than one affected process within an org' do
+            let(:org) { VCAP::CloudController::Organization.make(guid: 'org-guid', name: 'org-name', quota_definition: org_quota) }
+            let(:space) { VCAP::CloudController::Space.make(guid: 'space-guid', organization: org) }
+            let(:app_model) { VCAP::CloudController::AppModel.make(name: 'app', space: space) }
+            let!(:process_1) { VCAP::CloudController::ProcessModel.make(app: app_model, log_rate_limit: -1) }
+            let!(:process_2) { VCAP::CloudController::ProcessModel.make(app: app_model, log_rate_limit: -1) }
+
+            it 'only names the org once in the error message' do
+              expect do
+                OrganizationQuotasUpdate.update(org_quota, message)
+              end.to raise_error(OrganizationQuotasUpdate::Error, "Current usage exceeds new quota values. Org 'org-name' assigned this quota contains apps " \
+                                                                  'running with an unlimited log rate limit.')
+            end
+          end
+        end
       end
     end
   end

diff --git a/spec/unit/actions/space_quota_apply_spec.rb b/spec/unit/actions/space_quota_apply_spec.rb
@@ -62,6 +62,21 @@ module VCAP::CloudController
         end
       end
 
+      context 'when trying to set a log rate limit and there are apps with unlimited log rates' do
+        let(:visible_space_guids) { [space.guid] }
+        let(:app_model) { VCAP::CloudController::AppModel.make(name: 'name1', space: space) }
+        let!(:process_model) { VCAP::CloudController::ProcessModel.make(app: app_model, log_rate_limit: -1) }
+        let(:space_quota) { VCAP::CloudController::SpaceQuotaDefinition.make(organization: org, log_rate_limit: 2000) }
+
+        it 'raises an error' do
+          expect {
+            subject.apply(space_quota, message, visible_space_guids: visible_space_guids)
+          }.to raise_error(SpaceQuotaApply::Error,
+            'Current usage exceeds new quota values. ' \
+            'The space(s) being assigned this quota contain apps running with an unlimited log rate limit.')
+        end
+      end
+
       context "when the space is outside the space quota's org" do
         let(:other_space) { VCAP::CloudController::Space.make }
         let(:invalid_space_guid) { other_space.guid }