fix(gitlab): Heal webhooks on integration reinstall

src/sentry/integrations/gitlab/repository.py

@@ -61,6 +61,7 @@ def build_repository_config(
         }
 
     def on_create_repository(self, repo: RpcRepository, organization: RpcOrganization) -> None:
+        existing_webhook_id = repo.config.get("webhook_id")
         # Namespaced under "gitlab.repository." so these attributes group together in the
         # Sentry Logs UI.
         log_extra = {
@@ -75,25 +76,53 @@ def on_create_repository(self, repo: RpcRepository, organization: RpcOrganizatio
             "gitlab.repository.on_create_repository",
             extra={
                 **log_extra,
-                "gitlab.repository.has_existing_webhook": bool(repo.config.get("webhook_id")),
+                "gitlab.repository.has_existing_webhook": bool(existing_webhook_id),
             },
         )
-        if repo.config.get("webhook_id"):
-            logger.info(
-                "gitlab.repository.webhook_creation_skipped",
-                extra={**log_extra, "gitlab.repository.webhook_id": repo.config.get("webhook_id")},
-            )
-            return
         installation = self.get_installation(repo.integration_id, repo.organization_id)
         client = installation.get_client()
+        project_id = repo.config["project_id"]
+
+        # A stored webhook_id can survive an uninstall/reinstall of the integration
+        # (disassociate_organization_integration clears integration_id but leaves the
+        # config intact and never deletes the GitLab hook). We only ever persist a
+        # webhook_id for a hook we created, and GitLab never reuses hook ids, so the
+        # stored id resolves to exactly one of two states — let GitLab be the source
+        # of truth by addressing the hook directly:
+        #   - hook still exists  -> update it in place, refreshing its token + events
+        #                           (heals a rotated secret from reinstalling with a
+        #                           new OAuth app)
+        #   - hook is gone (404) -> fall through and create a fresh one
+        if existing_webhook_id:
+            try:
+                client.update_project_webhook(project_id, existing_webhook_id)
+            except ApiError as e:
+                if e.code != 404:
+                    raise installation.raise_error(e)
+                # The stored hook no longer exists on GitLab; recreate it below.
+            except Exception as e:
+                raise installation.raise_error(e)
+            else:
+                logger.info(
+                    "gitlab.repository.webhook_refreshed",
+                    extra={**log_extra, "gitlab.repository.webhook_id": existing_webhook_id},
+                )
+                return
+
         try:
-            hook_id = client.create_project_webhook(repo.config["project_id"])
+            hook_id = client.create_project_webhook(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)
         logger.info(
-            "gitlab.repository.webhook_created",
+            # A prior webhook_id that 404'd means we replaced a stale hook rather than
+            # creating one for the first time; distinguish the two for telemetry.
+            (
+                "gitlab.repository.webhook_stale_recreated"
+                if existing_webhook_id
+                else "gitlab.repository.webhook_created"
+            ),
             extra={**log_extra, "gitlab.repository.webhook_id": hook_id},
         )
 

tests/sentry/integrations/gitlab/test_repository.py

@@ -1,11 +1,13 @@
 from functools import cached_property
+from unittest.mock import call, patch
 
 import orjson
 import pytest
 import responses
 
 from fixtures.gitlab import COMMIT_DIFF_RESPONSE, COMMIT_LIST_RESPONSE, COMPARE_RESPONSE
 from sentry.integrations.gitlab.repository import GitlabRepositoryProvider
+from sentry.integrations.services.repository.service import repository_service
 from sentry.models.pullrequest import PullRequest
 from sentry.models.repository import Repository
 from sentry.shared_integrations.exceptions import IntegrationError
@@ -103,6 +105,190 @@ def test_create_repository(self) -> None:
         assert response.status_code == 201
         self.assert_repository(self.default_repository_config)
 
+    @responses.activate
+    def test_on_create_repository_refreshes_existing_webhook(self) -> None:
+        response = self.create_repository(self.default_repository_config, self.integration.id)
+        responses.reset()
+
+        repo = repository_service.get_repository(
+            organization_id=self.organization.id, id=response.data["id"]
+        )
+        assert repo is not None
+        webhook_id = repo.config["webhook_id"]
+
+        # The stored hook still exists, so we address it by id and update it in
+        # place (refreshing its token + events) rather than recreating it. Assert
+        # the refreshed token carries the integration's current secret — this is
+        # what heals a rotated secret after reinstalling against a new OAuth app.
+        def request_callback(request):
+            payload = orjson.loads(request.body)
+            expected_token = "{}:{}".format(
+                self.integration.external_id, self.integration.metadata["webhook_secret"]
+            )
+            assert payload["token"] == expected_token
+            assert payload["merge_requests_events"]
+            assert payload["push_events"]
+            return 200, {}, orjson.dumps({"id": webhook_id}).decode()
+
+        responses.add_callback(
+            responses.PUT,
+            "https://example.gitlab.com/api/v4/projects/%s/hooks/%s" % (self.gitlab_id, webhook_id),
+            callback=request_callback,
+        )
+
+        with patch("sentry.integrations.gitlab.repository.logger.info") as mock_logger_info:
+            self.provider.on_create_repository(repo, self.organization)
+
+        # Addressed by id: a single PUT, no listing.
+        assert [c.request.method for c in responses.calls] == ["PUT"]
+        # webhook_id is left untouched when we refresh an existing hook.
+        refreshed = self.get_repository(pk=response.data["id"])
+        assert refreshed.config["webhook_id"] == webhook_id
+        mock_logger_info.assert_has_calls(
+            [
+                call(
+                    "gitlab.repository.on_create_repository",
+                    extra={
+                        "gitlab.repository.organization_id": self.organization.id,
+                        "gitlab.repository.integration_id": self.integration.id,
+                        "gitlab.repository.repository_id": repo.id,
+                        "gitlab.repository.project_id": repo.config["project_id"],
+                        "gitlab.repository.has_existing_webhook": True,
+                    },
+                ),
+                call(
+                    "gitlab.repository.webhook_refreshed",
+                    extra={
+                        "gitlab.repository.organization_id": self.organization.id,
+                        "gitlab.repository.integration_id": self.integration.id,
+                        "gitlab.repository.repository_id": repo.id,
+                        "gitlab.repository.project_id": repo.config["project_id"],
+                        "gitlab.repository.webhook_id": webhook_id,
+                    },
+                ),
+            ]
+        )
+
+    @responses.activate
+    def test_on_create_repository_recreates_stale_webhook(self) -> None:
+        response = self.create_repository(self.default_repository_config, self.integration.id)
+        responses.reset()
+
+        repo = repository_service.get_repository(
+            organization_id=self.organization.id, id=response.data["id"]
+        )
+        assert repo is not None
+        stale_webhook_id = repo.config["webhook_id"]
+
+        # Updating the stored hook by id 404s (it no longer exists on GitLab), so a
+        # fresh hook is created and its new id stored.
+        responses.add(
+            responses.PUT,
+            "https://example.gitlab.com/api/v4/projects/%s/hooks/%s"
+            % (self.gitlab_id, stale_webhook_id),
+            status=404,
+        )
+        responses.add(
+            responses.POST,
+            "https://example.gitlab.com/api/v4/projects/%s/hooks" % self.gitlab_id,
+            json={"id": 200},
+        )
+
+        with patch("sentry.integrations.gitlab.repository.logger.info") as mock_logger_info:
+            self.provider.on_create_repository(repo, self.organization)
+
+        assert [c.request.method for c in responses.calls] == ["PUT", "POST"]
+        recreated = self.get_repository(pk=response.data["id"])
+        assert recreated.config["webhook_id"] == 200
+        mock_logger_info.assert_has_calls(
+            [
+                call(
+                    "gitlab.repository.on_create_repository",
+                    extra={
+                        "gitlab.repository.organization_id": self.organization.id,
+                        "gitlab.repository.integration_id": self.integration.id,
+                        "gitlab.repository.repository_id": repo.id,
+                        "gitlab.repository.project_id": repo.config["project_id"],
+                        "gitlab.repository.has_existing_webhook": True,
+                    },
+                ),
+                call(
+                    "gitlab.repository.webhook_stale_recreated",
+                    extra={
+                        "gitlab.repository.organization_id": self.organization.id,
+                        "gitlab.repository.integration_id": self.integration.id,
+                        "gitlab.repository.repository_id": repo.id,
+                        "gitlab.repository.project_id": repo.config["project_id"],
+                        "gitlab.repository.webhook_id": 200,
+                    },
+                ),
+            ]
+        )
+
+    @responses.activate
+    def test_on_create_repository_update_webhook_non_404_failure(self) -> None:
+        response = self.create_repository(self.default_repository_config, self.integration.id)
+        responses.reset()
+
+        repo = repository_service.get_repository(
+            organization_id=self.organization.id, id=response.data["id"]
+        )
+        assert repo is not None
+        webhook_id = repo.config["webhook_id"]
+
+        # A non-404 error while updating the hook surfaces as an error and does NOT
+        # fall through to create a duplicate hook.
+        responses.add(
+            responses.PUT,
+            "https://example.gitlab.com/api/v4/projects/%s/hooks/%s" % (self.gitlab_id, webhook_id),
+            status=503,
+        )
+
+        with pytest.raises(IntegrationError):
+            self.provider.on_create_repository(repo, self.organization)
+
+        # No POST: a non-404 failure must not fall through and create a duplicate hook.
+        methods = [c.request.method for c in responses.calls]
+        assert "POST" not in methods
+        assert set(methods) == {"PUT"}
+        unchanged = self.get_repository(pk=response.data["id"])
+        assert unchanged.config["webhook_id"] == webhook_id
+
+    @responses.activate
+    def test_on_create_repository_logs_webhook_creation(self) -> None:
+        self.add_create_repository_responses(self.default_repository_config)
+
+        with patch("sentry.integrations.gitlab.repository.logger.info") as mock_logger_info:
+            response = self.create_repository(self.default_repository_config, self.integration.id)
+
+        assert response.status_code == 201
+        repo = self.get_repository(pk=response.data["id"])
+
+        mock_logger_info.assert_has_calls(
+            [
+                call(
+                    "gitlab.repository.on_create_repository",
+                    extra={
+                        "gitlab.repository.organization_id": self.organization.id,
+                        "gitlab.repository.integration_id": self.integration.id,
+                        "gitlab.repository.repository_id": repo.id,
+                        "gitlab.repository.project_id": repo.config["project_id"],
+                        "gitlab.repository.has_existing_webhook": False,
+                    },
+                ),
+                call(
+                    "gitlab.repository.webhook_created",
+                    extra={
+                        "gitlab.repository.organization_id": self.organization.id,
+                        "gitlab.repository.integration_id": self.integration.id,
+                        "gitlab.repository.repository_id": repo.id,
+                        "gitlab.repository.project_id": repo.config["project_id"],
+                        "gitlab.repository.webhook_id": repo.config["webhook_id"],
+                    },
+                ),
+            ]
+        )
+
     @responses.activate
     def test_create_repository_verify_payload(self) -> None:
         def request_callback(request):