Fix AWS RI purchase: details assertion + reservation ID rules

pkg/common/identifiers.go

@@ -0,0 +1,31 @@
+package common
+
+import (
+	"strconv"
+	"strings"
+	"time"
+)
+
+// SanitizeReservationID returns an identifier safe for AWS reservation/reserved-instance
+// ID or name fields: only ASCII letters, digits, and hyphens; no leading/trailing
+// hyphen; no consecutive hyphens. Dots are replaced with hyphens. If the result
+// would be empty, returns fallbackPrefix plus a Unix timestamp.
+func SanitizeReservationID(id, fallbackPrefix string) string {
+	var b strings.Builder
+	for _, r := range id {
+		if (r >= 'a' && r <= 'z') || (r >= 'A' && r <= 'Z') || (r >= '0' && r <= '9') || r == '-' {
+			b.WriteRune(r)
+		} else if r == '.' {
+			b.WriteRune('-')
+		}
+	}
+	s := b.String()
+	for strings.Contains(s, "--") {
+		s = strings.ReplaceAll(s, "--", "-")
+	}
+	s = strings.Trim(s, "-")
+	if s == "" {
+		s = fallbackPrefix + strconv.FormatInt(time.Now().Unix(), 10)
+	}
+	return s
+}

providers/aws/recommendations/client.go

@@ -322,7 +322,14 @@ func (c *Client) parseOpenSearchDetails(rec *common.Recommendation, details *typ
 	osInfo := &common.SearchDetails{}
 
 	if esDetails.InstanceClass != nil && esDetails.InstanceSize != nil {
-		rec.ResourceType = fmt.Sprintf("%s.%s", *esDetails.InstanceClass, *esDetails.InstanceSize)
+		instanceSize := *esDetails.InstanceSize
+		// Cost Explorer may return InstanceSize as the full type (e.g. "t3.medium.search");
+		// concatenating with InstanceClass would duplicate (e.g. "t3.medium.t3.medium.search").
+		if strings.HasSuffix(instanceSize, ".search") {
+			rec.ResourceType = instanceSize
+		} else {
+			rec.ResourceType = fmt.Sprintf("%s.%s.search", *esDetails.InstanceClass, instanceSize)
+		}
 		osInfo.InstanceType = rec.ResourceType
 	}
 	if esDetails.Region != nil {

providers/aws/services/ec2/client.go

@@ -150,8 +150,8 @@ func (c *Client) PurchaseCommitment(ctx context.Context, rec common.Recommendati
 
 // findOfferingID finds the appropriate EC2 Reserved Instance offering ID
 func (c *Client) findOfferingID(ctx context.Context, rec common.Recommendation) (string, error) {
-	details, ok := rec.Details.(common.ComputeDetails)
-	if !ok {
+	details, ok := rec.Details.(*common.ComputeDetails)
+	if !ok || details == nil {
 		return "", fmt.Errorf("invalid service details for EC2")
 	}
 

providers/aws/services/ec2/client_test.go

@@ -264,7 +264,7 @@ func TestClient_ValidateOffering(t *testing.T) {
 		ResourceType:  "t3.micro",
 		PaymentOption: "partial-upfront",
 		Term:          "3yr",
-		Details: common.ComputeDetails{
+		Details: &common.ComputeDetails{
 			Platform: "Linux/UNIX",
 			Tenancy:  "default",
 			Scope:    "Region",
@@ -303,7 +303,7 @@ func TestClient_PurchaseCommitment(t *testing.T) {
 		Count:         2,
 		PaymentOption: "partial-upfront",
 		Term:          "3yr",
-		Details: common.ComputeDetails{
+		Details: &common.ComputeDetails{
 			Platform: "Linux/UNIX",
 			Tenancy:  "default",
 			Scope:    "Region",
@@ -351,7 +351,7 @@ func TestClient_GetOfferingDetails(t *testing.T) {
 		PaymentOption: "partial-upfront",
 		Term:          "3yr",
 		Count:         1,
-		Details: common.ComputeDetails{
+		Details: &common.ComputeDetails{
 			Platform: "Linux/UNIX",
 			Tenancy:  "default",
 			Scope:    "Region",

providers/aws/services/elasticache/client.go

@@ -123,7 +123,7 @@ func (c *Client) PurchaseCommitment(ctx context.Context, rec common.Recommendati
 		return result, result.Error
 	}
 
-	reservationID := fmt.Sprintf("elasticache-%s-%d", rec.ResourceType, time.Now().Unix())
+	reservationID := common.SanitizeReservationID(fmt.Sprintf("elasticache-%s-%d", rec.ResourceType, time.Now().Unix()), "elasticache-reserved-")
 
 	input := &elasticache.PurchaseReservedCacheNodesOfferingInput{
 		ReservedCacheNodesOfferingId: aws.String(offeringID),
@@ -154,8 +154,8 @@ func (c *Client) PurchaseCommitment(ctx context.Context, rec common.Recommendati
 
 // findOfferingID finds the appropriate Reserved Cache Node offering ID
 func (c *Client) findOfferingID(ctx context.Context, rec common.Recommendation) (string, error) {
-	details, ok := rec.Details.(common.CacheDetails)
-	if !ok {
+	details, ok := rec.Details.(*common.CacheDetails)
+	if !ok || details == nil {
 		return "", fmt.Errorf("invalid service details for ElastiCache")
 	}
 

providers/aws/services/elasticache/client_test.go

@@ -246,7 +246,7 @@ func TestClient_ValidateOffering(t *testing.T) {
 		ResourceType:  "cache.r6g.large",
 		PaymentOption: "no-upfront",
 		Term:          "3yr",
-		Details: common.CacheDetails{
+		Details: &common.CacheDetails{
 			Engine:   "redis",
 			NodeType: "cache.r6g.large",
 		},
@@ -283,7 +283,7 @@ func TestClient_PurchaseCommitment(t *testing.T) {
 		Count:         3,
 		PaymentOption: "partial-upfront",
 		Term:          "3yr",
-		Details: common.CacheDetails{
+		Details: &common.CacheDetails{
 			Engine:   "redis",
 			NodeType: "cache.m6g.xlarge",
 		},
@@ -336,7 +336,7 @@ func TestClient_GetOfferingDetails(t *testing.T) {
 		ResourceType:  "cache.r6g.xlarge",
 		PaymentOption: "all-upfront",
 		Term:          "1yr",
-		Details: common.CacheDetails{
+		Details: &common.CacheDetails{
 			Engine:   "redis",
 			NodeType: "cache.r6g.xlarge",
 		},

providers/aws/services/memorydb/client.go

@@ -118,7 +118,7 @@ func (c *Client) PurchaseCommitment(ctx context.Context, rec common.Recommendati
 		return result, result.Error
 	}
 
-	reservationID := fmt.Sprintf("memorydb-%s-%d", rec.ResourceType, time.Now().Unix())
+	reservationID := common.SanitizeReservationID(fmt.Sprintf("memorydb-%s-%d", rec.ResourceType, time.Now().Unix()), "memorydb-reserved-")
 
 	input := &memorydb.PurchaseReservedNodesOfferingInput{
 		ReservedNodesOfferingId: aws.String(offeringID),

providers/aws/services/opensearch/client.go

@@ -118,7 +118,7 @@ func (c *Client) PurchaseCommitment(ctx context.Context, rec common.Recommendati
 		return result, result.Error
 	}
 
-	reservationID := fmt.Sprintf("opensearch-%s-%d", rec.ResourceType, time.Now().Unix())
+	reservationID := common.SanitizeReservationID(fmt.Sprintf("opensearch-%s-%d", rec.ResourceType, time.Now().Unix()), "opensearch-reserved-")
 
 	input := &opensearch.PurchaseReservedInstanceOfferingInput{
 		ReservedInstanceOfferingId: aws.String(offeringID),
@@ -145,22 +145,31 @@ func (c *Client) PurchaseCommitment(ctx context.Context, rec common.Recommendati
 
 // findOfferingID finds the appropriate Reserved Instance offering ID
 func (c *Client) findOfferingID(ctx context.Context, rec common.Recommendation) (string, error) {
-	input := &opensearch.DescribeReservedInstanceOfferingsInput{
-		MaxResults: 100,
-	}
+	var nextToken *string
+	for {
+		input := &opensearch.DescribeReservedInstanceOfferingsInput{
+			MaxResults: 100,
+			NextToken:  nextToken,
+		}
 
-	result, err := c.client.DescribeReservedInstanceOfferings(ctx, input)
-	if err != nil {
-		return "", fmt.Errorf("failed to describe offerings: %w", err)
-	}
+		result, err := c.client.DescribeReservedInstanceOfferings(ctx, input)
+		if err != nil {
+			return "", fmt.Errorf("failed to describe offerings: %w", err)
+		}
 
-	for _, offering := range result.ReservedInstanceOfferings {
-		if string(offering.InstanceType) == rec.ResourceType {
-			if c.matchesPaymentOption(offering.PaymentOption, rec.PaymentOption) &&
-				c.matchesDuration(offering.Duration, rec.Term) {
-				return aws.ToString(offering.ReservedInstanceOfferingId), nil
+		for _, offering := range result.ReservedInstanceOfferings {
+			if string(offering.InstanceType) == rec.ResourceType {
+				if c.matchesPaymentOption(offering.PaymentOption, rec.PaymentOption) &&
+					c.matchesDuration(offering.Duration, rec.Term) {
+					return aws.ToString(offering.ReservedInstanceOfferingId), nil
+				}
 			}
 		}
+
+		if result.NextToken == nil || aws.ToString(result.NextToken) == "" {
+			break
+		}
+		nextToken = result.NextToken
 	}
 
 	return "", fmt.Errorf("no offerings found for %s", rec.ResourceType)

providers/aws/services/rds/client.go

@@ -127,8 +127,8 @@ func (c *Client) PurchaseCommitment(ctx context.Context, rec common.Recommendati
 		return result, result.Error
 	}
 
-	// Generate reservation ID
-	reservationID := fmt.Sprintf("rds-%s-%d", rec.ResourceType, time.Now().Unix())
+	// Generate reservation ID (letters, digits, hyphens only; no leading/trailing/double hyphen)
+	reservationID := common.SanitizeReservationID(fmt.Sprintf("rds-%s-%d", rec.ResourceType, time.Now().Unix()), "rds-reserved-")
 
 	// Create the purchase request
 	input := &rds.PurchaseReservedDBInstancesOfferingInput{
@@ -160,8 +160,8 @@ func (c *Client) PurchaseCommitment(ctx context.Context, rec common.Recommendati
 
 // findOfferingID finds the appropriate RDS Reserved Instance offering ID
 func (c *Client) findOfferingID(ctx context.Context, rec common.Recommendation) (string, error) {
-	details, ok := rec.Details.(common.DatabaseDetails)
-	if !ok {
+	details, ok := rec.Details.(*common.DatabaseDetails)
+	if !ok || details == nil {
 		return "", fmt.Errorf("invalid service details for RDS")
 	}
 

providers/aws/services/rds/client_test.go

@@ -274,7 +274,7 @@ func TestClient_ValidateOffering(t *testing.T) {
 		ResourceType:  "db.t3.medium",
 		PaymentOption: "no-upfront",
 		Term:          "3yr",
-		Details: common.DatabaseDetails{
+		Details: &common.DatabaseDetails{
 			Engine:   "mysql",
 			AZConfig: "multi-az",
 		},
@@ -311,7 +311,7 @@ func TestClient_ValidateOffering_NotFound(t *testing.T) {
 		ResourceType:  "db.t3.medium",
 		PaymentOption: "no-upfront",
 		Term:          "3yr",
-		Details: common.DatabaseDetails{
+		Details: &common.DatabaseDetails{
 			Engine:   "mysql",
 			AZConfig: "multi-az",
 		},
@@ -341,7 +341,7 @@ func TestClient_PurchaseCommitment(t *testing.T) {
 		Count:         2,
 		PaymentOption: "partial-upfront",
 		Term:          "3yr",
-		Details: common.DatabaseDetails{
+		Details: &common.DatabaseDetails{
 			Engine:   "aurora-mysql",
 			AZConfig: "multi-az",
 		},
@@ -396,7 +396,7 @@ func TestClient_PurchaseCommitment_EmptyResponse(t *testing.T) {
 		Count:         1,
 		PaymentOption: "all-upfront",
 		Term:          "1yr",
-		Details: common.DatabaseDetails{
+		Details: &common.DatabaseDetails{
 			Engine:   "mysql",
 			AZConfig: "single-az",
 		},
@@ -441,7 +441,7 @@ func TestClient_GetOfferingDetails(t *testing.T) {
 		ResourceType:  "db.m6g.large",
 		PaymentOption: "all-upfront",
 		Term:          "1yr",
-		Details: common.DatabaseDetails{
+		Details: &common.DatabaseDetails{
 			Engine:   "postgres",
 			AZConfig: "multi-az",
 		},