diff --git a/src/sentry/tasks/relocation.py b/src/sentry/tasks/relocation.py index 445227467e8bea..00d2f51a03e48c 100644 --- a/src/sentry/tasks/relocation.py +++ b/src/sentry/tasks/relocation.py @@ -11,11 +11,9 @@ import yaml from cryptography.fernet import Fernet from django.db import router, transaction -from django.utils import timezone as get_timezone from google.cloud.devtools.cloudbuild_v1 import Build from google.cloud.devtools.cloudbuild_v1 import CloudBuildClient as CloudBuildClient -from sentry import options from sentry.api.serializers.rest_framework.base import camel_to_snake_case, convert_dict_key_case from sentry.backup.dependencies import NormalizedModelName, get_model from sentry.backup.exports import export_in_config_scope, export_in_user_scope @@ -28,7 +26,6 @@ ) from sentry.backup.imports import import_in_organization_scope from sentry.filestore.gcs import GoogleCloudStorage -from sentry.http import get_server_hostname from sentry.models.files.file import File from sentry.models.files.utils import get_storage from sentry.models.importchunk import RegionImportChunk @@ -42,21 +39,21 @@ ) from sentry.models.user import User from sentry.services.hybrid_cloud.organization import organization_service -from sentry.services.hybrid_cloud.user.service import user_service from sentry.silo import SiloMode from sentry.tasks.base import instrumented_task from sentry.utils import json from sentry.utils.db import atomic_transaction -from sentry.utils.email.message_builder import MessageBuilder from sentry.utils.env import gcp_project_id from sentry.utils.relocation import ( RELOCATION_BLOB_SIZE, RELOCATION_FILE_TYPE, + EmailKind, OrderedTask, create_cloudbuild_yaml, fail_relocation, get_bucket_name, retry_task_or_fail_relocation, + send_relocation_update_email, start_relocation_task, ) @@ -317,29 +314,14 @@ def preprocessing_scan(uuid: str) -> None: # The user's import data looks basically okay - we can use this opportunity to send a # "your relocation request has been accepted and is in flight, please give it a few # hours" email. - msg = MessageBuilder( - subject=f"{options.get('mail.subject-prefix')} Your Relocation has Started", - template="sentry/emails/relocation-started.txt", - html_template="sentry/emails/relocation-started.html", - type="relocation.started", - context={ - "domain": get_server_hostname(), - "datetime": get_timezone.now(), + send_relocation_update_email( + relocation, + EmailKind.STARTED, + { "uuid": str(relocation.uuid), "orgs": relocation.want_org_slugs, }, ) - email_to = [] - owner = user_service.get_user(user_id=relocation.owner_id) - if owner is not None: - email_to.append(owner.email) - - if relocation.owner_id != relocation.creator_id: - creator = user_service.get_user(user_id=relocation.creator_id) - if creator is not None: - email_to.append(creator.email) - - msg.send_async(to=email_to) @instrumented_task( @@ -628,9 +610,14 @@ def _update_relocation_validation_attempt( if relocation_validation.status < status.value: relocation_validation.status = status.value relocation_validation_attempt.save() - return fail_relocation( - relocation, task, "Validation could not be completed. Please contact support." + + transaction.on_commit( + lambda: fail_relocation( + relocation, task, "Validation could not be completed. Please contact support." + ), + using=router.db_for_write(Relocation), ) + return # All remaining statuses are final, so we can update the owning `RelocationValidation` now. assert status in {ValidationStatus.INVALID, ValidationStatus.VALID} @@ -646,9 +633,15 @@ def _update_relocation_validation_attempt( "Validation result: invalid", extra={"uuid": relocation.uuid, "task": task.name}, ) - return fail_relocation( - relocation, task, "The data you provided failed validation. Please contact support." + transaction.on_commit( + lambda: fail_relocation( + relocation, + task, + "The data you provided failed validation. Please contact support.", + ), + using=router.db_for_write(Relocation), ) + return assert status == ValidationStatus.VALID relocation.step = Relocation.Step.IMPORTING.value diff --git a/src/sentry/templates/sentry/debug/mail/preview.html b/src/sentry/templates/sentry/debug/mail/preview.html index 0b167446dd8d4a..f80fc429066d8f 100644 --- a/src/sentry/templates/sentry/debug/mail/preview.html +++ b/src/sentry/templates/sentry/debug/mail/preview.html @@ -55,6 +55,7 @@ + diff --git a/src/sentry/templates/sentry/emails/relocation_failed.html b/src/sentry/templates/sentry/emails/relocation_failed.html new file mode 100644 index 00000000000000..2dbed0fd54d97c --- /dev/null +++ b/src/sentry/templates/sentry/emails/relocation_failed.html @@ -0,0 +1,17 @@ +{% extends "sentry/emails/base.html" %} + +{% load i18n %} + +{% block main %} +

Relocation Failed

+

Your relocation has failed for the following reason:

+ {%if reason != "" %} + + {{ reason }} + +
+
+ {% endif %} +

Please contact support for further assistance if necessary.

+

ID: {{ uuid }}

+{% endblock %} diff --git a/src/sentry/templates/sentry/emails/relocation_failed.txt b/src/sentry/templates/sentry/emails/relocation_failed.txt new file mode 100644 index 00000000000000..3896090c58e812 --- /dev/null +++ b/src/sentry/templates/sentry/emails/relocation_failed.txt @@ -0,0 +1,9 @@ +Your relocation has failed for the following reason: + +{%if reason != "" %} +{{ reason }} +{% endif %} + +Please contact support at https://help.sentry.io for further assistance if necessary. + +ID: {{ uuid }} diff --git a/src/sentry/utils/relocation.py b/src/sentry/utils/relocation.py index 4e5cc32449b486..35543a5ed83047 100644 --- a/src/sentry/utils/relocation.py +++ b/src/sentry/utils/relocation.py @@ -5,13 +5,19 @@ from enum import Enum, unique from functools import lru_cache from string import Template -from typing import Generator, Optional, Tuple +from typing import Any, Generator, Optional, Tuple +from django.utils import timezone + +from sentry import options from sentry.backup.dependencies import dependencies, get_model_name, sorted_dependencies from sentry.backup.scopes import RelocationScope +from sentry.http import get_server_hostname from sentry.models.files.utils import get_storage from sentry.models.relocation import Relocation, RelocationFile from sentry.models.user import User +from sentry.services.hybrid_cloud.user.service import user_service +from sentry.utils.email.message_builder import MessageBuilder as MessageBuilder logger = logging.getLogger("sentry.relocation.tasks") @@ -267,6 +273,37 @@ class OrderedTask(Enum): ) +class EmailKind(Enum): + STARTED = 0 + FAILED = 1 + SUCCEEDED = 2 + + +def send_relocation_update_email( + relocation: Relocation, email_kind: EmailKind, args: dict[str, Any] +): + name = str(email_kind.name) + name_lower = name.lower() + msg = MessageBuilder( + subject=f"{options.get('mail.subject-prefix')} Your Relocation has {name.capitalize()}", + template=f"sentry/emails/relocation-{name_lower}.txt", + html_template=f"sentry/emails/relocation-{name_lower}.html", + type=f"relocation.{name_lower}", + context={"domain": get_server_hostname(), "datetime": timezone.now(), **args}, + ) + email_to = [] + owner = user_service.get_user(user_id=relocation.owner_id) + if owner is not None: + email_to.append(owner.email) + + if relocation.owner_id != relocation.creator_id: + creator = user_service.get_user(user_id=relocation.creator_id) + if creator is not None: + email_to.append(creator.email) + + msg.send_async(to=email_to) + + def start_relocation_task( uuid: str, step: Relocation.Step, task: OrderedTask, allowed_task_attempts: int ) -> Tuple[Optional[Relocation], int]: @@ -341,7 +378,14 @@ def fail_relocation(relocation: Relocation, task: OrderedTask, reason: str = "") relocation.save() logger.info("Task failed", extra={"uuid": relocation.uuid, "task": task.name, "reason": reason}) - return + send_relocation_update_email( + relocation, + EmailKind.FAILED, + { + "uuid": str(relocation.uuid), + "reason": reason, + }, + ) @contextmanager diff --git a/src/sentry/web/debug_urls.py b/src/sentry/web/debug_urls.py index 52226bbef6859b..956a9b25d4d33a 100644 --- a/src/sentry/web/debug_urls.py +++ b/src/sentry/web/debug_urls.py @@ -123,6 +123,7 @@ re_path(r"^debug/mail/confirm-email/$", sentry.web.frontend.debug.mail.confirm_email), re_path(r"^debug/mail/recover-account/$", sentry.web.frontend.debug.mail.recover_account), re_path(r"^debug/mail/relocate-account/$", sentry.web.frontend.debug.mail.relocate_account), + re_path(r"^debug/mail/relocation-failed/$", sentry.web.frontend.debug.mail.relocation_failed), re_path(r"^debug/mail/relocation-started/$", sentry.web.frontend.debug.mail.relocation_started), re_path(r"^debug/mail/unable-to-delete-repo/$", DebugUnableToDeleteRepository.as_view()), re_path(r"^debug/mail/unable-to-fetch-commits/$", DebugUnableToFetchCommitsEmailView.as_view()), diff --git a/src/sentry/web/frontend/debug/mail.py b/src/sentry/web/frontend/debug/mail.py index 525c688620e73c..4770b9ab3bf59a 100644 --- a/src/sentry/web/frontend/debug/mail.py +++ b/src/sentry/web/frontend/debug/mail.py @@ -738,6 +738,20 @@ def relocate_account(request): ).render(request) +@login_required +def relocation_failed(request): + return MailPreview( + html_template="sentry/emails/relocation_failed.html", + text_template="sentry/emails/relocation_failed.txt", + context={ + "domain": get_server_hostname(), + "datetime": timezone.now(), + "uuid": str(uuid.uuid4().hex), + "reason": "This is a sample failure reason", + }, + ).render(request) + + @login_required def relocation_started(request): return MailPreview( diff --git a/tests/sentry/tasks/test_relocation.py b/tests/sentry/tasks/test_relocation.py index a5d18ef79dea47..56088ddf5bdacf 100644 --- a/tests/sentry/tasks/test_relocation.py +++ b/tests/sentry/tasks/test_relocation.py @@ -182,46 +182,77 @@ def mock_cloudbuild_client( fake_cloudbuild_client.get_build.return_value = SimpleNamespace(status=status) fake_cloudbuild_client.get_build.side_effect = None + def mock_message_builder(self, fake_message_builder: Mock): + fake_message_builder.return_value.send_async.return_value = MagicMock() + +@patch("sentry.utils.relocation.MessageBuilder") @patch("sentry.tasks.relocation.preprocessing_scan.delay") @region_silo_test class UploadingCompleteTest(RelocationTaskTestCase): - def test_success(self, preprocessing_scan_mock: Mock): + def test_success( + self, + preprocessing_scan_mock: Mock, + fake_message_builder: Mock, + ): + self.mock_message_builder(fake_message_builder) + uploading_complete(self.uuid) + assert fake_message_builder.call_count == 0 assert preprocessing_scan_mock.call_count == 1 - def test_retry_if_attempts_left(self, preprocessing_scan_mock: Mock): + def test_retry_if_attempts_left( + self, + preprocessing_scan_mock: Mock, + fake_message_builder: Mock, + ): RelocationFile.objects.filter(relocation=self.relocation).delete() + self.mock_message_builder(fake_message_builder) # An exception being raised will trigger a retry in celery. with pytest.raises(Exception): uploading_complete(self.uuid) + assert fake_message_builder.call_count == 0 + assert preprocessing_scan_mock.call_count == 0 + relocation = Relocation.objects.get(uuid=self.uuid) assert relocation.status == Relocation.Status.IN_PROGRESS.value assert not relocation.failure_reason - assert preprocessing_scan_mock.call_count == 0 - def test_fail_if_no_attempts_left(self, preprocessing_scan_mock: Mock): + def test_fail_if_no_attempts_left( + self, + preprocessing_scan_mock: Mock, + fake_message_builder: Mock, + ): self.relocation.latest_task = "UPLOADING_COMPLETE" self.relocation.latest_task_attempts = MAX_FAST_TASK_RETRIES self.relocation.save() RelocationFile.objects.filter(relocation=self.relocation).delete() + self.mock_message_builder(fake_message_builder) with pytest.raises(Exception): uploading_complete(self.uuid) + assert fake_message_builder.call_count == 1 + assert fake_message_builder.call_args.kwargs["type"] == "relocation.failed" + fake_message_builder.return_value.send_async.assert_called_once_with( + to=[self.owner.email, self.superuser.email] + ) + + assert preprocessing_scan_mock.call_count == 0 + relocation = Relocation.objects.get(uuid=self.uuid) assert relocation.status == Relocation.Status.FAILURE.value assert relocation.failure_reason == ERR_UPLOADING_FAILED - assert preprocessing_scan_mock.call_count == 0 @patch( "sentry.backup.helpers.KeyManagementServiceClient", new_callable=lambda: FakeKeyManagementServiceClient, ) +@patch("sentry.utils.relocation.MessageBuilder") @patch("sentry.tasks.relocation.preprocessing_baseline_config.delay") @region_silo_test class PreprocessingScanTest(RelocationTaskTestCase): @@ -231,28 +262,33 @@ def setUp(self): self.relocation.latest_task = "UPLOADING_COMPLETE" self.relocation.save() - @patch("sentry.utils.email.MessageBuilder.send_async") def test_success_admin_assisted_relocation( self, - send_async_mock: Mock, preprocessing_baseline_config_mock: Mock, + fake_message_builder: Mock, fake_kms_client: FakeKeyManagementServiceClient, ): + self.mock_message_builder(fake_message_builder) self.mock_kms_client(fake_kms_client) preprocessing_scan(self.uuid) assert fake_kms_client.asymmetric_decrypt.call_count == 1 assert fake_kms_client.get_public_key.call_count == 0 + + assert fake_message_builder.call_count == 1 + assert fake_message_builder.call_args.kwargs["type"] == "relocation.started" + fake_message_builder.return_value.send_async.assert_called_once_with( + to=[self.owner.email, self.superuser.email] + ) + assert preprocessing_baseline_config_mock.call_count == 1 - send_async_mock.assert_called_once_with(to=[self.owner.email, self.superuser.email]) assert Relocation.objects.get(uuid=self.uuid).want_usernames == ["testing@example.com"] - @patch("sentry.utils.email.MessageBuilder.send_async") def test_success_self_service_relocation( self, - send_async_mock: Mock, preprocessing_baseline_config_mock: Mock, + fake_message_builder: Mock, fake_kms_client: FakeKeyManagementServiceClient, ): self.mock_kms_client(fake_kms_client) @@ -263,186 +299,278 @@ def test_success_self_service_relocation( assert fake_kms_client.asymmetric_decrypt.call_count == 1 assert fake_kms_client.get_public_key.call_count == 0 + + assert fake_message_builder.call_count == 1 + assert fake_message_builder.call_args.kwargs["type"] == "relocation.started" + fake_message_builder.return_value.send_async.assert_called_once_with(to=[self.owner.email]) + assert preprocessing_baseline_config_mock.call_count == 1 - send_async_mock.assert_called_once_with(to=[self.owner.email]) + assert Relocation.objects.get(uuid=self.uuid).want_usernames == ["testing@example.com"] def test_retry_if_attempts_left( self, preprocessing_baseline_config_mock: Mock, + fake_message_builder: Mock, fake_kms_client: FakeKeyManagementServiceClient, ): RelocationFile.objects.filter(relocation=self.relocation).delete() + self.mock_message_builder(fake_message_builder) + self.mock_kms_client(fake_kms_client) # An exception being raised will trigger a retry in celery. with pytest.raises(Exception): - self.mock_kms_client(fake_kms_client) preprocessing_scan(self.uuid) - relocation = Relocation.objects.get(uuid=self.uuid) - assert relocation.status == Relocation.Status.IN_PROGRESS.value - assert not relocation.failure_reason assert fake_kms_client.asymmetric_decrypt.call_count == 0 assert fake_kms_client.get_public_key.call_count == 0 + assert fake_message_builder.call_count == 0 assert preprocessing_baseline_config_mock.call_count == 0 + relocation = Relocation.objects.get(uuid=self.uuid) + assert relocation.status == Relocation.Status.IN_PROGRESS.value + assert not relocation.failure_reason + def test_fail_if_no_attempts_left( self, preprocessing_baseline_config_mock: Mock, + fake_message_builder: Mock, fake_kms_client: FakeKeyManagementServiceClient, ): self.relocation.latest_task = "PREPROCESSING_SCAN" self.relocation.latest_task_attempts = MAX_FAST_TASK_RETRIES self.relocation.save() RelocationFile.objects.filter(relocation=self.relocation).delete() + self.mock_message_builder(fake_message_builder) self.mock_kms_client(fake_kms_client) with pytest.raises(Exception): preprocessing_scan(self.uuid) - relocation = Relocation.objects.get(uuid=self.uuid) - assert relocation.status == Relocation.Status.FAILURE.value - assert relocation.failure_reason == ERR_PREPROCESSING_INTERNAL assert fake_kms_client.asymmetric_decrypt.call_count == 0 assert fake_kms_client.get_public_key.call_count == 0 + + assert fake_message_builder.call_count == 1 + assert fake_message_builder.call_args.kwargs["type"] == "relocation.failed" + fake_message_builder.return_value.send_async.assert_called_once_with( + to=[self.owner.email, self.superuser.email] + ) + assert preprocessing_baseline_config_mock.call_count == 0 + relocation = Relocation.objects.get(uuid=self.uuid) + assert relocation.status == Relocation.Status.FAILURE.value + assert relocation.failure_reason == ERR_PREPROCESSING_INTERNAL + def test_fail_invalid_tarball( self, preprocessing_baseline_config_mock: Mock, + fake_message_builder: Mock, fake_kms_client: FakeKeyManagementServiceClient, ): file = RelocationFile.objects.get(relocation=self.relocation).file corrupted_tarball_bytes = bytearray(file.getfile().read())[9:] file.putfile(BytesIO(bytes(corrupted_tarball_bytes))) + self.mock_message_builder(fake_message_builder) self.mock_kms_client(fake_kms_client) preprocessing_scan(self.uuid) + assert fake_message_builder.call_count == 1 + assert fake_message_builder.call_args.kwargs["type"] == "relocation.failed" + fake_message_builder.return_value.send_async.assert_called_once_with( + to=[self.owner.email, self.superuser.email] + ) + + assert preprocessing_baseline_config_mock.call_count == 0 + relocation = Relocation.objects.get(uuid=self.uuid) assert relocation.status == Relocation.Status.FAILURE.value assert relocation.failure_reason == ERR_PREPROCESSING_INVALID_TARBALL - assert preprocessing_baseline_config_mock.call_count == 0 def test_fail_decryption_failure( self, preprocessing_baseline_config_mock: Mock, + fake_message_builder: Mock, fake_kms_client: FakeKeyManagementServiceClient, ): # Add invalid 2-octet UTF-8 sequence to the returned plaintext. + self.mock_message_builder(fake_message_builder) self.mock_kms_client(fake_kms_client) fake_kms_client.asymmetric_decrypt.return_value.plaintext += b"\xc3\x28" preprocessing_scan(self.uuid) + assert fake_message_builder.call_count == 1 + assert fake_message_builder.call_args.kwargs["type"] == "relocation.failed" + fake_message_builder.return_value.send_async.assert_called_once_with( + to=[self.owner.email, self.superuser.email] + ) + + assert preprocessing_baseline_config_mock.call_count == 0 + relocation = Relocation.objects.get(uuid=self.uuid) assert relocation.status == Relocation.Status.FAILURE.value assert relocation.failure_reason == ERR_PREPROCESSING_DECRYPTION - assert preprocessing_baseline_config_mock.call_count == 0 def test_fail_invalid_json( self, preprocessing_baseline_config_mock: Mock, + fake_message_builder: Mock, fake_kms_client: FakeKeyManagementServiceClient, ): file = RelocationFile.objects.get(relocation=self.relocation).file self.swap_file(file, "invalid-user.json") + self.mock_message_builder(fake_message_builder) self.mock_kms_client(fake_kms_client) preprocessing_scan(self.uuid) + assert fake_message_builder.call_count == 1 + assert fake_message_builder.call_args.kwargs["type"] == "relocation.failed" + fake_message_builder.return_value.send_async.assert_called_once_with( + to=[self.owner.email, self.superuser.email] + ) + + assert preprocessing_baseline_config_mock.call_count == 0 + relocation = Relocation.objects.get(uuid=self.uuid) assert relocation.status == Relocation.Status.FAILURE.value assert relocation.failure_reason == ERR_PREPROCESSING_INVALID_JSON - assert preprocessing_baseline_config_mock.call_count == 0 def test_fail_no_users( self, preprocessing_baseline_config_mock: Mock, + fake_message_builder: Mock, fake_kms_client: FakeKeyManagementServiceClient, ): file = RelocationFile.objects.get(relocation=self.relocation).file self.swap_file(file, "single-option.json") + self.mock_message_builder(fake_message_builder) self.mock_kms_client(fake_kms_client) preprocessing_scan(self.uuid) + assert fake_message_builder.call_count == 1 + assert fake_message_builder.call_args.kwargs["type"] == "relocation.failed" + fake_message_builder.return_value.send_async.assert_called_once_with( + to=[self.owner.email, self.superuser.email] + ) + + assert preprocessing_baseline_config_mock.call_count == 0 + relocation = Relocation.objects.get(uuid=self.uuid) assert relocation.status == Relocation.Status.FAILURE.value assert relocation.failure_reason == ERR_PREPROCESSING_NO_USERS - assert preprocessing_baseline_config_mock.call_count == 0 @patch("sentry.tasks.relocation.MAX_USERS_PER_RELOCATION", 0) def test_fail_too_many_users( self, preprocessing_baseline_config_mock: Mock, + fake_message_builder: Mock, fake_kms_client: FakeKeyManagementServiceClient, ): + self.mock_message_builder(fake_message_builder) self.mock_kms_client(fake_kms_client) preprocessing_scan(self.uuid) + assert fake_message_builder.call_count == 1 + assert fake_message_builder.call_args.kwargs["type"] == "relocation.failed" + fake_message_builder.return_value.send_async.assert_called_once_with( + to=[self.owner.email, self.superuser.email] + ) + + assert preprocessing_baseline_config_mock.call_count == 0 + relocation = Relocation.objects.get(uuid=self.uuid) assert relocation.status == Relocation.Status.FAILURE.value assert relocation.failure_reason == ERR_PREPROCESSING_TOO_MANY_USERS.substitute(count=1) - assert preprocessing_baseline_config_mock.call_count == 0 def test_fail_no_orgs( self, preprocessing_baseline_config_mock: Mock, + fake_message_builder: Mock, fake_kms_client: FakeKeyManagementServiceClient, ): file = RelocationFile.objects.get(relocation=self.relocation).file self.swap_file(file, "user-with-minimum-privileges.json") + self.mock_message_builder(fake_message_builder) self.mock_kms_client(fake_kms_client) preprocessing_scan(self.uuid) + assert fake_message_builder.call_count == 1 + assert fake_message_builder.call_args.kwargs["type"] == "relocation.failed" + fake_message_builder.return_value.send_async.assert_called_once_with( + to=[self.owner.email, self.superuser.email] + ) + + assert preprocessing_baseline_config_mock.call_count == 0 + relocation = Relocation.objects.get(uuid=self.uuid) assert relocation.status == Relocation.Status.FAILURE.value assert relocation.failure_reason == ERR_PREPROCESSING_NO_ORGS - assert preprocessing_baseline_config_mock.call_count == 0 @patch("sentry.tasks.relocation.MAX_ORGS_PER_RELOCATION", 0) def test_fail_too_many_orgs( self, preprocessing_baseline_config_mock: Mock, + fake_message_builder: Mock, fake_kms_client: FakeKeyManagementServiceClient, ): + self.mock_message_builder(fake_message_builder) self.mock_kms_client(fake_kms_client) preprocessing_scan(self.uuid) + assert fake_message_builder.call_count == 1 + assert fake_message_builder.call_args.kwargs["type"] == "relocation.failed" + fake_message_builder.return_value.send_async.assert_called_once_with( + to=[self.owner.email, self.superuser.email] + ) + + assert preprocessing_baseline_config_mock.call_count == 0 + relocation = Relocation.objects.get(uuid=self.uuid) assert relocation.status == Relocation.Status.FAILURE.value assert relocation.failure_reason == ERR_PREPROCESSING_TOO_MANY_ORGS.substitute(count=1) - assert preprocessing_baseline_config_mock.call_count == 0 def test_fail_missing_orgs( self, preprocessing_baseline_config_mock: Mock, + fake_message_builder: Mock, fake_kms_client: FakeKeyManagementServiceClient, ): orgs = ["does-not-exist"] relocation = Relocation.objects.get(uuid=self.uuid) relocation.want_org_slugs = orgs relocation.save() + self.mock_message_builder(fake_message_builder) self.mock_kms_client(fake_kms_client) preprocessing_scan(self.uuid) + assert fake_message_builder.call_count == 1 + assert fake_message_builder.call_args.kwargs["type"] == "relocation.failed" + fake_message_builder.return_value.send_async.assert_called_once_with( + to=[self.owner.email, self.superuser.email] + ) + + assert preprocessing_baseline_config_mock.call_count == 0 + relocation = Relocation.objects.get(uuid=self.uuid) assert relocation.status == Relocation.Status.FAILURE.value assert relocation.failure_reason == ERR_PREPROCESSING_MISSING_ORGS.substitute( orgs=",".join(orgs) ) - assert preprocessing_baseline_config_mock.call_count == 0 @patch( "sentry.backup.helpers.KeyManagementServiceClient", new_callable=lambda: FakeKeyManagementServiceClient, ) +@patch("sentry.utils.relocation.MessageBuilder") @patch("sentry.tasks.relocation.preprocessing_colliding_users.delay") @region_silo_test class PreprocessingBaselineConfigTest(RelocationTaskTestCase): @@ -455,14 +583,17 @@ def setUp(self): def test_success( self, preprocessing_colliding_users_mock: Mock, + fake_message_builder: Mock, fake_kms_client: FakeKeyManagementServiceClient, ): + self.mock_message_builder(fake_message_builder) self.mock_kms_client(fake_kms_client) preprocessing_baseline_config(self.uuid) assert fake_kms_client.asymmetric_decrypt.call_count == 0 assert fake_kms_client.get_public_key.call_count == 1 + assert fake_message_builder.call_count == 0 assert preprocessing_colliding_users_mock.call_count == 1 relocation_file = ( @@ -489,51 +620,67 @@ def test_success( def test_retry_if_attempts_left( self, preprocessing_colliding_users_mock: Mock, + fake_message_builder: Mock, fake_kms_client: FakeKeyManagementServiceClient, ): + self.mock_message_builder(fake_message_builder) + self.mock_kms_client(fake_kms_client) RelocationFile.objects.filter(relocation=self.relocation).delete() # An exception being raised will trigger a retry in celery. with pytest.raises(Exception): - self.mock_kms_client(fake_kms_client) fake_kms_client.get_public_key.side_effect = Exception("Test") preprocessing_baseline_config(self.uuid) - relocation = Relocation.objects.get(uuid=self.uuid) - assert relocation.status == Relocation.Status.IN_PROGRESS.value - assert not relocation.failure_reason assert fake_kms_client.asymmetric_decrypt.call_count == 0 assert fake_kms_client.get_public_key.call_count == 1 + assert fake_message_builder.call_count == 0 assert preprocessing_colliding_users_mock.call_count == 0 + relocation = Relocation.objects.get(uuid=self.uuid) + assert relocation.status == Relocation.Status.IN_PROGRESS.value + assert not relocation.failure_reason + def test_fail_if_no_attempts_left( self, preprocessing_colliding_users_mock: Mock, + fake_message_builder: Mock, fake_kms_client: FakeKeyManagementServiceClient, ): self.relocation.latest_task = "PREPROCESSING_BASELINE_CONFIG" self.relocation.latest_task_attempts = MAX_FAST_TASK_RETRIES self.relocation.save() RelocationFile.objects.filter(relocation=self.relocation).delete() + + self.mock_message_builder(fake_message_builder) self.mock_kms_client(fake_kms_client) fake_kms_client.get_public_key.side_effect = Exception("Test") with pytest.raises(Exception): preprocessing_baseline_config(self.uuid) - relocation = Relocation.objects.get(uuid=self.uuid) - assert relocation.status == Relocation.Status.FAILURE.value - assert relocation.failure_reason == ERR_PREPROCESSING_INTERNAL assert fake_kms_client.asymmetric_decrypt.call_count == 0 assert fake_kms_client.get_public_key.call_count == 1 + + assert fake_message_builder.call_count == 1 + assert fake_message_builder.call_args.kwargs["type"] == "relocation.failed" + fake_message_builder.return_value.send_async.assert_called_once_with( + to=[self.owner.email, self.superuser.email] + ) + assert preprocessing_colliding_users_mock.call_count == 0 + relocation = Relocation.objects.get(uuid=self.uuid) + assert relocation.status == Relocation.Status.FAILURE.value + assert relocation.failure_reason == ERR_PREPROCESSING_INTERNAL + @patch( "sentry.backup.helpers.KeyManagementServiceClient", new_callable=lambda: FakeKeyManagementServiceClient, ) +@patch("sentry.utils.relocation.MessageBuilder") @patch("sentry.tasks.relocation.preprocessing_complete.delay") @region_silo_test class PreprocessingCollidingUsersTest(RelocationTaskTestCase): @@ -551,14 +698,18 @@ def setUp(self): def test_success( self, preprocessing_complete_mock: Mock, + fake_message_builder: Mock, fake_kms_client: FakeKeyManagementServiceClient, ): + self.mock_message_builder(fake_message_builder) self.mock_kms_client(fake_kms_client) + preprocessing_colliding_users(self.uuid) - assert preprocessing_complete_mock.call_count == 1 assert fake_kms_client.asymmetric_decrypt.call_count == 0 assert fake_kms_client.get_public_key.call_count == 1 + assert fake_message_builder.call_count == 0 + assert preprocessing_complete_mock.call_count == 1 relocation_file = ( RelocationFile.objects.filter( @@ -584,47 +735,63 @@ def test_success( def test_retry_if_attempts_left( self, preprocessing_complete_mock: Mock, + fake_message_builder: Mock, fake_kms_client: FakeKeyManagementServiceClient, ): RelocationFile.objects.filter(relocation=self.relocation).delete() + self.mock_message_builder(fake_message_builder) + self.mock_kms_client(fake_kms_client) # An exception being raised will trigger a retry in celery. with pytest.raises(Exception): - self.mock_kms_client(fake_kms_client) fake_kms_client.get_public_key.side_effect = Exception("Test") preprocessing_colliding_users(self.uuid) - relocation = Relocation.objects.get(uuid=self.uuid) - assert relocation.status == Relocation.Status.IN_PROGRESS.value - assert not relocation.failure_reason assert fake_kms_client.asymmetric_decrypt.call_count == 0 assert fake_kms_client.get_public_key.call_count == 1 + assert fake_message_builder.call_count == 0 assert preprocessing_complete_mock.call_count == 0 + relocation = Relocation.objects.get(uuid=self.uuid) + assert relocation.status == Relocation.Status.IN_PROGRESS.value + assert not relocation.failure_reason + def test_fail_if_no_attempts_left( self, preprocessing_complete_mock: Mock, + fake_message_builder: Mock, fake_kms_client: FakeKeyManagementServiceClient, ): self.relocation.latest_task = "PREPROCESSING_COLLIDING_USERS" self.relocation.latest_task_attempts = MAX_FAST_TASK_RETRIES self.relocation.save() RelocationFile.objects.filter(relocation=self.relocation).delete() + + self.mock_message_builder(fake_message_builder) self.mock_kms_client(fake_kms_client) fake_kms_client.get_public_key.side_effect = Exception("Test") with pytest.raises(Exception): preprocessing_colliding_users(self.uuid) - relocation = Relocation.objects.get(uuid=self.uuid) - assert relocation.status == Relocation.Status.FAILURE.value - assert relocation.failure_reason == ERR_PREPROCESSING_INTERNAL assert fake_kms_client.asymmetric_decrypt.call_count == 0 assert fake_kms_client.get_public_key.call_count == 1 + + assert fake_message_builder.call_count == 1 + assert fake_message_builder.call_args.kwargs["type"] == "relocation.failed" + fake_message_builder.return_value.send_async.assert_called_once_with( + to=[self.owner.email, self.superuser.email] + ) + assert preprocessing_complete_mock.call_count == 0 + relocation = Relocation.objects.get(uuid=self.uuid) + assert relocation.failure_reason == ERR_PREPROCESSING_INTERNAL + assert relocation.status == Relocation.Status.FAILURE.value + +@patch("sentry.utils.relocation.MessageBuilder") @patch("sentry.tasks.relocation.validating_start.delay") @region_silo_test class PreprocessingCompleteTest(RelocationTaskTestCase): @@ -655,12 +822,17 @@ def setUp(self): ) assert file.blobs.count() > 1 # A bit bigger, so we get chunks. - def test_success(self, validating_start_mock: Mock): + def test_success( + self, + validating_start_mock: Mock, + fake_message_builder: Mock, + ): + self.mock_message_builder(fake_message_builder) assert not self.storage.exists(f"relocations/runs/{self.uuid}") preprocessing_complete(self.uuid) - self.relocation.refresh_from_db() + assert fake_message_builder.call_count == 0 assert validating_start_mock.call_count == 1 (_, files) = self.storage.listdir(f"relocations/runs/{self.uuid}/conf") @@ -699,40 +871,61 @@ def test_success(self, validating_start_mock: Mock): with kms_file: json.load(kms_file) + self.relocation.refresh_from_db() assert self.relocation.step == Relocation.Step.VALIDATING.value assert RelocationValidation.objects.filter(relocation=self.relocation).count() == 1 - def test_retry_if_attempts_left(self, validating_start_mock: Mock): + def test_retry_if_attempts_left( + self, + validating_start_mock: Mock, + fake_message_builder: Mock, + ): RelocationFile.objects.filter(relocation=self.relocation).delete() + self.mock_message_builder(fake_message_builder) # An exception being raised will trigger a retry in celery. with pytest.raises(Exception): preprocessing_complete(self.uuid) + assert fake_message_builder.call_count == 0 + assert validating_start_mock.call_count == 0 + relocation = Relocation.objects.get(uuid=self.uuid) assert relocation.status == Relocation.Status.IN_PROGRESS.value assert not relocation.failure_reason - assert validating_start_mock.call_count == 0 - def test_fail_if_no_attempts_left(self, validating_start_mock: Mock): + def test_fail_if_no_attempts_left( + self, + validating_start_mock: Mock, + fake_message_builder: Mock, + ): self.relocation.latest_task = "PREPROCESSING_COMPLETE" self.relocation.latest_task_attempts = MAX_FAST_TASK_RETRIES self.relocation.save() RelocationFile.objects.filter(relocation=self.relocation).delete() + self.mock_message_builder(fake_message_builder) with pytest.raises(Exception): preprocessing_complete(self.uuid) + assert fake_message_builder.call_count == 1 + assert fake_message_builder.call_args.kwargs["type"] == "relocation.failed" + fake_message_builder.return_value.send_async.assert_called_once_with( + to=[self.owner.email, self.superuser.email] + ) + + assert validating_start_mock.call_count == 0 + relocation = Relocation.objects.get(uuid=self.uuid) assert relocation.status == Relocation.Status.FAILURE.value assert relocation.failure_reason == ERR_PREPROCESSING_INTERNAL - assert validating_start_mock.call_count == 0 @patch( "sentry.tasks.relocation.CloudBuildClient", new_callable=lambda: FakeCloudBuildClient, ) +@patch("sentry.utils.relocation.MessageBuilder") @patch("sentry.tasks.relocation.validating_poll.delay") @region_silo_test class ValidatingStartTest(RelocationTaskTestCase): @@ -751,18 +944,21 @@ def setUp(self): def test_success( self, validating_poll_mock: Mock, + fake_message_builder: Mock, fake_cloudbuild_client: FakeCloudBuildClient, ): self.mock_cloudbuild_client(fake_cloudbuild_client, Build.Status(Build.Status.QUEUED)) + self.mock_message_builder(fake_message_builder) validating_start(self.uuid) - self.relocation.refresh_from_db() - self.relocation_validation.refresh_from_db() assert validating_poll_mock.call_count == 1 assert fake_cloudbuild_client.create_build.call_count == 1 - assert self.relocation_validation.attempts == 1 + + self.relocation.refresh_from_db() + self.relocation_validation.refresh_from_db() assert self.relocation_validation.status == ValidationStatus.IN_PROGRESS.value + assert self.relocation_validation.attempts == 1 relocation_validation_attempt = RelocationValidationAttempt.objects.get( relocation_validation=self.relocation_validation @@ -772,42 +968,55 @@ def test_success( def test_retry_if_attempts_left( self, validating_poll_mock: Mock, + fake_message_builder: Mock, fake_cloudbuild_client: FakeCloudBuildClient, ): + self.mock_cloudbuild_client(fake_cloudbuild_client, Build.Status(Build.Status.QUEUED)) + self.mock_message_builder(fake_message_builder) + # An exception being raised will trigger a retry in celery. with pytest.raises(Exception): - self.mock_cloudbuild_client(fake_cloudbuild_client, Build.Status(Build.Status.QUEUED)) fake_cloudbuild_client.create_build.side_effect = Exception("Test") validating_start(self.uuid) + assert fake_cloudbuild_client.create_build.call_count == 1 + assert fake_message_builder.call_count == 0 + assert validating_poll_mock.call_count == 0 + relocation = Relocation.objects.get(uuid=self.uuid) assert relocation.status == Relocation.Status.IN_PROGRESS.value assert not relocation.failure_reason - assert validating_poll_mock.call_count == 0 def test_fail_if_no_attempts_left( self, validating_poll_mock: Mock, + fake_message_builder: Mock, fake_cloudbuild_client: FakeCloudBuildClient, ): self.relocation.latest_task = "VALIDATING_START" self.relocation.latest_task_attempts = MAX_FAST_TASK_RETRIES self.relocation.save() + self.mock_cloudbuild_client(fake_cloudbuild_client, Build.Status(Build.Status.QUEUED)) fake_cloudbuild_client.create_build.side_effect = Exception("Test") + self.mock_message_builder(fake_message_builder) with pytest.raises(Exception): validating_start(self.uuid) + assert fake_cloudbuild_client.create_build.call_count == 1 + assert fake_message_builder.call_count == 1 + assert validating_poll_mock.call_count == 0 + relocation = Relocation.objects.get(uuid=self.uuid) assert relocation.status == Relocation.Status.FAILURE.value assert relocation.failure_reason == ERR_VALIDATING_INTERNAL - assert validating_poll_mock.call_count == 0 def test_fail_if_max_runs_attempted( self, validating_poll_mock: Mock, + fake_message_builder: Mock, fake_cloudbuild_client: FakeCloudBuildClient, ): for _ in range(3): @@ -824,20 +1033,24 @@ def test_fail_if_max_runs_attempted( self.relocation.save() self.mock_cloudbuild_client(fake_cloudbuild_client, Build.Status(Build.Status.QUEUED)) - fake_cloudbuild_client.create_build.side_effect = Exception("Test") + self.mock_message_builder(fake_message_builder) validating_start(self.uuid) + assert fake_cloudbuild_client.create_build.call_count == 0 + assert fake_message_builder.call_count == 1 + assert validating_poll_mock.call_count == 0 + relocation = Relocation.objects.get(uuid=self.uuid) assert relocation.status == Relocation.Status.FAILURE.value assert relocation.failure_reason == ERR_VALIDATING_MAX_RUNS - assert validating_poll_mock.call_count == 0 @patch( "sentry.tasks.relocation.CloudBuildClient", new_callable=lambda: FakeCloudBuildClient, ) +@patch("sentry.utils.relocation.MessageBuilder") @region_silo_test class ValidatingPollTest(RelocationTaskTestCase): def setUp(self): @@ -864,37 +1077,46 @@ def setUp(self): def test_success( self, validating_complete_mock: Mock, + fake_message_builder: Mock, fake_cloudbuild_client: FakeCloudBuildClient, ): self.mock_cloudbuild_client(fake_cloudbuild_client, Build.Status(Build.Status.SUCCESS)) + self.mock_message_builder(fake_message_builder) validating_poll(self.uuid, self.relocation_validation_attempt.build_id) + + assert fake_cloudbuild_client.get_build.call_count == 1 + assert fake_message_builder.call_count == 0 + assert validating_complete_mock.call_count == 1 + self.relocation.refresh_from_db() self.relocation_validation.refresh_from_db() self.relocation_validation_attempt.refresh_from_db() - - assert validating_complete_mock.call_count == 1 - assert fake_cloudbuild_client.get_build.call_count == 1 - assert self.relocation.latest_task == "VALIDATING_POLL" assert self.relocation_validation.status == ValidationStatus.IN_PROGRESS.value + assert self.relocation.latest_task == "VALIDATING_POLL" @patch("sentry.tasks.relocation.validating_start.delay") def test_timeout_starts_new_validation_attempt( self, validating_start_mock: Mock, + fake_message_builder: Mock, fake_cloudbuild_client: FakeCloudBuildClient, ): for stat in {Build.Status.TIMEOUT, Build.Status.EXPIRED}: + self.mock_message_builder(fake_message_builder) self.mock_cloudbuild_client(fake_cloudbuild_client, Build.Status(stat)) validating_start_mock.call_count = 0 validating_poll(self.uuid, self.relocation_validation_attempt.build_id) + + assert fake_cloudbuild_client.get_build.call_count == 1 + assert fake_message_builder.call_count == 0 + assert validating_start_mock.call_count == 1 + self.relocation.refresh_from_db() self.relocation_validation.refresh_from_db() self.relocation_validation_attempt.refresh_from_db() - assert validating_start_mock.call_count == 1 - assert fake_cloudbuild_client.get_build.call_count == 1 assert self.relocation.latest_task == "VALIDATING_START" assert self.relocation_validation.status == ValidationStatus.IN_PROGRESS.value assert self.relocation_validation_attempt.status == ValidationStatus.TIMEOUT.value @@ -903,6 +1125,7 @@ def test_timeout_starts_new_validation_attempt( def test_failure_starts_new_validation_attempt( self, validating_start_mock: Mock, + fake_message_builder: Mock, fake_cloudbuild_client: FakeCloudBuildClient, ): for stat in { @@ -911,16 +1134,18 @@ def test_failure_starts_new_validation_attempt( Build.Status.CANCELLED, }: self.mock_cloudbuild_client(fake_cloudbuild_client, Build.Status(stat)) + self.mock_message_builder(fake_message_builder) validating_start_mock.call_count = 0 validating_poll(self.uuid, self.relocation_validation_attempt.build_id) + assert fake_cloudbuild_client.get_build.call_count == 1 + assert fake_message_builder.call_count == 0 + assert validating_start_mock.call_count == 1 + self.relocation.refresh_from_db() self.relocation_validation.refresh_from_db() self.relocation_validation_attempt.refresh_from_db() - - assert validating_start_mock.call_count == 1 - assert fake_cloudbuild_client.get_build.call_count == 1 assert self.relocation.latest_task == "VALIDATING_START" assert self.relocation_validation.status == ValidationStatus.IN_PROGRESS.value assert self.relocation_validation_attempt.status == ValidationStatus.FAILURE.value @@ -929,6 +1154,7 @@ def test_failure_starts_new_validation_attempt( def test_in_progress_retries_poll( self, validating_poll_mock: Mock, + fake_message_builder: Mock, fake_cloudbuild_client: FakeCloudBuildClient, ): for stat in { @@ -937,16 +1163,18 @@ def test_in_progress_retries_poll( Build.Status.WORKING, }: self.mock_cloudbuild_client(fake_cloudbuild_client, Build.Status(stat)) + self.mock_message_builder(fake_message_builder) validating_poll_mock.call_count = 0 validating_poll(self.uuid, self.relocation_validation_attempt.build_id) + assert fake_cloudbuild_client.get_build.call_count == 1 + assert fake_message_builder.call_count == 0 + assert validating_poll_mock.call_count == 1 + self.relocation.refresh_from_db() self.relocation_validation.refresh_from_db() self.relocation_validation_attempt.refresh_from_db() - - assert validating_poll_mock.call_count == 1 - assert fake_cloudbuild_client.get_build.call_count == 1 assert self.relocation.latest_task == "VALIDATING_POLL" assert self.relocation_validation.status == ValidationStatus.IN_PROGRESS.value assert self.relocation_validation_attempt.status == ValidationStatus.IN_PROGRESS.value @@ -957,34 +1185,57 @@ def test_in_progress_retries_poll( == 1 ) + @patch("sentry.tasks.relocation.validating_poll.apply_async") def test_retry_if_attempts_left( self, + validating_poll_mock: Mock, + fake_message_builder: Mock, fake_cloudbuild_client: FakeCloudBuildClient, ): + self.mock_cloudbuild_client(fake_cloudbuild_client, Build.Status(Build.Status.QUEUED)) + self.mock_message_builder(fake_message_builder) + fake_cloudbuild_client.get_build.side_effect = Exception("Test") + # An exception being raised will trigger a retry in celery. with pytest.raises(Exception): - self.mock_cloudbuild_client(fake_cloudbuild_client, Build.Status(Build.Status.QUEUED)) - fake_cloudbuild_client.get_build.side_effect = Exception("Test") - validating_poll(self.uuid, self.relocation_validation_attempt.build_id) + assert fake_cloudbuild_client.get_build.call_count == 1 + assert fake_message_builder.call_count == 0 + assert validating_poll_mock.call_count == 0 + relocation = Relocation.objects.get(uuid=self.uuid) assert relocation.status == Relocation.Status.IN_PROGRESS.value assert not relocation.failure_reason + @patch("sentry.tasks.relocation.validating_poll.apply_async") def test_fail_if_no_attempts_left( self, + validating_poll_mock: Mock, + fake_message_builder: Mock, fake_cloudbuild_client: FakeCloudBuildClient, ): self.relocation.latest_task = "VALIDATING_POLL" self.relocation.latest_task_attempts = MAX_VALIDATION_POLLS self.relocation.save() + self.mock_cloudbuild_client(fake_cloudbuild_client, Build.Status(Build.Status.QUEUED)) fake_cloudbuild_client.get_build.side_effect = Exception("Test") + self.mock_message_builder(fake_message_builder) with pytest.raises(Exception): validating_poll(self.uuid, self.relocation_validation_attempt.build_id) + assert fake_cloudbuild_client.get_build.call_count == 1 + + assert fake_message_builder.call_count == 1 + assert fake_message_builder.call_args.kwargs["type"] == "relocation.failed" + fake_message_builder.return_value.send_async.assert_called_once_with( + to=[self.owner.email, self.superuser.email] + ) + + assert validating_poll_mock.call_count == 0 + relocation = Relocation.objects.get(uuid=self.uuid) assert relocation.status == Relocation.Status.FAILURE.value assert relocation.failure_reason == ERR_VALIDATING_INTERNAL @@ -1013,6 +1264,7 @@ def mock_invalid_finding(storage: Storage, uuid: str): ) +@patch("sentry.utils.relocation.MessageBuilder") @patch("sentry.tasks.relocation.importing.delay") @region_silo_test class ValidatingCompleteTest(RelocationTaskTestCase): @@ -1055,50 +1307,81 @@ def setUp(self): for file in files: self.storage.save(f"relocations/runs/{self.uuid}/findings/{file}", BytesIO(b"[]")) - def test_valid(self, importing_mock: Mock): + def test_valid( + self, + importing_mock: Mock, + fake_message_builder: Mock, + ): + self.mock_message_builder(fake_message_builder) + validating_complete(self.uuid, self.relocation_validation_attempt.build_id) + assert fake_message_builder.call_count == 0 + assert importing_mock.call_count == 1 + self.relocation.refresh_from_db() self.relocation_validation.refresh_from_db() self.relocation_validation_attempt.refresh_from_db() - assert self.relocation.latest_task == "VALIDATING_COMPLETE" assert self.relocation.step == Relocation.Step.IMPORTING.value assert self.relocation_validation.status == ValidationStatus.VALID.value assert self.relocation_validation_attempt.status == ValidationStatus.VALID.value - assert importing_mock.call_count == 1 - def test_invalid(self, importing_mock: Mock): + def test_invalid( + self, + importing_mock: Mock, + fake_message_builder: Mock, + ): + self.mock_message_builder(fake_message_builder) mock_invalid_finding(self.storage, self.uuid) validating_complete(self.uuid, self.relocation_validation_attempt.build_id) + assert fake_message_builder.call_count == 1 + assert fake_message_builder.call_args.kwargs["type"] == "relocation.failed" + fake_message_builder.return_value.send_async.assert_called_once_with( + to=[self.owner.email, self.superuser.email] + ) + + assert importing_mock.call_count == 0 + self.relocation.refresh_from_db() self.relocation_validation.refresh_from_db() self.relocation_validation_attempt.refresh_from_db() - assert self.relocation.latest_task == "VALIDATING_COMPLETE" assert self.relocation.step == Relocation.Step.VALIDATING.value assert self.relocation.failure_reason is not None assert self.relocation_validation.status == ValidationStatus.INVALID.value assert self.relocation_validation_attempt.status == ValidationStatus.INVALID.value + + def test_retry_if_attempts_left( + self, + importing_mock: Mock, + fake_message_builder: Mock, + ): + self.mock_message_builder(fake_message_builder) + self.storage.save( + f"relocations/runs/{self.uuid}/findings/null.json", + BytesIO(b"invalid-json"), + ) + + assert fake_message_builder.call_count == 0 assert importing_mock.call_count == 0 - def test_retry_if_attempts_left(self, _: Mock): # An exception being raised will trigger a retry in celery. with pytest.raises(Exception): - self.storage.save( - f"relocations/runs/{self.uuid}/findings/null.json", - BytesIO(b"invalid-json"), - ) - validating_complete(self.uuid, self.relocation_validation_attempt.build_id) relocation = Relocation.objects.get(uuid=self.uuid) assert relocation.status == Relocation.Status.IN_PROGRESS.value assert not relocation.failure_reason - def test_fail_if_no_attempts_left(self, _: Mock): + def test_fail_if_no_attempts_left( + self, + importing_mock: Mock, + fake_message_builder: Mock, + ): + self.mock_message_builder(fake_message_builder) self.relocation.latest_task = "VALIDATING_COMPLETE" self.relocation.latest_task_attempts = MAX_FAST_TASK_RETRIES self.relocation.save() @@ -1109,6 +1392,14 @@ def test_fail_if_no_attempts_left(self, _: Mock): with pytest.raises(Exception): validating_complete(self.uuid, self.relocation_validation_attempt.build_id) + assert fake_message_builder.call_count == 1 + assert fake_message_builder.call_args.kwargs["type"] == "relocation.failed" + fake_message_builder.return_value.send_async.assert_called_once_with( + to=[self.owner.email, self.superuser.email] + ) + + assert importing_mock.call_count == 0 + relocation = Relocation.objects.get(uuid=self.uuid) assert relocation.status == Relocation.Status.FAILURE.value assert relocation.failure_reason == ERR_VALIDATING_INTERNAL @@ -1249,7 +1540,7 @@ def test_fail_if_no_attempts_left(self, completed_mock: Mock): "sentry.tasks.relocation.CloudBuildClient", new_callable=lambda: FakeCloudBuildClient, ) -@patch("sentry.utils.email.MessageBuilder.send_async") +@patch("sentry.utils.relocation.MessageBuilder") @region_silo_test class EndToEndTest(RelocationTaskTestCase, TransactionTestCase): def setUp(self): @@ -1275,17 +1566,29 @@ def setUp(self): def test_valid_no_retries( self, - send_async_mock: Mock, + fake_message_builder: Mock, fake_cloudbuild_client: FakeCloudBuildClient, fake_kms_client: FakeKeyManagementServiceClient, ): self.mock_cloudbuild_client(fake_cloudbuild_client, Build.Status(Build.Status.SUCCESS)) self.mock_kms_client(fake_kms_client) + self.mock_message_builder(fake_message_builder) org_count = Organization.objects.filter(slug__startswith="testing").count() with self.tasks(): uploading_complete(self.relocation.uuid) + assert fake_cloudbuild_client.create_build.call_count == 1 + assert fake_cloudbuild_client.get_build.call_count == 1 + + assert fake_kms_client.asymmetric_decrypt.call_count == 2 + + assert fake_message_builder.call_count == 1 + assert fake_message_builder.call_args.kwargs["type"] == "relocation.started" + fake_message_builder.return_value.send_async.assert_called_once_with( + to=[self.owner.email, self.superuser.email] + ) + relocation = Relocation.objects.get(uuid=self.uuid) assert relocation.status == Relocation.Status.SUCCESS.value assert not relocation.failure_reason @@ -1311,29 +1614,35 @@ def test_valid_no_retries( "sentry.useremail", ] - assert send_async_mock.call_count == 1 - def test_invalid_no_retries( self, - send_async_mock: Mock, + fake_message_builder: Mock, fake_cloudbuild_client: FakeCloudBuildClient, fake_kms_client: FakeKeyManagementServiceClient, ): self.mock_cloudbuild_client(fake_cloudbuild_client, Build.Status(Build.Status.SUCCESS)) self.mock_kms_client(fake_kms_client) + self.mock_message_builder(fake_message_builder) mock_invalid_finding(self.storage, self.uuid) org_count = Organization.objects.filter(slug__startswith="testing").count() with self.tasks(): uploading_complete(self.relocation.uuid) + assert fake_cloudbuild_client.create_build.call_count == 1 + assert fake_cloudbuild_client.get_build.call_count == 1 + + assert fake_kms_client.asymmetric_decrypt.call_count == 1 + + assert fake_message_builder.call_count == 2 + assert fake_message_builder.call_args_list[0].kwargs["type"] == "relocation.failed" + assert fake_message_builder.call_args_list[1].kwargs["type"] == "relocation.started" + relocation = Relocation.objects.get(uuid=self.uuid) assert relocation.status == Relocation.Status.FAILURE.value assert relocation.failure_reason assert Organization.objects.filter(slug__startswith="testing").count() == org_count - assert send_async_mock.call_count == 1 - # TODO(getsentry/team-ospo#190): We should add "max retry" tests as well, but these are quite # hard to mock in celery at the moment. We may need to use the mock sync celery test scheduler, # rather than the "self.tasks()" approach above, to accomplish this. diff --git a/tests/sentry/utils/test_relocation.py b/tests/sentry/utils/test_relocation.py index ac9ed65b55d384..b013c3215cab3b 100644 --- a/tests/sentry/utils/test_relocation.py +++ b/tests/sentry/utils/test_relocation.py @@ -1,3 +1,4 @@ +from unittest.mock import MagicMock, Mock, patch from uuid import uuid4 import pytest @@ -29,18 +30,28 @@ def setUp(self): ) self.uuid = self.relocation.uuid + def mock_message_builder(self, fake_message_builder: Mock): + fake_message_builder.return_value.send_async.return_value = MagicMock() + +@patch("sentry.utils.relocation.MessageBuilder") class RelocationStartTestCase(RelocationUtilsTestCase): - def test_bad_relocation_not_found(self): + def test_bad_relocation_not_found(self, fake_message_builder: Mock): + self.mock_message_builder(fake_message_builder) + uuid = uuid4().hex (relocation, attempts_left) = start_relocation_task( uuid, Relocation.Step.UPLOADING, OrderedTask.UPLOADING_COMPLETE, 3 ) + assert fake_message_builder.call_count == 0 + assert relocation is None assert not attempts_left - def test_bad_relocation_completed(self): + def test_bad_relocation_completed(self, fake_message_builder: Mock): + self.mock_message_builder(fake_message_builder) + self.relocation.status = Relocation.Status.FAILURE.value self.relocation.save() @@ -48,20 +59,32 @@ def test_bad_relocation_completed(self): self.uuid, Relocation.Step.UPLOADING, OrderedTask.UPLOADING_COMPLETE, 3 ) + assert fake_message_builder.call_count == 0 + assert relocation is None assert not attempts_left assert Relocation.objects.get(uuid=self.uuid).status == Relocation.Status.FAILURE.value - def test_bad_unknown_task(self): + def test_bad_unknown_task(self, fake_message_builder: Mock): + self.mock_message_builder(fake_message_builder) + (relocation, attempts_left) = start_relocation_task( self.uuid, Relocation.Step.UPLOADING, OrderedTask.NONE, 3 ) + assert fake_message_builder.call_count == 1 + assert fake_message_builder.call_args.kwargs["type"] == "relocation.failed" + fake_message_builder.return_value.send_async.assert_called_once_with( + to=[self.owner.email, self.superuser.email] + ) + assert relocation is None assert not attempts_left assert Relocation.objects.get(uuid=self.uuid).status == Relocation.Status.FAILURE.value - def test_bad_task_out_of_order(self): + def test_bad_task_out_of_order(self, fake_message_builder: Mock): + self.mock_message_builder(fake_message_builder) + self.relocation.latest_task = OrderedTask.PREPROCESSING_SCAN.name self.relocation.save() @@ -69,15 +92,25 @@ def test_bad_task_out_of_order(self): self.uuid, Relocation.Step.UPLOADING, OrderedTask.UPLOADING_COMPLETE, 3 ) + assert fake_message_builder.call_count == 1 + assert fake_message_builder.call_args.kwargs["type"] == "relocation.failed" + fake_message_builder.return_value.send_async.assert_called_once_with( + to=[self.owner.email, self.superuser.email] + ) + assert relocation is None assert not attempts_left assert Relocation.objects.get(uuid=self.uuid).status == Relocation.Status.FAILURE.value - def test_good_first_task(self): + def test_good_first_task(self, fake_message_builder: Mock): + self.mock_message_builder(fake_message_builder) + (relocation, attempts_left) = start_relocation_task( self.uuid, Relocation.Step.UPLOADING, OrderedTask.UPLOADING_COMPLETE, 3 ) + assert fake_message_builder.call_count == 0 + assert relocation is not None assert attempts_left == 2 @@ -86,7 +119,9 @@ def test_good_first_task(self): assert relocation.step == Relocation.Step.UPLOADING.value assert relocation.status != Relocation.Status.FAILURE.value - def test_good_next_task(self): + def test_good_next_task(self, fake_message_builder: Mock): + self.mock_message_builder(fake_message_builder) + self.relocation.latest_task = OrderedTask.UPLOADING_COMPLETE.name self.relocation.save() @@ -96,6 +131,8 @@ def test_good_next_task(self): self.uuid, Relocation.Step.PREPROCESSING, OrderedTask.PREPROCESSING_SCAN, 3 ) + assert fake_message_builder.call_count == 0 + assert relocation is not None assert attempts_left == 2 @@ -105,31 +142,55 @@ def test_good_next_task(self): assert relocation.status != Relocation.Status.FAILURE.value +@patch("sentry.utils.relocation.MessageBuilder") class RelocationFailTestCase(RelocationUtilsTestCase): - def test_no_reason(self): + def test_no_reason(self, fake_message_builder: Mock): + self.mock_message_builder(fake_message_builder) + fail_relocation(self.relocation, OrderedTask.UPLOADING_COMPLETE) + assert fake_message_builder.call_count == 1 + assert fake_message_builder.call_args.kwargs["type"] == "relocation.failed" + fake_message_builder.return_value.send_async.assert_called_once_with( + to=[self.owner.email, self.superuser.email] + ) + relocation = Relocation.objects.get(uuid=self.uuid) assert relocation.status == Relocation.Status.FAILURE.value assert not relocation.failure_reason - def test_with_reason(self): + def test_with_reason(self, fake_message_builder: Mock): + self.mock_message_builder(fake_message_builder) + fail_relocation(self.relocation, OrderedTask.UPLOADING_COMPLETE, "foo") + assert fake_message_builder.call_count == 1 + assert fake_message_builder.call_args.kwargs["type"] == "relocation.failed" + fake_message_builder.return_value.send_async.assert_called_once_with( + to=[self.owner.email, self.superuser.email] + ) + relocation = Relocation.objects.get(uuid=self.uuid) assert relocation.status == Relocation.Status.FAILURE.value assert relocation.failure_reason == "foo" +@patch("sentry.utils.relocation.MessageBuilder") class RelocationRetryOrFailTestCase(RelocationUtilsTestCase): - def test_no_reason_attempts_left(self): + def test_no_reason_attempts_left(self, fake_message_builder: Mock): + self.mock_message_builder(fake_message_builder) + with pytest.raises(ValueError): with retry_task_or_fail_relocation(self.relocation, OrderedTask.UPLOADING_COMPLETE, 3): raise ValueError("Some sort of failure") + assert fake_message_builder.call_count == 0 + assert Relocation.objects.get(uuid=self.uuid).status == Relocation.Status.IN_PROGRESS.value - def test_no_reason_last_attempt(self): + def test_no_reason_last_attempt(self, fake_message_builder: Mock): + self.mock_message_builder(fake_message_builder) + # Wrap in `try/except` to make mypy happy. try: with retry_task_or_fail_relocation(self.relocation, OrderedTask.UPLOADING_COMPLETE, 0): @@ -137,21 +198,33 @@ def test_no_reason_last_attempt(self): except Exception: pass + assert fake_message_builder.call_count == 1 + assert fake_message_builder.call_args.kwargs["type"] == "relocation.failed" + fake_message_builder.return_value.send_async.assert_called_once_with( + to=[self.owner.email, self.superuser.email] + ) + assert Relocation.objects.get(uuid=self.uuid).status == Relocation.Status.FAILURE.value - def test_with_reason_attempts_left(self): + def test_with_reason_attempts_left(self, fake_message_builder: Mock): + self.mock_message_builder(fake_message_builder) + with pytest.raises(ValueError): with retry_task_or_fail_relocation( self.relocation, OrderedTask.UPLOADING_COMPLETE, 3, "foo" ): raise ValueError("Some sort of failure") + assert fake_message_builder.call_count == 0 + relocation = Relocation.objects.get(uuid=self.uuid) assert relocation is not None assert relocation.status == Relocation.Status.IN_PROGRESS.value assert not relocation.failure_reason - def test_with_reason_last_attempt(self): + def test_with_reason_last_attempt(self, fake_message_builder: Mock): + self.mock_message_builder(fake_message_builder) + # Wrap in `try/except` to make mypy happy. try: with retry_task_or_fail_relocation( @@ -161,6 +234,12 @@ def test_with_reason_last_attempt(self): except Exception: pass + assert fake_message_builder.call_count == 1 + assert fake_message_builder.call_args.kwargs["type"] == "relocation.failed" + fake_message_builder.return_value.send_async.assert_called_once_with( + to=[self.owner.email, self.superuser.email] + ) + relocation = Relocation.objects.get(uuid=self.uuid) assert relocation is not None assert relocation.status == Relocation.Status.FAILURE.value