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

Patching v3 #21

Merged
merged 22 commits into from
Mar 16, 2023
Merged

Patching v3 #21

merged 22 commits into from
Mar 16, 2023

Conversation

rwaffen
Copy link
Sponsor Member

@rwaffen rwaffen commented Mar 9, 2023

  • include firewall class for dependency packages
  • add ubuntu 22.04 and module dependencies to metadata
  • move kubectl and kubeadm to generic install classes
  • add firewall also for non firewalld systems
  • add kubeadm tool to controller
  • update class documentation
  • add module dependency for augeasproviders_core
  • add dependencies between sysctl and kmod
  • require file kubectl before using it
  • allow to set advertise_address

Copy link
Member

@ananace ananace left a comment

Choose a reason for hiding this comment

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

This would break literally every single one of the clusters that I currently manage, firewalld has absolutely nothing to do with the OS family.

If you need to have non-firewalld rules, then it'd be better to turn manage_firewall into some kind of other datatype - maybe Variant[Boolean, Enum['firewalld','iptables']] where iptable rules are created if it's set to iptables

manifests/repo.pp Outdated Show resolved Hide resolved
manifests/server.pp Outdated Show resolved Hide resolved
@rwaffen
Copy link
Sponsor Member Author

rwaffen commented Mar 10, 2023

This would break literally every single one of the clusters that I currently manage, firewalld has absolutely nothing to do with the OS family.

oh okay. isn't firewalld only a RedHat thing? the module the resources are from supports only RedHat systems. And i never saw firewalld on a ubuntu, does this work?

@ananace
Copy link
Member

ananace commented Mar 10, 2023

firewalld may only be the default firewall manager for RHEL 7 and derivates - as well as some other assorted smaller distributions, but it works on almost all other Linux distros as well. Including Debian and Ubuntu.
The firewalld module itself definitely works on non RedHat as well, I'm personally involved with systems that are using it on Ubuntu, Debian, Arch, Gentoo, and several RHEL derivates.

Perhaps the manage_firewall option could do some checks on facts (like firewalld_version) to decide on which actual implementation to use if it's set to true - alongside the enum to force a particular backend.

Copy link
Member

@tuxmea tuxmea left a comment

Choose a reason for hiding this comment

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

Some questions.

manifests/node/kubelet.pp Outdated Show resolved Hide resolved
manifests/node/kubelet.pp Show resolved Hide resolved
manifests/repo.pp Show resolved Hide resolved
manifests/server/apiserver.pp Show resolved Hide resolved
manifests/server/apiserver.pp Outdated Show resolved Hide resolved
manifests/server/etcd.pp Outdated Show resolved Hide resolved
manifests/server/etcd.pp Show resolved Hide resolved
@rwaffen
Copy link
Sponsor Member Author

rwaffen commented Mar 13, 2023

for all the firewall questions from @tuxmea I myself have no clear answer. @ananace do you have a opinion on that? How/if we should restrict these firewall settings?

@rwaffen rwaffen requested a review from ananace March 13, 2023 09:56
manifests/node.pp Outdated Show resolved Hide resolved
manifests/node/kubelet.pp Outdated Show resolved Hide resolved
manifests/server/apiserver.pp Outdated Show resolved Hide resolved
manifests/server/etcd.pp Outdated Show resolved Hide resolved
@ananace
Copy link
Member

ananace commented Mar 13, 2023

My thought on the firewall questions; If the user doesn't want to provide their own firewall, then having the ports be open on the public zone makes sense for at least firewalld, Kubernetes and etcd both include their own certificate authentication, so the attack surface is limited. And the user is unlikely to have populated any other zones if they're using the "I just want it to work" option that's been provided, and it's impossible to guarantee that RFC1918 or IPv6 ULA would actually encompass their network (I know it wouldn't for me).

It's probably best to note that the provided firewall is simple and likely not safe against attacks though.

@tuxmea
Copy link
Member

tuxmea commented Mar 15, 2023

Question: should we add firewall and firewalld module as dependency to metadata.json?

@rwaffen
Copy link
Sponsor Member Author

rwaffen commented Mar 15, 2023

it is already in here. see the changed files/metadata.json.

@tuxmea
Copy link
Member

tuxmea commented Mar 15, 2023

it is already in here. see the changed files/metadata.json.

OK. Already reviewed and unchanged, so the file did not show up. Sorry.

@rwaffen rwaffen requested a review from ananace March 15, 2023 09:48
Copy link
Member

@ananace ananace left a comment

Choose a reason for hiding this comment

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

Yep, this all looks good to me now, excepting some minor nitpicks in the documentation language - which is still leagues better than it not existing at all.

@rwaffen rwaffen merged commit 84cea8d into voxpupuli:master Mar 16, 2023
@rwaffen rwaffen deleted the patching branch March 16, 2023 09:13
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.

None yet

4 participants