PB-7332 : API Changes for Sharing / Unsharing Clusters to Users and Groups #247
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
pallav-px
requested review from
lalat-das,
prashanthpx,
siva-portworx,
vikasit12 and
vsundarraj-px
|
OSS Scan Results:
Total issues: 43 |
|
License Evaluation Results:
Total License Issues: 0 |
pkg/apis/v1/api.proto
Outdated
| @@ -1188,6 +1188,22 @@ service Cluster { | |||
| body : "*" | |||
| }; | |||
| } | |||
|
|
|||
| // share cluster add share access of your cluster to another users | |||
There was a problem hiding this comment.
nit: the first word is the functionName itself like
//ShareClsuter shares access to the cluster to the user(s) or group(s).
pkg/apis/v1/api.proto
Outdated
| }; | ||
| } | ||
|
|
||
| // Unshare cluster remove sharing access of your cluster from another users |
There was a problem hiding this comment.
//unshareClsuter removes shared access for the user(s) or group(s) to the cluster
pkg/apis/v1/api.proto
Outdated
| message ShareClusterRequest{ | ||
| string org_id = 1; | ||
| ObjectRef cluster_ref = 2; | ||
| // it will share the cluster to all the user and group |
There was a problem hiding this comment.
//userid of the user to share the cluster with
pkg/apis/v1/api.proto
Outdated
| ObjectRef cluster_ref = 2; | ||
| // it will share the cluster to all the user and group | ||
| repeated string user = 3; | ||
| repeated string group = 4; |
There was a problem hiding this comment.
//groupIDs to share the cluster with
There was a problem hiding this comment.
groupName it will be..there's nothing called group id
There was a problem hiding this comment.
can we use groups since its an array?
pkg/apis/v1/api.proto
Outdated
| string org_id = 1; | ||
| ObjectRef cluster_ref = 2; | ||
| // it will unshare the cluster from all the user and group | ||
| repeated string user = 3; |
There was a problem hiding this comment.
since this is array of userID, we need to mention the same and also change user to users
pkg/apis/v1/api.proto
Outdated
| // it will share the cluster to all the user and group | ||
| repeated string user = 3; | ||
| repeated string group = 4; | ||
| // share cluster backup is optional, if set to true, it will share the cluster backup |
There was a problem hiding this comment.
// If set to true will additionally share existing backups
pkg/apis/v1/api.proto
Outdated
| // it will unshare the cluster from all the user and group | ||
| repeated string user = 3; | ||
| repeated string group = 4; | ||
| // unshare cluster backup is optional, if set to true, it will unshare the cluster backup |
There was a problem hiding this comment.
// if set will unshare shared backups.
|
OSS Scan Results:
Total issues: 43 |
|
License Evaluation Results:
Total License Issues: 0 |
pkg/apis/v1/api.proto
Outdated
| @@ -1188,6 +1188,22 @@ service Cluster { | |||
| body : "*" | |||
| }; | |||
| } | |||
|
|
|||
| // ShareClsuter shares access to the cluster to the user(s) or group(s) | |||
There was a problem hiding this comment.
type: ShareClsuter -> ShareCluster, cannot ignore these comments cos they will show up as help in swagger
|
OSS Scan Results:
Total issues: 43 |
|
License Evaluation Results:
Total License Issues: 0 |
vsundarraj-px
approved these changes
Jun 28, 2024
pkg/apis/v1/api.proto
Outdated
| // group to share the cluster with | ||
| repeated string group = 4; | ||
| // unshare_cluster_backup is optional, if set to true, will unshare shared backups. | ||
|
|
prashanthpx
reviewed
Jun 28, 2024
| @@ -1188,6 +1188,22 @@ service Cluster { | |||
| body : "*" | |||
| }; | |||
| } | |||
|
|
|||
| // ShareCluster shares access to the cluster to the user(s) or group(s) | |||
| rpc ShareCluster(ShareClusterRequest) returns (ShareClusterResponse) { | |||
There was a problem hiding this comment.
For existing RBAC object we have UpdateOwnership and cluster Share is extension of existing RBAC to clusterObject. Any reason we are introducing two new API?
|
OSS Scan Results:
Total issues: 43 |
|
License Evaluation Results:
Total License Issues: 0 |
pkg/apis/v1/api.proto
Outdated
| string org_id = 1; | ||
| ObjectRef cluster_ref = 2; | ||
| // it will unshare the cluster from all the user and group | ||
| repeated string user = 3; |
There was a problem hiding this comment.
since this is array of userID, we need to mention the same and also change user to users
pkg/apis/v1/api.proto
Outdated
| // userid of the user to share the cluster with | ||
| repeated string user = 3; | ||
| // group to share the cluster with | ||
| repeated string group = 4; |
pkg/apis/v1/api.proto
Outdated
| // unshare_cluster_backup is optional, if set to true, will unshare shared backups. | ||
|
|
||
|
|
||
| bool unshare_cluster_backup = 5; |
There was a problem hiding this comment.
unshare_cluster_backups
pkg/apis/v1/api.proto
Outdated
| string org_id = 1; | ||
| ObjectRef cluster_ref = 2; | ||
| // userid of the user to share the cluster with | ||
| repeated string user = 3; |
There was a problem hiding this comment.
users since its an array
pkg/apis/v1/api.proto
Outdated
| ObjectRef cluster_ref = 2; | ||
| // it will share the cluster to all the user and group | ||
| repeated string user = 3; | ||
| repeated string group = 4; |
There was a problem hiding this comment.
can we use groups since its an array?
Share Cluster : Share Cluster to users and groups
Unshare Cluster: Revoke Shared Clusters from users and groups
Signed-off-by: Pallav <[email protected]>
|
OSS Scan Results:
Total issues: 43 |
|
License Evaluation Results:
Total License Issues: 0 |
pallav-px
added a commit
that referenced
this pull request
Aug 22, 2024
This reverts commit e2486ca.
pallav-px
added a commit
that referenced
this pull request
Aug 22, 2024
vsundarraj-px
pushed a commit
that referenced
this pull request
Sep 2, 2024
Share Cluster : Share Cluster to users and groups
Unshare Cluster: Revoke Shared Clusters from users and groups
Signed-off-by: Pallav <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Share Cluster : Share Cluster to users and groups
Unshare Cluster: Revoke Shared Clusters from users and groups
What this PR does / why we need it:
Which issue(s) this PR fixes (optional)
Closes #
Special notes for your reviewer: