[UDN isolation] Fix ACLs tier: move to the highest-prio Primary tier.

Start using new BuildACL for all functions that need non-default tier.

Signed-off-by: Nadia Pinaeva <[email protected]>

Author: npinaeva

go-controller/pkg/libovsdb/util/acl.go

@@ -135,8 +135,7 @@ func BuildACL(dbIDs *libovsdbops.DbObjectIDs, priority int, match, action string
 }
 
 func BuildANPACL(dbIDs *libovsdbops.DbObjectIDs, priority int, match, action string, aclT ACLPipelineType, logLevels *ACLLoggingLevels) *nbdb.ACL {
-	anpACL := BuildACLWithDefaultTier(dbIDs, priority, match, action, logLevels, aclT)
-	anpACL.Tier = GetACLTier(dbIDs)
+	anpACL := BuildACL(dbIDs, priority, match, action, logLevels, aclT, GetACLTier(dbIDs))
 	return anpACL
 }
 

go-controller/pkg/ovn/udn_isolation.go

@@ -30,6 +30,8 @@ const (
 	DenySecondaryACL      = "DenySecondary"
 	// OpenPortACLPrefix is used to build per-pod ACLs, pod name should be added to the prefix to build a unique name
 	OpenPortACLPrefix = "OpenPort-"
+	// the same tier is used for all UDN isolation ACLs
+	isolationTier = types.PrimaryACLTier
 )
 
 // setupUDNACLs should be called after the node's management port was configured
@@ -63,7 +65,8 @@ func (oc *DefaultNetworkController) setupUDNACLs(mgmtPortIPs []net.IP) error {
 	pgName := libovsdbutil.GetPortGroupName(pgIDs)
 	egressDenyIDs := oc.getUDNACLDbIDs(DenySecondaryACL, libovsdbutil.ACLEgress)
 	match := libovsdbutil.GetACLMatch(pgName, "", libovsdbutil.ACLEgress)
-	egressDenyACL := libovsdbutil.BuildACLWithDefaultTier(egressDenyIDs, types.PrimaryUDNDenyPriority, match, nbdb.ACLActionDrop, nil, libovsdbutil.LportEgress)
+	egressDenyACL := libovsdbutil.BuildACL(egressDenyIDs, types.PrimaryUDNDenyPriority, match, nbdb.ACLActionDrop,
+		nil, libovsdbutil.LportEgress, isolationTier)
 
 	getARPMatch := func(direction libovsdbutil.ACLDirection) string {
 		match := "("
@@ -89,15 +92,18 @@ func (oc *DefaultNetworkController) setupUDNACLs(mgmtPortIPs []net.IP) error {
 
 	egressARPIDs := oc.getUDNACLDbIDs(AllowHostARPACL, libovsdbutil.ACLEgress)
 	match = libovsdbutil.GetACLMatch(pgName, getARPMatch(libovsdbutil.ACLEgress), libovsdbutil.ACLEgress)
-	egressARPACL := libovsdbutil.BuildACLWithDefaultTier(egressARPIDs, types.PrimaryUDNAllowPriority, match, nbdb.ACLActionAllow, nil, libovsdbutil.LportEgress)
+	egressARPACL := libovsdbutil.BuildACL(egressARPIDs, types.PrimaryUDNAllowPriority, match, nbdb.ACLActionAllow,
+		nil, libovsdbutil.LportEgress, isolationTier)
 
 	ingressDenyIDs := oc.getUDNACLDbIDs(DenySecondaryACL, libovsdbutil.ACLIngress)
 	match = libovsdbutil.GetACLMatch(pgName, "", libovsdbutil.ACLIngress)
-	ingressDenyACL := libovsdbutil.BuildACLWithDefaultTier(ingressDenyIDs, types.PrimaryUDNDenyPriority, match, nbdb.ACLActionDrop, nil, libovsdbutil.LportIngress)
+	ingressDenyACL := libovsdbutil.BuildACL(ingressDenyIDs, types.PrimaryUDNDenyPriority, match, nbdb.ACLActionDrop,
+		nil, libovsdbutil.LportIngress, isolationTier)
 
 	ingressARPIDs := oc.getUDNACLDbIDs(AllowHostARPACL, libovsdbutil.ACLIngress)
 	match = libovsdbutil.GetACLMatch(pgName, getARPMatch(libovsdbutil.ACLIngress), libovsdbutil.ACLIngress)
-	ingressARPACL := libovsdbutil.BuildACLWithDefaultTier(ingressARPIDs, types.PrimaryUDNAllowPriority, match, nbdb.ACLActionAllow, nil, libovsdbutil.LportIngress)
+	ingressARPACL := libovsdbutil.BuildACL(ingressARPIDs, types.PrimaryUDNAllowPriority, match, nbdb.ACLActionAllow,
+		nil, libovsdbutil.LportIngress, isolationTier)
 
 	ingressAllowIDs := oc.getUDNACLDbIDs(AllowHostSecondaryACL, libovsdbutil.ACLIngress)
 	match = "("
@@ -114,7 +120,8 @@ func (oc *DefaultNetworkController) setupUDNACLs(mgmtPortIPs []net.IP) error {
 	}
 	match += ")"
 	match = libovsdbutil.GetACLMatch(pgName, match, libovsdbutil.ACLIngress)
-	ingressAllowACL := libovsdbutil.BuildACLWithDefaultTier(ingressAllowIDs, types.PrimaryUDNAllowPriority, match, nbdb.ACLActionAllowRelated, nil, libovsdbutil.LportIngress)
+	ingressAllowACL := libovsdbutil.BuildACL(ingressAllowIDs, types.PrimaryUDNAllowPriority, match, nbdb.ACLActionAllowRelated,
+		nil, libovsdbutil.LportIngress, isolationTier)
 
 	ops, err := libovsdbops.CreateOrUpdateACLsOps(oc.nbClient, nil, oc.GetSamplingConfig(), egressDenyACL, egressARPACL, ingressARPACL, ingressDenyACL, ingressAllowACL)
 	if err != nil {
@@ -199,12 +206,12 @@ func (oc *DefaultNetworkController) setUDNPodOpenPortsOps(podNamespacedName stri
 	ingressMatch, egressMatch, parseErr := getPortsMatches(podAnnotations, lspName)
 	// don't return on parseErr, as we need to cleanup potentially present ACLs from the previous config
 	ingressIDs := oc.getUDNOpenPortDbIDs(podNamespacedName, libovsdbutil.ACLIngress)
-	ingressACL := libovsdbutil.BuildACLWithDefaultTier(ingressIDs, types.PrimaryUDNAllowPriority,
-		ingressMatch, nbdb.ACLActionAllowRelated, nil, libovsdbutil.LportIngress)
+	ingressACL := libovsdbutil.BuildACL(ingressIDs, types.PrimaryUDNAllowPriority,
+		ingressMatch, nbdb.ACLActionAllowRelated, nil, libovsdbutil.LportIngress, isolationTier)
 
 	egressIDs := oc.getUDNOpenPortDbIDs(podNamespacedName, libovsdbutil.ACLEgress)
-	egressACL := libovsdbutil.BuildACLWithDefaultTier(egressIDs, types.PrimaryUDNAllowPriority,
-		egressMatch, nbdb.ACLActionAllow, nil, libovsdbutil.LportEgress)
+	egressACL := libovsdbutil.BuildACL(egressIDs, types.PrimaryUDNAllowPriority,
+		egressMatch, nbdb.ACLActionAllow, nil, libovsdbutil.LportEgress, isolationTier)
 
 	var err error
 	if ingressMatch == "" && egressMatch == "" || parseErr != nil {
@@ -282,14 +289,14 @@ func BuildAdvertisedNetworkSubnetsDropACL(advertisedNetworkSubnetsAddressSet add
 		dropMatches = append(dropMatches, fmt.Sprintf("(ip6.src == $%s && ip6.dst == $%s)", v6AddrSet, v6AddrSet))
 	}
 
-	dropACL := libovsdbutil.BuildACLWithDefaultTier(
+	dropACL := libovsdbutil.BuildACL(
 		GetAdvertisedNetworkSubnetsDropACLdbIDs(),
 		types.AdvertisedNetworkDenyPriority,
 		strings.Join(dropMatches, " || "),
 		nbdb.ACLActionDrop,
 		nil,
-		libovsdbutil.LportEgressAfterLB)
-	dropACL.Tier = types.PrimaryACLTier
+		libovsdbutil.LportEgressAfterLB,
+		isolationTier)
 	return dropACL
 }
 
@@ -325,14 +332,14 @@ func (bnc *BaseNetworkController) addAdvertisedNetworkIsolation(nodeName string)
 	ops = append(ops, addrOps...)
 
 	if len(passMatches) > 0 {
-		passACL := libovsdbutil.BuildACLWithDefaultTier(
+		passACL := libovsdbutil.BuildACL(
 			GetAdvertisedNetworkSubnetsPassACLdbIDs(bnc.controllerName, bnc.GetNetworkName(), bnc.GetNetworkID()),
 			types.AdvertisedNetworkPassPriority,
 			strings.Join(passMatches, " || "),
 			nbdb.ACLActionPass,
 			nil,
-			libovsdbutil.LportEgressAfterLB)
-		passACL.Tier = types.PrimaryACLTier
+			libovsdbutil.LportEgressAfterLB,
+			isolationTier)
 
 		ops, err = libovsdbops.CreateOrUpdateACLsOps(bnc.nbClient, ops, nil, passACL)
 		if err != nil {

go-controller/pkg/ovn/udn_isolation_test.go

@@ -0,0 +1,53 @@
+package ovn
+
+import (
+	"github.com/ovn-org/ovn-kubernetes/go-controller/pkg/config"
+	libovsdbops "github.com/ovn-org/ovn-kubernetes/go-controller/pkg/libovsdb/ops"
+	libovsdbutil "github.com/ovn-org/ovn-kubernetes/go-controller/pkg/libovsdb/util"
+	"github.com/ovn-org/ovn-kubernetes/go-controller/pkg/nbdb"
+	libovsdbtest "github.com/ovn-org/ovn-kubernetes/go-controller/pkg/testing/libovsdb"
+	"github.com/ovn-org/ovn-kubernetes/go-controller/pkg/types"
+
+	. "github.com/onsi/ginkgo/v2"
+	. "github.com/onsi/gomega"
+)
+
+var _ = Describe("UDN Isolation", func() {
+	BeforeEach(func() {
+		Expect(config.PrepareTestConfig()).To(Succeed())
+	})
+
+	It("ACLs should be updated to the Primary tier ", func() {
+		config.OVNKubernetesFeature.EnableMultiNetwork = true
+		config.OVNKubernetesFeature.EnableNetworkSegmentation = true
+		fakeController := getFakeController(DefaultNetworkControllerName)
+
+		// build port group with one ACL that has default tier
+		pgIDs := fakeController.getSecondaryPodsPortGroupDbIDs()
+		pgName := libovsdbutil.GetPortGroupName(pgIDs)
+		egressDenyIDs := fakeController.getUDNACLDbIDs(DenySecondaryACL, libovsdbutil.ACLEgress)
+		match := libovsdbutil.GetACLMatch(pgName, "", libovsdbutil.ACLEgress)
+		// in the real code we use BuildACL here instead of BuildACLWithDefaultTier
+		egressDenyACL := libovsdbutil.BuildACLWithDefaultTier(egressDenyIDs, types.PrimaryUDNDenyPriority, match, nbdb.ACLActionDrop,
+			nil, libovsdbutil.LportEgress)
+		// required to make sure port group correctly references the ACL
+		egressDenyACL.UUID = egressDenyIDs.String() + "-UUID"
+		pg := libovsdbutil.BuildPortGroup(pgIDs, nil, []*nbdb.ACL{egressDenyACL})
+
+		nbClient, nbCleanup, err := libovsdbtest.NewNBTestHarness(libovsdbtest.TestSetup{
+			NBData: []libovsdbtest.TestData{egressDenyACL, pg},
+		}, nil)
+		Expect(err).NotTo(HaveOccurred())
+		defer nbCleanup.Cleanup()
+		fakeController.nbClient = nbClient
+
+		// now run the setupUDNACLs function which should create all ACLs and update the existing ACLs to the Primary tier
+		Expect(fakeController.setupUDNACLs(nil)).To(Succeed())
+
+		// verify that the egressDenyACL is updated to the Primary 0
+		acls, err := libovsdbops.FindACLs(nbClient, []*nbdb.ACL{egressDenyACL})
+		Expect(err).NotTo(HaveOccurred())
+		Expect(acls).To(HaveLen(1))
+		Expect(acls[0].Tier).To(Equal(types.PrimaryACLTier))
+	})
+})