From 98ab372e1015dbeed55fe47629ca7f7e52233641 Mon Sep 17 00:00:00 2001 From: Chuang Wang Date: Tue, 30 Aug 2022 16:25:34 -0700 Subject: [PATCH] Populate the `*Run.Status.Provenance.ConfigSource` field Prior, remote `ResolutionRequest` CRD supports **recording** the source information in its Status that identifies where the remote resource came from. Similarly, `TaskRun/PipelineRun` CRD supports **receiving** the source information via a field in its status from remote ResolutionRequest. Specifically, this field named `ConfigSource` is a subfield of the `Provenance` field in status. In this PR, we are trying to pass the data from `ResolutionRequest` to pipeline reconciler so that the data can be captured in TaskRun/PipelineRun's `Status.Provenance.ConfigSource`. The implication of this change is that many functions' interface has been changed because we are passing this extra source data alongside the remote resoure. Note: - The `provenance` field in Run.status is behind a feature flag named `enable-provenance-in-status` , which was introduced in https://github.com/tektoncd/pipeline/pull/5670. The field will be populated iff the flag is set to `true`. - If a pipeline yaml is from remote place A, and the individual tasks are from other remote places, pipelinerun status will only record the source for the pipeline, and the individual taskrun status will record the source for the corresponding task. Related PRs/Issues: - https://github.com/tektoncd/pipeline/pull/5580 - https://github.com/tektoncd/pipeline/pull/5551 - https://github.com/tektoncd/pipeline/pull/5670 - https://github.com/tektoncd/pipeline/issues/5522 Signed-off-by: Chuang Wang --- pkg/reconciler/pipelinerun/pipelinerun.go | 19 +++- .../pipelinerun/pipelinerun_test.go | 86 +++++++++++++++--- .../pipelinerun/pipelinespec/pipelinespec.go | 23 +++-- .../pipelinespec/pipelinespec_test.go | 60 ++++++++---- .../pipelinerun/resources/pipelineref.go | 42 +++++---- .../pipelinerun/resources/pipelineref_test.go | 87 ++++++++++++++---- .../resources/pipelinerunresolution.go | 4 +- .../resources/pipelinerunresolution_test.go | 52 +++++++---- pkg/reconciler/taskrun/resources/taskref.go | 45 +++++---- .../taskrun/resources/taskref_test.go | 88 ++++++++++++++---- pkg/reconciler/taskrun/resources/taskspec.go | 25 ++++- .../taskrun/resources/taskspec_test.go | 65 +++++++++---- pkg/reconciler/taskrun/taskrun.go | 17 +++- pkg/reconciler/taskrun/taskrun_test.go | 91 ++++++++++++++++--- pkg/remote/oci/resolver.go | 20 ++-- pkg/remote/oci/resolver_test.go | 6 +- pkg/remote/resolution/resolver.go | 16 ++-- pkg/remote/resolution/resolver_test.go | 7 +- pkg/remote/resolver.go | 5 +- test/resolution.go | 4 +- 20 files changed, 566 insertions(+), 196 deletions(-) diff --git a/pkg/reconciler/pipelinerun/pipelinerun.go b/pkg/reconciler/pipelinerun/pipelinerun.go index 568c456bd9e..5c6d991b913 100644 --- a/pkg/reconciler/pipelinerun/pipelinerun.go +++ b/pkg/reconciler/pipelinerun/pipelinerun.go @@ -522,7 +522,7 @@ func (c *Reconciler) reconcile(ctx context.Context, pr *v1beta1.PipelineRun, get if len(pipelineSpec.Finally) > 0 { tasks = append(tasks, pipelineSpec.Finally...) } - pipelineRunState, err := c.resolvePipelineState(ctx, tasks, pipelineMeta, pr, providedResources) + pipelineRunState, err := c.resolvePipelineState(ctx, tasks, pipelineMeta.ObjectMeta, pr, providedResources) switch { case errors.Is(err, remote.ErrorRequestInProgress): message := fmt.Sprintf("PipelineRun %s/%s awaiting remote resource", pr.Namespace, pr.Name) @@ -1234,10 +1234,13 @@ func (c *Reconciler) updateLabelsAndAnnotations(ctx context.Context, pr *v1beta1 return newPr, nil } -func storePipelineSpecAndMergeMeta(pr *v1beta1.PipelineRun, ps *v1beta1.PipelineSpec, meta *metav1.ObjectMeta) error { +func storePipelineSpecAndMergeMeta(pr *v1beta1.PipelineRun, ps *v1beta1.PipelineSpec, meta *tresources.ResolvedObjectMeta) error { // Only store the PipelineSpec once, if it has never been set before. if pr.Status.PipelineSpec == nil { pr.Status.PipelineSpec = ps + if meta == nil { + return nil + } // Propagate labels from Pipeline to PipelineRun. PipelineRun labels take precedences over Pipeline. pr.ObjectMeta.Labels = kmap.Union(meta.Labels, pr.ObjectMeta.Labels) @@ -1247,6 +1250,18 @@ func storePipelineSpecAndMergeMeta(pr *v1beta1.PipelineRun, ps *v1beta1.Pipeline pr.ObjectMeta.Annotations = kmap.Union(meta.Annotations, pr.ObjectMeta.Annotations) } + + // Propagate ConfigSource from remote resolution to PipelineRun Status + // This lives outside of the status.spec check to avoid the case where only the spec is available in the first reconcile and source comes in next reconcile. + if meta != nil && meta.ConfigSource != nil { + if pr.Status.Provenance == nil { + pr.Status.Provenance = &v1beta1.Provenance{} + } + if pr.Status.Provenance.ConfigSource == nil { + pr.Status.Provenance.ConfigSource = meta.ConfigSource + } + } + return nil } diff --git a/pkg/reconciler/pipelinerun/pipelinerun_test.go b/pkg/reconciler/pipelinerun/pipelinerun_test.go index f4ee16052ad..887528e86fd 100644 --- a/pkg/reconciler/pipelinerun/pipelinerun_test.go +++ b/pkg/reconciler/pipelinerun/pipelinerun_test.go @@ -39,6 +39,7 @@ import ( resourcev1alpha1 "github.com/tektoncd/pipeline/pkg/apis/resource/v1alpha1" "github.com/tektoncd/pipeline/pkg/reconciler/events/cloudevent" "github.com/tektoncd/pipeline/pkg/reconciler/pipelinerun/resources" + tresources "github.com/tektoncd/pipeline/pkg/reconciler/taskrun/resources" ttesting "github.com/tektoncd/pipeline/pkg/reconciler/testing" "github.com/tektoncd/pipeline/pkg/reconciler/volumeclaim" resolutioncommon "github.com/tektoncd/pipeline/pkg/resolution/common" @@ -5523,7 +5524,7 @@ status: } } -func Test_storePipelineSpec(t *testing.T) { +func Test_storePipelineSpecAndConfigSource(t *testing.T) { pr := parse.MustParseV1beta1PipelineRun(t, ` metadata: name: test-pipeline-run-success @@ -5532,6 +5533,13 @@ metadata: annotations: io.annotation: value `) + configSource := &v1beta1.ConfigSource{ + URI: "abc.com", + Digest: map[string]string{ + "sha1": "a123", + }, + EntryPoint: "foo/bar", + } ps := v1beta1.PipelineSpec{Description: "foo-pipeline"} ps1 := v1beta1.PipelineSpec{Description: "bar-pipeline"} @@ -5540,24 +5548,74 @@ metadata: want.Status = v1beta1.PipelineRunStatus{ PipelineRunStatusFields: v1beta1.PipelineRunStatusFields{ PipelineSpec: ps.DeepCopy(), + Provenance: &v1beta1.Provenance{ + ConfigSource: configSource.DeepCopy(), + }, }, } want.ObjectMeta.Labels["tekton.dev/pipeline"] = pr.ObjectMeta.Name - // The first time we set it, it should get copied. - if err := storePipelineSpecAndMergeMeta(pr, &ps, &pr.ObjectMeta); err != nil { - t.Errorf("storePipelineSpec() error = %v", err) - } - if d := cmp.Diff(pr, want); d != "" { - t.Fatalf(diff.PrintWantGot(d)) + type args struct { + pipelineSpec *v1beta1.PipelineSpec + resolvedObjectMeta *tresources.ResolvedObjectMeta } - // The next time, it should not get overwritten - if err := storePipelineSpecAndMergeMeta(pr, &ps1, &metav1.ObjectMeta{}); err != nil { - t.Errorf("storePipelineSpec() error = %v", err) + var tests = []struct { + name string + reconcile1Args *args + reconcile2Args *args + wantPipelineRun *v1beta1.PipelineRun + }{ + { + name: "spec and source are available in the same reconcile", + reconcile1Args: &args{ + pipelineSpec: &ps, + resolvedObjectMeta: &tresources.ResolvedObjectMeta{ + ObjectMeta: &pr.ObjectMeta, + ConfigSource: configSource.DeepCopy(), + }, + }, + reconcile2Args: &args{ + pipelineSpec: &ps1, + resolvedObjectMeta: &tresources.ResolvedObjectMeta{}, + }, + wantPipelineRun: want, + }, + { + name: "spec comes in the first reconcile and source comes in next reconcile", + reconcile1Args: &args{ + pipelineSpec: &ps, + resolvedObjectMeta: &tresources.ResolvedObjectMeta{ + ObjectMeta: &pr.ObjectMeta, + }, + }, + reconcile2Args: &args{ + pipelineSpec: &ps, + resolvedObjectMeta: &tresources.ResolvedObjectMeta{ + ConfigSource: configSource.DeepCopy(), + }, + }, + wantPipelineRun: want, + }, } - if d := cmp.Diff(pr, want); d != "" { - t.Fatalf(diff.PrintWantGot(d)) + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + // mock first reconcile + if err := storePipelineSpecAndMergeMeta(pr, tc.reconcile1Args.pipelineSpec, tc.reconcile1Args.resolvedObjectMeta); err != nil { + t.Errorf("storePipelineSpec() error = %v", err) + } + if d := cmp.Diff(pr, tc.wantPipelineRun); d != "" { + t.Fatalf(diff.PrintWantGot(d)) + } + + // mock second reconcile + if err := storePipelineSpecAndMergeMeta(pr, tc.reconcile2Args.pipelineSpec, tc.reconcile2Args.resolvedObjectMeta); err != nil { + t.Errorf("storePipelineSpec() error = %v", err) + } + if d := cmp.Diff(pr, tc.wantPipelineRun); d != "" { + t.Fatalf(diff.PrintWantGot(d)) + } + }) } } @@ -5573,7 +5631,9 @@ func Test_storePipelineSpec_metadata(t *testing.T) { ObjectMeta: metav1.ObjectMeta{Name: "foo", Labels: pipelinerunlabels, Annotations: pipelinerunannotations}, } meta := metav1.ObjectMeta{Name: "bar", Labels: pipelinelabels, Annotations: pipelineannotations} - if err := storePipelineSpecAndMergeMeta(pr, &v1beta1.PipelineSpec{}, &meta); err != nil { + if err := storePipelineSpecAndMergeMeta(pr, &v1beta1.PipelineSpec{}, &tresources.ResolvedObjectMeta{ + ObjectMeta: &meta, + }); err != nil { t.Errorf("storePipelineSpecAndMergeMeta error = %v", err) } if d := cmp.Diff(pr.ObjectMeta.Labels, wantedlabels); d != "" { diff --git a/pkg/reconciler/pipelinerun/pipelinespec/pipelinespec.go b/pkg/reconciler/pipelinerun/pipelinespec/pipelinespec.go index 043f5105a17..21e85bbb613 100644 --- a/pkg/reconciler/pipelinerun/pipelinespec/pipelinespec.go +++ b/pkg/reconciler/pipelinerun/pipelinespec/pipelinespec.go @@ -22,32 +22,37 @@ import ( "fmt" "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1" + tresources "github.com/tektoncd/pipeline/pkg/reconciler/taskrun/resources" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) // GetPipeline is a function used to retrieve Pipelines. -type GetPipeline func(context.Context, string) (v1beta1.PipelineObject, error) +type GetPipeline func(context.Context, string) (v1beta1.PipelineObject, *v1beta1.ConfigSource, error) // GetPipelineData will retrieve the Pipeline metadata and Spec associated with the // provided PipelineRun. This can come from a reference Pipeline or from the PipelineRun's // metadata and embedded PipelineSpec. -func GetPipelineData(ctx context.Context, pipelineRun *v1beta1.PipelineRun, getPipeline GetPipeline) (*metav1.ObjectMeta, *v1beta1.PipelineSpec, error) { +func GetPipelineData(ctx context.Context, pipelineRun *v1beta1.PipelineRun, getPipeline GetPipeline) (*tresources.ResolvedObjectMeta, *v1beta1.PipelineSpec, error) { pipelineMeta := metav1.ObjectMeta{} + var configSource *v1beta1.ConfigSource pipelineSpec := v1beta1.PipelineSpec{} switch { case pipelineRun.Spec.PipelineRef != nil && pipelineRun.Spec.PipelineRef.Name != "": // Get related pipeline for pipelinerun - t, err := getPipeline(ctx, pipelineRun.Spec.PipelineRef.Name) + p, source, err := getPipeline(ctx, pipelineRun.Spec.PipelineRef.Name) if err != nil { return nil, nil, fmt.Errorf("error when listing pipelines for pipelineRun %s: %w", pipelineRun.Name, err) } - pipelineMeta = t.PipelineMetadata() - pipelineSpec = t.PipelineSpec() + pipelineMeta = p.PipelineMetadata() + pipelineSpec = p.PipelineSpec() + configSource = source case pipelineRun.Spec.PipelineSpec != nil: pipelineMeta = pipelineRun.ObjectMeta pipelineSpec = *pipelineRun.Spec.PipelineSpec + // TODO: if we want to set source for embedded pipeline, set it here. + // https://github.com/tektoncd/pipeline/issues/5522 case pipelineRun.Spec.PipelineRef != nil && pipelineRun.Spec.PipelineRef.Resolver != "": - pipeline, err := getPipeline(ctx, "") + pipeline, source, err := getPipeline(ctx, "") switch { case err != nil: return nil, nil, err @@ -57,10 +62,14 @@ func GetPipelineData(ctx context.Context, pipelineRun *v1beta1.PipelineRun, getP pipelineMeta = pipeline.PipelineMetadata() pipelineSpec = pipeline.PipelineSpec() } + configSource = source default: return nil, nil, fmt.Errorf("pipelineRun %s not providing PipelineRef or PipelineSpec", pipelineRun.Name) } pipelineSpec.SetDefaults(ctx) - return &pipelineMeta, &pipelineSpec, nil + return &tresources.ResolvedObjectMeta{ + ObjectMeta: &pipelineMeta, + ConfigSource: configSource, + }, &pipelineSpec, nil } diff --git a/pkg/reconciler/pipelinerun/pipelinespec/pipelinespec_test.go b/pkg/reconciler/pipelinerun/pipelinespec/pipelinespec_test.go index da9351f1c72..2dbb4e8a220 100644 --- a/pkg/reconciler/pipelinerun/pipelinespec/pipelinespec_test.go +++ b/pkg/reconciler/pipelinerun/pipelinespec/pipelinespec_test.go @@ -51,20 +51,26 @@ func TestGetPipelineSpec_Ref(t *testing.T) { }, }, } - gt := func(ctx context.Context, n string) (v1beta1.PipelineObject, error) { return pipeline, nil } - pipelineMeta, pipelineSpec, err := GetPipelineData(context.Background(), pr, gt) + gt := func(ctx context.Context, n string) (v1beta1.PipelineObject, *v1beta1.ConfigSource, error) { + return pipeline, nil, nil + } + resolvedObjectMeta, pipelineSpec, err := GetPipelineData(context.Background(), pr, gt) if err != nil { t.Fatalf("Did not expect error getting pipeline spec but got: %s", err) } - if pipelineMeta.Name != "orchestrate" { - t.Errorf("Expected pipeline name to be `orchestrate` but was %q", pipelineMeta.Name) + if resolvedObjectMeta.Name != "orchestrate" { + t.Errorf("Expected pipeline name to be `orchestrate` but was %q", resolvedObjectMeta.Name) } if len(pipelineSpec.Tasks) != 1 || pipelineSpec.Tasks[0].Name != "mytask" { t.Errorf("Pipeline Spec not resolved as expected, expected referenced Pipeline spec but got: %v", pipelineSpec) } + + if resolvedObjectMeta.ConfigSource != nil { + t.Errorf("Expected resolved configsource is nil, but got %v", resolvedObjectMeta.ConfigSource) + } } func TestGetPipelineSpec_Embedded(t *testing.T) { @@ -83,22 +89,26 @@ func TestGetPipelineSpec_Embedded(t *testing.T) { }, }, } - gt := func(ctx context.Context, n string) (v1beta1.PipelineObject, error) { - return nil, errors.New("shouldn't be called") + gt := func(ctx context.Context, n string) (v1beta1.PipelineObject, *v1beta1.ConfigSource, error) { + return nil, nil, errors.New("shouldn't be called") } - pipelineMeta, pipelineSpec, err := GetPipelineData(context.Background(), pr, gt) + resolvedObjectMeta, pipelineSpec, err := GetPipelineData(context.Background(), pr, gt) if err != nil { t.Fatalf("Did not expect error getting pipeline spec but got: %s", err) } - if pipelineMeta.Name != "mypipelinerun" { - t.Errorf("Expected pipeline name for embedded pipeline to default to name of pipeline run but was %q", pipelineMeta.Name) + if resolvedObjectMeta.Name != "mypipelinerun" { + t.Errorf("Expected pipeline name for embedded pipeline to default to name of pipeline run but was %q", resolvedObjectMeta.Name) } if len(pipelineSpec.Tasks) != 1 || pipelineSpec.Tasks[0].Name != "mytask" { t.Errorf("Pipeline Spec not resolved as expected, expected embedded Pipeline spec but got: %v", pipelineSpec) } + + if resolvedObjectMeta.ConfigSource != nil { + t.Errorf("Expected resolved configsource is nil, but got %v", resolvedObjectMeta.ConfigSource) + } } func TestGetPipelineSpec_Invalid(t *testing.T) { @@ -107,8 +117,8 @@ func TestGetPipelineSpec_Invalid(t *testing.T) { Name: "mypipelinerun", }, } - gt := func(ctx context.Context, n string) (v1beta1.PipelineObject, error) { - return nil, errors.New("shouldn't be called") + gt := func(ctx context.Context, n string) (v1beta1.PipelineObject, *v1beta1.ConfigSource, error) { + return nil, nil, errors.New("shouldn't be called") } _, _, err := GetPipelineData(context.Background(), tr, gt) if err == nil { @@ -148,11 +158,19 @@ func TestGetPipelineData_ResolutionSuccess(t *testing.T) { }, }}, } - getPipeline := func(ctx context.Context, n string) (v1beta1.PipelineObject, error) { + expectedConfigSource := &v1beta1.ConfigSource{ + URI: "abc.com", + Digest: map[string]string{ + "sha1": "a123", + }, + EntryPoint: "foo/bar", + } + + getPipeline := func(ctx context.Context, n string) (v1beta1.PipelineObject, *v1beta1.ConfigSource, error) { return &v1beta1.Pipeline{ ObjectMeta: *sourceMeta.DeepCopy(), Spec: *sourceSpec.DeepCopy(), - }, nil + }, expectedConfigSource.DeepCopy(), nil } ctx := context.Background() resolvedMeta, resolvedSpec, err := GetPipelineData(ctx, pr, getPipeline) @@ -165,6 +183,10 @@ func TestGetPipelineData_ResolutionSuccess(t *testing.T) { if d := cmp.Diff(sourceSpec, *resolvedSpec); d != "" { t.Errorf(diff.PrintWantGot(d)) } + + if d := cmp.Diff(expectedConfigSource, resolvedMeta.ConfigSource); d != "" { + t.Errorf("configsource did not match: %s", diff.PrintWantGot(d)) + } } func TestGetPipelineSpec_Error(t *testing.T) { @@ -178,8 +200,8 @@ func TestGetPipelineSpec_Error(t *testing.T) { }, }, } - gt := func(ctx context.Context, n string) (v1beta1.PipelineObject, error) { - return nil, errors.New("something went wrong") + gt := func(ctx context.Context, n string) (v1beta1.PipelineObject, *v1beta1.ConfigSource, error) { + return nil, nil, errors.New("something went wrong") } _, _, err := GetPipelineData(context.Background(), tr, gt) if err == nil { @@ -200,8 +222,8 @@ func TestGetPipelineData_ResolutionError(t *testing.T) { }, }, } - getPipeline := func(ctx context.Context, n string) (v1beta1.PipelineObject, error) { - return nil, errors.New("something went wrong") + getPipeline := func(ctx context.Context, n string) (v1beta1.PipelineObject, *v1beta1.ConfigSource, error) { + return nil, nil, errors.New("something went wrong") } ctx := context.Background() _, _, err := GetPipelineData(ctx, pr, getPipeline) @@ -223,8 +245,8 @@ func TestGetPipelineData_ResolvedNilPipeline(t *testing.T) { }, }, } - getPipeline := func(ctx context.Context, n string) (v1beta1.PipelineObject, error) { - return nil, nil + getPipeline := func(ctx context.Context, n string) (v1beta1.PipelineObject, *v1beta1.ConfigSource, error) { + return nil, nil, nil } ctx := context.Background() _, _, err := GetPipelineData(ctx, pr, getPipeline) diff --git a/pkg/reconciler/pipelinerun/resources/pipelineref.go b/pkg/reconciler/pipelinerun/resources/pipelineref.go index be2f1cb409c..d2d65258a29 100644 --- a/pkg/reconciler/pipelinerun/resources/pipelineref.go +++ b/pkg/reconciler/pipelinerun/resources/pipelineref.go @@ -44,36 +44,42 @@ func GetPipelineFunc(ctx context.Context, k8s kubernetes.Interface, tekton clien cfg := config.FromContextOrDefaults(ctx) pr := pipelineRun.Spec.PipelineRef namespace := pipelineRun.Namespace - // if the spec is already in the status, do not try to fetch it again, just use it as source of truth + // if the spec is already in the status, do not try to fetch it again, just use it as source of truth. + // Same for the Source field in the Status.Provenance. if pipelineRun.Status.PipelineSpec != nil { - return func(_ context.Context, name string) (v1beta1.PipelineObject, error) { + return func(_ context.Context, name string) (v1beta1.PipelineObject, *v1beta1.ConfigSource, error) { + var configSource *v1beta1.ConfigSource + if pipelineRun.Status.Provenance != nil { + configSource = pipelineRun.Status.Provenance.ConfigSource + } return &v1beta1.Pipeline{ ObjectMeta: metav1.ObjectMeta{ Name: name, Namespace: namespace, }, Spec: *pipelineRun.Status.PipelineSpec, - }, nil + }, configSource, nil }, nil } + switch { case cfg.FeatureFlags.EnableTektonOCIBundles && pr != nil && pr.Bundle != "": // Return an inline function that implements GetTask by calling Resolver.Get with the specified task type and // casting it to a PipelineObject. - return func(ctx context.Context, name string) (v1beta1.PipelineObject, error) { + return func(ctx context.Context, name string) (v1beta1.PipelineObject, *v1beta1.ConfigSource, error) { // If there is a bundle url at all, construct an OCI resolver to fetch the pipeline. kc, err := k8schain.New(ctx, k8s, k8schain.Options{ Namespace: namespace, ServiceAccountName: pipelineRun.Spec.ServiceAccountName, }) 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(pr.Bundle, kc) return resolvePipeline(ctx, resolver, name, k8s) }, nil case pr != nil && pr.Resolver != "" && requester != nil: - return func(ctx context.Context, name string) (v1beta1.PipelineObject, error) { + return func(ctx context.Context, name string) (v1beta1.PipelineObject, *v1beta1.ConfigSource, error) { stringReplacements, arrayReplacements, objectReplacements := paramsFromPipelineRun(ctx, pipelineRun) for k, v := range getContextReplacements("", pipelineRun) { stringReplacements[k] = v @@ -102,40 +108,42 @@ type LocalPipelineRefResolver struct { // GetPipeline will resolve a Pipeline from the local cluster using a versioned Tekton client. It will // return an error if it can't find an appropriate Pipeline for any reason. -func (l *LocalPipelineRefResolver) GetPipeline(ctx context.Context, name string) (v1beta1.PipelineObject, error) { +// TODO: if we want to set source for in-cluster pipeline, set it here. +// https://github.com/tektoncd/pipeline/issues/5522 +func (l *LocalPipelineRefResolver) GetPipeline(ctx context.Context, name string) (v1beta1.PipelineObject, *v1beta1.ConfigSource, error) { // 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 pipeline %s", name) + return nil, nil, fmt.Errorf("Must specify namespace to resolve reference to pipeline %s", name) } pipeline, err := l.Tektonclient.TektonV1beta1().Pipelines(l.Namespace).Get(ctx, name, metav1.GetOptions{}) if err != nil { - return nil, err + return nil, nil, err } if err := verifyResolvedPipeline(ctx, pipeline, l.K8sclient); err != nil { - return nil, err + return nil, nil, err } - return pipeline, nil + return pipeline, nil, nil } // resolvePipeline accepts an impl of remote.Resolver and attempts to // 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. -func resolvePipeline(ctx context.Context, resolver remote.Resolver, name string, k8s kubernetes.Interface) (v1beta1.PipelineObject, error) { - obj, err := resolver.Get(ctx, "pipeline", name) +func resolvePipeline(ctx context.Context, resolver remote.Resolver, name string, k8s kubernetes.Interface) (v1beta1.PipelineObject, *v1beta1.ConfigSource, error) { + obj, source, err := resolver.Get(ctx, "pipeline", name) if err != nil { - return nil, err + return nil, nil, err } pipelineObj, err := readRuntimeObjectAsPipeline(ctx, obj) if err != nil { - return nil, fmt.Errorf("failed to convert obj %s into Pipeline", obj.GetObjectKind().GroupVersionKind().String()) + return nil, nil, fmt.Errorf("failed to convert obj %s into Pipeline", obj.GetObjectKind().GroupVersionKind().String()) } // TODO(#5527): Consider move this function call to GetPipelineData if err := verifyResolvedPipeline(ctx, pipelineObj, k8s); err != nil { - return nil, err + return nil, nil, err } - return pipelineObj, nil + return pipelineObj, source, nil } // readRuntimeObjectAsPipeline tries to convert a generic runtime.Object diff --git a/pkg/reconciler/pipelinerun/resources/pipelineref_test.go b/pkg/reconciler/pipelinerun/resources/pipelineref_test.go index 9fca22211a9..bcc1c13236d 100644 --- a/pkg/reconciler/pipelinerun/resources/pipelineref_test.go +++ b/pkg/reconciler/pipelinerun/resources/pipelineref_test.go @@ -54,6 +54,14 @@ var ( APIVersion: "tekton.dev/v1beta1", }, } + + sampleConfigSource = &v1beta1.ConfigSource{ + URI: "abc.com", + Digest: map[string]string{ + "sha1": "a123", + }, + EntryPoint: "foo/bar", + } ) func TestLocalPipelineRef(t *testing.T) { @@ -96,16 +104,21 @@ func TestLocalPipelineRef(t *testing.T) { Tektonclient: tektonclient, } - task, err := lc.GetPipeline(ctx, tc.ref.Name) + resolvedPipeline, resolvedConfigSource, err := lc.GetPipeline(ctx, tc.ref.Name) if tc.wantErr && err == nil { t.Fatal("Expected error but found nil instead") } else if !tc.wantErr && err != nil { t.Fatalf("Received unexpected error ( %#v )", err) } - if d := cmp.Diff(task, tc.expected); tc.expected != nil && d != "" { + if d := cmp.Diff(resolvedPipeline, tc.expected); tc.expected != nil && d != "" { t.Error(diff.PrintWantGot(d)) } + + if resolvedConfigSource != nil { + t.Errorf("expected configsource is nil, but got %v", resolvedConfigSource) + } + }) } } @@ -197,7 +210,7 @@ func TestGetPipelineFunc(t *testing.T) { t.Fatalf("failed to get pipeline fn: %s", err.Error()) } - pipeline, err := fn(ctx, tc.ref.Name) + pipeline, configSource, err := fn(ctx, tc.ref.Name) if err != nil { t.Fatalf("failed to call pipelinefn: %s", err.Error()) } @@ -205,6 +218,10 @@ func TestGetPipelineFunc(t *testing.T) { if diff := cmp.Diff(pipeline, tc.expected); tc.expected != nil && diff != "" { t.Error(diff) } + + if configSource != nil { + t.Errorf("expected configsource is nil, but got %v", configSource) + } }) } } @@ -240,6 +257,9 @@ func TestGetPipelineFuncSpecAlreadyFetched(t *testing.T) { }, Status: v1beta1.PipelineRunStatus{PipelineRunStatusFields: v1beta1.PipelineRunStatusFields{ PipelineSpec: &pipelineSpec, + Provenance: &v1beta1.Provenance{ + ConfigSource: sampleConfigSource.DeepCopy(), + }, }}, } expectedPipeline := &v1beta1.Pipeline{ @@ -254,7 +274,7 @@ func TestGetPipelineFuncSpecAlreadyFetched(t *testing.T) { if err != nil { t.Fatalf("failed to get pipeline fn: %s", err.Error()) } - actualPipeline, err := fn(ctx, name) + actualPipeline, actualConfigSource, err := fn(ctx, name) if err != nil { t.Fatalf("failed to call pipelinefn: %s", err.Error()) } @@ -262,6 +282,10 @@ func TestGetPipelineFuncSpecAlreadyFetched(t *testing.T) { if diff := cmp.Diff(actualPipeline, expectedPipeline); expectedPipeline != nil && diff != "" { t.Error(diff) } + + if d := cmp.Diff(sampleConfigSource, actualConfigSource); d != "" { + t.Errorf("configSources did not match: %s", diff.PrintWantGot(d)) + } } func TestGetPipelineFunc_RemoteResolution(t *testing.T) { @@ -275,7 +299,8 @@ func TestGetPipelineFunc_RemoteResolution(t *testing.T) { "apiVersion: tekton.dev/v1beta1", pipelineYAMLString, }, "\n") - resolved := test.NewResolvedResource([]byte(pipelineYAML), nil, nil) + + resolved := test.NewResolvedResource([]byte(pipelineYAML), nil, sampleConfigSource.DeepCopy(), nil) requester := test.NewRequester(resolved, nil) fn, err := resources.GetPipelineFunc(ctx, nil, nil, requester, &v1beta1.PipelineRun{ ObjectMeta: metav1.ObjectMeta{Namespace: "default"}, @@ -288,7 +313,7 @@ func TestGetPipelineFunc_RemoteResolution(t *testing.T) { t.Fatalf("failed to get pipeline fn: %s", err.Error()) } - resolvedPipeline, err := fn(ctx, pipelineRef.Name) + resolvedPipeline, resolvedConfigSource, err := fn(ctx, pipelineRef.Name) if err != nil { t.Fatalf("failed to call pipelinefn: %s", err.Error()) } @@ -296,6 +321,10 @@ func TestGetPipelineFunc_RemoteResolution(t *testing.T) { if diff := cmp.Diff(pipeline, resolvedPipeline); diff != "" { t.Error(diff) } + + if d := cmp.Diff(sampleConfigSource, resolvedConfigSource); d != "" { + t.Errorf("configsource did not match: %s", diff.PrintWantGot(d)) + } } func TestGetPipelineFunc_RemoteResolution_ReplacedParams(t *testing.T) { @@ -320,7 +349,8 @@ func TestGetPipelineFunc_RemoteResolution_ReplacedParams(t *testing.T) { "apiVersion: tekton.dev/v1beta1", pipelineYAMLString, }, "\n") - resolved := test.NewResolvedResource([]byte(pipelineYAML), nil, nil) + + resolved := test.NewResolvedResource([]byte(pipelineYAML), nil, sampleConfigSource.DeepCopy(), nil) requester := &test.Requester{ ResolvedResource: resolved, Params: []v1beta1.Param{{ @@ -349,7 +379,7 @@ func TestGetPipelineFunc_RemoteResolution_ReplacedParams(t *testing.T) { t.Fatalf("failed to get pipeline fn: %s", err.Error()) } - resolvedPipeline, err := fn(ctx, pipelineRef.Name) + resolvedPipeline, resolvedConfigSource, err := fn(ctx, pipelineRef.Name) if err != nil { t.Fatalf("failed to call pipelinefn: %s", err.Error()) } @@ -358,6 +388,10 @@ func TestGetPipelineFunc_RemoteResolution_ReplacedParams(t *testing.T) { t.Error(diff) } + if d := cmp.Diff(sampleConfigSource, resolvedConfigSource); d != "" { + t.Errorf("configsource did not match: %s", diff.PrintWantGot(d)) + } + pipelineRefNotMatching := &v1beta1.PipelineRef{ ResolverRef: v1beta1.ResolverRef{ Resolver: "git", @@ -389,7 +423,7 @@ func TestGetPipelineFunc_RemoteResolution_ReplacedParams(t *testing.T) { t.Fatalf("failed to get pipeline fn: %s", err.Error()) } - _, err = fnNotMatching(ctx, pipelineRefNotMatching.Name) + _, _, err = fnNotMatching(ctx, pipelineRefNotMatching.Name) if err == nil { t.Fatal("expected error for non-matching params, did not get one") } @@ -404,7 +438,7 @@ func TestGetPipelineFunc_RemoteResolutionInvalidData(t *testing.T) { ctx = config.ToContext(ctx, cfg) pipelineRef := &v1beta1.PipelineRef{ResolverRef: v1beta1.ResolverRef{Resolver: "git"}} resolvesTo := []byte("INVALID YAML") - resource := test.NewResolvedResource(resolvesTo, nil, nil) + resource := test.NewResolvedResource(resolvesTo, nil, nil, nil) requester := test.NewRequester(resource, nil) fn, err := resources.GetPipelineFunc(ctx, nil, nil, requester, &v1beta1.PipelineRun{ ObjectMeta: metav1.ObjectMeta{Namespace: "default"}, @@ -416,7 +450,7 @@ func TestGetPipelineFunc_RemoteResolutionInvalidData(t *testing.T) { if err != nil { t.Fatalf("failed to get pipeline fn: %s", err.Error()) } - if _, err := fn(ctx, pipelineRef.Name); err == nil { + if _, _, err := fn(ctx, pipelineRef.Name); err == nil { t.Fatalf("expected error due to invalid pipeline data but saw none") } } @@ -517,13 +551,16 @@ func TestLocalPipelineRef_TrustedResourceVerification_Success(t *testing.T) { Tektonclient: tektonclient, } - pipeline, err := lc.GetPipeline(ctx, tc.ref.Name) + pipeline, source, err := lc.GetPipeline(ctx, tc.ref.Name) if err != nil { t.Fatalf("Received unexpected error ( %#v )", err) } if d := cmp.Diff(pipeline, tc.expected); d != "" { t.Error(diff.PrintWantGot(d)) } + if source != nil { + t.Errorf("expected source is nil, but got %v", source) + } }) } } @@ -591,13 +628,17 @@ func TestLocalPipelineRef_TrustedResourceVerification_Error(t *testing.T) { Tektonclient: tektonclient, } - pipeline, err := lc.GetPipeline(ctx, tc.ref.Name) + pipeline, source, err := lc.GetPipeline(ctx, tc.ref.Name) if err == nil || !errors.Is(err, tc.expectedErr) { t.Fatalf("Expected error %v but found %v instead", tc.expectedErr, err) } if d := cmp.Diff(pipeline, tc.expected); d != "" { t.Error(diff.PrintWantGot(d)) } + + if source != nil { + t.Errorf("expected source is nil, but got %v", source) + } }) } } @@ -615,7 +656,7 @@ func TestGetPipelineFunc_RemoteResolution_TrustedResourceVerification_Success(t t.Fatal("fail to marshal pipeline", err) } - resolvedUnsigned := test.NewResolvedResource(unsignedPipelineBytes, nil, nil) + resolvedUnsigned := test.NewResolvedResource(unsignedPipelineBytes, nil, sampleConfigSource.DeepCopy(), nil) requesterUnsigned := test.NewRequester(resolvedUnsigned, nil) signedPipeline, err := test.GetSignedPipeline(unsignedPipeline, signer, "signed") @@ -627,7 +668,7 @@ func TestGetPipelineFunc_RemoteResolution_TrustedResourceVerification_Success(t t.Fatal("fail to marshal pipeline", err) } - resolvedSigned := test.NewResolvedResource(signedPipelineBytes, nil, nil) + resolvedSigned := test.NewResolvedResource(signedPipelineBytes, nil, sampleConfigSource.DeepCopy(), nil) requesterSigned := test.NewRequester(resolvedSigned, nil) tamperedPipeline := signedPipeline.DeepCopy() @@ -636,7 +677,7 @@ func TestGetPipelineFunc_RemoteResolution_TrustedResourceVerification_Success(t if err != nil { t.Fatal("fail to marshal pipeline", err) } - resolvedTampered := test.NewResolvedResource(tamperedPipelineBytes, nil, nil) + resolvedTampered := test.NewResolvedResource(tamperedPipelineBytes, nil, sampleConfigSource.DeepCopy(), nil) requesterTampered := test.NewRequester(resolvedTampered, nil) pipelineRef := &v1beta1.PipelineRef{ResolverRef: v1beta1.ResolverRef{Resolver: "git"}} @@ -699,13 +740,16 @@ func TestGetPipelineFunc_RemoteResolution_TrustedResourceVerification_Success(t t.Fatalf("failed to get pipeline fn: %s", err.Error()) } - resolvedPipeline, err := fn(ctx, pipelineRef.Name) + resolvedPipeline, source, err := fn(ctx, pipelineRef.Name) if err != nil { t.Fatalf("Received unexpected error ( %#v )", err) } if d := cmp.Diff(tc.expected, resolvedPipeline); d != "" { t.Error(d) } + if d := cmp.Diff(sampleConfigSource, source); d != "" { + t.Errorf("configSources did not match: %s", diff.PrintWantGot(d)) + } }) } } @@ -723,7 +767,7 @@ func TestGetPipelineFunc_RemoteResolution_TrustedResourceVerification_Error(t *t t.Fatal("fail to marshal pipeline", err) } - resolvedUnsigned := test.NewResolvedResource(unsignedPipelineBytes, nil, nil) + resolvedUnsigned := test.NewResolvedResource(unsignedPipelineBytes, nil, sampleConfigSource.DeepCopy(), nil) requesterUnsigned := test.NewRequester(resolvedUnsigned, nil) signedPipeline, err := test.GetSignedPipeline(unsignedPipeline, signer, "signed") @@ -737,7 +781,7 @@ func TestGetPipelineFunc_RemoteResolution_TrustedResourceVerification_Error(t *t if err != nil { t.Fatal("fail to marshal pipeline", err) } - resolvedTampered := test.NewResolvedResource(tamperedPipelineBytes, nil, nil) + resolvedTampered := test.NewResolvedResource(tamperedPipelineBytes, nil, sampleConfigSource.DeepCopy(), nil) requesterTampered := test.NewRequester(resolvedTampered, nil) pipelineRef := &v1beta1.PipelineRef{ResolverRef: v1beta1.ResolverRef{Resolver: "git"}} @@ -778,13 +822,16 @@ func TestGetPipelineFunc_RemoteResolution_TrustedResourceVerification_Error(t *t t.Fatalf("failed to get pipeline fn: %s", err.Error()) } - resolvedPipeline, err := fn(ctx, pipelineRef.Name) + resolvedPipeline, source, err := fn(ctx, pipelineRef.Name) if err == nil || !errors.Is(err, tc.expectedErr) { t.Fatalf("Expected error %v but found %v instead", tc.expectedErr, err) } if d := cmp.Diff(tc.expected, resolvedPipeline); d != "" { t.Error(d) } + if source != nil { + t.Errorf("expected source is nil, but got %v", source) + } }) } } diff --git a/pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go b/pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go index 7fd4e81081b..ea467e12224 100644 --- a/pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go +++ b/pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go @@ -778,7 +778,9 @@ func resolveTask( spec = *taskRun.Status.TaskSpec taskName = pipelineTask.TaskRef.Name } else { - t, err = getTask(ctx, pipelineTask.TaskRef.Name) + // Following minimum status principle (TEP-0100), no need to propagate the source about PipelineTask up to PipelineRun status. + // Instead, the child TaskRun's status will be the place recording the source of individual task. + t, _, err = getTask(ctx, pipelineTask.TaskRef.Name) switch { case errors.Is(err, remote.ErrorRequestInProgress): return v1beta1.TaskSpec{}, "", "", err diff --git a/pkg/reconciler/pipelinerun/resources/pipelinerunresolution_test.go b/pkg/reconciler/pipelinerun/resources/pipelinerunresolution_test.go index 6c465db5a52..d59d726d7cc 100644 --- a/pkg/reconciler/pipelinerun/resources/pipelinerunresolution_test.go +++ b/pkg/reconciler/pipelinerun/resources/pipelinerunresolution_test.go @@ -49,8 +49,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, *v1beta1.ConfigSource, 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") @@ -1967,7 +1967,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, *v1beta1.ConfigSource, error) { + return task, nil, nil + } getTaskRun := func(name string) (*v1beta1.TaskRun, error) { return nil, nil } pipelineState := PipelineRunState{} @@ -2110,7 +2112,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, *v1beta1.ConfigSource, error) { + return task, nil, nil + } getTaskRun := func(name string) (*v1beta1.TaskRun, error) { return &trs[0], nil } pr := v1beta1.PipelineRun{ ObjectMeta: metav1.ObjectMeta{ @@ -2161,8 +2165,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, *v1beta1.ConfigSource, 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) @@ -2203,8 +2207,8 @@ func TestResolvePipelineRun_VerificationFailed(t *testing.T) { }}} providedResources := map[string]*resourcev1alpha1.PipelineResource{} - getTask := func(ctx context.Context, name string) (v1beta1.TaskObject, error) { - return nil, trustedresources.ErrorResourceVerificationFailed + getTask := func(ctx context.Context, name string) (v1beta1.TaskObject, *v1beta1.ConfigSource, error) { + return nil, nil, trustedresources.ErrorResourceVerificationFailed } getTaskRun := func(name string) (*v1beta1.TaskRun, error) { return nil, nil } pr := v1beta1.PipelineRun{ @@ -2268,7 +2272,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, *v1beta1.ConfigSource, error) { + return task, nil, nil + } getTaskRun := func(name string) (*v1beta1.TaskRun, error) { return &trs[0], nil } for _, tt := range tests { @@ -2338,7 +2344,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, *v1beta1.ConfigSource, 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 { @@ -2402,8 +2410,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, *v1beta1.ConfigSource, error) { + return taskWithOptionalResourcesDeprecated, nil, nil } getTaskRun := func(name string) (*v1beta1.TaskRun, error) { return nil, nil } @@ -2741,7 +2749,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, *v1beta1.ConfigSource, error) { + return task, nil, nil + } pr := v1beta1.PipelineRun{ ObjectMeta: metav1.ObjectMeta{ Name: "pipelinerun", @@ -2772,7 +2782,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, *v1beta1.ConfigSource, 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 } @@ -3561,7 +3573,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, *v1beta1.ConfigSource, 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 } @@ -3695,7 +3709,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, *v1beta1.ConfigSource, 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 } @@ -3797,7 +3813,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, *v1beta1.ConfigSource, 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 } diff --git a/pkg/reconciler/taskrun/resources/taskref.go b/pkg/reconciler/taskrun/resources/taskref.go index 742971a58d0..6f6be0018ca 100644 --- a/pkg/reconciler/taskrun/resources/taskref.go +++ b/pkg/reconciler/taskrun/resources/taskref.go @@ -58,16 +58,21 @@ func GetTaskKind(taskrun *v1beta1.TaskRun) v1beta1.TaskKind { // cluster or authorize against an external repositroy. It will figure out whether it needs to look in the cluster or in // a remote image to fetch the reference. It will also return the "kind" of the task being referenced. 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 the spec is already in the status, do not try to fetch it again, just use it as source of truth. + // Same for the Source field in the Status.Provenance. if taskrun.Status.TaskSpec != nil { - return func(_ context.Context, name string) (v1beta1.TaskObject, error) { + return func(_ context.Context, name string) (v1beta1.TaskObject, *v1beta1.ConfigSource, error) { + var configsource *v1beta1.ConfigSource + if taskrun.Status.Provenance != nil { + configsource = taskrun.Status.Provenance.ConfigSource + } return &v1beta1.Task{ ObjectMeta: metav1.ObjectMeta{ Name: name, Namespace: taskrun.Namespace, }, Spec: *taskrun.Status.TaskSpec, - }, nil + }, configsource, nil }, nil } return GetTaskFunc(ctx, k8s, tekton, requester, taskrun, taskrun.Spec.TaskRef, taskrun.Name, taskrun.Namespace, taskrun.Spec.ServiceAccountName) @@ -89,14 +94,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, *v1beta1.ConfigSource, 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) @@ -105,7 +110,7 @@ func GetTaskFunc(ctx context.Context, k8s kubernetes.Interface, tekton clientset case 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, *v1beta1.ConfigSource, error) { var replacedParams []v1beta1.Param if ownerAsTR, ok := owner.(*v1beta1.TaskRun); ok { stringReplacements, arrayReplacements := paramsFromTaskRun(ctx, ownerAsTR) @@ -139,22 +144,22 @@ 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, k8s kubernetes.Interface) (v1beta1.TaskObject, error) { +func resolveTask(ctx context.Context, resolver remote.Resolver, name string, kind v1beta1.TaskKind, k8s kubernetes.Interface) (v1beta1.TaskObject, *v1beta1.ConfigSource, 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, configSource, 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()) } // TODO(#5527): Consider move this function call to GetTaskData if err := verifyResolvedTask(ctx, taskObj, k8s); err != nil { - return nil, err + return nil, nil, err } - return taskObj, nil + return taskObj, configSource, nil } // readRuntimeObjectAsTask tries to convert a generic runtime.Object @@ -179,27 +184,29 @@ 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: if we want to set source for in-cluster task, set it here. +// https://github.com/tektoncd/pipeline/issues/5522 +func (l *LocalTaskRefResolver) GetTask(ctx context.Context, name string) (v1beta1.TaskObject, *v1beta1.ConfigSource, 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, err + return nil, nil, err } if err := verifyResolvedTask(ctx, task, l.K8sclient); err != nil { - return nil, err + return nil, nil, err } - return task, nil + return task, nil, nil } // IsGetTaskErrTransient returns true if an error returned by GetTask is retryable. diff --git a/pkg/reconciler/taskrun/resources/taskref_test.go b/pkg/reconciler/taskrun/resources/taskref_test.go index 5746ef1113a..034b6dcf43a 100644 --- a/pkg/reconciler/taskrun/resources/taskref_test.go +++ b/pkg/reconciler/taskrun/resources/taskref_test.go @@ -73,6 +73,13 @@ var ( }}, }, } + sampleConfigSource = &v1beta1.ConfigSource{ + URI: "abc.com", + Digest: map[string]string{ + "sha1": "a123", + }, + EntryPoint: "foo/bar", + } ) func TestGetTaskKind(t *testing.T) { @@ -205,7 +212,7 @@ func TestLocalTaskRef(t *testing.T) { Tektonclient: tektonclient, } - task, err := lc.GetTask(ctx, tc.ref.Name) + task, configSource, 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 { @@ -215,6 +222,11 @@ func TestLocalTaskRef(t *testing.T) { if d := cmp.Diff(task, tc.expected); tc.expected != nil && d != "" { t.Error(diff.PrintWantGot(d)) } + + // local cluster tasks have empty source for now. This may be changed in future. + if configSource != nil { + t.Errorf("expected configsource is nil, but got %v", configSource) + } }) } } @@ -438,7 +450,7 @@ func TestGetTaskFunc(t *testing.T) { t.Fatalf("failed to get task fn: %s", err.Error()) } - task, err := fn(ctx, tc.ref.Name) + task, configSource, err := fn(ctx, tc.ref.Name) if err != nil { t.Fatalf("failed to call taskfn: %s", err.Error()) } @@ -446,6 +458,11 @@ func TestGetTaskFunc(t *testing.T) { if diff := cmp.Diff(task, tc.expected); tc.expected != nil && diff != "" { t.Error(diff) } + + // local cluster task and bundle task have empty source for now. This may be changed in future. + if configSource != nil { + t.Errorf("expected configsource is nil, but got %v", configSource) + } }) } } @@ -472,6 +489,7 @@ echo hello `, }}, } + TaskRun := &v1beta1.TaskRun{ ObjectMeta: metav1.ObjectMeta{Namespace: "default"}, Spec: v1beta1.TaskRunSpec{ @@ -484,6 +502,9 @@ echo hello }, Status: v1beta1.TaskRunStatus{TaskRunStatusFields: v1beta1.TaskRunStatusFields{ TaskSpec: &TaskSpec, + Provenance: &v1beta1.Provenance{ + ConfigSource: sampleConfigSource.DeepCopy(), + }, }}, } expectedTask := &v1beta1.Task{ @@ -498,7 +519,7 @@ echo hello if err != nil { t.Fatalf("failed to get Task fn: %s", err.Error()) } - actualTask, err := fn(ctx, name) + actualTask, actualConfigSource, err := fn(ctx, name) if err != nil { t.Fatalf("failed to call Taskfn: %s", err.Error()) } @@ -506,6 +527,11 @@ echo hello if diff := cmp.Diff(actualTask, expectedTask); expectedTask != nil && diff != "" { t.Error(diff) } + + if d := cmp.Diff(sampleConfigSource, actualConfigSource); d != "" { + t.Errorf("configSources did not match: %s", diff.PrintWantGot(d)) + } + } func TestGetTaskFunc_RemoteResolution(t *testing.T) { @@ -519,7 +545,7 @@ func TestGetTaskFunc_RemoteResolution(t *testing.T) { "apiVersion: tekton.dev/v1beta1", taskYAMLString, }, "\n") - resolved := test.NewResolvedResource([]byte(taskYAML), nil, nil) + resolved := test.NewResolvedResource([]byte(taskYAML), nil, sampleConfigSource.DeepCopy(), nil) requester := test.NewRequester(resolved, nil) tr := &v1beta1.TaskRun{ ObjectMeta: metav1.ObjectMeta{Namespace: "default"}, @@ -533,11 +559,15 @@ func TestGetTaskFunc_RemoteResolution(t *testing.T) { t.Fatalf("failed to get task fn: %s", err.Error()) } - resolvedTask, err := fn(ctx, taskRef.Name) + resolvedTask, resolvedConfigSource, err := fn(ctx, taskRef.Name) if err != nil { t.Fatalf("failed to call pipelinefn: %s", err.Error()) } + if d := cmp.Diff(sampleConfigSource, resolvedConfigSource); d != "" { + t.Errorf("configSources did not match: %s", diff.PrintWantGot(d)) + } + if d := cmp.Diff(task, resolvedTask); d != "" { t.Error(d) } @@ -565,7 +595,8 @@ func TestGetTaskFunc_RemoteResolution_ReplacedParams(t *testing.T) { "apiVersion: tekton.dev/v1beta1", taskYAMLString, }, "\n") - resolved := test.NewResolvedResource([]byte(taskYAML), nil, nil) + + resolved := test.NewResolvedResource([]byte(taskYAML), nil, sampleConfigSource.DeepCopy(), nil) requester := &test.Requester{ ResolvedResource: resolved, Params: []v1beta1.Param{{ @@ -595,7 +626,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, resolvedConfigSource, err := fn(ctx, taskRef.Name) if err != nil { t.Fatalf("failed to call pipelinefn: %s", err.Error()) } @@ -604,6 +635,10 @@ func TestGetTaskFunc_RemoteResolution_ReplacedParams(t *testing.T) { t.Error(d) } + if d := cmp.Diff(sampleConfigSource, resolvedConfigSource); d != "" { + t.Errorf("configSources did not match: %s", diff.PrintWantGot(d)) + } + taskRefNotMatching := &v1beta1.TaskRef{ ResolverRef: v1beta1.ResolverRef{ Resolver: "git", @@ -636,7 +671,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") } @@ -651,7 +686,7 @@ func TestGetPipelineFunc_RemoteResolutionInvalidData(t *testing.T) { ctx = config.ToContext(ctx, cfg) taskRef := &v1beta1.TaskRef{ResolverRef: v1beta1.ResolverRef{Resolver: "git"}} resolvesTo := []byte("INVALID YAML") - resource := test.NewResolvedResource(resolvesTo, nil, nil) + resource := test.NewResolvedResource(resolvesTo, nil, nil, nil) requester := test.NewRequester(resource, nil) tr := &v1beta1.TaskRun{ ObjectMeta: metav1.ObjectMeta{Namespace: "default"}, @@ -664,7 +699,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") } } @@ -762,7 +797,7 @@ func TestLocalTaskRef_TrustedResourceVerification_Success(t *testing.T) { Tektonclient: tektonclient, } - task, err := lc.GetTask(ctx, tc.ref.Name) + task, source, err := lc.GetTask(ctx, tc.ref.Name) if err != nil { t.Fatalf("Received unexpected error %#v", err) } @@ -770,6 +805,10 @@ func TestLocalTaskRef_TrustedResourceVerification_Success(t *testing.T) { if d := cmp.Diff(task, tc.expected); d != "" { t.Error(diff.PrintWantGot(d)) } + + if source != nil { + t.Errorf("expected source for local task is nil, but got %v", source) + } }) } } @@ -835,13 +874,16 @@ func TestLocalTaskRef_TrustedResourceVerification_Error(t *testing.T) { Tektonclient: tektonclient, } - task, err := lc.GetTask(ctx, tc.ref.Name) + task, source, err := lc.GetTask(ctx, tc.ref.Name) if err == nil || !errors.Is(err, tc.expectedErr) { t.Fatalf("Expected error %v but found %v instead", tc.expectedErr, err) } if d := cmp.Diff(task, tc.expected); d != "" { t.Error(diff.PrintWantGot(d)) } + if source != nil { + t.Errorf("expected source for local task is nil, but got %v", source) + } }) } } @@ -859,7 +901,7 @@ func TestGetTaskFunc_RemoteResolution_TrustedResourceVerification_Success(t *tes t.Fatal("fail to marshal task", err) } - resolvedUnsigned := test.NewResolvedResource(unsignedTaskBytes, nil, nil) + resolvedUnsigned := test.NewResolvedResource(unsignedTaskBytes, nil, sampleConfigSource.DeepCopy(), nil) requesterUnsigned := test.NewRequester(resolvedUnsigned, nil) signedTask, err := test.GetSignedTask(unsignedTask, signer, "signed") @@ -871,7 +913,7 @@ func TestGetTaskFunc_RemoteResolution_TrustedResourceVerification_Success(t *tes t.Fatal("fail to marshal task", err) } - resolvedSigned := test.NewResolvedResource(signedTaskBytes, nil, nil) + resolvedSigned := test.NewResolvedResource(signedTaskBytes, nil, sampleConfigSource.DeepCopy(), nil) requesterSigned := test.NewRequester(resolvedSigned, nil) tamperedTask := signedTask.DeepCopy() @@ -880,7 +922,7 @@ func TestGetTaskFunc_RemoteResolution_TrustedResourceVerification_Success(t *tes if err != nil { t.Fatal("fail to marshal task", err) } - resolvedTampered := test.NewResolvedResource(tamperedTaskBytes, nil, nil) + resolvedTampered := test.NewResolvedResource(tamperedTaskBytes, nil, sampleConfigSource.DeepCopy(), nil) requesterTampered := test.NewRequester(resolvedTampered, nil) taskRef := &v1beta1.TaskRef{ResolverRef: v1beta1.ResolverRef{Resolver: "git"}} @@ -942,7 +984,7 @@ func TestGetTaskFunc_RemoteResolution_TrustedResourceVerification_Success(t *tes t.Fatalf("failed to get task fn: %s", err.Error()) } - resolvedTask, err := fn(ctx, taskRef.Name) + resolvedTask, source, err := fn(ctx, taskRef.Name) if err != nil { t.Fatalf("Received unexpected error ( %#v )", err) @@ -951,6 +993,10 @@ func TestGetTaskFunc_RemoteResolution_TrustedResourceVerification_Success(t *tes if d := cmp.Diff(tc.expected, resolvedTask); d != "" { t.Error(d) } + + if d := cmp.Diff(sampleConfigSource, source); d != "" { + t.Errorf("configSources did not match: %s", diff.PrintWantGot(d)) + } }) } } @@ -968,7 +1014,7 @@ func TestGetTaskFunc_RemoteResolution_TrustedResourceVerification_Error(t *testi t.Fatal("fail to marshal task", err) } - resolvedUnsigned := test.NewResolvedResource(unsignedTaskBytes, nil, nil) + resolvedUnsigned := test.NewResolvedResource(unsignedTaskBytes, nil, sampleConfigSource.DeepCopy(), nil) requesterUnsigned := test.NewRequester(resolvedUnsigned, nil) signedTask, err := test.GetSignedTask(unsignedTask, signer, "signed") @@ -982,7 +1028,7 @@ func TestGetTaskFunc_RemoteResolution_TrustedResourceVerification_Error(t *testi if err != nil { t.Fatal("fail to marshal task", err) } - resolvedTampered := test.NewResolvedResource(tamperedTaskBytes, nil, nil) + resolvedTampered := test.NewResolvedResource(tamperedTaskBytes, nil, sampleConfigSource.DeepCopy(), nil) requesterTampered := test.NewRequester(resolvedTampered, nil) taskRef := &v1beta1.TaskRef{ResolverRef: v1beta1.ResolverRef{Resolver: "git"}} @@ -1022,7 +1068,7 @@ func TestGetTaskFunc_RemoteResolution_TrustedResourceVerification_Error(t *testi t.Fatalf("failed to get task fn: %s", err.Error()) } - resolvedTask, err := fn(ctx, taskRef.Name) + resolvedTask, source, err := fn(ctx, taskRef.Name) if err == nil || !errors.Is(err, tc.expectedErr) { t.Fatalf("Expected error %v but found %v instead", tc.expectedErr, err) @@ -1031,6 +1077,10 @@ func TestGetTaskFunc_RemoteResolution_TrustedResourceVerification_Error(t *testi if d := cmp.Diff(tc.expected, resolvedTask); d != "" { t.Error(d) } + + if source != nil { + t.Errorf("expected source is nil, but got: %v", source) + } }) } } diff --git a/pkg/reconciler/taskrun/resources/taskspec.go b/pkg/reconciler/taskrun/resources/taskspec.go index aa836fa6051..0f94b8bcadd 100644 --- a/pkg/reconciler/taskrun/resources/taskspec.go +++ b/pkg/reconciler/taskrun/resources/taskspec.go @@ -26,7 +26,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, *v1beta1.ConfigSource, error) // GetTaskRun is a function used to retrieve TaskRuns type GetTaskRun func(string) (*v1beta1.TaskRun, error) @@ -34,26 +34,37 @@ type GetTaskRun func(string) (*v1beta1.TaskRun, error) // GetClusterTask is a function that will retrieve the Task from name and namespace. type GetClusterTask func(name string) (v1beta1.TaskObject, error) +// ResolvedObjectMeta contains both ObjectMeta and the metadata that identifies the source where the resource came from. +type ResolvedObjectMeta struct { + *metav1.ObjectMeta `json:",omitempty"` + // ConfigSource identifies where the spec came from. + ConfigSource *v1beta1.ConfigSource `json:",omitempty"` +} + // GetTaskData will retrieve the Task metadata and Spec associated with the // provided TaskRun. This can come from a reference Task or from the TaskRun's // metadata and embedded TaskSpec. -func GetTaskData(ctx context.Context, taskRun *v1beta1.TaskRun, getTask GetTask) (*metav1.ObjectMeta, *v1beta1.TaskSpec, error) { +func GetTaskData(ctx context.Context, taskRun *v1beta1.TaskRun, getTask GetTask) (*ResolvedObjectMeta, *v1beta1.TaskSpec, error) { taskMeta := metav1.ObjectMeta{} + var configSource *v1beta1.ConfigSource taskSpec := v1beta1.TaskSpec{} switch { case taskRun.Spec.TaskRef != nil && taskRun.Spec.TaskRef.Name != "": // Get related task for taskrun - t, err := getTask(ctx, taskRun.Spec.TaskRef.Name) + t, source, 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() + configSource = source case taskRun.Spec.TaskSpec != nil: taskMeta = taskRun.ObjectMeta taskSpec = *taskRun.Spec.TaskSpec + // TODO: if we want to set source for embedded taskspec, set it here. + // https://github.com/tektoncd/pipeline/issues/5522 case taskRun.Spec.TaskRef != nil && taskRun.Spec.TaskRef.Resolver != "": - task, err := getTask(ctx, taskRun.Name) + task, source, err := getTask(ctx, taskRun.Name) switch { case err != nil: return nil, nil, err @@ -63,10 +74,14 @@ func GetTaskData(ctx context.Context, taskRun *v1beta1.TaskRun, getTask GetTask) taskMeta = task.TaskMetadata() taskSpec = task.TaskSpec() } + configSource = source default: return nil, nil, fmt.Errorf("taskRun %s not providing TaskRef or TaskSpec", taskRun.Name) } taskSpec.SetDefaults(ctx) - return &taskMeta, &taskSpec, nil + return &ResolvedObjectMeta{ + ObjectMeta: &taskMeta, + ConfigSource: configSource, + }, &taskSpec, nil } diff --git a/pkg/reconciler/taskrun/resources/taskspec_test.go b/pkg/reconciler/taskrun/resources/taskspec_test.go index 6279f36c2de..0b4e22cd1f6 100644 --- a/pkg/reconciler/taskrun/resources/taskspec_test.go +++ b/pkg/reconciler/taskrun/resources/taskspec_test.go @@ -27,6 +27,16 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) +var ( + sampleConfigSource = &v1beta1.ConfigSource{ + URI: "abc.com", + Digest: map[string]string{ + "sha1": "a123", + }, + EntryPoint: "foo/bar", + } +) + func TestGetTaskSpec_Ref(t *testing.T) { task := &v1beta1.Task{ ObjectMeta: metav1.ObjectMeta{ @@ -48,20 +58,26 @@ func TestGetTaskSpec_Ref(t *testing.T) { }, }, } - gt := func(ctx context.Context, n string) (v1beta1.TaskObject, error) { return task, nil } - taskMeta, taskSpec, err := GetTaskData(context.Background(), tr, gt) + + gt := func(ctx context.Context, n string) (v1beta1.TaskObject, *v1beta1.ConfigSource, error) { + return task, sampleConfigSource.DeepCopy(), nil + } + resolvedObjectMeta, taskSpec, err := GetTaskData(context.Background(), tr, gt) if err != nil { t.Fatalf("Did not expect error getting task spec but got: %s", err) } - if taskMeta.Name != "orchestrate" { - t.Errorf("Expected task name to be `orchestrate` but was %q", taskMeta.Name) + if resolvedObjectMeta.Name != "orchestrate" { + t.Errorf("Expected task name to be `orchestrate` but was %q", resolvedObjectMeta.Name) } if len(taskSpec.Steps) != 1 || taskSpec.Steps[0].Name != "step1" { t.Errorf("Task Spec not resolved as expected, expected referenced Task spec but got: %v", taskSpec) } + if d := cmp.Diff(sampleConfigSource, resolvedObjectMeta.ConfigSource); d != "" { + t.Errorf("configsource did not match: %s", diff.PrintWantGot(d)) + } } func TestGetTaskSpec_Embedded(t *testing.T) { @@ -77,22 +93,27 @@ func TestGetTaskSpec_Embedded(t *testing.T) { }, }, } - gt := func(ctx context.Context, n string) (v1beta1.TaskObject, error) { - return nil, errors.New("shouldn't be called") + gt := func(ctx context.Context, n string) (v1beta1.TaskObject, *v1beta1.ConfigSource, error) { + return nil, nil, errors.New("shouldn't be called") } - taskMeta, taskSpec, err := GetTaskData(context.Background(), tr, gt) + resolvedObjectMeta, taskSpec, err := GetTaskData(context.Background(), tr, gt) if err != nil { t.Fatalf("Did not expect error getting task spec but got: %s", err) } - if taskMeta.Name != "mytaskrun" { - t.Errorf("Expected task name for embedded task to default to name of task run but was %q", taskMeta.Name) + if resolvedObjectMeta.Name != "mytaskrun" { + t.Errorf("Expected task name for embedded task to default to name of task run but was %q", resolvedObjectMeta.Name) } if len(taskSpec.Steps) != 1 || taskSpec.Steps[0].Name != "step1" { t.Errorf("Task Spec not resolved as expected, expected embedded Task spec but got: %v", taskSpec) } + + // embedded tasks have empty source for now. This may be changed in future. + if resolvedObjectMeta.ConfigSource != nil { + t.Errorf("resolved configsource for embedded task is expected to be empty, but got %v", resolvedObjectMeta.ConfigSource) + } } func TestGetTaskSpec_Invalid(t *testing.T) { @@ -101,8 +122,8 @@ func TestGetTaskSpec_Invalid(t *testing.T) { Name: "mytaskrun", }, } - gt := func(ctx context.Context, n string) (v1beta1.TaskObject, error) { - return nil, errors.New("shouldn't be called") + gt := func(ctx context.Context, n string) (v1beta1.TaskObject, *v1beta1.ConfigSource, error) { + return nil, nil, errors.New("shouldn't be called") } _, _, err := GetTaskData(context.Background(), tr, gt) if err == nil { @@ -121,8 +142,8 @@ func TestGetTaskSpec_Error(t *testing.T) { }, }, } - gt := func(ctx context.Context, n string) (v1beta1.TaskObject, error) { - return nil, errors.New("something went wrong") + gt := func(ctx context.Context, n string) (v1beta1.TaskObject, *v1beta1.ConfigSource, error) { + return nil, nil, errors.New("something went wrong") } _, _, err := GetTaskData(context.Background(), tr, gt) if err == nil { @@ -160,11 +181,12 @@ func TestGetTaskData_ResolutionSuccess(t *testing.T) { Script: `echo "hello world!"`, }}, } - getTask := func(ctx context.Context, n string) (v1beta1.TaskObject, error) { + + getTask := func(ctx context.Context, n string) (v1beta1.TaskObject, *v1beta1.ConfigSource, error) { return &v1beta1.Task{ ObjectMeta: *sourceMeta.DeepCopy(), Spec: *sourceSpec.DeepCopy(), - }, nil + }, sampleConfigSource.DeepCopy(), nil } ctx := context.Background() resolvedMeta, resolvedSpec, err := GetTaskData(ctx, tr, getTask) @@ -174,6 +196,11 @@ func TestGetTaskData_ResolutionSuccess(t *testing.T) { if sourceMeta.Name != resolvedMeta.Name { t.Errorf("Expected name %q but resolved to %q", sourceMeta.Name, resolvedMeta.Name) } + + if d := cmp.Diff(sampleConfigSource, resolvedMeta.ConfigSource); d != "" { + t.Errorf("configsource did not match: %s", diff.PrintWantGot(d)) + } + if d := cmp.Diff(sourceSpec, *resolvedSpec); d != "" { t.Errorf(diff.PrintWantGot(d)) } @@ -192,8 +219,8 @@ func TestGetPipelineData_ResolutionError(t *testing.T) { }, }, } - getTask := func(ctx context.Context, n string) (v1beta1.TaskObject, error) { - return nil, errors.New("something went wrong") + getTask := func(ctx context.Context, n string) (v1beta1.TaskObject, *v1beta1.ConfigSource, error) { + return nil, nil, errors.New("something went wrong") } ctx := context.Background() _, _, err := GetTaskData(ctx, tr, getTask) @@ -215,8 +242,8 @@ func TestGetTaskData_ResolvedNilTask(t *testing.T) { }, }, } - getTask := func(ctx context.Context, n string) (v1beta1.TaskObject, error) { - return nil, nil + getTask := func(ctx context.Context, n string) (v1beta1.TaskObject, *v1beta1.ConfigSource, error) { + return nil, nil, nil } ctx := context.Background() _, _, err := GetTaskData(ctx, tr, getTask) diff --git a/pkg/reconciler/taskrun/taskrun.go b/pkg/reconciler/taskrun/taskrun.go index acde4d95d39..0d04c4c53f7 100644 --- a/pkg/reconciler/taskrun/taskrun.go +++ b/pkg/reconciler/taskrun/taskrun.go @@ -914,10 +914,14 @@ func applyVolumeClaimTemplates(workspaceBindings []v1beta1.WorkspaceBinding, own return taskRunWorkspaceBindings } -func storeTaskSpecAndMergeMeta(tr *v1beta1.TaskRun, ts *v1beta1.TaskSpec, meta *metav1.ObjectMeta) error { +func storeTaskSpecAndMergeMeta(tr *v1beta1.TaskRun, ts *v1beta1.TaskSpec, meta *resources.ResolvedObjectMeta) error { // Only store the TaskSpec once, if it has never been set before. if tr.Status.TaskSpec == nil { tr.Status.TaskSpec = ts + if meta == nil { + return nil + } + // Propagate annotations from Task to TaskRun. TaskRun annotations take precedences over Task. tr.ObjectMeta.Annotations = kmap.Union(meta.Annotations, tr.ObjectMeta.Annotations) // Propagate labels from Task to TaskRun. TaskRun labels take precedences over Task. @@ -930,6 +934,17 @@ func storeTaskSpecAndMergeMeta(tr *v1beta1.TaskRun, ts *v1beta1.TaskSpec, meta * } } } + + // Propagate ConfigSource from remote resolution to TaskRun Status + // This lives outside of the status.spec check to avoid the case where only the spec is available in the first reconcile and source comes in next reconcile. + if meta != nil && meta.ConfigSource != nil { + if tr.Status.Provenance == nil { + tr.Status.Provenance = &v1beta1.Provenance{} + } + if tr.Status.Provenance.ConfigSource == nil { + tr.Status.Provenance.ConfigSource = meta.ConfigSource + } + } return nil } diff --git a/pkg/reconciler/taskrun/taskrun_test.go b/pkg/reconciler/taskrun/taskrun_test.go index aec1fd8e598..2724fc1bd45 100644 --- a/pkg/reconciler/taskrun/taskrun_test.go +++ b/pkg/reconciler/taskrun/taskrun_test.go @@ -4056,7 +4056,7 @@ status: } } -func Test_storeTaskSpec(t *testing.T) { +func Test_storeTaskSpecAndConfigSource(t *testing.T) { tr := parse.MustParseV1beta1TaskRun(t, ` metadata: annotations: @@ -4069,34 +4069,94 @@ spec: name: foo-task `) + configSource := &v1beta1.ConfigSource{ + URI: "https://abc.com.git", + Digest: map[string]string{ + "sha1": "xyz", + }, + EntryPoint: "foo/bar", + } + ts := v1beta1.TaskSpec{ Description: "foo-task", } ts1 := v1beta1.TaskSpec{ Description: "bar-task", } + want := tr.DeepCopy() want.Status = v1beta1.TaskRunStatus{ TaskRunStatusFields: v1beta1.TaskRunStatusFields{ TaskSpec: ts.DeepCopy(), + Provenance: &v1beta1.Provenance{ + ConfigSource: configSource.DeepCopy(), + }, }, } want.ObjectMeta.Labels["tekton.dev/task"] = tr.ObjectMeta.Name - // The first time we set it, it should get copied. - if err := storeTaskSpecAndMergeMeta(tr, &ts, &tr.ObjectMeta); err != nil { - t.Errorf("storeTaskSpec() error = %v", err) - } - if d := cmp.Diff(tr, want); d != "" { - t.Fatalf(diff.PrintWantGot(d)) + type args struct { + taskSpec *v1beta1.TaskSpec + resolvedObjectMeta *resources.ResolvedObjectMeta } - // The next time, it should not get overwritten - if err := storeTaskSpecAndMergeMeta(tr, &ts1, &metav1.ObjectMeta{}); err != nil { - t.Errorf("storeTaskSpec() error = %v", err) + var tests = []struct { + name string + reconcile1Args *args + reconcile2Args *args + wantTaskRun *v1beta1.TaskRun + }{ + { + name: "spec and source are available in the same reconcile", + reconcile1Args: &args{ + taskSpec: &ts, + resolvedObjectMeta: &resources.ResolvedObjectMeta{ + ObjectMeta: &tr.ObjectMeta, + ConfigSource: configSource.DeepCopy(), + }, + }, + reconcile2Args: &args{ + taskSpec: &ts1, + resolvedObjectMeta: &resources.ResolvedObjectMeta{}, + }, + wantTaskRun: want, + }, + { + name: "spec comes in the first reconcile and source comes in next reconcile", + reconcile1Args: &args{ + taskSpec: &ts, + resolvedObjectMeta: &resources.ResolvedObjectMeta{ + ObjectMeta: &tr.ObjectMeta, + }, + }, + reconcile2Args: &args{ + taskSpec: &ts, + resolvedObjectMeta: &resources.ResolvedObjectMeta{ + ConfigSource: configSource.DeepCopy(), + }, + }, + wantTaskRun: want, + }, } - if d := cmp.Diff(tr, want); d != "" { - t.Fatalf(diff.PrintWantGot(d)) + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + // mock first reconcile + if err := storeTaskSpecAndMergeMeta(tr, tc.reconcile1Args.taskSpec, tc.reconcile1Args.resolvedObjectMeta); err != nil { + t.Errorf("storePipelineSpec() error = %v", err) + } + if d := cmp.Diff(tr, tc.wantTaskRun); d != "" { + t.Fatalf(diff.PrintWantGot(d)) + } + + // mock second reconcile + if err := storeTaskSpecAndMergeMeta(tr, tc.reconcile2Args.taskSpec, tc.reconcile2Args.resolvedObjectMeta); err != nil { + t.Errorf("storePipelineSpec() error = %v", err) + } + if d := cmp.Diff(tr, tc.wantTaskRun); d != "" { + t.Fatalf(diff.PrintWantGot(d)) + } + }) } } @@ -4111,8 +4171,11 @@ func Test_storeTaskSpec_metadata(t *testing.T) { tr := &v1beta1.TaskRun{ ObjectMeta: metav1.ObjectMeta{Name: "foo", Labels: taskrunlabels, Annotations: taskrunannotations}, } - meta := metav1.ObjectMeta{Labels: tasklabels, Annotations: taskannotations} - if err := storeTaskSpecAndMergeMeta(tr, &v1beta1.TaskSpec{}, &meta); err != nil { + resolvedMeta := resources.ResolvedObjectMeta{ + ObjectMeta: &metav1.ObjectMeta{Labels: tasklabels, Annotations: taskannotations}, + } + + if err := storeTaskSpecAndMergeMeta(tr, &v1beta1.TaskSpec{}, &resolvedMeta); err != nil { t.Errorf("storeTaskSpecAndMergeMeta error = %v", err) } if d := cmp.Diff(tr.ObjectMeta.Labels, wantedlabels); d != "" { diff --git a/pkg/remote/oci/resolver.go b/pkg/remote/oci/resolver.go index 6a837ca7468..af49f4bd703 100644 --- a/pkg/remote/oci/resolver.go +++ b/pkg/remote/oci/resolver.go @@ -29,6 +29,7 @@ import ( imgname "github.com/google/go-containerregistry/pkg/name" v1 "github.com/google/go-containerregistry/pkg/v1" ociremote "github.com/google/go-containerregistry/pkg/v1/remote" + "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1" "github.com/tektoncd/pipeline/pkg/client/clientset/versioned/scheme" "github.com/tektoncd/pipeline/pkg/remote" "k8s.io/apimachinery/pkg/runtime" @@ -89,33 +90,33 @@ func (o *Resolver) List(ctx context.Context) ([]remote.ResolvedObject, error) { } // Get retrieves a specific object with the given Kind and name -func (o *Resolver) Get(ctx context.Context, kind, name string) (runtime.Object, error) { +func (o *Resolver) Get(ctx context.Context, kind, name string) (runtime.Object, *v1beta1.ConfigSource, error) { timeoutCtx, cancel := context.WithTimeout(ctx, o.timeout) defer cancel() img, err := o.retrieveImage(timeoutCtx) if err != nil { - return nil, err + return nil, nil, err } manifest, err := img.Manifest() if err != nil { - return nil, fmt.Errorf("could not parse image manifest: %w", err) + return nil, nil, fmt.Errorf("could not parse image manifest: %w", err) } if err := o.checkImageCompliance(manifest); err != nil { - return nil, err + return nil, nil, err } layers, err := img.Layers() if err != nil { - return nil, fmt.Errorf("could not read image layers: %w", err) + return nil, nil, fmt.Errorf("could not read image layers: %w", err) } layerMap := map[string]v1.Layer{} for _, l := range layers { digest, err := l.Digest() if err != nil { - return nil, fmt.Errorf("failed to find digest for layer: %w", err) + return nil, nil, fmt.Errorf("failed to find digest for layer: %w", err) } layerMap[digest.String()] = l } @@ -128,12 +129,13 @@ func (o *Resolver) Get(ctx context.Context, kind, name string) (runtime.Object, obj, err := readTarLayer(layerMap[l.Digest.String()]) if err != nil { // This could still be a raw layer so try to read it as that instead. - return readRawLayer(layers[idx]) + obj, err := readRawLayer(layers[idx]) + return obj, nil, err } - return obj, nil + return obj, nil, nil } } - return nil, fmt.Errorf("could not find object in image with kind: %s and name: %s", kind, name) + return nil, nil, fmt.Errorf("could not find object in image with kind: %s and name: %s", kind, name) } // retrieveImage will fetch the image's contents and manifest. diff --git a/pkg/remote/oci/resolver_test.go b/pkg/remote/oci/resolver_test.go index c1501fa5b7a..e0fb849dcd6 100644 --- a/pkg/remote/oci/resolver_test.go +++ b/pkg/remote/oci/resolver_test.go @@ -204,7 +204,7 @@ func TestOCIResolver(t *testing.T) { } for _, obj := range tc.objs { - actual, err := resolver.Get(context.Background(), strings.ToLower(obj.GetObjectKind().GroupVersionKind().Kind), getObjectName(obj)) + actual, source, err := resolver.Get(context.Background(), strings.ToLower(obj.GetObjectKind().GroupVersionKind().Kind), getObjectName(obj)) if err != nil { t.Fatalf("could not retrieve object from image: %#v", err) } @@ -212,6 +212,10 @@ func TestOCIResolver(t *testing.T) { if d := cmp.Diff(actual, obj); d != "" { t.Error(diff.PrintWantGot(d)) } + + if source != nil { + t.Errorf("expected source is nil, but received %v", source) + } } }) } diff --git a/pkg/remote/resolution/resolver.go b/pkg/remote/resolution/resolver.go index fb79f9e14f9..b8062daf940 100644 --- a/pkg/remote/resolution/resolver.go +++ b/pkg/remote/resolution/resolver.go @@ -57,31 +57,31 @@ func NewResolver(requester remoteresource.Requester, owner kmeta.OwnerRefable, r } // Get implements remote.Resolver. -func (resolver *Resolver) Get(ctx context.Context, _, _ string) (runtime.Object, error) { +func (resolver *Resolver) Get(ctx context.Context, _, _ string) (runtime.Object, *v1beta1.ConfigSource, error) { resolverName := remoteresource.ResolverName(resolver.resolverName) req, err := buildRequest(resolver.resolverName, resolver.owner, resolver.targetName, resolver.targetNamespace, resolver.params) if err != nil { - return nil, fmt.Errorf("error building request for remote resource: %w", err) + return nil, nil, fmt.Errorf("error building request for remote resource: %w", err) } resolved, err := resolver.requester.Submit(ctx, resolverName, req) switch { case errors.Is(err, resolutioncommon.ErrorRequestInProgress): - return nil, remote.ErrorRequestInProgress + return nil, nil, remote.ErrorRequestInProgress case err != nil: - return nil, fmt.Errorf("error requesting remote resource: %w", err) + return nil, nil, fmt.Errorf("error requesting remote resource: %w", err) case resolved == nil: - return nil, ErrorRequestedResourceIsNil + return nil, nil, ErrorRequestedResourceIsNil default: } data, err := resolved.Data() if err != nil { - return nil, &ErrorAccessingData{original: err} + return nil, nil, &ErrorAccessingData{original: err} } obj, _, err := scheme.Codecs.UniversalDeserializer().Decode(data, nil, nil) if err != nil { - return nil, &ErrorInvalidRuntimeObject{original: err} + return nil, nil, &ErrorInvalidRuntimeObject{original: err} } - return obj, nil + return obj, resolved.Source(), nil } // List implements remote.Resolver but is unused for remote resolution. diff --git a/pkg/remote/resolution/resolver_test.go b/pkg/remote/resolution/resolver_test.go index 1ff6da4b4c7..148c8268877 100644 --- a/pkg/remote/resolution/resolver_test.go +++ b/pkg/remote/resolution/resolver_test.go @@ -69,7 +69,7 @@ func TestGet_Successful(t *testing.T) { ResolvedResource: resolved, } resolver := NewResolver(requester, owner, "git", "", "", nil) - if _, err := resolver.Get(ctx, "foo", "bar"); err != nil { + if _, _, err := resolver.Get(ctx, "foo", "bar"); err != nil { t.Fatalf("unexpected error: %v", err) } @@ -123,10 +123,13 @@ func TestGet_Errors(t *testing.T) { ResolvedResource: tc.resolvedResource, } resolver := NewResolver(requester, owner, "git", "", "", nil) - obj, err := resolver.Get(ctx, "foo", "bar") + obj, source, err := resolver.Get(ctx, "foo", "bar") if obj != nil { t.Errorf("received unexpected resolved resource") } + if source != nil { + t.Errorf("expected source is nil, but received %v", source) + } if !errors.Is(err, tc.expectedGetErr) { t.Fatalf("expected %v received %v", tc.expectedGetErr, err) } diff --git a/pkg/remote/resolver.go b/pkg/remote/resolver.go index 11bc5877ad7..8f706f565f8 100644 --- a/pkg/remote/resolver.go +++ b/pkg/remote/resolver.go @@ -16,6 +16,7 @@ package remote import ( "context" + "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1" "k8s.io/apimachinery/pkg/runtime" ) @@ -28,8 +29,8 @@ type ResolvedObject struct { // Resolver defines a generic API to retrieve Tekton resources from remote locations. It allows 2 principle operations: // - List: retrieve a flat set of Tekton objects in this remote location -// - Get: retrieves a specific object with the given Kind and name. +// - Get: retrieves a specific object with the given Kind and name, and the source identifying where the resource came from. type Resolver interface { List(ctx context.Context) ([]ResolvedObject, error) - Get(ctx context.Context, kind, name string) (runtime.Object, error) + Get(ctx context.Context, kind, name string) (runtime.Object, *v1beta1.ConfigSource, error) } diff --git a/test/resolution.go b/test/resolution.go index 376b91e374c..43a7380d582 100644 --- a/test/resolution.go +++ b/test/resolution.go @@ -7,6 +7,7 @@ import ( "strings" "github.com/google/go-cmp/cmp" + "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1" pipelinev1beta1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1" resolution "github.com/tektoncd/pipeline/pkg/resolution/resource" "github.com/tektoncd/pipeline/test/diff" @@ -27,10 +28,11 @@ func NewRequester(resource resolution.ResolvedResource, err error) *Requester { // NewResolvedResource creates a mock resolved resource that is // populated with the given data and annotations or returns the given // error from its Data() method. -func NewResolvedResource(data []byte, annotations map[string]string, dataErr error) *ResolvedResource { +func NewResolvedResource(data []byte, annotations map[string]string, source *v1beta1.ConfigSource, dataErr error) *ResolvedResource { return &ResolvedResource{ ResolvedData: data, ResolvedAnnotations: annotations, + ResolvedSource: source, DataErr: dataErr, } }