-
Notifications
You must be signed in to change notification settings - Fork 360
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Hooks separate actions from entry catalog (#1443)
- Loading branch information
Showing
16 changed files
with
582 additions
and
95 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,137 @@ | ||
package actions | ||
|
||
import ( | ||
"errors" | ||
"fmt" | ||
"path" | ||
"regexp" | ||
|
||
"gopkg.in/yaml.v3" | ||
) | ||
|
||
type Action struct { | ||
Name string `yaml:"name"` | ||
Description string `yaml:"description"` | ||
On OnEvents `yaml:"on"` | ||
Hooks []ActionHook `yaml:"hooks"` | ||
} | ||
|
||
type OnEvents struct { | ||
PreMerge *ActionOn `yaml:"pre-merge"` | ||
PreCommit *ActionOn `yaml:"pre-commit"` | ||
} | ||
|
||
type ActionOn struct { | ||
Branches []string `yaml:"branches"` | ||
} | ||
|
||
type ActionHook struct { | ||
ID string `yaml:"id"` | ||
Type string `yaml:"type"` | ||
Description string `yaml:"description"` | ||
Properties map[string]string `yaml:"properties"` | ||
} | ||
|
||
type MatchSpec struct { | ||
EventType EventType | ||
Branch string | ||
} | ||
|
||
var ( | ||
reHookID = regexp.MustCompile(`^[_a-zA-Z][\-_a-zA-Z0-9]{1,255}$`) | ||
|
||
ErrInvalidAction = errors.New("invalid action") | ||
ErrInvalidEventType = errors.New("invalid event type") | ||
) | ||
|
||
func (a *Action) Validate() error { | ||
if a.Name == "" { | ||
return fmt.Errorf("%w 'name' is required", ErrInvalidAction) | ||
} | ||
if a.On.PreMerge == nil && a.On.PreCommit == nil { | ||
return fmt.Errorf("%w 'on' is required", ErrInvalidAction) | ||
} | ||
ids := make(map[string]struct{}) | ||
for i, hook := range a.Hooks { | ||
if !reHookID.MatchString(hook.ID) { | ||
return fmt.Errorf("hook[%d] missing ID: %w", i, ErrInvalidAction) | ||
} | ||
if _, found := ids[hook.ID]; found { | ||
return fmt.Errorf("hook[%d] duplicate ID '%s': %w", i, hook.ID, ErrInvalidAction) | ||
} | ||
ids[hook.ID] = struct{}{} | ||
if _, found := hooks[HookType(hook.Type)]; !found { | ||
return fmt.Errorf("hook[%d] type '%s' unknown: %w", i, hook.ID, ErrInvalidAction) | ||
} | ||
} | ||
return nil | ||
} | ||
|
||
func (a *Action) Match(spec MatchSpec) (bool, error) { | ||
// at least one matched event definition | ||
var actionOn *ActionOn | ||
switch spec.EventType { | ||
case EventTypePreCommit: | ||
actionOn = a.On.PreCommit | ||
case EventTypePreMerge: | ||
actionOn = a.On.PreMerge | ||
default: | ||
return false, ErrInvalidEventType | ||
} | ||
// if no action specified - no match | ||
if actionOn == nil { | ||
return false, nil | ||
} | ||
// if no branches spec found - all match | ||
if len(actionOn.Branches) == 0 { | ||
return true, nil | ||
} | ||
// find at least one match | ||
for _, b := range actionOn.Branches { | ||
matched, err := path.Match(b, spec.Branch) | ||
if err != nil { | ||
return false, err | ||
} | ||
if matched { | ||
return true, nil | ||
} | ||
} | ||
return false, nil | ||
} | ||
|
||
// ParseAction helper function to read, parse and validate Action from a reader | ||
func ParseAction(data []byte) (*Action, error) { | ||
var act Action | ||
err := yaml.Unmarshal(data, &act) | ||
if err != nil { | ||
return nil, err | ||
} | ||
err = act.Validate() | ||
if err != nil { | ||
return nil, err | ||
} | ||
return &act, nil | ||
} | ||
|
||
type Source interface { | ||
List() []string | ||
Load(name string) ([]byte, error) | ||
} | ||
|
||
func LoadActions(source Source) ([]*Action, error) { | ||
return nil, nil | ||
} | ||
|
||
func MatchActions(actions []*Action, spec MatchSpec) ([]*Action, error) { | ||
var matched []*Action | ||
for _, act := range actions { | ||
m, err := act.Match(spec) | ||
if err != nil { | ||
return nil, err | ||
} | ||
if m { | ||
matched = append(matched, act) | ||
} | ||
} | ||
return matched, nil | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,229 @@ | ||
package actions_test | ||
|
||
import ( | ||
"io/ioutil" | ||
"path" | ||
"testing" | ||
|
||
"github.com/go-test/deep" | ||
"github.com/treeverse/lakefs/actions" | ||
) | ||
|
||
func TestAction_ReadAction(t *testing.T) { | ||
tests := []struct { | ||
name string | ||
filename string | ||
wantErr bool | ||
}{ | ||
{name: "full", filename: "action_full.yaml", wantErr: false}, | ||
{name: "required", filename: "action_required.yaml", wantErr: false}, | ||
{name: "duplicate id", filename: "action_duplicate_id.yaml", wantErr: true}, | ||
{name: "invalid id", filename: "action_invalid_id.yaml", wantErr: true}, | ||
{name: "invalid hook type", filename: "action_invalid_type.yaml", wantErr: true}, | ||
{name: "invalid yaml", filename: "action_invalid_yaml.yaml", wantErr: true}, | ||
} | ||
for _, tt := range tests { | ||
t.Run(tt.name, func(t *testing.T) { | ||
data, err := ioutil.ReadFile(path.Join("testdata", tt.filename)) | ||
if err != nil { | ||
t.Fatalf("Failed to load testdata %s, err=%s", tt.filename, err) | ||
} | ||
act, err := actions.ParseAction(data) | ||
if (err != nil) != tt.wantErr { | ||
t.Errorf("ParseAction() error = %v, wantErr %t", err, tt.wantErr) | ||
} | ||
if err == nil && act == nil { | ||
t.Error("ParseAction() no error, missing Action") | ||
} | ||
}) | ||
} | ||
} | ||
|
||
func TestAction_Match(t *testing.T) { | ||
tests := []struct { | ||
name string | ||
on actions.OnEvents | ||
spec actions.MatchSpec | ||
want bool | ||
wantErr bool | ||
}{ | ||
{ | ||
name: "none - on pre-merge without branch", | ||
on: actions.OnEvents{}, | ||
spec: actions.MatchSpec{EventType: actions.EventTypePreMerge}, | ||
want: false, | ||
wantErr: false, | ||
}, | ||
{ | ||
name: "none - on invalid event type", | ||
on: actions.OnEvents{}, | ||
spec: actions.MatchSpec{EventType: "nothing"}, | ||
want: false, | ||
wantErr: true, | ||
}, | ||
{ | ||
name: "pre-merge - on pre-merge without branch", | ||
on: actions.OnEvents{PreMerge: &actions.ActionOn{}}, | ||
spec: actions.MatchSpec{EventType: actions.EventTypePreMerge}, | ||
want: true, | ||
wantErr: false, | ||
}, | ||
{ | ||
name: "pre-merge - on pre-commit without branch", | ||
on: actions.OnEvents{PreMerge: &actions.ActionOn{}}, | ||
spec: actions.MatchSpec{EventType: actions.EventTypePreCommit}, | ||
want: false, | ||
wantErr: false, | ||
}, | ||
{ | ||
name: "pre-commit - on pre-merge without branch", | ||
on: actions.OnEvents{PreCommit: &actions.ActionOn{}}, | ||
spec: actions.MatchSpec{EventType: actions.EventTypePreMerge}, | ||
want: false, | ||
wantErr: false, | ||
}, | ||
{ | ||
name: "pre-commit - on pre-commit without branch", | ||
on: actions.OnEvents{PreCommit: &actions.ActionOn{}}, | ||
spec: actions.MatchSpec{EventType: actions.EventTypePreCommit}, | ||
want: true, | ||
wantErr: false, | ||
}, | ||
{ | ||
name: "both - on pre-commit without branch", | ||
on: actions.OnEvents{PreCommit: &actions.ActionOn{}, PreMerge: &actions.ActionOn{}}, | ||
spec: actions.MatchSpec{EventType: actions.EventTypePreCommit}, | ||
want: true, | ||
wantErr: false, | ||
}, | ||
{ | ||
name: "both - on pre-merge without branch", | ||
on: actions.OnEvents{PreCommit: &actions.ActionOn{}, PreMerge: &actions.ActionOn{}}, | ||
spec: actions.MatchSpec{EventType: actions.EventTypePreMerge}, | ||
want: true, | ||
wantErr: false, | ||
}, | ||
{ | ||
name: "pre-commit master - on pre-commit master", | ||
on: actions.OnEvents{PreCommit: &actions.ActionOn{Branches: []string{"master"}}}, | ||
spec: actions.MatchSpec{EventType: actions.EventTypePreCommit, Branch: "master"}, | ||
want: true, | ||
wantErr: false, | ||
}, | ||
{ | ||
name: "pre-commit master - on pre-commit masterer", | ||
on: actions.OnEvents{PreCommit: &actions.ActionOn{Branches: []string{"master"}}}, | ||
spec: actions.MatchSpec{EventType: actions.EventTypePreCommit, Branch: "masterer"}, | ||
want: false, | ||
wantErr: false, | ||
}, | ||
{ | ||
name: "pre-commit ends with feature - on pre-commit new-feature", | ||
on: actions.OnEvents{PreCommit: &actions.ActionOn{Branches: []string{"*-feature"}}}, | ||
spec: actions.MatchSpec{EventType: actions.EventTypePreCommit, Branch: "new-feature"}, | ||
want: true, | ||
wantErr: false, | ||
}, | ||
{ | ||
name: "pre-commit branch a1 or b1 - on pre-commit b1", | ||
on: actions.OnEvents{PreCommit: &actions.ActionOn{Branches: []string{"a1", "b1"}}}, | ||
spec: actions.MatchSpec{EventType: actions.EventTypePreCommit, Branch: "b1"}, | ||
want: true, | ||
wantErr: false, | ||
}, | ||
{ | ||
name: "pre-commit branch invalid - on pre-commit master", | ||
on: actions.OnEvents{PreCommit: &actions.ActionOn{Branches: []string{"\\"}}}, | ||
spec: actions.MatchSpec{EventType: actions.EventTypePreCommit, Branch: "master"}, | ||
want: false, | ||
wantErr: true, | ||
}, | ||
} | ||
for _, tt := range tests { | ||
t.Run(tt.name, func(t *testing.T) { | ||
a := &actions.Action{ | ||
Name: tt.name, | ||
On: tt.on, | ||
} | ||
got, err := a.Match(tt.spec) | ||
if (err != nil) != tt.wantErr { | ||
t.Errorf("Match() error = %v, wantErr %v", err, tt.wantErr) | ||
return | ||
} | ||
if got != tt.want { | ||
t.Errorf("Match() got = %v, want %v", got, tt.want) | ||
} | ||
}) | ||
} | ||
} | ||
|
||
func TestMatchActions(t *testing.T) { | ||
tests := []struct { | ||
name string | ||
actions []*actions.Action | ||
spec actions.MatchSpec | ||
want []*actions.Action | ||
wantErr bool | ||
}{ | ||
{ | ||
name: "empty", | ||
actions: nil, | ||
spec: actions.MatchSpec{}, | ||
want: nil, | ||
wantErr: false, | ||
}, | ||
{ | ||
name: "all", | ||
actions: []*actions.Action{ | ||
{Name: "act1", On: actions.OnEvents{PreCommit: &actions.ActionOn{}}}, | ||
{Name: "act2", On: actions.OnEvents{PreCommit: &actions.ActionOn{}}}, | ||
}, | ||
spec: actions.MatchSpec{ | ||
EventType: actions.EventTypePreCommit, | ||
}, | ||
want: []*actions.Action{ | ||
{Name: "act1", On: actions.OnEvents{PreCommit: &actions.ActionOn{}}}, | ||
{Name: "act2", On: actions.OnEvents{PreCommit: &actions.ActionOn{}}}, | ||
}, | ||
wantErr: false, | ||
}, | ||
{ | ||
name: "none", | ||
actions: []*actions.Action{ | ||
{Name: "act1", On: actions.OnEvents{PreCommit: &actions.ActionOn{}}}, | ||
{Name: "act2", On: actions.OnEvents{PreCommit: &actions.ActionOn{}}}, | ||
}, | ||
spec: actions.MatchSpec{ | ||
EventType: actions.EventTypePreMerge, | ||
}, | ||
want: nil, | ||
wantErr: false, | ||
}, | ||
{ | ||
name: "one", | ||
actions: []*actions.Action{ | ||
{Name: "act1", On: actions.OnEvents{PreCommit: &actions.ActionOn{}}}, | ||
{Name: "act2", On: actions.OnEvents{PreMerge: &actions.ActionOn{}}}, | ||
}, | ||
spec: actions.MatchSpec{ | ||
EventType: actions.EventTypePreMerge, | ||
}, | ||
want: []*actions.Action{ | ||
{Name: "act2", On: actions.OnEvents{PreMerge: &actions.ActionOn{}}}, | ||
}, | ||
wantErr: false, | ||
}, | ||
} | ||
for _, tt := range tests { | ||
t.Run(tt.name, func(t *testing.T) { | ||
got, err := actions.MatchActions(tt.actions, tt.spec) | ||
if (err != nil) != tt.wantErr { | ||
t.Errorf("MatchActions() error = %v, wantErr %v", err, tt.wantErr) | ||
return | ||
} | ||
if diff := deep.Equal(got, tt.want); diff != nil { | ||
t.Error("MatchActions() found diff", diff) | ||
} | ||
}) | ||
} | ||
} |
Oops, something went wrong.