diff --git a/changelog/+sdb-profile-values.added.md b/changelog/+sdb-profile-values.added.md new file mode 100644 index 00000000..dc87e03f --- /dev/null +++ b/changelog/+sdb-profile-values.added.md @@ -0,0 +1 @@ +Profile configuration values containing SDB URIs (e.g. `token: sdb://osenv/GITHUB_TOKEN`) are now rendered on access via `config.get`. Salt itself only renders them inside the minion/master configuration during startup, so this makes them work in pillar-sourced profiles as well. diff --git a/changelog/+tracebacks.changed.md b/changelog/+tracebacks.changed.md new file mode 100644 index 00000000..ac16a54c --- /dev/null +++ b/changelog/+tracebacks.changed.md @@ -0,0 +1 @@ +Exception tracebacks are only logged at debug loglevel now diff --git a/changelog/+utils-refactor.changed.md b/changelog/+utils-refactor.changed.md new file mode 100644 index 00000000..b137c8c5 --- /dev/null +++ b/changelog/+utils-refactor.changed.md @@ -0,0 +1 @@ +Refactored the GitHub API interaction layer into reusable per-entity modules below `saltext.github.utils.github`. These raise `GitHubApiError`/`GitHubNotFoundError` (subclasses of `CommandExecutionError`) instead of returning error indicators. Existing execution module functions behave as before, except that unhandled API errors now surface as the above exceptions instead of raw PyGithub ones. diff --git a/docs/ref/utils/index.rst b/docs/ref/utils/index.rst index ec1e0af1..9d4d05ad 100644 --- a/docs/ref/utils/index.rst +++ b/docs/ref/utils/index.rst @@ -11,3 +11,12 @@ _________ utils utils.github + utils.github.client + utils.github.exceptions + utils.github.issues + utils.github.milestones + utils.github.pulls + utils.github.repos + utils.github.rulesets + utils.github.teams + utils.github.users diff --git a/docs/ref/utils/saltext.github.utils.github.client.rst b/docs/ref/utils/saltext.github.utils.github.client.rst new file mode 100644 index 00000000..72202b64 --- /dev/null +++ b/docs/ref/utils/saltext.github.utils.github.client.rst @@ -0,0 +1,5 @@ +saltext.github.utils.github.client +================================== + +.. automodule:: saltext.github.utils.github.client + :members: diff --git a/docs/ref/utils/saltext.github.utils.github.exceptions.rst b/docs/ref/utils/saltext.github.utils.github.exceptions.rst new file mode 100644 index 00000000..a64a161f --- /dev/null +++ b/docs/ref/utils/saltext.github.utils.github.exceptions.rst @@ -0,0 +1,5 @@ +saltext.github.utils.github.exceptions +====================================== + +.. automodule:: saltext.github.utils.github.exceptions + :members: diff --git a/docs/ref/utils/saltext.github.utils.github.issues.rst b/docs/ref/utils/saltext.github.utils.github.issues.rst new file mode 100644 index 00000000..ae450b0c --- /dev/null +++ b/docs/ref/utils/saltext.github.utils.github.issues.rst @@ -0,0 +1,5 @@ +saltext.github.utils.github.issues +================================== + +.. automodule:: saltext.github.utils.github.issues + :members: diff --git a/docs/ref/utils/saltext.github.utils.github.milestones.rst b/docs/ref/utils/saltext.github.utils.github.milestones.rst new file mode 100644 index 00000000..b64be801 --- /dev/null +++ b/docs/ref/utils/saltext.github.utils.github.milestones.rst @@ -0,0 +1,5 @@ +saltext.github.utils.github.milestones +====================================== + +.. automodule:: saltext.github.utils.github.milestones + :members: diff --git a/docs/ref/utils/saltext.github.utils.github.pulls.rst b/docs/ref/utils/saltext.github.utils.github.pulls.rst new file mode 100644 index 00000000..d5321249 --- /dev/null +++ b/docs/ref/utils/saltext.github.utils.github.pulls.rst @@ -0,0 +1,5 @@ +saltext.github.utils.github.pulls +================================= + +.. automodule:: saltext.github.utils.github.pulls + :members: diff --git a/docs/ref/utils/saltext.github.utils.github.repos.rst b/docs/ref/utils/saltext.github.utils.github.repos.rst new file mode 100644 index 00000000..b43706df --- /dev/null +++ b/docs/ref/utils/saltext.github.utils.github.repos.rst @@ -0,0 +1,5 @@ +saltext.github.utils.github.repos +================================= + +.. automodule:: saltext.github.utils.github.repos + :members: diff --git a/docs/ref/utils/saltext.github.utils.github.rulesets.rst b/docs/ref/utils/saltext.github.utils.github.rulesets.rst new file mode 100644 index 00000000..68cad6c8 --- /dev/null +++ b/docs/ref/utils/saltext.github.utils.github.rulesets.rst @@ -0,0 +1,5 @@ +saltext.github.utils.github.rulesets +==================================== + +.. automodule:: saltext.github.utils.github.rulesets + :members: diff --git a/docs/ref/utils/saltext.github.utils.github.teams.rst b/docs/ref/utils/saltext.github.utils.github.teams.rst new file mode 100644 index 00000000..dae2586e --- /dev/null +++ b/docs/ref/utils/saltext.github.utils.github.teams.rst @@ -0,0 +1,5 @@ +saltext.github.utils.github.teams +================================= + +.. automodule:: saltext.github.utils.github.teams + :members: diff --git a/docs/ref/utils/saltext.github.utils.github.users.rst b/docs/ref/utils/saltext.github.utils.github.users.rst new file mode 100644 index 00000000..2da335a3 --- /dev/null +++ b/docs/ref/utils/saltext.github.utils.github.users.rst @@ -0,0 +1,5 @@ +saltext.github.utils.github.users +================================= + +.. automodule:: saltext.github.utils.github.users + :members: diff --git a/src/saltext/github/modules/github.py b/src/saltext/github/modules/github.py index 56f7b927..a3927bce 100644 --- a/src/saltext/github/modules/github.py +++ b/src/saltext/github/modules/github.py @@ -15,6 +15,23 @@ Added the ``api_url`` configuration option to override the API endpoint we run against. Can be used for GitHub Enterprise instances. +.. versionadded:: 1.1.0 + + Profile configuration values containing SDB URIs are now rendered on + access. Salt itself only renders them inside the minion/master + configuration during startup, so this makes them work in pillar-sourced + profiles as well: + + .. code-block:: yaml + + # Requires a corresponding SDB profile in the minion configuration, + # e.g. + # osenv: + # driver: env + github: + token: sdb://osenv/GITHUB_TOKEN + org_name: my_organization + For example: .. code-block:: yaml @@ -37,17 +54,23 @@ api_url: https://api.github.com """ -import json import logging -import re from salt.exceptions import CommandExecutionError from saltext.github.utils import filter_unset try: - import github # pylint: disable=import-self - from github.GithubException import UnknownObjectException + from saltext.github.utils.github import client as gh_client + from saltext.github.utils.github import issues as gh_issues + from saltext.github.utils.github import milestones as gh_milestones + from saltext.github.utils.github import pulls as gh_pulls + from saltext.github.utils.github import repos as gh_repos + from saltext.github.utils.github import rulesets as gh_rulesets + from saltext.github.utils.github import teams as gh_teams + from saltext.github.utils.github import users as gh_users + from saltext.github.utils.github.exceptions import GitHubApiError + from saltext.github.utils.github.exceptions import GitHubNotFoundError HAS_LIBS = True except ImportError: # pragma: no cover @@ -84,14 +107,19 @@ def _get_config_value(profile, config_name): The configuration item's name to use to return configuration values. """ if "gh_profile" not in __context__ or profile not in __context__["gh_profile"]: - config = __salt__["config.option"](profile) - if not config: + if not __salt__["config.get"](profile): raise CommandExecutionError( f"Authentication information could not be found for the '{profile}' profile." ) - __context__.setdefault("gh_profile", {})[profile] = config + __context__.setdefault("gh_profile", {})[profile] = {} + + cache = __context__["gh_profile"][profile] + if config_name not in cache: + # Unlike config.option, config.get resolves SDB URIs in scalar values, + # hence request each value separately and cache the rendered result. + cache[config_name] = __salt__["config.get"](f"{profile}:{config_name}", default=None) - config_value = __context__["gh_profile"][profile].get(config_name) + config_value = cache[config_name] if config_value is None: raise CommandExecutionError( f"The '{config_name}' parameter was not found in the '{profile}' profile." @@ -110,42 +138,17 @@ def _get_api_url(profile): return API_URL_DEFAULT -def _get_client(profile) -> github.Github: +def _get_conn(profile): """ - Return the GitHub client, cached into __context__ for performance + Return the GitHub connection, cached into __context__ for performance """ token = _get_config_value(profile, "token") + org_name = _get_config_value(profile, "org_name") api_url = _get_api_url(profile) - key = "github.{}:{}:{}".format( # pylint: disable=consider-using-f-string - token, _get_config_value(profile, "org_name"), api_url - ) + key = f"github.{token}:{org_name}:{api_url}" if key not in __context__: - __context__[key] = github.Github( - auth=github.Auth.Token(token), base_url=api_url, per_page=100 - ) - return __context__[key] - - -def _get_repos(profile, ignore_cache=False): - org_name = _get_config_value(profile, "org_name") - key = f"github.{org_name}:repos" - - if key not in __context__ or ignore_cache: - client = _get_client(profile) - organization = client.get_organization(org_name) - - next_result = [] - - for repo in organization.get_repos(): - next_result.append(repo) - - # Cache a copy of each repo for single lookups - repo_key = f"github.{org_name}:{repo.name.lower()}:repo_info" - __context__[repo_key] = _repo_to_dict(repo) - - __context__[key] = next_result - + __context__[key] = gh_client.GitHubConnection(token, org_name, api_url=api_url) return __context__[key] @@ -166,13 +169,7 @@ def list_users(profile="github", ignore_cache=False): salt myminion github.list_users salt myminion github.list_users profile='my-github-profile' """ - org_name = _get_config_value(profile, "org_name") - key = f"github.{org_name}:users" - if key not in __context__ or ignore_cache: - client = _get_client(profile) - organization = client.get_organization(org_name) - __context__[key] = [member.login for member in organization.get_members()] - return __context__[key] + return gh_users.list_members(_get_conn(profile), ignore_cache=ignore_cache) def get_user(name, profile="github", user_details=False): @@ -203,38 +200,12 @@ def get_user(name, profile="github", user_details=False): # User is in the org, no need for additional Data return True - response = {} - client = _get_client(profile) - organization = client.get_organization(_get_config_value(profile, "org_name")) - try: - user = client.get_user(name) - except UnknownObjectException: - log.exception("Resource not found") + return gh_users.get_user_info(_get_conn(profile), name) + except GitHubNotFoundError: + log.error("Resource not found", exc_info_on_loglevel=logging.DEBUG) return False - response["company"] = user.company - response["created_at"] = user.created_at - response["email"] = user.email - response["html_url"] = user.html_url - response["id"] = user.id - response["login"] = user.login - response["name"] = user.name - response["type"] = user.type - response["url"] = user.url - - try: - membership = user.get_organization_membership(organization) - except UnknownObjectException: - response["membership_state"] = "nonexistent" - response["in_org"] = False - return response - - response["in_org"] = organization.has_in_members(user) - response["membership_state"] = membership.state - - return response - def add_user(name, profile="github"): """ @@ -253,22 +224,12 @@ def add_user(name, profile="github"): salt myminion github.add_user github-handle """ - client = _get_client(profile) - organization = client.get_organization(_get_config_value(profile, "org_name")) - try: - github_named_user = client.get_user(name) - except UnknownObjectException: - log.exception("Resource not found") + return gh_users.invite_member(_get_conn(profile), name) + except GitHubNotFoundError: + log.error("Resource not found", exc_info_on_loglevel=logging.DEBUG) return False - # There is no public method that also returns the resulting membership state - _, data = client.requester.requestJsonAndCheck( - "PUT", organization.url + "/memberships/" + github_named_user.login - ) - - return data.get("state") == "pending" - def remove_user(name, profile="github"): """ @@ -287,20 +248,12 @@ def remove_user(name, profile="github"): salt myminion github.remove_user github-handle """ - client = _get_client(profile) - organization = client.get_organization(_get_config_value(profile, "org_name")) - try: - git_user = client.get_user(name) - except UnknownObjectException: - log.exception("Resource not found") + return gh_users.remove_member(_get_conn(profile), name) + except GitHubNotFoundError: + log.error("Resource not found", exc_info_on_loglevel=logging.DEBUG) return False - if organization.has_in_members(git_user): - organization.remove_from_members(git_user) - - return not organization.has_in_members(git_user) - def get_issue(issue_number, repo_name=None, profile="github", output="min"): """ @@ -329,23 +282,22 @@ def get_issue(issue_number, repo_name=None, profile="github", output="min"): salt myminion github.get_issue 514 salt myminion github.get_issue 514 repo_name=salt """ - org_name = _get_config_value(profile, "org_name") if repo_name is None: repo_name = _get_config_value(profile, "repo_name") - action = "/".join(["repos", org_name, repo_name]) - command = "issues/" + str(issue_number) - ret = {} - issue_data = _query(profile, action=action, command=command) + try: + issue_data = gh_issues.get(_get_conn(profile), repo_name, issue_number) + except GitHubApiError: + return ret - issue_id = issue_data.get("dict", {}).get("id") + issue_id = (issue_data or {}).get("id") if issue_id: if output.lower() == "full": - ret[issue_id] = issue_data["dict"] + ret[issue_id] = issue_data else: - ret[issue_id] = _format_issue(issue_data["dict"]) + ret[issue_id] = gh_issues.format_issue(issue_data) return ret @@ -381,23 +333,13 @@ def get_issue_comments(issue_number, repo_name=None, profile="github", since=Non salt myminion github.get_issue_comments 514 salt myminion github.get_issue 514 repo_name=salt """ - org_name = _get_config_value(profile, "org_name") if repo_name is None: repo_name = _get_config_value(profile, "repo_name") - action = "/".join(["repos", org_name, repo_name]) - command = "/".join(["issues", str(issue_number), "comments"]) - - args = {} - if since: - args["since"] = since - - comments = _query(profile, action=action, command=command, args=args) - if comments.get("error"): - raise CommandExecutionError(comments["error"]) + comments = gh_issues.get_comments(_get_conn(profile), repo_name, issue_number, since=since) ret = {} - for comment in comments.get("dict", {}): + for comment in comments or []: comment_id = comment.get("id") if output == "full": ret[comment_id] = comment @@ -494,52 +436,31 @@ def get_issues( salt myminion github.get_issues my-github-repo """ - org_name = _get_config_value(profile, "org_name") if repo_name is None: repo_name = _get_config_value(profile, "repo_name") - action = "/".join(["repos", org_name, repo_name]) - args = {} - - # Build API arguments, as necessary. - if milestone: - args["milestone"] = milestone - if assignee: - args["assignee"] = assignee - if creator: - args["creator"] = creator - if mentioned: - args["mentioned"] = mentioned - if labels: - args["labels"] = labels - if since: - args["since"] = since - if per_page: - args["per_page"] = per_page - - # Only pass the following API args if they're not the defaults listed. - if state and state != "open": - args["state"] = state - if sort and sort != "created": - args["sort"] = sort - if direction and direction != "desc": - args["direction"] = direction + issues = gh_issues.list_all( + _get_conn(profile), + repo_name, + milestone=milestone, + state=state, + assignee=assignee, + creator=creator, + mentioned=mentioned, + labels=labels, + sort=sort, + direction=direction, + since=since, + per_page=per_page, + ) ret = {} - issues = _query(profile, action=action, command="issues", args=args) - if issues.get("error"): - raise CommandExecutionError(issues["error"]) - - for issue in issues.get("dict", {}): - # Pull requests are included in the issue list from GitHub - # Let's not include those in the return. - if issue.get("pull_request"): - continue + for issue in issues or []: issue_id = issue.get("id") if output == "full": ret[issue_id] = issue else: - ret[issue_id] = _format_issue(issue) + ret[issue_id] = gh_issues.format_issue(issue) return ret @@ -591,38 +512,25 @@ def get_milestones( salt myminion github.get_milestones """ - org_name = _get_config_value(profile, "org_name") if repo_name is None: repo_name = _get_config_value(profile, "repo_name") - action = "/".join(["repos", org_name, repo_name]) - args = {} - - if per_page: - args["per_page"] = per_page - - # Only pass the following API args if they're not the defaults listed. - if state and state != "open": - args["state"] = state - if sort and sort != "due_on": - args["sort"] = sort - if direction and direction != "asc": - args["direction"] = direction + milestones = gh_milestones.list_all( + _get_conn(profile), + repo_name, + state=state, + sort=sort, + direction=direction, + per_page=per_page, + ) ret = {} - milestones = _query(profile, action=action, command="milestones", args=args) - if milestones.get("error"): - raise CommandExecutionError(milestones["error"]) - - for milestone in milestones.get("dict", {}): + for milestone in milestones or []: milestone_id = milestone.get("id") if output == "full": ret[milestone_id] = milestone else: - milestone.pop("creator", None) - milestone.pop("html_url", None) - milestone.pop("labels_url", None) - ret[milestone_id] = milestone + ret[milestone_id] = gh_milestones.format_milestone(milestone) return ret @@ -660,62 +568,31 @@ def get_milestone(number=None, name=None, repo_name=None, profile="github", outp """ ret = {} - if not any([number, name]): + if not (number or name): raise CommandExecutionError("Either a milestone 'name' or 'number' must be provided.") - org_name = _get_config_value(profile, "org_name") if repo_name is None: repo_name = _get_config_value(profile, "repo_name") - action = "/".join(["repos", org_name, repo_name]) if number: - command = "milestones/" + str(number) - milestone_data = _query(profile, action=action, command=command) - milestone_data = milestone_data.get("dict", {}) - milestone_id = milestone_data.get("id") + try: + milestone_data = gh_milestones.get(_get_conn(profile), repo_name, number) + except GitHubApiError: + return ret + milestone_id = (milestone_data or {}).get("id") if milestone_id: if output.lower() == "full": ret[milestone_id] = milestone_data else: - milestone_data.pop("creator", None) - milestone_data.pop("html_url", None) - milestone_data.pop("labels_url", None) - ret[milestone_id] = milestone_data - return ret - - else: - milestones = get_milestones(repo_name=repo_name, profile=profile, output=output) - for key, val in milestones.items(): - if val.get("title") == name: - ret[key] = val - return ret - - return ret + ret[milestone_id] = gh_milestones.format_milestone(milestone_data) + return ret + milestones = get_milestones(repo_name=repo_name, profile=profile, output=output) + for key, val in milestones.items(): + if val.get("title") == name: + ret[key] = val + return ret -def _repo_to_dict(repo): - ret = {} - ret["id"] = repo.id - ret["name"] = repo.name - ret["full_name"] = repo.full_name - ret["owner"] = repo.owner.login - ret["private"] = repo.private - ret["html_url"] = repo.html_url - ret["description"] = repo.description - ret["fork"] = repo.fork - ret["homepage"] = repo.homepage - ret["size"] = repo.size - ret["stargazers_count"] = repo.stargazers_count - ret["watchers_count"] = repo.watchers_count - ret["language"] = repo.language - ret["open_issues_count"] = repo.open_issues_count - ret["forks"] = repo.forks - ret["open_issues"] = repo.open_issues - ret["watchers"] = repo.watchers - ret["default_branch"] = repo.default_branch - ret["has_issues"] = repo.has_issues - ret["has_wiki"] = repo.has_wiki - ret["has_downloads"] = repo.has_downloads return ret @@ -736,27 +613,7 @@ def get_repo_info(repo_name, profile="github", ignore_cache=False): salt myminion github.get_repo_info salt salt myminion github.get_repo_info salt profile='my-github-profile' """ - - org_name = _get_config_value(profile, "org_name") - key = "github.{}:{}:repo_info".format( # pylint: disable=consider-using-f-string - _get_config_value(profile, "org_name"), repo_name.lower() - ) - - if key not in __context__ or ignore_cache: - client = _get_client(profile) - try: - # With lazy fetching, client.get_repo returns a Repository object - # even if the repo does not exist - the exception is only raised - # once attributes of the object are accessed. - repo = client.get_repo("/".join([org_name, repo_name])) - ret = _repo_to_dict(repo) - - __context__[key] = ret - except github.UnknownObjectException as exc: - raise CommandExecutionError( - f"The '{repo_name}' repository under the '{org_name}' organization could not be found." - ) from exc - return __context__[key] + return gh_repos.get_info(_get_conn(profile), repo_name, ignore_cache=ignore_cache) def get_repo_teams(repo_name, profile="github"): @@ -776,25 +633,7 @@ def get_repo_teams(repo_name, profile="github"): salt myminion github.get_repo_teams salt salt myminion github.get_repo_teams salt profile='my-github-profile' """ - ret = [] - org_name = _get_config_value(profile, "org_name") - client = _get_client(profile) - - try: - repo = client.get_repo("/".join([org_name, repo_name])) - except github.UnknownObjectException as exc: - raise CommandExecutionError( - f"The '{repo_name}' repository under the '{org_name}' organization could not be found." - ) from exc - try: - teams = repo.get_teams() - for team in teams: - ret.append({"id": team.id, "name": team.name, "permission": team.permission}) - except github.UnknownObjectException as exc: # pragma: no cover - raise CommandExecutionError( - f"Unable to retrieve teams for repository '{repo_name}' under the '{org_name}' organization." - ) from exc - return ret + return gh_repos.get_teams(_get_conn(profile), repo_name) def list_repos(profile="github"): @@ -813,7 +652,7 @@ def list_repos(profile="github"): salt myminion github.list_repos salt myminion github.list_repos profile='my-github-profile' """ - return [repo.name for repo in _get_repos(profile)] + return [repo.name for repo in gh_repos.list_all(_get_conn(profile))] def list_private_repos(profile="github"): @@ -831,11 +670,7 @@ def list_private_repos(profile="github"): salt myminion github.list_private_repos salt myminion github.list_private_repos profile='my-github-profile' """ - repos = [] - for repo in _get_repos(profile): - if repo.private is True: - repos.append(repo.name) - return repos + return [repo.name for repo in gh_repos.list_all(_get_conn(profile)) if repo.private is True] def list_public_repos(profile="github"): @@ -852,11 +687,7 @@ def list_public_repos(profile="github"): salt myminion github.list_public_repos salt myminion github.list_public_repos profile='my-github-profile' """ - repos = [] - for repo in _get_repos(profile): - if repo.private is False: - repos.append(repo.name) - return repos + return [repo.name for repo in gh_repos.list_all(_get_conn(profile)) if repo.private is False] def add_repo( @@ -916,25 +747,22 @@ def add_repo( salt myminion github.add_repo 'repo_name' """ try: - client = _get_client(profile) - organization = client.get_organization(_get_config_value(profile, "org_name")) - parameters = filter_unset( - { - "description": description, - "homepage": homepage, - "private": private, - "has_issues": has_issues, - "has_wiki": has_wiki, - "has_downloads": has_downloads, - "auto_init": auto_init, - "gitignore_template": gitignore_template, - "license_template": license_template, - } + gh_repos.create( + _get_conn(profile), + name, + description=description, + homepage=homepage, + private=private, + has_issues=has_issues, + has_wiki=has_wiki, + has_downloads=has_downloads, + auto_init=auto_init, + gitignore_template=gitignore_template, + license_template=license_template, ) - organization.create_repo(name, **parameters) return True - except github.GithubException: - log.exception("Error creating a repo") + except GitHubApiError: + log.error("Error creating a repo", exc_info_on_loglevel=logging.DEBUG) return False @@ -984,34 +812,25 @@ def edit_repo( """ try: - allow_private_change = _get_config_value(profile, "allow_repo_privacy_changes") + allow_privacy_change = _get_config_value(profile, "allow_repo_privacy_changes") except CommandExecutionError: - allow_private_change = False - - if private is not None and not allow_private_change: - raise CommandExecutionError( - f"The private field is set to be changed for repo {name} but allow_repo_privacy_changes disallows this." - ) + allow_privacy_change = False try: - client = _get_client(profile) - organization = client.get_organization(_get_config_value(profile, "org_name")) - repo = organization.get_repo(name) - parameters = filter_unset( - { - "description": description, - "homepage": homepage, - "private": private, - "has_issues": has_issues, - "has_wiki": has_wiki, - "has_downloads": has_downloads, - } + gh_repos.update( + _get_conn(profile), + name, + description=description, + homepage=homepage, + private=private, + has_issues=has_issues, + has_wiki=has_wiki, + has_downloads=has_downloads, + allow_privacy_change=allow_privacy_change, ) - repo.edit(name, **parameters) - get_repo_info(name, profile=profile, ignore_cache=True) # Refresh cache return True - except github.GithubException: - log.exception("Error editing a repo") + except GitHubApiError: + log.error("Error editing a repo", exc_info_on_loglevel=logging.DEBUG) return False @@ -1031,20 +850,17 @@ def remove_repo(name, profile="github"): salt myminion github.remove_repo 'my-repo' """ + conn = _get_conn(profile) try: - get_repo_info(name, profile=profile) + gh_repos.get_info(conn, name) except CommandExecutionError: log.error("Repo %s to be removed does not exist.", name) return False try: - client = _get_client(profile) - organization = client.get_organization(_get_config_value(profile, "org_name")) - repo = organization.get_repo(name) - repo.delete() - _get_repos(profile=profile, ignore_cache=True) # refresh cache + gh_repos.delete(conn, name) return True - except github.GithubException: - log.exception("Error deleting a repo") + except GitHubApiError: + log.error("Error deleting a repo", exc_info_on_loglevel=logging.DEBUG) return False @@ -1065,7 +881,7 @@ def get_team(name, profile="github"): salt myminion github.get_team 'team_name' """ - return list_teams(profile).get(name) + return gh_teams.get(_get_conn(profile), name) def add_team( @@ -1105,26 +921,17 @@ def add_team( salt myminion github.add_team 'team_name' """ try: - client = _get_client(profile) - organization = client.get_organization(_get_config_value(profile, "org_name")) - parameters = filter_unset( - { - "description": description, - "permission": permission, - "privacy": privacy, - } + gh_teams.create( + _get_conn(profile), + name, + description=description, + repo_names=repo_names, + privacy=privacy, + permission=permission, ) - if repo_names is not None: - # create_team expects Repository objects, resolve the (full) names - parameters["repo_names"] = [ - client.get_repo(repo) if "/" in repo else organization.get_repo(repo) - for repo in repo_names - ] - organization.create_team(name, **parameters) - list_teams(ignore_cache=True, profile=profile) # Refresh cache return True - except github.GithubException: - log.exception("Error creating a team") + except GitHubApiError: + log.error("Error creating a team", exc_info_on_loglevel=logging.DEBUG) return False @@ -1154,27 +961,23 @@ def edit_team(name, description=None, privacy=None, permission=None, profile="gi salt myminion github.edit_team 'team_name' description='Team description' """ - team = get_team(name, profile=profile) + conn = _get_conn(profile) + team = gh_teams.get(conn, name) if not team: log.error("Team %s does not exist", name) return False try: - client = _get_client(profile) - organization = client.get_organization(_get_config_value(profile, "org_name")) - team = organization.get_team_by_slug(team["slug"]) - team.edit( + gh_teams.update( + conn, + team, name, - **filter_unset( - { - "description": description, - "privacy": privacy, - "permission": permission, - } - ), + description=description, + privacy=privacy, + permission=permission, ) return True - except UnknownObjectException: - log.exception("Resource not found") + except GitHubNotFoundError: + log.error("Resource not found", exc_info_on_loglevel=logging.DEBUG) return False @@ -1194,18 +997,16 @@ def remove_team(name, profile="github"): salt myminion github.remove_team 'team_name' """ - team_info = get_team(name, profile=profile) + conn = _get_conn(profile) + team_info = gh_teams.get(conn, name) if not team_info: log.error("Team %s to be removed does not exist.", name) return False try: - client = _get_client(profile) - organization = client.get_organization(_get_config_value(profile, "org_name")) - team = organization.get_team_by_slug(team_info["slug"]) - team.delete() - return list_teams(ignore_cache=True, profile=profile).get(name) is None - except github.GithubException: - log.exception("Error deleting a team") + gh_teams.delete(conn, team_info) + return gh_teams.get(conn, name) is None + except GitHubApiError: + log.error("Error deleting a team", exc_info_on_loglevel=logging.DEBUG) return False @@ -1229,36 +1030,16 @@ def list_team_repos(team_name, profile="github", ignore_cache=False): salt myminion github.list_team_repos 'team_name' """ - cached_team = get_team(team_name, profile=profile) + conn = _get_conn(profile) + cached_team = gh_teams.get(conn, team_name) if not cached_team: log.error("Team %s does not exist.", team_name) return False - # Return from cache if available - if cached_team.get("repos") and not ignore_cache: - return cached_team["repos"] - - try: - client = _get_client(profile) - organization = client.get_organization(_get_config_value(profile, "org_name")) - team = organization.get_team_by_slug(cached_team["slug"]) - except UnknownObjectException: - log.exception("Resource not found: %s", cached_team["id"]) - return {} try: - repos = {} - for repo in team.get_repos(): - permission = "pull" - if repo.permissions.admin: - permission = "admin" - elif repo.permissions.push: - permission = "push" - - repos[repo.name.lower()] = {"permission": permission} - cached_team["repos"] = repos - return repos - except UnknownObjectException: # pragma: no cover - log.exception("Resource not found: %s", cached_team["id"]) + return gh_teams.list_repos(conn, cached_team, ignore_cache=ignore_cache) + except GitHubNotFoundError: + log.error("Resource not found: %s", cached_team["id"], exc_info_on_loglevel=logging.DEBUG) return {} @@ -1286,27 +1067,23 @@ def add_team_repo(repo_name, team_name, profile="github", permission=None): salt myminion github.add_team_repo 'my_repo' 'team_name' """ - team_info = get_team(team_name, profile=profile) + conn = _get_conn(profile) + team_info = gh_teams.get(conn, team_name) if not team_info: log.error("Team %s does not exist", team_name) return False try: - client = _get_client(profile) - organization = client.get_organization(_get_config_value(profile, "org_name")) - team = organization.get_team_by_slug(team_info["slug"]) - repo = organization.get_repo(repo_name) - except UnknownObjectException: - log.exception("Resource not found: %s", team_info["id"]) - return False - if permission is None: - team.add_to_repos(repo) - elif not team.update_team_repository(repo, permission): - log.error( - "Failed setting permission %s on repo %s for team %s", permission, repo_name, team_name - ) + if not gh_teams.add_repo(conn, team_info, repo_name, permission=permission): + log.error( + "Failed setting permission %s on repo %s for team %s", + permission, + repo_name, + team_name, + ) + return False + except GitHubNotFoundError: + log.error("Resource not found: %s", team_info["id"], exc_info_on_loglevel=logging.DEBUG) return False - # Try to refresh cache - list_team_repos(team_name, profile=profile, ignore_cache=True) return True @@ -1329,20 +1106,16 @@ def remove_team_repo(repo_name, team_name, profile="github"): salt myminion github.remove_team_repo 'my_repo' 'team_name' """ - team_info = get_team(team_name, profile=profile) + conn = _get_conn(profile) + team_info = gh_teams.get(conn, team_name) if not team_info: log.error("Team %s does not exist", team_name) return False try: - client = _get_client(profile) - organization = client.get_organization(_get_config_value(profile, "org_name")) - team = organization.get_team_by_slug(team_info["slug"]) - repo = organization.get_repo(repo_name) - except UnknownObjectException: - log.exception("Resource not found: %s", team_info["id"]) + return gh_teams.remove_repo(conn, team_info, repo_name) + except GitHubNotFoundError: + log.error("Resource not found: %s", team_info["id"], exc_info_on_loglevel=logging.DEBUG) return False - team.remove_from_repos(repo) - return repo_name not in list_team_repos(team_name, profile=profile, ignore_cache=True) def list_team_members(team_name, profile="github", ignore_cache=False): @@ -1364,26 +1137,16 @@ def list_team_members(team_name, profile="github", ignore_cache=False): salt myminion github.list_team_members 'team_name' """ - cached_team = get_team(team_name, profile=profile) + conn = _get_conn(profile) + cached_team = gh_teams.get(conn, team_name) if not cached_team: log.error("Team %s does not exist.", team_name) return False - # Return from cache if available - if cached_team.get("members") and not ignore_cache: - return cached_team["members"] try: - client = _get_client(profile) - organization = client.get_organization(_get_config_value(profile, "org_name")) - team = organization.get_team_by_slug(cached_team["slug"]) - except UnknownObjectException: - log.exception("Resource not found: %s", cached_team["id"]) - return [] - try: - cached_team["members"] = [member.login.lower() for member in team.get_members()] - return cached_team["members"] - except UnknownObjectException: # pragma: no cover - log.exception("Resource not found: %s", cached_team["id"]) + return gh_teams.list_members(conn, cached_team, ignore_cache=ignore_cache) + except GitHubNotFoundError: + log.error("Resource not found: %s", cached_team["id"], exc_info_on_loglevel=logging.DEBUG) return [] @@ -1403,18 +1166,7 @@ def list_members_without_mfa(profile="github", ignore_cache=False): salt myminion github.list_members_without_mfa """ - key = "github.{}:non_mfa_users".format( # pylint: disable=consider-using-f-string - _get_config_value(profile, "org_name") - ) - - if key not in __context__ or ignore_cache: - client = _get_client(profile) - organization = client.get_organization(_get_config_value(profile, "org_name")) - - __context__[key] = [ - m.login.lower() for m in organization.get_members(filter_="2fa_disabled") - ] - return __context__[key] + return gh_users.list_members_without_mfa(_get_conn(profile), ignore_cache=ignore_cache) def is_team_member(name, team_name, profile="github"): @@ -1460,23 +1212,18 @@ def add_team_member(name, team_name, profile="github"): salt myminion github.add_team_member 'user_name' 'team_name' """ - team_info = get_team(team_name, profile=profile) + conn = _get_conn(profile) + team_info = gh_teams.get(conn, team_name) if not team_info: log.error("Team %s does not exist", team_name) return False try: - client = _get_client(profile) - organization = client.get_organization(_get_config_value(profile, "org_name")) - team = organization.get_team_by_slug(team_info["slug"]) - member = client.get_user(name) - except UnknownObjectException: - log.exception("Resource not found: %s", team_info["id"]) + gh_teams.add_member(conn, team_info, name) + except GitHubNotFoundError: + log.error("Resource not found: %s", team_info["id"], exc_info_on_loglevel=logging.DEBUG) return False - - try: - team.add_membership(member, "member") - except github.GithubException: - log.exception("Error in adding a member to a team") + except GitHubApiError: + log.error("Error in adding a member to a team", exc_info_on_loglevel=logging.DEBUG) return False return True @@ -1500,28 +1247,17 @@ def remove_team_member(name, team_name, profile="github"): salt myminion github.remove_team_member 'user_name' 'team_name' """ - team_info = get_team(team_name, profile=profile) + conn = _get_conn(profile) + team_info = gh_teams.get(conn, team_name) if not team_info: log.error("Team %s does not exist", team_name) return False try: - client = _get_client(profile) - organization = client.get_organization(_get_config_value(profile, "org_name")) - team = organization.get_team_by_slug(team_info["slug"]) - member = client.get_user(name) - - except UnknownObjectException: - log.exception("Resource not found: %s", team_info["id"]) + return gh_teams.remove_member(conn, team_info, name) + except GitHubNotFoundError: + log.error("Resource not found: %s", team_info["id"], exc_info_on_loglevel=logging.DEBUG) return False - team.remove_membership(member) - try: - # There is no non-deprecated endpoint for checking plain membership - team.get_team_membership(member) - except UnknownObjectException: - return True - return False - def list_teams(profile="github", ignore_cache=False): """ @@ -1539,29 +1275,7 @@ def list_teams(profile="github", ignore_cache=False): salt myminion github.list_teams """ - key = "github.{}:teams".format( # pylint: disable=consider-using-f-string - _get_config_value(profile, "org_name") - ) - - if key not in __context__ or ignore_cache: - client = _get_client(profile) - organization = client.get_organization(_get_config_value(profile, "org_name")) - teams_data = organization.get_teams() - teams = {} - for team in teams_data: - # These attributes are all present in the list response. Take care - # not to access any lazily completable attributes missing from it - # here since each one would trigger a separate request per team. - teams[team.name] = { - "id": team.id, - "slug": team.slug, - "description": team.description, - "permission": team.permission, - "privacy": team.privacy, - } - __context__[key] = teams - - return __context__[key] + return gh_teams.list_all(_get_conn(profile), ignore_cache=ignore_cache) def get_prs( @@ -1627,192 +1341,32 @@ def get_prs( salt myminion github.get_prs salt myminion github.get_prs base=2016.11 """ - org_name = _get_config_value(profile, "org_name") if repo_name is None: repo_name = _get_config_value(profile, "repo_name") - action = "/".join(["repos", org_name, repo_name]) - args = {} - - # Build API arguments, as necessary. - if head: - args["head"] = head - if base: - args["base"] = base - if per_page: - args["per_page"] = per_page - - # Only pass the following API args if they're not the defaults listed. - if state and state != "open": - args["state"] = state - if sort and sort != "created": - args["sort"] = sort - if direction and direction != "desc": - args["direction"] = direction + prs = gh_pulls.list_all( + _get_conn(profile), + repo_name, + state=state, + head=head, + base=base, + sort=sort, + direction=direction, + per_page=per_page, + ) ret = {} - prs = _query(profile, action=action, command="pulls", args=args) - if prs.get("error"): - raise CommandExecutionError(prs["error"]) - - for pr_ in prs.get("dict", {}): + for pr_ in prs or []: pr_id = pr_.get("id") if output == "full": ret[pr_id] = pr_ else: - ret[pr_id] = _format_pr(pr_) - - return ret - - -def _format_pr(pr_): - """ - Helper function to format API return information into a more manageable - and useful dictionary for pull request information. - - pr_ - The pull request to format. - """ - ret = { - "id": pr_.get("id"), - "pr_number": pr_.get("number"), - "state": pr_.get("state"), - "title": pr_.get("title"), - "user": pr_.get("user").get("login"), - "html_url": pr_.get("html_url"), - "base_branch": pr_.get("base").get("ref"), - } - - return ret - - -def _format_issue(issue): - """ - Helper function to format API return information into a more manageable - and useful dictionary for issue information. - - issue - The issue to format. - """ - ret = { - "id": issue.get("id"), - "issue_number": issue.get("number"), - "state": issue.get("state"), - "title": issue.get("title"), - "user": issue.get("user").get("login"), - "html_url": issue.get("html_url"), - } - - assignee = issue.get("assignee") - if assignee: - assignee = assignee.get("login") - - labels = issue.get("labels") - label_names = [] - for label in labels: - label_names.append(label.get("name")) - - milestone = issue.get("milestone") - if milestone: - milestone = milestone.get("title") - - ret["assignee"] = assignee - ret["labels"] = label_names - ret["milestone"] = milestone + ret[pr_id] = gh_pulls.format_pr(pr_) return ret -def _next_page_number(headers): - """ - Extract the next page number from a GitHub ``Link`` response header. - - Returns ``None`` when there is no further page. - """ - link = None - for key, val in (headers or {}).items(): - if key.lower() == "link": - link = val - break - if not link: - return None - match = re.search(r'<[^>]*[?&]page=(\d+)[^>]*>\s*;\s*rel="next"', link) - if match: - return match.group(1) - return None - - -def _query( - profile, - action=None, - command=None, - args=None, - method="GET", - header_dict=None, - data=None, - per_page=None, -): - """ - Make a web call to the GitHub API and deal with paginated results. - - Requests are dispatched through the PyGithub client (honoring the - profile's ``api_url``, authentication and retry handling). - """ - if not isinstance(args, dict): - args = {} - - path_segments = (([action] if isinstance(action, str) else action) or []) + ( - ([command] if isinstance(command, str) else command) or [] - ) - url = "/" + "/".join(str(segment) for segment in path_segments) - - log.debug("GitHub API path: %s", url) - - if per_page and "per_page" not in args: - args["per_page"] = per_page - - if isinstance(data, str): - data = json.loads(data) - - requester = _get_client(profile).requester - - complete_result = [] - next_page = None - while True: - if next_page: - args["page"] = next_page - try: - status, headers, text = requester.requestJson( - method, url, parameters=args or None, headers=header_dict, input=data - ) - except github.GithubException as err: - return {"status": err.status, "error": str(err)} - except Exception as err: # pylint: disable=broad-except - raise CommandExecutionError(f"GitHub API request failed: {err}") from err - log.debug("GitHub Response Status Code: %s", status) - decoded = json.loads(text) if text else None - - if status not in (200, 201, 204): - message = decoded.get("message", text) if isinstance(decoded, dict) else text - return {"status": status, "error": f"{status} {message}".strip(), "dict": decoded} - - if not isinstance(decoded, list): - # If only querying for one item, such as a single issue - # The GitHub API returns a single dictionary, instead of - # A list of dictionaries. In that case, we can return. - return {"status": status, "headers": headers, "dict": decoded} - - complete_result += decoded - ret = {"status": status, "headers": headers, "dict": complete_result} - - next_page = _next_page_number(headers) - if next_page is None: - break - - return ret - - -def _check_ruleset_params( +def _resolve_ruleset_params( profile, require_id=False, require_params=False, @@ -1827,7 +1381,8 @@ def _check_ruleset_params( **_, ): """ - Helper function to check for param values on command line and in config file + Helper function that merges CLI parameters with profile configuration + defaults for ruleset operations. profile The name of the profile configuration to use. @@ -1837,8 +1392,6 @@ def _check_ruleset_params( require_params Require ruleset_params payload. Defaults to false. - If true, just requires that ruleset_params is set. - You can also pass a set of named params to require in the payload. """ ruleset_type = ruleset_type or _get_config_value(profile, "ruleset_type") if ruleset_type not in ("repo", "org"): @@ -1847,10 +1400,8 @@ def _check_ruleset_params( if ruleset_type == "repo": owner = owner or _get_config_value(profile, "owner") repo_name = repo_name or _get_config_value(profile, "repo_name") - action = ["repos", owner, repo_name, "rulesets"] else: org_name = org_name or _get_config_value(profile, "org_name") - action = ["orgs", org_name, "rulesets"] # check for optional header_dict try: @@ -1865,31 +1416,34 @@ def _check_ruleset_params( # if ruleset is required, check for ruleset_id if require_id: ruleset_id = ruleset_id or _get_config_value(profile, "ruleset_id") - action.append(ruleset_id or _get_config_value(profile, "ruleset_id")) # if ruleset_params are required, check for ruleset_params if require_params: ruleset_params = ruleset_params or _get_config_value(profile, "ruleset_params") - if not isinstance(ruleset_params, dict): - raise CommandExecutionError("ruleset_params need to be a dict") - if require_params is not True: - if missing := require_params.difference(ruleset_params): - raise CommandExecutionError( - f"The following ruleset_params are required, but not defined: {', '.join(missing)}" - ) - - params = filter_unset( + + return filter_unset( { "ruleset_type": ruleset_type, "owner": owner, "repo_name": repo_name, "org_name": org_name, + "headers": header_dict, "ruleset_id": ruleset_id, "ruleset_params": ruleset_params, } ) - return action, header_dict, params + +def _ruleset_error_ret(err): + """ + Format a :class:`GitHubApiError` into the legacy ruleset return shape. + """ + ret = {"status": err.status, "error": str(err)} + if err.data is not None: + ret["dict"] = err.data + ret["comment"] = f"GitHub Response Status Code: {err}" + ret["result"] = False + return ret def get_ruleset(profile="github", **kwargs): @@ -1924,20 +1478,16 @@ def get_ruleset(profile="github", **kwargs): salt myminion github.get_ruleset salt myminion github.get_ruleset ruleset_id=1 """ - action, header_dict, _ = _check_ruleset_params(profile, require_id=True, **kwargs) - + params = _resolve_ruleset_params(profile, require_id=True, **kwargs) + params.pop("ruleset_params", None) try: - ret = _query(profile, action, header_dict=header_dict) - if not ret.get("error"): - return ret["dict"] - ret["comment"] = f"GitHub Response Status Code: {ret.get('error')}" - ret["result"] = False + return gh_rulesets.get( + _get_conn(profile), params.pop("ruleset_id"), params.pop("ruleset_type"), **params + ) + except GitHubApiError as err: + return _ruleset_error_ret(err) except CommandExecutionError: - ret = {} - ret["result"] = False - ret["comment"] = "Error getting ruleset" - - return ret + return {"result": False, "comment": "Error getting ruleset"} def delete_ruleset(profile="github", **kwargs): @@ -1972,20 +1522,16 @@ def delete_ruleset(profile="github", **kwargs): salt myminion github.delete_ruleset salt myminion github.delete_ruleset ruleset_id=1 """ - action, header_dict, params = _check_ruleset_params(profile, require_id=True, **kwargs) - + params = _resolve_ruleset_params(profile, require_id=True, **kwargs) + params.pop("ruleset_params", None) + ruleset_id = params.pop("ruleset_id") try: - ret = _query(profile, action, method="DELETE", header_dict=header_dict) - if ret.get("error"): - ret["comment"] = f"GitHub Response Status Code: {ret.get('error')}" - ret["result"] = False - else: - ret["comment"] = f"ruleset {params['ruleset_id']} successfully deleted" + gh_rulesets.delete(_get_conn(profile), ruleset_id, params.pop("ruleset_type"), **params) + return {"comment": f"ruleset {ruleset_id} successfully deleted"} + except GitHubApiError as err: + return _ruleset_error_ret(err) except CommandExecutionError: - ret = {} - ret["result"] = False - ret["comment"] = "Error deleting ruleset" - return ret + return {"result": False, "comment": "Error deleting ruleset"} def update_ruleset(profile="github", **kwargs): @@ -2014,7 +1560,7 @@ def update_ruleset(profile="github", **kwargs): headers to pass to Github API. ruleset_params - number of returned results (max 100). + The updated ruleset definition. CLI Example: @@ -2023,26 +1569,22 @@ def update_ruleset(profile="github", **kwargs): salt myminion github.update_ruleset salt myminion github.update_ruleset ruleset_id=1 """ - action, header_dict, params = _check_ruleset_params( - profile, require_id=True, require_params=True, **kwargs - ) - + params = _resolve_ruleset_params(profile, require_id=True, require_params=True, **kwargs) + ruleset_id = params.pop("ruleset_id") + ruleset_params = params.pop("ruleset_params") + gh_rulesets.check_params(ruleset_params) try: - ret = _query( - profile, - action, - method="PUT", - data=params["ruleset_params"], - header_dict=header_dict, + return gh_rulesets.update( + _get_conn(profile), + ruleset_id, + ruleset_params, + params.pop("ruleset_type"), + **params, ) - if not ret.get("error"): - return ret["dict"] - ret["comment"] = f"GitHub Response Status Code: {ret.get('error')}" - ret["result"] = False + except GitHubApiError as err: + return _ruleset_error_ret(err) except CommandExecutionError: - ret = {"result": False, "comment": "Error updating ruleset"} - - return ret + return {"result": False, "comment": "Error updating ruleset"} def add_ruleset(profile="github", **kwargs): @@ -2068,7 +1610,8 @@ def add_ruleset(profile="github", **kwargs): headers to pass to Github API. ruleset_params - number of returned results (max 100). + The ruleset definition. Must contain at least ``name`` and + ``enforcement``. CLI Example: @@ -2076,25 +1619,18 @@ def add_ruleset(profile="github", **kwargs): salt myminion github.add_ruleset """ - action, header_dict, params = _check_ruleset_params( - profile, require_params={"name", "enforcement"}, **kwargs - ) - + params = _resolve_ruleset_params(profile, require_params=True, **kwargs) + params.pop("ruleset_id", None) + ruleset_params = params.pop("ruleset_params") + gh_rulesets.check_params(ruleset_params, required={"name", "enforcement"}) try: - ret = _query( - profile, - action, - method="POST", - data=params["ruleset_params"], - header_dict=header_dict, + return gh_rulesets.create( + _get_conn(profile), ruleset_params, params.pop("ruleset_type"), **params ) - if not ret.get("error"): - return ret["dict"] - ret["comment"] = f"GitHub Response Status Code: {ret.get('error')}" - ret["result"] = False + except GitHubApiError as err: + return _ruleset_error_ret(err) except CommandExecutionError: - ret = {"result": False, "comment": "Error adding ruleset"} - return ret + return {"result": False, "comment": "Error adding ruleset"} def list_rulesets(profile="github", *, per_page=None, **kwargs): @@ -2128,18 +1664,20 @@ def list_rulesets(profile="github", *, per_page=None, **kwargs): salt myminion github.list_rulesets """ - action, header_dict, _ = _check_ruleset_params(profile, **kwargs) + params = _resolve_ruleset_params(profile, **kwargs) + params.pop("ruleset_id", None) + params.pop("ruleset_params", None) try: per_page = per_page if per_page is not None else _get_config_value(profile, "per_page") except CommandExecutionError: per_page = None try: - ret = _query(profile, action, header_dict=header_dict, per_page=per_page) - if not ret.get("error"): - return {"rulesets": ret["dict"] or None} - ret["comment"] = f"GitHub Response Status Code: {ret.get('error')}" - ret["result"] = False + rulesets = gh_rulesets.list_all( + _get_conn(profile), params.pop("ruleset_type"), per_page=per_page, **params + ) + return {"rulesets": rulesets or None} + except GitHubApiError as err: + return _ruleset_error_ret(err) except CommandExecutionError: - ret = {"result": False, "comment": "Error getting rulesets"} - return ret + return {"result": False, "comment": "Error getting rulesets"} diff --git a/src/saltext/github/utils/github.py b/src/saltext/github/utils/github/__init__.py similarity index 72% rename from src/saltext/github/utils/github.py rename to src/saltext/github/utils/github/__init__.py index 64299453..340be2f7 100644 --- a/src/saltext/github/utils/github.py +++ b/src/saltext/github/utils/github/__init__.py @@ -1,5 +1,13 @@ """ -Connection library for GitHub +Connection library for GitHub. + +The submodules of this package provide reusable primitives for talking +to the GitHub API on behalf of an organization, built around +:class:`~saltext.github.utils.github.client.GitHubConnection`. +They raise :class:`~saltext.github.utils.github.exceptions.GitHubApiError` +(or its more specific subclass +:class:`~saltext.github.utils.github.exceptions.GitHubNotFoundError`) +on API errors. """ import logging @@ -7,6 +15,9 @@ import salt.utils.http import salt.utils.json +from saltext.github.utils.github.exceptions import GitHubApiError # noqa: F401 +from saltext.github.utils.github.exceptions import GitHubNotFoundError # noqa: F401 + log = logging.getLogger(__name__) diff --git a/src/saltext/github/utils/github/client.py b/src/saltext/github/utils/github/client.py new file mode 100644 index 00000000..091ebb7d --- /dev/null +++ b/src/saltext/github/utils/github/client.py @@ -0,0 +1,188 @@ +""" +Shared plumbing for talking to the GitHub v3 API via PyGithub. + +The functions in this package operate on a :class:`GitHubConnection` and +raise :class:`~saltext.github.utils.github.exceptions.GitHubApiError` +(or its :class:`~saltext.github.utils.github.exceptions.GitHubNotFoundError` +subclass) on API errors. They are shared between the classic ``github`` +execution module and future resource modules. +""" + +import functools +import json +import logging +import re + +import github +from github.GithubException import UnknownObjectException +from salt.exceptions import CommandExecutionError + +from saltext.github.utils.github.exceptions import GitHubApiError +from saltext.github.utils.github.exceptions import GitHubNotFoundError + +log = logging.getLogger(__name__) + +API_URL_DEFAULT = "https://api.github.com" + + +class GitHubConnection: + """ + An authenticated PyGithub client bound to a single organization, + together with caches for API responses. + + token + Personal access token to authenticate with. + + org_name + The name of the organization this connection is scoped to. + + api_url + The base URL of the GitHub API. Set this when targeting a GitHub + Enterprise Server instance. Defaults to the public GitHub API. + """ + + def __init__(self, token, org_name, *, api_url=API_URL_DEFAULT): + self.org_name = org_name + self.api_url = (api_url or API_URL_DEFAULT).rstrip("/") + self.client = github.Github( + auth=github.Auth.Token(token), base_url=self.api_url, per_page=100 + ) + self.cache = {} + + @property + def org(self): + """ + The PyGithub ``Organization`` object this connection is scoped to (cached). + """ + if "org" not in self.cache: + self.cache["org"] = self.client.get_organization(self.org_name) + return self.cache["org"] + + +def _error_message(status, data, fallback=""): + message = data.get("message", fallback) if isinstance(data, dict) else (data or fallback) + return f"{status} {message}".strip() + + +def api_request(func): + """ + Decorator that translates PyGithub exceptions escaping ``func`` into + :class:`GitHubApiError`/:class:`GitHubNotFoundError`, ensuring callers + never need to handle PyGithub exception types. + """ + + @functools.wraps(func) + def wrapper(*args, **kwargs): + try: + return func(*args, **kwargs) + except UnknownObjectException as err: + raise GitHubNotFoundError( + _error_message(err.status, err.data), status=err.status, data=err.data + ) from err + except github.GithubException as err: + raise GitHubApiError( + _error_message(err.status, err.data), status=err.status, data=err.data + ) from err + + return wrapper + + +def query(conn, path, *, params=None, method="GET", headers=None, data=None, per_page=None): + """ + Make a direct call to the GitHub API, transparently following pagination. + + conn + The :class:`GitHubConnection` to run the request through (honoring + its ``api_url``, authentication and retry handling). + + path + The URL path to query, either as a string or a list of segments. + + params + Mapping of query parameters. + + method + HTTP method to use. Defaults to ``GET``. + + headers + Mapping of extra headers to send. + + data + Request body. JSON strings are decoded first. + + per_page + Pagination size, maximum 100. + + Returns the decoded response data. List results are merged across pages. + Raises :class:`GitHubNotFoundError` for missing resources and + :class:`GitHubApiError` for other error responses. + """ + args = dict(params) if isinstance(params, dict) else {} + segments = [path] if isinstance(path, str) else list(path or []) + url = "/" + "/".join(str(segment) for segment in segments) + + log.debug("GitHub API path: %s", url) + + if per_page and "per_page" not in args: + args["per_page"] = per_page + + if isinstance(data, str): + data = json.loads(data) + + requester = conn.client.requester + + complete_result = [] + next_page = None + while True: + if next_page: + args["page"] = next_page + try: + status, resp_headers, text = requester.requestJson( + method, url, parameters=args or None, headers=headers, input=data + ) + except github.GithubException as err: + exc_type = GitHubNotFoundError if err.status == 404 else GitHubApiError + raise exc_type( + _error_message(err.status, err.data), status=err.status, data=err.data + ) from err + except Exception as err: # pylint: disable=broad-except + raise CommandExecutionError(f"GitHub API request failed: {err}") from err + log.debug("GitHub Response Status Code: %s", status) + decoded = json.loads(text) if text else None + + if status not in (200, 201, 204): + exc_type = GitHubNotFoundError if status == 404 else GitHubApiError + raise exc_type(_error_message(status, decoded, text), status=status, data=decoded) + + if not isinstance(decoded, list): + # If only querying for one item, such as a single issue, + # the GitHub API returns a single dictionary, instead of + # a list of dictionaries. In that case, we can return. + return decoded + + complete_result += decoded + + next_page = _next_page_number(resp_headers) + if next_page is None: + break + + return complete_result + + +def _next_page_number(headers): + """ + Extract the next page number from a GitHub ``Link`` response header. + + Returns ``None`` when there is no further page. + """ + link = None + for key, val in (headers or {}).items(): + if key.lower() == "link": + link = val + break + if not link: + return None + match = re.search(r'<[^>]*[?&]page=(\d+)[^>]*>\s*;\s*rel="next"', link) + if match: + return match.group(1) + return None diff --git a/src/saltext/github/utils/github/exceptions.py b/src/saltext/github/utils/github/exceptions.py new file mode 100644 index 00000000..862e114f --- /dev/null +++ b/src/saltext/github/utils/github/exceptions.py @@ -0,0 +1,28 @@ +""" +Exceptions raised by the GitHub API helper modules. +""" + +from salt.exceptions import CommandExecutionError + + +class GitHubApiError(CommandExecutionError): + """ + The GitHub API returned an error response. + + status + The HTTP status code of the response, if available. + + data + The decoded response body, if available. + """ + + def __init__(self, message, *, status=None, data=None): + super().__init__(message) + self.status = status + self.data = data + + +class GitHubNotFoundError(GitHubApiError): + """ + A requested GitHub resource does not exist. + """ diff --git a/src/saltext/github/utils/github/issues.py b/src/saltext/github/utils/github/issues.py new file mode 100644 index 00000000..76a901fc --- /dev/null +++ b/src/saltext/github/utils/github/issues.py @@ -0,0 +1,148 @@ +""" +Operations on GitHub repository issues. +""" + +import logging + +from saltext.github.utils.github import client + +log = logging.getLogger(__name__) + + +def get(conn, repo_name, number): + """ + Return the raw data of a single issue. + + conn + The :class:`~saltext.github.utils.github.client.GitHubConnection` to use. + + repo_name + The name of the repository the issue belongs to. + + number + The number of the issue to retrieve. + """ + return client.query(conn, ["repos", conn.org_name, repo_name, "issues", str(number)]) + + +def get_comments(conn, repo_name, number, since=None): + """ + Return the raw comment data for a single issue. + + conn + The :class:`~saltext.github.utils.github.client.GitHubConnection` to use. + + repo_name + The name of the repository the issue belongs to. + + number + The number of the issue whose comments to retrieve. + + since + Only comments updated at or after this time are returned. This is a + timestamp in ISO 8601 format: ``YYYY-MM-DDTHH:MM:SSZ``. + """ + params = {} + if since: + params["since"] = since + return client.query( + conn, ["repos", conn.org_name, repo_name, "issues", str(number), "comments"], params=params + ) + + +def list_all( + conn, + repo_name, + *, + milestone=None, + state="open", + assignee=None, + creator=None, + mentioned=None, + labels=None, + sort="created", + direction="desc", + since=None, + per_page=None, +): + """ + Return the raw data of all issues in a repository matching the search + options. Pull requests are excluded. + + conn + The :class:`~saltext.github.utils.github.client.GitHubConnection` to use. + + repo_name + The name of the repository whose issues to list. + + Other parameters as described in + :py:func:`github.get_issues `. + """ + params = {} + + # Build API arguments, as necessary. + if milestone: + params["milestone"] = milestone + if assignee: + params["assignee"] = assignee + if creator: + params["creator"] = creator + if mentioned: + params["mentioned"] = mentioned + if labels: + params["labels"] = labels + if since: + params["since"] = since + if per_page: + params["per_page"] = per_page + + # Only pass the following API args if they're not the defaults listed. + if state and state != "open": + params["state"] = state + if sort and sort != "created": + params["sort"] = sort + if direction and direction != "desc": + params["direction"] = direction + + issues = client.query(conn, ["repos", conn.org_name, repo_name, "issues"], params=params) + + # Pull requests are included in the issue list from GitHub. + # Let's not include those in the return. + return [issue for issue in issues if not issue.get("pull_request")] + + +def format_issue(issue): + """ + Format API return information into a more manageable + and useful dictionary for issue information. + + issue + The issue to format. + """ + ret = { + "id": issue.get("id"), + "issue_number": issue.get("number"), + "state": issue.get("state"), + "title": issue.get("title"), + "user": issue.get("user").get("login"), + "html_url": issue.get("html_url"), + } + + assignee = issue.get("assignee") + if assignee: + assignee = assignee.get("login") + + labels = issue.get("labels") + label_names = [] + for label in labels: + label_names.append(label.get("name")) + + milestone = issue.get("milestone") + if milestone: + milestone = milestone.get("title") + + ret["assignee"] = assignee + ret["labels"] = label_names + ret["milestone"] = milestone + + return ret diff --git a/src/saltext/github/utils/github/milestones.py b/src/saltext/github/utils/github/milestones.py new file mode 100644 index 00000000..52f6f4cb --- /dev/null +++ b/src/saltext/github/utils/github/milestones.py @@ -0,0 +1,77 @@ +""" +Operations on GitHub repository milestones. +""" + +import logging + +from saltext.github.utils.github import client + +log = logging.getLogger(__name__) + + +def get(conn, repo_name, number): + """ + Return the raw data of a single milestone. + + conn + The :class:`~saltext.github.utils.github.client.GitHubConnection` to use. + + repo_name + The name of the repository the milestone belongs to. + + number + The number of the milestone to retrieve. + """ + return client.query(conn, ["repos", conn.org_name, repo_name, "milestones", str(number)]) + + +def list_all( + conn, + repo_name, + *, + state="open", + sort="due_on", + direction="asc", + per_page=None, +): + """ + Return the raw data of all milestones in a repository matching the + search options. + + conn + The :class:`~saltext.github.utils.github.client.GitHubConnection` to use. + + repo_name + The name of the repository whose milestones to list. + + Other parameters as described in + :py:func:`github.get_milestones `. + """ + params = {} + + if per_page: + params["per_page"] = per_page + + # Only pass the following API args if they're not the defaults listed. + if state and state != "open": + params["state"] = state + if sort and sort != "due_on": + params["sort"] = sort + if direction and direction != "asc": + params["direction"] = direction + + return client.query(conn, ["repos", conn.org_name, repo_name, "milestones"], params=params) + + +def format_milestone(milestone): + """ + Trim milestone data to the fields that are useful in (CLI) output. + + milestone + The milestone to format. + """ + return { + key: value + for key, value in milestone.items() + if key not in ("creator", "html_url", "labels_url") + } diff --git a/src/saltext/github/utils/github/pulls.py b/src/saltext/github/utils/github/pulls.py new file mode 100644 index 00000000..1de17b2a --- /dev/null +++ b/src/saltext/github/utils/github/pulls.py @@ -0,0 +1,75 @@ +""" +Operations on GitHub repository pull requests. +""" + +import logging + +from saltext.github.utils.github import client + +log = logging.getLogger(__name__) + + +def list_all( + conn, + repo_name, + *, + state="open", + head=None, + base=None, + sort="created", + direction="desc", + per_page=None, +): + """ + Return the raw data of all pull requests in a repository matching the + search options. + + conn + The :class:`~saltext.github.utils.github.client.GitHubConnection` to use. + + repo_name + The name of the repository whose pull requests to list. + + Other parameters as described in + :py:func:`github.get_prs `. + """ + params = {} + + # Build API arguments, as necessary. + if head: + params["head"] = head + if base: + params["base"] = base + if per_page: + params["per_page"] = per_page + + # Only pass the following API args if they're not the defaults listed. + if state and state != "open": + params["state"] = state + if sort and sort != "created": + params["sort"] = sort + if direction and direction != "desc": + params["direction"] = direction + + return client.query(conn, ["repos", conn.org_name, repo_name, "pulls"], params=params) + + +def format_pr(pr_): + r""" + Format API return information into a more manageable + and useful dictionary for pull request information. + + pr\_ + The pull request to format. + """ + ret = { + "id": pr_.get("id"), + "pr_number": pr_.get("number"), + "state": pr_.get("state"), + "title": pr_.get("title"), + "user": pr_.get("user").get("login"), + "html_url": pr_.get("html_url"), + "base_branch": pr_.get("base").get("ref"), + } + + return ret diff --git a/src/saltext/github/utils/github/repos.py b/src/saltext/github/utils/github/repos.py new file mode 100644 index 00000000..91edbc8e --- /dev/null +++ b/src/saltext/github/utils/github/repos.py @@ -0,0 +1,235 @@ +""" +Operations on GitHub organization repositories. +""" + +import logging + +from github.GithubException import UnknownObjectException +from salt.exceptions import CommandExecutionError + +from saltext.github.utils import filter_unset +from saltext.github.utils.github.client import api_request +from saltext.github.utils.github.exceptions import GitHubNotFoundError + +log = logging.getLogger(__name__) + + +def to_dict(repo): + """ + Render a PyGithub ``Repository`` object into a plain dictionary. + """ + ret = {} + ret["id"] = repo.id + ret["name"] = repo.name + ret["full_name"] = repo.full_name + ret["owner"] = repo.owner.login + ret["private"] = repo.private + ret["html_url"] = repo.html_url + ret["description"] = repo.description + ret["fork"] = repo.fork + ret["homepage"] = repo.homepage + ret["size"] = repo.size + ret["stargazers_count"] = repo.stargazers_count + ret["watchers_count"] = repo.watchers_count + ret["language"] = repo.language + ret["open_issues_count"] = repo.open_issues_count + ret["forks"] = repo.forks + ret["open_issues"] = repo.open_issues + ret["watchers"] = repo.watchers + ret["default_branch"] = repo.default_branch + ret["has_issues"] = repo.has_issues + ret["has_wiki"] = repo.has_wiki + ret["has_downloads"] = repo.has_downloads + return ret + + +@api_request +def list_all(conn, ignore_cache=False): + """ + Return all repositories of the organization as PyGithub ``Repository`` + objects (cached). + + conn + The :class:`~saltext.github.utils.github.client.GitHubConnection` to use. + + ignore_cache + Bypass the cached repository list. + """ + if "repos" not in conn.cache or ignore_cache: + repos = [] + for repo in conn.org.get_repos(): + repos.append(repo) + # Cache a copy of each repo for single lookups + conn.cache[("repo_info", repo.name.lower())] = to_dict(repo) + conn.cache["repos"] = repos + return conn.cache["repos"] + + +@api_request +def get_info(conn, name, ignore_cache=False): + """ + Return information about a single repository as a dictionary (cached). + + conn + The :class:`~saltext.github.utils.github.client.GitHubConnection` to use. + + name + The name of the repository. + + ignore_cache + Bypass the cached repository information. + + Raises :class:`~saltext.github.utils.github.exceptions.GitHubNotFoundError` + if the repository does not exist. + """ + key = ("repo_info", name.lower()) + if key not in conn.cache or ignore_cache: + try: + # With lazy fetching, client.get_repo returns a Repository object + # even if the repo does not exist - the exception is only raised + # once attributes of the object are accessed. + repo = conn.client.get_repo(f"{conn.org_name}/{name}") + conn.cache[key] = to_dict(repo) + except UnknownObjectException as exc: + raise GitHubNotFoundError( + f"The '{name}' repository under the '{conn.org_name}' organization " + "could not be found.", + status=exc.status, + data=exc.data, + ) from exc + return conn.cache[key] + + +@api_request +def get_teams(conn, name): + """ + Return the teams a repository belongs to. + + conn + The :class:`~saltext.github.utils.github.client.GitHubConnection` to use. + + name + The name of the repository. + + Raises :class:`~saltext.github.utils.github.exceptions.GitHubNotFoundError` + if the repository does not exist. + """ + try: + repo = conn.client.get_repo(f"{conn.org_name}/{name}") + teams = repo.get_teams() + return [{"id": team.id, "name": team.name, "permission": team.permission} for team in teams] + except UnknownObjectException as exc: + raise GitHubNotFoundError( + f"The '{name}' repository under the '{conn.org_name}' organization " + "could not be found.", + status=exc.status, + data=exc.data, + ) from exc + + +@api_request +def create( + conn, + name, + *, + description=None, + homepage=None, + private=None, + has_issues=None, + has_wiki=None, + has_downloads=None, + auto_init=None, + gitignore_template=None, + license_template=None, +): + """ + Create a new repository in the organization. + + conn + The :class:`~saltext.github.utils.github.client.GitHubConnection` to use. + + name + The name of the repository to create. + + Other parameters as described in + :py:func:`github.add_repo `. + """ + parameters = filter_unset( + { + "description": description, + "homepage": homepage, + "private": private, + "has_issues": has_issues, + "has_wiki": has_wiki, + "has_downloads": has_downloads, + "auto_init": auto_init, + "gitignore_template": gitignore_template, + "license_template": license_template, + } + ) + conn.org.create_repo(name, **parameters) + + +@api_request +def update( + conn, + name, + *, + description=None, + homepage=None, + private=None, + has_issues=None, + has_wiki=None, + has_downloads=None, + allow_privacy_change=False, +): + """ + Update an existing repository in the organization. + + conn + The :class:`~saltext.github.utils.github.client.GitHubConnection` to use. + + name + The name of the repository to update. + + allow_privacy_change + Whether a change of the repository's visibility is allowed. + Defaults to false. + + Other parameters as described in + :py:func:`github.edit_repo `. + """ + if private is not None and not allow_privacy_change: + raise CommandExecutionError( + f"The private field is set to be changed for repo {name} " + "but allow_repo_privacy_changes disallows this." + ) + repo = conn.org.get_repo(name) + parameters = filter_unset( + { + "description": description, + "homepage": homepage, + "private": private, + "has_issues": has_issues, + "has_wiki": has_wiki, + "has_downloads": has_downloads, + } + ) + repo.edit(name, **parameters) + get_info(conn, name, ignore_cache=True) # Refresh cache + + +@api_request +def delete(conn, name): + """ + Delete a repository from the organization. + + conn + The :class:`~saltext.github.utils.github.client.GitHubConnection` to use. + + name + The name of the repository to delete. + """ + repo = conn.org.get_repo(name) + repo.delete() + list_all(conn, ignore_cache=True) # Refresh cache diff --git a/src/saltext/github/utils/github/rulesets.py b/src/saltext/github/utils/github/rulesets.py new file mode 100644 index 00000000..2d8b8f89 --- /dev/null +++ b/src/saltext/github/utils/github/rulesets.py @@ -0,0 +1,170 @@ +""" +Operations on GitHub repository and organization rulesets. +""" + +import logging + +from salt.exceptions import CommandExecutionError + +from saltext.github.utils.github import client + +log = logging.getLogger(__name__) + + +def build_path(ruleset_type, *, owner=None, repo_name=None, org_name=None, ruleset_id=None): + """ + Build the API path for ruleset operations. + + ruleset_type + The type of ruleset. ``repo`` or ``org``. + + owner + The account owner of the repository. Required for ``repo`` rulesets. + + repo_name + The name of the repository. Required for ``repo`` rulesets. + + org_name + The name of the organization. Required for ``org`` rulesets. + + ruleset_id + The ID of a specific ruleset to address. + """ + if ruleset_type == "repo": + path = ["repos", owner, repo_name, "rulesets"] + elif ruleset_type == "org": + path = ["orgs", org_name, "rulesets"] + else: + raise CommandExecutionError(f"Invalid ruleset type: {ruleset_type}") + if ruleset_id is not None: + path.append(ruleset_id) + return path + + +def check_params(ruleset_params, required=None): + """ + Ensure a ruleset parameter payload is a mapping and contains all + required keys. + + ruleset_params + The payload to check. + + required + An optional set of parameter names that must be present. + """ + if not isinstance(ruleset_params, dict): + raise CommandExecutionError("ruleset_params need to be a dict") + if required and (missing := set(required).difference(ruleset_params)): + raise CommandExecutionError( + f"The following ruleset_params are required, but not defined: {', '.join(missing)}" + ) + + +def list_all( + conn, ruleset_type, *, owner=None, repo_name=None, org_name=None, headers=None, per_page=None +): + """ + Return all rulesets of a repository or organization. + + conn + The :class:`~saltext.github.utils.github.client.GitHubConnection` to use. + + Other parameters as described in :func:`build_path`. ``headers`` is a + mapping of extra headers to send. + """ + path = build_path(ruleset_type, owner=owner, repo_name=repo_name, org_name=org_name) + return client.query(conn, path, headers=headers, per_page=per_page) + + +def get(conn, ruleset_id, ruleset_type, *, owner=None, repo_name=None, org_name=None, headers=None): + """ + Return the data of a specific ruleset. + + conn + The :class:`~saltext.github.utils.github.client.GitHubConnection` to use. + + ruleset_id + The ID of the ruleset to retrieve. + + Other parameters as described in :func:`build_path`. ``headers`` is a + mapping of extra headers to send. + """ + path = build_path( + ruleset_type, owner=owner, repo_name=repo_name, org_name=org_name, ruleset_id=ruleset_id + ) + return client.query(conn, path, headers=headers) + + +def create( + conn, ruleset_params, ruleset_type, *, owner=None, repo_name=None, org_name=None, headers=None +): + """ + Create a new ruleset. Returns the data of the created ruleset. + + conn + The :class:`~saltext.github.utils.github.client.GitHubConnection` to use. + + ruleset_params + The ruleset definition. Must contain at least ``name`` and + ``enforcement``. + + Other parameters as described in :func:`build_path`. ``headers`` is a + mapping of extra headers to send. + """ + check_params(ruleset_params, required={"name", "enforcement"}) + path = build_path(ruleset_type, owner=owner, repo_name=repo_name, org_name=org_name) + return client.query(conn, path, method="POST", data=ruleset_params, headers=headers) + + +def update( + conn, + ruleset_id, + ruleset_params, + ruleset_type, + *, + owner=None, + repo_name=None, + org_name=None, + headers=None, +): + """ + Update an existing ruleset. Returns the data of the updated ruleset. + + conn + The :class:`~saltext.github.utils.github.client.GitHubConnection` to use. + + ruleset_id + The ID of the ruleset to update. + + ruleset_params + The updated ruleset definition. + + Other parameters as described in :func:`build_path`. ``headers`` is a + mapping of extra headers to send. + """ + check_params(ruleset_params) + path = build_path( + ruleset_type, owner=owner, repo_name=repo_name, org_name=org_name, ruleset_id=ruleset_id + ) + return client.query(conn, path, method="PUT", data=ruleset_params, headers=headers) + + +def delete( + conn, ruleset_id, ruleset_type, *, owner=None, repo_name=None, org_name=None, headers=None +): + """ + Delete a ruleset. + + conn + The :class:`~saltext.github.utils.github.client.GitHubConnection` to use. + + ruleset_id + The ID of the ruleset to delete. + + Other parameters as described in :func:`build_path`. ``headers`` is a + mapping of extra headers to send. + """ + path = build_path( + ruleset_type, owner=owner, repo_name=repo_name, org_name=org_name, ruleset_id=ruleset_id + ) + client.query(conn, path, method="DELETE", headers=headers) diff --git a/src/saltext/github/utils/github/teams.py b/src/saltext/github/utils/github/teams.py new file mode 100644 index 00000000..b5524313 --- /dev/null +++ b/src/saltext/github/utils/github/teams.py @@ -0,0 +1,289 @@ +""" +Operations on GitHub organization teams. + +Most functions take a ``team_info`` parameter, which is the team's entry +in the (cached) mapping returned by :func:`list_all`. Team-scoped caches +(repositories, members) are attached to this entry. +""" + +import logging + +from github.GithubException import UnknownObjectException + +from saltext.github.utils import filter_unset +from saltext.github.utils.github.client import api_request + +log = logging.getLogger(__name__) + + +@api_request +def list_all(conn, ignore_cache=False): + """ + Return all teams of the organization as a mapping of team name to + team details (cached). + + conn + The :class:`~saltext.github.utils.github.client.GitHubConnection` to use. + + ignore_cache + Bypass the cached team list. + """ + if "teams" not in conn.cache or ignore_cache: + teams = {} + for team in conn.org.get_teams(): + # These attributes are all present in the list response. Take care + # not to access any lazily completable attributes missing from it + # here since each one would trigger a separate request per team. + teams[team.name] = { + "id": team.id, + "slug": team.slug, + "description": team.description, + "permission": team.permission, + "privacy": team.privacy, + } + conn.cache["teams"] = teams + return conn.cache["teams"] + + +def get(conn, name): + """ + Return the (cached) details of a single team, or None if it does + not exist. + + conn + The :class:`~saltext.github.utils.github.client.GitHubConnection` to use. + + name + The name of the team. + """ + return list_all(conn).get(name) + + +@api_request +def create(conn, name, *, description=None, repo_names=None, privacy=None, permission=None): + """ + Create a new team in the organization. + + conn + The :class:`~saltext.github.utils.github.client.GitHubConnection` to use. + + name + The name of the team to create. + + Other parameters as described in + :py:func:`github.add_team `. + """ + parameters = filter_unset( + { + "description": description, + "permission": permission, + "privacy": privacy, + } + ) + if repo_names is not None: + # create_team expects Repository objects, resolve the (full) names + parameters["repo_names"] = [ + conn.client.get_repo(repo) if "/" in repo else conn.org.get_repo(repo) + for repo in repo_names + ] + conn.org.create_team(name, **parameters) + list_all(conn, ignore_cache=True) # Refresh cache + + +@api_request +def update(conn, team_info, name, *, description=None, privacy=None, permission=None): + """ + Update an existing team. + + conn + The :class:`~saltext.github.utils.github.client.GitHubConnection` to use. + + team_info + The team's entry from :func:`list_all`. + + name + The name of the team. + + Other parameters as described in + :py:func:`github.edit_team `. + """ + team = conn.org.get_team_by_slug(team_info["slug"]) + team.edit( + name, + **filter_unset( + { + "description": description, + "privacy": privacy, + "permission": permission, + } + ), + ) + + +@api_request +def delete(conn, team_info): + """ + Delete a team from the organization. + + conn + The :class:`~saltext.github.utils.github.client.GitHubConnection` to use. + + team_info + The team's entry from :func:`list_all`. + """ + team = conn.org.get_team_by_slug(team_info["slug"]) + team.delete() + list_all(conn, ignore_cache=True) # Refresh cache + + +@api_request +def list_repos(conn, team_info, ignore_cache=False): + """ + Return the repositories of a team as a mapping of (lowercase) repository + name to repository details (cached). + + conn + The :class:`~saltext.github.utils.github.client.GitHubConnection` to use. + + team_info + The team's entry from :func:`list_all`. + + ignore_cache + Bypass the cached team repositories. + """ + if team_info.get("repos") and not ignore_cache: + return team_info["repos"] + + team = conn.org.get_team_by_slug(team_info["slug"]) + repos = {} + for repo in team.get_repos(): + permission = "pull" + if repo.permissions.admin: + permission = "admin" + elif repo.permissions.push: + permission = "push" + + repos[repo.name.lower()] = {"permission": permission} + team_info["repos"] = repos + return repos + + +@api_request +def add_repo(conn, team_info, repo_name, permission=None): + """ + Add a repository to a team, or update the permission the team has + on it. Returns false if updating the permission failed. + + conn + The :class:`~saltext.github.utils.github.client.GitHubConnection` to use. + + team_info + The team's entry from :func:`list_all`. + + repo_name + The name of the repository to add. + + permission + The permission for team members within the repository, can be ``pull``, + ``push`` or ``admin``. If not specified, the default permission specified + on the team will be used. + """ + team = conn.org.get_team_by_slug(team_info["slug"]) + repo = conn.org.get_repo(repo_name) + if permission is None: + team.add_to_repos(repo) + elif not team.update_team_repository(repo, permission): + return False + list_repos(conn, team_info, ignore_cache=True) # Refresh cache + return True + + +@api_request +def remove_repo(conn, team_info, repo_name): + """ + Remove a repository from a team. Returns whether the repository is no + longer associated with the team afterwards. + + conn + The :class:`~saltext.github.utils.github.client.GitHubConnection` to use. + + team_info + The team's entry from :func:`list_all`. + + repo_name + The name of the repository to remove. + """ + team = conn.org.get_team_by_slug(team_info["slug"]) + repo = conn.org.get_repo(repo_name) + team.remove_from_repos(repo) + return repo_name not in list_repos(conn, team_info, ignore_cache=True) + + +@api_request +def list_members(conn, team_info, ignore_cache=False): + """ + Return the (lowercase) logins of a team's members (cached). + + conn + The :class:`~saltext.github.utils.github.client.GitHubConnection` to use. + + team_info + The team's entry from :func:`list_all`. + + ignore_cache + Bypass the cached team members. + """ + if team_info.get("members") and not ignore_cache: + return team_info["members"] + + team = conn.org.get_team_by_slug(team_info["slug"]) + team_info["members"] = [member.login.lower() for member in team.get_members()] + return team_info["members"] + + +@api_request +def add_member(conn, team_info, login): + """ + Add an organization member to a team. + + conn + The :class:`~saltext.github.utils.github.client.GitHubConnection` to use. + + team_info + The team's entry from :func:`list_all`. + + login + The login of the user to add. + """ + team = conn.org.get_team_by_slug(team_info["slug"]) + member = conn.client.get_user(login) + team.add_membership(member, "member") + + +@api_request +def remove_member(conn, team_info, login): + """ + Remove a member from a team. Returns whether the user is no longer + a member of the team afterwards. + + conn + The :class:`~saltext.github.utils.github.client.GitHubConnection` to use. + + team_info + The team's entry from :func:`list_all`. + + login + The login of the user to remove. + + Raises :class:`~saltext.github.utils.github.exceptions.GitHubNotFoundError` + if the team or user does not exist. + """ + team = conn.org.get_team_by_slug(team_info["slug"]) + member = conn.client.get_user(login) + team.remove_membership(member) + try: + # There is no non-deprecated endpoint for checking plain membership + team.get_team_membership(member) + except UnknownObjectException: + return True + return False diff --git a/src/saltext/github/utils/github/users.py b/src/saltext/github/utils/github/users.py new file mode 100644 index 00000000..7285623b --- /dev/null +++ b/src/saltext/github/utils/github/users.py @@ -0,0 +1,136 @@ +""" +Operations on GitHub organization members. +""" + +import logging + +from github.GithubException import UnknownObjectException + +from saltext.github.utils.github.client import api_request + +log = logging.getLogger(__name__) + + +@api_request +def list_members(conn, ignore_cache=False): + """ + Return the logins of all members of the organization (cached). + + conn + The :class:`~saltext.github.utils.github.client.GitHubConnection` to use. + + ignore_cache + Bypass the cached member list. + """ + if "users" not in conn.cache or ignore_cache: + conn.cache["users"] = [member.login for member in conn.org.get_members()] + return conn.cache["users"] + + +@api_request +def get_user_info(conn, login): + """ + Return information about a user, including their organization + membership state. + + conn + The :class:`~saltext.github.utils.github.client.GitHubConnection` to use. + + login + The login of the user to look up. + + Raises :class:`~saltext.github.utils.github.exceptions.GitHubNotFoundError` + if the user does not exist. + """ + user = conn.client.get_user(login) + + ret = { + "company": user.company, + "created_at": user.created_at, + "email": user.email, + "html_url": user.html_url, + "id": user.id, + "login": user.login, + "name": user.name, + "type": user.type, + "url": user.url, + } + + try: + membership = user.get_organization_membership(conn.org) + except UnknownObjectException: + ret["membership_state"] = "nonexistent" + ret["in_org"] = False + return ret + + ret["in_org"] = conn.org.has_in_members(user) + ret["membership_state"] = membership.state + + return ret + + +@api_request +def invite_member(conn, login): + """ + Invite a user into the organization. Returns whether the resulting + membership state is ``pending``. + + conn + The :class:`~saltext.github.utils.github.client.GitHubConnection` to use. + + login + The login of the user to invite. + + Raises :class:`~saltext.github.utils.github.exceptions.GitHubNotFoundError` + if the user does not exist. + """ + user = conn.client.get_user(login) + + # There is no public method that also returns the resulting membership state + _, data = conn.client.requester.requestJsonAndCheck( + "PUT", conn.org.url + "/memberships/" + user.login + ) + + return data.get("state") == "pending" + + +@api_request +def remove_member(conn, login): + """ + Remove a user from the organization. Returns whether the user is no + longer a member afterwards. + + conn + The :class:`~saltext.github.utils.github.client.GitHubConnection` to use. + + login + The login of the user to remove. + + Raises :class:`~saltext.github.utils.github.exceptions.GitHubNotFoundError` + if the user does not exist. + """ + user = conn.client.get_user(login) + + if conn.org.has_in_members(user): + conn.org.remove_from_members(user) + + return not conn.org.has_in_members(user) + + +@api_request +def list_members_without_mfa(conn, ignore_cache=False): + """ + Return the logins (lowercase) of all organization members without + MFA turned on (cached). + + conn + The :class:`~saltext.github.utils.github.client.GitHubConnection` to use. + + ignore_cache + Bypass the cached member list. + """ + if "members_without_mfa" not in conn.cache or ignore_cache: + conn.cache["members_without_mfa"] = [ + member.login.lower() for member in conn.org.get_members(filter_="2fa_disabled") + ] + return conn.cache["members_without_mfa"] diff --git a/tests/functional/modules/github/test_profiles.py b/tests/functional/modules/github/test_profiles.py index dd86314b..9a010824 100644 --- a/tests/functional/modules/github/test_profiles.py +++ b/tests/functional/modules/github/test_profiles.py @@ -3,10 +3,11 @@ internal cache refreshes (which used to fall back to the default profile). """ -import github as pygithub import pytest from salt.exceptions import CommandExecutionError +from saltext.github.utils.github.exceptions import GitHubNotFoundError + @pytest.fixture(scope="module") def minion_config_defaults(github_profile): @@ -25,7 +26,7 @@ def test_add_team_uses_profile(github, fake_org): def test_module_functions_honor_profile(github): assert set(github.list_repos(profile="primary")) == {"repo-a", "repo-b"} - with pytest.raises(pygithub.UnknownObjectException): + with pytest.raises(GitHubNotFoundError): github.list_repos() diff --git a/tests/functional/modules/github/test_sdb_profiles.py b/tests/functional/modules/github/test_sdb_profiles.py new file mode 100644 index 00000000..2b5c8950 --- /dev/null +++ b/tests/functional/modules/github/test_sdb_profiles.py @@ -0,0 +1,34 @@ +""" +Verify that profile configuration values can be sourced from SDB URIs, +e.g. environment variables via the ``env`` driver (useful in CI). + +Values in the minion configuration are rendered by Salt itself when the +configuration is loaded, hence the environment variable needs to be set +before the minion options are built. Access-time rendering (which covers +pillar-sourced profiles) is exercised in the unit tests. +""" + +import pytest + + +@pytest.fixture(scope="module") +def github_token_env(github_profile): + with pytest.MonkeyPatch.context() as mp: + mp.setenv("GITHUB_TOKEN", github_profile["token"]) + yield + + +@pytest.fixture(scope="module") +def minion_config_defaults(github_profile, github_token_env): # pylint: disable=unused-argument + profile = dict(github_profile) + profile["token"] = "sdb://osenv/GITHUB_TOKEN" + return { + "github": profile, + "osenv": {"driver": "env"}, + } + + +def test_profile_values_resolve_sdb_uris(github): + # The fake GitHub API server rejects requests carrying an invalid token, + # so this only passes if the SDB URI was rendered. + assert set(github.list_repos()) == {"repo-a", "repo-b"} diff --git a/tests/unit/modules/test_github.py b/tests/unit/modules/test_github.py index c3928da4..d472bbaf 100644 --- a/tests/unit/modules/test_github.py +++ b/tests/unit/modules/test_github.py @@ -6,13 +6,18 @@ from salt.modules import config as config_mod from saltext.github.modules import github as github_mod +from saltext.github.utils.github import client as gh_client +from saltext.github.utils.github.exceptions import GitHubApiError @pytest.fixture def configure_loader_modules(): module_globals = { - "__salt__": {"config.option": config_mod.option}, - "__opts__": {"test_ruleset": {"ruleset_type": "mocked_value"}}, + "__salt__": {"config.get": config_mod.get}, + "__opts__": { + "test_ruleset": {"ruleset_type": "mocked_value"}, + "sdb_profile": {"token": "sdb://osenv/GITHUB_TOKEN"}, + }, } return { github_mod: module_globals, @@ -31,77 +36,58 @@ def params(): } -def test__query_decodes_json_string_data(): - requester = MagicMock() - requester.requestJson.return_value = (200, {}, '{"written": true}') - client = MagicMock(requester=requester) - - with patch.object(github_mod, "_get_client", return_value=client): - ret = github_mod._query( - "test_ruleset", action="repos", method="POST", data='{"some": "payload"}' - ) - - assert ret == {"status": 200, "headers": {}, "dict": {"written": True}} - requester.requestJson.assert_called_once_with( - "POST", "/repos", parameters=None, headers=None, input={"some": "payload"} - ) - +@pytest.fixture(autouse=True) +def conn(): + with patch.object(github_mod, "_get_conn", return_value=MagicMock()): + yield -def test__query_github_exception(): - import github as pygithub # pylint: disable=import-outside-toplevel - requester = MagicMock() - requester.requestJson.side_effect = pygithub.GithubException(502, {"message": "bad gateway"}) - client = MagicMock(requester=requester) +def test_get_issue_without_id_returns_empty(): + with patch.object(gh_client, "query", return_value={"message": "no id here"}): + assert not github_mod.get_issue(1, repo_name="repo") - with patch.object(github_mod, "_get_client", return_value=client): - ret = github_mod._query("test_ruleset", action="repos") - assert ret["status"] == 502 - assert "bad gateway" in ret["error"] +def test_get_milestone_without_id_returns_empty(): + with patch.object(gh_client, "query", return_value={"message": "no id here"}): + assert not github_mod.get_milestone(1, repo_name="repo") -def test__query_unexpected_exception(): - requester = MagicMock() - requester.requestJson.side_effect = OSError("connection refused") - client = MagicMock(requester=requester) +def test__get_config_value_renders_sdb_uris(): + # This matters for pillar-sourced profiles, which are not rendered + # by Salt automatically, unlike values in the minion configuration. + def fake_sdb_get(value, *args, **kwargs): # pylint: disable=unused-argument + if isinstance(value, str) and value.startswith("sdb://"): + return "rendered-token" + return value - with patch.object(github_mod, "_get_client", return_value=client): - with pytest.raises(CommandExecutionError, match="connection refused"): - github_mod._query("test_ruleset", action="repos") + with patch.object(config_mod.sdb, "sdb_get", side_effect=fake_sdb_get): + assert github_mod._get_config_value("sdb_profile", "token") == "rendered-token" -def test__check_ruleset_params(): +def test__resolve_ruleset_params(): ## test no valid config with pytest.raises(CommandExecutionError): - github_mod._check_ruleset_params(profile="test_ruleset") + github_mod._resolve_ruleset_params(profile="test_ruleset") # test cli config kwargs = {"ruleset_type": "repo", "repo_name": "mock_repo_name", "owner": "mock_owner"} - ret = github_mod._check_ruleset_params(profile="test_ruleset", **kwargs) - assert ret == ( - ["repos", "mock_owner", "mock_repo_name", "rulesets"], - {}, - { - "ruleset_type": "repo", - "repo_name": "mock_repo_name", - "owner": "mock_owner", - }, - ) + ret = github_mod._resolve_ruleset_params(profile="test_ruleset", **kwargs) + assert ret == { + "ruleset_type": "repo", + "repo_name": "mock_repo_name", + "owner": "mock_owner", + "headers": {}, + } def test_get_ruleset(params): params["ruleset_id"] = 1 # test mock 200 status - mock_query = {"dict": {"id": "1"}, "status": 200} - expected = {"id": "1"} - - with patch.object(github_mod, "_query", return_value=mock_query): - assert github_mod.get_ruleset(profile="test_ruleset", **params) == expected + with patch.object(gh_client, "query", return_value={"id": "1"}): + assert github_mod.get_ruleset(profile="test_ruleset", **params) == {"id": "1"} # test mock 404 error - mock_query = {"error": "404 not found", "status": 404} expected = { "comment": "GitHub Response Status Code: 404 not found", "error": "404 not found", @@ -109,12 +95,12 @@ def test_get_ruleset(params): "result": False, } - with patch.object(github_mod, "_query", return_value=mock_query): + with patch.object(gh_client, "query", side_effect=GitHubApiError("404 not found", status=404)): assert github_mod.get_ruleset(profile="test_ruleset", **params) == expected - # test mock empty return + # test unexpected error expected = {"comment": "Error getting ruleset", "result": False} - with patch.object(github_mod, "_query", side_effect=CommandExecutionError): + with patch.object(gh_client, "query", side_effect=CommandExecutionError): assert github_mod.get_ruleset(profile="test_ruleset", **params) == expected @@ -122,11 +108,9 @@ def test_add_ruleset(params): params["ruleset_params"] = {"name": "mock_name", "enforcement": "disabled"} # test mock 201 status - mock_query = {"dict": {"name": "mock_name", "enforcement": "disabled"}, "status": 201} - expected = {"name": "mock_name", "enforcement": "disabled"} - with patch.object(github_mod, "_query", return_value=mock_query): + with patch.object(gh_client, "query", return_value=expected): assert github_mod.add_ruleset(profile="test_ruleset", **params) == expected # test mock 404 error @@ -137,38 +121,38 @@ def test_add_ruleset(params): "result": False, } - with patch.object(github_mod, "_query", return_value={"error": "404 not found", "status": 404}): + with patch.object(gh_client, "query", side_effect=GitHubApiError("404 not found", status=404)): assert github_mod.add_ruleset(profile="test_ruleset", **params) == expected - # test mock empty return + # test unexpected error expected = {"comment": "Error adding ruleset", "result": False} - with patch.object(github_mod, "_query", side_effect=CommandExecutionError): + with patch.object(gh_client, "query", side_effect=CommandExecutionError): assert github_mod.add_ruleset(profile="test_ruleset", **params) == expected -def test_list_rulesets(params): +def test_add_ruleset_requires_name_and_enforcement(params): + params["ruleset_params"] = {"name": "mock_name"} - # test mock 200 status - mock_query = { - "dict": [{"name": "mock_ruleset_1"}, {"name": "mock_ruleset_2"}], - "status": 200, - } + with pytest.raises(CommandExecutionError, match="enforcement"): + github_mod.add_ruleset(profile="test_ruleset", **params) - expected = {"rulesets": [{"name": "mock_ruleset_1"}, {"name": "mock_ruleset_2"}]} - with patch.object(github_mod, "_query", return_value=mock_query): - assert github_mod.list_rulesets(profile="test_ruleset", **params) == expected +def test_list_rulesets(params): - # test mock 200 status, no rulesets returned - mock_query = {"dict": [], "status": 200} - expected = {"rulesets": None} + # test mock 200 status + with patch.object( + gh_client, "query", return_value=[{"name": "mock_ruleset_1"}, {"name": "mock_ruleset_2"}] + ): + assert github_mod.list_rulesets(profile="test_ruleset", **params) == { + "rulesets": [{"name": "mock_ruleset_1"}, {"name": "mock_ruleset_2"}] + } - with patch.object(github_mod, "_query", return_value=mock_query): - assert github_mod.list_rulesets(profile="test_ruleset", **params) == expected + # test mock 200 status, no rulesets returned + with patch.object(gh_client, "query", return_value=[]): + assert github_mod.list_rulesets(profile="test_ruleset", **params) == {"rulesets": None} # test mock 404 error - mock_query = {"error": "404 not found", "status": 404} expected = { "comment": "GitHub Response Status Code: 404 not found", "error": "404 not found", @@ -176,12 +160,12 @@ def test_list_rulesets(params): "result": False, } - with patch.object(github_mod, "_query", return_value=mock_query): + with patch.object(gh_client, "query", side_effect=GitHubApiError("404 not found", status=404)): assert github_mod.list_rulesets(profile="test_ruleset", **params) == expected - # test mock empty return + # test unexpected error expected = {"comment": "Error getting rulesets", "result": False} - with patch.object(github_mod, "_query", side_effect=CommandExecutionError): + with patch.object(gh_client, "query", side_effect=CommandExecutionError): assert github_mod.list_rulesets(profile="test_ruleset", **params) == expected @@ -190,15 +174,12 @@ def test_update_ruleset(params): params["ruleset_id"] = 1 # test mock 200 status - mock_query = {"dict": {"name": "mock_name", "enforcement": "disabled"}, "status": 200} - expected = {"name": "mock_name", "enforcement": "disabled"} - with patch.object(github_mod, "_query", return_value=mock_query): + with patch.object(gh_client, "query", return_value=expected): assert github_mod.update_ruleset(profile="test_ruleset", **params) == expected # test mock 404 error - mock_query = {"error": "404 not found", "status": 404} expected = { "comment": "GitHub Response Status Code: 404 not found", "error": "404 not found", @@ -206,12 +187,12 @@ def test_update_ruleset(params): "result": False, } - with patch.object(github_mod, "_query", return_value=mock_query): + with patch.object(gh_client, "query", side_effect=GitHubApiError("404 not found", status=404)): assert github_mod.update_ruleset(profile="test_ruleset", **params) == expected - # test mock empty return + # test unexpected error expected = {"comment": "Error updating ruleset", "result": False} - with patch.object(github_mod, "_query", side_effect=CommandExecutionError): + with patch.object(gh_client, "query", side_effect=CommandExecutionError): assert github_mod.update_ruleset(profile="test_ruleset", **params) == expected @@ -219,14 +200,12 @@ def test_delete_ruleset(params): params["ruleset_id"] = 1 # test mock 204 status - mock_query = {"status": 204} - expected = {"comment": "ruleset 1 successfully deleted", "status": 204} + expected = {"comment": "ruleset 1 successfully deleted"} - with patch.object(github_mod, "_query", return_value=mock_query): + with patch.object(gh_client, "query", return_value=None): assert github_mod.delete_ruleset(profile="test_ruleset", **params) == expected # test mock 404 error - mock_query = {"error": "404 not found", "status": 404} expected = { "comment": "GitHub Response Status Code: 404 not found", "error": "404 not found", @@ -234,10 +213,21 @@ def test_delete_ruleset(params): "result": False, } - with patch.object(github_mod, "_query", return_value=mock_query): + with patch.object(gh_client, "query", side_effect=GitHubApiError("404 not found", status=404)): assert github_mod.delete_ruleset(profile="test_ruleset", **params) == expected - # test mock empty return + # test unexpected error expected = {"comment": "Error deleting ruleset", "result": False} - with patch.object(github_mod, "_query", side_effect=CommandExecutionError): + with patch.object(gh_client, "query", side_effect=CommandExecutionError): assert github_mod.delete_ruleset(profile="test_ruleset", **params) == expected + + +def test_ruleset_error_ret_includes_response_data(): + err = GitHubApiError("422 invalid", status=422, data={"message": "invalid"}) + assert github_mod._ruleset_error_ret(err) == { + "status": 422, + "error": "422 invalid", + "dict": {"message": "invalid"}, + "comment": "GitHub Response Status Code: 422 invalid", + "result": False, + } diff --git a/tests/unit/states/test_github.py b/tests/unit/states/test_github.py index 4427bc5c..c1ac2d21 100644 --- a/tests/unit/states/test_github.py +++ b/tests/unit/states/test_github.py @@ -1,7 +1,6 @@ from unittest import mock import pytest -import salt.modules.config as config_module from saltext.github.states import github as github_mod @@ -19,10 +18,6 @@ def configure_loader_modules(): }, "__opts__": {"test": False}, }, - config_module: { - "__salt__": {"config.option": config_module.option}, - "__opts__": {"test_ruleset": {"ruleset_type": "mocked_value"}}, - }, } diff --git a/tests/unit/utils/__init__.py b/tests/unit/utils/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/tests/unit/utils/test_client.py b/tests/unit/utils/test_client.py new file mode 100644 index 00000000..63257be5 --- /dev/null +++ b/tests/unit/utils/test_client.py @@ -0,0 +1,86 @@ +from unittest.mock import MagicMock + +import github as pygithub +import pytest +from salt.exceptions import CommandExecutionError + +from saltext.github.utils.github import client as gh_client +from saltext.github.utils.github.exceptions import GitHubApiError +from saltext.github.utils.github.exceptions import GitHubNotFoundError + + +@pytest.fixture +def requester(): + return MagicMock() + + +@pytest.fixture +def conn(requester): + return MagicMock(client=MagicMock(requester=requester)) + + +def test_query_decodes_json_string_data(conn, requester): + requester.requestJson.return_value = (200, {}, '{"written": true}') + + ret = gh_client.query(conn, "repos", method="POST", data='{"some": "payload"}') + + assert ret == {"written": True} + requester.requestJson.assert_called_once_with( + "POST", "/repos", parameters=None, headers=None, input={"some": "payload"} + ) + + +def test_query_merges_paginated_results(conn, requester): + requester.requestJson.side_effect = ( + (200, {"Link": '; rel="next"'}, '[{"id": 1}]'), + (200, {}, '[{"id": 2}]'), + ) + + ret = gh_client.query(conn, ["repos"]) + + assert ret == [{"id": 1}, {"id": 2}] + assert requester.requestJson.call_args.kwargs["parameters"] == {"page": "2"} + + +def test_query_github_exception(conn, requester): + requester.requestJson.side_effect = pygithub.GithubException(502, {"message": "bad gateway"}) + + with pytest.raises(GitHubApiError, match="bad gateway") as exc: + gh_client.query(conn, "repos") + + assert exc.value.status == 502 + + +def test_query_error_response(conn, requester): + requester.requestJson.return_value = (404, {}, '{"message": "Not Found"}') + + with pytest.raises(GitHubNotFoundError, match="404 Not Found") as exc: + gh_client.query(conn, "repos") + + assert exc.value.status == 404 + assert exc.value.data == {"message": "Not Found"} + + +def test_query_unexpected_exception(conn, requester): + requester.requestJson.side_effect = OSError("connection refused") + + with pytest.raises(CommandExecutionError, match="connection refused"): + gh_client.query(conn, "repos") + + +def test_api_request_translates_pygithub_exceptions(): + @gh_client.api_request + def _not_found(): + raise pygithub.UnknownObjectException(404, {"message": "Not Found"}) + + @gh_client.api_request + def _server_error(): + raise pygithub.GithubException(502, {"message": "bad gateway"}) + + with pytest.raises(GitHubNotFoundError, match="404 Not Found") as exc: + _not_found() + assert exc.value.status == 404 + + with pytest.raises(GitHubApiError, match="502 bad gateway") as exc: + _server_error() + assert exc.value.status == 502 diff --git a/tests/unit/utils/test_rulesets.py b/tests/unit/utils/test_rulesets.py new file mode 100644 index 00000000..ad5d4ee8 --- /dev/null +++ b/tests/unit/utils/test_rulesets.py @@ -0,0 +1,23 @@ +import pytest +from salt.exceptions import CommandExecutionError + +from saltext.github.utils.github import rulesets as gh_rulesets + + +def test_build_path_repo(): + assert gh_rulesets.build_path("repo", owner="me", repo_name="repo", ruleset_id=1) == [ + "repos", + "me", + "repo", + "rulesets", + 1, + ] + + +def test_build_path_org(): + assert gh_rulesets.build_path("org", org_name="testorg") == ["orgs", "testorg", "rulesets"] + + +def test_build_path_invalid_type(): + with pytest.raises(CommandExecutionError, match="Invalid ruleset type"): + gh_rulesets.build_path("bogus")