Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Bug]: Breaking change related to CONSUL_HTTP_ADDR parsing #394

Closed
orange-puff opened this issue Nov 6, 2024 · 2 comments
Closed

[Bug]: Breaking change related to CONSUL_HTTP_ADDR parsing #394

orange-puff opened this issue Nov 6, 2024 · 2 comments
Labels
bug Something isn't working

Comments

@orange-puff
Copy link

orange-puff commented Nov 6, 2024

Describe the bug

Before version v1.7.14.4 which was introduced with this commit ef310c2

If CONSUL_HTTP_ADDR was set to just an IP address, the default port of 8500 would be used
This is the commit right before the breaking commit to Client.cs

private void ConfigureFromEnvironment(UriBuilder consulAddress)

shows this.

Now, if CONSUL_HTTP_ADDR is set to just an IP address, it will be parsed using the dotnet Uri class which will resolve no port to the default port 80

The obvious solution from the consumer side is to change this environment variable. But, as I was looking into how many downstream things may break, I was wondering if we could introduce some mechanism to preserve the old behavior. Perhaps an environment variable (I do not care what the name is)
CONSULDOTNET_HTTP_PORT, which when specified, can override the uri parsed port.

Steps To Reproduce

  1. git checkout f6a8038
  2. run a test where CONSUL_HTTP_ADDR=127.0.0.1
  3. See port resolve to 8500
  4. git checkout ef310c2
  5. run the same test
  6. See port resolve to 80

Expected behavior

default port 8500 is used when not specified in the CONSUL_HTTP_ADDR environment variable

Environment

  • OS:
  • Consul Version:
  • consultdotnet Version

Logs

No response

Additional context

No response

@orange-puff orange-puff added the bug Something isn't working label Nov 6, 2024
@larrytamnjong
Copy link
Contributor

Thanks for reporting this issue, @orange-puff! I'm currently looking into it.

larrytamnjong added a commit to larrytamnjong/consuldotnet that referenced this issue Nov 12, 2024
larrytamnjong added a commit to larrytamnjong/consuldotnet that referenced this issue Nov 14, 2024
larrytamnjong added a commit to larrytamnjong/consuldotnet that referenced this issue Nov 14, 2024
larrytamnjong added a commit to larrytamnjong/consuldotnet that referenced this issue Nov 14, 2024
@marcin-krystianc
Copy link
Contributor

@orange-puff thanks for reporting the problem. We've published a version v1.7.14.5 with a fix. The upgrade process to the fixed version doesn't require any extra actions or steps apart from bumping the version.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants