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.
It used to be when we were using iproute2's CLI we needed to have the
fwmark as a hex number so we were passing it as a string in that format.
However, now that we use the netlink library directly, we already have
the fwmark in the condition that we need it. So instead of doing all of
these string <-> int conversions, lets just keep this simpler.
When ip rules are evaluated in the netlink library, default routes for
src and dst are equated to nil. This makes it difficult to evaluate
them and requires additional handling in order for them.
I filed an issue upstream so that this could potentially get fixed:
https://github.com/vishvananda/netlink/issues/1080 however if it doesn't
get resolved, this should allow us to move forward.
It has proven to be tricky to insert new rules without calling the
designated NewRule() function from the netlink library. Usually attempts
will fail with an operation not supported message.
This improves the reliability of rule insertion.
In order for a local route to be valid it needs to have the scope set to
host. When we were executing ip commands iproute2 just did this for us
to make the command accurate. Now that we're communicating with the
netlink socket, we need to do this conversion for ourselves.
Without this we get an error that says "invalid argument" from the
netlink subsystem. But if the route isn't local, then most of the
routing logic for services doesn't work correctly because it acts upon
external traffic as well as local traffic which isn't correct.
