Skip to content

Commit

Permalink
fix: use GitHub App token when authed with GitHub App (#257)
Browse files Browse the repository at this point in the history
* fix: use GitHub App token when authed with GitHub App

Currently we are still trying to use the GH_TOKEN env var when making api/graphql calls even after authenticating with a GitHub App Installation. This change fixes that.

Also fixed a few other things while in here:

- [x] split authentication to auth.py file (like other actions)
- [x] fix arguments to the count_comments_per_user function
- [x] add `maintenance` to pull request template

Signed-off-by: jmeridth <[email protected]>

* test: Add test to cover ignored users in mentor_count

Signed-off-by: Zack Koppert <[email protected]>

---------

Signed-off-by: jmeridth <[email protected]>
Signed-off-by: Zack Koppert <[email protected]>
Co-authored-by: Zack Koppert <[email protected]>
  • Loading branch information
jmeridth and zkoppert authored Apr 29, 2024
1 parent 0b5e8b4 commit fb94346
Show file tree
Hide file tree
Showing 7 changed files with 192 additions and 83 deletions.
2 changes: 1 addition & 1 deletion .github/pull_request_template.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,4 +21,4 @@ examples: "feat: add new logger" or "fix: remove unused imports"

### Reviewer

- [ ] Label as either `fix`, `documentation`, `enhancement`, `infrastructure`, or `breaking`
- [ ] Label as either `fix`, `documentation`, `enhancement`, `infrastructure`, `maintenance`, or `breaking`
63 changes: 63 additions & 0 deletions auth.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
"""
This is the module that contains functions related to authenticating
to GitHub.
"""

import github3
import requests


def auth_to_github(
gh_app_id: str,
gh_app_installation_id: int,
gh_app_private_key_bytes: bytes,
token: str,
ghe: str,
) -> github3.GitHub:
"""
Connect to GitHub.com or GitHub Enterprise, depending on env variables.
Returns:
github3.GitHub: A github api connection.
"""

if gh_app_id and gh_app_private_key_bytes and gh_app_installation_id:
gh = github3.github.GitHub()
gh.login_as_app_installation(
gh_app_private_key_bytes, gh_app_id, gh_app_installation_id
)
github_connection = gh
elif ghe and token:
github_connection = github3.github.GitHubEnterprise(ghe, token=token)
elif token:
github_connection = github3.login(token=token)
else:
raise ValueError(
"GH_TOKEN or the set of [GH_APP_ID, GH_APP_INSTALLATION_ID, GH_APP_PRIVATE_KEY] environment variables are not set"
)

return github_connection # type: ignore


def get_github_app_installation_token(
gh_app_id: str, gh_app_private_key_bytes: bytes, gh_app_installation_id: str
) -> str | None:
"""
Get a GitHub App Installation token.
Args:
gh_app_id (str): the GitHub App ID
gh_app_private_key_bytes (bytes): the GitHub App Private Key
gh_app_installation_id (str): the GitHub App Installation ID
Returns:
str: the GitHub App token
"""
jwt_headers = github3.apps.create_jwt_headers(gh_app_private_key_bytes, gh_app_id)
url = f"https://api.github.com/app/installations/{gh_app_installation_id}/access_tokens"

try:
response = requests.post(url, headers=jwt_headers, json=None, timeout=5)
response.raise_for_status()
except requests.exceptions.RequestException as e:
print(f"Request failed: {e}")
return None
return response.json().get("token")
51 changes: 14 additions & 37 deletions issue_metrics.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
search_issues(search_query: str, github_connection: github3.GitHub)
-> github3.structs.SearchIterator:
Searches for issues in a GitHub repository that match the given search query.
auth_to_github() -> github3.GitHub: Connect to GitHub API with token authentication.
get_per_issue_metrics(issues: Union[List[dict], List[github3.issues.Issue]],
discussions: bool = False), labels: Union[List[str], None] = None,
ignore_users: List[str] = [] -> tuple[List, int, int]:
Expand All @@ -25,6 +24,7 @@
from typing import List, Union

import github3
from auth import auth_to_github, get_github_app_installation_token
from classes import IssueWithMetrics
from config import EnvVars, get_env_vars
from discussions import get_discussions
Expand Down Expand Up @@ -92,38 +92,6 @@ def search_issues(
return issues


def auth_to_github(
gh_app_id: str,
gh_app_installation_id: int,
gh_app_private_key_bytes: bytes,
token: str,
ghe: str,
) -> github3.GitHub:
"""
Connect to GitHub.com or GitHub Enterprise, depending on env variables.
Returns:
github3.GitHub: A github api connection.
"""

if gh_app_id and gh_app_private_key_bytes and gh_app_installation_id:
gh = github3.github.GitHub()
gh.login_as_app_installation(
gh_app_private_key_bytes, gh_app_id, gh_app_installation_id
)
github_connection = gh
elif ghe and token:
github_connection = github3.github.GitHubEnterprise(ghe, token=token)
elif token:
github_connection = github3.login(token=token)
else:
raise ValueError(
"GH_TOKEN or the set of [GH_APP_ID, GH_APP_INSTALLATION_ID, GH_APP_PRIVATE_KEY] environment variables are not set"
)

return github_connection # type: ignore


def get_per_issue_metrics(
issues: Union[List[dict], List[github3.search.IssueSearchResult]], # type: ignore
env_vars: EnvVars,
Expand Down Expand Up @@ -175,9 +143,9 @@ def get_per_issue_metrics(
issue_with_metrics.mentor_activity = count_comments_per_user(
None,
issue,
ignore_users,
None,
None,
ignore_users,
max_comments_to_eval,
heavily_involved,
)
Expand Down Expand Up @@ -289,11 +257,20 @@ def main():
token = env_vars.gh_token
ignore_users = env_vars.ignore_users

gh_app_id = env_vars.gh_app_id
gh_app_installation_id = env_vars.gh_app_installation_id
gh_app_private_key_bytes = env_vars.gh_app_private_key_bytes

if not token and gh_app_id and gh_app_installation_id and gh_app_private_key_bytes:
token = get_github_app_installation_token(
gh_app_id, gh_app_private_key_bytes, gh_app_installation_id
)

# Auth to GitHub.com
github_connection = auth_to_github(
env_vars.gh_app_id,
env_vars.gh_app_installation_id,
env_vars.gh_app_private_key_bytes,
gh_app_id,
gh_app_installation_id,
gh_app_private_key_bytes,
token,
env_vars.ghe,
)
Expand Down
4 changes: 2 additions & 2 deletions most_active_mentors.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,8 @@ def count_comments_per_user(
Args:
issue (Union[github3.issues.Issue, None]): A GitHub issue.
pull_request (Union[github3.pulls.PullRequest, None]): A GitHub pull
request. ignore_users (List[str]): A list of GitHub usernames to
ignore.
request.
ignore_users (List[str]): A list of GitHub usernames to ignore.
max_comments_to_eval: Maximum number of comments per item to look at.
heavily_involved: Maximum number of comments to count for one
user per issue.
Expand Down
72 changes: 72 additions & 0 deletions test_auth.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
"""A module containing unit tests for the auth module.
This module contains unit tests for the functions in the auth module
that authenticate to github.
Classes:
TestAuthToGithub: A class to test the auth_to_github function.
"""

import unittest
from unittest.mock import MagicMock, patch

import github3
from auth import auth_to_github, get_github_app_installation_token


class TestAuthToGithub(unittest.TestCase):
"""Test the auth_to_github function."""

@patch("github3.github.GitHub.login_as_app_installation")
def test_auth_to_github_with_github_app(self, mock_login):
"""
Test the auth_to_github function when GitHub app
parameters provided.
"""
mock_login.return_value = MagicMock()
result = auth_to_github(12345, 678910, b"hello", "", "")

self.assertIsInstance(result, github3.github.GitHub)

def test_auth_to_github_with_token(self):
"""
Test the auth_to_github function when the token is provided.
"""
result = auth_to_github(None, None, b"", "token", "")

self.assertIsInstance(result, github3.github.GitHub)

def test_auth_to_github_without_authentication_information(self):
"""
Test the auth_to_github function when authentication information is not provided.
Expect a ValueError to be raised.
"""
with self.assertRaises(ValueError):
auth_to_github(None, None, b"", "", "")

def test_auth_to_github_with_ghe(self):
"""
Test the auth_to_github function when the GitHub Enterprise URL is provided.
"""
result = auth_to_github(None, None, b"", "token", "https://github.example.com")

self.assertIsInstance(result, github3.github.GitHubEnterprise)

@patch("github3.apps.create_jwt_headers", MagicMock(return_value="gh_token"))
@patch("requests.post")
def test_get_github_app_installation_token(self, mock_post):
"""
Test the get_github_app_installation_token function.
"""
dummy_token = "dummytoken"
mock_response = MagicMock()
mock_response.raise_for_status.return_value = None
mock_response.json.return_value = {"token": dummy_token}
mock_post.return_value = mock_response

result = get_github_app_installation_token(
b"gh_private_token", "gh_app_id", "gh_installation_id"
)

self.assertEqual(result, dummy_token)
42 changes: 0 additions & 42 deletions test_issue_metrics.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
Classes:
TestSearchIssues: A class to test the search_issues function.
TestAuthToGithub: A class to test the auth_to_github function.
TestGetPerIssueMetrics: A class to test the get_per_issue_metrics function.
TestGetEnvVars: A class to test the get_env_vars function.
TestMain: A class to test the main function.
Expand All @@ -18,11 +17,9 @@
from datetime import datetime, timedelta
from unittest.mock import MagicMock, patch

import github3
import issue_metrics
from issue_metrics import (
IssueWithMetrics,
auth_to_github,
get_env_vars,
get_owner_and_repository,
get_per_issue_metrics,
Expand Down Expand Up @@ -94,45 +91,6 @@ def test_get_owner_and_repository_without_either_in_query(self):
self.assertIsNone(result.get("repository"))


class TestAuthToGithub(unittest.TestCase):
"""Test the auth_to_github function."""

@patch("github3.github.GitHub.login_as_app_installation")
def test_auth_to_github_with_github_app(self, mock_login):
"""
Test the auth_to_github function when GitHub app
parameters provided.
"""
mock_login.return_value = MagicMock()
result = auth_to_github(12345, 678910, b"hello", "", "")

self.assertIsInstance(result, github3.github.GitHub)

def test_auth_to_github_with_token(self):
"""
Test the auth_to_github function when the token is provided.
"""
result = auth_to_github(None, None, b"", "token", "")

self.assertIsInstance(result, github3.github.GitHub)

def test_auth_to_github_without_authentication_information(self):
"""
Test the auth_to_github function when authentication information is not provided.
Expect a ValueError to be raised.
"""
with self.assertRaises(ValueError):
auth_to_github(None, None, b"", "", "")

def test_auth_to_github_with_ghe(self):
"""
Test the auth_to_github function when the GitHub Enterprise URL is provided.
"""
result = auth_to_github(None, None, b"", "token", "https://github.example.com")

self.assertIsInstance(result, github3.github.GitHubEnterprise)


class TestGetEnvVars(unittest.TestCase):
"""Test suite for the get_env_vars function."""

Expand Down
41 changes: 40 additions & 1 deletion test_most_active_mentors.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
class TestCountCommentsPerUser(unittest.TestCase):
"""Test the count_comments_per_user function."""

def test_count_comments_per_user(self):
def test_count_comments_per_user_limit(self):
"""Test that count_comments_per_user correctly counts user comments.
This test mocks the GitHub connection and issue comments, and checks
Expand Down Expand Up @@ -54,6 +54,45 @@ def test_count_comments_per_user(self):
# Check the results
self.assertEqual(result, expected_result)

def test_count_comments_per_user_with_ignores(self):
"""Test that count_comments_per_user correctly counts user comments with some users ignored."""
# Set up the mock GitHub issues
mock_issue1 = MagicMock()
mock_issue1.comments = 2
mock_issue1.issue.user.login = "issue_owner"
mock_issue1.created_at = "2023-01-01T00:00:00Z"

# Set up mock GitHub issue comments by several users
mock_issue1.issue.comments.return_value = []
for i in range(5):
mock_comment1 = MagicMock()
mock_comment1.user.login = "very_active_user"
mock_comment1.created_at = datetime.fromisoformat(
f"2023-01-02T{i:02d}:00:00Z"
)
# pylint: disable=maybe-no-member
mock_issue1.issue.comments.return_value.append(mock_comment1)
for i in range(5):
mock_comment1 = MagicMock()
mock_comment1.user.login = "very_active_user_ignored"
mock_comment1.created_at = datetime.fromisoformat(
f"2023-01-02T{i:02d}:00:00Z"
)
# pylint: disable=maybe-no-member
mock_issue1.issue.comments.return_value.append(mock_comment1)

# Call the function
result = count_comments_per_user(
mock_issue1, ignore_users=["very_active_user_ignored"]
)
# Only the comments by "very_active_user" should be counted,
# so the count should be 3 since that is the threshold for heavily involved
expected_result = {"very_active_user": 3}

# Check the results
self.assertEqual(result, expected_result)
self.assertNotIn("very_active_user_ignored", result)

def test_get_mentor_count(self):
"""Test that get_mentor_count correctly counts comments per user."""
mentor_activity = {"sue": 15, "bob": 10}
Expand Down

0 comments on commit fb94346

Please sign in to comment.