Skip to content

Commit

Permalink
pull request import (harness#2121)
Browse files Browse the repository at this point in the history
* bypass lint
* bump golang version
* using types from migrator
* moving Importing to the RepositoryOutput
* minor changes; improved unit tests
* pr comments
* pr import
* pull request import
  • Loading branch information
marko-gacesa authored and Harness committed Jul 6, 2024
1 parent 30dfa92 commit 5427130
Show file tree
Hide file tree
Showing 34 changed files with 843 additions and 77 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/ci-lint.yml
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ jobs:
- uses: actions/checkout@v3
- uses: actions/setup-go@v4
with:
go-version: '1.20'
go-version: '1.22'
- name: get dependencies
run: |
mkdir -p ./web/dist
Expand Down
2 changes: 1 addition & 1 deletion Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ RUN yarn && yarn build && yarn cache clean
# ---------------------------------------------------------#
# Build gitness image #
# ---------------------------------------------------------#
FROM --platform=$BUILDPLATFORM golang:1.20-alpine3.18 as builder
FROM --platform=$BUILDPLATFORM golang:1.22-alpine3.18 as builder

RUN apk update \
&& apk add --no-cache protoc build-base git
Expand Down
5 changes: 5 additions & 0 deletions app/api/controller/githook/pre_receive.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,11 @@ func (c *Controller) PreReceive(
return hook.Output{}, err
}

if !in.Internal && repo.State != enum.RepoStateActive && repo.State != enum.RepoStateMigrateGitPush {
output.Error = ptr.String("Push not allowed in the current repository state")
return output, nil
}

if err := c.limiter.RepoSize(ctx, in.RepoID); err != nil {
return hook.Output{}, fmt.Errorf(
"resource limit exceeded: %w",
Expand Down
8 changes: 6 additions & 2 deletions app/api/controller/pullreq/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import (
pullreqevents "github.com/harness/gitness/app/events/pullreq"
"github.com/harness/gitness/app/services/codecomments"
"github.com/harness/gitness/app/services/codeowners"
"github.com/harness/gitness/app/services/importer"
locker "github.com/harness/gitness/app/services/locker"
"github.com/harness/gitness/app/services/protection"
"github.com/harness/gitness/app/services/pullreq"
Expand Down Expand Up @@ -62,6 +63,7 @@ type Controller struct {
sseStreamer sse.Streamer
codeOwners *codeowners.Service
locker *locker.Locker
importer *importer.PullReq
}

func NewController(
Expand All @@ -87,6 +89,7 @@ func NewController(
sseStreamer sse.Streamer,
codeowners *codeowners.Service,
locker *locker.Locker,
importer *importer.PullReq,
) *Controller {
return &Controller{
tx: tx,
Expand All @@ -111,6 +114,7 @@ func NewController(
sseStreamer: sseStreamer,
codeOwners: codeowners,
locker: locker,
importer: importer,
}
}

Expand Down Expand Up @@ -152,8 +156,8 @@ func (c *Controller) getRepoCheckAccess(ctx context.Context,
return nil, fmt.Errorf("failed to find repository: %w", err)
}

if repo.Importing {
return nil, usererror.BadRequest("Repository import is in progress.")
if repo.State != enum.RepoStateActive {
return nil, usererror.BadRequest("Repository is not ready to use.")
}

if err = apiauth.CheckRepo(ctx, c.authorizer, session, repo, reqPermission); err != nil {
Expand Down
5 changes: 3 additions & 2 deletions app/api/controller/pullreq/wire.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
pullreqevents "github.com/harness/gitness/app/events/pullreq"
"github.com/harness/gitness/app/services/codecomments"
"github.com/harness/gitness/app/services/codeowners"
"github.com/harness/gitness/app/services/importer"
"github.com/harness/gitness/app/services/locker"
"github.com/harness/gitness/app/services/protection"
"github.com/harness/gitness/app/services/pullreq"
Expand All @@ -45,7 +46,7 @@ func ProvideController(tx dbtx.Transactor, urlProvider url.Provider, authorizer
checkStore store.CheckStore,
rpcClient git.Interface, eventReporter *pullreqevents.Reporter, codeCommentMigrator *codecomments.Migrator,
pullreqService *pullreq.Service, ruleManager *protection.Manager, sseStreamer sse.Streamer,
codeOwners *codeowners.Service, locker *locker.Locker,
codeOwners *codeowners.Service, locker *locker.Locker, importer *importer.PullReq,
) *Controller {
return NewController(tx, urlProvider, authorizer,
pullReqStore, pullReqActivityStore,
Expand All @@ -56,5 +57,5 @@ func ProvideController(tx dbtx.Transactor, urlProvider url.Provider, authorizer
checkStore,
rpcClient, eventReporter,
codeCommentMigrator,
pullreqService, ruleManager, sseStreamer, codeOwners, locker)
pullreqService, ruleManager, sseStreamer, codeOwners, locker, importer)
}
24 changes: 23 additions & 1 deletion app/api/controller/repo/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,8 @@ var errPublicRepoCreationDisabled = usererror.BadRequestf("Public repository cre

type RepositoryOutput struct {
types.Repository
IsPublic bool `json:"is_public" yaml:"is_public"`
IsPublic bool `json:"is_public" yaml:"is_public"`
Importing bool `json:"importing" yaml:"-"`
}

// TODO [CODE-1363]: remove after identifier migration.
Expand Down Expand Up @@ -156,6 +157,7 @@ func (c *Controller) getRepo(
ctx,
c.repoStore,
repoRef,
ActiveRepoStates,
)
}

Expand All @@ -174,6 +176,26 @@ func (c *Controller) getRepoCheckAccess(
session,
repoRef,
reqPermission,
ActiveRepoStates,
)
}

// getRepoCheckAccessForGit fetches a repo
// and checks if the current user has permission to access it.
func (c *Controller) getRepoCheckAccessForGit(
ctx context.Context,
session *auth.Session,
repoRef string,
reqPermission enum.Permission,
) (*types.Repository, error) {
return GetRepoCheckAccess(
ctx,
c.repoStore,
c.authorizer,
session,
repoRef,
reqPermission,
nil, // Any state allowed - we'll block in the pre-receive hook.
)
}

Expand Down
5 changes: 1 addition & 4 deletions app/api/controller/repo/create.go
Original file line number Diff line number Diff line change
Expand Up @@ -149,10 +149,7 @@ func (c *Controller) Create(ctx context.Context, session *auth.Session, in *Crea
repo.GitURL = c.urlProvider.GenerateGITCloneURL(repo.Path)
repo.GitSSHURL = c.urlProvider.GenerateGITCloneSSHURL(repo.Path)

repoOutput := &RepositoryOutput{
Repository: *repo,
IsPublic: in.IsPublic,
}
repoOutput := GetRepoOutputWithAccess(ctx, in.IsPublic, repo)

err = c.auditService.Log(ctx,
session.Principal,
Expand Down
2 changes: 1 addition & 1 deletion app/api/controller/repo/git_info_refs.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ func (c *Controller) GitInfoRefs(
gitProtocol string,
w io.Writer,
) error {
repo, err := c.getRepoCheckAccess(ctx, session, repoRef, enum.PermissionRepoView)
repo, err := c.getRepoCheckAccessForGit(ctx, session, repoRef, enum.PermissionRepoView)
if err != nil {
return fmt.Errorf("failed to verify repo access: %w", err)
}
Expand Down
2 changes: 1 addition & 1 deletion app/api/controller/repo/git_service_pack.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ func (c *Controller) GitServicePack(
permission = enum.PermissionRepoPush
}

repo, err := c.getRepoCheckAccess(ctx, session, repoRef, permission)
repo, err := c.getRepoCheckAccessForGit(ctx, session, repoRef, permission)
if err != nil {
return fmt.Errorf("failed to verify repo access: %w", err)
}
Expand Down
27 changes: 23 additions & 4 deletions app/api/controller/repo/helper.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,13 +26,18 @@ import (
"github.com/harness/gitness/app/store"
"github.com/harness/gitness/types"
"github.com/harness/gitness/types/enum"

"golang.org/x/exp/slices"
)

// GetRepo fetches an active repo (not one that is currently being imported).
var ActiveRepoStates = []enum.RepoState{enum.RepoStateActive}

// GetRepo fetches an repository.
func GetRepo(
ctx context.Context,
repoStore store.RepoStore,
repoRef string,
allowedStates []enum.RepoState,
) (*types.Repository, error) {
if repoRef == "" {
return nil, usererror.BadRequest("A valid repository reference must be provided.")
Expand All @@ -43,8 +48,8 @@ func GetRepo(
return nil, fmt.Errorf("failed to find repository: %w", err)
}

if repo.Importing {
return nil, usererror.BadRequest("Repository import is in progress.")
if len(allowedStates) > 0 && !slices.Contains(allowedStates, repo.State) {
return nil, usererror.BadRequest("Repository is not ready to use.")
}

return repo, nil
Expand All @@ -59,8 +64,9 @@ func GetRepoCheckAccess(
session *auth.Session,
repoRef string,
reqPermission enum.Permission,
allowedStates []enum.RepoState,
) (*types.Repository, error) {
repo, err := GetRepo(ctx, repoStore, repoRef)
repo, err := GetRepo(ctx, repoStore, repoRef, allowedStates)
if err != nil {
return nil, fmt.Errorf("failed to find repo: %w", err)
}
Expand All @@ -85,5 +91,18 @@ func GetRepoOutput(
return &RepositoryOutput{
Repository: *repo,
IsPublic: isPublic,
Importing: repo.State != enum.RepoStateActive,
}, nil
}

func GetRepoOutputWithAccess(
_ context.Context,
isPublic bool,
repo *types.Repository,
) *RepositoryOutput {
return &RepositoryOutput{
Repository: *repo,
IsPublic: isPublic,
Importing: repo.State != enum.RepoStateActive,
}
}
5 changes: 1 addition & 4 deletions app/api/controller/repo/import.go
Original file line number Diff line number Diff line change
Expand Up @@ -114,10 +114,7 @@ func (c *Controller) Import(ctx context.Context, session *auth.Session, in *Impo
log.Warn().Msgf("failed to insert audit log for import repository operation: %s", err)
}

return &RepositoryOutput{
Repository: *repo,
IsPublic: false,
}, nil
return GetRepoOutputWithAccess(ctx, false, repo), nil
}

func (c *Controller) sanitizeImportInput(in *ImportInput) error {
Expand Down
4 changes: 2 additions & 2 deletions app/api/controller/repo/move.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,8 @@ func (c *Controller) Move(ctx context.Context,
return nil, err
}

if repo.Importing {
return nil, usererror.BadRequest("can't move a repo that is being imported")
if repo.State != enum.RepoStateActive {
return nil, usererror.BadRequest("Can't move a repo that isn't ready to use.")
}

if err = apiauth.CheckRepo(ctx, c.authorizer, session, repo, enum.PermissionRepoEdit); err != nil {
Expand Down
2 changes: 1 addition & 1 deletion app/api/controller/repo/purge.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ func (c *Controller) PurgeNoAuth(
session *auth.Session,
repo *types.Repository,
) error {
if repo.Importing {
if repo.State == enum.RepoStateGitImport {
log.Ctx(ctx).Info().Msg("repository is importing. cancelling the import job.")
err := c.importer.Cancel(ctx, repo)
if err != nil {
Expand Down
5 changes: 1 addition & 4 deletions app/api/controller/repo/restore.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,8 +94,5 @@ func (c *Controller) RestoreNoAuth(
}

// Repos restored as private since public access data has been deleted upon deletion.
return &RepositoryOutput{
Repository: *repo,
IsPublic: false,
}, nil
return GetRepoOutputWithAccess(ctx, false, repo), nil
}
2 changes: 1 addition & 1 deletion app/api/controller/repo/soft_delete.go
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ func (c *Controller) SoftDeleteNoAuth(
return fmt.Errorf("failed to delete public access for repo: %w", err)
}

if repo.Importing {
if repo.State != enum.RepoStateActive {
return c.PurgeNoAuth(ctx, session, repo)
}

Expand Down
10 changes: 2 additions & 8 deletions app/api/controller/repo/update_public_access.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,10 +63,7 @@ func (c *Controller) UpdatePublicAccess(ctx context.Context,

// no op
if isPublic == in.IsPublic {
return &RepositoryOutput{
Repository: *repo,
IsPublic: isPublic,
}, nil
return GetRepoOutputWithAccess(ctx, isPublic, repo), nil
}

if err = c.publicAccess.Set(ctx, enum.PublicResourceTypeRepo, repo.Path, in.IsPublic); err != nil {
Expand Down Expand Up @@ -95,8 +92,5 @@ func (c *Controller) UpdatePublicAccess(ctx context.Context,
log.Ctx(ctx).Warn().Msgf("failed to insert audit log for update repository operation: %s", err)
}

return &RepositoryOutput{
Repository: *repo,
IsPublic: in.IsPublic,
}, nil
return GetRepoOutputWithAccess(ctx, in.IsPublic, repo), nil
}
1 change: 1 addition & 0 deletions app/api/controller/reposettings/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,5 +63,6 @@ func (c *Controller) getRepoCheckAccess(
session,
repoRef,
reqPermission,
repo.ActiveRepoStates,
)
}
20 changes: 7 additions & 13 deletions app/api/controller/space/import_repositories.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import (

apiauth "github.com/harness/gitness/app/api/auth"
"github.com/harness/gitness/app/api/controller/limiter"
repoCtrl "github.com/harness/gitness/app/api/controller/repo"
repoctrl "github.com/harness/gitness/app/api/controller/repo"
"github.com/harness/gitness/app/api/usererror"
"github.com/harness/gitness/app/auth"
"github.com/harness/gitness/app/paths"
Expand All @@ -39,8 +39,8 @@ type ImportRepositoriesInput struct {
}

type ImportRepositoriesOutput struct {
ImportingRepos []*repoCtrl.RepositoryOutput `json:"importing_repos"`
DuplicateRepos []*repoCtrl.RepositoryOutput `json:"duplicate_repos"` // repos which already exist in the space.
ImportingRepos []*repoctrl.RepositoryOutput `json:"importing_repos"`
DuplicateRepos []*repoctrl.RepositoryOutput `json:"duplicate_repos"` // repos which already exist in the space.
}

// getSpaceCheckAuthRepoCreation checks whether the user has permissions to create repos
Expand Down Expand Up @@ -165,12 +165,9 @@ func (c *Controller) ImportRepositories(
return ImportRepositoriesOutput{}, err
}

reposOut := make([]*repoCtrl.RepositoryOutput, len(repos))
reposOut := make([]*repoctrl.RepositoryOutput, len(repos))
for i, repo := range repos {
reposOut[i] = &repoCtrl.RepositoryOutput{
Repository: *repo,
IsPublic: false,
}
reposOut[i] = repoctrl.GetRepoOutputWithAccess(ctx, false, repo)

err = c.auditService.Log(ctx,
session.Principal,
Expand All @@ -187,12 +184,9 @@ func (c *Controller) ImportRepositories(
}
}

duplicateReposOut := make([]*repoCtrl.RepositoryOutput, len(duplicateRepos))
duplicateReposOut := make([]*repoctrl.RepositoryOutput, len(duplicateRepos))
for i, dupRepo := range duplicateRepos {
duplicateReposOut[i] = &repoCtrl.RepositoryOutput{
Repository: *dupRepo,
IsPublic: false,
}
duplicateReposOut[i] = repoctrl.GetRepoOutputWithAccess(ctx, false, dupRepo)
}

return ImportRepositoriesOutput{ImportingRepos: reposOut, DuplicateRepos: duplicateReposOut}, nil
Expand Down
3 changes: 2 additions & 1 deletion app/services/importer/provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import (
"github.com/harness/gitness/app/api/usererror"
"github.com/harness/gitness/app/paths"
"github.com/harness/gitness/types"
"github.com/harness/gitness/types/enum"

"github.com/drone/go-scm/scm"
"github.com/drone/go-scm/scm/driver/azure"
Expand Down Expand Up @@ -100,7 +101,7 @@ func (r *RepositoryInfo) ToRepo(
Updated: now,
ForkID: 0,
DefaultBranch: r.DefaultBranch,
Importing: true,
State: enum.RepoStateGitImport,
Path: paths.Concatenate(spacePath, identifier),
}, r.IsPublic
}
Expand Down
Loading

0 comments on commit 5427130

Please sign in to comment.