Dan Bungert has proposed merging ~dbungert/curtin:lp-1892494-apt-key-v2 into
curtin:master.
Commit message:
commands/apt: stop using apt-key
re-apply e099e32 and update for the modified test codebase.
Drops using apt-key in favor of placing the file direct on disk.
Includes a fix for placing the key in the correct directory.
Requested reviews:
curtin developers (curtin-dev)
For more details, see:
https://code.launchpad.net/~dbungert/curtin/+git/curtin/+merge/432335
Verified with unit tests and with a test build in Subiquity.
```
autoinstall:
apt:
sources:
mykey:
keyid: '491F9C0CCB70BB00D245C6FD433A228BEB109E69'
keyserver: 'keyserver.ubuntu.com'
```
This snippet results in the key being placed at
/etc/apt/trusted.gpg.d/mykey.asc.
--
Your team curtin developers is requested to review the proposed merge of
~dbungert/curtin:lp-1892494-apt-key-v2 into curtin:master.
diff --git a/curtin/commands/apt_config.py b/curtin/commands/apt_config.py
index 4f62a86..d625527 100644
--- a/curtin/commands/apt_config.py
+++ b/curtin/commands/apt_config.py
@@ -391,20 +391,29 @@ def apply_preserve_sources_list(target):
raise
-def add_apt_key_raw(key, target=None):
+def add_apt_key_raw(filename, key, target=None):
"""
actual adding of a key as defined in key argument
to the system
"""
- LOG.debug("Adding key:\n'%s'", key)
- try:
- util.subp(['apt-key', 'add', '-'], data=key.encode(), target=target)
- except util.ProcessExecutionError:
- LOG.exception("failed to add apt GPG Key to apt keyring")
- raise
+ if '-----BEGIN PGP PUBLIC KEY BLOCK-----' in str(key):
+ keyfile_ext = 'asc'
+ omode = 'w'
+ key = key.rstrip()
+ else:
+ keyfile_ext = 'gpg'
+ omode = 'wb'
+
+ # Store the key file in /etc/apt/trusted.gpg.d/
+ # The (sources.list) filename may be a full path
+ basename = os.path.basename(filename)
+ keyfile = f'/etc/apt/trusted.gpg.d/{basename}.{keyfile_ext}'
+ target_keyfile = paths.target_path(target, keyfile)
+ util.write_file(target_keyfile, key, mode=0o644, omode=omode)
+ LOG.debug("Adding key to '%s':\n'%s'", target_keyfile, key)
-def add_apt_key(ent, target=None):
+def add_apt_key(filename, ent, target=None):
"""
Add key to the system as defined in ent (if any).
Supports raw keys or keyid's
@@ -419,7 +428,7 @@ def add_apt_key(ent, target=None):
retries=(1, 2, 5, 10))
if 'key' in ent:
- add_apt_key_raw(ent['key'], target)
+ add_apt_key_raw(filename, ent['key'], target)
def add_apt_sources(srcdict, target=None, template_params=None,
@@ -443,7 +452,7 @@ def add_apt_sources(srcdict, target=None, template_params=None,
if 'filename' not in ent:
ent['filename'] = filename
- add_apt_key(ent, target)
+ add_apt_key(ent['filename'], ent, target)
if 'source' not in ent:
continue
diff --git a/tests/data/test.gpg b/tests/data/test.gpg
new file mode 100644
index 0000000..f7ba778
Binary files /dev/null and b/tests/data/test.gpg differ
diff --git a/tests/unittests/test_apt_source.py b/tests/unittests/test_apt_source.py
index 18de832..9fb3fc5 100644
--- a/tests/unittests/test_apt_source.py
+++ b/tests/unittests/test_apt_source.py
@@ -35,15 +35,30 @@ S0ORP6HXET3+jC8BMG4tBWCTK/XEZw==
=ACB2
-----END PGP PUBLIC KEY BLOCK-----"""
+EXPECTEDKEY_NOVER = u"""-----BEGIN PGP PUBLIC KEY BLOCK-----
+
+mI0ESuZLUgEEAKkqq3idtFP7g9hzOu1a8+v8ImawQN4TrvlygfScMU1TIS1eC7UQ
+NUA8Qqgr9iUaGnejb0VciqftLrU9D6WYHSKz+EITefgdyJ6SoQxjoJdsCpJ7o9Jy
+8PQnpRttiFm4qHu6BVnKnBNxw/z3ST9YMqW5kbMQpfxbGe+obRox59NpABEBAAG0
+HUxhdW5jaHBhZCBQUEEgZm9yIFNjb3R0IE1vc2VyiLYEEwECACAFAkrmS1ICGwMG
+CwkIBwMCBBUCCAMEFgIDAQIeAQIXgAAKCRAGILvPA2g/d3aEA/9tVjc10HOZwV29
+OatVuTeERjjrIbxflO586GLA8cp0C9RQCwgod/R+cKYdQcHjbqVcP0HqxveLg0RZ
+FJpWLmWKamwkABErwQLGlM/Hwhjfade8VvEQutH5/0JgKHmzRsoqfR+LMO6OS+Sm
+S0ORP6HXET3+jC8BMG4tBWCTK/XEZw==
+=ACB2
+-----END PGP PUBLIC KEY BLOCK-----"""
+
+EXPECTED_BINKEY = util.load_file("tests/data/test.gpg", decode=False)
+
ADD_APT_REPO_MATCH = r"^[\w-]+:\w"
-def load_tfile(filename):
+def load_tfile(filename, decode=True):
""" load_tfile
load file and return content after decoding
"""
try:
- content = util.load_file(filename, decode=True)
+ content = util.load_file(filename, decode=decode)
except Exception as error:
print('failed to load file content for test: %s' % error)
raise
@@ -86,8 +101,6 @@ class TestAptSourceConfig(CiTestCase):
self.aptlistfile2 = "single-deb2.list"
self.aptlistfile3 = "single-deb3.list"
self.matcher = re.compile(ADD_APT_REPO_MATCH).search
- self.add_patch('curtin.util.subp', 'm_subp')
- self.m_subp.return_value = ('s390x', '')
def _sources_filepath(self, filename):
# force the paths to be relative to the target
@@ -95,6 +108,11 @@ class TestAptSourceConfig(CiTestCase):
against_target = os.path.join('etc/apt/sources.list.d', filename)
return self.target + '/' + against_target
+ def _key_filepath(self, filename):
+ # always to trusted.gpg.d
+ basename = os.path.basename(filename)
+ return os.path.join(self.target, 'etc/apt/trusted.gpg.d', basename)
+
@staticmethod
def _add_apt_sources(*args, **kwargs):
with mock.patch.object(distro, 'apt_update'):
@@ -229,23 +247,46 @@ class TestAptSourceConfig(CiTestCase):
self.aptlistfile3: {'source': 'deb $MIRROR $RELEASE universe'}}
self._apt_src_replace_tri(cfg)
- def _apt_src_keyid(self, filename, cfg, keynum):
+ def _apt_src_keyid_txt(self, filename, cfg):
""" _apt_src_keyid
Test specification of a source + keyid
"""
params = self._get_default_params()
- with mock.patch("curtin.util.subp",
- return_value=('fakekey 1234', '')) as mockobj:
+ with mock.patch.object(gpg, 'getkeybyid',
+ return_value=EXPECTEDKEY_NOVER):
self._add_apt_sources(cfg, self.target, template_params=params,
aa_repo_match=self.matcher)
- # check if it added the right ammount of keys
- calls = []
- for _ in range(keynum):
- calls.append(call(['apt-key', 'add', '-'], data=b'fakekey 1234',
- target=self.target))
- mockobj.assert_has_calls(calls, any_order=True)
+ for ent in cfg:
+ key_filename = cfg[ent].get('filename', ent) + '.asc'
+ contents = load_tfile(self._key_filepath(key_filename))
+ self.assertMultiLineEqual(EXPECTEDKEY_NOVER, contents)
+
+ def _apt_src_keyid_bin(self, filename, cfg):
+ """ _apt_src_keyid
+ Test specification of a source + keyid
+ """
+ params = self._get_default_params()
+
+ with mock.patch.object(gpg, 'getkeybyid',
+ return_value=EXPECTED_BINKEY):
+ self._add_apt_sources(cfg, self.target, template_params=params,
+ aa_repo_match=self.matcher)
+
+ for ent in cfg:
+ key_filename = cfg[ent].get('filename', ent) + '.gpg'
+ contents = load_tfile(self._key_filepath(key_filename), False)
+ self.assertEqual(EXPECTED_BINKEY, contents)
+
+ def _apt_src_keyid(self, filename, cfg, key_type="txt"):
+ """ _apt_src_keyid
+ Test specification of a source + keyid
+ """
+ if key_type == "txt":
+ self._apt_src_keyid_txt(filename, cfg)
+ else:
+ self._apt_src_keyid_bin(filename, cfg)
contents = load_tfile(self._sources_filepath(filename))
self.assertTrue(re.search(r"%s %s %s %s\n" %
@@ -263,7 +304,17 @@ class TestAptSourceConfig(CiTestCase):
'smoser/cloud-init-test/ubuntu'
' xenial main'),
'keyid': "03683F77"}}
- self._apt_src_keyid(self.aptlistfile, cfg, 1)
+ self._apt_src_keyid(self.aptlistfile, cfg)
+
+ @mock.patch(ChrootableTargetStr, new=PseudoChrootableTarget)
+ def test_apt_src_keyid_bin(self):
+ """test_apt_src_keyid - Test source + keyid with filename being set"""
+ cfg = {self.aptlistfile: {'source': ('deb '
+ 'http://ppa.launchpad.net/'
+ 'smoser/cloud-init-test/ubuntu'
+ ' xenial main'),
+ 'keyid': "03683F77"}}
+ self._apt_src_keyid(self.aptlistfile, cfg, key_type='bin')
@mock.patch(ChrootableTargetStr, new=PseudoChrootableTarget)
def test_apt_src_keyid_tri(self):
@@ -285,7 +336,7 @@ class TestAptSourceConfig(CiTestCase):
' xenial multiverse'),
'keyid': "03683F77"}}
- self._apt_src_keyid(self.aptlistfile, cfg, 3)
+ self._apt_src_keyid(self.aptlistfile, cfg)
contents = load_tfile(self._sources_filepath(self.aptlistfile2))
self.assertTrue(re.search(r"%s %s %s %s\n" %
("deb",
@@ -305,18 +356,20 @@ class TestAptSourceConfig(CiTestCase):
def test_apt_src_key(self):
"""test_apt_src_key - Test source + key"""
params = self._get_default_params()
+ fake_key = u"""-----BEGIN PGP PUBLIC KEY BLOCK-----
+ fakekey 4321"""
+
cfg = {self.aptlistfile: {'source': ('deb '
'http://ppa.launchpad.net/'
'smoser/cloud-init-test/ubuntu'
' xenial main'),
- 'key': "fakekey 4321"}}
+ 'key': fake_key}}
- with mock.patch.object(util, 'subp') as mockobj:
- self._add_apt_sources(cfg, self.target, template_params=params,
- aa_repo_match=self.matcher)
+ self._add_apt_sources(cfg, self.target, template_params=params,
+ aa_repo_match=self.matcher)
- mockobj.assert_any_call(['apt-key', 'add', '-'], data=b'fakekey 4321',
- target=self.target)
+ contents = load_tfile(self._key_filepath(self.aptlistfile + '.asc'))
+ self.assertMultiLineEqual(fake_key, contents)
contents = load_tfile(self._sources_filepath(self.aptlistfile))
self.assertTrue(re.search(r"%s %s %s %s\n" %
@@ -330,14 +383,15 @@ class TestAptSourceConfig(CiTestCase):
def test_apt_src_keyonly(self):
"""test_apt_src_keyonly - Test key without source"""
params = self._get_default_params()
- cfg = {self.aptlistfile: {'key': "fakekey 4242"}}
+ fake_key = u"""-----BEGIN PGP PUBLIC KEY BLOCK-----
+ fakekey 4321"""
+ cfg = {self.aptlistfile: {'key': fake_key}}
- with mock.patch.object(util, 'subp') as mockobj:
- self._add_apt_sources(cfg, self.target, template_params=params,
- aa_repo_match=self.matcher)
+ self._add_apt_sources(cfg, self.target, template_params=params,
+ aa_repo_match=self.matcher)
- mockobj.assert_any_call(['apt-key', 'add', '-'], data=b'fakekey 4242',
- target=self.target)
+ contents = load_tfile(self._key_filepath(self.aptlistfile + '.asc'))
+ self.assertMultiLineEqual(fake_key, contents)
# filename should be ignored on key only
self.assertFalse(os.path.isfile(
@@ -349,13 +403,13 @@ class TestAptSourceConfig(CiTestCase):
params = self._get_default_params()
cfg = {self.aptlistfile: {'keyid': "03683F77"}}
- with mock.patch.object(util, 'subp',
- return_value=('fakekey 1212', '')) as mockobj:
+ with mock.patch.object(gpg, 'getkeybyid',
+ return_value=EXPECTEDKEY_NOVER):
self._add_apt_sources(cfg, self.target, template_params=params,
aa_repo_match=self.matcher)
- mockobj.assert_any_call(['apt-key', 'add', '-'], data=b'fakekey 1212',
- target=self.target)
+ contents = load_tfile(self._key_filepath(self.aptlistfile + '.asc'))
+ self.assertMultiLineEqual(EXPECTEDKEY_NOVER, contents)
# filename should be ignored on key only
self.assertFalse(os.path.isfile(
@@ -380,7 +434,7 @@ class TestAptSourceConfig(CiTestCase):
keycfg.get('keyserver',
'keyserver.ubuntu.com'),
retries=(1, 2, 5, 10))
- mockkey.assert_called_with(expectedkey, self.target)
+ mockkey.assert_called_with(self.aptlistfile, expectedkey, self.target)
# filename should be ignored on key only
self.assertFalse(os.path.isfile(
@@ -425,7 +479,7 @@ class TestAptSourceConfig(CiTestCase):
mockgetkey.assert_called_with('03683F77', 'test.random.com',
retries=(1, 2, 5, 10))
- mockadd.assert_called_with('fakekey', self.target)
+ mockadd.assert_called_with(self.aptlistfile, 'fakekey', self.target)
# filename should be ignored on key only
self.assertFalse(os.path.isfile(
--
Mailing list: https://launchpad.net/~curtin-dev
Post to : curtin-dev@lists.launchpad.net
Unsubscribe : https://launchpad.net/~curtin-dev
More help : https://help.launchpad.net/ListHelp
