From 0b6de5d0e4609108edf0686abe228b9e88fbb9b3 Mon Sep 17 00:00:00 2001 From: riddim-developer-bot Date: Sun, 14 Jun 2026 13:33:42 -0400 Subject: [PATCH] EPAC-2288 expose bill diff deployment route --- backend/manifest/deployment-services.json | 8 ++ backend/openapi/main_test.go | 2 + scripts/ci/backend_staging_smoke.py | 79 ++++++++++++++++++- .../ci/check_backend_manifest_deployment.py | 70 +++++++++++++++- scripts/ci/test_backend_staging_smoke.py | 39 +++++++++ .../tests/test_backend_manifest_deployment.py | 69 ++++++++++++++++ 6 files changed, 263 insertions(+), 4 deletions(-) diff --git a/backend/manifest/deployment-services.json b/backend/manifest/deployment-services.json index a0346122..60d949b4 100644 --- a/backend/manifest/deployment-services.json +++ b/backend/manifest/deployment-services.json @@ -156,6 +156,10 @@ { "method": "GET", "path": "/api/v1/bills/{id}/committee-stage" + }, + { + "method": "GET", + "path": "/api/v1/bills/{id}/diff" } ], "production": [ @@ -170,6 +174,10 @@ { "method": "GET", "path": "/api/v1/bills/{id}/committee-stage" + }, + { + "method": "GET", + "path": "/api/v1/bills/{id}/diff" } ] } diff --git a/backend/openapi/main_test.go b/backend/openapi/main_test.go index da6b0684..68558570 100644 --- a/backend/openapi/main_test.go +++ b/backend/openapi/main_test.go @@ -43,6 +43,7 @@ func TestOpenAPISpecEndpoint(t *testing.T) { "/api/v1/bills", "/api/v1/bills/{id}", "/api/v1/bills/{id}/committee-stage", + "/api/v1/bills/{id}/diff", "/api/v1/bills/{legisinfo_id}/lobbying-context", "/api/v1/calendar/house.ics", "/api/v1/config", @@ -77,6 +78,7 @@ func TestRequiredPathsHaveResponseSchemasAndExamples(t *testing.T) { "/api/v1/bills", "/api/v1/bills/{id}", "/api/v1/bills/{id}/committee-stage", + "/api/v1/bills/{id}/diff", "/api/v1/bills/{legisinfo_id}/lobbying-context", "/api/v1/calendar/house.ics", "/api/v1/config", diff --git a/scripts/ci/backend_staging_smoke.py b/scripts/ci/backend_staging_smoke.py index bac4160e..96644633 100755 --- a/scripts/ci/backend_staging_smoke.py +++ b/scripts/ci/backend_staging_smoke.py @@ -63,6 +63,9 @@ class SmokeCheck: # of which services are deployed. Set to a service name to skip automatically # when that service has deploy.=false in the manifest. service: str | None = None + # Full checks require known seeded/backfilled data and are skipped in the + # default contract mode. + full_only: bool = False def url(self, base_url: str) -> str: base = base_url.rstrip("/") @@ -110,6 +113,45 @@ def validate_bills(_: int, payload: Any) -> None: require_non_empty_list(body, "bills", "bills") +def is_api_gateway_not_found(status: int, payload: Any) -> bool: + return ( + status == 404 + and isinstance(payload, dict) + and payload.get("message") == "Not Found" + and "error" not in payload + ) + + +def validate_bill_diff_route(status: int, payload: Any) -> None: + body = require_dict(payload, "bills:diff-route") + if is_api_gateway_not_found(status, body): + raise SmokeFailure("bills:diff-route: API Gateway returned Not Found; route is missing or unsynced") + if status != 400: + raise SmokeFailure(f"bills:diff-route: expected service-owned HTTP 400 for missing from/to, got {status}") + if "error" not in body: + raise SmokeFailure("bills:diff-route: service-owned 400 response missing error key") + error_text = str(body["error"]).lower() + for required in ("from", "to"): + if required not in error_text: + raise SmokeFailure( + f"bills:diff-route: error body does not mention missing {required!r}: {body['error']}" + ) + + +def validate_bill_diff_payload(status: int, payload: Any) -> None: + body = require_dict(payload, "bills:diff-full") + if is_api_gateway_not_found(status, body): + raise SmokeFailure("bills:diff-full: API Gateway returned Not Found; route is missing or unsynced") + if status != 200: + raise SmokeFailure(f"bills:diff-full: expected HTTP 200 seeded diff payload, got {status}") + require_keys(body, "bills:diff-full", {"from", "to", "clauses"}) + for key in ("from", "to"): + version = body[key] + if not isinstance(version, dict) or not isinstance(version.get("id"), str) or not version["id"]: + raise SmokeFailure(f"bills:diff-full: {key} version must include a non-empty id") + require_non_empty_list(body, "bills:diff-full", "clauses") + + def validate_members(_: int, payload: Any) -> None: body = require_dict(payload, "members") require_keys(body, "members", {"members"}) @@ -291,6 +333,29 @@ def validate_hansard_search_manifest(status: int, payload: Any) -> None: deterministic_note="Contract check verifies the bills list endpoint is backed by a non-empty published artifact.", fixture_note="No fixture required; the current Parliament bills dataset should not be empty after ingestion.", ), + SmokeCheck( + name="bills:diff-route", + method="GET", + path="/api/v1/bills/C-8/diff", + query={}, + expected_statuses={400, 404}, + validator=validate_bill_diff_route, + service="bills", + deterministic_note="Route-reachability check omits from/to so the bills service returns its own HTTP 400 before diff data is required.", + fixture_note="No backfilled diff fixture required; API Gateway 404 is treated as a route exposure failure.", + ), + SmokeCheck( + name="bills:diff-full", + method="GET", + path="/api/v1/bills/C-8/diff", + query={"from": "C-8-v1", "to": "C-8-v3"}, + expected_statuses={200, 404}, + validator=validate_bill_diff_payload, + service="bills", + deterministic_note="Full-mode check asserts a seeded current-Parliament multi-version bill returns a concrete diff payload.", + fixture_note="Requires C-8 diff data to be backfilled in the selected environment; skipped unless --mode full is used.", + full_only=True, + ), SmokeCheck( name="members:list", method="GET", @@ -721,7 +786,8 @@ def list_checks() -> None: for check in CHECKS: query = f"?{urllib.parse.urlencode(check.query)}" if check.query else "" svc = f" [{check.service}]" if check.service else " [always]" - print(f"{check.method} {check.path}{query} - {check.name}{svc}") + mode = " [full]" if check.full_only else "" + print(f"{check.method} {check.path}{query} - {check.name}{svc}{mode}") def main() -> int: @@ -743,6 +809,12 @@ def main() -> int: default=os.environ.get("EPAC_BACKEND_SMOKE_SERVICES", os.environ.get("EPAC_STAGING_SMOKE_SERVICES", "")), help="JSON array or comma-separated service names to smoke. Defaults to manifest services with deploy.=true.", ) + parser.add_argument( + "--mode", + choices=("contract", "full"), + default=os.environ.get("EPAC_BACKEND_SMOKE_MODE", "contract"), + help="contract runs fixture-light checks; full also runs seeded/backfilled data assertions.", + ) args = parser.parse_args() if args.list: @@ -755,8 +827,9 @@ def main() -> int: if service_filter is not None: deployed_services &= service_filter - active_checks = [c for c in CHECKS if c.service is None or c.service in deployed_services] - skipped_checks = [c for c in CHECKS if c.service is not None and c.service not in deployed_services] + mode_checks = [c for c in CHECKS if args.mode == "full" or not c.full_only] + active_checks = [c for c in mode_checks if c.service is None or c.service in deployed_services] + skipped_checks = [c for c in mode_checks if c.service is not None and c.service not in deployed_services] if not active_checks: print("No active staging smoke checks remain after service filtering.", file=sys.stderr) diff --git a/scripts/ci/check_backend_manifest_deployment.py b/scripts/ci/check_backend_manifest_deployment.py index c66fc5e2..75fe3c0d 100755 --- a/scripts/ci/check_backend_manifest_deployment.py +++ b/scripts/ci/check_backend_manifest_deployment.py @@ -20,6 +20,10 @@ DEFAULT_REGION = "us-east-1" DEFAULT_ARTIFACT_BUCKET = "epac-artifacts-227530433709" +OPENAPI_HTTP_METHODS = {"get", "post", "put", "patch", "delete", "head", "options"} +OPENAPI_MANIFEST_TAG_CONTRACTS = { + "bills": "Bills", +} class DeploymentCheckError(Exception): @@ -139,6 +143,60 @@ def load_manifest(path: Path) -> dict[str, Any]: return json.load(handle) +def load_openapi(path: Path) -> dict[str, Any]: + with path.open(encoding="utf-8") as handle: + return json.load(handle) + + +def check_openapi_manifest_consistency( + manifest: dict[str, Any], + openapi: dict[str, Any], + env_name: str, + service_filter: set[str] | None = None, +) -> list[str]: + failures: list[str] = [] + for service_name, openapi_tag in OPENAPI_MANIFEST_TAG_CONTRACTS.items(): + if service_filter is not None and service_name not in service_filter: + continue + service = next((item for item in manifest.get("services", []) if item.get("name") == service_name), None) + if service is None or service.get("deploy", {}).get(env_name) is not True: + continue + + documented_routes = openapi_routes_for_exclusive_tag(openapi, openapi_tag) + manifest_routes = { + (str(route.get("method", "")).upper(), str(route.get("path", ""))) + for route in service.get("http", {}).get("routes", {}).get(env_name, []) + } + + for method, path in sorted(documented_routes - manifest_routes): + failures.append( + f"{service_name}: OpenAPI {method} {path} is missing from {env_name} deployment manifest" + ) + for method, path in sorted(manifest_routes - documented_routes): + failures.append( + f"{service_name}: manifest route {method} {path} is not documented by OpenAPI {openapi_tag} service contract" + ) + return failures + + +def openapi_routes_for_exclusive_tag(openapi: dict[str, Any], tag: str) -> set[tuple[str, str]]: + routes: set[tuple[str, str]] = set() + paths = openapi.get("paths", {}) + if not isinstance(paths, dict): + return routes + for path, operations in paths.items(): + if not isinstance(path, str) or not isinstance(operations, dict): + continue + for method, operation in operations.items(): + method_name = str(method).lower() + if method_name not in OPENAPI_HTTP_METHODS or not isinstance(operation, dict): + continue + tags = operation.get("tags", []) + if isinstance(tags, list) and set(tags) == {tag}: + routes.add((method_name.upper(), path)) + return routes + + def select_s3_http_services( manifest: dict[str, Any], env_name: str, @@ -396,6 +454,7 @@ def main() -> int: parser.add_argument("--api-id", required=True) parser.add_argument("--artifact-bucket", default=os.environ.get("EPAC_ARTIFACT_BUCKET", DEFAULT_ARTIFACT_BUCKET)) parser.add_argument("--manifest", type=Path, default=repo_root() / "backend" / "manifest" / "deployment-services.json") + parser.add_argument("--openapi", type=Path, default=repo_root() / "backend" / "openapi" / "openapi.json") parser.add_argument("--phase", choices=("topology", "ready"), default="ready") parser.add_argument("--scope", choices=("s3-http",), default="s3-http") parser.add_argument("--services", default=os.environ.get("EPAC_BACKEND_MANIFEST_SERVICES", "all")) @@ -403,15 +462,24 @@ def main() -> int: args = parser.parse_args() manifest = load_manifest(args.manifest) + openapi = load_openapi(args.openapi) + service_filter = parse_service_filter(args.services) services = select_s3_http_services( manifest, args.environment, - service_filter=parse_service_filter(args.services), + service_filter=service_filter, ) if not services: print(f"No {args.environment} {args.scope} services selected.") return 0 + failures = check_openapi_manifest_consistency(manifest, openapi, args.environment, service_filter) + if failures: + write_summary(args.environment, args.phase, failures, services) + for failure in failures: + print(f"::error::{failure}", file=sys.stderr) + return 1 + aws = AwsClient(args.region) routes = aws.routes(args.api_id) integrations = aws.integrations(args.api_id) diff --git a/scripts/ci/test_backend_staging_smoke.py b/scripts/ci/test_backend_staging_smoke.py index 47e64170..2303ec76 100644 --- a/scripts/ci/test_backend_staging_smoke.py +++ b/scripts/ci/test_backend_staging_smoke.py @@ -79,3 +79,42 @@ def test_bills_and_members_validators_require_non_empty_lists(): smoke.validate_bills(200, {"bills": []}) with pytest.raises(smoke.SmokeFailure, match="members must not be empty"): smoke.validate_members(200, {"members": []}) + + +def test_bill_diff_route_validator_distinguishes_api_gateway_404(): + smoke = load_smoke_module() + + smoke.validate_bill_diff_route(400, {"error": "missing required query parameters: from, to"}) + + with pytest.raises(smoke.SmokeFailure, match="API Gateway returned Not Found"): + smoke.validate_bill_diff_route(404, {"message": "Not Found"}) + + +def test_bill_diff_full_validator_requires_seeded_payload(): + smoke = load_smoke_module() + + smoke.validate_bill_diff_payload( + 200, + { + "from": {"id": "C-8-v1"}, + "to": {"id": "C-8-v3"}, + "clauses": [{"id": "clause-1"}], + }, + ) + + with pytest.raises(smoke.SmokeFailure, match="clauses must not be empty"): + smoke.validate_bill_diff_payload( + 200, + {"from": {"id": "C-8-v1"}, "to": {"id": "C-8-v3"}, "clauses": []}, + ) + + +def test_full_only_bill_diff_check_is_skipped_in_contract_mode(): + smoke = load_smoke_module() + + contract_checks = [check.name for check in smoke.CHECKS if not check.full_only] + full_checks = [check.name for check in smoke.CHECKS] + + assert "bills:diff-route" in contract_checks + assert "bills:diff-full" not in contract_checks + assert "bills:diff-full" in full_checks diff --git a/scripts/ci/tests/test_backend_manifest_deployment.py b/scripts/ci/tests/test_backend_manifest_deployment.py index 7b81aeac..4b8abf60 100644 --- a/scripts/ci/tests/test_backend_manifest_deployment.py +++ b/scripts/ci/tests/test_backend_manifest_deployment.py @@ -102,6 +102,75 @@ def test_unknown_s3_http_service_requires_an_explicit_contract(): checker.select_s3_http_services(manifest, "staging") +def test_openapi_manifest_consistency_requires_documented_bills_diff_route(): + checker = load_module() + openapi = { + "paths": { + "/api/v1/bills": {"get": {"tags": ["Bills"]}}, + "/api/v1/bills/{id}": {"get": {"tags": ["Bills"]}}, + "/api/v1/bills/{id}/committee-stage": {"get": {"tags": ["Bills"]}}, + "/api/v1/bills/{id}/diff": {"get": {"tags": ["Bills"]}}, + "/api/v1/bills/{legisinfo_id}/lobbying-context": {"get": {"tags": ["Bills", "Lobbying"]}}, + } + } + manifest = { + "services": [ + { + "name": "bills", + "deploy": {"staging": True}, + "http": { + "routes": { + "staging": [ + {"method": "GET", "path": "/api/v1/bills"}, + {"method": "GET", "path": "/api/v1/bills/{id}"}, + {"method": "GET", "path": "/api/v1/bills/{id}/committee-stage"}, + ] + } + }, + } + ] + } + + failures = checker.check_openapi_manifest_consistency(manifest, openapi, "staging") + + assert failures == [ + "bills: OpenAPI GET /api/v1/bills/{id}/diff is missing from staging deployment manifest" + ] + + +def test_openapi_manifest_consistency_reports_method_or_path_drift(): + checker = load_module() + openapi = { + "paths": { + "/api/v1/bills": {"get": {"tags": ["Bills"]}}, + "/api/v1/bills/{id}": {"get": {"tags": ["Bills"]}}, + } + } + manifest = { + "services": [ + { + "name": "bills", + "deploy": {"production": True}, + "http": { + "routes": { + "production": [ + {"method": "GET", "path": "/api/v1/bills"}, + {"method": "POST", "path": "/api/v1/bills/{id}"}, + ] + } + }, + } + ] + } + + failures = checker.check_openapi_manifest_consistency(manifest, openapi, "production") + + assert failures == [ + "bills: OpenAPI GET /api/v1/bills/{id} is missing from production deployment manifest", + "bills: manifest route POST /api/v1/bills/{id} is not documented by OpenAPI Bills service contract", + ] + + def test_policy_allows_api_gateway_when_source_arn_matches_api_id(): checker = load_module() policy = {