Skip to content

Commit

Permalink
Dereference the PipelineSpec into PipelineRun.Status
Browse files Browse the repository at this point in the history
The Pipeline definition used for a PipelineRun can change after the
PipelineRun has started. This poses problems for auditability post-run. Rather
than chase down every part of a Pipeline that we might like to audit later,
let's just add the entire thing here.

This is a follow up to #2444.
  • Loading branch information
dlorenc committed Apr 24, 2020
1 parent 35d5121 commit 0025bb5
Show file tree
Hide file tree
Showing 6 changed files with 55 additions and 1 deletion.
5 changes: 5 additions & 0 deletions docs/pipelineruns.md
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,11 @@ executed successfully or a failure occurs.
**Note:** A `PipelineRun` automatically creates corresponding `TaskRuns` for every
`Task` in your `Pipeline`.

The `Status` field tracks the current state of a `PipelineRun`, and can be used to monitor
progress.
This field contains the status of every `TaskRun`, as well as the full `PipelineSpec` used
to instantiate this `PipelineRun`, for full auditability.

## Configuring a `PipelineRun`

A `PipelineRun` definition supports the following fields:
Expand Down
1 change: 0 additions & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@ require (
golang.org/x/time v0.0.0-20191024005414-555d28b269f0 // indirect
golang.org/x/tools v0.0.0-20200214144324-88be01311a71 // indirect
gomodules.xyz/jsonpatch/v2 v2.1.0
google.golang.org/api v0.15.0 // indirect
google.golang.org/appengine v1.6.5 // indirect
k8s.io/api v0.17.3
k8s.io/apiextensions-apiserver v0.17.3 // indirect
Expand Down
3 changes: 3 additions & 0 deletions pkg/apis/pipeline/v1beta1/pipelinerun_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -255,6 +255,9 @@ type PipelineRunStatusFields struct {
// PipelineResults are the list of results written out by the pipeline task's containers
// +optional
PipelineResults []PipelineRunResult `json:"pipelineResults,omitempty"`

// PipelineRunSpec contains the exact spec used to instantiate the run
PipelineSpec *PipelineSpec `json:"pipelineSpec,omitempty"`
}

// PipelineRunResult used to describe the results of a pipeline
Expand Down
5 changes: 5 additions & 0 deletions pkg/apis/pipeline/v1beta1/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

14 changes: 14 additions & 0 deletions pkg/reconciler/pipelinerun/pipelinerun.go
Original file line number Diff line number Diff line change
Expand Up @@ -359,6 +359,11 @@ func (c *Reconciler) reconcile(ctx context.Context, pr *v1alpha1.PipelineRun) er
return nil
}

// Store the fetched PipelineSpec on the PipelineRun for auditing
if err := storePipelineSpec(ctx, pr, pipelineSpec); err != nil {
c.Logger.Errorf("Failed to store PipelineSpec on PipelineRun.Status for pipelinerun %s: %v", pr.Name, err)
}

// Propagate labels from Pipeline to PipelineRun.
if pr.ObjectMeta.Labels == nil {
pr.ObjectMeta.Labels = make(map[string]string, len(pipelineMeta.Labels)+1)
Expand Down Expand Up @@ -906,3 +911,12 @@ func (c *Reconciler) makeConditionCheckContainer(rprt *resources.ResolvedPipelin
cc := v1alpha1.ConditionCheck(*cctr)
return &cc, err
}

func storePipelineSpec(ctx context.Context, pr *v1alpha1.PipelineRun, ps *v1alpha1.PipelineSpec) error {
// Only store the PipelineSpec once, if it has never been set before.
if pr.Status.PipelineSpec == nil {
pr.Status.PipelineSpec = &v1beta1.PipelineSpec{}
return ps.ConvertTo(ctx, pr.Status.PipelineSpec)
}
return nil
}
28 changes: 28 additions & 0 deletions pkg/reconciler/pipelinerun/pipelinerun_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2042,3 +2042,31 @@ func TestReconcileWithPipelineResults(t *testing.T) {
t.Errorf("expected to see pipeline run results created. -want, +got: Diff %s", d)
}
}

func Test_storePipelineSpec(t *testing.T) {
ctx := context.Background()
pr := tb.PipelineRun("foo")

ps := tb.Pipeline("some-pipeline", tb.PipelineSpec(tb.PipelineDescription("foo-pipeline"))).Spec
want := &v1beta1.PipelineSpec{}
if err := ps.ConvertTo(ctx, want); err != nil {
t.Errorf("error converting to v1beta1: %v", err)
}

// The first time we set it, it should get copied.
if err := storePipelineSpec(ctx, pr, &ps); err != nil {
t.Errorf("storePipelineSpec() error = %v", err)
}
if d := cmp.Diff(pr.Status.PipelineSpec, want); d != "" {
t.Fatalf("-want, +got: %v", d)
}

ps.Description = "new-pipeline"
// The next time, it should not get overwritten
if err := storePipelineSpec(ctx, pr, &ps); err != nil {
t.Errorf("storePipelineSpec() error = %v", err)
}
if d := cmp.Diff(pr.Status.PipelineSpec, want); d != "" {
t.Fatalf("-want, +got: %v", d)
}
}

0 comments on commit 0025bb5

Please sign in to comment.