Skip to content

Add firewaldistance to InstallerConfig #145

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

Open
Honigeintopf wants to merge 2 commits into
base: master
Choose a base branch
from
+18 −15
Open

Add firewaldistance to InstallerConfig #145

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0826d35
Add firewaldistance to InstallerConfig
Honigeintopf Nov 6, 2024
c5ba4bd
Merge branch 'master' into add-firewalldistance-to-installerconfig
Honigeintopf May 19, 2025
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
Copy Markdown
Contributor

@Gerrit91 Gerrit91 Nov 6, 2024

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
Copy Markdown
Author

@Honigeintopf Honigeintopf Nov 6, 2024

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
Copy Markdown
Author

@Honigeintopf Honigeintopf Nov 6, 2024

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
Copy Markdown
Author

@Honigeintopf Honigeintopf Nov 6, 2024

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
Copy Markdown
Contributor

@Gerrit91 Gerrit91 Nov 6, 2024

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
Copy Markdown
Contributor

@mwindower mwindower Nov 7, 2024

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