From 77be8d77fcdcf70b59d7f091d523d36bbbcbd8a7 Mon Sep 17 00:00:00 2001 From: tobias-wilfert Date: Tue, 9 Sep 2025 16:40:34 +0200 Subject: [PATCH 1/2] Potential fix --- src/sentry/tempest/tasks.py | 8 ++++++- tests/sentry/tempest/test_tempest.py | 31 ++++++++++++++++++++++++++++ 2 files changed, 38 insertions(+), 1 deletion(-) diff --git a/src/sentry/tempest/tasks.py b/src/sentry/tempest/tasks.py index ec3eda20bdd617..5f4b4ba2bcadc7 100644 --- a/src/sentry/tempest/tasks.py +++ b/src/sentry/tempest/tasks.py @@ -13,6 +13,7 @@ from sentry.taskworker.config import TaskworkerConfig from sentry.taskworker.namespaces import tempest_tasks from sentry.tempest.models import MessageType, TempestCredentials +from sentry.tempest.utils import has_tempest_access logger = logging.getLogger(__name__) @@ -27,7 +28,12 @@ ) def poll_tempest(**kwargs): # FIXME: Once we have more traffic this needs to be done smarter. - for credentials in TempestCredentials.objects.all(): + for credentials in TempestCredentials.objects.select_related("project__organization").all(): + # Bruno wants to keeps the customers credentials around so need to + # skip if the credentials are associated with an org that no longer has tempest access. + if not has_tempest_access(credentials.project.organization): + continue + if credentials.latest_fetched_item_id is None: fetch_latest_item_id.apply_async( kwargs={"credentials_id": credentials.id}, diff --git a/tests/sentry/tempest/test_tempest.py b/tests/sentry/tempest/test_tempest.py index a9a36712e0c4f5..4ddc661a580a39 100644 --- a/tests/sentry/tempest/test_tempest.py +++ b/tests/sentry/tempest/test_tempest.py @@ -238,3 +238,34 @@ def test_poll_tempest_crashes_invalidates_config( # Second call -> should reuse existing ProjectKey and thus not invalidate config poll_tempest_crashes(self.credentials.id) mock_invalidate.assert_not_called() + + @patch("sentry.tempest.tasks.has_tempest_access") + @patch("sentry.tempest.tasks.fetch_latest_item_id") + @patch("sentry.tempest.tasks.poll_tempest_crashes") + def test_poll_tempest_skips_credentials_without_access( + self, mock_poll_crashes: MagicMock, mock_fetch_latest: MagicMock, mock_has_access: MagicMock + ) -> None: + """Test that poll_tempest skips credentials when organization doesn't have tempest access""" + project_with_access = self.create_project() + credentials_with_access = self.create_tempest_credentials(project_with_access) + credentials_with_access.latest_fetched_item_id = "42" + credentials_with_access.save() + + project_without_access = self.create_project() + credentials_without_access = self.create_tempest_credentials(project_without_access) + credentials_without_access.latest_fetched_item_id = "42" + credentials_without_access.save() + + def mock_access_check(organization): + return organization.id == project_with_access.organization.id + + mock_has_access.side_effect = mock_access_check + + poll_tempest() + + assert mock_has_access.call_count == 3 + mock_poll_crashes.apply_async.assert_called_once_with( + kwargs={"credentials_id": credentials_with_access.id}, + headers={"sentry-propagate-traces": False}, + ) + mock_fetch_latest.apply_async.assert_not_called() From eccfd32ff34b49b5cbe99d999b80241c3b0c4f5f Mon Sep 17 00:00:00 2001 From: tobias-wilfert Date: Mon, 15 Sep 2025 11:57:53 +0200 Subject: [PATCH 2/2] Attempt to fix the tests --- tests/sentry/tempest/test_tempest.py | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/tests/sentry/tempest/test_tempest.py b/tests/sentry/tempest/test_tempest.py index 4ddc661a580a39..76bfd0daf34a12 100644 --- a/tests/sentry/tempest/test_tempest.py +++ b/tests/sentry/tempest/test_tempest.py @@ -146,11 +146,13 @@ def test_poll_tempest_crashes_error(self, mock_fetch: MagicMock) -> None: mock_fetch.assert_called_once() assert "Fetching the crashes failed." in cm.output[0] + @patch("sentry.tempest.tasks.has_tempest_access") @patch("sentry.tempest.tasks.fetch_latest_item_id") @patch("sentry.tempest.tasks.poll_tempest_crashes") def test_poll_tempest_no_latest_id( - self, mock_poll_crashes: MagicMock, mock_fetch_latest: MagicMock + self, mock_poll_crashes: MagicMock, mock_fetch_latest: MagicMock, mock_has_access: MagicMock ) -> None: + mock_has_access.return_value = True # Ensure latest_fetched_item_id is None self.credentials.latest_fetched_item_id = None self.credentials.save() @@ -164,11 +166,13 @@ def test_poll_tempest_no_latest_id( ) mock_poll_crashes.apply_async.assert_not_called() + @patch("sentry.tempest.tasks.has_tempest_access") @patch("sentry.tempest.tasks.fetch_latest_item_id") @patch("sentry.tempest.tasks.poll_tempest_crashes") def test_poll_tempest_with_latest_id( - self, mock_poll_crashes: MagicMock, mock_fetch_latest: MagicMock + self, mock_poll_crashes: MagicMock, mock_fetch_latest: MagicMock, mock_has_access: MagicMock ) -> None: + mock_has_access.return_value = True # Set an existing ID self.credentials.latest_fetched_item_id = "42" self.credentials.save() @@ -246,18 +250,20 @@ def test_poll_tempest_skips_credentials_without_access( self, mock_poll_crashes: MagicMock, mock_fetch_latest: MagicMock, mock_has_access: MagicMock ) -> None: """Test that poll_tempest skips credentials when organization doesn't have tempest access""" - project_with_access = self.create_project() + org_with_access = self.create_organization() + project_with_access = self.create_project(organization=org_with_access) credentials_with_access = self.create_tempest_credentials(project_with_access) credentials_with_access.latest_fetched_item_id = "42" credentials_with_access.save() - project_without_access = self.create_project() + org_without_access = self.create_organization() + project_without_access = self.create_project(organization=org_without_access) credentials_without_access = self.create_tempest_credentials(project_without_access) credentials_without_access.latest_fetched_item_id = "42" credentials_without_access.save() def mock_access_check(organization): - return organization.id == project_with_access.organization.id + return organization.id == org_with_access.id mock_has_access.side_effect = mock_access_check