Skip to content
This repository

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse code

Improved support for SecurityGroup IDs

This patch makes it possible to specify GroupID in the options hash to the
aws computre requests operating on security groups. This is needed since
when working with VPC you must use GroupID instead of name.
  • Loading branch information...
commit 4402d6690f16c6694c392abe93b0ad20bf87c40e 1 parent defb7cc
Martin Forssen authored
23 lib/fog/aws.rb
@@ -221,6 +221,10 @@ def self.volume_id
221 221 "vol-#{Fog::Mock.random_hex(8)}"
222 222 end
223 223
  224 + def self.security_group_id
  225 + "sg-#{Fog::Mock.random_hex(8)}"
  226 + end
  227 +
224 228 def self.key_id(length=21)
225 229 #Probably close enough
226 230 Fog::Mock.random_selection('ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789',length)
@@ -230,5 +234,24 @@ def self.rds_address(db_name,region)
230 234 "#{db_name}.#{Fog::Mock.random_letters(rand(12) + 4)}.#{region}.rds.amazonaws.com"
231 235 end
232 236 end
  237 +
  238 + def self.parse_security_group_options(group_name, options)
  239 + if group_name.is_a?(Hash)
  240 + options = group_name
  241 + elsif group_name
  242 + if options.key?('GroupName')
  243 + raise Fog::Compute::AWS::Error, 'Arguments specified both group_name and GroupName in options'
  244 + end
  245 + options = options.clone
  246 + options['GroupName'] = group_name
  247 + end
  248 + if !options.key?('GroupName') && !options.key?('GroupId')
  249 + raise Fog::Compute::AWS::Error, 'Neither GroupName nor GroupId specified'
  250 + end
  251 + if options.key?('GroupName') && options.key?('GroupId')
  252 + raise Fog::Compute::AWS::Error, 'Both GroupName and GroupId specified'
  253 + end
  254 + options
  255 + end
233 256 end
234 257 end
1  lib/fog/aws/compute.rb
@@ -124,6 +124,7 @@ def self.data
124 124 'default' => {
125 125 'groupDescription' => 'default group',
126 126 'groupName' => 'default',
  127 + 'groupId' => 'sg-11223344',
127 128 'ipPermissionsEgress' => [],
128 129 'ipPermissions' => [
129 130 {
22 lib/fog/aws/requests/compute/authorize_security_group_ingress.rb
@@ -8,8 +8,10 @@ class Real
8 8 # Add permissions to a security group
9 9 #
10 10 # ==== Parameters
11   - # * group_name<~String> - Name of group
  11 + # * group_name<~String> - Name of group, optional (can also be specifed as GroupName in options)
12 12 # * options<~Hash>:
  13 + # * 'GroupName'<~String> - Name of security group to modify
  14 + # * 'GroupId'<~String> - Id of security group to modify
13 15 # * 'SourceSecurityGroupName'<~String> - Name of security group to authorize
14 16 # * 'SourceSecurityGroupOwnerId'<~String> - Name of owner to authorize
15 17 # or
@@ -39,11 +41,7 @@ class Real
39 41 #
40 42 # {Amazon API Reference}[http://docs.amazonwebservices.com/AWSEC2/latest/APIReference/ApiReference-query-AuthorizeSecurityGroupIngress.html]
41 43 def authorize_security_group_ingress(group_name, options = {})
42   - if group_name.is_a?(Hash)
43   - Fog::Logger.deprecation("Fog::AWS::Compute#authorize_security_group_ingress now requires the 'group_name' parameter. Only specifying an options hash is now deprecated [light_black](#{caller.first})[/]")
44   - options = group_name
45   - group_name = options.delete('GroupName')
46   - end
  44 + options = Fog::AWS.parse_security_group_options(group_name, options)
47 45
48 46 if ip_permissions = options.delete('IpPermissions')
49 47 options.merge!(indexed_ip_permissions_params(ip_permissions))
@@ -51,7 +49,6 @@ def authorize_security_group_ingress(group_name, options = {})
51 49
52 50 request({
53 51 'Action' => 'AuthorizeSecurityGroupIngress',
54   - 'GroupName' => group_name,
55 52 :idempotent => true,
56 53 :parser => Fog::Parsers::Compute::AWS::Basic.new
57 54 }.merge!(options))
@@ -85,10 +82,11 @@ def indexed_ip_permissions_params(ip_permissions)
85 82 class Mock
86 83
87 84 def authorize_security_group_ingress(group_name, options = {})
88   - if group_name.is_a?(Hash)
89   - Fog::Logger.deprecation("Fog::AWS::Compute#authorize_security_group_ingress now requires the 'group_name' parameter. Only specifying an options hash is now deprecated [light_black](#{caller.first})[/]")
90   - options = group_name
91   - group_name = options.delete('GroupName')
  85 + options = Fog::AWS.parse_security_group_options(group_name, options)
  86 + if options.key?('GroupName')
  87 + group_name = options['GroupName']
  88 + else
  89 + group_name = self.data[:security_groups].reject { |k,v| v['groupId'] != options['GroupId'] } .keys.first
92 90 end
93 91
94 92 verify_permission_options(options)
@@ -134,7 +132,7 @@ def authorize_security_group_ingress(group_name, options = {})
134 132 private
135 133
136 134 def verify_permission_options(options)
137   - if options.empty?
  135 + if options.size <= 1
138 136 raise Fog::Compute::AWS::Error.new("InvalidRequest => The request received was invalid.")
139 137 end
140 138 if options['IpProtocol'] && !['tcp', 'udp', 'icmp'].include?(options['IpProtocol'])
2  lib/fog/aws/requests/compute/create_security_group.rb
@@ -10,6 +10,7 @@ class Real
10 10 # ==== Parameters
11 11 # * group_name<~String> - Name of the security group.
12 12 # * group_description<~String> - Description of group.
  13 + # * vpc_id<~String> - ID of the VPC
13 14 #
14 15 # ==== Returns
15 16 # * response<~Excon::Response>:
@@ -38,6 +39,7 @@ def create_security_group(name, description, vpc_id=nil)
38 39 data = {
39 40 'groupDescription' => description,
40 41 'groupName' => name,
  42 + 'groupId' => Fog::AWS::Mock.security_group_id,
41 43 'ipPermissionsEgress' => [],
42 44 'ipPermissions' => [],
43 45 'ownerId' => self.data[:owner_id],
26 lib/fog/aws/requests/compute/delete_security_group.rb
@@ -8,7 +8,8 @@ class Real
8 8 # Delete a security group that you own
9 9 #
10 10 # ==== Parameters
11   - # * group_name<~String> - Name of the security group.
  11 + # * group_name<~String> - Name of the security group, must be nil if id is specified
  12 + # * group_id<~String> - Id of the security group, must be nil if name is specified
12 13 #
13 14 # ==== Returns
14 15 # * response<~Excon::Response>:
@@ -17,10 +18,20 @@ class Real
17 18 # * 'return'<~Boolean> - success?
18 19 #
19 20 # {Amazon API Reference}[http://docs.amazonwebservices.com/AWSEC2/latest/APIReference/ApiReference-query-DeleteSecurityGroup.html]
20   - def delete_security_group(name)
  21 + def delete_security_group(name, id = nil)
  22 + if name && id
  23 + raise Fog::Compute::AWS::Error.new("May not specify both group_name and group_id")
  24 + end
  25 + if name
  26 + type_id = 'GroupName'
  27 + identifier = name
  28 + else
  29 + type_id = 'GroupId'
  30 + identifier = id
  31 + end
21 32 request(
22 33 'Action' => 'DeleteSecurityGroup',
23   - 'GroupName' => name,
  34 + type_id => identifier,
24 35 :idempotent => true,
25 36 :parser => Fog::Parsers::Compute::AWS::Basic.new
26 37 )
@@ -29,11 +40,18 @@ def delete_security_group(name)
29 40 end
30 41
31 42 class Mock
32   - def delete_security_group(name)
  43 + def delete_security_group(name, id = nil)
33 44 if name == 'default'
34 45 raise Fog::Compute::AWS::Error.new("InvalidGroup.Reserved => The security group 'default' is reserved")
35 46 end
36 47
  48 + if name && id
  49 + raise Fog::Compute::AWS::Error.new("May not specify both group_name and group_id")
  50 + end
  51 + if id
  52 + name = self.data[:security_groups].reject { |k,v| v['groupId'] != id } .keys.first
  53 + end
  54 +
37 55 response = Excon::Response.new
38 56 if self.data[:security_groups][name]
39 57
2  lib/fog/aws/requests/compute/describe_security_groups.rb
@@ -73,6 +73,8 @@ def describe_security_groups(filters = {})
73 73 if permission_key = filter_key.split('ip-permission.')[1]
74 74 if permission_key == 'group-name'
75 75 security_group_info = security_group_info.reject{|security_group| !security_group['ipPermissions']['groups'].detect {|group| [*filter_value].include?(group['groupName'])}}
  76 + elsif permission_key == 'group-id'
  77 + security_group_info = security_group_info.reject{|security_group| !security_group['ipPermissions']['groups'].detect {|group| [*filter_value].include?(group['groupId'])}}
76 78 elsif permission_key == 'user-id'
77 79 security_group_info = security_group_info.reject{|security_group| !security_group['ipPermissions']['groups'].detect {|group| [*filter_value].include?(group['userId'])}}
78 80 else
6 lib/fog/aws/requests/compute/request_spot_instances.rb
@@ -25,7 +25,8 @@ class Real
25 25 # * 'LaunchSpecification.KeyName'<~String> - Name of a keypair to add to booting instances
26 26 # * 'LaunchSpecification.Monitoring.Enabled'<~Boolean> - Enables monitoring, defaults to disabled
27 27 # * 'LaunchSpecification.Placement.AvailabilityZone'<~String> - Placement constraint for instances
28   - # * 'LaunchSpecification.SecurityGroup'<~Array> or <~String> - Name of security group(s) for instances
  28 + # * 'LaunchSpecification.SecurityGroup'<~Array> or <~String> - Name of security group(s) for instances, not supported in VPC
  29 + # * 'LaunchSpecification.SecurityGroupId'<~Array> or <~String> - Id of security group(s) for instances, use this or LaunchSpecification.SecurityGroup
29 30 # * 'LaunchSpecification.UserData'<~String> - Additional data to provide to booting instances
30 31 # * 'Type'<~String> - spot instance request type in ['one-time', 'persistent']
31 32 # * 'ValidFrom'<~Time> - start date for request
@@ -64,6 +65,9 @@ def request_spot_instances(image_id, instance_type, spot_price, options = {})
64 65 if security_groups = options.delete('LaunchSpecification.SecurityGroup')
65 66 options.merge!(Fog::AWS.indexed_param('LaunchSpecification.SecurityGroup', [*security_groups]))
66 67 end
  68 + if security_group_ids = options.delete('LaunchSpecification.SecurityGroupId')
  69 + options.merge!(Fog::AWS.indexed_param('LaunchSpecification.SecurityGroupId', [*security_group_ids]))
  70 + end
67 71 if options['LaunchSpecification.UserData']
68 72 options['LaunchSpecification.UserData'] = Base64.encode64(options['LaunchSpecification.UserData'])
69 73 end
20 lib/fog/aws/requests/compute/revoke_security_group_ingress.rb
@@ -8,8 +8,10 @@ class Real
8 8 # Remove permissions from a security group
9 9 #
10 10 # ==== Parameters
11   - # * group_name<~String> - Name of group
  11 + # * group_name<~String> - Name of group, optional (can also be specifed as GroupName in options)
12 12 # * options<~Hash>:
  13 + # * 'GroupName'<~String> - Name of security group to modify
  14 + # * 'GroupId'<~String> - Id of security group to modify
13 15 # * 'SourceSecurityGroupName'<~String> - Name of security group to authorize
14 16 # * 'SourceSecurityGroupOwnerId'<~String> - Name of owner to authorize
15 17 # or
@@ -39,11 +41,7 @@ class Real
39 41 #
40 42 # {Amazon API Reference}[http://docs.amazonwebservices.com/AWSEC2/latest/APIReference/ApiReference-query-RevokeSecurityGroupIngress.html]
41 43 def revoke_security_group_ingress(group_name, options = {})
42   - if group_name.is_a?(Hash)
43   - Fog::Logger.deprecation("Fog::AWS::Compute#revoke_security_group_ingress now requires the 'group_name' parameter. Only specifying an options hash is now deprecated [light_black](#{caller.first})[/]")
44   - options = group_name
45   - group_name = options.delete('GroupName')
46   - end
  44 + options = Fog::AWS.parse_security_group_options(group_name, options)
47 45
48 46 if ip_permissions = options.delete('IpPermissions')
49 47 options.merge!(indexed_ip_permissions_params(ip_permissions))
@@ -51,7 +49,6 @@ def revoke_security_group_ingress(group_name, options = {})
51 49
52 50 request({
53 51 'Action' => 'RevokeSecurityGroupIngress',
54   - 'GroupName' => group_name,
55 52 :idempotent => true,
56 53 :parser => Fog::Parsers::Compute::AWS::Basic.new
57 54 }.merge!(options))
@@ -62,10 +59,11 @@ def revoke_security_group_ingress(group_name, options = {})
62 59 class Mock
63 60
64 61 def revoke_security_group_ingress(group_name, options = {})
65   - if group_name.is_a?(Hash)
66   - Fog::Logger.deprecation("Fog::AWS::Compute#revoke_security_group_ingress now requires the 'group_name' parameter. Only specifying an options hash is now deprecated [light_black](#{caller.first})[/]")
67   - options = group_name
68   - group_name = options.delete('GroupName')
  62 + options = Fog::AWS.parse_security_group_options(group_name, options)
  63 + if options.key?('GroupName')
  64 + group_name = options['GroupName']
  65 + else
  66 + group_name = self.data[:security_groups].reject { |k,v| v['groupId'] != options['GroupId'] } .keys.first
69 67 end
70 68
71 69 verify_permission_options(options)
6 lib/fog/aws/requests/compute/run_instances.rb
@@ -30,7 +30,8 @@ class Real
30 30 # * 'Ebs.DeleteOnTermination'<~String> - specifies whether or not to delete the volume on instance termination
31 31 # * 'ClientToken'<~String> - unique case-sensitive token for ensuring idempotency
32 32 # * 'DisableApiTermination'<~Boolean> - specifies whether or not to allow termination of the instance from the api
33   - # * 'SecurityGroup'<~Array> or <~String> - Name of security group(s) for instances (you must omit this parameter if using Virtual Private Clouds)
  33 + # * 'SecurityGroup'<~Array> or <~String> - Name of security group(s) for instances (not supported for VPC)
  34 + # * 'SecurityGroupId'<~Array> or <~String> - id's of security group(s) for instances, use this or SecurityGroup
34 35 # * 'InstanceInitiatedShutdownBehaviour'<~String> - specifies whether volumes are stopped or terminated when instance is shutdown, in [stop, terminate]
35 36 # * 'InstanceType'<~String> - Type of instance to boot. Valid options
36 37 # in ['t1.micro', 'm1.small', 'm1.large', 'm1.xlarge', 'c1.medium', 'c1.xlarge', 'm2.xlarge', m2.2xlarge', 'm2.4xlarge', 'cc1.4xlarge', 'cg1.4xlarge']
@@ -97,6 +98,9 @@ def run_instances(image_id, min_count, max_count, options = {})
97 98 if security_groups = options.delete('SecurityGroup')
98 99 options.merge!(Fog::AWS.indexed_param('SecurityGroup', [*security_groups]))
99 100 end
  101 + if security_group_ids = options.delete('SecurityGroupId')
  102 + options.merge!(Fog::AWS.indexed_param('SecurityGroupId', [*security_group_ids]))
  103 + end
100 104 if options['UserData']
101 105 options['UserData'] = Base64.encode64(options['UserData'])
102 106 end
68 tests/aws/requests/compute/security_group_tests.rb
@@ -31,6 +31,7 @@
31 31 Fog::Compute[:aws].create_security_group('fog_security_group_two', 'tests group').body
32 32 end
33 33
  34 + group_id = Fog::Compute[:aws].describe_security_groups('group-name' => 'fog_security_group').body['securityGroupInfo'].first['groupId']
34 35 to_be_revoked = []
35 36 expected_permissions = []
36 37
@@ -63,6 +64,10 @@
63 64 array_differences(expected_permissions, Fog::Compute[:aws].describe_security_groups('group-name' => 'fog_security_group').body['securityGroupInfo'].first['ipPermissions'])
64 65 end
65 66
  67 + tests("#describe_security_groups('group-id' => '#{group_id}')").returns([]) do
  68 + array_differences(expected_permissions, Fog::Compute[:aws].describe_security_groups('group-id' => group_id).body['securityGroupInfo'].first['ipPermissions'])
  69 + end
  70 +
66 71 permission = { 'SourceSecurityGroupName' => 'fog_security_group_two', 'SourceSecurityGroupOwnerId' => @owner_id }
67 72 tests("#authorize_security_group_ingress('fog_security_group', #{permission.inspect})").formats(AWS::Compute::Formats::BASIC) do
68 73 Fog::Compute[:aws].authorize_security_group_ingress('fog_security_group', permission).body
@@ -256,6 +261,52 @@
256 261 Fog::Compute[:aws].delete_security_group('fog_security_group_two').body
257 262 end
258 263
  264 + # Create security group in VPC
  265 + tests("#create_security_group('vpc_security_group', 'tests group')").formats(AWS::Compute::Formats::BASIC) do
  266 + Fog::Compute[:aws].create_security_group('vpc_security_group', 'tests group', 'vpc-11223344').body
  267 + end
  268 +
  269 + group_id = Fog::Compute[:aws].describe_security_groups('group-name' => 'vpc_security_group').body['securityGroupInfo'].first['groupId']
  270 +
  271 + # Access group with name in options array
  272 + permission = { 'IpProtocol' => 'tcp', 'FromPort' => '22', 'ToPort' => '22', 'CidrIp' => '10.0.0.0/8' }
  273 + expected_permissions = [
  274 + {"groups"=>[],
  275 + "ipRanges"=>[{"cidrIp"=>"10.0.0.0/8"}],
  276 + "ipProtocol"=>"tcp",
  277 + "fromPort"=>22,
  278 + "toPort"=>22}
  279 + ]
  280 +
  281 + options = permission.clone
  282 + options['GroupName'] = 'vpc_security_group'
  283 + tests("#authorize_security_group_ingress(#{options.inspect})").formats(AWS::Compute::Formats::BASIC) do
  284 + Fog::Compute[:aws].authorize_security_group_ingress(options).body
  285 + end
  286 +
  287 + tests("#describe_security_groups('group-name' => 'vpc_security_group')").returns([]) do
  288 + array_differences(expected_permissions, Fog::Compute[:aws].describe_security_groups('group-name' => 'vpc_security_group').body['securityGroupInfo'].first['ipPermissions'])
  289 + end
  290 +
  291 + tests("#revoke_security_group_ingress(#{options.inspect})").formats(AWS::Compute::Formats::BASIC) do
  292 + Fog::Compute[:aws].revoke_security_group_ingress(options).body
  293 + end
  294 +
  295 + # Access group with id in options array
  296 + options = permission.clone
  297 + options['GroupId'] = group_id
  298 + tests("#authorize_security_group_ingress(#{options.inspect})").formats(AWS::Compute::Formats::BASIC) do
  299 + Fog::Compute[:aws].authorize_security_group_ingress(options).body
  300 + end
  301 +
  302 + tests("#describe_security_groups('group-name' => 'vpc_security_group')").returns([]) do
  303 + array_differences(expected_permissions, Fog::Compute[:aws].describe_security_groups('group-name' => 'vpc_security_group').body['securityGroupInfo'].first['ipPermissions'])
  304 + end
  305 +
  306 + tests("#revoke_security_group_ingress(#{options.inspect})").formats(AWS::Compute::Formats::BASIC) do
  307 + Fog::Compute[:aws].revoke_security_group_ingress(options).body
  308 + end
  309 +
259 310 end
260 311 tests('failure') do
261 312
@@ -358,6 +409,23 @@
358 409 tests("#delete_security_group('default')").raises(Fog::Compute::AWS::Error) do
359 410 Fog::Compute[:aws].delete_security_group('default')
360 411 end
  412 +
  413 + broken_params = [
  414 + [ 'fog_security_group', { 'GroupName' => 'fog_security_group'}],
  415 + [ 'fog_security_group', { 'GroupId' => 'sg-11223344'}],
  416 + [ { 'GroupName' => 'fog_security_group', 'GroupId' => 'sg-11223344'}, nil]
  417 + ]
  418 +
  419 + broken_params.each do |list_elem|
  420 + tests("#authorize_security_group_ingress(#{list_elem[0].inspect}, #{list_elem[1].inspect})").raises(Fog::Compute::AWS::Error) do
  421 + Fog::Compute[:aws].authorize_security_group_ingress(list_elem[0], list_elem[1])
  422 + end
  423 +
  424 + tests("#revoke_security_group_ingress(#{list_elem[0].inspect}, #{list_elem[1].inspect})").raises(Fog::Compute::AWS::Error) do
  425 + Fog::Compute[:aws].revoke_security_group_ingress(list_elem[0], list_elem[1])
  426 + end
  427 + end
  428 +
361 429 end
362 430
363 431 end

0 comments on commit 4402d66

Please sign in to comment.
Something went wrong with that request. Please try again.