diff --git a/pkg/apis/pipeline/v1/when_types.go b/pkg/apis/pipeline/v1/when_types.go index dfd6948ce73..66e7164a779 100644 --- a/pkg/apis/pipeline/v1/when_types.go +++ b/pkg/apis/pipeline/v1/when_types.go @@ -105,10 +105,7 @@ type WhenExpressions []WhenExpression // if the Task should be skipped. func (wes WhenExpressions) AllowsExecution(evaluatedCEL map[string]bool) bool { for _, we := range wes { - if we.CEL != "" { - return evaluatedCEL[we.CEL] - } - if !we.isTrue() { + if !we.isTrue() || (we.CEL != "" && !evaluatedCEL[we.CEL]) { return false } } diff --git a/pkg/apis/pipeline/v1/when_types_test.go b/pkg/apis/pipeline/v1/when_types_test.go index 22fe919d9fa..7d706a4b303 100644 --- a/pkg/apis/pipeline/v1/when_types_test.go +++ b/pkg/apis/pipeline/v1/when_types_test.go @@ -96,7 +96,36 @@ func TestAllowsExecution(t *testing.T) { }, evaluatedCEL: map[string]bool{"'foo'!='foo'": false}, expected: false, - }} + }, + { + name: "multiple expressions - 1. CEL is true 2. In Op is false, expect false", + whenExpressions: WhenExpressions{ + { + CEL: "'foo'=='foo'", + }, + { + Input: "foo", + Operator: selection.In, + Values: []string{"bar"}, + }, + }, + evaluatedCEL: map[string]bool{"'foo'=='foo'": true}, + expected: false, + }, + { + name: "multiple expressions - 1. CEL is true 2. CEL is false, expect false", + whenExpressions: WhenExpressions{ + { + CEL: "'foo'!='foo'", + }, + { + CEL: "'xxx'!='xxx'", + }, + }, + evaluatedCEL: map[string]bool{"'foo'=='foo'": true, "'xxx'!='xxx'": false}, + expected: false, + }, + } for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { got := tc.whenExpressions.AllowsExecution(tc.evaluatedCEL) diff --git a/pkg/apis/pipeline/v1beta1/when_types.go b/pkg/apis/pipeline/v1beta1/when_types.go index 8cea2f5ebf1..f792ec199c8 100644 --- a/pkg/apis/pipeline/v1beta1/when_types.go +++ b/pkg/apis/pipeline/v1beta1/when_types.go @@ -105,10 +105,7 @@ type WhenExpressions []WhenExpression // if the Task should be skipped. func (wes WhenExpressions) AllowsExecution(evaluatedCEL map[string]bool) bool { for _, we := range wes { - if we.CEL != "" { - return evaluatedCEL[we.CEL] - } - if !we.isTrue() { + if !we.isTrue() || (we.CEL != "" && !evaluatedCEL[we.CEL]) { return false } } diff --git a/pkg/apis/pipeline/v1beta1/when_types_test.go b/pkg/apis/pipeline/v1beta1/when_types_test.go index 914040d48d8..a7a75a2a635 100644 --- a/pkg/apis/pipeline/v1beta1/when_types_test.go +++ b/pkg/apis/pipeline/v1beta1/when_types_test.go @@ -96,7 +96,36 @@ func TestAllowsExecution(t *testing.T) { }, evaluatedCEL: map[string]bool{"'foo'!='foo'": false}, expected: false, - }} + }, + { + name: "multiple expressions - 1. CEL is true 2. In Op is false, expect false", + whenExpressions: WhenExpressions{ + { + CEL: "'foo'=='foo'", + }, + { + Input: "foo", + Operator: selection.In, + Values: []string{"bar"}, + }, + }, + evaluatedCEL: map[string]bool{"'foo'=='foo'": true}, + expected: false, + }, + { + name: "multiple expressions - 1. CEL is true 2. CEL is false, expect false", + whenExpressions: WhenExpressions{ + { + CEL: "'foo'!='foo'", + }, + { + CEL: "'xxx'!='xxx'", + }, + }, + evaluatedCEL: map[string]bool{"'foo'=='foo'": true, "'xxx'!='xxx'": false}, + expected: false, + }, + } for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { got := tc.whenExpressions.AllowsExecution(tc.evaluatedCEL)