Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

prow: allow configuring the filenames used for OWNERS(_ALIASES) #20482

Merged

Conversation

stevekuznetsov
Copy link
Contributor

When a repository is a fork of an upstream repository with OWNERS, or
vendors a repository that also uses OWNERS, it is generally not intended
for users who are OWNERS upstream to have rights in the downstream.
Furthermore, changing the OWNERS files leads to merge conflicts as
changes from upstream are pulled in, which leads to headaches. Allowing
users to configure the files used by Prow to denote owners and aliases
side-steps this issue.

Signed-off-by: Steve Kuznetsov skuznets@redhat.com

/assign @alvaroaleman @cjwagner

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jan 14, 2021
@k8s-ci-robot k8s-ci-robot added area/prow Issues or PRs related to prow area/prow/plugins Issues or PRs related to prow's plugins for the hook component sig/testing Categorizes an issue or PR as relevant to SIG Testing. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Jan 14, 2021
@stevekuznetsov stevekuznetsov force-pushed the skuznets/configure-owners branch 2 times, most recently from abf0dcb to df00caa Compare January 14, 2021 22:26
@stevekuznetsov
Copy link
Contributor Author

Seventeen tries later I think bazel is finally happy ...

@cjwagner
Copy link
Member

The owners_dir_blacklist configuration field is designed to address this issue:

// OwnersDirBlacklist is used to configure regular expressions matching directories
// to ignore when searching for OWNERS{,_ALIAS} files in a repo.
OwnersDirBlacklist OwnersDirBlacklist `json:"owners_dir_blacklist,omitempty"`

Can that satisfy your use case? I'd prefer to avoid making the OWNERS file names configurable. I'd rather have consistency across repos so that repo admins can choose whether OWNERS files sourced from other repos should apply (and even allow some to apply while ignoring others).

@stevekuznetsov
Copy link
Contributor Author

@cjwagner no, in the case of a fork that's maintained actively, it's required that the entire codebase be changed (not only vendor) and that sub-directories, etc, in the codebase have reviewers and approvers as needed.

@k8s-ci-robot k8s-ci-robot added the area/prow/hook Issues or PRs related to prow's hook component label Jan 15, 2021
@stevekuznetsov stevekuznetsov force-pushed the skuznets/configure-owners branch 3 times, most recently from 412f316 to ffbd62f Compare January 18, 2021 16:13
When a repository is a fork of an upstream repository with OWNERS, or
vendors a repository that also uses OWNERS, it is generally not intended
for users who are OWNERS upstream to have rights in the downstream.
Furthermore, changing the OWNERS files leads to merge conflicts as
changes from upstream are pulled in, which leads to headaches. Allowing
users to configure the files used by Prow to denote owners and aliases
side-steps this issue.

Signed-off-by: Steve Kuznetsov <skuznets@redhat.com>
@stevekuznetsov
Copy link
Contributor Author

@alvaroaleman updated YAML

@stevekuznetsov
Copy link
Contributor Author

@cjwagner @alvaroaleman PTAL

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jan 21, 2021
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: alvaroaleman, stevekuznetsov

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot merged commit 548289b into kubernetes:master Jan 21, 2021
@k8s-ci-robot k8s-ci-robot added this to the v1.21 milestone Jan 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. area/prow/hook Issues or PRs related to prow's hook component area/prow/plugins Issues or PRs related to prow's plugins for the hook component area/prow Issues or PRs related to prow cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. sig/testing Categorizes an issue or PR as relevant to SIG Testing. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants