From 5a43689eb9608aec92d3545aef83b807caa72e10 Mon Sep 17 00:00:00 2001 From: Leander Rodrigues Date: Fri, 21 Jul 2023 13:17:11 -0400 Subject: [PATCH 1/5] =?UTF-8?q?=E2=99=BB=EF=B8=8F=20Small=20refactors=20fo?= =?UTF-8?q?r=20clarity?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/sentry_plugins/bitbucket/endpoints/webhook.py | 2 +- src/sentry_plugins/bitbucket/urls.py | 4 ++-- src/sentry_plugins/github/urls.py | 4 ++-- src/sentry_plugins/github/webhooks/__init__.py | 8 ++++---- src/sentry_plugins/github/webhooks/integration.py | 2 +- .../github/webhooks/non_integration.py | 2 +- src/sentry_plugins/gitlab/plugin.py | 14 -------------- 7 files changed, 11 insertions(+), 25 deletions(-) diff --git a/src/sentry_plugins/bitbucket/endpoints/webhook.py b/src/sentry_plugins/bitbucket/endpoints/webhook.py index 19332825631773..4524e8199b18f0 100644 --- a/src/sentry_plugins/bitbucket/endpoints/webhook.py +++ b/src/sentry_plugins/bitbucket/endpoints/webhook.py @@ -77,7 +77,7 @@ def __call__(self, organization, event): pass -class BitbucketWebhookEndpoint(View): +class BitbucketPluginWebhookEndpoint(View): _handlers = {"repo:push": PushEventWebhook} def get_handler(self, event_type): diff --git a/src/sentry_plugins/bitbucket/urls.py b/src/sentry_plugins/bitbucket/urls.py index 4d33a7ee3886c7..68c84ac2217990 100644 --- a/src/sentry_plugins/bitbucket/urls.py +++ b/src/sentry_plugins/bitbucket/urls.py @@ -1,10 +1,10 @@ from django.urls import re_path -from .endpoints.webhook import BitbucketWebhookEndpoint +from .endpoints.webhook import BitbucketPluginWebhookEndpoint urlpatterns = [ re_path( r"^organizations/(?P[^\/]+)/webhook/$", - BitbucketWebhookEndpoint.as_view(), + BitbucketPluginWebhookEndpoint.as_view(), ) ] diff --git a/src/sentry_plugins/github/urls.py b/src/sentry_plugins/github/urls.py index 4b22fe4f8c14ee..76ca2c7c54ec57 100644 --- a/src/sentry_plugins/github/urls.py +++ b/src/sentry_plugins/github/urls.py @@ -1,11 +1,11 @@ from django.urls import re_path -from .webhooks import GithubIntegrationsWebhookEndpoint, GithubWebhookEndpoint +from .webhooks import GithubIntegrationsWebhookEndpoint, GithubPluginWebhookEndpoint urlpatterns = [ re_path( r"^organizations/(?P[^\/]+)/webhook/$", - GithubWebhookEndpoint.as_view(), + GithubPluginWebhookEndpoint.as_view(), ), re_path( r"^installations/webhook/$", diff --git a/src/sentry_plugins/github/webhooks/__init__.py b/src/sentry_plugins/github/webhooks/__init__.py index e57fe0b3bfc0fe..e5bf6b2bf8b6f7 100644 --- a/src/sentry_plugins/github/webhooks/__init__.py +++ b/src/sentry_plugins/github/webhooks/__init__.py @@ -1,7 +1,7 @@ -from .integration import GithubIntegrationsWebhookEndpoint -from .non_integration import GithubWebhookEndpoint +from .integration import GithubPluginIntegrationsWebhookEndpoint +from .non_integration import GithubPluginWebhookEndpoint __all__ = ( - "GithubIntegrationsWebhookEndpoint", - "GithubWebhookEndpoint", + "GithubPluginIntegrationsWebhookEndpoint", + "GithubPluginWebhookEndpoint", ) diff --git a/src/sentry_plugins/github/webhooks/integration.py b/src/sentry_plugins/github/webhooks/integration.py index 729a9c99078fdc..d215022eff01f9 100644 --- a/src/sentry_plugins/github/webhooks/integration.py +++ b/src/sentry_plugins/github/webhooks/integration.py @@ -12,7 +12,7 @@ from .events import InstallationEventWebhook, InstallationRepositoryEventWebhook, PushEventWebhook -class GithubIntegrationsWebhookEndpoint(GithubWebhookBase): +class GithubPluginIntegrationsWebhookEndpoint(GithubWebhookBase): _handlers = { "push": PushEventWebhook, "installation": InstallationEventWebhook, diff --git a/src/sentry_plugins/github/webhooks/non_integration.py b/src/sentry_plugins/github/webhooks/non_integration.py index 31131578defb16..5cc73240f3d66e 100644 --- a/src/sentry_plugins/github/webhooks/non_integration.py +++ b/src/sentry_plugins/github/webhooks/non_integration.py @@ -12,7 +12,7 @@ logger = logging.getLogger("sentry.webhooks") -class GithubWebhookEndpoint(GithubWebhookBase): +class GithubPluginWebhookEndpoint(GithubWebhookBase): def get_logging_data(self, organization): return {"organization_id": organization.id} diff --git a/src/sentry_plugins/gitlab/plugin.py b/src/sentry_plugins/gitlab/plugin.py index ce5eb31c384aed..03b9f2cf24fb2a 100644 --- a/src/sentry_plugins/gitlab/plugin.py +++ b/src/sentry_plugins/gitlab/plugin.py @@ -18,20 +18,6 @@ class GitLabPlugin(CorePluginMixin, IssuePlugin2): conf_key = "gitlab" required_field = "gitlab_url" feature_descriptions = [ - FeatureDescription( - """ - Track commits and releases (learn more - [here](https://docs.sentry.io/learn/releases/)) - """, - IntegrationFeatures.COMMITS, - ), - FeatureDescription( - """ - Resolve Sentry issues via GitLab commits and merge requests by - including `Fixes PROJ-ID` in the message - """, - IntegrationFeatures.COMMITS, - ), FeatureDescription( """ Create GitLab issues from Sentry From 43350744455bd7f92aedd5683d67b1db88dcceb3 Mon Sep 17 00:00:00 2001 From: Leander Rodrigues Date: Mon, 31 Jul 2023 16:15:32 -0400 Subject: [PATCH 2/5] =?UTF-8?q?=F0=9F=9A=A7=20wip?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../integrations/classifications.py | 88 +++++++++++++++++++ .../integrations/integration_control.py | 12 +-- .../integrations/parsers/__init__.py | 2 + .../middleware/integrations/parsers/base.py | 3 + .../middleware/integrations/parsers/plugin.py | 7 ++ src/sentry/models/outbox.py | 1 + src/sentry_plugins/github/urls.py | 4 +- 7 files changed, 110 insertions(+), 7 deletions(-) create mode 100644 src/sentry/middleware/integrations/classifications.py create mode 100644 src/sentry/middleware/integrations/parsers/plugin.py diff --git a/src/sentry/middleware/integrations/classifications.py b/src/sentry/middleware/integrations/classifications.py new file mode 100644 index 00000000000000..4aec84c3581a81 --- /dev/null +++ b/src/sentry/middleware/integrations/classifications.py @@ -0,0 +1,88 @@ +import logging +from typing import Callable, Mapping, Type + +from django.http import HttpRequest, HttpResponse + +from sentry.middleware.integrations.parsers.base import BaseRequestParser + +from .parsers import ( + BitbucketRequestParser, + BitbucketServerRequestParser, + GithubEnterpriseRequestParser, + GithubRequestParser, + GitlabRequestParser, + JiraRequestParser, + JiraServerRequestParser, + MsTeamsRequestParser, + PluginRequestParser, + SlackRequestParser, + VstsRequestParser, +) + + +class BaseClassification: + def should_operate(self, request: HttpRequest) -> bool: + """ + Function to determine if an incoming request should be handled by this + classifcation of parsers or not. + """ + return False + + +class IntegrationClassification(BaseClassification): + logger = logging.getLogger(f"{__name__}.integration") + integration_prefix: str = "/extensions/" + """Prefix for all integration requests. See `src/sentry/web/urls.py`""" + setup_suffix: str = "/setup/" + """Suffix for PipelineAdvancerView on installation. See `src/sentry/web/urls.py`""" + + integration_parsers: Mapping[str, Type[BaseRequestParser]] = { + parser.provider: parser + for parser in [ + BitbucketRequestParser, + BitbucketServerRequestParser, + GithubEnterpriseRequestParser, + GithubRequestParser, + GitlabRequestParser, + JiraRequestParser, + JiraServerRequestParser, + MsTeamsRequestParser, + SlackRequestParser, + VstsRequestParser, + ] + } + + def should_operate(self, request: HttpRequest) -> bool: + is_integration = request.path.startswith(self.integration_prefix) + is_not_setup = not request.path.endswith(self.setup_suffix) + return is_integration and is_not_setup + + def get_response(self, request: HttpRequest, response_handler: Callable[[], HttpResponse]): + provider = self._identify_provider(request) + if not provider: + return response_handler(request) + + parser_class = self.integration_parsers.get(provider) + if not parser_class: + self.logger.error( + "integration_control.unknown_provider", + extra={"path": request.path, "provider": provider}, + ) + return response_handler(request) + + parser = parser_class( + request=request, + response_handler=response_handler, + ) + + return parser.get_response() + + +class PluginClassification(BaseClassification): + logger = logging.getLogger(f"{__name__}.plugin") + + def should_operate(self, request: HttpRequest) -> bool: + pass + + def get_response(self) -> HttpResponse: + pass diff --git a/src/sentry/middleware/integrations/integration_control.py b/src/sentry/middleware/integrations/integration_control.py index 1f722e78da4899..bec8ef53e48f30 100644 --- a/src/sentry/middleware/integrations/integration_control.py +++ b/src/sentry/middleware/integrations/integration_control.py @@ -2,7 +2,9 @@ import logging import re -from typing import Mapping, Type +from typing import Callable, Mapping, Type + +from django.http import HttpRequest, HttpResponse from sentry.silo import SiloMode @@ -46,10 +48,10 @@ class IntegrationControlMiddleware: parser.provider: parser for parser in ACTIVE_PARSERS } - def __init__(self, get_response): + def __init__(self, get_response: Callable[[], HttpResponse]): self.get_response = get_response - def _identify_provider(self, request) -> str | None: + def _identify_provider(self, request: HttpRequest) -> str | None: """ Parses the provider out of the request path e.g. `/extensions/slack/commands/` -> `slack` @@ -65,7 +67,7 @@ def _identify_provider(self, request) -> str | None: return None return result.group(1) - def _should_operate(self, request) -> bool: + def _should_operate(self, request: HttpRequest) -> bool: """ Determines whether this middleware will operate or just pass the request along. """ @@ -74,7 +76,7 @@ def _should_operate(self, request) -> bool: is_not_setup = not request.path.endswith(self.setup_suffix) return is_correct_silo and is_integration and is_not_setup - def __call__(self, request): + def __call__(self, request: HttpRequest): if not self._should_operate(request): return self.get_response(request) diff --git a/src/sentry/middleware/integrations/parsers/__init__.py b/src/sentry/middleware/integrations/parsers/__init__.py index 755f746ea29378..d83d155cc542eb 100644 --- a/src/sentry/middleware/integrations/parsers/__init__.py +++ b/src/sentry/middleware/integrations/parsers/__init__.py @@ -6,6 +6,7 @@ from .jira import JiraRequestParser from .jira_server import JiraServerRequestParser from .msteams import MsTeamsRequestParser +from .plugin import PluginRequestParser from .slack import SlackRequestParser from .vsts import VstsRequestParser @@ -18,6 +19,7 @@ "JiraServerRequestParser", "GithubEnterpriseRequestParser", "MsTeamsRequestParser", + "PluginRequestParser", "SlackRequestParser", "VstsRequestParser", ) diff --git a/src/sentry/middleware/integrations/parsers/base.py b/src/sentry/middleware/integrations/parsers/base.py index 5d23d47ae92c67..6ef7d6e3f6d9ef 100644 --- a/src/sentry/middleware/integrations/parsers/base.py +++ b/src/sentry/middleware/integrations/parsers/base.py @@ -144,6 +144,9 @@ def get_integration_from_request(self) -> Integration | None: # Optional Overrides + def should_operate_parser(self, request: HttpRequest): + return True + def get_organizations_from_integration( self, integration: Optional[Integration] = None ) -> Sequence[RpcOrganizationSummary]: diff --git a/src/sentry/middleware/integrations/parsers/plugin.py b/src/sentry/middleware/integrations/parsers/plugin.py new file mode 100644 index 00000000000000..80993cd626ff2f --- /dev/null +++ b/src/sentry/middleware/integrations/parsers/plugin.py @@ -0,0 +1,7 @@ +from sentry.middleware.integrations.parsers.base import BaseRequestParser +from sentry.models.outbox import WebhookProviderIdentifier + + +class PluginRequestParser(BaseRequestParser): + provider = "plugin" + webhook_identifier = WebhookProviderIdentifier.PLUGIN diff --git a/src/sentry/models/outbox.py b/src/sentry/models/outbox.py index 4d6121c2ed22f3..616c7d8359cc74 100644 --- a/src/sentry/models/outbox.py +++ b/src/sentry/models/outbox.py @@ -113,6 +113,7 @@ class WebhookProviderIdentifier(IntEnum): JIRA_SERVER = 7 GITHUB_ENTERPRISE = 8 BITBUCKET_SERVER = 9 + PLUGIN = 10 def _ensure_not_null(k: str, v: Any) -> Any: diff --git a/src/sentry_plugins/github/urls.py b/src/sentry_plugins/github/urls.py index 76ca2c7c54ec57..1213e07f35b37b 100644 --- a/src/sentry_plugins/github/urls.py +++ b/src/sentry_plugins/github/urls.py @@ -1,6 +1,6 @@ from django.urls import re_path -from .webhooks import GithubIntegrationsWebhookEndpoint, GithubPluginWebhookEndpoint +from .webhooks import GithubPluginIntegrationsWebhookEndpoint, GithubPluginWebhookEndpoint urlpatterns = [ re_path( @@ -9,6 +9,6 @@ ), re_path( r"^installations/webhook/$", - GithubIntegrationsWebhookEndpoint.as_view(), + GithubPluginIntegrationsWebhookEndpoint.as_view(), ), ] From aa7139f77e068995e3c9ad4bf1aae339d015ab1c Mon Sep 17 00:00:00 2001 From: Leander Rodrigues Date: Fri, 4 Aug 2023 12:02:51 -0700 Subject: [PATCH 3/5] =?UTF-8?q?=F0=9F=93=A3=20Add=20logging=20to=20check?= =?UTF-8?q?=20dead=20code?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/sentry_plugins/bitbucket/endpoints/webhook.py | 4 ++++ src/sentry_plugins/github/webhooks/integration.py | 8 ++++++++ src/sentry_plugins/github/webhooks/non_integration.py | 4 ++++ 3 files changed, 16 insertions(+) diff --git a/src/sentry_plugins/bitbucket/endpoints/webhook.py b/src/sentry_plugins/bitbucket/endpoints/webhook.py index edf3394842e9a5..abbbcbc820f54a 100644 --- a/src/sentry_plugins/bitbucket/endpoints/webhook.py +++ b/src/sentry_plugins/bitbucket/endpoints/webhook.py @@ -91,6 +91,10 @@ def dispatch(self, request: Request, *args, **kwargs) -> HttpResponse: return super().dispatch(request, *args, **kwargs) def post(self, request: Request, organization_id): + logger.error( + "bitbucket_plugin.deprecation_check", + extra={"organization_id": organization_id, "meta": request.META}, + ) try: organization = Organization.objects.get_from_cache(id=organization_id) except Organization.DoesNotExist: diff --git a/src/sentry_plugins/github/webhooks/integration.py b/src/sentry_plugins/github/webhooks/integration.py index d215022eff01f9..865110085c29e4 100644 --- a/src/sentry_plugins/github/webhooks/integration.py +++ b/src/sentry_plugins/github/webhooks/integration.py @@ -1,5 +1,7 @@ from __future__ import annotations +import logging + from django.http import HttpResponse from django.utils.decorators import method_decorator from django.views.decorators.csrf import csrf_exempt @@ -11,6 +13,8 @@ from .base import GithubWebhookBase from .events import InstallationEventWebhook, InstallationRepositoryEventWebhook, PushEventWebhook +logger = logging.getLogger(__name__) + class GithubPluginIntegrationsWebhookEndpoint(GithubWebhookBase): _handlers = { @@ -30,4 +34,8 @@ def get_secret(self, organization: Organization) -> str | None: return options.get("github.integration-hook-secret") def post(self, request: Request) -> HttpResponse: + logger.error( + "github_plugin.install.deprecation_check", + extra={"meta": request.META}, + ) return self.handle(request) diff --git a/src/sentry_plugins/github/webhooks/non_integration.py b/src/sentry_plugins/github/webhooks/non_integration.py index 5cc73240f3d66e..99c653af01162d 100644 --- a/src/sentry_plugins/github/webhooks/non_integration.py +++ b/src/sentry_plugins/github/webhooks/non_integration.py @@ -22,6 +22,10 @@ def get_secret(self, organization: Organization) -> str | None: ) def post(self, request: Request, organization_id): + logger.error( + "github_plugin.webhook.deprecation_check", + extra={"organization_id": organization_id, "meta": request.META}, + ) try: organization = Organization.objects.get_from_cache(id=organization_id) except Organization.DoesNotExist: From 78db92445b751fb6bea22b91e7b673fb4ed5f9bb Mon Sep 17 00:00:00 2001 From: Leander Rodrigues Date: Fri, 4 Aug 2023 12:20:57 -0700 Subject: [PATCH 4/5] =?UTF-8?q?=F0=9F=94=A5=20remove=20classification=20co?= =?UTF-8?q?de=20for=20now?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../integrations/classifications.py | 88 ------------------- .../integrations/parsers/__init__.py | 2 - .../middleware/integrations/parsers/plugin.py | 7 -- 3 files changed, 97 deletions(-) delete mode 100644 src/sentry/middleware/integrations/classifications.py delete mode 100644 src/sentry/middleware/integrations/parsers/plugin.py diff --git a/src/sentry/middleware/integrations/classifications.py b/src/sentry/middleware/integrations/classifications.py deleted file mode 100644 index 4aec84c3581a81..00000000000000 --- a/src/sentry/middleware/integrations/classifications.py +++ /dev/null @@ -1,88 +0,0 @@ -import logging -from typing import Callable, Mapping, Type - -from django.http import HttpRequest, HttpResponse - -from sentry.middleware.integrations.parsers.base import BaseRequestParser - -from .parsers import ( - BitbucketRequestParser, - BitbucketServerRequestParser, - GithubEnterpriseRequestParser, - GithubRequestParser, - GitlabRequestParser, - JiraRequestParser, - JiraServerRequestParser, - MsTeamsRequestParser, - PluginRequestParser, - SlackRequestParser, - VstsRequestParser, -) - - -class BaseClassification: - def should_operate(self, request: HttpRequest) -> bool: - """ - Function to determine if an incoming request should be handled by this - classifcation of parsers or not. - """ - return False - - -class IntegrationClassification(BaseClassification): - logger = logging.getLogger(f"{__name__}.integration") - integration_prefix: str = "/extensions/" - """Prefix for all integration requests. See `src/sentry/web/urls.py`""" - setup_suffix: str = "/setup/" - """Suffix for PipelineAdvancerView on installation. See `src/sentry/web/urls.py`""" - - integration_parsers: Mapping[str, Type[BaseRequestParser]] = { - parser.provider: parser - for parser in [ - BitbucketRequestParser, - BitbucketServerRequestParser, - GithubEnterpriseRequestParser, - GithubRequestParser, - GitlabRequestParser, - JiraRequestParser, - JiraServerRequestParser, - MsTeamsRequestParser, - SlackRequestParser, - VstsRequestParser, - ] - } - - def should_operate(self, request: HttpRequest) -> bool: - is_integration = request.path.startswith(self.integration_prefix) - is_not_setup = not request.path.endswith(self.setup_suffix) - return is_integration and is_not_setup - - def get_response(self, request: HttpRequest, response_handler: Callable[[], HttpResponse]): - provider = self._identify_provider(request) - if not provider: - return response_handler(request) - - parser_class = self.integration_parsers.get(provider) - if not parser_class: - self.logger.error( - "integration_control.unknown_provider", - extra={"path": request.path, "provider": provider}, - ) - return response_handler(request) - - parser = parser_class( - request=request, - response_handler=response_handler, - ) - - return parser.get_response() - - -class PluginClassification(BaseClassification): - logger = logging.getLogger(f"{__name__}.plugin") - - def should_operate(self, request: HttpRequest) -> bool: - pass - - def get_response(self) -> HttpResponse: - pass diff --git a/src/sentry/middleware/integrations/parsers/__init__.py b/src/sentry/middleware/integrations/parsers/__init__.py index d83d155cc542eb..755f746ea29378 100644 --- a/src/sentry/middleware/integrations/parsers/__init__.py +++ b/src/sentry/middleware/integrations/parsers/__init__.py @@ -6,7 +6,6 @@ from .jira import JiraRequestParser from .jira_server import JiraServerRequestParser from .msteams import MsTeamsRequestParser -from .plugin import PluginRequestParser from .slack import SlackRequestParser from .vsts import VstsRequestParser @@ -19,7 +18,6 @@ "JiraServerRequestParser", "GithubEnterpriseRequestParser", "MsTeamsRequestParser", - "PluginRequestParser", "SlackRequestParser", "VstsRequestParser", ) diff --git a/src/sentry/middleware/integrations/parsers/plugin.py b/src/sentry/middleware/integrations/parsers/plugin.py deleted file mode 100644 index 80993cd626ff2f..00000000000000 --- a/src/sentry/middleware/integrations/parsers/plugin.py +++ /dev/null @@ -1,7 +0,0 @@ -from sentry.middleware.integrations.parsers.base import BaseRequestParser -from sentry.models.outbox import WebhookProviderIdentifier - - -class PluginRequestParser(BaseRequestParser): - provider = "plugin" - webhook_identifier = WebhookProviderIdentifier.PLUGIN From 9a71c093339bbd7907ee392f326561c3ebae63e0 Mon Sep 17 00:00:00 2001 From: Leander Rodrigues Date: Fri, 4 Aug 2023 12:57:19 -0700 Subject: [PATCH 5/5] =?UTF-8?q?=F0=9F=94=A5=20remove=20more=20temp=20code?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/sentry/middleware/integrations/parsers/base.py | 3 --- src/sentry/models/outbox.py | 1 - 2 files changed, 4 deletions(-) diff --git a/src/sentry/middleware/integrations/parsers/base.py b/src/sentry/middleware/integrations/parsers/base.py index 6ef7d6e3f6d9ef..5d23d47ae92c67 100644 --- a/src/sentry/middleware/integrations/parsers/base.py +++ b/src/sentry/middleware/integrations/parsers/base.py @@ -144,9 +144,6 @@ def get_integration_from_request(self) -> Integration | None: # Optional Overrides - def should_operate_parser(self, request: HttpRequest): - return True - def get_organizations_from_integration( self, integration: Optional[Integration] = None ) -> Sequence[RpcOrganizationSummary]: diff --git a/src/sentry/models/outbox.py b/src/sentry/models/outbox.py index d76d7b1b84bc4e..52bad0863de884 100644 --- a/src/sentry/models/outbox.py +++ b/src/sentry/models/outbox.py @@ -113,7 +113,6 @@ class WebhookProviderIdentifier(IntEnum): JIRA_SERVER = 7 GITHUB_ENTERPRISE = 8 BITBUCKET_SERVER = 9 - PLUGIN = 10 def _ensure_not_null(k: str, v: Any) -> Any: