Skip to content

Commit

Permalink
Prevent quota updates or assignment with finite log rates (#2948)
Browse files Browse the repository at this point in the history
* 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.

* Prevent assignment of org and space quotas (v2)

Block assignment of org or space quotas via the v2 endpoint if the quota
has a finite log rate where the org or space contains processes that have
unlimited log rate limits.

Co-authored-by: Carson Long <lcarson@vmware.com>
  • Loading branch information
acrmp and ctlong authored Sep 13, 2022
1 parent 91c805e commit 6ebc911
Show file tree
Hide file tree
Showing 15 changed files with 387 additions and 5 deletions.
14 changes: 13 additions & 1 deletion app/actions/organization_quota_apply.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
40 changes: 40 additions & 0 deletions app/actions/organization_quotas_update.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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!

Expand Down Expand Up @@ -84,5 +94,35 @@ 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 = 'This quota is applied to ' +
if orgs.size == 1
"org '#{named_orgs}' which 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 which contain'
else
"orgs '#{named_orgs}' which 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
10 changes: 10 additions & 0 deletions app/actions/space_quota_apply.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
38 changes: 38 additions & 0 deletions app/actions/space_quota_update.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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!

Expand Down Expand Up @@ -79,5 +88,34 @@ 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 = 'This quota is applied to ' +
if spaces.size == 1
"space '#{named_spaces}' which 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 which contain'
else
"spaces '#{named_spaces}' which 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
12 changes: 12 additions & 0 deletions app/controllers/runtime/organizations_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,18 @@ def before_update(org)
end
end

if request_attrs['quota_definition_guid']
quota = QuotaDefinition.first(guid: request_attrs['quota_definition_guid'])
if quota.log_rate_limit != QuotaDefinition::UNLIMITED
affected_processes = org.processes_dataset
unless affected_processes.where(log_rate_limit: ProcessModel::UNLIMITED_LOG_RATE).empty?
raise CloudController::Errors::ApiError.new_from_details(
'UnprocessableEntity',
'Current usage exceeds new quota values. This org currently contains apps running with an unlimited log rate limit.')
end
end
end

super(org)
end

Expand Down
17 changes: 17 additions & 0 deletions app/controllers/runtime/space_quota_definitions_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,23 @@ def self.translate_validation_exception(e, attributes)
end
end

def before_update(quota)
if request_attrs['space'] && quota.log_rate_limit != QuotaDefinition::UNLIMITED
affected_processes = Space.dataset.
join(:apps, space_guid: :guid).
join(:processes, app_guid: :guid).
where(Sequel[:spaces][:guid] => request_attrs['space'])

unless affected_processes.where(log_rate_limit: ProcessModel::UNLIMITED_LOG_RATE).empty?
raise CloudController::Errors::ApiError.new_from_details(
'UnprocessableEntity',
'Current usage exceeds new quota values. This space currently contains apps running with an unlimited log rate limit.')
end
end

super(quota)
end

def delete(guid)
do_delete(find_guid_and_validate_access(:delete, guid))
end
Expand Down
1 change: 1 addition & 0 deletions app/models/runtime/process_model.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
27 changes: 27 additions & 0 deletions spec/request/organization_quotas_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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. ' \
"This quota is applied to org '#{org.name}' which contains apps running with an unlimited log rate limit.")
end
end
end

describe 'POST /v3/organization_quotas/:guid/relationships/organizations' do
Expand Down Expand Up @@ -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
Expand Down
25 changes: 25 additions & 0 deletions spec/request/space_quotas_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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. This quota is applied to space '#{space.name}' which contains apps running with an unlimited log rate limit.")
end
end
end

describe 'GET /v3/space_quotas' do
Expand Down Expand Up @@ -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
Expand Down
24 changes: 24 additions & 0 deletions spec/request/v2/organizations_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -99,4 +99,28 @@
)
end
end

describe 'PUT /v2/organizations/:guid' do
context 'when the quota has a finite log rate limit and there are apps with unlimited log rates' do
let(:admin_header) { headers_for(user, scopes: %w(cloud_controller.admin)) }
let(:org_quota) { VCAP::CloudController::QuotaDefinition.make(log_rate_limit: 100) }

let(:params) do
{
quota_definition_guid: org_quota.guid
}
end

let!(:space) { VCAP::CloudController::Space.make(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) }

it 'returns 422' do
put "/v2/organizations/#{org.guid}", params.to_json, admin_header
expect(last_response).to have_status_code(422)
expect(decoded_response['error_code']).to eq('CF-UnprocessableEntity')
expect(decoded_response['description']).to eq('Current usage exceeds new quota values. This org currently contains apps running with an unlimited log rate limit.')
end
end
end
end
24 changes: 24 additions & 0 deletions spec/request/v2/space_quota_definitions_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
require 'spec_helper'

RSpec.describe 'SpaceQuotaDefinitions' do
let(:user) { VCAP::CloudController::User.make }
let(:org) { VCAP::CloudController::Organization.make }

describe 'PUT /v2/space_quota_definitions/guid/spaces/space_guid' do
context 'when the quota has a finite log rate limit and there are apps with unlimited log rates' do
let(:admin_header) { headers_for(user, scopes: %w(cloud_controller.admin)) }
let(:space_quota) { VCAP::CloudController::SpaceQuotaDefinition.make(organization: org, log_rate_limit: 100) }

let!(:space) { VCAP::CloudController::Space.make(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) }

it 'returns 422' do
put "/v2/space_quota_definitions/#{space_quota.guid}/spaces/#{space.guid}", nil, admin_header
expect(last_response).to have_status_code(422)
expect(decoded_response['error_code']).to eq('CF-UnprocessableEntity')
expect(decoded_response['description']).to eq('Current usage exceeds new quota values. This space currently contains apps running with an unlimited log rate limit.')
end
end
end
end
17 changes: 16 additions & 1 deletion spec/unit/actions/organization_quota_apply_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
Expand Down Expand Up @@ -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
Loading

0 comments on commit 6ebc911

Please sign in to comment.