Skip to content
This repository has been archived by the owner on Nov 24, 2023. It is now read-only.

.*: support TLS #569

Merged
merged 56 commits into from
Jul 23, 2020
Merged

.*: support TLS #569

merged 56 commits into from
Jul 23, 2020

Conversation

WangXiangUSTC
Copy link
Contributor

@WangXiangUSTC WangXiangUSTC commented Mar 22, 2020 •
edited
Loading

What problem does this PR solve?

  1. Support TLS and online reload with new certs Support TLS and online reload with new certs #467
  2. Validate CommonName attribute for TLS certificate Validate CommonName attribute for TLS certificate #524

What is changed and how it works?

add security config for dm-master(include embed etcd), dm-worker, dmctl, source database and downstream database.

TODO:

  1. grpc gateway is not work when use tls, need to fix it
  2. when set AllowedCN for embed etcd, etcd cluster will run with error, and don't know why now, need to fix it later

Check List

Tests

  • Integration test

Related changes

  • Need to cherry-pick to the release branch
  • Need to update the documentation
  • Need to update the dm/dm-ansible
  • Need to be included in the release note

@WangXiangUSTC WangXiangUSTC added the status/WIP This PR is still work in progress label Mar 22, 2020
@codecov
Copy link

codecov bot commented Mar 23, 2020 •
edited
Loading

Codecov Report

Merging #569 into master will decrease coverage by 1.0737%.
The diff coverage is 39.2709%.

@@               Coverage Diff                @@
##             master       #569        +/-   ##
================================================
- Coverage   57.0981%   56.0244%   -1.0738%     
================================================
  Files           205        212         +7     
  Lines         21104      22110      +1006     
================================================
+ Hits          12050      12387       +337     
- Misses         7890       8502       +612     
- Partials       1164       1221        +57

@WangXiangUSTC WangXiangUSTC added priority/important Major change, requires approval from ≥2 primary reviewers status/PTAL This PR is ready for review. Add this label back after committing new changes type/feature New feature and removed status/WIP This PR is still work in progress labels Mar 23, 2020
@WangXiangUSTC
Copy link
Contributor Author

/run-all-tests

1 similar comment
@WangXiangUSTC
Copy link
Contributor Author

/run-all-tests

@WangXiangUSTC
Copy link
Contributor Author

did we test CommonName as pingcap/tidb#15137 ?

Yes, but it had something wrong and I can't locate the root problem now. I add a TODO in the description.

@WangXiangUSTC
Copy link
Contributor Author

/run-all-tests

@csuzhangxc csuzhangxc added the needs-update-docs Should update docs after this PR is merged. Remove this label once the docs are updated label Jul 21, 2020
csuzhangxc
csuzhangxc reviewed Jul 21, 2020
View reviewed changes
dm/config/security.go Show resolved Hide resolved
pkg/terror/error_list.go Show resolved Hide resolved
errors.toml Outdated Show resolved Hide resolved
dm/ctl/common/config.go Outdated Show resolved Hide resolved
dm/master/config.go Show resolved Hide resolved
dm/master/etcd.go Show resolved Hide resolved
dm/master/server.go Show resolved Hide resolved
dm/master/server.go Show resolved Hide resolved
dm/worker/config.go Show resolved Hide resolved
pkg/conn/basedb.go Show resolved Hide resolved
csuzhangxc
csuzhangxc reviewed Jul 22, 2020
View reviewed changes
dm/config/security.go Outdated Show resolved Hide resolved
csuzhangxc
csuzhangxc reviewed Jul 23, 2020
View reviewed changes
pkg/conn/basedb.go Outdated Show resolved Hide resolved
csuzhangxc
csuzhangxc approved these changes Jul 23, 2020
View reviewed changes
Copy link
Member

@csuzhangxc csuzhangxc left a comment

Choose a reason for hiding this comment

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

LGTM

@csuzhangxc csuzhangxc added status/LGT2 Two reviewers already commented LGTM, ready for merge and removed status/LGT1 One reviewer already commented LGTM labels Jul 23, 2020
@WangXiangUSTC WangXiangUSTC merged commit 7ec79ae into master Jul 23, 2020
@WangXiangUSTC WangXiangUSTC deleted the xiang/tls branch July 23, 2020 07:13
This was referenced Jul 23, 2020
Closed
Closed
@csuzhangxc
Copy link
Member

docs PR pingcap/docs-dm#228

@csuzhangxc csuzhangxc added already-update-docs The docs related to this PR already updated. Add this label once the docs are updated and removed needs-update-docs Should update docs after this PR is merged. Remove this label once the docs are updated labels Aug 14, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@GMHDBJD GMHDBJD GMHDBJD left review comments

@lance6716 lance6716 lance6716 left review comments

@csuzhangxc csuzhangxc csuzhangxc approved these changes

@lichunzhu lichunzhu Awaiting requested review from lichunzhu

Assignees
No one assigned
Labels
already-update-docs The docs related to this PR already updated. Add this label once the docs are updated priority/important Major change, requires approval from ≥2 primary reviewers status/LGT2 Two reviewers already commented LGTM, ready for merge type/feature New feature
Projects
None yet
Milestone
v2.0.0 RC
Development

Successfully merging this pull request may close these issues.
4 participants
@WangXiangUSTC @csuzhangxc @lance6716 @GMHDBJD