Skip to content

Commit

Permalink
Fix json annotations in WhenExpression
Browse files Browse the repository at this point in the history
When a pipeline with when expressions is created in v0.16.* then it is
run in a pipelinerun in v0.17.0 or v0.17.1, a pipeline validation error
about missing fields would be thrown -- as reported in #3382

That happened because json annotations were added to when expressions
type in v0.17.0 so that the fields would have lowercase, such as in the
code completion in tekton intellij plugin as described in
redhat-developer/intellij-tekton#223

Without the json annotations, the fields were stored with first letters
capitalized, that is Input, Operator and Values. With the json
annotations, the fields were expected to be lowercase, that is input,
operator and values, causing the missing fields error in pipeline
validation

As such, users would have to reapply pipelines definitions created in
previous versions to make them work in v0.17.0 or v0.17.1 -- to remove
this requirement, we need to support both the uppercase and lowercase
first letters for the annotations

Fixes #3382
  • Loading branch information
jerop authored and tekton-robot committed Oct 21, 2020
1 parent 330d64a commit 4edcbc1
Show file tree
Hide file tree
Showing 5 changed files with 101 additions and 14 deletions.
61 changes: 53 additions & 8 deletions pkg/apis/pipeline/v1beta1/when_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,24 +25,69 @@ import (
type WhenExpression struct {
// Input is the string for guard checking which can be a static input or an output from a parent Task
Input string `json:"input"`

// DeprecatedInput for backwards compatibility with <v0.17
// it is the string for guard checking which can be a static input or an output from a parent Task
// +optional
DeprecatedInput string `json:"Input,omitempty"`

// Operator that represents an Input's relationship to the values
Operator selection.Operator `json:"operator"`

// DeprecatedOperator for backwards compatibility with <v0.17
// it represents a DeprecatedInput's relationship to the DeprecatedValues
// +optional
DeprecatedOperator selection.Operator `json:"Operator,omitempty"`

// Values is an array of strings, which is compared against the input, for guard checking
// It must be non-empty
Values []string `json:"values"`

// DeprecatedValues for backwards compatibility with <v0.17
// it represents a DeprecatedInput's relationship to the DeprecatedValues
// +optional
DeprecatedValues []string `json:"Values,omitempty"`
}

// GetInput returns the input string for guard checking
// based on DeprecatedInput (<v0.17) and Input
func (we *WhenExpression) GetInput() string {
if we.Input == "" {
return we.DeprecatedInput
}
return we.Input
}

// GetOperator returns the relationship between input and values
// based on DeprecatedOperator (<v0.17) and Operator
func (we *WhenExpression) GetOperator() selection.Operator {
if we.Operator == "" {
return we.DeprecatedOperator
}
return we.Operator
}

// GetValues returns an array of strings which is compared against the input
// based on DeprecatedValues (<v0.17) and Values
func (we *WhenExpression) GetValues() []string {
if we.Values == nil {
return we.DeprecatedValues
}
return we.Values
}

func (we *WhenExpression) isInputInValues() bool {
for i := range we.Values {
if we.Values[i] == we.Input {
values := we.GetValues()
for i := range values {
if values[i] == we.GetInput() {
return true
}
}
return false
}

func (we *WhenExpression) isTrue() bool {
if we.Operator == selection.In {
if we.GetOperator() == selection.In {
return we.isInputInValues()
}
// selection.NotIn
Expand All @@ -57,21 +102,21 @@ func (we *WhenExpression) hasVariable() bool {
}

func (we *WhenExpression) applyReplacements(replacements map[string]string) WhenExpression {
replacedInput := ApplyReplacements(we.Input, replacements)
replacedInput := ApplyReplacements(we.GetInput(), replacements)

var replacedValues []string
for _, val := range we.Values {
for _, val := range we.GetValues() {
replacedValues = append(replacedValues, ApplyReplacements(val, replacements))
}

return WhenExpression{Input: replacedInput, Operator: we.Operator, Values: replacedValues}
return WhenExpression{Input: replacedInput, Operator: we.GetOperator(), Values: replacedValues}
}

// GetVarSubstitutionExpressions extracts all the values between "$(" and ")" in a When Expression
func (we *WhenExpression) GetVarSubstitutionExpressions() ([]string, bool) {
var allExpressions []string
allExpressions = append(allExpressions, validateString(we.Input)...)
for _, value := range we.Values {
allExpressions = append(allExpressions, validateString(we.GetInput())...)
for _, value := range we.GetValues() {
allExpressions = append(allExpressions, validateString(value)...)
}
return allExpressions, len(allExpressions) != 0
Expand Down
23 changes: 18 additions & 5 deletions pkg/apis/pipeline/v1beta1/when_validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,13 +48,26 @@ func (we *WhenExpression) validateWhenExpressionFields() *apis.FieldError {
if equality.Semantic.DeepEqual(we, &WhenExpression{}) || we == nil {
return apis.ErrMissingField(apis.CurrentField)
}
if !sets.NewString(validWhenOperators...).Has(string(we.Operator)) {
message := fmt.Sprintf("operator %q is not recognized. valid operators: %s", we.Operator, strings.Join(validWhenOperators, ","))
if !sets.NewString(validWhenOperators...).Has(string(we.GetOperator())) {
message := fmt.Sprintf("operator %q is not recognized. valid operators: %s", we.GetOperator(), strings.Join(validWhenOperators, ","))
return apis.ErrInvalidValue(message, apis.CurrentField)
}
if len(we.Values) == 0 {
if len(we.GetValues()) == 0 {
return apis.ErrInvalidValue("expecting non-empty values field", apis.CurrentField)
}
return we.validateWhenExpressionsDuplicateFields()
}

func (we *WhenExpression) validateWhenExpressionsDuplicateFields() *apis.FieldError {
if we.Input != "" && we.DeprecatedInput != "" {
return apis.ErrMultipleOneOf("input", "Input")
}
if we.Operator != "" && we.DeprecatedOperator != "" {
return apis.ErrMultipleOneOf("operator", "Operator")
}
if we.Values != nil && we.DeprecatedValues != nil {
return apis.ErrMultipleOneOf("values", "Values")
}
return nil
}

Expand All @@ -77,8 +90,8 @@ func (wes WhenExpressions) validateTaskResultsVariables() *apis.FieldError {

func (wes WhenExpressions) validatePipelineParametersVariables(prefix string, paramNames sets.String, arrayParamNames sets.String) (errs *apis.FieldError) {
for idx, we := range wes {
errs = errs.Also(validateStringVariable(we.Input, prefix, paramNames, arrayParamNames).ViaField("input").ViaFieldIndex("when", idx))
for _, val := range we.Values {
errs = errs.Also(validateStringVariable(we.GetInput(), prefix, paramNames, arrayParamNames).ViaField("input").ViaFieldIndex("when", idx))
for _, val := range we.GetValues() {
errs = errs.Also(validateStringVariable(val, prefix, paramNames, arrayParamNames).ViaField("values").ViaFieldIndex("when", idx))
}
}
Expand Down
24 changes: 24 additions & 0 deletions pkg/apis/pipeline/v1beta1/when_validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,30 @@ func TestWhenExpressions_Invalid(t *testing.T) {
Operator: selection.In,
Values: []string{"bar"},
}},
}, {
name: "multiple inputs",
wes: []WhenExpression{{
Input: "foo",
DeprecatedInput: "nay",
Operator: selection.In,
Values: []string{"bar"},
}},
}, {
name: "multiple operators",
wes: []WhenExpression{{
Input: "foo",
Operator: selection.In,
DeprecatedOperator: selection.NotIn,
Values: []string{"bar"},
}},
}, {
name: "multiple values",
wes: []WhenExpression{{
Input: "foo",
Operator: selection.In,
Values: []string{"bar"},
DeprecatedValues: []string{"bar"},
}},
}, {
name: "missing when expression",
wes: []WhenExpression{{}},
Expand Down
5 changes: 5 additions & 0 deletions pkg/apis/pipeline/v1beta1/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ func convertWhenExpressions(whenExpressions []v1beta1.WhenExpression, pipelineRu
if ok {
resolvedResultRefs, err := extractResultRefs(expressions, pipelineRunState)
if err != nil {
return nil, fmt.Errorf("unable to find result referenced by when expression with input %q in task %q: %w", whenExpression.Input, name, err)
return nil, fmt.Errorf("unable to find result referenced by when expression with input %q in task %q: %w", whenExpression.GetInput(), name, err)
}
if resolvedResultRefs != nil {
resolvedWhenExpressions = append(resolvedWhenExpressions, resolvedResultRefs...)
Expand Down

0 comments on commit 4edcbc1

Please sign in to comment.