From e90e84075754012d5692435b087e8e003723f49d Mon Sep 17 00:00:00 2001 From: mountford Date: Fri, 18 Aug 2017 16:12:44 -0700 Subject: [PATCH] Change GenerateZoneFilesAction to emit glue records only where appropriate Previously, GenerateZoneFilesAction mapreduced its way through all domains and hosts for the specified TLD(s), emitting information for each matching domain and host (subject to constraints like not being deleted and so on). This resulted in host information (aka glue records) for all hosts subordinate to domains in the specified TLD(s). This is incorrect. DNS glue records should only be present for hosts which act as nameservers for their superordinate domains. The new version of the mapreduce iterates only over domains. When a matching domain is found, a check is made to see whether any subordinate hosts are also nameservers for the domain, in which case host information is generated. The test was updated to reflect the new reality, and check for a couple additional nuances. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=165766472 --- .../tools/server/GenerateZoneFilesAction.java | 33 ++++++++++--------- .../server/GenerateZoneFilesActionTest.java | 10 ++++++ .../registry/tools/server/testdata/tld.zone | 6 ++-- 3 files changed, 31 insertions(+), 18 deletions(-) diff --git a/java/google/registry/tools/server/GenerateZoneFilesAction.java b/java/google/registry/tools/server/GenerateZoneFilesAction.java index 930ed2c35..372a156ec 100644 --- a/java/google/registry/tools/server/GenerateZoneFilesAction.java +++ b/java/google/registry/tools/server/GenerateZoneFilesAction.java @@ -143,7 +143,7 @@ public class GenerateZoneFilesAction implements Runnable, JsonActionRunner.JsonA new GenerateBindFileReducer(bucket, exportTime, gcsBufferSize), ImmutableList.of( new NullInput(), - createEntityInput(DomainResource.class, HostResource.class))); + createEntityInput(DomainResource.class))); ImmutableList filenames = FluentIterable.from(tlds) .transform( new Function() { @@ -160,7 +160,7 @@ public class GenerateZoneFilesAction implements Runnable, JsonActionRunner.JsonA "filenames", filenames); } - /** Mapper to find domains and hosts that were active at a given time. */ + /** Mapper to find domains that were active at a given time. */ static class GenerateBindFileMapper extends Mapper { private static final long serialVersionUID = 4647941823789859913L; @@ -190,13 +190,16 @@ public class GenerateZoneFilesAction implements Runnable, JsonActionRunner.JsonA for (String tld : tlds) { emit(tld, null); } - } else if (resource instanceof DomainResource) { - mapDomain((DomainResource) resource); } else { - mapHost((HostResource) resource); + mapDomain((DomainResource) resource); } } + // Originally, we mapped over domains and hosts separately, emitting the necessary information + // for each. But that doesn't work. All subordinate hosts in the specified TLD(s) would always + // be emitted in the final file, which is incorrect. Rather, to match the actual DNS glue + // records, we only want to emit host information for in-bailiwick hosts in the specified + // TLD(s), meaning those that act as nameservers for their respective superordinate domains. private void mapDomain(DomainResource domain) { // Domains never change their tld, so we can check if it's from the wrong tld right away. if (tlds.contains(domain.getTld())) { @@ -208,23 +211,23 @@ public class GenerateZoneFilesAction implements Runnable, JsonActionRunner.JsonA emit(domain.getTld(), stanza); getContext().incrementCounter(domain.getTld() + " domains"); } + emitForSubordinateHosts(domain); } } } - private void mapHost(HostResource host) { - host = loadAtPointInTime(host, exportTime).now(); - if (host != null) { // A null means the host was deleted (or not created) at this time. - // Find a matching tld. Hosts might change their tld, so check after the point-in-time load. - String fullyQualifiedHostName = host.getFullyQualifiedHostName(); - for (String tld : tlds) { - if (fullyQualifiedHostName.endsWith("." + tld)) { + private void emitForSubordinateHosts(DomainResource domain) { + ImmutableSet subordinateHosts = domain.getSubordinateHosts(); + if (!subordinateHosts.isEmpty()) { + for (HostResource unprojectedHost : ofy().load().keys(domain.getNameservers()).values()) { + HostResource host = loadAtPointInTime(unprojectedHost, exportTime).now(); + // A null means the host was deleted (or not created) at this time. + if ((host != null) && subordinateHosts.contains(host.getFullyQualifiedHostName())) { String stanza = hostStanza(host, dnsDefaultATtl); if (!stanza.isEmpty()) { - emit(tld, stanza); - getContext().incrementCounter(tld + " hosts"); + emit(domain.getTld(), stanza); + getContext().incrementCounter(domain.getTld() + " hosts"); } - return; } } } diff --git a/javatests/google/registry/tools/server/GenerateZoneFilesActionTest.java b/javatests/google/registry/tools/server/GenerateZoneFilesActionTest.java index f581826d0..1cb4f6361 100644 --- a/javatests/google/registry/tools/server/GenerateZoneFilesActionTest.java +++ b/javatests/google/registry/tools/server/GenerateZoneFilesActionTest.java @@ -72,6 +72,16 @@ public class GenerateZoneFilesActionTest extends MapreduceTestCase> nameservers = ImmutableSet.of(Key.create(host1), Key.create(host2)); + // This domain will have glue records, because it has a subordinate host which is its own + // nameserver. None of the other domains should have glue records, because their nameservers are + // subordinate to different domains. + persistResource(newDomainResource("bar.tld").asBuilder() + .addNameservers(nameservers) + .addSubordinateHost("ns.bar.tld") + .build()); + persistResource(newDomainResource("foo.tld").asBuilder() + .addSubordinateHost("ns.foo.tld") + .build()); persistResource(newDomainResource("ns-and-ds.tld").asBuilder() .addNameservers(nameservers) .setDsData(ImmutableSet.of(DelegationSignerData.create(1, 2, 3, new byte[] {0, 1, 2}))) diff --git a/javatests/google/registry/tools/server/testdata/tld.zone b/javatests/google/registry/tools/server/testdata/tld.zone index b9c95453c..b2c869f0a 100644 --- a/javatests/google/registry/tools/server/testdata/tld.zone +++ b/javatests/google/registry/tools/server/testdata/tld.zone @@ -1,5 +1,8 @@ $ORIGIN tld. +bar.tld 222 IN NS ns.bar.tld. +bar.tld 222 IN NS ns.foo.tld. + ns.bar.tld 11 IN A 127.0.0.1 ns.bar.tld 11 IN AAAA 0:0:0:0:0:0:0:1 @@ -9,6 +12,3 @@ ns-only.tld 222 IN NS ns.bar.tld. ns-and-ds.tld 222 IN NS ns.foo.tld. ns-and-ds.tld 222 IN NS ns.bar.tld. ns-and-ds.tld 3333 IN DS 1 2 3 000102 - -ns.foo.tld 11 IN A 127.0.0.1 -ns.foo.tld 11 IN AAAA 0:0:0:0:0:0:0:1