OWNERS: sync master to release-8.1 - #70405
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 PR replaces static ownership lists with synchronized aliases and path-based approval rules. ChangesRepository ownership model
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related issues
Possibly related PRs
Suggested labels: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 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 3-11: Update the catch-all filter in OWNERS so it does not match
go.mod alongside the exclusive go\.mod$ rule. Use the repository’s established
non-overlapping default filter, preserving sig-community-approvers and
sig-community-reviewers for other files while ensuring go.mod remains governed
by sig-critical-approvers-dep.
In `@pkg/executor/OWNERS`:
- Around line 4-10: Add a catch-all approver rule in pkg/executor/OWNERS for
files not matched by the existing BUILD.bazel and analyze*.go filters, assigning
sig-approvers-executor while preserving those specific rules and
no_parent_owners behavior.
🪄 Autofix
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 Plus
Run ID: 4cff7ffc-145f-4947-80b7-36dba1f2cce0
📒 Files selected for processing (48)
OWNERSOWNERS_ALIASESbr/OWNERSbr/pkg/storage/OWNERSdumpling/OWNERSlightning/OWNERSlightning/cmd/tidb-lightning-ctl/OWNERSlightning/cmd/tidb-lightning/OWNERSpkg/autoid_service/OWNERSpkg/bindinfo/OWNERSpkg/config/OWNERSpkg/ddl/OWNERSpkg/distsql/OWNERSpkg/disttask/OWNERSpkg/domain/OWNERSpkg/dumpformat/OWNERSpkg/dxf/OWNERSpkg/executor/OWNERSpkg/executor/aggfuncs/OWNERSpkg/executor/aggregate/OWNERSpkg/executor/importer/OWNERSpkg/executor/join/OWNERSpkg/executor/mppcoordmanager/OWNERSpkg/executor/sortexec/OWNERSpkg/executor/test/analyzetest/OWNERSpkg/executor/unionexec/OWNERSpkg/expression/OWNERSpkg/infoschema/OWNERSpkg/ingestor/OWNERSpkg/lightning/OWNERSpkg/lightning/config/OWNERSpkg/lock/OWNERSpkg/meta/OWNERSpkg/meta/metadef/OWNERSpkg/metrics/OWNERSpkg/objstore/OWNERSpkg/owner/OWNERSpkg/parser/OWNERSpkg/planner/OWNERSpkg/resourcemanager/OWNERSpkg/session/OWNERSpkg/sessionctx/vardef/OWNERSpkg/sessionctx/variable/OWNERSpkg/statistics/OWNERSpkg/table/OWNERSpkg/tablecodec/OWNERSpkg/tidb-binlog/OWNERSpkg/util/naming/OWNERS
💤 Files with no reviewable changes (3)
- pkg/disttask/OWNERS
- pkg/tidb-binlog/OWNERS
- br/pkg/storage/OWNERS
| filters: | ||
| go\.mod$: | ||
| approvers: | ||
| - sig-critical-approvers-dep | ||
| .*: | ||
| approvers: | ||
| - sig-community-approvers | ||
| reviewers: | ||
| - sig-community-reviewers |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
Prow OWNERS filters multiple matching regexes union or first match
💡 Result:
In Prow's OWNERS file configuration, the filters key uses Go regular expressions to apply specific settings to files [1][2]. When multiple filters are defined, Prow does not necessarily perform a simple "first-match" or "union" in the way one might expect for general rule overrides [3]. Based on the implementation in pkg/repoowners/repoowners.go, Prow iterates through the filters map and applies configurations to files that match the associated regular expression [3]. Because Go's map iteration order is randomized, relying on the order of regex definitions in the filters block to determine precedence is unreliable [3]. Key technical points include: - Scope: Filters are matched against file paths relative to the OWNERS file in which they are declared [1][2]. - Configuration: When filters is used, standard approvers, reviewers, or labels keys should not be defined at the top level of the OWNERS file [1][2]. Instead, a .* filter should be used as a catch-all if default settings are needed alongside more specific filters [1][2]. - Limitations: The approve plugin has historically had documented limitations regarding its interaction with complex regex filtering, as it makes assumptions about directory-level ownership that may conflict with fine-grained file-level filters [4][5]. For scenarios requiring complex precedence, it is best practice to design non-overlapping regexes to ensure predictable application of labels or ownership [1][2].
Citations:
- 1: https://www.kubernetes.dev/docs/guide/owners/
- 2: https://github.com/kubernetes/community/blob/main/contributors/guide/owners.md
- 3: https://github.com/kubernetes-sigs/prow/blob/master/pkg/repoowners/repoowners.go
- 4:
approveplugin makes assumptions about OWNERS file implementation kubernetes/test-infra#7690 - 5: https://workshop--kubernetes-contributor.netlify.app/docs/guide/owners/
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== candidates =="
git ls-files | rg '(^|/)OWNERS(_ALIASES)?|OWNERS' | sed -n '1,120p'
echo
echo "== OWNERS files =="
while IFS= read -r f; do
echo "--- $f"
wc -l "$f"
sed -n '1,80p' "$f" | cat -n
done < <(git ls-files | rg '(^|/)OWNERS$' | sed -n '1,20p')
echo
echo "== references to sig-critical-approvers-dep and sig-community-appr =="
rg -n "sig-critical-approvers-dep|sig-community-approvers|filters:|no_parent_owners: true" . || trueRepository: pingcap/tidb
Length of output: 1270
🌐 Web query:
Prow repoowners LoadFromInternal maps filters iteration order applying configuration when multiple filters match
💡 Result:
In Prow's repoowners package, the filters key in an OWNERS file is defined as a map where keys are Go regular expressions and values are Config objects [1][2]. The documentation and standard practices for Prow OWNERS files explicitly state that if filters are used, simple OWNERS configuration (such as approvers, reviewers, or labels) must not be defined at the top level of the file [2][3]. The only exception allowed at the top level when using filters is the options key (e.g., no_parent_owners) [2][3]. Regarding the iteration order and application of multiple matching filters: 1. Iteration Order: Because filters are stored as a standard Go map[string]Config (where the key is the string representation of the regular expression) [1], the iteration order over these filters is non-deterministic. Go maps do not guarantee a fixed order during iteration. 2. Multiple Matches: Prow's repoowners logic does not natively "merge" multiple matching filters into a single configuration object in a specific priority order. Instead, the repoowners implementation treats the filters as a mechanism to scope configuration to specific file paths using regex matching [1][2]. If multiple regular expressions match a given file path, Prow typically processes them in the non-deterministic order provided by the map iteration, and the resulting configuration is usually determined by whichever filter is applied or processed last, or based on how the specific plugin (e.g., approve, blunderbuss) consumes the RepoOwners struct [4][5]. Because of the non-deterministic nature of map iteration and the potential for conflicting configurations when multiple filters match a single file, it is highly recommended to design OWNERS file filters so that they are mutually exclusive or that they do not overlap in a way that would lead to ambiguous configuration application [2][3]. Users are advised to use a catch-all .* key inside filters if they wish to apply a base configuration across all files, rather than mixing top-level settings with filters [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: https://github.com/kubernetes-sigs/prow/blob/main/pkg/plugins/approve/approvers/owners.go
- 5: https://github.com/kubernetes-sigs/prow/blob/main/pkg/plugins/blunderbuss/blunderbuss.go
🌐 Web query:
"sig-critical-approvers-dep" "sig-community-approvers" OWNERS tidb
💡 Result:
In the TiDB project, sig-critical-approvers-dep and sig-community-approvers are alias groups defined within the OWNERS_ALIASES file, which are used to control code review and approval permissions via the OWNERS file [1][2][3]. 1. sig-critical-approvers-dep: This alias is specifically designated as the group of approvers responsible for managing and upgrading dependencies (e.g., changes to go.mod) [1][3]. 2. sig-community-approvers: This alias represents a broader group of community members authorized to approve changes [2][4]. In the repository's root OWNERS file, these aliases are used to apply different approval requirements based on file types [1][5]: - Files matching the pattern go.mod are governed by the sig-critical-approvers-dep group [1][6]. - All other files (the.* wildcard) are generally governed by the sig-community-approvers group [1][6]. The membership of these groups is typically managed in an external repository (often pingcap/community) and synced into the TiDB project [1][2].
Citations:
- 1: https://github.com/pingcap/tidb/blob/master/OWNERS
- 2: https://github.com/pingcap/tidb/blob/e72c97a3ba72f72348a1a0269f44244f369bcbef/OWNERS_ALIASES
- 3: https://github.com/pingcap/tidb/blob/bed8f2573a7d6189f1611d913130cfb3150c9100/OWNERS_ALIASES
- 4: https://github.com/pingcap/tidb/blob/998279fef140b84961c41a083ecf828ffdbb4172/OWNERS_ALIASES
- 5: OWNERS: sync master to release-8.1 #69500
- 6: https://github.com/pingcap/tidb/blob/c1abcf8e9c1e0aba58843d0d812ad78f3f7c5712/OWNERS
🌐 Web query:
Prow owners approve files require approval filters matching path approvers merge
💡 Result:
In Prow, the approval process is primarily governed by OWNERS files located within the repository, rather than by path-matching filters in the global plugin configuration [1]. The approval system operates as follows: 1. OWNERS Files: Approval is managed on a per-directory basis using OWNERS files [1]. Every file modified in a pull request (PR) must be approved by an individual listed as an approver in the corresponding directory's OWNERS file (or a parent directory's OWNERS file if inherited) [1]. Approvers signify consent by commenting /approve on the PR [1]. 2. Plugin Configuration (plugins.yaml): The global configuration for the approve plugin, located in plugins.yaml, handles repository-level settings rather than granular path-based approval filters [2][3]. Available settings include [2][4]: - require_self_approval: When set to true, this requires PR authors who have approver rights to explicitly comment /approve on their own PR, rather than receiving implicit approval [5][4]. - lgtm_acts_as_approve: When enabled, the /lgtm command counts as an approval [2][4]. - ignore_review_state: If enabled, the plugin ignores GitHub's native review state (e.g., an "Approve" review will not count as an /approve command) [2][4]. 3. Path-based Logic: While Prow does support "Filters" in some contexts (such as the RepoOwners configuration) to apply specific configurations to files matching regex patterns [6], these generally manage assigning labels or overriding default settings rather than defining custom approver lists for specific paths [6]. The standard mechanism for determining who can approve a specific path remains the OWNERS file structure [1][7]. To ensure a PR can be merged, all modified files must satisfy these approval requirements [1][7]. The Prow bot tracks this state and will prevent merging until the required approvals are registered [1][7].
Citations:
- 1: https://docs.prow.k8s.io/docs/components/plugins/approve/approvers/
- 2: https://github.com/kubernetes-sigs/prow/blob/f6cc3990c368/pkg/plugins/config.go
- 3: https://docs.prow.k8s.io/docs/components/core/hook/
- 4: https://github.com/kubernetes/test-infra/blob/96e55b3887c574f897052f722022ebe5acf35675/prow/plugins/plugin-config-documented.yaml
- 5: require_self_approval: true does not work as expected. kubernetes/test-infra#12693
- 6: https://github.com/kubernetes-sigs/prow/blob/master/pkg/repoowners/repoowners.go
- 7: https://github.com/kubernetes-sigs/prow/blob/main/pkg/plugins/approve/approvers/owners.go
🏁 Script executed:
#!/bin/bash
set -ueo pipefail
echo "== root OWNERS =="
cat -n OWNERS | sed -n '1,40p'
echo
echo "== OWNERS_ALIASES relevant aliases =="
rg -n "sig-critical-approvers-dep|sig-community-approvers|sig-community-reviewers" OWNERS_ALIASES || true
echo
echo "== pkg/config OWNERS =="
cat -n pkg/config/OWNERS | sed -n '1,40p'
echo
echo "== generated sync references =="
rg -n "OWNERS sync|sync.*OWNERS|sig-critical-approvers-dep|sig-community-approvers" . --glob '!*.png' --glob '!*.jpg' --glob '!node_modules/**' | sed -n '1,160p'Repository: pingcap/tidb
Length of output: 3878
🌐 Web query:
kubernetes approve plugin multiple OWNERS directories required approvals path filters merge logic
💡 Result:
In Kubernetes Prow, the approve plugin manages PR approvals by tracking files modified in a PR against a hierarchy of OWNERS files [1][2]. To satisfy approval requirements, at least one approver from each identified "leaf" OWNERS file must issue an /approve command [1][3][4]. Key mechanics include: 1. OWNERS Directory Hierarchy: Every directory can contain an OWNERS file, which applies to that directory and its subdirectories [1][5][4]. The plugin identifies the nearest (leaf) OWNERS file for every modified file in a PR [2][4]. 2. Multiple Approvals and Path Filters: - When using the filters key in an OWNERS file, you can apply specific approval/reviewer configurations to sub-paths using Go regular expressions [5][6]. These filters allow for granular control, such as requiring different approvers for specific file patterns within a directory [5][6]. - If a PR touches multiple directories, the plugin aggregates the required OWNERS files [2]. A PR cannot be merged until it has received an approval from a valid approver for each of those required files [1][5]. - If a user is an approver in a parent directory and a sub-directory, an approval in the parent may count for the entire PR (depending on configuration and file coverage), but generally, the system ensures each unique leaf requirement is satisfied [1]. 3. Merge Logic: - The approve plugin adds the "approved" label to a PR only when the total set of required approvals—derived from all modified files' corresponding OWNERS files—is met [1][5][4]. - The "removeSubdirs" logic in the implementation optimizes this by removing subdirectories from the requirement list if a parent directory already covers the path (unless specific "no parent" configurations prevent this), ensuring that the set of required approvals is pruned to the most efficient hierarchy [2]. - Automation, such as Tide, uses this "approved" label alongside other signals (like lgtm or test results) to decide when to merge the PR [5]. If you need to require multiple approvals for a single file or directory beyond the standard one-approver-per-file model, the system generally relies on the structure of the OWNERS file itself or complex filter setups, though the core requirement is one valid signature per mandatory OWNERS file [1][4].
Citations:
- 1: https://docs.prow.k8s.io/docs/components/plugins/approve/approvers/
- 2: https://github.com/kubernetes-sigs/prow/blob/main/pkg/plugins/approve/approvers/owners.go
- 3: https://github.com/kubernetes/community/blob/a74d906f0121c78114d79a3ac105aa2d36e24b57/contributors/devel/owners.md
- 4: https://github.com/kubernetes/community/blob/main/contributors/guide/owners.md
- 5: https://www.kubernetes.dev/docs/guide/owners/
- 6: https://github.com/kubernetes/community/blob/main/OWNERS
🌐 Web query:
Prow approve plugin multiple files single PR approvals only need one approver OWNERS
💡 Result:
In the Prow "approve" plugin, a single PR does not necessarily require multiple approvers, even if it modifies multiple files [1]. Approval is governed by the OWNERS files that cover the modified code [1][2]. The "approve" plugin requires that every file modified by a PR must be approved by an approver listed in the relevant OWNERS files [1]. However, a single approver can satisfy the approval requirements for multiple files or even the entire PR if that person is listed as an approver in an OWNERS file that covers all the modified paths (such as a root-level OWNERS file) [1]. Key points regarding the approval process: - Scope: Approvers listed in an OWNERS file have approval authority for that directory and all its subdirectories [1][3]. - Consolidation: If a PR modifies multiple files, Prow identifies the nearest OWNERS files for each change [4][2]. If one person is an approver in a directory that acts as a parent to all modified files, their single /approve command can satisfy the requirement for the entire PR [1]. - No Minimum Approvers: There is no hard requirement for multiple approvers unless the changes touch directories that are governed by different, non-overlapping OWNERS files where no single person is authorized across all of them [1]. If a PR touches code paths with disjoint ownership (i.e., different OWNERS files with no common approver), Prow will require approval from an authorized person for each of those distinct ownership domains [1][4][2]. In such cases, the PR cannot be merged until all necessary approvals are collected [4][2].
Citations:
- 1: https://docs.prow.k8s.io/docs/components/plugins/approve/approvers/
- 2: https://www.kubernetes.dev/docs/guide/owners/
- 3: https://workshop--kubernetes-contributor.netlify.app/docs/guide/owners/
- 4: https://github.com/kubernetes/community/blob/main/contributors/guide/owners.md
🌐 Web query:
"repoowners" "LoadFromInternal" "filters" "matched all filters" "approvers"
💡 Result:
In the context of Kubernetes Prow and OWNERS file management, the term filters refers to a configuration block that allows for path-specific owner overrides based on regular expressions [1][2]. When filters are defined in an OWNERS file: 1. All owner-related keys—specifically approvers, reviewers, labels, and emeritus_approvers—must be defined within the filters blocks [1][2]. They cannot coexist as top-level keys in the same file [2]. 2. The options key (such as no_parent_owners) is an explicit exception and may still be defined at the top level of the file alongside the filters block [1][3]. 3. The LoadFromInternal logic (typically found in Prow's repoowners package) is responsible for parsing these configurations [4][1]. Historically, some tools had difficulty when an OWNERS file contained both top-level configuration and filters [5]. Recent updates in various ecosystem tools (like OpenShift's ci-tools) have improved this by ensuring the system attempts to load the file as a FullConfig (which supports filters) first, falling back to a SimpleConfig only if no filters are present [5]. The phrase "matched all filters" relates to how Prow matches a file path against the regex keys within the filters map [1][2]. When a PR is submitted, the system evaluates the file path against these regex patterns to determine which approvers and reviewers are relevant for that specific file [4][6]. The repoowners package provides methods such as Approvers(path string) to look up these entities, handling inheritance (unless no_parent_owners is set) and merging the results from relevant OWNERS files [4][1][6].
Citations:
- 1: https://github.com/kubernetes/community/blob/main/contributors/guide/owners.md
- 2: https://www.kubernetes.dev/docs/guide/owners/
- 3: Update documentation for OWNERS file regarding no_parent_owners kubernetes/community#8484
- 4: https://github.com/kubernetes-sigs/prow/blob/master/pkg/repoowners/repoowners.go
- 5: fix: support OWNERS files with both top-level config and filters openshift/ci-tools#5128
- 6: https://github.com/kubernetes-sigs/prow/blob/main/pkg/plugins/approve/approvers/owners.go
🌐 Web query:
github kubernetes-sigs/prow pkg repoowners repoowners.go LoadFromInternal filters regex iteration
💡 Result:
Based on an examination of the kubernetes-sigs/prow repository, there is no method named LoadFromInternal in pkg/repoowners/repoowners.go [1]. The primary methods for loading repository owners in this package are LoadRepoOwners and LoadRepoOwnersSha [1]. The term LoadFromInternal appears in unrelated libraries (such as MapDB or various Java utilities) and is not a standard component of the Prow repoowners logic [2][3][4][5]. Furthermore, while Prow extensively uses regular expressions (regexp) throughout its codebase—for instance, in config validation [6][7] and branch protection policies [8]—the repoowners package focuses on parsing OWNERS and OWNERS_ALIAS files [9] rather than generic regex-based iteration over filters as part of a LoadFromInternal process [1]. If you are encountering a reference to LoadFromInternal in a specific Prow-related context, it may be a custom internal modification, a misunderstanding of a different package's naming conventions, or an artifact from a different project entirely.
Citations:
- 1: https://github.com/kubernetes-sigs/prow/blob/master/pkg/repoowners/repoowners.go
- 2: https://mapdb.org/dokka/latest/mapdb/org.mapdb/-store-trivial/load-from-internal.html
- 3: https://mapdb.org/javadoc/latest/mapdb/org/mapdb/StoreTrivial.html
- 4: https://mapdb.org/dokka/latest/mapdb/org.mapdb/-store-trivial/
- 5: https://android.googlesource.com/platform/external/desugar/+/master/java/com/google/devtools/build/android/desugar/LambdaDesugaring.java
- 6: https://pkg.go.dev/sigs.k8s.io/prow/pkg/config
- 7: https://github.com/kubernetes-sigs/prow/blob/f6cc3990c368/pkg/plugins/config.go
- 8: https://github.com/kubernetes-sigs/prow/blob/main/pkg/config/branch_protection.go
- 9: https://github.com/kubernetes/community/blob/main/contributors/guide/owners.md
🌐 Web query:
"Approvers(path string)" "repoowners" "Prow"
💡 Result:
In Prow, the term "Approvers" refers to the set of users defined in the approvers section of an OWNERS file [1][2]. These users have the authority to approve pull requests by issuing the /approve command [1][2]. From a technical perspective, specifically within the repoowners package of the Prow codebase, Approvers(path string) is a function used to retrieve the list of all approvers for a given file path [3]. This function behaves as follows: 1. It calculates the effective list of approvers for the specified file by traversing the directory tree up to the root, aggregating all users listed in the approvers sections of the relevant OWNERS files along that path [3]. 2. It returns a layeredsets.String (an ordered or layered set of strings), which includes both local approvers (those defined in the directory containing the file) and inherited approvers (those defined in parent directories) [3]. 3. This mechanism ensures that a PR touching a specific file can be approved by anyone who has "approver" status for that file's directory or any of its parent directories [1]. To distinguish between local and inherited approvers, the codebase also provides a LeafApprovers(path string) function, which returns only the approvers defined in the closest OWNERS file for the given path, excluding those from parent directories [3]. In summary, Approvers(path string) is the standard interface for Prow plugins (such as the approve plugin) to determine if a user has the necessary permissions to approve a change for a specific file [3][4][5].
Citations:
- 1: https://docs.prow.k8s.io/docs/components/plugins/approve/approvers/
- 2: https://www.kubernetes.dev/docs/guide/owners/
- 3: https://github.com/kubernetes-sigs/prow/blob/master/pkg/repoowners/repoowners.go
- 4: https://github.com/kubernetes-sigs/prow/blob/main/pkg/plugins/approve/approvers/owners.go
- 5: https://github.com/kubernetes-sigs/prow/blob/main/pkg/plugins/blunderbuss/blunderbuss.go
🌐 Web query:
"repoowners.go" "Prow" "for name := range f.filters" "matched"
💡 Result:
In the Prow codebase, specifically within pkg/repoowners/repoowners.go, the code block for pattern, config := range c.Filters (which corresponds to your reference to iterating over filters) is used to process the filters configuration defined in OWNERS files [1]. The filters key in an OWNERS file allows for granular control over ownership by mapping file paths—defined as Go regular expressions—to specific configurations [2][3]. The logic iterates through these filters to match against the file path being processed. When a match is found (or specifically, as it iterates through all defined filters), the system applies the corresponding Config (such as labels, approvers, or reviewers) to the path [1][2]. Key details regarding this process: - The regular expression keys are matched against paths relative to the OWNERS file in which they are declared [2][3]. - If a filters block is present, all other ownership-related keys (e.g., approvers, reviewers) must be defined within the filters block rather than at the top level [2][3]. - The options key (like no_parent_owners) is an exception and can be defined at the top level alongside filters [2][3]. - The loop in repoowners.go ensures that if a filter matches the current file path, the configuration associated with that regex pattern is applied to that path [1].
Citations:
- 1: https://github.com/kubernetes-sigs/prow/blob/master/pkg/repoowners/repoowners.go
- 2: https://www.kubernetes.dev/docs/guide/owners/
- 3: https://github.com/kubernetes/community/blob/main/contributors/guide/owners.md
🌐 Web query:
github.com/kubernetes/test-infra/contributors/devel/owners.md Prow OWNERS filters multiple filters
💡 Result:
In Prow, the filters key within an OWNERS file allows you to define directory-specific or file-specific configurations based on path matching [1][2]. Key characteristics of using filters: - Structure: It is a map where keys are Go regular expressions and values are simple OWNERS configurations (e.g., approvers, reviewers, labels, emeritus_approvers) [1][2]. - Matching: The regular expressions are matched against file paths relative to the directory containing the OWNERS file [1][2]. - Mutual Exclusivity: If you use the filters key, you must not define standard, top-level owner keys (like approvers or reviewers) in the same file [1][2]. All such keys must be moved inside the filters blocks [1][2]. - Global Matching: To include a default configuration alongside specific filters, you should use a .* key within the filters map [1][2]. - Exception: The options key (such as no_parent_owners) is an exception and can remain at the top level of the OWNERS file even when filters are used [1][2]. Example configuration: filters: ".*": labels: - re/all "\.go$": labels: - re/go Important Note: As of the most recent documentation, the approve plugin does not fully support filters [3]. Consequently, it is recommended to use filters primarily for labels rather than for managing approvers or reviewers [3]. Always ensure that configuration follows the pattern of placing all owner-related keys inside the filters map to avoid invalid configurations [1][2].
Citations:
- 1: https://github.com/kubernetes/community/blob/main/contributors/guide/owners.md
- 2: https://www.kubernetes.dev/docs/guide/owners/
- 3: https://workshop--kubernetes-contributor.netlify.app/docs/guide/owners/
🌐 Web query:
kubernetes-sigs/prow "pkg/repoowners/repoowners.go" "Filters" "For path := range c.Filters" "approvers"
💡 Result:
In the Kubernetes Prow repoowners package, the Filters field within FullConfig allows for path-specific OWNERS configurations by mapping Go regular expressions to Config objects [1][2]. When Filters are defined in an OWNERS file, they enable granular control over ownership (approvers, reviewers, labels) based on file paths relative to that OWNERS file [3][4]. The logic for processing these filters is implemented within pkg/repoowners/repoowners.go [1]. Specifically, when the repoowners logic evaluates a file path, it iterates through these filters to match the file against the provided regular expressions [1]. If a match is found, the configuration associated with that filter is applied to the file [1][3]. Key points regarding the use of Filters: * Usage: If filters are defined, you cannot define approvers, reviewers, or labels at the top level of the OWNERS file; they must be nested within the filters map [3][4]. * Options: The options key (such as no_parent_owners) remains an exception and can still be defined at the top level of the OWNERS file [3][4]. * Inheritance: While filters provide path-specific overrides, Prow's general approval logic still respects the hierarchical nature of OWNERS files, where parent directory approvers are typically inherited by subdirectories unless no_parent_owners is explicitly set to true [5][4]. When the Approvers method is called in repoowners.go, it retrieves the full set of approvers for a given file by resolving both the local OWNERS configuration (which may be determined by these Filters) and any applicable parent directory configurations [1].
Citations:
- 1: https://github.com/kubernetes-sigs/prow/blob/master/pkg/repoowners/repoowners.go
- 2: https://pkg.go.dev/sigs.k8s.io/prow/pkg/repoowners
- 3: https://github.com/kubernetes/community/blob/main/contributors/guide/owners.md
- 4: https://www.kubernetes.dev/docs/guide/owners/
- 5: https://docs.prow.k8s.io/docs/components/plugins/approve/approvers/
🌐 Web query:
sigs.k8s.io/prow "Approvers(path string) layeredsets.String" "Filters"
💡 Result:
In the sigs.k8s.io/prow codebase, the Approvers function is a method defined within the RepoOwner interface [1] (and similarly in the Repo interface used by the approve plugin [2]). It takes a file path as a string and returns a layeredsets.String [1][2]. The function is implemented in the repoowners package [1]. Its purpose is to resolve the set of users who have authority to approve a change to a specific file, based on the hierarchical structure of OWNERS files within the repository [1][3]. Function Signature: Approvers(path string) layeredsets.String [1][2] Key Technical Details: 1. Hierarchy Traversal: The function traverses the directory structure starting from the file's directory up to the root of the repository (or until a directory with the no_parent_owners option is encountered) to collect all applicable approvers [1]. 2. layeredsets.String: The returned type, layeredsets.String, is a data structure that maintains sets of strings organized by "layers" [1]. This allows Prow to preserve the hierarchical origin of the approvers (e.g., distinguishing between approvers defined in a deeply nested OWNERS file versus those defined at the repository root) [1]. 3. Filtering Logic: While the function signature does not explicitly contain a Filters argument, the underlying logic utilizes OWNERS file configurations—specifically the filters key in an OWNERS file—to determine which approvers apply to specific files based on regular expression matching [1][4]. The entriesForFile helper function is typically used to perform this resolution, matching file paths against these regex-based filters to aggregate the final set of approvers [1].
Citations:
- 1: https://github.com/kubernetes-sigs/prow/blob/master/pkg/repoowners/repoowners.go
- 2: https://github.com/kubernetes-sigs/prow/blob/main/pkg/plugins/approve/approvers/owners.go
- 3: https://docs.prow.k8s.io/docs/components/plugins/approve/approvers/
- 4: https://github.com/kubernetes/community/blob/main/contributors/guide/owners.md
🌐 Web query:
github kubernetes-sigs/prow pkg/plugins/approve/approvers owners.go FilesRequiredApprovalForPRTotal
💡 Result:
There is no function or constant named FilesRequiredApprovalForPRTotal in the kubernetes-sigs/prow repository's pkg/plugins/approve/approvers/owners.go file or elsewhere in the project. The approve plugin logic for determining if a pull request has met its approval requirements is implemented through methods such as AreFilesApproved and RequirementsMet within the Approvers struct in pkg/plugins/approve/approvers/owners.go [1]. These methods verify whether all relevant OWNERS files associated with the PR have been approved and check if other conditions, such as the presence of a required issue, have been satisfied [1]. The project does use various mechanisms to manage PR approvals, including: - Parsing OWNERS and OWNERS_ALIAS files to identify reviewers and approvers [2][3]. - Using the approve plugin to label PRs with "approved" once required approvals are provided by authorized users [4][2]. - Employing a layered file-list strategy to ensure that PRs exceeding the GitHub API's 3,000-file limit are still correctly evaluated for approval [5]. If you are looking for specific approval counts or requirements, Prow typically relies on the presence of the approved label, which the approve plugin adds only after validating that every file modified by the PR has been approved by an appropriate user defined in the corresponding OWNERS files [4].
Citations:
- 1: https://github.com/kubernetes-sigs/prow/blob/main/pkg/plugins/approve/approvers/owners.go
- 2: https://github.com/kubernetes/community/blob/main/contributors/guide/owners.md
- 3: https://github.com/kubernetes-sigs/prow/blob/master/pkg/repoowners/repoowners.go
- 4: https://docs.prow.k8s.io/docs/components/plugins/approve/approvers/
- 5: approve: fix silent approval bypass when PR exceeds GitHub file list API limit kubernetes-sigs/prow#707
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== go.mod files tracked =="
git ls-files 'go.mod' | sed -n '1,120p'Repository: pingcap/tidb
Length of output: 184
🌐 Web query:
"repoowners.go" "entriesForFile" "Filters" "approvers" "matched all filters"
💡 Result:
In the repoowners.go implementation within the Kubernetes Prow project, the entriesForFile function is responsible for determining the owners (approvers or reviewers) for a specific file by traversing the directory tree up to the repository root [1]. Regarding the mechanism of "filters" and "matched all filters": 1. Filter Logic: When an OWNERS file contains a filters block, the entriesForFile function evaluates these filters against the file path. The keys within the filters map are Go regular expressions [2]. These expressions are matched against the path of the target file relative to the directory containing the OWNERS file [2]. 2. Evaluation Process: During the directory traversal in entriesForFile, the code iterates through available filters. For each filter, it checks if the regex matches the relative path: if re == nil || re.MatchString(relative) { out.Insert(layerID, sets.List(s)...) } This means that if a filter's regex matches the file path, the owners defined under that filter are added to the result set [1]. 3. "Matched all filters": There is no singular "matched all filters" requirement in the standard entriesForFile logic. Instead, the implementation collects owners from every filter that matches the path. If multiple filters match, all their associated owners are aggregated into the final set [1]. The configuration documentation reinforces this behavior, noting that if filters are defined, they act as the source of truth for owners within that file's scope [2]. If a path matches multiple regex keys within a filters block, all matching configurations are applied cumulatively [1][2].
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
Make the go.mod filter exclusive before the catch-all.
go.mod matches both go\.mod$ and .*, so the root OWNERS file grants every approver group to that path. If the approval rules require sig-critical-approvers-dep members to approve dependency changes, this config can let community approvers satisfy the requirement. Rename .* to the non-overlapping default, or remove/approve from the generated sync only after the conflict is fixed upstream.
🤖 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 3 - 11, Update the catch-all filter in OWNERS so it does
not match go.mod alongside the exclusive go\.mod$ rule. Use the repository’s
established non-overlapping default filter, preserving sig-community-approvers
and sig-community-reviewers for other files while ensuring go.mod remains
governed by sig-critical-approvers-dep.
| filters: | ||
| "(^|/)BUILD\\.bazel$": | ||
| approvers: | ||
| - sig-community-approvers | ||
| "analyze.*\\.go$": | ||
| approvers: | ||
| - sig-approvers-stats |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
status=0
while IFS= read -r path; do
base=${path##*/}
case "$base" in
OWNERS|BUILD.bazel|analyze*.go) ;;
*) printf 'uncovered direct file: %s\n' "$path"; status=1 ;;
esac
done < <(git ls-files 'pkg/executor/*' | awk -F/ 'NF == 3')
rg -n '^ sig-approvers-executor:$' OWNERS_ALIASES
exit "$status"Repository: pingcap/tidb
Length of output: 203
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
echo "== Files under pkg/executor =="
git ls-files 'pkg/executor/*' | awk -F/] '!/^pkg\/executor\/OWNERS$/{ print }' | sort
echo
echo "== pkg/executor/OWNERS =="
if [ -f pkg/executor/OWNERS ]; then cat -n pkg/executor/OWNERS; else echo "missing"; fi
echo
echo "== matching sig-approvers-executor in OWNERS_ALIASES =="
rg -n '^ sig-approvers-executor:' OWNERS_ALIASES || true
echo
echo "== direct leaf files under pkg/executor (non-OWNERS, non-covering) =="
python3 - <<'PY'
import subprocess
for line in subprocess.check_output(["git","ls-files","pkg/executor/*"], text=True).splitlines():
parts=line.strip().split("/")
if len(parts) != 3:
continue
owner=parts[-1]
if owner in {"OWNERS"}:
continue
if owner == "BUILD.bazel":
continue
if owner.startswith("analyze.") and owner.endswith(".go"):
continue
print(line.strip())
PYRepository: pingcap/tidb
Length of output: 20620
Add a fallback approver for direct executor files.
pkg/executor/OWNERS sets no_parent_owners: true, and the filters cover only BUILD.bazel plus analyze*.go. Direct files such as pkg/executor/adapter.go, pkg/executor/admin.go, and pkg/executor/batch_checker.go then have no direct approver. Add sig-approvers-executor as a catch-all fallback for direct executor files.
Proposed fallback rule
"analyze.*\\.go$":
approvers:
- sig-approvers-stats
+ ".*":
+ approvers:
+ - sig-approvers-executor📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| filters: | |
| "(^|/)BUILD\\.bazel$": | |
| approvers: | |
| - sig-community-approvers | |
| "analyze.*\\.go$": | |
| approvers: | |
| - sig-approvers-stats | |
| filters: | |
| "(^|/)BUILD\\.bazel$": | |
| approvers: | |
| - sig-community-approvers | |
| "analyze.*\\.go$": | |
| approvers: | |
| - sig-approvers-stats | |
| ".*": | |
| approvers: | |
| - sig-approvers-executor |
🤖 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 4 - 10, Add a catch-all approver rule in
pkg/executor/OWNERS for files not matched by the existing BUILD.bazel and
analyze*.go filters, assigning sig-approvers-executor while preserving those
specific rules and no_parent_owners behavior.
50c2e46 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