Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prevent Service Interruption during rule updates #22

Closed
aidanmelen opened this issue Oct 8, 2022 · 0 comments · Fixed by #28
Closed

Prevent Service Interruption during rule updates #22

aidanmelen opened this issue Oct 8, 2022 · 0 comments · Fixed by #28
Labels
enhancement New feature or request

Comments

@aidanmelen
Copy link
Owner

aidanmelen commented Oct 8, 2022

Is your feature request related to a problem? Please describe.

A single aws_security_group_rule resource can result in one or many security group rules being created by the EC2 API. This can be desirable to minimize the footprint of the rules in the Terraform code, output and state. However, packing many rules into a single aws_security_group_rule resource can result in the unwanted side-effect of a single rule update resulting in replacement of the entire rule resource. I experimented with user managed keys so that the rules could take advantage of Create Before Destroy lifecycle management, but this was problematic for most scenarios as the EC2 API would attempt to create duplicate rules when lists where involved and fail.

Describe the solution you'd like

Unpack aws_security_group_rule resource arguments supplied by the user such that each aws_security_group_rule resource is guaranteed to result in the EC2 API creating exactly one rule.

Describe alternatives you've considered

I tried to implement a use managed key so that the for_each could update using create before destroy. This does work for some rules but for most scenarios this will fail with duplicate rule errors.

Additional context

Pros

  • Terraform code can maintain the tight packing of rules for readability while on the backend terraform will manage singular rule resources void of service interruptions caused by the packing side-effects.

  • The aws_security_group_rule resource source/destination argument limitations are difficult to understand. For example cidr_blocks, ipv6_cidr_blocks, and prefix_list_ids can be declared in the same resource; however, cidr_blocks and ipv6_cidr_blocks cannot be declared with source_security_group and self (and vice versa), but at the same time, prefix_list_ids can. ETC. This results in a lot of annoying plan time errors and code refactoring. With unpack, users can group any combination of aws_security_group_rule resource arguments because, in the end, all arguments will be unpacked into singular rule resources void of all argument limitations.

  • rule changes at plan time are singular which makes the addition and deletion of new rules clear and easy to comprehend.

Cons

  • Computed rules do not support unpack logic, but that should not be a surprise. Even if computed rules were somehow hacked together to make unpack logic work... count churn will eventually cause service interruptions and thus implementing unpack is moot. If you want to prevent service interruption, then use for_each with unpack enabled with ingress, egress, matrix_ingress, and matrix_egress module arguments.
  • rule changes at plan time are verbose as each rule gets a dedicated rule resource.
  • unpack logic adds complexity to the core module code. However, the null-unpack-aws-security-group-rules sub-module keeps the unpack logic somewhat DRY across ingress/egress and matrix_ingress/matrix_egress.
@aidanmelen aidanmelen added the enhancement New feature or request label Oct 8, 2022
@aidanmelen aidanmelen linked a pull request Oct 8, 2022 that will close this issue
@aidanmelen aidanmelen changed the title Prevent Service Interruption when updates occur Prevent Service Interruption on rule updates Oct 8, 2022
@aidanmelen aidanmelen changed the title Prevent Service Interruption on rule updates Prevent Service Interruption during rule updates Oct 8, 2022
This was referenced Oct 8, 2022
@aidanmelen aidanmelen linked a pull request Oct 10, 2022 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
1 participant