From 07ba0365fc1ae30d4f65969a4473d5cfeca98e8a Mon Sep 17 00:00:00 2001 From: David Karlsson <35727626+dvdksn@users.noreply.github.com> Date: Thu, 18 Jun 2026 15:50:42 +0200 Subject: [PATCH] hack/releaser: preserve query string on CloudFront redirects The CloudFront redirect Lambda dropped the request query string when issuing 301 responses, so UTM tags and other params were lost on redirected URLs. Append the query string to both the exact and prefix redirect Location headers, and add unit tests wired into the releaser CI job. Co-Authored-By: Claude Opus 4.8 (1M context) --- .github/workflows/build.yml | 4 +- docker-bake.hcl | 7 ++ hack/releaser/Dockerfile | 6 + hack/releaser/cloudfront-lambda-redirects.js | 13 +- .../cloudfront-lambda-redirects.test.js | 116 ++++++++++++++++++ 5 files changed, 143 insertions(+), 3 deletions(-) create mode 100644 hack/releaser/cloudfront-lambda-redirects.test.js diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 161fd2bdfffc..f92187e766b8 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -35,7 +35,9 @@ jobs: with: files: | docker-bake.hcl - targets: releaser-build + targets: | + releaser-build + releaser-test build: runs-on: ubuntu-24.04 diff --git a/docker-bake.hcl b/docker-bake.hcl index cd32840eb6c8..ce783485b7ed 100644 --- a/docker-bake.hcl +++ b/docker-bake.hcl @@ -76,6 +76,13 @@ target "test-go-redirects" { provenance = false } +target "releaser-test" { + context = "hack/releaser" + target = "releaser-test" + output = ["type=cacheonly"] + provenance = false +} + target "dockerfile-lint" { call = "check" } diff --git a/hack/releaser/Dockerfile b/hack/releaser/Dockerfile index c2983b304c56..fa4c6f6dc46e 100644 --- a/hack/releaser/Dockerfile +++ b/hack/releaser/Dockerfile @@ -15,6 +15,12 @@ RUN --mount=type=bind,target=. \ --mount=type=cache,target=/root/.cache/go-build \ go build -o /out/releaser . +# releaser-test runs the unit tests for the CloudFront redirect Lambda function. +FROM base AS releaser-test +RUN apk add --no-cache nodejs +RUN --mount=type=bind,target=. \ + node --test cloudfront-lambda-redirects.test.js + FROM base AS aws-lambda-invoke ARG DRY_RUN=false ARG AWS_REGION diff --git a/hack/releaser/cloudfront-lambda-redirects.js b/hack/releaser/cloudfront-lambda-redirects.js index 6fbc40e5b182..2ed58829f3ba 100644 --- a/hack/releaser/cloudfront-lambda-redirects.js +++ b/hack/releaser/cloudfront-lambda-redirects.js @@ -5,6 +5,15 @@ exports.handler = (event, context, callback) => { const request = event.Records[0].cf.request; const requestUrl = request.uri.replace(/\/$/, "") + // Preserve the query string (e.g. UTM tags) when issuing a redirect. + const withQuery = (location) => { + if (!request.querystring) { + return location; + } + const separator = location.includes('?') ? '&' : '?'; + return location + separator + request.querystring; + }; + const redirects = JSON.parse(`{{.RedirectsJSON}}`); for (let key in redirects) { const redirectTarget = key.replace(/\/$/, "") @@ -18,7 +27,7 @@ exports.handler = (event, context, callback) => { headers: { location: [{ key: 'Location', - value: redirects[key], + value: withQuery(redirects[key]), }], }, } @@ -44,7 +53,7 @@ exports.handler = (event, context, callback) => { headers: { location: [{ key: 'Location', - value: newlocation, + value: withQuery(newlocation), }], }, } diff --git a/hack/releaser/cloudfront-lambda-redirects.test.js b/hack/releaser/cloudfront-lambda-redirects.test.js new file mode 100644 index 000000000000..a9a3d4442353 --- /dev/null +++ b/hack/releaser/cloudfront-lambda-redirects.test.js @@ -0,0 +1,116 @@ +'use strict'; + +// Tests for cloudfront-lambda-redirects.js. +// +// The function source is a Go text/template (see aws.go) with two +// placeholders that are filled with JSON at build time. These tests render +// the template the same way aws.go does, then load and exercise the handler. +// +// Run with: node --test hack/releaser/cloudfront-lambda-redirects.test.js + +const { test } = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const path = require('node:path'); +const Module = require('node:module'); + +const SRC = path.join(__dirname, 'cloudfront-lambda-redirects.js'); + +const REDIRECTS = { + '/old/page/': '/new/page/', + '/target-with-query/': '/dest/?ref=docs', +}; + +const REDIRECTS_PREFIXES = [ + { prefix: 'keep/', strip: false }, + { prefix: 'strip/', strip: true }, +]; + +// Render the template the same way getLambdaFunctionZip in aws.go does, then +// evaluate it as a CommonJS module so we can call handler() directly. +function loadHandler() { + const rendered = fs + .readFileSync(SRC, 'utf8') + // Function replacers avoid String.replace's special handling of `$` + // sequences in the replacement, in case a redirect target contains one. + .replace('{{.RedirectsJSON}}', () => JSON.stringify(REDIRECTS)) + .replace('{{.RedirectsPrefixesJSON}}', () => JSON.stringify(REDIRECTS_PREFIXES)); + + const m = new Module(SRC); + m._compile(rendered, SRC); + return m.exports.handler; +} + +const handler = loadHandler(); + +// invoke wraps the callback-style handler in a promise. +function invoke({ uri, querystring = '', accept = 'text/html' }) { + const request = { + uri, + querystring, + headers: { accept: [{ key: 'Accept', value: accept }] }, + }; + const event = { Records: [{ cf: { request } }] }; + return new Promise((resolve, reject) => { + handler(event, {}, (err, result) => { + if (err) return reject(err); + resolve({ result, request }); + }); + }); +} + +function locationOf(result) { + return result.headers.location[0].value; +} + +test('exact redirect preserves the query string', async () => { + const { result } = await invoke({ + uri: '/old/page/', + querystring: 'utm_source=newsletter&utm_campaign=launch', + }); + assert.equal(result.status, '301'); + assert.equal(locationOf(result), '/new/page/?utm_source=newsletter&utm_campaign=launch'); +}); + +test('exact redirect without a query string is unchanged', async () => { + const { result } = await invoke({ uri: '/old/page/' }); + assert.equal(result.status, '301'); + assert.equal(locationOf(result), '/new/page/'); +}); + +test('exact redirect appends with & when target already has a query string', async () => { + const { result } = await invoke({ + uri: '/target-with-query/', + querystring: 'utm_medium=email', + }); + assert.equal(locationOf(result), '/dest/?ref=docs&utm_medium=email'); +}); + +test('prefix redirect (strip) preserves the query string', async () => { + const { result } = await invoke({ + uri: '/strip/some/path', + querystring: 'utm_source=x', + }); + assert.equal(result.status, '301'); + assert.equal(locationOf(result), '/some/path?utm_source=x'); +}); + +test('prefix redirect (no strip) preserves the query string', async () => { + const { result } = await invoke({ + uri: '/keep/anything', + querystring: 'utm_source=x', + }); + assert.equal(result.status, '301'); + assert.equal(locationOf(result), '/?utm_source=x'); +}); + +test('directory rewrite passes the request through with query string intact', async () => { + const { result, request } = await invoke({ + uri: '/some/page', + querystring: 'utm_source=x', + }); + // No redirect response: the handler forwards the (mutated) request. + assert.equal(result, request); + assert.equal(request.uri, '/some/page/index.html'); + assert.equal(request.querystring, 'utm_source=x'); +});