Provisioning: Fix repos stuck in deletion (#111918)

This commit is contained in:
Stephanie Hingtgen 2025-10-02 00:04:55 -06:00 committed by GitHub
parent ebbcef5583
commit 6f0a8a344a
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
5 changed files with 57 additions and 2 deletions

View File

@ -13,6 +13,7 @@ import (
// API errors that we need to convey after parsing real GH errors (or faking them). // API errors that we need to convey after parsing real GH errors (or faking them).
var ( var (
ErrResourceNotFound = errors.New("the resource does not exist") ErrResourceNotFound = errors.New("the resource does not exist")
ErrUnauthorized = errors.New("unauthorized")
//lint:ignore ST1005 this is not punctuation //lint:ignore ST1005 this is not punctuation
ErrServiceUnavailable = apierrors.NewServiceUnavailable("github is unavailable") ErrServiceUnavailable = apierrors.NewServiceUnavailable("github is unavailable")
ErrTooManyItems = errors.New("maximum number of items exceeded") ErrTooManyItems = errors.New("maximum number of items exceeded")

View File

@ -199,6 +199,9 @@ func (r *githubClient) DeleteWebhook(ctx context.Context, owner, repository stri
if ghErr.Response.StatusCode == http.StatusNotFound { if ghErr.Response.StatusCode == http.StatusNotFound {
return ErrResourceNotFound return ErrResourceNotFound
} }
if ghErr.Response.StatusCode == http.StatusUnauthorized || ghErr.Response.StatusCode == http.StatusForbidden {
return ErrUnauthorized
}
return err return err
} }

View File

@ -975,6 +975,27 @@ func TestGithubClient_DeleteWebhook(t *testing.T) {
webhookID: 789, webhookID: 789,
wantErr: ErrServiceUnavailable, 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", name: "other error",
mockHandler: mockhub.NewMockedHTTPClient( mockHandler: mockhub.NewMockedHTTPClient(

View File

@ -274,11 +274,15 @@ func (r *githubWebhookRepository) deleteWebhook(ctx context.Context) error {
id := r.config.Status.Webhook.ID id := r.config.Status.Webhook.ID
err := r.gh.DeleteWebhook(ctx, r.owner, r.repo, 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) return fmt.Errorf("delete webhook: %w", err)
} }
if errors.Is(err, ErrResourceNotFound) { 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 return nil
} }

View File

@ -1565,6 +1565,32 @@ func TestGitHubRepository_OnDelete(t *testing.T) {
// We don't return an error if the webhook is already gone // We don't return an error if the webhook is already gone
expectedError: nil, 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", name: "no webhook URL provided",
setupMock: func(_ *MockClient) {}, setupMock: func(_ *MockClient) {},