Skip to content

Commit

Permalink
Don't fail on deregistering a repo on repo delete (#4482)
Browse files Browse the repository at this point in the history
this may be a transcient error, so let's not fail and allow minder to
delete it.

This may result in a dangling webhook registration in github. Which is
not too problematic since Minder detects and is able to overwrite them.

Signed-off-by: Juan Antonio Osorio <[email protected]>
  • Loading branch information
JAORMX authored Sep 13, 2024
1 parent fb4cab1 commit af03cf2
Show file tree
Hide file tree
Showing 2 changed files with 7 additions and 7 deletions.
4 changes: 3 additions & 1 deletion internal/repositories/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -534,7 +534,9 @@ func (r *repositoryService) deleteRepository(

err = client.DeregisterEntity(ctx, pb.Entity_ENTITY_REPOSITORIES, repo.Properties)
if err != nil {
return fmt.Errorf("error deleting webhook: %w", err)
zerolog.Ctx(ctx).Error().
Dict("properties", repo.Properties.ToLogDict()).
Err(err).Msg("error deregistering repo")
}

_, err = db.WithTransaction(r.store, func(t db.ExtendQuerier) (*pb.Repository, error) {
Expand Down
10 changes: 4 additions & 6 deletions internal/repositories/service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -210,15 +210,14 @@ func TestRepositoryService_DeleteRepository(t *testing.T) {
ExpectedError: "error instantiating provider",
},
{
Name: "DeleteByName fails when entity cannot be deregistered",
Name: "DeleteByName still works when entity cannot be deregistered",
DeleteType: ByName,
DBSetup: newDBMock(withSuccessfulGetByName),
DBSetup: newDBMock(withSuccessfulGetByName, withSuccessfulDelete),
ServiceSetup: newPropSvcMock(withSuccessfulEntityWithProps),
ProviderManagerSetup: func(p provinfv1.Provider) pf.ProviderManagerMockBuilder {
return pf.NewProviderManagerMock(pf.WithSuccessfulInstantiateFromID(p))
},
ProviderSetup: newProviderMock(withFailedDeregister),
ExpectedError: "error deleting webhook",
},
{
Name: "DeleteByName by ID fails when repo cannot be deleted from DB",
Expand Down Expand Up @@ -264,15 +263,14 @@ func TestRepositoryService_DeleteRepository(t *testing.T) {
ExpectedError: "error instantiating provider",
},
{
Name: "DeleteByID fails when entity cannot be deregistered",
Name: "DeleteByID works when entity cannot be deregistered",
DeleteType: ByID,
DBSetup: newDBMock(),
DBSetup: newDBMock(withSuccessfulDelete),
ServiceSetup: newPropSvcMock(withSuccessfulEntityWithProps),
ProviderManagerSetup: func(p provinfv1.Provider) pf.ProviderManagerMockBuilder {
return pf.NewProviderManagerMock(pf.WithSuccessfulInstantiateFromID(p))
},
ProviderSetup: newProviderMock(withFailedDeregister),
ExpectedError: "error deleting webhook",
},
{
Name: "DeleteByID by ID fails when repo cannot be deleted from DB",
Expand Down

0 comments on commit af03cf2

Please sign in to comment.