Skip to content

Commit

Permalink
Remove old FAS calls from FMN
Browse files Browse the repository at this point in the history
FAS2 is no longer used by anything except FMN. Let's remove these calls from FMN
and use fasjson_client instead.

Signed-off-by: Michal Konečný <mkonecny@redhat.com>
  • Loading branch information
Zlopez committed Jul 20, 2022
1 parent 92541c6 commit 188101f
Show file tree
Hide file tree
Showing 7 changed files with 84 additions and 55 deletions.
33 changes: 13 additions & 20 deletions fmn/rules/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
import fedmsg.meta

from dogpile.cache import make_region
from fedora.client.fas2 import AccountSystem
import fasjson_client

log = logging.getLogger(__name__)

Expand Down Expand Up @@ -42,8 +42,8 @@ def compile_regex(pattern):


def get_fas(config):
""" Return a fedora.client.fas2.AccountSystem object if the provided
configuration contains a FAS username and password.
""" Return a fasjson_client object if the provided
configuration contains a fasjson configuration.
"""
global _FAS
if _FAS is not None:
Expand All @@ -52,20 +52,12 @@ def get_fas(config):
# In some development environments, having fas_credentials around is a
# pain.. so, let things proceed here, but emit a warning.
try:
creds = config['fas_credentials']
fasjson_config = config['fasjson']
except KeyError:
log.warning("No fas_credentials available. Unable to query FAS.")
log.warning("No fasjson config available. Unable to query FAS.")
return None

default_url = 'https://admin.fedoraproject.org/accounts/'

_FAS = AccountSystem(
creds.get('base_url', default_url),
username=creds['username'],
password=creds['password'],
cache_session=False,
insecure=creds.get('insecure', False)
)
client = fasjson_client.Client(url=fasjson.get('url'))

return _FAS

Expand Down Expand Up @@ -395,7 +387,7 @@ def get_user_of_group(config, fas, groupname):
''' Return the list of users in the specified group.
:arg config: a dict containing the fedmsg config
:arg fas: a fedora.client.fas2.AccountSystem object instanciated and loged
:arg fas: a fasjson_client object instanciated and loged
into FAS.
:arg groupname: the name of the group for which we want to retrieve the
members.
Expand All @@ -409,15 +401,15 @@ def get_user_of_group(config, fas, groupname):
def creator():
if not fas:
return set()
return set([u.username for u in fas.group_members(groupname)])
return set([u["username"] for u in fas.list_group_members(groupname).result])
return _cache.get_or_create(key, creator)


def get_groups_of_user(config, fas, username):
''' Return the list of (pkgdb) groups to which the user belongs.
:arg config: a dict containing the fedmsg config
:arg fas: a fedora.client.fas2.AccountSystem object instanciated and loged
:arg fas: a fasjson_client object instanciated and loged
into FAS.
:arg username: the name of a user for which we want to retrieve groups
:return: a list of FAS groups to which the user belongs.
Expand All @@ -432,9 +424,10 @@ def creator():
if not fas:
return []
results = []
for group in fas.person_by_username(username).get('memberships', []):
if group['group_type'] == 'pkgdb':
results.append(group.name)
for group in fas.list_all_entities("groups"):
members = set([u["username"] for u in fas.list_group_members(group["groupname"]).result])
if username in members:
results.append(group["groupname"])
return results

return _cache.get_or_create(key, creator)
Expand Down
21 changes: 13 additions & 8 deletions fmn/tests/rules/test_generic.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,16 +22,13 @@
import unittest

from fedmsg.meta import make_processors
from fedora.client.fas2 import AccountSystem
import mock

from fmn.rules import generic
from fmn.tests import Base


@mock.patch('fmn.rules.utils._FAS', new=AccountSystem(
'https://admin.fedoraproject.org/accounts/', username='jcline', password='dummypassword',
cache_session=False, insecure=False))
@mock.patch('fmn.rules.utils._FAS', new=mock.MagicMock())
class UserPackageFilterTests(Base):

def setUp(self):
Expand Down Expand Up @@ -160,22 +157,30 @@ def test_empty_message(self):
"""Assert that an empty message results in False."""
self.assertFalse(generic.user_package_filter(self.config, {}))

def test_no_matching_packages(self):
@mock.patch('fmn.rules.utils.get_packages_of_user')
def test_no_matching_packages(self, mock_packages):
"""Assert that a message with no matching packages results in False."""
mock_packages.return_value = {"rpms": {"package1", "package2", "package3"}}
self.assertFalse(generic.user_package_filter(self.config, self.rpm_msg, fasnick='jcline'))

def test_matching_packages(self):
@mock.patch('fmn.rules.utils.get_packages_of_user')
def test_matching_packages(self, mock_packages):
"""Assert that a message with matching packages results in True."""
mock_packages.return_value = {"rpms": {"python-pystray"}}
self.assertTrue(generic.user_package_filter(self.config, self.rpm_msg, fasnick='besser82'))

def test_different_namespaces(self):
@mock.patch('fmn.rules.utils.get_packages_of_user')
def test_different_namespaces(self, mock_packages):
"""Assert packages with the same name in a different namespace results in False."""
# remi owns the php RPM, but not the module.
mock_packages.return_value = {"rpms": {"php"}}
self.assertFalse(generic.user_package_filter(self.config, self.module_msg, fasnick='remi'))

def test_namespaces(self):
@mock.patch('fmn.rules.utils.get_packages_of_user')
def test_namespaces(self, mock_packages):
"""Assert that a message with a namespaced package works correctly."""
# ralph owns the php module
mock_packages.return_value = {"modules": {"php"}}
self.assertTrue(generic.user_package_filter(self.config, self.module_msg, fasnick='ralph'))


Expand Down
70 changes: 51 additions & 19 deletions fmn/tests/rules/test_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@

import unittest

from fedora.client.fas2 import AccountSystem
from requests.exceptions import ConnectTimeout, ReadTimeout
from dogpile import cache
import mock
Expand Down Expand Up @@ -150,9 +149,6 @@ def test_read_failure(self, mock_session):
self.assertEqual(set(), packages)


@mock.patch('fmn.rules.utils._FAS', new=AccountSystem(
'https://admin.fedoraproject.org/accounts/', username='jcline', password='dummypassword',
cache_session=False, insecure=False))
class GetPkgdb2PackagersForTests(Base):

def setUp(self):
Expand All @@ -176,19 +172,14 @@ def test_no_groups(self):

self.assertTrue(expected_packagers.issubset(packagers))

def test_groups(self):
@mock.patch('fmn.rules.utils.get_fas')
def test_groups(self, mock_get_fas):
infra_group = set([
'abompard',
'bochecha',
'bowlofeggs',
'codeblock',
'jcline',
'kevin',
'lmacken',
'pingou',
'puiterwijk',
'sayanchowdhury',
'toshio',
])
committers = set([
'abompard',
Expand All @@ -198,6 +189,28 @@ def test_groups(self):
])
expected_packagers = infra_group.union(committers)

mock_fasjson_client = mock.MagicMock()
mock_response = mock.MagicMock()
mock_response.result = [
{
"username": "abompard",
},
{
"username": "bochecha",
},
{
"username": "bowlofeggs",
},
{
"username": "codeblock",
},
{
"username": "jcline",
},
]
mock_fasjson_client.list_group_members.return_value = mock_response
mock_get_fas.return_value = mock_fasjson_client

packagers = utils._get_packagers_for(self.config, 'rpms/python-urllib3')

self.assertTrue(expected_packagers.issubset(packagers))
Expand Down Expand Up @@ -389,19 +402,14 @@ def test_no_groups(self):

self.assertEqual(expected_packagers, packagers)

def test_groups(self):
@mock.patch('fmn.rules.utils.get_fas')
def test_groups(self, mock_get_fas):
infra_group = set([
'abompard',
'bochecha',
'bowlofeggs',
'codeblock',
'jcline',
'kevin',
'lmacken',
'pingou',
'puiterwijk',
'sayanchowdhury',
'toshio',
])
committers = set([
'abompard',
Expand All @@ -410,9 +418,33 @@ def test_groups(self):
'sagarun',
])

expected_packagers = infra_group.union(committers)

mock_fasjson_client = mock.MagicMock()
mock_response = mock.MagicMock()
mock_response.result = [
{
"username": "abompard",
},
{
"username": "bochecha",
},
{
"username": "bowlofeggs",
},
{
"username": "codeblock",
},
{
"username": "jcline",
},
]
mock_fasjson_client.list_group_members.return_value = mock_response
mock_get_fas.return_value = mock_fasjson_client

packagers = utils._get_packagers_for(self.config, 'rpms/python-urllib3')

self.assertEqual(infra_group.union(committers), packagers)
self.assertEqual(expected_packagers, packagers)

def test_no_namespace(self):
"""Assert packages without a namespace default to RPM."""
Expand Down
11 changes: 6 additions & 5 deletions fmn/util.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import fedora.client
import fasjson_client

import logging
Expand Down Expand Up @@ -29,10 +28,12 @@ def get_fas_email(config, username):
have that alias available yet.
"""
try:
fas = fedora.client.AccountSystem(**config['fas_credentials'])
person = fas.person_by_username(username)
if person.get('email'):
return person['email']
fasjson = config["fasjson"]
client = fasjson_client.Client(url=fasjson.get('url'))
person = client.get_user(username=username).result

if person.get('emails'):
return person.get('emails')[0]
raise ValueError("No email found: %r" % username)
except Exception:
log.exception("Failed to get FAS email for %r" % username)
Expand Down
1 change: 0 additions & 1 deletion python-fmn.spec
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ BuildRequires: python3-docutils
BuildRequires: python3-dogpile-cache
BuildRequires: python3-fedmsg
BuildRequires: python3-fedmsg-meta-fedora-infrastructure
BuildRequires: python3-fedora
BuildRequires: python3-flake8
BuildRequires: python3-flask
BuildRequires: python3-flask-openid
Expand Down
1 change: 0 additions & 1 deletion requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ psycopg2
py3dns
pylibravatar
pyOpenSSL
python-fedora
python3-openid
python-openid-cla
python-openid-teams
Expand Down
2 changes: 1 addition & 1 deletion tox.ini
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ basepython=python3
deps =
flake8 > 3.0
commands =
python -m flake8 {posargs}
python -m flake8 fmn/ {posargs}

[flake8]
show-source = True
Expand Down

0 comments on commit 188101f

Please sign in to comment.