start of work to add spack audit packages-https checker (#25670)
This PR will add a new audit, specifically for spack package homepage urls (and eventually other kinds I suspect) to see if there is an http address that can be changed to https. Usage is as follows: ```bash $ spack audit packages-https <package> ``` And in list view: ```bash $ spack audit list generic: Generic checks relying on global variables configs: Sanity checks on compilers.yaml Sanity checks on packages.yaml packages: Sanity checks on specs used in directives packages-https: Sanity checks on https checks of package urls, etc. ``` I think it would be unwise to include with packages, because when run for all, since we do requests it takes a long time. I also like the idea of more well scoped checks - likely there will be other addresses for http/https within a package that we eventually check. For now, there are two error cases - one is when an https url is tried but there is some SSL error (or other error that means we cannot update to https): ```bash $ spack audit packages-https zoltan PKG-HTTPS-DIRECTIVES: 1 issue found 1. Error with attempting https for "zoltan": <urlopen error [SSL: CERTIFICATE_VERIFY_FAILED] certificate verify failed: Hostname mismatch, certificate is not valid for 'www.cs.sandia.gov'. (_ssl.c:1125)> ``` This is either not fixable, or could be fixed with a change to the url or (better) contacting the site owners to ask about some certificate or similar. The second case is when there is an http that needs to be https, which is a huge issue now, but hopefully not after this spack PR. ```bash $ spack audit packages-https xman Package "xman" uses http but has a valid https endpoint. ``` And then when a package is fixed: ```bash $ spack audit packages-https zlib PKG-HTTPS-DIRECTIVES: 0 issues found. ``` And that's mostly it. :) Signed-off-by: vsoch <vsoch@users.noreply.github.com> Co-authored-by: vsoch <vsoch@users.noreply.github.com>
This commit is contained in:
@@ -37,12 +37,16 @@ def _search_duplicate_compilers(error_cls):
|
||||
"""
|
||||
import collections
|
||||
import itertools
|
||||
import re
|
||||
|
||||
from six.moves.urllib.request import urlopen
|
||||
|
||||
try:
|
||||
from collections.abc import Sequence # novm
|
||||
except ImportError:
|
||||
from collections import Sequence
|
||||
|
||||
|
||||
#: Map an audit tag to a list of callables implementing checks
|
||||
CALLBACKS = {}
|
||||
|
||||
@@ -261,6 +265,45 @@ def _search_duplicate_specs_in_externals(error_cls):
|
||||
kwargs=('pkgs',)
|
||||
)
|
||||
|
||||
#: Sanity checks on linting
|
||||
# This can take some time, so it's run separately from packages
|
||||
package_https_directives = AuditClass(
|
||||
group='packages-https',
|
||||
tag='PKG-HTTPS-DIRECTIVES',
|
||||
description='Sanity checks on https checks of package urls, etc.',
|
||||
kwargs=('pkgs',)
|
||||
)
|
||||
|
||||
|
||||
@package_https_directives
|
||||
def _linting_package_file(pkgs, error_cls):
|
||||
"""Check for correctness of links
|
||||
"""
|
||||
import llnl.util.lang
|
||||
|
||||
import spack.repo
|
||||
import spack.spec
|
||||
|
||||
errors = []
|
||||
for pkg_name in pkgs:
|
||||
pkg = spack.repo.get(pkg_name)
|
||||
|
||||
# Does the homepage have http, and if so, does https work?
|
||||
if pkg.homepage.startswith('http://'):
|
||||
https = re.sub("http", "https", pkg.homepage, 1)
|
||||
try:
|
||||
response = urlopen(https)
|
||||
except Exception as e:
|
||||
msg = 'Error with attempting https for "{0}": '
|
||||
errors.append(error_cls(msg.format(pkg.name), [str(e)]))
|
||||
continue
|
||||
|
||||
if response.getcode() == 200:
|
||||
msg = 'Package "{0}" uses http but has a valid https endpoint.'
|
||||
errors.append(msg.format(pkg.name))
|
||||
|
||||
return llnl.util.lang.dedupe(errors)
|
||||
|
||||
|
||||
@package_directives
|
||||
def _unknown_variants_in_directives(pkgs, error_cls):
|
||||
|
@@ -2,6 +2,7 @@
|
||||
# Spack Project Developers. See the top-level COPYRIGHT file for details.
|
||||
#
|
||||
# SPDX-License-Identifier: (Apache-2.0 OR MIT)
|
||||
import llnl.util.tty as tty
|
||||
import llnl.util.tty.color as cl
|
||||
|
||||
import spack.audit
|
||||
@@ -19,12 +20,24 @@ def setup_parser(subparser):
|
||||
# Audit configuration files
|
||||
sp.add_parser('configs', help='audit configuration files')
|
||||
|
||||
# Https and other linting
|
||||
https_parser = sp.add_parser('packages-https', help='check https in packages')
|
||||
https_parser.add_argument(
|
||||
'--all',
|
||||
action='store_true',
|
||||
default=False,
|
||||
dest='check_all',
|
||||
help="audit all packages"
|
||||
)
|
||||
|
||||
# Audit package recipes
|
||||
pkg_parser = sp.add_parser('packages', help='audit package recipes')
|
||||
pkg_parser.add_argument(
|
||||
'name', metavar='PKG', nargs='*',
|
||||
help='package to be analyzed (if none all packages will be processed)',
|
||||
)
|
||||
|
||||
for group in [pkg_parser, https_parser]:
|
||||
group.add_argument(
|
||||
'name', metavar='PKG', nargs='*',
|
||||
help='package to be analyzed (if none all packages will be processed)',
|
||||
)
|
||||
|
||||
# List all checks
|
||||
sp.add_parser('list', help='list available checks and exits')
|
||||
@@ -41,6 +54,17 @@ def packages(parser, args):
|
||||
_process_reports(reports)
|
||||
|
||||
|
||||
def packages_https(parser, args):
|
||||
|
||||
# Since packages takes a long time, --all is required without name
|
||||
if not args.check_all and not args.name:
|
||||
tty.die("Please specify one or more packages to audit, or --all.")
|
||||
|
||||
pkgs = args.name or spack.repo.path.all_package_names()
|
||||
reports = spack.audit.run_group(args.subcommand, pkgs=pkgs)
|
||||
_process_reports(reports)
|
||||
|
||||
|
||||
def list(parser, args):
|
||||
for subcommand, check_tags in spack.audit.GROUPS.items():
|
||||
print(cl.colorize('@*b{' + subcommand + '}:'))
|
||||
@@ -58,6 +82,7 @@ def audit(parser, args):
|
||||
subcommands = {
|
||||
'configs': configs,
|
||||
'packages': packages,
|
||||
'packages-https': packages_https,
|
||||
'list': list
|
||||
}
|
||||
subcommands[args.subcommand](parser, args)
|
||||
|
@@ -31,3 +31,23 @@ def test_audit_configs(mutable_config, mock_packages):
|
||||
audit('configs', fail_on_error=False)
|
||||
# The mock configuration has duplicate definitions of some compilers
|
||||
assert audit.returncode == 1
|
||||
|
||||
|
||||
def test_audit_packages_https(mutable_config, mock_packages):
|
||||
|
||||
# Without providing --all should fail
|
||||
audit('packages-https', fail_on_error=False)
|
||||
# The mock configuration has duplicate definitions of some compilers
|
||||
assert audit.returncode == 1
|
||||
|
||||
# This uses http and should fail
|
||||
audit('packages-https', "preferred-test", fail_on_error=False)
|
||||
assert audit.returncode == 1
|
||||
|
||||
# providing one or more package names with https should work
|
||||
audit('packages-https', "cmake", fail_on_error=True)
|
||||
assert audit.returncode == 0
|
||||
|
||||
# providing one or more package names with https should work
|
||||
audit('packages-https', "cmake", "conflict", fail_on_error=True)
|
||||
assert audit.returncode == 0
|
||||
|
Reference in New Issue
Block a user