From 091f3a5900abe6f96cd7ed3b59071fcd5b3d736f Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Fri, 16 Oct 2026 22:05:20 +0200 Subject: [PATCH 1/2] build(aws): bump pkg pin to include EC2 commitment dimensions Bump pkg from v0.0.0-20261015085730-498b74f31f01 to v0.0.0-20261016194019-89772cf80f67 so the AWS module sees the Commitment Platform/Tenancy/Scope/AvailabilityZone fields and the recfilter matching on them. pkg commits in the range touching packages providers/aws imports: - 89772cf fix(recfilter): match EC2 commitments on platform, tenancy and scope - 9260573 feat(common): add EC2 platform, tenancy and scope dimensions - b69a17e fix(recfilter): treat a date-only commitment start as any time that day - 551be56 feat(exchange)!: pass the source RI's availability zone to LookupOffering - 25bffb5 fix(exchange): send the RI exchange accept once and halt on an unknown outcome 11200ee (reccontract) and the insurance commits are not imported by providers/aws. Build and race tests pass on the bumped pin. Refs #323 Co-Authored-By: Claude Opus 5.5 --- providers/aws/go.mod | 2 +- providers/aws/go.sum | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/providers/aws/go.mod b/providers/aws/go.mod index ea867ce6..29ad8236 100644 --- a/providers/aws/go.mod +++ b/providers/aws/go.mod @@ -3,7 +3,7 @@ module github.com/LeanerCloud/cloud-commitments-go/providers/aws go 1.26.9 require ( - github.com/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20261015085730-498b74f31f01 + github.com/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20261016194019-89772cf80f67 github.com/aws/aws-sdk-go-v2 v1.41.5 github.com/aws/aws-sdk-go-v2/config v1.29.12 github.com/aws/aws-sdk-go-v2/credentials v1.17.65 diff --git a/providers/aws/go.sum b/providers/aws/go.sum index ead905de..33275e77 100644 --- a/providers/aws/go.sum +++ b/providers/aws/go.sum @@ -1,5 +1,5 @@ -github.com/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20261015085730-498b74f31f01 h1:R2EQAvwLAxPrdlZXmF+bI7ioQYh1CxBkethx5hq6WWA= -github.com/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20261015085730-498b74f31f01/go.mod h1:8RTmzPazA8IeVZfn7wNaU7ILx0VK5eHZQozO21Mvtq8= +github.com/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20261016194019-89772cf80f67 h1:zpr+lPSdlXeA9ySU7FqHtgn4ADinmRhfJ5Y6jm9ankw= +github.com/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20261016194019-89772cf80f67/go.mod h1:SCDEgHG6evjPh0RsoXof+J3aMmGXNFMrwbJIle7rMpQ= github.com/aws/aws-sdk-go-v2 v1.41.5 h1:dj5kopbwUsVUVFgO4Fi5BIT3t4WyqIDjGKCangnV/yY= github.com/aws/aws-sdk-go-v2 v1.41.5/go.mod h1:mwsPRE8ceUUpiTgF7QmQIJ7lgsKUPQOUl3o72QBrE1o= github.com/aws/aws-sdk-go-v2/config v1.29.12 h1:Y/2a+jLPrPbHpFkpAAYkVEtJmxORlXoo5k2g1fa2sUo= From efaaa72501b6200effd504531d534f01ab90d0d0 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Fri, 16 Oct 2026 22:08:05 +0200 Subject: [PATCH 2/2] fix(aws): report EC2 reservation platform, tenancy and scope so dedupe can match them GetExistingCommitments filled none of Commitment.Platform, Tenancy, Scope or AvailabilityZone, so the recfilter matching on those dimensions was inert. Pass the raw SDK ProductDescription, InstanceTenancy, Scope and AvailabilityZone values; pkg/common normalizes them, so there is no second mapping table. Account stays unset, as before. Tests cover the listing (regional Linux default, zonal Windows dedicated, nil fields) and the listing through DuplicateChecker with parser-shaped recommendations: a Windows RI no longer suppresses a Linux rec, a zonal RI does not suppress a regional rec, a regional RI suppresses a zonal rec and a matching Linux regional RI suppresses. Follow-ups outside this change: RDS size-flex, the ElastiCache empty-description wildcard, MemoryDB engine and account keying. Refs #323 Co-Authored-By: Claude Opus 5.5 --- providers/aws/services/ec2/client.go | 5 + .../services/ec2/client_listing_dims_test.go | 106 ++++++++++++++++++ 2 files changed, 111 insertions(+) create mode 100644 providers/aws/services/ec2/client_listing_dims_test.go diff --git a/providers/aws/services/ec2/client.go b/providers/aws/services/ec2/client.go index ee5cd216..0ffa325f 100644 --- a/providers/aws/services/ec2/client.go +++ b/providers/aws/services/ec2/client.go @@ -113,6 +113,11 @@ func (c *Client) GetExistingCommitments(ctx context.Context) ([]common.Commitmen State: common.CommitmentState(ri.State), StartDate: aws.ToTime(ri.Start), EndDate: aws.ToTime(ri.End), + // Raw SDK values; pkg/common normalizes them when matching. + Platform: string(ri.ProductDescription), + Tenancy: string(ri.InstanceTenancy), + Scope: string(ri.Scope), + AvailabilityZone: aws.ToString(ri.AvailabilityZone), } commitments = append(commitments, commitment) diff --git a/providers/aws/services/ec2/client_listing_dims_test.go b/providers/aws/services/ec2/client_listing_dims_test.go new file mode 100644 index 00000000..56104313 --- /dev/null +++ b/providers/aws/services/ec2/client_listing_dims_test.go @@ -0,0 +1,106 @@ +package ec2 + +import ( + "context" + "testing" + "time" + + "github.com/LeanerCloud/cloud-commitments-go/pkg/common" + "github.com/LeanerCloud/cloud-commitments-go/pkg/recfilter" + "github.com/aws/aws-sdk-go-v2/aws" + "github.com/aws/aws-sdk-go-v2/service/ec2" + "github.com/aws/aws-sdk-go-v2/service/ec2/types" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" +) + +func sdkRI(id, product string, tenancy types.Tenancy, scope types.Scope, az *string) types.ReservedInstances { + return types.ReservedInstances{ + ReservedInstancesId: aws.String(id), + InstanceType: types.InstanceTypeM5Large, + InstanceCount: aws.Int32(2), + State: types.ReservedInstanceStateActive, + Start: aws.Time(time.Now().Add(-time.Hour)), + End: aws.Time(time.Now().Add(24 * time.Hour)), + ProductDescription: types.RIProductDescription(product), + InstanceTenancy: tenancy, + Scope: scope, + AvailabilityZone: az, + } +} + +func listingClient(ris ...types.ReservedInstances) *Client { + m := &MockEC2Client{} + m.On("DescribeReservedInstances", mock.Anything, mock.Anything). + Return(&ec2.DescribeReservedInstancesOutput{ReservedInstances: ris}, nil) + return &Client{client: m, region: "us-east-1"} +} + +func TestGetExistingCommitments_ReportsPlatformTenancyScope(t *testing.T) { + client := listingClient( + sdkRI("ri-regional", "Linux/UNIX", types.TenancyDefault, types.ScopeRegional, nil), + sdkRI("ri-zonal", "Windows", types.TenancyDedicated, types.ScopeAvailabilityZone, aws.String("us-east-1a")), + types.ReservedInstances{ReservedInstancesId: aws.String("ri-nil")}, + ) + got, err := client.GetExistingCommitments(context.Background()) + require.NoError(t, err) + require.Len(t, got, 3) + + assert.Equal(t, "Linux/UNIX", got[0].Platform) + assert.Equal(t, "default", got[0].Tenancy) + assert.Equal(t, "Region", got[0].Scope) + assert.Empty(t, got[0].AvailabilityZone) + + assert.Equal(t, "Windows", got[1].Platform) + assert.Equal(t, "dedicated", got[1].Tenancy) + assert.Equal(t, "Availability Zone", got[1].Scope) + assert.Equal(t, "us-east-1a", got[1].AvailabilityZone) + + assert.Empty(t, got[2].Platform) + assert.Empty(t, got[2].Tenancy) + assert.Empty(t, got[2].Scope) + assert.Empty(t, got[2].AvailabilityZone) +} + +// parserRec mirrors the ComputeDetails parseEC2Details builds from Cost Explorer. +func parserRec(scope, az string) common.Recommendation { + return common.Recommendation{ + Provider: common.ProviderAWS, Service: common.ServiceEC2, ResourceType: "m5.large", + Region: "us-east-1", Count: 1, + Details: &common.ComputeDetails{InstanceType: "m5.large", Platform: "Linux/UNIX", + Tenancy: "default", Scope: scope, AvailabilityZone: az}, + } +} + +func TestListingFeedsDuplicateChecker(t *testing.T) { + linuxRegional := sdkRI("ri-1", "Linux/UNIX", types.TenancyDefault, types.ScopeRegional, nil) + windowsRegional := sdkRI("ri-2", "Windows", types.TenancyDefault, types.ScopeRegional, nil) + linuxZonal := sdkRI("ri-3", "Linux/UNIX", types.TenancyDefault, types.ScopeAvailabilityZone, aws.String("us-east-1a")) + + tests := []struct { + name string + ri types.ReservedInstances + rec common.Recommendation + suppress bool + }{ + {"windows RI does not suppress linux rec", windowsRegional, parserRec("Region", ""), false}, + {"zonal RI does not suppress regional rec", linuxZonal, parserRec("Region", ""), false}, + {"regional RI suppresses zonal rec", linuxRegional, parserRec("Availability Zone", "us-east-1a"), true}, + {"matching linux regional RI suppresses", linuxRegional, parserRec("Region", ""), true}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + passed, filtered, err := recfilter.NewDuplicateChecker(0). + AdjustRecommendationsForExisting(context.Background(), []common.Recommendation{tt.rec}, listingClient(tt.ri)) + require.NoError(t, err) + if tt.suppress { + assert.Empty(t, passed) + assert.Len(t, filtered, 1) + } else { + assert.Len(t, passed, 1) + assert.Empty(t, filtered) + } + }) + } +}