diff --git a/mocks/pkg/ovs/interface.go b/mocks/pkg/ovs/interface.go index f1e9c6f3ca5..7e355df1b16 100644 --- a/mocks/pkg/ovs/interface.go +++ b/mocks/pkg/ovs/interface.go @@ -2063,34 +2063,49 @@ func (mr *MockACLMockRecorder) UpdateAnpRuleACLOps(pgName, asName, protocol, acl return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpdateAnpRuleACLOps", reflect.TypeOf((*MockACL)(nil).UpdateAnpRuleACLOps), pgName, asName, protocol, aclName, priority, aclAction, logACLActions, rulePorts, isIngress, isBanp) } +// UpdateDefaultBlockACLOps mocks base method. +func (m *MockACL) UpdateDefaultBlockACLOps(netpol, pgName, direction string, loggingEnabled bool) ([]ovsdb.Operation, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "UpdateDefaultBlockACLOps", netpol, pgName, direction, loggingEnabled) + ret0, _ := ret[0].([]ovsdb.Operation) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// UpdateDefaultBlockACLOps indicates an expected call of UpdateDefaultBlockACLOps. +func (mr *MockACLMockRecorder) UpdateDefaultBlockACLOps(netpol, pgName, direction, loggingEnabled any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpdateDefaultBlockACLOps", reflect.TypeOf((*MockACL)(nil).UpdateDefaultBlockACLOps), netpol, pgName, direction, loggingEnabled) +} + // UpdateEgressACLOps mocks base method. -func (m *MockACL) UpdateEgressACLOps(netpol, pgName, asEgressName, asExceptName, protocol, aclName string, npp []v10.NetworkPolicyPort, logEnable bool, logACLActions []ovnnb.ACLAction, namedPortMap map[string]*util.NamedPortInfo) ([]ovsdb.Operation, error) { +func (m *MockACL) UpdateEgressACLOps(pgName, asEgressName, asExceptName, protocol, aclName string, npp []v10.NetworkPolicyPort, logEnable bool, logACLActions []ovnnb.ACLAction, namedPortMap map[string]*util.NamedPortInfo) ([]ovsdb.Operation, error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "UpdateEgressACLOps", netpol, pgName, asEgressName, asExceptName, protocol, aclName, npp, logEnable, logACLActions, namedPortMap) + ret := m.ctrl.Call(m, "UpdateEgressACLOps", pgName, asEgressName, asExceptName, protocol, aclName, npp, logEnable, logACLActions, namedPortMap) ret0, _ := ret[0].([]ovsdb.Operation) ret1, _ := ret[1].(error) return ret0, ret1 } // UpdateEgressACLOps indicates an expected call of UpdateEgressACLOps. -func (mr *MockACLMockRecorder) UpdateEgressACLOps(netpol, pgName, asEgressName, asExceptName, protocol, aclName, npp, logEnable, logACLActions, namedPortMap any) *gomock.Call { +func (mr *MockACLMockRecorder) UpdateEgressACLOps(pgName, asEgressName, asExceptName, protocol, aclName, npp, logEnable, logACLActions, namedPortMap any) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpdateEgressACLOps", reflect.TypeOf((*MockACL)(nil).UpdateEgressACLOps), netpol, pgName, asEgressName, asExceptName, protocol, aclName, npp, logEnable, logACLActions, namedPortMap) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpdateEgressACLOps", reflect.TypeOf((*MockACL)(nil).UpdateEgressACLOps), pgName, asEgressName, asExceptName, protocol, aclName, npp, logEnable, logACLActions, namedPortMap) } // UpdateIngressACLOps mocks base method. -func (m *MockACL) UpdateIngressACLOps(netpol, pgName, asIngressName, asExceptName, protocol, aclName string, npp []v10.NetworkPolicyPort, logEnable bool, logACLActions []ovnnb.ACLAction, namedPortMap map[string]*util.NamedPortInfo) ([]ovsdb.Operation, error) { +func (m *MockACL) UpdateIngressACLOps(pgName, asIngressName, asExceptName, protocol, aclName string, npp []v10.NetworkPolicyPort, logEnable bool, logACLActions []ovnnb.ACLAction, namedPortMap map[string]*util.NamedPortInfo) ([]ovsdb.Operation, error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "UpdateIngressACLOps", netpol, pgName, asIngressName, asExceptName, protocol, aclName, npp, logEnable, logACLActions, namedPortMap) + ret := m.ctrl.Call(m, "UpdateIngressACLOps", pgName, asIngressName, asExceptName, protocol, aclName, npp, logEnable, logACLActions, namedPortMap) ret0, _ := ret[0].([]ovsdb.Operation) ret1, _ := ret[1].(error) return ret0, ret1 } // UpdateIngressACLOps indicates an expected call of UpdateIngressACLOps. -func (mr *MockACLMockRecorder) UpdateIngressACLOps(netpol, pgName, asIngressName, asExceptName, protocol, aclName, npp, logEnable, logACLActions, namedPortMap any) *gomock.Call { +func (mr *MockACLMockRecorder) UpdateIngressACLOps(pgName, asIngressName, asExceptName, protocol, aclName, npp, logEnable, logACLActions, namedPortMap any) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpdateIngressACLOps", reflect.TypeOf((*MockACL)(nil).UpdateIngressACLOps), netpol, pgName, asIngressName, asExceptName, protocol, aclName, npp, logEnable, logACLActions, namedPortMap) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpdateIngressACLOps", reflect.TypeOf((*MockACL)(nil).UpdateIngressACLOps), pgName, asIngressName, asExceptName, protocol, aclName, npp, logEnable, logACLActions, namedPortMap) } // UpdateLogicalSwitchACL mocks base method. @@ -5221,6 +5236,21 @@ func (mr *MockNbClientMockRecorder) UpdateDHCPOptions(subnet, mtu any) *gomock.C return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpdateDHCPOptions", reflect.TypeOf((*MockNbClient)(nil).UpdateDHCPOptions), subnet, mtu) } +// UpdateDefaultBlockACLOps mocks base method. +func (m *MockNbClient) UpdateDefaultBlockACLOps(netpol, pgName, direction string, loggingEnabled bool) ([]ovsdb.Operation, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "UpdateDefaultBlockACLOps", netpol, pgName, direction, loggingEnabled) + ret0, _ := ret[0].([]ovsdb.Operation) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// UpdateDefaultBlockACLOps indicates an expected call of UpdateDefaultBlockACLOps. +func (mr *MockNbClientMockRecorder) UpdateDefaultBlockACLOps(netpol, pgName, direction, loggingEnabled any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpdateDefaultBlockACLOps", reflect.TypeOf((*MockNbClient)(nil).UpdateDefaultBlockACLOps), netpol, pgName, direction, loggingEnabled) +} + // UpdateDnatAndSnat mocks base method. func (m *MockNbClient) UpdateDnatAndSnat(lrName, externalIP, logicalIP, lspName, externalMac, gatewayType string) error { m.ctrl.T.Helper() @@ -5236,18 +5266,18 @@ func (mr *MockNbClientMockRecorder) UpdateDnatAndSnat(lrName, externalIP, logica } // UpdateEgressACLOps mocks base method. -func (m *MockNbClient) UpdateEgressACLOps(netpol, pgName, asEgressName, asExceptName, protocol, aclName string, npp []v10.NetworkPolicyPort, logEnable bool, logACLActions []ovnnb.ACLAction, namedPortMap map[string]*util.NamedPortInfo) ([]ovsdb.Operation, error) { +func (m *MockNbClient) UpdateEgressACLOps(pgName, asEgressName, asExceptName, protocol, aclName string, npp []v10.NetworkPolicyPort, logEnable bool, logACLActions []ovnnb.ACLAction, namedPortMap map[string]*util.NamedPortInfo) ([]ovsdb.Operation, error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "UpdateEgressACLOps", netpol, pgName, asEgressName, asExceptName, protocol, aclName, npp, logEnable, logACLActions, namedPortMap) + ret := m.ctrl.Call(m, "UpdateEgressACLOps", pgName, asEgressName, asExceptName, protocol, aclName, npp, logEnable, logACLActions, namedPortMap) ret0, _ := ret[0].([]ovsdb.Operation) ret1, _ := ret[1].(error) return ret0, ret1 } // UpdateEgressACLOps indicates an expected call of UpdateEgressACLOps. -func (mr *MockNbClientMockRecorder) UpdateEgressACLOps(netpol, pgName, asEgressName, asExceptName, protocol, aclName, npp, logEnable, logACLActions, namedPortMap any) *gomock.Call { +func (mr *MockNbClientMockRecorder) UpdateEgressACLOps(pgName, asEgressName, asExceptName, protocol, aclName, npp, logEnable, logACLActions, namedPortMap any) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpdateEgressACLOps", reflect.TypeOf((*MockNbClient)(nil).UpdateEgressACLOps), netpol, pgName, asEgressName, asExceptName, protocol, aclName, npp, logEnable, logACLActions, namedPortMap) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpdateEgressACLOps", reflect.TypeOf((*MockNbClient)(nil).UpdateEgressACLOps), pgName, asEgressName, asExceptName, protocol, aclName, npp, logEnable, logACLActions, namedPortMap) } // UpdateGatewayChassis mocks base method. @@ -5270,18 +5300,18 @@ func (mr *MockNbClientMockRecorder) UpdateGatewayChassis(gwChassis any, fields . } // UpdateIngressACLOps mocks base method. -func (m *MockNbClient) UpdateIngressACLOps(netpol, pgName, asIngressName, asExceptName, protocol, aclName string, npp []v10.NetworkPolicyPort, logEnable bool, logACLActions []ovnnb.ACLAction, namedPortMap map[string]*util.NamedPortInfo) ([]ovsdb.Operation, error) { +func (m *MockNbClient) UpdateIngressACLOps(pgName, asIngressName, asExceptName, protocol, aclName string, npp []v10.NetworkPolicyPort, logEnable bool, logACLActions []ovnnb.ACLAction, namedPortMap map[string]*util.NamedPortInfo) ([]ovsdb.Operation, error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "UpdateIngressACLOps", netpol, pgName, asIngressName, asExceptName, protocol, aclName, npp, logEnable, logACLActions, namedPortMap) + ret := m.ctrl.Call(m, "UpdateIngressACLOps", pgName, asIngressName, asExceptName, protocol, aclName, npp, logEnable, logACLActions, namedPortMap) ret0, _ := ret[0].([]ovsdb.Operation) ret1, _ := ret[1].(error) return ret0, ret1 } // UpdateIngressACLOps indicates an expected call of UpdateIngressACLOps. -func (mr *MockNbClientMockRecorder) UpdateIngressACLOps(netpol, pgName, asIngressName, asExceptName, protocol, aclName, npp, logEnable, logACLActions, namedPortMap any) *gomock.Call { +func (mr *MockNbClientMockRecorder) UpdateIngressACLOps(pgName, asIngressName, asExceptName, protocol, aclName, npp, logEnable, logACLActions, namedPortMap any) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpdateIngressACLOps", reflect.TypeOf((*MockNbClient)(nil).UpdateIngressACLOps), netpol, pgName, asIngressName, asExceptName, protocol, aclName, npp, logEnable, logACLActions, namedPortMap) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpdateIngressACLOps", reflect.TypeOf((*MockNbClient)(nil).UpdateIngressACLOps), pgName, asIngressName, asExceptName, protocol, aclName, npp, logEnable, logACLActions, namedPortMap) } // UpdateLogicalRouter mocks base method. diff --git a/pkg/controller/network_policy.go b/pkg/controller/network_policy.go index 359cb91284a..51804e0fe4a 100644 --- a/pkg/controller/network_policy.go +++ b/pkg/controller/network_policy.go @@ -173,6 +173,15 @@ func (c *Controller) handleUpdateNp(key string) error { } if hasIngressRule(np) { + if protocolSet.Size() > 0 { + blockACLOps, err := c.OVNNbClient.UpdateDefaultBlockACLOps(key, pgName, ovnnb.ACLDirectionToLport, logEnable) + if err != nil { + klog.Errorf("failed to set default ingress block acl: %v", err) + return fmt.Errorf("failed to set default ingress block acl: %w", err) + } + ingressACLOps = append(ingressACLOps, blockACLOps...) + } + for _, protocol := range protocolSet.List() { for idx, npr := range np.Spec.Ingress { // A single address set must contain addresses of the same type and the name must be unique within table, so IPv4 and IPv6 address set should be different @@ -214,7 +223,7 @@ func (c *Controller) handleUpdateNp(key string) error { npp = npr.Ports } - ops, err := c.OVNNbClient.UpdateIngressACLOps(key, pgName, ingressAllowAsName, ingressExceptAsName, protocol, aclName, npp, logEnable, logActions, namedPortMap) + ops, err := c.OVNNbClient.UpdateIngressACLOps(pgName, ingressAllowAsName, ingressExceptAsName, protocol, aclName, npp, logEnable, logActions, namedPortMap) if err != nil { klog.Errorf("generate operations that add ingress acls to np %s: %v", key, err) return err @@ -236,7 +245,7 @@ func (c *Controller) handleUpdateNp(key string) error { return err } - ops, err := c.OVNNbClient.UpdateIngressACLOps(key, pgName, ingressAllowAsName, ingressExceptAsName, protocol, aclName, nil, logEnable, logActions, namedPortMap) + ops, err := c.OVNNbClient.UpdateIngressACLOps(pgName, ingressAllowAsName, ingressExceptAsName, protocol, aclName, nil, logEnable, logActions, namedPortMap) if err != nil { klog.Errorf("generate operations that add ingress acls to np %s: %v", key, err) return err @@ -302,6 +311,15 @@ func (c *Controller) handleUpdateNp(key string) error { } if hasEgressRule(np) { + if protocolSet.Size() > 0 { + blockACLOps, err := c.OVNNbClient.UpdateDefaultBlockACLOps(key, pgName, ovnnb.ACLDirectionFromLport, logEnable) + if err != nil { + klog.Errorf("failed to set default egress block acl: %v", err) + return fmt.Errorf("failed to set default egress block acl: %w", err) + } + egressACLOps = append(egressACLOps, blockACLOps...) + } + for _, protocol := range protocolSet.List() { for idx, npr := range np.Spec.Egress { // A single address set must contain addresses of the same type and the name must be unique within table, so IPv4 and IPv6 address set should be different @@ -343,7 +361,7 @@ func (c *Controller) handleUpdateNp(key string) error { npp = npr.Ports } - ops, err := c.OVNNbClient.UpdateEgressACLOps(key, pgName, egressAllowAsName, egressExceptAsName, protocol, aclName, npp, logEnable, logActions, namedPortMap) + ops, err := c.OVNNbClient.UpdateEgressACLOps(pgName, egressAllowAsName, egressExceptAsName, protocol, aclName, npp, logEnable, logActions, namedPortMap) if err != nil { klog.Errorf("generate operations that add egress acls to np %s: %v", key, err) return err @@ -365,7 +383,7 @@ func (c *Controller) handleUpdateNp(key string) error { return err } - ops, err := c.OVNNbClient.UpdateEgressACLOps(key, pgName, egressAllowAsName, egressExceptAsName, protocol, aclName, nil, logEnable, logActions, namedPortMap) + ops, err := c.OVNNbClient.UpdateEgressACLOps(pgName, egressAllowAsName, egressExceptAsName, protocol, aclName, nil, logEnable, logActions, namedPortMap) if err != nil { klog.Errorf("generate operations that add egress acls to np %s: %v", key, err) return err diff --git a/pkg/ovs/interface.go b/pkg/ovs/interface.go index 2982948a3ff..250edbd6907 100644 --- a/pkg/ovs/interface.go +++ b/pkg/ovs/interface.go @@ -158,8 +158,9 @@ type PortGroup interface { } type ACL interface { - UpdateIngressACLOps(netpol, pgName, asIngressName, asExceptName, protocol, aclName string, npp []netv1.NetworkPolicyPort, logEnable bool, logACLActions []ovnnb.ACLAction, namedPortMap map[string]*util.NamedPortInfo) ([]ovsdb.Operation, error) - UpdateEgressACLOps(netpol, pgName, asEgressName, asExceptName, protocol, aclName string, npp []netv1.NetworkPolicyPort, logEnable bool, logACLActions []ovnnb.ACLAction, namedPortMap map[string]*util.NamedPortInfo) ([]ovsdb.Operation, error) + UpdateDefaultBlockACLOps(netpol, pgName, direction string, loggingEnabled bool) ([]ovsdb.Operation, error) + UpdateIngressACLOps(pgName, asIngressName, asExceptName, protocol, aclName string, npp []netv1.NetworkPolicyPort, logEnable bool, logACLActions []ovnnb.ACLAction, namedPortMap map[string]*util.NamedPortInfo) ([]ovsdb.Operation, error) + UpdateEgressACLOps(pgName, asEgressName, asExceptName, protocol, aclName string, npp []netv1.NetworkPolicyPort, logEnable bool, logACLActions []ovnnb.ACLAction, namedPortMap map[string]*util.NamedPortInfo) ([]ovsdb.Operation, error) CreateGatewayACL(lsName, pgName, gateway, u2oInterconnectionIP string) error CreateNodeACL(pgName, nodeIPStr, joinIPStr string) error CreateSgDenyAllACL(sgName string) error diff --git a/pkg/ovs/ovn-nb-acl.go b/pkg/ovs/ovn-nb-acl.go index e3ee15b2294..efcf844b432 100644 --- a/pkg/ovs/ovn-nb-acl.go +++ b/pkg/ovs/ovn-nb-acl.go @@ -53,34 +53,56 @@ func setACLName(acl *ovnnb.ACL, name string) { acl.Name = ptr.To(name) } -// UpdateIngressACLOps return operation that creates an ingress ACL -func (c *OVNNbClient) UpdateIngressACLOps(netpol, pgName, asIngressName, asExceptName, protocol, aclName string, npp []netv1.NetworkPolicyPort, logEnable bool, logACLActions []ovnnb.ACLAction, namedPortMap map[string]*util.NamedPortInfo) ([]ovsdb.Operation, error) { - acls := make([]*ovnnb.ACL, 0) +// UpdateDefaultBlockACLOps returns operations to update/create the default block ACL +func (c *OVNNbClient) UpdateDefaultBlockACLOps(netpol, pgName, direction string, loggingEnabled bool) ([]ovsdb.Operation, error) { + portDirection := "outport" + priority := util.IngressDefaultDrop - if strings.HasSuffix(asIngressName, ".0") || strings.HasSuffix(asIngressName, ".all") { - // create the default drop rule for only once - // both IPv4 and IPv6 traffic should be forbade in dual-stack situation - allIPMatch := NewAndACLMatch( - NewACLMatch("outport", "==", "@"+pgName, ""), - NewACLMatch("ip", "", "", ""), - ) - options := func(acl *ovnnb.ACL) { - setACLName(acl, netpol) - if logEnable { - acl.Log = true - acl.Severity = ptr.To(ovnnb.ACLSeverityWarning) - } + if direction == ovnnb.ACLDirectionFromLport { + portDirection = "inport" + priority = util.EgressDefaultDrop + } + + // Block everything IP related (IPv4/IPv6/ICMPv4/ICMPv6/...) + allIPMatch := NewAndACLMatch( + NewACLMatch(portDirection, "==", "@"+pgName, ""), + NewACLMatch("ip", "", "", ""), + ) + + options := func(acl *ovnnb.ACL) { + setACLName(acl, netpol) + if loggingEnabled { + acl.Log = true + acl.Severity = ptr.To(ovnnb.ACLSeverityWarning) } - defaultDropACL, err := c.newACLWithoutCheck(pgName, ovnnb.ACLDirectionToLport, util.IngressDefaultDrop, allIPMatch.String(), ovnnb.ACLActionDrop, util.NetpolACLTier, options) - if err != nil { - klog.Error(err) - return nil, fmt.Errorf("new default drop ingress acl for port group %s: %w", pgName, err) + if direction == ovnnb.ACLDirectionFromLport { + if acl.Options == nil { + acl.Options = make(map[string]string) + } + acl.Options["apply-after-lb"] = "true" } + } - acls = append(acls, defaultDropACL) + defaultDropACL, err := c.newACLWithoutCheck(pgName, direction, priority, allIPMatch.String(), ovnnb.ACLActionDrop, util.NetpolACLTier, options) + if err != nil { + klog.Error(err) + return nil, fmt.Errorf("failed to create drop acl for port group %s: %w", pgName, err) } + ops, err := c.CreateAclsOps(pgName, portGroupKey, defaultDropACL) + if err != nil { + klog.Error(err) + return nil, fmt.Errorf("failed to create default drop acl ops for port group %s: %w", pgName, err) + } + + return ops, nil +} + +// UpdateIngressACLOps return operation that creates an ingress ACL +func (c *OVNNbClient) UpdateIngressACLOps(pgName, asIngressName, asExceptName, protocol, aclName string, npp []netv1.NetworkPolicyPort, logEnable bool, logACLActions []ovnnb.ACLAction, namedPortMap map[string]*util.NamedPortInfo) ([]ovsdb.Operation, error) { + acls := make([]*ovnnb.ACL, 0) + /* allow acl */ matches := newNetworkPolicyACLMatch(pgName, asIngressName, asExceptName, protocol, ovnnb.ACLDirectionToLport, npp, namedPortMap) for _, m := range matches { @@ -110,38 +132,9 @@ func (c *OVNNbClient) UpdateIngressACLOps(netpol, pgName, asIngressName, asExcep } // UpdateEgressACLOps return operation that creates an egress ACL -func (c *OVNNbClient) UpdateEgressACLOps(netpol, pgName, asEgressName, asExceptName, protocol, aclName string, npp []netv1.NetworkPolicyPort, logEnable bool, logACLActions []ovnnb.ACLAction, namedPortMap map[string]*util.NamedPortInfo) ([]ovsdb.Operation, error) { +func (c *OVNNbClient) UpdateEgressACLOps(pgName, asEgressName, asExceptName, protocol, aclName string, npp []netv1.NetworkPolicyPort, logEnable bool, logACLActions []ovnnb.ACLAction, namedPortMap map[string]*util.NamedPortInfo) ([]ovsdb.Operation, error) { acls := make([]*ovnnb.ACL, 0) - if strings.HasSuffix(asEgressName, ".0") || strings.HasSuffix(asEgressName, ".all") { - // create the default drop rule for only once - // both IPv4 and IPv6 traffic should be forbade in dual-stack situation - allIPMatch := NewAndACLMatch( - NewACLMatch("inport", "==", "@"+pgName, ""), - NewACLMatch("ip", "", "", ""), - ) - options := func(acl *ovnnb.ACL) { - setACLName(acl, netpol) - if logEnable { - acl.Log = true - acl.Severity = ptr.To(ovnnb.ACLSeverityWarning) - } - - if acl.Options == nil { - acl.Options = make(map[string]string) - } - acl.Options["apply-after-lb"] = "true" - } - - defaultDropACL, err := c.newACLWithoutCheck(pgName, ovnnb.ACLDirectionFromLport, util.EgressDefaultDrop, allIPMatch.String(), ovnnb.ACLActionDrop, util.NetpolACLTier, options) - if err != nil { - klog.Error(err) - return nil, fmt.Errorf("new default drop egress acl for port group %s: %w", pgName, err) - } - - acls = append(acls, defaultDropACL) - } - /* allow acl */ matches := newNetworkPolicyACLMatch(pgName, asEgressName, asExceptName, protocol, ovnnb.ACLDirectionFromLport, npp, namedPortMap) for _, m := range matches { @@ -356,7 +349,7 @@ func (c *OVNNbClient) CreateSgBaseACL(sgName, direction string) error { acl, err := c.newACL(pgName, direction, util.SecurityGroupBasePriority, match, ovnnb.ACLActionAllowRelated, util.NetpolACLTier) if err != nil { klog.Error(err) - klog.Errorf("new base ingress acl for security group %s: %v", sgName, err) + klog.Errorf("failed to create new base ingress acl for security group %s: %v", sgName, err) return } acls = append(acls, acl) diff --git a/pkg/ovs/ovn-nb-acl_test.go b/pkg/ovs/ovn-nb-acl_test.go index 02aea5b8fd4..7ba65019a84 100644 --- a/pkg/ovs/ovn-nb-acl_test.go +++ b/pkg/ovs/ovn-nb-acl_test.go @@ -65,6 +65,54 @@ func newACL(parentName, direction, priority, match, action string, tier int, opt return acl } +func (suite *OvnClientTestSuite) testUpdateDefaultBlockACLOps() { + t := suite.T() + t.Parallel() + + nbClient := suite.ovnNBClient + + expect := func(row ovsdb.Row, action, direction, match, priority string) { + intPriority, err := strconv.Atoi(priority) + require.NoError(t, err) + require.Equal(t, action, row["action"]) + require.Equal(t, direction, row["direction"]) + require.Equal(t, match, row["match"]) + require.Equal(t, intPriority, row["priority"]) + } + + t.Run("default block ingress", func(t *testing.T) { + t.Parallel() + + netpol := "default block ingress" + pgName := "test_create_block_ingress_acl_pg" + + err := nbClient.CreatePortGroup(pgName, nil) + require.NoError(t, err) + + ops, err := nbClient.UpdateDefaultBlockACLOps(netpol, pgName, ovnnb.ACLDirectionToLport, true) + require.NoError(t, err) + require.Len(t, ops, 2) + + expect(ops[0].Row, "drop", ovnnb.ACLDirectionToLport, fmt.Sprintf("outport == @%s && ip", pgName), util.IngressDefaultDrop) + }) + + t.Run("default block egress", func(t *testing.T) { + t.Parallel() + + netpol := "default block egress" + pgName := "test_create_block_egress_acl_pg" + + err := nbClient.CreatePortGroup(pgName, nil) + require.NoError(t, err) + + ops, err := nbClient.UpdateDefaultBlockACLOps(netpol, pgName, ovnnb.ACLDirectionFromLport, true) + require.NoError(t, err) + require.Len(t, ops, 2) + + expect(ops[0].Row, "drop", ovnnb.ACLDirectionFromLport, fmt.Sprintf("inport == @%s && ip", pgName), util.EgressDefaultDrop) + }) +} + func (suite *OvnClientTestSuite) testUpdateIngressACLOps() { t := suite.T() t.Parallel() @@ -83,7 +131,6 @@ func (suite *OvnClientTestSuite) testUpdateIngressACLOps() { t.Run("ipv4 acl", func(t *testing.T) { t.Parallel() - netpol := "ipv4 ingress" pgName := "test_create_v4_ingress_acl_pg" asIngressName := "test.default.ingress.allow.ipv4.all" asExceptName := "test.default.ingress.except.ipv4.all" @@ -95,14 +142,12 @@ func (suite *OvnClientTestSuite) testUpdateIngressACLOps() { npp := mockNetworkPolicyPort() - ops, err := nbClient.UpdateIngressACLOps(netpol, pgName, asIngressName, asExceptName, protocol, aclName, npp, true, nil, nil) + ops, err := nbClient.UpdateIngressACLOps(pgName, asIngressName, asExceptName, protocol, aclName, npp, true, nil, nil) require.NoError(t, err) - require.Len(t, ops, 4) - - expect(ops[0].Row, "drop", ovnnb.ACLDirectionToLport, fmt.Sprintf("outport == @%s && ip", pgName), util.IngressDefaultDrop) + require.Len(t, ops, 3) matches := newNetworkPolicyACLMatch(pgName, asIngressName, asExceptName, protocol, ovnnb.ACLDirectionToLport, npp, nil) - i := 1 + i := 0 for _, m := range matches { require.Equal(t, m, ops[i].Row["match"]) expect(ops[i].Row, ovnnb.ACLActionAllowRelated, ovnnb.ACLDirectionToLport, m, util.IngressAllowPriority) @@ -113,7 +158,6 @@ func (suite *OvnClientTestSuite) testUpdateIngressACLOps() { t.Run("ipv6 acl", func(t *testing.T) { t.Parallel() - netpol := "ipv6 ingress" pgName := "test_create_v6_ingress_acl_pg" asIngressName := "test.default.ingress.allow.ipv6.all" asExceptName := "test.default.ingress.except.ipv6.all" @@ -123,14 +167,12 @@ func (suite *OvnClientTestSuite) testUpdateIngressACLOps() { err := nbClient.CreatePortGroup(pgName, nil) require.NoError(t, err) - ops, err := nbClient.UpdateIngressACLOps(netpol, pgName, asIngressName, asExceptName, protocol, aclName, nil, true, nil, nil) + ops, err := nbClient.UpdateIngressACLOps(pgName, asIngressName, asExceptName, protocol, aclName, nil, true, nil, nil) require.NoError(t, err) - require.Len(t, ops, 3) - - expect(ops[0].Row, "drop", ovnnb.ACLDirectionToLport, fmt.Sprintf("outport == @%s && ip", pgName), util.IngressDefaultDrop) + require.Len(t, ops, 2) matches := newNetworkPolicyACLMatch(pgName, asIngressName, asExceptName, protocol, ovnnb.ACLDirectionToLport, nil, nil) - i := 1 + i := 0 for _, m := range matches { require.Equal(t, m, ops[i].Row["match"]) expect(ops[i].Row, ovnnb.ACLActionAllowRelated, ovnnb.ACLDirectionToLport, m, util.IngressAllowPriority) @@ -141,28 +183,26 @@ func (suite *OvnClientTestSuite) testUpdateIngressACLOps() { t.Run("test empty pgName", func(t *testing.T) { t.Parallel() - netpol := "ingress with empty pg name" pgName := "" asIngressName := "test.default.ingress.allow.ipv4.all" asExceptName := "test.default.ingress.except.ipv4.all" protocol := kubeovnv1.ProtocolIPv4 aclName := "test_create_v4_ingress_acl_pg" - _, err := nbClient.UpdateIngressACLOps(netpol, pgName, asIngressName, asExceptName, protocol, aclName, nil, true, nil, nil) + _, err := nbClient.UpdateIngressACLOps(pgName, asIngressName, asExceptName, protocol, aclName, nil, true, nil, nil) require.ErrorContains(t, err, "the port group name or logical switch name is required") }) t.Run("test empty pgName without suffix", func(t *testing.T) { t.Parallel() - netpol := "ingress with empty pg name and no suffix" pgName := "" asIngressName := "test.default.ingress.allow.ipv4" asExceptName := "test.default.ingress.except.ipv4" protocol := kubeovnv1.ProtocolIPv4 aclName := "test_create_v4_ingress_acl_pg" - _, err := nbClient.UpdateIngressACLOps(netpol, pgName, asIngressName, asExceptName, protocol, aclName, nil, true, nil, nil) + _, err := nbClient.UpdateIngressACLOps(pgName, asIngressName, asExceptName, protocol, aclName, nil, true, nil, nil) require.ErrorContains(t, err, "the port group name or logical switch name is required") }) } @@ -185,7 +225,6 @@ func (suite *OvnClientTestSuite) testUpdateEgressACLOps() { t.Run("ipv4 acl", func(t *testing.T) { t.Parallel() - netpol := "ipv4 egress" pgName := "test_create_v4_egress_acl_pg" asEgressName := "test.default.egress.allow.ipv4.all" asExceptName := "test.default.egress.except.ipv4.all" @@ -197,14 +236,12 @@ func (suite *OvnClientTestSuite) testUpdateEgressACLOps() { npp := mockNetworkPolicyPort() - ops, err := nbClient.UpdateEgressACLOps(netpol, pgName, asEgressName, asExceptName, protocol, aclName, npp, true, nil, nil) + ops, err := nbClient.UpdateEgressACLOps(pgName, asEgressName, asExceptName, protocol, aclName, npp, true, nil, nil) require.NoError(t, err) - require.Len(t, ops, 4) - - expect(ops[0].Row, "drop", ovnnb.ACLDirectionFromLport, fmt.Sprintf("inport == @%s && ip", pgName), util.EgressDefaultDrop) + require.Len(t, ops, 3) matches := newNetworkPolicyACLMatch(pgName, asEgressName, asExceptName, protocol, ovnnb.ACLDirectionFromLport, npp, nil) - i := 1 + i := 0 for _, m := range matches { require.Equal(t, m, ops[i].Row["match"]) expect(ops[i].Row, ovnnb.ACLActionAllowRelated, ovnnb.ACLDirectionFromLport, m, util.EgressAllowPriority) @@ -215,7 +252,6 @@ func (suite *OvnClientTestSuite) testUpdateEgressACLOps() { t.Run("ipv6 acl", func(t *testing.T) { t.Parallel() - netpol := "ipv6 egress" pgName := "test_create_v6_egress_acl_pg" asEgressName := "test.default.egress.allow.ipv6.all" asExceptName := "test.default.egress.except.ipv6.all" @@ -225,14 +261,12 @@ func (suite *OvnClientTestSuite) testUpdateEgressACLOps() { err := nbClient.CreatePortGroup(pgName, nil) require.NoError(t, err) - ops, err := nbClient.UpdateEgressACLOps(netpol, pgName, asEgressName, asExceptName, protocol, aclName, nil, true, nil, nil) + ops, err := nbClient.UpdateEgressACLOps(pgName, asEgressName, asExceptName, protocol, aclName, nil, true, nil, nil) require.NoError(t, err) - require.Len(t, ops, 3) - - expect(ops[0].Row, "drop", ovnnb.ACLDirectionFromLport, fmt.Sprintf("inport == @%s && ip", pgName), util.EgressDefaultDrop) + require.Len(t, ops, 2) matches := newNetworkPolicyACLMatch(pgName, asEgressName, asExceptName, protocol, ovnnb.ACLDirectionFromLport, nil, nil) - i := 1 + i := 0 for _, m := range matches { require.Equal(t, m, ops[i].Row["match"]) expect(ops[i].Row, ovnnb.ACLActionAllowRelated, ovnnb.ACLDirectionFromLport, m, util.EgressAllowPriority) @@ -243,28 +277,26 @@ func (suite *OvnClientTestSuite) testUpdateEgressACLOps() { t.Run("test empty pgName", func(t *testing.T) { t.Parallel() - netpol := "egress with empty pg name" pgName := "" asEgressName := "test.default.egress.allow.ipv4.all" asExceptName := "test.default.egress.except.ipv4.all" protocol := kubeovnv1.ProtocolIPv4 aclName := "test_create_v4_egress_acl_pg" - _, err := nbClient.UpdateEgressACLOps(netpol, pgName, asEgressName, asExceptName, protocol, aclName, nil, true, nil, nil) + _, err := nbClient.UpdateEgressACLOps(pgName, asEgressName, asExceptName, protocol, aclName, nil, true, nil, nil) require.ErrorContains(t, err, "the port group name or logical switch name is required") }) t.Run("test empty pgName without suffix", func(t *testing.T) { t.Parallel() - netpol := "egress with empty pg name and no suffix" pgName := "" asEgressName := "test.default.egress.allow.ipv4" asExceptName := "test.default.egress.except.ipv4" protocol := kubeovnv1.ProtocolIPv4 aclName := "test_create_v4_egress_acl_pg" - _, err := nbClient.UpdateEgressACLOps(netpol, pgName, asEgressName, asExceptName, protocol, aclName, nil, true, nil, nil) + _, err := nbClient.UpdateEgressACLOps(pgName, asEgressName, asExceptName, protocol, aclName, nil, true, nil, nil) require.ErrorContains(t, err, "the port group name or logical switch name is required") }) } diff --git a/pkg/ovs/ovn-nb-suite_test.go b/pkg/ovs/ovn-nb-suite_test.go index cd6834e365e..635b78b4333 100644 --- a/pkg/ovs/ovn-nb-suite_test.go +++ b/pkg/ovs/ovn-nb-suite_test.go @@ -703,6 +703,10 @@ func (suite *OvnClientTestSuite) Test_BatchDeleteAddressSetByNames() { } /* acl unit test */ +func (suite *OvnClientTestSuite) Test_testUpdateDefaultBlockAclOps() { + suite.testUpdateDefaultBlockACLOps() +} + func (suite *OvnClientTestSuite) Test_testUpdateIngressAclOps() { suite.testUpdateIngressACLOps() } diff --git a/test/e2e/kube-ovn/network-policy/network-policy.go b/test/e2e/kube-ovn/network-policy/network-policy.go index ec1d730969e..8cb3a0b74fc 100644 --- a/test/e2e/kube-ovn/network-policy/network-policy.go +++ b/test/e2e/kube-ovn/network-policy/network-policy.go @@ -177,7 +177,7 @@ var _ = framework.SerialDescribe("[group:network-policy]", func() { cmd := "curl -k -q -s --connect-timeout 2 https://" + net.JoinHostPort(clusterIP, "443") ginkgo.By(fmt.Sprintf(`Executing %q in pod %s/%s`, cmd, pod.Namespace, pod.Name)) - framework.WaitUntil(2*time.Second, time.Minute, func(_ context.Context) (bool, error) { + framework.WaitUntil(2*time.Second, 3*time.Minute, func(_ context.Context) (bool, error) { _, err := e2epodoutput.RunHostCmd(pod.Namespace, pod.Name, cmd) return err == nil, nil }, "")