diff --git a/gitrpc/internal/gitea/branch.go b/gitrpc/internal/gitea/branch.go index 82857983f..a25ef0e05 100644 --- a/gitrpc/internal/gitea/branch.go +++ b/gitrpc/internal/gitea/branch.go @@ -7,58 +7,12 @@ package gitea import ( "context" "fmt" - "strings" "github.com/harness/gitness/gitrpc/internal/types" gitea "code.gitea.io/gitea/modules/git" ) -// CreateBranch creates a new branch. -// Note: target is the commit (or points to the commit) the branch will be pointing to. -func (g Adapter) CreateBranch(ctx context.Context, repoPath string, - branchName string, target string) (*types.Branch, error) { - giteaRepo, err := gitea.OpenRepository(ctx, repoPath) - if err != nil { - return nil, err - } - defer giteaRepo.Close() - - // Get the commit object for the ref - giteaCommit, err := giteaRepo.GetCommit(target) - if err != nil { - return nil, processGiteaErrorf(err, "error getting commit for ref '%s'", target) - } - - // In case of target being an annotated tag, gitea is overwriting the commit message with the tag message. - // Reload the commit explicitly in case it's a tag (independent of whether it's annotated or not to simplify code) - // NOTE: we also allow passing refs/tags/tagName or tags/tagName, which is not covered by IsTagExist. - // Worst case we have a false positive and reload the same commit, we don't want false negatives though! - if strings.HasPrefix(target, gitea.TagPrefix) || strings.HasPrefix(target, "tags") || giteaRepo.IsTagExist(target) { - giteaCommit, err = giteaRepo.GetCommit(giteaCommit.ID.String()) - if err != nil { - return nil, processGiteaErrorf(err, "error getting commit for annotated tag '%s' (commitId '%s')", - target, giteaCommit.ID.String()) - } - } - - err = giteaRepo.CreateBranch(branchName, giteaCommit.ID.String()) - if err != nil { - return nil, processGiteaErrorf(err, "failed to create branch '%s'", branchName) - } - - commit, err := mapGiteaCommit(giteaCommit) - if err != nil { - return nil, fmt.Errorf("failed to map gitea commit: %w", err) - } - - return &types.Branch{ - Name: branchName, - SHA: giteaCommit.ID.String(), - Commit: commit, - }, nil -} - // GetBranch gets an existing branch. func (g Adapter) GetBranch(ctx context.Context, repoPath string, branchName string) (*types.Branch, error) { @@ -89,24 +43,3 @@ func (g Adapter) GetBranch(ctx context.Context, repoPath string, Commit: commit, }, nil } - -// DeleteBranch deletes an existing branch. -func (g Adapter) DeleteBranch(ctx context.Context, repoPath string, branchName string, force bool) (string, error) { - giteaRepo, err := gitea.OpenRepository(ctx, repoPath) - if err != nil { - return "", err - } - defer giteaRepo.Close() - - sha, err := giteaRepo.GetRefCommitID(branchName) - if err != nil { - return "", processGiteaErrorf(err, "failed to read sha of branch '%s'", branchName) - } - - err = giteaRepo.DeleteBranch(branchName, gitea.DeleteBranchOptions{Force: force}) - if err != nil { - return "", processGiteaErrorf(err, "failed to delete branch '%s'", branchName) - } - - return sha, nil -} diff --git a/gitrpc/internal/gitea/commit.go b/gitrpc/internal/gitea/commit.go index 347c95118..105a80a1c 100644 --- a/gitrpc/internal/gitea/commit.go +++ b/gitrpc/internal/gitea/commit.go @@ -93,12 +93,6 @@ func (g Adapter) ListCommits(ctx context.Context, repoPath string, } defer giteaRepo.Close() - // Get the refCommitSHA object for the ref - refCommitSHA, err := giteaRepo.GetRefCommitID(ref) - if err != nil { - return nil, processGiteaErrorf(err, "error getting commit ID for ref '%s'", ref) - } - args := []string{"rev-list"} // add pagination if requested @@ -112,17 +106,12 @@ func (g Adapter) ListCommits(ctx context.Context, repoPath string, } // add refCommitSHA as starting point - args = append(args, refCommitSHA) + args = append(args, ref) // return commits only up to a certain reference if requested if afterRef != "" { - var afterRefCommitSHA string - afterRefCommitSHA, err = giteaRepo.GetRefCommitID(afterRef) - if err != nil { - return nil, processGiteaErrorf(err, "error getting commit ID for afterRef '%s'", afterRef) - } - // ^SHA tells the rev-list command to return only commits that aren't reachable by SHA - args = append(args, fmt.Sprintf("^%s", afterRefCommitSHA)) + // ^REF tells the rev-list command to return only commits that aren't reachable by SHA + args = append(args, fmt.Sprintf("^%s", afterRef)) } stdout, _, runErr := gitea.NewCommand(giteaRepo.Ctx, args...).RunStdBytes(&gitea.RunOpts{Dir: giteaRepo.Path}) diff --git a/gitrpc/internal/service/branch.go b/gitrpc/internal/service/branch.go index 7abf5974e..2ed1010a6 100644 --- a/gitrpc/internal/service/branch.go +++ b/gitrpc/internal/service/branch.go @@ -7,6 +7,7 @@ package service import ( "context" "fmt" + "strings" "github.com/harness/gitness/gitrpc/internal/gitea" "github.com/harness/gitness/gitrpc/internal/types" @@ -48,14 +49,15 @@ func (s ReferenceService) CreateBranch(ctx context.Context, } // push to new branch (all changes should go through push flow for hooks and other safety meassures) - err = sharedRepo.Push(ctx, base, request.GetTarget(), request.GetBranchName()) + err = sharedRepo.PushBranch(ctx, base, request.GetTarget(), request.GetBranchName()) if err != nil { return nil, processGitErrorf(err, "failed to push new branch '%s'", request.GetBranchName()) } // get branch - // TODO: get it from shared repo to avoid opening another gitea repo - gitBranch, err := s.adapter.GetBranch(ctx, repoPath, request.GetBranchName()) + // TODO: get it from shared repo to avoid opening another gitea repo and having to strip here. + gitBranch, err := s.adapter.GetBranch(ctx, repoPath, + strings.TrimPrefix(request.GetBranchName(), gitReferenceNamePrefixBranch)) if err != nil { return nil, processGitErrorf(err, "failed to get gitea branch '%s'", request.GetBranchName()) } @@ -106,7 +108,7 @@ func (s ReferenceService) DeleteBranch(ctx context.Context, // push to new branch (all changes should go through push flow for hooks and other safety meassures) // NOTE: setting sourceRef to empty will delete the remote branch when pushing: // https://git-scm.com/docs/git-push#Documentation/git-push.txt-ltrefspecgt82308203 - err = sharedRepo.Push(ctx, base, "", request.GetBranchName()) + err = sharedRepo.PushDeleteBranch(ctx, base, request.GetBranchName()) if err != nil { return nil, processGitErrorf(err, "failed to delete branch '%s' from remote repo", request.GetBranchName()) } diff --git a/gitrpc/internal/service/interface.go b/gitrpc/internal/service/interface.go index 176874b67..e0a495951 100644 --- a/gitrpc/internal/service/interface.go +++ b/gitrpc/internal/service/interface.go @@ -36,9 +36,7 @@ type GitAdapter interface { GetFullCommitID(ctx context.Context, repoPath, shortID string) (string, error) GetAnnotatedTag(ctx context.Context, repoPath string, sha string) (*types.Tag, error) GetAnnotatedTags(ctx context.Context, repoPath string, shas []string) ([]types.Tag, error) - CreateBranch(ctx context.Context, repoPath string, branchName string, target string) (*types.Branch, error) GetBranch(ctx context.Context, repoPath string, branchName string) (*types.Branch, error) - DeleteBranch(ctx context.Context, repoPath string, branchName string, force bool) (string, error) GetCommitDivergences(ctx context.Context, repoPath string, requests []types.CommitDivergenceRequest, max int32) ([]types.CommitDivergence, error) GetRef(ctx context.Context, repoPath string, name string, refType types.RefType) (string, error) diff --git a/gitrpc/internal/service/operations.go b/gitrpc/internal/service/operations.go index 8e91fdb97..c2749c9ad 100644 --- a/gitrpc/internal/service/operations.go +++ b/gitrpc/internal/service/operations.go @@ -23,7 +23,8 @@ import ( ) const ( - filePrefix = "file://" + filePrefix = "file://" + defaultFilePermission = "100644" // 0o644 default file permission ) type CommitFilesService struct { @@ -81,35 +82,44 @@ func (s *CommitFilesService) CommitFiles(stream rpc.CommitFilesService_CommitFil return err } - if err = s.validateHeader(repo, header); err != nil { + // check if repo is empty + // IMPORTANT: we don't use gitea's repo.IsEmpty() as that only checks whether the default branch exists (in HEAD). + // 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 accidentaly 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) + if err != nil { + return fmt.Errorf("failed to determine if repo is empty: %w", err) + } + + // ensure input data is valid + if err = s.validateAndPrepareHeader(repo, isEmpty, header); err != nil { return err } + // collect all file actions from grpc stream actions := make([]fileAction, 0, 16) if err = s.collectActions(stream, &actions); err != nil { return err } - // create a shared repo + // create a new shared repo shared, err := NewSharedRepo(s.reposTempDir, base.GetRepoUid(), repo) if err != nil { return err } defer shared.Close(ctx) - if err = s.clone(ctx, repo, shared, header.GetBranchName()); err != nil { - return err - } - - // Get the commit of the original branch - commit, err := shared.GetBranchCommit(header.GetBranchName()) - if err != nil { - return err - } - - for _, action := range actions { - action := action - if err = s.processAction(ctx, shared, &action, commit); err != nil { + // handle empty repo separately (as branch doesn't exist, no commit exists, ...) + var parentCommitSHA string + if isEmpty { + err = s.prepareTreeEmptyRepo(ctx, shared, actions) + if err != nil { + return err + } + } else { + parentCommitSHA, err = s.prepareTree(ctx, shared, header.GetBranchName(), actions) + if err != nil { return err } } @@ -125,16 +135,16 @@ func (s *CommitFilesService) CommitFiles(stream rpc.CommitFilesService_CommitFil message += "\n\n" + strings.TrimSpace(header.GetMessage()) } // Now commit the tree - commitHash, err := shared.CommitTree(ctx, commit.ID.String(), author, committer, treeHash, message, false) + commitSHA, err := shared.CommitTree(ctx, parentCommitSHA, author, committer, treeHash, message, false) if err != nil { return err } - if err = shared.Push(ctx, base, commitHash, header.GetNewBranchName()); err != nil { + if err = shared.PushCommit(ctx, base, commitSHA, header.GetNewBranchName()); err != nil { return err } - commit, err = shared.GetCommit(commitHash) + commit, err := shared.GetCommit(commitSHA) if err != nil { return err } @@ -144,23 +154,86 @@ func (s *CommitFilesService) CommitFiles(stream rpc.CommitFilesService_CommitFil }) } -func (s *CommitFilesService) validateHeader(repo *git.Repository, header *rpc.CommitFilesRequestHeader) error { +func (s *CommitFilesService) prepareTree(ctx context.Context, shared *SharedRepo, + branchName string, actions []fileAction) (string, error) { + // clone original branch from repo + if err := s.clone(ctx, shared, branchName); err != nil { + return "", err + } + + // Get the latest commit of the original branch + commit, err := shared.GetBranchCommit(branchName) + if err != nil { + return "", err + } + + // execute all actions + for _, action := range actions { + action := action + if err = s.processAction(ctx, shared, &action, commit); err != nil { + return "", err + } + } + + return commit.ID.String(), nil +} + +func (s *CommitFilesService) prepareTreeEmptyRepo(ctx context.Context, shared *SharedRepo, + actions []fileAction) error { + // init a new repo (full clone would cause risk that by time of push someone wrote to the remote repo!) + err := shared.Init(ctx) + if err != nil { + return fmt.Errorf("failed to init shared tmp repo: %w", err) + } + + for _, action := range actions { + if action.header.Action != rpc.CommitFilesActionHeader_CREATE { + return types.ErrActionNotAllowedOnEmptyRepo + } + + filePath := files.CleanUploadFileName(action.header.GetPath()) + if filePath == "" { + return types.ErrInvalidPath + } + + reader := bytes.NewReader(action.content) + if err = createFile(ctx, shared, nil, filePath, defaultFilePermission, reader); err != nil { + return fmt.Errorf("failed to create file '%s': %w", action.header.Path, err) + } + } + + return nil +} + +func (s *CommitFilesService) validateAndPrepareHeader(repo *git.Repository, isEmpty bool, + header *rpc.CommitFilesRequestHeader) error { if header.GetBranchName() == "" { - branch, err := repo.GetDefaultBranch() + defaultBranchRef, err := repo.GetDefaultBranch() if err != nil { return err } - header.BranchName = branch + header.BranchName = defaultBranchRef } if header.GetNewBranchName() == "" { header.NewBranchName = header.GetBranchName() } + // trim refs/heads/ prefixes to avoid issues when calling gitea API + header.BranchName = strings.TrimPrefix(strings.TrimSpace(header.GetBranchName()), gitReferenceNamePrefixBranch) + header.NewBranchName = strings.TrimPrefix(strings.TrimSpace(header.GetNewBranchName()), gitReferenceNamePrefixBranch) + + // if the repo is empty then we can skip branch existance checks + if isEmpty { + return nil + } + + // ensure source branch exists if _, err := repo.GetBranch(header.GetBranchName()); err != nil { return err } + // ensure new branch doesn't exist yet (if new branch creation was requested) if header.GetBranchName() != header.GetNewBranchName() { existingBranch, err := repo.GetBranch(header.GetNewBranchName()) if existingBranch != nil { @@ -175,21 +248,18 @@ func (s *CommitFilesService) validateHeader(repo *git.Repository, header *rpc.Co func (s *CommitFilesService) clone( ctx context.Context, - repo *git.Repository, shared *SharedRepo, branch string, ) error { if err := shared.Clone(ctx, branch); err != nil { - empty, _ := repo.IsEmpty() - if !git.IsErrBranchNotExist(err) || !empty { - return err - } - if errInit := shared.Init(ctx); errInit != nil { - return errInit - } - return err + return fmt.Errorf("failed to clone branch '%s': %w", branch, err) } - return shared.SetDefaultIndex(ctx) + + if err := shared.SetDefaultIndex(ctx); err != nil { + return fmt.Errorf("failed to set default index: %w", err) + } + + return nil } func (s *CommitFilesService) collectActions( @@ -254,16 +324,15 @@ func (s *CommitFilesService) processAction( return types.ErrInvalidPath } - mode := "100644" // 0o644 default file permission reader := bytes.NewReader(action.content) switch header.Action { case rpc.CommitFilesActionHeader_CREATE: - err = createFile(ctx, shared, commit, filePath, mode, reader) + err = createFile(ctx, shared, commit, filePath, defaultFilePermission, reader) case rpc.CommitFilesActionHeader_UPDATE: - err = updateFile(ctx, shared, commit, filePath, header.GetSha(), mode, reader) + err = updateFile(ctx, shared, commit, filePath, header.GetSha(), defaultFilePermission, reader) case rpc.CommitFilesActionHeader_MOVE: - err = moveFile(ctx, shared, commit, filePath, mode, reader) + err = moveFile(ctx, shared, commit, filePath, defaultFilePermission, reader) case rpc.CommitFilesActionHeader_DELETE: err = deleteFile(ctx, shared, filePath) } @@ -271,22 +340,20 @@ func (s *CommitFilesService) processAction( return err } -func createFile(ctx context.Context, repo *SharedRepo, commit *git.Commit, filePath, - mode string, reader io.Reader) error { - if err := checkPath(commit, filePath, true); err != nil { - return err +func createFile(ctx context.Context, repo *SharedRepo, commit *git.Commit, + filePath, mode string, reader io.Reader) error { + // only check path availability if a source commit is available (empty repo won't have such a commit) + if commit != nil { + if err := checkPathAvailability(commit, filePath, true); err != nil { + return err + } } - filesInIndex, err := repo.LsFiles(ctx, filePath) - if err != nil { - return fmt.Errorf("listing files error %w", err) - } - if slices.Contains(filesInIndex, filePath) { - return fmt.Errorf("%s %w", filePath, types.ErrAlreadyExists) - } - hash, err := repo.HashObject(ctx, reader) + + hash, err := repo.WriteGitObject(ctx, reader) if err != nil { return fmt.Errorf("error hashing object %w", err) } + // Add the object to the index if err = repo.AddObjectToIndex(ctx, mode, hash, filePath); err != nil { return fmt.Errorf("error creating object: %w", err) @@ -296,27 +363,20 @@ func createFile(ctx context.Context, repo *SharedRepo, commit *git.Commit, fileP func updateFile(ctx context.Context, repo *SharedRepo, commit *git.Commit, filePath, sha, mode string, reader io.Reader) error { - filesInIndex, err := repo.LsFiles(ctx, filePath) + // get file mode from existing file (default unless executable) + entry, err := getFileEntry(commit, sha, filePath) if err != nil { - return fmt.Errorf("listing files error %w", err) + return fmt.Errorf("failed to get file entry: %w", err) } - if !slices.Contains(filesInIndex, filePath) { - return fmt.Errorf("%s %w", filePath, types.ErrNotFound) + if entry.IsExecutable() { + mode = "100755" } - if commit != nil { - var entry *git.TreeEntry - entry, err = getFileEntry(commit, sha, filePath) - if err != nil { - return err - } - if entry.IsExecutable() { - mode = "100755" - } - } - hash, err := repo.HashObject(ctx, reader) + + hash, err := repo.WriteGitObject(ctx, reader) if err != nil { return fmt.Errorf("error hashing object %w", err) } + if err = repo.AddObjectToIndex(ctx, mode, hash, filePath); err != nil { return fmt.Errorf("error updating object: %w", err) } @@ -338,9 +398,10 @@ func moveFile(ctx context.Context, repo *SharedRepo, commit *git.Commit, } } - if err = checkPath(commit, newPath, false); err != nil { + if err = checkPathAvailability(commit, newPath, false); err != nil { return err } + filesInIndex, err := repo.LsFiles(ctx, filePath) if err != nil { return fmt.Errorf("listing files error %w", err) @@ -348,16 +409,16 @@ func moveFile(ctx context.Context, repo *SharedRepo, commit *git.Commit, if !slices.Contains(filesInIndex, filePath) { return fmt.Errorf("%s %w", filePath, types.ErrNotFound) } - if slices.Contains(filesInIndex, newPath) { - return fmt.Errorf("%s %w", filePath, types.ErrAlreadyExists) - } - hash, err := repo.HashObject(ctx, buffer) + + hash, err := repo.WriteGitObject(ctx, buffer) if err != nil { return fmt.Errorf("error hashing object %w", err) } + if err = repo.AddObjectToIndex(ctx, mode, hash, newPath); err != nil { - return fmt.Errorf("created object: %w", err) + return fmt.Errorf("add object: %w", err) } + if err = repo.RemoveFilesFromIndex(ctx, filePath); err != nil { return fmt.Errorf("remove object: %w", err) } @@ -372,6 +433,7 @@ func deleteFile(ctx context.Context, repo *SharedRepo, filePath string) error { if !slices.Contains(filesInIndex, filePath) { return fmt.Errorf("%s %w", filePath, types.ErrNotFound) } + if err = repo.RemoveFilesFromIndex(ctx, filePath); err != nil { return fmt.Errorf("remove object: %w", err) } @@ -384,22 +446,28 @@ func getFileEntry( path string, ) (*git.TreeEntry, error) { entry, err := commit.GetTreeEntryByPath(path) + if git.IsErrNotExist(err) { + return nil, fmt.Errorf("%s %w", path, types.ErrNotFound) + } if err != nil { return nil, err } + // If a SHA was given and the SHA given doesn't match the SHA of the fromTreePath, throw error if sha == "" || sha != entry.ID.String() { return nil, fmt.Errorf("%w for path %s [given: %s, expected: %s]", types.ErrSHADoesNotMatch, path, sha, entry.ID.String()) } + return entry, nil } -func checkPath(commit *git.Commit, filePath string, isNewFile bool) error { - // For the path where this file will be created/updated, we need to make - // sure no parts of the path are existing files or links except for the last - // item in the path which is the file name, and that shouldn't exist IF it is - // a new file OR is being moved to a new path. +// checkPathAvailability ensures that the path is available for the requested operation. +// For the path where this file will be created/updated, we need to make +// sure no parts of the path are existing files or links except for the last +// item in the path which is the file name, and that shouldn't exist IF it is +// a new file OR is being moved to a new path. +func checkPathAvailability(commit *git.Commit, filePath string, isNewFile bool) error { parts := strings.Split(filePath, "/") subTreePath := "" for index, part := range parts { @@ -431,6 +499,21 @@ func checkPath(commit *git.Commit, filePath string, isNewFile bool) error { return nil } +// 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) { + // 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}) + if runErr != nil { + return false, fmt.Errorf("failed to trigger rev-list command: %w", runErr) + } + + return strings.TrimSpace(string(stdout)) == "", nil +} + func parsePayload(payload io.Reader, content io.Writer) (string, error) { newPath := "" reader := bufio.NewReader(payload) diff --git a/gitrpc/internal/service/shared_repo.go b/gitrpc/internal/service/shared_repo.go index 0cb35d8bb..86f3c7696 100644 --- a/gitrpc/internal/service/shared_repo.go +++ b/gitrpc/internal/service/shared_repo.go @@ -54,13 +54,14 @@ func (r *SharedRepo) Close(ctx context.Context) { } // Clone the base repository to our path and set branch as the HEAD. -func (r *SharedRepo) Clone(ctx context.Context, branch string) error { +func (r *SharedRepo) Clone(ctx context.Context, branchName string) error { if _, _, err := git.NewCommand(ctx, "clone", "-s", "--bare", "-b", - branch, r.remoteRepo.Path, r.tmpPath).RunStdString(nil); err != nil { + strings.TrimPrefix(branchName, gitReferenceNamePrefixBranch), + r.remoteRepo.Path, r.tmpPath).RunStdString(nil); err != nil { stderr := err.Error() if matched, _ := regexp.MatchString(".*Remote branch .* not found in upstream origin.*", stderr); matched { return git.ErrBranchNotExist{ - Name: branch, + Name: branchName, } } else if matched, _ = regexp.MatchString(".* repository .* does not exist.*", stderr); matched { return fmt.Errorf("%s %w", r.repoUID, types.ErrNotFound) @@ -154,8 +155,8 @@ func (r *SharedRepo) RemoveFilesFromIndex(ctx context.Context, filenames ...stri return nil } -// HashObject writes the provided content to the object db and returns its hash. -func (r *SharedRepo) HashObject(ctx context.Context, content io.Reader) (string, error) { +// WriteGitObject writes the provided content to the object db and returns its hash. +func (r *SharedRepo) WriteGitObject(ctx context.Context, content io.Reader) (string, error) { stdOut := new(bytes.Buffer) stdErr := new(bytes.Buffer) @@ -296,13 +297,29 @@ func (r *SharedRepo) CommitTreeWithDate( return strings.TrimSpace(stdout.String()), nil } -// Push the provided commitHash to the repository branch by the provided user. -func (r *SharedRepo) Push(ctx context.Context, writeRequest *rpc.WriteRequest, sourceRef, branch string) error { +func (r *SharedRepo) PushDeleteBranch(ctx context.Context, writeRequest *rpc.WriteRequest, + branch string) error { + return r.push(ctx, writeRequest, "", branch) +} + +func (r *SharedRepo) PushCommit(ctx context.Context, writeRequest *rpc.WriteRequest, + commitSHA string, branch string) error { + return r.push(ctx, writeRequest, commitSHA, branch) +} + +func (r *SharedRepo) PushBranch(ctx context.Context, writeRequest *rpc.WriteRequest, + sourceBranch string, branch string) error { + return r.push(ctx, writeRequest, GetReferenceFromBranchName(sourceBranch), branch) +} + +// push the provided commitHash to the repository branch by the provided user. +func (r *SharedRepo) push(ctx context.Context, writeRequest *rpc.WriteRequest, + sourceRef, branch string) error { // Because calls hooks we need to pass in the environment env := CreateEnvironmentForPush(ctx, writeRequest) if err := git.Push(ctx, r.tmpPath, git.PushOptions{ Remote: r.remoteRepo.Path, - Branch: strings.TrimSpace(sourceRef) + ":" + gitReferenceNamePrefixBranch + strings.TrimSpace(branch), + Branch: sourceRef + ":" + GetReferenceFromBranchName(branch), Env: env, }); err != nil { if git.IsErrPushOutOfDate(err) { @@ -327,7 +344,8 @@ func (r *SharedRepo) GetBranchCommit(branch string) (*git.Commit, error) { if r.repo == nil { return nil, fmt.Errorf("repository has not been cloned") } - return r.repo.GetBranchCommit(branch) + + return r.repo.GetBranchCommit(strings.TrimPrefix(branch, gitReferenceNamePrefixBranch)) } // GetCommit Gets the commit object of the given commit ID. @@ -359,3 +377,18 @@ func CreateEnvironmentForPush(ctx context.Context, writeRequest *rpc.WriteReques return environ } + +// GetReferenceFromBranchName assumes the provided value is the branch name (not the ref!) +// and first sanitizes the branch name (remove any spaces or 'refs/heads/' prefix) +// It then returns the full form of the branch reference. +func GetReferenceFromBranchName(branchName string) string { + // remove spaces + branchName = strings.TrimSpace(branchName) + // remove `refs/heads/` prefix (shouldn't be there, but if it is remove it to try to avoid complications) + // NOTE: This is used to reduce missconfigurations via api + // TODO: block via CLI, too + branchName = strings.TrimPrefix(branchName, gitReferenceNamePrefixBranch) + + // return reference + return gitReferenceNamePrefixBranch + branchName +} diff --git a/gitrpc/internal/types/errors.go b/gitrpc/internal/types/errors.go index f1a7ded27..725af454d 100644 --- a/gitrpc/internal/types/errors.go +++ b/gitrpc/internal/types/errors.go @@ -10,18 +10,19 @@ import ( ) var ( - ErrAlreadyExists = errors.New("already exists") - ErrInvalidArgument = errors.New("invalid argument") - ErrNotFound = errors.New("not found") - ErrInvalidPath = errors.New("path is invalid") - ErrUndefinedAction = errors.New("undefined action") - ErrContentSentBeforeAction = errors.New("content sent before action") - ErrActionListEmpty = errors.New("no commit actions to perform on repository") - ErrHeaderCannotBeEmpty = errors.New("header field cannot be empty") - ErrBaseCannotBeEmpty = errors.New("base field cannot be empty") - ErrSHADoesNotMatch = errors.New("sha does not match") - ErrEmptyLeftCommitID = errors.New("empty LeftCommitId") - ErrEmptyRightCommitID = errors.New("empty RightCommitId") + ErrAlreadyExists = errors.New("already exists") + ErrInvalidArgument = errors.New("invalid argument") + ErrNotFound = errors.New("not found") + ErrInvalidPath = errors.New("path is invalid") + ErrUndefinedAction = errors.New("undefined action") + ErrActionNotAllowedOnEmptyRepo = errors.New("action not allowed on empty repository") + ErrContentSentBeforeAction = errors.New("content sent before action") + ErrActionListEmpty = errors.New("no commit actions to perform on repository") + ErrHeaderCannotBeEmpty = errors.New("header field cannot be empty") + ErrBaseCannotBeEmpty = errors.New("base field cannot be empty") + ErrSHADoesNotMatch = errors.New("sha does not match") + ErrEmptyLeftCommitID = errors.New("empty LeftCommitId") + ErrEmptyRightCommitID = errors.New("empty RightCommitId") ) // MergeConflictsError represents an error if merging fails with a conflict.