Improve compile pipeline (#699)

Refactor
- use constants for strings
- more tests
- move constraint code into own package

Enhance
- all constrains use doublestart (glob pattern matching) now

Co-authored-by: Anbraten <anton@ju60.de>

docs/docs/20-usage/22-conditional-execution.md

@@ -159,7 +159,11 @@ when:
 
 ## `path`
 
-> NOTE: This feature is currently only available for GitHub and Gitea repositories.
+:::info
+This feature is currently only available for GitHub, Gitlab and Gitea.
+Pull requests aren't supported at the moment ([#697](https://github.com/woodpecker-ci/woodpecker/pull/697)).  
+Path conditions are ignored for tag events.
+:::
 
 Execute a step only on a pipeline with certain files being changed:
 

pipeline/frontend/yaml/compiler/compiler.go

@@ -2,6 +2,7 @@ package compiler
 
 import (
 	"fmt"
+	"strings"
 
 	backend "github.com/woodpecker-ci/woodpecker/pipeline/backend/types"
 	"github.com/woodpecker-ci/woodpecker/pipeline/frontend"
@@ -11,6 +12,16 @@ import (
 // TODO(bradrydzewski) compiler should handle user-defined volumes from YAML
 // TODO(bradrydzewski) compiler should handle user-defined networks from YAML
 
+const (
+	windowsPrefix = "windows/"
+
+	defaultCloneImage = "woodpeckerci/plugin-git:latest"
+	defaultCloneName  = "clone"
+
+	networkDriverNAT    = "nat"
+	networkDriverBridge = "bridge"
+)
+
 type Registry struct {
 	Hostname string
 	Username string
@@ -77,15 +88,15 @@ func (c *Compiler) Compile(conf *yaml.Config) *backend.Config {
 	})
 
 	// create a default network
-	if c.metadata.Sys.Arch == "windows/amd64" {
+	if strings.HasPrefix(c.metadata.Sys.Arch, windowsPrefix) {
 		config.Networks = append(config.Networks, &backend.Network{
 			Name:   fmt.Sprintf("%s_default", c.prefix),
-			Driver: "nat",
+			Driver: networkDriverNAT,
 		})
 	} else {
 		config.Networks = append(config.Networks, &backend.Network{
 			Name:   fmt.Sprintf("%s_default", c.prefix),
-			Driver: "bridge",
+			Driver: networkDriverBridge,
 		})
 	}
 
@@ -110,17 +121,17 @@ func (c *Compiler) Compile(conf *yaml.Config) *backend.Config {
 	// add default clone step
 	if !c.local && len(conf.Clone.Containers) == 0 && !conf.SkipClone {
 		container := &yaml.Container{
-			Name:        "clone",
+			Name:        defaultCloneName,
-			Image:       "woodpeckerci/plugin-git:latest",
+			Image:       defaultCloneImage,
 			Settings:    map[string]interface{}{"depth": "0"},
 			Environment: c.cloneEnv,
 		}
 		name := fmt.Sprintf("%s_clone", c.prefix)
-		step := c.createProcess(name, container, "clone")
+		step := c.createProcess(name, container, defaultCloneName)
 
 		stage := new(backend.Stage)
 		stage.Name = name
-		stage.Alias = "clone"
+		stage.Alias = defaultCloneName
 		stage.Steps = append(stage.Steps, step)
 
 		config.Stages = append(config.Stages, stage)
@@ -134,7 +145,7 @@ func (c *Compiler) Compile(conf *yaml.Config) *backend.Config {
 			stage.Alias = container.Name
 
 			name := fmt.Sprintf("%s_clone_%d", c.prefix, i)
-			step := c.createProcess(name, container, "clone")
+			step := c.createProcess(name, container, defaultCloneName)
 			for k, v := range c.cloneEnv {
 				step.Environment[k] = v
 			}

pipeline/frontend/yaml/config.go

@@ -3,6 +3,7 @@ package yaml
 import (
 	"gopkg.in/yaml.v3"
 
+	"github.com/woodpecker-ci/woodpecker/pipeline/frontend/yaml/constraint"
 	"github.com/woodpecker-ci/woodpecker/pipeline/frontend/yaml/types"
 )
 
@@ -11,7 +12,7 @@ type (
 	Config struct {
 		Cache     types.Stringorslice
 		Platform  string
-		Branches  Constraint
+		Branches  constraint.List
 		Workspace Workspace
 		Clone     Containers
 		Pipeline  Containers

pipeline/frontend/yaml/constraint/constraint.go

@@ -1,8 +1,7 @@
-package yaml
+package constraint
 
 import (
 	"fmt"
-	"path/filepath"
 	"strings"
 
 	"github.com/bmatcuk/doublestar/v4"
@@ -15,33 +14,33 @@ import (
 type (
 	// Constraints defines a set of runtime constraints.
 	Constraints struct {
-		Ref         Constraint
+		Ref         List
-		Repo        Constraint
+		Repo        List
-		Instance    Constraint
+		Instance    List
-		Platform    Constraint
+		Platform    List
-		Environment Constraint
+		Environment List
-		Event       Constraint
+		Event       List
-		Branch      Constraint
+		Branch      List
-		Status      Constraint
+		Status      List
-		Matrix      ConstraintMap
+		Matrix      Map
 		Local       types.BoolTrue
-		Path        ConstraintPath
+		Path        Path
 	}
 
-	// Constraint defines a runtime constraint.
+	// List defines a runtime constraint for exclude & include string slices.
-	Constraint struct {
+	List struct {
 		Include []string
 		Exclude []string
 	}
 
-	// ConstraintMap defines a runtime constraint map.
+	// Map defines a runtime constraint for exclude & include map strings.
-	ConstraintMap struct {
+	Map struct {
 		Include map[string]string
 		Exclude map[string]string
 	}
 
-	// ConstraintPath defines a runtime constrain for paths
+	// Path defines a runtime constrain for exclude & include paths.
-	ConstraintPath struct {
+	Path struct {
 		Include       []string
 		Exclude       []string
 		IgnoreMessage string `yaml:"ignore_message,omitempty"`
@@ -51,20 +50,26 @@ type (
 // Match returns true if all constraints match the given input. If a single
 // constraint fails a false value is returned.
 func (c *Constraints) Match(metadata frontend.Metadata) bool {
-	return c.Platform.Match(metadata.Sys.Arch) &&
+	match := c.Platform.Match(metadata.Sys.Arch) &&
 		c.Environment.Match(metadata.Curr.Target) &&
 		c.Event.Match(metadata.Curr.Event) &&
 		c.Branch.Match(metadata.Curr.Commit.Branch) &&
 		c.Repo.Match(metadata.Repo.Name) &&
 		c.Ref.Match(metadata.Curr.Commit.Ref) &&
 		c.Instance.Match(metadata.Sys.Host) &&
-		c.Matrix.Match(metadata.Job.Matrix) &&
+		c.Matrix.Match(metadata.Job.Matrix)
-		c.Path.Match(metadata.Curr.Commit.ChangedFiles, metadata.Curr.Commit.Message)
+
+	// changed files filter do only apply for pull-request and push events
+	if metadata.Curr.Event == frontend.EventPull || metadata.Curr.Event == frontend.EventPush {
+		match = match && c.Path.Match(metadata.Curr.Commit.ChangedFiles, metadata.Curr.Commit.Message)
+	}
+
+	return match
 }
 
 // Match returns true if the string matches the include patterns and does not
 // match any of the exclude patterns.
-func (c *Constraint) Match(v string) bool {
+func (c *List) Match(v string) bool {
 	if c.Excludes(v) {
 		return false
 	}
@@ -78,9 +83,9 @@ func (c *Constraint) Match(v string) bool {
 }
 
 // Includes returns true if the string matches the include patterns.
-func (c *Constraint) Includes(v string) bool {
+func (c *List) Includes(v string) bool {
 	for _, pattern := range c.Include {
-		if ok, _ := filepath.Match(pattern, v); ok {
+		if ok, _ := doublestar.Match(pattern, v); ok {
 			return true
 		}
 	}
@@ -88,9 +93,9 @@ func (c *Constraint) Includes(v string) bool {
 }
 
 // Excludes returns true if the string matches the exclude patterns.
-func (c *Constraint) Excludes(v string) bool {
+func (c *List) Excludes(v string) bool {
 	for _, pattern := range c.Exclude {
-		if ok, _ := filepath.Match(pattern, v); ok {
+		if ok, _ := doublestar.Match(pattern, v); ok {
 			return true
 		}
 	}
@@ -98,7 +103,7 @@ func (c *Constraint) Excludes(v string) bool {
 }
 
 // UnmarshalYAML unmarshals the constraint.
-func (c *Constraint) UnmarshalYAML(value *yaml.Node) error {
+func (c *List) UnmarshalYAML(value *yaml.Node) error {
 	out1 := struct {
 		Include types.Stringorslice
 		Exclude types.Stringorslice
@@ -125,7 +130,7 @@ func (c *Constraint) UnmarshalYAML(value *yaml.Node) error {
 
 // Match returns true if the params matches the include key values and does not
 // match any of the exclude key values.
-func (c *ConstraintMap) Match(params map[string]string) bool {
+func (c *Map) Match(params map[string]string) bool {
 	// when no includes or excludes automatically match
 	if len(c.Include) == 0 && len(c.Exclude) == 0 {
 		return true
@@ -136,7 +141,7 @@ func (c *ConstraintMap) Match(params map[string]string) bool {
 		var matches int
 
 		for key, val := range c.Exclude {
-			if params[key] == val {
+			if ok, _ := doublestar.Match(val, params[key]); ok {
 				matches++
 			}
 		}
@@ -145,7 +150,7 @@ func (c *ConstraintMap) Match(params map[string]string) bool {
 		}
 	}
 	for key, val := range c.Include {
-		if params[key] != val {
+		if ok, _ := doublestar.Match(val, params[key]); !ok {
 			return false
 		}
 	}
@@ -153,7 +158,7 @@ func (c *ConstraintMap) Match(params map[string]string) bool {
 }
 
 // UnmarshalYAML unmarshals the constraint map.
-func (c *ConstraintMap) UnmarshalYAML(unmarshal func(interface{}) error) error {
+func (c *Map) UnmarshalYAML(unmarshal func(interface{}) error) error {
 	out1 := struct {
 		Include map[string]string
 		Exclude map[string]string
@@ -176,7 +181,7 @@ func (c *ConstraintMap) UnmarshalYAML(unmarshal func(interface{}) error) error {
 }
 
 // UnmarshalYAML unmarshals the constraint.
-func (c *ConstraintPath) UnmarshalYAML(value *yaml.Node) error {
+func (c *Path) UnmarshalYAML(value *yaml.Node) error {
 	out1 := struct {
 		Include       types.Stringorslice `yaml:"include,omitempty"`
 		Exclude       types.Stringorslice `yaml:"exclude,omitempty"`
@@ -205,7 +210,7 @@ func (c *ConstraintPath) UnmarshalYAML(value *yaml.Node) error {
 
 // Match returns true if file paths in string slice matches the include and not exclude patterns
 //  or if commit message contains ignore message.
-func (c *ConstraintPath) Match(v []string, message string) bool {
+func (c *Path) Match(v []string, message string) bool {
 	// ignore file pattern matches if the commit message contains a pattern
 	if len(c.IgnoreMessage) > 0 && strings.Contains(strings.ToLower(message), strings.ToLower(c.IgnoreMessage)) {
 		return true
@@ -225,7 +230,7 @@ func (c *ConstraintPath) Match(v []string, message string) bool {
 }
 
 // Includes returns true if the string matches any of the include patterns.
-func (c *ConstraintPath) Includes(v []string) bool {
+func (c *Path) Includes(v []string) bool {
 	for _, pattern := range c.Include {
 		for _, file := range v {
 			if ok, _ := doublestar.Match(pattern, file); ok {
@@ -237,7 +242,7 @@ func (c *ConstraintPath) Includes(v []string) bool {
 }
 
 // Excludes returns true if the string matches any of the exclude patterns.
-func (c *ConstraintPath) Excludes(v []string) bool {
+func (c *Path) Excludes(v []string) bool {
 	for _, pattern := range c.Exclude {
 		for _, file := range v {
 			if ok, _ := doublestar.Match(pattern, file); ok {

pipeline/frontend/yaml/constraint/constraint_test.go

@@ -1,4 +1,4 @@
-package yaml
+package constraint
 
 import (
 	"testing"
@@ -285,10 +285,19 @@ func TestConstraintMap(t *testing.T) {
 			with: map[string]string{"GOLANG": "1.7", "REDIS": "3.0"},
 			want: false,
 		},
-		// TODO(bradrydzewski) eventually we should enable wildcard matching
 		{
 			conf: "{ GOLANG: 1.7, REDIS: 3.* }",
 			with: map[string]string{"GOLANG": "1.7", "REDIS": "3.0"},
+			want: true,
+		},
+		{
+			conf: "{ GOLANG: 1.7, BRANCH: release/**/test }",
+			with: map[string]string{"GOLANG": "1.7", "BRANCH": "release/v1.12.1//test"},
+			want: true,
+		},
+		{
+			conf: "{ GOLANG: 1.7, BRANCH: release/**/test }",
+			with: map[string]string{"GOLANG": "1.7", "BRANCH": "release/v1.12.1/qest"},
 			want: false,
 		},
 		// include syntax
@@ -368,10 +377,7 @@ func TestConstraintMap(t *testing.T) {
 	}
 	for _, test := range testdata {
 		c := parseConstraintMap(t, test.conf)
-		got, want := c.Match(test.with), test.want
+		assert.Equal(t, test.want, c.Match(test.with), "config: '%s', with: '%s'", test.conf, test.with)
-		if got != want {
-			t.Errorf("Expect %q matches %q is %v", test.with, test.conf, want)
-		}
 	}
 }
 
@@ -399,16 +405,16 @@ func TestConstraints(t *testing.T) {
 			want: true,
 		},
 		// environment constraint
-		// {
+		{
-		// 	conf: "{ branch: develop }",
+			conf: "{ branch: develop }",
-		// 	with: frontend.Metadata{Curr: frontend.Build{Commit: frontend.Commit{Branch: "master"}}},
+			with: frontend.Metadata{Curr: frontend.Build{Commit: frontend.Commit{Branch: "master"}}},
-		// 	want: false,
+			want: false,
-		// },
+		},
-		// {
+		{
-		// 	conf: "{ branch: master }",
+			conf: "{ branch: master }",
-		// 	with: frontend.Metadata{Curr: frontend.Build{Commit: frontend.Commit{Branch: "master"}}},
+			with: frontend.Metadata{Curr: frontend.Build{Commit: frontend.Commit{Branch: "master"}}},
-		// 	want: true,
+			want: true,
-		// },
+		},
 		// repo constraint
 		{
 			conf: "{ repo: owner/* }",
@@ -469,20 +475,20 @@ func parseConstraints(t *testing.T, s string) *Constraints {
 	return c
 }
 
-func parseConstraint(t *testing.T, s string) *Constraint {
+func parseConstraint(t *testing.T, s string) *List {
-	c := &Constraint{}
+	c := &List{}
 	assert.NoError(t, yaml.Unmarshal([]byte(s), c))
 	return c
 }
 
-func parseConstraintMap(t *testing.T, s string) *ConstraintMap {
+func parseConstraintMap(t *testing.T, s string) *Map {
-	c := &ConstraintMap{}
+	c := &Map{}
 	assert.NoError(t, yaml.Unmarshal([]byte(s), c))
 	return c
 }
 
-func parseConstraintPath(t *testing.T, s string) *ConstraintPath {
+func parseConstraintPath(t *testing.T, s string) *Path {
-	c := &ConstraintPath{}
+	c := &Path{}
 	assert.NoError(t, yaml.Unmarshal([]byte(s), c))
 	return c
 }

pipeline/frontend/yaml/container.go

@@ -5,6 +5,7 @@ import (
 
 	"gopkg.in/yaml.v3"
 
+	"github.com/woodpecker-ci/woodpecker/pipeline/frontend/yaml/constraint"
 	"github.com/woodpecker-ci/woodpecker/pipeline/frontend/yaml/types"
 )
 
@@ -57,7 +58,7 @@ type (
 		Volumes       types.Volumes          `yaml:"volumes,omitempty"`
 		Secrets       Secrets                `yaml:"secrets,omitempty"`
 		Sysctls       types.SliceorMap       `yaml:"sysctls,omitempty"`
-		Constraints   Constraints            `yaml:"when,omitempty"`
+		Constraints   constraint.Constraints `yaml:"when,omitempty"`
 		Settings      map[string]interface{} `yaml:"settings"`
 		// Deprecated
 		Vargs map[string]interface{} `yaml:",inline"` // TODO: remove deprecated with v0.16.0

pipeline/frontend/yaml/container_test.go

@@ -6,6 +6,7 @@ import (
 	"github.com/stretchr/testify/assert"
 	"gopkg.in/yaml.v3"
 
+	"github.com/woodpecker-ci/woodpecker/pipeline/frontend/yaml/constraint"
 	"github.com/woodpecker-ci/woodpecker/pipeline/frontend/yaml/types"
 )
 
@@ -109,8 +110,8 @@ func TestUnmarshalContainer(t *testing.T) {
 				{Source: "/etc/configs", Destination: "/etc/configs/", AccessMode: "ro"},
 			},
 		},
-		Constraints: Constraints{
+		Constraints: constraint.Constraints{
-			Branch: Constraint{
+			Branch: constraint.List{
 				Include: []string{"master"},
 			},
 		},
@@ -188,9 +189,9 @@ func TestUnmarshalContainers(t *testing.T) {
 						"tag":        stringsToInterface("next", "latest"),
 						"dry_run":    true,
 					},
-					Constraints: Constraints{
+					Constraints: constraint.Constraints{
-						Event:  Constraint{Include: []string{"push"}},
+						Event:  constraint.List{Include: []string{"push"}},
-						Branch: Constraint{Include: []string{"${CI_REPO_DEFAULT_BRANCH}"}},
+						Branch: constraint.List{Include: []string{"${CI_REPO_DEFAULT_BRANCH}"}},
 					},
 				},
 			},
@@ -216,9 +217,9 @@ func TestUnmarshalContainers(t *testing.T) {
 						"dockerfile": "docker/Dockerfile.cli",
 						"tag":        stringsToInterface("next"),
 					},
-					Constraints: Constraints{
+					Constraints: constraint.Constraints{
-						Event:  Constraint{Include: []string{"push"}},
+						Event:  constraint.List{Include: []string{"push"}},
-						Branch: Constraint{Include: []string{"${CI_REPO_DEFAULT_BRANCH}"}},
+						Branch: constraint.List{Include: []string{"${CI_REPO_DEFAULT_BRANCH}"}},
 					},
 				},
 			},