Merge branch 'lint-config-files' into 'master'

lint config files

See merge request fdroid/fdroidserver!1418

.gitlab-ci.yml

@@ -15,11 +15,12 @@ variables:
 # * python3-babel for compiling localization files
 # * gnupg-agent for the full signing setup
 # * python3-clint for fancy progress bars for users
+# * python3-pycountry for linting config/mirrors.yml
 buildserver run-tests:
   image: registry.gitlab.com/fdroid/fdroidserver:buildserver
   script:
     - apt-get update
-    - apt-get install gnupg-agent python3-babel python3-clint
+    - apt-get install gnupg-agent python3-babel python3-clint python3-pycountry
     - ./tests/run-tests
     # make sure that translations do not cause stacktraces
     - cd $CI_PROJECT_DIR/locale
@@ -152,6 +153,9 @@ ubuntu_jammy_pip:
     - $pip install sdkmanager
     - sdkmanager 'build-tools;33.0.0'
 
+    # pycountry is only for linting config/mirrors.yml, so its not in setup.py
+    - $pip install pycountry
+
     - $pip install dist/fdroidserver-*.tar.gz
     - tar xzf dist/fdroidserver-*.tar.gz
     - cd fdroidserver-*

fdroidserver/common.py

@@ -361,6 +361,26 @@ def regsub_file(pattern, repl, path):
         f.write(text)
 
 
+def config_type_check(path, data):
+    if Path(path).name == 'mirrors.yml':
+        expected_type = list
+    else:
+        expected_type = dict
+    if expected_type == dict:
+        if not isinstance(data, dict):
+            msg = _('{path} is not "key: value" dict, but a {datatype}!')
+            raise TypeError(msg.format(path=path, datatype=type(data).__name__))
+    elif not isinstance(data, expected_type):
+        msg = _('{path} is not {expected_type}, but a {datatype}!')
+        raise TypeError(
+            msg.format(
+                path=path,
+                expected_type=expected_type.__name__,
+                datatype=type(data).__name__,
+            )
+        )
+
+
 def read_config(opts=None):
     """Read the repository config.
 
@@ -401,11 +421,7 @@ def read_config(opts=None):
             config = yaml.safe_load(fp)
         if not config:
             config = {}
-        if not isinstance(config, dict):
-            msg = _('{path} is not "key: value" dict, but a {datatype}!')
-            raise TypeError(
-                msg.format(path=config_file, datatype=type(config).__name__)
-            )
+        config_type_check(config_file, config)
     elif os.path.exists(old_config_file):
         logging.warning(_("""{oldfile} is deprecated, use {newfile}""")
                         .format(oldfile=old_config_file, newfile=config_file))
@@ -413,12 +429,12 @@ def read_config(opts=None):
             code = compile(fp.read(), old_config_file, 'exec')
             exec(code, None, config)  # nosec TODO automatically migrate
 
-    for k in ('mirrors', 'install_list', 'uninstall_list', 'serverwebroot', 'servergitroot'):
-        if k in config:
-            if not type(config[k]) in (str, list, tuple):
-                logging.warning(
-                    _("'{field}' will be in random order! Use () or [] brackets if order is important!")
-                    .format(field=k))
+        for k in ('mirrors', 'install_list', 'uninstall_list', 'serverwebroot', 'servergitroot'):
+            if k in config:
+                if not type(config[k]) in (str, list, tuple):
+                    logging.warning(
+                        _("'{field}' will be in random order! Use () or [] brackets if order is important!")
+                        .format(field=k))
 
     # smartcardoptions must be a list since its command line args for Popen
     smartcardoptions = config.get('smartcardoptions')

fdroidserver/index.py

@@ -26,10 +26,10 @@ import json
 import logging
 import os
 import re
+import ruamel.yaml
 import shutil
 import tempfile
 import urllib.parse
-import yaml
 import zipfile
 import calendar
 import qrcode
@@ -1409,7 +1409,7 @@ def add_mirrors_to_repodict(repo_section, repodict):
                 )
             )
         with mirrors_yml.open() as fp:
-            mirrors_config = yaml.safe_load(fp)
+            mirrors_config = ruamel.yaml.YAML(typ='safe').load(fp)
         if not isinstance(mirrors_config, list):
             msg = _('{path} is not list, but a {datatype}!')
             raise TypeError(

fdroidserver/lint.py

@@ -17,9 +17,11 @@
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
 from argparse import ArgumentParser
+import difflib
 import re
 import sys
 import platform
+import ruamel.yaml
 import urllib.parse
 from pathlib import Path
 
@@ -739,6 +741,43 @@ def check_certificate_pinned_binaries(app):
             return
 
 
+def lint_config(arg):
+    path = Path(arg)
+    passed = True
+    yamllintresult = common.run_yamllint(path)
+    if yamllintresult:
+        print(yamllintresult)
+        passed = False
+
+    with path.open() as fp:
+        data = ruamel.yaml.YAML(typ='safe').load(fp)
+    common.config_type_check(arg, data)
+
+    if path.name == 'mirrors.yml':
+        import pycountry
+
+        valid_country_codes = [c.alpha_2 for c in pycountry.countries]
+        for mirror in data:
+            code = mirror.get('countryCode')
+            if code and code not in valid_country_codes:
+                passed = False
+                msg = _(
+                    '{path}: "{code}" is not a valid ISO_3166-1 alpha-2 country code!'
+                ).format(path=str(path), code=code)
+                if code.upper() in valid_country_codes:
+                    m = [code.upper()]
+                else:
+                    m = difflib.get_close_matches(
+                        code.upper(), valid_country_codes, 2, 0.5
+                    )
+                if m:
+                    msg += ' '
+                    msg += _('Did you mean {code}?').format(code=', '.join(sorted(m)))
+                print(msg)
+
+    return passed
+
+
 def main():
     global config, options
 
@@ -772,6 +811,38 @@ def main():
     load_antiFeatures_config()
     load_categories_config()
 
+    if options.force_yamllint:
+        import yamllint  # throw error if it is not installed
+
+        yamllint  # make pyflakes ignore this
+
+    paths = list()
+    for arg in options.appid:
+        if (
+            arg == 'config.yml'
+            or Path(arg).parent.name == 'config'
+            or Path(arg).parent.parent.name == 'config'  # localized
+        ):
+            paths.append(arg)
+
+    failed = 0
+    if paths:
+        for path in paths:
+            options.appid.remove(path)
+            if not lint_config(path):
+                failed += 1
+        # an empty list of appids means check all apps, avoid that if files were given
+        if not options.appid:
+            sys.exit(failed)
+
+    if not lint_metadata(options):
+        failed += 1
+
+    if failed:
+        sys.exit(failed)
+
+
+def lint_metadata(options):
     # Get all apps...
     allapps = metadata.read_metadata(options.appid)
     apps = common.read_app_args(options.appid, allapps, False)
@@ -791,11 +862,6 @@ def main():
         if app.Disabled:
             continue
 
-        if options.force_yamllint:
-            import yamllint  # throw error if it is not installed
-
-            yamllint  # make pyflakes ignore this
-
         # only run yamllint when linting individual apps.
         if options.appid or options.force_yamllint:
             # run yamllint on app metadata
@@ -856,8 +922,7 @@ def main():
                 anywarns = True
                 print("%s: %s" % (appid, warn))
 
-    if anywarns:
-        sys.exit(1)
+    return not anywarns
 
 
 # A compiled, public domain list of official SPDX license tags.  generated

tests/common.TestCase

@@ -2838,6 +2838,36 @@ class CommonTest(unittest.TestCase):
         with self.assertRaises(TypeError):
             fdroidserver.common.load_localized_config(CATEGORIES_CONFIG_NAME, 'repo')
 
+    def test_config_type_check_config_yml_dict(self):
+        fdroidserver.common.config_type_check('config.yml', dict())
+
+    def test_config_type_check_config_yml_list(self):
+        with self.assertRaises(TypeError):
+            fdroidserver.common.config_type_check('config.yml', list())
+
+    def test_config_type_check_config_yml_set(self):
+        with self.assertRaises(TypeError):
+            fdroidserver.common.config_type_check('config.yml', set())
+
+    def test_config_type_check_config_yml_str(self):
+        with self.assertRaises(TypeError):
+            fdroidserver.common.config_type_check('config.yml', str())
+
+    def test_config_type_check_mirrors_list(self):
+        fdroidserver.common.config_type_check('config/mirrors.yml', list())
+
+    def test_config_type_check_mirrors_dict(self):
+        with self.assertRaises(TypeError):
+            fdroidserver.common.config_type_check('config/mirrors.yml', dict())
+
+    def test_config_type_check_mirrors_set(self):
+        with self.assertRaises(TypeError):
+            fdroidserver.common.config_type_check('config/mirrors.yml', set())
+
+    def test_config_type_check_mirrors_str(self):
+        with self.assertRaises(TypeError):
+            fdroidserver.common.config_type_check('config/mirrors.yml', str())
+
 
 if __name__ == "__main__":
     os.chdir(os.path.dirname(__file__))

tests/get-country-region-data.py

@@ -0,0 +1,47 @@
+#!/usr/bin/env python3
+#
+# This generates a list of ISO_3166-1 alpha 2 country codes for use in lint.
+
+import collections
+import os
+import re
+import requests
+import requests_cache
+import sys
+import tempfile
+
+
+def main():
+    # we want all the data
+    url = 'https://api.worldbank.org/v2/country?format=json&per_page=500'
+    r = requests.get(url, timeout=30)
+    data = r.json()
+    if data[0]['pages'] != 1:
+        print(
+            'ERROR: %d pages in data, this script only reads one page!'
+            % data[0]['pages']
+        )
+        sys.exit(1)
+
+    iso2Codes = set()
+    ISO3166_1_alpha_2_codes = set()
+    names = dict()
+    regions = collections.defaultdict(set)
+    for country in data[1]:
+        iso2Code = country['iso2Code']
+        iso2Codes.add(iso2Code)
+        if country['region']['value'] == 'Aggregates':
+            continue
+        if re.match(r'[A-Z][A-Z]', iso2Code):
+            ISO3166_1_alpha_2_codes.add(iso2Code)
+            names[iso2Code] = country['name']
+            regions[country['region']['value']].add(country['name'])
+    for code in sorted(ISO3166_1_alpha_2_codes):
+        print(f"    '{code}',  # " + names[code])
+
+
+if __name__ == "__main__":
+    requests_cache.install_cache(
+        os.path.join(tempfile.gettempdir(), os.path.basename(__file__) + '.cache')
+    )
+    main()

tests/lint.TestCase

@@ -5,6 +5,7 @@
 import logging
 import optparse
 import os
+import ruamel.yaml
 import shutil
 import sys
 import tempfile
@@ -368,6 +369,75 @@ class LintTest(unittest.TestCase):
         app = fdroidserver.metadata.App({'Categories': ['bar']})
         self.assertEqual(0, len(list(fdroidserver.lint.check_categories(app))))
 
+    def test_lint_config_basic_mirrors_yml(self):
+        os.chdir(self.testdir)
+        yaml = ruamel.yaml.YAML(typ='safe')
+        with Path('mirrors.yml').open('w') as fp:
+            yaml.dump([{'url': 'https://example.com/fdroid/repo'}], fp)
+        self.assertTrue(fdroidserver.lint.lint_config('mirrors.yml'))
+
+    def test_lint_config_mirrors_yml_kenya_countryCode(self):
+        os.chdir(self.testdir)
+        yaml = ruamel.yaml.YAML(typ='safe')
+        with Path('mirrors.yml').open('w') as fp:
+            yaml.dump([{'url': 'https://foo.com/fdroid/repo', 'countryCode': 'KE'}], fp)
+        self.assertTrue(fdroidserver.lint.lint_config('mirrors.yml'))
+
+    def test_lint_config_mirrors_yml_invalid_countryCode(self):
+        """WV is "indeterminately reserved" so it should never be used."""
+        os.chdir(self.testdir)
+        yaml = ruamel.yaml.YAML(typ='safe')
+        with Path('mirrors.yml').open('w') as fp:
+            yaml.dump([{'url': 'https://foo.com/fdroid/repo', 'countryCode': 'WV'}], fp)
+        self.assertFalse(fdroidserver.lint.lint_config('mirrors.yml'))
+
+    def test_lint_config_mirrors_yml_alpha3_countryCode(self):
+        """Only ISO 3166-1 alpha 2 are supported"""
+        os.chdir(self.testdir)
+        yaml = ruamel.yaml.YAML(typ='safe')
+        with Path('mirrors.yml').open('w') as fp:
+            yaml.dump([{'url': 'https://de.com/fdroid/repo', 'countryCode': 'DEU'}], fp)
+        self.assertFalse(fdroidserver.lint.lint_config('mirrors.yml'))
+
+    def test_lint_config_mirrors_yml_one_invalid_countryCode(self):
+        """WV is "indeterminately reserved" so it should never be used."""
+        os.chdir(self.testdir)
+        yaml = ruamel.yaml.YAML(typ='safe')
+        with Path('mirrors.yml').open('w') as fp:
+            yaml.dump(
+                [
+                    {'url': 'https://bar.com/fdroid/repo', 'countryCode': 'BA'},
+                    {'url': 'https://foo.com/fdroid/repo', 'countryCode': 'FO'},
+                    {'url': 'https://wv.com/fdroid/repo', 'countryCode': 'WV'},
+                ],
+                fp,
+            )
+        self.assertFalse(fdroidserver.lint.lint_config('mirrors.yml'))
+
+    def test_lint_config_bad_mirrors_yml_dict(self):
+        os.chdir(self.testdir)
+        Path('mirrors.yml').write_text('baz: [foo, bar]\n')
+        with self.assertRaises(TypeError):
+            fdroidserver.lint.lint_config('mirrors.yml')
+
+    def test_lint_config_bad_mirrors_yml_float(self):
+        os.chdir(self.testdir)
+        Path('mirrors.yml').write_text('1.0\n')
+        with self.assertRaises(TypeError):
+            fdroidserver.lint.lint_config('mirrors.yml')
+
+    def test_lint_config_bad_mirrors_yml_int(self):
+        os.chdir(self.testdir)
+        Path('mirrors.yml').write_text('1\n')
+        with self.assertRaises(TypeError):
+            fdroidserver.lint.lint_config('mirrors.yml')
+
+    def test_lint_config_bad_mirrors_yml_str(self):
+        os.chdir(self.testdir)
+        Path('mirrors.yml').write_text('foo\n')
+        with self.assertRaises(TypeError):
+            fdroidserver.lint.lint_config('mirrors.yml')
+
 
 class LintAntiFeaturesTest(unittest.TestCase):
     def setUp(self):