refactor validation for node creation

controllers/common.go

@@ -59,61 +59,15 @@ func GetPeersList(networkName string) ([]models.PeersResponse, error) {
 }
 
 func ValidateNodeCreate(networkName string, node models.Node) error {
-
 	v := validator.New()
-	_ = v.RegisterValidation("address_check", func(fl validator.FieldLevel) bool {
-		isIpv4 := functions.IsIpNet(node.Address)
-		empty := node.Address == ""
-		return (empty || isIpv4)
-	})
-	_ = v.RegisterValidation("address6_check", func(fl validator.FieldLevel) bool {
-		isIpv6 := functions.IsIpNet(node.Address6)
-		empty := node.Address6 == ""
-		return (empty || isIpv6)
-	})
-	_ = v.RegisterValidation("endpoint_check", func(fl validator.FieldLevel) bool {
-		//var isFieldUnique bool = functions.IsFieldUnique(networkName, "endpoint", node.Endpoint)
-		isIp := functions.IsIpNet(node.Endpoint)
-		notEmptyCheck := node.Endpoint != ""
-		return (notEmptyCheck && isIp)
-	})
-	_ = v.RegisterValidation("localaddress_check", func(fl validator.FieldLevel) bool {
-		//var isFieldUnique bool = functions.IsFieldUnique(networkName, "endpoint", node.Endpoint)
-		isIp := functions.IsIpNet(node.LocalAddress)
-		empty := node.LocalAddress == ""
-		return (empty || isIp)
-	})
-
 	_ = v.RegisterValidation("macaddress_unique", func(fl validator.FieldLevel) bool {
 		var isFieldUnique bool = functions.IsFieldUnique(networkName, "macaddress", node.MacAddress)
 		return isFieldUnique
 	})
-
-	_ = v.RegisterValidation("macaddress_valid", func(fl validator.FieldLevel) bool {
-		_, err := net.ParseMAC(node.MacAddress)
-		return err == nil
-	})
-
-	_ = v.RegisterValidation("name_valid", func(fl validator.FieldLevel) bool {
-		isvalid := functions.NameInNodeCharSet(node.Name)
-		return isvalid
-	})
-
 	_ = v.RegisterValidation("network_exists", func(fl validator.FieldLevel) bool {
 		_, err := node.GetNetwork()
 		return err == nil
 	})
-	_ = v.RegisterValidation("pubkey_check", func(fl validator.FieldLevel) bool {
-		notEmptyCheck := node.PublicKey != ""
-		isBase64 := functions.IsBase64(node.PublicKey)
-		return (notEmptyCheck && isBase64)
-	})
-	_ = v.RegisterValidation("password_check", func(fl validator.FieldLevel) bool {
-		notEmptyCheck := node.Password != ""
-		goodLength := len(node.Password) > 5
-		return (notEmptyCheck && goodLength)
-	})
-
 	err := v.Struct(node)
 
 	if err != nil {
@@ -124,7 +78,7 @@ func ValidateNodeCreate(networkName string, node models.Node) error {
 	return err
 }
 
-func ValidateNodeUpdate(networkName string, node models.Node) error {
+func ValidateNodeUpdate(networkName string, node models.NodeUpdate) error {
 
 	v := validator.New()
 	_ = v.RegisterValidation("address_check", func(fl validator.FieldLevel) bool {
@@ -188,7 +142,7 @@ func ValidateNodeUpdate(networkName string, node models.Node) error {
 	return err
 }
 
-func UpdateNode(nodechange models.Node, node models.Node) (models.Node, error) {
+func UpdateNode(nodechange models.NodeUpdate, node models.Node) (models.Node, error) {
 	//Question: Is there a better way  of doing  this than a bunch of "if" statements? probably...
 	//Eventually, lets have a better way to check if any of the fields are filled out...
 	queryMac := node.MacAddress

controllers/common_test.go

@@ -13,6 +13,12 @@ type NodeValidationTC struct {
 	errorMessage string
 }
 
+type NodeValidationUpdateTC struct {
+	testname     string
+	node         models.NodeUpdate
+	errorMessage string
+}
+
 func TestCreateNode(t *testing.T) {
 }
 func TestDeleteNode(t *testing.T) {
@@ -43,28 +49,28 @@ func TestValidateNodeCreate(t *testing.T) {
 			node: models.Node{
 				Address: "256.0.0.1",
 			},
-			errorMessage: "Field validation for 'Address' failed on the 'address_check' tag",
+			errorMessage: "Field validation for 'Address' failed on the 'ipv4' tag",
 		},
 		NodeValidationTC{
 			testname: "BadAddress6",
 			node: models.Node{
 				Address6: "2607::abcd:efgh::1",
 			},
-			errorMessage: "Field validation for 'Address6' failed on the 'address6_check' tag",
+			errorMessage: "Field validation for 'Address6' failed on the 'ipv6' tag",
 		},
 		NodeValidationTC{
 			testname: "BadLocalAddress",
 			node: models.Node{
 				LocalAddress: "10.0.200.300",
 			},
-			errorMessage: "Field validation for 'LocalAddress' failed on the 'localaddress_check' tag",
+			errorMessage: "Field validation for 'LocalAddress' failed on the 'ip' tag",
 		},
 		NodeValidationTC{
 			testname: "InvalidName",
 			node: models.Node{
 				Name: "mynode*",
 			},
-			errorMessage: "Field validation for 'Name' failed on the 'name_valid' tag",
+			errorMessage: "Field validation for 'Name' failed on the 'alphanum' tag",
 		},
 		NodeValidationTC{
 			testname: "NameTooLong",
@@ -88,18 +94,32 @@ func TestValidateNodeCreate(t *testing.T) {
 			errorMessage: "Field validation for 'ListenPort' failed on the 'max' tag",
 		},
 		NodeValidationTC{
-			testname: "PublicKeyInvalid",
+			testname: "PublicKeyEmpty",
 			node: models.Node{
 				PublicKey: "",
 			},
-			errorMessage: "Field validation for 'PublicKey' failed on the 'pubkey_check' tag",
+			errorMessage: "Field validation for 'PublicKey' failed on the 'required' tag",
+		},
+		NodeValidationTC{
+			testname: "PublicKeyInvalid",
+			node: models.Node{
+				PublicKey: "junk%key",
+			},
+			errorMessage: "Field validation for 'PublicKey' failed on the 'base64' tag",
 		},
 		NodeValidationTC{
 			testname: "EndpointInvalid",
 			node: models.Node{
 				Endpoint: "10.2.0.300",
 			},
-			errorMessage: "Field validation for 'Endpoint' failed on the 'endpoint_check' tag",
+			errorMessage: "Field validation for 'Endpoint' failed on the 'ip' tag",
+		},
+		NodeValidationTC{
+			testname: "EndpointEmpty",
+			node: models.Node{
+				Endpoint: "",
+			},
+			errorMessage: "Field validation for 'Endpoint' failed on the 'required' tag",
 		},
 		NodeValidationTC{
 			testname: "PersistentKeepaliveMax",
@@ -113,7 +133,7 @@ func TestValidateNodeCreate(t *testing.T) {
 			node: models.Node{
 				MacAddress: "01:02:03:04:05",
 			},
-			errorMessage: "Field validation for 'MacAddress' failed on the 'macaddress_valid' tag",
+			errorMessage: "Field validation for 'MacAddress' failed on the 'mac' tag",
 		},
 		NodeValidationTC{
 			testname: "MacAddressMissing",
@@ -127,14 +147,14 @@ func TestValidateNodeCreate(t *testing.T) {
 			node: models.Node{
 				Password: "",
 			},
-			errorMessage: "Field validation for 'Password' failed on the 'password_check' tag",
+			errorMessage: "Field validation for 'Password' failed on the 'required' tag",
 		},
 		NodeValidationTC{
 			testname: "ShortPassword",
 			node: models.Node{
 				Password: "1234",
 			},
-			errorMessage: "Field validation for 'Password' failed on the 'password_check' tag",
+			errorMessage: "Field validation for 'Password' failed on the 'min' tag",
 		},
 		NodeValidationTC{
 			testname: "NoNetwork",
@@ -170,18 +190,119 @@ func TestValidateNodeCreate(t *testing.T) {
 }
 func TestValidateNodeUpdate(t *testing.T) {
 	//cases
-	t.Run("BlankAddress", func(t *testing.T) {
+	cases := []NodeValidationUpdateTC{
-	})
+		NodeValidationUpdateTC{
-	t.Run("BlankAddress6", func(t *testing.T) {
+			testname: "BadAddress",
-	})
+			node: models.NodeUpdate{
-	t.Run("Blank", func(t *testing.T) {
+				Address: "256.0.0.1",
+			},
+			errorMessage: "Field validation for 'Address' failed on the 'address_check' tag",
+		},
+		NodeValidationUpdateTC{
+			testname: "BadAddress6",
+			node: models.NodeUpdate{
+				Address6: "2607::abcd:efgh::1",
+			},
+			errorMessage: "Field validation for 'Address6' failed on the 'address6_check' tag",
+		},
+		NodeValidationUpdateTC{
+			testname: "BadLocalAddress",
+			node: models.NodeUpdate{
+				LocalAddress: "10.0.200.300",
+			},
+			errorMessage: "Field validation for 'LocalAddress' failed on the 'localaddress_check' tag",
+		},
+		NodeValidationUpdateTC{
+			testname: "InvalidName",
+			node: models.NodeUpdate{
+				Name: "mynode*",
+			},
+			errorMessage: "Field validation for 'Name' failed on the 'name_valid' tag",
+		},
+		NodeValidationUpdateTC{
+			testname: "NameTooLong",
+			node: models.NodeUpdate{
+				Name: "mynodexmynode",
+			},
+			errorMessage: "Field validation for 'Name' failed on the 'max' tag",
+		},
+		NodeValidationUpdateTC{
+			testname: "ListenPortMin",
+			node: models.NodeUpdate{
+				ListenPort: 1023,
+			},
+			errorMessage: "Field validation for 'ListenPort' failed on the 'min' tag",
+		},
+		NodeValidationUpdateTC{
+			testname: "ListenPortMax",
+			node: models.NodeUpdate{
+				ListenPort: 65536,
+			},
+			errorMessage: "Field validation for 'ListenPort' failed on the 'max' tag",
+		},
+		NodeValidationUpdateTC{
+			testname: "PublicKeyInvalid",
+			node: models.NodeUpdate{
+				PublicKey: "",
+			},
+			errorMessage: "Field validation for 'PublicKey' failed on the 'pubkey_check' tag",
+		},
+		NodeValidationUpdateTC{
+			testname: "EndpointInvalid",
+			node: models.NodeUpdate{
+				Endpoint: "10.2.0.300",
+			},
+			errorMessage: "Field validation for 'Endpoint' failed on the 'endpoint_check' tag",
+		},
+		NodeValidationUpdateTC{
+			testname: "PersistentKeepaliveMax",
+			node: models.NodeUpdate{
+				PersistentKeepalive: 1001,
+			},
+			errorMessage: "Field validation for 'PersistentKeepalive' failed on the 'max' tag",
+		},
+		NodeValidationUpdateTC{
+			testname: "MacAddressInvalid",
+			node: models.NodeUpdate{
+				MacAddress: "01:02:03:04:05",
+			},
+			errorMessage: "Field validation for 'MacAddress' failed on the 'macaddress_valid' tag",
+		},
+		NodeValidationUpdateTC{
+			testname: "MacAddressMissing",
+			node: models.NodeUpdate{
+				MacAddress: "",
+			},
+			errorMessage: "Field validation for 'MacAddress' failed on the 'required' tag",
+		},
+		NodeValidationUpdateTC{
+			testname: "EmptyPassword",
+			node: models.NodeUpdate{
+				Password: "",
+			},
+			errorMessage: "Field validation for 'Password' failed on the 'password_check' tag",
+		},
+		NodeValidationUpdateTC{
+			testname: "ShortPassword",
+			node: models.NodeUpdate{
+				Password: "1234",
+			},
+			errorMessage: "Field validation for 'Password' failed on the 'password_check' tag",
+		},
+		NodeValidationUpdateTC{
+			testname: "NoNetwork",
+			node: models.NodeUpdate{
+				Network: "badnet",
+			},
+			errorMessage: "Field validation for 'Network' failed on the 'network_exists' tag",
+		},
+	}
+	for _, tc := range cases {
+		t.Run(tc.testname, func(t *testing.T) {
+			err := ValidateNodeUpdate("skynet", tc.node)
+			assert.NotNil(t, err)
+			assert.Contains(t, err.Error(), tc.errorMessage)
 		})
+	}
 
-	//	for _, tc := range cases {
-	//		t.Run(tc.testname, func(t *testing.T) {
-	//			err := ValidateNodeUpdate(tc.node)
-	//			assert.NotNil(t, err)
-	//			assert.Contains(t, err.Error(), tc.errorMessage)
-	//		})
-	//  }
 }

controllers/nodeGrpcController.go

@@ -4,9 +4,10 @@ import (
 	"context"
 	"fmt"
 	"strconv"
+
+	"github.com/gravitl/netmaker/functions"
 	nodepb "github.com/gravitl/netmaker/grpc"
 	"github.com/gravitl/netmaker/models"
-	"github.com/gravitl/netmaker/functions"
 	"go.mongodb.org/mongo-driver/mongo"
 	"google.golang.org/grpc/codes"
 	"google.golang.org/grpc/status"
@@ -15,8 +16,8 @@ import (
 type NodeServiceServer struct {
 	NodeDB *mongo.Collection
 	nodepb.UnimplementedNodeServiceServer
-
 }
+
 func (s *NodeServiceServer) ReadNode(ctx context.Context, req *nodepb.ReadNodeReq) (*nodepb.ReadNodeRes, error) {
 	// convert string id (from proto) to mongoDB ObjectId
 	macaddress := req.GetMacaddress()
@@ -54,7 +55,6 @@ func (s *NodeServiceServer) ReadNode(ctx context.Context, req *nodepb.ReadNodeRe
 			Keepalive:       node.PersistentKeepalive,
 			Islocal:         *network.IsLocal,
 			Localrange:      network.LocalRange,
-
 		},
 	}
 	return response, nil
@@ -101,8 +101,6 @@ func (s *NodeServiceServer) CreateNode(ctx context.Context, req *nodepb.CreateNo
 		fmt.Println("Range if local: " + network.LocalRange)
 	}
 
-
-
 	if !validKey {
 		//Check to see if network will allow manual sign up
 		//may want to switch this up with the valid key check and avoid a DB call that way.
@@ -197,12 +195,11 @@ func (s *NodeServiceServer) CheckIn(ctx context.Context, req *nodepb.CheckInReq)
 	return response, nil
 }
 
-
 func (s *NodeServiceServer) UpdateNode(ctx context.Context, req *nodepb.UpdateNodeReq) (*nodepb.UpdateNodeRes, error) {
 	// Get the node data from the request
 	data := req.GetNode()
 	// Now we have to convert this into a NodeItem type to convert into BSON
-        nodechange := models.Node{
+	nodechange := models.NodeUpdate{
 		// ID:       primitive.NilObjectID,
 		MacAddress:          data.GetMacaddress(),
 		Name:                data.GetName(),
@@ -220,13 +217,11 @@ func (s *NodeServiceServer) UpdateNode(ctx context.Context, req *nodepb.UpdateNo
 		ListenPort:          data.GetListenport(),
 	}
 
-
 	// Convert the Id string to a MongoDB ObjectId
 	macaddress := nodechange.MacAddress
 	networkName := nodechange.Network
 	network, _ := functions.GetParentNetwork(networkName)
 
-
 	err := ValidateNodeUpdate(networkName, nodechange)
 	if err != nil {
 		return nil, err
@@ -240,7 +235,6 @@ func (s *NodeServiceServer) UpdateNode(ctx context.Context, req *nodepb.UpdateNo
 		)
 	}
 
-
 	newnode, err := UpdateNode(nodechange, node)
 
 	if err != nil {
@@ -267,7 +261,6 @@ func (s *NodeServiceServer) UpdateNode(ctx context.Context, req *nodepb.UpdateNo
 			Keepalive:    newnode.PersistentKeepalive,
 			Islocal:      *network.IsLocal,
 			Localrange:   network.LocalRange,
-
 		},
 	}, nil
 }
@@ -293,7 +286,6 @@ func (s *NodeServiceServer) DeleteNode(ctx context.Context, req *nodepb.DeleteNo
 		return nil, status.Errorf(codes.NotFound, fmt.Sprintf("Could not update network last modified date: %v", err))
 	}
 
-
 	return &nodepb.DeleteNodeRes{
 		Success: true,
 	}, nil
@@ -332,12 +324,10 @@ func (s *NodeServiceServer) GetPeers(req *nodepb.GetPeersReq, stream nodepb.Node
 		return status.Errorf(codes.Internal, fmt.Sprintf("Could not get node: %v", err))
 	}
 
-
 	err = TimestampNode(node, false, true, false)
 	if err != nil {
 		return status.Errorf(codes.Internal, fmt.Sprintf("Internal error occurred: %v", err))
 	}
 
-
 	return nil
 }

controllers/nodeHttpController.go

@@ -689,7 +689,7 @@ func updateNode(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	var nodechange models.Node
+	var nodechange models.NodeUpdate
 
 	// we decode our body request params
 	_ = json.NewDecoder(r.Body).Decode(&nodechange)

models/node.go

@@ -18,6 +18,38 @@ var seededRand *rand.Rand = rand.New(
 
 //node struct
 type Node struct {
+	ID                  primitive.ObjectID `json:"_id,omitempty" bson:"_id,omitempty"`
+	Address             string             `json:"address" bson:"address" validate:"omitempty,ipv4"`
+	Address6            string             `json:"address6" bson:"address6" validate:"omitempty,ipv6"`
+	LocalAddress        string             `json:"localaddress" bson:"localaddress" validate:"omitempty,ip"`
+	Name                string             `json:"name" bson:"name" validate:"omitempty,alphanum,max=12"`
+	ListenPort          int32              `json:"listenport" bson:"listenport" validate:"omitempty,numeric,min=1024,max=65535"`
+	PublicKey           string             `json:"publickey" bson:"publickey" validate:"required,base64"`
+	Endpoint            string             `json:"endpoint" bson:"endpoint" validate:"required,ip"`
+	PostUp              string             `json:"postup" bson:"postup"`
+	PostDown            string             `json:"postdown" bson:"postdown"`
+	AllowedIPs          string             `json:"allowedips" bson:"allowedips"`
+	PersistentKeepalive int32              `json:"persistentkeepalive" bson:"persistentkeepalive" validate:"omitempty,numeric,max=1000"`
+	SaveConfig          *bool              `json:"saveconfig" bson:"saveconfig"`
+	AccessKey           string             `json:"accesskey" bson:"accesskey"`
+	Interface           string             `json:"interface" bson:"interface"`
+	LastModified        int64              `json:"lastmodified" bson:"lastmodified"`
+	KeyUpdateTimeStamp  int64              `json:"keyupdatetimestamp" bson:"keyupdatetimestamp"`
+	ExpirationDateTime  int64              `json:"expdatetime" bson:"expdatetime"`
+	LastPeerUpdate      int64              `json:"lastpeerupdate" bson:"lastpeerupdate"`
+	LastCheckIn         int64              `json:"lastcheckin" bson:"lastcheckin"`
+	MacAddress          string             `json:"macaddress" bson:"macaddress" validate:"required,mac,macaddress_unique"`
+	CheckInInterval     int32              `json:"checkininterval" bson:"checkininterval"`
+	Password            string             `json:"password" bson:"password" validate:"required,min=6"`
+	Network             string             `json:"network" bson:"network" validate:"network_exists"`
+	IsPending           bool               `json:"ispending" bson:"ispending"`
+	IsGateway           bool               `json:"isgateway" bson:"isgateway"`
+	GatewayRange        string             `json:"gatewayrange" bson:"gatewayrange"`
+	PostChanges         string             `json:"postchanges" bson:"postchanges"`
+}
+
+//node update struct --- only validations are different
+type NodeUpdate struct {
 	ID                  primitive.ObjectID `json:"_id,omitempty" bson:"_id,omitempty"`
 	Address             string             `json:"address" bson:"address" validate:"address_check"`
 	Address6            string             `json:"address6" bson:"address6" validate:"address6_check"`
@@ -48,6 +80,29 @@ type Node struct {
 	PostChanges         string             `json:"postchanges" bson:"postchanges"`
 }
 
+//Duplicated function for NodeUpdates
+func (node *NodeUpdate) GetNetwork() (Network, error) {
+
+	var network Network
+
+	collection := mongoconn.NetworkDB
+	//collection := mongoconn.Client.Database("netmaker").Collection("networks")
+
+	ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
+
+	filter := bson.M{"netid": node.Network}
+	err := collection.FindOne(ctx, filter).Decode(&network)
+
+	defer cancel()
+
+	if err != nil {
+		//log.Fatal(err)
+		return network, err
+	}
+
+	return network, err
+}
+
 //TODO: Contains a fatal error return. Need to change
 //Used in contexts where it's not the Parent network.
 func (node *Node) GetNetwork() (Network, error) {