RMST-364 - Implementing cache-control header for git-reader routes

docs/deployment.rst

@@ -43,14 +43,17 @@ Container Entry Points
 ''''''''''''''''''''''
 
 * ``web`` (*default*): runs the Web API on port ``8000`` with Prometheus metrics on port ``9090``. The following env vars must be set:
-  - ``GIT_REPO_PATH``: the path to the checked out git repo folder
-  - ``SELF_CONTAINED``: whether to serve everything from the Web API
-  - ``ATTACHMENTS_BASE_URL``: the attachments base URL if ``SELF_CONTAINED=false``
+    - ``GIT_REPO_PATH``: the path to the checked out git repo folder
+    - ``SELF_CONTAINED``: whether to serve everything from the Web API
+    - ``ATTACHMENTS_BASE_URL``: the attachments base URL if ``SELF_CONTAINED=false``
+    - ``CACHE_CONTROL_SHORT_EXPIRES_SECONDS``: sets the ``cache-control`` response header to ``max-age={value}`` value for volatile endpoints. Default is 60.
+    - ``CACHE_CONTROL_LONG_EXPIRES_SECONDS``: sets the ``cache-control`` response header ``max-age={value}`` value for stable endpoints. Default is 3600.
+    - ``CACHE_CONTROL_STATIC_EXPIRES_SECONDS``: sets the ``cache-control`` response header ``max-age={value}`` value for static content, like attachments. Default is 604800 (1 week).
 
 * ``gitupdate``: initializes or update the checked out git repo folder. Requires the following settings:
-  - ``GIT_REPO_PATH``: the path to the checked out git repo folder
-  - ``GIT_REPO_URL``: the Git+SSH origin URL
-  - In order to avoid rate limiting with pulling large amounts of files from Git LFS, SSH authentication must be setup. Recommended way is to mount SSH keys into the container at ``/app/.ssh/id_ed25519`` and ``/app/.ssh/id_ed25519.pub``. See git-reader's README for more details.
+    - ``GIT_REPO_PATH``: the path to the checked out git repo folder
+    - ``GIT_REPO_URL``: the Git+SSH origin URL
+    - In order to avoid rate limiting with pulling large amounts of files from Git LFS, SSH authentication must be setup. Recommended way is to mount SSH keys into the container at ``/app/.ssh/id_ed25519`` and ``/app/.ssh/id_ed25519.pub``. See git-reader's README for more details.
 
 Liveliness and readiness probe at ``:8000/__lbheartbeat__``.
 

git-reader/README.md

@@ -8,10 +8,7 @@ docker build -t remote-settings-git-reader .
 
 ## Settings
 
-- ``GIT_REPO_PATH``: the path to the Git repository to use.
-- ``SELF_CONTAINED`` (default: `false`): if set to `true`, the application will serve all necessary content from the Git repository, including
-  attachments and certificates chains.
-- ``ATTACHMENTS_BASE_URL`` (default: `None`): this URL will be used as the base URL for attachments. If `SELF_CONTAINED` is `false`, this URL is required. With self-contained, the current domain will be used by default (`Host` request header) if not set.
+See the [Container Entry Points](https://remote-settings.readthedocs.io/en/latest/deployment.html#container-entry-points) documentation for details on settings.
 
 
 ## Running the application

git-reader/app.py

@@ -137,6 +137,18 @@ class Settings(BaseSettings):
         alias="granian_trusted_hosts",
         description="List of trusted hosts for proxy headers.",
     )
+    cache_control_short_expires_seconds: int = Field(
+        60,
+        description="Sets the cache-control response header to max-age={value} for volatile endpoints, default is 60",
+    )
+    cache_control_long_expires_seconds: int = Field(
+        3600,
+        description="Sets the cache-control response header to max-age={value} for stable endpoints, default is 3600",
+    )
+    cache_control_static_expires_seconds: int = Field(
+        604800,
+        description="Sets the cache-control response header to max-age={value} for static content, like attachments. Default is 604800 (1 week)",
+    )
 
 
 @lru_cache(maxsize=1)
@@ -541,6 +553,20 @@ async def requests_metrics(
     return response
 
 
+@app.middleware("http")
+async def set_default_headers(
+    request: Request,
+    call_next: Callable[[Request], Awaitable[Response]],
+) -> Response:
+    response = await call_next(request)
+    settings = get_settings()
+    if "cache-control" not in response.headers:
+        response.headers["cache-control"] = (
+            f"max-age={settings.cache_control_long_expires_seconds}"
+        )
+    return response
+
+
 @checks.register
 def git_repo_health() -> list:
     result = []
@@ -570,6 +596,7 @@ def hello_unsuffixed():
 @app.get(f"/{API_PREFIX}", response_model=HelloResponse)
 def hello(
     request: Request,
+    response: Response,
     settings: Settings = Depends(get_settings),
     git: GitService = Depends(GitService.dep),
 ) -> dict:
@@ -614,10 +641,12 @@ def hello(
     response_model=ChangesetResponse,
 )
 def monitor_changes(
+    response: Response,
     _expected: Annotated[int, Query(ge=0)],
     _since: Annotated[int, Query(ge=0)] | None = None,
     bucket: str | None = None,
     collection: str | None = None,
+    settings: Settings = Depends(get_settings),
     git: GitService = Depends(GitService.dep),
 ):
     if _since and _expected > 0 and _expected < _since:
@@ -626,6 +655,11 @@ def monitor_changes(
             detail="_expected must be superior to _since if both are provided",
         )
 
+    if _expected == 0 or f"{_expected}".startswith("9999"):
+        response.headers["cache-control"] = (
+            f"max-age={settings.cache_control_short_expires_seconds}"
+        )
+
     timestamp, metadata, changes = git.get_monitor_changes_changeset(
         _since=_since, bucket=bucket, collection=collection
     )
@@ -642,6 +676,7 @@ def monitor_changes(
 )
 def collection_changeset(
     request: Request,
+    response: Response,
     bid: str,
     cid: str,
     _expected: Annotated[int, Query(ge=0)],
@@ -671,6 +706,11 @@ def collection_changeset(
         without_since = request.url.remove_query_params("_since")
         return RedirectResponse(without_since, status_code=307)
 
+    if "-preview" in f"{bid}/{cid}":
+        response.headers["cache-control"] = (
+            f"max-age={settings.cache_control_short_expires_seconds}"
+        )
+
     if settings.self_contained:
         # Certificate chains are served from this server.
         x5u = metadata["signature"]["x5u"]
@@ -692,7 +732,14 @@ def collection_changeset(
 
 
 @app.get(f"/{API_PREFIX}__broadcasts__", response_model=BroadcastsResponse)
-def broadcasts(git: GitService = Depends(GitService.dep)):
+def broadcasts(
+    response: Response,
+    settings: Settings = Depends(get_settings),
+    git: GitService = Depends(GitService.dep),
+):
+    response.headers["cache-control"] = (
+        f"max-age={settings.cache_control_short_expires_seconds}"
+    )
     return git.get_broadcasts()
 
 
@@ -703,12 +750,16 @@ def broadcasts(git: GitService = Depends(GitService.dep)):
 )
 def cert_chain(
     pem: str,
+    response: Response,
     settings: Settings = Depends(get_settings),
     git: GitService = Depends(GitService.dep),
 ):
     if not settings.self_contained:
         raise HTTPException(status_code=404, detail="cert-chains/ not enabled")
     try:
+        response.headers["cache-control"] = (
+            f"max-age={settings.cache_control_static_expires_seconds}"
+        )
         return git.get_cert_chain(pem)
     except (FileNotFoundError, IsADirectoryError):
         raise HTTPException(status_code=404, detail=f"{pem} not found")
@@ -776,11 +827,23 @@ def attachments(
         cached_content: io.BytesIO = request.state.cache["_cached_startup_content"]
         cached_content.seek(0)
         # Stream from memory.
-        return StreamingResponse(cached_content, media_type="application/x-mozlz4")
+        return StreamingResponse(
+            cached_content,
+            media_type="application/x-mozlz4",
+            headers={
+                "cache-control": f"max-age={settings.cache_control_static_expires_seconds}"
+            },
+        )
 
     # Stream from disk
     mimetype, _ = mimetypes.guess_type(requested_path)
-    return FileResponse(requested_path, media_type=mimetype)
+    return FileResponse(
+        requested_path,
+        media_type=mimetype,
+        headers={
+            "cache-control": f"max-age={settings.cache_control_static_expires_seconds}"
+        },
+    )
 
 
 def app_factory() -> FastAPI:

git-reader/tests/test_api.py

@@ -131,6 +131,10 @@ def fake_repo(temp_dir):
                 "password-rules/def.json",
                 {"id": "def", "last_modified": 113456789, "pim": "pam"},
             ),
+            (
+                "password-rules-preview/abc.json",
+                {"id": "abc", "last_modified": 113456788, "foo": "baz"},
+            ),
             (
                 "password-rules/metadata.json",
                 {
@@ -142,6 +146,17 @@ def fake_repo(temp_dir):
                     ],
                 },
             ),
+            (
+                "password-rules-preview/metadata.json",
+                {
+                    "id": "password-rules-preview",
+                    "bucket": "main",
+                    "signature": {"x5u": "https://autograph/a/b/cert.pem"},
+                    "signatures": [
+                        {"x5u": "https://autograph/a/b/cert.pem"},
+                    ],
+                },
+            ),
         ],
         base_tree=base_tree,
     )
@@ -155,6 +170,13 @@ def fake_repo(temp_dir):
         author,
         "Message",
     )
+    repo.create_tag(
+        "v1/timestamps/main/password-rules-preview/113456789",
+        oid,
+        ObjectType.COMMIT,
+        author,
+        "Message",
+    )
 
     # Create a new version of this collection.
     base_tree = repo[oid].tree
@@ -209,14 +231,17 @@ def clear_settings_cache():
 
 
 @pytest.fixture
-def get_settings_override(temp_dir):
+def get_settings_override(temp_dir, monkeypatch):
     from app import Settings
 
+    # we must patch GIT_REPO_PATH to something, or the set_default_headers middleware will throw an error
+    monkeypatch.setenv("GIT_REPO_PATH", temp_dir)
+
     return lambda: Settings(self_contained=True, git_repo_path=temp_dir)
 
 
 @pytest.fixture
-def app(get_settings_override):
+def app(get_settings_override, monkeypatch):
     from app import app, get_settings
 
     app.dependency_overrides[get_settings] = get_settings_override
@@ -316,13 +341,15 @@ def test_hello_view(api_client):
     assert (
         data["capabilities"]["attachments"]["base_url"] == "http://test/v2/attachments/"
     )
+    assert resp.headers["cache-control"] == "max-age=3600"
 
 
 def test_broadcast_view(api_client):
     resp = api_client.get("/v2/__broadcasts__")
     assert resp.status_code == 200
     data = resp.json()
     assert "remote-settings/monitor_changes" in data["broadcasts"]
+    assert resp.headers["cache-control"] == "max-age=60"
 
 
 def test_monitor_changes_view(api_client):
@@ -335,6 +362,7 @@ def test_monitor_changes_view(api_client):
     assert data["changes"][0]["bucket"] == "main"
     assert data["changes"][0]["collection"] == "password-rules"
     assert "last_modified" in data["changes"][0]
+    assert resp.headers["cache-control"] == "max-age=60"
 
 
 def test_monitor_changes_view_filtered_since(api_client):
@@ -354,6 +382,7 @@ def test_monitor_changes_bad_expected(api_client, _expected):
         f"/v2/buckets/monitor/collections/changes/changeset{expected_param}"
     )
     assert resp.status_code in (400, 422)
+    assert resp.headers["cache-control"] == "max-age=3600"
 
 
 def test_monitor_changes_view_filtered_bad_since(api_client):
@@ -409,6 +438,7 @@ def test_changeset(api_client):
         == "http://test/v2/cert-chains/a/b/cert.pem"
     )
     assert data["changes"] == [{"id": "abc", "last_modified": 123456789, "foo": "bar"}]
+    assert resp.headers["cache-control"] == "max-age=3600"
 
 
 def test_changeset_unknown_collection(api_client):
@@ -418,6 +448,14 @@ def test_changeset_unknown_collection(api_client):
     assert resp.status_code == 404
 
 
+def test_changeset_preview_collection(api_client):
+    resp = api_client.get(
+        "/v2/buckets/main/collections/password-rules-preview/changeset?_expected=0"
+    )
+    assert resp.status_code == 200
+    assert resp.headers["cache-control"] == "max-age=60"
+
+
 @pytest.mark.parametrize(
     "since", ['"42"', '"not-a-number"', "123.456", "-1", '"223456789"']
 )
@@ -466,6 +504,7 @@ def test_changeset_since(api_client):
 def test_cert_chain(api_client):
     resp = api_client.get("/v2/cert-chains/a/b/cert.pem")
     assert resp.status_code == 200
+    assert resp.headers["cache-control"] == "max-age=604800"
     assert "-----BEGIN CERTIFICATE-----" in resp.text
 
 
@@ -489,6 +528,7 @@ def test_startup_rewrites_x5u(api_client, temp_dir):
 
     resp = api_client.get("/v2/attachments/bundles/startup.json.mozlz4")
     assert resp.status_code == 200
+    assert resp.headers["cache-control"] == "max-age=604800"
     data = read_json_mozlz4(resp.content)
     assert (
         data[0]["metadata"]["signature"]["x5u"]