Skip to content

Commit

Permalink
fix: validate bond slaves addressing
Browse files Browse the repository at this point in the history 
This extends network device machine configuration validation to make
sure that bond slaves don't have any addressing methods set, as this
might run into a conflict with the bond setup.

Also makes sure no interface is part of two bonds.

Signed-off-by: Andrey Smirnov <smirnov.andrey@gmail.com>
  • Loading branch information
@smira @talos-bot
smira authored and talos-bot committed Jul 7, 2021
1 parent 10c2875 commit 011e288
Show file tree
Hide file tree
Showing 2 changed files with 124 additions and 7 deletions.
36 changes: 29 additions & 7 deletions pkg/machinery/config/types/v1alpha1/v1alpha1_validation.go
View file
Expand Up @@ -56,7 +56,7 @@ var (
)

// NetworkDeviceCheck defines the function type for checks.
type NetworkDeviceCheck func(*Device) error
type NetworkDeviceCheck func(*Device, map[string]string) error

// Validate implements the config.Provider interface.
//nolint:gocyclo,cyclop
Expand Down Expand Up @@ -125,8 +125,22 @@ func (c *Config) Validate(mode config.RuntimeMode, options ...config.ValidationO
}

if c.MachineConfig.MachineNetwork != nil {
bondedInterfaces := map[string]string{}

for _, device := range c.MachineConfig.MachineNetwork.NetworkInterfaces {
if err := ValidateNetworkDevices(device, CheckDeviceInterface, CheckDeviceAddressing, CheckDeviceRoutes); err != nil {
if device.Bond() != nil {
for _, iface := range device.Bond().Interfaces() {
if otherIface, exists := bondedInterfaces[iface]; exists && otherIface != device.Interface() {
result = multierror.Append(result, fmt.Errorf("interface %q is declared as part of two bonds: %q and %q", iface, otherIface, device.Interface()))
}

bondedInterfaces[iface] = device.Interface()
}
}
}

for _, device := range c.MachineConfig.MachineNetwork.NetworkInterfaces {
if err := ValidateNetworkDevices(device, bondedInterfaces, CheckDeviceInterface, CheckDeviceAddressing, CheckDeviceRoutes); err != nil {
result = multierror.Append(result, err)
}
}
Expand Down Expand Up @@ -281,7 +295,7 @@ func (manifests ClusterInlineManifests) Validate() error {

// ValidateNetworkDevices runs the specified validation checks specific to the
// network devices.
func ValidateNetworkDevices(d *Device, checks ...NetworkDeviceCheck) error {
func ValidateNetworkDevices(d *Device, bondedInterfaces map[string]string, checks ...NetworkDeviceCheck) error {
var result *multierror.Error

if d == nil {
Expand All @@ -293,14 +307,14 @@ func ValidateNetworkDevices(d *Device, checks ...NetworkDeviceCheck) error {
}

for _, check := range checks {
result = multierror.Append(result, check(d))
result = multierror.Append(result, check(d, bondedInterfaces))
}

return result.ErrorOrNil()
}

// CheckDeviceInterface ensures that the interface has been specified.
func CheckDeviceInterface(d *Device) error {
func CheckDeviceInterface(d *Device, bondedInterfaces map[string]string) error {
var result *multierror.Error

if d == nil {
Expand Down Expand Up @@ -471,13 +485,21 @@ func checkWireguard(b *DeviceWireguardConfig) error {

// CheckDeviceAddressing ensures that an appropriate addressing method.
// has been specified.
func CheckDeviceAddressing(d *Device) error {
//
//nolint:gocyclo
func CheckDeviceAddressing(d *Device, bondedInterfaces map[string]string) error {
var result *multierror.Error

if d == nil {
return fmt.Errorf("empty device")
}

if _, bonded := bondedInterfaces[d.Interface()]; bonded {
if d.DeviceDHCP || d.DeviceCIDR != "" || d.DeviceVIPConfig != nil {
result = multierror.Append(result, fmt.Errorf("[%s] %q: %s", "networking.os.device", d.DeviceInterface, "bonded interface shouldn't have any addressing methods configured"))
}
}

// Test for both dhcp and cidr specified
if d.DeviceDHCP && d.DeviceCIDR != "" {
result = multierror.Append(result, fmt.Errorf("[%s] %q: %w", "networking.os.device", d.DeviceInterface, ErrBadAddressing))
Expand All @@ -501,7 +523,7 @@ func CheckDeviceAddressing(d *Device) error {
}

// CheckDeviceRoutes ensures that the specified routes are valid.
func CheckDeviceRoutes(d *Device) error {
func CheckDeviceRoutes(d *Device, bondedInterfaces map[string]string) error {
var result *multierror.Error

if d == nil {
Expand Down
95 changes: 95 additions & 0 deletions pkg/machinery/config/types/v1alpha1/v1alpha1_validation_test.go
View file
Expand Up @@ -313,6 +313,10 @@ func TestValidate(t *testing.T) {
BondUpDelay: 100,
BondPacketsPerSlave: 8,
BondADActorSysPrio: 48,
BondInterfaces: []string{
"eth0",
"eth1",
},
},
},
},
Expand All @@ -328,6 +332,97 @@ func TestValidate(t *testing.T) {
},
expectedError: "3 errors occurred:\n\t* invalid bond type roundrobin\n\t* bond.upDelay can't be set if miiMon is zero\n\t* bond.adActorSysPrio is only available in 802.3ad mode\n\n",
},
{
name: "BondDoubleBond",
config: &v1alpha1.Config{
ConfigVersion: "v1alpha1",
MachineConfig: &v1alpha1.MachineConfig{
MachineType: "controlplane",
MachineNetwork: &v1alpha1.NetworkConfig{
NetworkInterfaces: []*v1alpha1.Device{
{
DeviceInterface: "bond0",
DeviceBond: &v1alpha1.Bond{
BondInterfaces: []string{
"eth0",
"eth1",
},
},
},
{
DeviceInterface: "bond1",
DeviceBond: &v1alpha1.Bond{
BondInterfaces: []string{
"eth1",
"eth2",
},
},
},
},
},
},
ClusterConfig: &v1alpha1.ClusterConfig{
ControlPlane: &v1alpha1.ControlPlaneConfig{
Endpoint: &v1alpha1.Endpoint{
endpointURL,
},
},
},
},
expectedError: "1 error occurred:\n\t* interface \"eth1\" is declared as part of two bonds: \"bond0\" and \"bond1\"\n\n",
},
{
name: "BondSlaveAddressing",
config: &v1alpha1.Config{
ConfigVersion: "v1alpha1",
MachineConfig: &v1alpha1.MachineConfig{
MachineType: "controlplane",
MachineNetwork: &v1alpha1.NetworkConfig{
NetworkInterfaces: []*v1alpha1.Device{
{
DeviceInterface: "bond0",
DeviceBond: &v1alpha1.Bond{
BondInterfaces: []string{
"eth0",
"eth1",
},
},
},
{
DeviceInterface: "bond0",
DeviceBond: &v1alpha1.Bond{
BondInterfaces: []string{
"eth0",
"eth1",
},
},
},
{
DeviceInterface: "eth0",
DeviceDHCP: true,
},
{
DeviceInterface: "eth1",
DeviceCIDR: "192.168.0.1/24",
},
{
DeviceInterface: "eth2",
DeviceCIDR: "192.168.1.1/24",
},
},
},
},
ClusterConfig: &v1alpha1.ClusterConfig{
ControlPlane: &v1alpha1.ControlPlaneConfig{
Endpoint: &v1alpha1.Endpoint{
endpointURL,
},
},
},
},
expectedError: "2 errors occurred:\n\t* [networking.os.device] \"eth0\": bonded interface shouldn't have any addressing methods configured\n" +
"\t* [networking.os.device] \"eth1\": bonded interface shouldn't have any addressing methods configured\n\n",
},
{
name: "Wireguard",
config: &v1alpha1.Config{
Expand Down

0 comments on commit 011e288

Please sign in to comment.