From c14dd3c6d122b5a9ea1acade479c881db9c8e291 Mon Sep 17 00:00:00 2001 From: Christian Goll Date: Mon, 6 Feb 2023 17:00:24 +0100 Subject: [PATCH] check the yaml direclty after unmarshalling --- internal/app/wwctl/node/edit/main.go | 44 +++++++++----- internal/app/wwctl/profile/edit/main.go | 47 +++++++++------ internal/pkg/api/node/edit.go | 4 ++ internal/pkg/api/node/node.go | 12 ++++ internal/pkg/node/checkconf.go | 76 +++++++++++++++++++++++++ internal/pkg/node/constructors.go | 20 +++++++ internal/pkg/node/flags.go | 4 +- 7 files changed, 176 insertions(+), 31 deletions(-) create mode 100644 internal/pkg/node/checkconf.go diff --git a/internal/app/wwctl/node/edit/main.go b/internal/app/wwctl/node/edit/main.go index 0683f931..95d7cb8b 100644 --- a/internal/app/wwctl/node/edit/main.go +++ b/internal/app/wwctl/node/edit/main.go @@ -77,17 +77,38 @@ func CobraRunE(cmd *cobra.Command, args []string) error { // ignore error as only may occurs under strange circumstances buffer, _ := io.ReadAll(file) err = yaml.Unmarshal(buffer, modifiedNodeMap) - if err == nil { - nodeList := make([]string, len(nodeMap)) - i := 0 - for key := range nodeMap { - nodeList[i] = key - i++ - } - yes := apiutil.ConfirmationPrompt(fmt.Sprintf("Are you sure you want to modify %d nodes", len(modifiedNodeMap))) - if !yes { + if err != nil { + yes := apiutil.ConfirmationPrompt(fmt.Sprintf("Got following error on parsing: %s, Retry", err)) + if yes { + continue + } else { break } + } + var checkErrors []error + for nodeName, node := range modifiedNodeMap { + err = node.Check() + if err != nil { + checkErrors = append(checkErrors, fmt.Errorf("node: %s parse error: %s", nodeName, err)) + } + } + if len(checkErrors) != 0 { + yes := apiutil.ConfirmationPrompt(fmt.Sprintf("Got following error on parsing: %s, Retry", checkErrors)) + if yes { + continue + } else { + break + } + } + + nodeList := make([]string, len(nodeMap)) + i := 0 + for key := range nodeMap { + nodeList[i] = key + i++ + } + yes := apiutil.ConfirmationPrompt(fmt.Sprintf("Are you sure you want to modify %d nodes", len(modifiedNodeMap))) + if yes { err = apinode.NodeDelete(&wwapiv1.NodeDeleteParameter{NodeNames: nodeList, Force: true}) if err != nil { wwlog.Verbose("Problem deleting nodes before modification %s") @@ -99,11 +120,6 @@ func CobraRunE(cmd *cobra.Command, args []string) error { os.Exit(1) } break - } else { - yes := apiutil.ConfirmationPrompt(fmt.Sprintf("Got following error on parsing: %s, Retry", err)) - if !yes { - break - } } } else { break diff --git a/internal/app/wwctl/profile/edit/main.go b/internal/app/wwctl/profile/edit/main.go index 97d5a3ad..9ec085a7 100644 --- a/internal/app/wwctl/profile/edit/main.go +++ b/internal/app/wwctl/profile/edit/main.go @@ -71,24 +71,44 @@ func CobraRunE(cmd *cobra.Command, args []string) error { sum2 := hex.EncodeToString(hasher.Sum(nil)) wwlog.Debug("Hashes are before %s and after %s\n", sum1, sum2) if sum1 != sum2 { - wwlog.Debug("Nodes were modified") + wwlog.Debug("Profiles were modified") modifiedProfileMap := make(map[string]*node.NodeConf) _, _ = file.Seek(0, 0) // ignore error as only may occurs under strange circumstances buffer, _ := io.ReadAll(file) err = yaml.Unmarshal(buffer, modifiedProfileMap) - if err == nil { - nodeList := make([]string, len(profileMap)) - i := 0 - for key := range profileMap { - nodeList[i] = key - i++ - } - yes := apiutil.ConfirmationPrompt(fmt.Sprintf("Are you sure you want to modify %d nodes", len(modifiedProfileMap))) - if !yes { + if err != nil { + yes := apiutil.ConfirmationPrompt(fmt.Sprintf("Got following error on parsing: %s, Retry", err)) + if yes { + continue + } else { break } - err = apiprofile.ProfileDelete(&wwapiv1.NodeDeleteParameter{NodeNames: nodeList, Force: true}) + } + var checkErrors []error + for nodeName, node := range modifiedProfileMap { + err = node.Check() + if err != nil { + checkErrors = append(checkErrors, fmt.Errorf("profile: %s parse error: %s", nodeName, err)) + } + } + if len(checkErrors) != 0 { + yes := apiutil.ConfirmationPrompt(fmt.Sprintf("Got following error on parsing: %s, Retry", checkErrors)) + if yes { + continue + } else { + break + } + } + pList := make([]string, len(profileMap)) + i := 0 + for key := range profileMap { + pList[i] = key + i++ + } + yes := apiutil.ConfirmationPrompt(fmt.Sprintf("Are you sure you want to modify %d nodes", len(modifiedProfileMap))) + if yes { + err = apiprofile.ProfileDelete(&wwapiv1.NodeDeleteParameter{NodeNames: pList, Force: true}) if err != nil { wwlog.Verbose("Problem deleting nodes before modification %s") } @@ -99,11 +119,6 @@ func CobraRunE(cmd *cobra.Command, args []string) error { os.Exit(1) } break - } else { - yes := apiutil.ConfirmationPrompt(fmt.Sprintf("Got following error on parsing: %s, Retry", err)) - if !yes { - break - } } } else { break diff --git a/internal/pkg/api/node/edit.go b/internal/pkg/api/node/edit.go index 6cfa8768..f6653d0f 100644 --- a/internal/pkg/api/node/edit.go +++ b/internal/pkg/api/node/edit.go @@ -60,6 +60,10 @@ func NodeAddFromYaml(nodeList *wwapiv1.NodeYaml) (err error) { return errors.Wrap(err, "Could not unmarshall Yaml: %s\n") } for nodeName, node := range nodeMap { + err = node.Check() + if err != nil { + return errors.Errorf("error on node %s: %s", nodeName, err) + } nodeDB.Nodes[nodeName] = node } err = nodeDB.Persist() diff --git a/internal/pkg/api/node/node.go b/internal/pkg/api/node/node.go index 69ad38ff..b6a7e75d 100644 --- a/internal/pkg/api/node/node.go +++ b/internal/pkg/api/node/node.go @@ -50,6 +50,12 @@ func NodeAdd(nap *wwapiv1.NodeAddParameter) (err error) { // only key } // setting node from the received yaml + err = nodeConf.Check() + if err != nil { + err = fmt.Errorf("error on check of node %s: %s", n.Id.Get(), err) + return + + } n.SetFrom(&nodeConf) if netName != "" && nodeConf.NetDevs[netName].Ipaddr != "" { // if more nodes are added increment IPv4 address @@ -236,6 +242,12 @@ func NodeSetParameterCheck(set *wwapiv1.NodeSetParameter, console bool) (nodeDB wwlog.Error(fmt.Sprintf("%v", err.Error())) return } + err = nodeConf.Check() + if err != nil { + err = fmt.Errorf("error on check of node %s: %s", n.Id.Get(), err) + return + + } n.SetFrom(&nodeConf) if set.NetdevDelete != "" { if _, ok := n.NetDevs[set.NetdevDelete]; !ok { diff --git a/internal/pkg/node/checkconf.go b/internal/pkg/node/checkconf.go new file mode 100644 index 00000000..1fa052b5 --- /dev/null +++ b/internal/pkg/node/checkconf.go @@ -0,0 +1,76 @@ +package node + +import ( + "fmt" + "net/netip" + "reflect" + "strconv" + "strings" +) + +/* +Checks if for NodeConf all values can be parsed according to their type. +*/ +func (nodeConf *NodeConf) Check() (err error) { + nodeInfoType := reflect.TypeOf(nodeConf) + nodeInfoVal := reflect.ValueOf(nodeConf) + // now iterate of every field + for i := 0; i < nodeInfoVal.Elem().NumField(); i++ { + //wwlog.Debug("checking field: %s type: %s", nodeInfoType.Elem().Field(i).Name, nodeInfoVal.Elem().Field(i).Type()) + if nodeInfoType.Elem().Field(i).Type.Kind() == reflect.String { + err = checker(nodeInfoVal.Elem().Field(i).Interface().(string), nodeInfoType.Elem().Field(i).Tag.Get("type")) + if err != nil { + return fmt.Errorf("field: %s value:%s err: %s", nodeInfoType.Elem().Field(i).Name, nodeInfoVal.Elem().Field(i).String(), err) + } + } else if nodeInfoType.Elem().Field(i).Type.Kind() == reflect.Ptr && !nodeInfoVal.Elem().Field(i).IsNil() { + nestType := reflect.TypeOf(nodeInfoVal.Elem().Field(i).Interface()) + nestVal := reflect.ValueOf(nodeInfoVal.Elem().Field(i).Interface()) + for j := 0; j < nestType.Elem().NumField(); j++ { + if nestType.Elem().Field(j).Type.Kind() == reflect.String { + //wwlog.Debug("checking field: %s type: %s", nestType.Elem().Field(j).Name, nestType.Elem().Field(j).Tag.Get("type")) + err = checker(nestVal.Elem().Field(j).Interface().(string), nestType.Elem().Field(j).Tag.Get("type")) + if err != nil { + return fmt.Errorf("field: %s value:%s err: %s", nestType.Elem().Field(j).Name, nestVal.Elem().Field(j).String(), err) + } + } + } + } else if nodeInfoType.Elem().Field(i).Type == reflect.TypeOf(map[string]*NetDevs(nil)) { + netMap := nodeInfoVal.Elem().Field(i).Interface().(map[string]*NetDevs) + for _, val := range netMap { + netType := reflect.TypeOf(val) + netVal := reflect.ValueOf(val) + for j := 0; j < netType.Elem().NumField(); j++ { + err = checker(netVal.Elem().Field(j).String(), netType.Elem().Field(j).Tag.Get("type")) + if err != nil { + return fmt.Errorf("field: %s value:%s err: %s", netType.Elem().Field(j).Name, netVal.Elem().Field(j).String(), err) + } + } + } + } + } + return nil +} + +func checker(value string, valType string) (err error) { + if valType == "" || value == "" { + return nil + } + //wwlog.Debug("checker: %s is %s", value, valType) + switch valType { + case "": + return nil + case "bool": + if strings.ToLower(value) == "yes" { + return nil + } + if strings.ToLower(value) == "no" { + return nil + } + _, err = strconv.ParseBool(value) + return err + case "IP": + _, err = netip.ParseAddr(value) + return err + } + return nil +} diff --git a/internal/pkg/node/constructors.go b/internal/pkg/node/constructors.go index 10ee86ad..06fc4c05 100644 --- a/internal/pkg/node/constructors.go +++ b/internal/pkg/node/constructors.go @@ -2,6 +2,7 @@ package node import ( "errors" + "fmt" "os" "path" "sort" @@ -71,6 +72,21 @@ func New() (NodeYaml, error) { if err != nil { return ret, err } + wwlog.Debug("Checking nodes for types") + for nodeName, node := range ret.Nodes { + err = node.Check() + if err != nil { + wwlog.Warn("node: %s parsing error: %s", nodeName, err) + return ret, err + } + } + for profileName, profile := range ret.NodeProfiles { + err = profile.Check() + if err != nil { + wwlog.Warn("node: %s parsing error: %s", profileName, err) + return ret, err + } + } wwlog.Debug("Returning node object") cachedDB = ret @@ -165,6 +181,10 @@ func (config *NodeYaml) FindAllNodes() ([]NodeInfo, error) { node.Tags[keyname] = key delete(node.Keys, keyname) } + err = node.Check() + if err != nil { + return nil, fmt.Errorf("node: %s check error: %s", nodename, err) + } n.SetFrom(node) // only now the netdevs start to exist so that default values can be set for _, netdev := range n.NetDevs { diff --git a/internal/pkg/node/flags.go b/internal/pkg/node/flags.go index 8f42f91d..6a12b914 100644 --- a/internal/pkg/node/flags.go +++ b/internal/pkg/node/flags.go @@ -13,7 +13,9 @@ import ( ) /* -Create cmd line flags from the NodeConf fields +Create cmd line flags from the NodeConf fields. Returns a []func() where every function +must be called, as the commandline parser returns e.g. netip.IP objects which must be parsedf +back to strings. */ func (nodeConf *NodeConf) CreateFlags(baseCmd *cobra.Command, excludeList []string) (converters []func()) { nodeInfoType := reflect.TypeOf(nodeConf)