Named ports in initContainer sidecars do not work with NetworkPolicies #8881

george-angel · 2024-06-05T04:44:09Z

Expected Behavior

Named port in the initContainer spec allows the traffic if it matches config of the NetworkPolicy.

Current Behavior

We have recently migrated our traditional sidecar definitions to the new recommended initContainer sidecars.

We are running a CockroachDB StatefulSet that has a Vault sidecar, such that members 0 and 1 have a "legacy" sidecar container:

$ kubectl --context prod-aws -n partner-registration get -o json pod cockroachdb-0 | jq -r '.spec.containers[] | select(.name == "vault-credentials-agent") | .ports'
[
  {
    "containerPort": 8099,
    "name": "metrics",
    "protocol": "TCP"
  }
]

$ kubectl --context prod-aws -n partner-registration get -o json pod cockroachdb-1 | jq -r '.spec.containers[] | select(.name == "vault-credentials-agent") | .ports'
[
  {
    "containerPort": 8099,
    "name": "metrics",
    "protocol": "TCP"
  }
]

And member 2 has a new initContainer style sidecar:

$ kubectl --context prod-aws -n partner-registration get -o json pod cockroachdb-2 | jq -r '.spec.initContainers[] | select(.name == "vault-credentials-agent") | .ports[]'
{
  "containerPort": 8099,
  "name": "metrics",
  "protocol": "TCP"
}

They all expose a named port metrics.

We also have a NetworkPolicy in this namespace such that:

$ kubectl --context prod-aws -n partner-registration get netpol private-ingress-sys-prom -o yaml
apiVersion: networking.k8s.io/v1
kind: NetworkPolicy
metadata:
  annotations:
    kubectl.kubernetes.io/last-applied-configuration: |
      {"apiVersion":"networking.k8s.io/v1","kind":"NetworkPolicy","metadata":{"annotations":{},"name":"private-ingress-sys-prom","namespace":"partner-registration"},"spec":{"ingress":[{"from":[{"namespaceSelector":{"matchLabels":{"name":"sys-prom"}}}],"ports":[{"port":"metrics"}]}],"podSelector":{},"policyTypes":["Ingress"]}}
  creationTimestamp: "2021-03-26T10:52:05Z"
  generation: 3
  name: private-ingress-sys-prom
  namespace: partner-registration
  resourceVersion: "3454111968"
  uid: 26a53b37-53e3-4a8d-92dd-8e17836b087d
spec:
  ingress:
  - from:
    - namespaceSelector:
        matchLabels:
          name: sys-prom
    ports:
    - port: metrics
      protocol: TCP
  podSelector: {}
  policyTypes:
  - Ingress

Testing the connection from Prometheus I observe the following results for member 0 and 1:

$ kubectl --context prod-aws -n sys-prom exec -ti prometheus-system-0 -c prometheus -- wget -T2 -O - http://10.2.142.58:8099/__/metrics | head -4
Connecting to 10.2.142.58:8099 (10.2.142.58:8099)
writing to stdout
# HELP go_gc_duration_seconds A summary of the pause duration of garbage collection cycles.
# TYPE go_gc_duration_seconds summary

$ kubectl --context prod-aws -n sys-prom exec -ti prometheus-system-0 -c prometheus -- wget -T2 -O - http://10.2.13.56:8099/__/metrics | head -4
Connecting to 10.2.13.56:8099 (10.2.13.56:8099)
writing to stdout
# HELP go_gc_duration_seconds A summary of the pause duration of garbage collection cycles.
# TYPE go_gc_duration_seconds summary

And a timeout for member 2 (running initContainer sidecar):

$ kubectl --context prod-aws -n sys-prom exec -ti prometheus-system-0 -c prometheus -- wget -T2 -O - http://10.2.2.53:8099/__/metrics | head -4
Connecting to 10.2.2.53:8099 (10.2.2.53:8099)
wget: download timed out
command terminated with exit code 1

If I apply the following patch:

diff --git a/prod-aws/partner-registration/02-network-policies.yaml b/prod-aws/partner-registration/02-network-policies.yaml
index 9d3ea45d86..c685cc8667 100644
--- a/prod-aws/partner-registration/02-network-policies.yaml
+++ b/prod-aws/partner-registration/02-network-policies.yaml
@@ -43,7 +43,7 @@ spec:
             matchLabels:
               name: sys-prom
       ports:
-        - port: metrics
+        - port: 8099
 ---
 #  Allow private Ingress
 ---

Then I'm able to establish a connection with cockroachdb-2:

$ kubectl --context prod-aws -n sys-prom exec -ti prometheus-system-0 -c prometheus -- wget -T2 -O - http://10.2.2.53:8099/__/metrics | head -4
Connecting to 10.2.2.53:8099 (10.2.2.53:8099)
writing to stdout
# HELP go_gc_duration_seconds A summary of the pause duration of garbage collection cycles.
# TYPE go_gc_duration_seconds summary

This is the Kyverno config that injects our sidecars the describes the whole config of the injected initContainer sidecar: https://github.com/utilitywarehouse/system-manifests/blob/master/kyverno/policies/pods/injectSidecar.yaml#L40-L88

Possible Solution

@fasaxc suggested:

Probabaly just need to loop over init containers in this block: https://github.com/projectcalico/calico/blob/master/libcalico-go/lib/backend/k8s/conversion/workload_endpoint_default.go#L192

So perhaps something like this

diff --git a/libcalico-go/lib/backend/k8s/conversion/workload_endpoint_default.go b/libcalico-go/lib/backend/k8s/conversion/workload_endpoint_default.go
index db5b14d4b..ab4ada599 100644
--- a/libcalico-go/lib/backend/k8s/conversion/workload_endpoint_default.go
+++ b/libcalico-go/lib/backend/k8s/conversion/workload_endpoint_default.go
@@ -190,34 +190,10 @@ func (wc defaultWorkloadEndpointConverter) podToDefaultWorkloadEndpoint(pod *kap
 	// Map any named ports through.
 	var endpointPorts []libapiv3.WorkloadEndpointPort
 	for _, container := range pod.Spec.Containers {
-		for _, containerPort := range container.Ports {
-			if containerPort.ContainerPort != 0 && (containerPort.HostPort != 0 || containerPort.Name != "") {
-				var modelProto numorstring.Protocol
-				switch containerPort.Protocol {
-				case kapiv1.ProtocolUDP:
-					modelProto = numorstring.ProtocolFromString("udp")
-				case kapiv1.ProtocolSCTP:
-					modelProto = numorstring.ProtocolFromString("sctp")
-				case kapiv1.ProtocolTCP, kapiv1.Protocol("") /* K8s default is TCP. */ :
-					modelProto = numorstring.ProtocolFromString("tcp")
-				default:
-					log.WithFields(log.Fields{
-						"protocol": containerPort.Protocol,
-						"pod":      pod,
-						"port":     containerPort,
-					}).Debug("Ignoring named port with unknown protocol")
-					continue
-				}
-
-				endpointPorts = append(endpointPorts, libapiv3.WorkloadEndpointPort{
-					Name:     containerPort.Name,
-					Protocol: modelProto,
-					Port:     uint16(containerPort.ContainerPort),
-					HostPort: uint16(containerPort.HostPort),
-					HostIP:   containerPort.HostIP,
-				})
-			}
-		}
+		addEndpointPorts(pod, container, &endpointPorts)
+	}
+	for _, container := range pod.Spec.InitContainers {
+		addEndpointPorts(pod, container, &endpointPorts)
 	}
 
 	// Get the container ID if present.  This is used in the CNI plugin to distinguish different pods that have
@@ -269,6 +245,37 @@ func (wc defaultWorkloadEndpointConverter) podToDefaultWorkloadEndpoint(pod *kap
 	return &kvp, nil
 }
 
+func addEndpointPorts(pod *kapiv1.Pod, container kapiv1.Container, endpointPorts *[]libapiv3.WorkloadEndpointPort) {
+	for _, containerPort := range container.Ports {
+		if containerPort.ContainerPort != 0 && (containerPort.HostPort != 0 || containerPort.Name != "") {
+			var modelProto numorstring.Protocol
+			switch containerPort.Protocol {
+			case kapiv1.ProtocolUDP:
+				modelProto = numorstring.ProtocolFromString("udp")
+			case kapiv1.ProtocolSCTP:
+				modelProto = numorstring.ProtocolFromString("sctp")
+			case kapiv1.ProtocolTCP, kapiv1.Protocol("") /* K8s default is TCP. */ :
+				modelProto = numorstring.ProtocolFromString("tcp")
+			default:
+				log.WithFields(log.Fields{
+					"protocol": containerPort.Protocol,
+					"pod":      pod,
+					"port":     containerPort,
+				}).Debug("Ignoring named port with unknown protocol")
+				continue
+			}
+
+			*endpointPorts = append(*endpointPorts, libapiv3.WorkloadEndpointPort{
+				Name:     containerPort.Name,
+				Protocol: modelProto,
+				Port:     uint16(containerPort.ContainerPort),
+				HostPort: uint16(containerPort.HostPort),
+				HostIP:   containerPort.HostIP,
+			})
+		}
+	}
+}
+
 // HandleSourceIPSpoofingAnnotation parses the allowedSourcePrefixes annotation if present,
 // and returns the allowed prefixes as a slice of strings.
 func HandleSourceIPSpoofingAnnotation(annot map[string]string) ([]string, error) {

Steps to Reproduce (for bugs)

Create a StatefulSet with an initContainer sidecar that specifies a named port. Create a NetworkPolicy allowing access to the named port. You should observe timeouts trying to reach that port.

Context

Your Environment

Calico v3.27.3
Kubernetes Server Version: v1.29.0
Flatcar Container Linux by Kinvolk 3815.2.0 (Oklo)
Link to your project (optional): https://github.com/utilitywarehouse/system-manifests/blob/master/kyverno/policies/pods/injectSidecar.yaml#L40-L88

The text was updated successfully, but these errors were encountered:

mauri870 · 2024-06-05T12:36:29Z

Thanks for opening the issue, I'll give it a try (first time contributor) and see if I can get it implemented.

fasaxc · 2024-06-05T14:37:50Z

@mauri870 I think @george-angel was also planning to put up a patch, maybe confer with him (he mentioned it on our slack channel)

mauri870 · 2024-06-05T16:13:13Z

Thanks, I'll wait till I get a word from him.

george-angel · 2024-06-06T05:14:03Z

Thanks all, PR raised: #8885 .

@fasaxc mentioned we might want to dedup the ports. Tests pass for the following patch:

diff --git a/libcalico-go/lib/backend/k8s/conversion/conversion_test.go b/libcalico-go/lib/backend/k8s/conversion/conversion_test.go
index 2f0d29aca..9dd006656 100644
--- a/libcalico-go/lib/backend/k8s/conversion/conversion_test.go
+++ b/libcalico-go/lib/backend/k8s/conversion/conversion_test.go
@@ -215,6 +215,10 @@ var _ = Describe("Test Pod conversion", func() {
                                        },
                                        {
                                                Ports: []kapiv1.ContainerPort{
+                                                       {
+                                                               Name:          "no-proto-dup",
+                                                               ContainerPort: 1234,
+                                                       },
                                                        {
                                                                Name:          "tcp-proto",
                                                                Protocol:      kapiv1.ProtocolTCP,
@@ -295,6 +299,7 @@ var _ = Describe("Test Pod conversion", func() {
                Expect(wep.Value.(*libapiv3.WorkloadEndpoint).Spec.Ports).To(ConsistOf(
                        // No proto defaults to TCP (as defined in k8s API spec)
                        libapiv3.WorkloadEndpointPort{Name: "no-proto", Port: 1234, Protocol: nsProtoTCP},
+                       libapiv3.WorkloadEndpointPort{Name: "no-proto-dup", Port: 1234, Protocol: nsProtoTCP},
                        // Explicit TCP proto is OK too.
                        libapiv3.WorkloadEndpointPort{Name: "tcp-proto", Port: 1024, Protocol: nsProtoTCP},
                        // Host port should be parsed

So its currently possible to define duplicate ports in containers alone and that gets successfully converted to WorkloadEndpoint. Further guidance would be welcome.

fasaxc · 2024-06-07T13:27:45Z

I was mainly wondering if you could have two named ports with the same name. For example, one metrics port on your init container and one on the main container. I'd expect the k8s API server to validate against that, even though you could create a local struct in a test.

No need to address that under a PR to deal with initcontainers though. It may not even be an issue.

george-angel mentioned this issue Jun 5, 2024

Named ports in initContainer sidecars do not work with NetworkPolicies kubernetes/kubernetes#125285

Open

george-angel linked a pull request Jun 6, 2024 that will close this issue

Add initContainer ports to WorkloadEndpoint #8885

Open

3 tasks

matthewdupre added the kind/bug label Jun 18, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Named ports in initContainer sidecars do not work with NetworkPolicies #8881

Named ports in initContainer sidecars do not work with NetworkPolicies #8881

george-angel commented Jun 5, 2024 •

edited

Loading

mauri870 commented Jun 5, 2024

fasaxc commented Jun 5, 2024

mauri870 commented Jun 5, 2024

george-angel commented Jun 6, 2024

fasaxc commented Jun 7, 2024

Named ports in initContainer sidecars do not work with NetworkPolicies #8881

Named ports in initContainer sidecars do not work with NetworkPolicies #8881

Comments

george-angel commented Jun 5, 2024 • edited Loading

Expected Behavior

Current Behavior

Possible Solution

Steps to Reproduce (for bugs)

Context

Your Environment

mauri870 commented Jun 5, 2024

fasaxc commented Jun 5, 2024

mauri870 commented Jun 5, 2024

george-angel commented Jun 6, 2024

fasaxc commented Jun 7, 2024

george-angel commented Jun 5, 2024 •

edited

Loading