diff --git a/.github/scripts/prepare-deploy.py b/.github/scripts/prepare-deploy.py new file mode 100644 index 0000000..a874b77 --- /dev/null +++ b/.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.") diff --git a/.github/scripts/test_prepare_deploy.py b/.github/scripts/test_prepare_deploy.py new file mode 100644 index 0000000..d8831d0 --- /dev/null +++ b/.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() diff --git a/.github/workflows/deploy.yml b/.github/workflows/deploy.yml index 39c0540..8dd219a 100644 --- a/.github/workflows/deploy.yml +++ b/.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 \ No newline at end of file + NON_SECRET_CONFIG: '{"GOOGLE_CLIENT_ID": "NOTE_GOOGLE_CLIENT_ID", "GOOGLE_CALLBACK_URI": "NOTE_GOOGLE_CALLBACK_URI"}' + run: python3 .github/scripts/prepare-deploy.py