web-platform-tests / rfcs

web-platform-tests RFCs
75 stars 63 forks source link

RFC 72: Issues on Mac OS #79

Closed letitz closed 3 years ago

letitz commented 3 years ago

I've been implementing RFC 72 in Chromium [1] [2] and uncovered an issue on Mac OS: it seems that the loopback interface is only bound to 127.0.0.1/32 [3], not 127.0.0.1/8 as envisioned in the RFC.

This is an issue on Chromium's Mac OS test runners, and would probably be an issue on WPT Mac OS test runners as well as on developers' Mac machines. Fixing it requires issuing ifconfig commands with root privileges.

It is my understanding that root privileges should not be required to run WPTs. Is that correct? If so, I will have to propose an alternative approach that does not rely on serving WPTs from IP addresses other than 127.0.0.1.

[1] https://chromium-review.googlesource.com/c/chromium/src/+/2843473 [2] https://chromium-review.googlesource.com/c/chromium/src/+/2851238 [3] https://superuser.com/questions/458875/how-do-you-get-loopback-addresses-other-than-127-0-0-1-to-work-on-os-x

foolip commented 3 years ago

@letitz what are the tests that would be affected by this? We do support running Chrome on macOS but don't do it for wpt.fyi. But tests in infrastructure/ do get run on Chrome on macOS, so if you add a test there, we could tell if it's an issue generally of "just" for Chromium's CI setup.

letitz commented 3 years ago

Private Network Access tests that depend on the address space override mechanism described in RFC #72 would directly be affected. These live in fetch/private-network-access.

WPT local servers might also fail to bind to e.g. 127.1.0.1 on Mac OS, regardless of the browser under test. Such failures should be treated as non-fatal so as to allow tests to run anyway - tests that do not rely on address space overrides should not notice that the servers are not listening for incoming connections. Since I will implement all that logic, I can easily do that.

If tests outside infrastructure/ are never run by WPT CI on Mac OS, then wpt.fyi would indeed be unaffected by this problem. Very cool.

How do you feel about these tests failing on regular (non-Chromium-CI) Mac OS machines, such as those of web or browser developers?

letitz commented 3 years ago

I took a look at where to put the code to serve on more than a single IP address, and stumbled across infrastructure/server/wpt-server-http.sub.html. It seems a natural place to test that the servers are working on the alternate IPs...

letitz commented 3 years ago

Hm. Upon second thought, it seems that local development should not too much of a problem, we can add macOS-specific instructions near the existing /etc/hosts setup instructions instructing users to run a script that sets up loopback addresses.

foolip commented 3 years ago

If tests outside infrastructure/ are never run by WPT CI on Mac OS, then wpt.fyi would indeed be unaffected by this problem. Very cool.

They are run for Safari, but Chrome and Firefox only ever run infrastructure/ on macOS here in wpt's CI. I'm not sure what runs in Gecko's CI on macOS, but @jgraham knows.

Ultimately, if macOS-specific configuration is needed to make this work, then we can make it part of https://web-platform-tests.org/running-tests/from-local-system.html#macos-setup (which is currently wrong) and make sure the instructions match what is done in CI (on Azure Pipelines here in WPT).

jgraham commented 3 years ago

Gecko runs ~everything in CI on macOS. I don't know exactly how much flexibility we have when configuring those machines, but I would strongly prefer to avoid anything requiring system-level changes (they are difficult to do, difficult to configure alongside the tests, and liable to leak into other tasks run on the same machine pool).

letitz commented 3 years ago

This situation is unfortunate. One of the main rationales for choosing this approach in RFC #72 was that it avoided the need for system-level changes requiring root permissions... Otherwise, we could have added routes for public IPs in some obscure address block to loopback and be done with it.

It seems to me that adding routes for 127.0.0.2 or 127.2.0.1, or even 127.0.0.0/8 to loopback on CI machines should not be big problem. It is easy to do with ifconfig, and even if it leaked into other tasks environments, it would likely have no effect - most other OSes provide routes for this address block anyway.

If this is unacceptable, we could consider overriding the address space based on (IP, port) pairs, e.g.:

--ip-address-space-overrides=127.0.0.1:8002=private,127.0.0.1:8003=public

And run extra servers on different ports instead of different IP addresses. My main worry with such an approach is that it already took months to land RFC #72, and I was hoping to write comprehensive WPTs for Private Network Access in the next two weeks or so to prepare for shipping. If we could agree on a small change like this faster, I might be able to still make the timeline work.

jgraham commented 3 years ago

@ddragana should comment on whether that kind of port-specific override is OK for Gecko.

From the point of view of CI, "just run ifconfig" is not something I'd assume to be simple. Machines are often configured so that specific tasks don't have root and on mac in particular the lack of containerisation makes it harder to test and deploy configuration changes. Figuring out what's even possible here would probably involve pulling in the ops people who have detailed knowledge of how the machines are set up and what's achieveable. It's not impossible, but if we could do something at the browser level it would be a lot easier to ensure it can be deployed to different environments.

ddragana commented 3 years ago

@ddragana should comment on whether that kind of port-specific override is OK for Gecko.

We can make the port-specific override work in Gecko. This work-around is fine for Gecko.

letitz commented 3 years ago

Thanks @ddragana for the quick response! I'll send a PR to amend RFC #72.

letitz commented 3 years ago

83 should fix this issue.

letitz commented 3 years ago

83 was merged.