From 45abe96d0e7d99d817425aa8764f4e938e54916e Mon Sep 17 00:00:00 2001 From: hc-github-team-consul-core Date: Thu, 15 Aug 2024 14:44:19 -0400 Subject: [PATCH] Backport of Fix TestDNS_ServiceLookup_ARecordLimits so that it only creates test agents the minimal amount of time into release/1.19.x (#21612) * backport of commit 0ca8552d02ca5c112540e92f61716b7460695be8 * backport of commit ff2713b2386447fd3149a5509c9e5ba940652682 * backport of commit f8c8d0ac137230e763d308beb7acb00cc29d151a * backport of commit da2c4499f544822f91b16c405e099e641f41bc09 * backport of commit 38e824afb5863428d9477fd00ac3f5f6e818d3ed * backport of commit 8cf76bd353f43249e248546a3ed85b7dc2d3cad7 --------- Co-authored-by: John Murret --- agent/dns_service_lookup_test.go | 359 ++++++++++++++----------------- 1 file changed, 167 insertions(+), 192 deletions(-) diff --git a/agent/dns_service_lookup_test.go b/agent/dns_service_lookup_test.go index ef2a9dbe6d..34a48b75ba 100644 --- a/agent/dns_service_lookup_test.go +++ b/agent/dns_service_lookup_test.go @@ -2919,62 +2919,8 @@ func TestDNS_ServiceLookup_LargeResponses(t *testing.T) { } } -func testDNSServiceLookupResponseLimits(t *testing.T, answerLimit int, qType uint16, - expectedService, expectedQuery, expectedQueryID int, additionalHCL string) (bool, error) { - a := NewTestAgent(t, ` - node_name = "test-node" - dns_config { - udp_answer_limit = `+fmt.Sprintf("%d", answerLimit)+` - } - `+additionalHCL) - defer a.Shutdown() - testrpc.WaitForTestAgent(t, a.RPC, "dc1") - - choices := perfectlyRandomChoices(generateNumNodes, pctNodesWithIPv6) - for i := 0; i < generateNumNodes; i++ { - nodeAddress := fmt.Sprintf("127.0.0.%d", i+1) - if choices[i] { - nodeAddress = fmt.Sprintf("fe80::%d", i+1) - } - args := &structs.RegisterRequest{ - Datacenter: "dc1", - Node: fmt.Sprintf("foo%d", i), - Address: nodeAddress, - Service: &structs.NodeService{ - Service: "api-tier", - Port: 8080, - }, - } - - var out struct{} - if err := a.RPC(context.Background(), "Catalog.Register", args, &out); err != nil { - return false, fmt.Errorf("err: %v", err) - } - } - var id string - { - args := &structs.PreparedQueryRequest{ - Datacenter: "dc1", - Op: structs.PreparedQueryCreate, - Query: &structs.PreparedQuery{ - Name: "api-tier", - Service: structs.ServiceQuery{ - Service: "api-tier", - }, - }, - } - - if err := a.RPC(context.Background(), "PreparedQuery.Apply", args, &id); err != nil { - return false, fmt.Errorf("err: %v", err) - } - } - - // Look up the service directly and via prepared query. - questions := []string{ - "api-tier.service.consul.", - "api-tier.query.consul.", - id + ".query.consul.", - } +func testDNSServiceLookupResponseLimits(questions []string, answerLimit int, qType uint16, + expectedService, expectedQuery, expectedQueryID int, a *TestAgent) (bool, error) { for idx, question := range questions { m := new(dns.Msg) m.SetQuestion(question, qType) @@ -3011,78 +2957,24 @@ func testDNSServiceLookupResponseLimits(t *testing.T, answerLimit int, qType uin func checkDNSService( t *testing.T, - generateNumNodes int, - aRecordLimit int, + protocol string, + questions []string, + a *TestAgent, qType uint16, expectedResultsCount int, udpSize uint16, + setEDNS0 bool, ) { - a := NewTestAgent(t, ` - node_name = "test-node" - dns_config { - a_record_limit = `+fmt.Sprintf("%d", aRecordLimit)+` - udp_answer_limit = `+fmt.Sprintf("%d", aRecordLimit)+` - } - `) - defer a.Shutdown() - testrpc.WaitForTestAgent(t, a.RPC, "dc1") - - choices := perfectlyRandomChoices(generateNumNodes, pctNodesWithIPv6) - for i := 0; i < generateNumNodes; i++ { - nodeAddress := fmt.Sprintf("127.0.0.%d", i+1) - if choices[i] { - nodeAddress = fmt.Sprintf("fe80::%d", i+1) - } - args := &structs.RegisterRequest{ - Datacenter: "dc1", - Node: fmt.Sprintf("foo%d", i), - Address: nodeAddress, - Service: &structs.NodeService{ - Service: "api-tier", - Port: 8080, - }, - } - - var out struct{} - require.NoError(t, a.RPC(context.Background(), "Catalog.Register", args, &out)) - } - var id string - { - args := &structs.PreparedQueryRequest{ - Datacenter: "dc1", - Op: structs.PreparedQueryCreate, - Query: &structs.PreparedQuery{ - Name: "api-tier", - Service: structs.ServiceQuery{ - Service: "api-tier", - }, - }, - } - - require.NoError(t, a.RPC(context.Background(), "PreparedQuery.Apply", args, &id)) - } - - // Look up the service directly and via prepared query. - questions := []string{ - "api-tier.service.consul.", - "api-tier.query.consul.", - id + ".query.consul.", - } for _, question := range questions { - question := question t.Run("question: "+question, func(t *testing.T) { m := new(dns.Msg) m.SetQuestion(question, qType) - protocol := "tcp" - if udpSize > 0 { - protocol = "udp" - } - if udpSize > 512 { + if setEDNS0 { m.SetEdns0(udpSize, true) } - c := &dns.Client{Net: protocol, UDPSize: 8192} + c := &dns.Client{Net: protocol, UDPSize: udpSize} in, _, err := c.Exchange(m, a.DNSAddr()) require.NoError(t, err) @@ -3094,82 +2986,154 @@ func checkDNSService( } } +func registerServicesAndPreparedQuery(t *testing.T, generateNumNodes int, a *TestAgent, serviceUniquenessKey string) []string { + choices := perfectlyRandomChoices(generateNumNodes, pctNodesWithIPv6) + serviceName := fmt.Sprintf("api-tier-%s", serviceUniquenessKey) + for i := 0; i < generateNumNodes; i++ { + nodeAddress := fmt.Sprintf("127.0.0.%d", i+1) + if choices[i] { + nodeAddress = fmt.Sprintf("fe80::%d", i+1) + } + args := &structs.RegisterRequest{ + Datacenter: "dc1", + Node: fmt.Sprintf("foo%d", i), + Address: nodeAddress, + Service: &structs.NodeService{ + Service: serviceName, + Port: 8080, + }, + } + + var out struct{} + require.NoError(t, a.RPC(context.Background(), "Catalog.Register", args, &out)) + } + var preparedQueryID string + { + args := &structs.PreparedQueryRequest{ + Datacenter: "dc1", + Op: structs.PreparedQueryCreate, + Query: &structs.PreparedQuery{ + Name: serviceName, + Service: structs.ServiceQuery{ + Service: serviceName, + }, + }, + } + + require.NoError(t, a.RPC(context.Background(), "PreparedQuery.Apply", args, &preparedQueryID)) + } + + // Look up the service directly and via prepared query. + questions := []string{ + fmt.Sprintf("%s.service.consul.", serviceName), + fmt.Sprintf("%s.query.consul.", serviceName), + preparedQueryID + ".query.consul.", + } + return questions +} + func TestDNS_ServiceLookup_ARecordLimits(t *testing.T) { if testing.Short() { t.Skip("too slow for testing.Short") } + const ( + UDP = "udp" + TCP = "tcp" + ) - tests := []struct { - name string - aRecordLimit int - expectedAResults int - expectedAAAAResults int - expectedANYResults int - expectedSRVResults int - numNodesTotal int - udpSize uint16 - _unused_udpAnswerLimit int // NOTE: this field is not used - }{ + type testCase struct { + protocol string + aRecordLimit int + expectedAResults int + expectedAAAAResults int + expectedANYResults int + expectedSRVResults int + numNodesTotal int + udpSize uint16 + setEDNS0 bool + } + + type aRecordLimit struct { + name string + limit int + } + + tests := map[string]testCase{ // UDP + EDNS - {"udp-edns-1", 1, 1, 1, 1, 30, 30, 8192, 3}, - {"udp-edns-2", 2, 2, 2, 2, 30, 30, 8192, 3}, - {"udp-edns-3", 3, 3, 3, 3, 30, 30, 8192, 3}, - {"udp-edns-4", 4, 4, 4, 4, 30, 30, 8192, 3}, - {"udp-edns-5", 5, 5, 5, 5, 30, 30, 8192, 3}, - {"udp-edns-6", 6, 6, 6, 6, 30, 30, 8192, 3}, - {"udp-edns-max", 6, 2, 1, 3, 3, 3, 8192, 3}, - // All UDP without EDNS have a limit of 2 answers due to udpAnswerLimit - // Even SRV records are limit to 2 records - {"udp-limit-1", 1, 1, 0, 1, 1, 1, 512, 2}, - {"udp-limit-2", 2, 1, 1, 2, 2, 2, 512, 2}, - // AAAA results limited by size of payload - {"udp-limit-3", 3, 1, 1, 2, 2, 2, 512, 2}, - {"udp-limit-4", 4, 1, 1, 2, 2, 2, 512, 2}, - {"udp-limit-5", 5, 1, 1, 2, 2, 2, 512, 2}, - {"udp-limit-6", 6, 1, 1, 2, 2, 2, 512, 2}, - {"udp-limit-max", 6, 1, 1, 2, 2, 2, 512, 2}, + "udp-edns-1": {UDP, 1, 1, 1, 1, 30, 30, 8192, true}, + "udp-edns-2": {UDP, 2, 2, 2, 2, 30, 30, 8192, true}, + "udp-edns-3": {UDP, 3, 3, 3, 3, 30, 30, 8192, true}, + "udp-edns-4": {UDP, 4, 4, 4, 4, 30, 30, 8192, true}, + "udp-edns-5": {UDP, 5, 5, 5, 5, 30, 30, 8192, true}, + "udp-edns-6": {UDP, 6, 6, 6, 6, 30, 30, 8192, true}, + "udp-edns-max": {UDP, 6, 2, 1, 3, 3, 3, 8192, true}, // All UDP without EDNS and no udpAnswerLimit // Size of records is limited by UDP payload - {"udp-1", 1, 1, 0, 1, 1, 1, 512, 0}, - {"udp-2", 2, 1, 1, 2, 2, 2, 512, 0}, - {"udp-3", 3, 1, 1, 2, 2, 2, 512, 0}, - {"udp-4", 4, 1, 1, 2, 2, 2, 512, 0}, - {"udp-5", 5, 1, 1, 2, 2, 2, 512, 0}, - {"udp-6", 6, 1, 1, 2, 2, 2, 512, 0}, + "udp-1": {UDP, 1, 1, 0, 1, 1, 1, 512, false}, + "udp-2": {UDP, 2, 1, 1, 2, 2, 2, 512, false}, + "udp-3": {UDP, 3, 1, 1, 2, 2, 2, 512, false}, + "udp-4": {UDP, 4, 1, 1, 2, 2, 2, 512, false}, + "udp-5": {UDP, 5, 1, 1, 2, 2, 2, 512, false}, + "udp-6": {UDP, 6, 1, 1, 2, 2, 2, 512, false}, // Only 3 A and 3 SRV records on 512 bytes - {"udp-max", 6, 1, 1, 2, 2, 2, 512, 0}, + "udp-max": {UDP, 6, 1, 1, 2, 2, 2, 512, false}, - {"tcp-1", 1, 1, 1, 1, 30, 30, 0, 0}, - {"tcp-2", 2, 2, 2, 2, 30, 30, 0, 0}, - {"tcp-3", 3, 3, 3, 3, 30, 30, 0, 0}, - {"tcp-4", 4, 4, 4, 4, 30, 30, 0, 0}, - {"tcp-5", 5, 5, 5, 5, 30, 30, 0, 0}, - {"tcp-6", 6, 6, 6, 6, 30, 30, 0, 0}, - {"tcp-max", 6, 1, 1, 2, 2, 2, 0, 0}, + "tcp-1": {TCP, 1, 1, 1, 1, 30, 30, 0, false}, + "tcp-2": {TCP, 2, 2, 2, 2, 30, 30, 0, false}, + "tcp-3": {TCP, 3, 3, 3, 3, 30, 30, 0, false}, + "tcp-4": {TCP, 4, 4, 4, 4, 30, 30, 0, false}, + "tcp-5": {TCP, 5, 5, 5, 5, 30, 30, 0, false}, + "tcp-6": {TCP, 6, 6, 6, 6, 30, 30, 0, false}, + "tcp-max": {TCP, 6, 1, 1, 2, 2, 2, 0, false}, } - for _, test := range tests { - test := test // capture loop var + for _, recordLimit := range []aRecordLimit{ + {"1", 1}, + {"2", 2}, + {"3", 3}, + {"4", 4}, + {"5", 5}, + {"6", 6}, + {"max", 6}, + } { + t.Run("record-limit-"+recordLimit.name, func(t *testing.T) { + a := NewTestAgent(t, ` + node_name = "test-node" + dns_config { + a_record_limit = `+fmt.Sprintf("%d", recordLimit.limit)+` + udp_answer_limit = `+fmt.Sprintf("%d", recordLimit.limit)+` + } + `) - t.Run(test.name, func(t *testing.T) { + defer a.Shutdown() + testrpc.WaitForTestAgent(t, a.RPC, "dc1") - // All those queries should have at max queriesLimited elements + for _, testName := range []string{ + fmt.Sprintf("udp-edns-%s", recordLimit.name), + fmt.Sprintf("udp-%s", recordLimit.name), + fmt.Sprintf("tcp-%s", recordLimit.name), + } { + t.Run(testName, func(t *testing.T) { + test := tests[testName] + questions := registerServicesAndPreparedQuery(t, test.numNodesTotal, a, testName) - t.Run("A", func(t *testing.T) { - checkDNSService(t, test.numNodesTotal, test.aRecordLimit, dns.TypeA, test.expectedAResults, test.udpSize) - }) + t.Run("A", func(t *testing.T) { + checkDNSService(t, test.protocol, questions, a, dns.TypeA, test.expectedAResults, test.udpSize, test.setEDNS0) + }) - t.Run("AAAA", func(t *testing.T) { - checkDNSService(t, test.numNodesTotal, test.aRecordLimit, dns.TypeAAAA, test.expectedAAAAResults, test.udpSize) - }) + t.Run("AAAA", func(t *testing.T) { + checkDNSService(t, test.protocol, questions, a, dns.TypeAAAA, test.expectedAAAAResults, test.udpSize, test.setEDNS0) + }) - t.Run("ANY", func(t *testing.T) { - checkDNSService(t, test.numNodesTotal, test.aRecordLimit, dns.TypeANY, test.expectedANYResults, test.udpSize) - }) + t.Run("ANY", func(t *testing.T) { + checkDNSService(t, test.protocol, questions, a, dns.TypeANY, test.expectedANYResults, test.udpSize, test.setEDNS0) + }) - // No limits but the size of records for SRV records, since not subject to randomization issues - t.Run("SRV", func(t *testing.T) { - checkDNSService(t, test.expectedSRVResults, test.aRecordLimit, dns.TypeSRV, test.numNodesTotal, test.udpSize) - }) + // No limits but the size of records for SRV records, since not subject to randomization issues + t.Run("SRV", func(t *testing.T) { + checkDNSService(t, test.protocol, questions, a, dns.TypeSRV, test.numNodesTotal, test.udpSize, test.setEDNS0) + }) + }) + } }) } } @@ -3217,25 +3181,36 @@ func TestDNS_ServiceLookup_AnswerLimits(t *testing.T) { } for _, test := range tests { test := test // capture loop var - t.Run(fmt.Sprintf("A lookup %v", test), func(t *testing.T) { - ok, err := testDNSServiceLookupResponseLimits(t, test.udpAnswerLimit, dns.TypeA, test.expectedAService, test.expectedAQuery, test.expectedAQueryID, "") - if !ok { - t.Fatalf("Expected service A lookup %s to pass: %v", test.name, err) - } - }) + t.Run(fmt.Sprintf("answer-limit-%s", test.name), func(t *testing.T) { + a := NewTestAgent(t, fmt.Sprintf(` + node_name = "test-node" + dns_config { + udp_answer_limit = %d + }`, test.udpAnswerLimit)) + defer a.Shutdown() + testrpc.WaitForTestAgent(t, a.RPC, "dc1") + questions := registerServicesAndPreparedQuery(t, generateNumNodes, a, test.name) - t.Run(fmt.Sprintf("AAAA lookup %v", test), func(t *testing.T) { - ok, err := testDNSServiceLookupResponseLimits(t, test.udpAnswerLimit, dns.TypeAAAA, test.expectedAAAAService, test.expectedAAAAQuery, test.expectedAAAAQueryID, "") - if !ok { - t.Fatalf("Expected service AAAA lookup %s to pass: %v", test.name, err) - } - }) + t.Run(fmt.Sprintf("A lookup %v", test), func(t *testing.T) { + ok, err := testDNSServiceLookupResponseLimits(questions, test.udpAnswerLimit, dns.TypeA, test.expectedAService, test.expectedAQuery, test.expectedAQueryID, a) + if !ok { + t.Fatalf("Expected service A lookup %s to pass: %v", test.name, err) + } + }) - t.Run(fmt.Sprintf("ANY lookup %v", test), func(t *testing.T) { - ok, err := testDNSServiceLookupResponseLimits(t, test.udpAnswerLimit, dns.TypeANY, test.expectedANYService, test.expectedANYQuery, test.expectedANYQueryID, "") - if !ok { - t.Fatalf("Expected service ANY lookup %s to pass: %v", test.name, err) - } + t.Run(fmt.Sprintf("AAAA lookup %v", test), func(t *testing.T) { + ok, err := testDNSServiceLookupResponseLimits(questions, test.udpAnswerLimit, dns.TypeAAAA, test.expectedAAAAService, test.expectedAAAAQuery, test.expectedAAAAQueryID, a) + if !ok { + t.Fatalf("Expected service AAAA lookup %s to pass: %v", test.name, err) + } + }) + + t.Run(fmt.Sprintf("ANY lookup %v", test), func(t *testing.T) { + ok, err := testDNSServiceLookupResponseLimits(questions, test.udpAnswerLimit, dns.TypeANY, test.expectedANYService, test.expectedANYQuery, test.expectedANYQueryID, a) + if !ok { + t.Fatalf("Expected service ANY lookup %s to pass: %v", test.name, err) + } + }) }) } }