Arvados

17209-http-forward @ 54f5f3b0868709dc9a45963bc8224fa0ff4813df -- developer-run-tests: #4673

Rebased/rewrote stale branch (container gateway things changed a lot in Feature #19889: access current container logs at /arvados/v1/containers/{uuid}/log/{filename})

17209-http-forward @ a651cd2938c6592d4a60aa3c696ce681b0615d60 -- developer-run-tests: #4675

• test/fix "proxy to tunnel terminated at other controller process" case
• update permission check (currently it's just "is container readable?")

17209-http-forward @ 74153a23bede1c925167ec74a0aa17a5adbe479e -- developer-run-tests: #4676

17209-http-forward @ f5a7f839b82ec9c939c03e8924227018b651680d -- developer-run-tests: #4681

• All agreed upon points are implemented / addressed. Describe changes from pre-implementation design.
  • Permission check is the same as for container SSH access, i.e., ports can be accessed by admins, and by the user who submitted all of the CRs that reference the container
  • We didn't discuss a way to disable the feature via config, and I didn't add one. I figure this is OK (at least for now) since it is effectively disabled by default until someone configures upstream proxies, DNS, and TLS certificates, none of which is documented yet (#22613).
• Anything not implemented (discovered or discussed during work) has a follow-up story.
  • ✅ this is a minimal implementation, follow-up is on #22613, #17207, #22551
• Code is tested and passing, both automated and manual, what manual testing was done is described.
  • ✅ automated tests only.
• New or changed UX/UX and has gotten feedback from stakeholders.
  • n/a
• Documentation has been updated.
  • no, we decided not to until #22613
• Behaves appropriately at the intended scale (describe intended scale).
  • The permission checks (get user/container) aren't cached, so they could generate some real RailsAPI load if the container hosts (for example) a web app that makes lots of requests. Added a note to #22551 that we should have a cache to avoid doing a lookup on every request to a "public" port.
• Considered backwards and forwards compatibility issues between client and server.
  • none
• Follows our coding standards and GUI style guidelines.
  • ✅

• File lib/crunchrun/container_gateway.go: Leftover commented-out code at lines 366-370
• I'm guessing this feature will have a config knob like Services.WebDAV.ExternalURL, just not this first prototype? Being able to accept connections to a domain name like <uuid>-<port>--containers.zzzzz.example.com will be necessary on some deployments.

Apart from that, LGTM.

Lucas Di Pentima wrote in #note-15:

• File lib/crunchrun/container_gateway.go: Leftover commented-out code at lines 366-370

Oops, yes. Removed.

• I'm guessing this feature will have a config knob like Services.WebDAV.ExternalURL, just not this first prototype? Being able to accept connections to a domain name like <uuid>-<port>--containers.zzzzz.example.com will be necessary on some deployments.

Yes. I didn't add it here because it seemed too close to advertising the half-implemented feature. Added a note to #22613 description.

• ensure TLS certificates validate for *.containers.domain.example
• update Nginx config so *.containers.domain.example routes to controller, e.g.,

  server {
    // ...
    server_name domain.example ~\.containers\.domain\.example;
  

• point browser to https://{container-uuid}-{port}.containers.domain.example