From 96e33f5b4fac4e943c22777f194c122c1093188c Mon Sep 17 00:00:00 2001 From: Weimin Yu Date: Tue, 2 Apr 2024 20:44:05 -0400 Subject: [PATCH] Check for missing BSA unblockable domains (#2394) * Check for missing BSA unblockable domains All unblockable domains created before the last refresh run should be reported as unblockable (registered). All reserved domains that are not registered should be reported as unblockable (reserved). Note that transient errors may be reported for newly added reserved domains since we do not maintain update time for when a reserved label is associated with a TLD. However, this scenario is extremely rare in operations. * Addressing review --- .../registry/bsa/BsaValidateAction.java | 87 ++++++++++++++++++- .../registry/bsa/persistence/Queries.java | 58 +++++++++++++ .../registry/bsa/BsaValidateActionTest.java | 77 +++++++++++++++- .../registry/bsa/persistence/QueriesTest.java | 58 ++++++++++--- 4 files changed, 263 insertions(+), 17 deletions(-) diff --git a/core/src/main/java/google/registry/bsa/BsaValidateAction.java b/core/src/main/java/google/registry/bsa/BsaValidateAction.java index 6df8da570..4bbbf8771 100644 --- a/core/src/main/java/google/registry/bsa/BsaValidateAction.java +++ b/core/src/main/java/google/registry/bsa/BsaValidateAction.java @@ -17,12 +17,20 @@ 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 com.google.common.collect.ImmutableMap.toImmutableMap; +import static com.google.common.collect.ImmutableSet.toImmutableSet; import static google.registry.bsa.BsaTransactions.bsaQuery; +import static google.registry.bsa.ReservedDomainsUtils.getAllReservedNames; import static google.registry.bsa.ReservedDomainsUtils.isReservedDomain; import static google.registry.bsa.persistence.Queries.batchReadBsaLabelText; +import static google.registry.bsa.persistence.Queries.queryMissedRegisteredUnblockables; +import static google.registry.bsa.persistence.Queries.queryUnblockableDomainByLabels; +import static google.registry.model.tld.Tld.isEnrolledWithBsa; +import static google.registry.model.tld.Tlds.getTldEntitiesOfType; 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 google.registry.util.BatchedStreams.toBatches; import static javax.servlet.http.HttpServletResponse.SC_OK; import com.google.common.base.Joiner; @@ -38,9 +46,12 @@ 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.bsa.persistence.Queries.DomainLifeSpan; import google.registry.config.RegistryConfig.Config; import google.registry.model.ForeignKeyUtils; import google.registry.model.domain.Domain; +import google.registry.model.tld.Tld; +import google.registry.model.tld.Tld.TldType; import google.registry.persistence.VKey; import google.registry.request.Action; import google.registry.request.Response; @@ -124,7 +135,8 @@ public class BsaValidateAction implements Runnable { ImmutableList.Builder errorsBuilder = new ImmutableList.Builder<>(); errorsBuilder.addAll(checkBsaLabels(downloadJobName.get())); - errorsBuilder.addAll(checkUnblockableDomains()); + errorsBuilder.addAll(checkWronglyReportedUnblockableDomains()); + errorsBuilder.addAll(checkMissingUnblockableDomains()); ImmutableList errors = errorsBuilder.build(); @@ -173,7 +185,7 @@ public class BsaValidateAction implements Runnable { return errors.build(); } - ImmutableList checkUnblockableDomains() { + ImmutableList checkWronglyReportedUnblockableDomains() { ImmutableList.Builder errors = new ImmutableList.Builder<>(); Optional lastRead = Optional.empty(); ImmutableList batch; @@ -263,6 +275,77 @@ public class BsaValidateAction implements Runnable { return labelsBuilder.build(); } + ImmutableList checkMissingUnblockableDomains() { + DateTime now = clock.nowUtc(); + ImmutableList.Builder errors = new ImmutableList.Builder<>(); + errors.addAll(checkForMissingReservedUnblockables(now)); + errors.addAll(checkForMissingRegisteredUnblockables(now)); + return errors.build(); + } + + ImmutableList checkForMissingRegisteredUnblockables(DateTime now) { + ImmutableList.Builder errors = new ImmutableList.Builder<>(); + ImmutableList bsaEnabledTlds = + getTldEntitiesOfType(TldType.REAL).stream() + .filter(tld -> isEnrolledWithBsa(tld, now)) + .collect(toImmutableList()); + DateTime stalenessThreshold = now.minus(maxStaleness); + bsaEnabledTlds.stream() + .map(Tld::getTldStr) + .map(tld -> bsaQuery(() -> queryMissedRegisteredUnblockables(tld, now))) + .flatMap(ImmutableList::stream) + .filter(domainLifeSpan -> domainLifeSpan.creationTime().isBefore(stalenessThreshold)) + .map(DomainLifeSpan::domainName) + .forEach( + domainName -> + errors.add( + String.format( + "Registered domain %s missing or not recorded as REGISTERED", domainName))); + return errors.build(); + } + + ImmutableList checkForMissingReservedUnblockables(DateTime now) { + ImmutableList.Builder errors = new ImmutableList.Builder<>(); + try (Stream> reservedNames = + toBatches(getAllReservedNames(now), transactionBatchSize)) { + reservedNames + .map(this::checkOneBatchReservedDomainsForMissingUnblockables) + .forEach(errors::addAll); + } + return errors.build(); + } + + ImmutableList checkOneBatchReservedDomainsForMissingUnblockables( + ImmutableList batch) { + ImmutableSet labels = + batch.stream() + .map(InternetDomainName::from) + .map(d -> d.parts().get(0)) + .collect(toImmutableSet()); + ImmutableMap persistedUnblockables = + bsaQuery( + () -> + queryUnblockableDomainByLabels(labels) + .collect(toImmutableMap(UnblockableDomain::domainName, x -> x))); + ImmutableList.Builder errors = new ImmutableList.Builder<>(); + ImmutableSet acceptableReasons = + ImmutableSet.of(Reason.REGISTERED, Reason.RESERVED); + for (var domainName : batch) { + if (!persistedUnblockables.containsKey(domainName)) { + errors.add(String.format("Missing unblockable domain: %s is reserved.", domainName)); + continue; + } + var unblockable = persistedUnblockables.get(domainName); + if (!acceptableReasons.contains(unblockable.reason())) { + errors.add( + String.format( + "Wrong unblockable reason: %s should be reserved or registered, found %s.", + domainName, unblockable.reason())); + } + } + return errors.build(); + } + static String parseBlockListLine(String line) { int firstComma = line.indexOf(','); checkArgument(firstComma > 0, "Invalid block list line: %s", line); 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 8ca109100..d04457318 100644 --- a/core/src/main/java/google/registry/bsa/persistence/Queries.java +++ b/core/src/main/java/google/registry/bsa/persistence/Queries.java @@ -19,12 +19,14 @@ 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; +import static org.joda.time.DateTimeZone.UTC; 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.sql.Timestamp; import java.util.List; import java.util.Optional; import java.util.stream.Collectors; @@ -66,6 +68,11 @@ public final class Queries { .collect(toImmutableList()); } + public static Stream queryUnblockableDomainByLabels( + ImmutableCollection labels) { + return queryBsaUnblockableDomainByLabels(labels).map(BsaUnblockableDomain::toUnblockableDomain); + } + static Stream queryBsaUnblockableDomainByLabels( ImmutableCollection labels) { return ((Stream) @@ -141,4 +148,55 @@ public final class Queries { .setParameter("tlds", tlds) .getResultList()); } + + /** + * Finds all currently registered domains that match BSA labels but are not recorded as + * unblockable. + * + * @return The missing unblockables and their creation and deletion time. + */ + public static ImmutableList queryMissedRegisteredUnblockables( + String tld, DateTime now) { + String sqlTemplate = + """ + SELECT l.domain_name, creation_time, deletion_time + FROM + (SELECT d.domain_name, d.creation_time, d.deletion_time + FROM + "Domain" d + JOIN + (SELECT concat(label, '.', :tld) AS domain_name from "BsaLabel") b + ON b.domain_name = d.domain_name + WHERE deletion_time > ':now') l + LEFT OUTER JOIN + (SELECT concat(label, '.', tld) as domain_name + FROM "BsaUnblockableDomain" + WHERE tld = :tld and reason = 'REGISTERED') r + ON l.domain_name = r.domain_name + WHERE r.domain_name is null; + """; + // Native query: Hibernate's setParameter wrongly converts DateTime to bytea + String sql = sqlTemplate.replace(":now", now.toString()); + + return ((Stream) + tm().getEntityManager() + .createNativeQuery(sql) + .setParameter("tld", tld) + .getResultStream()) + .map(Object[].class::cast) + .map( + row -> + new DomainLifeSpan( + (String) row[0], + toDateTime((Timestamp) row[1]), + toDateTime((Timestamp) row[2]))) + .collect(toImmutableList()); + } + + // For testing convenience: 'assertEquals' fails between `new DateTime(timestamp)` and below. + static DateTime toDateTime(Timestamp timestamp) { + return new DateTime(timestamp.getTime(), UTC); + } + + public record DomainLifeSpan(String domainName, DateTime creationTime, DateTime deletionTime) {} } diff --git a/core/src/test/java/google/registry/bsa/BsaValidateActionTest.java b/core/src/test/java/google/registry/bsa/BsaValidateActionTest.java index 5bea623c9..987030aaf 100644 --- a/core/src/test/java/google/registry/bsa/BsaValidateActionTest.java +++ b/core/src/test/java/google/registry/bsa/BsaValidateActionTest.java @@ -15,13 +15,18 @@ package google.registry.bsa; import static com.google.common.base.Throwables.getStackTraceAsString; +import static com.google.common.collect.ImmutableMap.toImmutableMap; import static com.google.common.truth.Truth.assertThat; +import static google.registry.bsa.ReservedDomainsTestingUtils.addReservedListsToTld; +import static google.registry.bsa.ReservedDomainsTestingUtils.createReservedList; 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 google.registry.testing.DatabaseHelper.persistResource; +import static google.registry.util.DateTimeUtils.START_OF_TIME; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.ArgumentMatchers.startsWith; @@ -43,13 +48,16 @@ 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.model.tld.label.ReservationType; import google.registry.persistence.transaction.JpaTestExtensions; import google.registry.persistence.transaction.JpaTestExtensions.JpaIntegrationWithCoverageExtension; import google.registry.request.Response; import google.registry.testing.FakeClock; import google.registry.tldconfig.idn.IdnTableEnum; import google.registry.util.EmailMessage; +import java.util.Optional; import java.util.concurrent.Callable; +import java.util.stream.Stream; import javax.mail.internet.InternetAddress; import org.joda.time.DateTime; import org.joda.time.Duration; @@ -269,7 +277,7 @@ public class BsaValidateActionTest { persistUnblockableDomain(UnblockableDomain.of("label", "app", Reason.REGISTERED)); when(idnChecker.getAllValidIdns(anyString())).thenReturn(ImmutableSet.of(IdnTableEnum.JA)); - assertThat(action.checkUnblockableDomains()).isEmpty(); + assertThat(action.checkWronglyReportedUnblockableDomains()).isEmpty(); } @Test @@ -280,10 +288,75 @@ public class BsaValidateActionTest { persistUnblockableDomain(UnblockableDomain.of("label", "app", Reason.RESERVED)); when(idnChecker.getAllValidIdns(anyString())).thenReturn(ImmutableSet.of(IdnTableEnum.JA)); - assertThat(action.checkUnblockableDomains()) + assertThat(action.checkWronglyReportedUnblockableDomains()) .containsExactly("label.app: should be REGISTERED, found RESERVED"); } + @Test + void checkForMissingReservedUnblockables_success() { + persistResource( + createTld("app").asBuilder().setBsaEnrollStartTime(Optional.of(START_OF_TIME)).build()); + persistResource( + createTld("dev").asBuilder().setBsaEnrollStartTime(Optional.of(START_OF_TIME)).build()); + persistBsaLabel("registered-reserved"); + persistBsaLabel("reserved-only"); + persistBsaLabel("reserved-missing"); + persistBsaLabel("invalid-in-app"); + + persistUnblockableDomain(UnblockableDomain.of("registered-reserved", "app", Reason.REGISTERED)); + persistUnblockableDomain(UnblockableDomain.of("reserved-only", "app", Reason.RESERVED)); + persistUnblockableDomain(UnblockableDomain.of("invalid-in-app", "dev", Reason.RESERVED)); + + createReservedList( + "rl", + Stream.of("registered-reserved", "reserved-only", "reserved-missing") + .collect(toImmutableMap(x -> x, x -> ReservationType.RESERVED_FOR_SPECIFIC_USE))); + addReservedListsToTld("app", ImmutableList.of("rl")); + + ImmutableList errors = action.checkForMissingReservedUnblockables(fakeClock.nowUtc()); + assertThat(errors) + .containsExactly("Missing unblockable domain: reserved-missing.app is reserved."); + } + + @Test + void checkForMissingReservedUnblockablesInOneTld_success() { + persistResource( + createTld("app").asBuilder().setBsaEnrollStartTime(Optional.of(START_OF_TIME)).build()); + persistResource( + createTld("dev").asBuilder().setBsaEnrollStartTime(Optional.of(START_OF_TIME)).build()); + persistBsaLabel("reserved-missing-in-app"); + persistUnblockableDomain( + UnblockableDomain.of("reserved-missing-in-app", "dev", Reason.REGISTERED)); + + createReservedList( + "rl", + Stream.of("reserved-missing-in-app") + .collect(toImmutableMap(x -> x, x -> ReservationType.RESERVED_FOR_SPECIFIC_USE))); + addReservedListsToTld("app", ImmutableList.of("rl")); + addReservedListsToTld("dev", ImmutableList.of("rl")); + + ImmutableList errors = action.checkForMissingReservedUnblockables(fakeClock.nowUtc()); + assertThat(errors) + .containsExactly("Missing unblockable domain: reserved-missing-in-app.app is reserved."); + } + + @Test + void checkForMissingRegisteredUnblockables_success() { + persistResource( + createTld("app").asBuilder().setBsaEnrollStartTime(Optional.of(START_OF_TIME)).build()); + persistBsaLabel("registered"); + persistBsaLabel("registered-missing"); + persistUnblockableDomain(UnblockableDomain.of("registered", "app", Reason.REGISTERED)); + persistUnblockableDomain(UnblockableDomain.of("registered-missing", "app", Reason.RESERVED)); + persistActiveDomain("registered.app"); + persistActiveDomain("registered-missing.app"); + + ImmutableList errors = action.checkForMissingRegisteredUnblockables(fakeClock.nowUtc()); + assertThat(errors) + .containsExactly( + "Registered domain registered-missing.app missing or not recorded as REGISTERED"); + } + @Test void notificationSent_cannotAcquireLock() { when(bsaLock.executeWithLock(any())).thenReturn(false); diff --git a/core/src/test/java/google/registry/bsa/persistence/QueriesTest.java b/core/src/test/java/google/registry/bsa/persistence/QueriesTest.java index bbd7f22ba..979519bcd 100644 --- a/core/src/test/java/google/registry/bsa/persistence/QueriesTest.java +++ b/core/src/test/java/google/registry/bsa/persistence/QueriesTest.java @@ -21,18 +21,22 @@ import static google.registry.bsa.persistence.Queries.batchReadBsaLabelText; import static google.registry.bsa.persistence.Queries.deleteBsaLabelByLabels; import static google.registry.bsa.persistence.Queries.queryBsaLabelByLabels; import static google.registry.bsa.persistence.Queries.queryBsaUnblockableDomainByLabels; +import static google.registry.bsa.persistence.Queries.queryMissedRegisteredUnblockables; import static google.registry.bsa.persistence.Queries.queryNewlyCreatedDomains; import static google.registry.bsa.persistence.Queries.queryUnblockablesByNames; import static google.registry.persistence.transaction.TransactionManagerFactory.tm; import static google.registry.testing.DatabaseHelper.createTlds; import static google.registry.testing.DatabaseHelper.newDomain; +import static google.registry.testing.DatabaseHelper.persistActiveDomain; import static google.registry.testing.DatabaseHelper.persistDomainAsDeleted; import static google.registry.testing.DatabaseHelper.persistNewRegistrar; -import static google.registry.testing.DatabaseHelper.persistResource; +import static google.registry.util.DateTimeUtils.END_OF_TIME; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; +import google.registry.bsa.api.UnblockableDomain; import google.registry.bsa.persistence.BsaUnblockableDomain.Reason; +import google.registry.bsa.persistence.Queries.DomainLifeSpan; import google.registry.persistence.transaction.JpaTestExtensions; import google.registry.persistence.transaction.JpaTestExtensions.JpaIntegrationWithCoverageExtension; import google.registry.testing.FakeClock; @@ -200,16 +204,14 @@ class QueriesTest { createTlds("tld"); persistNewRegistrar("TheRegistrar"); // time 0: - persistResource( - newDomain("d1.tld").asBuilder().setCreationTimeForTest(fakeClock.nowUtc()).build()); + persistActiveDomain("d1.tld", fakeClock.nowUtc()); // time 0, deletion time 1 persistDomainAsDeleted( newDomain("will-delete.tld").asBuilder().setCreationTimeForTest(fakeClock.nowUtc()).build(), fakeClock.nowUtc().plusMillis(1)); fakeClock.advanceOneMilli(); // time 1 - persistResource( - newDomain("d2.tld").asBuilder().setCreationTimeForTest(fakeClock.nowUtc()).build()); + persistActiveDomain("d2.tld", fakeClock.nowUtc()); fakeClock.advanceOneMilli(); // Now is time 2 assertThat( @@ -226,16 +228,14 @@ class QueriesTest { createTlds("tld"); persistNewRegistrar("TheRegistrar"); // time 0: - persistResource( - newDomain("d1.tld").asBuilder().setCreationTimeForTest(fakeClock.nowUtc()).build()); + persistActiveDomain("d1.tld", fakeClock.nowUtc()); // time 0, deletion time 1 persistDomainAsDeleted( newDomain("will-delete.tld").asBuilder().setCreationTimeForTest(fakeClock.nowUtc()).build(), fakeClock.nowUtc().plusMillis(1)); fakeClock.advanceOneMilli(); // time 1 - persistResource( - newDomain("d2.tld").asBuilder().setCreationTimeForTest(fakeClock.nowUtc()).build()); + persistActiveDomain("d2.tld", fakeClock.nowUtc()); fakeClock.advanceOneMilli(); // Now is time 2, ask for domains created since time 1 assertThat( @@ -251,10 +251,8 @@ class QueriesTest { DateTime testStartTime = fakeClock.nowUtc(); createTlds("tld", "tld2"); persistNewRegistrar("TheRegistrar"); - persistResource( - newDomain("d1.tld").asBuilder().setCreationTimeForTest(fakeClock.nowUtc()).build()); - persistResource( - newDomain("d2.tld2").asBuilder().setCreationTimeForTest(fakeClock.nowUtc()).build()); + persistActiveDomain("d1.tld", fakeClock.nowUtc()); + persistActiveDomain("d2.tld2", fakeClock.nowUtc()); fakeClock.advanceOneMilli(); assertThat( bsaQuery( @@ -263,4 +261,38 @@ class QueriesTest { ImmutableList.of("tld"), testStartTime, fakeClock.nowUtc()))) .containsExactly("d1.tld"); } + + @Test + void queryMissedRegisteredUnblockables_success() { + createTlds("tld", "tld2"); + persistNewRegistrar("TheRegistrar"); + DateTime time1 = fakeClock.nowUtc(); + persistActiveDomain("unblocked1.tld", fakeClock.nowUtc()); + persistActiveDomain("unblocked2.tld2", fakeClock.nowUtc()); + persistActiveDomain("label1.tld", fakeClock.nowUtc()); + persistActiveDomain("label2.tld2", fakeClock.nowUtc()); + fakeClock.advanceOneMilli(); + DateTime time2 = fakeClock.nowUtc(); + persistDomainAsDeleted( + newDomain("label3.tld").asBuilder().setCreationTimeForTest(fakeClock.nowUtc()).build(), + fakeClock.nowUtc().plusMillis(1)); + // Deleted in the future + persistDomainAsDeleted( + newDomain("label3.tld2").asBuilder().setCreationTimeForTest(fakeClock.nowUtc()).build(), + fakeClock.nowUtc().plusHours(1)); + fakeClock.advanceOneMilli(); + assertThat(bsaQuery(() -> queryMissedRegisteredUnblockables("tld", fakeClock.nowUtc()))) + .containsExactly(new DomainLifeSpan("label1.tld", time1, END_OF_TIME)); + assertThat(bsaQuery(() -> queryMissedRegisteredUnblockables("tld2", fakeClock.nowUtc()))) + .containsExactly( + new DomainLifeSpan("label2.tld2", time1, END_OF_TIME), + new DomainLifeSpan("label3.tld2", time2, time2.plusHours(1))); + + BsaTestingUtils.persistUnblockableDomain( + UnblockableDomain.of("label2", "tld2", UnblockableDomain.Reason.REGISTERED)); + BsaTestingUtils.persistUnblockableDomain( + UnblockableDomain.of("label3", "tld2", UnblockableDomain.Reason.RESERVED)); + assertThat(bsaQuery(() -> queryMissedRegisteredUnblockables("tld2", fakeClock.nowUtc()))) + .containsExactly(new DomainLifeSpan("label3.tld2", time2, time2.plusHours(1))); + } }