squakez commented on code in PR #5597:
URL: https://github.com/apache/camel-k/pull/5597#discussion_r1629636250
##########
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:
Yes, I think the original idea was to use a way to represent certain options
in order to later merge in the Integration [1] (ie, knative trait only).
However, this changed when we have done the trait refactoring, creating a
direct dependency between traits and bindings [2]. This was in fact only used
in KnativeURI translator so far. I have the feeling that we should rework this
part and find a different approach instead of extending a potential wrong
design. What I mean is that the "trait" concept is an Integration thing and
extending it widely creates coupling we can regret in the future. @lburgazzoli
wdyt?
[1]
https://github.com/apache/camel-k/commit/19b66cffb432f98f8e29ca0bf71e6112f375c989#diff-abb6a6ae31c506ba2b634a29eba8930cd19766cdec4d7d3aae43fbc3b4a08996
[2]
https://github.com/apache/camel-k/commit/a92aaa405cf8a38a722c476251d04b99221f9682#diff-abb6a6ae31c506ba2b634a29eba8930cd19766cdec4d7d3aae43fbc3b4a08996
--
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
