From 848485b6ed935c2b7c095f0729fe6c4dc4a580af Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Tue, 9 Jun 2026 07:16:21 -0700 Subject: [PATCH] fix(aws/recommendations): case-insensitive EC2 tenancy so AWS RI recs are not dropped Cost Explorer returns the EC2 RI tenancy title-cased ("Shared", "Dedicated", "Host"), but resolveEC2Tenancy compared it case-sensitively against lowercase "shared"/"dedicated". Every real recommendation hit the default branch and errored; parseRecommendations logged a warning and continue'd, silently dropping all EC2 RI recommendations. With Savings Plans collection separately blocked by an IAM AccessDenied, AWS produced zero recommendations and the dashboard showed no AWS opportunities. Normalise the input with strings.ToLower(strings.TrimSpace(...)) before matching, mirroring the existing whitespace-tolerant handling on the RDS DeploymentOption path. The fail-loud M5 behaviour is preserved: "host" (Dedicated Hosts, no RI product) and any unknown value still error rather than collapse to a default-tenancy buy. Regression test: replicates the production casing ("Shared"/"Dedicated" plus surrounding whitespace) which fails on the pre-fix parser, and asserts title-cased "Host" still errors. --- .../aws/recommendations/parser_services.go | 13 ++- .../recommendations/parser_services_test.go | 82 +++++++++++++++++++ 2 files changed, 93 insertions(+), 2 deletions(-) diff --git a/providers/aws/recommendations/parser_services.go b/providers/aws/recommendations/parser_services.go index 4bc95a5f4..ce27eff78 100644 --- a/providers/aws/recommendations/parser_services.go +++ b/providers/aws/recommendations/parser_services.go @@ -82,15 +82,24 @@ func (c *Client) parseElastiCacheDetails(_ context.Context, rec *common.Recommen // directly. A nil pointer (CE omitted the field) also maps to default, because // CE only populates the field when it is non-default. // +// Matching is case- and whitespace-insensitive: Cost Explorer returns the +// value title-cased ("Shared", "Dedicated", "Host") even though older docs +// imply lowercase, so a case-sensitive compare silently rejected every real +// EC2 RI recommendation and the caller dropped it (no AWS opportunities on the +// dashboard). Normalising the input keeps the parser robust to AWS varying the +// casing without re-introducing a silent fallback. +// // Unknown tenancy values (e.g. "host" for Dedicated Hosts, which have no // corresponding RI product) return an error so the caller fails loud rather // than silently querying for and buying a default-tenancy RI on behalf of a // workload that requires a different tenancy class (M5 fix). func resolveEC2Tenancy(tenancy *string) (string, error) { - if tenancy == nil || *tenancy == "shared" { + if tenancy == nil { return string(ec2types.TenancyDefault), nil } - switch *tenancy { + switch strings.ToLower(strings.TrimSpace(*tenancy)) { + case "shared": + return string(ec2types.TenancyDefault), nil case "dedicated": return string(ec2types.TenancyDedicated), nil default: diff --git a/providers/aws/recommendations/parser_services_test.go b/providers/aws/recommendations/parser_services_test.go index 63003f287..750bbc8f3 100644 --- a/providers/aws/recommendations/parser_services_test.go +++ b/providers/aws/recommendations/parser_services_test.go @@ -344,6 +344,88 @@ func TestParseEC2Details(t *testing.T) { assert.Equal(t, "default", ec2Details.Tenancy) }, }, + { + // Regression for "AWS opportunities missing": Cost Explorer + // returns Tenancy title-cased as "Shared" (not lowercase + // "shared"). The pre-fix case-sensitive compare rejected it, + // parseRecommendations swallowed the error and continue'd, and + // every EC2 RI recommendation was dropped -> no AWS recs on the + // dashboard. This is the exact casing observed in production logs. + name: "EC2 CE title-cased Shared tenancy maps to default", + details: &types.ReservationPurchaseRecommendationDetail{ + InstanceDetails: &types.InstanceDetails{ + EC2InstanceDetails: &types.EC2InstanceDetails{ + InstanceType: aws.String("m5.large"), + Platform: aws.String("Linux/UNIX"), + Region: aws.String("US East (N. Virginia)"), + Tenancy: aws.String("Shared"), + }, + }, + }, + expectError: false, + validate: func(t *testing.T, rec *common.Recommendation) { + ec2Details, ok := rec.Details.(*common.ComputeDetails) + require.True(t, ok) + assert.Equal(t, "default", ec2Details.Tenancy) + }, + }, + { + // Same fix for the dedicated case: CE title-cases "Dedicated". + name: "EC2 CE title-cased Dedicated tenancy maps to dedicated", + details: &types.ReservationPurchaseRecommendationDetail{ + InstanceDetails: &types.InstanceDetails{ + EC2InstanceDetails: &types.EC2InstanceDetails{ + InstanceType: aws.String("r5.xlarge"), + Platform: aws.String("Windows"), + Region: aws.String("eu-central-1"), + Tenancy: aws.String("Dedicated"), + }, + }, + }, + expectError: false, + validate: func(t *testing.T, rec *common.Recommendation) { + ec2Details, ok := rec.Details.(*common.ComputeDetails) + require.True(t, ok) + assert.Equal(t, "dedicated", ec2Details.Tenancy) + }, + }, + { + // Leading/trailing whitespace must not defeat the mapping either. + name: "EC2 tenancy with surrounding whitespace maps to default", + details: &types.ReservationPurchaseRecommendationDetail{ + InstanceDetails: &types.InstanceDetails{ + EC2InstanceDetails: &types.EC2InstanceDetails{ + InstanceType: aws.String("m5.large"), + Platform: aws.String("Linux/UNIX"), + Region: aws.String("us-east-1"), + Tenancy: aws.String(" Shared "), + }, + }, + }, + expectError: false, + validate: func(t *testing.T, rec *common.Recommendation) { + ec2Details, ok := rec.Details.(*common.ComputeDetails) + require.True(t, ok) + assert.Equal(t, "default", ec2Details.Tenancy) + }, + }, + { + // M5 must still hold under the case-insensitive compare: title-cased + // "Host" (Dedicated Hosts, no RI product) must still error rather + // than collapse to default. + name: "EC2 title-cased Host tenancy still errors", + details: &types.ReservationPurchaseRecommendationDetail{ + InstanceDetails: &types.InstanceDetails{ + EC2InstanceDetails: &types.EC2InstanceDetails{ + InstanceType: aws.String("m5.large"), + Platform: aws.String("Linux/UNIX"), + Region: aws.String("us-east-1"), + Tenancy: aws.String("Host"), + }, + }, + }, + expectError: true, + }, { name: "Missing EC2 instance details", details: &types.ReservationPurchaseRecommendationDetail{