diff --git a/providers/aws/recommendations/parser_services.go b/providers/aws/recommendations/parser_services.go index cb456c58e..ed4591cd2 100644 --- a/providers/aws/recommendations/parser_services.go +++ b/providers/aws/recommendations/parser_services.go @@ -191,22 +191,31 @@ func (c *Client) parseRedshiftDetails(_ context.Context, rec *common.Recommendat // parseMemoryDBDetails extracts MemoryDB-specific details. // -// Cost Explorer does not populate InstanceDetails for MemoryDB reserved nodes: -// the MemoryDB-specific sub-struct does not exist in the AWS SDK's -// ReservationPurchaseRecommendationDetail, so the instance type must come from -// rec.ResourceType, which the upstream caller should have populated from the -// recommendation summary fields before dispatching here. +// Cost Explorer exposes MemoryDB reserved-node offerings via +// details.InstanceDetails.MemoryDBInstanceDetails (NodeType, Region), the same +// shape ElastiCache and Redshift use. The node type comes from that struct, not +// from a hardcoded default, so we never substitute a wrong instance type. // -// If rec.ResourceType is empty, the function returns an error so the -// recommendation is skipped loudly (logged by parseRecommendations) rather -// than silently substituting a wrong default instance type. -func (c *Client) parseMemoryDBDetails(_ context.Context, rec *common.Recommendation, _ *types.ReservationPurchaseRecommendationDetail) error { - if rec.ResourceType == "" { - return fmt.Errorf("MemoryDB recommendation has no ResourceType; cannot determine offering - Cost Explorer did not populate instance details") +// If the MemoryDB sub-struct or its NodeType is absent, the function returns an +// error so the recommendation is skipped loudly (logged by parseRecommendations) +// rather than silently guessing an offering. +func (c *Client) parseMemoryDBDetails(_ context.Context, rec *common.Recommendation, details *types.ReservationPurchaseRecommendationDetail) error { + if details == nil || details.InstanceDetails == nil || details.InstanceDetails.MemoryDBInstanceDetails == nil { + return fmt.Errorf("MemoryDB instance details not found; cannot determine offering - Cost Explorer did not populate instance details") + } + + mdbDetails := details.InstanceDetails.MemoryDBInstanceDetails + if mdbDetails.NodeType == nil || *mdbDetails.NodeType == "" { + return fmt.Errorf("MemoryDB recommendation has no NodeType; cannot determine offering") + } + + rec.ResourceType = *mdbDetails.NodeType + if mdbDetails.Region != nil { + rec.Region = normalizeRegionName(*mdbDetails.Region) } rec.Details = &common.CacheDetails{ Engine: "redis", - NodeType: rec.ResourceType, + NodeType: *mdbDetails.NodeType, } return nil } diff --git a/providers/aws/recommendations/parser_services_test.go b/providers/aws/recommendations/parser_services_test.go index e01c2bd94..b70e14b31 100644 --- a/providers/aws/recommendations/parser_services_test.go +++ b/providers/aws/recommendations/parser_services_test.go @@ -508,30 +508,63 @@ func TestParseRedshiftDetails(t *testing.T) { func TestParseMemoryDBDetails(t *testing.T) { client := &Client{} - details := &types.ReservationPurchaseRecommendationDetail{} - t.Run("empty ResourceType returns error", func(t *testing.T) { + t.Run("missing MemoryDB instance details returns error", func(t *testing.T) { rec := &common.Recommendation{} + err := client.parseMemoryDBDetails(context.Background(), rec, &types.ReservationPurchaseRecommendationDetail{}) + require.Error(t, err, "should fail loudly when MemoryDB instance details are absent") + assert.Contains(t, err.Error(), "instance details not found") + assert.Nil(t, rec.Details, "Details should remain nil on error") + assert.Empty(t, rec.ResourceType, "ResourceType should remain empty on error") + }) + + t.Run("nil NodeType returns error", func(t *testing.T) { + rec := &common.Recommendation{} + details := &types.ReservationPurchaseRecommendationDetail{ + InstanceDetails: &types.InstanceDetails{ + MemoryDBInstanceDetails: &types.MemoryDBInstanceDetails{ + Region: aws.String("US East (N. Virginia)"), + }, + }, + } err := client.parseMemoryDBDetails(context.Background(), rec, details) - require.Error(t, err, "should fail loudly when ResourceType is empty") - assert.Contains(t, err.Error(), "ResourceType") + require.Error(t, err, "should fail loudly when NodeType is absent") + assert.Contains(t, err.Error(), "NodeType") assert.Nil(t, rec.Details, "Details should remain nil on error") }) - t.Run("non-default instance type populates Details", func(t *testing.T) { - rec := &common.Recommendation{ResourceType: "db.r6g.large"} + t.Run("populates Details, ResourceType and Region from CE node type", func(t *testing.T) { + rec := &common.Recommendation{} + details := &types.ReservationPurchaseRecommendationDetail{ + InstanceDetails: &types.InstanceDetails{ + MemoryDBInstanceDetails: &types.MemoryDBInstanceDetails{ + NodeType: aws.String("db.r6g.large"), + Region: aws.String("US East (N. Virginia)"), + }, + }, + } err := client.parseMemoryDBDetails(context.Background(), rec, details) require.NoError(t, err) + assert.Equal(t, "db.r6g.large", rec.ResourceType) + assert.Equal(t, "us-east-1", rec.Region) cacheDetails, ok := rec.Details.(*common.CacheDetails) require.True(t, ok, "Details should be *common.CacheDetails") assert.Equal(t, "db.r6g.large", cacheDetails.NodeType) assert.Equal(t, "redis", cacheDetails.Engine) }) - t.Run("xlarge instance type populates Details", func(t *testing.T) { - rec := &common.Recommendation{ResourceType: "db.r6gd.xlarge"} + t.Run("xlarge node type populates Details", func(t *testing.T) { + rec := &common.Recommendation{} + details := &types.ReservationPurchaseRecommendationDetail{ + InstanceDetails: &types.InstanceDetails{ + MemoryDBInstanceDetails: &types.MemoryDBInstanceDetails{ + NodeType: aws.String("db.r6gd.xlarge"), + }, + }, + } err := client.parseMemoryDBDetails(context.Background(), rec, details) require.NoError(t, err) + assert.Equal(t, "db.r6gd.xlarge", rec.ResourceType) cacheDetails, ok := rec.Details.(*common.CacheDetails) require.True(t, ok, "Details should be *common.CacheDetails") assert.Equal(t, "db.r6gd.xlarge", cacheDetails.NodeType)