Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add frontend nginx container for logging and plausible proxying #3108

Merged
merged 7 commits into from
Oct 10, 2023
Merged
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/ci_cd.yml
Original file line number Diff line number Diff line change
Expand Up @@ -199,10 +199,11 @@ jobs:
- name: Build image `${{ matrix.image }}`
uses: docker/build-push-action@v5
with:
context: ${{ matrix.context || matrix.image }}
context: ${{ matrix.context }}
target: ${{ matrix.target }}
push: false
tags: openverse-${{ matrix.image }}
file: ${{ matrix.file }}
cache-from: type=gha,scope=${{ matrix.image }}
cache-to: type=gha,scope=${{ matrix.image }}
outputs: type=docker,dest=/tmp/${{ matrix.image }}.tar
Expand Down
17 changes: 15 additions & 2 deletions automations/python/workflows/set_matrix_images.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,12 @@ def ser_set(x):
"api": {"image": "api", "target": "api"},
"api_nginx": {"image": "api_nginx", "context": "api", "target": "nginx"},
"frontend": {"image": "frontend", "target": "app", "build-contexts": "repo_root=."},
"frontend_nginx": {
"image": "frontend_nginx",
"context": "frontend",
"file": "frontend/Dockerfile.nginx",
"target": "nginx",
},
}

if "ci_cd" in changes:
Expand All @@ -46,11 +52,18 @@ def ser_set(x):
build_matrix["image"] |= {"upstream_db", "ingestion_server", "api", "api_nginx"}
publish_matrix["image"] |= {"api", "api_nginx"}
if "frontend" in changes:
build_matrix["image"].add("frontend")
publish_matrix["image"].add("frontend")
build_matrix["image"] |= {"frontend", "frontend_nginx"}
publish_matrix["image"] |= {"frontend", "frontend_nginx"}

build_matrix["include"] = [includes[item] for item in build_matrix["image"]]

for item in build_matrix["include"]:
if "context" not in item:
item["context"] = item["image"]

if "file" not in item:
item["file"] = f"{item['context']}/Dockerfile"

do_build = "true" if len(build_matrix["image"]) else "false"
do_publish = "true" if len(publish_matrix["image"]) else "false"
build_matrix = json.dumps(build_matrix, default=ser_set)
Expand Down
17 changes: 17 additions & 0 deletions docker-compose.yml
Original file line number Diff line number Diff line change
Expand Up @@ -318,6 +318,23 @@ services:
- ./docker/nginx/templates:/etc/nginx/templates
- ./docker/nginx/certs:/etc/nginx/certs

frontend_nginx:
profiles:
- frontend
build:
context: ./frontend/
dockerfile: Dockerfile.nginx
target: nginx
args: # Automatically inferred from env vars, unless specified
- SEMANTIC_VERSION=${SEMANTIC_VERSION:-v1.0.0}
- FRONTEND_NODE_VERSION
- FRONTEND_PNPM_VERSION
ports:
- "50290:8080"
environment:
OPENVERSE_NGINX_UPSTREAM_URL: ${HOST_NETWORK_ADDRESS}:8443
OPENVERSE_NGINX_PLAUSIBLE_EVENT_URL: http://plausible:8000/api/event

volumes:
api-postgres:
catalog-postgres:
Expand Down
1 change: 1 addition & 0 deletions documentation/frontend/reference/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,4 +10,5 @@ playwright_tests
storybook_tests
miscellaneous
healthcheck
nginx
```
24 changes: 24 additions & 0 deletions documentation/frontend/reference/nginx.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
# Frontend reverse proxy

The frontend service relies on an Nginx reverse proxy in live environments to
facilitate per-request logging in a format that matches other services we run
(namely, Django). It also acts as a [proxy for Plausible][plausible_proxy],
removing the need for Nuxt to handle these requests, freeing it up to handle
rendering SSR requests.

[plausible_proxy]: https://plausible.io/docs/proxy/guides/nginx

Additionally, the reverse proxy may be used in the future for the following:

- Serving static Nuxt content, freeing Nuxt from serving these requests. This is
considered relatively low value due to Cloudflare caching already handling 99%
of the benefit this would bring for the vast majority of cases.

## Testing

To test the frontend reverse proxy locally, run `just frontend/up`. To test the
integration with your local Plausible container, follow the existing
instructions in [the frontend analytics documentation][analytics_docs].
Everything should "just work".

[analytics_docs]: /frontend/guides/analytics.md#plausible-set-up-and-first-run
3 changes: 0 additions & 3 deletions frontend/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,6 @@ ENV NODE_ENV=production
ENV SENTRY_DSN=https://b6466b74788a4a2f8a7912eea912beb7@o787041.ingest.sentry.io/5799642

ARG API_URL
ARG SEMANTIC_VERSION

RUN echo "{\"release\":\"${SEMANTIC_VERSION}\"}" > /home/node/frontend/src/static/version.json

RUN pnpm build:only

Expand Down
29 changes: 29 additions & 0 deletions frontend/Dockerfile.nginx
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
# Keeping this as a separate Dockerfile prevents the need to rely on additional
# contexts required for the frontend build (namely, `repo_root`). These are
# possible with compose as of https://github.com/docker/compose/pull/10369
# and do appear to work, but it increases the complexity and these features
# are not yet all available in every containerd implementation or distribution
# (Debian, for example, still distributes an older version of compose without
# this new feature). For the sake of keeping the local setup simpler and not
# requiring contributors to have bleeding-edge Docker and compose plugin versions,
# we can simply use a separate file for the time being. Once/if we start to use
# nginx to serve static files for the frontend, we will need to integrate this
# target into the shared Dockerfile.
Comment on lines +1 to +11
Copy link
Member

Choose a reason for hiding this comment

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

This is helpful documentation. Should we have it as an issue (potentially blocked) to ensure this is completed at some point?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes 👍 Done here #3180

#########
# NGINX #
#########

FROM docker.io/nginx:1.25.2-alpine as nginx

LABEL org.opencontainers.image.source = "https://github.com/WordPress/openverse"

WORKDIR /app

COPY nginx.conf.template /etc/nginx/templates/openverse-frontend.conf.template

# Only environment variables with this prefix will be available in the template
ENV NGINX_ENVSUBST_FILTER="OPENVERSE_NGINX_"
ENV OPENVERSE_NGINX_ENVIRONMENT="local"
# Add the release version to the docker container
ARG SEMANTIC_VERSION
ENV OPENVERSE_NGINX_GIT_REVISION=$SEMANTIC_VERSION
84 changes: 84 additions & 0 deletions frontend/nginx.conf.template
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
# The following variables are substituted into the environment using envsubst.
# This occurs automatically as part of the nginx image startup.
# See: https://github.com/nginxinc/docker-nginx/blob/456bf337ceb922a207651aa7c6077a316c3e368c/entrypoint/20-envsubst-on-templates.sh#L17
# - OPENVERSE_NGINX_UPSTREAM_URL
# - OPENVERSE_NGINX_GIT_REVISION
# - OPENVERSE_NGINX_ENVIRONMENT
# - OPENVERSE_NGINX_PLAUSIBLE_EVENT_URL

error_log /var/log/nginx/error.log;

log_format json_combined escape=json
'{'
'"time_local":"$time_local",'
'"remote_addr":"$remote_addr",'
'"remote_user":"$remote_user",'
'"request":"$request",'
'"status": "$status",'
'"host_header": "$host",'
'"body_bytes_sent":$body_bytes_sent,'
'"request_time":"$request_time",'
'"http_referrer":"$http_referer",'
'"http_user_agent":"$http_user_agent",'
'"upstream_response_time":$upstream_response_time,'
'"http_x_forwarded_for":"$http_x_forwarded_for"'
'}';

access_log /var/log/nginx/access.log json_combined;

tcp_nopush on;
tcp_nodelay on;
types_hash_max_size 2048;

# Compress large responses to save bandwidth and improve latency
gzip on;
gzip_min_length 860;
gzip_vary on;
gzip_proxied expired private auth;
gzip_types application/json text/plain application/javascript;
gzip_disable "MSIE [1-6]\.";

upstream ov_service {
server $OPENVERSE_NGINX_UPSTREAM_URL;
}

server {
sarayourfriend marked this conversation as resolved.
Show resolved Hide resolved
access_log /var/log/nginx/access.log json_combined;

listen 8080;
server_name _;
charset utf-8;
client_max_body_size 75M;
error_page 500 /500.json;
absolute_redirect off;

location / {
proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for;
proxy_set_header X-Forwarded-Proto $scheme;
proxy_set_header Host $http_host;
proxy_redirect off;
proxy_pass http://ov_service;
error_page 500 /500.json;
}

location /version {
default_type "application/json";
return 200 '{"release": "$OPENVERSE_NGINX_GIT_REVISION", "environment": "$OPENVERSE_NGINX_ENVIRONMENT"}';
}

# This is Docker's internal DNS resolver
# In localhost it will resolve the local Plausible and forward anything else to the host
# In production, AWS replaces
resolver 127.0.0.11;
set $plausible_event_url $OPENVERSE_NGINX_PLAUSIBLE_EVENT_URL;
location = /api/event {
proxy_pass $plausible_event_url;
proxy_set_header Host plausible.io;
proxy_buffering on;
proxy_http_version 1.1;

proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for;
proxy_set_header X-Forwarded-Proto $scheme;
proxy_set_header X-Forwarded-Host $host;
}
}
53 changes: 12 additions & 41 deletions frontend/nuxt.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,8 @@ import type http from "http"

import type { NuxtConfig } from "@nuxt/types"
import type { LocaleObject } from "@nuxtjs/i18n"
import type { Options as ProxyOptions } from "http-proxy-middleware"
import type { IncomingMessage, NextFunction } from "connect"

let plausibleLogged = false

if (process.env.NODE_ENV === "production") {
meta.push({
// @ts-expect-error: 'http-equiv' isn't allowed here by Nuxt
Expand Down Expand Up @@ -306,43 +303,6 @@ const config: NuxtConfig = {
},
},
},
proxy: {
// The key is appended to the address in the value.
"/api/event": {
target:
process.env.PLAUSIBLE_ORIGIN ??
(isProdNotPlaywright
? "https://plausible.io"
: "http://localhost:50288"),
// Prevent ECONNREFUSED errors in the server console.
logProvider: () => {
return {
...console,
error: isProdNotPlaywright
? console.error
: (...data: unknown[]) => {
if (
!plausibleLogged &&
data.some(
(item) =>
typeof item === "string" && item.includes("ECONNREFUSED")
)
) {
console.warn("Plausible is not running.")
plausibleLogged = true
}
},
}
},
// Prevent 504 errors in the browser console.
onError: (err, _req, res) => {
if (!isProdNotPlaywright && err.message.includes("ECONNREFUSED")) {
res.writeHead(200, { "Content-Type": "text/plain" })
res.end("plausible not running")
}
},
} satisfies ProxyOptions,
},
plausible: {
trackLocalhost: !isProdNotPlaywright,
},
Expand All @@ -356,7 +316,18 @@ const config: NuxtConfig = {
process.env.SITE_DOMAIN ??
(isProdNotPlaywright
? "https://openverse.org"
: `http://localhost:${port}`),
: /**
* We rely on the Nginx container running as `frontend_nginx`
* in the local compose stack to proxy requests. Therefore, the
* URL here is not for the Plausible container in the local stack,
* but the Nginx service, which then itself forwards the requests
* to the local Plausible instance.
*
* In production, the Nginx container is handling all requests
* made to the root URL (openverse.org), and is configured to
* forward Plausible requests to upstream Plausible.
*/
"http://localhost:50290"),
},
sentry: {
config: {
Expand Down
2 changes: 1 addition & 1 deletion frontend/test/playwright/utils/analytics.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ export function expectEventPayloadToMatch<T extends EventName>(

export const collectAnalyticsEvents = (context: BrowserContext) => {
const sentAnalyticsEvents: AnalyticEventResponses = []
context.route("/api/event", (route, request) => {
context.route(/\/api\/event$/, (route, request) => {
const postData = request.postData()
if (postData) {
const parsedData = JSON.parse(postData)
Expand Down
4 changes: 4 additions & 0 deletions justfile
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ PROD_ENV := env_var_or_default("PROD_ENV", "")
IS_CI := env_var_or_default("CI", "")
DC_USER := env_var_or_default("DC_USER", "opener")

export HOST_NETWORK_ADDRESS := if os() == "macos" { "host.docker.internal" } else { "172.17.0.1" }

# Show all available recipes, also recurses inside nested justfiles
@_default:
just --list --unsorted
Expand Down Expand Up @@ -123,6 +125,8 @@ export CATALOG_PY_VERSION := `just catalog/py-version`
export CATALOG_AIRFLOW_VERSION := `just catalog/airflow-version`
export API_PY_VERSION := `just api/py-version`
export INGESTION_PY_VERSION := `just ingestion_server/py-version`
export FRONTEND_NODE_VERSION := `just frontend/node-version`
export FRONTEND_PNPM_VERSION := `just frontend/pnpm-version`

versions:
#!/usr/bin/env bash
Expand Down
Loading