Add a rate limiter to remove all domain contacts action (#2838)

The maximum QPS defaults to 10, but can also be specified at runtime through
use of a query-string parameter.

BUG = http://b/439636188

.gitignore

@@ -18,6 +18,13 @@ gjf.out
 # virtual machine crash logs, see http://www.java.com/en/download/help/error_hotspot.xml
 hs_err_pid*
 
+# Environment-specific configuration files
+core/src/main/java/google/registry/config/files/nomulus-config-alpha.yaml
+core/src/main/java/google/registry/config/files/nomulus-config-crash.yaml
+core/src/main/java/google/registry/config/files/nomulus-config-production.yaml
+core/src/main/java/google/registry/config/files/nomulus-config-qa.yaml
+core/src/main/java/google/registry/config/files/nomulus-config-sandbox.yaml
+
 ######################################################################
 # Eclipse Ignores
 

core/src/main/java/google/registry/batch/BatchModule.java

@@ -29,9 +29,11 @@ import static google.registry.request.RequestParameters.extractRequiredParameter
 import static google.registry.request.RequestParameters.extractSetOfDatetimeParameters;
 
 import com.google.common.collect.ImmutableSet;
+import com.google.common.util.concurrent.RateLimiter;
 import dagger.Module;
 import dagger.Provides;
 import google.registry.request.Parameter;
+import jakarta.inject.Named;
 import jakarta.servlet.http.HttpServletRequest;
 import java.util.Optional;
 import org.joda.time.DateTime;
@@ -137,4 +139,18 @@ public class BatchModule {
   static boolean provideIsFast(HttpServletRequest req) {
     return extractBooleanParameter(req, PARAM_FAST);
   }
+
+  private static final int DEFAULT_MAX_QPS = 10;
+
+  @Provides
+  @Parameter("maxQps")
+  static int provideMaxQps(HttpServletRequest req) {
+    return extractOptionalIntParameter(req, "maxQps").orElse(DEFAULT_MAX_QPS);
+  }
+
+  @Provides
+  @Named("removeAllDomainContacts")
+  static RateLimiter provideRemoveAllDomainContactsRateLimiter(@Parameter("maxQps") int maxQps) {
+    return RateLimiter.create(maxQps);
+  }
 }

core/src/main/java/google/registry/batch/RemoveAllDomainContactsAction.java

@@ -30,6 +30,7 @@ import com.google.common.base.Ascii;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.flogger.FluentLogger;
+import com.google.common.util.concurrent.RateLimiter;
 import google.registry.config.RegistryConfig.Config;
 import google.registry.flows.EppController;
 import google.registry.flows.EppRequestSource;
@@ -48,6 +49,7 @@ import google.registry.request.Response;
 import google.registry.request.auth.Auth;
 import google.registry.request.lock.LockHandler;
 import jakarta.inject.Inject;
+import jakarta.inject.Named;
 import java.util.List;
 import java.util.concurrent.Callable;
 import java.util.logging.Level;
@@ -79,6 +81,7 @@ public class RemoveAllDomainContactsAction implements Runnable {
   private final EppController eppController;
   private final String registryAdminClientId;
   private final LockHandler lockHandler;
+  private final RateLimiter rateLimiter;
   private final Response response;
   private final String updateDomainXml;
   private int successes = 0;
@@ -91,10 +94,12 @@ public class RemoveAllDomainContactsAction implements Runnable {
       EppController eppController,
       @Config("registryAdminClientId") String registryAdminClientId,
       LockHandler lockHandler,
+      @Named("removeAllDomainContacts") RateLimiter rateLimiter,
       Response response) {
     this.eppController = eppController;
     this.registryAdminClientId = registryAdminClientId;
     this.lockHandler = lockHandler;
+    this.rateLimiter = rateLimiter;
     this.response = response;
     this.updateDomainXml =
         readResourceUtf8(RemoveAllDomainContactsAction.class, "domain_remove_contacts.xml");
@@ -146,7 +151,10 @@ public class RemoveAllDomainContactsAction implements Runnable {
                           .setMaxResults(BATCH_SIZE)
                           .getResultList());
 
-      domainRepoIdsBatch.forEach(this::runDomainUpdateFlow);
+      for (String domainRepoId : domainRepoIdsBatch) {
+        rateLimiter.acquire();
+        runDomainUpdateFlow(domainRepoId);
+      }
     } while (!domainRepoIdsBatch.isEmpty());
     String msg =
         String.format(
@@ -159,12 +167,18 @@ public class RemoveAllDomainContactsAction implements Runnable {
   private void runDomainUpdateFlow(String repoId) {
     // Create a new transaction that the flow's execution will be enlisted in that loads the domain
     // transactionally. This way we can ensure that nothing else has modified the domain in question
-    // in the intervening period since the query above found it.
-    boolean success = tm().transact(() -> runDomainUpdateFlowInner(repoId));
-    if (success) {
-      successes++;
-    } else {
-      failures++;
+    // in the intervening period since the query above found it. If a single domain update fails
+    // permanently, log it and move on to not block processing all the other domains.
+    try {
+      boolean success = tm().transact(() -> runDomainUpdateFlowInner(repoId));
+      if (success) {
+        successes++;
+      } else {
+        failures++;
+      }
+    } catch (Throwable t) {
+      logger.atWarning().withCause(t).log(
+          "Failed updating domain with repoId %s; skipping.", repoId);
     }
   }
 

core/src/main/java/google/registry/config/files/README.md

@@ -0,0 +1,13 @@
+# Nomulus Environment Configuration
+
+The configuration files for the different Nomulus environments are not included in this repository. To configure and run a specific environment, you will need to create the corresponding YAML configuration file in this directory.
+
+The following is a list of the environment configuration files that you may need to create:
+
+*   `nomulus-config-alpha.yaml`
+*   `nomulus-config-crash.yaml`
+*   `nomulus-config-qa.yaml`
+*   `nomulus-config-sandbox.yaml`
+*   `nomulus-config-production.yaml`
+
+Please create the relevant file for the environment you intend to use and populate it with the necessary configuration details.

core/src/main/java/google/registry/config/files/nomulus-config-alpha.yaml

@@ -1 +0,0 @@
-# Add environment-specific configuration here.

core/src/main/java/google/registry/config/files/nomulus-config-crash.yaml

@@ -1 +0,0 @@
-# Add environment-specific configuration here.

core/src/main/java/google/registry/config/files/nomulus-config-production.yaml

@@ -1 +0,0 @@
-# Add environment-specific configuration here.

core/src/main/java/google/registry/config/files/nomulus-config-qa.yaml

@@ -1 +0,0 @@
-# Add environment-specific configuration here.

core/src/main/java/google/registry/config/files/nomulus-config-sandbox.yaml

@@ -1 +0,0 @@
-# Add environment-specific configuration here.

core/src/main/java/google/registry/tools/CreateCdnsTld.java

@@ -47,6 +47,11 @@ final class CreateCdnsTld extends ConfirmingCommand {
   )
   String name;
 
+  @Parameter(
+      names = "--skip_sandbox_tld_check",
+      description = "In Sandbox, skip the dns_name format check.")
+  boolean skipSandboxTldCheck;
+
   @Inject
   @Config("projectId")
   String projectId;
@@ -61,10 +66,15 @@ final class CreateCdnsTld extends ConfirmingCommand {
   protected void init() {
     // Sandbox talks to production Cloud DNS.  As a result, we can't configure any domains with a
     // suffix that might be used by customers on the same nameserver set.  Limit the user to setting
-    // up *.test TLDs.
-    if (RegistryToolEnvironment.get() == RegistryToolEnvironment.SANDBOX
-        && !dnsName.endsWith(".test.")) {
-      throw new IllegalArgumentException("Sandbox TLDs must be of the form \"*.test.\"");
+    // up *.test TLDs unless the user declares that the name is approved.
+    //
+    // The name format check simply provides a user-friendly error message. If the user wrongly
+    // declares name approval, the request to the Cloud DNS API will still fail.
+    if (RegistryToolEnvironment.get() == RegistryToolEnvironment.SANDBOX) {
+      if (!skipSandboxTldCheck && !dnsName.endsWith(".test.")) {
+        throw new IllegalArgumentException(
+            "Sandbox TLDs must be approved or in the form \"*.test.\"");
+      }
     }
 
     managedZone =

core/src/main/java/google/registry/tools/GenerateAllocationTokensCommand.java

@@ -15,6 +15,7 @@
 package google.registry.tools;
 
 import static com.google.common.base.Preconditions.checkArgument;
+import static com.google.common.collect.ImmutableList.toImmutableList;
 import static com.google.common.collect.ImmutableSet.toImmutableSet;
 import static com.google.common.collect.Sets.difference;
 import static google.registry.model.billing.BillingBase.RenewalPriceBehavior.DEFAULT;
@@ -46,6 +47,9 @@ import google.registry.model.domain.token.AllocationToken;
 import google.registry.model.domain.token.AllocationToken.RegistrationBehavior;
 import google.registry.model.domain.token.AllocationToken.TokenStatus;
 import google.registry.model.domain.token.AllocationToken.TokenType;
+import google.registry.model.registrar.Registrar;
+import google.registry.model.tld.Tld;
+import google.registry.model.tld.Tlds;
 import google.registry.persistence.VKey;
 import google.registry.tools.params.MoneyParameter;
 import google.registry.tools.params.TransitionListParameter.TokenStatusTransitions;
@@ -291,10 +295,12 @@ class GenerateAllocationTokensCommand implements Command {
         !ImmutableList.of("").equals(allowedClientIds),
         "Either omit --allowed_client_ids if all registrars are allowed, or include a"
             + " comma-separated list");
+    verifyAllRegistrarIdsExist(allowedClientIds);
 
     checkArgument(
         !ImmutableList.of("").equals(allowedTlds),
         "Either omit --allowed_tlds if all TLDs are allowed, or include a comma-separated list");
+    verifyAllTldsExist(allowedTlds);
 
     if (ImmutableList.of("").equals(allowedEppActions)) {
       allowedEppActions = ImmutableList.of();
@@ -326,6 +332,34 @@ class GenerateAllocationTokensCommand implements Command {
     }
   }
 
+  static void verifyAllRegistrarIdsExist(@Nullable List<String> allowedClientIds) {
+    // a null/empty list means that all registrars are allowed
+    if (isNullOrEmpty(allowedClientIds)) {
+      return;
+    }
+    ImmutableSet<String> allRegistrarIds =
+        Registrar.loadAllKeysCached().stream()
+            .map(VKey::getKey)
+            .map(Object::toString)
+            .collect(toImmutableSet());
+    ImmutableList<String> badRegistrarIds =
+        allowedClientIds.stream()
+            .filter(id -> !allRegistrarIds.contains(id))
+            .collect(toImmutableList());
+    checkArgument(badRegistrarIds.isEmpty(), "Unknown registrar ID(s) %s", badRegistrarIds);
+  }
+
+  static void verifyAllTldsExist(@Nullable List<String> allowedTlds) {
+    // a null/empty list means that all TLDs are allowed
+    if (isNullOrEmpty(allowedTlds)) {
+      return;
+    }
+    ImmutableSet<String> allTlds = Tlds.getTldsOfType(Tld.TldType.REAL);
+    ImmutableList<String> badTlds =
+        allowedTlds.stream().filter(tld -> !allTlds.contains(tld)).collect(toImmutableList());
+    checkArgument(badTlds.isEmpty(), "Unknown REAL TLD(s) %s", badTlds);
+  }
+
   private void verifyTokenStringsDoNotExist() {
     ImmutableSet<String> existingTokenStrings =
         getExistingTokenStrings(ImmutableSet.copyOf(tokenStrings));

core/src/main/java/google/registry/tools/UpdateAllocationTokensCommand.java

@@ -157,6 +157,9 @@ final class UpdateAllocationTokensCommand extends UpdateOrDeleteAllocationTokens
       endToken = true;
     }
 
+    GenerateAllocationTokensCommand.verifyAllRegistrarIdsExist(allowedClientIds);
+    GenerateAllocationTokensCommand.verifyAllTldsExist(allowedTlds);
+
     tokensToSave =
         tm().transact(
                 () ->

core/src/test/java/google/registry/batch/RemoveAllDomainContactsActionTest.java

@@ -23,9 +23,11 @@ import static google.registry.testing.DatabaseHelper.newDomain;
 import static google.registry.testing.DatabaseHelper.persistActiveContact;
 import static google.registry.testing.DatabaseHelper.persistResource;
 import static google.registry.util.DateTimeUtils.START_OF_TIME;
+import static org.mockito.Mockito.mock;
 
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.ImmutableSortedMap;
+import com.google.common.util.concurrent.RateLimiter;
 import google.registry.flows.DaggerEppTestComponent;
 import google.registry.flows.EppController;
 import google.registry.flows.EppTestComponent.FakesAndMocksModule;
@@ -51,6 +53,7 @@ class RemoveAllDomainContactsActionTest {
       new JpaTestExtensions.Builder().buildIntegrationTestExtension();
 
   private final FakeResponse response = new FakeResponse();
+  private final RateLimiter rateLimiter = mock(RateLimiter.class);
   private RemoveAllDomainContactsAction action;
 
   @BeforeEach
@@ -69,7 +72,7 @@ class RemoveAllDomainContactsActionTest {
             .eppController();
     action =
         new RemoveAllDomainContactsAction(
-            eppController, "NewRegistrar", new FakeLockHandler(true), response);
+            eppController, "NewRegistrar", new FakeLockHandler(true), rateLimiter, response);
   }
 
   @Test

core/src/test/java/google/registry/tools/CreateCdnsTldTest.java

@@ -76,11 +76,37 @@ class CreateCdnsTldTest extends CommandTestCase<CreateCdnsTld> {
 
   @Test
   @MockitoSettings(strictness = Strictness.LENIENT)
-  void testSandboxTldRestrictions() {
+  void testSandboxTldRestrictions_Disallowed() {
     IllegalArgumentException thrown =
         assertThrows(
             IllegalArgumentException.class,
-            () -> runCommandInEnvironment(RegistryToolEnvironment.SANDBOX, "--dns_name=foobar."));
-    assertThat(thrown).hasMessageThat().contains("Sandbox TLDs must be of the form \"*.test.\"");
+            () ->
+                runCommandInEnvironment(
+                    RegistryToolEnvironment.SANDBOX,
+                    "--dns_name=foobar.",
+                    "--description=test run",
+                    "--force"));
+    assertThat(thrown)
+        .hasMessageThat()
+        .contains("Sandbox TLDs must be approved or in the form \"*.test.\"");
+  }
+
+  @Test
+  void testSandboxTldRestrictions_tldCheckSkipped() throws Exception {
+    runCommandInEnvironment(
+        RegistryToolEnvironment.SANDBOX,
+        "--dns_name=foobar.",
+        "--description=test run",
+        "--force",
+        "--skip_sandbox_tld_check");
+  }
+
+  @Test
+  void testSandboxTldRestrictions_testTld() throws Exception {
+    runCommandInEnvironment(
+        RegistryToolEnvironment.SANDBOX,
+        "--dns_name=abc.test.",
+        "--description=test run",
+        "--force");
   }
 }

core/src/test/java/google/registry/tools/GenerateAllocationTokensCommandTest.java

@@ -39,6 +39,7 @@ import google.registry.model.domain.fee.FeeQueryCommandExtensionItem.CommandName
 import google.registry.model.domain.token.AllocationToken;
 import google.registry.model.domain.token.AllocationToken.TokenStatus;
 import google.registry.model.reporting.HistoryEntry.HistoryEntryId;
+import google.registry.testing.DatabaseHelper;
 import google.registry.testing.DeterministicStringGenerator;
 import google.registry.testing.DeterministicStringGenerator.Rule;
 import google.registry.util.StringGenerator.Alphabets;
@@ -56,6 +57,7 @@ class GenerateAllocationTokensCommandTest extends CommandTestCase<GenerateAlloca
 
   @BeforeEach
   void beforeEach() {
+    DatabaseHelper.createTlds("tld", "example");
     command.stringGenerator = new DeterministicStringGenerator(Alphabets.BASE_58);
   }
 
@@ -531,6 +533,26 @@ class GenerateAllocationTokensCommandTest extends CommandTestCase<GenerateAlloca
         .isEqualTo("For DEFAULT_PROMO tokens, must specify --token_status_transitions");
   }
 
+  @Test
+  void testFailure_badTld() {
+    assertThat(
+            assertThrows(
+                IllegalArgumentException.class,
+                () -> runCommand("--number", "10", "--allowed_tlds", "badtld")))
+        .hasMessageThat()
+        .isEqualTo("Unknown REAL TLD(s) [badtld]");
+  }
+
+  @Test
+  void testFailure_badRegistrar() {
+    assertThat(
+            assertThrows(
+                IllegalArgumentException.class,
+                () -> runCommand("--number", "10", "--allowed_client_ids", "badregistrar")))
+        .hasMessageThat()
+        .isEqualTo("Unknown registrar ID(s) [badregistrar]");
+  }
+
   private AllocationToken createToken(
       String token,
       @Nullable HistoryEntryId redemptionHistoryEntryId,

core/src/test/java/google/registry/tools/UpdateAllocationTokensCommandTest.java

@@ -40,14 +40,21 @@ import google.registry.model.domain.fee.FeeQueryCommandExtensionItem.CommandName
 import google.registry.model.domain.token.AllocationToken;
 import google.registry.model.domain.token.AllocationToken.RegistrationBehavior;
 import google.registry.model.domain.token.AllocationToken.TokenStatus;
+import google.registry.testing.DatabaseHelper;
 import org.joda.money.CurrencyUnit;
 import org.joda.money.Money;
 import org.joda.time.DateTime;
+import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 
 /** Unit tests for {@link UpdateAllocationTokensCommand}. */
 class UpdateAllocationTokensCommandTest extends CommandTestCase<UpdateAllocationTokensCommand> {
 
+  @BeforeEach
+  void beforeEach() {
+    DatabaseHelper.createTlds("tld", "example");
+  }
+
   @Test
   void testUpdateTlds_setTlds() throws Exception {
     AllocationToken token =
@@ -64,14 +71,24 @@ class UpdateAllocationTokensCommandTest extends CommandTestCase<UpdateAllocation
     assertThat(reloadResource(token).getAllowedTlds()).isEmpty();
   }
 
+  @Test
+  void testUpdateTlds_badTlds() {
+    persistResource(builderWithPromo().build());
+    assertThat(
+            assertThrows(
+                IllegalArgumentException.class, () -> runCommandForced("--allowed_tlds=badtld")))
+        .hasMessageThat()
+        .isEqualTo("Unknown REAL TLD(s) [badtld]");
+  }
+
   @Test
   void testUpdateClientIds_setClientIds() throws Exception {
     AllocationToken token =
         persistResource(
             builderWithPromo().setAllowedRegistrarIds(ImmutableSet.of("toRemove")).build());
-    runCommandForced("--prefix", "token", "--allowed_client_ids", "clientone,clienttwo");
+    runCommandForced("--prefix", "token", "--allowed_client_ids", "TheRegistrar,NewRegistrar");
     assertThat(reloadResource(token).getAllowedRegistrarIds())
-        .containsExactly("clientone", "clienttwo");
+        .containsExactly("TheRegistrar", "NewRegistrar");
   }
 
   @Test
@@ -83,6 +100,17 @@ class UpdateAllocationTokensCommandTest extends CommandTestCase<UpdateAllocation
     assertThat(reloadResource(token).getAllowedRegistrarIds()).isEmpty();
   }
 
+  @Test
+  void testUpdateClientIds_badClientId() {
+    persistResource(builderWithPromo().build());
+    assertThat(
+            assertThrows(
+                IllegalArgumentException.class,
+                () -> runCommandForced("--allowed_client_ids=badregistrar")))
+        .hasMessageThat()
+        .isEqualTo("Unknown registrar ID(s) [badregistrar]");
+  }
+
   @Test
   void testUpdateEppActions_setEppActions() throws Exception {
     AllocationToken token =

release/cloudbuild-release.yaml

@@ -42,6 +42,20 @@ steps:
     rm -rf .git && rm -rf nomulus-internal/.git
     cp -rf nomulus-internal/* .
     rm -rf nomulus-internal
+# Remove environment configs from .gitignore
+- name: 'gcr.io/cloud-builders/git'
+  entrypoint: /bin/bash
+  args:
+  - -c
+  - |
+    set -e
+    sed -i \
+        -e '\#core/src/main/java/google/registry/config/files/nomulus-config-alpha.yaml#d' \
+        -e '\#core/src/main/java/google/registry/config/files/nomulus-config-crash.yaml#d' \
+        -e '\#core/src/main/java/google/registry/config/files/nomulus-config-production.yaml#d' \
+        -e '\#core/src/main/java/google/registry/config/files/nomulus-config-qa.yaml#d' \
+        -e '\#core/src/main/java/google/registry/config/files/nomulus-config-sandbox.yaml#d' \
+        .gitignore
 # Build the builder image and pull the base images, them upload them to GCR.
 - name: 'gcr.io/cloud-builders/docker'
   entrypoint: /bin/bash