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 yongxuanzhang@google.com
Yongxuanzhang · Jan 16, 2024 · d62e1e6 · d62e1e6
1 parent 416c35c
commit d62e1e6
Show file tree

Hide file tree

Showing 18 changed files with 817 additions and 35 deletions.
diff --git a/docs/pipeline-api.md b/docs/pipeline-api.md
@@ -2029,6 +2029,9 @@ misses some keys required for the object param declared in Pipeline spec.</p>
 <td><p>ReasonParamArrayIndexingInvalid indicates that the use of param array indexing is not under correct api fields feature gate
 or the array is out of bound.</p>
 </td>
+</tr><tr><td><p>&#34;ParamKeyNotExistent&#34;</p></td>
+<td><p>PipelineRunReasonParamKeyNotExistent indicates that the default object param doesn&rsquo;t have the key which the param reference requires</p>
+</td>
 </tr><tr><td><p>&#34;ParameterMissing&#34;</p></td>
 <td><p>ReasonParameterMissing indicates that the reason for the failure status is that the
 associated PipelineRun didn&rsquo;t provide all the required parameters</p>
@@ -5150,6 +5153,9 @@ TaskRuns failed due to reconciler/validation error should not use this reason.</
 </tr><tr><td><p>&#34;InvalidParamValue&#34;</p></td>
 <td><p>TaskRunReasonInvalidParamValue indicates that the TaskRun Param input value is not allowed.</p>
 </td>
+</tr><tr><td><p>&#34;ParamKeyNotExistent&#34;</p></td>
+<td><p>TaskRunReasonParamKeyNotExistent indicates that the default object param doesn&rsquo;t have the key which the param reference requires</p>
+</td>
 </tr><tr><td><p>&#34;ResourceVerificationFailed&#34;</p></td>
 <td><p>TaskRunReasonResourceVerificationFailed indicates that the task fails the trusted resource verification,
 it could be the content has changed, signature is invalid or public key is invalid</p>

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.provider.project_id)
+              echo $(params.foo.project_id)
+---
+kind: PipelineRun
+apiVersion: tekton.dev/v1
+metadata:
+  name: test-pipelinerun
+spec:
+  params:
+    - name: provider
+      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.provider.project_id)
+---
+kind: TaskRun
+apiVersion: tekton.dev/v1
+metadata:
+  name: example-tr
+spec:
+  params:
+    - name: foo
+      value:
+        project_id: foo
+        digest: bar
+    - name: provider
+      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 = "provider"
 )
 
 // SupportedVerbs returns the operations that validation should be called for
@@ -389,6 +390,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
@@ -417,6 +417,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
@@ -126,6 +126,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))
@@ -762,3 +763,26 @@ func ValidateStepResultsVariables(ctx context.Context, results []StepResult, scr
 	errs = errs.Also(substitution.ValidateNoReferencesToUnknownVariables(script, "results", resultsNames).ViaField("script"))
 	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)
+			}
+		}
+	}
+
+	// check if the object's key names are referenced correctly i.e. param.objectParam.key1
+	if len(objectKeys) > 0 {
+		err := validateVariables(ctx, steps, fmt.Sprintf("params\\.%s", ReservedParamName), objectKeys)
+		if err != nil {
+			return err
+		}
+	}
+
+	return nil
+}
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) {
@@ -2726,3 +2739,136 @@ func TestTaskSpecValidate_StepResults_Error(t *testing.T) {
 		})
 	}
 }
+
+func TestValidateReservedParamReferenceMissingKeys_Success(t *testing.T) {
+	tests := []struct {
+		name   string
+		steps  []v1.Step
+		params []v1.Param
+	}{{
+		name: "reference default param in all step fields",
+		steps: []v1.Step{{
+			Name:       fmt.Sprintf("echo $(params.%s.foo)", v1.ReservedParamName),
+			Image:      fmt.Sprintf("$(params.%s.foo)", v1.ReservedParamName),
+			Args:       []string{fmt.Sprintf("echo $(params.%s.foo)", v1.ReservedParamName)},
+			Command:    []string{fmt.Sprintf("echo $(params.%s.foo)", v1.ReservedParamName)},
+			WorkingDir: fmt.Sprintf("echo $(params.%s.foo)", v1.ReservedParamName),
+			VolumeMounts: []corev1.VolumeMount{{
+				Name: fmt.Sprintf("echo $(params.%s.foo)", v1.ReservedParamName),
+			}},
+			Script: fmt.Sprintf("echo $(params.%s.foo)", v1.ReservedParamName),
+		}},
+		params: []v1.Param{{
+			Name:  v1.ReservedParamName,
+			Value: *v1.NewObject(map[string]string{"foo": "bar"}),
+		}},
+	}, {
+		name: "don't check non-reserved param",
+		steps: []v1.Step{{
+			Name:       fmt.Sprintf("echo $(params.%s.foo)", "non-reserved"),
+			Image:      fmt.Sprintf("$(params.%s.foo)", "non-reserved"),
+			Args:       []string{fmt.Sprintf("echo $(params.%s.foo)", "non-reserved")},
+			Command:    []string{fmt.Sprintf("echo $(params.%s.foo)", "non-reserved")},
+			WorkingDir: fmt.Sprintf("echo $(params.%s.foo)", "non-reserved"),
+			VolumeMounts: []corev1.VolumeMount{{
+				Name: fmt.Sprintf("echo $(params.%s.foo)", "non-reserved"),
+			}},
+			Script: fmt.Sprintf("echo $(params.%s.foo)", "non-reserved"),
+		}},
+		params: []v1.Param{{
+			Name:  v1.ReservedParamName,
+			Value: *v1.NewObject(map[string]string{"foo": "bar"}),
+		}},
+	}}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			ctx := context.Background()
+			err := v1.ValidateReservedParamReferenceMissingKeys(ctx, tt.steps, tt.params)
+			if err != nil {
+				t.Errorf("ValidateReservedParamReferenceMissingKeys() = %v", err)
+			}
+		})
+	}
+}
+
+func TestValidateReservedParamReferenceMissingKeys_Error(t *testing.T) {
+	tests := []struct {
+		name   string
+		steps  []v1.Step
+		params []v1.Param
+	}{{
+		name: "reference non-existent key in name",
+		steps: []v1.Step{{
+			Name: fmt.Sprintf("echo $(params.%s.%s)", v1.ReservedParamName, "non-existent"),
+		}},
+		params: []v1.Param{{
+			Name:  v1.ReservedParamName,
+			Value: *v1.NewObject(map[string]string{"foo": "bar"}),
+		}},
+	}, {
+		name: "reference non-existent key in image",
+		steps: []v1.Step{{
+			Image: fmt.Sprintf("$(params.%s.%s)", v1.ReservedParamName, "non-existent"),
+		}},
+		params: []v1.Param{{
+			Name:  v1.ReservedParamName,
+			Value: *v1.NewObject(map[string]string{"foo": "bar"}),
+		}},
+	}, {
+		name: "reference non-existent key in args",
+		steps: []v1.Step{{
+			Args: []string{fmt.Sprintf("echo $(params.%s.%s)", v1.ReservedParamName, "non-existent")},
+		}},
+		params: []v1.Param{{
+			Name:  v1.ReservedParamName,
+			Value: *v1.NewObject(map[string]string{"foo": "bar"}),
+		}},
+	}, {
+		name: "reference non-existent key in command",
+		steps: []v1.Step{{
+			Command: []string{fmt.Sprintf("echo $(params.%s.%s)", v1.ReservedParamName, "non-existent")},
+		}},
+		params: []v1.Param{{
+			Name:  v1.ReservedParamName,
+			Value: *v1.NewObject(map[string]string{"foo": "bar"}),
+		}},
+	}, {
+		name: "reference non-existent key in workingdir",
+		steps: []v1.Step{{
+			WorkingDir: fmt.Sprintf("echo $(params.%s.%s)", v1.ReservedParamName, "non-existent"),
+		}},
+		params: []v1.Param{{
+			Name:  v1.ReservedParamName,
+			Value: *v1.NewObject(map[string]string{"foo": "bar"}),
+		}},
+	}, {
+		name: "reference non-existent key in volume",
+		steps: []v1.Step{{
+			VolumeMounts: []corev1.VolumeMount{{
+				Name: fmt.Sprintf("echo $(params.%s.%s)", v1.ReservedParamName, "non-existent"),
+			}},
+		}},
+		params: []v1.Param{{
+			Name:  v1.ReservedParamName,
+			Value: *v1.NewObject(map[string]string{"foo": "bar"}),
+		}},
+	}, {
+		name: "reference non-existent key in script",
+		steps: []v1.Step{{
+			Script: fmt.Sprintf("echo $(params.%s.%s)", v1.ReservedParamName, "non-existent"),
+		}},
+		params: []v1.Param{{
+			Name:  v1.ReservedParamName,
+			Value: *v1.NewObject(map[string]string{"foo": "bar"}),
+		}},
+	}}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			ctx := context.Background()
+			err := v1.ValidateReservedParamReferenceMissingKeys(ctx, tt.steps, tt.params)
+			if err == nil {
+				t.Errorf("expected error but got nil")
+			}
+		})
+	}
+}
diff --git a/pkg/apis/pipeline/v1/taskrun_types.go b/pkg/apis/pipeline/v1/taskrun_types.go
@@ -205,6 +205,8 @@ const (
 	// TaskRunReasonFailureIgnored is the reason set when the Taskrun has failed due to pod execution error and the failure is ignored for the owning PipelineRun.
 	// TaskRuns failed due to reconciler/validation error should not use this reason.
 	TaskRunReasonFailureIgnored TaskRunReason = "FailureIgnored"
+	// TaskRunReasonParamKeyNotExistent indicates that the default object param doesn't have the key which the param reference requires
+	TaskRunReasonParamKeyNotExistent TaskRunReason = "ParamKeyNotExistent"
 )
 
 func (t TaskRunReason) String() string {

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 = "provider"
 )
 
 // SupportedVerbs returns the operations that validation should be called for
@@ -408,6 +409,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{}