Skip to content

Commit a36265f

Browse files
akbarknakbarkn
committed
networkpolicy: provider-scoped policies for multi-network pods
- update provider‑scope logic (annotation parsing, default‑VPC ClusterIP gating, log clarifications). - update unit tests for new behavior and new helper coverage. - add NetworkPolicyForAnnotation. Signed-off-by: akbarkn <akbarkusumanegaralth@gmail.com>
1 parent 9bcdb92 commit a36265f

File tree

4 files changed

+270
-112
lines changed

4 files changed

+270
-112
lines changed

‎pkg/controller/network_policy.go‎

Lines changed: 48 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
package controller
22

33
import (
4-
"errors"
54
"fmt"
65
"reflect"
76
"slices"
@@ -29,7 +28,6 @@ import (
2928
const (
3029
NetworkPolicyEnforcementStandard = "standard"
3130
NetworkPolicyEnforcementLax = "lax"
32-
policyForAnnotation = "ovn.kubernetes.io/policy-for"
3331
)
3432

3533
func (c *Controller) enqueueAddNp(obj any) {
@@ -123,7 +121,7 @@ func (c *Controller) handleUpdateNp(key string) error {
123121
}
124122
logRate := parseACLLogRate(np.Annotations)
125123

126-
providers, includeSvc, err := parsePolicyFor(np)
124+
providers, err := parsePolicyFor(np)
127125
if err != nil {
128126
return err
129127
}
@@ -219,7 +217,7 @@ func (c *Controller) handleUpdateNp(key string) error {
219217
} else {
220218
var allow, except []string
221219
for _, npp := range npr.From {
222-
if allow, except, err = c.fetchPolicySelectedAddresses(np.Namespace, protocol, npp, providers, includeSvc); err != nil {
220+
if allow, except, err = c.fetchPolicySelectedAddresses(np.Namespace, protocol, npp, providers); err != nil {
223221
klog.Errorf("failed to fetch policy selected addresses, %v", err)
224222
return err
225223
}
@@ -366,7 +364,7 @@ func (c *Controller) handleUpdateNp(key string) error {
366364
} else {
367365
var allow, except []string
368366
for _, npp := range npr.To {
369-
if allow, except, err = c.fetchPolicySelectedAddresses(np.Namespace, protocol, npp, providers, includeSvc); err != nil {
367+
if allow, except, err = c.fetchPolicySelectedAddresses(np.Namespace, protocol, npp, providers); err != nil {
370368
klog.Errorf("failed to fetch policy selected addresses, %v", err)
371369
return err
372370
}
@@ -538,50 +536,53 @@ func (c *Controller) handleDeleteNp(key string) error {
538536
return nil
539537
}
540538

541-
func parsePolicyFor(np *netv1.NetworkPolicy) (map[string]struct{}, bool, error) {
542-
raw := strings.TrimSpace(np.Annotations[policyForAnnotation])
539+
func parsePolicyFor(np *netv1.NetworkPolicy) (set.Set[string], error) {
540+
raw := strings.TrimSpace(np.Annotations[util.NetworkPolicyForAnnotation])
543541
if raw == "" {
544-
return nil, true, nil
542+
return nil, nil
545543
}
546544

547-
providers := map[string]struct{}{}
548-
includeSvc := false
545+
providers := set.New[string]()
546+
invalidMsg := `ignore invalid network_policy_for entry %q, expect "ovn" or "<namespace>/<net-attach-def>"`
549547

550548
for _, token := range strings.Split(raw, ",") {
551549
t := strings.TrimSpace(token)
552550
if t == "" {
553551
continue
554552
}
555553

556-
switch strings.ToLower(t) {
557-
case "primary":
558-
providers[util.OvnProvider] = struct{}{}
559-
includeSvc = true
554+
if strings.EqualFold(t, "ovn") {
555+
providers.Insert(util.OvnProvider)
560556
continue
561-
case "default":
562-
return nil, false, fmt.Errorf("invalid policy-for entry %q (use 'primary')", t)
563-
case "all":
564-
return nil, false, fmt.Errorf("invalid policy-for entry %q (omit annotation for all)", t)
565557
}
566558
if strings.Contains(t, "/") {
567559
parts := strings.SplitN(t, "/", 2)
568560
if len(parts) != 2 || parts[0] == "" || parts[1] == "" {
569-
return nil, false, fmt.Errorf("invalid policy-for entry %q", t)
561+
klog.Warningf(invalidMsg, t)
562+
continue
570563
}
571564
provider := fmt.Sprintf("%s.%s.%s", parts[1], parts[0], util.OvnProvider)
572-
providers[provider] = struct{}{}
565+
providers.Insert(provider)
573566
continue
574567
}
575-
return nil, false, fmt.Errorf("invalid policy-for entry %q", t)
568+
klog.Warningf(invalidMsg, t)
576569
}
577570

578571
if len(providers) == 0 {
579-
return nil, false, errors.New("policy-for annotation has no valid entries")
572+
klog.Warning("network_policy_for annotation has no valid entries; policy selects no pods")
573+
return providers, nil
574+
}
575+
return providers, nil
576+
}
577+
578+
func netpolAppliesToProvider(provider string, providers set.Set[string]) bool {
579+
if providers == nil {
580+
return true
580581
}
581-
return providers, includeSvc, nil
582+
return providers.Has(provider)
582583
}
583584

584-
func (c *Controller) fetchSelectedPorts(namespace string, selector *metav1.LabelSelector, providers map[string]struct{}) ([]string, []string, error) {
585+
func (c *Controller) fetchSelectedPorts(namespace string, selector *metav1.LabelSelector, providers set.Set[string]) ([]string, []string, error) {
585586
var subnets []string
586587
sel, err := metav1.LabelSelectorAsSelector(selector)
587588
if err != nil {
@@ -603,23 +604,26 @@ func (c *Controller) fetchSelectedPorts(namespace string, selector *metav1.Label
603604
return nil, nil, fmt.Errorf("failed to get pod networks, %w", err)
604605
}
605606

607+
matchedProvider := false
606608
for _, podNet := range podNets {
607609
if !isOvnSubnet(podNet.Subnet) {
608610
continue
609611
}
610612
provider := podNet.ProviderName
611-
if providers != nil {
612-
if _, ok := providers[provider]; !ok {
613-
continue
614-
}
613+
if !netpolAppliesToProvider(provider, providers) {
614+
continue
615615
}
616+
matchedProvider = true
616617

617618
if pod.Annotations[fmt.Sprintf(util.AllocatedAnnotationTemplate, podNet.ProviderName)] == "true" {
618619
ports = append(ports, ovs.PodNameToPortName(podName, pod.Namespace, podNet.ProviderName))
619620
// Pod selected by networkpolicy has its own subnet which is not the default subnet
620621
subnets = append(subnets, podNet.Subnet.Name)
621622
}
622623
}
624+
if providers != nil && !matchedProvider {
625+
klog.V(4).Infof("skip pod %s/%s: no network attachment matches network_policy_for", pod.Namespace, pod.Name)
626+
}
623627
}
624628
subnets = slices.Compact(subnets)
625629
return ports, subnets, nil
@@ -643,7 +647,7 @@ func hasEgressRule(np *netv1.NetworkPolicy) bool {
643647
return np.Spec.Egress != nil
644648
}
645649

646-
func (c *Controller) fetchPolicySelectedAddresses(namespace, protocol string, npp netv1.NetworkPolicyPeer, providers map[string]struct{}, includeSvc bool) ([]string, []string, error) {
650+
func (c *Controller) fetchPolicySelectedAddresses(namespace, protocol string, npp netv1.NetworkPolicyPeer, providers set.Set[string]) ([]string, []string, error) {
647651
selectedAddresses := []string{}
648652
exceptAddresses := []string{}
649653

@@ -699,13 +703,13 @@ func (c *Controller) fetchPolicySelectedAddresses(namespace, protocol string, np
699703
klog.Errorf("failed to get pod nets %v", err)
700704
return nil, nil, err
701705
}
706+
matchedProvider := false
702707
for _, podNet := range podNets {
703708
provider := podNet.ProviderName
704-
if providers != nil {
705-
if _, ok := providers[provider]; !ok {
706-
continue
707-
}
709+
if !netpolAppliesToProvider(provider, providers) {
710+
continue
708711
}
712+
matchedProvider = true
709713

710714
podIPAnnotation := pod.Annotations[fmt.Sprintf(util.IPAddressAnnotationTemplate, podNet.ProviderName)]
711715
podIPs := strings.SplitSeq(podIPAnnotation, ",")
@@ -714,7 +718,10 @@ func (c *Controller) fetchPolicySelectedAddresses(namespace, protocol string, np
714718
selectedAddresses = append(selectedAddresses, podIP)
715719
}
716720
}
717-
if !includeSvc || len(svcs) == 0 {
721+
if len(svcs) == 0 {
722+
continue
723+
}
724+
if !shouldIncludeServiceIPs(podNet) {
718725
continue
719726
}
720727

@@ -724,11 +731,18 @@ func (c *Controller) fetchPolicySelectedAddresses(namespace, protocol string, np
724731
}
725732
selectedAddresses = append(selectedAddresses, svcIPs...)
726733
}
734+
if providers != nil && !matchedProvider {
735+
klog.V(4).Infof("skip pod %s/%s: no network attachment matches network_policy_for", pod.Namespace, pod.Name)
736+
}
727737
}
728738
}
729739
return selectedAddresses, exceptAddresses, nil
730740
}
731741

742+
func shouldIncludeServiceIPs(podNet *kubeovnNet) bool {
743+
return podNet != nil && podNet.Subnet != nil && podNet.Subnet.Spec.Vpc == util.DefaultVpc
744+
}
745+
732746
func svcMatchPods(svcs []*corev1.Service, pod *corev1.Pod, protocol string) ([]string, error) {
733747
matchSvcs := []string{}
734748
// find svc ip by pod's info

‎pkg/controller/network_policy_test.go‎

Lines changed: 70 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@ import (
66
netv1 "k8s.io/api/networking/v1"
77
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
88

9+
"k8s.io/utils/set"
10+
911
"github.com/stretchr/testify/require"
1012

1113
"github.com/kubeovn/kube-ovn/pkg/util"
@@ -15,66 +17,81 @@ func TestParsePolicyFor(t *testing.T) {
1517
t.Parallel()
1618

1719
tests := []struct {
18-
name string
19-
annotation *string
20-
wantProviders map[string]struct{}
21-
wantIncludeSvc bool
22-
wantErr bool
20+
name string
21+
annotation *string
22+
wantProviders set.Set[string]
23+
wantErr bool
2324
}{
2425
{
25-
name: "annotation omitted",
26-
annotation: nil,
27-
wantProviders: nil,
28-
wantIncludeSvc: true,
29-
wantErr: false,
26+
name: "annotation omitted",
27+
annotation: nil,
28+
wantProviders: nil,
29+
wantErr: false,
30+
},
31+
{
32+
name: "ovn only",
33+
annotation: ptrString("ovn"),
34+
wantProviders: set.New(
35+
util.OvnProvider,
36+
),
37+
wantErr: false,
3038
},
3139
{
32-
name: "primary only",
33-
annotation: ptrString("primary"),
34-
wantProviders: map[string]struct{}{
35-
util.OvnProvider: {},
36-
},
37-
wantIncludeSvc: true,
38-
wantErr: false,
40+
name: "duplicate ovn",
41+
annotation: ptrString("ovn, ovn"),
42+
wantProviders: set.New(
43+
util.OvnProvider,
44+
),
45+
wantErr: false,
3946
},
4047
{
4148
name: "secondary only",
4249
annotation: ptrString("ns1/net1"),
43-
wantProviders: map[string]struct{}{
44-
"net1.ns1." + util.OvnProvider: {},
45-
},
46-
wantIncludeSvc: false,
47-
wantErr: false,
50+
wantProviders: set.New(
51+
"net1.ns1." + util.OvnProvider,
52+
),
53+
wantErr: false,
54+
},
55+
{
56+
name: "ovn and secondary",
57+
annotation: ptrString(" ovn , ns1/net1 "),
58+
wantProviders: set.New(
59+
util.OvnProvider,
60+
"net1.ns1."+util.OvnProvider,
61+
),
62+
wantErr: false,
4863
},
4964
{
50-
name: "primary and secondary",
51-
annotation: ptrString(" primary , ns1/net1 "),
52-
wantProviders: map[string]struct{}{
53-
util.OvnProvider: {},
54-
"net1.ns1." + util.OvnProvider: {},
55-
},
56-
wantIncludeSvc: true,
57-
wantErr: false,
65+
name: "ovn and invalid",
66+
annotation: ptrString("ovn, foo"),
67+
wantProviders: set.New(
68+
util.OvnProvider,
69+
),
70+
wantErr: false,
5871
},
5972
{
60-
name: "invalid all",
61-
annotation: ptrString("all"),
62-
wantErr: true,
73+
name: "invalid all",
74+
annotation: ptrString("all"),
75+
wantProviders: set.New[string](),
76+
wantErr: false,
6377
},
6478
{
65-
name: "invalid default",
66-
annotation: ptrString("default"),
67-
wantErr: true,
79+
name: "invalid default",
80+
annotation: ptrString("default"),
81+
wantProviders: set.New[string](),
82+
wantErr: false,
6883
},
6984
{
70-
name: "invalid no entries",
71-
annotation: ptrString(","),
72-
wantErr: true,
85+
name: "invalid no entries",
86+
annotation: ptrString(","),
87+
wantProviders: set.New[string](),
88+
wantErr: false,
7389
},
7490
{
75-
name: "invalid token",
76-
annotation: ptrString("foo"),
77-
wantErr: true,
91+
name: "invalid token",
92+
annotation: ptrString("foo"),
93+
wantProviders: set.New[string](),
94+
wantErr: false,
7895
},
7996
}
8097

@@ -88,18 +105,17 @@ func TestParsePolicyFor(t *testing.T) {
88105
}
89106
if tt.annotation != nil {
90107
np.Annotations = map[string]string{
91-
policyForAnnotation: *tt.annotation,
108+
util.NetworkPolicyForAnnotation: *tt.annotation,
92109
}
93110
}
94111

95-
providers, includeSvc, err := parsePolicyFor(np)
112+
providers, err := parsePolicyFor(np)
96113
if tt.wantErr {
97114
require.Error(t, err)
98115
return
99116
}
100117

101118
require.NoError(t, err)
102-
require.Equal(t, tt.wantIncludeSvc, includeSvc)
103119
if tt.wantProviders == nil {
104120
require.Nil(t, providers)
105121
return
@@ -109,6 +125,15 @@ func TestParsePolicyFor(t *testing.T) {
109125
}
110126
}
111127

128+
func TestNetpolAppliesToProvider(t *testing.T) {
129+
t.Parallel()
130+
providers := set.New("ovn", "net1.ns1.ovn")
131+
require.True(t, netpolAppliesToProvider("ovn", providers))
132+
require.False(t, netpolAppliesToProvider("net2.ns2.ovn", providers))
133+
require.True(t, netpolAppliesToProvider("ovn", nil))
134+
require.False(t, netpolAppliesToProvider("ovn", set.New[string]()))
135+
}
136+
112137
func ptrString(s string) *string {
113138
return &s
114139
}

‎pkg/util/const.go‎

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

0 commit comments

Comments
 (0)