feat(encrypt): Support both encrypted and unencrypted passwords for the upstream and downstream database #633

Kuri-su · 2020-04-23T16:24:26Z

What problem does this PR solve?

UCP #506
fix #506

What is changed and how it works?

add and use 'decryptOrPlaintext' function

sre-bot · 2020-04-23T16:24:29Z

Thanks for your contribution. If your PR get merged, you will be rewarded 600 points.

pkg/utils/encrypt.go

…oml-200423

Kuri-su · 2020-04-28T12:37:24Z

@csuzhangxc PTAL, thx , I have no idea about this issue 😂

csuzhangxc

I think you can update some integration test cases (under ./tests directory) to use plaintext password (123456) now.

csuzhangxc · 2020-04-30T07:02:36Z

dm/ctl/common/config.go

-		ciphertext, err1 := utils.Encrypt(c.encrypt)
-		if err1 != nil {
-			return true, err1
+		ciphertext, err := utils.Encrypt(c.encrypt)


rename err1 to err will report some warning by make check.

dm/ctl/common/config.go:94:15: declaration of "err" shadows declaration at line 82 dm/ctl/common/config.go:103:14: declaration of "err" shadows declaration at line 82

csuzhangxc · 2020-04-30T07:03:28Z

pkg/utils/encrypt.go

+		return ciphertextB64, nil
+	}
+	return plaintext, nil
+}


another new line is needed here (with go fmt).

csuzhangxc · 2020-04-30T07:10:17Z

@csuzhangxc PTAL, thx , I have no idea about this issue 😂

This PR is what I want. 👍

…oml-200423

Kuri-su · 2020-05-06T04:48:15Z

@csuzhangxc OK, i'm done, but i don't found the reason of test case failed, can you help me see this problem, THX ; )

Kuri-su · 2020-05-06T04:51:19Z

@csuzhangxc OK, i'm done, but i don't found the reason of test case failed, can you help me see this problem, THX ; )

it said the failed point in https://github.com/pingcap/dm/blob/master/dm/master/server_test.go#L994 , But I did not find the connection between it and cryption modules

csuzhangxc · 2020-05-06T05:05:54Z

@csuzhangxc OK, i'm done, but i don't found the reason of test case failed, can you help me see this problem, THX ; )

it said the failed point in https://github.com/pingcap/dm/blob/master/dm/master/server_test.go#L994 , But I did not find the connection between it and cryption modules

our CI is not stable now, you may simply re-/run-all-tests if you can confirm no connection between it and cryption modules.

csuzhangxc · 2020-05-06T05:10:11Z

/run-all-tests

Kuri-su · 2020-05-06T06:24:11Z

/run-all-tests

yes.....In fact... i have tried /run-all-tests many times..... i delete comments when Jenkins job started.

but the error still exists, let me see this error later of today 😹

…oml-200423

Kuri-su · 2020-05-15T02:43:12Z

I will finish this issue in this weekend 😂

csuzhangxc · 2020-05-15T02:47:32Z

@Kuri-su now, we are moving our internal work to #sig-tools and #sig-tools-zh now, if you have interests, you can join one or two.

…oml-200423

Kuri-su · 2020-05-20T02:47:31Z

Finally 😂

WangXiangUSTC

rest LGTM

WangXiangUSTC · 2020-05-20T06:23:41Z

dm/config/source_config_test.go

+			func() *SourceConfig {
+				cfg := newConfig()
+				cfg.From.Password = "123456" // plaintext password
+				return cfg


how about adding test case config with encrypt password?

ok, i have added a testCase about encrypt password

WangXiangUSTC · 2020-05-20T06:25:02Z

dm/ctl/common/config.go

+		if err1 != nil {
+			return true, err1
+		}
+		fmt.Println(plaintext)


this line should be deleted

Perhaps we are shouldn't delete these lines, because the Decrypt method is called instead of DecryptOrPlaintext in L103 , the function may return a error

WangXiangUSTC · 2020-05-20T06:25:46Z

pkg/utils/encrypt.go

@@ -43,3 +43,12 @@ func Decrypt(ciphertextB64 string) (string, error) {
 	}
 	return string(plaintext), nil
 }
+
+// DecryptOrPlaintext tries to decrypt base64 encoded ciphertext to plaintext or return plaintext
+func DecryptOrPlaintext(ciphertextB64 string) (string, error) {


seems that dosen't need to return error

ok, i have cleaned the extra error variables

WangXiangUSTC

LGTM

csuzhangxc · 2020-05-20T07:49:32Z

@Kuri-su please update this branch with the base branch.

…oml-200423

sre-bot · 2020-05-20T08:10:49Z

Team □□□□□ complete task #506 and get 600 score, current score 1550

csuzhangxc · 2020-05-20T08:11:11Z

@Kuri-su thank you, ❤️

sre-bot · 2020-05-20T08:11:13Z

cherry pick to release-1.0 failed

sre-bot added the contribution For contributor label Apr 23, 2020

Kuri-su commented Apr 24, 2020

View reviewed changes

pkg/utils/encrypt.go Outdated Show resolved Hide resolved

Kuri-su changed the title ~~feat(encrypt): Support both encrypted and unencrypted passwords for the upstream and downstream database~~ [WIP]feat(encrypt): Support both encrypted and unencrypted passwords for the upstream and downstream database Apr 27, 2020

Kuri-su force-pushed the feature/kurisu-support-unencrypted-pw-in-toml-200423 branch from f670199 to b2a7f56 Compare April 28, 2020 12:26

Kuri-su marked this pull request as draft April 28, 2020 12:26

feat: add support encrypted and unencrypted in config file

bc8bac5

Kuri-su force-pushed the feature/kurisu-support-unencrypted-pw-in-toml-200423 branch from b2a7f56 to bc8bac5 Compare April 28, 2020 12:36

Kuri-su changed the title ~~[WIP]feat(encrypt): Support both encrypted and unencrypted passwords for the upstream and downstream database~~ feat(encrypt): Support both encrypted and unencrypted passwords for the upstream and downstream database Apr 28, 2020

Kuri-su marked this pull request as ready for review April 28, 2020 12:36

Merge branch 'master' into feature/kurisu-support-unencrypted-pw-in-t…

d84aece

…oml-200423

csuzhangxc reviewed Apr 30, 2020

View reviewed changes

Kuri-su and others added 3 commits May 6, 2020 08:45

fix():

53ebb15

Merge branch 'master' into feature/kurisu-support-unencrypted-pw-in-t…

438169f

…oml-200423

fix(test): test case

eb8f7e1

Kuri-su force-pushed the feature/kurisu-support-unencrypted-pw-in-toml-200423 branch from e0611fe to eb8f7e1 Compare May 7, 2020 01:02

Merge branch 'master' into feature/kurisu-support-unencrypted-pw-in-t…

bf05b56

…oml-200423

csuzhangxc mentioned this pull request May 13, 2020

DM uses a hard-coded encryption key #659

Closed

Merge branch 'master' into feature/kurisu-support-unencrypted-pw-in-t…

8912fac

…oml-200423

csuzhangxc added the type/feature New feature label May 20, 2020

csuzhangxc requested a review from WangXiangUSTC May 20, 2020 02:34

csuzhangxc added this to the v2.0.0 beta.2 milestone May 20, 2020

WangXiangUSTC reviewed May 20, 2020

View reviewed changes

fix(descrypt): clean code && add testCase

bde723b

WangXiangUSTC approved these changes May 20, 2020

View reviewed changes

WangXiangUSTC added status/LGT2 Two reviewers already commented LGTM, ready for merge and removed status/LGT1 One reviewer already commented LGTM labels May 20, 2020

Merge branch 'master' into feature/kurisu-support-unencrypted-pw-in-t…

870c528

…oml-200423

csuzhangxc added the needs-cherry-pick-release-1.0 This PR should be cherry-picked to release-1.0. Remove this label after cherry-picked to release-1.0 label May 20, 2020

csuzhangxc merged commit 9a4a5ba into pingcap:master May 20, 2020

Kuri-su added a commit to Kur-Public/dm that referenced this pull request May 20, 2020

feat: manual cherry merge PR pingcap#633 to release-1.0

db91087

Kuri-su mentioned this pull request May 20, 2020

feat: manual cherry merge PR #633 to release-1.0 #676

Merged

csuzhangxc mentioned this pull request May 22, 2020

zh: add files about manage task pingcap/docs-dm#90

Merged

2 tasks

csuzhangxc pushed a commit that referenced this pull request May 22, 2020

feat: manual cherry merge PR #633 to release-1.0 (#676)

6a5d369

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(encrypt): Support both encrypted and unencrypted passwords for the upstream and downstream database #633

feat(encrypt): Support both encrypted and unencrypted passwords for the upstream and downstream database #633

Kuri-su commented Apr 23, 2020

sre-bot commented Apr 23, 2020

Kuri-su commented Apr 28, 2020

csuzhangxc left a comment

csuzhangxc Apr 30, 2020

csuzhangxc Apr 30, 2020

csuzhangxc commented Apr 30, 2020

Kuri-su commented May 6, 2020

Kuri-su commented May 6, 2020

csuzhangxc commented May 6, 2020 •

edited

Loading

csuzhangxc commented May 6, 2020

Kuri-su commented May 6, 2020 •

edited

Loading

Kuri-su commented May 15, 2020

csuzhangxc commented May 15, 2020

Kuri-su commented May 20, 2020

WangXiangUSTC left a comment

WangXiangUSTC May 20, 2020

Kuri-su May 20, 2020

WangXiangUSTC May 20, 2020

Kuri-su May 20, 2020

WangXiangUSTC May 20, 2020

Kuri-su May 20, 2020 •

edited

Loading

WangXiangUSTC left a comment

csuzhangxc commented May 20, 2020

sre-bot commented May 20, 2020

csuzhangxc commented May 20, 2020

sre-bot commented May 20, 2020

feat(encrypt): Support both encrypted and unencrypted passwords for the upstream and downstream database #633

feat(encrypt): Support both encrypted and unencrypted passwords for the upstream and downstream database #633

Conversation

Kuri-su commented Apr 23, 2020

What problem does this PR solve?

What is changed and how it works?

sre-bot commented Apr 23, 2020

Kuri-su commented Apr 28, 2020

csuzhangxc left a comment

Choose a reason for hiding this comment

csuzhangxc Apr 30, 2020

Choose a reason for hiding this comment

csuzhangxc Apr 30, 2020

Choose a reason for hiding this comment

csuzhangxc commented Apr 30, 2020

Kuri-su commented May 6, 2020

Kuri-su commented May 6, 2020

csuzhangxc commented May 6, 2020 • edited Loading

csuzhangxc commented May 6, 2020

Kuri-su commented May 6, 2020 • edited Loading

Kuri-su commented May 15, 2020

csuzhangxc commented May 15, 2020

Kuri-su commented May 20, 2020

WangXiangUSTC left a comment

Choose a reason for hiding this comment

WangXiangUSTC May 20, 2020

Choose a reason for hiding this comment

Kuri-su May 20, 2020

Choose a reason for hiding this comment

WangXiangUSTC May 20, 2020

Choose a reason for hiding this comment

Kuri-su May 20, 2020

Choose a reason for hiding this comment

WangXiangUSTC May 20, 2020

Choose a reason for hiding this comment

Kuri-su May 20, 2020 • edited Loading

Choose a reason for hiding this comment

WangXiangUSTC left a comment

Choose a reason for hiding this comment

csuzhangxc commented May 20, 2020

sre-bot commented May 20, 2020

csuzhangxc commented May 20, 2020

sre-bot commented May 20, 2020

csuzhangxc commented May 6, 2020 •

edited

Loading

Kuri-su commented May 6, 2020 •

edited

Loading

Kuri-su May 20, 2020 •

edited

Loading