diff --git a/doc/source/installation.rst b/doc/source/installation.rst index aa33d271f..f258e1c70 100644 --- a/doc/source/installation.rst +++ b/doc/source/installation.rst @@ -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: diff --git a/nodepool/builder.py b/nodepool/builder.py index 60f7dd59b..744b4bc08 100644 --- a/nodepool/builder.py +++ b/nodepool/builder.py @@ -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() diff --git a/nodepool/cmd/builder.py b/nodepool/cmd/builder.py index 1138cba5f..7ac993b2a 100644 --- a/nodepool/cmd/builder.py +++ b/nodepool/cmd/builder.py @@ -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) diff --git a/nodepool/config.py b/nodepool/config.py index 537dfbc23..d9a710d6d 100755 --- a/nodepool/config.py +++ b/nodepool/config.py @@ -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 diff --git a/nodepool/tests/__init__.py b/nodepool/tests/__init__.py index 2d299d3e9..77aad1293 100644 --- a/nodepool/tests/__init__.py +++ b/nodepool/tests/__init__.py @@ -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() diff --git a/nodepool/tests/fixtures/secure.conf b/nodepool/tests/fixtures/secure.conf deleted file mode 100644 index b7db25411..000000000 --- a/nodepool/tests/fixtures/secure.conf +++ /dev/null @@ -1 +0,0 @@ -# Empty diff --git a/nodepool/tests/fixtures/secure_file_config.yaml b/nodepool/tests/fixtures/secure_file_config.yaml new file mode 100644 index 000000000..094284a32 --- /dev/null +++ b/nodepool/tests/fixtures/secure_file_config.yaml @@ -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 diff --git a/nodepool/tests/fixtures/secure_file_secure.yaml b/nodepool/tests/fixtures/secure_file_secure.yaml new file mode 100644 index 000000000..03a3444ca --- /dev/null +++ b/nodepool/tests/fixtures/secure_file_secure.yaml @@ -0,0 +1,4 @@ +zookeeper-servers: + - host: {zookeeper_host} + port: {zookeeper_port} + chroot: {zookeeper_chroot} diff --git a/nodepool/tests/test_commands.py b/nodepool/tests/test_commands.py index c9ee8993b..fe7829bed 100644 --- a/nodepool/tests/test_commands.py +++ b/nodepool/tests/test_commands.py @@ -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)) diff --git a/nodepool/tests/test_launcher.py b/nodepool/tests/test_launcher.py index 9b2fda2bc..975f10fcb 100644 --- a/nodepool/tests/test_launcher.py +++ b/nodepool/tests/test_launcher.py @@ -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, [])