Skip to content

Commit

Permalink
Infer PolicyTypes if missing
Browse files Browse the repository at this point in the history
In cases where Spec.PolicyTypes is not specified, it should
default to the existence of Ingress or Egress rules.

Updating end2end tests to cover also this scenario.

Signed-off-by: Andrea Panattoni <apanatto@redhat.com>
  • Loading branch information
zeeke committed Jun 22, 2023
1 parent 39e42b8 commit 971c73a
Show file tree
Hide file tree
Showing 4 changed files with 114 additions and 18 deletions.
2 changes: 0 additions & 2 deletions e2e/tests/simple-v4-egress.yml
Original file line number Diff line number Diff line change
Expand Up @@ -95,8 +95,6 @@ spec:
podSelector:
matchLabels:
name: pod-server
policyTypes:
- Egress
egress:
- to:
- podSelector:
Expand Down
2 changes: 0 additions & 2 deletions e2e/tests/simple-v4-ingress.yml
Original file line number Diff line number Diff line change
Expand Up @@ -95,8 +95,6 @@ spec:
podSelector:
matchLabels:
name: pod-server
policyTypes:
- Ingress
ingress:
- from:
- podSelector:
Expand Down
96 changes: 96 additions & 0 deletions pkg/server/policyrules_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1597,6 +1597,102 @@ COMMIT
Expect(buf.egressPorts.String()).To(Equal(portRules))
})

It("policyType should be implicitly inferred", func() {

policy1 := &multiv1beta1.MultiNetworkPolicy{
ObjectMeta: metav1.ObjectMeta{
Name: "ingressPolicies1",
Namespace: "testns1",
Annotations: map[string]string{
PolicyNetworkAnnotation: "net-attach1",
},
},
Spec: multiv1beta1.MultiNetworkPolicySpec{
PodSelector: metav1.LabelSelector{
MatchLabels: map[string]string{
"role": "targetpod",
},
},
Ingress: []multiv1beta1.MultiNetworkPolicyIngressRule{{
From: []multiv1beta1.MultiNetworkPolicyPeer{{
PodSelector: &metav1.LabelSelector{
MatchLabels: map[string]string{
"foobar": "enabled",
},
},
}},
}},
},
}

s := NewFakeServer("samplehost")
Expect(s).NotTo(BeNil())

AddNamespace(s, "testns1")

Expect(
s.netdefChanges.Update(nil, NewNetDef("testns1", "net-attach1", NewCNIConfig("testCNI", "multi"))),
).To(BeTrue())

pod1 := NewFakePodWithNetAnnotation(
"testns1",
"testpod1",
"net-attach1",
NewFakeNetworkStatus("testns1", "net-attach1", "192.168.1.1", "10.1.1.1"),
map[string]string{
"role": "targetpod",
})
pod1.Spec.NodeName = "samplehost"

AddPod(s, pod1)
podInfo1, err := s.podMap.GetPodInfo(pod1)
Expect(err).NotTo(HaveOccurred())

pod2 := NewFakePodWithNetAnnotation(
"testns1",
"testpod2",
"net-attach1",
NewFakeNetworkStatus("testns1", "net-attach1", "192.168.1.2", "10.1.1.2"),
map[string]string{
"foobar": "enabled",
})
AddPod(s, pod2)

Expect(
s.policyChanges.Update(nil, policy1),
).To(BeTrue())
s.policyMap.Update(s.policyChanges)

result := fakeiptables.NewFake()
s.ip4Tables = result

s.generatePolicyRulesForPod(pod1, podInfo1)

Expect(string(result.Lines)).To(Equal(`*filter
:MULTI-INGRESS - [0:0]
:MULTI-INGRESS-COMMON - [0:0]
:MULTI-EGRESS - [0:0]
:MULTI-EGRESS-COMMON - [0:0]
:MULTI-0-INGRESS - [0:0]
:MULTI-0-INGRESS-0-PORTS - [0:0]
:MULTI-0-INGRESS-0-FROM - [0:0]
-A MULTI-INGRESS-COMMON -m conntrack --ctstate RELATED,ESTABLISHED -j ACCEPT
-A MULTI-INGRESS -j MULTI-INGRESS-COMMON
-A MULTI-INGRESS -m comment --comment "policy:ingressPolicies1 net-attach-def:testns1/net-attach1" -i net1 -j MULTI-0-INGRESS
-A MULTI-INGRESS -m mark --mark 0x30000/0x30000 -j RETURN
-A MULTI-0-INGRESS -j MARK --set-xmark 0x0/0x30000
-A MULTI-0-INGRESS -j MULTI-0-INGRESS-0-PORTS
-A MULTI-0-INGRESS -j MULTI-0-INGRESS-0-FROM
-A MULTI-0-INGRESS -m mark --mark 0x30000/0x30000 -j RETURN
-A MULTI-INGRESS -j DROP
-A MULTI-0-INGRESS-0-PORTS -m comment --comment "no ingress ports, skipped" -j MARK --set-xmark 0x10000/0x10000
-A MULTI-0-INGRESS-0-FROM -i net1 -s 10.1.1.2 -j MARK --set-xmark 0x20000/0x20000
-A MULTI-0-INGRESS-0-FROM -i net1 -s 10.1.1.1 -j MARK --set-xmark 0x20000/0x20000
COMMIT
`))

})

Context("IPv6", func() {
It("shoud avoid using IPv4 addresses on ip6tables", func() {

Expand Down
32 changes: 18 additions & 14 deletions pkg/server/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -477,7 +477,7 @@ func (s *Server) syncMultiPolicy() {

netns, err := ns.GetNS(netnsPath)
if err != nil {
klog.Errorf("cannot get pod (%s/%s:%s) netns: %v", p.Namespace, p.Name, p.Status.Phase, err)
klog.Errorf("cannot get pod (%s/%s:%s) netns (%s): %v", p.Namespace, p.Name, p.Status.Phase, netnsPath, err)
continue
}

Expand Down Expand Up @@ -603,19 +603,7 @@ func (s *Server) generatePolicyRulesForPodAndFamily(pod *v1.Pod, podInfo *contro
}
}

var ingressEnable, egressEnable bool
if len(policy.Spec.PolicyTypes) == 0 {
ingressEnable = true
egressEnable = true
} else {
for _, v := range policy.Spec.PolicyTypes {
if strings.EqualFold(string(v), string(multiv1beta1.PolicyTypeIngress)) {
ingressEnable = true
} else if strings.EqualFold(string(v), string(multiv1beta1.PolicyTypeEgress)) {
egressEnable = true
}
}
}
var ingressEnable, egressEnable bool = getEnabledPolicyTypes(policy)
klog.V(8).Infof("ingress/egress = %v/%v\n", ingressEnable, egressEnable)

policyNetworksAnnot, ok := policy.GetAnnotations()[PolicyNetworkAnnotation]
Expand Down Expand Up @@ -706,3 +694,19 @@ func nadNamespacedName(o *netdefv1.NetworkAttachmentDefinition) string {
}
return o.GetNamespace() + "/" + o.GetName()
}

func getEnabledPolicyTypes(policy *multiv1beta1.MultiNetworkPolicy) (bool, bool) {
var ingressEnable, egressEnable bool
if len(policy.Spec.PolicyTypes) > 0 {
for _, v := range policy.Spec.PolicyTypes {
if strings.EqualFold(string(v), string(multiv1beta1.PolicyTypeIngress)) {
ingressEnable = true
} else if strings.EqualFold(string(v), string(multiv1beta1.PolicyTypeEgress)) {
egressEnable = true
}
}
return ingressEnable, egressEnable
}

return len(policy.Spec.Ingress) > 0, len(policy.Spec.Egress) > 0
}

0 comments on commit 971c73a

Please sign in to comment.