The current default is t2.micro, but should be db.t2.micro.

Hi,

Could you add auto_minor_version_upgrade parameter ?
This optional parameter defaults to yes.
https://www.terraform.io/docs/providers/aws/r/db_instance.html#auto_minor_version_upgrade

Thanks

Terraform has deprecated the name field in the aws_db_instance resource. We should use db_name here instead.

• Enable Performance Insights by default
• Stop supporting unencrypted storage. Generally the performance impact should be negligible.
• Remove "Enhanced Monitoring" support - it's marginally more expensive but just provides used space (in enhanced) vs free space (in normal). Our implementation is also not great.
• Default to not exporting logs to cloudwatch (in DB they just charge a large extra charge and you can view logs in DB directly). - Or remove this capability.

Review our projects and see what settings they are actually using.

could we add an input variable that allows us to use an existing security group instead of redefining a new one as mentioned in #15 ?

Amazon RDS for PostgreSQL has support for storage autoscaling which allows us to define an upper bound for storage, and then RDS automatically scales up capacity as storage utilization increases.

See:

• https://docs.aws.amazon.com/AmazonRDS/latest/UserGuide/USER_PIOPS.StorageTypes.html#USER_PIOPS.Autoscaling
• https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/db_instance#storage-autoscaling

A project was successfully using 2.5.0 of this module. An attempt to upgrade it to 2.6.0 caused the following exception:

* aws_db_instance.postgresql: Error modifying DB Instance dbtest: InvalidParameterCombination: A MonitoringRoleARN value is required if you specify a MonitoringInterval value other than 0.
        status code: 400, request id: 5afb5277-7312-4f52-8628-3ec71b8eeafb

This was triggered with monitoring_interval = 60. If monitoring_interval = 0, then the error does not surface.

For some reason, the CloudWatch alarms are not receiving baseline metrics for the RDS instance created.

Copied from azavea/terraform-aws-vpc#29.

Version 3.38.0 of the Terraform AWS Provider introduced a way to define a default set of tags that propagate to all AWS resources that support tags.

Any tags defined at the resource level are merged with tags defined at the provider level and are accessible via a new read-only attribute, tags_all.

e.g.:

# module.vpc.aws_vpc.default will be created
+ resource "aws_vpc" "default" {
    + arn                              = (known after apply)
    + assign_generated_ipv6_cidr_block = false
    + cidr_block                       = "10.0.0.0/16"
    + default_network_acl_id           = (known after apply)
    + default_route_table_id           = (known after apply)
    + default_security_group_id        = (known after apply)
    + dhcp_options_id                  = (known after apply)
    + enable_classiclink               = (known after apply)
    + enable_classiclink_dns_support   = (known after apply)
    + enable_dns_hostnames             = true
    + enable_dns_support               = true
    + id                               = (known after apply)
    + instance_tenancy                 = "default"
    + ipv6_association_id              = (known after apply)
    + ipv6_cidr_block                  = (known after apply)
    + main_route_table_id              = (known after apply)
    + owner_id                         = (known after apply)
    + tags                             = {
        + "Environment" = "Staging"
        + "Name"        = "vpcStaging"
        + "Project"     = "Something"
      }
    + tags_all                         = {
        + "Environment" = "Staging"
        + "Name"        = "vpcStaging"
        + "Project"     = "Something"
      }
  }

I think we should consider how we'd drop the Environment and Project tags and instructs folks to use default_tags.

We already allow users to override the aws_db_instance.storage_type via the module's storage_type variable, but in order to fully support setting storage_type to io1, we need to add an iops variable to set the specific number of IOPS for the volume.

The underlying aws_db_instance resource has support for deletion protection, which prevents the RDS instance from being deleted if enabled. We should surface this setting at the model level so that it can be modified to something other than its default value (false).

In #14, we created an empty default security group in order to give users the flexibility to define their own firewall rules. However, it would be useful to have some base rules that allows ingress/egress traffic on the database port, which can be a user-supplied parameter. We can then continue to output the security group id for further customization at the user's discretion.

It looks like the default functionality of this module is to specify a value for monitoring_role_arn on the aws_db_instance resource, and set the monitoring_interval to 0.

According to this Terraform error I received, the default functionality of this module may be broken:

Error: Error applying plan:

1 error(s) occurred:

* module.database.aws_db_instance.postgresql: 1 error(s) occurred:

* aws_db_instance.postgresql: Error creating DB Instance: InvalidParameterCombination: You must specify a MonitoringInterval value other than 0 when you specify a MonitoringRoleARN value.
	status code: 400, request id: d996bf5a-c269-4548-87ec-6b3676b7a9aa

If you try to upgrade the major version of a DB instance via Terraform, the apply operation will fail like:

Error: Error modifying DB Instance districtbuilder-staging: InvalidParameterCombination: The AllowMajorVersionUpgrade flag must be present when upgrading to a new major version.
        status code: 400, request id: 299dedfe-4b1e-4746-8b2a-5c562d9f8c89

The AWS docs don't rule out the possibility of modifying the DB engine via Terraform:

For major version upgrades, you must manually modify the DB engine version through the AWS Management Console, AWS CLI, or RDS API. For minor version upgrades, you can manually modify the engine version, or you can choose to enable auto minor version upgrades.

Paired with the use of name_prefix for the DB parameter group (PublicMapping/districtbuilder#478), we could have Terraform perform an entire major version upgrade. This would eliminate the need for us to ensure a parameter group exists and thread it through between version hops, freeing up more mental bandwidth for monitoring the upgrade process.

I could see us leaving this variable set to false when business is usual, but we set it to true when performing the upgrade.

See: https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/db_instance#allow_major_version_upgrade

Hi,

If final_snapshot_identifier is omitted, all snapshots are deleted when DB instance is destroyed and no final DB snapshot is created.

I think it would be good to change this behaviour.
Also, copy_tags_to_snapshot is handy.

Thanks
J.

This module isn't currently compatible with Terraform 0.12. The tags blocks and dimensions blocks in main.tf need to be explicitly defined as arguments with = {. The 0.12 upgrade guide explains why this version is more strict about the syntax.

We can also use new syntax for referencing expressions without quotes.

This could possibly be resolved with terraform 0.12upgrade. It would require a major version bump since the module would now require Terraform >= 0.12.

Hi,

auto_minor_version_upgrade is missing in aws_db_instance since version 2.

Thanks
J.

Currently, no alarm actions are associated with the CloudWatch alarms created.