TEP-0090: Include Matrix in Validation of Parameters in TaskRun…

… Reconciler

The `TaskRun` reconciler validates that the needed `Parameters` are provided,
no extra `Parameters` are provide and the types of `Parameters` are matching.

Prior to this change, the reconciler did the above validation considering the
`params` field in the `PipelineTask` only. In this change, we include `matrix`
field into the validation routine described above. This includes validating
that `Parameters` in the `Matrix` field are `Arrays` only, and that they are
substituting `Parameters` of type `String` in the underlying `Task`.

Related refactors were done in #4841.

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())

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)

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,26 @@ func wrongTypeParamsNames(params []v1beta1.Param, neededParamsTypes map[string]v
  wrongTypeParamNames = append(wrongTypeParamNames, param.Name)
  }
  }
+ for _, param := range matrix {
+ if param.Value.Type != v1beta1.ParamTypeArray {
+ wrongTypeParamNames = append(wrongTypeParamNames, param.Name)
+ continue
+ }
+ 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{}

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)
  }
  })
@@ -188,6 +196,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 +206,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: "foo",
+ Value: *v1beta1.NewArrayOrString("bar"),
+ }},
  }}
  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 +439,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")
  }
  })