Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prevent quota updates or assignment with finite log rates #2948

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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