From 2c631d578730874a4edefe9d005fe9f7c291fd64 Mon Sep 17 00:00:00 2001 From: Chunzhu Li Date: Mon, 12 Aug 2019 14:38:40 +0800 Subject: [PATCH] Return error when loading some undefined configs from local --- arbiter/config.go | 5 ++-- arbiter/config_test.go | 47 +++++++++++++++++++++++++++++++++++ drainer/config.go | 4 +-- drainer/config_test.go | 37 ++++++++++++++++++++++++++++ pkg/util/util.go | 22 +++++++++++++++++ pump/config.go | 4 +-- pump/config_test.go | 56 +++++++++++++++++++++++++++++++----------- reparo/config.go | 5 ++-- reparo/config_test.go | 43 ++++++++++++++++++++++++++++++++ 9 files changed, 196 insertions(+), 27 deletions(-) create mode 100644 arbiter/config_test.go diff --git a/arbiter/config.go b/arbiter/config.go index dabe2cc25..538b10faf 100644 --- a/arbiter/config.go +++ b/arbiter/config.go @@ -6,10 +6,10 @@ import ( "fmt" "os" - "github.com/BurntSushi/toml" "github.com/ngaut/log" "github.com/pingcap/errors" "github.com/pingcap/tidb-binlog/pkg/flags" + "github.com/pingcap/tidb-binlog/pkg/util" "github.com/pingcap/tidb-binlog/pkg/version" ) @@ -172,6 +172,5 @@ func (cfg *Config) adjustConfig() error { } func (cfg *Config) configFromFile(path string) error { - _, err := toml.DecodeFile(path, cfg) - return errors.Trace(err) + return util.StrictDecodeFile(path, "arbiter", cfg) } diff --git a/arbiter/config_test.go b/arbiter/config_test.go new file mode 100644 index 000000000..9afabdaac --- /dev/null +++ b/arbiter/config_test.go @@ -0,0 +1,47 @@ +package arbiter + +import ( + "bytes" + "io/ioutil" + "path" + + "github.com/BurntSushi/toml" + "github.com/pingcap/check" +) + +type TestConfigSuite struct { +} + +var _ = check.Suite(&TestConfigSuite{}) + +func (t *TestConfigSuite) TestParseConfigFileWithInvalidArgs(c *check.C) { + yc := struct { + LogLevel string `toml:"log-level" json:"log-level"` + ListenAddr string `toml:"addr" json:"addr"` + LogFile string `toml:"log-file" json:"log-file"` + UnrecognizedOptionTest bool `toml:"unrecognized-option-test" json:"unrecognized-option-test"` + }{ + "debug", + "127.0.0.1:8251", + "/tmp/arbiter", + true, + } + + var buf bytes.Buffer + e := toml.NewEncoder(&buf) + err := e.Encode(yc) + c.Assert(err, check.IsNil) + + configFilename := path.Join(c.MkDir(), "arbiter_config_invalid.toml") + err = ioutil.WriteFile(configFilename, buf.Bytes(), 0644) + c.Assert(err, check.IsNil) + + args := []string{ + "--config", + configFilename, + } + + cfg := NewConfig() + err = cfg.Parse(args) + c.Assert(err, check.ErrorMatches, ".*contained unknown configuration options: unrecognized-option-test.*") +} diff --git a/drainer/config.go b/drainer/config.go index fbe1a0467..13454e5a9 100644 --- a/drainer/config.go +++ b/drainer/config.go @@ -12,7 +12,6 @@ import ( "strings" "time" - "github.com/BurntSushi/toml" "github.com/ngaut/log" "github.com/pingcap/errors" "github.com/pingcap/parser/mysql" @@ -207,8 +206,7 @@ func (c *SyncerConfig) adjustDoDBAndTable() { } func (cfg *Config) configFromFile(path string) error { - _, err := toml.DecodeFile(path, cfg) - return errors.Trace(err) + return util.StrictDecodeFile(path, "drainer", cfg) } func adjustString(v *string, defValue string) { diff --git a/drainer/config_test.go b/drainer/config_test.go index bc5b0d43d..5adff9d1d 100644 --- a/drainer/config_test.go +++ b/drainer/config_test.go @@ -1,8 +1,12 @@ package drainer import ( + "bytes" + "io/ioutil" + "path" "testing" + "github.com/BurntSushi/toml" . "github.com/pingcap/check" "github.com/pingcap/parser/mysql" "github.com/pingcap/tidb-binlog/pkg/util" @@ -90,3 +94,36 @@ func (t *testDrainerSuite) TestAdjustConfig(c *C) { c.Assert(cfg.ListenAddr, Equals, "http://0.0.0.0:8257") c.Assert(cfg.AdvertiseAddr, Equals, "http://192.168.15.12:8257") } + +func (t *testDrainerSuite) TestConfigParsingFileWithInvalidOptions(c *C) { + yc := struct { + DataDir string `toml:"data-dir" json:"data-dir"` + ListenAddr string `toml:"addr" json:"addr"` + AdvertiseAddr string `toml:"advertise-addr" json:"advertise-addr"` + UnrecognizedOptionTest bool `toml:"unrecognized-option-test" json:"unrecognized-option-test"` + }{ + "data.drainer", + "192.168.15.10:8257", + "192.168.15.10:8257", + true, + } + + var buf bytes.Buffer + e := toml.NewEncoder(&buf) + err := e.Encode(yc) + c.Assert(err, IsNil) + + configFilename := path.Join(c.MkDir(), "drainer_config_invalid.toml") + err = ioutil.WriteFile(configFilename, buf.Bytes(), 0644) + c.Assert(err, IsNil) + + args := []string{ + "--config", + configFilename, + "-L", "debug", + } + + cfg := NewConfig() + err = cfg.Parse(args) + c.Assert(err, ErrorMatches, ".*contained unknown configuration options: unrecognized-option-test.*") +} diff --git a/pkg/util/util.go b/pkg/util/util.go index 1a76edfed..7bb359345 100644 --- a/pkg/util/util.go +++ b/pkg/util/util.go @@ -3,8 +3,10 @@ package util import ( "fmt" "net" + "strings" "time" + "github.com/BurntSushi/toml" "github.com/ngaut/log" "github.com/pingcap/errors" "github.com/pingcap/parser/model" @@ -140,3 +142,23 @@ func QueryLatestTsFromPD(tiStore kv.Storage) (int64, error) { return int64(version.Ver), nil } + +// StrictDecodeFile decodes the toml file strictly. If any item in confFile file is not mapped +// into the Config struct, issue an error and stop the server from starting. +func StrictDecodeFile(path, component string, cfg interface{}) error { + metaData, err := toml.DecodeFile(path, cfg) + if err != nil { + return errors.Trace(err) + } + + if undecoded := metaData.Undecoded(); len(undecoded) > 0 { + var undecodedItems []string + for _, item := range undecoded { + undecodedItems = append(undecodedItems, item.String()) + } + err = errors.Errorf("component %s's config file %s contained unknown configuration options: %s", + component, path, strings.Join(undecodedItems, ", ")) + } + + return errors.Trace(err) +} diff --git a/pump/config.go b/pump/config.go index 203cd0738..074cf5c7d 100644 --- a/pump/config.go +++ b/pump/config.go @@ -10,7 +10,6 @@ import ( "strings" "time" - "github.com/BurntSushi/toml" "github.com/pingcap/errors" "github.com/pingcap/tidb-binlog/pkg/flags" "github.com/pingcap/tidb-binlog/pkg/security" @@ -161,8 +160,7 @@ func (cfg *Config) Parse(arguments []string) error { } func (cfg *Config) configFromFile(path string) error { - _, err := toml.DecodeFile(path, cfg) - return errors.Trace(err) + return util.StrictDecodeFile(path, "pump", cfg) } func adjustString(v *string, defValue string) { diff --git a/pump/config_test.go b/pump/config_test.go index a2f066917..d9c685cea 100644 --- a/pump/config_test.go +++ b/pump/config_test.go @@ -4,6 +4,7 @@ import ( "bytes" "io/ioutil" "os" + "path" "github.com/BurntSushi/toml" . "github.com/pingcap/check" @@ -62,6 +63,7 @@ func (s *testConfigSuite) TestConfigParsingEnvFlags(c *C) { os.Setenv("PUMP_ADDR", "192.168.199.200:9000") os.Setenv("PUMP_PD_URLS", "http://127.0.0.1:2379,http://localhost:2379") os.Setenv("PUMP_DATA_DIR", "/tmp/pump") + defer os.Clearenv() cfg := NewConfig() mustSuccess(c, cfg.Parse(args)) @@ -71,7 +73,7 @@ func (s *testConfigSuite) TestConfigParsingEnvFlags(c *C) { func (s *testConfigSuite) TestConfigParsingFileFlags(c *C) { yc := struct { ListenAddr string `toml:"addr" json:"addr"` - AdvertiseAddr string `toml:"advertiser-addr" json:"advertise-addr"` + AdvertiseAddr string `toml:"advertise-addr" json:"advertise-addr"` EtcdURLs string `toml:"pd-urls" json:"pd-urls"` BinlogDir string `toml:"data-dir" json:"data-dir"` HeartbeatInterval uint `toml:"heartbeat-interval" json:"heartbeat-interval"` @@ -88,36 +90,60 @@ func (s *testConfigSuite) TestConfigParsingFileFlags(c *C) { err := e.Encode(yc) c.Assert(err, IsNil) - tmpfile := mustCreateCfgFile(c, buf.Bytes(), "pump_config") - defer os.Remove(tmpfile.Name()) + configFilename := path.Join(c.MkDir(), "pump_config.toml") + err = ioutil.WriteFile(configFilename, buf.Bytes(), 0644) + c.Assert(err, IsNil) args := []string{ "--config", - tmpfile.Name(), + configFilename, "-L", "debug", } - os.Clearenv() cfg := NewConfig() mustSuccess(c, cfg.Parse(args)) validateConfig(c, cfg) } -func mustSuccess(c *C, err error) { +func (s *testConfigSuite) TestConfigParsingFileWithInvalidArgs(c *C) { + yc := struct { + ListenAddr string `toml:"addr" json:"addr"` + AdvertiseAddr string `toml:"advertise-addr" json:"advertise-addr"` + EtcdURLs string `toml:"pd-urls" json:"pd-urls"` + BinlogDir string `toml:"data-dir" json:"data-dir"` + HeartbeatInterval uint `toml:"heartbeat-interval" json:"heartbeat-interval"` + UnrecognizedOptionTest bool `toml:"unrecognized-option-test" json:"unrecognized-option-test"` + }{ + "192.168.199.100:8260", + "192.168.199.100:8260", + "http://192.168.199.110:2379,http://hostname:2379", + "/tmp/pump", + 1500, + true, + } + + var buf bytes.Buffer + e := toml.NewEncoder(&buf) + err := e.Encode(yc) c.Assert(err, IsNil) -} -func mustCreateCfgFile(c *C, b []byte, prefix string) *os.File { - tmpfile, err := ioutil.TempFile("", prefix) - mustSuccess(c, err) + configFilename := path.Join(c.MkDir(), "pump_config_invalid.toml") + err = ioutil.WriteFile(configFilename, buf.Bytes(), 0644) + c.Assert(err, IsNil) - _, err = tmpfile.Write(b) - mustSuccess(c, err) + args := []string{ + "--config", + configFilename, + "-L", "debug", + } - err = tmpfile.Close() - mustSuccess(c, err) + cfg := NewConfig() + err = cfg.Parse(args) + c.Assert(err, ErrorMatches, ".*contained unknown configuration options: unrecognized-option-test.*") +} - return tmpfile +func mustSuccess(c *C, err error) { + c.Assert(err, IsNil) } func validateConfig(c *C, cfg *Config) { diff --git a/reparo/config.go b/reparo/config.go index 115c3a0b3..c87914fac 100644 --- a/reparo/config.go +++ b/reparo/config.go @@ -8,11 +8,11 @@ import ( "strings" "time" - "github.com/BurntSushi/toml" "github.com/ngaut/log" "github.com/pingcap/errors" "github.com/pingcap/tidb-binlog/pkg/filter" "github.com/pingcap/tidb-binlog/pkg/flags" + "github.com/pingcap/tidb-binlog/pkg/util" "github.com/pingcap/tidb-binlog/pkg/version" "github.com/pingcap/tidb-binlog/reparo/syncer" "github.com/pingcap/tidb/store/tikv/oracle" @@ -154,8 +154,7 @@ func (c *Config) adjustDoDBAndTable() { } func (c *Config) configFromFile(path string) error { - _, err := toml.DecodeFile(path, c) - return errors.Trace(err) + return util.StrictDecodeFile(path, "reparo", c) } func (c *Config) validate() error { diff --git a/reparo/config_test.go b/reparo/config_test.go index 3a25b0ac9..6889a20dd 100644 --- a/reparo/config_test.go +++ b/reparo/config_test.go @@ -1,10 +1,13 @@ package reparo import ( + "bytes" "fmt" + "io/ioutil" "path" "runtime" + "github.com/BurntSushi/toml" "github.com/pingcap/check" ) @@ -20,6 +23,46 @@ func (s *testConfigSuite) TestParseTemplateConfig(c *check.C) { c.Assert(err, check.IsNil, check.Commentf("arg: %s", arg)) } +func (s *testConfigSuite) TestParseConfigFileWithInvalidArgs(c *check.C) { + yc := struct { + Dir string `toml:"data-dir" json:"data-dir"` + StartDatetime string `toml:"start-datetime" json:"start-datetime"` + StopDatetime string `toml:"stop-datetime" json:"stop-datetime"` + StartTSO int64 `toml:"start-tso" json:"start-tso"` + StopTSO int64 `toml:"stop-tso" json:"stop-tso"` + LogFile string `toml:"log-file" json:"log-file"` + LogLevel string `toml:"log-level" json:"log-level"` + UnrecognizedOptionTest bool `toml:"unrecognized-option-test" json:"unrecognized-option-test"` + }{ + "/tmp/reparo", + "", + "", + 0, + 0, + "tmp/reparo/reparo.log", + "debug", + true, + } + + var buf bytes.Buffer + e := toml.NewEncoder(&buf) + err := e.Encode(yc) + c.Assert(err, check.IsNil) + + configFilename := path.Join(c.MkDir(), "reparo_config_invalid.toml") + err = ioutil.WriteFile(configFilename, buf.Bytes(), 0644) + c.Assert(err, check.IsNil) + + args := []string{ + "--config", + configFilename, + } + + cfg := NewConfig() + err = cfg.Parse(args) + c.Assert(err, check.ErrorMatches, ".*contained unknown configuration options: unrecognized-option-test.*") +} + func getTemplateConfigFilePath() string { // we put the template config file in "cmd/reapro/reparo.toml" _, filename, _, _ := runtime.Caller(0)