-
Notifications
You must be signed in to change notification settings - Fork 228
Change "127.0.0.1" to SIDECAR_IP to dynamically account for IPv4 vs. IPv6 environments #649
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
Changes from all commits
5e792b1
09d2bc5
0171346
d6c18c0
4aa8aba
95f3728
cdb5ba8
943596e
b4874d4
d23894d
be5b2a9
f4363d6
ff7c074
aed37ac
3ffc34d
5ff9e1d
d2ba277
0504596
5990bb8
6c09c69
fea50e6
5f75481
667efd3
ab3f874
78da63f
ade335f
a0a353b
d10b3d0
77f56f4
a5b9e4e
3917e70
08ab9dd
8fce9bd
29bf6a6
59dca39
52821d8
e391e95
424e70a
3797e31
e8ddb5c
cbc11cb
904387b
3b39e86
9c82edb
447edb1
cbf4f47
9543fcd
d9fd2c0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,6 +4,7 @@ | |
| .run/ | ||
| .vs/ | ||
| .vscode/ | ||
| .devcontainer/ | ||
|
|
||
| # Output folders | ||
| **/target/ | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -19,9 +19,11 @@ | |
| import io.grpc.ManagedChannelBuilder; | ||
|
|
||
| import java.io.IOException; | ||
| import java.net.InetAddress; | ||
| import java.net.InetSocketAddress; | ||
| import java.net.Socket; | ||
| import java.net.URI; | ||
| import java.net.UnknownHostException; | ||
|
|
||
| /** | ||
| * Utility methods for network, internal to Dapr SDK. | ||
|
|
@@ -112,4 +114,12 @@ private static void callWithRetry(Runnable function, long retryTimeoutMillisecon | |
| Thread.sleep(Math.min(remaining, RETRY_WAIT_MILLISECONDS)); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Retrieve loopback address for the host. | ||
| * @return The loopback address String | ||
| */ | ||
| public static String getHostLoopbackAddress() { | ||
| return InetAddress.getLoopbackAddress().getHostAddress(); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can this method automatically return the Ipv6 between "[]"?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @artursouza Sorry for the delay, finally had some time to look into this. I changed some unit tests locally to validate IPv6 sidecar IP constants contained within "[]" and it seems like your suggestion is viable - for both URL endpoint invocation formats and Dapr client instantiation. Great recommendation! However, before I proceed, I need to validate an implementation detail with you. To test for whether an address is IPv6, it would be convenient to leverage the Apache Commons Validator, though you have recommended against bringing in 3P libraries beyond the test scope. Is it possible to get an exception for this case, or do I have to write a custom IPv6 validator within NetworkUtils?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @willtsai Is it complex to write a validator for IPv6 ? both standard pattern and hex compressed pattern along with IPv4 validation?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Understood - should not be too complex but I wanted to see if the work (and subsequent unit tests) could be avoided. Seems like adding a new dependency is not worth it here, so I'll work on the custom validator.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I started the implementation for this and have a working validator, but then realized that this would introduce inconsistencies into the Thus, I think there are two options:
@artursouza @mukundansundar - what are your thoughts here? which option would you prefer? (I personally think option 1 introduces less surprises to the client who may not be expecting IPv6 address to be pre-formatted for http URLs) |
||
| } | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.