Skip to content

Commit

Permalink
Allow named ports for IPBlocks on ingress (#92)
Browse files Browse the repository at this point in the history
<!--  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.
  • Loading branch information
haouc authored Mar 30, 2024
2 parents 2cc70e6 + 0de0252 commit 22dea4b
Show file tree
Hide file tree
Showing 3 changed files with 229 additions and 11 deletions.
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
26 changes: 19 additions & 7 deletions pkg/resolvers/endpoints.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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.
Expand Down
212 changes: 209 additions & 3 deletions pkg/resolvers/endpoints_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})
}
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
}
}

0 comments on commit 22dea4b

Please sign in to comment.