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

v5.2.2: Update TLS docs #7318

Merged
merged 6 commits into from
Oct 25, 2021
Merged

v5.2.2: Update TLS docs #7318

merged 6 commits into from
Oct 25, 2021

Conversation

Liuxiaozhen12
Copy link
Contributor

Update TLS docs with:

TiDB 5.2.x additions
Remove the use of mysql_ssl_rsa_setup as this is replaced with AutoTLS
Add SSL Modes available in MySQL 8.0 client

First-time contributors' checklist

What is changed, added or deleted? (Required)

Which TiDB version(s) do your changes apply to? (Required)

Tips for choosing the affected version(s):

By default, CHOOSE MASTER ONLY so your changes will be applied to the next TiDB major or minor releases. If your PR involves a product feature behavior change or a compatibility change, CHOOSE THE AFFECTED RELEASE BRANCH(ES) AND MASTER.

For details, see tips for choosing the affected versions (in Chinese).

  • master (the latest development version)
  • v5.2 (TiDB 5.2 versions)
  • v5.1 (TiDB 5.1 versions)
  • v5.0 (TiDB 5.0 versions)
  • v4.0 (TiDB 4.0 versions)
  • v3.1 (TiDB 3.1 versions)
  • v3.0 (TiDB 3.0 versions)
  • v2.1 (TiDB 2.1 versions)

What is the related PR or file link(s)?

Do your changes match any of the following descriptions?

  • Delete files
  • Change aliases
  • Need modification after applied to another branch
  • Might cause conflicts after applied to another branch

@Liuxiaozhen12 Liuxiaozhen12 added type/enhancement The issue or PR belongs to an enhancement. translation/from-docs This PR is translated from a PR in pingcap/docs. area/security Relates to TiDB security and privilege. needs-cherry-pick-release-5.2 labels Oct 21, 2021
@ti-chi-bot
Copy link
Member

ti-chi-bot commented Oct 21, 2021 •
edited
Loading

[REVIEW NOTIFICATION]

This pull request has been approved by:

  • TomShawn

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 the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Oct 21, 2021
@TomShawn
Copy link
Contributor

/cc @s3nt3

@ti-chi-bot
Copy link
Member

@TomShawn: GitHub didn't allow me to request PR reviews from the following users: s3nt3.

Note that only pingcap members and repo collaborators can review this PR, and authors cannot review their own PRs.

In response to this:

/cc @s3nt3

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 kubernetes/test-infra repository.

@Liuxiaozhen12
Copy link
Contributor Author

/cc @s3nt3 Please help to tech review this PR translated from pingcap/docs#6323, thanks!

Liuxiaozhen12
Liuxiaozhen12 commented Oct 21, 2021
View reviewed changes
enable-tls-between-clients-and-servers.md Outdated
Comment on lines 44 to 46
- 在启动 TiDB 时，至少需要在配置文件中同时指定 `ssl-cert` 和 `ssl-key` 参数，才能使 TiDB 服务端接受安全连接。还可以指定 `ssl-ca` 参数进行客户端身份验证（请参见[配置启用身份验证](#配置启用身份验证)章节）。
- 参数指定的文件都为 PEM 格式。另外目前 TiDB 尚不支持加载有密码保护的私钥，因此必须提供一个没有密码的私钥文件。若提供的证书或私钥无效，则 TiDB 服务端将照常启动，但并不支持客户端加密连接到 TiDB 服务端。
- 若证书参数无误，则 TiDB 在启动时将会输出 `secure connection is enabled`，否则 TiDB 会输出 `secure connection is NOT ENABLED`。
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I refined this part because I think the above information can all be considered as "advice". @TomShawn PTAL, thanks!

Copy link
Contributor

Choose a reason for hiding this comment

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

LGTM

TomShawn
TomShawn reviewed Oct 21, 2021
View reviewed changes
@TomShawn
Copy link
Contributor

https://github.com/pingcap/docs-cn/pull/7318/files#diff-01451625e61643362bb7b8105d9cf6bd7fb3ef6fae5febf64a5b33ef7eca9d04R10 is not updated according to the docs PR.

s3nt3
s3nt3 reviewed Oct 21, 2021
View reviewed changes
enable-tls-between-clients-and-servers.md Outdated

若证书参数无误，则 TiDB 在启动时将会输出 `secure connection is enabled`，否则 TiDB 会输出 `secure connection is NOT ENABLED`。
> **注意：**
> 在 v5.2.0 版本之前，你可以使用 `mysql_ssl_rsa_setup --datadir=./certs` 生成证书。`mysql_ssal_rsa_setup` 工具是 MySQL 服务器的一部分。
Copy link
Contributor

Choose a reason for hiding this comment

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

这里好像有一个拼写错误：应当将 mysql_ssal_rsa_setup 改为 mysql_ssl_rsa_setup。

Copy link
Contributor

Choose a reason for hiding this comment

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

英文版已在 pingcap/docs#6655 中改正。

@Liuxiaozhen12
Copy link
Contributor Author

https://github.com/pingcap/docs-cn/pull/7318/files#diff-01451625e61643362bb7b8105d9cf6bd7fb3ef6fae5febf64a5b33ef7eca9d04R10 is not updated according to the docs PR.

fixed in cbfb220. Sorry, I thought the meaning is similar and I chose not to modify it before.

@ti-chi-bot ti-chi-bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Oct 21, 2021
TomShawn
TomShawn approved these changes Oct 25, 2021
View reviewed changes
Copy link
Contributor

@TomShawn TomShawn left a comment

Choose a reason for hiding this comment

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

LGTM
@s3nt3 PTAL again, thanks!

@ti-chi-bot ti-chi-bot added the status/LGT1 Indicates that a PR has LGTM 1. label Oct 25, 2021
@TomShawn
Copy link
Contributor

TomShawn commented Oct 25, 2021 •
edited
Loading

@Liuxiaozhen12 Once @s3nt3 approves this PR, please merge it on your own, including its cherry-pick PR, before the v5.2.2 release. Thanks!

@TomShawn
Copy link
Contributor

/remove-status LGT1
/status LGT2

@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 Oct 25, 2021
@TomShawn TomShawn added the priority/P1 The issue has P1 priority. label Oct 25, 2021
@s3nt3
Copy link
Contributor

s3nt3 commented Oct 25, 2021

LGTM

@Liuxiaozhen12
Copy link
Contributor Author

/merge

@ti-chi-bot
Copy link
Member

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

Commit hash: a30463d

@ti-chi-bot ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label Oct 25, 2021
@TomShawn
Copy link
Contributor

/verify

@TomShawn TomShawn merged commit eb8a6a6 into pingcap:master Oct 25, 2021
@ti-chi-bot ti-chi-bot mentioned this pull request Oct 25, 2021
Merged
13 tasks
@ti-chi-bot
Copy link
Member

In response to a cherrypick label: new pull request created: #7329.

ti-chi-bot added a commit that referenced this pull request Oct 25, 2021
@ti-chi-bot
v5.2.2: Update TLS docs (#7318) (#7329)
1b3e3b9
@Liuxiaozhen12 Liuxiaozhen12 deleted the tls_52 branch October 25, 2021 05:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@s3nt3 s3nt3 s3nt3 left review comments

@TomShawn TomShawn TomShawn approved these changes

Assignees
No one assigned
Labels
area/security Relates to TiDB security and privilege. priority/P1 The issue has P1 priority. size/L Denotes a PR that changes 100-499 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. translation/from-docs This PR is translated from a PR in pingcap/docs. type/enhancement The issue or PR belongs to an enhancement.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@Liuxiaozhen12 @ti-chi-bot @TomShawn @s3nt3