squakez commented on code in PR #5597:
URL: https://github.com/apache/camel-k/pull/5597#discussion_r1629412820


##########
pkg/trait/knative_test.go:
##########
@@ -361,6 +362,204 @@ func TestKnativeTriggerConfiguration(t *testing.T) {
                matching = matching && assert.Equal(t, "/events/evt.type", 
trigger.Spec.Subscriber.URI.Path)
                matching = matching && assert.Equal(t, "default-test-evttype", 
trigger.Name)
 
+               matching = matching && assert.NotNil(t, trigger.Spec.Filter)
+               matching = matching && assert.Len(t, 
trigger.Spec.Filter.Attributes, 2)
+               matching = matching && assert.Equal(t, 
trigger.Spec.Filter.Attributes["type"], "evt.type")
+               matching = matching && assert.Equal(t, 
trigger.Spec.Filter.Attributes["source"], "my-source")
+
+               return matching
+       }))
+}
+
+func TestKnativeTriggerConfigurationDefaultEventTypeFilter(t *testing.T) {
+       catalog, err := camel.DefaultCatalog()
+       require.NoError(t, err)
+
+       c, err := NewFakeClient("ns")
+       require.NoError(t, err)
+
+       traitCatalog := NewCatalog(c)
+
+       environment := Environment{
+               CamelCatalog: catalog,
+               Catalog:      traitCatalog,
+               Client:       c,
+               Integration: &v1.Integration{
+                       ObjectMeta: metav1.ObjectMeta{
+                               Name:      "test",
+                               Namespace: "ns",
+                       },
+                       Status: v1.IntegrationStatus{
+                               Phase: v1.IntegrationPhaseDeploying,
+                       },
+                       Spec: v1.IntegrationSpec{
+                               Profile: v1.TraitProfileKnative,
+                               Sources: []v1.SourceSpec{
+                                       {
+                                               DataSpec: v1.DataSpec{
+                                                       Name: "route.java",
+                                                       Content: `
+                                                               public class 
CartoonMessagesMover extends RouteBuilder {
+                                                                       public 
void configure() {
+                                                                               
from("knative:event/evt.type")
+                                                                               
        .log("${body}");
+                                                                       }
+                                                               }
+                                                       `,
+                                               },
+                                               Language: v1.LanguageJavaSource,
+                                       },
+                               },
+                               Traits: v1.Traits{
+                                       Knative: &traitv1.KnativeTrait{
+                                               Trait: traitv1.Trait{
+                                                       Enabled: 
pointer.Bool(true),
+                                               },
+                                       },
+                               },
+                       },
+               },
+               IntegrationKit: &v1.IntegrationKit{
+                       Status: v1.IntegrationKitStatus{
+                               Phase: v1.IntegrationKitPhaseReady,
+                       },
+               },
+               Platform: &v1.IntegrationPlatform{
+                       Spec: v1.IntegrationPlatformSpec{
+                               Cluster: v1.IntegrationPlatformClusterOpenShift,
+                               Build: v1.IntegrationPlatformBuildSpec{
+                                       PublishStrategy: 
v1.IntegrationPlatformBuildPublishStrategyS2I,
+                                       Registry:        
v1.RegistrySpec{Address: "registry"},
+                                       RuntimeVersion:  
catalog.Runtime.Version,
+                               },
+                               Profile: v1.TraitProfileKnative,
+                       },
+                       Status: v1.IntegrationPlatformStatus{
+                               Phase: v1.IntegrationPlatformPhaseReady,
+                       },
+               },
+               EnvVars:        make([]corev1.EnvVar, 0),
+               ExecutedTraits: make([]Trait, 0),
+               Resources:      k8sutils.NewCollection(),
+       }
+       environment.Platform.ResyncStatusFullConfig()
+
+       // don't care about conditions in this unit test
+       _, err = traitCatalog.apply(&environment)
+
+       require.NoError(t, err)
+       assert.NotEmpty(t, environment.ExecutedTraits)
+       assert.NotNil(t, environment.GetTrait("knative"))
+
+       assert.NotNil(t, environment.Resources.GetKnativeTrigger(func(trigger 
*eventing.Trigger) bool {

Review Comment:
   Why not taking the explicit trigger and doing each assert specifically. If 
that fails we know exactly what's going on.



##########
pkg/trait/knative_test.go:
##########
@@ -361,6 +362,204 @@ func TestKnativeTriggerConfiguration(t *testing.T) {
                matching = matching && assert.Equal(t, "/events/evt.type", 
trigger.Spec.Subscriber.URI.Path)
                matching = matching && assert.Equal(t, "default-test-evttype", 
trigger.Name)
 
+               matching = matching && assert.NotNil(t, trigger.Spec.Filter)
+               matching = matching && assert.Len(t, 
trigger.Spec.Filter.Attributes, 2)
+               matching = matching && assert.Equal(t, 
trigger.Spec.Filter.Attributes["type"], "evt.type")
+               matching = matching && assert.Equal(t, 
trigger.Spec.Filter.Attributes["source"], "my-source")
+
+               return matching
+       }))
+}
+
+func TestKnativeTriggerConfigurationDefaultEventTypeFilter(t *testing.T) {
+       catalog, err := camel.DefaultCatalog()
+       require.NoError(t, err)
+
+       c, err := NewFakeClient("ns")
+       require.NoError(t, err)
+
+       traitCatalog := NewCatalog(c)
+
+       environment := Environment{
+               CamelCatalog: catalog,
+               Catalog:      traitCatalog,
+               Client:       c,
+               Integration: &v1.Integration{
+                       ObjectMeta: metav1.ObjectMeta{
+                               Name:      "test",
+                               Namespace: "ns",
+                       },
+                       Status: v1.IntegrationStatus{
+                               Phase: v1.IntegrationPhaseDeploying,
+                       },
+                       Spec: v1.IntegrationSpec{
+                               Profile: v1.TraitProfileKnative,
+                               Sources: []v1.SourceSpec{
+                                       {
+                                               DataSpec: v1.DataSpec{
+                                                       Name: "route.java",
+                                                       Content: `
+                                                               public class 
CartoonMessagesMover extends RouteBuilder {
+                                                                       public 
void configure() {
+                                                                               
from("knative:event/evt.type")
+                                                                               
        .log("${body}");
+                                                                       }
+                                                               }
+                                                       `,
+                                               },
+                                               Language: v1.LanguageJavaSource,
+                                       },
+                               },
+                               Traits: v1.Traits{
+                                       Knative: &traitv1.KnativeTrait{
+                                               Trait: traitv1.Trait{
+                                                       Enabled: 
pointer.Bool(true),
+                                               },
+                                       },
+                               },
+                       },
+               },
+               IntegrationKit: &v1.IntegrationKit{
+                       Status: v1.IntegrationKitStatus{
+                               Phase: v1.IntegrationKitPhaseReady,
+                       },
+               },
+               Platform: &v1.IntegrationPlatform{
+                       Spec: v1.IntegrationPlatformSpec{
+                               Cluster: v1.IntegrationPlatformClusterOpenShift,
+                               Build: v1.IntegrationPlatformBuildSpec{
+                                       PublishStrategy: 
v1.IntegrationPlatformBuildPublishStrategyS2I,
+                                       Registry:        
v1.RegistrySpec{Address: "registry"},
+                                       RuntimeVersion:  
catalog.Runtime.Version,
+                               },
+                               Profile: v1.TraitProfileKnative,
+                       },
+                       Status: v1.IntegrationPlatformStatus{
+                               Phase: v1.IntegrationPlatformPhaseReady,
+                       },
+               },
+               EnvVars:        make([]corev1.EnvVar, 0),
+               ExecutedTraits: make([]Trait, 0),
+               Resources:      k8sutils.NewCollection(),
+       }
+       environment.Platform.ResyncStatusFullConfig()
+
+       // don't care about conditions in this unit test
+       _, err = traitCatalog.apply(&environment)
+
+       require.NoError(t, err)
+       assert.NotEmpty(t, environment.ExecutedTraits)
+       assert.NotNil(t, environment.GetTrait("knative"))
+
+       assert.NotNil(t, environment.Resources.GetKnativeTrigger(func(trigger 
*eventing.Trigger) bool {
+               matching := true
+
+               matching = matching && assert.Equal(t, "default", 
trigger.Spec.Broker)
+               matching = matching && assert.Equal(t, 
serving.SchemeGroupVersion.String(), trigger.Spec.Subscriber.Ref.APIVersion)
+               matching = matching && assert.Equal(t, "Service", 
trigger.Spec.Subscriber.Ref.Kind)
+               matching = matching && assert.Equal(t, "/events/evt.type", 
trigger.Spec.Subscriber.URI.Path)
+               matching = matching && assert.Equal(t, "default-test-evttype", 
trigger.Name)
+
+               matching = matching && assert.NotNil(t, trigger.Spec.Filter)
+               matching = matching && assert.Len(t, 
trigger.Spec.Filter.Attributes, 1)
+               matching = matching && assert.Equal(t, 
trigger.Spec.Filter.Attributes["type"], "evt.type")
+
+               return matching
+       }))
+}
+
+func TestKnativeTriggerConfigurationNoFilter(t *testing.T) {
+       catalog, err := camel.DefaultCatalog()
+       require.NoError(t, err)
+
+       c, err := NewFakeClient("ns")
+       require.NoError(t, err)
+
+       traitCatalog := NewCatalog(c)
+
+       environment := Environment{
+               CamelCatalog: catalog,
+               Catalog:      traitCatalog,
+               Client:       c,
+               Integration: &v1.Integration{
+                       ObjectMeta: metav1.ObjectMeta{
+                               Name:      "test",
+                               Namespace: "ns",
+                       },
+                       Status: v1.IntegrationStatus{
+                               Phase: v1.IntegrationPhaseDeploying,
+                       },
+                       Spec: v1.IntegrationSpec{
+                               Profile: v1.TraitProfileKnative,
+                               Sources: []v1.SourceSpec{
+                                       {
+                                               DataSpec: v1.DataSpec{
+                                                       Name: "route.java",
+                                                       Content: `
+                                                               public class 
CartoonMessagesMover extends RouteBuilder {
+                                                                       public 
void configure() {
+                                                                               
from("knative:event/evt.type")
+                                                                               
        .log("${body}");
+                                                                       }
+                                                               }
+                                                       `,
+                                               },
+                                               Language: v1.LanguageJavaSource,
+                                       },
+                               },
+                               Traits: v1.Traits{
+                                       Knative: &traitv1.KnativeTrait{
+                                               Trait: traitv1.Trait{
+                                                       Enabled: 
pointer.Bool(true),
+                                               },
+                                               FilterEventType: 
pointer.Bool(false),
+                                       },
+                               },
+                       },
+               },
+               IntegrationKit: &v1.IntegrationKit{
+                       Status: v1.IntegrationKitStatus{
+                               Phase: v1.IntegrationKitPhaseReady,
+                       },
+               },
+               Platform: &v1.IntegrationPlatform{
+                       Spec: v1.IntegrationPlatformSpec{
+                               Cluster: v1.IntegrationPlatformClusterOpenShift,
+                               Build: v1.IntegrationPlatformBuildSpec{
+                                       PublishStrategy: 
v1.IntegrationPlatformBuildPublishStrategyS2I,
+                                       Registry:        
v1.RegistrySpec{Address: "registry"},
+                                       RuntimeVersion:  
catalog.Runtime.Version,
+                               },
+                               Profile: v1.TraitProfileKnative,
+                       },
+                       Status: v1.IntegrationPlatformStatus{
+                               Phase: v1.IntegrationPlatformPhaseReady,
+                       },
+               },
+               EnvVars:        make([]corev1.EnvVar, 0),
+               ExecutedTraits: make([]Trait, 0),
+               Resources:      k8sutils.NewCollection(),
+       }
+       environment.Platform.ResyncStatusFullConfig()
+
+       // don't care about conditions in this unit test
+       _, err = traitCatalog.apply(&environment)
+
+       require.NoError(t, err)
+       assert.NotEmpty(t, environment.ExecutedTraits)
+       assert.NotNil(t, environment.GetTrait("knative"))
+
+       assert.NotNil(t, environment.Resources.GetKnativeTrigger(func(trigger 
*eventing.Trigger) bool {

Review Comment:
   Ditto



##########
pkg/util/bindings/knative_ref.go:
##########
@@ -93,26 +89,72 @@ func (k KnativeRefBindingProvider) Translate(ctx 
BindingContext, endpointCtx End
                if props["name"] == "" {
                        props["name"] = e.Ref.Name
                }
+
+               if endpointCtx.Type == v1.EndpointTypeSource {
+                       // Configure trigger filter attributes for the Knative 
event source
+                       for key, value := range props {
+                               if key == "cloudEventsType" {
+                                       // cloudEventsType is a synonym for 
type filter attribute
+                                       filterExpressions = 
append(filterExpressions, fmt.Sprintf("type=%s", value))
+                               } else if key != "name" {
+                                       filterExpressions = 
append(filterExpressions, fmt.Sprintf("%s=%s", key, value))
+                               }
+                       }
+               }
+
                if eventType, ok := props["type"]; ok {
-                       // consume prop
+                       // consume the type property and set it as URI path 
parameter
                        delete(props, "type")
                        serviceURI = fmt.Sprintf("knative:%s/%s", *serviceType, 
eventType)
+               } else if endpointCtx.Type == v1.EndpointTypeSink || 
endpointCtx.Type == v1.EndpointTypeAction {
+                       // Allowing no event type, but it can fail. See 
https://github.com/apache/camel-k-runtime/issues/536
+                       serviceURI = fmt.Sprintf("knative:%s", *serviceType)
+               } else if cloudEventsType, found := props["cloudEventsType"]; 
found {
+                       // set the cloud events type as URI path parameter, but 
keep it also as URI query param
+                       serviceURI = fmt.Sprintf("knative:%s/%s", *serviceType, 
cloudEventsType)
                } else {
-                       if endpointCtx.Type == v1.EndpointTypeSink || 
endpointCtx.Type == v1.EndpointTypeAction {
-                               // Allowing no event type, but it can fail. See 
https://github.com/apache/camel-k/v2-runtime/issues/536
-                               serviceURI = fmt.Sprintf("knative:%s", 
*serviceType)
-                       } else {
-                               return nil, errors.New(`property "type" must be 
provided when reading from the Broker`)
-                       }
+                       // Use default event type as a service URI path 
parameter as we need it for the Camel endpoint, but do not filter by the event 
type
+                       filterEventType = false
+                       serviceURI = fmt.Sprintf("knative:%s/%s", *serviceType, 
knativeapis.CamelCloudEventTypeDefault)
                }
        } else {
                serviceURI = fmt.Sprintf("knative:%s/%s", *serviceType, 
url.PathEscape(e.Ref.Name))
        }
 
+       // Remove filter attributes from props to avoid adding them to the 
service URI query params
+       for _, exp := range filterExpressions {
+               key, _ := property.SplitPropertyFileEntry(exp)
+               delete(props, key)
+       }
+
+       // Enrich service URI query params if not set
+       if props["apiVersion"] == "" {
+               props["apiVersion"] = e.Ref.APIVersion
+       }
+       if props["kind"] == "" {
+               props["kind"] = e.Ref.Kind
+       }
+
        serviceURI = uri.AppendParameters(serviceURI, props)
-       return &Binding{
+       var binding = Binding{
                URI: serviceURI,
-       }, nil
+       }
+
+       if len(filterExpressions) > 0 {

Review Comment:
   I am not sure it is a good way to configure it. The binding should know 
nothing about traits and its configuration I think.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscr...@camel.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to