From 2ffba93a73190faa9753bcbd77524b653d47e421 Mon Sep 17 00:00:00 2001 From: Pavlo Tkach <3469726+ptkach@users.noreply.github.com> Date: Tue, 16 Aug 2022 12:59:08 -0400 Subject: [PATCH] Add email notification when DNS update fails (#1734) --- .../registry/config/RegistryConfig.java | 24 ++++++ .../config/RegistryConfigSettings.java | 9 ++ .../registry/config/files/default-config.yaml | 22 +++++ .../registry/dns/PublishDnsUpdatesAction.java | 82 +++++++++++++++++-- .../dns/PublishDnsUpdatesActionTest.java | 53 +++++++++++- 5 files changed, 183 insertions(+), 7 deletions(-) diff --git a/core/src/main/java/google/registry/config/RegistryConfig.java b/core/src/main/java/google/registry/config/RegistryConfig.java index 3280d42ed..78cbf43cb 100644 --- a/core/src/main/java/google/registry/config/RegistryConfig.java +++ b/core/src/main/java/google/registry/config/RegistryConfig.java @@ -1300,6 +1300,30 @@ public final class RegistryConfig { return config.sslCertificateValidation.expirationWarningEmailSubjectText; } + @Provides + @Config("dnsUpdateFailEmailSubjectText") + public static String provideDnsUpdateFailEmailSubjectText(RegistryConfigSettings config) { + return config.dnsUpdate.dnsUpdateFailEmailSubjectText; + } + + @Provides + @Config("dnsUpdateFailEmailBodyText") + public static String provideDnsUpdateFailEmailBodyText(RegistryConfigSettings config) { + return config.dnsUpdate.dnsUpdateFailEmailBodyText; + } + + @Provides + @Config("dnsUpdateFailRegistryName") + public static String provideDnsUpdateFailRegistryName(RegistryConfigSettings config) { + return config.dnsUpdate.dnsUpdateFailRegistryName; + } + + @Provides + @Config("registrySupportEmail") + public static String provideRegistrySupportEmail(RegistryConfigSettings config) { + return config.dnsUpdate.registrySupportEmail; + } + @Provides @Config("allowedEcdsaCurves") public static ImmutableSet provideAllowedEcdsaCurves(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 be48858e2..bcbd84bb9 100644 --- a/core/src/main/java/google/registry/config/RegistryConfigSettings.java +++ b/core/src/main/java/google/registry/config/RegistryConfigSettings.java @@ -42,6 +42,7 @@ public class RegistryConfigSettings { public RegistryTool registryTool; public SslCertificateValidation sslCertificateValidation; public ContactHistory contactHistory; + public DnsUpdate dnsUpdate; /** Configuration options that apply to the entire App Engine project. */ public static class AppEngine { @@ -246,4 +247,12 @@ public class RegistryConfigSettings { public int minMonthsBeforeWipeOut; public int wipeOutQueryBatchSize; } + + /** Configuration for dns update. */ + public static class DnsUpdate { + public String dnsUpdateFailEmailSubjectText; + public String dnsUpdateFailEmailBodyText; + public String dnsUpdateFailRegistryName; + public String registrySupportEmail; + } } 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 36118a77c..d8f624cc2 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 @@ -477,6 +477,28 @@ contactHistory: # The batch size for querying ContactHistory table in the database. wipeOutQueryBatchSize: 500 +# Configuration options relevant to the DNS update functionality. +dnsUpdate: + dnsUpdateFailRegistryName: Example name + registrySupportEmail: email@example.com + # Email subject text template to notify partners after repeatedly failing DNS update + dnsUpdateFailEmailSubjectText: "[ACTION REQUIRED]: Incomplete DNS Update" + # Email body text template for failing DNS update that accepts 5 parameters: + # registrar name, domain or host address, 'domain' or 'host' as a string that failed, + # registry support email (see dnsUpdateFailRegistrySupportEmail) and registry display name + dnsUpdateFailEmailBodyText: > + Dear %1$s, + + We are contacting you regarding the changes you recently made to one of your %2$ss. + The DNS update for the %3$s %2$s failed to process. Please review your %2$s's DNS records + and ensure that it is valid before trying another update. + + If you have any questions or require additional support, please contact us + at %4$s. + + Regards, + %5$s + # Configuration options for checking SSL certificates. sslCertificateValidation: # A map specifying the maximum amount of days the certificate can be valid. diff --git a/core/src/main/java/google/registry/dns/PublishDnsUpdatesAction.java b/core/src/main/java/google/registry/dns/PublishDnsUpdatesAction.java index 2721494bf..4bef5b88f 100644 --- a/core/src/main/java/google/registry/dns/PublishDnsUpdatesAction.java +++ b/core/src/main/java/google/registry/dns/PublishDnsUpdatesAction.java @@ -14,6 +14,7 @@ package google.registry.dns; +import static com.google.common.collect.ImmutableList.toImmutableList; import static google.registry.dns.DnsConstants.DNS_PUBLISH_PUSH_QUEUE_NAME; import static google.registry.dns.DnsModule.PARAM_DNS_WRITER; import static google.registry.dns.DnsModule.PARAM_DOMAINS; @@ -22,6 +23,7 @@ import static google.registry.dns.DnsModule.PARAM_LOCK_INDEX; import static google.registry.dns.DnsModule.PARAM_NUM_PUBLISH_LOCKS; import static google.registry.dns.DnsModule.PARAM_PUBLISH_TASK_ENQUEUED; import static google.registry.dns.DnsModule.PARAM_REFRESH_REQUEST_CREATED; +import static google.registry.model.EppResourceUtils.loadByForeignKey; import static google.registry.request.Action.Method.POST; import static google.registry.request.RequestParameters.PARAM_TLD; import static google.registry.util.CollectionUtils.nullToEmpty; @@ -37,6 +39,10 @@ import google.registry.dns.DnsMetrics.ActionStatus; import google.registry.dns.DnsMetrics.CommitStatus; import google.registry.dns.DnsMetrics.PublishStatus; import google.registry.dns.writer.DnsWriter; +import google.registry.model.domain.Domain; +import google.registry.model.host.Host; +import google.registry.model.registrar.Registrar; +import google.registry.model.registrar.RegistrarPoc; import google.registry.model.tld.Registry; import google.registry.request.Action; import google.registry.request.Action.Service; @@ -46,6 +52,7 @@ import google.registry.request.Parameter; import google.registry.request.Response; import google.registry.request.auth.Auth; import google.registry.request.lock.LockHandler; +import google.registry.ui.server.SendEmailUtils; import google.registry.util.Clock; import google.registry.util.CloudTasksUtils; import google.registry.util.DomainNameUtils; @@ -102,6 +109,11 @@ public final class PublishDnsUpdatesAction implements Runnable, Callable { private final Clock clock; private final CloudTasksUtils cloudTasksUtils; private final Response response; + private final SendEmailUtils sendEmailUtils; + private final String dnsUpdateFailEmailSubjectText; + private final String dnsUpdateFailEmailBodyText; + private final String dnsUpdateFailRegistryName; + private final String registrySupportEmail; @Inject public PublishDnsUpdatesAction( @@ -114,6 +126,10 @@ public final class PublishDnsUpdatesAction implements Runnable, Callable { @Parameter(PARAM_HOSTS) Set hosts, @Parameter(PARAM_TLD) String tld, @Config("publishDnsUpdatesLockDuration") Duration timeout, + @Config("dnsUpdateFailEmailSubjectText") String dnsUpdateFailEmailSubjectText, + @Config("dnsUpdateFailEmailBodyText") String dnsUpdateFailEmailBodyText, + @Config("dnsUpdateFailRegistryName") String dnsUpdateFailRegistryName, + @Config("registrySupportEmail") String registrySupportEmail, @Header(APP_ENGINE_RETRY_HEADER) Optional appEngineRetryCount, @Header(CLOUD_TASKS_RETRY_HEADER) Optional cloudTasksRetryCount, DnsQueue dnsQueue, @@ -122,11 +138,13 @@ public final class PublishDnsUpdatesAction implements Runnable, Callable { LockHandler lockHandler, Clock clock, CloudTasksUtils cloudTasksUtils, + SendEmailUtils sendEmailUtils, Response response) { this.dnsQueue = dnsQueue; this.dnsWriterProxy = dnsWriterProxy; this.dnsMetrics = dnsMetrics; this.timeout = timeout; + this.sendEmailUtils = sendEmailUtils; this.retryCount = cloudTasksRetryCount.orElse( appEngineRetryCount.orElseThrow( @@ -143,6 +161,10 @@ public final class PublishDnsUpdatesAction implements Runnable, Callable { this.clock = clock; this.cloudTasksUtils = cloudTasksUtils; this.response = response; + this.dnsUpdateFailEmailBodyText = dnsUpdateFailEmailBodyText; + this.dnsUpdateFailEmailSubjectText = dnsUpdateFailEmailSubjectText; + this.dnsUpdateFailRegistryName = dnsUpdateFailRegistryName; + this.registrySupportEmail = registrySupportEmail; } private void recordActionResult(ActionStatus status) { @@ -209,9 +231,35 @@ public final class PublishDnsUpdatesAction implements Runnable, Callable { } else if (retryCount < RETRIES_BEFORE_PERMANENT_FAILURE) { // If the batch only contains 1 name, allow it more retries throw e; + } else { + // By the time we get here there's either single domain or a single host + domains.stream() + .findFirst() + .ifPresent( + dn -> { + Optional domain = loadByForeignKey(Domain.class, dn, clock.nowUtc()); + if (domain.isPresent()) { + notifyWithEmailAboutDnsUpdateFailure( + domain.get().getCurrentSponsorRegistrarId(), dn, false); + } else { + logger.atSevere().log(String.format("Domain entity for %s not found", dn)); + } + }); + + hosts.stream() + .findFirst() + .ifPresent( + hn -> { + Optional host = loadByForeignKey(Host.class, hn, clock.nowUtc()); + if (host.isPresent()) { + notifyWithEmailAboutDnsUpdateFailure( + host.get().getPersistedCurrentSponsorRegistrarId(), hn, true); + } else { + logger.atSevere().log(String.format("Host entity for %s not found", hn)); + } + }); } - // If we get here, we should terminate this task as it is likely a perpetually failing task. - // TODO(b/237302821): Send an email notifying partner the dns update failed + recordActionResult(ActionStatus.MAX_RETRIES_EXCEEDED); response.setStatus(SC_ACCEPTED); logger.atSevere().withCause(e).log("Terminated task after too many retries"); @@ -219,6 +267,30 @@ public final class PublishDnsUpdatesAction implements Runnable, Callable { return null; } + /** Sends an email to partners regarding a failure during DNS update */ + private void notifyWithEmailAboutDnsUpdateFailure( + String registrarId, String hostOrDomainName, Boolean isHost) { + Optional registrar = Registrar.loadByRegistrarIdCached(registrarId); + + if (registrar.isPresent()) { + sendEmailUtils.sendEmail( + dnsUpdateFailEmailSubjectText, + String.format( + dnsUpdateFailEmailBodyText, + registrar.get().getRegistrarName(), + hostOrDomainName, + isHost ? "host" : "domain", + registrySupportEmail, + dnsUpdateFailRegistryName), + registrar.get().getContacts().stream() + .filter(c -> c.getTypes().contains(RegistrarPoc.Type.ADMIN)) + .map(RegistrarPoc::getEmailAddress) + .collect(toImmutableList())); + } else { + logger.atSevere().log(String.format("Could not find registrar %s", registrarId)); + } + } + /** Splits the domains and hosts in a batch into smaller batches and adds them to the queue. */ private void splitBatch() { ImmutableList domainList = ImmutableList.copyOf(domains); @@ -294,8 +366,7 @@ public final class PublishDnsUpdatesAction implements Runnable, Callable { int domainsPublished = 0; int domainsRejected = 0; for (String domain : nullToEmpty(domains)) { - if (!DomainNameUtils.isUnder( - InternetDomainName.from(domain), InternetDomainName.from(tld))) { + if (!DomainNameUtils.isUnder(InternetDomainName.from(domain), InternetDomainName.from(tld))) { logger.atSevere().log("%s: skipping domain %s not under TLD.", tld, domain); domainsRejected += 1; } else { @@ -310,8 +381,7 @@ public final class PublishDnsUpdatesAction implements Runnable, Callable { int hostsPublished = 0; int hostsRejected = 0; for (String host : nullToEmpty(hosts)) { - if (!DomainNameUtils.isUnder( - InternetDomainName.from(host), InternetDomainName.from(tld))) { + if (!DomainNameUtils.isUnder(InternetDomainName.from(host), InternetDomainName.from(tld))) { logger.atSevere().log("%s: skipping host %s not under TLD.", tld, host); hostsRejected += 1; } else { diff --git a/core/src/test/java/google/registry/dns/PublishDnsUpdatesActionTest.java b/core/src/test/java/google/registry/dns/PublishDnsUpdatesActionTest.java index d5ae645fd..dce6e0c61 100644 --- a/core/src/test/java/google/registry/dns/PublishDnsUpdatesActionTest.java +++ b/core/src/test/java/google/registry/dns/PublishDnsUpdatesActionTest.java @@ -38,6 +38,7 @@ import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verifyNoMoreInteractions; import static org.mockito.Mockito.when; +import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import google.registry.dns.DnsMetrics.ActionStatus; @@ -54,13 +55,18 @@ import google.registry.testing.CloudTasksHelper.TaskMatcher; import google.registry.testing.FakeClock; import google.registry.testing.FakeLockHandler; import google.registry.testing.FakeResponse; +import google.registry.ui.server.SendEmailUtils; +import google.registry.util.EmailMessage; +import google.registry.util.SendEmailService; import java.util.Optional; import java.util.Set; +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.RegisterExtension; +import org.mockito.ArgumentCaptor; /** Unit tests for {@link PublishDnsUpdatesAction}. */ public class PublishDnsUpdatesActionTest { @@ -77,9 +83,18 @@ public class PublishDnsUpdatesActionTest { private final DnsQueue dnsQueue = mock(DnsQueue.class); private final CloudTasksHelper cloudTasksHelper = new CloudTasksHelper(); private PublishDnsUpdatesAction action; + private final SendEmailService emailService = mock(SendEmailService.class); + private SendEmailUtils sendEmailUtils; @BeforeEach - void beforeEach() { + void beforeEach() throws Exception { + sendEmailUtils = + new SendEmailUtils( + new InternetAddress("outgoing@registry.example"), + "UnitTest Registry", + ImmutableList.of("notification@test.example", "notification2@test.example"), + emailService); + createTld("xn--q9jyb4c"); persistResource( Registry.get("xn--q9jyb4c") @@ -138,6 +153,10 @@ public class PublishDnsUpdatesActionTest { hosts, tld, Duration.standardSeconds(10), + "Subj", + "Body %1$s %2$s %3$s %4$s %5$s", + "registry@test.com", + "awesomeRegistry", Optional.ofNullable(retryCount), Optional.empty(), dnsQueue, @@ -146,6 +165,7 @@ public class PublishDnsUpdatesActionTest { lockHandler, clock, cloudTasksHelper.getTestCloudTasksUtils(), + sendEmailUtils, response); } @@ -381,6 +401,37 @@ public class PublishDnsUpdatesActionTest { assertThat(response.getStatus()).isEqualTo(SC_ACCEPTED); } + @Test + void testDomainDnsUpdateRetriesExhausted_emailIsSent() { + action = + createAction("xn--q9jyb4c", ImmutableSet.of("example.xn--q9jyb4c"), ImmutableSet.of(), 10); + doThrow(new RuntimeException()).when(dnsWriter).commit(); + action.run(); + ArgumentCaptor contentCaptor = ArgumentCaptor.forClass(EmailMessage.class); + verify(emailService).sendEmail(contentCaptor.capture()); + EmailMessage emailMessage = contentCaptor.getValue(); + assertThat(emailMessage.subject()).isEqualTo("Subj"); + assertThat(emailMessage.body()) + .isEqualTo( + "Body The Registrar example.xn--q9jyb4c domain awesomeRegistry registry@test.com"); + } + + @Test + void testHostDnsUpdateRetriesExhausted_emailIsSent() { + action = + createAction( + "xn--q9jyb4c", ImmutableSet.of(), ImmutableSet.of("ns1.example.xn--q9jyb4c"), 10); + doThrow(new RuntimeException()).when(dnsWriter).commit(); + action.run(); + ArgumentCaptor contentCaptor = ArgumentCaptor.forClass(EmailMessage.class); + verify(emailService).sendEmail(contentCaptor.capture()); + EmailMessage emailMessage = contentCaptor.getValue(); + assertThat(emailMessage.subject()).isEqualTo("Subj"); + assertThat(emailMessage.body()) + .isEqualTo( + "Body The Registrar ns1.example.xn--q9jyb4c host awesomeRegistry registry@test.com"); + } + @Test void testTaskMissingRetryHeaders_throwsException() { IllegalStateException thrown =