diff --git a/providers/aws/services/ec2/client.go b/providers/aws/services/ec2/client.go index 3a2ed67e..83b0cb4e 100644 --- a/providers/aws/services/ec2/client.go +++ b/providers/aws/services/ec2/client.go @@ -3,6 +3,7 @@ package ec2 import ( "context" + "errors" "fmt" "log" "math" @@ -422,20 +423,9 @@ func buildEC2OfferingQuery(rec common.Recommendation, details *common.ComputeDet if err != nil { return ec2OfferingQuery{}, fmt.Errorf("EC2 recommendation for %s: %w", rec.ResourceType, err) } - az := strings.TrimSpace(details.AvailabilityZone) - if scope == types.ScopeAvailabilityZone && az == "" { - return ec2OfferingQuery{}, fmt.Errorf( - "EC2 recommendation for %s has zonal scope but no AvailabilityZone: "+ - "refusing to buy a zonal RI in an arbitrary AZ; re-fetch the recommendation", - rec.ResourceType, - ) - } - if scope == types.ScopeRegional && az != "" { - return ec2OfferingQuery{}, fmt.Errorf( - "EC2 recommendation for %s has regional scope but AvailabilityZone %q: "+ - "contradictory input; re-fetch the recommendation", - rec.ResourceType, az, - ) + az, err := resolveExchangeZone(scope, details.AvailabilityZone) + if err != nil { + return ec2OfferingQuery{}, fmt.Errorf("EC2 recommendation for %s: %w", rec.ResourceType, err) } return ec2OfferingQuery{ instanceType: types.InstanceType(rec.ResourceType), @@ -447,6 +437,27 @@ func buildEC2OfferingQuery(rec common.Recommendation, details *common.ComputeDet }, nil } +// resolveExchangeZone returns the trimmed AZ to send for scope ("" for +// regional). A zonal lookup without an AZ and a regional lookup with one are +// both refused rather than guessed. +func resolveExchangeZone(scope types.Scope, az string) (string, error) { + az = strings.TrimSpace(az) + if scope == types.ScopeAvailabilityZone && az == "" { + return "", errors.New("zonal RI offering needs an availability zone; re-fetch the recommendation") + } + if scope == types.ScopeRegional && az != "" { + return "", fmt.Errorf("regional RI offering must not name an availability zone (contradictory input); "+ + "re-fetch the recommendation: got %q", az) + } + return az, nil +} + +// offeringInZone reports whether o is in az; az == "" (regional) matches any +// offering. A nil offering AZ never matches a named AZ. +func offeringInZone(o types.ReservedInstancesOffering, az string) bool { + return az == "" || aws.ToString(o.AvailabilityZone) == az +} + // describeInputFromQuery builds the SDK request struct for one page of the // typed lookup. Typed fields land on AWS's primary indices; only scope has no // typed equivalent and stays in Filters[]. @@ -587,7 +598,7 @@ func scanEC2OfferingPage(offerings []types.ReservedInstancesOffering, q ec2Offer wantType := q.wantOfferingType for i := range offerings { o := &offerings[i] - if q.availabilityZone != "" && aws.ToString(o.AvailabilityZone) != q.availabilityZone { + if !offeringInZone(*o, q.availabilityZone) { log.Printf("EC2 findOffering skipping offering %s in AZ %q (want %q)", aws.ToString(o.ReservedInstancesOfferingId), aws.ToString(o.AvailabilityZone), q.availabilityZone) continue @@ -822,7 +833,10 @@ type FindConvertibleOfferingParams struct { ProductDescription string Tenancy string Scope string - Duration int64 + // AvailabilityZone is the source RI's AZ name: required for zonal scope and + // refused for regional scope. + AvailabilityZone string + Duration int64 } // FindConvertibleOffering finds a convertible RI offering ID for the given parameters. @@ -835,40 +849,71 @@ func (c *Client) FindConvertibleOffering(ctx context.Context, params FindConvert if err != nil { return "", err } - duration := params.Duration - if duration == 0 { - duration = OneYearSeconds - } - productDesc := params.ProductDescription - if productDesc == "" { - productDesc = defaultEC2Platform + // Keeping the target in the source RI's AZ is policy from #310/#289 (no + // silent AZ drift), not an AWS requirement of exchanges. + az, err := resolveExchangeZone(scope, params.AvailabilityZone) + if err != nil { + return "", err } - - filters := []types.Filter{ - {Name: aws.String("instance-type"), Values: []string{params.InstanceType}}, - {Name: aws.String("product-description"), Values: []string{productDesc}}, - {Name: aws.String("scope"), Values: []string{string(scope)}}, - {Name: aws.String("duration"), Values: []string{fmt.Sprintf("%d", duration)}}, + productDesc, duration, _ := normalizeTargetOfferingsParams(ListTargetOfferingsParams{ + ProductDescription: params.ProductDescription, + Duration: params.Duration, + }) + input := convertibleOfferingInput(params.InstanceType, productDesc, scope, tenancy, az, duration) + for page := 1; page <= maxTargetOfferingPages; page++ { + if err := ctx.Err(); err != nil { + return "", err + } + pageInput := *input + result, err := c.client.DescribeReservedInstancesOfferings(ctx, &pageInput) + if err != nil { + return "", fmt.Errorf("failed to describe convertible offerings: %w", err) + } + if id := firstOfferingInZone(result.ReservedInstancesOfferings, az); id != "" { + return id, nil + } + if isLastEC2Page(result.NextToken) { + return "", fmt.Errorf("no convertible offering found for %s (%s, %s, %s, AZ %q)", + params.InstanceType, productDesc, tenancy, scope, az) + } + input.NextToken = result.NextToken } + return "", fmt.Errorf("no convertible offering found for %s (%s, %s, %s, AZ %q) within %d pages: pagination cap reached", + params.InstanceType, productDesc, tenancy, scope, az, maxTargetOfferingPages) +} +// convertibleOfferingInput builds the first-page request for FindConvertibleOffering. +func convertibleOfferingInput(instanceType, productDesc string, scope types.Scope, tenancy types.Tenancy, az string, duration int64) *ec2.DescribeReservedInstancesOfferingsInput { input := &ec2.DescribeReservedInstancesOfferingsInput{ - Filters: filters, + Filters: []types.Filter{ + {Name: aws.String("instance-type"), Values: []string{instanceType}}, + {Name: aws.String("product-description"), Values: []string{productDesc}}, + {Name: aws.String("scope"), Values: []string{string(scope)}}, + {Name: aws.String("duration"), Values: []string{fmt.Sprintf("%d", duration)}}, + }, InstanceTenancy: tenancy, OfferingClass: types.OfferingClassTypeConvertible, IncludeMarketplace: aws.Bool(false), MaxResults: aws.Int32(20), } - - result, err := c.client.DescribeReservedInstancesOfferings(ctx, input) - if err != nil { - return "", fmt.Errorf("failed to describe convertible offerings: %w", err) + if az != "" { + input.AvailabilityZone = aws.String(az) } + return input +} - if len(result.ReservedInstancesOfferings) == 0 { - return "", fmt.Errorf("no convertible offering found for %s (%s, %s, %s)", params.InstanceType, productDesc, tenancy, scope) +// firstOfferingInZone returns the ID of the first offering in az, or "" when +// none matches. +func firstOfferingInZone(offerings []types.ReservedInstancesOffering, az string) string { + for i := range offerings { + o := offerings[i] + if offeringInZone(o, az) { + return aws.ToString(o.ReservedInstancesOfferingId) + } + log.Printf("FindConvertibleOffering skipping offering %s in AZ %q (want %q)", + aws.ToString(o.ReservedInstancesOfferingId), aws.ToString(o.AvailabilityZone), az) } - - return aws.ToString(result.ReservedInstancesOfferings[0].ReservedInstancesOfferingId), nil + return "" } // TargetOffering is one valid target offering for a convertible RI exchange. @@ -891,6 +936,7 @@ type ListTargetOfferingsParams struct { ProductDescription string Tenancy string Scope string + AvailabilityZone string Duration int64 OfferingType string } @@ -927,9 +973,12 @@ func normalizeTargetOfferingsParams(p ListTargetOfferingsParams) (productDesc st // appendTargetOfferings maps the SDK result page into TargetOffering values // and appends them to out. Extracted from ListTargetOfferings to keep it // under the gocyclo threshold. -func appendTargetOfferings(out []TargetOffering, offerings []types.ReservedInstancesOffering, scope string) []TargetOffering { +func appendTargetOfferings(out []TargetOffering, offerings []types.ReservedInstancesOffering, scope, az string) []TargetOffering { for i := range offerings { o := &offerings[i] + if !offeringInZone(*o, az) { + continue + } id := aws.ToString(o.ReservedInstancesOfferingId) if id == "" { continue @@ -975,6 +1024,10 @@ func (c *Client) ListTargetOfferings(ctx context.Context, params ListTargetOffer if err != nil { return nil, err } + az, err := resolveExchangeZone(scope, params.AvailabilityZone) + if err != nil { + return nil, err + } productDesc, duration, offeringType := normalizeTargetOfferingsParams(params) var out []TargetOffering @@ -995,11 +1048,14 @@ func (c *Client) ListTargetOfferings(ctx context.Context, params ListTargetOffer NextToken: nextToken, Filters: []types.Filter{{Name: aws.String("scope"), Values: []string{string(scope)}}}, } + if az != "" { + input.AvailabilityZone = aws.String(az) + } result, err := c.client.DescribeReservedInstancesOfferings(ctx, input) if err != nil { return nil, fmt.Errorf("describe target offerings: %w", err) } - out = appendTargetOfferings(out, result.ReservedInstancesOfferings, string(scope)) + out = appendTargetOfferings(out, result.ReservedInstancesOfferings, string(scope), az) if isLastEC2Page(result.NextToken) { break } diff --git a/providers/aws/services/ec2/client_test.go b/providers/aws/services/ec2/client_test.go index 94350e3a..d6770922 100644 --- a/providers/aws/services/ec2/client_test.go +++ b/providers/aws/services/ec2/client_test.go @@ -1532,18 +1532,23 @@ var validTenancyScopeCases = []struct { name string tenancy string scope string + az string wantTenancy types.Tenancy wantScope string }{ - {"default_region", "default", "Region", types.TenancyDefault, "Region"}, - {"dedicated_az", "dedicated", "Availability Zone", types.TenancyDedicated, "Availability Zone"}, - {"legacy_shared_region", "shared", "region", types.TenancyDefault, "Region"}, - {"legacy_az_hyphenated", "dedicated", "availability-zone", types.TenancyDedicated, "Availability Zone"}, + {"default_region", "default", "Region", "", types.TenancyDefault, "Region"}, + {"dedicated_az", "dedicated", "Availability Zone", "us-east-1a", types.TenancyDedicated, "Availability Zone"}, + {"legacy_shared_region", "shared", "region", "", types.TenancyDefault, "Region"}, + {"legacy_az_hyphenated", "dedicated", "availability-zone", "us-east-1a", types.TenancyDedicated, "Availability Zone"}, } // captureOfferingsInput returns a mock that records the request it receives // and answers with a single offering. -func captureOfferingsInput(offeringID string, got **ec2.DescribeReservedInstancesOfferingsInput) *MockEC2Client { +func captureOfferingsInput(offeringID, az string, got **ec2.DescribeReservedInstancesOfferingsInput) *MockEC2Client { + var offeringAZ *string + if az != "" { + offeringAZ = aws.String(az) + } mockEC2 := &MockEC2Client{} mockEC2.On("DescribeReservedInstancesOfferings", mock.Anything, mock.Anything). Run(func(args mock.Arguments) { @@ -1553,6 +1558,7 @@ func captureOfferingsInput(offeringID string, got **ec2.DescribeReservedInstance ReservedInstancesOfferings: []types.ReservedInstancesOffering{{ ReservedInstancesOfferingId: aws.String(offeringID), InstanceType: types.InstanceTypeM5Large, + AvailabilityZone: offeringAZ, }}, }, nil).Once() return mockEC2 @@ -1573,19 +1579,22 @@ func TestFindConvertibleOffering_TenancyScopeVariants_SendEnumValues(t *testing. t.Run(tc.name, func(t *testing.T) { t.Parallel() var got *ec2.DescribeReservedInstancesOfferingsInput - client := &Client{client: captureOfferingsInput("offering-"+tc.name, &got), region: "us-east-1"} + client := &Client{client: captureOfferingsInput("offering-"+tc.name, tc.az, &got), region: "us-east-1"} id, err := client.FindConvertibleOffering(context.Background(), FindConvertibleOfferingParams{ InstanceType: "m5.large", ProductDescription: "Linux/UNIX", Tenancy: tc.tenancy, Scope: tc.scope, + AvailabilityZone: tc.az, Duration: OneYearSeconds, }) require.NoError(t, err) assert.Equal(t, "offering-"+tc.name, id) require.NotNil(t, got) + assert.Equal(t, tc.az, aws.ToString(got.AvailabilityZone)) + assert.Nil(t, got.AvailabilityZoneId, "never send the zone id alongside the name") assert.Equal(t, tc.wantTenancy, got.InstanceTenancy) assert.Nil(t, filterValues(got.Filters, "instance-tenancy"), "instance-tenancy is not a documented filter") assert.Equal(t, []string{tc.wantScope}, filterValues(got.Filters, "scope")) @@ -1602,12 +1611,13 @@ func TestListTargetOfferings_TenancyScopeVariants_SendEnumValues(t *testing.T) { t.Run(tc.name, func(t *testing.T) { t.Parallel() var got *ec2.DescribeReservedInstancesOfferingsInput - client := &Client{client: captureOfferingsInput("offering-"+tc.name, &got), region: "us-east-1"} + client := &Client{client: captureOfferingsInput("offering-"+tc.name, tc.az, &got), region: "us-east-1"} offerings, err := client.ListTargetOfferings(context.Background(), ListTargetOfferingsParams{ ProductDescription: "Linux/UNIX", Tenancy: tc.tenancy, Scope: tc.scope, + AvailabilityZone: tc.az, Duration: OneYearSeconds, }) @@ -1616,6 +1626,8 @@ func TestListTargetOfferings_TenancyScopeVariants_SendEnumValues(t *testing.T) { assert.Equal(t, "offering-"+tc.name, offerings[0].OfferingID) assert.Equal(t, tc.wantScope, offerings[0].Scope) require.NotNil(t, got) + assert.Equal(t, tc.az, aws.ToString(got.AvailabilityZone)) + assert.Nil(t, got.AvailabilityZoneId, "never send the zone id alongside the name") assert.Equal(t, tc.wantTenancy, got.InstanceTenancy) assert.Equal(t, []string{tc.wantScope}, filterValues(got.Filters, "scope")) assert.Equal(t, types.OfferingClassTypeConvertible, got.OfferingClass) @@ -1837,3 +1849,277 @@ func TestFindOffering_ZonalRecWithoutAZRefusedBeforeAPICall(t *testing.T) { require.Error(t, err) assert.Nil(t, cap.LastDescribeOfferingsInput) } + +func TestResolveExchangeZone(t *testing.T) { + t.Parallel() + tests := []struct { + name string + scope types.Scope + az string + want string + wantErr string + }{ + {"zonal with AZ", types.ScopeAvailabilityZone, "us-east-1b", "us-east-1b", ""}, + {"zonal trims whitespace", types.ScopeAvailabilityZone, " us-east-1b ", "us-east-1b", ""}, + {"zonal blank errors", types.ScopeAvailabilityZone, "", "", "zonal RI offering needs an availability zone"}, + {"zonal whitespace-only errors", types.ScopeAvailabilityZone, " ", "", "zonal RI offering needs an availability zone"}, + {"regional with AZ errors", types.ScopeRegional, "us-east-1b", "", "regional RI offering must not name an availability zone"}, + {"regional blank ok", types.ScopeRegional, "", "", ""}, + {"regional whitespace-only ok", types.ScopeRegional, " ", "", ""}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + got, err := resolveExchangeZone(tc.scope, tc.az) + if tc.wantErr != "" { + require.Error(t, err) + assert.Contains(t, err.Error(), tc.wantErr) + assert.Contains(t, err.Error(), "re-fetch") + return + } + require.NoError(t, err) + assert.Equal(t, tc.want, got) + }) + } +} + +func TestOfferingInZone(t *testing.T) { + t.Parallel() + o1b := types.ReservedInstancesOffering{AvailabilityZone: aws.String("us-east-1b")} + assert.True(t, offeringInZone(o1b, "us-east-1b")) + assert.False(t, offeringInZone(o1b, "us-east-1a")) + assert.False(t, offeringInZone(types.ReservedInstancesOffering{}, "us-east-1b"), "nil AZ is a mismatch") + assert.True(t, offeringInZone(types.ReservedInstancesOffering{}, ""), "regional matches any offering") + assert.True(t, offeringInZone(o1b, "")) +} + +func zonedOffering(id, az string) types.ReservedInstancesOffering { + o := types.ReservedInstancesOffering{ + ReservedInstancesOfferingId: aws.String(id), + InstanceType: types.InstanceTypeM5Large, + } + if az != "" { + o.AvailabilityZone = aws.String(az) + } + return o +} + +// pagedOfferingsMock records every request and serves one page per call. +func pagedOfferingsMock(got *[]*ec2.DescribeReservedInstancesOfferingsInput, pages ...[]types.ReservedInstancesOffering) *MockEC2Client { + m := &MockEC2Client{} + for i, p := range pages { + out := &ec2.DescribeReservedInstancesOfferingsOutput{ReservedInstancesOfferings: p} + if i < len(pages)-1 { + out.NextToken = aws.String("next") + } + m.On("DescribeReservedInstancesOfferings", mock.Anything, mock.Anything). + Run(func(args mock.Arguments) { + *got = append(*got, args.Get(1).(*ec2.DescribeReservedInstancesOfferingsInput)) + }).Return(out, nil).Once() + } + return m +} + +func zonalFindParams(az string) FindConvertibleOfferingParams { + return FindConvertibleOfferingParams{ + InstanceType: "m5.large", ProductDescription: "Linux/UNIX", Tenancy: "default", + Scope: "Availability Zone", AvailabilityZone: az, Duration: OneYearSeconds, + } +} + +func zonalListParams(az string) ListTargetOfferingsParams { + return ListTargetOfferingsParams{ + ProductDescription: "Linux/UNIX", Tenancy: "default", + Scope: "Availability Zone", AvailabilityZone: az, Duration: OneYearSeconds, + } +} + +func TestFindConvertibleOffering_Zonal_SendsAZAndPicksMatchingOffering(t *testing.T) { + t.Parallel() + var got []*ec2.DescribeReservedInstancesOfferingsInput + m := pagedOfferingsMock(&got, []types.ReservedInstancesOffering{ + zonedOffering("off-1a", "us-east-1a"), zonedOffering("off-nil", ""), zonedOffering("off-1b", "us-east-1b"), + }) + client := &Client{client: m, region: "us-east-1"} + + id, err := client.FindConvertibleOffering(context.Background(), zonalFindParams(" us-east-1b ")) + + require.NoError(t, err) + assert.Equal(t, "off-1b", id) + require.Len(t, got, 1) + assert.Equal(t, "us-east-1b", aws.ToString(got[0].AvailabilityZone)) + assert.Nil(t, got[0].AvailabilityZoneId) +} + +func TestFindConvertibleOffering_Zonal_NoMatchingAZ_Errors(t *testing.T) { + t.Parallel() + var got []*ec2.DescribeReservedInstancesOfferingsInput + m := pagedOfferingsMock(&got, []types.ReservedInstancesOffering{ + zonedOffering("off-1a", "us-east-1a"), zonedOffering("off-nil", ""), + }) + client := &Client{client: m, region: "us-east-1"} + + id, err := client.FindConvertibleOffering(context.Background(), zonalFindParams("us-east-1b")) + + require.Error(t, err) + assert.Empty(t, id) + assert.Contains(t, err.Error(), "no convertible offering found") + assert.Contains(t, err.Error(), "us-east-1b") +} + +func TestFindConvertibleOffering_Zonal_MatchOnSecondPage_FollowsNextToken(t *testing.T) { + t.Parallel() + var got []*ec2.DescribeReservedInstancesOfferingsInput + m := pagedOfferingsMock(&got, + []types.ReservedInstancesOffering{zonedOffering("off-1a", "us-east-1a")}, + []types.ReservedInstancesOffering{zonedOffering("off-1b", "us-east-1b")}, + ) + client := &Client{client: m, region: "us-east-1"} + + id, err := client.FindConvertibleOffering(context.Background(), zonalFindParams("us-east-1b")) + + require.NoError(t, err) + assert.Equal(t, "off-1b", id) + require.Len(t, got, 2) + assert.Nil(t, got[0].NextToken) + assert.Equal(t, "next", aws.ToString(got[1].NextToken)) + assert.Equal(t, "us-east-1b", aws.ToString(got[1].AvailabilityZone)) +} + +func TestFindConvertibleOffering_Zonal_NoMatchAcrossPages_Errors(t *testing.T) { + t.Parallel() + var got []*ec2.DescribeReservedInstancesOfferingsInput + m := pagedOfferingsMock(&got, + []types.ReservedInstancesOffering{zonedOffering("off-1a", "us-east-1a")}, + []types.ReservedInstancesOffering{zonedOffering("off-1c", "us-east-1c")}, + ) + client := &Client{client: m, region: "us-east-1"} + + id, err := client.FindConvertibleOffering(context.Background(), zonalFindParams("us-east-1b")) + + require.Error(t, err) + assert.Empty(t, id) + assert.Contains(t, err.Error(), "no convertible offering found") + assert.NotContains(t, err.Error(), "pagination cap") + assert.Len(t, got, 2) +} + +func TestFindConvertibleOffering_Zonal_PageCapReached_Errors(t *testing.T) { + t.Parallel() + var got []*ec2.DescribeReservedInstancesOfferingsInput + pages := make([][]types.ReservedInstancesOffering, maxTargetOfferingPages+1) + for i := range pages { + pages[i] = []types.ReservedInstancesOffering{zonedOffering("off-1a", "us-east-1a")} + } + m := pagedOfferingsMock(&got, pages...) + client := &Client{client: m, region: "us-east-1"} + + id, err := client.FindConvertibleOffering(context.Background(), zonalFindParams("us-east-1b")) + + require.Error(t, err) + assert.Empty(t, id) + assert.Contains(t, err.Error(), "pagination cap reached") + assert.Contains(t, err.Error(), fmt.Sprintf("%d pages", maxTargetOfferingPages)) + assert.Len(t, got, maxTargetOfferingPages) +} + +func TestFindConvertibleOffering_ZoneMisuse_ErrorsBeforeAPICall(t *testing.T) { + t.Parallel() + zonalNoAZ := zonalFindParams("") + regionalWithAZ := zonalFindParams("us-east-1b") + regionalWithAZ.Scope = "Region" + for name, tc := range map[string]struct { + params FindConvertibleOfferingParams + wantErr string + }{ + "zonal without AZ": {zonalNoAZ, "zonal RI offering needs an availability zone"}, + "regional with AZ": {regionalWithAZ, "regional RI offering must not name an availability zone"}, + "zonal whitespace AZ": {zonalFindParams(" "), "zonal RI offering needs an availability zone"}, + } { + t.Run(name, func(t *testing.T) { + t.Parallel() + m := matchingOfferingMock() + client := &Client{client: m, region: "us-east-1"} + _, err := client.FindConvertibleOffering(context.Background(), tc.params) + require.Error(t, err) + assert.Contains(t, err.Error(), tc.wantErr) + m.AssertNotCalled(t, "DescribeReservedInstancesOfferings", mock.Anything, mock.Anything) + }) + } +} + +func TestFindConvertibleOffering_Regional_DoesNotSendAZ(t *testing.T) { + t.Parallel() + var got []*ec2.DescribeReservedInstancesOfferingsInput + m := pagedOfferingsMock(&got, []types.ReservedInstancesOffering{zonedOffering("off-reg", "")}) + client := &Client{client: m, region: "us-east-1"} + p := zonalFindParams("") + p.Scope = "Region" + + id, err := client.FindConvertibleOffering(context.Background(), p) + + require.NoError(t, err) + assert.Equal(t, "off-reg", id) + require.Len(t, got, 1) + assert.Nil(t, got[0].AvailabilityZone) +} + +func TestListTargetOfferings_Zonal_SendsAZAndFiltersOfferings(t *testing.T) { + t.Parallel() + var got []*ec2.DescribeReservedInstancesOfferingsInput + m := pagedOfferingsMock(&got, + []types.ReservedInstancesOffering{zonedOffering("off-1a", "us-east-1a"), zonedOffering("off-nil", "")}, + []types.ReservedInstancesOffering{zonedOffering("off-1b", "us-east-1b")}, + ) + client := &Client{client: m, region: "us-east-1"} + + offerings, err := client.ListTargetOfferings(context.Background(), zonalListParams("us-east-1b")) + + require.NoError(t, err) + require.Len(t, offerings, 1) + assert.Equal(t, "off-1b", offerings[0].OfferingID) + require.Len(t, got, 2, "both pages walked") + for _, in := range got { + assert.Equal(t, "us-east-1b", aws.ToString(in.AvailabilityZone)) + assert.Nil(t, in.AvailabilityZoneId) + } +} + +func TestListTargetOfferings_ZoneMisuse_ErrorsBeforeAPICall(t *testing.T) { + t.Parallel() + regionalWithAZ := zonalListParams("us-east-1b") + regionalWithAZ.Scope = "Region" + for name, tc := range map[string]struct { + params ListTargetOfferingsParams + wantErr string + }{ + "zonal without AZ": {zonalListParams(""), "zonal RI offering needs an availability zone"}, + "regional with AZ": {regionalWithAZ, "regional RI offering must not name an availability zone"}, + } { + t.Run(name, func(t *testing.T) { + t.Parallel() + m := matchingOfferingMock() + client := &Client{client: m, region: "us-east-1"} + _, err := client.ListTargetOfferings(context.Background(), tc.params) + require.Error(t, err) + assert.Contains(t, err.Error(), tc.wantErr) + m.AssertNotCalled(t, "DescribeReservedInstancesOfferings", mock.Anything, mock.Anything) + }) + } +} + +func TestListTargetOfferings_Regional_DoesNotSendAZ(t *testing.T) { + t.Parallel() + var got []*ec2.DescribeReservedInstancesOfferingsInput + m := pagedOfferingsMock(&got, []types.ReservedInstancesOffering{zonedOffering("off-reg", "")}) + client := &Client{client: m, region: "us-east-1"} + p := zonalListParams("") + p.Scope = "Region" + + offerings, err := client.ListTargetOfferings(context.Background(), p) + + require.NoError(t, err) + require.Len(t, offerings, 1) + require.Len(t, got, 1) + assert.Nil(t, got[0].AvailabilityZone) +}