Skip to content
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.

Sign up for GitHub

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

Jump to bottom

*: support ACCOUNT (UN)LOCK when creating/altering user #37052

Merged
merged 24 commits into from
Aug 25, 2022
Merged

*: support ACCOUNT (UN)LOCK when creating/altering user #37052

merged 24 commits into from
Aug 25, 2022

Conversation

CbcWestwolf
Copy link
Member

@CbcWestwolf CbcWestwolf commented Aug 11, 2022 •
edited by morgo
Loading

What problem does this PR solve?

Issue Number: close #37051 close #35961

Problem Summary:

ACCOUNT (UN)LOCK has not been implemented.

What is changed and how it works?

Lock/Unlock a user/role when creating/altering user.

Check List

Tests

  • Unit test
  • Integration test
  • Manual test (add detailed steps at last)
  • No code

Side effects

  • Performance regression: Consumes more CPU
  • Performance regression: Consumes more Memory
  • Breaking backward compatibility

Documentation

  • Affects user behaviors
  • Contains syntax changes
  • Contains variable changes
  • Contains experimental features
  • Changes MySQL compatibility

Manual Test

  1. start a cluster with 2 tidb-servers
  2. using root to connect to tidb-server-1 and create a user user1, and GRANT ALL on test.* to role1
  3. success to connect to tidb-server-2 using user1
  4. run alter user user1 account lock in tidb-server-1
  5. user1 can not connect again to tidb-server-2 after it 'exit' the connection
  6. run alter user user1 account unlock in tidb-server-1
  7. user1 can connect again to tidb-server-2

Release note

Please refer to Release Notes Language Style Guide to write a quality release note.

Before, TiDB users cannot 'lock' a user by creating/altering user statements.

After this PR, TiDB users can do this like MySQL.

@ti-chi-bot
Copy link
Member

ti-chi-bot commented Aug 11, 2022 •
edited
Loading

[REVIEW NOTIFICATION]

This pull request has been approved by:

  • bb7133
  • morgo

To complete the pull request process, please ask the reviewers in the list to review by filling /cc @reviewer in the comment.
After your PR has acquired the required number of LGTMs, you can assign this pull request to the committer in the list by filling /assign @committer in the comment to help you merge this pull request.

The full list of commands accepted by this bot can be found here.

Reviewer can indicate their review by submitting an approval review.
Reviewer can cancel approval by submitting a request changes review.

@ti-chi-bot ti-chi-bot added release-note-none Denotes a PR that doesn't merit a release note. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Aug 11, 2022
@sre-bot
Copy link
Contributor

sre-bot commented Aug 11, 2022 •
edited
Loading

Code Coverage Details: https://codecov.io/github/pingcap/tidb/commit/7333d41acf1bc4c4a9a0cb08d1e124e79f3972e1

@ti-chi-bot ti-chi-bot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Aug 11, 2022
@ti-chi-bot ti-chi-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 13, 2022
@CbcWestwolf CbcWestwolf force-pushed the support_account_lock branch from 0e5cbec to dcafd3b Compare August 13, 2022 16:41
@ti-chi-bot ti-chi-bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 13, 2022
@ti-chi-bot ti-chi-bot added release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed release-note-none Denotes a PR that doesn't merit a release note. labels Aug 15, 2022
@CbcWestwolf CbcWestwolf marked this pull request as ready for review August 15, 2022 13:02
@CbcWestwolf CbcWestwolf requested a review from a team as a code owner August 15, 2022 13:02
@ti-chi-bot ti-chi-bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Aug 15, 2022
@morgo morgo self-requested a review August 17, 2022 14:01
@ti-chi-bot ti-chi-bot added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Aug 17, 2022
@CbcWestwolf
Copy link
Member Author

/run-mysql-test tidb-test=pr/1923

tiancaiamao
tiancaiamao approved these changes Aug 22, 2022
View reviewed changes
Copy link
Contributor

@tiancaiamao tiancaiamao left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, you can split the PR for such kind.
A lot change is related to the Auth() interface which used to return boolean and now returns error...
The code change for ACCOUNT LOCK itself is not much.

@CbcWestwolf CbcWestwolf mentioned this pull request Aug 24, 2022
Closed
@CbcWestwolf
Copy link
Member Author

/run-mysql-test tidb-test=pr/1923

1 similar comment
@CbcWestwolf
Copy link
Member Author

/run-mysql-test tidb-test=pr/1923

@CbcWestwolf
Copy link
Member Author

/run-mysql-test tidb-test=pr/1923

@tiancaiamao
Copy link
Contributor

/merge

@ti-chi-bot
Copy link
Member

This pull request has been accepted and is ready to merge.

Commit hash: a6115df

@ti-chi-bot ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label Aug 25, 2022
@CbcWestwolf
Copy link
Member Author

/run-mysql-test tidb-test=pr/1923

@tiancaiamao
Copy link
Contributor

/unhold

@ti-chi-bot ti-chi-bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Aug 25, 2022
@CbcWestwolf
Copy link
Member Author

/run-mysql-test tidb-test=pr/1923

@ti-chi-bot ti-chi-bot merged commit a89ef1f into pingcap:master Aug 25, 2022
@tiancaiamao
Copy link
Contributor

/run-mysql-test tidb-test=pr/1923
/merge

@sre-bot
Copy link
Contributor

sre-bot commented Aug 25, 2022

TiDB MergeCI notify

🔴 Bad News! New failing [2] after this pr merged.
These new failed integration tests seem to be caused by the current PR, please try to fix these new failed integration tests, thanks!

CI Name Result Duration Compare with Parent commit
idc-jenkins-ci-tidb/common-test 🟥 failed 2, success 9, total 11 14 min New failing
idc-jenkins-ci-tidb/integration-common-test 🟥 failed 2, success 15, total 17 11 min New failing
idc-jenkins-ci/integration-cdc-test 🟢 all 36 tests passed 59 min Existing passed
idc-jenkins-ci-tidb/tics-test 🟢 all 1 tests passed 14 min Existing passed
idc-jenkins-ci-tidb/sqllogic-test-2 🟢 all 28 tests passed 6 min 33 sec Existing passed
idc-jenkins-ci-tidb/integration-ddl-test 🟢 all 6 tests passed 6 min 3 sec Existing passed
idc-jenkins-ci-tidb/sqllogic-test-1 🟢 all 26 tests passed 4 min 57 sec Existing passed
idc-jenkins-ci-tidb/mybatis-test 🟢 all 1 tests passed 3 min 53 sec Existing passed
idc-jenkins-ci-tidb/integration-compatibility-test 🟢 all 1 tests passed 3 min 4 sec Existing passed
idc-jenkins-ci-tidb/plugin-test 🟢 build success, plugin test success 4min Existing passed

@CbcWestwolf CbcWestwolf mentioned this pull request Aug 25, 2022
Merged
13 tasks
morgo added a commit to morgo/tidb that referenced this pull request Aug 27, 2022
@morgo
Merge remote-tracking branch 'upstream/master' into improve-sysvar-co…
028958e 
…verage

* upstream/master: (109 commits)
  *: rename TemporaryTableAttachedInfoSchema (pingcap#37408)
  *: enable noloopclosure (pingcap#37153)
  executor: add JSON opaque value condition to everywhere (pingcap#37390)
  *: refine mock session manager (pingcap#37400)
  lightning: check counter value to make code more robust (pingcap#37380)
  *: use go 1.19 (pingcap#36223)
  br: restore tiflash replica count after PiTR (pingcap#37181)
  testkit,executor: add MustQueryWitnContext API and fix unstable test TestCoprocessorPriority (pingcap#37382)
  executor: add conversion to opaque value for json_objectagg and json_arrayagg (pingcap#37337)
  expression: quote json path if necessarily (pingcap#37375)
  *: support ACCOUNT (UN)LOCK when creating/altering user (pingcap#37052)
  log-backup: support the ddl(exchange partition) when PiTR  (pingcap#37050)
  *: change `integration.BeforeTest` to `integration.BeforeTestExternal` to avoid skip tests (pingcap#37341)
  executor: fix unstable TestGlobalMemoryTrackerOnCleanUp (pingcap#37372)
  util/ranger: avoid building ranges twice in detachCNFCondAndBuildRangeForIndex (pingcap#37177)
  statistics: track used stats status during query (pingcap#37101)
  *: fix data race in `TestTiFlashGroupIndexWhenStartup` (pingcap#37371)
  lightning: fix dpanic log when init session vars (pingcap#37301)
  executor: let plan replayer record table tiflash replica (pingcap#37336)
  types: fix a bug in casting str2str when `union` (pingcap#37242)
  ...
@lcwangchao lcwangchao mentioned this pull request Jun 14, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tiancaiamao tiancaiamao tiancaiamao approved these changes

@bb7133 bb7133 bb7133 approved these changes

@morgo morgo morgo approved these changes

Assignees
No one assigned
Labels
release-note Denotes a PR that will be considered when it comes time to generate release notes. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. status/can-merge Indicates a PR has been approved by a committer. status/LGT2 Indicates that a PR has LGTM 2.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Support ACCOUNT LOCK/UNLOCK for TiDB Support LOCK USER/UNLOCK USER
7 participants
@CbcWestwolf @ti-chi-bot @sre-bot @hawkingrei @bb7133 @tiancaiamao @morgo