fix(aws): report EC2 reservation platform, tenancy and scope so dedupe can match them

[cristim]

Summary

This is PR 2 of the first #323 slice, and it makes the EC2 dedupe fix live.

#458 added Commitment.Platform/Tenancy/Scope/AvailabilityZone and their normalizers. #460 made the recfilter duplicate check match AWS EC2 commitments on them: an empty value is a wildcard, scope coverage is asymmetric, and accounts are not keyed. The EC2 listing filled none of these fields, so the matching was inert. Two commits fix that.

1. build(aws) bumps the aws pkg pin from v0.0.0-20261015085730-498b74f31f01 to v0.0.0-20261016194019-89772cf80f67. Of the pkg commits in between, these touch packages providers/aws imports: 89772cf, 9260573, b69a17e (recfilter/common), and 551be56 and 25bffb5 (exchange). Nothing in providers/aws references the changed LookupOfferingFunc. The full aws build and race suite passed on the bumped pin before any code change.
2. fix(aws) makes GetExistingCommitments (providers/aws/services/ec2/client.go) set the raw SDK values: ProductDescription, InstanceTenancy, Scope, and the AZ only when the SDK returns one. pkg normalizes them, so there is no second mapping table. Account stays unset, as before.

Effect: a recent Windows, dedicated or zonal reservation no longer suppresses a matching Linux, default-tenancy or regional EC2 recommendation, so that recommendation is no longer silently dropped. A regional reservation still covers zonal usage, and a commitment in another linked account still suppresses.

Follow-ups, still open on #323: RDS size-flexibility, the ElastiCache empty-description wildcard, the MemoryDB engine, and account keying. Account keying needs an RI-sharing signal; see the design note on the issue.

Refs #323 (this completes the EC2 part of the first slice; the issue stays open for the follow-ups below)

Verification (macOS, Go 1.26.9)

• (cd providers/aws && GOWORK=off go test -race -short ./...) passes. make build and make vet pass. golangci-lint v2.10.1 (the CI pin) reports 0 issues. GOWORK=off go build ./... passes in providers/aws and ci_cd_sanity_tests.
• TestGetExistingCommitments_ReportsPlatformTenancyScope uses an SDK-shaped DescribeReservedInstances fake. It covers a regional Linux default RI, a zonal Windows dedicated RI, and an RI with nil fields. The nil case yields empty fields, which act as wildcards.
• TestListingFeedsDuplicateChecker runs listed commitments through recfilter.NewDuplicateChecker(0), which uses the 24h default lookback, and AdjustRecommendationsForExisting. The recs are built in the shape parseEC2Details produces. It checks four cases:
  • a Windows RI does not suppress a Linux rec;
  • a zonal RI does not suppress a regional rec;
  • a regional RI suppresses a zonal rec;
  • a matching Linux regional RI suppresses.
• Fail-before: on the bumped pin without the listing change, the listing test and the Windows and zonal e2e cases fail.
• The independent reviewer ran mutation probes in a scratch copy. Dropping the Platform, Scope, Tenancy or AZ fill fails the listing test. Dropping Platform, or swapping AZ for Region, also fails the e2e test.
• Review: one diff pass by the same non-author reviewer who reviewed the plan and PRs 1a/1b found no actionable findings. Rebased onto 0b6cb12 (azure-only) and re-tested.

Fixtures only; no AWS call.

🤖 Generated with Claude Code

[coderabbitai]

Warning

Review limit reached

• Run on-demand review

_{This review includes 3 billable files and costs up to $0.75.}

• Ask an admin to make reviews automatic

Open in CodeRabbit

Reviews can continue after your included limit without a manual trigger. An admin must approve usage-based billing.

Or wait 59 minutes for your next included review.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available. Your 67 included PR review attempts over the past 7 days set your current allowance at 1 review per hour.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

• Configuration used: Repository: LeanerCloud/cloud-commitments-go/.coderabbit.yaml
• Review profile: CHILL
• Plan: Essentials
• Run ID: 303be9ec-89a0-49cd-aaef-15b77cd24216

📥 Commits

Reviewing files that changed from the base of the PR and between c2b1511 and efaaa72.

⛔ Files ignored due to path filters: 1

• providers/aws/go.sum is excluded by !**/*.sum

📒 Files selected for processing: 3

• providers/aws/go.mod
• providers/aws/services/ec2/client.go
• providers/aws/services/ec2/client_listing_dims_test.go

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

_{Comment @coderabbitai help to get the list of available commands.}

[cristim]

@coderabbitai review

[coderabbitai]

⚠️ Action not completed

Review rate limited.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[cristim]

Independent adversarial review: CLEAN at ebf82c6a74ac8deebf52ff980a22520a4370fee8

Reviewer: an independent agent that did not write this change (Sonnet, high effort), cold brief, full diff against main (0b6cb12). The PR body was treated as a claim.

Blocking findings: none.

Non-blocking notes (no action needed to merge):

• providers/aws/services/ec2/client.go:116-120: the SDK ProductDescription enum has only the four Linux/UNIX and Windows values (with and without (Amazon VPC)), so RHEL/SUSE never arrive through this API. knownPlatform (pkg/recfilter/dedupe_ec2.go:127) accepts them anyway; no failure scenario.
• The new tests do not push an (Amazon VPC) variant through the duplicate checker. NormalizePlatformName (pkg/common/engine.go:79-85) strips the suffix, so behavior is correct; a test case would be a small follow-up.

What was attacked:

• Pin: v0.0.0-20261016194019-89772cf80f67 matches the UTC commit time and hash of 89772cf (fix(recfilter): match EC2 commitments on platform, tenancy and scope when deduplicating #460), an ancestor of main; go.mod/go.sum lines updated together; GOWORK=off resolves it.
• Values vs normalizers at the pin: NormalizePlatformName (engine.go:79), NormalizeTenancyName (engine.go:98, default/dedicated/host), NormalizeScopeName (engine.go:114, Region/Availability Zone) match the SDK enum strings (ec2/types/enums.go:8692-8693, 9470-9472).
• Wildcards and scope (pkg/recfilter/dedupe_ec2.go:20-52): nil SDK fields become "" wildcards, not mismatches; a non-empty AZ forces zonal; a regional RI covers zonal recs, a zonal RI does not cover regional ones.
• Blast radius: only recfilter/dedupe_ec2.go:91 reads these Commitment fields in non-test code; the fields are omitempty, so JSON output only gains fields.
• Mutation probes (scratch copy, reverted): Scope set to "", AZ swapped for region, Tenancy dropped. Each made a new test fail.

Local verification (macOS, Go 1.27.2, exact SHA ebf82c6a)

Fixture-based evidence (SDK-shaped DescribeReservedInstances fake, real recfilter checker; no AWS call):

• Fail-before: with client.go reverted to the parent commit and the bumped pin kept, TestGetExistingCommitments_ReportsPlatformTenancyScope and TestListingFeedsDuplicateChecker FAIL.
• Pass-after: (cd providers/aws && GOWORK=off go test -race -short ./...) exit 0 (14 packages ok). make build and make vet pass.
• Real-account gap: not exercised against a live AWS account listing; not required to merge per the review gate.

CR waived: quota, adversarial review + local verification + green CI (CodeRabbit reported "Review rate limited" on this PR).

Labels mirror #323. Merge waits for CI green on this exact SHA.

[cristim]

CI is green on head ebf82c6a: pre-commit 38960414497 and Build & Test 38960414591.

[cristim]

Rebased onto main ca34022 (#438, a doc comment in pkg/ladder/capability.go only) because the ruleset requires up-to-date branches. New head cb58ee699445b4900a47b638094b485a79159ecb.

git range-diff 0b6cb12..ebf82c6 origin/main..cb58ee6:

1:  41fb677 = 1:  39f754c build(aws): bump pkg pin to include EC2 commitment dimensions
2:  ebf82c6 = 2:  cb58ee6 fix(aws): report EC2 reservation platform, tenancy and scope so dedupe can match them

Both commits are identical, so the CLEAN verdict for ebf82c6a carries over to cb58ee69. On the rebased head, make build passes and go test ./services/ec2/ (providers/aws, GOWORK=off) is ok. CI is re-running on cb58ee69. CR waived: quota, adversarial review + local verification + green CI.

[cristim]

Rebased again onto main c2b1511 (#459, Azure-only: providers/azure/services/savingsplans). New head efaaa72501b6200effd504531d534f01ab90d0d0.

git range-diff ca34022..cb58ee6 origin/main..efaaa72:

1:  39f754c = 1:  091f3a5 build(aws): bump pkg pin to include EC2 commitment dimensions
2:  cb58ee6 = 2:  efaaa72 fix(aws): report EC2 reservation platform, tenancy and scope so dedupe can match them

Both commits are identical, so the CLEAN verdict for ebf82c6a carries over to efaaa72. make build passes and the ec2 tests are ok. CI is re-running. CR waived: quota, adversarial review + local verification + green CI.

[cristim]

Merged by squash at reviewed head efaaa72501b6200effd504531d534f01ab90d0d0 as 26caeb99. Gates on that SHA: independent adversarial review CLEAN (verdict for ebf82c6a, carried by identical git range-diff across two rebases onto #438 and #459), local fixture verification with fail-before/pass-after, CI green (pre-commit 38964529521, Build & Test 38964529823), labels mirrored from #323, no unresolved threads. CR waived: quota, adversarial review + local verification + green CI. #323 stays open for the follow-ups listed in the PR body. Retrospective CodeRabbit review to run when quota returns.