You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@pulsar.apache.org by GitBox <gi...@apache.org> on 2022/02/21 19:28:11 UTC

[GitHub] [pulsar-helm-chart] bsheltonihs opened a new pull request #236: To address the function role vs clusterrole issue

bsheltonihs opened a new pull request #236:
URL: https://github.com/apache/pulsar-helm-chart/pull/236


   Fixes #230
   
   ### Motivation
   
   *When functions are enabled it deploys only with clusterrole and clusterrolebinding. However the use case I have is I am unable to those within a namespace.*
   
   ### Modifications
   
   *I followed the example in the already approved file "broker-cluster-role-binding.yaml" for the approach to deal with this issue. It allows for a role and rolebinding or clusterrole and clusterrolebinding and doesn't force you into only one.*
   
   ### Verifying this change
   
   - [ ] Make sure that the change passes the CI checks.
   


-- 
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: dev-unsubscribe@pulsar.apache.org

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



[GitHub] [pulsar-helm-chart] bsheltonihs commented on a change in pull request #236: To address the function role vs clusterrole issue

Posted by GitBox <gi...@apache.org>.
bsheltonihs commented on a change in pull request #236:
URL: https://github.com/apache/pulsar-helm-chart/pull/236#discussion_r812296075



##########
File path: charts/pulsar/values.yaml
##########
@@ -776,6 +776,11 @@ broker:
 ##
 functions:
   component: functions-worker
+  ## Pulsar: Functions Worker ClusterRole or Role
+  ## templates/broker-rbac.yaml
+  # Default is false which deploys functions with ClusterRole and ClusterRoleBinding at the cluster level
+  # Set to true to deploy functions with Role and RoleBinding inside the specified namespace
+  limit_to_namespace: false

Review comment:
       took the suggestion/recommendation and moved it under `functions.rbac.limit_to_namespace`




-- 
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: dev-unsubscribe@pulsar.apache.org

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



[GitHub] [pulsar-helm-chart] bsheltonihs commented on a change in pull request #236: To address the function role vs clusterrole issue

Posted by GitBox <gi...@apache.org>.
bsheltonihs commented on a change in pull request #236:
URL: https://github.com/apache/pulsar-helm-chart/pull/236#discussion_r812129671



##########
File path: charts/pulsar/values.yaml
##########
@@ -777,6 +777,8 @@ broker:
 functions:
   component: functions-worker
 
+  limit_to_namespace: false

Review comment:
       I would look to those who are experienced committers to determine if there should be some guidance/documentation on this. I have no issues providing that info and use cases if you want. 
   
   By setting the default ` limit_to_namespace` to be `false` it shouldn't impact the current deployments/use cases for those who have the access/ability to create `clusterrole` and `clusterrolebinding` would still be able to. By setting ` limit_to_namespace` to be `true` it provides the ability to deploy it within a namespace by using `role` and `rolebinding`.
   
   One use case for the latter is if you have to restrict tenants to a single namespace and thus can't allow access to use `clusterrole` or `clusterrolebinding`. Allowing `clusterrole` or `clusterrolebinding` requires an elevated level of permissions that reaches beyond just a given namespace. 
   
   By making this change it allows for the use/deployment of functions within a namespace just like what was done for the broker within the file `broker-cluster-role-binding.yaml`  




-- 
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: dev-unsubscribe@pulsar.apache.org

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



[GitHub] [pulsar-helm-chart] frankjkelly commented on a change in pull request #236: To address the function role vs clusterrole issue

Posted by GitBox <gi...@apache.org>.
frankjkelly commented on a change in pull request #236:
URL: https://github.com/apache/pulsar-helm-chart/pull/236#discussion_r812069495



##########
File path: charts/pulsar/values.yaml
##########
@@ -777,6 +777,8 @@ broker:
 functions:
   component: functions-worker
 
+  limit_to_namespace: false

Review comment:
       Do we need documentation on how this value should relate to `rbac. limit_to_namespace`
   https://github.com/apache/pulsar-helm-chart/blob/e0e53c1518ae7381013031db1f35ce77f35c27e7/charts/pulsar/values.yaml#L93
   
   Under what use-case would the two values be different?




-- 
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: dev-unsubscribe@pulsar.apache.org

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



[GitHub] [pulsar-helm-chart] bsheltonihs commented on pull request #236: To address the function role vs clusterrole issue

Posted by GitBox <gi...@apache.org>.
bsheltonihs commented on pull request #236:
URL: https://github.com/apache/pulsar-helm-chart/pull/236#issuecomment-1047888370


   I believe that I have made the needed changes for it to be backwards compatible.


-- 
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: dev-unsubscribe@pulsar.apache.org

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



[GitHub] [pulsar-helm-chart] bsheltonihs commented on a change in pull request #236: To address the function role vs clusterrole issue

Posted by GitBox <gi...@apache.org>.
bsheltonihs commented on a change in pull request #236:
URL: https://github.com/apache/pulsar-helm-chart/pull/236#discussion_r812135669



##########
File path: charts/pulsar/values.yaml
##########
@@ -777,6 +777,8 @@ broker:
 functions:
   component: functions-worker
 
+  limit_to_namespace: false

Review comment:
       I will note that I intentionally placed the attribute under `functions. limit_to_namespace` in the `values.yaml`. The logic for this was that I was trying to identify that this restricts `role` and `rolebinding` to the namespace only for `functions`.




-- 
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: dev-unsubscribe@pulsar.apache.org

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



[GitHub] [pulsar-helm-chart] sijie commented on a change in pull request #236: To address the function role vs clusterrole issue

Posted by GitBox <gi...@apache.org>.
sijie commented on a change in pull request #236:
URL: https://github.com/apache/pulsar-helm-chart/pull/236#discussion_r812291367



##########
File path: charts/pulsar/values.yaml
##########
@@ -776,6 +776,11 @@ broker:
 ##
 functions:
   component: functions-worker
+  ## Pulsar: Functions Worker ClusterRole or Role
+  ## templates/broker-rbac.yaml
+  # Default is false which deploys functions with ClusterRole and ClusterRoleBinding at the cluster level
+  # Set to true to deploy functions with Role and RoleBinding inside the specified namespace
+  limit_to_namespace: false

Review comment:
       @bsheltonihs can you make this setting to be `functions.rbac.limit_to_namespace`? it is probably better to be scoped under `functions.rbac` so people understand that it is a `rbac` related setting.




-- 
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: dev-unsubscribe@pulsar.apache.org

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