diff --git a/README.md b/README.md index 00733a2..eaab0e2 100644 --- a/README.md +++ b/README.md @@ -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 @@ -114,6 +115,14 @@ jobs: GH_TOKEN: ${{ secrets.GH_TOKEN }} ORGANIZATION: 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: + token: ${{ secrets.GITHUB_TOKEN }} ``` diff --git a/cleanowners.py b/cleanowners.py index 5fafc58..d7410a5 100644 --- a/cleanowners.py +++ b/cleanowners.py @@ -5,6 +5,7 @@ import auth import env import github3 +from markdown_writer import write_to_markdown def get_org(github_connection, organization): @@ -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 @@ -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: @@ -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) @@ -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 @@ -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, @@ -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""" @@ -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(): @@ -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 diff --git a/env.py b/env.py index 6f36d98..cc6d6b9 100644 --- a/env.py +++ b/env.py @@ -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. @@ -41,6 +57,7 @@ def get_env_vars( str, str, str, + bool, ]: """ Get the environment variables for use in the action. @@ -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: @@ -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 @@ -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, @@ -164,8 +177,9 @@ def get_env_vars( token, ghe, exempt_repositories_list, - dry_run_bool, + dry_run, title, body, commit_message, + issue_report, ) diff --git a/markdown_writer.py b/markdown_writer.py new file mode 100644 index 0000000..89f0be8 --- /dev/null +++ b/markdown_writer.py @@ -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") diff --git a/test_cleanowners.py b/test_cleanowners.py index b4cb86a..b846893 100644 --- a/test_cleanowners.py +++ b/test_cleanowners.py @@ -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 = """ @@ -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""" diff --git a/test_env.py b/test_env.py index 264ea36..3d06b98 100644 --- a/test_env.py +++ b/test_env.py @@ -30,6 +30,7 @@ def setUp(self): "ORGANIZATION", "REPOSITORY", "TITLE", + "ISSUE_REPORT", ] for key in env_keys: if key in os.environ: @@ -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) @@ -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) @@ -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) @@ -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): @@ -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) @@ -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) diff --git a/test_env_get_bool.py b/test_env_get_bool.py new file mode 100644 index 0000000..3165de1 --- /dev/null +++ b/test_env_get_bool.py @@ -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() diff --git a/test_markdown_writer.py b/test_markdown_writer.py new file mode 100644 index 0000000..48b9d3e --- /dev/null +++ b/test_markdown_writer.py @@ -0,0 +1,68 @@ +""" Unit tests for the write_to_markdown function in markdown_writer.py """ + +import unittest +from unittest.mock import call, mock_open, patch + +from markdown_writer import write_to_markdown + + +class TestWriteToMarkdown(unittest.TestCase): + """Test the write_to_markdown function""" + + def test_write_with_all_counts_and_no_users_to_remove(self): + """Test that the function writes the correct markdown when there are no users to remove""" + mock_file = mock_open() + with patch("builtins.open", mock_file): + write_to_markdown(0, 0, 2, 3, {}) + mock_file().write.assert_called_once_with( + "# Cleanowners Report\n\n" + "## Overall Stats\n" + "0 Users to Remove\n" + "0 Pull Requests created\n" + "2 Repositories with no CODEOWNERS file\n" + "3 Repositories with CODEOWNERS file\n" + ) + + def test_write_with_repos_and_users_with_users_to_remove(self): + """Test that the function writes the correct markdown when there are users to remove""" + mock_file = mock_open() + repo_users = {"repo1": ["user1", "user2"], "repo2": ["user3"]} + with patch("builtins.open", mock_file): + write_to_markdown(1, 2, 3, 4, repo_users) + calls = [ + call( + "# Cleanowners Report\n\n" + "## Overall Stats\n" + "1 Users to Remove\n" + "2 Pull Requests created\n" + "3 Repositories with no CODEOWNERS file\n" + "4 Repositories with CODEOWNERS file\n" + ), + call("## Repositories and Users to Remove\n"), + call("repo1\n"), + call("- user1\n"), + call("- user2\n"), + call("\n"), + call("repo2\n"), + call("- user3\n"), + call("\n"), + ] + mock_file().write.assert_has_calls(calls, any_order=False) + + def test_write_with_empty_inputs(self): + """Test that the function writes the correct markdown when all inputs are 0""" + mock_file = mock_open() + with patch("builtins.open", mock_file): + write_to_markdown(0, 0, 0, 0, {}) + mock_file().write.assert_called_once_with( + "# Cleanowners Report\n\n" + "## Overall Stats\n" + "0 Users to Remove\n" + "0 Pull Requests created\n" + "0 Repositories with no CODEOWNERS file\n" + "0 Repositories with CODEOWNERS file\n" + ) + + +if __name__ == "__main__": + unittest.main()