Skip to content
New issue

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

#

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

Already on GitHub? # to your account

remove allowed_security_groups_count variable #39

Closed
wants to merge 4 commits into from
Closed

remove allowed_security_groups_count variable #39

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
bb9935b
remove allowed_security_groups_count variable
May 9, 2019
c595162
remove aws_security_group_rule.default_ingress to remove allowed_secu…
May 10, 2019
72d7645
added allowed_cidr_blocks, allowed_ipv6_cidr_blocks, and allow_self
May 10, 2019
46c62d5
added db_subnet_group to be used for externally created db subnet group
May 10, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ module "db" {

replica_count = 1
allowed_security_groups = ["sg-12345678"]
allowed_security_groups_count = 1
instance_type = "db.r4.large"
storage_encrypted = true
apply_immediately = true
Expand Down Expand Up @@ -65,7 +64,6 @@ Terraform documentation is generated automatically using [pre-commit hooks](http
| Name | Description | Type | Default | Required |
|------|-------------|:----:|:-----:|:-----:|
| allowed\_security\_groups | A list of Security Group ID's to allow access to. | list | `[]` | no |
| allowed\_security\_groups\_count | The number of Security Groups being added, terraform doesn't let us use length() in a count field | string | `"0"` | no |
| apply\_immediately | Determines whether or not any DB modifications are applied immediately, or during the maintenance window | string | `"false"` | no |
| auto\_minor\_version\_upgrade | Determines whether minor engine upgrades will be performed automatically in the maintenance window | string | `"true"` | no |
| backup\_retention\_period | How long to keep backups for (in days) | string | `"7"` | no |
Expand Down
16 changes: 6 additions & 10 deletions main.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,15 +139,11 @@ resource "aws_security_group" "this" {
vpc_id = "${var.vpc_id}"

tags = "${var.tags}"
}

resource "aws_security_group_rule" "default_ingress" {
count = "${var.allowed_security_groups_count}"

type = "ingress"
from_port = "${aws_rds_cluster.this.port}"
to_port = "${aws_rds_cluster.this.port}"
protocol = "tcp"
source_security_group_id = "${element(var.allowed_security_groups, count.index)}"
security_group_id = "${aws_security_group.this.id}"
ingress {
from_port = "${local.port}"
to_port = "${local.port}"
protocol = "tcp"
security_groups = "${var.allowed_security_groups}"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think is is a list: security_groups = ["${var.allowed_security_groups}"]

Copy link
Author

@vdeyro vdeyro May 10, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if we make it like that. it will become a list within a list since allowed_security_groups is already a list. won't it? already tested it earlier and it worked as i expected.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if we make it like that. it will become a list within a list

Nope. It's just how TF works. e.g. https://github.com/terraform-aws-modules/terraform-aws-vpc/blob/master/main.tf#L44

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh thanks. didn't know that. but it's still working the way i have designed it. should i change it for consistency?

}
}
5 changes: 0 additions & 5 deletions variables.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,6 @@ variable "allowed_security_groups" {
default = []
}

variable "allowed_security_groups_count" {
description = "The number of Security Groups being added, terraform doesn't let us use length() in a count field"
default = 0
}

variable "vpc_id" {
description = "VPC ID"
}
Expand Down