mirror of
https://github.com/cozystack/cozystack.git
synced 2026-11-03 02:27:01 +09:00
myasnikovdaniil
7d7eb1c23e
## What this PR does
CODEOWNERS applies only the last matching pattern and it replaces every
broader rule including `*`, it does not merge them. Half of what that
costs is already fixed: `bd7b76691` added @kvaps and @lllamnyp to the
rules that were missing them, and `hack/codeowners-invariant.bats` keeps
them there. That invariant reads "every rule with owners is a superset
of the `*` owners", and the other half of the problem satisfies it
without a complaint:
```
/dashboards/ @myasnikovdaniil @IvanHunters @kvaps @lllamnyp <- widened by bd7b76691, 4 owners
/api/ @kvaps @lllamnyp <- unchanged, nothing to add, still 2
```
`/api/` is exactly equal to the catch-all, so it passes the invariant
perfectly while gating on two people. Eleven rules on main have that
shape: `*`, `/api/`, the four `/pkg/` ones, both api-gate paths, the
api-review-gate workflow, `.github/CODEOWNERS` and `packages/apps/vpc`.
That is 182 files.
The catch-all is those same two handles, so the invariant makes them the
floor for the whole repository and every path without a rule of its own
sits on that floor too. Raising the floor means raising `*`, after which
the invariant carries the new set into every rule by itself. That is the
shape of this PR: `*` goes from two owners to five, and all fifty rules
carry those five.
Widening the owner sets is not a security relaxation, and it is worth
saying why explicitly. Write access does not change for anyone, all
twelve collaborators already have it, and CODEOWNERS never decided who
may change code, only whose approval unblocks a merge. Owner approval is
OR and always was, so one @kvaps approval already closed `/api/` on its
own and two pairs of eyes were never required there, what changes is the
size of the pool and not the number of approvals needed. What actually
holds those paths is the required checks, which are AND and which this
PR does not touch: DCO, pre-commit, codegen-drift, zizmor, scorecard,
e2e, and `api-review-gate.yaml` keeps its own `API_OWNERS: "lllamnyp
kvaps"`, so a sizeable API change still needs an approval from one of
them specifically. The AND gate is untouched, only the OR gate got
wider. And 111 and 56 reviews against 712 merged PRs over the six months
to 2026-09-06 is not strict control, it is a queue.
Three of the nine maintainers in MAINTAINERS.md own no path on main, and
the reason differs per person. @kingdonb was listed: `c13c6ee2c` gave
him `packages/system/fluxcd` and `packages/system/fluxcd-operator`, then
`7cca3d75e` deleted both packages with the FluxCD tenant addon, so the
rules went with them, correctly. Nobody moved him onto the flux paths
that stayed, `packages/core/flux-aio`, `packages/system/flux-plunger`,
`packages/system/flux-shard-operator` and the matching `cmd` and
`internal` dirs, so that one is a loose end from a breaking change and
not a decision about him. @matthieu-robin and @mattia-eleuteri were
never in the file, in any revision. Either way CONTRIBUTOR_LADDER.md
says a maintainer may approve changes to any area of the project, and
the file says otherwise.
Per package scoping also didn't match who actually reviews. In their own
areas the named reviewers turn out to be the authors: over the same six
months @androndo submitted 3 reviews across 85 storage PRs, @scooby87 1
across 22 virtualization ones, @sircthulhu 1 across 20 keycloak ones.
The reviews in all three areas came from the same few generalists that
the narrow rules were excluding.
So rules now group by area instead of by package (77 rules and 19
distinct owner sets become 50 and 9), every rule names at least five
owners, and an area whose owner set doesn't differ from its tier is left
to the tier rule. Networking, storage, databases and cluster-api lose
their per package rules for that reason, the tier rules already name the
people MAINTAINERS.md records for them. Nobody loses coverage, every
handle owns at least as much as before. MAINTAINERS.md needs no change.
One thing I loosened on purpose and want you to look at: the two owner
rule on `cmd/api-gate`, `internal/apigate` and `api-review-gate.yaml` is
gone. The rationale in the file was that a PR could otherwise neuter the
detector because it is compiled from head checkout during bootstrap.
Those paths still require code owner review under the go source and CI
rules, and any of the twelve collaborators with write access can push to
a branch anyway, so what the narrow rule bought was delay and not
protection. If we want that property it needs a required check naming
reviewers, not an owner list.
`hack/codeowners-invariant.bats` passes on the new file. Its exemption
for a rule owned by exactly @tym83 @kvaps is dead code once the
governance block is gone, and a dead exemption there is a hole, any
future two owner rule with that set would be waived, so it goes too. I
checked by mutation that the test still fails on `path @kvaps @tym83`.
Measurements above are deliberately not in the file, they date. Comments
there are down to the two mechanics that fail silently (last match
replaces, and a handle without write access ignored without error) plus
why generated mirrors are listed with no owner.
I checked the result against `git ls-files` with last-match semantics:
no pattern matches nothing, no pattern shadowed by a later one, no rule
under five owners, every handle is a current collaborator with write
access. Glob expansions (`capi-*`, `kubevirt*`, `keycloak*`,
`backup*-controller`, `flux-*`) match the dirs they are meant to.
### Downstream repositories
- [x] No downstream repository is affected by this change
- [ ] [cozystack/website](https://github.com/cozystack/website) -
follow-up:
- [ ]
[cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack)
- follow-up:
- [ ]
[cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack)
- follow-up:
- [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up:
- [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up:
- [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) -
follow-up:
- [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) -
follow-up:
- [ ]
[cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server)
- follow-up:
- [ ]
[cozystack/external-apps-example](https://github.com/cozystack/external-apps-example)
- follow-up:
- [ ] [cozystack/examples](https://github.com/cozystack/examples) -
follow-up:
### Release note
```release-note
chore(codeowners): code ownership is grouped by area instead of by package and every rule names at least five owners, so no path requires a review from one of two people
```