- Notifications
You must be signed in to change notification settings - Fork 115
Support for security remote_cluster and associated privileges #3125
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Follow ups from comments on original PR:
done in 1d5d03a
done in 9a70a07
Thank you ! #3123 (comment) and #3123 (comment)
Still pending... |
Following you can find the validation results for the APIs you have changed.
You can validate these APIs yourself by using the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM from an ES security perspective -- deferring to @flobernd on the final recommendation around whether we want to go from string to enum for privilege values in the read APIs.
body: { cluster: string[]; index: IndexName[] } | ||
body: { | ||
cluster: ClusterPrivilege[] | ||
index: IndexName[] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not for this PR, but I think this is wrong: IndexName
should either be a string
or IndexPrivilege
-- IndexName
is something else entirely
specification/security/get_builtin_privileges/SecurityGetBuiltinPrivilegesResponse.ts Show resolved Hide resolved
Following you can find the validation results for the APIs you have changed.
You can validate these APIs yourself by using the |
1 similar comment
Following you can find the validation results for the APIs you have changed.
You can validate these APIs yourself by using the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for iterating! LGTM.
The backport to
To backport manually, run these commands in your terminal: # Fetch latest updates from GitHub git fetch # Create a new working tree git worktree add .worktrees/backport-8.x 8.x # Navigate to the new working tree cd .worktrees/backport-8.x # Create a new branch git switch --create backport-3125-to-8.x # Cherry-pick the merged commit of this pull request and resolve the conflicts git cherry-pick -x --mainline 1 dac7201aa2a52228954f70195c478647caeae7c1 # Push it to GitHub git push --set-upstream origin backport-3125-to-8.x # Go back to the original working tree cd ../.. # Delete the working tree git worktree remove .worktrees/backport-8.x Then, create a pull request where the |
Thanks! We use the same branching strategy but backporting is a bit different. I applied the "backport 8.x" label. |
This commit adds support for the remote_cluster in the role and role descriptors. Additionally: * adds missing references to remote_indices * add new cluster privilege monitor_stats * adds related version information where applicable * updates references to cluster from string[] to proper enumeration (cherry picked from commit dac7201)
Since the automatic backport failed due to a conflict, I created it manually at #3140 and would appreciate a review. |
This commit adds support for the remote_cluster in the role and role descriptors.
Additionally:
===
Note - this PR replaces #3123 since PR from forks won't pass CI.