From 6fc7f7f58b338fa920a6a570485836fe91b6de21 Mon Sep 17 00:00:00 2001 From: Travis Rhoden Date: Thu, 21 May 2015 14:35:38 -0400 Subject: [PATCH 1/7] [RM-11694] Require at least one host for 'admin' Signed-off-by: Travis Rhoden --- ceph_deploy/admin.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ceph_deploy/admin.py b/ceph_deploy/admin.py index 9c2b486f..6bea0dda 100644 --- a/ceph_deploy/admin.py +++ b/ceph_deploy/admin.py @@ -58,7 +58,7 @@ def make(parser): parser.add_argument( 'client', metavar='HOST', - nargs='*', + nargs='+', help='host to configure for ceph administration', ) parser.set_defaults( From debc76258e35aaa9ebc1eaeb717b82d7bded2bb5 Mon Sep 17 00:00:00 2001 From: Travis Rhoden Date: Thu, 21 May 2015 14:36:15 -0400 Subject: [PATCH 2/7] [RM-11694] Add CLI test for "ceph-deploy admin" Signed-off-by: Travis Rhoden --- ceph_deploy/tests/test_cli_admin.py | 49 +++++++++++++++++++++++++++++ 1 file changed, 49 insertions(+) create mode 100644 ceph_deploy/tests/test_cli_admin.py diff --git a/ceph_deploy/tests/test_cli_admin.py b/ceph_deploy/tests/test_cli_admin.py new file mode 100644 index 00000000..1c7b369d --- /dev/null +++ b/ceph_deploy/tests/test_cli_admin.py @@ -0,0 +1,49 @@ +import pytest +import subprocess + + +def test_help(tmpdir, cli): + with cli( + args=['ceph-deploy', 'admin', '--help'], + stdout=subprocess.PIPE, + ) as p: + result = p.stdout.read() + assert 'usage: ceph-deploy admin' in result + assert 'positional arguments' in result + assert 'optional arguments' in result + + +def test_bad_no_hosts(tmpdir, cli): + with pytest.raises(cli.Failed) as err: + with cli( + args=['ceph-deploy', 'admin'], + stderr=subprocess.PIPE, + ) as p: + result = p.stderr.read() + assert 'usage: ceph-deploy admin' in result + assert 'too few arguments' in result + assert err.value.status == 2 + + +def test_bad_no_conf(tmpdir, cli): + with pytest.raises(cli.Failed) as err: + with cli( + args=['ceph-deploy', 'admin', 'host1'], + stderr=subprocess.PIPE, + ) as p: + result = p.stderr.read() + assert 'No such file or directory: \'ceph.conf\'' in result + assert err.value.status == 1 + + +def test_bad_no_key(tmpdir, cli): + with tmpdir.join('ceph.conf').open('w'): + pass + with pytest.raises(cli.Failed) as err: + with cli( + args=['ceph-deploy', 'admin', 'host1'], + stderr=subprocess.PIPE, + ) as p: + result = p.stderr.read() + assert 'ceph.client.admin.keyring not found' in result + assert err.value.status == 1 From ab240e32061ba5f89fca3d145fa229e8c8284edd Mon Sep 17 00:00:00 2001 From: Travis Rhoden Date: Thu, 21 May 2015 16:09:29 -0400 Subject: [PATCH 3/7] [RM-11694] Remove extraneous line No need to get to the shortname of the host -- it was never used and was also overwriting the variable used in the loop. Signed-off-by: Travis Rhoden --- ceph_deploy/admin.py | 1 - 1 file changed, 1 deletion(-) diff --git a/ceph_deploy/admin.py b/ceph_deploy/admin.py index 6bea0dda..23276600 100644 --- a/ceph_deploy/admin.py +++ b/ceph_deploy/admin.py @@ -27,7 +27,6 @@ def admin(args): LOG.debug('Pushing admin keys and conf to %s', hostname) try: distro = hosts.get(hostname, username=args.username) - hostname = distro.conn.remote_module.shortname() distro.conn.remote_module.write_conf( args.cluster, From 628bd9ed6bfb84821ba7d2240cc41a8783cc4617 Mon Sep 17 00:00:00 2001 From: Travis Rhoden Date: Fri, 22 May 2015 10:33:07 -0400 Subject: [PATCH 4/7] [RM-11694] Add optional directory to write_file() This is mostly for testing purposes. The method has no way to specify a directory independently from the file name, which it makes it difficult to use with automated tests. It will get refactored more later, but for now add a new kwarg that pass in a dir to write to. Since a lot of callers are passing in absolute paths (.e.g. "/etc/ceph/keyring"), we look for the beginning slash and chop it off so that we can use os.path.join() and write into something like /tmp/etc/ceph. Signed-off-by: Travis Rhoden --- ceph_deploy/hosts/remotes.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/ceph_deploy/hosts/remotes.py b/ceph_deploy/hosts/remotes.py index 6c8e63aa..19be3e46 100644 --- a/ceph_deploy/hosts/remotes.py +++ b/ceph_deploy/hosts/remotes.py @@ -200,7 +200,11 @@ def write_monitor_keyring(keyring, monitor_keyring): write_file(keyring, monitor_keyring) -def write_file(path, content): +def write_file(path, content, directory=None): + if directory: + if path.startswith("/"): + path = path[1:] + path = os.path.join(directory, path) with file(path, 'w') as f: f.write(content) From 78fe41a682b6e5655d50091ff1d8b355e72edb2c Mon Sep 17 00:00:00 2001 From: Travis Rhoden Date: Fri, 22 May 2015 10:36:23 -0400 Subject: [PATCH 5/7] [RM-11694] Add test for admin key file permissions Signed-off-by: Travis Rhoden --- ceph_deploy/tests/test_cli_admin.py | 35 ++++++++++++++++++++++++++++- 1 file changed, 34 insertions(+), 1 deletion(-) diff --git a/ceph_deploy/tests/test_cli_admin.py b/ceph_deploy/tests/test_cli_admin.py index 1c7b369d..50b06a3c 100644 --- a/ceph_deploy/tests/test_cli_admin.py +++ b/ceph_deploy/tests/test_cli_admin.py @@ -1,6 +1,12 @@ -import pytest +import os import subprocess +import pytest +from mock import patch, MagicMock, Mock + +from ceph_deploy.cli import _main as main +from ceph_deploy.hosts import remotes +from ceph_deploy.tests.directory import directory def test_help(tmpdir, cli): with cli( @@ -47,3 +53,30 @@ def test_bad_no_key(tmpdir, cli): result = p.stderr.read() assert 'ceph.client.admin.keyring not found' in result assert err.value.status == 1 + + +def test_write_keyring(tmpdir): + with tmpdir.join('ceph.conf').open('w'): + pass + with tmpdir.join('ceph.client.admin.keyring').open('w'): + pass + + etc_ceph = os.path.join(str(tmpdir), 'etc', 'ceph') + os.makedirs(etc_ceph) + + distro = MagicMock() + distro.conn = MagicMock() + remotes.write_file.func_defaults = (str(tmpdir),) + distro.conn.remote_module = remotes + distro.conn.remote_module.write_conf = Mock() + + with patch('ceph_deploy.admin.hosts'): + with patch('ceph_deploy.admin.hosts.get', MagicMock(return_value=distro)): + with directory(str(tmpdir)): + main(args=['admin', 'host1']) + + keyring_file = os.path.join(etc_ceph, 'ceph.client.admin.keyring') + assert os.path.exists(keyring_file) + + file_mode = oct(os.stat(keyring_file).st_mode & 0777) + assert file_mode == oct(0600) From 5368d9d14d3bfbef480d1302511121768d557d5e Mon Sep 17 00:00:00 2001 From: Travis Rhoden Date: Fri, 22 May 2015 10:49:28 -0400 Subject: [PATCH 6/7] [RM-11694] Add file mode option to write_file() You an now pass a file mode to write_file with "mode=" and it will set the file mode that after writing the file. Signed-off-by: Travis Rhoden --- ceph_deploy/hosts/remotes.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ceph_deploy/hosts/remotes.py b/ceph_deploy/hosts/remotes.py index 19be3e46..ba3e7d83 100644 --- a/ceph_deploy/hosts/remotes.py +++ b/ceph_deploy/hosts/remotes.py @@ -200,12 +200,12 @@ def write_monitor_keyring(keyring, monitor_keyring): write_file(keyring, monitor_keyring) -def write_file(path, content, directory=None): +def write_file(path, content, mode=0644, directory=None): if directory: if path.startswith("/"): path = path[1:] path = os.path.join(directory, path) - with file(path, 'w') as f: + with os.fdopen(os.open(path, os.O_WRONLY | os.O_CREAT, mode), 'w') as f: f.write(content) From 8ef6d41e5bdbaf2717671c1ace237d35fa8ebcb4 Mon Sep 17 00:00:00 2001 From: Travis Rhoden Date: Fri, 22 May 2015 10:51:42 -0400 Subject: [PATCH 7/7] [RM-11694] Always write admin keyring with mode 0600 Signed-off-by: Travis Rhoden --- ceph_deploy/admin.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/ceph_deploy/admin.py b/ceph_deploy/admin.py index 23276600..0d7ba8a9 100644 --- a/ceph_deploy/admin.py +++ b/ceph_deploy/admin.py @@ -36,7 +36,8 @@ def admin(args): distro.conn.remote_module.write_file( '/etc/ceph/%s.client.admin.keyring' % args.cluster, - keyring + keyring, + 0600, ) distro.conn.exit()