diff --git a/.copier-answers.yml b/.copier-answers.yml index 87b7edfa..938bf017 100644 --- a/.copier-answers.yml +++ b/.copier-answers.yml @@ -22,7 +22,7 @@ os_support: package_name: github project_name: github python_requires: '3.10' -relax_pylint: true +relax_pylint: false salt_version: '3006' source_url: https://github.com/salt-extensions/saltext-github ssh_fixtures: false diff --git a/.pylintrc b/.pylintrc index 0eb642b7..f7efcb35 100755 --- a/.pylintrc +++ b/.pylintrc @@ -102,7 +102,10 @@ recursive=no # source root is an absolute path or a path relative to the current working # directory used to determine a package namespace for modules located under the # source root. -source-roots= +# Resolve the saltext namespace package correctly. Without this, pylint +# considers `github` (i.e. saltext.github) a top-level package, which +# shadows PyGithub's module during static analysis (=> no-member errors). +source-roots=src # Allow loading of arbitrary C extensions. Extensions are imported into the # active Python interpreter and may run arbitrary code. @@ -433,32 +436,17 @@ confidence=HIGH, # --enable=similarities". If you want to run only the classes checker, but have # no Warning level messages displayed, use "--disable=all --enable=classes # --disable=W". -disable=consider-using-f-string, - duplicate-code, +disable=duplicate-code, fixme, - inconsistent-return-statements, keyword-arg-before-vararg, line-too-long, logging-fstring-interpolation, missing-class-docstring, missing-function-docstring, missing-module-docstring, - no-else-return, - no-member, protected-access, - redefined-argument-from-local, - redefined-builtin, - redefined-outer-name, too-few-public-methods, - too-many-boolean-expressions, - too-many-branches, - too-many-instance-attributes, - too-many-lines, - too-many-locals, - too-many-nested-blocks, too-many-positional-arguments, - too-many-return-statements, - too-many-statements, ungrouped-imports, wrong-import-position diff --git a/changelog/+add_team_profile.fixed.md b/changelog/+add_team_profile.fixed.md new file mode 100644 index 00000000..7c367a47 --- /dev/null +++ b/changelog/+add_team_profile.fixed.md @@ -0,0 +1 @@ +Fixed `add_team` refreshing the team cache using the default profile instead of the configured one diff --git a/changelog/+api_url.added.md b/changelog/+api_url.added.md new file mode 100644 index 00000000..06a447ba --- /dev/null +++ b/changelog/+api_url.added.md @@ -0,0 +1 @@ +Added the optional `api_url` profile option for targeting GitHub Enterprise Server instances diff --git a/changelog/+is_team_member_missing_team.fixed.md b/changelog/+is_team_member_missing_team.fixed.md new file mode 100644 index 00000000..9e1a19b3 --- /dev/null +++ b/changelog/+is_team_member_missing_team.fixed.md @@ -0,0 +1 @@ +Fixed ``is_team_member`` crashing with a ``TypeError`` when the team does not exist. It now returns ``False``. diff --git a/changelog/+list_funcs_error_crash.fixed.md b/changelog/+list_funcs_error_crash.fixed.md new file mode 100644 index 00000000..217e6ee6 --- /dev/null +++ b/changelog/+list_funcs_error_crash.fixed.md @@ -0,0 +1 @@ +Fixed ``get_issues``, ``get_issue_comments``, ``get_prs`` and ``get_milestones`` crashing with an ``AttributeError`` when the API returned an error. They now raise a ``CommandExecutionError`` carrying the API error message. diff --git a/changelog/+milestone_pops.fixed.md b/changelog/+milestone_pops.fixed.md new file mode 100644 index 00000000..0c5c7f17 --- /dev/null +++ b/changelog/+milestone_pops.fixed.md @@ -0,0 +1 @@ +Fixed `get_milestones`/`get_milestone` crashing on API payloads missing optional keys diff --git a/changelog/+pygithub_auth.fixed.md b/changelog/+pygithub_auth.fixed.md new file mode 100644 index 00000000..84218ff5 --- /dev/null +++ b/changelog/+pygithub_auth.fixed.md @@ -0,0 +1 @@ +Fixed a PyGithub deprecation warning by migrating to `github.Auth.Token` authentication diff --git a/changelog/+pygithub_public_api.changed.md b/changelog/+pygithub_public_api.changed.md new file mode 100644 index 00000000..04b0e752 --- /dev/null +++ b/changelog/+pygithub_public_api.changed.md @@ -0,0 +1 @@ +The execution module now exclusively relies on public PyGithub APIs instead of internal ones (``_requester``, ``_identity``, ``_rawData``, manual ``PaginatedList`` construction). It also avoids deprecated API paths (``/teams/{team_id}/...``) by addressing teams by slug and checking team memberships via the ``memberships`` endpoints. The minimum supported PyGithub version is now 2.2. diff --git a/changelog/+remove_repo_missing.fixed.md b/changelog/+remove_repo_missing.fixed.md new file mode 100644 index 00000000..18ca145c --- /dev/null +++ b/changelog/+remove_repo_missing.fixed.md @@ -0,0 +1 @@ +Fixed `remove_repo` raising an exception instead of returning `False` for nonexistent repositories diff --git a/changelog/+repo_present_profile.fixed.md b/changelog/+repo_present_profile.fixed.md new file mode 100644 index 00000000..39a95a33 --- /dev/null +++ b/changelog/+repo_present_profile.fixed.md @@ -0,0 +1 @@ +Fixed `repo_present` not passing the configured profile when creating repositories diff --git a/changelog/+repo_present_team_add_comment.fixed.md b/changelog/+repo_present_team_add_comment.fixed.md new file mode 100644 index 00000000..0177a165 --- /dev/null +++ b/changelog/+repo_present_team_add_comment.fixed.md @@ -0,0 +1 @@ +Fixed ``repo_present`` reporting a failed team *removal* when *adding* a repository to a team failed. diff --git a/changelog/+ruleset_absent_delete_failure.fixed.md b/changelog/+ruleset_absent_delete_failure.fixed.md new file mode 100644 index 00000000..110a0971 --- /dev/null +++ b/changelog/+ruleset_absent_delete_failure.fixed.md @@ -0,0 +1 @@ +Fixed ``ruleset_absent`` reporting success when the ruleset could not be deleted. The module function returns a truthy error dict on failure, which was mistaken for success, and the failure result was subsequently overwritten by the "does not exist" fallback. diff --git a/changelog/+ruleset_states.fixed.md b/changelog/+ruleset_states.fixed.md new file mode 100644 index 00000000..3257489a --- /dev/null +++ b/changelog/+ruleset_states.fixed.md @@ -0,0 +1 @@ +Fixed `ruleset_present` drift detection, which compared server-populated fields and iterated the wrong parameter set, and made the ruleset states report forecast changes in test mode diff --git a/changelog/+single_http_stack.changed.md b/changelog/+single_http_stack.changed.md new file mode 100644 index 00000000..b9997402 --- /dev/null +++ b/changelog/+single_http_stack.changed.md @@ -0,0 +1 @@ +All API calls are now routed through the PyGithub client, honoring the configured `api_url`, authentication and retry handling uniformly diff --git a/changelog/+team_present_members.fixed.md b/changelog/+team_present_members.fixed.md new file mode 100644 index 00000000..500e6401 --- /dev/null +++ b/changelog/+team_present_members.fixed.md @@ -0,0 +1 @@ +Fixed `team_present` crashing when managing team members diff --git a/changelog/+testmode_projected_changes.fixed.md b/changelog/+testmode_projected_changes.fixed.md new file mode 100644 index 00000000..4ce05fee --- /dev/null +++ b/changelog/+testmode_projected_changes.fixed.md @@ -0,0 +1 @@ +All states now report the projected changes in test mode, matching the changes reported on actual application. Exception: ``team_present`` does not project member/repo changes for teams that do not exist yet. diff --git a/docs/ref/utils/index.rst b/docs/ref/utils/index.rst index d9d5f154..ec1e0af1 100644 --- a/docs/ref/utils/index.rst +++ b/docs/ref/utils/index.rst @@ -4,9 +4,10 @@ _________ Utilities _________ -.. currentmodule:: saltext.github.utils +.. currentmodule:: saltext.github .. autosummary:: :toctree: - github + utils + utils.github diff --git a/docs/ref/utils/saltext.github.utils.rst b/docs/ref/utils/saltext.github.utils.rst new file mode 100644 index 00000000..95c09c96 --- /dev/null +++ b/docs/ref/utils/saltext.github.utils.rst @@ -0,0 +1,5 @@ +saltext.github.utils +==================== + +.. automodule:: saltext.github.utils + :members: diff --git a/noxfile.py b/noxfile.py index 2ac3d3f7..1ea02115 100755 --- a/noxfile.py +++ b/noxfile.py @@ -376,7 +376,7 @@ def lint_tests(session): Run PyLint against the test suite. Set PYLINT_REPORT to a path to capture output. """ flags = [ - "--disable=I,redefined-outer-name,no-member,missing-module-docstring,missing-function-docstring,missing-class-docstring,attribute-defined-outside-init,inconsistent-return-statements,too-few-public-methods,too-many-public-methods,unused-argument", + "--disable=I,redefined-outer-name,no-member,missing-module-docstring,missing-function-docstring,missing-class-docstring,attribute-defined-outside-init,inconsistent-return-statements,too-few-public-methods,too-many-public-methods", ] if session.posargs: paths = session.posargs @@ -404,7 +404,7 @@ def lint_tests_pre_commit(session): Run PyLint against the code and the test suite. Set PYLINT_REPORT to a path to capture output. """ flags = [ - "--disable=I,redefined-outer-name,no-member,missing-module-docstring,missing-function-docstring,missing-class-docstring,attribute-defined-outside-init,inconsistent-return-statements,too-few-public-methods,too-many-public-methods,unused-argument", + "--disable=I,redefined-outer-name,no-member,missing-module-docstring,missing-function-docstring,missing-class-docstring,attribute-defined-outside-init,inconsistent-return-statements,too-few-public-methods,too-many-public-methods", ] if session.posargs: paths = session.posargs diff --git a/pyproject.toml b/pyproject.toml index f58004e0..8e366c7a 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -38,7 +38,7 @@ requires-python = ">= 3.10" dynamic = ["version"] dependencies = [ "salt>=3006", - "PyGithub", + "PyGithub>=2.2", ] [project.readme] diff --git a/src/saltext/github/modules/github.py b/src/saltext/github/modules/github.py index bb25e428..56f7b927 100644 --- a/src/saltext/github/modules/github.py +++ b/src/saltext/github/modules/github.py @@ -1,8 +1,6 @@ """ Module for interacting with the GitHub v3 API. -.. versionadded:: 2016.3.0 - :depends: PyGithub python module Configuration @@ -10,7 +8,12 @@ Configure this module by specifying the name of a configuration profile in the minion config, minion pillar, or master config. The module -will use the 'github' key by default, if defined. +will use the ``github`` key by default, if defined. + +.. versionadded:: 1.1.0 + + Added the ``api_url`` configuration option to override the API endpoint + we run against. Can be used for GitHub Enterprise instances. For example: @@ -27,28 +30,33 @@ # optional: it can be dangerous to change the privacy of a repository # in an automated way. set this to True to allow privacy modifications allow_repo_privacy_changes: False + + # optional: the base URL of the GitHub API. Set this when targeting + # a GitHub Enterprise Server instance, e.g. + # https://github.example.com/api/v3 + api_url: https://api.github.com """ +import json import logging +import re -import salt.utils.http -import salt.utils.json from salt.exceptions import CommandExecutionError +from saltext.github.utils import filter_unset + try: - # pylint: disable=no-name-in-module - import github - import github.NamedUser - import github.PaginatedList + import github # pylint: disable=import-self from github.GithubException import UnknownObjectException - # pylint: enable=no-name-in-module HAS_LIBS = True -except ImportError: +except ImportError: # pragma: no cover HAS_LIBS = False log = logging.getLogger(__name__) +API_URL_DEFAULT = "https://api.github.com" + __virtualname__ = "github" @@ -58,7 +66,7 @@ def __virtual__(): """ if HAS_LIBS: return __virtualname__ - return ( + return ( # pragma: no cover False, "The github execution module cannot be loaded: PyGithub library is not installed.", ) @@ -75,13 +83,15 @@ def _get_config_value(profile, config_name): config_name The configuration item's name to use to return configuration values. """ - config = __salt__["config.option"](profile) - if not config: - raise CommandExecutionError( - f"Authentication information could not be found for the '{profile}' profile." - ) + if "gh_profile" not in __context__ or profile not in __context__["gh_profile"]: + config = __salt__["config.option"](profile) + if not config: + raise CommandExecutionError( + f"Authentication information could not be found for the '{profile}' profile." + ) + __context__.setdefault("gh_profile", {})[profile] = config - config_value = config.get(config_name) + config_value = __context__["gh_profile"][profile].get(config_name) if config_value is None: raise CommandExecutionError( f"The '{config_name}' parameter was not found in the '{profile}' profile." @@ -90,53 +100,44 @@ def _get_config_value(profile, config_name): return config_value -def _get_client(profile): +def _get_api_url(profile): + """ + Return the configured GitHub API base URL, defaulting to the public one. + """ + try: + return _get_config_value(profile, "api_url").rstrip("/") + except CommandExecutionError: # pragma: no cover + return API_URL_DEFAULT + + +def _get_client(profile) -> github.Github: """ Return the GitHub client, cached into __context__ for performance """ token = _get_config_value(profile, "token") - key = "github.{}:{}".format( # pylint: disable=consider-using-f-string - token, _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 ) if key not in __context__: - __context__[key] = github.Github(token, per_page=100) + __context__[key] = github.Github( + auth=github.Auth.Token(token), base_url=api_url, per_page=100 + ) return __context__[key] -def _get_members(organization, params=None): - return github.PaginatedList.PaginatedList( - github.NamedUser.NamedUser, - organization._requester, - organization.url + "/members", - params, - ) - - -def _get_repos(profile, params=None, ignore_cache=False): - # Use cache when no params are given +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 or params is not None: - org_name = _get_config_value(profile, "org_name") + if key not in __context__ or ignore_cache: client = _get_client(profile) organization = client.get_organization(org_name) - result = github.PaginatedList.PaginatedList( - github.Repository.Repository, - organization._requester, - organization.url + "/repos", - params, - ) - - # Only cache results if no params were given (full scan) - if params is not None: - return result - next_result = [] - for repo in result: + for repo in organization.get_repos(): next_result.append(repo) # Cache a copy of each repo for single lookups @@ -158,8 +159,6 @@ def list_users(profile="github", ignore_cache=False): ignore_cache Bypasses the use of cached users. - .. versionadded:: 2016.11.0 - CLI Example: .. code-block:: bash @@ -172,7 +171,7 @@ def list_users(profile="github", ignore_cache=False): 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 _get_members(organization, None)] + __context__[key] = [member.login for member in organization.get_members()] return __context__[key] @@ -198,7 +197,6 @@ def get_user(name, profile="github", user_details=False): salt myminion github.get_user github-handle salt myminion github.get_user github-handle user_details=true - """ if not user_details and name in list_users(profile): @@ -226,16 +224,14 @@ def get_user(name, profile="github", user_details=False): response["url"] = user.url try: - _, data = organization._requester.requestJsonAndCheck( - "GET", organization.url + "/memberships/" + user._identity - ) + 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"] = data.get("state") + response["membership_state"] = membership.state return response @@ -266,8 +262,9 @@ def add_user(name, profile="github"): log.exception("Resource not found") return False - _, data = organization._requester.requestJsonAndCheck( - "PUT", organization.url + "/memberships/" + github_named_user._identity + # 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" @@ -309,8 +306,6 @@ def get_issue(issue_number, repo_name=None, profile="github", output="min"): """ Return information about a single issue in a named repository. - .. versionadded:: 2016.11.0 - issue_number The number of the issue to retrieve. @@ -359,8 +354,6 @@ def get_issue_comments(issue_number, repo_name=None, profile="github", since=Non """ Return information about the comments for a given issue in a named repository. - .. versionadded:: 2016.11.0 - issue_number The number of the issue for which to retrieve comments. @@ -400,6 +393,8 @@ def get_issue_comments(issue_number, repo_name=None, profile="github", since=Non args["since"] = since comments = _query(profile, action=action, command=command, args=args) + if comments.get("error"): + raise CommandExecutionError(comments["error"]) ret = {} for comment in comments.get("dict", {}): @@ -434,8 +429,6 @@ def get_issues( """ Returns information for all issues in a given repository, based on the search options. - .. versionadded:: 2016.11.0 - repo_name The name of the repository for which to list issues. This argument is required, either passed via the CLI, or defined in the configured @@ -534,6 +527,8 @@ def get_issues( 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 @@ -561,8 +556,6 @@ def get_milestones( """ Return information about milestones for a given repository. - .. versionadded:: 2016.11.0 - repo_name The name of the repository for which to list issues. This argument is required, either passed via the CLI, or defined in the configured @@ -597,7 +590,6 @@ def get_milestones( .. code-block:: bash salt myminion github.get_milestones - """ org_name = _get_config_value(profile, "org_name") if repo_name is None: @@ -619,15 +611,17 @@ def get_milestones( ret = {} milestones = _query(profile, action=action, command="milestones", args=args) + if milestones.get("error"): + raise CommandExecutionError(milestones["error"]) for milestone in milestones.get("dict", {}): milestone_id = milestone.get("id") if output == "full": ret[milestone_id] = milestone else: - milestone.pop("creator") - milestone.pop("html_url") - milestone.pop("labels_url") + milestone.pop("creator", None) + milestone.pop("html_url", None) + milestone.pop("labels_url", None) ret[milestone_id] = milestone return ret @@ -637,8 +631,6 @@ def get_milestone(number=None, name=None, repo_name=None, profile="github", outp """ Return information about a single milestone in a named repository. - .. versionadded:: 2016.11.0 - number The number of the milestone to retrieve. If provided, this option will be favored over ``name``. @@ -665,7 +657,6 @@ def get_milestone(number=None, name=None, repo_name=None, profile="github", outp salt myminion github.get_milestone 72 salt myminion github.get_milestone name=my_milestone - """ ret = {} @@ -686,9 +677,9 @@ def get_milestone(number=None, name=None, repo_name=None, profile="github", outp if output.lower() == "full": ret[milestone_id] = milestone_data else: - milestone_data.pop("creator") - milestone_data.pop("html_url") - milestone_data.pop("labels_url") + milestone_data.pop("creator", None) + milestone_data.pop("html_url", None) + milestone_data.pop("labels_url", None) ret[milestone_id] = milestone_data return ret @@ -732,8 +723,6 @@ def get_repo_info(repo_name, profile="github", ignore_cache=False): """ Return information for a given repo. - .. versionadded:: 2016.11.0 - repo_name The name of the repository. @@ -756,14 +745,10 @@ def get_repo_info(repo_name, profile="github", ignore_cache=False): 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])) - if not repo: - return {} - - # client.get_repo can return a github.Repository.Repository object, - # even if the repo is invalid. We need to catch the exception when - # we try to perform actions on the repo object, rather than above - # the if statement. ret = _repo_to_dict(repo) __context__[key] = ret @@ -778,8 +763,6 @@ def get_repo_teams(repo_name, profile="github"): """ Return teams belonging to a repository. - .. versionadded:: 2017.7.0 - repo_name The name of the repository from which to retrieve teams. @@ -807,7 +790,7 @@ def get_repo_teams(repo_name, profile="github"): teams = repo.get_teams() for team in teams: ret.append({"id": team.id, "name": team.name, "permission": team.permission}) - except github.UnknownObjectException as exc: + 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 @@ -820,8 +803,6 @@ def list_repos(profile="github"): repositories within the organization Dependent upon the access rights of the profile token. - .. versionadded:: 2016.11.0 - profile The name of the profile configuration to use. Defaults to ``github``. @@ -840,8 +821,6 @@ def list_private_repos(profile="github"): List private repositories within the organization. Dependent upon the access rights of the profile token. - .. versionadded:: 2016.11.0 - profile The name of the profile configuration to use. Defaults to ``github``. @@ -863,8 +842,6 @@ def list_public_repos(profile="github"): """ List public repositories within the organization. - .. versionadded:: 2016.11.0 - profile The name of the profile configuration to use. Defaults to ``github``. @@ -937,31 +914,24 @@ def add_repo( .. code-block:: bash salt myminion github.add_repo 'repo_name' - - .. versionadded:: 2016.11.0 """ try: client = _get_client(profile) organization = client.get_organization(_get_config_value(profile, "org_name")) - given_params = { - "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, - } - parameters = {"name": name} - for param_name, param_value in given_params.items(): - if param_value is not None: - parameters[param_name] = param_value - - organization._requester.requestJsonAndCheck( - "POST", organization.url + "/repos", input=parameters + 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, + } ) + organization.create_repo(name, **parameters) return True except github.GithubException: log.exception("Error creating a repo") @@ -1011,8 +981,6 @@ def edit_repo( .. code-block:: bash salt myminion github.add_repo 'repo_name' - - .. versionadded:: 2016.11.0 """ try: @@ -1029,21 +997,17 @@ def edit_repo( client = _get_client(profile) organization = client.get_organization(_get_config_value(profile, "org_name")) repo = organization.get_repo(name) - - given_params = { - "description": description, - "homepage": homepage, - "private": private, - "has_issues": has_issues, - "has_wiki": has_wiki, - "has_downloads": has_downloads, - } - parameters = {"name": name} - for param_name, param_value in given_params.items(): - if param_value is not None: - parameters[param_name] = param_value - - organization._requester.requestJsonAndCheck("PATCH", repo.url, input=parameters) + 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_repo_info(name, profile=profile, ignore_cache=True) # Refresh cache return True except github.GithubException: @@ -1066,11 +1030,10 @@ def remove_repo(name, profile="github"): .. code-block:: bash salt myminion github.remove_repo 'my-repo' - - .. versionadded:: 2016.11.0 """ - repo_info = get_repo_info(name, profile=profile) - if not repo_info: + try: + get_repo_info(name, profile=profile) + except CommandExecutionError: log.error("Repo %s to be removed does not exist.", name) return False try: @@ -1126,11 +1089,11 @@ def add_team( The names of repositories to add the team to. privacy - The level of privacy for the team, can be 'secret' or 'closed'. + The level of privacy for the team, can be ``secret`` or ``closed``. permission The default permission for new repositories added to the team, can be - 'pull', 'push' or 'admin'. + ``pull``, ``push`` or ``admin``. profile The name of the profile configuration to use. Defaults to ``github``. @@ -1140,28 +1103,25 @@ def add_team( .. code-block:: bash salt myminion github.add_team 'team_name' - - .. versionadded:: 2016.11.0 """ try: client = _get_client(profile) organization = client.get_organization(_get_config_value(profile, "org_name")) - parameters = {} - parameters["name"] = name - - if description is not None: - parameters["description"] = description - if repo_names is not None: - parameters["repo_names"] = repo_names - if permission is not None: - parameters["permission"] = permission - if privacy is not None: - parameters["privacy"] = privacy - - organization._requester.requestJsonAndCheck( - "POST", organization.url + "/teams", input=parameters + parameters = filter_unset( + { + "description": description, + "permission": permission, + "privacy": privacy, + } ) - list_teams(ignore_cache=True) # Refresh cache + 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") @@ -1179,11 +1139,11 @@ def edit_team(name, description=None, privacy=None, permission=None, profile="gi The description of the team. privacy - The level of privacy for the team, can be 'secret' or 'closed'. + The level of privacy for the team, can be ``secret`` or ``closed``. permission The default permission for new repositories added to the team, can be - 'pull', 'push' or 'admin'. + ``pull``, ``push`` or ``admin``. profile The name of the profile configuration to use. Defaults to ``github``. @@ -1193,8 +1153,6 @@ def edit_team(name, description=None, privacy=None, permission=None, profile="gi .. code-block:: bash salt myminion github.edit_team 'team_name' description='Team description' - - .. versionadded:: 2016.11.0 """ team = get_team(name, profile=profile) if not team: @@ -1203,19 +1161,17 @@ def edit_team(name, description=None, privacy=None, permission=None, profile="gi try: client = _get_client(profile) organization = client.get_organization(_get_config_value(profile, "org_name")) - team = organization.get_team(team["id"]) - - parameters = {} - if name is not None: - parameters["name"] = name - if description is not None: - parameters["description"] = description - if privacy is not None: - parameters["privacy"] = privacy - if permission is not None: - parameters["permission"] = permission - - team._requester.requestJsonAndCheck("PATCH", team.url, input=parameters) + team = organization.get_team_by_slug(team["slug"]) + team.edit( + name, + **filter_unset( + { + "description": description, + "privacy": privacy, + "permission": permission, + } + ), + ) return True except UnknownObjectException: log.exception("Resource not found") @@ -1237,8 +1193,6 @@ def remove_team(name, profile="github"): .. code-block:: bash salt myminion github.remove_team 'team_name' - - .. versionadded:: 2016.11.0 """ team_info = get_team(name, profile=profile) if not team_info: @@ -1247,7 +1201,7 @@ def remove_team(name, profile="github"): try: client = _get_client(profile) organization = client.get_organization(_get_config_value(profile, "org_name")) - team = organization.get_team(team_info["id"]) + 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: @@ -1274,8 +1228,6 @@ def list_team_repos(team_name, profile="github", ignore_cache=False): .. code-block:: bash salt myminion github.list_team_repos 'team_name' - - .. versionadded:: 2016.11.0 """ cached_team = get_team(team_name, profile=profile) if not cached_team: @@ -1284,14 +1236,15 @@ def list_team_repos(team_name, profile="github", ignore_cache=False): # Return from cache if available if cached_team.get("repos") and not ignore_cache: - return cached_team.get("repos") + return cached_team["repos"] try: client = _get_client(profile) organization = client.get_organization(_get_config_value(profile, "org_name")) - team = organization.get_team(cached_team["id"]) + 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(): @@ -1304,9 +1257,9 @@ def list_team_repos(team_name, profile="github", ignore_cache=False): repos[repo.name.lower()] = {"permission": permission} cached_team["repos"] = repos return repos - except UnknownObjectException: + except UnknownObjectException: # pragma: no cover log.exception("Resource not found: %s", cached_team["id"]) - return [] + return {} def add_team_repo(repo_name, team_name, profile="github", permission=None): @@ -1323,37 +1276,35 @@ def add_team_repo(repo_name, team_name, profile="github", permission=None): The name of the profile configuration to use. Defaults to ``github``. permission - The permission for team members within the repository, can be 'pull', - 'push' or 'admin'. If not specified, the default permission specified on + 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. - .. versionadded:: 2017.7.0 - CLI Example: .. code-block:: bash salt myminion github.add_team_repo 'my_repo' 'team_name' - - .. versionadded:: 2016.11.0 """ - team = get_team(team_name, profile=profile) - if not team: + team_info = get_team(team_name, profile=profile) + 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(team["id"]) + team = organization.get_team_by_slug(team_info["slug"]) repo = organization.get_repo(repo_name) except UnknownObjectException: - log.exception("Resource not found: %s", team["id"]) + 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 + ) return False - params = None - if permission is not None: - params = {"permission": permission} - - team._requester.requestJsonAndCheck("PUT", team.url + "/repos/" + repo._identity, input=params) # Try to refresh cache list_team_repos(team_name, profile=profile, ignore_cache=True) return True @@ -1377,20 +1328,18 @@ def remove_team_repo(repo_name, team_name, profile="github"): .. code-block:: bash salt myminion github.remove_team_repo 'my_repo' 'team_name' - - .. versionadded:: 2016.11.0 """ - team = get_team(team_name, profile=profile) - if not team: + team_info = get_team(team_name, profile=profile) + 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(team["id"]) + team = organization.get_team_by_slug(team_info["slug"]) repo = organization.get_repo(repo_name) except UnknownObjectException: - log.exception("Resource not found: %s", team["id"]) + log.exception("Resource not found: %s", team_info["id"]) return False team.remove_from_repos(repo) return repo_name not in list_team_repos(team_name, profile=profile, ignore_cache=True) @@ -1414,8 +1363,6 @@ def list_team_members(team_name, profile="github", ignore_cache=False): .. code-block:: bash salt myminion github.list_team_members 'team_name' - - .. versionadded:: 2016.11.0 """ cached_team = get_team(team_name, profile=profile) if not cached_team: @@ -1423,18 +1370,19 @@ def list_team_members(team_name, profile="github", ignore_cache=False): return False # Return from cache if available if cached_team.get("members") and not ignore_cache: - return cached_team.get("members") + return cached_team["members"] try: client = _get_client(profile) organization = client.get_organization(_get_config_value(profile, "org_name")) - team = organization.get_team(cached_team["id"]) + 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: + except UnknownObjectException: # pragma: no cover log.exception("Resource not found: %s", cached_team["id"]) return [] @@ -1454,8 +1402,6 @@ def list_members_without_mfa(profile="github", ignore_cache=False): .. code-block:: bash salt myminion github.list_members_without_mfa - - .. versionadded:: 2016.11.0 """ key = "github.{}:non_mfa_users".format( # pylint: disable=consider-using-f-string _get_config_value(profile, "org_name") @@ -1465,14 +1411,8 @@ def list_members_without_mfa(profile="github", ignore_cache=False): client = _get_client(profile) organization = client.get_organization(_get_config_value(profile, "org_name")) - filter_key = "filter" - # Silly hack to see if we're past PyGithub 1.26.0, where the name of - # the filter kwarg changed - if hasattr(github.Team.Team, "membership"): - filter_key = "filter_" - __context__[key] = [ - m.login.lower() for m in _get_members(organization, {filter_key: "2fa_disabled"}) + m.login.lower() for m in organization.get_members(filter_="2fa_disabled") ] return __context__[key] @@ -1496,10 +1436,9 @@ def is_team_member(name, team_name, profile="github"): .. code-block:: bash salt myminion github.is_team_member 'user_name' 'team_name' - - .. versionadded:: 2016.11.0 """ - return name.lower() in list_team_members(team_name, profile=profile) + # list_team_members returns False for missing teams + return name.lower() in (list_team_members(team_name, profile=profile) or []) def add_team_member(name, team_name, profile="github"): @@ -1520,31 +1459,22 @@ def add_team_member(name, team_name, profile="github"): .. code-block:: bash salt myminion github.add_team_member 'user_name' 'team_name' - - .. versionadded:: 2016.11.0 """ - team = get_team(team_name, profile=profile) - if not team: + team_info = get_team(team_name, profile=profile) + 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(team["id"]) + team = organization.get_team_by_slug(team_info["slug"]) member = client.get_user(name) except UnknownObjectException: - log.exception("Resource not found: %s", team["id"]) + log.exception("Resource not found: %s", team_info["id"]) return False try: - # Can't use team.add_membership due to this bug that hasn't made it into - # a PyGithub release yet https://github.com/PyGithub/PyGithub/issues/363 - team._requester.requestJsonAndCheck( - "PUT", - team.url + "/memberships/" + member._identity, - input={"role": "member"}, - parameters={"role": "member"}, - ) + team.add_membership(member, "member") except github.GithubException: log.exception("Error in adding a member to a team") return False @@ -1569,31 +1499,28 @@ def remove_team_member(name, team_name, profile="github"): .. code-block:: bash salt myminion github.remove_team_member 'user_name' 'team_name' - - .. versionadded:: 2016.11.0 """ - team = get_team(team_name, profile=profile) - if not team: + team_info = get_team(team_name, profile=profile) + 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(team["id"]) + team = organization.get_team_by_slug(team_info["slug"]) member = client.get_user(name) except UnknownObjectException: - log.exception("Resource not found: %s", team["id"]) + log.exception("Resource not found: %s", team_info["id"]) return False - if not hasattr(team, "remove_from_members"): - return ( - False, - "PyGithub 1.26.0 or greater is required for team management, please upgrade.", - ) - - team.remove_from_members(member) - return not team.has_in_members(member) + 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): @@ -1611,8 +1538,6 @@ def list_teams(profile="github", ignore_cache=False): .. code-block:: bash salt myminion github.list_teams - - .. versionadded:: 2016.11.0 """ key = "github.{}:teams".format( # pylint: disable=consider-using-f-string _get_config_value(profile, "org_name") @@ -1624,16 +1549,15 @@ def list_teams(profile="github", ignore_cache=False): teams_data = organization.get_teams() teams = {} for team in teams_data: - # Note that _rawData is used to access some properties here as they - # are not exposed in older versions of PyGithub. It's VERY important - # to use team._rawData instead of team.raw_data, as the latter forces - # an API call to retrieve team details again. + # 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._rawData["description"], + "description": team.description, "permission": team.permission, - "privacy": team._rawData["privacy"], + "privacy": team.privacy, } __context__[key] = teams @@ -1655,8 +1579,6 @@ def get_prs( Returns information for all pull requests in a given repository, based on the search options provided. - .. versionadded:: 2017.7.0 - repo_name The name of the repository for which to list pull requests. This argument is required, either passed via the CLI, or defined in the @@ -1730,6 +1652,8 @@ def get_prs( ret = {} prs = _query(profile, action=action, command="pulls", args=args) + if prs.get("error"): + raise CommandExecutionError(prs["error"]) for pr_ in prs.get("dict", {}): pr_id = pr_.get("id") @@ -1799,6 +1723,25 @@ def _format_issue(issue): 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, @@ -1807,152 +1750,146 @@ def _query( method="GET", header_dict=None, data=None, - url="https://api.github.com/", 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 = {} - if action: - url += action - - if command: - url += f"/{command}" + 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 URL: %s", url) + log.debug("GitHub API path: %s", url) - if header_dict is None: - header_dict = {} - - if not header_dict.get("Authorization"): - header_dict["Authorization"] = f"Bearer {_get_config_value(profile, 'token')}" - if per_page and "per_page" not in args.keys(): + if per_page and "per_page" not in args: args["per_page"] = per_page - header_dict["Accept"] = "application/vnd.github+json" + if isinstance(data, str): + data = json.loads(data) - decode = True - if method == "DELETE": - decode = False + requester = _get_client(profile).requester - # GitHub paginates all queries when returning many items. - # Gather all data using multiple queries and handle pagination. - ret = {} complete_result = [] - next_page = True - page_number = "" - while next_page is True: - if page_number: - args["page"] = page_number - result = salt.utils.http.query( - url, - method, - params=args, - data=data, - header_dict=header_dict, - decode=decode, - decode_type="json", - headers=True, - status=True, - text=True, - hide_fields=None, - opts=__opts__, - ) - log.debug("GitHub Response Status Code: %s", result.get("status")) - - if result.get("status") in [200, 201]: - - if isinstance(result["dict"], dict): - # 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 result - - ret = result - complete_result = complete_result + result["dict"] - ret["dict"] = complete_result - - else: - if result: - return result - raise CommandExecutionError - + next_page = None + while True: + if next_page: + args["page"] = next_page try: - link_info = result.get("headers").get("Link").split(",")[0] - except AttributeError: - # Only one page of data was returned; exit the loop. - next_page = False - continue - - if "next" in link_info: - # Get the 'next' page number from the Link header. - page_number = link_info.split(">")[0].split("&page=")[1] - else: - # Last page already processed; break the loop. - next_page = False + 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(profile, rule_id=False, rule_params=False, **kwargs): +def _check_ruleset_params( + profile, + require_id=False, + require_params=False, + *, + ruleset_type=None, + owner=None, + repo_name=None, + org_name=None, + header_dict=None, + ruleset_id=None, + ruleset_params=None, + **_, +): """ Helper function to check for param values on command line and in config file profile The name of the profile configuration to use. - ruleset_id - Defautls to false. If ruleset_id is required, set to True. - - ruleset_params - Defautls to false. If ruleset_params are required, set to True. + require_id + Require ruleset_id. Defaults to false. + 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. """ - if not kwargs.get("ruleset_type"): - kwargs["ruleset_type"] = _get_config_value(profile, "ruleset_type") - if kwargs["ruleset_type"] not in ["repo", "org"]: - raise CommandExecutionError - - if kwargs["ruleset_type"] == "repo": - if not kwargs.get("owner"): - kwargs["owner"] = _get_config_value(profile, "owner") - if not kwargs.get("repo_name"): - kwargs["repo_name"] = _get_config_value(profile, "repo_name") + ruleset_type = ruleset_type or _get_config_value(profile, "ruleset_type") + if ruleset_type not in ("repo", "org"): + raise CommandExecutionError(f"Invalid ruleset type: {ruleset_type}") - action = "/".join(["repos", kwargs["owner"], kwargs["repo_name"], "rulesets"]) + 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: - if not kwargs.get("org_name"): - kwargs["org_name"] = _get_config_value(profile, "org_name") - - action = "/".join(["orgs", kwargs["org_name"], "rulesets"]) + org_name = org_name or _get_config_value(profile, "org_name") + action = ["orgs", org_name, "rulesets"] # check for optional header_dict - if not kwargs.get("header_dict"): - try: - kwargs["header_dict"] = _get_config_value(profile, "header_dict") - except CommandExecutionError: - kwargs["header_dict"] = {} + try: + header_dict = header_dict or _get_config_value(profile, "header_dict") + except CommandExecutionError: + header_dict = {} - # verify header_dict is a dict - if not isinstance(kwargs["header_dict"], dict): - kwargs["header_dict"] = {} + # verify header_dict is a dict + if not isinstance(header_dict, dict): + header_dict = {} # if ruleset is required, check for ruleset_id - if rule_id: - if not kwargs.get("ruleset_id"): - kwargs["ruleset_id"] = _get_config_value(profile, "ruleset_id") - - action = action + "/" + str(kwargs["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 rule_params: - if not kwargs.get("ruleset_params"): - kwargs["ruleset_params"] = _get_config_value(profile, "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( + { + "ruleset_type": ruleset_type, + "owner": owner, + "repo_name": repo_name, + "org_name": org_name, + "ruleset_id": ruleset_id, + "ruleset_params": ruleset_params, + } + ) - return kwargs, action + return action, header_dict, params def get_ruleset(profile="github", **kwargs): @@ -1961,16 +1898,22 @@ def get_ruleset(profile="github", **kwargs): profile The name of the profile configuration to use. Defaults to ``github``. + ruleset_type - The type of ruleset. 'repo' or 'org'. + The type of ruleset. ``repo`` or ``org``. + ruleset_id Id of the ruleset. + owner The account owner of the repository. + repo_name The name of the repo. + org_name The name of the organization. + header_dict Headers to pass to Github API. @@ -1981,10 +1924,10 @@ def get_ruleset(profile="github", **kwargs): salt myminion github.get_ruleset salt myminion github.get_ruleset ruleset_id=1 """ - params, action = _check_ruleset_params(profile, rule_id=True, **kwargs) + action, header_dict, _ = _check_ruleset_params(profile, require_id=True, **kwargs) try: - ret = _query(profile, action, header_dict=params["header_dict"]) + 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')}" @@ -2003,16 +1946,22 @@ def delete_ruleset(profile="github", **kwargs): profile The name of the profile configuration to use. Defaults to ``github``. + ruleset_type - The type of ruleset. 'repo' or 'org'. + The type of ruleset. ``repo`` or ``org``. + owner The account owner of the repository. + repo_name The name of the repo. + ruleset_id Id of the ruleset. + org_name The name of the organization. + header_dict Headers to pass to Github API. @@ -2023,10 +1972,10 @@ def delete_ruleset(profile="github", **kwargs): salt myminion github.delete_ruleset salt myminion github.delete_ruleset ruleset_id=1 """ - params, action = _check_ruleset_params(profile, rule_id=True, **kwargs) + action, header_dict, params = _check_ruleset_params(profile, require_id=True, **kwargs) try: - ret = _query(profile, action, method="DELETE", header_dict=params["header_dict"]) + 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 @@ -2045,18 +1994,25 @@ def update_ruleset(profile="github", **kwargs): profile The name of the profile configuration to use. Defaults to ``github``. + ruleset_type - The type of ruleset. 'repo' or 'org'. + The type of ruleset. ``repo`` or ``org``. + ruleset_id Id of the ruleset. + owner The account owner of the repository. + repo_name The name of the repo. + org_name The name of the organization. + header_dict headers to pass to Github API. + ruleset_params number of returned results (max 100). @@ -2067,25 +2023,24 @@ def update_ruleset(profile="github", **kwargs): salt myminion github.update_ruleset salt myminion github.update_ruleset ruleset_id=1 """ - params, action = _check_ruleset_params(profile, rule_id=True, rule_params=True, **kwargs) - - if not isinstance(params["ruleset_params"], dict): - raise CommandExecutionError("params need to be a dict") + action, header_dict, params = _check_ruleset_params( + profile, require_id=True, require_params=True, **kwargs + ) - ruleset_params = salt.utils.json.dumps(params["ruleset_params"]) try: ret = _query( - profile, action, method="PUT", data=ruleset_params, header_dict=params["header_dict"] + profile, + action, + method="PUT", + data=params["ruleset_params"], + header_dict=header_dict, ) if not ret.get("error"): return ret["dict"] - else: - ret["comment"] = f"GitHub Response Status Code: {ret.get('error')}" - ret["result"] = False - except CommandExecutionError: - ret = {} + ret["comment"] = f"GitHub Response Status Code: {ret.get('error')}" ret["result"] = False - ret["comment"] = "Error updating ruleset" + except CommandExecutionError: + ret = {"result": False, "comment": "Error updating ruleset"} return ret @@ -2096,16 +2051,22 @@ def add_ruleset(profile="github", **kwargs): profile The name of the profile configuration to use. Defaults to ``github``. + ruleset_type - The type of ruleset. 'repo' or 'org'. + The type of ruleset. ``repo`` or ``org``. + owner The account owner of the repository. + repo_name The name of the repo. + org_name The name of the organization. + header_dict headers to pass to Github API. + ruleset_params number of returned results (max 100). @@ -2115,48 +2076,49 @@ def add_ruleset(profile="github", **kwargs): salt myminion github.add_ruleset """ - params, action = _check_ruleset_params(profile, rule_params=True, **kwargs) - - if not isinstance(params["ruleset_params"], dict): - raise CommandExecutionError("params need to be a dict") - if "name" not in params["ruleset_params"]: - raise CommandExecutionError("name is required") - if "enforcement" not in params["ruleset_params"]: - raise CommandExecutionError("enforcement is required") + action, header_dict, params = _check_ruleset_params( + profile, require_params={"name", "enforcement"}, **kwargs + ) - ruleset_params = salt.utils.json.dumps(params["ruleset_params"]) try: ret = _query( - profile, action, method="POST", data=ruleset_params, header_dict=params["header_dict"] + profile, + action, + method="POST", + data=params["ruleset_params"], + header_dict=header_dict, ) if not ret.get("error"): return ret["dict"] - else: - ret["comment"] = f"GitHub Response Status Code: {ret.get('error')}" - ret["result"] = False - except CommandExecutionError: - ret = {} + ret["comment"] = f"GitHub Response Status Code: {ret.get('error')}" ret["result"] = False - ret["comment"] = "Error adding ruleset" + except CommandExecutionError: + ret = {"result": False, "comment": "Error adding ruleset"} return ret -def list_rulesets(profile="github", **kwargs): +def list_rulesets(profile="github", *, per_page=None, **kwargs): """ List rulesets profile The name of the profile configuration to use. Defaults to ``github``. + ruleset_type - The type of ruleset. 'repo' or 'org'. + The type of ruleset. ``repo`` or ``org``. + owner The account owner of the repository. + repo_name The name of the repo. + org_name The name of the organization. + header_dict headers to pass to Github API. + per_page number of returned results (max 100). @@ -2166,27 +2128,18 @@ def list_rulesets(profile="github", **kwargs): salt myminion github.list_rulesets """ - params, action = _check_ruleset_params(profile, **kwargs) - - if not kwargs.get("per_page"): - try: - kwargs["per_page"] = _get_config_value(profile, "per_page") - except CommandExecutionError: - kwargs["per_page"] = None + action, header_dict, _ = _check_ruleset_params(profile, **kwargs) + 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=params["header_dict"], per_page=kwargs["per_page"] - ) + ret = _query(profile, action, header_dict=header_dict, per_page=per_page) if not ret.get("error"): - if ret["dict"]: - return {"rulesets": ret["dict"]} - return {"rulesets": None} - else: - ret["comment"] = f"GitHub Response Status Code: {ret.get('error')}" - ret["result"] = False - except CommandExecutionError: - ret = {} + return {"rulesets": ret["dict"] or None} + ret["comment"] = f"GitHub Response Status Code: {ret.get('error')}" ret["result"] = False - ret["comment"] = "Error getting rulesets" + except CommandExecutionError: + ret = {"result": False, "comment": "Error getting rulesets"} return ret diff --git a/src/saltext/github/states/github.py b/src/saltext/github/states/github.py index 1239e50b..dc4d8c9e 100644 --- a/src/saltext/github/states/github.py +++ b/src/saltext/github/states/github.py @@ -1,17 +1,5 @@ """ -Github User State Module - -.. versionadded:: 2016.3.0 - -This state is used to ensure presence of users in the Organization. - -.. code-block:: yaml - - ensure user test is present in github: - github.present: - - name: 'Example TestUser1' - - email: example@domain.com - - username: 'gitexample' +Manage organization teams/members, repositories and rulesets. """ import datetime @@ -22,14 +10,16 @@ log = logging.getLogger(__name__) +__virtualname__ = "github" + def __virtual__(): """ Only load if the github module is available in __salt__ """ if "github.list_users" in __salt__: - return "github" - return (False, "github module could not be loaded") + return __virtualname__ + return (False, "github module could not be loaded") # pragma: no cover def present(name, profile="github", **kwargs): @@ -40,9 +30,7 @@ def present(name, profile="github", **kwargs): ensure user test is present in github: github.present: - - name: 'gitexample' - - The following parameters are required: + - name: gitexample name This is the github handle of the user in the organization @@ -62,6 +50,8 @@ def present(name, profile="github", **kwargs): elif not target.get("in_org", False) and target.get("membership_state") != "pending": if __opts__["test"]: ret["comment"] = f"User {name} will be added to the org" + ret["changes"]["old"] = None + ret["changes"]["new"] = f"User {name} exists in the org now" return ret # add the user @@ -89,15 +79,12 @@ def absent(name, profile="github", **kwargs): ensure user test is absent in github: github.absent: - - name: 'Example TestUser1' + - name: Example TestUser1 - email: example@domain.com - - username: 'gitexample' - - The following parameters are required: + - username: gitexample name Github handle of the user in organization - """ ret = { @@ -113,6 +100,8 @@ def absent(name, profile="github", **kwargs): if isinstance(target, bool) or target.get("in_org", False): if __opts__["test"]: ret["comment"] = f"User {name} will be deleted" + ret["changes"]["old"] = f"User {name} exists" + ret["changes"]["new"] = f"User {name} deleted" ret["result"] = None return ret @@ -137,7 +126,7 @@ def absent(name, profile="github", **kwargs): return ret -def team_present( +def team_present( # pylint: disable=too-many-statements name, description=None, repo_names=None, @@ -162,27 +151,27 @@ def team_present( The names of repositories to add the team to. privacy - The level of privacy for the team, can be 'secret' or 'closed'. Defaults + The level of privacy for the team, can be ``secret`` or ``closed``. Defaults to secret. permission The default permission for new repositories added to the team, can be - 'pull', 'push' or 'admin'. Defaults to pull. + ``pull``, ``push`` or ``admin``. Defaults to pull. members The members belonging to the team, specified as a dict of member name to - optional configuration. Options include 'enforce_mfa_from' and 'mfa_exempt'. + optional configuration. Options include ``enforce_mfa_from`` and ``mfa_exempt``. enforce_mfa Whether to enforce MFA requirements on members of the team. If True then - all members without `mfa_exempt: True` configured will be removed from - the team. Note that `no_mfa_grace_seconds` may be set to allow members + all members without ``mfa_exempt: True`` configured will be removed from + the team. Note that ``no_mfa_grace_seconds`` may be set to allow members a grace period. no_mfa_grace_seconds The number of seconds of grace time that a member will have to enable MFA before being removed from the team. The grace period will begin from - `enforce_mfa_from` on the member configuration, which defaults to + ``enforce_mfa_from`` on the member configuration, which defaults to 1970/01/01. Example: @@ -191,20 +180,18 @@ def team_present( Ensure team test is present in github: github.team_present: - - name: 'test' + - name: test - members: user1: {} user2: {} Ensure team test_mfa is present in github: github.team_present: - - name: 'test_mfa' + - name: test_mfa - members: user1: enforce_mfa_from: 2016/06/15 - enforce_mfa: True - - .. versionadded:: 2016.11.0 """ ret = {"name": name, "changes": {}, "result": True, "comment": ""} @@ -221,22 +208,24 @@ def team_present( parameters["privacy"] = privacy if len(parameters) > 0: + team_change = { + "old": f"Team properties were {target}", + "new": f"Team properties (that changed) are {parameters}", + } if __opts__["test"]: test_comments.append(f"Team properties are set to be edited: {parameters}") ret["result"] = None + ret["changes"]["team"] = team_change else: result = __salt__["github.edit_team"](name, profile=profile, **parameters) if result: - ret["changes"]["team"] = { - "old": f"Team properties were {target}", - "new": f"Team properties (that changed) are {parameters}", - } + ret["changes"]["team"] = team_change else: ret["result"] = False ret["comment"] = "Failed to update team properties." return ret - current_repos = set(__salt__["github.list_team_repos"](name, profile=profile).keys()) + current_repos = set(__salt__["github.list_team_repos"](name, profile=profile)) repo_names = set(repo_names or []) repos_to_add = repo_names - current_repos @@ -250,16 +239,19 @@ def team_present( ) ) ret["result"] = None - else: - for repo_name in repos_to_add: + for repo_name in repos_to_add: + repo_change = { + "old": f"Repo {repo_name} is not in team {name}", + "new": f"Repo {repo_name} is in team {name}", + } + if __opts__["test"]: + ret["changes"][repo_name] = repo_change + else: result = __salt__["github.add_team_repo"]( repo_name, name, profile=profile, **kwargs ) if result: - ret["changes"][repo_name] = { - "old": f"Repo {repo_name} is not in team {name}", - "new": f"Repo {repo_name} is in team {name}", - } + ret["changes"][repo_name] = repo_change else: ret["result"] = False ret["comment"] = f"Failed to add repo {repo_name} to team {name}." @@ -273,16 +265,19 @@ def team_present( ) ) ret["result"] = None - else: - for repo_name in repos_to_remove: + for repo_name in repos_to_remove: + repo_change = { + "old": f"Repo {repo_name} is in team {name}", + "new": f"Repo {repo_name} is not in team {name}", + } + if __opts__["test"]: + ret["changes"][repo_name] = repo_change + else: result = __salt__["github.remove_team_repo"]( repo_name, name, profile=profile, **kwargs ) if result: - ret["changes"][repo_name] = { - "old": f"Repo {repo_name} is in team {name}", - "new": f"Repo {repo_name} is not in team {name}", - } + ret["changes"][repo_name] = repo_change else: ret["result"] = False ret["comment"] = f"Failed to remove repo {repo_name} from team {name}." @@ -290,7 +285,9 @@ def team_present( else: # Team does not exist - it will be created. if __opts__["test"]: + # Note: member/repo changes are not projected for new teams ret["comment"] = f"Team {name} is set to be created." + ret["changes"]["team"] = {"old": None, "new": f"Team {name} has been created"} ret["result"] = None return ret @@ -314,17 +311,19 @@ def team_present( manage_members = members is not None - mfa_deadline = datetime.datetime.utcnow() - datetime.timedelta(seconds=no_mfa_grace_seconds) + mfa_deadline = datetime.datetime.now(tz=datetime.timezone.utc) - datetime.timedelta( + seconds=no_mfa_grace_seconds + ) members_no_mfa = __salt__["github.list_members_without_mfa"](profile=profile) members_lower = {} - for member_name, info in members or {}.items(): + for member_name, info in (members or {}).items(): members_lower[member_name.lower()] = info member_change = False current_members = __salt__["github.list_team_members"](name, profile=profile) - for member, member_info in members or {}.items(): + for member, member_info in (members or {}).items(): log.info("Checking member %s in team %s", member, name) if member.lower() not in current_members: @@ -337,17 +336,20 @@ def team_present( ) else: # Add to team member_change = True + member_add = { + "old": f"User {member} is not in team {name}", + "new": f"User {member} is in team {name}", + } if __opts__["test"]: test_comments.append(f"User {member} set to be added to the team.") ret["result"] = None + ret["changes"][member] = member_add else: result = __salt__["github.add_team_member"]( member, name, profile=profile, **kwargs ) if result: - ret["changes"][member] = {} - ret["changes"][member]["old"] = f"User {member} is not in team {name}" - ret["changes"][member]["new"] = f"User {member} is in team {name}" + ret["changes"][member] = member_add else: ret["result"] = False ret["comment"] = f"Failed to add user {member} to team {name}." @@ -362,6 +364,11 @@ def team_present( if manage_members and member not in members_lower or (enforce_mfa and mfa_violation): # Remove from team member_change = True + extra_changes = " due to MFA violation" if mfa_violation else "" + member_removal = { + "old": f"User {member} is in team {name}", + "new": f"User {member} is not in team {name}{extra_changes}", + } if __opts__["test"]: if mfa_violation: test_comments.append( @@ -370,16 +377,13 @@ def team_present( else: test_comments.append(f"User {member} set to be removed from the team.") ret["result"] = None + ret["changes"][member] = member_removal else: result = __salt__["github.remove_team_member"]( member, name, profile=profile, **kwargs ) if result: - extra_changes = " due to MFA violation" if mfa_violation else "" - ret["changes"][member] = { - "old": f"User {member} is in team {name}", - "new": f"User {member} is not in team {name}{extra_changes}", - } + ret["changes"][member] = member_removal else: ret["result"] = False ret["comment"] = f"Failed to remove user {member} from team {name}." @@ -398,7 +402,7 @@ def _member_violates_mfa(member, member_info, mfa_deadline, members_without_mfa) return False enforce_mfa_from = datetime.datetime.strptime( member_info.get("enforce_mfa_from", "1970/01/01"), "%Y/%m/%d" - ) + ).replace(tzinfo=datetime.timezone.utc) return member.lower() in members_without_mfa and (mfa_deadline > enforce_mfa_from) @@ -412,15 +416,11 @@ def team_absent(name, profile="github", **kwargs): ensure team test is present in github: github.team_absent: - - name: 'test' - + - name: test - The following parameters are required: name This is the name of the team in the organization. - - .. versionadded:: 2016.11.0 """ ret = {"name": name, "changes": {}, "result": None, "comment": ""} @@ -430,22 +430,23 @@ def team_absent(name, profile="github", **kwargs): ret["comment"] = f"Team {name} does not exist" ret["result"] = True return ret - else: - if __opts__["test"]: - ret["comment"] = f"Team {name} will be deleted" - ret["result"] = None - return ret + if __opts__["test"]: + ret["comment"] = f"Team {name} will be deleted" + ret["changes"]["old"] = f"Team {name} exists" + ret["changes"]["new"] = f"Team {name} deleted" + ret["result"] = None + return ret - result = __salt__["github.remove_team"](name, profile=profile, **kwargs) + result = __salt__["github.remove_team"](name, profile=profile, **kwargs) - if result: - ret["comment"] = f"Deleted team {name}" - ret["changes"].setdefault("old", f"Team {name} exists") - ret["changes"].setdefault("new", f"Team {name} deleted") - ret["result"] = True - else: - ret["comment"] = f"Failed to delete {name}" - ret["result"] = False + if result: + ret["comment"] = f"Deleted team {name}" + ret["changes"].setdefault("old", f"Team {name} exists") + ret["changes"].setdefault("new", f"Team {name} deleted") + ret["result"] = True + else: + ret["comment"] = f"Failed to delete {name}" + ret["result"] = False return ret @@ -500,9 +501,7 @@ def repo_present( teams The teams for which this repo should belong to, specified as a dict of - team name to permission ('pull', 'push' or 'admin'). - - .. versionadded:: 2017.7.0 + team name to permission (``pull``, ``push`` or ``admin``). Example: @@ -510,10 +509,8 @@ def repo_present( Ensure repo my-repo is present in github: github.repo_present: - - name: 'my-repo' - - description: 'My very important repository' - - .. versionadded:: 2016.11.0 + - name: my-repo + - description: My very important repository """ ret = {"name": name, "changes": {}, "result": True, "comment": ""} @@ -582,7 +579,7 @@ def repo_present( else: add_params = dict(given_params) add_params.update(kwargs) - result = __salt__["github.add_repo"](name, **add_params) + result = __salt__["github.add_repo"](name, profile=profile, **add_params) if not result: ret["result"] = False @@ -619,23 +616,23 @@ def repo_present( # First remove any teams that aren't present for team_name in current_team_names: - if team_name not in teams: - team_change = { - "old": f"Repo {name} is in team {team_name}", - "new": f"Repo {name} is not in team {team_name}", - } + if team_name in teams: + continue + team_change = { + "old": f"Repo {name} is in team {team_name}", + "new": f"Repo {name} is not in team {team_name}", + } - if __opts__["test"]: - ret["changes"][team_name] = team_change - ret["result"] = None - else: - result = __salt__["github.remove_team_repo"](name, team_name, profile=profile) - if result: - ret["changes"][team_name] = team_change - else: - ret["result"] = False - ret["comment"] = f"Failed to remove repo {name} from team {team_name}." - return ret + if __opts__["test"]: + ret["changes"][team_name] = team_change + ret["result"] = None + else: + result = __salt__["github.remove_team_repo"](name, team_name, profile=profile) + if not result: + ret["result"] = False + ret["comment"] = f"Failed to remove repo {name} from team {team_name}." + return ret + ret["changes"][team_name] = team_change # Next add or modify any necessary teams for team_name, permission in teams.items(): @@ -651,12 +648,11 @@ def repo_present( result = __salt__["github.add_team_repo"]( name, team_name, profile=profile, permission=permission ) - if result: - ret["changes"][team_name] = team_change - else: + if not result: ret["result"] = False - ret["comment"] = f"Failed to remove repo {name} from team {team_name}." + ret["comment"] = f"Failed to add repo {name} to team {team_name}." return ret + ret["changes"][team_name] = team_change else: current_permission = ( __salt__["github.list_team_repos"](team_name, profile=profile) @@ -669,26 +665,26 @@ def repo_present( f"Failed to determine current permission for team {team_name} in repo {name}" ) return ret - elif current_permission != permission: - team_change = { - "old": f"Repo {name} in team {team_name} has permission {current_permission}", - "new": f"Repo {name} in team {team_name} has permission {permission}", - } - if __opts__["test"]: - ret["changes"][team_name] = team_change - ret["result"] = None - else: - result = __salt__["github.add_team_repo"]( - name, team_name, profile=profile, permission=permission + if current_permission == permission: + continue + team_change = { + "old": f"Repo {name} in team {team_name} has permission {current_permission}", + "new": f"Repo {name} in team {team_name} has permission {permission}", + } + if __opts__["test"]: + ret["changes"][team_name] = team_change + ret["result"] = None + else: + result = __salt__["github.add_team_repo"]( + name, team_name, profile=profile, permission=permission + ) + if not result: + ret["result"] = False + ret["comment"] = ( + f"Failed to set permission on repo {name} from team {team_name} to {permission}." ) - if result: - ret["changes"][team_name] = team_change - else: - ret["result"] = False - ret["comment"] = ( - f"Failed to set permission on repo {name} from team {team_name} to {permission}." - ) - return ret + return ret + ret["changes"][team_name] = team_change return ret @@ -702,14 +698,10 @@ def repo_absent(name, profile="github", **kwargs): ensure repo test is absent in github: github.repo_absent: - - name: 'test' - - The following parameters are required: + - name: test name This is the name of the repository in the organization. - - .. versionadded:: 2016.11.0 """ ret = {"name": name, "changes": {}, "result": None, "comment": ""} @@ -722,24 +714,27 @@ def repo_absent(name, profile="github", **kwargs): ret["comment"] = f"Repo {name} does not exist" ret["result"] = True return ret - else: - if __opts__["test"]: - ret["comment"] = f"Repo {name} will be deleted" - ret["result"] = None - return ret - result = __salt__["github.remove_repo"](name, profile=profile, **kwargs) + if __opts__["test"]: + ret["comment"] = f"Repo {name} will be deleted" + ret["changes"]["old"] = f"Repo {name} exists" + ret["changes"]["new"] = f"Repo {name} deleted" + ret["result"] = None + return ret - if result: - ret["comment"] = f"Deleted repo {name}" - ret["changes"].setdefault("old", f"Repo {name} exists") - ret["changes"].setdefault("new", f"Repo {name} deleted") - ret["result"] = True - else: - ret["comment"] = ( - f"Failed to delete repo {name}. Ensure the delete_repo scope is enabled if using OAuth." - ) - ret["result"] = False + result = __salt__["github.remove_repo"](name, profile=profile, **kwargs) + + if result: + ret["comment"] = f"Deleted repo {name}" + ret["changes"].setdefault("old", f"Repo {name} exists") + ret["changes"].setdefault("new", f"Repo {name} deleted") + ret["result"] = True + return ret + + ret["comment"] = ( + f"Failed to delete repo {name}. Ensure the delete_repo scope is enabled if using OAuth." + ) + ret["result"] = False return ret @@ -755,17 +750,16 @@ def ruleset_absent( ensure rulset test is absent in github: github.ruleset_absent: - - name: 'test' - - ruleset_type: 'repo' + - name: test + - ruleset_type: repo - owner: test - repo_name: test_repo - The following parameters are required: - name name of the ruleset. + ruleset_type - type of ruleset ('org' or 'repo') + type of ruleset (``org`` or ``repo``) """ ret = {"name": name, "changes": {}, "result": None, "comment": ""} @@ -787,31 +781,34 @@ def ruleset_absent( ret["comment"] = "error listing rulesets" return ret - if rulesets["rulesets"]: - for ruleset in rulesets["rulesets"]: - if ruleset["name"] == name: - if __opts__["test"]: - ret["comment"] = f"Ruleset {name} will be deleted" - ret["result"] = None - return ret - - kwargs.update({"ruleset_id": ruleset["id"]}) - result = __salt__["github.delete_ruleset"](profile, **kwargs) - - if result: - ret["comment"] = f"Deleted ruleset {name}" - ret["changes"]["old"] = f"ruleset {name} exists" - ret["changes"]["new"] = f"ruleset {name} deleted" - ret["result"] = True - return ret - else: - ret["comment"] = f"Failed to delete ruleset {name}" - ret["result"] = False + ruleset = next( + (ruleset for ruleset in (rulesets.get("rulesets") or []) if ruleset["name"] == name), None + ) + if ruleset is None: + ret["comment"] = f"Ruleset {name} does not exist" + ret["result"] = True + return ret - ret["comment"] = f"Ruleset {name} does not exist" - ret["result"] = True if __opts__["test"]: + ret["comment"] = f"Ruleset {name} will be deleted" + ret["changes"]["old"] = f"ruleset {name} exists" + ret["changes"]["new"] = f"ruleset {name} deleted" ret["result"] = None + return ret + + kwargs.update({"ruleset_id": ruleset["id"]}) + result = __salt__["github.delete_ruleset"](profile, **kwargs) + + # delete_ruleset returns a (truthy) dict with result: False on failure + if result.get("result") is not False: + ret["comment"] = f"Deleted ruleset {name}" + ret["changes"]["old"] = f"ruleset {name} exists" + ret["changes"]["new"] = f"ruleset {name} deleted" + ret["result"] = True + return ret + + ret["comment"] = f"Failed to delete ruleset {name}" + ret["result"] = False return ret @@ -842,15 +839,14 @@ def ruleset_present( target: branch enforcement: disabled - The following parameters are required: - name name of the ruleset. + ruleset_type - type of ruleset ('org' or 'repo') - ruleset_params - parameters to set the ruleset rules. i.e {'enforcement': 'disabled'} + type of ruleset (``org`` or ``repo``) + ruleset_params + parameters to set the ruleset rules. i.e ``{enforcement: disabled}`` """ ret = {"name": name, "changes": {}, "result": True, "comment": ""} @@ -876,52 +872,53 @@ def ruleset_present( ret["comment"] = "error listing rulesets" return ret - if rulesets["rulesets"]: - for ruleset in rulesets["rulesets"]: - if ruleset["name"] == name: - kwargs.update({"ruleset_id": ruleset["id"]}) - ruleset_info = __salt__["github.get_ruleset"](profile, **kwargs) - if ruleset_info.get("id"): - for key in ruleset_info and ruleset_params: - if ruleset_info.get(key) != ruleset_params.get(key): - changes = { - "old": f"Ruleset properties were {ruleset_info}", - "new": f"Ruleset properties (that changed) are {ruleset_params}", - } - ret["changes"] = changes - ret["comment"] = "ruleset updated" - if __opts__["test"]: - ret["changes"] = {} - ret["result"] = None - ret["comment"] = "ruleset will be updated" - else: - kwargs.update({"ruleset_params": ruleset_params}) - result = __salt__["github.update_ruleset"](profile, **kwargs) - if not result.get("id"): - ret["changes"] = {} - ret["comment"] = "Could not update ruleset" - return ret - ret["comment"] = "ruleset present" - if __opts__["test"]: - ret["result"] = None - ret["changes"] = {} - return ret - else: - ret["comment"] = "Could not get ruleset" - ret["result"] = False - return ret - - changes = {"old": "No existing ruleset found", "new": "Ruleset created"} - ret["changes"] = changes - if __opts__["test"]: - ret["changes"] = {} - ret["result"] = None - ret["comment"] = "ruleset will be added" - else: + ruleset = next( + (ruleset for ruleset in (rulesets.get("rulesets") or []) if ruleset["name"] == name), None + ) + if ruleset is None: + ret["changes"] = {"old": "No existing ruleset found", "new": "Ruleset created"} + if __opts__["test"]: + ret["result"] = None + ret["comment"] = "ruleset will be added" + return ret result = __salt__["github.add_ruleset"](profile, ruleset_params=ruleset_params, **kwargs) ret["comment"] = "ruleset added" if not result.get("id"): ret["changes"] = {} ret["result"] = False ret["comment"] = "ruleset could not be added" + return ret + + kwargs.update({"ruleset_id": ruleset["id"]}) + ruleset_info = __salt__["github.get_ruleset"](profile, **kwargs) + if not ruleset_info.get("id"): + ret["comment"] = "Could not get ruleset" + ret["result"] = False + return ret + + # Only diff the parameters that are being managed. The server + # response carries additional server-populated fields (id, + # timestamps, source, ...) that must not register as drift. + diff = {key: value for key, value in ruleset_params.items() if ruleset_info.get(key) != value} + if not diff: + ret["comment"] = "ruleset present" + return ret + + ret["changes"] = { + "old": {key: ruleset_info.get(key) for key in diff}, + "new": diff, + } + if __opts__["test"]: + ret["result"] = None + ret["comment"] = "ruleset will be updated" + return ret + + kwargs.update({"ruleset_params": ruleset_params}) + result = __salt__["github.update_ruleset"](profile, **kwargs) + if result.get("id"): + ret["comment"] = "ruleset updated" + else: + ret["changes"] = {} + ret["result"] = False + ret["comment"] = "Could not update ruleset" return ret diff --git a/src/saltext/github/utils/__init__.py b/src/saltext/github/utils/__init__.py index e69de29b..abf8be4a 100644 --- a/src/saltext/github/utils/__init__.py +++ b/src/saltext/github/utils/__init__.py @@ -0,0 +1,40 @@ +import typing +from collections.abc import Mapping +from types import EllipsisType + +K = typing.TypeVar("K") +V = typing.TypeVar("V") + + +@typing.overload +def filter_unset( + data: Mapping[K, V | None], + unset: None = None, +) -> dict[K, V]: ... +@typing.overload +def filter_unset( + data: Mapping[K, V | EllipsisType], + unset: EllipsisType, +) -> dict[K, V]: ... +@typing.overload +def filter_unset( + data: Mapping[K, V], + unset: object, +) -> dict[K, V]: ... +def filter_unset( + data: Mapping[K, V], + unset: object = None, +) -> dict[K, object]: + """ + Remove all unset values in a dictionary and return the filtered result. + + data + Dictionary to filter. + + unset + Identity of what counts as unset; evaluated via ``is not``. + Defaults to ``None``. Other working examples: ``...`` (ellipsis), + a specific object such as ``salt.defaults.NOT_SET`` or + a string literal (not recommended though). + """ + return {k: v for k, v in data.items() if v is not unset} diff --git a/tests/functional/conftest.py b/tests/functional/conftest.py index 0a4757cd..8c07249d 100644 --- a/tests/functional/conftest.py +++ b/tests/functional/conftest.py @@ -4,6 +4,8 @@ import pytest from saltfactories.utils.functional import Loaders +from tests.support.github import FakeGitHubAPI + log = logging.getLogger(__name__) @@ -31,12 +33,17 @@ def state_tree_prod(tmp_path_factory): # pragma: no cover @pytest.fixture(scope="module") -def minion_config_defaults(): # pragma: no cover +def minion_config_defaults(github_profile): # pragma: no cover """ Functional test modules can provide this fixture to tweak the default configuration dictionary passed to the minion factory """ - return {} + noprivacy = dict(github_profile) + noprivacy.pop("allow_repo_privacy_changes") + return { + "github": github_profile, + "github_noprivacy": noprivacy, + } @pytest.fixture(scope="module") @@ -148,3 +155,82 @@ def modules(loaders): # pragma: no cover @pytest.fixture(scope="module") def states(loaders): # pragma: no cover return loaders.states + + +@pytest.fixture(scope="module") +def github_profile(fake_github, fake_org): + return { + "token": fake_github.token, + "org_name": fake_org.data.login, + "api_url": fake_github.url, + "allow_repo_privacy_changes": True, + "repo_name": "repo-a", + } + + +@pytest.fixture(scope="session") +def fake_github(): + """ + Mock API implementation for functional tests + """ + api = FakeGitHubAPI() + with api.serve(): + yield api + + +@pytest.fixture(scope="session") +def fake_state(fake_github): + return fake_github.state + + +@pytest.fixture(scope="module") +def fake_org(fake_state, fake_setup): # pylint: disable=unused-argument + """ + Fake organization named ``testorg``. Ensure ``fake_setup`` creates it. + """ + return fake_state.get_org("testorg") + + +@pytest.fixture(autouse=True) +def _restore_fake_state(fake_github): + """ + Before each test, reset the state to the module's defaults as specified in + ``fake_setup``. Also, clear the request log. + """ + fake_github.reset() + + +@pytest.fixture(scope="module") +def fake_setup(fake_state): + """ + Default setup for fake_state. Override this per module, if necessary. + """ + user1 = fake_state.add_user("user1") + user2 = fake_state.add_user("user2", two_factor_authentication=False) + fake_state.add_user("newuser") + org = fake_state.add_org("testorg") + org.add_member(user1) + org.add_member(user2) + repo_a = org.add_repo("repo-a") + org.add_repo("repo-b", private=True) + org.add_ruleset("org-rules") + team_a = org.add_team("Team A") + repo_a.add_ruleset("repo-rules", enforcement="disabled") + team_a.add_member(user1) + team_a.add_repo(repo_a, "pull") + repo_a.add_milestone("v1.0", user1) + repo_a.add_milestone("v2.0", user1) + bug = repo_a.add_issue( + "Found a bug", user1, labels=["bug", "prio:high"], assignee="user2", milestone="v1.0" + ) + bug.add_comment(user2, "Working on it", updated_at="2026-10-11T14:00:00Z") + bug.add_comment(user1, "Thanks!", updated_at="2026-10-11T15:00:00Z") + repo_a.add_issue("Stale issue", user2, state="closed") + repo_a.add_pull("Fix the bug", user2, head="bugfix") + + +@pytest.fixture(scope="module", autouse=True) +def fake_state_setup(fake_state, request): + fake_state.reset() + request.getfixturevalue("fake_setup") + fake_state.snapshot() diff --git a/tests/functional/modules/github/__init__.py b/tests/functional/modules/github/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/tests/functional/modules/github/conftest.py b/tests/functional/modules/github/conftest.py new file mode 100644 index 00000000..460def4c --- /dev/null +++ b/tests/functional/modules/github/conftest.py @@ -0,0 +1,6 @@ +import pytest + + +@pytest.fixture +def github(modules): + return modules.github diff --git a/tests/functional/modules/github/test_issues.py b/tests/functional/modules/github/test_issues.py new file mode 100644 index 00000000..44f42b09 --- /dev/null +++ b/tests/functional/modules/github/test_issues.py @@ -0,0 +1,127 @@ +import pytest +from salt.exceptions import CommandExecutionError + + +@pytest.fixture +def repo_a(fake_org): + return fake_org.get_repo("repo-a") + + +def test_get_issues(github, repo_a): + bug = repo_a.issues["Found a bug"] + pull = repo_a.pulls["Fix the bug"] + ret = github.get_issues("repo-a") + # Only open issues by default, and PRs listed by the API are filtered out + assert set(ret) == {bug.id} + assert pull.id not in ret + formatted = ret[bug.id] + assert formatted["issue_number"] == bug.number + assert formatted["state"] == "open" + assert formatted["title"] == "Found a bug" + assert formatted["user"] == "user1" + assert formatted["assignee"] == "user2" + assert formatted["labels"] == ["bug", "prio:high"] + assert formatted["milestone"] == "v1.0" + + +@pytest.mark.usefixtures("repo_a") +def test_get_issues_all_states(github): + ret = github.get_issues("repo-a", state="all") + assert {issue["title"] for issue in ret.values()} == {"Found a bug", "Stale issue"} + + +def test_get_issues_full_output(github, repo_a): + bug = repo_a.issues["Found a bug"] + ret = github.get_issues("repo-a", output="full") + assert ret[bug.id]["labels"] == [{"name": "bug"}, {"name": "prio:high"}] + assert ret[bug.id]["user"]["login"] == "user1" + + +def test_get_issues_filter_args(github, fake_github, repo_a): + bug = repo_a.issues["Found a bug"] + ret = github.get_issues( + "repo-a", + milestone=1, + assignee="user2", + creator="user1", + mentioned="user2", + labels="bug", + since="2026-01-01T00:00:00Z", + sort="updated", + direction="asc", + per_page=10, + ) + assert bug.id in ret + (req,) = fake_github.filter_requests(path="*/repo-a/issues") + for key, value in ( + ("milestone", "1"), + ("assignee", "user2"), + ("creator", "user1"), + ("mentioned", "user2"), + ("labels", "bug"), + ("since", "2026-01-01T00:00:00Z"), + ("sort", "updated"), + ("direction", "asc"), + ("per_page", "10"), + ): + assert req.query[key] == value + + +def test_get_issues_label_filter_mismatch(github): + assert github.get_issues("repo-a", labels="nonexistent") == {} + + +def test_get_issues_repo_name_from_profile(github, repo_a): + bug = repo_a.issues["Found a bug"] + assert bug.id in github.get_issues() + + +def test_get_issues_missing_repo(github): + with pytest.raises(CommandExecutionError, match="404"): + github.get_issues("ghost") + + +def test_get_issue(github, repo_a): + bug = repo_a.issues["Found a bug"] + ret = github.get_issue(bug.number, repo_name="repo-a") + assert set(ret) == {bug.id} + assert ret[bug.id]["title"] == "Found a bug" + assert ret[bug.id]["user"] == "user1" + + +def test_get_issue_full_output(github, repo_a): + bug = repo_a.issues["Found a bug"] + # repo_name defaults to the profile value (repo-a) + ret = github.get_issue(bug.number, output="full") + assert ret[bug.id]["user"]["login"] == "user1" + + +def test_get_issue_missing(github): + assert github.get_issue(999, repo_name="repo-a") == {} + + +def test_get_issue_comments(github, repo_a): + bug = repo_a.issues["Found a bug"] + ret = github.get_issue_comments(bug.number, repo_name="repo-a") + assert len(ret) == 2 + assert {comment["user_login"] for comment in ret.values()} == {"user1", "user2"} + for comment_id, comment in ret.items(): + assert comment["id"] == comment_id + + +def test_get_issue_comments_since(github, repo_a): + bug = repo_a.issues["Found a bug"] + ret = github.get_issue_comments(bug.number, repo_name="repo-a", since="2026-10-11T14:30:00Z") + assert [comment["user_login"] for comment in ret.values()] == ["user1"] + + +def test_get_issue_comments_full_output(github, repo_a): + bug = repo_a.issues["Found a bug"] + # repo_name defaults to the profile value (repo-a) + ret = github.get_issue_comments(bug.number, output="full") + assert {comment["body"] for comment in ret.values()} == {"Working on it", "Thanks!"} + + +def test_get_issue_comments_missing_issue(github): + with pytest.raises(CommandExecutionError, match="404"): + github.get_issue_comments(999, repo_name="repo-a") diff --git a/tests/functional/modules/github/test_milestones.py b/tests/functional/modules/github/test_milestones.py new file mode 100644 index 00000000..1382da09 --- /dev/null +++ b/tests/functional/modules/github/test_milestones.py @@ -0,0 +1,71 @@ +import pytest +from salt.exceptions import CommandExecutionError + + +def test_get_milestones_min_output(github): + ret = github.get_milestones(repo_name="repo-a") + assert len(ret) == 2 + for milestone in ret.values(): + # min output strips these keys and must not crash when absent + assert "creator" not in milestone + assert "html_url" not in milestone + assert "labels_url" not in milestone + assert sorted(milestone["title"] for milestone in ret.values()) == ["v1.0", "v2.0"] + + +def test_get_milestones_full_output(github): + ret = github.get_milestones(repo_name="repo-a", output="full") + assert all(milestone["creator"]["login"] == "user1" for milestone in ret.values()) + + +def test_get_milestone_by_number(github): + # repo_name defaults to the profile value (repo-a) + ret = github.get_milestone(number=1) + assert [milestone["title"] for milestone in ret.values()] == ["v1.0"] + + +def test_get_milestone_by_name(github): + ret = github.get_milestone(name="v2.0", repo_name="repo-a") + assert [milestone["title"] for milestone in ret.values()] == ["v2.0"] + + +def test_get_milestones_repo_name_from_profile(github): + assert len(github.get_milestones()) == 2 + + +def test_get_milestones_filter_args(github, fake_github): + github.get_milestones( + repo_name="repo-a", state="closed", sort="completeness", direction="desc", per_page=10 + ) + (req,) = fake_github.filter_requests(path="*/repo-a/milestones") + for key, value in ( + ("state", "closed"), + ("sort", "completeness"), + ("direction", "desc"), + ("per_page", "10"), + ): + assert req.query[key] == value + + +def test_get_milestones_missing_repo(github): + with pytest.raises(CommandExecutionError, match="404"): + github.get_milestones("ghost") + + +def test_get_milestone_by_number_full_output(github): + ret = github.get_milestone(number=1, repo_name="repo-a", output="full") + (milestone,) = ret.values() + assert milestone["creator"]["login"] == "user1" + + +def test_get_milestone_by_number_missing(github): + assert github.get_milestone(number=99, repo_name="repo-a") == {} + + +def test_get_milestone_by_name_missing(github): + assert github.get_milestone(name="nope", repo_name="repo-a") == {} + + +def test_get_milestone_requires_name_or_number(github): + with pytest.raises(CommandExecutionError, match="'name' or 'number'"): + github.get_milestone(repo_name="repo-a") diff --git a/tests/functional/modules/github/test_profiles.py b/tests/functional/modules/github/test_profiles.py new file mode 100644 index 00000000..dd86314b --- /dev/null +++ b/tests/functional/modules/github/test_profiles.py @@ -0,0 +1,34 @@ +""" +Verify that all module functions honor a non-default profile, including +internal cache refreshes (which used to fall back to the default profile). +""" + +import github as pygithub +import pytest +from salt.exceptions import CommandExecutionError + + +@pytest.fixture(scope="module") +def minion_config_defaults(github_profile): + broken = github_profile.copy() + broken["org_name"] = "missing-org" + return {"github": broken, "primary": github_profile} + + +def test_add_team_uses_profile(github, fake_org): + # The internal team cache refresh used to drop the profile argument, + # which hit the (broken) default profile and failed the whole call. + assert github.add_team("new-team", description="New", profile="primary") is True + fake_org.get_team("new-team") + assert github.get_team("new-team", profile="primary") + + +def test_module_functions_honor_profile(github): + assert set(github.list_repos(profile="primary")) == {"repo-a", "repo-b"} + with pytest.raises(pygithub.UnknownObjectException): + github.list_repos() + + +def test_unconfigured_profile(github): + with pytest.raises(CommandExecutionError, match="could not be found"): + github.list_repos(profile="unconfigured") diff --git a/tests/functional/modules/github/test_prs.py b/tests/functional/modules/github/test_prs.py new file mode 100644 index 00000000..4b69a573 --- /dev/null +++ b/tests/functional/modules/github/test_prs.py @@ -0,0 +1,60 @@ +import pytest +from salt.exceptions import CommandExecutionError + + +@pytest.fixture +def repo_a(fake_org): + return fake_org.get_repo("repo-a") + + +def test_get_prs(github, repo_a): + pull = repo_a.pulls["Fix the bug"] + ret = github.get_prs("repo-a") + assert set(ret) == {pull.id} + formatted = ret[pull.id] + assert formatted["pr_number"] == pull.number + assert formatted["state"] == "open" + assert formatted["title"] == "Fix the bug" + assert formatted["user"] == "user2" + assert formatted["base_branch"] == "main" + + +def test_get_prs_full_output(github, repo_a): + pull = repo_a.pulls["Fix the bug"] + ret = github.get_prs("repo-a", output="full") + assert ret[pull.id]["head"]["ref"] == "bugfix" + assert ret[pull.id]["user"]["login"] == "user2" + + +def test_get_prs_filter_args(github, fake_github, repo_a): + pull = repo_a.pulls["Fix the bug"] + ret = github.get_prs( + "repo-a", + state="all", + head="testorg:bugfix", + base="main", + sort="updated", + direction="asc", + per_page=50, + ) + assert pull.id in ret + (req,) = fake_github.filter_requests(path="*/repo-a/pulls") + for key, value in ( + ("state", "all"), + ("head", "testorg:bugfix"), + ("base", "main"), + ("sort", "updated"), + ("direction", "asc"), + ("per_page", "50"), + ): + assert req.query[key] == value + + +def test_get_prs_repo_name_from_profile(github, repo_a): + pull = repo_a.pulls["Fix the bug"] + assert pull.id in github.get_prs() + + +def test_get_prs_missing_repo(github): + with pytest.raises(CommandExecutionError, match="404"): + github.get_prs("ghost") diff --git a/tests/functional/modules/github/test_repos.py b/tests/functional/modules/github/test_repos.py new file mode 100644 index 00000000..df5a91e9 --- /dev/null +++ b/tests/functional/modules/github/test_repos.py @@ -0,0 +1,105 @@ +import pytest +from salt.exceptions import CommandExecutionError + +from tests.support.github import NotFound + + +def test_list_repos(github): + assert set(github.list_repos()) == {"repo-a", "repo-b"} + + +def test_list_repos_uses_cache(github, fake_org): + assert set(github.list_repos()) == {"repo-a", "repo-b"} + fake_org.add_repo("repo-c") + # The repo list is cached (and offers no cache bypass) + assert set(github.list_repos()) == {"repo-a", "repo-b"} + + +def test_list_private_repos(github): + assert github.list_private_repos() == ["repo-b"] + + +def test_list_public_repos(github): + assert github.list_public_repos() == ["repo-a"] + + +def test_get_repo_info(github): + ret = github.get_repo_info("repo-a") + assert ret["name"] == "repo-a" + assert ret["full_name"] == "testorg/repo-a" + assert ret["description"] == "Test repo repo-a" + assert ret["private"] is False + assert ret["default_branch"] == "main" + assert ret["has_issues"] is True + + +def test_get_repo_info_missing(github): + with pytest.raises(CommandExecutionError): + github.get_repo_info("ghost") + + +def test_get_repo_info_uses_cache(github, fake_org): + assert github.get_repo_info("repo-a")["description"] == "Test repo repo-a" + fake_org.get_repo("repo-a").data.description = "mutated" + # Cached result is returned until the cache is bypassed + assert github.get_repo_info("repo-a")["description"] == "Test repo repo-a" + assert github.get_repo_info("repo-a", ignore_cache=True)["description"] == "mutated" + + +def test_list_repos_primes_repo_info_cache(github, fake_org): + github.list_repos() + fake_org.get_repo("repo-a").data.description = "mutated" + assert github.get_repo_info("repo-a")["description"] == "Test repo repo-a" + + +def test_add_repo(github, fake_org): + assert github.add_repo("new-repo", description="Brand new") is True + assert fake_org.get_repo("new-repo").description == "Brand new" + + +def test_edit_repo(github, fake_org): + assert github.edit_repo("repo-a", description="Updated") is True + assert fake_org.get_repo("repo-a").description == "Updated" + # edit_repo refreshes the cache itself + assert github.get_repo_info("repo-a")["description"] == "Updated" + + +def test_edit_repo_privacy_guard(github): + with pytest.raises(CommandExecutionError, match="privacy"): + github.edit_repo("repo-a", private=True, profile="github_noprivacy") + + +def test_remove_repo(github, fake_org): + assert github.remove_repo("repo-b") is True + with pytest.raises(NotFound): + fake_org.get_repo("repo-b") + + +def test_remove_repo_missing(github): + assert github.remove_repo("ghost") is False + + +def test_remove_repo_failure(github, fake_org): + # The repo vanishes server-side behind the module's cache + github.list_repos() + fake_org.remove_repo("repo-b") + assert github.remove_repo("repo-b") is False + + +def test_add_repo_duplicate(github): + # The API refuses to create a repo with an existing name + assert github.add_repo("repo-a") is False + + +def test_edit_repo_missing(github): + assert github.edit_repo("ghost", description="Nope") is False + + +def test_get_repo_teams(github): + ret = github.get_repo_teams("repo-a") + assert [team["name"] for team in ret] == ["Team A"] + + +def test_get_repo_teams_missing_repo(github): + with pytest.raises(CommandExecutionError, match="could not be found"): + github.get_repo_teams("ghost") diff --git a/tests/functional/modules/github/test_rulesets.py b/tests/functional/modules/github/test_rulesets.py new file mode 100644 index 00000000..6d5ef910 --- /dev/null +++ b/tests/functional/modules/github/test_rulesets.py @@ -0,0 +1,120 @@ +import pytest +from salt.exceptions import CommandExecutionError + + +def test_list_rulesets_repo(github): + ret = github.list_rulesets(ruleset_type="repo", owner="testorg", repo_name="repo-a") + assert [ruleset["name"] for ruleset in ret["rulesets"]] == ["repo-rules"] + + +def test_list_rulesets_org(github): + ret = github.list_rulesets(ruleset_type="org") + assert [ruleset["name"] for ruleset in ret["rulesets"]] == ["org-rules"] + + +def test_list_rulesets_empty(github): + ret = github.list_rulesets(ruleset_type="repo", owner="testorg", repo_name="repo-b") + assert ret["rulesets"] is None + + +def test_list_rulesets_paginated(github, fake_github, fake_org): + fake_org.add_ruleset("first-extra", enforcement="active") + fake_org.add_ruleset("second-extra", enforcement="active") + ret = github.list_rulesets(ruleset_type="org", per_page=1) + assert {ruleset["name"] for ruleset in ret["rulesets"]} == { + "org-rules", + "first-extra", + "second-extra", + } + pages = [req.query.get("page") for req in fake_github.filter_requests("get", path="*/rulesets")] + assert pages == [None, "2", "3"] + + +def test_get_ruleset(github, fake_org): + repo = fake_org.get_repo("repo-a") + rs_id = next(rs for rs in repo.rulesets).id + ret = github.get_ruleset( + ruleset_type="repo", owner="testorg", repo_name="repo-a", ruleset_id=rs_id + ) + assert ret["name"] == "repo-rules" + assert ret["enforcement"] == "disabled" + + +def test_get_ruleset_org(github, fake_org): + rs_id = fake_org.rulesets["org-rules"].id + ret = github.get_ruleset(ruleset_type="org", ruleset_id=rs_id) + assert ret["name"] == "org-rules" + assert ret["enforcement"] == "active" + + +def test_get_ruleset_missing(github): + ret = github.get_ruleset(ruleset_type="org", ruleset_id=999) + assert ret["result"] is False + assert "404" in ret["comment"] + + +def test_add_ruleset(github, fake_org): + params = {"name": "new-rules", "target": "tag", "enforcement": "active"} + ret = github.add_ruleset(ruleset_type="org", ruleset_params=params) + assert ret["id"] + assert next(rs for rs in fake_org.rulesets if rs.id == ret["id"]).name == "new-rules" + + +def test_add_ruleset_repo(github, fake_org): + params = {"name": "more-rules", "enforcement": "active"} + ret = github.add_ruleset( + ruleset_type="repo", owner="testorg", repo_name="repo-b", ruleset_params=params + ) + assert ret["id"] + assert "more-rules" in fake_org.get_repo("repo-b").rulesets + + +def test_add_ruleset_requires_enforcement(github): + with pytest.raises(CommandExecutionError, match="enforcement"): + github.add_ruleset(ruleset_type="org", ruleset_params={"name": "incomplete"}) + + +def test_add_ruleset_requires_name(github): + with pytest.raises(CommandExecutionError, match="name"): + github.add_ruleset(ruleset_type="org", ruleset_params={"enforcement": "active"}) + + +def test_update_ruleset(github, fake_org): + rs_id = next(rs for rs in fake_org.rulesets).id + ret = github.update_ruleset( + ruleset_type="org", + ruleset_id=rs_id, + ruleset_params={"name": "org-rules", "enforcement": "evaluate"}, + ) + assert ret["enforcement"] == "evaluate" + assert fake_org.get_ruleset(rs_id).enforcement == "evaluate" + + +def test_delete_ruleset(github, fake_org): + repo = fake_org.get_repo("repo-a") + rs_id = next(rs for rs in repo.rulesets).id + ret = github.delete_ruleset( + ruleset_type="repo", owner="testorg", repo_name="repo-a", ruleset_id=rs_id + ) + assert "successfully deleted" in ret["comment"] + assert not list(repo.rulesets) + + +def test_delete_ruleset_missing(github): + ret = github.delete_ruleset(ruleset_type="org", ruleset_id=999) + assert ret["result"] is False + + +def test_list_rulesets_invalid_type(github): + with pytest.raises(CommandExecutionError, match="Invalid ruleset type"): + github.list_rulesets(ruleset_type="bogus") + + +def test_list_rulesets_ignores_non_mapping_header_dict(github): + ret = github.list_rulesets(ruleset_type="org", header_dict="bogus") + assert [ruleset["name"] for ruleset in ret["rulesets"]] == ["org-rules"] + + +def test_add_ruleset_params_must_be_mapping(github): + with pytest.raises(CommandExecutionError, match="need to be a dict"): + github.add_ruleset(ruleset_type="org", ruleset_params="bogus") diff --git a/tests/functional/modules/github/test_teams.py b/tests/functional/modules/github/test_teams.py new file mode 100644 index 00000000..8f982ce5 --- /dev/null +++ b/tests/functional/modules/github/test_teams.py @@ -0,0 +1,239 @@ +from tests.support.github import Team + + +def _raise_api_error(*args, **kwargs): + raise RuntimeError("induced API failure") + + +def test_list_teams(github, fake_github): + ret = github.list_teams() + assert "Team A" in ret + assert ret["Team A"]["slug"] == "team-a" + assert ret["Team A"]["description"] == "Awesome Team: Team A" + assert ret["Team A"]["privacy"] == "closed" + assert ret["Team A"]["permission"] == "pull" + # Rendering the team list must not trigger per-team completion requests + assert [req.path for req in fake_github.requests] == [ + "/orgs/testorg", + "/orgs/testorg/teams", + ] + + +def test_list_teams_uses_cache(github, fake_org): + assert "Team A" in github.list_teams() + fake_org.add_team("Team B") + # Cached result is returned until the cache is bypassed + assert "Team B" not in github.list_teams() + assert "Team B" in github.list_teams(ignore_cache=True) + + +def test_get_team(github): + ret = github.get_team("Team A") + assert ret + assert ret["slug"] == "team-a" + + +def test_get_team_missing(github): + assert not github.get_team("ghost") + + +def test_add_team(github, fake_org): + assert github.add_team("Team B", description="Team B", privacy="closed") is True + created = fake_org.get_team("team-b") + assert created.description == "Team B" + # add_team refreshes the team cache itself + assert github.get_team("Team B") + + +def test_add_team_with_repos(github, fake_org): + assert github.add_team("Team B", repo_names=["testorg/repo-b"]) is True + assert fake_org.get_team("team-b").repos["repo-b"] == "pull" + assert github.list_team_repos("Team B")["repo-b"]["permission"] == "pull" + + +def test_edit_team(github, fake_org): + assert github.edit_team("Team A", description="Renovated") is True + assert fake_org.get_team("team-a").description == "Renovated" + + +def test_remove_team(github, fake_org): + assert "team-a" in [team.slug for team in fake_org.teams] + assert github.remove_team("Team A") is True + assert "team-a" not in [team.slug for team in fake_org.teams] + + +def test_remove_team_missing(github): + assert github.remove_team("ghost") is False + + +def test_add_team_duplicate(github): + # The API refuses to create a team whose slug already exists + assert github.add_team("Team A") is False + + +def test_edit_team_missing(github): + assert github.edit_team("ghost", description="Nope") is False + + +def test_edit_team_privacy_permission(github, fake_org): + assert github.edit_team("Team A", privacy="secret", permission="push") is True + team = fake_org.get_team("team-a") + assert team.privacy == "secret" + assert team.permission == "push" + + +def test_edit_team_stale_cache(github, fake_org): + # A team known to the module cache but deleted on the server side + github.list_teams() + fake_org.remove_team("team-a") + assert github.edit_team("Team A", description="Nope") is False + + +def test_remove_team_stale_cache(github, fake_org): + github.list_teams() + fake_org.remove_team("team-a") + assert github.remove_team("Team A") is False + + +def test_list_team_repos(github): + ret = github.list_team_repos("Team A") + assert set(ret) == {"repo-a"} + assert ret["repo-a"]["permission"] == "pull" + + +def test_add_team_repo(github, fake_org): + assert github.add_team_repo("repo-b", "Team A", permission="push") is True + assert fake_org.get_team("team-a").repos["repo-b"] == "push" + assert github.list_team_repos("Team A")["repo-b"]["permission"] == "push" + + +def test_remove_team_repo(github, fake_org): + assert github.remove_team_repo("repo-a", "Team A") is True + assert "repo-a" not in fake_org.get_team("team-a").repos + + +def test_list_team_members(github): + assert github.list_team_members("Team A") == ["user1"] + + +def test_is_team_member(github): + assert github.is_team_member("user1", "Team A") is True + assert github.is_team_member("user2", "Team A") is False + + +def test_add_team_member(github, fake_org): + assert github.add_team_member("user2", "Team A") is True + assert "user2" in fake_org.get_team("team-a").data.members + + +def test_remove_team_member(github, fake_org): + assert github.remove_team_member("user1", "Team A") is True + assert "user1" not in fake_org.get_team("team-a").members + + +def test_list_team_repos_missing_team(github): + assert github.list_team_repos("ghost") is False + + +def test_list_team_repos_uses_cache(github, fake_org): + assert set(github.list_team_repos("Team A")) == {"repo-a"} + fake_org.get_team("team-a").add_repo("repo-b", "admin") + # Cached result is returned until the cache is bypassed + assert set(github.list_team_repos("Team A")) == {"repo-a"} + refreshed = github.list_team_repos("Team A", ignore_cache=True) + assert set(refreshed) == {"repo-a", "repo-b"} + assert refreshed["repo-b"]["permission"] == "admin" + + +def test_list_team_repos_stale_cache(github, fake_org): + # A team known to the module cache but deleted on the server side + github.list_teams() + fake_org.remove_team("team-a") + assert github.list_team_repos("Team A") == {} + + +def test_list_team_members_missing_team(github): + assert github.list_team_members("ghost") is False + + +def test_list_team_members_stale_cache(github, fake_org): + github.list_teams() + fake_org.remove_team("team-a") + assert github.list_team_members("Team A") == [] + + +def test_is_team_member_missing_team(github): + assert github.is_team_member("user1", "ghost") is False + + +def test_list_team_members_uses_cache(github, fake_org): + assert github.list_team_members("Team A") == ["user1"] + fake_org.get_team("team-a").add_member("user2") + # Cached result is returned until the cache is bypassed + assert github.list_team_members("Team A") == ["user1"] + assert sorted(github.list_team_members("Team A", ignore_cache=True)) == ["user1", "user2"] + + +def test_add_team_repo_missing_team(github): + assert github.add_team_repo("repo-a", "ghost") is False + + +def test_add_team_repo_missing_repo(github): + assert github.add_team_repo("ghost", "Team A") is False + + +def test_add_team_repo_api_failure(github, monkeypatch): + # The fake responds with an error status when the operation raises + monkeypatch.setattr(Team, "add_repo", _raise_api_error) + assert github.add_team_repo("repo-b", "Team A", permission="push") is False + + +def test_remove_team_repo_missing_team(github): + assert github.remove_team_repo("repo-a", "ghost") is False + + +def test_remove_team_repo_missing_repo(github): + assert github.remove_team_repo("ghost", "Team A") is False + + +def test_add_team_member_missing_team(github): + assert github.add_team_member("user1", "ghost") is False + + +def test_add_team_member_missing_user(github): + assert github.add_team_member("ghost", "Team A") is False + + +def test_add_team_member_api_failure(github, monkeypatch): + monkeypatch.setattr(Team, "add_member", _raise_api_error) + assert github.add_team_member("user2", "Team A") is False + + +def test_remove_team_member_not_removed(github, monkeypatch): + # The API reports success, but the member is still present afterwards + monkeypatch.setattr(Team, "remove_member", lambda *args, **kwargs: None) + assert github.remove_team_member("user1", "Team A") is False + + +def test_remove_team_member_missing_team(github): + assert github.remove_team_member("user1", "ghost") is False + + +def test_remove_team_member_missing_user(github): + assert github.remove_team_member("ghost", "Team A") is False + + +def test_list_members_without_mfa_uses_cache(github, fake_state): + assert github.list_members_without_mfa() == ["user2"] + fake_state.get_user("user1").data.two_factor_authentication = False + # Cached result is returned until the cache is bypassed + assert github.list_members_without_mfa() == ["user2"] + assert sorted(github.list_members_without_mfa(ignore_cache=True)) == ["user1", "user2"] + + +def test_list_members_without_mfa(github, fake_github): + assert github.list_members_without_mfa() == ["user2"] + member_queries = [ + req.query["filter"] for req in fake_github.filter_requests(path="*/members", query="filter") + ] + assert "2fa_disabled" in member_queries diff --git a/tests/functional/modules/github/test_users.py b/tests/functional/modules/github/test_users.py new file mode 100644 index 00000000..3c01a2e4 --- /dev/null +++ b/tests/functional/modules/github/test_users.py @@ -0,0 +1,59 @@ +def test_list_users(github): + assert sorted(github.list_users()) == ["user1", "user2"] + + +def test_list_users_uses_cache(github, fake_org): + assert sorted(github.list_users()) == ["user1", "user2"] + fake_org.add_member("newuser") + # Cached result is returned until the cache is bypassed + assert sorted(github.list_users()) == ["user1", "user2"] + assert sorted(github.list_users(ignore_cache=True)) == ["newuser", "user1", "user2"] + + +def test_get_user_member_shortcut(github): + assert github.get_user("user1") is True + + +def test_get_user_details_member(github): + ret = github.get_user("user1", user_details=True) + assert ret["login"] == "user1" + assert ret["company"] == "Example Corp" + assert ret["in_org"] is True + assert ret["membership_state"] == "active" + + +def test_get_user_details_nonmember(github): + ret = github.get_user("newuser", user_details=True) + assert ret["in_org"] is False + assert ret["membership_state"] == "nonexistent" + + +def test_get_user_unknown(github): + assert github.get_user("ghost", user_details=True) is False + + +def test_add_user(github, fake_org): + assert github.add_user("newuser") is True + assert fake_org.get_member("newuser").state == "pending" + ret = github.get_user("newuser", user_details=True) + assert ret["membership_state"] == "pending" + assert ret["in_org"] is False + + +def test_remove_user(github, fake_org): + assert github.remove_user("user2") is True + assert "user2" not in [member.data.login for member in fake_org.members] + assert sorted(github.list_users(ignore_cache=True)) == ["user1"] + + +def test_remove_user_not_a_member(github): + # Idempotent: the user is not a member afterwards + assert github.remove_user("newuser") is True + + +def test_add_user_unknown(github): + assert github.add_user("ghost") is False + + +def test_remove_user_unknown(github): + assert github.remove_user("ghost") is False diff --git a/tests/functional/states/github/__init__.py b/tests/functional/states/github/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/tests/functional/states/github/conftest.py b/tests/functional/states/github/conftest.py new file mode 100644 index 00000000..ddc34451 --- /dev/null +++ b/tests/functional/states/github/conftest.py @@ -0,0 +1,11 @@ +import pytest + + +@pytest.fixture +def github(states): + return states.github + + +@pytest.fixture(params=(False, True)) +def testmode(request): + return request.param diff --git a/tests/functional/states/github/test_repo.py b/tests/functional/states/github/test_repo.py new file mode 100644 index 00000000..d5670cea --- /dev/null +++ b/tests/functional/states/github/test_repo.py @@ -0,0 +1,127 @@ +import pytest + +from tests.support.github import NotFound + + +@pytest.fixture +def _instant_sleep(monkeypatch): + # repo_present waits for repo creation to settle; pointless against the fake + monkeypatch.setattr("time.sleep", lambda _seconds: None) + + +def test_repo_present_create(github, fake_github, fake_org, _instant_sleep, testmode): + before = len(fake_github.mutations()) + ret = github.repo_present(name="new-repo", description="Fancy", test=testmode) + assert ret.result is not False + assert (ret.result is None) is testmode + assert "repo" in ret.changes + assert (len(fake_github.mutations()) == before) is testmode + if testmode: + with pytest.raises(NotFound): + fake_org.get_repo("new-repo") + else: + assert fake_org.get_repo("new-repo").description == "Fancy" + + +def test_repo_present_idempotent(github, testmode): + ret = github.repo_present(name="repo-a", description="Test repo repo-a", test=testmode) + assert ret.result is True + assert not ret.changes + + +def test_repo_present_converges_drift(github, fake_github, fake_org, testmode): + before = len(fake_github.mutations()) + ret = github.repo_present(name="repo-a", description="Managed description", test=testmode) + assert ret.result is not False + assert (ret.result is None) is testmode + assert "repo" in ret.changes + assert (len(fake_github.mutations()) == before) is testmode + expected = "Test repo repo-a" if testmode else "Managed description" + assert fake_org.get_repo("repo-a").description == expected + if not testmode: + rerun = github.repo_present(name="repo-a", description="Managed description") + assert rerun.result is True + assert not rerun.changes + + +def test_repo_present_create_with_teams(github, fake_github, fake_org, _instant_sleep, testmode): + before = len(fake_github.mutations()) + ret = github.repo_present(name="new-repo", teams={"Team A": "push"}, test=testmode) + assert ret.result is not False + assert (ret.result is None) is testmode + assert "repo" in ret.changes + assert "Team A" in ret.changes + assert (len(fake_github.mutations()) == before) is testmode + if testmode: + assert "new-repo" not in fake_org.repos + else: + assert fake_org.get_team("team-a").repos["new-repo"] == "push" + + +def test_repo_present_adds_repo_to_team(github, fake_github, fake_org, testmode): + before = len(fake_github.mutations()) + ret = github.repo_present(name="repo-b", teams={"Team A": "push"}, test=testmode) + assert ret.result is not False + assert (ret.result is None) is testmode + assert "Team A" in ret.changes + assert (len(fake_github.mutations()) == before) is testmode + team = fake_org.get_team("team-a") + if testmode: + assert "repo-b" not in team.repos + else: + assert team.repos["repo-b"] == "push" + + +def test_repo_present_team_permission_change(github, fake_github, fake_org, testmode): + before = len(fake_github.mutations()) + ret = github.repo_present(name="repo-a", teams={"Team A": "push"}, test=testmode) + assert ret.result is not False + assert (ret.result is None) is testmode + assert "Team A" in ret.changes + assert (len(fake_github.mutations()) == before) is testmode + expected = "pull" if testmode else "push" + assert fake_org.get_team("team-a").repos["repo-a"] == expected + + +def test_repo_present_team_permission_unchanged(github, fake_github, fake_org, testmode): + ret = github.repo_present(name="repo-a", teams={"Team A": "pull"}, test=testmode) + assert ret.result is True + assert not ret.changes + assert not fake_github.mutations() + assert fake_org.get_team("team-a").repos["repo-a"] == "pull" + + +def test_repo_present_team_removal(github, fake_github, fake_org, testmode): + # An empty teams dict removes the repo from unlisted teams + before = len(fake_github.mutations()) + ret = github.repo_present(name="repo-a", teams={}, test=testmode) + assert ret.result is not False + assert (ret.result is None) is testmode + assert "Team A" in ret.changes + assert (len(fake_github.mutations()) == before) is testmode + assert ("repo-a" in fake_org.get_team("team-a").repos) is testmode + + +def test_repo_present_missing_team(github): + ret = github.repo_present(name="repo-b", teams={"Ghost": "pull"}) + assert ret.result is False + assert "Failed" in ret.comment + + +def test_repo_absent(github, fake_github, fake_org, testmode): + before = len(fake_github.mutations()) + ret = github.repo_absent(name="repo-a", test=testmode) + assert ret.result is not False + assert (ret.result is None) is testmode + assert (len(fake_github.mutations()) == before) is testmode + assert ret.changes == {"old": "Repo repo-a exists", "new": "Repo repo-a deleted"} + if testmode: + assert "will be deleted" in ret.comment + fake_org.get_repo("repo-a") + else: + with pytest.raises(NotFound): + fake_org.get_repo("repo-a") + + rerun = github.repo_absent(name="repo-a") + assert rerun.result is True + assert not rerun.changes diff --git a/tests/functional/states/github/test_ruleset.py b/tests/functional/states/github/test_ruleset.py new file mode 100644 index 00000000..f0e33c21 --- /dev/null +++ b/tests/functional/states/github/test_ruleset.py @@ -0,0 +1,157 @@ +import pytest + + +@pytest.fixture +def repo_ruleset_args(): + return { + "ruleset_type": "repo", + "owner": "testorg", + "repo_name": "repo-a", + } + + +def test_ruleset_present_idempotent(github, repo_ruleset_args, testmode): + ret = github.ruleset_present( + name="repo-rules", + ruleset_params={"target": "branch", "enforcement": "disabled"}, + test=testmode, + **repo_ruleset_args, + ) + assert ret.result is True + assert not ret.changes + assert "present" in ret.comment + + +def test_ruleset_present_ignores_server_populated_fields(github, repo_ruleset_args, testmode): + # id/created_at/source etc. in the server response must not register + # as drift when only managed params are compared + ret = github.ruleset_present( + name="repo-rules", + ruleset_params={"enforcement": "disabled"}, + test=testmode, + **repo_ruleset_args, + ) + assert ret.result is True + assert not ret.changes + + +def test_ruleset_present_converges_drift( + github, fake_github, fake_org, repo_ruleset_args, testmode +): + before = len(fake_github.mutations()) + ret = github.ruleset_present( + name="repo-rules", + ruleset_params={"target": "branch", "enforcement": "active"}, + test=testmode, + **repo_ruleset_args, + ) + assert ret.result is not False + assert (ret.result is None) is testmode + assert ("will be" in ret.comment) is testmode + assert ret.changes["old"] == {"enforcement": "disabled"} + assert ret.changes["new"] == {"enforcement": "active"} + assert (len(fake_github.mutations()) == before) is testmode + repo = fake_org.get_repo("repo-a") + expected = "disabled" if testmode else "active" + assert repo.rulesets["repo-rules"].enforcement == expected + if not testmode: + rerun = github.ruleset_present( + name="repo-rules", + ruleset_params={"target": "branch", "enforcement": "active"}, + **repo_ruleset_args, + ) + assert rerun.result is True + assert not rerun.changes + + +def test_ruleset_present_create(github, fake_github, fake_org, testmode): + before = len(fake_github.mutations()) + ret = github.ruleset_present( + name="new-rules", + ruleset_type="org", + ruleset_params={"target": "branch", "enforcement": "active"}, + test=testmode, + ) + assert ret.result is not False + assert (ret.result is None) is testmode + assert ("will be" in ret.comment) is testmode + assert ret.changes + assert (len(fake_github.mutations()) == before) is testmode + assert ("new-rules" in fake_org.rulesets) is not testmode + + +def test_ruleset_absent(github, fake_github, fake_org, testmode): + before = len(fake_github.mutations()) + ret = github.ruleset_absent(name="org-rules", ruleset_type="org", test=testmode) + assert ret.result is not False + assert (ret.result is None) is testmode + assert (len(fake_github.mutations()) == before) is testmode + assert bool(list(fake_org.rulesets)) is testmode + assert ret.changes == {"old": "ruleset org-rules exists", "new": "ruleset org-rules deleted"} + if testmode: + assert "will be deleted" in ret.comment + else: + rerun = github.ruleset_absent(name="org-rules", ruleset_type="org") + assert rerun.result is True + assert not rerun.changes + + +def test_ruleset_absent_missing(github, testmode): + ret = github.ruleset_absent(name="ghost-rules", ruleset_type="org", test=testmode) + assert ret.result is True + assert not ret.changes + + +def test_ruleset_present_invalid_type(github): + ret = github.ruleset_present( + name="x", ruleset_type="bogus", ruleset_params={"enforcement": "active"} + ) + assert ret.result is False + assert "ruleset_type" in ret.comment + + +def test_ruleset_absent_invalid_type(github): + ret = github.ruleset_absent(name="x", ruleset_type="bogus") + assert ret.result is False + assert "ruleset_type" in ret.comment + + +def test_ruleset_present_list_error(github): + ret = github.ruleset_present( + name="x", + ruleset_type="repo", + owner="testorg", + repo_name="ghost", + ruleset_params={"enforcement": "active"}, + ) + assert ret.result is False + assert "error listing rulesets" in ret.comment + + +def test_ruleset_absent_list_error(github): + ret = github.ruleset_absent(name="x", ruleset_type="repo", owner="testorg", repo_name="ghost") + assert ret.result is False + assert "error listing rulesets" in ret.comment + + +def test_ruleset_present_update_failure(github, repo_ruleset_args): + # Parameters unknown to the API are rejected with a validation error + ret = github.ruleset_present( + name="repo-rules", + ruleset_params={"enforcement": "disabled", "bogus_param": True}, + **repo_ruleset_args, + ) + assert ret.result is False + assert "Could not update ruleset" in ret.comment + assert not ret.changes + + +def test_ruleset_present_create_failure(github): + ret = github.ruleset_present( + name="fresh-rules", + ruleset_type="org", + ruleset_params={"enforcement": "active", "bogus_param": True}, + ) + assert ret.result is False + assert "could not be added" in ret.comment + assert not ret.changes diff --git a/tests/functional/states/github/test_team.py b/tests/functional/states/github/test_team.py new file mode 100644 index 00000000..6260a93b --- /dev/null +++ b/tests/functional/states/github/test_team.py @@ -0,0 +1,217 @@ +import pytest + +from tests.support.github import NotFound + + +def test_team_present_create(github, fake_github, fake_org, testmode): + before = len(fake_github.mutations()) + ret = github.team_present( + name="team-b", description="Team B", privacy="closed", members={"user1": {}}, test=testmode + ) + assert ret.result is not False + assert (ret.result is None) is testmode + assert (len(fake_github.mutations()) == before) is testmode + assert "team" in ret.changes + if testmode: + # member changes are not projected for new teams + assert "set to be created" in ret.comment + assert "team-b" not in fake_org.teams + else: + assert "user1" in ret.changes + created = fake_org.get_team("team-b") + assert set(created.data.members) == {"user1"} + + +def test_team_present_idempotent(github, testmode): + ret = github.team_present( + name="Team A", + description="Awesome Team: Team A", + privacy="closed", + permission="pull", + members={"user1": {}}, + test=testmode, + ) + assert ret.result is True + assert not ret.changes + + +def test_team_present_converges_drift(github, fake_github, fake_org, testmode): + before = len(fake_github.mutations()) + ret = github.team_present( + name="Team A", + description="Updated A", + privacy="closed", + permission="pull", + members={"user2": {}}, + test=testmode, + ) + assert ret.result is not False + assert (ret.result is None) is testmode + assert (len(fake_github.mutations()) == before) is testmode + assert "team" in ret.changes + assert "user1" in ret.changes # removed (managed members) + assert "user2" in ret.changes # added + team = fake_org.get_team("team-a") + if testmode: + assert "set to be edited" in ret.comment + assert "User user2 set to be added" in ret.comment + assert "User user1 set to be removed" in ret.comment + assert team.description == "Awesome Team: Team A" + assert set(team.data.members) == {"user1"} + else: + assert team.description == "Updated A" + assert set(team.data.members) == {"user2"} + + +def test_team_present_permission_privacy_drift(github, fake_github, fake_org, testmode): + before = len(fake_github.mutations()) + ret = github.team_present( + name="Team A", + description="Awesome Team: Team A", + privacy="secret", + permission="push", + test=testmode, + ) + assert ret.result is not False + assert (ret.result is None) is testmode + assert (len(fake_github.mutations()) == before) is testmode + assert "team" in ret.changes + team = fake_org.get_team("team-a") + if testmode: + assert team.permission == "pull" + assert team.privacy == "closed" + else: + assert team.permission == "push" + assert team.privacy == "secret" + + +def test_team_present_create_failure(github): + # "team a" is not in the team cache (keyed by name), but its slug + # collides with the existing "Team A" on creation + ret = github.team_present(name="team a") + assert ret.result is False + assert "Failed to create team" in ret.comment + + +def test_team_present_manages_repos(github, fake_github, fake_org, testmode): + before = len(fake_github.mutations()) + ret = github.team_present( + name="Team A", + description="Awesome Team: Team A", + privacy="closed", + repo_names=["repo-b"], + test=testmode, + ) + assert ret.result is not False + assert (ret.result is None) is testmode + assert (len(fake_github.mutations()) == before) is testmode + assert "repo-b" in ret.changes # added + assert "repo-a" in ret.changes # removed (not listed) + team = fake_org.get_team("team-a") + if testmode: + assert "repos added" in ret.comment + assert "repos removed" in ret.comment + assert "repo-a" in team.repos + assert "repo-b" not in team.repos + else: + assert "repo-b" in team.repos + assert "repo-a" not in team.repos + + +def test_team_present_add_repo_failure(github): + ret = github.team_present( + name="Team A", + description="Awesome Team: Team A", + privacy="closed", + repo_names=["ghost"], + ) + assert ret.result is False + assert "Failed to add repo ghost" in ret.comment + + +def test_team_present_add_member_failure(github): + ret = github.team_present( + name="Team A", + description="Awesome Team: Team A", + privacy="closed", + members={"user1": {}, "ghost": {}}, + ) + assert ret.result is False + assert "Failed to add user ghost" in ret.comment + + +def test_team_present_enforce_mfa_blocks_addition(github, fake_github, fake_org, testmode): + # user2 has no MFA and is not in the team: they are not added, which + # test mode explains in a comment + before = len(fake_github.mutations()) + ret = github.team_present( + name="Team A", + description="Awesome Team: Team A", + privacy="closed", + permission="pull", + members={"user1": {}, "user2": {}}, + enforce_mfa=True, + test=testmode, + ) + assert ret.result is True # nothing else changes in either mode + assert len(fake_github.mutations()) == before + assert set(fake_org.get_team("team-a").data.members) == {"user1"} + assert ("will not be added to the team because they do not have MFA" in ret.comment) is testmode + + +def test_team_present_enforce_mfa_removal(github, fake_github, fake_org, testmode): + # user2 has no MFA and is already in the team + fake_org.get_team("team-a").add_member("user2") + before = len(fake_github.mutations()) + ret = github.team_present( + name="Team A", + description="Awesome Team: Team A", + privacy="closed", + permission="pull", + members={"user1": {}, "user2": {}}, + enforce_mfa=True, + test=testmode, + ) + assert ret.result is not False + assert (ret.result is None) is testmode + assert (len(fake_github.mutations()) == before) is testmode + assert "due to MFA violation" in ret.changes["user2"]["new"] + team = fake_org.get_team("team-a") + if testmode: + assert "set to be removed from the team because they do not have MFA" in ret.comment + assert "user2" in team.members + else: + assert "user2" not in team.members + + +def test_team_present_mfa_exempt_member(github, fake_org): + # mfa_exempt members are added even when MFA is enforced + ret = github.team_present( + name="Team A", + description="Awesome Team: Team A", + privacy="closed", + members={"user1": {}, "user2": {"mfa_exempt": True}}, + enforce_mfa=True, + ) + assert ret.result is True + assert "user2" in ret.changes + assert "user2" in fake_org.get_team("team-a").members + + +def test_team_absent(github, fake_github, fake_org, testmode): + before = len(fake_github.mutations()) + ret = github.team_absent(name="Team A", test=testmode) + assert ret.result is not False + assert (ret.result is None) is testmode + assert (len(fake_github.mutations()) == before) is testmode + assert ret.changes == {"old": "Team Team A exists", "new": "Team Team A deleted"} + if testmode: + assert "will be deleted" in ret.comment + fake_org.get_team("team-a") + else: + with pytest.raises(NotFound): + fake_org.get_team("team-a") + + rerun = github.team_absent(name="Team A") + assert rerun.result is True + assert not rerun.changes diff --git a/tests/functional/states/github/test_user.py b/tests/functional/states/github/test_user.py new file mode 100644 index 00000000..163a04a2 --- /dev/null +++ b/tests/functional/states/github/test_user.py @@ -0,0 +1,66 @@ +import pytest + +from tests.support.github import NotFound + + +def test_present_invites_user(github, fake_github, fake_org, testmode): + before = len(fake_github.mutations()) + ret = github.present(name="newuser", test=testmode) + assert ret.result is not False + assert (ret.result is None) is testmode + assert (len(fake_github.mutations()) == before) is testmode + assert ret.changes == {"old": None, "new": "User newuser exists in the org now"} + if testmode: + assert "will be added" in ret.comment + with pytest.raises(NotFound): + fake_org.get_member("newuser") + else: + assert fake_org.get_member("newuser").state == "pending" + + rerun = github.present(name="newuser") + assert rerun.result is True + assert not rerun.changes + assert "already been invited" in rerun.comment + + +def test_present_existing_member(github, testmode): + ret = github.present(name="user1", test=testmode) + assert ret.result is True + assert not ret.changes + assert "already in the org" in ret.comment + + +def test_present_unknown_user(github, testmode): + ret = github.present(name="ghost", test=testmode) + assert ret.result is False + + +def test_absent_removes_member(github, fake_github, fake_org, testmode): + before = len(fake_github.mutations()) + ret = github.absent(name="user2", test=testmode) + assert ret.result is not False + assert (ret.result is None) is testmode + assert (len(fake_github.mutations()) == before) is testmode + assert ret.changes == {"old": "User user2 exists", "new": "User user2 deleted"} + if testmode: + assert "will be deleted" in ret.comment + fake_org.get_member("user2") + else: + with pytest.raises(NotFound): + fake_org.get_member("user2") + + rerun = github.absent(name="user2") + assert rerun.result is True + assert not rerun.changes + + +def test_absent_nonmember(github, testmode): + ret = github.absent(name="newuser", test=testmode) + assert ret.result is True + assert not ret.changes + + +def test_absent_unknown_user(github, testmode): + ret = github.absent(name="ghost", test=testmode) + assert ret.result is True + assert "does not exist" in ret.comment diff --git a/tests/support/__init__.py b/tests/support/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/tests/support/github/__init__.py b/tests/support/github/__init__.py new file mode 100644 index 00000000..06f3c0f8 --- /dev/null +++ b/tests/support/github/__init__.py @@ -0,0 +1,79 @@ +""" +A stateful fake GitHub API server for functional tests. + +The server implements the subset of the GitHub REST API that this extension +consumes, backed by plain dicts. It allows exercising the execution and state +modules end-to-end (idempotence, drift detection, test mode) without network +access or mocking individual functions. + +Layout: + +:mod:`~tests.support.github.entities` + Typed entity model (``*Data`` storage dataclasses + hydrated wrappers), + ``EntityView``, ``NotFound``. +:mod:`~tests.support.github.state` + ``GlobalState`` — the dict-backed store with snapshot/restore. +:mod:`~tests.support.github.routing` + Route declaration decorators (``GET``/``POST``/...) and ``Request``. +:mod:`~tests.support.github.base` + ``APIBase`` — dispatch, shared resolvers, request log, HTTP plumbing. +:mod:`~tests.support.github.routes` + Per-area route mixins composed into :class:`FakeGitHubAPI` below. +""" + +from tests.support.github.base import ORG +from tests.support.github.base import TOKEN +from tests.support.github.entities import Entity +from tests.support.github.entities import EntityData +from tests.support.github.entities import EntityView +from tests.support.github.entities import Issue +from tests.support.github.entities import IssueComment +from tests.support.github.entities import Member +from tests.support.github.entities import Milestone +from tests.support.github.entities import NotFound +from tests.support.github.entities import Organization +from tests.support.github.entities import PullRequest +from tests.support.github.entities import Repo +from tests.support.github.entities import RepoId +from tests.support.github.entities import Ruleset +from tests.support.github.entities import Team +from tests.support.github.entities import TeamMember +from tests.support.github.entities import User +from tests.support.github.routes import IssueRoutes +from tests.support.github.routes import MilestoneRoutes +from tests.support.github.routes import OrgRoutes +from tests.support.github.routes import RepoRoutes +from tests.support.github.routes import RulesetRoutes +from tests.support.github.routes import TeamRoutes +from tests.support.github.routing import Request +from tests.support.github.state import GlobalState + +__all__ = [ + "ORG", + "TOKEN", + "Entity", + "EntityData", + "EntityView", + "FakeGitHubAPI", + "GlobalState", + "Issue", + "IssueComment", + "Member", + "Milestone", + "NotFound", + "Organization", + "PullRequest", + "Repo", + "RepoId", + "Request", + "Ruleset", + "Team", + "TeamMember", + "User", +] + + +class FakeGitHubAPI(OrgRoutes, RepoRoutes, TeamRoutes, RulesetRoutes, MilestoneRoutes, IssueRoutes): + """ + Dict-backed implementation of the consumed GitHub API surface. + """ diff --git a/tests/support/github/base.py b/tests/support/github/base.py new file mode 100644 index 00000000..0f771953 --- /dev/null +++ b/tests/support/github/base.py @@ -0,0 +1,250 @@ +""" +Request dispatch, shared route helpers and HTTP plumbing of the fake GitHub API. +""" + +import fnmatch +import json +import re +import threading +import traceback +from collections.abc import Callable +from contextlib import contextmanager +from http.server import BaseHTTPRequestHandler +from http.server import ThreadingHTTPServer +from urllib.parse import parse_qs +from urllib.parse import urlsplit + +from tests.support.github.entities import Organization +from tests.support.github.entities import Repo +from tests.support.github.routing import HTTP_METHOD +from tests.support.github.routing import Request +from tests.support.github.state import GlobalState + +TOKEN = "test-token" +ORG = "testorg" + + +class APIBase: + """ + Request dispatch and shared helpers. Routes are contributed by the + mixin classes in :mod:`tests.support.github.routes` and discovered + via the MRO on instantiation. + """ + + def __init__(self, token: str = TOKEN, state: GlobalState | None = None): + self.token = token + self.base_url = None + self.lock = threading.Lock() + self.requests = [] + self.state = state or GlobalState() + self._server = None + self._routes: dict[HTTP_METHOD, list[tuple[tuple[re.Pattern[str], ...], Callable]]] = {} + for name in dir(self): + if (routes := getattr(func := getattr(self, name), "routes", None)) is not None: + self._routes.setdefault(routes[0], []).append((routes[1], func)) + + # ------------------------------------------------------------------ + # State management + # ------------------------------------------------------------------ + + def reset(self): + """ + Restore the seeded state and clear the request log. + """ + with self.lock: + self.requests = [] + self.state.restore() + + # ------------------------------------------------------------------ + # Shared route helpers + # ------------------------------------------------------------------ + + def _resolve_org(self, match: re.Match | dict[str, str]) -> Organization: + if isinstance(match, re.Match): + groups = match.groupdict() + else: + groups = match + if (org := groups.get("org")) is not None: + func, identifier = self.state.get_org, org + elif (org_id := groups.get("org_id")) is not None: + func, identifier = self.state.get_org_id, org_id + else: + raise TypeError(f"no org or org_id specified: {groups}") # pragma: no cover + return func(identifier) + + def _resolve_repo(self, match: re.Match | dict[str, str]) -> Repo: + if isinstance(match, re.Match): + groups = match.groupdict() + else: + groups = match + if "repo" not in groups: + raise TypeError(f"no repo specified: {groups}") # pragma: no cover + org = self._resolve_org(groups) + return org.get_repo(groups["repo"]) + + # ------------------------------------------------------------------ + # Dispatch + # ------------------------------------------------------------------ + + def dispatch(self, method, path, query, body, auth): + """ + Route a request. Returns ``(status, headers, payload)``. + """ + with self.lock: + self.requests.append(Request(method, path, query, body)) + if auth not in (f"token {self.token}", f"Bearer {self.token}"): # pragma: no cover + return 401, {}, {"message": "Bad credentials"} + for patterns, func in self._routes.get(method, ()): + for route in patterns: + if match := route.fullmatch(path): + break + else: + continue + try: + return self._chunk(query, path, self._normalize(func(match, query, body))) + except KeyError as err: + return ( + 404, + {}, + {"message": f"Not Found: {err}\n\n{traceback.format_exception(err)}"}, + ) + except Exception as err: # pylint: disable=broad-except # pragma: no cover + return ( + 422, + {}, + { + "message": f"Fake server error: {err!r}\n\n{traceback.format_exception(err)}" + }, + ) + return ( # pragma: no cover + 404, + {}, + {"message": f"Not Found: no route for {method} {path}"}, + ) + + @staticmethod + def _normalize(result): + if isinstance(result, int): + return result, {}, None + if len(result) == 2: + status, payload = result + return status, {}, payload + return result + + def _chunk(self, query, path, result): + status, headers, body = result + if "per_page" not in query or status >= 400 or not isinstance(body, list): + return result + status, headers, body = result + per_page = int(query["per_page"]) + page = int(query.get("page", 1)) + start = (page - 1) * per_page + chunk = body[start : start + per_page] + if start + per_page < len(body): + headers["Link"] = ( + f"<{self.base_url}{path}?per_page={per_page}&page={page + 1}>; " f'rel="next"' + ) + elif page > 1: + # Like the real API, later pages always carry a Link header, + # even when there is no next page. + headers["Link"] = ( + f"<{self.base_url}{path}?per_page={per_page}&page={page - 1}>; " f'rel="prev"' + ) + return status, headers, chunk + + # ------------------------------------------------------------------ + # Test helpers + # ------------------------------------------------------------------ + + def filter_requests(self, method=None, path=None, query=None): + """ + Return all logged requests that match a spec. + + method + (List of) HTTP methods to filter logged requests for. + + path + (List of) fnmatch-style globs to filter logged requests for. + + query + (List of) query keys to filter logged requests for. + """ + ret = self.requests + if method is not None: + methods = ( + [method.upper()] if isinstance(method, str) else [meth.upper() for meth in method] + ) + ret = [req for req in ret if req.method in methods] + if path is not None: + paths = [path] if isinstance(path, str) else path + ret = [req for req in ret if any(fnmatch.fnmatch(req.path, ptrn) for ptrn in paths)] + if query is not None: + queries = [query] if isinstance(query, str) else query + ret = [req for req in ret if any(qry in req.query for qry in queries)] + return ret + + def mutations(self): + """ + Return all logged non-GET requests, for asserting test-mode behavior. + """ + return [req for req in self.requests if req.method != "GET"] + + # ------------------------------------------------------------------ + # HTTP server plumbing + # ------------------------------------------------------------------ + + @contextmanager + def serve(self): + """ + Run the server on an ephemeral localhost port. + """ + api = self + + class _Handler(BaseHTTPRequestHandler): + protocol_version = "HTTP/1.1" + + def _handle(self): + length = int(self.headers.get("Content-Length") or 0) + raw = self.rfile.read(length) if length else b"" + body = json.loads(raw) if raw else None + split = urlsplit(self.path) + query = {key: vals[0] for key, vals in parse_qs(split.query).items()} + status, headers, payload = api.dispatch( + self.command, + split.path, + query, + body, + self.headers.get("Authorization"), + ) + data = b"" if payload is None else json.dumps(payload).encode() + self.send_response(status) + for key, val in headers.items(): + self.send_header(key, val) + if data: + self.send_header("Content-Type", "application/json") + self.send_header("Content-Length", str(len(data))) + self.end_headers() + if data: + self.wfile.write(data) + + do_GET = do_POST = do_PATCH = do_PUT = do_DELETE = _handle + + def log_message(self, *args): + pass + + server = ThreadingHTTPServer(("127.0.0.1", 0), _Handler) + server.daemon_threads = True + self._server = server + self.base_url = self.state.base_url = f"http://127.0.0.1:{server.server_address[1]}" + thread = threading.Thread(target=server.serve_forever, daemon=True) + thread.start() + try: + yield self + finally: + server.shutdown() + server.server_close() + thread.join(timeout=5) + + @property + def url(self): + return self.base_url diff --git a/tests/support/github/entities.py b/tests/support/github/entities.py new file mode 100644 index 00000000..51376e5d --- /dev/null +++ b/tests/support/github/entities.py @@ -0,0 +1,973 @@ +""" +Entity model of the fake GitHub API. + +Each entity comes in two flavors: a plain ``*Data`` dataclass holding the +stored state and a hydrated ``Entity`` wrapper that is bound to the +:class:`~tests.support.github.state.GlobalState` and offers navigation, +payload rendering and persistence helpers. + +Payload ``url`` fields are rendered with the server's own base URL since +PyGithub asserts that followed URLs stay within the configured API host. +""" + +# The type-only entities ⇄ state cycle is intentional: entities are annotated +# with the GlobalState they are bound to, while the state stores entity data. +# pyright: reportImportCycles=false + +import zlib +from collections.abc import Callable +from collections.abc import ItemsView +from collections.abc import Iterator +from collections.abc import KeysView +from dataclasses import KW_ONLY +from dataclasses import asdict +from dataclasses import dataclass +from dataclasses import field +from dataclasses import replace +from typing import TYPE_CHECKING +from typing import Any +from typing import Generic +from typing import Literal +from typing import NamedTuple +from typing import TypeAlias +from typing import TypeVar + +if TYPE_CHECKING: + from tests.support.github.state import GlobalState + +CREATED_AT = "2026-10-11T13:37:42Z" + +OrgName: TypeAlias = str +RepoName: TypeAlias = str +TeamName: TypeAlias = str +UserName: TypeAlias = str + +ActorName: TypeAlias = UserName | OrgName + +K = TypeVar("K") +V = TypeVar("V") + + +class NotFound(KeyError): + """ + Raised for lookups of entities that don't exist. + Routes translate it (or any ``KeyError``) into a 404 response. + """ + + +class RepoId(NamedTuple): + owner: ActorName + name: RepoName + + def __str__(self): + return f"{self.owner}/{self.name}" + + @classmethod + def from_str(cls, inpt): + try: + owner, name = inpt.split("/", maxsplit=1) + except (AttributeError, ValueError) as err: # pragma: no cover + raise TypeError(f"Invalid repo_id: {inpt=}") from err + return cls(owner, name) + + @classmethod + def from_seq(cls, inpt): + try: + owner, name = inpt + except (TypeError, ValueError) as err: # pragma: no cover + raise TypeError(f"Invalid repo_id: {inpt=}") from err + return cls(owner, name) + + +@dataclass +class EntityData: + """ + Base class for entity data. Autogenerates an ID based on login/name/title attribute + and exposes a ``with_`` method to create an updated (detached) object. + + IDs are derived via CRC-32 so they are deterministic across test runs + (``hash()`` is randomized per process). Caveat: renaming an entity hence + changes its ID, unlike real GitHub. + """ + + id: int = field(init=False, repr=False) + + with_ = replace + + def __post_init__(self): + for attr in ("login", "name", "title"): + if key := getattr(self, attr, None): + self.id = zlib.crc32(f"{type(self).__name__}/{key}".encode()) + return + raise RuntimeError("Cannot generate ID, missing login, name or title") # pragma: no cover + + +@dataclass +class Entity: + """ + Base class for interactive entities that resolve to hydrated types and expose payloads. + Wraps ``EntityData``. + """ + + gh: "GlobalState" + data: EntityData + + def payload(self, /, **ovrr): + return asdict(self.data) | ovrr + + def with_(self, **upd): + """ + Return an updated copy. It is detached, i.e. NOT persisted in the + global state — use ``update`` for that. + """ + return replace(self, data=self.data.with_(**upd)) + + def update(self, **upd): + """ + Persist an updated version of this entity in the global state + and return the new entity. + """ + return self.gh.replace(self, self.with_(**upd)) + + def __getattr__(self, key): + return getattr(self.data, key) + + +class EntityView(Generic[K, V]): + """ + Read-only view over a keyed entity collection. + + Iteration yields the collection's *values* (entities or, for team repos, + permissions), while membership checks and item access accept the natural + key (login, name, slug, title). Membership checks also accept an entity. + """ + + def __init__(self, items: dict[K, V], resolve: Callable[[Any], K] | None = None): + self._items = items + self._resolve = resolve + + def _key(self, key: Any) -> K: + if self._resolve is not None and key not in self._items: + return self._resolve(key) + return key + + def __iter__(self) -> Iterator[V]: + return iter(self._items.values()) + + def __len__(self) -> int: + return len(self._items) + + def __contains__(self, key: object) -> bool: + if isinstance(key, Entity): + return any( + key.data is item.data for item in self._items.values() if isinstance(item, Entity) + ) + try: + return self._key(key) in self._items + except TypeError: + return False + + def __getitem__(self, key: Any) -> V: + try: + return self._items[self._key(key)] + except KeyError: + raise NotFound(key) from None + + def get(self, key: Any, default: V | None = None) -> V | None: + try: + return self[key] + except (NotFound, TypeError): + return default + + def keys(self) -> KeysView[K]: + return self._items.keys() + + def items(self) -> ItemsView[K, V]: + return self._items.items() + + def __repr__(self): + return f"{type(self).__name__}({self._items!r})" + + +@dataclass +class OrganizationData(EntityData): + name: str + _: KW_ONLY + login: OrgName = "" + company: str = "Example Corp" + + def __post_init__(self): + if not self.login: + self.login = self.name.lower().replace(" ", "-") + super().__post_init__() + + +@dataclass +class Organization(Entity): + data: OrganizationData + + def payload(self, /, **ovrr): + return ( + asdict(self.data) + | { + "url": f"{self.gh.base_url}/orgs/{self.data.login}", + "html_url": f"https://github.example.com/{self.data.login}", + } + | ovrr + ) + + @property + def members(self) -> "EntityView[UserName, Member]": + org = self.data.login + return EntityView( + { + login: Member(self.gh, member) + for (org_name, login), member in self.gh.members.items() + if org_name == org + } + ) + + @property + def repos(self) -> "EntityView[RepoName, Repo]": + org = self.data.login + return EntityView( + { + repo_id.name: Repo(self.gh, repo) + for repo_id, repo in self.gh.repos.items() + if repo_id.owner == org + } + ) + + @property + def rulesets(self) -> "EntityView[str, Ruleset]": + return EntityView( + { + data.name: Ruleset(self.gh, data) + for data in self.gh.rulesets_org[self.data.login].values() + } + ) + + @property + def teams(self) -> "EntityView[TeamName, Team]": + org = self.data.login + return EntityView( + { + slug: Team(self.gh, team) + for (org_name, slug), team in self.gh.teams.items() + if org_name == org + } + ) + + def add_repo(self, name: RepoName, **params) -> "Repo": + repo_id = RepoId(self.data.login, name) + assert repo_id not in self.gh.repos + repo = RepoData(name, self.data.login, **params) + return self.gh.add(repo) + + def get_repo(self, name: RepoName, *, exc=NotFound) -> "Repo": + return self.gh.get_repo(RepoId(self.data.login, name), exc=exc) + + def remove_repo(self, name: RepoName, *, exc=NotFound) -> None: + repo_id = RepoId(self.data.login, name) + if repo_id not in self.gh.repos: + raise exc(f"Repo {repo_id=} does not exist") + self.gh.repos.pop(repo_id) + for team in self.teams: + team.data.repos.pop(repo_id, None) + + def add_member(self, user: "User | UserData | UserName", *, exc=NotFound, **params) -> "Member": + login = user if isinstance(user, UserName) else user.login + if login not in self.gh.users: + raise exc(f"User {login=} does not exist") + membership_id = (self.data.login, login) + if membership_id in self.gh.members: + return Member(self.gh, self.gh.members[membership_id]) + member = MemberData(login, self.data.login, **params) + return self.gh.add(member) + + def get_member(self, login: str, *, exc=NotFound) -> "Member": + member_key = (self.data.login, login) + if member_key not in self.gh.members: + raise exc(f"Requested nonexistent member {login=} from org {self.data.login}") + return Member(self.gh, self.gh.members[member_key]) + + def remove_member(self, login: str) -> None: + member_key = (self.data.login, login) + self.gh.members.pop(member_key, None) + for team in self.teams: + team.data.members.pop(login, None) + + def active_members(self) -> "list[Member]": + # Pending invitees are not members yet, matching GitHub behavior. + return [member for member in self.members if member.state == "active"] + + def add_team(self, name: str, **params) -> "Team": + team = TeamData(name, self.data.login, **params) + team_key = (self.data.login, team.slug) + assert team_key not in self.gh.teams + return self.gh.add(team) + + def get_team(self, slug: str, *, exc=NotFound) -> "Team": + team_key = (self.data.login, slug) + if team_key not in self.gh.teams: + raise exc(f"Requested nonexistent team {slug=} from org {self.data.login}") + return Team(self.gh, self.gh.teams[team_key]) + + def remove_team(self, slug: str, *, exc=NotFound) -> None: + team_key = (self.data.login, slug) + if team_key not in self.gh.teams: + raise exc(f"Team {slug=} does not exist in org {self.data.login}") + self.gh.teams.pop(team_key) + + def add_ruleset(self, name: str, **params) -> "Ruleset": + ruleset = OrgRulesetData(name, self.data.login, **params) + return self.gh.add(ruleset) + + def get_ruleset(self, rs_id: int | str, *, exc=NotFound) -> "Ruleset": + rs_id = int(rs_id) + if rs_id not in self.gh.rulesets_org[self.data.login]: + raise exc(f"Org ruleset with id {rs_id=} does not exist for org {self.data.login=}") + return Ruleset(self.gh, self.gh.rulesets_org[self.data.login][rs_id]) + + def remove_ruleset(self, rs_id: int | str, *, exc=NotFound) -> None: + rs_id = int(rs_id) + if rs_id not in self.gh.rulesets_org[self.data.login]: + raise exc(f"Org ruleset with id {rs_id=} does not exist for org {self.data.login=}") + del self.gh.rulesets_org[self.data.login][rs_id] + + +@dataclass +class UserData(EntityData): + name: str + _: KW_ONLY + login: UserName = "" + email: str = "" + two_factor_authentication: bool = True + company: str = "Example Corp" + type: str = "User" + created_at: str = CREATED_AT + updated_at: str = CREATED_AT + public_repos: int = 2 + public_gists: int = 1 + followers: int = 42 + following: int = 37 + + def __post_init__(self): + if not self.login: + self.login = self.name.lower().replace(" ", "-") + if not self.email: + self.email = f"{self.login}@example.com" + super().__post_init__() + + +@dataclass +class User(Entity): + data: UserData + + def payload(self, /, **ovrr): + rend = asdict(self.data) + return ( + rend + | { + "url": f"{self.gh.base_url}/users/{self.data.login}", + "html_url": f"https://github.example.com/{self.data.login}", + } + | ovrr + ) + + +@dataclass +class MemberData(EntityData): + login: UserName + org: OrgName + _: KW_ONLY + role: Literal["admin", "member"] = "member" + state: Literal["active", "pending"] = "active" + + +@dataclass +class Member(Entity): + data: MemberData + + @property + def user(self) -> User: + return self.gh.get_user(self.data.login) + + @property + def org(self) -> Organization: + return self.gh.get_org(self.data.org) + + def payload(self, /, **ovrr): + ret = asdict(self.data) + ret.pop("org") + return ( + ret + | { + "url": f"{self.gh.base_url}/orgs/{self.data.org}/memberships/{self.data.login}", + "user": self.user.payload(), + } + | ovrr + ) + + +@dataclass +class TeamMemberData(EntityData): + login: UserName + org: OrgName + team: TeamName + _: KW_ONLY + role: Literal["maintainer", "member"] = "member" + state: Literal["active", "pending"] = "active" + + +@dataclass +class TeamMember(Entity): + data: TeamMemberData + + @property + def user(self) -> User: + return self.gh.get_user(self.data.login) + + @property + def org(self) -> Organization: + return self.gh.get_org(self.data.org) + + @property + def team(self) -> "Team": + return self.org.get_team(self.data.team) + + def payload(self, /, **ovrr): + ret = asdict(self.data) + ret.pop("login") + ret.pop("org") + ret.pop("team") + return ( + ret + | { + "url": ( + f"{self.gh.base_url}/organizations/{self.org.id}" + f"/team/{self.team.id}/memberships/{self.data.login}" + ), + } + | ovrr + ) + + +@dataclass +class RepoData(EntityData): + name: RepoName + owner: ActorName + _: KW_ONLY + owner_type: Literal["org", "user"] = "org" + description: str | None = None + homepage: str | None = None + private: bool = False + fork: bool = False + language: str = "Python" + default_branch: str = "main" + has_issues: bool = True + has_wiki: bool = True + has_downloads: bool = True + + def __post_init__(self): + super().__post_init__() + if self.description is None: + self.description = f"Test repo {self.name}" + + @property + def repo_id(self): + return RepoId(self.owner, self.name) + + @property + def full_name(self): + return f"{self.owner}/{self.name}" + + +@dataclass +class Repo(Entity): + data: RepoData + + @property + def owner(self) -> User | Organization: + if self.data.owner_type == "org": + return self.gh.get_org(self.data.owner) + return self.gh.get_user(self.data.owner) + + @property + def milestones(self) -> "EntityView[str, Milestone]": + return EntityView( + { + data.title: Milestone(self.gh, data) + for data in self.gh.milestones[self.data.repo_id].values() + } + ) + + @property + def rulesets(self) -> "EntityView[str, Ruleset]": + return EntityView( + { + data.name: Ruleset(self.gh, data) + for data in self.gh.rulesets_repo[self.data.repo_id].values() + } + ) + + def add_ruleset(self, name: str, **params) -> "Ruleset": + owner = self.owner + if not isinstance(owner, Organization): + raise NotImplementedError + ruleset = RepoRulesetData(name, self.data.repo_id, **params) + return self.gh.add(ruleset) + + def get_ruleset(self, rs_id: int | str, *, exc=NotFound) -> "Ruleset": + rs_id = int(rs_id) + if rs_id not in self.gh.rulesets_repo[self.data.repo_id]: + raise exc(f"Repo ruleset with id {rs_id} does not exist for repo {self.data.repo_id}") + return Ruleset(self.gh, self.gh.rulesets_repo[self.data.repo_id][rs_id]) + + def remove_ruleset(self, rs_id: int | str, *, exc=NotFound): + rs_id = int(rs_id) + if rs_id not in self.gh.rulesets_repo[self.data.repo_id]: + raise exc(f"Repo ruleset with id {rs_id} does not exist for repo {self.data.repo_id}") + del self.gh.rulesets_repo[self.data.repo_id][rs_id] + + def add_milestone( + self, title: str, creator: User | UserData | UserName, **params + ) -> "Milestone": + owner = self.owner + if not isinstance(owner, Organization): + raise NotImplementedError + milestone = MilestoneData( + title, + self.data.repo_id, + creator if isinstance(creator, UserName) else creator.login, + **params, + ) + self.gh.add(milestone) + return Milestone(self.gh, milestone) + + def get_milestone(self, number: int | str, *, exc=NotFound) -> "Milestone": + number = int(number) + if number not in self.gh.milestones[self.data.repo_id]: + raise exc(f"Milestone with number {number} does not exist for repo {self.data.repo_id}") + return Milestone(self.gh, self.gh.milestones[self.data.repo_id][number]) + + @property + def issues(self) -> "EntityView[str, Issue]": + return EntityView( + { + data.title: Issue(self.gh, data) + for data in self.gh.issues[self.data.repo_id].values() + } + ) + + @property + def pulls(self) -> "EntityView[str, PullRequest]": + return EntityView( + { + data.title: PullRequest(self.gh, data) + for data in self.gh.pulls[self.data.repo_id].values() + } + ) + + def add_issue(self, title: str, creator: User | UserData | UserName, **params) -> "Issue": + issue = IssueData( + title, + self.data.repo_id, + creator if isinstance(creator, UserName) else creator.login, + **params, + ) + return self.gh.add(issue) + + def get_issue(self, number: int | str, *, exc=NotFound) -> "Issue": + number = int(number) + if number not in self.gh.issues[self.data.repo_id]: + raise exc(f"Issue with number {number} does not exist for repo {self.data.repo_id}") + return Issue(self.gh, self.gh.issues[self.data.repo_id][number]) + + def add_pull(self, title: str, creator: User | UserData | UserName, **params) -> "PullRequest": + pull = PullRequestData( + title, + self.data.repo_id, + creator if isinstance(creator, UserName) else creator.login, + **params, + ) + return self.gh.add(pull) + + def get_pull(self, number: int | str, *, exc=NotFound) -> "PullRequest": + number = int(number) + if number not in self.gh.pulls[self.data.repo_id]: + raise exc( + f"Pull request with number {number} does not exist for repo {self.data.repo_id}" + ) + return PullRequest(self.gh, self.gh.pulls[self.data.repo_id][number]) + + def team_payload(self, permission, /, **ovrr): + rend = self.payload() + rend["permissions"] = { + "admin": permission == "admin", + "push": permission in ("push", "admin"), + "pull": True, + } + return rend | ovrr + + def payload(self, /, **ovrr): + rend = super().payload() | { + "full_name": self.data.full_name, + "owner": {"login": self.data.owner}, + "url": f"{self.gh.base_url}/repos/{self.data.full_name}", + "html_url": f"https://github.example.com/{self.data.full_name}", + "size": 100, + "stargazers_count": 1, + "watchers_count": 1, + "watchers": 1, + "forks": 0, + "open_issues": 0, + "open_issues_count": 0, + } + rend.pop("owner_type") + return rend | ovrr + + +@dataclass +class TeamData(EntityData): + name: str + org: OrgName + _: KW_ONLY + slug: TeamName = field(init=False) + description: str | None = None + privacy: str = "closed" + permission: Literal["pull", "push", "admin"] = "pull" + members: dict[UserName, TeamMemberData] = field(default_factory=dict) + repos: dict[RepoId, Literal["pull", "triage", "push", "maintain", "admin"] | str] = field( + default_factory=dict + ) + + def __post_init__(self): + super().__post_init__() + self.slug = self.name.lower().replace(" ", "-") + if self.description is None: + self.description = f"Awesome Team: {self.name}" + + +@dataclass +class Team(Entity): + data: TeamData + + @property + def members(self) -> "EntityView[UserName, TeamMember]": + return EntityView( + {login: TeamMember(self.gh, member) for login, member in self.data.members.items()} + ) + + @property + def repos(self) -> "EntityView[RepoId, str]": + # Keyed by RepoId, but str/(owner, name) keys are resolved as well. + def resolve(key) -> RepoId: + if isinstance(key, str): + if "/" in key: + return RepoId.from_str(key) + return RepoId(self.data.org, key) + return RepoId.from_seq(key) + + return EntityView(self.data.repos, resolve=resolve) + + @property + def org(self) -> "Organization": + return self.gh.get_org(self.data.org) + + def get_member(self, member: User | UserData | UserName) -> TeamMember | None: + login = member if isinstance(member, UserName) else member.data.login + if login not in self.data.members: + return None + return TeamMember(self.gh, self.data.members[login]) + + def add_member(self, member: User | UserData | UserName, **params) -> TeamMember: + login = member if isinstance(member, UserName) else member.login + self.org.add_member(login) # idempotent + membership = TeamMemberData(login=login, org=self.data.org, team=self.data.slug, **params) + self.data.members[login] = membership + return TeamMember(self.gh, membership) + + def remove_member(self, member: User | UserData | UserName): + login = member if isinstance(member, UserName) else member.data.login + self.data.members.pop(login, None) + + def add_repo( + self, + repo: Repo | RepoData | RepoName, + permission: Literal["pull", "triage", "push", "maintain", "admin"] | str = "pull", + *, + exc=NotFound, + ): + if isinstance(repo, RepoName): + repo_id = RepoId(self.data.org, repo) + else: + assert repo.data.owner == self.data.org + repo_id = repo.data.repo_id + if repo_id not in self.gh.repos: + raise exc(f"Repo {repo_id=} does not exist") + self.data.repos[repo_id] = permission or "pull" + + def remove_repo( + self, + repo: Repo | RepoData | RepoName, + ): + if isinstance(repo, RepoName): + repo_id = RepoId(self.data.org, repo) + else: + repo_id = repo.data.repo_id + self.data.repos.pop(repo_id, None) + + def payload(self, /, **ovrr): + rend = asdict(self.data) + rend.pop("members") + rend.pop("repos") + org = self.org + return ( + rend + | { + # Like the real API, advertise the modern by-ID URL + "url": f"{self.gh.base_url}/organizations/{org.id}/team/{self.data.id}", + "html_url": f"https://github.example.com/orgs/{self.data.org}/teams/{self.data.slug}", + "organization": org.payload(), + } + | ovrr + ) + + +@dataclass +class RulesetData(EntityData): + name: str + _: KW_ONLY + enforcement: Literal["disabled", "active", "evaluate"] = "active" + target: Literal["branch", "tag", "push"] = "branch" + created_at: str = CREATED_AT + updated_at: str = CREATED_AT + + +@dataclass +class RepoRulesetData(RulesetData): + source: RepoId + + +@dataclass +class OrgRulesetData(RulesetData): + source: OrgName + + +@dataclass +class Ruleset(Entity): + data: RepoRulesetData | OrgRulesetData + + def source(self) -> Repo | Organization: + if isinstance(self.data.source, RepoId): + return self.gh.get_repo(self.data.source) + return self.gh.get_org(self.data.source) + + def payload(self, /, **ovrr): + rend = asdict(self.data) + rend["source"] = str(rend["source"]) # render RepoID + rend["source_type"] = ( + "Repository" if isinstance(self.data, RepoRulesetData) else "Organization" + ) + return rend | ovrr + + +@dataclass +class IssueCommentData(EntityData): + issue: int + repo: RepoId + creator: UserName + body: str + _: KW_ONLY + created_at: str = CREATED_AT + updated_at: str = CREATED_AT + + def __post_init__(self): + # Comments carry no login/name/title, derive the ID from their contents. + self.id = zlib.crc32(f"{type(self).__name__}/{self.repo}#{self.issue}/{self.body}".encode()) + + +@dataclass +class IssueComment(Entity): + data: IssueCommentData + + @property + def user(self) -> User: + return self.gh.get_user(self.data.creator) + + def payload(self, /, **ovrr): + rend = asdict(self.data) + rend.pop("issue") + rend.pop("repo") + rend.pop("creator") + return ( + rend + | { + "url": f"{self.gh.base_url}/repos/{self.data.repo}/issues/comments/{self.data.id}", + "html_url": ( + f"https://github.example.com/{self.data.repo}/issues/" + f"{self.data.issue}#issuecomment-{self.data.id}" + ), + "user": self.user.payload(), + } + | ovrr + ) + + +@dataclass +class IssueData(EntityData): + title: str + repo: RepoId + creator: UserName + number: int = -1 + _: KW_ONLY + state: str = "open" + body: str = "" + labels: list[str] = field(default_factory=list) + assignee: UserName | None = None + milestone: str | None = None # milestone title + created_at: str = CREATED_AT + updated_at: str = CREATED_AT + comments: dict[int, IssueCommentData] = field(default_factory=dict) + + +@dataclass +class Issue(Entity): + data: IssueData + + @property + def user(self) -> User: + return self.gh.get_user(self.data.creator) + + @property + def comment_list(self) -> "EntityView[int, IssueComment]": + return EntityView( + {cid: IssueComment(self.gh, data) for cid, data in self.data.comments.items()} + ) + + def add_comment(self, creator: User | UserData | UserName, body: str, **params) -> IssueComment: + login = creator if isinstance(creator, UserName) else creator.login + comment = IssueCommentData(self.data.number, self.data.repo, login, body, **params) + self.data.comments[comment.id] = comment + return IssueComment(self.gh, comment) + + def payload(self, /, **ovrr): + rend = asdict(self.data) + rend.pop("comments") + rend.pop("creator") + rend.pop("repo") + rend["labels"] = [{"name": label} for label in self.data.labels] + rend["assignee"] = {"login": self.data.assignee} if self.data.assignee else None + rend["milestone"] = {"title": self.data.milestone} if self.data.milestone else None + return ( + rend + | { + "url": f"{self.gh.base_url}/repos/{self.data.repo}/issues/{self.data.number}", + "html_url": f"https://github.example.com/{self.data.repo}/issues/{self.data.number}", + "user": self.user.payload(), + } + | ovrr + ) + + +@dataclass +class PullRequestData(EntityData): + title: str + repo: RepoId + creator: UserName + number: int = -1 + _: KW_ONLY + state: str = "open" + body: str = "" + base: str = "main" + head: str = "feature" + created_at: str = CREATED_AT + updated_at: str = CREATED_AT + + +@dataclass +class PullRequest(Entity): + data: PullRequestData + + @property + def user(self) -> User: + return self.gh.get_user(self.data.creator) + + def payload(self, /, **ovrr): + rend = asdict(self.data) + rend.pop("creator") + rend.pop("repo") + rend["base"] = {"ref": self.data.base} + rend["head"] = {"ref": self.data.head} + return ( + rend + | { + "url": f"{self.gh.base_url}/repos/{self.data.repo}/pulls/{self.data.number}", + "html_url": f"https://github.example.com/{self.data.repo}/pull/{self.data.number}", + "user": self.user.payload(), + } + | ovrr + ) + + def issue_payload(self, /, **ovrr): + """ + Render the pull request as it appears in the issues list, which + includes pull requests marked with a ``pull_request`` key. + """ + rend = self.payload() + rend.pop("base") + rend.pop("head") + return ( + rend + | { + "labels": [], + "assignee": None, + "milestone": None, + "pull_request": { + "url": f"{self.gh.base_url}/repos/{self.data.repo}/pulls/{self.data.number}", + }, + } + | ovrr + ) + + +@dataclass +class MilestoneData(EntityData): + title: str + repo: RepoId + creator: UserName + number: int = -1 + _: KW_ONLY + state: str = "open" + description: str | None = None + open_issues: int = 1 + closed_issues: int = 0 + created_at: str = CREATED_AT + updated_at: str = CREATED_AT + due_on: str | None = None + + +@dataclass +class Milestone(Entity): + data: MilestoneData + + @property + def creator(self) -> User: + return self.gh.get_user(self.data.creator) + + @property + def repo(self) -> Repo: + return self.gh.get_repo(self.data.repo) + + def payload(self, /, **ovrr): + rend = asdict(self.data) + return ( + rend + | { + "creator": self.creator.payload(), + "html_url": f"https://github.example.com/{self.data.repo}/milestone/{self.data.number}", + "labels_url": f"https://github.example.com/{self.data.repo}/milestone/{self.data.number}/labels", + } + | ovrr + ) diff --git a/tests/support/github/routes/__init__.py b/tests/support/github/routes/__init__.py new file mode 100644 index 00000000..e7b42893 --- /dev/null +++ b/tests/support/github/routes/__init__.py @@ -0,0 +1,19 @@ +""" +Route mixins implementing the consumed GitHub API surface, grouped per area. +""" + +from tests.support.github.routes.issues import IssueRoutes +from tests.support.github.routes.milestones import MilestoneRoutes +from tests.support.github.routes.orgs import OrgRoutes +from tests.support.github.routes.repos import RepoRoutes +from tests.support.github.routes.rulesets import RulesetRoutes +from tests.support.github.routes.teams import TeamRoutes + +__all__ = [ + "IssueRoutes", + "MilestoneRoutes", + "OrgRoutes", + "RepoRoutes", + "RulesetRoutes", + "TeamRoutes", +] diff --git a/tests/support/github/routes/issues.py b/tests/support/github/routes/issues.py new file mode 100644 index 00000000..37b1d3a4 --- /dev/null +++ b/tests/support/github/routes/issues.py @@ -0,0 +1,56 @@ +""" +Routes: issues, issue comments and pull requests. +""" + +from tests.support.github.base import APIBase +from tests.support.github.routing import GET + + +def _state_matches(entity_state, query): + wanted = query.get("state", "open") + return wanted in {"all", entity_state} + + +class IssueRoutes(APIBase): + @GET(r"/repos///issues") + def _list_issues(self, match, query, _body): + repo = self._resolve_repo(match) + wanted_labels = set(filter(None, query.get("labels", "").split(","))) + rend = [] + for issue in sorted(repo.issues, key=lambda x: x.number): + if not _state_matches(issue.state, query): + continue + if wanted_labels and not wanted_labels.issubset(issue.labels): + continue + rend.append(issue.payload()) + # The issues list includes pull requests, marked with a pull_request key. + if not wanted_labels: + for pull in sorted(repo.pulls, key=lambda x: x.number): + if _state_matches(pull.state, query): + rend.append(pull.issue_payload()) + return 200, rend + + @GET(r"/repos///issues/") + def _get_issue(self, match, _query, _body): + repo = self._resolve_repo(match) + return 200, repo.get_issue(match["number"]).payload() + + @GET(r"/repos///issues//comments") + def _list_issue_comments(self, match, query, _body): + repo = self._resolve_repo(match) + issue = repo.get_issue(match["number"]) + since = query.get("since") + return 200, [ + comment.payload() + for comment in sorted(issue.comment_list, key=lambda x: x.created_at) + if since is None or comment.updated_at >= since + ] + + @GET(r"/repos///pulls") + def _list_pulls(self, match, query, _body): + repo = self._resolve_repo(match) + return 200, [ + pull.payload() + for pull in sorted(repo.pulls, key=lambda x: x.number) + if _state_matches(pull.state, query) + ] diff --git a/tests/support/github/routes/milestones.py b/tests/support/github/routes/milestones.py new file mode 100644 index 00000000..8da4bc5f --- /dev/null +++ b/tests/support/github/routes/milestones.py @@ -0,0 +1,19 @@ +""" +Routes: milestones. +""" + +from tests.support.github.base import APIBase +from tests.support.github.routing import GET + + +class MilestoneRoutes(APIBase): + @GET("repos///milestones") + def _list_milestones(self, match, _query, _body): + repo = self._resolve_repo(match) + return 200, [milestone.payload() for milestone in repo.milestones] + + @GET("repos///milestones/") + def _get_milestone(self, match, _query, _body): + repo = self._resolve_repo(match) + milestone = repo.get_milestone(match["number"]) + return 200, milestone.payload() diff --git a/tests/support/github/routes/orgs.py b/tests/support/github/routes/orgs.py new file mode 100644 index 00000000..9c65e4f6 --- /dev/null +++ b/tests/support/github/routes/orgs.py @@ -0,0 +1,54 @@ +""" +Routes: organization, users, members. +""" + +from tests.support.github.base import APIBase +from tests.support.github.entities import NotFound +from tests.support.github.routing import DELETE +from tests.support.github.routing import GET +from tests.support.github.routing import PUT + + +class OrgRoutes(APIBase): + @GET("orgs/") + def _get_org(self, match, _query, _body): + org = self._resolve_org(match) + return 200, org.payload() + + @GET("users/") + def _get_user(self, match, _query, _body): + return 200, self.state.get_user(match["login"]).payload() + + @GET("orgs//members") + def _list_members(self, match, query, _body): + org = self._resolve_org(match) + users = [member.user for member in org.active_members()] + if query.get("filter") == "2fa_disabled": + users = [user for user in users if not user.two_factor_authentication] + return 200, [user.payload() for user in users] + + @GET("orgs//members/") + def _is_member(self, match, _query, _body): + org = self._resolve_org(match) + if org.get_member(match["login"]).state == "active": + return 204 + raise NotFound(match["login"]) + + @DELETE("orgs//members/") + def _remove_member(self, match, _query, _body): + org = self._resolve_org(match) + login = match["login"] + org.remove_member(login) + return 204 + + @GET("orgs//memberships/") + def _get_membership(self, match, _query, _body): + org = self._resolve_org(match) + return 200, org.get_member(match["login"]).payload() + + @PUT("orgs//memberships/") + def _put_membership(self, match, _query, _body): + org = self._resolve_org(match) + login = match["login"] + member = org.add_member(login, state="pending") + return 200, member.payload() diff --git a/tests/support/github/routes/repos.py b/tests/support/github/routes/repos.py new file mode 100644 index 00000000..5df2b9b3 --- /dev/null +++ b/tests/support/github/routes/repos.py @@ -0,0 +1,60 @@ +""" +Routes: repositories. +""" + +from tests.support.github.base import APIBase +from tests.support.github.entities import Organization +from tests.support.github.routing import DELETE +from tests.support.github.routing import GET +from tests.support.github.routing import PATCH +from tests.support.github.routing import POST + + +class RepoRoutes(APIBase): + @GET("orgs//repos") + def _list_repos(self, match, _query, _body): + org = self._resolve_org(match) + return 200, [repo.payload() for repo in sorted(org.repos, key=lambda x: x.name)] + + @POST("orgs//repos") + def _create_repo(self, match, _query, body): + org = self._resolve_org(match) + name = body["name"] + repo_params = {"private": bool(body.get("private"))} + for fld in ("description", "homepage", "has_issues", "has_wiki", "has_downloads"): + if fld in body: + repo_params[fld] = body[fld] + repo = org.add_repo(name, **repo_params) + return 201, repo.payload() + + @GET("repos//") + def _get_repo(self, match, _query, _body): + org = self._resolve_org(match) + return 200, org.get_repo(match["repo"]).payload() + + @PATCH("repos//") + def _edit_repo(self, match, _query, body): + repo = self._resolve_repo(match) + upd = {} + for fld, value in body.items(): + if fld != "name": + upd[fld] = value + return 200, repo.update(**upd).payload() + + @DELETE("repos//") + def _delete_repo(self, match, _query, _body): + org = self._resolve_org(match) + org.remove_repo(match["repo"]) + return 204 + + @GET("repos///teams") + def _repo_teams(self, match, _query, _body): + repo = self._resolve_repo(match) + owner = repo.owner + if not isinstance(owner, Organization): + raise NotImplementedError + return 200, [ + team.payload() + for team in sorted(repo.owner.teams, key=lambda x: x.id) + if repo.data.repo_id in team.data.repos + ] diff --git a/tests/support/github/routes/rulesets.py b/tests/support/github/routes/rulesets.py new file mode 100644 index 00000000..bc341658 --- /dev/null +++ b/tests/support/github/routes/rulesets.py @@ -0,0 +1,75 @@ +""" +Routes: rulesets (repo- and org-scoped). +""" + +from tests.support.github.base import APIBase +from tests.support.github.entities import Ruleset +from tests.support.github.routing import DELETE +from tests.support.github.routing import GET +from tests.support.github.routing import POST +from tests.support.github.routing import PUT + + +class RulesetRoutes(APIBase): + def _get_ruleset(self, match) -> Ruleset: + if "repo" in match.groupdict(): + repo = self._resolve_repo(match) + return repo.get_ruleset(match["ruleset_id"]) + org = self._resolve_org(match) + return org.get_ruleset(match["ruleset_id"]) + + @GET("repos///rulesets") + def _list_repo_rulesets(self, match, _query, _body): + repo = self._resolve_repo(match) + rulesets = [ruleset.payload() for ruleset in repo.rulesets] + return 200, rulesets + + @GET("orgs//rulesets") + def _list_org_rulesets(self, match, _query, _body): + org = self._resolve_org(match) + rulesets = [ruleset.payload() for ruleset in org.rulesets] + return 200, rulesets + + @POST("repos///rulesets") + def _add_repo_ruleset(self, match, _query, body): + repo = self._resolve_repo(match) + ruleset = repo.add_ruleset(**dict(body)) + return 201, ruleset.payload() + + @POST("orgs//rulesets") + def _add_org_ruleset(self, match, _query, body): + org = self._resolve_org(match) + ruleset = org.add_ruleset(**dict(body)) + return 201, ruleset.payload() + + @GET("repos///rulesets/") + def _get_repo_ruleset(self, match, _query, _body): + ruleset = self._get_ruleset(match) + return 200, ruleset.payload() + + @GET("orgs//rulesets/") + def _get_org_ruleset(self, match, _query, _body): + ruleset = self._get_ruleset(match) + return 200, ruleset.payload() + + @PUT("repos///rulesets/") + def _update_repo_ruleset(self, match, _query, body): + ruleset = self._get_ruleset(match) + return 200, ruleset.update(**body).payload() + + @PUT("orgs//rulesets/") + def _update_org_ruleset(self, match, _query, body): + ruleset = self._get_ruleset(match) + return 200, ruleset.update(**body).payload() + + @DELETE("repos///rulesets/") + def _delete_repo_ruleset(self, match, _query, _body): + repo = self._resolve_repo(match) + repo.remove_ruleset(match["ruleset_id"]) + return 204 + + @DELETE("orgs//rulesets/") + def _delete_org_ruleset(self, match, _query, _body): + org = self._resolve_org(match) + org.remove_ruleset(match["ruleset_id"]) + return 204 diff --git a/tests/support/github/routes/teams.py b/tests/support/github/routes/teams.py new file mode 100644 index 00000000..04850883 --- /dev/null +++ b/tests/support/github/routes/teams.py @@ -0,0 +1,147 @@ +""" +Routes: teams, including legacy by-ID paths. +""" + +import re + +from tests.support.github.base import APIBase +from tests.support.github.entities import NotFound +from tests.support.github.entities import OrgName +from tests.support.github.entities import RepoId +from tests.support.github.entities import Team +from tests.support.github.entities import TeamMember +from tests.support.github.routing import DELETE +from tests.support.github.routing import GET +from tests.support.github.routing import PATCH +from tests.support.github.routing import POST +from tests.support.github.routing import PUT + + +def _team_uris(resource=""): + """ + Render the two URI flavors (by slug, by ID) of a team resource. + The deprecated legacy ``/teams/{team_id}/...`` paths are intentionally + unsupported so that their usage fails tests with a 404. + Repos are addressed by full name in both flavors. + """ + resource = resource.replace("", "/") + by_slug = f"orgs//teams//{resource}".rstrip("/") + by_id = f"organizations//team//{resource}".rstrip("/") + return by_slug, by_id + + +class TeamRoutes(APIBase): + def _team_by_id(self, team_id: int | str, org: OrgName | None = None) -> Team: + team_id = int(team_id) + for team_key, team in self.state.teams.items(): + if org is not None and team_key[0] != org: + continue + if team.id == team_id: + return Team(self.state, team) + raise NotFound(team_id) + + def _resolve_team(self, match: re.Match) -> Team: + # This is from a route, where NotFound exceptions indicate 404 + dic = match.groupdict() + org = ret = org_login = None + if "org_id" in dic or "org" in dic: + org = self._resolve_org(match) + org_login = org.data.login + if "team_id" in dic: + team_id = int(dic["team_id"]) + ret = self._team_by_id(team_id, org_login) + if "team_slug" in dic: + assert org + ret = org.get_team(dic["team_slug"]) + if ret is not None: + return ret + raise TypeError("Missing team_id or team_slug in match") + + @GET("orgs//teams") + def _list_teams(self, match, _query, _body): + org = self._resolve_org(match) + return 200, [team.payload() for team in sorted(org.teams, key=lambda x: x.id)] + + @POST("orgs//teams") + def _create_team(self, match, _query, body): + org = self._resolve_org(match) + if "privacy" not in body: + body["privacy"] = "secret" + for name in body.pop("repo_names", []): + # The real API expects full names, but tolerate bare ones (same org). + repo_id = RepoId.from_str(name) if "/" in name else RepoId(org.data.login, name) + self.state.get_repo(repo_id, exc=ValueError) + body.setdefault("repos", {})[repo_id] = "pull" + team = org.add_team(**body) + return 201, team.payload() + + @GET(*_team_uris()) + def _get_team(self, match, _query, _body): + return 200, self._resolve_team(match).payload() + + @PATCH(*_team_uris()) + def _edit_team(self, match, _query, body): + team = self._resolve_team(match) + new_team = team.update( + **{ + fld: body[fld] + for fld in ("name", "description", "privacy", "permission") + if body.get(fld) is not None + } + ) + return 200, new_team.payload() + + @DELETE(*_team_uris()) + def _delete_team(self, match, _query, _body): + team = self._resolve_team(match) + team.org.remove_team(team.slug) + return 204 + + @GET(*_team_uris("repos")) + def _team_repos(self, match, _query, _body): + team = self._resolve_team(match) + return 200, [ + self.state.get_repo(repo_id).team_payload(permission) + for repo_id, permission in sorted(team.repos.items()) + ] + + @PUT(*_team_uris("repos/")) + def _team_add_repo(self, match, _query, body): + team = self._resolve_team(match) + repo = self.state.get_repo((match["owner"], match["repo"])) + team.add_repo(repo, (body or {}).get("permission", "pull")) + return 204 + + @DELETE(*_team_uris("repos/")) + def _team_remove_repo(self, match, _query, _body): + team = self._resolve_team(match) + repo = self.state.get_repo((match["owner"], match["repo"])) + team.remove_repo(repo) + return 204 + + @GET(*_team_uris("members")) + def _team_members(self, match, _query, _body): + team = self._resolve_team(match) + return 200, [ + member.user.payload() for member in sorted(team.members, key=lambda x: x.data.login) + ] + + @GET(*_team_uris("memberships/")) + def _team_get_member(self, match, _query, _body): + team = self._resolve_team(match) + login = match["login"] + if login in team.data.members: + return 200, TeamMember(self.state, team.data.members[login]).payload() + raise NotFound(login) + + @DELETE(*_team_uris("memberships/")) + def _delete_membership(self, match, _query, _body): + team = self._resolve_team(match) + team.remove_member(match["login"]) + return 204 + + @PUT(*_team_uris("memberships/")) + def _team_add_member(self, match, _query, _body): + team = self._resolve_team(match) + membership = team.add_member(match["login"]) + return 200, membership.payload() diff --git a/tests/support/github/routing.py b/tests/support/github/routing.py new file mode 100644 index 00000000..365489d8 --- /dev/null +++ b/tests/support/github/routing.py @@ -0,0 +1,73 @@ +""" +Route declaration plumbing for the fake GitHub API. +""" + +import re +from typing import Any +from typing import Literal +from typing import NamedTuple +from typing import TypeAlias + +HTTP_METHOD: TypeAlias = Literal[ # pylint: disable=invalid-name + "GET", "POST", "PUT", "PATCH", "DELETE" +] + + +class Request(NamedTuple): + method: HTTP_METHOD + path: str + query: dict[str, Any] + body: Any + + +def _route(method, *specs): + """ + Mark a route-mixin method as the handler for an API route. + + Specs are either full regexes (detected via an explicit ``(?P<`` group) + or shorthand paths whose ```` segments are expanded into named + groups: ``\\d+`` for names ending in ``_id``/``number``, ``[^/]+`` + otherwise. A leading slash is optional in shorthand specs. + """ + + def decorator(func): + patterns = [] + for spec in specs: + if "(?P<" in spec: + # Already a full regex + patterns.append(spec) + continue + rend = [] + for sgmnt in spec.lstrip("/").split("/"): + if sgmnt.startswith("<") and sgmnt.endswith(">"): + sgmnt = ( + rf"(?P{sgmnt}\d+)" + if sgmnt.endswith(("_id>", "number>")) + else rf"(?P{sgmnt}[^/]+)" + ) + rend.append(sgmnt) + patterns.append("/" + "/".join(rend)) + func.routes = (method, tuple(re.compile(ptrn) for ptrn in patterns)) + return func + + return decorator + + +def GET(*specs): # pylint: disable=invalid-name + return _route("GET", *specs) + + +def POST(*specs): # pylint: disable=invalid-name + return _route("POST", *specs) + + +def PUT(*specs): # pylint: disable=invalid-name + return _route("PUT", *specs) + + +def PATCH(*specs): # pylint: disable=invalid-name + return _route("PATCH", *specs) + + +def DELETE(*specs): # pylint: disable=invalid-name + return _route("DELETE", *specs) diff --git a/tests/support/github/state.py b/tests/support/github/state.py new file mode 100644 index 00000000..71b970e8 --- /dev/null +++ b/tests/support/github/state.py @@ -0,0 +1,241 @@ +""" +Global dict-backed store of the fake GitHub API. +""" + +# pyright: reportImportCycles=false (type-only cycle with .entities, see there) + +from collections import defaultdict +from collections.abc import Sequence +from copy import deepcopy +from dataclasses import dataclass +from dataclasses import field + +from tests.support.github.entities import Entity +from tests.support.github.entities import EntityData +from tests.support.github.entities import Issue +from tests.support.github.entities import IssueData +from tests.support.github.entities import Member +from tests.support.github.entities import MemberData +from tests.support.github.entities import Milestone +from tests.support.github.entities import MilestoneData +from tests.support.github.entities import NotFound +from tests.support.github.entities import Organization +from tests.support.github.entities import OrganizationData +from tests.support.github.entities import OrgName +from tests.support.github.entities import OrgRulesetData +from tests.support.github.entities import PullRequest +from tests.support.github.entities import PullRequestData +from tests.support.github.entities import Repo +from tests.support.github.entities import RepoData +from tests.support.github.entities import RepoId +from tests.support.github.entities import RepoRulesetData +from tests.support.github.entities import Ruleset +from tests.support.github.entities import Team +from tests.support.github.entities import TeamData +from tests.support.github.entities import TeamName +from tests.support.github.entities import User +from tests.support.github.entities import UserData +from tests.support.github.entities import UserName + + +@dataclass +class GlobalState: # pylint: disable=too-many-instance-attributes + base_url: str = field(init=False, default="") + orgs: dict[OrgName, OrganizationData] = field(default_factory=dict) + users: dict[UserName, UserData] = field(default_factory=dict) + repos: dict[RepoId, RepoData] = field(default_factory=dict) + members: dict[tuple[OrgName, UserName], MemberData] = field(default_factory=dict) + teams: dict[tuple[OrgName, TeamName], TeamData] = field(default_factory=dict) + rulesets_org: dict[OrgName, dict[int, OrgRulesetData]] = field( + default_factory=lambda: defaultdict(dict) + ) + rulesets_repo: dict[RepoId, dict[int, RepoRulesetData]] = field( + default_factory=lambda: defaultdict(dict) + ) + milestones: dict[RepoId, dict[int, MilestoneData]] = field( + default_factory=lambda: defaultdict(dict) + ) + issues: dict[RepoId, dict[int, IssueData]] = field(default_factory=lambda: defaultdict(dict)) + pulls: dict[RepoId, dict[int, PullRequestData]] = field( + default_factory=lambda: defaultdict(dict) + ) + + def __post_init__(self): + self.snapshot() + + def snapshot(self): + self._init_orgs = deepcopy(self.orgs) + self._init_users = deepcopy(self.users) + self._init_repos = deepcopy(self.repos) + self._init_members = deepcopy(self.members) + self._init_teams = deepcopy(self.teams) + self._init_rulesets_org = deepcopy(self.rulesets_org) + self._init_rulesets_repo = deepcopy(self.rulesets_repo) + self._init_milestones = deepcopy(self.milestones) + self._init_issues = deepcopy(self.issues) + self._init_pulls = deepcopy(self.pulls) + + def reset(self): + self.orgs = {} + self.users = {} + self.repos = {} + self.members = {} + self.teams = {} + self.rulesets_org = defaultdict(dict) + self.rulesets_repo = defaultdict(dict) + self.milestones = defaultdict(dict) + self.issues = defaultdict(dict) + self.pulls = defaultdict(dict) + self._init_orgs = {} + self._init_users = {} + self._init_repos = {} + self._init_members = {} + self._init_teams = {} + self._init_rulesets_org = defaultdict(dict) + self._init_rulesets_repo = defaultdict(dict) + self._init_milestones = defaultdict(dict) + self._init_issues = defaultdict(dict) + self._init_pulls = defaultdict(dict) + + def restore(self): + self.orgs = deepcopy(self._init_orgs) + self.users = deepcopy(self._init_users) + self.repos = deepcopy(self._init_repos) + self.members = deepcopy(self._init_members) + self.teams = deepcopy(self._init_teams) + self.rulesets_org = deepcopy(self._init_rulesets_org) + self.rulesets_repo = deepcopy(self._init_rulesets_repo) + self.milestones = deepcopy(self._init_milestones) + self.issues = deepcopy(self._init_issues) + self.pulls = deepcopy(self._init_pulls) + + def _next_issue_number(self, repo_id: RepoId) -> int: + # Issues and pull requests share one number sequence per repo. + return len(self.issues[repo_id]) + len(self.pulls[repo_id]) + 1 + + def add_user(self, name: str, **params) -> User: + user = UserData(name, **params) + if user.login in self.users: + raise RuntimeError(f"User {user.login} already exists") + self.users[user.login] = user + return User(self, user) + + def add_org(self, name: str, **params) -> Organization: + org = OrganizationData(name, **params) + if org.login in self.orgs: + raise RuntimeError(f"Organization {org.login} already exists") + self.orgs[org.login] = org + return Organization(self, org) + + def get_user(self, login: str, *, exc=NotFound) -> User: + if login not in self.users: + raise exc(f"Requested nonexistent user {login=}") + return User(self, self.users[login]) + + def get_org(self, login: str, *, exc=NotFound) -> Organization: + if login not in self.orgs: + raise exc(f"Requested nonexistent org {login=}") + return Organization(self, self.orgs[login]) + + def get_org_id(self, org_id: int | str, *, exc=NotFound) -> Organization: + org_id = int(org_id) + res = next((org for org in self.orgs.values() if org.id == org_id), None) + if res is None: + raise exc(f"Requested nonexistent org_id {org_id=}") + return Organization(self, res) + + def get_repo(self, repo_id: RepoId | Sequence[str] | str, *, exc=NotFound) -> Repo: + if isinstance(repo_id, RepoId): + resolved = repo_id + elif isinstance(repo_id, str): + resolved = RepoId.from_str(repo_id) + else: + resolved = RepoId.from_seq(repo_id) + + if resolved.owner not in self.orgs: + if resolved.owner in self.users: + raise NotImplementedError + raise exc(f"Requested repo from nonexistent org {resolved.owner=}") + if resolved not in self.repos: + raise exc(f"Requested nonexistent repo {resolved.name=} from org {resolved.owner=}") + return Repo(self, self.repos[resolved]) + + def add(self, entity: EntityData | Entity): + if isinstance(entity, Entity): + data = entity.data + else: + data = entity + + if isinstance(data, RepoData): + self.repos[data.repo_id] = data + return Repo(self, data) + if isinstance(data, MemberData): + self.members[(data.org, data.login)] = data + return Member(self, data) + if isinstance(data, TeamData): + self.teams[(data.org, data.slug)] = data + return Team(self, data) + if isinstance(data, RepoRulesetData): + assert data.source in self.repos + self.rulesets_repo[data.source][data.id] = data + return Ruleset(self, data) + if isinstance(data, OrgRulesetData): + assert data.source in self.orgs + self.rulesets_org.setdefault(data.source, {})[data.id] = data + return Ruleset(self, data) + if isinstance(data, MilestoneData): + if data.number < 0: + data = data.with_(number=len(self.milestones[data.repo]) + 1) + self.milestones[data.repo][data.number] = data + return Milestone(self, data) + if isinstance(data, IssueData): + if data.number < 0: + data = data.with_(number=self._next_issue_number(data.repo)) + self.issues[data.repo][data.number] = data + return Issue(self, data) + if isinstance(data, PullRequestData): + if data.number < 0: + data = data.with_(number=self._next_issue_number(data.repo)) + self.pulls[data.repo][data.number] = data + return PullRequest(self, data) + if isinstance(data, UserData): + self.users[data.login] = data + return User(self, data) + if isinstance(data, OrganizationData): + self.orgs[data.login] = data + return Organization(self, data) + raise TypeError(f"Unknown object: {data=}") + + def replace(self, old: EntityData | Entity, new: EntityData | Entity): + if isinstance(old, Entity): + old_data = old.data + else: + old_data = old + if isinstance(new, Entity): + new_data = new.data + else: + new_data = new + assert type(old_data) is type(new_data) + if isinstance(old_data, RepoData): + self.repos.pop(old_data.repo_id) + elif isinstance(old_data, MemberData): + self.members.pop((old_data.org, old_data.login)) + elif isinstance(old_data, TeamData): + self.teams.pop((old_data.org, old_data.slug)) + elif isinstance(old_data, RepoRulesetData): + self.rulesets_repo[old_data.source].pop(old_data.id) + elif isinstance(old_data, OrgRulesetData): + self.rulesets_org[old_data.source].pop(old_data.id) + elif isinstance(old_data, MilestoneData): + self.milestones[old_data.repo].pop(old_data.number) + elif isinstance(old_data, IssueData): + self.issues[old_data.repo].pop(old_data.number) + elif isinstance(old_data, PullRequestData): + self.pulls[old_data.repo].pop(old_data.number) + elif isinstance(old_data, UserData): + self.users.pop(old_data.login) + elif isinstance(old_data, OrganizationData): + self.orgs.pop(old_data.login) + else: + raise TypeError(f"Unknown object: {old_data=}") + return self.add(new_data) diff --git a/tests/unit/modules/test_github.py b/tests/unit/modules/test_github.py index c16b1950..c3928da4 100644 --- a/tests/unit/modules/test_github.py +++ b/tests/unit/modules/test_github.py @@ -1,21 +1,22 @@ -from unittest import mock +from unittest.mock import MagicMock +from unittest.mock import patch import pytest -import salt.modules.config as config_module from salt.exceptions import CommandExecutionError +from salt.modules import config as config_mod -import saltext.github.modules.github as github_module +from saltext.github.modules import github as github_mod @pytest.fixture def configure_loader_modules(): module_globals = { - "__salt__": {"config.option": config_module.option}, + "__salt__": {"config.option": config_mod.option}, "__opts__": {"test_ruleset": {"ruleset_type": "mocked_value"}}, } return { - github_module: module_globals, - config_module: module_globals, + github_mod: module_globals, + config_mod: module_globals, } @@ -30,22 +31,62 @@ 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"} + ) + + +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) + + 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__query_unexpected_exception(): + requester = MagicMock() + requester.requestJson.side_effect = OSError("connection refused") + client = MagicMock(requester=requester) + + with patch.object(github_mod, "_get_client", return_value=client): + with pytest.raises(CommandExecutionError, match="connection refused"): + github_mod._query("test_ruleset", action="repos") + + def test__check_ruleset_params(): ## test no valid config with pytest.raises(CommandExecutionError): - github_module._check_ruleset_params(profile="test_ruleset") + github_mod._check_ruleset_params(profile="test_ruleset") # test cli config kwargs = {"ruleset_type": "repo", "repo_name": "mock_repo_name", "owner": "mock_owner"} - ret = github_module._check_ruleset_params(profile="test_ruleset", **kwargs) + 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", - "header_dict": {}, }, - "repos/mock_owner/mock_repo_name/rulesets", ) @@ -56,8 +97,8 @@ def test_get_ruleset(params): mock_query = {"dict": {"id": "1"}, "status": 200} expected = {"id": "1"} - with mock.patch("salt.utils.http.query", return_value=mock_query): - assert github_module.get_ruleset(profile="test_ruleset", **params) == expected + with patch.object(github_mod, "_query", return_value=mock_query): + assert github_mod.get_ruleset(profile="test_ruleset", **params) == expected # test mock 404 error mock_query = {"error": "404 not found", "status": 404} @@ -68,13 +109,13 @@ def test_get_ruleset(params): "result": False, } - with mock.patch("salt.utils.http.query", return_value=mock_query): - assert github_module.get_ruleset(profile="test_ruleset", **params) == expected + with patch.object(github_mod, "_query", return_value=mock_query): + assert github_mod.get_ruleset(profile="test_ruleset", **params) == expected # test mock empty return expected = {"comment": "Error getting ruleset", "result": False} - with mock.patch("salt.utils.http.query", return_value={}): - assert github_module.get_ruleset(profile="test_ruleset", **params) == expected + with patch.object(github_mod, "_query", side_effect=CommandExecutionError): + assert github_mod.get_ruleset(profile="test_ruleset", **params) == expected def test_add_ruleset(params): @@ -85,8 +126,8 @@ def test_add_ruleset(params): expected = {"name": "mock_name", "enforcement": "disabled"} - with mock.patch("salt.utils.http.query", return_value=mock_query): - assert github_module.add_ruleset(profile="test_ruleset", **params) == expected + with patch.object(github_mod, "_query", return_value=mock_query): + assert github_mod.add_ruleset(profile="test_ruleset", **params) == expected # test mock 404 error expected = { @@ -96,16 +137,14 @@ def test_add_ruleset(params): "result": False, } - with mock.patch( - "salt.utils.http.query", return_value={"error": "404 not found", "status": 404} - ): - assert github_module.add_ruleset(profile="test_ruleset", **params) == expected + with patch.object(github_mod, "_query", return_value={"error": "404 not found", "status": 404}): + assert github_mod.add_ruleset(profile="test_ruleset", **params) == expected # test mock empty return expected = {"comment": "Error adding ruleset", "result": False} - with mock.patch("salt.utils.http.query", return_value={}): - assert github_module.add_ruleset(profile="test_ruleset", **params) == expected + with patch.object(github_mod, "_query", side_effect=CommandExecutionError): + assert github_mod.add_ruleset(profile="test_ruleset", **params) == expected def test_list_rulesets(params): @@ -118,15 +157,15 @@ def test_list_rulesets(params): expected = {"rulesets": [{"name": "mock_ruleset_1"}, {"name": "mock_ruleset_2"}]} - with mock.patch("salt.utils.http.query", return_value=mock_query): - assert github_module.list_rulesets(profile="test_ruleset", **params) == expected + 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 mock_query = {"dict": [], "status": 200} expected = {"rulesets": None} - with mock.patch("salt.utils.http.query", return_value=mock_query): - assert github_module.list_rulesets(profile="test_ruleset", **params) == expected + with patch.object(github_mod, "_query", return_value=mock_query): + assert github_mod.list_rulesets(profile="test_ruleset", **params) == expected # test mock 404 error mock_query = {"error": "404 not found", "status": 404} @@ -137,13 +176,13 @@ def test_list_rulesets(params): "result": False, } - with mock.patch("salt.utils.http.query", return_value=mock_query): - assert github_module.list_rulesets(profile="test_ruleset", **params) == expected + with patch.object(github_mod, "_query", return_value=mock_query): + assert github_mod.list_rulesets(profile="test_ruleset", **params) == expected # test mock empty return expected = {"comment": "Error getting rulesets", "result": False} - with mock.patch("salt.utils.http.query", return_value={}): - assert github_module.list_rulesets(profile="test_ruleset", **params) == expected + with patch.object(github_mod, "_query", side_effect=CommandExecutionError): + assert github_mod.list_rulesets(profile="test_ruleset", **params) == expected def test_update_ruleset(params): @@ -155,8 +194,8 @@ def test_update_ruleset(params): expected = {"name": "mock_name", "enforcement": "disabled"} - with mock.patch("salt.utils.http.query", return_value=mock_query): - assert github_module.update_ruleset(profile="test_ruleset", **params) == expected + with patch.object(github_mod, "_query", return_value=mock_query): + assert github_mod.update_ruleset(profile="test_ruleset", **params) == expected # test mock 404 error mock_query = {"error": "404 not found", "status": 404} @@ -167,13 +206,13 @@ def test_update_ruleset(params): "result": False, } - with mock.patch("salt.utils.http.query", return_value=mock_query): - assert github_module.update_ruleset(profile="test_ruleset", **params) == expected + with patch.object(github_mod, "_query", return_value=mock_query): + assert github_mod.update_ruleset(profile="test_ruleset", **params) == expected # test mock empty return expected = {"comment": "Error updating ruleset", "result": False} - with mock.patch("salt.utils.http.query", return_value={}): - assert github_module.update_ruleset(profile="test_ruleset", **params) == expected + with patch.object(github_mod, "_query", side_effect=CommandExecutionError): + assert github_mod.update_ruleset(profile="test_ruleset", **params) == expected def test_delete_ruleset(params): @@ -183,8 +222,8 @@ def test_delete_ruleset(params): mock_query = {"status": 204} expected = {"comment": "ruleset 1 successfully deleted", "status": 204} - with mock.patch("salt.utils.http.query", return_value=mock_query): - assert github_module.delete_ruleset(profile="test_ruleset", **params) == expected + with patch.object(github_mod, "_query", return_value=mock_query): + assert github_mod.delete_ruleset(profile="test_ruleset", **params) == expected # test mock 404 error mock_query = {"error": "404 not found", "status": 404} @@ -195,10 +234,10 @@ def test_delete_ruleset(params): "result": False, } - with mock.patch("salt.utils.http.query", return_value=mock_query): - assert github_module.delete_ruleset(profile="test_ruleset", **params) == expected + with patch.object(github_mod, "_query", return_value=mock_query): + assert github_mod.delete_ruleset(profile="test_ruleset", **params) == expected # test mock empty return expected = {"comment": "Error deleting ruleset", "result": False} - with mock.patch("salt.utils.http.query", return_value={}): - assert github_module.delete_ruleset(profile="test_ruleset", **params) == expected + with patch.object(github_mod, "_query", side_effect=CommandExecutionError): + assert github_mod.delete_ruleset(profile="test_ruleset", **params) == expected diff --git a/tests/unit/states/test_github.py b/tests/unit/states/test_github.py index 1db65dec..4427bc5c 100644 --- a/tests/unit/states/test_github.py +++ b/tests/unit/states/test_github.py @@ -3,13 +3,13 @@ import pytest import salt.modules.config as config_module -import saltext.github.states.github as github_state +from saltext.github.states import github as github_mod @pytest.fixture def configure_loader_modules(): return { - github_state: { + github_mod: { "__salt__": { "github.list_rulesets": mock.MagicMock(return_value=None), "github.delete_ruleset": mock.MagicMock(return_value=None), @@ -37,8 +37,8 @@ def test_repo_absent_no_changes(): "comment": "Ruleset mock_repo_absent does not exist", } - with mock.patch.dict(github_state.__salt__, {"github.list_rulesets": mock_query}): - ret = github_state.ruleset_absent( + with mock.patch.dict(github_mod.__salt__, {"github.list_rulesets": mock_query}): + ret = github_mod.ruleset_absent( name="mock_repo_absent", ruleset_type="repo", profile="test_ruleset", @@ -50,8 +50,8 @@ def test_repo_absent_no_changes(): # test list rulesets are returned but ruleset to be deleted is not listed mock_query = mock.MagicMock(return_value={"rulesets": [{"name": "mocked_repo_absent"}]}) - with mock.patch.dict(github_state.__salt__, {"github.list_rulesets": mock_query}): - ret = github_state.ruleset_absent( + with mock.patch.dict(github_mod.__salt__, {"github.list_rulesets": mock_query}): + ret = github_mod.ruleset_absent( name="mock_repo_absent", ruleset_type="repo", profile="test_ruleset", @@ -60,12 +60,10 @@ def test_repo_absent_no_changes(): ) assert ret == expected - # test mode - expected["result"] = None - - with mock.patch.dict(github_state.__salt__, {"github.list_rulesets": mock_query}): - with mock.patch.dict(github_state.__opts__, {"test": True}): - ret = github_state.ruleset_absent( + # test mode: an already absent ruleset is still a fulfilled state + with mock.patch.dict(github_mod.__salt__, {"github.list_rulesets": mock_query}): + with mock.patch.dict(github_mod.__opts__, {"test": True}): + ret = github_mod.ruleset_absent( name="mock_repo_absent", ruleset_type="repo", profile="test_ruleset", @@ -91,10 +89,10 @@ def test_repo_absent_changes(): mock_delete = mock.MagicMock(return_value={"comment": "ruleset 1 successfully deleted"}) with mock.patch.dict( - github_state.__salt__, + github_mod.__salt__, {"github.list_rulesets": mock_query, "github.delete_ruleset": mock_delete}, ): - ret = github_state.ruleset_absent( + ret = github_mod.ruleset_absent( name="mock_repo_absent", ruleset_type="repo", profile="test_ruleset", @@ -103,10 +101,13 @@ def test_repo_absent_changes(): ) assert ret == expected - # test mode + # test mode: projected changes are reported expected = { "name": "mock_repo_absent", - "changes": {}, + "changes": { + "old": "ruleset mock_repo_absent exists", + "new": "ruleset mock_repo_absent deleted", + }, "result": None, "comment": "Ruleset mock_repo_absent will be deleted", } @@ -114,12 +115,12 @@ def test_repo_absent_changes(): mock_query = mock.MagicMock(return_value={"rulesets": [{"name": "mock_repo_absent", "id": 1}]}) mock_delete = mock.MagicMock(return_value={"comment": "ruleset 1 successfully deleted"}) - with mock.patch.dict(github_state.__opts__, {"test": True}): + with mock.patch.dict(github_mod.__opts__, {"test": True}): with mock.patch.dict( - github_state.__salt__, + github_mod.__salt__, {"github.list_rulesets": mock_query, "github.delete_ruleset": mock_delete}, ): - ret = github_state.ruleset_absent( + ret = github_mod.ruleset_absent( name="mock_repo_absent", ruleset_type="repo", profile="test_ruleset", @@ -130,23 +131,23 @@ def test_repo_absent_changes(): def test_repo_present_no_changes(): - # Test mode + # Test mode: a ruleset present as specified is a fulfilled state expected = { "name": "mock_repo_absent", "changes": {}, - "result": None, + "result": True, "comment": "ruleset present", } mock_query = mock.MagicMock(return_value={"rulesets": [{"name": "mock_repo_absent", "id": 1}]}) mock_ruleset = mock.MagicMock(return_value={"name": "mock_repo_absent", "id": 1}) - with mock.patch.dict(github_state.__opts__, {"test": True}): + with mock.patch.dict(github_mod.__opts__, {"test": True}): with mock.patch.dict( - github_state.__salt__, + github_mod.__salt__, {"github.list_rulesets": mock_query, "github.get_ruleset": mock_ruleset}, ): - ret = github_state.ruleset_present( + ret = github_mod.ruleset_present( name="mock_repo_absent", ruleset_type="repo", profile="test_ruleset", @@ -156,13 +157,11 @@ def test_repo_present_no_changes(): ) assert ret == expected - expected["result"] = True - with mock.patch.dict( - github_state.__salt__, + github_mod.__salt__, {"github.list_rulesets": mock_query, "github.get_ruleset": mock_ruleset}, ): - ret = github_state.ruleset_present( + ret = github_mod.ruleset_present( name="mock_repo_absent", ruleset_type="repo", profile="test_ruleset", @@ -177,8 +176,8 @@ def test_repo_present_update_ruleset(): expected = { "name": "mock_repo_absent", "changes": { - "old": "Ruleset properties were {'name': 'mock_repo_absent', 'id': 1, 'target': 'branch'}", - "new": "Ruleset properties (that changed) are {'target': 'tag', 'name': 'mock_repo_absent'}", + "old": {"target": "branch"}, + "new": {"target": "tag"}, }, "result": True, "comment": "ruleset updated", @@ -193,14 +192,14 @@ def test_repo_present_update_ruleset(): ) with mock.patch.dict( - github_state.__salt__, + github_mod.__salt__, { "github.list_rulesets": mock_query, "github.get_ruleset": mock_ruleset, "github.update_ruleset": mock_update, }, ): - ret = github_state.ruleset_present( + ret = github_mod.ruleset_present( name="mock_repo_absent", ruleset_type="repo", profile="test_ruleset", @@ -210,24 +209,27 @@ def test_repo_present_update_ruleset(): ) assert ret == expected - # Test mode + # Test mode: forecast changes are reported expected = { "name": "mock_repo_absent", - "changes": {}, + "changes": { + "old": {"target": "branch"}, + "new": {"target": "tag"}, + }, "result": None, "comment": "ruleset will be updated", } with mock.patch.dict( - github_state.__salt__, + github_mod.__salt__, { "github.list_rulesets": mock_query, "github.get_ruleset": mock_ruleset, "github.update_ruleset": mock_update, }, ): - with mock.patch.dict(github_state.__opts__, {"test": True}): - ret = github_state.ruleset_present( + with mock.patch.dict(github_mod.__opts__, {"test": True}): + ret = github_mod.ruleset_present( name="mock_repo_absent", ruleset_type="repo", profile="test_ruleset", @@ -253,9 +255,9 @@ def test_repo_present_add_ruleset(): ) with mock.patch.dict( - github_state.__salt__, {"github.list_rulesets": mock_query, "github.add_ruleset": mock_add} + github_mod.__salt__, {"github.list_rulesets": mock_query, "github.add_ruleset": mock_add} ): - ret = github_state.ruleset_present( + ret = github_mod.ruleset_present( name="mock_repo_absent", ruleset_type="repo", profile="test_ruleset", @@ -265,17 +267,16 @@ def test_repo_present_add_ruleset(): ) assert ret == expected - # Test mode + # Test mode: forecast changes are reported expected["result"] = None expected["comment"] = "ruleset will be added" - expected["changes"] = {} - with mock.patch.dict(github_state.__opts__, {"test": True}): + with mock.patch.dict(github_mod.__opts__, {"test": True}): with mock.patch.dict( - github_state.__salt__, + github_mod.__salt__, {"github.list_rulesets": mock_query, "github.add_ruleset": mock_add}, ): - ret = github_state.ruleset_present( + ret = github_mod.ruleset_present( name="mock_repo_absent", ruleset_type="repo", profile="test_ruleset", diff --git a/tests/unit/states/test_github_failures.py b/tests/unit/states/test_github_failures.py new file mode 100644 index 00000000..7f70e879 --- /dev/null +++ b/tests/unit/states/test_github_failures.py @@ -0,0 +1,214 @@ +""" +Failure branches that require API-level failure injection which the +functional fake cannot produce (mutations that are accepted over HTTP +but reported as failed by the module layer). +""" + +from unittest import mock + +import pytest +from salt.exceptions import CommandExecutionError + +from saltext.github.states import github as github_mod + + +@pytest.fixture +def configure_loader_modules(): + return { + github_mod: { + "__salt__": {}, + "__opts__": {"test": False}, + }, + } + + +def _patch_salt(**funcs): + return mock.patch.dict( + github_mod.__salt__, + {f"github.{name}": mock.MagicMock(**spec) for name, spec in funcs.items()}, + ) + + +def test_present_add_failure(): + with _patch_salt( + get_user={"return_value": {"in_org": False, "membership_state": "nonexistent"}}, + add_user={"return_value": False}, + ): + ret = github_mod.present("user1") + assert ret["result"] is False + assert "Failed to add user" in ret["comment"] + + +def test_absent_remove_failure(): + with _patch_salt( + get_user={"return_value": True}, + remove_user={"return_value": False}, + ): + ret = github_mod.absent("user1") + assert ret["result"] is False + assert "Failed to delete" in ret["comment"] + + +def test_team_absent_remove_failure(): + with _patch_salt( + get_team={"return_value": {"id": 1}}, + remove_team={"return_value": False}, + ): + ret = github_mod.team_absent("team") + assert ret["result"] is False + assert "Failed to delete" in ret["comment"] + + +def test_repo_absent_remove_failure(): + with _patch_salt( + get_repo_info={"return_value": {"name": "repo"}}, + remove_repo={"return_value": False}, + ): + ret = github_mod.repo_absent("repo") + assert ret["result"] is False + assert "delete_repo scope" in ret["comment"] + + +_TEAM_TARGET = {"description": "desc", "permission": "pull", "privacy": "closed"} + + +def test_team_present_edit_failure(): + with _patch_salt( + get_team={"return_value": _TEAM_TARGET}, + edit_team={"return_value": False}, + ): + ret = github_mod.team_present( + "team", description="changed", privacy="closed", permission="pull" + ) + assert ret["result"] is False + assert ret["comment"] == "Failed to update team properties." + + +def test_team_present_remove_repo_failure(): + with _patch_salt( + get_team={"return_value": _TEAM_TARGET}, + list_team_repos={"return_value": {"repo-a": {"permission": "pull"}}}, + add_team_repo={"return_value": True}, + remove_team_repo={"return_value": False}, + ): + ret = github_mod.team_present( + "team", + description="desc", + privacy="closed", + permission="pull", + repo_names=["repo-b"], + ) + assert ret["result"] is False + assert "Failed to remove repo repo-a" in ret["comment"] + + +def test_team_present_remove_member_failure(): + with _patch_salt( + get_team={"return_value": _TEAM_TARGET}, + list_team_repos={"return_value": {}}, + list_members_without_mfa={"return_value": []}, + list_team_members={"return_value": ["olduser"]}, + remove_team_member={"return_value": False}, + ): + ret = github_mod.team_present( + "team", description="desc", privacy="closed", permission="pull", members={} + ) + assert ret["result"] is False + assert "Failed to remove user olduser" in ret["comment"] + + +def test_repo_present_edit_failure(): + with _patch_salt( + list_repos={"return_value": ["repo"]}, + get_repo_info={"return_value": {"name": "repo", "description": "old"}}, + edit_repo={"return_value": False}, + ): + ret = github_mod.repo_present("repo", description="new") + assert ret["result"] is False + assert ret["comment"] == "Failed to update repo properties." + + +def test_repo_present_create_failure(): + with _patch_salt( + list_repos={"return_value": []}, + get_repo_info={"side_effect": CommandExecutionError("missing")}, + add_repo={"return_value": False}, + ): + ret = github_mod.repo_present("repo") + assert ret["result"] is False + assert "Failed to create repo" in ret["comment"] + + +def test_repo_present_verification_after_create_failure(): + # The post-creation team fetch keeps returning 404 + with ( + _patch_salt( + list_repos={"return_value": []}, + get_repo_info={"side_effect": CommandExecutionError("missing")}, + add_repo={"return_value": True}, + get_repo_teams={"side_effect": CommandExecutionError("404")}, + ), + mock.patch("time.sleep"), + ): + ret = github_mod.repo_present("repo") + assert ret["result"] is False + assert "Failed to verify repo" in ret["comment"] + + +def test_repo_present_remove_team_failure(): + with _patch_salt( + list_repos={"return_value": ["repo"]}, + get_repo_info={"return_value": {"name": "repo"}}, + get_repo_teams={"return_value": [{"name": "Team A"}]}, + remove_team_repo={"return_value": False}, + ): + ret = github_mod.repo_present("repo", teams={}) + assert ret["result"] is False + assert "Failed to remove repo repo from team Team A" in ret["comment"] + + +def test_repo_present_permission_lookup_failure(): + with _patch_salt( + list_repos={"return_value": ["repo"]}, + get_repo_info={"return_value": {"name": "repo"}}, + get_repo_teams={"return_value": [{"name": "Team A"}]}, + list_team_repos={"return_value": {}}, + ): + ret = github_mod.repo_present("repo", teams={"Team A": "push"}) + assert ret["result"] is False + assert "Failed to determine current permission" in ret["comment"] + + +def test_repo_present_permission_change_failure(): + with _patch_salt( + list_repos={"return_value": ["repo"]}, + get_repo_info={"return_value": {"name": "repo"}}, + get_repo_teams={"return_value": [{"name": "Team A"}]}, + list_team_repos={"return_value": {"repo": {"permission": "pull"}}}, + add_team_repo={"return_value": False}, + ): + ret = github_mod.repo_present("repo", teams={"Team A": "push"}) + assert ret["result"] is False + assert "Failed to set permission" in ret["comment"] + + +def test_ruleset_absent_delete_failure(): + with _patch_salt( + list_rulesets={"return_value": {"rulesets": [{"name": "rules", "id": 1}]}}, + delete_ruleset={"return_value": {"result": False, "comment": "403 nope"}}, + ): + ret = github_mod.ruleset_absent("rules", ruleset_type="org") + assert ret["result"] is False + assert "Failed to delete ruleset rules" in ret["comment"] + + +def test_ruleset_present_get_failure(): + with _patch_salt( + list_rulesets={"return_value": {"rulesets": [{"name": "rules", "id": 1}]}}, + get_ruleset={"return_value": {"result": False, "comment": "403 nope"}}, + ): + ret = github_mod.ruleset_present( + "rules", ruleset_type="org", ruleset_params={"enforcement": "active"} + ) + assert ret["result"] is False + assert ret["comment"] == "Could not get ruleset"