Skip to content
This repository has been archived by the owner on Nov 20, 2023. It is now read-only.

Respect configured host in bindings #1268

Merged
merged 3 commits into from
Jan 18, 2022
Merged

Conversation

veloek
Copy link
Contributor

@veloek veloek commented Jan 14, 2022

When setting ASPNETCORE_URLS env variable in ProcessRunner. This makes it possible to override which interface to bind to and also allow connections from anywhere by using a wildcard.

This is only tested for services configured as projects. Not sure how this affects other aspects of Tye.

Fixes #721

When setting ASPNETCORE_URLS env variable in ProcessRunner. This makes
it possible to override which interface to bind to and also allow
connections from anywhere by using a wildcard.
@philliphoff
Copy link
Contributor

@veloek In general, looks good. The only thing I've noticed in testing is that, while tye run will now respect the specified host, tye run --docker will not. That is, the container port will still be mapped to all IP addresses of the host (which is the Docker default for mapped ports). It appears this happens for similar reasons, where DockerRunner.cs ignores the Host property of the binding.

@veloek
Copy link
Contributor Author

veloek commented Jan 14, 2022

@philliphoff, good catch! Didn't try running my services in Docker containers, but it makes sense to fix that as well while at it 👍

@philliphoff
Copy link
Contributor

@veloek Take a look at my update to the PR and see if anything looks amiss.

@ravipal Since I've made changes, can you do a quick review as well?

@veloek
Copy link
Contributor Author

veloek commented Jan 14, 2022

@philliphoff LGTM. If I were to nitpick, I see you use capital S in String which seems inconsistent with the rest of the code.

@philliphoff
Copy link
Contributor

Ah, the hazards of owning codebases with entirely different style guides. :-)

@philliphoff philliphoff merged commit bd63c2b into dotnet:main Jan 18, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Services don't reflect the host setting
3 participants