ref(integrations): Move webhook creation to post-create hook

build_repository_config for GitLab, Bitbucket, and Bitbucket Server created webhooks eagerly before checking if the repo already existed in Sentry. This blocked adding those providers to the repo sync task, since it would create webhooks for every repo on every sync run.

This introduces on_create_repository hook that fires only after a repo is actually persisted. Webhooks are now created at the right time and skipped if the repo already has one. This unblocks adding all remaining SCM providers to the periodic sync.

Author: wedamija

src/sentry/integrations/bitbucket/repository.py

@@ -3,6 +3,8 @@
 from collections.abc import Mapping
 from typing import TYPE_CHECKING, Any
 
+from sentry.integrations.services.repository.model import RpcRepository
+from sentry.integrations.services.repository.service import repository_service
 from sentry.integrations.types import IntegrationProviderSlug
 from sentry.locks import locks
 from sentry.models.apitoken import generate_token
@@ -55,12 +57,23 @@ def get_webhook_secret(self, organization):
     def build_repository_config(
         self, organization: RpcOrganization, data: Mapping[str, Any]
     ) -> RepositoryConfig:
-        installation = self.get_installation(data.get("installation"), organization.id)
+        return {
+            "name": data["identifier"],
+            "external_id": data["external_id"],
+            "url": "https://bitbucket.org/{}".format(data["name"]),
+            "config": {"name": data["name"]},
+            "integration_id": data["installation"],
+        }
+
+    def on_create_repository(self, repo: RpcRepository, organization: RpcOrganization) -> None:
+        if repo.config.get("webhook_id"):
+            return
+        installation = self.get_installation(repo.integration_id, repo.organization_id)
         client = installation.get_client()
+        secret = installation.model.metadata.get("webhook_secret", "")
         try:
-            secret = installation.model.metadata.get("webhook_secret", "")
             resp = client.create_hook(
-                data["identifier"],
+                repo.config["name"],
                 {
                     "description": "sentry-bitbucket-repo-hook",
                     "url": absolute_uri(
@@ -73,14 +86,8 @@ def build_repository_config(
             )
         except Exception as e:
             installation.raise_error(e)
-        else:
-            return {
-                "name": data["identifier"],
-                "external_id": data["external_id"],
-                "url": "https://bitbucket.org/{}".format(data["name"]),
-                "config": {"name": data["name"], "webhook_id": resp["uuid"]},
-                "integration_id": data["installation"],
-            }
+        repo.config["webhook_id"] = resp["uuid"]
+        repository_service.update_repository(organization_id=organization.id, update=repo)
 
     def on_delete_repository(self, repo):
         installation = self.get_installation(repo.integration_id, repo.organization_id)

src/sentry/integrations/bitbucket_server/repository.py

@@ -7,6 +7,8 @@
 from django.core.cache import cache
 from django.urls import reverse
 
+from sentry.integrations.services.repository.model import RpcRepository
+from sentry.integrations.services.repository.service import repository_service
 from sentry.integrations.types import IntegrationProviderSlug
 from sentry.organizations.services.organization.model import RpcOrganization
 from sentry.plugins.providers.integration_repository import (
@@ -46,20 +48,38 @@ def build_repository_config(
         self, organization: RpcOrganization, data: Mapping[str, Any]
     ) -> RepositoryConfig:
         installation = self.get_installation(data.get("installation"), organization.id)
+        return {
+            "name": data["identifier"],
+            "external_id": data["external_id"],
+            "url": installation.model.metadata["base_url"]
+            + "/projects/{project}/repos/{repo}/browse".format(
+                project=data["project"], repo=data["repo"]
+            ),
+            "config": {
+                "name": data["identifier"],
+                "project": data["project"],
+                "repo": data["repo"],
+            },
+            "integration_id": data["installation"],
+        }
+
+    def on_create_repository(self, repo: RpcRepository, organization: RpcOrganization) -> None:
+        if repo.config.get("webhook_id"):
+            return
+        installation = self.get_installation(repo.integration_id, repo.organization_id)
         client = installation.get_client()
-
         try:
             resp = client.create_hook(
-                data["project"],
-                data["repo"],
+                repo.config["project"],
+                repo.config["repo"],
                 {
                     "name": "sentry-bitbucket-server-repo-hook",
                     "url": absolute_uri(
                         reverse(
                             "sentry-extensions-bitbucketserver-webhook",
                             kwargs={
                                 "organization_id": organization.id,
-                                "integration_id": data.get("installation"),
+                                "integration_id": repo.integration_id,
                             },
                         )
                     ),
@@ -69,22 +89,8 @@ def build_repository_config(
             )
         except Exception as e:
             installation.raise_error(e)
-        else:
-            return {
-                "name": data["identifier"],
-                "external_id": data["external_id"],
-                "url": installation.model.metadata["base_url"]
-                + "/projects/{project}/repos/{repo}/browse".format(
-                    project=data["project"], repo=data["repo"]
-                ),
-                "config": {
-                    "name": data["identifier"],
-                    "project": data["project"],
-                    "repo": data["repo"],
-                    "webhook_id": resp["id"],
-                },
-                "integration_id": data["installation"],
-            }
+        repo.config["webhook_id"] = resp["id"]
+        repository_service.update_repository(organization_id=organization.id, update=repo)
 
     def on_delete_repository(self, repo):
         installation = self.get_installation(repo.integration_id, repo.organization_id)

src/sentry/integrations/gitlab/repository.py

@@ -3,6 +3,8 @@
 from collections.abc import Mapping
 from typing import TYPE_CHECKING, Any
 
+from sentry.integrations.services.repository.model import RpcRepository
+from sentry.integrations.services.repository.service import repository_service
 from sentry.integrations.types import IntegrationProviderSlug
 from sentry.organizations.services.organization.model import RpcOrganization
 from sentry.plugins.providers import IntegrationRepositoryProvider
@@ -43,25 +45,30 @@ def get_repository_data(self, organization, config):
     def build_repository_config(
         self, organization: RpcOrganization, data: Mapping[str, Any]
     ) -> RepositoryConfig:
-        installation = self.get_installation(data.get("installation"), organization.id)
-        client = installation.get_client()
-        try:
-            hook_id = client.create_project_webhook(data["project_id"])
-        except Exception as e:
-            raise installation.raise_error(e)
         return {
             "name": data["name"],
             "external_id": data["external_id"],
             "url": data["url"],
             "config": {
                 "instance": data["instance"],
                 "path": data["path"],
-                "webhook_id": hook_id,
                 "project_id": data["project_id"],
             },
             "integration_id": data["installation"],
         }
 
+    def on_create_repository(self, repo: RpcRepository, organization: RpcOrganization) -> None:
+        if repo.config.get("webhook_id"):
+            return
+        installation = self.get_installation(repo.integration_id, repo.organization_id)
+        client = installation.get_client()
+        try:
+            hook_id = client.create_project_webhook(repo.config["project_id"])
+        except Exception as e:
+            raise installation.raise_error(e)
+        repo.config["webhook_id"] = hook_id
+        repository_service.update_repository(organization_id=organization.id, update=repo)
+
     def on_delete_repository(self, repo):
         """Clean up the attached webhook"""
         installation = self.get_installation(repo.integration_id, repo.organization_id)

src/sentry/integrations/source_code_management/sync_repos.py

@@ -39,15 +39,15 @@
 logger = logging.getLogger(__name__)
 
 # Providers to include in the periodic sync. Each must implement
-# get_repositories() returning RepositoryInfo with external_id.
-# GitLab, Bitbucket, and Bitbucket Server are excluded because their
-# build_repository_config creates webhooks as a side effect, and
-# create_repositories calls it for every repo before checking if it already
-# exists. This needs to be fixed before adding those providers.
+# get_repositories() returning RepositoryInfo with all fields needed
+# by their build_repository_config.
 # Perforce is excluded because it cannot derive external_id from its API.
 SCM_SYNC_PROVIDERS = [
     "github",
     "github_enterprise",
+    "gitlab",
+    "bitbucket",
+    "bitbucket_server",
     "vsts",
 ]
 

src/sentry/plugins/providers/integration_repository.py

@@ -19,7 +19,6 @@
 from sentry.integrations.services.repository import repository_service
 from sentry.integrations.services.repository.model import RpcCreateRepository, RpcRepository
 from sentry.integrations.source_code_management.repository import RepositoryIntegration
-from sentry.models.repository import Repository
 from sentry.organizations.services.organization.model import RpcOrganization
 from sentry.shared_integrations.exceptions import IntegrationError
 from sentry.signals import repo_linked
@@ -132,9 +131,11 @@ def create_repository(
             existing_repo.name = name
             existing_repo.integration_id = integration_id
             existing_repo.url = url
+            existing_repo.config = result.get("config") or {}
             repository_service.update_repository(
                 organization_id=organization.id, update=existing_repo
             )
+            self.on_create_repository(existing_repo, organization)
             metrics.incr("sentry.integration_repo_provider.repo_relink")
             return result, existing_repo
 
@@ -170,6 +171,7 @@ def create_repository(
             # also update the status if it was in a bad state
             repo.status = ObjectStatus.ACTIVE
             repository_service.update_repository(organization_id=organization.id, update=repo)
+            self.on_create_repository(repo, organization)
         else:
             create_repository = RpcCreateRepository.parse_obj(
                 {**repo_update_params, "status": ObjectStatus.ACTIVE}
@@ -178,16 +180,9 @@ def create_repository(
                 organization_id=organization.id, create=create_repository
             )
             if new_repository is not None:
+                self.on_create_repository(new_repository, organization)
                 return result, new_repository
 
-            # Try to delete webhook we just created
-            try:
-                self.on_delete_repository(
-                    Repository(organization_id=organization.id, **repo_update_params)
-                )
-            except IntegrationError:
-                pass
-
             # if possible update the repo with matching integration
             repositories = repository_service.get_repositories(
                 organization_id=organization.id,
@@ -272,17 +267,11 @@ def create_repositories(
                 organization_id=organization.id, create=create_repository
             )
             if new_repository is not None:
+                self.on_create_repository(new_repository, organization)
                 created_repos.append(new_repository)
                 continue
 
             missing_repos.append(repo_config)
-            # Try to delete webhook we just created
-            try:
-                self.on_delete_repository(
-                    Repository(organization_id=organization.id, **repo_config)
-                )
-            except IntegrationError:
-                pass
 
             # if possible update the repo with matching integration
             repositories = repository_service.get_repositories(
@@ -299,6 +288,8 @@ def create_repositories(
                 organization_id=organization.id,
                 updates=repos_to_update,
             )
+            for repo in repos_to_update:
+                self.on_create_repository(repo, organization)
 
         return created_repos, repos_to_update, missing_repos
 
@@ -382,6 +373,14 @@ def build_repository_config(
         """
         raise NotImplementedError
 
+    def on_create_repository(self, repo: RpcRepository, organization: RpcOrganization) -> None:
+        """Called after a repository is created or reactivated.
+
+        Override to perform post-creation setup like webhook creation.
+        The repo has already been persisted — update repo.config and call
+        repository_service.update_repository to store any new config values.
+        """
+
     def on_delete_repository(self, repo):
         pass
 

tests/sentry/integrations/bitbucket/test_repository.py

@@ -139,7 +139,7 @@ def test_build_repository_config(self) -> None:
             "external_id": REPO["uuid"],
             "url": "https://bitbucket.org/laurynsentry/helloworld",
             "integration_id": integration.id,
-            "config": {"name": full_repo_name, "webhook_id": webhook_id},
+            "config": {"name": full_repo_name},
         }
 
     def test_repository_external_slug(self) -> None:

tests/sentry/integrations/bitbucket_server/test_repository.py

@@ -235,7 +235,6 @@ def test_build_repository_config(self) -> None:
                 "name": full_repo_name,
                 "project": project,
                 "repo": repo,
-                "webhook_id": webhook_id,
             },
         }
 

tests/sentry/integrations/github/tasks/test_link_all_repos.py

@@ -272,29 +272,3 @@ def test_link_all_repos_repo_creation_error(
         assert end2.args[0] == EventLifecycleOutcome.SUCCESS
         assert end1.args[0] == EventLifecycleOutcome.HALTED
         assert_halt_metric(mock_record, LinkAllReposHaltReason.REPOSITORY_NOT_CREATED.value)
-
-    @patch("sentry.integrations.utils.metrics.EventLifecycle.record_event")
-    @patch("sentry.integrations.services.repository.repository_service.create_repository")
-    @patch("sentry.plugins.providers.IntegrationRepositoryProvider.on_delete_repository")
-    @responses.activate
-    def test_link_all_repos_repo_creation_exception(
-        self, mock_delete_repo, mock_create_repository, mock_record, _
-    ):
-        mock_create_repository.return_value = None
-        mock_delete_repo.side_effect = Exception
-
-        self._add_responses()
-
-        with pytest.raises(Exception):
-            link_all_repos(
-                integration_key=self.key,
-                integration_id=self.integration.id,
-                organization_id=self.organization.id,
-            )
-
-        assert len(mock_record.mock_calls) == 4
-        start1, start2, end2, end1 = mock_record.mock_calls
-        assert start1.args[0] == EventLifecycleOutcome.STARTED
-        assert start2.args[0] == EventLifecycleOutcome.STARTED
-        assert end2.args[0] == EventLifecycleOutcome.SUCCESS
-        assert end1.args[0] == EventLifecycleOutcome.FAILURE