[jakduch]

Summary

• remove the temporary duplicate permissions health check role
• stop running the check for Katello and Satellite flavors
• remove the obsolete check documentation

Merge timing

Foreman #11271 must be released, and the supported upgrade path must no longer need the safeguard, before this can be merged. This remains a merge prerequisite even though the pull request is open for review.

Testing

• parsed both modified flavor files as YAML
• ran ansible-lint on both modified flavor files
• verified no check_duplicate_permissions references remain
• ran git diff --check

Fixes #849

AI assistance: Codex. Project signature: GPT_5.6 Sol.

[coderabbitai]

No actionable comments were generated in the recent review.

Recent review info

Run configuration

• Source mode: Auto
• Repository: Unavailable
• Branch: Unavailable
• File: No configuration file from this repository applied
• Commit: Unavailable
• Layers: Repository UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: 9aa15759-2848-47f5-ade0-bbd580b1818b

Commits

Reviewing files that changed from the base of the PR and between bda4c5b and 3d1c8a8.

Files selected for processing: 4

• docs/developer/checks.md
• src/roles/check_duplicate_permissions/tasks/main.yaml
• src/vars/flavors/katello.yml
• src/vars/flavors/satellite.yml

Files with no reviewable changes: 1

• src/roles/check_duplicate_permissions/tasks/main.yaml

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

Walkthrough

The duplicate-permissions check was removed. Its task, entries in the Katello and Satellite health-check lists, and developer documentation were deleted.

Changes: Duplicate permissions check removal

Layer / File(s)	Summary
Remove the check and its references src/roles/check_duplicate_permissions/tasks/main.yaml, src/vars/flavors/katello.yml, src/vars/flavors/satellite.yml, docs/developer/checks.md	The check task was deleted. Its entries were removed from both flavor health-check lists, and its developer documentation was removed.

Priority: Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Other

Suggested reviewers: evgeni, qcjames53, arvind4501

Merge Risk: Minimal · up to 3d1c8

The removal is internally consistent. Before merging, confirm that the permission-deduplication migration has shipped throughout the supported upgrade path, as required by the PR.

Pre-merge checks: 5

Check name	Status	Explanation
Title check		The title "Remove duplicate permissions check" matches the main change. The PR removes the check_duplicate_permissions role, its entries in the Katello and Satellite flavor check lists, and its docume…
Description check		The description lists the removed health check role, the flavor changes for Katello and Satellite, and the removed check documentation. It also gives testing steps and a merge prerequisite tied to For…
Linked Issues check		Directly linked issue #849 has three coding requirements. (1) Remove the check_duplicate_permissions role: the diff deletes src/roles/check_duplicate_permissions/tasks/main.yaml (-26 lines). (2) Remov…
Out of Scope Changes check		The whole-PR diff against the merge base changes three files and deletes one role. Every change maps to #849 scope: the role task file deletion, the two flavor check-list entry removals, and the devel…
Docstring Coverage		No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Documentation-only edits and deletions within survi…

Full details: Title check

Explanation

The title "Remove duplicate permissions check" matches the main change. The PR removes the check_duplicate_permissions role, its entries in the Katello and Satellite flavor check lists, and its documentation. The title is short, specific, and states the primary change.

Full details: Description check

Explanation

The description lists the removed health check role, the flavor changes for Katello and Satellite, and the removed check documentation. It also gives testing steps and a merge prerequisite tied to Foreman #11271. It is directly related to the changeset.

Full details: Linked Issues check

Explanation

Directly linked issue #849 has three coding requirements. (1) Remove the check_duplicate_permissions role: the diff deletes src/roles/check_duplicate_permissions/tasks/main.yaml (-26 lines). (2) Remove the role from the flavor check lists: the diff removes check_duplicate_permissions from health_checks_to_execute in src/vars/flavors/katello.yml and src/vars/flavors/satellite.yml. (3) Remove the developer documentation: the diff removes the check_duplicate_permissions entry from docs/developer/checks.md. The role had no test directory at the merge base. src/roles/check_duplicate_permissions contained only tasks/main.yaml, and tests/unit/check_role_test.py at the head has no test for this role. The issue requirement to remove tests therefore has no coding work in this diff. The issue states that the removal must not land before Foreman #38465 / PR #11271 ships in the required upgrade path. The PR description records this as a merge prerequisite. That gate depends on an external release and is not a coding requirement, so this assessment does not evaluate it. Limit of this check: the file listing and the diff do not provide a full-tree text search. The absence of other references to check_duplicate_permissions rests on the author's reported verification and on the listed paths.

Full details: Out of Scope Changes check

Explanation

The whole-PR diff against the merge base changes three files and deletes one role. Every change maps to #849 scope: the role task file deletion, the two flavor check-list entry removals, and the developer documentation entry removal in docs/developer/checks.md. The diff contains no unrelated code, configuration, or dependency changes. The remaining check_features documentation entry is unchanged, as expected.

Full details: Docstring Coverage

Explanation

No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 0 functions across 0 files.

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[jakduch]

The failing deployment jobs hit the Candlepin heap/OOM problem fixed on current master by #865. I rebased this branch onto that fix; CI is running again.

[jakduch]

The remaining exact-head failure is the CentOS Stream 10 fapolicyd content-proxy job. The VM lost its running containers and fapolicyd then denied /bin/bash; this is the shared under-sized proxy failure fixed by PR #884. Its equivalent matrix job is fully green. I will rebase after that fix lands.

[jakduch]

The failed matrix never reached the PR changes: Vagrant Cloud returned HTTP 502 while downloading box metadata for the test machines. I retriggered the full CI on 9575c84.

[jakduch]

Rebased on current master in 3d1c8a8, including the merged VM memory fix #884. Python and Ansible lint are green in the new run; deployment tests are still running.

The merge condition in #849 still applies: the permission-uniqueness migration must be present in the supported upgrade path before removing this safeguard.