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

@@ -1278,6 +1278,7 @@ def main_script_replacements(cmd):
             "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

lib/spack/spack/cmd/ci.py

@@ -18,6 +18,7 @@
 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(

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(
+    results = ci.create_buildcache(None, buildcache_mirror_url="file:///fake-url-one")
-        None, pr_pipeline=True, buildcache_mirror_url="file:///fake-url-one"
-    )
 
     assert len(results) == 1
     assert results[0].success

lib/spack/spack/test/cmd/ci.py

@@ -860,6 +860,43 @@ def mystrip(s):
         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,

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