pagure-dist-git

Clone
Source Code
GIT

#24 Add support for the provenpackager group in pagure-dist-git
Merged 6 years ago by pingou. Opened 6 years ago by pingou.
provenpackager  into  master

file modified
+21 -17 
@@ -27,6 +27,8 @@ 
 

  from pagure.lib.git_auth import Gitolite3Auth, _read_file  # noqa: E402
 

  

  

+ _NOT_PROVENPACKAGER = ['rpms/firefox', 'rpms/thunderbird', 'rpms/xulrunner']
 

+ 

  _log = logging.getLogger(__name__)
 

  

  cache = dogpile.cache.make_region().configure(
 
@@ -116,32 +118,34 @@ 
 

              if project.is_fork:
 

                  access = 'RW+C'
 

  

+             users = sorted(set([project.user.user]).union(
 

+                 set(
 

+                     user.user
 

+                     for user in project.committers
 

+                     if user != project.user)))
 

+             groups = sorted(set(
 

+                 group.group_name
 

+                 for group in project.committer_groups
 

+             ))
 

+ 

              if repos == '' and not project.is_fork:
 

+                 if project.fullname not in _NOT_PROVENPACKAGER:
 

+                     groups.append('provenpackager')
 

                  # First, whitelist the supported branches from PDC
 

                  for branch in get_supported_branches(
 

                          project.namespace, project.name):
 

                      config.append('  %s %s = %s' % (
 

-                         access, branch, project.user.user))
 

-                     for user in project.committers:
 

-                         if user != project.user:
 

-                             config.append('  %s %s = %s' % (
 

-                                 access, branch, user.user))
 

+                         access, branch, ' '.join(users)
 

+                         )
 

+                     )
 

  

                  # Then, blacklist a pattern over that (after).
 

                  config.append(_blacklist)
 

  

-             if project.committer_groups:
 

-                 config.append('  %s = @%s' % (access, ' @'.join(
 

-                     [
 

-                         group.group_name
 

-                         for group in project.committer_groups
 

-                     ]
 

-                 )))
 

- 

-             config.append('  %s = %s' % (access, project.user.user))
 

-             for user in project.committers:
 

-                 if user != project.user:
 

-                     config.append('  %s = %s' % (access, user.user))
 

+             if groups:
 

+                 config.append('  %s = @%s' % (access, ' @'.join(groups)))
 

+ 

+             config.append('  %s = %s' % (access, ' '.join(users)))
 

  

              for deploykey in project.deploykeys:
 

                  access = 'R'

file modified
+109 -10 
@@ -24,6 +24,7 @@ 
 

    -    epel[0-9][0-9] = @all
 

    -    el[0-9] = @all
 

    -    olpc[0-9] = @all
 

+   RWC = @provenpackager
 

    RWC = pingou
 

  

  repo requests/test
 
@@ -38,6 +39,7 @@ 
 

    -    epel[0-9][0-9] = @all
 

    -    el[0-9] = @all
 

    -    olpc[0-9] = @all
 

+   RWC = @provenpackager
 

    RWC = pingou
 

  

  repo requests/test2
 
@@ -52,6 +54,7 @@ 
 

    -    epel[0-9][0-9] = @all
 

    -    el[0-9] = @all
 

    -    olpc[0-9] = @all
 

+   RWC = @provenpackager
 

    RWC = pingou
 

  

  repo requests/somenamespace/test3
 
@@ -170,6 +173,7 @@ 
 

    -    epel[0-9][0-9] = @all
 

    -    el[0-9] = @all
 

    -    olpc[0-9] = @all
 

+   RWC = @provenpackager
 

    RWC = pingou
 

  

  repo requests/test2
 
@@ -184,6 +188,7 @@ 
 

    -    epel[0-9][0-9] = @all
 

    -    el[0-9] = @all
 

    -    olpc[0-9] = @all
 

+   RWC = @provenpackager
 

    RWC = pingou
 

  

  repo requests/somenamespace/test3
 
@@ -191,28 +196,25 @@ 
 

  

  repo test
 

    R   = @all
 

-   RWC master = foo
 

-   RWC master = pingou
 

-   RWC f9000 = foo
 

-   RWC f9000 = pingou
 

+   RWC master = foo pingou
 

+   RWC f9000 = foo pingou
 

    -    f[0-9][0-9] = @all
 

    -    epel[0-9] = @all
 

    -    epel[0-9][0-9] = @all
 

    -    el[0-9] = @all
 

    -    olpc[0-9] = @all
 

-   RWC = foo
 

-   RWC = pingou
 

+   RWC = @provenpackager
 

+   RWC = foo pingou
 

  

  repo requests/test
 

-   RWC = foo
 

-   RWC = pingou
 

+   RWC = foo pingou
 

  

  # end of body'''
 

          self.assertMultiLineEqual(expected, contents.strip())
 

  

      def test_get_supported_branches(self):
 

          """ Test for real what is returned by PDC. """
 

-         expected = ['master', 'f26', 'f25', 'el6']
 

+         expected = ['master', 'f27', 'f26', 'f25', 'el6']
 

          actual = dist_git_auth.get_supported_branches('rpms', 'nethack')
 

          self.assertEquals(set(actual), set(expected))
 

  
@@ -277,6 +279,7 @@ 
 

    -    epel[0-9][0-9] = @all
 

    -    el[0-9] = @all
 

    -    olpc[0-9] = @all
 

+   RWC = @provenpackager
 

    RWC = pingou
 

  

  repo requests/test2
 
@@ -291,6 +294,7 @@ 
 

    -    epel[0-9][0-9] = @all
 

    -    el[0-9] = @all
 

    -    olpc[0-9] = @all
 

+   RWC = @provenpackager
 

    RWC = pingou
 

  

  repo requests/somenamespace/test3
 
@@ -305,7 +309,7 @@ 
 

    -    epel[0-9][0-9] = @all
 

    -    el[0-9] = @all
 

    -    olpc[0-9] = @all
 

-   RWC = @test_grp
 

+   RWC = @test_grp @provenpackager
 

    RWC = foo
 

  

  repo requests/test
 
@@ -357,6 +361,7 @@ 
 

    -    epel[0-9][0-9] = @all
 

    -    el[0-9] = @all
 

    -    olpc[0-9] = @all
 

+   RWC = @provenpackager
 

    RWC = pingou
 

  

  repo requests/test
 
@@ -371,6 +376,7 @@ 
 

    -    epel[0-9][0-9] = @all
 

    -    el[0-9] = @all
 

    -    olpc[0-9] = @all
 

+   RWC = @provenpackager
 

    RWC = pingou
 

  

  repo requests/test2
 
@@ -385,6 +391,7 @@ 
 

    -    epel[0-9][0-9] = @all
 

    -    el[0-9] = @all
 

    -    olpc[0-9] = @all
 

+   RWC = @provenpackager
 

    RWC = pingou
 

  

  repo requests/somenamespace/test3
 
@@ -399,3 +406,95 @@ 
 

  

  # end of body'''
 

          self.assertMultiLineEqual(expected, contents.strip())
 

+ 

+     @mock.patch('dist_git_auth.get_supported_branches')
 

+     def test_write_gitolite_acls_rpms_firefox(self, get_supported_branches):
 

+         """ Test generating the entire gitolite configuration file
 

+         with the firefox project in the rpms namespace (ie a project not
 

+         allowing provenpackager access).
 

+ 

+         """
 

+         get_supported_branches.return_value = ['master', 'f9000']
 

+         print("Initializing DB.")
 

+         item = pagure.lib.model.Project(
 

+             user_id=1,  # pingou
 

+             name='firefox',
 

+             description='The firefox project',
 

+             hook_token='aaabbbeee',
 

+             namespace='rpms',
 

+         )
 

+         self.session.add(item)
 

+         self.session.commit()
 

+ 

+         print("Generating %r" % self.configfile)
 

+         dist_git_auth.DistGitoliteAuth.write_gitolite_acls(
 

+             self.session,
 

+             configfile=self.configfile,
 

+             project=-1)
 

+ 

+         print("Checking the contents of %r" % self.configfile)
 

+         with open(self.configfile, 'r') as f:
 

+             contents = f.read()
 

+         expected = """repo rpms/firefox
 

+   R   = @all
 

+   RWC master = pingou
 

+   RWC f9000 = pingou
 

+   -    f[0-9][0-9] = @all
 

+   -    epel[0-9] = @all
 

+   -    epel[0-9][0-9] = @all
 

+   -    el[0-9] = @all
 

+   -    olpc[0-9] = @all
 

+   RWC = pingou
 

+ 

+ repo requests/rpms/firefox
 

+   RWC = pingou
 

+ 

+ # end of body
 

+ """
 

+         self.assertMultiLineEqual(contents.strip(), expected.strip())
 

+ 

+     @mock.patch('dist_git_auth.get_supported_branches')
 

+     def test_write_gitolite_acls_firefox(self, get_supported_branches):
 

+         """ Test generating the entire gitolite configuration file
 

+         with the firefox project.
 

+ 

+         """
 

+         get_supported_branches.return_value = ['master', 'f9000']
 

+         print("Initializing DB.")
 

+         item = pagure.lib.model.Project(
 

+             user_id=1,  # pingou
 

+             name='firefox',
 

+             description='The firefox project',
 

+             hook_token='aaabbbeee',
 

+             namespace=None,
 

+         )
 

+         self.session.add(item)
 

+         self.session.commit()
 

+ 

+         print("Generating %r" % self.configfile)
 

+         dist_git_auth.DistGitoliteAuth.write_gitolite_acls(
 

+             self.session,
 

+             configfile=self.configfile,
 

+             project=-1)
 

+ 

+         print("Checking the contents of %r" % self.configfile)
 

+         with open(self.configfile, 'r') as f:
 

+             contents = f.read()
 

+         expected = """repo firefox
 

+   R   = @all
 

+   RWC master = pingou
 

+   RWC f9000 = pingou
 

+   -    f[0-9][0-9] = @all
 

+   -    epel[0-9] = @all
 

+   -    epel[0-9][0-9] = @all
 

+   -    el[0-9] = @all
 

+   -    olpc[0-9] = @all
 

+   RWC = @provenpackager
 

+   RWC = pingou
 

+ 

+ repo requests/firefox
 

+   RWC = pingou
 

+ 

+ # end of body
 

+ """
 

+         self.assertMultiLineEqual(contents.strip(), expected.strip())

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

1 new commit added

  • Fix running the tests
6 years ago

1 new commit added

  • Adjust the logic to group the users and groups in a single line
6 years ago

You'll likely want to review this PR commit by commit

you don't really need to create list first, you can use generator..

set(x for x in y) works perfectly and in theory should eat less memory.

Good call, I used the same structure as above but wrongly here, thanks

1 new commit added

  • Drop using set([]) where not needed
6 years ago

rebased
6 years ago

You should instead just mock the PDC call out, because right now you are making the test suite depend on an external service, which might not stay around.

Or, as you noticed, suddenly return other data.

I agree but on the other side it allows ensuring the data structure doesn't suddenly change (and it's a single test that will break twice a year or so)

Well, if it changes that'd be a bug in PDC, not Pagure.

Agreed but I'd rather detect this here in the test suite than in prod (though prod will likely break before we run the tests here ^^)

Note that the other tests are mocking the call to PDC so if we were mocking it here as well as would basically be testing the mock works :)

Note that as far as I can find, there has not yet been a conclusion in the "should provenpackagers be able to commit to firefox/xulrunner/thunderbird repos" discussion (https://pagure.io/fesco/issue/1562).
We should not be making this decision unilaterally, but await FESCo's decision on this.

If you can find a record of this decision having been made, that would be great to add here.

1 new commit added

  • Some package do not allow provenpackager access
6 years ago

Adjusted :)

This code looks fine, although I still think you should mock out PDC, but that can be a different PR.

Thanks for the review! :)

Pull-Request has been merged by pingou
6 years ago

Thank you for the quick response. I'll push out the changes to those packages soonish (hopefully later today) and thus give it a test.
Metadata
None
No Tags
Changes Summary 2
+21 -17
file changed
dist_git_auth.py
+109 -10
file changed
dist_git_auth_tests.py
Powered by Pagure 5.13.3
Documentation • File an Issue • About • SSH Hostkey/Fingerprint
© Red Hat, Inc. and others.