fix(azure/savingsplans): fail closed on a live plan without an effective date

providers/azure/services/savingsplans/client.go

@@ -290,7 +290,26 @@ func convertSavingsPlan(sp *armbillingbenefits.SavingsPlanModel, subscriptionID
 		}
 	}
 
-	return &commitment, nil
+	return requireStartDate(&commitment)
+}
+
+// requireStartDate returns c, or fails closed on a non-terminal plan with no start date.
+func requireStartDate(c *common.Commitment) (*common.Commitment, error) {
+	if c.StartDate.IsZero() && !isTerminalSavingsPlanState(c.State) {
+		return nil, fmt.Errorf("owned savings plan %s is in state %q but reports no effectiveDateTime; the duplicate guard cannot date it", c.CommitmentID, c.State)
+	}
+	return c, nil
+}
+
+// isTerminalSavingsPlanState reports the states recfilter rejects before it
+// reads StartDate, so a zero start is harmless. Anything else, including an
+// unknown or defaulted state, is treated as live.
+func isTerminalSavingsPlanState(s common.CommitmentState) bool {
+	switch s {
+	case common.CommitmentStateFailed, common.CommitmentStateCanceled, common.CommitmentStateExpired:
+		return true
+	}
+	return false
 }
 
 var currencyCodePattern = regexp.MustCompile(`^[A-Za-z]{3}$`)

providers/azure/services/savingsplans/client_completeness_sdk_test.go

@@ -122,3 +122,57 @@ func newFixtureSavingsPlanClient(t *testing.T, server *httptest.Server) *armbill
 	require.NoError(t, err)
 	return sdk
 }
+
+func TestGetExistingCommitments_EffectiveDateTimeCompleteness(t *testing.T) {
+	const planID = "/providers/Microsoft.BillingBenefits/savingsPlanOrders/o1/savingsPlans/p1"
+	cases := []struct {
+		name    string
+		props   string
+		wantErr bool
+	}{
+		{"live state without start", `"provisioningState":"Succeeded"`, true},
+		{"pending state without start", `"provisioningState":"PendingBilling"`, true},
+		{"no state without start", ``, true},
+		{"canceled-state without start", `"provisioningState":"Cancelled"`, false}, //nolint:misspell // Azure wire value
+		{"expired without start", `"provisioningState":"Expired"`, false},
+		{"failed without start", `"provisioningState":"Failed"`, false},
+		{"live state with start", `"provisioningState":"Succeeded","effectiveDateTime":"2026-01-02T03:04:05Z"`, false},
+	}
+	for _, tc := range cases {
+		for _, dedupe := range []bool{false, true} {
+			if dedupe && !tc.wantErr {
+				continue
+			}
+			t.Run(fmt.Sprintf("%s/dedupe=%t", tc.name, dedupe), func(t *testing.T) {
+				props := `"billingScopeId":"/subscriptions/sub-a"`
+				if tc.props != "" {
+					props += "," + tc.props
+				}
+				server := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+					w.Header().Set("Content-Type", "application/json")
+					_, err := fmt.Fprintf(w, `{"value":[{"id":%q,"sku":{"name":"Compute_Savings_Plan"},"properties":{%s}}]}`, planID, props)
+					assert.NoError(t, err)
+				}))
+				t.Cleanup(server.Close)
+				client := NewClient(nil, "sub-a", "eastus")
+				client.SetListAllPager(newFixtureSavingsPlanClient(t, server).NewListAllPager(nil))
+				ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
+				defer cancel()
+				if dedupe {
+					recs := []common.Recommendation{{Provider: common.ProviderAzure, Service: common.ServiceSavingsPlansAll, ResourceType: "Compute", Count: 1}}
+					_, _, err := recfilter.NewDuplicateChecker(24).AdjustRecommendationsForExisting(ctx, recs, client)
+					require.ErrorContains(t, err, planID)
+					return
+				}
+				got, err := client.GetExistingCommitments(ctx)
+				if tc.wantErr {
+					require.ErrorContains(t, err, "incomplete savings plan inventory")
+					require.ErrorContains(t, err, planID)
+					return
+				}
+				require.NoError(t, err)
+				require.Len(t, got, 1)
+			})
+		}
+	}
+}

providers/azure/services/savingsplans/client_pager_cap_test.go

@@ -3,6 +3,7 @@ package savingsplans
 import (
 	"context"
 	"testing"
+	"time"
 
 	"github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/billingbenefits/armbillingbenefits"
 	"github.com/stretchr/testify/assert"
@@ -34,7 +35,7 @@ func ownedPlan(id string) *armbillingbenefits.SavingsPlanModel {
 	return &armbillingbenefits.SavingsPlanModel{
 		ID:         &id,
 		SKU:        &armbillingbenefits.SKU{Name: toPtr("Compute_Savings_Plan")},
-		Properties: &armbillingbenefits.SavingsPlanModelProperties{BillingScopeID: toPtr("/subscriptions/sub-abc")},
+		Properties: &armbillingbenefits.SavingsPlanModelProperties{BillingScopeID: toPtr("/subscriptions/sub-abc"), EffectiveDateTime: toPtr(time.Now())},
 	}
 }
 

providers/azure/services/savingsplans/client_test.go

@@ -192,6 +192,7 @@ func TestGetExistingCommitments_SubscriptionOwnership(t *testing.T) {
 			SKU: &armbillingbenefits.SKU{Name: toPtr("Compute_Savings_Plan")},
 			Properties: &armbillingbenefits.SavingsPlanModelProperties{
 				BillingScopeID:         toPtr(scope),
+				EffectiveDateTime:      toPtr(time.Now()),
 				AppliedScopeType:       toPtr(applied),
 				AppliedScopeProperties: &armbillingbenefits.AppliedScopeProperties{SubscriptionID: toPtr(subscription)},
 			},

providers/azure/services/savingsplans/scope_test.go

@@ -25,7 +25,7 @@ const (
 )
 
 func accountPlan(scope string, typ *armbillingbenefits.AppliedScopeType, props *armbillingbenefits.AppliedScopeProperties) *armbillingbenefits.SavingsPlanModel {
-	return &armbillingbenefits.SavingsPlanModel{ID: toPtr("plan-id"), SKU: &armbillingbenefits.SKU{Name: toPtr("Compute_Savings_Plan")}, Properties: &armbillingbenefits.SavingsPlanModelProperties{BillingScopeID: toPtr(scope), AppliedScopeType: typ, AppliedScopeProperties: props}}
+	return &armbillingbenefits.SavingsPlanModel{ID: toPtr("plan-id"), SKU: &armbillingbenefits.SKU{Name: toPtr("Compute_Savings_Plan")}, Properties: &armbillingbenefits.SavingsPlanModelProperties{BillingScopeID: toPtr(scope), EffectiveDateTime: toPtr(time.Now()), AppliedScopeType: typ, AppliedScopeProperties: props}}
 }
 
 // scoped builds applied-scope properties; "" leaves the field nil.