From 24fa5e0dbe6a5f232e9075c66d22ea32f9251296 Mon Sep 17 00:00:00 2001 From: Iurii Purisev <92510590+purisev@users.noreply.github.com> Date: Thu, 3 Sep 2026 04:02:44 +0000 Subject: [PATCH] fix(jira): link tickets by their public address ticket_url was always built from the host the provider connects to. When that host is internal - a cluster address, a proxy shim - every link Keep hands back is one nobody can open. Both providers already declare ticket_creation_url and never read it. Read it now: the public new-issue link gives the public base, and the connection host stays the fallback, so nothing changes for anyone who leaves the field empty. The link is parsed as a url rather than matched against one spelling of it, because Jira hands out the create form in several: CreateIssue.jspa, CreateIssue!default.jspa, either of them behind a context path and carrying a ?pid= query. A plain base url works too, with or without a scheme. A value that names no host at all keeps the links on the connection host instead of building a broken one. Signed-off-by: Iurii Purisev <92510590+purisev@users.noreply.github.com> --- .../documentation/jira-on-prem-provider.mdx | 17 +- .../providers/documentation/jira-provider.mdx | 16 ++ .../providers/jira-snippet-autogenerated.mdx | 2 +- .../jiraonprem-snippet-autogenerated.mdx | 2 +- keep/providers/jira_provider/jira_provider.py | 44 ++++- .../jiraonprem_provider.py | 44 ++++- tests/test_jira_provider.py | 174 ++++++++++++++++++ 7 files changed, 288 insertions(+), 11 deletions(-) diff --git a/docs/providers/documentation/jira-on-prem-provider.mdx b/docs/providers/documentation/jira-on-prem-provider.mdx index 3648b228da..162789a47f 100644 --- a/docs/providers/documentation/jira-on-prem-provider.mdx +++ b/docs/providers/documentation/jira-on-prem-provider.mdx @@ -7,4 +7,19 @@ import AutoGeneratedSnippet from '/snippets/providers/jiraonprem-snippet-autogen This is on-prem Jira provider documentation, for regular please check [Jira Provider](./jira-provider.md). - \ No newline at end of file + +## Public ticket links + +Keep returns a `ticket_url` for every issue it creates or updates, built from the host the provider +connects to. That host is not always one a person can open - Jira may sit behind a proxy, or Keep +may reach it over a cluster-internal address. + +Set `ticket_creation_url` to the public new-issue link and the returned links point at that host +instead. Any of the shapes Jira hands out is understood: + +- `https://jira.company.com/secure/CreateIssue.jspa` +- `https://jira.company.com/secure/CreateIssue!default.jspa?pid=10000&issuetype=1` +- `https://jira.company.com/jira/secure/CreateIssue.jspa`, Jira served under a context path +- `https://jira.company.com`, a plain base URL, with or without a scheme + +Leave the field empty and the links come from the host the provider connects to. diff --git a/docs/providers/documentation/jira-provider.mdx b/docs/providers/documentation/jira-provider.mdx index 33684c2ec1..c6886bc349 100644 --- a/docs/providers/documentation/jira-provider.mdx +++ b/docs/providers/documentation/jira-provider.mdx @@ -236,6 +236,22 @@ with: - Infrastructure ``` +## Public ticket links + +Keep returns a `ticket_url` for every issue it creates or updates, built from the host the provider +connects to. That host is not always one a person can open - Jira may sit behind a proxy, or Keep +may reach it over a cluster-internal address. + +Set `ticket_creation_url` to the public new-issue link and the returned links point at that host +instead. Any of the shapes Jira hands out is understood: + +- `https://company.atlassian.net/secure/CreateIssue.jspa` +- `https://company.atlassian.net/secure/CreateIssue!default.jspa?pid=10000&issuetype=1` +- `https://company.atlassian.net/jira/secure/CreateIssue.jspa`, Jira served under a context path +- `https://company.atlassian.net`, a plain base URL, with or without a scheme + +Leave the field empty and the links come from the host the provider connects to. + ## Notes ## Useful Links diff --git a/docs/snippets/providers/jira-snippet-autogenerated.mdx b/docs/snippets/providers/jira-snippet-autogenerated.mdx index 2375039b55..fb46921b0e 100644 --- a/docs/snippets/providers/jira-snippet-autogenerated.mdx +++ b/docs/snippets/providers/jira-snippet-autogenerated.mdx @@ -6,7 +6,7 @@ This provider requires authentication. - **email**: Atlassian Jira Email (required: True, sensitive: False) - **api_token**: Atlassian Jira API Token (required: True, sensitive: True) - **host**: Atlassian Jira Host (required: True, sensitive: False) -- **ticket_creation_url**: URL for creating new tickets (optional, will use default if not provided) (required: False, sensitive: False) +- **ticket_creation_url**: URL for creating new tickets (optional, will use default if not provided), also used as the public base for the ticket links Keep returns (required: False, sensitive: False) Certain scopes may be required to perform specific actions or queries via the provider. Below is a summary of relevant scopes and their use cases: - **BROWSE_PROJECTS**: Browse Jira Projects (mandatory) diff --git a/docs/snippets/providers/jiraonprem-snippet-autogenerated.mdx b/docs/snippets/providers/jiraonprem-snippet-autogenerated.mdx index 16bed6ded8..e71738d973 100644 --- a/docs/snippets/providers/jiraonprem-snippet-autogenerated.mdx +++ b/docs/snippets/providers/jiraonprem-snippet-autogenerated.mdx @@ -5,7 +5,7 @@ Do not edit it manually, as it will be overwritten */} This provider requires authentication. - **host**: Jira Host (required: True, sensitive: False) - **personal_access_token**: Jira PAT (required: True, sensitive: True) -- **ticket_creation_url**: URL for creating new tickets (required: False, sensitive: False) +- **ticket_creation_url**: URL for creating new tickets, also used as the public base for the ticket links Keep returns (required: False, sensitive: False) - **verify**: Verify the Jira server's TLS certificate (required: False, sensitive: False) Certain scopes may be required to perform specific actions or queries via the provider. Below is a summary of relevant scopes and their use cases: diff --git a/keep/providers/jira_provider/jira_provider.py b/keep/providers/jira_provider/jira_provider.py index 3b678794c3..db839d0cac 100644 --- a/keep/providers/jira_provider/jira_provider.py +++ b/keep/providers/jira_provider/jira_provider.py @@ -5,7 +5,7 @@ import dataclasses import json from typing import List, Optional -from urllib.parse import urlencode, urljoin +from urllib.parse import urlencode, urljoin, urlsplit, urlunsplit import pydantic import requests @@ -53,7 +53,7 @@ class JiraProviderAuthConfig: ticket_creation_url: str = dataclasses.field( metadata={ "required": False, - "description": "URL for creating new tickets (optional, will use default if not provided)", + "description": "URL for creating new tickets (optional, will use default if not provided), also used as the public base for the ticket links Keep returns", "sensitive": False, "hint": "https://keephq.atlassian.net/secure/CreateIssue.jspa", }, @@ -190,6 +190,42 @@ def jira_host(self) -> str: self._host = host return self._host + @property + def browse_host(self) -> str: + """Base url for the links a human clicks. + + The client sometimes has to connect through an internal address - a cluster + shim, a reverse proxy - and a link built from that address is unreachable for + anyone outside. ticket_creation_url carries the public address, so the base + comes from there, with the connection host standing in when it is unset. + """ + configured = (self.authentication_config.ticket_creation_url or "").strip() + if not configured: + return self.jira_host + + parts = urlsplit(configured) + if not parts.scheme or not parts.netloc: + # urlsplit sees a host only behind a scheme, so a bare one gets https; + # a value naming no host keeps an empty netloc either way + parts = urlsplit(f"https://{configured}") + # without a host there is nothing to build a link from, and the connection + # host is at least a link that opens + if not parts.netloc or any(char.isspace() for char in parts.netloc): + return self.jira_host + + # the field points at the new-issue form, and Jira spells that form in more + # than one way - CreateIssue.jspa, CreateIssue!default.jspa, either of them + # behind a context path and carrying a ?pid= - so drop a trailing .jspa page + # together with the /secure/ holding it, and keep whatever is left as the base + segments = [segment for segment in parts.path.split("/") if segment] + if segments and segments[-1].lower().endswith(".jspa"): + segments.pop() + if segments and segments[-1].lower() == "secure": + segments.pop() + + path = "/" + "/".join(segments) if segments else "" + return urlunsplit((parts.scheme, parts.netloc, path, "", "")) + def dispose(self): """ No need to dispose of anything, so just do nothing. @@ -596,7 +632,7 @@ def _notify( issue_key = self._extract_issue_key_from_issue_id(issue_id) - result["ticket_url"] = f"{self.jira_host}/browse/{issue_key}" + result["ticket_url"] = f"{self.browse_host}/browse/{issue_key}" # Apply transition if requested if transition_to: @@ -626,7 +662,7 @@ def _notify( custom_fields=custom_fields, **kwargs, ) - result["ticket_url"] = f"{self.jira_host}/browse/{result['issue']['key']}" + result["ticket_url"] = f"{self.browse_host}/browse/{result['issue']['key']}" # Apply transition if requested (on newly created issue) if transition_to: diff --git a/keep/providers/jiraonprem_provider/jiraonprem_provider.py b/keep/providers/jiraonprem_provider/jiraonprem_provider.py index febccca89e..4ab11a39c5 100644 --- a/keep/providers/jiraonprem_provider/jiraonprem_provider.py +++ b/keep/providers/jiraonprem_provider/jiraonprem_provider.py @@ -5,7 +5,7 @@ import dataclasses import json from typing import List -from urllib.parse import urlencode, urljoin +from urllib.parse import urlencode, urljoin, urlsplit, urlunsplit import pydantic import requests @@ -43,7 +43,7 @@ class JiraonpremProviderAuthConfig: ticket_creation_url: str = dataclasses.field( metadata={ "required": False, - "description": "URL for creating new tickets", + "description": "URL for creating new tickets, also used as the public base for the ticket links Keep returns", "sensitive": False, "hint": "https://jira.onprem.com/secure/CreateIssue.jspa", }, @@ -204,6 +204,42 @@ def jira_host(self): except Exception: return self.authentication_config.host + @property + def browse_host(self) -> str: + """Base url for the links a human clicks. + + The client sometimes has to connect through an internal address - a cluster + shim, a reverse proxy - and a link built from that address is unreachable for + anyone outside. ticket_creation_url carries the public address, so the base + comes from there, with the connection host standing in when it is unset. + """ + configured = (self.authentication_config.ticket_creation_url or "").strip() + if not configured: + return self.jira_host + + parts = urlsplit(configured) + if not parts.scheme or not parts.netloc: + # urlsplit sees a host only behind a scheme, so a bare one gets https; + # a value naming no host keeps an empty netloc either way + parts = urlsplit(f"https://{configured}") + # without a host there is nothing to build a link from, and the connection + # host is at least a link that opens + if not parts.netloc or any(char.isspace() for char in parts.netloc): + return self.jira_host + + # the field points at the new-issue form, and Jira spells that form in more + # than one way - CreateIssue.jspa, CreateIssue!default.jspa, either of them + # behind a context path and carrying a ?pid= - so drop a trailing .jspa page + # together with the /secure/ holding it, and keep whatever is left as the base + segments = [segment for segment in parts.path.split("/") if segment] + if segments and segments[-1].lower().endswith(".jspa"): + segments.pop() + if segments and segments[-1].lower() == "secure": + segments.pop() + + path = "/" + "/".join(segments) if segments else "" + return urlunsplit((parts.scheme, parts.netloc, path, "", "")) + def dispose(self): """ No need to dispose of anything, so just do nothing. @@ -554,7 +590,7 @@ def _notify( issue_key = self._extract_issue_key_from_issue_id(issue_id) - result["ticket_url"] = f"{self.jira_host}/browse/{issue_key}" + result["ticket_url"] = f"{self.browse_host}/browse/{issue_key}" self.logger.info("Updated a jira issue: " + str(result)) return result @@ -577,7 +613,7 @@ def _notify( priority=priority, **kwargs, ) - result["ticket_url"] = f"{self.jira_host}/browse/{result['issue']['key']}" + result["ticket_url"] = f"{self.browse_host}/browse/{result['issue']['key']}" self.logger.info("Notified jira!") return result diff --git a/tests/test_jira_provider.py b/tests/test_jira_provider.py index 19ba15e78f..ca6007152f 100644 --- a/tests/test_jira_provider.py +++ b/tests/test_jira_provider.py @@ -355,3 +355,177 @@ def test_notify_with_string_kwargs_handling(self, jira_provider): # If we get here without the "string indices must be integers" error, the fix worked assert result is not None + + def test_browse_host_defaults_to_the_connection_host( + self, jira_provider, jiraonprem_provider + ): + """With ticket_creation_url unset the links come from the connection host""" + assert jira_provider.browse_host == jira_provider.jira_host + assert jiraonprem_provider.browse_host == jiraonprem_provider.jira_host + + @staticmethod + def _jiraonprem_with_creation_url(context_manager, ticket_creation_url): + return JiraonpremProvider( + context_manager, + "test_jiraonprem", + ProviderConfig( + description="Test Jira On-Prem Provider", + authentication={ + "host": "https://jira.internal.svc", + "personal_access_token": "test_token", + "ticket_creation_url": ticket_creation_url, + }, + ), + ) + + @staticmethod + def _jira_with_creation_url(context_manager, ticket_creation_url): + return JiraProvider( + context_manager, + "test_jira", + ProviderConfig( + description="Test Jira Provider", + authentication={ + "email": "test@example.com", + "api_token": "test_token", + "host": "https://jira.internal.svc", + "ticket_creation_url": ticket_creation_url, + }, + ), + ) + + def test_browse_host_reads_the_public_url_from_the_create_form_link( + self, context_manager + ): + """The field holds a link to the new-issue form, the base of it is the public host""" + provider = self._jiraonprem_with_creation_url( + context_manager, "https://jira.company.com/secure/CreateIssue.jspa" + ) + + assert provider.browse_host == "https://jira.company.com" + assert provider.jira_host == "https://jira.internal.svc" + + @pytest.mark.parametrize( + "configured, expected", + [ + # the link the UI hands out + ( + "https://jira.company.com/secure/CreateIssue.jspa", + "https://jira.company.com", + ), + # Jira spells the same form in more than one way, and hangs the project + # and issue type off it as a query string + ( + "https://jira.company.com/secure/CreateIssue!default.jspa?pid=10000&issuetype=1", + "https://jira.company.com", + ), + ( + "https://jira.company.com/secure/CreateIssueDetails!init.jspa", + "https://jira.company.com", + ), + # served under a context path, which the browse link has to keep + ( + "https://company.com/jira/secure/CreateIssue.jspa?pid=10000", + "https://company.com/jira", + ), + # a plain base url, with or without a trailing slash or a scheme + ("https://jira.company.com", "https://jira.company.com"), + ("https://jira.company.com/", "https://jira.company.com"), + ("jira.company.com", "https://jira.company.com"), + ("jira.company.com/secure/CreateIssue.jspa", "https://jira.company.com"), + # a port and a plain-http host survive as they are + ( + "jira.company.com:8443/secure/CreateIssue.jspa", + "https://jira.company.com:8443", + ), + ("https://jira.company.com:8443", "https://jira.company.com:8443"), + ( + "http://jira.company.com/secure/CreateIssue.jspa", + "http://jira.company.com", + ), + # copied out of a text field with whitespace around it + ( + " https://jira.company.com/secure/CreateIssue.jspa ", + "https://jira.company.com", + ), + ], + ) + def test_browse_host_reads_every_shape_of_the_create_form_link( + self, context_manager, configured, expected + ): + """Both providers derive the same public base from the link they are given""" + assert ( + self._jiraonprem_with_creation_url(context_manager, configured).browse_host + == expected + ) + assert ( + self._jira_with_creation_url(context_manager, configured).browse_host + == expected + ) + + @pytest.mark.parametrize( + "configured", + ["", " ", "/secure/CreateIssue.jspa", "/jira/", "not a url at all"], + ) + def test_browse_host_falls_back_when_the_link_names_no_host( + self, context_manager, configured + ): + """A blank, host-less or unreadable value leaves the links on the connection host""" + provider = self._jiraonprem_with_creation_url(context_manager, configured) + + assert provider.browse_host == provider.jira_host + + @patch("requests.put") + @patch("requests.get") + def test_jiraonprem_ticket_url_points_at_the_public_host( + self, mock_get, mock_put, context_manager + ): + """An updated issue is linked by its public address, not the internal one""" + provider = JiraonpremProvider( + context_manager, + "test_jiraonprem", + ProviderConfig( + description="Test Jira On-Prem Provider", + authentication={ + "host": "https://jira.internal.svc", + "personal_access_token": "test_token", + "ticket_creation_url": "https://jira.company.com/secure/CreateIssue.jspa", + }, + ), + ) + + mock_get.return_value.status_code = 200 + mock_get.return_value.json.return_value = {"key": "TEST-123"} + mock_put.return_value.status_code = 204 + + result = provider._notify(issue_id="TEST-123", summary="Test Summary") + + assert result["ticket_url"] == "https://jira.company.com/browse/TEST-123" + + @patch("requests.put") + @patch("requests.get") + def test_jira_cloud_ticket_url_points_at_the_public_host( + self, mock_get, mock_put, context_manager + ): + """The cloud provider builds the link from the same field""" + provider = JiraProvider( + context_manager, + "test_jira", + ProviderConfig( + description="Test Jira Provider", + authentication={ + "email": "test@example.com", + "api_token": "test_token", + "host": "https://jira.internal.svc", + "ticket_creation_url": "https://company.atlassian.net/secure/CreateIssue.jspa", + }, + ), + ) + + mock_get.return_value.status_code = 200 + mock_get.return_value.json.return_value = {"key": "TEST-123"} + mock_put.return_value.status_code = 204 + + result = provider._notify(issue_id="TEST-123", summary="Test Summary") + + assert result["ticket_url"] == "https://company.atlassian.net/browse/TEST-123"