ThreeMammals / Ocelot.Provider.Consul

Repo for Consul integration with Ocelot
http://threemammals.com/ocelot
MIT License
18 stars 14 forks source link

Service should use Node address if not populated #7

Open bencyoung opened 6 years ago

bencyoung commented 6 years ago

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 commented 6 years ago

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

bencyoung commented 6 years ago

I'll see what I can do!

bencyoung commented 6 years ago

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 commented 6 years ago

I will take a look when I get a chance!