Skip to content
This repository has been archived by the owner on Jan 8, 2019. It is now read-only.

Service should use Node address if not populated #7

Open
bencyoung opened this issue Oct 8, 2018 · 4 comments
Open

Service should use Node address if not populated #7

bencyoung opened this issue Oct 8, 2018 · 4 comments
Labels
enhancement New feature or request help wanted Extra attention is needed

Comments

@bencyoung
Copy link

When a health check in Consul returns, it can have a null Address. This is because services can be registered with an empty address. The docs at https://www.consul.io/api/agent/service.html say:

Address (string: "") - Specifies the address of the service. If not provided, the agent's address is used as the address for the service during DNS queries.

If a service doesn't have an address then Ocelot should use the node address instead

@TomPallister
Copy link
Member

@bencyoung thanks for the info! Sadly I don’t have time to implement this atm happy to accept a PR for this functionality.

@TomPallister TomPallister added enhancement New feature or request help wanted Extra attention is needed good first issue Good for newcomers and removed good first issue Good for newcomers labels Oct 11, 2018
@bencyoung
Copy link
Author

I'll see what I can do!

@bencyoung
Copy link
Author

I can't easy upload a PR from where I am, but this patch I think does what's needed

diff --git a/src/Ocelot.Provider.Consul/Consul.cs b/src/Ocelot.Provider.Consul/Consul.cs
index b202cd5..6b4c42e 100644
--- a/src/Ocelot.Provider.Consul/Consul.cs
+++ b/src/Ocelot.Provider.Consul/Consul.cs
@@ -33,9 +33,15 @@
 
             foreach (var serviceEntry in queryResult.Response)
             {
-                if (IsValid(serviceEntry))
+                var address = serviceEntry.Service.Address;
+                if (string.IsNullOrEmpty(address))
                 {
-                    services.Add(BuildService(serviceEntry));
+                    address = serviceEntry.Node.Address;
+                }
+
+                if (IsValid(serviceEntry, address))
+                {
+                    services.Add(BuildService(serviceEntry, address));
                 }
                 else
                 {
@@ -46,19 +52,19 @@
             return services.ToList();
         }
 
-        private Service BuildService(ServiceEntry serviceEntry)
+        private Service BuildService(ServiceEntry serviceEntry, string address)
         {
             return new Service(
                 serviceEntry.Service.Service,
-                new ServiceHostAndPort(serviceEntry.Service.Address, serviceEntry.Service.Port),
+                new ServiceHostAndPort(address, serviceEntry.Service.Port),
                 serviceEntry.Service.ID,
                 GetVersionFromStrings(serviceEntry.Service.Tags),
                 serviceEntry.Service.Tags ?? Enumerable.Empty<string>());
         }
 
-        private bool IsValid(ServiceEntry serviceEntry)
+        private bool IsValid(ServiceEntry serviceEntry, string address)
         {
-            if (string.IsNullOrEmpty(serviceEntry.Service.Address) || serviceEntry.Service.Address.Contains("http://") || serviceEntry.Service.Address.Contains("https://") || serviceEntry.Service.Port <= 0)
+            if (string.IsNullOrEmpty(address) || address.Contains("http://") || address.Contains("https://") || serviceEntry.Service.Port <= 0)
             {
                 return false;
             }

@TomPallister
Copy link
Member

I will take a look when I get a chance!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

2 participants