feat: 🎸 support OPTIONS requests (CORS pre-flight requests) (#538)

* feat: 🎸 support OPTIONS requests (CORS pre-flight requests)

Fixes #532

* feat: 🎸 upgrade docker images

* fix: 🐛 don't set the Access-Control-Allow-Origin in nginx

The services are responsible to set it if required (note that when
credentials are passed in the request, the Access-Control-Allow-Origin
header must not return *, but the origin)

* fix: 🐛 remove obsolete configuration

nginx no longer caches the proxied responses

chart/docker-images.yaml

@@ -1,7 +1,7 @@
 {
   "dockerImage": {
-    "admin": "707930574880.dkr.ecr.us-east-1.amazonaws.com/hub-datasets-server-admin:sha-ff8e803",
-    "api": "707930574880.dkr.ecr.us-east-1.amazonaws.com/hub-datasets-server-api:sha-ff8e803",
+    "admin": "707930574880.dkr.ecr.us-east-1.amazonaws.com/hub-datasets-server-admin:sha-120ddb9",
+    "api": "707930574880.dkr.ecr.us-east-1.amazonaws.com/hub-datasets-server-api:sha-120ddb9",
     "reverseProxy": "docker.io/nginx:1.20",
     "worker": {
       "splits": "707930574880.dkr.ecr.us-east-1.amazonaws.com/hub-datasets-server-worker:sha-ff8e803",

chart/nginx-templates/default.conf.template

@@ -8,8 +8,6 @@ server {
   listen [::]:${PORT};
   server_name ${HOST};
 
-  add_header 'Access-Control-Allow-Origin' '*' always;
-
   location /openapi.json {
     alias /static-files/openapi.json;
   }

chart/templates/_helpers.tpl

@@ -111,15 +111,6 @@ The numba-cache/ subpath in the NFS
 {{- printf "%s/%s/%s/" .Chart.Name .Release.Name "cache-numba-2" }}
 {{- end }}
 
-{{/*
-The nginx-cache/ subpath in the NFS
-- in a subdirectory named as the chart (datasets-server/), and below it,
-- in a subdirectory named as the Release, so that Releases will not share the same dir
-*/}}
-{{- define "cache.nginx.subpath" -}}
-{{- printf "%s/%s/%s/" .Chart.Name .Release.Name "cache-nginx-2" }}
-{{- end }}
-
 {{/*
 The URL to access the mongodb instance created if mongodb.enable is true
 It's named using the Release name

chart/templates/reverse-proxy/_container.tpl

@@ -5,14 +5,6 @@
   env:
   - name: ASSETS_DIRECTORY
     value: {{ .Values.reverseProxy.assetsDirectory | quote }}
-  - name: CACHE_DIRECTORY
-    value: {{ .Values.reverseProxy.cacheDirectory | quote }}
-  - name: CACHE_INACTIVE
-    value: {{ .Values.reverseProxy.cacheInactive | quote }}
-  - name: CACHE_MAX_SIZE
-    value: {{ .Values.reverseProxy.cacheMaxSize | quote }}
-  - name: CACHE_ZONE_SIZE
-    value: {{ .Values.reverseProxy.cacheZoneSize | quote }}
   - name: HOST
     value: {{ .Values.reverseProxy.host | quote }}
   - name: PORT
@@ -39,11 +31,6 @@
     name: nfs
     subPath: "{{ include "assets.subpath" . }}"
     readOnly: true
-  - mountPath: {{ .Values.reverseProxy.cacheDirectory | quote }}
-    mountPropagation: None
-    name: nfs
-    subPath: "{{ include "cache.nginx.subpath" . }}"
-    readOnly: false
   readinessProbe:
     tcpSocket:
       port: {{ .Values.reverseProxy.readinessPort }}

chart/values.yaml

@@ -63,12 +63,7 @@ reverseProxy:
 
   # Directory of assets (audio files and images that will be served for the web)
   assetsDirectory: "/assets"
-  # Directory of the nginx cache
-  cacheDirectory: "/nginx-cache"
   readinessPort: 80
-  cacheInactive: 24h
-  cacheMaxSize: 1g
-  cacheZoneSize: 50m
   host: localhost
   nginxTemplateFile: "nginx-templates/default.conf.template"
   openapiFile: "static-files/openapi.json"

services/admin/src/admin/app.py

@@ -4,6 +4,7 @@
 from libutils.logger import init_logger
 from starlette.applications import Starlette
 from starlette.middleware import Middleware
+from starlette.middleware.cors import CORSMiddleware
 from starlette.middleware.gzip import GZipMiddleware
 from starlette.routing import Route
 from starlette_prometheus import PrometheusMiddleware
@@ -32,7 +33,13 @@ def create_app() -> Starlette:
     connect_to_queue(database=MONGO_QUEUE_DATABASE, host=MONGO_URL)
     prometheus = Prometheus()
 
-    middleware = [Middleware(GZipMiddleware), Middleware(PrometheusMiddleware, filter_unhandled_paths=True)]
+    middleware = [
+        Middleware(
+            CORSMiddleware, allow_origins=["*"], allow_methods=["*"], allow_headers=["*"], allow_credentials=True
+        ),
+        Middleware(GZipMiddleware),
+        Middleware(PrometheusMiddleware, filter_unhandled_paths=True),
+    ]
     routes = [
         Route("/healthcheck", endpoint=healthcheck_endpoint),
         Route("/metrics", endpoint=prometheus.endpoint),

services/admin/tests/test_app.py

@@ -30,6 +30,34 @@ def clean_mongo_databases() -> None:
     clean_queue_database()
 
 
+def test_cors(client: TestClient) -> None:
+    origin = "http://localhost:3000"
+    method = "GET"
+    header = "X-Requested-With"
+    response = client.options(
+        "/pending-jobs",
+        headers={
+            "Origin": origin,
+            "Access-Control-Request-Method": method,
+            "Access-Control-Request-Headers": header,
+        },
+    )
+    assert response.status_code == 200
+    assert (
+        origin in [o.strip() for o in response.headers["Access-Control-Allow-Origin"].split(",")]
+        or response.headers["Access-Control-Allow-Origin"] == "*"
+    )
+    assert (
+        header in [o.strip() for o in response.headers["Access-Control-Allow-Headers"].split(",")]
+        or response.headers["Access-Control-Expose-Headers"] == "*"
+    )
+    assert (
+        method in [o.strip() for o in response.headers["Access-Control-Allow-Methods"].split(",")]
+        or response.headers["Access-Control-Expose-Headers"] == "*"
+    )
+    assert response.headers["Access-Control-Allow-Credentials"] == "true"
+
+
 def test_get_healthcheck(client: TestClient) -> None:
     response = client.get("/healthcheck")
     assert response.status_code == 200

services/api/src/api/app.py

@@ -7,6 +7,7 @@
 from libutils.logger import init_logger
 from starlette.applications import Starlette
 from starlette.middleware import Middleware
+from starlette.middleware.cors import CORSMiddleware
 from starlette.middleware.gzip import GZipMiddleware
 from starlette.routing import BaseRoute, Mount, Route
 from starlette.staticfiles import StaticFiles
@@ -41,7 +42,13 @@ def create_app() -> Starlette:
     show_assets_dir(ASSETS_DIRECTORY)
     prometheus = Prometheus()
 
-    middleware = [Middleware(GZipMiddleware), Middleware(PrometheusMiddleware, filter_unhandled_paths=True)]
+    middleware = [
+        Middleware(
+            CORSMiddleware, allow_origins=["*"], allow_methods=["*"], allow_headers=["*"], allow_credentials=True
+        ),
+        Middleware(GZipMiddleware),
+        Middleware(PrometheusMiddleware, filter_unhandled_paths=True),
+    ]
     documented: List[BaseRoute] = [
         Route("/healthcheck", endpoint=healthcheck_endpoint),
         Route("/valid", endpoint=valid_datasets_endpoint),

services/api/tests/test_app.py

@@ -52,6 +52,34 @@ def clean_mongo_databases() -> None:
     clean_queue_database()
 
 
+def test_cors(client: TestClient) -> None:
+    origin = "http://localhost:3000"
+    method = "GET"
+    header = "X-Requested-With"
+    response = client.options(
+        "/splits?dataset=dataset1",
+        headers={
+            "Origin": origin,
+            "Access-Control-Request-Method": method,
+            "Access-Control-Request-Headers": header,
+        },
+    )
+    assert response.status_code == 200
+    assert (
+        origin in [o.strip() for o in response.headers["Access-Control-Allow-Origin"].split(",")]
+        or response.headers["Access-Control-Allow-Origin"] == "*"
+    )
+    assert (
+        header in [o.strip() for o in response.headers["Access-Control-Allow-Headers"].split(",")]
+        or response.headers["Access-Control-Expose-Headers"] == "*"
+    )
+    assert (
+        method in [o.strip() for o in response.headers["Access-Control-Allow-Methods"].split(",")]
+        or response.headers["Access-Control-Expose-Headers"] == "*"
+    )
+    assert response.headers["Access-Control-Allow-Credentials"] == "true"
+
+
 def test_get_valid_datasets(client: TestClient) -> None:
     response = client.get("/valid")
     assert response.status_code == 200

services/reverse-proxy/README.md

@@ -9,17 +9,12 @@ Note that the template configuration is located in [chart/nginx-templates/](../.
 The reverse proxy uses nginx:
 
 - it serves the static assets directly (the API also serves them if required, but it's unnecessary to go through starlette for this, and it generates errors in Safari, see [1](https://github.com/encode/starlette/issues/950) and [2](https://developer.apple.com/library/archive/documentation/AppleApplications/Reference/SafariWebContent/CreatingVideoforSafarioniPhone/CreatingVideoforSafarioniPhone.html#//apple_ref/doc/uid/TP40006514-SW6)
+- it serves the OpenAPI specification
 - it proxies the other requests to the API
-- it caches all the API responses, depending on their `cache-control` header
-- it sets the `Access-Control-Allow-Origin` header to `*` to allow cross-origin requests
 
 It takes various environment variables, all of them are mandatory:
 
 - `ASSETS_DIRECTORY`: the directory that contains the static assets, eg `/assets`
-- `CACHE_INACTIVE`: maximum duration before being removed from cache, eg `24h` (see [proxy_cache_path](https://nginx.org/en/docs/http/ngx_http_proxy_module.html#proxy_cache_path))
-- `CACHE_MAX_SIZE`: maximum size of the cache, eg `1g` (see [proxy_cache_path](https://nginx.org/en/docs/http/ngx_http_proxy_module.html#proxy_cache_path))
-- `CACHE_DIRECTORY`: the directory that contains the nginx cache, eg `/nginx-cache`
-- `CACHE_ZONE_SIZE`: size of the cache index, eg `50m` (see [proxy_cache_path](https://nginx.org/en/docs/http/ngx_http_proxy_module.html#proxy_cache_path))
 - `HOST`: domain of the reverse proxy, eg `localhost`
 - `PORT`: port of the reverse proxy, eg `80`
 - `URL_ADMIN`= URL of the admin, eg `http://admin:8080`
@@ -28,6 +23,5 @@ It takes various environment variables, all of them are mandatory:
 The image requires three directories to be mounted (from volumes):
 
 - `$ASSETS_DIRECTORY` (read-only): the directory that contains the static assets.
-- `$CACHE_DIRECTORY` (read/write): the directory that contains the nginx cache
 - `/etc/nginx/templates` (read-only): the directory that contains the nginx configuration template ([templates](./templates/))
 - `/staticfiles` (read-only): the directory that contains the static files (`openapi.json`).