Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 39 additions & 6 deletions internal/config/suppressions_build_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -205,12 +205,45 @@ func TestBuildSuppressions_ComputeSlugEC2(t *testing.T) {
}

func TestCanonicalSuppressionService(t *testing.T) {
assert.Equal(t, "compute", CanonicalSuppressionService("aws", "ec2"))
assert.Equal(t, "compute", CanonicalSuppressionService("aws", "compute"))
assert.Equal(t, "rds", CanonicalSuppressionService("aws", "rds"))
assert.Equal(t, "compute", CanonicalSuppressionService("azure", "compute"))
assert.Equal(t, "ec2", CanonicalSuppressionService("azure", "ec2"))
assert.Equal(t, "compute", CanonicalSuppressionService("gcp", "compute"))
cases := []struct{ provider, service, want string }{
{"aws", "ec2", "compute"},
{"aws", "rds", "relational-db"},
{"aws", "elasticache", "cache"},
{"aws", "opensearch", "search"},
{"aws", "redshift", "data-warehouse"},
{"aws", "compute", "compute"},
{"aws", "relational-db", "relational-db"},
{"aws", "cache", "cache"},
{"aws", "search", "search"},
{"aws", "data-warehouse", "data-warehouse"},
{"aws", "memorydb", "memorydb"},
{"azure", "compute", "compute"},
{"azure", "ec2", "ec2"},
{"azure", "rds", "rds"},
{"gcp", "compute", "compute"},
{"gcp", "redshift", "redshift"},
{"gcp", "elasticache", "elasticache"},
}
for _, tc := range cases {
t.Run(tc.provider+"/"+tc.service, func(t *testing.T) {
got := CanonicalSuppressionService(tc.provider, tc.service)
assert.Equal(t, tc.want, got)
assert.Equal(t, got, CanonicalSuppressionService(tc.provider, got), "idempotent")
})
}
}

// Issue #1420: a legacy "rds" rec and a canonical "relational-db" rec for the
// same tuple merge into one canonical suppression row with the summed count.
func TestBuildSuppressions_MergesRDSAliases(t *testing.T) {
gp := &GlobalConfig{GracePeriodDays: map[string]int{"aws": 7}}
sups := BuildSuppressions([]RecommendationRecord{
{Provider: "aws", Service: "rds", Region: "r", ResourceType: "db.r5.large", Count: 3},
{Provider: "aws", Service: "relational-db", Region: "r", ResourceType: "db.r5.large", Count: 2},
}, "exec-1", gp, time.Now())
require.Len(t, sups, 1)
assert.Equal(t, "relational-db", sups[0].Service)
assert.Equal(t, 5, sups[0].SuppressedCount)
}

// Issue #1412: AWS "ec2" and "compute" recs in one request share a bucket and
Expand Down
29 changes: 22 additions & 7 deletions internal/config/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -760,15 +760,30 @@ type SuppressionVariant struct {
Tenancy string `json:"tenancy"`
}

// CanonicalSuppressionService maps the legacy AWS EC2 slug "ec2" to the
// canonical "compute" slug so a suppression and a rec match whichever spelling
// each was stored under (issue #1412). 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
// 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.
var awsLegacyServiceSlugs = map[string]string{
"ec2": "compute",
"rds": "relational-db",
"elasticache": "cache",
"opensearch": "search",
"redshift": "data-warehouse",
}

// 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.
func CanonicalSuppressionService(provider, service string) string {
if provider == "aws" && service == "ec2" {
return "compute"
if provider != "aws" {
return service
}
if canonical, ok := awsLegacyServiceSlugs[service]; ok {
return canonical
}
return service
}
Expand Down
3 changes: 2 additions & 1 deletion internal/purchase/execution.go
Original file line number Diff line number Diff line change
Expand Up @@ -1299,7 +1299,8 @@ func mapServiceSlug(service string) (common.ServiceType, bool) {
"cache": common.ServiceCache,
"search": common.ServiceSearch,
"data-warehouse": common.ServiceDataWarehouse,
// Legacy AWS-only slugs.
// 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,
Expand Down
16 changes: 13 additions & 3 deletions internal/scheduler/scheduler_suppressions_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -359,11 +359,11 @@ func TestApplySuppressions_UnreadableRecDetailsMatchEveryRow(t *testing.T) {
assert.Equal(t, 3, out[0].Count)
}

// TestApplySuppressions_EC2SlugSpellings is the issue #1412 regression: AWS
// EC2 recs and suppressions can be stored as "ec2" or "compute", and a
// TestApplySuppressions_AWSLegacySlugSpellings covers issues #1412 and #1420: 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".
func TestApplySuppressions_EC2SlugSpellings(t *testing.T) {
func TestApplySuppressions_AWSLegacySlugSpellings(t *testing.T) {
cases := []struct {
name, provider, recService, supService string
wantCount int
Expand All @@ -372,6 +372,16 @@ func TestApplySuppressions_EC2SlugSpellings(t *testing.T) {
{"aws ec2 sup hides compute rec", "aws", "compute", "ec2", 2},
{"aws same slug compute", "aws", "compute", "compute", 2},
{"aws unrelated service not hidden", "aws", "ec2", "rds", 5},
{"aws legacy rds sup hides relational-db rec", "aws", "relational-db", "rds", 2},
{"aws relational-db sup hides legacy rds rec", "aws", "rds", "relational-db", 2},
{"aws legacy elasticache sup hides cache rec", "aws", "cache", "elasticache", 2},
{"aws cache sup hides legacy elasticache rec", "aws", "elasticache", "cache", 2},
{"aws legacy opensearch sup hides search rec", "aws", "search", "opensearch", 2},
{"aws search sup hides legacy opensearch rec", "aws", "opensearch", "search", 2},
{"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},
{"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},
{"gcp compute sup does not hide ec2 rec", "gcp", "ec2", "compute", 5},
Expand Down
Loading