feat: Upgrade to aws vpc security group rule

[kirkchong]

Description

This PR changes the use of aws_security_group_rule to aws_vpc_security_ingress/egress_rule

Motivation and Context

As discussed in 327, switching to use aws_vpc_ingress/egress_rule will allow the usage of tags and is the recommended way going forward.

Breaking Changes

A full rename of variables, fields as its a change of resources, and to keep the naming pattern consistent.
Notably:

• variable name changes as per variables.tf
• field name changes

    {
      from_port   = 15
      to_port     = 25
      ip_protocol = 6 # changed from protocol # changed from protocol
      description = "Service name with vpc cidr"
      cidr_ipv4   = module.vpc.vpc_cidr_block  #changed from cidr_blocks
    },
    {
      from_port   = 300
      to_port     = 400
      ip_protocol = "tcp"
      description = "Service ports (ipv6)"
      cidr_ipv6   = "2001:db8::/64" # changed from ipv6_cidr_blocks
    },
      {
      from_port                    = 10
      to_port                      = 10
      ip_protocol                  = 6
      description                  = "Service name"
      referenced_security_group_id = data.aws_security_group.default.id #changed from source_security_group_id
    },

• comma separated values like in cidr_blocks are no longer possible

 ingress_with_cidr_ipv4 = [
    {
      rule      = "postgresql-tcp"
      cidr_ipv4 = "0.0.0.0/0"  # "0.0.0.0/0,1.1.1.1/32" is no longer possible
    },

How Has This Been Tested?

• I have updated at least one of the examples/* to demonstrate and validate my change(s)
• I have tested and validated these changes using one or more of the provided examples/* projects
• I have run pre commit -a

I tested the changes individually for each resources with multiple configurations (e.g. 2 prefix list * 3 ingress rules etc)
I do intend to squash my commits, please go through them individually as I find it easier to review.
My constraints while writing this: "aws_security_group" allows for multiple fields of multiple values, such as having prefix_list_ids and cidr_blocks="1.1.1.1/32,0.0.0.0/0", for aws_vpc_security_ingress, only one field and one value is supported.

[kirkchong]

@bryantbiggs Could I get your help in reviewing please?

[bryantbiggs]

unfortunately, this is a massively disruptive change and it doesn't align with the types of changes we would most likely make today if we were to make this drastic of a change. specifically, changing from an array/list based data structure to a map would appear to simplify a lot of the prior logic that was designed at a time when maps/for_each/etc. were not readily available (or very well supported) in the language

[kirkchong]

Hi @bryantbiggs,
Thanks for responding. Are there any plans to update to use the vpc rules?
I did have to work around the arrays to get this to work.
I'm happy to raise another MR using another data structure. To prevent redundant effort, if there are some ideas regarding how to get this feature written that'll be great. For example, there are some conveniences such as the allow all rules and cidrs, as well as having the map be defined by user or constructing a map in the locals before using for_each:

ingress_cidr_ipv4 = ["10.10.0.0/16"]
ingress_cidr_ipv6 = ["2001:db8::/64"]
ingress_rules = ["https-443-tcp"]

[bryantbiggs]

I haven't looked at the various implementations here in details to plot out what changes we would look to make, but in general the goal would be to update to the latest provider resources and methods for creating the infrastructure correctly (per AWS API and Terraform provider), simplify where we can, use stable data structures (maps instead of lists), etc.

But the code changes are the easy part of the equation - the upgrade path and migration plus dealing with users is the more time consuming and arduous side of the process that comes with a disruptive change like this

[kirkchong]

I'll see if I can suggest some things over the weekends, probably won't be able to take a look until then.
Noted on the migration path, I'll try to keep that in mind and I'm sorry that happened.
Thanks for responding and for your time once again.