aws-ec2: surface rules for SecurityGroup.addIngressRule and SecurityGroup.addEgressRule
- Dominant language
- TypeScript
- Stars
- 12.9k
- Forks
- 4.6k
- Avg merge
- 2d 3h
- Merged PRs (30d)
- 83
Description
### Describe the feature
Since these rules may be created as a child of the given security group or as the child of another security group, and since they have generated IDs, it is not easy to get access to these rules. For example, if you wanted to change the `RemovalPolicy` of the `CfnSecurityGroupEgress` created through `addEgressRule` then you might try iterating through every child of the given security group and potentially every child of a peer security group (if `remoteRule` is true) to try to find an instance of `CfnSecurityGroupEgress`.
### Use Case
It would be nice to be able to interact with the created rules, such as adding dependencies, adding conditions, adding metadata, modifying removal policies, etc.. I know you could just create `CfnSecurityGroupIngress` or `CfnSecurityGroupEgress` without these methods, but that could mean repeating and maintaining a fair amount of boilerplate code. Also, surfacing these rules could be beneficial for extensibility of classes; e.g., creating a subclass of `SecurityGroup` that maybe overrides these methods, calls the super methods, and then adds some additional logic around the potentially created `CfnSecurityGroupIngress` and `CfnSecurityGroupEgress` rules.
### Proposed Solution
I think it would be preferable to just have these methods return `{ rule?: CfnSecurityGroupIngress }` and `{ rule?: CfnSecurityGroupEgress }`, respectively. However, I know that means returning an L1 construct from an L2 method. But the current behavior is not to return anything. So even if these return types are not ideal, I think they would be better than nothing.
Note that returning `T | undefined` works just as well. I just figured `{ rule?: T }` would be a little cleaner.
### Other Information
Alternative solutions:
- Creating an L2 return type
- Adding options to the methods for select use cases mentioned above (e.g. add options for `removalPolicy` and maybe `condition`, `dependencies`, etc.). This would not solve all use cases for this feature, but avoids changing the return type
### Acknowledgements
- [ ] I may be able to implement this feature request
- [ ] This feature might incur a breaking change
### CDK version used
2.41.0
### Environment details (OS name and version, etc.)
Windows 10 Enterprise Version 10.0.19044
Contributor guide
Research direction
Start by locating the SecurityGroup.addIngressRule and SecurityGroup.addEgressRule entry points and tracing how they create CfnSecurityGroupIngress and CfnSecurityGroupEgress, including the remoteRule path. Review the surrounding API conventions before deciding how the created rules should be exposed. Done means callers can access each created rule where applicable without changing existing rule behavior.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- aws, typescript
- Domain
- cloud, infrastructure, security
- Issue type
- Feature
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100