Skip to content

Commit

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

This is a replacement for #2399
  • Loading branch information
dlorenc committed Apr 22, 2020
1 parent 8b4f7c8 commit 5cff4f6
Show file tree
Hide file tree
Showing 6 changed files with 61 additions and 3 deletions.
6 changes: 3 additions & 3 deletions docs/taskruns.md
Original file line number Diff line number Diff line change
Expand Up @@ -316,9 +316,9 @@ If multiple `Steps` are defined in the `Task` invoked by the `TaskRun`, you can
status in the `steps.results` field using the following command, where `<name>` is the name of the target
`TaskRun`:

```
kubectl get taskrun <name>
```
The exact Task Spec used to instantiate the TaskRun is also included in the Status for full auditability.

### Steps

The corresponding statuses appear in the `status.steps` list in the order in which the `Steps` have been
specified in the `Task` definition.
Expand Down
3 changes: 3 additions & 0 deletions pkg/apis/pipeline/v1beta1/taskrun_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,9 @@ type TaskRunStatusFields struct {
// The list has one entry per sidecar in the manifest. Each entry is
// represents the imageid of the corresponding sidecar.
Sidecars []SidecarState `json:"sidecars,omitempty"`

// TaskSpec contains the Spec from the dereferenced Task definition used to instantiate this TaskRun.
TaskSpec *TaskSpec `json:"taskSpec,omitempty"`
}

// TaskRunResult used to describe the results of a task
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/taskrun/taskrun.go
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,11 @@ func (c *Reconciler) reconcile(ctx context.Context, tr *v1alpha1.TaskRun) error
return nil
}

// Store the fetched TaskSpec on the TaskRun for auditing
if err := storeTaskSpec(ctx, tr, taskSpec); err != nil {
c.Logger.Errorf("Failed to store TaskSpec on TaskRun.Statusfor taskrun %s: %v", tr.Name, err)
}

// Propagate labels from Task to TaskRun.
if tr.ObjectMeta.Labels == nil {
tr.ObjectMeta.Labels = make(map[string]string, len(taskMeta.Labels)+1)
Expand Down Expand Up @@ -698,3 +703,12 @@ func applyVolumeClaimTemplates(workspaceBindings []v1alpha1.WorkspaceBinding, ow
}
return taskRunWorkspaceBindings
}

func storeTaskSpec(ctx context.Context, tr *v1alpha1.TaskRun, ts *v1alpha1.TaskSpec) error {
// Only store the TaskSpec once, if it has never been set before.
if tr.Status.TaskSpec == nil {
tr.Status.TaskSpec = &v1beta1.TaskSpec{}
return ts.ConvertTo(ctx, tr.Status.TaskSpec)
}
return nil
}
32 changes: 32 additions & 0 deletions pkg/reconciler/taskrun/taskrun_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@ import (
"github.com/tektoncd/pipeline/pkg/apis/config"
"github.com/tektoncd/pipeline/pkg/apis/pipeline"
"github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1"
"github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1"

podconvert "github.com/tektoncd/pipeline/pkg/pod"
"github.com/tektoncd/pipeline/pkg/reconciler/taskrun/resources/cloudevent"
ttesting "github.com/tektoncd/pipeline/pkg/reconciler/testing"
Expand All @@ -38,6 +40,7 @@ import (
test "github.com/tektoncd/pipeline/test/v1alpha1"
corev1 "k8s.io/api/core/v1"
k8sapierrors "k8s.io/apimachinery/pkg/api/errors"

"k8s.io/apimachinery/pkg/api/resource"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
Expand Down Expand Up @@ -2339,3 +2342,32 @@ func TestFailTaskRun(t *testing.T) {
})
}
}

func Test_storeTaskSpec(t *testing.T) {

ctx := context.Background()
tr := tb.TaskRun("foo", tb.TaskRunSpec(tb.TaskRunTaskRef("foo-task")))

ts := tb.Task("some-task", tb.TaskSpec(tb.TaskDescription("foo-task"))).Spec
want := &v1beta1.TaskSpec{}
if err := ts.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 := storeTaskSpec(ctx, tr, &ts); err != nil {
t.Errorf("storeTaskSpec() error = %v", err)
}
if d := cmp.Diff(tr.Status.TaskSpec, want); d != "" {
t.Fatalf("-want, +got: %v", d)
}

ts.Description = "new-task"
// The next time, it should not get overwritten
if err := storeTaskSpec(ctx, tr, &ts); err != nil {
t.Errorf("storeTaskSpec() error = %v", err)
}
if d := cmp.Diff(tr.Status.TaskSpec, want); d != "" {
t.Fatalf("-want, +got: %v", d)
}
}
4 changes: 4 additions & 0 deletions test/taskrun_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -181,4 +181,8 @@ func TestTaskRunStatus(t *testing.T) {
if !strings.HasSuffix(taskrun.Status.Steps[0].ImageID, fqImageName) {
t.Fatalf("`ImageID: %s` does not end with `%s`", taskrun.Status.Steps[0].ImageID, fqImageName)
}

if d := cmp.Diff(taskrun.Status.TaskSpec, &task.Spec); d != "" {
t.Fatalf("-got, +want: %v", d)
}
}

0 comments on commit 5cff4f6

Please sign in to comment.