terraform-aws-modules / terraform-aws-security-group

Terraform module to create AWS Security Group resources πŸ‡ΊπŸ‡¦
https://registry.terraform.io/modules/terraform-aws-modules/security-group/aws
Other
562 stars 1.08k forks source link

A very low level question about using maps rather than sets to supply data to the module. #297

Closed rquadling closed 1 year ago

rquadling commented 1 year ago

Is your request related to a new offering from AWS?

Is this functionality available in the AWS provider for Terraform? See CHANGELOG.md, too.

Using a map rather than a list or set for resource iteration.

Is your request related to a problem? Please describe.

Often with the provider, some resources cannot be edited and so changing certain aspects of them means the resource is deleted and then created. This can be triggered by using sets and then iterating them. Adding a new entry to the end of the set, or removing the last entry on the set, all is well.

But if you want to remove the first entry, then, from Terraform's perspective, all the resources being created have been changed and if they can't be edited, they get deleted and recreated.

Describe the solution you'd like.

The solution I've used almost consistently is to use a map. If I am making a number of resources and all I've got is a set of labels (i.e. no additional properties are required), then I will use the set as the values for zipmap().

In specific relation to this module (which is why I'm looking), the ability to create multiple ingress_with_source_security_group_id

  ingress_with_source_security_group_id = [
    {
      rule                     = "http-80-tcp"
      source_security_group_id = data.terraform_remote_state.old_compute.outputs.shared_albs.private.security_group_id
    },
    {
      rule                     = "https-443-tcp" # Add a new rule for HTTPS on port 443
      source_security_group_id = data.terraform_remote_state.old_compute.outputs.shared_albs.private.security_group_id
    },
  ]

I end up with

module.service_sg.aws_security_group_rule.ingress_with_source_security_group_id[0]
module.service_sg.aws_security_group_rule.ingress_with_source_security_group_id[1]

So, our use case here is nothing compared to the capabilities of this module.

A better way would be to be able to supply a unique name via a map

  ingress_with_source_security_group_id = {
   private_http_80 = {
      rule                     = "http-80-tcp"
      source_security_group_id = data.terraform_remote_state.old_compute.outputs.shared_albs.private.security_group_id
    },
    private_https_443 = {
      rule                     = "https-443-tcp" # Add a new rule for HTTPS on port 443
      source_security_group_id = data.terraform_remote_state.old_compute.outputs.shared_albs.private.security_group_id
    },
  ]

And now we'd get

module.service_sg.aws_security_group_rule.ingress_with_source_security_group_id["private_http_80"]
module.service_sg.aws_security_group_rule.ingress_with_source_security_group_id["private_https_443"]

The resource key is purely OUR name, but is unique and essentially fixed.

If we want to rename it moved{} works. But we can delete and add to the ingress_with_source_security_group_id = {} map to edit, delete, re-order and if a name is present in the map , the resource doesn't get touched.

So. That's our approach.

Is this feasible for the terraform-aws-modules? Yes, potentially a massive amount of work, but does the issue of a terraform user suddenly deleting and recreating resources because of the order in a set really worth fixing?

Describe alternatives you've considered.

Additional context

rquadling commented 1 year ago

P.S. I'm happy to contribute if that is something that is to be considered. Define the approach for the migration, document it and then start the set -> map change.

github-actions[bot] commented 1 year ago

This issue has been automatically marked as stale because it has been open 30 days with no activity. Remove stale label or comment or this issue will be closed in 10 days

github-actions[bot] commented 1 year ago

This issue was automatically closed because of stale in 10 days

github-actions[bot] commented 12 months ago

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.