fix(config): canonicalise dash-free Savings Plans spellings in suppressions

internal/config/suppressions_build_test.go

@@ -217,6 +217,14 @@ func TestCanonicalSuppressionService(t *testing.T) {
 		{"aws", "search", "search"},
 		{"aws", "data-warehouse", "data-warehouse"},
 		{"aws", "memorydb", "memorydb"},
+		{"aws", "savingsplans-compute", "savings-plans-compute"},
+		{"aws", "savingsplans-ec2instance", "savings-plans-ec2instance"},
+		{"aws", "savingsplans-sagemaker", "savings-plans-sagemaker"},
+		{"aws", "savingsplans-database", "savings-plans-database"},
+		{"aws", "savings-plans-compute", "savings-plans-compute"},
+		{"aws", "savingsplans", "savingsplans"},
+		{"azure", "savingsplans", "savingsplans"},
+		{"azure", "savingsplans-compute", "savingsplans-compute"},
 		{"azure", "compute", "compute"},
 		{"azure", "ec2", "ec2"},
 		{"azure", "rds", "rds"},

internal/config/types.go

@@ -782,19 +782,36 @@ var awsLegacyServiceSlugs = map[string]string{
 	string(common.ServiceRedshift):    string(common.ServiceDataWarehouse),
 }
 
-// CanonicalSuppressionService maps legacy AWS service slugs to their canonical
-// slug so a suppression and a rec match whichever spelling each was stored
-// under (issues #1412, #1420). Other providers' services, including Azure and
-// GCP "compute", are returned unchanged. It lives in config, the lowest
-// package both the scheduler and BuildSuppressions import; the pinned
-// cloud-commitments-go common package has no such mapping.
+// awsSavingsPlansSpellings maps the dash-free spelling of each per-plan-type
+// Savings Plans slug, which the purchase manager's mapSavingsPlansSlug also
+// accepts, to the hyphenated library constant that recs carry (issue #1422).
+// The "savingsplans" umbrella is deliberately absent: folding it into one plan
+// type would let a single umbrella suppression hide another plan type's recs.
+// A purchase test checks every per-type key of mapSavingsPlansSlug against it.
+var awsSavingsPlansSpellings = map[string]string{
+	"savingsplans-compute":     string(common.ServiceSavingsPlansCompute),
+	"savingsplans-ec2instance": string(common.ServiceSavingsPlansEC2Instance),
+	"savingsplans-sagemaker":   string(common.ServiceSavingsPlansSageMaker),
+	"savingsplans-database":    string(common.ServiceSavingsPlansDatabase),
+}
+
+// CanonicalSuppressionService maps legacy AWS service slugs, and the dash-free
+// spellings of the per-type Savings Plans slugs, to their canonical slug so a
+// suppression and a rec match whichever spelling each was stored under
+// (issues #1412, #1420, #1422). Other providers' services, including Azure and
+// GCP "compute" and Azure "savingsplans", are returned unchanged. It lives in
+// config, the lowest package both the scheduler and BuildSuppressions import;
+// the pinned cloud-commitments-go common package has no such mapping.
 func CanonicalSuppressionService(provider, service string) string {
 	if provider != "aws" {
 		return service
 	}
 	if canonical, ok := awsLegacyServiceSlugs[service]; ok {
 		return canonical
 	}
+	if canonical, ok := awsSavingsPlansSpellings[service]; ok {
+		return canonical
+	}
 	return service
 }
 

internal/purchase/execution.go

@@ -1341,23 +1341,28 @@ var legacyAWSServiceSlugs = map[string]common.ServiceType{
 // flip the coverage_extra_test.go case (line ~60) to assert
 // ServiceType("savings-plans") (default-arm pass-through). The
 // "savingsplans-*" keys are forward-compat and should stay.
+//
+// A new per-plan-type spelling must also be added to
+// config.awsSavingsPlansSpellings, else a suppression stored under it misses
+// recs (#1422); a test ranges over savingsPlansSlugs to enforce that.
 func mapSavingsPlansSlug(service string) (common.ServiceType, bool) {
-	slugs := map[string]common.ServiceType{
-		"savings-plans":             common.ServiceSavingsPlansAll,
-		"savingsplans":              common.ServiceSavingsPlansAll,
-		"savings-plans-compute":     common.ServiceSavingsPlansCompute,
-		"savingsplans-compute":      common.ServiceSavingsPlansCompute,
-		"savings-plans-ec2instance": common.ServiceSavingsPlansEC2Instance,
-		"savingsplans-ec2instance":  common.ServiceSavingsPlansEC2Instance,
-		"savings-plans-sagemaker":   common.ServiceSavingsPlansSageMaker,
-		"savingsplans-sagemaker":    common.ServiceSavingsPlansSageMaker,
-		"savings-plans-database":    common.ServiceSavingsPlansDatabase,
-		"savingsplans-database":     common.ServiceSavingsPlansDatabase,
-	}
-	svc, ok := slugs[service]
+	svc, ok := savingsPlansSlugs[service]
 	return svc, ok
 }
 
+var savingsPlansSlugs = map[string]common.ServiceType{
+	"savings-plans":             common.ServiceSavingsPlansAll,
+	"savingsplans":              common.ServiceSavingsPlansAll,
+	"savings-plans-compute":     common.ServiceSavingsPlansCompute,
+	"savingsplans-compute":      common.ServiceSavingsPlansCompute,
+	"savings-plans-ec2instance": common.ServiceSavingsPlansEC2Instance,
+	"savingsplans-ec2instance":  common.ServiceSavingsPlansEC2Instance,
+	"savings-plans-sagemaker":   common.ServiceSavingsPlansSageMaker,
+	"savingsplans-sagemaker":    common.ServiceSavingsPlansSageMaker,
+	"savings-plans-database":    common.ServiceSavingsPlansDatabase,
+	"savingsplans-database":     common.ServiceSavingsPlansDatabase,
+}
+
 // updatePlanProgress advances the ramp schedule after a purchase.
 // Direct-execute purchases (Opportunities flow) have no plan to advance --
 // PlanID is empty and the Postgres UUID column would reject the query

internal/purchase/slug_alias_drift_test.go

@@ -42,3 +42,25 @@ func TestLegacyAWSSlugsMatchSuppressionAliases(t *testing.T) {
 		})
 	}
 }
+
+// TestSavingsPlansSpellingsMatchSuppressionAliases checks that every per-type
+// Savings Plans spelling the purchase manager accepts canonicalises, for
+// suppressions, to the slug it buys, so a suppression stored under the
+// dash-free spelling still hides the hyphenated rec (#1422). The umbrella
+// spellings must stay umbrellas: folding one into a plan type would let one
+// umbrella suppression hide every plan type's recs.
+func TestSavingsPlansSpellingsMatchSuppressionAliases(t *testing.T) {
+	for _, pt := range common.SavingsPlansPlanTypes() {
+		require.Contains(t, savingsPlansSlugs, string(pt), "per-type slug %q was removed from savingsPlansSlugs", pt)
+	}
+	for key, want := range savingsPlansSlugs {
+		t.Run(key, func(t *testing.T) {
+			canonical := config.CanonicalSuppressionService("aws", key)
+			if want == common.ServiceSavingsPlansAll {
+				assert.Equal(t, key, canonical, "umbrella spelling %q must pass through unchanged", key)
+				return
+			}
+			assert.Equal(t, string(want), canonical, "spelling %q is missing from config.awsSavingsPlansSpellings", key)
+		})
+	}
+}

internal/scheduler/scheduler_suppressions_test.go

@@ -359,7 +359,7 @@ func TestApplySuppressions_UnreadableRecDetailsMatchEveryRow(t *testing.T) {
 	assert.Equal(t, 3, out[0].Count)
 }
 
-// TestApplySuppressions_AWSLegacySlugSpellings covers issues #1412 and #1420: AWS
+// TestApplySuppressions_AWSLegacySlugSpellings covers issues #1412, #1420 and #1422: AWS
 // recs and suppressions can be stored under a legacy or canonical slug, and a
 // suppression hides the rec whichever spelling each uses. Azure and GCP
 // "compute" stay distinct from "ec2".
@@ -381,6 +381,11 @@ func TestApplySuppressions_AWSLegacySlugSpellings(t *testing.T) {
 		{"aws legacy redshift sup hides data-warehouse rec", "aws", "data-warehouse", "redshift", 2},
 		{"aws data-warehouse sup hides legacy redshift rec", "aws", "redshift", "data-warehouse", 2},
 		{"aws memorydb sup does not hide cache rec", "aws", "cache", "memorydb", 5},
+		{"aws dash-free compute SP sup hides compute SP rec", "aws", "savings-plans-compute", "savingsplans-compute", 2},
+		{"aws compute SP sup hides dash-free compute SP rec", "aws", "savingsplans-compute", "savings-plans-compute", 2},
+		{"aws dash-free database SP sup hides database SP rec", "aws", "savings-plans-database", "savingsplans-database", 2},
+		{"aws umbrella SP sup does not hide compute SP rec", "aws", "savings-plans-compute", "savingsplans", 5},
+		{"aws compute SP sup does not hide sagemaker SP rec", "aws", "savings-plans-sagemaker", "savingsplans-compute", 5},
 		{"azure legacy rds sup does not hide relational-db rec", "azure", "relational-db", "rds", 5},
 		{"azure compute sup hides compute rec", "azure", "compute", "compute", 2},
 		{"azure ec2 sup does not hide compute rec", "azure", "compute", "ec2", 5},