Skip to content

Commit 2d48088

Browse files
committed
fix: fbc bootstrap process
There have been couple of issues with the bootstraping of new FBC catalog versions: - The ISV repositories refused the PR because it wasn't created by project owner - now partner is asked to approve the PR - A commit was missing "Signed-off-by" - A script produced duplicated values in catalog mapping All these things have been fixed and 4.19 catalogs have been bootrstraped. JIRA: ISV-6057 Signed-off-by: Ales Raszka <araszka@redhat.com>
1 parent 1a8a5ac commit 2d48088

3 files changed

Lines changed: 115 additions & 20 deletions

File tree

operator-pipeline-images/operatorcert/entrypoints/check_permissions.py

Lines changed: 31 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,7 @@ def setup_argparser() -> argparse.ArgumentParser:
6868
return parser
6969

7070

71+
# pylint: disable=too-many-public-methods
7172
class OperatorReview:
7273
"""
7374
A class represents a pull request review and permissions check
@@ -244,7 +245,7 @@ def check_permissions(self) -> bool:
244245
# Members of the organization have permissions to submit a PR
245246
return True
246247
if self.is_partner():
247-
return self.check_permission_for_partner()
248+
return self.check_permission_for_partner(is_catalog_promotion_pr)
248249
if self.check_permission_for_community():
249250
return True
250251
# Enable PR approval on forked repository
@@ -350,13 +351,17 @@ def pr_owner_can_write(self) -> bool:
350351
return True
351352
return False
352353

353-
def check_permission_for_partner(self) -> bool:
354+
def check_permission_for_partner(self, is_catalog_promotion_pr: bool) -> bool:
354355
"""
355356
Check if the pull request owner has permissions to submit a PR for the for
356357
partner operator.
357358
A user has permissions to submit a PR if the user is listed in the certification
358359
project in Pyxis.
359360
361+
Args:
362+
is_catalog_promotion_pr (bool): A boolean value indicating if the pull request
363+
is a catalog promotion PR that needs to be reviewed by users
364+
360365
Raises:
361366
NoPermissionError: An exception raised when user does not have permissions
362367
to submit a PR
@@ -372,6 +377,12 @@ def check_permission_for_partner(self) -> bool:
372377
)
373378
container = project.get("container") or {}
374379
usernames = container.get("github_usernames") or []
380+
if is_catalog_promotion_pr:
381+
LOGGER.info(
382+
"Pull request is a catalog promotion PR. Asking for review from partners.."
383+
)
384+
self.request_review_from_partners(usernames)
385+
return False
375386
if self.pr_owner not in usernames:
376387
raise NoPermissionError(
377388
f"User {self.pr_owner} does not have permissions to submit a PR for "
@@ -465,6 +476,24 @@ def request_review_from_owners(self) -> None:
465476
["gh", "pr", "comment", self.pull_request_url, "--body", comment_text]
466477
)
467478

479+
def request_review_from_partners(self, reviewers: list[str]) -> None:
480+
"""
481+
Request review from the partner by adding a comment to the PR.
482+
"""
483+
reviewers_with_at = ", ".join(map(lambda x: f"@{x}", reviewers))
484+
comment_text = (
485+
"The author of the PR is not listed as one of the reviewers in certification project.\n"
486+
f"{reviewers_with_at}: please review the PR and approve it with an "
487+
"`/approve` comment.\n\n"
488+
"Consider adding the author of the PR to the list of reviewers in "
489+
"the certification project if you want automated merge without explicit "
490+
"approval."
491+
)
492+
493+
run_command(
494+
["gh", "pr", "comment", self.pull_request_url, "--body", comment_text]
495+
)
496+
468497

469498
def extract_operators_from_catalog(
470499
head_repo: OperatorRepo, catalog_operators: list[str]

operator-pipeline-images/tests/entrypoints/test_check_permissions.py

Lines changed: 56 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -340,39 +340,65 @@ def test_OperatorReview_pr_owner_can_write(
340340

341341

342342
@pytest.mark.parametrize(
343-
["project", "valid"],
343+
["project", "catalog_promotion_pr", "approved", "valid"],
344344
[
345-
pytest.param(None, False, id="no project"),
346-
pytest.param({}, False, id="no container"),
347-
pytest.param({"container": {}}, False, id="no github_usernames"),
345+
pytest.param(None, False, False, False, id="no project"),
346+
pytest.param({}, False, False, False, id="no container"),
347+
pytest.param({"container": {}}, False, False, False, id="no github_usernames"),
348348
pytest.param(
349-
{"container": {"github_usernames": None}}, False, id="no github_usernames"
349+
{"container": {"github_usernames": None}},
350+
False,
351+
False,
352+
False,
353+
id="no github_usernames",
350354
),
351355
pytest.param(
352356
{"container": {"github_usernames": ["user123"]}},
353357
False,
358+
False,
359+
False,
354360
id="user not in github_usernames",
355361
),
356362
pytest.param(
357363
{"container": {"github_usernames": ["owner"]}},
364+
False,
365+
True,
366+
True,
367+
id="user in github_usernames",
368+
),
369+
pytest.param(
370+
{"container": {"github_usernames": ["owner"]}},
371+
True,
372+
False,
358373
True,
359374
id="user in github_usernames",
360375
),
361376
],
362377
)
378+
@patch(
379+
"operatorcert.entrypoints.check_permissions.OperatorReview.request_review_from_partners"
380+
)
363381
@patch("operatorcert.entrypoints.check_permissions.pyxis.get_project")
364382
def test_OperatorReview_check_permission_for_partner(
365383
mock_pyxis_project: MagicMock,
384+
mock_request_review_from_partners: MagicMock,
366385
review_partner: check_permissions.OperatorReview,
367386
project: dict[str, Any],
387+
catalog_promotion_pr: bool,
388+
approved: bool,
368389
valid: bool,
369390
) -> None:
370391
mock_pyxis_project.return_value = project
371392
if valid:
372-
assert review_partner.check_permission_for_partner() == True
393+
assert (
394+
review_partner.check_permission_for_partner(catalog_promotion_pr)
395+
== approved
396+
)
397+
if catalog_promotion_pr:
398+
mock_request_review_from_partners.assert_called_once()
373399
else:
374400
with pytest.raises(check_permissions.NoPermissionError):
375-
review_partner.check_permission_for_partner()
401+
review_partner.check_permission_for_partner(catalog_promotion_pr)
376402

377403

378404
@pytest.mark.parametrize(
@@ -465,6 +491,29 @@ def test_OperatorReview_request_review_from_maintainers(
465491
)
466492

467493

494+
@patch("operatorcert.entrypoints.check_permissions.run_command")
495+
def test_OperatorReview_request_review_from_partners(
496+
mock_command: MagicMock,
497+
review_community: check_permissions.OperatorReview,
498+
) -> None:
499+
review_community.request_review_from_partners(["user1", "user2"])
500+
mock_command.assert_called_once_with(
501+
[
502+
"gh",
503+
"pr",
504+
"comment",
505+
review_community.pull_request_url,
506+
"--body",
507+
"The author of the PR is not listed as one of the reviewers in certification project.\n"
508+
f"@user1, @user2: please review the PR and approve it with an "
509+
"`/approve` comment.\n\n"
510+
"Consider adding the author of the PR to the list of reviewers in "
511+
"the certification project if you want automated merge without explicit "
512+
"approval.",
513+
]
514+
)
515+
516+
468517
@patch("operatorcert.entrypoints.check_permissions.run_command")
469518
def test_OperatorReview_request_review_from_owners(
470519
mock_command: MagicMock,

scripts/promote-catalog.py

Lines changed: 28 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -126,20 +126,26 @@ def add_catalog_to_ci_mapping(
126126
"""
127127
operator_ci_path = repo_dir / "operators" / operator_name / "ci.yaml"
128128
with open(operator_ci_path, "r") as ci_file:
129-
ci_content = YAML().load(ci_file)
129+
yaml = YAML()
130+
yaml.preserve_quotes = True
131+
132+
ci_content = yaml.load(ci_file)
130133

131134
# Append target version to the catalog mapping for
132135
# a same template as used for the source version
133136
mapping = ci_content.get("fbc", {}).get("catalog_mapping", {})
137+
all_catalog_from_mapping = [
138+
catalog for template in mapping for catalog in template.get("catalog_names", [])
139+
]
140+
if f"v{target_version}" in all_catalog_from_mapping:
141+
LOG.info(
142+
"Catalog %s already exists in the mapping for operator %s",
143+
target_version,
144+
operator_name,
145+
)
146+
return None
134147
for template in mapping:
135148
if f"v{source_version}" in template.get("catalog_names", []):
136-
if f"v{target_version}" in template.get("catalog_names", []):
137-
LOG.info(
138-
"Catalog %s already exists in the mapping for operator %s",
139-
target_version,
140-
operator_name,
141-
)
142-
return
143149
template["catalog_names"].append(f"v{target_version}")
144150
break
145151
else:
@@ -149,9 +155,12 @@ def add_catalog_to_ci_mapping(
149155
operator_name,
150156
)
151157
return None
158+
152159
with open(operator_ci_path, "w") as ci_file:
153160
yaml = YAML()
154161
yaml.explicit_start = True
162+
yaml.preserve_quotes = True
163+
yaml.indent(mapping=2, sequence=4, offset=2)
155164
yaml.dump(ci_content, ci_file)
156165
return operator_ci_path
157166

@@ -206,8 +215,16 @@ def commit_and_push(
206215
"""
207216
git_repo.index.add(files_to_commit)
208217
LOG.info("Committing and pushing changes to %s", head_branch)
209-
git_repo.index.commit(message)
210-
git_repo.remotes.origin.push(head_branch)
218+
219+
# Construct the Signed-off-by line
220+
name = git_repo.config_reader().get_value("user", "name")
221+
email = git_repo.config_reader().get_value("user", "email")
222+
223+
signed_off_by = f"Signed-off-by: {name} <{email}>"
224+
full_message = f"{message}\n\n{signed_off_by}"
225+
226+
git_repo.index.commit(full_message)
227+
git_repo.remotes.upstream.push(head_branch)
211228

212229

213230
def create_pr(
@@ -228,7 +245,7 @@ def create_pr(
228245
local_repo (str): Work directory
229246
"""
230247
gh_repo = ORGANIZATIONS[local_repo]["gh_repository"]
231-
LOG.info("Creating PR in %s for branch %s", gh_repo, head)
248+
LOG.info("Creating PR in %s for branch %s to %s", gh_repo, head, base)
232249
gh_api_url = f"https://api.github.com/repos/{gh_repo}/pulls"
233250
data = {"head": head, "base": base, "title": title, "body": body}
234251
return github.post(gh_api_url, data)

0 commit comments

Comments
 (0)