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

api/mesh/v1alpha1/externalservice.proto

@@ -57,6 +57,8 @@ message ExternalService {
     }
 
     TLS tls = 2;
+
+    bool skipVIPGeneration = 3 [ (doc.required) = true ];
   }
 
   Networking networking = 1 [ (doc.required) = true ];

docs/generated/resources/policy_external-service.md

@@ -34,7 +34,9 @@
         - `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".    
+
+    - `skipVIPGeneration` (required)
 
 - `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_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 'skipVIPGeneration' to 'true' under 'networking'", 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.SkipVIPGeneration {
+			addError := outboundSet.Add(vips.NewHostEntry(es.Spec.GetHost()), vips.OutboundEntry{
+				Port:   es.Spec.GetPortUInt32(),
+				TagSet: tags,
+				Origin: vips.OriginHost,
+			})
+			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 skipVIPGeneration 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",
+						SkipVIPGeneration: 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 skipVIPGeneration",
+		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 'skipVIPGeneration' to 'true' under 'networking'",
+		},
+	),
+)
+
 var _ = Describe("AllocateVIPs", func() {
 	It("should allocate new VIPs", func() {
 		// setup