cloud-init-dev team mailing list archive

[Scott Moser <smoser@xxxxxxxxxx>]

Scott Moser has proposed merging lp:~smoser/cloud-init/trunk.1568940 into lp:cloud-init.

Commit message:
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.

Requested reviews:
  cloud init development team (cloud-init-dev)
Related bugs:
  Bug #1568940 in cloud-init: "validation_key in client.rb should be filepath not actual validation key content"
  https://bugs.launchpad.net/cloud-init/+bug/1568940

For more details, see:
https://code.launchpad.net/~smoser/cloud-init/trunk.1568940/+merge/291635
-- 
Your team cloud init development team is requested to review the proposed merge of lp:~smoser/cloud-init/trunk.1568940 into lp:cloud-init.

=== modified file 'cloudinit/config/cc_chef.py'
--- cloudinit/config/cc_chef.py	2016-04-04 19:17:28 +0000
+++ cloudinit/config/cc_chef.py	2016-04-12 14:47:21 +0000
@@ -38,8 +38,10 @@
     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 @@
       server_url:
       show_time:
       ssl_verify_mode:
+      validation_cert:
       validation_key:
       validation_name:
 """
@@ -105,6 +108,7 @@
     # 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 @@
     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')

=== modified file 'doc/examples/cloud-config-chef.txt'
--- doc/examples/cloud-config-chef.txt	2012-12-12 15:39:43 +0000
+++ doc/examples/cloud-config-chef.txt	2016-04-12 14:47:21 +0000
@@ -67,7 +67,9 @@
 
  # 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-----

=== modified file 'templates/chef_client.rb.tmpl'
--- templates/chef_client.rb.tmpl	2014-10-11 23:59:50 +0000
+++ templates/chef_client.rb.tmpl	2016-04-12 14:47:21 +0000
@@ -26,7 +26,7 @@
 {% if validation_name %}
 validation_client_name "{{validation_name}}"
 {% endif %}
-{% if validation_key %}
+{% if validation_cert %}
 validation_key         "{{validation_key}}"
 {% endif %}
 {% if client_key %}

=== modified file 'tests/unittests/test_handler/test_handler_chef.py'
--- tests/unittests/test_handler/test_handler_chef.py	2015-02-10 20:32:32 +0000
+++ tests/unittests/test_handler/test_handler_chef.py	2016-04-12 14:47:21 +0000
@@ -75,17 +75,28 @@
             '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 @@
         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))