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

refactor(ruleset)!: improve match code efficiency and properly handle status #1255

Open
wants to merge 5 commits into
base: main
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
5 changes: 3 additions & 2 deletions compiler/native/compile.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ func (c *client) Compile(ctx context.Context, v interface{}) (*pipeline.Build, *
Tag: strings.TrimPrefix(c.build.GetRef(), "refs/tags/"),
Target: c.build.GetDeploy(),
Label: c.labels,
Status: c.build.GetStatus(),
}

// add instance when we have the metadata (local exec will not)
Expand Down Expand Up @@ -178,7 +179,7 @@ func (c *client) CompileLite(ctx context.Context, v interface{}, ruleData *pipel

for _, s := range stg.Steps {
cRuleset := s.Ruleset.ToPipeline()
if match, err := cRuleset.Match(ruleData); err == nil && match {
if match, err := ruleData.Match(*cRuleset); err == nil && match {
*purgedSteps = append(*purgedSteps, s)
}
}
Expand Down Expand Up @@ -215,7 +216,7 @@ func (c *client) CompileLite(ctx context.Context, v interface{}, ruleData *pipel

for _, s := range p.Steps {
cRuleset := s.Ruleset.ToPipeline()
if match, err := cRuleset.Match(ruleData); err == nil && match {
if match, err := ruleData.Match(*cRuleset); err == nil && match {
*purgedSteps = append(*purgedSteps, s)
}
}
Expand Down
138 changes: 69 additions & 69 deletions compiler/native/compile_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2102,10 +2102,10 @@ func TestNative_Compile_LegacyMergeAnchor(t *testing.T) {
Pull: "not_present",
Ruleset: pipeline.Ruleset{
If: pipeline.Rules{
Event: []string{"push"},
Event: []string{"push"},
Matcher: "filepath",
Operator: "and",
},
Matcher: "filepath",
Operator: "and",
},
},
&pipeline.Container{
Expand All @@ -2120,10 +2120,10 @@ func TestNative_Compile_LegacyMergeAnchor(t *testing.T) {
Pull: "not_present",
Ruleset: pipeline.Ruleset{
If: pipeline.Rules{
Event: []string{"push"},
Event: []string{"push"},
Matcher: "filepath",
Operator: "and",
},
Matcher: "filepath",
Operator: "and",
},
},
&pipeline.Container{
Expand All @@ -2138,10 +2138,10 @@ func TestNative_Compile_LegacyMergeAnchor(t *testing.T) {
Pull: "not_present",
Ruleset: pipeline.Ruleset{
If: pipeline.Rules{
Event: []string{"push"},
Event: []string{"push"},
Matcher: "filepath",
Operator: "and",
},
Matcher: "filepath",
Operator: "and",
},
},
},
Expand Down Expand Up @@ -3394,11 +3394,11 @@ func Test_CompileLite(t *testing.T) {
Pull: "not_present",
Ruleset: yaml.Ruleset{
If: yaml.Rules{
Event: []string{"push"},
Branch: []string{"main"},
Event: []string{"push"},
Branch: []string{"main"},
Matcher: "filepath",
Operator: "and",
},
Matcher: "filepath",
Operator: "and",
},
},
},
Expand All @@ -3414,11 +3414,11 @@ func Test_CompileLite(t *testing.T) {
Pull: "not_present",
Ruleset: yaml.Ruleset{
If: yaml.Rules{
Event: []string{"tag"},
Tag: []string{"v*"},
Event: []string{"tag"},
Tag: []string{"v*"},
Matcher: "filepath",
Operator: "and",
},
Matcher: "filepath",
Operator: "and",
},
},
},
Expand All @@ -3434,11 +3434,11 @@ func Test_CompileLite(t *testing.T) {
Pull: "not_present",
Ruleset: yaml.Ruleset{
If: yaml.Rules{
Event: []string{"tag"},
Tag: []string{"v*"},
Event: []string{"tag"},
Tag: []string{"v*"},
Matcher: "filepath",
Operator: "and",
},
Matcher: "filepath",
Operator: "and",
},
},
},
Expand All @@ -3454,11 +3454,11 @@ func Test_CompileLite(t *testing.T) {
Pull: "not_present",
Ruleset: yaml.Ruleset{
If: yaml.Rules{
Event: []string{"tag"},
Tag: []string{"v*"},
Event: []string{"tag"},
Tag: []string{"v*"},
Matcher: "filepath",
Operator: "and",
},
Matcher: "filepath",
Operator: "and",
},
},
},
Expand Down Expand Up @@ -3535,11 +3535,11 @@ func Test_CompileLite(t *testing.T) {
Pull: "not_present",
Ruleset: yaml.Ruleset{
If: yaml.Rules{
Event: []string{"push"},
Branch: []string{"main"},
Event: []string{"push"},
Branch: []string{"main"},
Matcher: "filepath",
Operator: "and",
},
Matcher: "filepath",
Operator: "and",
},
},
},
Expand All @@ -3555,11 +3555,11 @@ func Test_CompileLite(t *testing.T) {
Pull: "not_present",
Ruleset: yaml.Ruleset{
If: yaml.Rules{
Event: []string{"tag"},
Tag: []string{"v*"},
Event: []string{"tag"},
Tag: []string{"v*"},
Matcher: "filepath",
Operator: "and",
},
Matcher: "filepath",
Operator: "and",
},
},
},
Expand All @@ -3575,11 +3575,11 @@ func Test_CompileLite(t *testing.T) {
Pull: "not_present",
Ruleset: yaml.Ruleset{
If: yaml.Rules{
Event: []string{"tag"},
Tag: []string{"v*"},
Event: []string{"tag"},
Tag: []string{"v*"},
Matcher: "filepath",
Operator: "and",
},
Matcher: "filepath",
Operator: "and",
},
},
},
Expand All @@ -3595,11 +3595,11 @@ func Test_CompileLite(t *testing.T) {
Pull: "not_present",
Ruleset: yaml.Ruleset{
If: yaml.Rules{
Event: []string{"tag"},
Tag: []string{"v*"},
Event: []string{"tag"},
Tag: []string{"v*"},
Matcher: "filepath",
Operator: "and",
},
Matcher: "filepath",
Operator: "and",
},
},
},
Expand Down Expand Up @@ -3663,11 +3663,11 @@ func Test_CompileLite(t *testing.T) {
Pull: "not_present",
Ruleset: yaml.Ruleset{
If: yaml.Rules{
Event: []string{"push"},
Branch: []string{"main"},
Event: []string{"push"},
Branch: []string{"main"},
Matcher: "filepath",
Operator: "and",
},
Matcher: "filepath",
Operator: "and",
},
},
},
Expand Down Expand Up @@ -3726,11 +3726,11 @@ func Test_CompileLite(t *testing.T) {
Pull: "not_present",
Ruleset: yaml.Ruleset{
If: yaml.Rules{
Event: []string{"deployment:created"},
Target: []string{"production"},
Event: []string{"deployment:created"},
Target: []string{"production"},
Matcher: "filepath",
Operator: "and",
},
Matcher: "filepath",
Operator: "and",
},
},
{
Expand All @@ -3740,11 +3740,11 @@ func Test_CompileLite(t *testing.T) {
Pull: "not_present",
Ruleset: yaml.Ruleset{
If: yaml.Rules{
Path: []string{"src/*", "test/*"},
Event: []string{},
Path: []string{"src/*", "test/*"},
Event: []string{},
Matcher: "filepath",
Operator: "and",
},
Matcher: "filepath",
Operator: "and",
},
},
{
Expand Down Expand Up @@ -3830,11 +3830,11 @@ func Test_CompileLite(t *testing.T) {
Pull: "not_present",
Ruleset: yaml.Ruleset{
If: yaml.Rules{
Event: []string{"deployment:created"},
Target: []string{"production"},
Event: []string{"deployment:created"},
Target: []string{"production"},
Matcher: "filepath",
Operator: "and",
},
Matcher: "filepath",
Operator: "and",
},
},
{
Expand Down Expand Up @@ -4156,11 +4156,11 @@ func Test_CompileLite(t *testing.T) {
Pull: "not_present",
Ruleset: yaml.Ruleset{
If: yaml.Rules{
Event: []string{"tag"},
Tag: []string{"v*"},
Event: []string{"tag"},
Tag: []string{"v*"},
Matcher: "filepath",
Operator: "and",
},
Matcher: "filepath",
Operator: "and",
},
},
},
Expand All @@ -4176,11 +4176,11 @@ func Test_CompileLite(t *testing.T) {
Pull: "not_present",
Ruleset: yaml.Ruleset{
If: yaml.Rules{
Event: []string{"tag"},
Tag: []string{"v*"},
Event: []string{"tag"},
Tag: []string{"v*"},
Matcher: "filepath",
Operator: "and",
},
Matcher: "filepath",
Operator: "and",
},
},
},
Expand All @@ -4196,11 +4196,11 @@ func Test_CompileLite(t *testing.T) {
Pull: "not_present",
Ruleset: yaml.Ruleset{
If: yaml.Rules{
Event: []string{"tag"},
Tag: []string{"v*"},
Event: []string{"tag"},
Tag: []string{"v*"},
Matcher: "filepath",
Operator: "and",
},
Matcher: "filepath",
Operator: "and",
},
},
},
Expand Down
2 changes: 1 addition & 1 deletion compiler/native/native.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,8 @@

// FromCLIContext returns a Pipeline implementation that integrates with the supported registries.
//
//nolint:revive // ignore returning unexported client

func FromCLIContext(ctx *cli.Context) (*client, error) {

Check failure on line 59 in compiler/native/native.go

View workflow job for this annotation

GitHub Actions / golangci

[golangci] compiler/native/native.go#L59

unexported-return: exported func FromCLIContext returns unexported type *native.client, which can be annoying to use (revive)
Raw output
compiler/native/native.go:59:40: unexported-return: exported func FromCLIContext returns unexported type *native.client, which can be annoying to use (revive)
func FromCLIContext(ctx *cli.Context) (*client, error) {
                                       ^

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚫 [golangci] reported by reviewdog 🐶
unexported-return: exported func FromCLIContext returns unexported type *native.client, which can be annoying to use (revive)

logrus.Debug("creating registry clients from CLI configuration")

c := new(client)
Expand Down
2 changes: 1 addition & 1 deletion compiler/registry/github/github.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,8 @@
// New returns a Registry implementation that integrates
// with GitHub or a GitHub Enterprise instance.
//
//nolint:revive // ignore returning unexported client

func New(ctx context.Context, address, token string) (*client, error) {

Check failure on line 34 in compiler/registry/github/github.go

View workflow job for this annotation

GitHub Actions / golangci

[golangci] compiler/registry/github/github.go#L34

unexported-return: exported func New returns unexported type *github.client, which can be annoying to use (revive)
Raw output
compiler/registry/github/github.go:34:55: unexported-return: exported func New returns unexported type *github.client, which can be annoying to use (revive)
func New(ctx context.Context, address, token string) (*client, error) {
                                                      ^

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚫 [golangci] reported by reviewdog 🐶
unexported-return: exported func New returns unexported type *github.client, which can be annoying to use (revive)

// create the client object
c := &client{
URL: defaultURL,
Expand Down
2 changes: 1 addition & 1 deletion compiler/types/pipeline/build_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ func TestPipeline_Build_Purge(t *testing.T) {
Number: 2,
Pull: "always",
Ruleset: Ruleset{
If: Rules{Event: []string{"push"}, Branch: []string{"*-dev"}},
If: Rules{Event: []string{"push"}, Branch: []string{"*-dev"}, Matcher: "regexp"},
Operator: "and",
Matcher: "regexp",
},
Expand Down
Loading
Loading