From bb110d482660baf0b78d82795100c7d2ec0aecde Mon Sep 17 00:00:00 2001 From: David Simansky Date: Wed, 22 Mar 2023 13:31:28 +0100 Subject: [PATCH 1/4] feat: Add cesql filter field to trigger describe cmd --- pkg/kn/commands/trigger/describe.go | 10 ++++++++++ pkg/kn/commands/trigger/describe_test.go | 4 ++++ 2 files changed, 14 insertions(+) diff --git a/pkg/kn/commands/trigger/describe.go b/pkg/kn/commands/trigger/describe.go index 5ca33a7ab..19c156eb6 100644 --- a/pkg/kn/commands/trigger/describe.go +++ b/pkg/kn/commands/trigger/describe.go @@ -123,4 +123,14 @@ func writeTrigger(dw printers.PrefixWriter, trigger *v1beta1.Trigger, printDetai subWriter.WriteAttribute(key, value) } } + if len(trigger.Spec.Filters) > 0 { + // Split Filter and Filters (experimental) with new line + dw.WriteLine() + subWriter := dw.WriteAttribute("Filters (experimental)", "") + for _, filter := range trigger.Spec.Filters { + if filter.CESQL != "" { + subWriter.WriteAttribute("CESQL", filter.CESQL) + } + } + } } diff --git a/pkg/kn/commands/trigger/describe_test.go b/pkg/kn/commands/trigger/describe_test.go index c96a940fd..6dcfa1e8b 100644 --- a/pkg/kn/commands/trigger/describe_test.go +++ b/pkg/kn/commands/trigger/describe_test.go @@ -48,6 +48,7 @@ func TestSimpleDescribe(t *testing.T) { assert.Assert(t, util.ContainsAll(out, "Broker:", "mybroker")) assert.Assert(t, util.ContainsAll(out, "Filter:", "type", "foo.type.knative", "source", "src.eventing.knative")) + assert.Assert(t, util.ContainsAll(out, "Filters", "experimental", "CESQL", "LOWER", "type")) assert.Assert(t, util.ContainsAll(out, "Sink:", "Service", "myservicenamespace", "mysvc")) }) @@ -130,6 +131,9 @@ func getTriggerSinkRef() *v1beta1.Trigger { "source": "src.eventing.knative", }, }, + Filters: []v1beta1.SubscriptionsAPIFilter{ + {CESQL: "LOWER(type) = 'my-event-type'"}, + }, Subscriber: duckv1.Destination{ Ref: &duckv1.KReference{ Kind: "Service", From cc0999ed6ba30a34828d8722bea64435a2a3973c Mon Sep 17 00:00:00 2001 From: David Simansky Date: Wed, 22 Mar 2023 16:02:06 +0100 Subject: [PATCH 2/4] Refactor trigger describe based on defined fields in filter --- pkg/kn/commands/trigger/describe.go | 43 +++++++++++++++++++++++++++-- 1 file changed, 40 insertions(+), 3 deletions(-) diff --git a/pkg/kn/commands/trigger/describe.go b/pkg/kn/commands/trigger/describe.go index 19c156eb6..e19ea434d 100644 --- a/pkg/kn/commands/trigger/describe.go +++ b/pkg/kn/commands/trigger/describe.go @@ -16,6 +16,7 @@ package trigger import ( "errors" + "reflect" "github.com/spf13/cobra" "k8s.io/cli-runtime/pkg/genericclioptions" @@ -124,13 +125,49 @@ func writeTrigger(dw printers.PrefixWriter, trigger *v1beta1.Trigger, printDetai } } if len(trigger.Spec.Filters) > 0 { - // Split Filter and Filters (experimental) with new line + // Split 'Filter' and 'Filters (experimental)' with new line dw.WriteLine() subWriter := dw.WriteAttribute("Filters (experimental)", "") for _, filter := range trigger.Spec.Filters { - if filter.CESQL != "" { - subWriter.WriteAttribute("CESQL", filter.CESQL) + writeNesterFilters(subWriter, filter) + } + } +} + +// writeNesterFilters goes through SubscriptionsAPIFilter and writes its content accordingly +func writeNesterFilters(dw printers.PrefixWriter, filter v1beta1.SubscriptionsAPIFilter) { + v := reflect.ValueOf(filter) + for i := 0; i < v.NumField(); i++ { + field := v.Type().Field(i) + fieldValue := v.Field(i) + + // Write if it's non-zero string, fields: CESQL + if fieldValue.Kind() == reflect.String && !fieldValue.IsZero() { + dw.WriteAttribute(field.Name, fieldValue.String()) + } + // Write map[string]string key:value pairs of field: Exact, Prefix, Suffix + if fieldValue.Kind() == reflect.Map && fieldValue.Len() > 0 { + for k, v := range fieldValue.Interface().(map[string]string) { + dw.WriteAttribute(k, v) } } + + // iterate through []SubscriptionsAPIFilter of fields: All, Any + if fieldValue.Kind() == reflect.Slice { + for j := 0; j < fieldValue.Len(); j++ { + element := fieldValue.Index(j) + // Write filter field name only and created indentation + dw = dw.WriteAttribute(field.Name, "") + // Call write recursively for struct SubscriptionsAPIFilter + if element.Kind() == reflect.Struct { + writeNesterFilters(dw, element.Interface().(v1beta1.SubscriptionsAPIFilter)) + } + } + } + + // Call write recursively for struct SubscriptionsAPIFilter of field: Not + if fieldValue.Kind() == reflect.Struct { + writeNesterFilters(dw, fieldValue.Interface().(v1beta1.SubscriptionsAPIFilter)) + } } } From b22cbc0732e3129d6fed39829fa8f5fb01677e0d Mon Sep 17 00:00:00 2001 From: David Simansky Date: Wed, 22 Mar 2023 16:05:07 +0100 Subject: [PATCH 3/4] Fix comment typo --- pkg/kn/commands/trigger/describe.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/pkg/kn/commands/trigger/describe.go b/pkg/kn/commands/trigger/describe.go index e19ea434d..6cc25be92 100644 --- a/pkg/kn/commands/trigger/describe.go +++ b/pkg/kn/commands/trigger/describe.go @@ -129,13 +129,13 @@ func writeTrigger(dw printers.PrefixWriter, trigger *v1beta1.Trigger, printDetai dw.WriteLine() subWriter := dw.WriteAttribute("Filters (experimental)", "") for _, filter := range trigger.Spec.Filters { - writeNesterFilters(subWriter, filter) + writeNestedFilters(subWriter, filter) } } } -// writeNesterFilters goes through SubscriptionsAPIFilter and writes its content accordingly -func writeNesterFilters(dw printers.PrefixWriter, filter v1beta1.SubscriptionsAPIFilter) { +// writeNestedFilters goes through SubscriptionsAPIFilter and writes its content accordingly +func writeNestedFilters(dw printers.PrefixWriter, filter v1beta1.SubscriptionsAPIFilter) { v := reflect.ValueOf(filter) for i := 0; i < v.NumField(); i++ { field := v.Type().Field(i) @@ -156,18 +156,18 @@ func writeNesterFilters(dw printers.PrefixWriter, filter v1beta1.SubscriptionsAP if fieldValue.Kind() == reflect.Slice { for j := 0; j < fieldValue.Len(); j++ { element := fieldValue.Index(j) - // Write filter field name only and created indentation + // Write filter field name only and create next indentation dw = dw.WriteAttribute(field.Name, "") // Call write recursively for struct SubscriptionsAPIFilter if element.Kind() == reflect.Struct { - writeNesterFilters(dw, element.Interface().(v1beta1.SubscriptionsAPIFilter)) + writeNestedFilters(dw, element.Interface().(v1beta1.SubscriptionsAPIFilter)) } } } // Call write recursively for struct SubscriptionsAPIFilter of field: Not if fieldValue.Kind() == reflect.Struct { - writeNesterFilters(dw, fieldValue.Interface().(v1beta1.SubscriptionsAPIFilter)) + writeNestedFilters(dw, fieldValue.Interface().(v1beta1.SubscriptionsAPIFilter)) } } } From 0e2e46154ab35b24ff163ebd5c4f2c94f0c82124 Mon Sep 17 00:00:00 2001 From: David Simansky Date: Fri, 24 Mar 2023 00:55:22 +0100 Subject: [PATCH 4/4] Refactor without reflection --- pkg/kn/commands/trigger/describe.go | 75 ++++++----- pkg/kn/commands/trigger/describe_test.go | 157 ++++++++++++++++++++++- 2 files changed, 200 insertions(+), 32 deletions(-) diff --git a/pkg/kn/commands/trigger/describe.go b/pkg/kn/commands/trigger/describe.go index 6cc25be92..278c67499 100644 --- a/pkg/kn/commands/trigger/describe.go +++ b/pkg/kn/commands/trigger/describe.go @@ -16,11 +16,9 @@ package trigger import ( "errors" - "reflect" "github.com/spf13/cobra" "k8s.io/cli-runtime/pkg/genericclioptions" - "knative.dev/client/lib/printing" "knative.dev/client/pkg/kn/commands" "knative.dev/client/pkg/printers" @@ -136,38 +134,53 @@ func writeTrigger(dw printers.PrefixWriter, trigger *v1beta1.Trigger, printDetai // writeNestedFilters goes through SubscriptionsAPIFilter and writes its content accordingly func writeNestedFilters(dw printers.PrefixWriter, filter v1beta1.SubscriptionsAPIFilter) { - v := reflect.ValueOf(filter) - for i := 0; i < v.NumField(); i++ { - field := v.Type().Field(i) - fieldValue := v.Field(i) - - // Write if it's non-zero string, fields: CESQL - if fieldValue.Kind() == reflect.String && !fieldValue.IsZero() { - dw.WriteAttribute(field.Name, fieldValue.String()) + // All []SubscriptionsAPIFilter + if len(filter.All) > 0 { + // create new indentation after name + subWriter := dw.WriteAttribute("all", "") + for _, nestedFilter := range filter.All { + writeNestedFilters(subWriter, nestedFilter) } - // Write map[string]string key:value pairs of field: Exact, Prefix, Suffix - if fieldValue.Kind() == reflect.Map && fieldValue.Len() > 0 { - for k, v := range fieldValue.Interface().(map[string]string) { - dw.WriteAttribute(k, v) - } + } + // Any []SubscriptionsAPIFilter + if len(filter.Any) > 0 { + // create new indentation after name + subWriter := dw.WriteAttribute("any", "") + for _, nestedFilter := range filter.Any { + writeNestedFilters(subWriter, nestedFilter) } - - // iterate through []SubscriptionsAPIFilter of fields: All, Any - if fieldValue.Kind() == reflect.Slice { - for j := 0; j < fieldValue.Len(); j++ { - element := fieldValue.Index(j) - // Write filter field name only and create next indentation - dw = dw.WriteAttribute(field.Name, "") - // Call write recursively for struct SubscriptionsAPIFilter - if element.Kind() == reflect.Struct { - writeNestedFilters(dw, element.Interface().(v1beta1.SubscriptionsAPIFilter)) - } - } + } + // Not *SubscriptionsAPIFilter + if filter.Not != nil { + subWriter := dw.WriteAttribute("not", "") + writeNestedFilters(subWriter, *filter.Not) + } + // Exact map[string]string + if len(filter.Exact) > 0 { + // create new indentation after name + subWriter := dw.WriteAttribute("exact", "") + for k, v := range filter.Exact { + subWriter.WriteAttribute(k, v) } - - // Call write recursively for struct SubscriptionsAPIFilter of field: Not - if fieldValue.Kind() == reflect.Struct { - writeNestedFilters(dw, fieldValue.Interface().(v1beta1.SubscriptionsAPIFilter)) + } + // Prefix map[string]string + if len(filter.Prefix) > 0 { + // create new indentation after name + subWriter := dw.WriteAttribute("prefix", "") + for k, v := range filter.Prefix { + subWriter.WriteAttribute(k, v) } } + // Suffix map[string]string + if len(filter.Suffix) > 0 { + // create new indentation after name + subWriter := dw.WriteAttribute("suffix", "") + for k, v := range filter.Suffix { + subWriter.WriteAttribute(k, v) + } + } + // CESQL string + if filter.CESQL != "" { + dw.WriteAttribute("cesql", filter.CESQL) + } } diff --git a/pkg/kn/commands/trigger/describe_test.go b/pkg/kn/commands/trigger/describe_test.go index 6dcfa1e8b..4d5188965 100644 --- a/pkg/kn/commands/trigger/describe_test.go +++ b/pkg/kn/commands/trigger/describe_test.go @@ -15,10 +15,13 @@ package trigger import ( + "bytes" "encoding/json" "errors" "testing" + "knative.dev/client/pkg/printers" + "gotest.tools/v3/assert" "gotest.tools/v3/assert/cmp" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -48,7 +51,7 @@ func TestSimpleDescribe(t *testing.T) { assert.Assert(t, util.ContainsAll(out, "Broker:", "mybroker")) assert.Assert(t, util.ContainsAll(out, "Filter:", "type", "foo.type.knative", "source", "src.eventing.knative")) - assert.Assert(t, util.ContainsAll(out, "Filters", "experimental", "CESQL", "LOWER", "type")) + assert.Assert(t, util.ContainsAll(out, "Filters", "experimental", "cesql", "LOWER", "type")) assert.Assert(t, util.ContainsAll(out, "Sink:", "Service", "myservicenamespace", "mysvc")) }) @@ -113,6 +116,158 @@ func TestDescribeTriggerMachineReadable(t *testing.T) { recorder.Validate() } +func TestWriteNestedFilters(t *testing.T) { + testCases := []struct { + name string + filter v1beta1.SubscriptionsAPIFilter + expectedOutput string + }{ + { + name: "Exact filter", + filter: v1beta1.SubscriptionsAPIFilter{ + Exact: map[string]string{ + "type": "example"}}, + expectedOutput: "exact: \n" + + " type: example\n", + }, + { + name: "Prefix filter", + filter: v1beta1.SubscriptionsAPIFilter{ + Prefix: map[string]string{ + "type": "foo.bar"}}, + expectedOutput: "" + + "prefix: \n" + + " type: foo.bar\n", + }, + { + name: "Suffix filter", + filter: v1beta1.SubscriptionsAPIFilter{ + Suffix: map[string]string{ + "type": "foo.bar"}}, + expectedOutput: "" + + "suffix: \n" + + " type: foo.bar\n", + }, + { + name: "All filter", + filter: v1beta1.SubscriptionsAPIFilter{ + All: []v1beta1.SubscriptionsAPIFilter{ + {Exact: map[string]string{ + "type": "foo.bar"}}, + {Prefix: map[string]string{ + "source": "foo"}}, + {Suffix: map[string]string{ + "subject": "test"}}}}, + expectedOutput: "" + + "all: \n" + + " exact: \n" + + " type: foo.bar\n" + + " prefix: \n" + + " source: foo\n" + + " suffix: \n" + + " subject: test\n", + }, + { + name: "Any filter", + filter: v1beta1.SubscriptionsAPIFilter{ + Any: []v1beta1.SubscriptionsAPIFilter{ + {Exact: map[string]string{ + "type": "foo.bar"}}, + {Prefix: map[string]string{ + "source": "foo"}}, + {Suffix: map[string]string{ + "subject": "test"}}}, + }, + expectedOutput: "" + + "any: \n" + + " exact: \n" + + " type: foo.bar\n" + + " prefix: \n" + + " source: foo\n" + + " suffix: \n" + + " subject: test\n", + }, + { + name: "Nested All filter", + filter: v1beta1.SubscriptionsAPIFilter{ + All: []v1beta1.SubscriptionsAPIFilter{ + {Exact: map[string]string{ + "type": "foo.bar"}}, + {All: []v1beta1.SubscriptionsAPIFilter{ + {Prefix: map[string]string{ + "source": "foo"}}, + {Suffix: map[string]string{ + "subject": "test"}}}}}}, + expectedOutput: "" + + "all: \n" + + " exact: \n" + + " type: foo.bar\n" + + " all: \n" + + " prefix: \n" + + " source: foo\n" + + " suffix: \n" + + " subject: test\n", + }, + { + name: "Nested Any filter", + filter: v1beta1.SubscriptionsAPIFilter{ + Any: []v1beta1.SubscriptionsAPIFilter{ + {Exact: map[string]string{ + "type": "foo.bar"}}, + {Any: []v1beta1.SubscriptionsAPIFilter{ + {Prefix: map[string]string{ + "source": "foo"}}, + {Suffix: map[string]string{ + "subject": "test"}}}}}}, + expectedOutput: "" + + "any: \n" + + " exact: \n" + + " type: foo.bar\n" + + " any: \n" + + " prefix: \n" + + " source: foo\n" + + " suffix: \n" + + " subject: test\n", + }, + { + name: "Nested Not filter", + filter: v1beta1.SubscriptionsAPIFilter{ + Not: &v1beta1.SubscriptionsAPIFilter{ + Exact: map[string]string{ + "type": "foo.bar", + }, + Prefix: map[string]string{ + "type": "bar", + }, + CESQL: "select bar", + Not: &v1beta1.SubscriptionsAPIFilter{ + Suffix: map[string]string{ + "source": "foo"}}}}, + expectedOutput: "" + + "not: \n" + + " not: \n" + + " suffix: \n" + + " source: foo\n" + + " exact: \n" + + " type: foo.bar\n" + + " prefix: \n" + + " type: bar\n" + + " cesql: select bar\n", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + buf := &bytes.Buffer{} + dw := printers.NewPrefixWriter(buf) + writeNestedFilters(dw, tc.filter) + err := dw.Flush() + assert.NilError(t, err) + assert.Equal(t, tc.expectedOutput, buf.String()) + }) + } +} + func getTriggerSinkRef() *v1beta1.Trigger { return &v1beta1.Trigger{ TypeMeta: v1.TypeMeta{