Error with EFS security group if using existing VPC

[bodgit]

Description

If using an existing VPC rather than let this module create it, it now generates this error:

│ Error: "" is not a valid CIDR block: invalid CIDR address: 
│ 
│   with module.atlantis.module.efs_sg[0].module.sg.aws_security_group_rule.ingress_rules[0],
│   on .terraform/modules/atlantis.efs_sg/main.tf line 70, in resource "aws_security_group_rule" "ingress_rules":
│   70:   cidr_blocks      = var.ingress_cidr_blocks
│ 
╵

This is due to this line:

terraform-aws-atlantis/main.tf

Line 382 in 2df6bd6

ingress_cidr_blocks = [var.cidr]

var.cidr doesn't need to be set if you have the VPC already created and are passing it in with var.vpc_id.

Versions

• Terraform: 1.1.3

• Provider(s): aws 3.74.2

• Module: 3.12.0

Reproduction

Steps to reproduce the behavior:

Code Snippet to Reproduce

Use this module with the vpc_id parameter to use an existing VPC.

Expected behavior

It shouldn't be erroring, possibly should be using a datasource to get the CIDR block associated with the VPC referenced by var.vpc_id.

Actual behavior

See above.

[aairey]

terraform-aws-atlantis/variables.tf

Lines 62 to 66 in 2df6bd6

	variable "cidr" {
	description = "The CIDR block for the VPC which will be created if `vpc_id` is not specified"
	type = string
	default = ""
	}

The description is not correct, when you pass a vpc_id you still need to set var.cidr as var.enable_ephemeral_storage defaults to false.

[dgokcin]

@aairey so I should give the cidr block of my existing vpc?

[aairey]

Yes, but things might've changed now in the latest release. Haven't tried it yet.

[llamahunter]

Yes, but things might've changed now in the latest release. Haven't tried it yet.

It still does not work. There should be no need to specify the VPC CIDR block if you are specifying the vpc_id. The terraform should just look it up if it needs it.

[asantos-fuze]

@bodgit I've updated version v3.15.0 with changes you've made on this commit 54bd2d9 and was able to solve the cidr sg issue. Thanks

[bryancusatis]

We are still having this same issue.

[asantos-fuze]

@bryancusatis Apply the changes on commit 54bd2d9 made by @bodgit . This should solve the issue

[chroju]

I have the same issue.

Will commit bodgit@54bd2d9 not be merged into this repository ......?　 Shall I create a PR with the same one?

[github-actions]

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.