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 20, 2020
1 parent 9344e4f commit 8de5547
Show file tree
Hide file tree
Showing 5 changed files with 73 additions and 0 deletions.
2 changes: 2 additions & 0 deletions docs/taskruns.md
Original file line number Diff line number Diff line change
Expand Up @@ -303,6 +303,8 @@ For each step we also include the fully-qualified image used, with the digest.
If any pods have been [`OOMKilled`](https://kubernetes.io/docs/tasks/administer-cluster/out-of-resource/)
by Kubernetes, the `Taskrun` will be marked as failed even if the exit code is 0.

The exact Task Spec used to instantiate the TaskRun is also included in the Status for full auditability.

### Steps

If multiple `steps` are defined in the `Task` invoked by the `TaskRun`, we will see the
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.

11 changes: 11 additions & 0 deletions pkg/pod/status.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,15 @@ limitations under the License.
package pod

import (
"context"
"encoding/json"
"fmt"
"sort"
"strings"
"time"

"github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1"
"github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1"
"github.com/tektoncd/pipeline/pkg/names"
"github.com/tektoncd/pipeline/pkg/termination"
"go.uber.org/zap"
Expand Down Expand Up @@ -116,6 +118,15 @@ func MakeTaskRunStatus(logger *zap.SugaredLogger, tr v1alpha1.TaskRun, pod *core
trs.Steps = []v1alpha1.StepState{}
trs.Sidecars = []v1alpha1.SidecarState{}

// Only set this once, the first time.
if trs.TaskSpec == nil {
ts := v1beta1.TaskSpec{}
if err := taskSpec.ConvertTo(context.Background(), &ts); err != nil {
logger.Errorf("error setting taskrun.Status.taskSpec in taskrun %s: %w", tr.Name, err)
}
trs.TaskSpec = &ts
}

for _, s := range pod.Status.ContainerStatuses {
if IsContainerStep(s.Name) {
if s.State.Terminated != nil && len(s.State.Terminated.Message) != 0 {
Expand Down
52 changes: 52 additions & 0 deletions pkg/pod/status_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ limitations under the License.
package pod

import (
"context"
"testing"
"time"

Expand Down Expand Up @@ -721,6 +722,11 @@ func TestMakeTaskRunStatus(t *testing.T) {
// Common traits, set for test case brevity.
c.want.PodName = "pod"
c.want.StartTime = &metav1.Time{Time: startTime}
ts := v1beta1.TaskSpec{}
if err := c.taskSpec.ConvertTo(context.Background(), &ts); err != nil {
t.Errorf("error converting ts: %w", err)
}
c.want.TaskSpec = &ts

ensureTimeNotNil := cmp.Comparer(func(x, y *metav1.Time) bool {
if x == nil {
Expand All @@ -738,6 +744,52 @@ func TestMakeTaskRunStatus(t *testing.T) {
}
}

func TestTaskSpecOnlyCopiedOnce(t *testing.T) {
now := metav1.Now()
startTime := time.Date(2010, 1, 1, 1, 1, 1, 1, time.UTC)

tr := v1alpha1.TaskRun{
ObjectMeta: metav1.ObjectMeta{
Name: "task-run",
Namespace: "foo",
},
Status: v1alpha1.TaskRunStatus{
TaskRunStatusFields: v1alpha1.TaskRunStatusFields{
StartTime: &metav1.Time{Time: startTime},
},
},
}
pod := &corev1.Pod{
ObjectMeta: metav1.ObjectMeta{
Name: "pod",
Namespace: "foo",
CreationTimestamp: now,
},
}

// Set it once, make sure it gets copied.
ts := v1alpha1.TaskSpec{}
want := &v1beta1.TaskSpec{}
if err := ts.ConvertTo(context.Background(), want); err != nil {
t.Errorf("error converting ts: %w", err)
}
logger, _ := logging.NewLogger("", "status")
tr.Status = MakeTaskRunStatus(logger, tr, pod, ts)

if d := cmp.Diff(want, tr.Status.TaskSpec); d != "" {
t.Errorf("Diff(-want, +got): %s", d)
}

// Now modify it and run through again, making sure it doesn't change.
ts.Description = "foo"
tr.Status = MakeTaskRunStatus(logger, tr, pod, ts)
// Keep the same "want" as before.
if d := cmp.Diff(want, tr.Status.TaskSpec); d != "" {
t.Errorf("Diff(-want, +got): %s", d)
}

}

func TestSidecarsReady(t *testing.T) {
for _, c := range []struct {
desc string
Expand Down

0 comments on commit 8de5547

Please sign in to comment.