refactored some logic to use typed functions

controllers/config/dnsconfig/netmaker.hosts

@@ -1 +1,2 @@
-10.0.0.2         testnode.skynet myhost.skynet
+10.0.0.1         testnode.skynet
+10.0.0.2         myhost.skynet

controllers/node_test.go

@@ -5,7 +5,6 @@ import (
 
 	"github.com/gravitl/netmaker/database"
 	"github.com/gravitl/netmaker/logic"
-	"github.com/gravitl/netmaker/logic/acls"
 	nodeacls "github.com/gravitl/netmaker/logic/acls/node-acls"
 	"github.com/gravitl/netmaker/models"
 	"github.com/stretchr/testify/assert"
@@ -151,50 +150,52 @@ func TestNodeACLs(t *testing.T) {
 	node2 := models.Node{PublicKey: "DM5qhLAE20FG7BbfBCger+Ac9D2NDOwCtY1rbYDXf14=", Name: "testnode", Endpoint: "10.0.0.100", MacAddress: "01:02:03:04:05:07", Password: "password", Network: "skynet", OS: "linux"}
 	logic.CreateNode(&node1)
 	logic.CreateNode(&node2)
-	currentACL, err := nodeacls.CreateNetworkACL(acls.NetworkID(node1.Network))
 	t.Run("acls not present", func(t *testing.T) {
+		currentACL, err := nodeacls.CreateNetworkACL(nodeacls.NetworkID(node1.Network))
 		assert.Nil(t, err)
-		assert.Nil(t, currentACL[acls.NodeID(node1.ID)])
+		assert.Nil(t, currentACL[nodeacls.NodeID(node1.ID)])
-		assert.Nil(t, currentACL[acls.NodeID(node2.ID)])
+		assert.Nil(t, currentACL[nodeacls.NodeID(node2.ID)])
-		node1ACL, err := nodeacls.FetchNodeACL(acls.NetworkID(node1.Network), acls.NodeID(node1.ID))
+		node1ACL, err := nodeacls.FetchNodeACL(nodeacls.NetworkID(node1.Network), nodeacls.NodeID(node1.ID))
 		assert.NotNil(t, err)
 		assert.Nil(t, node1ACL)
 		assert.EqualError(t, err, "no node ACL present for node "+node1.ID)
 	})
 	t.Run("node acls exists after creates", func(t *testing.T) {
-		node1ACL, err := nodeacls.CreateNodeACL(acls.NetworkID(node1.Network), acls.NodeID(node1.ID), acls.Allowed)
+		node1ACL, err := nodeacls.CreateNodeACL(nodeacls.NetworkID(node1.Network), nodeacls.NodeID(node1.ID), nodeacls.Allowed)
 		assert.Nil(t, err)
 		assert.NotNil(t, node1ACL)
-		assert.Equal(t, node1ACL[acls.NodeID(node2.ID)], acls.NotPresent)
+		assert.Equal(t, node1ACL[nodeacls.NodeID(node2.ID)], nodeacls.NotPresent)
-		node2ACL, err := nodeacls.CreateNodeACL(acls.NetworkID(node1.Network), acls.NodeID(node2.ID), acls.Allowed)
+		node2ACL, err := nodeacls.CreateNodeACL(nodeacls.NetworkID(node1.Network), nodeacls.NodeID(node2.ID), nodeacls.Allowed)
 		assert.Nil(t, err)
 		assert.NotNil(t, node2ACL)
-		assert.Equal(t, acls.Allowed, node2ACL[acls.NodeID(node1.ID)])
+		assert.Equal(t, nodeacls.Allowed, node2ACL[nodeacls.NodeID(node1.ID)])
 	})
 	t.Run("node acls correct after fetch", func(t *testing.T) {
-		node1ACL, err := nodeacls.FetchNodeACL(acls.NetworkID(node1.Network), acls.NodeID(node1.ID))
+		node1ACL, err := nodeacls.FetchNodeACL(nodeacls.NetworkID(node1.Network), nodeacls.NodeID(node1.ID))
 		assert.Nil(t, err)
-		assert.Equal(t, acls.Allowed, node1ACL[acls.NodeID(node2.ID)])
+		assert.Equal(t, nodeacls.Allowed, node1ACL[nodeacls.NodeID(node2.ID)])
 	})
 	t.Run("node acls correct after modify", func(t *testing.T) {
-		retNetworkACL, err := nodeacls.ChangeNodeACL(acls.NetworkID(node1.Network), acls.NodeID(node1.ID), acls.NodeID(node2.ID), acls.NotAllowed)
+		currentACL, err := nodeacls.CreateNetworkACL(nodeacls.NetworkID(node1.Network))
 		assert.Nil(t, err)
-		assert.NotNil(t, retNetworkACL)
+		assert.NotNil(t, currentACL)
-		assert.Equal(t, acls.NotAllowed, retNetworkACL[acls.NodeID(node1.ID)][acls.NodeID(node2.ID)])
+		node1ACL, err := nodeacls.CreateNodeACL(nodeacls.NetworkID(node1.Network), nodeacls.NodeID(node1.ID), nodeacls.Allowed)
-		assert.Equal(t, acls.NotAllowed, retNetworkACL[acls.NodeID(node2.ID)][acls.NodeID(node1.ID)])
-	})
-	t.Run("node acls correct after erroneous modify", func(t *testing.T) {
-		retNetworkACL, err := nodeacls.ChangeNodeACL(acls.NetworkID(node1.Network), acls.NodeID(node1.ID), acls.NodeID(node2.ID), acls.NotPresent)
 		assert.Nil(t, err)
-		assert.NotNil(t, retNetworkACL)
+		node2ACL, err := nodeacls.CreateNodeACL(nodeacls.NetworkID(node1.Network), nodeacls.NodeID(node2.ID), nodeacls.Allowed)
-		assert.Equal(t, acls.NotAllowed, retNetworkACL[acls.NodeID(node1.ID)][acls.NodeID(node2.ID)])
+		assert.Nil(t, err)
-		assert.Equal(t, acls.NotAllowed, retNetworkACL[acls.NodeID(node2.ID)][acls.NodeID(node1.ID)])
+		assert.NotNil(t, node1ACL)
+		assert.NotNil(t, node2ACL)
+		currentACL, err = nodeacls.FetchCurrentACL(nodeacls.NetworkID(node1.Network))
+		assert.Nil(t, err)
+		currentACL.ChangeNodesAccess(nodeacls.NodeID(node1.ID), nodeacls.NodeID(node2.ID), nodeacls.NotAllowed)
+		assert.Equal(t, nodeacls.NotAllowed, currentACL[nodeacls.NodeID(node1.ID)][nodeacls.NodeID(node2.ID)])
+		assert.Equal(t, nodeacls.NotAllowed, currentACL[nodeacls.NodeID(node2.ID)][nodeacls.NodeID(node1.ID)])
 	})
 	t.Run("node acls removed", func(t *testing.T) {
-		retNetworkACL, err := nodeacls.RemoveNodeACL(acls.NetworkID(node1.Network), acls.NodeID(node1.ID))
+		retNetworkACL, err := nodeacls.RemoveNodeACL(nodeacls.NetworkID(node1.Network), nodeacls.NodeID(node1.ID))
 		assert.Nil(t, err)
 		assert.NotNil(t, retNetworkACL)
-		assert.Equal(t, acls.NotPresent, retNetworkACL[acls.NodeID(node2.ID)][acls.NodeID(node1.ID)])
+		assert.Equal(t, nodeacls.NotPresent, retNetworkACL[nodeacls.NodeID(node2.ID)][nodeacls.NodeID(node1.ID)])
 	})
 
 	deleteAllNodes()

logic/acls/node-acls/modify.go

@@ -4,41 +4,24 @@ import (
 	"encoding/json"
 
 	"github.com/gravitl/netmaker/database"
-	"github.com/gravitl/netmaker/logic/acls"
 )
 
-// ChangeNodeACL - takes in two node IDs of a given network and changes them to specified allowed or not value
-// returns the total network's ACL and error
-func ChangeNodeACL(networkID acls.NetworkID, node1, node2 acls.NodeID, value byte) (acls.NetworkACL, error) {
-	if value != acls.NotAllowed && value != acls.Allowed { // if invalid option make not allowed
-		value = acls.NotAllowed
-	}
-	currentACL, err := FetchCurrentACL(networkID)
-	if err != nil {
-		return nil, err
-	}
-	// == make the access control change ==
-	currentACL[node1][node2] = value
-	currentACL[node2][node1] = value
-	return UpsertNetworkACL(networkID, currentACL)
-}
-
 // CreateNodeACL - inserts or updates a node ACL on given network
-func CreateNodeACL(networkID acls.NetworkID, nodeID acls.NodeID, defaultVal byte) (acls.NodeACL, error) {
+func CreateNodeACL(networkID NetworkID, nodeID NodeID, defaultVal byte) (NodeACL, error) {
-	if defaultVal != acls.NotAllowed && defaultVal != acls.Allowed {
+	if defaultVal != NotAllowed && defaultVal != Allowed {
-		defaultVal = acls.NotAllowed
+		defaultVal = NotAllowed
 	}
 	var currentNetworkACL, err = FetchCurrentACL(networkID)
 	if err != nil {
 		return nil, err
 	}
-	var newNodeACL = make(acls.NodeACL)
+	var newNodeACL = make(NodeACL)
 	for existingNodeID := range currentNetworkACL {
 		currentNetworkACL[existingNodeID][nodeID] = defaultVal // set the old nodes to default value for new node
 		newNodeACL[existingNodeID] = defaultVal                // set the old nodes in new node ACL to default value
 	}
 	currentNetworkACL[nodeID] = newNodeACL                               // append the new node's ACL
-	retNetworkACL, err := UpsertNetworkACL(networkID, currentNetworkACL) // insert into db, return result
+	retNetworkACL, err := upsertNetworkACL(networkID, currentNetworkACL) // insert into db, return result
 	if err != nil {
 		return nil, err
 	}
@@ -46,55 +29,105 @@ func CreateNodeACL(networkID acls.NetworkID, nodeID acls.NodeID, defaultVal byte
 }
 
 // CreateNetworkACL - creates an empty ACL list in a given network
-func CreateNetworkACL(networkID acls.NetworkID) (acls.NetworkACL, error) {
+func CreateNetworkACL(networkID NetworkID) (NetworkACL, error) {
-	var networkACL = make(acls.NetworkACL)
+	var networkACL = make(NetworkACL)
-	return networkACL, database.Insert(string(networkID), string(convertNetworkACLtoACLJson(&networkACL)), database.NODE_ACLS_TABLE_NAME)
-}
-
-// UpsertNodeACL - applies a NodeACL to the db, overwrites or creates
-func UpsertNodeACL(networkID acls.NetworkID, nodeID acls.NodeID, nodeACL acls.NodeACL) (acls.NodeACL, error) {
-	currentNetACL, err := FetchCurrentACL(networkID)
-	if err != nil {
-		return nodeACL, err
-	}
-	currentNetACL[nodeID] = nodeACL
-	_, err = UpsertNetworkACL(networkID, currentNetACL)
-	return nodeACL, err
-}
-
-// UpsertNetworkACL - Inserts or updates a network ACL given the json string of the ACL and the network name
-// if nil, create it
-func UpsertNetworkACL(networkID acls.NetworkID, networkACL acls.NetworkACL) (acls.NetworkACL, error) {
-	if networkACL == nil {
-		networkACL = make(acls.NetworkACL)
-	}
 	return networkACL, database.Insert(string(networkID), string(convertNetworkACLtoACLJson(&networkACL)), database.NODE_ACLS_TABLE_NAME)
 }
 
 // RemoveNodeACL - removes a specific Node's ACL, returns the NetworkACL and error
-func RemoveNodeACL(networkID acls.NetworkID, nodeID acls.NodeID) (acls.NetworkACL, error) {
+func RemoveNodeACL(networkID NetworkID, nodeID NodeID) (NetworkACL, error) {
 	var currentNeworkACL, err = FetchCurrentACL(networkID)
 	if err != nil {
 		return nil, err
 	}
 	for currentNodeID := range currentNeworkACL {
 		if currentNodeID != nodeID {
-			delete(currentNeworkACL[currentNodeID], nodeID)
+			currentNeworkACL[currentNodeID].RemoveNode(nodeID)
 		}
 	}
 	delete(currentNeworkACL, nodeID)
-	return UpsertNetworkACL(networkID, currentNeworkACL)
+	return currentNeworkACL.Save(networkID)
 }
 
 // RemoveNetworkACL - just delete the network ACL
-func RemoveNetworkACL(networkID acls.NetworkID) error {
+func RemoveNetworkACL(networkID NetworkID) error {
 	return database.DeleteRecord(database.NODE_ACLS_TABLE_NAME, string(networkID))
 }
 
-func convertNetworkACLtoACLJson(networkACL *acls.NetworkACL) acls.ACLJson {
+// NodeACL.AllowNode - allows a node by ID in memory
+func (nodeACL NodeACL) AllowNode(nodeID NodeID) {
+	nodeACL[nodeID] = Allowed
+}
+
+// NodeACL.DisallowNode - disallows a node access by ID in memory
+func (nodeACL NodeACL) DisallowNode(nodeID NodeID) {
+	nodeACL[nodeID] = NotAllowed
+}
+
+// NodeACL.RemoveNode - removes a node from a NodeACL
+func (nodeACL NodeACL) RemoveNode(nodeID NodeID) {
+	delete(nodeACL, nodeID)
+}
+
+// NodeACL.Update - updates a nodeACL in DB
+func (nodeACL NodeACL) Save(networkID NetworkID, nodeID NodeID) (NodeACL, error) {
+	return upsertNodeACL(networkID, nodeID, nodeACL)
+}
+
+// NodeACL.IsNodeAllowed - sees if nodeID is allowed in referring NodeACL
+func (nodeACL NodeACL) IsNodeAllowed(nodeID NodeID) bool {
+	return nodeACL[nodeID] == Allowed
+}
+
+// NetworkACL.UpdateNodeACL - saves the state of a NodeACL in the NetworkACL in memory
+func (networkACL NetworkACL) UpdateNodeACL(nodeID NodeID, nodeACL NodeACL) NetworkACL {
+	networkACL[nodeID] = nodeACL
+	return networkACL
+}
+
+// NetworkACL.RemoveNodeACL - removes the state of a NodeACL in the NetworkACL in memory
+func (networkACL NetworkACL) RemoveNodeACL(nodeID NodeID) NetworkACL {
+	delete(networkACL, nodeID)
+	return networkACL
+}
+
+// NetworkACL.ChangeNodesAccess - changes the relationship between two nodes in memory
+func (networkACL NetworkACL) ChangeNodesAccess(nodeID1, nodeID2 NodeID, value byte) {
+	networkACL[nodeID1][nodeID2] = value
+	networkACL[nodeID2][nodeID1] = value
+}
+
+// NetworkACL.Save - saves the state of a NetworkACL to the db
+func (networkACL NetworkACL) Save(networkID NetworkID) (NetworkACL, error) {
+	return upsertNetworkACL(networkID, networkACL)
+}
+
+// == private ==
+
+// upsertNodeACL - applies a NodeACL to the db, overwrites or creates
+func upsertNodeACL(networkID NetworkID, nodeID NodeID, nodeACL NodeACL) (NodeACL, error) {
+	currentNetACL, err := FetchCurrentACL(networkID)
+	if err != nil {
+		return nodeACL, err
+	}
+	currentNetACL[nodeID] = nodeACL
+	_, err = upsertNetworkACL(networkID, currentNetACL)
+	return nodeACL, err
+}
+
+// upsertNetworkACL - Inserts or updates a network ACL given the json string of the ACL and the network name
+// if nil, create it
+func upsertNetworkACL(networkID NetworkID, networkACL NetworkACL) (NetworkACL, error) {
+	if networkACL == nil {
+		networkACL = make(NetworkACL)
+	}
+	return networkACL, database.Insert(string(networkID), string(convertNetworkACLtoACLJson(&networkACL)), database.NODE_ACLS_TABLE_NAME)
+}
+
+func convertNetworkACLtoACLJson(networkACL *NetworkACL) ACLJson {
 	data, err := json.Marshal(networkACL)
 	if err != nil {
 		return ""
 	}
-	return acls.ACLJson(data)
+	return ACLJson(data)
 }

logic/acls/node-acls/retrieve.go

@@ -5,20 +5,19 @@ import (
 	"fmt"
 
 	"github.com/gravitl/netmaker/database"
-	"github.com/gravitl/netmaker/logic/acls"
 )
 
 // AreNodesAllowed - checks if nodes are allowed to communicate in their network ACL
-func AreNodesAllowed(networkID acls.NetworkID, node1, node2 acls.NodeID) bool {
+func AreNodesAllowed(networkID NetworkID, node1, node2 NodeID) bool {
 	var currentNetworkACL, err = FetchCurrentACL(networkID)
 	if err != nil {
 		return false
 	}
-	return currentNetworkACL[node1][node2] == acls.Allowed && currentNetworkACL[node2][node1] == acls.Allowed
+	return currentNetworkACL[node1].IsNodeAllowed(node2) && currentNetworkACL[node2].IsNodeAllowed(node1)
 }
 
 // FetchNodeACL - fetches a specific node's ACL in a given network
-func FetchNodeACL(networkID acls.NetworkID, nodeID acls.NodeID) (acls.NodeACL, error) {
+func FetchNodeACL(networkID NetworkID, nodeID NodeID) (NodeACL, error) {
 	currentNetACL, err := FetchCurrentACL(networkID)
 	if err != nil {
 		return nil, err
@@ -30,7 +29,7 @@ func FetchNodeACL(networkID acls.NetworkID, nodeID acls.NodeID) (acls.NodeACL, e
 }
 
 // FetchNodeACLJson - fetches a node's acl in given network except returns the json string
-func FetchNodeACLJson(networkID acls.NetworkID, nodeID acls.NodeID) (acls.ACLJson, error) {
+func FetchNodeACLJson(networkID NetworkID, nodeID NodeID) (ACLJson, error) {
 	currentNodeACL, err := FetchNodeACL(networkID, nodeID)
 	if err != nil {
 		return "", err
@@ -39,16 +38,16 @@ func FetchNodeACLJson(networkID acls.NetworkID, nodeID acls.NodeID) (acls.ACLJso
 	if err != nil {
 		return "", err
 	}
-	return acls.ACLJson(jsonData), nil
+	return ACLJson(jsonData), nil
 }
 
 // FetchCurrentACL - fetches all current node rules in given network ACL
-func FetchCurrentACL(networkID acls.NetworkID) (acls.NetworkACL, error) {
+func FetchCurrentACL(networkID NetworkID) (NetworkACL, error) {
-	aclJson, err := FetchCurrentACLJson(acls.NetworkID(networkID))
+	aclJson, err := FetchCurrentACLJson(NetworkID(networkID))
 	if err != nil {
 		return nil, err
 	}
-	var currentNetworkACL acls.NetworkACL
+	var currentNetworkACL NetworkACL
 	if err := json.Unmarshal([]byte(aclJson), &currentNetworkACL); err != nil {
 		return nil, err
 	}
@@ -56,10 +55,10 @@ func FetchCurrentACL(networkID acls.NetworkID) (acls.NetworkACL, error) {
 }
 
 // FetchCurrentACLJson - fetch the current ACL of given network except in json string
-func FetchCurrentACLJson(networkID acls.NetworkID) (acls.ACLJson, error) {
+func FetchCurrentACLJson(networkID NetworkID) (ACLJson, error) {
 	currentACLs, err := database.FetchRecord(database.NODE_ACLS_TABLE_NAME, string(networkID))
 	if err != nil {
-		return acls.ACLJson(""), err
+		return ACLJson(""), err
 	}
-	return acls.ACLJson(currentACLs), nil
+	return ACLJson(currentACLs), nil
 }

logic/acls/node-acls/types.go

@@ -1,4 +1,4 @@
-package acls
+package nodeacls
 
 var (
 	// NotPresent - 0 - not present (default)