chef: straighten out validation_cert and validation_key

Now, validation_key is always a path to a file, as it is in
chef's client.rb syntax.

validation_cert is always the *content* of that file that should
be written. However, if validation_cert is the string "system",
then we do not write that value, but rather assume the file exists.


LP: #1568940

5 files changed, 85 insertions, 13 deletions

diff --git a/ChangeLog b/ChangeLog
index d12ad501..d7c2fc36 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -103,6 +103,7 @@
  - chef: fix chef install from gems (LP: #1553345)
  - systemd: do not specify After of obsolete syslog.target (LP: #1536964)
  - centos: Ensure that resolve conf object is written as a str (LP: #1479988)
+ - chef: straighten out validation_cert and validation_key (LP: #1568940)
 
 0.7.6:
  - open 0.7.6
diff --git a/cloudinit/config/cc_chef.py b/cloudinit/config/cc_chef.py
index 28711a59..4c28be6a 100644
--- a/cloudinit/config/cc_chef.py
+++ b/cloudinit/config/cc_chef.py
@@ -38,8 +38,10 @@ It can be configured with the following option structure::
     chef:
        directories: (defaulting to /etc/chef, /var/log/chef, /var/lib/chef,
                      /var/cache/chef, /var/backups/chef, /var/run/chef)
-       validation_key or validation_cert: (optional string to be written to
-                                           /etc/chef/validation.pem)
+       validation_cert: (optional string to be written to file validation_key)
+                        special value 'system' means set use existing file
+       validation_key: (optional the path for validation_cert. default
+                        /etc/chef/validation.pem)
        firstboot_path: (path to write run_list and initial_attributes keys that
                         should also be present in this configuration, defaults
                         to /etc/chef/firstboot.json)
@@ -64,6 +66,7 @@ It can be configured with the following option structure::
       server_url:
       show_time:
       ssl_verify_mode:
+      validation_cert:
       validation_key:
       validation_name:
 """
@@ -105,6 +108,7 @@ CHEF_RB_TPL_DEFAULTS = {
     # These are not symbols...
     'log_location': '/var/log/chef/client.log',
     'validation_key': CHEF_VALIDATION_PEM_PATH,
+    'validation_cert': None,
     'client_key': "/etc/chef/client.pem",
     'json_attribs': CHEF_FB_PATH,
     'file_cache_path': "/var/cache/chef",
@@ -201,13 +205,17 @@ def handle(name, cfg, cloud, log, _args):
     for d in itertools.chain(chef_dirs, REQUIRED_CHEF_DIRS):
         util.ensure_dir(d)
 
-    # Set the validation key based on the presence of either 'validation_key'
-    # or 'validation_cert'. In the case where both exist, 'validation_key'
-    # takes precedence
-    for key in ('validation_key', 'validation_cert'):
-        if key in chef_cfg and chef_cfg[key]:
-            util.write_file(CHEF_VALIDATION_PEM_PATH, chef_cfg[key])
-            break
+    vkey_path = chef_cfg.get('validation_key', CHEF_VALIDATION_PEM_PATH)
+    vcert = chef_cfg.get('validation_cert')
+    # special value 'system' means do not overwrite the file
+    # but still render the template to contain 'validation_key'
+    if vcert:
+        if vcert != "system":
+            util.write_file(vkey_path, vcert)
+        elif not os.path.isfile(vkey_path):
+            log.warn("chef validation_cert provided as 'system', but "
+                     "validation_key path '%s' does not exist.",
+                     vkey_path)
 
     # Create the chef config from template
     template_fn = cloud.get_template_filename('chef_client.rb')
diff --git a/doc/examples/cloud-config-chef.txt b/doc/examples/cloud-config-chef.txt
index 4edad653..b886cba2 100644
--- a/doc/examples/cloud-config-chef.txt
+++ b/doc/examples/cloud-config-chef.txt
@@ -67,7 +67,9 @@ chef:
 
  # Default validation name is chef-validator
  validation_name: "yourorg-validator"
- validation_key: |
+ # if validation_cert's value is "system" then it is expected
+ # that the file already exists on the system.
+ validation_cert: |
      -----BEGIN RSA PRIVATE KEY-----
      YOUR-ORGS-VALIDATION-KEY-HERE
      -----END RSA PRIVATE KEY-----
diff --git a/templates/chef_client.rb.tmpl b/templates/chef_client.rb.tmpl
index c4069d22..cbb6b15f 100644
--- a/templates/chef_client.rb.tmpl
+++ b/templates/chef_client.rb.tmpl
@@ -26,7 +26,7 @@ log_location           "{{log_location}}"
 {% if validation_name %}
 validation_client_name "{{validation_name}}"
 {% endif %}
-{% if validation_key %}
+{% if validation_cert %}
 validation_key         "{{validation_key}}"
 {% endif %}
 {% if client_key %}
diff --git a/tests/unittests/test_handler/test_handler_chef.py b/tests/unittests/test_handler/test_handler_chef.py
index edad88cb..7763f23b 100644
--- a/tests/unittests/test_handler/test_handler_chef.py
+++ b/tests/unittests/test_handler/test_handler_chef.py
@@ -75,17 +75,28 @@ class TestChef(t_help.FilesystemMockingTestCase):
             'chef': {
                 'server_url': 'localhost',
                 'validation_name': 'bob',
+                'validation_key': "/etc/chef/vkey.pem",
+                'validation_cert': "this is my cert",
             },
         }
         cc_chef.handle('chef', cfg, self.fetch_cloud('ubuntu'), LOG, [])
         for d in cc_chef.CHEF_DIRS:
             self.assertTrue(os.path.isdir(d))
         c = util.load_file(cc_chef.CHEF_RB_PATH)
+
+        # the content of these keys is not expected to be rendered to tmpl
+        unrendered_keys = ('validation_cert',)
         for k, v in cfg['chef'].items():
+            if k in unrendered_keys:
+                continue
             self.assertIn(v, c)
         for k, v in cc_chef.CHEF_RB_TPL_DEFAULTS.items():
-            if isinstance(v, six.string_types):
-                self.assertIn(v, c)
+            if k in unrendered_keys:
+                continue
+            # the value from the cfg overrides that in the default
+            val = cfg['chef'].get(k, v)
+            if isinstance(val, six.string_types):
+                self.assertIn(val, c)
         c = util.load_file(cc_chef.CHEF_FB_PATH)
         self.assertEqual({}, json.loads(c))
 
@@ -131,3 +142,53 @@ class TestChef(t_help.FilesystemMockingTestCase):
         c = util.load_file(cc_chef.CHEF_RB_PATH)
         self.assertNotIn('json_attribs', c)
         self.assertNotIn('Formatter.show_time', c)
+
+    @t_help.skipIf(not os.path.isfile(CLIENT_TEMPL),
+                   CLIENT_TEMPL + " is not available")
+    def test_validation_cert_and_validation_key(self):
+        # test validation_cert content is written to validation_key path
+        tpl_file = util.load_file('templates/chef_client.rb.tmpl')
+        self.patchUtils(self.tmp)
+        self.patchOS(self.tmp)
+
+        util.write_file('/etc/cloud/templates/chef_client.rb.tmpl', tpl_file)
+        v_path = '/etc/chef/vkey.pem'
+        v_cert = 'this is my cert'
+        cfg = {
+            'chef': {
+                'server_url': 'localhost',
+                'validation_name': 'bob',
+                'validation_key': v_path,
+                'validation_cert': v_cert
+            },
+        }
+        cc_chef.handle('chef', cfg, self.fetch_cloud('ubuntu'), LOG, [])
+        content = util.load_file(cc_chef.CHEF_RB_PATH)
+        self.assertIn(v_path, content)
+        util.load_file(v_path)
+        self.assertEqual(v_cert, util.load_file(v_path))
+
+    def test_validation_cert_with_system(self):
+        # test validation_cert content is not written over system file
+        tpl_file = util.load_file('templates/chef_client.rb.tmpl')
+        self.patchUtils(self.tmp)
+        self.patchOS(self.tmp)
+
+        v_path = '/etc/chef/vkey.pem'
+        v_cert = "system"
+        expected_cert = "this is the system file certificate"
+        cfg = {
+            'chef': {
+                'server_url': 'localhost',
+                'validation_name': 'bob',
+                'validation_key': v_path,
+                'validation_cert': v_cert
+            },
+        }
+        util.write_file('/etc/cloud/templates/chef_client.rb.tmpl', tpl_file)
+        util.write_file(v_path, expected_cert)
+        cc_chef.handle('chef', cfg, self.fetch_cloud('ubuntu'), LOG, [])
+        content = util.load_file(cc_chef.CHEF_RB_PATH)
+        self.assertIn(v_path, content)
+        util.load_file(v_path)
+        self.assertEqual(expected_cert, util.load_file(v_path))