fix(aws/rds): refuse explicitly when the offering page cap hides uniqueness

providers/aws/services/rds/client.go

@@ -300,6 +300,12 @@ func (c *Client) findReservationByID(ctx context.Context, reservationID string)
 // pages to walk before giving up. At MaxRecords=100 per page this caps the
 // search at 500 offerings. Exceeding the cap returns a diagnostic error instead
 // of timing out the Lambda budget (issue #688).
+//
+// The walk must reach the last page to prove an exact match is unique (issue
+// #288), so hitting the cap is a refuse-only outcome even when a match was
+// already found: the lookup errors and nothing is purchased (issue #303). The
+// request filters on class, engine, multi-AZ, duration and payment option, so
+// a result set anywhere near 500 rows is not expected.
 const maxOfferingPages = 5
 
 // findOfferingID finds the appropriate RDS Reserved Instance offering ID.
@@ -371,9 +377,10 @@ func (c *Client) fetchRDSOfferingPage(ctx context.Context, baseInput *rds.Descri
 	return rdsOfferingPageResult{id: id, marker: nextMarker}, nil
 }
 
-// paginateRDSOfferings walks DescribeReservedDBInstancesOfferings pages and returns
-// the first matching offering ID. It caps at maxOfferingPages to prevent Lambda
-// timeout exhaustion (issue #688).
+// paginateRDSOfferings walks every DescribeReservedDBInstancesOfferings page and
+// returns the single exact-match offering ID. It caps at maxOfferingPages to
+// prevent Lambda timeout exhaustion (issue #688); reaching the cap errors even
+// when a match was already seen (issue #303).
 func (c *Client) paginateRDSOfferings(ctx context.Context, rec common.Recommendation, details *common.DatabaseDetails, offeringType, execID string) (string, error) {
 	multiAZ := details.AZConfig == "multi-az"
 	normalizedEngine, err := c.lookupEngineName(details.Engine)
@@ -418,7 +425,7 @@ func (c *Client) paginateRDSOfferings(ctx context.Context, rec common.Recommenda
 	var found string
 	pages := 0
 	for page := 1; ; page++ {
-		if err := offeringPageAllowed(ctx, page, desc); err != nil {
+		if err := offeringPageAllowed(ctx, page, desc, found); err != nil {
 			return "", err
 		}
 		pr, err := c.fetchRDSOfferingPage(ctx, baseInput, marker, want, tag, page, t0)
@@ -442,14 +449,20 @@ func (c *Client) paginateRDSOfferings(ctx context.Context, rec common.Recommenda
 }
 
 // offeringPageAllowed stops the page walk on cancellation or at the page cap.
-func offeringPageAllowed(ctx context.Context, page int, desc string) error {
+// found is the exact match seen so far, if any; at the cap it cannot be proven
+// unique, so the walk refuses rather than return it (issue #303).
+func offeringPageAllowed(ctx context.Context, page int, desc, found string) error {
 	if err := ctx.Err(); err != nil {
 		return err
 	}
-	if page > maxOfferingPages {
-		return fmt.Errorf("pagination cap reached after %d pages for RDS %s (issue #688)", maxOfferingPages, desc)
+	if page <= maxOfferingPages {
+		return nil
+	}
+	if found != "" {
+		return fmt.Errorf("pagination cap reached after %d pages for RDS %s: offering %q matched but more pages remain, "+
+			"so it cannot be proven unique; refusing to purchase (issue #303)", maxOfferingPages, desc, found)
 	}
-	return nil
+	return fmt.Errorf("pagination cap reached after %d pages for RDS %s (issue #688)", maxOfferingPages, desc)
 }
 
 // pickOfferingID folds a newly found exact-match ID into the one found so far

providers/aws/services/rds/client_test.go

@@ -1336,3 +1336,58 @@ func TestFindOfferingID_LicensedEnginesRefusedBeforeAPICall(t *testing.T) {
 		})
 	}
 }
+
+// cappedOfferingPages returns maxOfferingPages pages, each with a next-page
+// marker, so the walk always reaches the page cap. matchPage (1-based) carries
+// an exact match; every other page is empty.
+func cappedOfferingPages(mockRDS *MockRDSClient, matchPage int) {
+	for i := range maxOfferingPages {
+		out := &rds.DescribeReservedDBInstancesOfferingsOutput{
+			ReservedDBInstancesOfferings: []types.ReservedDBInstancesOffering{},
+			Marker:                       aws.String(fmt.Sprintf("tok-%d", i+1)),
+		}
+		if i+1 == matchPage {
+			out.ReservedDBInstancesOfferings = []types.ReservedDBInstancesOffering{exactOffering("offering-early")}
+		}
+		mockRDS.On("DescribeReservedDBInstancesOfferings", mock.Anything, mock.Anything).Return(out, nil).Once()
+	}
+}
+
+// TestFindOfferingID_MatchBeforeCapRefusesWhenMorePagesRemain pins issue #303:
+// an exact match seen before the page cap cannot be proven unique while more
+// pages remain, so the lookup must refuse (never return the match) with an
+// error that names the unverified match, and must not fetch past the cap.
+func TestFindOfferingID_MatchBeforeCapRefusesWhenMorePagesRemain(t *testing.T) {
+	for _, matchPage := range []int{1, maxOfferingPages} {
+		t.Run(fmt.Sprintf("match on page %d", matchPage), func(t *testing.T) {
+			mockRDS := &MockRDSClient{}
+			t.Cleanup(func() { mockRDS.AssertExpectations(t) })
+			client := &Client{client: mockRDS, region: "us-east-1"}
+			cappedOfferingPages(mockRDS, matchPage)
+
+			id, err := client.findOfferingID(context.Background(), idempotencyTestRec(), "")
+
+			assert.Empty(t, id)
+			if assert.Error(t, err) {
+				assert.Contains(t, err.Error(), "pagination cap reached")
+				assert.Contains(t, err.Error(), `"offering-early"`)
+				assert.Contains(t, err.Error(), "cannot be proven unique")
+				assert.Contains(t, err.Error(), "refusing to purchase")
+			}
+			mockRDS.AssertNumberOfCalls(t, "DescribeReservedDBInstancesOfferings", maxOfferingPages)
+		})
+	}
+}
+
+// TestFindOfferingID_MatchOnLastAllowedPageSucceeds pins the cap boundary for
+// issue #303: when the final page is the last one the cap allows (no further
+// marker), the walk is complete and the unique match is returned.
+func TestFindOfferingID_MatchOnLastAllowedPageSucceeds(t *testing.T) {
+	pages := make([][]types.ReservedDBInstancesOffering, maxOfferingPages)
+	pages[maxOfferingPages-1] = []types.ReservedDBInstancesOffering{exactOffering("offering-last")}
+
+	id, err := findAcrossPages(t, pages...)
+
+	assert.NoError(t, err)
+	assert.Equal(t, "offering-last", id)
+}