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.

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

DM/Openapi: improve password input mechanism source #5369

Merged
merged 17 commits into from
May 13, 2022
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
32fce23
commit-message: init
WizardXiao May 10, 2022
a1e78f3
Merge branch 'master' into support-no-source-password
WizardXiao May 10, 2022
e3d8b42
commit-message: address some comments
WizardXiao May 11, 2022
0f8121c
commit-message: fix make check
WizardXiao May 11, 2022
e7e3e4a
Merge branch 'master' of https://github.com/WizardXiao/tiflow into su…
WizardXiao May 11, 2022
3338a0b
Merge branch 'support-no-source-password' of https://github.com/Wizar…
WizardXiao May 11, 2022
3b09022
commit-message: change var name to ObfuscatedPasswordForFeedback
WizardXiao May 11, 2022
22c1aa3
commit-message: merge master
WizardXiao May 12, 2022
e3c53b8
Merge branch 'master' into support-no-source-password
ti-chi-bot May 12, 2022
a5f9b95
Merge branch 'master' into support-no-source-password
WizardXiao May 12, 2022
e9b5032
Merge branch 'master' into support-no-source-password
ti-chi-bot May 12, 2022
a7fa594
Merge branch 'master' of https://github.com/WizardXiao/tiflow into su…
WizardXiao May 12, 2022
4209be6
Merge branch 'master' into support-no-source-password
ti-chi-bot May 12, 2022
34e94a5
Merge branch 'master' into support-no-source-password
ti-chi-bot May 12, 2022
ea62b22
Merge branch 'master' into support-no-source-password
ti-chi-bot May 13, 2022
3271f86
Merge branch 'master' of https://github.com/WizardXiao/tiflow into su…
WizardXiao May 13, 2022
a4ec3b2
Merge branch 'support-no-source-password' of https://github.com/Wizar…
WizardXiao May 13, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions dm/dm/config/source_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,10 @@ var getAllServerIDFunc = utils.GetAllServerID
//go:embed source.yaml
var SampleSourceConfig string

// ObfuscatedPasswordForFeedback is the source encryption password that returns to the foreground.
// PM's requirement, we always return obfuscated password to users.
var ObfuscatedPasswordForFeedback string = "******"

// PurgeConfig is the configuration for Purger.
type PurgeConfig struct {
Interval int64 `yaml:"interval" toml:"interval" json:"interval"` // check whether need to purge at this @Interval (seconds)
Expand Down
12 changes: 7 additions & 5 deletions dm/dm/config/source_converter.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ func SourceCfgToOpenAPISource(cfg *SourceConfig) openapi.Source {
Enable: cfg.Enable,
EnableGtid: cfg.EnableGTID,
Host: cfg.From.Host,
Password: "******", // PM's requirement, we always return obfuscated password to user
Password: &ObfuscatedPasswordForFeedback, // PM's requirement, we always return obfuscated password to users
Port: cfg.From.Port,
SourceName: cfg.SourceID,
User: cfg.From.User,
Expand Down Expand Up @@ -55,10 +55,12 @@ func SourceCfgToOpenAPISource(cfg *SourceConfig) openapi.Source {
func OpenAPISourceToSourceCfg(source openapi.Source) *SourceConfig {
cfg := NewSourceConfig()
from := DBConfig{
Host: source.Host,
Port: source.Port,
User: source.User,
Password: source.Password,
Host: source.Host,
Port: source.Port,
User: source.User,
}
if source.Password != nil {
from.Password = *source.Password
}
if source.Security != nil {
from.Security = &Security{
Expand Down
6 changes: 6 additions & 0 deletions dm/dm/master/openapi_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,12 @@ func (s *Server) updateSource(ctx context.Context, sourceName string, req openap
return nil, terror.ErrSchedulerSourceCfgNotExist.Generate(sourceName)
}
newCfg := config.OpenAPISourceToSourceCfg(req.Source)

// update's request will be no password when user doesn't input password and wants to use old password.
if req.Source.Password == nil {
newCfg.From.Password = oldCfg.From.Password
}

if err := checkAndAdjustSourceConfigFunc(ctx, newCfg); err != nil {
return nil, err
}
Expand Down
36 changes: 35 additions & 1 deletion dm/dm/master/openapi_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ func (s *OpenAPIControllerSuite) SetupSuite() {
Enable: true,
EnableGtid: false,
Host: dbCfg.Host,
Password: dbCfg.Password,
Password: &dbCfg.Password,
Port: dbCfg.Port,
User: dbCfg.User,
}
Expand Down Expand Up @@ -221,6 +221,40 @@ func (s *OpenAPIControllerSuite) TestSourceController() {
s.Nil(err)
s.Len(sourceList, 0)
}

// create and update no password source
{
// no password will use "" as password
source := *s.testSource
source.Password = nil
createReq := openapi.CreateSourceRequest{Source: source}
resp, err := server.createSource(ctx, createReq)
s.NoError(err)
s.EqualValues(source, *resp)
config := server.scheduler.GetSourceCfgByID(source.SourceName)
s.NotNil(config)
s.Equal("", config.From.Password)

// update to have password
updateReq := openapi.UpdateSourceRequest{Source: *s.testSource}
sourceAfterUpdated, err := server.updateSource(ctx, source.SourceName, updateReq)
s.NoError(err)
s.EqualValues(s.testSource, sourceAfterUpdated)

// update without password will use old password
source = *s.testSource
source.Password = nil
updateReq = openapi.UpdateSourceRequest{Source: source}
sourceAfterUpdated, err = server.updateSource(ctx, source.SourceName, updateReq)
s.NoError(err)
s.Equal(source, *sourceAfterUpdated)
// password is old
config = server.scheduler.GetSourceCfgByID(source.SourceName)
s.NotNil(config)
s.Equal(*s.testSource.Password, config.From.Password)

s.Nil(server.deleteSource(ctx, s.testSource.SourceName, false))
}
}

func (s *OpenAPIControllerSuite) TestTaskController() {
Expand Down
38 changes: 35 additions & 3 deletions dm/dm/master/openapi_view_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -372,7 +372,7 @@ func (s *OpenAPIViewSuite) TestTaskTemplatesAPI() {
SourceName: source1Name,
EnableGtid: false,
Host: dbCfg.Host,
Password: dbCfg.Password,
Password: &dbCfg.Password,
Port: dbCfg.Port,
User: dbCfg.User,
}
Expand Down Expand Up @@ -471,7 +471,7 @@ func (s *OpenAPIViewSuite) TestSourceAPI() {
Enable: true,
EnableGtid: false,
Host: dbCfg.Host,
Password: dbCfg.Password,
Password: &dbCfg.Password,
Port: dbCfg.Port,
User: dbCfg.User,
Purge: &openapi.Purge{Interval: &purgeInterVal},
Expand Down Expand Up @@ -702,6 +702,38 @@ func (s *OpenAPIViewSuite) TestSourceAPI() {
s.NoError(result.UnmarshalBodyToObject(&resultListSource2))
s.Len(resultListSource2.Data, 0)
s.Equal(0, resultListSource2.Total)

// create with no password
sourceNoPassword := source1
sourceNoPassword.Password = nil
createReqNoPassword := openapi.CreateSourceRequest{Source: sourceNoPassword}
result = testutil.NewRequest().Post(baseURL).WithJsonBody(createReqNoPassword).GoWithHTTPHandler(s.T(), s1.openapiHandles)
s.Equal(http.StatusCreated, result.Code())
s.NoError(result.UnmarshalBodyToObject(&resultSource))
s.Nil(resultSource.Password)

// update to have password
sourceHasPassword := source1
updateReqHasPassword := openapi.UpdateSourceRequest{Source: sourceHasPassword}
result = testutil.NewRequest().Put(source1URL).WithJsonBody(updateReqHasPassword).GoWithHTTPHandler(s.T(), s1.openapiHandles)
s.Equal(http.StatusOK, result.Code())
s.NoError(result.UnmarshalBodyToObject(&source1FromHTTP))
s.Equal(source1FromHTTP.Password, sourceHasPassword.Password)

// update with no password, will use old password
updateReqNoPassword := openapi.UpdateSourceRequest{Source: sourceNoPassword}
result = testutil.NewRequest().Put(source1URL).WithJsonBody(updateReqNoPassword).GoWithHTTPHandler(s.T(), s1.openapiHandles)
s.Equal(http.StatusOK, result.Code())
s.NoError(result.UnmarshalBodyToObject(&source1FromHTTP))
s.Nil(source1FromHTTP.Password)
// password is old
conf := s1.scheduler.GetSourceCfgByID(source1FromHTTP.SourceName)
s.NotNil(conf)
s.Equal(*sourceHasPassword.Password, conf.From.Password)

// delete source with --force
result = testutil.NewRequest().Delete(fmt.Sprintf("%s/%s?force=true", baseURL, source1.SourceName)).GoWithHTTPHandler(s.T(), s1.openapiHandles)
s.Equal(http.StatusNoContent, result.Code())
}

func (s *OpenAPIViewSuite) testImportTaskTemplate(task *openapi.Task, s1 *Server) {
Expand Down Expand Up @@ -774,7 +806,7 @@ func (s *OpenAPIViewSuite) TestTaskAPI() {
SourceName: source1Name,
EnableGtid: false,
Host: dbCfg.Host,
Password: dbCfg.Password,
Password: &dbCfg.Password,
Port: dbCfg.Port,
User: dbCfg.User,
}
Expand Down
4 changes: 2 additions & 2 deletions dm/dm/master/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -2316,7 +2316,7 @@ func (s *Server) GetCfg(ctx context.Context, req *pb.GetCfgRequest) (*pb.GetCfgR
// For the get-config command, we want to filter out fields that are not easily readable by humans,
// such as SSLXXBytes field in `Security` struct
taskCfg := config.SubTaskConfigsToTaskConfig(subCfgList...)
taskCfg.TargetDB.Password = "******"
taskCfg.TargetDB.Password = config.ObfuscatedPasswordForFeedback
if taskCfg.TargetDB.Security != nil {
taskCfg.TargetDB.Security.ClearSSLBytesData()
}
Expand Down Expand Up @@ -2420,7 +2420,7 @@ func (s *Server) GetCfg(ctx context.Context, req *pb.GetCfgRequest) (*pb.GetCfgR

return resp2, nil
}
sourceCfg.From.Password = "******"
sourceCfg.From.Password = config.ObfuscatedPasswordForFeedback
if sourceCfg.From.Security != nil {
sourceCfg.From.Security.ClearSSLBytesData()
}
Expand Down
119 changes: 60 additions & 59 deletions dm/openapi/gen.server.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion dm/openapi/gen.types.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 1 addition & 2 deletions dm/openapi/spec/dm.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -1376,7 +1376,7 @@ components:
type: string
example: "123456"
description: "source password"
nullable: false
nullable: true
WizardXiao marked this conversation as resolved.
Show resolved Hide resolved
enable_gtid:
type: boolean
example: false
Expand Down Expand Up @@ -1410,7 +1410,6 @@ components:
- "host"
- "port"
- "user"
- "password"
- "enable_gtid"
- "enable"
ShardingGroup:
Expand Down
16 changes: 16 additions & 0 deletions dm/tests/openapi/client/openapi_source_check
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,21 @@ def create_source2_success():
print("create_source1_success resp=", resp.json())
assert resp.status_code == 201

def update_source1_without_password_success():
req = {
"source": {
"case_sensitive": False,
"enable": True,
"enable_gtid": False,
"host": "127.0.0.1",
"port": 3306,
"source_name": SOURCE1_NAME,
"user": "root",
}
}
resp = requests.put(url=API_ENDPOINT + "/" + SOURCE1_NAME, json=req)
print("update_source1_without_password_success resp=", resp.json())
assert resp.status_code == 200

def list_source_success(source_count):
resp = requests.get(url=API_ENDPOINT)
Expand Down Expand Up @@ -220,6 +235,7 @@ if __name__ == "__main__":
"create_source_failed": create_source_failed,
"create_source1_success": create_source1_success,
"create_source2_success": create_source2_success,
"update_source1_without_password_success": update_source1_without_password_success,
"list_source_success": list_source_success,
"list_source_with_redirect": list_source_with_redirect,
"list_source_with_status_success": list_source_with_status_success,
Expand Down
3 changes: 3 additions & 0 deletions dm/tests/openapi/run.sh
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,9 @@ function test_source() {
# recreate source will failed
openapi_source_check "create_source_failed"

# update source1 without password success
openapi_source_check "update_source1_without_password_success"

# get source list success
openapi_source_check "list_source_success" 1

Expand Down