Skip to content

Commit

Permalink
cmd/devp2p: fix comparison of TXT record value (#22572)
Browse files Browse the repository at this point in the history
* cmd/devp2p: fix comparison of TXT record value

The AWS API returns quoted DNS strings, so we must encode the new value
before comparing it against the existing record content.

* cmd/devp2p: add test

* cmd/devp2p: fix typo and rename val -> newValue
  • Loading branch information
fjl authored Mar 25, 2021
1 parent 0fda25e commit bed74b3
Show file tree
Hide file tree
Showing 2 changed files with 42 additions and 17 deletions.
35 changes: 18 additions & 17 deletions cmd/devp2p/dns_route53.go
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ func (c *route53Client) deploy(name string, t *dnsdisc.Tree) error {
}
}

// wait for all change batches to propagate
// Wait for all change batches to propagate.
for _, change := range changesToCheck {
log.Info(fmt.Sprintf("Waiting for change request %s", *change.ChangeInfo.Id))
wreq := &route53.GetChangeInput{Id: change.ChangeInfo.Id}
Expand Down Expand Up @@ -196,24 +196,29 @@ func (c *route53Client) computeChanges(name string, records map[string]string, e
records = lrecords

var changes []types.Change
for path, val := range records {
for path, newValue := range records {
prevRecords, exists := existing[path]
prevValue := strings.Join(prevRecords.values, "")

// prevValue contains quoted strings, encode newValue to compare.
newValue = splitTXT(newValue)

// Assign TTL.
ttl := int64(rootTTL)
if path != name {
ttl = int64(treeNodeTTL)
}

prevRecords, exists := existing[path]
prevValue := strings.Join(prevRecords.values, "")
if !exists {
// Entry is unknown, push a new one
log.Info(fmt.Sprintf("Creating %s = %q", path, val))
changes = append(changes, newTXTChange("CREATE", path, ttl, splitTXT(val)))
} else if prevValue != val || prevRecords.ttl != ttl {
log.Info(fmt.Sprintf("Creating %s = %s", path, newValue))
changes = append(changes, newTXTChange("CREATE", path, ttl, newValue))
} else if prevValue != newValue || prevRecords.ttl != ttl {
// Entry already exists, only change its content.
log.Info(fmt.Sprintf("Updating %s from %q to %q", path, prevValue, val))
changes = append(changes, newTXTChange("UPSERT", path, ttl, splitTXT(val)))
log.Info(fmt.Sprintf("Updating %s from %s to %s", path, prevValue, newValue))
changes = append(changes, newTXTChange("UPSERT", path, ttl, newValue))
} else {
log.Info(fmt.Sprintf("Skipping %s = %q", path, val))
log.Debug(fmt.Sprintf("Skipping %s = %s", path, newValue))
}
}

Expand Down Expand Up @@ -288,21 +293,19 @@ func changeCount(ch types.Change) int {

// collectRecords collects all TXT records below the given name.
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.HostedZoneId = &c.zoneID
existing := make(map[string]recordSet)
for {
for page := 0; ; page++ {
log.Info("Loading existing TXT records", "name", name, "zone", c.zoneID, "page", page)
resp, err := c.api.ListResourceRecordSets(context.TODO(), &req)
if err != nil {
return existing, err
}

for _, set := range resp.ResourceRecordSets {
if !isSubdomain(*set.Name, name) || set.Type != types.RRTypeTxt {
continue
}

s := recordSet{ttl: *set.TTL}
for _, rec := range set.ResourceRecords {
s.values = append(s.values, *rec.Value)
Expand All @@ -314,14 +317,12 @@ func (c *route53Client) collectRecords(name string) (map[string]recordSet, error
if !resp.IsTruncated {
break
}

// Set the cursor to the next batc. From the AWS docs:
// Set the cursor to the next batch. From the AWS docs:
//
// To display the next page of results, get the values of NextRecordName,
// NextRecordType, and NextRecordIdentifier (if any) from the response. Then submit
// another ListResourceRecordSets request, and specify those values for
// StartRecordName, StartRecordType, and StartRecordIdentifier.

req.StartRecordIdentifier = resp.NextRecordIdentifier
req.StartRecordName = resp.NextRecordName
req.StartRecordType = resp.NextRecordType
Expand Down
24 changes: 24 additions & 0 deletions cmd/devp2p/dns_route53_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -162,5 +162,29 @@ func TestRoute53ChangeSort(t *testing.T) {
}
}

// This test checks that computeChanges compares the quoted value of the records correctly.
func TestRoute53NoChange(t *testing.T) {
// Existing record set.
testTree0 := map[string]recordSet{
"n": {ttl: rootTTL, values: []string{
`"enrtree-root:v1 e=JWXYDBPXYWG6FX3GMDIBFA6CJ4 l=C7HRFPF3BLGF3YR4DY5KX3SMBE seq=1 sig=o908WmNp7LibOfPsr4btQwatZJ5URBr2ZAuxvK4UWHlsB9sUOTJQaGAlLPVAhM__XJesCHxLISo94z5Z2a463gA"`,
}},
"2xs2367yhaxjfglzhvawlqd4zy.n": {ttl: treeNodeTTL, values: []string{
`"enr:-HW4QOFzoVLaFJnNhbgMoDXPnOvcdVuj7pDpqRvh6BRDO68aVi5ZcjB3vzQRZH2IcLBGHzo8uUN3snqmgTiE56CH3AMBgmlkgnY0iXNlY3AyNTZrMaECC2_24YYkYHEgdzxlSNKQEnHhuNAbNlMlWJxrJxbAFvA"`,
}},
}
// New set.
testTree1 := map[string]string{
"n": "enrtree-root:v1 e=JWXYDBPXYWG6FX3GMDIBFA6CJ4 l=C7HRFPF3BLGF3YR4DY5KX3SMBE seq=1 sig=o908WmNp7LibOfPsr4btQwatZJ5URBr2ZAuxvK4UWHlsB9sUOTJQaGAlLPVAhM__XJesCHxLISo94z5Z2a463gA",
"2XS2367YHAXJFGLZHVAWLQD4ZY.n": "enr:-HW4QOFzoVLaFJnNhbgMoDXPnOvcdVuj7pDpqRvh6BRDO68aVi5ZcjB3vzQRZH2IcLBGHzo8uUN3snqmgTiE56CH3AMBgmlkgnY0iXNlY3AyNTZrMaECC2_24YYkYHEgdzxlSNKQEnHhuNAbNlMlWJxrJxbAFvA",
}

var client route53Client
changes := client.computeChanges("n", testTree1, testTree0)
if len(changes) > 0 {
t.Fatalf("wrong changes (got %d, want 0)", len(changes))
}
}

func sp(s string) *string { return &s }
func ip(i int64) *int64 { return &i }

0 comments on commit bed74b3

Please sign in to comment.