From bf005e5ba6fec2d9384a8b938b79a7bf21843d08 Mon Sep 17 00:00:00 2001 From: Emma Munley Date: Fri, 10 Mar 2023 12:46:30 -0500 Subject: [PATCH] TEP-0118: Update TaskRun Validation for Matrix Include Params This commit updates TaskRun validation to account for Matrix Include Params. Note: This feature is still in preview mode. --- pkg/reconciler/taskrun/validate_taskrun.go | 43 +++----- .../taskrun/validate_taskrun_test.go | 99 +++++++++++++++---- 2 files changed, 95 insertions(+), 47 deletions(-) diff --git a/pkg/reconciler/taskrun/validate_taskrun.go b/pkg/reconciler/taskrun/validate_taskrun.go index a44418ca41a..bbfdda29d7e 100644 --- a/pkg/reconciler/taskrun/validate_taskrun.go +++ b/pkg/reconciler/taskrun/validate_taskrun.go @@ -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 diff --git a/pkg/reconciler/taskrun/validate_taskrun_test.go b/pkg/reconciler/taskrun/validate_taskrun_test.go index 79d0943e434..dcba5938338 100644 --- a/pkg/reconciler/taskrun/validate_taskrun_test.go +++ b/pkg/reconciler/taskrun/validate_taskrun_test.go @@ -18,6 +18,7 @@ package taskrun import ( "context" + "fmt" "testing" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -44,9 +45,8 @@ func TestValidateResolvedTask_ValidParams(t *testing.T) { { Name: "bar", Type: v1beta1.ParamTypeString, - }, - { - Name: "zoo", + }, { + Name: "include", Type: v1beta1.ParamTypeString, }, { Name: "arrayResultRef", @@ -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"), }, { @@ -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{ @@ -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) } }) } @@ -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"), }}, @@ -197,7 +203,7 @@ 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"), }}, @@ -207,8 +213,8 @@ func TestValidateResolvedTask_InvalidParams(t *testing.T) { TaskSpec: &task.Spec, }, matrix: &v1beta1.Matrix{ - Params: []v1beta1.Param{{ - Name: "bar", + Params: v1beta1.Params{{ + Name: "foo", Value: *v1beta1.NewStructuredValues("bar", "foo"), }}}, }, { @@ -216,7 +222,7 @@ func TestValidateResolvedTask_InvalidParams(t *testing.T) { rtr: &resources.ResolvedTask{ TaskSpec: &task.Spec, }, - params: []v1beta1.Param{{ + params: v1beta1.Params{{ Name: "foo", Value: *v1beta1.NewStructuredValues("test"), }, { @@ -245,6 +251,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