fix(recommendations): parseMemoryDBDetails uses rec fields, errors loudly on empty (closes #378)

providers/aws/recommendations/parser_services.go

@@ -2,7 +2,6 @@ package recommendations
 
 import (
 	"fmt"
-	"log"
 	"strings"
 
 	"github.com/aws/aws-sdk-go-v2/service/costexplorer/types"
@@ -156,8 +155,24 @@ func (c *Client) parseRedshiftDetails(rec *common.Recommendation, details *types
 	return nil
 }
 
-// parseMemoryDBDetails extracts MemoryDB-specific details
-func (c *Client) parseMemoryDBDetails(rec *common.Recommendation, details *types.ReservationPurchaseRecommendationDetail) error {
-	log.Printf("WARNING: MemoryDB does not provide instance details in Cost Explorer, skipping recommendation")
+// 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.
+//
+// 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(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")
+	}
+	rec.Details = &common.CacheDetails{
+		Engine:   "redis",
+		NodeType: rec.ResourceType,
+	}
 	return nil
 }

providers/aws/recommendations/parser_services_test.go

@@ -497,15 +497,33 @@ func TestParseRedshiftDetails(t *testing.T) {
 
 func TestParseMemoryDBDetails(t *testing.T) {
 	client := &Client{}
-
-	rec := &common.Recommendation{}
 	details := &types.ReservationPurchaseRecommendationDetail{}
 
-	err := client.parseMemoryDBDetails(rec, details)
-
-	// parseMemoryDBDetails logs a warning and skips the recommendation
-	// (MemoryDB does not expose instance details in Cost Explorer)
-	require.NoError(t, err)
-	assert.Empty(t, rec.ResourceType, "ResourceType should not be set when MemoryDB details are unavailable")
-	assert.Nil(t, rec.Details, "Details should remain nil when MemoryDB details are unavailable")
+	t.Run("empty ResourceType returns error", func(t *testing.T) {
+		rec := &common.Recommendation{}
+		err := client.parseMemoryDBDetails(rec, details)
+		require.Error(t, err, "should fail loudly when ResourceType is empty")
+		assert.Contains(t, err.Error(), "ResourceType")
+		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"}
+		err := client.parseMemoryDBDetails(rec, details)
+		require.NoError(t, err)
+		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"}
+		err := client.parseMemoryDBDetails(rec, details)
+		require.NoError(t, err)
+		cacheDetails, ok := rec.Details.(*common.CacheDetails)
+		require.True(t, ok, "Details should be *common.CacheDetails")
+		assert.Equal(t, "db.r6gd.xlarge", cacheDetails.NodeType)
+		assert.Equal(t, "redis", cacheDetails.Engine)
+	})
 }