Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RFC 72: Issues on Mac OS #79

Closed
letitz opened this issue Apr 28, 2021 · 12 comments
Closed

RFC 72: Issues on Mac OS #79

letitz opened this issue Apr 28, 2021 · 12 comments

Comments

@letitz
Copy link
Contributor

@letitz letitz commented Apr 28, 2021

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
Copy link
Member

@foolip foolip commented Apr 28, 2021

@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
Copy link
Contributor Author

@letitz letitz commented Apr 28, 2021

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
Copy link
Contributor Author

@letitz letitz commented Apr 29, 2021

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
Copy link
Contributor Author

@letitz letitz commented Apr 29, 2021

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
Copy link
Member

@foolip foolip commented May 4, 2021

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
Copy link
Contributor

@jgraham jgraham commented May 4, 2021

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
Copy link
Contributor Author

@letitz letitz commented May 5, 2021

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
Copy link
Contributor

@jgraham jgraham commented May 5, 2021

@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
Copy link

@ddragana ddragana commented May 5, 2021

@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
Copy link
Contributor Author

@letitz letitz commented May 5, 2021

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

@letitz
Copy link
Contributor Author

@letitz letitz commented May 6, 2021

#83 should fix this issue.

@letitz
Copy link
Contributor Author

@letitz letitz commented Jun 14, 2021

#83 was merged.

@letitz letitz closed this Jun 14, 2021
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

No branches or pull requests

4 participants