Skip to content

Commit c68b7d7

Browse files
JeromeJuJeromeJu
committed
Replace v1beta1.TaskObject with *v1beta1.Task in TaskRun Reconciler
This commit replaces the v1beta1.TaskObject interface with the *v1beta1.Task by converting ClusterTask to Task for resolving Tasks for TaskRun. The deprecated v1beta1 ClusterTasks are converted to Tasks since the GetTask func and its upstream callers only fetch a task spec and store it in the taskrun status while the kind info is not being used.
1 parent 54c0f84 commit c68b7d7

File tree

8 files changed

+78
-41
lines changed

8 files changed

+78
-41
lines changed

‎pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -692,7 +692,7 @@ func resolveTask(
692692
pipelineTask v1beta1.PipelineTask,
693693
) (v1beta1.TaskSpec, string, v1beta1.TaskKind, error) {
694694
var (
695-
t v1beta1.TaskObject
695+
t *v1beta1.Task
696696
err error
697697
spec v1beta1.TaskSpec
698698
taskName string

‎pkg/reconciler/pipelinerun/resources/pipelinerunresolution_test.go‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ import (
4747
func nopGetRun(string) (v1beta1.RunObject, error) {
4848
return nil, errors.New("GetRun should not be called")
4949
}
50-
func nopGetTask(context.Context, string) (v1beta1.TaskObject, *v1beta1.ConfigSource, error) {
50+
func nopGetTask(context.Context, string) (*v1beta1.Task, *v1beta1.ConfigSource, error) {
5151
return nil, nil, errors.New("GetTask should not be called")
5252
}
5353
func nopGetTaskRun(string) (*v1beta1.TaskRun, error) {
@@ -1871,7 +1871,7 @@ func TestResolvePipelineRun_PipelineTaskHasNoResources(t *testing.T) {
18711871
TaskRef: &v1beta1.TaskRef{Name: "task"},
18721872
}}
18731873

1874-
getTask := func(ctx context.Context, name string) (v1beta1.TaskObject, *v1beta1.ConfigSource, error) {
1874+
getTask := func(ctx context.Context, name string) (*v1beta1.Task, *v1beta1.ConfigSource, error) {
18751875
return task, nil, nil
18761876
}
18771877
getTaskRun := func(name string) (*v1beta1.TaskRun, error) { return &trs[0], nil }
@@ -1921,7 +1921,7 @@ func TestResolvePipelineRun_TaskDoesntExist(t *testing.T) {
19211921
}}}
19221922

19231923
// Return an error when the Task is retrieved, as if it didn't exist
1924-
getTask := func(ctx context.Context, name string) (v1beta1.TaskObject, *v1beta1.ConfigSource, error) {
1924+
getTask := func(ctx context.Context, name string) (*v1beta1.Task, *v1beta1.ConfigSource, error) {
19251925
return nil, nil, kerrors.NewNotFound(v1beta1.Resource("task"), name)
19261926
}
19271927
getTaskRun := func(name string) (*v1beta1.TaskRun, error) {
@@ -1962,7 +1962,7 @@ func TestResolvePipelineRun_VerificationFailed(t *testing.T) {
19621962
}},
19631963
}}}
19641964

1965-
getTask := func(ctx context.Context, name string) (v1beta1.TaskObject, *v1beta1.ConfigSource, error) {
1965+
getTask := func(ctx context.Context, name string) (*v1beta1.Task, *v1beta1.ConfigSource, error) {
19661966
return nil, nil, trustedresources.ErrResourceVerificationFailed
19671967
}
19681968
getTaskRun := func(name string) (*v1beta1.TaskRun, error) { return nil, nil }
@@ -2199,7 +2199,7 @@ func TestResolvePipeline_WhenExpressions(t *testing.T) {
21992199
WhenExpressions: []v1beta1.WhenExpression{ptwe1},
22002200
}
22012201

2202-
getTask := func(_ context.Context, name string) (v1beta1.TaskObject, *v1beta1.ConfigSource, error) {
2202+
getTask := func(_ context.Context, name string) (*v1beta1.Task, *v1beta1.ConfigSource, error) {
22032203
return task, nil, nil
22042204
}
22052205
pr := v1beta1.PipelineRun{
@@ -2232,7 +2232,7 @@ func TestIsCustomTask(t *testing.T) {
22322232
Name: "pipelinerun",
22332233
},
22342234
}
2235-
getTask := func(ctx context.Context, name string) (v1beta1.TaskObject, *v1beta1.ConfigSource, error) {
2235+
getTask := func(ctx context.Context, name string) (*v1beta1.Task, *v1beta1.ConfigSource, error) {
22362236
return task, nil, nil
22372237
}
22382238
getTaskRun := func(name string) (*v1beta1.TaskRun, error) { return nil, nil }
@@ -2999,7 +2999,7 @@ func TestIsMatrixed(t *testing.T) {
29992999
Name: "pipelinerun",
30003000
},
30013001
}
3002-
getTask := func(ctx context.Context, name string) (v1beta1.TaskObject, *v1beta1.ConfigSource, error) {
3002+
getTask := func(ctx context.Context, name string) (*v1beta1.Task, *v1beta1.ConfigSource, error) {
30033003
return task, nil, nil
30043004
}
30053005
getTaskRun := func(name string) (*v1beta1.TaskRun, error) { return &trs[0], nil }
@@ -3133,7 +3133,7 @@ func TestResolvePipelineRunTask_WithMatrix(t *testing.T) {
31333133
}}},
31343134
}
31353135

3136-
getTask := func(ctx context.Context, name string) (v1beta1.TaskObject, *v1beta1.ConfigSource, error) {
3136+
getTask := func(ctx context.Context, name string) (*v1beta1.Task, *v1beta1.ConfigSource, error) {
31373137
return task, nil, nil
31383138
}
31393139
getTaskRun := func(name string) (*v1beta1.TaskRun, error) { return taskRunsMap[name], nil }
@@ -3237,7 +3237,7 @@ func TestResolvePipelineRunTask_WithMatrixedCustomTask(t *testing.T) {
32373237
}}},
32383238
}}
32393239

3240-
getTask := func(ctx context.Context, name string) (v1beta1.TaskObject, *v1beta1.ConfigSource, error) {
3240+
getTask := func(ctx context.Context, name string) (*v1beta1.Task, *v1beta1.ConfigSource, error) {
32413241
return task, nil, nil
32423242
}
32433243
getTaskRun := func(name string) (*v1beta1.TaskRun, error) { return &trs[0], nil }

‎pkg/reconciler/taskrun/resources/taskref.go‎

Lines changed: 34 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ func GetTaskFuncFromTaskRun(ctx context.Context, k8s kubernetes.Interface, tekto
6262
// if the spec is already in the status, do not try to fetch it again, just use it as source of truth.
6363
// Same for the Source field in the Status.Provenance.
6464
if taskrun.Status.TaskSpec != nil {
65-
return func(_ context.Context, name string) (v1beta1.TaskObject, *v1beta1.ConfigSource, error) {
65+
return func(_ context.Context, name string) (*v1beta1.Task, *v1beta1.ConfigSource, error) {
6666
var configsource *v1beta1.ConfigSource
6767
if taskrun.Status.Provenance != nil {
6868
configsource = taskrun.Status.Provenance.ConfigSource
@@ -85,7 +85,7 @@ func GetVerifiedTaskFunc(ctx context.Context, k8s kubernetes.Interface, tekton c
8585
owner kmeta.OwnerRefable, taskref *v1beta1.TaskRef, trName string, namespace, saName string, verificationpolicies []*v1alpha1.VerificationPolicy) GetTask {
8686
get := GetTaskFunc(ctx, k8s, tekton, requester, owner, taskref, trName, namespace, saName)
8787

88-
return func(context.Context, string) (v1beta1.TaskObject, *v1beta1.ConfigSource, error) {
88+
return func(context.Context, string) (*v1beta1.Task, *v1beta1.ConfigSource, error) {
8989
t, s, err := get(ctx, taskref.Name)
9090
if err != nil {
9191
return nil, nil, fmt.Errorf("failed to get task: %w", err)
@@ -117,7 +117,7 @@ func GetTaskFunc(ctx context.Context, k8s kubernetes.Interface, tekton clientset
117117
case cfg.FeatureFlags.EnableTektonOCIBundles && tr != nil && tr.Bundle != "":
118118
// Return an inline function that implements GetTask by calling Resolver.Get with the specified task type and
119119
// casting it to a TaskObject.
120-
return func(ctx context.Context, name string) (v1beta1.TaskObject, *v1beta1.ConfigSource, error) {
120+
return func(ctx context.Context, name string) (*v1beta1.Task, *v1beta1.ConfigSource, error) {
121121
// If there is a bundle url at all, construct an OCI resolver to fetch the task.
122122
kc, err := k8schain.New(ctx, k8s, k8schain.Options{
123123
Namespace: namespace,
@@ -133,7 +133,7 @@ func GetTaskFunc(ctx context.Context, k8s kubernetes.Interface, tekton clientset
133133
case tr != nil && tr.Resolver != "" && requester != nil:
134134
// Return an inline function that implements GetTask by calling Resolver.Get with the specified task type and
135135
// casting it to a TaskObject.
136-
return func(ctx context.Context, name string) (v1beta1.TaskObject, *v1beta1.ConfigSource, error) {
136+
return func(ctx context.Context, name string) (*v1beta1.Task, *v1beta1.ConfigSource, error) {
137137
var replacedParams v1beta1.Params
138138
if ownerAsTR, ok := owner.(*v1beta1.TaskRun); ok {
139139
stringReplacements, arrayReplacements := paramsFromTaskRun(ctx, ownerAsTR)
@@ -165,8 +165,8 @@ func GetTaskFunc(ctx context.Context, k8s kubernetes.Interface, tekton clientset
165165
// resolveTask accepts an impl of remote.Resolver and attempts to
166166
// fetch a task with given name. An error is returned if the
167167
// remoteresource doesn't work or the returned data isn't a valid
168-
// v1beta1.TaskObject.
169-
func resolveTask(ctx context.Context, resolver remote.Resolver, name string, kind v1beta1.TaskKind, k8s kubernetes.Interface) (v1beta1.TaskObject, *v1beta1.ConfigSource, error) {
168+
// *v1beta1.Task.
169+
func resolveTask(ctx context.Context, resolver remote.Resolver, name string, kind v1beta1.TaskKind, k8s kubernetes.Interface) (*v1beta1.Task, *v1beta1.ConfigSource, error) {
170170
// Because the resolver will only return references with the same kind (eg ClusterTask), this will ensure we
171171
// don't accidentally return a Task with the same name but different kind.
172172
obj, configSource, err := resolver.Get(ctx, strings.TrimSuffix(strings.ToLower(string(kind)), "s"), name)
@@ -181,16 +181,18 @@ func resolveTask(ctx context.Context, resolver remote.Resolver, name string, kin
181181
}
182182

183183
// readRuntimeObjectAsTask tries to convert a generic runtime.Object
184-
// into a v1beta1.TaskObject type so that its meta and spec fields
184+
// into a *v1beta1.Task type so that its meta and spec fields
185185
// can be read. v1 object will be converted to v1beta1 and returned.
186-
// An error is returned if the given object is not a
187-
// TaskObject or if there is an error validating or upgrading an
188-
// older TaskObject into its v1beta1 equivalent.
186+
// An error is returned if the given object is not a Task nor a ClusterTask
187+
// or if there is an error validating or upgrading an older TaskObject into
188+
// its v1beta1 equivalent.
189189
// TODO(#5541): convert v1beta1 obj to v1 once we use v1 as the stored version
190-
func readRuntimeObjectAsTask(ctx context.Context, obj runtime.Object) (v1beta1.TaskObject, error) {
190+
func readRuntimeObjectAsTask(ctx context.Context, obj runtime.Object) (*v1beta1.Task, error) {
191191
switch obj := obj.(type) {
192-
case v1beta1.TaskObject:
192+
case *v1beta1.Task:
193193
return obj, nil
194+
case *v1beta1.ClusterTask:
195+
return convertClusterTaskToTask(*obj), nil
194196
case *v1.Task:
195197
t := &v1beta1.Task{
196198
TypeMeta: metav1.TypeMeta{
@@ -217,13 +219,13 @@ type LocalTaskRefResolver struct {
217219
// return an error if it can't find an appropriate Task for any reason.
218220
// TODO: if we want to set source for in-cluster task, set it here.
219221
// https://github.com/tektoncd/pipeline/issues/5522
220-
func (l *LocalTaskRefResolver) GetTask(ctx context.Context, name string) (v1beta1.TaskObject, *v1beta1.ConfigSource, error) {
222+
func (l *LocalTaskRefResolver) GetTask(ctx context.Context, name string) (*v1beta1.Task, *v1beta1.ConfigSource, error) {
221223
if l.Kind == v1beta1.ClusterTaskKind {
222224
task, err := l.Tektonclient.TektonV1beta1().ClusterTasks().Get(ctx, name, metav1.GetOptions{})
223225
if err != nil {
224226
return nil, nil, err
225227
}
226-
return task, nil, nil
228+
return convertClusterTaskToTask(*task), nil, nil
227229
}
228230

229231
// If we are going to resolve this reference locally, we need a namespace scope.
@@ -241,3 +243,21 @@ func (l *LocalTaskRefResolver) GetTask(ctx context.Context, name string) (v1beta
241243
func IsGetTaskErrTransient(err error) bool {
242244
return strings.Contains(err.Error(), errEtcdLeaderChange)
243245
}
246+
247+
// convertClusterTaskToTask converts deprecated v1beta1 ClusterTasks to Tasks for
248+
// the rest of reconciling process since GetTask func and its upstream callers only
249+
// fetches the task spec and stores it in the taskrun status while the kind info
250+
// is not being used.
251+
func convertClusterTaskToTask(ct v1beta1.ClusterTask) *v1beta1.Task {
252+
t := &v1beta1.Task{
253+
TypeMeta: metav1.TypeMeta{
254+
Kind: "Task",
255+
APIVersion: "tekton.dev/v1beta1",
256+
},
257+
}
258+
259+
t.Spec = ct.Spec
260+
t.ObjectMeta.Name = ct.ObjectMeta.Name
261+
262+
return t
263+
}

‎pkg/reconciler/taskrun/resources/taskref_test.go‎

Lines changed: 23 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -184,7 +184,11 @@ func TestLocalTaskRef(t *testing.T) {
184184
Name: "cluster-task",
185185
Kind: "ClusterTask",
186186
},
187-
expected: &v1beta1.ClusterTask{
187+
expected: &v1beta1.Task{
188+
TypeMeta: metav1.TypeMeta{
189+
APIVersion: "tekton.dev/v1beta1",
190+
Kind: "Task",
191+
},
188192
ObjectMeta: metav1.ObjectMeta{
189193
Name: "cluster-task",
190194
},
@@ -397,11 +401,11 @@ func TestGetTaskFunc(t *testing.T) {
397401
Kind: v1beta1.ClusterTaskKind,
398402
Bundle: u.Host + "/remote-cluster-task",
399403
},
400-
expected: &v1beta1.ClusterTask{
404+
expected: &v1beta1.Task{
401405
ObjectMeta: metav1.ObjectMeta{Name: "simple"},
402406
TypeMeta: metav1.TypeMeta{
403407
APIVersion: "tekton.dev/v1beta1",
404-
Kind: "ClusterTask",
408+
Kind: "Task",
405409
},
406410
},
407411
expectedKind: v1beta1.ClusterTaskKind,
@@ -422,8 +426,21 @@ func TestGetTaskFunc(t *testing.T) {
422426
Name: "simple",
423427
Kind: v1beta1.ClusterTaskKind,
424428
},
425-
expected: simpleClusterTask,
426-
expectedKind: v1beta1.ClusterTaskKind,
429+
expected: &v1beta1.Task{
430+
ObjectMeta: metav1.ObjectMeta{
431+
Name: "simple",
432+
},
433+
TypeMeta: metav1.TypeMeta{
434+
APIVersion: "tekton.dev/v1beta1",
435+
Kind: "Task",
436+
},
437+
Spec: v1beta1.TaskSpec{
438+
Steps: []v1beta1.Step{{
439+
Image: "something",
440+
}},
441+
},
442+
},
443+
expectedKind: v1beta1.NamespacedTaskKind,
427444
},
428445
}
429446

@@ -892,7 +909,7 @@ func TestGetVerifiedTaskFunc_VerifyError(t *testing.T) {
892909
name string
893910
requester *test.Requester
894911
verificationNoMatchPolicy string
895-
expected runtime.Object
912+
expected *v1beta1.Task
896913
expectedErr error
897914
}{{
898915
name: "unsigned task with fails verification with fail no match policy",

‎pkg/reconciler/taskrun/resources/taskspec.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ type ResolvedTask struct {
3535
}
3636

3737
// GetTask is a function used to retrieve Tasks.
38-
type GetTask func(context.Context, string) (v1beta1.TaskObject, *v1beta1.ConfigSource, error)
38+
type GetTask func(context.Context, string) (*v1beta1.Task, *v1beta1.ConfigSource, error)
3939

4040
// GetTaskRun is a function used to retrieve TaskRuns
4141
type GetTaskRun func(string) (*v1beta1.TaskRun, error)

‎pkg/reconciler/taskrun/resources/taskspec_test.go‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ func TestGetTaskSpec_Ref(t *testing.T) {
5050
},
5151
}
5252

53-
gt := func(ctx context.Context, n string) (v1beta1.TaskObject, *v1beta1.ConfigSource, error) {
53+
gt := func(ctx context.Context, n string) (*v1beta1.Task, *v1beta1.ConfigSource, error) {
5454
return task, sampleConfigSource.DeepCopy(), nil
5555
}
5656
resolvedObjectMeta, taskSpec, err := resources.GetTaskData(context.Background(), tr, gt)
@@ -84,7 +84,7 @@ func TestGetTaskSpec_Embedded(t *testing.T) {
8484
},
8585
},
8686
}
87-
gt := func(ctx context.Context, n string) (v1beta1.TaskObject, *v1beta1.ConfigSource, error) {
87+
gt := func(ctx context.Context, n string) (*v1beta1.Task, *v1beta1.ConfigSource, error) {
8888
return nil, nil, errors.New("shouldn't be called")
8989
}
9090
resolvedObjectMeta, taskSpec, err := resources.GetTaskData(context.Background(), tr, gt)
@@ -113,7 +113,7 @@ func TestGetTaskSpec_Invalid(t *testing.T) {
113113
Name: "mytaskrun",
114114
},
115115
}
116-
gt := func(ctx context.Context, n string) (v1beta1.TaskObject, *v1beta1.ConfigSource, error) {
116+
gt := func(ctx context.Context, n string) (*v1beta1.Task, *v1beta1.ConfigSource, error) {
117117
return nil, nil, errors.New("shouldn't be called")
118118
}
119119
_, _, err := resources.GetTaskData(context.Background(), tr, gt)
@@ -133,7 +133,7 @@ func TestGetTaskSpec_Error(t *testing.T) {
133133
},
134134
},
135135
}
136-
gt := func(ctx context.Context, n string) (v1beta1.TaskObject, *v1beta1.ConfigSource, error) {
136+
gt := func(ctx context.Context, n string) (*v1beta1.Task, *v1beta1.ConfigSource, error) {
137137
return nil, nil, errors.New("something went wrong")
138138
}
139139
_, _, err := resources.GetTaskData(context.Background(), tr, gt)
@@ -173,7 +173,7 @@ func TestGetTaskData_ResolutionSuccess(t *testing.T) {
173173
}},
174174
}
175175

176-
getTask := func(ctx context.Context, n string) (v1beta1.TaskObject, *v1beta1.ConfigSource, error) {
176+
getTask := func(ctx context.Context, n string) (*v1beta1.Task, *v1beta1.ConfigSource, error) {
177177
return &v1beta1.Task{
178178
ObjectMeta: *sourceMeta.DeepCopy(),
179179
Spec: *sourceSpec.DeepCopy(),
@@ -210,7 +210,7 @@ func TestGetPipelineData_ResolutionError(t *testing.T) {
210210
},
211211
},
212212
}
213-
getTask := func(ctx context.Context, n string) (v1beta1.TaskObject, *v1beta1.ConfigSource, error) {
213+
getTask := func(ctx context.Context, n string) (*v1beta1.Task, *v1beta1.ConfigSource, error) {
214214
return nil, nil, errors.New("something went wrong")
215215
}
216216
ctx := context.Background()
@@ -233,7 +233,7 @@ func TestGetTaskData_ResolvedNilTask(t *testing.T) {
233233
},
234234
},
235235
}
236-
getTask := func(ctx context.Context, n string) (v1beta1.TaskObject, *v1beta1.ConfigSource, error) {
236+
getTask := func(ctx context.Context, n string) (*v1beta1.Task, *v1beta1.ConfigSource, error) {
237237
return nil, nil, nil
238238
}
239239
ctx := context.Background()

‎pkg/trustedresources/verify.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ const (
4646
// Return an error when no policies are found and trusted-resources-verification-no-match-policy is set to fail,
4747
// or the resource fails to pass matched enforce verification policy
4848
// source is from ConfigSource.URI, which will be used to match policy patterns. k8s is used to fetch secret from cluster
49-
func VerifyTask(ctx context.Context, taskObj v1beta1.TaskObject, k8s kubernetes.Interface, source string, verificationpolicies []*v1alpha1.VerificationPolicy) error {
49+
func VerifyTask(ctx context.Context, taskObj *v1beta1.Task, k8s kubernetes.Interface, source string, verificationpolicies []*v1alpha1.VerificationPolicy) error {
5050
matchedPolicies, err := getMatchedPolicies(taskObj.TaskMetadata().Name, source, verificationpolicies)
5151
if err != nil {
5252
if errors.Is(err, ErrNoMatchedPolicies) {

‎pkg/trustedresources/verify_test.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -199,7 +199,7 @@ func TestVerifyTask_Success(t *testing.T) {
199199
mismatchedSource := "wrong source"
200200
tcs := []struct {
201201
name string
202-
task v1beta1.TaskObject
202+
task *v1beta1.Task
203203
source string
204204
signer signature.SignerVerifier
205205
verificationNoMatchPolicy string
@@ -276,7 +276,7 @@ func TestVerifyTask_Error(t *testing.T) {
276276
mismatchedSource := "wrong source"
277277
tcs := []struct {
278278
name string
279-
task v1beta1.TaskObject
279+
task *v1beta1.Task
280280
source string
281281
verificationPolicy []*v1alpha1.VerificationPolicy
282282
expectedError error

0 commit comments

Comments
 (0)