-
Notifications
You must be signed in to change notification settings - Fork 116
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
ufw addon #228
base: master
Are you sure you want to change the base?
ufw addon #228
Changes from all commits
573a284
fbf71f8
9596a4a
a114ba4
3691c7f
0e684ca
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,86 @@ | ||
package addons | ||
|
||
import ( | ||
"fmt" | ||
|
||
"github.com/xetys/hetzner-kube/pkg/clustermanager" | ||
) | ||
|
||
//UfwAddon installs ufw | ||
type UfwAddon struct { | ||
communicator clustermanager.NodeCommunicator | ||
nodeCidr string | ||
nodes []clustermanager.Node | ||
} | ||
|
||
//NewUfwAddon installs ufw to the cluster | ||
func NewUfwAddon(provider clustermanager.ClusterProvider, communicator clustermanager.NodeCommunicator) ClusterAddon { | ||
return UfwAddon{communicator: communicator, nodeCidr: provider.GetNodeCidr(), nodes: provider.GetAllNodes()} | ||
} | ||
|
||
func init() { | ||
addAddon(NewUfwAddon) | ||
} | ||
|
||
//Name returns the addons name | ||
func (addon UfwAddon) Name() string { | ||
return "ufw" | ||
} | ||
|
||
//Requires returns a slice with the name of required addons | ||
func (addon UfwAddon) Requires() []string { | ||
return []string{} | ||
} | ||
|
||
//Description returns the addons description | ||
func (addon UfwAddon) Description() string { | ||
return "Uncomplicated Firewall" | ||
} | ||
|
||
//URL returns the URL of the addons underlying project | ||
func (addon UfwAddon) URL() string { | ||
return "https://wiki.ubuntu.com/UncomplicatedFirewall" | ||
} | ||
|
||
//Install performs all steps to install the addon | ||
func (addon UfwAddon) Install(args ...string) { | ||
|
||
var nodeIPRules string | ||
for _, node := range addon.nodes { | ||
nodeIPRules += " && ufw allow in from " + node.IPAddress + " to any" | ||
} | ||
var output string | ||
for _, node := range addon.nodes { | ||
_, err := addon.communicator.RunCmd(node, "apt-get install -y ufw") | ||
FatalOnError(err) | ||
|
||
fmt.Println("ufw installed on " + node.Name) | ||
|
||
_, err = addon.communicator.RunCmd( | ||
node, | ||
"ufw --force reset"+ | ||
nodeIPRules+ | ||
" && ufw allow ssh"+ | ||
" && ufw allow in from "+addon.nodeCidr+" to any"+ // Kubernetes VPN overlay interface | ||
" && ufw allow in from 10.244.0.0/16 to any"+ // Kubernetes pod overlay interface | ||
" && ufw allow 80"+ | ||
" && ufw allow 443"+ | ||
" && ufw default deny incoming"+ | ||
" && ufw --force enable") | ||
FatalOnError(err) | ||
|
||
output, err = addon.communicator.RunCmd(node, "ufw status verbose") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Output is overridden on each iteration, maybe we should print the output (moving line 79 into foreach) or collect it here and then print? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am not sure if we want to print the output for each server, especially when it is fairly the same output. Let me know your thoughts? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Actually we are going to execute a command than report the result of the command execution to user as "changes applied to all nodes". IMHO this is not correct because (as reported above) we can have different rules based on node type, and should be "correct" to explain the change we add to each node or just report the info on expected reported situation to the user (that do not involve UFW execution report on a single node but the rule we are going to apply). Let's suppose the last node of the iteration already have some rule define (maybe manually?) that are not configured on all nodes, a user can suppose that the result (already existing rule + rule from addon) are configured on all nodes but this is not true. Maybe is an edge case and we prefer to ignore this situation? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The ideal situation would be to allow the user to override the rules or append new rules to the basic ones. You would also have to distinguish each rule based on the instance type. Which would make this addon much more complex than it is meant to be IMHO. |
||
FatalOnError(err) | ||
} | ||
|
||
fmt.Println("ufw enabled with the following rules:\n", output) | ||
} | ||
|
||
//Uninstall performs all steps to remove the addon | ||
func (addon UfwAddon) Uninstall() { | ||
for _, node := range addon.nodes { | ||
_, err := addon.communicator.RunCmd(node, "ufw --force reset && ufw --force disable") | ||
FatalOnError(err) | ||
} | ||
fmt.Println("ufw uninstalled") | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We apply the same rule for each node, but I think each kind of node should have some specific rules, eg: etcd/master node don't need to expose 80/443 (that I suppose should be exposed on workers); maybe I'm missing some part of the setup can you better explain me what we like to achieve with this addon?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So I think the ufw rules are the trickiest part of this addon. I tried to use the most common rules any new cluster would require. But you can always change these rules via script runner from #229 for ex.