fix: Too long identifier name error

[MaxymVlasov]

what

Fix too long identifier name without cluster recreation:

set id_length_limit = 40 for

module "aurora" {
  source  = "cloudposse/rds-cluster/aws"
  version = "1.10.0"
  ...
  id_length_limit = 40
}

will cause such recreation if final module.this.id will be between 40-60 characters

why

https://github.com/cloudposse/terraform-aws-rds-cluster/releases/tag/1.10.0 introduced random_pet, which adds 2 extra words on the top of module.this.id, and that exceed limit of 63 allowed characters for RDS names

╷
│ Error: creating RDS Cluster (company-staging-aurora-payments-ledger-service-aliases) Instance (company-staging-aurora-payments-ledger-service-aliases-promoted-piglet-1): operation error RDS: CreateDBInstance, https response error StatusCode: 400, RequestID: 1979b42f-b1df-4e00-b0ec-d3b629b3002d, api error InvalidParameterValue: Invalid database identifier:  company-staging-aurora-payments-ledger-service-aliases-promoted-piglet-1
│ 
│   with module.aurora_aliases.aws_rds_cluster_instance.default[0],
│   on .terraform/modules/aurora_aliases/main.tf line 261, in resource "aws_rds_cluster_instance" "default":
│  261: resource "aws_rds_cluster_instance" "default" {

This PR limit final identifier to 62-63 chars (depends on count of replica)

references

Fixing #213

[kevcube]

@MaxymVlasov we still want to retain that random_pet because when we create_before_destroy we need to have unique names for the new instances.

[MaxymVlasov]

@kevcube
TL;DR: id_length_limit in cloudposse/label/null calculate md5() of full_id, crop full_id and attach md5 at the end.

So you still have unique id based on pet_random, but which not violate AWS limitation

https://github.com/cloudposse/terraform-null-label/blob/5c464f42c388deb18222c2285c9e450b1fc233c7/main.tf#L161-L166

[kevcube]

Ah I see, I only looked at the part where you replace random_pet with module.rds_identifier and I assumed that was an identifier that was already being created, now I see it is a new one. LGTM

[kevcube]

/terratest

[kevcube]

/terratest

[MaxymVlasov]

@kevcube tests passed

[kevcube]

@kevcube tests passed

ok - one thing I'm wondering is how we release this, it feels like a fix, but also this will cause consumer's db instances to be recycled so maybe I release as a minor. Going to ask for backup.

[MaxymVlasov]

It will affect only DBs with a random_pet.id length == 61 (>=62 will see the same error as in PR description)
I don't know, how many DBs have exactly this number, so yeah, second eyes could be helpful

[kevcube]

It will affect only DBs with a random_pet.id length == 61 (>=62 will see the same error as in PR description) I don't know, how many DBs have exactly this number, so yeah, second eyes could be helpful

Ah I see, thanks for that info, I thought all identifiers would get the hash appended. In this case I feel safer merging it, I think it's a rare situation that someone has their instance name at the character limit, especially considering the random_pet adds some unpredictability to length.

[kevcube]

Although I cannot merge because I was the last pusher and also only approver 🤡 whoops!

[kevcube]

/terratest

[aknysh]

thanks @MaxymVlasov

[github-actions]

These changes were released in v1.11.1.

[wavemoran]

This should have been a major release as it does force destroy/create on existing databases. It pads out the identifier now. Eg, "foo-data-usw2-dev-airflow-primary-main-aurora-1" -> "foo-data-usw2-dev-airflow-primary-main-aurora-crucial-5b78a-1"

[MaxymVlasov]

@wavemoran not this PR, but #213