You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by GitBox <gi...@apache.org> on 2020/02/26 12:57:04 UTC
[GitHub] [pulsar] KannarFr opened a new pull request #6428: [Issue
5720][authorization provider] (WIP) Add more granularity
KannarFr opened a new pull request #6428: [Issue 5720][authorization provider] (WIP) Add more granularity
URL: https://github.com/apache/pulsar/pull/6428
Fixes #5720
### Motivation
Provide "real" authz abilities to pulsar resources.
### Modifications
Add stuff to `AuthorizationProvider` interface, and use them on every pulsar resource management auth (tenant, namespace, topics, functions, connectors, ...)
### Verifying this change
- [ ] Make sure that the change passes the CI checks.
This change added tests and can be verified as follows:
Will add specified test.
### Documentation
- Does this pull request introduce a new feature? yes
- If yes, how is the feature documented? not documented (yet, will do)
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [pulsar] sijie commented on issue #6428: [Issue 5720][authorization
provider] (WIP) Add more granularity
Posted by GitBox <gi...@apache.org>.
sijie commented on issue #6428: [Issue 5720][authorization provider] (WIP) Add more granularity
URL: https://github.com/apache/pulsar/pull/6428#issuecomment-604260371
@codelipenghui @jiazhai @rdhabalia @merlimat please take a look at this pull request
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [pulsar] KannarFr commented on issue #6428: [Issue
5720][authorization provider] Add granularity
Posted by GitBox <gi...@apache.org>.
KannarFr commented on issue #6428: [Issue 5720][authorization provider] Add granularity
URL: https://github.com/apache/pulsar/pull/6428#issuecomment-610910931
/pulsarbot run-failure-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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [pulsar] joefk commented on issue #6428: [Issue 5720][authorization
provider] (WIP) Add more granularity
Posted by GitBox <gi...@apache.org>.
joefk commented on issue #6428: [Issue 5720][authorization provider] (WIP) Add more granularity
URL: https://github.com/apache/pulsar/pull/6428#issuecomment-596888065
> Due to "real" authz plugin we should drop all roles/actions stuff which are in ZK. DefaultAuthzProvider should be updated regarding this.That being said, we need maintain legacy stuff so I would like to have your opinions about this.
I would like to see legacy stuff work as it is.
My concern is that is trying to impose a model which is not supported in Pulsar. Assumptions like "Any resource operation should be managed/authorized by the resource which owns it." and "ClusterOperations should manage Tenants, TenantOperation should manage Namespaces, etc. Hierarchical approach" it only makes me think that is being designed for some other system, which is not Pulsar.
Pulsar has no operational hierarchy beyond tenants. It is not designed for namespace admins or topic admins. The premise of pulsar is that a tenant manages all its namespaces. There are operations that can be logically managed at the namespace and topic level, but that is a convenience for the tenant admin.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [pulsar] sijie commented on a change in pull request #6428: [Issue
5720][authorization provider] Add granularity
Posted by GitBox <gi...@apache.org>.
sijie commented on a change in pull request #6428: [Issue 5720][authorization provider] Add granularity
URL: https://github.com/apache/pulsar/pull/6428#discussion_r405966371
##########
File path: pulsar-common/src/main/java/org/apache/pulsar/common/policies/data/NamespaceOperation.java
##########
@@ -0,0 +1,87 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.pulsar.common.policies.data;
+
+/**
+ * Namespace authorization operations.
+ */
+public enum NamespaceOperation {
+ CLEAR_BACKLOG,
+ DELETE_BUNDLE,
+ GET_ANTI_AFFINITY_GROUP,
+ GET_BACKLOG_QUOTAS,
+ GET_BUNDLES,
+ GET_COMPACTION_THRESHOLD,
+ GET_DELAYED_DELIVERY,
+ GET_DISPATCH_RATE,
+ GET_MAX_CONSUMERS_PER_SUBSCRIPTION,
+ GET_MAX_CONSUMERS_PER_TOPIC,
+ GET_MAX_PRODUCERS_PER_TOPIC,
+ GET_MAX_UNACKED_MESSAGES_PER_CONSUMER,
+ GET_MAX_UNACKED_MESSAGES_PER_SUBSCRIPTION,
+ GET_MESSAGE_TTL,
+ GET_OFFLOAD_DELETION_LAG,
+ GET_OFFLOAD_POLICIES,
+ GET_OFFLOAD_THRESHOLD,
+ GET_PERSISTENCE,
+ GET_PUBLISH_RATE,
+ GET_REPLICATION_CLUSTERS,
+ GET_REPLICATOR_DISPATCH_RATE,
+ GET_RETENTION,
+ GET_SCHEMA_AUTOUPDATE_STRATEGY,
+ GET_SCHEMA_COMPATIBILITY_STRATEGY,
+ GET_SCHEMA_VALIDATION_ENFORCED,
+ GET_SUBSCRIBE_RATE,
+ GET_SUBSCRIPTION_DISPATCH_RATE,
+ GRANT_PERMISSIONS,
Review comment:
I would actually move "permissions" related operations to a separate category.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [pulsar] KannarFr edited a comment on issue #6428: [Issue
5720][authorization provider] (WIP) Add more granularity
Posted by GitBox <gi...@apache.org>.
KannarFr edited a comment on issue #6428: [Issue 5720][authorization provider] (WIP) Add more granularity
URL: https://github.com/apache/pulsar/pull/6428#issuecomment-592485485
@joefk I think every permissions should be granted by authz provider. So we need to provide authz provider stuff to authorize or not, but it's not to the pulsar side to decide if access is grant or not. Maybe we can document critical stuff as mentioned unload but that's it. I'm not working on the PulsarAuthorizationProvider which is the default used, but on the AuthorizationProvider interface and its usages.
Thats being said, I can be wrong, WDYT?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [pulsar] KannarFr commented on a change in pull request #6428:
[Issue 5720][authorization provider] (WIP) Add more granularity
Posted by GitBox <gi...@apache.org>.
KannarFr commented on a change in pull request #6428: [Issue 5720][authorization provider] (WIP) Add more granularity
URL: https://github.com/apache/pulsar/pull/6428#discussion_r398545130
##########
File path: pulsar-common/src/main/java/org/apache/pulsar/common/policies/data/NamespaceOperation.java
##########
@@ -0,0 +1,83 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.pulsar.common.policies.data;
+
+/**
+ * Namespace authorization operations.
+ */
+public enum NamespaceOperation {
Review comment:
We can probably simplify these actions I totally agree, I followed the validated PIP-49. Simplify them is really hard: for example, list Namespaces in a tenant should be tenant level authz but only for list Namespaces because it can be done by a tenant user. But set rates for tenant must only be done by super User and not a user, so we need these split.
I guess we can have this issue a lot of times, not every endpoints, but it's hard to maintain. Which endpoint goes for this operation or this operation. Split all of them if easy to maintain. This comes with the defaultauthz provider which keeps the default role based as super user, tenant admin, or producer/consume authz. But the provider is passing the operations.
The goal here is to enable plugins authz to have all authz powers and all the context to authorize. In default case role is a user, but when you consider role as a token with all authorized info deserialized in custom authz provider it's so powerful.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [pulsar] sijie commented on a change in pull request #6428: [Issue
5720][authorization provider] Add granularity
Posted by GitBox <gi...@apache.org>.
sijie commented on a change in pull request #6428: [Issue 5720][authorization provider] Add granularity
URL: https://github.com/apache/pulsar/pull/6428#discussion_r405965538
##########
File path: pulsar-common/src/main/java/org/apache/pulsar/common/policies/data/BrokerOperation.java
##########
@@ -0,0 +1,35 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.pulsar.common.policies.data;
+
+/**
+ * Broker authorization operations.
+ */
+public enum BrokerOperation {
+ DELETE_DYNAMIC_CONFIG,
+ GET_ALL_DYNAMIC_CONFIG,
+ GET_INTERNAL_CONFIG,
+ GET_RUNTIME_CONFIG,
+ HEALTHCHECK,
+ LIST,
+ LIST_DYNAMIC_CONFIG,
+ NAMESPACES,
Review comment:
What doesn't `NAMESPACES` mean here?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [pulsar] sijie commented on a change in pull request #6428: [Issue
5720][authorization provider] Add granularity
Posted by GitBox <gi...@apache.org>.
sijie commented on a change in pull request #6428: [Issue 5720][authorization provider] Add granularity
URL: https://github.com/apache/pulsar/pull/6428#discussion_r406448379
##########
File path: pulsar-common/src/main/java/org/apache/pulsar/common/policies/data/NamespaceOperation.java
##########
@@ -0,0 +1,32 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.pulsar.common.policies.data;
+
+/**
+ * Namespace authorization operations.
+ */
+public enum NamespaceOperation {
+ AUTH_MODE,
+ BACKLOG,
Review comment:
The verb for this operation is "CLEAR_BACKLOG", not "BACKLOG".
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [pulsar] sijie commented on a change in pull request #6428: [Issue
5720][authorization provider] Add granularity
Posted by GitBox <gi...@apache.org>.
sijie commented on a change in pull request #6428: [Issue 5720][authorization provider] Add granularity
URL: https://github.com/apache/pulsar/pull/6428#discussion_r406447196
##########
File path: pulsar-common/src/main/java/org/apache/pulsar/common/policies/data/NamespaceOperation.java
##########
@@ -0,0 +1,32 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.pulsar.common.policies.data;
+
+/**
+ * Namespace authorization operations.
+ */
+public enum NamespaceOperation {
+ AUTH_MODE,
Review comment:
First, it is "subscription auth mode" not "auth mode".
Secondly, it is a policy not a verb to a namespace resource.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [pulsar] joefk commented on issue #6428: [Issue 5720][authorization
provider] (WIP) Add more granularity
Posted by GitBox <gi...@apache.org>.
joefk commented on issue #6428: [Issue 5720][authorization provider] (WIP) Add more granularity
URL: https://github.com/apache/pulsar/pull/6428#issuecomment-592807888
@KannarF
> I think every permissions should be granted by authz provider.
Some of them are not grantable. How did your arrive at that conclusion? That is a very simplistic view of the system. After going through PIP-49 and the discussions about that, what is your take?
As, you state in the description of your PR..
>And the biggest part of the task: go through every usage of validateSuperUserAccess and validateAdminAccessForTenant and check if they can be replaced with finer grain access
Can you share your findings for that? I did not see any
My concern is this. How can I, as a Pulsar operator (NOT a tenant) decide which of these operations I will delegate to my tenants for finer control ?
Operators should not be put in a position where if they were to deploy this authz provider, they would have to go and turn off/on certain actions for all fine-grained resources every time someone creates a namespace or topic. And the things you are proposing to delegate here in this proposal are an operators nightmare for a large cluster. Hence my ask. Put the list of the things that can be delegated into a config file, so that operators can decide ONCE when they DEPLOY Pulsar, about what can be potentially delegated. Don't hardcode it into the server, and create runtime maintenance work for operators
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [pulsar] sijie commented on a change in pull request #6428: [Issue
5720][authorization provider] Add granularity
Posted by GitBox <gi...@apache.org>.
sijie commented on a change in pull request #6428: [Issue 5720][authorization provider] Add granularity
URL: https://github.com/apache/pulsar/pull/6428#discussion_r406450103
##########
File path: pulsar-common/src/main/java/org/apache/pulsar/common/policies/data/NamespaceOperation.java
##########
@@ -0,0 +1,32 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.pulsar.common.policies.data;
+
+/**
+ * Namespace authorization operations.
+ */
+public enum NamespaceOperation {
+ AUTH_MODE,
+ BACKLOG,
+ BUNDLE,
+ LIST_TOPICS,
+ PERMISSION,
Review comment:
"PERMISSION" is not a verb.
There are two verbs here: "GRANT_PERMISSION" and "REVOKE_PERMISSION".
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [pulsar] sijie commented on a change in pull request #6428: [Issue
5720][authorization provider] Add granularity
Posted by GitBox <gi...@apache.org>.
sijie commented on a change in pull request #6428: [Issue 5720][authorization provider] Add granularity
URL: https://github.com/apache/pulsar/pull/6428#discussion_r406451481
##########
File path: pulsar-common/src/main/java/org/apache/pulsar/common/policies/data/PolicyName.java
##########
@@ -0,0 +1,44 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.pulsar.common.policies.data;
+
+/**
+ * PolicyName authorization operations.
+ */
+public enum PolicyName {
+ ALL,
+ ANTI_AFFINITY,
+ BACKLOG,
+ COMPACTION,
+ DELAYED,
+ DEDUPLICATION,
+ MAX_CONSUMERS,
+ MAX_PRODUCERS,
+ MAX_UNACKED,
+ OFFLOAD,
+ PERSISTENCE,
+ RATE,
+ RETENTION,
+ REPLICATION,
+ REPLICATION_RATE,
+ SCHEMA,
Review comment:
I think it should be SchemaCompatibilityStrategy ?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [pulsar] joefk commented on issue #6428: [Issue 5720][authorization
provider] (WIP) Add more granularity
Posted by GitBox <gi...@apache.org>.
joefk commented on issue #6428: [Issue 5720][authorization provider] (WIP) Add more granularity
URL: https://github.com/apache/pulsar/pull/6428#issuecomment-591780566
I see that there is an unload permission being granted to namespace owners? What is the rationale?
A few many unloads, and you ZK and cluster is dead.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [pulsar] zymap commented on issue #6428: [Issue 5720][authorization
provider] Add granularity
Posted by GitBox <gi...@apache.org>.
zymap commented on issue #6428: [Issue 5720][authorization provider] Add granularity
URL: https://github.com/apache/pulsar/pull/6428#issuecomment-610857088
/pulsarbot run-failure-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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [pulsar] KannarFr edited a comment on issue #6428: [Issue
5720][authorization provider] (WIP) Add more granularity
Posted by GitBox <gi...@apache.org>.
KannarFr edited a comment on issue #6428: [Issue 5720][authorization provider] (WIP) Add more granularity
URL: https://github.com/apache/pulsar/pull/6428#issuecomment-592485485
@joefk I think every permissions should be granted by authz provider. So we need to provide authz provider stuff to authorize or not, but it's not to the pulsar side to decide if access is grant or not. Maybe we can documents critical stuff as mentioned unload but that's it. I'm not working on the PulsarAuthorizationProvider which is the default used, but on the AuthorizationProvider interface and its usages.
Thats being said, I can be wrong, WDYT?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [pulsar] KannarFr commented on a change in pull request #6428:
[Issue 5720][authorization provider] Add granularity
Posted by GitBox <gi...@apache.org>.
KannarFr commented on a change in pull request #6428: [Issue 5720][authorization provider] Add granularity
URL: https://github.com/apache/pulsar/pull/6428#discussion_r406123372
##########
File path: pulsar-common/src/main/java/org/apache/pulsar/common/policies/data/NamespaceOperation.java
##########
@@ -0,0 +1,87 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.pulsar.common.policies.data;
+
+/**
+ * Namespace authorization operations.
+ */
+public enum NamespaceOperation {
+ CLEAR_BACKLOG,
+ DELETE_BUNDLE,
+ GET_ANTI_AFFINITY_GROUP,
+ GET_BACKLOG_QUOTAS,
+ GET_BUNDLES,
+ GET_COMPACTION_THRESHOLD,
+ GET_DELAYED_DELIVERY,
+ GET_DISPATCH_RATE,
+ GET_MAX_CONSUMERS_PER_SUBSCRIPTION,
+ GET_MAX_CONSUMERS_PER_TOPIC,
+ GET_MAX_PRODUCERS_PER_TOPIC,
+ GET_MAX_UNACKED_MESSAGES_PER_CONSUMER,
+ GET_MAX_UNACKED_MESSAGES_PER_SUBSCRIPTION,
+ GET_MESSAGE_TTL,
+ GET_OFFLOAD_DELETION_LAG,
+ GET_OFFLOAD_POLICIES,
+ GET_OFFLOAD_THRESHOLD,
+ GET_PERSISTENCE,
+ GET_PUBLISH_RATE,
+ GET_REPLICATION_CLUSTERS,
+ GET_REPLICATOR_DISPATCH_RATE,
+ GET_RETENTION,
+ GET_SCHEMA_AUTOUPDATE_STRATEGY,
+ GET_SCHEMA_COMPATIBILITY_STRATEGY,
+ GET_SCHEMA_VALIDATION_ENFORCED,
+ GET_SUBSCRIBE_RATE,
+ GET_SUBSCRIPTION_DISPATCH_RATE,
+ GRANT_PERMISSIONS,
+ GRANT_SUBSCRIPTION_PERMISSION,
+ REVOKE_PERMISSIONS,
+ REVOKE_SUBSCRIPTION_PERMISSION,
+ PERMISSIONS,
+ POLICIES,
Review comment:
Changed to GET_POLICIES.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [pulsar] sijie commented on a change in pull request #6428: [Issue
5720][authorization provider] Add granularity
Posted by GitBox <gi...@apache.org>.
sijie commented on a change in pull request #6428: [Issue 5720][authorization provider] Add granularity
URL: https://github.com/apache/pulsar/pull/6428#discussion_r406452036
##########
File path: pulsar-common/src/main/java/org/apache/pulsar/common/policies/data/TopicOperation.java
##########
@@ -0,0 +1,60 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.pulsar.common.policies.data;
+
+/**
+ * Topic authorization operations.
+ */
+public enum TopicOperation {
Review comment:
I would like to see we clean up these operations.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [pulsar] joefk edited a comment on issue #6428: [Issue
5720][authorization provider] (WIP) Add more granularity
Posted by GitBox <gi...@apache.org>.
joefk edited a comment on issue #6428: [Issue 5720][authorization provider] (WIP) Add more granularity
URL: https://github.com/apache/pulsar/pull/6428#issuecomment-592807888
@KannarF
> I think every permissions should be granted by authz provider.
Some of them are not grantable. How did your arrive at that conclusion? That is a very simplistic view of the system. After going through PIP-49 and the discussions about that, what is your take?
As, you state in the description of your PR..
>And the biggest part of the task: go through every usage of validateSuperUserAccess and validateAdminAccessForTenant and check if they can be replaced with finer grain access
Can you share your findings for that? I did not see any
My concern is this. How can I, as a Pulsar operator (NOT a tenant) decide which of these operations I will delegate to my tenants for finer control ?
Operators should not be put in a position where if they were to deploy this authz provider, they would have to go and turn off/on certain actions for all fine-grained resources every time someone creates a namespace or topic. And the things you are proposing to delegate here in this proposal are an operators nightmare for a large cluster. Hence my ask. Put the list of the things that can be delegated into a config file, so that operators can decide ONCE when they DEPLOY Pulsar, about what can be potentially delegated. Don't hardcode it into the server, and create runtime maintenance work for operators
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [pulsar] cckellogg commented on a change in pull request #6428:
[Issue 5720][authorization provider] (WIP) Add more granularity
Posted by GitBox <gi...@apache.org>.
cckellogg commented on a change in pull request #6428: [Issue 5720][authorization provider] (WIP) Add more granularity
URL: https://github.com/apache/pulsar/pull/6428#discussion_r398367362
##########
File path: pulsar-common/src/main/java/org/apache/pulsar/common/policies/data/NamespaceOperation.java
##########
@@ -0,0 +1,83 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.pulsar.common.policies.data;
+
+/**
+ * Namespace authorization operations.
+ */
+public enum NamespaceOperation {
Review comment:
I think we can simply these actions. I don't understand why an action is needed for every rest endpoint. For example, you can either update a namespace or not. You can either read a namespace or not. Why do you need fine grained control for updating/reading each field in a namespace? A simple Write permission can be applied to any action that changes a namespace.
In my opinion, having an action for every endpoint makes the code very difficult to change and it's hard to maintain. How will you depreciate actions and provide backward compatibility? What happens when you add a new field to a namespace policy? This could break exiting implementations and their permissions or force users to reconfigure authorization rules.
I see 2 basic actions needed to start for namespaces:
- Write
- Read
Same for the most of the other entities. We should start with a small set. These actions will become public api and will be very difficult to change or modify.
I disagree with the layout and operation mapping from PIP-49.
I agree with @sijie and think we should simplify the verbs and I think pulsar should move more towards a policy/role based system like nomad, consul, vault, and or kubernetes.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [pulsar] sijie commented on a change in pull request #6428: [Issue
5720][authorization provider] Add granularity
Posted by GitBox <gi...@apache.org>.
sijie commented on a change in pull request #6428: [Issue 5720][authorization provider] Add granularity
URL: https://github.com/apache/pulsar/pull/6428#discussion_r405966442
##########
File path: pulsar-common/src/main/java/org/apache/pulsar/common/policies/data/NamespaceOperation.java
##########
@@ -0,0 +1,87 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.pulsar.common.policies.data;
+
+/**
+ * Namespace authorization operations.
+ */
+public enum NamespaceOperation {
+ CLEAR_BACKLOG,
+ DELETE_BUNDLE,
+ GET_ANTI_AFFINITY_GROUP,
+ GET_BACKLOG_QUOTAS,
+ GET_BUNDLES,
+ GET_COMPACTION_THRESHOLD,
+ GET_DELAYED_DELIVERY,
+ GET_DISPATCH_RATE,
+ GET_MAX_CONSUMERS_PER_SUBSCRIPTION,
+ GET_MAX_CONSUMERS_PER_TOPIC,
+ GET_MAX_PRODUCERS_PER_TOPIC,
+ GET_MAX_UNACKED_MESSAGES_PER_CONSUMER,
+ GET_MAX_UNACKED_MESSAGES_PER_SUBSCRIPTION,
+ GET_MESSAGE_TTL,
+ GET_OFFLOAD_DELETION_LAG,
+ GET_OFFLOAD_POLICIES,
+ GET_OFFLOAD_THRESHOLD,
+ GET_PERSISTENCE,
+ GET_PUBLISH_RATE,
+ GET_REPLICATION_CLUSTERS,
+ GET_REPLICATOR_DISPATCH_RATE,
+ GET_RETENTION,
+ GET_SCHEMA_AUTOUPDATE_STRATEGY,
+ GET_SCHEMA_COMPATIBILITY_STRATEGY,
+ GET_SCHEMA_VALIDATION_ENFORCED,
+ GET_SUBSCRIBE_RATE,
+ GET_SUBSCRIPTION_DISPATCH_RATE,
+ GRANT_PERMISSIONS,
+ GRANT_SUBSCRIPTION_PERMISSION,
+ REVOKE_PERMISSIONS,
+ REVOKE_SUBSCRIPTION_PERMISSION,
+ PERMISSIONS,
+ POLICIES,
Review comment:
What does `POLICIES` operation mean here?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [pulsar] sijie commented on issue #6428: [Issue 5720][authorization
provider] (WIP) Add more granularity
Posted by GitBox <gi...@apache.org>.
sijie commented on issue #6428: [Issue 5720][authorization provider] (WIP) Add more granularity
URL: https://github.com/apache/pulsar/pull/6428#issuecomment-597394868
@KannarFr @joefk I think we should make sure this change here is more about adding the granularity to the authorization interface to have a better "role" based authorization model. but we shouldn't change the existing authorization model.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [pulsar] sijie commented on issue #6428: [Issue 5720][authorization
provider] (WIP) Add more granularity
Posted by GitBox <gi...@apache.org>.
sijie commented on issue #6428: [Issue 5720][authorization provider] (WIP) Add more granularity
URL: https://github.com/apache/pulsar/pull/6428#issuecomment-592996876
@KannarFr
Thank you for your contribution!
I think we should have a clear interface about resources and verbs. I feel that you are mixing resources with verbs and generating a lot of "unneeded" operations.
For example, each policy rule in a namespace policy should be treated as a resource. The operations to a given policy rule are `write` and `read`. So when you introduce a new policy rule, you don't need to introduce a new verb. Also, I don't think enum is a good way to allow extensibility. I think we can use a `string` for representing different policy rules within a namespace policy.
Kubernetes' API machinery provides a good example of this.
---
@joefk
I think the pull request here is to allow people to define their own authorization implementation since some organizations have the need to integrate Pulsar into its owner authentication/authorization system to control those resources. The purpose of this pull request is different from PIP-49. We attempt to improve the current authorization model. This pull request should NOT change our current authorization model as the concerns have been raised when discussing PIP-49.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [pulsar] sijie commented on a change in pull request #6428: [Issue
5720][authorization provider] Add granularity
Posted by GitBox <gi...@apache.org>.
sijie commented on a change in pull request #6428: [Issue 5720][authorization provider] Add granularity
URL: https://github.com/apache/pulsar/pull/6428#discussion_r405967099
##########
File path: pulsar-common/src/main/java/org/apache/pulsar/common/policies/data/TopicOperation.java
##########
@@ -0,0 +1,60 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.pulsar.common.policies.data;
+
+/**
+ * Topic authorization operations.
+ */
+public enum TopicOperation {
Review comment:
I think TopicOperation should be broken down into topic related operations and subscription-related operations. And we should reduce the number of `verbs` here.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [pulsar] zymap commented on issue #6428: [Issue 5720][authorization
provider] Add granularity
Posted by GitBox <gi...@apache.org>.
zymap commented on issue #6428: [Issue 5720][authorization provider] Add granularity
URL: https://github.com/apache/pulsar/pull/6428#issuecomment-610718159
@codelipenghui @jiazhai @rdhabalia @merlimat @sijie Please take a look. Thanks.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [pulsar] sijie commented on a change in pull request #6428: [Issue
5720][authorization provider] Add granularity
Posted by GitBox <gi...@apache.org>.
sijie commented on a change in pull request #6428: [Issue 5720][authorization provider] Add granularity
URL: https://github.com/apache/pulsar/pull/6428#discussion_r405966650
##########
File path: pulsar-common/src/main/java/org/apache/pulsar/common/policies/data/NamespaceOperation.java
##########
@@ -0,0 +1,87 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.pulsar.common.policies.data;
+
+/**
+ * Namespace authorization operations.
+ */
+public enum NamespaceOperation {
+ CLEAR_BACKLOG,
+ DELETE_BUNDLE,
+ GET_ANTI_AFFINITY_GROUP,
+ GET_BACKLOG_QUOTAS,
+ GET_BUNDLES,
+ GET_COMPACTION_THRESHOLD,
+ GET_DELAYED_DELIVERY,
+ GET_DISPATCH_RATE,
+ GET_MAX_CONSUMERS_PER_SUBSCRIPTION,
+ GET_MAX_CONSUMERS_PER_TOPIC,
+ GET_MAX_PRODUCERS_PER_TOPIC,
+ GET_MAX_UNACKED_MESSAGES_PER_CONSUMER,
+ GET_MAX_UNACKED_MESSAGES_PER_SUBSCRIPTION,
+ GET_MESSAGE_TTL,
+ GET_OFFLOAD_DELETION_LAG,
+ GET_OFFLOAD_POLICIES,
+ GET_OFFLOAD_THRESHOLD,
+ GET_PERSISTENCE,
+ GET_PUBLISH_RATE,
+ GET_REPLICATION_CLUSTERS,
+ GET_REPLICATOR_DISPATCH_RATE,
+ GET_RETENTION,
+ GET_SCHEMA_AUTOUPDATE_STRATEGY,
+ GET_SCHEMA_COMPATIBILITY_STRATEGY,
+ GET_SCHEMA_VALIDATION_ENFORCED,
+ GET_SUBSCRIBE_RATE,
+ GET_SUBSCRIPTION_DISPATCH_RATE,
+ GRANT_PERMISSIONS,
+ GRANT_SUBSCRIPTION_PERMISSION,
+ REVOKE_PERMISSIONS,
+ REVOKE_SUBSCRIPTION_PERMISSION,
+ PERMISSIONS,
+ POLICIES,
+ SCHEMA_AUTOUPDATE,
+ SET_BACKLOG_QUOTA,
+ SET_ANTI_AFFINITY_GROUP,
+ SET_COMPACTION_THRESHOLD,
+ SET_DEDUPLICATION,
+ SET_DELAYED_DELIVERY,
+ SET_ENCRYPTION_REQUIRED,
+ SET_MAX_CONSUMERS_PER_SUBSCRIPTION,
+ SET_MAX_CONSUMERS_PER_TOPIC,
+ SET_MAX_PRODUCERS_PER_TOPIC,
+ SET_MAX_UNACKED_MESSAGES_PER_CONSUMER,
+ SET_MAX_UNACKED_MESSAGES_PER_SUBSCRIPTION,
+ SET_MESSAGE_TTL,
+ SET_OFFLOAD_DELETION_LAG,
+ SET_OFFLOAD_POLICIES,
+ SET_OFFLOAD_THRESHOLD,
+ SET_PERSISTENCE,
+ SET_REPLICATION_CLUSTERS,
+ SET_REPLICATOR_DISPATCH_RATE,
+ SET_RETENTION,
+ SET_SCHEMA_AUTOUPDATE_STRATEGY,
+ SET_SCHEMA_COMPATIBILITY_STRATEGY,
+ SET_SCHEMA_VALIDATION_ENFORCED,
+ SET_SUBSCRIBE_RATE,
+ SET_SUBSCRIPTION_AUTH_MODE,
+ SET_SUBSCRIPTION_DISPATCH_RATE,
+ TOPICS,
Review comment:
What does `TOPICS` mean here? I think it should be prefixed with a verb.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [pulsar] KannarFr commented on a change in pull request #6428:
[Issue 5720][authorization provider] Add granularity
Posted by GitBox <gi...@apache.org>.
KannarFr commented on a change in pull request #6428: [Issue 5720][authorization provider] Add granularity
URL: https://github.com/apache/pulsar/pull/6428#discussion_r406123264
##########
File path: pulsar-common/src/main/java/org/apache/pulsar/common/policies/data/NamespaceOperation.java
##########
@@ -0,0 +1,87 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.pulsar.common.policies.data;
+
+/**
+ * Namespace authorization operations.
+ */
+public enum NamespaceOperation {
+ CLEAR_BACKLOG,
+ DELETE_BUNDLE,
+ GET_ANTI_AFFINITY_GROUP,
+ GET_BACKLOG_QUOTAS,
+ GET_BUNDLES,
+ GET_COMPACTION_THRESHOLD,
+ GET_DELAYED_DELIVERY,
+ GET_DISPATCH_RATE,
+ GET_MAX_CONSUMERS_PER_SUBSCRIPTION,
+ GET_MAX_CONSUMERS_PER_TOPIC,
+ GET_MAX_PRODUCERS_PER_TOPIC,
+ GET_MAX_UNACKED_MESSAGES_PER_CONSUMER,
+ GET_MAX_UNACKED_MESSAGES_PER_SUBSCRIPTION,
+ GET_MESSAGE_TTL,
+ GET_OFFLOAD_DELETION_LAG,
+ GET_OFFLOAD_POLICIES,
+ GET_OFFLOAD_THRESHOLD,
+ GET_PERSISTENCE,
+ GET_PUBLISH_RATE,
+ GET_REPLICATION_CLUSTERS,
+ GET_REPLICATOR_DISPATCH_RATE,
+ GET_RETENTION,
+ GET_SCHEMA_AUTOUPDATE_STRATEGY,
+ GET_SCHEMA_COMPATIBILITY_STRATEGY,
+ GET_SCHEMA_VALIDATION_ENFORCED,
+ GET_SUBSCRIBE_RATE,
+ GET_SUBSCRIPTION_DISPATCH_RATE,
+ GRANT_PERMISSIONS,
+ GRANT_SUBSCRIPTION_PERMISSION,
+ REVOKE_PERMISSIONS,
+ REVOKE_SUBSCRIPTION_PERMISSION,
+ PERMISSIONS,
+ POLICIES,
+ SCHEMA_AUTOUPDATE,
+ SET_BACKLOG_QUOTA,
+ SET_ANTI_AFFINITY_GROUP,
+ SET_COMPACTION_THRESHOLD,
+ SET_DEDUPLICATION,
+ SET_DELAYED_DELIVERY,
+ SET_ENCRYPTION_REQUIRED,
+ SET_MAX_CONSUMERS_PER_SUBSCRIPTION,
+ SET_MAX_CONSUMERS_PER_TOPIC,
+ SET_MAX_PRODUCERS_PER_TOPIC,
+ SET_MAX_UNACKED_MESSAGES_PER_CONSUMER,
+ SET_MAX_UNACKED_MESSAGES_PER_SUBSCRIPTION,
+ SET_MESSAGE_TTL,
+ SET_OFFLOAD_DELETION_LAG,
+ SET_OFFLOAD_POLICIES,
+ SET_OFFLOAD_THRESHOLD,
+ SET_PERSISTENCE,
+ SET_REPLICATION_CLUSTERS,
+ SET_REPLICATOR_DISPATCH_RATE,
+ SET_RETENTION,
+ SET_SCHEMA_AUTOUPDATE_STRATEGY,
+ SET_SCHEMA_COMPATIBILITY_STRATEGY,
+ SET_SCHEMA_VALIDATION_ENFORCED,
+ SET_SUBSCRIBE_RATE,
+ SET_SUBSCRIPTION_AUTH_MODE,
+ SET_SUBSCRIPTION_DISPATCH_RATE,
+ TOPICS,
Review comment:
Changed to LIST_TOPICS.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [pulsar] KannarFr commented on a change in pull request #6428:
[Issue 5720][authorization provider] Add granularity
Posted by GitBox <gi...@apache.org>.
KannarFr commented on a change in pull request #6428: [Issue 5720][authorization provider] Add granularity
URL: https://github.com/apache/pulsar/pull/6428#discussion_r406041208
##########
File path: pulsar-common/src/main/java/org/apache/pulsar/common/policies/data/TopicOperation.java
##########
@@ -0,0 +1,60 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.pulsar.common.policies.data;
+
+/**
+ * Topic authorization operations.
+ */
+public enum TopicOperation {
Review comment:
Removed, topics new authz usage will comes on another PR.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [pulsar] sijie commented on a change in pull request #6428: [Issue
5720][authorization provider] Add granularity
Posted by GitBox <gi...@apache.org>.
sijie commented on a change in pull request #6428: [Issue 5720][authorization provider] Add granularity
URL: https://github.com/apache/pulsar/pull/6428#discussion_r406450520
##########
File path: pulsar-common/src/main/java/org/apache/pulsar/common/policies/data/PolicyName.java
##########
@@ -0,0 +1,44 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.pulsar.common.policies.data;
+
+/**
+ * PolicyName authorization operations.
+ */
+public enum PolicyName {
+ ALL,
+ ANTI_AFFINITY,
+ BACKLOG,
+ COMPACTION,
+ DELAYED,
Review comment:
`DELAYED` is a very ambiguous name. I think we should call it `DELAYED_DELIVERY`.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [pulsar] KannarFr commented on a change in pull request #6428:
[Issue 5720][authorization provider] Add granularity
Posted by GitBox <gi...@apache.org>.
KannarFr commented on a change in pull request #6428: [Issue 5720][authorization provider] Add granularity
URL: https://github.com/apache/pulsar/pull/6428#discussion_r406038154
##########
File path: pulsar-common/src/main/java/org/apache/pulsar/common/policies/data/BrokerOperation.java
##########
@@ -0,0 +1,35 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.pulsar.common.policies.data;
+
+/**
+ * Broker authorization operations.
+ */
+public enum BrokerOperation {
+ DELETE_DYNAMIC_CONFIG,
Review comment:
Removed the enum, Broker will still uses isSuperUser.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [pulsar] sijie commented on a change in pull request #6428: [Issue
5720][authorization provider] Add granularity
Posted by GitBox <gi...@apache.org>.
sijie commented on a change in pull request #6428: [Issue 5720][authorization provider] Add granularity
URL: https://github.com/apache/pulsar/pull/6428#discussion_r406449265
##########
File path: pulsar-common/src/main/java/org/apache/pulsar/common/policies/data/NamespaceOperation.java
##########
@@ -0,0 +1,32 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.pulsar.common.policies.data;
+
+/**
+ * Namespace authorization operations.
+ */
+public enum NamespaceOperation {
+ AUTH_MODE,
+ BACKLOG,
+ BUNDLE,
Review comment:
I think NamespaceBundle is a resource not a verb.
The verb (operation) to the namespace bundle is:
- get: get the bundle data
- delete: delete a bundle
- add: add/split a bundle.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [pulsar] sijie commented on a change in pull request #6428: [Issue
5720][authorization provider] Add granularity
Posted by GitBox <gi...@apache.org>.
sijie commented on a change in pull request #6428: [Issue 5720][authorization provider] Add granularity
URL: https://github.com/apache/pulsar/pull/6428#discussion_r405963131
##########
File path: pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authorization/AuthorizationProvider.java
##########
@@ -173,4 +179,92 @@
CompletableFuture<Void> grantPermissionAsync(TopicName topicName, Set<AuthAction> actions, String role,
String authDataJson);
+ /**
+ * Grant authorization-action permission on a tenant to the given client
+ * @param tenantName
Review comment:
Since this is a public interface, I would suggest adding documentation on these fields. So that other people know how to implement this interface.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [pulsar] sijie commented on a change in pull request #6428: [Issue
5720][authorization provider] Add granularity
Posted by GitBox <gi...@apache.org>.
sijie commented on a change in pull request #6428: [Issue 5720][authorization provider] Add granularity
URL: https://github.com/apache/pulsar/pull/6428#discussion_r406451055
##########
File path: pulsar-common/src/main/java/org/apache/pulsar/common/policies/data/PolicyName.java
##########
@@ -0,0 +1,44 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.pulsar.common.policies.data;
+
+/**
+ * PolicyName authorization operations.
+ */
+public enum PolicyName {
+ ALL,
+ ANTI_AFFINITY,
+ BACKLOG,
+ COMPACTION,
+ DELAYED,
+ DEDUPLICATION,
+ MAX_CONSUMERS,
+ MAX_PRODUCERS,
+ MAX_UNACKED,
+ OFFLOAD,
+ PERSISTENCE,
+ RATE,
Review comment:
Should we distinguish publish rate, subscribe rate and dispatch rate here?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [pulsar] joefk edited a comment on issue #6428: [Issue
5720][authorization provider] (WIP) Add more granularity
Posted by GitBox <gi...@apache.org>.
joefk edited a comment on issue #6428: [Issue 5720][authorization provider] (WIP) Add more granularity
URL: https://github.com/apache/pulsar/pull/6428#issuecomment-591780566
I see that there is an unload permission being granted to namespace owners? What is the rationale?
A few many unloads, and you ZK and cluster is dead.
There is a reason these operations are restricted. So unless you are willing to introduce and implement adequate defense measures into Pulsar, these are all opening DOS and vulnerabilities into the system.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [pulsar] joefk commented on issue #6428: [Issue 5720][authorization
provider] (WIP) Add more granularity
Posted by GitBox <gi...@apache.org>.
joefk commented on issue #6428: [Issue 5720][authorization provider] (WIP) Add more granularity
URL: https://github.com/apache/pulsar/pull/6428#issuecomment-592200705
@KannarFr you should get familiar with the PIP-49 and the discussion on the dev list about it. Just because unload operates on a namespace, it does not imply that it should ever be executed by a namespace owner.
FWIW, I don't think it's feasible to implement an adequate restriction on unload in a simple way, because it's a system management operation, driven by system management needs.
At the least, these kind of shoot-yourself features should not be hard-coded into the broker as a possible option. It should be in a config file.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [pulsar] KannarFr commented on issue #6428: [Issue
5720][authorization provider] (WIP) Add more granularity
Posted by GitBox <gi...@apache.org>.
KannarFr commented on issue #6428: [Issue 5720][authorization provider] (WIP) Add more granularity
URL: https://github.com/apache/pulsar/pull/6428#issuecomment-591973127
@joefk I'm open to all discussion I don't know correctly this part. Can you explain why unloads are critical for Pulsar? We can imagine that only one unload is authorized at the same time.
The main goal of this issue is to provide real authz provider.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [pulsar] KannarFr commented on a change in pull request #6428:
[Issue 5720][authorization provider] (WIP) Add more granularity
Posted by GitBox <gi...@apache.org>.
KannarFr commented on a change in pull request #6428: [Issue 5720][authorization provider] (WIP) Add more granularity
URL: https://github.com/apache/pulsar/pull/6428#discussion_r398546669
##########
File path: pulsar-common/src/main/java/org/apache/pulsar/common/policies/data/NamespaceOperation.java
##########
@@ -0,0 +1,83 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.pulsar.common.policies.data;
+
+/**
+ * Namespace authorization operations.
+ */
+public enum NamespaceOperation {
Review comment:
Just rereading my words, and the example I gave is read/write as you mentioned, my bad. I just don't want to limit authz possibilities. And I'm not sure that Read/write cover all cases.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [pulsar] jiazhai commented on issue #6428: [Issue
5720][authorization provider] Add granularity
Posted by GitBox <gi...@apache.org>.
jiazhai commented on issue #6428: [Issue 5720][authorization provider] Add granularity
URL: https://github.com/apache/pulsar/pull/6428#issuecomment-611250785
/pulsarbot run-failure-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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [pulsar] KannarFr commented on issue #6428: [Issue
5720][authorization provider] Add granularity
Posted by GitBox <gi...@apache.org>.
KannarFr commented on issue #6428: [Issue 5720][authorization provider] Add granularity
URL: https://github.com/apache/pulsar/pull/6428#issuecomment-616138914
@sijie can you review?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [pulsar] KannarFr commented on a change in pull request #6428:
[Issue 5720][authorization provider] Add granularity
Posted by GitBox <gi...@apache.org>.
KannarFr commented on a change in pull request #6428: [Issue 5720][authorization provider] Add granularity
URL: https://github.com/apache/pulsar/pull/6428#discussion_r406671899
##########
File path: pulsar-common/src/main/java/org/apache/pulsar/common/policies/data/TenantOperation.java
##########
@@ -0,0 +1,29 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.pulsar.common.policies.data;
+
+/**
+ * Tenant authorization operations.
+ */
+public enum TenantOperation {
Review comment:
I think it's superUser stuff.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [pulsar] KannarFr edited a comment on issue #6428: [Issue
5720][authorization provider] (WIP) Add more granularity
Posted by GitBox <gi...@apache.org>.
KannarFr edited a comment on issue #6428: [Issue 5720][authorization provider] (WIP) Add more granularity
URL: https://github.com/apache/pulsar/pull/6428#issuecomment-592485485
@joefk I think every permissions should be enabled by authz provider. So we need to provide authz provider stuff to authorize or not, but it's not to the pulsar side to decide if access is grant or not. Maybe we can documents critical stuff as mentioned unload but that's it. I'm not working on the PulsarAuthorizationProvider which is the default used, but on the AuthorizationProvider interface and its usages.
Thats being said, I can be wrong, WDYT?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [pulsar] KannarFr commented on a change in pull request #6428:
[Issue 5720][authorization provider] Add granularity
Posted by GitBox <gi...@apache.org>.
KannarFr commented on a change in pull request #6428: [Issue 5720][authorization provider] Add granularity
URL: https://github.com/apache/pulsar/pull/6428#discussion_r406675260
##########
File path: pulsar-common/src/main/java/org/apache/pulsar/common/policies/data/PolicyName.java
##########
@@ -0,0 +1,44 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.pulsar.common.policies.data;
+
+/**
+ * PolicyName authorization operations.
+ */
+public enum PolicyName {
+ ALL,
+ ANTI_AFFINITY,
+ BACKLOG,
+ COMPACTION,
+ DELAYED,
+ DEDUPLICATION,
+ MAX_CONSUMERS,
+ MAX_PRODUCERS,
+ MAX_UNACKED,
+ OFFLOAD,
+ PERSISTENCE,
+ RATE,
Review comment:
I think we can merge them together.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [pulsar] sijie commented on a change in pull request #6428: [Issue
5720][authorization provider] Add granularity
Posted by GitBox <gi...@apache.org>.
sijie commented on a change in pull request #6428: [Issue 5720][authorization provider] Add granularity
URL: https://github.com/apache/pulsar/pull/6428#discussion_r406451872
##########
File path: pulsar-common/src/main/java/org/apache/pulsar/common/policies/data/TenantOperation.java
##########
@@ -0,0 +1,29 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.pulsar.common.policies.data;
+
+/**
+ * Tenant authorization operations.
+ */
+public enum TenantOperation {
Review comment:
Do we need to include the operations of adding and removing tenant admin?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [pulsar] KannarFr commented on issue #6428: [Issue
5720][authorization provider] (WIP) Add more granularity
Posted by GitBox <gi...@apache.org>.
KannarFr commented on issue #6428: [Issue 5720][authorization provider] (WIP) Add more granularity
URL: https://github.com/apache/pulsar/pull/6428#issuecomment-592921663
Hi,
Here we are defining an authz provider interface not the authz provider. We
want to provide the way to authz every cases as multitenancy stuff. In my
case all authorized info will be defined in the role String which will be a
token containing authorized information. Like this token has produce
operation authorized on this namespace or this topic.
In the case you are illustrating we just want that the PulsarAuthzProvider
(which is the default for now) handles some default cases as you mentioned.
And sure we could define default bindings in configuration file.
This stuff will come in other PR (I guess), currently I just want to impl
the granularity possibilities without breaking changes for current default
PulsarAuthzProvider.
Le sam. 29 févr. 2020 à 02:44, joefk <no...@github.com> a écrit :
> @KannarF
>
> I think every permissions should be granted by authz provider.
> Some of them are not grantable. How did your arrive at that conclusion?
> That is a very simplistic view of the system. After going through PIP-49
> and the discussions about that, what is your take?
>
> As, you state in the description of your PR..
>
> And the biggest part of the task: go through every usage of
> validateSuperUserAccess and validateAdminAccessForTenant and check if they
> can be replaced with finer grain access
> Can you share your findings for that? I did not see any
>
> My concern is this. How can I, as a Pulsar operator (NOT a tenant) decide
> which of these operations I will delegate to my tenants for finer control ?
>
> Operators should not be put in a position where if they were to deploy
> this authz provider, they would have to go and turn off/on certain actions
> for all fine-grained resources every time someone creates a namespace or
> topic. And the things you are proposing to delegate here in this proposal
> are an operators nightmare for a large cluster. Hence my ask. Put the list
> of the things that can be delegated into a config file, so that operators
> can decide ONCE when they DEPLOY Pulsar, about what can be potentially
> delegated. Don't hardcode it into the server, and create runtime
> maintenance work for operators
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> <https://github.com/apache/pulsar/pull/6428?email_source=notifications&email_token=ABZLQJMY5MQQXPQAXPIWBH3RFG4YLA5CNFSM4K4ES5QKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOENKYPUA#issuecomment-592807888>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/ABZLQJKYUH3RYTKDCFSZTUTRFG4YLANCNFSM4K4ES5QA>
> .
>
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [pulsar] sijie commented on a change in pull request #6428: [Issue
5720][authorization provider] Add granularity
Posted by GitBox <gi...@apache.org>.
sijie commented on a change in pull request #6428: [Issue 5720][authorization provider] Add granularity
URL: https://github.com/apache/pulsar/pull/6428#discussion_r405966181
##########
File path: pulsar-common/src/main/java/org/apache/pulsar/common/policies/data/NamespaceOperation.java
##########
@@ -0,0 +1,83 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.pulsar.common.policies.data;
+
+/**
+ * Namespace authorization operations.
+ */
+public enum NamespaceOperation {
Review comment:
I think this mixes the operations on a namespace and the operations on a namespace policy. I would suggest separating them.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [pulsar] KannarFr commented on issue #6428: [Issue
5720][authorization provider] (WIP) Add more granularity
Posted by GitBox <gi...@apache.org>.
KannarFr commented on issue #6428: [Issue 5720][authorization provider] (WIP) Add more granularity
URL: https://github.com/apache/pulsar/pull/6428#issuecomment-592485485
@joefk I think every permissions should be enabled by authz provider. So we need to provide authz provider stuff to authorize or not, but it's not to the pulsar side to decide if access is grant or not. Maybe we can documents critical stuff as mentioned unload but that's it. I'm not working on the PulsarAuthorizationProvider which is the default used, but on the AuthorizationProvider interface and its usages.
Thats being said, I can be wrong WDYT?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [pulsar] KannarFr commented on a change in pull request #6428:
[Issue 5720][authorization provider] (WIP) Add more granularity
Posted by GitBox <gi...@apache.org>.
KannarFr commented on a change in pull request #6428: [Issue 5720][authorization provider] (WIP) Add more granularity
URL: https://github.com/apache/pulsar/pull/6428#discussion_r389689932
##########
File path: pulsar-common/src/main/java/org/apache/pulsar/common/policies/data/NamespaceOperation.java
##########
@@ -0,0 +1,83 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.pulsar.common.policies.data;
+
+/**
+ * Namespace authorization operations.
+ */
+public enum NamespaceOperation {
Review comment:
I added these operations from PIP-49 (https://github.com/apache/pulsar/wiki/PIP-49%3A-Permission-levels-and-inheritance), but I'm currently reworking all. As you can see (for example), `TenantOperation` has `DELETE_NAMESPACES` which is not in `NamespaceOperation`. Any resource operation should be managed/authorized by the resource which owns it.
`ClusterOperations` should manage `Tenants`, `TenantOperation` should manage `Namespaces`, etc. Hierarchical approach.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [pulsar] KannarFr commented on issue #6428: [Issue
5720][authorization provider] (WIP) Add more granularity
Posted by GitBox <gi...@apache.org>.
KannarFr commented on issue #6428: [Issue 5720][authorization provider] (WIP) Add more granularity
URL: https://github.com/apache/pulsar/pull/6428#issuecomment-596539275
@sijie operations like `internalGrantPermissionOnNamespace` in `NamespacesBase.java` are antipattern with the reworking I'm doing. I don't know how to manage them. I think I should let them as they are. Due to "real" authz plugin we should drop all roles/actions stuff which are in ZK. DefaultAuthzProvider should be updated regarding this.
That being said, we need maintain legacy stuff so I would like to have your opinions about this.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [pulsar] KannarFr commented on a change in pull request #6428:
[Issue 5720][authorization provider] Add granularity
Posted by GitBox <gi...@apache.org>.
KannarFr commented on a change in pull request #6428: [Issue 5720][authorization provider] Add granularity
URL: https://github.com/apache/pulsar/pull/6428#discussion_r406041208
##########
File path: pulsar-common/src/main/java/org/apache/pulsar/common/policies/data/TopicOperation.java
##########
@@ -0,0 +1,60 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.pulsar.common.policies.data;
+
+/**
+ * Topic authorization operations.
+ */
+public enum TopicOperation {
Review comment:
Removed, topics new authz usage will comes on another PR.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [pulsar] jiazhai commented on issue #6428: [Issue
5720][authorization provider] Add granularity
Posted by GitBox <gi...@apache.org>.
jiazhai commented on issue #6428: [Issue 5720][authorization provider] Add granularity
URL: https://github.com/apache/pulsar/pull/6428#issuecomment-611281619
/pulsarbot run-failure-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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [pulsar] cckellogg commented on a change in pull request #6428:
[Issue 5720][authorization provider] (WIP) Add more granularity
Posted by GitBox <gi...@apache.org>.
cckellogg commented on a change in pull request #6428: [Issue 5720][authorization provider] (WIP) Add more granularity
URL: https://github.com/apache/pulsar/pull/6428#discussion_r385937991
##########
File path: pulsar-common/src/main/java/org/apache/pulsar/common/policies/data/NamespaceOperation.java
##########
@@ -0,0 +1,83 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.pulsar.common.policies.data;
+
+/**
+ * Namespace authorization operations.
+ */
+public enum NamespaceOperation {
Review comment:
In my opinion we should reduce these operations otherwise things will get out of hand. Any SET_ could just be Update or Write. Any GET_ could just be Read or Get. Same for the others below. My concern is things will get out of hand and these lists will be very difficult to maintain. Is there a way to group the operations to reduce the number? These are just needed for authorization right?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [pulsar] KannarFr commented on issue #6428: [Issue
5720][authorization provider] Add granularity
Posted by GitBox <gi...@apache.org>.
KannarFr commented on issue #6428: [Issue 5720][authorization provider] Add granularity
URL: https://github.com/apache/pulsar/pull/6428#issuecomment-610335219
All cleaned, waiting for CI outputs, but Namespaces & ServerCnx are ready.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [pulsar] sijie commented on a change in pull request #6428: [Issue
5720][authorization provider] Add granularity
Posted by GitBox <gi...@apache.org>.
sijie commented on a change in pull request #6428: [Issue 5720][authorization provider] Add granularity
URL: https://github.com/apache/pulsar/pull/6428#discussion_r405965284
##########
File path: pulsar-common/src/main/java/org/apache/pulsar/common/policies/data/BrokerOperation.java
##########
@@ -0,0 +1,35 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.pulsar.common.policies.data;
+
+/**
+ * Broker authorization operations.
+ */
+public enum BrokerOperation {
+ DELETE_DYNAMIC_CONFIG,
Review comment:
DynamicConfig is a cluster-wide resource. I think it should be a separate API.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services