fix(cmd): refuse an EC2 Instance Savings Plan without a family in the dry run too

validatePurchasePreconditions required only a region for an EC2 Instance
Savings Plan. With no instance family and no offering ID, the real run
refuses in the offering lookup (ambiguous or missing offerings) while the
dry run reported success. Require the family unless an offering ID is set.
The completeness fixture now carries a family, as a real Cost Explorer
EC2 Instance recommendation does.

Closes #2164

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Author: cristim

CHANGELOG.md

@@ -16,6 +16,9 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/).
 - Recommendations with no account ID or name are now reported in the drop summary
   when an account filter (`--include-accounts` or `--exclude-accounts`) removes
   them (#1613).
+- A dry run now refuses an EC2 Instance Savings Plan that has no instance family
+  and no offering ID, the same way the real run does, instead of reporting
+  success for a purchase the offering lookup would reject (#2164).
 - A purchase result with `Success=true` and a non-nil `Error` is now treated as
   a failure in the Ctrl-C summary, the audit-stop message, the per-service
   success/failed counts and the non-zero exit. The CSV `Success` column keeps the

cmd/recommendation_completeness_proxy_test.go

@@ -305,7 +305,7 @@ func (p *completenessProxy) spRecommendation(body []byte) (int, string, string)
 	const invalid = `{"HourlyCommitmentToPurchase":"not-a-number","EstimatedMonthlySavingsAmount":"10","UpfrontCost":"3"}`
 	details := valid
 	if request.SavingsPlansType == "EC2_INSTANCE_SP" {
-		details = strings.TrimSuffix(valid, "}") + `,"SavingsPlansDetails":{"Region":"us-east-1"}}`
+		details = strings.TrimSuffix(valid, "}") + `,"SavingsPlansDetails":{"InstanceFamily":"m5","Region":"us-east-1"}}`
 	}
 	switch p.details {
 	case "mixed":

cmd/savings_plan_purchase.go

@@ -151,9 +151,22 @@ func validateSavingsPlanPreconditions(rec common.Recommendation) error {
 	if fmt.Sprintf(spCommitmentFormat, d.HourlyCommitment) == "0.00" {
 		return fmt.Errorf("%s recommendation's hourly commitment %v would be sent as 0.00; it must be at least 0.005", rec.Service, d.HourlyCommitment)
 	}
-	if rec.Service == common.ServiceSavingsPlansEC2Instance && d.Region == "" {
+	if rec.Service == common.ServiceSavingsPlansEC2Instance {
+		return validateEC2InstanceSPOffering(d)
+	}
+	return nil
+}
+
+// validateEC2InstanceSPOffering mirrors the library's offering lookup: an
+// explicit offering ID skips it, otherwise it needs the region and family to
+// pick exactly one offering.
+func validateEC2InstanceSPOffering(d *common.SavingsPlanDetails) error {
+	if d.Region == "" {
 		return fmt.Errorf("EC2 Instance Savings Plan needs a region to pick the offering")
 	}
+	if d.OfferingID == "" && d.InstanceFamily == "" {
+		return fmt.Errorf("EC2 Instance Savings Plan needs an instance family or an offering ID to pick the offering")
+	}
 	return nil
 }
 

cmd/savings_plan_purchase_test.go

@@ -258,8 +258,14 @@ func TestValidatePurchasePreconditions(t *testing.T) {
 	ok := map[string]common.Recommendation{
 		"compute sp":      parserShapedSP(),
 		"ec2 instance sp": validEC2InstanceSP(),
-		"ec2":             validEC2Rec(),
-		"rds":             {Service: common.ServiceRDS, Region: "us-east-1"},
+		"ec2 instance sp offering id no family": mut(validEC2InstanceSP(), func(r *common.Recommendation) {
+			r.Details = &common.SavingsPlanDetails{PlanType: "EC2Instance", HourlyCommitment: 1, Region: "eu-west-1", OfferingID: "o-1"}
+		}),
+		"compute sp no family": mut(parserShapedSP(), func(r *common.Recommendation) {
+			r.Details = &common.SavingsPlanDetails{PlanType: "Compute", HourlyCommitment: 1}
+		}),
+		"ec2": validEC2Rec(),
+		"rds": {Service: common.ServiceRDS, Region: "us-east-1"},
 	}
 	for name, rec := range ok {
 		assert.NoError(t, validatePurchasePreconditions(rec, rec.Region, defaultTestAccount), name)
@@ -274,6 +280,9 @@ func TestValidatePurchasePreconditions(t *testing.T) {
 		"ec2 instance sp no region": mut(validEC2InstanceSP(), func(r *common.Recommendation) {
 			r.Details = &common.SavingsPlanDetails{PlanType: "EC2Instance", HourlyCommitment: 1, InstanceFamily: "m5"}
 		}),
+		"ec2 instance sp no family no offering": mut(validEC2InstanceSP(), func(r *common.Recommendation) {
+			r.Details = &common.SavingsPlanDetails{PlanType: "EC2Instance", HourlyCommitment: 1, Region: "eu-west-1"}
+		}),
 		"sp all":        mut(parserShapedSP(), func(r *common.Recommendation) { r.Service = common.ServiceSavingsPlansAll }),
 		"ec2 nil":       mut(validEC2Rec(), func(r *common.Recommendation) { r.Details = nil }),
 		"ec2 tenancy":   mut(validEC2Rec(), func(r *common.Recommendation) { r.Details.(*common.ComputeDetails).Tenancy = "" }),