From 97535e5b6ba4fc1edbcc33d32459d2a663aee3cc Mon Sep 17 00:00:00 2001 From: jeanluc Date: Wed, 14 Oct 2026 04:55:36 +0200 Subject: [PATCH 1/3] Also test without api_url set Porting the connection class to a dataclass makes its repr nicer. --- src/saltext/github/utils/gh/client.py | 19 ++++++-- .../modules/github/test_profiles.py | 48 ++++++++++++++++++- .../modules/github/test_rulesets.py | 3 +- .../modules/github_repo/test_owner.py | 4 +- tests/functional/resources/test_connection.py | 9 ++-- .../resources/test_multi_profile.py | 3 +- 6 files changed, 73 insertions(+), 13 deletions(-) diff --git a/src/saltext/github/utils/gh/client.py b/src/saltext/github/utils/gh/client.py index f5793c3b..3d8d3419 100644 --- a/src/saltext/github/utils/gh/client.py +++ b/src/saltext/github/utils/gh/client.py @@ -12,6 +12,10 @@ import json import logging import re +from dataclasses import KW_ONLY +from dataclasses import InitVar +from dataclasses import dataclass +from dataclasses import field import github from github.GithubException import UnknownObjectException @@ -25,6 +29,7 @@ API_URL_DEFAULT = "https://api.github.com" +@dataclass(slots=True) class GitHubConnection: """ An authenticated PyGithub client bound to a default repository owner @@ -44,13 +49,19 @@ class GitHubConnection: Enterprise Server instance. Defaults to the public GitHub API. """ - def __init__(self, token, owner=None, *, api_url=API_URL_DEFAULT): - self._owner = owner - self.api_url = (api_url or API_URL_DEFAULT).rstrip("/") + token: InitVar[str] + _owner: str | None = None + _: KW_ONLY + api_url: str = API_URL_DEFAULT + + cache: dict = field(default_factory=dict) + client: github.Github = field(init=False, repr=False) + + def __post_init__(self, token: str): + self.api_url = (self.api_url or API_URL_DEFAULT).rstrip("/") self.client = github.Github( auth=github.Auth.Token(token), base_url=self.api_url, per_page=100 ) - self.cache = {} @property def owner(self): diff --git a/tests/functional/modules/github/test_profiles.py b/tests/functional/modules/github/test_profiles.py index 6875b8cb..26745c75 100644 --- a/tests/functional/modules/github/test_profiles.py +++ b/tests/functional/modules/github/test_profiles.py @@ -6,14 +6,58 @@ import pytest from salt.exceptions import CommandExecutionError +import saltext.github.utils.gh.client from saltext.github.utils.gh.exceptions import GitHubNotFoundError +from tests.support.github import FakeGitHubAPI @pytest.fixture(scope="module") -def minion_config_defaults(github_profile): +def other_github(): + api = FakeGitHubAPI("other-token", auth_login="other-user") + api.reset() + api.state.add_user("other-user") + org = api.state.add_org("testorg") + org.add_repo("repo-a") + with api.serve(): + yield api + + +@pytest.fixture(scope="module") +def minion_config_defaults(github_profile, other_github): + no_url = {"token": "other-token", "owner": "testorg"} # we'll patch api_url for this one + api_url_set = {"token": "other-token", "api_url": other_github.url, "owner": "testorg"} broken = github_profile.copy() broken["org_name"] = "missing-org" - return {"github": broken, "primary": github_profile} + return { + "github": broken, + "primary": github_profile, + "api_url_set": api_url_set, + "no_url": no_url, + } + + +def test_api_url_is_respected(github, fake_github, other_github): + assert github.get_repo_info("repo-a", profile="primary") + assert github.get_repo_info("repo-a", profile="api_url_set") + + fake_github.state.get_org("testorg").add_repo("sentinel") + assert github.get_repo_info("sentinel", profile="primary") + with pytest.raises(GitHubNotFoundError, match="'sentinel' repository.*not be found"): + github.get_repo_info("sentinel", profile="api_url_set") + + # Force-override the default API URL, it's not specified in the profile. + # Thus also checks that an empty api_url works. + with pytest.MonkeyPatch.context() as monkeypatch: + orig_post_init = saltext.github.utils.gh.client.GitHubConnection.__post_init__ + monkeypatch.setattr( + saltext.github.utils.gh.client.GitHubConnection, + "__post_init__", + lambda x, *y: object.__setattr__(x, "api_url", other_github.url) + or orig_post_init(x, *y), + ) + assert github.get_repo_info("repo-a", profile="no_url") + with pytest.raises(GitHubNotFoundError, match="'sentinel' repository.*not be found"): + github.get_repo_info("sentinel", profile="no_url") def test_add_team_uses_profile(github, fake_org): diff --git a/tests/functional/modules/github/test_rulesets.py b/tests/functional/modules/github/test_rulesets.py index af14e29e..fe981a28 100644 --- a/tests/functional/modules/github/test_rulesets.py +++ b/tests/functional/modules/github/test_rulesets.py @@ -9,8 +9,9 @@ def minion_config_defaults(github_profile): owner_profile = { "token": github_profile["token"], "owner": github_profile["org_name"], - "api_url": github_profile["api_url"], } + if api_url := github_profile.get("api_url"): + owner_profile["api_url"] = api_url return {"github": github_profile, "github_owner": owner_profile} diff --git a/tests/functional/modules/github_repo/test_owner.py b/tests/functional/modules/github_repo/test_owner.py index 9e9b55d7..9bdbc836 100644 --- a/tests/functional/modules/github_repo/test_owner.py +++ b/tests/functional/modules/github_repo/test_owner.py @@ -16,13 +16,13 @@ def minion_config_defaults(github_profile): user_profile = { "token": github_profile["token"], "owner": "user1", - "api_url": github_profile["api_url"], "repo_name": "user-repo", } noowner_profile = { "token": github_profile["token"], - "api_url": github_profile["api_url"], } + if api_url := github_profile.get("api_url"): + user_profile["api_url"] = noowner_profile["api_url"] = api_url return { "github": github_profile, "github_user": user_profile, diff --git a/tests/functional/resources/test_connection.py b/tests/functional/resources/test_connection.py index eabbd1da..0d1d48a4 100644 --- a/tests/functional/resources/test_connection.py +++ b/tests/functional/resources/test_connection.py @@ -122,8 +122,9 @@ def test_empty_decl_derived_user(resource_funcs, res_opts, github_profile, for_r opts = copy.deepcopy(res_opts) opts["github"] = { "token": github_profile["token"], - "api_url": github_profile["api_url"], } + if api_url := github_profile.get("api_url"): + opts["github"]["api_url"] = api_url opts["pillar"]["resources"]["github"] = {} resource_funcs["github.init"](opts) assert resource_funcs["github.discover"](opts) == ["user1/repo-a", "user1/user-repo"] @@ -152,9 +153,10 @@ def test_empty_decl_profile_owner(resource_funcs, res_opts, github_profile): opts = copy.deepcopy(res_opts) opts["github"] = { "token": github_profile["token"], - "api_url": github_profile["api_url"], "owner": "testorg", } + if api_url := github_profile.get("api_url"): + opts["github"]["api_url"] = api_url opts["pillar"]["resources"]["github"] = {} resource_funcs["github.init"](opts) assert resource_funcs["github.discover"](opts) == ["testorg/repo-a", "testorg/repo-b"] @@ -171,8 +173,9 @@ def test_decl_alternative_profile_with_exclude(resource_funcs, res_opts, github_ opts.pop("github_noprivacy", None) opts["github_repos"] = { "token": github_profile["token"], - "api_url": github_profile["api_url"], } + if api_url := github_profile.get("api_url"): + opts["github_repos"]["api_url"] = api_url opts["pillar"]["resources"]["github"] = { "profile": "github_repos", "exclude": ["user-*"], diff --git a/tests/functional/resources/test_multi_profile.py b/tests/functional/resources/test_multi_profile.py index 53b9dfb4..04bb770d 100644 --- a/tests/functional/resources/test_multi_profile.py +++ b/tests/functional/resources/test_multi_profile.py @@ -30,9 +30,10 @@ def res_opts(minion_opts, resource_decl, github_profile): # scoped to the other organization. opts["github_other"] = { "token": github_profile["token"], - "api_url": github_profile["api_url"], "owner": "otherorg", } + if api_url := github_profile.get("api_url"): + opts["github_other"]["api_url"] = api_url return opts From 5b98591f882c216fadd085972692cbf3a1c1716b Mon Sep 17 00:00:00 2001 From: jeanluc Date: Wed, 14 Oct 2026 11:18:48 +0200 Subject: [PATCH 2/3] Cleanup and increase coverage --- .../+legacy_states_catch_errors.fixed.md | 1 + changelog/+partial_changes.fixed.md | 1 + docs/ref/utils/index.rst | 1 + .../saltext.github.utils.gh.resource.rst | 5 + src/saltext/github/modules/github.py | 105 ++--- src/saltext/github/modules/github_org.py | 64 ++- src/saltext/github/modules/github_repo.py | 169 +++----- .../resources/github/modules/actions.py | 37 +- .../resources/github/modules/deploy_key.py | 29 +- .../resources/github/modules/environment.py | 43 +- .../github/resources/github/modules/issue.py | 25 +- .../github/resources/github/modules/label.py | 33 +- .../resources/github/modules/milestone.py | 28 +- .../github/resources/github/modules/pages.py | 29 +- .../github/resources/github/modules/pull.py | 17 +- .../github/resources/github/modules/repo.py | 33 +- .../resources/github/modules/ruleset.py | 35 +- .../github/resources/github/states/actions.py | 88 ++-- .../resources/github/states/deploy_key.py | 64 +-- .../resources/github/states/environment.py | 68 +-- .../github/resources/github/states/label.py | 64 +-- .../github/resources/github/states/pages.py | 70 ++-- .../github/resources/github/states/repo.py | 83 ++-- .../github/resources/github/states/ruleset.py | 62 +-- src/saltext/github/states/github.py | 10 + src/saltext/github/states/github_org.py | 335 ++++++++------- src/saltext/github/states/github_repo.py | 326 +++++++-------- src/saltext/github/utils/gh/actions.py | 31 +- src/saltext/github/utils/gh/client.py | 201 +++++---- src/saltext/github/utils/gh/deploy_keys.py | 19 +- src/saltext/github/utils/gh/environments.py | 36 +- src/saltext/github/utils/gh/issues.py | 12 +- src/saltext/github/utils/gh/labels.py | 17 +- src/saltext/github/utils/gh/milestones.py | 35 +- src/saltext/github/utils/gh/pages.py | 18 +- src/saltext/github/utils/gh/pulls.py | 4 +- src/saltext/github/utils/gh/resource.py | 35 ++ src/saltext/github/utils/gh/rulesets.py | 12 +- .../github/utils/gh/statecores/__init__.py | 46 +++ .../utils/gh/statecores/environments.py | 32 +- .../github/utils/gh/statecores/repo.py | 92 ++--- src/saltext/github/utils/gh/teams.py | 9 +- tests/functional/conftest.py | 7 +- .../resources/modules/test_milestone.py | 28 ++ .../functional/resources/states/test_label.py | 6 +- tests/unit/modules/test_github.py | 226 ++++------ tests/unit/modules/test_github_org.py | 42 ++ tests/unit/states/test_error_handling.py | 138 +++++++ tests/unit/states/test_github.py | 315 ++++---------- tests/unit/states/test_github_failures.py | 389 +++++++++--------- tests/unit/states/test_github_org.py | 30 ++ tests/unit/utils/test_client.py | 35 +- tests/unit/utils/test_statecores.py | 40 ++ .../utils/test_statecores_environments.py | 44 ++ tests/unit/utils/test_statecores_repo.py | 71 ++++ tests/unit/utils/test_teams.py | 16 + 56 files changed, 1901 insertions(+), 1910 deletions(-) create mode 100644 changelog/+legacy_states_catch_errors.fixed.md create mode 100644 changelog/+partial_changes.fixed.md create mode 100644 docs/ref/utils/saltext.github.utils.gh.resource.rst create mode 100644 src/saltext/github/utils/gh/resource.py create mode 100644 tests/unit/modules/test_github_org.py create mode 100644 tests/unit/states/test_error_handling.py create mode 100644 tests/unit/states/test_github_org.py create mode 100644 tests/unit/utils/test_statecores.py create mode 100644 tests/unit/utils/test_statecores_environments.py create mode 100644 tests/unit/utils/test_statecores_repo.py create mode 100644 tests/unit/utils/test_teams.py diff --git a/changelog/+legacy_states_catch_errors.fixed.md b/changelog/+legacy_states_catch_errors.fixed.md new file mode 100644 index 00000000..02f875a9 --- /dev/null +++ b/changelog/+legacy_states_catch_errors.fixed.md @@ -0,0 +1 @@ +Fixed legacy `github` state functions rendering a traceback instead of a failed state when an unexpected `CommandExecutionError` was raised, e.g. caused by missing profile configuration diff --git a/changelog/+partial_changes.fixed.md b/changelog/+partial_changes.fixed.md new file mode 100644 index 00000000..819af824 --- /dev/null +++ b/changelog/+partial_changes.fixed.md @@ -0,0 +1 @@ +Fixed compound states (`github_repo.present`, `github_org.team_present`, environment management) reporting no changes when failing after some changes had already been applied diff --git a/docs/ref/utils/index.rst b/docs/ref/utils/index.rst index a0661f5a..dbc58f5c 100644 --- a/docs/ref/utils/index.rst +++ b/docs/ref/utils/index.rst @@ -24,6 +24,7 @@ _________ utils.gh.profile utils.gh.pulls utils.gh.repos + utils.gh.resource utils.gh.rulesets utils.gh.statecores utils.gh.statecores.actions diff --git a/docs/ref/utils/saltext.github.utils.gh.resource.rst b/docs/ref/utils/saltext.github.utils.gh.resource.rst new file mode 100644 index 00000000..38978855 --- /dev/null +++ b/docs/ref/utils/saltext.github.utils.gh.resource.rst @@ -0,0 +1,5 @@ +saltext.github.utils.gh.resource +================================ + +.. automodule:: saltext.github.utils.gh.resource + :members: diff --git a/src/saltext/github/modules/github.py b/src/saltext/github/modules/github.py index 58717b24..97d7082a 100644 --- a/src/saltext/github/modules/github.py +++ b/src/saltext/github/modules/github.py @@ -59,6 +59,10 @@ def __virtual__(): ) +# Anything below here is deprecated. Replacement functions are in github_org/github_repo. +# This module is intended for non-org/non-repo functionality now. + + def _get_conn(profile): """ Return the GitHub connection, cached into __context__ for performance @@ -66,6 +70,32 @@ def _get_conn(profile): return gh_profile.get_conn(profile, opts=__opts__, context=__context__) +def _get_conn_repo(profile, repo_name) -> tuple[object, str]: + return _get_conn(profile), repo_name or _get_config_value(profile, "repo_name") + + +def _get_config_value(profile, config_name): + """ + Helper function that returns a profile's configuration value based on + the supplied configuration name. + + profile + The profile name that contains configuration information. + + config_name + The configuration item's name to use to return configuration values. + """ + return gh_profile.get_config_value(profile, config_name, opts=__opts__, context=__context__) + + +def _get_owner(profile): + """ + Helper function that returns a profile's default repository owner + (``owner``, legacy alias ``org_name``), or None when unset. + """ + return gh_profile.get_owner(profile, opts=__opts__, context=__context__) + + def get_user(name, profile="github", user_details=False): """ Get a GitHub user by name. @@ -107,32 +137,6 @@ def get_user(name, profile="github", user_details=False): return False -# Anything below here is deprecated. Replacement functions are in github_org/github_repo. -# This module is intended for non-org/non-repo functionality now. - - -def _get_config_value(profile, config_name): - """ - Helper function that returns a profile's configuration value based on - the supplied configuration name. - - profile - The profile name that contains configuration information. - - config_name - The configuration item's name to use to return configuration values. - """ - return gh_profile.get_config_value(profile, config_name, opts=__opts__, context=__context__) - - -def _get_owner(profile): - """ - Helper function that returns a profile's default repository owner - (``owner``, legacy alias ``org_name``), or None when unset. - """ - return gh_profile.get_owner(profile, opts=__opts__, context=__context__) - - def list_users(profile="github", ignore_cache=False): """ List all users within the organization. @@ -240,12 +244,9 @@ def get_issue(issue_number, repo_name=None, profile="github", output="min"): salt myminion github.get_issue 514 salt myminion github.get_issue 514 repo_name=salt """ - if repo_name is None: - repo_name = _get_config_value(profile, "repo_name") - ret = {} try: - issue_data = gh_issues.get(_get_conn(profile), repo_name, issue_number) + issue_data = gh_issues.get(*_get_conn_repo(profile, repo_name), issue_number) except GitHubApiError: return ret @@ -294,10 +295,9 @@ def get_issue_comments(issue_number, repo_name=None, profile="github", since=Non salt myminion github.get_issue_comments 514 salt myminion github.get_issue 514 repo_name=salt """ - if repo_name is None: - repo_name = _get_config_value(profile, "repo_name") - - comments = gh_issues.get_comments(_get_conn(profile), repo_name, issue_number, since=since) + comments = gh_issues.get_comments( + *_get_conn_repo(profile, repo_name), issue_number, since=since + ) ret = {} for comment in comments or []: @@ -400,12 +400,8 @@ def get_issues( salt myminion github.get_issues my-github-repo """ - if repo_name is None: - repo_name = _get_config_value(profile, "repo_name") - issues = gh_issues.list_all( - _get_conn(profile), - repo_name, + *_get_conn_repo(profile, repo_name), milestone=milestone, state=state, assignee=assignee, @@ -479,12 +475,8 @@ def get_milestones( salt myminion github.get_milestones """ - if repo_name is None: - repo_name = _get_config_value(profile, "repo_name") - milestones = gh_milestones.list_all( - _get_conn(profile), - repo_name, + *_get_conn_repo(profile, repo_name), state=state, sort=sort, direction=direction, @@ -536,17 +528,14 @@ 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 = {} - if not (number or name): raise CommandExecutionError("Either a milestone 'name' or 'number' must be provided.") - if repo_name is None: - repo_name = _get_config_value(profile, "repo_name") + ret = {} if number: try: - milestone_data = gh_milestones.get(_get_conn(profile), repo_name, number) + milestone_data = gh_milestones.get(*_get_conn_repo(profile, repo_name), number) except GitHubApiError: return ret milestone_id = (milestone_data or {}).get("id") @@ -1093,17 +1082,13 @@ def add_team_repo(repo_name, team_name, profile="github", permission=None): log.error("Team %s does not exist", team_name) return False try: - if not gh_teams.add_repo(conn, team_info, repo_name, permission=permission): - log.error( - "Failed setting permission %s on repo %s for team %s", - permission, - repo_name, - team_name, - ) - return False + gh_teams.add_repo(conn, team_info, repo_name, permission=permission) except GitHubNotFoundError: log.error("Resource not found: %s", team_info["id"], exc_info_on_loglevel=logging.DEBUG) return False + except CommandExecutionError as err: + log.error(str(err), exc_info_on_loglevel=logging.DEBUG) + return False return True @@ -1387,12 +1372,8 @@ def get_prs( salt myminion github.get_prs salt myminion github.get_prs base=2016.11 """ - if repo_name is None: - repo_name = _get_config_value(profile, "repo_name") - prs = gh_pulls.list_all( - _get_conn(profile), - repo_name, + *_get_conn_repo(profile, repo_name), state=state, head=head, base=base, diff --git a/src/saltext/github/modules/github_org.py b/src/saltext/github/modules/github_org.py index f1ff6f83..78dbca1f 100644 --- a/src/saltext/github/modules/github_org.py +++ b/src/saltext/github/modules/github_org.py @@ -33,8 +33,6 @@ import logging -from salt.exceptions import CommandExecutionError - from saltext.github.utils import filter_fields from saltext.github.utils.gh import members as gh_members from saltext.github.utils.gh import profile as gh_profile @@ -50,13 +48,6 @@ def _get_conn(profile): return gh_profile.get_conn(profile, opts=__opts__, context=__context__) -def _get_team(conn, name, owner=None): - """ - Return a team's cached details, raising when it does not exist. - """ - return gh_teams.find(conn, name, owner=owner) - - def list_members(owner=None, ignore_cache=False, profile="github"): """ List the logins of all members of the organization. @@ -204,7 +195,7 @@ def get_team(name, owner=None, profile="github"): profile Profile configuration name to use. Defaults to ``github``. """ - return _strip_team_caches(_get_team(_get_conn(profile), name, owner=owner)) + return _strip_team_caches(gh_teams.find(_get_conn(profile), name, owner=owner)) def add_team( @@ -248,9 +239,8 @@ def add_team( profile Profile configuration name to use. Defaults to ``github``. """ - conn = _get_conn(profile) - gh_teams.create( - conn, + ght = _get_conn(profile).bind(gh_teams) + ght.create( name, description=description, repo_names=repo_names, @@ -258,7 +248,7 @@ def add_team( permission=permission, owner=owner, ) - return _strip_team_caches(_get_team(conn, name, owner=owner)) + return _strip_team_caches(ght.find(name, owner=owner)) def edit_team(name, description=None, privacy=None, permission=None, owner=None, profile="github"): @@ -291,17 +281,16 @@ def edit_team(name, description=None, privacy=None, permission=None, owner=None, profile Profile configuration name to use. Defaults to ``github``. """ - conn = _get_conn(profile) - gh_teams.update( - conn, - _get_team(conn, name, owner=owner), + ght = _get_conn(profile).bind(gh_teams) + ght.update( + ght.find(name, owner=owner), name, description=description, privacy=privacy, permission=permission, owner=owner, ) - return _strip_team_caches(_get_team(conn, name, owner=owner)) + return _strip_team_caches(ght.find(name, owner=owner)) def remove_team(name, owner=None, profile="github"): @@ -323,8 +312,8 @@ def remove_team(name, owner=None, profile="github"): profile Profile configuration name to use. Defaults to ``github``. """ - conn = _get_conn(profile) - gh_teams.delete(conn, _get_team(conn, name, owner=owner), owner=owner) + ght = _get_conn(profile).bind(gh_teams) + ght.delete(ght.find(name, owner=owner), owner=owner) return True @@ -355,10 +344,8 @@ def list_team_repos(team_name, owner=None, fields=None, ignore_cache=False, prof profile Profile configuration name to use. Defaults to ``github``. """ - conn = _get_conn(profile) - repos = gh_teams.list_repos( - conn, _get_team(conn, team_name, owner=owner), owner=owner, ignore_cache=ignore_cache - ) + ght = _get_conn(profile).bind(gh_teams) + repos = ght.list_repos(ght.find(team_name, owner=owner), owner=owner, ignore_cache=ignore_cache) return filter_fields({repo["name"]: repo for repo in repos.values()}, fields) @@ -389,13 +376,8 @@ def add_team_repo(repo_name, team_name, permission=None, owner=None, profile="gi profile Profile configuration name to use. Defaults to ``github``. """ - conn = _get_conn(profile) - if not gh_teams.add_repo( - conn, _get_team(conn, team_name, owner=owner), repo_name, permission=permission, owner=owner - ): - raise CommandExecutionError( - f"Failed setting permission {permission} on repo {repo_name} for team {team_name}." - ) + ght = _get_conn(profile).bind(gh_teams) + ght.add_repo(ght.find(team_name, owner=owner), repo_name, permission=permission, owner=owner) return True @@ -421,8 +403,8 @@ def remove_team_repo(repo_name, team_name, owner=None, profile="github"): profile Profile configuration name to use. Defaults to ``github``. """ - conn = _get_conn(profile) - gh_teams.remove_repo(conn, _get_team(conn, team_name, owner=owner), repo_name, owner=owner) + ght = _get_conn(profile).bind(gh_teams) + ght.remove_repo(ght.find(team_name, owner=owner), repo_name, owner=owner) return True @@ -448,9 +430,9 @@ def list_team_members(team_name, owner=None, ignore_cache=False, profile="github profile Profile configuration name to use. Defaults to ``github``. """ - conn = _get_conn(profile) - return gh_teams.list_members( - conn, _get_team(conn, team_name, owner=owner), owner=owner, ignore_cache=ignore_cache + ght = _get_conn(profile).bind(gh_teams) + return ght.list_members( + ght.find(team_name, owner=owner), owner=owner, ignore_cache=ignore_cache ) @@ -501,8 +483,8 @@ def add_team_member(name, team_name, owner=None, profile="github"): profile Profile configuration name to use. Defaults to ``github``. """ - conn = _get_conn(profile) - gh_teams.add_member(conn, _get_team(conn, team_name, owner=owner), name, owner=owner) + ght = _get_conn(profile).bind(gh_teams) + ght.add_member(ght.find(team_name, owner=owner), name, owner=owner) return True @@ -528,8 +510,8 @@ def remove_team_member(name, team_name, owner=None, profile="github"): profile Profile configuration name to use. Defaults to ``github``. """ - conn = _get_conn(profile) - gh_teams.remove_member(conn, _get_team(conn, team_name, owner=owner), name, owner=owner) + ght = _get_conn(profile).bind(gh_teams) + ght.remove_member(ght.find(team_name, owner=owner), name, owner=owner) return True diff --git a/src/saltext/github/modules/github_repo.py b/src/saltext/github/modules/github_repo.py index 11cc0aab..4ba38451 100644 --- a/src/saltext/github/modules/github_repo.py +++ b/src/saltext/github/modules/github_repo.py @@ -54,7 +54,6 @@ from saltext.github.utils.gh import pulls as gh_pulls from saltext.github.utils.gh import repos as gh_repos from saltext.github.utils.gh import rulesets as gh_rulesets -from saltext.github.utils.gh.exceptions import GitHubNotFoundError log = logging.getLogger(__name__) @@ -62,12 +61,16 @@ __func_alias__ = {"list_": "list"} +def _get_config_value(profile, config_name): + return gh_profile.get_config_value(profile, config_name, opts=__opts__, context=__context__) + + def _get_conn(profile): return gh_profile.get_conn(profile, opts=__opts__, context=__context__) -def _get_config_value(profile, config_name): - return gh_profile.get_config_value(profile, config_name, opts=__opts__, context=__context__) +def _get_conn_repo(profile, repo_name) -> tuple[object, str]: + return _get_conn(profile), repo_name or _get_config_value(profile, "repo_name") def list_(private=None, owner=None, fields=None, ignore_cache=False, profile="github"): @@ -176,10 +179,8 @@ def create( profile Profile configuration name to use. Defaults to ``github``. """ - repo_name = _resolve_repo_name(repo_name, profile) return gh_repos.create( - _get_conn(profile), - repo_name, + *_get_conn_repo(profile, repo_name), owner=owner, description=description, homepage=homepage, @@ -215,8 +216,9 @@ def get_info(repo_name=None, owner=None, ignore_cache=False, profile="github"): profile Profile configuration name to use. Defaults to ``github``. """ - repo_name = _resolve_repo_name(repo_name, profile) - return gh_repos.get_info(_get_conn(profile), repo_name, owner=owner, ignore_cache=ignore_cache) + return gh_repos.get_info( + *_get_conn_repo(profile, repo_name), owner=owner, ignore_cache=ignore_cache + ) def get_teams(repo_name=None, owner=None, fields=None, profile="github"): @@ -243,8 +245,7 @@ def get_teams(repo_name=None, owner=None, fields=None, profile="github"): profile Profile configuration name to use. Defaults to ``github``. """ - repo_name = _resolve_repo_name(repo_name, profile) - teams = gh_repos.get_teams(_get_conn(profile), repo_name, owner=owner) + teams = gh_repos.get_teams(*_get_conn_repo(profile, repo_name), owner=owner) return filter_fields({team["name"]: team for team in teams}, fields) @@ -301,14 +302,12 @@ def update( profile Profile configuration name to use. Defaults to ``github``. """ - repo_name = _resolve_repo_name(repo_name, profile) try: allow_privacy_change = _get_config_value(profile, "allow_repo_privacy_changes") except CommandExecutionError: allow_privacy_change = False return gh_repos.update( - _get_conn(profile), - repo_name, + *_get_conn_repo(profile, repo_name), owner=owner, description=description, homepage=homepage, @@ -340,8 +339,7 @@ def delete(repo_name=None, owner=None, profile="github"): profile Profile configuration name to use. Defaults to ``github``. """ - repo_name = _resolve_repo_name(repo_name, profile) - gh_repos.delete(_get_conn(profile), repo_name, owner=owner) + gh_repos.delete(*_get_conn_repo(profile, repo_name), owner=owner) return True @@ -419,10 +417,8 @@ def list_issues( profile Profile configuration name to use. Defaults to ``github``. """ - repo_name = _resolve_repo_name(repo_name, profile) issues = gh_issues.list_all( - _get_conn(profile), - repo_name, + *_get_conn_repo(profile, repo_name), milestone=milestone, state=state, assignee=assignee, @@ -460,8 +456,7 @@ def get_issue(number, repo_name=None, owner=None, profile="github"): profile Profile configuration name to use. Defaults to ``github``. """ - repo_name = _resolve_repo_name(repo_name, profile) - return gh_issues.get(_get_conn(profile), repo_name, number, owner=owner) + return gh_issues.get(*_get_conn_repo(profile, repo_name), number, owner=owner) def get_issue_comments(number, repo_name=None, owner=None, since=None, profile="github"): @@ -490,8 +485,9 @@ def get_issue_comments(number, repo_name=None, owner=None, since=None, profile=" profile Profile configuration name to use. Defaults to ``github``. """ - repo_name = _resolve_repo_name(repo_name, profile) - return gh_issues.get_comments(_get_conn(profile), repo_name, number, since=since, owner=owner) + return gh_issues.get_comments( + *_get_conn_repo(profile, repo_name), number, since=since, owner=owner + ) def list_milestones( @@ -540,10 +536,8 @@ def list_milestones( profile Profile configuration name to use. Defaults to ``github``. """ - repo_name = _resolve_repo_name(repo_name, profile) milestones = gh_milestones.list_all( - _get_conn(profile), - repo_name, + *_get_conn_repo(profile, repo_name), state=state, sort=sort, direction=direction, @@ -582,15 +576,9 @@ def get_milestone(number=None, name=None, repo_name=None, owner=None, profile="g if not (number or name): raise CommandExecutionError("Either a milestone 'name' or 'number' must be provided.") - repo_name = _resolve_repo_name(repo_name, profile) if number: - return gh_milestones.get(_get_conn(profile), repo_name, number, owner=owner) - - milestones = gh_milestones.list_all(_get_conn(profile), repo_name, state="all", owner=owner) - for milestone in milestones or []: - if milestone.get("title") == name: - return milestone - raise GitHubNotFoundError(f"Milestone {name} was not found in repository {repo_name}.") + return gh_milestones.get(*_get_conn_repo(profile, repo_name), number, owner=owner) + return gh_milestones.find_by_title(*_get_conn_repo(profile, repo_name), name, owner=owner) def list_pulls( @@ -649,10 +637,8 @@ def list_pulls( profile Profile configuration name to use. Defaults to ``github``. """ - repo_name = _resolve_repo_name(repo_name, profile) pulls = gh_pulls.list_all( - _get_conn(profile), - repo_name, + *_get_conn_repo(profile, repo_name), state=state, head=head, base=base, @@ -690,8 +676,7 @@ def list_rulesets(repo_name=None, owner=None, per_page=None, fields=None, profil profile Profile configuration name to use. Defaults to ``github``. """ - repo_name = _resolve_repo_name(repo_name, profile) - conn = _get_conn(profile) + conn, repo_name = _get_conn_repo(profile, repo_name) rulesets = gh_rulesets.list_all( conn, "repo", @@ -724,8 +709,7 @@ def get_ruleset(ruleset_id, repo_name=None, owner=None, profile="github"): profile Profile configuration name to use. Defaults to ``github``. """ - repo_name = _resolve_repo_name(repo_name, profile) - conn = _get_conn(profile) + conn, repo_name = _get_conn_repo(profile, repo_name) return gh_rulesets.get(conn, ruleset_id, "repo", owner=owner, repo_name=repo_name) @@ -752,8 +736,7 @@ def add_ruleset(ruleset_params, repo_name=None, owner=None, profile="github"): profile Profile configuration name to use. Defaults to ``github``. """ - repo_name = _resolve_repo_name(repo_name, profile) - conn = _get_conn(profile) + conn, repo_name = _get_conn_repo(profile, repo_name) return gh_rulesets.create(conn, ruleset_params, "repo", owner=owner, repo_name=repo_name) @@ -783,8 +766,7 @@ def update_ruleset(ruleset_id, ruleset_params, repo_name=None, owner=None, profi profile Profile configuration name to use. Defaults to ``github``. """ - repo_name = _resolve_repo_name(repo_name, profile) - conn = _get_conn(profile) + conn, repo_name = _get_conn_repo(profile, repo_name) return gh_rulesets.update( conn, ruleset_id, @@ -817,21 +799,11 @@ def delete_ruleset(ruleset_id, repo_name=None, owner=None, profile="github"): profile Profile configuration name to use. Defaults to ``github``. """ - repo_name = _resolve_repo_name(repo_name, profile) - conn = _get_conn(profile) + conn, repo_name = _get_conn_repo(profile, repo_name) gh_rulesets.delete(conn, ruleset_id, "repo", owner=owner, repo_name=repo_name) return True -def _resolve_repo_name(repo_name, profile): - """ - Default ``repo_name`` to the profile configuration when unset. - """ - if repo_name is None: - return _get_config_value(profile, "repo_name") - return repo_name - - def list_deploy_keys(repo_name=None, owner=None, fields=None, profile="github"): """ List the deploy keys of a repository. @@ -855,9 +827,8 @@ def list_deploy_keys(repo_name=None, owner=None, fields=None, profile="github"): profile Profile configuration name to use. Defaults to ``github``. """ - repo_name = _resolve_repo_name(repo_name, profile) return filter_fields( - gh_deploy_keys.list_all(_get_conn(profile), repo_name, owner=owner), fields + gh_deploy_keys.list_all(*_get_conn_repo(profile, repo_name), owner=owner), fields ) @@ -883,8 +854,7 @@ def get_deploy_key(key_id, repo_name=None, owner=None, profile="github"): profile Profile configuration name to use. Defaults to ``github``. """ - repo_name = _resolve_repo_name(repo_name, profile) - return gh_deploy_keys.get(_get_conn(profile), repo_name, key_id, owner=owner) + return gh_deploy_keys.get(*_get_conn_repo(profile, repo_name), key_id, owner=owner) def add_deploy_key(title, key, read_only=False, repo_name=None, owner=None, profile="github"): @@ -918,9 +888,8 @@ def add_deploy_key(title, key, read_only=False, repo_name=None, owner=None, prof profile Profile configuration name to use. Defaults to ``github``. """ - repo_name = _resolve_repo_name(repo_name, profile) return gh_deploy_keys.create( - _get_conn(profile), repo_name, title, key, read_only=bool(read_only), owner=owner + *_get_conn_repo(profile, repo_name), title, key, read_only=bool(read_only), owner=owner ) @@ -946,8 +915,7 @@ def remove_deploy_key(key_id, repo_name=None, owner=None, profile="github"): profile Profile configuration name to use. Defaults to ``github``. """ - repo_name = _resolve_repo_name(repo_name, profile) - gh_deploy_keys.delete(_get_conn(profile), repo_name, key_id, owner=owner) + gh_deploy_keys.delete(*_get_conn_repo(profile, repo_name), key_id, owner=owner) return True @@ -976,8 +944,7 @@ def list_actions_secrets(repo_name=None, owner=None, fields=None, profile="githu profile Profile configuration name to use. Defaults to ``github``. """ - repo_name = _resolve_repo_name(repo_name, profile) - secrets = gh_actions.list_secrets(_get_conn(profile), repo_name, owner=owner) + secrets = gh_actions.list_secrets(*_get_conn_repo(profile, repo_name), owner=owner) return filter_fields({secret["name"]: secret for secret in secrets}, fields) @@ -1004,8 +971,7 @@ def get_actions_secret(name, repo_name=None, owner=None, profile="github"): profile Profile configuration name to use. Defaults to ``github``. """ - repo_name = _resolve_repo_name(repo_name, profile) - return gh_actions.get_secret(_get_conn(profile), repo_name, name, owner=owner) + return gh_actions.get_secret(*_get_conn_repo(profile, repo_name), name, owner=owner) def set_actions_secret(name, value, repo_name=None, owner=None, profile="github"): @@ -1034,8 +1000,7 @@ def set_actions_secret(name, value, repo_name=None, owner=None, profile="github" profile Profile configuration name to use. Defaults to ``github``. """ - repo_name = _resolve_repo_name(repo_name, profile) - gh_actions.set_secret(_get_conn(profile), repo_name, name, value, owner=owner) + gh_actions.set_secret(*_get_conn_repo(profile, repo_name), name, value, owner=owner) return True @@ -1061,8 +1026,7 @@ def remove_actions_secret(name, repo_name=None, owner=None, profile="github"): profile Profile configuration name to use. Defaults to ``github``. """ - repo_name = _resolve_repo_name(repo_name, profile) - gh_actions.delete_secret(_get_conn(profile), repo_name, name, owner=owner) + gh_actions.delete_secret(*_get_conn_repo(profile, repo_name), name, owner=owner) return True @@ -1087,8 +1051,7 @@ def get_workflow_permissions(repo_name=None, owner=None, profile="github"): profile Profile configuration name to use. Defaults to ``github``. """ - repo_name = _resolve_repo_name(repo_name, profile) - return gh_actions.get_workflow_permissions(_get_conn(profile), repo_name, owner=owner) + return gh_actions.get_workflow_permissions(*_get_conn_repo(profile, repo_name), owner=owner) def set_workflow_permissions( @@ -1124,10 +1087,8 @@ def set_workflow_permissions( profile Profile configuration name to use. Defaults to ``github``. """ - repo_name = _resolve_repo_name(repo_name, profile) gh_actions.set_workflow_permissions( - _get_conn(profile), - repo_name, + *_get_conn_repo(profile, repo_name), default_workflow_permissions=default_workflow_permissions, can_approve_pull_request_reviews=can_approve_pull_request_reviews, owner=owner, @@ -1159,8 +1120,7 @@ def list_environments(repo_name=None, owner=None, fields=None, profile="github") profile Profile configuration name to use. Defaults to ``github``. """ - repo_name = _resolve_repo_name(repo_name, profile) - environments = gh_environments.list_all(_get_conn(profile), repo_name, owner=owner) + environments = gh_environments.list_all(*_get_conn_repo(profile, repo_name), owner=owner) return filter_fields({env["name"]: env for env in environments}, fields) @@ -1186,8 +1146,7 @@ def get_environment(name, repo_name=None, owner=None, profile="github"): profile Profile configuration name to use. Defaults to ``github``. """ - repo_name = _resolve_repo_name(repo_name, profile) - return gh_environments.get(_get_conn(profile), repo_name, name, owner=owner) + return gh_environments.get(*_get_conn_repo(profile, repo_name), name, owner=owner) def set_environment( @@ -1244,10 +1203,8 @@ def set_environment( profile Profile configuration name to use. Defaults to ``github``. """ - repo_name = _resolve_repo_name(repo_name, profile) return gh_environments.set_( - _get_conn(profile), - repo_name, + *_get_conn_repo(profile, repo_name), name, wait_timer=wait_timer, prevent_self_review=prevent_self_review, @@ -1279,8 +1236,7 @@ def remove_environment(name, repo_name=None, owner=None, profile="github"): profile Profile configuration name to use. Defaults to ``github``. """ - repo_name = _resolve_repo_name(repo_name, profile) - gh_environments.delete(_get_conn(profile), repo_name, name, owner=owner) + gh_environments.delete(*_get_conn_repo(profile, repo_name), name, owner=owner) return True @@ -1312,10 +1268,9 @@ def list_deployment_branch_policies( profile Profile configuration name to use. Defaults to ``github``. """ - repo_name = _resolve_repo_name(repo_name, profile) return filter_fields( gh_environments.list_branch_policies( - _get_conn(profile), repo_name, environment, owner=owner + *_get_conn_repo(profile, repo_name), environment, owner=owner ), fields, ) @@ -1356,9 +1311,8 @@ def add_deployment_branch_policy( profile Profile configuration name to use. Defaults to ``github``. """ - repo_name = _resolve_repo_name(repo_name, profile) return gh_environments.create_branch_policy( - _get_conn(profile), repo_name, environment, name, typ=typ, owner=owner + *_get_conn_repo(profile, repo_name), environment, name, typ=typ, owner=owner ) @@ -1389,9 +1343,8 @@ def remove_deployment_branch_policy( profile Profile configuration name to use. Defaults to ``github``. """ - repo_name = _resolve_repo_name(repo_name, profile) gh_environments.delete_branch_policy( - _get_conn(profile), repo_name, environment, policy_id, owner=owner + *_get_conn_repo(profile, repo_name), environment, policy_id, owner=owner ) return True @@ -1420,8 +1373,7 @@ def list_labels(repo_name=None, owner=None, fields=None, profile="github"): profile Profile configuration name to use. Defaults to ``github``. """ - repo_name = _resolve_repo_name(repo_name, profile) - labels = gh_labels.list_all(_get_conn(profile), repo_name, owner=owner) + labels = gh_labels.list_all(*_get_conn_repo(profile, repo_name), owner=owner) return filter_fields({label["name"]: label for label in labels}, fields) @@ -1447,8 +1399,7 @@ def get_label(name, repo_name=None, owner=None, profile="github"): profile Profile configuration name to use. Defaults to ``github``. """ - repo_name = _resolve_repo_name(repo_name, profile) - return gh_labels.get(_get_conn(profile), repo_name, name, owner=owner) + return gh_labels.get(*_get_conn_repo(profile, repo_name), name, owner=owner) def add_label(name, color=None, description=None, repo_name=None, owner=None, profile="github"): @@ -1480,9 +1431,12 @@ def add_label(name, color=None, description=None, repo_name=None, owner=None, pr profile Profile configuration name to use. Defaults to ``github``. """ - repo_name = _resolve_repo_name(repo_name, profile) return gh_labels.create( - _get_conn(profile), repo_name, name, color=color, description=description, owner=owner + *_get_conn_repo(profile, repo_name), + name, + color=color, + description=description, + owner=owner, ) @@ -1520,10 +1474,8 @@ def edit_label( profile Profile configuration name to use. Defaults to ``github``. """ - repo_name = _resolve_repo_name(repo_name, profile) return gh_labels.update( - _get_conn(profile), - repo_name, + *_get_conn_repo(profile, repo_name), name, new_name=new_name, color=color, @@ -1554,8 +1506,7 @@ def remove_label(name, repo_name=None, owner=None, profile="github"): profile Profile configuration name to use. Defaults to ``github``. """ - repo_name = _resolve_repo_name(repo_name, profile) - gh_labels.delete(_get_conn(profile), repo_name, name, owner=owner) + gh_labels.delete(*_get_conn_repo(profile, repo_name), name, owner=owner) return True @@ -1578,8 +1529,7 @@ def get_pages(repo_name=None, owner=None, profile="github"): profile Profile configuration name to use. Defaults to ``github``. """ - repo_name = _resolve_repo_name(repo_name, profile) - return gh_pages.get(_get_conn(profile), repo_name, owner=owner) + return gh_pages.get(*_get_conn_repo(profile, repo_name), owner=owner) def setup_pages( @@ -1620,10 +1570,8 @@ def setup_pages( profile Profile configuration name to use. Defaults to ``github``. """ - repo_name = _resolve_repo_name(repo_name, profile) return gh_pages.create( - _get_conn(profile), - repo_name, + *_get_conn_repo(profile, repo_name), build_type=build_type, source=gh_pages.build_source(source_branch, source_path), owner=owner, @@ -1676,10 +1624,8 @@ def update_pages( profile Profile configuration name to use. Defaults to ``github``. """ - repo_name = _resolve_repo_name(repo_name, profile) gh_pages.update( - _get_conn(profile), - repo_name, + *_get_conn_repo(profile, repo_name), build_type=build_type, source=gh_pages.build_source(source_branch, source_path), cname=cname, @@ -1708,6 +1654,5 @@ def remove_pages(repo_name=None, owner=None, profile="github"): profile Profile configuration name to use. Defaults to ``github``. """ - repo_name = _resolve_repo_name(repo_name, profile) - gh_pages.delete(_get_conn(profile), repo_name, owner=owner) + gh_pages.delete(*_get_conn_repo(profile, repo_name), owner=owner) return True diff --git a/src/saltext/github/resources/github/modules/actions.py b/src/saltext/github/resources/github/modules/actions.py index d5216034..dbcc78b1 100644 --- a/src/saltext/github/resources/github/modules/actions.py +++ b/src/saltext/github/resources/github/modules/actions.py @@ -11,21 +11,16 @@ import logging +from salt.utils.functools import namespaced_function + from saltext.github.utils import filter_fields from saltext.github.utils.gh import actions as gh_actions -from saltext.github.utils.gh import profile as gh_profile +from saltext.github.utils.gh import resource as gh_resource log = logging.getLogger(__name__) -def _conn(profile=None): - owner, _ = _ident() - return gh_profile.resource_conn(__opts__, __context__, profile=profile, owner=owner) - - -def _ident(): - owner, _, repo_name = __resource__["id"].partition("/") - return owner, repo_name +_core_args = namespaced_function(gh_resource.core_args, globals()) def list_secrets(*, fields=None, profile=None): @@ -48,8 +43,8 @@ def list_secrets(*, fields=None, profile=None): Profile configuration name to use. Defaults to the resource declaration's profile. """ - owner, repo_name = _ident() - secrets = gh_actions.list_secrets(_conn(profile), repo_name, owner=owner) + conn, owner, repo_name = _core_args(profile) + secrets = gh_actions.list_secrets(conn, repo_name, owner=owner) return filter_fields({secret["name"]: secret for secret in secrets}, fields) @@ -71,8 +66,8 @@ def get_secret(name, *, profile=None): Profile configuration name to use. Defaults to the resource declaration's profile. """ - owner, repo_name = _ident() - return gh_actions.get_secret(_conn(profile), repo_name, name, owner=owner) + conn, owner, repo_name = _core_args(profile) + return gh_actions.get_secret(conn, repo_name, name, owner=owner) def set_secret(name, value, *, profile=None): @@ -97,8 +92,8 @@ def set_secret(name, value, *, profile=None): Profile configuration name to use. Defaults to the resource declaration's profile. """ - owner, repo_name = _ident() - gh_actions.set_secret(_conn(profile), repo_name, name, value, owner=owner) + conn, owner, repo_name = _core_args(profile) + gh_actions.set_secret(conn, repo_name, name, value, owner=owner) return True @@ -119,8 +114,8 @@ def remove_secret(name, *, profile=None): Profile configuration name to use. Defaults to the resource declaration's profile. """ - owner, repo_name = _ident() - gh_actions.delete_secret(_conn(profile), repo_name, name, owner=owner) + conn, owner, repo_name = _core_args(profile) + gh_actions.delete_secret(conn, repo_name, name, owner=owner) return True @@ -140,8 +135,8 @@ def get_workflow_permissions(*, profile=None): Profile configuration name to use. Defaults to the resource declaration's profile. """ - owner, repo_name = _ident() - return gh_actions.get_workflow_permissions(_conn(profile), repo_name, owner=owner) + conn, owner, repo_name = _core_args(profile) + return gh_actions.get_workflow_permissions(conn, repo_name, owner=owner) def set_workflow_permissions( @@ -169,9 +164,9 @@ def set_workflow_permissions( Profile configuration name to use. Defaults to the resource declaration's profile. """ - owner, repo_name = _ident() + conn, owner, repo_name = _core_args(profile) gh_actions.set_workflow_permissions( - _conn(profile), + conn, repo_name, default_workflow_permissions=default_workflow_permissions, can_approve_pull_request_reviews=can_approve_pull_request_reviews, diff --git a/src/saltext/github/resources/github/modules/deploy_key.py b/src/saltext/github/resources/github/modules/deploy_key.py index c197d218..1fdf9ba7 100644 --- a/src/saltext/github/resources/github/modules/deploy_key.py +++ b/src/saltext/github/resources/github/modules/deploy_key.py @@ -10,23 +10,18 @@ import logging +from salt.utils.functools import namespaced_function + from saltext.github.utils import filter_fields from saltext.github.utils.gh import deploy_keys as gh_deploy_keys -from saltext.github.utils.gh import profile as gh_profile +from saltext.github.utils.gh import resource as gh_resource log = logging.getLogger(__name__) __func_alias__ = {"list_": "list"} -def _conn(profile=None): - owner, _ = _ident() - return gh_profile.resource_conn(__opts__, __context__, profile=profile, owner=owner) - - -def _ident(): - owner, _, repo_name = __resource__["id"].partition("/") - return owner, repo_name +_core_args = namespaced_function(gh_resource.core_args, globals()) def list_(*, fields=None, profile=None): @@ -47,8 +42,8 @@ def list_(*, fields=None, profile=None): Profile configuration name to use. Defaults to the resource declaration's profile. """ - owner, repo_name = _ident() - return filter_fields(gh_deploy_keys.list_all(_conn(profile), repo_name, owner=owner), fields) + conn, owner, repo_name = _core_args(profile) + return filter_fields(gh_deploy_keys.list_all(conn, repo_name, owner=owner), fields) def get(key_id, *, profile=None): @@ -68,8 +63,8 @@ def get(key_id, *, profile=None): Profile configuration name to use. Defaults to the resource declaration's profile. """ - owner, repo_name = _ident() - return gh_deploy_keys.get(_conn(profile), repo_name, key_id, owner=owner) + conn, owner, repo_name = _core_args(profile) + return gh_deploy_keys.get(conn, repo_name, key_id, owner=owner) def add(title, key, *, read_only=False, profile=None): @@ -99,9 +94,9 @@ def add(title, key, *, read_only=False, profile=None): Profile configuration name to use. Defaults to the resource declaration's profile. """ - owner, repo_name = _ident() + conn, owner, repo_name = _core_args(profile) return gh_deploy_keys.create( - _conn(profile), repo_name, title, key, read_only=bool(read_only), owner=owner + conn, repo_name, title, key, read_only=bool(read_only), owner=owner ) @@ -122,6 +117,6 @@ def remove(key_id, *, profile=None): Profile configuration name to use. Defaults to the resource declaration's profile. """ - owner, repo_name = _ident() - gh_deploy_keys.delete(_conn(profile), repo_name, key_id, owner=owner) + conn, owner, repo_name = _core_args(profile) + gh_deploy_keys.delete(conn, repo_name, key_id, owner=owner) return True diff --git a/src/saltext/github/resources/github/modules/environment.py b/src/saltext/github/resources/github/modules/environment.py index e7a5397a..02cdd5d9 100644 --- a/src/saltext/github/resources/github/modules/environment.py +++ b/src/saltext/github/resources/github/modules/environment.py @@ -10,23 +10,18 @@ import logging +from salt.utils.functools import namespaced_function + from saltext.github.utils import filter_fields from saltext.github.utils.gh import environments as gh_environments -from saltext.github.utils.gh import profile as gh_profile +from saltext.github.utils.gh import resource as gh_resource log = logging.getLogger(__name__) __func_alias__ = {"list_": "list", "set_": "set"} -def _conn(profile=None): - owner, _ = _ident() - return gh_profile.resource_conn(__opts__, __context__, profile=profile, owner=owner) - - -def _ident(): - owner, _, repo_name = __resource__["id"].partition("/") - return owner, repo_name +_core_args = namespaced_function(gh_resource.core_args, globals()) def list_(*, fields=None, profile=None): @@ -48,8 +43,8 @@ def list_(*, fields=None, profile=None): Profile configuration name to use. Defaults to the resource declaration's profile. """ - owner, repo_name = _ident() - environments = gh_environments.list_all(_conn(profile), repo_name, owner=owner) + conn, owner, repo_name = _core_args(profile) + environments = gh_environments.list_all(conn, repo_name, owner=owner) return filter_fields({env["name"]: env for env in environments}, fields) @@ -70,8 +65,8 @@ def get(name, *, profile=None): Profile configuration name to use. Defaults to the resource declaration's profile. """ - owner, repo_name = _ident() - return gh_environments.get(_conn(profile), repo_name, name, owner=owner) + conn, owner, repo_name = _core_args(profile) + return gh_environments.get(conn, repo_name, name, owner=owner) def set_( @@ -122,9 +117,9 @@ def set_( Profile configuration name to use. Defaults to the resource declaration's profile. """ - owner, repo_name = _ident() + conn, owner, repo_name = _core_args(profile) return gh_environments.set_( - _conn(profile), + conn, repo_name, name, wait_timer=wait_timer, @@ -152,8 +147,8 @@ def remove(name, *, profile=None): Profile configuration name to use. Defaults to the resource declaration's profile. """ - owner, repo_name = _ident() - gh_environments.delete(_conn(profile), repo_name, name, owner=owner) + conn, owner, repo_name = _core_args(profile) + gh_environments.delete(conn, repo_name, name, owner=owner) return True @@ -179,9 +174,9 @@ def list_branch_policies(environment, *, fields=None, profile=None): Profile configuration name to use. Defaults to the resource declaration's profile. """ - owner, repo_name = _ident() + conn, owner, repo_name = _core_args(profile) return filter_fields( - gh_environments.list_branch_policies(_conn(profile), repo_name, environment, owner=owner), + gh_environments.list_branch_policies(conn, repo_name, environment, owner=owner), fields, ) @@ -214,9 +209,9 @@ def add_branch_policy(environment, name, *, typ="branch", profile=None): Profile configuration name to use. Defaults to the resource declaration's profile. """ - owner, repo_name = _ident() + conn, owner, repo_name = _core_args(profile) return gh_environments.create_branch_policy( - _conn(profile), repo_name, environment, name, typ=typ, owner=owner + conn, repo_name, environment, name, typ=typ, owner=owner ) @@ -241,8 +236,6 @@ def remove_branch_policy(environment, policy_id, *, profile=None): Profile configuration name to use. Defaults to the resource declaration's profile. """ - owner, repo_name = _ident() - gh_environments.delete_branch_policy( - _conn(profile), repo_name, environment, policy_id, owner=owner - ) + conn, owner, repo_name = _core_args(profile) + gh_environments.delete_branch_policy(conn, repo_name, environment, policy_id, owner=owner) return True diff --git a/src/saltext/github/resources/github/modules/issue.py b/src/saltext/github/resources/github/modules/issue.py index 0513cbdb..1e623d9d 100644 --- a/src/saltext/github/resources/github/modules/issue.py +++ b/src/saltext/github/resources/github/modules/issue.py @@ -10,23 +10,18 @@ import logging +from salt.utils.functools import namespaced_function + from saltext.github.utils import filter_fields from saltext.github.utils.gh import issues as gh_issues -from saltext.github.utils.gh import profile as gh_profile +from saltext.github.utils.gh import resource as gh_resource log = logging.getLogger(__name__) __func_alias__ = {"list_": "list"} -def _conn(profile=None): - owner, _ = _ident() - return gh_profile.resource_conn(__opts__, __context__, profile=profile, owner=owner) - - -def _ident(): - owner, _, repo_name = __resource__["id"].partition("/") - return owner, repo_name +_core_args = namespaced_function(gh_resource.core_args, globals()) def list_( @@ -97,9 +92,9 @@ def list_( Profile configuration name to use. Defaults to the resource declaration's profile. """ - owner, repo_name = _ident() + conn, owner, repo_name = _core_args(profile) issues = gh_issues.list_all( - _conn(profile), + conn, repo_name, milestone=milestone, state=state, @@ -133,8 +128,8 @@ def get(number, *, profile=None): Profile configuration name to use. Defaults to the resource declaration's profile. """ - owner, repo_name = _ident() - return gh_issues.get(_conn(profile), repo_name, number, owner=owner) + conn, owner, repo_name = _core_args(profile) + return gh_issues.get(conn, repo_name, number, owner=owner) def get_comments(number, *, since=None, profile=None): @@ -158,5 +153,5 @@ def get_comments(number, *, since=None, profile=None): Profile configuration name to use. Defaults to the resource declaration's profile. """ - owner, repo_name = _ident() - return gh_issues.get_comments(_conn(profile), repo_name, number, since=since, owner=owner) + conn, owner, repo_name = _core_args(profile) + return gh_issues.get_comments(conn, repo_name, number, since=since, owner=owner) diff --git a/src/saltext/github/resources/github/modules/label.py b/src/saltext/github/resources/github/modules/label.py index cd13d3f3..690d2f6a 100644 --- a/src/saltext/github/resources/github/modules/label.py +++ b/src/saltext/github/resources/github/modules/label.py @@ -10,23 +10,18 @@ import logging +from salt.utils.functools import namespaced_function + from saltext.github.utils import filter_fields from saltext.github.utils.gh import labels as gh_labels -from saltext.github.utils.gh import profile as gh_profile +from saltext.github.utils.gh import resource as gh_resource log = logging.getLogger(__name__) __func_alias__ = {"list_": "list"} -def _conn(profile=None): - owner, _ = _ident() - return gh_profile.resource_conn(__opts__, __context__, profile=profile, owner=owner) - - -def _ident(): - owner, _, repo_name = __resource__["id"].partition("/") - return owner, repo_name +_core_args = namespaced_function(gh_resource.core_args, globals()) def list_(*, fields=None, profile=None): @@ -48,8 +43,8 @@ def list_(*, fields=None, profile=None): Profile configuration name to use. Defaults to the resource declaration's profile. """ - owner, repo_name = _ident() - labels = gh_labels.list_all(_conn(profile), repo_name, owner=owner) + conn, owner, repo_name = _core_args(profile) + labels = gh_labels.list_all(conn, repo_name, owner=owner) return filter_fields({label["name"]: label for label in labels}, fields) @@ -70,8 +65,8 @@ def get(name, *, profile=None): Profile configuration name to use. Defaults to the resource declaration's profile. """ - owner, repo_name = _ident() - return gh_labels.get(_conn(profile), repo_name, name, owner=owner) + conn, owner, repo_name = _core_args(profile) + return gh_labels.get(conn, repo_name, name, owner=owner) def add(name, *, color=None, description=None, profile=None): @@ -99,9 +94,9 @@ def add(name, *, color=None, description=None, profile=None): Profile configuration name to use. Defaults to the resource declaration's profile. """ - owner, repo_name = _ident() + conn, owner, repo_name = _core_args(profile) return gh_labels.create( - _conn(profile), repo_name, name, color=color, description=description, owner=owner + conn, repo_name, name, color=color, description=description, owner=owner ) @@ -132,9 +127,9 @@ def edit(name, *, new_name=None, color=None, description=None, profile=None): Profile configuration name to use. Defaults to the resource declaration's profile. """ - owner, repo_name = _ident() + conn, owner, repo_name = _core_args(profile) return gh_labels.update( - _conn(profile), + conn, repo_name, name, new_name=new_name, @@ -161,6 +156,6 @@ def remove(name, *, profile=None): Profile configuration name to use. Defaults to the resource declaration's profile. """ - owner, repo_name = _ident() - gh_labels.delete(_conn(profile), repo_name, name, owner=owner) + conn, owner, repo_name = _core_args(profile) + gh_labels.delete(conn, repo_name, name, owner=owner) return True diff --git a/src/saltext/github/resources/github/modules/milestone.py b/src/saltext/github/resources/github/modules/milestone.py index e61cd551..3db2d673 100644 --- a/src/saltext/github/resources/github/modules/milestone.py +++ b/src/saltext/github/resources/github/modules/milestone.py @@ -11,25 +11,18 @@ import logging from salt.exceptions import CommandExecutionError +from salt.utils.functools import namespaced_function from saltext.github.utils import filter_fields from saltext.github.utils.gh import milestones as gh_milestones -from saltext.github.utils.gh import profile as gh_profile -from saltext.github.utils.gh.exceptions import GitHubNotFoundError +from saltext.github.utils.gh import resource as gh_resource log = logging.getLogger(__name__) __func_alias__ = {"list_": "list"} -def _conn(profile=None): - owner, _ = _ident() - return gh_profile.resource_conn(__opts__, __context__, profile=profile, owner=owner) - - -def _ident(): - owner, _, repo_name = __resource__["id"].partition("/") - return owner, repo_name +_core_args = namespaced_function(gh_resource.core_args, globals()) def list_( @@ -66,9 +59,9 @@ def list_( Profile configuration name to use. Defaults to the resource declaration's profile. """ - owner, repo_name = _ident() + conn, owner, repo_name = _core_args(profile) milestones = gh_milestones.list_all( - _conn(profile), + conn, repo_name, state=state, sort=sort, @@ -104,12 +97,7 @@ def get(number=None, *, name=None, profile=None): if not (number or name): raise CommandExecutionError("Either a milestone 'name' or 'number' must be provided.") - owner, repo_name = _ident() + conn, owner, repo_name = _core_args(profile) if number: - return gh_milestones.get(_conn(profile), repo_name, number, owner=owner) - - milestones = gh_milestones.list_all(_conn(profile), repo_name, state="all", owner=owner) - for milestone in milestones or []: - if milestone.get("title") == name: - return milestone - raise GitHubNotFoundError(f"Milestone {name} was not found in repository {repo_name}.") + return gh_milestones.get(conn, repo_name, number, owner=owner) + return gh_milestones.find_by_title(conn, repo_name, name, owner=owner) diff --git a/src/saltext/github/resources/github/modules/pages.py b/src/saltext/github/resources/github/modules/pages.py index 0361bd70..02dd1356 100644 --- a/src/saltext/github/resources/github/modules/pages.py +++ b/src/saltext/github/resources/github/modules/pages.py @@ -10,20 +10,15 @@ import logging +from salt.utils.functools import namespaced_function + from saltext.github.utils.gh import pages as gh_pages -from saltext.github.utils.gh import profile as gh_profile +from saltext.github.utils.gh import resource as gh_resource log = logging.getLogger(__name__) -def _conn(profile=None): - owner, _ = _ident() - return gh_profile.resource_conn(__opts__, __context__, profile=profile, owner=owner) - - -def _ident(): - owner, _, repo_name = __resource__["id"].partition("/") - return owner, repo_name +_core_args = namespaced_function(gh_resource.core_args, globals()) def get(*, profile=None): @@ -41,8 +36,8 @@ def get(*, profile=None): Profile configuration name to use. Defaults to the resource declaration's profile. """ - owner, repo_name = _ident() - return gh_pages.get(_conn(profile), repo_name, owner=owner) + conn, owner, repo_name = _core_args(profile) + return gh_pages.get(conn, repo_name, owner=owner) def setup(*, build_type=None, source_branch=None, source_path=None, profile=None): @@ -71,9 +66,9 @@ def setup(*, build_type=None, source_branch=None, source_path=None, profile=None Profile configuration name to use. Defaults to the resource declaration's profile. """ - owner, repo_name = _ident() + conn, owner, repo_name = _core_args(profile) return gh_pages.create( - _conn(profile), + conn, repo_name, build_type=build_type, source=gh_pages.build_source(source_branch, source_path), @@ -122,9 +117,9 @@ def update( Profile configuration name to use. Defaults to the resource declaration's profile. """ - owner, repo_name = _ident() + conn, owner, repo_name = _core_args(profile) gh_pages.update( - _conn(profile), + conn, repo_name, build_type=build_type, source=gh_pages.build_source(source_branch, source_path), @@ -149,6 +144,6 @@ def remove(*, profile=None): Profile configuration name to use. Defaults to the resource declaration's profile. """ - owner, repo_name = _ident() - gh_pages.delete(_conn(profile), repo_name, owner=owner) + conn, owner, repo_name = _core_args(profile) + gh_pages.delete(conn, repo_name, owner=owner) return True diff --git a/src/saltext/github/resources/github/modules/pull.py b/src/saltext/github/resources/github/modules/pull.py index fb503721..9250d81a 100644 --- a/src/saltext/github/resources/github/modules/pull.py +++ b/src/saltext/github/resources/github/modules/pull.py @@ -10,23 +10,18 @@ import logging +from salt.utils.functools import namespaced_function + from saltext.github.utils import filter_fields -from saltext.github.utils.gh import profile as gh_profile from saltext.github.utils.gh import pulls as gh_pulls +from saltext.github.utils.gh import resource as gh_resource log = logging.getLogger(__name__) __func_alias__ = {"list_": "list"} -def _conn(profile=None): - owner, _ = _ident() - return gh_profile.resource_conn(__opts__, __context__, profile=profile, owner=owner) - - -def _ident(): - owner, _, repo_name = __resource__["id"].partition("/") - return owner, repo_name +_core_args = namespaced_function(gh_resource.core_args, globals()) def list_( @@ -80,9 +75,9 @@ def list_( Profile configuration name to use. Defaults to the resource declaration's profile. """ - owner, repo_name = _ident() + conn, owner, repo_name = _core_args(profile) pulls = gh_pulls.list_all( - _conn(profile), + conn, repo_name, state=state, head=head, diff --git a/src/saltext/github/resources/github/modules/repo.py b/src/saltext/github/resources/github/modules/repo.py index 05fdd7ea..13e5db1c 100644 --- a/src/saltext/github/resources/github/modules/repo.py +++ b/src/saltext/github/resources/github/modules/repo.py @@ -11,26 +11,17 @@ import logging +from salt.utils.functools import namespaced_function + from saltext.github.utils import filter_fields from saltext.github.utils.gh import profile as gh_profile from saltext.github.utils.gh import repos as gh_repos +from saltext.github.utils.gh import resource as gh_resource log = logging.getLogger(__name__) -def _profile(profile): - owner, _ = _ident() - return profile or gh_profile.resource_owner_profile(__opts__, owner) - - -def _conn(profile=None): - owner, _ = _ident() - return gh_profile.resource_conn(__opts__, __context__, profile=profile, owner=owner) - - -def _ident(): - owner, _, repo_name = __resource__["id"].partition("/") - return owner, repo_name +_core_args = namespaced_function(gh_resource.core_args, globals()) def ident(): @@ -45,7 +36,7 @@ def ident(): salt 'github:my-org/my-repo' repo.ident """ - owner, repo_name = _ident() + owner, _, repo_name = __resource__["id"].partition("/") return {"id": __resource__["id"], "owner": owner, "repo": repo_name} @@ -66,8 +57,8 @@ def info(*, ignore_cache=False, profile=None): Profile configuration name to use. Defaults to the resource declaration's profile. """ - owner, repo_name = _ident() - return gh_repos.get_info(_conn(profile), repo_name, owner=owner, ignore_cache=ignore_cache) + conn, owner, repo_name = _core_args(profile) + return gh_repos.get_info(conn, repo_name, owner=owner, ignore_cache=ignore_cache) def get_teams(*, fields=None, profile=None): @@ -89,8 +80,8 @@ def get_teams(*, fields=None, profile=None): Profile configuration name to use. Defaults to the resource declaration's profile. """ - owner, repo_name = _ident() - teams = gh_repos.get_teams(_conn(profile), repo_name, owner=owner) + conn, owner, repo_name = _core_args(profile) + teams = gh_repos.get_teams(conn, repo_name, owner=owner) return filter_fields({team["name"]: team for team in teams}, fields) @@ -141,16 +132,16 @@ def update( Profile configuration name to use. Defaults to the resource declaration's profile. """ - owner, repo_name = _ident() + conn, owner, repo_name = _core_args(profile) allow_privacy_change = gh_profile.get_config_value( - _profile(profile), + profile or gh_profile.resource_owner_profile(__opts__, owner), "allow_repo_privacy_changes", opts=__opts__, context=__context__, default=False, ) return gh_repos.update( - _conn(profile), + conn, repo_name, owner=owner, description=description, diff --git a/src/saltext/github/resources/github/modules/ruleset.py b/src/saltext/github/resources/github/modules/ruleset.py index 62a5688f..ae2c5aee 100644 --- a/src/saltext/github/resources/github/modules/ruleset.py +++ b/src/saltext/github/resources/github/modules/ruleset.py @@ -10,8 +10,10 @@ import logging +from salt.utils.functools import namespaced_function + from saltext.github.utils import filter_fields -from saltext.github.utils.gh import profile as gh_profile +from saltext.github.utils.gh import resource as gh_resource from saltext.github.utils.gh import rulesets as gh_rulesets log = logging.getLogger(__name__) @@ -19,14 +21,7 @@ __func_alias__ = {"list_": "list"} -def _conn(profile=None): - owner, _ = _ident() - return gh_profile.resource_conn(__opts__, __context__, profile=profile, owner=owner) - - -def _ident(): - owner, _, repo_name = __resource__["id"].partition("/") - return owner, repo_name +_core_args = namespaced_function(gh_resource.core_args, globals()) def list_(*, per_page=None, fields=None, profile=None): @@ -50,9 +45,9 @@ def list_(*, per_page=None, fields=None, profile=None): Profile configuration name to use. Defaults to the resource declaration's profile. """ - owner, repo_name = _ident() + conn, owner, repo_name = _core_args(profile) rulesets = gh_rulesets.list_all( - _conn(profile), "repo", owner=owner, repo_name=repo_name, per_page=per_page + conn, "repo", owner=owner, repo_name=repo_name, per_page=per_page ) return filter_fields(rulesets, fields) @@ -74,8 +69,8 @@ def get(ruleset_id, *, profile=None): Profile configuration name to use. Defaults to the resource declaration's profile. """ - owner, repo_name = _ident() - return gh_rulesets.get(_conn(profile), ruleset_id, "repo", owner=owner, repo_name=repo_name) + conn, owner, repo_name = _core_args(profile) + return gh_rulesets.get(conn, ruleset_id, "repo", owner=owner, repo_name=repo_name) def add(ruleset_params, *, profile=None): @@ -96,10 +91,8 @@ def add(ruleset_params, *, profile=None): Profile configuration name to use. Defaults to the resource declaration's profile. """ - owner, repo_name = _ident() - return gh_rulesets.create( - _conn(profile), ruleset_params, "repo", owner=owner, repo_name=repo_name - ) + conn, owner, repo_name = _core_args(profile) + return gh_rulesets.create(conn, ruleset_params, "repo", owner=owner, repo_name=repo_name) def update(ruleset_id, ruleset_params, *, profile=None): @@ -123,9 +116,9 @@ def update(ruleset_id, ruleset_params, *, profile=None): Profile configuration name to use. Defaults to the resource declaration's profile. """ - owner, repo_name = _ident() + conn, owner, repo_name = _core_args(profile) return gh_rulesets.update( - _conn(profile), ruleset_id, ruleset_params, "repo", owner=owner, repo_name=repo_name + conn, ruleset_id, ruleset_params, "repo", owner=owner, repo_name=repo_name ) @@ -146,6 +139,6 @@ def remove(ruleset_id, *, profile=None): Profile configuration name to use. Defaults to the resource declaration's profile. """ - owner, repo_name = _ident() - gh_rulesets.delete(_conn(profile), ruleset_id, "repo", owner=owner, repo_name=repo_name) + conn, owner, repo_name = _core_args(profile) + gh_rulesets.delete(conn, ruleset_id, "repo", owner=owner, repo_name=repo_name) return True diff --git a/src/saltext/github/resources/github/states/actions.py b/src/saltext/github/resources/github/states/actions.py index 37a9ec1f..65d12e3b 100644 --- a/src/saltext/github/resources/github/states/actions.py +++ b/src/saltext/github/resources/github/states/actions.py @@ -12,38 +12,19 @@ import logging -from salt.exceptions import CommandExecutionError +from salt.utils.functools import namespaced_function -from saltext.github.utils.gh import profile as gh_profile +from saltext.github.utils.gh import resource as gh_resource from saltext.github.utils.gh.statecores import actions as sc_actions +from saltext.github.utils.gh.statecores import catch_errors log = logging.getLogger(__name__) -def _ident(): - """ - Resource identity. Salt's authoring docs promise ``__resource__`` - inside state overrides, but no released Salt packs it there — fall - back to the ``repo.ident`` execution override, which does see it. - """ - try: - rid = __resource__["id"] - except NameError: - return __salt__["repo.ident"]() - owner, _, repo_name = rid.partition("/") - return {"id": rid, "owner": owner, "repo": repo_name} - - -def _core_args(profile): - ident = _ident() - conn = gh_profile.resource_conn(__opts__, __context__, profile=profile, owner=ident["owner"]) - return conn, ident["owner"], ident["repo"] - - -def _failed(name, err): - return {"name": name, "changes": {}, "result": False, "comment": str(err)} +_core_args = namespaced_function(gh_resource.core_args, globals()) +@catch_errors def secret_present(name, value=None, force=False, profile=None): """ Ensure an Actions secret is present in the targeted repository. @@ -70,21 +51,19 @@ def secret_present(name, value=None, force=False, profile=None): Profile configuration name to use. Defaults to the resource declaration's profile. """ - try: - conn, owner, repo_name = _core_args(profile) - return sc_actions.secret_present( - conn, - name, - repo_name, - value=value, - force=force, - owner=owner, - test=__opts__["test"], - ) - except CommandExecutionError as err: - return _failed(name, err) - - + conn, owner, repo_name = _core_args(profile) + return sc_actions.secret_present( + conn, + name, + repo_name, + value=value, + force=force, + owner=owner, + test=__opts__["test"], + ) + + +@catch_errors def secret_absent(name, profile=None): """ Ensure an Actions secret is absent from the targeted repository. @@ -97,13 +76,11 @@ def secret_absent(name, profile=None): Profile configuration name to use. Defaults to the resource declaration's profile. """ - try: - conn, owner, repo_name = _core_args(profile) - return sc_actions.secret_absent(conn, name, repo_name, owner=owner, test=__opts__["test"]) - except CommandExecutionError as err: - return _failed(name, err) + conn, owner, repo_name = _core_args(profile) + return sc_actions.secret_absent(conn, name, repo_name, owner=owner, test=__opts__["test"]) +@catch_errors def workflow_permissions_managed( name, default_workflow_permissions=None, @@ -128,16 +105,13 @@ def workflow_permissions_managed( Profile configuration name to use. Defaults to the resource declaration's profile. """ - try: - conn, owner, repo_name = _core_args(profile) - return sc_actions.workflow_permissions_managed( - conn, - name, - repo_name, - default_workflow_permissions=default_workflow_permissions, - can_approve_pull_request_reviews=can_approve_pull_request_reviews, - owner=owner, - test=__opts__["test"], - ) - except CommandExecutionError as err: - return _failed(name, err) + conn, owner, repo_name = _core_args(profile) + return sc_actions.workflow_permissions_managed( + conn, + name, + repo_name, + default_workflow_permissions=default_workflow_permissions, + can_approve_pull_request_reviews=can_approve_pull_request_reviews, + owner=owner, + test=__opts__["test"], + ) diff --git a/src/saltext/github/resources/github/states/deploy_key.py b/src/saltext/github/resources/github/states/deploy_key.py index 2dddce80..805db21e 100644 --- a/src/saltext/github/resources/github/states/deploy_key.py +++ b/src/saltext/github/resources/github/states/deploy_key.py @@ -11,38 +11,19 @@ import logging -from salt.exceptions import CommandExecutionError +from salt.utils.functools import namespaced_function -from saltext.github.utils.gh import profile as gh_profile +from saltext.github.utils.gh import resource as gh_resource +from saltext.github.utils.gh.statecores import catch_errors from saltext.github.utils.gh.statecores import deploy_keys as sc_deploy_keys log = logging.getLogger(__name__) -def _ident(): - """ - Resource identity. Salt's authoring docs promise ``__resource__`` - inside state overrides, but no released Salt packs it there — fall - back to the ``repo.ident`` execution override, which does see it. - """ - try: - rid = __resource__["id"] - except NameError: - return __salt__["repo.ident"]() - owner, _, repo_name = rid.partition("/") - return {"id": rid, "owner": owner, "repo": repo_name} - - -def _core_args(profile): - ident = _ident() - conn = gh_profile.resource_conn(__opts__, __context__, profile=profile, owner=ident["owner"]) - return conn, ident["owner"], ident["repo"] - - -def _failed(name, err): - return {"name": name, "changes": {}, "result": False, "comment": str(err)} +_core_args = namespaced_function(gh_resource.core_args, globals()) +@catch_errors def present(name, key, read_only=False, profile=None): """ Ensure a deploy key with this title and public key is present @@ -74,21 +55,19 @@ def present(name, key, read_only=False, profile=None): Profile configuration name to use. Defaults to the resource declaration's profile. """ - try: - conn, owner, repo_name = _core_args(profile) - return sc_deploy_keys.present( - conn, - name, - key, - repo_name, - read_only=read_only, - owner=owner, - test=__opts__["test"], - ) - except CommandExecutionError as err: - return _failed(name, err) - - + conn, owner, repo_name = _core_args(profile) + return sc_deploy_keys.present( + conn, + name, + key, + repo_name, + read_only=read_only, + owner=owner, + test=__opts__["test"], + ) + + +@catch_errors def absent(name, profile=None): """ Ensure no deploy key with this title is present in the targeted @@ -101,8 +80,5 @@ def absent(name, profile=None): Profile configuration name to use. Defaults to the resource declaration's profile. """ - try: - conn, owner, repo_name = _core_args(profile) - return sc_deploy_keys.absent(conn, name, repo_name, owner=owner, test=__opts__["test"]) - except CommandExecutionError as err: - return _failed(name, err) + conn, owner, repo_name = _core_args(profile) + return sc_deploy_keys.absent(conn, name, repo_name, owner=owner, test=__opts__["test"]) diff --git a/src/saltext/github/resources/github/states/environment.py b/src/saltext/github/resources/github/states/environment.py index 66e9e4c4..5c2dc93d 100644 --- a/src/saltext/github/resources/github/states/environment.py +++ b/src/saltext/github/resources/github/states/environment.py @@ -11,38 +11,19 @@ import logging -from salt.exceptions import CommandExecutionError +from salt.utils.functools import namespaced_function -from saltext.github.utils.gh import profile as gh_profile +from saltext.github.utils.gh import resource as gh_resource +from saltext.github.utils.gh.statecores import catch_errors from saltext.github.utils.gh.statecores import environments as sc_environments log = logging.getLogger(__name__) -def _ident(): - """ - Resource identity. Salt's authoring docs promise ``__resource__`` - inside state overrides, but no released Salt packs it there — fall - back to the ``repo.ident`` execution override, which does see it. - """ - try: - rid = __resource__["id"] - except NameError: - return __salt__["repo.ident"]() - owner, _, repo_name = rid.partition("/") - return {"id": rid, "owner": owner, "repo": repo_name} - - -def _core_args(profile): - ident = _ident() - conn = gh_profile.resource_conn(__opts__, __context__, profile=profile, owner=ident["owner"]) - return conn, ident["owner"], ident["repo"] - - -def _failed(name, err): - return {"name": name, "changes": {}, "result": False, "comment": str(err)} +_core_args = namespaced_function(gh_resource.core_args, globals()) +@catch_errors def present( name, wait_timer=None, @@ -88,23 +69,21 @@ def present( Profile configuration name to use. Defaults to the resource declaration's profile. """ - try: - conn, owner, repo_name = _core_args(profile) - return sc_environments.present( - conn, - name, - repo_name, - wait_timer=wait_timer, - prevent_self_review=prevent_self_review, - reviewers=reviewers, - deployment_branches=deployment_branches, - owner=owner, - test=__opts__["test"], - ) - except CommandExecutionError as err: - return _failed(name, err) - - + conn, owner, repo_name = _core_args(profile) + return sc_environments.present( + conn, + name, + repo_name, + wait_timer=wait_timer, + prevent_self_review=prevent_self_review, + reviewers=reviewers, + deployment_branches=deployment_branches, + owner=owner, + test=__opts__["test"], + ) + + +@catch_errors def absent(name, profile=None): """ Ensure an environment is absent from the targeted repository. @@ -116,8 +95,5 @@ def absent(name, profile=None): Profile configuration name to use. Defaults to the resource declaration's profile. """ - try: - conn, owner, repo_name = _core_args(profile) - return sc_environments.absent(conn, name, repo_name, owner=owner, test=__opts__["test"]) - except CommandExecutionError as err: - return _failed(name, err) + conn, owner, repo_name = _core_args(profile) + return sc_environments.absent(conn, name, repo_name, owner=owner, test=__opts__["test"]) diff --git a/src/saltext/github/resources/github/states/label.py b/src/saltext/github/resources/github/states/label.py index a9eeaeae..720443a2 100644 --- a/src/saltext/github/resources/github/states/label.py +++ b/src/saltext/github/resources/github/states/label.py @@ -11,38 +11,19 @@ import logging -from salt.exceptions import CommandExecutionError +from salt.utils.functools import namespaced_function -from saltext.github.utils.gh import profile as gh_profile +from saltext.github.utils.gh import resource as gh_resource +from saltext.github.utils.gh.statecores import catch_errors from saltext.github.utils.gh.statecores import labels as sc_labels log = logging.getLogger(__name__) -def _ident(): - """ - Resource identity. Salt's authoring docs promise ``__resource__`` - inside state overrides, but no released Salt packs it there — fall - back to the ``repo.ident`` execution override, which does see it. - """ - try: - rid = __resource__["id"] - except NameError: - return __salt__["repo.ident"]() - owner, _, repo_name = rid.partition("/") - return {"id": rid, "owner": owner, "repo": repo_name} - - -def _core_args(profile): - ident = _ident() - conn = gh_profile.resource_conn(__opts__, __context__, profile=profile, owner=ident["owner"]) - return conn, ident["owner"], ident["repo"] - - -def _failed(name, err): - return {"name": name, "changes": {}, "result": False, "comment": str(err)} +_core_args = namespaced_function(gh_resource.core_args, globals()) +@catch_errors def present(name, color=None, description=None, profile=None): """ Ensure a label is present in the targeted repository and configured @@ -62,21 +43,19 @@ def present(name, color=None, description=None, profile=None): Profile configuration name to use. Defaults to the resource declaration's profile. """ - try: - conn, owner, repo_name = _core_args(profile) - return sc_labels.present( - conn, - name, - repo_name, - color=color, - description=description, - owner=owner, - test=__opts__["test"], - ) - except CommandExecutionError as err: - return _failed(name, err) - - + conn, owner, repo_name = _core_args(profile) + return sc_labels.present( + conn, + name, + repo_name, + color=color, + description=description, + owner=owner, + test=__opts__["test"], + ) + + +@catch_errors def absent(name, profile=None): """ Ensure a label is absent from the targeted repository. @@ -88,8 +67,5 @@ def absent(name, profile=None): Profile configuration name to use. Defaults to the resource declaration's profile. """ - try: - conn, owner, repo_name = _core_args(profile) - return sc_labels.absent(conn, name, repo_name, owner=owner, test=__opts__["test"]) - except CommandExecutionError as err: - return _failed(name, err) + conn, owner, repo_name = _core_args(profile) + return sc_labels.absent(conn, name, repo_name, owner=owner, test=__opts__["test"]) diff --git a/src/saltext/github/resources/github/states/pages.py b/src/saltext/github/resources/github/states/pages.py index a8eae493..45615442 100644 --- a/src/saltext/github/resources/github/states/pages.py +++ b/src/saltext/github/resources/github/states/pages.py @@ -11,38 +11,19 @@ import logging -from salt.exceptions import CommandExecutionError +from salt.utils.functools import namespaced_function -from saltext.github.utils.gh import profile as gh_profile +from saltext.github.utils.gh import resource as gh_resource +from saltext.github.utils.gh.statecores import catch_errors from saltext.github.utils.gh.statecores import pages as sc_pages log = logging.getLogger(__name__) -def _ident(): - """ - Resource identity. Salt's authoring docs promise ``__resource__`` - inside state overrides, but no released Salt packs it there — fall - back to the ``repo.ident`` execution override, which does see it. - """ - try: - rid = __resource__["id"] - except NameError: - return __salt__["repo.ident"]() - owner, _, repo_name = rid.partition("/") - return {"id": rid, "owner": owner, "repo": repo_name} - - -def _core_args(profile): - ident = _ident() - conn = gh_profile.resource_conn(__opts__, __context__, profile=profile, owner=ident["owner"]) - return conn, ident["owner"], ident["repo"] - - -def _failed(name, err): - return {"name": name, "changes": {}, "result": False, "comment": str(err)} +_core_args = namespaced_function(gh_resource.core_args, globals()) +@catch_errors def present( name, build_type=None, @@ -80,24 +61,22 @@ def present( Profile configuration name to use. Defaults to the resource declaration's profile. """ - try: - conn, owner, repo_name = _core_args(profile) - return sc_pages.present( - conn, - name, - repo_name, - build_type=build_type, - source_branch=source_branch, - source_path=source_path, - cname=cname, - https_enforced=https_enforced, - owner=owner, - test=__opts__["test"], - ) - except CommandExecutionError as err: - return _failed(name, err) - - + conn, owner, repo_name = _core_args(profile) + return sc_pages.present( + conn, + name, + repo_name, + build_type=build_type, + source_branch=source_branch, + source_path=source_path, + cname=cname, + https_enforced=https_enforced, + owner=owner, + test=__opts__["test"], + ) + + +@catch_errors def absent(name, profile=None): """ Ensure GitHub Pages are disabled for the targeted repository. @@ -109,8 +88,5 @@ def absent(name, profile=None): Profile configuration name to use. Defaults to the resource declaration's profile. """ - try: - conn, owner, repo_name = _core_args(profile) - return sc_pages.absent(conn, name, repo_name, owner=owner, test=__opts__["test"]) - except CommandExecutionError as err: - return _failed(name, err) + conn, owner, repo_name = _core_args(profile) + return sc_pages.absent(conn, name, repo_name, owner=owner, test=__opts__["test"]) diff --git a/src/saltext/github/resources/github/states/repo.py b/src/saltext/github/resources/github/states/repo.py index 50689745..ee42f7e6 100644 --- a/src/saltext/github/resources/github/states/repo.py +++ b/src/saltext/github/resources/github/states/repo.py @@ -11,38 +11,20 @@ import logging -from salt.exceptions import CommandExecutionError +from salt.utils.functools import namespaced_function from saltext.github.utils.gh import profile as gh_profile +from saltext.github.utils.gh import resource as gh_resource +from saltext.github.utils.gh.statecores import catch_errors from saltext.github.utils.gh.statecores import repo as sc_repo log = logging.getLogger(__name__) -def _ident(): - """ - Resource identity. Salt's authoring docs promise ``__resource__`` - inside state overrides, but no released Salt packs it there — fall - back to the ``repo.ident`` execution override, which does see it. - """ - try: - rid = __resource__["id"] - except NameError: - return __salt__["repo.ident"]() - owner, _, repo_name = rid.partition("/") - return {"id": rid, "owner": owner, "repo": repo_name} - - -def _core_args(profile): - ident = _ident() - conn = gh_profile.resource_conn(__opts__, __context__, profile=profile, owner=ident["owner"]) - return conn, ident["owner"], ident["repo"] - - -def _failed(name, err): - return {"name": name, "changes": {}, "result": False, "comment": str(err)} +_core_args = namespaced_function(gh_resource.core_args, globals()) +@catch_errors def managed( name, description=None, @@ -98,32 +80,29 @@ def managed( Profile configuration name to use. Defaults to the resource declaration's profile. """ - try: - conn, owner, repo_name = _core_args(profile) - allow_privacy_change = gh_profile.get_config_value( - profile or gh_profile.resource_owner_profile(__opts__, owner), - "allow_repo_privacy_changes", - opts=__opts__, - context=__context__, - default=False, - ) - ret = sc_repo.present( - conn, - repo_name, - owner=owner, - description=description, - homepage=homepage, - private=private, - has_issues=has_issues, - has_wiki=has_wiki, - has_downloads=has_downloads, - default_branch=default_branch, - teams=teams, - allow_privacy_change=allow_privacy_change, - allow_create=False, - test=__opts__["test"], - ) - ret["name"] = name - return ret - except CommandExecutionError as err: - return _failed(name, err) + conn, owner, repo_name = _core_args(profile) + allow_privacy_change = gh_profile.get_config_value( + profile or gh_profile.resource_owner_profile(__opts__, owner), + "allow_repo_privacy_changes", + opts=__opts__, + context=__context__, + default=False, + ) + ret = sc_repo.present( + conn, + repo_name, + owner=owner, + description=description, + homepage=homepage, + private=private, + has_issues=has_issues, + has_wiki=has_wiki, + has_downloads=has_downloads, + default_branch=default_branch, + teams=teams, + allow_privacy_change=allow_privacy_change, + allow_create=False, + test=__opts__["test"], + ) + ret["name"] = name + return ret diff --git a/src/saltext/github/resources/github/states/ruleset.py b/src/saltext/github/resources/github/states/ruleset.py index c0e2210e..83e6a933 100644 --- a/src/saltext/github/resources/github/states/ruleset.py +++ b/src/saltext/github/resources/github/states/ruleset.py @@ -11,38 +11,19 @@ import logging -from salt.exceptions import CommandExecutionError +from salt.utils.functools import namespaced_function -from saltext.github.utils.gh import profile as gh_profile +from saltext.github.utils.gh import resource as gh_resource +from saltext.github.utils.gh.statecores import catch_errors from saltext.github.utils.gh.statecores import rulesets as sc_rulesets log = logging.getLogger(__name__) -def _ident(): - """ - Resource identity. Salt's authoring docs promise ``__resource__`` - inside state overrides, but no released Salt packs it there — fall - back to the ``repo.ident`` execution override, which does see it. - """ - try: - rid = __resource__["id"] - except NameError: - return __salt__["repo.ident"]() - owner, _, repo_name = rid.partition("/") - return {"id": rid, "owner": owner, "repo": repo_name} - - -def _core_args(profile): - ident = _ident() - conn = gh_profile.resource_conn(__opts__, __context__, profile=profile, owner=ident["owner"]) - return conn, ident["owner"], ident["repo"] - - -def _failed(name, err): - return {"name": name, "changes": {}, "result": False, "comment": str(err)} +_core_args = namespaced_function(gh_resource.core_args, globals()) +@catch_errors def present(name, ruleset_params=None, profile=None): """ Ensure a ruleset is present in the targeted repository and @@ -73,20 +54,18 @@ def present(name, ruleset_params=None, profile=None): Profile configuration name to use. Defaults to the resource declaration's profile. """ - try: - conn, owner, repo_name = _core_args(profile) - return sc_rulesets.present( - conn, - name, - repo_name, - ruleset_params=ruleset_params, - owner=owner, - test=__opts__["test"], - ) - except CommandExecutionError as err: - return _failed(name, err) - - + conn, owner, repo_name = _core_args(profile) + return sc_rulesets.present( + conn, + name, + repo_name, + ruleset_params=ruleset_params, + owner=owner, + test=__opts__["test"], + ) + + +@catch_errors def absent(name, profile=None): """ Ensure a ruleset is absent from the targeted repository. @@ -98,8 +77,5 @@ def absent(name, profile=None): Profile configuration name to use. Defaults to the resource declaration's profile. """ - try: - conn, owner, repo_name = _core_args(profile) - return sc_rulesets.absent(conn, name, repo_name, owner=owner, test=__opts__["test"]) - except CommandExecutionError as err: - return _failed(name, err) + conn, owner, repo_name = _core_args(profile) + return sc_rulesets.absent(conn, name, repo_name, owner=owner, test=__opts__["test"]) diff --git a/src/saltext/github/states/github.py b/src/saltext/github/states/github.py index 8a623daf..a272723c 100644 --- a/src/saltext/github/states/github.py +++ b/src/saltext/github/states/github.py @@ -14,6 +14,8 @@ from salt.exceptions import CommandExecutionError +from saltext.github.utils.gh.statecores import catch_errors as _catch_errors + log = logging.getLogger(__name__) __virtualname__ = "github" @@ -28,6 +30,7 @@ def __virtual__(): return (False, "github module could not be loaded") # pragma: no cover +@_catch_errors def present(name, profile="github", **kwargs): """ Ensure a user is present @@ -80,6 +83,7 @@ def present(name, profile="github", **kwargs): return ret +@_catch_errors def absent(name, profile="github", **kwargs): """ Ensure a github user is absent @@ -138,6 +142,7 @@ def absent(name, profile="github", **kwargs): return ret +@_catch_errors def team_present( # pylint: disable=too-many-statements name, description=None, @@ -421,6 +426,7 @@ def _member_violates_mfa(member, member_info, mfa_deadline, members_without_mfa) return member.lower() in members_without_mfa and (mfa_deadline > enforce_mfa_from) +@_catch_errors def team_absent(name, profile="github", **kwargs): """ Ensure a team is absent. @@ -468,6 +474,7 @@ def team_absent(name, profile="github", **kwargs): return ret +@_catch_errors def repo_present( name, description=None, @@ -709,6 +716,7 @@ def repo_present( return ret +@_catch_errors def repo_absent(name, profile="github", **kwargs): """ Ensure a repo is absent. @@ -762,6 +770,7 @@ def repo_absent(name, profile="github", **kwargs): return ret +@_catch_errors def ruleset_absent( name, ruleset_type, profile="github", owner=None, repo_name=None, org_name=None, **kwargs ): @@ -840,6 +849,7 @@ def ruleset_absent( return ret +@_catch_errors def ruleset_present( name, ruleset_type, diff --git a/src/saltext/github/states/github_org.py b/src/saltext/github/states/github_org.py index 0ee223c1..0b028895 100644 --- a/src/saltext/github/states/github_org.py +++ b/src/saltext/github/states/github_org.py @@ -10,12 +10,15 @@ from saltext.github.utils import filter_unset from saltext.github.utils.gh.exceptions import GitHubNotFoundError +from saltext.github.utils.gh.statecores import StateError +from saltext.github.utils.gh.statecores import catch_errors log = logging.getLogger(__name__) __virtualname__ = "github_org" +@catch_errors def member_present(name, profile="github"): """ Ensure a user is a member of the organization. Since GitHub @@ -32,40 +35,36 @@ def member_present(name, profile="github"): ret = {"name": name, "changes": {}, "result": True, "comment": ""} try: - try: - info = __salt__["github_org.get_member"](name, profile=profile) - except GitHubNotFoundError as err: - # astroid on Python 3.10 fails to infer `err` as an exception - # FIXME: Remove this once CI lint runs on a more recent Python version - raise CommandExecutionError( # pylint: disable=bad-exception-cause - f"User {name} does not exist" - ) from err - - if info["in_org"]: - ret["comment"] = f"User {name} is already a member of the organization" - return ret - if info["membership_state"] == "pending": - ret["comment"] = f"User {name} has already been invited" - return ret - - ret["changes"]["invited"] = name - if __opts__["test"]: - ret["result"] = None - ret["comment"] = f"User {name} would have been invited" - return ret - - if __salt__["github_org.add_member"](name, profile=profile): - ret["comment"] = f"User {name} has been invited" - else: - ret["changes"] = {"added": name} - ret["comment"] = f"User {name} has been added" - except CommandExecutionError as err: - ret["result"] = False - ret["comment"] = str(err) - ret["changes"] = {} + info = __salt__["github_org.get_member"](name, profile=profile) + except GitHubNotFoundError as err: + # astroid on Python 3.10 fails to infer `err` as an exception + # FIXME: Remove this once CI lint runs on a more recent Python version + raise CommandExecutionError( # pylint: disable=bad-exception-cause + f"User {name} does not exist" + ) from err + + if info["in_org"]: + ret["comment"] = f"User {name} is already a member of the organization" + return ret + if info["membership_state"] == "pending": + ret["comment"] = f"User {name} has already been invited" + return ret + + ret["changes"]["invited"] = name + if __opts__["test"]: + ret["result"] = None + ret["comment"] = f"User {name} would have been invited" + return ret + + if __salt__["github_org.add_member"](name, profile=profile): + ret["comment"] = f"User {name} has been invited" + else: + ret["changes"] = {"added": name} + ret["comment"] = f"User {name} has been added" return ret +@catch_errors def member_absent(name, profile="github"): """ Ensure a user is not a member of the organization. @@ -79,28 +78,23 @@ def member_absent(name, profile="github"): ret = {"name": name, "changes": {}, "result": True, "comment": ""} try: - try: - info = __salt__["github_org.get_member"](name, profile=profile) - except GitHubNotFoundError: - ret["comment"] = f"User {name} does not exist" - return ret - - if not info["in_org"]: - ret["comment"] = f"User {name} is not a member of the organization" - return ret - - ret["changes"]["removed"] = name - if __opts__["test"]: - ret["result"] = None - ret["comment"] = f"User {name} would have been removed from the organization" - return ret - - __salt__["github_org.remove_member"](name, profile=profile) - ret["comment"] = f"User {name} has been removed from the organization" - except CommandExecutionError as err: - ret["result"] = False - ret["comment"] = str(err) - ret["changes"] = {} + info = __salt__["github_org.get_member"](name, profile=profile) + except GitHubNotFoundError: + ret["comment"] = f"User {name} does not exist" + return ret + + if not info["in_org"]: + ret["comment"] = f"User {name} is not a member of the organization" + return ret + + ret["changes"]["removed"] = name + if __opts__["test"]: + ret["result"] = None + ret["comment"] = f"User {name} would have been removed from the organization" + return ret + + __salt__["github_org.remove_member"](name, profile=profile) + ret["comment"] = f"User {name} has been removed from the organization" return ret @@ -118,6 +112,7 @@ def _case_insensitive_diff(current, desired): return added, removed +@catch_errors def team_present( name, description=None, @@ -133,9 +128,9 @@ def team_present( .. note:: - Unlike the deprecated ``github.team_present`` state, this state - does not implement per-team MFA enforcement. Require 2FA for - the whole organization in its GitHub settings instead. + Unlike the deprecated :py:func:`github.team_present ` + state, this state does not implement per-team MFA enforcement. + Require 2FA for the whole organization in its GitHub settings instead. name Name of the team in the organization. @@ -168,75 +163,84 @@ def team_present( } try: - try: - target = __salt__["github_org.get_team"](name, profile=profile) - except GitHubNotFoundError: - target = None - - settings = filter_unset( - {"description": description, "privacy": privacy, "permission": permission} - ) - repos_added = repos_removed = members_added = members_removed = [] - - if target is None: - ret["changes"]["created"] = name - repos_added = sorted(repo_names or []) - members_added = sorted(members or []) - else: - diff = {key: val for key, val in settings.items() if target[key] != val} - if diff: - ret["changes"]["old"] = {key: target[key] for key in diff} - ret["changes"]["new"] = diff - if repo_names is not None: - current_repos = __salt__["github_org.list_team_repos"](name, profile=profile) - repos_added, repos_removed = _case_insensitive_diff(current_repos, repo_names) - if members is not None: - current_members = __salt__["github_org.list_team_members"](name, profile=profile) - members_added, members_removed = _case_insensitive_diff(current_members, members) - - for field, values in ( - ("repos_added", repos_added), - ("repos_removed", repos_removed), - ("members_added", members_added), - ("members_removed", members_removed), - ): - if values: - ret["changes"][field] = values - - if not ret["changes"]: - return ret - - verb = "created" if target is None else "updated" - if __opts__["test"]: - ret["result"] = None - ret["comment"] = f"Team {name} would have been {verb}" - return ret - + target = __salt__["github_org.get_team"](name, profile=profile) + except GitHubNotFoundError: + target = None + + settings = filter_unset( + {"description": description, "privacy": privacy, "permission": permission} + ) + repos_added = repos_removed = members_added = members_removed = [] + + if target is None: + ret["changes"]["created"] = name + repos_added = sorted(repo_names or []) + members_added = sorted(members or []) + else: + diff = {key: val for key, val in settings.items() if target[key] != val} + if diff: + ret["changes"]["old"] = {key: target[key] for key in diff} + ret["changes"]["new"] = diff + if repo_names is not None: + current_repos = __salt__["github_org.list_team_repos"](name, profile=profile) + repos_added, repos_removed = _case_insensitive_diff(current_repos, repo_names) + if members is not None: + current_members = __salt__["github_org.list_team_members"](name, profile=profile) + members_added, members_removed = _case_insensitive_diff(current_members, members) + + for field, values in ( + ("repos_added", repos_added), + ("repos_removed", repos_removed), + ("members_added", members_added), + ("members_removed", members_removed), + ): + if values: + ret["changes"][field] = values + + if not ret["changes"]: + return ret + + verb = "created" if target is None else "updated" + if __opts__["test"]: + ret["result"] = None + ret["comment"] = f"Team {name} would have been {verb}" + return ret + + applied = {} + try: if target is None: __salt__["github_org.add_team"]( name, repo_names=repo_names, profile=profile, **settings ) + applied["created"] = name + if repos_added: # Attached during team creation + applied["repos_added"] = repos_added else: if "new" in ret["changes"]: __salt__["github_org.edit_team"](name, profile=profile, **ret["changes"]["new"]) + applied["old"] = ret["changes"]["old"] + applied["new"] = ret["changes"]["new"] for repo_name in repos_added: __salt__["github_org.add_team_repo"](repo_name, name, profile=profile) + applied.setdefault("repos_added", []).append(repo_name) for repo_name in repos_removed: __salt__["github_org.remove_team_repo"](repo_name, name, profile=profile) + applied.setdefault("repos_removed", []).append(repo_name) for member in members_added: __salt__["github_org.add_team_member"](member, name, profile=profile) + applied.setdefault("members_added", []).append(member) for member in members_removed: __salt__["github_org.remove_team_member"](member, name, profile=profile) + applied.setdefault("members_removed", []).append(member) if members_added or members_removed: # Refresh team cache __salt__["github_org.list_team_members"](name, profile=profile, ignore_cache=True) - ret["comment"] = f"Team {name} has been {verb}" except CommandExecutionError as err: - ret["result"] = False - ret["comment"] = str(err) - ret["changes"] = {} + raise StateError(str(err), changes=applied) from err + ret["comment"] = f"Team {name} has been {verb}" return ret +@catch_errors def team_absent(name, profile="github"): """ Ensure a named team does not exist. @@ -250,27 +254,23 @@ def team_absent(name, profile="github"): ret = {"name": name, "changes": {}, "result": True, "comment": ""} try: - try: - __salt__["github_org.get_team"](name, profile=profile) - except GitHubNotFoundError: - ret["comment"] = f"Team {name} does not exist" - return ret - - ret["changes"]["deleted"] = name - if __opts__["test"]: - ret["result"] = None - ret["comment"] = f"Team {name} would have been deleted" - return ret - - __salt__["github_org.remove_team"](name, profile=profile) - ret["comment"] = f"Team {name} has been deleted" - except CommandExecutionError as err: - ret["result"] = False - ret["comment"] = str(err) - ret["changes"] = {} + __salt__["github_org.get_team"](name, profile=profile) + except GitHubNotFoundError: + ret["comment"] = f"Team {name} does not exist" + return ret + + ret["changes"]["deleted"] = name + if __opts__["test"]: + ret["result"] = None + ret["comment"] = f"Team {name} would have been deleted" + return ret + + __salt__["github_org.remove_team"](name, profile=profile) + ret["comment"] = f"Team {name} has been deleted" return ret +@catch_errors def ruleset_present(name, ruleset_params=None, profile="github"): """ Ensure an organization ruleset exists as specified. @@ -304,46 +304,42 @@ def ruleset_present(name, ruleset_params=None, profile="github"): ruleset_params = dict(ruleset_params or {}) ruleset_params["name"] = name - try: - rulesets = __salt__["github_org.list_rulesets"](profile=profile) - ruleset = next((rls for rls in rulesets or [] if rls["name"] == name), None) - - if ruleset is None: - ret["changes"]["created"] = name - if __opts__["test"]: - ret["result"] = None - ret["comment"] = f"Ruleset {name} would have been created" - return ret - __salt__["github_org.add_ruleset"](ruleset_params, profile=profile) - ret["comment"] = f"Ruleset {name} has been created" - return ret - - ruleset_info = __salt__["github_org.get_ruleset"](ruleset["id"], profile=profile) - - # 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: val for key, val in ruleset_params.items() if ruleset_info.get(key) != val} - if not diff: - ret["comment"] = f"Ruleset {name} is already configured as specified" - return ret + rulesets = __salt__["github_org.list_rulesets"](profile=profile) + ruleset = next((rls for rls in rulesets or [] if rls["name"] == name), None) - ret["changes"]["old"] = {key: ruleset_info.get(key) for key in diff} - ret["changes"]["new"] = diff + if ruleset is None: + ret["changes"]["created"] = name if __opts__["test"]: ret["result"] = None - ret["comment"] = f"Ruleset {name} would have been updated" + ret["comment"] = f"Ruleset {name} would have been created" return ret - - __salt__["github_org.update_ruleset"](ruleset["id"], ruleset_params, profile=profile) - ret["comment"] = f"Ruleset {name} has been updated" - except CommandExecutionError as err: - ret["result"] = False - ret["comment"] = str(err) - ret["changes"] = {} + __salt__["github_org.add_ruleset"](ruleset_params, profile=profile) + ret["comment"] = f"Ruleset {name} has been created" + return ret + + ruleset_info = __salt__["github_org.get_ruleset"](ruleset["id"], profile=profile) + + # 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: val for key, val in ruleset_params.items() if ruleset_info.get(key) != val} + if not diff: + ret["comment"] = f"Ruleset {name} is already configured as specified" + return ret + + ret["changes"]["old"] = {key: ruleset_info.get(key) for key in diff} + ret["changes"]["new"] = diff + if __opts__["test"]: + ret["result"] = None + ret["comment"] = f"Ruleset {name} would have been updated" + return ret + + __salt__["github_org.update_ruleset"](ruleset["id"], ruleset_params, profile=profile) + ret["comment"] = f"Ruleset {name} has been updated" return ret +@catch_errors def ruleset_absent(name, profile="github"): """ Ensure a named organization ruleset does not exist. @@ -356,23 +352,18 @@ def ruleset_absent(name, profile="github"): """ ret = {"name": name, "changes": {}, "result": True, "comment": ""} - try: - rulesets = __salt__["github_org.list_rulesets"](profile=profile) - ruleset = next((rls for rls in rulesets or [] if rls["name"] == name), None) - if ruleset is None: - ret["comment"] = f"Ruleset {name} does not exist" - return ret + rulesets = __salt__["github_org.list_rulesets"](profile=profile) + ruleset = next((rls for rls in rulesets or [] if rls["name"] == name), None) + if ruleset is None: + ret["comment"] = f"Ruleset {name} does not exist" + return ret - ret["changes"]["deleted"] = name - if __opts__["test"]: - ret["result"] = None - ret["comment"] = f"Ruleset {name} would have been deleted" - return ret + ret["changes"]["deleted"] = name + if __opts__["test"]: + ret["result"] = None + ret["comment"] = f"Ruleset {name} would have been deleted" + return ret - __salt__["github_org.delete_ruleset"](ruleset["id"], profile=profile) - ret["comment"] = f"Ruleset {name} has been deleted" - except CommandExecutionError as err: - ret["result"] = False - ret["comment"] = str(err) - ret["changes"] = {} + __salt__["github_org.delete_ruleset"](ruleset["id"], profile=profile) + ret["comment"] = f"Ruleset {name} has been deleted" return ret diff --git a/src/saltext/github/states/github_repo.py b/src/saltext/github/states/github_repo.py index 731257a3..0dae3e33 100644 --- a/src/saltext/github/states/github_repo.py +++ b/src/saltext/github/states/github_repo.py @@ -23,10 +23,9 @@ import logging -from salt.exceptions import CommandExecutionError - from saltext.github.utils.gh import profile as gh_profile from saltext.github.utils.gh.statecores import actions as sc_actions +from saltext.github.utils.gh.statecores import catch_errors from saltext.github.utils.gh.statecores import deploy_keys as sc_deploy_keys from saltext.github.utils.gh.statecores import environments as sc_environments from saltext.github.utils.gh.statecores import labels as sc_labels @@ -62,13 +61,7 @@ def _allow_privacy_change(profile): ) -def _failed(name, err): - """ - Render a raised error into a failed state return. - """ - return {"name": name, "changes": {}, "result": False, "comment": str(err)} - - +@catch_errors def present( name, description=None, @@ -142,29 +135,27 @@ def present( profile Profile configuration name to use. Defaults to ``github``. """ - try: - return sc_repo.present( - _get_conn(profile), - name, - owner=owner, - 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, - default_branch=default_branch, - teams=teams, - allow_privacy_change=_allow_privacy_change(profile), - test=__opts__["test"], - ) - except CommandExecutionError as err: - return _failed(name, err) + return sc_repo.present( + _get_conn(profile), + name, + owner=owner, + 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, + default_branch=default_branch, + teams=teams, + allow_privacy_change=_allow_privacy_change(profile), + test=__opts__["test"], + ) +@catch_errors def absent(name, owner=None, profile="github"): """ Ensure a repository is absent. @@ -178,12 +169,10 @@ def absent(name, owner=None, profile="github"): profile Profile configuration name to use. Defaults to ``github``. """ - try: - return sc_repo.absent(_get_conn(profile), name, owner=owner, test=__opts__["test"]) - except CommandExecutionError as err: - return _failed(name, err) + return sc_repo.absent(_get_conn(profile), name, owner=owner, test=__opts__["test"]) +@catch_errors def ruleset_present(name, ruleset_params=None, repo_name=None, owner=None, profile="github"): """ Ensure a repository ruleset is present and configured as specified. @@ -219,19 +208,17 @@ def ruleset_present(name, ruleset_params=None, repo_name=None, owner=None, profi profile Profile configuration name to use. Defaults to ``github``. """ - try: - return sc_rulesets.present( - _get_conn(profile), - name, - _resolve_repo_name(repo_name, profile), - ruleset_params=ruleset_params, - owner=owner, - test=__opts__["test"], - ) - except CommandExecutionError as err: - return _failed(name, err) + return sc_rulesets.present( + _get_conn(profile), + name, + _resolve_repo_name(repo_name, profile), + ruleset_params=ruleset_params, + owner=owner, + test=__opts__["test"], + ) +@catch_errors def ruleset_absent(name, repo_name=None, owner=None, profile="github"): """ Ensure a repository ruleset is absent. @@ -248,18 +235,16 @@ def ruleset_absent(name, repo_name=None, owner=None, profile="github"): profile Profile configuration name to use. Defaults to ``github``. """ - try: - return sc_rulesets.absent( - _get_conn(profile), - name, - _resolve_repo_name(repo_name, profile), - owner=owner, - test=__opts__["test"], - ) - except CommandExecutionError as err: - return _failed(name, err) + return sc_rulesets.absent( + _get_conn(profile), + name, + _resolve_repo_name(repo_name, profile), + owner=owner, + test=__opts__["test"], + ) +@catch_errors def deploy_key_present(name, key, read_only=False, repo_name=None, owner=None, profile="github"): """ Ensure a deploy key with this title and public key is present @@ -297,20 +282,18 @@ def deploy_key_present(name, key, read_only=False, repo_name=None, owner=None, p profile Profile configuration name to use. Defaults to ``github``. """ - try: - return sc_deploy_keys.present( - _get_conn(profile), - name, - key, - _resolve_repo_name(repo_name, profile), - read_only=read_only, - owner=owner, - test=__opts__["test"], - ) - except CommandExecutionError as err: - return _failed(name, err) + return sc_deploy_keys.present( + _get_conn(profile), + name, + key, + _resolve_repo_name(repo_name, profile), + read_only=read_only, + owner=owner, + test=__opts__["test"], + ) +@catch_errors def deploy_key_absent(name, repo_name=None, owner=None, profile="github"): """ Ensure no deploy key with this title is present in a repository. @@ -327,18 +310,16 @@ def deploy_key_absent(name, repo_name=None, owner=None, profile="github"): profile Profile configuration name to use. Defaults to ``github``. """ - try: - return sc_deploy_keys.absent( - _get_conn(profile), - name, - _resolve_repo_name(repo_name, profile), - owner=owner, - test=__opts__["test"], - ) - except CommandExecutionError as err: - return _failed(name, err) + return sc_deploy_keys.absent( + _get_conn(profile), + name, + _resolve_repo_name(repo_name, profile), + owner=owner, + test=__opts__["test"], + ) +@catch_errors def actions_secret_present( name, value=None, force=False, repo_name=None, owner=None, profile="github" ): @@ -372,20 +353,18 @@ def actions_secret_present( profile Profile configuration name to use. Defaults to ``github``. """ - try: - return sc_actions.secret_present( - _get_conn(profile), - name, - _resolve_repo_name(repo_name, profile), - value=value, - force=force, - owner=owner, - test=__opts__["test"], - ) - except CommandExecutionError as err: - return _failed(name, err) + return sc_actions.secret_present( + _get_conn(profile), + name, + _resolve_repo_name(repo_name, profile), + value=value, + force=force, + owner=owner, + test=__opts__["test"], + ) +@catch_errors def actions_secret_absent(name, repo_name=None, owner=None, profile="github"): """ Ensure an Actions secret is absent from a repository. @@ -403,18 +382,16 @@ def actions_secret_absent(name, repo_name=None, owner=None, profile="github"): profile Profile configuration name to use. Defaults to ``github``. """ - try: - return sc_actions.secret_absent( - _get_conn(profile), - name, - _resolve_repo_name(repo_name, profile), - owner=owner, - test=__opts__["test"], - ) - except CommandExecutionError as err: - return _failed(name, err) + return sc_actions.secret_absent( + _get_conn(profile), + name, + _resolve_repo_name(repo_name, profile), + owner=owner, + test=__opts__["test"], + ) +@catch_errors def workflow_permissions_managed( name, default_workflow_permissions=None, @@ -446,20 +423,18 @@ def workflow_permissions_managed( profile Profile configuration name to use. Defaults to ``github``. """ - try: - return sc_actions.workflow_permissions_managed( - _get_conn(profile), - name, - repo_name or name, - default_workflow_permissions=default_workflow_permissions, - can_approve_pull_request_reviews=can_approve_pull_request_reviews, - owner=owner, - test=__opts__["test"], - ) - except CommandExecutionError as err: - return _failed(name, err) + return sc_actions.workflow_permissions_managed( + _get_conn(profile), + name, + repo_name or name, + default_workflow_permissions=default_workflow_permissions, + can_approve_pull_request_reviews=can_approve_pull_request_reviews, + owner=owner, + test=__opts__["test"], + ) +@catch_errors def environment_present( name, wait_timer=None, @@ -503,22 +478,20 @@ def environment_present( profile Profile configuration name to use. Defaults to ``github``. """ - try: - return sc_environments.present( - _get_conn(profile), - name, - _resolve_repo_name(repo_name, profile), - wait_timer=wait_timer, - prevent_self_review=prevent_self_review, - reviewers=reviewers, - deployment_branches=deployment_branches, - owner=owner, - test=__opts__["test"], - ) - except CommandExecutionError as err: - return _failed(name, err) + return sc_environments.present( + _get_conn(profile), + name, + _resolve_repo_name(repo_name, profile), + wait_timer=wait_timer, + prevent_self_review=prevent_self_review, + reviewers=reviewers, + deployment_branches=deployment_branches, + owner=owner, + test=__opts__["test"], + ) +@catch_errors def environment_absent(name, repo_name=None, owner=None, profile="github"): """ Ensure an environment is absent from a repository. @@ -535,18 +508,16 @@ def environment_absent(name, repo_name=None, owner=None, profile="github"): profile Profile configuration name to use. Defaults to ``github``. """ - try: - return sc_environments.absent( - _get_conn(profile), - name, - _resolve_repo_name(repo_name, profile), - owner=owner, - test=__opts__["test"], - ) - except CommandExecutionError as err: - return _failed(name, err) + return sc_environments.absent( + _get_conn(profile), + name, + _resolve_repo_name(repo_name, profile), + owner=owner, + test=__opts__["test"], + ) +@catch_errors def label_present(name, color=None, description=None, repo_name=None, owner=None, profile="github"): """ Ensure a label is present in a repository and configured as @@ -571,20 +542,18 @@ def label_present(name, color=None, description=None, repo_name=None, owner=None profile Profile configuration name to use. Defaults to ``github``. """ - try: - return sc_labels.present( - _get_conn(profile), - name, - _resolve_repo_name(repo_name, profile), - color=color, - description=description, - owner=owner, - test=__opts__["test"], - ) - except CommandExecutionError as err: - return _failed(name, err) + return sc_labels.present( + _get_conn(profile), + name, + _resolve_repo_name(repo_name, profile), + color=color, + description=description, + owner=owner, + test=__opts__["test"], + ) +@catch_errors def label_absent(name, repo_name=None, owner=None, profile="github"): """ Ensure a label is absent from a repository. @@ -601,18 +570,16 @@ def label_absent(name, repo_name=None, owner=None, profile="github"): profile Profile configuration name to use. Defaults to ``github``. """ - try: - return sc_labels.absent( - _get_conn(profile), - name, - _resolve_repo_name(repo_name, profile), - owner=owner, - test=__opts__["test"], - ) - except CommandExecutionError as err: - return _failed(name, err) + return sc_labels.absent( + _get_conn(profile), + name, + _resolve_repo_name(repo_name, profile), + owner=owner, + test=__opts__["test"], + ) +@catch_errors def pages_present( name, build_type=None, @@ -657,23 +624,21 @@ def pages_present( profile Profile configuration name to use. Defaults to ``github``. """ - try: - return sc_pages.present( - _get_conn(profile), - name, - repo_name or name, - build_type=build_type, - source_branch=source_branch, - source_path=source_path, - cname=cname, - https_enforced=https_enforced, - owner=owner, - test=__opts__["test"], - ) - except CommandExecutionError as err: - return _failed(name, err) + return sc_pages.present( + _get_conn(profile), + name, + repo_name or name, + build_type=build_type, + source_branch=source_branch, + source_path=source_path, + cname=cname, + https_enforced=https_enforced, + owner=owner, + test=__opts__["test"], + ) +@catch_errors def pages_absent(name, repo_name=None, owner=None, profile="github"): """ Ensure GitHub Pages are disabled for a repository. @@ -690,13 +655,10 @@ def pages_absent(name, repo_name=None, owner=None, profile="github"): profile Profile configuration name to use. Defaults to ``github``. """ - try: - return sc_pages.absent( - _get_conn(profile), - name, - repo_name or name, - owner=owner, - test=__opts__["test"], - ) - except CommandExecutionError as err: - return _failed(name, err) + return sc_pages.absent( + _get_conn(profile), + name, + repo_name or name, + owner=owner, + test=__opts__["test"], + ) diff --git a/src/saltext/github/utils/gh/actions.py b/src/saltext/github/utils/gh/actions.py index a1ee6d91..983ae9bc 100644 --- a/src/saltext/github/utils/gh/actions.py +++ b/src/saltext/github/utils/gh/actions.py @@ -9,15 +9,10 @@ from nacl import public from saltext.github.utils import filter_unset -from saltext.github.utils.gh import client log = logging.getLogger(__name__) -def _secrets_path(conn, owner, repo_name, *suffix): - return ["repos", owner or conn.owner, repo_name, "actions", "secrets", *suffix] - - def list_secrets(conn, repo_name, *, owner=None): """ Return all Actions secrets of a repository. Secret values are write-only, @@ -32,7 +27,7 @@ def list_secrets(conn, repo_name, *, owner=None): owner Owner of the repository. Defaults to the connection's owner. """ - return client.query(conn, _secrets_path(conn, owner, repo_name), per_page=100)["secrets"] + return conn.repo_query(owner, repo_name, "actions", "secrets", per_page=100)["secrets"] def get_secret(conn, repo_name, name, *, owner=None): @@ -54,7 +49,7 @@ def get_secret(conn, repo_name, name, *, owner=None): Raises :class:`~saltext.github.utils.gh.exceptions.GitHubNotFoundError` if the secret does not exist. """ - return client.query(conn, _secrets_path(conn, owner, repo_name, name)) + return conn.repo_query(owner, repo_name, "actions", "secrets", name) def get_public_key(conn, repo_name, *, owner=None): @@ -71,7 +66,7 @@ def get_public_key(conn, repo_name, *, owner=None): owner Owner of the repository. Defaults to the connection's owner. """ - return client.query(conn, _secrets_path(conn, owner, repo_name, "public-key")) + return conn.repo_query(owner, repo_name, "actions", "secrets", "public-key") def encrypt_secret(public_key, value): @@ -111,9 +106,9 @@ def set_secret(conn, repo_name, name, value, *, owner=None): Owner of the repository. Defaults to the connection's owner. """ pubkey = get_public_key(conn, repo_name, owner=owner) - client.query( - conn, - _secrets_path(conn, owner, repo_name, name), + conn.repo_query( + owner, + *(repo_name, "actions", "secrets", name), method="PUT", data={ "encrypted_value": encrypt_secret(pubkey["key"], value), @@ -138,11 +133,7 @@ def delete_secret(conn, repo_name, name, *, owner=None): owner Owner of the repository. Defaults to the connection's owner. """ - client.query(conn, _secrets_path(conn, owner, repo_name, name), method="DELETE") - - -def _workflow_permissions_path(conn, owner, repo_name): - return ["repos", owner or conn.owner, repo_name, "actions", "permissions", "workflow"] + conn.repo_query(owner, repo_name, "actions", "secrets", name, method="DELETE") def get_workflow_permissions(conn, repo_name, *, owner=None): @@ -160,7 +151,7 @@ def get_workflow_permissions(conn, repo_name, *, owner=None): owner Owner of the repository. Defaults to the connection's owner. """ - return client.query(conn, _workflow_permissions_path(conn, owner, repo_name)) + return conn.repo_query(owner, repo_name, "actions", "permissions", "workflow") def set_workflow_permissions( @@ -191,9 +182,9 @@ def set_workflow_permissions( owner Owner of the repository. Defaults to the connection's owner. """ - client.query( - conn, - _workflow_permissions_path(conn, owner, repo_name), + conn.repo_query( + owner, + *(repo_name, "actions", "permissions", "workflow"), method="PUT", data=filter_unset( { diff --git a/src/saltext/github/utils/gh/client.py b/src/saltext/github/utils/gh/client.py index 3d8d3419..10a62c36 100644 --- a/src/saltext/github/utils/gh/client.py +++ b/src/saltext/github/utils/gh/client.py @@ -29,6 +29,27 @@ API_URL_DEFAULT = "https://api.github.com" +class ModWrapper: + __slots__ = ("conn", "mod") + + def __init__(self, conn, mod): + self.conn = conn + self.mod = mod + + def __getattr__(self, attr): + func = getattr(self.mod, attr) + if not callable(func): + return func + + @functools.wraps(func) + def _owner_default(*args, owner=..., **kwargs): + if owner is ...: + return func(self.conn, *args, **kwargs) + return func(self.conn, *args, owner=owner or self.conn.owner, **kwargs) + + return _owner_default + + @dataclass(slots=True) class GitHubConnection: """ @@ -120,6 +141,104 @@ def owner_type(self, owner=None): self.cache[key] = self.client.get_user(owner).type return self.cache[key] + def bind(self, mod): + """ + Wrap a module to pass the connection as the first positional argument + and auto-default the ``owner`` param to the one defined on the client + when any function is called. + """ + return ModWrapper(self, mod) + + def repo_query(self, owner, *path, **kwargs): + """ + Make a direct call to the ``/repos`` GitHub API, defaulting the first following + path segment to the configured ``owner`` if unspecified. + Otherwise delegates to ``query``. + + owner + Pass an explicit owner. Defaults to the owner configured for the connection. + + Pass an arbitrary number of path segments as string-valued positional args. + """ + return self.query(["repos", owner or self.owner] + list(path), **kwargs) + + def query(self, path, *, params=None, method="GET", headers=None, data=None, per_page=None): + """ + Make a direct call to the GitHub API, transparently following pagination. + + path + URL path to query, either as a string or a list of segments. + + params + Mapping of query parameters. + + method + HTTP method to use. Defaults to ``GET``. + + headers + Mapping of extra headers to send. + + data + Request body. JSON strings are decoded first. + + per_page + Pagination size, maximum 100. + + Returns the decoded response data. List results are merged across pages. + Raises :class:`GitHubNotFoundError` for missing resources and + :class:`GitHubApiError` for other error responses. + """ + args = dict(params) if isinstance(params, dict) else {} + segments = [path] if isinstance(path, str) else list(path or []) + url = "/" + "/".join(str(segment) for segment in segments) + + log.debug("GitHub API path: %s", url) + + if per_page and "per_page" not in args: + args["per_page"] = per_page + + if isinstance(data, str): + data = json.loads(data) + + requester = self.client.requester + + complete_result = [] + next_page = None + while True: + if next_page: + args["page"] = next_page + try: + status, resp_headers, text = requester.requestJson( + method, url, parameters=args or None, headers=headers, input=data + ) + except github.GithubException as err: + exc_type = GitHubNotFoundError if err.status == 404 else GitHubApiError + raise exc_type( + _error_message(err.status, err.data), status=err.status, data=err.data + ) from err + except Exception as err: # pylint: disable=broad-except + raise CommandExecutionError(f"GitHub API request failed: {err}") from err + log.debug("GitHub Response Status Code: %s", status) + decoded = json.loads(text) if text else None + + if not 200 <= status < 300: + exc_type = GitHubNotFoundError if status == 404 else GitHubApiError + raise exc_type(_error_message(status, decoded, text), status=status, data=decoded) + + if not isinstance(decoded, list): + # If only querying for one item, such as a single issue, + # the GitHub API returns a single dictionary, instead of + # a list of dictionaries. In that case, we can return. + return decoded + + complete_result += decoded + + next_page = _next_page_number(resp_headers) + if next_page is None: + break + + return complete_result + def _error_message(status, data, fallback=""): message = data.get("message", fallback) if isinstance(data, dict) else (data or fallback) @@ -149,88 +268,6 @@ def wrapper(*args, **kwargs): return wrapper -def query(conn, path, *, params=None, method="GET", headers=None, data=None, per_page=None): - """ - Make a direct call to the GitHub API, transparently following pagination. - - conn - :class:`GitHubConnection` to run the request through (honoring - its ``api_url``, authentication and retry handling). - - path - URL path to query, either as a string or a list of segments. - - params - Mapping of query parameters. - - method - HTTP method to use. Defaults to ``GET``. - - headers - Mapping of extra headers to send. - - data - Request body. JSON strings are decoded first. - - per_page - Pagination size, maximum 100. - - Returns the decoded response data. List results are merged across pages. - Raises :class:`GitHubNotFoundError` for missing resources and - :class:`GitHubApiError` for other error responses. - """ - args = dict(params) if isinstance(params, dict) else {} - segments = [path] if isinstance(path, str) else list(path or []) - url = "/" + "/".join(str(segment) for segment in segments) - - log.debug("GitHub API path: %s", url) - - if per_page and "per_page" not in args: - args["per_page"] = per_page - - if isinstance(data, str): - data = json.loads(data) - - requester = conn.client.requester - - complete_result = [] - next_page = None - while True: - if next_page: - args["page"] = next_page - try: - status, resp_headers, text = requester.requestJson( - method, url, parameters=args or None, headers=headers, input=data - ) - except github.GithubException as err: - exc_type = GitHubNotFoundError if err.status == 404 else GitHubApiError - raise exc_type( - _error_message(err.status, err.data), status=err.status, data=err.data - ) from err - except Exception as err: # pylint: disable=broad-except - raise CommandExecutionError(f"GitHub API request failed: {err}") from err - log.debug("GitHub Response Status Code: %s", status) - decoded = json.loads(text) if text else None - - if not 200 <= status < 300: - exc_type = GitHubNotFoundError if status == 404 else GitHubApiError - raise exc_type(_error_message(status, decoded, text), status=status, data=decoded) - - if not isinstance(decoded, list): - # If only querying for one item, such as a single issue, - # the GitHub API returns a single dictionary, instead of - # a list of dictionaries. In that case, we can return. - return decoded - - complete_result += decoded - - next_page = _next_page_number(resp_headers) - if next_page is None: - break - - return complete_result - - def _next_page_number(headers): """ Extract the next page number from a GitHub ``Link`` response header. diff --git a/src/saltext/github/utils/gh/deploy_keys.py b/src/saltext/github/utils/gh/deploy_keys.py index 173f81bd..6a059a61 100644 --- a/src/saltext/github/utils/gh/deploy_keys.py +++ b/src/saltext/github/utils/gh/deploy_keys.py @@ -4,15 +4,9 @@ import logging -from saltext.github.utils.gh import client - log = logging.getLogger(__name__) -def _path(conn, owner, repo_name, *suffix): - return ["repos", owner or conn.owner, repo_name, "keys", *suffix] - - def key_body(key): """ Reduce an SSH public key to its comparable part (type + base64 body), @@ -37,7 +31,7 @@ def list_all(conn, repo_name, *, owner=None): owner Owner of the repository. Defaults to the connection's owner. """ - return client.query(conn, _path(conn, owner, repo_name)) + return conn.repo_query(owner, repo_name, "keys") def get(conn, repo_name, key_id, *, owner=None): @@ -59,7 +53,7 @@ def get(conn, repo_name, key_id, *, owner=None): Raises :class:`~saltext.github.utils.gh.exceptions.GitHubNotFoundError` if the deploy key does not exist. """ - return client.query(conn, _path(conn, owner, repo_name, key_id)) + return conn.repo_query(owner, repo_name, "keys", key_id) def create(conn, repo_name, title, key, *, read_only=False, owner=None): @@ -86,9 +80,10 @@ def create(conn, repo_name, title, key, *, read_only=False, owner=None): owner Owner of the repository. Defaults to the connection's owner. """ - return client.query( - conn, - _path(conn, owner, repo_name), + return conn.repo_query( + owner, + repo_name, + "keys", method="POST", data={"title": title, "key": key, "read_only": read_only}, ) @@ -110,4 +105,4 @@ def delete(conn, repo_name, key_id, *, owner=None): owner Owner of the repository. Defaults to the connection's owner. """ - client.query(conn, _path(conn, owner, repo_name, key_id), method="DELETE") + conn.repo_query(owner, repo_name, "keys", key_id, method="DELETE") diff --git a/src/saltext/github/utils/gh/environments.py b/src/saltext/github/utils/gh/environments.py index 2718e80f..6c015039 100644 --- a/src/saltext/github/utils/gh/environments.py +++ b/src/saltext/github/utils/gh/environments.py @@ -8,16 +8,13 @@ from salt.exceptions import CommandExecutionError from saltext.github.utils import filter_unset -from saltext.github.utils.gh import client log = logging.getLogger(__name__) -def _path(conn, owner, repo_name, *suffix): +def _path(*suffix): # Environment names can contain characters that are invalid in URL paths - return ["repos", owner or conn.owner, repo_name, "environments"] + [ - quote(str(segment), safe="") for segment in suffix - ] + return ["environments"] + [quote(str(segment), safe="") for segment in suffix] def branch_policy_from_patterns(deployment_branches): @@ -84,7 +81,7 @@ def list_all(conn, repo_name, *, owner=None): owner Owner of the repository. Defaults to the connection's owner. """ - return client.query(conn, _path(conn, owner, repo_name), per_page=100)["environments"] + return conn.repo_query(owner, repo_name, "environments", per_page=100)["environments"] def get(conn, repo_name, name, *, owner=None): @@ -106,7 +103,7 @@ def get(conn, repo_name, name, *, owner=None): Raises :class:`~saltext.github.utils.gh.exceptions.GitHubNotFoundError` if the environment does not exist. """ - return client.query(conn, _path(conn, owner, repo_name, name)) + return conn.repo_query(owner, repo_name, *_path(name)) def set_( @@ -164,7 +161,7 @@ def set_( } ) data["deployment_branch_policy"] = deployment_branch_policy - return client.query(conn, _path(conn, owner, repo_name, name), method="PUT", data=data) + return conn.repo_query(owner, repo_name, *_path(name), method="PUT", data=data) def delete(conn, repo_name, name, *, owner=None): @@ -183,7 +180,7 @@ def delete(conn, repo_name, name, *, owner=None): owner Owner of the repository. Defaults to the connection's owner. """ - client.query(conn, _path(conn, owner, repo_name, name), method="DELETE") + conn.repo_query(owner, repo_name, *_path(name), method="DELETE") def list_branch_policies(conn, repo_name, environment, *, owner=None): @@ -202,9 +199,10 @@ def list_branch_policies(conn, repo_name, environment, *, owner=None): owner Owner of the repository. Defaults to the connection's owner. """ - return client.query( - conn, - _path(conn, owner, repo_name, environment, "deployment-branch-policies"), + return conn.repo_query( + owner, + repo_name, + *_path(environment, "deployment-branch-policies"), per_page=100, )["branch_policies"] @@ -237,9 +235,10 @@ def create_branch_policy(conn, repo_name, environment, name, *, typ="branch", ow owner Owner of the repository. Defaults to the connection's owner. """ - return client.query( - conn, - _path(conn, owner, repo_name, environment, "deployment-branch-policies"), + return conn.repo_query( + owner, + repo_name, + *_path(environment, "deployment-branch-policies"), method="POST", data={"name": name, "type": typ}, ) @@ -264,8 +263,9 @@ def delete_branch_policy(conn, repo_name, environment, policy_id, *, owner=None) owner Owner of the repository. Defaults to the connection's owner. """ - client.query( - conn, - _path(conn, owner, repo_name, environment, "deployment-branch-policies", policy_id), + conn.repo_query( + owner, + repo_name, + *_path(environment, "deployment-branch-policies", policy_id), method="DELETE", ) diff --git a/src/saltext/github/utils/gh/issues.py b/src/saltext/github/utils/gh/issues.py index e4785c81..9b0508c3 100644 --- a/src/saltext/github/utils/gh/issues.py +++ b/src/saltext/github/utils/gh/issues.py @@ -4,8 +4,6 @@ import logging -from saltext.github.utils.gh import client - log = logging.getLogger(__name__) @@ -25,7 +23,7 @@ def get(conn, repo_name, number, *, owner=None): owner Owner of the repository. Defaults to the connection's owner. """ - return client.query(conn, ["repos", owner or conn.owner, repo_name, "issues", str(number)]) + return conn.repo_query(owner, repo_name, "issues", str(number)) def get_comments(conn, repo_name, number, since=None, *, owner=None): @@ -51,9 +49,9 @@ def get_comments(conn, repo_name, number, since=None, *, owner=None): params = {} if since: params["since"] = since - return client.query( - conn, - ["repos", owner or conn.owner, repo_name, "issues", str(number), "comments"], + return conn.repo_query( + owner, + *(repo_name, "issues", str(number), "comments"), params=params, ) @@ -149,7 +147,7 @@ def list_all( if direction and direction != "desc": params["direction"] = direction - issues = client.query(conn, ["repos", owner or conn.owner, repo_name, "issues"], params=params) + issues = conn.repo_query(owner, repo_name, "issues", params=params) # Pull requests are included in the issue list from GitHub. # Let's not include those in the return. diff --git a/src/saltext/github/utils/gh/labels.py b/src/saltext/github/utils/gh/labels.py index 6ec3137e..6735c4e2 100644 --- a/src/saltext/github/utils/gh/labels.py +++ b/src/saltext/github/utils/gh/labels.py @@ -6,16 +6,13 @@ from urllib.parse import quote from saltext.github.utils import filter_unset -from saltext.github.utils.gh import client log = logging.getLogger(__name__) -def _path(conn, owner, repo_name, *suffix): +def _path(*suffix): # Label names can contain characters that are invalid in URL paths - return ["repos", owner or conn.owner, repo_name, "labels"] + [ - quote(str(segment), safe="") for segment in suffix - ] + return ["labels"] + [quote(str(segment), safe="") for segment in suffix] def normalize_color(color): @@ -43,7 +40,7 @@ def list_all(conn, repo_name, *, owner=None): owner Owner of the repository. Defaults to the connection's owner. """ - return client.query(conn, _path(conn, owner, repo_name)) + return conn.repo_query(owner, repo_name, "labels") def get(conn, repo_name, name, *, owner=None): @@ -65,7 +62,7 @@ def get(conn, repo_name, name, *, owner=None): Raises :class:`~saltext.github.utils.gh.exceptions.GitHubNotFoundError` if the label does not exist. """ - return client.query(conn, _path(conn, owner, repo_name, name)) + return conn.repo_query(owner, repo_name, *_path(name)) def create(conn, repo_name, name, *, color=None, description=None, owner=None): @@ -92,7 +89,7 @@ def create(conn, repo_name, name, *, color=None, description=None, owner=None): Owner of the repository. Defaults to the connection's owner. """ data = filter_unset({"name": name, "color": normalize_color(color), "description": description}) - return client.query(conn, _path(conn, owner, repo_name), method="POST", data=data) + return conn.repo_query(owner, repo_name, "labels", method="POST", data=data) def update(conn, repo_name, name, *, new_name=None, color=None, description=None, owner=None): @@ -123,7 +120,7 @@ def update(conn, repo_name, name, *, new_name=None, color=None, description=None data = filter_unset( {"new_name": new_name, "color": normalize_color(color), "description": description} ) - return client.query(conn, _path(conn, owner, repo_name, name), method="PATCH", data=data) + return conn.repo_query(owner, repo_name, *_path(name), method="PATCH", data=data) def delete(conn, repo_name, name, *, owner=None): @@ -142,4 +139,4 @@ def delete(conn, repo_name, name, *, owner=None): owner Owner of the repository. Defaults to the connection's owner. """ - client.query(conn, _path(conn, owner, repo_name, name), method="DELETE") + conn.repo_query(owner, repo_name, *_path(name), method="DELETE") diff --git a/src/saltext/github/utils/gh/milestones.py b/src/saltext/github/utils/gh/milestones.py index 52ca12d6..7ca90b17 100644 --- a/src/saltext/github/utils/gh/milestones.py +++ b/src/saltext/github/utils/gh/milestones.py @@ -4,7 +4,7 @@ import logging -from saltext.github.utils.gh import client +from saltext.github.utils.gh.exceptions import GitHubNotFoundError log = logging.getLogger(__name__) @@ -25,7 +25,7 @@ def get(conn, repo_name, number, *, owner=None): owner Owner of the repository. Defaults to the connection's owner. """ - return client.query(conn, ["repos", owner or conn.owner, repo_name, "milestones", str(number)]) + return conn.repo_query(owner, repo_name, "milestones", str(number)) def list_all( @@ -78,9 +78,34 @@ def list_all( if direction and direction != "asc": params["direction"] = direction - return client.query( - conn, ["repos", owner or conn.owner, repo_name, "milestones"], params=params - ) + return conn.repo_query(owner, repo_name, "milestones", params=params) + + +def find_by_title(conn, repo_name, title, *, owner=None): + """ + Return a milestone by its title. Since the GitHub API only addresses + milestones by number, this searches the complete list. + + conn + :class:`~saltext.github.utils.gh.client.GitHubConnection` to use. + + repo_name + Name of the repository the milestone is part of. + + title + Title of the milestone. + + owner + Owner of the repository. Defaults to the connection's owner. + + Raises :class:`~saltext.github.utils.gh.exceptions.GitHubNotFoundError` + if no milestone carries the title. + """ + milestones = list_all(conn, repo_name, state="all", owner=owner) + for milestone in milestones or []: + if milestone.get("title") == title: + return milestone + raise GitHubNotFoundError(f"Milestone {title} was not found in repository {repo_name}.") def format_milestone(milestone): diff --git a/src/saltext/github/utils/gh/pages.py b/src/saltext/github/utils/gh/pages.py index 0554bbbd..86ef3314 100644 --- a/src/saltext/github/utils/gh/pages.py +++ b/src/saltext/github/utils/gh/pages.py @@ -5,15 +5,10 @@ import logging from saltext.github.utils import filter_unset -from saltext.github.utils.gh import client log = logging.getLogger(__name__) -def _path(conn, owner, repo_name): - return ["repos", owner or conn.owner, repo_name, "pages"] - - def build_source(branch=None, path=None): """ Build a Pages ``source`` payload from branch and path, or ``None`` @@ -45,7 +40,7 @@ def get(conn, repo_name, *, owner=None): Raises :class:`~saltext.github.utils.gh.exceptions.GitHubNotFoundError` if the repository has no Pages site. """ - return client.query(conn, _path(conn, owner, repo_name)) + return conn.repo_query(owner, repo_name, "pages") def create(conn, repo_name, *, build_type=None, source=None, owner=None): @@ -70,9 +65,10 @@ def create(conn, repo_name, *, build_type=None, source=None, owner=None): owner Owner of the repository. Defaults to the connection's owner. """ - return client.query( - conn, - _path(conn, owner, repo_name), + return conn.repo_query( + owner, + repo_name, + "pages", method="POST", data=filter_unset({"build_type": build_type, "source": source}), ) @@ -113,7 +109,7 @@ def update( ) if cname is not ...: data["cname"] = cname - client.query(conn, _path(conn, owner, repo_name), method="PUT", data=data) + conn.repo_query(owner, repo_name, "pages", method="PUT", data=data) def delete(conn, repo_name, *, owner=None): @@ -129,4 +125,4 @@ def delete(conn, repo_name, *, owner=None): owner Owner of the repository. Defaults to the connection's owner. """ - client.query(conn, _path(conn, owner, repo_name), method="DELETE") + conn.repo_query(owner, repo_name, "pages", method="DELETE") diff --git a/src/saltext/github/utils/gh/pulls.py b/src/saltext/github/utils/gh/pulls.py index 35997e76..44378b86 100644 --- a/src/saltext/github/utils/gh/pulls.py +++ b/src/saltext/github/utils/gh/pulls.py @@ -4,8 +4,6 @@ import logging -from saltext.github.utils.gh import client - log = logging.getLogger(__name__) @@ -75,7 +73,7 @@ def list_all( if direction and direction != "desc": params["direction"] = direction - return client.query(conn, ["repos", owner or conn.owner, repo_name, "pulls"], params=params) + return conn.repo_query(owner, repo_name, "pulls", params=params) def format_pr(pr_): diff --git a/src/saltext/github/utils/gh/resource.py b/src/saltext/github/utils/gh/resource.py new file mode 100644 index 00000000..d69b479c --- /dev/null +++ b/src/saltext/github/utils/gh/resource.py @@ -0,0 +1,35 @@ +""" +Shared plumbing for the :py:mod:`github ` +resource type's execution and state modules. + +The functions here reference loader globals (``__resource__``, +``__salt__``, ``__opts__``, ``__context__``), so consuming modules must +rebind them to their own namespace via +:func:`salt.utils.functools.namespaced_function`:: + + from salt.utils.functools import namespaced_function + + from saltext.github.utils.gh import resource as gh_resource + + _core_args = namespaced_function(gh_resource.core_args, globals()) +""" + +from saltext.github.utils.gh import profile as gh_profile + + +def core_args(profile=None): + """ + Return a connection for the targeted repository, along with the + repository's owner and name. + + Salt's authoring docs promise ``__resource__`` inside state + overrides, but no released Salt packs it there — in state modules, + fall back to the ``repo.ident`` execution override, which does see it. + """ + try: + rid = __resource__["id"] + except NameError: + rid = __salt__["repo.ident"]()["id"] + owner, _, repo_name = rid.partition("/") + conn = gh_profile.resource_conn(__opts__, __context__, profile=profile, owner=owner) + return conn, owner, repo_name diff --git a/src/saltext/github/utils/gh/rulesets.py b/src/saltext/github/utils/gh/rulesets.py index da440b12..58079361 100644 --- a/src/saltext/github/utils/gh/rulesets.py +++ b/src/saltext/github/utils/gh/rulesets.py @@ -6,8 +6,6 @@ from salt.exceptions import CommandExecutionError -from saltext.github.utils.gh import client - log = logging.getLogger(__name__) @@ -80,7 +78,7 @@ def list_all( repo_name=repo_name, org_name=org_name or conn.owner, ) - return client.query(conn, path, headers=headers, per_page=per_page) + return conn.query(path, headers=headers, per_page=per_page) def get(conn, ruleset_id, ruleset_type, *, owner=None, repo_name=None, org_name=None, headers=None): @@ -103,7 +101,7 @@ def get(conn, ruleset_id, ruleset_type, *, owner=None, repo_name=None, org_name= org_name=org_name or conn.owner, ruleset_id=ruleset_id, ) - return client.query(conn, path, headers=headers) + return conn.query(path, headers=headers) def create( @@ -128,7 +126,7 @@ def create( repo_name=repo_name, org_name=org_name or conn.owner, ) - return client.query(conn, path, method="POST", data=ruleset_params, headers=headers) + return conn.query(path, method="POST", data=ruleset_params, headers=headers) def update( @@ -165,7 +163,7 @@ def update( org_name=org_name or conn.owner, ruleset_id=ruleset_id, ) - return client.query(conn, path, method="PUT", data=ruleset_params, headers=headers) + return conn.query(path, method="PUT", data=ruleset_params, headers=headers) def delete( @@ -190,4 +188,4 @@ def delete( org_name=org_name or conn.owner, ruleset_id=ruleset_id, ) - client.query(conn, path, method="DELETE", headers=headers) + conn.query(path, method="DELETE", headers=headers) diff --git a/src/saltext/github/utils/gh/statecores/__init__.py b/src/saltext/github/utils/gh/statecores/__init__.py index 496c7eda..cfb9dc03 100644 --- a/src/saltext/github/utils/gh/statecores/__init__.py +++ b/src/saltext/github/utils/gh/statecores/__init__.py @@ -14,6 +14,52 @@ failures, which the wrappers translate into a failed state return. """ +import functools + +from salt.exceptions import CommandExecutionError + + +class StateError(CommandExecutionError): + """ + Raised when applying state changes failed. Compound states can + attach the changes that were applied before the failure, which + :func:`failed`/:func:`catch_errors` include in the state return. + """ + + def __init__(self, message, *, changes=None): + super().__init__(message) + self.changes = changes or {} + + +def failed(name, err): + """ + Render a raised error into a failed state return, including + partially applied changes carried by the error (:class:`StateError`). + """ + return { + "name": name, + "changes": getattr(err, "changes", None) or {}, + "result": False, + "comment": str(err), + } + + +def catch_errors(func): + """ + Wrap a state function to render raised + :class:`CommandExecutionError ` + into a failed state return instead of a traceback. + """ + + @functools.wraps(func) + def wrapper(name, *args, **kwargs): + try: + return func(name, *args, **kwargs) + except CommandExecutionError as err: + return failed(name, err) + + return wrapper + def differs(current, desired): """ diff --git a/src/saltext/github/utils/gh/statecores/environments.py b/src/saltext/github/utils/gh/statecores/environments.py index c0faa8e0..8098802e 100644 --- a/src/saltext/github/utils/gh/statecores/environments.py +++ b/src/saltext/github/utils/gh/statecores/environments.py @@ -4,9 +4,12 @@ import logging +from salt.exceptions import CommandExecutionError + from saltext.github.utils import filter_unset from saltext.github.utils.gh import environments as gh_environments from saltext.github.utils.gh.exceptions import GitHubNotFoundError +from saltext.github.utils.gh.statecores import StateError log = logging.getLogger(__name__) @@ -145,13 +148,28 @@ def present( ret["comment"] = f"Environment {name} would have been {verb}" return ret - if current is None or settings_diff: - gh_environments.set_(conn, repo_name, name, owner=owner, **set_kwargs) - for pattern in branches_added: - gh_environments.create_branch_policy(conn, repo_name, name, pattern, owner=owner) - for policy in current_policies: - if policy["name"] in branches_removed: - gh_environments.delete_branch_policy(conn, repo_name, name, policy["id"], owner=owner) + applied = {} + try: + if current is None or settings_diff: + gh_environments.set_(conn, repo_name, name, owner=owner, **set_kwargs) + if current is None: + applied["created"] = name + else: + applied["old"] = ret["changes"]["old"] + applied["new"] = ret["changes"]["new"] + for pattern in branches_added: + gh_environments.create_branch_policy(conn, repo_name, name, pattern, owner=owner) + applied.setdefault("deployment_branches", {}).setdefault("added", []).append(pattern) + for policy in current_policies: + if policy["name"] in branches_removed: + gh_environments.delete_branch_policy( + conn, repo_name, name, policy["id"], owner=owner + ) + applied.setdefault("deployment_branches", {}).setdefault("removed", []).append( + policy["name"] + ) + except CommandExecutionError as err: + raise StateError(str(err), changes=applied) from err ret["comment"] = f"Environment {name} has been {verb}" return ret diff --git a/src/saltext/github/utils/gh/statecores/repo.py b/src/saltext/github/utils/gh/statecores/repo.py index 55a7fef7..6efaba30 100644 --- a/src/saltext/github/utils/gh/statecores/repo.py +++ b/src/saltext/github/utils/gh/statecores/repo.py @@ -9,6 +9,7 @@ from saltext.github.utils.gh import repos as gh_repos from saltext.github.utils.gh import teams as gh_teams from saltext.github.utils.gh.exceptions import GitHubNotFoundError +from saltext.github.utils.gh.statecores import StateError from saltext.github.utils.gh.statecores import differs log = logging.getLogger(__name__) @@ -57,24 +58,11 @@ def _repo_teams_diff(ret, conn, name, teams, owner): def _set_team_repo(conn, name, team_name, permission, owner): """ - Add a repository to a team or update the team's permission on it, - raising when setting the permission failed. + Add a repository to a team or update the team's permission on it. """ - if not gh_teams.add_repo( + gh_teams.add_repo( conn, gh_teams.find(conn, team_name, owner=owner), name, permission=permission, owner=owner - ): - raise CommandExecutionError( - f"Failed setting permission {permission} on repo {name} for team {team_name}." - ) - - -def _create_repo(conn, name, create_params, teams, owner): - """ - Create a repository and associate the requested teams. - """ - gh_repos.create(conn, name, owner=owner, **create_params) - for team_name, permission in (teams or {}).items(): - _set_team_repo(conn, name, team_name, permission, owner) + ) def present( @@ -177,41 +165,51 @@ def present( ret["comment"] = f"Repo {name} would have been {verb}" return ret - if target is None: - _create_repo( - conn, - name, - { - "description": description, - "homepage": homepage, - "private": private, - "has_issues": has_issues, - "has_wiki": has_wiki, - "has_downloads": has_downloads, - "auto_init": auto_init, - "gitignore_template": gitignore_template, - "license_template": license_template, - }, - teams, - owner, - ) - else: - if update_params: - gh_repos.update( + applied = {} + try: + if target is None: + gh_repos.create( conn, name, owner=owner, - allow_privacy_change=allow_privacy_change, - **update_params, - ) - for team_name in teams_added: - _set_team_repo(conn, name, team_name, teams[team_name], owner) - for team_name in teams_removed: - gh_teams.remove_repo( - conn, gh_teams.find(conn, team_name, owner=owner), name, owner=owner + 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, ) - for team_name in team_permissions: - _set_team_repo(conn, name, team_name, teams[team_name], owner) + applied["created"] = name + for team_name in sorted(teams or {}): + _set_team_repo(conn, name, team_name, teams[team_name], owner) + applied.setdefault("teams_added", []).append(team_name) + else: + if update_params: + gh_repos.update( + conn, + name, + owner=owner, + allow_privacy_change=allow_privacy_change, + **update_params, + ) + applied["old"] = ret["changes"]["old"] + applied["new"] = ret["changes"]["new"] + for team_name in teams_added: + _set_team_repo(conn, name, team_name, teams[team_name], owner) + applied.setdefault("teams_added", []).append(team_name) + for team_name in teams_removed: + gh_teams.remove_repo( + conn, gh_teams.find(conn, team_name, owner=owner), name, owner=owner + ) + applied.setdefault("teams_removed", []).append(team_name) + for team_name, permission in team_permissions.items(): + _set_team_repo(conn, name, team_name, teams[team_name], owner) + applied.setdefault("team_permissions", {})[team_name] = permission + except CommandExecutionError as err: + raise StateError(str(err), changes=applied) from err ret["comment"] = f"Repo {name} has been {verb}" return ret diff --git a/src/saltext/github/utils/gh/teams.py b/src/saltext/github/utils/gh/teams.py index 150fd278..dff05ac0 100644 --- a/src/saltext/github/utils/gh/teams.py +++ b/src/saltext/github/utils/gh/teams.py @@ -10,6 +10,7 @@ from saltext.github.utils import filter_unset from saltext.github.utils.gh.client import api_request +from saltext.github.utils.gh.exceptions import GitHubApiError from saltext.github.utils.gh.exceptions import GitHubNotFoundError log = logging.getLogger(__name__) @@ -242,7 +243,7 @@ def list_repos(conn, team_info, *, owner=None, ignore_cache=False): def add_repo(conn, team_info, repo_name, permission=None, *, owner=None): """ Add a repository to a team, or update the permission the team has - on it. Returns false if updating the permission failed. + on it. Raises when updating the permission failed. conn :class:`~saltext.github.utils.gh.client.GitHubConnection` to use. @@ -266,9 +267,11 @@ def add_repo(conn, team_info, repo_name, permission=None, *, owner=None): if permission is None: team.add_to_repos(repo) elif not team.update_team_repository(repo, permission): - return False + raise GitHubApiError( + f"Failed setting permission {permission} on repo {repo_name} " + f"for team {team_info['name']}." + ) list_repos(conn, team_info, owner=owner, ignore_cache=True) # Refresh cache - return True @api_request diff --git a/tests/functional/conftest.py b/tests/functional/conftest.py index 5cd947cb..d32e67ae 100644 --- a/tests/functional/conftest.py +++ b/tests/functional/conftest.py @@ -2,6 +2,7 @@ import shutil import pytest +from saltfactories.utils import random_string from saltfactories.utils.functional import Loaders from tests.support.github import DEPLOY_KEY_A @@ -12,7 +13,8 @@ @pytest.fixture(scope="package") def minion_id(): # pragma: no cover - return "func-tests-minion-opts" + # Random to avoid factory directory collisions, e.g. with pytest-xdist + return random_string("func-tests-minion-") @pytest.fixture(scope="module") @@ -126,7 +128,8 @@ def master_opts( } ) factory = salt_factories.salt_master_daemon( - "func-tests-master-opts", + # Random to avoid factory directory collisions, e.g. with pytest-xdist + random_string("func-tests-master-"), defaults=master_config_defaults or None, overrides=master_config_overrides, ) diff --git a/tests/functional/resources/modules/test_milestone.py b/tests/functional/resources/modules/test_milestone.py index dc218414..073c9a4d 100644 --- a/tests/functional/resources/modules/test_milestone.py +++ b/tests/functional/resources/modules/test_milestone.py @@ -1,3 +1,7 @@ +import pytest +from salt.exceptions import CommandExecutionError + + def test_list(res_modules, for_resource): with for_resource("testorg/repo-a"): milestones = res_modules["milestone.list"](fields="title") @@ -8,3 +12,27 @@ def test_get_by_name(res_modules, for_resource): with for_resource("testorg/repo-a"): milestone = res_modules["milestone.get"](name="v1.0") assert milestone["title"] == "v1.0" + + +def test_get_by_number(res_modules, for_resource): + with for_resource("testorg/repo-a"): + milestone = res_modules["milestone.get"](1) + assert milestone["title"] == "v1.0" + + +def test_get_missing_number(res_modules, for_resource): + with for_resource("testorg/repo-a"): + with pytest.raises(CommandExecutionError, match="Not Found"): + res_modules["milestone.get"](42) + + +def test_get_missing_name(res_modules, for_resource): + with for_resource("testorg/repo-a"): + with pytest.raises(CommandExecutionError, match="was not found"): + res_modules["milestone.get"](name="ghost") + + +def test_get_requires_identifier(res_modules, for_resource): + with for_resource("testorg/repo-a"): + with pytest.raises(CommandExecutionError, match="'name' or 'number'"): + res_modules["milestone.get"]() diff --git a/tests/functional/resources/states/test_label.py b/tests/functional/resources/states/test_label.py index 21ec3168..a8858da7 100644 --- a/tests/functional/resources/states/test_label.py +++ b/tests/functional/resources/states/test_label.py @@ -1,3 +1,5 @@ +import inspect + import pytest @@ -29,8 +31,10 @@ def test_state_reads_packed_resource(res_states, monkeypatch): ``repo.ident`` fallback. No resource contextvar is set here, so a successful run proves the direct path was taken. """ + # Unwrap the catch_errors decorator: packing happens on the module's + # globals, which the decorator's wrapper function does not share. monkeypatch.setitem( - res_states["label.present"].__globals__, + inspect.unwrap(res_states["label.present"]).__globals__, "__resource__", {"id": "testorg/repo-a", "type": "github"}, ) diff --git a/tests/unit/modules/test_github.py b/tests/unit/modules/test_github.py index 4fb28949..7bbe1dbe 100644 --- a/tests/unit/modules/test_github.py +++ b/tests/unit/modules/test_github.py @@ -3,25 +3,35 @@ import pytest from salt.exceptions import CommandExecutionError -from salt.modules import config as config_mod from saltext.github.modules import github as github_mod -from saltext.github.utils.gh import client as gh_client from saltext.github.utils.gh.exceptions import GitHubApiError +RULESET_PARAMS = {"name": "mock_name", "enforcement": "disabled"} + +# Function-specific call kwargs and the comment returned on unexpected errors +RULESET_FUNCS = { + "get_ruleset": ({"ruleset_id": 1}, "Error getting ruleset"), + "add_ruleset": ({"ruleset_params": RULESET_PARAMS}, "Error adding ruleset"), + "list_rulesets": ({}, "Error getting rulesets"), + "update_ruleset": ( + {"ruleset_id": 1, "ruleset_params": RULESET_PARAMS}, + "Error updating ruleset", + ), + "delete_ruleset": ({"ruleset_id": 1}, "Error deleting ruleset"), +} + @pytest.fixture def configure_loader_modules(): - module_globals = { - "__salt__": {"config.get": config_mod.get}, - "__opts__": { - "test_ruleset": {"ruleset_type": "mocked_value"}, - "sdb_profile": {"token": "sdb://osenv/GITHUB_TOKEN"}, - }, - } return { - github_mod: module_globals, - config_mod: module_globals, + github_mod: { + "__opts__": { + "github": {"token": "token_from_default_profile"}, + "test_ruleset": {"ruleset_type": "mocked_value"}, + "sdb_profile": {"token": "sdb://osenv/GITHUB_TOKEN"}, + }, + }, } @@ -36,20 +46,41 @@ def params(): } -@pytest.fixture(autouse=True) +@pytest.fixture def conn(): - with patch.object(github_mod, "_get_conn", return_value=MagicMock()): - yield + conn_mock = MagicMock() + with patch.object(github_mod, "_get_conn", autospec=True, return_value=conn_mock): + yield conn_mock + + +def _call_ruleset_func(func, params): + return getattr(github_mod, func)(profile="test_ruleset", **params, **RULESET_FUNCS[func][0]) + + +@pytest.mark.usefixtures("conn") +def test_add_team_repo_reports_permission_failure(): + with ( + patch.object(github_mod.gh_teams, "get", autospec=True, return_value={"slug": "team"}), + patch.object( + github_mod.gh_teams, + "add_repo", + autospec=True, + side_effect=GitHubApiError("Failed setting permission push"), + ), + patch.object(github_mod, "log", autospec=True) as log_mock, + ): + assert github_mod.add_team_repo("repo", "team", permission="push") is False + assert "Failed setting permission" in log_mock.error.call_args[0][0] -def test_get_issue_without_id_returns_empty(): - with patch.object(gh_client, "query", return_value={"message": "no id here"}): - assert not github_mod.get_issue(1, repo_name="repo") +def test_get_issue_without_id_returns_empty(conn): + conn.repo_query.return_value = {"message": "no id here"} + assert not github_mod.get_issue(1, repo_name="repo") -def test_get_milestone_without_id_returns_empty(): - with patch.object(gh_client, "query", return_value={"message": "no id here"}): - assert not github_mod.get_milestone(1, repo_name="repo") +def test_get_milestone_without_id_returns_empty(conn): + conn.repo_query.return_value = {"message": "no id here"} + assert not github_mod.get_milestone(1, repo_name="repo") def test__get_config_value_renders_sdb_uris(): @@ -60,7 +91,7 @@ def fake_sdb_get(value, *args, **kwargs): # pylint: disable=unused-argument return "rendered-token" return value - with patch.object(config_mod.sdb, "sdb_get", side_effect=fake_sdb_get): + with patch("salt.utils.sdb.sdb_get", side_effect=fake_sdb_get): assert github_mod._get_config_value("sdb_profile", "token") == "rendered-token" @@ -80,56 +111,45 @@ def test__resolve_ruleset_params(): } -def test_get_ruleset(params): - params["ruleset_id"] = 1 - - # test mock 200 status - with patch.object(gh_client, "query", return_value={"id": "1"}): - assert github_mod.get_ruleset(profile="test_ruleset", **params) == {"id": "1"} - - # test mock 404 error - expected = { +@pytest.mark.parametrize( + "func,query_ret,expected", + ( + ("get_ruleset", {"id": "1"}, {"id": "1"}), + ("add_ruleset", RULESET_PARAMS, RULESET_PARAMS), + ( + "list_rulesets", + [{"name": "mock_ruleset_1"}, {"name": "mock_ruleset_2"}], + {"rulesets": [{"name": "mock_ruleset_1"}, {"name": "mock_ruleset_2"}]}, + ), + ("list_rulesets", [], {"rulesets": None}), + ("update_ruleset", RULESET_PARAMS, RULESET_PARAMS), + ("delete_ruleset", None, {"comment": "ruleset 1 successfully deleted"}), + ), +) +def test_ruleset_funcs_success(func, query_ret, expected, params, conn): + conn.query.return_value = query_ret + assert _call_ruleset_func(func, params) == expected + + +@pytest.mark.parametrize("func", RULESET_FUNCS) +def test_ruleset_funcs_api_error(func, params, conn): + conn.query.side_effect = GitHubApiError("404 not found", status=404) + assert _call_ruleset_func(func, params) == { "comment": "GitHub Response Status Code: 404 not found", "error": "404 not found", "status": 404, "result": False, } - with patch.object(gh_client, "query", side_effect=GitHubApiError("404 not found", status=404)): - assert github_mod.get_ruleset(profile="test_ruleset", **params) == expected - - # test unexpected error - expected = {"comment": "Error getting ruleset", "result": False} - with patch.object(gh_client, "query", side_effect=CommandExecutionError): - assert github_mod.get_ruleset(profile="test_ruleset", **params) == expected - - -def test_add_ruleset(params): - params["ruleset_params"] = {"name": "mock_name", "enforcement": "disabled"} - # test mock 201 status - expected = {"name": "mock_name", "enforcement": "disabled"} - - with patch.object(gh_client, "query", return_value=expected): - assert github_mod.add_ruleset(profile="test_ruleset", **params) == expected - - # test mock 404 error - expected = { - "comment": "GitHub Response Status Code: 404 not found", - "error": "404 not found", - "status": 404, +@pytest.mark.parametrize("func", RULESET_FUNCS) +def test_ruleset_funcs_unexpected_error(func, params, conn): + conn.query.side_effect = CommandExecutionError + assert _call_ruleset_func(func, params) == { + "comment": RULESET_FUNCS[func][1], "result": False, } - with patch.object(gh_client, "query", side_effect=GitHubApiError("404 not found", status=404)): - assert github_mod.add_ruleset(profile="test_ruleset", **params) == expected - - # test unexpected error - expected = {"comment": "Error adding ruleset", "result": False} - - with patch.object(gh_client, "query", side_effect=CommandExecutionError): - assert github_mod.add_ruleset(profile="test_ruleset", **params) == expected - def test_add_ruleset_requires_name_and_enforcement(params): params["ruleset_params"] = {"name": "mock_name"} @@ -138,90 +158,6 @@ def test_add_ruleset_requires_name_and_enforcement(params): github_mod.add_ruleset(profile="test_ruleset", **params) -def test_list_rulesets(params): - - # test mock 200 status - with patch.object( - gh_client, "query", return_value=[{"name": "mock_ruleset_1"}, {"name": "mock_ruleset_2"}] - ): - assert github_mod.list_rulesets(profile="test_ruleset", **params) == { - "rulesets": [{"name": "mock_ruleset_1"}, {"name": "mock_ruleset_2"}] - } - - # test mock 200 status, no rulesets returned - with patch.object(gh_client, "query", return_value=[]): - assert github_mod.list_rulesets(profile="test_ruleset", **params) == {"rulesets": None} - - # test mock 404 error - expected = { - "comment": "GitHub Response Status Code: 404 not found", - "error": "404 not found", - "status": 404, - "result": False, - } - - with patch.object(gh_client, "query", side_effect=GitHubApiError("404 not found", status=404)): - assert github_mod.list_rulesets(profile="test_ruleset", **params) == expected - - # test unexpected error - expected = {"comment": "Error getting rulesets", "result": False} - with patch.object(gh_client, "query", side_effect=CommandExecutionError): - assert github_mod.list_rulesets(profile="test_ruleset", **params) == expected - - -def test_update_ruleset(params): - params["ruleset_params"] = {"name": "mock_name", "enforcement": "disabled"} - params["ruleset_id"] = 1 - - # test mock 200 status - expected = {"name": "mock_name", "enforcement": "disabled"} - - with patch.object(gh_client, "query", return_value=expected): - assert github_mod.update_ruleset(profile="test_ruleset", **params) == expected - - # test mock 404 error - expected = { - "comment": "GitHub Response Status Code: 404 not found", - "error": "404 not found", - "status": 404, - "result": False, - } - - with patch.object(gh_client, "query", side_effect=GitHubApiError("404 not found", status=404)): - assert github_mod.update_ruleset(profile="test_ruleset", **params) == expected - - # test unexpected error - expected = {"comment": "Error updating ruleset", "result": False} - with patch.object(gh_client, "query", side_effect=CommandExecutionError): - assert github_mod.update_ruleset(profile="test_ruleset", **params) == expected - - -def test_delete_ruleset(params): - params["ruleset_id"] = 1 - - # test mock 204 status - expected = {"comment": "ruleset 1 successfully deleted"} - - with patch.object(gh_client, "query", return_value=None): - assert github_mod.delete_ruleset(profile="test_ruleset", **params) == expected - - # test mock 404 error - expected = { - "comment": "GitHub Response Status Code: 404 not found", - "error": "404 not found", - "status": 404, - "result": False, - } - - with patch.object(gh_client, "query", side_effect=GitHubApiError("404 not found", status=404)): - assert github_mod.delete_ruleset(profile="test_ruleset", **params) == expected - - # test unexpected error - expected = {"comment": "Error deleting ruleset", "result": False} - with patch.object(gh_client, "query", side_effect=CommandExecutionError): - assert github_mod.delete_ruleset(profile="test_ruleset", **params) == expected - - def test_ruleset_error_ret_includes_response_data(): err = GitHubApiError("422 invalid", status=422, data={"message": "invalid"}) assert github_mod._ruleset_error_ret(err) == { diff --git a/tests/unit/modules/test_github_org.py b/tests/unit/modules/test_github_org.py new file mode 100644 index 00000000..eb6801b5 --- /dev/null +++ b/tests/unit/modules/test_github_org.py @@ -0,0 +1,42 @@ +from unittest.mock import MagicMock +from unittest.mock import patch + +import pytest +from salt.exceptions import CommandExecutionError + +from saltext.github.modules import github_org as github_org_mod +from saltext.github.utils.gh.client import ModWrapper +from saltext.github.utils.gh.exceptions import GitHubApiError + + +@pytest.fixture +def configure_loader_modules(): + return { + github_org_mod: { + "__opts__": {"github": {"token": "token_from_default_profile"}}, + }, + } + + +@pytest.fixture +def conn(): + conn_mock = MagicMock() + conn_mock.owner = "mock_org" + conn_mock.bind.side_effect = lambda mod: ModWrapper(conn_mock, mod) + with patch.object(github_org_mod, "_get_conn", autospec=True, return_value=conn_mock): + yield conn_mock + + +@pytest.mark.usefixtures("conn") +def test_add_team_repo_reports_permission_failure(): + with ( + patch.object(github_org_mod.gh_teams, "find", autospec=True, return_value={"slug": "team"}), + patch.object( + github_org_mod.gh_teams, + "add_repo", + autospec=True, + side_effect=GitHubApiError("Failed setting permission push"), + ), + ): + with pytest.raises(CommandExecutionError, match="Failed setting permission push"): + github_org_mod.add_team_repo("repo", "team", permission="push") diff --git a/tests/unit/states/test_error_handling.py b/tests/unit/states/test_error_handling.py new file mode 100644 index 00000000..506bb675 --- /dev/null +++ b/tests/unit/states/test_error_handling.py @@ -0,0 +1,138 @@ +""" +Ensure every public state function catches ``CommandExecutionError`` +raised by the layers below (execution modules, state cores, profile +resolution) and reports it as a failed state instead of a traceback. +""" + +import contextlib +import inspect +import re +from unittest import mock + +import pytest +from salt.exceptions import CommandExecutionError + +from saltext.github.resources.github.states import actions as res_actions +from saltext.github.resources.github.states import deploy_key as res_deploy_key +from saltext.github.resources.github.states import environment as res_environment +from saltext.github.resources.github.states import label as res_label +from saltext.github.resources.github.states import pages as res_pages +from saltext.github.resources.github.states import repo as res_repo +from saltext.github.resources.github.states import ruleset as res_ruleset +from saltext.github.states import github as github_mod +from saltext.github.states import github_org as github_org_mod +from saltext.github.states import github_repo as github_repo_mod +from saltext.github.utils.gh import resource as gh_resource + +ERROR_MESSAGE = "something went wrong below the state layer" + +MODULES = { + "github": github_mod, + "github_org": github_org_mod, + "github_repo": github_repo_mod, + "resources.actions": res_actions, + "resources.deploy_key": res_deploy_key, + "resources.environment": res_environment, + "resources.label": res_label, + "resources.pages": res_pages, + "resources.repo": res_repo, + "resources.ruleset": res_ruleset, +} + +# Values for required parameters that are validated before any API call +REQUIRED_ARG_VALUES = { + "ruleset_type": "repo", +} + +# github.repo_absent interprets a raising ``get_repo_info`` as a missing +# repository (the module layer raises for 404s), so the injected error +# maps to a fulfilled state instead of a failed one. +EXPECTED_OVERRIDES = { + ("github", "repo_absent"): { + "name": "state-name", + "changes": {}, + "result": True, + "comment": "Repo state-name does not exist", + }, +} + + +def _public_funcs(mod): + return sorted( + name + for name, func in inspect.getmembers(mod, inspect.isfunction) + if not name.startswith("_") and func.__module__ == mod.__name__ + ) + + +def _raising_salt_funcs(mod): + """ + All ``__salt__`` functions a module calls, each raising. Includes + calls made by the shared resource helpers, which are namespaced + into the resource modules. + """ + source = inspect.getsource(mod) + inspect.getsource(gh_resource) + return { + salt_func: mock.MagicMock(side_effect=CommandExecutionError(ERROR_MESSAGE)) + for salt_func in set(re.findall(r'__salt__\[\s*"([^"]+)"\s*\]', source)) + } + + +CASES = [ + pytest.param(mod_name, func_name, id=f"{mod_name}.{func_name}") + for mod_name, mod in MODULES.items() + for func_name in _public_funcs(mod) +] + + +@pytest.fixture +def configure_loader_modules(): + return { + mod: { + "__salt__": _raising_salt_funcs(mod), + "__opts__": {"test": False}, + "__context__": {}, + } + for mod in MODULES.values() + } + + +@pytest.fixture(autouse=True) +def raising_gh_profile(): + """ + Make connection/profile resolution fail for state modules that + call the utils directly instead of going through ``__salt__``. + """ + gh_profile_mock = mock.MagicMock() + gh_profile_mock.get_conn.side_effect = CommandExecutionError(ERROR_MESSAGE) + gh_profile_mock.resource_conn.side_effect = CommandExecutionError(ERROR_MESSAGE) + gh_profile_mock.get_config_value.side_effect = CommandExecutionError(ERROR_MESSAGE) + with contextlib.ExitStack() as stack: + for mod in MODULES.values(): + if hasattr(mod, "gh_profile"): + stack.enter_context(mock.patch.object(mod, "gh_profile", gh_profile_mock)) + yield + + +@pytest.mark.parametrize("mod_name,func_name", CASES) +def test_state_funcs_catch_command_execution_errors(mod_name, func_name): + func = getattr(MODULES[mod_name], func_name) + kwargs = { + param.name: REQUIRED_ARG_VALUES.get(param.name, "dummy") + for param in inspect.signature(func).parameters.values() + if param.name != "name" + and param.default is inspect.Parameter.empty + and param.kind not in (param.VAR_POSITIONAL, param.VAR_KEYWORD) + } + + ret = func("state-name", **kwargs) + + assert ret == EXPECTED_OVERRIDES.get( + (mod_name, func_name), + { + "name": "state-name", + "changes": {}, + "result": False, + "comment": ERROR_MESSAGE, + }, + ) diff --git a/tests/unit/states/test_github.py b/tests/unit/states/test_github.py index c1ac2d21..bdc88b98 100644 --- a/tests/unit/states/test_github.py +++ b/tests/unit/states/test_github.py @@ -4,9 +4,22 @@ from saltext.github.states import github as github_mod +RULESET_KWARGS = { + "name": "mock_ruleset", + "ruleset_type": "repo", + "profile": "test_ruleset", + "owner": "mock", + "repo_name": "mock_repo", +} + + +@pytest.fixture(params=(False, True)) +def testmode(request): + return request.param + @pytest.fixture -def configure_loader_modules(): +def configure_loader_modules(testmode): return { github_mod: { "__salt__": { @@ -16,267 +29,95 @@ def configure_loader_modules(): "github.update_ruleset": mock.MagicMock(return_value=None), "github.add_ruleset": mock.MagicMock(return_value=None), }, - "__opts__": {"test": False}, + "__opts__": {"test": testmode}, }, } -def test_repo_absent_no_changes(): - # test list rulesets are None - mock_query = mock.MagicMock(return_value={"rulesets": []}) - - expected = { - "name": "mock_repo_absent", - "changes": {}, - "result": True, - "comment": "Ruleset mock_repo_absent does not exist", - } - - 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", - owner="mock", - repo_name="mock_repo", - ) - assert ret == expected - - # 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_mod.__salt__, {"github.list_rulesets": mock_query}): - ret = github_mod.ruleset_absent( - name="mock_repo_absent", - ruleset_type="repo", - profile="test_ruleset", - owner="mock", - repo_name="mock_repo", - ) - assert ret == expected +@pytest.mark.parametrize("existing", ([], [{"name": "other_ruleset"}])) +@pytest.mark.usefixtures("testmode") +def test_ruleset_absent_no_changes(existing): + github_mod.__salt__["github.list_rulesets"].return_value = {"rulesets": existing} - # 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", - owner="mock", - repo_name="mock_repo", - ) - assert ret == expected + ret = github_mod.ruleset_absent(**RULESET_KWARGS) + assert ret["result"] is True + assert not ret["changes"] + assert ret["comment"] == "Ruleset mock_ruleset does not exist" + github_mod.__salt__["github.delete_ruleset"].assert_not_called() -def test_repo_absent_changes(): - expected = { - "name": "mock_repo_absent", - "changes": { - "old": "ruleset mock_repo_absent exists", - "new": "ruleset mock_repo_absent deleted", - }, - "result": True, - "comment": "Deleted ruleset mock_repo_absent", +def test_ruleset_absent_changes(testmode): + github_mod.__salt__["github.list_rulesets"].return_value = { + "rulesets": [{"name": "mock_ruleset", "id": 1}] } - - 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_mod.__salt__, - {"github.list_rulesets": mock_query, "github.delete_ruleset": mock_delete}, - ): - ret = github_mod.ruleset_absent( - name="mock_repo_absent", - ruleset_type="repo", - profile="test_ruleset", - owner="mock", - repo_name="mock_repo", - ) - assert ret == expected - - # test mode: projected changes are reported - expected = { - "name": "mock_repo_absent", - "changes": { - "old": "ruleset mock_repo_absent exists", - "new": "ruleset mock_repo_absent deleted", - }, - "result": None, - "comment": "Ruleset mock_repo_absent will be deleted", + github_mod.__salt__["github.delete_ruleset"].return_value = { + "comment": "ruleset 1 successfully deleted" } - mock_query = mock.MagicMock(return_value={"rulesets": [{"name": "mock_repo_absent", "id": 1}]}) - mock_delete = mock.MagicMock(return_value={"comment": "ruleset 1 successfully deleted"}) + ret = github_mod.ruleset_absent(**RULESET_KWARGS) - with mock.patch.dict(github_mod.__opts__, {"test": True}): - with mock.patch.dict( - github_mod.__salt__, - {"github.list_rulesets": mock_query, "github.delete_ruleset": mock_delete}, - ): - ret = github_mod.ruleset_absent( - name="mock_repo_absent", - ruleset_type="repo", - profile="test_ruleset", - owner="mock", - repo_name="mock_repo", - ) - assert ret == expected + assert ret["result"] is (None if testmode else True) + assert ret["changes"] == { + "old": "ruleset mock_ruleset exists", + "new": "ruleset mock_ruleset deleted", + } + assert ret["comment"] == ( + "Ruleset mock_ruleset will be deleted" if testmode else "Deleted ruleset mock_ruleset" + ) + assert github_mod.__salt__["github.delete_ruleset"].called is not testmode -def test_repo_present_no_changes(): - # Test mode: a ruleset present as specified is a fulfilled state - expected = { - "name": "mock_repo_absent", - "changes": {}, - "result": True, - "comment": "ruleset present", +@pytest.mark.usefixtures("testmode") +def test_ruleset_present_no_changes(): + github_mod.__salt__["github.list_rulesets"].return_value = { + "rulesets": [{"name": "mock_ruleset", "id": 1}] } + github_mod.__salt__["github.get_ruleset"].return_value = {"name": "mock_ruleset", "id": 1} - 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}) + ret = github_mod.ruleset_present(**RULESET_KWARGS, ruleset_params={}) - with mock.patch.dict(github_mod.__opts__, {"test": True}): - with mock.patch.dict( - github_mod.__salt__, - {"github.list_rulesets": mock_query, "github.get_ruleset": mock_ruleset}, - ): - ret = github_mod.ruleset_present( - name="mock_repo_absent", - ruleset_type="repo", - profile="test_ruleset", - owner="mock", - repo_name="mock_repo", - ruleset_params={}, - ) - assert ret == expected + assert ret["result"] is True + assert not ret["changes"] + assert ret["comment"] == "ruleset present" + github_mod.__salt__["github.update_ruleset"].assert_not_called() + github_mod.__salt__["github.add_ruleset"].assert_not_called() - with mock.patch.dict( - github_mod.__salt__, - {"github.list_rulesets": mock_query, "github.get_ruleset": mock_ruleset}, - ): - ret = github_mod.ruleset_present( - name="mock_repo_absent", - ruleset_type="repo", - profile="test_ruleset", - owner="mock", - repo_name="mock_repo", - ruleset_params={}, - ) - assert ret == expected - -def test_repo_present_update_ruleset(): - expected = { - "name": "mock_repo_absent", - "changes": { - "old": {"target": "branch"}, - "new": {"target": "tag"}, - }, - "result": True, - "comment": "ruleset updated", +def test_ruleset_present_update_ruleset(testmode): + github_mod.__salt__["github.list_rulesets"].return_value = { + "rulesets": [{"name": "mock_ruleset", "id": 1}] } - - 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, "target": "branch"} - ) - mock_update = mock.MagicMock( - return_value={"name": "mock_repo_absent", "id": 1, "target": "tag"} - ) - - with mock.patch.dict( - github_mod.__salt__, - { - "github.list_rulesets": mock_query, - "github.get_ruleset": mock_ruleset, - "github.update_ruleset": mock_update, - }, - ): - ret = github_mod.ruleset_present( - name="mock_repo_absent", - ruleset_type="repo", - profile="test_ruleset", - owner="mock", - repo_name="mock_repo", - ruleset_params={"target": "tag"}, - ) - assert ret == expected - - # Test mode: forecast changes are reported - expected = { - "name": "mock_repo_absent", - "changes": { - "old": {"target": "branch"}, - "new": {"target": "tag"}, - }, - "result": None, - "comment": "ruleset will be updated", + github_mod.__salt__["github.get_ruleset"].return_value = { + "name": "mock_ruleset", + "id": 1, + "target": "branch", + } + github_mod.__salt__["github.update_ruleset"].return_value = { + "name": "mock_ruleset", + "id": 1, + "target": "tag", } - with mock.patch.dict( - github_mod.__salt__, - { - "github.list_rulesets": mock_query, - "github.get_ruleset": mock_ruleset, - "github.update_ruleset": mock_update, - }, - ): - with mock.patch.dict(github_mod.__opts__, {"test": True}): - ret = github_mod.ruleset_present( - name="mock_repo_absent", - ruleset_type="repo", - profile="test_ruleset", - owner="mock", - repo_name="mock_repo", - ruleset_params={"target": "tag"}, - ) - assert ret == expected + ret = github_mod.ruleset_present(**RULESET_KWARGS, ruleset_params={"target": "tag"}) + assert ret["result"] is (None if testmode else True) + assert ret["changes"] == {"old": {"target": "branch"}, "new": {"target": "tag"}} + assert ret["comment"] == ("ruleset will be updated" if testmode else "ruleset updated") + assert github_mod.__salt__["github.update_ruleset"].called is not testmode -def test_repo_present_add_ruleset(): - expected = { - "name": "mock_repo_absent", - "changes": {"old": "No existing ruleset found", "new": "Ruleset created"}, - "result": True, - "comment": "ruleset added", +def test_ruleset_present_add_ruleset(testmode): + github_mod.__salt__["github.list_rulesets"].return_value = {"rulesets": None} + github_mod.__salt__["github.add_ruleset"].return_value = { + "name": "mock_ruleset", + "id": 1, + "enforcement": "disabled", } - mock_query = mock.MagicMock(return_value={"rulesets": None}) - mock_add = mock.MagicMock( - return_value={"name": "mock_repo_absent", "id": 1, "enforcement": "disabled"} - ) - - with mock.patch.dict( - github_mod.__salt__, {"github.list_rulesets": mock_query, "github.add_ruleset": mock_add} - ): - ret = github_mod.ruleset_present( - name="mock_repo_absent", - ruleset_type="repo", - profile="test_ruleset", - owner="mock", - repo_name="mock_repo", - ruleset_params={"enforcement": "disabled"}, - ) - assert ret == expected - - # Test mode: forecast changes are reported - expected["result"] = None - expected["comment"] = "ruleset will be added" + ret = github_mod.ruleset_present(**RULESET_KWARGS, ruleset_params={"enforcement": "disabled"}) - with mock.patch.dict(github_mod.__opts__, {"test": True}): - with mock.patch.dict( - github_mod.__salt__, - {"github.list_rulesets": mock_query, "github.add_ruleset": mock_add}, - ): - ret = github_mod.ruleset_present( - name="mock_repo_absent", - ruleset_type="repo", - profile="test_ruleset", - owner="mock", - repo_name="mock_repo", - ruleset_params={"enforcement": "disabled"}, - ) - assert ret == expected + assert ret["result"] is (None if testmode else True) + assert ret["changes"] == {"old": "No existing ruleset found", "new": "Ruleset created"} + assert ret["comment"] == ("ruleset will be added" if testmode else "ruleset added") + assert github_mod.__salt__["github.add_ruleset"].called is not testmode diff --git a/tests/unit/states/test_github_failures.py b/tests/unit/states/test_github_failures.py index 7f70e879..c1a061e9 100644 --- a/tests/unit/states/test_github_failures.py +++ b/tests/unit/states/test_github_failures.py @@ -11,6 +11,198 @@ from saltext.github.states import github as github_mod +_TEAM_TARGET = {"description": "desc", "permission": "pull", "privacy": "closed"} + +FAILURE_CASES = ( + pytest.param( + "present", + "user1", + {}, + { + "get_user": {"return_value": {"in_org": False, "membership_state": "nonexistent"}}, + "add_user": {"return_value": False}, + }, + "Failed to add user", + id="present-add", + ), + pytest.param( + "absent", + "user1", + {}, + { + "get_user": {"return_value": True}, + "remove_user": {"return_value": False}, + }, + "Failed to delete", + id="absent-remove", + ), + pytest.param( + "team_absent", + "team", + {}, + { + "get_team": {"return_value": {"id": 1}}, + "remove_team": {"return_value": False}, + }, + "Failed to delete", + id="team_absent-remove", + ), + pytest.param( + "repo_absent", + "repo", + {}, + { + "get_repo_info": {"return_value": {"name": "repo"}}, + "remove_repo": {"return_value": False}, + }, + "delete_repo scope", + id="repo_absent-remove", + ), + pytest.param( + "team_present", + "team", + {"description": "changed", "privacy": "closed", "permission": "pull"}, + { + "get_team": {"return_value": _TEAM_TARGET}, + "edit_team": {"return_value": False}, + }, + "Failed to update team properties.", + id="team_present-edit", + ), + pytest.param( + "team_present", + "team", + { + "description": "desc", + "privacy": "closed", + "permission": "pull", + "repo_names": ["repo-b"], + }, + { + "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}, + }, + "Failed to remove repo repo-a", + id="team_present-remove-repo", + ), + pytest.param( + "team_present", + "team", + {"description": "desc", "privacy": "closed", "permission": "pull", "members": {}}, + { + "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}, + }, + "Failed to remove user olduser", + id="team_present-remove-member", + ), + pytest.param( + "repo_present", + "repo", + {"description": "new"}, + { + "list_repos": {"return_value": ["repo"]}, + "get_repo_info": {"return_value": {"name": "repo", "description": "old"}}, + "edit_repo": {"return_value": False}, + }, + "Failed to update repo properties.", + id="repo_present-edit", + ), + pytest.param( + "repo_present", + "repo", + {}, + { + "list_repos": {"return_value": []}, + "get_repo_info": {"side_effect": CommandExecutionError("missing")}, + "add_repo": {"return_value": False}, + }, + "Failed to create repo", + id="repo_present-create", + ), + pytest.param( + "repo_present", + "repo", + {}, + { + "list_repos": {"return_value": []}, + "get_repo_info": {"side_effect": CommandExecutionError("missing")}, + "add_repo": {"return_value": True}, + # The post-creation team fetch keeps returning 404 + "get_repo_teams": {"side_effect": CommandExecutionError("404")}, + }, + "Failed to verify repo", + id="repo_present-verify-after-create", + ), + pytest.param( + "repo_present", + "repo", + {"teams": {}}, + { + "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}, + }, + "Failed to remove repo repo from team Team A", + id="repo_present-remove-team", + ), + pytest.param( + "repo_present", + "repo", + {"teams": {"Team A": "push"}}, + { + "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": {}}, + }, + "Failed to determine current permission", + id="repo_present-permission-lookup", + ), + pytest.param( + "repo_present", + "repo", + {"teams": {"Team A": "push"}}, + { + "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}, + }, + "Failed to set permission", + id="repo_present-permission-change", + ), + pytest.param( + "ruleset_absent", + "rules", + {"ruleset_type": "org"}, + { + "list_rulesets": {"return_value": {"rulesets": [{"name": "rules", "id": 1}]}}, + "delete_ruleset": {"return_value": {"result": False, "comment": "403 nope"}}, + }, + "Failed to delete ruleset rules", + id="ruleset_absent-delete", + ), + pytest.param( + "ruleset_present", + "rules", + {"ruleset_type": "org", "ruleset_params": {"enforcement": "active"}}, + { + "list_rulesets": {"return_value": {"rulesets": [{"name": "rules", "id": 1}]}}, + "get_ruleset": {"return_value": {"result": False, "comment": "403 nope"}}, + }, + "Could not get ruleset", + id="ruleset_present-get", + ), +) + @pytest.fixture def configure_loader_modules(): @@ -22,193 +214,12 @@ def configure_loader_modules(): } -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"} - ) +@pytest.mark.parametrize("func,name,kwargs,salt_mocks,expected_comment", FAILURE_CASES) +def test_failure_is_reported(func, name, kwargs, salt_mocks, expected_comment): + patched = { + f"github.{salt_func}": mock.MagicMock(**spec) for salt_func, spec in salt_mocks.items() + } + with mock.patch.dict(github_mod.__salt__, patched), mock.patch("time.sleep"): + ret = getattr(github_mod, func)(name, **kwargs) assert ret["result"] is False - assert ret["comment"] == "Could not get ruleset" + assert expected_comment in ret["comment"] diff --git a/tests/unit/states/test_github_org.py b/tests/unit/states/test_github_org.py new file mode 100644 index 00000000..7a053e18 --- /dev/null +++ b/tests/unit/states/test_github_org.py @@ -0,0 +1,30 @@ +from unittest import mock + +import pytest +from salt.exceptions import CommandExecutionError + +from saltext.github.states import github_org as github_org_mod +from saltext.github.utils.gh.exceptions import GitHubNotFoundError + + +@pytest.fixture +def configure_loader_modules(): + return { + github_org_mod: { + "__salt__": { + "github_org.get_team": mock.MagicMock(side_effect=GitHubNotFoundError("missing")), + "github_org.add_team": mock.MagicMock(), + "github_org.add_team_member": mock.MagicMock( + side_effect=[None, CommandExecutionError("API hiccup")] + ), + }, + "__opts__": {"test": False}, + }, + } + + +def test_team_present_reports_partial_changes_on_failure(): + ret = github_org_mod.team_present("team", members=["user1", "user2"]) + assert ret["result"] is False + assert ret["comment"] == "API hiccup" + assert ret["changes"] == {"created": "team", "members_added": ["user1"]} diff --git a/tests/unit/utils/test_client.py b/tests/unit/utils/test_client.py index b6109846..5a5215b4 100644 --- a/tests/unit/utils/test_client.py +++ b/tests/unit/utils/test_client.py @@ -17,13 +17,18 @@ def requester(): @pytest.fixture def conn(requester): - return MagicMock(client=MagicMock(requester=requester)) + with patch( + "saltext.github.utils.gh.client.github.Github", + autospec=True, + return_value=MagicMock(requester=requester), + ): + yield gh_client.GitHubConnection("token") def test_query_decodes_json_string_data(conn, requester): requester.requestJson.return_value = (200, {}, '{"written": true}') - ret = gh_client.query(conn, "repos", method="POST", data='{"some": "payload"}') + ret = conn.query("repos", method="POST", data='{"some": "payload"}') assert ret == {"written": True} requester.requestJson.assert_called_once_with( @@ -37,7 +42,7 @@ def test_query_merges_paginated_results(conn, requester): (200, {}, '[{"id": 2}]'), ) - ret = gh_client.query(conn, ["repos"]) + ret = conn.query(["repos"]) assert ret == [{"id": 1}, {"id": 2}] assert requester.requestJson.call_args.kwargs["parameters"] == {"page": "2"} @@ -47,7 +52,7 @@ def test_query_github_exception(conn, requester): requester.requestJson.side_effect = pygithub.GithubException(502, {"message": "bad gateway"}) with pytest.raises(GitHubApiError, match="bad gateway") as exc: - gh_client.query(conn, "repos") + conn.query("repos") assert exc.value.status == 502 @@ -56,7 +61,7 @@ def test_query_error_response(conn, requester): requester.requestJson.return_value = (404, {}, '{"message": "Not Found"}') with pytest.raises(GitHubNotFoundError, match="404 Not Found") as exc: - gh_client.query(conn, "repos") + conn.query("repos") assert exc.value.status == 404 assert exc.value.data == {"message": "Not Found"} @@ -66,7 +71,7 @@ def test_query_unexpected_exception(conn, requester): requester.requestJson.side_effect = OSError("connection refused") with pytest.raises(CommandExecutionError, match="connection refused"): - gh_client.query(conn, "repos") + conn.query("repos") def test_api_request_translates_pygithub_exceptions(): @@ -93,22 +98,22 @@ def token_conn(): yield gh_client.GitHubConnection("token") -def test_owner_defaults_to_token_user(token_conn): - token_conn.client.get_user.return_value.login = "tokenuser" - assert token_conn.owner == "tokenuser" - token_conn.client.get_user.assert_called_once_with() +def test_owner_defaults_to_token_user(conn): + conn.client.get_user.return_value.login = "tokenuser" + assert conn.owner == "tokenuser" + conn.client.get_user.assert_called_once_with() # The resolution result is cached - assert token_conn.owner == "tokenuser" - token_conn.client.get_user.assert_called_once_with() + assert conn.owner == "tokenuser" + conn.client.get_user.assert_called_once_with() -def test_owner_lookup_failure_hints_at_profile_parameter(token_conn): +def test_owner_lookup_failure_hints_at_profile_parameter(conn): # GitHub App installation tokens are not tied to a user account - token_conn.client.get_user.side_effect = pygithub.GithubException( + conn.client.get_user.side_effect = pygithub.GithubException( 403, {"message": "Resource not accessible by integration"}, None ) with pytest.raises(CommandExecutionError, match="require the 'owner' profile parameter"): - token_conn.owner # pylint: disable=pointless-statement + conn.owner # pylint: disable=pointless-statement def test_explicit_owner_skips_token_lookup(): diff --git a/tests/unit/utils/test_statecores.py b/tests/unit/utils/test_statecores.py new file mode 100644 index 00000000..75d11514 --- /dev/null +++ b/tests/unit/utils/test_statecores.py @@ -0,0 +1,40 @@ +from salt.exceptions import CommandExecutionError + +from saltext.github.utils.gh import statecores + + +def test_catch_errors_renders_failed_state(): + @statecores.catch_errors + def _state(name): + raise CommandExecutionError("damn") + + assert _state("mystate") == { + "name": "mystate", + "changes": {}, + "result": False, + "comment": "damn", + } + + +def test_catch_errors_reports_partial_changes(): + @statecores.catch_errors + def _state(name): + raise statecores.StateError("damn", changes={"created": "foo"}) + + assert _state("mystate") == { + "name": "mystate", + "changes": {"created": "foo"}, + "result": False, + "comment": "damn", + } + + +def test_catch_errors_passes_through_success(): + ret = {"name": "mystate", "changes": {}, "result": True, "comment": "all good"} + + @statecores.catch_errors + def _state(name, extra, *, kwarg=None): + assert (name, extra, kwarg) == ("mystate", "pos", "kw") + return ret + + assert _state("mystate", "pos", kwarg="kw") is ret diff --git a/tests/unit/utils/test_statecores_environments.py b/tests/unit/utils/test_statecores_environments.py new file mode 100644 index 00000000..4a3100a5 --- /dev/null +++ b/tests/unit/utils/test_statecores_environments.py @@ -0,0 +1,44 @@ +from contextlib import ExitStack +from unittest.mock import MagicMock +from unittest.mock import patch + +import pytest +from salt.exceptions import CommandExecutionError + +from saltext.github.utils.gh.statecores import environments as sc_environments + + +@pytest.fixture +def gh_mocks(): + mocks = {} + with ExitStack() as stack: + for func in ( + "get", + "normalize", + "branch_policy_from_patterns", + "list_branch_policies", + "set_", + "create_branch_policy", + "delete_branch_policy", + ): + mocks[func] = stack.enter_context( + patch.object(sc_environments.gh_environments, func, autospec=True) + ) + yield mocks + + +def test_present_reports_partial_branch_policy_changes(gh_mocks): + policy = {"protected_branches": False, "custom_branch_policies": True} + gh_mocks["get"].return_value = {"name": "release"} + gh_mocks["normalize"].return_value = {"deployment_branch_policy": policy} + gh_mocks["branch_policy_from_patterns"].return_value = policy + gh_mocks["list_branch_policies"].return_value = [] + gh_mocks["create_branch_policy"].side_effect = [None, CommandExecutionError("quota exceeded")] + + with pytest.raises(CommandExecutionError, match="quota exceeded") as exc: + sc_environments.present( + MagicMock(), "release", "repo", deployment_branches=["br-a", "br-b"] + ) + + assert exc.value.changes == {"deployment_branches": {"added": ["br-a"]}} + gh_mocks["set_"].assert_not_called() diff --git a/tests/unit/utils/test_statecores_repo.py b/tests/unit/utils/test_statecores_repo.py new file mode 100644 index 00000000..155558dd --- /dev/null +++ b/tests/unit/utils/test_statecores_repo.py @@ -0,0 +1,71 @@ +from contextlib import ExitStack +from unittest.mock import MagicMock +from unittest.mock import patch + +import pytest +from salt.exceptions import CommandExecutionError + +from saltext.github.utils.gh.exceptions import GitHubApiError +from saltext.github.utils.gh.exceptions import GitHubNotFoundError +from saltext.github.utils.gh.statecores import repo as sc_repo + + +@pytest.fixture +def gh_mocks(): + mocks = {} + with ExitStack() as stack: + for mod, funcs in ( + (sc_repo.gh_repos, ("list_all", "get_info", "create", "update", "get_teams")), + (sc_repo.gh_teams, ("find", "list_repos", "add_repo", "remove_repo")), + ): + for func in funcs: + mocks[func] = stack.enter_context(patch.object(mod, func, autospec=True)) + mocks["find"].return_value = {"slug": "team-a"} + mocks["add_repo"].side_effect = GitHubApiError(FAILURE_MSG) + yield mocks + + +@pytest.fixture +def conn(): + return MagicMock() + + +FAILURE_MSG = "Failed setting permission push on repo repo for team Team A." + + +def test_present_reports_team_association_failure_on_create(gh_mocks, conn): + gh_mocks["get_info"].side_effect = GitHubNotFoundError("missing") + with pytest.raises(CommandExecutionError, match=FAILURE_MSG): + sc_repo.present(conn, "repo", teams={"Team A": "push"}) + + +def test_present_reports_team_association_failure_on_added_team(gh_mocks, conn): + gh_mocks["get_info"].return_value = {} + gh_mocks["get_teams"].return_value = [] + with pytest.raises(CommandExecutionError, match=FAILURE_MSG): + sc_repo.present(conn, "repo", teams={"Team A": "push"}) + + +def test_present_reports_permission_update_failure(gh_mocks, conn): + gh_mocks["get_info"].return_value = {} + gh_mocks["get_teams"].return_value = [{"name": "Team A"}] + gh_mocks["list_repos"].return_value = {"repo": {"permission": "pull"}} + with pytest.raises(CommandExecutionError, match=FAILURE_MSG): + sc_repo.present(conn, "repo", teams={"Team A": "push"}) + + +def test_present_reports_partial_team_changes(gh_mocks, conn): + gh_mocks["get_info"].return_value = {} + gh_mocks["get_teams"].return_value = [] + gh_mocks["add_repo"].side_effect = [None, GitHubApiError(FAILURE_MSG)] + with pytest.raises(CommandExecutionError, match=FAILURE_MSG) as exc: + sc_repo.present(conn, "repo", teams={"Team A": "push", "Team B": "push"}) + assert exc.value.changes == {"teams_added": ["Team A"]} + + +def test_present_reports_partial_changes_on_create(gh_mocks, conn): + gh_mocks["get_info"].side_effect = GitHubNotFoundError("missing") + gh_mocks["add_repo"].side_effect = [None, GitHubApiError(FAILURE_MSG)] + with pytest.raises(CommandExecutionError, match=FAILURE_MSG) as exc: + sc_repo.present(conn, "repo", teams={"Team A": "push", "Team B": "push"}) + assert exc.value.changes == {"created": "repo", "teams_added": ["Team A"]} diff --git a/tests/unit/utils/test_teams.py b/tests/unit/utils/test_teams.py new file mode 100644 index 00000000..1d1145db --- /dev/null +++ b/tests/unit/utils/test_teams.py @@ -0,0 +1,16 @@ +from unittest.mock import MagicMock + +import pytest + +from saltext.github.utils.gh import teams as gh_teams +from saltext.github.utils.gh.exceptions import GitHubApiError + + +def test_add_repo_raises_on_permission_update_failure(): + conn = MagicMock() + team = conn.get_org.return_value.get_team_by_slug.return_value + team.update_team_repository.return_value = False + with pytest.raises( + GitHubApiError, match="Failed setting permission push on repo repo for team Team A" + ): + gh_teams.add_repo(conn, {"slug": "team-a", "name": "Team A"}, "repo", permission="push") From 2d8d8fd12e986b23a1df04dd769e42aae188dd31 Mon Sep 17 00:00:00 2001 From: jeanluc Date: Wed, 14 Oct 2026 11:33:09 +0200 Subject: [PATCH 3/3] Add xdist, fix pylint not-callable false positibe --- pyproject.toml | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index 74243b93..11de8261 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -81,17 +81,23 @@ docs = [ ] docsauto = ["sphinx-autobuild"] lint = [ - "pylint==4.0.4", + "pylint==4.0.9", ] tests = [ "pytest>=7.2.0", "pytest-salt-factories>=1.0.0", "pytest-instafail", + "pytest-xdist", ] [project.entry-points."salt.loader"] "saltext.github" = "saltext.github" +[tool.pytest.ini_options] +# Best test distribution for the module-scoped functional fixtures. +# Only effective when parallelizing via ``-n ``/``-n auto``. +addopts = "--dist=loadscope" + [tool.uv] # Salt's legacy setup.py requires old setuptools. # Since uv 0.6.0, it does not fall back to reading requirements from egg-info.