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

master/worker/task add strictly config verification #212

Merged
merged 4 commits into from
Jul 24, 2019
Merged

master/worker/task add strictly config verification #212

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
51834f7
add task config verify
3pointer Jul 22, 2019
7d5c0eb
remove useless test config
3pointer Jul 22, 2019
42566c1
add decodefile tests
3pointer Jul 23, 2019
c65fc5a
remove useless config in dm-ansible
3pointer Jul 23, 2019
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: 2 additions & 2 deletions dm/config/task.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -294,7 +294,7 @@ func (c *TaskConfig) DecodeFile(fpath string) error {
return errors.Annotatef(err, "read config file %v", fpath)
}

err = yaml.Unmarshal(bs, c)
err = yaml.UnmarshalStrict(bs, c)
Copy link
Contributor

Choose a reason for hiding this comment

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

we can add one more test to cover DecodeFile with invalid yaml config

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

if err != nil {
return errors.Trace(err)
}
Expand All @@ -304,7 +304,7 @@ func (c *TaskConfig) DecodeFile(fpath string) error {

// Decode loads config from file data
func (c *TaskConfig) Decode(data string) error {
err := yaml.Unmarshal([]byte(data), c)
err := yaml.UnmarshalStrict([]byte(data), c)
if err != nil {
return errors.Trace(err)
}
Expand Down
84 changes: 84 additions & 0 deletions dm/config/task_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
// Copyright 2019 PingCAP, Inc.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// See the License for the specific language governing permissions and
// limitations under the License.

package config

import (
. "github.com/pingcap/check"
)

var errorTaskConfig1 = `---
name: test
task-mode: all
is-sharding: true
meta-schema: "dm_meta"
remove-meta: false
enable-heartbeat: true
timezone: "Asia/Shanghai"
ignore-checking-items: ["all"]

target-database:
host: "127.0.0.1"
port: 4000
user: "root"
password: ""

mysql-instances:
- source-id: "mysql-replica-01"
server-id: 101
black-white-list: "instance"
route-rules: ["sharding-route-rules-table", "sharding-route-rules-schema"]
column-mapping-rules: ["instance-1"]
mydumper-config-name: "global"
loader-config-name: "global"
syncer-config-name: "global"
`
var errorTaskConfig2 = `---
name: test
name: test1
task-mode: all
is-sharding: true
meta-schema: "dm_meta"
remove-meta: false
enable-heartbeat: true
timezone: "Asia/Shanghai"
ignore-checking-items: ["all"]

target-database:
host: "127.0.0.1"
port: 4000
user: "root"
password: ""

mysql-instances:
- source-id: "mysql-replica-01"
black-white-list: "instance"
route-rules: ["sharding-route-rules-table", "sharding-route-rules-schema"]
column-mapping-rules: ["instance-1"]
mydumper-config-name: "global"
loader-config-name: "global"
syncer-config-name: "global"
`

func (t *testConfig) TestInvalidTaskConfig(c *C) {
taskConfig := NewTaskConfig()
err := taskConfig.Decode(errorTaskConfig1)
// field server-id is not a member of TaskConfig
c.Check(err, NotNil)
c.Assert(err, ErrorMatches, "*line 19: found character that cannot start any token*")

err = taskConfig.Decode(errorTaskConfig2)
// field name duplicate
c.Check(err, NotNil)
c.Assert(err, ErrorMatches, "*line 3: field name already set in type config.TaskConfig*")
}
10 changes: 9 additions & 1 deletion dm/master/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,15 @@ func (c *Config) Parse(arguments []string) error {

// configFromFile loads config from file.
func (c *Config) configFromFile(path string) error {
_, err := toml.DecodeFile(path, c)
metaData, err := toml.DecodeFile(path, c)
undecoded := metaData.Undecoded()
if len(undecoded) > 0 && err == nil {
var undecodedItems []string
for _, item := range undecoded {
undecodedItems = append(undecodedItems, item.String())
}
return errors.Errorf("master config contained unknown configuration options: %s", strings.Join(undecodedItems, ","))
}
return errors.Trace(err)
}

Expand Down
13 changes: 13 additions & 0 deletions dm/master/config_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -181,4 +181,17 @@ dm-worker = "172.16.10.72:8262"`)
cfg.ConfigFile = filepath
err = cfg.Reload()
c.Assert(err, check.NotNil)

filepath2 := path.Join(c.MkDir(), "test_invalid_config.toml")
// field still remain undecoded in config will cause verify failed
configContent2 := []byte(`
master-addr = ":8261"
aaa = "xxx"
[[deploy]]
dm-worker = "172.16.10.72:8262"`)
err = ioutil.WriteFile(filepath2, configContent2, 0644)
err = cfg.configFromFile(filepath2)
c.Assert(err, check.NotNil)
c.Assert(err, check.ErrorMatches, "*master config contained unknown configuration options: aaa*")
}
2 changes: 0 additions & 2 deletions dm/master/server_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,6 @@ target-database:
mysql-instances:
- source-id: "mysql-replica-01"
server-id: 101
black-white-list: "instance"
route-rules: ["sharding-route-rules-table", "sharding-route-rules-schema"]
column-mapping-rules: ["instance-1"]
Expand All @@ -61,7 +60,6 @@ mysql-instances:
syncer-config-name: "global"
- source-id: "mysql-replica-02"
server-id: 102
black-white-list: "instance"
route-rules: ["sharding-route-rules-table", "sharding-route-rules-schema"]
column-mapping-rules: ["instance-2"]
Expand Down
11 changes: 10 additions & 1 deletion dm/worker/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,16 @@ func (c *Config) verify() error {

// configFromFile loads config from file.
func (c *Config) configFromFile(path string) error {
_, err := toml.DecodeFile(path, c)
metaData, err := toml.DecodeFile(path, c)
undecoded := metaData.Undecoded()
if len(undecoded) > 0 && err == nil {
var undecodedItems []string
for _, item := range undecoded {
undecodedItems = append(undecodedItems, item.String())
}
return errors.Errorf("worker config contained unknown configuration options: %s", strings.Join(undecodedItems, ","))
}

if err != nil {
return errors.Trace(err)
}
Expand Down
14 changes: 14 additions & 0 deletions dm/worker/config_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ package worker

import (
"fmt"
"io/ioutil"
"path"
"strings"

Expand Down Expand Up @@ -73,6 +74,19 @@ func (t *testServer) TestConfig(c *C) {
clone3, err := cfg.DecryptPassword()
c.Assert(err, IsNil)
c.Assert(clone3, DeepEquals, cfg)

// test invalid config
dir2 := c.MkDir()
configFile := path.Join(dir2, "dm-worker-invalid.toml")
configContent := []byte(`
worker-addr = ":8262"
aaa = "xxx"
`)
err = ioutil.WriteFile(configFile, configContent, 0644)
c.Assert(err, IsNil)
err = cfg.configFromFile(configFile)
c.Assert(err, NotNil)
c.Assert(err, ErrorMatches, "*worker config contained unknown configuration options: aaa*")
}

func (t *testServer) TestConfigVerify(c *C) {
Expand Down
2 changes: 0 additions & 2 deletions tests/all_mode/conf/dm-task.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,12 @@ target-database:

mysql-instances:
- source-id: "mysql-replica-01"
server-id: 101
black-white-list: "instance"
mydumper-config-name: "global"
loader-config-name: "global"
syncer-config-name: "global"

- source-id: "mysql-replica-02"
server-id: 102
black-white-list: "instance"
mydumper-config-name: "global"
loader-config-name: "global"
Expand Down
1 change: 0 additions & 1 deletion tests/all_mode/conf/dm-worker1.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
server-id = 101
source-id = "mysql-replica-01"
flavor = "mysql"
meta-file = "relay.meta"
enable-gtid = false
relay-binlog-name = ""
relay-binlog-gtid = ""
Expand Down
1 change: 0 additions & 1 deletion tests/all_mode/conf/dm-worker2.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
server-id = 102
source-id = "mysql-replica-02"
flavor = "mysql"
meta-file = "relay.meta"
enable-gtid = false
relay-binlog-name = ""
relay-binlog-gtid = ""
Expand Down
2 changes: 0 additions & 2 deletions tests/dmctl_basic/conf/dm-task.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ target-database:

mysql-instances:
- source-id: "mysql-replica-01"
server-id: 101
black-white-list: "instance"
route-rules: ["sharding-route-rules-table", "sharding-route-rules-schema"]
column-mapping-rules: ["instance-1"]
Expand All @@ -24,7 +23,6 @@ mysql-instances:
syncer-config-name: "global"

- source-id: "mysql-replica-02"
server-id: 102
black-white-list: "instance"
route-rules: ["sharding-route-rules-table", "sharding-route-rules-schema"]
column-mapping-rules: ["instance-2"]
Expand Down
1 change: 0 additions & 1 deletion tests/dmctl_basic/conf/dm-worker1.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
server-id = 101
source-id = "mysql-replica-01"
flavor = "mysql"
meta-file = "relay.meta"
enable-gtid = false
relay-binlog-name = ""
relay-binlog-gtid = ""
Expand Down
1 change: 0 additions & 1 deletion tests/dmctl_basic/conf/dm-worker2.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
server-id = 102
source-id = "mysql-replica-02"
flavor = "mysql"
meta-file = "relay.meta"
enable-gtid = false
relay-binlog-name = ""
relay-binlog-gtid = ""
Expand Down
2 changes: 0 additions & 2 deletions tests/incremental_mode/conf/dm-task.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ target-database:

mysql-instances:
- source-id: "mysql-replica-01"
server-id: 101
meta:
binlog-name: binlog-name-placeholder-1
binlog-pos: binlog-pos-placeholder-1
Expand All @@ -27,7 +26,6 @@ mysql-instances:
syncer-config-name: "global"

- source-id: "mysql-replica-02"
server-id: 102
meta:
binlog-name: binlog-name-placeholder-2
binlog-pos: binlog-pos-placeholder-2
Expand Down
1 change: 0 additions & 1 deletion tests/incremental_mode/conf/dm-worker1.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
server-id = 101
source-id = "mysql-replica-01"
flavor = "mysql"
meta-file = "relay.meta"
enable-gtid = false
relay-binlog-name = ""
relay-binlog-gtid = ""
Expand Down
1 change: 0 additions & 1 deletion tests/incremental_mode/conf/dm-worker2.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
server-id = 102
source-id = "mysql-replica-02"
flavor = "mysql"
meta-file = "relay.meta"
enable-gtid = false
relay-binlog-name = ""
relay-binlog-gtid = ""
Expand Down
2 changes: 0 additions & 2 deletions tests/load_interrupt/conf/dm-task.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,12 @@ target-database:

mysql-instances:
- source-id: "mysql-replica-01"
server-id: 101
black-white-list: "instance"
mydumper-config-name: "global"
loader-config-name: "global"
syncer-config-name: "global"

- source-id: "mysql-replica-02"
server-id: 102
black-white-list: "instance"
mydumper-config-name: "global"
loader-config-name: "global"
Expand Down
1 change: 0 additions & 1 deletion tests/load_interrupt/conf/dm-worker1.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
server-id = 101
source-id = "mysql-replica-01"
flavor = "mysql"
meta-file = "relay.meta"
enable-gtid = false
relay-binlog-name = ""
relay-binlog-gtid = ""
Expand Down
1 change: 0 additions & 1 deletion tests/load_interrupt/conf/dm-worker2.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
server-id = 102
source-id = "mysql-replica-02"
flavor = "mysql"
meta-file = "relay.meta"
enable-gtid = false
relay-binlog-name = ""
relay-binlog-gtid = ""
Expand Down
2 changes: 0 additions & 2 deletions tests/online_ddl/conf/dm-task.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ target-database:

mysql-instances:
- source-id: "mysql-replica-01"
server-id: 101
black-white-list: "instance"
route-rules: ["sharding-route-rules-table", "sharding-route-rules-schema"]
column-mapping-rules: ["instance-1"]
Expand All @@ -25,7 +24,6 @@ mysql-instances:
syncer-config-name: "global"

- source-id: "mysql-replica-02"
server-id: 102
meta:
binlog-name: binlog.000001
binlog-pos: 4
Expand Down
1 change: 0 additions & 1 deletion tests/online_ddl/conf/dm-worker1.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
server-id = 101
source-id = "mysql-replica-01"
flavor = "mysql"
meta-file = "relay.meta"
enable-gtid = false
relay-binlog-name = ""
relay-binlog-gtid = ""
Expand Down
1 change: 0 additions & 1 deletion tests/online_ddl/conf/dm-worker2.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
server-id = 102
source-id = "mysql-replica-02"
flavor = "mysql"
meta-file = "relay.meta"
enable-gtid = false
relay-binlog-name = ""
relay-binlog-gtid = ""
Expand Down
1 change: 0 additions & 1 deletion tests/print_status/conf/dm-task.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ target-database:

mysql-instances:
- source-id: "mysql-replica-01"
server-id: 101
black-white-list: "instance"
mydumper-config-name: "global"
loader-config-name: "global"
Expand Down
1 change: 0 additions & 1 deletion tests/print_status/conf/dm-worker1.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
server-id = 101
source-id = "mysql-replica-01"
flavor = "mysql"
meta-file = "relay.meta"
enable-gtid = false
relay-binlog-name = ""
relay-binlog-gtid = ""
Expand Down
3 changes: 0 additions & 3 deletions tests/safe_mode/conf/dm-task.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ target-database:

mysql-instances:
- source-id: "mysql-replica-01"
server-id: 101
black-white-list: "instance"
route-rules: ["sharding-route-rules-table", "sharding-route-rules-schema"]
column-mapping-rules: ["instance-1"]
Expand All @@ -24,7 +23,6 @@ mysql-instances:
syncer-config-name: "global"

- source-id: "mysql-replica-02"
server-id: 102
black-white-list: "instance"
route-rules: ["sharding-route-rules-table", "sharding-route-rules-schema"]
column-mapping-rules: ["instance-2"]
Expand Down Expand Up @@ -79,7 +77,6 @@ loaders:
global:
pool-size: 16
dir: "./dumped_data"
extra-args: "-B safe_mode_test"

syncers:
global:
Expand Down
Loading