Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

cmd/devp2p: fix comparison of TXT record value #22572

Merged
merged 3 commits into from
Mar 25, 2021
Merged

cmd/devp2p: fix comparison of TXT record value #22572

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1819299
cmd/devp2p: fix comparison of TXT record value
fjl Mar 25, 2021
75e7036
cmd/devp2p: add test
fjl Mar 25, 2021
350f5a5
cmd/devp2p: fix typo and rename val -> newValue
fjl Mar 25, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 15 additions & 15 deletions cmd/devp2p/dns_route53.go
View file
Open in desktop
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 @@ -197,23 +197,27 @@ func (c *route53Client) computeChanges(name string, records map[string]string, e

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

// Assign TTL.
ttl := int64(rootTTL)
if path != name {
ttl = int64(treeNodeTTL)
}
// prevValue is quoted, encode value for to compare.
val = splitTXT(val)

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)))
log.Info(fmt.Sprintf("Creating %s = %s", path, val))
changes = append(changes, newTXTChange("CREATE", path, ttl, val))
} else if prevValue != val || 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, val))
changes = append(changes, newTXTChange("UPSERT", path, ttl, val))
} else {
log.Info(fmt.Sprintf("Skipping %s = %q", path, val))
log.Debug(fmt.Sprintf("Skipping %s = %s", path, val))
}
}

Expand Down Expand Up @@ -288,21 +292,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 +316,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
View file
Open in desktop
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 }