diff --git a/pkg/reconciler/pipelinerun/pipelinerun.go b/pkg/reconciler/pipelinerun/pipelinerun.go index 568c456bd9e..c975f22869b 100644 --- a/pkg/reconciler/pipelinerun/pipelinerun.go +++ b/pkg/reconciler/pipelinerun/pipelinerun.go @@ -392,7 +392,7 @@ func (c *Reconciler) reconcile(ctx context.Context, pr *v1beta1.PipelineRun, get return controller.NewPermanentError(err) default: // Store the fetched PipelineSpec on the PipelineRun for auditing - if err := storePipelineSpecAndMergeMeta(pr, pipelineSpec, pipelineMeta); err != nil { + if err := storePipelineSpecAndMergeMeta(ctx, pr, pipelineSpec, pipelineMeta); err != nil { logger.Errorf("Failed to store PipelineSpec on PipelineRun.Status for pipelinerun %s: %v", pr.Name, err) } } @@ -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(ctx context.Context, 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,19 @@ 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. + cfg := config.FromContextOrDefaults(ctx) + if cfg.FeatureFlags.EnableProvenanceInStatus && 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..59df92f4a80 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,75 @@ 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) { + ctx := ttesting.EnableFeatureFlagField(context.Background(), t, "enable-provenance-in-status") + // mock first reconcile + if err := storePipelineSpecAndMergeMeta(ctx, 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(ctx, 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 +5632,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(context.Background(), 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 2afc22097e1..0566eff244c 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 823357d93f7..0601929eeb6 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..66cd3c9d792 100644 --- a/pkg/reconciler/taskrun/taskrun.go +++ b/pkg/reconciler/taskrun/taskrun.go @@ -347,7 +347,7 @@ func (c *Reconciler) prepare(ctx context.Context, tr *v1beta1.TaskRun) (*v1beta1 return nil, nil, controller.NewPermanentError(err) default: // Store the fetched TaskSpec on the TaskRun for auditing - if err := storeTaskSpecAndMergeMeta(tr, taskSpec, taskMeta); err != nil { + if err := storeTaskSpecAndMergeMeta(ctx, tr, taskSpec, taskMeta); err != nil { logger.Errorf("Failed to store TaskSpec on TaskRun.Statusfor taskrun %s: %v", tr.Name, err) } } @@ -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(ctx context.Context, 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,18 @@ 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. + cfg := config.FromContextOrDefaults(ctx) + if cfg.FeatureFlags.EnableProvenanceInStatus && 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..9d0e33c6de6 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,95 @@ 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) { + ctx := ttesting.EnableFeatureFlagField(context.Background(), t, "enable-provenance-in-status") + // mock first reconcile + if err := storeTaskSpecAndMergeMeta(ctx, 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(ctx, 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 +4172,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(context.Background(), 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/reconciler/testing/configmap.go b/pkg/reconciler/testing/configmap.go index 2a53124c2db..4a0c9f82626 100644 --- a/pkg/reconciler/testing/configmap.go +++ b/pkg/reconciler/testing/configmap.go @@ -17,10 +17,12 @@ limitations under the License. package testing import ( + "context" "fmt" "io/ioutil" "testing" + "github.com/tektoncd/pipeline/pkg/apis/config" corev1 "k8s.io/api/core/v1" "sigs.k8s.io/yaml" ) @@ -45,3 +47,19 @@ func ConfigMapFromTestFile(t *testing.T, name string) *corev1.ConfigMap { return &cm } + +// EnableFeatureFlagField enables a boolean feature flag in an existing context (for use in testing). +func EnableFeatureFlagField(ctx context.Context, t *testing.T, flagName string) context.Context { + featureFlags, err := config.NewFeatureFlagsFromMap(map[string]string{ + flagName: "true", + }) + + if err != nil { + t.Fatalf("Fail to create a feature config: %v", err) + } + + cfg := &config.Config{ + FeatureFlags: featureFlags, + } + return config.ToContext(ctx, cfg) +} 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 fb4611b6a32..a2d3e89c6cf 100644 --- a/pkg/remote/oci/resolver_test.go +++ b/pkg/remote/oci/resolver_test.go @@ -203,7 +203,7 @@ func TestOCIResolver(t *testing.T) { } for _, obj := range tc.objs { - actual, err := resolver.Get(context.Background(), strings.ToLower(obj.GetObjectKind().GroupVersionKind().Kind), test.GetObjectName(obj)) + actual, source, err := resolver.Get(context.Background(), strings.ToLower(obj.GetObjectKind().GroupVersionKind().Kind), test.GetObjectName(obj)) if err != nil { t.Fatalf("could not retrieve object from image: %#v", err) } @@ -211,6 +211,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, } }