- Notifications
You must be signed in to change notification settings - Fork923
fix: allow ports in wildcard url configuration#11657
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
Uh oh!
There was an error while loading.Please reload this page.
Conversation
Emyrk commentedJan 17, 2024 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
This stack of pull requests is managed by Graphite.Learn more about stacking. |
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.
Needs tests for actual serving too, and the frontend should be checked to ensure it functions with this
@deansheather I checked the frontend. I'll add some tests. |
This just forwards the port to the ui that generates urls.Our existing parsing + regex already supported ports forsubdomain app requests.
a673da8
toed4410c
Compare
Uh oh!
There was an error while loading.Please reload this page.
This just forwards the port to the ui that generates urls.
Our existing parsing + regex already supported ports for
subdomain app requests.
Closes#8189
Tested with subdomain apps and port forwarding.
Remanent of the past
We have this old assumption that apphost and accessurl are on the same port.
coder/coderd/workspaceapps.go
Lines 35 to 38 in2b4ea6d
This might now not be the case. For backwards compatibility, we have to keep this here, but it could be a problem if the access url has a port, and the wildcard domain does not.
Workspace proxies?
Workspace proxies do not have this "port inheritance" thing going on. I think the port inheritance is a mistake, but for backwards compatibility, it has to be kept on the primary.