Merge pull request #1267 from dmach/build-ignore-hdrmd5-mismatches-from-local-cache

build: New option 'disable_hdrmd5_check' to ignore hdrmd5 mismatches

behave/container-run.sh

@@ -12,6 +12,7 @@ podman run \
     --volume="$TOPDIR":/opt/obs \
     --cap-add SYS_PTRACE \
     -p 1443:443 \
+    -p 1082:82 \
     obs-server
 
 sleep 0.5

behave/container-setup-prebuilt-rpms.sh

@@ -49,8 +49,12 @@ rm -rf "$TMP_DIR"
 
 # build package 'test:factory/test-pkgB'
 TMP_DIR=$(mktemp -d)
-rpmbuild -ba "$FIXTURES_DIR/pac/test-pkgB-2.spec" --define "_topdir $TMP_DIR"
+setarch i586 rpmbuild -ba "$FIXTURES_DIR/pac/test-pkgB-2.spec" --define "_topdir $TMP_DIR"
 upload_rpms "$TMP_DIR" test:factory standard i586 test-pkgB
+rm -rf "$TMP_DIR"
+
+TMP_DIR=$(mktemp -d)
+rpmbuild -ba "$FIXTURES_DIR/pac/test-pkgB-2.spec" --define "_topdir $TMP_DIR"
 upload_rpms "$TMP_DIR" test:factory standard x86_64 test-pkgB
 rm -rf "$TMP_DIR"
 
@@ -84,6 +88,11 @@ rm -rf "$TMP_DIR"
 /usr/lib/obs/server/bs_sched --testmode x86_64
 
 
+# run publisher
+# noarch packages from x86_64 win over those from i586
+/usr/lib/obs/server/bs_publish --testmode
+
+
 # create fake empty files that usually accompany RPMs
 ARCHES="i586 x86_64"
 PACKAGES="test-pkgA test-pkgB multibuild-pkg multibuild-pkg:flavor1 multibuild-pkg:flavor2"

behave/container-setup.sh

@@ -26,6 +26,9 @@ sed -i -E 's!^(\s*)PassengerRuby .*!\1PassengerRuby "/usr/bin/ruby.ruby3.1"!' /e
 # enable apache SSL server flag
 sed -i 's!^APACHE_SERVER_FLAGS=.*!APACHE_SERVER_FLAGS="SSL"!' /etc/sysconfig/apache2
 
+# also listen on the port that is exported to an unprivileged user
+sed -i 's!^<VirtualHost \*:82>!<VirtualHost *:82 *:1082>!' /etc/apache2/vhosts.d/obs.conf
+
 
 # enable apache mods
 APACHE_MODS="passenger rewrite proxy proxy_http xforward headers ssl socache_shmcb"
@@ -96,6 +99,11 @@ cd /srv/www/obs/api
 RAILS_ENV=production SAFETY_ASSURED=1 bin/rails db:setup writeconfiguration data:schema:load
 
 
+# update configuration and write it to disk
+echo "update configurations set download_url='http://localhost:1082';" | su -s /bin/sh - mysql -c "mysql api_production"
+cd /srv/www/obs/api; RAILS_ENV=production SAFETY_ASSURED=1 bin/rails writeconfiguration
+
+
 # fix perms
 chown -R wwwrun:www /srv/www/obs/api/log/
 chown -R wwwrun:www /srv/www/obs/api/tmp/

behave/fixtures/prj/test_factory.xml

@@ -9,14 +9,6 @@
     <disable/>
   </build>
 
-  <publish>
-    <disable/>
-  </publish>
-
-  <useforbuild>
-    <disable/>
-  </useforbuild>
-
   <repository name="standard">
     <path project="openSUSE.org:openSUSE:Tumbleweed" repository="standard"/>
     <arch>x86_64</arch>

osc/build.py

@@ -1372,8 +1372,11 @@ def main(apiurl, opts, argv):
                 print("Error: cannot get hdrmd5 for %s" % i.fullfilename)
                 sys.exit(1)
             if hdrmd5 != i.hdrmd5:
-                print("WARNING: OBS BUG hdrmd5 mismatch for %s: %s != %s" % (i.fullfilename, hdrmd5, i.hdrmd5))
-                # sys.exit(1)
+                if conf.config["api_host_options"][apiurl]["disable_hdrmd5_check"]:
+                    print(f"Warning: Ignoring a hdrmd5 mismatch for {i.fullfilename}: {hdrmd5} (actual) != {i.hdrmd5} (expected)")
+                else:
+                    print(f"Error: hdrmd5 mismatch for {i.fullfilename}: {hdrmd5} (actual) != {i.hdrmd5} (expected)")
+                    sys.exit(1)
 
     print('Writing build configuration')
 

osc/conf.py

@@ -129,6 +129,28 @@ DEFAULTS = {'apiurl': 'https://api.opensuse.org',
             'cookiejar': _identify_osccookiejar(),
             # fallback for osc build option --no-verify
             'no_verify': '0',
+
+            # Disable hdrmd5 checks of downloaded and cached packages in `osc build`
+            # Recommended value: 0
+            #
+            # OBS builds the noarch packages once per binary arch.
+            # Such noarch packages are supposed to be nearly identical across all build arches,
+            # any discrepancy in the payload and dependencies is considered a packaging bug.
+            # But to guarantee that the local builds work identically to builds in OBS,
+            # using the arch-specific copy of the noarch package is required.
+            # Unfortunatelly only one of the noarch packages gets distributed
+            # and can be downloaded from a local mirror.
+            # All other noarch packages are available through the OBS API only.
+            # Since there is currently no information about hdrmd5 checksums of published noarch packages,
+            # we download them, verify hdrmd5 and re-download the package from OBS API on mismatch.
+            #
+            # The same can also happen for architecture depend packages when someone is messing around
+            # with the source history or the release number handling in a way that it is not increasing.
+            #
+            # If you want to save some bandwidth and don't care about the exact rebuilds
+            # you can turn this option on to disable hdrmd5 checks completely.
+            'disable_hdrmd5_check': '0',
+
             # enable project tracking by default
             'do_package_tracking': '1',
             # default for osc build
@@ -187,15 +209,17 @@ if not os.path.isfile('/usr/lib/build/vc') and os.path.isfile('/usr/lib/obs-buil
     DEFAULTS['vc-cmd'] = '/usr/lib/obs-build/vc'
 
 api_host_options = ['user', 'pass', 'passx', 'aliases', 'http_headers', 'realname', 'email', 'sslcertck', 'cafile', 'capath', 'trusted_prj',
-                    'downloadurl', 'sshkey']
+                    'downloadurl', 'sshkey', 'disable_hdrmd5_check']
 
+
+# _integer_opts and _boolean_opts specify option types for both global options as well as api_host_options
 _integer_opts = ('build-jobs',)
 _boolean_opts = (
     'debug', 'do_package_tracking', 'http_debug', 'post_mortem', 'traceback', 'check_filelist',
     'checkout_no_colon', 'checkout_rooted', 'check_for_request_on_action', 'linkcontrol', 'show_download_progress', 'request_show_interactive',
-    'request_show_source_buildstatus', 'review_inherit_group', 'use_keyring', 'no_verify', 'builtin_signature_check',
+    'request_show_source_buildstatus', 'review_inherit_group', 'use_keyring', 'no_verify', 'disable_hdrmd5_check', 'builtin_signature_check',
     'http_full_debug', 'include_request_from_project', 'local_service_run', 'buildlog_strip_time', 'no_preinstallimage',
-    'status_mtime_heuristic', 'print_web_links', 'ccache', 'sccache', 'build-shell-after-fail')
+    'status_mtime_heuristic', 'print_web_links', 'ccache', 'sccache', 'build-shell-after-fail', 'allow_http', 'sslcertck', )
 
 
 def apply_option_types(config, conffile=""):
@@ -211,6 +235,8 @@ def apply_option_types(config, conffile=""):
     typed_opts = ((_boolean_opts, cp.getboolean), (_integer_opts, cp.getint))
     for opts, meth in typed_opts:
         for opt in opts:
+            if opt not in config:
+                continue
             try:
                 config[opt] = meth('general', opt)
             except ValueError as e:
@@ -826,11 +852,15 @@ def get_config(override_conffile=None,
             credentials.AbstractCredentialsManager.config_entry,
         )
         for key in optional:
-            if cp.has_option(url, key):
-                if key in ('sslcertck', 'allow_http'):
-                    api_host_options[apiurl][key] = cp.getboolean(url, key)
-                else:
-                    api_host_options[apiurl][key] = cp.get(url, key)
+            if not cp.has_option(url, key):
+                continue
+            if key in _boolean_opts:
+                api_host_options[apiurl][key] = cp.getboolean(url, key)
+            elif key in _integer_opts:
+                api_host_options[apiurl][key] = cp.getint(url, key)
+            else:
+                api_host_options[apiurl][key] = cp.get(url, key)
+
         if cp.has_option(url, 'build-root', proper=True):
             api_host_options[apiurl]['build-root'] = cp.get(url, 'build-root', raw=True)
 
@@ -858,6 +888,10 @@ def get_config(override_conffile=None,
         if api_host_options[apiurl]['sshkey'] is None:
             api_host_options[apiurl]['sshkey'] = config['sshkey']
 
+        api_host_options[apiurl]["disable_hdrmd5_check"] = config["disable_hdrmd5_check"]
+        if cp.has_option(url, "disable_hdrmd5_check"):
+            api_host_options[apiurl][key] = cp.getboolean(url, "disable_hdrmd5_check")
+
     # add the auth data we collected to the config dict
     config['api_host_options'] = api_host_options
     config['apiurl_aliases'] = aliases

osc/fetch.py

@@ -205,6 +205,7 @@ class Fetcher:
         return urllist
 
     def run(self, buildinfo):
+        apiurl = buildinfo.apiurl
         cached = 0
         all = len(buildinfo.deps)
         for i in buildinfo.deps:
@@ -221,12 +222,24 @@ class Fetcher:
                 cached += 1
                 if not i.name.startswith('container:') and i.pacsuffix != 'rpm':
                     continue
+
+                hdrmd5_is_valid = True
                 if i.hdrmd5:
                     if i.name.startswith('container:'):
                         hdrmd5 = dgst(i.fullfilename)
+                        if hdrmd5 != i.hdrmd5:
+                            hdrmd5_is_valid = False
                     else:
                         hdrmd5 = packagequery.PackageQuery.queryhdrmd5(i.fullfilename)
-                    if not hdrmd5 or hdrmd5 != i.hdrmd5:
+                        if hdrmd5 != i.hdrmd5:
+                            if conf.config["api_host_options"][apiurl]["disable_hdrmd5_check"]:
+                                print(f"Warning: Ignoring a hdrmd5 mismatch for {i.fullfilename}: {hdrmd5} (actual) != {i.hdrmd5} (expected)")
+                                hdrmd5_is_valid = True
+                            else:
+                                print(f"The file will be redownloaded from the API due to a hdrmd5 mismatch for {i.fullfilename}: {hdrmd5} (actual) != {i.hdrmd5} (expected)")
+                                hdrmd5_is_valid = False
+
+                    if not hdrmd5_is_valid:
                         os.unlink(i.fullfilename)
                         cached -= 1
 
@@ -258,19 +271,15 @@ class Fetcher:
                         # mark it for downloading from the API
                         self.__add_cpio(i)
                     else:
-                        # if the checksum of the downloaded package doesn't match,
-                        # delete it and mark it for downloading from the API
-                        #
-                        # wbrown 2022 - is there a reason to keep these md5's at all? md5 is
-                        # broken from a security POV so these aren't a trusted source for validation
-                        # of the file content. They are often incorrect forcing download via the API
-                        # which for anyone outside the EU is excruciating. And when they are ignored
-                        # builds work and progress anyway? So what do they even do? What are they
-                        # for? They should just be removed.
                         hdrmd5 = packagequery.PackageQuery.queryhdrmd5(i.fullfilename)
-                        if not hdrmd5 or hdrmd5 != i.hdrmd5:
-                            print('%s/%s: allowing invalid file, probably an OBS bug - hdrmd5 did not match - %s != %s'
-                                % (i.project, i.name, hdrmd5, i.hdrmd5))
+                        if hdrmd5 != i.hdrmd5:
+                            if conf.config["api_host_options"][apiurl]["disable_hdrmd5_check"]:
+                                print(f"Warning: Ignoring a hdrmd5 mismatch for {i.fullfilename}: {hdrmd5} (actual) != {i.hdrmd5} (expected)")
+                            else:
+                                print(f"The file will be redownloaded from the API due to a hdrmd5 mismatch for {i.fullfilename}: {hdrmd5} (actual) != {i.hdrmd5} (expected)")
+                                os.unlink(i.fullfilename)
+                                self.__add_cpio(i)
+
                 except KeyboardInterrupt:
                     print('Cancelled by user (ctrl-c)')
                     print('Exiting.')

tests/test_conf.py

@@ -16,23 +16,31 @@ class TestConf(unittest.TestCase):
     def test_bool_opts_defaults(self):
         config = osc.conf.config
         for opt in osc.conf._boolean_opts:
+            if opt not in config:
+                continue
             self.assertIsInstance(config[opt], bool, msg=f"option: '{opt}'")
 
     def test_int_opts_defaults(self):
         config = osc.conf.config
         for opt in osc.conf._integer_opts:
+            if opt not in config:
+                continue
             self.assertIsInstance(config[opt], int, msg=f"option: '{opt}'")
 
     def test_bool_opts(self):
         osc.conf.get_config()
         config = osc.conf.config
         for opt in osc.conf._boolean_opts:
+            if opt not in config:
+                continue
             self.assertIsInstance(config[opt], bool, msg=f"option: '{opt}'")
 
     def test_int_opts(self):
         osc.conf.get_config()
         config = osc.conf.config
         for opt in osc.conf._integer_opts:
+            if opt not in config:
+                continue
             self.assertIsInstance(config[opt], int, msg=f"option: '{opt}'")