Merge lp:~vila/charms/precise/tarmac/fix-template-expansion into lp:~canonical-ci-engineering/charms/precise/tarmac/trunk

[Vincent Ladeuil]

Fix template expansion that led to ~tarmac/.ssh/known_hosts corruption

This fixes the template expansion issue that led to ~tarmac/.ssh/known_hosts corruption with failing tests reproducing the issue.

An easier way would have been to add an additional newline in the templates (so one survive) but this is likely to regress.

[Evan (ev)]

Spot on. Thanks!

review: Approve

[lp:~vila/charms/precise/tarmac/fix-template-expansion]

64. By Vincent Ladeuil on 2014-11-14

Merge further fixes from the cleanup branch

=== modified file 'hooks/hooks.py'
--- hooks/hooks.py	2014-11-14 13:31:41 +0000
+++ hooks/hooks.py	2014-11-14 16:46:19 +0000
@@ -45,8 +45,10 @@
 
 
 def get_config_template(name):
-    from jinja2 import Environment, FileSystemLoader
-    template_env = Environment(loader=FileSystemLoader(get_template_dir()))
+    import jinja2
+    fs_loader = jinja2.FileSystemLoader(get_template_dir())
+    template_env = jinja2.Environment(loader=fs_loader,
+                                      keep_trailing_newline=True)
     return template_env.get_template(name)
 
 

=== modified file 'unit_tests/test_hooks.py'
--- unit_tests/test_hooks.py	2014-11-14 14:00:28 +0000
+++ unit_tests/test_hooks.py	2014-11-14 16:46:19 +0000
@@ -109,6 +109,7 @@
         with open(os.path.join(self.conf_dir, 'tarmac.conf')) as fp:
             expanded = fp.read()
         self.assertIn('log_file = {}/foo.log'.format(self.log_dir), expanded)
+        self.assertTrue(expanded.endswith('\n'))
 
     def test_branch_population(self):
         params = {'log_file': 'foo.log',
@@ -122,6 +123,7 @@
 
         self.assertIn('[lp:foo]', expanded)
         self.assertIn('check_command = /bin/false', expanded)
+        self.assertTrue(expanded.endswith('\n'))
 
     def test_cron_population(self):
         user = os.environ['USER']
@@ -135,6 +137,7 @@
         self.assertIn('TARMAC_CONFIG_HOME={}'.format(self.conf_dir), expanded)
         self.assertIn('OS_USERNAME=ev', expanded)
         self.assertIn('OS_PASSWORD=foo', expanded)
+        self.assertTrue(expanded.endswith('\n'))
 
     def test_credentials_population(self):
         params = {'oauth_consumer_key': 'A',
@@ -151,6 +154,26 @@
         self.assertIn('consumer_secret = \n', expanded)
         self.assertIn('access_token = B', expanded)
         self.assertIn('access_secret = C', expanded)
+        self.assertTrue(expanded.endswith('\n'))
+
+    def test_ssh_config_population(self):
+        params = {'user': os.environ['USER'],
+                  'group': grp.getgrgid(os.getgroups()[0]).gr_name,
+                 }
+        hooks.update_ssh_template(params)
+        with open(os.path.join(self.ssh_dir, 'config')) as fp:
+            expanded = fp.read()
+        self.assertTrue(expanded.endswith('\n'))
+
+    def test_ssh_known_hosts_population(self):
+        params = {'user': os.environ['USER'],
+                  'group': grp.getgrgid(os.getgroups()[0]).gr_name,
+                 }
+        hooks.update_ssh_template(params)
+        with open(os.path.join(self.ssh_dir, 'known_hosts')) as fp:
+            expanded = fp.read()
+        self.assertIn('bazaar.launchpad.net', expanded)
+        self.assertTrue(expanded.endswith('\n'))
 
 
 if __name__ == '__main__':