Skip to content

Add envoy proxy for per-beamline ingress routing - #11

Open
gilesknap wants to merge 7 commits into
auto-roll-deploymentsfrom
envoy-proxy
Open

gilesknap wants to merge 7 commits into
auto-roll-deploymentsfrom
envoy-proxy

Conversation

@gilesknap

Copy link
Copy Markdown
Contributor

Ports DiamondLightSource/diamond-tiled#2 by @ZohebShaikh into this repo's chart (helm/tiled → Charts/tiled). The original commit's authorship is kept.

Kyverno's cluster policy disallows regex ingress paths. So the ingress sends / to an envoy Service, which matches the beamline name in the path and forwards to that beamline's own tiled Service and Deployment. Each entry in beamlines: in the values gets its own Deployment.

How this differs from the original PR:

  • The TILED_SINGLE_USER_API_KEY env var is kept, as optional, rather than removed.
  • Everything already in Charts/tiled stays: the dls-tiled image, versions stamped from the tag, and IfNotPresent.

Known issue, carried over from the original: the ingress template treats ingress.hosts as a list of strings, but values.yaml still holds {host, paths} maps. With ingress.enabled=true it renders a bad host.

A design review of this chart as a central tiled for all of DLS is in progress. Follow-ups will come from that.

🤖 Generated with Claude Code

https://claude.ai/code/session_01CCtPFejfd3WP589miPqaL3

@codecov

codecov Bot commented Oct 13, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 61.64%. Comparing base (168221a) to head (8061bff).

❗ There is a different number of reports uploaded between BASE (168221a) and HEAD (8061bff). Click for more details.

HEAD has 1 upload less than BASE
Flag BASE (168221a) HEAD (8061bff)
2 1
Additional details and impacted files
@@                    Coverage Diff                     @@
##           auto-roll-deployments      #11       +/-   ##
==========================================================
- Coverage                  90.55%   61.64%   -28.91%     
==========================================================
  Files                          5        3        -2     
  Lines                        868      219      -649     
==========================================================
- Hits                         786      135      -651     
- Misses                        82       84        +2     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@gilesknap

gilesknap commented Oct 13, 2026 •

Copy link
Copy Markdown
Contributor Author

Thanks for this, @ZohebShaikh . Routing in envoy is a sensible way round the Kyverno regex-path restriction. Envoy forwards the path unchanged, so the links tiled returns stay correct: tiled builds them from Host / X-Forwarded-Proto (tiled/server/utils.py:80-81), and envoy keeps the XFP header that ingress-nginx sets. I had two independent AI-assisted reviews done against the plan for dls-tiled to become one central tiled for all of DLS, and checked them against rendered manifests and the tiled/envoy sources. These are the combined findings.

Envoy config: blocking for a data service

  1. Route timeout. No route sets timeout, so envoy applies its 15 s default. Large /array/full or /asset/bytes reads (tiled allows responses up to 300 MB) and slow searches will return 504. Add timeout: 0s (or a generous explicit value) to each route.
  2. WebSockets. The HCM has no upgrade_configs. Envoy then answers the /api/v1/stream/single/... upgrade with a 403 (conn_manager_impl.cc, createUpgradeFilterChain), so live streaming to clients breaks. Add upgrade_configs: [{upgrade_type: websocket}].
  3. Admin API on 0.0.0.0:9901. Any pod in the cluster can POST /quitquitquit and take the only proxy down. Bind admin to 127.0.0.1 and probe a health-check route on the main listener instead.
  4. Image. envoyproxy/envoy:v1.29-latest is a floating tag on an archived line (last release 1.29.12, 2024-12). Pin a supported version (1.39.x) so Renovate can track it.
  5. Single replica, no PDB. Envoy is now in front of every beamline. Run 2+ replicas with a PodDisruptionBudget and add an access log. Also pass imagePullSecrets, nodeSelector, tolerations and affinity through to the envoy Deployment.

Routing
6. ^/.*/<name>(/.*)?$ matches the beamline name in any path segment, so /api/v1/metadata/b01-1/<visit>/i22 is sent to the i22 pod. Anchor it on the tree-root segment, i.e. what follows /api/v1/<endpoint>/ (plus /zarr/v2/). Also escape the name with regexQuoteMeta and consider normalize_path: true.
7. Fail the render if envoy.defaultBeamline is not in beamlines or beamlines is empty. Otherwise envoy rejects the config at startup and the whole ingress goes dark.

Templates
8. As you noted in the description, ingress.hosts is used as strings in ingress.yaml/NOTES.txt, but values.yaml has {host, paths} maps. The rendered rule is host: "map[host:chart-example.local ...]".
9. hpa.yaml and tests/test-connection.yaml still target <fullname>, which no longer exists. Loop the HPA over beamlines, and point the test at the envoy Service.
10. beamlines[].port changes the containerPort, Service and envoy endpoint, but tiled still listens on 8000 (the image CMD sets --port 8000). Drop the option, or set the listen port from it too.
11. Minor: the values.yaml comment about the entrypoint chown is inaccurate. The chown only runs as uid 0, so with runAsUser: 1000 it is skipped. Also, <fullname>-<beamline> can exceed 63 characters.

Direction: one central tiled for DLS
As written, every beamline Deployment is identical apart from supplementalGroups: same ConfigMap, same tree at /, an ephemeral sqlite catalog under /storage, and the same TILED_SINGLE_USER_API_KEY secret. For a central service I think the chart needs:

  • Per-beamline config and storage. tiled expands $VARS in its config, so injecting TILED_BEAMLINE=<name> per Deployment and using trees: [{path: /$TILED_BEAMLINE, args: {uri: postgresql://.../$TILED_BEAMLINE, ...}}] would be cheap. Volumes would become per-beamline too (a global RWO PVC can't be mounted by N pods across nodes). Without this, the p51 pod only answers /…/p51/... if its own catalog happens to hold a top-level p51 node.
  • Shared authentication.secret_keys, plus per-beamline authorization through tiled's access policy (OPA/ExternalPolicyDecisionPoint), rather than one API key that opens every beamline. All /api/v1/auth/* traffic lands on the default pod, so tokens must verify on every other pod.

The larger question is whether a separate pod per beamline is a hard requirement (for POSIX group access to /dls/<bl>/data). If it isn't, one tiled server can mount every beamline's catalog at /p51, /i22, ... through its trees config. Clients would see exactly the same URLs, with no envoy, a root listing that shows every beamline, and one pool to scale. Because the URLs are the same either way, the choice can be revisited later. What's the reason for per-beamline pods on tiled-staging?

ZohebShaikh and others added 3 commits October 14, 2026 13:15
Ported from DiamondLightSource/diamond-tiled#2. The Kyverno cluster policy
disallows regex ingress paths, so "/" routes to an envoy Service that
matches beamline names in the path and forwards to each beamline's own
tiled Service and Deployment.

Paths move from helm/tiled to Charts/tiled. The TILED_SINGLE_USER_API_KEY
env var is kept, as optional, rather than removed.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CCtPFejfd3WP589miPqaL3
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@ZohebShaikh
ZohebShaikh changed the base branch from main to auto-roll-deployments October 14, 2026 12:16
…only

Every beamline pod now uses the catalog at TILED_CATALOG_URI, from the
tiled-secrets secret, and mounts all of /dls read only rather than a
beamline specific path. Access to each beamline's data is still limited
by its supplementalGroups.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
ZohebShaikh and others added 3 commits October 14, 2026 17:52
Envoy forwards the path unchanged, so the beamline is the first node
under the tree's root rather than the tree's mount point.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@fajinyuan

Copy link
Copy Markdown

If I understand the discussion with Cloud team correctly, 'a separate pod per beamline is a hard requirement (for POSIX group access to /dls//data)' to prevent cross-beamline data access. @ZohebShaikh could you confirm if this is still the case?
So we have to look into a federated deployment scheme, i.e. instance per beamline deployment but managed/configured centrally.
I have very little knowledge about kubernetes cloud based service environment to know how to do this correctly. @gilesknap comments here should be followed.

@ZohebShaikh

Copy link
Copy Markdown
Contributor

Yes it is a hard requirement from scicomp

@fajinyuan

fajinyuan commented Oct 16, 2026 •

Copy link
Copy Markdown

Here is the copilot suggestion to my question of 'how to do a federated deployment in kubernetes, i.e. one instance per beamline but all instance managed and configured centrally. Is there a standard architect for this?'
https://dlsltd-my.sharepoint.com/:w:/g/personal/fajin_yuan_diamond_ac_uk/IQDnuOgTMFs7QbHy_hj-MEWEAR5IbY6_DvUZH0fepcCpZmw?e=di8P0n

Hopefully it provides a way to avoid expose regular expression
s in kubernetes ingress layer.

Could we adapt the deployment management and configuration suggested in this link for Tiled deployment per beamline?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants