dedup the config validation logic in bare-metal mode

Signed-off-by: sh2 <[email protected]>

go.mod

@@ -12,6 +12,7 @@ require (
 	github.com/onsi/ginkgo/v2 v2.4.0
 	github.com/onsi/gomega v1.23.0
 	github.com/spf13/cobra v1.6.1
+	github.com/stretchr/testify v1.8.1
 	gopkg.in/yaml.v3 v3.0.1
 	helm.sh/helm/v3 v3.11.1
 	k8s.io/api v0.26.0
@@ -97,6 +98,7 @@ require (
 	github.com/opencontainers/image-spec v1.1.0-rc2 // indirect
 	github.com/peterbourgon/diskv v2.0.1+incompatible // indirect
 	github.com/pkg/errors v0.9.1 // indirect
+	github.com/pmezard/go-difflib v1.0.0 // indirect
 	github.com/prometheus/client_golang v1.14.0 // indirect
 	github.com/prometheus/client_model v0.3.0 // indirect
 	github.com/prometheus/common v0.37.0 // indirect

go.sum

@@ -546,14 +546,19 @@ github.com/spf13/viper v1.8.1/go.mod h1:o0Pch8wJ9BVSWGQMbra6iw0oQ5oktSIBaujf1rJH
 github.com/stoewer/go-strcase v1.2.0/go.mod h1:IBiWB2sKIp3wVVQ3Y035++gc+knqhUQag1KpM8ahLw8=
 github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
 github.com/stretchr/objx v0.1.1/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
-github.com/stretchr/objx v0.2.0 h1:Hbg2NidpLE8veEBkEZTL3CvlkUIVzuU9jDplZO54c48=
+github.com/stretchr/objx v0.4.0/go.mod h1:YvHI0jy2hoMjB+UWwv71VJQ9isScKT/TqJzVSSt89Yw=
+github.com/stretchr/objx v0.5.0 h1:1zr/of2m5FGMsad5YfcqgdqdWrIhu+EBEJRhR1U7z/c=
+github.com/stretchr/objx v0.5.0/go.mod h1:Yh+to48EsGEfYuaHDzXPcE3xhTkx73EhmCGUpEOglKo=
 github.com/stretchr/testify v1.2.2/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXfy6kDkUVs=
 github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI=
 github.com/stretchr/testify v1.4.0/go.mod h1:j7eGeouHqKxXV5pUuKE4zz7dFj8WfuZ+81PSLYec5m4=
 github.com/stretchr/testify v1.5.1/go.mod h1:5W2xD1RspED5o8YsWQXVCued0rvSQ+mT+I5cxcmMvtA=
 github.com/stretchr/testify v1.6.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg=
 github.com/stretchr/testify v1.7.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg=
+github.com/stretchr/testify v1.7.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg=
+github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO+kdMU+MU=
 github.com/stretchr/testify v1.8.1 h1:w7B6lhMri9wdJUVmEZPGGhZzrYTPvgJArz7wNPgYKsk=
+github.com/stretchr/testify v1.8.1/go.mod h1:w2LPCIKwWwSfY2zedu0+kehJoqGctiVI29o6fzry7u4=
 github.com/subosito/gotenv v1.2.0/go.mod h1:N0PQaV/YGNqwC0u51sEeR/aUtSLEXKX9iv69rRypqCw=
 github.com/xeipuuv/gojsonpointer v0.0.0-20180127040702-4e3ac2762d5f h1:J9EGpcZtP0E/raorCMxlFGSTBrsSlaDGf3jU/qvAE2c=
 github.com/xeipuuv/gojsonpointer v0.0.0-20180127040702-4e3ac2762d5f/go.mod h1:N2zxlSyiKSe5eX1tZViRH5QA0qijqEDrYZiPEAiq3wU=

pkg/deployer/baremetal/config.go

@@ -18,27 +18,42 @@ import (
 	"fmt"
 	"net"
 	"os"
+	"reflect"
 	"strconv"
+	"strings"
+)
+
+const (
+	ValidationTag = "validate"
+
+	validateNotNil   = "not-nil"
+	validateNotEmpty = "not-empty"
+	validateIsAddr   = "is-addr"
 )
 
 // Config is the desired state of a GreptimeDB cluster on bare metal.
+//
+// The field of Config that with `validate` tag will be validated
+// against its requirement. Each filed has only one requirement.
+//
+// Each field of Config can also have its own exported method `Validate`.
 type Config struct {
-	Cluster *Cluster `yaml:"cluster"`
-	Etcd    *Etcd    `yaml:"etcd"`
+	Cluster *Cluster `yaml:"cluster" validate:"not-nil"`
+	Etcd    *Etcd    `yaml:"etcd" validate:"not-nil"`
 }
 
 type Cluster struct {
-	Artifact *Artifact `yaml:"artifact"`
-	Frontend *Frontend `yaml:"frontend"`
-	Meta     *Meta     `yaml:"meta"`
-	Datanode *Datanode `yaml:"datanode"`
+	Artifact *Artifact `yaml:"artifact" validate:"not-nil"`
+	Frontend *Frontend `yaml:"frontend" validate:"not-nil"`
+	Meta     *Meta     `yaml:"meta" validate:"not-nil"`
+	Datanode *Datanode `yaml:"datanode" validate:"not-nil"`
 }
 
 type Frontend struct {
-	GRPCAddr     string `yaml:"grpcAddr"`
-	HTTPAddr     string `yaml:"httpAddr"`
-	PostgresAddr string `yaml:"postgresAddr"`
-	MetaAddr     string `yaml:"metaAddr"`
+	GRPCAddr     string `yaml:"grpcAddr" validate:"is-addr"`
+	HTTPAddr     string `yaml:"httpAddr" validate:"is-addr"`
+	PostgresAddr string `yaml:"postgresAddr" validate:"is-addr"`
+	MetaAddr     string `yaml:"metaAddr" validate:"is-addr"`
 
 	LogLevel string `yaml:"logLevel"`
 }
@@ -47,8 +62,8 @@ type Datanode struct {
 	Replicas int `yaml:"replicas"`
 	NodeID   int `yaml:"nodeID"`
 
-	RPCAddr  string `yaml:"rpcAddr"`
-	HTTPAddr string `yaml:"httpAddr"`
+	RPCAddr  string `yaml:"rpcAddr" validate:"not-empty,is-addr"`
+	HTTPAddr string `yaml:"httpAddr" validate:"not-empty,is-addr"`
 
 	DataDir      string `yaml:"dataDir"`
 	WalDir       string `yaml:"walDir"`
@@ -58,16 +73,16 @@ type Datanode struct {
 }
 
 type Meta struct {
-	StoreAddr  string `yaml:"storeAddr"`
-	ServerAddr string `yaml:"serverAddr"`
-	BindAddr   string `yaml:"bindAddr"`
-	HTTPAddr   string `yaml:"httpAddr"`
+	StoreAddr  string `yaml:"storeAddr" validate:"is-addr"`
+	ServerAddr string `yaml:"serverAddr" validate:"is-addr"`
+	BindAddr   string `yaml:"bindAddr" validate:"is-addr"`
+	HTTPAddr   string `yaml:"httpAddr" validate:"not-empty,is-addr"`
 
 	LogLevel string `yaml:"logLevel"`
 }
 
 type Etcd struct {
-	Artifact *Artifact `yaml:"artifact"`
+	Artifact *Artifact `yaml:"artifact" validate:"not-nil"`
 }
 
 type Artifact struct {
@@ -79,108 +94,111 @@ type Artifact struct {
 	Version string `yaml:"version"`
 }
 
-func (c *Config) Validate() error {
-	if c.Cluster == nil {
-		return fmt.Errorf("empty cluster config")
-	}
-	if c.Etcd == nil {
-		return fmt.Errorf("empty etcd config")
+// ValidateConfig validate config in bare-metal mode.
+func ValidateConfig(config *Config) error {
+	if config == nil {
+		return fmt.Errorf("no config to validate")
 	}
 
-	if err := c.Cluster.validate(); err != nil {
-		return err
-	}
-	if err := c.Etcd.validate(); err != nil {
+	err := validateConfigWithSingleValue(config, "")
+	if err != nil {
 		return err
 	}
 
 	return nil
 }
 
-func (cluster *Cluster) validate() error {
-	if cluster.Artifact == nil {
-		return fmt.Errorf("cluster artifact field is nil")
+// validateConfigWithSingleValue validate every single config value.
+func validateConfigWithSingleValue(config interface{}, path string) error {
+	valueOf := reflect.ValueOf(config)
+	if valueOf.Kind() == reflect.Ptr {
+		valueOf = valueOf.Elem()
 	}
-	if err := cluster.Artifact.validate(); err != nil {
-		return err
+	if valueOf.Kind() != reflect.Struct {
+		return nil
 	}
 
-	if cluster.Frontend == nil {
-		return fmt.Errorf("frontend field is nil")
-	}
-	if err := cluster.Frontend.validate(); err != nil {
-		return err
+	typeOf := reflect.TypeOf(config)
+	if typeOf.Kind() == reflect.Ptr {
+		typeOf = typeOf.Elem()
 	}
+	for i := 0; i < valueOf.NumField(); i++ {
+		validateTypes := typeOf.Field(i).Tag.Get(ValidationTag)
 
-	if cluster.Meta == nil {
-		return fmt.Errorf("meta field is nil")
-	}
-	if err := cluster.Meta.validate(); err != nil {
-		return err
-	}
+		fieldPath := fmt.Sprintf("%s.%s", path, typeOf.Field(i).Name)
+		if len(validateTypes) > 0 {
+			if err := validateTags(validateTypes, valueOf.Field(i)); err != nil {
+				return fmt.Errorf("error at field `%s`: %v", fieldPath, err)
+			}
+		}
+		// Perform field validation that defined by `Validate` method.
+		if method := valueOf.Field(i).MethodByName("Validate"); method.IsValid() {
+			if err := method.Call(nil)[0]; !err.IsNil() {
+				return fmt.Errorf("error at field `%s`: %v", fieldPath, err)
+			}
+		}
 
-	if cluster.Datanode == nil {
-		return fmt.Errorf("datanode field is nil")
-	}
-	if err := cluster.Datanode.validate(); err != nil {
-		return err
+		if err := validateConfigWithSingleValue(valueOf.Field(i).Interface(), fieldPath); err != nil {
+			return err
+		}
 	}
-
 	return nil
 }
 
-func (etcd *Etcd) validate() error {
-	if etcd.Artifact == nil {
-		return fmt.Errorf("etcd artifact field is nil")
-	}
-	if err := etcd.Artifact.validate(); err != nil {
-		return err
+func validateTags(types string, value reflect.Value) error {
+	tags := strings.Split(types, ",")
+	for _, tag := range tags {
+		switch tag {
+		case validateNotNil:
+			if value.Type().Kind() == reflect.Ptr && value.IsNil() {
+				return fmt.Errorf("got nil")
+			}
+		case validateNotEmpty:
+			if value.Type().Kind() != reflect.Ptr && value.Len() == 0 {
+				return fmt.Errorf("got empty")
+			}
+		case validateIsAddr:
+			if value.Type().Kind() == reflect.String && value.Len() > 0 {
+				return validateAddr(value.String())
+			}
+		default:
+			return fmt.Errorf("unfamilier validate tag: %s", tag)
+		}
 	}
 	return nil
 }
 
-// TODO(zyy17): Add the validation of the options.
-func (frontend *Frontend) validate() error {
-	return nil
-}
+func validateAddr(addr string) error {
+	addr, port, err := net.SplitHostPort(addr)
+	if err != nil {
+		return err
+	}
 
-func (meta *Meta) validate() error {
-	if meta.HTTPAddr == "" {
-		return fmt.Errorf("empty meta http addr")
+	ip := net.ParseIP(addr)
+	if ip == nil {
+		return fmt.Errorf("invalid ip address '%s'", addr)
 	}
-	if err := checkAddr(meta.HTTPAddr); err != nil {
-		return err
+
+	p, err := strconv.Atoi(port)
+	if err != nil || p < 1 || p > 65535 {
+		return fmt.Errorf("invalid port '%s'", port)
 	}
+
 	return nil
 }
 
-func (datanode *Datanode) validate() error {
+func (datanode *Datanode) Validate() error {
 	if datanode.Replicas <= 0 {
 		return fmt.Errorf("invalid replicas '%d'", datanode.Replicas)
 	}
 
 	if datanode.NodeID < 0 {
 		return fmt.Errorf("invalid nodeID '%d'", datanode.NodeID)
 	}
-
-	if datanode.RPCAddr == "" {
-		return fmt.Errorf("empty datanode rpc addr")
-	}
-	if err := checkAddr(datanode.RPCAddr); err != nil {
-		return err
-	}
-
-	if datanode.HTTPAddr == "" {
-		return fmt.Errorf("empty datanode http addr")
-	}
-	if err := checkAddr(datanode.HTTPAddr); err != nil {
-		return err
-	}
-
 	return nil
 }
 
-func (artifact *Artifact) validate() error {
+func (artifact *Artifact) Validate() error {
 	if artifact.Version == "" && artifact.Local == "" {
 		return fmt.Errorf("empty artifact")
 	}
@@ -201,25 +219,6 @@ func (artifact *Artifact) validate() error {
 	return nil
 }
 
-func checkAddr(addr string) error {
-	addr, port, err := net.SplitHostPort(addr)
-	if err != nil {
-		return err
-	}
-
-	ip := net.ParseIP(addr)
-	if ip == nil {
-		return fmt.Errorf("invalid ip address '%s'", addr)
-	}
-
-	p, err := strconv.Atoi(port)
-	if err != nil || p < 1 || p > 65535 {
-		return fmt.Errorf("invalid port '%s'", port)
-	}
-
-	return nil
-}
-
 func defaultConfig() *Config {
 	return &Config{
 		Cluster: &Cluster{

pkg/deployer/baremetal/config_test.go

@@ -0,0 +1,72 @@
+// Copyright 2023 Greptime Team
+//
+// 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,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package baremetal
+
+import (
+	"fmt"
+	"gopkg.in/yaml.v3"
+	"os"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+type testCase struct {
+	name   string
+	expect bool
+	errmsg string
+}
+
+var testCases = []testCase{
+	{
+		name:   "default-config",
+		expect: true,
+	},
+	{
+		name:   "config-with-nil-part",
+		expect: false,
+		errmsg: "error at field `.Cluster.Meta`: got nil",
+	},
+	{
+		name:   "config-with-empty-part",
+		expect: false,
+		errmsg: "error at field `.Cluster.Datanode.HTTPAddr`: got empty",
+	},
+	{
+		name:   "config-with-invalid-addr",
+		expect: false,
+		errmsg: "error at field `.Cluster.Datanode.HTTPAddr`: invalid ip address '12345.0.0.0'",
+	},
+}
+
+func TestValidateConfig(t *testing.T) {
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			content, err := os.ReadFile(fmt.Sprintf("testdata/%s.yaml", tc.name))
+			assert.NoError(t, err)
+
+			var config *Config
+			err = yaml.Unmarshal(content, &config)
+			assert.NoError(t, err)
+
+			err = ValidateConfig(config)
+			if tc.expect {
+				assert.NoError(t, err)
+			} else {
+				assert.EqualError(t, err, tc.errmsg, "got wrong error")
+			}
+		})
+	}
+}