diff --git a/docs/pipelines.md b/docs/pipelines.md index 300835e9ddf..44071ab5a3a 100644 --- a/docs/pipelines.md +++ b/docs/pipelines.md @@ -777,12 +777,23 @@ There are a lot of scenarios where `when` expressions can be really useful. Some #### Use CEL expression in WhenExpression -> :seedling: **`CEL in WhenExpression` is an [alpha](install.md#alpha-features) feature.** +> :seedling: **`CEL in WhenExpression` is an [alpha](additional-configs.md#alpha-features) feature.** > The `enable-cel-in-whenexpression` feature flag must be set to `"true"` to enable the use of `CEL` in `WhenExpression`. > > :warning: This feature is in a preview mode. > It is still in a very early stage of development and is not yet fully functional +`CEL` expression is validated at admission webhook and a validation error will be returned if the expression is invalid. + +**Note:** To use Tekton's [variable substitution](variables.md), you need to wrap the reference with single quotes. This also means that if you pass another CEL expression via `params` or `results`, it won't be executed. Therefore CEL injection is disallowed. + +For example: +``` +This is valid: '$(params.foo)' == 'foo' +This is invalid: $(params.foo) == 'foo' +CEL's variable substitution is not supported yet and thus invalid: params.foo == 'foo' +``` + #### Guarding a `Task` and its dependent `Tasks` To guard a `Task` and its dependent Tasks: diff --git a/pkg/apis/pipeline/v1/when_validation.go b/pkg/apis/pipeline/v1/when_validation.go index fb8a92487d9..d71804101a3 100644 --- a/pkg/apis/pipeline/v1/when_validation.go +++ b/pkg/apis/pipeline/v1/when_validation.go @@ -21,8 +21,7 @@ import ( "fmt" "strings" - // TODO(#7244): Pull the cel-go library for now, the following PR will use the library. - _ "github.com/google/cel-go/cel" + "github.com/google/cel-go/cel" "github.com/tektoncd/pipeline/pkg/apis/config" "k8s.io/apimachinery/pkg/api/equality" "k8s.io/apimachinery/pkg/selection" @@ -54,6 +53,21 @@ func (we *WhenExpression) validateWhenExpressionFields(ctx context.Context) *api if we.Input != "" || we.Operator != "" || len(we.Values) != 0 { return apis.ErrGeneric(fmt.Sprintf("cel and input+operator+values cannot be set in one WhenExpression: %v", we)) } + + // We need to compile the CEL expression and check if it is a valid expression + // note that at the validation webhook, Tekton's variables are not substituted, + // so they need to be wrapped with single quotes. + // e.g. This is a valid CEL expression: '$(params.foo)' == 'foo'; + // But this is not a valid expression since CEL cannot recognize: $(params.foo) == 'foo'; + // This is not valid since we don't pass params to CEL's environment: params.foo == 'foo'; + env, err := cel.NewEnv() + if err != nil { + return apis.ErrGeneric("err: %s", err.Error()) + } + _, iss := env.Compile(we.CEL) + if iss.Err() != nil { + return apis.ErrGeneric("invalid cel expression: %s with err: %s", we.CEL, iss.Err().Error()) + } return nil } diff --git a/pkg/apis/pipeline/v1/when_validation_test.go b/pkg/apis/pipeline/v1/when_validation_test.go index f3dbeb962d6..eb4c615bcf3 100644 --- a/pkg/apis/pipeline/v1/when_validation_test.go +++ b/pkg/apis/pipeline/v1/when_validation_test.go @@ -116,10 +116,35 @@ func TestCELinWhenExpressions_Valid(t *testing.T) { name string wes WhenExpressions }{{ - name: "valid cel", + name: "valid operator - Equal", wes: []WhenExpression{{ CEL: " 'foo' == 'foo' ", }}, + }, { + name: "valid operator - NotEqual", + wes: []WhenExpression{{ + CEL: " 'foo' != 'foo' ", + }}, + }, { + name: "valid operator - In", + wes: []WhenExpression{{ + CEL: "'foo' in ['foo', 'bar']", + }}, + }, { + name: "valid operator - NotIn", + wes: []WhenExpression{{ + CEL: "!('foo' in ['foo', 'bar'])", + }}, + }, { + name: "valid regex expression", + wes: []WhenExpression{{ + CEL: "'release/v1'.matches('release/.*')", + }}, + }, { + name: "valid variable reference syntax", + wes: []WhenExpression{{ + CEL: "'$(params.foo)' in ['foo', 'bar']", + }}, }} for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -141,6 +166,18 @@ func TestCELWhenExpressions_Invalid(t *testing.T) { CEL: " 'foo' == 'foo' ", }}, enableCELInWhenExpression: false, + }, { + name: "variable reference should be wrapped with single quotes", + wes: []WhenExpression{{ + CEL: " $(params.foo) == 'foo' ", + }}, + enableCELInWhenExpression: true, + }, { + name: "variables not declared in environment", + wes: []WhenExpression{{ + CEL: " params.foo == 'foo' ", + }}, + enableCELInWhenExpression: true, }, { name: "CEL should not coexist with input+operator+values", wes: []WhenExpression{{ diff --git a/pkg/apis/pipeline/v1beta1/when_validation.go b/pkg/apis/pipeline/v1beta1/when_validation.go index 8279950afab..e66dbb73bf5 100644 --- a/pkg/apis/pipeline/v1beta1/when_validation.go +++ b/pkg/apis/pipeline/v1beta1/when_validation.go @@ -21,6 +21,7 @@ import ( "fmt" "strings" + "github.com/google/cel-go/cel" "github.com/tektoncd/pipeline/pkg/apis/config" "k8s.io/apimachinery/pkg/api/equality" "k8s.io/apimachinery/pkg/selection" @@ -52,6 +53,20 @@ func (we *WhenExpression) validateWhenExpressionFields(ctx context.Context) *api if we.Input != "" || we.Operator != "" || len(we.Values) != 0 { return apis.ErrGeneric(fmt.Sprintf("cel and input+operator+values cannot be set in one WhenExpression: %v", we)) } + // We need to compile the CEL expression and check if it is a valid expression + // note that at the validation webhook, Tekton's variables are not substituted, + // so they need to be wrapped with single quotes. + // e.g. This is a valid CEL expression: '$(params.foo)' == 'foo'; + // But this is not a valid expression since CEL cannot recognize: $(params.foo) == 'foo'; + // This is not valid since we don't pass params to CEL's environment: params.foo == 'foo'; + env, err := cel.NewEnv() + if err != nil { + return apis.ErrGeneric("err: %s", err.Error()) + } + _, iss := env.Compile(we.CEL) + if iss.Err() != nil { + return apis.ErrGeneric("invalid cel expression: %s with err: %s", we.CEL, iss.Err().Error()) + } return nil } diff --git a/pkg/apis/pipeline/v1beta1/when_validation_test.go b/pkg/apis/pipeline/v1beta1/when_validation_test.go index 16ec85b52fd..0393cb3c288 100644 --- a/pkg/apis/pipeline/v1beta1/when_validation_test.go +++ b/pkg/apis/pipeline/v1beta1/when_validation_test.go @@ -116,10 +116,35 @@ func TestCELinWhenExpressions_Valid(t *testing.T) { name string wes WhenExpressions }{{ - name: "valid cel", + name: "valid operator - Equal", wes: []WhenExpression{{ CEL: " 'foo' == 'foo' ", }}, + }, { + name: "valid operator - NotEqual", + wes: []WhenExpression{{ + CEL: " 'foo' != 'foo' ", + }}, + }, { + name: "valid operator - In", + wes: []WhenExpression{{ + CEL: "'foo' in ['foo', 'bar']", + }}, + }, { + name: "valid operator - NotIn", + wes: []WhenExpression{{ + CEL: "!('foo' in ['foo', 'bar'])", + }}, + }, { + name: "valid regex expression", + wes: []WhenExpression{{ + CEL: "'release/v1'.matches('release/.*')", + }}, + }, { + name: "valid variable reference syntax", + wes: []WhenExpression{{ + CEL: "'$(params.foo)' in ['foo', 'bar']", + }}, }} for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -141,6 +166,18 @@ func TestCELWhenExpressions_Invalid(t *testing.T) { CEL: " 'foo' == 'foo' ", }}, enableCELInWhenExpression: false, + }, { + name: "variable reference should be wrapped with single quotes", + wes: []WhenExpression{{ + CEL: " $(params.foo) == 'foo' ", + }}, + enableCELInWhenExpression: true, + }, { + name: "variables not declared in environment", + wes: []WhenExpression{{ + CEL: " params.foo == 'foo' ", + }}, + enableCELInWhenExpression: true, }, { name: "CEL should not coexist with input+operator+values", wes: []WhenExpression{{