From 9a452ace11503c9ef3a082bfbd02767eb666bb5d Mon Sep 17 00:00:00 2001 From: Joseph Schorr Date: Tue, 20 Feb 2018 13:58:14 -0500 Subject: [PATCH] Add configurable limits for number of builds allowed under a namespace We also support that limit being increased automatically once a successful billing charge has gone through --- config.py | 13 ++++---- data/database.py | 2 ++ ..._add_maximum_build_queue_count_setting_.py | 26 ++++++++++++++++ data/model/user.py | 12 ++++++- data/queue.py | 13 ++++++++ endpoints/building.py | 20 +++++------- endpoints/test/test_building.py | 31 +++++++++++++++++++ endpoints/webhooks.py | 31 ++++++++++++------- test/testconfig.py | 1 + util/config/schema.py | 9 ++++-- 10 files changed, 125 insertions(+), 33 deletions(-) create mode 100644 data/migrations/versions/152bb29a1bb3_add_maximum_build_queue_count_setting_.py create mode 100644 endpoints/test/test_building.py diff --git a/config.py b/config.py index c0ac210e9..045997c62 100644 --- a/config.py +++ b/config.py @@ -451,12 +451,6 @@ class DefaultConfig(ImmutableConfig): # Location of the static marketing site. STATIC_SITE_BUCKET = None - # Count and duration used to produce a rate of builds allowed to be queued per repository before - # rejecting requests. Values less than zero disable rate limiting. - # Example: 10 builds per minute is accomplished by setting ITEMS = 10, SECS = 60 - MAX_BUILD_QUEUE_RATE_ITEMS = -1 - MAX_BUILD_QUEUE_RATE_SECS = -1 - # Site key and secret key for using recaptcha. FEATURE_RECAPTCHA = False RECAPTCHA_SITE_KEY = None @@ -507,3 +501,10 @@ class DefaultConfig(ImmutableConfig): # If enabled, ensures that API calls are made with the X-Requested-With header # when called from a browser. BROWSER_API_CALLS_XHR_ONLY = True + + # If set to a non-None integer value, the default number of maximum builds for a namespace. + DEFAULT_NAMESPACE_MAXIMUM_BUILD_COUNT = None + + # For Billing Support Only: The number of allowed builds on a namespace that has been billed + # successfully. + BILLED_NAMESPACE_MAXIMUM_BUILD_COUNT = None diff --git a/data/database.py b/data/database.py index 19bc24e11..fd19aa9db 100644 --- a/data/database.py +++ b/data/database.py @@ -439,6 +439,8 @@ class User(BaseModel): company = CharField(null=True) location = CharField(null=True) + maximum_queued_builds_count = IntegerField(null=True) + def delete_instance(self, recursive=False, delete_nullable=False): # If we are deleting a robot account, only execute the subset of queries necessary. if self.robot: diff --git a/data/migrations/versions/152bb29a1bb3_add_maximum_build_queue_count_setting_.py b/data/migrations/versions/152bb29a1bb3_add_maximum_build_queue_count_setting_.py new file mode 100644 index 000000000..e1003ec8e --- /dev/null +++ b/data/migrations/versions/152bb29a1bb3_add_maximum_build_queue_count_setting_.py @@ -0,0 +1,26 @@ +"""Add maximum build queue count setting to user table + +Revision ID: 152bb29a1bb3 +Revises: 7367229b38d9 +Create Date: 2018-02-20 13:34:34.902415 + +""" + +# revision identifiers, used by Alembic. +revision = '152bb29a1bb3' +down_revision = 'cbc8177760d9' + +from alembic import op +import sqlalchemy as sa +from sqlalchemy.dialects import mysql + +def upgrade(tables): + # ### commands auto generated by Alembic - please adjust! ### + op.add_column('user', sa.Column('maximum_queued_builds_count', sa.Integer(), nullable=True)) + # ### end Alembic commands ### + + +def downgrade(tables): + # ### commands auto generated by Alembic - please adjust! ### + op.drop_column('user', 'maximum_queued_builds_count') + # ### end Alembic commands ### diff --git a/data/model/user.py b/data/model/user.py index c1fd36165..763e7305e 100644 --- a/data/model/user.py +++ b/data/model/user.py @@ -80,7 +80,9 @@ def create_user_noverify(username, email, email_required=True, prompts=tuple()): try: default_expr_s = _convert_to_s(config.app_config['DEFAULT_TAG_EXPIRATION']) - new_user = User.create(username=username, email=email, removed_tag_expiration_s=default_expr_s) + default_max_builds = config.app_config.get('DEFAULT_NAMESPACE_MAXIMUM_BUILD_COUNT') + new_user = User.create(username=username, email=email, removed_tag_expiration_s=default_expr_s, + maximum_queued_builds_count=default_max_builds) for prompt in prompts: create_user_prompt(new_user, prompt) @@ -88,6 +90,14 @@ def create_user_noverify(username, email, email_required=True, prompts=tuple()): except Exception as ex: raise DataModelException(ex.message) +def increase_maximum_build_count(user, maximum_queued_builds_count): + """ Increases the maximum number of allowed builds on the namespace, if greater than that + already present. + """ + if (user.maximum_queued_builds_count is not None and + maximum_queued_builds_count > user.maximum_queued_builds_count): + user.maximum_queued_builds_count = maximum_queued_builds_count + user.save() def is_username_unique(test_username): try: diff --git a/data/queue.py b/data/queue.py index ed515cdba..985d5852e 100644 --- a/data/queue.py +++ b/data/queue.py @@ -74,6 +74,19 @@ class WorkQueue(object): ._available_jobs(now, name_match_query) .where(~(QueueItem.queue_name << running_query))) + def num_available_jobs(self, canonical_name_list): + """ + Returns the number of available queue items with a given prefix. + """ + def strip_slash(name): + return name.lstrip('/') + canonical_name_list = map(strip_slash, canonical_name_list) + + available = self._available_jobs(datetime.utcnow(), + '/'.join([self._queue_name] + canonical_name_list) + '%') + + return available.count() + def num_available_jobs_between(self, available_min_time, available_max_time, canonical_name_list): """ Returns the number of available queue items with a given prefix, between the two provided times. diff --git a/endpoints/building.py b/endpoints/building.py index 81bf984b8..8298f08ac 100644 --- a/endpoints/building.py +++ b/endpoints/building.py @@ -16,9 +16,6 @@ from util.morecollections import AttrDict logger = logging.getLogger(__name__) -MAX_BUILD_QUEUE_RATE_ITEMS = app.config.get('MAX_BUILD_QUEUE_RATE_ITEMS', -1) -MAX_BUILD_QUEUE_RATE_SECS = app.config.get('MAX_BUILD_QUEUE_RATE_SECS', -1) - class MaximumBuildsQueuedException(Exception): """ @@ -32,15 +29,14 @@ def start_build(repository, prepared_build, pull_robot_name=None): if repository.kind.name != 'image': raise Exception('Attempt to start a build for application repository %s' % repository.id) - if MAX_BUILD_QUEUE_RATE_ITEMS > 0 and MAX_BUILD_QUEUE_RATE_SECS > 0: - queue_item_canonical_name = [repository.namespace_user.username, repository.name] - now = datetime.utcnow() - available_min = now - timedelta(seconds=MAX_BUILD_QUEUE_RATE_SECS) - available_builds = dockerfile_build_queue.num_available_jobs_between(available_min, - now, - queue_item_canonical_name) - if available_builds >= MAX_BUILD_QUEUE_RATE_ITEMS: - raise MaximumBuildsQueuedException() + if repository.namespace_user.maximum_queued_builds_count is not None: + queue_item_canonical_name = [repository.namespace_user.username] + available_builds = dockerfile_build_queue.num_available_jobs(queue_item_canonical_name) + if available_builds >= repository.namespace_user.maximum_queued_builds_count: + logger.debug('Prevented queueing of build under namespace %s due to reaching max: %s', + repository.namespace_user.username, + repository.namespace_user.maximum_queued_builds_count) + raise MaximumBuildsQueuedException() host = app.config['SERVER_HOSTNAME'] repo_path = '%s/%s/%s' % (host, repository.namespace_user.username, repository.name) diff --git a/endpoints/test/test_building.py b/endpoints/test/test_building.py new file mode 100644 index 000000000..079bb8ca8 --- /dev/null +++ b/endpoints/test/test_building.py @@ -0,0 +1,31 @@ +import pytest + +from data import model +from endpoints.building import start_build, PreparedBuild, MaximumBuildsQueuedException + +from test.fixtures import * + +def test_maximum_builds(app): + # Change the maximum number of builds to 1. + user = model.user.create_user('foobar', 'password', 'foo@example.com') + user.maximum_queued_builds_count = 1 + user.save() + + repo = model.repository.create_repository('foobar', 'somerepo', user) + + # Try to queue a build; should succeed. + prepared_build = PreparedBuild() + prepared_build.build_name = 'foo' + prepared_build.is_manual = True + prepared_build.dockerfile_id = 'foobar' + prepared_build.archive_url = 'someurl' + prepared_build.tags = ['latest'] + prepared_build.subdirectory = '/' + prepared_build.context = '/' + prepared_build.metadata = {} + + start_build(repo, prepared_build) + + # Try to queue a second build; should fail. + with pytest.raises(MaximumBuildsQueuedException): + start_build(repo, prepared_build) diff --git a/endpoints/webhooks.py b/endpoints/webhooks.py index 370f5d20e..8b65e6b69 100644 --- a/endpoints/webhooks.py +++ b/endpoints/webhooks.py @@ -2,7 +2,7 @@ import logging from flask import request, make_response, Blueprint -from app import billing as stripe +from app import billing as stripe, app from data import model from auth.decorators import process_auth from auth.permissions import ModifyRepositoryPermission @@ -26,22 +26,29 @@ def stripe_webhook(): logger.debug('Stripe webhook call: %s', request_data) customer_id = request_data.get('data', {}).get('object', {}).get('customer', None) - user = model.user.get_user_or_org_by_customer_id(customer_id) if customer_id else None + namespace = model.user.get_user_or_org_by_customer_id(customer_id) if customer_id else None event_type = request_data['type'] if 'type' in request_data else None if event_type == 'charge.succeeded': invoice_id = request_data['data']['object']['invoice'] - if user and user.invoice_email: - # Lookup the invoice. - invoice = stripe.Invoice.retrieve(invoice_id) - if invoice: - invoice_html = renderInvoiceToHtml(invoice, user) - send_invoice_email(user.invoice_email_address or user.email, invoice_html) + namespace = model.user.get_user_or_org_by_customer_id(customer_id) if customer_id else None + if namespace: + # Increase the namespace's build allowance, since we had a successful charge. + build_maximum = app.config.get('BILLED_NAMESPACE_MAXIMUM_BUILD_COUNT') + if build_maximum is not None: + model.user.increase_maximum_build_count(namespace, build_maximum) + + if namespace.invoice_email: + # Lookup the invoice. + invoice = stripe.Invoice.retrieve(invoice_id) + if invoice: + invoice_html = renderInvoiceToHtml(invoice, namespace) + send_invoice_email(namespace.invoice_email_address or namespace.email, invoice_html) elif event_type.startswith('customer.subscription.'): - cust_email = user.email if user is not None else 'unknown@domain.com' - quay_username = user.username if user is not None else 'unknown' + cust_email = namespace.email if namespace is not None else 'unknown@domain.com' + quay_username = namespace.username if namespace is not None else 'unknown' change_type = '' if event_type.endswith('.deleted'): @@ -63,8 +70,8 @@ def stripe_webhook(): send_subscription_change(change_type, customer_id, cust_email, quay_username) elif event_type == 'invoice.payment_failed': - if user: - send_payment_failed(user.email, user.username) + if namespace: + send_payment_failed(namespace.email, namespace.username) return make_response('Okay') diff --git a/test/testconfig.py b/test/testconfig.py index e0d1e360f..c3d564239 100644 --- a/test/testconfig.py +++ b/test/testconfig.py @@ -101,3 +101,4 @@ class TestConfig(DefaultConfig): TAG_EXPIRATION_OPTIONS = ['0s', '1s', '1d', '1w', '2w', '4w'] + DEFAULT_NAMESPACE_MAXIMUM_BUILD_COUNT = None diff --git a/util/config/schema.py b/util/config/schema.py index 64b8e58f4..408099e0c 100644 --- a/util/config/schema.py +++ b/util/config/schema.py @@ -45,7 +45,6 @@ INTERNAL_ONLY_PROPERTIES = { 'SYSTEM_SERVICE_BLACKLIST', 'JWTPROXY_SIGNER', 'SECURITY_SCANNER_INDEXING_MIN_ID', - 'MAX_BUILD_QUEUE_RATE_SECS', 'STATIC_SITE_BUCKET', 'LABEL_KEY_RESERVED_PREFIXES', 'TEAM_SYNC_WORKER_FREQUENCY', @@ -65,13 +64,14 @@ INTERNAL_ONLY_PROPERTIES = { 'MAIL_FAIL_SILENTLY', 'LOCAL_OAUTH_HANDLER', 'USE_CDN', - 'MAX_BUILD_QUEUE_RATE_ITEMS', 'ANALYTICS_TYPE', 'EXCEPTION_LOG_TYPE', 'SENTRY_DSN', 'SENTRY_PUBLIC_DSN', + 'BILLED_NAMESPACE_MAXIMUM_BUILD_COUNT', + 'SECURITY_SCANNER_ENDPOINT_BATCH', 'SECURITY_SCANNER_API_TIMEOUT_SECONDS', 'SECURITY_SCANNER_API_TIMEOUT_POST_SECONDS', @@ -699,6 +699,11 @@ CONFIG_SCHEMA = { 'description': 'Whether to support Dockerfile build. Defaults to True', 'x-example': True, }, + 'DEFAULT_NAMESPACE_MAXIMUM_BUILD_COUNT': { + 'type': ['number', 'null'], + 'description': 'If not None, the default maximum number of builds that can be queued in a namespace.', + 'x-example': 20, + }, # Login 'FEATURE_GITHUB_LOGIN': {