diff --git a/java/google/registry/dns/writer/clouddns/CloudDnsWriter.java b/java/google/registry/dns/writer/clouddns/CloudDnsWriter.java index 499d3419c..ef8fca782 100644 --- a/java/google/registry/dns/writer/clouddns/CloudDnsWriter.java +++ b/java/google/registry/dns/writer/clouddns/CloudDnsWriter.java @@ -67,8 +67,8 @@ import org.joda.time.Duration; public class CloudDnsWriter extends BaseDnsWriter { /** - * The name of the dns writer, as used in {@code Registry.dnsWriter}. Remember to change - * the value on affected Registry objects to prevent runtime failures. + * The name of the dns writer, as used in {@code Registry.dnsWriter}. Remember to change the value + * on affected Registry objects to prevent runtime failures. */ public static final String NAME = "CloudDnsWriter"; @@ -153,16 +153,16 @@ public class CloudDnsWriter extends BaseDnsWriter { } } - // Construct NS records (if any). Set nameserverData = domainResource.get().loadNameserverFullyQualifiedHostNames(); + Set subordinateHosts = domainResource.get().getSubordinateHosts(); if (!nameserverData.isEmpty()) { HashSet nsRrData = new HashSet<>(); for (String hostName : nameserverData) { nsRrData.add(getAbsoluteHostName(hostName)); // Construct glue records for subordinate NS hostnames (if any) - if (hostName.endsWith(domainName)) { + if (subordinateHosts.contains(hostName)) { publishSubordinateHost(hostName); } } @@ -285,19 +285,15 @@ public class CloudDnsWriter extends BaseDnsWriter { logger.info("Wrote to Cloud DNS"); } - /** - * Returns the glue records for in-bailiwick nameservers for the given domain+records. - */ + /** Returns the glue records for in-bailiwick nameservers for the given domain+records. */ private Stream filterGlueRecords(String domainName, Stream records) { return records .filter(record -> record.getType().equals("NS")) .flatMap(record -> record.getRrdatas().stream()) - .filter(hostName -> hostName.endsWith(domainName) && !hostName.equals(domainName)); + .filter(hostName -> hostName.endsWith("." + domainName) && !hostName.equals(domainName)); } - /** - * Mutate the zone with the provided {@code desiredRecords}. - */ + /** Mutate the zone with the provided {@code desiredRecords}. */ @VisibleForTesting void mutateZone(ImmutableMap> desiredRecords) { // Fetch all existing records for names that this writer is trying to modify diff --git a/javatests/google/registry/dns/writer/clouddns/CloudDnsWriterTest.java b/javatests/google/registry/dns/writer/clouddns/CloudDnsWriterTest.java index 1f9db26b3..e259c5bb5 100644 --- a/javatests/google/registry/dns/writer/clouddns/CloudDnsWriterTest.java +++ b/javatests/google/registry/dns/writer/clouddns/CloudDnsWriterTest.java @@ -110,7 +110,6 @@ public class CloudDnsWriterTest { return listResourceRecordSetsRequest; } - @Before public void setUp() throws Exception { createTld("tld"); @@ -133,8 +132,7 @@ public class CloudDnsWriterTest { when(dnsConnection.changes()).thenReturn(changes); when(dnsConnection.resourceRecordSets()).thenReturn(resourceRecordSets); when(resourceRecordSets.list(anyString(), anyString())) - .thenAnswer( - invocationOnMock -> newListResourceRecordSetsRequestMock()); + .thenAnswer(invocationOnMock -> newListResourceRecordSetsRequestMock()); when(changes.create(anyString(), zoneNameCaptor.capture(), changeCaptor.capture())) .thenReturn(createChangeRequest); // Change our stub zone when a request to change the records is executed @@ -165,6 +163,22 @@ public class CloudDnsWriterTest { assertThat(stubZone).containsExactlyElementsIn(expectedRecords); } + /** Returns a a zone cut with records for a domain and given nameservers, with no glue records. */ + private static ImmutableSet fakeDomainRecords( + String domainName, String... nameservers) { + ImmutableSet.Builder recordSetBuilder = new ImmutableSet.Builder<>(); + if (nameservers.length > 0) { + recordSetBuilder.add( + new ResourceRecordSet() + .setKind("dns#resourceRecordSet") + .setType("NS") + .setName(domainName + ".") + .setTtl(222) + .setRrdatas(ImmutableList.copyOf(nameservers))); + } + return recordSetBuilder.build(); + } + /** Returns a a zone cut with records for a domain */ private static ImmutableSet fakeDomainRecords( String domainName, @@ -337,13 +351,13 @@ public class CloudDnsWriterTest { @Test public void testLoadDomain_withInBailiwickNs_IPv4() throws Exception { persistResource( - fakeDomain( + fakeDomain( "example.tld", ImmutableSet.of(persistResource(fakeHost("0.ip4.example.tld", IPv4))), - 0)) - .asBuilder() - .addSubordinateHost("0.ip4.example.tld") - .build(); + 0) + .asBuilder() + .addSubordinateHost("0.ip4.example.tld") + .build()); writer.publishDomain("example.tld"); verifyZone(fakeDomainRecords("example.tld", 1, 0, 0, 0)); @@ -352,18 +366,30 @@ public class CloudDnsWriterTest { @Test public void testLoadDomain_withInBailiwickNs_IPv6() throws Exception { persistResource( - fakeDomain( + fakeDomain( "example.tld", ImmutableSet.of(persistResource(fakeHost("0.ip6.example.tld", IPv6))), - 0)) - .asBuilder() - .addSubordinateHost("0.ip6.example.tld") - .build(); + 0) + .asBuilder() + .addSubordinateHost("0.ip6.example.tld") + .build()); writer.publishDomain("example.tld"); verifyZone(fakeDomainRecords("example.tld", 0, 1, 0, 0)); } + @Test + public void testLoadDomain_withNameserveThatEndsWithDomainName() throws Exception { + persistResource( + fakeDomain( + "example.tld", + ImmutableSet.of(persistResource(fakeHost("ns.another-example.tld", IPv4))), + 0)); + writer.publishDomain("example.tld"); + + verifyZone(fakeDomainRecords("example.tld", "ns.another-example.tld.")); + } + @Test public void testLoadHost_externalHost() throws Exception { writer.publishHost("ns1.example.com"); @@ -380,13 +406,13 @@ public class CloudDnsWriterTest { // Model the domain with only one NS record -- this is equivalent to creating it // with two NS records and then deleting one persistResource( - fakeDomain( + fakeDomain( "example.tld", ImmutableSet.of(persistResource(fakeHost("0.ip4.example.tld", IPv4))), - 0)) - .asBuilder() - .addSubordinateHost("0.ip4.example.tld") - .build(); + 0) + .asBuilder() + .addSubordinateHost("0.ip4.example.tld") + .build()); // Ask the writer to delete the deleted NS record and glue writer.publishHost("1.ip4.example.tld"); @@ -456,13 +482,13 @@ public class CloudDnsWriterTest { // 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( + fakeDomain( "example.tld", ImmutableSet.of(persistResource(fakeHost("0.ip4.example.tld", IPv4))), - 0)) - .asBuilder() - .addSubordinateHost("ns1.example.tld") - .build(); + 0) + .asBuilder() + .addSubordinateHost("0.ip4.example.tld") + .build()); writer.publishDomain("example.tld"); writer.publishHost("0.ip4.example.tld");