zydronium/google-nomulus
1
0
Fork 0
mirror of https://github.com/google/nomulus.git synced 2025-07-21 18:26:12 +02:00
Code Issues Projects Releases Packages Wiki Activity

Build DNS changes with HashMap instead of Builder

Browse source 
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
This commit is contained in:
mmuller 2017-09-26 13:52:31 -07:00 • committed by Ben McIlwain
parent 8908686f23
commit 0c8b5bc8bf
2 changed files with 26 additions and 7 deletions
Download patch file Download diff file Expand all files Collapse all files

15
java/google/registry/dns/writer/clouddns/CloudDnsWriter.java
View file

@ -46,6 +46,7 @@ import java.io.IOException;
import java.net.Inet4Address; import java.net.Inet4Address;
import java.net.Inet6Address; import java.net.Inet6Address;
import java.net.InetAddress; import java.net.InetAddress;
import java.util.HashMap;
import java.util.HashSet; import java.util.HashSet;
import java.util.List; import java.util.List;
import java.util.Set; import java.util.Set;
@ -82,8 +83,8 @@ public class CloudDnsWriter extends BaseDnsWriter {
private final String projectId; private final String projectId;
private final String zoneName; private final String zoneName;
private final Dns dnsConnection; private final Dns dnsConnection;
private final ImmutableMap.Builder<String, ImmutableSet<ResourceRecordSet>> private final HashMap<String, ImmutableSet<ResourceRecordSet>> desiredRecords =
desiredRecordsBuilder = new ImmutableMap.Builder<>(); new HashMap<String, ImmutableSet<ResourceRecordSet>>();
@Inject @Inject
CloudDnsWriter( CloudDnsWriter(
@ -121,7 +122,7 @@ public class CloudDnsWriter extends BaseDnsWriter {
// desiredRecordsBuilder is populated with an empty set to indicate that all existing records // desiredRecordsBuilder is populated with an empty set to indicate that all existing records
// should be deleted. // should be deleted.
if (!domainResource.isPresent() || !domainResource.get().shouldPublishToDns()) { if (!domainResource.isPresent() || !domainResource.get().shouldPublishToDns()) {
desiredRecordsBuilder.put(absoluteDomainName, ImmutableSet.<ResourceRecordSet>of()); desiredRecords.put(absoluteDomainName, ImmutableSet.<ResourceRecordSet>of());
return; return;
} }
@ -171,7 +172,7 @@ public class CloudDnsWriter extends BaseDnsWriter {
} }
} }
desiredRecordsBuilder.put(absoluteDomainName, domainRecords.build()); desiredRecords.put(absoluteDomainName, domainRecords.build());
logger.finefmt( logger.finefmt(
"Will write %s records for domain %s", domainRecords.build().size(), absoluteDomainName); "Will write %s records for domain %s", domainRecords.build().size(), absoluteDomainName);
} }
@ -189,7 +190,7 @@ public class CloudDnsWriter extends BaseDnsWriter {
// Return early if the host is deleted. // Return early if the host is deleted.
if (!host.isPresent()) { if (!host.isPresent()) {
desiredRecordsBuilder.put(absoluteHostName, ImmutableSet.<ResourceRecordSet>of()); desiredRecords.put(absoluteHostName, ImmutableSet.<ResourceRecordSet>of());
return; return;
} }
@ -227,7 +228,7 @@ public class CloudDnsWriter extends BaseDnsWriter {
.setRrdatas(ImmutableList.copyOf(aaaaRrData))); .setRrdatas(ImmutableList.copyOf(aaaaRrData)));
} }
desiredRecordsBuilder.put(absoluteHostName, domainRecords.build()); desiredRecords.put(absoluteHostName, domainRecords.build());
} }
/** /**
@ -273,7 +274,7 @@ public class CloudDnsWriter extends BaseDnsWriter {
@Override @Override
protected void commitUnchecked() { protected void commitUnchecked() {
retrier.callWithRetry( retrier.callWithRetry(
getMutateZoneCallback(desiredRecordsBuilder.build()), ZoneStateException.class); getMutateZoneCallback(ImmutableMap.copyOf(desiredRecords)), ZoneStateException.class);
logger.info("Wrote to Cloud DNS"); logger.info("Wrote to Cloud DNS");
} }

18
javatests/google/registry/dns/writer/clouddns/CloudDnsWriterTest.java
View file

@ -471,4 +471,22 @@ public class CloudDnsWriterTest {
verifyZone(ImmutableSet.<ResourceRecordSet>of()); 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));
}
} }