Browse Source

addressing review comments

Matthew R Kasun 3 years ago
parent
commit
03d17a0520
7 changed files with 47 additions and 49 deletions
  1. 12 17
      controllers/node.go
  2. 2 2
      controllers/node_grpc.go
  3. 4 6
      controllers/relay.go
  4. 2 0
      logic/peers.go
  5. 17 17
      mq/mq.go
  6. 4 3
      netclient/daemon/macos.go
  7. 6 4
      netclient/daemon/systemd.go

+ 12 - 17
controllers/node.go

@@ -431,11 +431,10 @@ func uncordonNode(w http.ResponseWriter, r *http.Request) {
 	}
 	go func() {
 		if err := mq.NodeUpdate(&node); err != nil {
-			logger.Log(1, "error publishing node update"+err.Error())
+			logger.Log(1, "error publishing node update", err.Error())
 		}
 		if err := mq.UpdatePeers(&node); err != nil {
-			logger.Log(1, "error publishing peer update "+err.Error())
-			return
+			logger.Log(1, "error publishing peer update ", err.Error())
 		}
 	}()
 	logger.Log(1, r.Header.Get("user"), "uncordoned node", node.Name)
@@ -464,11 +463,10 @@ func createEgressGateway(w http.ResponseWriter, r *http.Request) {
 	}
 	go func() {
 		if err := mq.NodeUpdate(&node); err != nil {
-			logger.Log(1, "error publishing node update"+err.Error())
+			logger.Log(1, "error publishing node update", err.Error())
 		}
 		if err := mq.UpdatePeers(&node); err != nil {
 			logger.Log(1, "error publishing peer update "+err.Error())
-			return
 		}
 	}()
 	logger.Log(1, r.Header.Get("user"), "created egress gateway on node", gateway.NodeID, "on network", gateway.NetID)
@@ -491,11 +489,10 @@ func deleteEgressGateway(w http.ResponseWriter, r *http.Request) {
 	}
 	go func() {
 		if err := mq.NodeUpdate(&node); err != nil {
-			logger.Log(1, "error publishing node update"+err.Error())
+			logger.Log(1, "error publishing node update", err.Error())
 		}
 		if err := mq.UpdatePeers(&node); err != nil {
-			logger.Log(1, "error publishing peer update "+err.Error())
-			return
+			logger.Log(1, "error publishing peer update ", err.Error())
 		}
 	}()
 	logger.Log(1, r.Header.Get("user"), "deleted egress gateway", nodeid, "on network", netid)
@@ -517,11 +514,10 @@ func createIngressGateway(w http.ResponseWriter, r *http.Request) {
 	}
 	go func() {
 		if err := mq.NodeUpdate(&node); err != nil {
-			logger.Log(1, "error publishing node update"+err.Error())
+			logger.Log(1, "error publishing node update", err.Error())
 		}
 		if err := mq.UpdatePeers(&node); err != nil {
-			logger.Log(1, "error publishing peer update "+err.Error())
-			return
+			logger.Log(1, "error publishing peer update ", err.Error())
 		}
 	}()
 	logger.Log(1, r.Header.Get("user"), "created ingress gateway on node", nodeid, "on network", netid)
@@ -540,11 +536,10 @@ func deleteIngressGateway(w http.ResponseWriter, r *http.Request) {
 	}
 	go func() {
 		if err := mq.NodeUpdate(&node); err != nil {
-			logger.Log(1, "error publishing node update"+err.Error())
+			logger.Log(1, "error publishing node update", err.Error())
 		}
 		if err := mq.UpdatePeers(&node); err != nil {
-			logger.Log(1, "error publishing peer update "+err.Error())
-			return
+			logger.Log(1, "error publishing peer update ", err.Error())
 		}
 	}()
 	logger.Log(1, r.Header.Get("user"), "deleted ingress gateway", nodeid)
@@ -619,11 +614,11 @@ func updateNode(w http.ResponseWriter, r *http.Request) {
 	json.NewEncoder(w).Encode(newNode)
 	go func() {
 		if err := mq.NodeUpdate(&newNode); err != nil {
-			logger.Log(1, "error publishing node update"+err.Error())
+			logger.Log(1, "error publishing node update", err.Error())
 		}
 		if logic.ShouldPeersUpdate(&node, &newNode) {
 			if err := mq.UpdatePeers(&newNode); err != nil {
-				logger.Log(1, "error publishing peer update after node update"+err.Error())
+				logger.Log(1, "error publishing peer update after node update", err.Error())
 			}
 		}
 	}()
@@ -655,7 +650,7 @@ func deleteNode(w http.ResponseWriter, r *http.Request) {
 	node.Action = models.NODE_DELETE
 	go func() {
 		if err := mq.NodeUpdate(&node); err != nil {
-			logger.Log(1, "error publishing node delete "+err.Error())
+			logger.Log(1, "error publishing node delete ", err.Error())
 		}
 	}()
 	logger.Log(1, r.Header.Get("user"), "Deleted node", nodeid, "from network", params["network"])

+ 2 - 2
controllers/node_grpc.go

@@ -94,7 +94,7 @@ func (s *NodeServiceServer) CreateNode(ctx context.Context, req *nodepb.Object)
 	// notify other nodes on network of new peer
 	go func() {
 		if err := mq.UpdatePeers(&node); err != nil {
-			logger.Log(0, "failed to inform peers of new node "+err.Error())
+			logger.Log(0, "failed to inform peers of new node ", err.Error())
 		}
 	}()
 
@@ -161,7 +161,7 @@ func (s *NodeServiceServer) DeleteNode(ctx context.Context, req *nodepb.Object)
 	// notify other nodes on network of deleted peer
 	go func() {
 		if err := mq.UpdatePeers(&node); err != nil {
-			logger.Log(0, "failed to inform peers of deleted node "+err.Error())
+			logger.Log(0, "failed to inform peers of deleted node ", err.Error())
 		}
 	}()
 

+ 4 - 6
controllers/relay.go

@@ -32,11 +32,10 @@ func createRelay(w http.ResponseWriter, r *http.Request) {
 	}
 	go func() {
 		if err := mq.NodeUpdate(&node); err != nil {
-			logger.Log(1, "error publishing node update"+err.Error())
+			logger.Log(1, "error publishing node update", err.Error())
 		}
 		if err := mq.UpdatePeers(&node); err != nil {
-			logger.Log(1, "error publishing peer update "+err.Error())
-			return
+			logger.Log(1, "error publishing peer update ", err.Error())
 		}
 	}()
 	logger.Log(1, r.Header.Get("user"), "created relay on node", relay.NodeID, "on network", relay.NetID)
@@ -59,11 +58,10 @@ func deleteRelay(w http.ResponseWriter, r *http.Request) {
 	}
 	go func() {
 		if err := mq.NodeUpdate(&node); err != nil {
-			logger.Log(1, "error publishing node update"+err.Error())
+			logger.Log(1, "error publishing node update", err.Error())
 		}
 		if err := mq.UpdatePeers(&node); err != nil {
-			logger.Log(1, "error publishing peer update "+err.Error())
-			return
+			logger.Log(1, "error publishing peer update ", err.Error())
 		}
 	}()
 	logger.Log(1, r.Header.Get("user"), "deleted relay server", nodeid, "on network", netid)

+ 2 - 0
logic/peers.go

@@ -12,6 +12,7 @@ import (
 	"golang.zx2c4.com/wireguard/wgctrl/wgtypes"
 )
 
+// GetPeerUpdate - gets a wireguard peer config for each peer of a node
 func GetPeerUpdate(node *models.Node) (models.PeerUpdate, error) {
 	var peerUpdate models.PeerUpdate
 	var peers []wgtypes.PeerConfig
@@ -60,6 +61,7 @@ func GetPeerUpdate(node *models.Node) (models.PeerUpdate, error) {
 	return peerUpdate, nil
 }
 
+// GetAllowedIPs - calculates the wireguard allowedip field for a peer of a node based on the peer and node settings
 func GetAllowedIPs(node, peer *models.Node) []net.IPNet {
 	var allowedips []net.IPNet
 	var gateways []string

+ 17 - 17
mq/mq.go

@@ -14,14 +14,14 @@ import (
 	"github.com/gravitl/netmaker/servercfg"
 )
 
-// default message handler - only called in GetDebug == true
+// DefaultHandler default message queue handler - only called when GetDebug == true
 var DefaultHandler mqtt.MessageHandler = func(client mqtt.Client, msg mqtt.Message) {
-	logger.Log(0, "MQTT Message: Topic: "+string(msg.Topic())+" Message: "+string(msg.Payload()))
+	logger.Log(0, "MQTT Message: Topic: ", string(msg.Topic()), " Message: ", string(msg.Payload()))
 }
 
 // Ping message Handler -- handles ping topic from client nodes
 var Ping mqtt.MessageHandler = func(client mqtt.Client, msg mqtt.Message) {
-	logger.Log(0, "Ping Handler: "+msg.Topic())
+	logger.Log(0, "Ping Handler: ", msg.Topic())
 	go func() {
 		id, err := GetID(msg.Topic())
 		if err != nil {
@@ -30,7 +30,7 @@ var Ping mqtt.MessageHandler = func(client mqtt.Client, msg mqtt.Message) {
 		}
 		node, err := logic.GetNodeByID(id)
 		if err != nil {
-			logger.Log(0, "mq-ping error getting node: "+err.Error())
+			logger.Log(0, "mq-ping error getting node: ", err.Error())
 			record, err := database.FetchRecord(database.NODES_TABLE_NAME, id)
 			if err != nil {
 				logger.Log(0, "error reading database ", err.Error())
@@ -42,7 +42,7 @@ var Ping mqtt.MessageHandler = func(client mqtt.Client, msg mqtt.Message) {
 		}
 		node.SetLastCheckIn()
 		if err := logic.UpdateNode(&node, &node); err != nil {
-			logger.Log(0, "error updating node "+err.Error())
+			logger.Log(0, "error updating node ", err.Error())
 		}
 		logger.Log(0, "ping processed")
 		// --TODO --set client version once feature is implemented.
@@ -55,26 +55,26 @@ var UpdateNode mqtt.MessageHandler = func(client mqtt.Client, msg mqtt.Message)
 	go func() {
 		id, err := GetID(msg.Topic())
 		if err != nil {
-			logger.Log(1, "error getting node.ID sent on "+msg.Topic()+" "+err.Error())
+			logger.Log(1, "error getting node.ID sent on ", msg.Topic(), err.Error())
 			return
 		}
-		logger.Log(1, "Update Node Handler"+id)
+		logger.Log(1, "Update Node Handler", id)
 		var newNode models.Node
 		if err := json.Unmarshal(msg.Payload(), &newNode); err != nil {
-			logger.Log(1, "error unmarshaling payload "+err.Error())
+			logger.Log(1, "error unmarshaling payload ", err.Error())
 			return
 		}
 		currentNode, err := logic.GetNodeByID(newNode.ID)
 		if err != nil {
-			logger.Log(1, "error getting node "+newNode.ID+" "+err.Error())
+			logger.Log(1, "error getting node ", newNode.ID, err.Error())
 			return
 		}
 		if err := logic.UpdateNode(&currentNode, &newNode); err != nil {
-			logger.Log(1, "error saving node"+err.Error())
+			logger.Log(1, "error saving node", err.Error())
 		}
 		if logic.ShouldPeersUpdate(&currentNode, &newNode) {
 			if err := PublishPeerUpdate(client, &newNode); err != nil {
-				logger.Log(1, "error publishing peer update "+err.Error())
+				logger.Log(1, "error publishing peer update ", err.Error())
 				return
 			}
 		}
@@ -85,22 +85,22 @@ var UpdateNode mqtt.MessageHandler = func(client mqtt.Client, msg mqtt.Message)
 func PublishPeerUpdate(client mqtt.Client, newNode *models.Node) error {
 	networkNodes, err := logic.GetNetworkNodes(newNode.Network)
 	if err != nil {
-		logger.Log(1, "err getting Network Nodes"+err.Error())
+		logger.Log(1, "err getting Network Nodes", err.Error())
 		return err
 	}
 	for _, node := range networkNodes {
 		peerUpdate, err := logic.GetPeerUpdate(&node)
 		if err != nil {
-			logger.Log(1, "error getting peer update for node "+node.ID+" "+err.Error())
+			logger.Log(1, "error getting peer update for node ", node.ID, err.Error())
 			continue
 		}
 		data, err := json.Marshal(&peerUpdate)
 		if err != nil {
-			logger.Log(2, "error marshaling peer update "+err.Error())
+			logger.Log(2, "error marshaling peer update ", err.Error())
 			return err
 		}
 		if token := client.Publish("update/peers/"+node.ID, 0, false, data); token.Wait() && token.Error() != nil {
-			logger.Log(2, "error publishing peer update to peer "+node.ID+" "+token.Error().Error())
+			logger.Log(2, "error publishing peer update to peer ", node.ID, token.Error().Error())
 			return err
 		}
 	}
@@ -125,11 +125,11 @@ func NodeUpdate(node *models.Node) error {
 	defer client.Disconnect(250)
 	data, err := json.Marshal(node)
 	if err != nil {
-		logger.Log(2, "error marshalling node update "+err.Error())
+		logger.Log(2, "error marshalling node update ", err.Error())
 		return err
 	}
 	if token := client.Publish("update/"+node.ID, 0, false, data); token.Wait() && token.Error() != nil {
-		logger.Log(2, "error publishing peer update to peer "+node.ID+" "+token.Error().Error())
+		logger.Log(2, "error publishing peer update to peer ", node.ID, token.Error().Error())
 		return err
 	}
 	return nil

+ 4 - 3
netclient/daemon/macos.go

@@ -10,6 +10,7 @@ import (
 )
 
 const MAC_SERVICE_NAME = "com.gravitl.netclient"
+const EXEC_DIR = "/sbin/"
 
 // SetupMacDaemon - Creates a daemon service from the netclient under LaunchAgents for MacOS
 func SetupMacDaemon(interval string) error {
@@ -20,8 +21,8 @@ func SetupMacDaemon(interval string) error {
 	}
 	binarypath := dir + "/netclient"
 
-	if !ncutils.FileExists("/sbin/netclient") {
-		err = ncutils.Copy(binarypath, "/sbin/netclient")
+	if !ncutils.FileExists(EXEC_DIR + "netclient") {
+		err = ncutils.Copy(binarypath, EXEC_DIR+"netclient")
 		if err != nil {
 			log.Println(err)
 			return err
@@ -51,7 +52,7 @@ func CleanupMac() {
 	}
 
 	os.RemoveAll(ncutils.GetNetclientPath())
-	os.Remove("/sbin/netclient")
+	os.Remove(EXEC_DIR + "netclient")
 }
 
 // CreateMacService - Creates the mac service file for LaunchDaemons

+ 6 - 4
netclient/daemon/systemd.go

@@ -10,6 +10,8 @@ import (
 	"github.com/gravitl/netmaker/netclient/ncutils"
 )
 
+const EXEC_DIR = "/usr/sbin/"
+
 // SetupSystemDDaemon - sets system daemon for supported machines
 func SetupSystemDDaemon(interval string) error {
 
@@ -31,8 +33,8 @@ func SetupSystemDDaemon(interval string) error {
 	}
 	//install binary
 	//should check if the existing binary is the corect version -- for now only copy if file doesn't exist
-	if !ncutils.FileExists("/usr/sbin/netclient") {
-		err = ncutils.Copy(binarypath, "/usr/sbin/netclient")
+	if !ncutils.FileExists(EXEC_DIR + "netclient") {
+		err = ncutils.Copy(binarypath, EXEC_DIR+"netclient")
 		if err != nil {
 			log.Println(err)
 			return err
@@ -61,7 +63,7 @@ WantedBy=multi-user.target
 	}
 	_, _ = ncutils.RunCmd("systemctl enable netclient.service", true)
 	_, _ = ncutils.RunCmd("systemctl daemon-reload", true)
-	_, _ = ncutils.RunCmd("systemctl start netclient.server", true)
+	_, _ = ncutils.RunCmd("systemctl start netclient.service", true)
 	return nil
 }
 
@@ -69,7 +71,7 @@ func CleanupLinux() {
 	if err := os.RemoveAll(ncutils.GetNetclientPath()); err != nil {
 		ncutils.PrintLog("Removing netclient configs: "+err.Error(), 1)
 	}
-	if err := os.Remove("/usr/sbin/netclient"); err != nil {
+	if err := os.Remove(EXEC_DIR + "netclient"); err != nil {
 		ncutils.PrintLog("Removing netclient binary: "+err.Error(), 1)
 	}
 }