Do not leave orphan glues for DnsUpdateWriter

This CL implements similar logic to deal with orphan glues as [] did
for ZonemanWriter. We are enforcing the policy that a glue record
should be deleted when authoritative NS record referring to it is
removed.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=128838082

java/google/registry/dns/writer/dnsupdate/DnsUpdateWriter.java

@@ -15,8 +15,14 @@
 package google.registry.dns.writer.dnsupdate;
 
 import static com.google.common.base.Verify.verify;
+import static com.google.common.collect.Sets.intersection;
+import static com.google.common.collect.Sets.union;
 import static google.registry.model.EppResourceUtils.loadByUniqueId;
 
+import com.google.common.base.Joiner;
+import com.google.common.base.Optional;
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableSet;
 import com.google.common.net.InternetDomainName;
 import google.registry.config.ConfigModule.Config;
 import google.registry.model.dns.DnsWriter;
@@ -90,17 +96,28 @@ public class DnsUpdateWriter implements DnsWriter {
     this.clock = clock;
   }
 
-  @Override
+  /**
-  public void publishDomain(String domainName) {
+   * Publish the domain, while keeping tracking of which host refresh quest triggered this domain
+   * refresh. Delete the requesting host in addition to all subordinate hosts.
+   *
+   * @param domainName the fully qualified domain name, with no trailing dot
+   * @param requestingHostName the fully qualified host name, with no trailing dot, that triggers
+   *     this domain refresh request
+   */
+  private void publishDomain(String domainName, String requestingHostName) {
     DomainResource domain = loadByUniqueId(DomainResource.class, domainName, clock.nowUtc());
     try {
       Update update = new Update(toAbsoluteName(findTldFromName(domainName)));
       update.delete(toAbsoluteName(domainName), Type.ANY);
-      if (domain != null && domain.shouldPublishToDns()) {
+      if (domain != null) {
-        update.add(makeNameServerSet(domain));
+        // As long as the domain exists, orphan glues should be cleaned.
-        update.add(makeDelegationSignerSet(domain));
+        deleteSubordinateHostAddressSet(domain, requestingHostName, update);
+        if (domain.shouldPublishToDns()) {
+          addInBailiwickNameServerSet(domain, update);
+          update.add(makeNameServerSet(domain));
+          update.add(makeDelegationSignerSet(domain));
+        }
       }
-
       Message response = transport.send(update);
       verify(
           response.getRcode() == Rcode.NOERROR,
@@ -111,27 +128,32 @@ public class DnsUpdateWriter implements DnsWriter {
       throw new RuntimeException("publishDomain failed: " + domainName, e);
     }
   }
+  
+  @Override
+  public void publishDomain(String domainName) {
+    publishDomain(domainName, null);
+  }
 
   @Override
   public void publishHost(String hostName) {
-    HostResource host = loadByUniqueId(HostResource.class, hostName, clock.nowUtc());
+    // Get the superordinate domain name of the host.
-    try {
+    InternetDomainName host = InternetDomainName.from(hostName);
-      Update update = new Update(toAbsoluteName(findTldFromName(hostName)));
+    ImmutableList<String> hostParts = host.parts();
-      update.delete(toAbsoluteName(hostName), Type.ANY);
+    Optional<InternetDomainName> tld = Registries.findTldForName(host);
-      if (host != null) {
-        update.add(makeAddressSet(host));
-        update.add(makeV6AddressSet(host));
-      }
 
-      Message response = transport.send(update);
+    // host not managed by our registry, no need to update DNS.
-      verify(
+    if (!tld.isPresent()) {
-          response.getRcode() == Rcode.NOERROR,
+      return;
-          "DNS server failed host update for '%s' rcode: %s",
-          hostName,
-          Rcode.string(response.getRcode()));
-    } catch (IOException e) {
-      throw new RuntimeException("publishHost failed: " + hostName, e);
     }
+
+    ImmutableList<String> tldParts = tld.get().parts();
+    ImmutableList<String> domainParts =
+        hostParts.subList(hostParts.size() - tldParts.size() - 1, hostParts.size());
+    String domain = Joiner.on(".").join(domainParts);
+
+    // Refresh the superordinate domain, always delete the host first to ensure idempotency,
+    // and only publish the host if it is a glue record.
+    publishDomain(domain, hostName);
   }
 
   /**
@@ -157,6 +179,29 @@ public class DnsUpdateWriter implements DnsWriter {
     return signerSet;
   }
 
+  private void deleteSubordinateHostAddressSet(
+      DomainResource domain, String additionalHost, Update update) throws TextParseException {
+    for (String hostName :
+        union(
+            domain.getSubordinateHosts(),
+            (additionalHost == null
+                ? ImmutableSet.<String>of()
+                : ImmutableSet.of(additionalHost)))) {
+      update.delete(toAbsoluteName(hostName), Type.ANY);
+    }
+  }
+
+  private void addInBailiwickNameServerSet(DomainResource domain, Update update)
+      throws TextParseException {
+    for (String hostName :
+        intersection(
+            domain.loadNameserverFullyQualifiedHostNames(), domain.getSubordinateHosts())) {
+      HostResource host = loadByUniqueId(HostResource.class, hostName, clock.nowUtc());
+      update.add(makeAddressSet(host));
+      update.add(makeV6AddressSet(host));
+    }
+  }
+
   private RRset makeNameServerSet(DomainResource domain) throws TextParseException {
     RRset nameServerSet = new RRset();
     for (String hostName : domain.loadNameserverFullyQualifiedHostNames()) {

javatests/google/registry/dns/writer/dnsupdate/DnsUpdateWriterTest.java

@@ -18,6 +18,8 @@ import static com.google.common.io.BaseEncoding.base16;
 import static com.google.common.truth.Truth.assertThat;
 import static com.google.common.truth.Truth.assert_;
 import static google.registry.testing.DatastoreHelper.createTld;
+import static google.registry.testing.DatastoreHelper.newDomainResource;
+import static google.registry.testing.DatastoreHelper.newHostResource;
 import static google.registry.testing.DatastoreHelper.persistActiveDomain;
 import static google.registry.testing.DatastoreHelper.persistActiveHost;
 import static google.registry.testing.DatastoreHelper.persistActiveSubordinateHost;
@@ -177,38 +179,138 @@ public class DnsUpdateWriterTest {
   @Test
   public void publishHostCreatePublishesAddressRecords() throws Exception {
     HostResource host =
-        persistActiveSubordinateHost("ns1.example.tld", persistActiveDomain("example.tld"))
+        persistResource(
+            newHostResource("ns1.example.tld")
+                .asBuilder()
+                .setInetAddresses(
+                    ImmutableSet.of(
+                        InetAddresses.forString("10.0.0.1"),
+                        InetAddresses.forString("10.1.0.1"),
+                        InetAddresses.forString("fd0e:a5c8:6dfb:6a5e:0:0:0:1")))
+                .build());
+    persistResource(
+        newDomainResource("example.tld")
             .asBuilder()
-            .setInetAddresses(
+            .addSubordinateHost("ns1.example.tld")
-                ImmutableSet.of(
+            .addNameservers(ImmutableSet.of(Ref.create(host)))
-                    InetAddresses.forString("10.0.0.1"),
+            .build());
-                    InetAddresses.forString("10.1.0.1"),
-                    InetAddresses.forString("fd0e:a5c8:6dfb:6a5e:0:0:0:1")))
-            .build();
-    persistResource(host);
 
     writer.publishHost("ns1.example.tld");
 
     verify(mockResolver).send(updateCaptor.capture());
     Update update = updateCaptor.getValue();
     assertThatUpdatedZoneIs(update, "tld.");
+    assertThatUpdateDeletes(update, "example.tld.", Type.ANY);
     assertThatUpdateDeletes(update, "ns1.example.tld.", Type.ANY);
     assertThatUpdateAdds(update, "ns1.example.tld.", Type.A, "10.0.0.1", "10.1.0.1");
     assertThatUpdateAdds(update, "ns1.example.tld.", Type.AAAA, "fd0e:a5c8:6dfb:6a5e:0:0:0:1");
-    assertThatTotalUpdateSetsIs(update, 3); // The delete, the A, and AAAA sets
+    assertThatUpdateAdds(update, "example.tld.", Type.NS, "ns1.example.tld.");
+    assertThatTotalUpdateSetsIs(update, 5);
   }
 
   @Test
   public void publishHostDeleteRemovesDnsRecords() throws Exception {
     persistDeletedHost("ns1.example.tld", clock.nowUtc());
+    persistActiveDomain("example.tld");
 
     writer.publishHost("ns1.example.tld");
 
     verify(mockResolver).send(updateCaptor.capture());
     Update update = updateCaptor.getValue();
     assertThatUpdatedZoneIs(update, "tld.");
+    assertThatUpdateDeletes(update, "example.tld.", Type.ANY);
     assertThatUpdateDeletes(update, "ns1.example.tld.", Type.ANY);
-    assertThatTotalUpdateSetsIs(update, 1); // Just the delete set
+    assertThatTotalUpdateSetsIs(update, 2); // Just the delete set
+  }
+
+  @Test
+  public void publishHostDeleteRemovesGlueRecords() throws Exception {
+    persistDeletedHost("ns1.example.tld", clock.nowUtc());
+    persistResource(
+        persistActiveDomain("example.tld")
+            .asBuilder()
+            .setNameservers(ImmutableSet.of(Ref.create(persistActiveHost("ns1.example.com"))))
+            .build());
+
+    writer.publishHost("ns1.example.tld");
+
+    verify(mockResolver).send(updateCaptor.capture());
+    Update update = updateCaptor.getValue();
+    assertThatUpdatedZoneIs(update, "tld.");
+    assertThatUpdateDeletes(update, "example.tld.", Type.ANY);
+    assertThatUpdateDeletes(update, "ns1.example.tld.", Type.ANY);
+    assertThatUpdateAdds(update, "example.tld.", Type.NS, "ns1.example.com.");
+    assertThatTotalUpdateSetsIs(update, 3);
+  }
+
+  @Test
+  public void publishDomainExternalAndInBailiwickNameServer() throws Exception {
+    HostResource externalNameserver = persistResource(newHostResource("ns1.example.com"));
+    HostResource inBailiwickNameserver =
+        persistResource(
+            newHostResource("ns1.example.tld")
+                .asBuilder()
+                .setInetAddresses(
+                    ImmutableSet.of(
+                        InetAddresses.forString("10.0.0.1"),
+                        InetAddresses.forString("10.1.0.1"),
+                        InetAddresses.forString("fd0e:a5c8:6dfb:6a5e:0:0:0:1")))
+                .build());
+
+    persistResource(
+        newDomainResource("example.tld")
+            .asBuilder()
+            .addSubordinateHost("ns1.example.tld")
+            .addNameservers(
+                ImmutableSet.of(Ref.create(externalNameserver), Ref.create(inBailiwickNameserver)))
+            .build());
+
+    writer.publishDomain("example.tld");
+
+    verify(mockResolver).send(updateCaptor.capture());
+    Update update = updateCaptor.getValue();
+    assertThatUpdatedZoneIs(update, "tld.");
+    assertThatUpdateDeletes(update, "example.tld.", Type.ANY);
+    assertThatUpdateDeletes(update, "ns1.example.tld.", Type.ANY);
+    assertThatUpdateAdds(update, "example.tld.", Type.NS, "ns1.example.com.", "ns1.example.tld.");
+    assertThatUpdateAdds(update, "ns1.example.tld.", Type.A, "10.0.0.1", "10.1.0.1");
+    assertThatUpdateAdds(update, "ns1.example.tld.", Type.AAAA, "fd0e:a5c8:6dfb:6a5e:0:0:0:1");
+    assertThatTotalUpdateSetsIs(update, 5);
+  }
+
+  @Test
+  public void publishDomainDeleteOrphanGlues() throws Exception {
+    HostResource inBailiwickNameserver =
+        persistResource(
+            newHostResource("ns1.example.tld")
+                .asBuilder()
+                .setInetAddresses(
+                    ImmutableSet.of(
+                        InetAddresses.forString("10.0.0.1"),
+                        InetAddresses.forString("10.1.0.1"),
+                        InetAddresses.forString("fd0e:a5c8:6dfb:6a5e:0:0:0:1")))
+                .build());
+
+    persistResource(
+        newDomainResource("example.tld")
+            .asBuilder()
+            .addSubordinateHost("ns1.example.tld")
+            .addSubordinateHost("foo.example.tld")
+            .addNameservers(ImmutableSet.of(Ref.create(inBailiwickNameserver)))
+            .build());
+
+    writer.publishDomain("example.tld");
+
+    verify(mockResolver).send(updateCaptor.capture());
+    Update update = updateCaptor.getValue();
+    assertThatUpdatedZoneIs(update, "tld.");
+    assertThatUpdateDeletes(update, "example.tld.", Type.ANY);
+    assertThatUpdateDeletes(update, "ns1.example.tld.", Type.ANY);
+    assertThatUpdateDeletes(update, "foo.example.tld.", Type.ANY);
+    assertThatUpdateAdds(update, "example.tld.", Type.NS, "ns1.example.tld.");
+    assertThatUpdateAdds(update, "ns1.example.tld.", Type.A, "10.0.0.1", "10.1.0.1");
+    assertThatUpdateAdds(update, "ns1.example.tld.", Type.AAAA, "fd0e:a5c8:6dfb:6a5e:0:0:0:1");
+    assertThatTotalUpdateSetsIs(update, 6);
   }
 
   @Test