#364: Detect and reject configurations of service IP range overrlaps with node IPs

[tyiying]

issue:
Probably due to inadequate documentation, people think that they can define service IPs that overlap with node IPs. This breaks things in multiple ways (ARP poisoning in L2 mode, wrong kube-proxy rules executed in all modes...), and is not at all a supported mode of operation.
The controller should detect and reject such configurations, as well as refuse to allocate IPs if they overlap with node IPs.

Fixed #364: Detect and reject configurations where service IP range overlaps with node IPs
Is this a BUG FIX or a FEATURE ?:

Uncomment only one, leave it on its own line:

/kind bug

/kind cleanup
/kind feature
/kind design
/kind flake
/kind failing
/kind documentation
/kind regression

What this PR does / why we need it:
Probably due to inadequate documentation, people think that they can define service IPs that overlap with node IPs. This breaks things in multiple ways (ARP poisoning in L2 mode, wrong kube-proxy rules executed in all modes...), and is not at all a supported mode of operation.
This PR does two things:

1. Add a webhook validation to check IPAddressPool IP range, if it is overlap with node IPs, reject the configuration of IPAddressPool
2. Add check for service configuration when user defines loadbalancerIP via spec.loadBalancerIP or metallb.universe.tf/loadBalancerIPs annotation. If there's any conflict with node IPs, service will stay in pending state.

Special notes for your reviewer:

• Webhook validation rejection for IPAddressPool has explicit error message so it is easy for user to adjust the configuration. Also webhook validation on IPAddressPool does the rejection before automatic IP allocation take place at service configuration stage.
• There is no webhook validation for service configuration, leaving service in pending state is intuitive, since user can only know what went wrong by checking the log of the controller. So I tried to limit the checking of service configuration to only user configured loadBlancerIP annotation fields.
• This PR doesn't cover the scenario of nodeIP changes, i.e. via scale-in/scale-out nodes. But I would expect that after initial prevention of overlapping nodeIPs with loadbalancer IPs subnets. The chance that new node IPs overlapping with existing loadbalancer IP subnets should be very small.

Release note:
Might need to warn user not to use the nodeIPs subnet for loadbalance IPs.

[oribon]

thanks a lot for submitting this!
can you please add relevant unit and e2e tests?
also, I still need to think about it, but maybe driving the validation from the pool controller where we don't let pools that contain node ips enter the internal config rather than introducing c.nodes is better (for involving less pieces). wdyt? cc @fedepaol

[oribon]

hi @tyiying, can you please add unit+e2e tests? regardless of the approach we'll take with the implementation these will be useful for the next reviews :)