diff --git a/admyral/actions/__init__.py b/admyral/actions/__init__.py index 58905975..03e2dcef 100644 --- a/admyral/actions/__init__.py +++ b/admyral/actions/__init__.py @@ -60,15 +60,10 @@ list_groups_per_user, list_1password_audit_events, search_github_enterprise_audit_logs, -) - -from admyral.actions.integrations.compliance.github import ( - list_merged_prs, - list_commit_history_for_pr, - list_review_history_for_pr, - list_commits, - get_raw_commit_diff_between_two_commits, - get_commit_diff_info_between_two_commits, + list_merged_pull_requests, + list_commit_history_for_pull_request, + list_review_history_for_pull_request, + compare_two_github_commits, ) @@ -116,10 +111,8 @@ "okta_get_all_user_types", "get_okta_logs", "search_github_enterprise_audit_logs", - "list_merged_prs", - "list_commit_history_for_pr", - "list_review_history_for_pr", - "list_commits", - "get_raw_commit_diff_between_two_commits", - "get_commit_diff_info_between_two_commits", + "list_merged_pull_requests", + "list_commit_history_for_pull_request", + "list_review_history_for_pull_request", + "compare_two_github_commits", ] diff --git a/admyral/actions/integrations/compliance/__init__.py b/admyral/actions/integrations/compliance/__init__.py index 67c5fede..8b0577e9 100644 --- a/admyral/actions/integrations/compliance/__init__.py +++ b/admyral/actions/integrations/compliance/__init__.py @@ -7,12 +7,10 @@ ) from admyral.actions.integrations.compliance.github import ( search_github_enterprise_audit_logs, - list_merged_prs, - list_commit_history_for_pr, - list_review_history_for_pr, - list_commits, - get_raw_commit_diff_between_two_commits, - get_commit_diff_info_between_two_commits, + list_merged_pull_requests, + list_commit_history_for_pull_request, + list_review_history_for_pull_request, + compare_two_github_commits, ) __all__ = [ @@ -20,10 +18,8 @@ "list_groups_per_user", "list_1password_audit_events", "search_github_enterprise_audit_logs", - "list_merged_prs", - "list_commit_history_for_pr", - "list_review_history_for_pr", - "list_commits", - "get_raw_commit_diff_between_two_commits", - "get_commit_diff_info_between_two_commits", + "list_merged_pull_requests", + "list_commit_history_for_pull_request", + "list_review_history_for_pull_request", + "compare_two_github_commits", ] diff --git a/admyral/actions/integrations/compliance/github.py b/admyral/actions/integrations/compliance/github.py index 1694309b..10c58b44 100644 --- a/admyral/actions/integrations/compliance/github.py +++ b/admyral/actions/integrations/compliance/github.py @@ -4,8 +4,9 @@ https://docs.github.com/en/authentication/keeping-your-account-and-data-secure/creating-a-personal-access-token """ -from typing import Annotated, Literal +from typing import Annotated, Literal, Union from httpx import Client +from dateutil import parser from admyral.action import action, ArgumentMetadata from admyral.context import ctx @@ -22,6 +23,47 @@ def get_github_enterprise_client(access_token: str, enterprise: str) -> Client: ) +def get_github_client(access_token: str) -> Client: + return Client( + base_url="https://api.github.com", + headers={ + "Authorization": f"Bearer {access_token}", + "Accept": "application/vnd.github.v3+json", + }, + ) + + +def _get_events_with_pagination( + client: Client, + url: str, + params: dict, + limit: int | None = None, + event_filter_fn: Union[callable, None] = None, + early_stop_fn: Union[callable, None] = None, +) -> list[dict[str, JsonValue]]: + events = [] + while limit is None or len(events) < limit: + response = client.get(url, params=params) + response.raise_for_status() + events_on_page = response.json() + + if event_filter_fn: + events.extend(filter(event_filter_fn, events_on_page)) + else: + events.extend(events_on_page) + + if early_stop_fn and early_stop_fn(events_on_page): + break + + if "next" in response.links: + url = response.links["next"]["url"][len(str(client.base_url)) :] + params = None + else: + break + + return events + + @action( display_name="Search Enterprise Audit Logs", display_namespace="GitHub", @@ -84,77 +126,23 @@ def search_github_enterprise_audit_logs( elif end_time: params["created"] = f"<={end_time}" - url = "/audit-log" - events = [] - while limit is None or len(events) < limit: - response = client.get( - url, - params=params, - ) - response.raise_for_status() - events.extend(response.json()) - - if "next" in response.links: - url = response.links["next"]["url"][len(str(client.base_url)) :] - params = None - else: - break + events = _get_events_with_pagination( + client=client, + url="/audit-log", + params=params, + limit=limit, + ) return events if limit is None else events[:limit] -def get_github_client(access_token: str) -> Client: - return Client( - base_url="https://api.github.com", - headers={ - "Authorization": f"Bearer {access_token}", - "Accept": "application/vnd.github.v3+json", - }, - ) - - -def _get_events_with_pagination( - client: Client, - url: str, - params: dict, - limit: int, - start_time: str = "", - end_time: str = "", - date_field: str = "", -) -> list[dict[str, JsonValue]]: - events = [] - while limit is None or len(events) < limit: - response = client.get(url, params=params) - response.raise_for_status() - events_on_page = response.json() - - if start_time and end_time and date_field: - for event in events_on_page: - if start_time <= event[date_field] <= end_time: - events.append(event) - if limit: - if len(events) >= limit: - break - - else: - events.extend(events_on_page) - - if "next" in response.links: - url = response.links["next"]["url"][len(str(client.base_url)) :] - params = None - else: - break - - return events - - @action( - display_name="Get Commit Diff Info Between Two Commits", + display_name="Compare Two GitHub Commits", display_namespace="GitHub", description="Get the commit diff info between two commits.", secrets_placeholders=["GITHUB_SECRET"], ) -def get_commit_diff_info_between_two_commits( +def compare_two_github_commits( repo_owner: Annotated[ str, ArgumentMetadata( @@ -183,6 +171,15 @@ def get_commit_diff_info_between_two_commits( description="The head commit (commit ID or SHA)", ), ], + diff_type: Annotated[ + Literal["json", "diff"], + ArgumentMetadata( + display_name="Diff Type", + description="The type of diff to return. 'json' returns the diff as JSON. " + "'diff' returns the diff as string in the same format as the CLI command " + "git diff BASE..HEAD. Default: 'json'.", + ), + ] = "json", ) -> dict[str, JsonValue]: # https://docs.github.com/en/rest/commits/commits?apiVersion=2022-11-28#compare-two-commits @@ -192,78 +189,34 @@ def get_commit_diff_info_between_two_commits( with get_github_client(access_token=access_token) as client: url = f"/repos/{repo_owner}/{repo_name}/compare/{base}...{head}" - response = client.get(url) - response.raise_for_status() - - return response.json() + if diff_type == "diff": + headers = {"Accept": "application/vnd.github.v3.diff"} + diff = "" + while True: + response = client.get(url, headers=headers) + response.raise_for_status() + diff += response.text + if "next" in response.links: + url = response.links["next"]["url"][len(str(client.base_url)) :] + else: + break -@action( - display_name="Get Raw Commit Diff between Two Commits", - display_namespace="GitHub", - description="Get the raw commit diff between two commits.", - secrets_placeholders=["GITHUB_SECRET"], -) -def get_raw_commit_diff_between_two_commits( - repo_owner: Annotated[ - str, - ArgumentMetadata( - display_name="Repository Owner", - description="The owner of the repository", - ), - ], - repo_name: Annotated[ - str, - ArgumentMetadata( - display_name="Repository Name", - description="The name of the repository", - ), - ], - base: Annotated[ - str, - ArgumentMetadata( - display_name="Base", - description="The base commit (commit ID or SHA)", - ), - ], - head: Annotated[ - str, - ArgumentMetadata( - display_name="Head", - description="The head commit (commit ID or SHA)", - ), - ], -) -> str: - # https://docs.github.com/en/rest/commits/commits?apiVersion=2022-11-28#compare-two-commits + return diff - secret = ctx.get().secrets.get("GITHUB_SECRET") - access_token = secret["access_token"] - - with get_github_client(access_token=access_token) as client: - headers = {"Accept": "application/vnd.github.v3.diff"} - url = f"/repos/{repo_owner}/{repo_name}/compare/{base}...{head}" - - diff = "" - while True: - response = client.get(url, headers=headers) - response.raise_for_status() - diff += response.text - - if "next" in response.links: - url = response.links["next"]["url"][len(str(client.base_url)) :] - else: - break + response = client.get(url) + response.raise_for_status() - return diff + return response.json() @action( - display_name="List Merged PRs for a Repository", + display_name="List Merged Pull Requests for a Repository", display_namespace="GitHub", - description="List all merged PRs for a repository", + description="List all merged pull requests for a repository.", secrets_placeholders=["GITHUB_SECRET"], ) -def list_merged_prs( +def list_merged_pull_requests( repo_owner: Annotated[ str, ArgumentMetadata( @@ -284,14 +237,14 @@ def list_merged_prs( display_name="Start Time", description="The start time for the cases to list. Must be in ISO 8601 format (YYYY-MM-DDTHH:MM:SSZ).", ), - ] = "1970-01-01T00:00:00Z", + ] = None, end_time: Annotated[ str | None, ArgumentMetadata( display_name="End Time", description="The end time for the cases to list. Must be in ISO 8601 format (YYYY-MM-DDTHH:MM:SSZ).", ), - ] = "2100-01-01T00:00:00Z", + ] = None, limit: Annotated[ str | None, ArgumentMetadata( @@ -312,28 +265,34 @@ def list_merged_prs( "direction": "desc", "per_page": 100, } - url = f"/repos/{repo_owner}/{repo_name}/pulls" + + # convert into datetime object + start_time = parser.isoparse( + start_time if start_time else "1970-01-01T00:00:00Z" + ) + end_time = parser.isoparse(end_time if end_time else "2100-01-01T00:00:00Z") events = _get_events_with_pagination( client=client, - url=url, + url=f"/repos/{repo_owner}/{repo_name}/pulls", params=params, limit=limit, - start_time=start_time, - end_time=end_time, - date_field="merged_at", + event_filter_fn=lambda event: event["merged_at"] + and start_time <= parser.isoparse(event["merged_at"]) <= end_time, + early_stop_fn=lambda events: end_time + < parser.isoparse(events[-1]["updated_at"]), ) return events @action( - display_name="Get Commit History for a PR", + display_name="Get Commit History for a Pull Request", display_namespace="GitHub", - description="List commit history for a PR from most recent to oldest", + description="List commit history for a Pull Request from most recent to oldest", secrets_placeholders=["GITHUB_SECRET"], ) -def list_commit_history_for_pr( +def list_commit_history_for_pull_request( repo_owner: Annotated[ str, ArgumentMetadata( @@ -348,34 +307,13 @@ def list_commit_history_for_pr( description="The name of the repository", ), ], - pr_number: Annotated[ + pull_request_number: Annotated[ int, ArgumentMetadata( - display_name="PR Number", + display_name="Pull Request Number", description="The name of the repository", ), ], - start_time: Annotated[ - str | None, - ArgumentMetadata( - display_name="Start Time", - description="The start time for the cases to list. Must be in ISO 8601 format (YYYY-MM-DDTHH:MM:SSZ).", - ), - ] = "1970-01-01T00:00:00Z", - end_time: Annotated[ - str | None, - ArgumentMetadata( - display_name="End Time", - description="The end time for the cases to list. Must be in ISO 8601 format (YYYY-MM-DDTHH:MM:SSZ).", - ), - ] = "2100-01-01T00:00:00Z", - limit: Annotated[ - str | None, - ArgumentMetadata( - display_name="Limit", - description="The maximum number of cases to list.", - ), - ] = None, ) -> list[dict[str, JsonValue]]: # https://docs.github.com/en/rest/commits/commits?apiVersion=2022-11-28 @@ -383,31 +321,21 @@ def list_commit_history_for_pr( access_token = secret["access_token"] with get_github_client(access_token=access_token) as client: - params = { - "per_page": 100, - } - url = f"/repos/{repo_owner}/{repo_name}/pulls/{pr_number}/commits" - events = _get_events_with_pagination( - client=client, url=url, params=params, limit=limit + client=client, + url=f"/repos/{repo_owner}/{repo_name}/pulls/{pull_request_number}/commits", + params={"per_page": 100}, ) - - events_in_time_range = [ - event - for event in events - if start_time <= event["commit"]["committer"]["date"] <= end_time - ] - - return events_in_time_range if limit is None else events_in_time_range[:limit] + return events @action( - display_name="Get Approval History for a PR", + display_name="Get Approval History for a Pull Request", display_namespace="GitHub", - description="List aproval history for a PR", + description="List approval history for a Pull Request", secrets_placeholders=["GITHUB_SECRET"], ) -def list_review_history_for_pr( +def list_review_history_for_pull_request( repo_owner: Annotated[ str, ArgumentMetadata( @@ -422,10 +350,10 @@ def list_review_history_for_pr( description="The name of the repository", ), ], - pr_number: Annotated[ + pull_request_number: Annotated[ int, ArgumentMetadata( - display_name="PR Number", + display_name="Pull Request Number", description="The name of the repository", ), ], @@ -435,27 +363,6 @@ def list_review_history_for_pr( display_name="State", description="The state of the reviews to list.", ), - ], - start_time: Annotated[ - str | None, - ArgumentMetadata( - display_name="Start Time", - description="The start time for the cases to list. Must be in ISO 8601 format (YYYY-MM-DDTHH:MM:SSZ).", - ), - ] = "1970-01-01T00:00:00Z", - end_time: Annotated[ - str | None, - ArgumentMetadata( - display_name="End Time", - description="The end time for the cases to list. Must be in ISO 8601 format (YYYY-MM-DDTHH:MM:SSZ).", - ), - ] = "2100-01-01T00:00:00Z", - limit: Annotated[ - str | None, - ArgumentMetadata( - display_name="Limit", - description="The maximum number of cases to list.", - ), ] = None, ) -> list[dict[str, JsonValue]]: # https://docs.github.com/en/rest/pulls/reviews?apiVersion=2022-11-28 @@ -467,82 +374,16 @@ def list_review_history_for_pr( "per_page": 100, } - url = f"/repos/{repo_owner}/{repo_name}/pulls/{pr_number}/reviews" - events = _get_events_with_pagination( - client=client, - url=url, - params=params, - limit=limit, - start_time=start_time, - end_time=end_time, - date_field="submitted_at", - ) - if state: - filtered_for_state = [ - review for review in events if review["state"] == state - ] - return filtered_for_state if limit is None else filtered_for_state[:limit] + event_filter_fn = lambda event: event["state"] == state # noqa: E731 else: - return events if limit is None else events[:limit] - - -@action( - display_name="List Commits", - display_namespace="GitHub", - description="List commits", - secrets_placeholders=["GITHUB_SECRET"], -) -def list_commits( - repo_owner: Annotated[ - str, - ArgumentMetadata( - display_name="Repository Owner", - description="The owner of the repository", - ), - ], - repo_name: Annotated[ - str, - ArgumentMetadata( - display_name="Repository Name", - description="The name of the repository", - ), - ], - since: Annotated[ - str | None, - ArgumentMetadata( - display_name="Since", - description="The start time for the cases to list. Must be in ISO 8601 format (YYYY-MM-DDTHH:MM:SSZ).", - ), - ] = "1970-01-01T00:00:00Z", - until: Annotated[ - str | None, - ArgumentMetadata( - display_name="Until", - description="The end time for the cases to list. Must be in ISO 8601 format (YYYY-MM-DDTHH:MM:SSZ).", - ), - ] = "2099-12-31T00:00:00Z", - limit: Annotated[ - str | None, - ArgumentMetadata( - display_name="Limit", - description="The maximum number of cases to list.", - ), - ] = None, -) -> list[dict[str, JsonValue]]: - # https://docs.github.com/en/rest/commits/commits?apiVersion=2022-11-28#get-a-commit - secret = ctx.get().secrets.get("GITHUB_SECRET") - access_token = secret["access_token"] + event_filter_fn = None - with get_github_client(access_token=access_token) as client: - params = { - "since": since, - "until": until, - "per_page": 100, - } - url = f"/repos/{repo_owner}/{repo_name}/commits" events = _get_events_with_pagination( - client=client, url=url, params=params, limit=limit + client=client, + url=f"/repos/{repo_owner}/{repo_name}/pulls/{pull_request_number}/reviews", + params=params, + event_filter_fn=event_filter_fn, ) - return events if limit is None else events[:limit] + return events diff --git a/examples/access_review/workflows/github_handle_pr_with_unreviewed_commits.py b/examples/access_review/workflows/github_handle_pr_with_unreviewed_commits.py index e5607fc9..f1e2ae28 100644 --- a/examples/access_review/workflows/github_handle_pr_with_unreviewed_commits.py +++ b/examples/access_review/workflows/github_handle_pr_with_unreviewed_commits.py @@ -1,27 +1,31 @@ from typing import Annotated +from dateutil import parser from admyral.workflow import workflow, Webhook from admyral.typings import JsonValue from admyral.action import action, ArgumentMetadata from admyral.actions import ( - list_commit_history_for_pr, - list_review_history_for_pr, + list_commit_history_for_pull_request, + list_review_history_for_pull_request, send_slack_message_to_user_by_email, + send_slack_message, create_jira_issue, - get_raw_commit_diff_between_two_commits, - get_commit_diff_info_between_two_commits, + compare_two_github_commits, openai_chat_completion, wait, + transform, ) +from admyral.utils.collections import is_empty @action( - display_name="Check PR for unreviewed commits", + display_name="Check if PR has unreviewed commits", display_namespace="GitHub", description="Check PR for commits after last approval", secrets_placeholders=["GITHUB_SECRET"], + requirements=["dateutil"], ) -def check_pr_for_unreviewed_commits( +def has_pr_unreviewed_commits( pull_request: Annotated[ dict[str, JsonValue], ArgumentMetadata( @@ -44,164 +48,66 @@ def check_pr_for_unreviewed_commits( ), ], ) -> dict[str, JsonValue]: - pr_number = pull_request.get("number") - commit_history = list_commit_history_for_pr( - repo_owner=repo_owner, repo_name=repo_name, pr_number=pr_number + commit_history = list_commit_history_for_pull_request( + repo_owner=repo_owner, + repo_name=repo_name, + pull_request_number=pull_request["number"], ) - latest_commit = sorted( + if is_empty(commit_history): + return {"has_unreviewed_commits": False} + + # Identify the latest commit + last_commit = sorted( commit_history, - key=lambda x: x["commit"]["committer"]["date"], + key=lambda x: parser.parse(x["commit"]["committer"]["date"]), reverse=True, )[0] + last_commit_id = last_commit["sha"] - approval_history = list_review_history_for_pr( + # Identify the last approved commit + approval_history = list_review_history_for_pull_request( repo_owner=repo_owner, repo_name=repo_name, - pr_number=pr_number, + pull_request_number=pull_request["number"], state="APPROVED", ) + approval_history = sorted( + approval_history, key=lambda x: parser.parse(x["submitted_at"]), reverse=True + ) - # If there are no approvals, the latest approved commit is the last commit on the branch from which the PR was created - if not approval_history: - latest_approval_id = pull_request.get("base").get("sha") - else: - latest_approval = approval_history[0] - latest_approval_id = latest_approval.get("commit_id") + # filter out self-approvals + # => we only disallow self-approvals of the user who created the PR + approval_history = [ + approval + for approval in approval_history + if approval["user"]["login"] != pull_request["user"]["login"] + ] - latest_commit_id = latest_commit.get("sha") + if is_empty(approval_history): + return { + "has_unreviewed_commits": True, + "last_commit_id": last_commit_id, + "last_approved_commit_id": pull_request["base"]["sha"], + } - if latest_commit_id != latest_approval_id: + last_approved_commit_id = approval_history[0]["commit_id"] + if last_approved_commit_id != last_commit_id: return { "has_unreviewed_commits": True, - "latest_commit_id": latest_commit_id, - "latest_reviewed_commit": latest_approval_id, + "last_commit_id": last_commit_id, + "last_approved_commit_id": last_approved_commit_id, } return {"has_unreviewed_commits": False} @action( - display_name="Follow up on PR with Unreviewed Commits", + display_name="Count Number of Changes in Git Diff", display_namespace="GitHub", - description="Follow up on PR with unreviewed commits", - secrets_placeholders=["GITHUB_SECRET"], -) -def follow_up_on_pr_with_unreviewed_commits( - pull_request: Annotated[ - dict[str, JsonValue], - ArgumentMetadata( - display_name="Merged Pull Request", - description="Pull request of interest", - ), - ], - repo_owner: Annotated[ - str, - ArgumentMetadata( - display_name="Repository Owner", - description="The owner of the repository", - ), - ], - repo_name: Annotated[ - str, - ArgumentMetadata( - display_name="Repository Name", - description="The name of the repository", - ), - ], -) -> bool: - pr_number = pull_request.get("number") - approval_history = sorted( - list_review_history_for_pr( - repo_owner=repo_owner, - repo_name=repo_name, - pr_number=pr_number, - state="APPROVED", - ), - key=lambda x: x["submitted_at"], - reverse=True, - ) - - if approval_history: - latest_approval = approval_history[0] - date_of_last_review = latest_approval.get("submitted_at") - else: - date_of_last_review = pull_request.get("created_at") - - commit_history = list_commit_history_for_pr( - repo_owner=repo_owner, repo_name=repo_name, pr_number=pr_number - ) - unreviewd_commit_history = [ - commit - for commit in commit_history - if commit.get("commit").get("committer").get("date") > date_of_last_review - ] - - excluded_reviewers = [] - - for assigne in pull_request.get("assignees"): - person = assigne.get("login") - if person not in excluded_reviewers: - excluded_reviewers.append(assigne.get("login")) - for commit in unreviewd_commit_history: - committer = commit.get("committer").get("id") - author = commit.get("author").get("id") - if committer not in excluded_reviewers: - excluded_reviewers.append(committer) - if author not in excluded_reviewers: - excluded_reviewers.append(author) - - review_history = list_review_history_for_pr( - repo_owner=repo_owner, repo_name=repo_name, pr_number=pr_number, state=None - ) - - merged_at = pull_request.get("merged_at") - - # TODO: After merge, or after the latest commit? - review_history_after_merge = [ - review for review in review_history if review.get("submitted_at") > merged_at - ] - - for review in review_history_after_merge: - user_info = review.get("user") - if ( - user_info.get("id") not in excluded_reviewers - or user_info.get("login") not in excluded_reviewers - ): - return True - else: - return False - - -@action( - display_name="Build Slack Message Based on Git Diff Analysis", - display_namespace="Utility", - description="Analyze the analysis of a Git diff and build a slack message", + description="Count the number of changes in a git diff", ) -def build_slack_message_based_on_ai_decision( - git_diff_analysis: Annotated[ - str, - ArgumentMetadata( - display_name="Git Diff Analysis", - description="Analysis of a Git diff", - ), - ], -) -> str: - message = "" - - if git_diff_analysis.startswith("1"): - message += "Major changes detected:\n" - message += f"Summary of the changes: {git_diff_analysis}\n" - - return message - - -@action( - display_name="Check Number of Changes in Git Diff", - display_namespace="GitHub", - description="Check if the git diff is too large to be interpreted by the AI", -) -def check_number_of_changes_in_git_diff( +def count_number_of_changes_in_git_diff( git_diff: Annotated[ dict[str, JsonValue], ArgumentMetadata( @@ -209,18 +115,13 @@ def check_number_of_changes_in_git_diff( description="The git diff to be checked", ), ], -) -> bool: - changes = 0 - - for changed_file in git_diff.get("files"): - changes += changed_file.get("changes") - - return changes > 50 +) -> int: + return sum(map(lambda file: file["changes"], git_diff["files"])) @workflow(description="Handle PR with unreviewed commits", triggers=[Webhook()]) def handle_pr_with_unreviewed_commits(payload: dict[str, JsonValue]): - unreviewd_commits_result = check_pr_for_unreviewed_commits( + unreviewd_commits_result = has_pr_unreviewed_commits( pull_request=payload["pull_request"], repo_owner=payload["repo_owner"], repo_name=payload["repo_name"], @@ -228,54 +129,27 @@ def handle_pr_with_unreviewed_commits(payload: dict[str, JsonValue]): ) if unreviewd_commits_result["has_unreviewed_commits"]: - commit_diff_info = get_commit_diff_info_between_two_commits( + commit_diff_info = compare_two_github_commits( repo_owner=payload["repo_owner"], repo_name=payload["repo_name"], - base=unreviewd_commits_result["latest_reviewed_commit"], - head=unreviewd_commits_result["latest_commit_id"], + base=unreviewd_commits_result["last_approved_commit_id"], + head=unreviewd_commits_result["last_commit_id"], + diff_type="json", secrets={"GITHUB_SECRET": "github_secret"}, ) - git_diff_is_too_large = check_number_of_changes_in_git_diff( + line_changes_count = count_number_of_changes_in_git_diff( git_diff=commit_diff_info ) - if git_diff_is_too_large: - jira_issue = create_jira_issue( - summary=f"Unreviewed commits in merged Pull Request {payload["pull_request"]['html_url']}.", - project_id="10001", - issue_type="Bug", - description={ - "content": [ - { - "content": [ - { - "text": "Unreviewed changes with too many changes. Please review manually.", - "type": "text", - } - ], - "type": "paragraph", - }, - ], - "type": "doc", - "version": 1, - }, - priority="High", - secrets={"JIRA_SECRET": "jira_secret"}, - ) - - send_slack_message_to_user_by_email( - email="leon@admyral.ai", - text=f"Unreviewed commits in merged Pull Request ({payload["pull_request"]['html_url']}).\n Jira issue created: https://christesting123.atlassian.net/browse/{jira_issue['key']} \n--------------\n", - secrets={"SLACK_SECRET": "slack_secret"}, - ) - - else: - commit_diff = get_raw_commit_diff_between_two_commits( + if line_changes_count < 50: + # Perform classification of the git diff changes + commit_diff = compare_two_github_commits( repo_owner=payload["repo_owner"], repo_name=payload["repo_name"], - base=unreviewd_commits_result["latest_reviewed_commit"], - head=unreviewd_commits_result["latest_commit_id"], + base=unreviewd_commits_result["last_approved_commit_id"], + head=unreviewd_commits_result["last_commit_id"], + diff_type="diff", secrets={"GITHUB_SECRET": "github_secret"}, ) @@ -302,7 +176,7 @@ def new_feature():\ secrets={"OPENAI_SECRET": "openai_secret"}, ) - git_diff_major_minor_change_decision = openai_chat_completion( + is_major_change = openai_chat_completion( model="gpt-4o", prompt=f"You are an expert software engineer and should interpret summaries of git diffs, if there were major or only minor changes.\ Major changes would be adding a new feature or changing the functionality of an existing feature, hence potentially breaking changes.\ @@ -311,57 +185,75 @@ def new_feature():\ 1. The changes should get reviewed as they include potentially breaking changes or alter the functionality of the code.\ 2. The changes don't have to be reviewed as they only include minor changes which do not alter or add functionality of the software in any way.\ Here is the summary of the changes:\n{git_diff_summary}\ - Please only answer with either 1 or 2 and nothing else.", + Please only answer with either 1 or 2 followed by \n and nothing else.", + stop_tokens=["\n"], secrets={"OPENAI_SECRET": "openai_secret"}, ) + else: + # The diff is too large, hence, we have a major change + is_major_change = transform(value="1") - message = build_slack_message_based_on_ai_decision( - git_diff_analysis=git_diff_major_minor_change_decision, + if is_major_change == "1": + jira_issue = create_jira_issue( + summary=f"Unreviewed commits were merged. Pull Request: {payload["pull_request"]["title"]}", + project_id="10001", # TODO: set your project id here + issue_type="Bug", + description={ + "content": [ + { + "content": [ + { + "text": f"Title: {payload['pull_request']['title']}\n\ + Description: {payload['pull_request']['body']}\n\ + Repository: {payload['repo_owner']}/{payload['repo_name']}\n\ + User: {payload['pull_request']['user']['login']}\n\ + Link: {payload['pull_request']['html_url']}\n\ + Closed At: {payload['pull_request']['closed_at']}\n", + "type": "text", + } + ], + "type": "paragraph", + }, + ], + "type": "doc", + "version": 1, + }, + priority="High", + secrets={"JIRA_SECRET": "jira_secret"}, ) - if message: - jira_issue = create_jira_issue( - summary=f"Unreviewed commits in merged Pull Request {payload["pull_request"]['html_url']}", - project_id="10001", - issue_type="Bug", - description={ - "content": [ - { - "content": [ - { - "text": f"AI Summary: {git_diff_summary}", - "type": "text", - } - ], - "type": "paragraph", - }, - ], - "type": "doc", - "version": 1, - }, - priority="High", - secrets={"JIRA_SECRET": "jira_secret"}, - ) + # Send to an alert channel + first_message = send_slack_message( + channel_id="C06QP0KV1L2", # TODO: set your slack channel here + # TODO: set your Jira URL in the test message + text=f"Unreviewed commits were merged. \n \ + Pull Request: {payload["pull_request"]["title"]} \n \ + {payload["pull_request"]["html_url"]} \n\ + User: {payload['pull_request']['user']['login']}\n\ + Jira issue : https://admyral.atlassian.net/browse/{jira_issue["key"]} \n\ + \n \ + Please let the changes be reviewed by a peer within the next 24 hours and close the ticket after the review.", + secrets={"SLACK_SECRET": "slack_secret"}, + ) - first_message = send_slack_message_to_user_by_email( - email="leon@admyral.ai", - text=f"Unreviewed commits in merged Pull Request ({payload["pull_request"]['html_url']}).\n{git_diff_summary}\n Jira issue created: https://christesting123.atlassian.net/browse/{jira_issue['key']} \n--------------\n", - secrets={"SLACK_SECRET": "slack_secret"}, - ) + wait_res = wait( + seconds=120, run_after=[first_message] + ) # TODO: configure your wait time here - wait_res = wait(seconds=120, run_after=[first_message]) + # check again whether there are still unreviewed commits + unreviewd_commits_result = has_pr_unreviewed_commits( + pull_request=payload["pull_request"], + repo_owner=payload["repo_owner"], + repo_name=payload["repo_name"], + secrets={"GITHUB_SECRET": "github_secret"}, + run_after=[wait_res], + ) - follow_up_done = follow_up_on_pr_with_unreviewed_commits( - pull_request=payload["pull_request"], - repo_owner=payload["repo_owner"], - repo_name=payload["repo_name"], - run_after=[wait_res], - secrets={"GITHUB_SECRET": "github_secret"}, + if unreviewd_commits_result["has_unreviewed_commits"]: + # Send message to compliance manager + send_slack_message_to_user_by_email( + email="daniel@admyral.dev", # TODO: set your email here + text=f"ATTENTION: There was no follow up on the merged unreviewed commits of pull request {payload['pull_request']['title']}.\n \ + Jira Ticket: https://admyral.atlassian.net/browse/{jira_issue['key']}.", # TODO: set your Jira URL here + secrets={"SLACK_SECRET": "slack_secret"}, ) - - if not follow_up_done: - send_slack_message_to_user_by_email( - email="leon@admyral.ai", - text=f"There was no follow up on the merged Pull Request {payload['pull_request']['html_url']} with unreviewed commits after the initial message. Please check the Pull Request, follow up accordingly and close the Jira Ticket https://christesting123.atlassian.net/browse/{jira_issue['key']}.\n--------------\n", - secrets={"SLACK_SECRET": "slack_secret"}, - ) diff --git a/examples/access_review/workflows/github_unreviewed_commits_are_merged.py b/examples/access_review/workflows/github_unreviewed_commits_are_merged.py index 587bfaff..81a2a98f 100644 --- a/examples/access_review/workflows/github_unreviewed_commits_are_merged.py +++ b/examples/access_review/workflows/github_unreviewed_commits_are_merged.py @@ -6,9 +6,7 @@ from admyral.workflow import workflow, Schedule from admyral.typings import JsonValue from admyral.action import action, ArgumentMetadata -from admyral.actions import ( - list_merged_prs, -) +from admyral.actions import list_merged_pull_requests @action( @@ -18,18 +16,18 @@ ) def get_time_range_of_last_full_hour() -> tuple[str, str]: end_time = datetime.now(UTC).replace(minute=0, second=0, microsecond=0) - start_time = (end_time - timedelta(hours=1)).isoformat() + "Z" - return (start_time, end_time.isoformat() + "Z") + start_time = (end_time - timedelta(hours=1)).isoformat().replace("+00:00", "Z") + return (start_time, end_time.isoformat().replace("+00:00", "Z")) @action( - display_name="Check PRs for merging unreviewed commits", + display_name="Check closed PRs for unreviewed merged commits", display_namespace="Utilities", description="Iterate over all PRs and check if approved commit is last commit", secrets_placeholders=["GITHUB_SECRET"], requirements=["requests"], ) -def check_prs_for_merging_unreviewed_commits( +def check_pull_requests_for_unreviewed_merged_commits( pull_requests: Annotated[ list[dict[str, JsonValue]], ArgumentMetadata( @@ -52,8 +50,12 @@ def check_prs_for_merging_unreviewed_commits( ), ], ) -> None: + # TODO: update webhook_id and webhook_secret with the values form the "handle_pr_with_unreviewed_commits" workflow + webhook_id = "0effe4ca-0385-4786-a36d-6e6172ff25b0" + webhook_secret = "cb0762a35395230f595c2487878a257e45762ea0286b62b92db542de4740f63c" + webhook_url = f"http://127.0.0.1:8000/webhooks/{webhook_id}/{webhook_secret}" + for pr in pull_requests: - webhook_url = "http://127.0.0.1:8000/webhooks/be941ac5-4d35-4fc3-baf3-5c1eb65d5096/3ea17bf42a97e58e417b73a133528bf9aab7bab904e1c3fbafdb65e0b18dc66e" payload = { "repo_owner": repo_owner, "repo_name": repo_name, @@ -67,23 +69,23 @@ def check_prs_for_merging_unreviewed_commits( @workflow( description="Alert if unreviewed commits are merged, which should have been reviewed", - triggers=[Schedule(cron="0 * * * *", repo_name="", repo_owner="")], + triggers=[Schedule(cron="0 * * * *")], ) def github_unreviewed_commits_are_merged(payload: dict[str, JsonValue]): start_end_end_time = get_time_range_of_last_full_hour() - merged_prs_to_check = list_merged_prs( - repo_owner=payload["repo_owner"], - repo_name=payload["repo_name"], + merged_prs = list_merged_pull_requests( + repo_owner="admyral", # TODO: set repo owner + repo_name="admyral", # TODO: set repo name start_time=start_end_end_time[0], end_time=start_end_end_time[1], secrets={"GITHUB_SECRET": "github_secret"}, ) - if merged_prs_to_check: - check_prs_for_merging_unreviewed_commits( - pull_requests=merged_prs_to_check, - repo_name=payload["repo_name"], - repo_owner=payload["repo_owner"], + if merged_prs: + check_pull_requests_for_unreviewed_merged_commits( + pull_requests=merged_prs, + repo_owner="admyral", # TODO: set repo owner + repo_name="admyral", # TODO: set repo name secrets={"GITHUB_SECRET": "github_secret"}, )