kubernikus

kubernikus copied to clipboard

This PR refactors the EnsureKubernikusRuleInSecurityGroup to support reconciling more then one rule. It now also creates the rule lazily only if no other broader rules are already covering the wanted rules.

In addition to the podCIDR rule we had, it now also added egress rules for ntp and the cluster apiserver.

This PR create the following rules lazily if there isn't already a rule allowing the traffic:

egress:

• all, to same security group (pod-to-pod)
• udp port 53 (dns)
• udp port 123 (ntp)
• port 443 keppel.$region.cloud.sap, objectstore-3.$region.cloud.sap, keppel.global.global.cloud.sap, objectstore-3.eu-de-1.cloud.sap (for pulling kube-proxy,kubelet, wormhole, flannel images)

ingress:

• all, from same security group (pod-to-pod)
• port 30000 - 32767, from private network (load balancer to members)

TODO:

• [x] Modify our e2e tests to use a fresh security group without any rules to make sure the e2e tests passes with our enforced rules only.
• [x] Also we should probaply expand our network tests to include host <-> pod tests and also test the loadbalancer datapath. --> Switch to kube-detective for thoroughly testing all combinitions
• [x] Add metric for failing reconcilitions
• [x] remove rule for objectstore.eu-de-1.cloud.sap

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

I haven't checked the code but only your comment but IIRC for UDP you want a port 123 Ingress rule for NTP and port 53 Ingress for DNS as well (because it's stateless so the firewall can't track the state). Also for DNS you might want a TCP port 53 Egress rule as well as DNS now can use both.

I haven't checked the code but only your comment but IIRC for UDP you want a port 123 Ingress rule for NTP and port 53 Ingress for DNS as well (because it's stateless so the firewall can't track the state).

Are you sure that's the case? Because allowing udp egress 53 and 123 definitlty fixes dns issues and time sync issues. So it seems to me nsx-t is doing some "connection tracking" for udp packets as well (like iptables does, e.g. state ESTABLISHED,RELATED)

Also for DNS you might want a TCP port 53 Egress rule as well as DNS now can use both.

That's a valid point

At least for NTP a customer had this issue today (not in a kubernikus environment) so I assume that the ingress rules are needed as well, but I haven't tested it myself. Here is the discussion: https://convergedcloud.slack.com/archives/C374AQJ3W/p1656505078940829