Skip to content

Commit 1b37def

Browse files
committed
feat(taskspec): include inline steps in Status.Steps when StepActions are used
When at least one StepAction is referenced, `TaskRun.Status.Steps` is pre-populated to capture provenance for remote steps during resolution. However, inline steps were not included at that stage and only appeared later during pod-based status reconciliation. This contributed to confusing ordering and visibility for dashboards relying on `Status.Steps`. In the Tekton Dashboard specifically, StepAction-backed steps showed up first (because they were present in `Status.Steps` right after resolution), while inline steps appeared only after the pod was created and `MakeTaskRunStatus` had run to append them. This resulted in steps "popping" into view and reshuffling, which was confusing. When a `TaskRun` references at least one `StepAction`, in the resolution flow, the inline `Step` should also append an entry in `Status.Steps` even if it has no `Provenance`. add comment for readibility
1 parent 67c167e commit 1b37def

File tree

2 files changed

+217
-5
lines changed

2 files changed

+217
-5
lines changed

pkg/reconciler/taskrun/resources/taskspec.go

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -174,6 +174,8 @@ func resolveStepRef(ctx context.Context, taskSpec v1.TaskSpec, taskRun *v1.TaskR
174174

175175
// updateTaskRunProvenance update the TaskRun's status with source provenance information for a given step
176176
func updateTaskRunProvenance(taskRun *v1.TaskRun, stepName string, stepIndex int, source *v1.RefSource, stepStatusIndex map[string]int) {
177+
var provenance *v1.Provenance
178+
177179
// The StepState already exists. Update it in place
178180
if index, found := stepStatusIndex[stepName]; found {
179181
if taskRun.Status.Steps[index].Provenance == nil {
@@ -183,10 +185,12 @@ func updateTaskRunProvenance(taskRun *v1.TaskRun, stepName string, stepIndex int
183185
return
184186
}
185187

188+
provenance = &v1.Provenance{RefSource: source}
189+
186190
// No existing StepState found. Create and append a new one
187191
newState := v1.StepState{
188192
Name: pod.TrimStepPrefix(pod.StepName(stepName, stepIndex)),
189-
Provenance: &v1.Provenance{RefSource: source},
193+
Provenance: provenance,
190194
}
191195
taskRun.Status.Steps = append(taskRun.Status.Steps, newState)
192196
}
@@ -237,15 +241,13 @@ func GetStepActionsData(ctx context.Context, taskSpec v1.TaskSpec, taskRun *v1.T
237241
for i, step := range taskSpec.Steps {
238242
if step.Ref == nil {
239243
steps[i] = step
244+
updateTaskRunProvenance(taskRun, step.Name, i, nil, stepStatusIndex) // create StepState for inline step with nil provenance
240245
continue
241246
}
242247

243248
stepRefResolution := stepRefResolutions[i]
244249
steps[i] = *stepRefResolution.resolvedStep
245-
246-
if stepRefResolution.source != nil {
247-
updateTaskRunProvenance(taskRun, stepRefResolution.resolvedStep.Name, i, stepRefResolution.source, stepStatusIndex)
248-
}
250+
updateTaskRunProvenance(taskRun, stepRefResolution.resolvedStep.Name, i, stepRefResolution.source, stepStatusIndex)
249251
}
250252

251253
return steps, nil

pkg/reconciler/taskrun/resources/taskspec_test.go

Lines changed: 210 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -756,6 +756,216 @@ spec:
756756
}
757757
}
758758

759+
func TestGetStepActionsData_Status(t *testing.T) {
760+
firstStepAction := parse.MustParseV1beta1StepAction(t, `
761+
metadata:
762+
name: first-stepaction
763+
namespace: default
764+
spec:
765+
image: myImage
766+
`)
767+
firstStepActionSource := v1.RefSource{
768+
URI: "ref-source",
769+
Digest: map[string]string{"sha256": "abcd123456"},
770+
}
771+
772+
firstStepActionBytes, err := yaml.Marshal(firstStepAction)
773+
if err != nil {
774+
t.Fatal("failed to marshal StepAction", err)
775+
}
776+
rr := test.NewResolvedResource(firstStepActionBytes, map[string]string{}, &firstStepActionSource, nil)
777+
requester := test.NewRequester(rr, nil, resource.ResolverPayload{})
778+
779+
tests := []struct {
780+
name string
781+
tr *v1.TaskRun
782+
stepActions []*v1beta1.StepAction
783+
want v1.TaskRunStatus
784+
}{
785+
{
786+
name: "inline only",
787+
tr: &v1.TaskRun{
788+
ObjectMeta: metav1.ObjectMeta{
789+
Name: "mytaskrun",
790+
Namespace: "default",
791+
},
792+
Spec: v1.TaskRunSpec{
793+
TaskSpec: &v1.TaskSpec{
794+
Steps: []v1.Step{
795+
{
796+
Name: "first-inline",
797+
Image: "ubuntu",
798+
},
799+
{
800+
Name: "second-inline",
801+
Image: "ubuntu",
802+
},
803+
},
804+
},
805+
},
806+
},
807+
want: v1.TaskRunStatus{},
808+
}, {
809+
name: "StepAction only",
810+
tr: &v1.TaskRun{
811+
ObjectMeta: metav1.ObjectMeta{
812+
Name: "mytaskrun",
813+
Namespace: "default",
814+
},
815+
Spec: v1.TaskRunSpec{
816+
TaskSpec: &v1.TaskSpec{
817+
Steps: []v1.Step{
818+
{
819+
Name: "first-remote",
820+
Ref: &v1.Ref{
821+
Name: "first-stepaction",
822+
ResolverRef: v1.ResolverRef{
823+
Resolver: "foobar",
824+
},
825+
},
826+
},
827+
{
828+
Name: "second-remote",
829+
Ref: &v1.Ref{
830+
Name: "second-stepaction",
831+
},
832+
},
833+
},
834+
},
835+
},
836+
},
837+
stepActions: []*v1beta1.StepAction{
838+
firstStepAction,
839+
{
840+
ObjectMeta: metav1.ObjectMeta{
841+
Name: "second-stepaction",
842+
Namespace: "default",
843+
},
844+
Spec: v1beta1.StepActionSpec{
845+
Image: "myimage",
846+
},
847+
},
848+
},
849+
want: v1.TaskRunStatus{
850+
TaskRunStatusFields: v1.TaskRunStatusFields{
851+
Steps: []v1.StepState{
852+
{
853+
Name: "first-remote",
854+
Provenance: &v1.Provenance{
855+
RefSource: &v1.RefSource{
856+
URI: "ref-source",
857+
Digest: map[string]string{"sha256": "abcd123456"},
858+
},
859+
},
860+
},
861+
{
862+
Name: "second-remote",
863+
Provenance: &v1.Provenance{},
864+
},
865+
},
866+
},
867+
},
868+
},
869+
{
870+
name: "mixed inline and StepAction",
871+
tr: &v1.TaskRun{
872+
ObjectMeta: metav1.ObjectMeta{
873+
Name: "mytaskrun",
874+
Namespace: "default",
875+
},
876+
Spec: v1.TaskRunSpec{
877+
TaskSpec: &v1.TaskSpec{
878+
Steps: []v1.Step{
879+
{
880+
Name: "first-inline",
881+
Image: "ubuntu",
882+
},
883+
{
884+
Name: "second-remote",
885+
Ref: &v1.Ref{
886+
Name: "first-stepaction",
887+
ResolverRef: v1.ResolverRef{
888+
Resolver: "foobar",
889+
},
890+
},
891+
},
892+
{
893+
Name: "third-inline",
894+
Image: "ubuntu",
895+
},
896+
{
897+
Name: "fourth-remote",
898+
Ref: &v1.Ref{
899+
Name: "second-stepaction",
900+
},
901+
},
902+
},
903+
},
904+
},
905+
},
906+
stepActions: []*v1beta1.StepAction{
907+
firstStepAction,
908+
{
909+
ObjectMeta: metav1.ObjectMeta{
910+
Name: "second-stepaction",
911+
Namespace: "default",
912+
},
913+
Spec: v1beta1.StepActionSpec{
914+
Image: "myimage",
915+
},
916+
},
917+
},
918+
want: v1.TaskRunStatus{
919+
TaskRunStatusFields: v1.TaskRunStatusFields{
920+
Steps: []v1.StepState{
921+
{
922+
Name: "first-inline",
923+
Provenance: &v1.Provenance{},
924+
},
925+
{
926+
Name: "second-remote",
927+
Provenance: &v1.Provenance{
928+
RefSource: &v1.RefSource{
929+
URI: "ref-source",
930+
Digest: map[string]string{"sha256": "abcd123456"},
931+
},
932+
},
933+
},
934+
{
935+
Name: "third-inline",
936+
Provenance: &v1.Provenance{},
937+
},
938+
{
939+
Name: "fourth-remote",
940+
Provenance: &v1.Provenance{},
941+
},
942+
},
943+
},
944+
},
945+
},
946+
}
947+
948+
for _, tt := range tests {
949+
t.Run(tt.name, func(t *testing.T) {
950+
ctx := t.Context()
951+
tektonclient := fake.NewSimpleClientset()
952+
for _, sa := range tt.stepActions {
953+
if err := tektonclient.Tracker().Add(sa); err != nil {
954+
t.Fatal(err)
955+
}
956+
}
957+
958+
_, err := GetStepActionsData(ctx, *tt.tr.Spec.TaskSpec, tt.tr, tektonclient, nil, requester)
959+
if err != nil {
960+
t.Fatalf("Did not expect an error but got : %s", err)
961+
}
962+
if d := cmp.Diff(tt.want, tt.tr.Status); d != "" {
963+
t.Errorf("the taskrun status did not match what was expected diff: %s", diff.PrintWantGot(d))
964+
}
965+
})
966+
}
967+
}
968+
759969
func TestGetStepActionsData(t *testing.T) {
760970
taskRunUser := int64(1001)
761971
stepActionUser := int64(1000)

0 commit comments

Comments
 (0)