Skip to content

Commit

Permalink
TEP-0118: Update TaskRun Validation for Matrix Include Params
Browse files Browse the repository at this point in the history
This commit updates TaskRun validation to account for Matrix Include Params.

Note: This feature is still in preview mode.
  • Loading branch information
EmmaMunley committed Mar 22, 2023
1 parent 013ffad commit dc39b2b
Show file tree
Hide file tree
Showing 2 changed files with 119 additions and 47 deletions.
43 changes: 14 additions & 29 deletions pkg/reconciler/taskrun/validate_taskrun.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,53 +30,38 @@ import (
"k8s.io/apimachinery/pkg/util/sets"
)

func validateParams(ctx context.Context, paramSpecs []v1beta1.ParamSpec, params []v1beta1.Param, matrix *v1beta1.Matrix) error {
func validateParams(ctx context.Context, paramSpecs []v1beta1.ParamSpec, params v1beta1.Params, matrix *v1beta1.Matrix) error {
neededParamsNames, neededParamsTypes := neededParamsNamesAndTypes(paramSpecs)
var matrixParams []v1beta1.Param
if matrix != nil {
matrixParams = matrix.Params
}
providedParamsNames := providedParamsNames(append(params, matrixParams...))
providedParams := params
matrixAllParams := matrix.GetAllParams()
providedParams = append(providedParams, matrixAllParams...)
providedParamsNames := providedParams.ExtractNames()
if missingParamsNames := missingParamsNames(neededParamsNames, providedParamsNames, paramSpecs); len(missingParamsNames) != 0 {
return fmt.Errorf("missing values for these params which have no default values: %s", missingParamsNames)
}
if wrongTypeParamNames := wrongTypeParamsNames(params, matrixParams, neededParamsTypes); len(wrongTypeParamNames) != 0 {
if wrongTypeParamNames := wrongTypeParamsNames(params, matrixAllParams, neededParamsTypes); len(wrongTypeParamNames) != 0 {
return fmt.Errorf("param types don't match the user-specified type: %s", wrongTypeParamNames)
}
if missingKeysObjectParamNames := MissingKeysObjectParamNames(paramSpecs, params); len(missingKeysObjectParamNames) != 0 {
return fmt.Errorf("missing keys for these params which are required in ParamSpec's properties %v", missingKeysObjectParamNames)
}

return nil
}

func neededParamsNamesAndTypes(paramSpecs []v1beta1.ParamSpec) ([]string, map[string]v1beta1.ParamType) {
var neededParamsNames []string
func neededParamsNamesAndTypes(paramSpecs []v1beta1.ParamSpec) (sets.String, map[string]v1beta1.ParamType) {
neededParamsNames := sets.String{}
neededParamsTypes := make(map[string]v1beta1.ParamType)
neededParamsNames = make([]string, 0, len(paramSpecs))
for _, inputResourceParam := range paramSpecs {
neededParamsNames = append(neededParamsNames, inputResourceParam.Name)
neededParamsNames.Insert(inputResourceParam.Name)
neededParamsTypes[inputResourceParam.Name] = inputResourceParam.Type
}
return neededParamsNames, neededParamsTypes
}

func providedParamsNames(params []v1beta1.Param) []string {
providedParamsNames := make([]string, 0, len(params))
for _, param := range params {
providedParamsNames = append(providedParamsNames, param.Name)
}
return providedParamsNames
}

func missingParamsNames(neededParams []string, providedParams []string, paramSpecs []v1beta1.ParamSpec) []string {
missingParamsNames := list.DiffLeft(neededParams, providedParams)
func missingParamsNames(neededParams sets.String, providedParams sets.String, paramSpecs []v1beta1.ParamSpec) []string {
missingParamsNames := neededParams.Difference(providedParams)
var missingParamsNamesWithNoDefaults []string
for _, param := range missingParamsNames {
for _, inputResourceParam := range paramSpecs {
if inputResourceParam.Name == param && inputResourceParam.Default == nil {
missingParamsNamesWithNoDefaults = append(missingParamsNamesWithNoDefaults, param)
}
for _, inputResourceParam := range paramSpecs {
if missingParamsNames.Has(inputResourceParam.Name) && inputResourceParam.Default == nil {
missingParamsNamesWithNoDefaults = append(missingParamsNamesWithNoDefaults, inputResourceParam.Name)
}
}
return missingParamsNamesWithNoDefaults
Expand Down
123 changes: 105 additions & 18 deletions pkg/reconciler/taskrun/validate_taskrun_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package taskrun

import (
"context"
"fmt"
"testing"

metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
Expand All @@ -44,9 +45,8 @@ func TestValidateResolvedTask_ValidParams(t *testing.T) {
{
Name: "bar",
Type: v1beta1.ParamTypeString,
},
{
Name: "zoo",
}, {
Name: "include",
Type: v1beta1.ParamTypeString,
}, {
Name: "arrayResultRef",
Expand Down Expand Up @@ -81,7 +81,7 @@ func TestValidateResolvedTask_ValidParams(t *testing.T) {
rtr := &resources.ResolvedTask{
TaskSpec: &task.Spec,
}
p := []v1beta1.Param{{
p := v1beta1.Params{{
Name: "foo",
Value: *v1beta1.NewStructuredValues("somethinggood"),
}, {
Expand All @@ -104,14 +104,17 @@ func TestValidateResolvedTask_ValidParams(t *testing.T) {
"key3": "val3",
}),
}}
m := []v1beta1.Param{{
Name: "zoo",
Value: *v1beta1.NewStructuredValues("a", "b", "c"),
}}
if err := ValidateResolvedTask(ctx, p, &v1beta1.Matrix{Params: m}, rtr); err != nil {
m := &v1beta1.Matrix{
Include: []v1beta1.IncludeParams{{
Name: "build-1",
Params: v1beta1.Params{{
Name: "include", Value: v1beta1.ParamValue{Type: v1beta1.ParamTypeString, StringVal: "string-1"},
}},
}},
}
if err := ValidateResolvedTask(ctx, p, m, rtr); err != nil {
t.Fatalf("Did not expect to see error when validating TaskRun with correct params but saw %v", err)
}

t.Run("alpha-extra-params", func(t *testing.T) {
ctx := config.ToContext(ctx, &config.Config{FeatureFlags: &config.FeatureFlags{EnableAPIFields: "alpha"}})
extra := v1beta1.Param{
Expand All @@ -122,8 +125,11 @@ func TestValidateResolvedTask_ValidParams(t *testing.T) {
Name: "extraarray",
Value: *v1beta1.NewStructuredValues("i", "am", "an", "extra", "array", "param"),
}
if err := ValidateResolvedTask(ctx, append(p, extra), &v1beta1.Matrix{Params: append(m, extraarray)}, rtr); err != nil {
t.Fatalf("Did not expect to see error when validating TaskRun with correct params but saw %v", err)
for _, include := range m.Include {
include.Params = append(include.Params, extraarray)
}
if err := ValidateResolvedTask(ctx, append(p, extra), m, rtr); err != nil {
t.Fatalf("Did not expect to see error when validating TaskRun with extra params but saw %v", err)
}
})
}
Expand Down Expand Up @@ -182,12 +188,12 @@ func TestValidateResolvedTask_InvalidParams(t *testing.T) {
rtr: &resources.ResolvedTask{
TaskSpec: &task.Spec,
},
params: []v1beta1.Param{{
params: v1beta1.Params{{
Name: "foobar",
Value: *v1beta1.NewStructuredValues("somethingfun"),
}},
matrix: &v1beta1.Matrix{
Params: []v1beta1.Param{{
Params: v1beta1.Params{{
Name: "barfoo",
Value: *v1beta1.NewStructuredValues("bar", "foo"),
}},
Expand All @@ -197,26 +203,50 @@ func TestValidateResolvedTask_InvalidParams(t *testing.T) {
rtr: &resources.ResolvedTask{
TaskSpec: &task.Spec,
},
params: []v1beta1.Param{{
params: v1beta1.Params{{
Name: "foo",
Value: *v1beta1.NewStructuredValues("bar", "foo"),
}},
}, {
name: "invalid-string-in-matrix-params",
rtr: &resources.ResolvedTask{
TaskSpec: &task.Spec,
},
matrix: &v1beta1.Matrix{
Params: v1beta1.Params{{
Name: "foo",
Value: *v1beta1.NewStructuredValues("bar"),
}}},
}, {
name: "invalid-type-in-matrix",
rtr: &resources.ResolvedTask{
TaskSpec: &task.Spec,
},
matrix: &v1beta1.Matrix{
Params: []v1beta1.Param{{
Name: "bar",
Params: v1beta1.Params{{
Name: "foo",
Value: *v1beta1.NewStructuredValues("bar", "foo"),
}}},
}, {
name: "invalid-arr-in-matrix-include",
rtr: &resources.ResolvedTask{
TaskSpec: &task.Spec,
},
matrix: &v1beta1.Matrix{
Include: []v1beta1.IncludeParams{{
Name: "build-1",
Params: v1beta1.Params{{
Name: "bar",
Value: *v1beta1.NewStructuredValues("bar", "foo"),
}},
}},
},
}, {
name: "missing object param keys",
rtr: &resources.ResolvedTask{
TaskSpec: &task.Spec,
},
params: []v1beta1.Param{{
params: v1beta1.Params{{
Name: "foo",
Value: *v1beta1.NewStructuredValues("test"),
}, {
Expand Down Expand Up @@ -245,6 +275,63 @@ func TestValidateResolvedTask_InvalidParams(t *testing.T) {
}
}

func TestValidateResolvedTask_InValidExtraParams(t *testing.T) {
ctx := context.Background()
task := &v1beta1.Task{
ObjectMeta: metav1.ObjectMeta{Name: "foo"},
Spec: v1beta1.TaskSpec{
Steps: []v1beta1.Step{{
Image: "myimage",
Command: []string{"mycmd"},
}},
Params: []v1beta1.ParamSpec{
{
Name: "bar",
Type: v1beta1.ParamTypeArray,
}, {
Name: "include",
Type: v1beta1.ParamTypeString,
},
},
},
}
rtr := &resources.ResolvedTask{
TaskSpec: &task.Spec,
}
p := v1beta1.Params{{
Name: "bar",
Value: *v1beta1.NewStructuredValues("somethinggood"),
}}
m := &v1beta1.Matrix{
Include: []v1beta1.IncludeParams{{
Name: "build-1",
Params: v1beta1.Params{{
Name: "include", Value: v1beta1.ParamValue{Type: v1beta1.ParamTypeString, StringVal: "string-1"},
}},
}},
}
if err := ValidateResolvedTask(ctx, p, m, rtr); err == nil {
t.Errorf("Expected to see error when validating invalid resolved TaskRun with wrong params but saw none")
}
t.Run("alpha-extra-params", func(t *testing.T) {
extra := v1beta1.Param{
Name: "extrastr",
Value: *v1beta1.NewStructuredValues("extra"),
}
extraarray := v1beta1.Param{
Name: "extraarray",
Value: *v1beta1.NewStructuredValues("i", "am", "an", "extra", "array", "param"),
}
for _, include := range m.Include {
include.Params = append(include.Params, extraarray)
}
if err := ValidateResolvedTask(ctx, append(p, extra), m, rtr); err == nil {
t.Errorf("Expected to see error when validating invalid resolved TaskRun with wrong params but saw none")
}

})
}

func TestValidateOverrides(t *testing.T) {
tcs := []struct {
name string
Expand Down

0 comments on commit dc39b2b

Please sign in to comment.