feat: add devcontainers for node and playwright #825
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Carlos Feria <[email protected]>
Contributor
Reviewer's GuideThis PR introduces two dedicated DevContainer workflows—one for Node.js-based UI development and another for Playwright-based end-to-end testing—updates the development README with streamlined instructions, and removes outdated template scripts. Flow diagram for devcontainer selection and environment setupflowchart TD
A["User opens repository in VSCode"] --> B{"Select DevContainer"}
B -->|"Node"| C["Start Node DevContainer"]
B -->|"Playwright"| D["Start Playwright DevContainer"]
C --> E["NodeJS 22 environment for UI development"]
D --> F["Playwright environment for E2E tests (host network)"]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey there - I've reviewed your changes - here's some feedback:
- The node Dockerfile only inherits from Ubuntu and doesn’t install Node.js or required dev tools—either add the Node.js installation steps or base it on an official Node image.
- Both docker-compose.yml files are missing a version/schema declaration, which could cause compatibility issues—add a
version: '3.x'or Compose field at the top. - You’re duplicating a lot of configuration across two devcontainer folders; consider using devcontainer.json profiles in a single setup to centralize shared settings.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The node Dockerfile only inherits from Ubuntu and doesn’t install Node.js or required dev tools—either add the Node.js installation steps or base it on an official Node image.
- Both docker-compose.yml files are missing a version/schema declaration, which could cause compatibility issues—add a `version: '3.x'` or Compose field at the top.
- You’re duplicating a lot of configuration across two devcontainer folders; consider using devcontainer.json profiles in a single setup to centralize shared settings.
## Individual Comments
### Comment 1
<location> `.devcontainer/node/docker-compose.yml:16` </location>
<code_context>
+ environment:
+ TRUSTIFY_API_URL: http://trustify:8080
+ OIDC_SERVER_URL: http://localhost:9090/realms/trustify
+ command: tail -f /dev/null
+ volumes:
+ - ../..:/workspace:cached
</code_context>
<issue_to_address>
**suggestion:** Consider replacing 'tail -f /dev/null' with a more meaningful default command.
This approach may hide startup issues. Prefer setting the default command to launch the UI service, or clearly document why 'tail -f /dev/null' is used.
Suggested implementation:
```
# The default command is set to start the UI service.
# Replace 'npm start' with the actual start command if different.
command: npm start
```
If the actual UI start command is not `npm start`, replace it with the correct command (e.g., `yarn start`, `pnpm dev`, or the entrypoint script for your UI).
If you must keep `tail -f /dev/null` for development reasons, add a comment above it explaining why, e.g.:
```
# 'tail -f /dev/null' is used to keep the container running for debugging purposes.
# Replace with the actual UI start command for production or normal development.
command: tail -f /dev/null
```
</issue_to_address>
### Comment 2
<location> `.devcontainer/playwright/docker-compose.yml:9` </location>
<code_context>
+ security_opt:
+ - "label=disable"
+ userns_mode: "keep-id"
+ network_mode: host
+ volumes:
+ - ../..:/workspace:cached
</code_context>
<issue_to_address>
**🚨 suggestion (security):** Using 'network_mode: host' may have security and portability implications.
If host networking is necessary for Playwright tests, please document the rationale and any associated security risks.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Signed-off-by: Carlos Feria <[email protected]>
Signed-off-by: Carlos Feria <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR allows the existence of 2 different devcontainers:
node: used for normal development of the UI as it has NodeJS 22 installed and also includes VSCode extensions ready for them to be used in daily UI developmentplaywright: used for executing the e2e tests. We need a separate devcontainer because it needs to run in the host network due to the fact that the oidc urls need to be "localhost" relatedMore or less depends on guacsec/trustify#2096 as we need also a backend dev environment for the UI dev environment to work.
Summary by Sourcery
Introduce dedicated devcontainers for Node-based UI development and Playwright-based end-to-end testing, update documentation and clean up legacy scripts.
New Features:
Documentation:
Chores: