infrastructure/toddlers
1
0
Fork 0
Code Issues 9 Pull requests 1 Projects Releases Packages Wiki Activity Actions

Use the messages from journal-to-fedora-messaging instead of those from Noggin

Browse source 
Use the messages from
[journal-to-fedora-messaging](https://journal-to-fedora-messaging-messages.readthedocs.io)
instead of those from Noggin, as they are more reliable: they are
emitted even if the action is done directly in IPA.

Signed-off-by: Aurélien Bompard <aurelien@bompard.org>
This commit is contained in:
Aurélien Bompard 2025-04-02 16:59:09 +02:00 • committed by abompard
parent 254e00c59a
commit 5775425f16
8 changed files with 181 additions and 148 deletions
Download patch file Download diff file Expand all files Collapse all files

4
README.rst
View file

@ -134,12 +134,12 @@ Fedora-messaging has a replay and reconsume commands since version 3.7
http get https://apps.fedoraproject.org./datagrepper/v2/search \
topic==<topic> delta==<delta>
Example command for search of messages in topic org.fedoraproject.prod.fas.group.member.removed sent in the last day:
Example command for search of messages in topic org.fedoraproject.prod.ipa.group_remove_member.v1 sent in the last day:
::
http get https://apps.fedoraproject.org/datagrepper/v2/search \
topic==org.fedoraproject.prod.fas.groups.member.removed delta==86400
topic==org.fedoraproject.prod.ipa.group_remove_member.v1 delta==86400
- Get the message ID and supply it to the fedora-messaging reconsume command, with local config:

4
docs/pagure_fas_groups_sync.md
View file

@ -14,5 +14,5 @@ and remove/add users to pagure group based on the changes in FAS.
The sync toddler accepts following topics.
* org.fedoraproject.*.fas.group.member.sponsor - New member added to group
* org.fedoraproject.*.toddlers.trigger.pagure_fas_groups_sync - Message triggered by toddlers cron
* `org.fedoraproject.*.ipa.group_add_member.v1` - New member added to group
* `org.fedoraproject.*.toddlers.trigger.pagure_fas_groups_sync` - Message triggered by toddlers cron

32
poetry.lock generated
View file

@ -1,4 +1,4 @@
# This file is automatically @generated by Poetry 1.8.5 and should not be changed by hand.
# This file is automatically @generated by Poetry 1.8.3 and should not be changed by hand.
[[package]]
name = "arrow"
@ -1019,6 +1019,20 @@ MarkupSafe = ">=2.0"
[package.extras]
i18n = ["Babel (>=2.7)"]
[[package]]
name = "journal-to-fedora-messaging-messages"
version = "1.0.1"
description = "A schema package for messages sent by Journal to Fedora Messaging"
optional = false
python-versions = "<4.0,>=3.9"
files = [
{file = "journal_to_fedora_messaging_messages-1.0.1-py3-none-any.whl", hash = "sha256:6eb7b01054b89bd304d16105066e47a15f6627ef022c56f4c3d05e48519a7ecf"},
{file = "journal_to_fedora_messaging_messages-1.0.1.tar.gz", hash = "sha256:e9cc8295095fadf19285bb0b9439dca2db06dda2bf64fc96f521fa52f85c9d57"},
]
[package.dependencies]
fedora-messaging = ">=3.3.0,<4.0.0"
[[package]]
name = "jsonpointer"
version = "3.0.0"
@ -1409,20 +1423,6 @@ files = [
[package.extras]
nicer-shell = ["ipython"]
[[package]]
name = "noggin-messages"
version = "1.1.0"
description = "Fedora Messaging message schemas for Noggin."
optional = false
python-versions = "<4.0.0,>=3.8.0"
files = [
{file = "noggin_messages-1.1.0-py3-none-any.whl", hash = "sha256:c6d049ade672eac0aa81a1dba3f425235af03730a1a603dc728878b35833fdc3"},
{file = "noggin_messages-1.1.0.tar.gz", hash = "sha256:78ce070a0c7b89824eef97ee79773681832bfc1b2eb7737845d7eb0d97dec730"},
]
[package.dependencies]
fedora-messaging = ">=2.0.1"
[[package]]
name = "openidc-client"
version = "0.6.0"
@ -2802,4 +2802,4 @@ cffi = ["cffi (>=1.11)"]
[metadata]
lock-version = "2.0"
python-versions = "^3.11"
content-hash = "8048917cc41a98dbe75498694aff79dc8163adc6c309cad211ffc8840b6990e9"
content-hash = "f119d99c27f1863ecd18c636eea8a13305fd5ebfae74d6e883a69560cbf4c8d5"

2
pyproject.toml
View file

@ -18,7 +18,6 @@ fedora-messaging-git-hook-messages = "^1.0.1"
gitpython = "^3.1.43"
koji = "^1.34.2"
requests = "^2.32.3"
noggin-messages = "^1.1.0"
pagure-messages = "^1.2.0"
pygobject = "^3.48.2"
python-fedora = "^1.1.1"
@ -30,6 +29,7 @@ pylibmc = "^1.6.3"
ipalib = "^4.12.2"
cryptography = "<44.0.0"
ipaclient = "^4.12.2"
journal-to-fedora-messaging-messages = "^1.0.1"
[tool.poetry.group.dev.dependencies]
pytest = "*"

166
tests/plugins/test_cleaning_packager_group.py
View file

@ -5,12 +5,37 @@ Unit tests for `toddler.plugins.cleaning_packager_groups`
import logging
from unittest.mock import MagicMock, patch
from journal_to_fedora_messaging_messages.ipa import IpaGroupRemoveMemberV1
import pytest
from toddlers.exceptions import PagureError
import toddlers.plugins.cleaning_packager_groups as cleaning_packager_groups
@pytest.fixture
def group_remove_member_message():
return IpaGroupRemoveMemberV1(
{
"IPA_API_PARAMS": (
'{"cn": "packager", "all": false, "raw": false, "version": "2.254", '
'"no_members": false, "user": ["lenkaseg"]}'
),
"__REALTIME_TIMESTAMP": "1742838354523018",
"IPA_API_ACTOR": "agent@EXAMPLE.COM",
"IPA_API_COMMAND": "group_remove_member",
"IPA_API_RESULT": "SUCCESS",
"MESSAGE": (
"[IPA.API] agent@EXAMPLE.COM: group_remove_member: SUCCESS [ldap2_139734790206096] "
'{"cn": "packager", "all": false, "raw": false, "version": "2.254", '
'"no_members": false, "user": ["lenkaseg"]}'
),
"_HOSTNAME": "ipa.example.com",
"_COMM": "httpd",
"PRIORITY": "5",
}
)
class TestAcceptTopic:
"""
Test class for `toddler.plugins.cleaning_packager_groups.CleanPackagerGroups.accepts_topic`
@ -28,9 +53,9 @@ class TestAcceptTopic:
@pytest.mark.parametrize(
"topic",
[
"org.fedoraproject.*.fas.group.member.removed",
"org.fedoraproject.prod.fas.group.member.removed",
"org.fedoraproject.stg.fas.group.member.removed",
"org.fedoraproject.*.ipa.group_remove_member.v1",
"org.fedoraproject.prod.ipa.group_remove_member.v1",
"org.fedoraproject.stg.ipa.group_remove_member.v1",
],
)
def test_accepts_topic_valid(self, topic, toddler):
@ -50,35 +75,10 @@ class TestProcess:
self.toddler_cls = cleaning_packager_groups.CleanPackagerGroups()
self.toddler_cls._ipa_session = MagicMock()
def test_process(self):
"""
Assert that process is called correctly.
"""
config = {"pagure": {"pagure.prod.url"}, "pagure_stg": {"pagure.stg.url"}}
message = MagicMock()
message.body = {
"msg": {"agent": "lenkaseg", "group": "packager", "user": "patrikp"},
"headers": {
"fedora_messaging_group_communishift-admins": True,
"fedora_messaging_schema": "noggin.group.member.sponsor.v1",
"fedora_messaging_severity": 20,
"fedora_messaging_user_lenkaseg": True,
"fedora_messaging_user_patrikp": True,
"priority": 0,
"sent-at": "2024-07-04T15:48:25+00:00",
},
"id": "35d54838-b12c-49d5-949c-c503a71e7ae5",
"priority": 0,
"queue": "null",
"topic": "org.fedoraproject.stg.fas.group.member.remove",
}
self.toddler_cls.process = MagicMock()
self.toddler_cls.process(config, message)
self.toddler_cls.process.assert_called_once_with(config, message)
@patch("toddlers.utils.pagure.set_pagure")
def test_not_packager_group(self, mock_set_pagure, caplog):
def test_not_packager_group(
self, mock_set_pagure, group_remove_member_message, caplog
):
"""
Assert that if the group in the message is not "packager", plugin stops.
"""
@ -90,58 +90,74 @@ class TestProcess:
"pagure_stg": "https://stg.pagure.io",
"watched_groups": ["packager"],
}
message = MagicMock()
message.body = {
"msg": {
"group": "not-packager",
"user": "lenkaseg",
},
"topic": "org.fedoraproject.stg.fas.group.member.remove",
}
self.toddler_cls.process(config, message)
group_remove_member_message.body["IPA_API_PARAMS"] = (
'{"cn": "not-packager", "all": false, "raw": false, "version": "2.254", '
'"no_members": false, "user": ["lenkaseg"]}'
)
self.toddler_cls.process(config, group_remove_member_message)
assert (
caplog.records[-1].message
== "User lenkaseg was not removed from packager group, but "
== "User(s) lenkaseg was not removed from packager group, but "
"from not-packager group, nothing to do."
)
@pytest.mark.parametrize(
"distgit_groups,ipa_groups,message_1,message_2,message_3,message_4",
"distgit_groups,ipa_groups,messages",
(
(
["group1", "group2"],
{"result": {"memberof_group": ["group1", "group3"]}},
"User lenkaseg removed from distgit group: group1",
"User lenkaseg removed from ipa group: group1",
"User lenkaseg should be removed from following groups: ['group1']",
"Ipa groups: ['group1', 'group3']",
(
"Fetching all distgit groups:",
"Distgit groups found: ['group1', 'group2']",
"User lenkaseg was removed from packager group, removing from "
"packager-related groups as well.",
"User lenkaseg removed from distgit group: packager",
"Fetching groups user lenkaseg is member of in ipa:",
"Ipa groups: ['group1', 'group3']",
"User lenkaseg should be removed from following groups: ['group1']",
"User lenkaseg removed from ipa group: group1",
"User lenkaseg removed from distgit group: group1",
),
),
(
None,
{"result": {"memberof_group": ["group1", "group3"]}},
"No distgit groups found, bailing.",
"Distgit groups found: None",
"Fetching all distgit groups:",
"User lenkaseg was removed from packager group, removing from "
"packager-related groups as well.",
(
"Fetching all distgit groups:",
"Distgit groups found: None",
"No distgit groups found, bailing.",
),
),
(
["group1", "group2"],
{"result": {"memberof_group": ["group3", "group4"]}},
"Seems that user lenkaseg is not member of any groups shared "
"between ipa and distgit",
"Ipa groups: ['group3', 'group4']",
"Fetching groups user lenkaseg is member of in ipa:",
"User lenkaseg removed from distgit group: packager",
(
"Fetching all distgit groups:",
"Distgit groups found: ['group1', 'group2']",
"User lenkaseg was removed from packager group, removing from "
"packager-related groups as well.",
"User lenkaseg removed from distgit group: packager",
"Fetching groups user lenkaseg is member of in ipa:",
"Ipa groups: ['group3', 'group4']",
"Seems that user lenkaseg is not member of any groups shared "
"between ipa and distgit",
),
),
(
["group1", "group2"],
{"result": {"memberof_group": []}},
"Seems that user lenkaseg is not member of any groups "
"shared between ipa and distgit",
"Ipa groups: []",
"Fetching groups user lenkaseg is member of in ipa:",
"User lenkaseg removed from distgit group: packager",
(
"Fetching all distgit groups:",
"Distgit groups found: ['group1', 'group2']",
"User lenkaseg was removed from packager group, removing from "
"packager-related groups as well.",
"User lenkaseg removed from distgit group: packager",
"Fetching groups user lenkaseg is member of in ipa:",
"Ipa groups: []",
"Seems that user lenkaseg is not member of any groups shared "
"between ipa and distgit",
),
),
),
ids=(
@ -157,10 +173,8 @@ class TestProcess:
mock_pagure,
distgit_groups,
ipa_groups,
message_1,
message_2,
message_3,
message_4,
messages,
group_remove_member_message,
caplog,
):
"""
@ -178,16 +192,10 @@ class TestProcess:
"pagure_stg": "https://example.stg.io",
"watched_groups": ["packager"],
}
message = MagicMock()
message.body = {
"msg": {"group": "packager", "user": "lenkaseg", "agent": "agent"},
"topic": "org.fedoraproject.stg.fas.group.member.remove",
}
self.toddler_cls.process(config, message)
assert caplog.records[-1].message == message_1
assert caplog.records[-2].message == message_2
assert caplog.records[-3].message == message_3
assert caplog.records[-4].message == message_4
self.toddler_cls.process(config, group_remove_member_message)
recorded_messages = tuple(r.message for r in caplog.records)
print(recorded_messages)
assert recorded_messages == messages
@pytest.mark.parametrize(
"ipa_remove_member,error,message_1,message_2",
@ -215,6 +223,7 @@ class TestProcess:
error,
message_1,
message_2,
group_remove_member_message,
caplog,
):
"""
@ -238,13 +247,8 @@ class TestProcess:
"pagure_api_key": "pagure_key",
"watched_groups": ["packager"],
}
message = MagicMock()
message.body = {
"msg": {"group": "packager", "user": "lenkaseg", "agent": "agent"},
"topic": "org.fedoraproject.stg.fas.group.member.remove",
}
self.toddler_cls.process(config, message)
self.toddler_cls.process(config, group_remove_member_message)
self.toddler_cls._ipa_session.Command.user_show.assert_called_with(
uid="lenkaseg"
)

73
tests/plugins/test_pagure_fas_groups_sync.py
View file

@ -6,13 +6,37 @@ import logging
from unittest.mock import Mock, patch
from fedora_messaging.api import Message
from noggin_messages import MemberSponsorV1
from journal_to_fedora_messaging_messages.ipa import IpaGroupAddMemberV1
import pytest
from toddlers.exceptions import PagureError
from toddlers.plugins import pagure_fas_groups_sync
@pytest.fixture
def group_add_member_message():
return IpaGroupAddMemberV1(
{
"IPA_API_PARAMS": (
'{"cn": "fas_group", "all": false, "raw": false, "version": "2.254", '
'"no_members": false, "user": ["user"]}'
),
"__REALTIME_TIMESTAMP": "1742838354523018",
"IPA_API_ACTOR": "agent@EXAMPLE.COM",
"IPA_API_COMMAND": "group_add_member",
"IPA_API_RESULT": "SUCCESS",
"MESSAGE": (
"[IPA.API] agent@EXAMPLE.COM: group_add_member: SUCCESS [ldap2_139734790206096] "
'{"cn": "fas_group", "all": false, "raw": false, "version": "2.254", '
'"no_members": false, "user": ["user"]}'
),
"_HOSTNAME": "ipa.example.com",
"_COMM": "httpd",
"PRIORITY": "5",
}
)
class TestAcceptsTopic:
"""
Test class for `toddlers.plugins.pagure_fas_groups_sync.PagureFASGroupsSync.accepts_topic`
@ -30,11 +54,11 @@ class TestAcceptsTopic:
@pytest.mark.parametrize(
"topic",
[
"org.fedoraproject.*.fas.group.member.sponsor",
"org.fedoraproject.*.ipa.group_add_member.v1",
"org.fedoraproject.*.toddlers.trigger.pagure_fas_groups_sync",
"org.fedoraproject.stg.fas.group.member.sponsor",
"org.fedoraproject.stg.ipa.group_add_member.v1",
"org.fedoraproject.stg.toddlers.trigger.pagure_fas_groups_sync",
"org.fedoraproject.prod.fas.group.member.sponsor",
"org.fedoraproject.prod.ipa.group_add_member.v1",
"org.fedoraproject.prod.toddlers.trigger.pagure_fas_groups_sync",
],
)
@ -77,7 +101,9 @@ class TestProcess:
@patch("toddlers.utils.pagure.set_pagure")
@patch("toddlers.utils.fedora_account.set_fasjson")
def test_process_sponsor_message(self, mock_fas, mock_pagure, toddler):
def test_process_sponsor_message(
self, mock_fas, mock_pagure, toddler, group_add_member_message
):
"""
Assert that sponsor message is processed correctly.
"""
@ -85,12 +111,9 @@ class TestProcess:
mock_pagure_obj = Mock()
mock_pagure.return_value = mock_pagure_obj
config = {"group_map": {"fas_group": "pagure_group"}}
msg = MemberSponsorV1(
{"msg": {"agent": "agent", "user": "user", "group": "fas_group"}}
)
# Test
toddler.process(config, msg)
toddler.process(config, group_add_member_message)
# Assertions
mock_pagure_obj.add_member_to_group.assert_called_with("user", "pagure_group")
@ -101,7 +124,11 @@ class TestProcess:
@patch("toddlers.utils.pagure.set_pagure")
@patch("toddlers.utils.fedora_account.set_fasjson")
def test_process_sponsor_message_user_not_in_pagure(
self, mock_fas, mock_pagure, toddler
self,
mock_fas,
mock_pagure,
toddler,
group_add_member_message,
):
"""
Assert that sponsor message is processed correctly when user
@ -112,12 +139,9 @@ class TestProcess:
mock_pagure_obj.user_exists.return_value = False
mock_pagure.return_value = mock_pagure_obj
config = {"group_map": {"fas_group": "pagure_group"}}
msg = MemberSponsorV1(
{"msg": {"agent": "agent", "user": "user", "group": "fas_group"}}
)
# Test
toddler.process(config, msg)
toddler.process(config, group_add_member_message)
# Assertions
mock_pagure_obj.add_member_to_group.assert_not_called()
@ -128,7 +152,12 @@ class TestProcess:
@patch("toddlers.utils.pagure.set_pagure")
@patch("toddlers.utils.fedora_account.set_fasjson")
def test_process_sponsor_message_failure(
self, mock_fas, mock_pagure, caplog, toddler
self,
mock_fas,
mock_pagure,
caplog,
toddler,
group_add_member_message,
):
"""
Assert that failure during processing sponsor message is handled correctly.
@ -139,12 +168,9 @@ class TestProcess:
mock_pagure_obj.add_member_to_group.side_effect = PagureError("PagureError")
mock_pagure.return_value = mock_pagure_obj
config = {"group_map": {"fas_group": "pagure_group"}}
msg = MemberSponsorV1(
{"msg": {"agent": "agent", "user": "user", "group": "fas_group"}}
)
# Test
toddler.process(config, msg)
toddler.process(config, group_add_member_message)
# Assertions
mock_pagure_obj.add_member_to_group.assert_called_with("user", "pagure_group")
@ -157,7 +183,7 @@ class TestProcess:
@patch("toddlers.utils.pagure.set_pagure")
@patch("toddlers.utils.fedora_account.set_fasjson")
def test_process_sponsor_message_skipping(
self, mock_fas, mock_pagure, caplog, toddler
self, mock_fas, mock_pagure, caplog, toddler, group_add_member_message
):
"""
Assert that group is skipped when we don't care about it.
@ -167,12 +193,13 @@ class TestProcess:
mock_pagure_obj = Mock()
mock_pagure.return_value = mock_pagure_obj
config = {"group_map": {"fas_group": "pagure_group"}}
msg = MemberSponsorV1(
{"msg": {"agent": "agent", "user": "user", "group": "some_group"}}
group_add_member_message.body["IPA_API_PARAMS"] = (
'{"cn": "some_group", "all": false, "raw": false, "version": "2.254", '
'"no_members": false, "user": ["user"]}'
)
# Test
toddler.process(config, msg)
toddler.process(config, group_add_member_message)
# Assertions
mock_pagure_obj.add_member_to_group.assert_not_called()

29
toddlers/plugins/cleaning_packager_groups.py
View file

@ -1,6 +1,6 @@
"""
The script is triggered by fedora-messaging messages published under the topic
``toddlers.trigger.fas.group.member.remove`` and checks if a user was removed
``ipa.group_remove_member.v1`` and checks if a user was removed
from group "packagers". If yes, user is removed from packager related groups
as well.
"""
@ -22,7 +22,7 @@ class CleanPackagerGroups(ToddlerBase):
name: str = "clean_packagers_groups"
amqp_topics: list[str] = [
"org.fedoraproject.*.fas.group.member.removed",
"org.fedoraproject.*.ipa.group_remove_member.v1",
]
pagure: pagure.Pagure
@ -37,7 +37,7 @@ class CleanPackagerGroups(ToddlerBase):
from this specific topic.
"""
return topic.startswith("org.fedoraproject.") and topic.endswith(
"fas.group.member.removed"
"ipa.group_remove_member.v1"
)
def _get_ipa_session(self, config): # pragma: no cover
@ -58,24 +58,17 @@ class CleanPackagerGroups(ToddlerBase):
def process(self, config, message):
"""Process a given message. Remove user that is being removed from
a packager group from all groups in distgit"""
msg = message.body
user = msg["msg"]["user"]
group = msg["msg"]["group"]
group = message.group
watched_groups = config.get("watched_groups")
# If user is not removed from packager group, bail
if group not in watched_groups:
_log.info(
f"User {user} was not removed from packager group, "
f"User(s) {', '.join(message.user_names)} was not removed from packager group, "
f"but from {group} group, nothing to do."
)
return
_log.info(
f"User {user} was removed from {group} group, removing from "
"packager-related groups as well."
)
self.dist_git = pagure.set_pagure(
{
"pagure_url": config.get("dist_git_url"),
@ -90,6 +83,17 @@ class CleanPackagerGroups(ToddlerBase):
_log.info("No distgit groups found, bailing.")
return
ipa_session = self._get_ipa_session(config)
for user in message.user_names:
self._process_removal(config, user, group, distgit_groups, ipa_session)
def _process_removal(self, config, user, group, distgit_groups, ipa_session):
_log.info(
f"User {user} was removed from {group} group, removing from "
"packager-related groups as well."
)
# Remove user from the distgit group that triggered the toddler
try:
self.dist_git.remove_member_from_group(user, group)
@ -99,7 +103,6 @@ class CleanPackagerGroups(ToddlerBase):
f"Error while removing user {user} from distgit group {group}"
)
ipa_session = self._get_ipa_session(config)
_log.info(f"Fetching groups user {user} is member of in ipa:")
ipa_user = ipa_session.Command.user_show(uid=user)
ipa_groups = ipa_user["result"]["memberof_group"]

19
toddlers/plugins/pagure_fas_groups_sync.py
View file

@ -12,7 +12,6 @@ import sys
from typing import Dict
from fedora_messaging.api import Message
from noggin_messages import MemberSponsorV1
from toddlers.base import ToddlerBase
from toddlers.exceptions import PagureError
@ -32,7 +31,7 @@ class PagureFASGroupsSync(ToddlerBase):
name: str = "pagure_fas_groups_sync"
amqp_topics: list = [
"org.fedoraproject.*.fas.group.member.sponsor",
"org.fedoraproject.*.ipa.group_add_member.v1",
"org.fedoraproject.*.toddlers.trigger.pagure_fas_groups_sync",
]
@ -51,7 +50,7 @@ class PagureFASGroupsSync(ToddlerBase):
:returns: True if topic is accepted, False otherwise.
"""
if topic.startswith("org.fedoraproject."):
if topic.endswith("fas.group.member.sponsor"):
if topic.endswith("ipa.group_add_member.v1"):
return True
if topic.endswith("toddlers.trigger.pagure_fas_groups_sync"):
return True
@ -81,14 +80,14 @@ class PagureFASGroupsSync(ToddlerBase):
for group in group_map:
self.sync_group(group, group_map[group])
if topic.endswith("fas.group.member.sponsor"):
member_sponsor_message = MemberSponsorV1(message.body)
user = member_sponsor_message.user_name
for group in member_sponsor_message.groups:
if group in group_map:
if topic.endswith("ipa.group_add_member.v1"):
for user in message.user_names:
if message.group in group_map:
try:
if self.pagure.user_exists(user):
self.pagure.add_member_to_group(user, group_map[group])
self.pagure.add_member_to_group(
user, group_map[message.group]
)
except PagureError as e:
_log.exception(str(e))
else:
@ -97,7 +96,7 @@ class PagureFASGroupsSync(ToddlerBase):
"but we don't care about this group. "
"Skipping.",
user,
group,
message.group,
)
def sync_group(self, fas_group: str, pagure_group: str):