Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(policy): Support marshalling of config #796

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 20 additions & 20 deletions policy/approval/approve.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,32 +29,32 @@ import (
)

type Rule struct {
Name string `yaml:"name"`
Description string `yaml:"description"`
Predicates predicate.Predicates `yaml:"if"`
Options Options `yaml:"options"`
Requires Requires `yaml:"requires"`
Name string `yaml:"name,omitempty"`
Description string `yaml:"description,omitempty"`
Predicates predicate.Predicates `yaml:"if,omitempty"`
Options Options `yaml:"options,omitempty"`
Requires Requires `yaml:"requires,omitempty"`
}

type Options struct {
AllowAuthor bool `yaml:"allow_author"`
AllowContributor bool `yaml:"allow_contributor"`
AllowNonAuthorContributor bool `yaml:"allow_non_author_contributor"`
InvalidateOnPush bool `yaml:"invalidate_on_push"`
AllowAuthor bool `yaml:"allow_author,omitempty"`
AllowContributor bool `yaml:"allow_contributor,omitempty"`
AllowNonAuthorContributor bool `yaml:"allow_non_author_contributor,omitempty"`
InvalidateOnPush bool `yaml:"invalidate_on_push,omitempty"`

IgnoreEditedComments bool `yaml:"ignore_edited_comments"`
IgnoreUpdateMerges bool `yaml:"ignore_update_merges"`
IgnoreCommitsBy common.Actors `yaml:"ignore_commits_by"`
IgnoreEditedComments bool `yaml:"ignore_edited_comments,omitempty"`
IgnoreUpdateMerges bool `yaml:"ignore_update_merges,omitempty"`
IgnoreCommitsBy common.Actors `yaml:"ignore_commits_by,omitempty"`

RequestReview RequestReview `yaml:"request_review"`
RequestReview RequestReview `yaml:"request_review,omitempty"`

Methods *common.Methods `yaml:"methods"`
Methods *common.Methods `yaml:"methods,omitempty"`
}

type RequestReview struct {
Enabled bool `yaml:"enabled"`
Mode common.RequestMode `yaml:"mode"`
Count int `yaml:"count"`
Enabled bool `yaml:"enabled,omitempty"`
Mode common.RequestMode `yaml:"mode,omitempty"`
Count int `yaml:"count,omitempty"`
}

func (opts *Options) GetMethods() *common.Methods {
Expand All @@ -78,9 +78,9 @@ func (opts *Options) GetMethods() *common.Methods {
}

type Requires struct {
Count int `yaml:"count"`
Count int `yaml:"count,omitempty"`
Actors common.Actors `yaml:",inline"`
Conditions predicate.Predicates `yaml:"conditions"`
Conditions predicate.Predicates `yaml:"conditions,omitempty"`
}

func (r *Rule) Trigger() common.Trigger {
Expand Down Expand Up @@ -411,7 +411,7 @@ func (r *Rule) filteredCommits(ctx context.Context, prctx pull.Context) ([]*pull
commits = sortCommits(commits, prctx.HeadSHA())

ignoreUpdates := r.Options.IgnoreUpdateMerges
ignoreCommits := !r.Options.IgnoreCommitsBy.IsEmpty()
ignoreCommits := !r.Options.IgnoreCommitsBy.IsZero()

if !ignoreUpdates && !ignoreCommits {
return commits, nil
Expand Down
16 changes: 8 additions & 8 deletions policy/common/actor.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,21 +26,21 @@ import (
// team and organization memberships. The set of allowed actors is the union of
// all conditions in this structure.
type Actors struct {
Users []string `yaml:"users" json:"users"`
Teams []string `yaml:"teams" json:"teams"`
Organizations []string `yaml:"organizations" json:"organizations"`
Users []string `yaml:"users,omitempty" json:"users"`
Teams []string `yaml:"teams,omitempty" json:"teams"`
Organizations []string `yaml:"organizations,omitempty" json:"organizations"`

// Deprecated: use Permissions with "admin" or "write"
Admins bool `yaml:"admins" json:"-"`
WriteCollaborators bool `yaml:"write_collaborators" json:"-"`
Admins bool `yaml:"admins,omitempty" json:"-"`
WriteCollaborators bool `yaml:"write_collaborators,omitempty" json:"-"`

// A list of GitHub collaborator permissions that are allowed. Values may
// be any of "admin", "maintain", "write", "triage", and "read".
Permissions []pull.Permission `yaml:"permissions" json:"permissions"`
Permissions []pull.Permission `yaml:"permissions,omitempty" json:"permissions"`
}

// IsEmpty returns true if no conditions for actors are defined.
func (a *Actors) IsEmpty() bool {
// IsZero returns true if no conditions for actors are defined.
func (a *Actors) IsZero() bool {
return a == nil || (len(a.Users) == 0 && len(a.Teams) == 0 && len(a.Organizations) == 0 &&
len(a.Permissions) == 0 && !a.Admins && !a.WriteCollaborators)
}
Expand Down
10 changes: 5 additions & 5 deletions policy/common/actor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -117,17 +117,17 @@ func TestIsActor(t *testing.T) {

func TestIsEmpty(t *testing.T) {
a := &Actors{}
assert.True(t, a.IsEmpty(), "Actors struct was not empty")
assert.True(t, a.IsZero(), "Actors struct was not empty")

a = &Actors{Users: []string{"user"}}
assert.False(t, a.IsEmpty(), "Actors struct was empty")
assert.False(t, a.IsZero(), "Actors struct was empty")

a = &Actors{Teams: []string{"org/team"}}
assert.False(t, a.IsEmpty(), "Actors struct was empty")
assert.False(t, a.IsZero(), "Actors struct was empty")

a = &Actors{Organizations: []string{"org"}}
assert.False(t, a.IsEmpty(), "Actors struct was empty")
assert.False(t, a.IsZero(), "Actors struct was empty")

a = nil
assert.True(t, a.IsEmpty(), "nil struct was not empty")
assert.True(t, a.IsZero(), "nil struct was not empty")
}
8 changes: 8 additions & 0 deletions policy/common/regexp.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,14 @@ type Regexp struct {
r *regexp.Regexp
}

func (r Regexp) MarshalYAML() (interface{}, error) {
return r.String(), nil
}

func (r Regexp) IsZero() bool {
return r.r == nil
}

func NewRegexp(pattern string) (Regexp, error) {
r, err := regexp.Compile(pattern)
if err != nil {
Expand Down
24 changes: 16 additions & 8 deletions policy/disapproval/disapprove.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,18 +27,26 @@ import (
)

type Policy struct {
Predicates predicate.Predicates `yaml:"if"`
Options Options `yaml:"options"`
Requires Requires `yaml:"requires"`
Predicates predicate.Predicates `yaml:"if,omitempty"`
Options Options `yaml:"options,omitempty"`
Requires Requires `yaml:"requires,omitempty"`
}

type Options struct {
Methods Methods `yaml:"methods"`
Methods Methods `yaml:"methods,omitempty"`
}

func (opts Options) IsZero() bool {
return opts.Methods.IsZero()
}

type Methods struct {
Disapprove *common.Methods `yaml:"disapprove"`
Revoke *common.Methods `yaml:"revoke"`
Disapprove *common.Methods `yaml:"disapprove,omitempty"`
Revoke *common.Methods `yaml:"revoke,omitempty"`
}

func (m *Methods) IsZero() bool {
return m.Disapprove == nil && m.Revoke == nil
}

func (opts *Options) GetDisapproveMethods() *common.Methods {
Expand Down Expand Up @@ -82,7 +90,7 @@ type Requires struct {
func (p *Policy) Trigger() common.Trigger {
t := common.TriggerCommit

if !p.Requires.IsEmpty() {
if !p.Requires.IsZero() {
dm := p.Options.GetDisapproveMethods()
rm := p.Options.GetRevokeMethods()

Expand Down Expand Up @@ -136,7 +144,7 @@ func (p *Policy) Evaluate(ctx context.Context, prctx pull.Context) (res common.R
}
}
res.PredicateResults = predicateResults
if p.Requires.IsEmpty() {
if p.Requires.IsZero() {
log.Debug().Msg("no users are allowed to disapprove; skipping")

res.StatusDescription = "No disapproval policy is specified or the policy is empty"
Expand Down
14 changes: 7 additions & 7 deletions policy/policy.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,19 +29,19 @@ import (
// with the default value being the configured default policy file location. The Ref is optional,
// and the default branch of the Remote repository will be used.
type RemoteConfig struct {
Remote string `yaml:"remote"`
Path string `yaml:"path"`
Ref string `yaml:"ref"`
Remote string `yaml:"remote,omitempty"`
Path string `yaml:"path,omitempty"`
Ref string `yaml:"ref,omitempty"`
}

type Config struct {
Policy Policy `yaml:"policy"`
ApprovalRules []*approval.Rule `yaml:"approval_rules"`
Policy Policy `yaml:"policy,omitempty"`
ApprovalRules []*approval.Rule `yaml:"approval_rules,omitempty"`
}

type Policy struct {
Approval approval.Policy `yaml:"approval"`
Disapproval *disapproval.Policy `yaml:"disapproval"`
Approval approval.Policy `yaml:"approval,omitempty"`
Disapproval *disapproval.Policy `yaml:"disapproval,omitempty"`
}

func ParsePolicy(c *Config) (common.Evaluator, error) {
Expand Down
130 changes: 130 additions & 0 deletions policy/policy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,18 @@ package policy
import (
"context"
"errors"
"regexp"
"testing"

"github.com/palantir/policy-bot/policy/approval"
"github.com/palantir/policy-bot/policy/common"
"github.com/palantir/policy-bot/policy/disapproval"
"github.com/palantir/policy-bot/policy/predicate"
"github.com/palantir/policy-bot/pull"
"github.com/palantir/policy-bot/pull/pulltest"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"gopkg.in/yaml.v2"
)

type StaticEvaluator common.Result
Expand Down Expand Up @@ -113,6 +118,131 @@ func TestEvaluator(t *testing.T) {
})
}

func TestConfigMarshalYaml(t *testing.T) {
tests := []struct {
name string
config Config
expected string
}{
{
name: "empty",
config: Config{},
},
{
name: "withDisapproval",
config: Config{
Policy: Policy{
Disapproval: &disapproval.Policy{},
},
},
expected: `policy:
disapproval: {}
`,
},
{
name: "withApprovalRules",
config: Config{
ApprovalRules: []*approval.Rule{
{
Name: "rule1",
},
},
},
expected: `approval_rules:
- name: rule1
`,
},
{
name: "withChangedFiles",
config: Config{
ApprovalRules: []*approval.Rule{
{
Name: "rule1",
Predicates: predicate.Predicates{
ChangedFiles: &predicate.ChangedFiles{
Paths: []common.Regexp{
common.NewCompiledRegexp(regexp.MustCompile(`^\.github/workflows/.*\.yml$`)),
},
},
},
},
},
},
expected: `approval_rules:
- name: rule1
if:
changed_files:
paths:
- ^\.github/workflows/.*\.yml$
`,
},
{
name: "author",
config: Config{
ApprovalRules: []*approval.Rule{
{
Name: "rule1",
Predicates: predicate.Predicates{
HasAuthorIn: &predicate.HasAuthorIn{
Actors: common.Actors{
Users: []string{"author1", "author2"},
},
},
AuthorIsOnlyContributor: new(predicate.AuthorIsOnlyContributor),
},
},
},
},
expected: `approval_rules:
- name: rule1
if:
has_author_in:
users:
- author1
- author2
author_is_only_contributor: false
`,
},
{
name: "modifiedLines",
config: Config{
ApprovalRules: []*approval.Rule{
{
Name: "rule1",
Predicates: predicate.Predicates{
ModifiedLines: &predicate.ModifiedLines{
Additions: predicate.ComparisonExpr{
Op: predicate.OpGreaterThan,
Value: 10,
},
},
},
},
},
},
expected: `approval_rules:
- name: rule1
if:
modified_lines:
additions: '> 10'
`,
},
}

for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
expected := test.expected
if expected == "" {
expected = "{}\n"
}

b, err := yaml.Marshal(test.config)
require.NoError(t, err)
require.Equal(t, expected, string(b))
})
}
}

func castToResult(e common.Evaluator) *common.Result {
return (*common.Result)(e.(*StaticEvaluator))
}
4 changes: 2 additions & 2 deletions policy/predicate/branch.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import (
)

type TargetsBranch struct {
Pattern common.Regexp `yaml:"pattern"`
Pattern common.Regexp `yaml:"pattern,omitempty"`
}

var _ Predicate = &TargetsBranch{}
Expand Down Expand Up @@ -54,7 +54,7 @@ func (pred *TargetsBranch) Trigger() common.Trigger {
}

type FromBranch struct {
Pattern common.Regexp `yaml:"pattern"`
Pattern common.Regexp `yaml:"pattern,omitempty"`
}

var _ Predicate = &FromBranch{}
Expand Down
Loading