gitresolver pipes extra resolved data through Annotation

Change 1:
- before: The resolvedResource returned by gitresolver only contains revision
and the actual yaml content.
- now: The resolvedResource returned by gitresolver contains the actual yaml
content and all information about git source - url, revision, pathToYaml

Change 2:
- before: The revision information in git resolvedResource is the original
value provided by user, which can be actual commit hash, or branch name or tag.
- now: For the branch name and tag, we fetch its actual commit hash and save
it in the revision field of the resolvedResource.

Change 3:
- before: The resolvedResource's annotations are not used/passed to the runobject at all.
- now: The annotations are passed to the runobject along with the annotations in resolved task.

Motivation: Tekton Chains needs to capture the source of the config file (pipeline.yaml
or task.yaml) in the SLSA provenance's `predicate.invocation.configSource` field.
- Passing all 3 fields all the way from resolver to runobject will make it possible for Chains
to capture the information without modifying user-defined spec. (change 1 + 3)
- Aligning revision field to be actual hash helps Chains capture the actual diest which
is required for the `predicate.invocation.configSource` field. (change 2)

pkg/reconciler/pipelinerun/resources/pipelineref.go

@@ -111,8 +111,9 @@ func (l *LocalPipelineRefResolver) GetPipeline(ctx context.Context, name string)
 // fetch a pipeline with given name. An error is returned if the
 // resolution doesn't work or the returned data isn't a valid
 // v1beta1.PipelineObject.
+// TODO (@chuangw6): passing resolved annotations on pipeline level
 func resolvePipeline(ctx context.Context, resolver remote.Resolver, name string) (v1beta1.PipelineObject, error) {
-	obj, err := resolver.Get(ctx, "pipeline", name)
+	obj, _, err := resolver.Get(ctx, "pipeline", name)
 	if err != nil {
 		return nil, err
 	}

pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go

@@ -776,7 +776,7 @@ func resolveTask(
 			spec = *taskRun.Status.TaskSpec
 			taskName = pipelineTask.TaskRef.Name
 		} else {
-			t, err = getTask(ctx, pipelineTask.TaskRef.Name)
+			t, _, err = getTask(ctx, pipelineTask.TaskRef.Name)
 			switch {
 			case errors.Is(err, remote.ErrorRequestInProgress):
 				return v1beta1.TaskSpec{}, "", "", err

pkg/reconciler/pipelinerun/resources/pipelinerunresolution_test.go

@@ -48,8 +48,8 @@ import (
 func nopGetRun(string) (*v1alpha1.Run, error) {
 	return nil, errors.New("GetRun should not be called")
 }
-func nopGetTask(context.Context, string) (v1beta1.TaskObject, error) {
-	return nil, errors.New("GetTask should not be called")
+func nopGetTask(context.Context, string) (v1beta1.TaskObject, map[string]string, error) {
+	return nil, nil, errors.New("GetTask should not be called")
 }
 func nopGetTaskRun(string) (*v1beta1.TaskRun, error) {
 	return nil, errors.New("GetTaskRun should not be called")
@@ -1959,7 +1959,9 @@ func TestResolvePipelineRun(t *testing.T) {
 	}
 	// The Task "task" doesn't actually take any inputs or outputs, but validating
 	// that is not done as part of Run resolution
-	getTask := func(ctx context.Context, name string) (v1beta1.TaskObject, error) { return task, nil }
+	getTask := func(ctx context.Context, name string) (v1beta1.TaskObject, map[string]string, error) {
+		return task, nil, nil
+	}
 	getTaskRun := func(name string) (*v1beta1.TaskRun, error) { return nil, nil }
 
 	pipelineState := PipelineRunState{}
@@ -2102,7 +2104,9 @@ func TestResolvePipelineRun_PipelineTaskHasNoResources(t *testing.T) {
 	}}
 	providedResources := map[string]*resourcev1alpha1.PipelineResource{}
 
-	getTask := func(ctx context.Context, name string) (v1beta1.TaskObject, error) { return task, nil }
+	getTask := func(ctx context.Context, name string) (v1beta1.TaskObject, map[string]string, error) {
+		return task, nil, nil
+	}
 	getTaskRun := func(name string) (*v1beta1.TaskRun, error) { return &trs[0], nil }
 	pr := v1beta1.PipelineRun{
 		ObjectMeta: metav1.ObjectMeta{
@@ -2152,8 +2156,8 @@ func TestResolvePipelineRun_TaskDoesntExist(t *testing.T) {
 	providedResources := map[string]*resourcev1alpha1.PipelineResource{}
 
 	// Return an error when the Task is retrieved, as if it didn't exist
-	getTask := func(ctx context.Context, name string) (v1beta1.TaskObject, error) {
-		return nil, kerrors.NewNotFound(v1beta1.Resource("task"), name)
+	getTask := func(ctx context.Context, name string) (v1beta1.TaskObject, map[string]string, error) {
+		return nil, nil, kerrors.NewNotFound(v1beta1.Resource("task"), name)
 	}
 	getTaskRun := func(name string) (*v1beta1.TaskRun, error) {
 		return nil, kerrors.NewNotFound(v1beta1.Resource("taskrun"), name)
@@ -2221,7 +2225,9 @@ func TestResolvePipelineRun_ResourceBindingsDontExist(t *testing.T) {
 	}}
 	providedResources := map[string]*resourcev1alpha1.PipelineResource{}
 
-	getTask := func(ctx context.Context, name string) (v1beta1.TaskObject, error) { return task, nil }
+	getTask := func(ctx context.Context, name string) (v1beta1.TaskObject, map[string]string, error) {
+		return task, nil, nil
+	}
 	getTaskRun := func(name string) (*v1beta1.TaskRun, error) { return &trs[0], nil }
 
 	for _, tt := range tests {
@@ -2291,7 +2297,9 @@ func TestResolvePipelineRun_withExistingTaskRuns(t *testing.T) {
 
 	// The Task "task" doesn't actually take any inputs or outputs, but validating
 	// that is not done as part of Run resolution
-	getTask := func(_ context.Context, name string) (v1beta1.TaskObject, error) { return task, nil }
+	getTask := func(_ context.Context, name string) (v1beta1.TaskObject, map[string]string, error) {
+		return task, nil, nil
+	}
 	getTaskRun := func(name string) (*v1beta1.TaskRun, error) { return nil, nil }
 	resolvedTask, err := ResolvePipelineTask(context.Background(), pr, getTask, getTaskRun, nopGetRun, p.Spec.Tasks[0], providedResources)
 	if err != nil {
@@ -2355,8 +2363,8 @@ func TestResolvedPipelineRun_PipelineTaskHasOptionalResources(t *testing.T) {
 		},
 	}
 
-	getTask := func(ctx context.Context, name string) (v1beta1.TaskObject, error) {
-		return taskWithOptionalResourcesDeprecated, nil
+	getTask := func(ctx context.Context, name string) (v1beta1.TaskObject, map[string]string, error) {
+		return taskWithOptionalResourcesDeprecated, nil, nil
 	}
 	getTaskRun := func(name string) (*v1beta1.TaskRun, error) { return nil, nil }
 
@@ -2694,7 +2702,9 @@ func TestResolvePipeline_WhenExpressions(t *testing.T) {
 
 	providedResources := map[string]*resourcev1alpha1.PipelineResource{}
 
-	getTask := func(_ context.Context, name string) (v1beta1.TaskObject, error) { return task, nil }
+	getTask := func(_ context.Context, name string) (v1beta1.TaskObject, map[string]string, error) {
+		return task, nil, nil
+	}
 	pr := v1beta1.PipelineRun{
 		ObjectMeta: metav1.ObjectMeta{
 			Name: "pipelinerun",
@@ -2725,7 +2735,9 @@ func TestIsCustomTask(t *testing.T) {
 			Name: "pipelinerun",
 		},
 	}
-	getTask := func(ctx context.Context, name string) (v1beta1.TaskObject, error) { return task, nil }
+	getTask := func(ctx context.Context, name string) (v1beta1.TaskObject, map[string]string, error) {
+		return task, nil, nil
+	}
 	getTaskRun := func(name string) (*v1beta1.TaskRun, error) { return nil, nil }
 	getRun := func(name string) (*v1alpha1.Run, error) { return nil, nil }
 
@@ -3514,7 +3526,9 @@ func TestIsMatrixed(t *testing.T) {
 			Name: "pipelinerun",
 		},
 	}
-	getTask := func(ctx context.Context, name string) (v1beta1.TaskObject, error) { return task, nil }
+	getTask := func(ctx context.Context, name string) (v1beta1.TaskObject, map[string]string, error) {
+		return task, nil, nil
+	}
 	getTaskRun := func(name string) (*v1beta1.TaskRun, error) { return &trs[0], nil }
 	getRun := func(name string) (*v1alpha1.Run, error) { return &runs[0], nil }
 
@@ -3644,7 +3658,9 @@ func TestResolvePipelineRunTask_WithMatrix(t *testing.T) {
 		Outputs: map[string]*resourcev1alpha1.PipelineResource{},
 	}
 
-	getTask := func(ctx context.Context, name string) (v1beta1.TaskObject, error) { return task, nil }
+	getTask := func(ctx context.Context, name string) (v1beta1.TaskObject, map[string]string, error) {
+		return task, nil, nil
+	}
 	getTaskRun := func(name string) (*v1beta1.TaskRun, error) { return taskRunsMap[name], nil }
 	getRun := func(name string) (*v1alpha1.Run, error) { return &runs[0], nil }
 
@@ -3744,7 +3760,9 @@ func TestResolvePipelineRunTask_WithMatrixedCustomTask(t *testing.T) {
 		}},
 	}}
 
-	getTask := func(ctx context.Context, name string) (v1beta1.TaskObject, error) { return task, nil }
+	getTask := func(ctx context.Context, name string) (v1beta1.TaskObject, map[string]string, error) {
+		return task, nil, nil
+	}
 	getTaskRun := func(name string) (*v1beta1.TaskRun, error) { return &trs[0], nil }
 	getRun := func(name string) (*v1alpha1.Run, error) { return runsMap[name], nil }
 

pkg/reconciler/taskrun/resources/taskref.go

@@ -58,14 +58,14 @@ func GetTaskKind(taskrun *v1beta1.TaskRun) v1beta1.TaskKind {
 func GetTaskFuncFromTaskRun(ctx context.Context, k8s kubernetes.Interface, tekton clientset.Interface, requester remoteresource.Requester, taskrun *v1beta1.TaskRun) (GetTask, error) {
 	// if the spec is already in the status, do not try to fetch it again, just use it as source of truth
 	if taskrun.Status.TaskSpec != nil {
-		return func(_ context.Context, name string) (v1beta1.TaskObject, error) {
+		return func(_ context.Context, name string) (v1beta1.TaskObject, map[string]string, error) {
 			return &v1beta1.Task{
 				ObjectMeta: metav1.ObjectMeta{
 					Name:      name,
 					Namespace: taskrun.Namespace,
 				},
 				Spec: *taskrun.Status.TaskSpec,
-			}, nil
+			}, nil, nil
 		}, nil
 	}
 	return GetTaskFunc(ctx, k8s, tekton, requester, taskrun, taskrun.Spec.TaskRef, taskrun.Name, taskrun.Namespace, taskrun.Spec.ServiceAccountName)
@@ -87,14 +87,14 @@ func GetTaskFunc(ctx context.Context, k8s kubernetes.Interface, tekton clientset
 	case cfg.FeatureFlags.EnableTektonOCIBundles && tr != nil && tr.Bundle != "":
 		// Return an inline function that implements GetTask by calling Resolver.Get with the specified task type and
 		// casting it to a TaskObject.
-		return func(ctx context.Context, name string) (v1beta1.TaskObject, error) {
+		return func(ctx context.Context, name string) (v1beta1.TaskObject, map[string]string, error) {
 			// If there is a bundle url at all, construct an OCI resolver to fetch the task.
 			kc, err := k8schain.New(ctx, k8s, k8schain.Options{
 				Namespace:          namespace,
 				ServiceAccountName: saName,
 			})
 			if err != nil {
-				return nil, fmt.Errorf("failed to get keychain: %w", err)
+				return nil, nil, fmt.Errorf("failed to get keychain: %w", err)
 			}
 			resolver := oci.NewResolver(tr.Bundle, kc)
 
@@ -103,7 +103,7 @@ func GetTaskFunc(ctx context.Context, k8s kubernetes.Interface, tekton clientset
 	case cfg.FeatureFlags.EnableAPIFields == config.AlphaAPIFields && tr != nil && tr.Resolver != "" && requester != nil:
 		// Return an inline function that implements GetTask by calling Resolver.Get with the specified task type and
 		// casting it to a TaskObject.
-		return func(ctx context.Context, name string) (v1beta1.TaskObject, error) {
+		return func(ctx context.Context, name string) (v1beta1.TaskObject, map[string]string, error) {
 			params := map[string]string{}
 			var replacedParams []v1beta1.Param
 			if ownerAsTR, ok := owner.(*v1beta1.TaskRun); ok {
@@ -137,18 +137,18 @@ func GetTaskFunc(ctx context.Context, k8s kubernetes.Interface, tekton clientset
 // fetch a task with given name. An error is returned if the
 // remoteresource doesn't work or the returned data isn't a valid
 // v1beta1.TaskObject.
-func resolveTask(ctx context.Context, resolver remote.Resolver, name string, kind v1beta1.TaskKind) (v1beta1.TaskObject, error) {
+func resolveTask(ctx context.Context, resolver remote.Resolver, name string, kind v1beta1.TaskKind) (v1beta1.TaskObject, map[string]string, error) {
 	// Because the resolver will only return references with the same kind (eg ClusterTask), this will ensure we
 	// don't accidentally return a Task with the same name but different kind.
-	obj, err := resolver.Get(ctx, strings.TrimSuffix(strings.ToLower(string(kind)), "s"), name)
+	obj, annotations, err := resolver.Get(ctx, strings.TrimSuffix(strings.ToLower(string(kind)), "s"), name)
 	if err != nil {
-		return nil, err
+		return nil, nil, err
 	}
 	taskObj, err := readRuntimeObjectAsTask(ctx, obj)
 	if err != nil {
-		return nil, fmt.Errorf("failed to convert obj %s into Task", obj.GetObjectKind().GroupVersionKind().String())
+		return nil, nil, fmt.Errorf("failed to convert obj %s into Task", obj.GetObjectKind().GroupVersionKind().String())
 	}
-	return taskObj, nil
+	return taskObj, annotations, nil
 }
 
 // readRuntimeObjectAsTask tries to convert a generic runtime.Object
@@ -174,20 +174,25 @@ type LocalTaskRefResolver struct {
 
 // GetTask will resolve either a Task or ClusterTask from the local cluster using a versioned Tekton client. It will
 // return an error if it can't find an appropriate Task for any reason.
-func (l *LocalTaskRefResolver) GetTask(ctx context.Context, name string) (v1beta1.TaskObject, error) {
+// TODO (@chuangw6): if we want to pass any annotations for in-cluster task, we can pass them through the second returned value
+func (l *LocalTaskRefResolver) GetTask(ctx context.Context, name string) (v1beta1.TaskObject, map[string]string, error) {
 	if l.Kind == v1beta1.ClusterTaskKind {
 		task, err := l.Tektonclient.TektonV1beta1().ClusterTasks().Get(ctx, name, metav1.GetOptions{})
 		if err != nil {
-			return nil, err
+			return nil, nil, err
 		}
-		return task, nil
+		return task, nil, nil
 	}
 
 	// If we are going to resolve this reference locally, we need a namespace scope.
 	if l.Namespace == "" {
-		return nil, fmt.Errorf("must specify namespace to resolve reference to task %s", name)
+		return nil, nil, fmt.Errorf("must specify namespace to resolve reference to task %s", name)
+	}
+	task, err := l.Tektonclient.TektonV1beta1().Tasks(l.Namespace).Get(ctx, name, metav1.GetOptions{})
+	if err != nil {
+		return nil, nil, err
 	}
-	return l.Tektonclient.TektonV1beta1().Tasks(l.Namespace).Get(ctx, name, metav1.GetOptions{})
+	return task, nil, nil
 }
 
 // IsGetTaskErrTransient returns true if an error returned by GetTask is retryable.

pkg/reconciler/taskrun/resources/taskref_test.go

@@ -202,7 +202,7 @@ func TestLocalTaskRef(t *testing.T) {
 				Tektonclient: tektonclient,
 			}
 
-			task, err := lc.GetTask(ctx, tc.ref.Name)
+			task, _, err := lc.GetTask(ctx, tc.ref.Name)
 			if tc.wantErr && err == nil {
 				t.Fatal("Expected error but found nil instead")
 			} else if !tc.wantErr && err != nil {
@@ -436,7 +436,7 @@ func TestGetTaskFunc(t *testing.T) {
 				t.Fatalf("failed to get task fn: %s", err.Error())
 			}
 
-			task, err := fn(ctx, tc.ref.Name)
+			task, _, err := fn(ctx, tc.ref.Name)
 			if err != nil {
 				t.Fatalf("failed to call taskfn: %s", err.Error())
 			}
@@ -496,7 +496,7 @@ echo hello
 	if err != nil {
 		t.Fatalf("failed to get Task fn: %s", err.Error())
 	}
-	actualTask, err := fn(ctx, name)
+	actualTask, _, err := fn(ctx, name)
 	if err != nil {
 		t.Fatalf("failed to call Taskfn: %s", err.Error())
 	}
@@ -518,7 +518,8 @@ func TestGetTaskFunc_RemoteResolution(t *testing.T) {
 		"apiVersion: tekton.dev/v1beta1",
 		taskYAMLString,
 	}, "\n")
-	resolved := test.NewResolvedResource([]byte(taskYAML), nil, nil)
+	expectedAnnotations := map[string]string{"url": "abc.com", "digest": "sha123"}
+	resolved := test.NewResolvedResource([]byte(taskYAML), expectedAnnotations, nil)
 	requester := test.NewRequester(resolved, nil)
 	tr := &v1beta1.TaskRun{
 		ObjectMeta: metav1.ObjectMeta{Namespace: "default"},
@@ -532,11 +533,15 @@ func TestGetTaskFunc_RemoteResolution(t *testing.T) {
 		t.Fatalf("failed to get task fn: %s", err.Error())
 	}
 
-	resolvedTask, err := fn(ctx, taskRef.Name)
+	resolvedTask, annotation, err := fn(ctx, taskRef.Name)
 	if err != nil {
 		t.Fatalf("failed to call pipelinefn: %s", err.Error())
 	}
 
+	if d := cmp.Diff(expectedAnnotations, annotation); d != "" {
+		t.Errorf("annotations did not match: %s", diff.PrintWantGot(d))
+	}
+
 	if d := cmp.Diff(task, resolvedTask); d != "" {
 		t.Error(d)
 	}
@@ -583,7 +588,7 @@ func TestGetTaskFunc_RemoteResolution_ReplacedParams(t *testing.T) {
 		t.Fatalf("failed to get task fn: %s", err.Error())
 	}
 
-	resolvedTask, err := fn(ctx, taskRef.Name)
+	resolvedTask, _, err := fn(ctx, taskRef.Name)
 	if err != nil {
 		t.Fatalf("failed to call pipelinefn: %s", err.Error())
 	}
@@ -618,7 +623,7 @@ func TestGetTaskFunc_RemoteResolution_ReplacedParams(t *testing.T) {
 		t.Fatalf("failed to get task fn: %s", err.Error())
 	}
 
-	_, err = fnNotMatching(ctx, taskRefNotMatching.Name)
+	_, _, err = fnNotMatching(ctx, taskRefNotMatching.Name)
 	if err == nil {
 		t.Fatal("expected error for non-matching params, did not get one")
 	}
@@ -647,7 +652,7 @@ func TestGetPipelineFunc_RemoteResolutionInvalidData(t *testing.T) {
 	if err != nil {
 		t.Fatalf("failed to get pipeline fn: %s", err.Error())
 	}
-	if _, err := fn(ctx, taskRef.Name); err == nil {
+	if _, _, err := fn(ctx, taskRef.Name); err == nil {
 		t.Fatalf("expected error due to invalid pipeline data but saw none")
 	}
 }

pkg/reconciler/taskrun/resources/taskspec.go

@@ -27,7 +27,7 @@ import (
 )
 
 // GetTask is a function used to retrieve Tasks.
-type GetTask func(context.Context, string) (v1beta1.TaskObject, error)
+type GetTask func(context.Context, string) (v1beta1.TaskObject, map[string]string, error)
 
 // GetTaskRun is a function used to retrieve TaskRuns
 type GetTaskRun func(string) (*v1beta1.TaskRun, error)
@@ -45,29 +45,41 @@ func GetTaskData(ctx context.Context, taskRun *v1beta1.TaskRun, getTask GetTask)
 	switch {
 	case taskRun.Spec.TaskRef != nil && taskRun.Spec.TaskRef.Name != "":
 		// Get related task for taskrun
-		t, err := getTask(ctx, taskRun.Spec.TaskRef.Name)
+		task, annotation, err := getTask(ctx, taskRun.Spec.TaskRef.Name)
 		if err != nil {
 			return nil, nil, fmt.Errorf("error when listing tasks for taskRun %s: %w", taskRun.Name, err)
 		}
-		taskMeta = t.TaskMetadata()
-		taskSpec = t.TaskSpec()
+		taskMeta = task.TaskMetadata()
+		taskMeta.Annotations = appendAnnotations(taskMeta.Annotations, annotation)
+		taskSpec = task.TaskSpec()
 		taskSpec.SetDefaults(ctx)
 	case taskRun.Spec.TaskSpec != nil:
 		taskMeta = taskRun.ObjectMeta
 		taskSpec = *taskRun.Spec.TaskSpec
 	case cfg.FeatureFlags.EnableAPIFields == config.AlphaAPIFields && taskRun.Spec.TaskRef != nil && taskRun.Spec.TaskRef.Resolver != "":
-		task, err := getTask(ctx, taskRun.Name)
+		task, annotation, err := getTask(ctx, taskRun.Name)
 		switch {
 		case err != nil:
 			return nil, nil, err
 		case task == nil:
 			return nil, nil, errors.New("resolution of remote resource completed successfully but no task was returned")
 		default:
 			taskMeta = task.TaskMetadata()
+			taskMeta.Annotations = appendAnnotations(taskMeta.Annotations, annotation)
 			taskSpec = task.TaskSpec()
 		}
 	default:
 		return nil, nil, fmt.Errorf("taskRun %s not providing TaskRef or TaskSpec", taskRun.Name)
 	}
 	return &taskMeta, &taskSpec, nil
 }
+
+func appendAnnotations(original, added map[string]string) map[string]string {
+	for key, val := range added {
+		// Do not override original
+		if _, ok := original[key]; !ok {
+			original[key] = val
+		}
+	}
+	return original
+}