Ocelot.Provider.Consul icon indicating copy to clipboard operation
Ocelot.Provider.Consul copied to clipboard

Service should use Node address if not populated

Open bencyoung opened this issue 6 years ago • 4 comments

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

bencyoung avatar Oct 08 '18 12:10 bencyoung

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

TomPallister avatar Oct 11 '18 06:10 TomPallister

I'll see what I can do!

bencyoung avatar Oct 11 '18 06:10 bencyoung

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;
             }

bencyoung avatar Oct 11 '18 10:10 bencyoung

I will take a look when I get a chance!

TomPallister avatar Oct 13 '18 10:10 TomPallister