Merge pull request #2839 from gravitl/NET-1047

NET-1047: ACLs mutex fix
gravitl · Mar 4, 2024 · c45f7bf · c45f7bf
2 parents 077ec49 + 8e05807
commit c45f7bf
Show file tree

Hide file tree

Showing 4 changed files with 3 additions and 7 deletions.
diff --git a/logic/acls/nodeacls/modify.go b/logic/acls/nodeacls/modify.go
@@ -77,14 +77,12 @@ func RemoveNodeACL(networkID NetworkID, nodeID NodeID) (acls.ACLContainer, error
 	if err != nil {
 		return nil, err
 	}
-	acls.AclMutex.Lock()
 	for currentNodeID := range currentNetworkACL {
 		if NodeID(currentNodeID) != nodeID {
 			currentNetworkACL[currentNodeID].Remove(acls.AclID(nodeID))
 		}
 	}
 	delete(currentNetworkACL, acls.AclID(nodeID))
-	acls.AclMutex.Unlock()
 	return currentNetworkACL.Save(acls.ContainerID(networkID))
 }
 

diff --git a/logic/acls/nodeacls/retrieve.go b/logic/acls/nodeacls/retrieve.go
@@ -15,8 +15,10 @@ func AreNodesAllowed(networkID NetworkID, node1, node2 NodeID) bool {
 	}
 	var allowed bool
 	acls.AclMutex.RLock()
-	allowed = currentNetworkACL[acls.AclID(node1)].IsAllowed(acls.AclID(node2)) && currentNetworkACL[acls.AclID(node2)].IsAllowed(acls.AclID(node1))
+	currNetworkACLNode1 := currentNetworkACL[acls.AclID(node1)]
+	currNetworkACLNode2 := currentNetworkACL[acls.AclID(node2)]
 	acls.AclMutex.RUnlock()
+	allowed = currNetworkACLNode1.IsAllowed(acls.AclID(node2)) && currNetworkACLNode2.IsAllowed(acls.AclID(node1))
 	return allowed
 }
 

diff --git a/logic/hosts.go b/logic/hosts.go
@@ -418,7 +418,6 @@ func DissasociateNodeFromHost(n *models.Node, h *models.Host) error {
 	if err := DeleteNodeByID(n); err != nil {
 		return err
 	}
-
 	return UpsertHost(h)
 }
 

diff --git a/logic/nodes.go b/logic/nodes.go
@@ -189,7 +189,6 @@ func UpdateNode(currentNode *models.Node, newNode *models.Node) error {
 func DeleteNode(node *models.Node, purge bool) error {
 	alreadyDeleted := node.PendingDelete || node.Action == models.NODE_DELETE
 	node.Action = models.NODE_DELETE
-
 	//delete ext clients if node is ingress gw
 	if node.IsIngressGateway {
 		if err := DeleteGatewayExtClients(node.ID.String(), node.Network); err != nil {
@@ -235,7 +234,6 @@ func DeleteNode(node *models.Node, purge bool) error {
 	if node.IsInternetGateway {
 		UnsetInternetGw(node)
 	}
-
 	if !purge && !alreadyDeleted {
 		newnode := *node
 		newnode.PendingDelete = true
@@ -281,7 +279,6 @@ func GetNodeByHostRef(hostid, network string) (node models.Node, err error) {
 func DeleteNodeByID(node *models.Node) error {
 	var err error
 	var key = node.ID.String()
-
 	if err = database.DeleteRecord(database.NODES_TABLE_NAME, key); err != nil {
 		if !database.IsEmptyRecord(err) {
 			return err
-Original file line number
+Diff line change
@@ Expand Up @@
     	if err := DeleteNodeByID(n); err != nil {
     		return err
     	}
     	return UpsertHost(h)
     }
@@ Expand Down @@