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

Automate re-run/abort permission based on OWNERS file #307

Open
smg247 opened this issue Oct 22, 2024 · 6 comments
Open

Automate re-run/abort permission based on OWNERS file #307

smg247 opened this issue Oct 22, 2024 · 6 comments
Labels
kind/feature Categorizes issue or PR as related to a new feature.

Comments

@smg247
Copy link
Contributor

smg247 commented Oct 22, 2024

It is currently possible to configure permissions for users and teams to re-run or abort jobs based on the DefaultReRunAuthConfigs defined in the main prow config. This configuration is not sharded, so it cannot be split into the sharded repos' configurations. It also requires manual update to keep in sync with repo owners.

I propose that there be a new boolean field to dictate that users found in OWNERS files (possibly just approvers) be automatically, implicitly populated in the config for the respective repo. This would allow for re-run and abort privileges for all owners of the individual repo, and reduce support burden on the prow maintaining team(s).

@smg247
Copy link
Contributor Author

smg247 commented Oct 22, 2024

/kind feature

@k8s-ci-robot k8s-ci-robot added the kind/feature Categorizes issue or PR as related to a new feature. label Oct 22, 2024
@petr-muller
Copy link
Contributor

I propose that there be a new boolean field to dictate that users found in OWNERS files (possibly just approvers) be automatically, implicitly populated in the config for the respective repo

  • We'd still need the config to be sharded, right? So that individual repos could opt-in.
  • I'd like to avoid adding booleans for the same reason kube API conventions discourages them - the ideas tend to evolve. Even for OWNERS there are immediately two options I see people ask for (or four, if you consider none and all): approvers and reviewers

But yeah, I think this is useful 👍

@smg247
Copy link
Contributor Author

smg247 commented Oct 25, 2024

We'd still need the config to be sharded, right? So that individual repos could opt-in.

yes, sharding the config is definitely required here.

I'd like to avoid adding booleans for the same reason kube API conventions discourages them - the ideas tend to evolve. Even for OWNERS there are immediately two options I see people ask for (or four, if you consider none and all): approvers and reviewers

Very good point, the configuration will require some more thought

@BenTheElder
Copy link
Member

I propose that there be a new boolean field to dictate that users found in OWNERS files (possibly just approvers) be automatically, implicitly populated in the config for the respective repo.

That sounds expensive and problematic, would prow clone every repo and walk all the OWNERS files? How often?
Kubernetes has hundreds of repos.

What about all of the periodic jobs that aren't necessarily linked to any particular repo? Is this only for postsubmits?

@smg247
Copy link
Contributor Author

smg247 commented Nov 7, 2024

That sounds expensive and problematic, would prow clone every repo and walk all the OWNERS files? How often?
Kubernetes has hundreds of repos.

Good point. I wasn't thinking about the Kubernetes instance there. The OpenShift instance already has a periodic job that syncs owners files from the respective repos into the respective prow config directories. In that case, it wouldn't come at any expense at all.

What about all of the periodic jobs that aren't necessarily linked to any particular repo? Is this only for postsubmits?

Again, the OpenShift instance includes the repo refs for periodics in the extra_refs. So we would be able to determine this.

It is sounding like this feature may be better off to be downstream given these missing extra details that exist in OpenShift...

@BenTheElder
Copy link
Member

Good point. I wasn't thinking about the Kubernetes instance there. The OpenShift instance already has a periodic job that syncs owners files from the respective repos into the respective prow config directories. In that case, it wouldn't come at any expense at all.

Ah, yeah that's definitely not a standard aspect of Prow, hadn't seen that before. I don't think we'd want to build prow features that had a strong dependency on this.

Again, the OpenShift instance includes the repo refs for periodics in the extra_refs. So we would be able to determine this.

That's just part of prow, BUT, it's entirely possible to run testing where you don't clone any repos.

Kubernetes does this by having one periodic publishing builds and other periodics consuming those builds.
Which is a BIG compute savings, but means a LOT of periodics aren't cloning the repo under test (and might even be cloning another repo with some secondary tooling) so the repo refs are at best a weak indicator.


I do think we need better ability to grant rerun permissions, FWIW. Not sure what a practical version of that looks like though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature Categorizes issue or PR as related to a new feature.
Projects
None yet
Development

No branches or pull requests

4 participants