Skip to content

Commit

Permalink
cluster/spec: ignore changes of variable fields when validating
Browse files Browse the repository at this point in the history
  • Loading branch information
AstroProfundis committed Jul 20, 2020
1 parent ec7f801 commit 7f6e6f9
Show file tree
Hide file tree
Showing 11 changed files with 84 additions and 25 deletions.
18 changes: 13 additions & 5 deletions pkg/cluster/edit/diff.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,9 @@ import (
)

const (
validateTagName = "validate"
validateTagValue = "editable"
validateTagName = "validate"
validateTagEditable = "editable"
validateTagIgnore = "ignore"
)

// ShowDiff write diff result into the Writer.
Expand Down Expand Up @@ -60,22 +61,29 @@ func ValidateSpecDiff(s1, s2 interface{}) error {
for _, c := range changelog {
if len(c.Path) > 0 {
// c.Path will be the tag value if TagName matched on the field
if c.Type == diff.UPDATE && c.Path[len(c.Path)-1] == validateTagValue {
if c.Type == diff.UPDATE && c.Path[len(c.Path)-1] == validateTagEditable {
// If the field is marked as editable, it is allowed to be modified no matter
// its parent level element is marked as editable or not
continue
}

pathEditable := true
pathIgnore := false
for _, p := range c.Path {
if _, err := strconv.Atoi(p); err == nil {
// ignore slice offset counts
continue
}
if p != validateTagValue {
if p == validateTagIgnore {
pathIgnore = true
continue
}
if p != validateTagEditable {
pathEditable = false
}
}
if pathEditable && (c.Type == diff.CREATE || c.Type == diff.DELETE) {
// if the path has any ignorable item, just ignore it
if pathIgnore || pathEditable && (c.Type == diff.CREATE || c.Type == diff.DELETE) {
// If *every* parent elements on the path are all marked as editable,
// AND the field itself is marked as editable, it is allowed to add or delete
continue
Expand Down
53 changes: 52 additions & 1 deletion pkg/cluster/edit/diff_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ func TestDiff(t *testing.T) {
type sampleDataMeta struct {
IntSlice []int `yaml:"ints,omitempty"`
StrSlice []string `yaml:"strs,omitempty" validate:"editable"`
MapSlice []map[string]interface{} `yaml:"maps,omitempty"`
MapSlice []map[string]interface{} `yaml:"maps,omitempty" validate:"ignore"`
StrElem string `yaml:"stre" validate:"editable"`
StructSlice1 []sampleDataElem `yaml:"slice1" validate:"editable"`
StructSlice2 []sampleDataElem `yaml:"slice2,omitempty"`
Expand Down Expand Up @@ -428,3 +428,54 @@ slice2:
err = ValidateSpecDiff(d1, d2)
c.Assert(err, IsNil)
}

func (d *diffSuite) TestValidateSpecDiff6(c *C) {
var d1 sampleDataMeta
var d2 sampleDataMeta
var err error

err = yaml.Unmarshal([]byte(`
ints: [11, 12, 13]
maps:
- key0: 0
- dot.key1: 1
- dotkey.subkey.1: "1"
`), &d1)
c.Assert(err, IsNil)

// Modify key without dot in name, in ignorable slice
err = yaml.Unmarshal([]byte(`
ints: [11, 12, 13]
maps:
- key0: 1
- dot.key1: 1
- dotkey.subkey.1: "1"
`), &d2)
c.Assert(err, IsNil)
err = ValidateSpecDiff(d1, d2)
c.Assert(err, IsNil)

// Modify key with one dot in name, in ignorable slice
err = yaml.Unmarshal([]byte(`
ints: [11, 12, 13]
maps:
- key0: 0
- dot.key1: 11
- dotkey.subkey.1: "1"
`), &d2)
c.Assert(err, IsNil)
err = ValidateSpecDiff(d1, d2)
c.Assert(err, IsNil)

// Modify key with two dots and number in name, in ignorable slice
err = yaml.Unmarshal([]byte(`
ints: [11, 12, 13]
maps:
- key0: 0
- dot.key1: 1
- dotkey.subkey.1: "12"
`), &d2)
c.Assert(err, IsNil)
err = ValidateSpecDiff(d1, d2)
c.Assert(err, IsNil)
}
2 changes: 1 addition & 1 deletion pkg/cluster/spec/cdc.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ type CDCSpec struct {
LogDir string `yaml:"log_dir,omitempty"`
Offline bool `yaml:"offline,omitempty"`
NumaNode string `yaml:"numa_node,omitempty" validate:"editable"`
Config map[string]interface{} `yaml:"config,omitempty" validate:"editable"`
Config map[string]interface{} `yaml:"config,omitempty" validate:"ignore"`
ResourceControl meta.ResourceControl `yaml:"resource_control,omitempty" validate:"editable"`
Arch string `yaml:"arch,omitempty"`
OS string `yaml:"os,omitempty"`
Expand Down
2 changes: 1 addition & 1 deletion pkg/cluster/spec/drainer.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ type DrainerSpec struct {
CommitTS int64 `yaml:"commit_ts,omitempty"`
Offline bool `yaml:"offline,omitempty"`
NumaNode string `yaml:"numa_node,omitempty" validate:"editable"`
Config map[string]interface{} `yaml:"config,omitempty" validate:"editable"`
Config map[string]interface{} `yaml:"config,omitempty" validate:"ignore"`
ResourceControl meta.ResourceControl `yaml:"resource_control,omitempty" validate:"editable"`
Arch string `yaml:"arch,omitempty"`
OS string `yaml:"os,omitempty"`
Expand Down
2 changes: 1 addition & 1 deletion pkg/cluster/spec/pd.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ type PDSpec struct {
DataDir string `yaml:"data_dir,omitempty"`
LogDir string `yaml:"log_dir,omitempty"`
NumaNode string `yaml:"numa_node,omitempty" validate:"editable"`
Config map[string]interface{} `yaml:"config,omitempty" validate:"editable"`
Config map[string]interface{} `yaml:"config,omitempty" validate:"ignore"`
ResourceControl meta.ResourceControl `yaml:"resource_control,omitempty" validate:"editable"`
Arch string `yaml:"arch,omitempty"`
OS string `yaml:"os,omitempty"`
Expand Down
2 changes: 1 addition & 1 deletion pkg/cluster/spec/pump.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ type PumpSpec struct {
LogDir string `yaml:"log_dir,omitempty"`
Offline bool `yaml:"offline,omitempty"`
NumaNode string `yaml:"numa_node,omitempty" validate:"editable"`
Config map[string]interface{} `yaml:"config,omitempty" validate:"editable"`
Config map[string]interface{} `yaml:"config,omitempty" validate:"ignore"`
ResourceControl meta.ResourceControl `yaml:"resource_control,omitempty" validate:"editable"`
Arch string `yaml:"arch,omitempty"`
OS string `yaml:"os,omitempty"`
Expand Down
18 changes: 9 additions & 9 deletions pkg/cluster/spec/spec.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,21 +78,21 @@ type (

// ServerConfigs represents the server runtime configuration
ServerConfigs struct {
TiDB map[string]interface{} `yaml:"tidb" validate:"editable"`
TiKV map[string]interface{} `yaml:"tikv" validate:"editable"`
PD map[string]interface{} `yaml:"pd" validate:"editable"`
TiFlash map[string]interface{} `yaml:"tiflash" validate:"editable"`
TiFlashLearner map[string]interface{} `yaml:"tiflash-learner" validate:"editable"`
Pump map[string]interface{} `yaml:"pump" validate:"editable"`
Drainer map[string]interface{} `yaml:"drainer" validate:"editable"`
CDC map[string]interface{} `yaml:"cdc" validate:"editable"`
TiDB map[string]interface{} `yaml:"tidb"`
TiKV map[string]interface{} `yaml:"tikv"`
PD map[string]interface{} `yaml:"pd"`
TiFlash map[string]interface{} `yaml:"tiflash"`
TiFlashLearner map[string]interface{} `yaml:"tiflash-learner"`
Pump map[string]interface{} `yaml:"pump"`
Drainer map[string]interface{} `yaml:"drainer"`
CDC map[string]interface{} `yaml:"cdc"`
}

// Specification represents the specification of topology.yaml
Specification struct {
GlobalOptions GlobalOptions `yaml:"global,omitempty" validate:"editable"`
MonitoredOptions MonitoredOptions `yaml:"monitored,omitempty" validate:"editable"`
ServerConfigs ServerConfigs `yaml:"server_configs,omitempty" validate:"editable"`
ServerConfigs ServerConfigs `yaml:"server_configs,omitempty" validate:"ignore"`
TiDBServers []TiDBSpec `yaml:"tidb_servers"`
TiKVServers []TiKVSpec `yaml:"tikv_servers"`
TiFlashServers []TiFlashSpec `yaml:"tiflash_servers"`
Expand Down
2 changes: 1 addition & 1 deletion pkg/cluster/spec/tidb.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ type TiDBSpec struct {
DeployDir string `yaml:"deploy_dir,omitempty"`
LogDir string `yaml:"log_dir,omitempty"`
NumaNode string `yaml:"numa_node,omitempty" validate:"editable"`
Config map[string]interface{} `yaml:"config,omitempty" validate:"editable"`
Config map[string]interface{} `yaml:"config,omitempty" validate:"ignore"`
ResourceControl meta.ResourceControl `yaml:"resource_control,omitempty" validate:"editable"`
Arch string `yaml:"arch,omitempty"`
OS string `yaml:"os,omitempty"`
Expand Down
2 changes: 1 addition & 1 deletion pkg/cluster/spec/tiflash.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ type TiFlashSpec struct {
TmpDir string `yaml:"tmp_path,omitempty"`
Offline bool `yaml:"offline,omitempty"`
NumaNode string `yaml:"numa_node,omitempty" validate:"editable"`
Config map[string]interface{} `yaml:"config,omitempty" validate:"editable"`
Config map[string]interface{} `yaml:"config,omitempty" validate:"ignore"`
LearnerConfig map[string]interface{} `yaml:"learner_config,omitempty" validate:"editable"`
ResourceControl meta.ResourceControl `yaml:"resource_control,omitempty" validate:"editable"`
Arch string `yaml:"arch,omitempty"`
Expand Down
2 changes: 1 addition & 1 deletion pkg/cluster/spec/tikv.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ type TiKVSpec struct {
LogDir string `yaml:"log_dir,omitempty"`
Offline bool `yaml:"offline,omitempty"`
NumaNode string `yaml:"numa_node,omitempty" validate:"editable"`
Config map[string]interface{} `yaml:"config,omitempty" validate:"editable"`
Config map[string]interface{} `yaml:"config,omitempty" validate:"ignore"`
ResourceControl meta.ResourceControl `yaml:"resource_control,omitempty" validate:"editable"`
Arch string `yaml:"arch,omitempty"`
OS string `yaml:"os,omitempty"`
Expand Down
6 changes: 3 additions & 3 deletions pkg/dm/spec/topology_dm.go
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ type MasterSpec struct {
DataDir string `yaml:"data_dir,omitempty"`
LogDir string `yaml:"log_dir,omitempty"`
NumaNode string `yaml:"numa_node,omitempty" validate:"editable"`
Config map[string]interface{} `yaml:"config,omitempty" validate:"editable"`
Config map[string]interface{} `yaml:"config,omitempty" validate:"ignore"`
ResourceControl ResourceControl `yaml:"resource_control,omitempty" validate:"editable"`
Arch string `yaml:"arch,omitempty"`
OS string `yaml:"os,omitempty"`
Expand Down Expand Up @@ -183,7 +183,7 @@ type WorkerSpec struct {
DataDir string `yaml:"data_dir,omitempty"`
LogDir string `yaml:"log_dir,omitempty"`
NumaNode string `yaml:"numa_node,omitempty" validate:"editable"`
Config map[string]interface{} `yaml:"config,omitempty" validate:"editable"`
Config map[string]interface{} `yaml:"config,omitempty" validate:"ignore"`
ResourceControl ResourceControl `yaml:"resource_control,omitempty" validate:"editable"`
Arch string `yaml:"arch,omitempty"`
OS string `yaml:"os,omitempty"`
Expand Down Expand Up @@ -236,7 +236,7 @@ type PortalSpec struct {
LogDir string `yaml:"log_dir,omitempty"`
Timeout int `yaml:"timeout" default:"5"`
NumaNode string `yaml:"numa_node,omitempty" validate:"editable"`
Config map[string]interface{} `yaml:"config,omitempty" validate:"editable"`
Config map[string]interface{} `yaml:"config,omitempty" validate:"ignore"`
ResourceControl ResourceControl `yaml:"resource_control,omitempty" validate:"editable"`
Arch string `yaml:"arch,omitempty"`
OS string `yaml:"os,omitempty"`
Expand Down

0 comments on commit 7f6e6f9

Please sign in to comment.