Skip to content

Commit

Permalink
Support more than one IP per interface and IPv6 for results returned …
Browse files Browse the repository at this point in the history
…by CNI

Signed-off-by: Thomas Lefebvre <tlefebvre@cloudflare.com>
  • Loading branch information
th0m committed Mar 24, 2023
1 parent aa97886 commit e0b0c97
Show file tree
Hide file tree
Showing 7 changed files with 88 additions and 89 deletions.
61 changes: 28 additions & 33 deletions cni/vmconf/vmconf.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,19 +12,19 @@
// permissions and limitations under the License.

/*
Package vmconf defines an interface for converting particular CNI invocation
results to networking configuration usable by a VM. It expects the CNI result
to have the following properties:
* The results should contain an interface for a tap device, which will be used
as the VM's tap device.
* The results should contain an interface with the same name as the tap device
but with sandbox ID set to the containerID provided during CNI invocation.
This should be a "pseudo-interface", not one that has actually been created.
It represents the configuration that should be applied to the VM internally.
The CNI "containerID" is, in this case, used more as a "vmID" to represent
the VM's internal network interface.
* If the CNI results specify an IP associated with this interface, that IP
should be used to statically configure the VM's internal network interface.
Package vmconf defines an interface for converting particular CNI invocation
results to networking configuration usable by a VM. It expects the CNI result
to have the following properties:
- The results should contain an interface for a tap device, which will be used
as the VM's tap device.
- The results should contain an interface with the same name as the tap device
but with sandbox ID set to the containerID provided during CNI invocation.
This should be a "pseudo-interface", not one that has actually been created.
It represents the configuration that should be applied to the VM internally.
The CNI "containerID" is, in this case, used more as a "vmID" to represent
the VM's internal network interface.
- If the CNI results specify an IP associated with this interface, that IP
should be used to statically configure the VM's internal network interface.
*/
package vmconf

Expand Down Expand Up @@ -62,7 +62,7 @@ type StaticNetworkConf struct {
VMMTU int
// VMIPConfig is the ip configuration that callers should configure their VM's internal
// primary interface to use.
VMIPConfig *current.IPConfig
VMIPConfig []*current.IPConfig
// VMRoutes are the routes that callers should configure their VM's internal route table
// to have
VMRoutes []*types.Route
Expand All @@ -88,32 +88,32 @@ type StaticNetworkConf struct {
//
// Due to the limitation of "ip=", not all configuration specified in StaticNetworkConf can be
// applied automatically. In particular:
// * The MacAddr and MTU cannot be applied
// * The only routes created will match what's specified in VMIPConfig; VMRoutes will be ignored.
// * Only up to two namesevers can be supplied. If VMNameservers is has more than 2 entries, only
// the first two in the slice will be applied in the VM.
// * VMDomain, VMSearchDomains and VMResolverOptions will be ignored
// * Nameserver settings are also only set in /proc/net/pnp. Most applications will thus require
// /etc/resolv.conf to be a symlink to /proc/net/pnp in order to resolve names as expected.
// - The MacAddr and MTU cannot be applied
// - The only routes created will match what's specified in VMIPConfig; VMRoutes will be ignored.
// - Only up to two namesevers can be supplied. If VMNameservers is has more than 2 entries, only
// the first two in the slice will be applied in the VM.
// - VMDomain, VMSearchDomains and VMResolverOptions will be ignored
// - Nameserver settings are also only set in /proc/net/pnp. Most applications will thus require
// /etc/resolv.conf to be a symlink to /proc/net/pnp in order to resolve names as expected.
func (c StaticNetworkConf) IPBootParam() string {
// See "ip=" section of kernel linked above for details on each field listed below.

// client-ip is really just the ip that will be assigned to the primary interface
clientIP := c.VMIPConfig.Address.IP.String()
clientIP := c.VMIPConfig[0].Address.IP.String()

// don't set nfs server IP
const serverIP = ""

// default gateway for the network; used to generate a corresponding route table entry
defaultGateway := c.VMIPConfig.Gateway.String()
defaultGateway := c.VMIPConfig[0].Gateway.String()

// subnet mask used to generate a corresponding route table entry for the primary interface
// (must be provided in dotted decimal notation)
subnetMask := fmt.Sprintf("%d.%d.%d.%d",
c.VMIPConfig.Address.Mask[0],
c.VMIPConfig.Address.Mask[1],
c.VMIPConfig.Address.Mask[2],
c.VMIPConfig.Address.Mask[3],
c.VMIPConfig[0].Address.Mask[0],
c.VMIPConfig[0].Address.Mask[1],
c.VMIPConfig[0].Address.Mask[2],
c.VMIPConfig[0].Address.Mask[3],
)

// the "hostname" field actually just configures a hostname value for DHCP requests, thus no need to set it
Expand Down Expand Up @@ -168,11 +168,6 @@ func StaticNetworkConfFrom(result types.Result, containerID string) (*StaticNetw

// find the IP associated with the VM iface
vmIPs := internal.InterfaceIPs(currentResult, vmIface.Name, vmIface.Sandbox)
if len(vmIPs) != 1 {
return nil, fmt.Errorf("expected to find 1 IP for vm interface %q, but instead found %+v",
vmIface.Name, vmIPs)
}
vmIP := vmIPs[0]

netNS, err := ns.GetNS(tapIface.Sandbox)
if err != nil {
Expand All @@ -189,7 +184,7 @@ func StaticNetworkConfFrom(result types.Result, containerID string) (*StaticNetw
NetNSPath: tapIface.Sandbox,
VMMacAddr: vmIface.Mac,
VMMTU: tapMTU,
VMIPConfig: vmIP,
VMIPConfig: vmIPs,
VMRoutes: currentResult.Routes,
VMNameservers: currentResult.DNS.Nameservers,
VMDomain: currentResult.DNS.Domain,
Expand Down
12 changes: 7 additions & 5 deletions cni/vmconf/vmconf_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,12 +68,14 @@ func TestIPBootParams(t *testing.T) {
VMMacAddr: "00:11:22:33:44:55",
VMIfName: "eth0",
VMMTU: 1337,
VMIPConfig: &current.IPConfig{
Address: net.IPNet{
IP: net.IPv4(10, 0, 0, 2),
Mask: net.IPv4Mask(255, 255, 255, 0),
VMIPConfig: []*current.IPConfig{
&current.IPConfig{
Address: net.IPNet{
IP: net.IPv4(10, 0, 0, 2),
Mask: net.IPv4Mask(255, 255, 255, 0),
},
Gateway: net.IPv4(10, 0, 0, 1),
},
Gateway: net.IPv4(10, 0, 0, 1),
},
VMRoutes: []*types.Route{{
Dst: net.IPNet{
Expand Down
4 changes: 2 additions & 2 deletions examples/cmd/snapshotting/example_demo.go
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ func connectToVM(m *sdk.Machine, sshKeyPath string) (*ssh.Client, error) {
return nil, errors.New("No network interfaces")
}

ip := m.Cfg.NetworkInterfaces[0].StaticConfiguration.IPConfiguration.IPAddr.IP // IP of VM
ip := m.Cfg.NetworkInterfaces[0].StaticConfiguration.IPConfiguration[0].IPAddr.IP // IP of VM

return ssh.Dial("tcp", fmt.Sprintf("%s:22", ip), config)
}
Expand Down Expand Up @@ -194,7 +194,7 @@ func createSnapshotSSH(ctx context.Context, socketPath, memPath, snapPath string
}
}()

vmIP := m.Cfg.NetworkInterfaces[0].StaticConfiguration.IPConfiguration.IPAddr.IP.String()
vmIP := m.Cfg.NetworkInterfaces[0].StaticConfiguration.IPConfiguration[0].IPAddr.IP.String()
fmt.Printf("IP of VM: %v\n", vmIP)

sshKeyPath := filepath.Join(dir, "root-drive-ssh-key")
Expand Down
9 changes: 6 additions & 3 deletions machine.go
Original file line number Diff line number Diff line change
Expand Up @@ -503,8 +503,11 @@ func (m *Machine) setupKernelArgs(ctx context.Context) error {
// If any network interfaces have a static IP configured, we need to set the "ip=" boot param.
// Validation that we are not overriding an existing "ip=" setting happens in the network validation
if staticIPInterface := m.Cfg.NetworkInterfaces.staticIPInterface(); staticIPInterface != nil {
ipBootParam := staticIPInterface.StaticConfiguration.IPConfiguration.ipBootParam()
kernelArgs["ip"] = &ipBootParam
// Only generate the ip= boot param if there is a single ip on the interface
if len(staticIPInterface.StaticConfiguration.IPConfiguration) == 1 {
ipBootParam := staticIPInterface.StaticConfiguration.IPConfiguration[0].ipBootParam()
kernelArgs["ip"] = &ipBootParam
}
}

m.Cfg.KernelArgs = kernelArgs.String()
Expand Down Expand Up @@ -649,7 +652,7 @@ func (m *Machine) startVMM(ctx context.Context) error {
return nil
}

//StopVMM stops the current VMM.
// StopVMM stops the current VMM.
func (m *Machine) StopVMM() error {
return m.stopVMM()
}
Expand Down
4 changes: 2 additions & 2 deletions machine_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1998,7 +1998,7 @@ func connectToVM(m *Machine, sshKeyPath string) (*ssh.Client, error) {
return nil, errors.New("No network interfaces")
}

ip := m.Cfg.NetworkInterfaces.staticIPInterface().StaticConfiguration.IPConfiguration.IPAddr.IP
ip := m.Cfg.NetworkInterfaces.staticIPInterface().StaticConfiguration.IPConfiguration[0].IPAddr.IP

return ssh.Dial("tcp", fmt.Sprintf("%s:22", ip), config)
}
Expand Down Expand Up @@ -2193,7 +2193,7 @@ func TestLoadSnapshot(t *testing.T) {
require.NoError(t, err)
defer client.Close()

ipToRestore = m.Cfg.NetworkInterfaces.staticIPInterface().StaticConfiguration.IPConfiguration.IPAddr.IP.String()
ipToRestore = m.Cfg.NetworkInterfaces.staticIPInterface().StaticConfiguration.IPConfiguration[0].IPAddr.IP.String()

session, err := client.NewSession()
require.NoError(t, err)
Expand Down
32 changes: 17 additions & 15 deletions network.go
Original file line number Diff line number Diff line change
Expand Up @@ -141,19 +141,18 @@ func (networkInterfaces NetworkInterfaces) setupNetwork(
MacAddress: vmNetConf.VMMacAddr,
}

if vmNetConf.VMIPConfig != nil {
for _, vmIPCfg := range vmNetConf.VMIPConfig {
if len(vmNetConf.VMNameservers) > 2 {
logger.Warnf("more than 2 nameservers provided from CNI result, only the first 2 %+v will be applied",
vmNetConf.VMNameservers[:2])
vmNetConf.VMNameservers = vmNetConf.VMNameservers[:2]
}

cniNetworkInterface.StaticConfiguration.IPConfiguration = &IPConfiguration{
IPAddr: vmNetConf.VMIPConfig.Address,
Gateway: vmNetConf.VMIPConfig.Gateway,
cniNetworkInterface.StaticConfiguration.IPConfiguration = append(cniNetworkInterface.StaticConfiguration.IPConfiguration, &IPConfiguration{
IPAddr: vmIPCfg.Address,
Gateway: vmIPCfg.Gateway,
Nameservers: vmNetConf.VMNameservers,
IfName: cniNetworkInterface.CNIConfiguration.VMIfName,
}
})
}
}

Expand Down Expand Up @@ -484,16 +483,16 @@ type StaticNetworkConfiguration struct {

// IPConfiguration (optional) allows a static IP, gateway and up to 2 DNS nameservers
// to be automatically configured within the VM upon startup.
IPConfiguration *IPConfiguration
IPConfiguration []*IPConfiguration
}

func (staticConf StaticNetworkConfiguration) validate() error {
if staticConf.HostDevName == "" {
return fmt.Errorf("HostDevName must be provided if StaticNetworkConfiguration is provided: %+v", staticConf)
}

if staticConf.IPConfiguration != nil {
err := staticConf.IPConfiguration.validate()
for _, ipCfg := range staticConf.IPConfiguration {
err := ipCfg.validate()
if err != nil {
return err
}
Expand Down Expand Up @@ -522,10 +521,10 @@ type IPConfiguration struct {
}

func (ipConf IPConfiguration) validate() error {
// Make sure only ipv4 is being provided (for now).
// Make sure it is a valid ip.
for _, ip := range []net.IP{ipConf.IPAddr.IP, ipConf.Gateway} {
if ip.To4() == nil {
return fmt.Errorf("invalid ip, only ipv4 addresses are supported: %+v", ip)
if ip.To4() == nil && ip.To16() == nil {
return fmt.Errorf("invalid ip: %+v", ip)
}
}

Expand All @@ -538,11 +537,14 @@ func (ipConf IPConfiguration) validate() error {

func (conf IPConfiguration) ipBootParam() string {
// the vmconf package already has a function for doing this, just re-use it

vmConf := vmconf.StaticNetworkConf{
VMNameservers: conf.Nameservers,
VMIPConfig: &current.IPConfig{
Address: conf.IPAddr,
Gateway: conf.Gateway,
VMIPConfig: []*current.IPConfig{
{
Address: conf.IPAddr,
Gateway: conf.Gateway,
},
},
VMIfName: conf.IfName,
}
Expand Down
55 changes: 26 additions & 29 deletions network_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,23 +44,27 @@ var (
kernelArgsWithIP = parseKernelArgs("foo=bar this=phony ip=whatevz")

// These RFC 5737 IPs are reserved for documentation, they are not usable
validIPConfiguration = &IPConfiguration{
IPAddr: net.IPNet{
IP: net.IPv4(198, 51, 100, 2),
Mask: net.IPv4Mask(255, 255, 255, 0),
validIPConfiguration = []*IPConfiguration{
&IPConfiguration{
IPAddr: net.IPNet{
IP: net.IPv4(198, 51, 100, 2),
Mask: net.IPv4Mask(255, 255, 255, 0),
},
Gateway: net.IPv4(198, 51, 100, 1),
Nameservers: []string{"192.0.2.1", "192.0.2.2"},
},
Gateway: net.IPv4(198, 51, 100, 1),
Nameservers: []string{"192.0.2.1", "192.0.2.2"},
}

// IPv6 is currently invalid
// These RFC 3849 IPs are reserved for documentation, they are not usable
invalidIPConfiguration = &IPConfiguration{
IPAddr: net.IPNet{
IP: net.ParseIP("2001:db8:a0b:12f0::2"),
Mask: net.CIDRMask(24, 128),
invalidIPConfiguration = []*IPConfiguration{
&IPConfiguration{
IPAddr: net.IPNet{
IP: net.ParseIP("2001:db8:a0b:12f0::2"),
Mask: net.CIDRMask(24, 128),
},
Gateway: net.ParseIP("2001:db8:a0b:12f0::1"),
},
Gateway: net.ParseIP("2001:db8:a0b:12f0::1"),
}

validStaticNetworkInterface = NetworkInterface{
Expand Down Expand Up @@ -99,30 +103,23 @@ func TestNetworkStaticValidationFails_TooManyNameservers(t *testing.T) {
staticNetworkConfig := StaticNetworkConfiguration{
MacAddress: mockMacAddrString,
HostDevName: tapName,
IPConfiguration: &IPConfiguration{
IPAddr: net.IPNet{
IP: net.IPv4(198, 51, 100, 2),
Mask: net.IPv4Mask(255, 255, 255, 0),
IPConfiguration: []*IPConfiguration{
&IPConfiguration{
IPAddr: net.IPNet{
IP: net.IPv4(198, 51, 100, 2),
Mask: net.IPv4Mask(255, 255, 255, 0),
},
Gateway: net.IPv4(198, 51, 100, 1),
Nameservers: []string{"192.0.2.1", "192.0.2.2", "192.0.2.3"},
},
Gateway: net.IPv4(198, 51, 100, 1),
Nameservers: []string{"192.0.2.1", "192.0.2.2", "192.0.2.3"},
},
}

err := staticNetworkConfig.validate()
assert.Error(t, err, "network config with more than two nameservers did not result in validation error")
}

func TestNetworkStaticValidationFails_IPConfiguration(t *testing.T) {
staticNetworkConfig := StaticNetworkConfiguration{
MacAddress: mockMacAddrString,
HostDevName: tapName,
IPConfiguration: invalidIPConfiguration,
}

err := staticNetworkConfig.validate()
assert.Error(t, err, "invalid network config hostdevname did not result in validation error")
}
// TestNetworkStaticValidationFails_IPConfiguration removed as IPv6 support was added in this fork

func TestNetworkCNIValidation(t *testing.T) {
err := validCNIInterface.CNIConfiguration.validate()
Expand Down Expand Up @@ -448,12 +445,12 @@ func startCNIMachine(t *testing.T, ctx context.Context, m *Machine) string {
assert.NotEmpty(t, staticConfig.HostDevName,
"static config should have host dev name set")

ipConfig := staticConfig.IPConfiguration
ipConfig := staticConfig.IPConfiguration[0]
require.NotNil(t, ipConfig,
"cni configuration should have updated network interface ip configuration")

require.Equal(t, m.Cfg.NetworkInterfaces[0].CNIConfiguration.VMIfName,
staticConfig.IPConfiguration.IfName,
staticConfig.IPConfiguration[0].IfName,
"interface name should be propagated to static conf")

return ipConfig.IPAddr.IP.String()
Expand Down

0 comments on commit e0b0c97

Please sign in to comment.