From 0df1335855307b5c700f52dae68eb07d5fee9976 Mon Sep 17 00:00:00 2001 From: Atefeh Mohseni-Ejiyeh Date: Thu, 8 Jun 2023 00:09:34 +0000 Subject: [PATCH] [HARNESS] Delete Space on Harness Account/Org/Project deletion (#83) --- Makefile | 8 ++++- cli/server/harness.wire.go | 6 ++-- cli/server/harness.wire_gen.go | 13 +++++++-- cli/server/standalone.wire_gen.go | 1 - gitrpc/internal/service/repo.go | 3 +- gitrpc/repo.go | 1 - internal/api/controller/repo/create.go | 3 +- internal/api/controller/space/delete.go | 29 ++++++++++--------- .../controller/space/delete_repositories.go | 2 +- internal/api/handler/space/delete.go | 3 +- internal/api/usererror/translate.go | 2 +- internal/router/api.go | 6 ++-- mocks/mock_client.go | 3 +- mocks/mock_store.go | 3 +- 14 files changed, 48 insertions(+), 35 deletions(-) diff --git a/Makefile b/Makefile index c384f64a6..26c0970ca 100644 --- a/Makefile +++ b/Makefile @@ -157,7 +157,13 @@ proto: --go-grpc_opt=paths=source_relative \ ./gitrpc/proto/*.proto - +harness-proto: + @protoc --proto_path=./harness/proto \ + --go_out=./harness/rpc \ + --go_opt=paths=source_relative \ + --go-grpc_out=./harness/rpc \ + --go-grpc_opt=paths=source_relative \ + ./harness/proto/*.proto ########################################### # Install Tools and deps # diff --git a/cli/server/harness.wire.go b/cli/server/harness.wire.go index 47c285c81..f31cfc305 100644 --- a/cli/server/harness.wire.go +++ b/cli/server/harness.wire.go @@ -19,6 +19,8 @@ import ( "github.com/harness/gitness/harness/bootstrap" "github.com/harness/gitness/harness/client" "github.com/harness/gitness/harness/router" + harnessservices "github.com/harness/gitness/harness/services" + harnessevents "github.com/harness/gitness/harness/services/events" "github.com/harness/gitness/harness/store" "github.com/harness/gitness/harness/types/check" "github.com/harness/gitness/internal/api/controller/githook" @@ -33,7 +35,6 @@ import ( gitevents "github.com/harness/gitness/internal/events/git" pullreqevents "github.com/harness/gitness/internal/events/pullreq" "github.com/harness/gitness/internal/server" - "github.com/harness/gitness/internal/services" "github.com/harness/gitness/internal/services/codecomments" pullreqservice "github.com/harness/gitness/internal/services/pullreq" "github.com/harness/gitness/internal/services/webhook" @@ -51,11 +52,12 @@ func initSystem(ctx context.Context, config *gitnesstypes.Config) (*system, erro wire.Build( newSystem, ProvideHarnessConfig, + harnessevents.WireSet, ProvideRedis, bootstrap.WireSet, database.WireSet, pullreqservice.WireSet, - services.WireSet, + harnessservices.WireSet, cache.WireSet, server.WireSet, url.WireSet, diff --git a/cli/server/harness.wire_gen.go b/cli/server/harness.wire_gen.go index a19c9460c..29836fec5 100644 --- a/cli/server/harness.wire_gen.go +++ b/cli/server/harness.wire_gen.go @@ -7,7 +7,6 @@ package server import ( "context" - "github.com/harness/gitness/events" "github.com/harness/gitness/gitrpc" server2 "github.com/harness/gitness/gitrpc/server" @@ -17,6 +16,8 @@ import ( "github.com/harness/gitness/harness/bootstrap" "github.com/harness/gitness/harness/client" "github.com/harness/gitness/harness/router" + "github.com/harness/gitness/harness/services" + events4 "github.com/harness/gitness/harness/services/events" "github.com/harness/gitness/harness/store" "github.com/harness/gitness/harness/types/check" "github.com/harness/gitness/internal/api/controller/githook" @@ -32,7 +33,6 @@ import ( events2 "github.com/harness/gitness/internal/events/pullreq" router2 "github.com/harness/gitness/internal/router" "github.com/harness/gitness/internal/server" - "github.com/harness/gitness/internal/services" "github.com/harness/gitness/internal/services/codecomments" pullreq2 "github.com/harness/gitness/internal/services/pullreq" "github.com/harness/gitness/internal/services/webhook" @@ -186,7 +186,14 @@ func initSystem(ctx context.Context, config *types.Config) (*system, error) { if err != nil { return nil, err } - servicesServices := services.ProvideServices(webhookService, pullreqService) + eventsService, err := events4.ProvideEventService(ctx, typesConfig, spaceController, config) + if err != nil { + return nil, err + } + servicesServices, err := services.ProvideServices(ctx, webhookService, pullreqService, eventsService) + if err != nil { + return nil, err + } serverSystem := newSystem(bootstrapBootstrap, serverServer, server3, manager, servicesServices) return serverSystem, nil } diff --git a/cli/server/standalone.wire_gen.go b/cli/server/standalone.wire_gen.go index 08d6e0c9b..d6a229b1d 100644 --- a/cli/server/standalone.wire_gen.go +++ b/cli/server/standalone.wire_gen.go @@ -7,7 +7,6 @@ package server import ( "context" - "github.com/harness/gitness/events" "github.com/harness/gitness/gitrpc" server2 "github.com/harness/gitness/gitrpc/server" diff --git a/gitrpc/internal/service/repo.go b/gitrpc/internal/service/repo.go index a9276b6cc..8e9f5a169 100644 --- a/gitrpc/internal/service/repo.go +++ b/gitrpc/internal/service/repo.go @@ -219,8 +219,7 @@ func (s RepositoryService) DeleteRepository( } repoPath := getFullPathForRepo(s.reposRoot, base.RepoUid) - // check if directory exists - // if dir does not exist already fail silently + if _, err := os.Stat(repoPath); err != nil && os.IsNotExist(err) { return nil, ErrNotFound(err) } else if err != nil { diff --git a/gitrpc/repo.go b/gitrpc/repo.go index c1417c215..a1b3a58c9 100644 --- a/gitrpc/repo.go +++ b/gitrpc/repo.go @@ -122,7 +122,6 @@ func (c *Client) DeleteRepository(ctx context.Context, params *DeleteRepositoryP if params == nil { return ErrNoParamsProvided } - _, err := c.repoService.DeleteRepository(ctx, &rpc.DeleteRepositoryRequest{ Base: mapToRPCWriteRequest(params.WriteParams), }) diff --git a/internal/api/controller/repo/create.go b/internal/api/controller/repo/create.go index 30ee25887..5aaa6d40f 100644 --- a/internal/api/controller/repo/create.go +++ b/internal/api/controller/repo/create.go @@ -87,8 +87,7 @@ func (c *Controller) Create(ctx context.Context, session *auth.Session, in *Crea // cleanup git repo if we fail to create repo in db (best effort deletion) defer func() { if dberr != nil { - err := c.DeleteRepositoryRPC(ctx, session, repo) - if err != nil { + if err := c.DeleteRepositoryRPC(ctx, session, repo); err != nil { log.Ctx(ctx).Warn().Err(err).Msg("gitrpc failed to delete repo for cleanup") } } diff --git a/internal/api/controller/space/delete.go b/internal/api/controller/space/delete.go index 1b8f7d9a5..63a22a6db 100644 --- a/internal/api/controller/space/delete.go +++ b/internal/api/controller/space/delete.go @@ -24,30 +24,25 @@ func (c *Controller) Delete(ctx context.Context, session *auth.Session, spaceRef if err = apiauth.CheckSpace(ctx, c.authorizer, session, space, enum.PermissionSpaceDelete, false); err != nil { return err } - sfilter := &types.SpaceFilter{ + + return c.DeleteNoAuth(ctx, session, space.ID) +} + +// DeleteNoAuth bypasses PermissionSpaceDelete, PermissionSpaceView, PermissionRepoView, and PermissionRepoDelete. +func (c *Controller) DeleteNoAuth(ctx context.Context, session *auth.Session, spaceID int64) error { + filter := &types.SpaceFilter{ Page: 1, Size: int(math.MaxInt), Query: "", Order: enum.OrderAsc, Sort: enum.SpaceAttrNone, } - return c.DeleteNoAuth(ctx, session, space.ID, sfilter) -} - -// DeleteNoAuth bypasses these permission -// PermissionSpaceDelete, PermissionSpaceView, PermissionRepoView, PermissionRepoDelete. -func (c *Controller) DeleteNoAuth( - ctx context.Context, - session *auth.Session, - spaceID int64, - filter *types.SpaceFilter, -) error { subSpaces, _, err := c.ListSpacesNoAuth(ctx, spaceID, filter) if err != nil { return fmt.Errorf("failed to list space %d sub spaces: %w", spaceID, err) } for _, space := range subSpaces { - err = c.DeleteNoAuth(ctx, session, space.ID, filter) + err = c.DeleteNoAuth(ctx, session, space.ID) if err != nil { return fmt.Errorf("failed to delete space %d: %w", space.ID, err) } @@ -62,3 +57,11 @@ func (c *Controller) DeleteNoAuth( } return nil } + +func (c *Controller) DeleteWithPathNoAuth(ctx context.Context, session *auth.Session, spacePath string) error { + space, err := c.spaceStore.FindByRef(ctx, spacePath) + if err != nil { + return err + } + return c.DeleteNoAuth(ctx, session, space.ID) +} diff --git a/internal/api/controller/space/delete_repositories.go b/internal/api/controller/space/delete_repositories.go index d3c8f0fcd..cbb2cfec1 100644 --- a/internal/api/controller/space/delete_repositories.go +++ b/internal/api/controller/space/delete_repositories.go @@ -14,7 +14,7 @@ import ( "github.com/harness/gitness/types/enum" ) -// deleteRepositoriesNoAuth does not check PermissionRepoView, and PermissionRepoDelete permissions +// deleteRepositoriesNoAuth does not check PermissionRepoView, and PermissionRepoDelete permissions. // Call this through Delete(Space) api to make sure the caller has DeleteSpace permission. func (c *Controller) deleteRepositoriesNoAuth(ctx context.Context, session *auth.Session, spaceID int64) error { filter := &types.RepoFilter{ diff --git a/internal/api/handler/space/delete.go b/internal/api/handler/space/delete.go index 1c74d7a00..c93b2ead4 100644 --- a/internal/api/handler/space/delete.go +++ b/internal/api/handler/space/delete.go @@ -7,6 +7,7 @@ package space import ( "net/http" + "github.com/harness/gitness/internal/api/controller/repo" "github.com/harness/gitness/internal/api/controller/space" "github.com/harness/gitness/internal/api/render" "github.com/harness/gitness/internal/api/request" @@ -15,7 +16,7 @@ import ( /* * Deletes a space. */ -func HandleDelete(spaceCtrl *space.Controller) http.HandlerFunc { +func HandleDelete(spaceCtrl *space.Controller, repoCtrl *repo.Controller) http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { ctx := r.Context() session, _ := request.AuthSessionFrom(ctx) diff --git a/internal/api/usererror/translate.go b/internal/api/usererror/translate.go index 6e27f4db8..086d93f95 100644 --- a/internal/api/usererror/translate.go +++ b/internal/api/usererror/translate.go @@ -13,8 +13,8 @@ import ( "github.com/harness/gitness/internal/services/webhook" "github.com/harness/gitness/internal/store" "github.com/harness/gitness/types/check" - "github.com/harness/go-rbac" + "github.com/harness/go-rbac" "github.com/rs/zerolog/log" ) diff --git a/internal/router/api.go b/internal/router/api.go index 382979ff8..bc3bff094 100644 --- a/internal/router/api.go +++ b/internal/router/api.go @@ -109,7 +109,7 @@ func setupRoutesV1(r chi.Router, repoCtrl *repo.Controller, spaceCtrl *space.Controller, pullreqCtrl *pullreq.Controller, webhookCtrl *webhook.Controller, githookCtrl *githook.Controller, saCtrl *serviceaccount.Controller, userCtrl *user.Controller, principalCtrl *principal.Controller) { - setupSpaces(r, spaceCtrl) + setupSpaces(r, spaceCtrl, repoCtrl) setupRepos(r, repoCtrl, pullreqCtrl, webhookCtrl) setupUser(r, userCtrl) setupServiceAccounts(r, saCtrl) @@ -121,7 +121,7 @@ func setupRoutesV1(r chi.Router, setupResources(r) } -func setupSpaces(r chi.Router, spaceCtrl *space.Controller) { +func setupSpaces(r chi.Router, spaceCtrl *space.Controller, repoCtrl *repo.Controller) { r.Route("/spaces", func(r chi.Router) { // Create takes path and parentId via body, not uri r.Post("/", handlerspace.HandleCreate(spaceCtrl)) @@ -130,7 +130,7 @@ func setupSpaces(r chi.Router, spaceCtrl *space.Controller) { // space operations r.Get("/", handlerspace.HandleFind(spaceCtrl)) r.Patch("/", handlerspace.HandleUpdate(spaceCtrl)) - r.Delete("/", handlerspace.HandleDelete(spaceCtrl)) + r.Delete("/", handlerspace.HandleDelete(spaceCtrl, repoCtrl)) r.Post("/move", handlerspace.HandleMove(spaceCtrl)) r.Get("/spaces", handlerspace.HandleListSpaces(spaceCtrl)) diff --git a/mocks/mock_client.go b/mocks/mock_client.go index fbd985bd4..3216e04cb 100644 --- a/mocks/mock_client.go +++ b/mocks/mock_client.go @@ -8,10 +8,9 @@ import ( context "context" reflect "reflect" + gomock "github.com/golang/mock/gomock" user "github.com/harness/gitness/internal/api/controller/user" types "github.com/harness/gitness/types" - - gomock "github.com/golang/mock/gomock" ) // MockClient is a mock of Client interface. diff --git a/mocks/mock_store.go b/mocks/mock_store.go index 8ad9973bf..30b7ec550 100644 --- a/mocks/mock_store.go +++ b/mocks/mock_store.go @@ -8,10 +8,9 @@ import ( context "context" reflect "reflect" + gomock "github.com/golang/mock/gomock" types "github.com/harness/gitness/types" enum "github.com/harness/gitness/types/enum" - - gomock "github.com/golang/mock/gomock" ) // MockPrincipalStore is a mock of PrincipalStore interface.