claranet / terraform-azurerm-regions

Terraform module to handle Azure Regions
Apache License 2.0
62 stars 58 forks source link

[BUG] logic behind .location_short region names #7

Closed Sysa closed 1 year ago

Sysa commented 1 year ago

Community Note

Hello there, First of all thank you for this module and contributions to it. It is not really a bug but was discovered during usage of the module and caused some problems. So this bug report will contain an explanation of the case, question and possible workarounds. Might be the place to bring up a discussion about possible solutions too.

Default azure sdk/cli does not contain a short region notation (like 3-4 chars) for its locations.

This command gives a brief view on what it has under the hood: az account list-locations -o table

DisplayName               Name                 RegionalDisplayName
------------------------  -------------------  -------------------------------------
East US                   eastus               (US) East US
East US 2                 eastus2              (US) East US 2
South Central US          southcentralus       (US) South Central US
West US 2                 westus2              (US) West US 2
West US 3                 westus3              (US) West US 3
Australia East            australiaeast        (Asia Pacific) Australia East
Southeast Asia            southeastasia        (Asia Pacific) Southeast Asia
North Europe              northeurope          (Europe) North Europe
Sweden Central            swedencentral        (Europe) Sweden Central
UK South                  uksouth              (Europe) UK South
West Europe               westeurope           (Europe) West Europe
Central US                centralus            (US) Central US

The name field is kind of short name notation but not really short. At this point everyone is up to use their own logic of combination of country+region to make a short name for a location. I believe it is where this terraform module was born initially, to cover such gap in implementation.

According to the list of mapping azure regions in this module: https://github.com/claranet/terraform-azurerm-regions/blob/master/REGIONS.md#azure-regions-mapping-list - there is a "Short notation" and it feels all right for most of the popular regions until you start using more regions.

There is also a comprehensive (and boring) list of country codes which can be used as an international reference: https://en.wikipedia.org/wiki/List_of_ISO_3166_country_codes

What is the issue then? Let me pick few regions to highlight my concerns:

List below is from this terraform module ("azure-regions-mapping-list"):

Region name Short notation Internal terraform notation my comment
North Europe eun eu-north feels ok
West Europe euw eu-west feels ok
Central US uc us-central why it is just 'uc' and not usc? god thanks it is not 'cu' and leading to Cuba
UAE North uaen uae-north feels ok but with the logic out of 'uc' it may be just 'ae'
North Central US unc us-north-central why it is 'unc' not 'usnc'? Why Europe regions then has two letters represents 'eu' not just 'e'?
Canada East cae can-east hmm.. all right
East Asia ae asia-east why it is 'ae' not 'ase'? oh, because we have 'ase' as Southeast Asia already. moreover 'ae' is leading more for UAE not the East Asia tbh (see ISO 3166 and domain name for UAE is .ae)

My question is what is the logic behind such naming notation for short region names? I don't see any, please point me to it. I thought it might be something like "at least 3 chars" should represent the region to improve unambiguously of short naming.

I started using this module from EU regions and then spread more to others and found such a misleading logic (in my opinion) which does not give any consistency to what I usually see in different projects/companies. But I might be biased and make only assumptions which also can be wrong. But what I certain know is that I never see the US shortcut as just "U" (except for US Gov-specific regions which usually a 'UG' but in this module for US Gov I see just 'gov' - what?! Because plain 'u' is already in use to reflect regular US regions...)

I don't see any neat solution to that because it will either break already existing naming notations people have (who have been using this module from day 0) or just have another mapping field like .location_short2. but what if a person would need a mix of those? I don't know but would like to hear your opinion or suggestion.

So far I have dumb workarounds like that to have your own re-mapping in particular places/regions where you need it:

locals {
  location_short_map = {
    unc : "usnc"
    euw : "euw"
    eun : "eun"
  }
}

module "redis_regions" {
  for_each     = toset(var.regions_to_deploy[local.env])
  source       = "claranet/regions/azurerm"
  version      = "6.1.0"
  azure_region = each.key
}

resource "whatever_azure_resource" "resource_name" {
  for_each = module.redis_regions
  name                          = "bla-bla-${local.location_short_map[each.value.location_short]}-bla-bla"
  location                      = each.value.location

Thank you for your time and again, thank you for this module and contributions to it! Sincerely, Alex

Terraform Version

v1.5.6

AzureRM Provider Version

6.1.0

Affected Resource(s)/Data Source(s)

azurerm_*

Terraform Configuration Files

N/A

Debug Output/Panic Output

N/A

Expected Behaviour

I expect some straightforward logic for naming convention or written explanation why certain names for regions were picked up.

Actual Behaviour

Misleading short names for regions

Steps to Reproduce

No response

Important Factoids

No response

References

No response

Shr3ps commented 1 year ago

Thanks @Sysa for this great issue explanation. I think you're right, region with only "2" chars are mostly regions we don't use for our customers.

Your suggestion about having at least 3 chars to apply TLA logic seems good.

About the module itself, I think it's not big deal having a breaking change along with a new major module version, people using old one can still pin an older version to avoid those changes.

Do you think you can challenge the list and open a Pull-Request?

Sysa commented 1 year ago

Thanks @Shr3ps for prompt response!

I'll try to set up a development environment and raise a PR for this. might take a while.

Let's then agree on the definition of a potential feature/fix: -it would completely change the short name naming convention and not have any additional like "short2" or "short_old" to have backwards compatibility. old style short name can be achieved by pinning the right version of the module for it. -it will have a requirement of minimum chars for short-name of a region (let's say 3)

From my side, I'll do additional research about shorting geo names in public/private clouds to make the solution as generic as possible and explore potential edge cases.

Any suggestions are welcome.