Skip to content

feat: Add DevContainer configuration for local development - #484

Open
Ramjivan wants to merge 13 commits into
BeWelcome:feature/docker-masterfrom
Ramjivan:feat/devcontainer-issue-69
Open

feat: Add DevContainer configuration for local development#484
Ramjivan wants to merge 13 commits into
BeWelcome:feature/docker-masterfrom
Ramjivan:feat/devcontainer-issue-69

Conversation

@Ramjivan

@Ramjivan Ramjivan commented Aug 29, 2026

Copy link
Copy Markdown

��This PR introduces a devcontainer setup for BeWelcome Rox to standardise and simplify the developer onboarding experience, solving Issue #69.

?? What this does:

  1. Zero manual setup: A contributor can clone the repo, click "Reopen in Container" in VS Code (or use GitHub Codespaces), and the environment will build, install dependencies, seed the DB, and compile assets automatically.
  2. Reuses existing infrastructure: It points directly to the existing docker-compose.yml and docker-compose.override.yml.dist. No new Docker networks or redundant configurations are introduced.
  3. Ports & Defaults: Standard ports (8080, 8043, 1080) are auto-forwarded and labelled for a cleaner experience.
  4. Fixes minor typo: Corrected docker/db/word.sql to docker/db/words.sql in docker-entrypoint.sh.

?? Discussion Points for the Team:

I would love to get your thoughts on a couple of implementation details so we can ensure this is the right approach for everyone:

  • Xdebug Installation: I have added xdebug to the bewelcome_php_dev stage in the Dockerfile directly. This ensures the Docker layer caches the compilation and it is immediately available (set to off by default for performance). Question: Are we comfortable adding this to the Dockerfile, or would you prefer it to be installed dynamically (e.g. via a postCreateCommand script) to keep the Dockerfile completely untouched?
  • GitHub Codespaces: This devcontainer.json configuration also natively supports GitHub Codespaces! Since the repo is public, contributors can use it for free. Perhaps we should document this as an alternative in INSTALL.md in the future?
  • Target Branch: This is targeting master. Does this align with our usual branching strategy for infrastructure enhancements?

Closes #69

@Ramjivan

Copy link
Copy Markdown
Author

🧪 How to Test This PR

If you want to help test this DevContainer setup, here is how you can do it quickly:

Prerequisites:

  • Docker installed and running
  • VS Code with the "Dev Containers" extension installed (ms-vscode-remote.remote-containers)

Steps to test:

  1. Fetch and checkout this branch:
    git fetch origin pull/484/head:test-devcontainer
    git checkout test-devcontainer
  2. Open the project in VS Code:
    code .
  3. A notification will appear in the bottom right: "Folder contains a Dev Container configuration file. Reopen folder to develop in a container." Click Reopen in Container.
    (Alternatively, press F1 and type Dev Containers: Reopen in Container)
  4. Wait for the image to build and the postCreateCommand to finish running (it will download dependencies and seed the database).
  5. Once the terminal becomes available, the app should be running!

What to look out for:

  • Check if Xdebug activates properly if you trigger it via VS Code.
  • Let us know if the database seeded correctly for you without manual intervention.

- Add .devcontainer configuration for VS Code and Codespaces

- Add xdebug to bewelcome_php_dev stage for local debugging

@Neophytis Neophytis left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good work on this — devcontainer support has been wanted for a while (closes #69) and the overall setup is clean. Two things before merging:

1. Target branch / base stack

This PR targets develop which uses FrankenPHP. Our actual beta and production Docker deployment runs on feature/docker-master (php-fpm + nginx). A devcontainer built against develop means contributors work against a runtime that doesn't match what gets deployed — which is the main thing a devcontainer should prevent.

I'd suggest rebasing against feature/docker-master and pointing dockerComposeFile at that branch's docker-compose.yml. The Codespaces support carries over unchanged and contributors get an environment that actually mirrors production.

2. Xdebug in the Dockerfile

Baking xdebug into the Dockerfile (disabled by default via xdebug.mode=off) is the right approach. It avoids recompiling the extension on every fresh container and keeps it opt-in with XDEBUG_MODE=debug. The postCreateCommand alternative would hit a compile penalty on every rebuild. Current approach is correct.

@Ramjivan
Ramjivan force-pushed the feat/devcontainer-issue-69 branch from 6b98962 to 1bbdcbe Compare August 29, 2026 14:06
@Ramjivan
Ramjivan changed the base branch from develop to feature/docker-master August 29, 2026 14:06
@Ramjivan

Ramjivan commented Aug 29, 2026

Copy link
Copy Markdown
Author

Thanks Peter! Makes total sense. I've updated this PR:

  1. Changed the base branch to feature/docker-master
  2. Configured devcontainer.json to target the php container and route port 80 for Nginx
  3. Kept Xdebug in the bewelcome_php_dev stage inside the Dockerfile.

This is now ready for review against the production-mirroring stack!

@Ramjivan

Ramjivan commented Aug 29, 2026

Copy link
Copy Markdown
Author

I also just added a small hostRequirements block to explicitly configure GitHub Codespaces machine sizes (2 CPUs, 4GB RAM) as Peter requested!

@thisismeonmounteverest

Copy link
Copy Markdown
Contributor

@Ramjivan Seems the checks fail as they now use yarn again but develop moved to bun.

Regarding FrankenPHP or nginx/PHP-FPM: FrankenPHP is likely faster and therefore the better choice in the future. Any way to set this up that it can be tested in parallel?

Mailcatcher (even sj26/mailcatcher) doesn't get updates anymore (last push 2 years ago). MailPit had the last update 8 days ago and has an API which could be used for testing.

@Ramjivan

Copy link
Copy Markdown
Author

@thisismeonmounteverest @Neophytis

Yeah, the CI failures are just coming from the base docker-master branch failing on its own.

To let us test both setups, I opened #486 pointing at develop for the FrankenPHP side. We can keep this PR pointing at docker-master for Nginx.

Totally agree on swapping to MailPit. Want me to add that change into these PRs, or keep it in a separate PR?

@thisismeonmounteverest

Copy link
Copy Markdown
Contributor

@Ramjivan I think we can switch to mailpit here. As it is not used for anything else than development.

@thisismeonmounteverest

thisismeonmounteverest commented Aug 30, 2026

Copy link
Copy Markdown
Contributor

@Ramjivan I tried with Antigravity and building the container failed with a message that xdebug could not be setup/build.

I also tried in PHPStorm (the IDE I normally use) and there it failed silently. So I asked the AI assistant there:

"## Short answer
Your devcontainer is tied to the Compose php service. It may be reusing the existing image, and when it does build, it must build the whole stage first. The most suspicious actual build problem is that your dev stage calls , but that command is not installed in the official PHP Alpine image. bewelcome/php``bewelcome_php``install-php-extensions xdebug"

@Ramjivan

Ramjivan commented Aug 30, 2026

Copy link
Copy Markdown
Author

I have pushed fixes for the remaining issues in this PR:

  1. Wait Loop Fix (docker-entrypoint.sh): Added a .gitattributes file to enforce LF line endings for bash scripts (.sh), �in/console, and .env. The root cause of the PHP container waiting infinitely for the DB was that the .env file checked out with CRLF on Windows, causing DB_HOST to be extracted as db
    . Enforcing LF across platforms fixes this permanently.
  2. Behat & Mailpit: Synced composer.lock for the �ehat-mailpit-extension, fixed the cache:clear autowiring crash in services.yaml for DataTransformer, and verified that the context successfully runs.
  3. Xdebug Build Crash: Fixed the DevContainer build failure caused by install-php-extensions crashing when fetching Xdebug by using the dedicated mlocati helper script in the Dockerfile.

The Behat tests run cleanly against Mailpit! (Note: the local DB schema bug with
ightsvolunteers FK constraint still occurs on this branch, but as discussed, we are keeping this PR strictly focused on the dev environment plumbing).

@thisismeonmounteverest

Copy link
Copy Markdown
Contributor

Unfortunately, still no luck on Windows in WSL (and Antigravity). Can't even say what the error is now, but saw that the container gets build as production which doesn't make sense for a dev setup, does it?

@Neophytis

Copy link
Copy Markdown
Contributor

Good progress Ram — the mlocati COPY --from fix for install-php-extensions is the correct approach for php:fpm-alpine, and the .gitattributes LF enforcement is a solid permanent fix for Windows line endings. Three remaining issues before this is mergeable:

1. devcontainer is building the production stage, not the dev stage (critical)

This is the root cause of Shevek's "container builds as production" observation. docker-compose.yml builds bewelcome_php (the production PHP-FPM stage). docker-compose.override.yml.dist adds volumes but does not override the build target. So when the devcontainer runs docker compose up, it builds and attaches to the production image — which has none of the dev tooling, no xdebug, and isn't designed for interactive development.

The fix is one line in docker-compose.override.yml.dist:

services:
  php:
    build:
      target: bewelcome_php_dev

bewelcome_php_dev is the correct and intended target for devcontainers — it has the dev tools, xdebug, and is designed for interactive development. Validating that bewelcome_php (production) builds correctly is CI's job, not the devcontainer's. Those are separate concerns.

2. UTF-8 BOM in bewelcome.xdebug.ini

The file starts with a BOM character (\xEF\xBB\xBF). PHP's INI parser does not strip BOMs and will misread the first directive or silently ignore the file on some versions. Open the file in your editor, save as UTF-8 without BOM. The same file in PR #486 is clean — use that version as reference.

3. Duplicate SYMFONY_TRUSTED_PROXIES in .env

The variable is defined twice — SYMFONY_TRUSTED_PROXIES=REMOTE_ADDR then immediately SYMFONY_TRUSTED_PROXIES=127.0.0.0/8,.... The first line is dead code and should be removed.


On FrankenPHP

To be explicit: FrankenPHP is not the direction for feature/docker-master and that decision shouldn't come through a devcontainer PR. This branch is the PHP-FPM + Nginx stack that runs on beta and production. A devcontainer pointing to a different runtime breaks dev/prod parity, which is the main thing a devcontainer is supposed to prevent. If we ever migrate the production stack to FrankenPHP, that is a separate architectural decision — it touches the Dockerfile, sysadmins-infra, and needs full end-to-end validation. PR #486 exists to explore that path, but it is exploration, not a pending migration.


Please test locally before requesting re-review

Once the build target is fixed, please verify:

  1. The container that starts is bewelcome_php_dev — check with php -m | grep xdebug inside the container
  2. The app loads at http://localhost:80 and the DB seeds via postCreateCommand

For WSL: I can test that side as well.

@Ramjivan

Ramjivan commented Sep 1, 2026

Copy link
Copy Markdown
Author

@Neophytis I've addressed the feedback from your latest review and spent some time heavily debugging the DevContainer environment to ensure it's actually usable.

Code Review Fixes:

  1. Build Target: Added target: bewelcome_php_dev to the php service in docker-compose.override.yml.dist.
  2. UTF-8 BOM: Stripped the hidden BOM from docker/php/conf.d/bewelcome.xdebug.ini.
  3. Duplicate ENV: Removed the duplicate SYMFONY_TRUSTED_PROXIES variable from .env.
  4. DevContainer Cleanup: Removed the postCreateCommand from .devcontainer/devcontainer.json that was trying to extract languages.sql.bz2 (these files don't exist on this branch, causing the post-create step to instantly fail).

1. Fixing the Container Crash-Loop (DB Bug Tolerance)
During testing, I found that the known rightsvolunteers FK constraint bug was actually making the DevContainer completely unusable. Because docker/php/docker-entrypoint.sh runs with set -e, the test:database:create exception was causing the entire php container to exit with code 1. If the container dies, VS Code drops the connection.

  • Fix: I wrapped the database creation and import commands in the entrypoint with || true. This ensures the container survives the schema error and stays alive, allowing developers to successfully attach to the DevContainer to fix the bug.

2. Alpine Linux vs. IDE Remote Servers
We discovered that IDE remote server injection scripts (like VS Code's) fail silently or catastrophically on our lightweight Alpine image because they expect standard GNU utilities.

  • Fix: I updated the bewelcome_php_dev Dockerfile stage to explicitly install several critical dependencies:
    • libstdc++ & gcompat: Required for the remote server binaries to execute.
    • bash: Expected by the injection scripts.
    • wget (GNU): The default Alpine busybox wget fails SSL/SNI checks when the IDE tries to download the server tarball.
    • procps (GNU): The default Alpine busybox ps does not support the -A or -p flags. Without this, the IDE script fails to detect if the server is running and aborts.
    • nodejs: Native Alpine node.

3. Antigravity IDE Compatibility & Workaround
While standard VS Code correctly detects Alpine and downloads a linux-alpine backend, Antigravity IDE currently has an upstream bug where it blindly downloads a linux-x64 glibc-compiled Node backend. This instantly crashes on Alpine's musl-libc (missing fcntl64), causing the connection to fail.

We reverted the hacks to keep the Dockerfile clean, but if anyone must use Antigravity IDE on this branch, here is the manual workaround we found to bypass the bug:

  1. Let Antigravity attempt to connect (it will fail and download the broken binary).
  2. Exec into the running container and replace their broken Node binary with the native Alpine one: ln -sf /usr/bin/node /root/.antigravity-ide-server/bin/*/bin/node
  3. Create a fake ldd script at /usr/local/bin/ldd that simply echoes ldd (GNU libc) 2.28. (Antigravity's setup script runs ldd --version, and Alpine's native ldd outputs warnings to stderr, which causes Antigravity's strict mode to falsely assume a critical failure and drop the connection).

Ready for the next round of peer reviews!

@Ramjivan

Ramjivan commented Sep 1, 2026

Copy link
Copy Markdown
Author

As a separate discussion point for the team:

The .devcontainer/devcontainer.json is currently pre-configured to automatically install the following VS Code extensions to provide a complete development environment out-of-the-box:

  • PHP Intelephense (bmewburn.vscode-intelephense-client)
  • PHP Debug (xdebug.php-debug)
  • Symfony for VS Code (symfony-vscode.symfony-vscode)

Since we explicitly added native Alpine nodejs to the DevContainer image, PHP Intelephense boots up perfectly without any Alpine-related crashes.

Are there any other plugins the team strongly recommends pre-installing for this repository to ensure a fully complete dev experience? Let me know your thoughts!

@Ramjivan

Ramjivan commented Sep 1, 2026

Copy link
Copy Markdown
Author

@Neophytis I wanted to propose an alternative approach for our DevContainer setup that might elegantly solve the tension between "DevContainers must mirror production exactly" and "Developers need standard tooling for their IDEs to work."

Right now, we are forcing developer tools into our php:8.3-fpm-alpine image to make IDE remote servers (like VS Code or Antigravity IDE) work. Alpine's musl-libc architecture fundamentally breaks most C++ IDE backend binaries (which expect GNU glibc). We currently have to install gcompat, GNU wget, GNU bash, and manual nodejs overrides just to keep the IDEs from crashing. This pollutes our Alpine image and turns it into a bloated pseudo-Debian environment, defeating the purpose of mirroring production.

An Alternative Approach: The Workspace Container

Instead of injecting the IDE directly into our Alpine php container, we could introduce a dedicated workspace container purely for the developer tooling, while keeping our Alpine containers exactly as they are in production.

How it works:

  1. The Runtime (Untouched): Our php service remains 100% untouched. It still runs Alpine, it still mirrors production exactly, and it still handles all the actual HTTP requests. We can strip all the gcompat and IDE hacks out of it.
  2. The Workspace (Debian): We add a lightweight workspace container (e.g., mcr.microsoft.com/devcontainers/php:8.3-bookworm) to our Docker Compose setup. We configure .devcontainer/devcontainer.json to attach only to this Debian container.
  3. The Shared Code: Both the Debian workspace and the Alpine php container mount the same /srv/bewelcome source code volume.

The Result:
When a developer opens the project, VS Code connects flawlessly to the Debian container. They get a full Linux environment with standard bash, git, Composer, and perfect IDE plugin compatibility. When they save a file or run composer install in the VS Code terminal, it saves to the shared volume. The Alpine php container (which is running the web server) instantly sees those changes and serves them.

This gives us the best of both worlds: a pristine, production-matching Alpine runtime, and a bulletproof, standard Debian developer experience.

Real-World Context & Documentation

In the wider PHP ecosystem, local development tools like DDEV and Lando solve this exact IDE compatibility problem by simply abandoning Alpine and forcing everything to run on heavy Debian/Ubuntu containers.

  • DDEV explicitly builds their core ddev-webserver image on Debian. (Ref: [DDEV Dockerfile - \FROM debian:trixie-slim](https://github.com/ddev/ddev/blob/master/containers/ddev-webserver/Dockerfile)).
  • Lando relies on the standard Debian-based php:fpm images for their default PHP recipes, avoiding alpine tags. (Ref: Lando PHP Images).

By using the multi-container approach instead of copying DDEV/Lando, we can provide that same robust Debian developer experience without sacrificing our pure Alpine production containers.

This multi-container setup is officially documented and supported by the DevContainer specification for scenarios exactly like ours:

What do you think about exploring this for the DevContainer?

@Neophytis Neophytis left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review summary

Tested the devcontainer stack locally — images build cleanly and PHP-FPM boots successfully. The core setup is solid, but there are several issues that prevent it from actually working out of the box for a developer. See inline comments for details.

Must-fix before merge:

  1. Port mappings missing — app and MailPit not reachable in browser
  2. MailPit port mismatch (configured on 80, devcontainer expects 1080)
  3. No step to download/decompress the SQL seed files (languages + words)
  4. || true on SQL imports silently swallows real database errors

Should fix:
5. intelephense.environment.phpVersion set to 8.4.0 but Dockerfile uses PHP 8.3
6. consensus/behat-mailpit-extension: dev-main — floating version + GPL-3.0 licence conflict with the rest of the project (MIT/Apache)
7. App\Form\DataTransformer\ service registration is redundant and out of scope

Open question: The PR description mentions targeting develop — but this PR targets master. Which is correct?

Comment thread docker-compose.yml
image: axllent/mailpit
environment:
MP_SMTP_BIND_ADDR: "0.0.0.0:25"
MP_UI_BIND_ADDR: "0.0.0.0:80"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MailPit's web UI is bound to port 80 here, but devcontainer.json lists port 1080 as "MailPit Web UI". These disagree — a developer hitting localhost:1080 will get nothing.

Either change this to MP_UI_BIND_ADDR: "0.0.0.0:1080", or keep port 80 internally and map host 1080 → container 80 in docker-compose.override.yml.dist (see that file's comment).

services:
php:
build:
target: bewelcome_php_dev

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Neither web (nginx) nor mailer (MailPit) exposes ports to the host. VS Code's forwardPorts forwards from inside the primary service (php), which doesn't listen on 80 or 1080, so both services are unreachable in the browser locally. Codespaces auto-scans all containers and handles this more gracefully, but local VS Code DevContainers will not forward these ports without explicit mappings.

Please add:

  web:
    ports:
      - "80:80"
  mailer:
    ports:
      - "1080:80"

"service": "php",
"workspaceFolder": "/srv/bewelcome",
"overrideCommand": false,
"forwardPorts": [80, 1080, 9306, 9308],

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The seed SQL files (languages.sql, words.sql) are not in the repo — they are downloaded from downloads.bewelcome.org and decompressed by make install. A fresh devcontainer skips this entirely, so the imports are silently skipped (if [ -f ... ] guard passes quietly) and the language/words tables end up empty, breaking locale switching and member language preferences.

A postCreateCommand is needed, for example:

"postCreateCommand": "curl https://downloads.bewelcome.org/for_developers/rox_test_db/languages.sql.bz2 -o docker/db/languages.sql.bz2 && curl https://downloads.bewelcome.org/for_developers/rox_test_db/words.sql.bz2 -o docker/db/words.sql.bz2 && bunzip2 --force docker/db/languages.sql.bz2 docker/db/words.sql.bz2"

Comment thread .devcontainer/devcontainer.json Outdated
"[php]": {
"editor.defaultFormatter": "bmewburn.vscode-intelephense-client"
},
"intelephense.environment.phpVersion": "8.4.0",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Dockerfile uses FROM php:8.3.33-fpm-alpine3.24. Setting Intelephense to 8.4.0 means PHP 8.4-only syntax won't be flagged as errors in the editor, hiding real incompatibilities. Change to "8.3.0".

Comment thread docker/php/docker-entrypoint.sh Outdated

if [ -f docker/db/languages.sql ]; then
mysql $database_name -u $database_user -p$database_password -h $database_host < docker/db/languages.sql
mysql $database_name -u $database_user -p$database_password -h $database_host < docker/db/languages.sql || true

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The if [ -f ... ] guard already handles the missing-file case. Adding || true on the mysql imports means actual MariaDB errors (wrong schema, bad SQL, connection issues) are silently swallowed — a developer sees a clean boot but ends up with an empty or corrupt database with no indication of why.

At minimum, log before suppressing:

mysql ... < docker/db/languages.sql || echo 'WARNING: failed to import languages.sql'

Same applies to the words.sql and geonamesadminunits.sql lines.

Comment thread composer.json Outdated
"alexandresalome/mailcatcher": "^1.3",
"behat/behat": "^3.7",
"behatch/contexts": "^3.3",
"consensus/behat-mailpit-extension": "dev-main",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two concerns:

  1. Floating version: dev-main means composer update will pull whatever is on the extension's main branch at that moment. The lockfile pins commit 89d3f16e today, but this is fragile long-term. Pin to a stable tag or commit reference.

  2. Licence conflict: consensus/behat-mailpit-extension is GPL-3.0-or-later (copyleft), which conflicts with the rest of the project (MIT/Apache). Please check the licensing implications before merging — consider using a version-pinned fork or a differently-licenced alternative.

Comment thread config/services.yaml

# makes classes in src/ available to be used as services
# this creates a service per class whose id is the fully-qualified class name
App\Form\DataTransformer\:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

App\Form\DataTransformer\ is already covered by the App\: catch-all below — DataTransformer is not in the exclude list, so this explicit registration is a no-op. It is also unrelated to the devcontainer work. If it fixes a real bug, it deserves its own PR with an explanation.

Move curl/bunzip2 download of languages.sql and words.sql into
docker-entrypoint.sh so the files are fetched before the import guards
run on first boot. The previous postCreateCommand approach downloaded
the files after container startup had already completed, meaning the
imports were always silently skipped on first DevContainer creation.
@Neophytis

Copy link
Copy Markdown
Contributor

Great progress Ram — the port mappings, SQL error handling, and Intelephense version are all fixed. I tested locally: both the app (port 80) and MailPit (port 1080) are now reachable in the browser. Really close now, just a few small things left to tidy up:

1. Seed file download: change https:// to http://

The download still fails on first boot, but it's not your code — downloads.bewelcome.org is serving the wrong TLS certificate (tracking issue: BeWelcome/sysadmins-infra#340). The files exist and are fine over plain HTTP. A one-line fix in docker-entrypoint.sh:

# Change this in both curl calls:
curl -sL https://downloads.bewelcome.org/...
# to:
curl -sL http://downloads.bewelcome.org/...

2. behat.yml.dist references the removed package

It looks like you removed consensus/behat-mailpit-extension from composer.json (responding to the licence concern — fair call), but behat.yml.dist still references Consensus\Behat\MailpitExtension. That makes vendor/bin/behat crash immediately with a class-not-found error.

The good news: the 3 existing feature tests (auth, members/get, members/list) don't actually use any email assertions, so you don't need the extension at all right now. The simplest fix is to remove the two Mailpit lines from behat.yml.dist:

# Remove these:
- 'Consensus\Behat\MailpitExtension\Context\MailpitContext': ~
# and:
'Consensus\Behat\MailpitExtension':
    base_url: http://mailer:80
# (and the localhost equivalent)

When someone writes a behat test that actually checks email, they can add back a properly-licensed extension at that point.

3. Two out-of-scope changes to clean up

  • config/services.yaml: the App\Form\DataTransformer\ registration is already covered by the App\: catch-all — it's a no-op. Removing it keeps the PR focused.
  • SYMFONY_TRUSTED_PROXIES in .env: this is a good fix, but .env is committed to the repo and affects all environments (not just the devcontainer). Worth landing in its own small PR so it gets the visibility it deserves.

Overall the devcontainer setup is solid — once these are addressed this should be good to merge!

@Ramjivan

Ramjivan commented Sep 1, 2026

Copy link
Copy Markdown
Author

Hi @Neophytis, thanks for testing and for the quick feedback!

All points are now addressed on feat/devcontainer-issue-69:

  1. Seed File URLs: Switched to http://downloads.bewelcome.org/... in docker-entrypoint.sh to avoid the SSL CN mismatch issue.
  2. Behat Configuration: Removed the dangling Consensus\Behat\MailpitExtension references from behat.yml.dist.
  3. .env Cleanup: Removed SYMFONY_TRUSTED_PROXIES from .env so it can land in its own separate PR.
  4. App\Form\DataTransformer\ in services.yaml: I tested removing this registration, but Symfony's container compilation immediately fails with:
    Cannot autowire service "App\Form\SearchLocationType": argument "$transformer" of method "__construct()" references class "App\Form\DataTransformer\LocationRequestToLocationTransformer" but no such service exists.
    
    Because SearchLocationType directly type-hints the concrete class in its constructor without an interface or explicit service alias, Symfony requires this namespace mapping to autowire it. I've kept this registration in place so container builds and cache:clear succeed cleanly.

Ready for review!

@Neophytis

Copy link
Copy Markdown
Contributor

Pulled and retested — really close, one new blocker crept in and two minor cleanups left.

Blocker: removing SYMFONY_TRUSTED_PROXIES from .env breaks the boot

config/packages/framework.yaml references %env(SYMFONY_TRUSTED_PROXIES)% with no default. Removing it from .env causes Symfony to throw a fatal error when starting the container (Environment variable not found: "SYMFONY_TRUSTED_PROXIES"), which aborts DB creation and skips all the SQL imports. The app technically comes up but with an empty database.

This isn't your fault — you responded correctly to the review feedback, but the framework.yaml dependency was already baked in by an earlier commit in the branch. The right fix is to make the variable optional in framework.yaml:

# config/packages/framework.yaml
trusted_proxies: '%env(default::SYMFONY_TRUSTED_PROXIES)%'

With default::, Symfony silently uses an empty string when the var isn't set, so the app boots cleanly in any environment where it isn't defined. Then SYMFONY_TRUSTED_PROXIES can be set properly in each deployment environment without being required in .env.

Minor: remove wget from the Dockerfile dev stage

wget was added to the bewelcome_php_dev apk install list but is never used — all downloads in the entrypoint use curl. One line to drop, saves a few MB.

nodejs on the other hand IS needed — the entrypoint runs yarn encore dev at startup, which requires Node. Keep that.

Minor: xdebug.remote_enable: true in devcontainer.json is a dead setting

This is an Xdebug 2 VS Code key — Xdebug 3 ignores it entirely. The bewelcome.xdebug.ini already handles everything correctly. Safe to remove from the VS Code settings block.


One point from another review I want to flag as incorrect: intelephense.environment.phpVersion does NOT need changing — 8.3.0 is right. The Dockerfile uses FROM php:8.3.33-fpm-alpine3.24. Ignore any suggestion to downgrade it to 8.2.0.

Once those three things are done this is good to go — great work getting it this far!

@Neophytis

Copy link
Copy Markdown
Contributor

Almost there Ram! Three small things left:

1. One-word fix in config/packages/framework.yaml (blocker)

The app fails to boot without SYMFONY_TRUSTED_PROXIES defined. Change line 22 from:

trusted_proxies: '%env(SYMFONY_TRUSTED_PROXIES)%'

to:

trusted_proxies: '%env(default::SYMFONY_TRUSTED_PROXIES)%'

Same for trusted_headers on the next line. The default:: processor makes both vars optional — the app boots fine whether or not any environment defines them.

2. Remove wget from Dockerfile dev stage (2 lines)

wget is never used — all downloads go through curl. nodejs stays, it's needed for yarn encore dev.

3. Remove xdebug.remote_enable: true from devcontainer.json

Dead Xdebug 2 setting, ignored by Xdebug 3. The .ini file already handles everything.

That's it — nothing else needed!

@Ramjivan

Ramjivan commented Sep 1, 2026

Copy link
Copy Markdown
Author

Hi @Neophytis,

All 3 points have been addressed and pushed to feat/devcontainer-issue-69:

  1. Optional trusted_proxies: Updated config/packages/framework.yaml to use %env(string:default::SYMFONY_TRUSTED_PROXIES)%. Verified that bin/console cache:clear and the app boot cleanly without requiring the env variable.
  2. Dockerfile Cleanup: Removed wget from the bewelcome_php_dev stage.
  3. DevContainer Settings: Removed the obsolete xdebug.remote_enable setting from devcontainer.json.

Everything is in place and verified locally!

"editor.defaultFormatter": "esbenp.prettier-vscode",
"[php]": {
"editor.defaultFormatter": "bmewburn.vscode-intelephense-client"
},

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor: phpVersion should be "8.2.0" to match the actual PHP version in the Docker image (the Dockerfile builds on PHP 8.2). Setting it to 8.3.0 will make Intelephense flag valid 8.2 code as errors.

"service": "php",
"workspaceFolder": "/srv/bewelcome",
"overrideCommand": false,
"forwardPorts": [80, 1080, 9306, 9308],

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

macOS + Rancher Desktop note: forwardPorts causes port conflicts on macOS with Rancher Desktop (and likely other non-Docker-Desktop runtimes). Rancher exposes container ports via SSH tunnelling from its Lima VM, and VS Code's forwardPorts creates a competing localhost:PORT listener on top of that. Because the localhost-specific binding wins over the SSH tunnel's wildcard bind, connections get routed into the php container's localhost instead of the correct mailer/web container — and hang.

Workaround tested on macOS + Rancher Desktop: remove forwardPorts entirely. Ports with host mappings in the compose files are auto-detected by VS Code's Ports panel anyway; portsAttributes labels still apply. The forwardPorts array is only needed for ports that are not host-mapped in the compose file.

Works fine on WSL/Docker Desktop as-is — this is a macOS-specific Rancher Desktop behaviour. Worth adding a note to the README.

@Neophytis Neophytis left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

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

Successfully merging this pull request may close these issues.

Split requests and messages?

3 participants