Option for root_helper when checking namespace

Adds a configuration option to use the root helper in the ip netns list
command executed by the IP library when checking for the existence of a
namespace.  This prevents an unprivileged l3 agent from erroneously trying
to create another namespace when one already exists. This is necessary in
environments with constrained permissions on /var/run/netns via umask or
other access controls.

However, due to the overhead incurred by calling sudo every time on systems
where this restriction isn't in place, this configuration won't be desired
all of the time. So this patch also adds a sanity check that reports back
whether or not the root_helper is required for a deployment.

DocImpact

Closes-Bug: #1348812
Closes-Bug: #1311804
Change-Id: If7560161de3be6066af0d9866e6b5cd7c7247c33

diff --git a/etc/neutron.conf b/etc/neutron.conf
index e1c2cc258a2a1470739a07856190666ab9eb0174..95b3af1afbd8b2a7303804b27b931d948b0e340c 100644 (file)
--- a/etc/neutron.conf
+++ b/etc/neutron.conf
@@ -554,6 +554,11 @@ lock_path = $state_path/lock
 # each rule's purpose. (System must support the iptables comments module.)
 # comment_iptables_rules = True
 
+# Use the root helper when listing the namespaces on a system. This may not
+# be required depending on the security configuration. If the root helper is
+# not required, set this to False for a performance improvement.
+# use_helper_for_ns_read = True
+
 # =========== items for agent management extension =============
 # seconds between nodes reporting state to server; should be less than
 # agent_down_time, best if it is half or less than agent_down_time

diff --git a/neutron/agent/common/config.py b/neutron/agent/common/config.py
index 3ee41dde851f91a2a3575238c14c76e55c16f444..43335fe45786ecd2fb45faf3e9c680bed2e9d422 100644 (file)
--- a/neutron/agent/common/config.py
+++ b/neutron/agent/common/config.py
@@ -27,6 +27,10 @@ LOG = logging.getLogger(__name__)
 ROOT_HELPER_OPTS = [
     cfg.StrOpt('root_helper', default='sudo',
                help=_('Root helper application.')),
+    cfg.BoolOpt('use_helper_for_ns_read',
+                default=True,
+                help=_('Use the root helper to read the namespaces from '
+                       'the operating system.')),
 ]
 
 AGENT_STATE_OPTS = [

diff --git a/neutron/agent/linux/ip_lib.py b/neutron/agent/linux/ip_lib.py
index e7a4e56613b589e957e77dcc483f52a5bd92fe63..4ca7591df614e8311783bac69a3ea9e0624d4c76 100644 (file)
--- a/neutron/agent/linux/ip_lib.py
+++ b/neutron/agent/linux/ip_lib.py
@@ -554,8 +554,11 @@ class IpNetnsCommand(IpCommandBase):
             check_exit_code=check_exit_code, extra_ok_codes=extra_ok_codes)
 
     def exists(self, name):
-        output = self._parent._execute('o', 'netns', ['list'])
-
+        root_helper = self._parent.root_helper
+        if not cfg.CONF.AGENT.use_helper_for_ns_read:
+            root_helper = None
+        output = self._parent._execute('o', 'netns', ['list'],
+                                       root_helper=root_helper)
         for line in output.split('\n'):
             if name == line.strip():
                 return True

diff --git a/neutron/cmd/sanity/checks.py b/neutron/cmd/sanity/checks.py
index c672546a1bfcf54af1a8e2f073a1796dab83091a..de7de5482f03445b6e2abf9825071e2196d678b7 100644 (file)
--- a/neutron/cmd/sanity/checks.py
+++ b/neutron/cmd/sanity/checks.py
@@ -15,12 +15,14 @@
 
 import netaddr
 
+from neutron.agent.linux import ip_lib
 from neutron.agent.linux import ip_link_support
 from neutron.agent.linux import ovs_lib
 from neutron.agent.linux import utils as agent_utils
 from neutron.common import utils
 from neutron.i18n import _LE
 from neutron.openstack.common import log as logging
+from neutron.openstack.common import uuidutils
 from neutron.plugins.common import constants as const
 from neutron.plugins.openvswitch.common import constants as ovs_const
 
@@ -107,3 +109,16 @@ def vf_management_supported(root_helper):
                           "ip link command"))
         return False
     return True
+
+
+def netns_read_requires_helper(root_helper):
+    ipw = ip_lib.IPWrapper(root_helper)
+    nsname = "netnsreadtest-" + uuidutils.generate_uuid()
+    ipw.netns.add(nsname)
+    try:
+        # read without root_helper. if exists, not required.
+        ipw_nohelp = ip_lib.IPWrapper()
+        exists = ipw_nohelp.netns.exists(nsname)
+    finally:
+        ipw.netns.delete(nsname)
+    return not exists

diff --git a/neutron/cmd/sanity_check.py b/neutron/cmd/sanity_check.py
index 1ab1ff57ed293bd868fd211de55efa47677d38ed..76bb46b801bb929f591b45865d5c37135aa59a21 100644 (file)
--- a/neutron/cmd/sanity_check.py
+++ b/neutron/cmd/sanity_check.py
@@ -17,7 +17,7 @@ import sys
 
 from neutron.cmd.sanity import checks
 from neutron.common import config
-from neutron.i18n import _LE
+from neutron.i18n import _LE, _LW
 from neutron.openstack.common import log as logging
 from oslo.config import cfg
 
@@ -31,6 +31,8 @@ cfg.CONF.import_group('ml2_sriov',
 
 class BoolOptCallback(cfg.BoolOpt):
     def __init__(self, name, callback, **kwargs):
+        if 'default' not in kwargs:
+            kwargs['default'] = False
         self.callback = callback
         super(BoolOptCallback, self).__init__(name, **kwargs)
 
@@ -54,6 +56,27 @@ def check_ovs_patch():
     return result
 
 
+def check_read_netns():
+    required = checks.netns_read_requires_helper(
+        root_helper=cfg.CONF.AGENT.root_helper)
+    if not required and cfg.CONF.AGENT.use_helper_for_ns_read:
+        LOG.warning(_LW("The user that is executing neutron can read the "
+                        "namespaces without using the root_helper. Disable "
+                        "the use_helper_for_ns_read option to avoid a "
+                        "performance impact."))
+        # Don't fail because nothing is actually broken. Just not optimal.
+        result = True
+    elif required and not cfg.CONF.AGENT.use_helper_for_ns_read:
+        LOG.error(_LE("The user that is executing neutron does not have "
+                      "permissions to read the namespaces. Enable the "
+                      "use_helper_for_ns_read configuration option."))
+        result = False
+    else:
+        # everything is configured appropriately
+        result = True
+    return result
+
+
 def check_nova_notify():
     result = checks.nova_notify_supported()
     if not result:
@@ -85,17 +108,18 @@ def check_vf_management():
 
 # Define CLI opts to test specific features, with a calback for the test
 OPTS = [
-    BoolOptCallback('ovs_vxlan', check_ovs_vxlan, default=False,
+    BoolOptCallback('ovs_vxlan', check_ovs_vxlan,
                     help=_('Check for vxlan support')),
-    BoolOptCallback('ovs_patch', check_ovs_patch, default=False,
+    BoolOptCallback('ovs_patch', check_ovs_patch,
                     help=_('Check for patch port support')),
-    BoolOptCallback('nova_notify', check_nova_notify, default=False,
+    BoolOptCallback('nova_notify', check_nova_notify,
                     help=_('Check for nova notification support')),
-    BoolOptCallback('arp_responder', check_arp_responder, default=False,
+    BoolOptCallback('arp_responder', check_arp_responder,
                     help=_('Check for ARP responder support')),
-    BoolOptCallback('vf_management', check_vf_management, default=False,
+    BoolOptCallback('vf_management', check_vf_management,
                     help=_('Check for VF management support')),
-
+    BoolOptCallback('read_netns', check_read_netns,
+                    help=_('Check netns permission settings')),
 ]
 
 
@@ -118,6 +142,8 @@ def enable_tests_from_config():
         cfg.CONF.set_override('arp_responder', True)
     if cfg.CONF.ml2_sriov.agent_required:
         cfg.CONF.set_override('vf_management', True)
+    if not cfg.CONF.AGENT.use_helper_for_ns_read:
+        cfg.CONF.set_override('read_netns', True)
 
 
 def all_tests_passed():

diff --git a/neutron/tests/functional/agent/linux/base.py b/neutron/tests/functional/agent/linux/base.py
index b900d56fd65a8d41bc12e0242c0e1a308e208601..034632deaad31b0ee491f153409ef5ce1724420d 100644 (file)
--- a/neutron/tests/functional/agent/linux/base.py
+++ b/neutron/tests/functional/agent/linux/base.py
@@ -15,7 +15,9 @@
 import random
 
 import netaddr
+from oslo.config import cfg
 
+from neutron.agent.common import config
 from neutron.agent.linux import ip_lib
 from neutron.agent.linux import ovs_lib
 from neutron.agent.linux import utils
@@ -44,6 +46,10 @@ def get_rand_veth_name():
 
 class BaseLinuxTestCase(functional_base.BaseSudoTestCase):
 
+    def setUp(self):
+        super(BaseLinuxTestCase, self).setUp()
+        config.register_root_helper(cfg.CONF)
+
     def check_command(self, cmd, error_text, skip_msg, root_helper=None):
         try:
             utils.execute(cmd, root_helper=root_helper)

diff --git a/neutron/tests/functional/sanity/test_sanity.py b/neutron/tests/functional/sanity/test_sanity.py
index 4ee357d426729bfef79b4f2dec13054a937fee35..3006ffa5e33150f273a4316c70a8073b3f22d618 100644 (file)
--- a/neutron/tests/functional/sanity/test_sanity.py
+++ b/neutron/tests/functional/sanity/test_sanity.py
@@ -55,3 +55,6 @@ class SanityTestCaseRoot(functional_base.BaseSudoTestCase):
 
     def test_vf_management_runs(self):
         checks.vf_management_supported(self.root_helper)
+
+    def test_namespace_root_read_detection_runs(self):
+        checks.netns_read_requires_helper(self.root_helper)