From eca37d13eaf5bb9bcd439d44cc8cd55ecc868a19 Mon Sep 17 00:00:00 2001
From: Tristan Cacqueray <tdecacqu@redhat.com>
Date: Fri, 27 Apr 2018 07:11:57 +0000
Subject: [PATCH] builder: support setting diskimage env-vars in secure
 configuration

This change enables using diskimage-builder elements with secret securely.
For example, a rhel diskimage needs a REG_PASSWORD that could be define in
the secure file like so:

diskimages:
  - name: rhel-7
    env-vars:
      REG_PASSWORD: secret-password

Change-Id: I814318ae0b5c9e4665f3fa3f011d8a687b540fac
---
 doc/source/installation.rst                            | 10 ++++++++--
 nodepool/config.py                                     | 10 ++++++++++
 nodepool/tests/fixtures/secure_file_secure.yaml        |  5 +++++
 nodepool/tests/test_launcher.py                        |  4 ++++
 .../notes/secure-dib-env-c6013bab90406988.yaml         |  4 ++++
 5 files changed, 31 insertions(+), 2 deletions(-)
 create mode 100644 releasenotes/notes/secure-dib-env-c6013bab90406988.yaml

diff --git a/doc/source/installation.rst b/doc/source/installation.rst
index ecc49f516..4f6744b38 100644
--- a/doc/source/installation.rst
+++ b/doc/source/installation.rst
@@ -58,11 +58,17 @@ The Nodepool configuration file is described in :ref:`configuration`.
 
 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.
+specifying ZooKeeper credentials and diskimage env-vars.
+If ZooKeeper credentials or diskimage env-vars 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.
 
+Secrets stored in diskimage env-vars may be leaked by the elements or in
+the image build logs. Before using sensitive information in env-vars, please
+carefully audit the elements that are enabled and ensure they are handling
+the environment safely.
+
 There is an optional logging configuration file, specified with the ``-l``
 option. The logging configuration file can accept either:
 
diff --git a/nodepool/config.py b/nodepool/config.py
index 733ed3a64..714a25309 100755
--- a/nodepool/config.py
+++ b/nodepool/config.py
@@ -102,6 +102,14 @@ class Config(ConfigValue):
             d.username = diskimage.get('username', 'zuul')
             self.diskimages[d.name] = d
 
+    def setSecureDiskimageEnv(self, diskimages, secure_config_path):
+        for diskimage in diskimages:
+            if diskimage['name'] not in self.diskimages:
+                raise Exception('%s: unknown diskimage %s' %
+                                (secure_config_path, diskimage['name']))
+            self.diskimages[diskimage['name']].env_vars.update(
+                diskimage['env-vars'])
+
     def setLabels(self, labels_cfg):
         if not labels_cfg:
             return
@@ -222,3 +230,5 @@ def loadSecureConfig(config, secure_config_path):
 
     # TODO(Shrews): Support ZooKeeper auth
     config.setZooKeeperServers(secure.get('zookeeper-servers'))
+    config.setSecureDiskimageEnv(
+        secure.get('diskimages', []), secure_config_path)
diff --git a/nodepool/tests/fixtures/secure_file_secure.yaml b/nodepool/tests/fixtures/secure_file_secure.yaml
index 03a3444ca..3d1d26e91 100644
--- a/nodepool/tests/fixtures/secure_file_secure.yaml
+++ b/nodepool/tests/fixtures/secure_file_secure.yaml
@@ -2,3 +2,8 @@ zookeeper-servers:
   - host: {zookeeper_host}
     port: {zookeeper_port}
     chroot: {zookeeper_chroot}
+
+diskimages:
+  - name: fake-image
+    env-vars:
+      REG_PASSWORD: secret
diff --git a/nodepool/tests/test_launcher.py b/nodepool/tests/test_launcher.py
index 6fe4e3e82..8f7b797f3 100644
--- a/nodepool/tests/test_launcher.py
+++ b/nodepool/tests/test_launcher.py
@@ -1064,6 +1064,10 @@ class TestLauncher(tests.DBTestCase):
         pool.start()
         self.wait_for_config(pool)
 
+        fake_image = pool.config.diskimages['fake-image']
+        self.assertIn('REG_PASSWORD', fake_image.env_vars)
+        self.assertEqual('secret', fake_image.env_vars['REG_PASSWORD'])
+
         zk_servers = pool.config.zookeeper_servers
         self.assertEqual(1, len(zk_servers))
         key = list(zk_servers.keys())[0]
diff --git a/releasenotes/notes/secure-dib-env-c6013bab90406988.yaml b/releasenotes/notes/secure-dib-env-c6013bab90406988.yaml
new file mode 100644
index 000000000..696c17b8e
--- /dev/null
+++ b/releasenotes/notes/secure-dib-env-c6013bab90406988.yaml
@@ -0,0 +1,4 @@
+---
+features:
+  - |
+    Diskimages env-vars can be set in the secure.conf file.