feat: [CODE-3279]: revert PR API (#3657)

* pass the patch filename as a param to git
* revert doesn't return conflict
* revert PR API
This commit is contained in:
Marko Gaćeša 2025-04-10 11:16:55 +00:00 committed by Harness
parent 2fdb6d6655
commit 5e1b1b407e
9 changed files with 467 additions and 1 deletions

View File

@ -0,0 +1,139 @@
// Copyright 2023 Harness, Inc.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
package pullreq
import (
"context"
"fmt"
"strconv"
"strings"
"time"
"github.com/harness/gitness/app/api/controller"
"github.com/harness/gitness/app/api/usererror"
"github.com/harness/gitness/app/auth"
"github.com/harness/gitness/errors"
"github.com/harness/gitness/git"
"github.com/harness/gitness/git/sha"
"github.com/harness/gitness/types"
"github.com/harness/gitness/types/enum"
)
type RevertInput struct {
Title string `json:"title"`
Message string `json:"message"`
// RevertBranch is the name of new branch that will be created on which the revert commit will be put.
// It's optional, if no value has been provided the default ("revert-pullreq-<number>") would be used.
RevertBranch string `json:"revert_branch"`
}
func (in *RevertInput) sanitize() {
in.Title = strings.TrimSpace(in.Title)
in.Message = strings.TrimSpace(in.Message)
in.RevertBranch = strings.TrimSpace(in.RevertBranch)
}
func (c *Controller) Revert(
ctx context.Context,
session *auth.Session,
repoRef string,
pullreqNum int64,
in *RevertInput,
) (*types.RevertResponse, error) {
in.sanitize()
repo, err := c.getRepoCheckAccess(ctx, session, repoRef, enum.PermissionRepoPush)
if err != nil {
return nil, fmt.Errorf("failed to acquire access to repo: %w", err)
}
pr, err := c.pullreqStore.FindByNumber(ctx, repo.ID, pullreqNum)
if err != nil {
return nil, fmt.Errorf("failed to acquire access to repo: %w", err)
}
if pr.State != enum.PullReqStateMerged {
return nil, usererror.BadRequest("Only merged pull requests can be reverted.")
}
readParams := git.CreateReadParams(repo)
writeParams, err := controller.CreateRPCInternalWriteParams(ctx, c.urlProvider, session, repo)
if err != nil {
return nil, fmt.Errorf("failed to create RPC write params: %w", err)
}
revertBranch := in.RevertBranch
if revertBranch == "" {
revertBranch = "revert-pullreq-" + strconv.FormatInt(pullreqNum, 10)
}
_, err = c.git.GetBranch(ctx, &git.GetBranchParams{
ReadParams: readParams,
BranchName: revertBranch,
})
if err != nil && !errors.IsNotFound(err) {
return nil, fmt.Errorf("failed to get revert branch: %w", err)
}
if err == nil {
return nil, errors.InvalidArgument("Branch %q already exists.", revertBranch)
}
title := in.Title
message := in.Message
if title == "" {
title = fmt.Sprintf("Revert of #%d", pr.Number)
}
commitMessage := git.CommitMessage(title, message)
author := controller.IdentityFromPrincipalInfo(*session.Principal.ToPrincipalInfo())
committer := controller.SystemServicePrincipalInfo()
now := time.Now()
result, err := c.git.Revert(ctx, &git.RevertParams{
WriteParams: writeParams,
ParentCommitSHA: sha.Must(*pr.MergeSHA),
FromCommitSHA: sha.Must(pr.MergeBaseSHA),
ToCommitSHA: sha.Must(pr.SourceSHA),
RevertBranch: revertBranch,
Message: commitMessage,
Committer: committer,
CommitterDate: &now,
Author: author,
AuthorDate: &now,
})
if err != nil {
return nil, fmt.Errorf("failed to revert pull request: %w", err)
}
gitCommit, err := c.git.GetCommit(ctx, &git.GetCommitParams{
ReadParams: readParams,
Revision: result.CommitSHA.String(),
})
if err != nil {
return nil, fmt.Errorf("failed to get revert commit: %w", err)
}
commit, err := controller.MapCommit(&gitCommit.Commit)
if err != nil {
return nil, fmt.Errorf("failed to map revert commit: %w", err)
}
return &types.RevertResponse{
Branch: revertBranch,
Commit: *commit,
}, nil
}

View File

@ -25,7 +25,7 @@ import (
"github.com/harness/gitness/app/api/request" "github.com/harness/gitness/app/api/request"
) )
// HandleCreate returns a http.HandlerFunc that creates a new pull request. // HandleMerge returns a http.HandlerFunc that merges the pull request.
func HandleMerge(pullreqCtrl *pullreq.Controller) http.HandlerFunc { func HandleMerge(pullreqCtrl *pullreq.Controller) http.HandlerFunc {
return func(w http.ResponseWriter, r *http.Request) { return func(w http.ResponseWriter, r *http.Request) {
ctx := r.Context() ctx := r.Context()

View File

@ -0,0 +1,58 @@
// Copyright 2023 Harness, Inc.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
package pullreq
import (
"encoding/json"
"net/http"
"github.com/harness/gitness/app/api/controller/pullreq"
"github.com/harness/gitness/app/api/render"
"github.com/harness/gitness/app/api/request"
)
func HandleRevert(pullreqCtrl *pullreq.Controller) http.HandlerFunc {
return func(w http.ResponseWriter, r *http.Request) {
ctx := r.Context()
session, _ := request.AuthSessionFrom(ctx)
repoRef, err := request.GetRepoRefFromPath(r)
if err != nil {
render.TranslatedUserError(ctx, w, err)
return
}
in := new(pullreq.RevertInput)
err = json.NewDecoder(r.Body).Decode(in)
if err != nil {
render.BadRequestf(ctx, w, "Invalid Request Body: %s.", err)
return
}
pullreqNumber, err := request.GetPullReqNumberFromPath(r)
if err != nil {
render.TranslatedUserError(ctx, w, err)
return
}
pr, err := pullreqCtrl.Revert(ctx, session, repoRef, pullreqNumber, in)
if err != nil {
render.TranslatedUserError(ctx, w, err)
return
}
render.JSON(w, http.StatusOK, pr)
}
}

View File

@ -704,6 +704,22 @@ func pullReqOperations(reflector *openapi3.Reflector) {
_ = reflector.Spec.AddOperation(http.MethodPost, _ = reflector.Spec.AddOperation(http.MethodPost,
"/repos/{repo_ref}/pullreq/{pullreq_number}/merge", mergePullReqOp) "/repos/{repo_ref}/pullreq/{pullreq_number}/merge", mergePullReqOp)
revertPullReqOp := openapi3.Operation{}
revertPullReqOp.WithTags("pullreq")
revertPullReqOp.WithMapOfAnything(map[string]interface{}{"operationId": "revertPullReqOp"})
_ = reflector.SetRequest(&revertPullReqOp, &struct {
pullReqRequest
pullreq.RevertInput
}{}, http.MethodPost)
_ = reflector.SetJSONResponse(&revertPullReqOp, new(types.RevertResponse), http.StatusOK)
_ = reflector.SetJSONResponse(&revertPullReqOp, new(usererror.Error), http.StatusBadRequest)
_ = reflector.SetJSONResponse(&revertPullReqOp, new(usererror.Error), http.StatusUnauthorized)
_ = reflector.SetJSONResponse(&revertPullReqOp, new(usererror.Error), http.StatusForbidden)
_ = reflector.SetJSONResponse(&revertPullReqOp, new(usererror.Error), http.StatusNotFound)
_ = reflector.SetJSONResponse(&revertPullReqOp, new(usererror.Error), http.StatusMethodNotAllowed)
_ = reflector.Spec.AddOperation(http.MethodPost,
"/repos/{repo_ref}/pullreq/{pullreq_number}/revert", revertPullReqOp)
opListCommits := openapi3.Operation{} opListCommits := openapi3.Operation{}
opListCommits.WithTags("pullreq") opListCommits.WithTags("pullreq")
opListCommits.WithMapOfAnything(map[string]interface{}{"operationId": "listPullReqCommits"}) opListCommits.WithMapOfAnything(map[string]interface{}{"operationId": "listPullReqCommits"})

View File

@ -690,6 +690,7 @@ func SetupPullReq(r chi.Router, pullreqCtrl *pullreq.Controller) {
r.Post("/", handlerpullreq.HandleReviewSubmit(pullreqCtrl)) r.Post("/", handlerpullreq.HandleReviewSubmit(pullreqCtrl))
}) })
r.Post("/merge", handlerpullreq.HandleMerge(pullreqCtrl)) r.Post("/merge", handlerpullreq.HandleMerge(pullreqCtrl))
r.Post("/revert", handlerpullreq.HandleRevert(pullreqCtrl))
r.Get("/commits", handlerpullreq.HandleCommits(pullreqCtrl)) r.Get("/commits", handlerpullreq.HandleCommits(pullreqCtrl))
r.Get("/metadata", handlerpullreq.HandleMetadata(pullreqCtrl)) r.Get("/metadata", handlerpullreq.HandleMetadata(pullreqCtrl))
r.Route("/branch", func(r chi.Router) { r.Route("/branch", func(r chi.Router) {

View File

@ -102,6 +102,8 @@ type Interface interface {
*/ */
Merge(ctx context.Context, in *MergeParams) (MergeOutput, error) Merge(ctx context.Context, in *MergeParams) (MergeOutput, error)
Revert(ctx context.Context, in *RevertParams) (RevertOutput, error)
/* /*
* Blame services * Blame services
*/ */

214
git/revert.go Normal file
View File

@ -0,0 +1,214 @@
// Copyright 2023 Harness, Inc.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
package git
import (
"context"
"fmt"
"os"
"time"
"github.com/harness/gitness/errors"
"github.com/harness/gitness/git/api"
gitenum "github.com/harness/gitness/git/enum"
"github.com/harness/gitness/git/hook"
"github.com/harness/gitness/git/parser"
"github.com/harness/gitness/git/sha"
"github.com/harness/gitness/git/sharedrepo"
"github.com/rs/zerolog/log"
)
// RevertParams is input structure object for the revert operation.
type RevertParams struct {
WriteParams
ParentCommitSHA sha.SHA
FromCommitSHA sha.SHA
ToCommitSHA sha.SHA
RevertBranch string
Message string
Committer *Identity
CommitterDate *time.Time
Author *Identity
AuthorDate *time.Time
}
func (p *RevertParams) Validate() error {
if err := p.WriteParams.Validate(); err != nil {
return err
}
if p.Message == "" {
return errors.InvalidArgument("commit message is empty")
}
if p.RevertBranch == "" {
return errors.InvalidArgument("revert branch is missing")
}
return nil
}
type RevertOutput struct {
CommitSHA sha.SHA
}
// Revert creates a revert commit. The revert commit contains all changes introduced
// by the commits between params.FromCommitSHA and params.ToCommitSHA.
// The newly created commit will have the parent set as params.ParentCommitSHA.
// This method can be used to revert a pull request:
// * params.ParentCommit = pr.MergeSHA
// * params.FromCommitSHA = pr.MergeBaseSHA
// * params.ToCommitSHA = pr.SourceSHA.
func (s *Service) Revert(ctx context.Context, params *RevertParams) (RevertOutput, error) {
err := params.Validate()
if err != nil {
return RevertOutput{}, fmt.Errorf("params not valid: %w", err)
}
repoPath := getFullPathForRepo(s.reposRoot, params.RepoUID)
// Check the merge base commit of the FromCommit and the ToCommit.
// We expect that the merge base would be equal to the FromCommit.
// This guaranties that the FromCommit is an ancestor of the ToCommit
// and that the two commits have simple commit history.
// The diff could be found with the simple 'git diff',
// rather than going though the merge base with 'git diff --merge-base'.
if params.FromCommitSHA.Equal(params.ToCommitSHA) {
return RevertOutput{}, errors.InvalidArgument("from and to commits can't be the same commit.")
}
mergeBaseCommitSHA, _, err := s.git.GetMergeBase(ctx, repoPath, "origin",
params.FromCommitSHA.String(), params.ToCommitSHA.String())
if err != nil {
return RevertOutput{}, fmt.Errorf("failed to get merge base: %w", err)
}
if !params.FromCommitSHA.Equal(mergeBaseCommitSHA) {
return RevertOutput{}, errors.InvalidArgument("from and to commits must not branch out.")
}
// Set the author and the committer. The rules for setting these are the same as for the Merge method.
now := time.Now().UTC()
committer := api.Signature{Identity: api.Identity(params.Actor), When: now}
if params.Committer != nil {
committer.Identity = api.Identity(*params.Committer)
}
if params.CommitterDate != nil {
committer.When = *params.CommitterDate
}
author := committer
if params.Author != nil {
author.Identity = api.Identity(*params.Author)
}
if params.AuthorDate != nil {
author.When = *params.AuthorDate
}
// Prepare the message for the revert commit.
message := parser.CleanUpWhitespace(params.Message)
// Reference updater
refRevertBranch, err := GetRefPath(params.RevertBranch, gitenum.RefTypeBranch)
if err != nil {
return RevertOutput{}, fmt.Errorf("failed to generate revert branch ref name: %w", err)
}
refUpdater, err := hook.CreateRefUpdater(s.hookClientFactory, params.EnvVars, repoPath)
if err != nil {
return RevertOutput{}, fmt.Errorf("failed to create reference updater: %w", err)
}
// Temp file to hold the diff patch.
diffPatch, err := os.CreateTemp(s.sharedRepoRoot, "revert-*.patch")
if err != nil {
return RevertOutput{}, fmt.Errorf("failed to create temporary file to hold the diff patch: %w", err)
}
diffPatchName := diffPatch.Name()
defer func() {
if err = os.Remove(diffPatchName); err != nil {
log.Ctx(ctx).Warn().Err(err).Str("path", diffPatchName).Msg("failed to remove temp file")
}
}()
// Create the revert commit
var commitSHA sha.SHA
err = sharedrepo.Run(ctx, refUpdater, s.sharedRepoRoot, repoPath, func(s *sharedrepo.SharedRepo) error {
if err := s.WriteDiff(ctx, params.ToCommitSHA.String(), params.FromCommitSHA.String(), diffPatch); err != nil {
return fmt.Errorf("failed to find diff between the two commits: %w", err)
}
if err := diffPatch.Close(); err != nil {
return fmt.Errorf("failed to close patch file: %w", err)
}
if err := s.SetIndex(ctx, params.ParentCommitSHA); err != nil {
return fmt.Errorf("failed to set parent commit index: %w", err)
}
if err := s.ApplyToIndex(ctx, diffPatchName); err != nil {
return fmt.Errorf("failed to apply revert diff: %w", err)
}
treeSHA, err := s.WriteTree(ctx)
if err != nil {
return fmt.Errorf("failed to write revert tree: %w", err)
}
commitSHA, err = s.CommitTree(ctx, &author, &committer, treeSHA, message, false, params.ParentCommitSHA)
if err != nil {
return fmt.Errorf("failed to create revert commit: %w", err)
}
refUpdates := []hook.ReferenceUpdate{
{
Ref: refRevertBranch,
Old: sha.Nil, // Expect that the revert branch doesn't exist.
New: commitSHA,
},
}
err = refUpdater.Init(ctx, refUpdates)
if err != nil {
return fmt.Errorf("failed to set init value of the revert reference %s: %w", refRevertBranch, err)
}
return nil
})
if err != nil {
return RevertOutput{}, fmt.Errorf("failed to revert: %w", err)
}
return RevertOutput{
CommitSHA: commitSHA,
}, nil
}

View File

@ -466,6 +466,37 @@ func (r *SharedRepo) MergeBase(
return strings.TrimSpace(stdout.String()), nil return strings.TrimSpace(stdout.String()), nil
} }
// WriteDiff runs git diff between two revisions and stores the output to the provided writer.
// The diff output would also include changes to binary files.
func (r *SharedRepo) WriteDiff(
ctx context.Context,
revFrom, revTo string,
wr io.Writer,
) error {
cmd := command.New("diff", command.WithFlag("--binary"),
command.WithArg(revFrom), command.WithArg(revTo))
if err := cmd.Run(ctx, command.WithDir(r.repoPath), command.WithStdout(wr)); err != nil {
return fmt.Errorf("failed to diff in shared repo: %w", err)
}
return nil
}
// ApplyToIndex runs 'git apply --cached' which would update the current index with the provided diff patch.
func (r *SharedRepo) ApplyToIndex(
ctx context.Context,
inputFileName string,
) error {
cmd := command.New("apply", command.WithFlag("--cached"), command.WithArg(inputFileName))
if err := cmd.Run(ctx, command.WithDir(r.repoPath)); err != nil {
return fmt.Errorf("failed to apply a patch in shared repo: %w", err)
}
return nil
}
func (r *SharedRepo) CreateFile( func (r *SharedRepo) CreateFile(
ctx context.Context, ctx context.Context,
treeishSHA sha.SHA, treeishSHA sha.SHA,

View File

@ -292,3 +292,8 @@ type PullReqRepo struct {
PullRequest *PullReq `json:"pull_request"` PullRequest *PullReq `json:"pull_request"`
Repository *RepositoryCore `json:"repository"` Repository *RepositoryCore `json:"repository"`
} }
type RevertResponse struct {
Branch string `json:"branch"`
Commit Commit `json:"commit"`
}