Move non-sensitive runtime configuration out of Secret Manager

.github/scripts/prepare-deploy.py

@@ -0,0 +1,113 @@
+"""Prepare an atomic Cloud Run deployment, migrating only allowlisted configuration."""
+
+import copy
+import json
+import os
+from pathlib import Path
+import subprocess
+import sys
+
+
+def gcloud(*args):
+    result = subprocess.run(
+        ["gcloud", *args, "--quiet"], capture_output=True, text=True, check=True
+    )
+    return result.stdout
+
+
+def prepare(service, image, config_names, read_secret):
+    # Export only writable metadata; keep the service's runtime configuration.
+    manifest = {
+        "apiVersion": "serving.knative.dev/v1",
+        "kind": "Service",
+        "metadata": {
+            key: copy.deepcopy(service["metadata"][key])
+            for key in ("name", "namespace", "labels", "annotations")
+            if key in service["metadata"]
+        },
+        "spec": copy.deepcopy(service["spec"]),
+    }
+    annotations = manifest["metadata"].get("annotations", {})
+    for key in (
+        "serving.knative.dev/creator", "serving.knative.dev/lastModifier",
+        "run.googleapis.com/operation-id", "run.googleapis.com/ingress-status",
+        "run.googleapis.com/urls", "run.googleapis.com/client-name",
+        "run.googleapis.com/client-version",
+    ):
+        annotations.pop(key, None)
+    template = manifest["spec"]["template"]
+    template.setdefault("metadata", {}).pop("name", None)
+    containers = template["spec"]["containers"]
+    if len(containers) != 1:
+        raise ValueError("Expected one container; refusing to change another container")
+    container = containers[0]
+    env = {entry["name"]: entry for entry in container.get("env", [])}
+    for name, expected_secret in config_names.items():
+        entry = env.get(name)
+        if not entry:
+            raise ValueError(f"Missing existing configuration: {name}")
+        if "value" in entry:
+            continue  # Subsequent deployments no longer read Secret Manager.
+        ref = entry.get("valueFrom", {}).get("secretKeyRef", {})
+        if ref.get("name") != expected_secret:
+            raise ValueError(f"Unexpected secret reference for {name}; refusing conversion")
+        value = read_secret(expected_secret, ref["key"])
+        if not value:
+            raise ValueError(f"Empty configuration: {name}")
+        env[name] = {"name": name, "value": value}
+    container["env"] = list(env.values())
+    container["image"] = image
+    # The new revision must receive traffic before the old secrets can be retired.
+    manifest["spec"]["traffic"] = [{"latestRevision": True, "percent": 100}]
+    return manifest
+
+
+def main():
+    service_name, region, image = (os.environ[k] for k in ("SERVICE_NAME", "GCP_REGION", "IMAGE"))
+    config_names = json.loads(os.environ["NON_SECRET_CONFIG"])
+    service = json.loads(gcloud("run", "services", "describe", service_name,
+                                "--region", region, "--format=json"))
+    project = service["metadata"]["namespace"]
+
+    def read_secret(name, version):
+        return gcloud("secrets", "versions", "access", version,
+                      "--secret", name, "--project", project)
+
+    manifest = prepare(service, image, config_names, read_secret)
+    path = Path(os.environ["RUNNER_TEMP"]) / "cloud-run-service.json"
+    path.write_text(json.dumps(manifest), encoding="utf-8")
+    path.chmod(0o600)
+    try:
+        # Replacing the manifest switches secretKeyRef to value in one revision,
+        # avoiding an intermediate deployment with missing configuration.
+        gcloud("run", "services", "replace", str(path), "--region", region)
+        deployed = json.loads(gcloud("run", "services", "describe", service_name,
+                                    "--region", region, "--format=json"))
+        status = deployed["status"]
+        ready = status.get("latestReadyRevisionName")
+        if not any(c.get("type") == "Ready" and c.get("status") == "True"
+                   for c in status.get("conditions", [])):
+            raise ValueError("The service is not ready")
+        if not ready:
+            raise ValueError("A ready revision was not confirmed")
+        if ready != status.get("latestCreatedRevisionName"):
+            raise ValueError("The latest revision is not ready")
+        if not any(t.get("revisionName") == ready and t.get("percent") == 100
+                   for t in status.get("traffic", [])):
+            raise ValueError("The new revision is not serving all traffic")
+        current = {e["name"]: e for e in deployed["spec"]["template"]["spec"]["containers"][0]["env"]}
+        expected = {e["name"]: e for e in manifest["spec"]["template"]["spec"]["containers"][0]["env"]}
+        if current != expected:
+            raise ValueError("Deployed configuration differs from the prepared values")
+        print(f"Previous ready revision: {service.get('status', {}).get('latestReadyRevisionName')}")
+        print(f"Verified ready revision {ready}: configuration uses ordinary environment variables.")
+    finally:
+        path.unlink(missing_ok=True)
+
+
+if __name__ == "__main__":
+    try:
+        main()
+    except subprocess.CalledProcessError as error:
+        # Never print captured command output, which may contain configuration.
+        sys.exit(f"gcloud failed (exit {error.returncode}); no secret values were logged.")

.github/scripts/test_prepare_deploy.py

@@ -0,0 +1,100 @@
+import copy
+import importlib.util
+import json
+import os
+from pathlib import Path
+import tempfile
+import unittest
+from unittest.mock import Mock, patch
+
+spec = importlib.util.spec_from_file_location("prepare_deploy", Path(__file__).with_name("prepare-deploy.py"))
+module = importlib.util.module_from_spec(spec)
+spec.loader.exec_module(module)
+
+
+class MigrationTests(unittest.TestCase):
+    def setUp(self):
+        self.service = {
+            "metadata": {"name": "app", "namespace": "123", "uid": "readonly",
+                         "annotations": {"run.googleapis.com/ingress": "all",
+                                         "run.googleapis.com/operation-id": "readonly"}},
+            "spec": {"template": {"metadata": {"name": "app-old"}, "spec": {
+                "serviceAccountName": "runtime@example.com",
+                "containers": [{"image": "old", "env": [
+                    {"name": "HOST", "valueFrom": {"secretKeyRef": {"name": "CONFIG_HOST", "key": "latest"}}},
+                    {"name": "PASSWORD", "valueFrom": {"secretKeyRef": {"name": "PASSWORD", "key": "1"}}},
+                    {"name": "MODE", "value": "production"}]}]}},
+                "traffic": [{"revisionName": "app-old", "percent": 100}]},
+            "status": {"latestReadyRevisionName": "app-old"},
+        }
+
+    def test_only_allowlisted_configuration_is_converted_and_runtime_is_preserved(self):
+        original = copy.deepcopy(self.service)
+        reader = Mock(return_value="host,with=special\ncharacters")
+        result = module.prepare(self.service, "new-image", {"HOST": "CONFIG_HOST"}, reader)
+        env = result["spec"]["template"]["spec"]["containers"][0]["env"]
+        self.assertEqual(env[0], {"name": "HOST", "value": "host,with=special\ncharacters"})
+        self.assertEqual(env[1:], original["spec"]["template"]["spec"]["containers"][0]["env"][1:])
+        self.assertEqual(result["spec"]["template"]["spec"]["serviceAccountName"], "runtime@example.com")
+        self.assertEqual(result["spec"]["template"]["spec"]["containers"][0]["image"], "new-image")
+        self.assertNotIn("name", result["spec"]["template"]["metadata"])
+        self.assertNotIn("uid", result["metadata"])
+        self.assertEqual(result["metadata"]["annotations"], {"run.googleapis.com/ingress": "all"})
+        self.assertEqual(result["spec"]["traffic"], [{"latestRevision": True, "percent": 100}])
+        self.assertEqual(self.service, original)
+        reader.assert_called_once_with("CONFIG_HOST", "latest")
+
+    def test_redeployment_never_reads_the_deleted_secret(self):
+        first = module.prepare(self.service, "image", {"HOST": "CONFIG_HOST"}, lambda *_: "db.example")
+        reader = Mock(side_effect=AssertionError("Secret must not be accessed again"))
+        second = module.prepare(first, "new-image", {"HOST": "CONFIG_HOST"}, reader)
+        self.assertEqual(second["spec"]["template"]["spec"]["containers"][0]["env"][0]["value"], "db.example")
+        reader.assert_not_called()
+
+    def test_unexpected_secret_is_not_exposed(self):
+        with self.assertRaisesRegex(ValueError, "Unexpected secret"):
+            module.prepare(self.service, "image", {"HOST": "OTHER"}, Mock())
+
+    def test_missing_or_empty_config_stops_before_deployment(self):
+        with self.assertRaisesRegex(ValueError, "Missing existing"):
+            module.prepare(self.service, "image", {"MISSING": "MISSING"}, Mock())
+        with self.assertRaisesRegex(ValueError, "Empty configuration"):
+            module.prepare(self.service, "image", {"HOST": "CONFIG_HOST"}, lambda *_: "")
+
+    def test_multiple_containers_are_rejected(self):
+        self.service["spec"]["template"]["spec"]["containers"].append({"image": "sidecar"})
+        with self.assertRaisesRegex(ValueError, "one container"):
+            module.prepare(self.service, "image", {"HOST": "CONFIG_HOST"}, Mock())
+
+    def test_deployment_is_verified_and_temporary_manifest_is_removed(self):
+        deployed = module.prepare(self.service, "new-image", {"HOST": "CONFIG_HOST"}, lambda *_: "db.example")
+        deployed["status"] = {
+            "conditions": [{"type": "Ready", "status": "True"}],
+            "latestReadyRevisionName": "app-new", "latestCreatedRevisionName": "app-new",
+            "traffic": [{"revisionName": "app-new", "percent": 100}],
+        }
+        with tempfile.TemporaryDirectory() as directory:
+            settings = {"SERVICE_NAME": "app", "GCP_REGION": "region", "IMAGE": "new-image",
+                        "NON_SECRET_CONFIG": '{"HOST":"CONFIG_HOST"}', "RUNNER_TEMP": directory}
+            with patch.dict(os.environ, settings), patch.object(module, "gcloud", side_effect=[
+                json.dumps(self.service), "db.example", "", json.dumps(deployed)
+            ]) as cli:
+                module.main()
+            self.assertFalse((Path(directory) / "cloud-run-service.json").exists())
+            self.assertEqual(cli.call_args_list[2].args[:3], ("run", "services", "replace"))
+
+    def test_secret_access_failure_prevents_any_deployment(self):
+        with tempfile.TemporaryDirectory() as directory:
+            settings = {"SERVICE_NAME": "app", "GCP_REGION": "region", "IMAGE": "new-image",
+                        "NON_SECRET_CONFIG": '{"HOST":"CONFIG_HOST"}', "RUNNER_TEMP": directory}
+            with patch.dict(os.environ, settings), patch.object(module, "gcloud", side_effect=[
+                json.dumps(self.service), RuntimeError("Access denied")
+            ]) as cli:
+                with self.assertRaisesRegex(RuntimeError, "Access denied"):
+                    module.main()
+            self.assertEqual(cli.call_count, 2)
+            self.assertFalse((Path(directory) / "cloud-run-service.json").exists())
+
+
+if __name__ == "__main__":
+    unittest.main()

.github/workflows/deploy.yml

@@ -1,11 +1,12 @@
 name: Deploy to Cloud Run
 
 on:
+  pull_request:
   push:
     branches: ["master"]
 
 concurrency:
-  group: deploy-main
+  group: ${{ github.workflow }}-${{ github.ref }}
   cancel-in-progress: true
 
 permissions:
@@ -19,7 +20,16 @@ env:
   SERVICE_NAME: ${{ vars.SERVICE_NAME || 'webnotetakingapp-service' }}
 
 jobs:
+  validate:
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v4
+      - name: Test configuration migration
+        run: python3 -m unittest discover -s .github/scripts -v
+
   deploy:
+    if: github.event_name == 'push'
+    needs: validate
     name: Build, Push, and Deploy
     runs-on: ubuntu-latest
 
@@ -68,22 +78,8 @@ jobs:
       - name: Set up gcloud SDK
         uses: google-github-actions/setup-gcloud@v3
 
-      - name: Deploy to Cloud Run
-        shell: bash
+      - name: Deploy to Cloud Run and verify configuration
         env:
           IMAGE: ${{ env.DOCKERHUB_REPO }}:${{ steps.vars.outputs.SHORT_SHA }}
-        run: |
-          if [[ -z "${SERVICE_NAME}" ]]; then
-            echo "SERVICE_NAME variable is not set (Repository variable)." >&2
-            exit 1
-          fi
-
-          gcloud run deploy "${SERVICE_NAME}" \
-            --image "$IMAGE" \
-            --region "${GCP_REGION}" \
-            --platform managed \
-            --allow-unauthenticated \
-            --port 3000 \
-            --set-env-vars NODE_ENV=production,\
-            --set-secrets MONGODB_URI=NOTE_MONGODB_URI:latest,SESSION_SECRET=NOTE_SESSION_SECRET:latest,GOOGLE_CLIENT_ID=NOTE_GOOGLE_CLIENT_ID:latest,GOOGLE_CLIENT_SECRET=NOTE_GOOGLE_CLIENT_SECRET:latest,GOOGLE_CALLBACK_URI=NOTE_GOOGLE_CALLBACK_URI:latest \
-            --min-instances=0 --max-instances=2
+          NON_SECRET_CONFIG: '{"GOOGLE_CLIENT_ID": "NOTE_GOOGLE_CLIENT_ID", "GOOGLE_CALLBACK_URI": "NOTE_GOOGLE_CALLBACK_URI"}'
+        run: python3 .github/scripts/prepare-deploy.py