From e32f391a1dd127470e7550e8d640b7154c7f9ef2 Mon Sep 17 00:00:00 2001
From: Jan Fajerski <jfajerski@suse.com>
Date: Wed, 18 Nov 2020 12:18:48 +0100
Subject: [PATCH] ceph-volume batch: reject partitions in argparser

Fixes: https://tracker.ceph.com/issues/47966

Signed-off-by: Jan Fajerski <jfajerski@suse.com>
(cherry picked from commit 9742efa907aa54b3135f5daf73080b7be12534eb)
---
 .../ceph_volume/devices/lvm/batch.py          |  8 ++--
 .../tests/devices/lvm/test_batch.py           | 14 +++++++
 .../ceph_volume/util/arg_validators.py        | 37 ++++++++++++++-----
 3 files changed, 46 insertions(+), 13 deletions(-)

diff --git a/src/ceph-volume/ceph_volume/devices/lvm/batch.py b/src/ceph-volume/ceph_volume/devices/lvm/batch.py
index 2dbe20f05e4d..a6f7632b3170 100644
--- a/src/ceph-volume/ceph_volume/devices/lvm/batch.py
+++ b/src/ceph-volume/ceph_volume/devices/lvm/batch.py
@@ -175,28 +175,28 @@ class Batch(object):
             'devices',
             metavar='DEVICES',
             nargs='*',
-            type=arg_validators.ValidDevice(),
+            type=arg_validators.ValidBatchDevice(),
             default=[],
             help='Devices to provision OSDs',
         )
         parser.add_argument(
             '--db-devices',
             nargs='*',
-            type=arg_validators.ValidDevice(),
+            type=arg_validators.ValidBatchDevice(),
             default=[],
             help='Devices to provision OSDs db volumes',
         )
         parser.add_argument(
             '--wal-devices',
             nargs='*',
-            type=arg_validators.ValidDevice(),
+            type=arg_validators.ValidBatchDevice(),
             default=[],
             help='Devices to provision OSDs wal volumes',
         )
         parser.add_argument(
             '--journal-devices',
             nargs='*',
-            type=arg_validators.ValidDevice(),
+            type=arg_validators.ValidBatchDevice(),
             default=[],
             help='Devices to provision OSDs journal volumes',
         )
diff --git a/src/ceph-volume/ceph_volume/tests/devices/lvm/test_batch.py b/src/ceph-volume/ceph_volume/tests/devices/lvm/test_batch.py
index 44a05bfa8287..7c968ae81d5e 100644
--- a/src/ceph-volume/ceph_volume/tests/devices/lvm/test_batch.py
+++ b/src/ceph-volume/ceph_volume/tests/devices/lvm/test_batch.py
@@ -1,7 +1,12 @@
 import pytest
 import json
 import random
+
+from argparse import ArgumentError
+from mock import MagicMock, patch
+
 from ceph_volume.devices.lvm import batch
+from ceph_volume.util import arg_validators
 
 
 class TestBatch(object):
@@ -19,6 +24,15 @@ class TestBatch(object):
             batch.ensure_disjoint_device_lists(devices, db_devices)
         assert 'Device lists are not disjoint' in str(disjoint_ex.value)
 
+    @patch('ceph_volume.util.arg_validators.Device')
+    def test_reject_partition(self, mocked_device):
+        mocked_device.return_value = MagicMock(
+            is_partition=True,
+            has_gpt_headers=False,
+        )
+        with pytest.raises(ArgumentError):
+            arg_validators.ValidBatchDevice()('foo')
+
     @pytest.mark.parametrize('format_', ['pretty', 'json', 'json-pretty'])
     def test_report(self, format_, factory, conf_ceph_stub, mock_device_generator):
         # just ensure reporting works
diff --git a/src/ceph-volume/ceph_volume/util/arg_validators.py b/src/ceph-volume/ceph_volume/util/arg_validators.py
index a04c19924bbd..94cb4f691dbe 100644
--- a/src/ceph-volume/ceph_volume/util/arg_validators.py
+++ b/src/ceph-volume/ceph_volume/util/arg_validators.py
@@ -12,11 +12,23 @@ class ValidDevice(object):
         self.as_string = as_string
         self.gpt_ok = gpt_ok
 
-    def __call__(self, string):
-        device = Device(string)
+    def __call__(self, dev_path):
+        device = self._is_valid_device(dev_path)
+        return self._format_device(device)
+
+    def _format_device(self, device):
+        if self.as_string:
+            if device.is_lv:
+                # all codepaths expect an lv path to be returned in this format
+                return "{}/{}".format(device.vg_name, device.lv_name)
+            return device.path
+        return device
+
+    def _is_valid_device(self, dev_path):
+        device = Device(dev_path)
         error = None
         if not device.exists:
-            error = "Unable to proceed with non-existing device: %s" % string
+            error = "Unable to proceed with non-existing device: %s" % dev_path
         # FIXME this is not a nice API, this validator was meant to catch any
         # non-existing devices upfront, not check for gpt headers. Now this
         # needs to optionally skip checking gpt headers which is beyond
@@ -24,19 +36,26 @@ class ValidDevice(object):
         # configure this with a list of checks that can be excluded/included on
         # __init__
         elif device.has_gpt_headers and not self.gpt_ok:
-            error = "GPT headers found, they must be removed on: %s" % string
+            error = "GPT headers found, they must be removed on: %s" % dev_path
 
         if error:
             raise argparse.ArgumentError(None, error)
 
-        if self.as_string:
-            if device.is_lv:
-                # all codepaths expect an lv path to be returned in this format
-                return "{}/{}".format(device.vg_name, device.lv_name)
-            return string
         return device
 
 
+class ValidBatchDevice(ValidDevice):
+
+    def __call__(self, dev_path):
+        dev = self._is_valid_device(dev_path)
+        if dev.is_partition:
+            raise argparse.ArgumentError(
+                None,
+                '{} is a partition, please pass '
+                'LVs or raw block devices'.format(dev_path))
+        return self._format_device(dev)
+
+
 class OSDPath(object):
     """
     Validate path exists and it looks like an OSD directory.
-- 
2.47.3