diff --git a/core/src/main/java/google/registry/bsa/BsaValidateAction.java b/core/src/main/java/google/registry/bsa/BsaValidateAction.java index 158161cdb..e93f636fd 100644 --- a/core/src/main/java/google/registry/bsa/BsaValidateAction.java +++ b/core/src/main/java/google/registry/bsa/BsaValidateAction.java @@ -16,27 +16,42 @@ package google.registry.bsa; import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.base.Throwables.getStackTraceAsString; +import static com.google.common.collect.ImmutableList.toImmutableList; import static google.registry.bsa.BsaTransactions.bsaQuery; +import static google.registry.bsa.ReservedDomainsUtils.isReservedDomain; import static google.registry.bsa.persistence.Queries.batchReadBsaLabelText; +import static google.registry.persistence.transaction.TransactionManagerFactory.replicaTm; import static google.registry.request.Action.Method.GET; import static google.registry.request.Action.Method.POST; import static javax.servlet.http.HttpServletResponse.SC_OK; import com.google.common.base.Joiner; import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Iterables; import com.google.common.collect.Sets; import com.google.common.collect.Sets.SetView; import com.google.common.flogger.FluentLogger; +import com.google.common.net.InternetDomainName; +import google.registry.bsa.api.UnblockableDomain; +import google.registry.bsa.api.UnblockableDomain.Reason; import google.registry.bsa.persistence.DownloadScheduler; +import google.registry.bsa.persistence.Queries; import google.registry.config.RegistryConfig.Config; +import google.registry.model.ForeignKeyUtils; +import google.registry.model.domain.Domain; +import google.registry.persistence.VKey; import google.registry.request.Action; import google.registry.request.Response; import google.registry.request.auth.Auth; +import google.registry.util.Clock; +import java.util.Objects; import java.util.Optional; import java.util.stream.Stream; import javax.inject.Inject; +import org.joda.time.DateTime; +import org.joda.time.Duration; /** Validates the BSA data in the database against the most recent block lists. */ @Action( @@ -53,6 +68,8 @@ public class BsaValidateAction implements Runnable { private final IdnChecker idnChecker; private final BsaEmailSender emailSender; private final int transactionBatchSize; + private final Duration maxStaleness; + private final Clock clock; private final BsaLock bsaLock; private final Response response; @@ -62,12 +79,16 @@ public class BsaValidateAction implements Runnable { IdnChecker idnChecker, BsaEmailSender emailSender, @Config("bsaTxnBatchSize") int transactionBatchSize, + @Config("bsaValidationMaxStaleness") Duration maxStaleness, + Clock clock, BsaLock bsaLock, Response response) { this.gcsClient = gcsClient; this.idnChecker = idnChecker; this.emailSender = emailSender; this.transactionBatchSize = transactionBatchSize; + this.maxStaleness = maxStaleness; + this.clock = clock; this.bsaLock = bsaLock; this.response = response; } @@ -103,6 +124,7 @@ public class BsaValidateAction implements Runnable { ImmutableList.Builder errorsBuilder = new ImmutableList.Builder<>(); errorsBuilder.addAll(checkBsaLabels(downloadJobName.get())); + errorsBuilder.addAll(checkUnblockableDomains()); ImmutableList errors = errorsBuilder.build(); @@ -150,6 +172,66 @@ public class BsaValidateAction implements Runnable { return errors.build(); } + ImmutableList checkUnblockableDomains() { + ImmutableList.Builder errors = new ImmutableList.Builder<>(); + Optional lastRead = Optional.empty(); + ImmutableList batch; + do { + batch = Queries.batchReadUnblockableDomains(lastRead, transactionBatchSize); + ImmutableMap> activeDomains = + ForeignKeyUtils.load( + Domain.class, + batch.stream().map(UnblockableDomain::domainName).collect(toImmutableList()), + clock.nowUtc()); + for (var unblockable : batch) { + verifyDomainStillUnblockableWithReason(unblockable, activeDomains).ifPresent(errors::add); + } + if (!batch.isEmpty()) { + lastRead = Optional.of(Iterables.getLast(batch)); + } + } while (batch.size() == transactionBatchSize); + return errors.build(); + } + + Optional verifyDomainStillUnblockableWithReason( + UnblockableDomain domain, ImmutableMap> activeDomains) { + DateTime now = clock.nowUtc(); + boolean isRegistered = activeDomains.containsKey(domain.domainName()); + boolean isReserved = isReservedDomain(domain.domainName(), now); + InternetDomainName domainName = InternetDomainName.from(domain.domainName()); + boolean isInvalid = idnChecker.getAllValidIdns(domainName.parts().get(0)).isEmpty(); + + Reason expectedReason = + isRegistered + ? Reason.REGISTERED + : (isReserved ? Reason.RESERVED : (isInvalid ? Reason.INVALID : null)); + if (Objects.equals(expectedReason, domain.reason())) { + return Optional.empty(); + } + if (isRegistered || domain.reason().equals(Reason.REGISTERED)) { + if (isStalenessAllowed(isRegistered, activeDomains.get(domain.domainName()))) { + return Optional.empty(); + } + } + return Optional.of( + String.format( + "%s: should be %s, found %s", + domain.domainName(), + expectedReason != null ? expectedReason.name() : "BLOCKABLE", + domain.reason())); + } + + boolean isStalenessAllowed(boolean isNewDomain, VKey domainVKey) { + Domain domain = bsaQuery(() -> replicaTm().loadByKey(domainVKey)); + var now = clock.nowUtc(); + if (isNewDomain) { + return domain.getCreationTime().plus(maxStaleness).isAfter(now); + } else { + return domain.getDeletionTime().isBefore(now) + && domain.getDeletionTime().plus(maxStaleness).isAfter(now); + } + } + /** Returns unique labels across all block lists in the download specified by {@code jobName}. */ ImmutableSet fetchDownloadedLabels(String jobName) { ImmutableSet.Builder labelsBuilder = new ImmutableSet.Builder<>(); @@ -171,10 +253,11 @@ public class BsaValidateAction implements Runnable { Optional lastRead = Optional.empty(); do { batch = batchReadBsaLabelText(lastRead, batchSize); - batch.forEach(labelsBuilder::add); + labelsBuilder.addAll(batch); if (!batch.isEmpty()) { lastRead = Optional.of(Iterables.getLast(batch)); } + } while (batch.size() == batchSize); return labelsBuilder.build(); } diff --git a/core/src/main/java/google/registry/bsa/persistence/Queries.java b/core/src/main/java/google/registry/bsa/persistence/Queries.java index c85585483..8ca109100 100644 --- a/core/src/main/java/google/registry/bsa/persistence/Queries.java +++ b/core/src/main/java/google/registry/bsa/persistence/Queries.java @@ -15,6 +15,7 @@ package google.registry.bsa.persistence; import static com.google.common.base.Verify.verify; +import static com.google.common.collect.ImmutableList.toImmutableList; import static google.registry.bsa.BsaStringUtils.DOMAIN_SPLITTER; import static google.registry.bsa.BsaTransactions.bsaQuery; import static google.registry.persistence.transaction.TransactionManagerFactory.tm; @@ -22,6 +23,7 @@ import static google.registry.persistence.transaction.TransactionManagerFactory. import com.google.common.collect.ImmutableCollection; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; +import google.registry.bsa.api.UnblockableDomain; import google.registry.model.CreateAutoTimestamp; import java.util.List; import java.util.Optional; @@ -45,7 +47,6 @@ public final class Queries { public static ImmutableList batchReadBsaLabelText( Optional lastRead, int batchSize) { - return ImmutableList.copyOf( bsaQuery( () -> @@ -58,6 +59,13 @@ public final class Queries { .getResultList())); } + public static ImmutableList batchReadUnblockableDomains( + Optional lastRead, int batchSize) { + return batchReadUnblockables(lastRead.map(BsaUnblockableDomain::of), batchSize).stream() + .map(BsaUnblockableDomain::toUnblockableDomain) + .collect(toImmutableList()); + } + static Stream queryBsaUnblockableDomainByLabels( ImmutableCollection labels) { return ((Stream) diff --git a/core/src/main/java/google/registry/config/RegistryConfig.java b/core/src/main/java/google/registry/config/RegistryConfig.java index f2e5f9e2c..fec748db5 100644 --- a/core/src/main/java/google/registry/config/RegistryConfig.java +++ b/core/src/main/java/google/registry/config/RegistryConfig.java @@ -1488,6 +1488,12 @@ public final class RegistryConfig { return Duration.standardSeconds(config.bsa.domainCreateTxnCommitTimeLagSeconds); } + @Provides + @Config("bsaValidationMaxStaleness") + public static Duration provideBsaValidationMaxStaleness(RegistryConfigSettings config) { + return Duration.standardSeconds(config.bsa.bsaValidationMaxStalenessSeconds); + } + @Provides @Config("bsaAuthUrl") public static String provideBsaAuthUrl(RegistryConfigSettings config) { diff --git a/core/src/main/java/google/registry/config/RegistryConfigSettings.java b/core/src/main/java/google/registry/config/RegistryConfigSettings.java index 70600e082..eab156452 100644 --- a/core/src/main/java/google/registry/config/RegistryConfigSettings.java +++ b/core/src/main/java/google/registry/config/RegistryConfigSettings.java @@ -275,6 +275,7 @@ public class RegistryConfigSettings { public int bsaMaxNopIntervalHours; public int bsaTxnBatchSize; public int domainCreateTxnCommitTimeLagSeconds; + public int bsaValidationMaxStalenessSeconds; public String authUrl; public int authTokenExpirySeconds; public Map dataUrls; diff --git a/core/src/main/java/google/registry/config/files/default-config.yaml b/core/src/main/java/google/registry/config/files/default-config.yaml index 764b703e0..7b85a803e 100644 --- a/core/src/main/java/google/registry/config/files/default-config.yaml +++ b/core/src/main/java/google/registry/config/files/default-config.yaml @@ -640,6 +640,9 @@ bsa: # Number of entities (labels and unblockable domains) to process in a single # DB transaction. bsaTxnBatchSize: 1000 + # Used by `BsaValidateAction`: ignore inconsistencies caused by recent events + # in the past. This is roughly equal to two `BsaRefreshAction` runs. + bsaValidationMaxStalenessSeconds: 3600 # Http endpoint for acquiring Auth tokens. authUrl: "https://" diff --git a/core/src/test/java/google/registry/bsa/BsaValidateActionTest.java b/core/src/test/java/google/registry/bsa/BsaValidateActionTest.java index 94511ac10..e5251171f 100644 --- a/core/src/test/java/google/registry/bsa/BsaValidateActionTest.java +++ b/core/src/test/java/google/registry/bsa/BsaValidateActionTest.java @@ -16,7 +16,12 @@ package google.registry.bsa; import static com.google.common.base.Throwables.getStackTraceAsString; import static com.google.common.truth.Truth.assertThat; +import static google.registry.bsa.persistence.BsaTestingUtils.persistBsaLabel; import static google.registry.bsa.persistence.BsaTestingUtils.persistDownloadSchedule; +import static google.registry.bsa.persistence.BsaTestingUtils.persistUnblockableDomain; +import static google.registry.testing.DatabaseHelper.createTld; +import static google.registry.testing.DatabaseHelper.persistActiveDomain; +import static google.registry.testing.DatabaseHelper.persistDeletedDomain; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.ArgumentMatchers.startsWith; @@ -31,9 +36,12 @@ import com.google.cloud.storage.contrib.nio.testing.LocalStorageHelper; import com.google.common.base.Joiner; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; +import google.registry.bsa.api.UnblockableDomain; +import google.registry.bsa.api.UnblockableDomain.Reason; import google.registry.bsa.persistence.BsaTestingUtils; import google.registry.gcs.GcsUtils; import google.registry.groups.GmailClient; +import google.registry.model.domain.Domain; import google.registry.persistence.transaction.JpaTestExtensions; import google.registry.persistence.transaction.JpaTestExtensions.JpaIntegrationWithCoverageExtension; import google.registry.request.Response; @@ -43,6 +51,7 @@ import google.registry.util.EmailMessage; import java.util.concurrent.Callable; import javax.mail.internet.InternetAddress; import org.joda.time.DateTime; +import org.joda.time.Duration; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; @@ -58,6 +67,8 @@ public class BsaValidateActionTest { private static final String DOWNLOAD_JOB_NAME = "job"; + private static final Duration MAX_STALENESS = Duration.standardMinutes(1); + FakeClock fakeClock = new FakeClock(DateTime.parse("2023-11-09T02:08:57.880Z")); @RegisterExtension @@ -90,8 +101,11 @@ public class BsaValidateActionTest { idnChecker, new BsaEmailSender(gmailClient, emailRecipient), /* transactionBatchSize= */ 500, + MAX_STALENESS, + fakeClock, bsaLock, response); + createTld("app"); } static void createBlockList(GcsClient gcsClient, BlockListType blockListType, String content) @@ -214,6 +228,61 @@ public class BsaValidateActionTest { assertThat(allErrors).contains("Found 1 unexpected labels in the DB. Examples: [test3]"); } + @Test + void isStalenessAllowed_newDomain_allowed() { + persistBsaLabel("label"); + Domain domain = persistActiveDomain("label.app", fakeClock.nowUtc()); + fakeClock.advanceBy(MAX_STALENESS.minus(Duration.standardSeconds(1))); + assertThat(action.isStalenessAllowed(true, domain.createVKey())).isTrue(); + } + + @Test + void isStalenessAllowed_newDomain_notAllowed() { + persistBsaLabel("label"); + Domain domain = persistActiveDomain("label.app", fakeClock.nowUtc()); + fakeClock.advanceBy(MAX_STALENESS); + assertThat(action.isStalenessAllowed(true, domain.createVKey())).isFalse(); + } + + @Test + void isStalenessAllowed_deletedDomain_allowed() { + persistBsaLabel("label"); + Domain domain = persistDeletedDomain("label.app", fakeClock.nowUtc()); + fakeClock.advanceBy(MAX_STALENESS.minus(Duration.standardSeconds(1))); + assertThat(action.isStalenessAllowed(false, domain.createVKey())).isTrue(); + } + + @Test + void isStalenessAllowed_deletedDomain_notAllowed() { + persistBsaLabel("label"); + Domain domain = persistDeletedDomain("label.app", fakeClock.nowUtc()); + fakeClock.advanceBy(MAX_STALENESS); + assertThat(action.isStalenessAllowed(false, domain.createVKey())).isFalse(); + } + + @Test + void checkUnblockableDomain_noError() { + createTld("app"); + persistActiveDomain("label.app"); + persistBsaLabel("label"); + persistUnblockableDomain(UnblockableDomain.of("label", "app", Reason.REGISTERED)); + when(idnChecker.getAllValidIdns(anyString())).thenReturn(ImmutableSet.of(IdnTableEnum.JA)); + + assertThat(action.checkUnblockableDomains()).isEmpty(); + } + + @Test + void checkUnblockableDomain_error() { + createTld("app"); + persistActiveDomain("label.app"); + persistBsaLabel("label"); + persistUnblockableDomain(UnblockableDomain.of("label", "app", Reason.RESERVED)); + when(idnChecker.getAllValidIdns(anyString())).thenReturn(ImmutableSet.of(IdnTableEnum.JA)); + + assertThat(action.checkUnblockableDomains()) + .containsExactly("label.app: should be REGISTERED, found RESERVED"); + } + @Test void notificationSent_cannotAcquireLock() { when(bsaLock.executeWithLock(any())).thenReturn(false);