#15 Sign all modules (koji tags with 'module-' prefix) with module_key/module_keyid.
Merged 6 years ago by ralph. Opened 6 years ago by jkaluza.
jkaluza/robosignatory sign-modules  into  master

file modified
+2 -8
@@ -6,7 +6,6 @@ 

      'robosignatory.enabled.atomicsigner': True,

      'robosignatory.pdc_url': 'https://pdc.fedoraproject.org/rest_api/v1',

      'robosignatory.module_prefixes': ['module-'],

-     'robosignatory.base_module_names': ['base-runtime'],
ralph commented 6 years ago

We'll need to remember to patch our ansible config here too.

  

      'robosignatory.signing': {

          # This should be the name of an entrypoint plugin that provides
@@ -22,6 +21,8 @@ 

      'robosignatory.koji_instances': {

          'primary': {

              'url': 'http://koji.fedoraproject.org/kojihub',

+             'module_key': 'fedora26',

+             'module_keyid': 'xxxxxxx',

              'options': {

                  # Only ssl and kerberos are supported at the moment

                  'authmethod': 'ssl',
@@ -36,13 +37,6 @@ 

                      'keyid': 'xxxxxxxx'

                  },

              ],

-             'module_streams': [

-                 {

-                     'stream': 'master',

-                     'key': 'fedora26',

-                     'keyid': 'xxxxxxxx'

-                 }

-             ]

          },

      },

  

file modified
+19 -212
@@ -3,7 +3,6 @@ 

  import fedmsg

  import fedmsg.consumers

  import robosignatory.utils as utils

- from pdc_client import PDCClient

  

  import logging

  log = logging.getLogger("robosignatory.tagconsumer")
@@ -31,12 +30,6 @@ 

  

          self.module_prefixes = \

              tuple(self.config['robosignatory.module_prefixes'])

-         self.valid_base_module_names = \

-             tuple(self.config['robosignatory.base_module_names'])

- 

-         self.pdc_client = PDCClient(

-             server=self.config['robosignatory.pdc_url'], develop=True,

-             ssl_verify=True)

  

          signing_config = self.config['robosignatory.signing']

          self.signer = utils.get_signing_helper(**signing_config)
@@ -63,7 +56,8 @@ 

  

              instance_obj = {'client': client,

                              'tags': {},

-                             'module_streams': {}}

+                             'module_key': None,

+                             'module_keyid': None}

              for tag in instance_info['tags']:

                  if tag['from'] in instance_obj['tags']:

                      raise Exception('From detected twice: %s' % tag['from'])
@@ -71,12 +65,10 @@ 

                                                       'key': tag['key'],

                                                       'keyid': tag['keyid']}

  

-             for stream in instance_info['module_streams']:

-                 if stream['stream'] in instance_obj['module_streams']:

-                     raise Exception('Module stream detected twice: %s'

-                                     % stream['stream'])

-                 instance_obj['module_streams'][stream['stream']] = {

-                     'key': stream['key'], 'keyid': stream['keyid']}

+             if ("module_key" in instance_info

+                     and "module_keyid" in instance_info):

+                 instance_obj["module_key"] = instance_info["module_key"]

+                 instance_obj["module_keyid"] = instance_info["module_keyid"]

  

              self.koji_clients[instance] = instance_obj

  
@@ -113,128 +105,11 @@ 

  

          instance = self.koji_clients[koji_instance]

          if tag in instance['tags']:

-             if not instance['tags'][tag]:

-                 log.info("No valid base module tag found in inheritance tree "

-                          "for %s, skipping" % tag)

-             else:

-                 self.dowork(build_nvr, build_id, tag, koji_instance,

-                             skip_tagging=False)

+             self.dowork(build_nvr, build_id, tag, koji_instance,

+                         skip_tagging=False)

          elif tag.startswith(self.module_prefixes):

              self.sign_modular_rpms(build_nvr, build_id, tag, koji_instance)

  

- 

-     def verify_base_module_tag(self, tag, active=True):

-         """

-         Verifies that the base module tag is valid. Sets the tag['stream']

-         and tag['verified'].

-         """

-         query = {}

-         tag_name = tag["name"]

-         if tag_name.endswith("-build"):

-             tag_name = tag_name[:-len("-build")]

- 

-         query["koji_tag"] = tag_name

-         if active is not None:

-             query["active"] = active

- 

-         if tag_name == 'f26-modularity':

-             # This was the manually created tag used to initially bootstrap

-             # the modularity infrastructure.  We have to handle it as a

-             # special case for now until we can duplicate it into two

-             # separate tags with two separate PDC entries for rawhide and

-             # f26.  See discussion here:

-             # https://meetbot.fedoraproject.org/fedora-meeting-2/2017-05-15/releng.2017-05-15-14.31.log.html

- 

-             # This first part of this conditional can be removed when the

-             # following ticket is resolved:

-             # https://pagure.io/releng/issue/6791

-             query['variant_version'] = 'f26'

- 

-         retval = self.pdc_client.unreleasedvariants(page_size=-1, **query)

- 

-         if not retval or len(retval) != 1:

-             tag["verified"] = False

-             return tag

- 

-         if retval[0]["variant_name"] not in self.valid_base_module_names:

-             tag["verified"] = False

-             return tag

- 

-         tag["verified"] = True

-         tag["stream"] = retval[0]["variant_version"]

-         tag["name"] = tag_name

-         return tag

- 

-     def get_base_module_tag(self, session, info, parent_tags=None):

-         """

-         Recursively traverse the inheritance hiearchy of tags in Koji to find

-         out the base module tag.

-         """

- 

-         # Handle only tags with modular prefix.

-         # ... but also handle a special case for "f26-modularity" until

-         # https://pagure.io/releng/issue/6791 is resolved.

-         if (not info['name'].startswith(self.module_prefixes)

-             and info['name'] != "f26-modularity"):

-             return None

- 

- 

-         # Store the dest_tag as a possible base tag (we don't know yet).

-         base_module_tag = {"id": info['id'],

-                            "name": info['name'],

-                            "verified": False,

-                            "stream": None}

- 

-         # Get the inheritance data and filter out tags from parent_tags set.

-         # Following those tags would bring us back to the already seen target.

-         inheritance_data = session.getInheritanceData(info['name'])

-         inheritance_data = [data for data in inheritance_data

-                             if data['parent_id'] not in parent_tags]

- 

-         # Iterate over all the tags this tag inherits from. There may be many of

-         # them, because single module can build-require multiple other modules.

-         for inherited in inheritance_data:

-             # Make a note to ourselves that we have seen this parent_tag.

-             parent_tag_id = inherited['parent_id']

-             parent_tags.add(parent_tag_id)

- 

-             # Get tag info for the parent_tag.

-             info = session.getTag(parent_tag_id)

-             if info is None:

-                 continue

- 

-             # TODO - remove the "f26-modularity" part of this conditional once

-             # the following ticket is resolved:  https://pagure.io/releng/issue/6791

-             if not info['name'].endswith("-build") and info['name'] != "f26-modularity":

-                 info = session.getTag(info['name'] + "-build")

-                 if info is None:

-                     continue

- 

-             # Check if parent_tag is valid base module tag.

-             maybe_tag = self.verify_base_module_tag(info)

-             if not maybe_tag or not maybe_tag['verified']:

-                 # Try to recursively find all the parents of this parent tag.

-                 maybe_tag = self.get_base_module_tag(session, info, parent_tags)

-                 if not maybe_tag:

-                     continue

- 

-                 # Verify that the found tag is really a valid base module tag.

-                 maybe_tag = self.verify_base_module_tag(maybe_tag)

- 

-             if maybe_tag['verified']:

-                 # In case we have already found valid tag in the previous subtree

-                 # and right now we have another one, compare that their streams

-                 # are matching.

-                 if (base_module_tag['verified']

-                         and maybe_tag['stream'] != base_module_tag['stream']):

-                     err = "Multiple base module streams found in inheritance tree."

-                     log.warn(err)

-                     raise MultipleStreamsError(err)

-                 else:

-                     base_module_tag = maybe_tag

- 

-         return base_module_tag

- 

      def sign_modular_rpms(self, build_nvr, build_id, tag, koji_instance):

          # Skip the -build tag.

          if tag.endswith("-build"):
@@ -242,73 +117,17 @@ 

              return

  

          instance = self.koji_clients[koji_instance]

-         session = instance["client"]

- 

-         # Get the tag to find out its id.

-         info = session.getTag(tag + "-build")

-         if info is None:

-             log.info("Build tag for Koji tag %s not known, skipping" % tag)

+         if not instance["module_key"] or not instance["module_keyid"]:

+             log.info("Skipping tag %s - module_key or module_keyid not "

+                      "defined." % tag)

              return

  

-         # Try to find out if the current tag is a base module before traversing

-         # the tag inheritance tree. The builds are tagged to module tag before

-         # it is marked as complete, therefore we do not want to check whether

-         # the base module is active.

-         maybe_tag = {

-             "id": info["id"],

-             "name": info['name'],

-             "verified": False,

-             "stream": None,

-         }

-         maybe_tag = self.verify_base_module_tag(maybe_tag, active=None)

-         if maybe_tag["verified"]:

-             base_module_tag = maybe_tag

-         else:

-             # build tag inherits from the main tag, so when we are evaluating

-             # which tag is the right one to check when examining tag inheritance

-             # we have to know that we do not want to go back to the main tag.

-             # Therefore we have to track the set of parent tags.

-             parent_tags = set([info['id']])

- 

-             # Resulting base module tag according to which we will use the right key.

-             try:

-                 base_module_tag = self.get_base_module_tag(

-                     session, info, parent_tags=parent_tags)

-             except MultipleStreamsError:

-                 # Set to unverified tag

-                 base_module_tag = maybe_tag

- 

-         # Set the cache to None, so in case this module build does not have

-         # valid base module stream, we do not query PDC on every RPM, but just

-         # skip the tag altogether.

-         instance["tags"][tag] = None

- 

-         if not base_module_tag:

-             log.info("No base module tag found in inheritance tree for "

-                      "%s, skipping" % tag)

-             return

- 

-         if not base_module_tag["verified"]:

-             log.info("No verified base module tag found in inheritance tree for "

-                      "%s, skipping. Found tag: %r" % (tag, base_module_tag))

-             return

- 

-         if base_module_tag["stream"] not in instance['module_streams']:

-             log.info("Base module stream %s not allowed for "

-                      "auto-sign" % base_module_tag["stream"])

-             return

- 

-         stream_info = instance['module_streams'][base_module_tag["stream"]]

-         # We are not moving to any tag after signing.

-         stream_info["to"] = tag

- 

-         # Cache the stream_info (which is in the same format as tag_info), so

-         # for next package in this tag, we know what key to use without

-         # traversing the Koji tag inheritance tree.

-         instance["tags"][tag] = stream_info

- 

+         tag_info = {}

+         tag_info["to"] = tag

+         tag_info["key"] = instance["module_key"]

+         tag_info["keyid"] = instance["module_keyid"]

          self.dowork(build_nvr, build_id, tag, koji_instance,

-                     tag_info=stream_info)

+                     tag_info=tag_info)

  

  

      def dowork(self, build_nvr, build_id, tag, koji_instance,
@@ -392,19 +211,7 @@ 

              log.info("No build to sign in given tag.")

              return

  

-         # Try signing the first build. This queries the PDC to find out

-         # the base module for a build and therefore is slow. Therefore

-         # we do that only for the first build. In the end it adds the

-         # resulting signing key to instance["tags"] cache and that is what

-         # we use for the rest of builds in this module.

-         self.sign_modular_rpms(builds[0]["nvr"], builds[0]["build_id"], tag,

-                                koji_instance)

-         if tag not in instance["tags"] or not instance["tags"][tag]:

-             # No need to log anything, sign_module_rpms logs an error

-             # when it cannot sign a module build.

-             return

- 

-         # Sign the rest of builds in tag.

+         # Sign the builds in tag.

          for build in builds[1:]:

-             self.dowork(build["nvr"], build["build_id"], tag, koji_instance,

-                         skip_tagging=False)

+             self.sign_modular_rpms(build["nvr"], build["build_id"], tag,

+                                    koji_instance)

While releasing modular F26, we started signing all modules using the single key no matter from which stream they came from. This has never resulted in a code change, so while signing, robosignatory still inspects the module inheritance tree to find out what is the base module for given module and from what stream it has been built. This is useless and makes robosignatory slower and more complex.

It has also started to be a problem for F27, because the list of base modules changed - we no longer have base-runtime module, but we have platform module instead. This platform module cannot be signed currently, because it is new base module and robosignatory does not have it listed in the base_modules config option.

This PR removes the inheritance tree checks and also the stream checks and just signs any module with the single configured key. This should fix modular builds signing forever :).

rebased

6 years ago

Hmm, I wonder - can we also sign the module build itself? Does that even make sense? Maybe we sign the resulting modulemd in the build? i.e. https://koji.fedoraproject.org/koji/buildinfo?buildID=953079

Not sure, I have no idea if it's possible to sign modulemd like that. But anyway, this should be part of anothe RFE/PR. This one is here really to remove blocker that we currently cannot sign "platform" module in prod, which is blocking others from building their own modules.

Right, no problem with that

We'll need to remember to patch our ansible config here too.

This looks good to me.

... I wish we had a test suite to exercise it, though.

FYI, here are the relevant modules I see that might need manual signing.

scripts/mbs❯ python find-unsigned-modules.py platform-master shim-master bootstrap-master
Using cache file /var/tmp/find-unsigned-modules-cache.dbm
- platform-master-20170807115511 with tag module-e507a2873dda189a is not signed.
+ shim-master-20170502110601 with tag module-5d197e359fd3ea1d is signed with a3cc4e62
- bootstrap-master-1 with tag module-bootstrap-rawhide is not signed.

@psabata are there any others we should check for while @puiterwijk is on the system later doing an upgrade?

Pull-Request has been merged by ralph

6 years ago

@ralph I'm building atomic:master right now. It will take some time but I'm not sure when the signing happens -- whether immediately on RPM build or on module build. We may want to wait.