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

Replace Dns.GetHostEntry, it might not return the original IP address #728

Merged

Conversation

Arkatufus
Copy link
Contributor

@Arkatufus Arkatufus commented Aug 4, 2022

On some platforms, having the exact IP address is crucial (eg. in Azure App Services), need to preserve the original IP address being passed into the settings.

@Arkatufus Arkatufus changed the title Replace Dns.GetHostentry, it might not return the original IP address Replace Dns.GetHostEntry, it might not return the original IP address Aug 5, 2022
Copy link
Member

@Aaronontheweb Aaronontheweb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

!Equals(i, IPAddress.IPv6Loopback));
if (IPAddress.TryParse(_settings.HostName, out _address))
{
if (_address.Equals(IPAddress.Any) || _address.Equals(IPAddress.IPv6Any))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

else
{
_host = _settings.HostName;
_address = Dns.GetHostAddresses(_host)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to add a test for when:

  1. Hostname is valid for routing (externally)
  2. Hostname is not assigned to anything locally via DHCP

This is an extremely common scenario in cloud environments outside of K8s since much of the network address translation happens at the software level above the VM. It's why we have the public-hostname and public-port settings in Akka.Remote.

I would not worry about this right now though - save it for integration testing with Azure App Service.

@Aaronontheweb Aaronontheweb merged commit 0836f2b into akkadotnet:dev Aug 8, 2022
@Aaronontheweb Aaronontheweb added bug Something isn't working akka-discovery azure labels Aug 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
akka-discovery azure bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants