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

Proposed by Aaron Bentley on 2016-02-02
Status: Merged
Merged at revision: 262
Proposed branch: lp:~abentley/juju-core/sign-branch
Merge into: lp:~juju-qa/juju-core/cd-release-juju
Prerequisite: lp:~abentley/juju-core/sign-metadata
Diff against target: 232 lines (+223/-0)
2 files modified
sign_branch.py (+90/-0)
tests/test_sign_branch.py (+133/-0)
To merge this branch: bzr merge lp:~abentley/juju-core/sign-branch
Reviewer Review Type Date Requested Status
Curtis Hovey (community) code 2016-02-02 Approve on 2016-02-02
Review via email: mp+284783@code.launchpad.net

Commit message

Add sign_branch script.

Description of the change

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.

To post a comment you must log in.
Curtis Hovey (sinzui) wrote :

I have two questions inline.

review: Needs Information (code)
Aaron Bentley (abentley) wrote :

-----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) wrote :

Thank you.

review: Approve (code)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== added file 'sign_branch.py'
2--- sign_branch.py 1970-01-01 00:00:00 +0000
3+++ sign_branch.py 2016-02-02 19:18:04 +0000
4@@ -0,0 +1,90 @@
5+#!/usr/bin/env python
6+from argparse import ArgumentParser
7+import os
8+import subprocess
9+import sys
10+
11+from sign_metadata import sign_metadata
12+from utility import temp_dir
13+
14+
15+class RunBzr:
16+
17+ def __init__(self, bzr_home=None, quiet=False):
18+ self.bzr_home = bzr_home
19+ self.quiet = quiet
20+
21+ def bzr(self, command, *args):
22+ cc_args = ['bzr', '--no-aliases']
23+ cc_args.append(command)
24+ cc_args.extend(args)
25+ env = dict(os.environ)
26+ if self.bzr_home is not None:
27+ env['BZR_HOME'] = self.bzr_home
28+ with open('/dev/null', 'w') as null:
29+ if self.quiet:
30+ err = null
31+ out = null
32+ else:
33+ err = None
34+ out = None
35+ subprocess.check_call(cc_args, env=env, stderr=err, stdout=out)
36+
37+
38+class SignBranch:
39+
40+ def __init__(self, unsigned_branch, signed_branch, temp_branch,
41+ signing_key, signing_passphrase_file, bzr):
42+ self.unsigned_branch = unsigned_branch
43+ self.signed_branch = signed_branch
44+ self.temp_branch = temp_branch
45+ self.signing_key = signing_key
46+ self.signing_passphrase_file = signing_passphrase_file
47+ self.bzr = bzr
48+
49+ def merge(self):
50+ self.bzr.bzr('merge', '-d', self.temp_branch, self.unsigned_branch)
51+ # Ensure nothing left behind.
52+ self.bzr.bzr('clean-tree', '-d', self.temp_branch, '--force',
53+ '--ignored', '--unknown')
54+
55+ def sign_metadata(self):
56+ for dirpath, dirnames, filenames in os.walk(self.temp_branch):
57+ for filename in filenames:
58+ if not (filename.endswith('.sjson') or
59+ filename.endswith('.json.gpg')):
60+ continue
61+ os.unlink(os.path.join(dirpath, filename))
62+ subdir = os.path.join(self.temp_branch, 'v1')
63+ sign_metadata(self.signing_key, subdir, self.signing_passphrase_file)
64+
65+ def sign_branch(self):
66+ self.bzr.bzr('checkout', self.signed_branch, self.temp_branch)
67+ self.merge()
68+ self.bzr.bzr('commit', self.temp_branch, '-m', 'Merged unsigned json')
69+ self.sign_metadata()
70+ self.bzr.bzr('add', self.temp_branch)
71+ self.bzr.bzr('commit', self.temp_branch, '-m', 'Signed json')
72+
73+
74+def parse_args(argv=None):
75+ parser = ArgumentParser()
76+ parser.add_argument('unsigned', help='The unsigned branch to sign')
77+ parser.add_argument('signed', help='The signed branch to update')
78+ parser.add_argument('signing_key', help='Key to sign with.')
79+ parser.add_argument('-p', '--signing-passphrase-file',
80+ help='Signing passphrase file path.')
81+ return parser.parse_args(argv)
82+
83+
84+def main():
85+ args = parse_args()
86+ with temp_dir() as temp_branch:
87+ sb = SignBranch(args.unsigned, args.signed, temp_branch,
88+ args.signing_key, args.signing_passphrase_file,
89+ RunBzr())
90+ sb.sign_branch()
91+
92+
93+if __name__ == '__main__':
94+ sys.exit(main())
95
96=== added file 'tests/test_sign_branch.py'
97--- tests/test_sign_branch.py 1970-01-01 00:00:00 +0000
98+++ tests/test_sign_branch.py 2016-02-02 19:18:04 +0000
99@@ -0,0 +1,133 @@
100+from argparse import Namespace
101+import json
102+import os
103+from unittest import TestCase
104+
105+from mock import (
106+ call,
107+ patch,
108+ MagicMock,
109+ )
110+
111+from sign_branch import (
112+ parse_args,
113+ RunBzr,
114+ SignBranch,
115+ )
116+from tests.test_sign_metadata import (
117+ fake_gpg,
118+ gpg_header,
119+ gpg_footer,
120+ )
121+from utility import temp_dir
122+
123+
124+class TestBzr(TestCase):
125+
126+ def test_bzr(self):
127+ bzr = RunBzr('foo_home')
128+ with patch('subprocess.check_call', autospec=True) as cc_mock:
129+ bzr.bzr('command1', 'arg1', 'arg2')
130+ env = dict(os.environ)
131+ env['BZR_HOME'] = 'foo_home'
132+ cc_mock.assert_called_once_with(
133+ ['bzr', '--no-aliases', 'command1', 'arg1', 'arg2'], env=env,
134+ stderr=None, stdout=None)
135+
136+
137+class TestParseArgs(TestCase):
138+
139+ def test_minimum(self):
140+ args = parse_args(['unsigned1', 'signed1', 'key1'])
141+ self.assertEqual(
142+ Namespace(unsigned='unsigned1', signed='signed1',
143+ signing_key='key1', signing_passphrase_file=None),
144+ args)
145+
146+ def test_passphrase_file(self):
147+ args = parse_args(['unsigned1', 'signed1', 'key1',
148+ '--signing-passphrase-file', 'foo'])
149+ self.assertEqual('foo', args.signing_passphrase_file)
150+
151+
152+class TestSignBranch(TestCase):
153+
154+ def get_example_json(self):
155+ return json.dumps({'path': 'index.json'})
156+
157+ def get_example_sjson(self):
158+ return '{}{}{}'.format(
159+ gpg_header(),
160+ self.get_example_json().replace('.json', '.sjson'),
161+ gpg_footer())
162+
163+ def prepare_branches(self, bzr, temp_root):
164+ signed_branch = os.path.join(temp_root, 'signed')
165+ bzr.bzr('init', signed_branch)
166+ bzr.bzr('commit', signed_branch, '-m', 'Start empty', '--unchanged')
167+ unsigned_branch = os.path.join(temp_root, 'unsigned')
168+ bzr.bzr('branch', signed_branch, unsigned_branch)
169+ v1_dir = os.path.join(unsigned_branch, 'v1')
170+ os.mkdir(v1_dir)
171+ filename = os.path.join(v1_dir, 'foo.json')
172+ with open(filename, 'w') as json_file:
173+ json_file.write(self.get_example_json())
174+ bzr.bzr('add', filename)
175+ bzr.bzr('commit', unsigned_branch, '-m', 'Add foo.json')
176+ return unsigned_branch, signed_branch
177+
178+ def test_sign_branch(self):
179+ with temp_dir() as temp_root:
180+ bzr_home = os.path.join(temp_root, 'bazaar_home')
181+ os.mkdir(bzr_home)
182+ bzr = RunBzr(bzr_home, quiet=True)
183+ bzr.bzr('whoami', 'J Random Hacker <jrandom@example.org>')
184+ unsigned_branch, signed_branch = self.prepare_branches(bzr,
185+ temp_root)
186+ with temp_dir() as temp_branch:
187+ sign_branch = SignBranch(
188+ unsigned_branch, signed_branch, temp_branch, None, None,
189+ bzr)
190+ with patch('sign_metadata.run', autospec=True,
191+ side_effect=fake_gpg):
192+ sign_branch.sign_branch()
193+ bzr.bzr('update', signed_branch)
194+ self.assertItemsEqual(
195+ ['foo.json', 'foo.json.gpg', 'foo.sjson'],
196+ os.listdir(os.path.join(signed_branch, 'v1')))
197+ with open(os.path.join(signed_branch, 'v1', 'foo.sjson')) as f:
198+ self.assertEqual(self.get_example_sjson(), f.read())
199+
200+ def test_clean_after_merge(self):
201+ bzr = MagicMock()
202+ sb = SignBranch('unsigned', 'signed', 'temp', None, None, bzr)
203+ sb.merge()
204+ self.assertEqual([
205+ call('merge', '-d', 'temp', 'unsigned'),
206+ call('clean-tree', '-d', 'temp', '--force', '--ignored',
207+ '--unknown'),
208+ ], bzr.bzr.mock_calls)
209+
210+ def test_remove_old_signed(self):
211+ with temp_dir() as temp_branch:
212+ v1_dir = os.path.join(temp_branch, 'v1')
213+ os.mkdir(v1_dir)
214+ with open(os.path.join(v1_dir, 'foo.json'), 'w') as foo:
215+ foo.write('')
216+ foo_sjson = os.path.join(v1_dir, 'foo.sjson')
217+ with open(foo_sjson, 'w') as foo:
218+ foo.write('')
219+ with open(os.path.join(v1_dir, 'bar.sjson'), 'w') as foo:
220+ foo.write('')
221+ with open(os.path.join(v1_dir, 'baz.json.gpg'), 'w') as foo:
222+ foo.write('')
223+ sb = SignBranch('unsigned', 'signed', temp_branch, None, None,
224+ None)
225+ with patch('sign_metadata.run', autospec=True,
226+ side_effect=fake_gpg):
227+ sb.sign_metadata()
228+ self.assertItemsEqual(
229+ ['foo.json', 'foo.sjson', 'foo.json.gpg'],
230+ os.listdir(v1_dir))
231+ with open(foo_sjson) as f:
232+ self.assertNotEqual('', f.read())

Subscribers

People subscribed via source and target branches