xxq250
/
gitness
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

allow optional rule bypass (#764)

This commit is contained in:
Marko Gacesa 2023-11-03 18:31:10 +00:00 committed by Harness
parent afb21a3ca3
commit 941bc7a0fd
19 changed files with 488 additions and 90 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
app/api/controller/githook/pre_receive.go
View File

@ -123,6 +123,7 @@ func (c *Controller) checkProtectionRules(
violations, err := protectionRules.RefChangeVerify(ctx, protection.RefChangeVerifyInput{
Actor: &session.Principal,
AllowBypass: true,
IsRepoOwner: isRepoOwner,
Repo: repo,
RefAction: refAction,

9
app/api/controller/pullreq/merge.go
View File

@ -37,9 +37,10 @@ import (
)
type MergeInput struct {
Method enum.MergeMethod `json:"method"`
SourceSHA string `json:"source_sha"`
DryRun bool `json:"dry_run"`
Method enum.MergeMethod `json:"method"`
SourceSHA string `json:"source_sha"`
BypassRules bool `json:"bypass_rules"`
DryRun bool `json:"dry_run"`
}
// Merge merges the pull request.
@ -93,6 +94,7 @@ func (c *Controller) Merge(
return nil, nil, usererror.BadRequest("Pull request must be open")
}
// TODO: Uncomment when the front-end side starts sending SourceSHA.
/*
if pr.SourceSHA != in.SourceSHA {
return nil, nil,
@ -153,6 +155,7 @@ func (c *Controller) Merge(
ruleOut, violations, err := protectionRules.MergeVerify(ctx, protection.MergeVerifyInput{
Actor: &session.Principal,
AllowBypass: in.BypassRules,
IsRepoOwner: isRepoOwner,
TargetRepo: targetRepo,
SourceRepo: sourceRepo,

1
app/api/controller/repo/commit.go
View File

@ -79,6 +79,7 @@ func (c *Controller) CommitFiles(ctx context.Context,
violations, err := rules.RefChangeVerify(ctx, protection.RefChangeVerifyInput{
Actor: &session.Principal,
AllowBypass: true,
IsRepoOwner: isRepoOwner,
Repo: repo,
RefAction: refAction,

1
app/api/controller/repo/create_branch.go
View File

@ -58,6 +58,7 @@ func (c *Controller) CreateBranch(ctx context.Context,
violations, err := rules.RefChangeVerify(ctx, protection.RefChangeVerifyInput{
Actor: &session.Principal,
AllowBypass: true,
IsRepoOwner: isRepoOwner,
Repo: repo,
RefAction: protection.RefActionCreate,

1
app/api/controller/repo/create_commit_tag.go
View File

@ -62,6 +62,7 @@ func (c *Controller) CreateCommitTag(ctx context.Context,
violations, err := rules.RefChangeVerify(ctx, protection.RefChangeVerifyInput{
Actor: &session.Principal,
AllowBypass: true,
IsRepoOwner: isRepoOwner,
Repo: repo,
RefAction: protection.RefActionCreate,

1
app/api/controller/repo/delete_branch.go
View File

@ -53,6 +53,7 @@ func (c *Controller) DeleteBranch(ctx context.Context,
violations, err := rules.RefChangeVerify(ctx, protection.RefChangeVerifyInput{
Actor: &session.Principal,
AllowBypass: true,
IsRepoOwner: isRepoOwner,
Repo: repo,
RefAction: protection.RefActionDelete,

1
app/api/controller/repo/delete_tag.go
View File

@ -44,6 +44,7 @@ func (c *Controller) DeleteTag(ctx context.Context,
violations, err := rules.RefChangeVerify(ctx, protection.RefChangeVerifyInput{
Actor: &session.Principal,
AllowBypass: true,
IsRepoOwner: isRepoOwner,
Repo: repo,
RefAction: protection.RefActionDelete,

11
app/services/protection/bypass.go
View File

@ -16,6 +16,10 @@ package protection
import (
"fmt"
"github.com/harness/gitness/types"
"golang.org/x/exp/slices"
)
type DefBypass struct {
@ -23,6 +27,13 @@ type DefBypass struct {
RepoOwners bool `json:"repo_owners,omitempty"`
}
func (v DefBypass) matches(actor *types.Principal, isRepoOwner bool) bool {
return actor != nil &&
(actor.Admin ||
v.RepoOwners && isRepoOwner ||
slices.Contains(v.UserIDs, actor.ID))
}
func (v DefBypass) Sanitize() error {
if err := validateIDSlice(v.UserIDs); err != nil {
return fmt.Errorf("user IDs error: %w", err)

85
app/services/protection/bypass_test.go Normal file
View File

@ -0,0 +1,85 @@
// 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 protection
import (
"testing"
"github.com/harness/gitness/types"
)
func TestBranch_matches(t *testing.T) {
user := &types.Principal{ID: 42}
admin := &types.Principal{ID: 66, Admin: true}
tests := []struct {
name string
bypass DefBypass
actor *types.Principal
owner bool
exp bool
}{
{
name: "empty",
bypass: DefBypass{UserIDs: nil, RepoOwners: false},
actor: user,
exp: false,
},
{
name: "admin",
bypass: DefBypass{UserIDs: nil, RepoOwners: false},
actor: admin,
exp: true,
},
{
name: "repo-owners-false",
bypass: DefBypass{UserIDs: nil, RepoOwners: false},
actor: user,
owner: true,
exp: false,
},
{
name: "repo-owners-true",
bypass: DefBypass{UserIDs: nil, RepoOwners: true},
actor: user,
owner: true,
exp: true,
},
{
name: "selected-false",
bypass: DefBypass{UserIDs: []int64{1, 66}, RepoOwners: false},
actor: user,
exp: false,
},
{
name: "selected-true",
bypass: DefBypass{UserIDs: []int64{1, 42, 66}, RepoOwners: false},
actor: user,
exp: true,
},
}
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
if err := test.bypass.Sanitize(); err != nil {
t.Errorf("invalid: %s", err.Error())
}
if want, got := test.exp, test.bypass.matches(test.actor, test.owner); want != got {
t.Errorf("want=%t got=%t", want, got)
}
})
}
}

2
app/services/protection/pattern.go
View File

@ -74,7 +74,7 @@ func (p *Pattern) Matches(branchName, defaultName string) bool {
func patternValidate(pattern string) error {
if pattern == "" {
return ErrPatternEmptyPattern
return ErrPatternEmpty
}
_, err := doublestar.Match(pattern, "test")
if err != nil {

4
app/services/protection/pattern_test.go
View File

@ -124,12 +124,12 @@ func TestPattern_Validate(t *testing.T) {
{
name: "empty-include-globstar",
pattern: Pattern{Default: false, Include: []string{""}, Exclude: nil},
expect: ErrPatternEmptyPattern,
expect: ErrPatternEmpty,
},
{
name: "empty-exclude-globstar",
pattern: Pattern{Default: false, Include: nil, Exclude: []string{""}},
expect: ErrPatternEmptyPattern,
expect: ErrPatternEmpty,
},
{
name: "bad-include-pattern",

39
app/services/protection/rule_branch.go
View File

@ -19,8 +19,6 @@ import (
"fmt"
"github.com/harness/gitness/types"
"golang.org/x/exp/slices"
)
var TypeBranch types.RuleType = "branch"
@ -37,27 +35,39 @@ var (
_ Definition = (*Branch)(nil)
)
//nolint:gocognit // well aware of this
func (v *Branch) MergeVerify(
ctx context.Context,
in MergeVerifyInput,
) (MergeVerifyOutput, []types.RuleViolations, error) {
if v.isBypassed(in.Actor, in.IsRepoOwner) {
return MergeVerifyOutput{}, nil, nil
) (out MergeVerifyOutput, violations []types.RuleViolations, err error) {
out, violations, err = v.PullReq.MergeVerify(ctx, in)
if err != nil {
return
}
return v.PullReq.MergeVerify(ctx, in)
bypassed := in.AllowBypass && v.Bypass.matches(in.Actor, in.IsRepoOwner)
for i := range violations {
violations[i].Bypassed = bypassed
}
return
}
func (v *Branch) RefChangeVerify(
ctx context.Context,
in RefChangeVerifyInput,
) ([]types.RuleViolations, error) {
if v.isBypassed(in.Actor, in.IsRepoOwner) || in.RefType != RefTypeBranch || len(in.RefNames) == 0 {
return nil, nil
) (violations []types.RuleViolations, err error) {
if in.RefType != RefTypeBranch || len(in.RefNames) == 0 {
return []types.RuleViolations{}, nil
}
return v.Lifecycle.RefChangeVerify(ctx, in)
violations, err = v.Lifecycle.RefChangeVerify(ctx, in)
bypassed := in.AllowBypass && v.Bypass.matches(in.Actor, in.IsRepoOwner)
for i := range violations {
violations[i].Bypassed = bypassed
}
return
}
func (v *Branch) Sanitize() error {
@ -75,10 +85,3 @@ func (v *Branch) Sanitize() error {
return nil
}
func (v *Branch) isBypassed(actor *types.Principal, isRepoOwner bool) bool {
return actor != nil &&
(actor.Admin ||
v.Bypass.RepoOwners && isRepoOwner ||
slices.Contains(v.Bypass.UserIDs, actor.ID))
}

248
app/services/protection/rule_branch_test.go
View File

@ -15,73 +15,239 @@
package protection
import (
"context"
"testing"
"github.com/harness/gitness/types"
)
func TestBranch_isBypass(t *testing.T) {
// nolint:gocognit // it's a unit test
func TestBranch_MergeVerify(t *testing.T) {
user := &types.Principal{ID: 42}
admin := &types.Principal{ID: 66, Admin: true}
tests := []struct {
name string
bypass DefBypass
actor *types.Principal
owner bool
exp bool
branch Branch
in MergeVerifyInput
expOut MergeVerifyOutput
expVs []types.RuleViolations
}{
{
name: "empty",
bypass: DefBypass{UserIDs: nil, RepoOwners: false},
actor: user,
exp: false,
branch: Branch{},
in: MergeVerifyInput{Actor: user},
expOut: MergeVerifyOutput{},
expVs: []types.RuleViolations{},
},
{
name: "admin",
bypass: DefBypass{UserIDs: nil, RepoOwners: false},
actor: admin,
exp: true,
name: "admin-no-bypass",
branch: Branch{
Bypass: DefBypass{},
PullReq: DefPullReq{
StatusChecks: DefStatusChecks{RequireUIDs: []string{"abc"}},
Comments: DefComments{RequireResolveAll: true},
Merge: DefMerge{DeleteBranch: true},
},
},
in: MergeVerifyInput{
Actor: admin,
AllowBypass: false,
PullReq: &types.PullReq{UnresolvedCount: 1},
},
expOut: MergeVerifyOutput{
DeleteSourceBranch: true,
},
expVs: []types.RuleViolations{
{
Bypassed: false,
Violations: []types.Violation{
{Code: codePullReqCommentsReqResolveAll},
{Code: codePullReqStatusChecksReqUIDs},
},
},
},
},
{
name: "repo-owners-false",
bypass: DefBypass{UserIDs: nil, RepoOwners: false},
actor: user,
owner: true,
exp: false,
},
{
name: "repo-owners-true",
bypass: DefBypass{UserIDs: nil, RepoOwners: true},
actor: user,
owner: true,
exp: true,
},
{
name: "selected-false",
bypass: DefBypass{UserIDs: []int64{1, 66}, RepoOwners: false},
actor: user,
exp: false,
},
{
name: "selected-true",
bypass: DefBypass{UserIDs: []int64{1, 42, 66}, RepoOwners: false},
actor: user,
exp: true,
name: "user-bypass",
branch: Branch{
Bypass: DefBypass{UserIDs: []int64{user.ID}},
PullReq: DefPullReq{
StatusChecks: DefStatusChecks{RequireUIDs: []string{"abc"}},
Comments: DefComments{RequireResolveAll: true},
Merge: DefMerge{DeleteBranch: true},
},
},
in: MergeVerifyInput{
Actor: user,
AllowBypass: true,
PullReq: &types.PullReq{UnresolvedCount: 1},
},
expOut: MergeVerifyOutput{
DeleteSourceBranch: true,
},
expVs: []types.RuleViolations{
{
Bypassed: true,
Violations: []types.Violation{
{Code: codePullReqCommentsReqResolveAll},
{Code: codePullReqStatusChecksReqUIDs},
},
},
},
},
}
ctx := context.Background()
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
branch := Branch{Bypass: test.bypass}
if err := branch.Sanitize(); err != nil {
if err := test.branch.Sanitize(); err != nil {
t.Errorf("invalid: %s", err.Error())
return
}
if want, got := test.exp, branch.isBypassed(test.actor, test.owner); want != got {
t.Errorf("want=%t got=%t", want, got)
out, results, err := test.branch.MergeVerify(ctx, test.in)
if err != nil {
t.Errorf("error: %s", err.Error())
return
}
if want, got := test.expOut, out; want != got {
t.Errorf("output: want=%+v got=%+v", want, got)
}
if want, got := len(test.expVs), len(results); want != got {
t.Errorf("number of violations mismatch: want=%d got=%d", want, got)
return
}
for i := range results {
if want, got := test.expVs[i].Bypassed, results[i].Bypassed; want != got {
t.Errorf("rule result %d, bypassed mismatch: want=%t got=%t", i, want, got)
return
}
if want, got := len(test.expVs[i].Violations), len(results[i].Violations); want != got {
t.Errorf("rule result %d, violations count mismatch: want=%d got=%d", i, want, got)
return
}
for j := range results[i].Violations {
if want, got := test.expVs[i].Violations[j].Code, results[i].Violations[j].Code; want != got {
t.Errorf("rule result %d, violation %d, code mismatch: want=%s got=%s", i, j, want, got)
}
}
}
})
}
}
// nolint:gocognit // it's a unit test
func TestBranch_RefChangeVerify(t *testing.T) {
user := &types.Principal{ID: 42}
admin := &types.Principal{ID: 66, Admin: true}
tests := []struct {
name string
branch Branch
in RefChangeVerifyInput
expVs []types.RuleViolations
}{
{
name: "empty",
branch: Branch{
Bypass: DefBypass{},
Lifecycle: DefLifecycle{},
},
in: RefChangeVerifyInput{
Actor: user,
},
expVs: []types.RuleViolations{},
},
{
name: "admin-no-bypass",
branch: Branch{
Bypass: DefBypass{},
Lifecycle: DefLifecycle{DeleteForbidden: true},
},
in: RefChangeVerifyInput{
Actor: admin,
AllowBypass: false,
RefAction: RefActionDelete,
RefType: RefTypeBranch,
RefNames: []string{"abc"},
},
expVs: []types.RuleViolations{
{
Bypassed: false,
Violations: []types.Violation{
{Code: codeLifecycleDelete},
},
},
},
},
{
name: "owner-bypass",
branch: Branch{
Bypass: DefBypass{RepoOwners: true},
Lifecycle: DefLifecycle{DeleteForbidden: true},
},
in: RefChangeVerifyInput{
Actor: admin,
AllowBypass: true,
IsRepoOwner: true,
RefAction: RefActionDelete,
RefType: RefTypeBranch,
RefNames: []string{"abc"},
},
expVs: []types.RuleViolations{
{
Bypassed: true,
Violations: []types.Violation{
{Code: codeLifecycleDelete},
},
},
},
},
}
ctx := context.Background()
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
if err := test.branch.Sanitize(); err != nil {
t.Errorf("invalid: %s", err.Error())
return
}
results, err := test.branch.RefChangeVerify(ctx, test.in)
if err != nil {
t.Errorf("error: %s", err.Error())
return
}
if want, got := len(test.expVs), len(results); want != got {
t.Errorf("number of violations mismatch: want=%d got=%d", want, got)
return
}
for i := range results {
if want, got := test.expVs[i].Bypassed, results[i].Bypassed; want != got {
t.Errorf("rule result %d, bypassed mismatch: want=%t got=%t", i, want, got)
return
}
if want, got := len(test.expVs[i].Violations), len(results[i].Violations); want != got {
t.Errorf("rule result %d, violations count mismatch: want=%d got=%d", i, want, got)
return
}
for j := range results[i].Violations {
if want, got := test.expVs[i].Violations[j].Code, results[i].Violations[j].Code; want != got {
t.Errorf("rule result %d, violation %d, code mismatch: want=%s got=%s", i, j, want, got)
}
}
}
})
}

14
app/services/protection/service.go
View File

@ -54,7 +54,7 @@ type (
var (
ErrUnrecognizedType = errors.New("unrecognized protection type")
ErrAlreadyRegistered = errors.New("protection type already registered")
ErrPatternEmptyPattern = errors.New("name pattern can't be empty")
ErrPatternEmpty = errors.New("name pattern can't be empty")
ErrInvalidGlobstarPattern = errors.New("invalid globstar pattern")
)
@ -132,15 +132,3 @@ func (m *Manager) ForRepository(ctx context.Context, repoID int64) (Protection,
manager: m,
}, nil
}
func (m *Manager) MergeVerifierForRepository(ctx context.Context, repoID int64) (MergeVerifier, error) {
ruleInfos, err := m.ruleStore.ListAllRepoRules(ctx, repoID)
if err != nil {
return nil, fmt.Errorf("failed to list rules for repository: %w", err)
}
return ruleSet{
rules: ruleInfos,
manager: m,
}, nil
}

133
app/services/protection/service_test.go Normal file
View File

@ -0,0 +1,133 @@
// 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 protection
import (
"encoding/json"
"testing"
"github.com/harness/gitness/types"
"github.com/harness/gitness/types/enum"
)
func TestIsCritical(t *testing.T) {
tests := []struct {
name string
input []types.RuleViolations
exp bool
}{
{
name: "empty",
input: []types.RuleViolations{},
exp: false,
},
{
name: "non-critical",
input: []types.RuleViolations{
{
Rule: types.RuleInfo{State: enum.RuleStateMonitor},
Bypassed: false,
Violations: []types.Violation{{Code: "x"}, {Code: "x"}},
},
{
Rule: types.RuleInfo{State: enum.RuleStateActive},
Bypassed: true,
Violations: []types.Violation{{Code: "x"}, {Code: "x"}},
},
{
Rule: types.RuleInfo{State: enum.RuleStateActive},
Bypassed: false,
Violations: []types.Violation{},
},
},
exp: false,
},
{
name: "critical",
input: []types.RuleViolations{
{
Rule: types.RuleInfo{State: enum.RuleStateActive},
Bypassed: false,
Violations: []types.Violation{{Code: "x"}, {Code: "x"}},
},
},
exp: true,
},
}
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
if want, got := test.exp, IsCritical(test.input); want != got {
t.Errorf("want=%t got=%t", want, got)
}
})
}
}
func TestManager_SanitizeJSON(t *testing.T) {
tests := []struct {
name string
ruleTypes []types.RuleType
ruleType types.RuleType
errReg error
errSan error
}{
{
name: "success",
ruleTypes: []types.RuleType{TypeBranch},
ruleType: TypeBranch,
},
{
name: "duplicate",
ruleTypes: []types.RuleType{TypeBranch, TypeBranch},
ruleType: TypeBranch,
errReg: ErrAlreadyRegistered,
},
{
name: "unregistered",
ruleTypes: []types.RuleType{},
ruleType: TypeBranch,
errSan: ErrUnrecognizedType,
},
}
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
m := NewManager(nil)
err := func() error {
for _, ruleType := range test.ruleTypes {
err := m.Register(ruleType, func() Definition { return &Branch{} })
if err != nil {
return err
}
}
return nil
}()
// nolint:errorlint // deliberately comparing errors with ==
if test.errReg != err {
t.Errorf("register type error mismatch: want=%v got=%v", test.errReg, err)
return
}
_, err = m.SanitizeJSON(test.ruleType, json.RawMessage("{}"))
// nolint:errorlint // deliberately comparing errors with ==
if test.errSan != err {
t.Errorf("register type error mismatch: want=%v got=%v", test.errSan, err)
return
}
})
}
}

10
app/services/protection/set.go
View File

@ -95,16 +95,8 @@ func (s ruleSet) RefChangeVerify(ctx context.Context, in RefChangeVerifyInput) (
}
func backFillRule(vs []types.RuleViolations, rule types.RuleInfo) []types.RuleViolations {
violations := make([]types.RuleViolations, 0, len(vs))
for i := range vs {
if len(vs[i].Violations) == 0 {
continue
}
vs[i].Rule = rule
violations = append(violations, vs[i])
}
return violations
return vs
}

7
app/services/protection/verify_lifecycle.go
View File

@ -27,6 +27,7 @@ type (
RefChangeVerifyInput struct {
Actor *types.Principal
AllowBypass bool
IsRepoOwner bool
Repo *types.Repository
RefAction RefAction
@ -90,7 +91,11 @@ func (v *DefLifecycle) RefChangeVerify(_ context.Context, in RefChangeVerifyInpu
}
}
return []types.RuleViolations{violations}, nil
if len(violations.Violations) > 0 {
return []types.RuleViolations{violations}, nil
}
return nil, nil
}
func (*DefLifecycle) Sanitize() error {

7
app/services/protection/verify_pullreq.go
View File

@ -33,6 +33,7 @@ type (
MergeVerifyInput struct {
Actor *types.Principal
AllowBypass bool
IsRepoOwner bool
Membership *types.Membership
TargetRepo *types.Repository
@ -163,7 +164,11 @@ func (v *DefPullReq) MergeVerify(
}
}
return out, []types.RuleViolations{violations}, nil
if len(violations.Violations) > 0 {
return out, []types.RuleViolations{violations}, nil
}
return out, nil, nil
}
type DefApprovals struct {

3
types/rule.go
View File

@ -63,6 +63,7 @@ type Violation struct {
// RuleViolations holds several violations of a rule.
type RuleViolations struct {
Rule RuleInfo `json:"rule"`
Bypassed bool `json:"bypassed"`
Violations []Violation `json:"violations"`
}
@ -83,7 +84,7 @@ func (violations *RuleViolations) Addf(code, format string, params ...any) {
}
func (violations *RuleViolations) IsCritical() bool {
return violations.Rule.State == enum.RuleStateActive
return violations.Rule.State == enum.RuleStateActive && !violations.Bypassed && len(violations.Violations) > 0
}
// RuleInfo holds basic info about a rule that is used to describe the rule in RuleViolations.