Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion .github/workflows/review-deploy.yml
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,8 @@ jobs:
--timeout=120s \
--run-service-account=run-mdn-review-functions@${{ secrets.GCP_PROJECT_NAME }}.iam.gserviceaccount.com \
--set-env-vars="IGNORED_ROUTES=" \
--set-env-vars="WILDCARD_ENABLED=true" \
--set-env-vars="REVIEW_ROUTING=true" \
--set-env-vars="CACHE_DISABLED=true" \
--set-env-vars="SOURCE_CONTENT=https://storage.googleapis.com/${{ vars.GCP_BUCKET_NAME }}/" \
--set-env-vars="SENTRY_DSN=${{ secrets.SENTRY_DSN_CLOUD_FUNCTION }}" \
--set-env-vars="SENTRY_ENVIRONMENT=review" \
Expand Down
9 changes: 6 additions & 3 deletions cloud-function/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,12 @@ The function uses the following environment variables:
client build is served.
- `SOURCE_API` (default: `"https://developer.allizom.org/"`) - The URL at which
the API is served.
- `WILDCARD_ENABLED` (default: `false`) - If enabled, accepts any `Host` header
value, and uses the leftmost subdomain to route into a subdirectory of
`SOURCE_CONTENT`.
- `REVIEW_ROUTING` (default: `false`) - If enabled, accepts any `Host` header
value, uses the leftmost subdomain to route into a subdirectory of
`SOURCE_CONTENT`, and falls back to production for missing assets.
- `CACHE_DISABLED` (default: `false`) - If enabled, disables response caching
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needs updating if the above change is accepted.

(`Cache-Control: no-store` for non-hashed 200 responses, and skips the
in-memory 404 cache).

The placement handler uses the following environment variables:

Expand Down
8 changes: 6 additions & 2 deletions cloud-function/src/env.js
Original file line number Diff line number Diff line change
Expand Up @@ -103,8 +103,12 @@ export const BSA_ZONE_KEYS = Object.fromEntries(
(process.env["BSA_ZONE_KEYS"] ?? "").split(";").map((k) => k.split(":"))
);

export const WILDCARD_ENABLED = Boolean(
JSON.parse(process.env["WILDCARD_ENABLED"] || "false")
export const REVIEW_ROUTING = Boolean(
JSON.parse(process.env["REVIEW_ROUTING"] || "false")
);

export const CACHE_DISABLED = Boolean(
JSON.parse(process.env["CACHE_DISABLED"] || "false")
);

// HTTPS.
Expand Down
8 changes: 4 additions & 4 deletions cloud-function/src/handlers/proxy-content.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import {
} from "http-proxy-middleware";

import { withContentResponseHeaders } from "../headers.js";
import { Source, sourceUri, WILDCARD_ENABLED } from "../env.js";
import { CACHE_DISABLED, REVIEW_ROUTING, Source, sourceUri } from "../env.js";
import { PROXY_TIMEOUT } from "../constants.js";
import { isLiveSampleURL } from "../utils.js";
import { ACTIVE_LOCALES } from "../internal/constants/index.js";
Expand All @@ -22,7 +22,7 @@ const target = sourceUri(Source.content);
const router = (req) => {
let actualTarget = target;

if (WILDCARD_ENABLED) {
if (REVIEW_ROUTING) {
const { host } = req.headers;

if (typeof host === "string") {
Expand Down Expand Up @@ -129,7 +129,7 @@ export const proxyContentAssets = createContentProxyMiddleware(
return Buffer.from(await enUsAsset.arrayBuffer());
}

if (WILDCARD_ENABLED) {
if (REVIEW_ROUTING) {
// Fallback to prod.
const prodUrl = new URL(req.url ?? "", "https://developer.mozilla.org/");
res.statusCode = 303;
Expand All @@ -154,7 +154,7 @@ async function get404ForLocale(locale) {
} else {
const response = await fetch(`${target}${locale}/404/index.html`);
notFoundBuffer = response.arrayBuffer();
if (!WILDCARD_ENABLED) {
if (!CACHE_DISABLED) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This feels more like REVIEW_ROUTING rather than CACHE_DISABLED: we are disabling a cache, but it's not anything to do with the cache-control header.

Suggested change
if (!CACHE_DISABLED) {
if (!REVIEW_ROUTING) {

Presumably we do this to allow some aspect of review routing to work, and don't need to enable it independently?

if (response.ok) {
notFoundBufferCache[locale] = notFoundBuffer;
} else {
Expand Down
4 changes: 2 additions & 2 deletions cloud-function/src/headers.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

import { CSP_VALUE } from "./internal/constants/index.js";
import { isLiveSampleURL } from "./utils.js";
import { WILDCARD_ENABLED } from "./env.js";
import { CACHE_DISABLED } from "./env.js";

const HASHED_MAX_AGE = 60 * 60 * 24 * 365;
const DEFAULT_MAX_AGE = 60 * 60;
Expand Down Expand Up @@ -72,7 +72,7 @@ function getCacheControl(statusCode, url) {
}

if (200 <= statusCode && statusCode < 300) {
if (WILDCARD_ENABLED && !HASHED_REGEX.test(url)) {
if (CACHE_DISABLED && !HASHED_REGEX.test(url)) {
return NO_CACHE_VALUE;
}

Expand Down
6 changes: 3 additions & 3 deletions cloud-function/src/middlewares/require-origin.js
Original file line number Diff line number Diff line change
@@ -1,17 +1,17 @@
/** @import { NextFunction, Request, Response } from "express" */
/** @import { OriginType } from "../env.js" */

import { WILDCARD_ENABLED, getOriginFromRequest } from "../env.js";
import { REVIEW_ROUTING, getOriginFromRequest } from "../env.js";

/**
* Creates a middleware that requires the request to come from one of the expected origins.
* Returns 404 if the origin doesn't match (unless WILDCARD_ENABLED is true).
* Returns 404 if the origin doesn't match (unless REVIEW_ROUTING is true).
* @param {...OriginType} expectedOrigins - The allowed origin types
* @returns {(req: Request, res: Response, next: NextFunction) => void} Express middleware function
*/
export function requireOrigin(...expectedOrigins) {
return async (req, res, next) => {
if (WILDCARD_ENABLED) {
if (REVIEW_ROUTING) {
return next();
}

Expand Down
Loading