[GitHub] [camel-k] lburgazzoli commented on a diff in pull request #3523: fix(knative): Set a knative label in the namespace to allow sinkbinding injection

[GitBox <gi...@apache.org>]

lburgazzoli commented on code in PR #3523:
URL: https://github.com/apache/camel-k/pull/3523#discussion_r940947302


##########
pkg/platform/operator.go:
##########
@@ -133,6 +139,35 @@ func IsOperatorAllowedOnNamespace(ctx context.Context, c ctrl.Reader, namespace
 	return !alreadyOwned, nil
 }
 
+// EnableKnativeBindInNamespace sets the "bindings.knative.dev/include=true" label to the namespace if is openshift and knative is installed
+// Returns true if the label was set in the namespace
+// see https://knative.dev/v1.3-docs/eventing/custom-event-source/sinkbinding/create-a-sinkbinding/
+func EnableKnativeBindInNamespace(ctx context.Context, client client.Client, namespace string) (bool, error) {

Review Comment:
   I wonder if it is expected that the camel-k operator is the one setting the label on the namespace as this basically may override any set-up done by an administrator (i.e. it may be possible that you want to restrict some user to not being to use some functionalities) and also, it require additional privileges.
   
   Maybe we should:
   1. wait for the namespace to be updated before proceeding with deploying the integration
   2. add an additional condition to report this status 
   



-- 
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-unsubscribe@camel.apache.org

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