-
Notifications
You must be signed in to change notification settings - Fork 1.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Named ports in initContainer sidecars do not work with NetworkPolicies #8881
Comments
Thanks for opening the issue, I'll give it a try (first time contributor) and see if I can get it implemented. |
@mauri870 I think @george-angel was also planning to put up a patch, maybe confer with him (he mentioned it on our slack channel) |
Thanks, I'll wait till I get a word from him. |
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 |
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. |
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:
And member 2 has a new initContainer style sidecar:
They all expose a named port
metrics
.We also have a NetworkPolicy in this namespace such that:
Testing the connection from Prometheus I observe the following results for member 0 and 1:
And a timeout for member 2 (running initContainer sidecar):
If I apply the following patch:
Then I'm able to establish a connection with cockroachdb-2:
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:
So perhaps something like this
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
The text was updated successfully, but these errors were encountered: