TEP-0075: Validate object keys, PipelineRunSpec -> PipelineSpec

Add validation that checks if object param value from PipelineRunSpec
misses some keys required for that object param declared in PipelineSpec.

pkg/reconciler/pipelinerun/pipelinerun.go

@@ -82,6 +82,9 @@ const (
 	// parameter(s) declared in the PipelineRun do not have the some declared type as the
 	// parameters(s) declared in the Pipeline that they are supposed to override.
 	ReasonParameterTypeMismatch = "ParameterTypeMismatch"
+	// ReasonObjectParameterMissKeys indicates that the object param value provided from PipelineRun spec
+	// misses some keys required for the object param declared in Pipeline spec.
+	ReasonObjectParameterMissKeys = "ObjectParameterMissKeys"
 	// ReasonCouldntGetTask indicates that the reason for the failure status is that the
 	// associated Pipeline's Tasks couldn't all be retrieved
 	ReasonCouldntGetTask = "CouldntGetTask"
@@ -425,15 +428,22 @@ func (c *Reconciler) reconcile(ctx context.Context, pr *v1beta1.PipelineRun, get
 
 	// Ensure that the parameters from the PipelineRun are overriding Pipeline parameters with the same type.
 	// Weird substitution issues can occur if this is not validated (ApplyParameters() does not verify type).
-	err = resources.ValidateParamTypesMatching(pipelineSpec, pr)
-	if err != nil {
+	if err = resources.ValidateParamTypesMatching(pipelineSpec, pr); err != nil {
 		// This Run has failed, so we need to mark it as failed and stop reconciling it
 		pr.Status.MarkFailed(ReasonParameterTypeMismatch,
 			"PipelineRun %s/%s parameters have mismatching types with Pipeline %s/%s's parameters: %s",
 			pr.Namespace, pr.Name, pr.Namespace, pipelineMeta.Name, err)
 		return controller.NewPermanentError(err)
 	}
 
+	if err = resources.ValidateObjectParamRequiredKeys(pipelineSpec.Params, pr.Spec.Params); err != nil {
+		// This Run has failed, so we need to mark it as failed and stop reconciling it
+		pr.Status.MarkFailed(ReasonObjectParameterMissKeys,
+			"PipelineRun %s/%s parameters is missing some object keys required by Pipeline %s/%s's parameters: %s",
+			pr.Namespace, pr.Name, pr.Namespace, pipelineMeta.Name, err)
+		return controller.NewPermanentError(err)
+	}
+
 	// Ensure that the workspaces expected by the Pipeline are provided by the PipelineRun.
 	if err := resources.ValidateWorkspaceBindings(pipelineSpec, pr); err != nil {
 		pr.Status.MarkFailed(ReasonInvalidWorkspaceBinding,

pkg/reconciler/pipelinerun/pipelinerun_test.go

@@ -913,6 +913,18 @@ spec:
       type: %s
 `, v1beta1.ParamTypeArray)),
 		parse.MustParseTask(t, fmt.Sprintf(`
+metadata:
+  name: a-task-that-needs-object-params
+  namespace: foo
+spec:
+  params:
+    - name: some-param
+      type: %s
+      properties:
+        key1: {}
+        key2: {}
+`, v1beta1.ParamTypeObject)),
+		parse.MustParseTask(t, fmt.Sprintf(`
 metadata:
   name: a-task-that-needs-a-resource
   namespace: foo
@@ -988,6 +1000,22 @@ spec:
       taskRef:
         name: a-task-that-needs-array-params
 `, v1beta1.ParamTypeArray)),
+		parse.MustParsePipeline(t, fmt.Sprintf(`
+metadata:
+  name: a-pipeline-with-object-params
+  namespace: foo
+spec:
+  params:
+    - name: some-param
+      type: %s
+      properties:
+        key1: {}
+        key2: {}
+  tasks:
+    - name: some-task
+      taskRef:
+        name: a-task-that-needs-object-params
+`, v1beta1.ParamTypeObject)),
 		parse.MustParsePipeline(t, `
 metadata:
   name: a-pipeline-with-missing-conditions
@@ -1129,6 +1157,26 @@ spec:
 			"Normal Started",
 			"Warning Failed PipelineRun foo/pipeline-mismatching-param-type parameters have mismatching types",
 		},
+	}, {
+		name: "invalid-pipeline-missing-object-keys",
+		pipelineRun: parse.MustParsePipelineRun(t, `
+metadata:
+  name: pipeline-missing-object-param-keys
+  namespace: foo
+spec:
+  pipelineRef:
+    name: a-pipeline-with-object-params
+  params:
+    - name: some-param
+      value:
+        key1: "a"
+`),
+		reason:         ReasonObjectParameterMissKeys,
+		permanentError: true,
+		wantEvents: []string{
+			"Normal Started",
+			"Warning Failed PipelineRun foo/pipeline-missing-object-param-keys parameters is missing some object keys",
+		},
 	}, {
 		name: "invalid-pipeline-missing-conditions-shd-stop-reconciling",
 		pipelineRun: parse.MustParsePipelineRun(t, `

pkg/reconciler/pipelinerun/resources/validate_params.go

@@ -21,6 +21,7 @@ import (
 
 	"github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1"
 	"github.com/tektoncd/pipeline/pkg/list"
+	"github.com/tektoncd/pipeline/pkg/reconciler/taskrun"
 )
 
 // ValidateParamTypesMatching validate that parameters in PipelineRun override corresponding parameters in Pipeline of the same type.
@@ -73,3 +74,12 @@ func ValidateRequiredParametersProvided(pipelineParameters *[]v1beta1.ParamSpec,
 	}
 	return nil
 }
+
+func ValidateObjectParamRequiredKeys(pipelineParameters []v1beta1.ParamSpec, pipelineRunParameters []v1beta1.Param) error {
+	missings := taskrun.MissingKeysObjectParamNames(pipelineParameters, pipelineRunParameters)
+	if len(missings) != 0 {
+		return fmt.Errorf("PipelineRun missing object keys for parameters: %v", missings)
+	}
+
+	return nil
+}

pkg/reconciler/taskrun/validate_resources.go

@@ -81,7 +81,7 @@ func validateParams(ctx context.Context, paramSpecs []v1beta1.ParamSpec, params
 	if wrongTypeParamNames := wrongTypeParamsNames(params, matrix, 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 {
+	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)
 	}
 
@@ -157,9 +157,9 @@ func wrongTypeParamsNames(params []v1beta1.Param, matrix []v1beta1.Param, needed
 	return wrongTypeParamNames
 }
 
-// missingKeysObjectParamNames checks if all required keys of object type params are provided in taskrun params.
+// MissingKeysObjectParamNames checks if all required keys of object type params are provided in taskrun params.
 // TODO (@chuangw6): This might be refactored out to support single pair validation.
-func missingKeysObjectParamNames(paramSpecs []v1beta1.ParamSpec, params []v1beta1.Param) []string {
+func MissingKeysObjectParamNames(paramSpecs []v1beta1.ParamSpec, params []v1beta1.Param) []string {
 	neededKeys := make(map[string][]string)
 	providedKeys := make(map[string][]string)