Guimove / terraform-aws-bastion

Terraform module which creates SSH bastion infrastructure on AWS
https://registry.terraform.io/modules/Guimove/bastion/aws
Apache License 2.0
205 stars 184 forks source link

updating main.tf and adding to locals.tf to resolve ipv6 error #132

Closed alsidneio closed 2 years ago

alsidneio commented 2 years ago

Description

Commit https://github.com/Guimove/terraform-aws-bastion/commit/df6830573f2832e9a50e419c8284c133fda19463 added the ipv6_cidr_blocks argument to the aws_security_group_rule ingress resource on line 39 of main.tf https://github.com/Guimove/terraform-aws-bastion/blob/e6332cdd7daf1b7fa004e248b58b07c7727660f7/main.tf#L39

This addition creates an error if the subnets do not have ipv6 addresses resulting in similar to the following error:

Error: "" is not a valid CIDR block: invalid CIDR address: 
│ 
│   with module.test_bastion.module.test_bastion.aws_security_group_rule.ingress_bastion[0],
│   on .terraform/modules/test_bastion.test_bastion/main.tf line 39, in resource "aws_security_group_rule" "ingress_bastion":
│   39:   ipv6_cidr_blocks = concat(data.aws_subnet.subnets.*.ipv6_cidr_block, var.ipv6_cidrs)

Changes Made

Guimove commented 2 years ago

Hello,

Thanks for your contribution, I've done that version just before seeing your PR : https://github.com/Guimove/terraform-aws-bastion/pull/133

Yours's better because it avoid any risk of null value, mine suppose that their is a default value for the var.ipv6_cidrs

Do you mind to do the same rework for ipv4 ? Just to get the same way to manage both type of cidr

Thanks

Guimove commented 2 years ago

LGTM !!!! 🚀