The following pull request was submitted through Github.
It can be accessed and reviewed at: https://github.com/lxc/lxd/pull/8092
This e-mail was sent by the LXC bot, direct replies will not reach the author
unless they happen to be subscribed to this list.
=== Description (from pull-request) ===
This PR adds validation of OVN NIC external routes (`ipv4.routes.external` and `ipv6.routes.external`) settings to check they do not overlap with any other OVN NICs that are connected to networks that use the same uplink, and that they do not overlap with any OVN networks (including the OVN network the OVN NIC we are validation is connected to) that use external subnets and also use the same uplink.
From 4c1e883f9dbb88e9e4850ae9c98e5a23839d3f74 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parr...@canonical.com>
Date: Wed, 28 Oct 2020 14:10:00 +0000
Subject: [PATCH 1/6] lxd/project/project: Updates
StorageVolumeProjectFromRecord to not return error (as never populated)
Signed-off-by: Thomas Parrott <thomas.parr...@canonical.com>
---
lxd/project/project.go | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/lxd/project/project.go b/lxd/project/project.go
index d7d40479fa..f1dbf556b5 100644
--- a/lxd/project/project.go
+++ b/lxd/project/project.go
@@ -79,26 +79,26 @@ func StorageVolumeProject(c *db.Cluster, projectName
string, volumeType int) (st
return "", errors.Wrapf(err, "Failed to load project %q",
projectName)
}
- return StorageVolumeProjectFromRecord(project, volumeType)
+ return StorageVolumeProjectFromRecord(project, volumeType), nil
}
-// StorageVolumeProjectFromRecord returns the project name to use to for the
volume based on the requested project.
-// For custom volume type, if the project specified has the
"features.storage.volumes" flag enabled then the
+// StorageVolumeProjectFromRecord returns the project name to use to for the
volume based on the supplied project.
+// For custom volume type, if the project supplied has the
"features.storage.volumes" flag enabled then the
// project name is returned, otherwise the default project name is returned.
For all other volume types the
// supplied project's name is returned.
-func StorageVolumeProjectFromRecord(p *api.Project, volumeType int) (string,
error) {
+func StorageVolumeProjectFromRecord(p *api.Project, volumeType int) string {
// Non-custom volumes always use the project specified.
if volumeType != db.StoragePoolVolumeTypeCustom {
- return p.Name, nil
+ return p.Name
}
// Custom volumes only use the project specified if the project has the
features.storage.volumes feature
// enabled, otherwise the legacy behaviour of using the default project
for custom volumes is used.
if shared.IsTrue(p.Config["features.storage.volumes"]) {
- return p.Name, nil
+ return p.Name
}
- return Default, nil
+ return Default
}
// NetworkProject returns the project name to use for the network based on the
requested project.
From 94a12db197d93e724177c6e883aa2c8c4d77d66b Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parr...@canonical.com>
Date: Wed, 28 Oct 2020 14:10:51 +0000
Subject: [PATCH 2/6] lxd/project/project: Adds NetworkProjectFromRecord
function
And updates NetworkProject to use it.
Signed-off-by: Thomas Parrott <thomas.parr...@canonical.com>
---
lxd/project/project.go | 21 +++++++++++++++++----
1 file changed, 17 insertions(+), 4 deletions(-)
diff --git a/lxd/project/project.go b/lxd/project/project.go
index f1dbf556b5..202f059650 100644
--- a/lxd/project/project.go
+++ b/lxd/project/project.go
@@ -106,16 +106,29 @@ func StorageVolumeProjectFromRecord(p *api.Project,
volumeType int) string {
// otherwise the default project name is returned. The second return value is
the project's config if non-default
// project is being returned, nil if not.
func NetworkProject(c *db.Cluster, projectName string) (string,
map[string]string, error) {
- project, err := c.GetProject(projectName)
+ p, err := c.GetProject(projectName)
if err != nil {
return "", nil, errors.Wrapf(err, "Failed to load project %q",
projectName)
}
+ projectName = NetworkProjectFromRecord(p)
+
+ if projectName != Default {
+ return projectName, p.Config, nil
+ }
+
+ return Default, nil, nil
+}
+
+// NetworkProjectFromRecord returns the project name to use for the network
based on the supplied project.
+// If the project supplied has the "features.networks" flag enabled then the
project name is returned,
+// otherwise the default project name is returned.
+func NetworkProjectFromRecord(p *api.Project) string {
// Networks only use the project specified if the project has the
features.networks feature enabled,
// otherwise the legacy behaviour of using the default project for
networks is used.
- if shared.IsTrue(project.Config["features.networks"]) {
- return projectName, project.Config, nil
+ if shared.IsTrue(p.Config["features.networks"]) {
+ return p.Name
}
- return Default, nil, nil
+ return Default
}
From b6c2647d92e2278444bd4201afc5eeb79bdf575a Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parr...@canonical.com>
Date: Wed, 28 Oct 2020 17:38:16 +0000
Subject: [PATCH 3/6] Applying: lxd/storage/utils:
project.StorageVolumeProjectFromRecord usage
Signed-off-by: Thomas Parrott <thomas.parr...@canonical.com>
---
lxd/storage/utils.go | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lxd/storage/utils.go b/lxd/storage/utils.go
index d4f40b0dc4..e98ff029bf 100644
--- a/lxd/storage/utils.go
+++ b/lxd/storage/utils.go
@@ -701,7 +701,7 @@ func VolumeUsedByInstances(s *state.State, poolName string,
projectName string,
volumeNameWithType := fmt.Sprintf("%s/%s", volumeTypeName, volumeName)
return s.Cluster.InstanceList(func(inst db.Instance, p api.Project,
profiles []api.Profile) error {
- instStorageProject, err :=
project.StorageVolumeProjectFromRecord(&p, volumeType)
+ instStorageProject :=
project.StorageVolumeProjectFromRecord(&p, volumeType)
if err != nil {
return err
}
From 8c3aae2040633ca921f1d362554ce8bc9913a962 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parr...@canonical.com>
Date: Wed, 28 Oct 2020 17:38:43 +0000
Subject: [PATCH 4/6] lxd/network/driver/ovn: Adds NIC external route overlap
validation of other OVN external network subnets and OVN NIC external routes
Signed-off-by: Thomas Parrott <thomas.parr...@canonical.com>
---
lxd/network/driver_ovn.go | 193 ++++++++++++++++++++++++++++++++++++--
1 file changed, 185 insertions(+), 8 deletions(-)
diff --git a/lxd/network/driver_ovn.go b/lxd/network/driver_ovn.go
index bfda8d7ba7..81f3ebf7a7 100644
--- a/lxd/network/driver_ovn.go
+++ b/lxd/network/driver_ovn.go
@@ -17,6 +17,8 @@ import (
"github.com/lxc/lxd/lxd/cluster"
"github.com/lxc/lxd/lxd/db"
+ deviceConfig "github.com/lxc/lxd/lxd/device/config"
+ "github.com/lxc/lxd/lxd/instance"
"github.com/lxc/lxd/lxd/locking"
"github.com/lxc/lxd/lxd/network/openvswitch"
"github.com/lxc/lxd/lxd/project"
@@ -1869,12 +1871,10 @@ func (n *ovn) getInstanceDevicePortName(instanceID int,
deviceName string) openv
}
// InstanceDevicePortValidateExternalRoutes validates the external routes for
an OVN instance port.
-func (n *ovn) InstanceDevicePortValidateExternalRoutes(externalRoutes
[]*net.IPNet) error {
- // Load the project to get uplink network restrictions.
- p, err := n.state.Cluster.GetProject(n.project)
- if err != nil {
- return errors.Wrapf(err, "Failed to load network restrictions
from project %q", n.project)
- }
+func (n *ovn) InstanceDevicePortValidateExternalRoutes(deviceInstance
instance.Instance, deviceName string, portExternalRoutes []*net.IPNet) error {
+ var err error
+ var p *api.Project
+ var projectNetworks map[string]map[int64]api.Network
// Get uplink routes.
uplinkRoutes, err := n.uplinkRoutes(n.config["network"])
@@ -1882,19 +1882,91 @@ func (n *ovn)
InstanceDevicePortValidateExternalRoutes(externalRoutes []*net.IPN
return err
}
+ err = n.state.Cluster.Transaction(func(tx *db.ClusterTx) error {
+ // Load the project to get uplink network restrictions.
+ p, err = tx.GetProject(n.project)
+ if err != nil {
+ return errors.Wrapf(err, "Failed to load network
restrictions from project %q", n.project)
+ }
+
+ // Get all managed networks across all projects.
+ projectNetworks, err = tx.GetNonPendingNetworks()
+ if err != nil {
+ return errors.Wrapf(err, "Failed to load all networks")
+ }
+
+ return nil
+ })
+ if err != nil {
+ return err
+ }
+
+ // Work out which OVN networks (in all projects and including our own)
use the same uplink as us.
+ ovnProjectNetworksWithOurUplink := make(map[string][]*api.Network)
+ for netProject, networks := range projectNetworks {
+ for _, network := range networks {
+ netInfo := network // Local var for adding pointer to
ovnProjectNetworksWithOurUplink.
+ // Skip non-OVN networks or those networks that don't
use the same uplink as us.
+ if netInfo.Type != "ovn" || netInfo.Config["network"]
!= n.config["network"] {
+ continue
+ }
+
+ if ovnProjectNetworksWithOurUplink[netProject] == nil {
+ ovnProjectNetworksWithOurUplink[netProject] =
[]*api.Network{&netInfo}
+ } else {
+ ovnProjectNetworksWithOurUplink[netProject] =
append(ovnProjectNetworksWithOurUplink[netProject], &netInfo)
+ }
+ }
+ }
+
+ // Get external subnets used by other OVN networks using our uplink.
+ ovnNetworkExternalSubnets, err :=
n.ovnNetworkExternalSubnets(ovnProjectNetworksWithOurUplink, uplinkRoutes)
+ if err != nil {
+ return err
+ }
+
// Get project restricted routes.
projectRestrictedSubnets, err := n.projectRestrictedSubnets(p,
n.config["network"])
if err != nil {
return err
}
- for _, externalRoute := range externalRoutes {
- err = n.validateExternalSubnet(uplinkRoutes,
projectRestrictedSubnets, externalRoute)
+ // If validating with an instance, get external routes configured on
OVN NICs (excluding ours) using
+ // networks that use our uplink.
+ var ovnNICExternalRoutes []*net.IPNet
+ if deviceInstance != nil {
+ ovnNICExternalRoutes, err =
n.ovnNICExternalRoutes(deviceInstance, deviceName,
ovnProjectNetworksWithOurUplink)
if err != nil {
return err
}
}
+ for _, portExternalRoute := range portExternalRoutes {
+ // Check the external port route is allowed within both the
uplink's external routes and any
+ // project restricted subnets.
+ err = n.validateExternalSubnet(uplinkRoutes,
projectRestrictedSubnets, portExternalRoute)
+ if err != nil {
+ return err
+ }
+
+ // Check the external port route doesn't fall within any
existing OVN network external subnets.
+ for _, ovnNetworkExternalSubnet := range
ovnNetworkExternalSubnets {
+ if SubnetContains(ovnNetworkExternalSubnet,
portExternalRoute) || SubnetContains(portExternalRoute,
ovnNetworkExternalSubnet) {
+ // This error is purposefully vague so that it
doesn't reveal any names of
+ // resources potentially outside of the NIC's
project.
+ return fmt.Errorf("External route %q overlaps
with another OVN network's external subnet", portExternalRoute.String())
+ }
+ }
+
+ for _, ovnNICExternalRoute := range ovnNICExternalRoutes {
+ if SubnetContains(ovnNICExternalRoute,
portExternalRoute) || SubnetContains(portExternalRoute, ovnNICExternalRoute) {
+ // This error is purposefully vague so that it
doesn't reveal any names of
+ // resources potentially outside of the NIC's
project.
+ return fmt.Errorf("External route %q overlaps
with another OVN NIC's external route", portExternalRoute.String())
+ }
+ }
+ }
+
return nil
}
@@ -2187,3 +2259,108 @@ func (n *ovn) DHCPv6Subnet() *net.IPNet {
return subnet
}
+
+// ovnNetworkExternalSubnets returns a list of external subnets used by OVN
networks (including our own) using the
+// same uplink as this OVN network. OVN networks are considered to be using
external subnets if their ipv4.address
+// and/or ipv6.address are in the uplink's external routes and the associated
NAT is disabled for the IP family.
+func (n *ovn) ovnNetworkExternalSubnets(tx *db.ClusterTx, uplinkRoutes
[]*net.IPNet) ([]*net.IPNet, error) {
+ // Get all managed networks across all projects.
+ projectNetworks, err := tx.GetNonPendingNetworks()
+ if err != nil {
+ return nil, errors.Wrapf(err, "Failed to load all networks")
+ }
+
+ externalSubnets := make([]*net.IPNet, 0)
+ for _, networks := range projectNetworks {
+ for _, netInfo := range networks {
+ // Skip non-OVN networks or those networks that don't
use the same uplink as us.
+ if netInfo.Type != "ovn" || netInfo.Config["network"]
!= n.config["network"] {
+ continue
+ }
+
+ for _, keyPrefix := range []string{"ipv4", "ipv6"} {
+ if
!shared.IsTrue(netInfo.Config[fmt.Sprintf("%s.nat", keyPrefix)]) {
+ _, ipNet, _ :=
net.ParseCIDR(netInfo.Config[fmt.Sprintf("%s.address", keyPrefix)])
+ if ipNet == nil {
+ // If the network doesn't have
a valid IP, skip it.
+ continue
+ }
+
+ // Check the network's subnet is a
valid external route on uplink.
+ err =
n.validateExternalSubnet(uplinkRoutes, nil, ipNet)
+ if err != nil {
+ return nil, errors.Wrapf(err,
"Failed checking if OVN network external subnet %q is valid external route on
uplink %q", ipNet.String(), n.config["network"])
+ }
+
+ externalSubnets =
append(externalSubnets, ipNet)
+ }
+ }
+ }
+ }
+
+ return externalSubnets, nil
+}
+
+// ovnNICExternalRoutes returns a list of external routes currently used by
OVN NICs (excluding our own) that are
+// connected to OVN networks that share the same uplink as this network uses.
+func (n *ovn) ovnNICExternalRoutes(ourDeviceInstance instance.Instance,
ourDeviceName string, ovnProjectNetworksWithOurUplink
map[string][]*api.Network) ([]*net.IPNet, error) {
+ externalRoutes := make([]*net.IPNet, 0)
+
+ // nicUsesNetwork returns true if the nicDev's "network" property
matches one of the projectNetworks names
+ // and the instNetworkProject matches the projectNetworks's project. As
we only use network name and
+ // project to match we rely on projectNetworks only including OVN
networks that use our uplink.
+ nicUsesNetwork := func(instNetworkProject string, nicDev
map[string]string, projectNetworks map[string][]*api.Network) bool {
+ for netProject, networks := range projectNetworks {
+ for _, network := range networks {
+ if netProject == instNetworkProject &&
network.Name == nicDev["network"] {
+ return true
+ }
+ }
+ }
+
+ return false
+ }
+
+ err := n.state.Cluster.InstanceList(func(inst db.Instance, p
api.Project, profiles []api.Profile) error {
+ // Get the instance's effective network project name.
+ instNetworkProject := project.NetworkProjectFromRecord(&p)
+ devices :=
db.ExpandInstanceDevices(deviceConfig.NewDevices(inst.Devices),
profiles).CloneNative()
+
+ // Iterate through each of the instance's devices, looking for
OVN NICs that are linked to networks
+ // that use our uplink.
+ for devName, devConfig := range devices {
+ if devConfig["type"] != "nic" {
+ continue
+ }
+
+ // Skip our own device.
+ if ourDeviceInstance != nil && inst.Name ==
ourDeviceInstance.Name() && inst.Project == ourDeviceInstance.Project() &&
ourDeviceName == devName {
+ continue
+ }
+
+ // Check whether the NIC device references one of the
OVN networks supplied.
+ if !nicUsesNetwork(instNetworkProject, devConfig,
ovnProjectNetworksWithOurUplink) {
+ continue
+ }
+
+ // For OVN NICs that are connected to networks that use
the same uplink as we do, check
+ // if they have any external routes configured, and if
so add them to the list to return.
+ for _, keyPrefix := range []string{"ipv4", "ipv6"} {
+ _, ipNet, _ :=
net.ParseCIDR(devConfig[fmt.Sprintf("%s.routes.external", keyPrefix)])
+ if ipNet == nil {
+ // If the NIC device doesn't have a
valid external route setting, skip.
+ continue
+ }
+
+ externalRoutes = append(externalRoutes, ipNet)
+ }
+ }
+
+ return nil
+ })
+ if err != nil {
+ return nil, err
+ }
+
+ return externalRoutes, nil
+}
From e9e411a061695d23245eeed25742b3d8ba1e9e21 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parr...@canonical.com>
Date: Wed, 28 Oct 2020 17:39:26 +0000
Subject: [PATCH 5/6] lxd/device/nic/ovn: Updates ovnNet interface's
InstanceDevicePortValidateExternalRoutes to add instance argument
Signed-off-by: Thomas Parrott <thomas.parr...@canonical.com>
---
lxd/device/nic_ovn.go | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lxd/device/nic_ovn.go b/lxd/device/nic_ovn.go
index f743083165..5e86e662ba 100644
--- a/lxd/device/nic_ovn.go
+++ b/lxd/device/nic_ovn.go
@@ -28,7 +28,7 @@ import (
type ovnNet interface {
network.Network
- InstanceDevicePortValidateExternalRoutes(externalRoutes []*net.IPNet)
error
+ InstanceDevicePortValidateExternalRoutes(deviceInstance
instance.Instance, deviceName string, externalRoutes []*net.IPNet) error
InstanceDevicePortAdd(instanceID int, instanceName string, deviceName
string, mac net.HardwareAddr, ips []net.IP, internalRoutes []*net.IPNet,
externalRoutes []*net.IPNet) (openvswitch.OVNSwitchPort, error)
InstanceDevicePortDelete(instanceID int, deviceName string,
internalRoutes []*net.IPNet, externalRoutes []*net.IPNet) error
InstanceDevicePortDynamicIPs(instanceID int, deviceName string)
([]net.IP, error)
From c47c3ecc793876764e21e7531c2c5ff4b31ce99a Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parr...@canonical.com>
Date: Wed, 28 Oct 2020 17:40:07 +0000
Subject: [PATCH 6/6] lxd/device/nic/ovn:
d.network.InstanceDevicePortValidateExternalRoutes usage
Signed-off-by: Thomas Parrott <thomas.parr...@canonical.com>
---
lxd/device/nic_ovn.go | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lxd/device/nic_ovn.go b/lxd/device/nic_ovn.go
index 5e86e662ba..5eb94e5125 100644
--- a/lxd/device/nic_ovn.go
+++ b/lxd/device/nic_ovn.go
@@ -179,7 +179,7 @@ func (d *nicOVN) validateConfig(instConf
instance.ConfigReader) error {
}
}
- err =
d.network.InstanceDevicePortValidateExternalRoutes(externalRoutes)
+ err =
d.network.InstanceDevicePortValidateExternalRoutes(d.inst, d.name,
externalRoutes)
if err != nil {
return err
}
_______________________________________________
lxc-devel mailing list
lxc-devel@lists.linuxcontainers.org
http://lists.linuxcontainers.org/listinfo/lxc-devel
