Skip to content

Commit

Permalink
gitresolver: Pass ConfigSource from ResolutionRequest.Status to *Run.…
Browse files Browse the repository at this point in the history 
…Status.

Related to tektoncd#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 tektoncd#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 <[email protected]>
  • Loading branch information
@chuangw6
chuangw6 committed Oct 19, 2022
1 parent a132c7a commit 3a9669d
Show file tree
Hide file tree
Showing 20 changed files with 374 additions and 163 deletions.
41 changes: 26 additions & 15 deletions pkg/reconciler/pipelinerun/pipelinerun.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)
}
}
Expand Down Expand Up @@ -413,15 +413,15 @@ 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)
}

if err := resources.ValidateResourceBindings(pipelineSpec, pr); err != nil {
// 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)
Expand All @@ -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
Expand All @@ -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)
}

Expand All @@ -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)
}

Expand All @@ -474,15 +474,15 @@ 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)
}

// Ensure that the workspaces expected by the Pipeline are provided by the PipelineRun.
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)
}

Expand All @@ -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
Expand All @@ -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)
Expand Down Expand Up @@ -1253,17 +1253,28 @@ 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
if resolvedObjectMeta == nil {
return nil
}

// Propogate ConfigSource from remote resolution to PipelineRun Status
if resolvedObjectMeta.ConfigSource != nil {
if pr.Status.Provenance == nil {
pr.Status.Provenance = &v1beta1.Provenance{}
}
pr.Status.Provenance.ConfigSource = resolvedObjectMeta.ConfigSource
}

// Propagate labels from Pipeline to PipelineRun. PipelineRun labels take precedences over Pipeline.
pr.ObjectMeta.Labels = kmap.Union(meta.Labels, pr.ObjectMeta.Labels)
pr.ObjectMeta.Labels[pipeline.PipelineLabelKey] = meta.Name
pr.ObjectMeta.Labels = kmap.Union(resolvedObjectMeta.Labels, pr.ObjectMeta.Labels)
pr.ObjectMeta.Labels[pipeline.PipelineLabelKey] = resolvedObjectMeta.Name

// Propagate annotations from Pipeline to PipelineRun. PipelineRun annotations take precedences over Pipeline.
pr.ObjectMeta.Annotations = kmap.Union(meta.Annotations, pr.ObjectMeta.Annotations)
pr.ObjectMeta.Annotations = kmap.Union(resolvedObjectMeta.Annotations, pr.ObjectMeta.Annotations)

}
return nil
Expand Down
29 changes: 25 additions & 4 deletions pkg/reconciler/pipelinerun/pipelinerun_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -5535,7 +5536,7 @@ status:
}
}

func Test_storePipelineSpec(t *testing.T) {
func Test_storePipelineSpecAndConfigSource(t *testing.T) {
pr := parse.MustParsePipelineRun(t, `
metadata:
name: test-pipeline-run-success
Expand All @@ -5552,20 +5553,38 @@ metadata:
want.Status = v1beta1.PipelineRunStatus{
PipelineRunStatusFields: v1beta1.PipelineRunStatusFields{
PipelineSpec: ps.DeepCopy(),
Provenance: &v1beta1.Provenance{
ConfigSource: &v1beta1.ConfigSource{
URI: "abc.com",
Digest: map[string]string{
"sha1": "a123",
},
EntryPoint: "foo/bar",
},
},
},
}
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 != "" {
t.Fatalf(diff.PrintWantGot(d))
}

// 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 != "" {
Expand All @@ -5585,7 +5604,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 != "" {
Expand Down
23 changes: 15 additions & 8 deletions pkg/reconciler/pipelinerun/pipelinespec/pipelinespec.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
}
60 changes: 41 additions & 19 deletions pkg/reconciler/pipelinerun/pipelinespec/pipelinespec_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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) {
Expand All @@ -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 {
Expand Down Expand Up @@ -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)
Expand All @@ -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) {
Expand All @@ -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 {
Expand All @@ -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)
Expand All @@ -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)
Expand Down
Loading

0 comments on commit 3a9669d

Please sign in to comment.