Page MenuHomePhabricator

*.toolforge.org hostnames unexpectly treated as tools.wmflabs.org when the URL's path starts with a match to a grid engine webservice
Closed, ResolvedPublic

Description

Discovered by @MusikAnimal and reported on irc.

Examples:

'dspull' is not special; it was just the first tool I saw running on the job grid to test with. The behavior is the same for any webservice running on the job grid.

urlproxy.lua is allowing too many things to fall through to prefix based lookups. When the route_backend_and_exit_if_ok(subdomain, "/") case fails, the next check should be to see if ngx.var.http_host is exactly tools.wmflabs.org. If it is not, then an ngx.exit(ngx.OK) return should be done to pass handling on to the Kubernetes ingress layer.

Event Timeline

bd808 triaged this task as High priority.May 27 2020, 10:39 PM
bd808 moved this task from Inbox to Soon! on the cloud-services-team (Kanban) board.

Change 599139 had a related patch set uploaded (by BryanDavis; owner: Bryan Davis):
[operations/puppet@production] dynamicproxy: Short-circuit urlproxy lookups against canonical_domain

https://gerrit.wikimedia.org/r/599139

Change 599139 merged by Arturo Borrero Gonzalez:
[operations/puppet@production] dynamicproxy: Short-circuit urlproxy lookups against canonical_domain

https://gerrit.wikimedia.org/r/599139

Change 599315 had a related patch set uploaded (by Arturo Borrero Gonzalez; owner: Arturo Borrero Gonzalez):
[operations/puppet@production] toolforge: urlproxy: refresh code comments

https://gerrit.wikimedia.org/r/599315

Change 599315 merged by Arturo Borrero Gonzalez:
[operations/puppet@production] toolforge: urlproxy: refresh code comments

https://gerrit.wikimedia.org/r/599315

aborrero claimed this task.

Thanks @MusikAnimal and @bd808 for this!

I did some tests in both toolsbeta (before merging) and in tools (after merging) and everything seems fine now.
The two examples in the task description now lands where they are supposed to!