From 621f22fc05a030056a864c690fe82eefd08bcbe0 Mon Sep 17 00:00:00 2001 From: Josema Camacho Date: Tue, 1 Sep 2026 14:45:55 +0200 Subject: [PATCH] fix(api): address Jira issue review feedback --- api/src/backend/api/tests/test_models.py | 118 +++++++++--------- api/src/backend/api/tests/test_views.py | 45 ++++++- api/src/backend/tasks/jobs/integrations.py | 7 +- .../backend/tasks/tests/test_integrations.py | 29 +++++ .../prowler-app-jira-integration.mdx | 2 + 5 files changed, 139 insertions(+), 62 deletions(-) diff --git a/api/src/backend/api/tests/test_models.py b/api/src/backend/api/tests/test_models.py index b5095cd4ea..1574e446e8 100644 --- a/api/src/backend/api/tests/test_models.py +++ b/api/src/backend/api/tests/test_models.py @@ -4,6 +4,7 @@ from uuid import uuid4 import pytest from allauth.socialaccount.models import SocialApp from api.db_router import MainRouter +from api.db_utils import rls_transaction from api.models import ( JiraIssue, ProviderComplianceScore, @@ -534,17 +535,18 @@ class TestJiraIssueModel: self, jira_integration_fixture, aws_provider, findings_fixture ): finding = findings_fixture[0] - issue = JiraIssue.objects.create( - tenant_id=jira_integration_fixture.tenant_id, - integration=jira_integration_fixture, - provider=aws_provider, - finding_uid=finding.uid, - finding_id=finding.id, - issue_id="10001", - issue_key="TEST-1", - issue_url="https://test.atlassian.net/browse/TEST-1", - project_key="TEST", - ) + with rls_transaction(str(jira_integration_fixture.tenant_id)): + issue = JiraIssue.objects.create( + tenant_id=jira_integration_fixture.tenant_id, + integration=jira_integration_fixture, + provider=aws_provider, + finding_uid=finding.uid, + finding_id=finding.id, + issue_id="10001", + issue_key="TEST-1", + issue_url="https://test.atlassian.net/browse/TEST-1", + project_key="TEST", + ) assert issue.is_linked assert not issue.is_done assert issue.issue_status_category is None @@ -553,14 +555,15 @@ class TestJiraIssueModel: self, jira_integration_fixture, aws_provider, findings_fixture ): finding = findings_fixture[0] - issue = JiraIssue.objects.create( - tenant_id=jira_integration_fixture.tenant_id, - integration=jira_integration_fixture, - provider=aws_provider, - finding_uid=finding.uid, - finding_id=finding.id, - delivery_attempt_token=uuid4(), - ) + with rls_transaction(str(jira_integration_fixture.tenant_id)): + issue = JiraIssue.objects.create( + tenant_id=jira_integration_fixture.tenant_id, + integration=jira_integration_fixture, + provider=aws_provider, + finding_uid=finding.uid, + finding_id=finding.id, + delivery_attempt_token=uuid4(), + ) assert not issue.is_linked def test_unique_per_integration_provider_and_finding_uid( @@ -575,29 +578,30 @@ class TestJiraIssueModel: "finding_id": finding.id, "project_key": "TEST", } - JiraIssue.objects.create( - provider=provider, - issue_id="10001", - issue_key="TEST-1", - issue_url="https://test.atlassian.net/browse/TEST-1", - **common, - ) - # Same finding uid on another provider is a different finding - JiraIssue.objects.create( - provider=provider2, - issue_id="10002", - issue_key="TEST-2", - issue_url="https://test.atlassian.net/browse/TEST-2", - **common, - ) - with pytest.raises(IntegrityError), transaction.atomic(): + with rls_transaction(str(jira_integration_fixture.tenant_id)): JiraIssue.objects.create( provider=provider, - issue_id="10003", - issue_key="TEST-3", - issue_url="https://test.atlassian.net/browse/TEST-3", + issue_id="10001", + issue_key="TEST-1", + issue_url="https://test.atlassian.net/browse/TEST-1", **common, ) + # Same finding uid on another provider is a different finding + JiraIssue.objects.create( + provider=provider2, + issue_id="10002", + issue_key="TEST-2", + issue_url="https://test.atlassian.net/browse/TEST-2", + **common, + ) + with pytest.raises(IntegrityError), transaction.atomic(): + JiraIssue.objects.create( + provider=provider, + issue_id="10003", + issue_key="TEST-3", + issue_url="https://test.atlassian.net/browse/TEST-3", + **common, + ) def test_delivery_attempt_token_is_unique_when_present( self, jira_integration_fixture, aws_provider_pair, findings_fixture @@ -610,29 +614,31 @@ class TestJiraIssueModel: "finding_id": findings_fixture[0].id, "delivery_attempt_token": attempt_token, } - JiraIssue.objects.create( - provider=provider, - finding_uid="finding-one", - **common, - ) - - with pytest.raises(IntegrityError), transaction.atomic(): + with rls_transaction(str(jira_integration_fixture.tenant_id)): JiraIssue.objects.create( - provider=provider2, - finding_uid="finding-two", + provider=provider, + finding_uid="finding-one", **common, ) + with pytest.raises(IntegrityError), transaction.atomic(): + JiraIssue.objects.create( + provider=provider2, + finding_uid="finding-two", + **common, + ) + def test_link_fields_are_all_or_none( self, jira_integration_fixture, aws_provider, findings_fixture ): finding = findings_fixture[0] - with pytest.raises(IntegrityError), transaction.atomic(): - JiraIssue.objects.create( - tenant_id=jira_integration_fixture.tenant_id, - integration=jira_integration_fixture, - provider=aws_provider, - finding_uid=finding.uid, - finding_id=finding.id, - issue_id="10001", - ) + with rls_transaction(str(jira_integration_fixture.tenant_id)): + with pytest.raises(IntegrityError), transaction.atomic(): + JiraIssue.objects.create( + tenant_id=jira_integration_fixture.tenant_id, + integration=jira_integration_fixture, + provider=aws_provider, + finding_uid=finding.uid, + finding_id=finding.id, + issue_id="10001", + ) diff --git a/api/src/backend/api/tests/test_views.py b/api/src/backend/api/tests/test_views.py index 37e69e64fe..f86d2eb7d7 100644 --- a/api/src/backend/api/tests/test_views.py +++ b/api/src/backend/api/tests/test_views.py @@ -13599,15 +13599,50 @@ class TestJiraIssueViewSet: ) assert response.status_code == status.HTTP_404_NOT_FOUND - def test_retrieve_other_tenant_returns_404( + def test_other_tenant_issue_is_hidden( self, authenticated_client, jira_issues_fixture, tenants_fixture ): linked, *_ = jira_issues_fixture - JiraIssue.objects.using(MainRouter.admin_db).filter(id=linked.id).update( - tenant_id=tenants_fixture[2].id - ) + other_tenant = tenants_fixture[2] + with rls_transaction(str(other_tenant.id)): + other_provider = Provider.objects.create( + tenant_id=other_tenant.id, + provider=Provider.ProviderChoices.AWS, + uid="999999999999", + alias="other-tenant-provider", + ) + other_integration = Integration.objects.create( + tenant_id=other_tenant.id, + enabled=True, + connected=True, + integration_type=Integration.IntegrationChoices.JIRA, + configuration={"projects": {"OTHER": "Other project"}}, + credentials={ + "domain": "other-tenant", + "user_mail": "other-tenant@example.com", + "api_token": "fake-token", + }, + ) + other_issue = JiraIssue.objects.create( + tenant_id=other_tenant.id, + integration=other_integration, + provider=other_provider, + finding_uid="other-tenant-finding", + finding_id=datetime_to_uuid7(datetime.now(UTC)), + issue_id="20001", + issue_key="OTHER-1", + issue_url="https://other-tenant.atlassian.net/browse/OTHER-1", + project_key="OTHER", + ) + + response = authenticated_client.get(reverse("jiraissue-list")) + assert response.status_code == status.HTTP_200_OK + ids = {item["id"] for item in response.json()["data"]} + assert str(linked.id) in ids + assert str(other_issue.id) not in ids + response = authenticated_client.get( - reverse("jiraissue-detail", kwargs={"pk": linked.id}) + reverse("jiraissue-detail", kwargs={"pk": other_issue.id}) ) assert response.status_code == status.HTTP_404_NOT_FOUND diff --git a/api/src/backend/tasks/jobs/integrations.py b/api/src/backend/tasks/jobs/integrations.py index 04fd74ed25..98ee896bd6 100644 --- a/api/src/backend/tasks/jobs/integrations.py +++ b/api/src/backend/tasks/jobs/integrations.py @@ -544,6 +544,7 @@ def get_tenant_name(tenant_id: str) -> str: # Findings are pre-checked in bounded index lookups however many a dispatch carries. JIRA_DEDUP_CHUNK_SIZE = 500 JIRA_SKIPPED_REPORT_LIMIT = 100 +JIRA_ERROR_REPORT_MAX_LENGTH = 8192 def _load_finding_refs(finding_ids: list[str]) -> dict[str, tuple[str, str]]: @@ -688,6 +689,8 @@ def _apply_jira_issue_status( def _update_latest_jira_finding_id( tenant_id: str, row: JiraIssue, finding_id: str ) -> None: + # Finding IDs are monotonic UUIDv7 values, so ordering reflects scan recency + # and prevents stale dispatches from moving the ledger pointer backward. with rls_transaction(tenant_id, using=MainRouter.default_db): JiraIssue.objects.filter(id=row.id, finding_id__lt=finding_id).update( finding_id=finding_id, @@ -1110,7 +1113,9 @@ def send_findings_to_jira( "skipped_count": skipped_count, } if error_messages: - result["error"] = "; ".join(dict.fromkeys(error_messages)) + result["error"] = "; ".join(dict.fromkeys(error_messages))[ + :JIRA_ERROR_REPORT_MAX_LENGTH + ] if skipped: result["skipped"] = skipped return result diff --git a/api/src/backend/tasks/tests/test_integrations.py b/api/src/backend/tasks/tests/test_integrations.py index 4a207384a3..68f30f2661 100644 --- a/api/src/backend/tasks/tests/test_integrations.py +++ b/api/src/backend/tasks/tests/test_integrations.py @@ -2669,6 +2669,35 @@ class TestJiraIssueDedup: finding_uid__in=[finding.uid for finding in findings] ).exists() + def test_error_summary_is_bounded( + self, + jira_mock, + jira_integration_fixture, + findings_fixture, + monkeypatch, + ): + max_length = 96 + monkeypatch.setattr( + "tasks.jobs.integrations.JIRA_ERROR_REPORT_MAX_LENGTH", max_length + ) + messages = ["First Jira error " * 8, "Second Jira error " * 8] + jira_mock.send_finding.side_effect = None + jira_mock.send_finding.side_effect = [ + JiraCreationResult( + outcome=JiraCreationOutcome.CONFIRMED_REJECTION, + error_message=message, + ) + for message in messages + ] + + result = self._send( + jira_integration_fixture, + jira_mock, + [finding.id for finding in findings_fixture], + ) + + assert result["error"] == "; ".join(messages)[:max_length] + def test_uncertain_initial_send_retains_reservation( self, jira_mock, jira_integration_fixture, findings_fixture ): diff --git a/docs/user-guide/tutorials/prowler-app-jira-integration.mdx b/docs/user-guide/tutorials/prowler-app-jira-integration.mdx index 0bdc34fe98..655eb9f01f 100644 --- a/docs/user-guide/tutorials/prowler-app-jira-integration.mdx +++ b/docs/user-guide/tutorials/prowler-app-jira-integration.mdx @@ -138,6 +138,8 @@ Prowler Cloud always includes the Finding URL. In Prowler Local Server, set `DJA ### Sending a Finding That Already Has a Jira Issue + + Prowler remembers each confirmed Jira issue created for a Finding, keyed by the Finding UID, so sending the same Finding again does not create a duplicate issue: * If the linked issue is still open in Jira, the Finding is skipped and the existing issue key is reported in the task result (`skipped_count`, `skipped`).