diff --git a/mocks/pkg/ovs/interface.go b/mocks/pkg/ovs/interface.go index 98cbc0553c9..1741de4224b 100644 --- a/mocks/pkg/ovs/interface.go +++ b/mocks/pkg/ovs/interface.go @@ -1871,32 +1871,32 @@ func (mr *MockACLMockRecorder) CreateSgDenyAllACL(sgName any) *gomock.Call { } // DeleteAcls mocks base method. -func (m *MockACL) DeleteAcls(parentName, parentType, direction string, externalIDs map[string]string) error { +func (m *MockACL) DeleteAcls(parentName, parentType, direction string, externalIDs map[string]string, tier int) error { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "DeleteAcls", parentName, parentType, direction, externalIDs) + ret := m.ctrl.Call(m, "DeleteAcls", parentName, parentType, direction, externalIDs, tier) ret0, _ := ret[0].(error) return ret0 } // DeleteAcls indicates an expected call of DeleteAcls. -func (mr *MockACLMockRecorder) DeleteAcls(parentName, parentType, direction, externalIDs any) *gomock.Call { +func (mr *MockACLMockRecorder) DeleteAcls(parentName, parentType, direction, externalIDs, tier any) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteAcls", reflect.TypeOf((*MockACL)(nil).DeleteAcls), parentName, parentType, direction, externalIDs) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteAcls", reflect.TypeOf((*MockACL)(nil).DeleteAcls), parentName, parentType, direction, externalIDs, tier) } // DeleteAclsOps mocks base method. -func (m *MockACL) DeleteAclsOps(parentName, parentType, direction string, externalIDs map[string]string) ([]ovsdb.Operation, error) { +func (m *MockACL) DeleteAclsOps(parentName, parentType, direction string, externalIDs map[string]string, tier int) ([]ovsdb.Operation, error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "DeleteAclsOps", parentName, parentType, direction, externalIDs) + ret := m.ctrl.Call(m, "DeleteAclsOps", parentName, parentType, direction, externalIDs, tier) ret0, _ := ret[0].([]ovsdb.Operation) ret1, _ := ret[1].(error) return ret0, ret1 } // DeleteAclsOps indicates an expected call of DeleteAclsOps. -func (mr *MockACLMockRecorder) DeleteAclsOps(parentName, parentType, direction, externalIDs any) *gomock.Call { +func (mr *MockACLMockRecorder) DeleteAclsOps(parentName, parentType, direction, externalIDs, tier any) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteAclsOps", reflect.TypeOf((*MockACL)(nil).DeleteAclsOps), parentName, parentType, direction, externalIDs) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteAclsOps", reflect.TypeOf((*MockACL)(nil).DeleteAclsOps), parentName, parentType, direction, externalIDs, tier) } // SGLostACL mocks base method. @@ -3150,32 +3150,32 @@ func (mr *MockNbClientMockRecorder) CreateVirtualLogicalSwitchPorts(lsName any, } // DeleteAcls mocks base method. -func (m *MockNbClient) DeleteAcls(parentName, parentType, direction string, externalIDs map[string]string) error { +func (m *MockNbClient) DeleteAcls(parentName, parentType, direction string, externalIDs map[string]string, tier int) error { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "DeleteAcls", parentName, parentType, direction, externalIDs) + ret := m.ctrl.Call(m, "DeleteAcls", parentName, parentType, direction, externalIDs, tier) ret0, _ := ret[0].(error) return ret0 } // DeleteAcls indicates an expected call of DeleteAcls. -func (mr *MockNbClientMockRecorder) DeleteAcls(parentName, parentType, direction, externalIDs any) *gomock.Call { +func (mr *MockNbClientMockRecorder) DeleteAcls(parentName, parentType, direction, externalIDs, tier any) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteAcls", reflect.TypeOf((*MockNbClient)(nil).DeleteAcls), parentName, parentType, direction, externalIDs) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteAcls", reflect.TypeOf((*MockNbClient)(nil).DeleteAcls), parentName, parentType, direction, externalIDs, tier) } // DeleteAclsOps mocks base method. -func (m *MockNbClient) DeleteAclsOps(parentName, parentType, direction string, externalIDs map[string]string) ([]ovsdb.Operation, error) { +func (m *MockNbClient) DeleteAclsOps(parentName, parentType, direction string, externalIDs map[string]string, tier int) ([]ovsdb.Operation, error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "DeleteAclsOps", parentName, parentType, direction, externalIDs) + ret := m.ctrl.Call(m, "DeleteAclsOps", parentName, parentType, direction, externalIDs, tier) ret0, _ := ret[0].([]ovsdb.Operation) ret1, _ := ret[1].(error) return ret0, ret1 } // DeleteAclsOps indicates an expected call of DeleteAclsOps. -func (mr *MockNbClientMockRecorder) DeleteAclsOps(parentName, parentType, direction, externalIDs any) *gomock.Call { +func (mr *MockNbClientMockRecorder) DeleteAclsOps(parentName, parentType, direction, externalIDs, tier any) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteAclsOps", reflect.TypeOf((*MockNbClient)(nil).DeleteAclsOps), parentName, parentType, direction, externalIDs) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteAclsOps", reflect.TypeOf((*MockNbClient)(nil).DeleteAclsOps), parentName, parentType, direction, externalIDs, tier) } // DeleteAddressSet mocks base method. diff --git a/pkg/controller/admin_network_policy.go b/pkg/controller/admin_network_policy.go index aaa9814f068..b40475a702e 100644 --- a/pkg/controller/admin_network_policy.go +++ b/pkg/controller/admin_network_policy.go @@ -190,7 +190,7 @@ func (c *Controller) handleAddAnp(key string) (err error) { return err } - ingressACLOps, err := c.OVNNbClient.DeleteAclsOps(pgName, portGroupKey, "to-lport", nil) + ingressACLOps, err := c.OVNNbClient.DeleteAclsOps(pgName, portGroupKey, "to-lport", nil, -1) if err != nil { klog.Errorf("failed to generate clear operations for anp %s ingress acls: %v", key, err) return err @@ -266,7 +266,7 @@ func (c *Controller) handleAddAnp(key string) (err error) { return fmt.Errorf("failed to delete unused ingress address set for anp %s: %w", key, err) } - egressACLOps, err := c.OVNNbClient.DeleteAclsOps(pgName, portGroupKey, "from-lport", nil) + egressACLOps, err := c.OVNNbClient.DeleteAclsOps(pgName, portGroupKey, "from-lport", nil, -1) if err != nil { klog.Errorf("failed to generate clear operations for anp %s egress acls: %v", key, err) return err diff --git a/pkg/controller/baseline_admin_network_policy.go b/pkg/controller/baseline_admin_network_policy.go index 819d7a18872..82d900dfc32 100644 --- a/pkg/controller/baseline_admin_network_policy.go +++ b/pkg/controller/baseline_admin_network_policy.go @@ -148,7 +148,7 @@ func (c *Controller) handleAddBanp(key string) (err error) { return err } - ingressACLOps, err := c.OVNNbClient.DeleteAclsOps(pgName, portGroupKey, "to-lport", nil) + ingressACLOps, err := c.OVNNbClient.DeleteAclsOps(pgName, portGroupKey, "to-lport", nil, -1) if err != nil { klog.Errorf("failed to generate clear operations for banp %s ingress acls: %v", key, err) return err @@ -225,7 +225,7 @@ func (c *Controller) handleAddBanp(key string) (err error) { return fmt.Errorf("failed to delete unused ingress address set for banp %s: %w", key, err) } - egressACLOps, err := c.OVNNbClient.DeleteAclsOps(pgName, portGroupKey, "from-lport", nil) + egressACLOps, err := c.OVNNbClient.DeleteAclsOps(pgName, portGroupKey, "from-lport", nil, -1) if err != nil { klog.Errorf("failed to generate clear operations for banp %s egress acls: %v", key, err) return err diff --git a/pkg/controller/controller.go b/pkg/controller/controller.go index 7f7c0529592..741fa5c6159 100644 --- a/pkg/controller/controller.go +++ b/pkg/controller/controller.go @@ -995,6 +995,8 @@ func (c *Controller) Run(ctx context.Context) { } } + c.handleUpgrading() + // start workers to do all the network operations c.startWorkers(ctx) @@ -1131,6 +1133,21 @@ func (c *Controller) shutdown() { } } +func (c *Controller) handleUpgrading() { + if err := c.upgradeSecurityGroups(); err != nil { + util.LogFatalAndExit(err, "failed to upgrade security groups") + } + if err := c.upgradeSubnets(); err != nil { + util.LogFatalAndExit(err, "failed to upgrade subnets") + } + if err := c.upgradeNetworkPolicies(); err != nil { + util.LogFatalAndExit(err, "failed to upgrade network policies") + } + if err := c.upgradeNodes(); err != nil { + util.LogFatalAndExit(err, "failed to upgrade nodes") + } +} + func (c *Controller) startWorkers(ctx context.Context) { klog.Info("Starting workers") diff --git a/pkg/controller/network_policy.go b/pkg/controller/network_policy.go index 55835a8feb6..3ed9a912647 100644 --- a/pkg/controller/network_policy.go +++ b/pkg/controller/network_policy.go @@ -62,6 +62,40 @@ func (c *Controller) enqueueUpdateNp(oldObj, newObj interface{}) { } } +// for upgrading from v1.12.x to v1.13.x +func (c *Controller) upgradeNetworkPoliciesForV1_13() error { + // clear legacy acls in tier 0 for all network policies + // including ingress, egress and subnet gateway acls + nps, err := c.npsLister.NetworkPolicies(corev1.NamespaceAll).List(labels.Everything()) + if err != nil { + klog.Errorf("failed to list network policies %v", err) + return err + } + + for _, np := range nps { + npName := np.Name + nameArray := []rune(np.Name) + if !unicode.IsLetter(nameArray[0]) { + npName = "np" + np.Name + } + pgName := strings.ReplaceAll(fmt.Sprintf("%s.%s", npName, np.Namespace), "-", ".") + + if err = c.OVNNbClient.DeleteAcls(pgName, portGroupKey, "", nil, util.DefaultACLTier); err != nil { + klog.Errorf("clear legacy network policy %s acls: %v", pgName, err) + return err + } + } + + return nil +} + +func (c *Controller) upgradeNetworkPolicies() error { + if err := c.upgradeNetworkPoliciesForV1_13(); err != nil { + return err + } + return nil +} + func (c *Controller) createAsForNetpol(ns, name, direction, asName string, addresses []string) error { if err := c.OVNNbClient.CreateAddressSet(asName, map[string]string{ networkPolicyKey: fmt.Sprintf("%s/%s/%s", ns, name, direction), @@ -165,7 +199,7 @@ func (c *Controller) handleUpdateNp(key string) error { return err } - ingressACLOps, err := c.OVNNbClient.DeleteAclsOps(pgName, portGroupKey, "to-lport", nil) + ingressACLOps, err := c.OVNNbClient.DeleteAclsOps(pgName, portGroupKey, "to-lport", nil, -1) if err != nil { klog.Errorf("generate operations that clear np %s ingress acls: %v", key, err) return err @@ -281,7 +315,7 @@ func (c *Controller) handleUpdateNp(key string) error { } } } else { - if err = c.OVNNbClient.DeleteAcls(pgName, portGroupKey, "to-lport", nil); err != nil { + if err = c.OVNNbClient.DeleteAcls(pgName, portGroupKey, "to-lport", nil, -1); err != nil { klog.Errorf("delete np %s ingress acls: %v", key, err) return err } @@ -294,7 +328,7 @@ func (c *Controller) handleUpdateNp(key string) error { } } - egressACLOps, err := c.OVNNbClient.DeleteAclsOps(pgName, portGroupKey, "from-lport", nil) + egressACLOps, err := c.OVNNbClient.DeleteAclsOps(pgName, portGroupKey, "from-lport", nil, -1) if err != nil { klog.Errorf("generate operations that clear np %s egress acls: %v", key, err) return err @@ -408,7 +442,7 @@ func (c *Controller) handleUpdateNp(key string) error { } } } else { - if err = c.OVNNbClient.DeleteAcls(pgName, portGroupKey, "from-lport", nil); err != nil { + if err = c.OVNNbClient.DeleteAcls(pgName, portGroupKey, "from-lport", nil, -1); err != nil { klog.Errorf("delete np %s egress acls: %v", key, err) return err } diff --git a/pkg/controller/node.go b/pkg/controller/node.go index 343d6731685..af3c778bafc 100644 --- a/pkg/controller/node.go +++ b/pkg/controller/node.go @@ -94,6 +94,38 @@ func nodeUnderlayAddressSetName(node string, af int) string { return fmt.Sprintf("node_%s_underlay_v%d", strings.ReplaceAll(node, "-", "_"), af) } +// for upgrading from v1.12.x to v1.13.x +func (c *Controller) upgradeNodesForV1_13() error { + // clear legacy acls in tier 0 for node port group + nodes, err := c.nodesLister.List(labels.Everything()) + if err != nil { + klog.Errorf("failed to list nodes: %v", err) + return err + } + + for _, node := range nodes { + pgName := strings.ReplaceAll(node.Annotations[util.PortNameAnnotation], "-", ".") + + if pgName == "" { + continue + } + + if err = c.OVNNbClient.DeleteAcls(pgName, portGroupKey, "", nil, util.DefaultACLTier); err != nil { + klog.Errorf("delete legacy node acl for node pg %s: %v", pgName, err) + return err + } + } + + return nil +} + +func (c *Controller) upgradeNodes() error { + if err := c.upgradeNodesForV1_13(); err != nil { + return err + } + return nil +} + func (c *Controller) handleAddNode(key string) error { c.nodeKeyMutex.LockKey(key) defer func() { _ = c.nodeKeyMutex.UnlockKey(key) }() @@ -786,7 +818,7 @@ func (c *Controller) checkAndUpdateNodePortGroup() error { } } else { // clear all acl - if err = c.OVNNbClient.DeleteAcls(pgName, portGroupKey, "", nil); err != nil { + if err = c.OVNNbClient.DeleteAcls(pgName, portGroupKey, "", nil, -1); err != nil { klog.Errorf("delete node acl for node pg %s: %v", pgName, err) } } diff --git a/pkg/controller/security_group.go b/pkg/controller/security_group.go index 8e9890f5a3b..fe92fd08fb6 100644 --- a/pkg/controller/security_group.go +++ b/pkg/controller/security_group.go @@ -62,6 +62,39 @@ func (c *Controller) enqueueDeleteSg(obj interface{}) { c.delSgQueue.Add(key) } +// for upgrading from v1.12.x to v1.13.x +func (c *Controller) upgradeSecurityGroupsForV1_13() error { + // clear legacy acls in tier 0 for deny all sg + pgName := ovs.GetSgPortGroupName(util.DenyAllSecurityGroup) + if err := c.OVNNbClient.DeleteAcls(pgName, portGroupKey, "", nil, util.DefaultACLTier); err != nil { + klog.Error(err) + return fmt.Errorf("delete legacy acls from port group %s: %w", pgName, err) + } + + // clear legacy acls in tier 0 for all sg port groups + sgs, err := c.sgsLister.List(labels.Everything()) + if err != nil { + klog.Errorf("failed to list security groups: %v", err) + return err + } + for _, sg := range sgs { + pgName := ovs.GetSgPortGroupName(sg.Name) + if err := c.OVNNbClient.DeleteAcls(pgName, portGroupKey, "", nil, util.DefaultACLTier); err != nil { + klog.Error(err) + return fmt.Errorf("delete legacy acls from port group %s: %w", pgName, err) + } + } + + return nil +} + +func (c *Controller) upgradeSecurityGroups() error { + if err := c.upgradeSecurityGroupsForV1_13(); err != nil { + return err + } + return nil +} + func (c *Controller) initDefaultDenyAllSecurityGroup() error { pgName := ovs.GetSgPortGroupName(util.DenyAllSecurityGroup) if err := c.OVNNbClient.CreatePortGroup(pgName, map[string]string{ diff --git a/pkg/controller/subnet.go b/pkg/controller/subnet.go index 7c506f252ff..25e629d6307 100644 --- a/pkg/controller/subnet.go +++ b/pkg/controller/subnet.go @@ -152,6 +152,31 @@ func (c *Controller) enqueueUpdateSubnet(oldObj, newObj interface{}) { } } +// for upgrading from v1.12.x to v1.13.x +func (c *Controller) upgradeSubnetsForV1_13() error { + // clear legacy acls in tier 0 for all subnets + subnets, err := c.subnetsLister.List(labels.Everything()) + if err != nil { + klog.Errorf("failed to list subnets %v", err) + return err + } + + for _, subnet := range subnets { + if err = c.OVNNbClient.DeleteAcls(subnet.Name, logicalSwitchKey, "", nil, util.DefaultACLTier); err != nil { + klog.Errorf("clear legacy logical switch %s acls: %v", subnet.Name, err) + return err + } + } + return nil +} + +func (c *Controller) upgradeSubnets() error { + if err := c.upgradeSubnetsForV1_13(); err != nil { + return err + } + return nil +} + func (c *Controller) formatSubnet(subnet *kubeovnv1.Subnet) (*kubeovnv1.Subnet, error) { var ( changed bool @@ -790,7 +815,7 @@ func (c *Controller) handleAddOrUpdateSubnet(key string) error { } } else { // clear acl when direction is "" - if err = c.OVNNbClient.DeleteAcls(subnet.Name, logicalSwitchKey, "", nil); err != nil { + if err = c.OVNNbClient.DeleteAcls(subnet.Name, logicalSwitchKey, "", nil, -1); err != nil { if err = c.patchSubnetStatus(subnet, "ResetLogicalSwitchAclFailed", err.Error()); err != nil { klog.Error(err) return err @@ -890,7 +915,7 @@ func (c *Controller) handleDeleteLogicalSwitch(key string) (err error) { } // clear acl when direction is "" - if err = c.OVNNbClient.DeleteAcls(key, logicalSwitchKey, "", nil); err != nil { + if err = c.OVNNbClient.DeleteAcls(key, logicalSwitchKey, "", nil, -1); err != nil { klog.Errorf("clear logical switch %s acls: %v", key, err) return err } diff --git a/pkg/ovs/interface.go b/pkg/ovs/interface.go index 67a230b9a16..612fa39f32c 100644 --- a/pkg/ovs/interface.go +++ b/pkg/ovs/interface.go @@ -162,8 +162,8 @@ type ACL interface { SetACLLog(pgName string, logEnable, isIngress bool) error SetLogicalSwitchPrivate(lsName, cidrBlock, nodeSwitchCIDR string, allowSubnets []string) error SGLostACL(sg *kubeovnv1.SecurityGroup) (bool, error) - DeleteAcls(parentName, parentType, direction string, externalIDs map[string]string) error - DeleteAclsOps(parentName, parentType, direction string, externalIDs map[string]string) ([]ovsdb.Operation, error) + DeleteAcls(parentName, parentType, direction string, externalIDs map[string]string, tier int) error + DeleteAclsOps(parentName, parentType, direction string, externalIDs map[string]string, tier int) ([]ovsdb.Operation, error) UpdateAnpRuleACLOps(pgName, asName, protocol, aclName string, priority int, aclAction ovnnb.ACLAction, logACLActions []ovnnb.ACLAction, rulePorts []v1alpha1.AdminNetworkPolicyPort, isIngress, isBanp bool) ([]ovsdb.Operation, error) } diff --git a/pkg/ovs/ovn-nb-acl.go b/pkg/ovs/ovn-nb-acl.go index 07143253ec6..54ad501ca5c 100644 --- a/pkg/ovs/ovn-nb-acl.go +++ b/pkg/ovs/ovn-nb-acl.go @@ -384,7 +384,7 @@ func (c *OVNNbClient) UpdateSgACL(sg *kubeovnv1.SecurityGroup, direction string) pgName := GetSgPortGroupName(sg.Name) // clear acl - if err := c.DeleteAcls(pgName, portGroupKey, direction, nil); err != nil { + if err := c.DeleteAcls(pgName, portGroupKey, direction, nil, -1); err != nil { klog.Error(err) return fmt.Errorf("delete direction '%s' acls from port group %s: %w", direction, pgName, err) } @@ -441,7 +441,7 @@ func (c *OVNNbClient) UpdateSgACL(sg *kubeovnv1.SecurityGroup, direction string) } func (c *OVNNbClient) UpdateLogicalSwitchACL(lsName, cidrBlock string, subnetAcls []kubeovnv1.ACL, allowEWTraffic bool) error { - if err := c.DeleteAcls(lsName, logicalSwitchKey, "", map[string]string{"subnet": lsName}); err != nil { + if err := c.DeleteAcls(lsName, logicalSwitchKey, "", map[string]string{"subnet": lsName}, -1); err != nil { klog.Error(err) return fmt.Errorf("delete subnet acls from %s: %w", lsName, err) } @@ -530,7 +530,7 @@ func (c *OVNNbClient) UpdateACL(acl *ovnnb.ACL, fields ...interface{}) error { // SetLogicalSwitchPrivate will drop all ingress traffic except allow subnets, same subnet and node subnet func (c *OVNNbClient) SetLogicalSwitchPrivate(lsName, cidrBlock, nodeSwitchCIDR string, allowSubnets []string) error { // clear acls - if err := c.DeleteAcls(lsName, logicalSwitchKey, "", nil); err != nil { + if err := c.DeleteAcls(lsName, logicalSwitchKey, "", nil, -1); err != nil { klog.Error(err) return fmt.Errorf("clear logical switch %s acls: %w", lsName, err) } @@ -731,8 +731,8 @@ func (c *OVNNbClient) CreateBareACL(parentName, direction, priority, match, acti // DeleteAcls delete several acl once, // delete to-lport and from-lport direction acl when direction is empty, otherwise one-way // parentType is 'ls' or 'pg' -func (c *OVNNbClient) DeleteAcls(parentName, parentType, direction string, externalIDs map[string]string) error { - ops, err := c.DeleteAclsOps(parentName, parentType, direction, externalIDs) +func (c *OVNNbClient) DeleteAcls(parentName, parentType, direction string, externalIDs map[string]string, tier int) error { + ops, err := c.DeleteAclsOps(parentName, parentType, direction, externalIDs, tier) if err != nil { klog.Error(err) return err @@ -822,14 +822,16 @@ func (c *OVNNbClient) GetACL(parent, direction, priority, match string, ignoreNo // result should include all to-lport and from-lport acls when direction is empty, // result should include all acls when externalIDs is empty, // result should include all acls which externalIDs[key] is not empty when externalIDs[key] is "" +// result should include all acls when tier is -1 +// result should include all acls in specific tier when tier is not -1 // TODO: maybe add other filter conditions(priority or match) -func (c *OVNNbClient) ListAcls(direction string, externalIDs map[string]string) ([]ovnnb.ACL, error) { +func (c *OVNNbClient) ListAcls(direction string, externalIDs map[string]string, tier int) ([]ovnnb.ACL, error) { ctx, cancel := context.WithTimeout(context.Background(), c.Timeout) defer cancel() aclList := make([]ovnnb.ACL, 0) - if err := c.WhereCache(aclFilter(direction, externalIDs)).List(ctx, &aclList); err != nil { + if err := c.WhereCache(aclFilter(direction, externalIDs, tier)).List(ctx, &aclList); err != nil { klog.Error(err) return nil, fmt.Errorf("list acls: %w", err) } @@ -1091,8 +1093,10 @@ func newNetworkPolicyACLMatch(pgName, asAllowName, asExceptName, protocol, direc // result should include all to-lport and from-lport acls when direction is empty, // result should include all acls when externalIDs is empty, // result should include all acls which externalIDs[key] is not empty when externalIDs[key] is "" +// result should include all acls when tier is -1 +// result should include all acls in specific tier when tier is not -1 // TODO: maybe add other filter conditions(priority or match) -func aclFilter(direction string, externalIDs map[string]string) func(acl *ovnnb.ACL) bool { +func aclFilter(direction string, externalIDs map[string]string, tier int) func(acl *ovnnb.ACL) bool { return func(acl *ovnnb.ACL) bool { if len(acl.ExternalIDs) < len(externalIDs) { return false @@ -1118,6 +1122,10 @@ func aclFilter(direction string, externalIDs map[string]string) func(acl *ovnnb. return false } + if tier != -1 && acl.Tier != tier { + return false + } + return true } } @@ -1173,7 +1181,7 @@ func (c *OVNNbClient) CreateAclsOps(parentName, parentType string, acls ...*ovnn // DeleteAcls return operation which delete several acl once, // delete to-lport and from-lport direction acl when direction is empty, otherwise one-way // parentType is 'ls' or 'pg' -func (c *OVNNbClient) DeleteAclsOps(parentName, parentType, direction string, externalIDs map[string]string) ([]ovsdb.Operation, error) { +func (c *OVNNbClient) DeleteAclsOps(parentName, parentType, direction string, externalIDs map[string]string, tier int) ([]ovsdb.Operation, error) { if parentName == "" { return nil, errors.New("the port group name or logical switch name is required") } @@ -1185,7 +1193,7 @@ func (c *OVNNbClient) DeleteAclsOps(parentName, parentType, direction string, ex externalIDs[aclParentKey] = parentName /* delete acls from port group or logical switch */ - acls, err := c.ListAcls(direction, externalIDs) + acls, err := c.ListAcls(direction, externalIDs, tier) if err != nil { klog.Error(err) return nil, fmt.Errorf("list type %s %s acls: %w", parentType, parentName, err) diff --git a/pkg/ovs/ovn-nb-acl_test.go b/pkg/ovs/ovn-nb-acl_test.go index 5ac5a6f3445..50170e43605 100644 --- a/pkg/ovs/ovn-nb-acl_test.go +++ b/pkg/ovs/ovn-nb-acl_test.go @@ -1393,6 +1393,93 @@ func (suite *OvnClientTestSuite) testDeleteAcls() { err = nbClient.CreateBareLogicalSwitch(lsName) require.NoError(t, err) + t.Run("delete legacy acls from port group", func(t *testing.T) { + priority := "5601" + basePort := 5601 + acls := make([]*ovnnb.ACL, 0, 5) + + // create legacy acls + // to-lport + for i := 0; i < 2; i++ { + match := fmt.Sprintf("%s && tcp.dst == %d", matchPrefix, basePort+i) + acl, err := nbClient.newACL(pgName, ovnnb.ACLDirectionToLport, priority, match, ovnnb.ACLActionAllowRelated, util.DefaultACLTier) + require.NoError(t, err) + acls = append(acls, acl) + } + + // from-lport + for i := 0; i < 3; i++ { + match := fmt.Sprintf("%s && tcp.dst == %d", matchPrefix, basePort+i) + acl, err := nbClient.newACL(pgName, ovnnb.ACLDirectionFromLport, priority, match, ovnnb.ACLActionAllowRelated, util.DefaultACLTier) + require.NoError(t, err) + acls = append(acls, acl) + } + + err = nbClient.CreateAcls(pgName, portGroupKey, acls...) + require.NoError(t, err) + + pg, err := nbClient.GetPortGroup(pgName, false) + require.NoError(t, err) + require.Len(t, pg.ACLs, 5) + + err = nbClient.DeleteAcls(pgName, portGroupKey, "", nil, util.DefaultACLTier) + require.NoError(t, err) + + pg, err = nbClient.GetPortGroup(pgName, false) + require.NoError(t, err) + require.Empty(t, pg.ACLs) + }) + + t.Run("delete acls in specific tier from port group", func(t *testing.T) { + priority := "5601" + basePort := 5601 + acls := make([]*ovnnb.ACL, 0, 5) + + // to-lport + for i := 0; i < 2; i++ { + match := fmt.Sprintf("%s && tcp.dst == %d", matchPrefix, basePort+i) + + // tier 0 + acl0, err := nbClient.newACL(pgName, ovnnb.ACLDirectionToLport, priority, match, ovnnb.ACLActionAllowRelated, util.DefaultACLTier) + require.NoError(t, err) + acls = append(acls, acl0) + + // tier 2 + acl2, err := nbClient.newACL(pgName, ovnnb.ACLDirectionToLport, priority, match, ovnnb.ACLActionAllowRelated, util.NetpolACLTier) + require.NoError(t, err) + acls = append(acls, acl2) + } + + // from-lport + for i := 0; i < 3; i++ { + match := fmt.Sprintf("%s && tcp.dst == %d", matchPrefix, basePort+i) + + // tier 0 + acl0, err := nbClient.newACL(pgName, ovnnb.ACLDirectionFromLport, priority, match, ovnnb.ACLActionAllowRelated, util.DefaultACLTier) + require.NoError(t, err) + acls = append(acls, acl0) + + // tier 2 + acl2, err := nbClient.newACL(pgName, ovnnb.ACLDirectionFromLport, priority, match, ovnnb.ACLActionAllowRelated, util.NetpolACLTier) + require.NoError(t, err) + acls = append(acls, acl2) + } + + err = nbClient.CreateAcls(pgName, portGroupKey, acls...) + require.NoError(t, err) + + pg, err := nbClient.GetPortGroup(pgName, false) + require.NoError(t, err) + require.Len(t, pg.ACLs, 10) + + err = nbClient.DeleteAcls(pgName, portGroupKey, "", nil, util.DefaultACLTier) + require.NoError(t, err) + + pg, err = nbClient.GetPortGroup(pgName, false) + require.NoError(t, err) + require.Len(t, pg.ACLs, 5) + }) + t.Run("delete all direction acls from port group", func(t *testing.T) { priority := "5601" basePort := 5601 @@ -1421,7 +1508,7 @@ func (suite *OvnClientTestSuite) testDeleteAcls() { require.NoError(t, err) require.Len(t, pg.ACLs, 5) - err = nbClient.DeleteAcls(pgName, portGroupKey, "", nil) + err = nbClient.DeleteAcls(pgName, portGroupKey, "", nil, -1) require.NoError(t, err) pg, err = nbClient.GetPortGroup(pgName, false) @@ -1458,7 +1545,7 @@ func (suite *OvnClientTestSuite) testDeleteAcls() { require.Len(t, pg.ACLs, 5) /* delete to-lport direction acl */ - err = nbClient.DeleteAcls(pgName, portGroupKey, ovnnb.ACLDirectionToLport, nil) + err = nbClient.DeleteAcls(pgName, portGroupKey, ovnnb.ACLDirectionToLport, nil, -1) require.NoError(t, err) pg, err = nbClient.GetPortGroup(pgName, false) @@ -1466,7 +1553,7 @@ func (suite *OvnClientTestSuite) testDeleteAcls() { require.Len(t, pg.ACLs, 3) /* delete from-lport direction acl */ - err = nbClient.DeleteAcls(pgName, portGroupKey, ovnnb.ACLDirectionFromLport, nil) + err = nbClient.DeleteAcls(pgName, portGroupKey, ovnnb.ACLDirectionFromLport, nil, -1) require.NoError(t, err) pg, err = nbClient.GetPortGroup(pgName, false) @@ -1502,7 +1589,7 @@ func (suite *OvnClientTestSuite) testDeleteAcls() { require.NoError(t, err) require.Len(t, ls.ACLs, 5) - err = nbClient.DeleteAcls(lsName, logicalSwitchKey, "", nil) + err = nbClient.DeleteAcls(lsName, logicalSwitchKey, "", nil, -1) require.NoError(t, err) ls, err = nbClient.GetLogicalSwitch(lsName, false) @@ -1539,7 +1626,7 @@ func (suite *OvnClientTestSuite) testDeleteAcls() { require.Len(t, ls.ACLs, 5) /* delete to-lport direction acl */ - err = nbClient.DeleteAcls(lsName, logicalSwitchKey, ovnnb.ACLDirectionToLport, nil) + err = nbClient.DeleteAcls(lsName, logicalSwitchKey, ovnnb.ACLDirectionToLport, nil, -1) require.NoError(t, err) ls, err = nbClient.GetLogicalSwitch(lsName, false) @@ -1547,7 +1634,7 @@ func (suite *OvnClientTestSuite) testDeleteAcls() { require.Len(t, ls.ACLs, 3) /* delete from-lport direction acl */ - err = nbClient.DeleteAcls(lsName, logicalSwitchKey, ovnnb.ACLDirectionFromLport, nil) + err = nbClient.DeleteAcls(lsName, logicalSwitchKey, ovnnb.ACLDirectionFromLport, nil, -1) require.NoError(t, err) ls, err = nbClient.GetLogicalSwitch(lsName, false) @@ -1584,7 +1671,7 @@ func (suite *OvnClientTestSuite) testDeleteAcls() { require.NoError(t, err) /* delete to-lport direction acl */ - err = nbClient.DeleteAcls(lsName, logicalSwitchKey, ovnnb.ACLDirectionToLport, map[string]string{"subnet": lsName}) + err = nbClient.DeleteAcls(lsName, logicalSwitchKey, ovnnb.ACLDirectionToLport, map[string]string{"subnet": lsName}, -1) require.NoError(t, err) ls, err = nbClient.GetLogicalSwitch(lsName, false) @@ -1593,7 +1680,7 @@ func (suite *OvnClientTestSuite) testDeleteAcls() { }) t.Run("should no err when acls does not exist", func(t *testing.T) { - err = nbClient.DeleteAcls("test-nonexist-ls", logicalSwitchKey, ovnnb.ACLDirectionToLport, map[string]string{"subnet": "test-nonexist-ls"}) + err = nbClient.DeleteAcls("test-nonexist-ls", logicalSwitchKey, ovnnb.ACLDirectionToLport, map[string]string{"subnet": "test-nonexist-ls"}, -1) require.NoError(t, err) }) @@ -1615,7 +1702,7 @@ func (suite *OvnClientTestSuite) testDeleteAcls() { err = failedNbClient.CreateAcls(lsName, logicalSwitchKey, acls...) require.Error(t, err) // TODO:// should err but not for now - err = failedNbClient.DeleteAcls(lsName, logicalSwitchKey, ovnnb.ACLDirectionToLport, map[string]string{"subnet": lsName}) + err = failedNbClient.DeleteAcls(lsName, logicalSwitchKey, ovnnb.ACLDirectionToLport, map[string]string{"subnet": lsName}, -1) require.NoError(t, err) }) } @@ -1799,7 +1886,7 @@ func (suite *OvnClientTestSuite) testListAcls() { } /* list all direction acl */ - out, err := nbClient.ListAcls("", nil) + out, err := nbClient.ListAcls("", nil, -1) require.NoError(t, err) count := 0 for _, v := range out { @@ -2025,7 +2112,7 @@ func (suite *OvnClientTestSuite) testACLFilter() { } /* include all direction acl */ - filterFunc := aclFilter("", nil) + filterFunc := aclFilter("", nil, -1) count := 0 for _, acl := range acls { if filterFunc(acl) { @@ -2035,7 +2122,7 @@ func (suite *OvnClientTestSuite) testACLFilter() { require.Equal(t, count, 11) /* include all direction acl with external ids */ - filterFunc = aclFilter("", map[string]string{aclParentKey: pgName}) + filterFunc = aclFilter("", map[string]string{aclParentKey: pgName}, -1) count = 0 for _, acl := range acls { if filterFunc(acl) { @@ -2045,7 +2132,7 @@ func (suite *OvnClientTestSuite) testACLFilter() { require.Equal(t, count, 5) /* include to-lport acl */ - filterFunc = aclFilter(ovnnb.ACLDirectionToLport, nil) + filterFunc = aclFilter(ovnnb.ACLDirectionToLport, nil, -1) count = 0 for _, acl := range acls { if filterFunc(acl) { @@ -2055,7 +2142,7 @@ func (suite *OvnClientTestSuite) testACLFilter() { require.Equal(t, count, 4) /* include to-lport acl with external ids */ - filterFunc = aclFilter(ovnnb.ACLDirectionToLport, map[string]string{aclParentKey: pgName}) + filterFunc = aclFilter(ovnnb.ACLDirectionToLport, map[string]string{aclParentKey: pgName}, -1) count = 0 for _, acl := range acls { if filterFunc(acl) { @@ -2065,7 +2152,7 @@ func (suite *OvnClientTestSuite) testACLFilter() { require.Equal(t, count, 2) /* include from-lport acl */ - filterFunc = aclFilter(ovnnb.ACLDirectionFromLport, nil) + filterFunc = aclFilter(ovnnb.ACLDirectionFromLport, nil, -1) count = 0 for _, acl := range acls { if filterFunc(acl) { @@ -2075,7 +2162,7 @@ func (suite *OvnClientTestSuite) testACLFilter() { require.Equal(t, count, 7) /* include all from-lport acl with acl parent key*/ - filterFunc = aclFilter(ovnnb.ACLDirectionFromLport, map[string]string{aclParentKey: ""}) + filterFunc = aclFilter(ovnnb.ACLDirectionFromLport, map[string]string{aclParentKey: ""}, -1) count = 0 for _, acl := range acls { if filterFunc(acl) { @@ -2094,7 +2181,7 @@ func (suite *OvnClientTestSuite) testACLFilter() { filterFunc := aclFilter("", map[string]string{ aclParentKey: pgName, "key": "value", - }) + }, -1) require.False(t, filterFunc(acl)) }) diff --git a/pkg/ovs/ovn-nb-suite_test.go b/pkg/ovs/ovn-nb-suite_test.go index 054d39f8607..a9c03fbf79e 100644 --- a/pkg/ovs/ovn-nb-suite_test.go +++ b/pkg/ovs/ovn-nb-suite_test.go @@ -1248,7 +1248,7 @@ func Test_scratch(t *testing.T) { ovnClient, err := newOvnNbClient(t, endpoint, 10) require.NoError(t, err) - err = ovnClient.DeleteAcls("test_pg", portGroupKey, ovnnb.ACLDirectionToLport, nil) + err = ovnClient.DeleteAcls("test_pg", portGroupKey, ovnnb.ACLDirectionToLport, nil, -1) require.NoError(t, err) } diff --git a/pkg/ovs/ovn-nb.go b/pkg/ovs/ovn-nb.go index 7d66342db4a..63f55a50db0 100644 --- a/pkg/ovs/ovn-nb.go +++ b/pkg/ovs/ovn-nb.go @@ -118,7 +118,7 @@ func (c *OVNNbClient) DeleteSecurityGroup(sgName string) error { pgName := GetSgPortGroupName(sgName) // clear acl - if err := c.DeleteAcls(pgName, portGroupKey, "", nil); err != nil { + if err := c.DeleteAcls(pgName, portGroupKey, "", nil, -1); err != nil { klog.Error(err) return fmt.Errorf("delete acls from port group %s: %w", pgName, err) } diff --git a/pkg/util/const.go b/pkg/util/const.go index a54e73260e9..3f1693446ea 100644 --- a/pkg/util/const.go +++ b/pkg/util/const.go @@ -160,6 +160,7 @@ const ( AnpMaxPriority = 99 AnpACLMaxPriority = 30000 BanpACLMaxPriority = 1800 + DefaultACLTier = 0 AnpACLTier = 1 NetpolACLTier = 2 BanpACLTier = 3