OWNERS: sync master to release-8.1 - #69542
ti-chi-bot wants to merge 0 commit into
Conversation
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
📝 WalkthroughWalkthroughThe root ChangesOWNERS Routing and Alias Overhaul
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 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: 2
🤖 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-10: The OWNERS fallback rule currently overlaps with the
go.mod-specific rule, so go.mod still picks up the generic approvers/reviewers
from the `.*` entry. Update the matching patterns in OWNERS so the fallback does
not apply to `go.mod`—for example by narrowing the generic rule or using a
negative lookahead—while keeping the `go\.mod$` entry mapped only to the dep
approvers.
In `@pkg/executor/OWNERS`:
- Around line 2-7: The OWNERS config for pkg/executor currently only assigns
approvers for files matching analyze.*\.go, so top-level package files like
adapter.go, admin.go, and BUILD.bazel end up with no fallback approver because
no_parent_owners is enabled. Update the OWNERS entries to add a catch-all local
rule or another fallback owner set that covers the remaining direct files in
pkg/executor/, while keeping the existing analyze.*\.go approver mapping intact.
🪄 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: d5aaa42c-fa07-4b02-b7ab-0c059ecf203a
📒 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 | ||
| reviewers: |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
In Kubernetes/Prow OWNERS files, when multiple filtersregexes match the same path, areapproversandreviewers merged from all matching filters or does one rule take precedence?
💡 Result:
In Kubernetes/Prow OWNERS files, when multiple filters match a file path, the approvers and reviewers are merged from all matching filter configurations [1]. Prow evaluates these filters and aggregates the owners (approvers/reviewers) specified in every matching regex block for the given path [1]. Key points regarding the behavior of filters in Prow OWNERS files: 1. Aggregation (Merging): Prow's repository owners parser iterates through the defined filters for a file path. If a file matches multiple regex patterns defined in the filters map, the corresponding owners (approvers, reviewers, etc.) from each of those matching filters are combined into a collective set [1]. 2. Structural Requirement: When using the filters key in an OWNERS file, you must define all owner-related configurations (such as approvers, reviewers, and labels) inside the filters blocks [2][3]. Defining these keys at the top level of the OWNERS file alongside a filters block is generally invalid or deprecated, though some implementations or tooling may handle mixed configurations differently [2][4]. 3. File System Context: Filters are matched against paths relative to the directory containing the OWNERS file [2][3]. 4. Precedence: There is no strict "first-match-wins" precedence rule where only one filter applies; instead, the system is designed to allow additive ownership through the merging of all matching filter criteria [1]. It is important to note that while the Prow core logic supports this merging for various metadata, developers should verify if specific plugins (like the older versions of the approve plugin) fully support filters for all owner-related fields, as historical documentation has noted limitations in plugin-specific implementations [5][6]. Always prioritize the official Kubernetes community guidance on OWNERS files when configuring repository access [2][3].
Citations:
- 1: https://github.com/kubernetes-sigs/prow/blob/master/pkg/repoowners/repoowners.go
- 2: https://github.com/kubernetes/community/blob/main/contributors/guide/owners.md
- 3: https://www.kubernetes.dev/docs/guide/owners/
- 4: fix: support OWNERS files with both top-level config and filters openshift/ci-tools#5128
- 5: [WIP] Added OWNERS for localization projects kubernetes/website#14108
- 6: https://workshop--kubernetes-contributor.netlify.app/docs/guide/owners/
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the OWNERS file and any nearby docs or configs that define ownership behavior.
git ls-files | rg '(^|/)OWNERS(\.yaml|\.yml)?$|owners'
printf '\n--- OWNERS files ---\n'
for f in $(git ls-files | rg '(^|/)OWNERS(\.yaml|\.yml)?$'); do
echo "### $f"
cat -n "$f"
echo
done
printf '\n--- docs mentioning filters/OWNERS ---\n'
rg -n --hidden --glob '!**/.git/**' 'filters|OWNERS' .Repository: pingcap/tidb
Length of output: 50368
Exclude go.mod from the fallback filter. go\.mod$ and .* both match go.mod, and matching OWNERS filters are additive, so go.mod will still inherit sig-community-approvers/sig-community-reviewers. Use a narrower fallback pattern or a negative lookahead if this file should route only to the dep approvers.
🤖 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 - 10, The OWNERS fallback rule currently overlaps with
the go.mod-specific rule, so go.mod still picks up the generic
approvers/reviewers from the `.*` entry. Update the matching patterns in OWNERS
so the fallback does not apply to `go.mod`—for example by narrowing the generic
rule or using a negative lookahead—while keeping the `go\.mod$` entry mapped
only to the dep approvers.
| options: | ||
| no_parent_owners: true | ||
| filters: | ||
| "analyze.*\\.go$": | ||
| approvers: | ||
| - sig-approvers-stats |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
printf 'Direct files in pkg/executor not matched by analyze filter:\n'
fd -t f . pkg/executor -d 1 \
| sed 's#^\./##' \
| grep -vE '^pkg/executor/OWNERS$' \
| grep -vE '^pkg/executor/analyze.*\.go$' || true
printf '\nOWNERS files under pkg/executor:\n'
fd '^OWNERS$' pkg/executor | sed 's#^\./##'Repository: pingcap/tidb
Length of output: 5033
Add a fallback owner for pkg/executor/
no_parent_owners: true leaves every direct file in pkg/executor/ without approvers unless it matches analyze.*\.go or has its own nested OWNERS file. That excludes files like adapter.go, admin.go, BUILD.bazel, and the rest of the top-level package files. Add a catch-all rule or another local OWNERS entry for the remaining files.
🤖 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, The OWNERS config for pkg/executor
currently only assigns approvers for files matching analyze.*\.go, so top-level
package files like adapter.go, admin.go, and BUILD.bazel end up with no fallback
approver because no_parent_owners is enabled. Update the OWNERS entries to add a
catch-all local rule or another fallback owner set that covers the remaining
direct files in pkg/executor/, while keeping the existing analyze.*\.go approver
mapping intact.
6398479 to
986af29
Compare
|
[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/XS
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