fix(kuma-cp): multiple external services pointing to same address

api/mesh/v1alpha1/externalservice.proto

@@ -57,6 +57,11 @@ message ExternalService {
     }
 
     TLS tls = 2;
+
+    // If disableHostDNSEntry is set to true then a DNS entry for the external
+    // service (in the form of host:port, where host is taken from
+    // networking.address field) won't be generated
+    bool disableHostDNSEntry = 3;
   }
 
   Networking networking = 1 [ (doc.required) = true ];

docs/generated/resources/policy_external-service.md

@@ -34,7 +34,13 @@
         - `serverName` (optional)
 
             ServerName overrides the default Server Name Indicator set by Kuma.
-            The default value is set to "address" specified in "networking".
+            The default value is set to "address" specified in "networking".    
+
+    - `disableHostDNSEntry` (optional)
+
+        If disableHostDNSEntry is set to true then a DNS entry for the external
+        service (in the form of host:port, where host is taken from
+        networking.address field) won't be generated
 
 - `tags` (required)
 

pkg/dns/vips/interfaces.go

@@ -26,7 +26,7 @@ func (t EntryType) String() string {
 }
 
 // HostnameEntry is the definition of a DNS entry. The type indicates where the entry comes from
-// (.e.g: Service is auto-generated, FullyQualifiedDomain comes from `virtual-outbound` policies...)
+// (.e.g: Service and Host are auto-generated, FullyQualifiedDomain comes from `virtual-outbound` policies...)
 type HostnameEntry struct {
 	Type EntryType `json:"type"`
 	Name string    `json:"name"`

pkg/dns/vips/virtual_outbound.go

@@ -26,12 +26,12 @@ func (vo *VirtualOutbound) Equal(other *VirtualOutbound) bool {
 }
 
 const (
-	OriginHost    = "host"
 	OriginService = "service"
 	OriginKube    = "kubernetes"
 )
 
 var OriginVirtualOutbound = func(name string) string { return "virtual-outbound:" + name }
+var OriginHost = func(name string) string { return "external-service:" + name }
 
 type OutboundEntry struct {
 	Port   uint32

pkg/dns/vips/virtual_outbound_view.go

@@ -46,7 +46,11 @@ func (vo *VirtualOutboundMeshView) Add(entry HostnameEntry, outbound OutboundEnt
 			if existingOutbound.String() == outbound.String() {
 				return nil
 			}
-			return fmt.Errorf("can't add %s:%d from %s because it's already used by entity defined in:'%s'", entry.Name, outbound.Port, outbound.Origin, existingOutbound.Origin)
+			if entry.Type == Host {
+				return fmt.Errorf("autogenerated DNS entry %s:%d conflicts with existing entry, this happens when there are two external services with the same 'networking.address', to disable automatic DNS generation in external services set 'networking.disableHostDNSEntry=true' in at least one of these externalService", entry.Name, outbound.Port)
+			} else {
+				return fmt.Errorf("can't add %s:%d from %s because it's already used by entity defined in:'%s'", entry.Name, outbound.Port, outbound.Origin, existingOutbound.Origin)
+			}
 		}
 	}
 	vo.byHostname[entry].Outbounds = append(vo.byHostname[entry].Outbounds, outbound)

pkg/dns/vips_allocator.go

@@ -6,6 +6,7 @@ import (
 	"net"
 	"strings"
 
+	"github.com/pkg/errors"
 	"go.uber.org/multierr"
 
 	mesh_proto "github.com/kumahq/kuma/api/mesh/v1alpha1"
@@ -230,11 +231,16 @@ func (d *VIPsAllocator) BuildVirtualOutboundMeshView(ctx context.Context, mesh s
 		if d.serviceVipEnabled {
 			errs = multierr.Append(errs, addDefault(outboundSet, es.Spec.GetService(), es.Spec.GetPortUInt32()))
 		}
-		errs = multierr.Append(errs, outboundSet.Add(vips.NewHostEntry(es.Spec.GetHost()), vips.OutboundEntry{
-			Port:   es.Spec.GetPortUInt32(),
-			TagSet: tags,
-			Origin: vips.OriginHost,
-		}))
+		if !es.Spec.Networking.DisableHostDNSEntry {
+			addError := outboundSet.Add(vips.NewHostEntry(es.Spec.GetHost()), vips.OutboundEntry{
+				Port:   es.Spec.GetPortUInt32(),
+				TagSet: tags,
+				Origin: vips.OriginHost(es.GetMeta().GetName()),
+			})
+			if addError != nil {
+				errs = multierr.Append(errs, errors.Wrapf(addError, "cannot add outbound for external service '%s'", es.GetMeta().GetName()))
+			}
+		}
 		for _, vob := range Match(virtualOutbounds.Items, tags) {
 			addFromVirtualOutbound(outboundSet, vob, tags, es.Descriptor().Name, es.Meta.GetName())
 		}

pkg/dns/vips_allocator_test.go

@@ -206,6 +206,21 @@ var _ = Describe("VIP Allocator", func() {
 	})
 })
 
+func resourceManagerWithResources(ctx context.Context, resources map[model.ResourceKey]model.Resource) manager.ResourceManager {
+	rm := manager.NewResourceManager(memory.NewStore())
+	meshes := map[string]bool{}
+
+	for k, res := range resources {
+		if exists := meshes[k.Mesh]; !exists {
+			Expect(rm.Create(ctx, mesh.NewMeshResource(), store.CreateBy(model.WithoutMesh(k.Mesh)))).ToNot(HaveOccurred())
+			meshes[k.Mesh] = true
+		}
+		Expect(rm.Create(ctx, res, store.CreateBy(k))).ToNot(HaveOccurred())
+	}
+
+	return rm
+}
+
 type outboundViewTestCase struct {
 	givenResources      map[model.ResourceKey]model.Resource
 	whenZone            string
@@ -218,17 +233,8 @@ type outboundViewTestCase struct {
 var _ = DescribeTable("outboundView",
 	func(tc outboundViewTestCase) {
 		// Given
-		rm := manager.NewResourceManager(memory.NewStore())
-		meshes := map[string]bool{}
-
 		ctx := context.Background()
-		for k, res := range tc.givenResources {
-			if exists := meshes[k.Mesh]; !exists {
-				Expect(rm.Create(ctx, mesh.NewMeshResource(), store.CreateBy(model.WithoutMesh(k.Mesh)))).ToNot(HaveOccurred())
-				meshes[k.Mesh] = true
-			}
-			Expect(rm.Create(ctx, res, store.CreateBy(k))).ToNot(HaveOccurred())
-		}
+		rm := resourceManagerWithResources(ctx, tc.givenResources)
 
 		cfg := dns_server.Config{
 			Domain:            "mesh",
@@ -380,6 +386,48 @@ var _ = DescribeTable("outboundView",
 			},
 		},
 	}),
+	Entry("two external services with same address and disableHostDNSEntry set to true", outboundViewTestCase{
+		givenResources: map[model.ResourceKey]model.Resource{
+			model.WithMesh("mesh", "es-1"): &mesh.ExternalServiceResource{
+				Spec: &mesh_proto.ExternalService{
+					Networking: &mesh_proto.ExternalService_Networking{
+						Address: "external.service.com:8080",
+					},
+					Tags: map[string]string{
+						mesh_proto.ServiceTag: "my-external-service-1",
+					},
+				},
+			},
+			model.WithMesh("mesh", "es-2"): &mesh.ExternalServiceResource{
+				Spec: &mesh_proto.ExternalService{
+					Networking: &mesh_proto.ExternalService_Networking{
+						Address:             "external.service.com:8080",
+						DisableHostDNSEntry: true,
+					},
+					Tags: map[string]string{
+						mesh_proto.ServiceTag: "my-external-service-2",
+					},
+				},
+			},
+		},
+		whenMesh: "mesh",
+		thenHostnameEntries: []vips.HostnameEntry{
+			vips.NewServiceEntry("my-external-service-1"),
+			vips.NewServiceEntry("my-external-service-2"),
+			vips.NewHostEntry("external.service.com"),
+		},
+		thenOutbounds: map[vips.HostnameEntry][]vips.OutboundEntry{
+			vips.NewServiceEntry("my-external-service-1"): {
+				{TagSet: map[string]string{mesh_proto.ServiceTag: "my-external-service-1"}, Origin: "service", Port: 8080},
+			},
+			vips.NewServiceEntry("my-external-service-2"): {
+				{TagSet: map[string]string{mesh_proto.ServiceTag: "my-external-service-2"}, Origin: "service", Port: 8080},
+			},
+			vips.NewHostEntry("external.service.com"): {
+				{TagSet: map[string]string{mesh_proto.ServiceTag: "my-external-service-1"}, Origin: "host", Port: 8080},
+			},
+		},
+	}),
 	Entry("zone ingress", outboundViewTestCase{
 		givenResources: map[model.ResourceKey]model.Resource{
 			model.WithMesh("default", "ingress-1"): &mesh.ZoneIngressResource{
@@ -608,6 +656,53 @@ var _ = DescribeTable("outboundView",
 	}),
 )
 
+type outboundViewErrorTestCase struct {
+	givenResources map[model.ResourceKey]model.Resource
+	buildError     string
+}
+
+var _ = DescribeTable("outboundView",
+	func(tc outboundViewErrorTestCase) {
+		ctx := context.Background()
+		rm := resourceManagerWithResources(ctx, tc.givenResources)
+
+		// When
+		allocator, err := dns.NewVIPsAllocator(rm, nil, dns_server.Config{}, "zone-a")
+		Expect(err).NotTo(HaveOccurred())
+		serviceSet, err := allocator.BuildVirtualOutboundMeshView(ctx, "mesh")
+
+		Expect(serviceSet).To(BeNil())
+		Expect(err.Error()).To(MatchRegexp(tc.buildError))
+	},
+	Entry("should fail when two external services with same address are defined without disableHostDNSEntry",
+		outboundViewErrorTestCase{
+			givenResources: map[model.ResourceKey]model.Resource{
+				model.WithMesh("mesh", "es-1"): &mesh.ExternalServiceResource{
+					Spec: &mesh_proto.ExternalService{
+						Networking: &mesh_proto.ExternalService_Networking{
+							Address: "external.service.com:8080",
+						},
+						Tags: map[string]string{
+							mesh_proto.ServiceTag: "my-external-service-1",
+						},
+					},
+				},
+				model.WithMesh("mesh", "es-2"): &mesh.ExternalServiceResource{
+					Spec: &mesh_proto.ExternalService{
+						Networking: &mesh_proto.ExternalService_Networking{
+							Address: "external.service.com:8080",
+						},
+						Tags: map[string]string{
+							mesh_proto.ServiceTag: "my-external-service-2",
+						},
+					},
+				},
+			},
+			buildError: "cannot add outbound for external service 'es-\\d': autogenerated DNS entry external.service.com:8080 conflicts with existing entry, this happens when there are two external services with the same 'networking.address', to disable automatic DNS generation in external services set 'disableHostDNSEntry' to 'true' under 'networking'",
+		},
+	),
+)
+
 var _ = Describe("AllocateVIPs", func() {
 	It("should allocate new VIPs", func() {
 		// setup