From d08403b1ecd4dc7e316429d0b9369f3646cfcbe9 Mon Sep 17 00:00:00 2001 From: Sandy Carter Date: Wed, 17 Jan 2024 16:28:51 -0500 Subject: [PATCH 1/3] github wrapper: Use Auth class to encapsulate token --- .../clang_tidy_review/__init__.py | 24 +++++++++++++++---- .../clang_tidy_review/post.py | 6 +++-- .../clang_tidy_review/review.py | 6 +++-- 3 files changed, 28 insertions(+), 8 deletions(-) diff --git a/post/clang_tidy_review/clang_tidy_review/__init__.py b/post/clang_tidy_review/clang_tidy_review/__init__.py index 586f4f1..a6a6855 100644 --- a/post/clang_tidy_review/clang_tidy_review/__init__.py +++ b/post/clang_tidy_review/clang_tidy_review/__init__.py @@ -3,6 +3,7 @@ # SPDX-License-Identifier: MIT # See LICENSE for more information +import argparse import fnmatch import itertools import json @@ -20,7 +21,7 @@ import re import io import zipfile -from github import Github +from github import Github, Auth from github.Requester import Requester from github.PaginatedList import PaginatedList from github.WorkflowRun import WorkflowRun @@ -58,6 +59,17 @@ class PRReview(TypedDict): comments: List[PRReviewComment] +def add_auth_arguments(parser: argparse.ArgumentParser): + parser.add_argument("--token", help="github auth token") + + +def get_auth_from_arguments(args: argparse.Namespace) -> Auth: + if args.token: + return Auth.Token(args.token) + + raise argparse.ArgumentError(None, "authentication method not supplied") + + def build_clang_tidy_warnings( line_filter, build_dir, @@ -160,18 +172,22 @@ def load_clang_tidy_warnings(): class PullRequest: """Add some convenience functions not in PyGithub""" - def __init__(self, repo: str, pr_number: Optional[int], token: str) -> None: + def __init__(self, repo: str, pr_number: Optional[int], auth: Auth) -> None: self.repo_name = repo self.pr_number = pr_number - self.token = token + self.auth = auth # Choose API URL, default to public GitHub self.api_url = os.environ.get("GITHUB_API_URL", "https://api.github.com") - github = Github(token, base_url=self.api_url) + github = Github(auth=self.auth, base_url=self.api_url) self.repo = github.get_repo(f"{repo}") self._pull_request = None + @property + def token(self): + return self.auth.token + @property def pull_request(self): if self._pull_request is None: diff --git a/post/clang_tidy_review/clang_tidy_review/post.py b/post/clang_tidy_review/clang_tidy_review/post.py index 78d306e..d949884 100755 --- a/post/clang_tidy_review/clang_tidy_review/post.py +++ b/post/clang_tidy_review/clang_tidy_review/post.py @@ -19,6 +19,8 @@ post_annotations, bool_argument, REVIEW_FILE, + add_auth_arguments, + get_auth_from_arguments, ) @@ -40,7 +42,6 @@ def main() -> int: type=str, default='clang-tidy review says "All clean, LGTM! :+1:"', ) - parser.add_argument("--token", help="github auth token") parser.add_argument( "--dry-run", help="Run and generate review, but don't post", action="store_true" ) @@ -55,6 +56,7 @@ def main() -> int: type=bool_argument, default=False, ) + add_auth_arguments(parser) parser.add_argument( "reviews", metavar="REVIEW_FILES", @@ -66,7 +68,7 @@ def main() -> int: args = parser.parse_args() - pull_request = PullRequest(args.repo, None, args.token) + pull_request = PullRequest(args.repo, None, get_auth_from_arguments(args)) # Try to read the review artifacts if they're already present metadata = load_metadata() diff --git a/post/clang_tidy_review/clang_tidy_review/review.py b/post/clang_tidy_review/clang_tidy_review/review.py index a3d3357..dbb76f7 100755 --- a/post/clang_tidy_review/clang_tidy_review/review.py +++ b/post/clang_tidy_review/clang_tidy_review/review.py @@ -22,6 +22,8 @@ post_annotations, bool_argument, set_output, + add_auth_arguments, + get_auth_from_arguments, ) @@ -110,10 +112,10 @@ def main(): type=bool_argument, default=False, ) - parser.add_argument("--token", help="github auth token") parser.add_argument( "--dry-run", help="Run and generate review, but don't post", action="store_true" ) + add_auth_arguments(parser) args = parser.parse_args() @@ -147,7 +149,7 @@ def main(): elif os.path.exists(build_compile_commands): fix_absolute_paths(build_compile_commands, args.base_dir) - pull_request = PullRequest(args.repo, args.pr, args.token) + pull_request = PullRequest(args.repo, args.pr, get_auth_from_arguments(args)) review = create_review( pull_request, From 8f5e16ad0d87b964d3de23825dbd71a10e6ee98d Mon Sep 17 00:00:00 2001 From: Sandy Carter Date: Wed, 17 Jan 2024 17:26:16 -0500 Subject: [PATCH 2/3] Add github app authentication arguments --- .../clang_tidy_review/__init__.py | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/post/clang_tidy_review/clang_tidy_review/__init__.py b/post/clang_tidy_review/clang_tidy_review/__init__.py index a6a6855..f76f051 100644 --- a/post/clang_tidy_review/clang_tidy_review/__init__.py +++ b/post/clang_tidy_review/clang_tidy_review/__init__.py @@ -60,13 +60,34 @@ class PRReview(TypedDict): def add_auth_arguments(parser: argparse.ArgumentParser): + # Token parser.add_argument("--token", help="github auth token") + # App + group_app = parser.add_argument_group("github app installation auth") + group_app.add_argument("--app-id", type=int, help="app ID") + group_app.add_argument("--private-key", type=str, help="app private key as a string") + group_app.add_argument("--private-key-file-path", type=str, help="app private key .pom file path") + group_app.add_argument("--installation-id", type=int, help="app installation ID") def get_auth_from_arguments(args: argparse.Namespace) -> Auth: if args.token: return Auth.Token(args.token) + if args.app_id and (args.private_key or args.private_key_file_path) and args.installation_id: + if args.private_key: + private_key = args.private_key + else: + private_key = open(args.private_key_file_path).read() + return Auth.AppAuth(args.app_id, private_key).get_installation_auth( + args.installation_id + ) + elif args.app_id or args.private_key or args.private_key_file_path or args.installation_id: + raise argparse.ArgumentError( + None, + "--app-id, --private-key[-file-path] and --installation-id must be supplied together", + ) + raise argparse.ArgumentError(None, "authentication method not supplied") From 72b3a12d9bb044aa6b6b610a98333ef6e3bd70e9 Mon Sep 17 00:00:00 2001 From: bwrsandman Date: Thu, 18 Jan 2024 00:51:47 +0000 Subject: [PATCH 3/3] Apply black changes --- .../clang_tidy_review/__init__.py | 21 +++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/post/clang_tidy_review/clang_tidy_review/__init__.py b/post/clang_tidy_review/clang_tidy_review/__init__.py index f76f051..72b162b 100644 --- a/post/clang_tidy_review/clang_tidy_review/__init__.py +++ b/post/clang_tidy_review/clang_tidy_review/__init__.py @@ -65,8 +65,12 @@ def add_auth_arguments(parser: argparse.ArgumentParser): # App group_app = parser.add_argument_group("github app installation auth") group_app.add_argument("--app-id", type=int, help="app ID") - group_app.add_argument("--private-key", type=str, help="app private key as a string") - group_app.add_argument("--private-key-file-path", type=str, help="app private key .pom file path") + group_app.add_argument( + "--private-key", type=str, help="app private key as a string" + ) + group_app.add_argument( + "--private-key-file-path", type=str, help="app private key .pom file path" + ) group_app.add_argument("--installation-id", type=int, help="app installation ID") @@ -74,7 +78,11 @@ def get_auth_from_arguments(args: argparse.Namespace) -> Auth: if args.token: return Auth.Token(args.token) - if args.app_id and (args.private_key or args.private_key_file_path) and args.installation_id: + if ( + args.app_id + and (args.private_key or args.private_key_file_path) + and args.installation_id + ): if args.private_key: private_key = args.private_key else: @@ -82,7 +90,12 @@ def get_auth_from_arguments(args: argparse.Namespace) -> Auth: return Auth.AppAuth(args.app_id, private_key).get_installation_auth( args.installation_id ) - elif args.app_id or args.private_key or args.private_key_file_path or args.installation_id: + elif ( + args.app_id + or args.private_key + or args.private_key_file_path + or args.installation_id + ): raise argparse.ArgumentError( None, "--app-id, --private-key[-file-path] and --installation-id must be supplied together",