Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Replace v1beta1.TaskObject with *v1beta1.Task in TaskRun Reconciler #6471

Merged
merged 1 commit into from
Apr 3, 2023
Merged

Replace v1beta1.TaskObject with *v1beta1.Task in TaskRun Reconciler #6471

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -692,7 +692,7 @@ func resolveTask(
pipelineTask v1beta1.PipelineTask,
) (v1beta1.TaskSpec, string, v1beta1.TaskKind, error) {
var (
t v1beta1.TaskObject
t *v1beta1.Task
err error
spec v1beta1.TaskSpec
taskName string
Expand Down
18 changes: 9 additions & 9 deletions pkg/reconciler/pipelinerun/resources/pipelinerunresolution_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ import (
func nopGetRun(string) (v1beta1.RunObject, error) {
return nil, errors.New("GetRun should not be called")
}
func nopGetTask(context.Context, string) (v1beta1.TaskObject, *v1beta1.ConfigSource, error) {
func nopGetTask(context.Context, string) (*v1beta1.Task, *v1beta1.ConfigSource, error) {
return nil, nil, errors.New("GetTask should not be called")
}
func nopGetTaskRun(string) (*v1beta1.TaskRun, error) {
Expand Down Expand Up @@ -1871,7 +1871,7 @@ func TestResolvePipelineRun_PipelineTaskHasNoResources(t *testing.T) {
TaskRef: &v1beta1.TaskRef{Name: "task"},
}}

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

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

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

getTask := func(_ context.Context, name string) (v1beta1.TaskObject, *v1beta1.ConfigSource, error) {
getTask := func(_ context.Context, name string) (*v1beta1.Task, *v1beta1.ConfigSource, error) {
return task, nil, nil
}
pr := v1beta1.PipelineRun{
Expand Down Expand Up @@ -2232,7 +2232,7 @@ func TestIsCustomTask(t *testing.T) {
Name: "pipelinerun",
},
}
getTask := func(ctx context.Context, name string) (v1beta1.TaskObject, *v1beta1.ConfigSource, error) {
getTask := func(ctx context.Context, name string) (*v1beta1.Task, *v1beta1.ConfigSource, error) {
return task, nil, nil
}
getTaskRun := func(name string) (*v1beta1.TaskRun, error) { return nil, nil }
Expand Down Expand Up @@ -2999,7 +2999,7 @@ func TestIsMatrixed(t *testing.T) {
Name: "pipelinerun",
},
}
getTask := func(ctx context.Context, name string) (v1beta1.TaskObject, *v1beta1.ConfigSource, error) {
getTask := func(ctx context.Context, name string) (*v1beta1.Task, *v1beta1.ConfigSource, error) {
return task, nil, nil
}
getTaskRun := func(name string) (*v1beta1.TaskRun, error) { return &trs[0], nil }
Expand Down Expand Up @@ -3133,7 +3133,7 @@ func TestResolvePipelineRunTask_WithMatrix(t *testing.T) {
}}},
}

getTask := func(ctx context.Context, name string) (v1beta1.TaskObject, *v1beta1.ConfigSource, error) {
getTask := func(ctx context.Context, name string) (*v1beta1.Task, *v1beta1.ConfigSource, error) {
return task, nil, nil
}
getTaskRun := func(name string) (*v1beta1.TaskRun, error) { return taskRunsMap[name], nil }
Expand Down Expand Up @@ -3237,7 +3237,7 @@ func TestResolvePipelineRunTask_WithMatrixedCustomTask(t *testing.T) {
}}},
}}

getTask := func(ctx context.Context, name string) (v1beta1.TaskObject, *v1beta1.ConfigSource, error) {
getTask := func(ctx context.Context, name string) (*v1beta1.Task, *v1beta1.ConfigSource, error) {
return task, nil, nil
}
getTaskRun := func(name string) (*v1beta1.TaskRun, error) { return &trs[0], nil }
Expand Down
48 changes: 34 additions & 14 deletions pkg/reconciler/taskrun/resources/taskref.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ func GetTaskFuncFromTaskRun(ctx context.Context, k8s kubernetes.Interface, tekto
// if the spec is already in the status, do not try to fetch it again, just use it as source of truth.
// Same for the Source field in the Status.Provenance.
if taskrun.Status.TaskSpec != nil {
return func(_ context.Context, name string) (v1beta1.TaskObject, *v1beta1.ConfigSource, error) {
return func(_ context.Context, name string) (*v1beta1.Task, *v1beta1.ConfigSource, error) {
var configsource *v1beta1.ConfigSource
if taskrun.Status.Provenance != nil {
configsource = taskrun.Status.Provenance.ConfigSource
Expand All @@ -85,7 +85,7 @@ func GetVerifiedTaskFunc(ctx context.Context, k8s kubernetes.Interface, tekton c
owner kmeta.OwnerRefable, taskref *v1beta1.TaskRef, trName string, namespace, saName string, verificationpolicies []*v1alpha1.VerificationPolicy) GetTask {
get := GetTaskFunc(ctx, k8s, tekton, requester, owner, taskref, trName, namespace, saName)

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

// readRuntimeObjectAsTask tries to convert a generic runtime.Object
// into a v1beta1.TaskObject type so that its meta and spec fields
// into a *v1beta1.Task type so that its meta and spec fields
// can be read. v1 object will be converted to v1beta1 and returned.
// An error is returned if the given object is not a
// TaskObject or if there is an error validating or upgrading an
// older TaskObject into its v1beta1 equivalent.
// An error is returned if the given object is not a Task nor a ClusterTask
// or if there is an error validating or upgrading an older TaskObject into
// its v1beta1 equivalent.
// TODO(#5541): convert v1beta1 obj to v1 once we use v1 as the stored version
func readRuntimeObjectAsTask(ctx context.Context, obj runtime.Object) (v1beta1.TaskObject, error) {
func readRuntimeObjectAsTask(ctx context.Context, obj runtime.Object) (*v1beta1.Task, error) {
switch obj := obj.(type) {
case v1beta1.TaskObject:
case *v1beta1.Task:
return obj, nil
case *v1beta1.ClusterTask:
return convertClusterTaskToTask(*obj), nil
case *v1.Task:
t := &v1beta1.Task{
TypeMeta: metav1.TypeMeta{
Expand All @@ -217,13 +219,13 @@ type LocalTaskRefResolver struct {
// return an error if it can't find an appropriate Task for any reason.
// TODO: if we want to set source for in-cluster task, set it here.
// https://github.com/tektoncd/pipeline/issues/5522
func (l *LocalTaskRefResolver) GetTask(ctx context.Context, name string) (v1beta1.TaskObject, *v1beta1.ConfigSource, error) {
func (l *LocalTaskRefResolver) GetTask(ctx context.Context, name string) (*v1beta1.Task, *v1beta1.ConfigSource, error) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we update the doc string here to reflect the conversion of Cluster Task to Task?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks Emma, good point! I updated the TaskObject and added the docstring at the helper function.

if l.Kind == v1beta1.ClusterTaskKind {
task, err := l.Tektonclient.TektonV1beta1().ClusterTasks().Get(ctx, name, metav1.GetOptions{})
if err != nil {
return nil, nil, err
}
return task, nil, nil
return convertClusterTaskToTask(*task), nil, nil
}

// If we are going to resolve this reference locally, we need a namespace scope.
Expand All @@ -241,3 +243,21 @@ func (l *LocalTaskRefResolver) GetTask(ctx context.Context, name string) (v1beta
func IsGetTaskErrTransient(err error) bool {
return strings.Contains(err.Error(), errEtcdLeaderChange)
}

// convertClusterTaskToTask converts deprecated v1beta1 ClusterTasks to Tasks for
// the rest of reconciling process since GetTask func and its upstream callers only
// fetches the task spec and stores it in the taskrun status while the kind info
// is not being used.
func convertClusterTaskToTask(ct v1beta1.ClusterTask) *v1beta1.Task {
t := &v1beta1.Task{
TypeMeta: metav1.TypeMeta{
Kind: "Task",
APIVersion: "tekton.dev/v1beta1",
},
}

t.Spec = ct.Spec
t.ObjectMeta.Name = ct.ObjectMeta.Name

return t
}
29 changes: 23 additions & 6 deletions pkg/reconciler/taskrun/resources/taskref_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,11 @@ func TestLocalTaskRef(t *testing.T) {
Name: "cluster-task",
Kind: "ClusterTask",
},
expected: &v1beta1.ClusterTask{
expected: &v1beta1.Task{
TypeMeta: metav1.TypeMeta{
APIVersion: "tekton.dev/v1beta1",
Kind: "Task",
},
ObjectMeta: metav1.ObjectMeta{
Name: "cluster-task",
},
Expand Down Expand Up @@ -397,11 +401,11 @@ func TestGetTaskFunc(t *testing.T) {
Kind: v1beta1.ClusterTaskKind,
Bundle: u.Host + "/remote-cluster-task",
},
expected: &v1beta1.ClusterTask{
expected: &v1beta1.Task{
ObjectMeta: metav1.ObjectMeta{Name: "simple"},
TypeMeta: metav1.TypeMeta{
APIVersion: "tekton.dev/v1beta1",
Kind: "ClusterTask",
Kind: "Task",
},
},
expectedKind: v1beta1.ClusterTaskKind,
Expand All @@ -422,8 +426,21 @@ func TestGetTaskFunc(t *testing.T) {
Name: "simple",
Kind: v1beta1.ClusterTaskKind,
},
expected: simpleClusterTask,
expectedKind: v1beta1.ClusterTaskKind,
expected: &v1beta1.Task{
ObjectMeta: metav1.ObjectMeta{
Name: "simple",
},
TypeMeta: metav1.TypeMeta{
APIVersion: "tekton.dev/v1beta1",
Kind: "Task",
},
Spec: v1beta1.TaskSpec{
Steps: []v1beta1.Step{{
Image: "something",
}},
},
},
expectedKind: v1beta1.NamespacedTaskKind,
},
}

Expand Down Expand Up @@ -892,7 +909,7 @@ func TestGetVerifiedTaskFunc_VerifyError(t *testing.T) {
name string
requester *test.Requester
verificationNoMatchPolicy string
expected runtime.Object
expected *v1beta1.Task
expectedErr error
}{{
name: "unsigned task with fails verification with fail no match policy",
Expand Down
2 changes: 1 addition & 1 deletion pkg/reconciler/taskrun/resources/taskspec.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ type ResolvedTask struct {
}

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

// GetTaskRun is a function used to retrieve TaskRuns
type GetTaskRun func(string) (*v1beta1.TaskRun, error)
Expand Down
14 changes: 7 additions & 7 deletions pkg/reconciler/taskrun/resources/taskspec_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ func TestGetTaskSpec_Ref(t *testing.T) {
},
}

gt := func(ctx context.Context, n string) (v1beta1.TaskObject, *v1beta1.ConfigSource, error) {
gt := func(ctx context.Context, n string) (*v1beta1.Task, *v1beta1.ConfigSource, error) {
return task, sampleConfigSource.DeepCopy(), nil
}
resolvedObjectMeta, taskSpec, err := resources.GetTaskData(context.Background(), tr, gt)
Expand Down Expand Up @@ -84,7 +84,7 @@ func TestGetTaskSpec_Embedded(t *testing.T) {
},
},
}
gt := func(ctx context.Context, n string) (v1beta1.TaskObject, *v1beta1.ConfigSource, error) {
gt := func(ctx context.Context, n string) (*v1beta1.Task, *v1beta1.ConfigSource, error) {
return nil, nil, errors.New("shouldn't be called")
}
resolvedObjectMeta, taskSpec, err := resources.GetTaskData(context.Background(), tr, gt)
Expand Down Expand Up @@ -113,7 +113,7 @@ func TestGetTaskSpec_Invalid(t *testing.T) {
Name: "mytaskrun",
},
}
gt := func(ctx context.Context, n string) (v1beta1.TaskObject, *v1beta1.ConfigSource, error) {
gt := func(ctx context.Context, n string) (*v1beta1.Task, *v1beta1.ConfigSource, error) {
return nil, nil, errors.New("shouldn't be called")
}
_, _, err := resources.GetTaskData(context.Background(), tr, gt)
Expand All @@ -133,7 +133,7 @@ func TestGetTaskSpec_Error(t *testing.T) {
},
},
}
gt := func(ctx context.Context, n string) (v1beta1.TaskObject, *v1beta1.ConfigSource, error) {
gt := func(ctx context.Context, n string) (*v1beta1.Task, *v1beta1.ConfigSource, error) {
return nil, nil, errors.New("something went wrong")
}
_, _, err := resources.GetTaskData(context.Background(), tr, gt)
Expand Down Expand Up @@ -173,7 +173,7 @@ func TestGetTaskData_ResolutionSuccess(t *testing.T) {
}},
}

getTask := func(ctx context.Context, n string) (v1beta1.TaskObject, *v1beta1.ConfigSource, error) {
getTask := func(ctx context.Context, n string) (*v1beta1.Task, *v1beta1.ConfigSource, error) {
return &v1beta1.Task{
ObjectMeta: *sourceMeta.DeepCopy(),
Spec: *sourceSpec.DeepCopy(),
Expand Down Expand Up @@ -210,7 +210,7 @@ func TestGetPipelineData_ResolutionError(t *testing.T) {
},
},
}
getTask := func(ctx context.Context, n string) (v1beta1.TaskObject, *v1beta1.ConfigSource, error) {
getTask := func(ctx context.Context, n string) (*v1beta1.Task, *v1beta1.ConfigSource, error) {
return nil, nil, errors.New("something went wrong")
}
ctx := context.Background()
Expand All @@ -233,7 +233,7 @@ func TestGetTaskData_ResolvedNilTask(t *testing.T) {
},
},
}
getTask := func(ctx context.Context, n string) (v1beta1.TaskObject, *v1beta1.ConfigSource, error) {
getTask := func(ctx context.Context, n string) (*v1beta1.Task, *v1beta1.ConfigSource, error) {
return nil, nil, nil
}
ctx := context.Background()
Expand Down
2 changes: 1 addition & 1 deletion pkg/trustedresources/verify.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ const (
// Return an error when no policies are found and trusted-resources-verification-no-match-policy is set to fail,
// or the resource fails to pass matched enforce verification policy
// source is from ConfigSource.URI, which will be used to match policy patterns. k8s is used to fetch secret from cluster
func VerifyTask(ctx context.Context, taskObj v1beta1.TaskObject, k8s kubernetes.Interface, source string, verificationpolicies []*v1alpha1.VerificationPolicy) error {
func VerifyTask(ctx context.Context, taskObj *v1beta1.Task, k8s kubernetes.Interface, source string, verificationpolicies []*v1alpha1.VerificationPolicy) error {
matchedPolicies, err := getMatchedPolicies(taskObj.TaskMetadata().Name, source, verificationpolicies)
if err != nil {
if errors.Is(err, ErrNoMatchedPolicies) {
Expand Down
4 changes: 2 additions & 2 deletions pkg/trustedresources/verify_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,7 @@ func TestVerifyTask_Success(t *testing.T) {
mismatchedSource := "wrong source"
tcs := []struct {
name string
task v1beta1.TaskObject
task *v1beta1.Task
source string
signer signature.SignerVerifier
verificationNoMatchPolicy string
Expand Down Expand Up @@ -276,7 +276,7 @@ func TestVerifyTask_Error(t *testing.T) {
mismatchedSource := "wrong source"
tcs := []struct {
name string
task v1beta1.TaskObject
task *v1beta1.Task
source string
verificationPolicy []*v1alpha1.VerificationPolicy
expectedError error
Expand Down