diff --git a/pkg/reconciler/pipelinerun/pipelinerun.go b/pkg/reconciler/pipelinerun/pipelinerun.go index 0059a099647..284531fa386 100644 --- a/pkg/reconciler/pipelinerun/pipelinerun.go +++ b/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,8 +428,7 @@ 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", @@ -434,6 +436,14 @@ func (c *Reconciler) reconcile(ctx context.Context, pr *v1beta1.PipelineRun, get 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, diff --git a/pkg/reconciler/pipelinerun/pipelinerun_test.go b/pkg/reconciler/pipelinerun/pipelinerun_test.go index b8aaf16f67c..5c4d2fa3b6a 100644 --- a/pkg/reconciler/pipelinerun/pipelinerun_test.go +++ b/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, ` diff --git a/pkg/reconciler/pipelinerun/resources/validate_params.go b/pkg/reconciler/pipelinerun/resources/validate_params.go index febfe3ee8e3..03b84056503 100644 --- a/pkg/reconciler/pipelinerun/resources/validate_params.go +++ b/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 +} diff --git a/pkg/reconciler/taskrun/validate_resources.go b/pkg/reconciler/taskrun/validate_resources.go index 96d3a3589bb..347138ca0c9 100644 --- a/pkg/reconciler/taskrun/validate_resources.go +++ b/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)