Skip to content

Commit

Permalink
Support aliases and groups in allowed_* config options for Koji
Browse files Browse the repository at this point in the history
Introduce aliases `all_admins` and `all_committers` and also support
groups in allowed_pr_authors and allowed_committers config options

Fixes packit/packit#2088
Requires packit/ogr#834
  • Loading branch information
lbarcziova committed Jan 24, 2024
1 parent d4377bb commit 033fee3
Show file tree
Hide file tree
Showing 3 changed files with 122 additions and 4 deletions.
5 changes: 5 additions & 0 deletions packit_service/constants.py
Original file line number Diff line number Diff line change
Expand Up @@ -289,3 +289,8 @@ def from_number(number: int):
"of the `pull_from_upstream` job. We believe this adjustment will simplify the onboarding "
"process and enhance the overall user experience. "
)


class KojiAllowedAccountsAlias(Enum):
all_admins = "all_admins"
all_committers = "all_committers"
71 changes: 68 additions & 3 deletions packit_service/worker/checker/distgit.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,9 @@
import logging
import re

from ogr.abstract import AccessLevel
from packit.config.aliases import get_branches
from packit_service.constants import MSG_GET_IN_TOUCH
from packit_service.constants import MSG_GET_IN_TOUCH, KojiAllowedAccountsAlias
from packit_service.worker.checker.abstract import Checker, ActorChecker
from packit_service.worker.events import (
PushPagureEvent,
Expand Down Expand Up @@ -43,6 +44,66 @@ def contains_specfile_change(self):
return False
return True

@staticmethod
def is_koji_allowed_accounts_alias(value: str) -> bool:
return any(value == alias.value for alias in KojiAllowedAccountsAlias)

def check_allowed_accounts(
self, accounts_list: list[str], account_to_check: str
) -> bool:
"""
Check whether the account_to_check matches one of the values in accounts_list
(considering the groups and aliases).
"""
logger.info(f"Checking {account_to_check} in list of accounts: {accounts_list}")

direct_account_names = [
value
for value in accounts_list
if not self.is_koji_allowed_accounts_alias(value)
and not value.startswith("@")
]

# check the direct account names to prevent unneeded API interactions
if account_to_check in direct_account_names:
return True

all_accounts = set()

for value in accounts_list:
if self.is_koji_allowed_accounts_alias(value):
all_accounts.update(self.expand_maintainer_alias(value))
elif value.startswith("@"):
try:
# remove @
group = self.project.get_group(value[1:])
all_accounts.update(group.members)
except Exception:
pass
else:
all_accounts.update(value)

logger.debug(f"Expanded accounts list: {all_accounts}")
return account_to_check in all_accounts

def expand_maintainer_alias(self, alias: str) -> set[str]:
"""
Expand the 'all_admins' and 'all_committers' aliases to users.
"""
# see AccessLevel mapping
# https://github.com/packit/ogr/blob/d183a6c6459231c2a60bacd6b827502c92a130ef/ogr/abstract.py#L1079
# all_admins -> Pagure "admin" and "maintainer" access
# all_committers -> on top of that "commit" access
access_levels = [AccessLevel.maintain]

if alias == KojiAllowedAccountsAlias.all_committers.value:
access_levels.extend([AccessLevel.admin, AccessLevel.push])

accounts = self.project.get_users_with_given_access(access_levels)

logger.debug(f"Expanded {alias}: {accounts}")
return accounts

def pre_check(self) -> bool:
if self.data.event_type in (PushPagureEvent.__name__,):
if self.data.git_ref not in (
Expand Down Expand Up @@ -71,7 +132,9 @@ def pre_check(self) -> bool:

pr_author = self.get_pr_author()
logger.debug(f"PR author: {pr_author}")
if pr_author not in self.job_config.allowed_pr_authors:
if not self.check_allowed_accounts(
self.job_config.allowed_pr_authors, pr_author
):
logger.info(
f"Push event {self.data.identifier} with corresponding PR created by"
f" {pr_author} that is not allowed in project "
Expand All @@ -81,7 +144,9 @@ def pre_check(self) -> bool:
else:
committer = self.data.event_dict["committer"]
logger.debug(f"Committer: {committer}")
if committer not in self.job_config.allowed_committers:
if not self.check_allowed_accounts(
self.job_config.allowed_committers, committer
):
logger.info(
f"Push event {self.data.identifier} done by "
f"{committer} that is not allowed in project "
Expand Down
50 changes: 49 additions & 1 deletion tests/unit/test_checkers.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,15 @@
import pytest
from flexmock import flexmock

from ogr.abstract import AccessLevel
from ogr.services.pagure import PagureProject
from packit.config import (
CommonPackageConfig,
JobType,
JobConfigTriggerType,
JobConfigView,
JobConfig,
PackageConfig,
)

from packit.config.commands import TestCommandConfig
Expand All @@ -19,7 +22,10 @@
IsJobConfigTriggerMatching as IsJobConfigTriggerMatchingCopr,
IsPackageMatchingJobView,
)
from packit_service.worker.checker.distgit import IsUpstreamTagMatchingConfig
from packit_service.worker.checker.distgit import (
IsUpstreamTagMatchingConfig,
PermissionOnDistgit,
)
from packit_service.worker.checker.koji import (
IsJobConfigTriggerMatching as IsJobConfigTriggerMatchingKoji,
)
Expand Down Expand Up @@ -919,3 +925,45 @@ def test_sync_release_matching_tag(upstream_tag_include, upstream_tag_exclude, r
)

assert checker.pre_check() == result


@pytest.mark.parametrize(
"account, allowed_pr_authors, should_pass",
(
("admin-1", ["all_admins"], False),
("admin-2", ["all_admins"], False),
),
)
def test_koji_check_allowed_accounts(
distgit_push_event,
account,
allowed_pr_authors,
should_pass,
):
jobs = [
JobConfig(
type=JobType.koji_build,
trigger=JobConfigTriggerType.commit,
packages={
"package": CommonPackageConfig(
dist_git_branches=["f36"],
allowed_pr_authors=allowed_pr_authors,
)
},
),
]

package_config = PackageConfig(
jobs=jobs,
packages={"package": CommonPackageConfig()},
)
job_config = jobs[0]

flexmock(PagureProject).should_receive("get_users_with_given_access").with_args(
[AccessLevel.maintain]
).and_return("admin-1")

checker = PermissionOnDistgit(
package_config, job_config, distgit_push_event.get_dict()
)
assert checker.check_allowed_accounts(allowed_pr_authors, account) == should_pass

0 comments on commit 033fee3

Please sign in to comment.