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

fix: do not set default kind when taskRef resolver is present #7763

Merged
merged 1 commit into from
Apr 25, 2024
Merged

fix: do not set default kind when taskRef resolver is present #7763

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
6 changes: 3 additions & 3 deletions pkg/apis/pipeline/v1/pipeline_defaults.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
Comment on lines +56 to 59
Copy link
Member

Choose a reason for hiding this comment

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

This will continue to work even after TEP-0154 is done

if pt.TaskSpec != nil {
pt.TaskSpec.SetDefaults(ctx)
Expand Down
2 changes: 0 additions & 2 deletions pkg/apis/pipeline/v1/pipeline_defaults_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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",
},
Expand All @@ -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",
},
Expand Down
6 changes: 3 additions & 3 deletions pkg/apis/pipeline/v1/taskrun_defaults.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
2 changes: 0 additions & 2 deletions pkg/apis/pipeline/v1/taskrun_defaults_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -349,7 +349,6 @@ func TestTaskRunDefaulting(t *testing.T) {
},
Spec: v1.TaskRunSpec{
TaskRef: &v1.TaskRef{
Kind: "Task",
ResolverRef: v1.ResolverRef{
Resolver: "git",
},
Expand Down Expand Up @@ -378,7 +377,6 @@ func TestTaskRunDefaulting(t *testing.T) {
},
Spec: v1.TaskRunSpec{
TaskRef: &v1.TaskRef{
Kind: "Task",
ResolverRef: v1.ResolverRef{
Resolver: "custom resolver",
},
Expand Down
6 changes: 3 additions & 3 deletions pkg/apis/pipeline/v1beta1/pipeline_defaults.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 0 additions & 2 deletions pkg/apis/pipeline/v1beta1/pipeline_defaults_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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",
},
Expand All @@ -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",
},
Expand Down
1 change: 1 addition & 0 deletions pkg/apis/pipeline/v1beta1/taskref_conversion.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 = ""
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Remove the original "kind" attribute to stay consistent with the behavior elsewhere: once a resolver is used, "kind" is no longer necessary.

sink.ResolverRef = v1.ResolverRef{
Resolver: "bundles",
Params: v1.Params{{
Expand Down
6 changes: 3 additions & 3 deletions pkg/apis/pipeline/v1beta1/taskrun_defaults.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
2 changes: 0 additions & 2 deletions pkg/apis/pipeline/v1beta1/taskrun_defaults_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -359,7 +359,6 @@ func TestTaskRunDefaulting(t *testing.T) {
},
Spec: v1beta1.TaskRunSpec{
TaskRef: &v1beta1.TaskRef{
Kind: "Task",
ResolverRef: v1beta1.ResolverRef{
Resolver: "git",
},
Expand Down Expand Up @@ -388,7 +387,6 @@ func TestTaskRunDefaulting(t *testing.T) {
},
Spec: v1beta1.TaskRunSpec{
TaskRef: &v1beta1.TaskRef{
Kind: "Task",
ResolverRef: v1beta1.ResolverRef{
Resolver: "custom resolver",
},
Expand Down
1 change: 0 additions & 1 deletion pkg/reconciler/pipelinerun/pipelinerun_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8380,7 +8380,6 @@ metadata:
spec:
serviceAccountName: test-sa
taskRef:
kind: Task
resolver: bar
`)

Expand Down
1 change: 0 additions & 1 deletion pkg/reconciler/pipelinerun/pipelinespec/pipelinespec_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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",
},
Expand Down
10 changes: 2 additions & 8 deletions test/conversion_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -710,7 +710,6 @@ spec:
serviceAccountName: default
timeout: 1h
taskRef:
kind: Task
resolver: bundles
params:
- name: bundle
Expand Down Expand Up @@ -746,10 +745,9 @@ metadata:
namespace: %s
spec:
taskRunTemplate:
timeouts:
timeouts:
pipeline: 1h
pipelineRef:
kind: Pipeline
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The pipelineRef does not have a kind field.

pipeline/pkg/apis/pipeline/v1beta1/pipelineref_types.go

Lines 19 to 37 in d714545

// PipelineRef can be used to refer to a specific instance of a Pipeline.
type PipelineRef struct {
// Name of the referent; More info: http://kubernetes.io/docs/user-guide/identifiers#names
Name string `json:"name,omitempty"`
// API version of the referent
// +optional
APIVersion string `json:"apiVersion,omitempty"`
// Bundle url reference to a Tekton Bundle.
//
// Deprecated: Please use ResolverRef with the bundles resolver instead.
// +optional
Bundle string `json:"bundle,omitempty"`
// ResolverRef allows referencing a Pipeline in a remote location
// like a git repo. This field is only supported when the alpha
// feature gate is enabled.
// +optional
ResolverRef `json:",omitempty"`
}

pipeline/pkg/apis/pipeline/v1/pipelineref_types.go

Lines 19 to 32 in d714545

// PipelineRef can be used to refer to a specific instance of a Pipeline.
type PipelineRef struct {
// Name of the referent; More info: http://kubernetes.io/docs/user-guide/identifiers#names
Name string `json:"name,omitempty"`
// API version of the referent
// +optional
APIVersion string `json:"apiVersion,omitempty"`
// ResolverRef allows referencing a Pipeline in a remote location
// like a git repo. This field is only supported when the alpha
// feature gate is enabled.
// +optional
ResolverRef `json:",omitempty"`
}

resolver: bundles
params:
- name: bundle
Expand All @@ -767,7 +765,6 @@ status:
tasks:
- name: hello-world
taskRef:
kind: Task
resolver: bundles
params:
- name: bundle
Expand All @@ -788,7 +785,6 @@ metadata:
spec:
timeout: 1h
taskRef:
kind: Task
resolver: bundles
params:
- name: bundle
Expand Down Expand Up @@ -822,10 +818,9 @@ metadata:
name: %s
namespace: %s
spec:
timeouts:
timeouts:
pipeline: 1h
pipelineRef:
kind: Pipeline
resolver: bundles
params:
- name: bundle
Expand All @@ -843,7 +838,6 @@ status:
tasks:
- name: hello-world
taskRef:
kind: Task
resolver: bundles
params:
- name: bundle
Expand Down