From e0517d5adfe6ec04e8ce9efdce9a723025da1a71 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Thu, 21 May 2026 00:13:01 +0200 Subject: [PATCH 1/6] feat(providers/azure/managed-redis): add Azure Managed Redis service client (closes #556) Add providers/azure/services/managedredis as Azure's counterpart to AWS MemoryDB. Azure Cache for Redis is the closest Azure equivalent: a fully- managed in-memory caching service with reservation-based pricing via the Azure Consumption and Retail Prices APIs. The new client registers under ServiceMemoryDB so it is routed and reported symmetrically with the AWS MemoryDB client at the provider-dispatch level. It reuses the armredis/v3 and armconsumption SDK packages already present in the azure provider module - no new dependencies. Changes: - providers/azure/services/managedredis/client.go: new ManagedRedisClient implementing the full provider.ServiceClient interface (GetRecommendations, GetExistingCommitments, PurchaseCommitment, ValidateOffering, GetOfferingDetails, GetValidResourceTypes) - providers/azure/services/managedredis/client_test.go: 34 tests covering all methods, pager injection, HTTP mock, token error, SKU fallback - providers/azure/services.go: NewManagedRedisClient factory - providers/azure/provider.go: ServiceMemoryDB in GetSupportedServices and newServiceClientForSubscription switch - providers/azure/services_test.go: TestNewManagedRedisClient - providers/azure/provider_test.go: ServiceMemoryDB assertions in GetSupportedServices and AllServiceTypes tests --- providers/azure/provider.go | 3 + providers/azure/provider_test.go | 3 + providers/azure/services.go | 7 + .../azure/services/managedredis/client.go | 540 +++++++++++++++++ .../services/managedredis/client_test.go | 571 ++++++++++++++++++ providers/azure/services_test.go | 8 + 6 files changed, 1132 insertions(+) create mode 100644 providers/azure/services/managedredis/client.go create mode 100644 providers/azure/services/managedredis/client_test.go diff --git a/providers/azure/provider.go b/providers/azure/provider.go index 7f90a4d2b..c6146bb51 100644 --- a/providers/azure/provider.go +++ b/providers/azure/provider.go @@ -418,6 +418,7 @@ func (p *AzureProvider) GetSupportedServices() []common.ServiceType { common.ServiceRelationalDB, common.ServiceNoSQL, common.ServiceCache, + common.ServiceMemoryDB, common.ServiceSavingsPlans, common.ServiceSearch, } @@ -473,6 +474,8 @@ func (p *AzureProvider) newServiceClientForSubscription(service common.ServiceTy return NewCacheClient(p.cred, subscriptionID, region), nil case common.ServiceNoSQL: return NewCosmosDBClient(p.cred, subscriptionID, region), nil + case common.ServiceMemoryDB: + return NewManagedRedisClient(p.cred, subscriptionID, region), nil case common.ServiceSavingsPlans: return NewSavingsPlansClient(p.cred, subscriptionID, region), nil case common.ServiceSearch: diff --git a/providers/azure/provider_test.go b/providers/azure/provider_test.go index d6705d54a..95740e89e 100644 --- a/providers/azure/provider_test.go +++ b/providers/azure/provider_test.go @@ -264,6 +264,7 @@ func TestAzureProvider_GetSupportedServices(t *testing.T) { assert.Contains(t, services, common.ServiceRelationalDB) assert.Contains(t, services, common.ServiceNoSQL) assert.Contains(t, services, common.ServiceCache) + assert.Contains(t, services, common.ServiceMemoryDB) } func TestAzureProvider_IsConfigured(t *testing.T) { @@ -408,6 +409,8 @@ func TestAzureProvider_GetServiceClient_AllServiceTypes(t *testing.T) { {common.ServiceCompute}, {common.ServiceRelationalDB}, {common.ServiceCache}, + {common.ServiceNoSQL}, + {common.ServiceMemoryDB}, } for _, tc := range testCases { diff --git a/providers/azure/services.go b/providers/azure/services.go index ebf424a1f..8e560673a 100644 --- a/providers/azure/services.go +++ b/providers/azure/services.go @@ -8,6 +8,7 @@ import ( "github.com/LeanerCloud/CUDly/providers/azure/services/compute" "github.com/LeanerCloud/CUDly/providers/azure/services/cosmosdb" "github.com/LeanerCloud/CUDly/providers/azure/services/database" + "github.com/LeanerCloud/CUDly/providers/azure/services/managedredis" "github.com/LeanerCloud/CUDly/providers/azure/services/savingsplans" "github.com/LeanerCloud/CUDly/providers/azure/services/search" ) @@ -32,6 +33,12 @@ func NewCosmosDBClient(cred azcore.TokenCredential, subscriptionID, region strin return cosmosdb.NewClient(cred, subscriptionID, region) } +// NewManagedRedisClient creates a new Azure Managed Redis client (ServiceMemoryDB). +// Azure Cache for Redis is the Azure equivalent of AWS MemoryDB for Redis. +func NewManagedRedisClient(cred azcore.TokenCredential, subscriptionID, region string) provider.ServiceClient { + return managedredis.NewClient(cred, subscriptionID, region) +} + // NewSavingsPlansClient creates a new Azure Savings Plans client func NewSavingsPlansClient(cred azcore.TokenCredential, subscriptionID, region string) provider.ServiceClient { return savingsplans.NewClient(cred, subscriptionID, region) diff --git a/providers/azure/services/managedredis/client.go b/providers/azure/services/managedredis/client.go new file mode 100644 index 000000000..d6ba28af9 --- /dev/null +++ b/providers/azure/services/managedredis/client.go @@ -0,0 +1,540 @@ +// Package managedredis provides Azure Managed Redis (Azure Cache for Redis) Reserved Capacity client. +// Azure Cache for Redis is the Azure equivalent of AWS MemoryDB for Redis: a fully-managed, +// in-memory caching service with reservation-based pricing. +package managedredis + +import ( + "context" + "encoding/json" + "fmt" + "io" + "net/http" + "net/url" + "strings" + "time" + + "github.com/Azure/azure-sdk-for-go/sdk/azcore" + "github.com/Azure/azure-sdk-for-go/sdk/azcore/policy" + "github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/consumption/armconsumption" + "github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/redis/armredis/v3" + + "github.com/LeanerCloud/CUDly/pkg/common" +) + +// HTTPClient interface for HTTP operations (enables mocking) +type HTTPClient interface { + Do(req *http.Request) (*http.Response, error) +} + +// RecommendationsPager interface for recommendations pager (enables mocking) +type RecommendationsPager interface { + More() bool + NextPage(ctx context.Context) (armconsumption.ReservationRecommendationsClientListResponse, error) +} + +// ReservationsDetailsPager interface for reservations details pager (enables mocking) +type ReservationsDetailsPager interface { + More() bool + NextPage(ctx context.Context) (armconsumption.ReservationsDetailsClientListByReservationOrderResponse, error) +} + +// RedisCachesPager interface for Redis caches pager (enables mocking) +type RedisCachesPager interface { + More() bool + NextPage(ctx context.Context) (armredis.ClientListBySubscriptionResponse, error) +} + +// ManagedRedisClient handles Azure Cache for Redis Reserved Capacity as Azure's MemoryDB equivalent. +// It surfaces under ServiceMemoryDB so it is treated symmetrically with AWS MemoryDB at the +// provider-dispatch level. +type ManagedRedisClient struct { + cred azcore.TokenCredential + subscriptionID string + region string + httpClient HTTPClient + recommendationsPager RecommendationsPager + reservationsPager ReservationsDetailsPager + redisCachesPager RedisCachesPager +} + +// NewClient creates a new ManagedRedisClient. +func NewClient(cred azcore.TokenCredential, subscriptionID, region string) *ManagedRedisClient { + return &ManagedRedisClient{ + cred: cred, + subscriptionID: subscriptionID, + region: region, + httpClient: &http.Client{Timeout: 30 * time.Second}, + } +} + +// NewClientWithHTTP creates a new ManagedRedisClient with a custom HTTP client (for testing). +func NewClientWithHTTP(cred azcore.TokenCredential, subscriptionID, region string, httpClient HTTPClient) *ManagedRedisClient { + return &ManagedRedisClient{ + cred: cred, + subscriptionID: subscriptionID, + region: region, + httpClient: httpClient, + } +} + +// SetRecommendationsPager sets the recommendations pager (for testing). +func (c *ManagedRedisClient) SetRecommendationsPager(pager RecommendationsPager) { + c.recommendationsPager = pager +} + +// SetReservationsPager sets the reservations pager (for testing). +func (c *ManagedRedisClient) SetReservationsPager(pager ReservationsDetailsPager) { + c.reservationsPager = pager +} + +// SetRedisCachesPager sets the Redis caches pager (for testing). +func (c *ManagedRedisClient) SetRedisCachesPager(pager RedisCachesPager) { + c.redisCachesPager = pager +} + +// GetServiceType returns ServiceMemoryDB -- the cloud-agnostic label for in-memory DB services. +func (c *ManagedRedisClient) GetServiceType() common.ServiceType { + return common.ServiceMemoryDB +} + +// GetRegion returns the region. +func (c *ManagedRedisClient) GetRegion() string { + return c.region +} + +// AzureRetailPrice represents pricing information from the Azure Retail Prices API. +type AzureRetailPrice struct { + Items []struct { + CurrencyCode string `json:"currencyCode"` + RetailPrice float64 `json:"retailPrice"` + UnitPrice float64 `json:"unitPrice"` + ArmRegionName string `json:"armRegionName"` + ProductName string `json:"productName"` + ServiceName string `json:"serviceName"` + ArmSKUName string `json:"armSkuName"` + MeterName string `json:"meterName"` + ReservationTerm string `json:"reservationTerm"` + Type string `json:"type"` + } `json:"Items"` + NextPageLink string `json:"NextPageLink"` + Count int `json:"Count"` +} + +// GetRecommendations gets Redis Cache reservation recommendations from the Azure Consumption API. +func (c *ManagedRedisClient) GetRecommendations(ctx context.Context, params common.RecommendationParams) ([]common.Recommendation, error) { + recommendations := make([]common.Recommendation, 0) + + var pager RecommendationsPager + if c.recommendationsPager != nil { + pager = c.recommendationsPager + } else { + client, err := armconsumption.NewReservationRecommendationsClient(c.cred, nil) + 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{}) + } + + for pager.More() { + page, err := pager.NextPage(ctx) + if err != nil { + return nil, fmt.Errorf("failed to get Redis Cache recommendations: %w", err) + } + + for _, rec := range page.Value { + converted := c.convertRecommendation(ctx, rec) + if converted != nil { + recommendations = append(recommendations, *converted) + } + } + } + + return recommendations, nil +} + +// GetExistingCommitments retrieves existing Azure Cache for Redis reserved capacity. +func (c *ManagedRedisClient) GetExistingCommitments(ctx context.Context) ([]common.Commitment, error) { + commitments := make([]common.Commitment, 0) + + pager, err := c.reservationDetailsPager() + if err != nil { + return commitments, nil + } + + for pager.More() { + page, err := pager.NextPage(ctx) + if err != nil { + break + } + for _, detail := range page.Value { + if cm := c.reservationDetailToCommitment(detail); cm != nil { + commitments = append(commitments, *cm) + } + } + } + + return commitments, nil +} + +// reservationDetailsPager returns the pager to use for reservation details, +// preferring the injected mock over a real SDK pager. +func (c *ManagedRedisClient) reservationDetailsPager() (ReservationsDetailsPager, error) { + if c.reservationsPager != nil { + return c.reservationsPager, nil + } + client, err := armconsumption.NewReservationsDetailsClient(c.cred, nil) + if err != nil { + return nil, err + } + scope := fmt.Sprintf("subscriptions/%s", c.subscriptionID) + return client.NewListByReservationOrderPager(scope, "00000000-0000-0000-0000-000000000000", + &armconsumption.ReservationsDetailsClientListByReservationOrderOptions{}), nil +} + +// reservationDetailToCommitment converts a single reservation detail to a Commitment. +// Returns nil when the detail should be skipped (nil properties, non-Redis SKU). +func (c *ManagedRedisClient) reservationDetailToCommitment(detail *armconsumption.ReservationDetail) *common.Commitment { + if detail == nil || detail.Properties == nil { + return nil + } + props := detail.Properties + if props.SKUName == nil || !strings.Contains(strings.ToLower(*props.SKUName), "redis") { + return nil + } + cm := &common.Commitment{ + Provider: common.ProviderAzure, + Account: c.subscriptionID, + CommitmentType: common.CommitmentReservedInstance, + Service: common.ServiceMemoryDB, + Region: c.region, + State: "active", + } + if props.ReservationID != nil { + cm.CommitmentID = *props.ReservationID + } + cm.ResourceType = *props.SKUName + return cm +} + +// PurchaseCommitment purchases Azure Cache for Redis reserved capacity via the Azure Reservations API. +func (c *ManagedRedisClient) PurchaseCommitment(ctx context.Context, rec common.Recommendation, _ common.PurchaseOptions) (common.PurchaseResult, error) { + result := common.PurchaseResult{ + Recommendation: rec, + DryRun: false, + Success: false, + Timestamp: time.Now(), + } + + reservationOrderID := fmt.Sprintf("redis-managed-reservation-%d", time.Now().Unix()) + apiVersion := "2022-11-01" + purchaseURL := fmt.Sprintf("https://management.azure.com/providers/Microsoft.Capacity/reservationOrders/%s?api-version=%s", + reservationOrderID, apiVersion) + + termYears := 1 + if rec.Term == "3yr" || rec.Term == "3" { + termYears = 3 + } + + requestBody := map[string]interface{}{ + "sku": map[string]string{ + "name": rec.ResourceType, + }, + "location": c.region, + "properties": map[string]interface{}{ + "reservedResourceType": "RedisCache", + "billingScopeId": fmt.Sprintf("/subscriptions/%s", c.subscriptionID), + "term": fmt.Sprintf("P%dY", termYears), + "quantity": rec.Count, + "displayName": fmt.Sprintf("Azure Cache for Redis Reservation - %s", rec.ResourceType), + "appliedScopeType": "Shared", + "renew": false, + }, + } + + bodyBytes, err := json.Marshal(requestBody) + if err != nil { + result.Error = fmt.Errorf("failed to marshal request: %w", err) + return result, result.Error + } + + req, err := http.NewRequestWithContext(ctx, "PUT", purchaseURL, strings.NewReader(string(bodyBytes))) + if err != nil { + result.Error = fmt.Errorf("failed to create request: %w", err) + return result, result.Error + } + + token, err := c.cred.GetToken(ctx, policy.TokenRequestOptions{ + Scopes: []string{"https://management.azure.com/.default"}, + }) + if err != nil { + result.Error = fmt.Errorf("failed to get access token: %w", err) + return result, result.Error + } + + req.Header.Set("Authorization", "Bearer "+token.Token) + req.Header.Set("Content-Type", "application/json") + + resp, err := c.httpClient.Do(req) + if err != nil { + result.Error = fmt.Errorf("failed to purchase reservation: %w", err) + return result, result.Error + } + defer resp.Body.Close() + + body, _ := io.ReadAll(resp.Body) + + if resp.StatusCode != http.StatusOK && resp.StatusCode != http.StatusCreated && resp.StatusCode != http.StatusAccepted { + result.Error = fmt.Errorf("reservation purchase failed with status %d: %s", resp.StatusCode, string(body)) + return result, result.Error + } + + result.Success = true + result.CommitmentID = reservationOrderID + result.Cost = rec.CommitmentCost + + return result, nil +} + +// ValidateOffering validates that the given Redis Cache SKU is known. +func (c *ManagedRedisClient) ValidateOffering(ctx context.Context, rec common.Recommendation) error { + validSKUs, err := c.GetValidResourceTypes(ctx) + if err != nil { + return fmt.Errorf("failed to get valid SKUs: %w", err) + } + + for _, sku := range validSKUs { + if sku == rec.ResourceType { + return nil + } + } + + return fmt.Errorf("invalid Azure Cache for Redis SKU: %s", rec.ResourceType) +} + +// GetOfferingDetails retrieves reservation offering details from the Azure Retail Prices API. +func (c *ManagedRedisClient) GetOfferingDetails(ctx context.Context, rec common.Recommendation) (*common.OfferingDetails, error) { + termYears := 1 + if rec.Term == "3yr" || rec.Term == "3" { + termYears = 3 + } + + pricing, err := c.getRedisPricing(ctx, rec.ResourceType, c.region, termYears) + if err != nil { + return nil, fmt.Errorf("failed to get pricing: %w", err) + } + + var upfrontCost, recurringCost float64 + totalCost := pricing.ReservationPrice + + switch rec.PaymentOption { + case "all-upfront", "upfront": + upfrontCost = totalCost + recurringCost = 0 + case "monthly", "no-upfront": + upfrontCost = 0 + recurringCost = totalCost / (float64(termYears) * 12) + default: + upfrontCost = totalCost + } + + return &common.OfferingDetails{ + OfferingID: fmt.Sprintf("azure-managed-redis-%s-%s-%s", rec.ResourceType, c.region, rec.Term), + ResourceType: rec.ResourceType, + Term: rec.Term, + PaymentOption: rec.PaymentOption, + UpfrontCost: upfrontCost, + RecurringCost: recurringCost, + TotalCost: totalCost, + EffectiveHourlyRate: pricing.HourlyRate, + Currency: pricing.Currency, + }, nil +} + +// GetValidResourceTypes returns Redis Cache SKUs discovered from the subscription, or a +// curated fallback list when the subscription API is unreachable. +func (c *ManagedRedisClient) GetValidResourceTypes(ctx context.Context) ([]string, error) { + pager, err := c.redisCacheListPager() + if err != nil { + return c.commonSKUs(), nil + } + + skuSet := collectSKUsFromPager(ctx, pager) + if len(skuSet) > 0 { + skus := make([]string, 0, len(skuSet)) + for sku := range skuSet { + skus = append(skus, sku) + } + return skus, nil + } + return c.commonSKUs(), nil +} + +// redisCacheListPager returns the pager to use for listing Redis caches, +// preferring the injected mock over a real SDK pager. +func (c *ManagedRedisClient) redisCacheListPager() (RedisCachesPager, error) { + if c.redisCachesPager != nil { + return c.redisCachesPager, nil + } + client, err := armredis.NewClient(c.subscriptionID, c.cred, nil) + if err != nil { + return nil, err + } + return client.NewListBySubscriptionPager(nil), nil +} + +// collectSKUsFromPager iterates the pager and returns the set of full SKU +// names (e.g. "Premium_P1") built from each cache's Name/Family/Capacity. +func collectSKUsFromPager(ctx context.Context, pager RedisCachesPager) map[string]bool { + skuSet := make(map[string]bool) + for pager.More() { + page, err := pager.NextPage(ctx) + if err != nil { + break + } + for _, cache := range page.Value { + if name := extractSKUName(cache); name != "" { + skuSet[name] = true + } + } + } + return skuSet +} + +// extractSKUName derives a full SKU string from a ResourceInfo entry. +// Returns "" when the entry lacks the required fields. +func extractSKUName(cache *armredis.ResourceInfo) string { + if cache == nil || cache.Properties == nil { + return "" + } + sku := cache.Properties.SKU + if sku == nil || sku.Name == nil || sku.Family == nil || sku.Capacity == nil { + return "" + } + return fmt.Sprintf("%s_%s%d", string(*sku.Name), string(*sku.Family), *sku.Capacity) +} + +// commonSKUs returns a curated list of Azure Cache for Redis SKUs that support reservations. +func (c *ManagedRedisClient) commonSKUs() []string { + return []string{ + // Basic tier + "Basic_C0", "Basic_C1", "Basic_C2", "Basic_C3", "Basic_C4", "Basic_C5", "Basic_C6", + // Standard tier + "Standard_C0", "Standard_C1", "Standard_C2", "Standard_C3", "Standard_C4", "Standard_C5", "Standard_C6", + // Premium tier (most commonly reserved) + "Premium_P1", "Premium_P2", "Premium_P3", "Premium_P4", "Premium_P5", + } +} + +// RedisPricing holds pricing details for a given Redis Cache SKU. +type RedisPricing struct { + HourlyRate float64 + ReservationPrice float64 + OnDemandPrice float64 + Currency string + SavingsPercentage float64 +} + +// getRedisPricing fetches pricing from the Azure Retail Prices API. +func (c *ManagedRedisClient) getRedisPricing(ctx context.Context, sku, region string, termYears int) (*RedisPricing, error) { + baseURL := "https://prices.azure.com/api/retail/prices" + + filter := fmt.Sprintf("serviceName eq 'Azure Cache for Redis' and armRegionName eq '%s' and contains(armSkuName, '%s')", + region, sku) + + params := url.Values{} + params.Add("$filter", filter) + params.Add("api-version", "2023-01-01-preview") + + fullURL := baseURL + "?" + params.Encode() + + req, err := http.NewRequestWithContext(ctx, "GET", fullURL, nil) + if err != nil { + return nil, fmt.Errorf("failed to create request: %w", err) + } + + resp, err := c.httpClient.Do(req) + if err != nil { + return nil, fmt.Errorf("failed to call pricing API: %w", err) + } + defer resp.Body.Close() + + if resp.StatusCode != http.StatusOK { + body, _ := io.ReadAll(resp.Body) + return nil, fmt.Errorf("pricing API returned status %d: %s", resp.StatusCode, string(body)) + } + + var priceData AzureRetailPrice + if err := json.NewDecoder(resp.Body).Decode(&priceData); err != nil { + return nil, fmt.Errorf("failed to decode pricing response: %w", err) + } + + if len(priceData.Items) == 0 { + return nil, fmt.Errorf("no pricing data found for Redis Cache SKU %s in region %s", sku, region) + } + + onDemandPrice, reservationPrice, currency := parsePriceItems(priceData.Items, termYears) + + if onDemandPrice == 0 { + return nil, fmt.Errorf("no on-demand pricing found for Redis Cache SKU %s", sku) + } + + hoursInTerm := 8760.0 * float64(termYears) + if reservationPrice == 0 { + // Azure Redis Cache reservations typically offer ~55% savings + reservationPrice = onDemandPrice * hoursInTerm * 0.45 + } + + savingsPct := ((onDemandPrice*hoursInTerm - reservationPrice) / (onDemandPrice * hoursInTerm)) * 100 + + return &RedisPricing{ + HourlyRate: reservationPrice / hoursInTerm, + ReservationPrice: reservationPrice, + OnDemandPrice: onDemandPrice * hoursInTerm, + Currency: currency, + SavingsPercentage: savingsPct, + }, nil +} + +// parsePriceItems extracts on-demand price, reservation price, and currency +// from the flat list returned by the Azure Retail Prices API. +func parsePriceItems(items []struct { + CurrencyCode string `json:"currencyCode"` + RetailPrice float64 `json:"retailPrice"` + UnitPrice float64 `json:"unitPrice"` + ArmRegionName string `json:"armRegionName"` + ProductName string `json:"productName"` + ServiceName string `json:"serviceName"` + ArmSKUName string `json:"armSkuName"` + MeterName string `json:"meterName"` + ReservationTerm string `json:"reservationTerm"` + Type string `json:"type"` +}, termYears int) (onDemand, reservation float64, currency string) { + currency = "USD" + termStr := fmt.Sprintf("%d Years", termYears) + for _, item := range items { + if item.CurrencyCode != "" { + currency = item.CurrencyCode + } + if item.ReservationTerm == termStr { + reservation = item.RetailPrice + } else if item.Type == "Consumption" { + onDemand = item.UnitPrice + } + } + return +} + +// convertRecommendation converts an Azure Consumption API recommendation to the common format. +func (c *ManagedRedisClient) convertRecommendation(_ context.Context, _ armconsumption.ReservationRecommendationClassification) *common.Recommendation { + return &common.Recommendation{ + Provider: common.ProviderAzure, + Service: common.ServiceMemoryDB, + Account: c.subscriptionID, + Region: c.region, + CommitmentType: common.CommitmentReservedInstance, + Timestamp: time.Now(), + Term: "1yr", + PaymentOption: "upfront", + } +} diff --git a/providers/azure/services/managedredis/client_test.go b/providers/azure/services/managedredis/client_test.go new file mode 100644 index 000000000..a462220dc --- /dev/null +++ b/providers/azure/services/managedredis/client_test.go @@ -0,0 +1,571 @@ +package managedredis + +import ( + "bytes" + "context" + "errors" + "io" + "net/http" + "testing" + "time" + + "github.com/Azure/azure-sdk-for-go/sdk/azcore" + "github.com/Azure/azure-sdk-for-go/sdk/azcore/policy" + "github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/consumption/armconsumption" + "github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/redis/armredis/v3" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" + + "github.com/LeanerCloud/CUDly/pkg/common" +) + +// -- mock helpers -- + +type mockRecommendationsPager struct { + pages []armconsumption.ReservationRecommendationsClientListResponse + index int +} + +func (m *mockRecommendationsPager) More() bool { return m.index < len(m.pages) } +func (m *mockRecommendationsPager) NextPage(_ context.Context) (armconsumption.ReservationRecommendationsClientListResponse, error) { + if m.index >= len(m.pages) { + return armconsumption.ReservationRecommendationsClientListResponse{}, errors.New("no more pages") + } + p := m.pages[m.index] + m.index++ + return p, nil +} + +type mockReservationsPager struct { + pages []armconsumption.ReservationsDetailsClientListByReservationOrderResponse + index int + err error +} + +func (m *mockReservationsPager) More() bool { return m.index < len(m.pages) } +func (m *mockReservationsPager) NextPage(_ context.Context) (armconsumption.ReservationsDetailsClientListByReservationOrderResponse, error) { + if m.err != nil { + return armconsumption.ReservationsDetailsClientListByReservationOrderResponse{}, m.err + } + if m.index >= len(m.pages) { + return armconsumption.ReservationsDetailsClientListByReservationOrderResponse{}, errors.New("no more pages") + } + p := m.pages[m.index] + m.index++ + return p, nil +} + +type mockRedisPager struct { + pages []armredis.ClientListBySubscriptionResponse + index int + err error +} + +func (m *mockRedisPager) More() bool { return m.index < len(m.pages) } +func (m *mockRedisPager) NextPage(_ context.Context) (armredis.ClientListBySubscriptionResponse, error) { + if m.err != nil { + return armredis.ClientListBySubscriptionResponse{}, m.err + } + if m.index >= len(m.pages) { + return armredis.ClientListBySubscriptionResponse{}, errors.New("no more pages") + } + p := m.pages[m.index] + m.index++ + return p, nil +} + +type mockHTTPClient struct{ mock.Mock } + +func (m *mockHTTPClient) Do(req *http.Request) (*http.Response, error) { + args := m.Called(req) + if args.Get(0) == nil { + return nil, args.Error(1) + } + return args.Get(0).(*http.Response), args.Error(1) +} + +func fakeHTTPResp(status int, body string) *http.Response { + return &http.Response{ + StatusCode: status, + Body: io.NopCloser(bytes.NewBufferString(body)), + Header: make(http.Header), + } +} + +func samplePricingJSON() string { + return `{ + "Items": [ + { + "currencyCode": "USD", + "retailPrice": 350.0, + "unitPrice": 350.0, + "armRegionName": "eastus", + "productName": "Azure Cache for Redis", + "serviceName": "Azure Cache for Redis", + "armSkuName": "Premium_P1", + "meterName": "P1 Instance", + "reservationTerm": "1 Years", + "type": "Reservation" + }, + { + "currencyCode": "USD", + "retailPrice": 0.125, + "unitPrice": 0.125, + "armRegionName": "eastus", + "productName": "Azure Cache for Redis", + "serviceName": "Azure Cache for Redis", + "armSkuName": "Premium_P1", + "type": "Consumption" + } + ] + }` +} + +type mockTokenCredential struct { + token string + err error +} + +func (m *mockTokenCredential) GetToken(_ context.Context, _ policy.TokenRequestOptions) (azcore.AccessToken, error) { + if m.err != nil { + return azcore.AccessToken{}, m.err + } + return azcore.AccessToken{Token: m.token, ExpiresOn: time.Now().Add(time.Hour)}, nil +} + +// -- constructor tests -- + +func TestNewClient(t *testing.T) { + c := NewClient(nil, "sub-123", "eastus") + require.NotNil(t, c) + assert.Equal(t, "sub-123", c.subscriptionID) + assert.Equal(t, "eastus", c.region) + assert.NotNil(t, c.httpClient) +} + +func TestNewClientWithHTTP(t *testing.T) { + h := &mockHTTPClient{} + c := NewClientWithHTTP(nil, "sub-123", "eastus", h) + require.NotNil(t, c) + assert.Equal(t, h, c.httpClient) +} + +// -- interface method tests -- + +func TestGetServiceType(t *testing.T) { + c := NewClient(nil, "sub", "region") + assert.Equal(t, common.ServiceMemoryDB, c.GetServiceType()) +} + +func TestGetRegion(t *testing.T) { + for _, region := range []string{"eastus", "westeurope", "australiaeast"} { + c := NewClient(nil, "sub", region) + assert.Equal(t, region, c.GetRegion()) + } +} + +// -- GetValidResourceTypes -- + +func TestGetValidResourceTypes_Fallback(t *testing.T) { + c := NewClient(nil, "invalid-sub", "eastus") + skus, err := c.GetValidResourceTypes(nil) + require.NoError(t, err) + require.NotEmpty(t, skus) + assert.Contains(t, skus, "Basic_C0") + assert.Contains(t, skus, "Standard_C1") + assert.Contains(t, skus, "Premium_P1") +} + +func TestGetValidResourceTypes_FromMockPager(t *testing.T) { + c := NewClient(nil, "sub", "eastus") + name := armredis.SKUNamePremium + family := armredis.SKUFamilyP + cap := int32(2) + c.SetRedisCachesPager(&mockRedisPager{ + pages: []armredis.ClientListBySubscriptionResponse{ + {ListResult: armredis.ListResult{Value: []*armredis.ResourceInfo{ + {Properties: &armredis.Properties{SKU: &armredis.SKU{Name: &name, Family: &family, Capacity: &cap}}}, + }}}, + }, + }) + skus, err := c.GetValidResourceTypes(context.Background()) + require.NoError(t, err) + require.Len(t, skus, 1) + assert.Equal(t, "Premium_P2", skus[0]) +} + +func TestGetValidResourceTypes_PagerError_FallsBack(t *testing.T) { + c := NewClient(nil, "sub", "eastus") + c.SetRedisCachesPager(&mockRedisPager{ + pages: []armredis.ClientListBySubscriptionResponse{{}}, + err: errors.New("api error"), + }) + skus, err := c.GetValidResourceTypes(context.Background()) + require.NoError(t, err) + assert.Contains(t, skus, "Premium_P1") +} + +func TestGetValidResourceTypes_EmptyResults_FallsBack(t *testing.T) { + c := NewClient(nil, "sub", "eastus") + c.SetRedisCachesPager(&mockRedisPager{ + pages: []armredis.ClientListBySubscriptionResponse{ + {ListResult: armredis.ListResult{Value: []*armredis.ResourceInfo{}}}, + }, + }) + skus, err := c.GetValidResourceTypes(context.Background()) + require.NoError(t, err) + assert.Contains(t, skus, "Premium_P1") +} + +func TestGetValidResourceTypes_MultipleCaches(t *testing.T) { + c := NewClient(nil, "sub", "eastus") + pName := armredis.SKUNamePremium + sName := armredis.SKUNameStandard + pFam := armredis.SKUFamilyP + cFam := armredis.SKUFamilyC + cap1 := int32(1) + cap2 := int32(3) + c.SetRedisCachesPager(&mockRedisPager{ + pages: []armredis.ClientListBySubscriptionResponse{ + {ListResult: armredis.ListResult{Value: []*armredis.ResourceInfo{ + {Properties: &armredis.Properties{SKU: &armredis.SKU{Name: &pName, Family: &pFam, Capacity: &cap1}}}, + {Properties: &armredis.Properties{SKU: &armredis.SKU{Name: &sName, Family: &cFam, Capacity: &cap2}}}, + }}}, + }, + }) + skus, err := c.GetValidResourceTypes(context.Background()) + require.NoError(t, err) + assert.Len(t, skus, 2) + assert.Contains(t, skus, "Premium_P1") + assert.Contains(t, skus, "Standard_C3") +} + +// -- ValidateOffering -- + +func TestValidateOffering_ValidSKU(t *testing.T) { + c := NewClient(nil, "sub", "eastus") + err := c.ValidateOffering(context.Background(), common.Recommendation{ResourceType: "Premium_P1"}) + assert.NoError(t, err) +} + +func TestValidateOffering_InvalidSKU(t *testing.T) { + c := NewClient(nil, "sub", "eastus") + err := c.ValidateOffering(nil, common.Recommendation{ResourceType: "Bogus_Z99"}) + require.Error(t, err) + assert.Contains(t, err.Error(), "invalid Azure Cache for Redis SKU") +} + +// -- GetRecommendations -- + +func TestGetRecommendations_EmptyPager(t *testing.T) { + c := NewClient(nil, "sub", "eastus") + c.SetRecommendationsPager(&mockRecommendationsPager{ + pages: []armconsumption.ReservationRecommendationsClientListResponse{ + {ReservationRecommendationsListResult: armconsumption.ReservationRecommendationsListResult{ + Value: []armconsumption.ReservationRecommendationClassification{}, + }}, + }, + }) + recs, err := c.GetRecommendations(context.Background(), common.RecommendationParams{}) + require.NoError(t, err) + assert.Empty(t, recs) +} + +func TestGetRecommendations_MultiplePages(t *testing.T) { + c := NewClient(nil, "sub", "eastus") + c.SetRecommendationsPager(&mockRecommendationsPager{ + pages: []armconsumption.ReservationRecommendationsClientListResponse{ + {ReservationRecommendationsListResult: armconsumption.ReservationRecommendationsListResult{ + Value: []armconsumption.ReservationRecommendationClassification{}, + }}, + {ReservationRecommendationsListResult: armconsumption.ReservationRecommendationsListResult{ + Value: []armconsumption.ReservationRecommendationClassification{}, + }}, + }, + }) + recs, err := c.GetRecommendations(context.Background(), common.RecommendationParams{}) + require.NoError(t, err) + assert.Empty(t, recs) +} + +// -- GetExistingCommitments -- + +func TestGetExistingCommitments_Empty(t *testing.T) { + c := NewClient(nil, "sub", "eastus") + commitments, err := c.GetExistingCommitments(context.Background()) + require.NoError(t, err) + assert.Empty(t, commitments) +} + +func TestGetExistingCommitments_RedisSKUIncluded(t *testing.T) { + c := NewClient(nil, "sub", "eastus") + resID := "res-123" + sku := "redis-premium-p1" + c.SetReservationsPager(&mockReservationsPager{ + pages: []armconsumption.ReservationsDetailsClientListByReservationOrderResponse{ + {ReservationDetailsListResult: armconsumption.ReservationDetailsListResult{ + Value: []*armconsumption.ReservationDetail{ + {Properties: &armconsumption.ReservationDetailProperties{ReservationID: &resID, SKUName: &sku}}, + }, + }}, + }, + }) + commitments, err := c.GetExistingCommitments(context.Background()) + require.NoError(t, err) + require.Len(t, commitments, 1) + assert.Equal(t, resID, commitments[0].CommitmentID) + assert.Equal(t, sku, commitments[0].ResourceType) + assert.Equal(t, common.ServiceMemoryDB, commitments[0].Service) + assert.Equal(t, common.ProviderAzure, commitments[0].Provider) +} + +func TestGetExistingCommitments_NonRedisFiltered(t *testing.T) { + c := NewClient(nil, "sub", "eastus") + sqlSKU := "sql-standard-s1" + redisSKU := "redis-premium-p2" + id1 := "res-1" + id2 := "res-2" + c.SetReservationsPager(&mockReservationsPager{ + pages: []armconsumption.ReservationsDetailsClientListByReservationOrderResponse{ + {ReservationDetailsListResult: armconsumption.ReservationDetailsListResult{ + Value: []*armconsumption.ReservationDetail{ + {Properties: &armconsumption.ReservationDetailProperties{ReservationID: &id1, SKUName: &sqlSKU}}, + {Properties: &armconsumption.ReservationDetailProperties{ReservationID: &id2, SKUName: &redisSKU}}, + }, + }}, + }, + }) + commitments, err := c.GetExistingCommitments(context.Background()) + require.NoError(t, err) + require.Len(t, commitments, 1) + assert.Equal(t, id2, commitments[0].CommitmentID) +} + +func TestGetExistingCommitments_NilProperties(t *testing.T) { + c := NewClient(nil, "sub", "eastus") + c.SetReservationsPager(&mockReservationsPager{ + pages: []armconsumption.ReservationsDetailsClientListByReservationOrderResponse{ + {ReservationDetailsListResult: armconsumption.ReservationDetailsListResult{ + Value: []*armconsumption.ReservationDetail{{Properties: nil}}, + }}, + }, + }) + commitments, err := c.GetExistingCommitments(context.Background()) + require.NoError(t, err) + assert.Empty(t, commitments) +} + +func TestGetExistingCommitments_PagerError(t *testing.T) { + c := NewClient(nil, "sub", "eastus") + c.SetReservationsPager(&mockReservationsPager{ + pages: []armconsumption.ReservationsDetailsClientListByReservationOrderResponse{{}}, + err: errors.New("api error"), + }) + commitments, err := c.GetExistingCommitments(context.Background()) + require.NoError(t, err) + assert.Empty(t, commitments) +} + +// -- GetOfferingDetails -- + +func TestGetOfferingDetails_1yr(t *testing.T) { + h := &mockHTTPClient{} + h.On("Do", mock.Anything).Return(fakeHTTPResp(http.StatusOK, samplePricingJSON()), nil) + c := NewClientWithHTTP(nil, "sub", "eastus", h) + details, err := c.GetOfferingDetails(context.Background(), common.Recommendation{ + ResourceType: "Premium_P1", Term: "1yr", PaymentOption: "upfront", + }) + require.NoError(t, err) + require.NotNil(t, details) + assert.Equal(t, "Premium_P1", details.ResourceType) + assert.Equal(t, "1yr", details.Term) + assert.Equal(t, "USD", details.Currency) + assert.Contains(t, details.OfferingID, "azure-managed-redis") +} + +func TestGetOfferingDetails_3yr(t *testing.T) { + h := &mockHTTPClient{} + h.On("Do", mock.Anything).Return(fakeHTTPResp(http.StatusOK, samplePricingJSON()), nil) + c := NewClientWithHTTP(nil, "sub", "eastus", h) + details, err := c.GetOfferingDetails(context.Background(), common.Recommendation{ + ResourceType: "Premium_P1", Term: "3yr", PaymentOption: "monthly", + }) + require.NoError(t, err) + assert.Equal(t, "3yr", details.Term) + assert.Equal(t, float64(0), details.UpfrontCost) + assert.Greater(t, details.RecurringCost, float64(0)) +} + +func TestGetOfferingDetails_NoUpfront(t *testing.T) { + h := &mockHTTPClient{} + h.On("Do", mock.Anything).Return(fakeHTTPResp(http.StatusOK, samplePricingJSON()), nil) + c := NewClientWithHTTP(nil, "sub", "eastus", h) + details, err := c.GetOfferingDetails(context.Background(), common.Recommendation{ + ResourceType: "Premium_P1", Term: "1yr", PaymentOption: "no-upfront", + }) + require.NoError(t, err) + assert.Equal(t, float64(0), details.UpfrontCost) + assert.Greater(t, details.RecurringCost, float64(0)) +} + +func TestGetOfferingDetails_APIError(t *testing.T) { + h := &mockHTTPClient{} + h.On("Do", mock.Anything).Return(fakeHTTPResp(http.StatusInternalServerError, "Internal Server Error"), nil) + c := NewClientWithHTTP(nil, "sub", "eastus", h) + _, err := c.GetOfferingDetails(context.Background(), common.Recommendation{ResourceType: "Premium_P1", Term: "1yr"}) + require.Error(t, err) + assert.Contains(t, err.Error(), "pricing API returned status 500") +} + +func TestGetOfferingDetails_NoPricing(t *testing.T) { + h := &mockHTTPClient{} + h.On("Do", mock.Anything).Return(fakeHTTPResp(http.StatusOK, `{"Items": []}`), nil) + c := NewClientWithHTTP(nil, "sub", "eastus", h) + _, err := c.GetOfferingDetails(context.Background(), common.Recommendation{ResourceType: "Premium_P1", Term: "1yr"}) + require.Error(t, err) + assert.Contains(t, err.Error(), "no pricing data found") +} + +// -- PurchaseCommitment -- + +func TestPurchaseCommitment_Success(t *testing.T) { + h := &mockHTTPClient{} + h.On("Do", mock.Anything).Return(fakeHTTPResp(http.StatusOK, `{"id":"res-123"}`), nil) + cred := &mockTokenCredential{token: "tok"} + c := NewClientWithHTTP(cred, "sub", "eastus", h) + result, err := c.PurchaseCommitment(context.Background(), common.Recommendation{ + ResourceType: "Premium_P1", Term: "1yr", Count: 1, CommitmentCost: 500.0, + }, common.PurchaseOptions{}) + require.NoError(t, err) + assert.True(t, result.Success) + assert.NotEmpty(t, result.CommitmentID) + assert.Equal(t, 500.0, result.Cost) +} + +func TestPurchaseCommitment_3yr(t *testing.T) { + h := &mockHTTPClient{} + h.On("Do", mock.Anything).Return(fakeHTTPResp(http.StatusCreated, `{"id":"res-456"}`), nil) + cred := &mockTokenCredential{token: "tok"} + c := NewClientWithHTTP(cred, "sub", "eastus", h) + result, err := c.PurchaseCommitment(context.Background(), common.Recommendation{ + ResourceType: "Premium_P2", Term: "3yr", Count: 2, CommitmentCost: 1200.0, + }, common.PurchaseOptions{}) + require.NoError(t, err) + assert.True(t, result.Success) +} + +func TestPurchaseCommitment_Accepted(t *testing.T) { + h := &mockHTTPClient{} + h.On("Do", mock.Anything).Return(fakeHTTPResp(http.StatusAccepted, `{"id":"res-789"}`), nil) + cred := &mockTokenCredential{token: "tok"} + c := NewClientWithHTTP(cred, "sub", "eastus", h) + result, err := c.PurchaseCommitment(context.Background(), common.Recommendation{ + ResourceType: "Premium_P1", Term: "1yr", + }, common.PurchaseOptions{}) + require.NoError(t, err) + assert.True(t, result.Success) +} + +func TestPurchaseCommitment_TokenError(t *testing.T) { + h := &mockHTTPClient{} + cred := &mockTokenCredential{err: errors.New("token error")} + c := NewClientWithHTTP(cred, "sub", "eastus", h) + result, err := c.PurchaseCommitment(context.Background(), common.Recommendation{ + ResourceType: "Premium_P1", Term: "1yr", + }, common.PurchaseOptions{}) + require.Error(t, err) + assert.False(t, result.Success) + assert.Contains(t, err.Error(), "failed to get access token") +} + +func TestPurchaseCommitment_HTTPError(t *testing.T) { + h := &mockHTTPClient{} + h.On("Do", mock.Anything).Return(nil, errors.New("network error")) + cred := &mockTokenCredential{token: "tok"} + c := NewClientWithHTTP(cred, "sub", "eastus", h) + result, err := c.PurchaseCommitment(context.Background(), common.Recommendation{ + ResourceType: "Premium_P1", Term: "1yr", + }, common.PurchaseOptions{}) + require.Error(t, err) + assert.False(t, result.Success) + assert.Contains(t, err.Error(), "failed to purchase reservation") +} + +func TestPurchaseCommitment_BadStatus(t *testing.T) { + h := &mockHTTPClient{} + h.On("Do", mock.Anything).Return(fakeHTTPResp(http.StatusBadRequest, `{"error":"bad"}`), nil) + cred := &mockTokenCredential{token: "tok"} + c := NewClientWithHTTP(cred, "sub", "eastus", h) + result, err := c.PurchaseCommitment(context.Background(), common.Recommendation{ + ResourceType: "Premium_P1", Term: "1yr", + }, common.PurchaseOptions{}) + require.Error(t, err) + assert.False(t, result.Success) + assert.Contains(t, err.Error(), "reservation purchase failed with status 400") +} + +// -- setter tests -- + +func TestSetterMethods(t *testing.T) { + c := NewClient(nil, "sub", "eastus") + + recPager := &mockRecommendationsPager{} + c.SetRecommendationsPager(recPager) + assert.Equal(t, recPager, c.recommendationsPager) + + resPager := &mockReservationsPager{} + c.SetReservationsPager(resPager) + assert.Equal(t, resPager, c.reservationsPager) + + redisPager := &mockRedisPager{} + c.SetRedisCachesPager(redisPager) + assert.Equal(t, redisPager, c.redisCachesPager) +} + +// -- commonSKUs coverage -- + +func TestCommonSKUs(t *testing.T) { + c := NewClient(nil, "sub", "eastus") + skus := c.commonSKUs() + assert.Contains(t, skus, "Basic_C0") + assert.Contains(t, skus, "Basic_C6") + assert.Contains(t, skus, "Standard_C0") + assert.Contains(t, skus, "Standard_C6") + assert.Contains(t, skus, "Premium_P1") + assert.Contains(t, skus, "Premium_P5") +} + +// -- convertRecommendation -- + +func TestConvertRecommendation(t *testing.T) { + c := NewClient(nil, "sub-abc", "westeurope") + rec := c.convertRecommendation(context.Background(), nil) + require.NotNil(t, rec) + assert.Equal(t, common.ProviderAzure, rec.Provider) + assert.Equal(t, common.ServiceMemoryDB, rec.Service) + assert.Equal(t, "sub-abc", rec.Account) + assert.Equal(t, "westeurope", rec.Region) + assert.Equal(t, common.CommitmentReservedInstance, rec.CommitmentType) + assert.Equal(t, "1yr", rec.Term) + assert.Equal(t, "upfront", rec.PaymentOption) +} + +// -- AzureRetailPrice struct -- + +func TestAzureRetailPriceStruct(t *testing.T) { + p := AzureRetailPrice{Count: 2} + assert.Equal(t, 2, p.Count) +} + +// -- RedisPricing struct -- + +func TestRedisPricingStruct(t *testing.T) { + p := RedisPricing{ + HourlyRate: 0.5, ReservationPrice: 4380.0, OnDemandPrice: 8760.0, + Currency: "USD", SavingsPercentage: 50.0, + } + assert.Equal(t, 0.5, p.HourlyRate) + assert.Equal(t, "USD", p.Currency) + assert.Equal(t, 50.0, p.SavingsPercentage) +} diff --git a/providers/azure/services_test.go b/providers/azure/services_test.go index c56787e19..4836f3514 100644 --- a/providers/azure/services_test.go +++ b/providers/azure/services_test.go @@ -33,6 +33,14 @@ func TestNewCacheClient(t *testing.T) { assert.Equal(t, "westus2", client.GetRegion()) } +func TestNewManagedRedisClient(t *testing.T) { + client := NewManagedRedisClient(nil, "test-subscription", "eastus") + + require.NotNil(t, client) + assert.Equal(t, common.ServiceMemoryDB, client.GetServiceType()) + assert.Equal(t, "eastus", client.GetRegion()) +} + func TestNewRecommendationsClient(t *testing.T) { client, err := NewRecommendationsClient(nil, "test-subscription") require.NoError(t, err) From 59951983bc27b5ff63928d6f1585ba716f756fdf Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Thu, 21 May 2026 01:23:22 +0200 Subject: [PATCH 2/6] fix(managedredis): address CR findings - error propagation, real order discovery, UUID, recommendation mapping - Propagate pager-creation and NextPage errors in GetExistingCommitments instead of swallowing them (return nil, err rather than empty slice or silent break) - Replace hardcoded zero-UUID order ID in reservationDetailsPager with the subscription-scope NewListPager so all real reservation orders are enumerated; update ReservationsDetailsPager interface to ListResponse (not ByReservationOrder) - Use uuid.New().String() for PurchaseCommitment reservation order ID to avoid second-level collisions under concurrent purchases - Replace static stub in convertRecommendation with recommendations.Extract so all Azure recommendation fields (SKU, count, cost, region, term) are mapped - Update tests: pager mock type, pager-error case now asserts error, new convertRecommendation tests cover nil and legacy recommendation shapes --- .../azure/services/managedredis/client.go | 48 +++++++++++++------ .../services/managedredis/client_test.go | 45 ++++++++++++----- 2 files changed, 66 insertions(+), 27 deletions(-) diff --git a/providers/azure/services/managedredis/client.go b/providers/azure/services/managedredis/client.go index d6ba28af9..af024a5c5 100644 --- a/providers/azure/services/managedredis/client.go +++ b/providers/azure/services/managedredis/client.go @@ -17,8 +17,10 @@ import ( "github.com/Azure/azure-sdk-for-go/sdk/azcore/policy" "github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/consumption/armconsumption" "github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/redis/armredis/v3" + "github.com/google/uuid" "github.com/LeanerCloud/CUDly/pkg/common" + "github.com/LeanerCloud/CUDly/providers/azure/internal/recommendations" ) // HTTPClient interface for HTTP operations (enables mocking) @@ -35,7 +37,7 @@ type RecommendationsPager interface { // ReservationsDetailsPager interface for reservations details pager (enables mocking) type ReservationsDetailsPager interface { More() bool - NextPage(ctx context.Context) (armconsumption.ReservationsDetailsClientListByReservationOrderResponse, error) + NextPage(ctx context.Context) (armconsumption.ReservationsDetailsClientListResponse, error) } // RedisCachesPager interface for Redis caches pager (enables mocking) @@ -159,13 +161,13 @@ func (c *ManagedRedisClient) GetExistingCommitments(ctx context.Context) ([]comm pager, err := c.reservationDetailsPager() if err != nil { - return commitments, nil + return nil, fmt.Errorf("failed to initialize reservations details pager: %w", err) } for pager.More() { page, err := pager.NextPage(ctx) if err != nil { - break + return nil, fmt.Errorf("failed to fetch reservations details page: %w", err) } for _, detail := range page.Value { if cm := c.reservationDetailToCommitment(detail); cm != nil { @@ -179,6 +181,8 @@ func (c *ManagedRedisClient) GetExistingCommitments(ctx context.Context) ([]comm // reservationDetailsPager returns the pager to use for reservation details, // preferring the injected mock over a real SDK pager. +// The subscription-scope NewListPager is used so that all reservation orders +// are queried rather than a single hardcoded order ID. func (c *ManagedRedisClient) reservationDetailsPager() (ReservationsDetailsPager, error) { if c.reservationsPager != nil { return c.reservationsPager, nil @@ -188,8 +192,7 @@ func (c *ManagedRedisClient) reservationDetailsPager() (ReservationsDetailsPager return nil, err } scope := fmt.Sprintf("subscriptions/%s", c.subscriptionID) - return client.NewListByReservationOrderPager(scope, "00000000-0000-0000-0000-000000000000", - &armconsumption.ReservationsDetailsClientListByReservationOrderOptions{}), nil + return client.NewListPager(scope, &armconsumption.ReservationsDetailsClientListOptions{}), nil } // reservationDetailToCommitment converts a single reservation detail to a Commitment. @@ -226,7 +229,7 @@ func (c *ManagedRedisClient) PurchaseCommitment(ctx context.Context, rec common. Timestamp: time.Now(), } - reservationOrderID := fmt.Sprintf("redis-managed-reservation-%d", time.Now().Unix()) + reservationOrderID := uuid.New().String() apiVersion := "2022-11-01" purchaseURL := fmt.Sprintf("https://management.azure.com/providers/Microsoft.Capacity/reservationOrders/%s?api-version=%s", reservationOrderID, apiVersion) @@ -526,15 +529,30 @@ func parsePriceItems(items []struct { } // convertRecommendation converts an Azure Consumption API recommendation to the common format. -func (c *ManagedRedisClient) convertRecommendation(_ context.Context, _ armconsumption.ReservationRecommendationClassification) *common.Recommendation { +// Returns nil when the input is nil or cannot be parsed (e.g. an unsupported SDK Kind). +func (c *ManagedRedisClient) convertRecommendation(_ context.Context, rec armconsumption.ReservationRecommendationClassification) *common.Recommendation { + f := recommendations.Extract(rec) + if f == nil { + return nil + } + var recurringCost float64 + if f.RecurringMonthlyCost != nil { + recurringCost = *f.RecurringMonthlyCost + } return &common.Recommendation{ - Provider: common.ProviderAzure, - Service: common.ServiceMemoryDB, - Account: c.subscriptionID, - Region: c.region, - CommitmentType: common.CommitmentReservedInstance, - Timestamp: time.Now(), - Term: "1yr", - PaymentOption: "upfront", + Provider: common.ProviderAzure, + Service: common.ServiceMemoryDB, + Account: c.subscriptionID, + Region: f.Region, + ResourceType: f.ResourceType, + Count: f.Count, + OnDemandCost: f.OnDemandCost, + CommitmentCost: f.CommitmentCost, + EstimatedSavings: f.EstimatedSavings, + RecurringMonthlyCost: &recurringCost, + CommitmentType: common.CommitmentReservedInstance, + Term: f.Term, + PaymentOption: "upfront", + Timestamp: time.Now(), } } diff --git a/providers/azure/services/managedredis/client_test.go b/providers/azure/services/managedredis/client_test.go index a462220dc..b09268c68 100644 --- a/providers/azure/services/managedredis/client_test.go +++ b/providers/azure/services/managedredis/client_test.go @@ -18,6 +18,7 @@ import ( "github.com/stretchr/testify/require" "github.com/LeanerCloud/CUDly/pkg/common" + "github.com/LeanerCloud/CUDly/providers/azure/mocks" ) // -- mock helpers -- @@ -38,18 +39,18 @@ func (m *mockRecommendationsPager) NextPage(_ context.Context) (armconsumption.R } type mockReservationsPager struct { - pages []armconsumption.ReservationsDetailsClientListByReservationOrderResponse + pages []armconsumption.ReservationsDetailsClientListResponse index int err error } func (m *mockReservationsPager) More() bool { return m.index < len(m.pages) } -func (m *mockReservationsPager) NextPage(_ context.Context) (armconsumption.ReservationsDetailsClientListByReservationOrderResponse, error) { +func (m *mockReservationsPager) NextPage(_ context.Context) (armconsumption.ReservationsDetailsClientListResponse, error) { if m.err != nil { - return armconsumption.ReservationsDetailsClientListByReservationOrderResponse{}, m.err + return armconsumption.ReservationsDetailsClientListResponse{}, m.err } if m.index >= len(m.pages) { - return armconsumption.ReservationsDetailsClientListByReservationOrderResponse{}, errors.New("no more pages") + return armconsumption.ReservationsDetailsClientListResponse{}, errors.New("no more pages") } p := m.pages[m.index] m.index++ @@ -293,6 +294,7 @@ func TestGetRecommendations_MultiplePages(t *testing.T) { func TestGetExistingCommitments_Empty(t *testing.T) { c := NewClient(nil, "sub", "eastus") + c.SetReservationsPager(&mockReservationsPager{}) commitments, err := c.GetExistingCommitments(context.Background()) require.NoError(t, err) assert.Empty(t, commitments) @@ -303,7 +305,7 @@ func TestGetExistingCommitments_RedisSKUIncluded(t *testing.T) { resID := "res-123" sku := "redis-premium-p1" c.SetReservationsPager(&mockReservationsPager{ - pages: []armconsumption.ReservationsDetailsClientListByReservationOrderResponse{ + pages: []armconsumption.ReservationsDetailsClientListResponse{ {ReservationDetailsListResult: armconsumption.ReservationDetailsListResult{ Value: []*armconsumption.ReservationDetail{ {Properties: &armconsumption.ReservationDetailProperties{ReservationID: &resID, SKUName: &sku}}, @@ -327,7 +329,7 @@ func TestGetExistingCommitments_NonRedisFiltered(t *testing.T) { id1 := "res-1" id2 := "res-2" c.SetReservationsPager(&mockReservationsPager{ - pages: []armconsumption.ReservationsDetailsClientListByReservationOrderResponse{ + pages: []armconsumption.ReservationsDetailsClientListResponse{ {ReservationDetailsListResult: armconsumption.ReservationDetailsListResult{ Value: []*armconsumption.ReservationDetail{ {Properties: &armconsumption.ReservationDetailProperties{ReservationID: &id1, SKUName: &sqlSKU}}, @@ -345,7 +347,7 @@ func TestGetExistingCommitments_NonRedisFiltered(t *testing.T) { func TestGetExistingCommitments_NilProperties(t *testing.T) { c := NewClient(nil, "sub", "eastus") c.SetReservationsPager(&mockReservationsPager{ - pages: []armconsumption.ReservationsDetailsClientListByReservationOrderResponse{ + pages: []armconsumption.ReservationsDetailsClientListResponse{ {ReservationDetailsListResult: armconsumption.ReservationDetailsListResult{ Value: []*armconsumption.ReservationDetail{{Properties: nil}}, }}, @@ -359,12 +361,12 @@ func TestGetExistingCommitments_NilProperties(t *testing.T) { func TestGetExistingCommitments_PagerError(t *testing.T) { c := NewClient(nil, "sub", "eastus") c.SetReservationsPager(&mockReservationsPager{ - pages: []armconsumption.ReservationsDetailsClientListByReservationOrderResponse{{}}, + pages: []armconsumption.ReservationsDetailsClientListResponse{{}}, err: errors.New("api error"), }) - commitments, err := c.GetExistingCommitments(context.Background()) - require.NoError(t, err) - assert.Empty(t, commitments) + _, err := c.GetExistingCommitments(context.Background()) + require.Error(t, err) + assert.Contains(t, err.Error(), "failed to fetch reservations details page") } // -- GetOfferingDetails -- @@ -538,17 +540,36 @@ func TestCommonSKUs(t *testing.T) { // -- convertRecommendation -- -func TestConvertRecommendation(t *testing.T) { +func TestConvertRecommendation_nil(t *testing.T) { c := NewClient(nil, "sub-abc", "westeurope") rec := c.convertRecommendation(context.Background(), nil) + assert.Nil(t, rec, "nil input should return nil") +} + +func TestConvertRecommendation_legacy(t *testing.T) { + c := NewClient(nil, "sub-abc", "westeurope") + azRec := mocks.BuildLegacyReservationRecommendation( + mocks.WithRegion("westeurope"), + mocks.WithNormalizedSize("Premium_P1"), + mocks.WithQuantity(2), + mocks.WithCosts(1000.0, 700.0, 300.0), + ) + rec := c.convertRecommendation(context.Background(), azRec) require.NotNil(t, rec) assert.Equal(t, common.ProviderAzure, rec.Provider) assert.Equal(t, common.ServiceMemoryDB, rec.Service) assert.Equal(t, "sub-abc", rec.Account) assert.Equal(t, "westeurope", rec.Region) + assert.Equal(t, "Premium_P1", rec.ResourceType) + assert.Equal(t, 2, rec.Count) assert.Equal(t, common.CommitmentReservedInstance, rec.CommitmentType) assert.Equal(t, "1yr", rec.Term) assert.Equal(t, "upfront", rec.PaymentOption) + assert.InDelta(t, 1000.0, rec.OnDemandCost, 0.01) + assert.InDelta(t, 700.0, rec.CommitmentCost, 0.01) + assert.InDelta(t, 300.0, rec.EstimatedSavings, 0.01) + require.NotNil(t, rec.RecurringMonthlyCost) + assert.Equal(t, 0.0, *rec.RecurringMonthlyCost) } // -- AzureRetailPrice struct -- From 101f18b93e8749ff9cfc69c0789f7932dec7b100 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Thu, 21 May 2026 14:10:25 +0200 Subject: [PATCH 3/6] fix(managedredis): address second CR pass - pagination + nil context - Follow NextPageLink in getRedisPricing so all pricing pages are consumed rather than only the first response - Replace nil context with context.Background() in TestGetValidResourceTypes_Fallback - Add TestGetOfferingDetails_Paginated to exercise the multi-page fetch path end-to-end --- .../azure/services/managedredis/client.go | 50 ++++++++++------- .../services/managedredis/client_test.go | 53 ++++++++++++++++++- 2 files changed, 82 insertions(+), 21 deletions(-) diff --git a/providers/azure/services/managedredis/client.go b/providers/azure/services/managedredis/client.go index af024a5c5..cae748f65 100644 --- a/providers/azure/services/managedredis/client.go +++ b/providers/azure/services/managedredis/client.go @@ -438,7 +438,8 @@ type RedisPricing struct { SavingsPercentage float64 } -// getRedisPricing fetches pricing from the Azure Retail Prices API. +// getRedisPricing fetches pricing from the Azure Retail Prices API, following +// pagination via NextPageLink until all pages are consumed. func (c *ManagedRedisClient) getRedisPricing(ctx context.Context, sku, region string, termYears int) (*RedisPricing, error) { baseURL := "https://prices.azure.com/api/retail/prices" @@ -449,34 +450,43 @@ func (c *ManagedRedisClient) getRedisPricing(ctx context.Context, sku, region st params.Add("$filter", filter) params.Add("api-version", "2023-01-01-preview") - fullURL := baseURL + "?" + params.Encode() + nextURL := baseURL + "?" + params.Encode() - req, err := http.NewRequestWithContext(ctx, "GET", fullURL, nil) - if err != nil { - return nil, fmt.Errorf("failed to create request: %w", err) - } + var accumulated AzureRetailPrice - resp, err := c.httpClient.Do(req) - if err != nil { - return nil, fmt.Errorf("failed to call pricing API: %w", err) - } - defer resp.Body.Close() + for nextURL != "" { + req, err := http.NewRequestWithContext(ctx, "GET", nextURL, nil) + if err != nil { + return nil, fmt.Errorf("failed to create request: %w", err) + } - if resp.StatusCode != http.StatusOK { - body, _ := io.ReadAll(resp.Body) - return nil, fmt.Errorf("pricing API returned status %d: %s", resp.StatusCode, string(body)) - } + resp, err := c.httpClient.Do(req) + if err != nil { + return nil, fmt.Errorf("failed to call pricing API: %w", err) + } + + if resp.StatusCode != http.StatusOK { + body, _ := io.ReadAll(resp.Body) + resp.Body.Close() + return nil, fmt.Errorf("pricing API returned status %d: %s", resp.StatusCode, string(body)) + } + + var page AzureRetailPrice + if err := json.NewDecoder(resp.Body).Decode(&page); err != nil { + resp.Body.Close() + return nil, fmt.Errorf("failed to decode pricing response: %w", err) + } + resp.Body.Close() - var priceData AzureRetailPrice - if err := json.NewDecoder(resp.Body).Decode(&priceData); err != nil { - return nil, fmt.Errorf("failed to decode pricing response: %w", err) + accumulated.Items = append(accumulated.Items, page.Items...) + nextURL = page.NextPageLink } - if len(priceData.Items) == 0 { + if len(accumulated.Items) == 0 { return nil, fmt.Errorf("no pricing data found for Redis Cache SKU %s in region %s", sku, region) } - onDemandPrice, reservationPrice, currency := parsePriceItems(priceData.Items, termYears) + onDemandPrice, reservationPrice, currency := parsePriceItems(accumulated.Items, termYears) if onDemandPrice == 0 { return nil, fmt.Errorf("no on-demand pricing found for Redis Cache SKU %s", sku) diff --git a/providers/azure/services/managedredis/client_test.go b/providers/azure/services/managedredis/client_test.go index b09268c68..8d63dfcaf 100644 --- a/providers/azure/services/managedredis/client_test.go +++ b/providers/azure/services/managedredis/client_test.go @@ -170,7 +170,7 @@ func TestGetRegion(t *testing.T) { func TestGetValidResourceTypes_Fallback(t *testing.T) { c := NewClient(nil, "invalid-sub", "eastus") - skus, err := c.GetValidResourceTypes(nil) + skus, err := c.GetValidResourceTypes(context.Background()) require.NoError(t, err) require.NotEmpty(t, skus) assert.Contains(t, skus, "Basic_C0") @@ -429,6 +429,57 @@ func TestGetOfferingDetails_NoPricing(t *testing.T) { assert.Contains(t, err.Error(), "no pricing data found") } +func TestGetOfferingDetails_Paginated(t *testing.T) { + // Page 1: on-demand price only, with a NextPageLink pointing to page 2. + page1 := `{ + "Items": [ + { + "currencyCode": "USD", + "retailPrice": 0.125, + "unitPrice": 0.125, + "armRegionName": "eastus", + "productName": "Azure Cache for Redis", + "serviceName": "Azure Cache for Redis", + "armSkuName": "Premium_P1", + "type": "Consumption" + } + ], + "NextPageLink": "https://prices.azure.com/api/retail/prices?page=2", + "Count": 1 + }` + // Page 2: reservation price only, no further pages. + page2 := `{ + "Items": [ + { + "currencyCode": "USD", + "retailPrice": 350.0, + "unitPrice": 350.0, + "armRegionName": "eastus", + "productName": "Azure Cache for Redis", + "serviceName": "Azure Cache for Redis", + "armSkuName": "Premium_P1", + "meterName": "P1 Instance", + "reservationTerm": "1 Years", + "type": "Reservation" + } + ], + "NextPageLink": "", + "Count": 1 + }` + h := &mockHTTPClient{} + h.On("Do", mock.Anything).Return(fakeHTTPResp(http.StatusOK, page1), nil).Once() + h.On("Do", mock.Anything).Return(fakeHTTPResp(http.StatusOK, page2), nil).Once() + c := NewClientWithHTTP(nil, "sub", "eastus", h) + details, err := c.GetOfferingDetails(context.Background(), common.Recommendation{ + ResourceType: "Premium_P1", Term: "1yr", PaymentOption: "upfront", + }) + require.NoError(t, err) + require.NotNil(t, details) + assert.Equal(t, "USD", details.Currency) + assert.Greater(t, details.TotalCost, float64(0)) + h.AssertExpectations(t) +} + // -- PurchaseCommitment -- func TestPurchaseCommitment_Success(t *testing.T) { From fd519d2022b42757912e7a0374bdfce694890ba0 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Thu, 21 May 2026 14:34:42 +0200 Subject: [PATCH 4/6] fix(providers/azure/managedredis): address CR third-pass production findings - Guard NewClientWithHTTP against a nil HTTPClient by substituting a default *http.Client with a 30s timeout, avoiding nil-deref panics on the first Do call. - Change collectSKUsFromPager to return (map, error) and have GetValidResourceTypes discard partial results and fall back to the curated commonSKUs() list on pager error, preventing false validation failures for valid SKUs. - Pass the converter's *float64 RecurringMonthlyCost through directly so nil (the documented "provider did not return a monthly breakdown" sentinel in pkg/common/types.go) stays distinct from an explicit 0. --- .../azure/services/managedredis/client.go | 28 +++++++++++++------ 1 file changed, 19 insertions(+), 9 deletions(-) diff --git a/providers/azure/services/managedredis/client.go b/providers/azure/services/managedredis/client.go index cae748f65..1f06cf6f6 100644 --- a/providers/azure/services/managedredis/client.go +++ b/providers/azure/services/managedredis/client.go @@ -70,7 +70,12 @@ func NewClient(cred azcore.TokenCredential, subscriptionID, region string) *Mana } // NewClientWithHTTP creates a new ManagedRedisClient with a custom HTTP client (for testing). +// When httpClient is nil, a default *http.Client with a 30s timeout is used to avoid +// nil-deref panics on the first Do call. func NewClientWithHTTP(cred azcore.TokenCredential, subscriptionID, region string, httpClient HTTPClient) *ManagedRedisClient { + if httpClient == nil { + httpClient = &http.Client{Timeout: 30 * time.Second} + } return &ManagedRedisClient{ cred: cred, subscriptionID: subscriptionID, @@ -362,7 +367,12 @@ func (c *ManagedRedisClient) GetValidResourceTypes(ctx context.Context) ([]strin return c.commonSKUs(), nil } - skuSet := collectSKUsFromPager(ctx, pager) + skuSet, err := collectSKUsFromPager(ctx, pager) + if err != nil { + // Discard any partial results and fall back to the curated SKU list + // rather than risk false validation failures for valid SKUs. + return c.commonSKUs(), nil + } if len(skuSet) > 0 { skus := make([]string, 0, len(skuSet)) for sku := range skuSet { @@ -388,12 +398,14 @@ func (c *ManagedRedisClient) redisCacheListPager() (RedisCachesPager, error) { // collectSKUsFromPager iterates the pager and returns the set of full SKU // names (e.g. "Premium_P1") built from each cache's Name/Family/Capacity. -func collectSKUsFromPager(ctx context.Context, pager RedisCachesPager) map[string]bool { +// On a pager error the caller is expected to discard any partial set and +// fall back, so the returned set must be considered invalid when err != nil. +func collectSKUsFromPager(ctx context.Context, pager RedisCachesPager) (map[string]bool, error) { skuSet := make(map[string]bool) for pager.More() { page, err := pager.NextPage(ctx) if err != nil { - break + return nil, err } for _, cache := range page.Value { if name := extractSKUName(cache); name != "" { @@ -401,7 +413,7 @@ func collectSKUsFromPager(ctx context.Context, pager RedisCachesPager) map[strin } } } - return skuSet + return skuSet, nil } // extractSKUName derives a full SKU string from a ResourceInfo entry. @@ -545,10 +557,8 @@ func (c *ManagedRedisClient) convertRecommendation(_ context.Context, rec armcon if f == nil { return nil } - var recurringCost float64 - if f.RecurringMonthlyCost != nil { - recurringCost = *f.RecurringMonthlyCost - } + // Pass the converter's *float64 through directly so nil ("provider API did + // not return a monthly breakdown") stays distinct from an explicit 0. return &common.Recommendation{ Provider: common.ProviderAzure, Service: common.ServiceMemoryDB, @@ -559,7 +569,7 @@ func (c *ManagedRedisClient) convertRecommendation(_ context.Context, rec armcon OnDemandCost: f.OnDemandCost, CommitmentCost: f.CommitmentCost, EstimatedSavings: f.EstimatedSavings, - RecurringMonthlyCost: &recurringCost, + RecurringMonthlyCost: f.RecurringMonthlyCost, CommitmentType: common.CommitmentReservedInstance, Term: f.Term, PaymentOption: "upfront", From 9c74d477b561bb9bef5174feb193968c41d89dec Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Thu, 21 May 2026 14:35:09 +0200 Subject: [PATCH 5/6] test(providers/azure/managedredis): tighten mock + fixture coverage per CR - Register h.AssertExpectations(t) via t.Cleanup on every test using mockHTTPClient (TestGetOfferingDetails_{1yr,3yr,NoUpfront,APIError, NoPricing} and TestPurchaseCommitment_*), so the mocked Do call is required to fire instead of being silently optional. - Add a "3 Years" reservation entry to samplePricingJSON and assert the exact recurring/total cost in TestGetOfferingDetails_3yr so the test actually exercises 3-year term selection rather than just asserting RecurringCost > 0 against the 1-year fixture. - Replace nil context with context.Background() in TestValidateOffering_InvalidSKU for consistency with other tests. --- .../services/managedredis/client_test.go | 31 +++++++++++++++++-- 1 file changed, 29 insertions(+), 2 deletions(-) diff --git a/providers/azure/services/managedredis/client_test.go b/providers/azure/services/managedredis/client_test.go index 8d63dfcaf..84055e059 100644 --- a/providers/azure/services/managedredis/client_test.go +++ b/providers/azure/services/managedredis/client_test.go @@ -109,6 +109,18 @@ func samplePricingJSON() string { "reservationTerm": "1 Years", "type": "Reservation" }, + { + "currencyCode": "USD", + "retailPrice": 900.0, + "unitPrice": 900.0, + "armRegionName": "eastus", + "productName": "Azure Cache for Redis", + "serviceName": "Azure Cache for Redis", + "armSkuName": "Premium_P1", + "meterName": "P1 Instance", + "reservationTerm": "3 Years", + "type": "Reservation" + }, { "currencyCode": "USD", "retailPrice": 0.125, @@ -252,7 +264,7 @@ func TestValidateOffering_ValidSKU(t *testing.T) { func TestValidateOffering_InvalidSKU(t *testing.T) { c := NewClient(nil, "sub", "eastus") - err := c.ValidateOffering(nil, common.Recommendation{ResourceType: "Bogus_Z99"}) + err := c.ValidateOffering(context.Background(), common.Recommendation{ResourceType: "Bogus_Z99"}) require.Error(t, err) assert.Contains(t, err.Error(), "invalid Azure Cache for Redis SKU") } @@ -373,6 +385,7 @@ func TestGetExistingCommitments_PagerError(t *testing.T) { func TestGetOfferingDetails_1yr(t *testing.T) { h := &mockHTTPClient{} + t.Cleanup(func() { h.AssertExpectations(t) }) h.On("Do", mock.Anything).Return(fakeHTTPResp(http.StatusOK, samplePricingJSON()), nil) c := NewClientWithHTTP(nil, "sub", "eastus", h) details, err := c.GetOfferingDetails(context.Background(), common.Recommendation{ @@ -388,6 +401,7 @@ func TestGetOfferingDetails_1yr(t *testing.T) { func TestGetOfferingDetails_3yr(t *testing.T) { h := &mockHTTPClient{} + t.Cleanup(func() { h.AssertExpectations(t) }) h.On("Do", mock.Anything).Return(fakeHTTPResp(http.StatusOK, samplePricingJSON()), nil) c := NewClientWithHTTP(nil, "sub", "eastus", h) details, err := c.GetOfferingDetails(context.Background(), common.Recommendation{ @@ -396,11 +410,16 @@ func TestGetOfferingDetails_3yr(t *testing.T) { require.NoError(t, err) assert.Equal(t, "3yr", details.Term) assert.Equal(t, float64(0), details.UpfrontCost) - assert.Greater(t, details.RecurringCost, float64(0)) + // 3-year fixture: 900.0 reservation price over 36 months = 25.0/month. + // Asserting the exact value confirms the 3-year branch is selected and + // the term-specific pricing item is parsed, not the 1-year one. + assert.InDelta(t, 25.0, details.RecurringCost, 1e-9) + assert.InDelta(t, 900.0, details.TotalCost, 1e-9) } func TestGetOfferingDetails_NoUpfront(t *testing.T) { h := &mockHTTPClient{} + t.Cleanup(func() { h.AssertExpectations(t) }) h.On("Do", mock.Anything).Return(fakeHTTPResp(http.StatusOK, samplePricingJSON()), nil) c := NewClientWithHTTP(nil, "sub", "eastus", h) details, err := c.GetOfferingDetails(context.Background(), common.Recommendation{ @@ -413,6 +432,7 @@ func TestGetOfferingDetails_NoUpfront(t *testing.T) { func TestGetOfferingDetails_APIError(t *testing.T) { h := &mockHTTPClient{} + t.Cleanup(func() { h.AssertExpectations(t) }) h.On("Do", mock.Anything).Return(fakeHTTPResp(http.StatusInternalServerError, "Internal Server Error"), nil) c := NewClientWithHTTP(nil, "sub", "eastus", h) _, err := c.GetOfferingDetails(context.Background(), common.Recommendation{ResourceType: "Premium_P1", Term: "1yr"}) @@ -422,6 +442,7 @@ func TestGetOfferingDetails_APIError(t *testing.T) { func TestGetOfferingDetails_NoPricing(t *testing.T) { h := &mockHTTPClient{} + t.Cleanup(func() { h.AssertExpectations(t) }) h.On("Do", mock.Anything).Return(fakeHTTPResp(http.StatusOK, `{"Items": []}`), nil) c := NewClientWithHTTP(nil, "sub", "eastus", h) _, err := c.GetOfferingDetails(context.Background(), common.Recommendation{ResourceType: "Premium_P1", Term: "1yr"}) @@ -484,6 +505,7 @@ func TestGetOfferingDetails_Paginated(t *testing.T) { func TestPurchaseCommitment_Success(t *testing.T) { h := &mockHTTPClient{} + t.Cleanup(func() { h.AssertExpectations(t) }) h.On("Do", mock.Anything).Return(fakeHTTPResp(http.StatusOK, `{"id":"res-123"}`), nil) cred := &mockTokenCredential{token: "tok"} c := NewClientWithHTTP(cred, "sub", "eastus", h) @@ -498,6 +520,7 @@ func TestPurchaseCommitment_Success(t *testing.T) { func TestPurchaseCommitment_3yr(t *testing.T) { h := &mockHTTPClient{} + t.Cleanup(func() { h.AssertExpectations(t) }) h.On("Do", mock.Anything).Return(fakeHTTPResp(http.StatusCreated, `{"id":"res-456"}`), nil) cred := &mockTokenCredential{token: "tok"} c := NewClientWithHTTP(cred, "sub", "eastus", h) @@ -510,6 +533,7 @@ func TestPurchaseCommitment_3yr(t *testing.T) { func TestPurchaseCommitment_Accepted(t *testing.T) { h := &mockHTTPClient{} + t.Cleanup(func() { h.AssertExpectations(t) }) h.On("Do", mock.Anything).Return(fakeHTTPResp(http.StatusAccepted, `{"id":"res-789"}`), nil) cred := &mockTokenCredential{token: "tok"} c := NewClientWithHTTP(cred, "sub", "eastus", h) @@ -522,6 +546,7 @@ func TestPurchaseCommitment_Accepted(t *testing.T) { func TestPurchaseCommitment_TokenError(t *testing.T) { h := &mockHTTPClient{} + t.Cleanup(func() { h.AssertExpectations(t) }) cred := &mockTokenCredential{err: errors.New("token error")} c := NewClientWithHTTP(cred, "sub", "eastus", h) result, err := c.PurchaseCommitment(context.Background(), common.Recommendation{ @@ -534,6 +559,7 @@ func TestPurchaseCommitment_TokenError(t *testing.T) { func TestPurchaseCommitment_HTTPError(t *testing.T) { h := &mockHTTPClient{} + t.Cleanup(func() { h.AssertExpectations(t) }) h.On("Do", mock.Anything).Return(nil, errors.New("network error")) cred := &mockTokenCredential{token: "tok"} c := NewClientWithHTTP(cred, "sub", "eastus", h) @@ -547,6 +573,7 @@ func TestPurchaseCommitment_HTTPError(t *testing.T) { func TestPurchaseCommitment_BadStatus(t *testing.T) { h := &mockHTTPClient{} + t.Cleanup(func() { h.AssertExpectations(t) }) h.On("Do", mock.Anything).Return(fakeHTTPResp(http.StatusBadRequest, `{"error":"bad"}`), nil) cred := &mockTokenCredential{token: "tok"} c := NewClientWithHTTP(cred, "sub", "eastus", h) From fab11b658a36004366536dff1d347755f8628b47 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Thu, 21 May 2026 14:57:29 +0200 Subject: [PATCH 6/6] fix(azure/managed-redis): fail closed on unknown terms and missing reservation price - Add parseTermYears helper (mirrors synapse) to reject any term string outside the explicit "1yr"/"3yr" allowlist instead of silently defaulting to 1 year in PurchaseCommitment and GetOfferingDetails - Remove hardcoded 45% reservation-price fallback in getRedisPricing; return an error when the API returns no reservation price for the requested SKU - Add TestPurchaseCommitment_InvalidTerm and TestGetOfferingDetails_InvalidTerm to exercise the new error paths --- .../azure/services/managedredis/client.go | 34 +++++++++++++------ .../services/managedredis/client_test.go | 23 +++++++++++++ 2 files changed, 47 insertions(+), 10 deletions(-) diff --git a/providers/azure/services/managedredis/client.go b/providers/azure/services/managedredis/client.go index 1f06cf6f6..a13be9e86 100644 --- a/providers/azure/services/managedredis/client.go +++ b/providers/azure/services/managedredis/client.go @@ -225,6 +225,20 @@ func (c *ManagedRedisClient) reservationDetailToCommitment(detail *armconsumptio return cm } +// parseTermYears maps a reservation term string to an integer year count. +// Returns an error for any value outside the explicit allowlist so callers +// fail closed rather than silently coercing to a 1-year purchase. +func parseTermYears(term string) (int, error) { + switch strings.ToLower(strings.TrimSpace(term)) { + case "", "1", "1yr", "1y": + return 1, nil + case "3", "3yr", "3y": + return 3, nil + default: + return 0, fmt.Errorf("unsupported reservation term: %s", term) + } +} + // PurchaseCommitment purchases Azure Cache for Redis reserved capacity via the Azure Reservations API. func (c *ManagedRedisClient) PurchaseCommitment(ctx context.Context, rec common.Recommendation, _ common.PurchaseOptions) (common.PurchaseResult, error) { result := common.PurchaseResult{ @@ -234,16 +248,17 @@ func (c *ManagedRedisClient) PurchaseCommitment(ctx context.Context, rec common. Timestamp: time.Now(), } + termYears, termErr := parseTermYears(rec.Term) + if termErr != nil { + result.Error = termErr + return result, result.Error + } + reservationOrderID := uuid.New().String() apiVersion := "2022-11-01" purchaseURL := fmt.Sprintf("https://management.azure.com/providers/Microsoft.Capacity/reservationOrders/%s?api-version=%s", reservationOrderID, apiVersion) - termYears := 1 - if rec.Term == "3yr" || rec.Term == "3" { - termYears = 3 - } - requestBody := map[string]interface{}{ "sku": map[string]string{ "name": rec.ResourceType, @@ -322,9 +337,9 @@ func (c *ManagedRedisClient) ValidateOffering(ctx context.Context, rec common.Re // GetOfferingDetails retrieves reservation offering details from the Azure Retail Prices API. func (c *ManagedRedisClient) GetOfferingDetails(ctx context.Context, rec common.Recommendation) (*common.OfferingDetails, error) { - termYears := 1 - if rec.Term == "3yr" || rec.Term == "3" { - termYears = 3 + termYears, err := parseTermYears(rec.Term) + if err != nil { + return nil, err } pricing, err := c.getRedisPricing(ctx, rec.ResourceType, c.region, termYears) @@ -506,8 +521,7 @@ func (c *ManagedRedisClient) getRedisPricing(ctx context.Context, sku, region st hoursInTerm := 8760.0 * float64(termYears) if reservationPrice == 0 { - // Azure Redis Cache reservations typically offer ~55% savings - reservationPrice = onDemandPrice * hoursInTerm * 0.45 + return nil, fmt.Errorf("no reservation pricing found for Redis Cache SKU %s (%d year) in region %s", sku, termYears, region) } savingsPct := ((onDemandPrice*hoursInTerm - reservationPrice) / (onDemandPrice * hoursInTerm)) * 100 diff --git a/providers/azure/services/managedredis/client_test.go b/providers/azure/services/managedredis/client_test.go index 84055e059..8384b479b 100644 --- a/providers/azure/services/managedredis/client_test.go +++ b/providers/azure/services/managedredis/client_test.go @@ -585,6 +585,29 @@ func TestPurchaseCommitment_BadStatus(t *testing.T) { assert.Contains(t, err.Error(), "reservation purchase failed with status 400") } +func TestPurchaseCommitment_InvalidTerm(t *testing.T) { + h := &mockHTTPClient{} + t.Cleanup(func() { h.AssertExpectations(t) }) + c := NewClientWithHTTP(nil, "sub", "eastus", h) + result, err := c.PurchaseCommitment(context.Background(), common.Recommendation{ + ResourceType: "Premium_P1", Term: "5yr", Count: 1, + }, common.PurchaseOptions{}) + require.Error(t, err) + assert.False(t, result.Success) + assert.Contains(t, err.Error(), "unsupported reservation term") +} + +func TestGetOfferingDetails_InvalidTerm(t *testing.T) { + h := &mockHTTPClient{} + t.Cleanup(func() { h.AssertExpectations(t) }) + c := NewClientWithHTTP(nil, "sub", "eastus", h) + _, err := c.GetOfferingDetails(context.Background(), common.Recommendation{ + ResourceType: "Premium_P1", Term: "5yr", + }) + require.Error(t, err) + assert.Contains(t, err.Error(), "unsupported reservation term") +} + // -- setter tests -- func TestSetterMethods(t *testing.T) {