repository secret was being saved to repo when the update failed

tasks/tests/unit/test_upload_task.py

@@ -421,7 +421,7 @@ def test_upload_task_upload_processing_delay_not_enough_delay(
             jsonified_redis_queue
         )
         mock_redis.keys[f"latest_upload/{commit.repoid}/{commit.commitid}"] = (
-            datetime.utcnow() - timedelta(seconds=10)
+            datetime.now() - timedelta(seconds=10)
         ).timestamp()
         with pytest.raises(Retry):
             UploadTask().run_impl(dbsession, commit.repoid, commit.commitid)
@@ -455,7 +455,7 @@ def test_upload_task_upload_processing_delay_enough_delay(
             repository__name="example-python",
         )
         mock_redis.keys[f"latest_upload/{commit.repoid}/{commit.commitid}"] = (
-            datetime.utcnow() - timedelta(seconds=1200)
+            datetime.now() - timedelta(seconds=1200)
         ).timestamp()
         mock_configuration.set_params({"setup": {"upload_processing_delay": 1000}})
         mocker.patch.object(UploadTask, "app", celery_app)
@@ -1636,6 +1636,35 @@ def test_doesnt_need_webhook_secret_backfill_no_hookid(
         assert repository.webhook_secret is None
         gitlab_provider.edit_webhook.assert_not_called()
 
+    def test_needs_webhook_secret_backfill_error(
+        self, dbsession, mocker, mock_configuration
+    ):
+        mock_configuration.set_params(
+            {"gitlab_enterprise": {"bot": {"key": "somekey"}}}
+        )
+        repository = RepositoryFactory.create(
+            repoid="5678", hookid="1234", webhook_secret=None
+        )
+        dbsession.add(repository)
+        commit = CommitFactory.create(repository=repository)
+        dbsession.add(commit)
+        gitlab_e_provider = mocker.MagicMock(
+            GitlabEnterprise, get_commit_diff=mock.AsyncMock(return_value={})
+        )
+        mock_repo_provider = mocker.patch(
+            "services.repository._get_repo_provider_service_instance"
+        )
+        mock_repo_provider.return_value = gitlab_e_provider
+        gitlab_e_provider.data = mocker.MagicMock()
+        gitlab_e_provider.service = "gitlab"
+        gitlab_e_provider.edit_webhook.side_effect = TorngitClientError
+        task = UploadTask()
+
+        res = task.possibly_setup_webhooks(commit, gitlab_e_provider)
+        assert res is False
+        assert repository.webhook_secret is None
+        gitlab_e_provider.edit_webhook.assert_called_once()
+
     def test_upload_not_ready_to_build_report(
         self, dbsession, mocker, mock_configuration, mock_repo_provider, mock_redis
     ):

tasks/upload.py

@@ -971,10 +971,10 @@ def possibly_setup_webhooks(self, commit, repository_service):
                 if repository_service.service in ["gitlab", "gitlab_enterprise"]:
                     # we use per-repo webhook secrets in this case
                     webhook_secret = repository.webhook_secret or str(uuid.uuid4())
-                    repository.webhook_secret = webhook_secret
                 else:
                     # service-level config value will be used instead in this case
                     webhook_secret = None
+
                 if should_post_webhook:
                     hook_result = async_to_sync(create_webhook_on_provider)(
                         repository_service, webhook_secret=webhook_secret
@@ -991,13 +991,15 @@ def possibly_setup_webhooks(self, commit, repository_service):
                     )
                     repository.hookid = hookid
                     repo_data["repo"]["hookid"] = hookid
+                    repository.webhook_secret = webhook_secret
                     return True  # was_setup
                 else:
                     async_to_sync(gitlab_webhook_update)(
                         repository_service=repository_service,
                         hookid=repository.hookid,
                         secret=webhook_secret,
                     )
+                    repository.webhook_secret = webhook_secret
                     log.info(
                         "Updated hook",
                         extra=dict(
@@ -1017,6 +1019,7 @@ def possibly_setup_webhooks(self, commit, repository_service):
                         commit=commit.commitid,
                         action="SET" if should_post_webhook else "EDIT",
                     ),
+                    exc_info=True,
                 )
         return False