From 6891a8473dd51f55b04d87d3aad7159b13848cca Mon Sep 17 00:00:00 2001
From: Cristian Magherusan-Stanciu <cristi.magherusan@gmail.com>
Date: Mon, 20 Jul 2026 20:48:22 +0200
Subject: [PATCH] fix(azure/managedredis): pass subscription scope (not filter)
 to recommendations pager (follow-up to #1455)

NewListPager's first argument is the billing scope; managedredis passed the
ODATA filter string there, producing a malformed URL where every request
errored -- Azure Managed Redis reservation recommendations were entirely broken
on main. The filter belongs in ClientListOptions.Filter (the sibling
cache/client.go has the correct shape). Also add the pagination cap +
per-page ctx.Err() check every other Azure service client has.

Extracted recommendationsListArgs() so a unit test can assert the subscription
scope shape without a real Azure client (the injected mock pager bypasses
NewListPager, which is why the wrong shape slipped through green tests).
---
 .../azure/services/managedredis/client.go     | 29 +++++++++++++++++--
 .../services/managedredis/client_test.go      | 15 ++++++++++
 2 files changed, 41 insertions(+), 3 deletions(-)

diff --git a/providers/azure/services/managedredis/client.go b/providers/azure/services/managedredis/client.go
index 3554f3ae9..3e7bb3a01 100644
--- a/providers/azure/services/managedredis/client.go
+++ b/providers/azure/services/managedredis/client.go
@@ -25,6 +25,23 @@ import (
 	"github.com/LeanerCloud/CUDly/providers/azure/services/internal/reservations"
 )
 
+// maxRecsPages caps Consumption API recommendation pagination (matches the
+// sibling Azure service clients, e.g. cache/client.go).
+const maxRecsPages = 10
+
+// recommendationsListArgs builds the (scope, options) for the Consumption
+// ReservationRecommendations pager. NewListPager's first argument is the
+// billing scope (the subscription), NOT the ODATA filter -- passing the filter
+// as the scope produces a malformed URL where every request errors (the exact
+// failure mode documented in compute/client.go). The filter goes in
+// options.Filter. Extracted so a unit test can assert the scope shape without a
+// real Azure client (the injected mock pager bypasses NewListPager entirely).
+func (c *ManagedRedisClient) recommendationsListArgs() (string, *armconsumption.ReservationRecommendationsClientListOptions) {
+	scope := fmt.Sprintf("/subscriptions/%s", c.subscriptionID)
+	filter := "properties/scope eq 'Shared' and properties/resourceType eq 'RedisCache'"
+	return scope, &armconsumption.ReservationRecommendationsClientListOptions{Filter: &filter}
+}
+
 // HTTPClient interface for HTTP operations (enables mocking)
 type HTTPClient interface {
 	Do(req *http.Request) (*http.Response, error)
@@ -126,11 +143,17 @@ func (c *ManagedRedisClient) GetRecommendations(ctx context.Context, params comm
 		if err != nil {
 			return nil, fmt.Errorf("failed to create consumption client: %w", err)
 		}
-		filter := "properties/scope eq 'Shared' and properties/resourceType eq 'RedisCache'"
-		pager = client.NewListPager(filter, &armconsumption.ReservationRecommendationsClientListOptions{})
+		scope, opts := c.recommendationsListArgs()
+		pager = client.NewListPager(scope, opts)
 	}
 
-	for pager.More() {
+	for pageIdx := 0; pager.More(); pageIdx++ {
+		if err := ctx.Err(); err != nil {
+			return nil, fmt.Errorf("context cancelled during pagination: %w", err)
+		}
+		if pageIdx >= maxRecsPages {
+			return nil, fmt.Errorf("managedredis: GetRecommendations pagination cap (%d pages) reached", maxRecsPages)
+		}
 		page, err := pager.NextPage(ctx)
 		if err != nil {
 			return nil, fmt.Errorf("failed to get Redis Cache recommendations: %w", err)
diff --git a/providers/azure/services/managedredis/client_test.go b/providers/azure/services/managedredis/client_test.go
index de1a391b2..28adf7b69 100644
--- a/providers/azure/services/managedredis/client_test.go
+++ b/providers/azure/services/managedredis/client_test.go
@@ -332,6 +332,21 @@ func TestValidateOffering_InvalidSKU(t *testing.T) {
 
 // -- GetRecommendations --
 
+// TestRecommendationsListArgs_UsesSubscriptionScope is the regression guard for
+// the pager-construction bug: NewListPager's first argument must be the
+// subscription billing scope, not the ODATA filter (the wrong shape produced a
+// malformed URL that errored on every request, breaking Managed Redis recs).
+// The injected mock pager bypasses NewListPager, so this asserts the args helper.
+func TestRecommendationsListArgs_UsesSubscriptionScope(t *testing.T) {
+	c := NewClient(nil, "sub-123", "eastus")
+	scope, opts := c.recommendationsListArgs()
+	assert.Equal(t, "/subscriptions/sub-123", scope,
+		"first NewListPager arg must be the subscription scope, not the filter")
+	require.NotNil(t, opts)
+	require.NotNil(t, opts.Filter, "the ODATA filter must be passed via options.Filter")
+	assert.Contains(t, *opts.Filter, "resourceType eq 'RedisCache'")
+}
+
 func TestGetRecommendations_EmptyPager(t *testing.T) {
 	c := NewClient(nil, "sub", "eastus")
 	c.SetRecommendationsPager(&mockRecommendationsPager{