Merge pull request #74 from engineyard/invalid-parameter-combo

[rds] prevent final snapshot on replicas

lib/fog/aws/requests/rds/create_db_snapshot.rb

@@ -30,6 +30,7 @@ module Fog
           end
 
           server_data = data[:servers][identifier]
+
           unless server_data
             raise Fog::AWS::RDS::NotFound.new("DBInstance #{identifier} not found")
           end

lib/fog/aws/requests/rds/delete_db_instance.rb

@@ -18,10 +18,10 @@ module Fog
           params = {}
           params['FinalDBSnapshotIdentifier'] = snapshot_identifier if snapshot_identifier
           request({
-            'Action'  => 'DeleteDBInstance',
+            'Action'               => 'DeleteDBInstance',
             'DBInstanceIdentifier' => identifier,
-            'SkipFinalSnapshot' => skip_snapshot,
-            :parser   => Fog::Parsers::AWS::RDS::DeleteDBInstance.new
+            'SkipFinalSnapshot'    => skip_snapshot,
+            :parser                => Fog::Parsers::AWS::RDS::DeleteDBInstance.new
           }.merge(params))
         end
       end
@@ -30,20 +30,26 @@ module Fog
         def delete_db_instance(identifier, snapshot_identifier, skip_snapshot = false)
           response = Excon::Response.new
 
+
+          server_set = self.data[:servers][identifier] ||
+            raise(Fog::AWS::RDS::NotFound.new("DBInstance #{identifier} not found"))
+
           unless skip_snapshot
-            create_db_snapshot(identifier, snapshot_identifier)
+            if server_set["ReadReplicaSourceDBInstanceIdentifier"]
+              raise Fog::AWS::RDS::Error.new("InvalidParameterCombination => FinalDBSnapshotIdentifier can not be specified when deleting a replica instance")
+            else
+              create_db_snapshot(identifier, snapshot_identifier)
+            end
           end
 
-          if server_set = self.data[:servers].delete(identifier)
-            response.status = 200
-            response.body = {
-              "ResponseMetadata"=>{ "RequestId"=> Fog::AWS::Mock.request_id },
-              "DeleteDBInstanceResult" => { "DBInstance" => server_set }
-            }
-            response
-          else
-            raise Fog::AWS::RDS::NotFound.new("DBInstance #{identifier} not found")
-          end
+          self.data[:servers].delete(identifier)
+
+          response.status = 200
+          response.body = {
+            "ResponseMetadata"       => { "RequestId"  => Fog::AWS::Mock.request_id },
+            "DeleteDBInstanceResult" => { "DBInstance" => server_set }
+          }
+          response
         end
       end
     end

tests/models/rds/server_tests.rb

@@ -1,7 +1,4 @@
 Shindo.tests("AWS::RDS | server", ['aws', 'rds']) do
-  # Disabled due to https://github.com/fog/fog/1546
-  pending
-
   model_tests(Fog::AWS[:rds].servers, rds_default_server_params) do
     # We'll need this later; create it early to avoid waiting
     @instance_with_final_snapshot = Fog::AWS[:rds].servers.create(rds_default_server_params.merge(:id => uniq_id("fog-snapshot-test"), :backup_retention_period => 1))
@@ -89,6 +86,10 @@ Shindo.tests("AWS::RDS | server", ['aws', 'rds']) do
       returns(@instance_with_final_snapshot.id) { replica.read_replica_source }
 
       replica.wait_for { ready? }
+
+      # FinalDBSnapshotIdentifier can not be specified when deleting a replica instance
+      raises(Fog::AWS::RDS::Error) { replica.destroy("foobar") }
+
       replica.destroy
     end