Skip to content

Commit 22dea4b

Browse files
haouchaouc
authoredMar 30, 2024
Allow named ports for IPBlocks on ingress (#92)
<!-- Thanks for sending a pull request! Here are some tips for you: 1. Ensure you have added the unit tests for your changes. 2. Ensure you have included output of manual testing done in the Testing section. 3. Ensure number of lines of code for new or existing methods are within the reasonable limit. 4. Ensure your change works on existing clusters after upgrade. --> **What type of PR is this?** <!-- Add one of the following: bug cleanup documentation feature --> bug **Which issue does this PR fix**: #81 **What does this PR do / Why do we need it**: Allows named ports when using IPBlocks (ingress rules only) **If an issue # is not available please add steps to reproduce and the controller logs**: **Testing done on this change**: <!-- output of manual testing/integration tests results and also attach logs showing the fix being resolved --> Manually applied Cx config and policyendpoint looks fine ``` Spec: Ingress: Cidr: 172.17.0.0/16 Ports: Port: 80 Protocol: TCP Cidr: 192.168.8.106 Ports: Port: 443 Protocol: TCP Pod Isolation: Ingress Pod Selector: Match Labels: App: target Pod Selector Endpoints: Host IP: 192.168.98.89 Name: target Namespace: default Pod IP: 192.168.125.203 Policy Ref: Name: allow-web-traffic Namespace: default ``` Using allow all in ingress rule ``` Spec: Ingress: Cidr: 0.0.0.0/0 Ports: Port: 80 Protocol: TCP Port: 443 Protocol: TCP Cidr: ::/0 Ports: Port: 80 Protocol: TCP Port: 443 Protocol: TCP Pod Isolation: Ingress Pod Selector: Match Labels: App: target Pod Selector Endpoints: Host IP: 192.168.98.89 Name: target Namespace: default Pod IP: 192.168.116.203 Policy Ref: Name: allow-web-traffic Namespace: default ``` **Automation added to e2e**: <!-- List the e2e tests you added as part of this PR. If no, create an issue with enhancement/testing label --> 1. NP allowing ingress from IPBlock + 2 named ports 2. NP allowing ingress from all CIDRs + 2 named ports **Will this PR introduce any new dependencies?**: <!-- e.g. new K8s API --> No **Will this break upgrades or downgrades. Has updating a running cluster been tested?**: No **Does this PR introduce any user-facing change?**: <!-- If yes, a release note update is required: Enter your extended release note in the block below. If the PR requires additional actions from users switching to the new release, include the string "action required". --> Yes ```release-note Allow for using named ports when using IPBlocks for the ingress source. ``` By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
2 parents 2cc70e6 + 0de0252 commit 22dea4b

File tree

3 files changed

+229
-11
lines changed

3 files changed

+229
-11
lines changed
 

‎Makefile

+1-1
Original file line numberDiff line numberDiff line change
@@ -165,7 +165,7 @@ $(ENVTEST): $(LOCALBIN)
165165
.PHONY: ko
166166
ko: $(KO)
167167
$(KO): $(LOCALBIN)
168-
test -s $(LOCALBIN)/ko || GOBIN=$(LOCALBIN) go install github.com/google/ko@v0.11.2
168+
test -s $(LOCALBIN)/ko || GOBIN=$(LOCALBIN) go install github.com/google/ko@v0.15.2
169169

170170
.PHONY: mockgen
171171
mockgen: $(MOCKGEN)

‎pkg/resolvers/endpoints.go

+19-7
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ func (r *defaultEndpointsResolver) computeIngressEndpoints(ctx context.Context,
6565
for _, rule := range policy.Spec.Ingress {
6666
r.logger.V(1).Info("computing ingress addresses", "peers", rule.From)
6767
if rule.From == nil {
68-
ingressEndpoints = append(ingressEndpoints, r.getAllowAllNetworkPeers(rule.Ports)...)
68+
ingressEndpoints = append(ingressEndpoints, r.getAllowAllNetworkPeers(ctx, policy, rule.Ports, networking.PolicyTypeIngress)...)
6969
continue
7070
}
7171
resolvedPeers, err := r.resolveNetworkPeers(ctx, policy, rule.From, rule.Ports, networking.PolicyTypeIngress)
@@ -83,7 +83,7 @@ func (r *defaultEndpointsResolver) computeEgressEndpoints(ctx context.Context, p
8383
for _, rule := range policy.Spec.Egress {
8484
r.logger.V(1).Info("computing egress addresses", "peers", rule.To)
8585
if rule.To == nil {
86-
egressEndpoints = append(egressEndpoints, r.getAllowAllNetworkPeers(rule.Ports)...)
86+
egressEndpoints = append(egressEndpoints, r.getAllowAllNetworkPeers(ctx, policy, rule.Ports, networking.PolicyTypeEgress)...)
8787
continue
8888
}
8989
resolvedPeers, err := r.resolveNetworkPeers(ctx, policy, rule.To, rule.Ports, networking.PolicyTypeEgress)
@@ -130,11 +130,17 @@ func (r *defaultEndpointsResolver) computePodSelectorEndpoints(ctx context.Conte
130130
return podEndpoints, nil
131131
}
132132

133-
func (r *defaultEndpointsResolver) getAllowAllNetworkPeers(ports []networking.NetworkPolicyPort) []policyinfo.EndpointInfo {
133+
func (r *defaultEndpointsResolver) getAllowAllNetworkPeers(ctx context.Context, policy *networking.NetworkPolicy, ports []networking.NetworkPolicyPort, policyType networking.PolicyType) []policyinfo.EndpointInfo {
134134
var portList []policyinfo.Port
135135
for _, port := range ports {
136-
if port := r.convertToPolicyInfoPortForCIDRs(port); port != nil {
137-
portList = append(portList, *port)
136+
portInfo := r.convertToPolicyInfoPortForCIDRs(port)
137+
if portInfo != nil {
138+
portList = append(portList, *portInfo)
139+
} else {
140+
if policyType == networking.PolicyTypeIngress {
141+
ports := r.getIngressRulesPorts(ctx, policy.Namespace, &policy.Spec.PodSelector, []networking.NetworkPolicyPort{port})
142+
portList = append(portList, ports...)
143+
}
138144
}
139145
}
140146
if len(ports) != 0 && len(portList) == 0 {
@@ -163,8 +169,14 @@ func (r *defaultEndpointsResolver) resolveNetworkPeers(ctx context.Context, poli
163169
}
164170
var portList []policyinfo.Port
165171
for _, port := range ports {
166-
if port := r.convertToPolicyInfoPortForCIDRs(port); port != nil {
167-
portList = append(portList, *port)
172+
portInfo := r.convertToPolicyInfoPortForCIDRs(port)
173+
if portInfo != nil {
174+
portList = append(portList, *portInfo)
175+
} else {
176+
if policyType == networking.PolicyTypeIngress {
177+
ports := r.getIngressRulesPorts(ctx, policy.Namespace, &policy.Spec.PodSelector, []networking.NetworkPolicyPort{port})
178+
portList = append(portList, ports...)
179+
}
168180
}
169181
}
170182
// A non-empty input port list would imply the user wants to allow traffic only on the specified ports.

‎pkg/resolvers/endpoints_test.go

+209-3
Original file line numberDiff line numberDiff line change
@@ -176,7 +176,7 @@ func TestEndpointsResolver_getAllowAllNetworkPeers(t *testing.T) {
176176
for _, tt := range tests {
177177
t.Run(tt.name, func(t *testing.T) {
178178
resolver := &defaultEndpointsResolver{}
179-
got := resolver.getAllowAllNetworkPeers(tt.args.ports)
179+
got := resolver.getAllowAllNetworkPeers(context.TODO(), nil, tt.args.ports, networking.PolicyTypeEgress)
180180
assert.Equal(t, tt.want, got)
181181
})
182182
}
@@ -819,7 +819,7 @@ func TestEndpointsResolver_ResolveNetworkPeers(t *testing.T) {
819819
),
820820
)
821821
if rule.From == nil {
822-
ingressEndpoints = append(ingressEndpoints, resolver.getAllowAllNetworkPeers(rule.Ports)...)
822+
ingressEndpoints = append(ingressEndpoints, resolver.getAllowAllNetworkPeers(ctx, policy, rule.Ports, networking.PolicyTypeIngress)...)
823823
continue
824824
}
825825
resolvedPeers, err := resolver.resolveNetworkPeers(ctx, policy, rule.From, rule.Ports, networking.PolicyTypeIngress)
@@ -863,7 +863,7 @@ func TestEndpointsResolver_ResolveNetworkPeers(t *testing.T) {
863863

864864
for _, rule := range policy.Spec.Egress {
865865
if rule.To == nil {
866-
egressEndpoints = append(egressEndpoints, resolver.getAllowAllNetworkPeers(rule.Ports)...)
866+
egressEndpoints = append(egressEndpoints, resolver.getAllowAllNetworkPeers(ctx, policy, rule.Ports, networking.PolicyTypeEgress)...)
867867
continue
868868
}
869869
resolvedPeers, err := resolver.resolveNetworkPeers(ctx, policy, rule.To, rule.Ports, networking.PolicyTypeEgress)
@@ -895,3 +895,209 @@ func TestEndpointsResolver_ResolveNetworkPeers(t *testing.T) {
895895
assert.Equal(t, *policy.Spec.Egress[0].Ports[0].EndPort, *egPE.Ports[0].EndPort)
896896
}
897897
}
898+
899+
func TestEndpointsResolver_ResolveNetworkPeers_NamedIngressPortsIPBlocks(t *testing.T) {
900+
protocolTCP := corev1.ProtocolTCP
901+
port8080 := int32(8080)
902+
port9090 := int32(9090)
903+
904+
dstPod := corev1.Pod{
905+
ObjectMeta: metav1.ObjectMeta{
906+
Name: "pod1",
907+
Namespace: "dst",
908+
},
909+
Spec: corev1.PodSpec{
910+
Containers: []corev1.Container{
911+
{
912+
Name: "pod1",
913+
Ports: []corev1.ContainerPort{
914+
{
915+
ContainerPort: port8080,
916+
Protocol: corev1.ProtocolTCP,
917+
Name: "src-port",
918+
},
919+
{
920+
ContainerPort: port9090,
921+
Protocol: corev1.ProtocolTCP,
922+
Name: "src-port2",
923+
},
924+
},
925+
},
926+
},
927+
},
928+
Status: corev1.PodStatus{
929+
PodIP: "1.0.0.1",
930+
},
931+
}
932+
933+
portsMap := map[string]int32{
934+
"src-port": port8080,
935+
"src-port2": port9090,
936+
}
937+
938+
// the policy is applied to dst namespace on dst pod
939+
policy := &networking.NetworkPolicy{
940+
ObjectMeta: metav1.ObjectMeta{
941+
Name: "netpol",
942+
Namespace: "dst",
943+
},
944+
Spec: networking.NetworkPolicySpec{
945+
PodSelector: metav1.LabelSelector{},
946+
PolicyTypes: []networking.PolicyType{networking.PolicyTypeIngress},
947+
Ingress: []networking.NetworkPolicyIngressRule{
948+
{
949+
From: []networking.NetworkPolicyPeer{
950+
{
951+
IPBlock: &networking.IPBlock{
952+
CIDR: "100.64.0.0/16",
953+
},
954+
},
955+
},
956+
Ports: []networking.NetworkPolicyPort{
957+
{
958+
Protocol: &protocolTCP,
959+
Port: &intstr.IntOrString{Type: intstr.String, StrVal: "src-port"},
960+
},
961+
{
962+
Protocol: &protocolTCP,
963+
Port: &intstr.IntOrString{Type: intstr.String, StrVal: "src-port2"},
964+
},
965+
},
966+
},
967+
},
968+
},
969+
}
970+
971+
ctrl := gomock.NewController(t)
972+
defer ctrl.Finish()
973+
974+
mockClient := mock_client.NewMockClient(ctrl)
975+
resolver := NewEndpointsResolver(mockClient, logr.New(&log.NullLogSink{}))
976+
977+
var ingressEndpoints []policyinfo.EndpointInfo
978+
ctx := context.TODO()
979+
for _, rule := range policy.Spec.Ingress {
980+
podList := &corev1.PodList{}
981+
gomock.InOrder(
982+
mockClient.EXPECT().List(gomock.Any(), podList, gomock.Any()).DoAndReturn(
983+
func(ctx context.Context, podList *corev1.PodList, opts ...client.ListOption) error {
984+
podList.Items = []corev1.Pod{dstPod}
985+
return nil
986+
},
987+
),
988+
mockClient.EXPECT().List(gomock.Any(), podList, gomock.Any()).DoAndReturn(
989+
func(ctx context.Context, podList *corev1.PodList, opts ...client.ListOption) error {
990+
podList.Items = []corev1.Pod{dstPod}
991+
return nil
992+
},
993+
),
994+
)
995+
if rule.From == nil {
996+
ingressEndpoints = append(ingressEndpoints, resolver.getAllowAllNetworkPeers(ctx, policy, rule.Ports, networking.PolicyTypeIngress)...)
997+
continue
998+
}
999+
resolvedPeers, err := resolver.resolveNetworkPeers(ctx, policy, rule.From, rule.Ports, networking.PolicyTypeIngress)
1000+
assert.NoError(t, err)
1001+
ingressEndpoints = append(ingressEndpoints, resolvedPeers...)
1002+
}
1003+
1004+
ingPE := ingressEndpoints[0]
1005+
1006+
// Should allow ingress from 100.64.0.0/16 on ports 8080 and 9090
1007+
assert.Equal(t, "100.64.0.0/16", string(ingPE.CIDR))
1008+
assert.Equal(t, 2, len(ingPE.Ports))
1009+
assert.Equal(t, dstPod.Spec.Containers[0].Ports[0].ContainerPort, *ingPE.Ports[0].Port)
1010+
assert.Equal(t, dstPod.Spec.Containers[0].Ports[1].ContainerPort, *ingPE.Ports[1].Port)
1011+
assert.Equal(t, portsMap[policy.Spec.Ingress[0].Ports[0].Port.StrVal], *ingPE.Ports[0].Port)
1012+
assert.Equal(t, portsMap[policy.Spec.Ingress[0].Ports[1].Port.StrVal], *ingPE.Ports[1].Port)
1013+
1014+
dstPod2 := corev1.Pod{
1015+
ObjectMeta: metav1.ObjectMeta{
1016+
Name: "pod2",
1017+
Namespace: "dst2",
1018+
},
1019+
Spec: corev1.PodSpec{
1020+
Containers: []corev1.Container{
1021+
{
1022+
Name: "pod2",
1023+
Ports: []corev1.ContainerPort{
1024+
{
1025+
ContainerPort: port8080,
1026+
Protocol: corev1.ProtocolTCP,
1027+
Name: "src-port",
1028+
},
1029+
{
1030+
ContainerPort: port9090,
1031+
Protocol: corev1.ProtocolTCP,
1032+
Name: "src-port2",
1033+
},
1034+
},
1035+
},
1036+
},
1037+
},
1038+
Status: corev1.PodStatus{
1039+
PodIP: "1.0.0.2",
1040+
},
1041+
}
1042+
1043+
policyAll := &networking.NetworkPolicy{
1044+
ObjectMeta: metav1.ObjectMeta{
1045+
Name: "netpolAll",
1046+
Namespace: "dst2",
1047+
},
1048+
Spec: networking.NetworkPolicySpec{
1049+
PodSelector: metav1.LabelSelector{},
1050+
PolicyTypes: []networking.PolicyType{networking.PolicyTypeIngress},
1051+
Ingress: []networking.NetworkPolicyIngressRule{
1052+
{
1053+
Ports: []networking.NetworkPolicyPort{
1054+
{
1055+
Protocol: &protocolTCP,
1056+
Port: &intstr.IntOrString{Type: intstr.String, StrVal: "src-port"},
1057+
},
1058+
{
1059+
Protocol: &protocolTCP,
1060+
Port: &intstr.IntOrString{Type: intstr.String, StrVal: "src-port2"},
1061+
},
1062+
},
1063+
},
1064+
},
1065+
},
1066+
}
1067+
1068+
var ingressEndpointsAll []policyinfo.EndpointInfo
1069+
for _, rule := range policyAll.Spec.Ingress {
1070+
podList := &corev1.PodList{}
1071+
gomock.InOrder(
1072+
mockClient.EXPECT().List(gomock.Any(), podList, gomock.Any()).DoAndReturn(
1073+
func(ctx context.Context, podList *corev1.PodList, opts ...client.ListOption) error {
1074+
podList.Items = []corev1.Pod{dstPod2}
1075+
return nil
1076+
},
1077+
),
1078+
mockClient.EXPECT().List(gomock.Any(), podList, gomock.Any()).DoAndReturn(
1079+
func(ctx context.Context, podList *corev1.PodList, opts ...client.ListOption) error {
1080+
podList.Items = []corev1.Pod{dstPod2}
1081+
return nil
1082+
},
1083+
),
1084+
)
1085+
if rule.From == nil {
1086+
ingressEndpointsAll = append(ingressEndpointsAll, resolver.getAllowAllNetworkPeers(ctx, policy, rule.Ports, networking.PolicyTypeIngress)...)
1087+
continue
1088+
}
1089+
resolvedPeers, err := resolver.resolveNetworkPeers(ctx, policy, rule.From, rule.Ports, networking.PolicyTypeIngress)
1090+
assert.NoError(t, err)
1091+
ingressEndpointsAll = append(ingressEndpointsAll, resolvedPeers...)
1092+
}
1093+
1094+
// Should allow ingress from all addresses on ports 8080 and 9090
1095+
for _, ingPE := range ingressEndpointsAll {
1096+
assert.True(t, "0.0.0.0/0" == string(ingPE.CIDR) || "::/0" == string(ingPE.CIDR))
1097+
assert.Equal(t, 2, len(ingPE.Ports))
1098+
assert.Equal(t, dstPod2.Spec.Containers[0].Ports[0].ContainerPort, *ingPE.Ports[0].Port)
1099+
assert.Equal(t, dstPod2.Spec.Containers[0].Ports[1].ContainerPort, *ingPE.Ports[1].Port)
1100+
assert.Equal(t, portsMap[policy.Spec.Ingress[0].Ports[0].Port.StrVal], *ingPE.Ports[0].Port)
1101+
assert.Equal(t, portsMap[policy.Spec.Ingress[0].Ports[1].Port.StrVal], *ingPE.Ports[1].Port)
1102+
}
1103+
}

0 commit comments

Comments
 (0)
Failed to load comments.