From 3558ce7af4e922233a9693a21fe0d0354d92e315 Mon Sep 17 00:00:00 2001 From: Felix Lange Date: Fri, 17 Jan 2020 11:32:29 +0100 Subject: [PATCH] cmd/devp2p: submit Route53 changes in batches (#20524) This change works around the 32k RDATA character limit per change request and fixes several issues in the deployer which prevented it from working for our production trees. --- cmd/devp2p/dns_cloudflare.go | 4 +- cmd/devp2p/dns_route53.go | 129 +++++++++++++++++++-------- cmd/devp2p/dns_route53_test.go | 156 +++++++++++++++++++++++++++++++++ cmd/devp2p/dnscmd.go | 5 ++ 4 files changed, 255 insertions(+), 39 deletions(-) create mode 100644 cmd/devp2p/dns_route53_test.go diff --git a/cmd/devp2p/dns_cloudflare.go b/cmd/devp2p/dns_cloudflare.go index 93b995432..0d1ed7071 100644 --- a/cmd/devp2p/dns_cloudflare.go +++ b/cmd/devp2p/dns_cloudflare.go @@ -130,9 +130,9 @@ func (c *cloudflareClient) uploadRecords(name string, records map[string]string) if !exists { // Entry is unknown, push a new one to Cloudflare. log.Info(fmt.Sprintf("Creating %s = %q", path, val)) - ttl := 1 + ttl := rootTTL if path != name { - ttl = 2147483647 // Max TTL permitted by Cloudflare + ttl = treeNodeTTL // Max TTL permitted by Cloudflare } _, err = c.CreateDNSRecord(c.zoneID, cloudflare.DNSRecord{Type: "TXT", Name: path, Content: val, TTL: ttl}) } else if old.Content != val { diff --git a/cmd/devp2p/dns_route53.go b/cmd/devp2p/dns_route53.go index 1e9b39b0e..bdad5c2e9 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" @@ -31,6 +32,10 @@ import ( "gopkg.in/urfave/cli.v1" ) +// The Route53 limits change sets to this size. DNS changes need to be split +// up into multiple batches to work around the limit. +const route53ChangeLimit = 30000 + var ( route53AccessKeyFlag = cli.StringFlag{ Name: "access-key-id", @@ -53,6 +58,11 @@ type route53Client struct { zoneID string } +type recordSet struct { + values []string + ttl int64 +} + // newRoute53Client sets up a Route53 API client from command line flags. func newRoute53Client(ctx *cli.Context) *route53Client { akey := ctx.String(route53AccessKeyFlag.Name) @@ -78,31 +88,39 @@ 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 request. - log.Info(fmt.Sprintf("Submitting %d changes to Route53", len(changes))) - batch := new(route53.ChangeBatch) - batch.SetChanges(changes) - batch.SetComment(fmt.Sprintf("enrtree update of %s at seq %d", name, t.Seq())) - req := &route53.ChangeResourceRecordSetsInput{HostedZoneId: &c.zoneID, ChangeBatch: batch} - resp, err := c.api.ChangeResourceRecordSets(req) - if err != nil { - return err - } + // Submit change batches. + batches := splitChanges(changes, route53ChangeLimit) + for i, changes := range batches { + log.Info(fmt.Sprintf("Submitting %d changes to Route53", len(changes))) + batch := new(route53.ChangeBatch) + batch.SetChanges(changes) + batch.SetComment(fmt.Sprintf("enrtree update %d/%d of %s at seq %d", i+1, len(batches), name, t.Seq())) + req := &route53.ChangeResourceRecordSetsInput{HostedZoneId: &c.zoneID, ChangeBatch: batch} + resp, err := c.api.ChangeResourceRecordSets(req) + if err != nil { + return err + } - // Wait for the change to be applied. - log.Info(fmt.Sprintf("Waiting for change request %s", *resp.ChangeInfo.Id)) - wreq := &route53.GetChangeInput{Id: resp.ChangeInfo.Id} - return c.api.WaitUntilResourceRecordSetsChanged(wreq) + log.Info(fmt.Sprintf("Waiting for change request %s", *resp.ChangeInfo.Id)) + wreq := &route53.GetChangeInput{Id: resp.ChangeInfo.Id} + if err := c.api.WaitUntilResourceRecordSetsChanged(wreq); err != nil { + return err + } + } + return nil } // checkZone verifies zone information for the given domain. @@ -137,7 +155,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]recordSet) []*route53.Change { // Convert all names to lowercase. lrecords := make(map[string]string, len(records)) for name, r := range records { @@ -145,22 +163,15 @@ 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 + ttl := int64(rootTTL) if path != name { - ttl = 2147483647 + ttl = int64(treeNodeTTL) } prevRecords, exists := existing[path] - prevValue := combineTXT(prevRecords) + prevValue := combineTXT(prevRecords.values) if !exists { // Entry is unknown, push a new one log.Info(fmt.Sprintf("Creating %s = %q", path, val)) @@ -175,32 +186,76 @@ func (c *route53Client) computeChanges(name string, records map[string]string) ( } // Iterate over the old records and delete anything stale. - for path, values := range existing { + for path, set := range existing { if _, ok := records[path]; ok { continue } // Stale entry, nuke it. - log.Info(fmt.Sprintf("Deleting %s = %q", path, combineTXT(values))) - changes = append(changes, newTXTChange("DELETE", path, 1, values)) + log.Info(fmt.Sprintf("Deleting %s = %q", path, combineTXT(set.values))) + changes = append(changes, newTXTChange("DELETE", path, set.ttl, set.values)) } - return changes, nil + + sortChanges(changes) + return changes +} + +// 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 > limit { + batches = append(batches, nil) + batchSize = 0 + } + batches[len(batches)-1] = append(batches[len(batches)-1], ch) + batchSize += size + } + 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 { + if rr.Value != nil { + size += len(*rr.Value) + } + } + return size } // collectRecords collects all TXT records below the given name. -func (c *route53Client) collectRecords(name string) (map[string][]string, error) { +func (c *route53Client) collectRecords(name string) (map[string]recordSet, error) { log.Info(fmt.Sprintf("Retrieving existing TXT records on %s (%s)", name, c.zoneID)) var req route53.ListResourceRecordSetsInput req.SetHostedZoneId(c.zoneID) - existing := make(map[string][]string) + existing := make(map[string]recordSet) err := c.api.ListResourceRecordSetsPages(&req, func(resp *route53.ListResourceRecordSetsOutput, last bool) bool { for _, set := range resp.ResourceRecordSets { if !isSubdomain(*set.Name, name) || *set.Type != "TXT" { continue } - name := strings.TrimSuffix(*set.Name, ".") + s := recordSet{ttl: *set.TTL} for _, rec := range set.ResourceRecords { - existing[name] = append(existing[name], *rec.Value) + s.values = append(s.values, *rec.Value) } + name := strings.TrimSuffix(*set.Name, ".") + existing[name] = s } return true }) @@ -208,7 +263,7 @@ func (c *route53Client) collectRecords(name string) (map[string][]string, error) } // newTXTChange creates a change to a TXT record. -func newTXTChange(action, name string, ttl int, values []string) *route53.Change { +func newTXTChange(action, name string, ttl int64, values []string) *route53.Change { var c route53.Change var r route53.ResourceRecordSet var rrs []*route53.ResourceRecord @@ -219,7 +274,7 @@ func newTXTChange(action, name string, ttl int, values []string) *route53.Change } r.SetType("TXT") r.SetName(name) - r.SetTTL(int64(ttl)) + r.SetTTL(ttl) r.SetResourceRecords(rrs) c.SetAction(action) c.SetResourceRecordSet(&r) diff --git a/cmd/devp2p/dns_route53_test.go b/cmd/devp2p/dns_route53_test.go new file mode 100644 index 000000000..c64f1d169 --- /dev/null +++ b/cmd/devp2p/dns_route53_test.go @@ -0,0 +1,156 @@ +// Copyright 2020 The go-ethereum Authors +// This file is part of go-ethereum. +// +// go-ethereum is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// go-ethereum is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with go-ethereum. If not, see . + +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]recordSet{ + "2kfjogvxdqtxxugbh7gs7naaai.n": {ttl: 3333, values: []string{ + `"enr:-HW4QO1ml1DdXLeZLsUxewnthhUy8eROqkDyoMTyavfks9JlYQIlMFEUoM78PovJDPQrAkrb3LRJ-"`, + `"vtrymDguKCOIAWAgmlkgnY0iXNlY3AyNTZrMaEDffaGfJzgGhUif1JqFruZlYmA31HzathLSWxfbq_QoQ4"`, + }}, + "fdxn3sn67na5dka4j2gok7bvqi.n": {ttl: treeNodeTTL, values: []string{`"enrtree-branch:"`}}, + "n": {ttl: rootTTL, values: []string{`"enrtree-root:v1 e=2KFJOGVXDQTXXUGBH7GS7NAAAI l=FDXN3SN67NA5DKA4J2GOK7BVQI seq=0 sig=v_-J_q_9ICQg5ztExFvLQhDBGMb0lZPJLhe3ts9LAcgqhOhtT3YFJsl8BWNDSwGtamUdR-9xl88_w-X42SVpjwE"`}}, + } + + 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(treeNodeTTL), + 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(treeNodeTTL), + Type: sp("TXT"), + }, + }, + { + Action: sp("CREATE"), + ResourceRecordSet: &route53.ResourceRecordSet{ + Name: sp("h4fht4b454p6uxfd7jcyq5pwdy.n"), + ResourceRecords: []*route53.ResourceRecord{{ + Value: sp(`"enr:-HW4QAggRauloj2SDLtIHN1XBkvhFZ1vtf1raYQp9TBW2RD5EEawDzbtSmlXUfnaHcvwOizhVYLtr7e6vw7NAf6mTuoCgmlkgnY0iXNlY3AyNTZrMaECjrXI8TLNXU0f8cthpAMxEshUyQlK-AM0PW2wfrnacNI"`), + }}, + TTL: ip(treeNodeTTL), + 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(treeNodeTTL), + Type: sp("TXT"), + }, + }, + { + Action: sp("CREATE"), + ResourceRecordSet: &route53.ResourceRecordSet{ + Name: sp("mhtdo6tmubria2xwg5ludack24.n"), + ResourceRecords: []*route53.ResourceRecord{{ + Value: sp(`"enr:-HW4QLAYqmrwllBEnzWWs7I5Ev2IAs7x_dZlbYdRdMUx5EyKHDXp7AV5CkuPGUPdvbv1_Ms1CPfhcGCvSElSosZmyoqAgmlkgnY0iXNlY3AyNTZrMaECriawHKWdDRk2xeZkrOXBQ0dfMFLHY4eENZwdufn1S1o"`), + }}, + TTL: ip(treeNodeTTL), + 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(rootTTL), + Type: sp("TXT"), + }, + }, + { + Action: sp("DELETE"), + ResourceRecordSet: &route53.ResourceRecordSet{ + Name: sp("2kfjogvxdqtxxugbh7gs7naaai.n"), + ResourceRecords: []*route53.ResourceRecord{ + {Value: sp(`"enr:-HW4QO1ml1DdXLeZLsUxewnthhUy8eROqkDyoMTyavfks9JlYQIlMFEUoM78PovJDPQrAkrb3LRJ-"`)}, + {Value: sp(`"vtrymDguKCOIAWAgmlkgnY0iXNlY3AyNTZrMaEDffaGfJzgGhUif1JqFruZlYmA31HzathLSWxfbq_QoQ4"`)}, + }, + TTL: ip(3333), + Type: sp("TXT"), + }, + }, + { + Action: sp("DELETE"), + ResourceRecordSet: &route53.ResourceRecordSet{ + Name: sp("fdxn3sn67na5dka4j2gok7bvqi.n"), + ResourceRecords: []*route53.ResourceRecord{{ + Value: sp(`"enrtree-branch:"`), + }}, + TTL: ip(treeNodeTTL), + 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:8], + } + 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 } diff --git a/cmd/devp2p/dnscmd.go b/cmd/devp2p/dnscmd.go index 892deb7dc..45673558d 100644 --- a/cmd/devp2p/dnscmd.go +++ b/cmd/devp2p/dnscmd.go @@ -96,6 +96,11 @@ var ( } ) +const ( + rootTTL = 1 + treeNodeTTL = 2147483647 +) + // dnsSync performs dnsSyncCommand. func dnsSync(ctx *cli.Context) error { var (