From da73725fa9cea35437e5305388eb0b3f06fa29d4 Mon Sep 17 00:00:00 2001 From: qingliu Date: Sun, 17 Mar 2024 19:15:01 +0800 Subject: [PATCH] fix: do not set default kind when taskRef resolver is present fix #7762 Do not set default kind when taskRef resolver is present, keep the original configuration of the user. --- pkg/apis/pipeline/v1/pipeline_defaults.go | 6 +++--- pkg/apis/pipeline/v1/pipeline_defaults_test.go | 2 -- pkg/apis/pipeline/v1/taskrun_defaults.go | 6 +++--- pkg/apis/pipeline/v1/taskrun_defaults_test.go | 2 -- pkg/apis/pipeline/v1beta1/pipeline_defaults.go | 6 +++--- pkg/apis/pipeline/v1beta1/pipeline_defaults_test.go | 2 -- pkg/apis/pipeline/v1beta1/taskref_conversion.go | 1 + pkg/apis/pipeline/v1beta1/taskrun_defaults.go | 6 +++--- pkg/apis/pipeline/v1beta1/taskrun_defaults_test.go | 2 -- pkg/reconciler/pipelinerun/pipelinerun_test.go | 1 - .../pipelinerun/pipelinespec/pipelinespec_test.go | 1 - test/conversion_test.go | 10 ++-------- 12 files changed, 15 insertions(+), 30 deletions(-) diff --git a/pkg/apis/pipeline/v1/pipeline_defaults.go b/pkg/apis/pipeline/v1/pipeline_defaults.go index a6c7190e8e1..47fb605f05f 100644 --- a/pkg/apis/pipeline/v1/pipeline_defaults.go +++ b/pkg/apis/pipeline/v1/pipeline_defaults.go @@ -50,12 +50,12 @@ func (ps *PipelineSpec) SetDefaults(ctx context.Context) { func (pt *PipelineTask) SetDefaults(ctx context.Context) { cfg := config.FromContextOrDefaults(ctx) if pt.TaskRef != nil { - if pt.TaskRef.Kind == "" { - pt.TaskRef.Kind = NamespacedTaskKind - } if pt.TaskRef.Name == "" && pt.TaskRef.Resolver == "" { pt.TaskRef.Resolver = ResolverName(cfg.Defaults.DefaultResolverType) } + if pt.TaskRef.Kind == "" && pt.TaskRef.Resolver == "" { + pt.TaskRef.Kind = NamespacedTaskKind + } } if pt.TaskSpec != nil { pt.TaskSpec.SetDefaults(ctx) diff --git a/pkg/apis/pipeline/v1/pipeline_defaults_test.go b/pkg/apis/pipeline/v1/pipeline_defaults_test.go index bbe7485e414..94ab55ab1a0 100644 --- a/pkg/apis/pipeline/v1/pipeline_defaults_test.go +++ b/pkg/apis/pipeline/v1/pipeline_defaults_test.go @@ -207,7 +207,6 @@ func TestPipelineTask_SetDefaults(t *testing.T) { want: &v1.PipelineTask{ Name: "foo", TaskRef: &v1.TaskRef{ - Kind: v1.NamespacedTaskKind, ResolverRef: v1.ResolverRef{ Resolver: "git", }, @@ -229,7 +228,6 @@ func TestPipelineTask_SetDefaults(t *testing.T) { want: &v1.PipelineTask{ Name: "foo", TaskRef: &v1.TaskRef{ - Kind: v1.NamespacedTaskKind, ResolverRef: v1.ResolverRef{ Resolver: "custom resolver", }, diff --git a/pkg/apis/pipeline/v1/taskrun_defaults.go b/pkg/apis/pipeline/v1/taskrun_defaults.go index 79fa50f3cdc..bb21abf8fb5 100644 --- a/pkg/apis/pipeline/v1/taskrun_defaults.go +++ b/pkg/apis/pipeline/v1/taskrun_defaults.go @@ -59,12 +59,12 @@ func (tr *TaskRun) SetDefaults(ctx context.Context) { func (trs *TaskRunSpec) SetDefaults(ctx context.Context) { cfg := config.FromContextOrDefaults(ctx) if trs.TaskRef != nil { - if trs.TaskRef.Kind == "" { - trs.TaskRef.Kind = NamespacedTaskKind - } if trs.TaskRef.Name == "" && trs.TaskRef.Resolver == "" { trs.TaskRef.Resolver = ResolverName(cfg.Defaults.DefaultResolverType) } + if trs.TaskRef.Kind == "" && trs.TaskRef.Resolver == "" { + trs.TaskRef.Kind = NamespacedTaskKind + } } if trs.Timeout == nil { diff --git a/pkg/apis/pipeline/v1/taskrun_defaults_test.go b/pkg/apis/pipeline/v1/taskrun_defaults_test.go index 35b26e69029..420e9c20fba 100644 --- a/pkg/apis/pipeline/v1/taskrun_defaults_test.go +++ b/pkg/apis/pipeline/v1/taskrun_defaults_test.go @@ -349,7 +349,6 @@ func TestTaskRunDefaulting(t *testing.T) { }, Spec: v1.TaskRunSpec{ TaskRef: &v1.TaskRef{ - Kind: "Task", ResolverRef: v1.ResolverRef{ Resolver: "git", }, @@ -378,7 +377,6 @@ func TestTaskRunDefaulting(t *testing.T) { }, Spec: v1.TaskRunSpec{ TaskRef: &v1.TaskRef{ - Kind: "Task", ResolverRef: v1.ResolverRef{ Resolver: "custom resolver", }, diff --git a/pkg/apis/pipeline/v1beta1/pipeline_defaults.go b/pkg/apis/pipeline/v1beta1/pipeline_defaults.go index ec28f038e94..8d43263454e 100644 --- a/pkg/apis/pipeline/v1beta1/pipeline_defaults.go +++ b/pkg/apis/pipeline/v1beta1/pipeline_defaults.go @@ -50,12 +50,12 @@ func (ps *PipelineSpec) SetDefaults(ctx context.Context) { func (pt *PipelineTask) SetDefaults(ctx context.Context) { cfg := config.FromContextOrDefaults(ctx) if pt.TaskRef != nil { - if pt.TaskRef.Kind == "" { - pt.TaskRef.Kind = NamespacedTaskKind - } if pt.TaskRef.Name == "" && pt.TaskRef.Resolver == "" { pt.TaskRef.Resolver = ResolverName(cfg.Defaults.DefaultResolverType) } + if pt.TaskRef.Kind == "" && pt.TaskRef.Resolver == "" { + pt.TaskRef.Kind = NamespacedTaskKind + } } if pt.TaskSpec != nil { pt.TaskSpec.SetDefaults(ctx) diff --git a/pkg/apis/pipeline/v1beta1/pipeline_defaults_test.go b/pkg/apis/pipeline/v1beta1/pipeline_defaults_test.go index f78c82f034d..8d6c75f9858 100644 --- a/pkg/apis/pipeline/v1beta1/pipeline_defaults_test.go +++ b/pkg/apis/pipeline/v1beta1/pipeline_defaults_test.go @@ -207,7 +207,6 @@ func TestPipelineTask_SetDefaults(t *testing.T) { want: &v1beta1.PipelineTask{ Name: "foo", TaskRef: &v1beta1.TaskRef{ - Kind: v1beta1.NamespacedTaskKind, ResolverRef: v1beta1.ResolverRef{ Resolver: "git", }, @@ -229,7 +228,6 @@ func TestPipelineTask_SetDefaults(t *testing.T) { want: &v1beta1.PipelineTask{ Name: "foo", TaskRef: &v1beta1.TaskRef{ - Kind: v1beta1.NamespacedTaskKind, ResolverRef: v1beta1.ResolverRef{ Resolver: "custom resolver", }, diff --git a/pkg/apis/pipeline/v1beta1/taskref_conversion.go b/pkg/apis/pipeline/v1beta1/taskref_conversion.go index ab5f2a5614e..c089e9e0dbf 100644 --- a/pkg/apis/pipeline/v1beta1/taskref_conversion.go +++ b/pkg/apis/pipeline/v1beta1/taskref_conversion.go @@ -49,6 +49,7 @@ func (tr *TaskRef) ConvertFrom(ctx context.Context, source v1.TaskRef) { // default and it will be in beta before the stored version of CRD getting swapped to v1. func (tr TaskRef) convertBundleToResolver(sink *v1.TaskRef) { if tr.Bundle != "" { + sink.Kind = "" sink.ResolverRef = v1.ResolverRef{ Resolver: "bundles", Params: v1.Params{{ diff --git a/pkg/apis/pipeline/v1beta1/taskrun_defaults.go b/pkg/apis/pipeline/v1beta1/taskrun_defaults.go index 2e4896d47b9..6fdc800153d 100644 --- a/pkg/apis/pipeline/v1beta1/taskrun_defaults.go +++ b/pkg/apis/pipeline/v1beta1/taskrun_defaults.go @@ -59,12 +59,12 @@ func (tr *TaskRun) SetDefaults(ctx context.Context) { func (trs *TaskRunSpec) SetDefaults(ctx context.Context) { cfg := config.FromContextOrDefaults(ctx) if trs.TaskRef != nil { - if trs.TaskRef.Kind == "" { - trs.TaskRef.Kind = NamespacedTaskKind - } if trs.TaskRef.Name == "" && trs.TaskRef.Resolver == "" { trs.TaskRef.Resolver = ResolverName(cfg.Defaults.DefaultResolverType) } + if trs.TaskRef.Kind == "" && trs.TaskRef.Resolver == "" { + trs.TaskRef.Kind = NamespacedTaskKind + } } if trs.Timeout == nil { diff --git a/pkg/apis/pipeline/v1beta1/taskrun_defaults_test.go b/pkg/apis/pipeline/v1beta1/taskrun_defaults_test.go index f3079e71cb5..f0801824d85 100644 --- a/pkg/apis/pipeline/v1beta1/taskrun_defaults_test.go +++ b/pkg/apis/pipeline/v1beta1/taskrun_defaults_test.go @@ -359,7 +359,6 @@ func TestTaskRunDefaulting(t *testing.T) { }, Spec: v1beta1.TaskRunSpec{ TaskRef: &v1beta1.TaskRef{ - Kind: "Task", ResolverRef: v1beta1.ResolverRef{ Resolver: "git", }, @@ -388,7 +387,6 @@ func TestTaskRunDefaulting(t *testing.T) { }, Spec: v1beta1.TaskRunSpec{ TaskRef: &v1beta1.TaskRef{ - Kind: "Task", ResolverRef: v1beta1.ResolverRef{ Resolver: "custom resolver", }, diff --git a/pkg/reconciler/pipelinerun/pipelinerun_test.go b/pkg/reconciler/pipelinerun/pipelinerun_test.go index de293772183..849ea04af44 100644 --- a/pkg/reconciler/pipelinerun/pipelinerun_test.go +++ b/pkg/reconciler/pipelinerun/pipelinerun_test.go @@ -8380,7 +8380,6 @@ metadata: spec: serviceAccountName: test-sa taskRef: - kind: Task resolver: bar `) diff --git a/pkg/reconciler/pipelinerun/pipelinespec/pipelinespec_test.go b/pkg/reconciler/pipelinerun/pipelinespec/pipelinespec_test.go index 7ffd5fb283f..f1dd8a23514 100644 --- a/pkg/reconciler/pipelinerun/pipelinespec/pipelinespec_test.go +++ b/pkg/reconciler/pipelinerun/pipelinespec/pipelinespec_test.go @@ -203,7 +203,6 @@ func TestGetPipelineData_ResolutionSuccess(t *testing.T) { Tasks: []v1.PipelineTask{{ Name: "pt1", TaskRef: &v1.TaskRef{ - Kind: "Task", ResolverRef: v1.ResolverRef{ Resolver: "foo", }, diff --git a/test/conversion_test.go b/test/conversion_test.go index aead7504501..489fad5aced 100644 --- a/test/conversion_test.go +++ b/test/conversion_test.go @@ -710,7 +710,6 @@ spec: serviceAccountName: default timeout: 1h taskRef: - kind: Task resolver: bundles params: - name: bundle @@ -746,10 +745,9 @@ metadata: namespace: %s spec: taskRunTemplate: - timeouts: + timeouts: pipeline: 1h pipelineRef: - kind: Pipeline resolver: bundles params: - name: bundle @@ -767,7 +765,6 @@ status: tasks: - name: hello-world taskRef: - kind: Task resolver: bundles params: - name: bundle @@ -788,7 +785,6 @@ metadata: spec: timeout: 1h taskRef: - kind: Task resolver: bundles params: - name: bundle @@ -822,10 +818,9 @@ metadata: name: %s namespace: %s spec: - timeouts: + timeouts: pipeline: 1h pipelineRef: - kind: Pipeline resolver: bundles params: - name: bundle @@ -843,7 +838,6 @@ status: tasks: - name: hello-world taskRef: - kind: Task resolver: bundles params: - name: bundle