lhotari commented on code in PR #15576:
URL: https://github.com/apache/pulsar/pull/15576#discussion_r872088658
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/v1/Namespaces.java:
##########
@@ -314,6 +314,49 @@ public void
revokePermissionOnSubscription(@PathParam("property") String propert
internalRevokePermissionsOnSubscription(subscription, role);
}
+ @PUT
+ @Path("/{property}/{cluster}/{namespace}/implicitPermissionOnSubscription")
+ @ApiOperation(hidden = true, value = "Allow a consumer's role to have
implicit permission to consume from a"
+ + " subscription.")
+ @ApiResponses(value = {@ApiResponse(code = 403, message = "Don't have
admin permission"),
+ @ApiResponse(code = 404, message = "Property or cluster or
namespace doesn't exist"),
+ @ApiResponse(code = 409, message = "Concurrent modification"),
+ @ApiResponse(code = 501, message = "Authorization is not
enabled")})
+ public void grantImplicitPermissionOnSubscription(
+ @PathParam("property") String property, @PathParam("cluster")
String cluster,
+ @PathParam("namespace") String namespace) {
+ validateNamespaceName(property, cluster, namespace);
+ internalSetImplicitPermissionOnSubscription(true);
+ }
+
+ @DELETE
+ @Path("/{property}/{cluster}/{namespace}/implicitPermissionOnSubscription")
+ @ApiOperation(hidden = true, value = "Require a consumer's role to have
explicit permission to consume from a"
+ + " subscription.")
+ @ApiResponses(value = {@ApiResponse(code = 403, message = "Don't have
admin permission"),
+ @ApiResponse(code = 404, message = "Property or cluster or
namespace doesn't exist"),
+ @ApiResponse(code = 409, message = "Concurrent modification"),
+ @ApiResponse(code = 501, message = "Authorization is not
enabled")})
+ public void revokeImplicitPermissionOnSubscription(
+ @PathParam("property") String property, @PathParam("cluster")
String cluster,
+ @PathParam("namespace") String namespace) {
+ validateNamespaceName(property, cluster, namespace);
+ internalSetImplicitPermissionOnSubscription(false);
+ }
+
+ @GET
+ @Path("/{property}/{cluster}/{namespace}/implicitPermissionOnSubscription")
+ @ApiOperation(value = "Get permission on subscription required for
namespace.")
+ @ApiResponses(value = {@ApiResponse(code = 403, message = "Don't have
admin permission"),
+ @ApiResponse(code = 404, message = "Property or cluster or
namespace doesn't exist"),
+ @ApiResponse(code = 409, message = "Namespace is not empty")})
+ public boolean getImplicitPermissionOnSubscription(@PathParam("property")
String property,
+ @PathParam("cluster")
String cluster,
+ @PathParam("namespace")
String namespace) {
+ validateNamespaceName(property, cluster, namespace);
+ return getImplicitPermissionOnSubscription();
+ }
+
Review Comment:
Does this have to be added to the v1 API at all?
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/v2/Namespaces.java:
##########
@@ -267,6 +267,48 @@ public void
revokePermissionOnSubscription(@PathParam("property") String propert
internalRevokePermissionsOnSubscription(subscription, role);
}
+ @PUT
Review Comment:
```suggestion
@POST
```
Other API methods to grant permissions are using POST.
##########
pulsar-common/src/main/java/org/apache/pulsar/client/admin/internal/data/AuthPoliciesImpl.java:
##########
@@ -42,6 +42,10 @@ public final class AuthPoliciesImpl implements AuthPolicies {
@JsonProperty("subscription_auth_roles")
private Map<String, Set<String>> subscriptionAuthentication = new
TreeMap<>();
+ // Default value is set in the builder
+ @JsonProperty(value = "implicit_subscription_auth")
+ private boolean implicitSubscriptionAuth;
Review Comment:
Good points Enrico. Yes this should be addressed.
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/NamespacesBase.java:
##########
@@ -698,6 +698,24 @@ protected void internalGrantPermissionOnNamespace(String
role, Set<AuthAction> a
}
}
+ protected boolean getImplicitPermissionOnSubscription() {
+ validateNamespaceOperation(namespaceName,
NamespaceOperation.GET_PERMISSION);
+ Policies policies = getNamespacePolicies(namespaceName);
+ return policies.auth_policies.isImplicitSubscriptionAuth();
+ }
+
+ protected void internalSetImplicitPermissionOnSubscription(boolean
isImplicitPermissionOnSubscription) {
+ if (isImplicitPermissionOnSubscription) {
+ validateNamespaceOperation(namespaceName,
NamespaceOperation.GRANT_PERMISSION);
+ } else {
+ validateNamespaceOperation(namespaceName,
NamespaceOperation.REVOKE_PERMISSION);
+ }
+ validatePoliciesReadOnlyAccess();
+ updatePolicies(namespaceName, policies -> {
+
policies.auth_policies.setImplicitSubscriptionAuth(isImplicitPermissionOnSubscription);
+ return policies;
+ });
+ }
Review Comment:
It might be worth considering PIP-149/#14365 here and use the async style to
implement the API.
To clarify it: the server side implementation should by async, but the
client API can contain both sync and async API methods.
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/v2/Namespaces.java:
##########
@@ -267,6 +267,48 @@ public void
revokePermissionOnSubscription(@PathParam("property") String propert
internalRevokePermissionsOnSubscription(subscription, role);
}
+ @PUT
+ @Path("/{property}/{namespace}/implicitPermissionOnSubscription")
+ @ApiOperation(hidden = true, value = "Allow a consumer's role to have
implicit permission to consume from a"
+ + " subscription.")
+ @ApiResponses(value = {@ApiResponse(code = 403, message = "Don't have
admin permission"),
+ @ApiResponse(code = 404, message = "Property or cluster or
namespace doesn't exist"),
+ @ApiResponse(code = 409, message = "Concurrent modification"),
+ @ApiResponse(code = 501, message = "Authorization is not
enabled")})
+ public void grantImplicitPermissionOnSubscription(
+ @PathParam("property") String property,
+ @PathParam("namespace") String namespace) {
+ validateNamespaceName(property, namespace);
+ internalSetImplicitPermissionOnSubscription(true);
Review Comment:
It might be worth considering PIP-149/#14365 here and use the async style to
implement the API.
To clarify it: the server side implementation should by async, but the
client API can contain both sync and async API methods.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
