Add create_before_destroy for parameter group. Make subnet group optional (#110)

aknysh · cloudpossebot · web-flow · commit e215164e6efb · 2021-03-16T09:01:54.000-04:00
* Update parameter and option groups. Update tests

* Update parameter and option groups. Update tests

* Update parameter and option groups. Update tests

* Auto Format

* Update parameter and option groups. Update tests

Co-authored-by: cloudpossebot &lt;11232728+cloudpossebot@users.noreply.github.com&gt;
diff --git a/.github/mergify.yml b/.github/mergify.yml
@@ -56,3 +56,10 @@ pull_request_rules:
       changes_requested: true
       approved: true
       message: "This Pull Request has been updated, so we're dismissing all reviews."
+
+- name: "close Pull Requests without files changed"
+  conditions:
+    - "#files=0"
+  actions:
+    close:
+      message: "This pull request has been automatically closed by Mergify because there are no longer any changes."
diff --git a/.github/workflows/auto-format.yml b/.github/workflows/auto-format.yml
@@ -6,7 +6,7 @@ on:
 jobs:
   auto-format:
     runs-on: ubuntu-latest
-    container: cloudposse/build-harness:slim-latest
+    container: cloudposse/build-harness:latest
     steps:
     # Checkout the pull request branch
     #  "An action in a workflow run can’t trigger a new workflow run. For example, if an action pushes code using
@@ -29,6 +29,8 @@ jobs:
     - name: Auto Format
       if: github.event.pull_request.state == 'open'
       shell: bash
+      env:
+        GITHUB_TOKEN: "${{ secrets.PUBLIC_REPO_ACCESS_TOKEN }}"
       run: make BUILD_HARNESS_PATH=/build-harness PACKAGES_PREFER_HOST=true -f /build-harness/templates/Makefile.build-harness pr/auto-format/host
 
     # Commit changes (if any) to the PR branch
diff --git a/.github/workflows/auto-release.yml b/.github/workflows/auto-release.yml
@@ -3,17 +3,23 @@ name: auto-release
 on:
   push:
     branches:
-    - master
+      - master
 
 jobs:
   publish:
     runs-on: ubuntu-latest
     steps:
-    # Drafts your next Release notes as Pull Requests are merged into "master"
-    - uses: release-drafter/release-drafter@v5
-      with:
-        publish: true
-        prerelease: false
-        config-name: auto-release.yml
-      env:
-        GITHUB_TOKEN: ${{ secrets.PUBLIC_REPO_ACCESS_TOKEN }}
+      # Get PR from merged commit to master
+      - uses: actions-ecosystem/action-get-merged-pull-request@v1
+        id: get-merged-pull-request
+        with:
+          github_token: ${{ secrets.PUBLIC_REPO_ACCESS_TOKEN }}
+      # Drafts your next Release notes as Pull Requests are merged into "master"
+      - uses: release-drafter/release-drafter@v5
+        if: "!contains(steps.get-merged-pull-request.outputs.labels, 'no-release')"
+        with:
+          publish: true
+          prerelease: false
+          config-name: auto-release.yml
+        env:
+          GITHUB_TOKEN: ${{ secrets.PUBLIC_REPO_ACCESS_TOKEN }}
diff --git a/LICENSE b/LICENSE
@@ -186,7 +186,7 @@
       same "printed page" as the copyright notice for easier
       identification within third-party archives.
 
-   Copyright 2017-2019 Cloud Posse, LLC
+   Copyright 2017-2021 Cloud Posse, LLC
 
    Licensed under the Apache License, Version 2.0 (the "License");
    you may not use this file except in compliance with the License.
diff --git a/README.md b/README.md
@@ -224,6 +224,7 @@ Available targets:
 | associate\_security\_group\_ids | The IDs of the existing security groups to associate with the DB instance | `list(string)` | `[]` | no |
 | attributes | Additional attributes (e.g. `1`) | `list(string)` | `[]` | no |
 | auto\_minor\_version\_upgrade | Allow automated minor version upgrade (e.g. from Postgres 9.5.3 to Postgres 9.5.4) | `bool` | `true` | no |
+| availability\_zone | The AZ for the RDS instance. Specify one of `subnet_ids`, `db_subnet_group_name` or `availability_zone`. If `availability_zone` is provided, the instance will be placed into the default VPC or EC2 Classic | `string` | `null` | no |
 | backup\_retention\_period | Backup retention period in days. Must be > 0 to enable backups | `number` | `0` | no |
 | backup\_window | When AWS can perform DB snapshots, can't overlap with maintenance window | `string` | `"22:00-03:00"` | no |
 | ca\_cert\_identifier | The identifier of the CA certificate for the DB instance | `string` | `"rds-ca-2019"` | no |
@@ -236,6 +237,7 @@ Available targets:
 | db\_options | A list of DB options to apply with an option group. Depends on DB engine | <pre>list(object({<br>    db_security_group_memberships  = list(string)<br>    option_name                    = string<br>    port                           = number<br>    version                        = string<br>    vpc_security_group_memberships = list(string)<br><br>    option_settings = list(object({<br>      name  = string<br>      value = string<br>    }))<br>  }))</pre> | `[]` | no |
 | db\_parameter | A list of DB parameters to apply. Note that parameters may differ from a DB family to another | <pre>list(object({<br>    apply_method = string<br>    name         = string<br>    value        = string<br>  }))</pre> | `[]` | no |
 | db\_parameter\_group | The DB parameter group family name. The value depends on DB engine used. See [DBParameterGroupFamily](https://docs.aws.amazon.com/AmazonRDS/latest/APIReference/API_CreateDBParameterGroup.html#API_CreateDBParameterGroup_RequestParameters) for instructions on how to retrieve applicable value. | `string` | n/a | yes |
+| db\_subnet\_group\_name | Name of DB subnet group. DB instance will be created in the VPC associated with the DB subnet group. Specify one of `subnet_ids`, `db_subnet_group_name` or `availability_zone` | `string` | `null` | no |
 | deletion\_protection | Set to true to enable deletion protection on the RDS instance | `bool` | `false` | no |
 | delimiter | Delimiter to be used between `namespace`, `environment`, `stage`, `name` and `attributes`.<br>Defaults to `-` (hyphen). Set to `""` to use no delimiter at all. | `string` | `null` | no |
 | dns\_zone\_id | The ID of the DNS Zone in Route53 where a new DNS record will be created for the DB host name | `string` | `""` | no |
@@ -276,7 +278,7 @@ Available targets:
 | stage | Stage, e.g. 'prod', 'staging', 'dev', OR 'source', 'build', 'test', 'deploy', 'release' | `string` | `null` | no |
 | storage\_encrypted | (Optional) Specifies whether the DB instance is encrypted. The default is false if not specified | `bool` | `true` | no |
 | storage\_type | One of 'standard' (magnetic), 'gp2' (general purpose SSD), or 'io1' (provisioned IOPS SSD) | `string` | `"standard"` | no |
-| subnet\_ids | List of subnets for the DB | `list(string)` | n/a | yes |
+| subnet\_ids | List of subnet IDs for the DB. DB instance will be created in the VPC associated with the DB subnet group provisioned using the subnet IDs. Specify one of `subnet_ids`, `db_subnet_group_name` or `availability_zone` | `list(string)` | `[]` | no |
 | tags | Additional tags (e.g. `map('BusinessUnit','XYZ')` | `map(string)` | `{}` | no |
 | vpc\_id | VPC ID the DB instance will be created in | `string` | n/a | yes |
 
@@ -292,7 +294,7 @@ Available targets:
 | option\_group\_id | ID of the Option Group |
 | parameter\_group\_id | ID of the Parameter Group |
 | security\_group\_id | ID of the Security Group |
-| subnet\_group\_id | ID of the Subnet Group |
+| subnet\_group\_id | ID of the created Subnet Group |
 <!-- markdownlint-restore -->
 
 
diff --git a/docs/terraform.md b/docs/terraform.md
@@ -45,6 +45,7 @@
 | associate\_security\_group\_ids | The IDs of the existing security groups to associate with the DB instance | `list(string)` | `[]` | no |
 | attributes | Additional attributes (e.g. `1`) | `list(string)` | `[]` | no |
 | auto\_minor\_version\_upgrade | Allow automated minor version upgrade (e.g. from Postgres 9.5.3 to Postgres 9.5.4) | `bool` | `true` | no |
+| availability\_zone | The AZ for the RDS instance. Specify one of `subnet_ids`, `db_subnet_group_name` or `availability_zone`. If `availability_zone` is provided, the instance will be placed into the default VPC or EC2 Classic | `string` | `null` | no |
 | backup\_retention\_period | Backup retention period in days. Must be > 0 to enable backups | `number` | `0` | no |
 | backup\_window | When AWS can perform DB snapshots, can't overlap with maintenance window | `string` | `"22:00-03:00"` | no |
 | ca\_cert\_identifier | The identifier of the CA certificate for the DB instance | `string` | `"rds-ca-2019"` | no |
@@ -57,6 +58,7 @@
 | db\_options | A list of DB options to apply with an option group. Depends on DB engine | <pre>list(object({<br>    db_security_group_memberships  = list(string)<br>    option_name                    = string<br>    port                           = number<br>    version                        = string<br>    vpc_security_group_memberships = list(string)<br><br>    option_settings = list(object({<br>      name  = string<br>      value = string<br>    }))<br>  }))</pre> | `[]` | no |
 | db\_parameter | A list of DB parameters to apply. Note that parameters may differ from a DB family to another | <pre>list(object({<br>    apply_method = string<br>    name         = string<br>    value        = string<br>  }))</pre> | `[]` | no |
 | db\_parameter\_group | The DB parameter group family name. The value depends on DB engine used. See [DBParameterGroupFamily](https://docs.aws.amazon.com/AmazonRDS/latest/APIReference/API_CreateDBParameterGroup.html#API_CreateDBParameterGroup_RequestParameters) for instructions on how to retrieve applicable value. | `string` | n/a | yes |
+| db\_subnet\_group\_name | Name of DB subnet group. DB instance will be created in the VPC associated with the DB subnet group. Specify one of `subnet_ids`, `db_subnet_group_name` or `availability_zone` | `string` | `null` | no |
 | deletion\_protection | Set to true to enable deletion protection on the RDS instance | `bool` | `false` | no |
 | delimiter | Delimiter to be used between `namespace`, `environment`, `stage`, `name` and `attributes`.<br>Defaults to `-` (hyphen). Set to `""` to use no delimiter at all. | `string` | `null` | no |
 | dns\_zone\_id | The ID of the DNS Zone in Route53 where a new DNS record will be created for the DB host name | `string` | `""` | no |
@@ -97,7 +99,7 @@
 | stage | Stage, e.g. 'prod', 'staging', 'dev', OR 'source', 'build', 'test', 'deploy', 'release' | `string` | `null` | no |
 | storage\_encrypted | (Optional) Specifies whether the DB instance is encrypted. The default is false if not specified | `bool` | `true` | no |
 | storage\_type | One of 'standard' (magnetic), 'gp2' (general purpose SSD), or 'io1' (provisioned IOPS SSD) | `string` | `"standard"` | no |
-| subnet\_ids | List of subnets for the DB | `list(string)` | n/a | yes |
+| subnet\_ids | List of subnet IDs for the DB. DB instance will be created in the VPC associated with the DB subnet group provisioned using the subnet IDs. Specify one of `subnet_ids`, `db_subnet_group_name` or `availability_zone` | `list(string)` | `[]` | no |
 | tags | Additional tags (e.g. `map('BusinessUnit','XYZ')` | `map(string)` | `{}` | no |
 | vpc\_id | VPC ID the DB instance will be created in | `string` | n/a | yes |
 
@@ -113,5 +115,5 @@
 | option\_group\_id | ID of the Option Group |
 | parameter\_group\_id | ID of the Parameter Group |
 | security\_group\_id | ID of the Security Group |
-| subnet\_group\_id | ID of the Subnet Group |
+| subnet\_group\_id | ID of the created Subnet Group |
 <!-- markdownlint-restore -->
diff --git a/examples/complete/main.tf b/examples/complete/main.tf
@@ -3,44 +3,49 @@ provider "aws" {
 }
 
 module "vpc" {
-  source     = "cloudposse/vpc/aws"
-  version    = "0.18.2"
-  context    = module.this.context
+  source  = "cloudposse/vpc/aws"
+  version = "0.21.1"
+
   cidr_block = "172.16.0.0/16"
+
+  context = module.this.context
 }
 
 module "subnets" {
-  source               = "cloudposse/dynamic-subnets/aws"
-  version              = "0.34.0"
-  context              = module.this.context
+  source  = "cloudposse/dynamic-subnets/aws"
+  version = "0.38.0"
+
   availability_zones   = var.availability_zones
   vpc_id               = module.vpc.vpc_id
   igw_id               = module.vpc.igw_id
   cidr_block           = module.vpc.vpc_cidr_block
   nat_gateway_enabled  = false
   nat_instance_enabled = false
+
+  context = module.this.context
 }
 
 module "rds_instance" {
-  source              = "../../"
-  context             = module.this.context
-  database_name       = var.database_name
-  database_user       = var.database_user
-  database_password   = var.database_password
-  database_port       = var.database_port
-  multi_az            = var.multi_az
-  storage_type        = var.storage_type
-  allocated_storage   = var.allocated_storage
-  storage_encrypted   = var.storage_encrypted
-  engine              = var.engine
-  engine_version      = var.engine_version
-  instance_class      = var.instance_class
-  db_parameter_group  = var.db_parameter_group
-  publicly_accessible = var.publicly_accessible
-  vpc_id              = module.vpc.vpc_id
-  subnet_ids          = module.subnets.private_subnet_ids
-  security_group_ids  = [module.vpc.vpc_default_security_group_id]
-  apply_immediately   = var.apply_immediately
+  source               = "../../"
+  database_name        = var.database_name
+  database_user        = var.database_user
+  database_password    = var.database_password
+  database_port        = var.database_port
+  multi_az             = var.multi_az
+  storage_type         = var.storage_type
+  allocated_storage    = var.allocated_storage
+  storage_encrypted    = var.storage_encrypted
+  engine               = var.engine
+  engine_version       = var.engine_version
+  instance_class       = var.instance_class
+  db_parameter_group   = var.db_parameter_group
+  publicly_accessible  = var.publicly_accessible
+  vpc_id               = module.vpc.vpc_id
+  subnet_ids           = module.subnets.private_subnet_ids
+  security_group_ids   = [module.vpc.vpc_default_security_group_id]
+  apply_immediately    = var.apply_immediately
+  availability_zone    = var.availability_zone
+  db_subnet_group_name = var.db_subnet_group_name
 
   db_parameter = [
     {
@@ -54,4 +59,6 @@ module "rds_instance" {
       apply_method = "immediate"
     }
   ]
+
+  context = module.this.context
 }
diff --git a/examples/complete/outputs.tf b/examples/complete/outputs.tf
@@ -15,7 +15,7 @@ output "instance_endpoint" {
 
 output "subnet_group_id" {
   value       = module.rds_instance.subnet_group_id
-  description = "ID of the Subnet Group"
+  description = "ID of the created Subnet Group"
 }
 
 output "security_group_id" {
diff --git a/examples/complete/variables.tf b/examples/complete/variables.tf
@@ -37,6 +37,18 @@ variable "multi_az" {
   description = "Set to true if multi AZ deployment must be supported"
 }
 
+variable "availability_zone" {
+  type        = string
+  default     = null
+  description = "The AZ for the RDS instance. Specify one of `subnet_ids`, `db_subnet_group_name` or `availability_zone`. If `availability_zone` is provided, the instance will be placed into the default VPC or EC2 Classic"
+}
+
+variable "db_subnet_group_name" {
+  type        = string
+  default     = null
+  description = "Name of DB subnet group. DB instance will be created in the VPC associated with the DB subnet group. Specify one of `subnet_ids`, `db_subnet_group_name` or `availability_zone`"
+}
+
 variable "storage_type" {
   type        = string
   description = "One of 'standard' (magnetic), 'gp2' (general purpose SSD), or 'io1' (provisioned IOPS SSD)"
diff --git a/main.tf b/main.tf
@@ -8,10 +8,20 @@ module "final_snapshot_label" {
 locals {
   computed_major_engine_version = var.engine == "postgres" ? join(".", slice(split(".", var.engine_version), 0, 1)) : join(".", slice(split(".", var.engine_version), 0, 2))
   major_engine_version          = var.major_engine_version == "" ? local.computed_major_engine_version : var.major_engine_version
+
+  subnet_ids_provided           = var.subnet_ids != null && length(var.subnet_ids) > 0
+  db_subnet_group_name_provided = var.db_subnet_group_name != null && var.db_subnet_group_name != ""
+
+  db_subnet_group_name = local.db_subnet_group_name_provided ? var.db_subnet_group_name : (
+    local.subnet_ids_provided ? join("", aws_db_subnet_group.default.*.name) : null
+  )
+
+  availability_zone = var.multi_az ? null : var.availability_zone
 }
 
 resource "aws_db_instance" "default" {
-  count                 = module.this.enabled ? 1 : 0
+  count = module.this.enabled ? 1 : 0
+
   identifier            = module.this.id
   name                  = var.database_name
   username              = var.database_user
@@ -32,8 +42,10 @@ resource "aws_db_instance" "default" {
     )
   )
 
+  db_subnet_group_name = local.db_subnet_group_name
+  availability_zone    = local.availability_zone
+
   ca_cert_identifier          = var.ca_cert_identifier
-  db_subnet_group_name        = join("", aws_db_subnet_group.default.*.name)
   parameter_group_name        = length(var.parameter_group_name) > 0 ? var.parameter_group_name : join("", aws_db_parameter_group.default.*.name)
   option_group_name           = length(var.option_group_name) > 0 ? var.option_group_name : join("", aws_db_option_group.default.*.name)
   license_model               = var.license_model
@@ -62,13 +74,27 @@ resource "aws_db_instance" "default" {
 
   monitoring_interval = var.monitoring_interval
   monitoring_role_arn = var.monitoring_role_arn
+
+  depends_on = [
+    aws_db_subnet_group.default,
+    aws_security_group.default,
+    aws_db_parameter_group.default,
+    aws_db_option_group.default
+  ]
+
+  lifecycle {
+    ignore_changes = [
+      snapshot_identifier, # if created from a snapshot, will be non-null at creation, but null afterwards
+    ]
+  }
 }
 
 resource "aws_db_parameter_group" "default" {
-  count  = length(var.parameter_group_name) == 0 && module.this.enabled ? 1 : 0
-  name   = module.this.id
-  family = var.db_parameter_group
-  tags   = module.this.tags
+  count = length(var.parameter_group_name) == 0 && module.this.enabled ? 1 : 0
+
+  name_prefix = "${module.this.id}${module.this.delimiter}"
+  family      = var.db_parameter_group
+  tags        = module.this.tags
 
   dynamic "parameter" {
     for_each = var.db_parameter
@@ -78,11 +104,16 @@ resource "aws_db_parameter_group" "default" {
       value        = parameter.value.value
     }
   }
+
+  lifecycle {
+    create_before_destroy = true
+  }
 }
 
 resource "aws_db_option_group" "default" {
-  count                = length(var.option_group_name) == 0 && module.this.enabled ? 1 : 0
-  name                 = module.this.id
+  count = length(var.option_group_name) == 0 && module.this.enabled ? 1 : 0
+
+  name_prefix          = "${module.this.id}${module.this.delimiter}"
   engine_name          = var.engine
   major_engine_version = local.major_engine_version
   tags                 = module.this.tags
@@ -112,22 +143,25 @@ resource "aws_db_option_group" "default" {
 }
 
 resource "aws_db_subnet_group" "default" {
-  count      = module.this.enabled ? 1 : 0
+  count = module.this.enabled && local.subnet_ids_provided && ! local.db_subnet_group_name_provided ? 1 : 0
+
   name       = module.this.id
   subnet_ids = var.subnet_ids
   tags       = module.this.tags
 }
 
 resource "aws_security_group" "default" {
-  count       = module.this.enabled ? 1 : 0
+  count = module.this.enabled ? 1 : 0
+
   name        = module.this.id
   description = "Allow inbound traffic from the security groups"
   vpc_id      = var.vpc_id
   tags        = module.this.tags
 }
 
 resource "aws_security_group_rule" "ingress_security_groups" {
-  count                    = module.this.enabled ? length(var.security_group_ids) : 0
+  count = module.this.enabled ? length(var.security_group_ids) : 0
+
   description              = "Allow inbound traffic from existing Security Groups"
   type                     = "ingress"
   from_port                = var.database_port
@@ -138,7 +172,8 @@ resource "aws_security_group_rule" "ingress_security_groups" {
 }
 
 resource "aws_security_group_rule" "ingress_cidr_blocks" {
-  count             = module.this.enabled && length(var.allowed_cidr_blocks) > 0 ? 1 : 0
+  count = module.this.enabled && length(var.allowed_cidr_blocks) > 0 ? 1 : 0
+
   description       = "Allow inbound traffic from CIDR blocks"
   type              = "ingress"
   from_port         = var.database_port
@@ -160,11 +195,13 @@ resource "aws_security_group_rule" "egress" {
 }
 
 module "dns_host_name" {
-  source   = "cloudposse/route53-cluster-hostname/aws"
-  version  = "0.12.0"
+  source  = "cloudposse/route53-cluster-hostname/aws"
+  version = "0.12.0"
+
   enabled  = length(var.dns_zone_id) > 0 && module.this.enabled
   dns_name = var.host_name
   zone_id  = var.dns_zone_id
   records  = coalescelist(aws_db_instance.default.*.address, [""])
-  context  = module.this.context
+
+  context = module.this.context
 }
diff --git a/outputs.tf b/outputs.tf
@@ -20,7 +20,7 @@ output "instance_endpoint" {
 
 output "subnet_group_id" {
   value       = join("", aws_db_subnet_group.default.*.id)
-  description = "ID of the Subnet Group"
+  description = "ID of the created Subnet Group"
 }
 
 output "security_group_id" {
diff --git a/test/src/examples_complete_test.go b/test/src/examples_complete_test.go
diff --git a/variables.tf b/variables.tf

Original file line number	Diff line number	Diff line change
`@@ -15,7 +15,7 @@ output "instance_endpoint" {`
`15`	`15`
`16`	`16`	`output "subnet_group_id" {`
`17`	`17`	`value = module.rds_instance.subnet_group_id`
`18`		`- description = "ID of the Subnet Group"`
	`18`	`+ description = "ID of the created Subnet Group"`
`19`	`19`	`}`
`20`	`20`
`21`	`21`	`output "security_group_id" {`
Original file line number	Diff line number	Diff line change
`@@ -20,7 +20,7 @@ output "instance_endpoint" {`
`20`	`20`
`21`	`21`	`output "subnet_group_id" {`
`22`	`22`	`value = join("", aws_db_subnet_group.default.*.id)`
`23`		`- description = "ID of the Subnet Group"`
	`23`	`+ description = "ID of the created Subnet Group"`
`24`	`24`	`}`
`25`	`25`
`26`	`26`	`output "security_group_id" {`