#9635 Fix the URL and method used to remove someone's ACL in dist-git
Merged 5 months ago by humaton. Opened 5 months ago by pingou.
pingou/releng master  into  master

@@ -138,6 +138,18 @@ 

      return level

  

  

+ def get_bugzilla_overrides(username, namespace, name):

+     """ Returns whether the specified username is set in the bugzilla overrides

+     of the specified package.. """

+     _log.debug(

+         "Checking for bugzilla overrides on %s/%s for %s", namespace, name, username

+     )

+     base_url = dist_git_base.rstrip("/")

+     session = retry_session()

+     req = session.get(f"{dist_git_base}/_dg/bzoverrides/{namespace}/{name}")

+     return req.json()

+ 

+ 

  def unwatch_package(namespace, name, username):

      """ Reset the watch status of the given user on the specified project. """

      _log.debug("Going to reset watch status of %s on %s/%s", username, namespace, name)
@@ -195,14 +207,14 @@ 

      session = retry_session()

  

      # Remove ACL on the package

-     url = f"{base_url}/{namespace}/{name}/git/modifyacls"

+     url = f"{base_url}/api/0/{namespace}/{name}/git/modifyacls"

      headers = {"Authorization": f"token {pagure_token}"}

      data = {

          "user_type": usertype,

          "name": username,

      }

  

-     req = session.patch(url, data=data, headers=headers)

+     req = session.post(url, data=data, headers=headers)

      if not req.ok:

          print("**** REQUEST FAILED")

          print("  - Remove ACL")
@@ -218,6 +230,33 @@ 

          unwatch_package(namespace, name, username)

  

  

+ def reset_bugzilla_overrides(username, namespace, name, overrides):

+     """ Reset the the bugzilla overrides of the specified package so that the

+     specified user no longer has one. """

+     _log.debug(

+         "Resetting bugzilla overrides on %s/%s for %s", namespace, name, username

+     )

+     base_url = dist_git_base.rstrip("/")

+     url = f"{base_url}/_dg/bzoverrides/{namespace}/{name}"

+     headers = {"Authorization": f"token {pagure_token}"}

+ 

+     for key in overrides:

+         if overrides[key] == username:

+             overrides[key] = None

+ 

+     session = retry_session()

+     req = session.post(url, headers=headers, data=overrides)

+     if not req.ok:

+         print("**** REQUEST FAILED")

+         print("  - Remove bugzilla overrides")

+         print(req.url)

+         print(data)

+         print(req.text)

+     else:

+         print(f"  {username} has no longer a bugzilla overrides on {namespace}/{name}")

+     session.close()

+ 

+ 

  def main(args):

      """ For the specified list of users, retrieve what they are maintaining

      or watching in dist-git."""
@@ -253,29 +292,55 @@ 

          _log.debug("Loading usernames for the CLI arguments")

          usernames = args.usernames

  

+     # We load the info from the pagure_bz file which will tell us everything

+     # that would be synced to bugzilla (POC and CC)

      _log.debug("Loading info from dist-git's pagure_bz.json file")

      session = retry_session()

      req = session.get(f"{dist_git_base}/extras/pagure_bz.json")

      pagure_bz = req.json()

      session.close()

  

-     packages_per_user = collections.defaultdict(list)

+     packages_per_user = collections.defaultdict(set)

      for namespace in pagure_bz:

          for package in pagure_bz[namespace]:

              _log.debug("Processing %s/%s", namespace, package)

              for user in pagure_bz[namespace][package]:

                  if user in usernames:

-                     packages_per_user[user].append(f"{namespace}/{package}")

+                     packages_per_user[user].add(f"{namespace}/{package}")

+ 

+     # On the top of this, we'll also query the list from dist-git directly as

+     # the previous source of info while quicker to query will not include

+     # the packages that the packagers have access to but set their watch status

+     # to "unwatch".

+     # However, we only need to run this if we want to know about packages someone

+     # maintains (ie: we can bypass this section if ``--watch`` is passed to the

+     # CLI).

+     if args.report in ["all", "maintain"]:

+         for username in sorted(usernames):

+             _log.debug("Loading info from dist-git's %s's page", username)

+             url = f"{dist_git_base}/api/0/user/{username}?per_page=50"

+             while url:

+                 req = session.get(url)

+                 data = req.json()

+                 for repo in data["repos"]:

+                     maintainers = set(repo["user"]["name"])

+                     for acl in repo["access_users"]:

+                         maintainers.update(set(repo["access_users"][acl]))

+                     if username in maintainers:

+                         namespace = repo["namespace"]

+                         package = repo["name"]

+                         packages_per_user[username].add(f"{namespace}/{package}")

+                 url = data["repos_pagination"]["next"]

  

      for username in sorted(usernames):

          _log.debug("Processing user: %s", username)

          for pkg in sorted(packages_per_user[username]):

              level = user_access(session, username, pkg)

+             namespace, name = pkg.split("/", 1)

              if level:

                  if args.report in ["all", "maintain"]:

-                     print(f"{username} is {level} of {pkg}")

+                     print(f"{username} is {level} of {namespace}/{name}")

                      if args.retire:

-                         namespace, name = pkg.split("/", 1)

                          if level == "main admin":

                              orphan_package(namespace, name, username)

                          elif level == "maintainer":
@@ -283,10 +348,16 @@ 

  

              else:

                  if args.report in ["all", "watch"]:

-                     print(f"{username} is watching {pkg}")

+                     print(f"{username} is watching {namespace}/{name}")

                      if args.retire:

-                         namespace, name = pkg.split("/", 1)

                          unwatch_package(namespace, name, username)

+ 

+             overrides = get_bugzilla_overrides(username, namespace, name)

+             if username in overrides.values():

+                 print(f"{username} has a bugzilla override on {namespace}/{name}")

+                 if args.retire:

+                     reset_bugzilla_overrides(username, namespace, name, overrides)

+ 

          print()

  

  

This should have been fixed in the original PR but apparently it
was merged before my push took effect

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

This should fix the error that @churchyard was seeing in the original PR.

I've since found an small oddity which I'm investigating, so feel free to merge this PR but don't be surprised if another one shows up :)

1 new commit added

  • When querying the list of packages maintained by people, also query dist-git itself
5 months ago

There is now a second commit in this PR that fixes the oddity I had found. Basically if someone had "unwatch" a package, we were not taking it into account (since that wouldn't be synced to bugzilla).

This will make the script a little slower, but more reliable.

1 new commit added

  • Small optimization: do not query dist-git if we're only interested in packages watched
5 months ago

I plan to test this and report back.

This fixes the problem I had before.

Is seems that the bugzilla json is generated cron-ish-ly, so subsequent runs with --retire still report that the users are watching things. I can recheck again once the json is renewed (when is that?).

As a matter of completeness I think the script could also remove the user from bugzilla assignee overrides.

Is seems that the bugzilla json is generated cron-ish-ly, so subsequent runs with --retire still report that the users are watching things. I can recheck again once the json is renewed (when is that?).

That is correct (it's in the description in the --help), the cron runs hourly.

As a matter of completeness I think the script could also remove the user from bugzilla assignee overrides.

Good idea, I'll see to that

Waiting for at least an hour to see the results.

1 new commit added

  • Check if the packager has a bugzilla overrides and if so, reset it
5 months ago

Support for bugzilla overrides added, but we should double-check that it changes only the override of interest, leaving the other one as is (e.i: check that if we retire user "bar" on Fedora, we don't change the override to "foo" on Fedora EPEL)

This return seem bogus. The variable is not defined.

Waiting for at least an hour to see the results.

Works.

Sigh, I shouldn't rename variables before pushing...

4 new commits added

  • Check if the packager has a bugzilla overrides and if so, reset it
  • Small optimization: do not query dist-git if we're only interested in packages watched
  • When querying the list of packages maintained by people, also query dist-git itself
  • Fix the URL and method used to remove someone's ACL in dist-git
5 months ago

@churchyard comments fixed (and hopefully code as well :))

Pull-Request has been merged by humaton

5 months ago
Metadata