feat(providers/aws): rich self-describing RI display names (closes #687)

providers/aws/services/ec2/client.go

@@ -230,10 +230,28 @@ func (c *Client) findRIByIdempotencyToken(ctx context.Context, token string) (st
 // since AWS sometimes needs a couple of seconds before the RI ID is visible
 // to CreateTags. Non-NotFound errors short-circuit immediately.
 func (c *Client) tagReservedInstance(ctx context.Context, riID string, rec common.Recommendation, source, idempotencyToken string) error {
+	// EC2 PurchaseReservedInstancesOfferingInput has no customer-supplied name or
+	// ID field. A Name tag (issue #687) is the only way to make the RI
+	// self-describing in the AWS console without cross-referencing CUDly's DB.
+	// BuildReservationName produces the same rich {svc}-{region}-{sku}-{count}x-
+	// {term}-{paymt}-{ts}-{rand} format used by the other AWS service clients.
+	displayName := common.BuildReservationName(common.ReservationNameFields{
+		Service:      "ec2",
+		Region:       rec.Region,
+		ResourceType: rec.ResourceType,
+		Count:        rec.Count,
+		Term:         rec.Term,
+		Payment:      rec.PaymentOption,
+		Now:          time.Now(),
+	}, "ec2-reserved-")
 	tags := []types.Tag{
+		{Key: aws.String("Name"), Value: aws.String(displayName)},
 		{Key: aws.String("Purpose"), Value: aws.String("Reserved Instance Purchase")},
 		{Key: aws.String("ResourceType"), Value: aws.String(rec.ResourceType)},
 		{Key: aws.String("Region"), Value: aws.String(rec.Region)},
+		{Key: aws.String("Count"), Value: aws.String(fmt.Sprintf("%d", rec.Count))},
+		{Key: aws.String("Term"), Value: aws.String(rec.Term)},
+		{Key: aws.String("PaymentOption"), Value: aws.String(rec.PaymentOption)},
 		{Key: aws.String("PurchaseDate"), Value: aws.String(time.Now().Format("2006-01-02"))},
 		{Key: aws.String("Tool"), Value: aws.String("CUDly")},
 	}

providers/aws/services/ec2/client_test.go

@@ -669,3 +669,109 @@ func TestFindOfferingID_HappyPath(t *testing.T) {
 	assert.NoError(t, err)
 	assert.Equal(t, "offering-ok", id)
 }
+
+// TestClient_tagReservedInstance_NameTagPresent asserts that a purchase on the
+// no-token CLI path (issue #687) stamps a self-describing Name tag on the EC2
+// RI. EC2 PurchaseReservedInstancesOfferingInput has no customer-supplied name
+// field, so the Name tag is the only way to identify the reservation in the AWS
+// console without cross-referencing CUDly's purchase audit log.
+func TestClient_tagReservedInstance_NameTagPresent(t *testing.T) {
+	t.Parallel()
+	mockEC2 := &MockEC2Client{}
+	client := &Client{client: mockEC2, region: "us-west-2"}
+
+	rec := common.Recommendation{
+		Service:       common.ServiceCompute,
+		ResourceType:  "m5.xlarge",
+		Region:        "us-west-2",
+		Count:         3,
+		PaymentOption: "all-upfront",
+		Term:          "1yr",
+		Details:       &common.ComputeDetails{Platform: "Linux/UNIX", Tenancy: "default", Scope: "Region"},
+	}
+
+	var capturedTags []types.Tag
+	mockEC2.On("CreateTags", mock.Anything, mock.MatchedBy(func(in *ec2.CreateTagsInput) bool {
+		capturedTags = in.Tags
+		return len(in.Resources) == 1 && in.Resources[0] == "ri-name-test"
+	})).Return(&ec2.CreateTagsOutput{}, nil)
+
+	err := client.tagReservedInstance(context.Background(), "ri-name-test", rec, "", "")
+	assert.NoError(t, err)
+
+	tagMap := make(map[string]string, len(capturedTags))
+	for _, tag := range capturedTags {
+		tagMap[aws.ToString(tag.Key)] = aws.ToString(tag.Value)
+	}
+
+	name, ok := tagMap["Name"]
+	assert.True(t, ok, "Name tag must be present in CreateTags call")
+	assert.True(t, len(name) > 0, "Name tag must be non-empty")
+	assert.LessOrEqual(t, len(name), 60, "Name must fit the 60-char AWS reservation-name cap")
+	// Key segments that make the RI self-describing without CUDly:
+	assert.Contains(t, name, "ec2", "Name must start with the service code")
+	assert.Contains(t, name, "us-west-2", "Name must embed the region")
+	assert.Contains(t, name, "m5-xlarge", "Name must embed the SKU (dots->hyphens)")
+	assert.Contains(t, name, "3x", "Name must embed the count")
+	assert.Contains(t, name, "1yr", "Name must embed the term")
+
+	mockEC2.AssertExpectations(t)
+}
+
+// TestClient_PurchaseCommitment_NameTagInCreateTagsRequest asserts that an
+// end-to-end purchase on the no-token CLI path (issue #687) produces a
+// CreateTags call that includes a self-describing Name tag.
+func TestClient_PurchaseCommitment_NameTagInCreateTagsRequest(t *testing.T) {
+	t.Parallel()
+	mockEC2 := &MockEC2Client{}
+	client := &Client{client: mockEC2, region: "ap-southeast-1"}
+
+	rec := common.Recommendation{
+		Service:       common.ServiceCompute,
+		ResourceType:  "r6g.large",
+		Region:        "ap-southeast-1",
+		Count:         2,
+		PaymentOption: "no-upfront",
+		Term:          "3yr",
+		Details:       &common.ComputeDetails{Platform: "Linux/UNIX", Tenancy: "default", Scope: "Region"},
+	}
+
+	// No idempotency token -> skip DescribeReservedInstances guard
+	mockEC2.On("DescribeReservedInstancesOfferings", mock.Anything, mock.Anything).
+		Return(&ec2.DescribeReservedInstancesOfferingsOutput{
+			ReservedInstancesOfferings: []types.ReservedInstancesOffering{{
+				ReservedInstancesOfferingId: aws.String("off-name-e2e"),
+				InstanceType:                types.InstanceTypeR6gLarge,
+				Duration:                    aws.Int64(94608000),
+				OfferingType:                types.OfferingTypeValuesNoUpfront,
+				ProductDescription:          types.RIProductDescriptionLinuxUnix,
+				InstanceTenancy:             types.TenancyDefault,
+			}},
+		}, nil)
+
+	mockEC2.On("PurchaseReservedInstancesOffering", mock.Anything, mock.Anything).
+		Return(&ec2.PurchaseReservedInstancesOfferingOutput{
+			ReservedInstancesId: aws.String("ri-name-e2e"),
+		}, nil)
+
+	var capturedName string
+	mockEC2.On("CreateTags", mock.Anything, mock.MatchedBy(func(in *ec2.CreateTagsInput) bool {
+		for _, tag := range in.Tags {
+			if aws.ToString(tag.Key) == "Name" {
+				capturedName = aws.ToString(tag.Value)
+				return true
+			}
+		}
+		return false
+	})).Return(&ec2.CreateTagsOutput{}, nil)
+
+	result, err := client.PurchaseCommitment(context.Background(), rec, common.PurchaseOptions{})
+	assert.NoError(t, err)
+	assert.True(t, result.Success)
+
+	assert.True(t, len(capturedName) > 0, "Name tag must be set on the CreateTags call")
+	assert.Contains(t, capturedName, "ec2", "service code must appear in Name: %q", capturedName)
+	assert.Contains(t, capturedName, "ap-southeast-1", "region must appear in Name: %q", capturedName)
+
+	mockEC2.AssertExpectations(t)
+}

providers/aws/services/rds/client_test.go

@@ -733,7 +733,7 @@ func TestFindOfferingID_PaginationCapFires(t *testing.T) {
 			}, nil).Once()
 	}
 
-	_, err := client.findOfferingID(context.Background(), rec)
+	_, err := client.findOfferingID(context.Background(), rec, "")
 
 	if assert.Error(t, err) {
 		assert.Contains(t, err.Error(), "pagination cap reached")
@@ -763,7 +763,7 @@ func TestFindOfferingID_WrongVariantRejected(t *testing.T) {
 			},
 		}, nil).Once()
 
-	_, err := client.findOfferingID(context.Background(), rec)
+	_, err := client.findOfferingID(context.Background(), rec, "")
 
 	if assert.Error(t, err) {
 		assert.Contains(t, err.Error(), "payment option")
@@ -792,7 +792,7 @@ func TestFindOfferingID_HappyPath(t *testing.T) {
 			},
 		}, nil).Once()
 
-	id, err := client.findOfferingID(context.Background(), rec)
+	id, err := client.findOfferingID(context.Background(), rec, "")
 
 	assert.NoError(t, err)
 	assert.Equal(t, "offering-ok", id)

providers/aws/services/redshift/client.go

@@ -347,10 +347,20 @@ func (c *Client) tagReservedNode(ctx context.Context, nodeID string, rec common.
 	// does not accept a customer-supplied node ID, so the only way to make
 	// the reserved node identifiable from the AWS console alone (without
 	// cross-referencing CUDly) is to encode the same descriptors that the
-	// other AWS service clients embed in their reservation name. Term,
-	// PaymentOption, and Count join the pre-existing NodeType/Region/
-	// PurchaseDate set.
+	// other AWS service clients embed in their reservation name. The Name tag
+	// uses BuildReservationName to produce the same rich format so operators
+	// can identify the node without cross-referencing CUDly's purchase audit log.
+	displayName := common.BuildReservationName(common.ReservationNameFields{
+		Service:      "redshift",
+		Region:       rec.Region,
+		ResourceType: rec.ResourceType,
+		Count:        rec.Count,
+		Term:         rec.Term,
+		Payment:      rec.PaymentOption,
+		Now:          time.Now(),
+	}, "redshift-reserved-")
 	tags := []redshifttypes.Tag{
+		{Key: aws.String("Name"), Value: aws.String(displayName)},
 		{Key: aws.String("Purpose"), Value: aws.String("Reserved Node Purchase")},
 		{Key: aws.String("NodeType"), Value: aws.String(rec.ResourceType)},
 		{Key: aws.String("Region"), Value: aws.String(rec.Region)},

providers/aws/services/redshift/client_test.go

@@ -430,10 +430,14 @@ func TestClient_PurchaseCommitment_TagsCarryRichDescriptors(t *testing.T) {
 		}
 		// Required new descriptors from #687 (the existing NodeType / Region /
 		// PurchaseDate / Tool / Purpose tags are covered by the existing
-		// TagsWithResolvedARN test).
+		// TagsWithResolvedARN test). Name tag carries the rich self-describing
+		// identifier so the node is findable in the AWS console without CUDly.
+		name := got["Name"]
 		return got["Count"] == "2" &&
 			got["Term"] == "3yr" &&
-			got["PaymentOption"] == "partial-upfront"
+			got["PaymentOption"] == "partial-upfront" &&
+			len(name) > 0 &&
+			name[:8] == "redshift"
 	})).Return(&redshift.CreateTagsOutput{}, nil)
 
 	result, err := client.PurchaseCommitment(context.Background(), rec, common.PurchaseOptions{Source: common.PurchaseSourceCLI})
@@ -1141,7 +1145,7 @@ func TestFindOfferingID_PaginationCapFires(t *testing.T) {
 			}, nil).Once()
 	}
 
-	_, err := client.findOfferingID(context.Background(), rec)
+	_, err := client.findOfferingID(context.Background(), rec, "")
 
 	if assert.Error(t, err) {
 		assert.Contains(t, err.Error(), "pagination cap reached")
@@ -1173,7 +1177,7 @@ func TestFindOfferingID_WrongVariantRejected(t *testing.T) {
 			},
 		}, nil).Once()
 
-	id, err := client.findOfferingID(context.Background(), rec)
+	id, err := client.findOfferingID(context.Background(), rec, "")
 
 	assert.Error(t, err, "unknown offering type must not return success")
 	assert.Empty(t, id)
@@ -1200,8 +1204,63 @@ func TestFindOfferingID_HappyPath(t *testing.T) {
 			},
 		}, nil).Once()
 
-	id, err := client.findOfferingID(context.Background(), rec)
+	id, err := client.findOfferingID(context.Background(), rec, "")
 
 	assert.NoError(t, err)
 	assert.Equal(t, "offering-ok", id)
 }
+
+// TestClient_tagReservedNode_NameTagPresent asserts that tagReservedNode (issue
+// #687) stamps a self-describing Name tag on the Redshift reserved node. The
+// Redshift purchase API has no customer-supplied node ID, so the Name tag is
+// the only way to identify the node in the AWS console without CUDly.
+func TestClient_tagReservedNode_NameTagPresent(t *testing.T) {
+	t.Parallel()
+	mockRS := &MockRedshiftClient{}
+	mockSTS := &MockRedshiftSTSClient{}
+	client := &Client{
+		client:    mockRS,
+		stsClient: mockSTS,
+		region:    "eu-central-1",
+	}
+
+	rec := common.Recommendation{
+		Service:       common.ServiceDataWarehouse,
+		ResourceType:  "ra3.4xlarge",
+		Count:         1,
+		PaymentOption: "no-upfront",
+		Term:          "1yr",
+		Region:        "eu-central-1",
+	}
+
+	mockSTS.On("GetCallerIdentity", mock.Anything, mock.Anything).
+		Return(&sts.GetCallerIdentityOutput{Account: aws.String("111222333444")}, nil)
+
+	var capturedTags []types.Tag
+	mockRS.On("CreateTags", mock.Anything, mock.MatchedBy(func(in *redshift.CreateTagsInput) bool {
+		capturedTags = in.Tags
+		return true
+	})).Return(&redshift.CreateTagsOutput{}, nil)
+
+	err := client.tagReservedNode(context.Background(), "rn-tag-test", rec, "test-source", "")
+	assert.NoError(t, err)
+
+	tagMap := make(map[string]string, len(capturedTags))
+	for _, tag := range capturedTags {
+		tagMap[aws.ToString(tag.Key)] = aws.ToString(tag.Value)
+	}
+
+	name, ok := tagMap["Name"]
+	assert.True(t, ok, "Name tag must be present in CreateTags call")
+	assert.True(t, len(name) > 0, "Name tag must be non-empty")
+	assert.LessOrEqual(t, len(name), 60, "Name must fit the 60-char cap")
+	// Key segments that make the node self-describing without CUDly:
+	assert.Contains(t, name, "redshift", "Name must carry the service code")
+	assert.Contains(t, name, "eu-central-1", "Name must embed the region")
+	assert.Contains(t, name, "ra3-4xlarge", "Name must embed the SKU (dots->hyphens)")
+	assert.Contains(t, name, "1x", "Name must embed the count")
+	assert.Contains(t, name, "1yr", "Name must embed the term")
+
+	mockRS.AssertExpectations(t)
+	mockSTS.AssertExpectations(t)
+}