feat(gateway): auto generate hostname for crossMesh listeners (#4421)

* feat(gateway): auto generate hostname for crossMesh listeners
* test(pkg/dns): reuse config
* refactor(pkg/config/dns-server): don't stutter struct name

Signed-off-by: Mike Beaumont <mjboamail@gmail.com>
(cherry picked from commit 4e611f5)

pkg/config/app/kuma-cp/config.go

@@ -134,7 +134,7 @@ type Config struct {
 	// Multizone Config
 	Multizone *multizone.MultizoneConfig `yaml:"multizone,omitempty"`
 	// DNS Server Config
-	DNSServer *dns_server.DNSServerConfig `yaml:"dnsServer,omitempty"`
+	DNSServer *dns_server.Config `yaml:"dnsServer,omitempty"`
 	// Diagnostics configuration
 	Diagnostics *diagnostics.DiagnosticsConfig `yaml:"diagnostics,omitempty"`
 	// Dataplane Server configuration

pkg/config/dns-server/config.go

@@ -8,7 +8,7 @@ import (
 )
 
 // DNS Server configuration
-type DNSServerConfig struct {
+type Config struct {
 	// The domain that the server will resolve the services for
 	Domain string `yaml:"domain" envconfig:"kuma_dns_server_domain"`
 	// CIDR used to allocate virtual IPs from
@@ -17,21 +17,21 @@ type DNSServerConfig struct {
 	ServiceVipEnabled bool `yaml:"serviceVipEnabled" envconfig:"kuma_dns_server_service_vip_enabled"`
 }
 
-func (g *DNSServerConfig) Sanitize() {
+func (g *Config) Sanitize() {
 }
 
-func (g *DNSServerConfig) Validate() error {
+func (g *Config) Validate() error {
 	_, _, err := net.ParseCIDR(g.CIDR)
 	if err != nil {
 		return errors.New("Must provide a valid CIDR")
 	}
 	return nil
 }
 
-var _ config.Config = &DNSServerConfig{}
+var _ config.Config = &Config{}
 
-func DefaultDNSServerConfig() *DNSServerConfig {
-	return &DNSServerConfig{
+func DefaultDNSServerConfig() *Config {
+	return &Config{
 		ServiceVipEnabled: true,
 		Domain:            "mesh",
 		CIDR:              "240.0.0.0/4",

pkg/core/resources/apis/mesh/gateway_validator.go

@@ -88,12 +88,6 @@ func validateMeshGatewayConf(path validators.PathBuilder, conf *mesh_proto.MeshG
 			}
 		}
 
-		if l.GetCrossMesh() {
-			if l.GetHostname() == "" {
-				err.AddViolationAt(path.Index(i).Field("hostname"), "cannot be empty with crossMesh")
-			}
-		}
-
 		if tls := l.GetTls(); tls != nil && !l.GetCrossMesh() {
 			switch tls.GetMode() {
 			case mesh_proto.MeshGateway_TLS_NONE:

pkg/core/resources/apis/mesh/gateway_validator_test.go

@@ -69,6 +69,23 @@ conf:
     crossMesh: true
     protocol: HTTP`,
 		),
+		Entry("crossMesh with no hostname", `
+type: MeshGateway
+name: gateway
+mesh: default
+selectors:
+  - match:
+      kuma.io/service: gateway
+tags:
+  product: edge
+conf:
+  listeners:
+  - protocol: HTTP
+    port: 99
+    crossMesh: true
+    tags:
+      name: http`,
+		),
 	)
 
 	DescribeErrorCases(
@@ -310,28 +327,6 @@ conf:
       name: https
 `),
 
-		ErrorCase("crossMesh and no hostname",
-			validators.Violation{
-				Field:   "conf.listeners[0].hostname",
-				Message: "cannot be empty with crossMesh",
-			}, `
-type: MeshGateway
-name: gateway
-mesh: default
-selectors:
-  - match:
-      kuma.io/service: gateway
-tags:
-  product: edge
-conf:
-  listeners:
-  - protocol: HTTP
-    port: 99
-    crossMesh: true
-    tags:
-      name: http
-`),
-
 		ErrorCase("crossMesh and multiple services",
 			validators.Violation{
 				Field:   "selectors[1]",

pkg/dns/vips_allocator.go

@@ -9,6 +9,7 @@ import (
 	"go.uber.org/multierr"
 
 	mesh_proto "github.com/kumahq/kuma/api/mesh/v1alpha1"
+	dns_server "github.com/kumahq/kuma/pkg/config/dns-server"
 	"github.com/kumahq/kuma/pkg/core"
 	config_manager "github.com/kumahq/kuma/pkg/core/config/manager"
 	core_mesh "github.com/kumahq/kuma/pkg/core/resources/apis/mesh"
@@ -25,18 +26,20 @@ type VIPsAllocator struct {
 	persistence       *vips.Persistence
 	cidr              string
 	serviceVipEnabled bool
+	dnsSuffix         string
 }
 
 // NewVIPsAllocator creates new object of VIPsAllocator. You can either
 // call method CreateOrUpdateVIPConfig manually or start VIPsAllocator as a component.
 // In the latter scenario it will call CreateOrUpdateVIPConfig every 'tickInterval'
 // for all meshes in the store.
-func NewVIPsAllocator(rm manager.ReadOnlyResourceManager, configManager config_manager.ConfigManager, serviceVipEnabled bool, cidr string) (*VIPsAllocator, error) {
+func NewVIPsAllocator(rm manager.ReadOnlyResourceManager, configManager config_manager.ConfigManager, config dns_server.Config) (*VIPsAllocator, error) {
 	return &VIPsAllocator{
 		rm:                rm,
 		persistence:       vips.NewPersistence(rm, configManager),
-		serviceVipEnabled: serviceVipEnabled,
-		cidr:              cidr,
+		serviceVipEnabled: config.ServiceVipEnabled,
+		cidr:              config.CIDR,
+		dnsSuffix:         config.Domain,
 	}, nil
 }
 
@@ -61,7 +64,7 @@ func (d *VIPsAllocator) CreateOrUpdateVIPConfig(ctx context.Context, mesh string
 		return err
 	}
 
-	newView, err := BuildVirtualOutboundMeshView(ctx, d.rm, d.serviceVipEnabled, mesh)
+	newView, err := BuildVirtualOutboundMeshView(ctx, d.rm, d.serviceVipEnabled, d.dnsSuffix, mesh)
 	if err != nil {
 		return err
 	}
@@ -81,7 +84,7 @@ func (d *VIPsAllocator) createOrUpdateVIPConfigs(ctx context.Context, mesh strin
 		return err
 	}
 
-	newView, err := BuildVirtualOutboundMeshView(ctx, d.rm, d.serviceVipEnabled, mesh)
+	newView, err := BuildVirtualOutboundMeshView(ctx, d.rm, d.serviceVipEnabled, d.dnsSuffix, mesh)
 	if err != nil {
 		return err
 	}
@@ -130,14 +133,24 @@ func (d *VIPsAllocator) createOrUpdateMeshVIPConfig(
 	return d.persistence.Set(ctx, mesh, out)
 }
 
-func addFromMeshGateway(outboundSet *vips.VirtualOutboundMeshView, mesh string, gateway *core_mesh.MeshGatewayResource) {
+func generatedHostname(meta model.ResourceMeta, suffix string) string {
+	return fmt.Sprintf("internal.%s.%s.%s", meta.GetName(), meta.GetMesh(), suffix)
+}
+
+func addFromMeshGateway(outboundSet *vips.VirtualOutboundMeshView, dnsSuffix, mesh string, gateway *core_mesh.MeshGatewayResource) {
 	for i, listener := range gateway.Spec.Conf.Listeners {
 		// We only setup outbounds for cross mesh listeners and only ones with a
 		// concrete hostname.
-		if !listener.CrossMesh || strings.Contains(listener.Hostname, "*") {
+		if !listener.CrossMesh {
 			continue
 		}
 
+		hostname := listener.Hostname
+
+		if hostname == "" || strings.Contains(hostname, "*") {
+			hostname = generatedHostname(gateway.GetMeta(), dnsSuffix)
+		}
+
 		// We only allow one selector with a crossMesh listener
 		for _, selector := range gateway.Spec.Selectors {
 			tags := mesh_proto.Merge(
@@ -148,7 +161,6 @@ func addFromMeshGateway(outboundSet *vips.VirtualOutboundMeshView, mesh string,
 				},
 				selector.GetMatch(),
 			)
-			hostname := listener.Hostname
 			origin := fmt.Sprintf("mesh-gateway:%s:%s:%s", mesh, gateway.GetMeta().GetName(), hostname)
 
 			entry := vips.OutboundEntry{
@@ -164,7 +176,7 @@ func addFromMeshGateway(outboundSet *vips.VirtualOutboundMeshView, mesh string,
 	}
 }
 
-func BuildVirtualOutboundMeshView(ctx context.Context, rm manager.ReadOnlyResourceManager, serviceVipEnabled bool, mesh string) (*vips.VirtualOutboundMeshView, error) {
+func BuildVirtualOutboundMeshView(ctx context.Context, rm manager.ReadOnlyResourceManager, serviceVipEnabled bool, dnsSuffix string, mesh string) (*vips.VirtualOutboundMeshView, error) {
 	outboundSet := vips.NewEmptyVirtualOutboundView()
 
 	virtualOutbounds := core_mesh.VirtualOutboundResourceList{}
@@ -236,7 +248,7 @@ func BuildVirtualOutboundMeshView(ctx context.Context, rm manager.ReadOnlyResour
 		}
 
 		for _, gateway := range gateways.Items {
-			addFromMeshGateway(outboundSet, meshName, gateway)
+			addFromMeshGateway(outboundSet, dnsSuffix, meshName, gateway)
 		}
 	}
 

pkg/dns/vips_allocator_test.go

@@ -8,6 +8,7 @@ import (
 	"github.com/pkg/errors"
 
 	mesh_proto "github.com/kumahq/kuma/api/mesh/v1alpha1"
+	dns_server "github.com/kumahq/kuma/pkg/config/dns-server"
 	config_manager "github.com/kumahq/kuma/pkg/core/config/manager"
 	"github.com/kumahq/kuma/pkg/core/resources/apis/mesh"
 	config_model "github.com/kumahq/kuma/pkg/core/resources/apis/system"
@@ -52,6 +53,12 @@ func (e *errConfigManager) Update(ctx context.Context, r *config_model.ConfigRes
 	return errors.Errorf("error during update, mesh = %s", meshName)
 }
 
+var testConfig = dns_server.Config{
+	ServiceVipEnabled: true,
+	CIDR:              "240.0.0.0/24",
+	Domain:            "mesh",
+}
+
 var _ = Describe("VIP Allocator", func() {
 	var rm manager.ResourceManager
 	var cm config_manager.ConfigManager
@@ -81,7 +88,7 @@ var _ = Describe("VIP Allocator", func() {
 		err = rm.Create(context.Background(), &mesh.DataplaneResource{Spec: dp("web")}, store.CreateByKey("dp-3", "mesh-2"))
 		Expect(err).ToNot(HaveOccurred())
 
-		allocator, err = dns.NewVIPsAllocator(rm, cm, true, "240.0.0.0/24")
+		allocator, err = dns.NewVIPsAllocator(rm, cm, testConfig)
 		Expect(err).ToNot(HaveOccurred())
 	})
 
@@ -143,7 +150,7 @@ var _ = Describe("VIP Allocator", func() {
 	It("should return error if failed to update VIP config", func() {
 		errConfigManager := &errConfigManager{ConfigManager: cm}
 		ctx := context.Background()
-		errAllocator, err := dns.NewVIPsAllocator(rm, errConfigManager, true, "240.0.0.0/24")
+		errAllocator, err := dns.NewVIPsAllocator(rm, errConfigManager, testConfig)
 		Expect(err).ToNot(HaveOccurred())
 
 		err = errAllocator.CreateOrUpdateVIPConfig(ctx, "mesh-1", NoModifications)
@@ -159,7 +166,7 @@ var _ = Describe("VIP Allocator", func() {
 
 	It("should try to update all meshes and return combined error", func() {
 		errConfigManager := &errConfigManager{ConfigManager: cm}
-		errAllocator, err := dns.NewVIPsAllocator(rm, errConfigManager, true, "240.0.0.0/24")
+		errAllocator, err := dns.NewVIPsAllocator(rm, errConfigManager, testConfig)
 		Expect(err).ToNot(HaveOccurred())
 
 		err = errAllocator.CreateOrUpdateVIPConfigs(context.Background())
@@ -223,7 +230,7 @@ var _ = DescribeTable("outboundView",
 		}
 
 		// When
-		serviceSet, err := dns.BuildVirtualOutboundMeshView(ctx, rm, !tc.whenSkipServiceVips, tc.whenMesh)
+		serviceSet, err := dns.BuildVirtualOutboundMeshView(ctx, rm, !tc.whenSkipServiceVips, "mesh", tc.whenMesh)
 
 		// Then
 		Expect(err).ToNot(HaveOccurred())
@@ -289,6 +296,13 @@ var _ = DescribeTable("outboundView",
 							Tags: map[string]string{
 								"listener": "internal",
 							},
+						}, {
+							Port:      81,
+							CrossMesh: true,
+							Protocol:  mesh_proto.MeshGateway_Listener_HTTP,
+							Tags: map[string]string{
+								"listener": "internal2",
+							},
 						}, {
 							Hostname: "*",
 							Port:     80,
@@ -310,7 +324,7 @@ var _ = DescribeTable("outboundView",
 			},
 		},
 		whenMesh:            "mesh1",
-		thenHostnameEntries: []vips.HostnameEntry{vips.NewFqdnEntry("gateway2.mesh")},
+		thenHostnameEntries: []vips.HostnameEntry{vips.NewFqdnEntry("gateway2.mesh"), vips.NewFqdnEntry("internal.gateway.mesh2.mesh")},
 		thenOutbounds: map[vips.HostnameEntry][]vips.OutboundEntry{
 			vips.NewFqdnEntry("gateway2.mesh"): {{
 				TagSet: map[string]string{
@@ -322,6 +336,16 @@ var _ = DescribeTable("outboundView",
 				Origin: "mesh-gateway:mesh2:gateway:gateway2.mesh",
 				Port:   80,
 			}},
+			vips.NewFqdnEntry("internal.gateway.mesh2.mesh"): {{
+				TagSet: map[string]string{
+					"listener":            "internal2",
+					"gateway":             "prod",
+					mesh_proto.ServiceTag: "gateway",
+					"kuma.io/mesh":        "mesh2",
+				},
+				Origin: "mesh-gateway:mesh2:gateway:internal.gateway.mesh2.mesh",
+				Port:   81,
+			}},
 		},
 	}),
 	Entry("external service", outboundViewTestCase{

pkg/plugins/runtime/k8s/plugin.go

@@ -185,8 +185,7 @@ func addDNS(mgr kube_ctrl.Manager, rt core_runtime.Runtime, converter k8s_common
 	vipsAllocator, err := dns.NewVIPsAllocator(
 		rt.ResourceManager(),
 		rt.ConfigManager(),
-		rt.Config().DNSServer.ServiceVipEnabled,
-		rt.Config().DNSServer.CIDR,
+		*rt.Config().DNSServer,
 	)
 	if err != nil {
 		return err

pkg/plugins/runtime/universal/plugin.go

@@ -35,8 +35,7 @@ func addDNS(rt core_runtime.Runtime) error {
 	vipsAllocator, err := dns.NewVIPsAllocator(
 		rt.ReadOnlyResourceManager(),
 		rt.ConfigManager(),
-		rt.Config().DNSServer.ServiceVipEnabled,
-		rt.Config().DNSServer.CIDR,
+		*rt.Config().DNSServer,
 	)
 	if err != nil {
 		return err