From 51986b3086eb022dd4c99f16b0b4265ef3d54d92 Mon Sep 17 00:00:00 2001 From: Dibyo Mukherjee Date: Thu, 1 Apr 2021 16:46:02 -0400 Subject: [PATCH] Migrate core interceptors to new format This commit updates the defaulting webhook to automatically set any Triggers using the old style syntax for core interceptors to the new ref/params based syntax. Fixes #1009 Signed-off-by: Dibyo Mukherjee --- .../v1alpha1/event_listener_defaults.go | 7 + .../triggers/v1alpha1/trigger_defaults.go | 8 + .../v1alpha1/trigger_defaults_test.go | 145 ++++++++++++++++++ pkg/apis/triggers/v1alpha1/trigger_types.go | 74 +++++++++ .../triggers/v1alpha1/trigger_types_test.go | 38 +++-- 5 files changed, 256 insertions(+), 16 deletions(-) diff --git a/pkg/apis/triggers/v1alpha1/event_listener_defaults.go b/pkg/apis/triggers/v1alpha1/event_listener_defaults.go index df2728c7e..c7b30d0eb 100644 --- a/pkg/apis/triggers/v1alpha1/event_listener_defaults.go +++ b/pkg/apis/triggers/v1alpha1/event_listener_defaults.go @@ -19,6 +19,7 @@ package v1alpha1 import ( "context" + "knative.dev/pkg/logging" "knative.dev/pkg/ptr" ) @@ -36,6 +37,12 @@ func (el *EventListener) SetDefaults(ctx context.Context) { triggerSpecBindingArray(el.Spec.Triggers[i].Bindings).defaultBindings() for _, ti := range t.Interceptors { ti.defaultInterceptorKind() + if err := ti.updateCoreInterceptors(); err != nil { + // The err only happens due to malformed JSON and should never really happen + // We can't return an error here, so print out the error + logger := logging.FromContext(ctx) + logger.Errorf("failed to setDefaults for trigger: %s; err: %s", t.Name, err) + } } } // Remove Deprecated Resource Fields diff --git a/pkg/apis/triggers/v1alpha1/trigger_defaults.go b/pkg/apis/triggers/v1alpha1/trigger_defaults.go index 4e3ee7580..8abb60ca9 100644 --- a/pkg/apis/triggers/v1alpha1/trigger_defaults.go +++ b/pkg/apis/triggers/v1alpha1/trigger_defaults.go @@ -18,6 +18,8 @@ package v1alpha1 import ( "context" + + "knative.dev/pkg/logging" ) type triggerSpecBindingArray []*TriggerSpecBinding @@ -30,6 +32,12 @@ func (t *Trigger) SetDefaults(ctx context.Context) { triggerSpecBindingArray(t.Spec.Bindings).defaultBindings() for _, ti := range t.Spec.Interceptors { ti.defaultInterceptorKind() + if err := ti.updateCoreInterceptors(); err != nil { + // The err only happens due to malformed JSON and should never really happen + // We can't return an error here, so print out the error + logger := logging.FromContext(ctx) + logger.Errorf("failed to setDefaults for trigger: %s; err: %s", t.Name, err) + } } } diff --git a/pkg/apis/triggers/v1alpha1/trigger_defaults_test.go b/pkg/apis/triggers/v1alpha1/trigger_defaults_test.go index c93761da8..908fc7b7f 100644 --- a/pkg/apis/triggers/v1alpha1/trigger_defaults_test.go +++ b/pkg/apis/triggers/v1alpha1/trigger_defaults_test.go @@ -22,6 +22,7 @@ import ( "github.com/google/go-cmp/cmp" "github.com/tektoncd/triggers/pkg/apis/triggers/v1alpha1" + "github.com/tektoncd/triggers/test" ) func TestTriggerSetDefaults(t *testing.T) { @@ -98,6 +99,150 @@ func TestTriggerSetDefaults(t *testing.T) { }}, }, }, + }, { + name: "default deprecatedGithub to new ref/params", + in: &v1alpha1.Trigger{ + Spec: v1alpha1.TriggerSpec{ + Interceptors: []*v1alpha1.TriggerInterceptor{{ + DeprecatedGitHub: &v1alpha1.GitHubInterceptor{ + SecretRef: &v1alpha1.SecretRef{ + SecretKey: "key", + SecretName: "name", + }, + EventTypes: []string{"push"}, + }, + }}, + }, + }, + wc: v1alpha1.WithUpgradeViaDefaulting, + want: &v1alpha1.Trigger{ + Spec: v1alpha1.TriggerSpec{ + Interceptors: []*v1alpha1.TriggerInterceptor{{ + Ref: v1alpha1.InterceptorRef{ + Name: "github", + Kind: v1alpha1.ClusterInterceptorKind, + }, + Params: []v1alpha1.InterceptorParams{{ + Name: "secretRef", + Value: test.ToV1JSON(t, &v1alpha1.SecretRef{ + SecretKey: "key", + SecretName: "name", + }), + }, { + Name: "eventTypes", + Value: test.ToV1JSON(t, []string{"push"}), + }}, + }}, + }, + }, + }, { + name: "default deprecatedGitlab to new ref/params", + in: &v1alpha1.Trigger{ + Spec: v1alpha1.TriggerSpec{ + Interceptors: []*v1alpha1.TriggerInterceptor{{ + DeprecatedGitLab: &v1alpha1.GitLabInterceptor{ + SecretRef: &v1alpha1.SecretRef{ + SecretKey: "key", + SecretName: "name", + }, + EventTypes: []string{"push"}, + }, + }}, + }, + }, + wc: v1alpha1.WithUpgradeViaDefaulting, + want: &v1alpha1.Trigger{ + Spec: v1alpha1.TriggerSpec{ + Interceptors: []*v1alpha1.TriggerInterceptor{{ + Ref: v1alpha1.InterceptorRef{ + Name: "gitlab", + Kind: v1alpha1.ClusterInterceptorKind, + }, + Params: []v1alpha1.InterceptorParams{{ + Name: "secretRef", + Value: test.ToV1JSON(t, &v1alpha1.SecretRef{ + SecretKey: "key", + SecretName: "name", + }), + }, { + Name: "eventTypes", + Value: test.ToV1JSON(t, []string{"push"}), + }}, + }}, + }, + }, + }, { + name: "default deprecatedBitbucket to new ref/params", + in: &v1alpha1.Trigger{ + Spec: v1alpha1.TriggerSpec{ + Interceptors: []*v1alpha1.TriggerInterceptor{{ + DeprecatedBitbucket: &v1alpha1.BitbucketInterceptor{ + SecretRef: &v1alpha1.SecretRef{ + SecretKey: "key", + SecretName: "name", + }, + EventTypes: []string{"push"}, + }, + }}, + }, + }, + wc: v1alpha1.WithUpgradeViaDefaulting, + want: &v1alpha1.Trigger{ + Spec: v1alpha1.TriggerSpec{ + Interceptors: []*v1alpha1.TriggerInterceptor{{ + Ref: v1alpha1.InterceptorRef{ + Name: "bitbucket", + Kind: v1alpha1.ClusterInterceptorKind, + }, + Params: []v1alpha1.InterceptorParams{{ + Name: "secretRef", + Value: test.ToV1JSON(t, &v1alpha1.SecretRef{ + SecretKey: "key", + SecretName: "name", + }), + }, { + Name: "eventTypes", + Value: test.ToV1JSON(t, []string{"push"}), + }}, + }}, + }, + }, + }, { + name: "default deprecatedCEL to new ref/params", + in: &v1alpha1.Trigger{ + Spec: v1alpha1.TriggerSpec{ + Interceptors: []*v1alpha1.TriggerInterceptor{{ + DeprecatedCEL: &v1alpha1.CELInterceptor{ + Filter: "body.foo == bar", + Overlays: []v1alpha1.CELOverlay{{ + Key: "abc", + Expression: "body.foo", + }}, + }, + }}, + }, + }, + wc: v1alpha1.WithUpgradeViaDefaulting, + want: &v1alpha1.Trigger{ + Spec: v1alpha1.TriggerSpec{ + Interceptors: []*v1alpha1.TriggerInterceptor{{ + Ref: v1alpha1.InterceptorRef{ + Name: "cel", + Kind: v1alpha1.ClusterInterceptorKind, + }, + Params: []v1alpha1.InterceptorParams{{ + Name: "filter", + Value: test.ToV1JSON(t, "body.foo == bar"), + }, { + Name: "overlays", + Value: test.ToV1JSON(t, []v1alpha1.CELOverlay{{ + Key: "abc", + Expression: "body.foo", + }}), + }}, + }}, + }, + }, }} for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { diff --git a/pkg/apis/triggers/v1alpha1/trigger_types.go b/pkg/apis/triggers/v1alpha1/trigger_types.go index 98d948b32..94513927a 100644 --- a/pkg/apis/triggers/v1alpha1/trigger_types.go +++ b/pkg/apis/triggers/v1alpha1/trigger_types.go @@ -17,6 +17,9 @@ limitations under the License. package v1alpha1 import ( + "encoding/json" + "fmt" + "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1" corev1 "k8s.io/api/core/v1" apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" @@ -135,6 +138,77 @@ func (ti *TriggerInterceptor) defaultInterceptorKind() { } } +func (ti *TriggerInterceptor) updateCoreInterceptors() error { + if ti == nil { + return nil + } + if ti.Ref.Name != "" { + return nil + } + ti.Ref.Name = ti.GetName() + ti.Params = []InterceptorParams{} + switch ti.Ref.Name { + case "bitbucket": + if err := addToParams(&ti.Params, "secretRef", ti.DeprecatedBitbucket.SecretRef); err != nil { + return err + } + if err := addToParams(&ti.Params, "eventTypes", ti.DeprecatedBitbucket.EventTypes); err != nil { + return err + } + ti.DeprecatedBitbucket = nil + case "gitlab": + if err := addToParams(&ti.Params, "secretRef", ti.DeprecatedGitLab.SecretRef); err != nil { + return err + } + if err := addToParams(&ti.Params, "eventTypes", ti.DeprecatedGitLab.EventTypes); err != nil { + return err + } + ti.DeprecatedGitLab = nil + case "github": + if err := addToParams(&ti.Params, "secretRef", ti.DeprecatedGitHub.SecretRef); err != nil { + return err + } + if err := addToParams(&ti.Params, "eventTypes", ti.DeprecatedGitHub.EventTypes); err != nil { + return err + } + ti.DeprecatedGitHub = nil + case "cel": + if err := addToParams(&ti.Params, "filter", ti.DeprecatedCEL.Filter); err != nil { + return err + } + if err := addToParams(&ti.Params, "overlays", ti.DeprecatedCEL.Overlays); err != nil { + return err + } + ti.DeprecatedCEL = nil + } + return nil +} + +func addToParams(params *[]InterceptorParams, name string, val interface{}) error { + if val == nil { + return nil + } + v, err := toV1JSON(val) + if err != nil { + return err + } + *params = append(*params, InterceptorParams{ + Name: name, + Value: v, + }) + return nil +} + +func toV1JSON(v interface{}) (apiextensionsv1.JSON, error) { + b, err := json.Marshal(v) + if err != nil { + return apiextensionsv1.JSON{}, fmt.Errorf("json.Marshal() failed: %s", err) + } + return apiextensionsv1.JSON{ + Raw: b, + }, nil +} + // GetName returns the name for the given interceptor func (ti *TriggerInterceptor) GetName() string { // This is temporary until we implement #869 diff --git a/pkg/apis/triggers/v1alpha1/trigger_types_test.go b/pkg/apis/triggers/v1alpha1/trigger_types_test.go index 28aeb6ca2..d41e5be41 100644 --- a/pkg/apis/triggers/v1alpha1/trigger_types_test.go +++ b/pkg/apis/triggers/v1alpha1/trigger_types_test.go @@ -14,46 +14,44 @@ See the License for the specific language governing permissions and limitations under the License. */ -package v1alpha1_test +package v1alpha1 import ( "testing" - - "github.com/tektoncd/triggers/pkg/apis/triggers/v1alpha1" ) func TestGetName(t *testing.T) { for _, tc := range []struct { - in v1alpha1.TriggerInterceptor + in TriggerInterceptor want string }{{ - in: v1alpha1.TriggerInterceptor{ - DeprecatedCEL: &v1alpha1.CELInterceptor{}, + in: TriggerInterceptor{ + DeprecatedCEL: &CELInterceptor{}, }, want: "cel", }, { - in: v1alpha1.TriggerInterceptor{ - DeprecatedGitLab: &v1alpha1.GitLabInterceptor{}, + in: TriggerInterceptor{ + DeprecatedGitLab: &GitLabInterceptor{}, }, want: "gitlab", }, { - in: v1alpha1.TriggerInterceptor{ - DeprecatedGitHub: &v1alpha1.GitHubInterceptor{}, + in: TriggerInterceptor{ + DeprecatedGitHub: &GitHubInterceptor{}, }, want: "github", }, { - in: v1alpha1.TriggerInterceptor{ - DeprecatedBitbucket: &v1alpha1.BitbucketInterceptor{}, + in: TriggerInterceptor{ + DeprecatedBitbucket: &BitbucketInterceptor{}, }, want: "bitbucket", }, { - in: v1alpha1.TriggerInterceptor{ - Webhook: &v1alpha1.WebhookInterceptor{}, + in: TriggerInterceptor{ + Webhook: &WebhookInterceptor{}, }, want: "", }, { - in: v1alpha1.TriggerInterceptor{ - Ref: v1alpha1.InterceptorRef{ + in: TriggerInterceptor{ + Ref: InterceptorRef{ Name: "pluggable-interceptor", }, }, @@ -67,3 +65,11 @@ func TestGetName(t *testing.T) { }) } } + +func TestUpdateCoreInterceptors_Error(t *testing.T) { + var ti *TriggerInterceptor + + if err := ti.updateCoreInterceptors(); err != nil { + t.Fatalf("updateCoreInterceptors() unexpected error: %s", err) + } +}