fix(recommendations): wire MemoryDB NodeType from CE InstanceDetails (follow-up to #539, closes #378)

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
 }

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)