From 6de527086430340c6c17b737cd6643f2baec514f Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Sun, 25 Oct 2026 19:51:21 +0100 Subject: [PATCH] fix(aws/recommendations): map Taipei, New Zealand and the remaining partition region names Add Cost Explorer display names for ap-east-2 (Taipei) and ap-southeast-6 (New Zealand), plus the China, GovCloud, ISO and European Sovereign Cloud descriptions found in aws-sdk-go-v2 internal/endpoints/awsrulesfn/partitions.json. Hoist the alias table to a package-level regionAliases map, drop the prefix branch that returned its input unchanged (same fallthrough result for every input), and make the collision test table-driven over the full map. Refs #53 Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 1 + providers/aws/recommendations/converters.go | 111 +++++++++--------- .../aws/recommendations/converters_test.go | 57 ++++++--- 3 files changed, 100 insertions(+), 69 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f94aac05..35455669 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -218,6 +218,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/). - **AWS/OpenSearch:** the STS account-ID lookup used for post-purchase tagging caches only a successful result, so one throttled or timed-out `GetCallerIdentity` no longer disables tagging for the client's lifetime (#53). - **AWS/recommendations:** `normalizeRegionName` maps the current `Europe (Ireland|Frankfurt|London|Paris|Stockholm)` Cost Explorer display names to region codes, so coverage lookups keyed by region code match those recommendations (#53). - **AWS/recommendations:** `normalizeRegionName` also maps `Asia Pacific (Malaysia|Thailand)`, `Canada West (Calgary)` and `Mexico (Central)` to `ap-southeast-5`, `ap-southeast-7`, `ca-west-1` and `mx-central-1` (#53). +- **AWS/recommendations:** `normalizeRegionName` also maps `Asia Pacific (Taipei)` (`ap-east-2`), `Asia Pacific (New Zealand)` (`ap-southeast-6`) and the China, GovCloud, ISO and European Sovereign Cloud region descriptions from the SDK partitions file (#53). The alias table is now a package-level `regionAliases` map, the no-op region-code prefix branch is removed (output unchanged for every input), and the collision test covers the whole map. - **AWS/Savings Plans:** `GetExistingCommitments` returns an error naming the plan ID and value when a plan has an unparseable start or end date, instead of silently leaving the date zero (#53). - **AWS/Savings Plans:** `GetExistingCommitments` returns an error naming the plan ID when a plan has no start or end date; a zero start date was never treated as recent by the purchase dedupe, so a duplicate purchase could slip through (#53). - **AWS/RDS:** `PurchaseCommitment` refuses an offering whose upfront (`FixedPrice` x count) exceeds the approved recommendation upfront by more than 5%, or that is priced in a non-USD currency; a zero approved upfront is not checked (#53). diff --git a/providers/aws/recommendations/converters.go b/providers/aws/recommendations/converters.go index bce4fa59..0f607cf3 100644 --- a/providers/aws/recommendations/converters.go +++ b/providers/aws/recommendations/converters.go @@ -2,7 +2,6 @@ package recommendations import ( "fmt" - "strings" "github.com/aws/aws-sdk-go-v2/service/costexplorer/types" @@ -94,61 +93,67 @@ func convertSavingsPlansLookbackPeriod(period string) (types.LookbackPeriodInDay return convertLookbackPeriodE(period) } +// regionAliases maps Cost Explorer location display names to region codes. +// Descriptions come from aws-sdk-go-v2 internal/endpoints/awsrulesfn/partitions.json. +var regionAliases = map[string]string{ + "US East (N. Virginia)": "us-east-1", + "US East (Ohio)": "us-east-2", + "US West (N. California)": "us-west-1", + "US West (Oregon)": "us-west-2", + "EU (Ireland)": "eu-west-1", + "EU (Frankfurt)": "eu-central-1", + "EU (London)": "eu-west-2", + "EU (Paris)": "eu-west-3", + "EU (Stockholm)": "eu-north-1", + "Asia Pacific (Singapore)": "ap-southeast-1", + "Asia Pacific (Sydney)": "ap-southeast-2", + "Asia Pacific (Tokyo)": "ap-northeast-1", + "Asia Pacific (Seoul)": "ap-northeast-2", + "Asia Pacific (Mumbai)": "ap-south-1", + "South America (Sao Paulo)": "sa-east-1", + "Canada (Central)": "ca-central-1", + "Middle East (Bahrain)": "me-south-1", + "Africa (Cape Town)": "af-south-1", + "Asia Pacific (Hong Kong)": "ap-east-1", + "Asia Pacific (Osaka)": "ap-northeast-3", + "Asia Pacific (Jakarta)": "ap-southeast-3", + "Europe (Ireland)": "eu-west-1", + "Europe (Frankfurt)": "eu-central-1", + "Europe (London)": "eu-west-2", + "Europe (Paris)": "eu-west-3", + "Europe (Stockholm)": "eu-north-1", + "Europe (Milan)": "eu-south-1", + "Middle East (UAE)": "me-central-1", + "Asia Pacific (Hyderabad)": "ap-south-2", + "Europe (Spain)": "eu-south-2", + "Europe (Zurich)": "eu-central-2", + "Asia Pacific (Melbourne)": "ap-southeast-4", + "Israel (Tel Aviv)": "il-central-1", + "Asia Pacific (Malaysia)": "ap-southeast-5", + "Asia Pacific (Thailand)": "ap-southeast-7", + "Canada West (Calgary)": "ca-west-1", + "Mexico (Central)": "mx-central-1", + "Asia Pacific (Taipei)": "ap-east-2", + "Asia Pacific (New Zealand)": "ap-southeast-6", + "China (Beijing)": "cn-north-1", + "China (Ningxia)": "cn-northwest-1", + "AWS European Sovereign Cloud (Germany)": "eusc-de-east-1", + "US ISO East": "us-iso-east-1", + "US ISO WEST": "us-iso-west-1", + "US ISOB East (Ohio)": "us-isob-east-1", + "US ISOB West": "us-isob-west-1", + "EU ISOE West": "eu-isoe-west-1", + "US ISOF EAST": "us-isof-east-1", + "US ISOF SOUTH": "us-isof-south-1", + "AWS GovCloud (US-East)": "us-gov-east-1", + "AWS GovCloud (US-West)": "us-gov-west-1", +} + // normalizeRegionName converts AWS region display names to region codes. +// Anything that is not a known display name (including region codes) is returned as-is. func normalizeRegionName(region string) string { - // AWS Cost Explorer sometimes returns region names like "US East (N. Virginia)" - // Convert these to standard region codes - regionMap := map[string]string{ - "US East (N. Virginia)": "us-east-1", - "US East (Ohio)": "us-east-2", - "US West (N. California)": "us-west-1", - "US West (Oregon)": "us-west-2", - "EU (Ireland)": "eu-west-1", - "EU (Frankfurt)": "eu-central-1", - "EU (London)": "eu-west-2", - "EU (Paris)": "eu-west-3", - "EU (Stockholm)": "eu-north-1", - "Asia Pacific (Singapore)": "ap-southeast-1", - "Asia Pacific (Sydney)": "ap-southeast-2", - "Asia Pacific (Tokyo)": "ap-northeast-1", - "Asia Pacific (Seoul)": "ap-northeast-2", - "Asia Pacific (Mumbai)": "ap-south-1", - "South America (Sao Paulo)": "sa-east-1", - "Canada (Central)": "ca-central-1", - "Middle East (Bahrain)": "me-south-1", - "Africa (Cape Town)": "af-south-1", - "Asia Pacific (Hong Kong)": "ap-east-1", - "Asia Pacific (Osaka)": "ap-northeast-3", - "Asia Pacific (Jakarta)": "ap-southeast-3", - "Europe (Ireland)": "eu-west-1", - "Europe (Frankfurt)": "eu-central-1", - "Europe (London)": "eu-west-2", - "Europe (Paris)": "eu-west-3", - "Europe (Stockholm)": "eu-north-1", - "Europe (Milan)": "eu-south-1", - "Middle East (UAE)": "me-central-1", - "Asia Pacific (Hyderabad)": "ap-south-2", - "Europe (Spain)": "eu-south-2", - "Europe (Zurich)": "eu-central-2", - "Asia Pacific (Melbourne)": "ap-southeast-4", - "Israel (Tel Aviv)": "il-central-1", - "Asia Pacific (Malaysia)": "ap-southeast-5", - "Asia Pacific (Thailand)": "ap-southeast-7", - "Canada West (Calgary)": "ca-west-1", - "Mexico (Central)": "mx-central-1", - } - - if normalized, ok := regionMap[region]; ok { + if normalized, ok := regionAliases[region]; ok { return normalized } - - // If already a region code, return as-is - if strings.HasPrefix(region, "us-") || strings.HasPrefix(region, "eu-") || - strings.HasPrefix(region, "ap-") || strings.HasPrefix(region, "sa-") || - strings.HasPrefix(region, "ca-") || strings.HasPrefix(region, "me-") || - strings.HasPrefix(region, "af-") || strings.HasPrefix(region, "il-") { - return region - } - return region } diff --git a/providers/aws/recommendations/converters_test.go b/providers/aws/recommendations/converters_test.go index 6f6aa9ad..196c856d 100644 --- a/providers/aws/recommendations/converters_test.go +++ b/providers/aws/recommendations/converters_test.go @@ -577,26 +577,51 @@ func TestNormalizeRegionName(t *testing.T) { } } -// TestNormalizeRegionName_NoCollisions pins that distinct display names map to -// distinct region codes, so a new alias cannot silently shadow another region. +// TestNormalizeRegionName_NoCollisions walks the full alias map: every display +// name resolves to its code, no two names share a code except the documented +// EU/Europe prefix pairs, and the regions added from partitions.json are pinned. func TestNormalizeRegionName_NoCollisions(t *testing.T) { t.Parallel() + euPairs := map[string]string{ + "EU (Ireland)": "Europe (Ireland)", "EU (Frankfurt)": "Europe (Frankfurt)", + "EU (London)": "Europe (London)", "EU (Paris)": "Europe (Paris)", + "EU (Stockholm)": "Europe (Stockholm)", + } byCode := map[string]string{} + for name, code := range regionAliases { + t.Run(name, func(t *testing.T) { + assert.Equal(t, code, normalizeRegionName(name)) + assert.NotEqual(t, name, code) + }) + prev, dup := byCode[code] + if dup { + assert.True(t, euPairs[name] == prev || euPairs[prev] == name, + "%q and %q both map to %q", prev, name, code) + } + byCode[code] = name + } +} + +// TestNormalizeRegionName_PartitionsJSONAliases pins the aliases taken from +// aws-sdk-go-v2 internal/endpoints/awsrulesfn/partitions.json (v1.41.5). +func TestNormalizeRegionName_PartitionsJSONAliases(t *testing.T) { + t.Parallel() for name, code := range map[string]string{ - "Asia Pacific (Malaysia)": "ap-southeast-5", - "Asia Pacific (Thailand)": "ap-southeast-7", - "Canada West (Calgary)": "ca-west-1", - "Mexico (Central)": "mx-central-1", - "Canada (Central)": "ca-central-1", - "Asia Pacific (Jakarta)": "ap-southeast-3", - "Asia Pacific (Singapore)": "ap-southeast-1", - "Asia Pacific (Sydney)": "ap-southeast-2", - "Asia Pacific (Melbourne)": "ap-southeast-4", + "Asia Pacific (Taipei)": "ap-east-2", + "Asia Pacific (New Zealand)": "ap-southeast-6", + "China (Beijing)": "cn-north-1", + "China (Ningxia)": "cn-northwest-1", + "AWS European Sovereign Cloud (Germany)": "eusc-de-east-1", + "US ISO East": "us-iso-east-1", + "US ISO WEST": "us-iso-west-1", + "US ISOB East (Ohio)": "us-isob-east-1", + "US ISOB West": "us-isob-west-1", + "EU ISOE West": "eu-isoe-west-1", + "US ISOF EAST": "us-isof-east-1", + "US ISOF SOUTH": "us-isof-south-1", + "AWS GovCloud (US-East)": "us-gov-east-1", + "AWS GovCloud (US-West)": "us-gov-west-1", } { - got := normalizeRegionName(name) - assert.Equal(t, code, got, name) - prev, dup := byCode[got] - assert.False(t, dup, "%q and %q both map to %q", prev, name, got) - byCode[got] = name + assert.Equal(t, code, normalizeRegionName(name), name) } }