aws-cloudformation / aws-cloudformation/cloudformation-coverage-roadmap

AWS::EC2::SecurityGroupIngress causing false positive drift within referenced AWS::EC2::SecurityGroup

Open
#1,198 6 comments 5 reactions 0 assignees View on GitHub
enhancement
Dominant language
No language data
Stars
1.1k
Forks
62
PR merge metrics
No merged PRs in 30d

Description

### Name of the resource

AWS::EC2::SecurityGroupIngress

### Resource Name

AWS::EC2::SecurityGroup

### Issue Description

It's possible to add a rule within a `AWS::EC2::SecurityGroup` using SecurityGroupIngress rule directly as part of the resource OR defining a dedicated resource called `AWS::EC2::SecurityGroupIngress`. Within this `AWS::EC2::SecurityGroupIngress` need to specify the SecurityGroupId, where is rule should be added. This 2nd common pattern causing a causing false positive drift within referenced `AWS::EC2::SecurityGroup`.

### Expected Behavior

CloudFormation drift detection must be able to “understand” rules added via AWS::EC2::SecurityGroupIngress to an existing AWS::EC2::SecurityGroup.

### Observed Behavior

Rules within SecurityGroup which had been added via AWS::EC2::SecurityGroupIngress are reported as drift. CloudFormation drift detection doesn't "resolve" rules from AWS::EC2::SecurityGroupIngress resource. Users of CloudFormation drift detection assume this rules are added outside of CloudFormation. Once checking the rules via AWS Console the rules reported in drift are in place. Details please see via "Test Cases"

### Test Cases

**(1)**
Add a Stack A which containing a AWS::EC2::SecurityGroup and some rules:
```
---
AWSTemplateFormatVersion: '2010-09-09'
Description: Stack A (hosting the SecGroup with sample rules 1 and 2) - case 10148943561

Resources:
ServerSecGroup:
Type: AWS::EC2::SecurityGroup
Properties:
GroupDescription: SecGroup in Stack A e.g. for an NLB
GroupName: 'rogo-testA'
SecurityGroupIngress:
- IpProtocol: tcp
FromPort: 22
ToPort: 22
CidrIp: 10.10.10.100/32
Description: Rule 1 defined in SecurityGroup directly
- IpProtocol: tcp
FromPort: 6777
ToPort: 6777
CidrIp: 10.10.10.101/32
Description: Rule 2 defined in SecurityGroup directly
Tags:
- Key: ApplicationID
Value: sample
VpcId:
Fn::ImportValue: VPC1-VPC-ID
```
After creation you can see, that Stack is state IN_SYNC:
![stackA-beforeRefViaSecurityGroupIngress_GIT](https://user-images.githubusercontent.com/16392819/172047636-4df0eae5-1c93-4135-ad5b-8da9d09d8e00.jpg)

**(2)**
Catch the GroupId of AWS::EC2::SecurityGroup from Stack A and add a Stack B, which contains AWS::EC2::SecurityGroupIngress with reference to this SecurityGroup GroupId:
```
---
AWSTemplateFormatVersion: '2010-09-09'
Description: Stack B (AWS::EC2::SecurityGroupIngress causing drift false-positive within referenced SecGroup) - case 10148943561

Resources:
ServerSecGroupIngress1:
Type: AWS::EC2::SecurityGroupIngress
Properties:
GroupId: sg-0f7274xR0beRtx
IpProtocol: tcp
FromPort: 6777
ToPort: 6777
CidrIp: 10.20.20.202/32
Description: Rule 3 added by SecurityGroupIngress

ServerSecGroupIngress2:
Type: AWS::EC2::SecurityGroupIngress
Properties:
GroupId: sg-0f7274xR0beRtx
IpProtocol: tcp
FromPort: 22
ToPort: 22
CidrIp: 10.20.20.200/32
Description: Rule 4 added by SecurityGroupIngress
```

After the stack is created as well you can see the correct/expected result in the SecurityGroup:
![SecGroupRes_GIT](https://user-images.githubusercontent.com/16392819/172047776-08b64ed1-7eec-491d-9c98-f53d0cb01018.jpg)

**(3)**
Run the CloudFormation drift detection again on Stack A and see the bug:
![stackA-DRIFTafterSecurityGroupIngress_GIT](https://user-images.githubusercontent.com/16392819/172047791-a9ac2c8a-771d-4d8b-b663-5347fa15e275.jpg)
Both rules (here called Rule 3 + Rule 4 coming from AWS::EC2::SecurityGroupIngress) are *reported as drift*, even this rules are part of the SecurityGroup! The stack is in state DRIFTED.

### Other Details

Managing SecurityGroup rules is an important and fundamental part of IaC. Having this said, CloudFormation drift detection is the essential part for monitoring your existing setup, especially SecurityGroup rules.

Contributor guide

Open the contributing guide

Research direction

No repository files or tests are identified in the issue. Start by reproducing the Stack A and Stack B CloudFormation templates, then run drift detection on Stack A and compare the reported rules with the actual security group. Done means rules created through AWS::EC2::SecurityGroupIngress are not reported as drift in the referenced AWS::EC2::SecurityGroup.

Written by the indexing model from the issue text.

Assessment

Tech stack
aws
Domain
cloud, infrastructure
Issue type
Bug
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.