#8895 Grant @churchyard (temporary) permissions to push to "PR only" repos at src.fp.o
Closed: Fixed 3 years ago by pingou. Opened 3 years ago by churchyard.

Describe what you would like us to do:

src.fp.o has the ability to protect direct pushes (Pull Requests required mode).
I love that feature and use it myself, since we sometimes have extensive CI/PR workflow.

However, as a member of both provenpackager and cvsadmin groups, I'd assume I can still override it somehow. At least releng has that kind of power when doing mass rebuilds commits.

In couple weeks, I'll do mass Python 3.9 bumps/commits. I'd like to be able to override this rule, at least during that action. Can you please help me with that?

When do you need this to be done by? (2020/05/20)



Hm, I'm not sure if this is something we support at the moment. I'll look into it.

Out of curiosity, do you have any idea of how many packages would be impacted by this?

Hm, I'm not sure if this is something we support at the moment. I'll look into it.

How does releng do it?

FWIW I believe in the past, provenpackagers were able to bypass this feature.

Out of curiosity, do you have any idea of how many packages would be impacted by this?

No idea. At least a dozen, which I will be able to manually turn of and back on when puhsing, but if it is more, that'll be tedious.

If there is an APi call to:

  • check if PR-only is enabled
  • toggle the setting

I can incorporate that to the push script.

These API endpoints do exist if you're interested.

How does releng do it?

That's the cvsadmin group so you should be able to use it or it's broken for them as well

In that case it's broken now but it sued to work fine.

These API endpoints do exist if you're interested.

I plan to examine that option only if the "cvsadmins can do anything" approach s not feasible.

So the releng user is part of the relenggroup which is used to by-pass that restriction for mass-rebuilds.
Looking at the git history, I had changed it to rely on the cvsadmin group but @mohanboddu disagreed.

@mohanboddu what about adding the releng user to the cvsadmin group?

This would allow all the members of the cvsadmin group to do anything.

The alternative I can think of would be to adjust the code to support a list of groups instead of a single one, but considering the relenggroup has only one member it feels a little bit odd.

What do you think?

I am totally fine not being in such group for most of my regular packaging work and escalate myself via ticket when needed. That I also consider more transparent.

@mohanboddu what about adding the releng user to the cvsadmin group?

I didn't want people to get confused if they see random commits from different people, hence I suggested to only use it for releng user.

But if there is a use case, we can look at options.

IMHO, provenpackagers should just be allowed to bypass that option.

We have had a number of provenpackagers made wide changes over a collection of packages in one scripted thing... I think thats a great use of that power and we shouldn't block it.

ok, on further discussion with @pingou the setting there would let them bypass every check, which I do not think we want.

So now I am thinking perhaps we want to disallow the 'only pr' setting in src.fp.o ?

So now I am thinking perhaps we want to disallow the 'only pr' setting in src.fp.o ?

That would be a huge feature drop.

why? The team maintaining a package could normally always use PR's. There's already the mass rebuilds that are not PR's every cycle.

The only downsides I see:
You don't want provenpackagers to push direct commits ever. But that makes mass cleanups of things much harder.
Members of the team sometimes forget and push things and would like to be blocked?

Members of the team sometimes forget and push things and would like to be blocked?

That, mostly.

So after some discussion on pros and cons of the different approaches we thought about, we believe that a good solution will be to adjust pagure-dist-git to allow provenpackager to by-pass the PR-only setting.

It will force anyone that is not a provenpackager into the PR-only workflow if it was enabled while allowing provenpackager to commit directly which can be handy in case of broken composes and so on.

What do you think?

Best case solution IMHO: provenpackagers get busted, but can do --force. That's arguably hard to have.

What you say will basically get us back to what we had before this broke, hence I agree.

@churchyard I'm thinking to bring this to the devel list for feedback.

We considered:
- disabling entirely the PR-only setting
- allow provenpackager to by-pass PR-only
- replace relenggroup by cvsadmin in the config which basically by-passes all the checks (thus allowing cvsadmins to commit to orphan, retired packages, restrictred branches or namespaces)

In your case, all three options will have the same effect :)

Doing a --force option is kinda hard without touching git itself (or without forcing the use of fedpkg push).

I suggest to 1) revert to previously working behavior (provenpackagers can push even to the PR-only repos) and then 2) bring a discussion about long term solution to devel

Metadata Update from @mohanboddu:
- Issue priority set to: Waiting on Assignee (was: Needs Review)
- Issue tagged with: groomed, medium-gain, medium-trouble

3 years ago

ok, I am fine with that. @pingou can you work on this?

Metadata Update from @pingou:
- Issue assigned to pingou

3 years ago

The PR above has been reviewed by @nphilipp (Thanks!), merged, released in 1.7.0 which has been deployed in staging and production and tested there by both @churchyard and I (Thanks Miro!).

So from the point of view of this ticket, this is resolved and I will close it as such.

One last question @churchyard do you want to bring the topic to the devel list to find a longer term solution to this question, or shall I ?

Metadata Update from @pingou:
- Issue close_status updated to: Fixed
- Issue status updated to: Closed (was: Open)

3 years ago

One last question @churchyard do you want to bring the topic to the devel list to find a longer term solution to this question, or shall I ?

I am not available to summarize the problem and our options any time this week. I can put it to my long term todo.

I am not available to summarize the problem and our options any time this week. I can put it to my long term todo.

Then I'll draft something and share it with you. If your time allows and if you could skim through it before I send it to devel, it would be nice.

Skimming trough and commenting on a draft is more likely possible for me, thanks!

Login to comment on this ticket.

Metadata
Related Pull Requests
  • #108 Merged 3 years ago