diff --git a/apiserver/controllers/organizations.go b/apiserver/controllers/organizations.go index 10cacba3..ad158a84 100644 --- a/apiserver/controllers/organizations.go +++ b/apiserver/controllers/organizations.go @@ -157,7 +157,7 @@ func (a *APIController) DeleteOrgHandler(w http.ResponseWriter, r *http.Request) return } - if err := a.r.DeleteOrganization(ctx, orgID); err != nil { + if err := a.r.DeleteOrganization(ctx, orgID, false); err != nil { log.Printf("removing org: %+v", err) handleError(w, err) return diff --git a/apiserver/controllers/repositories.go b/apiserver/controllers/repositories.go index 615520c1..20eac630 100644 --- a/apiserver/controllers/repositories.go +++ b/apiserver/controllers/repositories.go @@ -157,7 +157,7 @@ func (a *APIController) DeleteRepoHandler(w http.ResponseWriter, r *http.Request return } - if err := a.r.DeleteRepository(ctx, repoID); err != nil { + if err := a.r.DeleteRepository(ctx, repoID, false); err != nil { log.Printf("fetching repo: %s", err) handleError(w, err) return diff --git a/go.sum b/go.sum index 156d5492..1e8205d2 100644 --- a/go.sum +++ b/go.sum @@ -48,7 +48,6 @@ github.com/frankban/quicktest v1.7.2/go.mod h1:jaStnuzAqU1AJdCO0l53JDCJrVDKcS03D github.com/frankban/quicktest v1.10.0/go.mod h1:ui7WezCLWMWxVWr1GETZY3smRy0G4KWq9vcPtJmFl7Y= github.com/frankban/quicktest v1.14.3 h1:FJKSZTDHjyhriyC81FLQ0LY93eSai0ZyR/ZIkd3ZUKE= github.com/frankban/quicktest v1.14.3/go.mod h1:mgiwOwqx65TmIk1wJ6Q7wvnVMocbUorkibMOrVTHZps= -github.com/go-logfmt/logfmt v0.5.1/go.mod h1:WYhtIu8zTZfxdn5+rREduYbwxfcBr/Vr6KEVveWlfTs= github.com/go-logr/logr v1.2.2/go.mod h1:jdQByPbusPIv2/zmleS9BjJVeZ6kBagPoEUsqbVz/1A= github.com/go-logr/logr v1.2.3 h1:2DntVwHkVopvECVRSlL5PSo9eG+cAkDCuckLubN+rq0= github.com/go-logr/logr v1.2.3/go.mod h1:jdQByPbusPIv2/zmleS9BjJVeZ6kBagPoEUsqbVz/1A= @@ -186,8 +185,6 @@ github.com/jinzhu/now v1.1.5/go.mod h1:d3SSVoowX0Lcu0IBviAWJpolVfI5UJVZZ7cO71lE/ github.com/joho/godotenv v1.3.0/go.mod h1:7hK45KPybAkOC6peb+G5yklZfMxEjkZhHbwpqxOKXbg= github.com/josharian/intern v1.0.0 h1:vlS4z54oSdjm0bgjRigI+G1HpF+tI+9rE5LLzOg8HmY= github.com/josharian/intern v1.0.0/go.mod h1:5DoeVV0s6jJacbCEi61lwdGj/aVlrQvzHFFd8Hwg//Y= -github.com/jpillora/backoff v1.0.0/go.mod h1:J/6gKK9jxlEcS3zixgDgUAsiuZ7yrSoa/FX5e0EB2j4= -github.com/json-iterator/go v1.1.12/go.mod h1:e30LSqwooZae/UwlEbR2852Gd8hjQvJoHmT4TnhNGBo= github.com/juju/clock v1.0.3 h1:yJHIsWXeU8j3QcBdiess09SzfiXRRrsjKPn2whnMeds= github.com/juju/clock v1.0.3/go.mod h1:HIBvJ8kiV/n7UHwKuCkdYL4l/MDECztHR2sAvWDxxf0= github.com/juju/errors v1.0.0 h1:yiq7kjCLll1BiaRuNY53MGI0+EQ3rF6GB+wvboZDefM= @@ -248,10 +245,7 @@ github.com/mitchellh/mapstructure v1.3.3/go.mod h1:bFUtVrKA4DC2yAKiSyO/QUcy7e+RR github.com/mitchellh/mapstructure v1.4.1/go.mod h1:bFUtVrKA4DC2yAKiSyO/QUcy7e+RRV2QTWOzhPopBRo= github.com/mitchellh/mapstructure v1.5.0 h1:jeMsZIYE/09sWLaz43PL7Gy6RuMjD2eJVyuac5Z2hdY= github.com/mitchellh/mapstructure v1.5.0/go.mod h1:bFUtVrKA4DC2yAKiSyO/QUcy7e+RRV2QTWOzhPopBRo= -github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd/go.mod h1:6dJC0mAP4ikYIbvyc7fijjWJddQyLn8Ig3JB5CqoB9Q= -github.com/modern-go/reflect2 v1.0.2/go.mod h1:yWuevngMOJpCy52FWWMvUC8ws7m/LJsjYzDa0/r8luk= github.com/montanaflynn/stats v0.0.0-20171201202039-1bf9dbcd8cbe/go.mod h1:wL8QJuTMNUDYhXwkmfOly8iTdp5TEcJFWZD2D7SIkUc= -github.com/mwitkow/go-conntrack v0.0.0-20190716064945-2f068394615f/go.mod h1:qRWi+5nqEBWmkhHvq77mSJWrCKwh8bxhgT7d/eI7P4U= github.com/nbutton23/zxcvbn-go v0.0.0-20210217022336-fa2cb2858354 h1:4kuARK6Y6FxaNu/BnU2OAaLF86eTVhP2hjTB6iMvItA= github.com/nbutton23/zxcvbn-go v0.0.0-20210217022336-fa2cb2858354/go.mod h1:KSVJerMDfblTH7p5MZaTt+8zaT2iEk3AkVb9PQdZuE8= github.com/niemeyer/pretty v0.0.0-20200227124842-a10e7caefd8e/go.mod h1:zD1mROLANZcx1PVRCS0qkT7pwLkGfwJo4zjcN/Tysno= @@ -304,8 +298,6 @@ github.com/sirupsen/logrus v1.4.2/go.mod h1:tLMulIdttU9McNUspp0xgXVQah82FyeX6Mwd github.com/sirupsen/logrus v1.9.0 h1:trlNQbNUG3OdDrDil03MCb1H2o9nJ1x4/5LYw7byDE0= github.com/sirupsen/logrus v1.9.0/go.mod h1:naHLuLoDiP4jHNo9R0sCBMtWGeIprob74mVsIT4qYEQ= github.com/spf13/cobra v0.0.3/go.mod h1:1l0Ry5zgKvJasoi3XT1TypsSe7PqH0Sj9dhYf7v3XqQ= -github.com/spf13/cobra v1.7.0 h1:hyqWnYt1ZQShIddO5kBpj3vu05/++x6tJ6dg8EC572I= -github.com/spf13/cobra v1.7.0/go.mod h1:uLxZILRyS/50WlhOIKD7W6V5bgeIt+4sICxh6uRMrb0= github.com/spf13/cobra v1.7.1-0.20230723113155-fd865a44e3c4 h1:6be13R0JVLZN659yPzYYO0O1nYeSByDy5eqi85JKG/Y= github.com/spf13/cobra v1.7.1-0.20230723113155-fd865a44e3c4/go.mod h1:uLxZILRyS/50WlhOIKD7W6V5bgeIt+4sICxh6uRMrb0= github.com/spf13/pflag v1.0.3/go.mod h1:DYY7MBk1bdzusC3SYhjObp+wFpr4gzcvqqNjLnInEg4= diff --git a/runner/organizations.go b/runner/organizations.go index 4482372d..cb74888c 100644 --- a/runner/organizations.go +++ b/runner/organizations.go @@ -124,7 +124,7 @@ func (r *Runner) GetOrganizationByID(ctx context.Context, orgID string) (params. return org, nil } -func (r *Runner) DeleteOrganization(ctx context.Context, orgID string) error { +func (r *Runner) DeleteOrganization(ctx context.Context, orgID string, keepWebhook bool) error { if !auth.IsAdmin(ctx) { return runnerErrors.ErrUnauthorized } @@ -148,14 +148,16 @@ func (r *Runner) DeleteOrganization(ctx context.Context, orgID string) error { return runnerErrors.NewBadRequestError("org has pools defined (%s)", strings.Join(poolIds, ", ")) } - poolMgr, err := r.poolManagerCtrl.GetOrgPoolManager(org) - if err != nil { - return errors.Wrap(err, "fetching pool manager") - } + if !keepWebhook { + poolMgr, err := r.poolManagerCtrl.GetOrgPoolManager(org) + if err != nil { + return errors.Wrap(err, "fetching pool manager") + } - if err := poolMgr.UninstallWebhook(ctx); err != nil { - // TODO(gabriel-samfira): Should we error out here? - log.Printf("failed to uninstall webhook: %s", err) + if err := poolMgr.UninstallWebhook(ctx); err != nil { + // TODO(gabriel-samfira): Should we error out here? + log.Printf("failed to uninstall webhook: %s", err) + } } if err := r.poolManagerCtrl.DeleteOrgPoolManager(org); err != nil { diff --git a/runner/organizations_test.go b/runner/organizations_test.go index f6c1e170..3aa3e427 100644 --- a/runner/organizations_test.go +++ b/runner/organizations_test.go @@ -254,7 +254,7 @@ func (s *OrgTestSuite) TestGetOrganizationByIDErrUnauthorized() { func (s *OrgTestSuite) TestDeleteOrganization() { s.Fixtures.PoolMgrCtrlMock.On("DeleteOrgPoolManager", mock.AnythingOfType("params.Organization")).Return(nil) - err := s.Runner.DeleteOrganization(s.Fixtures.AdminContext, s.Fixtures.StoreOrgs["test-org-3"].ID) + err := s.Runner.DeleteOrganization(s.Fixtures.AdminContext, s.Fixtures.StoreOrgs["test-org-3"].ID, true) s.Fixtures.PoolMgrCtrlMock.AssertExpectations(s.T()) s.Require().Nil(err) @@ -264,7 +264,7 @@ func (s *OrgTestSuite) TestDeleteOrganization() { } func (s *OrgTestSuite) TestDeleteOrganizationErrUnauthorized() { - err := s.Runner.DeleteOrganization(context.Background(), "dummy-org-id") + err := s.Runner.DeleteOrganization(context.Background(), "dummy-org-id", true) s.Require().Equal(runnerErrors.ErrUnauthorized, err) } @@ -275,7 +275,7 @@ func (s *OrgTestSuite) TestDeleteOrganizationPoolDefinedFailed() { s.FailNow(fmt.Sprintf("cannot create store organizations pool: %v", err)) } - err = s.Runner.DeleteOrganization(s.Fixtures.AdminContext, s.Fixtures.StoreOrgs["test-org-1"].ID) + err = s.Runner.DeleteOrganization(s.Fixtures.AdminContext, s.Fixtures.StoreOrgs["test-org-1"].ID, true) s.Require().Equal(runnerErrors.NewBadRequestError("org has pools defined (%s)", pool.ID), err) } @@ -283,7 +283,7 @@ func (s *OrgTestSuite) TestDeleteOrganizationPoolDefinedFailed() { func (s *OrgTestSuite) TestDeleteOrganizationPoolMgrFailed() { s.Fixtures.PoolMgrCtrlMock.On("DeleteOrgPoolManager", mock.AnythingOfType("params.Organization")).Return(s.Fixtures.ErrMock) - err := s.Runner.DeleteOrganization(s.Fixtures.AdminContext, s.Fixtures.StoreOrgs["test-org-1"].ID) + err := s.Runner.DeleteOrganization(s.Fixtures.AdminContext, s.Fixtures.StoreOrgs["test-org-1"].ID, true) s.Fixtures.PoolMgrCtrlMock.AssertExpectations(s.T()) s.Require().Equal(fmt.Sprintf("deleting org pool manager: %s", s.Fixtures.ErrMock.Error()), err.Error()) diff --git a/runner/repositories.go b/runner/repositories.go index 436c32b7..512c21e4 100644 --- a/runner/repositories.go +++ b/runner/repositories.go @@ -123,7 +123,7 @@ func (r *Runner) GetRepositoryByID(ctx context.Context, repoID string) (params.R return repo, nil } -func (r *Runner) DeleteRepository(ctx context.Context, repoID string) error { +func (r *Runner) DeleteRepository(ctx context.Context, repoID string, keepWebhook bool) error { if !auth.IsAdmin(ctx) { return runnerErrors.ErrUnauthorized } @@ -147,14 +147,16 @@ func (r *Runner) DeleteRepository(ctx context.Context, repoID string) error { return runnerErrors.NewBadRequestError("repo has pools defined (%s)", strings.Join(poolIds, ", ")) } - poolMgr, err := r.poolManagerCtrl.GetRepoPoolManager(repo) - if err != nil { - return errors.Wrap(err, "fetching pool manager") - } + if !keepWebhook { + poolMgr, err := r.poolManagerCtrl.GetRepoPoolManager(repo) + if err != nil { + return errors.Wrap(err, "fetching pool manager") + } - if err := poolMgr.UninstallWebhook(ctx); err != nil { - // TODO(gabriel-samfira): Should we error out here? - log.Printf("failed to uninstall webhook: %s", err) + if err := poolMgr.UninstallWebhook(ctx); err != nil { + // TODO(gabriel-samfira): Should we error out here? + log.Printf("failed to uninstall webhook: %s", err) + } } if err := r.poolManagerCtrl.DeleteRepoPoolManager(repo); err != nil { diff --git a/runner/repositories_test.go b/runner/repositories_test.go index 0b2c527a..4c918124 100644 --- a/runner/repositories_test.go +++ b/runner/repositories_test.go @@ -257,7 +257,7 @@ func (s *RepoTestSuite) TestGetRepositoryByIDErrUnauthorized() { func (s *RepoTestSuite) TestDeleteRepository() { s.Fixtures.PoolMgrCtrlMock.On("DeleteRepoPoolManager", mock.AnythingOfType("params.Repository")).Return(nil) - err := s.Runner.DeleteRepository(s.Fixtures.AdminContext, s.Fixtures.StoreRepos["test-repo-1"].ID) + err := s.Runner.DeleteRepository(s.Fixtures.AdminContext, s.Fixtures.StoreRepos["test-repo-1"].ID, true) s.Fixtures.PoolMgrCtrlMock.AssertExpectations(s.T()) s.Require().Nil(err) @@ -267,7 +267,7 @@ func (s *RepoTestSuite) TestDeleteRepository() { } func (s *RepoTestSuite) TestDeleteRepositoryErrUnauthorized() { - err := s.Runner.DeleteRepository(context.Background(), "dummy-repo-id") + err := s.Runner.DeleteRepository(context.Background(), "dummy-repo-id", true) s.Require().Equal(runnerErrors.ErrUnauthorized, err) } @@ -278,7 +278,7 @@ func (s *RepoTestSuite) TestDeleteRepositoryPoolDefinedFailed() { s.FailNow(fmt.Sprintf("cannot create store repositories pool: %v", err)) } - err = s.Runner.DeleteRepository(s.Fixtures.AdminContext, s.Fixtures.StoreRepos["test-repo-1"].ID) + err = s.Runner.DeleteRepository(s.Fixtures.AdminContext, s.Fixtures.StoreRepos["test-repo-1"].ID, true) s.Require().Equal(runnerErrors.NewBadRequestError("repo has pools defined (%s)", pool.ID), err) } @@ -286,7 +286,7 @@ func (s *RepoTestSuite) TestDeleteRepositoryPoolDefinedFailed() { func (s *RepoTestSuite) TestDeleteRepositoryPoolMgrFailed() { s.Fixtures.PoolMgrCtrlMock.On("DeleteRepoPoolManager", mock.AnythingOfType("params.Repository")).Return(s.Fixtures.ErrMock) - err := s.Runner.DeleteRepository(s.Fixtures.AdminContext, s.Fixtures.StoreRepos["test-repo-1"].ID) + err := s.Runner.DeleteRepository(s.Fixtures.AdminContext, s.Fixtures.StoreRepos["test-repo-1"].ID, true) s.Fixtures.PoolMgrCtrlMock.AssertExpectations(s.T()) s.Require().Equal(fmt.Sprintf("deleting repo pool manager: %s", s.Fixtures.ErrMock.Error()), err.Error())