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

Network sanity checks #277

Merged
merged 1 commit into from
Apr 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
112 changes: 112 additions & 0 deletions microcloud/cmd/microcloud/main_init.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
"github.com/canonical/lxd/shared"
lxdAPI "github.com/canonical/lxd/shared/api"
"github.com/canonical/lxd/shared/logger"
"github.com/canonical/lxd/shared/validate"
cephTypes "github.com/canonical/microceph/microceph/api/types"
cephClient "github.com/canonical/microceph/microceph/client"
"github.com/canonical/microcluster/client"
Expand Down Expand Up @@ -141,6 +142,11 @@ func (c *cmdInit) RunInteractive(cmd *cobra.Command, args []string) error {
return err
}

err = validateSystems(s, systems)
if err != nil {
return err
}

err = setupCluster(s, systems)
if err != nil {
return err
Expand Down Expand Up @@ -414,6 +420,112 @@ func AddPeers(sh *service.Handler, systems map[string]InitSystem) error {
return nil
}

// validateGatewayNet ensures that the ipv{4,6} gateway in a network's `config`
// is a valid CIDR address and that any ovn.ranges if present fall within the
// gateway. `ipPrefix` is one of "ipv4" or "ipv6".
func validateGatewayNet(config map[string]string, ipPrefix string, cidrValidator func(string) error) (ovnIPRanges []*shared.IPRange, err error) {
gateway, hasGateway := config[ipPrefix+".gateway"]
ovnRanges, hasOVNRanges := config[ipPrefix+".ovn.ranges"]

var gatewayIP net.IP
var gatewayNet *net.IPNet

if hasGateway {
err = cidrValidator(gateway)
if err != nil {
return nil, fmt.Errorf("Invalid %s.gateway %q: %w", ipPrefix, gateway, err)
}

gatewayIP, gatewayNet, err = net.ParseCIDR(gateway)
if err != nil {
return nil, fmt.Errorf("Invalid %s.gateway %q: %w", ipPrefix, gateway, err)
}
}

if hasGateway && hasOVNRanges {
ovnIPRanges, err = shared.ParseIPRanges(ovnRanges, gatewayNet)
if err != nil {
return nil, fmt.Errorf("Invalid %s.ovn.ranges %q: %w", ipPrefix, ovnRanges, err)
}
}

for _, ipRange := range ovnIPRanges {
if ipRange.ContainsIP(gatewayIP) {
return nil, fmt.Errorf("UPLINK %s.ovn.ranges must not include gateway address %q", ipPrefix, gatewayNet.IP)
}
}

return ovnIPRanges, nil
}

func validateSystems(s *service.Handler, systems map[string]InitSystem) (err error) {
curSystem, bootstrap := systems[s.Name]
if !bootstrap {
return nil
}

// Assume that the UPLINK network on each system is the same, so grab just
// the gateways from the current node's UPLINK to verify against the other
// systems' management addrs
var ip4OVNRanges, ip6OVNRanges []*shared.IPRange

for _, network := range curSystem.Networks {
if network.Type == "physical" && network.Name == "UPLINK" {
Copy link
Member

Choose a reason for hiding this comment

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

I'm not enjoying the hardcoded "UPLINK" here, we should be using a constant for "UPLINK" and then ensuring that everywhere that needs to reference the uplink network by name uses this constant - this allows for quick understanding of the uses of that network and updating of it if needed.

I've burying configuration assumptions in lower level functions like this can lead to brittle code in the future if those assumptions change and its forgotten/unknown about where to update.

Alternatively we could have the dependencies of the validateSystems function accept them as arguments, i.e uplinkNetName string and have it passed in by the caller so we aren't tightly coupling things together.

Copy link
Contributor

Choose a reason for hiding this comment

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

@MggMuggins you can use this method to grab the corresponding network type. These methods are how we keep the payloads that we send to LXD consistent.

You can either instantiate a new LXDService or extract the receiver from handler.Services[types.LXD].(*service.LXDService) from the service.Handler that carries through the whole init.

ip4OVNRanges, err = validateGatewayNet(network.Config, "ipv4", validate.IsNetworkAddressCIDRV4)
if err != nil {
return err
}

ip6OVNRanges, err = validateGatewayNet(network.Config, "ipv6", validate.IsNetworkAddressCIDRV6)
if err != nil {
return err
}

nameservers, hasNameservers := network.Config["dns.nameservers"]
if hasNameservers {
isIP := func(s string) error {
ip := net.ParseIP(s)
if ip == nil {
return fmt.Errorf("Invalid IP %q", s)
} else {
return nil
}
}

err = validate.IsListOf(isIP)(nameservers)
if err != nil {
return fmt.Errorf("Invalid dns.nameservers: %w", err)
}
}

break
}
}

// Ensure that no system's management address falls within the OVN ranges
// to prevent OVN from allocating an IP that's already in use.
for systemName, system := range systems {
systemAddr := net.ParseIP(system.ServerInfo.Address)
if systemAddr == nil {
return fmt.Errorf("Invalid address %q for system %q", system.ServerInfo.Address, systemName)
}

for _, ipRange := range ip4OVNRanges {
if ipRange.ContainsIP(systemAddr) {
return fmt.Errorf("UPLINK ipv4.ovn.ranges must not include system address %q", systemAddr)
}
}

for _, ipRange := range ip6OVNRanges {
if ipRange.ContainsIP(systemAddr) {
return fmt.Errorf("UPLINK ipv6.ovn.ranges must not include system address %q", systemAddr)
}
}
}

return nil
}

// setupCluster Bootstraps the cluster if necessary, adds all peers to the cluster, and completes any post cluster
// configuration.
func setupCluster(s *service.Handler, systems map[string]InitSystem) error {
Expand Down
5 changes: 5 additions & 0 deletions microcloud/cmd/microcloud/main_init_preseed.go
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,11 @@ func (c *CmdControl) RunPreseed(cmd *cobra.Command, init bool) error {
}
}

err = validateSystems(s, systems)
if err != nil {
return err
}

return setupCluster(s, systems)
}

Expand Down
224 changes: 224 additions & 0 deletions microcloud/cmd/microcloud/main_init_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,224 @@
package main

import (
"testing"

lxdAPI "github.com/canonical/lxd/shared/api"

"github.com/canonical/microcloud/microcloud/mdns"
"github.com/canonical/microcloud/microcloud/service"
)

func newSystemWithNetworks(address string, networks []lxdAPI.NetworksPost) InitSystem {
return InitSystem{
ServerInfo: mdns.ServerInfo{
Name: "testSystem",
Address: address,
},
Networks: networks,
}
}

func newSystemWithUplinkNetConfig(address string, config map[string]string) InitSystem {
return newSystemWithNetworks(address, []lxdAPI.NetworksPost{{
Name: "UPLINK",
Type: "physical",
NetworkPut: lxdAPI.NetworkPut{
Config: config,
},
}})
}

func newTestHandler(addr string, t *testing.T) *service.Handler {
handler, err := service.NewHandler("testSystem", addr, "/tmp/microcloud_test_hander", true, true)
if err != nil {
t.Fatalf("Failed to create test service handler: %s", err)
}

return handler
}

func newTestSystemsMap(systems ...InitSystem) map[string]InitSystem {
systemsMap := map[string]InitSystem{}

for _, system := range systems {
systemsMap[system.ServerInfo.Name] = system
}

return systemsMap
}

func ensureValidateSystemsPasses(handler *service.Handler, testSystems map[string]InitSystem, t *testing.T) {
for testName, system := range testSystems {
systems := newTestSystemsMap(system)

err := validateSystems(handler, systems)
if err != nil {
t.Fatalf("Valid system %q failed validate: %s", testName, err)
}
}
}

func ensureValidateSystemsFails(handler *service.Handler, testSystems map[string]InitSystem, t *testing.T) {
for testName, system := range testSystems {
systems := newTestSystemsMap(system)

err := validateSystems(handler, systems)
if err == nil {
t.Fatalf("Invalid system %q passed validation", testName)
}
}
}

func TestValidateSystemsIP4(t *testing.T) {
address := "192.168.1.27"
handler := newTestHandler(address, t)

// Each entry in these maps is validated individually
validSystems := map[string]InitSystem{
"plainGateway": newSystemWithUplinkNetConfig(address, map[string]string{
"ipv4.gateway": "10.234.0.1/16",
}),
"dns": newSystemWithUplinkNetConfig(address, map[string]string{
"ipv4.gateway": "10.234.0.1/16",
"dns.nameservers": "1.1.1.1,8.8.8.8",
}),
"16Net": newSystemWithUplinkNetConfig(address, map[string]string{
"ipv4.gateway": "10.42.0.1/16",
"ipv4.ovn.ranges": "10.42.1.1-10.42.5.255",
}),
"24Net": newSystemWithUplinkNetConfig(address, map[string]string{
"ipv4.gateway": "190.168.4.1/24",
"ipv4.ovn.ranges": "190.168.4.50-190.168.4.60",
}),
}

ensureValidateSystemsPasses(handler, validSystems, t)

invalidSystems := map[string]InitSystem{
"invalidNameservers1": newSystemWithUplinkNetConfig(address, map[string]string{
"ipv4.gateway": "10.234.0.1/16",
"dns.nameservers": "8.8",
}),
"invalidNameservers2": newSystemWithUplinkNetConfig(address, map[string]string{
"ipv4.gateway": "10.234.0.1/16",
"dns.nameservers": "8.8.8.128/23",
}),
"gatewayIsSubnetAddr": newSystemWithUplinkNetConfig(address, map[string]string{
"ipv4.gateway": "192.168.28.0/24",
}),
"backwardsRange": newSystemWithUplinkNetConfig(address, map[string]string{
"ipv4.gateway": "10.42.0.1/16",
"ipv4.ovn.ranges": "10.42.5.255-10.42.1.1",
}),
"rangesOutsideGateway": newSystemWithUplinkNetConfig(address, map[string]string{
"ipv4.gateway": "10.1.1.0/24",
"ipv4.ovn.ranges": "10.2.2.50-10.2.2.100",
}),
"rangesContainGateway": newSystemWithUplinkNetConfig(address, map[string]string{
"ipv4.gateway": "192.168.1.1/24",
"ipv4.ovn.ranges": "192.168.1.1-192.168.1.20",
}),
"rangesContainSystem": newSystemWithUplinkNetConfig(address, map[string]string{
"ipv4.gateway": "192.168.1.1/16",
"ipv4.ovn.ranges": "192.168.1.20-192.168.1.30",
}),
}

ensureValidateSystemsFails(handler, invalidSystems, t)
}

func TestValidateSystemsIP6(t *testing.T) {
address := "fc00:feed:beef::bed1"
handler := newTestHandler(address, t)

validSystems := map[string]InitSystem{
"plainGateway": newSystemWithUplinkNetConfig(address, map[string]string{
"ipv6.gateway": "fc00:bad:feed::1/64",
}),
"dns": newSystemWithUplinkNetConfig(address, map[string]string{
"ipv6.gateway": "fc00:feed:f00d::1/64",
"dns.nameservers": "2001:4860:4860::8888,2001:4860:4860::8844",
}),
"dnsMulti": newSystemWithUplinkNetConfig(address, map[string]string{
"ipv6.gateway": "fc00:feed:f00d::1/64",
"dns.nameservers": "2001:4860:4860::8888,1.1.1.1",
}),
"64Net": newSystemWithUplinkNetConfig(address, map[string]string{
"ipv6.gateway": "fc00:bad:feed::1/64",
"ipv6.ovn.ranges": "fc00:bad:feed::f-fc00:bad:feed::fffe",
}),
}

ensureValidateSystemsPasses(handler, validSystems, t)

invalidSystems := map[string]InitSystem{
"invalidNameservers1": newSystemWithUplinkNetConfig(address, map[string]string{
"ipv6.gateway": "fc00:feed:f00d::1/64",
"dns.nameservers": "8:8",
}),
"invalidNameservers2": newSystemWithUplinkNetConfig(address, map[string]string{
"ipv6.gateway": "fc00:feed:f00d::1/64",
"dns.nameservers": "2001:4860:4860::8888/32",
}),
"gatewayIsSubnetAddr": newSystemWithUplinkNetConfig(address, map[string]string{
"ipv6.gateway": "fc00:feed:f00d::0/64",
}),
"rangesOutsideGateway": newSystemWithUplinkNetConfig(address, map[string]string{
"ipv6.gateway": "fc00:feed:f00d::1/64",
"ipv6.ovn.ranges": "fc00:feed:beef::f-fc00:feed:beef::fffe",
}),
"rangesContainGateway": newSystemWithUplinkNetConfig(address, map[string]string{
"ipv6.gateway": "fc00:feed:f00d::1/64",
"ipv6.ovn.ranges": "fc00:feed:f00d::1-fc00:feed:f00d::ff",
}),
"rangesContainSystem": newSystemWithUplinkNetConfig(address, map[string]string{
"ipv6.gateway": "fc00:feed:beef::1/64",
"ipv6.ovn.ranges": "fc00:feed:beef::bed1-fc00:feed:beef::bedf",
}),
}

ensureValidateSystemsFails(handler, invalidSystems, t)
}

func TestValidateSystemsMultiSystem(t *testing.T) {
localAddr := "10.23.1.20"
handler := newTestHandler(localAddr, t)

uplinkConfig := map[string]string{
"ipv4.gateway": "10.23.1.1/16",
"ipv4.ovn.ranges": "10.23.1.50-10.23.1.100",
}

sys1 := newSystemWithUplinkNetConfig(localAddr, uplinkConfig)

sys2 := newSystemWithUplinkNetConfig("10.23.1.72", uplinkConfig)
sys2.ServerInfo.Name = "sys2"

systems := newTestSystemsMap(sys1, sys2)

err := validateSystems(handler, systems)
if err == nil {
t.Fatalf("sys2 with conflicting management IP and ipv4.ovn.ranges passed validation")
}

localAddr = "fc00:bad:feed::f00d"
handler = newTestHandler(localAddr, t)

uplinkConfig = map[string]string{
"ipv6.gateway": "fc00:bad:feed::1/64",
"ipv6.ovn.ranges": "fc00:bad:feed::1-fc00:bad:feed::ff",
}

sys3 := newSystemWithUplinkNetConfig(localAddr, uplinkConfig)

sys4 := newSystemWithUplinkNetConfig("fc00:bad:feed::60", uplinkConfig)
sys4.ServerInfo.Name = "sys4"

systems = newTestSystemsMap(sys3, sys4)

err = validateSystems(handler, systems)
if err == nil {
t.Fatalf("sys4 with conflicting management IP and ipv6.ovn.ranges passed validation")
}
}
Loading