From c82b78b088c6e99a511018ed4f6ac2f1fb364420 Mon Sep 17 00:00:00 2001 From: John Murret Date: Tue, 30 Jan 2024 15:34:35 -0700 Subject: [PATCH] NET-7165 - fix address and target setting (#20403) --- agent/discovery/query_fetcher_v1.go | 50 ++++-- agent/discovery/query_fetcher_v1_test.go | 192 ++++++++++++++++++++++- agent/dns/dns_address.go | 2 +- agent/dns/dns_address_test.go | 14 ++ agent/dns/query_locality.go | 42 ----- agent/dns/query_locality_ce.go | 57 ------- agent/dns/query_locality_ce_test.go | 60 ------- agent/dns/query_locality_test.go | 74 --------- agent/dns/router.go | 148 ++++++++++------- 9 files changed, 332 insertions(+), 307 deletions(-) delete mode 100644 agent/dns/query_locality.go delete mode 100644 agent/dns/query_locality_ce.go delete mode 100644 agent/dns/query_locality_ce_test.go delete mode 100644 agent/dns/query_locality_test.go diff --git a/agent/discovery/query_fetcher_v1.go b/agent/discovery/query_fetcher_v1.go index ff7837cb97..0b3318a372 100644 --- a/agent/discovery/query_fetcher_v1.go +++ b/agent/discovery/query_fetcher_v1.go @@ -127,6 +127,10 @@ func (f *V1DataFetcher) FetchNodes(ctx Context, req *QueryPayload) ([]*Result, e Type: ResultTypeNode, Metadata: node.Meta, Target: node.Node, + Tenancy: ResultTenancy{ + EnterpriseMeta: cfg.defaultEntMeta, + Datacenter: cfg.datacenter, + }, }) return results, nil @@ -358,19 +362,10 @@ func (f *V1DataFetcher) fetchServiceBasedOnTenancy(ctx Context, req *QueryPayloa out.Nodes.Shuffle() results := make([]*Result, 0, len(out.Nodes)) for _, node := range out.Nodes { - target := node.Service.Address - resultType := ResultTypeService - // TODO (v2-dns): IMPORTANT!!!!: this needs to be revisited in how dns v1 utilizes - // the nodeaddress when the service address is an empty string. Need to figure out - // if this can be removed and dns recursion and process can work with only the - // address set to the node.address and the target set to the service.address. - // We may have to look at modifying the discovery result if more metadata is needed to send along. - if target == "" { - target = node.Node.Node - resultType = ResultTypeNode - } + address, target, resultType := getAddressTargetAndResultType(node) + results = append(results, &Result{ - Address: node.Node.Address, + Address: address, Type: resultType, Target: target, Weight: uint32(findWeight(node)), @@ -386,6 +381,37 @@ func (f *V1DataFetcher) fetchServiceBasedOnTenancy(ctx Context, req *QueryPayloa return results, nil } +// getAddressTargetAndResultType returns the address, target and result type for a check service node. +func getAddressTargetAndResultType(node structs.CheckServiceNode) (string, string, ResultType) { + // Set address and target + // if service address is present, set target and address based on service. + // otherwise get it from the node. + address := node.Service.Address + target := node.Service.Service + resultType := ResultTypeService + + addressIP := net.ParseIP(address) + if addressIP == nil { + resultType = ResultTypeNode + if node.Service.Address != "" { + // cases where service address is foo or foo.node.consul + // For usage in DNS, these discovery results necessitate a CNAME record. + // These cases can be inferred from the discovery result when Type is Node and + // target is not an IP. + target = node.Service.Address + } else { + // cases where service address is empty and the service is bound to + // node with an address. These do not require a CNAME record in. + // For usage in DNS, these discovery results do not require a CNAME record. + // These cases can be inferred from the discovery result when Type is Node and + // target is not an IP. + target = node.Node.Node + } + address = node.Node.Address + } + return address, target, resultType +} + // findWeight returns the weight of a service node. func findWeight(node structs.CheckServiceNode) int { // By default, when only_passing is false, warning and passing nodes are returned diff --git a/agent/discovery/query_fetcher_v1_test.go b/agent/discovery/query_fetcher_v1_test.go index add1b7bda7..9abcccf160 100644 --- a/agent/discovery/query_fetcher_v1_test.go +++ b/agent/discovery/query_fetcher_v1_test.go @@ -21,7 +21,7 @@ import ( "github.com/hashicorp/consul/sdk/testutil" ) -// Test_FetchService tests the FetchService method in scenarios where the RPC +// Test_FetchVirtualIP tests the FetchVirtualIP method in scenarios where the RPC // call succeeds and fails. func Test_FetchVirtualIP(t *testing.T) { // set these to confirm that RPC call does not use them for this particular RPC @@ -119,3 +119,193 @@ func Test_FetchVirtualIP(t *testing.T) { }) } } + +// Test_FetchEndpoints tests the FetchEndpoints method in scenarios where the RPC +// call succeeds and fails. +func Test_FetchEndpoints(t *testing.T) { + // set these to confirm that RPC call does not use them for this particular RPC + rc := &config.RuntimeConfig{ + DNSAllowStale: true, + DNSMaxStale: 100, + DNSUseCache: true, + DNSCacheMaxAge: 100, + } + tests := []struct { + name string + queryPayload *QueryPayload + context Context + rpcFuncForServiceNodes func(ctx context.Context, req structs.ServiceSpecificRequest) (structs.IndexedCheckServiceNodes, cache.ResultMeta, error) + expectedResults []*Result + expectedErr error + }{ + { + name: "when service address is IPv4, result type is service, address is service address and target is service name", + queryPayload: &QueryPayload{ + Name: "service-name", + Tenancy: QueryTenancy{ + EnterpriseMeta: defaultEntMeta, + }, + }, + rpcFuncForServiceNodes: func(ctx context.Context, req structs.ServiceSpecificRequest) (structs.IndexedCheckServiceNodes, cache.ResultMeta, error) { + return structs.IndexedCheckServiceNodes{ + Nodes: []structs.CheckServiceNode{ + { + Node: &structs.Node{ + Address: "node-address", + Node: "node-name", + }, + Service: &structs.NodeService{ + Address: "127.0.0.1", + Service: "service-name", + }, + }, + }, + }, cache.ResultMeta{}, nil + }, + context: Context{ + Token: "test-token", + }, + expectedResults: []*Result{ + { + Address: "127.0.0.1", + Target: "service-name", + Type: ResultTypeService, + Weight: 1, + }, + }, + expectedErr: nil, + }, + { + name: "when service address is IPv6, result type is service, address is service address and target is service name", + queryPayload: &QueryPayload{ + Name: "service-name", + Tenancy: QueryTenancy{ + EnterpriseMeta: defaultEntMeta, + }, + }, + rpcFuncForServiceNodes: func(ctx context.Context, req structs.ServiceSpecificRequest) (structs.IndexedCheckServiceNodes, cache.ResultMeta, error) { + return structs.IndexedCheckServiceNodes{ + Nodes: []structs.CheckServiceNode{ + { + Node: &structs.Node{ + Address: "node-address", + Node: "node-name", + }, + Service: &structs.NodeService{ + Address: "2001:db8:1:2:cafe::1337", + Service: "service-name", + }, + }, + }, + }, cache.ResultMeta{}, nil + }, + context: Context{ + Token: "test-token", + }, + expectedResults: []*Result{ + { + Address: "2001:db8:1:2:cafe::1337", + Target: "service-name", + Type: ResultTypeService, + Weight: 1, + }, + }, + expectedErr: nil, + }, + { + name: "when service address is not IP but is not empty, result type is node, address is node address, and target is service address", + queryPayload: &QueryPayload{ + Name: "service-name", + Tenancy: QueryTenancy{ + EnterpriseMeta: defaultEntMeta, + }, + }, + rpcFuncForServiceNodes: func(ctx context.Context, req structs.ServiceSpecificRequest) (structs.IndexedCheckServiceNodes, cache.ResultMeta, error) { + return structs.IndexedCheckServiceNodes{ + Nodes: []structs.CheckServiceNode{ + { + Node: &structs.Node{ + Address: "node-address", + Node: "node-name", + }, + Service: &structs.NodeService{ + Address: "foo", + Service: "service-name", + }, + }, + }, + }, cache.ResultMeta{}, nil + }, + context: Context{ + Token: "test-token", + }, + expectedResults: []*Result{ + { + Address: "node-address", + Target: "foo", + Type: ResultTypeNode, + Weight: 1, + }, + }, + expectedErr: nil, + }, + { + name: "when service address is empty, result type is node, address is node address, and target is node name", + queryPayload: &QueryPayload{ + Name: "service-name", + Tenancy: QueryTenancy{ + EnterpriseMeta: defaultEntMeta, + }, + }, + rpcFuncForServiceNodes: func(ctx context.Context, req structs.ServiceSpecificRequest) (structs.IndexedCheckServiceNodes, cache.ResultMeta, error) { + return structs.IndexedCheckServiceNodes{ + Nodes: []structs.CheckServiceNode{ + { + Node: &structs.Node{ + Address: "node-address", + Node: "node-name", + }, + Service: &structs.NodeService{ + Address: "", + Service: "service-name", + }, + }, + }, + }, cache.ResultMeta{}, nil + }, + context: Context{ + Token: "test-token", + }, + expectedResults: []*Result{ + { + Address: "node-address", + Target: "node-name", + Type: ResultTypeNode, + Weight: 1, + }, + }, + expectedErr: nil, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + logger := testutil.Logger(t) + mockRPC := cachetype.NewMockRPC(t) + // TODO (v2-dns): mock these properly + translateServicePortFunc := func(dc string, port int, taggedAddresses map[string]structs.ServiceAddress) int { return 0 } + rpcFuncForSamenessGroup := func(ctx context.Context, req *structs.ConfigEntryQuery) (structs.SamenessGroupConfigEntry, cache.ResultMeta, error) { + return structs.SamenessGroupConfigEntry{}, cache.ResultMeta{}, nil + } + getFromCacheFunc := func(ctx context.Context, t string, r cache.Request) (interface{}, cache.ResultMeta, error) { + return nil, cache.ResultMeta{}, nil + } + + df := NewV1DataFetcher(rc, acl.DefaultEnterpriseMeta(), getFromCacheFunc, mockRPC.RPC, tc.rpcFuncForServiceNodes, rpcFuncForSamenessGroup, translateServicePortFunc, logger) + + results, err := df.FetchEndpoints(tc.context, tc.queryPayload, LookupTypeService) + require.Equal(t, tc.expectedErr, err) + require.Equal(t, tc.expectedResults, results) + }) + } +} diff --git a/agent/dns/dns_address.go b/agent/dns/dns_address.go index caadf68422..e1e61f689f 100644 --- a/agent/dns/dns_address.go +++ b/agent/dns/dns_address.go @@ -83,5 +83,5 @@ func (a *dnsAddress) IsInternalFQDNOrIP(domain string) bool { // IsExternalFQDN returns true if the address is a FQDN and is external to the domain. func (a *dnsAddress) IsExternalFQDN(domain string) bool { - return !a.IsIP() && a.IsFQDN() && !strings.HasSuffix(a.FQDN(), domain) + return !a.IsIP() && a.IsFQDN() && strings.Count(a.FQDN(), ".") > 1 && !strings.HasSuffix(a.FQDN(), domain) } diff --git a/agent/dns/dns_address_test.go b/agent/dns/dns_address_test.go index bf55c295ba..93460437f2 100644 --- a/agent/dns/dns_address_test.go +++ b/agent/dns/dns_address_test.go @@ -95,6 +95,20 @@ func Test_dnsAddress(t *testing.T) { isInternalFQDNOrIP: true, }, }, + { + name: "server name", + input: "web.", + expectedResults: expectedResults{ + isIp: false, + stringResult: "web.", + fqdn: "web.", + isFQDN: true, + isEmptyString: false, + isExternalFQDN: false, + isInternalFQDN: false, + isInternalFQDNOrIP: false, + }, + }, { name: "external FQDN without trailing period", input: "web.service.vault", diff --git a/agent/dns/query_locality.go b/agent/dns/query_locality.go deleted file mode 100644 index 6ad2ea27bd..0000000000 --- a/agent/dns/query_locality.go +++ /dev/null @@ -1,42 +0,0 @@ -// Copyright (c) HashiCorp, Inc. -// SPDX-License-Identifier: BUSL-1.1 - -package dns - -import "github.com/hashicorp/consul/acl" - -// queryLocality is the locality parsed from a DNS query. -type queryLocality struct { - // datacenter is the datacenter parsed from a label that has an explicit datacenter part. - // Example query: .virtual..ns..ap..dc.consul - datacenter string - - // peer is the peer name parsed from a label that has explicit parts. - // Example query: .virtual..ns..peer..ap.consul - peer string - - // peerOrDatacenter is parsed from DNS queries where the datacenter and peer name are - // specified in the same query part. - // Example query: .virtual..consul - // - // Note that this field should only be a "peer" for virtual queries, since virtual IPs should - // not be shared between datacenters. In all other cases, it should be considered a DC. - peerOrDatacenter string - - acl.EnterpriseMeta -} - -// EffectiveDatacenter returns the datacenter parsed from a query, or a default -// value if none is specified. -func (l queryLocality) EffectiveDatacenter(defaultDC string) string { - // Prefer the value parsed from a query with explicit parts: .ns..ap..dc - if l.datacenter != "" { - return l.datacenter - } - // Fall back to the ambiguously parsed DC or Peer. - if l.peerOrDatacenter != "" { - return l.peerOrDatacenter - } - // If all are empty, use a default value. - return defaultDC -} diff --git a/agent/dns/query_locality_ce.go b/agent/dns/query_locality_ce.go deleted file mode 100644 index 23a080c0df..0000000000 --- a/agent/dns/query_locality_ce.go +++ /dev/null @@ -1,57 +0,0 @@ -// Copyright (c) HashiCorp, Inc. -// SPDX-License-Identifier: BUSL-1.1 - -//go:build !consulent - -package dns - -import ( - "github.com/hashicorp/consul/acl" - "github.com/hashicorp/consul/agent/config" -) - -// ParseLocality can parse peer name or datacenter from a DNS query's labels. -// Peer name is parsed from the same query part that datacenter is, so given this ambiguity -// we parse a "peerOrDatacenter". The caller or RPC handler are responsible for disambiguating. -func ParseLocality(labels []string, defaultEnterpriseMeta acl.EnterpriseMeta, _ enterpriseDNSConfig) (queryLocality, bool) { - locality := queryLocality{ - EnterpriseMeta: defaultEnterpriseMeta, - } - - switch len(labels) { - case 2, 4: - // Support the following formats: - // - [..dc] - // - [..peer] - for i := 0; i < len(labels); i += 2 { - switch labels[i+1] { - case "dc": - locality.datacenter = labels[i] - case "peer": - locality.peer = labels[i] - default: - return queryLocality{}, false - } - } - // Return error when both datacenter and peer are specified. - if locality.datacenter != "" && locality.peer != "" { - return queryLocality{}, false - } - return locality, true - case 1: - return queryLocality{peerOrDatacenter: labels[0]}, true - - case 0: - return queryLocality{}, true - } - - return queryLocality{}, false -} - -// enterpriseDNSConfig is the configuration for enterprise DNS. -type enterpriseDNSConfig struct{} - -// getEnterpriseDNSConfig returns the enterprise DNS configuration. -func getEnterpriseDNSConfig(conf *config.RuntimeConfig) enterpriseDNSConfig { - return enterpriseDNSConfig{} -} diff --git a/agent/dns/query_locality_ce_test.go b/agent/dns/query_locality_ce_test.go deleted file mode 100644 index 59d49ed336..0000000000 --- a/agent/dns/query_locality_ce_test.go +++ /dev/null @@ -1,60 +0,0 @@ -// Copyright (c) HashiCorp, Inc. -// SPDX-License-Identifier: BUSL-1.1 - -//go:build !consulent - -package dns - -import ( - "github.com/hashicorp/consul/acl" -) - -func getTestCases() []testCaseParseLocality { - testCases := []testCaseParseLocality{ - { - name: "test [..dc]", - labels: []string{"test-dc", "dc"}, - enterpriseDNSConfig: enterpriseDNSConfig{}, - expectedResult: queryLocality{ - EnterpriseMeta: acl.EnterpriseMeta{}, - datacenter: "test-dc", - }, - expectedOK: true, - }, - { - name: "test [..peer]", - labels: []string{"test-peer", "peer"}, - enterpriseDNSConfig: enterpriseDNSConfig{}, - expectedResult: queryLocality{ - EnterpriseMeta: acl.EnterpriseMeta{}, - peer: "test-peer", - }, - expectedOK: true, - }, - { - name: "test 1 label", - labels: []string{"test-peer"}, - enterpriseDNSConfig: enterpriseDNSConfig{}, - expectedResult: queryLocality{ - EnterpriseMeta: acl.EnterpriseMeta{}, - peerOrDatacenter: "test-peer", - }, - expectedOK: true, - }, - { - name: "test 0 labels", - labels: []string{}, - enterpriseDNSConfig: enterpriseDNSConfig{}, - expectedResult: queryLocality{}, - expectedOK: true, - }, - { - name: "test 3 labels returns not found", - labels: []string{"test-dc", "dc", "test-blah"}, - enterpriseDNSConfig: enterpriseDNSConfig{}, - expectedResult: queryLocality{}, - expectedOK: false, - }, - } - return testCases -} diff --git a/agent/dns/query_locality_test.go b/agent/dns/query_locality_test.go deleted file mode 100644 index 84008a7a6d..0000000000 --- a/agent/dns/query_locality_test.go +++ /dev/null @@ -1,74 +0,0 @@ -// Copyright (c) HashiCorp, Inc. -// SPDX-License-Identifier: BUSL-1.1 - -package dns - -import ( - "testing" - - "github.com/hashicorp/consul/acl" - "github.com/stretchr/testify/require" -) - -type testCaseParseLocality struct { - name string - labels []string - defaultMeta acl.EnterpriseMeta - enterpriseDNSConfig enterpriseDNSConfig - expectedResult queryLocality - expectedOK bool -} - -func Test_ParseLocality(t *testing.T) { - testCases := getTestCases() - for _, tc := range testCases { - t.Run(tc.name, func(t *testing.T) { - actualResult, actualOK := ParseLocality(tc.labels, tc.defaultMeta, tc.enterpriseDNSConfig) - require.Equal(t, tc.expectedOK, actualOK) - require.Equal(t, tc.expectedResult, actualResult) - - }) - } - -} - -func Test_EffectiveDatacenter(t *testing.T) { - type testCase struct { - name string - queryLocality queryLocality - defaultDC string - expected string - } - testCases := []testCase{ - { - name: "return datacenter first", - queryLocality: queryLocality{ - datacenter: "test-dc", - peerOrDatacenter: "test-peer", - }, - defaultDC: "default-dc", - expected: "test-dc", - }, - { - name: "return peerOrDatacenter second", - queryLocality: queryLocality{ - peerOrDatacenter: "test-peer", - }, - defaultDC: "default-dc", - expected: "test-peer", - }, - { - name: "return defaultDC as fallback", - queryLocality: queryLocality{}, - defaultDC: "default-dc", - expected: "default-dc", - }, - } - - for _, tc := range testCases { - t.Run(tc.name, func(t *testing.T) { - got := tc.queryLocality.EffectiveDatacenter(tc.defaultDC) - require.Equal(t, tc.expected, got) - }) - } -} diff --git a/agent/dns/router.go b/agent/dns/router.go index 39562c90d3..06b1caa026 100644 --- a/agent/dns/router.go +++ b/agent/dns/router.go @@ -195,17 +195,25 @@ func (r *Router) handleRequestRecursively(req *dns.Msg, reqCtx discovery.Context return resp } + // Need to pass the question name to properly support recursion and the + // trimming of the domain suffixes. + qName := dns.CanonicalName(req.Question[0].Name) + if maxRecursionLevel < maxRecursionLevelDefault { + // Get the QName without the domain suffix + qName = r.trimDomain(qName) + } + reqType := parseRequestType(req) - results, query, err := r.getQueryResults(req, reqCtx, reqType, configCtx) + results, query, err := r.getQueryResults(req, reqCtx, reqType, configCtx, qName) switch { case errors.Is(err, errNameNotFound): - r.logger.Error("name not found", "name", req.Question[0].Name) + r.logger.Error("name not found", "name", qName) ecsGlobal := !errors.Is(err, discovery.ErrECSNotGlobal) return createAuthoritativeResponse(req, configCtx, responseDomain, dns.RcodeNameError, ecsGlobal) // TODO (v2-dns): there is another case here where the discovery service returns "name not found" case errors.Is(err, discovery.ErrNoData): - r.logger.Debug("no data available", "name", req.Question[0].Name) + r.logger.Debug("no data available", "name", qName) ecsGlobal := !errors.Is(err, discovery.ErrECSNotGlobal) return createAuthoritativeResponse(req, configCtx, responseDomain, dns.RcodeSuccess, ecsGlobal) @@ -224,6 +232,21 @@ func (r *Router) handleRequestRecursively(req *dns.Msg, reqCtx discovery.Context return resp } +// trimDomain trims the domain from the question name. +func (r *Router) trimDomain(questionName string) string { + longer := r.domain + shorter := r.altDomain + + if len(shorter) > len(longer) { + longer, shorter = shorter, longer + } + + if strings.HasSuffix(questionName, "."+strings.TrimLeft(longer, ".")) { + return strings.TrimSuffix(questionName, longer) + } + return strings.TrimSuffix(questionName, shorter) +} + // getTTLForResult returns the TTL for a given result. func getTTLForResult(name string, query *discovery.Query, cfg *RouterDynamicConfig) uint32 { switch { @@ -243,7 +266,8 @@ func getTTLForResult(name string, query *discovery.Query, cfg *RouterDynamicConf } // getQueryResults returns a discovery.Result from a DNS message. -func (r *Router) getQueryResults(req *dns.Msg, reqCtx discovery.Context, reqType requestType, cfg *RouterDynamicConfig) ([]*discovery.Result, *discovery.Query, error) { +func (r *Router) getQueryResults(req *dns.Msg, reqCtx discovery.Context, + reqType requestType, cfg *RouterDynamicConfig, qName string) ([]*discovery.Result, *discovery.Query, error) { var query *discovery.Query switch reqType { case requestTypeConsul: @@ -272,9 +296,9 @@ func (r *Router) getQueryResults(req *dns.Msg, reqCtx discovery.Context, reqType } return results, query, nil case requestTypeIP: - ip := dnsutil.IPFromARPA(req.Question[0].Name) + ip := dnsutil.IPFromARPA(qName) if ip == nil { - r.logger.Error("error building IP from DNS request", "name", req.Question[0].Name) + r.logger.Error("error building IP from DNS request", "name", qName) return nil, nil, errNameNotFound } results, err := r.processor.QueryByIP(ip, reqCtx) @@ -742,8 +766,10 @@ func buildAddressResults(req *dns.Msg) ([]*discovery.Result, error) { // getAnswerAndExtra creates the dns answer and extra from discovery results. func (r *Router) getAnswerExtraAndNs(result *discovery.Result, req *dns.Msg, reqCtx discovery.Context, - query *discovery.Query, cfg *RouterDynamicConfig, domain string, remoteAddress net.Addr, maxRecursionLevel int) (answer []dns.RR, extra []dns.RR, ns []dns.RR) { - address, target := getAddressAndTargetFromDiscoveryResult(result, r.domain) + query *discovery.Query, cfg *RouterDynamicConfig, domain string, remoteAddress net.Addr, + maxRecursionLevel int) (answer []dns.RR, extra []dns.RR, ns []dns.RR) { + target := newDNSAddress(result.Target) + address := newDNSAddress(result.Address) qName := req.Question[0].Name ttlLookupName := qName if query != nil { @@ -781,17 +807,20 @@ func (r *Router) getAnswerExtraAndNs(result *discovery.Result, req *dns.Msg, req case qType == dns.TypeSRV: // We put A/AAAA/CNAME records in the additional section for SRV requests a, e := r.getAnswerExtrasForAddressAndTarget(address, target, req, reqCtx, - result, ttl, remoteAddress, maxRecursionLevel) + result, ttl, remoteAddress, cfg, maxRecursionLevel) answer = append(answer, a...) extra = append(extra, e...) - cfg := r.dynamicConfig.Load().(*RouterDynamicConfig) if cfg.NodeMetaTXT { - extra = append(extra, makeTXTRecord(target.FQDN(), result, ttl)...) + name := target.FQDN() + if !target.IsInternalFQDN(r.domain) && !target.IsExternalFQDN(r.domain) { + name = canonicalNameForResult(result, r.domain) + } + extra = append(extra, makeTXTRecord(name, result, ttl)...) } default: a, e := r.getAnswerExtrasForAddressAndTarget(address, target, req, reqCtx, - result, ttl, remoteAddress, maxRecursionLevel) + result, ttl, remoteAddress, cfg, maxRecursionLevel) answer = append(answer, a...) extra = append(extra, e...) } @@ -801,65 +830,74 @@ func (r *Router) getAnswerExtraAndNs(result *discovery.Result, req *dns.Msg, req // getAnswerExtrasForAddressAndTarget creates the dns answer and extra from address and target dnsAddress pairs. func (r *Router) getAnswerExtrasForAddressAndTarget(address *dnsAddress, target *dnsAddress, req *dns.Msg, reqCtx discovery.Context, result *discovery.Result, ttl uint32, remoteAddress net.Addr, - maxRecursionLevel int) (answer []dns.RR, extra []dns.RR) { + cfg *RouterDynamicConfig, maxRecursionLevel int) (answer []dns.RR, extra []dns.RR) { qName := req.Question[0].Name reqType := parseRequestType(req) - cfg := r.dynamicConfig.Load().(*RouterDynamicConfig) switch { + // Virtual IPs and Address requests + // both return IPs with empty targets + case (reqType == requestTypeAddress || result.Type == discovery.ResultTypeVirtual) && + target.IsEmptyString() && address.IsIP(): + a, e := getAnswerExtrasForIP(qName, address, req.Question[0], reqType, + result, ttl) + answer = append(a, answer...) + extra = append(e, extra...) - // There is no target and the address is a FQDN (external service) + // Address is a FQDN and requires a CNAME lookup. case address.IsFQDN(): a, e := r.makeRecordFromFQDN(address.FQDN(), result, req, reqCtx, cfg, ttl, remoteAddress, maxRecursionLevel) answer = append(a, answer...) extra = append(e, extra...) - // The target is a FQDN (internal or external service name) - case result.Type != discovery.ResultTypeNode && target.IsFQDN(): - a, e := r.makeRecordFromFQDN(target.FQDN(), result, req, reqCtx, - cfg, ttl, remoteAddress, maxRecursionLevel) - answer = append(answer, a...) - extra = append(extra, e...) - - // There is no target and the address is an IP - case address.IsIP(): - // TODO (v2-dns): Do not CNAME node address in case of WAN address. - ipRecordName := target.FQDN() - if maxRecursionLevel < maxRecursionLevelDefault || ipRecordName == "" { - ipRecordName = qName + // Target is FQDN that point to IP + case target.IsFQDN() && address.IsIP(): + var a, e []dns.RR + if result.Type == discovery.ResultTypeNode { + // if it is a node record it means the service address pointed to a node + // and the node address was used. So we create an A record for the node address, + // as well as a CNAME for the service to node mapping. + name := target.FQDN() + if !target.IsInternalFQDN(r.domain) && !target.IsExternalFQDN(r.domain) { + name = canonicalNameForResult(result, r.domain) + } else if target.IsInternalFQDN(r.domain) { + answer = append(answer, makeCNAMERecord(qName, canonicalNameForResult(result, r.domain), ttl)) + } + a, e = getAnswerExtrasForIP(name, address, req.Question[0], reqType, + result, ttl) + } else { + // if it is a service record, it means that the service address had the IP directly + // and there was not a need for an intermediate CNAME. + a, e = getAnswerExtrasForIP(qName, address, req.Question[0], reqType, + result, ttl) } - a, e := getAnswerExtrasForIP(ipRecordName, address, req.Question[0], reqType, result, ttl) - answer = append(answer, a...) - extra = append(extra, e...) - - // The target is an IP - case target.IsIP(): - a, e := getAnswerExtrasForIP(qName, target, req.Question[0], reqType, result, ttl) - answer = append(answer, a...) - extra = append(extra, e...) - - // The target is a CNAME for the service we are looking - // for. So we use the address. - case target.FQDN() == req.Question[0].Name && address.IsIP(): - a, e := getAnswerExtrasForIP(qName, address, req.Question[0], reqType, result, ttl) answer = append(answer, a...) extra = append(extra, e...) // The target is a FQDN (internal or external service name) default: - a, e := r.makeRecordFromFQDN(target.FQDN(), result, req, reqCtx, cfg, ttl, remoteAddress, maxRecursionLevel) + a, e := r.makeRecordFromFQDN(target.FQDN(), result, req, reqCtx, cfg, + ttl, remoteAddress, maxRecursionLevel) answer = append(a, answer...) extra = append(e, extra...) } return } -// getAddressAndTargetFromDiscoveryResult returns the address and target from a discovery result. -func getAnswerExtrasForIP(name string, addr *dnsAddress, question dns.Question, reqType requestType, result *discovery.Result, ttl uint32) (answer []dns.RR, extra []dns.RR) { - record := makeIPBasedRecord(name, addr, ttl) +// getAnswerExtrasForIP creates the dns answer and extra from IP dnsAddress pairs. +func getAnswerExtrasForIP(name string, addr *dnsAddress, question dns.Question, + reqType requestType, result *discovery.Result, ttl uint32) (answer []dns.RR, extra []dns.RR) { qType := question.Qtype + // Have to pass original question name here even if the system has recursed + // and stripped off the domain suffix. + recHdrName := question.Name + if qType == dns.TypeSRV { + recHdrName = name + } + record := makeIPBasedRecord(recHdrName, addr, ttl) + isARecordWhenNotExplicitlyQueried := record.Header().Rrtype == dns.TypeA && qType != dns.TypeA && qType != dns.TypeANY isAAAARecordWhenNotExplicitlyQueried := record.Header().Rrtype == dns.TypeAAAA && qType != dns.TypeAAAA && qType != dns.TypeANY @@ -970,7 +1008,7 @@ MORE_REC: } answers := []dns.RR{ - makeCNAMERecord(result, q.Name, ttl), + makeCNAMERecord(q.Name, result.Target, ttl), } answers = append(answers, additional...) @@ -978,15 +1016,15 @@ MORE_REC: } // makeCNAMERecord returns a CNAME record for the given name and target. -func makeCNAMERecord(result *discovery.Result, qName string, ttl uint32) *dns.CNAME { +func makeCNAMERecord(name string, target string, ttl uint32) *dns.CNAME { return &dns.CNAME{ Hdr: dns.RR_Header{ - Name: qName, + Name: name, Rrtype: dns.TypeCNAME, Class: dns.ClassINET, Ttl: ttl, }, - Target: dns.Fqdn(result.Target), + Target: dns.Fqdn(target), } } @@ -1047,13 +1085,3 @@ func makeTXTRecord(name string, result *discovery.Result, ttl uint32) []dns.RR { } return extra } - -// getAddressAndTargetFromCheckServiceNode returns the address and target for a given discovery.Result -func getAddressAndTargetFromDiscoveryResult(result *discovery.Result, domain string) (*dnsAddress, *dnsAddress) { - target := newDNSAddress(result.Target) - if !target.IsEmptyString() && !target.IsInternalFQDNOrIP(domain) { - target.SetAddress(canonicalNameForResult(result, domain)) - } - address := newDNSAddress(result.Address) - return address, target -}