Merge pull request #7932 from hashicorp/ingress/internal-ui-endpoint-multiple-ports

Update gateway-services-nodes API endpoint to allow multiple addresses

agent/dns.go

@@ -318,7 +318,11 @@ START:
 }
 
 func serviceNodeCanonicalDNSName(sn *structs.ServiceNode, domain string) string {
-	return serviceCanonicalDNSName(sn.ServiceName, sn.Datacenter, domain, &sn.EnterpriseMeta)
+	return serviceCanonicalDNSName(sn.ServiceName, "service", sn.Datacenter, domain, &sn.EnterpriseMeta)
+}
+
+func serviceIngressDNSName(service, datacenter, domain string, entMeta *structs.EnterpriseMeta) string {
+	return serviceCanonicalDNSName(service, "ingress", datacenter, domain, entMeta)
 }
 
 // handlePtr is used to handle "reverse" DNS queries

agent/dns_oss.go

@@ -26,6 +26,6 @@ func (d *DNSServer) parseDatacenterAndEnterpriseMeta(labels []string, _ *dnsConf
 	return false
 }
 
-func serviceCanonicalDNSName(name, datacenter, domain string, _ *structs.EnterpriseMeta) string {
+func serviceCanonicalDNSName(name, kind, datacenter, domain string, _ *structs.EnterpriseMeta) string {
-	return fmt.Sprintf("%s.service.%s.%s", name, datacenter, domain)
+	return fmt.Sprintf("%s.%s.%s.%s", name, kind, datacenter, domain)
 }

agent/dns_test.go

@@ -1041,7 +1041,7 @@ func TestDNS_ServiceReverseLookup(t *testing.T) {
 	if !ok {
 		t.Fatalf("Bad: %#v", in.Answer[0])
 	}
-	if ptrRec.Ptr != serviceCanonicalDNSName("db", "dc1", "consul", nil)+"." {
+	if ptrRec.Ptr != serviceCanonicalDNSName("db", "service", "dc1", "consul", nil)+"." {
 		t.Fatalf("Bad: %#v", ptrRec)
 	}
 }
@@ -1089,7 +1089,7 @@ func TestDNS_ServiceReverseLookup_IPV6(t *testing.T) {
 	if !ok {
 		t.Fatalf("Bad: %#v", in.Answer[0])
 	}
-	if ptrRec.Ptr != serviceCanonicalDNSName("db", "dc1", "consul", nil)+"." {
+	if ptrRec.Ptr != serviceCanonicalDNSName("db", "service", "dc1", "consul", nil)+"." {
 		t.Fatalf("Bad: %#v", ptrRec)
 	}
 }
@@ -1139,7 +1139,7 @@ func TestDNS_ServiceReverseLookup_CustomDomain(t *testing.T) {
 	if !ok {
 		t.Fatalf("Bad: %#v", in.Answer[0])
 	}
-	if ptrRec.Ptr != serviceCanonicalDNSName("db", "dc1", "custom", nil)+"." {
+	if ptrRec.Ptr != serviceCanonicalDNSName("db", "service", "dc1", "custom", nil)+"." {
 		t.Fatalf("Bad: %#v", ptrRec)
 	}
 }

agent/structs/config_entry_gateways.go

@@ -376,6 +376,23 @@ type GatewayService struct {
 
 type GatewayServices []*GatewayService
 
+func (g *GatewayService) Addresses(defaultHosts []string) []string {
+	if g.Port == 0 {
+		return nil
+	}
+
+	hosts := g.Hosts
+	if len(hosts) == 0 {
+		hosts = defaultHosts
+	}
+
+	var addresses []string
+	for _, h := range hosts {
+		addresses = append(addresses, fmt.Sprintf("%s:%d", h, g.Port))
+	}
+	return addresses
+}
+
 func (g *GatewayService) IsSame(o *GatewayService) bool {
 	return g.Gateway.Matches(&o.Gateway) &&
 		g.Service.Matches(&o.Service) &&

agent/structs/config_entry_gateways_test.go

@@ -77,10 +77,7 @@ func TestIngressConfigEntry_Normalize(t *testing.T) {
 		},
 	}
 
-	for _, test := range cases {
+	for _, tc := range cases {
-		// We explicitly copy the variable for the range statement so that can run
-		// tests in parallel.
-		tc := test
 		t.Run(tc.name, func(t *testing.T) {
 			err := tc.entry.Normalize()
 			require.NoError(t, err)
@@ -436,10 +433,7 @@ func TestIngressConfigEntry_Validate(t *testing.T) {
 		},
 	}
 
-	for _, test := range cases {
+	for _, tc := range cases {
-		// We explicitly copy the variable for the range statement so that can run
-		// tests in parallel.
-		tc := test
 		t.Run(tc.name, func(t *testing.T) {
 			err := tc.entry.Validate()
 			if tc.expectErr != "" {
@@ -546,10 +540,7 @@ func TestTerminatingConfigEntry_Validate(t *testing.T) {
 		},
 	}
 
-	for _, test := range cases {
+	for _, tc := range cases {
-		// We explicitly copy the variable for the range statement so that can run
-		// tests in parallel.
-		tc := test
 		t.Run(tc.name, func(t *testing.T) {
 
 			err := tc.entry.Validate()
@@ -562,3 +553,58 @@ func TestTerminatingConfigEntry_Validate(t *testing.T) {
 		})
 	}
 }
+
+func TestGatewayService_Addresses(t *testing.T) {
+	cases := []struct {
+		name     string
+		input    GatewayService
+		argument []string
+		expected []string
+	}{
+		{
+			name:     "port is zero",
+			input:    GatewayService{},
+			expected: nil,
+		},
+		{
+			name: "no hosts with empty array",
+			input: GatewayService{
+				Port: 8080,
+			},
+			expected: nil,
+		},
+		{
+			name: "no hosts with default",
+			input: GatewayService{
+				Port: 8080,
+			},
+			argument: []string{
+				"service.ingress.dc.domain",
+				"service.ingress.dc.alt.domain",
+			},
+			expected: []string{
+				"service.ingress.dc.domain:8080",
+				"service.ingress.dc.alt.domain:8080",
+			},
+		},
+		{
+			name: "user-defined hosts",
+			input: GatewayService{
+				Port:  8080,
+				Hosts: []string{"*.test.example.com", "other.example.com"},
+			},
+			argument: []string{
+				"service.ingress.dc.domain",
+				"service.ingress.alt.domain",
+			},
+			expected: []string{"*.test.example.com:8080", "other.example.com:8080"},
+		},
+	}
+
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			addresses := tc.input.Addresses(tc.argument)
+			require.ElementsMatch(t, tc.expected, addresses)
+		})
+	}
+}

agent/ui_endpoint.go

@@ -6,6 +6,7 @@ import (
 	"sort"
 	"strings"
 
+	"github.com/hashicorp/consul/agent/config"
 	"github.com/hashicorp/consul/agent/structs"
 	"github.com/hashicorp/consul/api"
 )
@@ -16,7 +17,9 @@ import (
 const metaExternalSource = "external-source"
 
 type GatewayConfig struct {
-	ListenerPort int
+	Addresses []string `json:",omitempty"`
+	// internal to track uniqueness
+	addressesSet map[string]struct{}
 }
 
 // ServiceSummary is used to summarize a service
@@ -161,7 +164,7 @@ RPC:
 
 	// Generate the summary
 	// TODO (gateways) (freddy) Have Internal.ServiceDump return ServiceDump instead. Need to add bexpr filtering for type.
-	return summarizeServices(out.Nodes.ToServiceDump()), nil
+	return summarizeServices(out.Nodes.ToServiceDump(), s.agent.config), nil
 }
 
 // UIGatewayServices is used to query all the nodes for services associated with a gateway along with their gateway config
@@ -195,10 +198,11 @@ RPC:
 		}
 		return nil, err
 	}
-	return summarizeServices(out.Dump), nil
+
+	return summarizeServices(out.Dump, s.agent.config), nil
 }
 
-func summarizeServices(dump structs.ServiceDump) []*ServiceSummary {
+func summarizeServices(dump structs.ServiceDump, cfg *config.RuntimeConfig) []*ServiceSummary {
 	// Collect the summary information
 	var services []structs.ServiceID
 	summary := make(map[structs.ServiceID]*ServiceSummary)
@@ -220,8 +224,9 @@ func summarizeServices(dump structs.ServiceDump) []*ServiceSummary {
 
 	for _, csn := range dump {
 		if csn.GatewayService != nil {
-			sum := getService(csn.GatewayService.Service.ToServiceID())
+			gwsvc := csn.GatewayService
-			sum.GatewayConfig.ListenerPort = csn.GatewayService.Port
+			sum := getService(gwsvc.Service.ToServiceID())
+			modifySummaryForGatewayService(cfg, sum, gwsvc)
 		}
 
 		// Will happen in cases where we only have the GatewayServices mapping
@@ -299,3 +304,38 @@ func summarizeServices(dump structs.ServiceDump) []*ServiceSummary {
 	}
 	return output
 }
+
+func modifySummaryForGatewayService(
+	cfg *config.RuntimeConfig,
+	sum *ServiceSummary,
+	gwsvc *structs.GatewayService,
+) {
+	var dnsAddresses []string
+	for _, domain := range []string{cfg.DNSDomain, cfg.DNSAltDomain} {
+		// If the domain is empty, do not use it to construct a valid DNS
+		// address
+		if domain == "" {
+			continue
+		}
+		dnsAddresses = append(dnsAddresses, serviceIngressDNSName(
+			gwsvc.Service.Name,
+			cfg.Datacenter,
+			domain,
+			&gwsvc.Service.EnterpriseMeta,
+		))
+	}
+
+	for _, addr := range gwsvc.Addresses(dnsAddresses) {
+		// check for duplicates, a service will have a ServiceInfo struct for
+		// every instance that is registered.
+		if _, ok := sum.GatewayConfig.addressesSet[addr]; !ok {
+			if sum.GatewayConfig.addressesSet == nil {
+				sum.GatewayConfig.addressesSet = make(map[string]struct{})
+			}
+			sum.GatewayConfig.addressesSet[addr] = struct{}{}
+			sum.GatewayConfig.Addresses = append(
+				sum.GatewayConfig.Addresses, addr,
+			)
+		}
+	}
+}

agent/ui_endpoint_test.go

@@ -535,7 +535,7 @@ func TestUIGatewayServiceNodes_Terminating(t *testing.T) {
 func TestUIGatewayServiceNodes_Ingress(t *testing.T) {
 	t.Parallel()
 
-	a := NewTestAgent(t, "")
+	a := NewTestAgent(t, `alt_domain = "alt.consul."`)
 	defer a.Shutdown()
 
 	// Register ingress gateway and a service that will be associated with it
@@ -593,6 +593,18 @@ func TestUIGatewayServiceNodes_Ingress(t *testing.T) {
 		}
 		require.NoError(t, a.RPC("Catalog.Register", &arg, &regOutput))
 
+		// Set web protocol to http
+		svcDefaultsReq := structs.ConfigEntryRequest{
+			Datacenter: "dc1",
+			Entry: &structs.ServiceConfigEntry{
+				Name:     "web",
+				Protocol: "http",
+			},
+		}
+		var configOutput bool
+		require.NoError(t, a.RPC("ConfigEntry.Apply", &svcDefaultsReq, &configOutput))
+		require.True(t, configOutput)
+
 		// Register ingress-gateway config entry, linking it to db and redis (does not exist)
 		args := &structs.IngressGatewayConfigEntry{
 			Name: "ingress-gateway",
@@ -609,13 +621,23 @@ func TestUIGatewayServiceNodes_Ingress(t *testing.T) {
 				},
 				{
 					Port:     8080,
-					Protocol: "tcp",
+					Protocol: "http",
 					Services: []structs.IngressService{
 						{
 							Name: "web",
 						},
 					},
 				},
+				{
+					Port:     8081,
+					Protocol: "http",
+					Services: []structs.IngressService{
+						{
+							Name:  "web",
+							Hosts: []string{"*.test.example.com"},
+						},
+					},
+				},
 			},
 		}
 
@@ -624,7 +646,6 @@ func TestUIGatewayServiceNodes_Ingress(t *testing.T) {
 			Datacenter: "dc1",
 			Entry:      args,
 		}
-		var configOutput bool
 		require.NoError(t, a.RPC("ConfigEntry.Apply", &req, &configOutput))
 		require.True(t, configOutput)
 	}
@@ -636,11 +657,23 @@ func TestUIGatewayServiceNodes_Ingress(t *testing.T) {
 	assert.Nil(t, err)
 	assertIndex(t, resp)
 
+	// Construct expected addresses so that differences between OSS/Ent are handled by code
+	webDNS := serviceIngressDNSName("web", "dc1", "consul.", structs.DefaultEnterpriseMeta())
+	webDNSAlt := serviceIngressDNSName("web", "dc1", "alt.consul.", structs.DefaultEnterpriseMeta())
+	dbDNS := serviceIngressDNSName("db", "dc1", "consul.", structs.DefaultEnterpriseMeta())
+	dbDNSAlt := serviceIngressDNSName("db", "dc1", "alt.consul.", structs.DefaultEnterpriseMeta())
+
 	dump := obj.([]*ServiceSummary)
 	expect := []*ServiceSummary{
 		{
-			Name:           "web",
+			Name: "web",
-			GatewayConfig:  GatewayConfig{ListenerPort: 8080},
+			GatewayConfig: GatewayConfig{
+				Addresses: []string{
+					fmt.Sprintf("%s:8080", webDNS),
+					fmt.Sprintf("%s:8080", webDNSAlt),
+					"*.test.example.com:8081",
+				},
+			},
 			EnterpriseMeta: *structs.DefaultEnterpriseMeta(),
 		},
 		{
@@ -651,9 +684,19 @@ func TestUIGatewayServiceNodes_Ingress(t *testing.T) {
 			ChecksPassing:  1,
 			ChecksWarning:  1,
 			ChecksCritical: 0,
-			GatewayConfig:  GatewayConfig{ListenerPort: 8888},
+			GatewayConfig: GatewayConfig{
+				Addresses: []string{
+					fmt.Sprintf("%s:8888", dbDNS),
+					fmt.Sprintf("%s:8888", dbDNSAlt),
+				},
+			},
 			EnterpriseMeta: *structs.DefaultEnterpriseMeta(),
 		},
 	}
+
+	// internal accounting that users don't see can be blown away
+	for _, sum := range dump {
+		sum.GatewayConfig.addressesSet = nil
+	}
 	assert.ElementsMatch(t, expect, dump)
 }