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

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

@@ -3,6 +3,7 @@ package main
 import (
 	"fmt"
 	"strconv"
+	"strings"
 	"time"
 
 	"github.com/LeanerCloud/cloud-commitments-go/pkg/common"
@@ -151,9 +152,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 == "" && strings.TrimSpace(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,12 @@ 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"}
+		}),
+		"ec2 instance sp whitespace family no offering": mut(validEC2InstanceSP(), func(r *common.Recommendation) {
+			r.Details = &common.SavingsPlanDetails{PlanType: "EC2Instance", HourlyCommitment: 1, Region: "eu-west-1", InstanceFamily: "   "}
+		}),
 		"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 = "" }),