From d42a78a3aa138ca14e1b06555ae649971799e618 Mon Sep 17 00:00:00 2001
From: Samuel Merritt <sam@swiftstack.com>
Date: Sun, 17 Mar 2013 15:07:20 -0700
Subject: [PATCH] Basic ring builder validation.

This prevents people from creating bogus ring builder files.

Example: "swift-ring-builder object.builder create 33 0.9 -4".

Fixes bug 924577.

Change-Id: I7bfc04f7fa5f55f70a4eaae96c414f6b2872e283
---
 swift/common/ring/builder.py          | 12 +++++++++++-
 test/unit/common/ring/test_builder.py | 12 ++++++++++++
 2 files changed, 23 insertions(+), 1 deletion(-)

diff --git a/swift/common/ring/builder.py b/swift/common/ring/builder.py
index 43185789a6..d0aa8fbe91 100644
--- a/swift/common/ring/builder.py
+++ b/swift/common/ring/builder.py
@@ -39,12 +39,22 @@ class RingBuilder(object):
     a rebalance request is an isolated request or due to added, changed, or
     removed devices.
 
-    :param part_power: number of partitions = 2**part_power
+    :param part_power: number of partitions = 2**part_power.
     :param replicas: number of replicas for each partition
     :param min_part_hours: minimum number of hours between partition changes
     """
 
     def __init__(self, part_power, replicas, min_part_hours):
+        if part_power > 32:
+            raise ValueError("part_power must be at most 32 (was %d)"
+                             % (part_power,))
+        if replicas < 1:
+            raise ValueError("replicas must be at least 1 (was %.6f)"
+                             % (replicas,))
+        if min_part_hours < 0:
+            raise ValueError("min_part_hours must be non-negative (was %d)"
+                             % (min_part_hours,))
+
         self.part_power = part_power
         self.replicas = replicas
         self.min_part_hours = min_part_hours
diff --git a/test/unit/common/ring/test_builder.py b/test/unit/common/ring/test_builder.py
index 5a1d4faf10..83a2be7edc 100644
--- a/test/unit/common/ring/test_builder.py
+++ b/test/unit/common/ring/test_builder.py
@@ -47,6 +47,18 @@ class TestRingBuilder(unittest.TestCase):
         self.assertEquals(rb.devs_changed, False)
         self.assertEquals(rb.version, 0)
 
+    def test_overlarge_part_powers(self):
+        ring.RingBuilder(32, 3, 1)  # passes by not crashing
+        self.assertRaises(ValueError, ring.RingBuilder, 33, 3, 1)
+
+    def test_insufficient_replicas(self):
+        ring.RingBuilder(8, 1.0, 1)  # passes by not crashing
+        self.assertRaises(ValueError, ring.RingBuilder, 8, 0.999, 1)
+
+    def test_negative_min_part_hours(self):
+        ring.RingBuilder(8, 3, 0)  # passes by not crashing
+        self.assertRaises(ValueError, ring.RingBuilder, 8, 3, -1)
+
     def test_get_ring(self):
         rb = ring.RingBuilder(8, 3, 1)
         rb.add_dev({'id': 0, 'region': 0, 'zone': 0, 'weight': 1,