diff options
-rw-r--r-- | buildscripts/client/jiraclient.py | 53 | ||||
-rw-r--r-- | buildscripts/jiraclient.py | 87 | ||||
-rw-r--r-- | buildscripts/tests/test_validate_commit_message.py | 69 | ||||
-rwxr-xr-x | buildscripts/validate_commit_message.py | 207 | ||||
-rw-r--r-- | etc/evergreen.yml | 6 | ||||
-rw-r--r-- | evergreen/commit_message_validate.sh | 1 |
6 files changed, 268 insertions, 155 deletions
diff --git a/buildscripts/client/jiraclient.py b/buildscripts/client/jiraclient.py new file mode 100644 index 00000000000..6251d043e25 --- /dev/null +++ b/buildscripts/client/jiraclient.py @@ -0,0 +1,53 @@ +"""Module to access a JIRA server.""" +from enum import Enum + +import jira +from pydantic import BaseSettings + + +class SecurityLevel(Enum): + """Security level of SERVER tickets.""" + + MONGO_INTERNAL = "Mongo Internal" + NONE = "None" + + +class JiraAuth(BaseSettings): + """OAuth information to connect to Jira.""" + + access_token: str + access_token_secret: str + consumer_key: str + key_cert: str + + class Config: + """Configuration for JiraAuth.""" + + env_prefix = "JIRA_AUTH_" + + +class JiraClient(object): + """A client for JIRA.""" + + def __init__(self, server: str, jira_auth: JiraAuth) -> None: + """ + Initialize the JiraClient with the server URL and user credentials. + + :param server: Jira Server to connect to. + :param jira_auth: OAuth connection information. + """ + opts = {"server": server, "verify": True} + self._jira = jira.JIRA(options=opts, oauth=jira_auth.dict(), validate=True) + + def get_ticket_security_level(self, key: str) -> SecurityLevel: + """ + Lookup the security level of the given ticket. + + :param key: Key of ticket to query. + :return: Security level of the given ticket. + """ + ticket = self._jira.issue(key) + if hasattr(ticket.fields, "security"): + security_level = ticket.fields.security + return SecurityLevel(security_level.name) + return SecurityLevel.NONE diff --git a/buildscripts/jiraclient.py b/buildscripts/jiraclient.py deleted file mode 100644 index fd0a3fc43b6..00000000000 --- a/buildscripts/jiraclient.py +++ /dev/null @@ -1,87 +0,0 @@ -"""Module to access a JIRA server.""" - -import jira - - -class JiraClient(object): - """A client for JIRA.""" - - CLOSE_TRANSITION_NAME = "Close Issue" - RESOLVE_TRANSITION_NAME = "Resolve Issue" - FIXED_RESOLUTION_NAME = "Fixed" - WONT_FIX_RESOLUTION_NAME = "Won't Fix" - - def __init__( # pylint: disable=too-many-arguments - self, server, username=None, password=None, access_token=None, access_token_secret=None, - consumer_key=None, key_cert=None): - """Initialize the JiraClient with the server URL and user credentials.""" - opts = {"server": server, "verify": True} - basic_auth = None - oauth_dict = None - if access_token and access_token_secret and consumer_key and key_cert: - oauth_dict = { - "access_token": access_token, "access_token_secret": access_token_secret, - "consumer_key": consumer_key, "key_cert": key_cert - } - elif username and password: - basic_auth = (username, password) - else: - raise TypeError("Must specify Basic Auth (using arguments username & password)" - " or OAuth (using arguments access_token, access_token_secret," - " consumer_key & key_cert_file) credentials") - self._jira = jira.JIRA(options=opts, basic_auth=basic_auth, oauth=oauth_dict, validate=True) - - self._transitions = {} - self._resolutions = {} - - def create_issue(self, project, summary, description, labels=None): - """Create an issue.""" - fields = { - "project": project, "issuetype": {"name": "Task"}, "summary": summary, - "description": description - } - new_issue = self._jira.create_issue(fields=fields) - if labels: - new_issue.update(fields={"labels": labels}) - return new_issue.key - - def close_issue(self, issue_key, resolution, fix_version=None): - """Close an issue.""" - issue = self._jira.issue(issue_key) - resolution_id = self._get_resolution_id(resolution) - if resolution_id is None: - raise ValueError("No resolution found for '{0}'. Leaving issue '{1}' open.".format( - resolution, issue_key)) - close_transition_id = self._get_transition_id(issue, JiraClient.CLOSE_TRANSITION_NAME) - if close_transition_id is None: - raise ValueError("No transition found for '{0}'. Leaving issue '{1}' open.".format( - JiraClient.CLOSE_TRANSITION_NAME, issue_key)) - fields = {"resolution": {"id": resolution_id}} - if fix_version: - fields["fixVersions"] = [{"name": fix_version}] - self._jira.transition_issue(issue, close_transition_id, fields=fields) - - def _get_transition_id(self, issue, name): - project_key = issue.fields.project.key - project_transitions = self._transitions.setdefault(project_key, {}) - - transition_id = project_transitions.get(name) - if transition_id: - return transition_id - transitions = self._jira.transitions(issue) - for transition in transitions: - project_transitions[transition["name"]] = transition["id"] - if transition["name"] == name: - transition_id = transition["id"] - return transition_id - - def _get_resolution_id(self, name): - resolution_id = self._resolutions.get(name) - if resolution_id is not None: - return resolution_id - resolutions = self._jira.resolutions() - for resolution in resolutions: - self._resolutions[resolution.name] = resolution.id - if resolution.name == name: - resolution_id = resolution.id - return resolution_id diff --git a/buildscripts/tests/test_validate_commit_message.py b/buildscripts/tests/test_validate_commit_message.py index acd6008da44..db341a5add1 100644 --- a/buildscripts/tests/test_validate_commit_message.py +++ b/buildscripts/tests/test_validate_commit_message.py @@ -1,12 +1,12 @@ """Unit tests for the evergreen_task_timeout script.""" import itertools import unittest -from typing import List +from typing import List, Optional from unittest.mock import MagicMock -from evergreen import EvergreenApi - import buildscripts.validate_commit_message as under_test +from buildscripts.client.jiraclient import JiraClient, SecurityLevel +from evergreen import EvergreenApi # pylint: disable=missing-docstring,no-self-use @@ -19,18 +19,35 @@ INVALID_MESSAGES = [ ] -def create_mock_evg_client(code_change_messages: List[str]) -> MagicMock: - mock_code_change = MagicMock() - mock_code_change.commit_messages = code_change_messages +def create_mock_code_change(code_change_messages: List[str], branch_name: Optional[str] = None): + mock_code_change = MagicMock( + commit_messages=code_change_messages, + branch_name=branch_name if branch_name else "mongodb-mongo-master", + ) + return mock_code_change + - mock_patch = MagicMock() - mock_patch.module_code_changes = [mock_code_change] +def create_mock_patch(code_change_messages: List[str], branch_name: Optional[str] = None): + mock_code_change = create_mock_code_change(code_change_messages, branch_name) + mock_patch = MagicMock(module_code_changes=[mock_code_change]) + return mock_patch + + +def create_mock_evg_client(code_change_messages: List[str], + branch_name: Optional[str] = None) -> MagicMock: + mock_patch = create_mock_patch(code_change_messages, branch_name) mock_evg_client = MagicMock(spec_set=EvergreenApi) mock_evg_client.patch_by_id.return_value = mock_patch return mock_evg_client +def create_mock_jira_client(): + mock_jira = MagicMock(spec_set=JiraClient) + mock_jira.get_ticket_security_level.return_value = SecurityLevel.NONE + return mock_jira + + def interleave_new_format(older): """Create a new list containing a new and old format copy of each string.""" newer = [ @@ -58,16 +75,20 @@ class ValidateCommitMessageTest(unittest.TestCase): 'Revert "Import wiredtiger: 58115abb6fbb3c1cc7bfd087d41a47347bce9a69 from branch mongodb-4.4"', ] mock_evg_api = create_mock_evg_client(interleave_new_format(messages)) + mock_jira = create_mock_jira_client() + orchestrator = under_test.CommitMessageValidationOrchestrator(mock_evg_api, mock_jira) - is_valid = under_test.validate_commit_messages("version_id", mock_evg_api) + is_valid = orchestrator.validate_commit_messages("version_id") self.assertEqual(is_valid, under_test.STATUS_OK) def test_private(self): messages = ["XYZ-1"] mock_evg_api = create_mock_evg_client(interleave_new_format(messages)) + mock_jira = create_mock_jira_client() + orchestrator = under_test.CommitMessageValidationOrchestrator(mock_evg_api, mock_jira) - is_valid = under_test.validate_commit_messages("version_id", mock_evg_api) + is_valid = orchestrator.validate_commit_messages("version_id") self.assertEqual(is_valid, under_test.STATUS_ERROR) @@ -76,10 +97,34 @@ class ValidateCommitMessageTest(unittest.TestCase): "Fix lint", "EVG-1", # Test valid projects with various number lengths "SERVER-20", - "XYZ-1" + "XYZ-1", ] mock_evg_api = create_mock_evg_client(interleave_new_format(messages)) + mock_jira = create_mock_jira_client() + orchestrator = under_test.CommitMessageValidationOrchestrator(mock_evg_api, mock_jira) - is_valid = under_test.validate_commit_messages("version_id", mock_evg_api) + is_valid = orchestrator.validate_commit_messages("version_id") self.assertEqual(is_valid, under_test.STATUS_ERROR) + + def test_internal_ticket_to_public_repo_should_fail(self): + message = "SERVER-20" + mock_evg_api = create_mock_evg_client(interleave_new_format([message])) + mock_jira = create_mock_jira_client() + mock_jira.get_ticket_security_level.return_value = SecurityLevel.MONGO_INTERNAL + orchestrator = under_test.CommitMessageValidationOrchestrator(mock_evg_api, mock_jira) + + is_valid = orchestrator.validate_commit_messages("version_id") + + self.assertEqual(is_valid, under_test.STATUS_ERROR) + + def test_internal_ticket_to_private_repo_should_succeed(self): + message = "SERVER-20" + mock_evg_api = create_mock_evg_client(interleave_new_format([message]), "private-repo") + mock_jira = create_mock_jira_client() + mock_jira.get_ticket_security_level.return_value = SecurityLevel.MONGO_INTERNAL + orchestrator = under_test.CommitMessageValidationOrchestrator(mock_evg_api, mock_jira) + + is_valid = orchestrator.validate_commit_messages("version_id") + + self.assertEqual(is_valid, under_test.STATUS_OK) diff --git a/buildscripts/validate_commit_message.py b/buildscripts/validate_commit_message.py index 61fcce06f85..3aff60e89c1 100755 --- a/buildscripts/validate_commit_message.py +++ b/buildscripts/validate_commit_message.py @@ -28,19 +28,27 @@ # """Validate that the commit message is ok.""" import argparse +import logging import os import re import sys -import logging -from evergreen import RetryingEvergreenApi, EvergreenApi +from typing import List, Optional + +from evergreen import EvergreenApi, RetryingEvergreenApi +from buildscripts.client.jiraclient import JiraAuth, JiraClient, SecurityLevel + +JIRA_SERVER = "https://jira.mongodb.org" EVG_CONFIG_FILE = "~/.evergreen.yml" +SERVER_TICKET_PREFIX = "SERVER-" +PUBLIC_PROJECT_PREFIX = "mongodb-mongo-" + LOGGER = logging.getLogger(__name__) ERROR_MSG = """ ################################################################################ Encountered an invalid commit message. Please correct to the commit message to -continue. +continue. Commit message should start with a Public Jira ticket, an "Import" for wiredtiger or tools, or a "Revert" message. @@ -50,35 +58,33 @@ or tools, or a "Revert" message. ################################################################################ """ -COMMON_PUBLIC_PATTERN = r''' +COMMON_PUBLIC_PATTERN = r""" ((?P<revert>Revert)\s+[\"\']?)? # Revert (optional) ((?P<ticket>(?:EVG|SERVER|WT)-[0-9]+)[\"\']?\s*) # ticket identifier (?P<body>(?:(?!\(cherry\spicked\sfrom).)*)? # To also capture the body (?P<backport>\(cherry\spicked\sfrom.*)? # back port (optional) - ''' + """ """Common Public pattern format.""" -COMMON_LINT_PATTERN = r'(?P<lint>Fix\slint)' +COMMON_LINT_PATTERN = r"(?P<lint>Fix\slint)" """Common Lint pattern format.""" -COMMON_IMPORT_PATTERN = r'(?P<imported>Import\s(wiredtiger|tools):\s.*)' +COMMON_IMPORT_PATTERN = r"(?P<imported>Import\s(wiredtiger|tools):\s.*)" """Common Import pattern format.""" -COMMON_REVERT_IMPORT_PATTERN = r'Revert\s+[\"\']?(?P<imported>Import\s(wiredtiger|tools):\s.*)' +COMMON_REVERT_IMPORT_PATTERN = (r"Revert\s+[\"\']?(?P<imported>Import\s(wiredtiger|tools):\s.*)") """Common revert Import pattern format.""" -COMMON_PRIVATE_PATTERN = r''' +COMMON_PRIVATE_PATTERN = r""" ((?P<revert>Revert)\s+[\"\']?)? # Revert (optional) ((?P<ticket>[A-Z]+-[0-9]+)[\"\']?\s*) # ticket identifier (?P<body>(?:(?!('\s(into\s'(([^/]+))/(([^:]+)):(([^']+))'))).)*)? # To also capture the body -''' +""" """Common Private pattern format.""" STATUS_OK = 0 STATUS_ERROR = 1 -GIT_SHOW_COMMAND = ["git", "show", "-1", "-s", "--format=%s"] - def new_patch_description(pattern: str) -> str: """ @@ -92,7 +98,7 @@ def new_patch_description(pattern: str) -> str: :return: A pattern to match the new format for the patch description. """ return (r"""^((?P<commitqueue>Commit\sQueue\sMerge:)\s')""" - f'{pattern}' + f"{pattern}" # r"""('\s(?P<into>into\s'((?P<owner>[^/]+))/((?P<repo>[^:]+)):((?P<branch>[^']+))'))""" ) @@ -108,63 +114,146 @@ def old_patch_description(pattern: str) -> str: :param pattern: The pattern to wrap. :return: A pattern to match the old format for the patch description. """ - return r'^' f'{pattern}' + return r"^" f"{pattern}" # NOTE: re.VERBOSE is for visibility / debugging. As such significant white space must be # escaped (e.g ' ' to \s). VALID_PATTERNS = [ - re.compile(new_patch_description(COMMON_PUBLIC_PATTERN), re.MULTILINE | re.DOTALL | re.VERBOSE), - re.compile(old_patch_description(COMMON_PUBLIC_PATTERN), re.MULTILINE | re.DOTALL | re.VERBOSE), - re.compile(new_patch_description(COMMON_LINT_PATTERN), re.MULTILINE | re.DOTALL | re.VERBOSE), - re.compile(old_patch_description(COMMON_LINT_PATTERN), re.MULTILINE | re.DOTALL | re.VERBOSE), - re.compile(new_patch_description(COMMON_IMPORT_PATTERN), re.MULTILINE | re.DOTALL | re.VERBOSE), - re.compile(old_patch_description(COMMON_IMPORT_PATTERN), re.MULTILINE | re.DOTALL | re.VERBOSE), re.compile( - new_patch_description(COMMON_REVERT_IMPORT_PATTERN), re.MULTILINE | re.DOTALL | re.VERBOSE), + new_patch_description(COMMON_PUBLIC_PATTERN), + re.MULTILINE | re.DOTALL | re.VERBOSE, + ), + re.compile( + old_patch_description(COMMON_PUBLIC_PATTERN), + re.MULTILINE | re.DOTALL | re.VERBOSE, + ), + re.compile( + new_patch_description(COMMON_LINT_PATTERN), + re.MULTILINE | re.DOTALL | re.VERBOSE, + ), re.compile( - old_patch_description(COMMON_REVERT_IMPORT_PATTERN), re.MULTILINE | re.DOTALL | re.VERBOSE), + old_patch_description(COMMON_LINT_PATTERN), + re.MULTILINE | re.DOTALL | re.VERBOSE, + ), + re.compile( + new_patch_description(COMMON_IMPORT_PATTERN), + re.MULTILINE | re.DOTALL | re.VERBOSE, + ), + re.compile( + old_patch_description(COMMON_IMPORT_PATTERN), + re.MULTILINE | re.DOTALL | re.VERBOSE, + ), + re.compile( + new_patch_description(COMMON_REVERT_IMPORT_PATTERN), + re.MULTILINE | re.DOTALL | re.VERBOSE, + ), + re.compile( + old_patch_description(COMMON_REVERT_IMPORT_PATTERN), + re.MULTILINE | re.DOTALL | re.VERBOSE, + ), ] """valid public patterns.""" PRIVATE_PATTERNS = [ re.compile( - new_patch_description(COMMON_PRIVATE_PATTERN), re.MULTILINE | re.DOTALL | re.VERBOSE), + new_patch_description(COMMON_PRIVATE_PATTERN), + re.MULTILINE | re.DOTALL | re.VERBOSE, + ), re.compile( - old_patch_description(COMMON_PRIVATE_PATTERN), re.MULTILINE | re.DOTALL | re.VERBOSE), + old_patch_description(COMMON_PRIVATE_PATTERN), + re.MULTILINE | re.DOTALL | re.VERBOSE, + ), ] """private patterns.""" -def validate_commit_messages(version_id: str, evg_api: EvergreenApi) -> int: - """ - Validate the commit messages for the given build. - - :param version_id: ID of version to validate. - :param evg_api: Evergreen API client. - :return: True if all commit messages were valid. - """ - found_error = False - code_changes = evg_api.patch_by_id(version_id).module_code_changes - for change in code_changes: - for message in change.commit_messages: - if any(valid_pattern.match(message) for valid_pattern in VALID_PATTERNS): - continue - elif any(private_pattern.match(message) for private_pattern in PRIVATE_PATTERNS): - print( - ERROR_MSG.format(error_msg="Reference to a private project", - branch=change.branch_name, commit_message=message)) - found_error = True - else: - print( - ERROR_MSG.format(error_msg="Commit without a ticket", branch=change.branch_name, - commit_message=message)) - found_error = True - - return STATUS_ERROR if found_error else STATUS_OK - - -def main(argv=None): +class CommitMessageValidationOrchestrator: + """An orchestrator to validate that commit messages are valid.""" + + def __init__(self, evg_api: EvergreenApi, jira_client: JiraClient) -> None: + """ + Initialize the orchestrator. + + :param evg_api: Evergreen API client. + :param jira_client: Client to Jira API. + """ + self.evg_api = evg_api + self.jira_client = jira_client + + def validate_ticket(self, ticket: str, project: str) -> bool: + """ + Check that the given Jira ticket has a proper security level. + + Commits targeting a public project should not have a defined security level (these are + public by default). + + :param ticket: Ticket to check. + :param project: Project commit is targeting. + :return: True if ticket is valid. + """ + if ticket.startswith(SERVER_TICKET_PREFIX) and project.startswith(PUBLIC_PROJECT_PREFIX): + security_level = self.jira_client.get_ticket_security_level(ticket) + return security_level == SecurityLevel.NONE + return True + + def validate_msg(self, message: str, project: str) -> bool: + """ + Check that the given message is valid. + + :param message: Commit message to validate. + :param project: Project commit is targeting. + :return: True if the message is valid. + """ + valid_matches = [valid_pattern.match(message) for valid_pattern in VALID_PATTERNS] + if any(valid_matches): + ticket_matches = [pattern.match(message) for pattern in VALID_PATTERNS[0:2]] + for match in [ticket_match for ticket_match in ticket_matches if ticket_match]: + if not self.validate_ticket(match.group("ticket"), project): + print( + ERROR_MSG.format( + error_msg="Reference to a internal Jira Ticket", + branch=project, + commit_message=message, + )) + return False + return True + elif any(private_pattern.match(message) for private_pattern in PRIVATE_PATTERNS): + print( + ERROR_MSG.format( + error_msg="Reference to a private project", + branch=project, + commit_message=message, + )) + return False + else: + print( + ERROR_MSG.format( + error_msg="Commit without a ticket", + branch=project, + commit_message=message, + )) + return False + + def validate_commit_messages(self, version_id: str) -> int: + """ + Validate the commit messages for the given build. + + :param version_id: ID of version to validate. + :param evg_api: Evergreen API client. + :return: True if all commit messages were valid. + """ + found_error = False + code_changes = self.evg_api.patch_by_id(version_id).module_code_changes + for change in code_changes: + for message in change.commit_messages: + is_valid = self.validate_msg(message, change.branch_name) + found_error = found_error or not is_valid + + return STATUS_ERROR if found_error else STATUS_OK + + +def main(argv: Optional[List[str]] = None) -> int: """Execute Main function to validate commit messages.""" parser = argparse.ArgumentParser( usage="Validate the commit message. " @@ -174,12 +263,18 @@ def main(argv=None): metavar="version id", help="The id of the version to validate", ) - parser.add_argument("--evg-config-file", default=EVG_CONFIG_FILE, - help="Path to evergreen configuration file containing auth information.") + parser.add_argument( + "--evg-config-file", + default=EVG_CONFIG_FILE, + help="Path to evergreen configuration file containing auth information.", + ) args = parser.parse_args(argv) evg_api = RetryingEvergreenApi.get_api(config_file=os.path.expanduser(args.evg_config_file)) + jira_auth = JiraAuth() + jira_client = JiraClient(JIRA_SERVER, jira_auth) + orchestrator = CommitMessageValidationOrchestrator(evg_api, jira_client) - return validate_commit_messages(args.version_id, evg_api) + return orchestrator.validate_commit_messages(args.version_id) if __name__ == "__main__": diff --git a/etc/evergreen.yml b/etc/evergreen.yml index 3a35b773caa..eb64f2010ec 100644 --- a/etc/evergreen.yml +++ b/etc/evergreen.yml @@ -6862,6 +6862,7 @@ tasks: - *kill_processes - *cleanup_environment - func: "set up venv" + - func: "upload pip requirements" - func: "configure evergreen api credentials" - command: subprocess.exec type: test @@ -6869,6 +6870,11 @@ tasks: binary: bash args: - "./src/evergreen/commit_message_validate.sh" + env: + JIRA_AUTH_ACCESS_TOKEN: ${jira_auth_access_token} + JIRA_AUTH_ACCESS_TOKEN_SECRET: ${jira_auth_access_token_secret} + JIRA_AUTH_CONSUMER_KEY: ${jira_auth_consumer_key} + JIRA_AUTH_KEY_CERT: ${jira_auth_key_cert} - name: check_for_todos tags: [] diff --git a/evergreen/commit_message_validate.sh b/evergreen/commit_message_validate.sh index 3d7825ae6e1..b633bf4308c 100644 --- a/evergreen/commit_message_validate.sh +++ b/evergreen/commit_message_validate.sh @@ -7,6 +7,7 @@ set -o verbose set -o errexit if [ "${is_commit_queue}" = "true" ]; then activate_venv + $python -m pip --disable-pip-version-check install --upgrade cryptography || exit 1 $python buildscripts/validate_commit_message.py \ --evg-config-file ./.evergreen.yml \ ${version_id} |