Env: Fix loopback requests when running on non-default ports#77057
Env: Fix loopback requests when running on non-default ports#77057
Conversation
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the Unlinked AccountsThe following contributors have not linked their GitHub and WordPress.org accounts: @noreply@agent.dev, @tbclark3, @adrianduffell, @Sam-Smyth, @celtislab, @collinr3. Contributors, please read how to link your accounts to ensure your work is properly credited in WordPress releases. If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
|
Size Change: 0 B Total Size: 7.74 MB ℹ️ View Unchanged
|
|
I'm looking into the failing E2E tests because they seem to only be failing on this branch. It might be a side-effect of fixing the loopback requests but I'll double-check. |
|
So I ran the failing tests locally both with the fix in this branch and without and got a failure both times. It doesn't appear that the failing E2E test is related to the work in this branch. It seems like the issue is the test itself. |
|
@jeryj pinging you because I think you've had some involvement with the test specs that are failing. Anything stand out to you about why they might be failing? Or is it something that should be looked into as an artifact of all the work that has been done around the areas being tested (or is there someone else who should be pinged)? |
|
@nerrad I bet it has to do with permalinks. The failure is related to the expected URL returned: For testing entity bound links, we needed to use pretty permalinks to reliably test that updates to the slug would be picked up by bound navigation links, so the expected returned URL had a format like The |
ObliviousHarmony
left a comment
There was a problem hiding this comment.
Thanks @nerrad,
I ran through all of the testing instructions and it works great, the code is aligned with existing functionality.
Regarding the E2E failure, like you, I reproduced it on both trunk and this branch. For testing purposes I destroyed my entire local environment when switching branches to make sure nothing from these changes persisted in the container. I poked at it with Claude and @jeryj's feedback but it has absolutely no idea how this branch could relate.
|
The local navigation block e2e test failures were due to the Home keypress wasn't working in Playwright for Mac OS. I have a fix in #77102 that should help make debugging the test failures here more reliable. |
When wp-env runs on any port other than 80, PHP loopback requests inside the WordPress container (WP-Cron, REST API loopback, Site Health diagnostic) fail with "Connection refused" because Apache only listens on port 80 while WP_HOME points to the host-mapped port (e.g. localhost:8888). Fix by injecting two Apache config lines into the generated WordPress Dockerfile that make Apache also listen on the host-mapped port. This is a no-op when the port is 80 or 443. Validated approach from docker-library/wordpress#611. Fixes #20569
Merges the docker-config test file into the existing build-docker-compose-config test file (same runtime/docker module area) and removes duplicate assertions. Reduces from 8 to 4 tests with no loss of coverage: one unit test for each of getLoopbackPortConfig's three branches (non-default port, port 80, port 443) plus one wordpressDockerFileContents integration test that proves the helper is wired in and each environment uses its own port.
9cfe7d2 to
f959ed2
Compare
|
I rebased this PR on top of trunk. This ensures that artifacts are correctly generated when E2E tests fail, which should help in investigating the problem. See #77093 |
According to Playwright's report, this hypothesis seems correct. The preview title, which was previously
However, upon further consideration, isn't this the intended behavior? That is, the loopback request is fixed in the local environment updated by this PR, causing the request to I tested this on my own site, and the link preview displays the site title even though it's my own site page:
Perhaps we should fix navigation block tests? |




What?
Makes Apache inside the wp-env WordPress container also listen on the host-mapped port, so PHP loopback requests (WP-Cron, REST API loopback, Site Health diagnostic,
add_editor_style()) succeed instead of gettingcURL error 7: Connection refused.Fixes #20569
Why?
Docker maps host port (e.g.
8888) → container port80. wp-env setsWP_HOME = http://localhost:8888. But Apache inside the container only listens on80. When PHP code doeswp_remote_get( WP_HOME ), it hitslocalhost:8888on the container's loopback interface where nothing is listening.This has been a known issue since 2020 (#20569) with various workarounds (
ALTERNATE_WP_CRON,host.docker.internalhosts hack, etc.), but the proper fix — validated in the upstream docker-library/wordpress#611 comment — is to make Apache listen on both ports.How?
Adds a
getLoopbackPortConfig( port )helper todocker-config.jsthat returns two Dockerfile RUN steps:These are injected into the generated
WordPress.DockerfileandTests-WordPress.Dockerfileonly whenport !== 80 && port !== 443. The helper returns''for port 80/443, keeping the Dockerfile identical to today's output for those cases.The port comes from
config.env[env].port, which is already finalised by the timewriteDockerFilesruns and is the same integer used in the docker-compose port mapping.Containers not touched:
cli/tests-cli(Alpine, no Apache),phpmyadmin,mysql/tests-mysql.Testing
Unit tests (new)
8 tests in
packages/env/lib/test/docker-config.js:getLoopbackPortConfigreturns correct Apache directives for non-default port80replacement)''for port 80 and 443wordpressDockerFileContentsinjects per-env port for development and testsnpx wp-scripts test-unit-js --config test/unit/jest.config.js --testPathPattern='packages/env/lib/test/docker-config' --no-coverageAll 153 tests across the full env package pass.
Manual verification steps
npx wp-env run wordpress 'cat /etc/apache2/ports.conf'→ shows bothListen 80andListen <port>npx wp-env run wordpress 'cat /etc/apache2/sites-enabled/000-default.conf' | grep VirtualHost→<VirtualHost *:80 *:<port>>npx wp-env run wordpress 'curl -sS -o /dev/null -w "%{http_code}\n" http://localhost:<port>/wp-login.php'→ 200/302npx wp-env run cli wp cron event run --due-now→ no cURL errorshttp://localhost:<port>/wp-admin/site-health.php→ no loopback failureListen 80and<VirtualHost *:80>WP_ENV_PORTrebuilds with new portKnown limitation —
wp cron testfrom the CLI containernpx wp-env run cli wp cron teststill fails withcURL error 7.wp cron testspawns a loopback from inside the CLI container, whoselocalhostis the CLI container itself (Alpine, no Apache) — not the wordpress container. Fixing this needs a different mechanism (e.g. resolvingWP_HOME's host to the wordpress service from the CLI container).Question for reviewers: handle in this PR, or split into a follow-up? I'm on the fence but leaning toward followup.