Merge pull request #1404 from rackspace/identity_fix

Rackspace Cloud Block Storage models have the potential to create unintended resources

lib/fog/rackspace/block_storage.rb

@@ -4,6 +4,7 @@ module Fog
   module Rackspace
     class BlockStorage < Fog::Service
 
+      class IdentifierTaken < Fog::Errors::Error; end
       class ServiceError < Fog::Rackspace::Errors::ServiceError; end
       class InternalServerError < Fog::Rackspace::Errors::InternalServerError; end
       class BadRequest < Fog::Rackspace::Errors::BadRequest; end

lib/fog/rackspace/models/block_storage/snapshot.rb

@@ -26,6 +26,7 @@ module Fog
 
         def save(force = false)
           requires :volume_id
+          raise IdentifierTaken.new('Resaving may cause a duplicate snapshot to be created') if identity
           data = connection.create_snapshot(volume_id, {
             :display_name => display_name,
             :display_description => display_description,

lib/fog/rackspace/models/block_storage/volume.rb

@@ -42,6 +42,7 @@ module Fog
 
         def save
           requires :size
+          raise IdentifierTaken.new('Resaving may cause a duplicate volume to be created') if identity
           data = connection.create_volume(size, {
             :display_name => display_name,
             :display_description => display_description,

tests/rackspace/models/block_storage/snapshot_tests.rb

@@ -3,18 +3,25 @@ Shindo.tests('Fog::Rackspace::BlockStorage | snapshot', ['rackspace']) do
   pending if Fog.mocking?
 
   service = Fog::Rackspace::BlockStorage.new
-  volume = service.volumes.create({
-    :display_name => "fog_#{Time.now.to_i.to_s}",
-    :size => 100
-  })
+  begin
+    volume = service.volumes.create({
+      :display_name => "fog_#{Time.now.to_i.to_s}",
+      :size => 100
+    })
 
-  volume.wait_for { ready? }
+    volume.wait_for { ready? }
 
-  options = { :display_name => "fog_#{Time.now.to_i.to_s}", :volume_id => volume.id }
-  model_tests(service.snapshots, options, false) do
-    @instance.wait_for { ready? }
+    options = { :display_name => "fog_#{Time.now.to_i.to_s}", :volume_id => volume.id }
+    model_tests(service.snapshots, options, false) do
+      @instance.wait_for { ready? }
+    
+      tests('double save').raises(Fog::Rackspace::BlockStorage::IdentifierTaken) do
+        @instance.save
+      end    
+    end
+
+    volume.wait_for { snapshots.empty? }
+  ensure
+    volume.destroy if volume
   end
-
-  volume.wait_for { snapshots.empty? }
-  volume.destroy
 end

tests/rackspace/models/block_storage/volume_tests.rb

@@ -6,7 +6,11 @@ Shindo.tests('Fog::Rackspace::BlockStorage | volume', ['rackspace']) do
   options = { :display_name => "fog_#{Time.now.to_i.to_s}", :size => 100 }
 
   model_tests(service.volumes, options, false) do
-    @instance.wait_for { ready? }
+    @instance.wait_for(timeout=1200) { ready? }
+
+    tests('double save').raises(Fog::Rackspace::BlockStorage::IdentifierTaken) do
+      @instance.save
+    end
 
     tests('#attached?').succeeds do
       @instance.state = 'in-use'
@@ -20,7 +24,7 @@ Shindo.tests('Fog::Rackspace::BlockStorage | volume', ['rackspace']) do
 
         returns(true) { @instance.snapshots.first.id == snapshot.id }
       ensure
-        snapshot.destroy
+        snapshot.destroy if snapshot
       end
     end