-
Notifications
You must be signed in to change notification settings - Fork 709
br: add a new authentication method for Azure #22267
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
base: master
Are you sure you want to change the base?
Conversation
Summary of ChangesHello @qiancai, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! 此拉取请求的标题表明其目的是引入新的Azure认证方法。然而,根据提供的补丁,实际的更改是添加了一个名为 "temp.md" 的测试文件。这可能意味着主要功能更改尚未提交,或者 "temp.md" 是一个用于验证CI/CD流程或其他测试目的的占位符。 Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
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.
Code Review
你好,这个 PR 的标题是“br: 添加新的Auzre认证方法”,并且链接了一个相关的中文文档 PR,但实际的文件变更只是添加了一个名为 temp.md 的测试文件。这与 PR 的意图严重不符。
请检查你的提交,并推送正确的文档变更。这个 temp.md 文件应该被移除。
另外,PR 标题中 “Auzre” 似乎是 “Azure” 的拼写错误。
Synced from: pingcap/docs-cn#21206 Target PR: pingcap#22267 AI Provider: gemini Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
|
/gemini review |
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.
Code Review
This pull request adds documentation for a new Azure authentication method using Managed Identity. The changes are clear and well-structured. I've provided a couple of minor suggestions to improve phrasing and clarity, in line with the repository's style guide.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
/cc @RidRisR |
|
@qiancai: GitHub didn't allow me to request PR reviews from the following users: RidRisR. Note that only pingcap members and repo collaborators can review this PR, and authors cannot review their own PRs. DetailsIn response to this:
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-sigs/prow repository. |
|
/cc @Oreoxmt |
|
|
||
| - **System-assigned managed identity**: | ||
|
|
||
| When using a system-assigned managed identity, there is no need to configure any Azure-related environment variables. You can simply run the BR backup command. |
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.
| When using a system-assigned managed identity, there is no need to configure any Azure-related environment variables. You can simply run the BR backup command. | |
| When using a system-assigned managed identity, there is no need to configure any Azure-related environment variables. You can run the BR backup command directly. |
|
|
||
| > **Note:** | ||
| > | ||
| > Ensure that the `AZURE_CLIENT_ID`, `AZURE_TENANT_ID`, or `AZURE_CLIENT_SECRET` environment variables do **not** exist in the running environment. Otherwise, the Azure SDK might prioritize other authentication methods, preventing the managed identity from taking effect. |
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.
| > Ensure that the `AZURE_CLIENT_ID`, `AZURE_TENANT_ID`, or `AZURE_CLIENT_SECRET` environment variables do **not** exist in the running environment. Otherwise, the Azure SDK might prioritize other authentication methods, preventing the managed identity from taking effect. | |
| > Ensure that the `AZURE_CLIENT_ID`, `AZURE_TENANT_ID`, and `AZURE_CLIENT_SECRET` environment variables are **not** set in the runtime environment. Otherwise, the Azure SDK might prioritize other authentication methods, preventing the managed identity from taking effect. |
|
|
||
| - **User-assigned managed identity**: | ||
|
|
||
| When using a user-assigned managed identity, you need to configure the `AZURE_CLIENT_ID` environment variable in the running environment of TiKV and BR, set its value to the client ID of the managed identity, and then run the BR backup command. The detailed steps are as follows: |
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.
| When using a user-assigned managed identity, you need to configure the `AZURE_CLIENT_ID` environment variable in the running environment of TiKV and BR, set its value to the client ID of the managed identity, and then run the BR backup command. The detailed steps are as follows: | |
| When using a user-assigned managed identity, you need to configure the `AZURE_CLIENT_ID` environment variable in the runtime environment of TiKV and BR, set its value to the client ID of the managed identity, and then run the BR backup command. The detailed steps are as follows: |
|
|
||
| The following steps use the TiKV port `24000` and the systemd service name `tikv-24000` as an example: | ||
|
|
||
| 1. Open the service configuration editor by running the following command: |
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.
| 1. Open the service configuration editor by running the following command: | |
| 1. Open the systemd service editor by running the following command: |
| systemctl edit tikv-24000 | ||
| ``` | ||
|
|
||
| 2. Configure the `AZURE_CLIENT_ID` environment variable using your client ID: |
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.
| 2. Configure the `AZURE_CLIENT_ID` environment variable using your client ID: | |
| 2. Set the `AZURE_CLIENT_ID` environment variable to your managed identity client ID: |
Oreoxmt
left a comment
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.
Rest LGTM
[LGTM Timeline notifier]Timeline:
|
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).
What is the related PR or file link(s)?
Do your changes match any of the following descriptions?