computed_major_engine_version not working for postgres 9.6.18

[JCFerrerG]

Describe the Bug

computed major_engine_version fails when...

var.engine == "postgres"
var.engine_version == "9.6.18"

The computed value should be 9.6 but actually results in 9 and the following error:

Error: Error creating DB Option Group: InvalidParameterCombination: Cannot find major version 9 for postgres

Expected Behavior

When var.engine_version == "9.6.18", the computed_major_engine_version should be 9.6.

Steps to Reproduce

Steps to reproduce the behavior:

1. Copy examples/complete to a new terraform configuration.
2. Change the following variables in fixtures.us-east-2.tfvars:

engine = "postgresql"
engine_version = "9.6.18"
major_engine_version = "9.6" # or set to empty string
db_parameter_group = "postgres9.6"

3. Run the plan terraform plan -var-file="fixtures.us-east-2.tfvars". Observe that...

resource "aws_db_option_group" "default" {
     ...
     engine_name              = "postgres"
     major_engine_version     = "9"
     ...
}

4. Apply the configuration with terraform apply -var-file="fixtures.us-east-2.tfvars
5. See error

Environment (please complete the following information):

Anything that will help us triage the bug will help. Here are some ideas:

• OS: OSX
• cloudeposse/terraform-aws-rds v0.26.0
• terraform v0.12.29

[JCFerrerG]

Actually, my expectation that the major_engine_version follow a pattern of x.x like 9.6 is inconsistent with the assumptions that were implemented in https://github.com/cloudposse/terraform-aws-rds/pull/42 (which expects that version to be a single number).

Open questions:

• What is the pattern for major version on DB option groups?
• Is that pattern consistent for different versions of postgres?

Maybe it is infeasible to calculate major_engine_version consistently?

If that's the case, I might suggest making major_engine_version required and removing the ability to calculate it (as is the case with aws_db_option_group).

For now, I'll be specifying the var.major_engine_version like in this example: https://github.com/JCFerrerG/terraform-aws-rds/commit/bfd31510dbfe86211a616505fbac6980bfee223a

[antdking]

If it's of use, we're using a different way to calculate this internally:

# leave the regex open ended, Aurora, Oracle and SQLServer can all contain characters
version_parts = regex("^(?P<major>[0-9]+)(?:\\.(?P<minor>[0-9]+))?", var.engine_version)
version_parts_number = {
  major = tonumber(local.version_parts["major"])
}
is_postgres                  = var.engine == "postgres"
uses_shorthand_major_version = local.is_postgres && local.version_parts_number["major"] >= 10

computed_major_engine_version = local.uses_shorthand_major_version ? local.version_parts["major"] : "${local.version_parts["major"]}.${local.version_parts["minor"]}"
major_engine_version          = var.major_engine_version == "" ? local.computed_major_engine_version : var.major_engine_version

It's working for postgres 9.6, 11, and 12. Feel free to use it here. We're also using this to set aws_db_instance.engine_version, so we don't get state changes whenever an auto-minor update happens.

while the comment talks about other databases, this has only been tested with Postgres. Looking at the docs for sqlserver and oracle, it should still work as expected.