Skip to content

Commit

Permalink
Add keepWebhook flag when deleting entities
Browse files Browse the repository at this point in the history
Signed-off-by: Gabriel Adrian Samfira <[email protected]>
  • Loading branch information
gabriel-samfira committed Aug 16, 2023
1 parent 762a3ee commit ce88f62
Show file tree
Hide file tree
Showing 7 changed files with 30 additions and 34 deletions.
2 changes: 1 addition & 1 deletion apiserver/controllers/organizations.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion apiserver/controllers/repositories.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 0 additions & 8 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -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=
Expand Down Expand Up @@ -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=
Expand Down Expand Up @@ -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=
Expand Down Expand Up @@ -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=
Expand Down
18 changes: 10 additions & 8 deletions runner/organizations.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand All @@ -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 {
Expand Down
8 changes: 4 additions & 4 deletions runner/organizations_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)
}
Expand All @@ -275,15 +275,15 @@ 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)
}

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())
Expand Down
18 changes: 10 additions & 8 deletions runner/repositories.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand All @@ -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 {
Expand Down
8 changes: 4 additions & 4 deletions runner/repositories_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)
}
Expand All @@ -278,15 +278,15 @@ 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)
}

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())
Expand Down

0 comments on commit ce88f62

Please sign in to comment.