From 04eb7935dbe5a5289f1e953d78d677b766cb4445 Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Mon, 17 Jan 2022 14:43:30 +0100 Subject: [PATCH] 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 --- .../docs/20-usage/22-conditional-execution.md | 6 +- pipeline/frontend/yaml/compiler/compiler.go | 27 +++++-- pipeline/frontend/yaml/config.go | 3 +- .../yaml/{ => constraint}/constraint.go | 75 ++++++++++--------- .../yaml/{ => constraint}/constraint_test.go | 50 +++++++------ pipeline/frontend/yaml/container.go | 3 +- pipeline/frontend/yaml/container_test.go | 17 +++-- 7 files changed, 105 insertions(+), 76 deletions(-) rename pipeline/frontend/yaml/{ => constraint}/constraint.go (75%) rename pipeline/frontend/yaml/{ => constraint}/constraint_test.go (91%) diff --git a/docs/docs/20-usage/22-conditional-execution.md b/docs/docs/20-usage/22-conditional-execution.md index bc73a8b84..76dbe9dc4 100644 --- a/docs/docs/20-usage/22-conditional-execution.md +++ b/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: diff --git a/pipeline/frontend/yaml/compiler/compiler.go b/pipeline/frontend/yaml/compiler/compiler.go index 807cbbd37..cbff32ec7 100644 --- a/pipeline/frontend/yaml/compiler/compiler.go +++ b/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", - Image: "woodpeckerci/plugin-git:latest", + Name: defaultCloneName, + 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 } diff --git a/pipeline/frontend/yaml/config.go b/pipeline/frontend/yaml/config.go index 27c4517a7..9296828da 100644 --- a/pipeline/frontend/yaml/config.go +++ b/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 diff --git a/pipeline/frontend/yaml/constraint.go b/pipeline/frontend/yaml/constraint/constraint.go similarity index 75% rename from pipeline/frontend/yaml/constraint.go rename to pipeline/frontend/yaml/constraint/constraint.go index d0b5aa636..64305d926 100644 --- a/pipeline/frontend/yaml/constraint.go +++ b/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 - Repo Constraint - Instance Constraint - Platform Constraint - Environment Constraint - Event Constraint - Branch Constraint - Status Constraint - Matrix ConstraintMap + Ref List + Repo List + Instance List + Platform List + Environment List + Event List + Branch List + Status List + Matrix Map Local types.BoolTrue - Path ConstraintPath + Path Path } - // Constraint defines a runtime constraint. - Constraint struct { + // List defines a runtime constraint for exclude & include string slices. + List struct { Include []string Exclude []string } - // ConstraintMap defines a runtime constraint map. - ConstraintMap struct { + // Map defines a runtime constraint for exclude & include map strings. + Map struct { Include map[string]string Exclude map[string]string } - // ConstraintPath defines a runtime constrain for paths - ConstraintPath struct { + // Path defines a runtime constrain for exclude & include paths. + 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.Path.Match(metadata.Curr.Commit.ChangedFiles, metadata.Curr.Commit.Message) + c.Matrix.Match(metadata.Job.Matrix) + + // 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 { diff --git a/pipeline/frontend/yaml/constraint_test.go b/pipeline/frontend/yaml/constraint/constraint_test.go similarity index 91% rename from pipeline/frontend/yaml/constraint_test.go rename to pipeline/frontend/yaml/constraint/constraint_test.go index 40373c42f..5eb9e4658 100644 --- a/pipeline/frontend/yaml/constraint_test.go +++ b/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 - if got != want { - t.Errorf("Expect %q matches %q is %v", test.with, test.conf, want) - } + assert.Equal(t, test.want, c.Match(test.with), "config: '%s', with: '%s'", test.conf, test.with) } } @@ -399,16 +405,16 @@ func TestConstraints(t *testing.T) { want: true, }, // environment constraint - // { - // conf: "{ branch: develop }", - // with: frontend.Metadata{Curr: frontend.Build{Commit: frontend.Commit{Branch: "master"}}}, - // want: false, - // }, - // { - // conf: "{ branch: master }", - // with: frontend.Metadata{Curr: frontend.Build{Commit: frontend.Commit{Branch: "master"}}}, - // want: true, - // }, + { + conf: "{ branch: develop }", + with: frontend.Metadata{Curr: frontend.Build{Commit: frontend.Commit{Branch: "master"}}}, + want: false, + }, + { + conf: "{ branch: master }", + with: frontend.Metadata{Curr: frontend.Build{Commit: frontend.Commit{Branch: "master"}}}, + 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 { - c := &Constraint{} +func parseConstraint(t *testing.T, s string) *List { + c := &List{} assert.NoError(t, yaml.Unmarshal([]byte(s), c)) return c } -func parseConstraintMap(t *testing.T, s string) *ConstraintMap { - c := &ConstraintMap{} +func parseConstraintMap(t *testing.T, s string) *Map { + c := &Map{} assert.NoError(t, yaml.Unmarshal([]byte(s), c)) return c } -func parseConstraintPath(t *testing.T, s string) *ConstraintPath { - c := &ConstraintPath{} +func parseConstraintPath(t *testing.T, s string) *Path { + c := &Path{} assert.NoError(t, yaml.Unmarshal([]byte(s), c)) return c } diff --git a/pipeline/frontend/yaml/container.go b/pipeline/frontend/yaml/container.go index 9b30356f0..8543e7b42 100644 --- a/pipeline/frontend/yaml/container.go +++ b/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 diff --git a/pipeline/frontend/yaml/container_test.go b/pipeline/frontend/yaml/container_test.go index ad7708ec7..52cd0d82c 100644 --- a/pipeline/frontend/yaml/container_test.go +++ b/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{ - Branch: Constraint{ + Constraints: constraint.Constraints{ + Branch: constraint.List{ Include: []string{"master"}, }, }, @@ -188,9 +189,9 @@ func TestUnmarshalContainers(t *testing.T) { "tag": stringsToInterface("next", "latest"), "dry_run": true, }, - Constraints: Constraints{ - Event: Constraint{Include: []string{"push"}}, - Branch: Constraint{Include: []string{"${CI_REPO_DEFAULT_BRANCH}"}}, + Constraints: constraint.Constraints{ + Event: constraint.List{Include: []string{"push"}}, + 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{ - Event: Constraint{Include: []string{"push"}}, - Branch: Constraint{Include: []string{"${CI_REPO_DEFAULT_BRANCH}"}}, + Constraints: constraint.Constraints{ + Event: constraint.List{Include: []string{"push"}}, + Branch: constraint.List{Include: []string{"${CI_REPO_DEFAULT_BRANCH}"}}, }, }, },