aws-cloudformation / cloudformation-coverage-roadmap

The AWS CloudFormation Public Coverage Roadmap
https://aws.amazon.com/cloudformation/
Creative Commons Attribution Share Alike 4.0 International
1.1k stars 53 forks source link

AWS::EC2::SecurityGroup - Handle duplicates when defining SecurityGroupIngress as property of AWS::EC2::SecurityGroup #612

Open brachna9 opened 3 years ago

brachna9 commented 3 years ago

1. Title

Handle duplicates when defining SecurityGroupIngress as property of AWS::EC2::SecurityGroup

2. Scope of request

AWS::EC2::SecurityGroupIngress handles duplicates, however, when creating EC2 security group ingresses as a property of the security group resource (AWS::EC2::SecurityGroup) duplicates are not handled in the same way.

3. Expected behavior

If a duplicate entry is made in the SecurityGroupIngress property of "AWS::EC2::SecurityGroup" resource, CloudFormation should not show it as a drift. Also, the ingress rule should not be removed if only duplicate entry is removed from the ingress rules list.

Sample snippet to replicate -

AWSTemplateFormatVersion: 2010-09-09
Resources:
  TestSg:
    Type: 'AWS::EC2::SecurityGroup'
    Properties:
      GroupDescription: Test duplicate ingress
      VpcId: <Replace with vpc ID>
      SecurityGroupIngress:
        - IpProtocol: tcp
          FromPort: 8080
          ToPort: 8080
          CidrIp: 172.24.186.0/23
          Description: EntryA 
        - IpProtocol: tcp
          FromPort: 8080
          ToPort: 8080
          CidrIp: 172.24.186.0/23
          Description: EntryB (Duplicate)

Drift Result when this stack is created -

Screenshot 2020-08-21 at 10 02 21 AM

Drift Result when Duplicate ingress (EntryB) is removed in update -

Screenshot 2020-08-21 at 10 17 58 AM

Category - Compute

Benjamin-L commented 2 years ago

I'm running into this same issue. It's also worth noting that declaring the ingress rules inline as part of the AWS::EC2::SecurityGroup resource does disallow duplicates, if the duplicate rules are declared when the security group is first created. If the duplicate rule is added as part of a later stack update, it appears to accept it silently, and then further updates to the duplicate rules result in the single rule being deleted.

AndreasHuber-CH commented 2 years ago

I can confirm that the same issue occurs if you have an ingress rule present in a AWS::EC2::SecurityGroup and an duplicate entry created through a AWS::EC2::SecurityGroupIngress defined elsewhere. The 2nd rule is silently ignored and you don't even get a warning at stack creation time. But when you try cleaning up the seemingly useless AWS::EC2::SecurityGroupIngress entry in stack update, the ingress rule is actually removed from the security group, exactly as described above.

ziarrdan commented 1 year ago

This is a known behavior caused by duplicate definitions of Ingress/Egress rules in the template definition of AWS::EC2::SecurityGroup. To prevent this from occurring, CloudFormation recommends removing duplicate entries which would cause this unwanted behavior. We will update the documentation with this guidance by 10/28.

Steps to remove duplicate ingress/egress rules.

  1. Record the resource identifier of the SecurityGroup resource.
  2. Record the unique SecurityGroup Rules associated with the resource. You can obtain this through the drift detection output (actual properties). This output contains unique rules only.
  3. Update stack to add DeletionPolicy=Retain to the SecurityGroup resource in the template. https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-attribute-deletionpolicy.html
  4. Update stack to detached the SecurityGroup resource, the resource will be removed from the stack but the live resource will be retained.
  5. Follow https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/resource-import-existing-stack.html to import the SecurityGroup resource back into the stack's template. Use the resource identifier found in step 1 and make sure the SecurityGroup resource definition in the template contains only the unique rules found in step 2.