Check if device name is valid when adding to the ring

Currently device names can be empty or start and/or end with spaces.
This can create unexpected results, for example these three commands
are all valid:

swift-ring-builder account.builder add "r1z1-127.0.0.1:6000/" 1
swift-ring-builder account.builder add "r1z1-127.0.0.1:6000/sda " 1
swift-ring-builder account.builder add "r1z1-127.0.0.1:6000/ meta" 1

This patch validates device names and prevents empty names or names
starting and/or ending with spaces.

Also fixed the test "test_warn_at_risk" - the test passed if the
exception was not raised.

Closes-Bug: 1438579

Change-Id: I811b0eae7db503279e6429d985275bbab8b29c9f
This commit is contained in:
Christian Schwede 2015-03-31 08:51:18 +00:00 committed by John Dickinson
parent 3c419dfb12
commit d3213fb1fe
3 changed files with 48 additions and 2 deletions
swift
test/unit/cli

@ -34,7 +34,7 @@ from swift.common.ring.utils import validate_args, \
validate_and_normalize_ip, build_dev_from_opts, \
parse_builder_ring_filename_args, parse_search_value, \
parse_search_values_from_opts, parse_change_values_from_opts, \
dispersion_report
dispersion_report, validate_device_name
from swift.common.utils import lock_parent_directory
MAJOR_VERSION = 1
@ -220,6 +220,9 @@ def _parse_add_values(argvish):
while i < len(rest) and rest[i] != '_':
i += 1
device_name = rest[1:i]
if not validate_device_name(device_name):
raise ValueError('Invalid device name')
rest = rest[i:]
meta = ''

@ -515,6 +515,9 @@ def build_dev_from_opts(opts):
(opts.replication_ip or opts.ip))
replication_port = opts.replication_port or opts.port
if not validate_device_name(opts.device):
raise ValueError('Invalid device name')
return {'region': opts.region, 'zone': opts.zone, 'ip': ip,
'port': opts.port, 'device': opts.device, 'meta': opts.meta,
'replication_ip': replication_ip,
@ -569,3 +572,10 @@ def get_tier_name(tier, builder):
device = builder.devs[tier[3]] or {}
return "r%sz%s-%s/%s" % (tier[0], tier[1], tier[2],
device.get('device', 'IDd%s' % tier[3]))
def validate_device_name(device_name):
return not (
device_name.startswith(' ') or
device_name.endswith(' ') or
len(device_name) == 0)

@ -1711,10 +1711,43 @@ class TestCommands(unittest.TestCase, RunSwiftRingBuilderMixin):
ring.devs[0]['weight'] = 10
ring.save(self.tmpfile)
argv = ["", self.tmpfile, "rebalance"]
err = None
try:
ringbuilder.main(argv)
except SystemExit as e:
self.assertEquals(e.code, 1)
err = e
self.assertEquals(err.code, 1)
def test_invalid_device_name(self):
self.create_sample_ring()
for device_name in ["", " ", " sda1", "sda1 ", " meta "]:
err = 0
argv = ["",
self.tmpfile,
"add",
"r1z1-127.0.0.1:6000/%s" % device_name,
"1"]
try:
ringbuilder.main(argv)
except SystemExit as exc:
err = exc
self.assertEquals(err.code, 2)
argv = ["",
self.tmpfile,
"add",
"--region", "1",
"--zone", "1",
"--ip", "127.0.0.1",
"--port", "6000",
"--device", device_name,
"--weight", "100"]
try:
ringbuilder.main(argv)
except SystemExit as exc:
err = exc
self.assertEquals(err.code, 2)
class TestRebalanceCommand(unittest.TestCase, RunSwiftRingBuilderMixin):