refactor security group mocks to use groupId as the key

This prevents group name collisions.  Specfically, default security
group name collisions

lib/fog/aws/requests/compute/authorize_security_group_egress.rb

@@ -57,14 +57,16 @@ module Fog
       class Mock
         def authorize_security_group_egress(group_name, options = {})
           options = Fog::AWS.parse_security_group_options(group_name, options)
-          if options.key?('GroupName')
+
-            group_name = options['GroupName']
+          group = if options.key?('GroupName')
-          else
+                    self.data[:security_groups].values.find { |v| v['groupName'] == options['GroupName'] }
-            group_name = self.data[:security_groups].reject { |k,v| v['groupId'] != options['GroupId'] } .keys.first
+                  else
-          end
+                    self.data[:security_groups][options.fetch('GroupId')]
+                  end
 
           response = Excon::Response.new
-          group = self.data[:security_groups][group_name] || raise(Fog::Compute::AWS::NotFound.new("The security group '#{group_name}' does not exist"))
+          group ||
+            raise(Fog::Compute::AWS::NotFound.new("The security group '#{group_name}' does not exist"))
 
           verify_permission_options(options, group['vpcId'] != nil)
 

lib/fog/aws/requests/compute/authorize_security_group_ingress.rb

@@ -80,14 +80,16 @@ module Fog
       class Mock
         def authorize_security_group_ingress(group_name, options = {})
           options = Fog::AWS.parse_security_group_options(group_name, options)
-          if options.key?('GroupName')
+
-            group_name = options['GroupName']
+          group = if options.key?('GroupName')
-          else
+                    self.data[:security_groups].values.find { |v| v['groupName'] == options['GroupName'] }
-            group_name = self.data[:security_groups].reject { |k,v| v['groupId'] != options['GroupId'] } .keys.first
+                  else
-          end
+                    self.data[:security_groups][options.fetch('GroupId')]
+                  end
 
           response = Excon::Response.new
-          group = self.data[:security_groups][group_name] || raise(Fog::Compute::AWS::NotFound.new("The security group '#{group_name}' does not exist"))
+          group ||
+            raise(Fog::Compute::AWS::NotFound.new("The security group '#{group_name}' does not exist"))
 
           verify_permission_options(options, group['vpcId'] != nil)
 
@@ -150,14 +152,15 @@ module Fog
                          else
                            options['SourceSecurityGroupName']
                          end
-            source_group_id=self.data[:security_groups][group_name]['groupId']
+            source_group_id, _ = self.data[:security_groups].find { |_,v| v['groupName'] == group_name }
+
             ['tcp', 'udp'].each do |protocol|
               normalized_permissions << {
                 'ipProtocol' => protocol,
                 'fromPort'   => 1,
                 'toPort'     => 65535,
                 'groups'     => [{
-                  'groupName' => options['SourceSecurityGroupName'],
+                  'groupName' => group_name,
                   'userId'    => options['SourceSecurityGroupOwnerId'] || self.data[:owner_id],
                   'groupId'   => source_group_id
                 }],
@@ -169,7 +172,7 @@ module Fog
               'fromPort'   => -1,
               'toPort'     => -1,
               'groups'     => [{
-                'groupName' => options['SourceSecurityGroupName'],
+                'groupName' => group_name,
                 'userId'    => options['SourceSecurityGroupOwnerId'] || self.data[:owner_id],
                 'groupId'   => source_group_id
               }],
@@ -188,18 +191,19 @@ module Fog
 
               groups = (permission['Groups'] || []).map do |authorized_group|
                 security_group = if group_name = authorized_group['GroupName']
-                                   self.data[:security_groups][group_name]
+                                   self.data[:security_groups].values.find { |sg| sg['groupName'] == group_name }
                                  elsif group_id = authorized_group['GroupId']
-                                   self.data[:security_groups].values.find { |sg| sg['groupId'] == group_id }
+                                   self.data[:security_groups][group_id]
-                                 end ||
+                                 end
-                                 raise(Fog::Compute::AWS::NotFound.new("The security group '#{group_name || group_id}' does not exist"))
+                security_group ||
+                  raise(Fog::Compute::AWS::NotFound.new("The security group '#{group_name || group_id}' does not exist"))
 
-                                 {
+                {
-                                   'groupName' => authorized_group['GroupName'] || security_group["groupName"],
+                  'groupName' => authorized_group['GroupName'] || security_group['groupName'],
-                                   'userId'    => authorized_group['UserId']    || self.data[:owner_id],
+                  'userId'    => authorized_group['UserId']    || self.data[:owner_id],
-                                   'groupId'   => authorized_group["GroupId"]   || security_group['groupId']
+                  'groupId'   => authorized_group["GroupId"]   || security_group['groupId']
-                                 }
+                }
-                  end
+              end
 
 
               if ['tcp', 'udp', 'icmp'].include?(permission['IpProtocol'])
@@ -208,7 +212,7 @@ module Fog
                   'fromPort'   => Integer(permission['FromPort']),
                   'toPort'     => Integer(permission['ToPort']),
                   'groups'     => groups,
-                  'ipRanges' => (permission['IpRanges'] || []).map {|r| { 'cidrIp' => r['CidrIp'] } }
+                  'ipRanges'   => (permission['IpRanges'] || []).map {|r| { 'cidrIp' => r['CidrIp'] } }
                 }
               else
                 normalized_permissions << {

lib/fog/aws/requests/compute/create_network_interface.rb

@@ -73,11 +73,11 @@ module Fog
               groups = {}
               if options['GroupSet']
                 options['GroupSet'].each do |group_id|
-                  group_obj = self.data[:security_groups].select { |k,v| v['groupId'] == group_id }.first
+                  group_obj = self.data[:security_groups][group_id]
                   if group_obj.nil?
                     raise Fog::Compute::AWS::Error.new("Unknown security group '#{group_id}' specified")
                   end
-                  groups[group_id] = group_obj
+                  groups[group_id] = group_obj['groupName']
                 end
               end
 

lib/fog/aws/requests/compute/create_security_group.rb

@@ -35,27 +35,29 @@ module Fog
           response = Excon::Response.new
 
           vpc_id ||= Fog::AWS::Mock.default_vpc_for(region)
+          group_id = Fog::AWS::Mock.security_group_id
 
-          unless self.data[:security_groups][name]
+          if self.data[:security_groups].find { |_,v| v['groupName'] == name }
-            data = {
+            raise Fog::Compute::AWS::Error,
-              'groupDescription'    => description,
+              "InvalidGroup.Duplicate => The security group '#{name}' already exists"
-              'groupName'           => name,
-              'groupId'             => Fog::AWS::Mock.security_group_id,
-              'ipPermissionsEgress' => [],
-              'ipPermissions'       => [],
-              'ownerId'             => self.data[:owner_id],
-              'vpcId'               => vpc_id
-            }
-            self.data[:security_groups][name] = data
-            response.body = {
-              'requestId' => Fog::AWS::Mock.request_id,
-              'groupId'   => data['groupId'],
-              'return'    => true
-            }
-            response
-          else
-            raise Fog::Compute::AWS::Error.new("InvalidGroup.Duplicate => The security group '#{name}' already exists")
           end
+
+          self.data[:security_groups][group_id] = {
+            'groupDescription'    => description,
+            'groupName'           => name,
+            'groupId'             => group_id,
+            'ipPermissionsEgress' => [],
+            'ipPermissions'       => [],
+            'ownerId'             => self.data[:owner_id],
+            'vpcId'               => vpc_id
+          }
+
+          response.body = {
+            'requestId' => Fog::AWS::Mock.request_id,
+            'groupId'   => group_id,
+            'return'    => true
+          }
+          response
         end
       end
     end

lib/fog/aws/requests/compute/create_vpc.rb

@@ -80,20 +80,24 @@ module Fog
 
               # create default security groups
               default_elb_group_name = "default_elb_#{Fog::Mock.random_hex(6)}"
+              default_elb_group_id = Fog::AWS::Mock.security_group_id
 
-              Fog::Compute::AWS::Mock.data[region][@aws_access_key_id][:security_groups][default_elb_group_name] = {
+              Fog::Compute::AWS::Mock.data[region][@aws_access_key_id][:security_groups][default_elb_group_id] = {
                 'groupDescription'    => 'default_elb security group',
                 'groupName'           => default_elb_group_name,
-                'groupId'             => Fog::AWS::Mock.security_group_id,
+                'groupId'             => default_elb_group_id,
                 'ipPermissions'       => [],
                 'ownerId'             => self.data[:owner_id],
                 'vpcId'               => vpc_id
               }
 
-              Fog::Compute::AWS::Mock.data[region][@aws_access_key_id][:security_groups]['default'] = {
+              default_group_name = 'default'
-                'groupDescription'    => 'default',
+              default_group_id = Fog::AWS::Mock.security_group_id
-                'groupName'           => 'default',
+
-                'groupId'             => Fog::AWS::Mock.security_group_id,
+              Fog::Compute::AWS::Mock.data[region][@aws_access_key_id][:security_groups][default_group_id] = {
+                'groupDescription'    => default_group_name,
+                'groupName'           => default_group_name,
+                'groupId'             => default_group_id,
                 'ipPermissions'       => [],
                 'ownerId'             => self.data[:owner_id],
                 'vpcId'               => vpc_id

lib/fog/aws/requests/compute/delete_security_group.rb

@@ -47,12 +47,12 @@ module Fog
             raise Fog::Compute::AWS::Error.new("May not specify both group_name and group_id")
           end
 
-          if id
+          if name
-            name = self.data[:security_groups].reject { |k,v| v['groupId'] != id } .keys.first
+            id, _ = self.data[:security_groups].find { |_,v| v['groupName'] == name }
           end
 
-          unless self.data[:security_groups][name]
+          unless self.data[:security_groups][id]
-            raise Fog::Compute::AWS::NotFound.new("The security group '#{name}' does not exist")
+            raise Fog::Compute::AWS::NotFound.new("The security group '#{id}' does not exist")
           end
 
           response = Excon::Response.new
@@ -61,14 +61,14 @@ module Fog
 
           # ec2 authorizations
           self.region_data.each do |_, key_data|
-            key_data[:security_groups].each do |group_name, group|
+            key_data[:security_groups].each do |group_id, group|
-              next if group == self.data[:security_groups][name]
+              next if group == self.data[:security_groups][group_id]
 
               group['ipPermissions'].each do |group_ip_permission|
                 group_ip_permission['groups'].each do |group_group_permission|
-                  if group_group_permission['groupName'] == name &&
+                  if group_group_permission['groupId'] == group_id &&
                       group_group_permission['userId'] == self.data[:owner_id]
-                    used_by_groups << "#{key_data[:owner_id]}:#{group_name}"
+                    used_by_groups << "#{key_data[:owner_id]}:#{group['groupName']}"
                   end
                 end
               end
@@ -100,7 +100,7 @@ module Fog
             raise Fog::Compute::AWS::Error.new("InUse => There are active instances using security group '#{name}'")
           end
 
-          self.data[:security_groups].delete(name)
+          self.data[:security_groups].delete(id)
           response.status = 200
           response.body = {
             'requestId' => Fog::AWS::Mock.request_id,

lib/fog/aws/requests/compute/describe_network_interface_attribute.rb

@@ -42,23 +42,25 @@ module Fog
       class Mock
         def describe_network_interface_attribute(network_interface_id, attribute)
           response = Excon::Response.new
-          if self.data[:network_interfaces][network_interface_id]
+          network_interface = self.data[:network_interfaces][network_interface_id]
 
-            response.status = 200
+
-            response.body = {
+          unless network_interface
-              'requestId'          => Fog::AWS::Mock.request_id,
-              'networkInterfaceId' => network_interface_id
-            }
-            case attribute
-            when 'description', 'groupSet', 'sourceDestCheck', 'attachment'
-              response.body[attribute] = self.data[:network_interfaces][network_interface_id][attribute]
-            else
-              raise Fog::Compute::AWS::Error.new("Illegal attribute '#{attribute}' specified")
-            end
-            response
-          else
             raise Fog::Compute::AWS::NotFound.new("The network interface '#{network_interface_id}' does not exist")
           end
+
+          response.status = 200
+          response.body = {
+            'requestId'          => Fog::AWS::Mock.request_id,
+            'networkInterfaceId' => network_interface_id
+          }
+          case attribute
+          when 'description', 'groupSet', 'sourceDestCheck', 'attachment'
+            response.body[attribute] = network_interface[attribute]
+          else
+            raise Fog::Compute::AWS::Error.new("Illegal attribute '#{attribute}' specified")
+          end
+          response
         end
       end
     end

lib/fog/aws/requests/compute/describe_security_groups.rb

@@ -66,22 +66,46 @@ module Fog
             'protocol'  => 'ipProtocol',
             'to-port'   => 'toPort'
           }
-          security_group_groups = lambda { |security_group| (security_group['ipPermissions'] || []).map { |permission| permission["groups"] }.flatten.compact.uniq }
+
+          security_group_groups = lambda do |security_group|
+            (security_group['ipPermissions'] || []).map do |permission|
+              permission['groups']
+            end.flatten.compact.uniq
+          end
+
           for filter_key, filter_value in filters
             if permission_key = filter_key.split('ip-permission.')[1]
               if permission_key == 'group-name'
-                security_group_info = security_group_info.reject{|security_group| !security_group_groups.call(security_group).find {|group| [*filter_value].include?(group['groupName'])}}
+                security_group_info = security_group_info.reject do |security_group|
+                  !security_group_groups.call(security_group).find do |group|
+                    [*filter_value].include?(group['groupName'])
+                  end
+                end
               elsif permission_key == 'group-id'
-                security_group_info = security_group_info.reject{|security_group| !security_group_groups.call(security_group).find {|group| [*filter_value].include?(group['groupId'])}}
+                security_group_info = security_group_info.reject do |security_group|
+                  !security_group_groups.call(security_group).find do |group|
+                    [*filter_value].include?(group['groupId'])
+                  end
+                end
               elsif permission_key == 'user-id'
-                security_group_info = security_group_info.reject{|security_group| !security_group_groups.call(security_group).find {|group| [*filter_value].include?(group['userId'])}}
+                security_group_info = security_group_info.reject do |security_group|
+                  !security_group_groups.call(security_group).find do |group|
+                    [*filter_value].include?(group['userId'])
+                  end
+                end
               else
                 aliased_key = permission_aliases[filter_key]
-                security_group_info = security_group_info.reject{|security_group| !security_group['ipPermissions'].find {|permission| [*filter_value].include?(permission[aliased_key])}}
+                security_group_info = security_group_info.reject do |security_group|
+                  !security_group['ipPermissions'].find do |permission|
+                    [*filter_value].include?(permission[aliased_key])
+                  end
+                end
               end
             else
               aliased_key = aliases[filter_key]
-              security_group_info = security_group_info.reject{|security_group| ![*filter_value].include?(security_group[aliased_key])}
+              security_group_info = security_group_info.reject do |security_group|
+                ![*filter_value].include?(security_group[aliased_key])
+              end
             end
           end
 

lib/fog/aws/requests/compute/modify_network_interface_attribute.rb

@@ -54,11 +54,11 @@ module Fog
             when 'groupSet'
               groups = {}
               value.each do |group_id|
-                name = self.data[:security_groups].select { |k,v| v['groupId'] == group_id } .first.first
+                security_group = self.data[:security_groups][group_id]
-                if name.nil?
+                if security_group.nil?
                   raise Fog::Compute::AWS::Error.new("Unknown security group '#{group_id}' specified")
                 end
-                groups[group_id] = name
+                groups[group_id] = security_group['groupName']
               end
               nic['groupSet'] = groups
             when 'sourceDestCheck'

lib/fog/aws/requests/compute/revoke_security_group_egress.rb

@@ -57,40 +57,34 @@ module Fog
       class Mock
         def revoke_security_group_egress(group_name, options = {})
           options = Fog::AWS.parse_security_group_options(group_name, options)
-          if options.key?('GroupName')
-            group_name = options['GroupName']
-          else
-            group_name = self.data[:security_groups].reject { |k,v| v['groupId'] != options['GroupId'] } .keys.first
-          end
 
+          group = self.data[:security_groups].values.find { |v| v['groupName'] == group_name }
+
+          group ||
+            raise(Fog::Compute::AWS::NotFound.new("The security group '#{group_name}' does not exist"))
           response = Excon::Response.new
-          group = self.data[:security_groups][group_name]
 
-          if group
+          verify_permission_options(options, group['vpcId'] != nil)
-            verify_permission_options(options, group['vpcId'] != nil)
 
-            normalized_permissions = normalize_permissions(options)
+          normalized_permissions = normalize_permissions(options)
 
-            normalized_permissions.each do |permission|
+          normalized_permissions.each do |permission|
-              if matching_permission = find_matching_permission_egress(group, permission)
+            if matching_permission = find_matching_permission_egress(group, permission)
-                matching_permission['ipRanges'] -= permission['ipRanges']
+              matching_permission['ipRanges'] -= permission['ipRanges']
-                matching_permission['groups'] -= permission['groups']
+              matching_permission['groups'] -= permission['groups']
 
-                if matching_permission['ipRanges'].empty? && matching_permission['groups'].empty?
+              if matching_permission['ipRanges'].empty? && matching_permission['groups'].empty?
-                  group['ipPermissionsEgress'].delete(matching_permission)
+                group['ipPermissionsEgress'].delete(matching_permission)
-                end
               end
             end
-
-            response.status = 200
-            response.body = {
-              'requestId' => Fog::AWS::Mock.request_id,
-              'return'    => true
-            }
-            response
-          else
-            raise Fog::Compute::AWS::NotFound.new("The security group '#{group_name}' does not exist")
           end
+
+          response.status = 200
+          response.body = {
+            'requestId' => Fog::AWS::Mock.request_id,
+            'return'    => true
+          }
+          response
         end
       end
     end

lib/fog/aws/requests/compute/revoke_security_group_ingress.rb

@@ -57,40 +57,33 @@ module Fog
       class Mock
         def revoke_security_group_ingress(group_name, options = {})
           options = Fog::AWS.parse_security_group_options(group_name, options)
-          if options.key?('GroupName')
+          group = self.data[:security_groups].values.find { |v| v['groupName'] == group_name }
-            group_name = options['GroupName']
-          else
-            group_name = self.data[:security_groups].reject { |k,v| v['groupId'] != options['GroupId'] } .keys.first
-          end
 
+          group ||
+            raise(Fog::Compute::AWS::NotFound.new("The security group '#{group_name}' does not exist"))
           response = Excon::Response.new
-          group = self.data[:security_groups][group_name]
 
-          if group
+          verify_permission_options(options, group['vpcId'] != nil)
-            verify_permission_options(options, group['vpcId'] != nil)
 
-            normalized_permissions = normalize_permissions(options)
+          normalized_permissions = normalize_permissions(options)
 
-            normalized_permissions.each do |permission|
+          normalized_permissions.each do |permission|
-              if matching_permission = find_matching_permission(group, permission)
+            if matching_permission = find_matching_permission(group, permission)
-                matching_permission['ipRanges'] -= permission['ipRanges']
+              matching_permission['ipRanges'] -= permission['ipRanges']
-                matching_permission['groups'] -= permission['groups']
+              matching_permission['groups'] -= permission['groups']
 
-                if matching_permission['ipRanges'].empty? && matching_permission['groups'].empty?
+              if matching_permission['ipRanges'].empty? && matching_permission['groups'].empty?
-                  group['ipPermissions'].delete(matching_permission)
+                group['ipPermissions'].delete(matching_permission)
-                end
               end
             end
-
-            response.status = 200
-            response.body = {
-              'requestId' => Fog::AWS::Mock.request_id,
-              'return'    => true
-            }
-            response
-          else
-            raise Fog::Compute::AWS::NotFound.new("The security group '#{group_name}' does not exist")
           end
+
+          response.status = 200
+          response.body = {
+            'requestId' => Fog::AWS::Mock.request_id,
+            'return'    => true
+          }
+          response
         end
       end
     end

lib/fog/aws/requests/efs/create_mount_target.rb

@@ -25,8 +25,10 @@ module Fog
       class Mock
         def create_mount_target(file_system_id, subnet_id, options={})
           response               = Excon::Response.new
-          default_security_group = mock_compute.data[:security_groups]['default']
+          default_security_group = mock_compute.data[:security_groups].find do |_, sg|
-          security_groups        = options["SecurityGroups"] || [default_security_group['groupId']]
+            sg['groupDescription'] == 'default_elb security group'
+          end
+          security_groups        = options["SecurityGroups"] || [default_security_group.first]
 
           unless file_system = self.data[:file_systems][file_system_id]
             raise Fog::AWS::EFS::NotFound.new("invalid file system ID: #{file_system_id}")
@@ -41,8 +43,10 @@ module Fog
             raise Fog::AWS::EFS::InvalidSubnet.new("invalid subnet ID: #{subnet_id}")
           end
 
-          security_groups.each do |sg|
+          security_groups.each do |group_id|
-            raise Fog::AWS::EFS::NotFound.new("invalid security group ID: #{sg}") unless mock_compute.data[:security_groups].values.detect { |sgd| sgd["groupId"] == sg }
+            unless mock_compute.data[:security_groups][group_id]
+              raise Fog::AWS::EFS::NotFound.new("invalid security group ID: #{group_id}")
+            end
           end
 
           id = "fsmt-#{Fog::Mock.random_letters(8)}"

tests/models/efs/mount_target_tests.rb

@@ -5,7 +5,9 @@ Shindo.tests("AWS::EFS | mount target", ["aws", "efs"]) do
   if Fog.mocking?
     vpc = Fog::Compute[:aws].vpcs.create(:cidr_block => "10.0.0.0/16")
     subnet = Fog::Compute[:aws].subnets.create(:vpc_id => vpc.id, :cidr_block => "10.0.1.0/24")
-    default_security_group_data = Fog::Compute[:aws].data[:security_groups]['default']
+    default_security_group_data = Fog::Compute[:aws].data[:security_groups].values.find do |sg|
+      sg['groupDescription'] == 'default_elb security group'
+    end
     default_security_group = Fog::Compute[:aws].security_groups.new(default_security_group_data)
   else
     vpc = Fog::Compute[:aws].vpcs.first

tests/requests/compute/network_interface_tests.rb

@@ -47,18 +47,6 @@ Shindo.tests('Fog::Compute[:aws] | network interface requests', ['aws']) do
     @subnet_id         = @subnet.subnet_id
     @security_group_id = @security_group.group_id
 
-    @security_groups = [
-      @security_group.name, {
-        'groupDescription'  => @security_group.description,
-        'groupName'         => @security_group.name,
-        'groupId'           => @security_group_id,
-        'ipPermissionsEgress' => [],
-        'ipPermissions'     => [],
-        'ownerId'           => @owner_id,
-        'vpcId'             => @vpc.id
-      }
-    ]
-
     DESCRIPTION = "Small and green"
     tests("#create_network_interface(#{@subnet_id})").formats(@network_interface_create_format) do
       data = Fog::Compute[:aws].create_network_interface(@subnet_id, {"PrivateIpAddress" => "10.0.10.23"}).body
@@ -164,7 +152,8 @@ Shindo.tests('Fog::Compute[:aws] | network interface requests', ['aws']) do
     tests("#describe_network_interface_attribute(#{@nic2_id}, 'description')").returns(DESCRIPTION) do
       Fog::Compute[:aws].describe_network_interface_attribute(@nic2_id, 'description').body["description"]
     end
-    tests("#describe_network_interface_attribute(#{@nic2_id}, 'groupSet')").returns({ @security_group_id => @security_groups }) do
+
+    tests("#describe_network_interface_attribute(#{@nic2_id}, 'groupSet')").returns({ @security_group_id => @security_group.name }) do
       Fog::Compute[:aws].describe_network_interface_attribute(@nic2_id, 'groupSet').body["groupSet"]
     end