From b9c87b00050ba5efe558d7e76e6aef7e65047721 Mon Sep 17 00:00:00 2001 From: Felix Lange Date: Tue, 14 Jan 2020 09:57:47 +0100 Subject: [PATCH] cmd/devp2p: ensure Route53 changes are sorted --- cmd/devp2p/dns_route53.go | 42 ++++++----- cmd/devp2p/dns_route53_test.go | 124 +++++++++++++++++++++++++++++++++ 2 files changed, 150 insertions(+), 16 deletions(-) create mode 100644 cmd/devp2p/dns_route53_test.go diff --git a/cmd/devp2p/dns_route53.go b/cmd/devp2p/dns_route53.go index 34c6c34286..5e11df8e38 100644 --- a/cmd/devp2p/dns_route53.go +++ b/cmd/devp2p/dns_route53.go @@ -19,6 +19,7 @@ package main import ( "errors" "fmt" + "sort" "strconv" "strings" @@ -82,18 +83,20 @@ func (c *route53Client) deploy(name string, t *dnsdisc.Tree) error { } // Compute DNS changes. - records := t.ToTXT(name) - changes, err := c.computeChanges(name, records) + existing, err := c.collectRecords(name) if err != nil { return err } + log.Info(fmt.Sprintf("Found %d TXT records", len(existing))) + records := t.ToTXT(name) + changes := c.computeChanges(name, records, existing) if len(changes) == 0 { log.Info("No DNS changes needed") return nil } // Submit change batches. - batches := splitChangeBatches(changes) + batches := splitChanges(changes, route53ChangeLimit) for i, changes := range batches { log.Info(fmt.Sprintf("Submitting %d changes to Route53", len(changes))) batch := new(route53.ChangeBatch) @@ -146,7 +149,7 @@ func (c *route53Client) findZoneID(name string) (string, error) { } // computeChanges creates DNS changes for the given record. -func (c *route53Client) computeChanges(name string, records map[string]string) ([]*route53.Change, error) { +func (c *route53Client) computeChanges(name string, records map[string]string, existing map[string][]string) []*route53.Change { // Convert all names to lowercase. lrecords := make(map[string]string, len(records)) for name, r := range records { @@ -154,13 +157,6 @@ func (c *route53Client) computeChanges(name string, records map[string]string) ( } records = lrecords - // Get existing records. - existing, err := c.collectRecords(name) - if err != nil { - return nil, err - } - log.Info(fmt.Sprintf("Found %d TXT records", len(existing))) - var changes []*route53.Change for path, val := range records { ttl := 1 @@ -192,18 +188,31 @@ func (c *route53Client) computeChanges(name string, records map[string]string) ( log.Info(fmt.Sprintf("Deleting %s = %q", path, combineTXT(values))) changes = append(changes, newTXTChange("DELETE", path, 1, values)) } - return changes, nil + + sortChanges(changes) + return changes } -// splitChangeBatches splits up DNS changes such that each change batch -// is smaller than the limit. -func splitChangeBatches(changes []*route53.Change) [][]*route53.Change { +// sortChanges ensures DNS changes are in leaf-added -> root-changed -> leaf-deleted order. +func sortChanges(changes []*route53.Change) { + score := map[string]int{"CREATE": 1, "UPSERT": 2, "DELETE": 3} + sort.Slice(changes, func(i, j int) bool { + if *changes[i].Action == *changes[j].Action { + return *changes[i].ResourceRecordSet.Name < *changes[j].ResourceRecordSet.Name + } + return score[*changes[i].Action] < score[*changes[j].Action] + }) +} + +// splitChanges splits up DNS changes such that each change batch +// is smaller than the given RDATA limit. +func splitChanges(changes []*route53.Change, limit int) [][]*route53.Change { var batches [][]*route53.Change var batchSize int for _, ch := range changes { // Start new batch if this change pushes the current one over the limit. size := changeSize(ch) - if len(batches) == 0 || batchSize+size > route53ChangeLimit { + if len(batches) == 0 || batchSize+size > limit { batches = append(batches, nil) batchSize = 0 } @@ -213,6 +222,7 @@ func splitChangeBatches(changes []*route53.Change) [][]*route53.Change { return batches } +// changeSize returns the RDATA size of a DNS change. func changeSize(ch *route53.Change) int { size := 0 for _, rr := range ch.ResourceRecordSet.ResourceRecords { diff --git a/cmd/devp2p/dns_route53_test.go b/cmd/devp2p/dns_route53_test.go new file mode 100644 index 0000000000..9d3aeab0e3 --- /dev/null +++ b/cmd/devp2p/dns_route53_test.go @@ -0,0 +1,124 @@ +package main + +import ( + "reflect" + "testing" + + "github.com/aws/aws-sdk-go/service/route53" +) + +// This test checks that computeChanges/splitChanges create DNS changes in +// leaf-added -> root-changed -> leaf-deleted order. +func TestRoute53ChangeSort(t *testing.T) { + testTree0 := map[string][]string{ + "fdxn3sn67na5dka4j2gok7bvqi.n": {"enrtree-branch:"}, + "n": {"enrtree-root:v1 e=FDXN3SN67NA5DKA4J2GOK7BVQI l=FDXN3SN67NA5DKA4J2GOK7BVQI seq=0 sig=Me7k32DXwG06XmqzNW4b3opMrGXQV9Atv6hmJrVPuNx0QZ-PugeoYlMMndDxyN8cOkWGCxOze26WOyD2JAsAvgE"}, + } + + testTree1 := map[string]string{ + "n": "enrtree-root:v1 e=JWXYDBPXYWG6FX3GMDIBFA6CJ4 l=C7HRFPF3BLGF3YR4DY5KX3SMBE seq=1 sig=o908WmNp7LibOfPsr4btQwatZJ5URBr2ZAuxvK4UWHlsB9sUOTJQaGAlLPVAhM__XJesCHxLISo94z5Z2a463gA", + "C7HRFPF3BLGF3YR4DY5KX3SMBE.n": "enrtree://AM5FCQLWIZX2QFPNJAP7VUERCCRNGRHWZG3YYHIUV7BVDQ5FDPRT2@morenodes.example.org", + "JWXYDBPXYWG6FX3GMDIBFA6CJ4.n": "enrtree-branch:2XS2367YHAXJFGLZHVAWLQD4ZY,H4FHT4B454P6UXFD7JCYQ5PWDY,MHTDO6TMUBRIA2XWG5LUDACK24", + "2XS2367YHAXJFGLZHVAWLQD4ZY.n": "enr:-HW4QOFzoVLaFJnNhbgMoDXPnOvcdVuj7pDpqRvh6BRDO68aVi5ZcjB3vzQRZH2IcLBGHzo8uUN3snqmgTiE56CH3AMBgmlkgnY0iXNlY3AyNTZrMaECC2_24YYkYHEgdzxlSNKQEnHhuNAbNlMlWJxrJxbAFvA", + "H4FHT4B454P6UXFD7JCYQ5PWDY.n": "enr:-HW4QAggRauloj2SDLtIHN1XBkvhFZ1vtf1raYQp9TBW2RD5EEawDzbtSmlXUfnaHcvwOizhVYLtr7e6vw7NAf6mTuoCgmlkgnY0iXNlY3AyNTZrMaECjrXI8TLNXU0f8cthpAMxEshUyQlK-AM0PW2wfrnacNI", + "MHTDO6TMUBRIA2XWG5LUDACK24.n": "enr:-HW4QLAYqmrwllBEnzWWs7I5Ev2IAs7x_dZlbYdRdMUx5EyKHDXp7AV5CkuPGUPdvbv1_Ms1CPfhcGCvSElSosZmyoqAgmlkgnY0iXNlY3AyNTZrMaECriawHKWdDRk2xeZkrOXBQ0dfMFLHY4eENZwdufn1S1o", + } + + wantChanges := []*route53.Change{ + { + Action: sp("CREATE"), + ResourceRecordSet: &route53.ResourceRecordSet{ + Name: sp("2xs2367yhaxjfglzhvawlqd4zy.n"), + ResourceRecords: []*route53.ResourceRecord{{ + Value: sp(`"enr:-HW4QOFzoVLaFJnNhbgMoDXPnOvcdVuj7pDpqRvh6BRDO68aVi5ZcjB3vzQRZH2IcLBGHzo8uUN3snqmgTiE56CH3AMBgmlkgnY0iXNlY3AyNTZrMaECC2_24YYkYHEgdzxlSNKQEnHhuNAbNlMlWJxrJxbAFvA"`), + }}, + TTL: ip(2147483647), + Type: sp("TXT"), + }, + }, + { + Action: sp("CREATE"), + ResourceRecordSet: &route53.ResourceRecordSet{ + Name: sp("c7hrfpf3blgf3yr4dy5kx3smbe.n"), + ResourceRecords: []*route53.ResourceRecord{{ + Value: sp(`"enrtree://AM5FCQLWIZX2QFPNJAP7VUERCCRNGRHWZG3YYHIUV7BVDQ5FDPRT2@morenodes.example.org"`), + }}, + TTL: ip(2147483647), + Type: sp("TXT"), + }, + }, + { + Action: sp("CREATE"), + ResourceRecordSet: &route53.ResourceRecordSet{ + Name: sp("h4fht4b454p6uxfd7jcyq5pwdy.n"), + ResourceRecords: []*route53.ResourceRecord{{ + Value: sp(`"enr:-HW4QAggRauloj2SDLtIHN1XBkvhFZ1vtf1raYQp9TBW2RD5EEawDzbtSmlXUfnaHcvwOizhVYLtr7e6vw7NAf6mTuoCgmlkgnY0iXNlY3AyNTZrMaECjrXI8TLNXU0f8cthpAMxEshUyQlK-AM0PW2wfrnacNI"`), + }}, + TTL: ip(2147483647), + Type: sp("TXT"), + }, + }, + { + Action: sp("CREATE"), + ResourceRecordSet: &route53.ResourceRecordSet{ + Name: sp("jwxydbpxywg6fx3gmdibfa6cj4.n"), + ResourceRecords: []*route53.ResourceRecord{{ + Value: sp(`"enrtree-branch:2XS2367YHAXJFGLZHVAWLQD4ZY,H4FHT4B454P6UXFD7JCYQ5PWDY,MHTDO6TMUBRIA2XWG5LUDACK24"`), + }}, + TTL: ip(2147483647), + Type: sp("TXT"), + }, + }, + { + Action: sp("CREATE"), + ResourceRecordSet: &route53.ResourceRecordSet{ + Name: sp("mhtdo6tmubria2xwg5ludack24.n"), + ResourceRecords: []*route53.ResourceRecord{{ + Value: sp(`"enr:-HW4QLAYqmrwllBEnzWWs7I5Ev2IAs7x_dZlbYdRdMUx5EyKHDXp7AV5CkuPGUPdvbv1_Ms1CPfhcGCvSElSosZmyoqAgmlkgnY0iXNlY3AyNTZrMaECriawHKWdDRk2xeZkrOXBQ0dfMFLHY4eENZwdufn1S1o"`), + }}, + TTL: ip(2147483647), + Type: sp("TXT"), + }, + }, + { + Action: sp("UPSERT"), + ResourceRecordSet: &route53.ResourceRecordSet{ + Name: sp("n"), + ResourceRecords: []*route53.ResourceRecord{{ + Value: sp(`"enrtree-root:v1 e=JWXYDBPXYWG6FX3GMDIBFA6CJ4 l=C7HRFPF3BLGF3YR4DY5KX3SMBE seq=1 sig=o908WmNp7LibOfPsr4btQwatZJ5URBr2ZAuxvK4UWHlsB9sUOTJQaGAlLPVAhM__XJesCHxLISo94z5Z2a463gA"`), + }}, + TTL: ip(1), + Type: sp("TXT"), + }, + }, + { + Action: sp("DELETE"), + ResourceRecordSet: &route53.ResourceRecordSet{ + Name: sp("fdxn3sn67na5dka4j2gok7bvqi.n"), + ResourceRecords: []*route53.ResourceRecord{{ + Value: sp("enrtree-branch:"), + }}, + TTL: ip(1), + Type: sp("TXT"), + }, + }, + } + + var client route53Client + changes := client.computeChanges("n", testTree1, testTree0) + if !reflect.DeepEqual(changes, wantChanges) { + t.Fatalf("wrong changes (got %d, want %d)", len(changes), len(wantChanges)) + } + + wantSplit := [][]*route53.Change{ + wantChanges[:4], + wantChanges[4:7], + } + split := splitChanges(changes, 600) + if !reflect.DeepEqual(split, wantSplit) { + t.Fatalf("wrong split batches: got %d, want %d", len(split), len(wantSplit)) + } +} + +func sp(s string) *string { return &s } +func ip(i int64) *int64 { return &i }