diff --git a/pkg/reconciler/pipelinerun/pipelinerun.go b/pkg/reconciler/pipelinerun/pipelinerun.go index e8947b9b0a7..62d4356c57f 100644 --- a/pkg/reconciler/pipelinerun/pipelinerun.go +++ b/pkg/reconciler/pipelinerun/pipelinerun.go @@ -482,7 +482,7 @@ func (c *Reconciler) reconcile(ctx context.Context, pr *v1beta1.PipelineRun, get for _, rprt := range pipelineRunFacts.State { if !rprt.IsCustomTask() { - err := taskrun.ValidateResolvedTaskResources(ctx, rprt.PipelineTask.Params, rprt.ResolvedTaskResources) + err := taskrun.ValidateResolvedTaskResources(ctx, rprt.PipelineTask.Params, rprt.PipelineTask.Matrix, rprt.ResolvedTaskResources) if err != nil { logger.Errorf("Failed to validate pipelinerun %q with error %v", pr.Name, err) pr.Status.MarkFailed(ReasonFailedValidation, err.Error()) diff --git a/pkg/reconciler/taskrun/taskrun.go b/pkg/reconciler/taskrun/taskrun.go index d1bea6bb0cc..119959f8ec5 100644 --- a/pkg/reconciler/taskrun/taskrun.go +++ b/pkg/reconciler/taskrun/taskrun.go @@ -332,7 +332,7 @@ func (c *Reconciler) prepare(ctx context.Context, tr *v1beta1.TaskRun) (*v1beta1 return nil, nil, controller.NewPermanentError(err) } - if err := ValidateResolvedTaskResources(ctx, tr.Spec.Params, rtr); err != nil { + if err := ValidateResolvedTaskResources(ctx, tr.Spec.Params, []v1beta1.Param{}, rtr); err != nil { logger.Errorf("TaskRun %q resources are invalid: %v", tr.Name, err) tr.Status.MarkResourceFailed(podconvert.ReasonFailedValidation, err) return nil, nil, controller.NewPermanentError(err) diff --git a/pkg/reconciler/taskrun/validate_resources.go b/pkg/reconciler/taskrun/validate_resources.go index 68fbc367b87..c16e48b8280 100644 --- a/pkg/reconciler/taskrun/validate_resources.go +++ b/pkg/reconciler/taskrun/validate_resources.go @@ -69,16 +69,16 @@ func validateResources(requiredResources []v1beta1.TaskResource, providedResourc return nil } -func validateParams(ctx context.Context, paramSpecs []v1beta1.ParamSpec, params []v1beta1.Param) error { +func validateParams(ctx context.Context, paramSpecs []v1beta1.ParamSpec, params []v1beta1.Param, matrix []v1beta1.Param) error { neededParamsNames, neededParamsTypes := neededParamsNamesAndTypes(paramSpecs) - providedParamsNames := providedParamsNames(params) + providedParamsNames := providedParamsNames(append(params, matrix...)) 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 extraParamsNames := extraParamsNames(ctx, neededParamsNames, providedParamsNames); len(extraParamsNames) != 0 { return fmt.Errorf("didn't need these params but they were provided anyway: %s", extraParamsNames) } - if wrongTypeParamNames := wrongTypeParamsNames(params, neededParamsTypes); len(wrongTypeParamNames) != 0 { + if wrongTypeParamNames := wrongTypeParamsNames(params, matrix, neededParamsTypes); len(wrongTypeParamNames) != 0 { return fmt.Errorf("param types don't match the user-specified type: %s", wrongTypeParamNames) } @@ -129,7 +129,7 @@ func extraParamsNames(ctx context.Context, neededParams []string, providedParams return nil } -func wrongTypeParamsNames(params []v1beta1.Param, neededParamsTypes map[string]v1beta1.ParamType) []string { +func wrongTypeParamsNames(params []v1beta1.Param, matrix []v1beta1.Param, neededParamsTypes map[string]v1beta1.ParamType) []string { var wrongTypeParamNames []string for _, param := range params { if _, ok := neededParamsTypes[param.Name]; !ok { @@ -141,12 +141,22 @@ func wrongTypeParamsNames(params []v1beta1.Param, neededParamsTypes map[string]v wrongTypeParamNames = append(wrongTypeParamNames, param.Name) } } + for _, param := range matrix { + if _, ok := neededParamsTypes[param.Name]; !ok { + // Ignore any missing params - this happens when extra params were + // passed to the task that aren't being used. + continue + } + if neededParamsTypes[param.Name] != v1beta1.ParamTypeString { + wrongTypeParamNames = append(wrongTypeParamNames, param.Name) + } + } return wrongTypeParamNames } // ValidateResolvedTaskResources validates task inputs, params and output matches taskrun -func ValidateResolvedTaskResources(ctx context.Context, params []v1beta1.Param, rtr *resources.ResolvedTaskResources) error { - if err := validateParams(ctx, rtr.TaskSpec.Params, params); err != nil { +func ValidateResolvedTaskResources(ctx context.Context, params []v1beta1.Param, matrix []v1beta1.Param, rtr *resources.ResolvedTaskResources) error { + if err := validateParams(ctx, rtr.TaskSpec.Params, params, matrix); err != nil { return fmt.Errorf("invalid input params for task %s: %w", rtr.TaskName, err) } inputs := []v1beta1.TaskResource{} diff --git a/pkg/reconciler/taskrun/validate_resources_test.go b/pkg/reconciler/taskrun/validate_resources_test.go index 685efb3c42f..fb7d36654e2 100644 --- a/pkg/reconciler/taskrun/validate_resources_test.go +++ b/pkg/reconciler/taskrun/validate_resources_test.go @@ -115,7 +115,7 @@ func TestValidateResolvedTaskResources_ValidResources(t *testing.T) { }, }, } - if err := ValidateResolvedTaskResources(ctx, []v1beta1.Param{}, rtr); err != nil { + if err := ValidateResolvedTaskResources(ctx, []v1beta1.Param{}, []v1beta1.Param{}, rtr); err != nil { t.Fatalf("Did not expect to see error when validating valid resolved TaskRun but saw %v", err) } } @@ -138,6 +138,10 @@ func TestValidateResolvedTaskResources_ValidParams(t *testing.T) { Name: "bar", Type: v1beta1.ParamTypeString, }, + { + Name: "zoo", + Type: v1beta1.ParamTypeString, + }, }, }, } @@ -151,7 +155,11 @@ func TestValidateResolvedTaskResources_ValidParams(t *testing.T) { Name: "bar", Value: *v1beta1.NewArrayOrString("somethinggood"), }} - if err := ValidateResolvedTaskResources(ctx, p, rtr); err != nil { + m := []v1beta1.Param{{ + Name: "zoo", + Value: *v1beta1.NewArrayOrString("a", "b", "c"), + }} + if err := ValidateResolvedTaskResources(ctx, p, m, rtr); err != nil { t.Fatalf("Did not expect to see error when validating TaskRun with correct params but saw %v", err) } @@ -161,7 +169,7 @@ func TestValidateResolvedTaskResources_ValidParams(t *testing.T) { Name: "extra", Value: *v1beta1.NewArrayOrString("i am an extra param"), } - if err := ValidateResolvedTaskResources(ctx, append(p, extra), rtr); err != nil { + if err := ValidateResolvedTaskResources(ctx, append(p, extra), m, rtr); err != nil { t.Fatalf("Did not expect to see error when validating TaskRun with correct params but saw %v", err) } }) @@ -180,6 +188,9 @@ func TestValidateResolvedTaskResources_InvalidParams(t *testing.T) { { Name: "foo", Type: v1beta1.ParamTypeString, + }, { + Name: "bar", + Type: v1beta1.ParamTypeArray, }, }, }, @@ -188,6 +199,7 @@ func TestValidateResolvedTaskResources_InvalidParams(t *testing.T) { name string rtr *resources.ResolvedTaskResources params []v1beta1.Param + matrix []v1beta1.Param }{{ name: "missing-params", rtr: &resources.ResolvedTaskResources{ @@ -197,10 +209,32 @@ func TestValidateResolvedTaskResources_InvalidParams(t *testing.T) { Name: "foobar", Value: *v1beta1.NewArrayOrString("somethingfun"), }}, + matrix: []v1beta1.Param{{ + Name: "barfoo", + Value: *v1beta1.NewArrayOrString("bar", "foo"), + }}, + }, { + name: "invalid-type-in-params", + rtr: &resources.ResolvedTaskResources{ + TaskSpec: &task.Spec, + }, + params: []v1beta1.Param{{ + Name: "foo", + Value: *v1beta1.NewArrayOrString("bar", "foo"), + }}, + }, { + name: "invalid-type-in-matrix", + rtr: &resources.ResolvedTaskResources{ + TaskSpec: &task.Spec, + }, + matrix: []v1beta1.Param{{ + Name: "bar", + Value: *v1beta1.NewArrayOrString("bar", "foo"), + }}, }} for _, tc := range tcs { t.Run(tc.name, func(t *testing.T) { - if err := ValidateResolvedTaskResources(ctx, tc.params, tc.rtr); err == nil { + if err := ValidateResolvedTaskResources(ctx, tc.params, tc.matrix, tc.rtr); err == nil { t.Errorf("Expected to see error when validating invalid resolved TaskRun with wrong params but saw none") } }) @@ -408,7 +442,7 @@ func TestValidateResolvedTaskResources_InvalidResources(t *testing.T) { for _, tc := range tcs { t.Run(tc.name, func(t *testing.T) { - if err := ValidateResolvedTaskResources(ctx, []v1beta1.Param{}, tc.rtr); err == nil { + if err := ValidateResolvedTaskResources(ctx, []v1beta1.Param{}, []v1beta1.Param{}, tc.rtr); err == nil { t.Errorf("Expected to see error when validating invalid resolved TaskRun but saw none") } })