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.

4 files changed, 50 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,