diff --git a/apps/provisioning/pkg/repository/github/client.go b/apps/provisioning/pkg/repository/github/client.go index 5b21a5a4f1e..00f1ca0946b 100644 --- a/apps/provisioning/pkg/repository/github/client.go +++ b/apps/provisioning/pkg/repository/github/client.go @@ -13,6 +13,7 @@ import ( // API errors that we need to convey after parsing real GH errors (or faking them). var ( ErrResourceNotFound = errors.New("the resource does not exist") + ErrUnauthorized = errors.New("unauthorized") //lint:ignore ST1005 this is not punctuation ErrServiceUnavailable = apierrors.NewServiceUnavailable("github is unavailable") ErrTooManyItems = errors.New("maximum number of items exceeded") diff --git a/apps/provisioning/pkg/repository/github/impl.go b/apps/provisioning/pkg/repository/github/impl.go index c9f5468bc8d..c80f1e7a7f9 100644 --- a/apps/provisioning/pkg/repository/github/impl.go +++ b/apps/provisioning/pkg/repository/github/impl.go @@ -199,6 +199,9 @@ func (r *githubClient) DeleteWebhook(ctx context.Context, owner, repository stri if ghErr.Response.StatusCode == http.StatusNotFound { return ErrResourceNotFound } + if ghErr.Response.StatusCode == http.StatusUnauthorized || ghErr.Response.StatusCode == http.StatusForbidden { + return ErrUnauthorized + } return err } diff --git a/apps/provisioning/pkg/repository/github/impl_test.go b/apps/provisioning/pkg/repository/github/impl_test.go index fedbc2a9850..57f954e906e 100644 --- a/apps/provisioning/pkg/repository/github/impl_test.go +++ b/apps/provisioning/pkg/repository/github/impl_test.go @@ -975,6 +975,27 @@ func TestGithubClient_DeleteWebhook(t *testing.T) { webhookID: 789, wantErr: ErrServiceUnavailable, }, + { + name: "unauthorized to delete the webhook", + mockHandler: mockhub.NewMockedHTTPClient( + mockhub.WithRequestMatchHandler( + mockhub.DeleteReposHooksByOwnerByRepoByHookId, + http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusUnauthorized) + require.NoError(t, json.NewEncoder(w).Encode(github.ErrorResponse{ + Response: &http.Response{ + StatusCode: http.StatusUnauthorized, + }, + Message: "401 bad credentials", + })) + }), + ), + ), + owner: "test-owner", + repository: "test-repo", + webhookID: 789, + wantErr: ErrUnauthorized, + }, { name: "other error", mockHandler: mockhub.NewMockedHTTPClient( diff --git a/apps/provisioning/pkg/repository/github/webhook.go b/apps/provisioning/pkg/repository/github/webhook.go index 3c37b00cba7..c2e13681f33 100644 --- a/apps/provisioning/pkg/repository/github/webhook.go +++ b/apps/provisioning/pkg/repository/github/webhook.go @@ -274,11 +274,15 @@ func (r *githubWebhookRepository) deleteWebhook(ctx context.Context) error { id := r.config.Status.Webhook.ID err := r.gh.DeleteWebhook(ctx, r.owner, r.repo, id) - if err != nil && !errors.Is(err, ErrResourceNotFound) { + if err != nil && !errors.Is(err, ErrResourceNotFound) && !errors.Is(err, ErrUnauthorized) { return fmt.Errorf("delete webhook: %w", err) } if errors.Is(err, ErrResourceNotFound) { - logger.Info("webhook does not exist", "url", r.config.Status.Webhook.URL, "id", id) + logger.Warn("webhook no longer exists", "url", r.config.Status.Webhook.URL, "id", id) + return nil + } + if errors.Is(err, ErrUnauthorized) { + logger.Warn("webhook deletion failed. no longer authorized to delete this webhook", "url", r.config.Status.Webhook.URL, "id", id) return nil } diff --git a/apps/provisioning/pkg/repository/github/webhook_test.go b/apps/provisioning/pkg/repository/github/webhook_test.go index 5021623fe90..49faf733378 100644 --- a/apps/provisioning/pkg/repository/github/webhook_test.go +++ b/apps/provisioning/pkg/repository/github/webhook_test.go @@ -1565,6 +1565,32 @@ func TestGitHubRepository_OnDelete(t *testing.T) { // We don't return an error if the webhook is already gone expectedError: nil, }, + { + name: "unauthorized to delete the webhook", + setupMock: func(m *MockClient) { + m.On("DeleteWebhook", mock.Anything, "grafana", "grafana", int64(123)). + Return(ErrUnauthorized) + }, + config: &provisioning.Repository{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-repo", + }, + Spec: provisioning.RepositorySpec{ + GitHub: &provisioning.GitHubRepositoryConfig{ + Branch: "main", + }, + }, + Status: provisioning.RepositoryStatus{ + Webhook: &provisioning.WebhookStatus{ + ID: 123, + URL: "https://example.com/webhook", + }, + }, + }, + webhookURL: "https://example.com/webhook", + // We don't return an error if access to the webhook is revoked + expectedError: nil, + }, { name: "no webhook URL provided", setupMock: func(_ *MockClient) {},