diff --git a/app/api/controller/pullreq/comment_apply_suggestions.go b/app/api/controller/pullreq/comment_apply_suggestions.go index e358f0af4..59fa00f95 100644 --- a/app/api/controller/pullreq/comment_apply_suggestions.go +++ b/app/api/controller/pullreq/comment_apply_suggestions.go @@ -84,9 +84,7 @@ func (i *CommentApplySuggestionsInput) sanitize() error { type CommentApplySuggestionsOutput struct { CommitID string `json:"commit_id"` - - DryRunRules bool `json:"dry_run_rules,omitempty"` - RuleViolations []types.RuleViolations `json:"rule_violations,omitempty"` + types.DryRunRulesOutput } // CommentApplySuggestions applies suggestions for code comments. @@ -138,8 +136,10 @@ func (c *Controller) CommentApplySuggestions( if in.DryRunRules { return CommentApplySuggestionsOutput{ - DryRunRules: true, - RuleViolations: violations, + DryRunRulesOutput: types.DryRunRulesOutput{ + DryRunRules: true, + RuleViolations: violations, + }, }, nil, nil } @@ -386,7 +386,9 @@ func (c *Controller) CommentApplySuggestions( } return CommentApplySuggestionsOutput{ - CommitID: commitOut.CommitID.String(), - RuleViolations: violations, + CommitID: commitOut.CommitID.String(), + DryRunRulesOutput: types.DryRunRulesOutput{ + RuleViolations: violations, + }, }, nil, nil } diff --git a/app/api/controller/repo/commit.go b/app/api/controller/repo/commit.go index 102c1bf3f..f6df9415a 100644 --- a/app/api/controller/repo/commit.go +++ b/app/api/controller/repo/commit.go @@ -97,8 +97,10 @@ func (c *Controller) CommitFiles(ctx context.Context, if in.DryRunRules { return types.CommitFilesResponse{ - DryRunRules: true, - RuleViolations: violations, + DryRunRulesOutput: types.DryRunRulesOutput{ + DryRunRules: true, + RuleViolations: violations, + }, }, nil, nil } @@ -154,7 +156,9 @@ func (c *Controller) CommitFiles(ctx context.Context, } return types.CommitFilesResponse{ - CommitID: commit.CommitID.String(), - RuleViolations: violations, + CommitID: commit.CommitID.String(), + DryRunRulesOutput: types.DryRunRulesOutput{ + RuleViolations: violations, + }, }, nil, nil } diff --git a/app/api/controller/repo/create_branch.go b/app/api/controller/repo/create_branch.go index 428ed25ef..5b6d23573 100644 --- a/app/api/controller/repo/create_branch.go +++ b/app/api/controller/repo/create_branch.go @@ -45,7 +45,7 @@ func (c *Controller) CreateBranch(ctx context.Context, session *auth.Session, repoRef string, in *CreateBranchInput, -) (*Branch, []types.RuleViolations, error) { +) (*types.Branch, []types.RuleViolations, error) { repo, err := c.getRepoCheckAccess(ctx, session, repoRef, enum.PermissionRepoPush) if err != nil { return nil, nil, err diff --git a/app/api/controller/repo/delete_branch.go b/app/api/controller/repo/delete_branch.go index 22168f1d2..7f63f9c62 100644 --- a/app/api/controller/repo/delete_branch.go +++ b/app/api/controller/repo/delete_branch.go @@ -34,10 +34,11 @@ func (c *Controller) DeleteBranch(ctx context.Context, branchName string, bypassRules bool, dryRunRules bool, -) ([]types.RuleViolations, error) { + commitSha string, +) (types.DeleteBranchOutput, []types.RuleViolations, error) { repo, err := c.getRepoCheckAccess(ctx, session, repoRef, enum.PermissionRepoPush) if err != nil { - return nil, err + return types.DeleteBranchOutput{}, nil, err } // make sure user isn't deleting the default branch @@ -45,12 +46,12 @@ func (c *Controller) DeleteBranch(ctx context.Context, // and 'refs/heads/branch1' would fail if 'branch1' exists. // TODO: Add functional test to ensure the scenario is covered! if branchName == repo.DefaultBranch { - return nil, usererror.ErrDefaultBranchCantBeDeleted + return types.DeleteBranchOutput{}, nil, usererror.ErrDefaultBranchCantBeDeleted } rules, isRepoOwner, err := c.fetchRules(ctx, session, repo) if err != nil { - return nil, err + return types.DeleteBranchOutput{}, nil, err } violations, err := rules.RefChangeVerify(ctx, protection.RefChangeVerifyInput{ @@ -63,28 +64,38 @@ func (c *Controller) DeleteBranch(ctx context.Context, RefNames: []string{branchName}, }) if err != nil { - return nil, fmt.Errorf("failed to verify protection rules: %w", err) - } - if protection.IsCritical(violations) { - return violations, nil + return types.DeleteBranchOutput{}, nil, fmt.Errorf("failed to verify protection rules: %w", err) } if dryRunRules { - return []types.RuleViolations{}, nil + return types.DeleteBranchOutput{ + DryRunRulesOutput: types.DryRunRulesOutput{ + DryRunRules: true, + RuleViolations: violations, + }, + }, nil, nil + } + + if protection.IsCritical(violations) { + return types.DeleteBranchOutput{}, violations, nil } writeParams, err := controller.CreateRPCInternalWriteParams(ctx, c.urlProvider, session, repo) if err != nil { - return nil, fmt.Errorf("failed to create RPC write params: %w", err) + return types.DeleteBranchOutput{}, nil, fmt.Errorf("failed to create RPC write params: %w", err) } err = c.git.DeleteBranch(ctx, &git.DeleteBranchParams{ WriteParams: writeParams, BranchName: branchName, + SHA: commitSha, }) if err != nil { - return nil, err + return types.DeleteBranchOutput{}, nil, err } - return nil, nil + return types.DeleteBranchOutput{ + DryRunRulesOutput: types.DryRunRulesOutput{ + RuleViolations: violations, + }}, nil, nil } diff --git a/app/api/controller/repo/get_branch.go b/app/api/controller/repo/get_branch.go index a1a06521f..2e444c9ac 100644 --- a/app/api/controller/repo/get_branch.go +++ b/app/api/controller/repo/get_branch.go @@ -20,6 +20,7 @@ import ( "github.com/harness/gitness/app/auth" "github.com/harness/gitness/git" + "github.com/harness/gitness/types" "github.com/harness/gitness/types/enum" ) @@ -28,7 +29,7 @@ func (c *Controller) GetBranch(ctx context.Context, session *auth.Session, repoRef string, branchName string, -) (*Branch, error) { +) (*types.Branch, error) { repo, err := c.getRepoCheckAccess(ctx, session, repoRef, enum.PermissionRepoView) if err != nil { return nil, err diff --git a/app/api/controller/repo/list_branches.go b/app/api/controller/repo/list_branches.go index a464da158..4d9a3062c 100644 --- a/app/api/controller/repo/list_branches.go +++ b/app/api/controller/repo/list_branches.go @@ -25,19 +25,13 @@ import ( "github.com/harness/gitness/types/enum" ) -type Branch struct { - Name string `json:"name"` - SHA string `json:"sha"` - Commit *types.Commit `json:"commit,omitempty"` -} - // ListBranches lists the branches of a repo. func (c *Controller) ListBranches(ctx context.Context, session *auth.Session, repoRef string, includeCommit bool, filter *types.BranchFilter, -) ([]Branch, error) { +) ([]types.Branch, error) { repo, err := c.getRepoCheckAccess(ctx, session, repoRef, enum.PermissionRepoView) if err != nil { return nil, err @@ -56,7 +50,7 @@ func (c *Controller) ListBranches(ctx context.Context, return nil, err } - branches := make([]Branch, len(rpcOut.Branches)) + branches := make([]types.Branch, len(rpcOut.Branches)) for i := range rpcOut.Branches { branches[i], err = mapBranch(rpcOut.Branches[i]) if err != nil { @@ -95,16 +89,16 @@ func mapToRPCSortOrder(o enum.Order) git.SortOrder { } } -func mapBranch(b git.Branch) (Branch, error) { +func mapBranch(b git.Branch) (types.Branch, error) { var commit *types.Commit if b.Commit != nil { var err error commit, err = controller.MapCommit(b.Commit) if err != nil { - return Branch{}, err + return types.Branch{}, err } } - return Branch{ + return types.Branch{ Name: b.Name, SHA: b.SHA.String(), Commit: commit, diff --git a/app/api/handler/repo/delete_branch.go b/app/api/handler/repo/delete_branch.go index 5b569c54a..b275b76db 100644 --- a/app/api/handler/repo/delete_branch.go +++ b/app/api/handler/repo/delete_branch.go @@ -52,15 +52,18 @@ func HandleDeleteBranch(repoCtrl *repo.Controller) http.HandlerFunc { return } - violations, err := repoCtrl.DeleteBranch(ctx, session, repoRef, branchName, bypassRules, dryRunRules) + commitSha := request.GetCommitSHAFromQueryOrDefault(r) + + out, violations, err := repoCtrl.DeleteBranch(ctx, session, repoRef, branchName, bypassRules, dryRunRules, commitSha) if err != nil { render.TranslatedUserError(ctx, w, err) + return } if violations != nil { render.Violations(w, violations) return } - render.DeleteSuccessful(w) + render.JSON(w, http.StatusOK, out) } } diff --git a/app/api/openapi/repo.go b/app/api/openapi/repo.go index 85b6640f7..1039e3090 100644 --- a/app/api/openapi/repo.go +++ b/app/api/openapi/repo.go @@ -542,6 +542,20 @@ var queryParameterDryRunRules = openapi3.ParameterOrRef{ }, } +var queryParameterCommitSHA = openapi3.ParameterOrRef{ + Parameter: &openapi3.Parameter{ + Name: request.QueryParamCommitSHA, + In: openapi3.ParameterInQuery, + Description: ptr.String("Commit SHA the branch is at"), + Required: ptr.Bool(false), + Schema: &openapi3.SchemaOrRef{ + Schema: &openapi3.Schema{ + Type: ptrSchemaType(openapi3.SchemaTypeString), + }, + }, + }, +} + var queryParameterDeletedAt = openapi3.ParameterOrRef{ Parameter: &openapi3.Parameter{ Name: request.QueryParamDeletedAt, @@ -900,7 +914,7 @@ func repoOperations(reflector *openapi3.Reflector) { opCreateBranch.WithTags("repository") opCreateBranch.WithMapOfAnything(map[string]interface{}{"operationId": "createBranch"}) _ = reflector.SetRequest(&opCreateBranch, new(createBranchRequest), http.MethodPost) - _ = reflector.SetJSONResponse(&opCreateBranch, new(repo.Branch), http.StatusCreated) + _ = reflector.SetJSONResponse(&opCreateBranch, new(types.Branch), http.StatusCreated) _ = reflector.SetJSONResponse(&opCreateBranch, new(usererror.Error), http.StatusBadRequest) _ = reflector.SetJSONResponse(&opCreateBranch, new(usererror.Error), http.StatusInternalServerError) _ = reflector.SetJSONResponse(&opCreateBranch, new(usererror.Error), http.StatusUnauthorized) @@ -912,7 +926,7 @@ func repoOperations(reflector *openapi3.Reflector) { opGetBranch.WithTags("repository") opGetBranch.WithMapOfAnything(map[string]interface{}{"operationId": "getBranch"}) _ = reflector.SetRequest(&opGetBranch, new(getBranchRequest), http.MethodGet) - _ = reflector.SetJSONResponse(&opGetBranch, new(repo.Branch), http.StatusOK) + _ = reflector.SetJSONResponse(&opGetBranch, new(types.Branch), http.StatusOK) _ = reflector.SetJSONResponse(&opGetBranch, new(usererror.Error), http.StatusInternalServerError) _ = reflector.SetJSONResponse(&opGetBranch, new(usererror.Error), http.StatusUnauthorized) _ = reflector.SetJSONResponse(&opGetBranch, new(usererror.Error), http.StatusForbidden) @@ -922,9 +936,9 @@ func repoOperations(reflector *openapi3.Reflector) { opDeleteBranch := openapi3.Operation{} opDeleteBranch.WithTags("repository") opDeleteBranch.WithMapOfAnything(map[string]interface{}{"operationId": "deleteBranch"}) - opDeleteBranch.WithParameters(queryParameterBypassRules, queryParameterDryRunRules) + opDeleteBranch.WithParameters(queryParameterBypassRules, queryParameterDryRunRules, queryParameterCommitSHA) _ = reflector.SetRequest(&opDeleteBranch, new(deleteBranchRequest), http.MethodDelete) - _ = reflector.SetJSONResponse(&opDeleteBranch, nil, http.StatusNoContent) + _ = reflector.SetJSONResponse(&opDeleteBranch, new(types.DeleteBranchOutput), http.StatusOK) _ = reflector.SetJSONResponse(&opDeleteBranch, new(usererror.Error), http.StatusInternalServerError) _ = reflector.SetJSONResponse(&opDeleteBranch, new(usererror.Error), http.StatusUnauthorized) _ = reflector.SetJSONResponse(&opDeleteBranch, new(usererror.Error), http.StatusForbidden) @@ -939,7 +953,7 @@ func repoOperations(reflector *openapi3.Reflector) { queryParameterQueryBranches, queryParameterOrder, queryParameterSortBranch, QueryParameterPage, QueryParameterLimit) _ = reflector.SetRequest(&opListBranches, new(listBranchesRequest), http.MethodGet) - _ = reflector.SetJSONResponse(&opListBranches, []repo.Branch{}, http.StatusOK) + _ = reflector.SetJSONResponse(&opListBranches, []types.Branch{}, http.StatusOK) _ = reflector.SetJSONResponse(&opListBranches, new(usererror.Error), http.StatusInternalServerError) _ = reflector.SetJSONResponse(&opListBranches, new(usererror.Error), http.StatusUnauthorized) _ = reflector.SetJSONResponse(&opListBranches, new(usererror.Error), http.StatusForbidden) diff --git a/app/api/request/git.go b/app/api/request/git.go index ec6f63b72..25039d2b3 100644 --- a/app/api/request/git.go +++ b/app/api/request/git.go @@ -27,10 +27,13 @@ import ( ) const ( + HeaderParamGitProtocol = "Git-Protocol" + + PathParamCommitSHA = "commit_sha" + QueryParamGitRef = "git_ref" QueryParamIncludeCommit = "include_commit" QueryParamIncludeDirectories = "include_directories" - PathParamCommitSHA = "commit_sha" QueryParamLineFrom = "line_from" QueryParamLineTo = "line_to" QueryParamPath = "path" @@ -40,7 +43,7 @@ const ( QueryParamIncludeStats = "include_stats" QueryParamInternal = "internal" QueryParamService = "service" - HeaderParamGitProtocol = "Git-Protocol" + QueryParamCommitSHA = "commit_sha" ) func GetGitRefFromQueryOrDefault(r *http.Request, deflt string) string { @@ -172,3 +175,7 @@ func GetFileDiffFromQuery(r *http.Request) (files gittypes.FileDiffRequests) { } return } + +func GetCommitSHAFromQueryOrDefault(r *http.Request) string { + return QueryParamOrDefault(r, QueryParamCommitSHA, "") +} diff --git a/git/branch.go b/git/branch.go index 391514c0f..75817ea72 100644 --- a/git/branch.go +++ b/git/branch.go @@ -73,6 +73,7 @@ type DeleteBranchParams struct { WriteParams // BranchName is the name of the branch BranchName string + SHA string } type ListBranchesParams struct { @@ -162,13 +163,14 @@ func (s *Service) DeleteBranch(ctx context.Context, params *DeleteBranchParams) repoPath := getFullPathForRepo(s.reposRoot, params.RepoUID) branchRef := api.GetReferenceFromBranchName(params.BranchName) + commitSha, _ := sha.NewOrEmpty(params.SHA) refUpdater, err := hook.CreateRefUpdater(s.hookClientFactory, params.EnvVars, repoPath, branchRef) if err != nil { return fmt.Errorf("failed to create ref updater to create the branch: %w", err) } - err = refUpdater.Do(ctx, sha.None, sha.Nil) // delete whatever is there + err = refUpdater.Do(ctx, commitSha, sha.Nil) if errors.IsNotFound(err) { return errors.NotFound("branch %q does not exist", params.BranchName) } diff --git a/types/branch.go b/types/branch.go new file mode 100644 index 000000000..6d2be4115 --- /dev/null +++ b/types/branch.go @@ -0,0 +1,25 @@ +// 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 types + +type Branch struct { + Name string `json:"name"` + SHA string `json:"sha"` + Commit *Commit `json:"commit,omitempty"` +} + +type DeleteBranchOutput struct { + DryRunRulesOutput +} diff --git a/types/commit.go b/types/commit.go index a56f6c1a9..1e73533d2 100644 --- a/types/commit.go +++ b/types/commit.go @@ -16,7 +16,6 @@ package types // CommitFilesResponse holds commit id. type CommitFilesResponse struct { - DryRunRules bool `json:"dry_run_rules,omitempty"` - CommitID string `json:"commit_id"` - RuleViolations []RuleViolations `json:"rule_violations,omitempty"` + CommitID string `json:"commit_id"` + DryRunRulesOutput } diff --git a/types/rule.go b/types/rule.go index 76cabb4fd..b3545516b 100644 --- a/types/rule.go +++ b/types/rule.go @@ -205,3 +205,8 @@ type RulesViolations struct { Message string `json:"message"` Violations []RuleViolations `json:"violations"` } + +type DryRunRulesOutput struct { + DryRunRules bool `json:"dry_run_rules,omitempty"` + RuleViolations []RuleViolations `json:"rule_violations,omitempty"` +}