Dataplane Roles API#1750
Conversation
| dataplanev1alpha2connect.RegisterUserServiceHandlerGatewayServer(gwMux, userSvcV1alpha2, connectgateway.WithInterceptors(hookOutput.Interceptors...)) | ||
| dataplanev1alpha2connect.RegisterTransformServiceHandlerGatewayServer(gwMux, transformSvcV1alpha2, connectgateway.WithInterceptors(hookOutput.Interceptors...)) | ||
| dataplanev1alpha2connect.RegisterKafkaConnectServiceHandlerGatewayServer(gwMux, kafkaConnectSvcV1alpha2, connectgateway.WithInterceptors(hookOutput.Interceptors...)) | ||
| dataplanev1alpha2connect.RegisterCloudStorageServiceHandlerGatewayServer(gwMux, cloudStorageSvcV1Alpha2, connectgateway.WithInterceptors(hookOutput.Interceptors...)) |
There was a problem hiding this comment.
This seems like a bug and omission where never registered cloud storage API with gateway mux?
| rpc ListRoleMembers(ListRoleMembersRequest) returns (ListRoleMembersResponse) { | ||
| option (redpanda.api.auth.v1.authorization) = { | ||
| required_permission: PERMISSION_ADMIN | ||
| required_permission: PERMISSION_VIEW |
There was a problem hiding this comment.
Should ListRoles, GetRole, and ListRoleMembers be available for viewer permissions?
| required_permission: PERMISSION_VIEW | ||
| api: API_REDPANDA_ADMIN | ||
| }; | ||
| } |
There was a problem hiding this comment.
Should ListRoles, GetRole, and ListRoleMembers be available for viewer permissions?
There was a problem hiding this comment.
Should be admin as well I think. We did this change as well with ListTransforms to make this a call that requires admin permissions.
There was a problem hiding this comment.
Changed all permissions to be PERMISSION_ADMIN
|
The latest Buf updates on your PR. Results from workflow Buf CI / push-module (pull_request).
|
|
|
||
| // Page size. | ||
| int32 page_size = 2 [(buf.validate.field).int32 = { | ||
| gte: -1 |
There was a problem hiding this comment.
Why not greater or equal to 1? Do we not paginate for -1 page size?
There was a problem hiding this comment.
Where we support it, this seems to be pattern to indicate retrieval of all items, and no pagination is needed. It's less ambiguous than 0.
|
|
||
| if (Features.rolesApi) { | ||
| await client.deleteRole({ roleName: name, deleteAcls }); | ||
| await client.deleteRole({ request: { roleName: name, deleteAcls } }); |
There was a problem hiding this comment.
For these changes we already merged protobuf V2 into main branch so it might require some small syntax updates. If you need help with it, let me know, we need to rebase this PR
| @@ -0,0 +1,85 @@ | |||
| // @generated by protoc-gen-connect-es v1.2.0 with parameter "target=ts,import_extension=" | |||
There was a problem hiding this comment.
These protos may need regenerating after rebasing so that we use new connect-es/protobuf-es version
There was a problem hiding this comment.
👏 All looks good here
3 participants
Add definition for the Roles Dataplane API.
The existing Console Roles API wraps the new Dataplane API, consistent with other similar APIs.
Hook up unimplemented API.
Few drive-by fixes.
Might need some help fixing frontend.
I assume the existing UX functionality is covered by frontend tests.