-
Notifications
You must be signed in to change notification settings - Fork 63
Conversation
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
@ireneontheway,Thanks for your review. However, LGTM is restricted to Reviewers or higher roles.See the corresponding SIG page for more information. Related SIGs: docs(slack). |
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
Co-authored-by: ireneontheway <[email protected]>
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
BTW @GMHDBJD we also support to set TLS config for DM components, do we need to update docs too? |
Will add a TLS tutorials later. PTAL again. |
en/dm-master-configuration-file.md
Outdated
ssl-ca: "/path/to/ca.pem" | ||
ssl-cert: "/path/to/cert.pem" | ||
ssl-key: "/path/to/key.pem" |
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.
the config format is TOML, not YAML here.
en/dm-worker-configuration-file.md
Outdated
ssl-ca: "/path/to/ca.pem" | ||
ssl-cert: "/path/to/cert.pem" | ||
ssl-key: "/path/to/key.pem" |
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.
ditto.
zh/dm-master-configuration-file.md
Outdated
ssl-ca: "/path/to/ca.pem" | ||
ssl-cert: "/path/to/cert.pem" | ||
ssl-key: "/path/to/key.pem" | ||
cert-allowed-cn = ["dm"] |
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.
ditto.
zh/dm-worker-configuration-file.md
Outdated
ssl-ca: "/path/to/ca.pem" | ||
ssl-cert: "/path/to/cert.pem" | ||
ssl-key: "/path/to/key.pem" |
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.
ditto.
zh/dm-master-configuration-file.md
Outdated
@@ -47,3 +51,7 @@ join = "" | |||
| `advertise-peer-urls` | DM-master 向外界宣告的对等 URL。默认为 `peer-urls` 的值。| | |||
| `initial-cluster` | 初始集群中所有 DM-master 的 `advertise-peer-urls` 的值。| | |||
| `join` | 集群里已有的 DM-master 的 `advertise-peer-urls` 的值。如果是新加入的 DM-master 节点,使用 `join` 替代 `initial-cluster`。| | |||
| `ssl-ca` | DM-master 组件 SSL CA 证书所在的路径 | | |||
| `ssl-cert` | DM-master 组件用于连接的 PEM 格式的 X509 证书所在的路径 | |
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.
用于连接
要加一个宾语吗?
This reverts commit c151589.
en/dm-master-configuration-file.md
Outdated
@@ -49,3 +54,7 @@ This section introduces the configuration parameters of DM-master. | |||
| `advertise-peer-urls` | Specifies the peer URL that DM-master advertises to the outside world. The value of `advertise-peer-urls` is by default the same as that of `peer-urls`. | | |||
| `initial-cluster` | The value of `initial-cluster` is the combination of the `advertise-peer-urls` value of all DM-master nodes in the initial cluster. | | |||
| `join` | The value of `join` is the combination of the `advertise-peer-urls` value of the existed DM-master nodes in the cluster. If the DM-master node is newly added, replace `initial-cluster` with `join`. | | |||
| `ssl-ca` | Path of file that contains list of trusted SSL CAs for connection with DM-master components. | |
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.
does English version need to update?
zh/dm-worker-configuration-file.md
Outdated
@@ -36,3 +41,7 @@ join = "127.0.0.1:8261,127.0.0.1:8361,127.0.0.1:8461" | |||
| `worker-addr` | DM-worker 服务的地址,可以省略 IP 信息,例如:":8262"。| | |||
| `advertise-addr` | DM-worker 向外界宣告的地址。 | | |||
| `join` | 对应一个或多个 DM-master 配置中的 [`master-addr`](dm-master-configuration-file.md#global-配置)。 | | |||
| `ssl-ca` | DM-worker 组件用于连接其它组件的 SSL CA 证书所在的路径 | |
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.
哦,好像它们自己也还用这个参数来监听连接
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.
稍微改了一下
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
@ireneontheway PTAL again? |
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
Co-authored-by: ireneontheway <[email protected]>
Co-authored-by: ireneontheway <[email protected]>
Co-authored-by: ireneontheway <[email protected]>
Co-authored-by: ireneontheway <[email protected]>
Co-authored-by: ireneontheway <[email protected]>
Co-authored-by: ireneontheway <[email protected]>
Co-authored-by: ireneontheway <[email protected]>
Co-authored-by: ireneontheway <[email protected]>
@ireneontheway PTAL again, thx |
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
@ireneontheway,Thanks for your review. However, LGTM is restricted to Reviewers or higher roles.See the corresponding SIG page for more information. Related SIGs: docs(slack). |
@TomShawn two LGTM now, PTAL |
What is changed, added or deleted? (Required)
add tls config in source config file and task config file.
Which DM version(s) do your changes apply to? (Required)
What is the related PR or file link(s)?
Do your changes match any of the following descriptions?