fix(aws/savingsplans): error on a plan with no start or end date

CHANGELOG.md

@@ -218,6 +218,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/).
 - **AWS/OpenSearch:** the STS account-ID lookup used for post-purchase tagging caches only a successful result, so one throttled or timed-out `GetCallerIdentity` no longer disables tagging for the client's lifetime (#53).
 - **AWS/recommendations:** `normalizeRegionName` maps the current `Europe (Ireland|Frankfurt|London|Paris|Stockholm)` Cost Explorer display names to region codes, so coverage lookups keyed by region code match those recommendations (#53).
 - **AWS/Savings Plans:** `GetExistingCommitments` returns an error naming the plan ID and value when a plan has an unparseable start or end date, instead of silently leaving the date zero (#53).
+- **AWS/Savings Plans:** `GetExistingCommitments` returns an error naming the plan ID when a plan has no start or end date; a zero start date was never treated as recent by the purchase dedupe, so a duplicate purchase could slip through (#53).
 - **AWS/RDS:** `PurchaseCommitment` refuses an offering whose upfront (`FixedPrice` x count) exceeds the approved recommendation upfront by more than 5%, or that is priced in a non-USD currency; a zero approved upfront is not checked (#53).
 - Bump Go to 1.26.10 to pick up the fix for GO-2026-6809 (crypto/x509), which `govulncheck` now reports against 1.26.9.
 - **AWS/MemoryDB, OpenSearch, Redshift:** `PurchaseCommitment` refuses an offering whose upfront (`FixedPrice` x count) exceeds the approved recommendation upfront by more than 5%, or that is priced in a non-USD currency (MemoryDB offerings carry no currency, so only the recommendation's currency is checked there), or, for MemoryDB, an upfront offering with a zero price or one bought in a China region (cn-north-1, cn-northwest-1), where the assumed USD price is wrong; a zero approved upfront is not checked. All AWS RI price guards (EC2, RDS, MemoryDB, OpenSearch, Redshift) also refuse a NaN or infinite approved upfront (#53).

providers/aws/services/savingsplans/client.go

@@ -185,8 +185,9 @@ func (c *Client) GetExistingCommitments(ctx context.Context) ([]common.Commitmen
 // common.Commitment, returning ok=false for entries that should be skipped
 // (missing ID, or a plan type that does not match this client's scope). It is
 // split out of GetExistingCommitments to keep that function under the
-// cyclomatic limit. A start or end date that is present but not RFC3339 is an
-// error naming the plan ID and the bad value.
+// cyclomatic limit. A start or end date that is missing or not RFC3339 is an
+// error naming the plan ID (and the bad value); a zero StartDate would never
+// count as recent in the purchase dedupe.
 func (c *Client) toCommitment(sp types.SavingsPlan, service common.ServiceType) (common.Commitment, bool, error) {
 	if sp.SavingsPlanId == nil {
 		return common.Commitment{}, false, nil
@@ -206,20 +207,22 @@ func (c *Client) toCommitment(sp types.SavingsPlan, service common.ServiceType)
 		State:          common.CommitmentState(sp.State),
 	}
 
-	if sp.Start != nil {
-		startTime, err := time.Parse(time.RFC3339, *sp.Start)
-		if err != nil {
-			return common.Commitment{}, false, fmt.Errorf("savings plan %s: invalid start date %q: %w", *sp.SavingsPlanId, *sp.Start, err)
-		}
-		commitment.StartDate = startTime
+	if sp.Start == nil {
+		return common.Commitment{}, false, fmt.Errorf("savings plan %s: missing start date", *sp.SavingsPlanId)
 	}
-	if sp.End != nil {
-		endTime, err := time.Parse(time.RFC3339, *sp.End)
-		if err != nil {
-			return common.Commitment{}, false, fmt.Errorf("savings plan %s: invalid end date %q: %w", *sp.SavingsPlanId, *sp.End, err)
-		}
-		commitment.EndDate = endTime
+	startTime, err := time.Parse(time.RFC3339, *sp.Start)
+	if err != nil {
+		return common.Commitment{}, false, fmt.Errorf("savings plan %s: invalid start date %q: %w", *sp.SavingsPlanId, *sp.Start, err)
+	}
+	commitment.StartDate = startTime
+	if sp.End == nil {
+		return common.Commitment{}, false, fmt.Errorf("savings plan %s: missing end date", *sp.SavingsPlanId)
+	}
+	endTime, err := time.Parse(time.RFC3339, *sp.End)
+	if err != nil {
+		return common.Commitment{}, false, fmt.Errorf("savings plan %s: invalid end date %q: %w", *sp.SavingsPlanId, *sp.End, err)
 	}
+	commitment.EndDate = endTime
 
 	return commitment, true, nil
 }

providers/aws/services/savingsplans/client_test.go

@@ -156,6 +156,8 @@ func TestClient_GetExistingCommitments(t *testing.T) {
 								SavingsPlanId:   aws.String("sp-123"),
 								SavingsPlanType: types.SavingsPlanTypeCompute,
 								State:           types.SavingsPlanStateActive,
+								Start:           aws.String("2024-01-01T00:00:00Z"),
+								End:             aws.String("2027-01-01T00:00:00Z"),
 							},
 							{
 								// No SavingsPlanId - should be skipped
@@ -230,8 +232,8 @@ func TestClient_GetExistingCommitments(t *testing.T) {
 func TestClient_GetExistingCommitments_InvalidDateNamesPlanAndValue(t *testing.T) {
 	t.Parallel()
 	for field, plan := range map[string]types.SavingsPlan{
-		"start": {SavingsPlanId: aws.String("sp-bad"), SavingsPlanType: types.SavingsPlanTypeCompute, Start: aws.String("not-a-date")},
-		"end":   {SavingsPlanId: aws.String("sp-bad"), SavingsPlanType: types.SavingsPlanTypeCompute, End: aws.String("not-a-date")},
+		"start": {SavingsPlanId: aws.String("sp-bad"), SavingsPlanType: types.SavingsPlanTypeCompute, Start: aws.String("not-a-date"), End: aws.String("2027-01-01T00:00:00Z")},
+		"end":   {SavingsPlanId: aws.String("sp-bad"), SavingsPlanType: types.SavingsPlanTypeCompute, Start: aws.String("2024-01-01T00:00:00Z"), End: aws.String("not-a-date")},
 	} {
 		t.Run(field, func(t *testing.T) {
 			m := &MockSavingsPlansClient{}
@@ -247,6 +249,28 @@ func TestClient_GetExistingCommitments_InvalidDateNamesPlanAndValue(t *testing.T
 	}
 }
 
+// TestClient_GetExistingCommitments_MissingDateNamesPlan pins that a plan with
+// no start or end date is an error, not a zero date that the purchase dedupe
+// would never treat as recent.
+func TestClient_GetExistingCommitments_MissingDateNamesPlan(t *testing.T) {
+	t.Parallel()
+	for field, plan := range map[string]types.SavingsPlan{
+		"start": {SavingsPlanId: aws.String("sp-nodate"), SavingsPlanType: types.SavingsPlanTypeCompute, End: aws.String("2027-01-01T00:00:00Z")},
+		"end":   {SavingsPlanId: aws.String("sp-nodate"), SavingsPlanType: types.SavingsPlanTypeCompute, Start: aws.String("2024-01-01T00:00:00Z")},
+	} {
+		t.Run(field, func(t *testing.T) {
+			m := &MockSavingsPlansClient{}
+			m.On("DescribeSavingsPlans", mock.Anything, mock.Anything).
+				Return(&savingsplans.DescribeSavingsPlansOutput{SavingsPlans: []types.SavingsPlan{plan}}, nil).Once()
+			c := &Client{client: m, region: "us-east-1", planType: types.SavingsPlanTypeCompute}
+			_, err := c.GetExistingCommitments(context.Background())
+			require.Error(t, err)
+			assert.Contains(t, err.Error(), "sp-nodate")
+			assert.Contains(t, err.Error(), "missing "+field+" date")
+		})
+	}
+}
+
 // TestClient_GetExistingCommitments_UmbrellaMode verifies that an SP client
 // constructed with an empty plan type (legacy umbrella mode, used by the
 // AWS provider's `case common.ServiceSavingsPlansAll` branch in GetServiceClient)
@@ -263,16 +287,22 @@ func TestClient_GetExistingCommitments_UmbrellaMode(t *testing.T) {
 					SavingsPlanId:   aws.String("sp-compute"),
 					SavingsPlanType: types.SavingsPlanTypeCompute,
 					State:           types.SavingsPlanStateActive,
+					Start:           aws.String("2024-01-01T00:00:00Z"),
+					End:             aws.String("2027-01-01T00:00:00Z"),
 				},
 				{
 					SavingsPlanId:   aws.String("sp-sagemaker"),
 					SavingsPlanType: types.SavingsPlanTypeSagemaker,
 					State:           types.SavingsPlanStateActive,
+					Start:           aws.String("2024-01-01T00:00:00Z"),
+					End:             aws.String("2027-01-01T00:00:00Z"),
 				},
 				{
 					SavingsPlanId:   aws.String("sp-database"),
 					SavingsPlanType: types.SavingsPlanTypeDatabase,
 					State:           types.SavingsPlanStateActive,
+					Start:           aws.String("2024-01-01T00:00:00Z"),
+					End:             aws.String("2027-01-01T00:00:00Z"),
 				},
 			},
 		}, nil).Once()
@@ -309,6 +339,8 @@ func TestClient_GetExistingCommitments_Pagination(t *testing.T) {
 				SavingsPlanId:   aws.String("sp-page1"),
 				SavingsPlanType: types.SavingsPlanTypeCompute,
 				State:           types.SavingsPlanStateActive,
+				Start:           aws.String("2024-01-01T00:00:00Z"),
+				End:             aws.String("2027-01-01T00:00:00Z"),
 				Region:          aws.String("us-east-1"),
 			},
 		},
@@ -324,6 +356,8 @@ func TestClient_GetExistingCommitments_Pagination(t *testing.T) {
 				SavingsPlanId:   aws.String("sp-page2"),
 				SavingsPlanType: types.SavingsPlanTypeCompute,
 				State:           types.SavingsPlanStateActive,
+				Start:           aws.String("2024-01-01T00:00:00Z"),
+				End:             aws.String("2027-01-01T00:00:00Z"),
 				Region:          aws.String("us-west-2"),
 			},
 		},
@@ -366,6 +400,8 @@ func TestClient_GetExistingCommitments_CtxCancellation(t *testing.T) {
 				SavingsPlanId:   aws.String("sp-1"),
 				SavingsPlanType: types.SavingsPlanTypeCompute,
 				State:           types.SavingsPlanStateActive,
+				Start:           aws.String("2024-01-01T00:00:00Z"),
+				End:             aws.String("2027-01-01T00:00:00Z"),
 			},
 		},
 		NextToken: aws.String("token-page2"),
@@ -1728,6 +1764,8 @@ func TestClient_GetExistingCommitments_RequestsPaymentPending(t *testing.T) {
 			SavingsPlanId:   aws.String("sp-pending"),
 			SavingsPlanType: types.SavingsPlanTypeCompute,
 			State:           types.SavingsPlanStatePaymentPending,
+			Start:           aws.String("2024-01-01T00:00:00Z"),
+			End:             aws.String("2027-01-01T00:00:00Z"),
 		}},
 	}, nil).Once()
 	t.Cleanup(func() { mockClient.AssertExpectations(t) })