pagure-dist-git

#88 Drop two early return statements that by-pass the check for PR only
Merged by pingou. Opened by pingou.
fix_pr_only  into  master

Download 88.patch

Basically, we were returning True before we had a chance to check if
the project allowed to direct commits vs enforcing a PR only workflow.
RCM and releng are not concerned by this though.

Signed-off-by: Pierre-Yves Chibon pingou@pingoured.fr

rebased onto a99a426ce05121d543db709457d531ded90dc143

I don't understand the code in question deeply enough, but I can test this on staging. Let me know once it is there.

1 new commit added

  • Expand on the documentation in the code

2 new commits added

  • Expand on the documentation in the code
  • Drop two early return statements that by-pass the check for PR only

spaces on a blank line?

I don't understand the code in question deeply enough, but I can test this on staging. Let me know once it is there.

It is already there :)

removed yes, my IDE does it automatically on save :]

:thumbsup: 

[git (master)]$ git commit --allow-empty
[master c4d96d5] Test 123
[git (master)]$ git remote -v
origin  ssh://churchyard@pkgs.stg.fedoraproject.org/rpms/git.git (fetch)
origin  ssh://churchyard@pkgs.stg.fedoraproject.org/rpms/git.git (push)
[git (master)]$ git push 
Enumerating objects: 1, done.
Counting objects: 100% (1/1), done.
Writing objects: 100% (1/1), 188 bytes | 188.00 KiB/s, done.
Total 1 (delta 0), reused 0 (delta 0)
remote: Performing pre-check...
remote: Pre-check results in
remote: Welcome to repoSpanner 0.5+11.c6a026e428baaa9b34c8fd54a6f1c0a6f2ae062e.el7.infra, node fedora01.rpms.stg.fedoraproject.org
remote: Delta resolving finished
remote: Validating objects...
remote: Objects validated
remote: Finishing hook runner preparation...
remote: Telling hook runner to grab new contents...
remote: Running pre-receive hook...
remote: Protected namespaces: ['rpms', 'modules', 'container']
remote: Blocking unspecified refs: False
remote: Blacklists: [<_sre.SRE_Pattern object at 0x7fad9d7de7b0>]
remote: User: User: 232 - name churchyard
remote: User groups: set([u'modularity-wg', u'provenpackager', u'packager', u'cvsadmin', u'python-sig'])
remote: Committer: True
remote: SIG memberships: set([])
remote: RCM: False
remote: Branch refs/heads/master is supported
remote: Unspecified branch push
remote: A pull request is required for this branch
remote: Denied push for ref 'refs/heads/master' for user 'churchyard'
remote: All changes have been rejected
remote: Hook returned error
remote: ERR Pre-receive hook refused push
error: remote unpack failed: FAIL: Pre-receive hook refused push
To ssh://pkgs.stg.fedoraproject.org/rpms/git.git
 ! [remote rejected] master -> master (unpack FAIL: Pre-receive hook refused push)
error: failed to push some refs to 'ssh://churchyard@pkgs.stg.fedoraproject.org/rpms/git.git'

It seems to work. However do, cvsadmins get an exception? I'm fine if we don't.

Also, the error doesn't really seem to be actionable.

do, cvsadmins get an exception? I'm fine if we don't.

Yes they do, releng/cvsadmins must be able to push at all time :)

Also, the error doesn't really seem to be actionable.

It's definitely a bit hidden in the verbose output but it's there:
remote: A pull request is required for this branch

Yes they do, releng/cvsadmins must be able to push at all time :)

In that case it doesn't work :(

Also, the error doesn't really seem to be actionable.

It's definitely a bit hidden in the verbose output but it's there:
remote: A pull request is required for this branch

Oh, right, sorry about that. Indeed it is hidden quite well.
Edited by churchyard

Yes they do, releng/cvsadmins must be able to push at all time :)

In that case it doesn't work :(

Are you in that group in staging? (FAS staging) if you are, could you try again? I've adjusted the configuration a bit.

Is this good enough?

remote: User groups: set([u'modularity-wg', u'provenpackager', u'packager', u'cvsadmin', u'python-sig'])

Trying again.

 
$ git push 
Enumerating objects: 1, done.
Counting objects: 100% (1/1), done.
Writing objects: 100% (1/1), 188 bytes | 188.00 KiB/s, done.
Total 1 (delta 0), reused 0 (delta 0)
remote: Performing pre-check...
remote: Pre-check results in
remote: Welcome to repoSpanner 0.5+11.c6a026e428baaa9b34c8fd54a6f1c0a6f2ae062e.el7.infra, node fedora01.rpms.stg.fedoraproject.org
remote: Delta resolving finished
remote: Validating objects...
remote: Objects validated
remote: Finishing hook runner preparation...
remote: Telling hook runner to grab new contents...
remote: Running pre-receive hook...
remote: Protected namespaces: ['rpms', 'modules', 'container']
remote: Blocking unspecified refs: False
remote: Blacklists: [<_sre.SRE_Pattern object at 0x7f3c3c9187b0>]
remote: User: User: 232 - name churchyard
remote: User groups: set([u'modularity-wg', u'provenpackager', u'packager', u'cvsadmin', u'python-sig'])
remote: Committer: True
remote: SIG memberships: set([])
remote: RCM: False
remote: Branch refs/heads/master is supported
remote: Unspecified branch push
remote: A pull request is required for this branch
remote: Denied push for ref 'refs/heads/master' for user 'churchyard'
remote: All changes have been rejected
remote: Hook returned error
remote: ERR Pre-receive hook refused push
error: remote unpack failed: FAIL: Pre-receive hook refused push
To ssh://pkgs.stg.fedoraproject.org/rpms/git.git
 ! [remote rejected] master -> master (unpack FAIL: Pre-receive hook refused push)
error: failed to push some refs to 'ssh://churchyard@pkgs.stg.fedoraproject.org/rpms/git.git'

Are you in that group in staging? (FAS staging)

Just checked, you are :)

remote: RCM: False

wich comes from

https://pagure.io/pagure-dist-git/blob/a99a426ce05121d543db709457d531ded90dc143/f/dist_git_auth.py#_190:

is_rcm = bool(self.rcm_group and self.rcm_group in usergroups)

and rcm_group...

self.rcm_group = pagure_config.get("RCM_GROUP")

and RCM_GROUP... https://infrastructure.fedoraproject.org/cgit/ansible.git/tree/roles/distgit/pagure/templates/pagure_shared.cfg#n69:

RCM_GROUP = 'relenggroup'

so, looking code and config on ansible, cvsadmin should be blocked, and it is

and we'll need to touch the is_rcm check to modify it to accept lists if cvsadmin should be able to push on this case
Edited by jlanda

1 new commit added

  • Allow RCM/releng to by-pass a PR-only workflow

3 new commits added

  • Allow RCM/releng to by-pass a PR-only workflow
  • Expand on the documentation in the code
  • Drop two early return statements that by-pass the check for PR only

:thumbsup:

Thanks for the review @jlanda and your help testing it in staging @churchyard !

Pull-Request has been merged by pingou
Metadata
None
No Tags
Static archive. pagure.io was sunset in 2026; this is a read-only snapshot.
Red Hat Logo

Fedora is sponsored by Red Hat. Learn more about the relationship between Red Hat and Fedora »

© Red Hat, Inc. and others.