Add support for a secure ZooKeeper configuration

The secure config file has largely been unused and ignored for v3.
This add support for reading ZooKeeper credentials from the secure
file. Note that actually specifying authentication credentials is
left for future work, but this adds the framework necessary for that.

ZooKeeper creds can be in both the normal config file and the secure
file. If specified in both, the data in the secure configuration wins.

Change-Id: I5d9c12c00f5e85ef258128337cdb99809f86b8ed
This commit is contained in:
David Shrewsbury 2018-01-08 13:53:17 -05:00
parent 007ba7e660
commit 477a40044b
10 changed files with 163 additions and 40 deletions

View File

@ -56,9 +56,12 @@ Nodepool has one required configuration file, which defaults to
``/etc/nodepool/nodepool.yaml``. This can be changed with the ``-c`` option.
The Nodepool configuration file is described in :ref:`configuration`.
Although there is support for a secure file that is used to store nodepool
configurations that contain sensitive data, this is currently not used, but
may be in the future.
There is support for a secure file that is used to store nodepool
configurations that contain sensitive data. It currently only supports
specifying ZooKeeper credentials. If ZooKeeper credentials are defined in
both configuration files, the data in the secure file takes precedence.
The secure file location can be changed with the ``-s`` option and follows
the same file format as the Nodepool configuration file.
There is an optional logging configuration file, specified with the ``-l``
option. The logging configuration file can accept either:

View File

@ -108,13 +108,14 @@ class DibImageFile(object):
class BaseWorker(threading.Thread):
def __init__(self, builder_id, config_path, interval, zk):
def __init__(self, builder_id, config_path, secure_path, interval, zk):
super(BaseWorker, self).__init__()
self.log = logging.getLogger("nodepool.builder.BaseWorker")
self.daemon = True
self._running = False
self._config = None
self._config_path = config_path
self._secure_path = secure_path
self._zk = zk
self._hostname = socket.gethostname()
self._statsd = stats.get_client()
@ -146,9 +147,10 @@ class CleanupWorker(BaseWorker):
and any local DIB builds.
'''
def __init__(self, name, builder_id, config_path, interval, zk):
def __init__(self, name, builder_id, config_path, secure_path,
interval, zk):
super(CleanupWorker, self).__init__(builder_id, config_path,
interval, zk)
secure_path, interval, zk)
self.log = logging.getLogger("nodepool.builder.CleanupWorker.%s" % name)
self.name = 'CleanupWorker.%s' % name
@ -507,6 +509,8 @@ class CleanupWorker(BaseWorker):
Body of run method for exception handling purposes.
'''
new_config = nodepool_config.loadConfig(self._config_path)
if self._secure_path:
nodepool_config.loadSecureConfig(new_config, self._secure_path)
if not self._config:
self._config = new_config
@ -519,8 +523,9 @@ class CleanupWorker(BaseWorker):
class BuildWorker(BaseWorker):
def __init__(self, name, builder_id, config_path, interval, zk, dib_cmd):
super(BuildWorker, self).__init__(builder_id, config_path,
def __init__(self, name, builder_id, config_path, secure_path,
interval, zk, dib_cmd):
super(BuildWorker, self).__init__(builder_id, config_path, secure_path,
interval, zk)
self.log = logging.getLogger("nodepool.builder.BuildWorker.%s" % name)
self.name = 'BuildWorker.%s' % name
@ -781,6 +786,8 @@ class BuildWorker(BaseWorker):
'''
# NOTE: For the first iteration, we expect self._config to be None
new_config = nodepool_config.loadConfig(self._config_path)
if self._secure_path:
nodepool_config.loadSecureConfig(new_config, self._secure_path)
if not self._config:
self._config = new_config
@ -792,9 +799,10 @@ class BuildWorker(BaseWorker):
class UploadWorker(BaseWorker):
def __init__(self, name, builder_id, config_path, interval, zk):
def __init__(self, name, builder_id, config_path, secure_path,
interval, zk):
super(UploadWorker, self).__init__(builder_id, config_path,
interval, zk)
secure_path, interval, zk)
self.log = logging.getLogger("nodepool.builder.UploadWorker.%s" % name)
self.name = 'UploadWorker.%s' % name
@ -803,6 +811,8 @@ class UploadWorker(BaseWorker):
Reload the nodepool configuration file.
'''
new_config = nodepool_config.loadConfig(self._config_path)
if self._secure_path:
nodepool_config.loadSecureConfig(new_config, self._secure_path)
if not self._config:
self._config = new_config
@ -1039,17 +1049,19 @@ class NodePoolBuilder(object):
'''
log = logging.getLogger("nodepool.builder.NodePoolBuilder")
def __init__(self, config_path, num_builders=1, num_uploaders=4,
fake=False):
def __init__(self, config_path, secure_path=None,
num_builders=1, num_uploaders=4, fake=False):
'''
Initialize the NodePoolBuilder object.
:param str config_path: Path to configuration file.
:param str secure_path: Path to secure configuration file.
:param int num_builders: Number of build workers to start.
:param int num_uploaders: Number of upload workers to start.
:param bool fake: Whether to fake the image builds.
'''
self._config_path = config_path
self._secure_path = secure_path
self._config = None
self._num_builders = num_builders
self._build_workers = []
@ -1090,6 +1102,8 @@ class NodePoolBuilder(object):
def _getAndValidateConfig(self):
config = nodepool_config.loadConfig(self._config_path)
if self._secure_path:
nodepool_config.loadSecureConfig(config, self._secure_path)
if not config.zookeeper_servers.values():
raise RuntimeError('No ZooKeeper servers specified in config.')
if not config.imagesdir:
@ -1127,20 +1141,23 @@ class NodePoolBuilder(object):
# Create build and upload worker objects
for i in range(self._num_builders):
w = BuildWorker(i, builder_id, self._config_path,
w = BuildWorker(i, builder_id,
self._config_path, self._secure_path,
self.build_interval, self.zk, self.dib_cmd)
w.start()
self._build_workers.append(w)
for i in range(self._num_uploaders):
w = UploadWorker(i, builder_id, self._config_path,
w = UploadWorker(i, builder_id,
self._config_path, self._secure_path,
self.upload_interval, self.zk)
w.start()
self._upload_workers.append(w)
if self.cleanup_interval > 0:
self._janitor = CleanupWorker(
0, builder_id, self._config_path,
0, builder_id,
self._config_path, self._secure_path,
self.cleanup_interval, self.zk)
self._janitor.start()

View File

@ -34,6 +34,8 @@ class NodePoolBuilderApp(nodepool.cmd.NodepoolDaemonApp):
parser.add_argument('-c', dest='config',
default='/etc/nodepool/nodepool.yaml',
help='path to config file')
parser.add_argument('-s', dest='secure',
help='path to secure config file')
parser.add_argument('--build-workers', dest='build_workers',
default=1, help='number of build workers',
type=int)
@ -46,10 +48,12 @@ class NodePoolBuilderApp(nodepool.cmd.NodepoolDaemonApp):
return parser
def run(self):
self.nb = builder.NodePoolBuilder(self.args.config,
self.args.build_workers,
self.args.upload_workers,
self.args.fake)
self.nb = builder.NodePoolBuilder(
self.args.config,
secure_path=self.args.secure,
num_builders=self.args.build_workers,
num_uploaders=self.args.upload_workers,
fake=self.args.fake)
signal.signal(signal.SIGINT, self.sigint_handler)

View File

@ -16,7 +16,6 @@
# See the License for the specific language governing permissions and
# limitations under the License.
from six.moves import configparser as ConfigParser
import time
import yaml
@ -66,7 +65,7 @@ def get_provider_config(provider):
return OpenStackProviderConfig(provider)
def loadConfig(config_path):
def openConfig(path):
retry = 3
# Since some nodepool code attempts to dynamically re-read its config
@ -75,7 +74,7 @@ def loadConfig(config_path):
# attempt to reload it.
while True:
try:
config = yaml.load(open(config_path))
config = yaml.load(open(path))
break
except IOError as e:
if e.errno == 2:
@ -85,6 +84,11 @@ def loadConfig(config_path):
raise e
if retry == 0:
raise e
return config
def loadConfig(config_path):
config = openConfig(config_path)
# Reset the shared os_client_config instance
OpenStackProviderConfig.os_client_config = None
@ -126,8 +130,6 @@ def loadConfig(config_path):
d.rebuild_age = int(diskimage.get('rebuild-age', 86400))
d.env_vars = diskimage.get('env-vars', {})
if not isinstance(d.env_vars, dict):
#self.log.error("%s: ignoring env-vars; "
# "should be a dict" % d.name)
d.env_vars = {}
d.image_types = set(diskimage.get('formats', []))
d.pause = bool(diskimage.get('pause', False))
@ -149,7 +151,18 @@ def loadConfig(config_path):
def loadSecureConfig(config, secure_config_path):
secure = ConfigParser.ConfigParser()
secure.readfp(open(secure_config_path))
secure = openConfig(secure_config_path)
if not secure: # empty file
return
#config.dburi = secure.get('database', 'dburi')
# Eliminate any servers defined in the normal config
if secure.get('zookeeper-servers', []):
config.zookeeper_servers = {}
# TODO(Shrews): Support ZooKeeper auth
for server in secure.get('zookeeper-servers', []):
z = zk.ZooKeeperConnectionConfig(server['host'],
server.get('port', 2181),
server.get('chroot', None))
name = z.host + '_' + str(z.port)
config.zookeeper_servers[name] = z

View File

@ -254,15 +254,17 @@ class BaseTestCase(testtools.TestCase):
class BuilderFixture(fixtures.Fixture):
def __init__(self, configfile, cleanup_interval):
def __init__(self, configfile, cleanup_interval, securefile=None):
super(BuilderFixture, self).__init__()
self.configfile = configfile
self.securefile = securefile
self.cleanup_interval = cleanup_interval
self.builder = None
def setUp(self):
super(BuilderFixture, self).setUp()
self.builder = builder.NodePoolBuilder(self.configfile)
self.builder = builder.NodePoolBuilder(
self.configfile, secure_path=self.securefile)
self.builder.cleanup_interval = self.cleanup_interval
self.builder.build_interval = .1
self.builder.upload_interval = .1
@ -278,7 +280,6 @@ class DBTestCase(BaseTestCase):
def setUp(self):
super(DBTestCase, self).setUp()
self.log = logging.getLogger("tests")
self.secure_conf = self._setup_secure()
self.setupZK()
def setup_config(self, filename, images_dir=None):
@ -306,15 +307,18 @@ class DBTestCase(BaseTestCase):
new_configfile = self.setup_config(filename, self._config_images_dir)
os.rename(new_configfile, configfile)
def _setup_secure(self):
def setup_secure(self, filename):
# replace entries in secure.conf
configfile = os.path.join(os.path.dirname(__file__),
'fixtures', 'secure.conf')
'fixtures', filename)
(fd, path) = tempfile.mkstemp()
with open(configfile, 'rb') as conf_fd:
config = conf_fd.read()
os.write(fd, config)
#os.write(fd, config.format(dburi=self.dburi))
config = conf_fd.read().decode('utf8')
data = config.format(
zookeeper_host=self.zookeeper_host,
zookeeper_port=self.zookeeper_port,
zookeeper_chroot=self.zookeeper_chroot)
os.write(fd, data.encode('utf8'))
os.close(fd)
return path
@ -458,7 +462,8 @@ class DBTestCase(BaseTestCase):
return req
def useNodepool(self, *args, **kwargs):
args = (self.secure_conf,) + args
secure_conf = kwargs.pop('secure_conf', None)
args = (secure_conf,) + args
pool = launcher.NodePool(*args, **kwargs)
pool.cleanup_interval = .5
pool.delete_interval = .5
@ -470,8 +475,10 @@ class DBTestCase(BaseTestCase):
self.addCleanup(app.stop)
return app
def _useBuilder(self, configfile, cleanup_interval=.5):
self.useFixture(BuilderFixture(configfile, cleanup_interval))
def _useBuilder(self, configfile, securefile=None, cleanup_interval=.5):
self.useFixture(
BuilderFixture(configfile, cleanup_interval, securefile)
)
def setupZK(self):
f = ZookeeperServerFixture()

View File

@ -1 +0,0 @@
# Empty

View File

@ -0,0 +1,47 @@
elements-dir: .
images-dir: '{images_dir}'
zookeeper-servers:
- host: invalid_host
port: 1
chroot: invalid_chroot
labels:
- name: fake-label
min-ready: 1
providers:
- name: fake-provider
cloud: fake
driver: fake
region-name: fake-region
rate: 0.0001
diskimages:
- name: fake-image
meta:
key: value
key2: value
pools:
- name: main
max-servers: 96
availability-zones:
- az1
networks:
- net-name
labels:
- name: fake-label
diskimage: fake-image
min-ram: 8192
flavor-name: 'Fake'
diskimages:
- name: fake-image
elements:
- fedora
- vm
release: 21
env-vars:
TMPDIR: /opt/dib_tmp
DIB_IMAGE_CACHE: /opt/dib_cache
DIB_CLOUD_IMAGES: http://download.fedoraproject.org/pub/fedora/linux/releases/test/21-Beta/Cloud/Images/x86_64/
BASE_IMAGE_FILE: Fedora-Cloud-Base-20141029-21_Beta.x86_64.qcow2

View File

@ -0,0 +1,4 @@
zookeeper-servers:
- host: {zookeeper_host}
port: {zookeeper_port}
chroot: {zookeeper_chroot}

View File

@ -31,7 +31,7 @@ class TestNodepoolCMD(tests.DBTestCase):
super(TestNodepoolCMD, self).setUp()
def patch_argv(self, *args):
argv = ["nodepool", "-s", self.secure_conf]
argv = ["nodepool"]
argv.extend(args)
self.useFixture(fixtures.MonkeyPatch('sys.argv', argv))

View File

@ -862,3 +862,32 @@ class TestLauncher(tests.DBTestCase):
self.assertEqual('fake', label3_nodes[0].public_ipv4)
self.assertEqual('', label3_nodes[0].public_ipv6)
self.assertEqual('fake', label3_nodes[0].interface_ip)
def test_secure_file(self):
"""Test using secure.conf file"""
configfile = self.setup_config('secure_file_config.yaml')
securefile = self.setup_secure('secure_file_secure.yaml')
pool = self.useNodepool(
configfile,
secure_conf=securefile,
watermark_sleep=1)
self._useBuilder(configfile, securefile=securefile)
pool.start()
self.wait_for_config(pool)
zk_servers = pool.config.zookeeper_servers
self.assertEqual(1, len(zk_servers))
key = list(zk_servers.keys())[0]
self.assertEqual(self.zookeeper_host, zk_servers[key].host)
self.assertEqual(self.zookeeper_port, zk_servers[key].port)
self.assertEqual(self.zookeeper_chroot, zk_servers[key].chroot)
image = self.waitForImage('fake-provider', 'fake-image')
self.assertEqual(image.username, 'zuul')
nodes = self.waitForNodes('fake-label')
self.assertEqual(len(nodes), 1)
self.assertEqual(nodes[0].provider, 'fake-provider')
self.assertEqual(nodes[0].type, 'fake-label')
self.assertEqual(nodes[0].username, 'zuul')
self.assertNotEqual(nodes[0].host_keys, [])