From fbd86f88e792d5316c29b0cdc6e0883297143573 Mon Sep 17 00:00:00 2001 From: Chuang Wang Date: Tue, 30 Aug 2022 16:25:34 -0700 Subject: [PATCH] gitresolver: Pass ConfigSource from ResolutionRequest.Status to *Run.Status. Related to https://github.com/tektoncd/pipeline/issues/5522 Change 1: - Before: `ResolutionRequest.Status.ConfigSource` field was empty for the gitresolver. - Now: - `ResolutionRequest.Status.ConfigSource.URI` is set to be the user-provided URI for Anonymous Cloning, and is set to be the Repo's clone url for SCM API cloning. - `ResolutionRequest.Status.ConfigSource.Digest` is filled with one entry: the key is "sha1", and the value is the actual commit sha at the time of the gitresolver resolving the remote resource. - `ResolutionRequest.Status.ConfigSource.EntryPoint` is set to be the path of the task/pipeline yaml file. Change 2: - Before: The `ConfigSource` information in `ResolutionRequest` was not passed to pipeline/task reconciler. - Now: The `ConfigSource` information is not passed to pipeline/task reconciler and finally stored in pipelinerun/taskrun status.Provenance.ConfigSource field. Motivation: See https://github.com/tektoncd/pipeline/issues/5522 for details The tl;dr is that Tekton Chains needs to capture the source of the config file (pipeline.yaml or task.yaml) in the SLSA provenance's `predicate.invocation.configSource` field. Therefore, we need to pass the ConfigSource information from resolvers and to make it available in TaskRun/PipelineRun status. Signed-off-by: Chuang Wang --- pkg/reconciler/pipelinerun/pipelinerun.go | 56 ++++++--------- .../pipelinerun/pipelinerun_test.go | 18 ++++- .../pipelinerun/pipelinespec/pipelinespec.go | 23 +++--- .../pipelinespec/pipelinespec_test.go | 60 +++++++++++----- .../pipelinerun/resources/pipelineref.go | 28 ++++---- .../pipelinerun/resources/pipelineref_test.go | 49 ++++++++++--- .../resources/pipelinerunresolution.go | 2 +- .../resources/pipelinerunresolution_test.go | 48 +++++++++---- pkg/reconciler/taskrun/resources/taskref.go | 33 +++++---- .../taskrun/resources/taskref_test.go | 31 +++++--- pkg/reconciler/taskrun/resources/taskspec.go | 23 ++++-- .../taskrun/resources/taskspec_test.go | 71 ++++++++++++++----- pkg/reconciler/taskrun/taskrun.go | 43 +++++------ pkg/reconciler/taskrun/taskrun_test.go | 35 +++++++-- pkg/remote/oci/resolver.go | 21 +++--- pkg/remote/oci/resolver_test.go | 2 +- pkg/remote/resolution/resolver.go | 16 ++--- pkg/remote/resolution/resolver_test.go | 4 +- pkg/remote/resolver.go | 6 +- test/resolution.go | 4 +- 20 files changed, 367 insertions(+), 206 deletions(-) diff --git a/pkg/reconciler/pipelinerun/pipelinerun.go b/pkg/reconciler/pipelinerun/pipelinerun.go index 5f82dd61f2f..843c3cfb72e 100644 --- a/pkg/reconciler/pipelinerun/pipelinerun.go +++ b/pkg/reconciler/pipelinerun/pipelinerun.go @@ -365,7 +365,7 @@ func (c *Reconciler) reconcile(ctx context.Context, pr *v1beta1.PipelineRun, get return nil } - pipelineMeta, pipelineSpec, err := rprp.GetPipelineData(ctx, pr, getPipelineFunc) + resolvedObjectMeta, pipelineSpec, err := rprp.GetPipelineData(ctx, pr, getPipelineFunc) switch { case errors.Is(err, remote.ErrorRequestInProgress): message := fmt.Sprintf("PipelineRun %s/%s awaiting remote resource", pr.Namespace, pr.Name) @@ -379,7 +379,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(pr, pipelineSpec, resolvedObjectMeta); err != nil { logger.Errorf("Failed to store PipelineSpec on PipelineRun.Status for pipelinerun %s: %v", pr.Name, err) } } @@ -413,7 +413,7 @@ func (c *Reconciler) reconcile(ctx context.Context, pr *v1beta1.PipelineRun, get // This Run has failed, so we need to mark it as failed and stop reconciling it pr.Status.MarkFailed(ReasonFailedValidation, "Pipeline %s/%s can't be Run; it has an invalid spec: %s", - pipelineMeta.Namespace, pipelineMeta.Name, err) + resolvedObjectMeta.Namespace, resolvedObjectMeta.Name, err) return controller.NewPermanentError(err) } @@ -421,7 +421,7 @@ func (c *Reconciler) reconcile(ctx context.Context, pr *v1beta1.PipelineRun, get // This Run has failed, so we need to mark it as failed and stop reconciling it pr.Status.MarkFailed(ReasonInvalidBindings, "PipelineRun %s/%s doesn't bind Pipeline %s/%s's PipelineResources correctly: %s", - pr.Namespace, pr.Name, pr.Namespace, pipelineMeta.Name, err) + pr.Namespace, pr.Name, pr.Namespace, resolvedObjectMeta.Name, err) return controller.NewPermanentError(err) } providedResources, err := resources.GetResourcesFromBindings(pr, c.resourceLister.PipelineResources(pr.Namespace).Get) @@ -438,7 +438,7 @@ func (c *Reconciler) reconcile(ctx context.Context, pr *v1beta1.PipelineRun, get // This Run has failed, so we need to mark it as failed and stop reconciling it pr.Status.MarkFailed(ReasonCouldntGetResource, "PipelineRun %s/%s can't be Run; it tries to bind Resources that don't exist: %s", - pipelineMeta.Namespace, pr.Name, err) + resolvedObjectMeta.Namespace, pr.Name, err) return controller.NewPermanentError(err) } // Ensure that the PipelineRun provides all the parameters required by the Pipeline @@ -456,7 +456,7 @@ func (c *Reconciler) reconcile(ctx context.Context, pr *v1beta1.PipelineRun, get // This Run has failed, so we need to mark it as failed and stop reconciling it pr.Status.MarkFailed(ReasonParameterTypeMismatch, "PipelineRun %s/%s parameters have mismatching types with Pipeline %s/%s's parameters: %s", - pr.Namespace, pr.Name, pr.Namespace, pipelineMeta.Name, err) + pr.Namespace, pr.Name, pr.Namespace, resolvedObjectMeta.Name, err) return controller.NewPermanentError(err) } @@ -465,7 +465,7 @@ func (c *Reconciler) reconcile(ctx context.Context, pr *v1beta1.PipelineRun, get // This Run has failed, so we need to mark it as failed and stop reconciling it pr.Status.MarkFailed(ReasonObjectParameterMissKeys, "PipelineRun %s/%s parameters is missing object keys required by Pipeline %s/%s's parameters: %s", - pr.Namespace, pr.Name, pr.Namespace, pipelineMeta.Name, err) + pr.Namespace, pr.Name, pr.Namespace, resolvedObjectMeta.Name, err) return controller.NewPermanentError(err) } @@ -474,7 +474,7 @@ func (c *Reconciler) reconcile(ctx context.Context, pr *v1beta1.PipelineRun, get // This Run has failed, so we need to mark it as failed and stop reconciling it pr.Status.MarkFailed(ReasonObjectParameterMissKeys, "PipelineRun %s/%s parameters is missing object keys required by Pipeline %s/%s's parameters: %s", - pr.Namespace, pr.Name, pr.Namespace, pipelineMeta.Name, err) + pr.Namespace, pr.Name, pr.Namespace, resolvedObjectMeta.Name, err) return controller.NewPermanentError(err) } @@ -482,7 +482,7 @@ func (c *Reconciler) reconcile(ctx context.Context, pr *v1beta1.PipelineRun, get if err := resources.ValidateWorkspaceBindings(pipelineSpec, pr); err != nil { pr.Status.MarkFailed(ReasonInvalidWorkspaceBinding, "PipelineRun %s/%s doesn't bind Pipeline %s/%s's Workspaces correctly: %s", - pr.Namespace, pr.Name, pr.Namespace, pipelineMeta.Name, err) + pr.Namespace, pr.Name, pr.Namespace, resolvedObjectMeta.Name, err) return controller.NewPermanentError(err) } @@ -496,7 +496,7 @@ func (c *Reconciler) reconcile(ctx context.Context, pr *v1beta1.PipelineRun, get // Apply parameter substitution from the PipelineRun pipelineSpec = resources.ApplyParameters(ctx, pipelineSpec, pr) - pipelineSpec = resources.ApplyContexts(pipelineSpec, pipelineMeta.Name, pr) + pipelineSpec = resources.ApplyContexts(pipelineSpec, resolvedObjectMeta.Name, pr) pipelineSpec = resources.ApplyWorkspaces(pipelineSpec, pr) // Update pipelinespec of pipelinerun's status field pr.Status.PipelineSpec = pipelineSpec @@ -509,7 +509,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, resolvedObjectMeta.ObjectMeta, pr, providedResources) switch { case errors.Is(err, remote.ErrorRequestInProgress): message := fmt.Sprintf("PipelineRun %s/%s awaiting remote resource", pr.Namespace, pr.Name) @@ -1253,35 +1253,23 @@ 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, resolvedObjectMeta *tresources.ResolvedObjectMeta) error { // Only store the PipelineSpec once, if it has never been set before. if pr.Status.PipelineSpec == nil { pr.Status.PipelineSpec = ps - - // Propagate labels from Pipeline to PipelineRun. - if pr.ObjectMeta.Labels == nil { - pr.ObjectMeta.Labels = make(map[string]string, len(meta.Labels)+1) - } - for key, value := range meta.Labels { - // Do not override duplicates between PipelineRun and Pipeline - // PipelineRun labels take precedences over Pipeline - if _, ok := pr.ObjectMeta.Labels[key]; !ok { - pr.ObjectMeta.Labels[key] = value - } + if resolvedObjectMeta == nil { + return nil } - pr.ObjectMeta.Labels[pipeline.PipelineLabelKey] = meta.Name + // TODO (@chuangw6): Set configsource + // test ... + // https://github.com/tektoncd/pipeline/pull/5580 + // pr.Status.Provenance.ConfigSource = resolvedObjectMeta.ConfigSource + // Propagate labels from Pipeline to PipelineRun. + pr.ObjectMeta.Labels = kmap.Union(resolvedObjectMeta.Labels, pr.ObjectMeta.Labels) + pr.ObjectMeta.Labels[pipeline.PipelineLabelKey] = resolvedObjectMeta.Name // Propagate annotations from Pipeline to PipelineRun. - if pr.ObjectMeta.Annotations == nil { - pr.ObjectMeta.Annotations = make(map[string]string, len(meta.Annotations)) - } - for key, value := range meta.Annotations { - // Do not override duplicates between PipelineRun and Pipeline - // PipelineRun labels take precedences over Pipeline - if _, ok := pr.ObjectMeta.Annotations[key]; !ok { - pr.ObjectMeta.Annotations[key] = value - } - } + pr.ObjectMeta.Annotations = kmap.Union(resolvedObjectMeta.Annotations, pr.ObjectMeta.Annotations) } return nil diff --git a/pkg/reconciler/pipelinerun/pipelinerun_test.go b/pkg/reconciler/pipelinerun/pipelinerun_test.go index 28afde48f7a..c08401ed34c 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" @@ -5557,7 +5558,16 @@ metadata: 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 { + if err := storePipelineSpecAndMergeMeta(pr, &ps, &tresources.ResolvedObjectMeta{ + ObjectMeta: &pr.ObjectMeta, + ConfigSource: &v1beta1.ConfigSource{ + URI: "abc.com", + Digest: map[string]string{ + "sha1": "a123", + }, + EntryPoint: "foo/bar", + }, + }); err != nil { t.Errorf("storePipelineSpec() error = %v", err) } if d := cmp.Diff(pr, want); d != "" { @@ -5565,7 +5575,7 @@ metadata: } // The next time, it should not get overwritten - if err := storePipelineSpecAndMergeMeta(pr, &ps1, &metav1.ObjectMeta{}); err != nil { + if err := storePipelineSpecAndMergeMeta(pr, &ps1, &tresources.ResolvedObjectMeta{}); err != nil { t.Errorf("storePipelineSpec() error = %v", err) } if d := cmp.Diff(pr, want); d != "" { @@ -5585,7 +5595,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..c8923123c08 100644 --- a/pkg/reconciler/pipelinerun/pipelinespec/pipelinespec.go +++ b/pkg/reconciler/pipelinerun/pipelinespec/pipelinespec.go @@ -22,32 +22,35 @@ 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) + // Get related p for pipelinerun + 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 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 +60,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..206a20fc8f4 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, 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 780f3c42eee..25c151a282f 100644 --- a/pkg/reconciler/pipelinerun/resources/pipelineref.go +++ b/pkg/reconciler/pipelinerun/resources/pipelineref.go @@ -44,34 +44,34 @@ func GetPipelineFunc(ctx context.Context, k8s kubernetes.Interface, tekton clien 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 pipelineRun.Status.PipelineSpec != nil { - return func(_ context.Context, name string) (v1beta1.PipelineObject, error) { + return func(_ context.Context, name string) (v1beta1.PipelineObject, *v1beta1.ConfigSource, error) { return &v1beta1.Pipeline{ ObjectMeta: metav1.ObjectMeta{ Name: name, Namespace: namespace, }, Spec: *pipelineRun.Status.PipelineSpec, - }, nil + }, nil, 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) }, 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 @@ -98,28 +98,30 @@ 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) { +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) } - return l.Tektonclient.TektonV1beta1().Pipelines(l.Namespace).Get(ctx, name, metav1.GetOptions{}) + pipeline, err := l.Tektonclient.TektonV1beta1().Pipelines(l.Namespace).Get(ctx, name, metav1.GetOptions{}) + return pipeline, nil, err } // 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) (v1beta1.PipelineObject, error) { - obj, err := resolver.Get(ctx, "pipeline", name) +func resolvePipeline(ctx context.Context, resolver remote.Resolver, name string) (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()) } - 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 5c486d165b9..5b757e13c0b 100644 --- a/pkg/reconciler/pipelinerun/resources/pipelineref_test.go +++ b/pkg/reconciler/pipelinerun/resources/pipelineref_test.go @@ -93,7 +93,7 @@ func TestLocalPipelineRef(t *testing.T) { Tektonclient: tektonclient, } - task, err := lc.GetPipeline(ctx, tc.ref.Name) + task, annotationsFromResolution, 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 { @@ -103,6 +103,11 @@ func TestLocalPipelineRef(t *testing.T) { if d := cmp.Diff(task, tc.expected); tc.expected != nil && d != "" { t.Error(diff.PrintWantGot(d)) } + + if annotationsFromResolution != nil { + t.Errorf("expected annotations from resolution is nil, but got %v", annotationsFromResolution) + } + }) } } @@ -194,7 +199,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()) } @@ -202,6 +207,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 from resolution is nil, but got %v", configSource) + } }) } } @@ -251,7 +260,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, annotationsFromResolution, err := fn(ctx, name) if err != nil { t.Fatalf("failed to call pipelinefn: %s", err.Error()) } @@ -259,6 +268,10 @@ func TestGetPipelineFuncSpecAlreadyFetched(t *testing.T) { if diff := cmp.Diff(actualPipeline, expectedPipeline); expectedPipeline != nil && diff != "" { t.Error(diff) } + + if annotationsFromResolution != nil { + t.Errorf("expected annotations from resolution is nil, but got %v", annotationsFromResolution) + } } func TestGetPipelineFunc_RemoteResolution(t *testing.T) { @@ -272,7 +285,15 @@ func TestGetPipelineFunc_RemoteResolution(t *testing.T) { "apiVersion: tekton.dev/v1beta1", pipelineYAMLString, }, "\n") - resolved := test.NewResolvedResource([]byte(pipelineYAML), nil, nil) + + expectedConfigSource := &v1beta1.ConfigSource{ + URI: "abc.com", + Digest: map[string]string{ + "sha1": "a123", + }, + EntryPoint: "foo/bar", + } + resolved := test.NewResolvedResource([]byte(pipelineYAML), nil, expectedConfigSource, nil) requester := test.NewRequester(resolved, nil) fn, err := resources.GetPipelineFunc(ctx, nil, nil, requester, &v1beta1.PipelineRun{ ObjectMeta: metav1.ObjectMeta{Namespace: "default"}, @@ -285,7 +306,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()) } @@ -293,6 +314,10 @@ func TestGetPipelineFunc_RemoteResolution(t *testing.T) { if diff := cmp.Diff(pipeline, resolvedPipeline); diff != "" { t.Error(diff) } + + if d := cmp.Diff(expectedConfigSource, resolvedConfigSource); d != "" { + t.Errorf("annotations did not match: %s", diff.PrintWantGot(d)) + } } func TestGetPipelineFunc_RemoteResolution_ReplacedParams(t *testing.T) { @@ -317,7 +342,7 @@ 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, nil, nil) requester := &test.Requester{ ResolvedResource: resolved, Params: []v1beta1.Param{{ @@ -346,7 +371,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()) } @@ -355,6 +380,10 @@ func TestGetPipelineFunc_RemoteResolution_ReplacedParams(t *testing.T) { t.Error(diff) } + if resolvedConfigSource != nil { + t.Errorf("expected resolved configsource is nil, but got %s", resolvedConfigSource) + } + pipelineRefNotMatching := &v1beta1.PipelineRef{ ResolverRef: v1beta1.ResolverRef{ Resolver: "git", @@ -386,7 +415,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") } @@ -401,7 +430,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"}, @@ -413,7 +442,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") } } diff --git a/pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go b/pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go index a1a6260045e..01ca34e1ec0 100644 --- a/pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go +++ b/pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go @@ -777,7 +777,7 @@ func resolveTask( spec = *taskRun.Status.TaskSpec taskName = pipelineTask.TaskRef.Name } else { - t, err = getTask(ctx, pipelineTask.TaskRef.Name) + t, _, err = getTask(ctx, pipelineTask.TaskRef.Name) switch { case errors.Is(err, remote.ErrorRequestInProgress): return v1beta1.TaskSpec{}, "", "", err diff --git a/pkg/reconciler/pipelinerun/resources/pipelinerunresolution_test.go b/pkg/reconciler/pipelinerun/resources/pipelinerunresolution_test.go index 9f36b883279..4fb7d2f3a04 100644 --- a/pkg/reconciler/pipelinerun/resources/pipelinerunresolution_test.go +++ b/pkg/reconciler/pipelinerun/resources/pipelinerunresolution_test.go @@ -48,8 +48,8 @@ import ( func nopGetRun(string) (*v1alpha1.Run, error) { return nil, errors.New("GetRun should not be called") } -func nopGetTask(context.Context, string) (v1beta1.TaskObject, error) { - return nil, errors.New("GetTask should not be called") +func nopGetTask(context.Context, string) (v1beta1.TaskObject, *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") @@ -1966,7 +1966,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{} @@ -2109,7 +2111,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{ @@ -2160,8 +2164,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) @@ -2229,7 +2233,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 { @@ -2299,7 +2305,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 { @@ -2363,8 +2371,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 } @@ -2702,7 +2710,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", @@ -2733,7 +2743,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 } @@ -3522,7 +3534,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 } @@ -3656,7 +3670,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 } @@ -3758,7 +3774,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 54c0bb84605..f2c391ba356 100644 --- a/pkg/reconciler/taskrun/resources/taskref.go +++ b/pkg/reconciler/taskrun/resources/taskref.go @@ -57,15 +57,16 @@ func GetTaskKind(taskrun *v1beta1.TaskRun) v1beta1.TaskKind { // 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 + // TODO (@chuangw6): if we want to pass any annotations for embedded task, we can set them here. 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) { return &v1beta1.Task{ ObjectMeta: metav1.ObjectMeta{ Name: name, Namespace: taskrun.Namespace, }, Spec: *taskrun.Status.TaskSpec, - }, nil + }, nil, nil }, nil } return GetTaskFunc(ctx, k8s, tekton, requester, taskrun, taskrun.Spec.TaskRef, taskrun.Name, taskrun.Namespace, taskrun.Spec.ServiceAccountName) @@ -87,14 +88,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) @@ -103,7 +104,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) @@ -136,18 +137,18 @@ func GetTaskFunc(ctx context.Context, k8s kubernetes.Interface, tekton clientset // fetch a task with given name. An error is returned if the // remoteresource doesn't work or the returned data isn't a valid // v1beta1.TaskObject. -func resolveTask(ctx context.Context, resolver remote.Resolver, name string, kind v1beta1.TaskKind) (v1beta1.TaskObject, error) { +func resolveTask(ctx context.Context, resolver remote.Resolver, name string, kind v1beta1.TaskKind) (v1beta1.TaskObject, *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()) } - return taskObj, nil + return taskObj, configSource, nil } // readRuntimeObjectAsTask tries to convert a generic runtime.Object @@ -173,20 +174,22 @@ type LocalTaskRefResolver struct { // GetTask will resolve either a Task or ClusterTask from the local cluster using a versioned Tekton client. It will // return an error if it can't find an appropriate Task for any reason. -func (l *LocalTaskRefResolver) GetTask(ctx context.Context, name string) (v1beta1.TaskObject, error) { +// TODO (@chuangw6): if we want to pass any annotations for in-cluster task, we can pass them through the second returned value +func (l *LocalTaskRefResolver) GetTask(ctx context.Context, name string) (v1beta1.TaskObject, *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) } - return l.Tektonclient.TektonV1beta1().Tasks(l.Namespace).Get(ctx, name, metav1.GetOptions{}) + task, err := l.Tektonclient.TektonV1beta1().Tasks(l.Namespace).Get(ctx, name, metav1.GetOptions{}) + return task, nil, err } // 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 001853ca525..e9446cb2bba 100644 --- a/pkg/reconciler/taskrun/resources/taskref_test.go +++ b/pkg/reconciler/taskrun/resources/taskref_test.go @@ -202,7 +202,7 @@ func TestLocalTaskRef(t *testing.T) { Tektonclient: tektonclient, } - task, err := lc.GetTask(ctx, tc.ref.Name) + task, _, err := lc.GetTask(ctx, tc.ref.Name) if tc.wantErr && err == nil { t.Fatal("Expected error but found nil instead") } else if !tc.wantErr && err != nil { @@ -436,7 +436,7 @@ func TestGetTaskFunc(t *testing.T) { t.Fatalf("failed to get task fn: %s", err.Error()) } - task, err := fn(ctx, tc.ref.Name) + task, _, err := fn(ctx, tc.ref.Name) if err != nil { t.Fatalf("failed to call taskfn: %s", err.Error()) } @@ -496,7 +496,7 @@ echo hello if err != nil { t.Fatalf("failed to get Task fn: %s", err.Error()) } - actualTask, err := fn(ctx, name) + actualTask, _, err := fn(ctx, name) if err != nil { t.Fatalf("failed to call Taskfn: %s", err.Error()) } @@ -517,7 +517,14 @@ func TestGetTaskFunc_RemoteResolution(t *testing.T) { "apiVersion: tekton.dev/v1beta1", taskYAMLString, }, "\n") - resolved := test.NewResolvedResource([]byte(taskYAML), nil, nil) + expectedConfigSource := &v1beta1.ConfigSource{ + URI: "abc.com", + Digest: map[string]string{ + "sha1": "a123", + }, + EntryPoint: "foo/bar", + } + resolved := test.NewResolvedResource([]byte(taskYAML), nil, expectedConfigSource, nil) requester := test.NewRequester(resolved, nil) tr := &v1beta1.TaskRun{ ObjectMeta: metav1.ObjectMeta{Namespace: "default"}, @@ -531,11 +538,15 @@ func TestGetTaskFunc_RemoteResolution(t *testing.T) { t.Fatalf("failed to get task fn: %s", err.Error()) } - resolvedTask, err := fn(ctx, taskRef.Name) + resolvedTask, gotConfigSource, err := fn(ctx, taskRef.Name) if err != nil { t.Fatalf("failed to call pipelinefn: %s", err.Error()) } + if d := cmp.Diff(expectedConfigSource, gotConfigSource); d != "" { + t.Errorf("annotations did not match: %s", diff.PrintWantGot(d)) + } + if d := cmp.Diff(task, resolvedTask); d != "" { t.Error(d) } @@ -563,7 +574,7 @@ 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, nil, nil) requester := &test.Requester{ ResolvedResource: resolved, Params: []v1beta1.Param{{ @@ -593,7 +604,7 @@ func TestGetTaskFunc_RemoteResolution_ReplacedParams(t *testing.T) { t.Fatalf("failed to get task fn: %s", err.Error()) } - resolvedTask, err := fn(ctx, taskRef.Name) + resolvedTask, _, err := fn(ctx, taskRef.Name) if err != nil { t.Fatalf("failed to call pipelinefn: %s", err.Error()) } @@ -634,7 +645,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") } @@ -649,7 +660,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"}, @@ -662,7 +673,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") } } diff --git a/pkg/reconciler/taskrun/resources/taskspec.go b/pkg/reconciler/taskrun/resources/taskspec.go index aa836fa6051..ab95acb8277 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,35 @@ 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 tells where the file comes from. +type ResolvedObjectMeta struct { + *metav1.ObjectMeta `json:",omitempty"` + // ConfigSource identifying where the spec comes 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 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 +72,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..735d66cd3a8 100644 --- a/pkg/reconciler/taskrun/resources/taskspec_test.go +++ b/pkg/reconciler/taskrun/resources/taskspec_test.go @@ -48,20 +48,33 @@ 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) + + expectedConfigSource := &v1beta1.ConfigSource{ + URI: "abc.com", + Digest: map[string]string{ + "sha1": "a123", + }, + EntryPoint: "foo/bar", + } + gt := func(ctx context.Context, n string) (v1beta1.TaskObject, *v1beta1.ConfigSource, error) { + return task, expectedConfigSource, 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(expectedConfigSource, resolvedObjectMeta.ConfigSource); d != "" { + t.Errorf("configsource did not match: %s", diff.PrintWantGot(d)) + } } func TestGetTaskSpec_Embedded(t *testing.T) { @@ -77,22 +90,26 @@ 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) } + + if resolvedObjectMeta.ConfigSource != nil { + t.Errorf("resolved configsource is expected to be empty, but got %v", resolvedObjectMeta.ConfigSource) + } } func TestGetTaskSpec_Invalid(t *testing.T) { @@ -101,8 +118,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 +138,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 { @@ -152,6 +169,9 @@ func TestGetTaskData_ResolutionSuccess(t *testing.T) { } sourceMeta := metav1.ObjectMeta{ Name: "task", + Annotations: map[string]string{ + "foo": "bar", + }, } sourceSpec := v1beta1.TaskSpec{ Steps: []v1beta1.Step{{ @@ -160,11 +180,19 @@ func TestGetTaskData_ResolutionSuccess(t *testing.T) { Script: `echo "hello world!"`, }}, } - getTask := func(ctx context.Context, n string) (v1beta1.TaskObject, error) { + + expectedConfigSource := &v1beta1.ConfigSource{ + URI: "abc.com", + Digest: map[string]string{ + "sha1": "a123", + }, + EntryPoint: "foo/bar", + } + getTask := func(ctx context.Context, n string) (v1beta1.TaskObject, *v1beta1.ConfigSource, error) { return &v1beta1.Task{ ObjectMeta: *sourceMeta.DeepCopy(), Spec: *sourceSpec.DeepCopy(), - }, nil + }, expectedConfigSource, nil } ctx := context.Background() resolvedMeta, resolvedSpec, err := GetTaskData(ctx, tr, getTask) @@ -174,6 +202,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(expectedConfigSource, 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 +225,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 +248,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 0bf77a330e6..3fe99404eaf 100644 --- a/pkg/reconciler/taskrun/taskrun.go +++ b/pkg/reconciler/taskrun/taskrun.go @@ -327,7 +327,7 @@ func (c *Reconciler) prepare(ctx context.Context, tr *v1beta1.TaskRun) (*v1beta1 return nil, nil, err } - taskMeta, taskSpec, err := resources.GetTaskData(ctx, tr, getTaskfunc) + resolvedObjectMeta, taskSpec, err := resources.GetTaskData(ctx, tr, getTaskfunc) switch { case errors.Is(err, remote.ErrorRequestInProgress): message := fmt.Sprintf("TaskRun %s/%s awaiting remote resource", tr.Namespace, tr.Name) @@ -342,7 +342,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(tr, taskSpec, resolvedObjectMeta); err != nil { logger.Errorf("Failed to store TaskSpec on TaskRun.Statusfor taskrun %s: %v", tr.Name, err) } } @@ -353,7 +353,7 @@ func (c *Reconciler) prepare(ctx context.Context, tr *v1beta1.TaskRun) (*v1beta1 inputs = tr.Spec.Resources.Inputs outputs = tr.Spec.Resources.Outputs } - rtr, err := resources.ResolveTaskResources(taskSpec, taskMeta.Name, resources.GetTaskKind(tr), inputs, outputs, c.resourceLister.PipelineResources(tr.Namespace).Get) + rtr, err := resources.ResolveTaskResources(taskSpec, resolvedObjectMeta.Name, resources.GetTaskKind(tr), inputs, outputs, c.resourceLister.PipelineResources(tr.Namespace).Get) if err != nil { if k8serrors.IsNotFound(err) && tknreconciler.IsYoungResource(tr) { // For newly created resources, don't fail immediately. @@ -909,37 +909,28 @@ 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, resolvedObjectMeta *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 resolvedObjectMeta == nil { + // return nil + // } + // TODO (@chuangw6): Set configsource + // test ... + // https://github.com/tektoncd/pipeline/pull/5580 + // tr.Status.Provenance.ConfigSource = meta.ConfigSource + // Propagate annotations from Task to TaskRun. - if tr.ObjectMeta.Annotations == nil { - tr.ObjectMeta.Annotations = make(map[string]string, len(meta.Annotations)) - } - for key, value := range meta.Annotations { - // Do not override duplicates between TaskRun and Task - // TaskRun labels take precedences over Task - if _, ok := tr.ObjectMeta.Annotations[key]; !ok { - tr.ObjectMeta.Annotations[key] = value - } - } + tr.ObjectMeta.Annotations = kmap.Union(resolvedObjectMeta.ObjectMeta.Annotations, tr.ObjectMeta.Annotations) + // Propagate labels from Task to TaskRun. - if tr.ObjectMeta.Labels == nil { - tr.ObjectMeta.Labels = make(map[string]string, len(meta.Labels)+1) - } - for key, value := range meta.Labels { - // Do not override duplicates between TaskRun and Task - // TaskRun labels take precedences over Task - if _, ok := tr.ObjectMeta.Labels[key]; !ok { - tr.ObjectMeta.Labels[key] = value - } - } + tr.ObjectMeta.Labels = kmap.Union(resolvedObjectMeta.ObjectMeta.Labels, tr.ObjectMeta.Labels) if tr.Spec.TaskRef != nil { if tr.Spec.TaskRef.Kind == "ClusterTask" { - tr.ObjectMeta.Labels[pipeline.ClusterTaskLabelKey] = meta.Name + tr.ObjectMeta.Labels[pipeline.ClusterTaskLabelKey] = resolvedObjectMeta.Name } else { - tr.ObjectMeta.Labels[pipeline.TaskLabelKey] = meta.Name + tr.ObjectMeta.Labels[pipeline.TaskLabelKey] = resolvedObjectMeta.Name } } } diff --git a/pkg/reconciler/taskrun/taskrun_test.go b/pkg/reconciler/taskrun/taskrun_test.go index b75204b7f6a..66a66b3e1a2 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_storeTaskSpecAndAnnotationsInStatus(t *testing.T) { tr := parse.MustParseTaskRun(t, ` metadata: annotations: @@ -4084,7 +4084,16 @@ spec: 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 { + if err := storeTaskSpecAndMergeMeta(tr, &ts, &resources.ResolvedObjectMeta{ + ObjectMeta: &tr.ObjectMeta, + ConfigSource: &v1beta1.ConfigSource{ + URI: "https://abc.com.git", + Digest: map[string]string{ + "sha1": "xyz", + }, + EntryPoint: "foo/bar", + }, + }); err != nil { t.Errorf("storeTaskSpec() error = %v", err) } if d := cmp.Diff(tr, want); d != "" { @@ -4092,7 +4101,7 @@ spec: } // The next time, it should not get overwritten - if err := storeTaskSpecAndMergeMeta(tr, &ts1, &metav1.ObjectMeta{}); err != nil { + if err := storeTaskSpecAndMergeMeta(tr, &ts1, &resources.ResolvedObjectMeta{}); err != nil { t.Errorf("storeTaskSpec() error = %v", err) } if d := cmp.Diff(tr, want); d != "" { @@ -4111,8 +4120,18 @@ 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}, + ConfigSource: &v1beta1.ConfigSource{ + URI: "abc.com", + Digest: map[string]string{ + "sha1": "a123", + }, + EntryPoint: "foo/bar", + }, + } + + if err := storeTaskSpecAndMergeMeta(tr, &v1beta1.TaskSpec{}, &resolvedMeta); err != nil { t.Errorf("storeTaskSpecAndMergeMeta error = %v", err) } if d := cmp.Diff(tr.ObjectMeta.Labels, wantedlabels); d != "" { @@ -4121,6 +4140,12 @@ func Test_storeTaskSpec_metadata(t *testing.T) { if d := cmp.Diff(tr.ObjectMeta.Annotations, wantedannotations); d != "" { t.Fatalf(diff.PrintWantGot(d)) } + + // TODO (chuangw6): uncomment this line + // https://github.com/tektoncd/pipeline/pull/5580 + // if d := cmp.Diff(tr.Status.Provenance.ConfigSource, resolvedMeta.ConfigSource); d != nil { + // t.Fatalf(diff.PrintWantGot(d)) + // } } func TestWillOverwritePodAffinity(t *testing.T) { diff --git a/pkg/remote/oci/resolver.go b/pkg/remote/oci/resolver.go index 6a837ca7468..b4999028f7b 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,34 @@ 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) { +// TODO (@chuangw6): add returned annotation from resolution here for OCI bundle source +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 +130,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..f397dd954fd 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, _, 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) } 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..5e7f0cfc832 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,7 +123,7 @@ func TestGet_Errors(t *testing.T) { ResolvedResource: tc.resolvedResource, } resolver := NewResolver(requester, owner, "git", "", "", nil) - obj, err := resolver.Get(ctx, "foo", "bar") + obj, _, err := resolver.Get(ctx, "foo", "bar") if obj != nil { t.Errorf("received unexpected resolved resource") } diff --git a/pkg/remote/resolver.go b/pkg/remote/resolver.go index 11bc5877ad7..f9cbfce3397 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,9 @@ 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 ref information. +// But the second returned value might be changed to a structured type. See https://github.com/tektoncd/pipeline/issues/5529. 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, } }