Make VPN Network configurable

[timebertt]

How to categorize this PR?

/area networking
/kind enhancement

What this PR does / why we need it:

This PR makes use of gardener/vpn2#78 to allow configuring the used VPN network on the seed-level via Seed.spec.networks.vpn.
The field is defaulted to the current hard-coded values and non-default network CIDRs must have the same size as the current hard-coded values. Network disjointedness with other seed and shoot networks is ensured in validation, admission, and scheduling.

See the umbrella issue and the doc commit for more details on motivation and conditions.

Which issue(s) this PR fixes:
Fixes #8987

Special notes for your reviewer:
Supersedes #8991

Release note:

Operators can configure the network range of the shoot VPN tunnel using the new `Seed.spec.networks.vpn` field. See [this document](https://github.com/gardener/gardener/blob/master/docs/usage/reversed-vpn-tunnel.md#configurable-vpn-network) for more details.

[timebertt]

TL;DR: I pushed a new commit which should fix the e2e test failures for HA clusters.

---

Explanation:
I figured out why the e2e tests for HA clusters were consistently failing.
The gardenlet logs pointed towards kube-apiserver being unable to talk to metrics-server:

2024-04-16T20:38:52.370616999Z stderr F {"level":"error","ts":"2024-04-16T20:38:52.370Z","msg":"Error","controller":"shoot","object":{"name":"e2e-rotate","namespace":"garden-local"},"namespace":"garden-local","name":"e2e-rotate","reconcileID":"4a09b9c2-0273-4f05-b899-a231bb2821ff","operation":"reconcile","flow":"Shoot cluster reconciliation","task":"Labeling resources after modification of encryption config or to encrypt them with new ETCD encryption key","error":"retry failed with context deadline exceeded, last error: error discovering server preferred resources: unable to retrieve the complete list of server APIs: metrics.k8s.io/v1beta1: the server is currently unable to handle the request","stacktrace":"github.com/gardener/gardener/pkg/utils/flow.(*execution).runNode.func2\n\tgithub.com/gardener/gardener/pkg/utils/flow/flow.go:241"}

I could reproduce this locally:

$ k get apiservice v1beta1.metrics.k8s.io -oyaml
...
status:
  conditions:
  - lastTransitionTime: "2024-04-17T06:28:21Z"
    message: 'failing or missing response from https://10.3.1.18:8443/apis/metrics.k8s.io/v1beta1:
      Get "https://10.3.1.18:8443/apis/metrics.k8s.io/v1beta1": dial tcp 10.3.1.18:8443:
      connect: connection refused'
    reason: FailedDiscoveryCheck
    status: "False"
    type: Available

This was due to missing routes for the shoot networks in the kube-apiserver:

$ k -n shoot--local--e2e-rotate exec -it kube-apiserver-744ccc7b7c-cmcmc -c vpn-path-controller -- sh
~ # ip r
default via 169.254.1.1 dev eth0
10.5.0.0/26 dev tap0 proto kernel scope link src 10.5.0.8
10.5.0.64/26 dev tap1 proto kernel scope link src 10.5.0.72
10.5.0.192/26 dev bond0 proto kernel scope link src 10.5.0.214
169.254.1.1 dev eth0 scope link

$ k -n shoot--local--e2e-rotate logs kube-apiserver-744ccc7b7c-cmcmc vpn-path-controller
[Wed Apr 17 06:43:49 UTC 2024]: 194=err 195=err using
[Wed Apr 17 06:43:53 UTC 2024]: 194=err 195=err using

This in turn was due to the bonding device on the vpn-shoot-client side not using an IP from the configured VPN network but from the default network:

$ ks exec -it vpn-shoot-1 -c vpn-shoot-s1 -- sh
~ # ip a s dev bond0
6: bond0: <BROADCAST,MULTICAST,MASTER,UP,LOWER_UP> mtu 1500 qdisc noqueue state UP group default qlen 1000
    link/ether 56:1d:92:09:22:d6 brd ff:ff:ff:ff:ff:ff
    inet 192.168.123.194/26 scope global bond0
       valid_lft forever preferred_lft forever
    inet6 fe80::541d:92ff:fe09:22d6/64 scope link
       valid_lft forever preferred_lft forever

The bonding device was incorrectly configured because the VPN_NETWORK env var was missing in the vpn-shoot-init container of the vpn-shoot.
This is fixed in the latest commit.

[timebertt]

/cc @axel7born @ScheererJ

[axel7born]

Thank you for your change.
To me it looks good.
I'm not so sure about the vpn vs VPN.

Also, I tested the change with a non-default range in the extension setup with HA VPN without issues.

[timebertt]

Thanks @axel7born for your review. I addressed the suggestions and capitalized VPN consistently across the entire codebase.
PTAL :)

[axel7born]

/lgtm

[timebertt]

/retest

[ScheererJ]

Thanks a lot for the enhancement. I have a few questions/comments.

[ScheererJ]

Overall, the motivation to make the VPN range configurable seems to come from the desire to make as much private IP space as possible available to shoot owners. As the network used here is not routed within the seed/shoot, i.e. there are no packets on the wire with for example 192.168.123.1 as IP addresses, would it not be a better solution to simply switch the VPN to a pure IPv6 tunnel with potentially IPv4 or IPv6 workload to be transferred? Did you consider this approach?

[timebertt]

the motivation to make the VPN range configurable seems to come from the desire to make as much private IP space as possible available to shoot owners

Correct.

As the network used here is not routed within the seed/shoot, i.e. there are no packets on the wire with for example 192.168.123.1 as IP addresses, would it not be a better solution to simply switch the VPN to a pure IPv6 tunnel with potentially IPv4 or IPv6 workload to be transferred?

We didn't consider this approach. Maybe we could have considered it if it had been suggested in the proposal issue 😄

Even if there are no packets on the wire with addresses from the VPN network, I assume that there must not be overlaps between seed/shoot networks and the VPN network. Otherwise, the routes will be ambiguous. Do I get this right?
If so, using an IPv6 VPN network might solve the problem for IPv4 shoots (obviously). Nevertheless, choosing a hard-coded IPv6 network will yield the same "problem" that it imposes additional restrictions (apart from seed networks) on what networks can be chosen for shoots. I quote "problem" because it will be less restrictive in IPv6.
I still think operators would like to put all "restricted" networks (seeds and VPN) into the same IP space, even for IPv6.
Feel free to correct me if my naive thinking doesn't make sense 😄

That being said, I would love to continue with the current approach as it's almost complete. I feel like it doesn't bring too much complexity that it's worth going back to the drawing board and implementing a totally different approach.

[axel7born]

Please continue with this approach!
Johannes is on vacation this week. But we had a conversation on Friday, where he said this idea should not block this PR.

[ScheererJ]

Maybe we could have considered it if it had been suggested in the proposal issue 😄

As you know, feedback can come at any point in time. An idea that does not fit to your timeline is not automatically a bad one.

I disagree with your opinion that with IPv6 it would be a problem. If we would use the hard-coded ULA range as done now with IPv6-only clusters we would only have a very unlikely collision if someone else used the same range. (RFC 4193 requires the global id to be random.)
It would even be possible to reserve a separate global range or use some of the special IPv6 ranges to prevent any chance of collision at all. Therefore, I do believe that you can solve the problem of the scarce IPv4 private address space by using IPv6 for the VPN.
Feel free to convince me otherwise, though.

The overlap concerns between seed and shoot clusters with regards to node, pods and services have not a lot to do with the choice of the VPN range. In other words, the seed/shoot disjointness still needs to be ensured regardless what happens with the VPN.

As the VPN range is not routed in the network, I am not sure if operators are that much interested in it. Of course, if it causes other restrictions it may be relevant, but that is again only relevant in the context of scarce IPv4 space.

Feel free to continue with the approach. As @axel7born already mentioned, I have no hard feelings against this approach, but I do feel that we just use another set of band-aid instead of actually solving the problem.

[timebertt]

An idea that does not fit to your timeline is not automatically a bad one.

Definitely not! I didn't want to say so.

I do feel that we just use another set of band-aid instead of actually solving the problem.

Agreed.

TBH, I do find your suggestion compelling but I lack some knowledge to comprehend it fully.
I would like to discuss the details of your idea. A meeting will probably be difficult this week, so I propose taking this to next week's hackathon :)

In the meantime, I will address the remaining comments in this PR so that we have a finished implementation to go back to if we decide on this approach after all.

[gardener-prow]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please ask for approval from scheererj. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[timebertt]

@axel7born I addressed all suggestions added by @ScheererJ. Can you take another look?

[timebertt]

Rebased and re-generated pkg/apis/core/v1beta1/generated.pb.go to resolve conflicts.

[timebertt]

/hold
We want to discuss the overall approach for solving the motivating problem, see #9597 (comment)

[gardener-ci-robot]

The Gardener project currently lacks enough active contributors to adequately respond to all PRs.
This bot triages PRs according to the following rules:

• After 15d of inactivity, lifecycle/stale is applied
• After 15d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 7d of inactivity since lifecycle/rotten was applied, the PR is closed

You can:

• Mark this PR as fresh with /remove-lifecycle stale
• Mark this PR as rotten with /lifecycle rotten
• Close this PR with /close

/lifecycle stale

[rfranzke]

Shall we close this PR in favor of https://github.com/gardener-community/hackathon/blob/main/2024-05_Schelklingen/README.md#%EF%B8%8F-pure-ipv6-based-vpn-tunnel / gardener/vpn2#83? :)

[timebertt]

Definitely! The pure IPv6-based tunnel is the superior solution to the problem.
/close

[gardener-prow]

@timebertt: Closed this PR.

In response to this:

Definitely! The pure IPv6-based tunnel is the superior solution to the problem.
/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.