Minor fixes to dnsupdate writer

java/google/registry/dns/writer/api/DnsWriter.java

@@ -41,9 +41,9 @@ public interface DnsWriter extends AutoCloseable {
   /**
    * Loads {@code hostName} from datastore and publishes its A/AAAA glue records to the DNS server.
    * Replaces existing records for the exact name supplied, with an A or AAAA record (as
-   * appropriate) for each address stored in the registry, for the supplied host name. If the host is
+   * appropriate) for each address stored in the registry, for the supplied host name. If the host
-   * deleted then the existing records are deleted. Assumes that this method will only be called for
+   * is deleted then the existing records are deleted. Assumes that this method will only be called
-   * in-bailiwick hosts. The registry does not have addresses for other hosts.
+   * for in-bailiwick hosts. The registry does not have addresses for other hosts.
    *
    * @param hostName the fully qualified host name, with no trailing dot
    */

java/google/registry/dns/writer/dnsupdate/BUILD

@@ -2,6 +2,8 @@ package(
     default_visibility = ["//java/google/registry:registry_project"],
 )
 
+licenses(["notice"])  # Apache 2.0
+
 
 java_library(
     name = "dnsupdate",
@@ -13,14 +15,14 @@ java_library(
         "//java/com/google/common/io",
         "//java/com/google/common/net",
         "//java/com/google/common/primitives",
-        "//java/google/registry/config",
-        "//java/google/registry/dns/writer/api",
-        "//java/google/registry/model",
-        "//java/google/registry/util",
         "//third_party/java/dagger",
         "//third_party/java/dnsjava",
         "//third_party/java/joda_time",
         "//third_party/java/jsr305_annotations",
         "//third_party/java/jsr330_inject",
+        "//java/google/registry/config",
+        "//java/google/registry/dns/writer/api",
+        "//java/google/registry/model",
+        "//java/google/registry/util",
     ],
 )

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

@@ -25,6 +25,7 @@ import google.registry.config.ConfigModule.Config;
 import org.joda.time.Duration;
 import org.xbill.DNS.Message;
 import org.xbill.DNS.Opcode;
+import org.xbill.DNS.SimpleResolver;
 
 import java.io.DataInputStream;
 import java.io.IOException;
@@ -33,6 +34,7 @@ import java.io.OutputStream;
 import java.net.InetAddress;
 import java.net.Socket;
 import java.nio.ByteBuffer;
+import java.nio.channels.SocketChannel;
 
 import javax.inject.Inject;
 import javax.net.SocketFactory;

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

@@ -21,6 +21,7 @@ import google.registry.config.ConfigModule.Config;
 
 import org.joda.time.Duration;
 
+/** Dagger module that provides DNS configuration settings. */
 @Module
 public class DnsUpdateConfigModule {
 

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

@@ -66,8 +66,8 @@ import javax.inject.Inject;
  * <p>Only NS, DS, A, and AAAA records are published, and in particular no DNSSEC signing is done
  * assuming that this will be done by a third party DNS provider.
  *
- * <p>Each publish call is treated as an atomic update to the DNS. If an update fails an exception is
+ * <p>Each publish call is treated as an atomic update to the DNS. If an update fails an exception
- * thrown, expecting the caller to retry the update later. The SOA record serial number is
+ * is thrown, expecting the caller to retry the update later. The SOA record serial number is
  * implicitly incremented by the server on each UPDATE message, as required by RFC 2136. Care must
  * be taken to make sure the SOA serial number does not go backwards if the entire TLD (zone) is
  * "reset" to empty and republished.
@@ -193,13 +193,16 @@ public class DnsUpdateWriter implements DnsWriter {
   }
 
   private RRset makeV6AddressSet(String hostName, Iterable<InetAddress> addresses)
-      throws TextParseException {
+      throws TextParseException, IOException {
     RRset addressSet = new RRset();
     for (InetAddress address : addresses) {
       if (address instanceof Inet6Address) {
         AAAARecord record =
             new AAAARecord(
-                toAbsoluteName(hostName), DClass.IN, dnsTimeToLive.getStandardSeconds(), address);
+                toAbsoluteName(hostName),
+                DClass.IN,
+                dnsTimeToLive.getStandardSeconds(),
+                new org.xbill.DNS.Inet6Address(address.getAddress()));
         addressSet.addRR(record);
       }
     }

javatests/google/registry/dns/writer/dnsupdate/BUILD

@@ -0,0 +1,39 @@
+package(
+    default_testonly = 1,
+    default_visibility = ["//java/google/registry:registry_project"],
+)
+
+licenses(["notice"])  # Apache 2.0
+
+load("//java/com/google/testing/builddefs:GenTestRules.bzl", "GenTestRules")
+
+
+java_library(
+    name = "dnsupdate",
+    srcs = glob(["*.java"]),
+    deps = [
+        "//java/com/google/common/base",
+        "//java/com/google/common/collect",
+        "//java/com/google/common/io",
+        "//java/com/google/common/net",
+        "//third_party/java/dagger",
+        "//third_party/java/dnsjava",
+        "//third_party/java/joda_time",
+        "//third_party/java/jsr305_annotations",
+        "//third_party/java/junit",
+        "//third_party/java/mockito",
+        "//third_party/java/objectify:objectify-v4_1",
+        "//third_party/java/re2j",
+        "//third_party/java/servlet/servlet_api",
+        "//third_party/java/truth",
+        "//java/google/registry/dns/writer/dnsupdate",
+        "//java/google/registry/model",
+        "//javatests/google/registry/testing",
+    ],
+)
+
+GenTestRules(
+    name = "GeneratedTestRules",
+    test_files = glob(["*Test.java"]),
+    deps = [":dnsupdate"],
+)

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

@@ -22,11 +22,12 @@ import static org.mockito.Mockito.when;
 
 import com.google.common.base.VerifyException;
 
+import google.registry.testing.ExceptionRule;
+
 import org.joda.time.Duration;
 import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
-import org.junit.rules.ExpectedException;
 import org.junit.runner.RunWith;
 import org.mockito.Mock;
 import org.mockito.runners.MockitoJUnitRunner;
@@ -45,7 +46,6 @@ import org.xbill.DNS.utils.base16;
 import java.io.ByteArrayInputStream;
 import java.io.ByteArrayOutputStream;
 import java.io.EOFException;
-import java.io.IOException;
 import java.io.InputStream;
 import java.net.InetAddress;
 import java.net.Socket;
@@ -61,13 +61,18 @@ public class DnsMessageTransportTest {
 
   private static final String UPDATE_HOST = "127.0.0.1";
 
-  @Mock private SocketFactory mockFactory;
+  @Mock
-  @Mock private Socket mockSocket;
+  private SocketFactory mockFactory;
+
+  @Mock
+  private Socket mockSocket;
+
   private Message simpleQuery;
   private Message expectedResponse;
   private DnsMessageTransport resolver;
 
-  @Rule public ExpectedException thrown = ExpectedException.none();
+  @Rule
+  public final ExceptionRule thrown = new ExceptionRule();
 
   @Before
   public void before() throws Exception {
@@ -153,8 +158,7 @@ public class DnsMessageTransportTest {
     }
     ByteArrayOutputStream outputStream = new ByteArrayOutputStream();
     when(mockSocket.getOutputStream()).thenReturn(outputStream);
-    thrown.expect(IllegalArgumentException.class);
+    thrown.expect(IllegalArgumentException.class, "message larger than maximum");
-    thrown.expectMessage("message larger than maximum");
 
     resolver.send(oversize);
   }
@@ -165,8 +169,8 @@ public class DnsMessageTransportTest {
     when(mockSocket.getInputStream())
         .thenReturn(new ByteArrayInputStream(messageToBytesWithLength(expectedResponse)));
     when(mockSocket.getOutputStream()).thenReturn(new ByteArrayOutputStream());
-    thrown.expect(VerifyException.class);
+    thrown.expect(
-    thrown.expectMessage(
+        VerifyException.class,
         "response ID "
             + expectedResponse.getHeader().getID()
             + " does not match query ID "
@@ -182,8 +186,8 @@ public class DnsMessageTransportTest {
     when(mockSocket.getInputStream())
         .thenReturn(new ByteArrayInputStream(messageToBytesWithLength(expectedResponse)));
     when(mockSocket.getOutputStream()).thenReturn(new ByteArrayOutputStream());
-    thrown.expect(VerifyException.class);
+    thrown.expect(
-    thrown.expectMessage("response opcode 'STATUS' does not match query opcode 'QUERY'");
+        VerifyException.class, "response opcode 'STATUS' does not match query opcode 'QUERY'");
 
     resolver.send(simpleQuery);
   }
@@ -196,7 +200,7 @@ public class DnsMessageTransportTest {
     return message;
   }
 
-  private byte[] messageToBytesWithLength(Message message) throws IOException {
+  private byte[] messageToBytesWithLength(Message message) {
     byte[] bytes = message.toWire();
     ByteBuffer buffer =
         ByteBuffer.allocate(bytes.length + DnsMessageTransport.MESSAGE_LENGTH_FIELD_BYTES);

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

@@ -39,6 +39,7 @@ import google.registry.model.eppcommon.StatusValue;
 import google.registry.model.host.HostResource;
 import google.registry.model.ofy.Ofy;
 import google.registry.testing.AppEngineRule;
+import google.registry.testing.ExceptionRule;
 import google.registry.testing.FakeClock;
 import google.registry.testing.InjectRule;
 
@@ -49,7 +50,6 @@ import org.joda.time.Duration;
 import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
-import org.junit.rules.ExpectedException;
 import org.junit.runner.RunWith;
 import org.mockito.ArgumentCaptor;
 import org.mockito.Captor;
@@ -76,16 +76,20 @@ public class DnsUpdateWriterTest {
   public final AppEngineRule appEngine =
       AppEngineRule.builder().withDatastore().withTaskQueue().build();
 
-  @Rule public ExpectedException thrown = ExpectedException.none();
+  @Rule
+  public final ExceptionRule thrown = new ExceptionRule();
 
-  @Rule public final InjectRule inject = new InjectRule();
+  @Rule
+  public final InjectRule inject = new InjectRule();
 
   private final FakeClock clock = new FakeClock(DateTime.parse("1971-01-01TZ"));
 
-  @Mock private DnsMessageTransport mockResolver;
+  @Mock
-  @Captor private ArgumentCaptor<Update> updateCaptor;
+  private DnsMessageTransport mockResolver;
-  private DelegationSignerData testSignerData =
+
-      DelegationSignerData.create(1, 3, 1, base16().decode("0123456789ABCDEF"));
+  @Captor
+  private ArgumentCaptor<Update> updateCaptor;
+
   private DnsUpdateWriter writer;
 
   @Before
@@ -127,7 +131,9 @@ public class DnsUpdateWriterTest {
             .asBuilder()
             .setNameservers(
                 ImmutableSet.of(ReferenceUnion.create(persistActiveHost("ns1.example.tld"))))
-            .setDsData(ImmutableSet.of(testSignerData))
+            .setDsData(
+                ImmutableSet.of(
+                    DelegationSignerData.create(1, 3, 1, base16().decode("0123456789ABCDEF"))))
             .build();
     persistResource(domain);
 
@@ -184,7 +190,7 @@ public class DnsUpdateWriterTest {
                 ImmutableSet.of(
                     InetAddresses.forString("10.0.0.1"),
                     InetAddresses.forString("10.1.0.1"),
-                    InetAddresses.forString("fd0e:a5c8:6dfb:6a5e:0:0:0:1")))
+                    InetAddresses.forString("fd0e:a5c8:6dfb:6a5e::1")))
             .build();
     persistResource(host);
 
@@ -195,7 +201,7 @@ public class DnsUpdateWriterTest {
     assertThatUpdatedZoneIs(update, "tld.");
     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");
+    assertThatUpdateAdds(update, "ns1.example.tld.", Type.AAAA, "fd0e:a5c8:6dfb:6a5e::1");
     assertThatTotalUpdateSetsIs(update, 3); // The delete, the A, and AAAA sets
   }
 
@@ -222,8 +228,7 @@ public class DnsUpdateWriterTest {
             .build();
     persistResource(domain);
     when(mockResolver.send(any(Message.class))).thenReturn(messageWithResponseCode(Rcode.SERVFAIL));
-    thrown.expect(VerifyException.class);
+    thrown.expect(VerifyException.class, "SERVFAIL");
-    thrown.expectMessage("SERVFAIL");
 
     writer.publishDomain("example.tld");
   }
@@ -237,8 +242,7 @@ public class DnsUpdateWriterTest {
             .build();
     persistResource(host);
     when(mockResolver.send(any(Message.class))).thenReturn(messageWithResponseCode(Rcode.SERVFAIL));
-    thrown.expect(VerifyException.class);
+    thrown.expect(VerifyException.class, "SERVFAIL");
-    thrown.expectMessage("SERVFAIL");
 
     writer.publishHost("ns1.example.tld");
   }