Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make VPN Network configurable #64

Closed

Conversation

SimonKienzler
Copy link
Contributor

What this PR does / why we need it:

In gardener/gardener#8987 we proposed to make the VPN network range configurable. Regardless of how exactly this feature is implemented on the Gardener side (e.g. per Shoot or per Seed), the requirements for the vpn2 components stay the same: They need to be able to handle an additional environment variable called VPN_NETWORK and produce a correct VPN config from its value.

This PR introduces some changes to both the seed-server and the shoot-client components to fulfill these requirements.

Which issue(s) this PR fixes:

Part of gardener/gardener#8987

Special notes for your reviewer:

  • When the VPN_NETWORK environment variable is not set, the implementation should behave exactly the same as before, ensuring backwards-compatibility.
  • A previously implicit requirement (IPv4 networks always have a size of /24) is now explicitly tested, and the script fails if the network has a different size.
  • I don't know how or where the shoot-client/path-controller.sh is used, but assumed that it needs to be adapted just as the network-connection.sh scripts.
  • From my understanding, the go parts of vpn2 don't need to be altered for this change. Please advise if I missed something there.

Release note:

The VPN components now support configuring a custom VPN network using the VPN_NETWORK environment variable.

@gardener-robot
Copy link

@SimonKienzler Thank you for your contribution.

@gardener-robot gardener-robot added needs/review Needs review size/m Size of pull request is medium (see gardener-robot robot/bots/size.py) labels Dec 22, 2023
@gardener-robot-ci-3
Copy link
Contributor

Thank you @SimonKienzler for your contribution. Before I can start building your PR, a member of the organization must set the required label(s) {'reviewed/ok-to-test'}. Once started, you can check the build status in the PR checks section below.

@axel7born
Copy link
Collaborator

/ok-to-test

@gardener-robot gardener-robot added the needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Dec 22, 2023
@axel7born axel7born added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Dec 22, 2023
@gardener-robot-ci-3 gardener-robot-ci-3 removed the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Dec 22, 2023
@gardener-robot
Copy link

@DockToFuture, @marwinski, @ScheererJ, @axel7born You have pull request review open invite, please check

@axel7born
Copy link
Collaborator

Hi @SimonKienzler,
thank's for this PR!
Did you test the HA case?
I just tried it and there was no working VPN connection.
I tested without your g/g change so that it should use the default range.
I didn't try to debug the problem yet.

@SimonKienzler
Copy link
Contributor Author

Thanks for looking into it @axel7born - I'll investigate the HA problems as soon as I can, maybe I missed something.

@SimonKienzler
Copy link
Contributor Author

Hey @axel7born, we had some priority shifts for our next sprint and I won't be able to spend time working on this feature in the next two weeks. Just wanted to let you know, sorry for any inconvenience. If you find time to debug the troubling HA setup, I'd be happy about any pointers you can provide. Otherwise, we'll pick up the investigation and continue this feature again in a few weeks.

@timebertt
Copy link
Member

@SimonKienzler what's the status of this PR? Should we hand this one over to a colleague?

@timebertt
Copy link
Member

/assign

@timebertt
Copy link
Member

I'm unable to edit the PR description.
Hence, I will open a new PR to simplify things.
/close

@gardener-robot
Copy link

@timebertt Command /close is not available to you but only to a Maintainer, Member, Author, Owner.

@timebertt timebertt mentioned this pull request Apr 5, 2024
8 tasks
@timebertt
Copy link
Member

@axel7born can you close this PR in favor of #78 and review the new changes there?

@axel7born
Copy link
Collaborator

/close this PR in favor of #78

@gardener-robot gardener-robot added the status/closed Issue is closed (either delivered or triaged) label Apr 5, 2024
@timebertt timebertt deleted the configurable-vpn-network branch April 5, 2024 10:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) needs/review Needs review size/m Size of pull request is medium (see gardener-robot robot/bots/size.py) status/closed Issue is closed (either delivered or triaged)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants