status-im
/
consul
mirror of https://github.com/status-im/consul.git
2
0
Fork 0
Code Issues Projects Releases Wiki Activity

fix(v2dns): add node ttl to workloads, comment cleanup, and changelog (#20643) 

* fix(v2dns): add node ttl to workloads, plus comment cleanup

* docs(v2dns): changelog
This commit is contained in:
Dan Stough 2024-02-14 17:38:11 -05:00 committed by GitHub
parent 9f7626d501
commit 14efb28086
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
12 changed files with 34 additions and 47 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
.changelog/20643.txt Normal file
View File

@ -0,0 +1,7 @@
```release-note:feature
dns: adds experimental support for a refactored DNS server that is v1 and v2 Catalog compatible.
Use `v2dns` in the `experiments` agent config to enable.
It will automatically be enabled when using the `resource-apis` (Catalog v2) experiment.
The new DNS implementation will be the default in Consul 1.19.
See the [Consul 1.18.x Release Notes](https://developer.hashicorp.com/consul/docs/release-notes/consul/v1_18_x) for deprecated DNS features.
```

1
agent/discovery/discovery.go
View File

@ -20,7 +20,6 @@ var (
// ECSNotGlobalError may be used to wrap an error or nil, to indicate that the
// EDNS client subnet source scope is not global.
// TODO (v2-dns): prepared queries errors are wrapped by this
type ECSNotGlobalError struct {
error
}

3
agent/discovery/query_fetcher_v1.go
View File

@ -58,7 +58,6 @@ type v1DataFetcherDynamicConfig struct {
// V1DataFetcher is used to fetch data from the V1 catalog.
type V1DataFetcher struct {
// TODO(v2-dns): store this in the config.
defaultEnterpriseMeta acl.EnterpriseMeta
dynamicConfig atomic.Value
logger hclog.Logger
@ -275,7 +274,7 @@ func (f *V1DataFetcher) FetchRecordsByIp(reqCtx Context, ip net.IP) ([]*Result,
}
// nothing found locally, recurse
// TODO: (v2-dns) implement recursion
// TODO: (v2-dns) implement recursion (NET-7883)
//d.handleRecurse(resp, req)
return nil, fmt.Errorf("unhandled error in FetchRecordsByIp")

2
agent/discovery/query_fetcher_v1_test.go
View File

@ -103,7 +103,6 @@ func Test_FetchVirtualIP(t *testing.T) {
*reply = tc.expectedResult.Service.Address
}
})
// TODO (v2-dns): mock these properly
translateServicePortFunc := func(dc string, port int, taggedAddresses map[string]structs.ServiceAddress) int { return 0 }
rpcFuncForServiceNodes := func(ctx context.Context, req structs.ServiceSpecificRequest) (structs.IndexedCheckServiceNodes, cache.ResultMeta, error) {
return structs.IndexedCheckServiceNodes{}, cache.ResultMeta{}, nil
@ -166,7 +165,6 @@ func Test_FetchEndpoints(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

6
agent/dns/parser_test.go
View File

@ -1,6 +0,0 @@
// Copyright (c) HashiCorp, Inc.
// SPDX-License-Identifier: BUSL-1.1
package dns
// TODO (v2-dns): parser tests

15
agent/dns/router.go
View File

@ -59,7 +59,6 @@ type Context struct {
type RouterDynamicConfig struct {
ARecordLimit int
DisableCompression bool
EnableDefaultFailover bool // TODO (v2-dns): plumbing required for this new V2 setting. This is the agent configured default
EnableTruncate bool
NodeMetaTXT bool
NodeTTL time.Duration
@ -331,13 +330,6 @@ func getTTLForResult(name string, overrideTTL *uint32, query *discovery.Query, c
}
switch query.QueryType {
// TODO (v2-dns): currently have to do this related to the results type being changed to node whe
// the v1 data fetcher encounters a blank service address and uses the node address instead.
// we will revisiting this when look at modifying the discovery result struct to
// possibly include additional metadata like the node address.
case discovery.QueryTypeWorkload:
// TODO (v2-dns): we need to discuss what we want to do for workload TTLs
return 0
case discovery.QueryTypeService, discovery.QueryTypePreparedQuery:
ttl, ok := cfg.getTTLForService(name)
if ok {
@ -554,7 +546,6 @@ func (r *Router) serializeQueryResults(req *dns.Msg, reqCtx Context,
// The datacenter should be empty during translation if it is a peering lookup.
// This should be fine because we should always prefer the WAN address.
// TODO (v2-dns): this needs a clean up so we're not assuming this everywhere.
address := ""
if result.Service != nil {
address = result.Service.Address
@ -982,22 +973,20 @@ func (r *Router) getAnswerExtraAndNs(result *discovery.Result, port discovery.Po
}
answer = append(answer, ptr)
case qType == dns.TypeNS:
// TODO (v2-dns): fqdn in V1 has the datacenter included, this would need to be added to discovery.Result
resultType := result.Type
target := result.Node.Name
if parseRequestType(req) == requestTypeConsul && resultType == discovery.ResultTypeService {
resultType = discovery.ResultTypeNode
}
fqdn := canonicalNameForResult(resultType, target, domain, result.Tenancy, port.Name)
extraRecord := makeIPBasedRecord(fqdn, nodeAddress, ttl) // TODO (v2-dns): this is not sufficient, because recursion and CNAMES are supported
extraRecord := makeIPBasedRecord(fqdn, nodeAddress, ttl)
answer = append(answer, makeNSRecord(domain, fqdn, ttl))
extra = append(extra, extraRecord)
case qType == dns.TypeSOA:
// TODO (v2-dns): fqdn in V1 has the datacenter included, this would need to be added to discovery.Result
// to be returned in the result.
fqdn := canonicalNameForResult(result.Type, result.Node.Name, domain, result.Tenancy, port.Name)
extraRecord := makeIPBasedRecord(fqdn, nodeAddress, ttl) // TODO (v2-dns): this is not sufficient, because recursion and CNAMES are supported
extraRecord := makeIPBasedRecord(fqdn, nodeAddress, ttl)
ns = append(ns, makeNSRecord(domain, fqdn, ttl))
extra = append(extra, extraRecord)

14
agent/dns/router_test.go
View File

@ -1874,6 +1874,7 @@ func Test_HandleRequest(t *testing.T) {
Name: "api.port.foo.workload.consul.",
Rrtype: dns.TypeA,
Class: dns.ClassINET,
Ttl: 123,
},
A: net.ParseIP("1.2.3.4"),
},
@ -1932,6 +1933,7 @@ func Test_HandleRequest(t *testing.T) {
Name: "foo.workload.consul.",
Rrtype: dns.TypeA,
Class: dns.ClassINET,
Ttl: 123,
},
A: net.ParseIP("1.2.3.4"),
},
@ -1994,6 +1996,7 @@ func Test_HandleRequest(t *testing.T) {
Name: "foo.workload.bar.ns.baz.ap.dc3.dc.consul.",
Rrtype: dns.TypeA,
Class: dns.ClassINET,
Ttl: 123,
},
A: net.ParseIP("1.2.3.4"),
},
@ -2058,6 +2061,7 @@ func Test_HandleRequest(t *testing.T) {
Name: "api.port.foo.workload.consul.",
Rrtype: dns.TypeCNAME,
Class: dns.ClassINET,
Ttl: 123,
},
Target: "foo.example.com.",
},
@ -2165,6 +2169,7 @@ func Test_HandleRequest(t *testing.T) {
Name: "api.port.foo.workload.consul.",
Rrtype: dns.TypeCNAME,
Class: dns.ClassINET,
Ttl: 123,
},
Target: "foo.example.com.",
},
@ -2173,6 +2178,7 @@ func Test_HandleRequest(t *testing.T) {
Name: "foo.example.com.",
Rrtype: dns.TypeA,
Class: dns.ClassINET,
Ttl: 123,
},
A: net.ParseIP("1.2.3.4"),
},
@ -2280,15 +2286,17 @@ func Test_HandleRequest(t *testing.T) {
Name: "api.port.foo.workload.consul.",
Rrtype: dns.TypeCNAME,
Class: dns.ClassINET,
Ttl: 123,
},
Target: "foo.example.com.",
},
// TODO (v2-dns): this next record is wrong per the RFC
// TODO (v2-dns): this next record is wrong per the RFC-1034 mentioned in the comment above (NET-8060)
&dns.A{
Hdr: dns.RR_Header{
Name: "foo.example.com.",
Rrtype: dns.TypeCNAME,
Class: dns.ClassINET,
Ttl: 123,
},
A: net.ParseIP("1.2.3.4"),
},
@ -2383,7 +2391,7 @@ func Test_HandleRequest(t *testing.T) {
Answer: []dns.RR{
&dns.A{
Hdr: dns.RR_Header{
Name: "foo.service.consul.", // TODO (v2-dns): verify this shouldn't include tenancy for workloads
Name: "foo.service.consul.",
Rrtype: dns.TypeA,
Class: dns.ClassINET,
Ttl: uint32(123),
@ -2882,7 +2890,7 @@ func Test_HandleRequest(t *testing.T) {
},
A: net.ParseIP("1.2.3.4"),
},
// TODO (v2-dns): This needs to be de-dupped
// TODO (v2-dns): This needs to be de-duplicated (NET-8064)
&dns.A{
Hdr: dns.RR_Header{
Name: "foo-1.example.com.",

6
agent/dns/server_test.go
View File

@ -1,6 +0,0 @@
// Copyright (c) HashiCorp, Inc.
// SPDX-License-Identifier: BUSL-1.1
package dns
// TODO (v2-dns): add at least one test to listen and serve with a dummy router.

3
agent/dns_node_lookup_test.go
View File

@ -155,8 +155,7 @@ func TestDNS_NodeLookup_CaseInsensitive(t *testing.T) {
}
}
// TODO (v2-dns): NET-7632 - Fix node lookup when question name has a period in it.
// Answer is coming back empty
// V2 DNS does not support node names with a period. This will be deprecated.
func TestDNS_NodeLookup_PeriodName(t *testing.T) {
if testing.Short() {
t.Skip("too slow for testing.Short")

2
agent/dns_service_lookup_test.go
View File

@ -1981,7 +1981,7 @@ func TestDNS_ServiceLookup_CaseInsensitive(t *testing.T) {
}
}
// TODO (v2-dns): NET-7632 - Fix node and prepared query lookups when question name has a period in it
// V2 DNS: we have deprecated support for service tags w/ periods
func TestDNS_ServiceLookup_TagPeriod(t *testing.T) {
if testing.Short() {
t.Skip("too slow for testing.Short")

2
agent/dns_test.go
View File

@ -3623,7 +3623,7 @@ func TestDNS_V1ConfigReload(t *testing.T) {
}
// TODO (v2-dns) add a test for checking the V2 DNS Server reloads the config
// TODO (v2-dns) add a test for checking the V2 DNS Server reloads the config (NET-8056)
func TestDNS_ReloadConfig_DuringQuery(t *testing.T) {
if testing.Short() {

2
agent/grpc-external/services/dns/server_v2.go
View File

@ -72,7 +72,7 @@ func (s *ServerV2) Query(ctx context.Context, req *pbdns.QueryRequest) (*pbdns.Q
return nil, status.Error(codes.Internal, fmt.Sprintf("failure decoding dns request: %s", err.Error()))
}
// TODO (v2-dns): parse token and other context metadata from the grpc request/metadata
// TODO (v2-dns): parse token and other context metadata from the grpc request/metadata (NET-7885)
reqCtx := agentdns.Context{
Token: s.TokenFunc(),
}