From b31ae8c2572adbc8df0495e7bd803aec35e9ffd5 Mon Sep 17 00:00:00 2001 From: Hitesh Aringa Date: Fri, 27 Oct 2023 23:46:34 +0000 Subject: [PATCH] [CODE-1011]: Disable case sensitive branch search and remove gitea open repo (#732) --- .vscode/launch.json | 2 +- Dockerfile | 2 +- Makefile | 2 +- gitrpc/internal/gitea/commit.go | 15 +++++---------- gitrpc/internal/gitea/ref.go | 2 +- gitrpc/internal/service/branch.go | 18 +++--------------- gitrpc/internal/service/http.go | 2 +- gitrpc/internal/service/operations.go | 23 +++++++++++++++++------ gitrpc/internal/service/ref.go | 15 +++++---------- gitrpc/internal/service/shared_repo.go | 20 ++++++++++---------- gitrpc/internal/service/tag.go | 14 ++------------ 11 files changed, 47 insertions(+), 68 deletions(-) diff --git a/.vscode/launch.json b/.vscode/launch.json index 2d47006e7..c08542f19 100644 --- a/.vscode/launch.json +++ b/.vscode/launch.json @@ -9,7 +9,7 @@ "type": "go", "request": "launch", "mode": "auto", - "buildFlags": "-tags=gogit", + "buildFlags": "", "program": "cmd/gitness", "args": ["server", "../../.local.env"] } diff --git a/Dockerfile b/Dockerfile index 19e604bdd..53b021708 100644 --- a/Dockerfile +++ b/Dockerfile @@ -58,7 +58,7 @@ RUN --mount=type=cache,target=/root/.cache/go-build \ LDFLAGS="-X github.com/harness/gitness/version.GitCommit=${GIT_COMMIT} -X github.com/harness/gitness/version.major=${GITNESS_VERSION_MAJOR} -X github.com/harness/gitness/version.minor=${GITNESS_VERSION_MINOR} -X github.com/harness/gitness/version.patch=${GITNESS_VERSION_PATCH} -extldflags '-static'" && \ CGO_ENABLED=1 \ GOOS=$TARGETOS GOARCH=$TARGETARCH \ - CC=$CC go build -tags=gogit -ldflags="$LDFLAGS" -o ./gitness ./cmd/gitness + CC=$CC go build -ldflags="$LDFLAGS" -o ./gitness ./cmd/gitness ### Pull CA Certs FROM --platform=$BUILDPLATFORM alpine:latest as cert-image diff --git a/Makefile b/Makefile index fcccb68f5..2c1810e4a 100644 --- a/Makefile +++ b/Makefile @@ -40,7 +40,7 @@ tools: $(tools) ## Install tools required for the build build: generate ## Build the all-in-one gitness binary @echo "Building Gitness Server" - go build -tags=gogit -o ./gitness ./cmd/gitness + go build -o ./gitness ./cmd/gitness test: generate ## Run the go tests @echo "Running tests" diff --git a/gitrpc/internal/gitea/commit.go b/gitrpc/internal/gitea/commit.go index 4c67efa91..5adae25d0 100644 --- a/gitrpc/internal/gitea/commit.go +++ b/gitrpc/internal/gitea/commit.go @@ -92,7 +92,8 @@ func getGiteaCommits(giteaRepo *gitea.Repository, commitIDs []string) ([]*gitea. } func (g Adapter) listCommitSHAs( - giteaRepo *gitea.Repository, + ctx context.Context, + repoPath string, ref string, page int, limit int, @@ -136,7 +137,7 @@ func (g Adapter) listCommitSHAs( args = append(args, "--committer", filter.Committer) } - stdout, _, runErr := gitea.NewCommand(giteaRepo.Ctx, args...).RunStdBytes(&gitea.RunOpts{Dir: giteaRepo.Path}) + stdout, _, runErr := gitea.NewCommand(ctx, args...).RunStdBytes(&gitea.RunOpts{Dir: repoPath}) if runErr != nil { // TODO: handle error in case they don't have a common merge base! return nil, processGiteaErrorf(runErr, "failed to trigger rev-list command") @@ -156,13 +157,7 @@ func (g Adapter) ListCommitSHAs( limit int, filter types.CommitFilter, ) ([]string, error) { - giteaRepo, err := gitea.OpenRepository(ctx, repoPath) - if err != nil { - return nil, processGiteaErrorf(err, "failed to open repository") - } - defer giteaRepo.Close() - - return g.listCommitSHAs(giteaRepo, ref, page, limit, filter) + return g.listCommitSHAs(ctx, repoPath, ref, page, limit, filter) } // ListCommits lists the commits reachable from ref. @@ -179,7 +174,7 @@ func (g Adapter) ListCommits(ctx context.Context, } defer giteaRepo.Close() - commitSHAs, err := g.listCommitSHAs(giteaRepo, ref, page, limit, filter) + commitSHAs, err := g.listCommitSHAs(ctx, repoPath, ref, page, limit, filter) if err != nil { return nil, nil, err } diff --git a/gitrpc/internal/gitea/ref.go b/gitrpc/internal/gitea/ref.go index c14583ab9..2cf6c4b4d 100644 --- a/gitrpc/internal/gitea/ref.go +++ b/gitrpc/internal/gitea/ref.go @@ -79,7 +79,7 @@ func (g Adapter) WalkReferences(ctx context.Context, sortArg, "--count", fmt.Sprint(opts.MaxWalkDistance), - "--ignore-case", + // "--ignore-case" // slows down performance drastically } args = append(args, opts.Patterns...) err := gitea.NewCommand(ctx, args...).Run(rc) diff --git a/gitrpc/internal/service/branch.go b/gitrpc/internal/service/branch.go index de437b323..d68ee1868 100644 --- a/gitrpc/internal/service/branch.go +++ b/gitrpc/internal/service/branch.go @@ -47,17 +47,11 @@ func (s ReferenceService) CreateBranch( repoPath := getFullPathForRepo(s.reposRoot, base.GetRepoUid()) - // TODO: why are we using gitea operations here?! - repo, err := git.OpenRepository(ctx, repoPath) - if err != nil { - return nil, processGitErrorf(err, "failed to open repo") - } - - if ok, err := repo.IsEmpty(); ok { + if ok, err := repoIsEmpty(ctx, repoPath); ok { return nil, ErrInvalidArgumentf("branch cannot be created on empty repository", err) } - sharedRepo, err := NewSharedRepo(s.tmpDir, base.GetRepoUid(), repo) + sharedRepo, err := NewSharedRepo(s.tmpDir, base.GetRepoUid(), repoPath) if err != nil { return nil, processGitErrorf(err, "failed to create new shared repo") } @@ -145,13 +139,7 @@ func (s ReferenceService) DeleteBranch(ctx context.Context, repoPath := getFullPathForRepo(s.reposRoot, base.GetRepoUid()) - // TODO: why are we using gitea operations here?! - repo, err := git.OpenRepository(ctx, repoPath) - if err != nil { - return nil, processGitErrorf(err, "failed to open repo") - } - - sharedRepo, err := NewSharedRepo(s.tmpDir, base.GetRepoUid(), repo) + sharedRepo, err := NewSharedRepo(s.tmpDir, base.GetRepoUid(), repoPath) if err != nil { return nil, processGitErrorf(err, "failed to create new shared repo") } diff --git a/gitrpc/internal/service/http.go b/gitrpc/internal/service/http.go index 448843b4e..28abe3f1a 100644 --- a/gitrpc/internal/service/http.go +++ b/gitrpc/internal/service/http.go @@ -153,7 +153,6 @@ func serviceRPC(ctx context.Context, stdin io.Reader, stdout io.Writer, if protocol != "" && safeGitProtocolHeader.MatchString(protocol) { environ = append(environ, "GIT_PROTOCOL="+protocol) } - var ( stderr bytes.Buffer ) @@ -170,6 +169,7 @@ func serviceRPC(ctx context.Context, stdin io.Reader, stdout io.Writer, if err != nil && err.Error() != "signal: killed" { log.Ctx(ctx).Err(err).Msgf("Fail to serve RPC(%s) in %s: %v - %s", service, dir, err, stderr.String()) } + return err } diff --git a/gitrpc/internal/service/operations.go b/gitrpc/internal/service/operations.go index 534f5ac99..19914926b 100644 --- a/gitrpc/internal/service/operations.go +++ b/gitrpc/internal/service/operations.go @@ -109,13 +109,13 @@ func (s *CommitFilesService) CommitFiles(stream rpc.CommitFilesService_CommitFil // This can be an issue in case someone created a branch already in the repo (just default branch is missing). // In that case the user can accidentally create separate git histories (which most likely is unintended). // If the user wants to actually build a disconnected commit graph they can use the cli. - isEmpty, err := repoHasBranches(ctx, repo) + hasBranches, err := repoHasBranches(ctx, repoPath) if err != nil { return ErrInternalf("failed to determine if repository is empty", err) } // ensure input data is valid - if err = s.validateAndPrepareHeader(repo, isEmpty, header); err != nil { + if err = s.validateAndPrepareHeader(repo, !hasBranches, header); err != nil { return err } @@ -126,7 +126,7 @@ func (s *CommitFilesService) CommitFiles(stream rpc.CommitFilesService_CommitFil } // create a new shared repo - shared, err := NewSharedRepo(s.reposTempDir, base.GetRepoUid(), repo) + shared, err := NewSharedRepo(s.reposTempDir, base.GetRepoUid(), repoPath) if err != nil { return processGitErrorf(err, "failed to create shared repository") } @@ -134,7 +134,7 @@ func (s *CommitFilesService) CommitFiles(stream rpc.CommitFilesService_CommitFil // handle empty repo separately (as branch doesn't exist, no commit exists, ...) var parentCommitSHA string - if isEmpty { + if !hasBranches { err = s.prepareTreeEmptyRepo(ctx, shared, actions) if err != nil { return err @@ -528,11 +528,22 @@ func checkPathAvailability(commit *git.Commit, filePath string, isNewFile bool) // repoHasBranches returns true iff there's at least one branch in the repo (any branch) // NOTE: This is different from repo.Empty(), // as it doesn't care whether the existing branch is the default branch or not. -func repoHasBranches(ctx context.Context, repo *git.Repository) (bool, error) { +func repoHasBranches(ctx context.Context, repoPath string) (bool, error) { // repo has branches IFF there's at least one commit that is reachable via a branch // (every existing branch points to a commit) stdout, _, runErr := git.NewCommand(ctx, "rev-list", "--max-count", "1", "--branches"). - RunStdBytes(&git.RunOpts{Dir: repo.Path}) + RunStdBytes(&git.RunOpts{Dir: repoPath}) + if runErr != nil { + return false, processGitErrorf(runErr, "failed to trigger rev-list command") + } + + return strings.TrimSpace(string(stdout)) != "", nil +} + +func repoIsEmpty(ctx context.Context, repoPath string) (bool, error) { + // repoIsEmpty IFF there's no commit + stdout, _, runErr := git.NewCommand(ctx, "rev-list", "--max-count", "1", "--all"). + RunStdBytes(&git.RunOpts{Dir: repoPath}) if runErr != nil { return false, processGitErrorf(runErr, "failed to trigger rev-list command") } diff --git a/gitrpc/internal/service/ref.go b/gitrpc/internal/service/ref.go index 0f1d341f4..4e78ed544 100644 --- a/gitrpc/internal/service/ref.go +++ b/gitrpc/internal/service/ref.go @@ -234,17 +234,11 @@ func (s ReferenceService) UpdateRef(ctx context.Context, return nil, err } - // TODO: why are we using gitea operations here?! - repo, err := git.OpenRepository(ctx, repoPath) - if err != nil { - return nil, processGitErrorf(err, "failed to open repo") + if ok, err := repoIsEmpty(ctx, repoPath); ok { + return nil, ErrInvalidArgumentf("reference cannot be updated on empty repository", err) } - if ok, err := repo.IsEmpty(); ok { - return nil, ErrInvalidArgumentf("branch cannot be created on empty repository", err) - } - - sharedRepo, err := NewSharedRepo(s.tmpDir, base.GetRepoUid(), repo) + sharedRepo, err := NewSharedRepo(s.tmpDir, base.GetRepoUid(), repoPath) if err != nil { return nil, processGitErrorf(err, "failed to create new shared repo") } @@ -257,7 +251,7 @@ func (s ReferenceService) UpdateRef(ctx context.Context, } pushOpts := types.PushOptions{ - Remote: sharedRepo.remoteRepo.Path, + Remote: repoPath, Env: CreateEnvironmentForPush(ctx, base), } @@ -274,6 +268,7 @@ func (s ReferenceService) UpdateRef(ctx context.Context, pushOpts.ForceWithLease = reference + ":" + request.GetOldValue() } + // TODO: why are we using gitea operations here? // TODO: our shared repo has so much duplication, that should be changed IMHO. err = gitea.Push(ctx, sharedRepo.tmpPath, pushOpts) if err != nil { diff --git a/gitrpc/internal/service/shared_repo.go b/gitrpc/internal/service/shared_repo.go index b49f36b4d..14f93a125 100644 --- a/gitrpc/internal/service/shared_repo.go +++ b/gitrpc/internal/service/shared_repo.go @@ -37,22 +37,22 @@ import ( // SharedRepo is a type to wrap our upload repositories as a shallow clone. type SharedRepo struct { - repoUID string - repo *git.Repository - remoteRepo *git.Repository - tmpPath string + repoUID string + repo *git.Repository + remoteRepoPath string + tmpPath string } // NewSharedRepo creates a new temporary upload repository. -func NewSharedRepo(baseTmpDir, repoUID string, remoteRepo *git.Repository) (*SharedRepo, error) { +func NewSharedRepo(baseTmpDir, repoUID string, remoteRepoPath string) (*SharedRepo, error) { tmpPath, err := tempdir.CreateTemporaryPath(baseTmpDir, repoUID) if err != nil { return nil, err } t := &SharedRepo{ - repoUID: repoUID, - remoteRepo: remoteRepo, - tmpPath: tmpPath, + repoUID: repoUID, + remoteRepoPath: remoteRepoPath, + tmpPath: tmpPath, } return t, nil } @@ -71,7 +71,7 @@ func (r *SharedRepo) Clone(ctx context.Context, branchName string) error { if branchName != "" { args = append(args, "-b", strings.TrimPrefix(branchName, gitReferenceNamePrefixBranch)) } - args = append(args, r.remoteRepo.Path, r.tmpPath) + args = append(args, r.remoteRepoPath, r.tmpPath) if _, _, err := git.NewCommand(ctx, args...).RunStdString(nil); err != nil { stderr := err.Error() @@ -338,7 +338,7 @@ func (r *SharedRepo) push(ctx context.Context, writeRequest *rpc.WriteRequest, // Because calls hooks we need to pass in the environment env := CreateEnvironmentForPush(ctx, writeRequest) if err := gitea.Push(ctx, r.tmpPath, types.PushOptions{ - Remote: r.remoteRepo.Path, + Remote: r.remoteRepoPath, Branch: sourceRef + ":" + destinationRef, Env: env, }); err != nil { diff --git a/gitrpc/internal/service/tag.go b/gitrpc/internal/service/tag.go index 7f64b72d8..753a5236b 100644 --- a/gitrpc/internal/service/tag.go +++ b/gitrpc/internal/service/tag.go @@ -231,12 +231,7 @@ func (s ReferenceService) CreateCommitTag( repoPath := getFullPathForRepo(s.reposRoot, base.GetRepoUid()) - repo, err := git.OpenRepository(ctx, repoPath) - if err != nil { - return nil, processGitErrorf(err, "failed to open repo") - } - - sharedRepo, err := NewSharedRepo(s.tmpDir, base.GetRepoUid(), repo) + sharedRepo, err := NewSharedRepo(s.tmpDir, base.GetRepoUid(), repoPath) if err != nil { return nil, processGitErrorf(err, "failed to create new shared repo") } @@ -337,12 +332,7 @@ func (s ReferenceService) DeleteTag( repoPath := getFullPathForRepo(s.reposRoot, base.GetRepoUid()) - repo, err := git.OpenRepository(ctx, repoPath) - if err != nil { - return nil, processGitErrorf(err, "failed to open repo") - } - - sharedRepo, err := NewSharedRepo(s.tmpDir, base.GetRepoUid(), repo) + sharedRepo, err := NewSharedRepo(s.tmpDir, base.GetRepoUid(), repoPath) if err != nil { return nil, processGitErrorf(err, "failed to create new shared repo") }