chore(ri-exchange): forward the source RI availability zone to the go offering lookups

go.mod

@@ -84,10 +84,10 @@ require (
 	github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/reservations/armreservations v1.1.0
 	github.com/Azure/azure-sdk-for-go/sdk/security/keyvault/azkeys v1.4.0
 	github.com/Azure/azure-sdk-for-go/sdk/security/keyvault/azsecrets v1.4.0
-	github.com/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20261015143912-740dfbd72456
-	github.com/LeanerCloud/cloud-commitments-go/providers/aws v0.0.0-20261015143912-740dfbd72456
-	github.com/LeanerCloud/cloud-commitments-go/providers/azure v0.0.0-20261015143912-740dfbd72456
-	github.com/LeanerCloud/cloud-commitments-go/providers/gcp v0.0.0-20261015143912-740dfbd72456
+	github.com/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20261016145956-551be568dc21
+	github.com/LeanerCloud/cloud-commitments-go/providers/aws v0.0.0-20261016145956-551be568dc21
+	github.com/LeanerCloud/cloud-commitments-go/providers/azure v0.0.0-20261016145956-551be568dc21
+	github.com/LeanerCloud/cloud-commitments-go/providers/gcp v0.0.0-20261016145956-551be568dc21
 	github.com/aws/aws-lambda-go v1.47.0
 	github.com/aws/aws-sdk-go-v2/service/kms v1.50.4
 	github.com/aws/aws-sdk-go-v2/service/lambda v1.89.0

go.sum

@@ -92,14 +92,14 @@ github.com/GoogleCloudPlatform/opentelemetry-operations-go/internal/cloudmock v0
 github.com/GoogleCloudPlatform/opentelemetry-operations-go/internal/cloudmock v0.54.0/go.mod h1:vB2GH9GAYYJTO3mEn8oYwzEdhlayZIdQz6zdzgUIRvA=
 github.com/GoogleCloudPlatform/opentelemetry-operations-go/internal/resourcemapping v0.54.0 h1:s0WlVbf9qpvkh1c/uDAPElam0WrL7fHRIidgZJ7UqZI=
 github.com/GoogleCloudPlatform/opentelemetry-operations-go/internal/resourcemapping v0.54.0/go.mod h1:Mf6O40IAyB9zR/1J8nGDDPirZQQPbYJni8Yisy7NTMc=
-github.com/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20261015143912-740dfbd72456 h1:ZPVVIwrZ28I+f+Fe6DUK6g/nVGIJj06nNhI32M7rt9I=
-github.com/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20261015143912-740dfbd72456/go.mod h1:8RTmzPazA8IeVZfn7wNaU7ILx0VK5eHZQozO21Mvtq8=
-github.com/LeanerCloud/cloud-commitments-go/providers/aws v0.0.0-20261015143912-740dfbd72456 h1:3lNYgah7tF+bnOXv619jgiSguUhpiI2I2sFmLrj7Ba8=
-github.com/LeanerCloud/cloud-commitments-go/providers/aws v0.0.0-20261015143912-740dfbd72456/go.mod h1:uOUuTVRjTm0Fhu/Vra2M2NES0w4pvZUSfZLsGyCd18s=
-github.com/LeanerCloud/cloud-commitments-go/providers/azure v0.0.0-20261015143912-740dfbd72456 h1:agVjEMOqGsehx98Lb4IyIPlCzf6VIBNHjGekUfCxkpU=
-github.com/LeanerCloud/cloud-commitments-go/providers/azure v0.0.0-20261015143912-740dfbd72456/go.mod h1:8vqO2ZlDzjX/Z4bESZVp/IenhTifkySeF0VJwQKihLA=
-github.com/LeanerCloud/cloud-commitments-go/providers/gcp v0.0.0-20261015143912-740dfbd72456 h1:+pjt8F5bHFHz9OFYEOXUESdIE1KVSfdQH3URqsjaIfw=
-github.com/LeanerCloud/cloud-commitments-go/providers/gcp v0.0.0-20261015143912-740dfbd72456/go.mod h1:KV+Ov04+ZMBGv1wA0s9lEHLinlg1p74XrNafXYLwKmE=
+github.com/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20261016145956-551be568dc21 h1:gZSO852B+hqQWi6lSvPaOWFtWigIw3vVr7HDtX6CMR8=
+github.com/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20261016145956-551be568dc21/go.mod h1:SCDEgHG6evjPh0RsoXof+J3aMmGXNFMrwbJIle7rMpQ=
+github.com/LeanerCloud/cloud-commitments-go/providers/aws v0.0.0-20261016145956-551be568dc21 h1:Es0vAPXDxERSD6KGQr/ZUg4aVlBAmMsB2XkosI0IqPI=
+github.com/LeanerCloud/cloud-commitments-go/providers/aws v0.0.0-20261016145956-551be568dc21/go.mod h1:uOUuTVRjTm0Fhu/Vra2M2NES0w4pvZUSfZLsGyCd18s=
+github.com/LeanerCloud/cloud-commitments-go/providers/azure v0.0.0-20261016145956-551be568dc21 h1:PSl5J95u1vS3cvVeci9SkXk/R5iek0NbrxwxxozqyaY=
+github.com/LeanerCloud/cloud-commitments-go/providers/azure v0.0.0-20261016145956-551be568dc21/go.mod h1:8vqO2ZlDzjX/Z4bESZVp/IenhTifkySeF0VJwQKihLA=
+github.com/LeanerCloud/cloud-commitments-go/providers/gcp v0.0.0-20261016145956-551be568dc21 h1:J+c1GcA/i1c3korxqxwkRMOFgJ5OtyeoPxlsU3a3q1U=
+github.com/LeanerCloud/cloud-commitments-go/providers/gcp v0.0.0-20261016145956-551be568dc21/go.mod h1:KV+Ov04+ZMBGv1wA0s9lEHLinlg1p74XrNafXYLwKmE=
 github.com/Microsoft/go-winio v0.6.2 h1:F2VQgta7ecxGYO8k3ZZz3RS8fVIXVxONVUPlNERoyfY=
 github.com/Microsoft/go-winio v0.6.2/go.mod h1:yd8OoFMLzJbo9gZq8j5qaps8bJ9aShtEA8Ipt1oGCvU=
 github.com/aws/aws-lambda-go v1.47.0 h1:0H8s0vumYx/YKs4sE7YM0ktwL2eWse+kfopsRI1sXVI=

internal/api/handler_ri_exchange.go

@@ -156,18 +156,56 @@ func (h *Handler) listTargetOfferings(ctx context.Context, req *events.LambdaFun
 		return nil, NewClientError(404, fmt.Sprintf("source RI %q not found in region %s", sourceRIID, cfg.Region))
 	}
 
+	offerings, err := targetOfferingsForSource(ctx, ec2Client, sourceRI)
+	if err != nil {
+		return nil, err
+	}
+
+	return &TargetOfferingsResponse{Offerings: offerings}, nil
+}
+
+// targetOfferingsForSource lists the exchange targets for sourceRI, keeping a
+// zonal RI's targets in its own AZ. A zone refusal (the source RI's scope and
+// AZ disagree) is a 422.
+func targetOfferingsForSource(ctx context.Context, ec2Client targetOfferingsEC2Client, sourceRI *ec2svc.ConvertibleRI) ([]ec2svc.TargetOffering, error) {
 	offerings, err := ec2Client.ListTargetOfferings(ctx, ec2svc.ListTargetOfferingsParams{
 		ProductDescription: sourceRI.ProductDescription,
 		Tenancy:            sourceRI.InstanceTenancy,
 		Scope:              sourceRI.Scope,
+		AvailabilityZone:   sourceRI.AvailabilityZone,
 		Duration:           sourceRI.Duration,
 		OfferingType:       sourceRI.OfferingType,
 	})
 	if err != nil {
+		if isOfferingZoneRefusal(err) {
+			return nil, NewClientError(422, fmt.Sprintf("source RI %q: %v", sourceRI.ReservedInstanceID, err))
+		}
 		return nil, fmt.Errorf("failed to list target offerings: %w", err)
 	}
+	return offerings, nil
+}
 
-	return &TargetOfferingsResponse{Offerings: offerings}, nil
+// offeringZoneRefusals are the messages the EC2 provider returns when it
+// refuses a zone-inconsistent offering lookup before calling AWS: a zonal RI
+// without an AZ, or a regional RI that names one. The provider exports no
+// sentinel, so these are matched by text; the target-offerings tests run the
+// real provider client so a reworded message fails them.
+var offeringZoneRefusals = []string{
+	"zonal RI offering needs an availability zone",
+	"regional RI offering must not name an availability zone",
+}
+
+// isOfferingZoneRefusal reports whether err is the provider's zone refusal.
+// It is the source RI's data that is inconsistent, so the caller maps it to a
+// 4xx rather than a 500.
+func isOfferingZoneRefusal(err error) bool {
+	msg := err.Error()
+	for _, r := range offeringZoneRefusals {
+		if strings.Contains(msg, r) {
+			return true
+		}
+	}
+	return false
 }
 
 // azureExchangeClient is the narrow interface that listExchangeableAzureRIs,

internal/api/target_offerings_zone_test.go

@@ -0,0 +1,121 @@
+package api
+
+import (
+	"context"
+	"testing"
+
+	"github.com/aws/aws-sdk-go-v2/aws"
+	awsec2sdk "github.com/aws/aws-sdk-go-v2/service/ec2"
+	ec2types "github.com/aws/aws-sdk-go-v2/service/ec2/types"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+
+	ec2svc "github.com/LeanerCloud/cloud-commitments-go/providers/aws/services/ec2"
+)
+
+// Issue #921: GET /api/ri-exchange/target-offerings sends the source RI's AZ
+// to the provider, and the provider's zone refusal is a 422. The real
+// ec2svc.Client runs over a fake EC2 API: no AWS call.
+
+// zoneRecordingTargetsAPI serves the source RI and records the AZ of every
+// offering lookup. The embedded nil API panics on any other call.
+type zoneRecordingTargetsAPI struct {
+	ec2svc.API
+	ri          ec2types.ReservedInstances
+	lookupZones []*string
+}
+
+func (a *zoneRecordingTargetsAPI) DescribeReservedInstances(context.Context, *awsec2sdk.DescribeReservedInstancesInput, ...func(*awsec2sdk.Options)) (*awsec2sdk.DescribeReservedInstancesOutput, error) {
+	return &awsec2sdk.DescribeReservedInstancesOutput{ReservedInstances: []ec2types.ReservedInstances{a.ri}}, nil
+}
+
+func (a *zoneRecordingTargetsAPI) DescribeReservedInstancesOfferings(_ context.Context, in *awsec2sdk.DescribeReservedInstancesOfferingsInput, _ ...func(*awsec2sdk.Options)) (*awsec2sdk.DescribeReservedInstancesOfferingsOutput, error) {
+	a.lookupZones = append(a.lookupZones, in.AvailabilityZone)
+	return &awsec2sdk.DescribeReservedInstancesOfferingsOutput{
+		ReservedInstancesOfferings: []ec2types.ReservedInstancesOffering{{
+			ReservedInstancesOfferingId: aws.String("4b2293b4-5fbc-4017-9c75-d5a9d3aa8c91"),
+			InstanceType:                ec2types.InstanceType("m5.xlarge"),
+			AvailabilityZone:            in.AvailabilityZone,
+		}},
+	}, nil
+}
+
+func runTargetOfferingsForZone(t *testing.T, scope ec2types.Scope, az string) (any, *zoneRecordingTargetsAPI, error) {
+	t.Helper()
+	ri := ec2types.ReservedInstances{
+		ReservedInstancesId: aws.String(targetOfferingsSourceRI),
+		InstanceType:        ec2types.InstanceType("m5.large"),
+		InstanceCount:       aws.Int32(1),
+		ProductDescription:  ec2types.RIProductDescription("Linux/UNIX"),
+		InstanceTenancy:     ec2types.TenancyDefault,
+		Scope:               scope,
+		Duration:            aws.Int64(31536000),
+		OfferingType:        ec2types.OfferingTypeValuesNoUpfront,
+		OfferingClass:       ec2types.OfferingClassTypeConvertible,
+	}
+	if az != "" {
+		ri.AvailabilityZone = aws.String(az)
+	}
+	api := &zoneRecordingTargetsAPI{ri: ri}
+	h := targetOfferingsHandler(t, scopedOutAccount, false)
+	h.targetOfferingsEC2Factory = func(cfg aws.Config) targetOfferingsEC2Client {
+		c := ec2svc.NewClient(cfg)
+		c.SetEC2API(api)
+		return c
+	}
+	res, err := NewRouter(h).Route(context.Background(), "GET", "/api/ri-exchange/target-offerings", targetOfferingsRequest())
+	return res, api, err
+}
+
+func TestListTargetOfferings_ZonalSourceForwardsAvailabilityZone(t *testing.T) {
+	res, api, err := runTargetOfferingsForZone(t, ec2types.ScopeAvailabilityZone, "us-east-1b")
+
+	require.NoError(t, err)
+	require.NotEmpty(t, api.lookupZones)
+	for _, z := range api.lookupZones {
+		assert.Equal(t, "us-east-1b", aws.ToString(z))
+	}
+	resp, ok := res.(*TargetOfferingsResponse)
+	require.True(t, ok)
+	assert.Len(t, resp.Offerings, 1)
+}
+
+func TestListTargetOfferings_RegionalSourcePassesNoAvailabilityZone(t *testing.T) {
+	_, api, err := runTargetOfferingsForZone(t, ec2types.ScopeRegional, "")
+
+	require.NoError(t, err)
+	require.NotEmpty(t, api.lookupZones)
+	for _, z := range api.lookupZones {
+		assert.Nil(t, z)
+	}
+}
+
+func TestListTargetOfferings_ZonalSourceWithoutAZIs422(t *testing.T) {
+	_, api, err := runTargetOfferingsForZone(t, ec2types.ScopeAvailabilityZone, "")
+
+	require.Error(t, err)
+	ce, ok := IsClientError(err)
+	require.True(t, ok, "zone refusal must be a client error, got %v", err)
+	assert.Equal(t, 422, ce.code)
+	assert.Contains(t, ce.message, "zonal RI offering needs an availability zone")
+	assert.Contains(t, ce.message, targetOfferingsSourceRI)
+	assert.Empty(t, api.lookupZones, "no AWS offering lookup without an AZ")
+}
+
+func TestIsOfferingZoneRefusal(t *testing.T) {
+	assert.False(t, isOfferingZoneRefusal(assert.AnError))
+	for _, msg := range offeringZoneRefusals {
+		assert.True(t, isOfferingZoneRefusal(&clientError{message: "wrapped: " + msg}), msg)
+	}
+}
+
+func TestListTargetOfferings_RegionalSourceWithAZIs422(t *testing.T) {
+	_, api, err := runTargetOfferingsForZone(t, ec2types.ScopeRegional, "us-east-1b")
+
+	require.Error(t, err)
+	ce, ok := IsClientError(err)
+	require.True(t, ok, "zone refusal must be a client error, got %v", err)
+	assert.Equal(t, 422, ce.code)
+	assert.Contains(t, ce.message, "regional RI offering must not name an availability zone")
+	assert.Empty(t, api.lookupZones)
+}

internal/server/handler_ri_exchange.go

@@ -26,7 +26,7 @@ type riExchangeClients struct {
 	listConvertibleRIs func(ctx context.Context) ([]ec2svc.ConvertibleRI, error)
 	getRIUtilization   func(ctx context.Context, lookbackDays int) ([]recommendations.RIUtilization, error)
 	exchangeClient     exchange.ExchangeClientInterface
-	lookupOffering     func(ctx context.Context, instanceType, productDesc, tenancy, scope string, duration int64) (string, error)
+	lookupOffering     exchange.LookupOfferingFunc
 	accountID          string
 	region             string
 }
@@ -59,22 +59,37 @@ func (app *Application) handleRIExchangeReshape(ctx context.Context) (*exchange.
 			return recsClient.GetRIUtilization(ctx, lookbackDays, awsCfg.Region)
 		},
 		exchangeClient: exchange.NewExchangeClient(awsCfg),
-		lookupOffering: func(ctx context.Context, instanceType, productDesc, tenancy, scope string, duration int64) (string, error) {
-			return ec2Client.FindConvertibleOffering(ctx, ec2svc.FindConvertibleOfferingParams{
-				InstanceType:       instanceType,
-				ProductDescription: productDesc,
-				Tenancy:            tenancy,
-				Scope:              scope,
-				Duration:           duration,
-			})
-		},
-		accountID: resolveAccountID(ctx, awsCfg),
-		region:    awsCfg.Region,
+		lookupOffering: newConvertibleOfferingLookup(ec2Client),
+		accountID:      resolveAccountID(ctx, awsCfg),
+		region:         awsCfg.Region,
 	}
 
 	return app.executeRIExchangeReshape(ctx, cfg, clients)
 }
 
+// convertibleOfferingFinder is the slice of the EC2 client the offering
+// lookup needs. Satisfied by *ec2svc.Client.
+type convertibleOfferingFinder interface {
+	FindConvertibleOffering(ctx context.Context, params ec2svc.FindConvertibleOfferingParams) (string, error)
+}
+
+// newConvertibleOfferingLookup adapts the EC2 client to exchange.LookupOfferingFunc.
+// AvailabilityZone is the source RI's AZ (empty for regional RIs); the provider
+// refuses a zonal lookup without one, so a zonal RI never exchanges into an
+// offering in another zone.
+func newConvertibleOfferingLookup(finder convertibleOfferingFinder) exchange.LookupOfferingFunc {
+	return func(ctx context.Context, p exchange.LookupOfferingParams) (string, error) {
+		return finder.FindConvertibleOffering(ctx, ec2svc.FindConvertibleOfferingParams{
+			InstanceType:       p.InstanceType,
+			ProductDescription: p.ProductDescription,
+			Tenancy:            p.Tenancy,
+			Scope:              p.Scope,
+			AvailabilityZone:   p.AvailabilityZone,
+			Duration:           p.Duration,
+		})
+	}
+}
+
 // executeRIExchangeReshape contains the core exchange logic, separated from AWS
 // client construction for testability.
 func (app *Application) executeRIExchangeReshape(ctx context.Context, cfg *config.GlobalConfig, clients riExchangeClients) (*exchange.AutoExchangeResult, error) {
@@ -263,6 +278,7 @@ func convertForAutoExchange(instances []ec2svc.ConvertibleRI, utilData []recomme
 			ProductDescription: inst.ProductDescription,
 			InstanceTenancy:    inst.InstanceTenancy,
 			Scope:              inst.Scope,
+			AvailabilityZone:   inst.AvailabilityZone,
 			Duration:           inst.Duration,
 		}
 	}

internal/server/handler_ri_exchange_reservation_integration_test.go

@@ -88,7 +88,7 @@ func TestRIExchangeAutoWorkers_ConcreteStoreAndFixtureClient(t *testing.T) {
 		listConvertibleRIs: func(context.Context) ([]ec2svc.ConvertibleRI, error) { return ri(standaloneRIID), nil },
 		getRIUtilization:   func(context.Context, int) ([]recommendations.RIUtilization, error) { return util(standaloneRIID), nil },
 		exchangeClient:     quoteClient(standaloneRIID, &standaloneExecutions),
-		lookupOffering: func(context.Context, string, string, string, string, int64) (string, error) {
+		lookupOffering: func(context.Context, exchange.LookupOfferingParams) (string, error) {
 			return "offering-test", nil
 		},
 		accountID: "555555555555", region: "us-east-1",
@@ -108,7 +108,7 @@ func TestRIExchangeAutoWorkers_ConcreteStoreAndFixtureClient(t *testing.T) {
 	riInfos, utilInfos, metadata := convertForAutoExchange(ri(ladderRIID), util(ladderRIID))
 	ladder, err := exchange.RunAutoExchange(ctx, exchange.RunAutoExchangeParams{
 		Store: newConfigExchangeStoreAdapter(store), ExchangeClient: quoteClient(ladderRIID, &ladderExecutions),
-		LookupOffering: func(context.Context, string, string, string, string, int64) (string, error) {
+		LookupOffering: func(context.Context, exchange.LookupOfferingParams) (string, error) {
 			return "offering-test", nil
 		},
 		RIs: riInfos, Utilization: utilInfos, RIMetadata: metadata,

internal/server/handler_ri_exchange_test.go

@@ -460,7 +460,7 @@ func TestExecuteRIExchangeReshape_NoConvertibleRIs(t *testing.T) {
 			return nil, nil
 		},
 		exchangeClient: &mockExchangeClient{},
-		lookupOffering: func(ctx context.Context, instanceType, productDesc, tenancy, scope string, duration int64) (string, error) {
+		lookupOffering: func(ctx context.Context, _ exchange.LookupOfferingParams) (string, error) {
 			return "", nil
 		},
 		accountID: "123456789",
@@ -587,7 +587,7 @@ func TestExecuteRIExchangeReshape_ManualMode(t *testing.T) {
 				}, nil
 			},
 		},
-		lookupOffering: func(ctx context.Context, instanceType, productDesc, tenancy, scope string, duration int64) (string, error) {
+		lookupOffering: func(ctx context.Context, _ exchange.LookupOfferingParams) (string, error) {
 			return "offering-123", nil
 		},
 		accountID: "123456789",
@@ -706,7 +706,7 @@ func TestExecuteRIExchangeReshape_AutoMode(t *testing.T) {
 				}, nil
 			},
 		},
-		lookupOffering: func(ctx context.Context, instanceType, productDesc, tenancy, scope string, duration int64) (string, error) {
+		lookupOffering: func(ctx context.Context, _ exchange.LookupOfferingParams) (string, error) {
 			return "offering-456", nil
 		},
 		accountID: "123456789",
@@ -821,7 +821,7 @@ func TestExecuteRIExchangeReshape_DailyCapHitMidRun(t *testing.T) {
 				}, nil
 			},
 		},
-		lookupOffering: func(ctx context.Context, instanceType, productDesc, tenancy, scope string, duration int64) (string, error) {
+		lookupOffering: func(ctx context.Context, _ exchange.LookupOfferingParams) (string, error) {
 			return "offering-cap-test", nil
 		},
 		accountID: "123456789",

internal/server/ladder_write.go

@@ -11,7 +11,6 @@ import (
 	pkgladder "github.com/LeanerCloud/cloud-commitments-go/pkg/ladder"
 	awsprovider "github.com/LeanerCloud/cloud-commitments-go/providers/aws"
 	awsladder "github.com/LeanerCloud/cloud-commitments-go/providers/aws/ladder"
-	ec2svc "github.com/LeanerCloud/cloud-commitments-go/providers/aws/services/ec2"
 )
 
 // exchangeRunnerAdapter bridges internal/server wiring (exchange store, EC2 exchange
@@ -60,20 +59,10 @@ func (a *exchangeRunnerAdapter) RunAutoExchange(ctx context.Context, cfg exchang
 	riInfos, utilInfos, riMetadata := convertForAutoExchange(instances, utilData)
 	store := newConfigExchangeStoreAdapter(a.app.Config)
 
-	lookupOffering := func(ctx context.Context, instanceType, productDesc, tenancy, scope string, duration int64) (string, error) {
-		return ec2Client.FindConvertibleOffering(ctx, ec2svc.FindConvertibleOfferingParams{
-			InstanceType:       instanceType,
-			ProductDescription: productDesc,
-			Tenancy:            tenancy,
-			Scope:              scope,
-			Duration:           duration,
-		})
-	}
-
 	return exchange.RunAutoExchange(ctx, exchange.RunAutoExchangeParams{
 		Store:          store,
 		ExchangeClient: exchange.NewExchangeClient(awsCfg),
-		LookupOffering: lookupOffering,
+		LookupOffering: newConvertibleOfferingLookup(ec2Client),
 		RIs:            riInfos,
 		Utilization:    utilInfos,
 		Config:         cfg,