This combines five defensive fixes in the Network Services Controller:
1. shuffle(): check rand.Int error before dereferencing result
- rand.Int returns (nil, err) on failure, but the result was
dereferenced before the error check, causing a nil panic
2. NodePort healthcheck: add RWMutex to protect shared maps
- UpdateServicesInfo writes serviceInfoMap/endpointsInfoMap from
the sync goroutine while HTTP handlers read concurrently
3. setupIpvsFirewall: use continue instead of return in dual-stack loop
- return nil after clearing one IP family's chain skipped the
second family entirely on dual-stack nodes
4. setupMangleTableRule/cleanupMangleTableRule: add nil check for ParseIP
- net.ParseIP result was used without nil check, causing panic
on malformed IP strings from service annotations
5. synctypeIpvs: track errors across both sync steps for heartbeat
- err from syncIpvsServices was overwritten by syncHairpinIptablesRules,
masking IPVS failures from the health check system
This implements support for KEP-1860. When a LoadBalancer ingress has ipMode set to 'Proxy', kube-router will skip adding the IP to the local IPVS table and will not hijack the traffic. If ipMode is 'VIP' or unset, the current behavior is maintained.
Fixes#2014
Moves all Service VIP range configurations into pkg/svcip this is where
validation and querying of ranges goes rather than passing each range to
each controller.
It also centralizes the validation logic since NRC and NSC need
basically equivalent logic. It additionally adds a RangeQuerier
interface for the NPC and LBC controllers which require knowing the
literal ranges.
includes workaround for musl hardcoded protocol table that
is missing SCTP support by using protocol name to
numeric value mapping in ipset entries
closes: https://github.com/cloudnativelabs/kube-router/issues/1019
Signed-off-by: Roman Kuzmitskii <roman@damex.org>
Logic errors & regressions relating to traffic policies make up
approximately 8 or so preventable historical issues with the project.
Therefore prioritizing them as a unit testing surface.
The problem here stems from the fact that when netpol generates its list of expected ipsets, it includes the inet6:
prefix, however, when the proxy and routing controller sent their list of expected ipsets, they did not do so. This
meant that no matter how we handled it in ipset.go it was wrong for one or the other use-cases.
I decided to standardize on the netpol way of sending the list of expected ipset names so that BuildIPSetRestore() can
function in the same way for all invocations.
Attempt to filter out sets that we are not authoritative for to avoid
race conditions with other operators (like Istio) that might be
attempting to modify ipsets at the same time.
This was originally added in PR #210, but it appears to cause more
problems in my testing scenarios than it solves. When this is enabled,
it makes it so that services cannot be routed to from kube workers to
DSR enabled services when routed to other nodes in the cluster.
Previously, kube-router was only considering externalIPs when setting up
source routing policy, notably absent was consideration of LoadBalancer
IPs which are equally important for getting right with DSR.
This appears to have been a long-standing use-case that was never
correctly considered since when kube-router added a LoadBalancer
controller.
Instead of deleting and just hoping for the best, this change makes it
so that we check first whether or not a route exists. This helps to
reduce needless warnings that the user receives and is just all around
more accurate.
