Merge "Allow metadata proxy to log with nobody user/group"
This commit is contained in:
commit
3f45031d68
|
@ -257,6 +257,14 @@ lock_path = $state_path/lock
|
|||
# (if empty: agent effective group)
|
||||
# metadata_proxy_group =
|
||||
|
||||
# Enable/Disable log watch by metadata proxy, it should be disabled when
|
||||
# metadata_proxy_user/group is not allowed to read/write its log file and
|
||||
# 'copytruncate' logrotate option must be used if logrotate is enabled on
|
||||
# metadata proxy log files. Option default value is deduced from
|
||||
# metadata_proxy_user: watch log is enabled if metadata_proxy_user is agent
|
||||
# effective user id/name.
|
||||
# metadata_proxy_watch_log =
|
||||
|
||||
# Location of Metadata Proxy UNIX domain socket
|
||||
# metadata_proxy_socket = $state_path/metadata_proxy
|
||||
# =========== end of items for metadata proxy configuration ==============
|
||||
|
|
|
@ -73,7 +73,7 @@ PROCESS_MONITOR_OPTS = [
|
|||
]
|
||||
|
||||
|
||||
def get_log_args(conf, log_file_name):
|
||||
def get_log_args(conf, log_file_name, **kwargs):
|
||||
cmd_args = []
|
||||
if conf.debug:
|
||||
cmd_args.append('--debug')
|
||||
|
@ -91,6 +91,8 @@ def get_log_args(conf, log_file_name):
|
|||
log_dir = os.path.dirname(conf.log_file)
|
||||
if log_dir:
|
||||
cmd_args.append('--log-dir=%s' % log_dir)
|
||||
if kwargs.get('metadata_proxy_watch_log') is False:
|
||||
cmd_args.append('--metadata_proxy_watch_log=false')
|
||||
else:
|
||||
if conf.use_syslog:
|
||||
cmd_args.append('--use-syslog')
|
||||
|
|
|
@ -15,6 +15,8 @@
|
|||
import atexit
|
||||
import fcntl
|
||||
import grp
|
||||
import logging as std_logging
|
||||
from logging import handlers
|
||||
import os
|
||||
import pwd
|
||||
import signal
|
||||
|
@ -56,6 +58,25 @@ def setgid(group_id_or_name):
|
|||
raise exceptions.FailToDropPrivilegesExit(msg)
|
||||
|
||||
|
||||
def unwatch_log():
|
||||
"""Replace WatchedFileHandler handlers by FileHandler ones.
|
||||
|
||||
Neutron logging uses WatchedFileHandler handlers but they do not
|
||||
support privileges drop, this method replaces them by FileHandler
|
||||
handlers supporting privileges drop.
|
||||
"""
|
||||
log_root = logging.getLogger(None).logger
|
||||
to_replace = [h for h in log_root.handlers
|
||||
if isinstance(h, handlers.WatchedFileHandler)]
|
||||
for handler in to_replace:
|
||||
new_handler = std_logging.FileHandler(handler.baseFilename,
|
||||
mode=handler.mode,
|
||||
encoding=handler.encoding,
|
||||
delay=handler.delay)
|
||||
log_root.removeHandler(handler)
|
||||
log_root.addHandler(new_handler)
|
||||
|
||||
|
||||
def drop_privileges(user=None, group=None):
|
||||
"""Drop privileges to user/group privileges."""
|
||||
if user is None and group is None:
|
||||
|
@ -136,7 +157,7 @@ class Daemon(object):
|
|||
"""
|
||||
def __init__(self, pidfile, stdin='/dev/null', stdout='/dev/null',
|
||||
stderr='/dev/null', procname='python', uuid=None,
|
||||
user=None, group=None):
|
||||
user=None, group=None, watch_log=True):
|
||||
self.stdin = stdin
|
||||
self.stdout = stdout
|
||||
self.stderr = stderr
|
||||
|
@ -144,6 +165,7 @@ class Daemon(object):
|
|||
self.pidfile = Pidfile(pidfile, procname, uuid)
|
||||
self.user = user
|
||||
self.group = group
|
||||
self.watch_log = watch_log
|
||||
|
||||
def _fork(self):
|
||||
try:
|
||||
|
@ -206,4 +228,6 @@ class Daemon(object):
|
|||
|
||||
start() will call this method after the process has daemonized.
|
||||
"""
|
||||
if not self.watch_log:
|
||||
unwatch_log()
|
||||
drop_privileges(self.user, self.group)
|
||||
|
|
|
@ -17,6 +17,7 @@ import fcntl
|
|||
import glob
|
||||
import httplib
|
||||
import os
|
||||
import pwd
|
||||
import shlex
|
||||
import socket
|
||||
import struct
|
||||
|
@ -334,6 +335,15 @@ def ensure_directory_exists_without_file(path):
|
|||
ensure_dir(dirname)
|
||||
|
||||
|
||||
def is_effective_user(user_id_or_name):
|
||||
"""Returns True if user_id_or_name is effective user (id/name)."""
|
||||
euid = os.geteuid()
|
||||
if str(user_id_or_name) == str(euid):
|
||||
return True
|
||||
effective_user_name = pwd.getpwuid(euid).pw_name
|
||||
return user_id_or_name == effective_user_name
|
||||
|
||||
|
||||
class UnixDomainHTTPConnection(httplib.HTTPConnection):
|
||||
"""Connection class for HTTP over UNIX domain socket."""
|
||||
def __init__(self, host, port=None, strict=None, timeout=None,
|
||||
|
|
|
@ -21,6 +21,7 @@ from oslo_log import log as logging
|
|||
from neutron.agent.common import config
|
||||
from neutron.agent.l3 import namespaces
|
||||
from neutron.agent.linux import external_process
|
||||
from neutron.agent.linux import utils
|
||||
from neutron.common import exceptions
|
||||
from neutron.services import advanced_service
|
||||
|
||||
|
@ -47,7 +48,18 @@ class MetadataDriver(advanced_service.AdvancedService):
|
|||
default='',
|
||||
help=_("Group (gid or name) running metadata proxy after "
|
||||
"its initialization (if empty: agent effective "
|
||||
"group)"))
|
||||
"group)")),
|
||||
cfg.BoolOpt('metadata_proxy_watch_log',
|
||||
default=None,
|
||||
help=_("Enable/Disable log watch by metadata proxy. It "
|
||||
"should be disabled when metadata_proxy_user/group "
|
||||
"is not allowed to read/write its log file and "
|
||||
"copytruncate logrotate option must be used if "
|
||||
"logrotate is enabled on metadata proxy log "
|
||||
"files. Option default value is deduced from "
|
||||
"metadata_proxy_user: watch log is enabled if "
|
||||
"metadata_proxy_user is agent effective user "
|
||||
"id/name.")),
|
||||
]
|
||||
|
||||
def __init__(self, l3_agent):
|
||||
|
@ -112,10 +124,17 @@ class MetadataDriver(advanced_service.AdvancedService):
|
|||
'port': port})]
|
||||
|
||||
@classmethod
|
||||
def _get_metadata_proxy_user_group(cls, conf):
|
||||
user = conf.metadata_proxy_user or os.geteuid()
|
||||
group = conf.metadata_proxy_group or os.getegid()
|
||||
return user, group
|
||||
def _get_metadata_proxy_user_group_watchlog(cls, conf):
|
||||
user = conf.metadata_proxy_user or str(os.geteuid())
|
||||
group = conf.metadata_proxy_group or str(os.getegid())
|
||||
|
||||
watch_log = conf.metadata_proxy_watch_log
|
||||
if watch_log is None:
|
||||
# NOTE(cbrandily): Commonly, log watching can be enabled only
|
||||
# when metadata proxy user is agent effective user (id/name).
|
||||
watch_log = utils.is_effective_user(user)
|
||||
|
||||
return user, group, watch_log
|
||||
|
||||
@classmethod
|
||||
def _get_metadata_proxy_callback(cls, port, conf, network_id=None,
|
||||
|
@ -131,7 +150,8 @@ class MetadataDriver(advanced_service.AdvancedService):
|
|||
|
||||
def callback(pid_file):
|
||||
metadata_proxy_socket = conf.metadata_proxy_socket
|
||||
user, group = cls._get_metadata_proxy_user_group(conf)
|
||||
user, group, watch_log = (
|
||||
cls._get_metadata_proxy_user_group_watchlog(conf))
|
||||
proxy_cmd = ['neutron-ns-metadata-proxy',
|
||||
'--pid_file=%s' % pid_file,
|
||||
'--metadata_proxy_socket=%s' % metadata_proxy_socket,
|
||||
|
@ -141,7 +161,8 @@ class MetadataDriver(advanced_service.AdvancedService):
|
|||
'--metadata_proxy_user=%s' % user,
|
||||
'--metadata_proxy_group=%s' % group]
|
||||
proxy_cmd.extend(config.get_log_args(
|
||||
conf, 'neutron-ns-metadata-proxy-%s.log' % uuid))
|
||||
conf, 'neutron-ns-metadata-proxy-%s.log' % uuid,
|
||||
metadata_proxy_watch_log=watch_log))
|
||||
return proxy_cmd
|
||||
|
||||
return callback
|
||||
|
|
|
@ -110,10 +110,10 @@ class NetworkMetadataProxyHandler(object):
|
|||
|
||||
class ProxyDaemon(daemon.Daemon):
|
||||
def __init__(self, pidfile, port, network_id=None, router_id=None,
|
||||
user=None, group=None):
|
||||
user=None, group=None, watch_log=True):
|
||||
uuid = network_id or router_id
|
||||
super(ProxyDaemon, self).__init__(pidfile, uuid=uuid, user=user,
|
||||
group=group)
|
||||
group=group, watch_log=watch_log)
|
||||
self.network_id = network_id
|
||||
self.router_id = router_id
|
||||
self.port = port
|
||||
|
@ -160,6 +160,11 @@ def main():
|
|||
default=None,
|
||||
help=_("Group (gid or name) running metadata proxy after "
|
||||
"its initialization")),
|
||||
cfg.BoolOpt('metadata_proxy_watch_log',
|
||||
default=True,
|
||||
help=_("Watch file log. Log watch should be disabled when "
|
||||
"metadata_proxy_user/group has no read/write "
|
||||
"permissions on metadata proxy log file.")),
|
||||
]
|
||||
|
||||
cfg.CONF.register_cli_opts(opts)
|
||||
|
@ -173,7 +178,8 @@ def main():
|
|||
network_id=cfg.CONF.network_id,
|
||||
router_id=cfg.CONF.router_id,
|
||||
user=cfg.CONF.metadata_proxy_user,
|
||||
group=cfg.CONF.metadata_proxy_group)
|
||||
group=cfg.CONF.metadata_proxy_group,
|
||||
watch_log=cfg.CONF.metadata_proxy_watch_log)
|
||||
|
||||
if cfg.CONF.daemonize:
|
||||
proxy.start()
|
||||
|
|
|
@ -210,7 +210,18 @@ class TestPathUtilities(base.BaseTestCase):
|
|||
self.assertFalse(utils.cmd_matches_expected('foo', 'bar'))
|
||||
|
||||
|
||||
class FakeUser(object):
|
||||
def __init__(self, name):
|
||||
self.pw_name = name
|
||||
|
||||
|
||||
class TestBaseOSUtils(base.BaseTestCase):
|
||||
|
||||
EUID = 123
|
||||
EUNAME = 'user'
|
||||
EGID = 456
|
||||
EGNAME = 'group'
|
||||
|
||||
@mock.patch.object(os.path, 'isdir', return_value=False)
|
||||
@mock.patch.object(os, 'makedirs')
|
||||
def test_ensure_dir_not_exist(self, makedirs, isdir):
|
||||
|
@ -225,6 +236,34 @@ class TestBaseOSUtils(base.BaseTestCase):
|
|||
isdir.assert_called_once_with('/the')
|
||||
self.assertFalse(makedirs.called)
|
||||
|
||||
@mock.patch('os.geteuid', return_value=EUID)
|
||||
@mock.patch('pwd.getpwuid', return_value=FakeUser(EUNAME))
|
||||
def test_is_effective_user_id(self, getpwuid, geteuid):
|
||||
self.assertTrue(utils.is_effective_user(self.EUID))
|
||||
geteuid.assert_called_once_with()
|
||||
self.assertFalse(getpwuid.called)
|
||||
|
||||
@mock.patch('os.geteuid', return_value=EUID)
|
||||
@mock.patch('pwd.getpwuid', return_value=FakeUser(EUNAME))
|
||||
def test_is_effective_user_str_id(self, getpwuid, geteuid):
|
||||
self.assertTrue(utils.is_effective_user(str(self.EUID)))
|
||||
geteuid.assert_called_once_with()
|
||||
self.assertFalse(getpwuid.called)
|
||||
|
||||
@mock.patch('os.geteuid', return_value=EUID)
|
||||
@mock.patch('pwd.getpwuid', return_value=FakeUser(EUNAME))
|
||||
def test_is_effective_user_name(self, getpwuid, geteuid):
|
||||
self.assertTrue(utils.is_effective_user(self.EUNAME))
|
||||
geteuid.assert_called_once_with()
|
||||
getpwuid.assert_called_once_with(self.EUID)
|
||||
|
||||
@mock.patch('os.geteuid', return_value=EUID)
|
||||
@mock.patch('pwd.getpwuid', return_value=FakeUser(EUNAME))
|
||||
def test_is_not_effective_user(self, getpwuid, geteuid):
|
||||
self.assertFalse(utils.is_effective_user('wrong'))
|
||||
geteuid.assert_called_once_with()
|
||||
getpwuid.assert_called_once_with(self.EUID)
|
||||
|
||||
|
||||
class TestUnixDomainHttpConnection(base.BaseTestCase):
|
||||
def test_connect(self):
|
||||
|
|
|
@ -61,6 +61,7 @@ class TestMetadataDriverProcess(base.BaseTestCase):
|
|||
|
||||
EUID = 123
|
||||
EGID = 456
|
||||
EUNAME = 'neutron'
|
||||
|
||||
def setUp(self):
|
||||
super(TestMetadataDriverProcess, self).setUp()
|
||||
|
@ -79,11 +80,13 @@ class TestMetadataDriverProcess(base.BaseTestCase):
|
|||
cfg.CONF.register_opts(metadata_driver.MetadataDriver.OPTS)
|
||||
|
||||
def _test_spawn_metadata_proxy(self, expected_user, expected_group,
|
||||
user='', group=''):
|
||||
user='', group='', watch_log=True):
|
||||
router_id = _uuid()
|
||||
router_ns = 'qrouter-%s' % router_id
|
||||
metadata_port = 8080
|
||||
ip_class_path = 'neutron.agent.linux.ip_lib.IPWrapper'
|
||||
is_effective_user = 'neutron.agent.linux.utils.is_effective_user'
|
||||
fake_is_effective_user = lambda x: x in [self.EUNAME, str(self.EUID)]
|
||||
|
||||
cfg.CONF.set_override('metadata_proxy_user', user)
|
||||
cfg.CONF.set_override('metadata_proxy_group', group)
|
||||
|
@ -94,42 +97,58 @@ class TestMetadataDriverProcess(base.BaseTestCase):
|
|||
with contextlib.nested(
|
||||
mock.patch('os.geteuid', return_value=self.EUID),
|
||||
mock.patch('os.getegid', return_value=self.EGID),
|
||||
mock.patch(ip_class_path)) as (geteuid, getegid, ip_mock):
|
||||
mock.patch(is_effective_user,
|
||||
side_effect=fake_is_effective_user),
|
||||
mock.patch(ip_class_path)) as (
|
||||
geteuid, getegid, is_effective_user, ip_mock):
|
||||
agent.metadata_driver.spawn_monitored_metadata_proxy(
|
||||
agent.process_monitor,
|
||||
router_ns,
|
||||
metadata_port,
|
||||
agent.conf,
|
||||
router_id=router_id)
|
||||
netns_execute_args = [
|
||||
'neutron-ns-metadata-proxy',
|
||||
mock.ANY,
|
||||
mock.ANY,
|
||||
'--router_id=%s' % router_id,
|
||||
mock.ANY,
|
||||
'--metadata_port=%s' % metadata_port,
|
||||
'--metadata_proxy_user=%s' % expected_user,
|
||||
'--metadata_proxy_group=%s' % expected_group,
|
||||
'--debug',
|
||||
'--verbose',
|
||||
'--log-file=neutron-ns-metadata-proxy-%s.log' %
|
||||
router_id]
|
||||
if not watch_log:
|
||||
netns_execute_args.append(
|
||||
'--metadata_proxy_watch_log=false')
|
||||
ip_mock.assert_has_calls([
|
||||
mock.call(namespace=router_ns),
|
||||
mock.call().netns.execute([
|
||||
'neutron-ns-metadata-proxy',
|
||||
mock.ANY,
|
||||
mock.ANY,
|
||||
'--router_id=%s' % router_id,
|
||||
mock.ANY,
|
||||
'--metadata_port=%s' % metadata_port,
|
||||
'--metadata_proxy_user=%s' % expected_user,
|
||||
'--metadata_proxy_group=%s' % expected_group,
|
||||
'--debug',
|
||||
'--verbose',
|
||||
'--log-file=neutron-ns-metadata-proxy-%s.log' %
|
||||
router_id
|
||||
], addl_env=None)
|
||||
mock.call().netns.execute(netns_execute_args, addl_env=None)
|
||||
])
|
||||
|
||||
def test_spawn_metadata_proxy_with_user(self):
|
||||
self._test_spawn_metadata_proxy('user', self.EGID, user='user')
|
||||
def test_spawn_metadata_proxy_with_agent_user(self):
|
||||
self._test_spawn_metadata_proxy(
|
||||
self.EUNAME, str(self.EGID), user=self.EUNAME)
|
||||
|
||||
def test_spawn_metadata_proxy_with_uid(self):
|
||||
self._test_spawn_metadata_proxy('321', self.EGID, user='321')
|
||||
def test_spawn_metadata_proxy_with_nonagent_user(self):
|
||||
self._test_spawn_metadata_proxy(
|
||||
'notneutron', str(self.EGID), user='notneutron', watch_log=False)
|
||||
|
||||
def test_spawn_metadata_proxy_with_agent_uid(self):
|
||||
self._test_spawn_metadata_proxy(
|
||||
str(self.EUID), str(self.EGID), user=str(self.EUID))
|
||||
|
||||
def test_spawn_metadata_proxy_with_nonagent_uid(self):
|
||||
self._test_spawn_metadata_proxy(
|
||||
'321', str(self.EGID), user='321', watch_log=False)
|
||||
|
||||
def test_spawn_metadata_proxy_with_group(self):
|
||||
self._test_spawn_metadata_proxy(self.EUID, 'group', group='group')
|
||||
self._test_spawn_metadata_proxy(str(self.EUID), 'group', group='group')
|
||||
|
||||
def test_spawn_metadata_proxy_with_gid(self):
|
||||
self._test_spawn_metadata_proxy(self.EUID, '654', group='654')
|
||||
self._test_spawn_metadata_proxy(str(self.EUID), '654', group='654')
|
||||
|
||||
def test_spawn_metadata_proxy(self):
|
||||
self._test_spawn_metadata_proxy(self.EUID, self.EGID)
|
||||
self._test_spawn_metadata_proxy(str(self.EUID), str(self.EGID))
|
||||
|
|
|
@ -292,7 +292,8 @@ class TestProxyDaemon(base.BaseTestCase):
|
|||
router_id='router_id',
|
||||
network_id=None,
|
||||
user=mock.ANY,
|
||||
group=mock.ANY),
|
||||
group=mock.ANY,
|
||||
watch_log=mock.ANY),
|
||||
mock.call().start()]
|
||||
)
|
||||
|
||||
|
@ -315,6 +316,7 @@ class TestProxyDaemon(base.BaseTestCase):
|
|||
router_id='router_id',
|
||||
network_id=None,
|
||||
user=mock.ANY,
|
||||
group=mock.ANY),
|
||||
group=mock.ANY,
|
||||
watch_log=mock.ANY),
|
||||
mock.call().run()]
|
||||
)
|
||||
|
|
Loading…
Reference in New Issue