add support for reserved default object param

This commit adds support for reserved default object param.Users can reference to default object param values without needing to declaring them in remote Task&Pipeline. Validation err will be returned if referencing to non-existent object key. Signed-off-by: Yongxuan Zhang [email protected]
Yongxuanzhang · Dec 12, 2023 · 1f81f21 · 1f81f21
1 parent 7c040de
commit 1f81f21
Show file tree

Hide file tree

Showing 12 changed files with 528 additions and 12 deletions.
diff --git a/examples/v1/pipelineruns/alpha/remote-pipeline-ref-reserved-param.yaml b/examples/v1/pipelineruns/alpha/remote-pipeline-ref-reserved-param.yaml
@@ -0,0 +1,46 @@
+apiVersion: tekton.dev/v1
+kind: Pipeline
+metadata:
+  name: echo-message-pipeline
+spec:
+  params:
+    - name: hello
+      properties:
+        project_id:
+          type: string
+      type: object
+  tasks:
+    - name: task1
+      params:
+      - name: foo
+        value:
+          project_id: $(params.hello.project_id)
+      taskSpec:
+        params:
+          - name: foo
+            type: object
+            properties:
+              project_id:
+                type: string
+        steps:
+          - name: echo
+            image: ubuntu
+            script: |
+              #!/bin/bash
+              echo $(params.TBD.project_id)
+              echo $(params.foo.project_id)
+---
+kind: PipelineRun
+apiVersion: tekton.dev/v1
+metadata:
+  name: test-pipelinerun
+spec:
+  params:
+    - name: TBD
+      value:
+        project_id: foo
+    - name: hello
+      value:
+        project_id: foo
+  pipelineRef:
+    name: echo-message-pipeline
diff --git a/examples/v1/taskruns/alpha/remote-task-ref-reserved-param.yaml b/examples/v1/taskruns/alpha/remote-task-ref-reserved-param.yaml
@@ -0,0 +1,37 @@
+kind: Task
+apiVersion: tekton.dev/v1
+metadata:
+  name: example-task
+spec:
+  params:
+    - name: foo
+      type: object
+      properties:
+        project_id:
+          type: string
+        digest:
+          type: string
+  steps:
+    - image: ubuntu
+      name: echo
+      script: |
+        echo -n $(params.foo.project_id)
+        echo -n $(params.TBD.project_id)
+---
+kind: TaskRun
+apiVersion: tekton.dev/v1
+metadata:
+  name: example-tr
+spec:
+  params:
+    - name: foo
+      value:
+        project_id: foo
+        digest: bar
+    - name: TBD
+      value:
+        project_id: foo
+        digest: bar
+  taskRef:
+    name: example-task
+    kind: task
diff --git a/pkg/apis/pipeline/v1/pipeline_validation.go b/pkg/apis/pipeline/v1/pipeline_validation.go
@@ -37,10 +37,11 @@ var _ apis.Validatable = (*Pipeline)(nil)
 var _ resourcesemantics.VerbLimited = (*Pipeline)(nil)
 
 const (
-	taskRef      = "taskRef"
-	taskSpec     = "taskSpec"
-	pipelineRef  = "pipelineRef"
-	pipelineSpec = "pipelineSpec"
+	taskRef           = "taskRef"
+	taskSpec          = "taskSpec"
+	pipelineRef       = "pipelineRef"
+	pipelineSpec      = "pipelineSpec"
+	ReservedParamName = "TBD"
 )
 
 // SupportedVerbs returns the operations that validation should be called for
@@ -395,6 +396,7 @@ func (ps *PipelineSpec) validatePipelineParameterUsage(ctx context.Context) (err
 // validatePipelineTaskParameterUsage validates that parameters referenced in the Pipeline Tasks are declared by the Pipeline
 func validatePipelineTaskParameterUsage(tasks []PipelineTask, params ParamSpecs) (errs *apis.FieldError) {
 	allParamNames := sets.NewString(params.GetNames()...)
+	allParamNames.Insert(ReservedParamName)
 	_, arrayParams, objectParams := params.SortByType()
 	arrayParamNames := sets.NewString(arrayParams.GetNames()...)
 	objectParameterNameKeys := map[string][]string{}

diff --git a/pkg/apis/pipeline/v1/pipeline_validation_test.go b/pkg/apis/pipeline/v1/pipeline_validation_test.go
@@ -18,6 +18,7 @@ package v1
 
 import (
 	"context"
+	"fmt"
 	"testing"
 
 	"github.com/google/go-cmp/cmp"
@@ -114,6 +115,24 @@ func TestPipeline_Validate_Success(t *testing.T) {
 				},
 			},
 		},
+	}, {
+		name: "valid reserved param",
+		wc:   cfgtesting.EnableAlphaAPIFields,
+		p: &Pipeline{
+			ObjectMeta: metav1.ObjectMeta{Name: "pipeline"},
+			Spec: PipelineSpec{
+				Tasks: []PipelineTask{{
+					Name: "pipeline-task-use-reserved-param",
+					TaskSpec: &EmbeddedTask{TaskSpec: TaskSpec{
+						Steps: []Step{{
+							Name:   "some-step",
+							Image:  "some-image",
+							Script: fmt.Sprintf("echo $(params.%s.foo)", ReservedParamName),
+						}},
+					}},
+				}},
+			},
+		},
 	}}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {

diff --git a/pkg/apis/pipeline/v1/pipelinerun_types.go b/pkg/apis/pipeline/v1/pipelinerun_types.go
@@ -414,6 +414,8 @@ const (
 	PipelineRunReasonCELEvaluationFailed PipelineRunReason = "CELEvaluationFailed"
 	// PipelineRunReasonInvalidParamValue indicates that the PipelineRun Param input value is not allowed.
 	PipelineRunReasonInvalidParamValue PipelineRunReason = "InvalidParamValue"
+	// PipelineRunReasonParamKeyNotExistent indicates that the default object param doesn't have the key which the param reference requires
+	PipelineRunReasonParamKeyNotExistent PipelineRunReason = "ParamKeyNotExistent"
 )
 
 // PipelineTaskOnErrorAnnotation is used to pass the failure strategy to TaskRun pods from PipelineTask OnError field

diff --git a/pkg/apis/pipeline/v1/task_validation.go b/pkg/apis/pipeline/v1/task_validation.go
@@ -132,6 +132,7 @@ func ValidateUsageOfDeclaredParameters(ctx context.Context, steps []Step, params
 	var errs *apis.FieldError
 	_, _, objectParams := params.SortByType()
 	allParameterNames := sets.NewString(params.GetNames()...)
+	allParameterNames.Insert(ReservedParamName)
 	errs = errs.Also(validateVariables(ctx, steps, "params", allParameterNames))
 	errs = errs.Also(validateObjectUsage(ctx, steps, objectParams))
 	errs = errs.Also(ValidateObjectParamsHaveProperties(ctx, params))
@@ -643,6 +644,27 @@ func validateVariables(ctx context.Context, steps []Step, prefix string, vars se
 	return errs
 }
 
+// Reserved Param doesn't have properties, so we need to check all the references if they are using a non-existent key from the param
+func ValidateReservedParamReferenceMissingKeys(ctx context.Context, steps []Step, params Params) error {
+	objectKeys := sets.NewString()
+	// collect all the existent keys from the object reserved param.
+	for _, p := range params {
+		if p.Name == ReservedParamName && p.Value.Type == ParamTypeObject {
+			for key := range p.Value.ObjectVal {
+				objectKeys.Insert(key)
+			}
+		}
+	}
+	if len(objectKeys) > 0 {
+		// check if the object's key names are referenced correctly i.e. param.objectParam.key1
+		err := validateVariables(ctx, steps, fmt.Sprintf("params\\.%s", ReservedParamName), objectKeys)
+		if err != nil {
+			return err
+		}
+	}
+	return nil
+}
+
 // ValidateNameFormat validates that the name format of all param types follows the rules
 func ValidateNameFormat(stringAndArrayParams sets.String, objectParams []ParamSpec) (errs *apis.FieldError) {
 	// checking string or array name format

diff --git a/pkg/apis/pipeline/v1/task_validation_test.go b/pkg/apis/pipeline/v1/task_validation_test.go
@@ -61,6 +61,19 @@ func TestTaskValidate(t *testing.T) {
 				}},
 			},
 		},
+	}, {
+		name: "valid reserved param",
+		wc:   cfgtesting.EnableAlphaAPIFields,
+		t: &v1.Task{
+			ObjectMeta: metav1.ObjectMeta{Name: "task"},
+			Spec: v1.TaskSpec{
+				Steps: []v1.Step{{
+					Name:   "some-step",
+					Image:  "some-image",
+					Script: fmt.Sprintf("echo $(params.%s.foo)", v1.ReservedParamName),
+				}},
+			},
+		},
 	}}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {

diff --git a/pkg/apis/pipeline/v1beta1/pipeline_validation.go b/pkg/apis/pipeline/v1beta1/pipeline_validation.go
@@ -38,10 +38,11 @@ var _ apis.Validatable = (*Pipeline)(nil)
 var _ resourcesemantics.VerbLimited = (*Pipeline)(nil)
 
 const (
-	taskRef      = "taskRef"
-	taskSpec     = "taskSpec"
-	pipelineRef  = "pipelineRef"
-	pipelineSpec = "pipelineSpec"
+	taskRef           = "taskRef"
+	taskSpec          = "taskSpec"
+	pipelineRef       = "pipelineRef"
+	pipelineSpec      = "pipelineSpec"
+	ReservedParamName = "TBD"
 )
 
 // SupportedVerbs returns the operations that validation should be called for
@@ -414,6 +415,7 @@ func (ps *PipelineSpec) validatePipelineParameterUsage(ctx context.Context) (err
 // validatePipelineTaskParameterUsage validates that parameters referenced in the Pipeline Tasks are declared by the Pipeline
 func validatePipelineTaskParameterUsage(tasks []PipelineTask, params ParamSpecs) (errs *apis.FieldError) {
 	allParamNames := sets.NewString(params.getNames()...)
+	allParamNames.Insert(ReservedParamName)
 	_, arrayParams, objectParams := params.sortByType()
 	arrayParamNames := sets.NewString(arrayParams.getNames()...)
 	objectParameterNameKeys := map[string][]string{}

diff --git a/pkg/reconciler/pipelinerun/pipelinerun.go b/pkg/reconciler/pipelinerun/pipelinerun.go
@@ -671,6 +671,20 @@ func (c *Reconciler) reconcile(ctx context.Context, pr *v1.PipelineRun, getPipel
 	}
 
 	if pipelineRunFacts.State.IsBeforeFirstTaskRun() {
+		// check if the object's key names are referenced correctly i.e. param.objectParam.key1
+		for _, prs := range pipelineRunState {
+			if prs.ResolvedTask == nil {
+				continue
+			}
+			if validateErr := v1.ValidateReservedParamReferenceMissingKeys(ctx, prs.ResolvedTask.TaskSpec.Steps, pr.Spec.Params); validateErr != nil {
+				logger.Errorf("PipelineRun %s params references error %v", pr.Name, validateErr)
+				pr.Status.MarkFailed(v1.PipelineRunReasonParamKeyNotExistent.String(),
+					"PipelineRun %s/%s doesn't has the references key: %s",
+					pr.Namespace, pr.Name, err)
+				return controller.NewPermanentError(validateErr)
+			}
+		}
+
 		if err := resources.ValidatePipelineTaskResults(pipelineRunFacts.State); err != nil {
 			logger.Errorf("Failed to resolve task result reference for %q with error %v", pr.Name, err)
 			pr.Status.MarkFailed(v1.PipelineRunReasonInvalidTaskResultReference.String(), err.Error())

diff --git a/pkg/reconciler/pipelinerun/pipelinerun_test.go b/pkg/reconciler/pipelinerun/pipelinerun_test.go
@@ -4627,18 +4627,18 @@ spec:
       steps:
         - name: s1
           image: alpine
-          script: | 
+          script: |
             echo $(params.version) + $(params.tag)
   - name: b-task
-    params: 
+    params:
     - name: ref-p1
       value: $(params.version)
     - name: ref-p2
       value: "v3"
     taskRef:
       name: ref-task
   - name: c-task-matrixed
-    matrix: 
+    matrix:
       params:
       - name: ref-p1
         value: [v1, v2]
@@ -4717,7 +4717,7 @@ spec:
       steps:
         - name: s1
           image: alpine
-          script: | 
+          script: |
             echo $(params.version)
 `)}
 	prs := []*v1.PipelineRun{parse.MustParseV1PipelineRun(t, `

diff --git a/pkg/reconciler/taskrun/taskrun.go b/pkg/reconciler/taskrun/taskrun.go
@@ -417,6 +417,12 @@ func (c *Reconciler) prepare(ctx context.Context, tr *v1.TaskRun) (*v1.TaskSpec,
 		Kind:     resources.GetTaskKind(tr),
 	}
 
+	if validateErr := v1.ValidateReservedParamReferenceMissingKeys(ctx, taskSpec.Steps, tr.Spec.Params); validateErr != nil {
+		logger.Errorf("TaskRun %s params references error %v", tr.Name, validateErr)
+		tr.Status.MarkResourceFailed(v1.TaskRunReason(v1.PipelineRunReasonParamKeyNotExistent.String()), validateErr)
+		return nil, nil, controller.NewPermanentError(validateErr)
+	}
+
 	if err := validateTaskSpecRequestResources(taskSpec); err != nil {
 		logger.Errorf("TaskRun %s taskSpec request resources are invalid: %v", tr.Name, err)
 		tr.Status.MarkResourceFailed(v1.TaskRunReasonFailedValidation, err)