From 94836c2ecb27f5e7f0bcb1dc97a32938cc872288 Mon Sep 17 00:00:00 2001 From: David Stone Date: Sun, 28 Jun 2026 15:34:06 -0700 Subject: [PATCH] feat: select output format by URL suffix and move to CloudFront free plan Format (JSON vs iCal) is now chosen solely by the URL suffix (`.json` vs the default) instead of the Accept header. Because the request path is always part of the CloudFront cache key, the two representations can no longer collide behind a single URL. That lets us drop the custom Accept-keyed cache policy and the access-logs bucket and switch to a managed policy, keeping the distribution on CloudFront's $0 plan. CACHING_OPTIMIZED is the managed policy used here because it keeps the viewer Host out of the cache key. That is required: the API Gateway origin routes by Host and has no custom domain, so the viewer Host must be stripped (hence ALL_VIEWER_EXCEPT_HOST_HEADER). The UseOriginCacheControlHeaders and Amplify policies all force Host into the cache key, which makes CloudFront forward the viewer Host and API Gateway answer 403 Forbidden. - lambda: pick format from the `.json` suffix only; strip `.json` like `.ics`; remove the now-dead Accept-header handling (get_mime_type + its tests) - cdk: delete the custom cache policy and access-logs bucket; use the managed CACHING_OPTIMIZED policy; drop CloudFront access logging - tests: json integration fixture now uses a `.json` path with a non-JSON Accept (proving the suffix drives the format); smoke test hits /{id}.json Co-Authored-By: Claude Opus 4.8 (1M context) --- cdk/lib/trashcal-cdk-stack.ts | 51 +++---------- cdk/smoke.test.ts | 6 +- lambda/src/lib.rs | 81 ++------------------- lambda/src/trashcal.rs | 5 +- lambda/tests/data/path_based_with_json.json | 6 +- 5 files changed, 26 insertions(+), 123 deletions(-) diff --git a/cdk/lib/trashcal-cdk-stack.ts b/cdk/lib/trashcal-cdk-stack.ts index b021db65..99e1fb84 100644 --- a/cdk/lib/trashcal-cdk-stack.ts +++ b/cdk/lib/trashcal-cdk-stack.ts @@ -19,7 +19,6 @@ import { } from "aws-cdk-github-oidc"; import * as cloudfront from "aws-cdk-lib/aws-cloudfront"; import * as cloudfrontOrigins from "aws-cdk-lib/aws-cloudfront-origins"; -import * as s3 from "aws-cdk-lib/aws-s3"; export interface TrashcalCdkStackProps extends cdk.StackProps { domainName: string; @@ -85,46 +84,8 @@ export class TrashcalCdkStack extends cdk.Stack { integration: trashcalIntegration, }); - let cachePolicy = new cloudfront.CachePolicy( - this, - "trashcal-cache-policy", - { - minTtl: cdk.Duration.hours(1), - maxTtl: cdk.Duration.days(2), - defaultTtl: cdk.Duration.days(1), - enableAcceptEncodingBrotli: true, - enableAcceptEncodingGzip: true, - headerBehavior: cloudfront.CacheHeaderBehavior.allowList( - // we need to add the Accept header to the cache key because otherwise json and ical collide - "Accept" - ), - } - ); - - // CloudFront access logs bucket. Standard (legacy) CloudFront logging - // delivers log files using bucket ACLs, so ACLs must stay enabled - // (BUCKET_OWNER_PREFERRED) and the bucket can't use SSE-KMS — hence - // SSE-S3. The 90-day lifecycle rule caps growth. This replaces the old - // out-of-band `trashcal-access-logs` bucket, which can be decommissioned - // once this distribution is delivering logs here. - const accessLogsBucket = new s3.Bucket(this, "trashcal-access-logs", { - blockPublicAccess: s3.BlockPublicAccess.BLOCK_ALL, - objectOwnership: s3.ObjectOwnership.BUCKET_OWNER_PREFERRED, - encryption: s3.BucketEncryption.S3_MANAGED, - enforceSSL: true, - lifecycleRules: [ - { - id: "expire-access-logs", - expiration: cdk.Duration.days(90), - }, - ], - }); - new cloudfront.Distribution(this, "cloudfront-api", { domainNames: [props.domainName], - enableLogging: true, - logBucket: accessLogsBucket, - logFilePrefix: "cloudfront/", defaultBehavior: { origin: new cloudfrontOrigins.HttpOrigin( `${api.apiId}.execute-api.${cdk.Stack.of(this).region}.amazonaws.com` @@ -134,7 +95,17 @@ export class TrashcalCdkStack extends cdk.Stack { originRequestPolicy: cloudfront.OriginRequestPolicy.ALL_VIEWER_EXCEPT_HOST_HEADER, - cachePolicy: cachePolicy, + // Managed policy (no custom cache policy / no access logging) keeps us on + // CloudFront's free plan. CACHING_OPTIMIZED is the one that works here: it + // keeps NO request headers in the cache key. That matters because the API + // Gateway origin routes by Host and has no custom domain, so the viewer Host + // must not be forwarded (hence ALL_VIEWER_EXCEPT_HOST_HEADER above). The + // UseOriginCacheControlHeaders / Amplify policies all force Host into the + // cache key, which makes CloudFront forward the viewer Host and API Gateway + // answer 403 Forbidden. Format is selected by URL suffix, so the path — + // always in the cache key — keeps JSON and iCal separate. TTL follows the + // Expires header the Lambda emits. + cachePolicy: cloudfront.CachePolicy.CACHING_OPTIMIZED, }, certificate: props.cert, }); diff --git a/cdk/smoke.test.ts b/cdk/smoke.test.ts index dfdcb980..07a61be5 100644 --- a/cdk/smoke.test.ts +++ b/cdk/smoke.test.ts @@ -20,11 +20,7 @@ describe("post deploy", () => { }); it("works with a json calendar", async () => { - let response = await fetch(`${baseUrl}/${id}`, { - headers: { - accept: "application/json", - }, - }); + let response = await fetch(`${baseUrl}/${id}.json`); let body = await response.json(); console.log(body); diff --git a/lambda/src/lib.rs b/lambda/src/lib.rs index f8257a3d..c23e8034 100644 --- a/lambda/src/lib.rs +++ b/lambda/src/lib.rs @@ -1,5 +1,4 @@ use anyhow::Result; -use http::{HeaderMap, HeaderValue}; use crate::trashcal::trashcal; use http::header::{CONTENT_DISPOSITION, CONTENT_TYPE, EXPIRES}; @@ -15,14 +14,17 @@ pub mod pickup_calendar; pub mod trashcal; #[instrument] -pub async fn get_trashcal(id: &str, accept: &str, whimsy: bool) -> Result> { - let is_ics_request = id.contains(".ics"); +pub async fn get_trashcal(id: &str, whimsy: bool) -> Result> { + // Format is chosen by the URL suffix alone, never the Accept header: CloudFront's + // cache key always includes the path, so `.json` and the default (iCal) land in + // separate cache entries instead of colliding behind one URL. + let is_json_request = id.contains(".json"); let calendar = trashcal(id).await?; // build the response as either json or calendar let resp = Response::builder().status(StatusCode::OK); - let resp = if accept.starts_with("application/json") && !is_ics_request { + let resp = if is_json_request { info!( message = "Returning calendar as JSON", address = %calendar.address, @@ -58,79 +60,12 @@ pub async fn trashcal_handler(event: Request) -> Result> { .first("id") .or_else(|| query.first("id")) .unwrap_or("null"); - let accept = get_mime_type(event.headers()); - + // get the whimsy parameter, defaults to true let whimsy = query .first("whimsy") .map(|v| v != "false") .unwrap_or(true); - get_trashcal(id, accept, whimsy).await -} - -/// Safely get the accept header. Fastmail apparently doesn't send an accept header at all (!) -pub fn get_mime_type(headers: &HeaderMap) -> &str { - headers - .get(http::header::ACCEPT) - .and_then(|x| x.to_str().ok()) - .unwrap_or("text/calendar") -} - -#[cfg(test)] -mod test { - use http::HeaderMap; - use std::sync::Once; - use tracing_subscriber::{layer::SubscriberExt, util::SubscriberInitExt}; - - use crate::get_mime_type; - - static INIT: Once = Once::new(); - - fn init_tracing() { - INIT.call_once(|| { - tracing_subscriber::registry() - .with( - tracing_subscriber::EnvFilter::try_from_default_env() - .unwrap_or_else(|_| "trashcal=debug,tower_http=debug".into()), - ) - .with(tracing_subscriber::fmt::layer()) - .init(); - }); - } - - #[test] - fn test_missing_accept_header() { - init_tracing(); - let headers = HeaderMap::new(); - let result = get_mime_type(&headers); - assert_eq!(result, "text/calendar"); - } - - #[test] - fn test_json() { - init_tracing(); - let mut headers = HeaderMap::new(); - headers.insert(http::header::ACCEPT, "application/json".parse().unwrap()); - let result = get_mime_type(&headers); - assert_eq!(result, "application/json"); - } - - #[test] - fn test_calendar() { - init_tracing(); - let mut headers = HeaderMap::new(); - headers.insert(http::header::ACCEPT, "text/calendar".parse().unwrap()); - let result = get_mime_type(&headers); - assert_eq!(result, "text/calendar"); - } - - #[test] - fn test_weird_header_value() { - init_tracing(); - let mut headers = HeaderMap::new(); - headers.insert(http::header::ACCEPT, "💩".parse().unwrap()); - let result = get_mime_type(&headers); - assert_eq!(result, "text/calendar"); - } + get_trashcal(id, whimsy).await } diff --git a/lambda/src/trashcal.rs b/lambda/src/trashcal.rs index ba7a7977..e164f653 100644 --- a/lambda/src/trashcal.rs +++ b/lambda/src/trashcal.rs @@ -9,8 +9,9 @@ pub async fn trashcal(id: &str) -> Result { // as far as I can tell, all IDs start with a4O ensure!(id.starts_with("a4O"), Error::IdError(id.to_string())); - // rip out .ics for Paul - let id = id.replace(".ics", ""); + // rip out the format suffix (.ics for Paul, .json for the website) before + // building the upstream URL + let id = id.replace(".ics", "").replace(".json", ""); info!("Getting trashcal"); let url = format!("https://getitdone.sandiego.gov/CollectionDetail?id={id}"); diff --git a/lambda/tests/data/path_based_with_json.json b/lambda/tests/data/path_based_with_json.json index ece35ab0..ebfe2736 100644 --- a/lambda/tests/data/path_based_with_json.json +++ b/lambda/tests/data/path_based_with_json.json @@ -1,11 +1,11 @@ { "version": "2.0", "routeKey": "$default", - "rawPath": "/a4Ot0000001E8i4EAC", + "rawPath": "/a4Ot0000001E8i4EAC.json", "rawQueryString": "", "cookies": [], "headers": { - "accept": "application/json" + "accept": "text/calendar" }, "queryStringParameters": {}, "requestContext": { @@ -28,7 +28,7 @@ "timeEpoch": 1583348638390 }, "body": "", - "pathParameters": { "id": "a4Ot0000001E8i4EAC" }, + "pathParameters": { "id": "a4Ot0000001E8i4EAC.json" }, "isBase64Encoded": false, "stageVariables": {} }