search

kubernetes-sigs / prow

Prow is a Kubernetes based CI/CD system developed to serve the Kubernetes community. This repository contains Prow source code and Hugo sources for Prow documentation site.
https://docs.prow.k8s.io
Apache License 2.0
129 stars 99 forks source link

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

Open smg247 opened 1 month ago

smg247 commented 1 month ago

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 commented 1 month ago

/kind feature

petr-muller commented 4 weeks ago

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

But yeah, I think this is useful :+1:

smg247 commented 3 weeks ago

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 commented 2 weeks ago

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 commented 2 weeks ago

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 commented 2 weeks ago

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.