diff --git a/internal/services/configstore/action/projectgroup.go b/internal/services/configstore/action/projectgroup.go index c44b11c..a17b634 100644 --- a/internal/services/configstore/action/projectgroup.go +++ b/internal/services/configstore/action/projectgroup.go @@ -18,6 +18,7 @@ import ( "context" "encoding/json" "path" + "strings" "agola.io/agola/internal/datamanager" "agola.io/agola/internal/db" @@ -28,6 +29,24 @@ import ( errors "golang.org/x/xerrors" ) +func (h *ActionHandler) GetProjectGroup(ctx context.Context, projectGroupRef string) (*types.ProjectGroup, error) { + var projectGroup *types.ProjectGroup + err := h.readDB.Do(ctx, func(tx *db.Tx) error { + var err error + projectGroup, err = h.readDB.GetProjectGroup(tx, projectGroupRef) + return err + }) + if err != nil { + return nil, err + } + + if projectGroup == nil { + return nil, util.NewErrNotFound(errors.Errorf("project group %q doesn't exist", projectGroupRef)) + } + + return projectGroup, nil +} + func (h *ActionHandler) GetProjectGroupSubgroups(ctx context.Context, projectGroupRef string) ([]*types.ProjectGroup, error) { var projectGroups []*types.ProjectGroup err := h.readDB.Do(ctx, func(tx *db.Tx) error { @@ -109,6 +128,10 @@ func (h *ActionHandler) CreateProjectGroup(ctx context.Context, projectGroup *ty return nil, err } + if projectGroup.Parent.Type != types.ConfigTypeProjectGroup { + return nil, util.NewErrBadRequest(errors.Errorf("wrong project group parent type %q", projectGroup.Parent.Type)) + } + var cgt *datamanager.ChangeGroupsUpdateToken // must do all the checks in a single transaction to avoid concurrent changes @@ -120,6 +143,9 @@ func (h *ActionHandler) CreateProjectGroup(ctx context.Context, projectGroup *ty if parentProjectGroup == nil { return util.NewErrBadRequest(errors.Errorf("project group with id %q doesn't exist", projectGroup.Parent.ID)) } + // TODO(sgotti) now we are doing a very ugly thing setting the request + // projectgroup parent ID that can be both an ID or a ref. Then we are fixing + // it to an ID here. Change the request format to avoid this. projectGroup.Parent.ID = parentProjectGroup.ID groupPath, err := h.readDB.GetProjectGroupPath(tx, parentProjectGroup) @@ -194,14 +220,28 @@ func (h *ActionHandler) UpdateProjectGroup(ctx context.Context, req *UpdateProje if pg == nil { return util.NewErrBadRequest(errors.Errorf("project group with ref %q doesn't exist", req.ProjectGroupRef)) } - // check that the project.ID matches + // check that the project group ID matches if pg.ID != req.ProjectGroup.ID { return util.NewErrBadRequest(errors.Errorf("project group with ref %q has a different id", req.ProjectGroupRef)) } - // check parent exists - switch pg.Parent.Type { + if pg.Parent.Type != req.ProjectGroup.Parent.Type { + return util.NewErrBadRequest(errors.Errorf("changing project group parent type isn't supported")) + } + + switch req.ProjectGroup.Parent.Type { + case types.ConfigTypeOrg: + fallthrough + case types.ConfigTypeUser: + // Cannot update root project group parent + if pg.Parent.Type != req.ProjectGroup.Parent.Type || pg.Parent.ID != req.ProjectGroup.Parent.ID { + return util.NewErrBadRequest(errors.Errorf("cannot change root project group parent type or id")) + } + // if the project group is a root project group force the name to be empty + req.ProjectGroup.Name = "" + case types.ConfigTypeProjectGroup: + // check parent exists group, err := h.readDB.GetProjectGroup(tx, req.ProjectGroup.Parent.ID) if err != nil { return err @@ -209,30 +249,25 @@ func (h *ActionHandler) UpdateProjectGroup(ctx context.Context, req *UpdateProje if group == nil { return util.NewErrBadRequest(errors.Errorf("project group with id %q doesn't exist", req.ProjectGroup.Parent.ID)) } + // TODO(sgotti) now we are doing a very ugly thing setting the request + // projectgroup parent ID that can be both an ID or a ref. Then we are fixing + // it to an ID here. Change the request format to avoid this. + req.ProjectGroup.Parent.ID = group.ID } - // currently we don't support changing parent - // TODO(sgotti) handle project move (changed parent project group) - if pg.Parent.Type != req.ProjectGroup.Parent.Type { - return util.NewErrBadRequest(errors.Errorf("changing project group parent isn't supported")) - } - if pg.Parent.ID != req.ProjectGroup.Parent.ID { - return util.NewErrBadRequest(errors.Errorf("changing project group parent isn't supported")) - } - - // if the project group is a root project group force the name to be empty - if pg.Parent.Type == types.ConfigTypeOrg || - pg.Parent.Type == types.ConfigTypeUser { - req.ProjectGroup.Name = "" - } - - pgPath, err := h.readDB.GetProjectGroupPath(tx, pg) + curPGParentPath, err := h.readDB.GetPath(tx, pg.Parent.Type, pg.Parent.ID) if err != nil { return err } - pgp := path.Join(path.Dir(pgPath), req.ProjectGroup.Name) + curPGP := path.Join(curPGParentPath, pg.Name) - if pg.Name != req.ProjectGroup.Name { + pgParentPath, err := h.readDB.GetPath(tx, req.ProjectGroup.Parent.Type, req.ProjectGroup.Parent.ID) + if err != nil { + return err + } + pgp := path.Join(pgParentPath, req.ProjectGroup.Name) + + if pg.Name != req.ProjectGroup.Name || pg.Parent.ID != req.ProjectGroup.Parent.ID { // check duplicate project group name ap, err := h.readDB.GetProjectGroupByName(tx, req.ProjectGroup.Parent.ID, req.ProjectGroup.Name) if err != nil { @@ -241,11 +276,21 @@ func (h *ActionHandler) UpdateProjectGroup(ctx context.Context, req *UpdateProje if ap != nil { return util.NewErrBadRequest(errors.Errorf("project group with name %q, path %q already exists", req.ProjectGroup.Name, pgp)) } + // Cannot move inside itself or a child project group + if strings.HasPrefix(pgp, curPGP+"/") { + return util.NewErrBadRequest(errors.Errorf("cannot move project group inside itself or child project group")) + } } // changegroup is the project group path. Use "projectpath" prefix as it must // cover both projects and projectgroups cgNames := []string{util.EncodeSha256Hex("projectpath-" + pgp)} + + // add new projectpath + if pg.Parent.ID != req.ProjectGroup.Parent.ID { + cgNames = append(cgNames, util.EncodeSha256Hex("projectpath-"+pgp)) + } + cgt, err = h.readDB.GetChangeGroupsUpdateTokens(tx, cgNames) if err != nil { return err diff --git a/internal/services/configstore/api/projectgroup.go b/internal/services/configstore/api/projectgroup.go index 82a0652..264dfd1 100644 --- a/internal/services/configstore/api/projectgroup.go +++ b/internal/services/configstore/api/projectgroup.go @@ -30,7 +30,6 @@ import ( "github.com/gorilla/mux" "go.uber.org/zap" - errors "golang.org/x/xerrors" ) func projectGroupResponse(ctx context.Context, readDB *readdb.ReadDB, projectGroup *types.ProjectGroup) (*csapitypes.ProjectGroup, error) { @@ -82,11 +81,12 @@ func projectGroupsResponse(ctx context.Context, readDB *readdb.ReadDB, projectGr type ProjectGroupHandler struct { log *zap.SugaredLogger + ah *action.ActionHandler readDB *readdb.ReadDB } -func NewProjectGroupHandler(logger *zap.Logger, readDB *readdb.ReadDB) *ProjectGroupHandler { - return &ProjectGroupHandler{log: logger.Sugar(), readDB: readDB} +func NewProjectGroupHandler(logger *zap.Logger, ah *action.ActionHandler, readDB *readdb.ReadDB) *ProjectGroupHandler { + return &ProjectGroupHandler{log: logger.Sugar(), ah: ah, readDB: readDB} } func (h *ProjectGroupHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { @@ -99,20 +99,9 @@ func (h *ProjectGroupHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) return } - var projectGroup *types.ProjectGroup - err = h.readDB.Do(ctx, func(tx *db.Tx) error { - var err error - projectGroup, err = h.readDB.GetProjectGroup(tx, projectGroupRef) - return err - }) - if err != nil { + projectGroup, err := h.ah.GetProjectGroup(ctx, projectGroupRef) + if httpError(w, err) { h.log.Errorf("err: %+v", err) - httpError(w, err) - return - } - - if projectGroup == nil { - httpError(w, util.NewErrNotFound(errors.Errorf("project group %q doesn't exist", projectGroupRef))) return } diff --git a/internal/services/configstore/configstore.go b/internal/services/configstore/configstore.go index f919bf5..2e6ea6b 100644 --- a/internal/services/configstore/configstore.go +++ b/internal/services/configstore/configstore.go @@ -177,7 +177,7 @@ func (s *Configstore) setupDefaultRouter() http.Handler { maintenanceModeHandler := api.NewMaintenanceModeHandler(logger, s.ah, s.e) exportHandler := api.NewExportHandler(logger, s.ah) - projectGroupHandler := api.NewProjectGroupHandler(logger, s.readDB) + projectGroupHandler := api.NewProjectGroupHandler(logger, s.ah, s.readDB) projectGroupSubgroupsHandler := api.NewProjectGroupSubgroupsHandler(logger, s.ah, s.readDB) projectGroupProjectsHandler := api.NewProjectGroupProjectsHandler(logger, s.ah, s.readDB) createProjectGroupHandler := api.NewCreateProjectGroupHandler(logger, s.ah, s.readDB) diff --git a/internal/services/configstore/configstore_test.go b/internal/services/configstore/configstore_test.go index 3ab766f..0a09bfd 100644 --- a/internal/services/configstore/configstore_test.go +++ b/internal/services/configstore/configstore_test.go @@ -831,6 +831,156 @@ func TestProjectUpdate(t *testing.T) { }) } +func TestProjectGroupUpdate(t *testing.T) { + dir, err := ioutil.TempDir("", "agola") + if err != nil { + t.Fatalf("unexpected err: %v", err) + } + defer os.RemoveAll(dir) + + ctx := context.Background() + + cs, tetcd := setupConfigstore(ctx, t, dir) + defer shutdownEtcd(tetcd) + + t.Logf("starting cs") + go func() { + _ = cs.Run(ctx) + }() + + // TODO(sgotti) change the sleep with a real check that all is ready + time.Sleep(2 * time.Second) + + user, err := cs.ah.CreateUser(ctx, &action.CreateUserRequest{UserName: "user01"}) + if err != nil { + t.Fatalf("unexpected err: %v", err) + } + + // TODO(sgotti) change the sleep with a real check that user is in readdb + time.Sleep(2 * time.Second) + + pg01 := &types.ProjectGroup{Name: "pg01", Parent: types.Parent{Type: types.ConfigTypeProjectGroup, ID: path.Join("user", user.Name)}, Visibility: types.VisibilityPublic} + pg01, err = cs.ah.CreateProjectGroup(ctx, pg01) + if err != nil { + t.Fatalf("unexpected err: %v", err) + } + pg02 := &types.ProjectGroup{Name: "pg02", Parent: types.Parent{Type: types.ConfigTypeProjectGroup, ID: path.Join("user", user.Name)}, Visibility: types.VisibilityPublic} + pg02, err = cs.ah.CreateProjectGroup(ctx, pg02) + if err != nil { + t.Fatalf("unexpected err: %v", err) + } + pg03 := &types.ProjectGroup{Name: "pg03", Parent: types.Parent{Type: types.ConfigTypeProjectGroup, ID: path.Join("user", user.Name)}, Visibility: types.VisibilityPublic} + pg03, err = cs.ah.CreateProjectGroup(ctx, pg03) + if err != nil { + t.Fatalf("unexpected err: %v", err) + } + pg04 := &types.ProjectGroup{Name: "pg01", Parent: types.Parent{Type: types.ConfigTypeProjectGroup, ID: path.Join("user", user.Name, "pg01")}, Visibility: types.VisibilityPublic} + _, err = cs.ah.CreateProjectGroup(ctx, pg04) + if err != nil { + t.Fatalf("unexpected err: %v", err) + } + pg05 := &types.ProjectGroup{Name: "pg01", Parent: types.Parent{Type: types.ConfigTypeProjectGroup, ID: path.Join("user", user.Name, "pg02")}, Visibility: types.VisibilityPublic} + pg05, err = cs.ah.CreateProjectGroup(ctx, pg05) + if err != nil { + t.Fatalf("unexpected err: %v", err) + } + + t.Run("rename project group keeping same parent", func(t *testing.T) { + projectGroupName := "pg03" + pg03.Name = "newpg03" + _, err := cs.ah.UpdateProjectGroup(ctx, &action.UpdateProjectGroupRequest{ProjectGroupRef: path.Join("user", user.Name, projectGroupName), ProjectGroup: pg03}) + if err != nil { + t.Fatalf("unexpected err: %v", err) + } + }) + t.Run("move project to project group having project with same name", func(t *testing.T) { + projectGroupName := "pg01" + expectedErr := fmt.Sprintf("project group with name %q, path %q already exists", projectGroupName, path.Join("user", user.Name, projectGroupName)) + pg05.Parent.ID = path.Join("user", user.Name) + _, err := cs.ah.UpdateProjectGroup(ctx, &action.UpdateProjectGroupRequest{ProjectGroupRef: path.Join("user", user.Name, "pg02", projectGroupName), ProjectGroup: pg05}) + if err.Error() != expectedErr { + t.Fatalf("expected err %v, got err: %v", expectedErr, err) + } + }) + t.Run("move project group to project group changing name", func(t *testing.T) { + projectGroupName := "pg01" + pg05.Name = "newpg01" + pg05.Parent.ID = path.Join("user", user.Name) + _, err := cs.ah.UpdateProjectGroup(ctx, &action.UpdateProjectGroupRequest{ProjectGroupRef: path.Join("user", user.Name, "pg02", projectGroupName), ProjectGroup: pg05}) + if err != nil { + t.Fatalf("unexpected err: %v", err) + } + }) + t.Run("move project group inside itself", func(t *testing.T) { + projectGroupName := "pg02" + expectedErr := "cannot move project group inside itself or child project group" + pg02.Parent.ID = path.Join("user", user.Name, "pg02") + _, err := cs.ah.UpdateProjectGroup(ctx, &action.UpdateProjectGroupRequest{ProjectGroupRef: path.Join("user", user.Name, projectGroupName), ProjectGroup: pg02}) + if err.Error() != expectedErr { + t.Fatalf("expected err %v, got err: %v", expectedErr, err) + } + }) + t.Run("move project group to child project group", func(t *testing.T) { + projectGroupName := "pg01" + expectedErr := "cannot move project group inside itself or child project group" + pg01.Parent.ID = path.Join("user", user.Name, "pg01", "pg01") + _, err := cs.ah.UpdateProjectGroup(ctx, &action.UpdateProjectGroupRequest{ProjectGroupRef: path.Join("user", user.Name, projectGroupName), ProjectGroup: pg01}) + if err.Error() != expectedErr { + t.Fatalf("expected err %v, got err: %v", expectedErr, err) + } + }) + t.Run("change root project group parent", func(t *testing.T) { + + var rootPG *types.ProjectGroup + rootPG, err := cs.ah.GetProjectGroup(ctx, path.Join("user", user.Name)) + if err != nil { + t.Fatalf("unexpected err: %v", err) + } + rootPG.Parent.ID = path.Join("user", user.Name, "pg01") + + expectedErr := "cannot change root project group parent type or id" + _, err = cs.ah.UpdateProjectGroup(ctx, &action.UpdateProjectGroupRequest{ProjectGroupRef: path.Join("user", user.Name), ProjectGroup: rootPG}) + if err.Error() != expectedErr { + t.Fatalf("expected err %v, got err: %v", expectedErr, err) + } + }) + t.Run("change root project group name", func(t *testing.T) { + var rootPG *types.ProjectGroup + rootPG, err := cs.ah.GetProjectGroup(ctx, path.Join("user", user.Name)) + if err != nil { + t.Fatalf("unexpected err: %v", err) + } + rootPG.Name = "rootpgnewname" + + expectedErr := "project group name for root project group must be empty" + _, err = cs.ah.UpdateProjectGroup(ctx, &action.UpdateProjectGroupRequest{ProjectGroupRef: path.Join("user", user.Name), ProjectGroup: rootPG}) + if err.Error() != expectedErr { + t.Fatalf("expected err %v, got err: %v", expectedErr, err) + } + }) + t.Run("change root project group visibility", func(t *testing.T) { + var rootPG *types.ProjectGroup + rootPG, err := cs.ah.GetProjectGroup(ctx, path.Join("user", user.Name)) + if err != nil { + t.Fatalf("unexpected err: %v", err) + } + rootPG.Visibility = types.VisibilityPrivate + + _, err = cs.ah.UpdateProjectGroup(ctx, &action.UpdateProjectGroupRequest{ProjectGroupRef: path.Join("user", user.Name), ProjectGroup: rootPG}) + if err != nil { + t.Fatalf("unexpected err: %v", err) + } + + rootPG, err = cs.ah.GetProjectGroup(ctx, path.Join("user", user.Name)) + if err != nil { + t.Fatalf("unexpected err: %v", err) + } + if rootPG.Visibility != types.VisibilityPrivate { + t.Fatalf("expected visiblity %q, got visibility: %q", types.VisibilityPublic, rootPG.Visibility) + } + }) +} + func TestProjectGroupDelete(t *testing.T) { dir, err := ioutil.TempDir("", "agola") if err != nil {