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

[Feature] Add option to use an existing private network. #1000

Merged
merged 5 commits into from
Oct 11, 2023

Conversation

valkenburg-prevue-ch
Copy link
Contributor

This PR adds two variables for passing an existing private network to the kube-hetzner module. Changes are minimal. Motivation for this is addition of other non-k8s machines in the private network prior to setting up the cluster. One reason is for example the use of a proxy (using HTTP_PROXY and HTTPS_PROXY environment variables), which allows you to reserve a single ip address for outgoing requests of the entire cluster.

kube.tf.example Outdated Show resolved Hide resolved
main.tf Outdated Show resolved Hide resolved
main.tf Outdated Show resolved Hide resolved
Copy link
Collaborator

@mysticaltech mysticaltech left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@valkenburg-prevue-ch This is great, however, please see my comments, probabably not needed to have the boolean variable.

Also, the way we use the private network is very specific, maybe provide guidance or refer to the right part in the readme, at the level where you collect the network id in kube.tf.example, so that people know what kind of ip ranges need to be free when using this feature. And naturally make sure they understand that this is for exceptional use cases, for advanced users only.

variables.tf Outdated Show resolved Hide resolved
@valkenburg-prevue-ch
Copy link
Contributor Author

Also, the way we use the private network is very specific, maybe provide guidance or refer to the right part in the readme, at the level where you collect the network id in kube.tf.example, so that people know what kind of ip ranges need to be free when using this feature. And naturally make sure they understand that this is for exceptional use cases, for advanced users only.

Thanks for reviewing! And yes, I will adapt the PR with more and better guidance in the docs.

Perhaps you can guess where this is going: I've been setting up a proxy server, which with this PR can be part of the private network. It starts to work flawlessly, so all my egress goes through one pre-reserved ipv4 address which is not blacklisted anywhere. I'll probably share that project soon. It's an http-proxy and a dns-proxy. I'm wondering if it makes sense to make it a NAT-router as well, so one can finally create a private-ip only network (for which a use of bastion-host parameters in all the ssh connections is needed, that's a simple PR which I'm however postponing until I find that it makes sense).

…moved. Added documentation for variable existing_network_id in kube.tf and README.md.
@valkenburg-prevue-ch
Copy link
Contributor Author

Ok, found a workaround. Added documentation.

How about setting the default cidr to 10.0.0.0/9 ? There's plenty of room in that range. All ips are already correctly computed using the cidr functions of tf, and the default service and cluster ranges are inside 10.0.0.0/9 already. This would mean that any user at any time can add stuff to the private network in the 10.128.0.0/9 range, without ever having to dig into the cidr settings.

mysticaltech
mysticaltech previously approved these changes Oct 4, 2023
Copy link
Collaborator

@mysticaltech mysticaltech left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@valkenburg-prevue-ch Wonderful job, this is super valuable.

@mysticaltech
Copy link
Collaborator

@valkenburg-prevue-ch LGTM, please just make sure the validate terraform (probably just formatting) passes and we're good to go!

@M4t7e @aleksasiriski I think you will like that one.

kube.tf.example Show resolved Hide resolved
@valkenburg-prevue-ch
Copy link
Contributor Author

Weird, Validate Terraform crashes, and its logs say "this is indicative of a bug in terraform". Not sure what I can do about it.

@valkenburg-prevue-ch
Copy link
Contributor Author

Terraform is broken at the moment. Forgive me the naughtiness, but I retriggered an older validation which had succeeded, and it crashes too:
https://github.com/kube-hetzner/terraform-hcloud-kube-hetzner/actions/runs/6363783912

Must have been a regression in the docker image used for the validation.

@mysticaltech mysticaltech changed the base branch from master to staging October 11, 2023 12:56
@mysticaltech mysticaltech merged commit c4451cd into staging Oct 11, 2023
@mysticaltech mysticaltech deleted the feat-variable-network-id branch October 11, 2023 12:57
@mysticaltech
Copy link
Collaborator

@valkenburg-prevue-ch Please could to push a fix to staging for the following error when the variable is not present. So that it's backward compatible.

Screenshot from 2023-10-11 16-27-09

@valkenburg-prevue-ch
Copy link
Contributor Author

valkenburg-prevue-ch commented Oct 11, 2023 via email

@mysticaltech
Copy link
Collaborator

Will do.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants