diff --git a/pkg/controller/admin_network_policy.go b/pkg/controller/admin_network_policy.go index aaa9814f068..f9cc7fec543 100644 --- a/pkg/controller/admin_network_policy.go +++ b/pkg/controller/admin_network_policy.go @@ -232,7 +232,7 @@ func (c *Controller) handleAddAnp(key string) (err error) { } aclPriority := util.AnpACLMaxPriority - int(anp.Spec.Priority*100) - index - aclAction := convertAction(anpr.Action, "") + aclAction := anpACLAction(anpr.Action) rulePorts := []v1alpha1.AdminNetworkPolicyPort{} if anpr.Ports != nil { rulePorts = *anpr.Ports @@ -299,7 +299,7 @@ func (c *Controller) handleAddAnp(key string) (err error) { } aclPriority := util.AnpACLMaxPriority - int(anp.Spec.Priority*100) - index - aclAction := convertAction(anpr.Action, "") + aclAction := anpACLAction(anpr.Action) rulePorts := []v1alpha1.AdminNetworkPolicyPort{} if anpr.Ports != nil { rulePorts = *anpr.Ports @@ -947,35 +947,26 @@ func getAnpAddressSetName(pgName, ruleName string, index int, isIngress bool) (s return asV4Name, asV6Name } -func convertAction(anpRuleAction v1alpha1.AdminNetworkPolicyRuleAction, banpRuleAction v1alpha1.BaselineAdminNetworkPolicyRuleAction) (aclAction ovnnb.ACLAction) { - switch anpRuleAction { +func anpACLAction(action v1alpha1.AdminNetworkPolicyRuleAction) ovnnb.ACLAction { + switch action { case v1alpha1.AdminNetworkPolicyRuleActionAllow: - aclAction = ovnnb.ACLActionAllowRelated + return ovnnb.ACLActionAllowRelated case v1alpha1.AdminNetworkPolicyRuleActionDeny: - aclAction = ovnnb.ACLActionDrop + return ovnnb.ACLActionDrop case v1alpha1.AdminNetworkPolicyRuleActionPass: - aclAction = ovnnb.ACLActionPass + return ovnnb.ACLActionPass } - - switch banpRuleAction { - case v1alpha1.BaselineAdminNetworkPolicyRuleActionAllow: - aclAction = ovnnb.ACLActionAllowRelated - case v1alpha1.BaselineAdminNetworkPolicyRuleActionDeny: - aclAction = ovnnb.ACLActionDrop - } - return + return ovnnb.ACLActionDrop } func isRulesArrayEmpty(ruleNames [util.AnpMaxRules]ChangedName) bool { - isEmpty := true for _, ruleName := range ruleNames { // The ruleName can be omitted default if ruleName.curRuleName != "" || ruleName.isMatch { - isEmpty = false - break + return false } } - return isEmpty + return true } func (c *Controller) fetchNodesAddrs(nodeSelector labels.Selector) ([]string, []string, error) { @@ -1002,8 +993,7 @@ func (c *Controller) fetchNodesAddrs(nodeSelector labels.Selector) ([]string, [] } func fetchCIDRAddrs(networks []v1alpha1.CIDR) ([]string, []string) { - v4Addresses := make([]string, 0, len(networks)) - v6Addresses := make([]string, 0, len(networks)) + var v4Addresses, v6Addresses []string for _, network := range networks { if _, _, err := net.ParseCIDR(string(network)); err != nil { diff --git a/pkg/controller/admin_network_policy_test.go b/pkg/controller/admin_network_policy_test.go new file mode 100644 index 00000000000..bc8c3646db7 --- /dev/null +++ b/pkg/controller/admin_network_policy_test.go @@ -0,0 +1,304 @@ +package controller + +import ( + "testing" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + v1alpha1 "sigs.k8s.io/network-policy-api/apis/v1alpha1" + + "github.com/stretchr/testify/require" + + "github.com/kubeovn/kube-ovn/pkg/ovsdb/ovnnb" + "github.com/kubeovn/kube-ovn/pkg/util" +) + +func TestValidateAnpConfig(t *testing.T) { + t.Parallel() + + fakeController := newFakeController(t) + ctrl := fakeController.fakeController + + anp := &v1alpha1.AdminNetworkPolicy{ + ObjectMeta: metav1.ObjectMeta{ + Name: "anp-prio", + }, + Spec: v1alpha1.AdminNetworkPolicySpec{ + Priority: util.AnpMaxPriority, + Subject: v1alpha1.AdminNetworkPolicySubject{Namespaces: &metav1.LabelSelector{}}, + }, + } + + t.Run("normal", func(t *testing.T) { + err := ctrl.validateAnpConfig(anp) + require.NoError(t, err) + }) + + t.Run("conflict priority", func(t *testing.T) { + ctrl.anpPrioNameMap = map[int32]string{anp.Spec.Priority: anp.Name + "-conflict"} + err := ctrl.validateAnpConfig(anp) + require.ErrorContains(t, err, "can not create anp with same priority") + }) + + t.Run("priority out of range", func(t *testing.T) { + anp.Spec.Priority = util.AnpMaxPriority + 1 + err := ctrl.validateAnpConfig(anp) + require.ErrorContains(t, err, "is greater than max value") + }) +} + +func TestFetchCIDRAddrs(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + networks []v1alpha1.CIDR + v4Addrs []string + v6Addrs []string + }{ + { + name: "nil", + }, + { + name: "empty", + networks: []v1alpha1.CIDR{}, + }, + { + name: "ipv4 only", + networks: []v1alpha1.CIDR{"1.1.1.0/24"}, + v4Addrs: []string{"1.1.1.0/24"}, + }, + { + name: "ipv6 only", + networks: []v1alpha1.CIDR{"fd00::/64"}, + v6Addrs: []string{"fd00::/64"}, + }, + { + name: "mixed", + networks: []v1alpha1.CIDR{"1.1.1.0/24", "fd00::/64"}, + v4Addrs: []string{"1.1.1.0/24"}, + v6Addrs: []string{"fd00::/64"}, + }, + { + name: "invalid ipv4 cidr", + networks: []v1alpha1.CIDR{"1.1.1.0/33", "fd00::/64"}, + v6Addrs: []string{"fd00::/64"}, + }, + { + name: "invalid ipv6 cidr", + networks: []v1alpha1.CIDR{"1.1.1.0/24", "fd00::/129"}, + v4Addrs: []string{"1.1.1.0/24"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + v4Addrs, v6Addrs := fetchCIDRAddrs(tt.networks) + require.Equal(t, tt.v4Addrs, v4Addrs) + require.Equal(t, tt.v6Addrs, v6Addrs) + }) + } +} + +func TestIsRulesArrayEmpty(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + arg [util.AnpMaxRules]ChangedName + ret bool + }{ + { + "empty", + [util.AnpMaxRules]ChangedName{}, + true, + }, + { + "rule name", + [util.AnpMaxRules]ChangedName{{curRuleName: "foo"}}, + false, + }, + { + "match", + [util.AnpMaxRules]ChangedName{{isMatch: true}}, + false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ret := isRulesArrayEmpty(tt.arg) + require.Equal(t, tt.ret, ret) + }) + } +} + +func TestGetAnpName(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + arg string + ret string + }{ + { + "normal", + "foo", + "foo", + }, + { + "start with digital", + "123", + "anp123", + }, + { + "start with hyphen", + "-foo", + "anp-foo", + }, + { + "start with underscore", + "_foo", + "anp_foo", + }, + { + "start with dot", + ".foo", + "anp.foo", + }, + { + "start with slash", + "/foo", + "anp/foo", + }, + { + "start with colon", + ":foo", + "anp:foo", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ret := getAnpName(tt.arg) + require.Equal(t, tt.ret, ret) + }) + } +} + +func TestGetAnpAddressSetName(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + pgName string + ruleName string + index int + isIngress bool + v4Name string + v6Name string + }{ + { + "ingress", + "foo", + "bar", + 1, + true, + "foo.ingress.1.bar.IPv4", + "foo.ingress.1.bar.IPv6", + }, + { + "egress", + "bar", + "foo", + 0, + false, + "bar.egress.0.foo.IPv4", + "bar.egress.0.foo.IPv6", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + v4Name, v6Name := getAnpAddressSetName(tt.pgName, tt.ruleName, tt.index, tt.isIngress) + require.Equal(t, tt.v4Name, v4Name) + require.Equal(t, tt.v6Name, v6Name) + }) + } +} + +func TestIsLabelsMatch(t *testing.T) { + t.Parallel() + + nsLabelMap := make(map[string]string, 1) + nsLabelMap["nsName"] = "test-ns" + nsCmpLabelMap := make(map[string]string, 1) + nsCmpLabelMap["nsName"] = "test-ns-cmp" + nsLabels := metav1.LabelSelector{MatchLabels: nsLabelMap} + + t.Run("check namespace label match", func(t *testing.T) { + isMatch := isLabelsMatch(&nsLabels, nil, nsLabelMap, nil) + require.True(t, isMatch) + + isMatch = isLabelsMatch(&nsLabels, nil, nsCmpLabelMap, nil) + require.False(t, isMatch) + }) + + podLabelMap := make(map[string]string, 1) + podLabelMap["podName"] = "test-pod" + podCmpLabelMap := make(map[string]string, 1) + podCmpLabelMap["podName"] = "test-pod-cmp" + podLabels := metav1.LabelSelector{MatchLabels: podLabelMap} + nsPod := v1alpha1.NamespacedPod{NamespaceSelector: nsLabels, PodSelector: podLabels} + + t.Run("check pod label match", func(t *testing.T) { + isMatch := isLabelsMatch(nil, &nsPod, nsLabelMap, podLabelMap) + require.True(t, isMatch) + + isMatch = isLabelsMatch(nil, &nsPod, nsCmpLabelMap, podLabelMap) + require.False(t, isMatch) + + isMatch = isLabelsMatch(nil, &nsPod, nsLabelMap, podCmpLabelMap) + require.False(t, isMatch) + + isMatch = isLabelsMatch(nil, &nsPod, nsCmpLabelMap, podCmpLabelMap) + require.False(t, isMatch) + }) +} + +func TestAnpACLAction(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + arg v1alpha1.AdminNetworkPolicyRuleAction + ret ovnnb.ACLAction + }{ + { + "allow", + v1alpha1.AdminNetworkPolicyRuleActionAllow, + ovnnb.ACLActionAllowRelated, + }, + { + "pass", + v1alpha1.AdminNetworkPolicyRuleActionPass, + ovnnb.ACLActionPass, + }, + { + "deny", + v1alpha1.AdminNetworkPolicyRuleActionDeny, + ovnnb.ACLActionDrop, + }, + { + "unknown", + "foo", + ovnnb.ACLActionDrop, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ret := anpACLAction(tt.arg) + require.Equal(t, tt.ret, ret) + }) + } +} diff --git a/pkg/controller/baseline_admin_network_policy.go b/pkg/controller/baseline_admin_network_policy.go index 819d7a18872..0ed57182fae 100644 --- a/pkg/controller/baseline_admin_network_policy.go +++ b/pkg/controller/baseline_admin_network_policy.go @@ -13,6 +13,7 @@ import ( v1alpha1 "sigs.k8s.io/network-policy-api/apis/v1alpha1" kubeovnv1 "github.com/kubeovn/kube-ovn/pkg/apis/kubeovn/v1" + "github.com/kubeovn/kube-ovn/pkg/ovsdb/ovnnb" "github.com/kubeovn/kube-ovn/pkg/util" ) @@ -108,6 +109,16 @@ func (c *Controller) enqueueUpdateBanp(oldObj, newObj interface{}) { } } +func banpACLAction(action v1alpha1.BaselineAdminNetworkPolicyRuleAction) ovnnb.ACLAction { + switch action { + case v1alpha1.BaselineAdminNetworkPolicyRuleActionAllow: + return ovnnb.ACLActionAllowRelated + case v1alpha1.BaselineAdminNetworkPolicyRuleActionDeny: + return ovnnb.ACLActionDrop + } + return ovnnb.ACLActionDrop +} + func (c *Controller) handleAddBanp(key string) (err error) { // Only one banp with default name can be created in cluster, no need to check c.banpKeyMutex.LockKey(key) @@ -191,7 +202,7 @@ func (c *Controller) handleAddBanp(key string) (err error) { // use 1700-1800 for banp acl priority aclPriority := util.BanpACLMaxPriority - index - aclAction := convertAction("", banpr.Action) + aclAction := banpACLAction(banpr.Action) rulePorts := []v1alpha1.AdminNetworkPolicyPort{} if banpr.Ports != nil { rulePorts = *banpr.Ports @@ -258,7 +269,7 @@ func (c *Controller) handleAddBanp(key string) (err error) { } aclPriority := util.BanpACLMaxPriority - index - aclAction := convertAction("", banpr.Action) + aclAction := banpACLAction(banpr.Action) rulePorts := []v1alpha1.AdminNetworkPolicyPort{} if banpr.Ports != nil { rulePorts = *banpr.Ports diff --git a/pkg/controller/baseline_admin_network_policy_test.go b/pkg/controller/baseline_admin_network_policy_test.go new file mode 100644 index 00000000000..4ce2de21f22 --- /dev/null +++ b/pkg/controller/baseline_admin_network_policy_test.go @@ -0,0 +1,44 @@ +package controller + +import ( + "testing" + + v1alpha1 "sigs.k8s.io/network-policy-api/apis/v1alpha1" + + "github.com/stretchr/testify/require" + + "github.com/kubeovn/kube-ovn/pkg/ovsdb/ovnnb" +) + +func TestBanpACLAction(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + arg v1alpha1.BaselineAdminNetworkPolicyRuleAction + ret ovnnb.ACLAction + }{ + { + "allow", + v1alpha1.BaselineAdminNetworkPolicyRuleActionAllow, + ovnnb.ACLActionAllowRelated, + }, + { + "deny", + v1alpha1.BaselineAdminNetworkPolicyRuleActionDeny, + ovnnb.ACLActionDrop, + }, + { + "unknown", + "foo", + ovnnb.ACLActionDrop, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ret := banpACLAction(tt.arg) + require.Equal(t, tt.ret, ret) + }) + } +}