Automation Rules: support external versions

Allow to build an external version based on its source and base branch.

Ref #7653

Author: stsewd

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,10 +325,13 @@ 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')
@@ -530,10 +538,13 @@ 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')

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.

readthedocs/builds/forms.py

@@ -14,6 +14,8 @@
     ALL_VERSIONS,
     BRANCH,
     BRANCH_TEXT,
+    EXTERNAL,
+    EXTERNAL_TEXT,
     TAG,
     TAG_TEXT,
 )
@@ -113,6 +115,7 @@ class Meta:
             'predefined_match_arg',
             'match_arg',
             'version_type',
+            'action_arg',
             'action',
         ]
         # Don't pollute the UI with help texts
@@ -133,6 +136,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 +159,38 @@ 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 this type of rule."""
+        action = self.cleaned_data['action']
+        version_type = self.cleaned_data['version_type']
+        allowed_actions = VersionAutomationRule.allowed_actions_on_external_versions.keys()
+        if version_type == EXTERNAL and action not in allowed_actions:
+            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.
+        Indicates the pattern of 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 +221,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()

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

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,
@@ -1035,16 +1050,18 @@ def run(self, version, *args, **kwargs):
         Run an action if `version` matches the rule.
 
         :type version: readthedocs.builds.models.Version
-        :returns: True if the action was performed
+        :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(), *args, **kwargs)
             if match:
-                self.apply_action(version, result)
-                return True
-        return False
+                action_result = self.apply_action(version, result)
+                return True, action_result
+        return False, None
 
-    def match(self, version, match_arg):
+    def match(self, version, match_arg, *args, **kwargs):
         """
         Returns True and the match result if the version matches the rule.
 
@@ -1067,10 +1084,11 @@ 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, match_result, self.action_arg)
 
     def move(self, steps):
         """
@@ -1172,52 +1190,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, *args, **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(