Skip to content

Commit

Permalink
Merge pull request #90 from github/report-as-issue-markdown
Browse files Browse the repository at this point in the history
  • Loading branch information
zkoppert committed Jun 8, 2024
2 parents ab356ad + 0eeb3aa commit c8770cb
Show file tree
Hide file tree
Showing 8 changed files with 263 additions and 12 deletions.
9 changes: 9 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ This action can be configured to authenticate with GitHub App Installation or Pe
| `REPOSITORY` | Required to have `ORGANIZATION` or `REPOSITORY` | | The name of the repository and organization which you want this action to work from. ie. `github/cleanowners` or a comma separated list of multiple repositories `github/cleanowners,super-linter/super-linter` |
| `EXEMPT_REPOS` | False | "" | These repositories will be exempt from this action. ex: If my org is set to `github` then I might want to exempt a few of the repos but get the rest by setting `EXEMPT_REPOS` to `github/cleanowners,github/contributors` |
| `DRY_RUN` | False | False | If set to true, this action will not create any pull requests. It will only log the repositories that could have the `CODEOWNERS` file updated. This is useful for testing or discovering the scope of this issue in your organization. |
| `ISSUE_REPORT` | False | False | If set to true, this action will create an issue in the repository with the report on the repositories that had users removed from the `CODEOWNERS` file. |

### Example workflows

Expand Down Expand Up @@ -114,6 +115,14 @@ jobs:
GH_TOKEN: ${{ secrets.GH_TOKEN }}
ORGANIZATION: <YOUR_ORGANIZATION_GOES_HERE>
EXEMPT_REPOS: "org_name/repo_name_1, org_name/repo_name_2"
ISSUE_REPORT: true
- name: Create issue
uses: peter-evans/create-issue-from-file@v5
with:
title: Cleanowners Report
content-filepath: ./report.md
assignees: <YOUR_GITHUB_HANDLE_HERE>
token: ${{ secrets.GITHUB_TOKEN }}

```
Expand Down
30 changes: 28 additions & 2 deletions cleanowners.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import auth
import env
import github3
from markdown_writer import write_to_markdown


def get_org(github_connection, organization):
Expand Down Expand Up @@ -33,6 +34,7 @@ def main(): # pragma: no cover
title,
body,
commit_message,
issue_report,
) = env.get_env_vars()

# Auth to GitHub.com or GHE
Expand All @@ -59,6 +61,7 @@ def main(): # pragma: no cover
# Get the repositories from the organization or list of repositories
repos = get_repos_iterator(organization, repository_list, github_connection)

repo_and_users_to_remove = {}
for repo in repos:
# Check if the repository is in the exempt_repositories_list
if repo.full_name in exempt_repositories_list:
Expand Down Expand Up @@ -109,6 +112,8 @@ def main(): # pragma: no cover
# Extract the usernames from the CODEOWNERS file
usernames = get_usernames_from_codeowners(codeowners_file_contents)

usernames_to_remove = []
codeowners_file_contents_new = None
for username in usernames:
org = organization if organization else repo.owner.login
gh_org = get_org(github_connection, org)
Expand All @@ -122,6 +127,7 @@ def main(): # pragma: no cover
f"\t{username} is not a member of {org}. Suggest removing them from {repo.full_name}"
)
users_count += 1
usernames_to_remove.append(username)
if not dry_run:
# Remove that username from the codeowners_file_contents
file_changed = True
Expand All @@ -130,9 +136,18 @@ def main(): # pragma: no cover
codeowners_file_contents.decoded.replace(bytes_username, b"")
)

# Store the repo and users to remove for reporting later
if usernames_to_remove:
repo_and_users_to_remove[repo] = usernames_to_remove

# Update the CODEOWNERS file if usernames were removed
if file_changed:
eligble_for_pr_count += 1
new_usernames = get_usernames_from_codeowners(codeowners_file_contents_new)
if len(new_usernames) == 0:
print(
f"\twarning: All usernames removed from CODEOWNERS in {repo.full_name}."
)
try:
pull = commit_changes(
title,
Expand Down Expand Up @@ -166,6 +181,15 @@ def main(): # pragma: no cover
f"{round((codeowners_count / (codeowners_count + no_codeowners_count)) * 100, 2)}% of repositories had CODEOWNERS files"
)

if issue_report:
write_to_markdown(
users_count,
pull_count,
no_codeowners_count,
codeowners_count,
repo_and_users_to_remove,
)


def get_repos_iterator(organization, repository_list, github_connection):
"""Get the repositories from the organization or list of repositories"""
Expand All @@ -182,7 +206,7 @@ def get_repos_iterator(organization, repository_list, github_connection):
return repos


def get_usernames_from_codeowners(codeowners_file_contents):
def get_usernames_from_codeowners(codeowners_file_contents, ignore_teams=True):
"""Extract the usernames from the CODEOWNERS file"""
usernames = []
for line in codeowners_file_contents.decoded.splitlines():
Expand All @@ -201,7 +225,9 @@ def get_usernames_from_codeowners(codeowners_file_contents):
handle = handle.split()[0]
# Identify team handles by the presence of a slash.
# Ignore teams because non-org members cannot be in a team.
if "/" not in handle:
if ignore_teams and "/" not in handle:
usernames.append(handle)
elif not ignore_teams:
usernames.append(handle)
return usernames

Expand Down
32 changes: 23 additions & 9 deletions env.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,22 @@
from dotenv import load_dotenv


def get_bool_env_var(env_var_name: str, default: bool = False) -> bool:
"""Get a boolean environment variable.
Args:
env_var_name: The name of the environment variable to retrieve.
default: The default value to return if the environment variable is not set.
Returns:
The value of the environment variable as a boolean.
"""
ev = os.environ.get(env_var_name, "")
if ev == "" and default:
return default
return ev.strip().lower() == "true"


def get_int_env_var(env_var_name: str) -> int | None:
"""Get an integer environment variable.
Expand Down Expand Up @@ -41,6 +57,7 @@ def get_env_vars(
str,
str,
str,
bool,
]:
"""
Get the environment variables for use in the action.
Expand All @@ -61,6 +78,7 @@ def get_env_vars(
title (str): The title to use for the pull request
body (str): The body to use for the pull request
message (str): Commit message to use
issue_report (bool): Whether or not to create an issue report with the results
"""
if not test:
Expand Down Expand Up @@ -115,14 +133,7 @@ def get_env_vars(
repository.strip() for repository in exempt_repos.split(",")
]

dry_run = os.getenv("DRY_RUN")
dry_run = dry_run.lower() if dry_run else None
if dry_run:
if dry_run not in ("true", "false"):
raise ValueError("DRY_RUN environment variable not 'true' or 'false'")
dry_run_bool = dry_run == "true"
else:
dry_run_bool = False
dry_run = get_bool_env_var("DRY_RUN")

title = os.getenv("TITLE")
# make sure that title is a string with less than 70 characters
Expand Down Expand Up @@ -155,6 +166,8 @@ def get_env_vars(
"Remove users no longer in this organization from CODEOWNERS file"
)

issue_report = get_bool_env_var("ISSUE_REPORT")

return (
organization,
repositories_list,
Expand All @@ -164,8 +177,9 @@ def get_env_vars(
token,
ghe,
exempt_repositories_list,
dry_run_bool,
dry_run,
title,
body,
commit_message,
issue_report,
)
27 changes: 27 additions & 0 deletions markdown_writer.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
"""Write the results to a markdown file"""


def write_to_markdown(
users_count,
pull_count,
no_codeowners_count,
codeowners_count,
repo_and_users_to_remove,
):
"""Write the results to a markdown file"""
with open("report.md", "w", encoding="utf-8") as file:
file.write(
"# Cleanowners Report\n\n"
"## Overall Stats\n"
f"{users_count} Users to Remove\n"
f"{pull_count} Pull Requests created\n"
f"{no_codeowners_count} Repositories with no CODEOWNERS file\n"
f"{codeowners_count} Repositories with CODEOWNERS file\n"
)
if repo_and_users_to_remove:
file.write("## Repositories and Users to Remove\n")
for repo, users in repo_and_users_to_remove.items():
file.write(f"{repo}\n")
for user in users:
file.write(f"- {user}\n")
file.write("\n")
21 changes: 20 additions & 1 deletion test_cleanowners.py
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ def test_commit_changes(self, mock_uuid):
class TestGetUsernamesFromCodeowners(unittest.TestCase):
"""Test the get_usernames_from_codeowners function in cleanowners.py"""

def test_get_usernames_from_codeowners(self):
def test_get_usernames_from_codeowners_ignore_teams(self):
"""Test the get_usernames_from_codeowners function."""
codeowners_file_contents = MagicMock()
codeowners_file_contents.decoded = """
Expand All @@ -82,6 +82,25 @@ def test_get_usernames_from_codeowners(self):

self.assertEqual(result, expected_usernames)

def test_get_usernames_from_codeowners_with_teams(self):
"""Test the get_usernames_from_codeowners function."""
codeowners_file_contents = MagicMock()
codeowners_file_contents.decoded = """
# Comment
@user1
@user2
@org/team
# Another comment
@user3 @user4
""".encode(
"ASCII"
)
expected_usernames = ["user1", "user2", "org/team", "user3", "user4"]

result = get_usernames_from_codeowners(codeowners_file_contents, False)

self.assertEqual(result, expected_usernames)


class TestGetOrganization(unittest.TestCase):
"""Test the get_org function in cleanowners.py"""
Expand Down
7 changes: 7 additions & 0 deletions test_env.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ def setUp(self):
"ORGANIZATION",
"REPOSITORY",
"TITLE",
"ISSUE_REPORT",
]
for key in env_keys:
if key in os.environ:
Expand Down Expand Up @@ -67,6 +68,7 @@ def test_get_env_vars_with_org(self):
TITLE,
BODY,
COMMIT_MESSAGE,
False,
)
result = get_env_vars(True)
self.assertEqual(result, expected_result)
Expand Down Expand Up @@ -104,6 +106,7 @@ def test_get_env_vars_with_github_app_and_repos(self):
TITLE,
BODY,
COMMIT_MESSAGE,
False,
)
result = get_env_vars(True)
self.assertEqual(result, expected_result)
Expand Down Expand Up @@ -141,6 +144,7 @@ def test_get_env_vars_with_token_and_repos(self):
TITLE,
BODY,
COMMIT_MESSAGE,
False,
)
result = get_env_vars(True)
self.assertEqual(result, expected_result)
Expand All @@ -156,6 +160,7 @@ def test_get_env_vars_with_token_and_repos(self):
"GH_TOKEN": TOKEN,
"ORGANIZATION": ORGANIZATION,
"TITLE": TITLE,
"ISSUE_REPORT": "true",
},
)
def test_get_env_vars_optional_values(self):
Expand All @@ -173,6 +178,7 @@ def test_get_env_vars_optional_values(self):
TITLE,
BODY,
COMMIT_MESSAGE,
True,
)
result = get_env_vars(True)
self.assertEqual(result, expected_result)
Expand Down Expand Up @@ -221,6 +227,7 @@ def test_get_env_vars_with_repos_no_dry_run(self):
"Clean up CODEOWNERS file",
"Consider these updates to the CODEOWNERS file to remove users no longer in this organization.",
"Remove users no longer in this organization from CODEOWNERS file",
False,
)
result = get_env_vars(True)
self.assertEqual(result, expected_result)
Expand Down
81 changes: 81 additions & 0 deletions test_env_get_bool.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
"""Test the get_bool_env_var function"""

import os
import unittest
from unittest.mock import patch

from env import get_bool_env_var


class TestEnv(unittest.TestCase):
"""Test the get_bool_env_var function"""

@patch.dict(
os.environ,
{
"TEST_BOOL": "true",
},
clear=True,
)
def test_get_bool_env_var_that_exists_and_is_true(self):
"""Test that gets a boolean environment variable that exists and is true"""
result = get_bool_env_var("TEST_BOOL", False)
self.assertTrue(result)

@patch.dict(
os.environ,
{
"TEST_BOOL": "false",
},
clear=True,
)
def test_get_bool_env_var_that_exists_and_is_false(self):
"""Test that gets a boolean environment variable that exists and is false"""
result = get_bool_env_var("TEST_BOOL", False)
self.assertFalse(result)

@patch.dict(
os.environ,
{
"TEST_BOOL": "nope",
},
clear=True,
)
def test_get_bool_env_var_that_exists_and_is_false_due_to_invalid_value(self):
"""Test that gets a boolean environment variable that exists and is false
due to an invalid value
"""
result = get_bool_env_var("TEST_BOOL", False)
self.assertFalse(result)

@patch.dict(
os.environ,
{
"TEST_BOOL": "false",
},
clear=True,
)
def test_get_bool_env_var_that_does_not_exist_and_default_value_returns_true(self):
"""Test that gets a boolean environment variable that does not exist
and default value returns: true
"""
result = get_bool_env_var("DOES_NOT_EXIST", True)
self.assertTrue(result)

@patch.dict(
os.environ,
{
"TEST_BOOL": "true",
},
clear=True,
)
def test_get_bool_env_var_that_does_not_exist_and_default_value_returns_false(self):
"""Test that gets a boolean environment variable that does not exist
and default value returns: false
"""
result = get_bool_env_var("DOES_NOT_EXIST", False)
self.assertFalse(result)


if __name__ == "__main__":
unittest.main()
Loading

0 comments on commit c8770cb

Please sign in to comment.