From b616a822a068ef81aa47c6d1857524388ab23f6a Mon Sep 17 00:00:00 2001
From: Thomas Anderson <127358482+zc-devs@users.noreply.github.com>
Date: Mon, 3 Jul 2023 00:45:22 +0300
Subject: [PATCH] Fixed when:evaluate on non-standard (non-CI*) env vars
 (#1907)

Makes it possible to evaluate `when` constraint on custom environment
variables.
---
 docs/docs/20-usage/20-pipeline-syntax.md        |  9 ++++++++-
 docs/docs/20-usage/50-environment.md            |  2 +-
 pipeline/frontend/yaml/compiler/compiler.go     |  8 ++++----
 pipeline/frontend/yaml/constraint/constraint.go | 17 +++++++++++------
 .../frontend/yaml/constraint/constraint_test.go | 17 ++++++++++++++++-
 pipeline/frontend/yaml/parse_test.go            |  8 ++++----
 pipeline/stepBuilder.go                         |  2 +-
 server/pipeline/filter.go                       |  2 +-
 8 files changed, 46 insertions(+), 19 deletions(-)

diff --git a/docs/docs/20-usage/20-pipeline-syntax.md b/docs/docs/20-usage/20-pipeline-syntax.md
index 276e4df6f..c674e3ce3 100644
--- a/docs/docs/20-usage/20-pipeline-syntax.md
+++ b/docs/docs/20-usage/20-pipeline-syntax.md
@@ -444,7 +444,7 @@ when:
 
 #### `evaluate`
 
-Execute a step only if the provided evaluate expression is equal to true. Each [`CI_` variable](./50-environment.md#built-in-environment-variables) can be used inside the expression.
+Execute a step only if the provided evaluate expression is equal to true. Both built-in [`CI_`](./50-environment.md#built-in-environment-variables) and custom variables can be used inside the expression.
 
 The expression syntax can be found in [the docs](https://github.com/antonmedv/expr/blob/master/docs/Language-Definition.md) of the underlying library.
 
@@ -476,6 +476,13 @@ when:
   - evaluate: 'CI_COMMIT_PULL_REQUEST_LABELS contains "deploy"'
 ```
 
+Skip step only if `SKIP=true`, run otherwise or if undefined:
+
+```yaml
+when:
+  - evaluate: 'SKIP != "true"'
+```
+
 ### `group` - Parallel execution
 
 Woodpecker supports parallel step execution for same-machine fan-in and fan-out. Parallel steps are configured using the `group` attribute. This instructs the pipeline runner to execute the named group in parallel.
diff --git a/docs/docs/20-usage/50-environment.md b/docs/docs/20-usage/50-environment.md
index 67f610b0f..cb2bf2362 100644
--- a/docs/docs/20-usage/50-environment.md
+++ b/docs/docs/20-usage/50-environment.md
@@ -1,6 +1,6 @@
 # Environment variables
 
-Woodpecker provides the ability to pass environment variables to individual pipeline steps. Example pipeline step with custom environment variables:
+Woodpecker provides the ability to pass environment variables to individual pipeline steps. Note that these can't overwrite any existing, built-in variables. Example pipeline step with custom environment variables:
 
 ```diff
 steps:
diff --git a/pipeline/frontend/yaml/compiler/compiler.go b/pipeline/frontend/yaml/compiler/compiler.go
index 988d37bd2..d89f6d638 100644
--- a/pipeline/frontend/yaml/compiler/compiler.go
+++ b/pipeline/frontend/yaml/compiler/compiler.go
@@ -104,7 +104,7 @@ func New(opts ...Option) *Compiler {
 func (c *Compiler) Compile(conf *yaml_types.Workflow) (*backend_types.Config, error) {
 	config := new(backend_types.Config)
 
-	if match, err := conf.When.Match(c.metadata, true); !match && err == nil {
+	if match, err := conf.When.Match(c.metadata, true, c.env); !match && err == nil {
 		// This pipeline does not match the configured filter so return an empty config and stop further compilation.
 		// An empty pipeline will just be skipped completely.
 		return config, nil
@@ -177,7 +177,7 @@ func (c *Compiler) Compile(conf *yaml_types.Workflow) (*backend_types.Config, er
 		config.Stages = append(config.Stages, stage)
 	} else if !c.local && !conf.SkipClone {
 		for i, container := range conf.Clone.ContainerList {
-			if match, err := container.When.Match(c.metadata, false); !match && err == nil {
+			if match, err := container.When.Match(c.metadata, false, c.env); !match && err == nil {
 				continue
 			} else if err != nil {
 				return nil, err
@@ -212,7 +212,7 @@ func (c *Compiler) Compile(conf *yaml_types.Workflow) (*backend_types.Config, er
 		stage.Alias = nameServices
 
 		for i, container := range conf.Services.ContainerList {
-			if match, err := container.When.Match(c.metadata, false); !match && err == nil {
+			if match, err := container.When.Match(c.metadata, false, c.env); !match && err == nil {
 				continue
 			} else if err != nil {
 				return nil, err
@@ -234,7 +234,7 @@ func (c *Compiler) Compile(conf *yaml_types.Workflow) (*backend_types.Config, er
 			continue
 		}
 
-		if match, err := container.When.Match(c.metadata, false); !match && err == nil {
+		if match, err := container.When.Match(c.metadata, false, c.env); !match && err == nil {
 			continue
 		} else if err != nil {
 			return nil, err
diff --git a/pipeline/frontend/yaml/constraint/constraint.go b/pipeline/frontend/yaml/constraint/constraint.go
index 1a1fd63f4..006308070 100644
--- a/pipeline/frontend/yaml/constraint/constraint.go
+++ b/pipeline/frontend/yaml/constraint/constraint.go
@@ -8,6 +8,7 @@ import (
 
 	"github.com/antonmedv/expr"
 	"github.com/bmatcuk/doublestar/v4"
+	"golang.org/x/exp/maps"
 	"gopkg.in/yaml.v3"
 
 	"github.com/woodpecker-ci/woodpecker/pipeline/frontend/metadata"
@@ -62,9 +63,9 @@ func (when *When) IsEmpty() bool {
 }
 
 // Returns true if at least one of the internal constraints is true.
-func (when *When) Match(metadata metadata.Metadata, global bool) (bool, error) {
+func (when *When) Match(metadata metadata.Metadata, global bool, env map[string]string) (bool, error) {
 	for _, c := range when.Constraints {
-		match, err := c.Match(metadata, global)
+		match, err := c.Match(metadata, global, env)
 		if err != nil {
 			return false, err
 		}
@@ -76,7 +77,7 @@ func (when *When) Match(metadata metadata.Metadata, global bool) (bool, error) {
 	if when.IsEmpty() {
 		// test against default Constraints
 		empty := &Constraint{}
-		return empty.Match(metadata, global)
+		return empty.Match(metadata, global, env)
 	}
 	return false, nil
 }
@@ -139,7 +140,7 @@ func (when *When) UnmarshalYAML(value *yaml.Node) error {
 
 // Match returns true if all constraints match the given input. If a single
 // constraint fails a false value is returned.
-func (c *Constraint) Match(m metadata.Metadata, global bool) (bool, error) {
+func (c *Constraint) Match(m metadata.Metadata, global bool, env map[string]string) (bool, error) {
 	match := true
 	if !global {
 		// apply step only filters
@@ -167,8 +168,12 @@ func (c *Constraint) Match(m metadata.Metadata, global bool) (bool, error) {
 	}
 
 	if c.Evaluate != "" {
-		env := m.Environ()
-		out, err := expr.Compile(c.Evaluate, expr.Env(env), expr.AsBool())
+		if env == nil {
+			env = m.Environ()
+		} else {
+			maps.Copy(env, m.Environ())
+		}
+		out, err := expr.Compile(c.Evaluate, expr.Env(env), expr.AllowUndefinedVariables(), expr.AsBool())
 		if err != nil {
 			return false, err
 		}
diff --git a/pipeline/frontend/yaml/constraint/constraint_test.go b/pipeline/frontend/yaml/constraint/constraint_test.go
index 0e92be18e..3643ef9a1 100644
--- a/pipeline/frontend/yaml/constraint/constraint_test.go
+++ b/pipeline/frontend/yaml/constraint/constraint_test.go
@@ -405,6 +405,7 @@ func TestConstraints(t *testing.T) {
 		desc string
 		conf string
 		with metadata.Metadata
+		env  map[string]string
 		want bool
 	}{
 		{
@@ -510,6 +511,20 @@ func TestConstraints(t *testing.T) {
 			with: metadata.Metadata{Curr: metadata.Pipeline{Event: metadata.EventPush}, Repo: metadata.Repo{Owner: "owner", Name: "repo"}},
 			want: true,
 		},
+		{
+			desc: "filter by eval based on custom variable",
+			conf: `{ evaluate: 'TESTVAR == "testval"' }`,
+			with: metadata.Metadata{Curr: metadata.Pipeline{Event: metadata.EventManual}},
+			env:  map[string]string{"TESTVAR": "testval"},
+			want: true,
+		},
+		{
+			desc: "filter by eval based on custom variable",
+			conf: `{ evaluate: 'TESTVAR == "testval"' }`,
+			with: metadata.Metadata{Curr: metadata.Pipeline{Event: metadata.EventManual}},
+			env:  map[string]string{"TESTVAR": "qwe"},
+			want: false,
+		},
 	}
 
 	for _, test := range testdata {
@@ -517,7 +532,7 @@ func TestConstraints(t *testing.T) {
 			conf, err := frontend.EnvVarSubst(test.conf, test.with.Environ())
 			assert.NoError(t, err)
 			c := parseConstraints(t, conf)
-			got, err := c.Match(test.with, false)
+			got, err := c.Match(test.with, false, test.env)
 			if err != nil {
 				t.Errorf("Match returned error: %v", err)
 			}
diff --git a/pipeline/frontend/yaml/parse_test.go b/pipeline/frontend/yaml/parse_test.go
index 3f9f9aef8..d72eaef38 100644
--- a/pipeline/frontend/yaml/parse_test.go
+++ b/pipeline/frontend/yaml/parse_test.go
@@ -84,7 +84,7 @@ func TestParse(t *testing.T) {
 					Curr: metadata.Pipeline{
 						Event: "tester",
 					},
-				}, false)
+				}, false, nil)
 				g.Assert(match).Equal(true)
 				g.Assert(err).IsNil()
 			})
@@ -94,7 +94,7 @@ func TestParse(t *testing.T) {
 					Curr: metadata.Pipeline{
 						Event: "tester2",
 					},
-				}, false)
+				}, false, nil)
 				g.Assert(match).Equal(true)
 				g.Assert(err).IsNil()
 			})
@@ -106,7 +106,7 @@ func TestParse(t *testing.T) {
 							Branch: "tester",
 						},
 					},
-				}, true)
+				}, true, nil)
 				g.Assert(match).Equal(true)
 				g.Assert(err).IsNil()
 			})
@@ -116,7 +116,7 @@ func TestParse(t *testing.T) {
 					Curr: metadata.Pipeline{
 						Event: "push",
 					},
-				}, false)
+				}, false, nil)
 				g.Assert(match).Equal(false)
 				g.Assert(err).IsNil()
 			})
diff --git a/pipeline/stepBuilder.go b/pipeline/stepBuilder.go
index 3e5afc584..3c7d5eef2 100644
--- a/pipeline/stepBuilder.go
+++ b/pipeline/stepBuilder.go
@@ -118,7 +118,7 @@ func (b *StepBuilder) Build() ([]*Item, error) {
 			}
 
 			// checking if filtered.
-			if match, err := parsed.When.Match(workflowMetadata, true); !match && err == nil {
+			if match, err := parsed.When.Match(workflowMetadata, true, environ); !match && err == nil {
 				log.Debug().Str("pipeline", workflow.Name).Msg(
 					"Marked as skipped, dose not match metadata",
 				)
diff --git a/server/pipeline/filter.go b/server/pipeline/filter.go
index 4b3e9fcba..a001d9b38 100644
--- a/server/pipeline/filter.go
+++ b/server/pipeline/filter.go
@@ -72,7 +72,7 @@ func checkIfFiltered(repo *model.Repo, p *model.Pipeline, forgeYamlConfigs []*fo
 		log.Trace().Msgf("config '%s': %#v", forgeYamlConfig.Name, parsedPipelineConfig)
 
 		// ignore if the pipeline was filtered by matched constraints
-		if match, err := parsedPipelineConfig.When.Match(matchMetadata, true); !match && err == nil {
+		if match, err := parsedPipelineConfig.When.Match(matchMetadata, true, p.AdditionalVariables); !match && err == nil {
 			continue
 		} else if err != nil {
 			return false, err