From b93defd18b58677302728b9eda7645cba5d2ad38 Mon Sep 17 00:00:00 2001
From: Rain Suo <yugsuo@gmail.com>
Date: Wed, 27 Nov 2024 11:04:49 +0800
Subject: [PATCH] delete legacy denyall acls when upgrading to v1.13.x (#4742)

the acls in v1.13.x are in tier 2 rather than tier 0 in v1.12.x, which
results that legacy denyall sg will drop all traffics if a pod bound a sg,
because acls in tier 0 have the higest priority. we should recreate acls
in denyall sg when upgrading to v1.13.x.

Signed-off-by: Rain Suo <yugsuo@gmail.com>
---
 pkg/ovs/ovn-nb-acl.go      | 26 +++++++++++++++++++++++
 pkg/ovs/ovn-nb-acl_test.go | 43 ++++++++++++++++++++++++++++++++++++++
 pkg/util/const.go          |  1 +
 3 files changed, 70 insertions(+)

diff --git a/pkg/ovs/ovn-nb-acl.go b/pkg/ovs/ovn-nb-acl.go
index 07143253ec6..db5c5f49c4d 100644
--- a/pkg/ovs/ovn-nb-acl.go
+++ b/pkg/ovs/ovn-nb-acl.go
@@ -277,6 +277,32 @@ func (c *OVNNbClient) CreateNodeACL(pgName, nodeIPStr, joinIPStr string) error {
 func (c *OVNNbClient) CreateSgDenyAllACL(sgName string) error {
 	pgName := GetSgPortGroupName(sgName)
 
+	// for upgrading from v1.12.x to v1.13.x
+	// see https://github.com/kubeovn/kube-ovn/issues/4742
+	oldIngressACL, err := c.GetACL(pgName, ovnnb.ACLDirectionToLport, util.SecurityGroupDropPriority, fmt.Sprintf("outport == @%s && ip", pgName), true)
+	if err != nil {
+		klog.Error(err)
+		return err
+	}
+	if oldIngressACL != nil && oldIngressACL.Tier == util.DefaultACLTier {
+		if err := c.DeleteACL(pgName, portGroupKey, ovnnb.ACLDirectionToLport, util.SecurityGroupDropPriority, fmt.Sprintf("outport == @%s && ip", pgName)); err != nil {
+			klog.Errorf("delete legacy acl from port group %s: %v", pgName, err)
+			return err
+		}
+	}
+	oldEgressACL, err := c.GetACL(pgName, ovnnb.ACLDirectionFromLport, util.SecurityGroupDropPriority, fmt.Sprintf("inport == @%s && ip", pgName), true)
+	if err != nil {
+		klog.Error(err)
+		return err
+	}
+	if oldEgressACL != nil && oldEgressACL.Tier == util.DefaultACLTier {
+		if err := c.DeleteACL(pgName, portGroupKey, ovnnb.ACLDirectionFromLport, util.SecurityGroupDropPriority, fmt.Sprintf("inport == @%s && ip", pgName)); err != nil {
+			klog.Errorf("delete legacy acl from port group %s: %v", pgName, err)
+			return err
+		}
+	}
+
+	// create new acls for deny all sg
 	ingressACL, err := c.newACL(pgName, ovnnb.ACLDirectionToLport, util.SecurityGroupDropPriority, fmt.Sprintf("outport == @%s && ip", pgName), ovnnb.ACLActionDrop, util.NetpolACLTier)
 	if err != nil {
 		klog.Error(err)
diff --git a/pkg/ovs/ovn-nb-acl_test.go b/pkg/ovs/ovn-nb-acl_test.go
index 5ac5a6f3445..523a4eb407b 100644
--- a/pkg/ovs/ovn-nb-acl_test.go
+++ b/pkg/ovs/ovn-nb-acl_test.go
@@ -612,6 +612,49 @@ func (suite *OvnClientTestSuite) testCreateSgDenyAllACL() {
 		require.Contains(t, pg.ACLs, egressACL.UUID)
 	})
 
+	t.Run("upgrading create sg deny all acl", func(t *testing.T) {
+		sgName := "test_create_deny_all_acl_pg"
+		pgName := GetSgPortGroupName(sgName)
+
+		err := nbClient.CreatePortGroup(pgName, nil)
+		require.NoError(t, err)
+
+		// init legacy acls
+		legacyIngressACL, err := nbClient.newACL(pgName, ovnnb.ACLDirectionToLport, util.SecurityGroupDropPriority, fmt.Sprintf("outport == @%s && ip", pgName), ovnnb.ACLActionDrop, util.DefaultACLTier)
+		require.NoError(t, err)
+
+		legacyEgressACL, err := nbClient.newACL(pgName, ovnnb.ACLDirectionFromLport, util.SecurityGroupDropPriority, fmt.Sprintf("inport == @%s && ip", pgName), ovnnb.ACLActionDrop, util.DefaultACLTier)
+		require.NoError(t, err)
+
+		err = nbClient.CreateAcls(pgName, portGroupKey, legacyIngressACL, legacyEgressACL)
+		require.NoError(t, err)
+
+		// create deny all acls normally
+		err = nbClient.CreateSgDenyAllACL(sgName)
+		require.NoError(t, err)
+
+		pg, err := nbClient.GetPortGroup(pgName, false)
+		require.NoError(t, err)
+
+		// ingress acl
+		match := fmt.Sprintf("outport == @%s && ip", pgName)
+		ingressACL, err := nbClient.GetACL(pgName, ovnnb.ACLDirectionToLport, util.SecurityGroupDropPriority, match, false)
+		require.NoError(t, err)
+		expect := newACL(pgName, ovnnb.ACLDirectionToLport, util.SecurityGroupDropPriority, match, ovnnb.ACLActionDrop, util.NetpolACLTier)
+		expect.UUID = ingressACL.UUID
+		require.Equal(t, expect, ingressACL)
+		require.Contains(t, pg.ACLs, ingressACL.UUID)
+
+		// egress acl
+		match = fmt.Sprintf("inport == @%s && ip", pgName)
+		egressACL, err := nbClient.GetACL(pgName, ovnnb.ACLDirectionFromLport, util.SecurityGroupDropPriority, match, false)
+		require.NoError(t, err)
+		expect = newACL(pgName, ovnnb.ACLDirectionFromLport, util.SecurityGroupDropPriority, match, ovnnb.ACLActionDrop, util.NetpolACLTier)
+		expect.UUID = egressACL.UUID
+		require.Equal(t, expect, egressACL)
+		require.Contains(t, pg.ACLs, egressACL.UUID)
+	})
+
 	t.Run("should print log err when sg name does not exist", func(t *testing.T) {
 		sgName := "test_nonexist_pg"
 		err := nbClient.CreateSgDenyAllACL(sgName)
diff --git a/pkg/util/const.go b/pkg/util/const.go
index 8acda2daaf0..9fd399a4569 100644
--- a/pkg/util/const.go
+++ b/pkg/util/const.go
@@ -157,6 +157,7 @@ const (
 	AnpMaxPriority       = 99
 	AnpACLMaxPriority    = 30000
 	BanpACLMaxPriority   = 1800
+	DefaultACLTier       = 0
 	AnpACLTier           = 1
 	NetpolACLTier        = 2
 	BanpACLTier          = 3