- Notifications
You must be signed in to change notification settings - Fork3.1k
feat: configurable external ports by vhost#2678
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:main
Are you sure you want to change the base?
Uh oh!
There was an error while loading.Please reload this page.
Conversation
1715b9d to265ad44CompareThere was a problem hiding this 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 adds support for configurable external HTTP and HTTPS ports on a per-virtual-host basis. Previously, external ports were globally configured; now they can be overridden per vhost usingEXTERNAL_HTTP_PORT andEXTERNAL_HTTPS_PORT environment variables or via theVIRTUAL_HOST_MULTIPORTS configuration.
Key changes:
- Added per-vhost external port configuration in nginx template
- Created comprehensive test suite for single-host and multi-port scenarios
- Modified template to use vhost-specific ports in listen directives and redirects
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| nginx.tmpl | Modified to extract and store per-vhost external port configurations from environment variables, and use them in server listen directives and redirect URIs |
| test/test_external-ports/test_virtual-host.yml | Test configuration for four scenarios: custom http+https ports, custom https only, custom http only, and default ports |
| test/test_external-ports/test_virtual-host.py | Test assertions verifying correct port usage in redirects for single-host scenarios |
| test/test_external-ports/test_virtual-host-multiport.yml | Test configuration for multi-port virtual hosts with various port configurations |
| test/test_external-ports/test_virtual-host-multiport.py | Test assertions verifying correct port usage for multi-port virtual hosts including subpaths |
| test/test_external-ports/compose.base.override.yml | Docker compose override exposing the custom ports for testing |
| test/test_external-ports/certs/nginx-proxy.tld.key | SSL private key for test certificate |
| test/test_external-ports/certs/nginx-proxy.tld.crt | SSL certificate for testing HTTPS functionality |
💡Add Copilot custom instructions for smarter, more guided reviews.Learn how to get started.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Pull Request Overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated no new comments.
💡Add Copilot custom instructions for smarter, more guided reviews.Learn how to get started.
p12tic left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
LGTM
Uh oh!
There was an error while loading.Please reload this page.
https://example.org:8448=>synapse:8448https://example.org:8448/path=>synapse:80https://matrix.example.org=>synapse:8080external_http_portand / orexternal_https_portcan be defined at the virtual host level, and will be used for all of the virtual paths of this virtual host.For containers that don't use
VIRTUAL_HOST_MULTIPORTS, two new environment variables serve the same purpose :EXTERNAL_HTTP_PORTandEXTERNAL_HTTPS_PORT, ie:https://example.org:8448=>synapse:8448