Skip to content

Commit

Permalink
Separate v1beta1.TaskObject to Task and ClusterTask in TaskRun Reconc…
Browse files Browse the repository at this point in the history 
…iler

This commit separates v1beta1.TaskObject to Task and ClusterTask to get prepared for v1 storage swap.
  • Loading branch information
@JeromeJu
JeromeJu committed Mar 30, 2023
1 parent 87aa800 commit b8c2462
Show file tree
Hide file tree
Showing 8 changed files with 241 additions and 161 deletions.
15 changes: 11 additions & 4 deletions pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -692,7 +692,8 @@ func resolveTask(
pipelineTask v1beta1.PipelineTask,
) (v1beta1.TaskSpec, string, v1beta1.TaskKind, error) {
var (
t v1beta1.TaskObject
t *v1beta1.Task
ct *v1beta1.ClusterTask
err error
spec v1beta1.TaskSpec
taskName string
Expand All @@ -707,7 +708,7 @@ func resolveTask(
} else {
// Following minimum status principle (TEP-0100), no need to propagate the source about PipelineTask up to PipelineRun status.
// Instead, the child TaskRun's status will be the place recording the source of individual task.
t, _, err = getTask(ctx, pipelineTask.TaskRef.Name)
t, ct, _, err = getTask(ctx, pipelineTask.TaskRef.Name)
switch {
case errors.Is(err, remote.ErrRequestInProgress):
return v1beta1.TaskSpec{}, "", "", err
Expand All @@ -719,8 +720,14 @@ func resolveTask(
Msg: err.Error(),
}
default:
spec = t.TaskSpec()
taskName = t.TaskMetadata().Name
if ct == nil && t != nil {
spec = t.TaskSpec()
taskName = t.TaskMetadata().Name
}
if ct != nil && t == nil {
spec = ct.TaskSpec()
taskName = ct.TaskMetadata().Name
}
}
}
kind = pipelineTask.TaskRef.Kind
Expand Down
36 changes: 18 additions & 18 deletions pkg/reconciler/pipelinerun/resources/pipelinerunresolution_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,8 @@ 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) {
return nil, nil, errors.New("GetTask should not be called")
func nopGetTask(context.Context, string) (*v1beta1.Task, *v1beta1.ClusterTask, *v1beta1.ConfigSource, error) {
return nil, nil, nil, errors.New("GetTask should not be called")
}
func nopGetTaskRun(string) (*v1beta1.TaskRun, error) {
return nil, errors.New("GetTaskRun should not be called")
Expand Down Expand Up @@ -1871,8 +1871,8 @@ func TestResolvePipelineRun_PipelineTaskHasNoResources(t *testing.T) {
TaskRef: &v1beta1.TaskRef{Name: "task"},
}}

getTask := func(ctx context.Context, name string) (v1beta1.TaskObject, *v1beta1.ConfigSource, error) {
return task, nil, nil
getTask := func(ctx context.Context, name string) (*v1beta1.Task, *v1beta1.ClusterTask, *v1beta1.ConfigSource, error) {
return task, nil, nil, nil
}
getTaskRun := func(name string) (*v1beta1.TaskRun, error) { return &trs[0], nil }
pr := v1beta1.PipelineRun{
Expand Down Expand Up @@ -1921,8 +1921,8 @@ 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) {
return nil, nil, kerrors.NewNotFound(v1beta1.Resource("task"), name)
getTask := func(ctx context.Context, name string) (*v1beta1.Task, *v1beta1.ClusterTask, *v1beta1.ConfigSource, error) {
return nil, nil, nil, kerrors.NewNotFound(v1beta1.Resource("task"), name)
}
getTaskRun := func(name string) (*v1beta1.TaskRun, error) {
return nil, kerrors.NewNotFound(v1beta1.Resource("taskrun"), name)
Expand Down Expand Up @@ -1962,8 +1962,8 @@ func TestResolvePipelineRun_VerificationFailed(t *testing.T) {
}},
}}}

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

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

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

getTask := func(ctx context.Context, name string) (v1beta1.TaskObject, *v1beta1.ConfigSource, error) {
return task, nil, nil
getTask := func(ctx context.Context, name string) (*v1beta1.Task, *v1beta1.ClusterTask, *v1beta1.ConfigSource, error) {
return task, nil, nil, nil
}
getTaskRun := func(name string) (*v1beta1.TaskRun, error) { return &trs[0], nil }
getRun := func(name string) (v1beta1.RunObject, error) { return runsMap[name], nil }
Expand Down
70 changes: 38 additions & 32 deletions pkg/reconciler/taskrun/resources/taskref.go
View file
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.ClusterTask, *v1beta1.ConfigSource, error) {
var configsource *v1beta1.ConfigSource
if taskrun.Status.Provenance != nil {
configsource = taskrun.Status.Provenance.ConfigSource
Expand All @@ -73,7 +73,7 @@ func GetTaskFuncFromTaskRun(ctx context.Context, k8s kubernetes.Interface, tekto
Namespace: taskrun.Namespace,
},
Spec: *taskrun.Status.TaskSpec,
}, configsource, nil
}, nil, configsource, nil
}
}
return GetVerifiedTaskFunc(ctx, k8s, tekton, requester, taskrun, taskrun.Spec.TaskRef, taskrun.Name, taskrun.Namespace, taskrun.Spec.ServiceAccountName, verificationpolicies)
Expand All @@ -85,19 +85,23 @@ 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) {
t, s, err := get(ctx, taskref.Name)
return func(context.Context, string) (*v1beta1.Task, *v1beta1.ClusterTask, *v1beta1.ConfigSource, error) {
t, ct, s, err := get(ctx, taskref.Name)
if err != nil {
return nil, nil, fmt.Errorf("failed to get task: %w", err)
return nil, nil, nil, fmt.Errorf("failed to get task: %w", err)
}
// TODO: check if we do not need to pass in ClusterTask for verification
if ct != nil {
return nil, ct, s, nil
}
var source string
if s != nil {
source = s.URI
}
if err := trustedresources.VerifyTask(ctx, t, k8s, source, verificationpolicies); err != nil {
return nil, nil, fmt.Errorf("GetVerifiedTaskFunc failed: %w: %v", trustedresources.ErrResourceVerificationFailed, err)
return nil, nil, nil, fmt.Errorf("GetVerifiedTaskFunc failed: %w: %v", trustedresources.ErrResourceVerificationFailed, err)
}
return t, s, nil
return t, nil, s, nil
}
}

Expand All @@ -117,14 +121,14 @@ 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.ClusterTask, *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,
ServiceAccountName: saName,
})
if err != nil {
return nil, nil, fmt.Errorf("failed to get keychain: %w", err)
return nil, nil, nil, fmt.Errorf("failed to get keychain: %w", err)
}
resolver := oci.NewResolver(tr.Bundle, kc)

Expand All @@ -133,7 +137,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.ClusterTask, *v1beta1.ConfigSource, error) {
var replacedParams []v1beta1.Param
if ownerAsTR, ok := owner.(*v1beta1.TaskRun); ok {
stringReplacements, arrayReplacements := paramsFromTaskRun(ctx, ownerAsTR)
Expand Down Expand Up @@ -165,32 +169,34 @@ 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 or v1beta1.ClusterTask
func resolveTask(ctx context.Context, resolver remote.Resolver, name string, kind v1beta1.TaskKind, k8s kubernetes.Interface) (*v1beta1.Task, *v1beta1.ClusterTask, *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)
if err != nil {
return nil, nil, err
return nil, nil, nil, err
}
taskObj, err := readRuntimeObjectAsTask(ctx, obj)
taskObj, ct, err := readRuntimeObjectAsTask(ctx, obj)
if err != nil {
return nil, nil, fmt.Errorf("failed to convert obj %s into Task", obj.GetObjectKind().GroupVersionKind().String())
return nil, nil, nil, fmt.Errorf("failed to convert obj %s into Task", obj.GetObjectKind().GroupVersionKind().String())
}
return taskObj, configSource, nil
return taskObj, ct, configSource, nil
}

// readRuntimeObjectAsTask tries to convert a generic runtime.Object
// into a v1beta1.TaskObject 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
// into a v1beta1.Task or v1beta1.ClusterTask 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
// Task or 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, *v1beta1.ClusterTask, error) {
switch obj := obj.(type) {
case v1beta1.TaskObject:
return obj, nil
case *v1beta1.Task:
return obj, nil, nil
case *v1beta1.ClusterTask:
return nil, obj, nil
case *v1.Task:
t := &v1beta1.Task{
TypeMeta: metav1.TypeMeta{
Expand All @@ -199,11 +205,11 @@ func readRuntimeObjectAsTask(ctx context.Context, obj runtime.Object) (v1beta1.T
},
}
if err := t.ConvertFrom(ctx, obj); err != nil {
return nil, err
return nil, nil, err
}
return t, nil
return t, nil, nil
}
return nil, errors.New("resource is not a task")
return nil, nil, errors.New("resource is not a task")
}

// LocalTaskRefResolver uses the current cluster to resolve a task reference.
Expand All @@ -217,24 +223,24 @@ 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.ClusterTask, *v1beta1.ConfigSource, error) {
if l.Kind == v1beta1.ClusterTaskKind {
task, err := l.Tektonclient.TektonV1beta1().ClusterTasks().Get(ctx, name, metav1.GetOptions{})
if err != nil {
return nil, nil, err
return nil, nil, nil, err
}
return task, nil, nil
return nil, task, nil, nil
}

// If we are going to resolve this reference locally, we need a namespace scope.
if l.Namespace == "" {
return nil, nil, fmt.Errorf("must specify namespace to resolve reference to task %s", name)
return nil, nil, nil, fmt.Errorf("must specify namespace to resolve reference to task %s", name)
}
task, err := l.Tektonclient.TektonV1beta1().Tasks(l.Namespace).Get(ctx, name, metav1.GetOptions{})
if err != nil {
return nil, nil, err
return nil, nil, nil, err
}
return task, nil, nil
return task, nil, nil, nil
}

// IsGetTaskErrTransient returns true if an error returned by GetTask is retryable.
Expand Down
Loading

0 comments on commit b8c2462

Please sign in to comment.