From 24bec9a76bcc2d8e93670047ed0ee8c16d9d7620 Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Thu, 26 Aug 2021 15:06:38 +0200 Subject: default network: do not validate the used subnets The default network should not be validated against used subnets, we have to ensure that this network can always be created even when a subnet is already used on the host. This could happen if you run a container on this net, then the cni interface will be created on the host and "block" this subnet from being used again. Therefore the next podman command tries to create the default net again and it would fail because it thinks the network is used on the host. Signed-off-by: Paul Holzinger --- libpod/network/cni/config.go | 84 ++++++++++++++++++++++++------------------ libpod/network/cni/network.go | 2 +- libpod/network/cni/run_test.go | 4 ++ 3 files changed, 53 insertions(+), 37 deletions(-) (limited to 'libpod') diff --git a/libpod/network/cni/config.go b/libpod/network/cni/config.go index 44842c664..d31cd3002 100644 --- a/libpod/network/cni/config.go +++ b/libpod/network/cni/config.go @@ -24,7 +24,7 @@ func (n *cniNetwork) NetworkCreate(net types.Network) (types.Network, error) { if err != nil { return types.Network{}, err } - network, err := n.networkCreate(net, true) + network, err := n.networkCreate(net, false) if err != nil { return types.Network{}, err } @@ -33,94 +33,106 @@ func (n *cniNetwork) NetworkCreate(net types.Network) (types.Network, error) { return *network.libpodNet, nil } -func (n *cniNetwork) networkCreate(net types.Network, writeToDisk bool) (*network, error) { +// networkCreate will fill out the given network struct and return the new network entry. +// If defaultNet is true it will not validate against used subnets and it will not write the cni config to disk. +func (n *cniNetwork) networkCreate(newNetwork types.Network, defaultNet bool) (*network, error) { // if no driver is set use the default one - if net.Driver == "" { - net.Driver = types.DefaultNetworkDriver + if newNetwork.Driver == "" { + newNetwork.Driver = types.DefaultNetworkDriver } // FIXME: Should we use a different type for network create without the ID field? // the caller is not allowed to set a specific ID - if net.ID != "" { + if newNetwork.ID != "" { return nil, errors.Wrap(define.ErrInvalidArg, "ID can not be set for network create") } - if net.Labels == nil { - net.Labels = map[string]string{} + if newNetwork.Labels == nil { + newNetwork.Labels = map[string]string{} } - if net.Options == nil { - net.Options = map[string]string{} + if newNetwork.Options == nil { + newNetwork.Options = map[string]string{} } - if net.IPAMOptions == nil { - net.IPAMOptions = map[string]string{} + if newNetwork.IPAMOptions == nil { + newNetwork.IPAMOptions = map[string]string{} } var name string var err error // validate the name when given - if net.Name != "" { - if !define.NameRegex.MatchString(net.Name) { - return nil, errors.Wrapf(define.RegexError, "network name %s invalid", net.Name) + if newNetwork.Name != "" { + if !define.NameRegex.MatchString(newNetwork.Name) { + return nil, errors.Wrapf(define.RegexError, "network name %s invalid", newNetwork.Name) } - if _, ok := n.networks[net.Name]; ok { - return nil, errors.Wrapf(define.ErrNetworkExists, "network name %s already used", net.Name) + if _, ok := n.networks[newNetwork.Name]; ok { + return nil, errors.Wrapf(define.ErrNetworkExists, "network name %s already used", newNetwork.Name) } } else { name, err = n.getFreeDeviceName() if err != nil { return nil, err } - net.Name = name + newNetwork.Name = name } - usedNetworks, err := n.getUsedSubnets() - if err != nil { - return nil, err + // Only get the used networks for validation if we do not create the default network. + // The default network should not be validated against used subnets, we have to ensure + // that this network can always be created even when a subnet is already used on the host. + // This could happen if you run a container on this net, then the cni interface will be + // created on the host and "block" this subnet from being used again. + // Therefore the next podman command tries to create the default net again and it would + // fail because it thinks the network is used on the host. + var usedNetworks []*net.IPNet + if !defaultNet { + usedNetworks, err = n.getUsedSubnets() + if err != nil { + return nil, err + } } - switch net.Driver { + switch newNetwork.Driver { case types.BridgeNetworkDriver: // if the name was created with getFreeDeviceName set the interface to it as well - if name != "" && net.NetworkInterface == "" { - net.NetworkInterface = name + if name != "" && newNetwork.NetworkInterface == "" { + newNetwork.NetworkInterface = name } - err = n.createBridge(&net, usedNetworks) + err = n.createBridge(&newNetwork, usedNetworks) if err != nil { return nil, err } case types.MacVLANNetworkDriver: - err = createMacVLAN(&net) + err = createMacVLAN(&newNetwork) if err != nil { return nil, err } default: - return nil, errors.Wrapf(define.ErrInvalidArg, "unsupported driver %s", net.Driver) + return nil, errors.Wrapf(define.ErrInvalidArg, "unsupported driver %s", newNetwork.Driver) } - for i := range net.Subnets { - err := validateSubnet(&net.Subnets[i], !net.Internal, usedNetworks) + for i := range newNetwork.Subnets { + err := validateSubnet(&newNetwork.Subnets[i], !newNetwork.Internal, usedNetworks) if err != nil { return nil, err } - if util.IsIPv6(net.Subnets[i].Subnet.IP) { - net.IPv6Enabled = true + if util.IsIPv6(newNetwork.Subnets[i].Subnet.IP) { + newNetwork.IPv6Enabled = true } } // generate the network ID - net.ID = getNetworkIDFromName(net.Name) + newNetwork.ID = getNetworkIDFromName(newNetwork.Name) // FIXME: Should this be a hard error? - if net.DNSEnabled && net.Internal && hasDNSNamePlugin(n.cniPluginDirs) { - logrus.Warnf("dnsname and internal networks are incompatible. dnsname plugin not configured for network %s", net.Name) - net.DNSEnabled = false + if newNetwork.DNSEnabled && newNetwork.Internal && hasDNSNamePlugin(n.cniPluginDirs) { + logrus.Warnf("dnsname and internal networks are incompatible. dnsname plugin not configured for network %s", newNetwork.Name) + newNetwork.DNSEnabled = false } - cniConf, path, err := n.createCNIConfigListFromNetwork(&net, writeToDisk) + cniConf, path, err := n.createCNIConfigListFromNetwork(&newNetwork, !defaultNet) if err != nil { return nil, err } - return &network{cniNet: cniConf, libpodNet: &net, filename: path}, nil + return &network{cniNet: cniConf, libpodNet: &newNetwork, filename: path}, nil } // NetworkRemove will remove the Network with the given name or ID. diff --git a/libpod/network/cni/network.go b/libpod/network/cni/network.go index 479f9e0eb..4e4ea7ee5 100644 --- a/libpod/network/cni/network.go +++ b/libpod/network/cni/network.go @@ -179,7 +179,7 @@ func (n *cniNetwork) createDefaultNetwork() (*network, error) { {Subnet: n.defaultSubnet}, }, } - return n.networkCreate(net, false) + return n.networkCreate(net, true) } // getNetwork will lookup a network by name or ID. It returns an diff --git a/libpod/network/cni/run_test.go b/libpod/network/cni/run_test.go index 32e88ca61..f6da22a76 100644 --- a/libpod/network/cni/run_test.go +++ b/libpod/network/cni/run_test.go @@ -140,6 +140,10 @@ var _ = Describe("run CNI", func() { Expect(res[defNet].DNSServerIPs).To(BeEmpty()) Expect(res[defNet].DNSSearchDomains).To(BeEmpty()) + // reload the interface so the networks are reload from disk + libpodNet, err := getNetworkInterface(cniConfDir, false) + Expect(err).To(BeNil()) + err = libpodNet.Teardown(netNSContainer.Path(), types.TeardownOptions(setupOpts)) Expect(err).To(BeNil()) }) -- cgit v1.2.3-54-g00ecf