(CLOUD-269) Allow setting ingress rules for default security groups in VPC

examples/vpc-example/init.pp

@@ -18,6 +18,19 @@
   }]
 }
 
+ec2_securitygroup { 'sample-vpc::default':
+  ensure      => present,
+  region      => 'sa-east-1',
+  description => 'default VPC security group',
+  ingress     => [{
+    protocol => 'tcp',
+    port     => 22,
+    cidr     => '0.0.0.0/0'
+  },{
+    security_group => 'default',
+  }],
+}
+
 ec2_vpc_subnet { 'sample-subnet':
   ensure            => present,
   region            => 'sa-east-1',

lib/puppet/provider/ec2_securitygroup/v2.rb

@@ -86,10 +86,13 @@ def self.security_group_to_hash(region, group)
         vpc_name_tag ? vpc_name_tag.value : nil
       end
     end
+    name = group[:group_name]
+    name = "#{vpc_name}::#{name}" if vpc_name && name == 'default'
     {
-      id: group.group_id,
-      name: group.group_name,
-      description: group.description,
+      name: name,
+      group_name: group[:group_name],
+      id: group[:group_id],
+      description: group[:description],
       ensure: :present,
       ingress: format_ingress_rules(ec2, group),
       vpc: vpc_name,

lib/puppet/type/ec2_securitygroup.rb

@@ -6,14 +6,20 @@
 
   ensurable
 
-  newparam(:name, namevar: true) do
-    desc 'the name of the security group'
+  newparam(:name) do
+    desc 'the name of the security group resource'
+    isnamevar
     validate do |value|
       fail 'security groups must have a name' if value == ''
       fail 'name should be a String' unless value.is_a?(String)
     end
   end
 
+  newparam(:group_name) do
+    desc 'the name of the security group'
+    isnamevar
+  end
+
   newproperty(:region) do
     desc 'the region in which to launch the security group'
     validate do |value|
@@ -51,6 +57,7 @@ def insync?(is)
 
   newproperty(:vpc) do
     desc 'A VPC to which the group should be associated'
+    isnamevar
     validate do |value|
       fail 'vpc should be a String' unless value.is_a?(String)
     end
@@ -71,4 +78,25 @@ def should_autorequire?(rule)
   autorequire(:ec2_vpc) do
     self[:vpc]
   end
+
+  # When you create a VPC you automatically get a security group called default. You can't change the name.
+  # This lack of uniqueness makes managing these default security groups difficult. Enter a composite namevar.
+  # We support two name formats:
+  #
+  #   1. {some-security-group}
+  #   2. {some-vpc-name}::default
+  #
+  # Note that we only support prefixing a security group name with the vpc name for the default security group
+  # at this point. This avoids the issue of otherwise needing to store the resources in two places for non-default
+  # VPC security groups.
+  #
+  # In the case of a a default security group, we maintain the full name (including the VPC name) in the name property
+  # as otherwise it won't be unique and uniqueness and composite namevars are fun.
+  def self.title_patterns
+    [
+      [ /^(([\w\-]+)::(default))$/, [ [ :name, lambda {|x| x} ], [ :vpc, lambda {|x| x} ], [ :group_name, lambda {|x| x} ] ] ],
+      [ /^((.*))$/, [ [ :name, lambda {|x| x} ], [ :group_name, lambda {|x| x} ] ] ]
+    ]
+  end
+
 end

spec/acceptance/fixtures/vpc.pp.tmpl

@@ -11,6 +11,27 @@ ec2_vpc { '{{name}}-vpc':
   },
 }
 
+ec2_securitygroup { '{{name}}-vpc::default':
+  ensure      => {{ensure}},
+  region      => '{{region}}',
+  description => 'default VPC security group',
+  ingress     => [
+    {{#security_group_ingress}}
+      {
+        {{#values}}
+          {{k}} => '{{v}}',
+        {{/values}}
+      },
+    {{/security_group_ingress}}
+  ],
+  tags        => {
+  {{#tags}}
+    {{k}} => '{{v}}',
+  {{/tags}}
+  },
+  require     => Ec2_vpc['{{name}}-vpc'],
+}
+
 ec2_securitygroup { '{{name}}-sg':
   ensure      => {{ensure}},
   description => 'VPC accceptance test',

spec/acceptance/vpc_spec.rb

@@ -50,10 +50,12 @@ def find_instance(name)
     finder(name, 'get_instances')
   end
 
-  def find_security_group(name)
-    group_response = @aws.ec2_client.describe_security_groups(filters: [
+  def find_security_group(name, vpc_id=nil)
+    filters = [
       {name: 'group-name', values: [name]}
-    ])
+    ]
+    filters << {name: 'vpc-id', values: [vpc_id]} unless vpc_id.nil?
+    group_response = @aws.ec2_client.describe_security_groups(filters: filters)
     items = group_response.data.security_groups
     expect(items.count).to eq(1)
     items.first
@@ -118,6 +120,7 @@ def generate_ip
       @vgw = find_vpn_gateway("#{@name}-vgw")
       @cgw = find_customer_gateway("#{@name}-cgw")
       @sg = find_security_group("#{@name}-sg")
+      @dsg = find_security_group('default', @vpc.vpc_id)
     end
 
     after(:all) do
@@ -155,6 +158,12 @@ def generate_ip
       expect(@aws.tag_difference(@sg, @config[:tags])).to be_empty
     end
 
+    it 'should manage the default VPC security group' do
+      expect(@dsg).not_to be_nil
+      expect(@dsg.vpc_id).to eq(@vpc.vpc_id)
+      expect(@aws.tag_difference(@dsg, @config[:tags])).to be_empty
+    end
+
     it 'should create a route table' do
       table = find_route_table("#{@name}-routes")
       expect(table).not_to be_nil

spec/unit/type/ec2_securitygroup_spec.rb

@@ -67,4 +67,10 @@
     expect(type_class).to order_tags_on_output
   end
 
+  it 'should extract the vpc from a composite name' do
+    sg = type_class.new({name: 'sample::default'})
+    expect(sg[:vpc]).to eq('sample')
+    expect(sg[:group_name]).to eq('default')
+  end
+
 end