From de246b93924d897c24882e15e4b0afbc2bc83a2e Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Thu, 12 Nov 2020 18:24:24 -0500 Subject: [PATCH] Allow to build an external version based on its source and base branch. - Use a namedtuple to pass more data about the external version around - Automation rules can now receive extra kwargs to be passed down to the match and action functions - Automation rules now return a tuple, the first element indicate if rule matched and the second is the return value from the action, this was done, so we can know if the build was triggered or not for external versions. - Moved the actions to the base class, since it doesn't look like we are going to have a different set of actions - If the user doesn't have automation rules for external versions we just build everything as before, but if the user has at least one, we use the rules to trigger the build. Ref https://github.com/readthedocs/readthedocs.org/pull/7653 --- docs/automation-rules.rst | 31 +++++- .../autobuild-docs-for-pull-requests.rst | 5 + readthedocs/api/v2/views/integrations.py | 53 ++++++---- readthedocs/builds/automation_actions.py | 24 +++++ readthedocs/builds/forms.py | 51 ++++++++- readthedocs/builds/managers.py | 6 +- .../0029_add_build_external_version_action.py | 18 ++++ readthedocs/builds/models.py | 100 ++++++++---------- readthedocs/builds/utils.py | 48 +++++++-- readthedocs/core/views/hooks.py | 56 ++++++---- .../projects/js/automation-rules.js | 71 ++++++++++++- .../static/projects/js/automation-rules.js | 2 +- readthedocs/rtd_tests/tests/test_api.py | 97 +++++++++++++++-- .../rtd_tests/tests/test_automation_rules.py | 56 ++++++++-- 14 files changed, 483 insertions(+), 135 deletions(-) create mode 100644 readthedocs/builds/migrations/0029_add_build_external_version_action.py diff --git a/docs/automation-rules.rst b/docs/automation-rules.rst index 66ce2896ac2..ac453aa14ac 100644 --- a/docs/automation-rules.rst +++ b/docs/automation-rules.rst @@ -50,6 +50,9 @@ Actions When a rule matches a new version, the specified action is performed on that version. Currently, the following actions are available: +For branches and tags +~~~~~~~~~~~~~~~~~~~~~ + - **Activate version**: Activates and builds the version. - **Hide version**: Hides the version. If the version is not active, activates it and builds the version. See :ref:`versions:Version States`. @@ -71,7 +74,6 @@ Currently, the following actions are available: You can use the ``Set version as default`` action to change the default version before deleting the current one. - .. note:: If your versions follow :pep:`440`, @@ -79,6 +81,17 @@ Currently, the following actions are available: The stable version is also automatically updated at the same time. See more in :doc:`versions`. +For pull/merge requests +~~~~~~~~~~~~~~~~~~~~~~~ + +- **Build version**: When a pull request is opened or re-opened, + build the version if it matches the source and base branch. + +.. note:: + + By default Read the Docs will build all versions from pull requests + if there aren't any rules for external versions. + Order ----- @@ -141,6 +154,22 @@ Activate all new tags and branches that start with ``v`` or ``V`` - Version type: ``Branch`` - Action: ``Activate version`` +Build pull requests from the ``main`` branch only +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +- Custom match: ``.*`` +- Version type: ``External`` +- Base branch: ``^main$`` +- Action: ``Build version`` + +Build all pull request where the source branch has the ``docs/`` prefix +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +- Custom match: ``^docs/`` +- Version type: ``External`` +- Base branch: ``.*`` +- Action: ``Build version`` + Activate all new tags that don't contain the ``-nightly`` suffix ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ diff --git a/docs/guides/autobuild-docs-for-pull-requests.rst b/docs/guides/autobuild-docs-for-pull-requests.rst index cb289d8f3b1..65cfe7c99da 100644 --- a/docs/guides/autobuild-docs-for-pull-requests.rst +++ b/docs/guides/autobuild-docs-for-pull-requests.rst @@ -39,6 +39,11 @@ Features and after the build has finished we send success notification if the build succeeded without any error or failure notification if the build failed. +- **Build based on the source and base branch**: By default we build all pull requests, + but you can setup rules to build only from a given base branch, + or from pull requests where the source branch matches a pattern. + See :doc:`/automation-rules`. + .. figure:: ../_static/images/guides/github-build-status-reporting.gif :align: center :alt: GitHub Build Status Reporting for Pull Requests. diff --git a/readthedocs/api/v2/views/integrations.py b/readthedocs/api/v2/views/integrations.py index 884887701fb..c4d06d01e3d 100644 --- a/readthedocs/api/v2/views/integrations.py +++ b/readthedocs/api/v2/views/integrations.py @@ -3,6 +3,7 @@ import hashlib import hmac import json +from functools import namedtuple import logging import re @@ -56,6 +57,12 @@ BITBUCKET_PUSH = 'repo:push' +ExternalVersionData = namedtuple( + 'ExternalVersionData', + ['id', 'source_branch', 'base_branch', 'commit'], +) + + class WebhookMixin: """Base class for Webhook mixins.""" @@ -227,20 +234,20 @@ def get_external_version_response(self, project): :param project: Project instance :type project: readthedocs.projects.models.Project """ - identifier, verbose_name = self.get_external_version_data() + version_data = self.get_external_version_data() # create or get external version object using `verbose_name`. - external_version = get_or_create_external_version( - project, identifier, verbose_name - ) + external_version = get_or_create_external_version(project, version_data) # returns external version verbose_name (pull/merge request number) to_build = build_external_version( - project=project, version=external_version, commit=identifier + project=project, + version=external_version, + version_data=version_data, ) return { - 'build_triggered': True, + 'build_triggered': bool(to_build), 'project': project.slug, - 'versions': [to_build], + 'versions': [to_build] if to_build else [], } def get_delete_external_version_response(self, project): @@ -258,11 +265,9 @@ def get_delete_external_version_response(self, project): :param project: Project instance :type project: Project """ - identifier, verbose_name = self.get_external_version_data() + version_data = self.get_external_version_data() # Delete external version - deleted_version = delete_external_version( - project, identifier, verbose_name - ) + deleted_version = delete_external_version(project, version_data) return { 'version_deleted': deleted_version is not None, 'project': project.slug, @@ -320,13 +325,16 @@ def get_data(self): def get_external_version_data(self): """Get Commit Sha and pull request number from payload.""" try: - identifier = self.data['pull_request']['head']['sha'] - verbose_name = str(self.data['number']) - - return identifier, verbose_name + data = ExternalVersionData( + id=str(self.data['number']), + commit=self.data['pull_request']['head']['sha'], + source_branch=self.data['pull_request']['head']['ref'], + base_branch=self.data['pull_request']['base']['ref'], + ) + return data except KeyError: - raise ParseError('Parameters "sha" and "number" are required') + raise ParseError('Invalid payload') def is_payload_valid(self): """ @@ -530,13 +538,16 @@ def is_payload_valid(self): def get_external_version_data(self): """Get commit SHA and merge request number from payload.""" try: - identifier = self.data['object_attributes']['last_commit']['id'] - verbose_name = str(self.data['object_attributes']['iid']) - - return identifier, verbose_name + data = ExternalVersionData( + id=str(self.data['object_attributes']['iid']), + commit=self.data['object_attributes']['last_commit']['id'], + source_branch=self.data['object_attributes']['source_branch'], + base_branch=self.data['object_attributes']['target_branch'], + ) + return data except KeyError: - raise ParseError('Parameters "id" and "iid" are required') + raise ParseError('Invalid payload') def handle_webhook(self): """ diff --git a/readthedocs/builds/automation_actions.py b/readthedocs/builds/automation_actions.py index 2be65058334..3e8e38e61ae 100644 --- a/readthedocs/builds/automation_actions.py +++ b/readthedocs/builds/automation_actions.py @@ -10,6 +10,7 @@ import logging +from readthedocs.builds.utils import match_regex from readthedocs.core.utils import trigger_build from readthedocs.projects.constants import PRIVATE, PUBLIC @@ -31,6 +32,29 @@ def activate_version(version, match_result, action_arg, *args, **kwargs): ) +def build_external_version(version, match_result, action_arg, version_data, **kwargs): + """ + Build an external version if matches the given base branch. + + :param action_arg: A pattern to match the base branch. + :param version_data: `ExternalVersionData` instance. + :returns: A boolean indicating if the build was triggered. + """ + base_branch_regex = action_arg + result = match_regex( + base_branch_regex, + version_data.base_branch, + ) + if result: + trigger_build( + project=version.project, + version=version, + commit=version.identifier, + ) + return True + return False + + def set_default_version(version, match_result, action_arg, *args, **kwargs): """ Sets version as the project's default version. diff --git a/readthedocs/builds/forms.py b/readthedocs/builds/forms.py index 4d71211a9a6..700e8d15a5f 100644 --- a/readthedocs/builds/forms.py +++ b/readthedocs/builds/forms.py @@ -1,5 +1,6 @@ """Django forms for the builds app.""" +import itertools import re import textwrap @@ -14,6 +15,8 @@ ALL_VERSIONS, BRANCH, BRANCH_TEXT, + EXTERNAL, + EXTERNAL_TEXT, TAG, TAG_TEXT, ) @@ -99,8 +102,9 @@ class RegexAutomationRuleForm(forms.ModelForm): """ A regular expression to match the version. - Check the documentation for valid patterns. + Check the documentation + for valid patterns. """ )), required=False, @@ -113,6 +117,7 @@ class Meta: 'predefined_match_arg', 'match_arg', 'version_type', + 'action_arg', 'action', ] # Don't pollute the UI with help texts @@ -133,6 +138,7 @@ def __init__(self, *args, **kwargs): (None, '-' * 9), (BRANCH, BRANCH_TEXT), (TAG, TAG_TEXT), + (EXTERNAL, EXTERNAL_TEXT), ] # Remove privacy actions not available in community @@ -155,6 +161,48 @@ def __init__(self, *args, **kwargs): if self.instance.pk and self.instance.predefined_match_arg: self.initial['match_arg'] = self.instance.get_match_arg() + def clean_action(self): + """Check the action is allowed for the type of version.""" + action = self.cleaned_data['action'] + version_type = self.cleaned_data['version_type'] + internal_allowed_actions = set( + itertools.chain( + VersionAutomationRule.allowed_actions_on_create.keys(), + VersionAutomationRule.allowed_actions_on_delete.keys(), + ) + ) + allowed_actions = { + BRANCH: internal_allowed_actions, + TAG: internal_allowed_actions, + EXTERNAL: set(VersionAutomationRule.allowed_actions_on_external_versions.keys()), + } + if action not in allowed_actions.get(version_type, []): + raise forms.ValidationError( + _('Invalid action for this version type.'), + ) + return action + + def clean_action_arg(self): + """ + Validate the action argument. + + Currently only external versions accept this argument, + and it's the pattern to match the base_branch. + """ + action_arg = self.cleaned_data['action_arg'] + version_type = self.cleaned_data['version_type'] + if version_type == EXTERNAL: + if not action_arg: + action_arg = '.*' + try: + re.compile(action_arg) + return action_arg + except Exception: + raise forms.ValidationError( + _('Invalid Python regular expression.'), + ) + return '' + def clean_match_arg(self): """Check that a custom match was given if a predefined match wasn't used.""" match_arg = self.cleaned_data['match_arg'] @@ -185,6 +233,7 @@ def save(self, commit=True): predefined_match_arg=self.cleaned_data['predefined_match_arg'], version_type=self.cleaned_data['version_type'], action=self.cleaned_data['action'], + action_arg=self.cleaned_data['action_arg'], ) if not rule.description: rule.description = rule.get_description() diff --git a/readthedocs/builds/managers.py b/readthedocs/builds/managers.py index 5c9ccba0bc7..77860187803 100644 --- a/readthedocs/builds/managers.py +++ b/readthedocs/builds/managers.py @@ -194,8 +194,7 @@ class VersionAutomationRuleManager(PolymorphicManager): """ def add_rule( - self, *, project, description, match_arg, version_type, - action, action_arg=None, predefined_match_arg=None, + self, *, project, description, match_arg, version_type, action, **kwargs, ): """ Append an automation rule to `project`. @@ -219,9 +218,8 @@ def add_rule( priority=priority, description=description, match_arg=match_arg, - predefined_match_arg=predefined_match_arg, version_type=version_type, action=action, - action_arg=action_arg, + **kwargs, ) return rule diff --git a/readthedocs/builds/migrations/0029_add_build_external_version_action.py b/readthedocs/builds/migrations/0029_add_build_external_version_action.py new file mode 100644 index 00000000000..4e7d74ef2d4 --- /dev/null +++ b/readthedocs/builds/migrations/0029_add_build_external_version_action.py @@ -0,0 +1,18 @@ +# Generated by Django 2.2.16 on 2020-11-16 17:37 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('builds', '0028_add_delete_version_action'), + ] + + operations = [ + migrations.AlterField( + model_name='versionautomationrule', + name='action', + field=models.CharField(choices=[('activate-version', 'Activate version'), ('hide-version', 'Hide version'), ('make-version-public', 'Make version public'), ('make-version-private', 'Make version private'), ('set-default-version', 'Set version as default'), ('delete-version', 'Delete version (on branch/tag deletion)'), ('build-external-version', 'Build version')], help_text='Action to apply to matching versions', max_length=32, verbose_name='Action'), + ), + ] diff --git a/readthedocs/builds/models.py b/readthedocs/builds/models.py index 749cff0a8df..ca5f9432072 100644 --- a/readthedocs/builds/models.py +++ b/readthedocs/builds/models.py @@ -6,7 +6,6 @@ import re from shutil import rmtree -import regex from django.conf import settings from django.core.files.storage import get_storage_class from django.db import models @@ -58,6 +57,7 @@ get_bitbucket_username_repo, get_github_username_repo, get_gitlab_username_repo, + match_regex, ) from readthedocs.builds.version_slug import VersionSlugField from readthedocs.config import LATEST_CONFIGURATION_VERSION @@ -952,6 +952,7 @@ class VersionAutomationRule(PolymorphicModel, TimeStampedModel): MAKE_VERSION_PUBLIC_ACTION = 'make-version-public' MAKE_VERSION_PRIVATE_ACTION = 'make-version-private' SET_DEFAULT_VERSION_ACTION = 'set-default-version' + BUILD_EXTERNAL_VERSION = 'build-external-version' ACTIONS = ( (ACTIVATE_VERSION_ACTION, _('Activate version')), @@ -960,10 +961,24 @@ class VersionAutomationRule(PolymorphicModel, TimeStampedModel): (MAKE_VERSION_PRIVATE_ACTION, _('Make version private')), (SET_DEFAULT_VERSION_ACTION, _('Set version as default')), (DELETE_VERSION_ACTION, _('Delete version (on branch/tag deletion)')), + (BUILD_EXTERNAL_VERSION, _('Build version')), ) - allowed_actions_on_create = {} - allowed_actions_on_delete = {} + allowed_actions_on_create = { + ACTIVATE_VERSION_ACTION: actions.activate_version, + HIDE_VERSION_ACTION: actions.hide_version, + MAKE_VERSION_PUBLIC_ACTION: actions.set_public_privacy_level, + MAKE_VERSION_PRIVATE_ACTION: actions.set_private_privacy_level, + SET_DEFAULT_VERSION_ACTION: actions.set_default_version, + } + + allowed_actions_on_delete = { + DELETE_VERSION_ACTION: actions.delete_version, + } + + allowed_actions_on_external_versions = { + BUILD_EXTERNAL_VERSION: actions.build_external_version, + } project = models.ForeignKey( Project, @@ -1030,32 +1045,35 @@ def get_match_arg(self): ) return match_arg or self.match_arg - def run(self, version, *args, **kwargs): + def run(self, version, **kwargs): """ Run an action if `version` matches the rule. :type version: readthedocs.builds.models.Version - :returns: True if the action was performed + :param kwargs: All extra keywords will be passed to the match and action functions. + :returns: A tuple of (boolean, ANY), where the first element + indicates if the action was performed, and the second is the result + returned by the action. """ if version.type == self.version_type: - match, result = self.match(version, self.get_match_arg()) + match, result = self.match(version, self.get_match_arg(), **kwargs) if match: - self.apply_action(version, result) - return True - return False + action_result = self.apply_action(version, result, **kwargs) + return True, action_result + return False, None - def match(self, version, match_arg): + def match(self, version, match_arg, **kwargs): """ Returns True and the match result if the version matches the rule. :type version: readthedocs.builds.models.Version :param str match_arg: Additional argument to perform the match - :returns: A tuple of (boolean, match_resul). + :returns: A tuple of (boolean, match_result). The result will be passed to `apply_action`. """ return False, None - def apply_action(self, version, match_result): + def apply_action(self, version, match_result, **kwargs): """ Apply the action from allowed_actions_on_*. @@ -1067,10 +1085,16 @@ def apply_action(self, version, match_result): action = ( self.allowed_actions_on_create.get(self.action) or self.allowed_actions_on_delete.get(self.action) + or self.allowed_actions_on_external_versions.get(self.action) ) if action is None: raise NotImplementedError - action(version, match_result, self.action_arg) + return action( + version=version, + match_result=match_result, + action_arg=self.action_arg, + **kwargs, + ) def move(self, steps): """ @@ -1172,52 +1196,16 @@ def __str__(self): class RegexAutomationRule(VersionAutomationRule): - TIMEOUT = 1 # timeout in seconds - - allowed_actions_on_create = { - VersionAutomationRule.ACTIVATE_VERSION_ACTION: actions.activate_version, - VersionAutomationRule.HIDE_VERSION_ACTION: actions.hide_version, - VersionAutomationRule.MAKE_VERSION_PUBLIC_ACTION: actions.set_public_privacy_level, - VersionAutomationRule.MAKE_VERSION_PRIVATE_ACTION: actions.set_private_privacy_level, - VersionAutomationRule.SET_DEFAULT_VERSION_ACTION: actions.set_default_version, - } - - allowed_actions_on_delete = { - VersionAutomationRule.DELETE_VERSION_ACTION: actions.delete_version, - } - class Meta: proxy = True - def match(self, version, match_arg): - """ - Find a match using regex.search. - - .. note:: - - We use the regex module with the timeout - arg to avoid ReDoS. - - We could use a finite state machine type of regex too, - but there isn't a stable library at the time of writting this code. - """ - try: - match = regex.search( - match_arg, - version.verbose_name, - # Compatible with the re module - flags=regex.VERSION0, - timeout=self.TIMEOUT, - ) - return bool(match), match - except TimeoutError: - log.warning( - 'Timeout while parsing regex. pattern=%s, input=%s', - match_arg, version.verbose_name, - ) - except Exception as e: - log.info('Error parsing regex: %s', e) - return False, None + def match(self, version, match_arg, **kwargs): + version_name = version.verbose_name + if version.is_external: + version_data = kwargs['version_data'] + version_name = version_data.source_branch + result = match_regex(match_arg, version_name) + return bool(result), result def get_edit_url(self): return reverse( diff --git a/readthedocs/builds/utils.py b/readthedocs/builds/utils.py index 0702e460690..0274f188b19 100644 --- a/readthedocs/builds/utils.py +++ b/readthedocs/builds/utils.py @@ -1,7 +1,8 @@ """Utilities for the builds app.""" - +import logging from contextlib import contextmanager +import regex from celery.five import monotonic from django.core.cache import cache @@ -11,13 +12,15 @@ GITLAB_REGEXS, ) +log = logging.getLogger(__name__) + LOCK_EXPIRE = 60 * 180 # Lock expires in 3 hours def get_github_username_repo(url): if 'github' in url: - for regex in GITHUB_REGEXS: - match = regex.search(url) + for pattern in GITHUB_REGEXS: + match = pattern.search(url) if match: return match.groups() return (None, None) @@ -25,8 +28,8 @@ def get_github_username_repo(url): def get_bitbucket_username_repo(url=None): if 'bitbucket' in url: - for regex in BITBUCKET_REGEXS: - match = regex.search(url) + for pattern in BITBUCKET_REGEXS: + match = pattern.search(url) if match: return match.groups() return (None, None) @@ -34,8 +37,8 @@ def get_bitbucket_username_repo(url=None): def get_gitlab_username_repo(url=None): if 'gitlab' in url: - for regex in GITLAB_REGEXS: - match = regex.search(url) + for pattern in GITLAB_REGEXS: + match = pattern.search(url) if match: return match.groups() return (None, None) @@ -61,3 +64,34 @@ def memcache_lock(lock_id, oid): # to lessen the chance of releasing an expired lock # owned by someone else. cache.delete(lock_id) + + +def match_regex(pattern, text, timeout=1): + """ + Find a match using regex.search. + + .. note:: + + We use the regex module with the timeout + arg to avoid ReDoS. + + We could use a finite state machine type of regex too, + but there isn't a stable library at the time of writting this code. + """ + try: + match = regex.search( + pattern, + text, + # Compatible with the re module + flags=regex.VERSION0, + timeout=timeout, + ) + return match + except TimeoutError: + log.warning( + 'Timeout while parsing regex. pattern=%s, input=%s', + pattern, text, + ) + except Exception as e: + log.info('Error parsing regex: %s', e) + return None diff --git a/readthedocs/core/views/hooks.py b/readthedocs/core/views/hooks.py index f08770d44ef..a3105af9ddd 100644 --- a/readthedocs/core/views/hooks.py +++ b/readthedocs/core/views/hooks.py @@ -3,11 +3,11 @@ import logging from readthedocs.builds.constants import EXTERNAL +from readthedocs.builds.models import RegexAutomationRule from readthedocs.core.utils import trigger_build -from readthedocs.projects.models import Project, Feature +from readthedocs.projects.models import Feature, Project from readthedocs.projects.tasks import sync_repository_task - log = logging.getLogger(__name__) @@ -119,7 +119,7 @@ def trigger_sync_versions(project): return None -def get_or_create_external_version(project, identifier, verbose_name): +def get_or_create_external_version(project, version_data): """ Get or create external versions using `identifier` and `verbose_name`. @@ -128,13 +128,13 @@ def get_or_create_external_version(project, identifier, verbose_name): :param project: Project instance :param identifier: Commit Hash :param verbose_name: pull/merge request number - :returns: External version. + :returns: External version. :rtype: Version """ external_version, created = project.versions.get_or_create( - verbose_name=verbose_name, + verbose_name=version_data.id, type=EXTERNAL, - defaults={'identifier': identifier, 'active': True}, + defaults={'identifier': version_data.commit, 'active': True}, ) if created: @@ -144,20 +144,19 @@ def get_or_create_external_version(project, identifier, verbose_name): ) else: # identifier will change if there is a new commit to the Pull/Merge Request - if external_version.identifier != identifier: - external_version.identifier = identifier + if external_version.identifier != version_data.commit: + external_version.identifier = version_data.commit external_version.save() - log.info( - '(Update External Version) Updated Version: [%s] ', - external_version.slug + 'Commit from external version updated. project=%s version=%s', + project.slug, external_version.slug, ) return external_version -def delete_external_version(project, identifier, verbose_name): +def delete_external_version(project, version_data): """ - Delete external versions using `identifier` and `verbose_name`. + Delete external versions using the id and latest commit. if external version does not exist then returns `None`. @@ -167,9 +166,13 @@ def delete_external_version(project, identifier, verbose_name): :returns: verbose_name (pull/merge request number). :rtype: str """ - external_version = project.versions(manager=EXTERNAL).filter( - verbose_name=verbose_name, identifier=identifier - ).first() + external_version = ( + project.versions(manager=EXTERNAL) + .filter( + verbose_name=version_data.id, + identifier=version_data.commit, + ).first() + ) if external_version: # Delete External Version @@ -183,11 +186,14 @@ def delete_external_version(project, identifier, verbose_name): return None -def build_external_version(project, version, commit): +def build_external_version(project, version, version_data): """ Where we actually trigger builds for external versions. All pull/merge request webhook logic should route here to call ``trigger_build``. + + If there is at least one automation rule for external versions, + we leverage the build to the rules instead. """ if not project.has_valid_webhook: project.has_valid_webhook = True @@ -199,6 +205,16 @@ def build_external_version(project, version, commit): project.slug, version.slug, ) - trigger_build(project=project, version=version, commit=commit, force=True) - - return version.verbose_name + version_built = None + allowed_actions = RegexAutomationRule.allowed_actions_on_external_versions + rules = project.automation_rules.filter(action__in=allowed_actions) + + if not rules: + trigger_build(project=project, version=version, commit=version.identifier, force=True) + version_built = version.verbose_name + + for rule in rules: + _, built = rule.run(version, version_data=version_data) + if built: + version_built = version.verbose_name + return version_built diff --git a/readthedocs/projects/static-src/projects/js/automation-rules.js b/readthedocs/projects/static-src/projects/js/automation-rules.js index 1debaf8989a..569d36f41ae 100644 --- a/readthedocs/projects/static-src/projects/js/automation-rules.js +++ b/readthedocs/projects/static-src/projects/js/automation-rules.js @@ -2,7 +2,8 @@ var $ = require('jquery'); -function set_help_text(value) { + +function set_match_help_text(value) { var help_texts = { 'all-versions': 'All versions will be matched.', 'semver-versions': 'Versions incremented based on semantic versioning rules will be matched.', @@ -11,12 +12,58 @@ function set_help_text(value) { $('#id_predefined_match_arg').siblings('.helptext').text(help_texts[value]); } +function set_action_help_text() { + var action_arg = $('#id_action_arg'); + if (action_arg.val() === '') { + action_arg.val('^main$'); + } + action_arg.siblings('label').text('Base branch:'); + action_arg.siblings('.helptext').html( + 'Pattern to match the base branch. ' + + '' + + 'Check the documentation' + + ' ' + + 'for valid patterns.' + ); +} + + +function reset_actions(type) { + var actions = { + '': '---------', + 'activate-version': 'Activate version', + 'delete-version': 'Hide version', + 'hide-version': 'Make version public', + 'make-version-public': 'Make version private', + 'make-version-private': 'Set version as default', + 'set-default-version': 'Delete version (on branch/tag deletion)', + }; + if (type === 'external') { + actions = { + '': '---------', + 'build-external-version': 'Build version', + }; + } + var element = $('#id_action'); + var current = $('#id_action :selected').val(); + element.empty(); + $.each(actions, function (key, value) { + var option = $("").attr("value", key).text(value); + if (key === current) { + option.attr('selected', true); + } + element.append(option); + }); +} + + $(function () { + // Match var value = $('#id_predefined_match_arg').val(); if (value !== '') { $('#id_match_arg').parent().hide(); } - set_help_text(value); + set_match_help_text(value); $('#id_predefined_match_arg').bind('change', function (ev) { if (this.value === '') { @@ -24,6 +71,24 @@ $(function () { } else { $('#id_match_arg').parent().hide(); } - set_help_text(this.value); + set_match_help_text(this.value); + }); + + // Action argument + var type = $('#id_version_type').val(); + if (type !== 'external') { + $('#id_action_arg').parent().hide(); + } + set_action_help_text(); + reset_actions(type); + + $('#id_version_type').bind('change', function (e) { + if (this.value === 'external') { + set_action_help_text(); + $('#id_action_arg').parent().show(); + } else { + $('#id_action_arg').parent().hide(); + } + reset_actions(this.value); }); }); diff --git a/readthedocs/projects/static/projects/js/automation-rules.js b/readthedocs/projects/static/projects/js/automation-rules.js index 8250f423e25..9fb05a377df 100644 --- a/readthedocs/projects/static/projects/js/automation-rules.js +++ b/readthedocs/projects/static/projects/js/automation-rules.js @@ -1 +1 @@ -require=function a(o,u,s){function c(r,e){if(!u[r]){if(!o[r]){var n="function"==typeof require&&require;if(!e&&n)return n(r,!0);if(d)return d(r,!0);var i=new Error("Cannot find module '"+r+"'");throw i.code="MODULE_NOT_FOUND",i}var t=u[r]={exports:{}};o[r][0].call(t.exports,function(e){return c(o[r][1][e]||e)},t,t.exports,a,o,u,s)}return u[r].exports}for(var d="function"==typeof require&&require,e=0;eCheck the documentation for valid patterns.')}function o(e){var t={"":"---------","activate-version":"Activate version","delete-version":"Hide version","hide-version":"Make version public","make-version-public":"Make version private","make-version-private":"Set version as default","set-default-version":"Delete version (on branch/tag deletion)"};"external"===e&&(t={"":"---------","build-external-version":"Build version"});var r=a("#id_action"),n=a("#id_action :selected").val();r.empty(),a.each(t,function(e,t){var i=a("").attr("value",e).text(t);e===n&&i.attr("selected",!0),r.append(i)})}a(function(){var e=a("#id_predefined_match_arg").val();""!==e&&a("#id_match_arg").parent().hide(),r(e),a("#id_predefined_match_arg").bind("change",function(e){""===this.value?a("#id_match_arg").parent().show():a("#id_match_arg").parent().hide(),r(this.value)});var t=a("#id_version_type").val();"external"!==t&&a("#id_action_arg").parent().hide(),n(),o(t),a("#id_version_type").bind("change",function(e){"external"===this.value?(n(),a("#id_action_arg").parent().show()):a("#id_action_arg").parent().hide(),o(this.value)})})},{jquery:"jquery"}]},{},[]); \ No newline at end of file diff --git a/readthedocs/rtd_tests/tests/test_api.py b/readthedocs/rtd_tests/tests/test_api.py index dc4298ae4a5..0ada7e691ee 100644 --- a/readthedocs/rtd_tests/tests/test_api.py +++ b/readthedocs/rtd_tests/tests/test_api.py @@ -1,8 +1,8 @@ import base64 import datetime import json - from unittest import mock + from allauth.socialaccount.models import SocialAccount from django.contrib.auth.models import User from django.http import QueryDict @@ -16,13 +16,13 @@ GITHUB_CREATE, GITHUB_DELETE, GITHUB_EVENT_HEADER, - GITHUB_PUSH, - GITHUB_SIGNATURE_HEADER, GITHUB_PULL_REQUEST, + GITHUB_PULL_REQUEST_CLOSED, GITHUB_PULL_REQUEST_OPENED, GITHUB_PULL_REQUEST_REOPENED, - GITHUB_PULL_REQUEST_CLOSED, GITHUB_PULL_REQUEST_SYNC, + GITHUB_PUSH, + GITHUB_SIGNATURE_HEADER, GITLAB_MERGE_REQUEST, GITLAB_MERGE_REQUEST_CLOSE, GITLAB_MERGE_REQUEST_MERGE, @@ -37,8 +37,14 @@ GitLabWebhookView, ) from readthedocs.api.v2.views.task_views import get_status_data -from readthedocs.builds.constants import LATEST, EXTERNAL -from readthedocs.builds.models import Build, BuildCommandResult, Version +from readthedocs.builds.constants import EXTERNAL, LATEST +from readthedocs.builds.models import ( + Build, + BuildCommandResult, + RegexAutomationRule, + Version, + VersionAutomationRule, +) from readthedocs.integrations.models import Integration from readthedocs.oauth.models import RemoteOrganization, RemoteRepository from readthedocs.projects.models import ( @@ -48,7 +54,6 @@ Project, ) - super_auth = base64.b64encode(b'super:test').decode('utf-8') eric_auth = base64.b64encode(b'eric:test').decode('utf-8') @@ -840,9 +845,13 @@ def setUp(self): "number": 2, "pull_request": { "head": { - "sha": self.commit - } - } + "sha": self.commit, + "ref": 'source_branch', + }, + "base": { + "ref": "master", + }, + }, } self.gitlab_merge_request_payload = { "object_kind": GITLAB_MERGE_REQUEST, @@ -851,7 +860,9 @@ def setUp(self): "last_commit": { "id": self.commit }, - "action": "open" + "action": "open", + "source_branch": "source_branch", + "target_branch": "master", }, } self.gitlab_payload = { @@ -1121,6 +1132,70 @@ def test_github_pull_request_synchronize_event(self, trigger_build, core_trigger # `synchronize` webhook event updated the identifier (commit hash) self.assertNotEqual(prev_identifier, external_version.identifier) + @mock.patch('readthedocs.builds.automation_actions.trigger_build') + def test_github_pull_request_automation_rule_build_version(self, automation_rules_trigger_build, trigger_build): + rule = get( + RegexAutomationRule, + project=self.project, + priority=0, + match_arg='.*', + action=VersionAutomationRule.BUILD_EXTERNAL_VERSION, + action_arg='^master$', + version_type=EXTERNAL, + ) + + client = APIClient() + headers = {GITHUB_EVENT_HEADER: GITHUB_PULL_REQUEST} + resp = client.post( + '/api/v2/webhook/github/{}/'.format(self.project.slug), + self.github_pull_request_payload, + format='json', + **headers + ) + external_version = self.project.versions(manager=EXTERNAL).first() + + self.assertEqual(resp.status_code, status.HTTP_200_OK) + self.assertTrue(resp.data['build_triggered']) + self.assertEqual(resp.data['project'], self.project.slug) + self.assertEqual(resp.data['versions'], [external_version.verbose_name]) + + trigger_build.assert_not_called() + automation_rules_trigger_build.assert_called_once_with( + project=self.project, + version=external_version, + commit=self.commit + ) + + @mock.patch('readthedocs.builds.automation_actions.trigger_build') + def test_github_pull_request_automation_rule_dont_build_version(self, automation_rules_trigger_build, trigger_build): + rule = get( + RegexAutomationRule, + project=self.project, + priority=0, + match_arg='.*', + action=VersionAutomationRule.BUILD_EXTERNAL_VERSION, + action_arg='^dont-match-me$', + version_type=EXTERNAL, + ) + + client = APIClient() + headers = {GITHUB_EVENT_HEADER: GITHUB_PULL_REQUEST} + resp = client.post( + '/api/v2/webhook/github/{}/'.format(self.project.slug), + self.github_pull_request_payload, + format='json', + **headers + ) + external_version = self.project.versions(manager=EXTERNAL).first() + + self.assertEqual(resp.status_code, status.HTTP_200_OK) + self.assertFalse(resp.data['build_triggered']) + self.assertEqual(resp.data['project'], self.project.slug) + self.assertEqual(resp.data['versions'], []) + + trigger_build.assert_not_called() + automation_rules_trigger_build.assert_not_called() + @mock.patch('readthedocs.core.utils.trigger_build') def test_github_pull_request_closed_event(self, trigger_build, core_trigger_build): client = APIClient() diff --git a/readthedocs/rtd_tests/tests/test_automation_rules.py b/readthedocs/rtd_tests/tests/test_automation_rules.py index a7f17d99551..c74ad4f5411 100644 --- a/readthedocs/rtd_tests/tests/test_automation_rules.py +++ b/readthedocs/rtd_tests/tests/test_automation_rules.py @@ -3,9 +3,11 @@ import pytest from django_dynamic_fixture import get +from readthedocs.api.v2.views.integrations import ExternalVersionData from readthedocs.builds.constants import ( ALL_VERSIONS, BRANCH, + EXTERNAL, LATEST, SEMVER_VERSIONS, TAG, @@ -91,7 +93,7 @@ def test_match( action=VersionAutomationRule.ACTIVATE_VERSION_ACTION, version_type=version_type, ) - assert rule.run(version) is result + assert rule.run(version)[0] is result @pytest.mark.parametrize( 'version_name,result', @@ -125,7 +127,7 @@ def test_predefined_match_all_versions(self, version_name, result, version_type) action=VersionAutomationRule.ACTIVATE_VERSION_ACTION, version_type=version_type, ) - assert rule.run(version) is result + assert rule.run(version)[0] is result @pytest.mark.parametrize( 'version_name,result', @@ -161,7 +163,7 @@ def test_predefined_match_semver_versions(self, version_name, result, version_ty action=VersionAutomationRule.ACTIVATE_VERSION_ACTION, version_type=version_type, ) - assert rule.run(version) is result + assert rule.run(version)[0] is result @mock.patch('readthedocs.builds.automation_actions.trigger_build') def test_action_activation(self, trigger_build): @@ -180,7 +182,7 @@ def test_action_activation(self, trigger_build): action=VersionAutomationRule.ACTIVATE_VERSION_ACTION, version_type=TAG, ) - assert rule.run(version) is True + assert rule.run(version)[0] is True assert version.active is True trigger_build.assert_called_once() @@ -203,7 +205,7 @@ def test_action_delete_version(self, version_type): action=VersionAutomationRule.DELETE_VERSION_ACTION, version_type=version_type, ) - assert rule.run(version) is True + assert rule.run(version)[0] is True assert not self.project.versions.filter(slug=slug).exists() @pytest.mark.parametrize('version_type', [BRANCH, TAG]) @@ -228,7 +230,7 @@ def test_action_delete_version_on_default_version(self, version_type): action=VersionAutomationRule.DELETE_VERSION_ACTION, version_type=version_type, ) - assert rule.run(version) is True + assert rule.run(version)[0] is True assert self.project.versions.filter(slug=slug).exists() def test_action_set_default_version(self): @@ -248,7 +250,7 @@ def test_action_set_default_version(self): version_type=TAG, ) assert self.project.get_default_version() == LATEST - assert rule.run(version) is True + assert rule.run(version)[0] is True assert self.project.get_default_version() == version.slug @mock.patch('readthedocs.builds.automation_actions.trigger_build') @@ -269,7 +271,7 @@ def test_version_hide_action(self, trigger_build): action=VersionAutomationRule.HIDE_VERSION_ACTION, version_type=TAG, ) - assert rule.run(version) is True + assert rule.run(version)[0] is True assert version.active is True assert version.hidden is True trigger_build.assert_called_once() @@ -293,7 +295,7 @@ def test_version_make_public_action(self, trigger_build): action=VersionAutomationRule.MAKE_VERSION_PUBLIC_ACTION, version_type=TAG, ) - assert rule.run(version) is True + assert rule.run(version)[0] is True assert version.privacy_level == PUBLIC trigger_build.assert_not_called() @@ -316,10 +318,44 @@ def test_version_make_private_action(self, trigger_build): action=VersionAutomationRule.MAKE_VERSION_PRIVATE_ACTION, version_type=TAG, ) - assert rule.run(version) is True + assert rule.run(version)[0] is True assert version.privacy_level == PRIVATE trigger_build.assert_not_called() + @mock.patch('readthedocs.builds.automation_actions.trigger_build') + def test_external_version_build(self, trigger_build): + commit = 'abcd1234' + version = get( + Version, + verbose_name='50', + project=self.project, + active=True, + hidden=False, + type=EXTERNAL, + identifier=commit, + ) + version_data = ExternalVersionData( + id='50', + source_branch='new-feature', + base_branch='main', + commit=commit, + ) + rule = get( + RegexAutomationRule, + project=self.project, + priority=0, + match_arg='.*', + action=VersionAutomationRule.BUILD_EXTERNAL_VERSION, + action_arg='^main$', + version_type=EXTERNAL, + ) + assert rule.run(version, version_data=version_data) == (True, True) + trigger_build.assert_called_once_with( + project=self.project, + version=version, + commit=commit, + ) + @pytest.mark.django_db class TestAutomationRuleManager: