-
Notifications
You must be signed in to change notification settings - Fork 5.8k
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
*: change SHOW CONFIG to require CONFIG privilege #25379
Conversation
/run-check_dev_2 |
/lgtm |
@bb7133: Please use GitHub review feature instead of For the reason we drop support to the commands, see also this page. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the ti-community-infra/tichi repository. |
Since it is a compatibility breaker, we need to update the document accordingly. |
PTAL @tiancaiamao @djshow832 |
Did you mean release notes, or something specific like SHOW CONFIG docs? |
I think we need to do the both, and explicitly announce that this compatibility change in the release note. |
[REVIEW NOTIFICATION] This pull request has been approved by:
To complete the pull request process, please ask the reviewers in the list to review by filling The full list of commands accepted by this bot can be found here. Reviewer can indicate their review by submitting an approval review. |
/merge |
This pull request has been accepted and is ready to merge. Commit hash: 230fe4c
|
/rebuild |
/merge |
Signed-off-by: ti-srebot <ti-srebot@pingcap.com>
cherry pick to release-5.1 in PR #25433 |
What problem does this PR solve?
Problem Summary:
The original design document for SEM specified that
SHOW CONFIG
would be disabled, butSET CONFIG
is still permitted with the config privilege. From further discussion, this is a weird behavior, sinceSET CONFIG
is not useful whenSHOW CONFIG
is disabled.Since it is desired that
SHOW CONFIG
is hidden from lower privileged users, the next discussion topic is what permission should it require? SinceSET CONFIG
requires theCONFIG
privilege, the natural conclusion is thatSHOW CONFIG
should also.This PR makes this change to the privileges, and updates the original proposal.
What is changed and how it works?
What's Changed:
The
SHOW CONFIG
statement now requires theCONFIG
privileges to be assigned.Related changes
Check List
Tests
Side effects
Release note
SHOW CONFIG
statement now requires theCONFIG
privileges to be assigned.