mirror of
https://github.com/google/nomulus.git
synced 2025-08-28 20:13:46 +02:00
Add success/failure notifications for the RelockDomainAction (#733)
* Add success/failure notifications for the RelockDomainAction If a relock fails for some reason, we should noisily notify both our alerting email and also the registry lock contacts for the registrar in question. The consequences of a silent failure could be large so it's something we want to avoid if at all possible. In addition, we only retry tasks up to two times (one in 5min, one in 10min). This model of retries / notifications, as well as the language contained in the emails, have been LGTMed by Bruno and Kirsten * Change the wording on the success email * Change the times in which we send emails For transient failures: - Retry every ten minutes for six hours - Send an email after a half hour (three failures) saying that we'll retry - Send a success email if we succeed any time after that For non-transient failures: Send an email with the error message and don't retry * Add a test for the max-failure-email * Responses to CR - retry indefinitely - send an email to just the alert address if we can't find the lock - refactor the task enqueuer a bit * non-transient -> non-retryable * Use a lenient stubber for the AESU * Add a DS transaction around the re-lock
This commit is contained in:
gbrodman
GitHub
parent
0423c7ae22
commit
1bba68dd96
7 changed files with 435 additions and 94 deletions
|
|
@ -31,7 +31,7 @@ import static google.registry.testing.TestLogHandlerUtils.assertLogMessage;
|
|||
import static org.joda.time.Duration.standardDays;
|
||||
import static org.joda.time.Duration.standardHours;
|
||||
import static org.joda.time.Duration.standardSeconds;
|
||||
import static org.junit.jupiter.api.Assertions.assertThrows;
|
||||
import static org.junit.Assert.assertThrows;
|
||||
import static org.mockito.Mockito.when;
|
||||
|
||||
import com.google.common.collect.ImmutableSortedSet;
|
||||
|
|
@ -159,7 +159,7 @@ public class AsyncTaskEnqueuerTest {
|
|||
.setRegistrarPocId("someone@example.com")
|
||||
.setVerificationCode("hi")
|
||||
.build());
|
||||
asyncTaskEnqueuer.enqueueDomainRelock(lock);
|
||||
asyncTaskEnqueuer.enqueueDomainRelock(lock.getRelockDuration().get(), lock.getRevisionId(), 0);
|
||||
assertTasksEnqueued(
|
||||
QUEUE_ASYNC_ACTIONS,
|
||||
new TaskMatcher()
|
||||
|
|
@ -169,6 +169,7 @@ public class AsyncTaskEnqueuerTest {
|
|||
.param(
|
||||
RelockDomainAction.OLD_UNLOCK_REVISION_ID_PARAM,
|
||||
String.valueOf(lock.getRevisionId()))
|
||||
.param(RelockDomainAction.PREVIOUS_ATTEMPTS_PARAM, "0")
|
||||
.etaDelta(
|
||||
standardHours(6).minus(standardSeconds(30)),
|
||||
standardHours(6).plus(standardSeconds(30))));
|
||||
|
|
@ -188,9 +189,9 @@ public class AsyncTaskEnqueuerTest {
|
|||
.setVerificationCode("hi")
|
||||
.build());
|
||||
assertThat(
|
||||
assertThrows(
|
||||
IllegalArgumentException.class,
|
||||
() -> asyncTaskEnqueuer.enqueueDomainRelock(lockWithoutDuration)))
|
||||
assertThrows(
|
||||
IllegalArgumentException.class,
|
||||
() -> asyncTaskEnqueuer.enqueueDomainRelock(lockWithoutDuration)))
|
||||
.hasMessageThat()
|
||||
.isEqualTo(
|
||||
String.format(
|
||||
|
|
|
|||
|
|
@ -15,10 +15,12 @@
|
|||
package google.registry.batch;
|
||||
|
||||
import static com.google.common.truth.Truth.assertThat;
|
||||
import static google.registry.batch.AsyncTaskEnqueuer.QUEUE_ASYNC_ACTIONS;
|
||||
import static google.registry.model.eppcommon.StatusValue.PENDING_DELETE;
|
||||
import static google.registry.model.eppcommon.StatusValue.PENDING_TRANSFER;
|
||||
import static google.registry.model.ofy.ObjectifyService.ofy;
|
||||
import static google.registry.testing.DatastoreHelper.createTlds;
|
||||
import static google.registry.testing.DatastoreHelper.deleteResource;
|
||||
import static google.registry.testing.DatastoreHelper.newDomainBase;
|
||||
import static google.registry.testing.DatastoreHelper.persistActiveHost;
|
||||
import static google.registry.testing.DatastoreHelper.persistDomainAsDeleted;
|
||||
|
|
@ -26,9 +28,16 @@ import static google.registry.testing.DatastoreHelper.persistResource;
|
|||
import static google.registry.testing.SqlHelper.getMostRecentVerifiedRegistryLockByRepoId;
|
||||
import static google.registry.testing.SqlHelper.getRegistryLockByVerificationCode;
|
||||
import static google.registry.testing.SqlHelper.saveRegistryLock;
|
||||
import static google.registry.testing.TaskQueueHelper.assertNoTasksEnqueued;
|
||||
import static google.registry.testing.TaskQueueHelper.assertTasksEnqueued;
|
||||
import static google.registry.tools.LockOrUnlockDomainCommand.REGISTRY_LOCK_STATUSES;
|
||||
import static javax.servlet.http.HttpServletResponse.SC_NO_CONTENT;
|
||||
import static javax.servlet.http.HttpServletResponse.SC_OK;
|
||||
import static org.joda.time.Duration.standardSeconds;
|
||||
import static org.mockito.Mockito.lenient;
|
||||
import static org.mockito.Mockito.mock;
|
||||
import static org.mockito.Mockito.verify;
|
||||
import static org.mockito.Mockito.verifyNoMoreInteractions;
|
||||
|
||||
import com.google.common.collect.ImmutableSet;
|
||||
import google.registry.model.domain.DomainBase;
|
||||
|
|
@ -38,17 +47,27 @@ import google.registry.testing.AppEngineExtension;
|
|||
import google.registry.testing.DeterministicStringGenerator;
|
||||
import google.registry.testing.FakeClock;
|
||||
import google.registry.testing.FakeResponse;
|
||||
import google.registry.testing.TaskQueueHelper.TaskMatcher;
|
||||
import google.registry.testing.UserInfo;
|
||||
import google.registry.tools.DomainLockUtils;
|
||||
import google.registry.util.AppEngineServiceUtils;
|
||||
import google.registry.util.EmailMessage;
|
||||
import google.registry.util.SendEmailService;
|
||||
import google.registry.util.StringGenerator.Alphabets;
|
||||
import java.util.Optional;
|
||||
import javax.mail.internet.InternetAddress;
|
||||
import org.joda.time.DateTime;
|
||||
import org.joda.time.Duration;
|
||||
import org.junit.jupiter.api.AfterEach;
|
||||
import org.junit.jupiter.api.BeforeEach;
|
||||
import org.junit.jupiter.api.Test;
|
||||
import org.junit.jupiter.api.extension.ExtendWith;
|
||||
import org.junit.jupiter.api.extension.RegisterExtension;
|
||||
import org.mockito.Mock;
|
||||
import org.mockito.junit.jupiter.MockitoExtension;
|
||||
|
||||
/** Unit tests for {@link RelockDomainAction}. */
|
||||
@ExtendWith(MockitoExtension.class)
|
||||
public class RelockDomainActionTest {
|
||||
|
||||
private static final String DOMAIN_NAME = "example.tld";
|
||||
|
|
@ -56,7 +75,7 @@ public class RelockDomainActionTest {
|
|||
private static final String POC_ID = "marla.singer@example.com";
|
||||
|
||||
private final FakeResponse response = new FakeResponse();
|
||||
private final FakeClock clock = new FakeClock();
|
||||
private final FakeClock clock = new FakeClock(DateTime.parse("2015-05-18T12:34:56Z"));
|
||||
private final DomainLockUtils domainLockUtils =
|
||||
new DomainLockUtils(
|
||||
new DeterministicStringGenerator(Alphabets.BASE_58),
|
||||
|
|
@ -68,15 +87,18 @@ public class RelockDomainActionTest {
|
|||
public final AppEngineExtension appEngineRule =
|
||||
AppEngineExtension.builder()
|
||||
.withDatastoreAndCloudSql()
|
||||
.withTaskQueue()
|
||||
.withUserService(UserInfo.create(POC_ID, "12345"))
|
||||
.build();
|
||||
|
||||
private DomainBase domain;
|
||||
private RegistryLock oldLock;
|
||||
@Mock private SendEmailService sendEmailService;
|
||||
private AsyncTaskEnqueuer asyncTaskEnqueuer;
|
||||
private RelockDomainAction action;
|
||||
|
||||
@BeforeEach
|
||||
void beforeEach() {
|
||||
void beforeEach() throws Exception {
|
||||
createTlds("tld", "net");
|
||||
HostResource host = persistActiveHost("ns1.example.net");
|
||||
domain = persistResource(newDomainBase(DOMAIN_NAME, host));
|
||||
|
|
@ -88,9 +110,22 @@ public class RelockDomainActionTest {
|
|||
domainLockUtils.administrativelyApplyUnlock(
|
||||
DOMAIN_NAME, CLIENT_ID, false, Optional.empty());
|
||||
assertThat(reloadDomain(domain).getStatusValues()).containsNoneIn(REGISTRY_LOCK_STATUSES);
|
||||
|
||||
AppEngineServiceUtils appEngineServiceUtils = mock(AppEngineServiceUtils.class);
|
||||
lenient()
|
||||
.when(appEngineServiceUtils.getServiceHostname("backend"))
|
||||
.thenReturn("backend.hostname.fake");
|
||||
|
||||
asyncTaskEnqueuer =
|
||||
AsyncTaskEnqueuerTest.createForTesting(appEngineServiceUtils, clock, Duration.ZERO);
|
||||
action = createAction(oldLock.getRevisionId());
|
||||
}
|
||||
|
||||
@AfterEach
|
||||
void afterEach() {
|
||||
verifyNoMoreInteractions(sendEmailService);
|
||||
}
|
||||
|
||||
@Test
|
||||
void testLock() {
|
||||
action.run();
|
||||
|
|
@ -104,29 +139,36 @@ public class RelockDomainActionTest {
|
|||
}
|
||||
|
||||
@Test
|
||||
void testFailure_unknownCode() {
|
||||
void testFailure_unknownCode() throws Exception {
|
||||
action = createAction(12128675309L);
|
||||
action.run();
|
||||
assertThat(response.getStatus()).isEqualTo(SC_NO_CONTENT);
|
||||
assertThat(response.getPayload()).isEqualTo("Relock failed: Unknown revision ID 12128675309");
|
||||
assertThat(response.getPayload()).isEqualTo("Re-lock failed: Unknown revision ID 12128675309");
|
||||
assertTaskEnqueued(1, 12128675309L, Duration.standardMinutes(10)); // should retry, transient
|
||||
}
|
||||
|
||||
@Test
|
||||
void testFailure_pendingDelete() {
|
||||
void testFailure_pendingDelete() throws Exception {
|
||||
persistResource(domain.asBuilder().setStatusValues(ImmutableSet.of(PENDING_DELETE)).build());
|
||||
action.run();
|
||||
String expectedFailureMessage = "Domain example.tld has a pending delete.";
|
||||
assertThat(response.getStatus()).isEqualTo(SC_NO_CONTENT);
|
||||
assertThat(response.getPayload())
|
||||
.isEqualTo(String.format("Relock failed: Domain %s has a pending delete", DOMAIN_NAME));
|
||||
.isEqualTo(String.format("Re-lock failed: %s", expectedFailureMessage));
|
||||
assertNonTransientFailureEmail(expectedFailureMessage);
|
||||
assertNoTasksEnqueued(QUEUE_ASYNC_ACTIONS);
|
||||
}
|
||||
|
||||
@Test
|
||||
void testFailure_pendingTransfer() {
|
||||
void testFailure_pendingTransfer() throws Exception {
|
||||
persistResource(domain.asBuilder().setStatusValues(ImmutableSet.of(PENDING_TRANSFER)).build());
|
||||
action.run();
|
||||
String expectedFailureMessage = "Domain example.tld has a pending transfer.";
|
||||
assertThat(response.getStatus()).isEqualTo(SC_NO_CONTENT);
|
||||
assertThat(response.getPayload())
|
||||
.isEqualTo(String.format("Relock failed: Domain %s has a pending transfer", DOMAIN_NAME));
|
||||
.isEqualTo(String.format("Re-lock failed: %s", expectedFailureMessage));
|
||||
assertNonTransientFailureEmail(expectedFailureMessage);
|
||||
assertNoTasksEnqueued(QUEUE_ASYNC_ACTIONS);
|
||||
}
|
||||
|
||||
@Test
|
||||
|
|
@ -135,29 +177,64 @@ public class RelockDomainActionTest {
|
|||
action.run();
|
||||
assertThat(response.getStatus()).isEqualTo(SC_NO_CONTENT);
|
||||
assertThat(response.getPayload())
|
||||
.isEqualTo("Domain example.tld is already manually relocked, skipping automated relock.");
|
||||
.isEqualTo("Domain example.tld is already manually re-locked, skipping automated re-lock.");
|
||||
assertNoTasksEnqueued(QUEUE_ASYNC_ACTIONS);
|
||||
}
|
||||
|
||||
@Test
|
||||
void testFailure_domainDeleted() {
|
||||
void testFailure_domainDeleted() throws Exception {
|
||||
persistDomainAsDeleted(domain, clock.nowUtc());
|
||||
action.run();
|
||||
String expectedFailureMessage = "Domain example.tld has been deleted.";
|
||||
assertThat(response.getStatus()).isEqualTo(SC_NO_CONTENT);
|
||||
assertThat(response.getPayload())
|
||||
.isEqualTo(String.format("Relock failed: Domain %s has been deleted", DOMAIN_NAME));
|
||||
.isEqualTo(String.format("Re-lock failed: %s", expectedFailureMessage));
|
||||
assertNonTransientFailureEmail(expectedFailureMessage);
|
||||
assertNoTasksEnqueued(QUEUE_ASYNC_ACTIONS);
|
||||
}
|
||||
|
||||
@Test
|
||||
void testFailure_domainTransferred() {
|
||||
void testFailure_domainTransferred() throws Exception {
|
||||
persistResource(domain.asBuilder().setPersistedCurrentSponsorClientId("NewRegistrar").build());
|
||||
action.run();
|
||||
String expectedFailureMessage =
|
||||
"Domain example.tld has been transferred from registrar TheRegistrar to registrar "
|
||||
+ "NewRegistrar since the unlock.";
|
||||
assertThat(response.getStatus()).isEqualTo(SC_NO_CONTENT);
|
||||
assertThat(response.getPayload())
|
||||
.isEqualTo(
|
||||
String.format(
|
||||
"Relock failed: Domain %s has been transferred from registrar %s to registrar "
|
||||
+ "%s since the unlock",
|
||||
DOMAIN_NAME, CLIENT_ID, "NewRegistrar"));
|
||||
.isEqualTo(String.format("Re-lock failed: %s", expectedFailureMessage));
|
||||
assertNonTransientFailureEmail(expectedFailureMessage);
|
||||
assertNoTasksEnqueued(QUEUE_ASYNC_ACTIONS);
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testFailure_transientFailure_enqueuesTask() {
|
||||
// Hard-delete the domain to simulate a DB failure
|
||||
deleteResource(domain);
|
||||
action.run();
|
||||
assertThat(response.getStatus()).isEqualTo(SC_NO_CONTENT);
|
||||
assertThat(response.getPayload()).isEqualTo("Re-lock failed: null");
|
||||
assertTaskEnqueued(1);
|
||||
}
|
||||
|
||||
@Test
|
||||
void testFailure_sufficientTransientFailures_sendsEmail() throws Exception {
|
||||
// Hard-delete the domain to simulate a DB failure
|
||||
deleteResource(domain);
|
||||
action = createAction(oldLock.getRevisionId(), RelockDomainAction.FAILURES_BEFORE_EMAIL);
|
||||
action.run();
|
||||
assertTaskEnqueued(RelockDomainAction.FAILURES_BEFORE_EMAIL + 1);
|
||||
assertTransientFailureEmail();
|
||||
assertThat(response.getStatus()).isEqualTo(SC_NO_CONTENT);
|
||||
assertThat(response.getPayload()).isEqualTo("Re-lock failed: null");
|
||||
}
|
||||
|
||||
@Test
|
||||
void testSuccess_afterSufficientFailures_sendsEmail() throws Exception {
|
||||
action = createAction(oldLock.getRevisionId(), RelockDomainAction.FAILURES_BEFORE_EMAIL + 1);
|
||||
action.run();
|
||||
assertThat(response.getStatus()).isEqualTo(SC_OK);
|
||||
assertSuccessEmailSent();
|
||||
}
|
||||
|
||||
@Test
|
||||
|
|
@ -170,14 +247,108 @@ public class RelockDomainActionTest {
|
|||
action.run();
|
||||
assertThat(response.getStatus()).isEqualTo(SC_NO_CONTENT);
|
||||
assertThat(response.getPayload())
|
||||
.isEqualTo("Domain example.tld is already manually relocked, skipping automated relock.");
|
||||
.isEqualTo("Domain example.tld is already manually re-locked, skipping automated re-lock.");
|
||||
assertNoTasksEnqueued(QUEUE_ASYNC_ACTIONS);
|
||||
}
|
||||
|
||||
@Test
|
||||
void testFailure_slowsDown() throws Exception {
|
||||
deleteResource(domain);
|
||||
action = createAction(oldLock.getRevisionId(), RelockDomainAction.ATTEMPTS_BEFORE_SLOWDOWN);
|
||||
action.run();
|
||||
assertTaskEnqueued(
|
||||
RelockDomainAction.ATTEMPTS_BEFORE_SLOWDOWN + 1,
|
||||
oldLock.getRevisionId(),
|
||||
Duration.standardHours(1));
|
||||
}
|
||||
|
||||
private void assertSuccessEmailSent() throws Exception {
|
||||
EmailMessage expectedEmail =
|
||||
EmailMessage.newBuilder()
|
||||
.setSubject("Successful re-lock of domain example.tld")
|
||||
.setBody(
|
||||
"The domain example.tld was successfully re-locked.\n\nPlease "
|
||||
+ "contact support at support@example.com if you have any questions.")
|
||||
.setRecipients(
|
||||
ImmutableSet.of(new InternetAddress("Marla.Singer.RegistryLock@crr.com")))
|
||||
.setFrom(new InternetAddress("outgoing@example.com"))
|
||||
.build();
|
||||
verify(sendEmailService).sendEmail(expectedEmail);
|
||||
}
|
||||
|
||||
private void assertNonTransientFailureEmail(String exceptionMessage) throws Exception {
|
||||
String expectedBody =
|
||||
String.format(
|
||||
"There was an error when automatically re-locking example.tld. Error message: %s\n\n"
|
||||
+ "Please contact support at support@example.com if you have any questions.",
|
||||
exceptionMessage);
|
||||
assertFailureEmailWithBody(
|
||||
expectedBody, ImmutableSet.of(new InternetAddress("Marla.Singer.RegistryLock@crr.com")));
|
||||
}
|
||||
|
||||
private void assertTransientFailureEmail() throws Exception {
|
||||
String expectedBody =
|
||||
"There was an unexpected error when automatically re-locking example.tld. We will continue "
|
||||
+ "retrying the lock for five hours. Please contact support at support@example.com if "
|
||||
+ "you have any questions";
|
||||
assertFailureEmailWithBody(
|
||||
expectedBody,
|
||||
ImmutableSet.of(
|
||||
new InternetAddress("Marla.Singer.RegistryLock@crr.com"),
|
||||
new InternetAddress("alerts@example.com")));
|
||||
}
|
||||
|
||||
private void assertFailureEmailWithBody(String body, ImmutableSet<InternetAddress> recipients)
|
||||
throws Exception {
|
||||
EmailMessage expectedEmail =
|
||||
EmailMessage.newBuilder()
|
||||
.setSubject("Error re-locking domain example.tld")
|
||||
.setBody(body)
|
||||
.setRecipients(recipients)
|
||||
.setFrom(new InternetAddress("outgoing@example.com"))
|
||||
.build();
|
||||
verify(sendEmailService).sendEmail(expectedEmail);
|
||||
}
|
||||
|
||||
private void assertTaskEnqueued(int numAttempts) {
|
||||
assertTaskEnqueued(numAttempts, oldLock.getRevisionId(), Duration.standardMinutes(10));
|
||||
}
|
||||
|
||||
private void assertTaskEnqueued(int numAttempts, long oldUnlockRevisionId, Duration duration) {
|
||||
assertTasksEnqueued(
|
||||
QUEUE_ASYNC_ACTIONS,
|
||||
new TaskMatcher()
|
||||
.url(RelockDomainAction.PATH)
|
||||
.method("POST")
|
||||
.header("Host", "backend.hostname.fake")
|
||||
.param(
|
||||
RelockDomainAction.OLD_UNLOCK_REVISION_ID_PARAM,
|
||||
String.valueOf(oldUnlockRevisionId))
|
||||
.param(RelockDomainAction.PREVIOUS_ATTEMPTS_PARAM, String.valueOf(numAttempts))
|
||||
.etaDelta(duration.minus(standardSeconds(30)), duration.plus(standardSeconds(30))));
|
||||
}
|
||||
|
||||
private DomainBase reloadDomain(DomainBase domain) {
|
||||
return ofy().load().entity(domain).now();
|
||||
}
|
||||
|
||||
private RelockDomainAction createAction(Long oldUnlockRevisionId) {
|
||||
return new RelockDomainAction(oldUnlockRevisionId, domainLockUtils, response);
|
||||
private RelockDomainAction createAction(Long oldUnlockRevisionId) throws Exception {
|
||||
return createAction(oldUnlockRevisionId, 0);
|
||||
}
|
||||
|
||||
private RelockDomainAction createAction(Long oldUnlockRevisionId, int previousAttempts)
|
||||
throws Exception {
|
||||
InternetAddress alertRecipientAddress = new InternetAddress("alerts@example.com");
|
||||
InternetAddress gSuiteOutgoingAddress = new InternetAddress("outgoing@example.com");
|
||||
return new RelockDomainAction(
|
||||
oldUnlockRevisionId,
|
||||
previousAttempts,
|
||||
alertRecipientAddress,
|
||||
gSuiteOutgoingAddress,
|
||||
"support@example.com",
|
||||
sendEmailService,
|
||||
domainLockUtils,
|
||||
response,
|
||||
asyncTaskEnqueuer);
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -271,6 +271,7 @@ public final class DomainLockUtilsTest {
|
|||
.param(
|
||||
RelockDomainAction.OLD_UNLOCK_REVISION_ID_PARAM,
|
||||
String.valueOf(lock.getRevisionId()))
|
||||
.param(RelockDomainAction.PREVIOUS_ATTEMPTS_PARAM, "0")
|
||||
.etaDelta(
|
||||
standardHours(6).minus(standardSeconds(30)),
|
||||
standardDays(6).plus(standardSeconds(30))));
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue