OWNERS: sync master to release-8.1#69500
Conversation
skip-checks: true Signed-off-by: Ti Chi Robot <ti-community-prow-bot@tidb.io>
ti-chi-bot
added
approved
cherry-pick-approved
|
Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
ti-chi-bot
Bot
added
do-not-merge/release-note-label-needed
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
ti-chi-bot
Bot
added
size/XXL
📝 WalkthroughWalkthroughThis PR rewrites repository ownership metadata: the root OWNERS and alias lists are restructured, executor and several package OWNERS files are added or updated, and some paths now use regex-based ownership filters. ChangesOwnership governance updates
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@OWNERS`:
- Around line 4-9: The OWNERS fallback matcher still captures go.mod changes
because the broad pattern overlaps with the go.mod rule. Update the OWNERS
matchers so the fallback entry explicitly excludes go.mod, using the existing
go.mod and wildcard patterns as the reference points, so changes to dependency
files remain owned only by sig-critical-approvers-dep.
In `@pkg/executor/OWNERS`:
- Around line 2-7: Restore ownership coverage in the OWNERS file by adding a
broader filter or additional rules so files under pkg/executor beyond
analyze*.go still inherit approvers despite no_parent_owners: true. Update the
OWNERS configuration to cover the remaining pkg/executor files and
subdirectories such as internal, lockstats, metrics, test, and testdata, using
the existing OWNERS entries and filter structure as the place to make the
change.
In `@pkg/meta/metadef/OWNERS`:
- Around line 5-10: The catch-all ".*" owner rule in OWNERS overlaps with the
existing "(OWNERS|system_tables_def\\.go)$" rule, so those restricted paths end
up getting sig-approvers-meta as well. Update the OWNERS patterns to explicitly
exclude OWNERS and system_tables_def.go from the catch-all, or split the rule
set so the restricted paths remain covered only by
sig-critical-approvers-tidb-server; use the existing rule keys in OWNERS to
place the exclusion cleanly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: f3b38fc0-0410-46f7-9b5e-69596a66ddf2
📒 Files selected for processing (21)
OWNERSOWNERS_ALIASESpkg/dumpformat/OWNERSpkg/dxf/OWNERSpkg/executor/OWNERSpkg/executor/aggfuncs/OWNERSpkg/executor/aggregate/OWNERSpkg/executor/join/OWNERSpkg/executor/mppcoordmanager/OWNERSpkg/executor/sortexec/OWNERSpkg/executor/test/analyzetest/OWNERSpkg/executor/unionexec/OWNERSpkg/ingestor/OWNERSpkg/lightning/config/OWNERSpkg/meta/metadef/OWNERSpkg/metrics/OWNERSpkg/objstore/OWNERSpkg/session/OWNERSpkg/sessionctx/vardef/OWNERSpkg/statistics/OWNERSpkg/util/naming/OWNERS
| go\.mod$: | ||
| approvers: | ||
| - sig-critical-approvers-dep | ||
| .*: | ||
| approvers: | ||
| - sig-community-approvers |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify that the current OWNERS filters overlap for go.mod.
python3 - <<'PY'
import re
patterns = [r"go\.mod$", r".*"]
for path in ["go.mod", "pkg/example/go.mod", "README.md"]:
print(path, "=>", [p for p in patterns if re.search(p, path)])
PYRepository: pingcap/tidb
Length of output: 242
Exclude go.mod from the fallback matcher.
go\.mod$ also matches pkg/example/go.mod, and .* matches the same paths, so Prow will still accumulate sig-community-approvers for go.mod changes. Make the fallback pattern explicitly exclude go.mod so dependency-owned files stay gated to sig-critical-approvers-dep.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@OWNERS` around lines 4 - 9, The OWNERS fallback matcher still captures go.mod
changes because the broad pattern overlaps with the go.mod rule. Update the
OWNERS matchers so the fallback entry explicitly excludes go.mod, using the
existing go.mod and wildcard patterns as the reference points, so changes to
dependency files remain owned only by sig-critical-approvers-dep.
| options: | ||
| no_parent_owners: true | ||
| filters: | ||
| "analyze.*\\.go$": | ||
| approvers: | ||
| - sig-approvers-stats |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Direct files under pkg/executor not matched by analyze.*\\.go$:"
find pkg/executor -maxdepth 1 -type f | sort | grep -Ev '/(OWNERS|analyze.*\.go)$' || true
echo
echo "Immediate child directories without their own OWNERS file:"
find pkg/executor -mindepth 1 -maxdepth 1 -type d | sort | while read -r dir; do
[ -f "$dir/OWNERS" ] || echo "$dir"
doneRepository: pingcap/tidb
Length of output: 4903
Restore ownership coverage for the rest of pkg/executor.
no_parent_owners: true cuts off inheritance, but the only filter here covers analyze*.go. Every other file under pkg/executor—plus child directories like internal, lockstats, metrics, test, and testdata that have no OWNERS file—loses approver coverage.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@pkg/executor/OWNERS` around lines 2 - 7, Restore ownership coverage in the
OWNERS file by adding a broader filter or additional rules so files under
pkg/executor beyond analyze*.go still inherit approvers despite
no_parent_owners: true. Update the OWNERS configuration to cover the remaining
pkg/executor files and subdirectories such as internal, lockstats, metrics,
test, and testdata, using the existing OWNERS entries and filter structure as
the place to make the change.
| "(OWNERS|system_tables_def\\.go)$": | ||
| approvers: | ||
| - sig-critical-approvers-tidb-server | ||
| ".*": | ||
| approvers: | ||
| - sig-approvers-meta |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
In Kubernetes/Prow OWNERS files, when multiple filters entries match the same path, are approvers merged from all matches or does one filter take precedence?
💡 Result:
In Kubernetes/Prow OWNERS files, when multiple filters entries match the same path, the approvers (and reviewers) are merged from all matching filters [1][2]. The Prow repoowners package evaluates filters by applying the configuration defined in each matching regex key [1]. Internally, when determining the set of approvers or reviewers for a file, Prow gathers the owners defined across all filters that match the file's path [1][2]. This additive behavior means that if multiple filter regexes match a single file, the effective set of approvers is the union of the approvers defined in each of those matching filter blocks [1]. It is important to note that when using filters, you should not define approvers or reviewers at the top level of the OWNERS file, as they must be defined within the filters blocks [3][4]. While some tools or specific implementations may have historically ignored or handled mixed configurations differently, standard Prow logic treats the filters as an additive configuration mechanism for identifying relevant owners [1][2].
Citations:
- 1: https://github.com/kubernetes-sigs/prow/blob/master/pkg/repoowners/repoowners.go
- 2: fix: support OWNERS files with both top-level config and filters openshift/ci-tools#5128
- 3: https://github.com/kubernetes/community/blob/main/contributors/guide/owners.md
- 4: https://www.kubernetes.dev/docs/guide/owners/
Exclude OWNERS and system_tables_def.go from the catch-all rule
".*" also matches those paths, so sig-approvers-meta is added alongside sig-critical-approvers-tidb-server. If those files should stay restricted, split the catch-all or add an explicit exclusion.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@pkg/meta/metadef/OWNERS` around lines 5 - 10, The catch-all ".*" owner rule
in OWNERS overlaps with the existing "(OWNERS|system_tables_def\\.go)$" rule, so
those restricted paths end up getting sig-approvers-meta as well. Update the
OWNERS patterns to explicitly exclude OWNERS and system_tables_def.go from the
catch-all, or split the rule set so the restricted paths remain covered only by
sig-critical-approvers-tidb-server; use the existing rule keys in OWNERS to
place the exclusion cleanly.
1 participant
What problem does this PR solve?
Sync
OWNERS*files frommastertorelease-8.1.What changed and how does it work?
This PR was auto-generated by Prow and only updates
OWNERSandOWNERS_ALIASESfiles./release-note-none
skip-issue-check
lgtm
approved
cherry-pick-approved
Summary by CodeRabbit