From 5ef1c4214d60320ea4c32dce68b02aeee5ca43e7 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Mon, 26 Oct 2026 04:11:20 +0100 Subject: [PATCH 1/2] chore(config): tie suppression slug aliases to the library service constants awsLegacyServiceSlugs spelled its keys and values as string literals that only pointed at mapServiceSlug in comments, so an alias added to one table and not the other would reintroduce the suppression double-buy from #1420. Write both sides as string(common.Service*) constants, hoist the legacy entries of mapServiceSlug into a package-level map (behaviour unchanged), and add a drift test that ranges over it and checks each key maps through config.CanonicalSuppressionService to a canonical slug mapServiceSlug accepts and resolves to the expected family (memorydb passes through). Savings Plans spelling aliases (item 2) are left open: the in-repo producers only emit the hyphenated per-type slugs, so reach is low. Part of #1422 Co-Authored-By: Claude Opus 5.5 --- internal/config/types.go | 15 +++++---- internal/purchase/execution.go | 25 +++++++++----- internal/purchase/slug_alias_drift_test.go | 39 ++++++++++++++++++++++ 3 files changed, 63 insertions(+), 16 deletions(-) create mode 100644 internal/purchase/slug_alias_drift_test.go diff --git a/internal/config/types.go b/internal/config/types.go index 504aa5840..79fa9ae2a 100644 --- a/internal/config/types.go +++ b/internal/config/types.go @@ -7,6 +7,7 @@ import ( "strings" "time" + "github.com/LeanerCloud/cloud-commitments-go/pkg/common" "github.com/LeanerCloud/cloud-commitments-go/pkg/ladder" ) @@ -762,14 +763,14 @@ type SuppressionVariant struct { // awsLegacyServiceSlugs maps the AWS service slugs that rec rows persisted // before slug normalisation still carry to their canonical spelling. -// memorydb has no canonical slug and is deliberately absent. Mirrors the legacy -// entries of mapServiceSlug in internal/purchase/execution.go. +// memorydb has no canonical slug and is deliberately absent. Keyed and valued by the +// library constants; a purchase test checks it against mapServiceSlug. var awsLegacyServiceSlugs = map[string]string{ - "ec2": "compute", - "rds": "relational-db", - "elasticache": "cache", - "opensearch": "search", - "redshift": "data-warehouse", + string(common.ServiceEC2): string(common.ServiceCompute), + string(common.ServiceRDS): string(common.ServiceRelationalDB), + string(common.ServiceElastiCache): string(common.ServiceCache), + string(common.ServiceOpenSearch): string(common.ServiceSearch), + string(common.ServiceRedshift): string(common.ServiceDataWarehouse), } // CanonicalSuppressionService maps legacy AWS service slugs to their canonical diff --git a/internal/purchase/execution.go b/internal/purchase/execution.go index 30a8ff1d4..16679db4b 100644 --- a/internal/purchase/execution.go +++ b/internal/purchase/execution.go @@ -1299,19 +1299,26 @@ func mapServiceSlug(service string) (common.ServiceType, bool) { "cache": common.ServiceCache, "search": common.ServiceSearch, "data-warehouse": common.ServiceDataWarehouse, - // Legacy AWS-only slugs. A new legacy alias must also be added to - // config.awsLegacyServiceSlugs, else suppressions miss it (#1420). - "ec2": common.ServiceEC2, - "rds": common.ServiceRDS, - "elasticache": common.ServiceElastiCache, - "opensearch": common.ServiceOpenSearch, - "redshift": common.ServiceRedshift, - "memorydb": common.ServiceMemoryDB, } - svc, ok := slugs[service] + if svc, ok := slugs[service]; ok { + return svc, true + } + svc, ok := legacyAWSServiceSlugs[service] return svc, ok } +// legacyAWSServiceSlugs are the AWS-only slugs. A new alias must also be added +// to config.awsLegacyServiceSlugs, else suppressions miss it (#1420); a test +// ranges over this map to enforce that. +var legacyAWSServiceSlugs = map[string]common.ServiceType{ + "ec2": common.ServiceEC2, + "rds": common.ServiceRDS, + "elasticache": common.ServiceElastiCache, + "opensearch": common.ServiceOpenSearch, + "redshift": common.ServiceRedshift, + "memorydb": common.ServiceMemoryDB, +} + // mapSavingsPlansSlug normalizes both the canonical hyphenated SP slugs and // the dash-free spellings the frontend has historically sent into the // matching common.ServiceType. The map covers the legacy umbrella plus the diff --git a/internal/purchase/slug_alias_drift_test.go b/internal/purchase/slug_alias_drift_test.go new file mode 100644 index 000000000..30edfd8d7 --- /dev/null +++ b/internal/purchase/slug_alias_drift_test.go @@ -0,0 +1,39 @@ +package purchase + +import ( + "testing" + + "github.com/LeanerCloud/cloud-commitments-go/pkg/common" + "github.com/LeanerCloud/cloud-commitments-platform/internal/config" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// TestLegacyAWSSlugsMatchSuppressionAliases guards the pair of alias tables +// (config.awsLegacyServiceSlugs and legacyAWSServiceSlugs) against drifting +// apart, which would let a suppression miss a rec spelled the other way (#1420). +func TestLegacyAWSSlugsMatchSuppressionAliases(t *testing.T) { + canonicalFamily := map[string]common.ServiceType{ + "ec2": common.ServiceCompute, + "rds": common.ServiceRelationalDB, + "elasticache": common.ServiceCache, + "opensearch": common.ServiceSearch, + "redshift": common.ServiceDataWarehouse, + } + for key := range legacyAWSServiceSlugs { + t.Run(key, func(t *testing.T) { + canonical := config.CanonicalSuppressionService("aws", key) + if key == string(common.ServiceMemoryDB) { + // Documented exception: no canonical slug, passes through. + assert.Equal(t, key, canonical) + return + } + want, ok := canonicalFamily[key] + require.True(t, ok, "legacy key %q has no expected canonical family; add it to this test and config.awsLegacyServiceSlugs", key) + assert.NotEqual(t, key, canonical, "legacy key %q is missing from config.awsLegacyServiceSlugs", key) + got, ok := mapServiceSlug(canonical) + require.True(t, ok, "mapServiceSlug rejects canonical slug %q", canonical) + assert.Equal(t, want, got) + }) + } +} From 0bdd100fcf15e66e84f0cffae9aeb23b8511d7ce Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Mon, 26 Oct 2026 04:48:37 +0100 Subject: [PATCH 2/2] test(purchase): fail the alias drift test when a legacy slug is removed The drift test ranged only over legacyAWSServiceSlugs, so deleting an entry such as rds or memorydb from that map passed silently. Assert the expected legacy keys are present before the loop. Part of #1422 Co-Authored-By: Claude Opus 5.5 --- internal/purchase/slug_alias_drift_test.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/internal/purchase/slug_alias_drift_test.go b/internal/purchase/slug_alias_drift_test.go index 30edfd8d7..739088705 100644 --- a/internal/purchase/slug_alias_drift_test.go +++ b/internal/purchase/slug_alias_drift_test.go @@ -20,6 +20,11 @@ func TestLegacyAWSSlugsMatchSuppressionAliases(t *testing.T) { "opensearch": common.ServiceSearch, "redshift": common.ServiceDataWarehouse, } + // Deleting an entry from legacyAWSServiceSlugs must not pass silently. + for key := range canonicalFamily { + require.Contains(t, legacyAWSServiceSlugs, key, "legacy slug %q was removed from legacyAWSServiceSlugs", key) + } + require.Contains(t, legacyAWSServiceSlugs, string(common.ServiceMemoryDB), "legacy slug memorydb was removed from legacyAWSServiceSlugs") for key := range legacyAWSServiceSlugs { t.Run(key, func(t *testing.T) { canonical := config.CanonicalSuppressionService("aws", key)