zhangyiss/spack
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Build cache: make signed/unsigned a mirror property (#41507)

Browse Source 
* Add `signed` property to mirror config

* make unsigned a tri-state: true/false overrides mirror config, none takes mirror config

* test commands

* Document this

* add a test
This commit is contained in:
Harmen Stoppels
2023-12-11 22:14:59 +01:00
committed by GitHub
parent 045f398f3d
commit 8c29e90fa9
12 changed files with 237 additions and 63 deletions
Download Patch File Download Diff File Expand all files Collapse all files

38
lib/spack/docs/binary_caches.rst
View File

@@ -153,7 +153,43 @@ keyring, and trusting all downloaded keys.
List of popular build caches
^^^^^^^^^^^^^^^^^^^^^^^^^^^^
* `Extreme-scale Scientific Software Stack (E4S) <https://e4s-project.github.io/>`_: `build cache <https://oaciss.uoregon.edu/e4s/inventory.html>`_
* `Extreme-scale Scientific Software Stack (E4S) <https://e4s-project.github.io/>`_: `build cache <https://oaciss.uoregon.edu/e4s/inventory.html>`_'
-------------------
Build cache signing
-------------------
By default, Spack will add a cryptographic signature to each package pushed to
a build cache, and verifies the signature when installing from a build cache.
Keys for signing can be managed with the :ref:`spack gpg <cmd-spack-gpg>` command,
as well as ``spack buildcache keys`` as mentioned above.
You can disable signing when pushing with ``spack buildcache push --unsigned``,
and disable verification when installing from any build cache with
``spack install --no-check-signature``.
Alternatively, signing and verification can be enabled or disabled on a per build cache
basis:
.. code-block:: console
$ spack mirror add --signed <name> <url> # enable signing and verification
$ spack mirror add --unsigned <name> <url> # disable signing and verification
$ spack mirror set --signed <name> # enable signing and verification for an existing mirror
$ spack mirror set --unsigned <name> # disable signing and verification for an existing mirror
Or you can directly edit the ``mirrors.yaml`` configuration file:
.. code-block:: yaml
mirrors:
<name>:
url: <url>
signed: false # disable signing and verification
See also :ref:`mirrors`.
----------
Relocation

71
lib/spack/spack/binary_distribution.py
View File

@@ -25,7 +25,7 @@
import warnings
from contextlib import closing, contextmanager
from gzip import GzipFile
from typing import Dict, List, NamedTuple, Optional, Set, Tuple
from typing import Dict, Iterable, List, NamedTuple, Optional, Set, Tuple
from urllib.error import HTTPError, URLError
import llnl.util.filesystem as fsys
@@ -1605,14 +1605,14 @@ def _get_valid_spec_file(path: str, max_supported_layout: int) -> Tuple[Dict, in
return spec_dict, layout_version
def download_tarball(spec, unsigned=False, mirrors_for_spec=None):
def download_tarball(spec, unsigned: Optional[bool] = False, mirrors_for_spec=None):
"""
Download binary tarball for given package into stage area, returning
path to downloaded tarball if successful, None otherwise.
Args:
spec (spack.spec.Spec): Concrete spec
unsigned (bool): Whether or not to require signed binaries
unsigned: if ``True`` or ``False`` override the mirror signature verification defaults
mirrors_for_spec (list): Optional list of concrete specs and mirrors
obtained by calling binary_distribution.get_mirrors_for_spec().
These will be checked in order first before looking in other
@@ -1633,7 +1633,9 @@ def download_tarball(spec, unsigned=False, mirrors_for_spec=None):
"signature_verified": "true-if-binary-pkg-was-already-verified"
}
"""
configured_mirrors = spack.mirror.MirrorCollection(binary=True).values()
configured_mirrors: Iterable[spack.mirror.Mirror] = spack.mirror.MirrorCollection(
binary=True
).values()
if not configured_mirrors:
tty.die("Please add a spack mirror to allow download of pre-compiled packages.")
@@ -1651,8 +1653,16 @@ def download_tarball(spec, unsigned=False, mirrors_for_spec=None):
# mirror for the spec twice though.
try_first = [i["mirror_url"] for i in mirrors_for_spec] if mirrors_for_spec else []
try_next = [i.fetch_url for i in configured_mirrors if i.fetch_url not in try_first]
mirror_urls = try_first + try_next
mirrors = try_first + try_next
# TODO: turn `mirrors_for_spec` into a list of Mirror instances, instead of doing that here.
def fetch_url_to_mirror(url):
for mirror in configured_mirrors:
if mirror.fetch_url == url:
return mirror
return spack.mirror.Mirror(url)
mirrors = [fetch_url_to_mirror(url) for url in mirror_urls]
tried_to_verify_sigs = []
@@ -1661,14 +1671,17 @@ def download_tarball(spec, unsigned=False, mirrors_for_spec=None):
# we remove support for deprecated spec formats and buildcache layouts.
for try_signed in (True, False):
for mirror in mirrors:
# Override mirror's default if
currently_unsigned = unsigned if unsigned is not None else not mirror.signed
# If it's an OCI index, do things differently, since we cannot compose URLs.
parsed = urllib.parse.urlparse(mirror)
fetch_url = mirror.fetch_url
# TODO: refactor this to some "nice" place.
if parsed.scheme == "oci":
ref = spack.oci.image.ImageReference.from_string(mirror[len("oci://") :]).with_tag(
spack.oci.image.default_tag(spec)
)
if fetch_url.startswith("oci://"):
ref = spack.oci.image.ImageReference.from_string(
fetch_url[len("oci://") :]
).with_tag(spack.oci.image.default_tag(spec))
# Fetch the manifest
try:
@@ -1705,7 +1718,7 @@ def download_tarball(spec, unsigned=False, mirrors_for_spec=None):
except InvalidMetadataFile as e:
tty.warn(
f"Ignoring binary package for {spec.name}/{spec.dag_hash()[:7]} "
f"from {mirror} due to invalid metadata file: {e}"
f"from {fetch_url} due to invalid metadata file: {e}"
)
local_specfile_stage.destroy()
continue
@@ -1727,13 +1740,16 @@ def download_tarball(spec, unsigned=False, mirrors_for_spec=None):
"tarball_stage": tarball_stage,
"specfile_stage": local_specfile_stage,
"signature_verified": False,
"signature_required": not currently_unsigned,
}
else:
ext = "json.sig" if try_signed else "json"
specfile_path = url_util.join(mirror, BUILD_CACHE_RELATIVE_PATH, specfile_prefix)
specfile_path = url_util.join(
fetch_url, BUILD_CACHE_RELATIVE_PATH, specfile_prefix
)
specfile_url = f"{specfile_path}.{ext}"
spackfile_url = url_util.join(mirror, BUILD_CACHE_RELATIVE_PATH, tarball)
spackfile_url = url_util.join(fetch_url, BUILD_CACHE_RELATIVE_PATH, tarball)
local_specfile_stage = try_fetch(specfile_url)
if local_specfile_stage:
local_specfile_path = local_specfile_stage.save_filename
@@ -1746,21 +1762,21 @@ def download_tarball(spec, unsigned=False, mirrors_for_spec=None):
except InvalidMetadataFile as e:
tty.warn(
f"Ignoring binary package for {spec.name}/{spec.dag_hash()[:7]} "
f"from {mirror} due to invalid metadata file: {e}"
f"from {fetch_url} due to invalid metadata file: {e}"
)
local_specfile_stage.destroy()
continue
if try_signed and not unsigned:
if try_signed and not currently_unsigned:
# If we found a signed specfile at the root, try to verify
# the signature immediately. We will not download the
# tarball if we could not verify the signature.
tried_to_verify_sigs.append(specfile_url)
signature_verified = try_verify(local_specfile_path)
if not signature_verified:
tty.warn("Failed to verify: {0}".format(specfile_url))
tty.warn(f"Failed to verify: {specfile_url}")
if unsigned or signature_verified or not try_signed:
if currently_unsigned or signature_verified or not try_signed:
# We will download the tarball in one of three cases:
# 1. user asked for --no-check-signature
# 2. user didn't ask for --no-check-signature, but we
@@ -1783,6 +1799,7 @@ def download_tarball(spec, unsigned=False, mirrors_for_spec=None):
"tarball_stage": tarball_stage,
"specfile_stage": local_specfile_stage,
"signature_verified": signature_verified,
"signature_required": not currently_unsigned,
}
local_specfile_stage.destroy()
@@ -1981,7 +1998,7 @@ def is_backup_file(file):
relocate.relocate_text(text_names, prefix_to_prefix_text)
def _extract_inner_tarball(spec, filename, extract_to, unsigned, remote_checksum):
def _extract_inner_tarball(spec, filename, extract_to, signature_required: bool, remote_checksum):
stagepath = os.path.dirname(filename)
spackfile_name = tarball_name(spec, ".spack")
spackfile_path = os.path.join(stagepath, spackfile_name)
@@ -2001,7 +2018,7 @@ def _extract_inner_tarball(spec, filename, extract_to, unsigned, remote_checksum
else:
raise ValueError("Cannot find spec file for {0}.".format(extract_to))
if not unsigned:
if signature_required:
if os.path.exists("%s.asc" % specfile_path):
suppress = config.get("config:suppress_gpg_warnings", False)
try:
@@ -2050,7 +2067,7 @@ def _tar_strip_component(tar: tarfile.TarFile, prefix: str):
m.linkname = m.linkname[result.end() :]
def extract_tarball(spec, download_result, unsigned=False, force=False, timer=timer.NULL_TIMER):
def extract_tarball(spec, download_result, force=False, timer=timer.NULL_TIMER):
"""
extract binary tarball for given package into install area
"""
@@ -2076,7 +2093,8 @@ def extract_tarball(spec, download_result, unsigned=False, force=False, timer=ti
bchecksum = spec_dict["binary_cache_checksum"]
filename = download_result["tarball_stage"].save_filename
signature_verified = download_result["signature_verified"]
signature_verified: bool = download_result["signature_verified"]
signature_required: bool = download_result["signature_required"]
tmpdir = None
if layout_version == 0:
@@ -2085,7 +2103,9 @@ def extract_tarball(spec, download_result, unsigned=False, force=False, timer=ti
# and another tarball containing the actual install tree.
tmpdir = tempfile.mkdtemp()
try:
tarfile_path = _extract_inner_tarball(spec, filename, tmpdir, unsigned, bchecksum)
tarfile_path = _extract_inner_tarball(
spec, filename, tmpdir, signature_required, bchecksum
)
except Exception as e:
_delete_staged_downloads(download_result)
shutil.rmtree(tmpdir)
@@ -2098,9 +2118,10 @@ def extract_tarball(spec, download_result, unsigned=False, force=False, timer=ti
# the tarball.
tarfile_path = filename
if not unsigned and not signature_verified:
if signature_required and not signature_verified:
raise UnsignedPackageException(
"To install unsigned packages, use the --no-check-signature option."
"To install unsigned packages, use the --no-check-signature option, "
"or configure the mirror with signed: false."
)
# compute the sha256 checksum of the tarball
@@ -2213,7 +2234,7 @@ def install_root_node(spec, unsigned=False, force=False, sha256=None):
# don't print long padded paths while extracting/relocating binaries
with spack.util.path.filter_padding():
tty.msg('Installing "{0}" from a buildcache'.format(spec.format()))
extract_tarball(spec, download_result, unsigned, force)
extract_tarball(spec, download_result, force)
spack.hooks.post_install(spec, False)
spack.store.STORE.db.add(spec, spack.store.STORE.layout)

38
lib/spack/spack/cmd/buildcache.py
View File

@@ -76,7 +76,19 @@ def setup_parser(subparser: argparse.ArgumentParser):
)
push_sign = push.add_mutually_exclusive_group(required=False)
push_sign.add_argument(
"--unsigned", "-u", action="store_true", help="push unsigned buildcache tarballs"
"--unsigned",
"-u",
action="store_false",
dest="signed",
default=None,
help="push unsigned buildcache tarballs",
)
push_sign.add_argument(
"--signed",
action="store_true",
dest="signed",
default=None,
help="push signed buildcache tarballs",
)
push_sign.add_argument(
"--key", "-k", metavar="key", type=str, default=None, help="key for signing"
@@ -328,17 +340,27 @@ def push_fn(args):
"The flag `--allow-root` is the default in Spack 0.21, will be removed in Spack 0.22"
)
mirror: spack.mirror.Mirror = args.mirror
# Check if this is an OCI image.
try:
image_ref = spack.oci.oci.image_from_mirror(args.mirror)
image_ref = spack.oci.oci.image_from_mirror(mirror)
except ValueError:
image_ref = None
push_url = mirror.push_url
# When neither --signed, --unsigned nor --key are specified, use the mirror's default.
if args.signed is None and not args.key:
unsigned = not mirror.signed
else:
unsigned = not (args.key or args.signed)
# For OCI images, we require dependencies to be pushed for now.
if image_ref:
if "dependencies" not in args.things_to_install:
tty.die("Dependencies must be pushed for OCI images.")
if not args.unsigned:
if not unsigned:
tty.warn(
"Code signing is currently not supported for OCI images. "
"Use --unsigned to silence this warning."
@@ -351,12 +373,10 @@ def push_fn(args):
dependencies="dependencies" in args.things_to_install,
)
url = args.mirror.push_url
# When pushing multiple specs, print the url once ahead of time, as well as how
# many specs are being pushed.
if len(specs) > 1:
tty.info(f"Selected {len(specs)} specs to push to {url}")
tty.info(f"Selected {len(specs)} specs to push to {push_url}")
failed = []
@@ -373,10 +393,10 @@ def push_fn(args):
try:
bindist.push_or_raise(
spec,
url,
push_url,
bindist.PushOptions(
force=args.force,
unsigned=args.unsigned,
unsigned=unsigned,
key=args.key,
regenerate_index=args.update_index,
),
@@ -384,7 +404,7 @@ def push_fn(args):
msg = f"{_progress(i, len(specs))}Pushed {_format_spec(spec)}"
if len(specs) == 1:
msg += f" to {url}"
msg += f" to {push_url}"
tty.info(msg)
except bindist.NoOverwriteException:

4
lib/spack/spack/cmd/install.py
View File

@@ -162,8 +162,8 @@ def setup_parser(subparser):
"--no-check-signature",
action="store_true",
dest="unsigned",
default=False,
help="do not check signatures of binary packages",
default=None,
help="do not check signatures of binary packages (override mirror config)",
)
subparser.add_argument(
"--show-log-on-error",

39
lib/spack/spack/cmd/mirror.py
View File

@@ -107,6 +107,23 @@ def setup_parser(subparser):
"and source use `--type binary --type source` (default)"
),
)
add_parser_signed = add_parser.add_mutually_exclusive_group(required=False)
add_parser_signed.add_argument(
"--unsigned",
help="do not require signing and signature verification when pushing and installing from "
"this build cache",
action="store_false",
default=None,
dest="signed",
)
add_parser_signed.add_argument(
"--signed",
help="require signing and signature verification when pushing and installing from this "
"build cache",
action="store_true",
default=None,
dest="signed",
)
arguments.add_connection_args(add_parser, False)
# Remove
remove_parser = sp.add_parser("remove", aliases=["rm"], help=mirror_remove.__doc__)
@@ -157,6 +174,23 @@ def setup_parser(subparser):
),
)
set_parser.add_argument("--url", help="url of mirror directory from 'spack mirror create'")
set_parser_unsigned = set_parser.add_mutually_exclusive_group(required=False)
set_parser_unsigned.add_argument(
"--unsigned",
help="do not require signing and signature verification when pushing and installing from "
"this build cache",
action="store_false",
default=None,
dest="signed",
)
set_parser_unsigned.add_argument(
"--signed",
help="require signing and signature verification when pushing and installing from this "
"build cache",
action="store_true",
default=None,
dest="signed",
)
set_parser.add_argument(
"--scope",
action=arguments.ConfigScope,
@@ -186,6 +220,7 @@ def mirror_add(args):
or args.type
or args.oci_username
or args.oci_password
or args.signed is not None
):
connection = {"url": args.url}
if args.s3_access_key_id and args.s3_access_key_secret:
@@ -201,6 +236,8 @@ def mirror_add(args):
if args.type:
connection["binary"] = "binary" in args.type
connection["source"] = "source" in args.type
if args.signed is not None:
connection["signed"] = args.signed
mirror = spack.mirror.Mirror(connection, name=args.name)
else:
mirror = spack.mirror.Mirror(args.url, name=args.name)
@@ -233,6 +270,8 @@ def _configure_mirror(args):
changes["endpoint_url"] = args.s3_endpoint_url
if args.oci_username and args.oci_password:
changes["access_pair"] = [args.oci_username, args.oci_password]
if getattr(args, "signed", None) is not None:
changes["signed"] = args.signed
# argparse cannot distinguish between --binary and --no-binary when same dest :(
# notice that set-url does not have these args, so getattr

26
lib/spack/spack/installer.py
View File

@@ -381,7 +381,10 @@ def _print_timer(pre: str, pkg_id: str, timer: timer.BaseTimer) -> None:
def _install_from_cache(
pkg: "spack.package_base.PackageBase", cache_only: bool, explicit: bool, unsigned: bool = False
pkg: "spack.package_base.PackageBase",
cache_only: bool,
explicit: bool,
unsigned: Optional[bool] = False,
) -> bool:
"""
Extract the package from binary cache
@@ -391,8 +394,7 @@ def _install_from_cache(
cache_only: only extract from binary cache
explicit: ``True`` if installing the package was explicitly
requested by the user, otherwise, ``False``
unsigned: ``True`` if binary package signatures to be checked,
otherwise, ``False``
unsigned: if ``True`` or ``False`` override the mirror signature verification defaults
Return: ``True`` if the package was extract from binary cache, ``False`` otherwise
"""
@@ -462,7 +464,7 @@ def _process_external_package(pkg: "spack.package_base.PackageBase", explicit: b
def _process_binary_cache_tarball(
pkg: "spack.package_base.PackageBase",
explicit: bool,
unsigned: bool,
unsigned: Optional[bool],
mirrors_for_spec: Optional[list] = None,
timer: timer.BaseTimer = timer.NULL_TIMER,
) -> bool:
@@ -472,8 +474,7 @@ def _process_binary_cache_tarball(
Args:
pkg: the package being installed
explicit: the package was explicitly requested by the user
unsigned: ``True`` if binary package signatures to be checked,
otherwise, ``False``
unsigned: if ``True`` or ``False`` override the mirror signature verification defaults
mirrors_for_spec: Optional list of concrete specs and mirrors
obtained by calling binary_distribution.get_mirrors_for_spec().
timer: timer to keep track of binary install phases.
@@ -493,9 +494,7 @@ def _process_binary_cache_tarball(
tty.msg(f"Extracting {package_id(pkg)} from binary cache")
with timer.measure("install"), spack.util.path.filter_padding():
binary_distribution.extract_tarball(
pkg.spec, download_result, unsigned=unsigned, force=False, timer=timer
)
binary_distribution.extract_tarball(pkg.spec, download_result, force=False, timer=timer)
pkg.installed_from_binary_cache = True
spack.store.STORE.db.add(pkg.spec, spack.store.STORE.layout, explicit=explicit)
@@ -505,7 +504,7 @@ def _process_binary_cache_tarball(
def _try_install_from_binary_cache(
pkg: "spack.package_base.PackageBase",
explicit: bool,
unsigned: bool = False,
unsigned: Optional[bool] = None,
timer: timer.BaseTimer = timer.NULL_TIMER,
) -> bool:
"""
@@ -514,8 +513,7 @@ def _try_install_from_binary_cache(
Args:
pkg: package to be extracted from binary cache
explicit: the package was explicitly requested by the user
unsigned: ``True`` if binary package signatures to be checked,
otherwise, ``False``
unsigned: if ``True`` or ``False`` override the mirror signature verification defaults
timer: timer to keep track of binary install phases.
"""
# Early exit if no binary mirrors are configured.
@@ -825,7 +823,7 @@ def _add_default_args(self) -> None:
("restage", False),
("skip_patch", False),
("tests", False),
("unsigned", False),
("unsigned", None),
("verbose", False),
]:
_ = self.install_args.setdefault(arg, default)
@@ -1663,7 +1661,7 @@ def _install_task(self, task: BuildTask, install_status: InstallStatus) -> None:
use_cache = task.use_cache
tests = install_args.get("tests", False)
assert isinstance(tests, (bool, list)) # make mypy happy.
unsigned = bool(install_args.get("unsigned"))
unsigned: Optional[bool] = install_args.get("unsigned")
pkg, pkg_id = task.pkg, task.pkg_id

6
lib/spack/spack/mirror.py
View File

@@ -133,6 +133,10 @@ def binary(self):
def source(self):
return isinstance(self._data, str) or self._data.get("source", True)
@property
def signed(self) -> bool:
return isinstance(self._data, str) or self._data.get("signed", True)
@property
def fetch_url(self):
"""Get the valid, canonicalized fetch URL"""
@@ -146,7 +150,7 @@ def push_url(self):
def _update_connection_dict(self, current_data: dict, new_data: dict, top_level: bool):
keys = ["url", "access_pair", "access_token", "profile", "endpoint_url"]
if top_level:
keys += ["binary", "source"]
keys += ["binary", "source", "signed"]
changed = False
for key in keys:
if key in new_data and current_data.get(key) != new_data[key]:

1
lib/spack/spack/schema/mirrors.py
View File

@@ -42,6 +42,7 @@
"properties": {
"source": {"type": "boolean"},
"binary": {"type": "boolean"},
"signed": {"type": "boolean"},
"fetch": fetch_and_push,
"push": fetch_and_push,
**connection, # type: ignore

34
lib/spack/spack/test/cmd/buildcache.py
View File

@@ -331,3 +331,37 @@ def fake_push(node, push_url, options):
# Ensure no duplicates
assert len(set(packages_to_push)) == len(packages_to_push)
@pytest.mark.parametrize("signed", [True, False])
def test_push_and_install_with_mirror_marked_unsigned_does_not_require_extra_flags(
tmp_path, mutable_database, mock_gnupghome, signed
):
"""Tests whether marking a mirror as unsigned makes it possible to push and install to/from
it without requiring extra flags on the command line (and no signing keys configured)."""
# Create a named mirror with signed set to True or False
add_flag = "--signed" if signed else "--unsigned"
mirror("add", add_flag, "my-mirror", str(tmp_path))
spec = mutable_database.query_local("libelf", installed=True)[0]
# Push
if signed:
# Need to pass "--unsigned" to override the mirror's default
args = ["push", "--update-index", "--unsigned", "my-mirror", f"/{spec.dag_hash()}"]
else:
# No need to pass "--unsigned" if the mirror is unsigned
args = ["push", "--update-index", "my-mirror", f"/{spec.dag_hash()}"]
buildcache(*args)
# Install
if signed:
# Need to pass "--no-check-signature" to avoid install errors
kwargs = {"cache_only": True, "unsigned": True}
else:
# No need to pass "--no-check-signature" if the mirror is unsigned
kwargs = {"cache_only": True}
spec.package.do_uninstall(force=True)
spec.package.do_install(**kwargs)

9
lib/spack/spack/test/cmd/mirror.py
View File

@@ -398,3 +398,12 @@ def test_mirror_set_2(mutable_config):
"url": "http://example.com",
"push": {"url": "http://example2.com", "access_pair": ["username", "password"]},
}
def test_mirror_add_set_signed(mutable_config):
mirror("add", "--signed", "example", "http://example.com")
assert spack.config.get("mirrors:example") == {"url": "http://example.com", "signed": True}
mirror("set", "--unsigned", "example")
assert spack.config.get("mirrors:example") == {"url": "http://example.com", "signed": False}
mirror("set", "--signed", "example")
assert spack.config.get("mirrors:example") == {"url": "http://example.com", "signed": True}