Skip to content

Conversation

@myabc
Copy link
Contributor

@myabc myabc commented Jul 24, 2025

⚠️ Developers will need to manually updated their docker-compose.override.yml on merging this PR edit: not anymore

Vite now supports specifying allowed hosts as an __VITE_ADDITIONAL_SERVER_ALLOWED_HOSTS.

Alternative fix to #19687 / a1555af: Only overrides allowed hosts in the development environments that need it (i.e. Docker + TLS stack).

See: https://vite.dev/config/server-options.html#server-allowedhosts
See: vitejs/vite#19325

Vite now supports specifying allowed hosts as an
`__VITE_ADDITIONAL_SERVER_ALLOWED_HOSTS`.

Alternative fix to a1555af: Only
overrides allowed hosts in the development environments that need it
(i.e. Docker + TLS stack).

See: https://vite.dev/config/server-options.html#server-allowedhosts
See: vitejs/vite#19325
@myabc myabc changed the title Set allowed hosts via environment variable Fix running ng serve [alt]: Set allowed hosts via environment variable Jul 24, 2025
@myabc myabc requested review from a team, NobodysNightmare and Copilot July 24, 2025 10:21
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR moves the Vite development server's allowed hosts configuration from the Angular build configuration to an environment variable approach, allowing for more flexible configuration in different deployment environments.

  • Removes hardcoded allowedHosts configuration from angular.json
  • Adds environment variable __VITE_ADDITIONAL_SERVER_ALLOWED_HOSTS to Docker TLS configuration
  • Includes a minor grammar fix in documentation

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
frontend/angular.json Removes hardcoded allowedHosts configuration for development target
docs/development/development-environment/docker/README.md Corrects grammar in test documentation
docker/dev/tls/docker-compose.core-override.example.yml Adds environment variable for allowed hosts and port mappings

@myabc myabc marked this pull request as ready for review July 24, 2025 10:22
@myabc myabc changed the title Fix running ng serve [alt]: Set allowed hosts via environment variable Fix running ng serve (alt): Set allowed hosts via environment variable Jul 24, 2025
@myabc myabc changed the title Fix running ng serve (alt): Set allowed hosts via environment variable Fix running ng serve (alt): Set allowed hosts via environment variable Jul 24, 2025
@myabc myabc added javascript Pull requests that update Javascript code needs review tinyfix docker labels Jul 24, 2025

frontend:
environment:
__VITE_ADDITIONAL_SERVER_ALLOWED_HOSTS: "openproject-assets.local"
Copy link
Contributor

Choose a reason for hiding this comment

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

Assuming that it's non-breaking for other users, could we specify this on the docker-compose.yml already? Then it would work out of the box for everyone using the default hostnames. If you want to use a different hostname, the override is still possible.

It seems to be fine to have these ports settings as well as
__VITE_ADDITIONAL_SERVER_ALLOWED_HOSTS envvar set in docker-compose.yml
directly. So, there is less to override to get your docker TLS dev stack working.
@ba1ash ba1ash requested a review from NobodysNightmare July 24, 2025 12:43
@ba1ash ba1ash merged commit c437f65 into dev Jul 24, 2025
13 of 14 checks passed
@ba1ash ba1ash deleted the fix/ng-serve-docker-tls-stack branch July 24, 2025 13:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

docker javascript Pull requests that update Javascript code needs review tinyfix

Development

Successfully merging this pull request may close these issues.

4 participants