Repository navigation
NO-ISSUE: Let ci-duty bypass branch protections with its role - #280
Conversation
The force-merge checkbox of a pull request ("Merge without waiting for
requirements to be met") is offered only to someone who can bypass every
rule on the branch. ci-duty is already a bypass actor of the rulesets and
of the review requirements, but its repository role, ci-maintainer, had no
permission to bypass branch protections, so a pull request ejected from
the merge queue showed the team only "Merge when ready".
Add bypass_branch_protection to the ci-maintainer role (osac and
osac-test-infra) and say so in its description and in the README.
Signed-off-by: Elior Erez <eerez@redhat.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
WalkthroughThe ChangesCI Maintainer Role
Priority: ➖ Normal Estimated code review effort: 1 (Trivial) | ~4 minutes Change: Feature Suggested labels: Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation |
|---|---|---|
| Description Check |  |
Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check | |
The title names the main change: adding branch-protection bypass to the ci-maintainer role for the ci-duty team. The raw summary confirms the README and organization.tf changes add bypass_branch_prote… |
| Docstring Coverage | |
No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0… |
| Linked Issues check | |
Check skipped because no linked issues were found for this pull request. |
| Out of Scope Changes check | |
Check skipped because no linked issues were found for this pull request. |
| No-Hardcoded-Secrets | |
The pull request changes only README.md and organization.tf. Added content contains permission names, documentation, and the bypass_branch_protection literal. It adds no API key, token value, pa… |
| No-Weak-Crypto | |
The pull request changes only README.md and organization.tf. The diff adds the GitHub permission bypass_branch_protection and updates documentation. It introduces no MD5, SHA1, DES, RC4, 3DES, Blowf… |
| No-Injection-Vectors | |
PASS. The pull request changes only static README text and Terraform role metadata. The added bypass_branch_protection permission and descriptions contain no SQL concatenation, shell execution, eval… |
| Container-Privileges | |
PASS: The pull request changes only README.md and organization.tf. It adds a GitHub repository-role permission and documentation. It does not modify container or Kubernetes manifests, and it intro… |
| No-Sensitive-Data-In-Logs | |
The pull request changes only README.md and organization.tf. The diff adds a repository-role permission, descriptions, and comments. It adds no logging statements or sensitive values such as passwords… |
| Ai-Attribution | |
No AI tool use is mentioned in the authored PR description or commit message. The only commit trailer is Signed-off-by; no Co-Authored-By, Assisted-by, or Generated-by trailer is present. The “AI-gene… |
Full details: Title check
-
Explanation
The title names the main change: adding branch-protection bypass to the ci-maintainer role for the ci-duty team. The raw summary confirms the README and organization.tf changes add bypass_branch_protection to that role. The title is short and specific. The 'NO-ISSUE:' prefix is a tracking tag and does not affect the assessment.
Full details: Docstring Coverage
-
Explanation
No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0 files. (2 skipped: 2 unsupported.)
Full details: No-Hardcoded-Secrets
-
Explanation
The pull request changes only
README.mdandorganization.tf. Added content contains permission names, documentation, and thebypass_branch_protectionliteral. It adds no API key, token value, password, private key, credential-bearing URL, or long base64/hex string.
Full details: No-Weak-Crypto
-
Explanation
The pull request changes only README.md and organization.tf. The diff adds the GitHub permission
bypass_branch_protectionand updates documentation. It introduces no MD5, SHA1, DES, RC4, 3DES, Blowfish, ECB usage, custom cryptography, or secret/token comparisons.
Full details: No-Injection-Vectors
-
Explanation
PASS. The pull request changes only static README text and Terraform role metadata. The added
bypass_branch_protectionpermission and descriptions contain no SQL concatenation, shell execution, eval/exec, unsafe deserialization, unsafe YAML loading, or HTML injection.
Full details: Container-Privileges
-
Explanation
PASS: The pull request changes only
README.mdandorganization.tf. It adds a GitHub repository-role permission and documentation. It does not modify container or Kubernetes manifests, and it introduces none of the listed privileged settings.
Full details: No-Sensitive-Data-In-Logs
-
Explanation
The pull request changes only README.md and organization.tf. The diff adds a repository-role permission, descriptions, and comments. It adds no logging statements or sensitive values such as passwords, tokens, API keys, PII, session IDs, hostnames, or customer data.
Full details: Ai-Attribution
-
Explanation
No AI tool use is mentioned in the authored PR description or commit message. The only commit trailer is Signed-off-by; no Co-Authored-By, Assisted-by, or Generated-by trailer is present. The “AI-generated summary” label is review metadata, not contributor attribution.
✨ Finishing Touches
-
🧪 Generate unit tests (beta)
-
- Create a new PR
-
Comment @coderabbitai help to get the list of available commands.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
The
ci-maintainerrepository role of theci-dutyteam (onosacandosac-test-infra) gets the permissionbypass_branch_protection("Bypass branch protections").Why: the force-merge checkbox of a pull request ("Merge without waiting for requirements to be met") is offered only to someone who can bypass every rule on the branch.
ci-dutyalready bypasses the rulesets (ruleset_bypass_team_ids) and, onosac, the review requirements (pull_request_bypassers), but the role itself had no bypass permission. People of the team reported that a pull request that was ejected from the merge queue shows them only "Merge when ready" and no force merge, while one that never was queued shows the checkbox.What this changes: one entry in the permissions of
github_organization_repository_role.ci_maintainerand its description and comment, and a line in the README. Nothing else. After the apply, the role has the seven permissions it has in this repository (the Actions secrets and variables ones among them) plus the new one.Note: the permission was added by hand in the organization settings for a test, which also left the live role without the Actions secrets and variables permissions. The next apply puts both right, so please merge this before anyone edits the role again in the settings, or the apply will remove the manual bypass permission again.
Not verified: that the checkbox shows for a member of the team on an ejected pull request once the role has the permission. I could only check, with an account of the team, that GitHub already reports the team as able to bypass the ruleset and to merge as admin on every pull request. Please check on an ejected pull request after the apply.
Summary
bypass_branch_protectionto theci-maintainerrepository role forosacandosac-test-infra. The role description and comments now document this permission alongside its existing repository and Actions permissions.ci-dutymembers bypass branch protection when merging, including access to GitHub’s “Merge without waiting for requirements to be met” checkbox. That behavior was not verified on an ejected pull request.Risk classification
The applied risk label and the criteria for
risk:ship,risk:show, orrisk:askwere not supplied. The repository search found no labeling criteria. The classification, including whether the change was close to another label, cannot be determined from the available evidence.