From 0de025261ddfb4bb07f07d7fbafb282975a243ac Mon Sep 17 00:00:00 2001 From: Joseph Chen Date: Tue, 26 Mar 2024 22:44:49 +0000 Subject: [PATCH] Allow named ports for IPBlocks on ingress --- Makefile | 2 +- pkg/resolvers/endpoints.go | 26 ++-- pkg/resolvers/endpoints_test.go | 212 +++++++++++++++++++++++++++++++- 3 files changed, 229 insertions(+), 11 deletions(-) diff --git a/Makefile b/Makefile index 4fff67b..2adf3dc 100644 --- a/Makefile +++ b/Makefile @@ -165,7 +165,7 @@ $(ENVTEST): $(LOCALBIN) .PHONY: ko ko: $(KO) $(KO): $(LOCALBIN) - test -s $(LOCALBIN)/ko || GOBIN=$(LOCALBIN) go install github.com/google/ko@v0.11.2 + test -s $(LOCALBIN)/ko || GOBIN=$(LOCALBIN) go install github.com/google/ko@v0.15.2 .PHONY: mockgen mockgen: $(MOCKGEN) diff --git a/pkg/resolvers/endpoints.go b/pkg/resolvers/endpoints.go index a56ce80..7f4b86c 100644 --- a/pkg/resolvers/endpoints.go +++ b/pkg/resolvers/endpoints.go @@ -65,7 +65,7 @@ func (r *defaultEndpointsResolver) computeIngressEndpoints(ctx context.Context, for _, rule := range policy.Spec.Ingress { r.logger.V(1).Info("computing ingress addresses", "peers", rule.From) if rule.From == nil { - ingressEndpoints = append(ingressEndpoints, r.getAllowAllNetworkPeers(rule.Ports)...) + ingressEndpoints = append(ingressEndpoints, r.getAllowAllNetworkPeers(ctx, policy, rule.Ports, networking.PolicyTypeIngress)...) continue } resolvedPeers, err := r.resolveNetworkPeers(ctx, policy, rule.From, rule.Ports, networking.PolicyTypeIngress) @@ -83,7 +83,7 @@ func (r *defaultEndpointsResolver) computeEgressEndpoints(ctx context.Context, p for _, rule := range policy.Spec.Egress { r.logger.V(1).Info("computing egress addresses", "peers", rule.To) if rule.To == nil { - egressEndpoints = append(egressEndpoints, r.getAllowAllNetworkPeers(rule.Ports)...) + egressEndpoints = append(egressEndpoints, r.getAllowAllNetworkPeers(ctx, policy, rule.Ports, networking.PolicyTypeEgress)...) continue } resolvedPeers, err := r.resolveNetworkPeers(ctx, policy, rule.To, rule.Ports, networking.PolicyTypeEgress) @@ -130,11 +130,17 @@ func (r *defaultEndpointsResolver) computePodSelectorEndpoints(ctx context.Conte return podEndpoints, nil } -func (r *defaultEndpointsResolver) getAllowAllNetworkPeers(ports []networking.NetworkPolicyPort) []policyinfo.EndpointInfo { +func (r *defaultEndpointsResolver) getAllowAllNetworkPeers(ctx context.Context, policy *networking.NetworkPolicy, ports []networking.NetworkPolicyPort, policyType networking.PolicyType) []policyinfo.EndpointInfo { var portList []policyinfo.Port for _, port := range ports { - if port := r.convertToPolicyInfoPortForCIDRs(port); port != nil { - portList = append(portList, *port) + portInfo := r.convertToPolicyInfoPortForCIDRs(port) + if portInfo != nil { + portList = append(portList, *portInfo) + } else { + if policyType == networking.PolicyTypeIngress { + ports := r.getIngressRulesPorts(ctx, policy.Namespace, &policy.Spec.PodSelector, []networking.NetworkPolicyPort{port}) + portList = append(portList, ports...) + } } } if len(ports) != 0 && len(portList) == 0 { @@ -163,8 +169,14 @@ func (r *defaultEndpointsResolver) resolveNetworkPeers(ctx context.Context, poli } var portList []policyinfo.Port for _, port := range ports { - if port := r.convertToPolicyInfoPortForCIDRs(port); port != nil { - portList = append(portList, *port) + portInfo := r.convertToPolicyInfoPortForCIDRs(port) + if portInfo != nil { + portList = append(portList, *portInfo) + } else { + if policyType == networking.PolicyTypeIngress { + ports := r.getIngressRulesPorts(ctx, policy.Namespace, &policy.Spec.PodSelector, []networking.NetworkPolicyPort{port}) + portList = append(portList, ports...) + } } } // A non-empty input port list would imply the user wants to allow traffic only on the specified ports. diff --git a/pkg/resolvers/endpoints_test.go b/pkg/resolvers/endpoints_test.go index f91b609..ccf99f2 100644 --- a/pkg/resolvers/endpoints_test.go +++ b/pkg/resolvers/endpoints_test.go @@ -176,7 +176,7 @@ func TestEndpointsResolver_getAllowAllNetworkPeers(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { resolver := &defaultEndpointsResolver{} - got := resolver.getAllowAllNetworkPeers(tt.args.ports) + got := resolver.getAllowAllNetworkPeers(context.TODO(), nil, tt.args.ports, networking.PolicyTypeEgress) assert.Equal(t, tt.want, got) }) } @@ -819,7 +819,7 @@ func TestEndpointsResolver_ResolveNetworkPeers(t *testing.T) { ), ) if rule.From == nil { - ingressEndpoints = append(ingressEndpoints, resolver.getAllowAllNetworkPeers(rule.Ports)...) + ingressEndpoints = append(ingressEndpoints, resolver.getAllowAllNetworkPeers(ctx, policy, rule.Ports, networking.PolicyTypeIngress)...) continue } resolvedPeers, err := resolver.resolveNetworkPeers(ctx, policy, rule.From, rule.Ports, networking.PolicyTypeIngress) @@ -863,7 +863,7 @@ func TestEndpointsResolver_ResolveNetworkPeers(t *testing.T) { for _, rule := range policy.Spec.Egress { if rule.To == nil { - egressEndpoints = append(egressEndpoints, resolver.getAllowAllNetworkPeers(rule.Ports)...) + egressEndpoints = append(egressEndpoints, resolver.getAllowAllNetworkPeers(ctx, policy, rule.Ports, networking.PolicyTypeEgress)...) continue } resolvedPeers, err := resolver.resolveNetworkPeers(ctx, policy, rule.To, rule.Ports, networking.PolicyTypeEgress) @@ -895,3 +895,209 @@ func TestEndpointsResolver_ResolveNetworkPeers(t *testing.T) { assert.Equal(t, *policy.Spec.Egress[0].Ports[0].EndPort, *egPE.Ports[0].EndPort) } } + +func TestEndpointsResolver_ResolveNetworkPeers_NamedIngressPortsIPBlocks(t *testing.T) { + protocolTCP := corev1.ProtocolTCP + port8080 := int32(8080) + port9090 := int32(9090) + + dstPod := corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "pod1", + Namespace: "dst", + }, + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + { + Name: "pod1", + Ports: []corev1.ContainerPort{ + { + ContainerPort: port8080, + Protocol: corev1.ProtocolTCP, + Name: "src-port", + }, + { + ContainerPort: port9090, + Protocol: corev1.ProtocolTCP, + Name: "src-port2", + }, + }, + }, + }, + }, + Status: corev1.PodStatus{ + PodIP: "1.0.0.1", + }, + } + + portsMap := map[string]int32{ + "src-port": port8080, + "src-port2": port9090, + } + + // the policy is applied to dst namespace on dst pod + policy := &networking.NetworkPolicy{ + ObjectMeta: metav1.ObjectMeta{ + Name: "netpol", + Namespace: "dst", + }, + Spec: networking.NetworkPolicySpec{ + PodSelector: metav1.LabelSelector{}, + PolicyTypes: []networking.PolicyType{networking.PolicyTypeIngress}, + Ingress: []networking.NetworkPolicyIngressRule{ + { + From: []networking.NetworkPolicyPeer{ + { + IPBlock: &networking.IPBlock{ + CIDR: "100.64.0.0/16", + }, + }, + }, + Ports: []networking.NetworkPolicyPort{ + { + Protocol: &protocolTCP, + Port: &intstr.IntOrString{Type: intstr.String, StrVal: "src-port"}, + }, + { + Protocol: &protocolTCP, + Port: &intstr.IntOrString{Type: intstr.String, StrVal: "src-port2"}, + }, + }, + }, + }, + }, + } + + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + mockClient := mock_client.NewMockClient(ctrl) + resolver := NewEndpointsResolver(mockClient, logr.New(&log.NullLogSink{})) + + var ingressEndpoints []policyinfo.EndpointInfo + ctx := context.TODO() + for _, rule := range policy.Spec.Ingress { + podList := &corev1.PodList{} + gomock.InOrder( + mockClient.EXPECT().List(gomock.Any(), podList, gomock.Any()).DoAndReturn( + func(ctx context.Context, podList *corev1.PodList, opts ...client.ListOption) error { + podList.Items = []corev1.Pod{dstPod} + return nil + }, + ), + mockClient.EXPECT().List(gomock.Any(), podList, gomock.Any()).DoAndReturn( + func(ctx context.Context, podList *corev1.PodList, opts ...client.ListOption) error { + podList.Items = []corev1.Pod{dstPod} + return nil + }, + ), + ) + if rule.From == nil { + ingressEndpoints = append(ingressEndpoints, resolver.getAllowAllNetworkPeers(ctx, policy, rule.Ports, networking.PolicyTypeIngress)...) + continue + } + resolvedPeers, err := resolver.resolveNetworkPeers(ctx, policy, rule.From, rule.Ports, networking.PolicyTypeIngress) + assert.NoError(t, err) + ingressEndpoints = append(ingressEndpoints, resolvedPeers...) + } + + ingPE := ingressEndpoints[0] + + // Should allow ingress from 100.64.0.0/16 on ports 8080 and 9090 + assert.Equal(t, "100.64.0.0/16", string(ingPE.CIDR)) + assert.Equal(t, 2, len(ingPE.Ports)) + assert.Equal(t, dstPod.Spec.Containers[0].Ports[0].ContainerPort, *ingPE.Ports[0].Port) + assert.Equal(t, dstPod.Spec.Containers[0].Ports[1].ContainerPort, *ingPE.Ports[1].Port) + assert.Equal(t, portsMap[policy.Spec.Ingress[0].Ports[0].Port.StrVal], *ingPE.Ports[0].Port) + assert.Equal(t, portsMap[policy.Spec.Ingress[0].Ports[1].Port.StrVal], *ingPE.Ports[1].Port) + + dstPod2 := corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "pod2", + Namespace: "dst2", + }, + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + { + Name: "pod2", + Ports: []corev1.ContainerPort{ + { + ContainerPort: port8080, + Protocol: corev1.ProtocolTCP, + Name: "src-port", + }, + { + ContainerPort: port9090, + Protocol: corev1.ProtocolTCP, + Name: "src-port2", + }, + }, + }, + }, + }, + Status: corev1.PodStatus{ + PodIP: "1.0.0.2", + }, + } + + policyAll := &networking.NetworkPolicy{ + ObjectMeta: metav1.ObjectMeta{ + Name: "netpolAll", + Namespace: "dst2", + }, + Spec: networking.NetworkPolicySpec{ + PodSelector: metav1.LabelSelector{}, + PolicyTypes: []networking.PolicyType{networking.PolicyTypeIngress}, + Ingress: []networking.NetworkPolicyIngressRule{ + { + Ports: []networking.NetworkPolicyPort{ + { + Protocol: &protocolTCP, + Port: &intstr.IntOrString{Type: intstr.String, StrVal: "src-port"}, + }, + { + Protocol: &protocolTCP, + Port: &intstr.IntOrString{Type: intstr.String, StrVal: "src-port2"}, + }, + }, + }, + }, + }, + } + + var ingressEndpointsAll []policyinfo.EndpointInfo + for _, rule := range policyAll.Spec.Ingress { + podList := &corev1.PodList{} + gomock.InOrder( + mockClient.EXPECT().List(gomock.Any(), podList, gomock.Any()).DoAndReturn( + func(ctx context.Context, podList *corev1.PodList, opts ...client.ListOption) error { + podList.Items = []corev1.Pod{dstPod2} + return nil + }, + ), + mockClient.EXPECT().List(gomock.Any(), podList, gomock.Any()).DoAndReturn( + func(ctx context.Context, podList *corev1.PodList, opts ...client.ListOption) error { + podList.Items = []corev1.Pod{dstPod2} + return nil + }, + ), + ) + if rule.From == nil { + ingressEndpointsAll = append(ingressEndpointsAll, resolver.getAllowAllNetworkPeers(ctx, policy, rule.Ports, networking.PolicyTypeIngress)...) + continue + } + resolvedPeers, err := resolver.resolveNetworkPeers(ctx, policy, rule.From, rule.Ports, networking.PolicyTypeIngress) + assert.NoError(t, err) + ingressEndpointsAll = append(ingressEndpointsAll, resolvedPeers...) + } + + // Should allow ingress from all addresses on ports 8080 and 9090 + for _, ingPE := range ingressEndpointsAll { + assert.True(t, "0.0.0.0/0" == string(ingPE.CIDR) || "::/0" == string(ingPE.CIDR)) + assert.Equal(t, 2, len(ingPE.Ports)) + assert.Equal(t, dstPod2.Spec.Containers[0].Ports[0].ContainerPort, *ingPE.Ports[0].Port) + assert.Equal(t, dstPod2.Spec.Containers[0].Ports[1].ContainerPort, *ingPE.Ports[1].Port) + assert.Equal(t, portsMap[policy.Spec.Ingress[0].Ports[0].Port.StrVal], *ingPE.Ports[0].Port) + assert.Equal(t, portsMap[policy.Spec.Ingress[0].Ports[1].Port.StrVal], *ingPE.Ports[1].Port) + } +}