Skip to content

Commit

Permalink
Build DNS changes with HashMap instead of Builder
Browse files Browse the repository at this point in the history
The existing CloudDnsWriter code uses ImmutableMap.Builder to construct the
map of DNS records to update.  This has been seen to fail on alpha, presumably
in a cases where host records and domain records produce duplicate updates for
a host.

Convert the Builder to a HashMap, allowing us to safely overwrite existing
records in the case of duplicates.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=170103421
  • Loading branch information
mindhog authored and CydeWeys committed Oct 4, 2017
1 parent 8908686 commit 0c8b5bc
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 7 deletions.
15 changes: 8 additions & 7 deletions java/google/registry/dns/writer/clouddns/CloudDnsWriter.java
Expand Up @@ -46,6 +46,7 @@
import java.net.Inet4Address;
import java.net.Inet6Address;
import java.net.InetAddress;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.Set;
Expand Down Expand Up @@ -82,8 +83,8 @@ public class CloudDnsWriter extends BaseDnsWriter {
private final String projectId;
private final String zoneName;
private final Dns dnsConnection;
private final ImmutableMap.Builder<String, ImmutableSet<ResourceRecordSet>>
desiredRecordsBuilder = new ImmutableMap.Builder<>();
private final HashMap<String, ImmutableSet<ResourceRecordSet>> desiredRecords =
new HashMap<String, ImmutableSet<ResourceRecordSet>>();

@Inject
CloudDnsWriter(
Expand Down Expand Up @@ -121,7 +122,7 @@ public void publishDomain(String domainName) {
// desiredRecordsBuilder is populated with an empty set to indicate that all existing records
// should be deleted.
if (!domainResource.isPresent() || !domainResource.get().shouldPublishToDns()) {
desiredRecordsBuilder.put(absoluteDomainName, ImmutableSet.<ResourceRecordSet>of());
desiredRecords.put(absoluteDomainName, ImmutableSet.<ResourceRecordSet>of());
return;
}

Expand Down Expand Up @@ -171,7 +172,7 @@ public void publishDomain(String domainName) {
}
}

desiredRecordsBuilder.put(absoluteDomainName, domainRecords.build());
desiredRecords.put(absoluteDomainName, domainRecords.build());
logger.finefmt(
"Will write %s records for domain %s", domainRecords.build().size(), absoluteDomainName);
}
Expand All @@ -189,7 +190,7 @@ private void publishSubordinateHost(String hostName) {

// Return early if the host is deleted.
if (!host.isPresent()) {
desiredRecordsBuilder.put(absoluteHostName, ImmutableSet.<ResourceRecordSet>of());
desiredRecords.put(absoluteHostName, ImmutableSet.<ResourceRecordSet>of());
return;
}

Expand Down Expand Up @@ -227,7 +228,7 @@ private void publishSubordinateHost(String hostName) {
.setRrdatas(ImmutableList.copyOf(aaaaRrData)));
}

desiredRecordsBuilder.put(absoluteHostName, domainRecords.build());
desiredRecords.put(absoluteHostName, domainRecords.build());
}

/**
Expand Down Expand Up @@ -273,7 +274,7 @@ public void publishHost(String hostName) {
@Override
protected void commitUnchecked() {
retrier.callWithRetry(
getMutateZoneCallback(desiredRecordsBuilder.build()), ZoneStateException.class);
getMutateZoneCallback(ImmutableMap.copyOf(desiredRecords)), ZoneStateException.class);
logger.info("Wrote to Cloud DNS");
}

Expand Down
Expand Up @@ -471,4 +471,22 @@ public void testLoadDomain_withPendingDelete() throws Exception {

verifyZone(ImmutableSet.<ResourceRecordSet>of());
}

@Test
public void testDuplicateRecords() throws Exception {
// In publishing DNS records, we can end up publishing information on the same host twice
// (through a domain change and a host change), so this scenario needs to work.
persistResource(
fakeDomain(
"example.tld",
ImmutableSet.of(persistResource(fakeHost("0.ip4.example.tld", IPv4))),
0))
.asBuilder()
.addSubordinateHost("ns1.example.tld")
.build();
writer.publishDomain("example.tld");
writer.publishHost("0.ip4.example.tld");

verifyZone(fakeDomainRecords("example.tld", 1, 0, 0, 0));
}
}

0 comments on commit 0c8b5bc

Please sign in to comment.