koji

Clone

#2054 editSideTag API call
Merged 5 years ago by tkopecek. Opened 5 years ago by tkopecek.
tkopecek/koji issue1998  into  master

file modified
+8 
@@ -93,6 +93,14 @@ 
 

          # forbid everything else
 

          all :: deny
 

  

+     package_list =
 

+         # allow blocking for owners in their sidetags
 

+         match action block && is_sidetag_owner :: allow
 

+         all :: deny
 

+ 

+ There are two special policy tests `is_sidetag` and `is_sidetag_owner` with
 

+ expectable behaviour.
 

+ 

  Now Sidetag Koji plugin should be installed.  To verify that, run
 

  `koji list-api` command -- it should now display `createSideTag`
 

  as one of available API calls.

file modified
+41 -1 
@@ -10,7 +10,7 @@ 
 

  import koji
 

  from koji.plugin import export_cli
 

  from koji_cli.commands import anon_handle_wait_repo
 

- from koji_cli.lib import _, activate_session
 

+ from koji_cli.lib import _, activate_session, arg_filter
 

  

  

  @export_cli
 
@@ -93,3 +93,43 @@ 
 

  

      for tag in session.listSideTags(basetag=opts.basetag, user=user):
 

          print(tag["name"])
 

+ 

+ 

+ @export_cli
 

+ def handle_edit_sidetag(options, session, args):
 

+     "Edit sidetag"
 

+     usage = _("usage: %(prog)s edit-sidetag [options]")
 

+     usage += _("\n(Specify the --help global option for a list of other help options)")
 

+     parser = ArgumentParser(usage=usage)
 

+     parser.add_argument("sidetag", help="name of sidetag")
 

+     parser.add_argument("--debuginfo", action="store_true", default=None,
 

+                         help=_("Generate debuginfo repository"))
 

+     parser.add_argument("--no-debuginfo", action="store_false", dest="debuginfo")
 

+     parser.add_argument("--rpm-macro", action="append", default=[], metavar="key=value",
 

+                         dest="rpm_macros", help=_("Set tag-specific rpm macros"))
 

+     parser.add_argument("--remove-rpm-macro", action="append", default=[], metavar="key",
 

+                         dest="remove_rpm_macros", help=_("Remove rpm macros"))
 

+ 

+     opts = parser.parse_args(args)
 

+ 

+     if opts.debuginfo is None and not opts.rpm_macros and not opts.remove_rpm_macros:
 

+         parser.error("At least one option needs to be specified")
 

+ 

+     activate_session(session, options)
 

+ 

+     kwargs = {}
 

+     if opts.debuginfo is not None:
 

+         kwargs['debuginfo'] = opts.debuginfo
 

+ 

+     if opts.rpm_macros:
 

+         rpm_macros = {}
 

+         for xopt in opts.rpm_macros:
 

+             key, value = xopt.split('=', 1)
 

+             value = arg_filter(value)
 

+             rpm_macros[key] = value
 

+         kwargs['rpm_macros'] = rpm_macros
 

+ 

+     if opts.remove_rpm_macros:
 

+         kwargs['remove_rpm_macros'] = opts.remove_rpm_macros
 

+ 

+     session.editSideTag(opts.sidetag, **kwargs)

file modified
+93 -8 
@@ -4,6 +4,7 @@ 
 

  import sys
 

  

  import koji
 

+ import koji.policy
 

  from koji.context import context
 

  from koji.plugin import callback, export
 

  sys.path.insert(0, "/usr/share/koji-hub/")
 
@@ -13,17 +14,56 @@ 
 

      _create_tag,
 

      _delete_build_target,
 

      _delete_tag,
 

+     _edit_tag,
 

      assert_policy,
 

      get_build_target,
 

+     readInheritanceData,
 

      get_tag,
 

      get_user,
 

-     nextval
 

+     nextval,
 

+     policy_get_user
 

  )
 

  

  CONFIG_FILE = "/etc/koji-hub/plugins/sidetag.conf"
 

  CONFIG = None
 

  

  

+ def is_sidetag(taginfo, raise_error=False):
 

+     """Check, that given tag is sidetag"""
 

+     result = bool(taginfo['extra'].get('sidetag'))
 

+     if not result and raise_error:
 

+         raise koji.GenericError("Not a sidetag: %(name)s" % taginfo)
 

+ 

+ 

+ def is_sidetag_owner(taginfo, user, raise_error=False):
 

+     """Check, that given user is owner of the sidetag"""
 

+     result = (taginfo['extra'].get('sidetag') and
 

+               taginfo['extra'].get('sidetag_user_id') == user['id'])
 

+     if not result and raise_error:
 

+         raise koji.ActionNotAllowed("This is not your sidetag")
 

+ 

+ 

+ # Policy tests
 

+ class SidetagTest(koji.policy.MatchTest):
 

+     """Checks, if tag is a sidetag"""
 

+     name = 'is_sidetag'
 

+ 

+     def run(self, data):
 

+         tag = get_tag(data['tag'])
 

+         return is_sidetag(tag)
 

+ 

+ 

+ class SidetagOwnerTest(koji.policy.MatchTest):
 

+     """Checks, if user is a real owner of sidetag"""
 

+     name = 'is_sidetag_owner'
 

+ 

+     def run(self, data):
 

+         user = policy_get_user(data)
 

+         tag = get_tag(data['tag'])
 

+         return is_sidetag_owner(tag, user)
 

+ 

+ 

+ # API calls
 

  @export
 

  def createSideTag(basetag, debuginfo=False):
 

      """Create a side tag.
 
@@ -94,11 +134,9 @@ 
 

      sidetag = get_tag(sidetag, strict=True)
 

  

      # sanity/access
 

-     if not sidetag["extra"].get("sidetag"):
 

-         raise koji.GenericError("Not a sidetag: %(name)s" % sidetag)
 

-     if sidetag["extra"].get("sidetag_user_id") != user["id"]:
 

-         if not context.session.hasPerm("admin"):
 

-             raise koji.ActionNotAllowed("This is not your sidetag")
 

+     is_sidetag(sidetag, raise_error=True)
 

+     is_sidetag_owner(sidetag, user, raise_error=True)
 

+ 

      _remove_sidetag(sidetag)
 

  

  
@@ -170,6 +208,54 @@ 
 

      return query.execute()
 

  

  

+ @export
 

+ def editSideTag(sidetag, debuginfo=None, rpm_macros=None, remove_rpm_macros=None):
 

+     """Restricted ability to modify sidetags, parent tag must have:
 

+     sidetag_debuginfo_allowed: 1
 

+     sidetag_rpm_macros_allowed: 1
 

+     in extra, if modifying functions should work. For blocking/unblocking
 

+     further policy must be compatible with these operations.
 

+ 

+     :param sidetag: sidetag id or name
 

+     :type sidetag: int or str
 

+     :param debuginfo: set or disable debuginfo repo generation
 

+     :type debuginfo: bool
 

+     :param rpm_macros: add/update rpms macros in extra
 

+     :type rpm_macros: dict
 

+     :param remove_rpm_macros: remove rpm macros from extra
 

+     :type remove_rpm_macros: list of str
 

+     """
 

+ 

+     context.session.assertLogin()
 

+     user = get_user(context.session.user_id, strict=True)
 

+     sidetag = get_tag(sidetag, strict=True)
 

+ 

+     # sanity/access
 

+     is_sidetag(sidetag, raise_error=True)
 

+     is_sidetag_owner(sidetag, user, raise_error=True)
 

+ 

+     parent_id = readInheritanceData(sidetag['id'])[0]['parent_id']
 

+     parent = get_tag(parent_id)
 

+ 

+     if debuginfo is not None and not parent['extra'].get('sidetag_debuginfo_allowed'):
 

+         raise koji.GenericError("Debuginfo setting is not allowed in parent tag.")
 

+ 

+     if (rpm_macros is not None or remove_rpm_macros is not None) \
 

+             and not parent['extra'].get('sidetag_rpm_macros_allowed'):
 

+         raise koji.GenericError("RPM macros change is not allowed in parent tag.")
 

+ 

+     kwargs = {'extra': {}}
 

+     if debuginfo is not None:
 

+         kwargs['extra']['with_debuginfo'] = bool(debuginfo)
 

+     if rpm_macros is not None:
 

+         for macro, value in rpm_macros.items():
 

+             kwargs['extra']['rpm.macro.%s' % macro] = value
 

+     if remove_rpm_macros is not None:
 

+         kwargs['remove_extra'] = ['rpm.macro.%s' % m for m in remove_rpm_macros]
 

+ 

+     _edit_tag(sidetag['id'], **kwargs)
 

+ 

+ 

  def handle_sidetag_untag(cbtype, *args, **kws):
 

      """Remove a side tag when its last build is untagged
 

  
@@ -184,8 +270,7 @@ 
 

      if not tag:
 

          # also shouldn't happen, but just in case
 

          return
 

-     if not tag["extra"].get("sidetag"):
 

-         # not a side tag
 

+     if not is_sidetag(tag):
 

          return
 

      # is the tag now empty?
 

      query = QueryProcessor(

New API call for editing basic info on sidetags. Needs to be applied
with proper policies.

Fixes: https://pagure.io/koji/issue/1998

I'm not sure, that it is the way to go. You need to modify parent tag to explicitly allow debuginfo. It is ok, bigger problem is un/blocking which needs valid policy for this. I've sent extra to policy, so it can check, that it is sidetag, but it doesn't look neither easy, neither safe.

@mikem ?

Why do we want to have separate API call to emulate existing block-pkg/unblock-pkg?

Yep, it is ugly. I think that we can completely move this out and use normal commands, if we send more information to policy. (it is not the case for debuginfo)

For un/block I've no ability to compare, that calling user is sidetag owner. Not sure, how to easily workaround this than to have separate call.

pretty please pagure-ci rebuild
5 years ago

rebased onto fcd2048f8277e961cb8f148c94e01750a4815631
5 years ago

rebased onto d50bcc6430e62f79b5a40d2e7d44a8c1ed01758e
5 years ago

I've removed those lines and instead created plugin-specific policy test. So, if using sidetag plugin, you now can use also is_sidetag_owner policy check, which would enabled you to create policy rules for un/blocking packages based on user/tag, etc. combinations (no needs to change hub in this way). Does it look better?
Edited 5 years ago by tkopecek

Like 

match action block && tag f30-test-* && is_sidetag_owner :: allow

Would be also nice to have is_sidetag policy check, so that we can allow more things based on it.

1 new commit added

  • add is_sidetag policy test
5 years ago

Makes sense, added.

can we reuse policy checks here? I mean this code is basically duplicating what's being checked in policy function

Can't say much for the code, but the example of policy makes sense to me and I'd like to see it in Fedora.

I'm not sure if it can handle (un)block-pkg/(un)tag-pkg/regen-repo. If so, then I am super-happy.

Oh, and also set rpm macro for the tag. like a rpm.dist

1 new commit added

  • simplify checks
5 years ago

1 new commit added

  • edit rpm macros
5 years ago

1 new commit added

  • fixes
5 years ago

Why can't we allow setting RPM macro same way as edit-tag does?

It would mean allowing a user to set any 'extra' fields. I don't want people to mess with all the settings - I think it would limit expectations of what can come from sidetag. Even setting of random rpm macros seems to me as a dangerous thing.

Can we just have policy what kind of edits is person allowed to do?

Basically any kind of edits should be fine for side tags.

:thumbsup:
I'm thinking if it is worthy to have a common tag.extra test

Metadata Update from @tkopecek:
- Pull-request tagged with: testing-ready
5 years ago

Metadata Update from @jcupova:
- Pull-request tagged with: testing-done
5 years ago

rebased onto 7635258
5 years ago

Commit 6410117 fixes this pull-request

Pull-Request has been merged by tkopecek
5 years ago
Changes Summary 3
+8 -0
file changed
docs/source/plugins.rst
+41 -1
file changed
plugins/cli/sidetag_cli.py
+93 -8
file changed
plugins/hub/sidetag_hub.py
Powered by Pagure 5.14.1
Documentation • File an Issue • About • SSH Hostkey/Fingerprint
© Red Hat, Inc. and others.