From 2c74b433aad521c3dc8b38234a8948351ecad54c Mon Sep 17 00:00:00 2001 From: Scott Wittenburg Date: Wed, 26 Jul 2023 09:16:15 -0600 Subject: ci: Make signing requirement explicit (#38995) Instead of inferring whether to sign binaries, make it explicit, and fail rebuild jobs early if signing is required but cannot be accomplished. --- lib/spack/spack/ci.py | 7 ++--- lib/spack/spack/cmd/ci.py | 10 +++++- lib/spack/spack/test/ci.py | 5 +-- lib/spack/spack/test/cmd/ci.py | 37 +++++++++++++++++++++++ share/spack/gitlab/cloud_pipelines/.gitlab-ci.yml | 3 ++ 5 files changed, 53 insertions(+), 9 deletions(-) diff --git a/lib/spack/spack/ci.py b/lib/spack/spack/ci.py index a530c645c4..efe69eaabd 100644 --- a/lib/spack/spack/ci.py +++ b/lib/spack/spack/ci.py @@ -1278,6 +1278,7 @@ def generate_gitlab_ci_yaml( "SPACK_CI_SHARED_PR_MIRROR_URL": shared_pr_mirror or "None", "SPACK_REBUILD_CHECK_UP_TO_DATE": str(prune_dag), "SPACK_REBUILD_EVERYTHING": str(rebuild_everything), + "SPACK_REQUIRE_SIGNING": os.environ.get("SPACK_REQUIRE_SIGNING", "False"), } if remote_mirror_override: @@ -1957,9 +1958,9 @@ def process_command(name, commands, repro_dir): def create_buildcache( input_spec: spack.spec.Spec, *, - pr_pipeline: bool, pipeline_mirror_url: Optional[str] = None, buildcache_mirror_url: Optional[str] = None, + sign_binaries: bool = False, ) -> List[PushResult]: """Create the buildcache at the provided mirror(s). @@ -1967,12 +1968,10 @@ def create_buildcache( input_spec: Installed spec to package and push buildcache_mirror_url: URL for the buildcache mirror pipeline_mirror_url: URL for the pipeline mirror - pr_pipeline: True if the CI job is for a PR + sign_binaries: Whether or not to sign buildcache entry Returns: A list of PushResults, indicating success or failure. """ - sign_binaries = pr_pipeline is False and can_sign_binaries() - results = [] # Create buildcache in either the main remote mirror, or in the diff --git a/lib/spack/spack/cmd/ci.py b/lib/spack/spack/cmd/ci.py index 6e794d1bf5..0f7aad450a 100644 --- a/lib/spack/spack/cmd/ci.py +++ b/lib/spack/spack/cmd/ci.py @@ -18,6 +18,7 @@ import spack.config as cfg import spack.environment as ev import spack.hash_types as ht import spack.mirror +import spack.util.gpg as gpg_util import spack.util.url as url_util import spack.util.web as web_util @@ -270,6 +271,13 @@ def ci_rebuild(args): spack_ci_stack_name = os.environ.get("SPACK_CI_STACK_NAME") shared_pr_mirror_url = os.environ.get("SPACK_CI_SHARED_PR_MIRROR_URL") rebuild_everything = os.environ.get("SPACK_REBUILD_EVERYTHING") + require_signing = os.environ.get("SPACK_REQUIRE_SIGNING") + + # Fail early if signing is required but we don't have a signing key + sign_binaries = require_signing is not None and require_signing.lower() == "true" + if sign_binaries and not spack_ci.can_sign_binaries(): + gpg_util.list(False, True) + tty.die("SPACK_REQUIRE_SIGNING=True => spack must have exactly one signing key") # Construct absolute paths relative to current $CI_PROJECT_DIR ci_project_dir = os.environ.get("CI_PROJECT_DIR") @@ -655,7 +663,7 @@ def ci_rebuild(args): input_spec=job_spec, buildcache_mirror_url=buildcache_mirror_url, pipeline_mirror_url=pipeline_mirror_url, - pr_pipeline=spack_is_pr_pipeline, + sign_binaries=sign_binaries, ): msg = tty.msg if result.success else tty.warn msg( diff --git a/lib/spack/spack/test/ci.py b/lib/spack/spack/test/ci.py index 3ce64f7795..4a69edee2a 100644 --- a/lib/spack/spack/test/ci.py +++ b/lib/spack/spack/test/ci.py @@ -457,7 +457,6 @@ def test_ci_create_buildcache(tmpdir, working_env, config, mock_packages, monkey results = ci.create_buildcache( None, - pr_pipeline=True, buildcache_mirror_url="file:///fake-url-one", pipeline_mirror_url="file:///fake-url-two", ) @@ -469,9 +468,7 @@ def test_ci_create_buildcache(tmpdir, working_env, config, mock_packages, monkey assert result2.success assert result2.url == "file:///fake-url-two" - results = ci.create_buildcache( - None, pr_pipeline=True, buildcache_mirror_url="file:///fake-url-one" - ) + results = ci.create_buildcache(None, buildcache_mirror_url="file:///fake-url-one") assert len(results) == 1 assert results[0].success diff --git a/lib/spack/spack/test/cmd/ci.py b/lib/spack/spack/test/cmd/ci.py index 09d942cf51..65dcba3c6e 100644 --- a/lib/spack/spack/test/cmd/ci.py +++ b/lib/spack/spack/test/cmd/ci.py @@ -860,6 +860,43 @@ def test_ci_rebuild( env_cmd("deactivate") +def test_ci_require_signing( + tmpdir, working_env, mutable_mock_env_path, mock_gnupghome, ci_base_environment +): + spack_yaml_contents = """ +spack: + specs: + - archive-files + mirrors: + test-mirror: file:///no-such-mirror + ci: + pipeline-gen: + - submapping: + - match: + - archive-files + build-job: + tags: + - donotcare + image: donotcare +""" + filename = str(tmpdir.join("spack.yaml")) + with open(filename, "w") as f: + f.write(spack_yaml_contents) + + with tmpdir.as_cwd(): + env_cmd("activate", "--without-view", "--sh", "-d", ".") + + # Run without the variable to make sure we don't accidentally require signing + output = ci_cmd("rebuild", output=str, fail_on_error=False) + assert "spack must have exactly one signing key" not in output + + # Now run with the variable to make sure it works + os.environ.update({"SPACK_REQUIRE_SIGNING": "True"}) + output = ci_cmd("rebuild", output=str, fail_on_error=False) + + assert "spack must have exactly one signing key" in output + + def test_ci_nothing_to_rebuild( tmpdir, working_env, diff --git a/share/spack/gitlab/cloud_pipelines/.gitlab-ci.yml b/share/spack/gitlab/cloud_pipelines/.gitlab-ci.yml index fa5faf8665..4b927a04ae 100644 --- a/share/spack/gitlab/cloud_pipelines/.gitlab-ci.yml +++ b/share/spack/gitlab/cloud_pipelines/.gitlab-ci.yml @@ -67,6 +67,7 @@ default: variables: SPACK_PIPELINE_TYPE: "spack_protected_branch" SPACK_COPY_BUILDCACHE: "s3://spack-binaries/${CI_COMMIT_REF_NAME}" + SPACK_REQUIRE_SIGNING: "True" AWS_ACCESS_KEY_ID: ${PROTECTED_MIRRORS_AWS_ACCESS_KEY_ID} AWS_SECRET_ACCESS_KEY: ${PROTECTED_MIRRORS_AWS_SECRET_ACCESS_KEY} - if: $CI_COMMIT_REF_NAME =~ /^releases\/v.*/ @@ -77,6 +78,7 @@ default: SPACK_COPY_BUILDCACHE: "s3://spack-binaries/${CI_COMMIT_REF_NAME}" SPACK_PRUNE_UNTOUCHED: "False" SPACK_PRUNE_UP_TO_DATE: "False" + SPACK_REQUIRE_SIGNING: "True" AWS_ACCESS_KEY_ID: ${PROTECTED_MIRRORS_AWS_ACCESS_KEY_ID} AWS_SECRET_ACCESS_KEY: ${PROTECTED_MIRRORS_AWS_SECRET_ACCESS_KEY} - if: $CI_COMMIT_TAG =~ /^develop-[\d]{4}-[\d]{2}-[\d]{2}$/ || $CI_COMMIT_TAG =~ /^v.*/ @@ -797,6 +799,7 @@ deprecated-ci-build: when: always variables: SPACK_PIPELINE_TYPE: "spack_protected_branch" + SPACK_REQUIRE_SIGNING: "True" - if: $CI_COMMIT_REF_NAME =~ /^pr[\d]+_.*$/ # Pipelines on PR branches rebuild only what's missing, and do extra pruning when: always -- cgit v1.2.3-60-g2f50