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.

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

Remove the template.Name field #898

Merged
merged 1 commit into from
Jan 13, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
21 changes: 11 additions & 10 deletions pkg/apis/triggers/v1alpha1/event_listener_validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import (
"k8s.io/apimachinery/pkg/api/resource"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
duckv1 "knative.dev/pkg/apis/duck/v1"
"knative.dev/pkg/ptr"
)

func Test_EventListenerValidate(t *testing.T) {
Expand Down Expand Up @@ -142,7 +143,7 @@ func Test_EventListenerValidate(t *testing.T) {
},
Spec: v1alpha1.EventListenerSpec{
Triggers: []v1alpha1.EventListenerTrigger{{
Template: &v1alpha1.EventListenerTemplate{Name: "tt"},
Template: &v1alpha1.EventListenerTemplate{Ref: ptr.String("tt")},
Bindings: []*v1alpha1.EventListenerBinding{{
Name: "bname",
Spec: &v1alpha1.TriggerBindingSpec{
Expand Down Expand Up @@ -270,7 +271,7 @@ func TestEventListenerValidate_error(t *testing.T) {
},
Spec: v1alpha1.EventListenerSpec{
Triggers: []v1alpha1.EventListenerTrigger{{
Template: &v1alpha1.EventListenerTemplate{Name: "tt"},
Template: &v1alpha1.EventListenerTemplate{Ref: ptr.String("tt")},
Bindings: []*v1alpha1.EventListenerBinding{{
Ref: "tb",
Name: "",
Expand All @@ -294,7 +295,7 @@ func TestEventListenerValidate_error(t *testing.T) {
Spec: v1alpha1.EventListenerSpec{
Triggers: []v1alpha1.EventListenerTrigger{{
Bindings: []*v1alpha1.EventListenerBinding{{Ref: "", Kind: v1alpha1.NamespacedTriggerBindingKind}},
Template: &v1alpha1.EventListenerTemplate{Name: "tt"},
Template: &v1alpha1.EventListenerTemplate{Ref: ptr.String("tt")},
}},
},
},
Expand All @@ -308,7 +309,7 @@ func TestEventListenerValidate_error(t *testing.T) {
Spec: v1alpha1.EventListenerSpec{
Triggers: []v1alpha1.EventListenerTrigger{{
Bindings: []*v1alpha1.EventListenerBinding{{Ref: "tb", Kind: ""}},
Template: &v1alpha1.EventListenerTemplate{Name: "tt"},
Template: &v1alpha1.EventListenerTemplate{Ref: ptr.String("tt")},
}},
},
},
Expand All @@ -322,7 +323,7 @@ func TestEventListenerValidate_error(t *testing.T) {
Spec: v1alpha1.EventListenerSpec{
Triggers: []v1alpha1.EventListenerTrigger{{
Bindings: []*v1alpha1.EventListenerBinding{{Kind: v1alpha1.NamespacedTriggerBindingKind, Ref: "tb"}},
Template: &v1alpha1.EventListenerTemplate{Name: "tt", APIVersion: "invalid"},
Template: &v1alpha1.EventListenerTemplate{Ref: ptr.String("tt"), APIVersion: "invalid"},
}},
},
},
Expand All @@ -336,7 +337,7 @@ func TestEventListenerValidate_error(t *testing.T) {
Spec: v1alpha1.EventListenerSpec{
Triggers: []v1alpha1.EventListenerTrigger{{
Bindings: []*v1alpha1.EventListenerBinding{{Kind: v1alpha1.NamespacedTriggerBindingKind, Ref: "tb"}},
Template: &v1alpha1.EventListenerTemplate{Name: "", APIVersion: "v1alpha1"},
Template: &v1alpha1.EventListenerTemplate{Ref: ptr.String(""), APIVersion: "v1alpha1"},
}},
},
},
Expand All @@ -358,7 +359,7 @@ func TestEventListenerValidate_error(t *testing.T) {
Spec: v1alpha1.EventListenerSpec{
Triggers: []v1alpha1.EventListenerTrigger{{
Bindings: []*v1alpha1.EventListenerBinding{{Kind: v1alpha1.NamespacedTriggerBindingKind, Ref: "tb"}},
Template: &v1alpha1.EventListenerTemplate{Name: "tt"},
Template: &v1alpha1.EventListenerTemplate{Ref: ptr.String("tt")},
Interceptors: []*v1alpha1.EventInterceptor{{}},
}},
},
Expand All @@ -373,7 +374,7 @@ func TestEventListenerValidate_error(t *testing.T) {
Spec: v1alpha1.EventListenerSpec{
Triggers: []v1alpha1.EventListenerTrigger{{
Bindings: []*v1alpha1.EventListenerBinding{{Kind: v1alpha1.NamespacedTriggerBindingKind, Ref: "tb"}},
Template: &v1alpha1.EventListenerTemplate{Name: "tt"},
Template: &v1alpha1.EventListenerTemplate{Ref: ptr.String("tt")},
Interceptors: []*v1alpha1.EventInterceptor{{
Webhook: &v1alpha1.WebhookInterceptor{
ObjectRef: &corev1.ObjectReference{
Expand Down Expand Up @@ -447,7 +448,7 @@ func TestEventListenerValidate_error(t *testing.T) {
Spec: v1alpha1.EventListenerSpec{
Triggers: []v1alpha1.EventListenerTrigger{{
Bindings: []*v1alpha1.EventListenerBinding{{Kind: v1alpha1.NamespacedTriggerBindingKind, Ref: "tb"}},
Template: &v1alpha1.EventListenerTemplate{Name: "tt"},
Template: &v1alpha1.EventListenerTemplate{Ref: ptr.String("tt")},
Interceptors: []*v1alpha1.EventInterceptor{{
GitHub: &v1alpha1.GitHubInterceptor{},
GitLab: &v1alpha1.GitLabInterceptor{},
Expand All @@ -466,7 +467,7 @@ func TestEventListenerValidate_error(t *testing.T) {
Spec: v1alpha1.EventListenerSpec{
Triggers: []v1alpha1.EventListenerTrigger{{
Bindings: []*v1alpha1.EventListenerBinding{{Kind: v1alpha1.NamespacedTriggerBindingKind, Ref: "tb"}},
Template: &v1alpha1.EventListenerTemplate{Name: "tt"},
Template: &v1alpha1.EventListenerTemplate{Ref: ptr.String("tt")},
Interceptors: []*v1alpha1.EventInterceptor{{
CEL: &v1alpha1.CELInterceptor{},
}},
Expand Down
9 changes: 0 additions & 9 deletions pkg/apis/triggers/v1alpha1/trigger_defaults.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@ func (t *Trigger) SetDefaults(ctx context.Context) {
}
}
t.Spec.Bindings = bindings
templateNameToRef(&t.Spec.Template)
}

// set default TriggerBinding kind for Bindings in TriggerSpec
Expand All @@ -59,11 +58,3 @@ func (t triggerSpecBindingArray) defaultBindings() {
}
}
}

func templateNameToRef(template *TriggerSpecTemplate) {
name := template.Name
if name != "" && template.Ref == nil {
template.Ref = &name
template.Name = ""
}
}
19 changes: 0 additions & 19 deletions pkg/apis/triggers/v1alpha1/trigger_defaults_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -113,25 +113,6 @@ func TestTriggerSetDefaults(t *testing.T) {
}},
},
},
}, {
name: "sets template name to ref",
wc: v1alpha1.WithUpgradeViaDefaulting,
in: &v1alpha1.Trigger{
Spec: v1alpha1.TriggerSpec{
Template: v1alpha1.TriggerSpecTemplate{
Name: "tt-name",
},
},
},
want: &v1alpha1.Trigger{
Spec: v1alpha1.TriggerSpec{
Bindings: []*v1alpha1.TriggerSpecBinding{},
Template: v1alpha1.TriggerSpecTemplate{
Ref: ptr.String("tt-name"),
Name: "",
},
},
},
}}
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
Expand Down
2 changes: 0 additions & 2 deletions pkg/apis/triggers/v1alpha1/trigger_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,6 @@ type TriggerSpec struct {
}

type TriggerSpecTemplate struct {
// Deprecated: Use Ref instead
Name string `json:"name"`
Ref *string `json:"ref,omitempty"`
APIVersion string `json:"apiversion,omitempty"`
Spec *TriggerTemplateSpec `json:"spec,omitempty"`
Expand Down
9 changes: 5 additions & 4 deletions pkg/apis/triggers/v1alpha1/trigger_types_convert_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"testing"

"github.com/google/go-cmp/cmp"
"knative.dev/pkg/ptr"
)

func TestToEventListenerTrigger(t *testing.T) {
Expand All @@ -38,13 +39,13 @@ func TestToEventListenerTrigger(t *testing.T) {
in: TriggerSpec{
Name: "foo",
Template: TriggerSpecTemplate{
Name: "baz",
Ref: ptr.String("baz"),
},
},
out: EventListenerTrigger{
Name: "foo",
Template: &EventListenerTemplate{
Name: "baz",
Ref: ptr.String("baz"),
},
},
},
Expand All @@ -69,7 +70,7 @@ func TestToEventListenerTrigger(t *testing.T) {
},
}},
Template: TriggerSpecTemplate{
Name: "a",
Ref: ptr.String("a"),
APIVersion: "b",
},
},
Expand All @@ -92,7 +93,7 @@ func TestToEventListenerTrigger(t *testing.T) {
},
}},
Template: &EventListenerTemplate{
Name: "a",
Ref: ptr.String("a"),
APIVersion: "b",
},
},
Expand Down
10 changes: 0 additions & 10 deletions pkg/apis/triggers/v1alpha1/trigger_validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,16 +55,6 @@ func (t TriggerSpecTemplate) validate(ctx context.Context) (errs *apis.FieldErro
}
}

// Validate only one of Name or Ref is set.
if t.Name != "" && t.Ref != nil {
errs = errs.Also(apis.ErrMultipleOneOf("template.name", "template.ref"))
}

// Set Ref to Name
if t.Name != "" {
t.Ref = &t.Name
}

switch {
case t.Spec != nil && t.Ref != nil:
errs = errs.Also(apis.ErrMultipleOneOf("template.spec", "template.ref"))
Expand Down
50 changes: 13 additions & 37 deletions pkg/apis/triggers/v1alpha1/trigger_validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ func Test_TriggerValidate(t *testing.T) {
Ref: "ref-to-another-binding",
Kind: v1alpha1.NamespacedTriggerBindingKind,
}},
Template: v1alpha1.TriggerSpecTemplate{Name: "my-tt"},
Template: v1alpha1.TriggerSpecTemplate{Ref: ptr.String("baz")},
},
},
}, {
Expand Down Expand Up @@ -144,7 +144,7 @@ func Test_TriggerValidate(t *testing.T) {
},
Spec: v1alpha1.TriggerSpec{
Bindings: []*v1alpha1.TriggerSpecBinding{{Spec: &v1alpha1.TriggerBindingSpec{}, Name: "foo"}},
Template: v1alpha1.TriggerSpecTemplate{Name: "tt"},
Template: v1alpha1.TriggerSpecTemplate{Ref: ptr.String("tt")},
},
},
}, {
Expand Down Expand Up @@ -187,7 +187,7 @@ func Test_TriggerValidate(t *testing.T) {
},
Spec: v1alpha1.TriggerSpec{
Template: v1alpha1.TriggerSpecTemplate{
Name: "ref-to-a-template",
Ref: ptr.String("ref-to-a-template"),
},
},
},
Expand Down Expand Up @@ -219,7 +219,7 @@ func TestTriggerValidate_error(t *testing.T) {
},
Spec: v1alpha1.TriggerSpec{
Bindings: []*v1alpha1.TriggerSpecBinding{{Name: "", Kind: v1alpha1.NamespacedTriggerBindingKind, Ref: "", APIVersion: "v1alpha1"}},
Template: v1alpha1.TriggerSpecTemplate{Name: "tt"},
Template: v1alpha1.TriggerSpecTemplate{Ref: ptr.String("tt")},
},
},
}, {
Expand All @@ -231,7 +231,7 @@ func TestTriggerValidate_error(t *testing.T) {
},
Spec: v1alpha1.TriggerSpec{
Bindings: []*v1alpha1.TriggerSpecBinding{{Name: "tb", Kind: "", Ref: "tb", APIVersion: "v1alpha1"}},
Template: v1alpha1.TriggerSpecTemplate{Name: "tt"},
Template: v1alpha1.TriggerSpecTemplate{Ref: ptr.String("tt")},
},
},
}, {
Expand All @@ -243,7 +243,7 @@ func TestTriggerValidate_error(t *testing.T) {
},
Spec: v1alpha1.TriggerSpec{
Bindings: []*v1alpha1.TriggerSpecBinding{{Kind: "BadKind", Ref: "tb", APIVersion: "v1alpha1"}},
Template: v1alpha1.TriggerSpecTemplate{Name: "tt"},
Template: v1alpha1.TriggerSpecTemplate{Ref: ptr.String("tt")},
},
},
}, {
Expand All @@ -255,7 +255,7 @@ func TestTriggerValidate_error(t *testing.T) {
},
Spec: v1alpha1.TriggerSpec{
Bindings: []*v1alpha1.TriggerSpecBinding{{Name: "foo"}},
Template: v1alpha1.TriggerSpecTemplate{Name: "tt"},
Template: v1alpha1.TriggerSpecTemplate{Ref: ptr.String("tt")},
},
},
}, {
Expand All @@ -267,31 +267,7 @@ func TestTriggerValidate_error(t *testing.T) {
},
Spec: v1alpha1.TriggerSpec{
Bindings: []*v1alpha1.TriggerSpecBinding{{Name: "tb", Kind: v1alpha1.NamespacedTriggerBindingKind, Ref: "tb", APIVersion: "v1alpha1"}},
Template: v1alpha1.TriggerSpecTemplate{Name: "tt", APIVersion: "invalid"},
},
},
}, {
name: "Template with missing Name", // TODO(#791): Remove when Name is removed
tr: &v1alpha1.Trigger{
ObjectMeta: metav1.ObjectMeta{
Name: "name",
Namespace: "namespace",
},
Spec: v1alpha1.TriggerSpec{
Bindings: []*v1alpha1.TriggerSpecBinding{{Name: "tb", Kind: v1alpha1.NamespacedTriggerBindingKind, Ref: "tb", APIVersion: "v1alpha1"}},
Template: v1alpha1.TriggerSpecTemplate{Name: "", APIVersion: "v1alpha1"},
},
},
}, {
name: "Template with both Name and Ref", // TODO(#791): Remove when Name is removed
tr: &v1alpha1.Trigger{
ObjectMeta: metav1.ObjectMeta{
Name: "name",
Namespace: "namespace",
},
Spec: v1alpha1.TriggerSpec{
Bindings: []*v1alpha1.TriggerSpecBinding{{Name: "tb", Kind: v1alpha1.NamespacedTriggerBindingKind, Ref: "tb", APIVersion: "v1alpha1"}},
Template: v1alpha1.TriggerSpecTemplate{Name: "tt", Ref: ptr.String("tt"), APIVersion: "v1alpha1"},
Template: v1alpha1.TriggerSpecTemplate{Ref: ptr.String("tt"), APIVersion: "invalid"},
},
},
}, {
Expand Down Expand Up @@ -335,7 +311,7 @@ func TestTriggerValidate_error(t *testing.T) {
},
Spec: v1alpha1.TriggerSpec{
Bindings: []*v1alpha1.TriggerSpecBinding{{Name: "tb", Kind: v1alpha1.NamespacedTriggerBindingKind, Ref: "tb", APIVersion: "v1alpha1"}},
Template: v1alpha1.TriggerSpecTemplate{Name: "tt", APIVersion: "v1alpha1"},
Template: v1alpha1.TriggerSpecTemplate{Ref: ptr.String("tt"), APIVersion: "v1alpha1"},
Interceptors: []*v1alpha1.TriggerInterceptor{{}},
},
},
Expand All @@ -348,7 +324,7 @@ func TestTriggerValidate_error(t *testing.T) {
},
Spec: v1alpha1.TriggerSpec{
Bindings: []*v1alpha1.TriggerSpecBinding{{Name: "tb", Kind: v1alpha1.NamespacedTriggerBindingKind, Ref: "tb", APIVersion: "v1alpha1"}},
Template: v1alpha1.TriggerSpecTemplate{Name: "tt", APIVersion: "v1alpha1"},
Template: v1alpha1.TriggerSpecTemplate{Ref: ptr.String("tt"), APIVersion: "v1alpha1"},
Interceptors: []*v1alpha1.TriggerInterceptor{{
Webhook: &v1alpha1.WebhookInterceptor{
ObjectRef: &corev1.ObjectReference{
Expand Down Expand Up @@ -420,7 +396,7 @@ func TestTriggerValidate_error(t *testing.T) {
},
Spec: v1alpha1.TriggerSpec{
Bindings: []*v1alpha1.TriggerSpecBinding{{Name: "tb", Kind: v1alpha1.NamespacedTriggerBindingKind, Ref: "tb"}},
Template: v1alpha1.TriggerSpecTemplate{Name: "tt"},
Template: v1alpha1.TriggerSpecTemplate{Ref: ptr.String("tt")},
Interceptors: []*v1alpha1.TriggerInterceptor{{
GitHub: &v1alpha1.GitHubInterceptor{},
GitLab: &v1alpha1.GitLabInterceptor{},
Expand All @@ -437,7 +413,7 @@ func TestTriggerValidate_error(t *testing.T) {
},
Spec: v1alpha1.TriggerSpec{
Bindings: []*v1alpha1.TriggerSpecBinding{{Name: "tb", Kind: v1alpha1.NamespacedTriggerBindingKind, Ref: "tb"}},
Template: v1alpha1.TriggerSpecTemplate{Name: "tt"},
Template: v1alpha1.TriggerSpecTemplate{Ref: ptr.String("tt")},
Interceptors: []*v1alpha1.TriggerInterceptor{{
CEL: &v1alpha1.CELInterceptor{},
}},
Expand Down Expand Up @@ -480,7 +456,7 @@ func TestTriggerValidate_error(t *testing.T) {
ObjectMeta: metav1.ObjectMeta{Name: "name"},
Spec: v1alpha1.TriggerSpec{
Template: v1alpha1.TriggerSpecTemplate{
Name: "ttname",
Ref: ptr.String("tt-name"),
Spec: &v1alpha1.TriggerTemplateSpec{
ResourceTemplates: []v1alpha1.TriggerResourceTemplate{{
RawExtension: test.RawExtension(t, pipelinev1.PipelineRun{
Expand Down
8 changes: 4 additions & 4 deletions pkg/sink/sink_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -679,7 +679,7 @@ func TestHandleEventPassesURLThrough(t *testing.T) {
Spec: triggersv1.EventListenerSpec{
Triggers: []triggersv1.EventListenerTrigger{{
Bindings: []*triggersv1.EventListenerBinding{{Ref: "tb", Kind: "TriggerBinding"}},
Template: &triggersv1.EventListenerTemplate{Name: "tt"},
Template: &triggersv1.EventListenerTemplate{Ref: ptr.String("tt")},
Interceptors: []*triggersv1.EventInterceptor{{
CEL: &triggersv1.CELInterceptor{
Overlays: []triggersv1.CELOverlay{
Expand Down Expand Up @@ -793,7 +793,7 @@ func TestHandleEventWithWebhookInterceptors(t *testing.T) {
for i := 0; i < numTriggers; i++ {
trigger := triggersv1.EventListenerTrigger{
Bindings: []*triggersv1.EventListenerBinding{{Ref: "tb", Kind: "TriggerBinding"}},
Template: &triggersv1.EventListenerTemplate{Name: "tt"},
Template: &triggersv1.EventListenerTemplate{Ref: ptr.String("tt")},
Interceptors: []*triggersv1.EventInterceptor{{
Webhook: &triggersv1.WebhookInterceptor{
ObjectRef: interceptorObjectRef,
Expand Down Expand Up @@ -1300,7 +1300,7 @@ func TestHandleEventWithInterceptorsAndTriggerAuth(t *testing.T) {
Triggers: []triggersv1.EventListenerTrigger{{
ServiceAccountName: testCase.userVal,
Bindings: []*triggersv1.EventListenerBinding{{Ref: "tb", Kind: "TriggerBinding"}},
Template: &triggersv1.EventListenerTemplate{Name: "tt"},
Template: &triggersv1.EventListenerTemplate{Ref: ptr.String("tt")},
Interceptors: []*triggersv1.EventInterceptor{{
GitHub: &triggersv1.GitHubInterceptor{
SecretRef: &triggersv1.SecretRef{
Expand Down Expand Up @@ -1387,7 +1387,7 @@ func TestHandleEventWithBitbucketInterceptors(t *testing.T) {
Triggers: []triggersv1.EventListenerTrigger{{
ServiceAccountName: userWithPermissions,
Bindings: []*triggersv1.EventListenerBinding{{Ref: "tb", Kind: "TriggerBinding"}},
Template: &triggersv1.EventListenerTemplate{Name: "tt"},
Template: &triggersv1.EventListenerTemplate{Ref: ptr.String("tt")},
Interceptors: []*triggersv1.EventInterceptor{{
Bitbucket: &triggersv1.BitbucketInterceptor{
SecretRef: &triggersv1.SecretRef{
Expand Down
4 changes: 0 additions & 4 deletions pkg/template/resource.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,10 +62,6 @@ func ResolveTrigger(trigger triggersv1.Trigger, getTB getTriggerBinding, getCTB
var ttName string
if trigger.Spec.Template.Ref != nil {
ttName = *trigger.Spec.Template.Ref
} else {
// TODO(#791): Remove Name field
// Ignore staticcheck linter as it will complain about using deprecated type
ttName = trigger.Spec.Template.Name //nolint:staticcheck
}
resolvedTT, err = getTT(ttName)
if err != nil {
Expand Down
Loading