Skip to content

Commit 32fa764

Browse files
committed
fix(codeowners): cover the tests and the SBOM check
The gate's tests are part of its enforcement surface: weakening an assertion is the quietest way to let a later change to the gate pass unnoticed. verify-image-sbom.sh decides whether a published image may ship without an SBOM describing the Rust code inside it, so it belongs under the same ownership even though it gates releases, not merges. Regroups the block by what each rule protects and restates why the outside maintainers are spelled out alongside the team. Signed-off-by: Jim Meyer <jimeyer@nvidia.com>
1 parent 8c3d04e commit 32fa764

2 files changed

Lines changed: 21 additions & 15 deletions

File tree

‎.github/CODEOWNERS‎

Lines changed: 20 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,29 +1,35 @@
1-
# Broad ownership — core team reviews everything
1+
## Broad ownership — core team reviews everything
22
* @NVIDIA/openshell-codeowners @mrunalp @sjenning @derekwaynecarr
33

4-
# Vouch list — maintainers only (bot commits bypass, but manual edits need review)
4+
## Vouch list — maintainers only (bot commits bypass, but manual edits need review)
55
.github/VOUCHED.td @NVIDIA/openshell-codeowners
66

7-
# Merge enforcement — these paths decide who can merge, so they keep an explicit
8-
# owner independent of any broad ownership rule above.
9-
#
10-
# A workflow triggered by pull_request_review runs the pull request's copy of
11-
# the workflow file, not the one on main, so a contributor can rewrite the
12-
# approval gate in the same pull request the gate is meant to block. Requiring
13-
# a code owner here is what closes that: the tampered job still runs and still
14-
# reports green, but the pull request cannot merge without an approval GitHub
15-
# enforces natively.
7+
## Merge enforcement — these paths decide who can merge, so they notify the full
8+
# list of maintainers to raise scrutiny on changes here.
9+
#
10+
# PR checks run local to the branch, so unless the check specifically avoids
11+
# using branch-local files, a PR could change its own gates and appear to pass
12+
# them. Covering the required checks here closes that gap.
1613
#
1714
# The whole workflows directory is covered, not just the approval gate, because
1815
# any workflow can claim a required check context or ask for a broader token.
1916
#
20-
# The three logins are named alongside the team because they are outside
21-
# collaborators and cannot join it. Owning these paths by team alone would mean
22-
# no Red Hat maintainer could approve a change to the merge gate.
17+
# Normally, we'd use a GitHub team to cover all maintainers, but org-level
18+
# policies prevent outside collaborators from being on a GH team at this time
19+
# so we spell them out explicitly.
20+
21+
# Core check mechanisms
2322
/.github/workflows/ @NVIDIA/openshell-codeowners @mrunalp @sjenning @derekwaynecarr
2423
/.github/actions/ @NVIDIA/openshell-codeowners @mrunalp @sjenning @derekwaynecarr
2524
/.github/CODEOWNERS @NVIDIA/openshell-codeowners @mrunalp @sjenning @derekwaynecarr
2625
/.github/zizmor.yml @NVIDIA/openshell-codeowners @mrunalp @sjenning @derekwaynecarr
26+
27+
# `MAINTAINERS.md` maintainer approval checks
2728
/MAINTAINERS.md @NVIDIA/openshell-codeowners @mrunalp @sjenning @derekwaynecarr
2829
/tasks/scripts/check_maintainer_approval.py @NVIDIA/openshell-codeowners @mrunalp @sjenning @derekwaynecarr
2930
/tasks/scripts/alert_maintainer_change.py @NVIDIA/openshell-codeowners @mrunalp @sjenning @derekwaynecarr
31+
/tasks/scripts/check_maintainer_approval_test.py @NVIDIA/openshell-codeowners @mrunalp @sjenning @derekwaynecarr
32+
/tasks/scripts/alert_maintainer_change_test.py @NVIDIA/openshell-codeowners @mrunalp @sjenning @derekwaynecarr
33+
34+
# Auditable SBOM check
35+
/tasks/scripts/verify-image-sbom.sh @NVIDIA/openshell-codeowners @mrunalp @sjenning @derekwaynecarr

‎CONTRIBUTING.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ Use agents and the repository skills as needed to understand the affected code,
7070

7171
Every pull request must be approved by someone listed in [MAINTAINERS.md](MAINTAINERS.md) before it can merge. This is enforced by the `OpenShell / Maintainer Approval` status check, which turns green once one of those reviewers approves. Reviews from other contributors are welcome and count toward the general approval requirement, but they do not satisfy this check.
7272

73-
Pull requests that touch the enforcement machinery itself — anything under `.github/workflows/`, `.github/actions/`, `MAINTAINERS.md`, `.github/CODEOWNERS`, or the approval helpers in `tasks/scripts/` — additionally require a code owner's approval, listed in [.github/CODEOWNERS](.github/CODEOWNERS). The status check reads its inputs from `main`, but a workflow triggered by a review runs the pull request's own copy of the workflow file, so GitHub's native code owner requirement is what keeps a change from disabling the gate that would have blocked it.
73+
Pull requests that touch the enforcement machinery itself — anything under `.github/workflows/`, `.github/actions/`, `MAINTAINERS.md`, `.github/CODEOWNERS`, or the enforcement scripts in `tasks/scripts/` — additionally require a code owner's approval, listed in [.github/CODEOWNERS](.github/CODEOWNERS). The status check reads its inputs from `main`, but a workflow triggered by a review runs the pull request's own copy of the workflow file, so GitHub's native code owner requirement is what keeps a change from disabling the gate that would have blocked it.
7474

7575
Maintainers are not requested automatically. If your pull request has been idle, ask for a reviewer in the pull request or in the CNCF Slack channel rather than waiting.
7676

0 commit comments

Comments
 (0)