diff --git a/components/cluster/command/edit_config.go b/components/cluster/command/edit_config.go index 6bfa7bcb4f..0954c4a6b5 100644 --- a/components/cluster/command/edit_config.go +++ b/components/cluster/command/edit_config.go @@ -23,11 +23,11 @@ import ( "github.com/fatih/color" perrs "github.com/pingcap/errors" "github.com/pingcap/tiup/pkg/cliutil" - "github.com/pingcap/tiup/pkg/cluster/edit" "github.com/pingcap/tiup/pkg/cluster/spec" "github.com/pingcap/tiup/pkg/logger" "github.com/pingcap/tiup/pkg/logger/log" "github.com/pingcap/tiup/pkg/meta" + tiuputils "github.com/pingcap/tiup/pkg/utils" "github.com/spf13/cobra" "gopkg.in/yaml.v2" ) @@ -93,7 +93,7 @@ func editTopo(clusterName string, metadata *spec.ClusterMeta) error { return perrs.AddStack(err) } - err = edit.OpenFileInEditor(name) + err = tiuputils.OpenFileInEditor(name) if err != nil { return perrs.AddStack(err) } @@ -116,7 +116,11 @@ func editTopo(clusterName string, metadata *spec.ClusterMeta) error { return nil } - edit.ShowDiff(string(data), string(newData), os.Stdout) + // report error if immutable field has been changed + if err := tiuputils.ValidateSpecDiff(metadata.Topology, newTopo); err != nil { + return err + } + tiuputils.ShowDiff(string(data), string(newData), os.Stdout) if !skipConfirm { if err := cliutil.PromptForConfirmOrAbortError( diff --git a/components/dm/command/edit_config.go b/components/dm/command/edit_config.go index 505ab47794..70ca6ecf36 100644 --- a/components/dm/command/edit_config.go +++ b/components/dm/command/edit_config.go @@ -24,7 +24,6 @@ import ( "github.com/fatih/color" perrs "github.com/pingcap/errors" "github.com/pingcap/tiup/pkg/cliutil" - "github.com/pingcap/tiup/pkg/cluster/edit" "github.com/pingcap/tiup/pkg/cluster/meta" "github.com/pingcap/tiup/pkg/logger" "github.com/pingcap/tiup/pkg/logger/log" @@ -87,7 +86,7 @@ func editTopo(clusterName string, metadata *meta.DMMeta) error { return perrs.AddStack(err) } - err = edit.OpenFileInEditor(name) + err = tiuputils.OpenFileInEditor(name) if err != nil { return perrs.AddStack(err) } @@ -110,7 +109,7 @@ func editTopo(clusterName string, metadata *meta.DMMeta) error { return nil } - edit.ShowDiff(string(data), string(newData), os.Stdout) + tiuputils.ShowDiff(string(data), string(newData), os.Stdout) if !skipConfirm { if err := cliutil.PromptForConfirmOrAbortError( diff --git a/go.mod b/go.mod index ccb7d68938..ffb45d3c8b 100644 --- a/go.mod +++ b/go.mod @@ -48,6 +48,7 @@ require ( github.com/pingcap/kvproto v0.0.0-20200518112156-d4aeb467de29 github.com/pingcap/pd/v4 v4.0.0 github.com/pingcap/tidb-insight v0.3.1 + github.com/r3labs/diff v0.0.0-20200627101315-aecd9dd05dd2 github.com/relex/aini v1.1.3 github.com/sergi/go-diff v1.0.1-0.20180205163309-da645544ed44 github.com/shirou/gopsutil v2.20.3+incompatible diff --git a/go.sum b/go.sum index 7a4380f694..6998089501 100644 --- a/go.sum +++ b/go.sum @@ -669,6 +669,8 @@ github.com/prometheus/procfs v0.0.5/go.mod h1:4A/X28fw3Fc593LaREMrKMqOKvUAntwMDa github.com/prometheus/procfs v0.0.6 h1:0qbH+Yqu/cj1ViVLvEWCP6qMQ4efWUj6bQqOEA0V0U4= github.com/prometheus/procfs v0.0.6/go.mod h1:7Qr8sr6344vo1JqZ6HhLceV9o3AJ1Ff+GxbHq6oeK9A= github.com/prometheus/tsdb v0.7.1/go.mod h1:qhTCs0VvXwvX/y3TZrWD7rabWM+ijKTux40TwIPHuXU= +github.com/r3labs/diff v0.0.0-20200627101315-aecd9dd05dd2 h1:786HUIrynbbk5PzUf9Rp3aAUkNRksUiiipSAlyJ68As= +github.com/r3labs/diff v0.0.0-20200627101315-aecd9dd05dd2/go.mod h1:7WjXasNzi0vJetRcB/RqNl5dlIsmXcTTLmF5IoH6Xig= github.com/rakyll/statik v0.1.6/go.mod h1:OEi9wJV/fMUAGx1eNjq75DKDsJVuEv1U0oYdX6GX8Zs= github.com/rcrowley/go-metrics v0.0.0-20190826022208-cac0b30c2563/go.mod h1:bCqnVzQkZxMG4s8nGwiZ5l3QUCyqpo9Y+/ZMZ9VjZe4= github.com/relex/aini v1.1.3 h1:xooajfI84UYB+jWdxKe2w1zXXdi89Fc+L6W4CjHb0wg= diff --git a/pkg/cluster/edit/diff.go b/pkg/cluster/edit/diff.go deleted file mode 100644 index be6c8d480d..0000000000 --- a/pkg/cluster/edit/diff.go +++ /dev/null @@ -1,31 +0,0 @@ -// Copyright 2020 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 edit - -import ( - "fmt" - "io" - - "github.com/sergi/go-diff/diffmatchpatch" -) - -// ShowDiff write diff result into the Writer. -// return false if there's no diff. -func ShowDiff(t1 string, t2 string, w io.Writer) { - dmp := diffmatchpatch.New() - diffs := dmp.DiffMain(t1, t2, false) - diffs = dmp.DiffCleanupSemantic(diffs) - - fmt.Fprint(w, dmp.DiffPrettyText(diffs)) -} diff --git a/pkg/cluster/spec/alertmanager.go b/pkg/cluster/spec/alertmanager.go index e047991511..4f1c0b86b0 100644 --- a/pkg/cluster/spec/alertmanager.go +++ b/pkg/cluster/spec/alertmanager.go @@ -26,15 +26,15 @@ import ( // AlertManagerSpec represents the AlertManager topology specification in topology.yaml type AlertManagerSpec struct { Host string `yaml:"host"` - SSHPort int `yaml:"ssh_port,omitempty"` + SSHPort int `yaml:"ssh_port,omitempty" validate:"ssh_port:editable"` Imported bool `yaml:"imported,omitempty"` WebPort int `yaml:"web_port" default:"9093"` ClusterPort int `yaml:"cluster_port" default:"9094"` DeployDir string `yaml:"deploy_dir,omitempty"` DataDir string `yaml:"data_dir,omitempty"` LogDir string `yaml:"log_dir,omitempty"` - NumaNode string `yaml:"numa_node,omitempty"` - ResourceControl meta.ResourceControl `yaml:"resource_control,omitempty"` + NumaNode string `yaml:"numa_node,omitempty" validate:"numa_node:editable"` + ResourceControl meta.ResourceControl `yaml:"resource_control,omitempty" validate:"resource_control:editable"` Arch string `yaml:"arch,omitempty"` OS string `yaml:"os,omitempty"` } diff --git a/pkg/cluster/spec/cdc.go b/pkg/cluster/spec/cdc.go index 8dfd7ac14b..f4d689fca3 100644 --- a/pkg/cluster/spec/cdc.go +++ b/pkg/cluster/spec/cdc.go @@ -25,15 +25,15 @@ import ( // CDCSpec represents the Drainer topology specification in topology.yaml type CDCSpec struct { Host string `yaml:"host"` - SSHPort int `yaml:"ssh_port,omitempty"` + SSHPort int `yaml:"ssh_port,omitempty" validate:"ssh_port:editable"` Imported bool `yaml:"imported,omitempty"` Port int `yaml:"port" default:"8300"` DeployDir string `yaml:"deploy_dir,omitempty"` LogDir string `yaml:"log_dir,omitempty"` Offline bool `yaml:"offline,omitempty"` - NumaNode string `yaml:"numa_node,omitempty"` - Config map[string]interface{} `yaml:"config,omitempty"` - ResourceControl meta.ResourceControl `yaml:"resource_control,omitempty"` + NumaNode string `yaml:"numa_node,omitempty" validate:"numa_node:editable"` + Config map[string]interface{} `yaml:"config,omitempty" validate:"config:ignore"` + ResourceControl meta.ResourceControl `yaml:"resource_control,omitempty" validate:"resource_control:editable"` Arch string `yaml:"arch,omitempty"` OS string `yaml:"os,omitempty"` } diff --git a/pkg/cluster/spec/drainer.go b/pkg/cluster/spec/drainer.go index 24b7a87da0..f7763a89d3 100644 --- a/pkg/cluster/spec/drainer.go +++ b/pkg/cluster/spec/drainer.go @@ -27,7 +27,7 @@ import ( // DrainerSpec represents the Drainer topology specification in topology.yaml type DrainerSpec struct { Host string `yaml:"host"` - SSHPort int `yaml:"ssh_port,omitempty"` + SSHPort int `yaml:"ssh_port,omitempty" validate:"ssh_port:editable"` Imported bool `yaml:"imported,omitempty"` Port int `yaml:"port" default:"8249"` DeployDir string `yaml:"deploy_dir,omitempty"` @@ -35,9 +35,9 @@ type DrainerSpec struct { LogDir string `yaml:"log_dir,omitempty"` CommitTS int64 `yaml:"commit_ts,omitempty"` Offline bool `yaml:"offline,omitempty"` - NumaNode string `yaml:"numa_node,omitempty"` - Config map[string]interface{} `yaml:"config,omitempty"` - ResourceControl meta.ResourceControl `yaml:"resource_control,omitempty"` + NumaNode string `yaml:"numa_node,omitempty" validate:"numa_node:editable"` + Config map[string]interface{} `yaml:"config,omitempty" validate:"config:ignore"` + ResourceControl meta.ResourceControl `yaml:"resource_control,omitempty" validate:"resource_control:editable"` Arch string `yaml:"arch,omitempty"` OS string `yaml:"os,omitempty"` } diff --git a/pkg/cluster/spec/grafana.go b/pkg/cluster/spec/grafana.go index a1e8924423..24ffbe7057 100644 --- a/pkg/cluster/spec/grafana.go +++ b/pkg/cluster/spec/grafana.go @@ -27,11 +27,11 @@ import ( // GrafanaSpec represents the Grafana topology specification in topology.yaml type GrafanaSpec struct { Host string `yaml:"host"` - SSHPort int `yaml:"ssh_port,omitempty"` + SSHPort int `yaml:"ssh_port,omitempty" validate:"ssh_port:editable"` Imported bool `yaml:"imported,omitempty"` Port int `yaml:"port" default:"3000"` DeployDir string `yaml:"deploy_dir,omitempty"` - ResourceControl meta.ResourceControl `yaml:"resource_control,omitempty"` + ResourceControl meta.ResourceControl `yaml:"resource_control,omitempty" validate:"resource_control:editable"` Arch string `yaml:"arch,omitempty"` OS string `yaml:"os,omitempty"` } diff --git a/pkg/cluster/spec/pd.go b/pkg/cluster/spec/pd.go index b216945ba8..28d96026a7 100644 --- a/pkg/cluster/spec/pd.go +++ b/pkg/cluster/spec/pd.go @@ -31,7 +31,7 @@ import ( type PDSpec struct { Host string `yaml:"host"` ListenHost string `yaml:"listen_host,omitempty"` - SSHPort int `yaml:"ssh_port,omitempty"` + SSHPort int `yaml:"ssh_port,omitempty" validate:"ssh_port:editable"` Imported bool `yaml:"imported,omitempty"` // Use Name to get the name with a default value if it's empty. Name string `yaml:"name"` @@ -40,9 +40,9 @@ type PDSpec struct { DeployDir string `yaml:"deploy_dir,omitempty"` DataDir string `yaml:"data_dir,omitempty"` LogDir string `yaml:"log_dir,omitempty"` - NumaNode string `yaml:"numa_node,omitempty"` - Config map[string]interface{} `yaml:"config,omitempty"` - ResourceControl meta.ResourceControl `yaml:"resource_control,omitempty"` + NumaNode string `yaml:"numa_node,omitempty" validate:"numa_node:editable"` + Config map[string]interface{} `yaml:"config,omitempty" validate:"config:ignore"` + ResourceControl meta.ResourceControl `yaml:"resource_control,omitempty" validate:"resource_control:editable"` Arch string `yaml:"arch,omitempty"` OS string `yaml:"os,omitempty"` } diff --git a/pkg/cluster/spec/prometheus.go b/pkg/cluster/spec/prometheus.go index 3d270b30bc..47c9a513b1 100644 --- a/pkg/cluster/spec/prometheus.go +++ b/pkg/cluster/spec/prometheus.go @@ -27,15 +27,15 @@ import ( // PrometheusSpec represents the Prometheus Server topology specification in topology.yaml type PrometheusSpec struct { Host string `yaml:"host"` - SSHPort int `yaml:"ssh_port,omitempty"` + SSHPort int `yaml:"ssh_port,omitempty" validate:"ssh_port:editable"` Imported bool `yaml:"imported,omitempty"` Port int `yaml:"port" default:"9090"` DeployDir string `yaml:"deploy_dir,omitempty"` DataDir string `yaml:"data_dir,omitempty"` LogDir string `yaml:"log_dir,omitempty"` - NumaNode string `yaml:"numa_node,omitempty"` - Retention string `yaml:"storage_retention,omitempty"` - ResourceControl meta.ResourceControl `yaml:"resource_control,omitempty"` + NumaNode string `yaml:"numa_node,omitempty" validate:"numa_node:editable"` + Retention string `yaml:"storage_retention,omitempty" validate:"storage_retention:editable"` + ResourceControl meta.ResourceControl `yaml:"resource_control,omitempty" validate:"resource_control:editable"` Arch string `yaml:"arch,omitempty"` OS string `yaml:"os,omitempty"` } diff --git a/pkg/cluster/spec/pump.go b/pkg/cluster/spec/pump.go index 2c83e0fa14..3e5581774c 100644 --- a/pkg/cluster/spec/pump.go +++ b/pkg/cluster/spec/pump.go @@ -27,16 +27,16 @@ import ( // PumpSpec represents the Pump topology specification in topology.yaml type PumpSpec struct { Host string `yaml:"host"` - SSHPort int `yaml:"ssh_port,omitempty"` + SSHPort int `yaml:"ssh_port,omitempty" validate:"ssh_port:editable"` Imported bool `yaml:"imported,omitempty"` Port int `yaml:"port" default:"8250"` DeployDir string `yaml:"deploy_dir,omitempty"` DataDir string `yaml:"data_dir,omitempty"` LogDir string `yaml:"log_dir,omitempty"` Offline bool `yaml:"offline,omitempty"` - NumaNode string `yaml:"numa_node,omitempty"` - Config map[string]interface{} `yaml:"config,omitempty"` - ResourceControl meta.ResourceControl `yaml:"resource_control"` + NumaNode string `yaml:"numa_node,omitempty" validate:"numa_node:editable"` + Config map[string]interface{} `yaml:"config,omitempty" validate:"config:ignore"` + ResourceControl meta.ResourceControl `yaml:"resource_control,omitempty" validate:"resource_control:editable"` Arch string `yaml:"arch,omitempty"` OS string `yaml:"os,omitempty"` } diff --git a/pkg/cluster/spec/spec.go b/pkg/cluster/spec/spec.go index a94cdcd202..8aba666439 100644 --- a/pkg/cluster/spec/spec.go +++ b/pkg/cluster/spec/spec.go @@ -56,11 +56,11 @@ type ( // specification in topology.yaml GlobalOptions struct { User string `yaml:"user,omitempty" default:"tidb"` - SSHPort int `yaml:"ssh_port,omitempty" default:"22"` + SSHPort int `yaml:"ssh_port,omitempty" default:"22" validate:"ssh_port:editable"` DeployDir string `yaml:"deploy_dir,omitempty" default:"deploy"` DataDir string `yaml:"data_dir,omitempty" default:"data"` LogDir string `yaml:"log_dir,omitempty"` - ResourceControl meta.ResourceControl `yaml:"resource_control,omitempty"` + ResourceControl meta.ResourceControl `yaml:"resource_control,omitempty" validate:"resource_control:editable"` OS string `yaml:"os,omitempty" default:"linux"` Arch string `yaml:"arch,omitempty" default:"amd64"` } @@ -72,8 +72,8 @@ type ( DeployDir string `yaml:"deploy_dir,omitempty"` DataDir string `yaml:"data_dir,omitempty"` LogDir string `yaml:"log_dir,omitempty"` - NumaNode string `yaml:"numa_node,omitempty"` - ResourceControl meta.ResourceControl `yaml:"resource_control,omitempty"` + NumaNode string `yaml:"numa_node,omitempty" validate:"numa_node:editable"` + ResourceControl meta.ResourceControl `yaml:"resource_control,omitempty" validate:"resource_control:editable"` } // ServerConfigs represents the server runtime configuration @@ -90,9 +90,9 @@ type ( // Specification represents the specification of topology.yaml Specification struct { - GlobalOptions GlobalOptions `yaml:"global,omitempty"` - MonitoredOptions MonitoredOptions `yaml:"monitored,omitempty"` - ServerConfigs ServerConfigs `yaml:"server_configs,omitempty"` + GlobalOptions GlobalOptions `yaml:"global,omitempty" validate:"global:editable"` + MonitoredOptions MonitoredOptions `yaml:"monitored,omitempty" validate:"monitored:editable"` + ServerConfigs ServerConfigs `yaml:"server_configs,omitempty" validate:"server_configs:ignore"` TiDBServers []TiDBSpec `yaml:"tidb_servers"` TiKVServers []TiKVSpec `yaml:"tikv_servers"` TiFlashServers []TiFlashSpec `yaml:"tiflash_servers"` diff --git a/pkg/cluster/spec/tidb.go b/pkg/cluster/spec/tidb.go index b2a603f9ce..c193d396ba 100644 --- a/pkg/cluster/spec/tidb.go +++ b/pkg/cluster/spec/tidb.go @@ -28,15 +28,15 @@ import ( type TiDBSpec struct { Host string `yaml:"host"` ListenHost string `yaml:"listen_host,omitempty"` - SSHPort int `yaml:"ssh_port,omitempty"` + SSHPort int `yaml:"ssh_port,omitempty" validate:"ssh_port:editable"` Imported bool `yaml:"imported,omitempty"` Port int `yaml:"port" default:"4000"` StatusPort int `yaml:"status_port" default:"10080"` DeployDir string `yaml:"deploy_dir,omitempty"` LogDir string `yaml:"log_dir,omitempty"` - NumaNode string `yaml:"numa_node,omitempty"` - Config map[string]interface{} `yaml:"config,omitempty"` - ResourceControl meta.ResourceControl `yaml:"resource_control,omitempty"` + NumaNode string `yaml:"numa_node,omitempty" validate:"numa_node:editable"` + Config map[string]interface{} `yaml:"config,omitempty" validate:"config:ignore"` + ResourceControl meta.ResourceControl `yaml:"resource_control,omitempty" validate:"resource_control:editable"` Arch string `yaml:"arch,omitempty"` OS string `yaml:"os,omitempty"` } diff --git a/pkg/cluster/spec/tiflash.go b/pkg/cluster/spec/tiflash.go index e49a827a86..52f6997006 100644 --- a/pkg/cluster/spec/tiflash.go +++ b/pkg/cluster/spec/tiflash.go @@ -34,7 +34,7 @@ import ( // TiFlashSpec represents the TiFlash topology specification in topology.yaml type TiFlashSpec struct { Host string `yaml:"host"` - SSHPort int `yaml:"ssh_port,omitempty"` + SSHPort int `yaml:"ssh_port,omitempty" validate:"ssh_port:editable"` Imported bool `yaml:"imported,omitempty"` TCPPort int `yaml:"tcp_port" default:"9000"` HTTPPort int `yaml:"http_port" default:"8123"` @@ -47,10 +47,10 @@ type TiFlashSpec struct { LogDir string `yaml:"log_dir,omitempty"` TmpDir string `yaml:"tmp_path,omitempty"` Offline bool `yaml:"offline,omitempty"` - NumaNode string `yaml:"numa_node,omitempty"` - Config map[string]interface{} `yaml:"config,omitempty"` - LearnerConfig map[string]interface{} `yaml:"learner_config,omitempty"` - ResourceControl meta.ResourceControl `yaml:"resource_control,omitempty"` + NumaNode string `yaml:"numa_node,omitempty" validate:"numa_node:editable"` + Config map[string]interface{} `yaml:"config,omitempty" validate:"config:ignore"` + LearnerConfig map[string]interface{} `yaml:"learner_config,omitempty" validate:"learner_config:editable"` + ResourceControl meta.ResourceControl `yaml:"resource_control,omitempty" validate:"resource_control:editable"` Arch string `yaml:"arch,omitempty"` OS string `yaml:"os,omitempty"` } diff --git a/pkg/cluster/spec/tikv.go b/pkg/cluster/spec/tikv.go index 552cd7f7d0..7051db9e95 100644 --- a/pkg/cluster/spec/tikv.go +++ b/pkg/cluster/spec/tikv.go @@ -30,7 +30,7 @@ import ( type TiKVSpec struct { Host string `yaml:"host"` ListenHost string `yaml:"listen_host,omitempty"` - SSHPort int `yaml:"ssh_port,omitempty"` + SSHPort int `yaml:"ssh_port,omitempty" validate:"ssh_port:editable"` Imported bool `yaml:"imported,omitempty"` Port int `yaml:"port" default:"20160"` StatusPort int `yaml:"status_port" default:"20180"` @@ -38,9 +38,9 @@ type TiKVSpec struct { DataDir string `yaml:"data_dir,omitempty"` LogDir string `yaml:"log_dir,omitempty"` Offline bool `yaml:"offline,omitempty"` - NumaNode string `yaml:"numa_node,omitempty"` - Config map[string]interface{} `yaml:"config,omitempty"` - ResourceControl meta.ResourceControl `yaml:"resource_control,omitempty"` + NumaNode string `yaml:"numa_node,omitempty" validate:"numa_node:editable"` + Config map[string]interface{} `yaml:"config,omitempty" validate:"config:ignore"` + ResourceControl meta.ResourceControl `yaml:"resource_control,omitempty" validate:"resource_control:editable"` Arch string `yaml:"arch,omitempty"` OS string `yaml:"os,omitempty"` } diff --git a/pkg/cluster/spec/tispark.go b/pkg/cluster/spec/tispark.go index 7cabca5ec9..0f8274c496 100644 --- a/pkg/cluster/spec/tispark.go +++ b/pkg/cluster/spec/tispark.go @@ -32,13 +32,13 @@ import ( // TiSparkMasterSpec is the topology specification for TiSpark master node type TiSparkMasterSpec struct { Host string `yaml:"host"` - SSHPort int `yaml:"ssh_port,omitempty"` + SSHPort int `yaml:"ssh_port,omitempty" validate:"ssh_port:editable"` Imported bool `yaml:"imported,omitempty"` Port int `yaml:"port" default:"7077"` WebPort int `yaml:"web_port" default:"8080"` DeployDir string `yaml:"deploy_dir,omitempty"` - SparkConfigs map[string]interface{} `yaml:"spark_config,omitempty"` - SparkEnvs map[string]string `yaml:"spark_env,omitempty"` + SparkConfigs map[string]interface{} `yaml:"spark_config,omitempty" validate:"spark_config:editable"` + SparkEnvs map[string]string `yaml:"spark_env,omitempty" validate:"spark_env:editable"` Arch string `yaml:"arch,omitempty"` OS string `yaml:"os,omitempty"` } @@ -72,7 +72,7 @@ func (s TiSparkMasterSpec) Status(pdList ...string) string { // TiSparkWorkerSpec is the topology specification for TiSpark slave nodes type TiSparkWorkerSpec struct { Host string `yaml:"host"` - SSHPort int `yaml:"ssh_port,omitempty"` + SSHPort int `yaml:"ssh_port,omitempty" validate:"ssh_port:editable"` Imported bool `yaml:"imported,omitempty"` Port int `yaml:"port" default:"7078"` WebPort int `yaml:"web_port" default:"8081"` diff --git a/pkg/dm/spec/topology_dm.go b/pkg/dm/spec/topology_dm.go index 3e48b6337c..468d5bb072 100644 --- a/pkg/dm/spec/topology_dm.go +++ b/pkg/dm/spec/topology_dm.go @@ -112,7 +112,7 @@ func AllDMComponentNames() (roles []string) { // MasterSpec represents the Master topology specification in topology.yaml type MasterSpec struct { Host string `yaml:"host"` - SSHPort int `yaml:"ssh_port,omitempty"` + SSHPort int `yaml:"ssh_port,omitempty" validate:"ssh_port:editable"` Imported bool `yaml:"imported,omitempty"` // Use Name to get the name with a default value if it's empty. Name string `yaml:"name"` @@ -121,9 +121,9 @@ type MasterSpec struct { DeployDir string `yaml:"deploy_dir,omitempty"` DataDir string `yaml:"data_dir,omitempty"` LogDir string `yaml:"log_dir,omitempty"` - NumaNode string `yaml:"numa_node,omitempty"` - Config map[string]interface{} `yaml:"config,omitempty"` - ResourceControl ResourceControl `yaml:"resource_control,omitempty"` + NumaNode string `yaml:"numa_node,omitempty" validate:"numa_node:editable"` + Config map[string]interface{} `yaml:"config,omitempty" validate:"config:ignore"` + ResourceControl ResourceControl `yaml:"resource_control,omitempty" validate:"resource_control:editable"` Arch string `yaml:"arch,omitempty"` OS string `yaml:"os,omitempty"` } @@ -174,7 +174,7 @@ func (s MasterSpec) IsImported() bool { // WorkerSpec represents the Master topology specification in topology.yaml type WorkerSpec struct { Host string `yaml:"host"` - SSHPort int `yaml:"ssh_port,omitempty"` + SSHPort int `yaml:"ssh_port,omitempty" validate:"ssh_port:editable"` Imported bool `yaml:"imported,omitempty"` // Use Name to get the name with a default value if it's empty. Name string `yaml:"name"` @@ -182,9 +182,9 @@ type WorkerSpec struct { DeployDir string `yaml:"deploy_dir,omitempty"` DataDir string `yaml:"data_dir,omitempty"` LogDir string `yaml:"log_dir,omitempty"` - NumaNode string `yaml:"numa_node,omitempty"` - Config map[string]interface{} `yaml:"config,omitempty"` - ResourceControl ResourceControl `yaml:"resource_control,omitempty"` + NumaNode string `yaml:"numa_node,omitempty" validate:"numa_node:editable"` + Config map[string]interface{} `yaml:"config,omitempty" validate:"config:ignore"` + ResourceControl ResourceControl `yaml:"resource_control,omitempty" validate:"resource_control:editable"` Arch string `yaml:"arch,omitempty"` OS string `yaml:"os,omitempty"` } @@ -228,16 +228,16 @@ func (s WorkerSpec) IsImported() bool { // PortalSpec represents the Portal topology specification in topology.yaml type PortalSpec struct { Host string `yaml:"host"` - SSHPort int `yaml:"ssh_port,omitempty"` + SSHPort int `yaml:"ssh_port,omitempty" validate:"ssh_port:editable"` Imported bool `yaml:"imported,omitempty"` Port int `yaml:"port" default:"8280"` DeployDir string `yaml:"deploy_dir,omitempty"` DataDir string `yaml:"data_dir,omitempty"` LogDir string `yaml:"log_dir,omitempty"` Timeout int `yaml:"timeout" default:"5"` - NumaNode string `yaml:"numa_node,omitempty"` - Config map[string]interface{} `yaml:"config,omitempty"` - ResourceControl ResourceControl `yaml:"resource_control,omitempty"` + NumaNode string `yaml:"numa_node,omitempty" validate:"numa_node:editable"` + Config map[string]interface{} `yaml:"config,omitempty" validate:"config:ignore"` + ResourceControl ResourceControl `yaml:"resource_control,omitempty" validate:"resource_control:editable"` Arch string `yaml:"arch,omitempty"` OS string `yaml:"os,omitempty"` } diff --git a/pkg/meta/resource_ctrl.go b/pkg/meta/resource_ctrl.go index d7423e1fc4..1b9aa22a2d 100644 --- a/pkg/meta/resource_ctrl.go +++ b/pkg/meta/resource_ctrl.go @@ -16,8 +16,8 @@ package meta // ResourceControl is used to control the system resource // See: https://www.freedesktop.org/software/systemd/man/systemd.resource-control.html type ResourceControl struct { - MemoryLimit string `yaml:"memory_limit,omitempty"` - CPUQuota string `yaml:"cpu_quota,omitempty"` - IOReadBandwidthMax string `yaml:"io_read_bandwidth_max,omitempty"` - IOWriteBandwidthMax string `yaml:"io_write_bandwidth_max,omitempty"` + MemoryLimit string `yaml:"memory_limit,omitempty" validate:"memory_limit:editable"` + CPUQuota string `yaml:"cpu_quota,omitempty" validate:"cpu_quota:editable"` + IOReadBandwidthMax string `yaml:"io_read_bandwidth_max,omitempty" validate:"io_read_bandwidth_max:editable"` + IOWriteBandwidthMax string `yaml:"io_write_bandwidth_max,omitempty" validate:"io_write_bandwidth_max:editable"` } diff --git a/pkg/repository/v1manifest/types_test.go b/pkg/repository/v1manifest/types_test.go index dd2a7cc6bc..f57093167e 100644 --- a/pkg/repository/v1manifest/types_test.go +++ b/pkg/repository/v1manifest/types_test.go @@ -22,8 +22,8 @@ import ( func TestComponentList(t *testing.T) { manifest := &Index{ Components: map[string]ComponentItem{ - "comp1": ComponentItem{}, - "comp2": ComponentItem{Yanked: true}, + "comp1": {}, + "comp2": {Yanked: true}, }, } diff --git a/pkg/utils/diff.go b/pkg/utils/diff.go new file mode 100644 index 0000000000..2c7e7dc3ed --- /dev/null +++ b/pkg/utils/diff.go @@ -0,0 +1,144 @@ +// Copyright 2020 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 utils + +import ( + "fmt" + "io" + "strconv" + "strings" + + "github.com/r3labs/diff" + "github.com/sergi/go-diff/diffmatchpatch" +) + +const ( + validateTagName = "validate" + validateTagEditable = "editable" + validateTagIgnore = "ignore" + // r3labs/diff drops everything after the first ',' in the tag value, so we use a different + // seperator for the tag value and its options + validateTagSeperator = ":" +) + +// ShowDiff write diff result into the Writer. +// return false if there's no diff. +func ShowDiff(t1 string, t2 string, w io.Writer) { + dmp := diffmatchpatch.New() + diffs := dmp.DiffMain(t1, t2, false) + diffs = dmp.DiffCleanupSemantic(diffs) + + fmt.Fprint(w, dmp.DiffPrettyText(diffs)) +} + +// ValidateSpecDiff checks and validates the new spec to see if the modified +// keys are all marked as editable +func ValidateSpecDiff(s1, s2 interface{}) error { + differ, err := diff.NewDiffer( + diff.TagName(validateTagName), + ) + if err != nil { + return err + } + changelog, err := differ.Diff(s1, s2) + if err != nil { + return err + } + + if len(changelog) == 0 { + return nil + } + + msg := make([]string, 0) + for _, c := range changelog { + if len(c.Path) > 0 { + _, leafCtl := parseValidateTagValue(c.Path[len(c.Path)-1]) + // c.Path will be the tag value if TagName matched on the field + if c.Type == diff.UPDATE && leafCtl == 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 { + key, ctl := parseValidateTagValue(p) + if _, err := strconv.Atoi(key); err == nil { + // ignore slice offset counts + continue + } + if ctl == validateTagIgnore { + pathIgnore = true + continue + } + if ctl != validateTagEditable { + pathEditable = false + } + } + // 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 + } + } + + // build error messages + switch c.Type { + case diff.CREATE: + msg = append(msg, fmt.Sprintf("added %s with value '%v'", buildFieldPath(c.Path), c.To)) + case diff.DELETE: + msg = append(msg, fmt.Sprintf("removed %s with value '%v'", buildFieldPath(c.Path), c.From)) + case diff.UPDATE: + msg = append(msg, fmt.Sprintf("%s changed from '%v' to '%v'", buildFieldPath(c.Path), c.From, c.To)) + } + } + + if len(msg) > 0 { + return fmt.Errorf("immutable field changed: %s", strings.Join(msg, ", ")) + } + return nil +} + +func parseValidateTagValue(v string) (string, string) { + key := "" + ctl := "" + pvs := strings.Split(v, validateTagSeperator) + switch len(pvs) { + case 1: + // if only one field is set in tag value + // use it as both the field name and control command + key = pvs[0] + ctl = pvs[0] + case 2: + key = pvs[0] + ctl = pvs[1] + default: + panic(fmt.Sprintf("invalid tag value %s for %s, only one or two fields allowed", v, validateTagName)) + } + + return key, ctl +} + +func buildFieldPath(rawPath []string) string { + namedPath := make([]string, 0) + for _, p := range rawPath { + pvs := strings.Split(p, validateTagSeperator) + if len(pvs) >= 1 { + namedPath = append(namedPath, pvs[0]) + } + } + return strings.Join(namedPath, ".") +} diff --git a/pkg/utils/diff_test.go b/pkg/utils/diff_test.go new file mode 100644 index 0000000000..24539e20bf --- /dev/null +++ b/pkg/utils/diff_test.go @@ -0,0 +1,475 @@ +// 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 utils + +import ( + . "github.com/pingcap/check" + "gopkg.in/yaml.v2" +) + +type diffSuite struct { +} + +var _ = Suite(&diffSuite{}) + +type sampleDataMeta struct { + IntSlice []int `yaml:"ints,omitempty"` + StrSlice []string `yaml:"strs,omitempty" validate:"strs:editable"` + MapSlice []map[string]interface{} `yaml:"maps,omitempty" validate:"maps:ignore"` + StrElem string `yaml:"stre" validate:"editable"` + StructSlice1 []sampleDataElem `yaml:"slice1" validate:"slice1:editable"` + StructSlice2 []sampleDataElem `yaml:"slice2,omitempty"` + StructSlice3 []sampleDataEditable `yaml:"slice3,omitempty" validate:"slice3:editable"` +} + +type sampleDataElem struct { + StrElem1 string `yaml:"str1" validate:"str1:editable"` + StrElem2 string `yaml:"str2,omitempty" validate:"str2:editable"` + IntElem int `yaml:"int"` + InterfaceElem interface{} `yaml:"interface,omitempty" validate:"interface:editable"` + InterfaceSlice []map[string]interface{} `yaml:"mapslice,omitempty" validate:"mapslice:editable"` +} + +type sampleDataEditable struct { + StrElem1 string `yaml:"str1" validate:"str1:editable"` + StrElem2 string `yaml:"str2,omitempty" validate:"str2:editable"` +} + +func (d *diffSuite) TestValidateSpecDiff1(c *C) { + var d1 sampleDataMeta + var d2 sampleDataMeta + var err error + + err = yaml.Unmarshal([]byte(` +ints: [11, 12, 13] +strs: + - str1 + - "str2" +`), &d1) + c.Assert(err, IsNil) + // unchanged + err = ValidateSpecDiff(d1, d1) + c.Assert(err, IsNil) + + // swap element order + err = yaml.Unmarshal([]byte(` +ints: [11, 13, 12] +strs: + - str2 + - "str1" +`), &d2) + c.Assert(err, IsNil) + err = ValidateSpecDiff(d1, d2) + c.Assert(err, IsNil) + + // add editable element (without specifing alias) + err = yaml.Unmarshal([]byte(` +ints: [11, 13, 12] +strs: + - "str1" + - str2 +stre: "test1.3" +`), &d2) + c.Assert(err, IsNil) + err = ValidateSpecDiff(d1, d2) + c.Assert(err, IsNil) + + // add item to immutable element + err = yaml.Unmarshal([]byte(` +ints: [11, 12, 13, 14] +`), &d2) + c.Assert(err, IsNil) + err = ValidateSpecDiff(d1, d2) + c.Assert(err, NotNil) + c.Assert(err.Error(), Equals, "immutable field changed: added IntSlice.3 with value '14'") +} + +func (d *diffSuite) TestValidateSpecDiff2(c *C) { + var d1 sampleDataMeta + var d2 sampleDataMeta + var err error + + err = yaml.Unmarshal([]byte(` +ints: [11, 12, 13] +slice1: + - str1: strv11 + str2: strv21 + int: 42 + interface: 11 + - str1: strv12 + str2: strv22 + int: 42 + interface: "12" +`), &d1) + c.Assert(err, IsNil) + + // change editable filed of item in editable slice + err = yaml.Unmarshal([]byte(` +ints: [11, 12, 13] +slice1: + - str1: strv11 + str2: strv233 + int: 42 + interface: 11 + - str1: strv12 + str2: strv22 + int: 42 + interface: "12" +`), &d2) + c.Assert(err, IsNil) + err = ValidateSpecDiff(d1, d2) + c.Assert(err, IsNil) + + // change immutable filed of item in editable slice + err = yaml.Unmarshal([]byte(` +ints: [11, 12, 13] +slice1: + - str1: strv11 + str2: strv21 + int: 42 + interface: 11 + - str1: strv12 + str2: strv22 + int: 43 + interface: "12" +`), &d2) + c.Assert(err, IsNil) + err = ValidateSpecDiff(d1, d2) + c.Assert(err, NotNil) + c.Assert(err.Error(), Equals, "immutable field changed: slice1.1.IntElem changed from '42' to '43'") + + // Add item with immutable field to editable slice + err = yaml.Unmarshal([]byte(` +ints: [11, 12, 13] +slice1: + - str1: strv11 + str2: strv21 + int: 42 + interface: 11 + - str1: strv12 + str2: strv22 + int: 42 + interface: "12" + - str1: strv13 + str2: strv23 + int: 42 + interface: "13" +`), &d2) + c.Assert(err, IsNil) + err = ValidateSpecDiff(d1, d2) + c.Assert(err, NotNil) + c.Assert(err.Error(), Equals, "immutable field changed: added slice1.2.IntElem with value '42'") + + // Delete item with immutable field from editable slice + err = yaml.Unmarshal([]byte(` +ints: [11, 12, 13] +slice1: + - str1: strv11 + str2: strv21 + int: 42 + interface: 11 +`), &d2) + c.Assert(err, IsNil) + err = ValidateSpecDiff(d1, d2) + c.Assert(err, NotNil) + c.Assert(err.Error(), Equals, "immutable field changed: removed slice1.1.IntElem with value '42'") +} + +func (d *diffSuite) TestValidateSpecDiff3(c *C) { + var d1 sampleDataMeta + var d2 sampleDataMeta + var err error + + err = yaml.Unmarshal([]byte(` +ints: [11, 12, 13] +slice2: + - str1: strv11 + str2: strv21 + int: 42 + interface: 11 + - str1: strv12 + str2: strv22 + int: 42 + interface: "12" +`), &d1) + c.Assert(err, IsNil) + + // change editable filed of item in immutable slice + err = yaml.Unmarshal([]byte(` +ints: [11, 12, 13] +slice2: + - str1: strv11 + str2: strv233 + int: 42 + interface: 11 + - str1: strv12 + str2: strv22 + int: 42 + interface: "12" +`), &d2) + c.Assert(err, IsNil) + err = ValidateSpecDiff(d1, d2) + c.Assert(err, IsNil) + + // change immutable filed of item in immutable slice + err = yaml.Unmarshal([]byte(` +ints: [11, 12, 13] +slice2: + - str1: strv11 + str2: strv21 + int: 42 + interface: 11 + - str1: strv12 + str2: strv22 + int: 43 + interface: "12" +`), &d2) + c.Assert(err, IsNil) + err = ValidateSpecDiff(d1, d2) + c.Assert(err, NotNil) + c.Assert(err.Error(), Equals, "immutable field changed: StructSlice2.1.IntElem changed from '42' to '43'") + + // Add item to immutable slice + err = yaml.Unmarshal([]byte(` +ints: [11, 12, 13] +slice2: + - str1: strv11 + str2: strv21 + int: 42 + interface: 11 + - str1: strv12 + str2: strv22 + int: 42 + interface: "12" + - str1: strv31 + str2: strv32 +`), &d2) + c.Assert(err, IsNil) + err = ValidateSpecDiff(d1, d2) + c.Assert(err, NotNil) + c.Assert(err.Error(), Equals, "immutable field changed: added StructSlice2.2.str1 with value 'strv31', added StructSlice2.2.str2 with value 'strv32'") + + // Remove item from immutable slice + err = yaml.Unmarshal([]byte(` +ints: [11, 12, 13] +slice2: + - str1: strv11 + str2: strv21 + int: 42 + interface: 11 +`), &d2) + c.Assert(err, IsNil) + err = ValidateSpecDiff(d1, d2) + c.Assert(err, NotNil) + c.Assert(err.Error(), Equals, "immutable field changed: removed StructSlice2.1.str1 with value 'strv12', removed StructSlice2.1.str2 with value 'strv22', removed StructSlice2.1.IntElem with value '42', removed StructSlice2.1.interface with value '12'") +} + +func (d *diffSuite) TestValidateSpecDiff4(c *C) { + var d1 sampleDataMeta + var d2 sampleDataMeta + var err error + + err = yaml.Unmarshal([]byte(` +ints: [11, 12, 13] +slice3: + - str1: strv11 + str2: strv21 +`), &d1) + c.Assert(err, IsNil) + + // Add item with only editable fields to editable slice + err = yaml.Unmarshal([]byte(` +ints: [11, 12, 13] +slice3: + - str1: strv11 + str2: strv21 + - str1: strv21 + str2: strv22 +`), &d2) + c.Assert(err, IsNil) + err = ValidateSpecDiff(d1, d2) + c.Assert(err, IsNil) + + // Remove item with only editable fields from editable slice + err = yaml.Unmarshal([]byte(` +ints: [11, 12, 13] +slice3: + - str1: strv21 + str2: strv22 +`), &d2) + c.Assert(err, IsNil) + err = ValidateSpecDiff(d1, d2) + c.Assert(err, IsNil) +} + +func (d *diffSuite) TestValidateSpecDiff5(c *C) { + var d1 sampleDataMeta + var d2 sampleDataMeta + var err error + + err = yaml.Unmarshal([]byte(` +ints: [11, 12, 13] +slice1: + - str1: strv11 + str2: strv21 + interslice: + - key0: 0 + - str1: strv12 + str2: strv22 +slice2: + - str1: strv13 + str2: strv14 + interslice: + - key0: 0 +`), &d1) + c.Assert(err, IsNil) + + // Modify item of editable slice in item of editable slice + err = yaml.Unmarshal([]byte(` +ints: [11, 12, 13] +slice1: + - str1: strv11 + str2: strv21 + interslice: + - key0: 0.1 + - str1: strv12 + str2: strv22 + interslice: + - key1: 1 + - key2: "v2" +slice2: + - str1: strv13 + str2: strv14 + interslice: + - key0: 0 +`), &d2) + c.Assert(err, IsNil) + err = ValidateSpecDiff(d1, d2) + c.Assert(err, IsNil) + + // Modify item of editable slice in item of editable slice + err = yaml.Unmarshal([]byte(` +ints: [11, 12, 13] +slice1: + - str1: strv11 + str2: strv21 + interslice: + - key0: 0 + - str1: strv12 + str2: strv22 + interslice: + - key1: 1 + - key2: "v2" +slice2: + - str1: strv13 + str2: strv14 + interslice: + - key0: 0.2 +`), &d2) + c.Assert(err, IsNil) + err = ValidateSpecDiff(d1, d2) + c.Assert(err, IsNil) + + // Add item to editable slice to item of editable slice + err = yaml.Unmarshal([]byte(` +ints: [11, 12, 13] +slice1: + - str1: strv11 + str2: strv21 + interslice: + - key0: 0 + - str1: strv12 + str2: strv22 + interslice: + - key1: 1 + - key2: "v2" +slice2: + - str1: strv13 + str2: strv14 + interslice: + - key0: 0 +`), &d2) + c.Assert(err, IsNil) + err = ValidateSpecDiff(d1, d2) + c.Assert(err, IsNil) + + // Add item to editable slice to item of immutable slice + err = yaml.Unmarshal([]byte(` +ints: [11, 12, 13] +slice1: + - str1: strv11 + str2: strv21 + interslice: + - key0: 0 + - str1: strv12 + str2: strv22 +slice2: + - str1: strv13 + str2: strv14 + interslice: + - key0: 0 + - key3: 3.0 +`), &d2) + c.Assert(err, IsNil) + 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) +} diff --git a/pkg/cluster/edit/edit.go b/pkg/utils/edit.go similarity index 98% rename from pkg/cluster/edit/edit.go rename to pkg/utils/edit.go index 0726332bdf..b87b682bb2 100644 --- a/pkg/cluster/edit/edit.go +++ b/pkg/utils/edit.go @@ -11,7 +11,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -package edit +package utils import ( "os"