Merge lp:~abentley/juju-core/sign-branch into lp:~juju-qa/juju-core/cd-release-juju

[Aaron Bentley]

Add sign_branch script.

This branch adds the sign_branch script to cd-release-juju.

It takes as its arguments an unsigned branch, a signed branch, and a signing key.

It checks out a local copy of the signed branch, merges the unsigned branch into it, signs and commits it.

[Curtis Hovey (sinzui)]

I have two questions inline.

review: Needs Information (code)

[Aaron Bentley (abentley)]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

>> + if not (filename.endswith('.sjson') or +
>> filename.endswith('.json.gpg')): + continue +
>> os.unlink(os.path.join(dirpath, filename))
>
> I don't think we need the continue statement when the if-statement
> looks for truth if (filename.endswith('.sjson') or
> filename.endswith('.json.gpg')): os.unlink(os.path.join(dirpath,
> filename))

Okay.

>> + def sign_branch(self): + self.bzr.bzr('checkout',
>> self.signed_branch, self.temp_branch) + self.merge() +
>> self.bzr.bzr('commit', self.temp_branch, '-m', 'Merged unsigned
>> json') + self.sign_metadata() + self.bzr.bzr('add',
>> self.temp_branch)
>
> If we were to stop making the ":" files, would bzr raise an error
> if it saw the signed ":" missing?

No. The sjson & gpg would be deleted by loop you commented on
previously. When Bazaar commits, it will notice that several
versioned files have been deleted, and unversion them.

We wouldn't want to use "bzr rm" instead of os.unlink, because in most
cases, the sjson will be replaced and we should not consider the
replacement to be a brand-new-file.

There isn't, AFAICT, a bzr command to "bzr rm" any files that have
already been deleted.

Of course, I could write this as a bzrlib client, but I thought it
would be shorter and easier to maintain using the commandline.

Aaron
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

iQEcBAEBCAAGBQJWsQyKAAoJEK84cMOcf+9heHoIAKOUAYNue6nlq3pvjPQ++O8y
/jtnHaq0OvLccG1+skfvnOC5la27n/bEYdwgeVovxHo8UY6Xrf3CAY6vT9hnlyTd
3noWqd2tJLogv19ia9XNQwtvLm/ocFyAVTD1XYDZh7tgq1/Rhb3UFwFMhEstJeCU
aXEKHVHFFMzLpA7E0VDi4K9Bqhg31Q1k/PEXd/lFhPKv1XFLSu5c+zy+Kmq4/eM+
BaotTD3jGeFvIY/ChkWfO42z7pTZaKRUiMCnjRdkZ/LmHTNB3PvdvSKkCnqH6xdX
Gxfk5h1XveqLndiGNrFDNuHfQRFLqFGOiGZZOKmJaoCu5Xn+3rGmHUUICcxxabI=
=RZgW
-----END PGP SIGNATURE-----

[Curtis Hovey (sinzui)]

Thank you.

review: Approve (code)

=== added file 'sign_branch.py'
--- sign_branch.py	1970-01-01 00:00:00 +0000
+++ sign_branch.py	2016-02-02 19:18:04 +0000
@@ -0,0 +1,90 @@
+#!/usr/bin/env python
+from argparse import ArgumentParser
+import os
+import subprocess
+import sys
+
+from sign_metadata import sign_metadata
+from utility import temp_dir
+
+
+class RunBzr:
+
+    def __init__(self, bzr_home=None, quiet=False):
+        self.bzr_home = bzr_home
+        self.quiet = quiet
+
+    def bzr(self, command, *args):
+        cc_args = ['bzr', '--no-aliases']
+        cc_args.append(command)
+        cc_args.extend(args)
+        env = dict(os.environ)
+        if self.bzr_home is not None:
+            env['BZR_HOME'] = self.bzr_home
+        with open('/dev/null', 'w') as null:
+            if self.quiet:
+                err = null
+                out = null
+            else:
+                err = None
+                out = None
+            subprocess.check_call(cc_args, env=env, stderr=err, stdout=out)
+
+
+class SignBranch:
+
+    def __init__(self, unsigned_branch, signed_branch, temp_branch,
+                 signing_key, signing_passphrase_file, bzr):
+        self.unsigned_branch = unsigned_branch
+        self.signed_branch = signed_branch
+        self.temp_branch = temp_branch
+        self.signing_key = signing_key
+        self.signing_passphrase_file = signing_passphrase_file
+        self.bzr = bzr
+
+    def merge(self):
+        self.bzr.bzr('merge', '-d', self.temp_branch, self.unsigned_branch)
+        # Ensure nothing left behind.
+        self.bzr.bzr('clean-tree', '-d', self.temp_branch, '--force',
+                     '--ignored', '--unknown')
+
+    def sign_metadata(self):
+        for dirpath, dirnames, filenames in os.walk(self.temp_branch):
+            for filename in filenames:
+                if not (filename.endswith('.sjson') or
+                        filename.endswith('.json.gpg')):
+                    continue
+                os.unlink(os.path.join(dirpath, filename))
+        subdir = os.path.join(self.temp_branch, 'v1')
+        sign_metadata(self.signing_key, subdir, self.signing_passphrase_file)
+
+    def sign_branch(self):
+        self.bzr.bzr('checkout', self.signed_branch, self.temp_branch)
+        self.merge()
+        self.bzr.bzr('commit', self.temp_branch, '-m', 'Merged unsigned json')
+        self.sign_metadata()
+        self.bzr.bzr('add', self.temp_branch)
+        self.bzr.bzr('commit', self.temp_branch, '-m', 'Signed json')
+
+
+def parse_args(argv=None):
+    parser = ArgumentParser()
+    parser.add_argument('unsigned', help='The unsigned branch to sign')
+    parser.add_argument('signed', help='The signed branch to update')
+    parser.add_argument('signing_key', help='Key to sign with.')
+    parser.add_argument('-p', '--signing-passphrase-file',
+                        help='Signing passphrase file path.')
+    return parser.parse_args(argv)
+
+
+def main():
+    args = parse_args()
+    with temp_dir() as temp_branch:
+        sb = SignBranch(args.unsigned, args.signed, temp_branch,
+                        args.signing_key, args.signing_passphrase_file,
+                        RunBzr())
+        sb.sign_branch()
+
+
+if __name__ == '__main__':
+    sys.exit(main())

=== added file 'tests/test_sign_branch.py'
--- tests/test_sign_branch.py	1970-01-01 00:00:00 +0000
+++ tests/test_sign_branch.py	2016-02-02 19:18:04 +0000
@@ -0,0 +1,133 @@
+from argparse import Namespace
+import json
+import os
+from unittest import TestCase
+
+from mock import (
+    call,
+    patch,
+    MagicMock,
+    )
+
+from sign_branch import (
+    parse_args,
+    RunBzr,
+    SignBranch,
+    )
+from tests.test_sign_metadata import (
+    fake_gpg,
+    gpg_header,
+    gpg_footer,
+    )
+from utility import temp_dir
+
+
+class TestBzr(TestCase):
+
+    def test_bzr(self):
+        bzr = RunBzr('foo_home')
+        with patch('subprocess.check_call', autospec=True) as cc_mock:
+            bzr.bzr('command1', 'arg1', 'arg2')
+        env = dict(os.environ)
+        env['BZR_HOME'] = 'foo_home'
+        cc_mock.assert_called_once_with(
+            ['bzr', '--no-aliases', 'command1', 'arg1', 'arg2'], env=env,
+            stderr=None, stdout=None)
+
+
+class TestParseArgs(TestCase):
+
+    def test_minimum(self):
+        args = parse_args(['unsigned1', 'signed1', 'key1'])
+        self.assertEqual(
+            Namespace(unsigned='unsigned1', signed='signed1',
+                      signing_key='key1', signing_passphrase_file=None),
+            args)
+
+    def test_passphrase_file(self):
+        args = parse_args(['unsigned1', 'signed1', 'key1',
+                           '--signing-passphrase-file', 'foo'])
+        self.assertEqual('foo', args.signing_passphrase_file)
+
+
+class TestSignBranch(TestCase):
+
+    def get_example_json(self):
+        return json.dumps({'path': 'index.json'})
+
+    def get_example_sjson(self):
+        return '{}{}{}'.format(
+            gpg_header(),
+            self.get_example_json().replace('.json', '.sjson'),
+            gpg_footer())
+
+    def prepare_branches(self, bzr, temp_root):
+        signed_branch = os.path.join(temp_root, 'signed')
+        bzr.bzr('init', signed_branch)
+        bzr.bzr('commit', signed_branch, '-m', 'Start empty', '--unchanged')
+        unsigned_branch = os.path.join(temp_root, 'unsigned')
+        bzr.bzr('branch', signed_branch, unsigned_branch)
+        v1_dir = os.path.join(unsigned_branch, 'v1')
+        os.mkdir(v1_dir)
+        filename = os.path.join(v1_dir, 'foo.json')
+        with open(filename, 'w') as json_file:
+            json_file.write(self.get_example_json())
+        bzr.bzr('add', filename)
+        bzr.bzr('commit', unsigned_branch, '-m', 'Add foo.json')
+        return unsigned_branch, signed_branch
+
+    def test_sign_branch(self):
+        with temp_dir() as temp_root:
+            bzr_home = os.path.join(temp_root, 'bazaar_home')
+            os.mkdir(bzr_home)
+            bzr = RunBzr(bzr_home, quiet=True)
+            bzr.bzr('whoami', 'J Random Hacker <[e-mail]>')
+            unsigned_branch, signed_branch = self.prepare_branches(bzr,
+                                                                   temp_root)
+            with temp_dir() as temp_branch:
+                sign_branch = SignBranch(
+                    unsigned_branch, signed_branch, temp_branch, None, None,
+                    bzr)
+                with patch('sign_metadata.run', autospec=True,
+                           side_effect=fake_gpg):
+                    sign_branch.sign_branch()
+            bzr.bzr('update', signed_branch)
+            self.assertItemsEqual(
+                ['foo.json', 'foo.json.gpg', 'foo.sjson'],
+                os.listdir(os.path.join(signed_branch, 'v1')))
+            with open(os.path.join(signed_branch, 'v1', 'foo.sjson')) as f:
+                self.assertEqual(self.get_example_sjson(), f.read())
+
+    def test_clean_after_merge(self):
+        bzr = MagicMock()
+        sb = SignBranch('unsigned', 'signed', 'temp', None, None, bzr)
+        sb.merge()
+        self.assertEqual([
+            call('merge', '-d', 'temp', 'unsigned'),
+            call('clean-tree', '-d', 'temp', '--force', '--ignored',
+                 '--unknown'),
+            ], bzr.bzr.mock_calls)
+
+    def test_remove_old_signed(self):
+        with temp_dir() as temp_branch:
+            v1_dir = os.path.join(temp_branch, 'v1')
+            os.mkdir(v1_dir)
+            with open(os.path.join(v1_dir, 'foo.json'), 'w') as foo:
+                foo.write('')
+            foo_sjson = os.path.join(v1_dir, 'foo.sjson')
+            with open(foo_sjson, 'w') as foo:
+                foo.write('')
+            with open(os.path.join(v1_dir, 'bar.sjson'), 'w') as foo:
+                foo.write('')
+            with open(os.path.join(v1_dir, 'baz.json.gpg'), 'w') as foo:
+                foo.write('')
+            sb = SignBranch('unsigned', 'signed', temp_branch, None, None,
+                            None)
+            with patch('sign_metadata.run', autospec=True,
+                       side_effect=fake_gpg):
+                sb.sign_metadata()
+            self.assertItemsEqual(
+                ['foo.json', 'foo.sjson', 'foo.json.gpg'],
+                os.listdir(v1_dir))
+            with open(foo_sjson) as f:
+                self.assertNotEqual('', f.read())