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.

Sign up for GitHub

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

Add firewaldistance to InstallerConfig #145

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Add firewaldistance to InstallerConfig #145

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 14 additions & 13 deletions cmd/install.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -220,19 +220,20 @@ func (h *hammer) writeInstallerConfig(machine *models.V1MachineResponse, rootUUi
}

y := &api.InstallerConfig{
Hostname: *alloc.Hostname,
SSHPublicKey: sshPubkeys,
Networks: alloc.Networks,
MachineUUID: h.spec.MachineUUID,
Password: h.spec.ConsolePassword,
Console: console,
Timestamp: time.Now().Format(time.RFC3339),
Nics: h.onlyNicsWithNeighbors(machine.Hardware.Nics),
VPN: alloc.Vpn,
Role: *alloc.Role,
RaidEnabled: raidEnabled,
RootUUID: rootUUiD,
FirewallRules: alloc.FirewallRules,
Hostname: *alloc.Hostname,
SSHPublicKey: sshPubkeys,
Networks: alloc.Networks,
MachineUUID: h.spec.MachineUUID,
Password: h.spec.ConsolePassword,
Console: console,
Timestamp: time.Now().Format(time.RFC3339),
Nics: h.onlyNicsWithNeighbors(machine.Hardware.Nics),
VPN: alloc.Vpn,
Role: *alloc.Role,
RaidEnabled: raidEnabled,
RootUUID: rootUUiD,
FirewallRules: alloc.FirewallRules,
FirewallDistance: 0,
Copy link
Contributor

Choose a reason for hiding this comment

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

In general I am not sure if this is required in the metal-hammer at all, but if we want to do it here, too, then the distance should be high because only then newly created firewalls do not attract traffic immediately.

Copy link
Author

Choose a reason for hiding this comment

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

This was the idea behind it and it works pretty good in the code.
metal-stack/metal-networker#98 (comment)

Copy link
Author

Choose a reason for hiding this comment

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

https://github.com/metal-stack/firewall-controller/blob/d23ded771bcf0e8a96d2699e571ec8cf66c05d76/pkg/network/network.go#L73

Copy link
Author

Choose a reason for hiding this comment

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

A High distance like 10 should be enough?

Copy link
Contributor

Choose a reason for hiding this comment

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

Highest value used by the FCM is 8: https://github.com/metal-stack/firewall-controller-manager/blob/91feb1f750de70a2008b81f3bf0dda41aa4be13f/api/v2/types_firewallset.go#L15

Copy link
Contributor

Choose a reason for hiding this comment

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

In general I am not sure if this is required in the metal-hammer at all, but if we want to do it here, too, then the distance should be high because only then newly created firewalls do not attract traffic immediately.

It's good to have that already in the metal-hammer. Otherwise if we roll firewalls and the new one starts up it has the smallest distance until the firewall-controller starts. This might disturb outgoing traffic flows.

If firewalls start with a high distance they attract no traffic and the firewall-controller-manager decides to configure a lower distance once the new one is configured and the old firewall is destroyed.

}

yamlContent, err := yaml.Marshal(y)
Expand Down
2 changes: 2 additions & 0 deletions pkg/api/api.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,8 @@ type InstallerConfig struct {
RootUUID string `yaml:"root_uuid"`
// FirewallRules if not empty firewall rules to enforce
FirewallRules *models.V1FirewallRules `yaml:"firewall_rules"`
// FirewallDistance is used by the firewall-controller only not by machines
FirewallDistance uint8 `yaml:"firewall_distance"`
}

// FIXME legacy structs remove once old images are gone
Expand Down
Loading