diff --git a/providers/aws/recommendations/parser_services.go b/providers/aws/recommendations/parser_services.go index 1ad78486..7d0be7b4 100644 --- a/providers/aws/recommendations/parser_services.go +++ b/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 } diff --git a/providers/aws/recommendations/parser_services_test.go b/providers/aws/recommendations/parser_services_test.go index 81fdbf47..f155b5bb 100644 --- a/providers/aws/recommendations/parser_services_test.go +++ b/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) + }) }