Skip to content

Commit 70c50d0

Browse files
committed
networkpolicy: provider-scoped policies for multi-network pods
- add ovn.kubernetes.io/policy-for parsing and provider filtering - gate Service ClusterIP inclusion to default VPC - add unit + e2e tests (skip if NAD CRD missing) Signed-off-by: akbarkn <akbarkusumanegaralth@gmail.com>
1 parent 7dfd023 commit 70c50d0

File tree

4 files changed

+699
-5
lines changed

4 files changed

+699
-5
lines changed

pkg/controller/network_policy.go

Lines changed: 78 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,8 @@ func (c *Controller) handleUpdateNp(key string) error {
121121
}
122122
logRate := parseACLLogRate(np.Annotations)
123123

124+
providers := parsePolicyFor(np)
125+
124126
npName := np.Name
125127
nameArray := []rune(np.Name)
126128
if !unicode.IsLetter(nameArray[0]) {
@@ -142,7 +144,7 @@ func (c *Controller) handleUpdateNp(key string) error {
142144
}
143145

144146
namedPortMap := c.namedPort.GetNamedPortByNs(np.Namespace)
145-
ports, subnetNames, err := c.fetchSelectedPorts(np.Namespace, &np.Spec.PodSelector)
147+
ports, subnetNames, err := c.fetchSelectedPorts(np.Namespace, &np.Spec.PodSelector, providers)
146148
if err != nil {
147149
klog.Errorf("fetch ports belongs to np %s: %v", key, err)
148150
return err
@@ -212,7 +214,7 @@ func (c *Controller) handleUpdateNp(key string) error {
212214
} else {
213215
var allow, except []string
214216
for _, npp := range npr.From {
215-
if allow, except, err = c.fetchPolicySelectedAddresses(np.Namespace, protocol, npp); err != nil {
217+
if allow, except, err = c.fetchPolicySelectedAddresses(np.Namespace, protocol, npp, providers); err != nil {
216218
klog.Errorf("failed to fetch policy selected addresses, %v", err)
217219
return err
218220
}
@@ -359,7 +361,7 @@ func (c *Controller) handleUpdateNp(key string) error {
359361
} else {
360362
var allow, except []string
361363
for _, npp := range npr.To {
362-
if allow, except, err = c.fetchPolicySelectedAddresses(np.Namespace, protocol, npp); err != nil {
364+
if allow, except, err = c.fetchPolicySelectedAddresses(np.Namespace, protocol, npp, providers); err != nil {
363365
klog.Errorf("failed to fetch policy selected addresses, %v", err)
364366
return err
365367
}
@@ -531,7 +533,53 @@ func (c *Controller) handleDeleteNp(key string) error {
531533
return nil
532534
}
533535

534-
func (c *Controller) fetchSelectedPorts(namespace string, selector *metav1.LabelSelector) ([]string, []string, error) {
536+
func parsePolicyFor(np *netv1.NetworkPolicy) set.Set[string] {
537+
raw := strings.TrimSpace(np.Annotations[util.NetworkPolicyForAnnotation])
538+
if raw == "" {
539+
return nil
540+
}
541+
542+
providers := set.New[string]()
543+
invalidMsg := `ignore invalid network_policy_for annotation %q for netpol %s/%s, expect "ovn" or "<namespace>/<net-attach-def>"`
544+
545+
for _, token := range strings.Split(raw, ",") {
546+
t := strings.TrimSpace(token)
547+
if t == "" {
548+
continue
549+
}
550+
551+
if strings.EqualFold(t, "ovn") {
552+
providers.Insert(util.OvnProvider)
553+
continue
554+
}
555+
if strings.Contains(t, "/") {
556+
parts := strings.SplitN(t, "/", 2)
557+
if len(parts) != 2 || parts[0] == "" || parts[1] == "" {
558+
klog.Warningf(invalidMsg, t, np.Namespace, np.Name)
559+
continue
560+
}
561+
provider := fmt.Sprintf("%s.%s.%s", parts[1], parts[0], util.OvnProvider)
562+
providers.Insert(provider)
563+
continue
564+
}
565+
klog.Warningf(invalidMsg, t, np.Namespace, np.Name)
566+
}
567+
568+
if providers.Len() == 0 {
569+
klog.Warningf("network_policy_for annotation has no valid entries; policy %s/%s selects no pods", np.Namespace, np.Name)
570+
return providers
571+
}
572+
return providers
573+
}
574+
575+
func netpolAppliesToProvider(provider string, providers set.Set[string]) bool {
576+
if providers == nil {
577+
return true
578+
}
579+
return providers.Has(provider)
580+
}
581+
582+
func (c *Controller) fetchSelectedPorts(namespace string, selector *metav1.LabelSelector, providers set.Set[string]) ([]string, []string, error) {
535583
var subnets []string
536584
sel, err := metav1.LabelSelectorAsSelector(selector)
537585
if err != nil {
@@ -553,17 +601,25 @@ func (c *Controller) fetchSelectedPorts(namespace string, selector *metav1.Label
553601
return nil, nil, fmt.Errorf("failed to get pod networks, %w", err)
554602
}
555603

604+
matchedProvider := false
556605
for _, podNet := range podNets {
557606
if !isOvnSubnet(podNet.Subnet) {
558607
continue
559608
}
609+
if !netpolAppliesToProvider(podNet.ProviderName, providers) {
610+
continue
611+
}
612+
matchedProvider = true
560613

561614
if pod.Annotations[fmt.Sprintf(util.AllocatedAnnotationTemplate, podNet.ProviderName)] == "true" {
562615
ports = append(ports, ovs.PodNameToPortName(podName, pod.Namespace, podNet.ProviderName))
563616
// Pod selected by networkpolicy has its own subnet which is not the default subnet
564617
subnets = append(subnets, podNet.Subnet.Name)
565618
}
566619
}
620+
if providers != nil && !matchedProvider {
621+
klog.V(4).Infof("skip pod %s/%s: no network attachment matches network_policy_for", pod.Namespace, pod.Name)
622+
}
567623
}
568624
subnets = slices.Compact(subnets)
569625
return ports, subnets, nil
@@ -587,7 +643,7 @@ func hasEgressRule(np *netv1.NetworkPolicy) bool {
587643
return np.Spec.Egress != nil
588644
}
589645

590-
func (c *Controller) fetchPolicySelectedAddresses(namespace, protocol string, npp netv1.NetworkPolicyPeer) ([]string, []string, error) {
646+
func (c *Controller) fetchPolicySelectedAddresses(namespace, protocol string, npp netv1.NetworkPolicyPeer, providers set.Set[string]) ([]string, []string, error) {
591647
selectedAddresses := []string{}
592648
exceptAddresses := []string{}
593649

@@ -643,7 +699,14 @@ func (c *Controller) fetchPolicySelectedAddresses(namespace, protocol string, np
643699
klog.Errorf("failed to get pod nets %v", err)
644700
return nil, nil, err
645701
}
702+
matchedProvider := false
646703
for _, podNet := range podNets {
704+
provider := podNet.ProviderName
705+
if !netpolAppliesToProvider(provider, providers) {
706+
continue
707+
}
708+
matchedProvider = true
709+
647710
podIPAnnotation := pod.Annotations[fmt.Sprintf(util.IPAddressAnnotationTemplate, podNet.ProviderName)]
648711
podIPs := strings.SplitSeq(podIPAnnotation, ",")
649712
for podIP := range podIPs {
@@ -654,18 +717,28 @@ func (c *Controller) fetchPolicySelectedAddresses(namespace, protocol string, np
654717
if len(svcs) == 0 {
655718
continue
656719
}
720+
if !shouldIncludeServiceIPs(podNet) {
721+
continue
722+
}
657723

658724
svcIPs, err := svcMatchPods(svcs, pod, protocol)
659725
if err != nil {
660726
return nil, nil, err
661727
}
662728
selectedAddresses = append(selectedAddresses, svcIPs...)
663729
}
730+
if providers != nil && !matchedProvider {
731+
klog.V(4).Infof("skip pod %s/%s: no network attachment matches network_policy_for", pod.Namespace, pod.Name)
732+
}
664733
}
665734
}
666735
return selectedAddresses, exceptAddresses, nil
667736
}
668737

738+
func shouldIncludeServiceIPs(podNet *kubeovnNet) bool {
739+
return podNet != nil && podNet.Subnet != nil && podNet.Subnet.Spec.Vpc == util.DefaultVpc
740+
}
741+
669742
func svcMatchPods(svcs []*corev1.Service, pod *corev1.Pod, protocol string) ([]string, error) {
670743
matchSvcs := []string{}
671744
// find svc ip by pod's info
Lines changed: 122 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,122 @@
1+
package controller
2+
3+
import (
4+
"testing"
5+
6+
netv1 "k8s.io/api/networking/v1"
7+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
8+
9+
"k8s.io/utils/set"
10+
11+
"github.com/stretchr/testify/require"
12+
13+
"github.com/kubeovn/kube-ovn/pkg/util"
14+
)
15+
16+
func TestParsePolicyFor(t *testing.T) {
17+
t.Parallel()
18+
19+
tests := []struct {
20+
name string
21+
annotation *string
22+
wantProviders set.Set[string]
23+
}{
24+
{
25+
name: "annotation omitted",
26+
annotation: nil,
27+
wantProviders: nil,
28+
},
29+
{
30+
name: "ovn only",
31+
annotation: ptrString("ovn"),
32+
wantProviders: set.New(
33+
util.OvnProvider,
34+
),
35+
},
36+
{
37+
name: "duplicate ovn",
38+
annotation: ptrString("ovn, ovn"),
39+
wantProviders: set.New(
40+
util.OvnProvider,
41+
),
42+
},
43+
{
44+
name: "secondary only",
45+
annotation: ptrString("ns1/net1"),
46+
wantProviders: set.New(
47+
"net1.ns1." + util.OvnProvider,
48+
),
49+
},
50+
{
51+
name: "ovn and secondary",
52+
annotation: ptrString(" ovn , ns1/net1 "),
53+
wantProviders: set.New(
54+
util.OvnProvider,
55+
"net1.ns1."+util.OvnProvider,
56+
),
57+
},
58+
{
59+
name: "ovn and invalid",
60+
annotation: ptrString("ovn, foo"),
61+
wantProviders: set.New(
62+
util.OvnProvider,
63+
),
64+
},
65+
{
66+
name: "invalid all",
67+
annotation: ptrString("all"),
68+
wantProviders: set.New[string](),
69+
},
70+
{
71+
name: "invalid default",
72+
annotation: ptrString("default"),
73+
wantProviders: set.New[string](),
74+
},
75+
{
76+
name: "invalid no entries",
77+
annotation: ptrString(","),
78+
wantProviders: set.New[string](),
79+
},
80+
{
81+
name: "invalid token",
82+
annotation: ptrString("foo"),
83+
wantProviders: set.New[string](),
84+
},
85+
}
86+
87+
for _, tt := range tests {
88+
t.Run(tt.name, func(t *testing.T) {
89+
np := &netv1.NetworkPolicy{
90+
ObjectMeta: metav1.ObjectMeta{
91+
Name: "np",
92+
Namespace: "default",
93+
},
94+
}
95+
if tt.annotation != nil {
96+
np.Annotations = map[string]string{
97+
util.NetworkPolicyForAnnotation: *tt.annotation,
98+
}
99+
}
100+
101+
providers := parsePolicyFor(np)
102+
if tt.wantProviders == nil {
103+
require.Nil(t, providers)
104+
return
105+
}
106+
require.Equal(t, tt.wantProviders, providers)
107+
})
108+
}
109+
}
110+
111+
func TestNetpolAppliesToProvider(t *testing.T) {
112+
t.Parallel()
113+
providers := set.New("ovn", "net1.ns1.ovn")
114+
require.True(t, netpolAppliesToProvider("ovn", providers))
115+
require.False(t, netpolAppliesToProvider("net2.ns2.ovn", providers))
116+
require.True(t, netpolAppliesToProvider("ovn", nil))
117+
require.False(t, netpolAppliesToProvider("ovn", set.New[string]()))
118+
}
119+
120+
func ptrString(s string) *string {
121+
return &s
122+
}

pkg/util/const.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,7 @@ const (
125125
NodeNameLabel = "ovn.kubernetes.io/node-name"
126126
NetworkPolicyLogAnnotation = "ovn.kubernetes.io/enable_log"
127127
NetworkPolicyEnforcementAnnotation = "ovn.kubernetes.io/network_policy_enforcement"
128+
NetworkPolicyForAnnotation = "ovn.kubernetes.io/network_policy_for"
128129
ACLActionsLogAnnotation = "ovn.kubernetes.io/log_acl_actions"
129130
ACLLogMeterAnnotation = "ovn.kubernetes.io/acl_log_meter_rate"
130131

0 commit comments

Comments
 (0)