Better handle deletion of prober domains

This implements a two-part deletion process for prober domains that were
not deleted properly by the prober (which is usually caused by a transient
network failure). The first time the mapreduce is run, such domains are
soft-deleted, so that their DNS entries can be removed correctly, and then
they are hard-deleted in the subsequent run.

Currently, all domains are hard-deleted the first time this mapreduce
runs, even if they were never soft-deleted correctly, which means that
their published DNS entries won't be correctly handled.

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

java/google/registry/batch/DeleteProberDataAction.java

@@ -14,29 +14,40 @@
 
 package google.registry.batch;
 
+import static com.google.common.base.Preconditions.checkState;
+import static google.registry.flows.ResourceFlowUtils.updateForeignKeyIndexDeletionTime;
 import static google.registry.mapreduce.MapreduceRunner.PARAM_DRY_RUN;
 import static google.registry.model.ofy.ObjectifyService.ofy;
 import static google.registry.model.registry.Registries.getTldsOfType;
+import static google.registry.model.reporting.HistoryEntry.Type.DOMAIN_DELETE;
 import static google.registry.request.Action.Method.POST;
+import static org.joda.time.DateTimeZone.UTC;
 
 import com.google.appengine.tools.mapreduce.Mapper;
 import com.google.common.base.Function;
 import com.google.common.base.Predicate;
 import com.google.common.base.Splitter;
+import com.google.common.base.Strings;
 import com.google.common.collect.FluentIterable;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Iterables;
 import com.googlecode.objectify.Key;
+import com.googlecode.objectify.VoidWork;
 import com.googlecode.objectify.Work;
+import google.registry.config.RegistryConfig.Config;
+import google.registry.dns.DnsQueue;
 import google.registry.mapreduce.MapreduceRunner;
 import google.registry.mapreduce.inputs.EppResourceInputs;
+import google.registry.model.EppResourceUtils;
 import google.registry.model.domain.DomainApplication;
 import google.registry.model.domain.DomainBase;
+import google.registry.model.domain.DomainResource;
 import google.registry.model.index.EppResourceIndex;
 import google.registry.model.index.ForeignKeyIndex;
 import google.registry.model.registry.Registry;
 import google.registry.model.registry.Registry.TldType;
+import google.registry.model.reporting.HistoryEntry;
 import google.registry.request.Action;
 import google.registry.request.Parameter;
 import google.registry.request.Response;
@@ -45,6 +56,7 @@ import google.registry.util.FormattingLogger;
 import google.registry.util.PipelineUtils;
 import java.util.List;
 import javax.inject.Inject;
+import org.joda.time.DateTime;
 
 /**
  * Deletes all prober DomainResources and their subordinate history entries, poll messages, and
@@ -62,17 +74,21 @@ public class DeleteProberDataAction implements Runnable {
   private static final FormattingLogger logger = FormattingLogger.getLoggerForCallerClass();
 
   @Inject @Parameter(PARAM_DRY_RUN) boolean isDryRun;
+  @Inject @Config("registryAdminClientId") String registryAdminClientId;
   @Inject MapreduceRunner mrRunner;
   @Inject Response response;
   @Inject DeleteProberDataAction() {}
 
   @Override
   public void run() {
+    checkState(
+        !Strings.isNullOrEmpty(registryAdminClientId),
+        "Registry admin client ID must be configured for prober data deletion to work");
     response.sendJavaScriptRedirect(PipelineUtils.createJobPath(mrRunner
         .setJobName("Delete prober data")
         .setModuleName("backend")
         .runMapOnly(
-            new DeleteProberDataMapper(getProberRoidSuffixes(), isDryRun),
+            new DeleteProberDataMapper(getProberRoidSuffixes(), isDryRun, registryAdminClientId),
             ImmutableList.of(EppResourceInputs.createKeyInput(DomainBase.class)))));
   }
 
@@ -97,14 +113,18 @@ public class DeleteProberDataAction implements Runnable {
   /** Provides the map method that runs for each existing DomainBase entity. */
   public static class DeleteProberDataMapper extends Mapper<Key<DomainBase>, Void, Void> {
 
-    private static final long serialVersionUID = 1737761271804180412L;
+    private static final DnsQueue dnsQueue = DnsQueue.create();
+    private static final long serialVersionUID = -7724537393697576369L;
 
     private final ImmutableSet<String> proberRoidSuffixes;
     private final Boolean isDryRun;
+    private final String registryAdminClientId;
 
-    public DeleteProberDataMapper(ImmutableSet<String> proberRoidSuffixes, Boolean isDryRun) {
+    public DeleteProberDataMapper(
+        ImmutableSet<String> proberRoidSuffixes, Boolean isDryRun, String registryAdminClientId) {
       this.proberRoidSuffixes = proberRoidSuffixes;
       this.isDryRun = isDryRun;
+      this.registryAdminClientId = registryAdminClientId;
     }
 
     @Override
@@ -123,22 +143,47 @@ public class DeleteProberDataAction implements Runnable {
     }
 
     private void deleteDomain(final Key<DomainBase> domainKey) {
-      final DomainBase domain = ofy().load().key(domainKey).now();
+      final DomainBase domainBase = ofy().load().key(domainKey).now();
-      if (domain == null) {
+      if (domainBase == null) {
         // Depending on how stale Datastore indexes are, we can get keys to resources that are
         // already deleted (e.g. by a recent previous invocation of this mapreduce). So ignore them.
         getContext().incrementCounter("already deleted");
         return;
       }
-      if (domain instanceof DomainApplication) {
+      if (domainBase instanceof DomainApplication) {
         // Cover the case where we somehow have a domain application with a prober ROID suffix.
         getContext().incrementCounter("skipped, domain application");
         return;
       }
+
+      DomainResource domain = (DomainResource) domainBase;
       if (domain.getFullyQualifiedDomainName().equals("nic." + domain.getTld())) {
         getContext().incrementCounter("skipped, NIC domain");
         return;
       }
+      if (domain.getCreationTime().isAfter(DateTime.now(UTC).minusHours(1))) {
+        getContext().incrementCounter("skipped, domain too new");
+        return;
+      }
+      if (!domain.getSubordinateHosts().isEmpty()) {
+        logger.warningfmt("Cannot delete domain %s because it has subordinate hosts.", domainKey);
+        getContext().incrementCounter("skipped, had subordinate host(s)");
+        return;
+      }
+
+      // If the domain is still active, that means that the prober encountered a failure and did not
+      // successfully soft-delete the domain (thus leaving its DNS entry published). We soft-delete
+      // it now so that the DNS entry can be handled. The domain will then be hard-deleted the next
+      // time the mapreduce is run.
+      if (EppResourceUtils.isActive(domain, DateTime.now(UTC))) {
+        if (isDryRun) {
+          logger.infofmt("Would soft-delete the active domain: %s", domainKey);
+        } else {
+          softDeleteDomain(domain);
+        }
+        getContext().incrementCounter("domains soft-deleted");
+        return;
+      }
 
       final Key<EppResourceIndex> eppIndex = Key.create(EppResourceIndex.create(domainKey));
       final Key<? extends ForeignKeyIndex<?>> fki = ForeignKeyIndex.createKey(domain);
@@ -155,15 +200,42 @@ public class DeleteProberDataAction implements Runnable {
               .addAll(domainAndDependentKeys)
               .build();
           if (isDryRun) {
-            logger.infofmt("Would delete the following entities: %s", allKeys);
+            logger.infofmt("Would hard-delete the following entities: %s", allKeys);
           } else {
             ofy().deleteWithoutBackup().keys(allKeys);
           }
           return allKeys.size();
         }
       });
-      getContext().incrementCounter("domains deleted");
+      getContext().incrementCounter("domains hard-deleted");
-      getContext().incrementCounter("total entities deleted", entitiesDeleted);
+      getContext().incrementCounter("total entities hard-deleted", entitiesDeleted);
+    }
+
+    private void softDeleteDomain(final DomainResource domain) {
+      ofy().transactNew(new VoidWork() {
+        @Override
+        public void vrun() {
+          DomainResource deletedDomain = domain
+              .asBuilder()
+              .setDeletionTime(ofy().getTransactionTime())
+              .setStatusValues(null)
+              .build();
+          HistoryEntry historyEntry = new HistoryEntry.Builder()
+              .setParent(domain)
+              .setType(DOMAIN_DELETE)
+              .setModificationTime(ofy().getTransactionTime())
+              .setBySuperuser(true)
+              .setReason("Deletion of prober data")
+              .setClientId(registryAdminClientId)
+              .build();
+          // Note that we don't bother handling grace periods, billing events, pending transfers,
+          // poll messages, or auto-renews because these will all be hard-deleted the next time the
+          // mapreduce runs anyway.
+          ofy().save().entities(deletedDomain, historyEntry);
+          updateForeignKeyIndexDeletionTime(deletedDomain);
+          dnsQueue.addDomainRefreshTask(deletedDomain.getFullyQualifiedDomainName());
+        }
+      });
     }
   }
 }

javatests/google/registry/batch/DeleteProberDataActionTest.java

@@ -15,13 +15,21 @@
 package google.registry.batch;
 
 import static com.google.common.truth.Truth.assertThat;
+import static com.google.common.truth.Truth.assert_;
+import static google.registry.model.EppResourceUtils.loadByForeignKey;
 import static google.registry.model.ofy.ObjectifyService.ofy;
 import static google.registry.testing.DatastoreHelper.createTld;
+import static google.registry.testing.DatastoreHelper.newDomainResource;
 import static google.registry.testing.DatastoreHelper.persistActiveDomain;
+import static google.registry.testing.DatastoreHelper.persistActiveHost;
 import static google.registry.testing.DatastoreHelper.persistDeletedDomain;
+import static google.registry.testing.DatastoreHelper.persistDomainAsDeleted;
 import static google.registry.testing.DatastoreHelper.persistResource;
 import static google.registry.testing.DatastoreHelper.persistSimpleResource;
+import static google.registry.testing.TaskQueueHelper.assertDnsTasksEnqueued;
+import static google.registry.util.DateTimeUtils.END_OF_TIME;
 import static google.registry.util.DateTimeUtils.START_OF_TIME;
+import static org.joda.time.DateTimeZone.UTC;
 
 import com.google.common.collect.ImmutableSet;
 import com.googlecode.objectify.Key;
@@ -75,6 +83,7 @@ public class DeleteProberDataActionTest extends MapreduceTestCase<DeleteProberDa
     action.mrRunner = makeDefaultRunner();
     action.response = new FakeResponse();
     action.isDryRun = false;
+    action.registryAdminClientId = "TheRegistrar";
   }
 
   private void runMapreduce() throws Exception {
@@ -107,14 +116,90 @@ public class DeleteProberDataActionTest extends MapreduceTestCase<DeleteProberDa
   }
 
   @Test
-  public void testSuccess_dryRunDoesntDeleteData() throws Exception {
+  public void testDryRun_doesntDeleteData() throws Exception {
     Set<ImmutableObject> tldEntities = persistLotsOfDomains("tld");
     Set<ImmutableObject> oaEntities = persistLotsOfDomains("oa-canary.test");
     action.isDryRun = true;
+    runMapreduce();
     assertNotDeleted(tldEntities);
     assertNotDeleted(oaEntities);
   }
 
+  @Test
+  public void testSuccess_activeDomain_isSoftDeleted() throws Exception {
+    DomainResource domain = persistResource(
+        newDomainResource("blah.ib-any.test")
+            .asBuilder()
+            .setCreationTimeForTest(DateTime.now(UTC).minusYears(1))
+            .build());
+    runMapreduce();
+    DateTime timeAfterDeletion = DateTime.now(UTC);
+    assertThat(loadByForeignKey(DomainResource.class, "blah.ib-any.test", timeAfterDeletion))
+        .isNull();
+    assertThat(ofy().load().entity(domain).now().getDeletionTime()).isLessThan(timeAfterDeletion);
+    assertDnsTasksEnqueued("blah.ib-any.test");
+  }
+
+  @Test
+  public void test_recentlyCreatedDomain_isntDeletedYet() throws Exception {
+    persistResource(
+        newDomainResource("blah.ib-any.test")
+            .asBuilder()
+            .setCreationTimeForTest(DateTime.now(UTC).minusSeconds(1))
+            .build());
+    runMapreduce();
+    DomainResource domain =
+        loadByForeignKey(DomainResource.class, "blah.ib-any.test", DateTime.now(UTC));
+    assertThat(domain).isNotNull();
+    assertThat(domain.getDeletionTime()).isEqualTo(END_OF_TIME);
+  }
+
+  @Test
+  public void testDryRun_doesntSoftDeleteData() throws Exception {
+    DomainResource domain = persistResource(
+        newDomainResource("blah.ib-any.test")
+            .asBuilder()
+            .setCreationTimeForTest(DateTime.now(UTC).minusYears(1))
+            .build());
+    action.isDryRun = true;
+    runMapreduce();
+    assertThat(ofy().load().entity(domain).now().getDeletionTime()).isEqualTo(END_OF_TIME);
+  }
+
+  @Test
+  public void test_domainWithSubordinateHosts_isSkipped() throws Exception {
+    persistActiveHost("ns1.blah.ib-any.test");
+    DomainResource nakedDomain =
+        persistDeletedDomain("todelete.ib-any.test", DateTime.now(UTC).minusYears(1));
+    DomainResource domainWithSubord =
+        persistDomainAsDeleted(
+            newDomainResource("blah.ib-any.test")
+                .asBuilder()
+                .setSubordinateHosts(ImmutableSet.of("ns1.blah.ib-any.test"))
+                .build(),
+            DateTime.now(UTC).minusYears(1));
+    runMapreduce();
+    assertThat(ofy().load().entity(domainWithSubord).now()).isNotNull();
+    assertThat(ofy().load().entity(nakedDomain).now()).isNull();
+  }
+
+  @Test
+  public void testFailure_registryAdminClientId_isRequiredForSoftDeletion() throws Exception {
+    persistResource(
+        newDomainResource("blah.ib-any.test")
+            .asBuilder()
+            .setCreationTimeForTest(DateTime.now(UTC).minusYears(1))
+            .build());
+    action.registryAdminClientId = null;
+    try {
+      runMapreduce();
+    } catch (IllegalStateException e) {
+      assertThat(e).hasMessageThat().contains("Registry admin client ID must be configured");
+      return;
+    }
+    assert_().fail("Expected IllegalStateException");
+  }
+
   /**
    * Persists and returns a domain and a descendant history entry, billing event, and poll message,
    * along with the ForeignKeyIndex and EppResourceIndex.