From adfb7f55c37b1d5244ac6e6f55df3cdc5b2257d3 Mon Sep 17 00:00:00 2001 From: Dibyo Mukherjee Date: Fri, 23 Apr 2021 17:19:38 -0400 Subject: [PATCH] refactor: move list of allowed ResourceTemplate types to its own package This will help reduce the number of places we keep track of the list of types that Triggers can create especially as we add the v1beta1 types. Partially addresses #494 and part of the work needed for #1067 --- pkg/apis/triggers/config/allowed_types.go | 30 +++++++++++++++ .../event_listener_validation_test.go | 38 +++++++++++++++++++ .../v1alpha1/trigger_template_types.go | 7 ---- .../v1alpha1/trigger_template_validation.go | 3 +- 4 files changed, 70 insertions(+), 8 deletions(-) create mode 100644 pkg/apis/triggers/config/allowed_types.go diff --git a/pkg/apis/triggers/config/allowed_types.go b/pkg/apis/triggers/config/allowed_types.go new file mode 100644 index 000000000..9e26e2584 --- /dev/null +++ b/pkg/apis/triggers/config/allowed_types.go @@ -0,0 +1,30 @@ +package config + +import ( + pipelinev1alpha1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1" + pipelinev1beta1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/serializer" + utilruntime "k8s.io/apimachinery/pkg/util/runtime" +) + +var Decoder runtime.Decoder + +// TODO(dibyom): We should have a way of configuring this instead of an init function? +func init() { + scheme := runtime.NewScheme() + utilruntime.Must(pipelinev1alpha1.AddToScheme(scheme)) + utilruntime.Must(pipelinev1beta1.AddToScheme(scheme)) + codec := serializer.NewCodecFactory(scheme) + Decoder = codec.UniversalDecoder( + pipelinev1alpha1.SchemeGroupVersion, + pipelinev1beta1.SchemeGroupVersion, + ) +} + +// EnsureAllowedType returns nil if the resourceTemplate has an apiVersion +// and kind field set to one of the allowed ones. +func EnsureAllowedType(rt runtime.RawExtension) error { + _, err := runtime.Decode(Decoder, rt.Raw) + return err +} diff --git a/pkg/apis/triggers/v1alpha1/event_listener_validation_test.go b/pkg/apis/triggers/v1alpha1/event_listener_validation_test.go index 6b31b595c..bd3b4a7cc 100644 --- a/pkg/apis/triggers/v1alpha1/event_listener_validation_test.go +++ b/pkg/apis/triggers/v1alpha1/event_listener_validation_test.go @@ -20,6 +20,7 @@ import ( "context" "testing" + pipelinev1alpha1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1" "github.com/tektoncd/triggers/pkg/apis/triggers/v1alpha1" "github.com/tektoncd/triggers/test" bldr "github.com/tektoncd/triggers/test/builder" @@ -274,6 +275,43 @@ func Test_EventListenerValidate(t *testing.T) { }, }, }, + }, { + name: "valid EventListener with embedded TriggerTemplate", + el: &v1alpha1.EventListener{ + ObjectMeta: metav1.ObjectMeta{ + Name: "name", + Namespace: "namespace", + }, + Spec: v1alpha1.EventListenerSpec{ + Triggers: []v1alpha1.EventListenerTrigger{{ + Bindings: []*v1alpha1.EventListenerBinding{{ + Ref: "tb", + Kind: v1alpha1.NamespacedTriggerBindingKind, + APIVersion: "v1alpha1", // TODO: APIVersions seem wrong? + }}, + Template: &v1alpha1.EventListenerTemplate{ + Spec: &v1alpha1.TriggerTemplateSpec{ + Params: []v1alpha1.ParamSpec{{ + Name: "foo", + Description: "desc", + Default: ptr.String("val"), + }}, + ResourceTemplates: []v1alpha1.TriggerResourceTemplate{{ + RawExtension: test.RawExtension(t, pipelinev1alpha1.PipelineRun{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "tekton.dev/v1alpha1", + Kind: "TaskRun", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "$(tt.params.foo)", + }, + }), + }}, + }, + }, + }}, + }, + }, }} for _, test := range tests { diff --git a/pkg/apis/triggers/v1alpha1/trigger_template_types.go b/pkg/apis/triggers/v1alpha1/trigger_template_types.go index 7464c654c..d3401fa91 100644 --- a/pkg/apis/triggers/v1alpha1/trigger_template_types.go +++ b/pkg/apis/triggers/v1alpha1/trigger_template_types.go @@ -82,10 +82,3 @@ type TriggerTemplateList struct { metav1.ListMeta `json:"metadata,omitempty"` Items []TriggerTemplate `json:"items"` } - -// IsAllowedType returns true if the resourceTemplate has an apiVersion -// and kind field set to one of the allowed ones. -func (trt *TriggerResourceTemplate) IsAllowedType() error { - _, err := runtime.Decode(Decoder, trt.RawExtension.Raw) - return err -} diff --git a/pkg/apis/triggers/v1alpha1/trigger_template_validation.go b/pkg/apis/triggers/v1alpha1/trigger_template_validation.go index 7376e1cf4..cf4c65a60 100644 --- a/pkg/apis/triggers/v1alpha1/trigger_template_validation.go +++ b/pkg/apis/triggers/v1alpha1/trigger_template_validation.go @@ -23,6 +23,7 @@ import ( "strings" "github.com/tektoncd/pipeline/pkg/apis/validate" + "github.com/tektoncd/triggers/pkg/apis/triggers/config" "k8s.io/apimachinery/pkg/api/equality" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/util/sets" @@ -53,7 +54,7 @@ func (s *TriggerTemplateSpec) validate(ctx context.Context) (errs *apis.FieldErr func validateResourceTemplates(templates []TriggerResourceTemplate) (errs *apis.FieldError) { for i, trt := range templates { - if err := trt.IsAllowedType(); err != nil { + if err := config.EnsureAllowedType(trt.RawExtension); err != nil { if runtime.IsMissingVersion(err) { errs = errs.Also(apis.ErrMissingField(fmt.Sprintf("[%d].apiVersion", i))) }