fix(gcp/cloudstorage): apply the currency unification to the Cloud Storage extractor

[IAmJSD]

Root cause

extractStoragePricingFromSKUs (providers/gcp/services/cloudstorage/client.go) seeded currency = "USD" and then overwrote it with the currency of whichever priced SKU came last. A catalog result with mixed currencies, or SKUs with no currency code, therefore reported a currency that did not match the prices it summed. This is the Cloud Storage slice of #102 that #239 left out because #205 (now merged) touched the same file.

Fix

Mirrors #239 exactly for Cloud Storage: the USD seed is removed and each priced SKU's currency is folded in with billingcurrency.Unify(currency, curr, sku.SkuId), returning its error. A missing or conflicting currency now fails pricing instead of mislabelling it. No hardcoded "USD" remains in client.go.

The regression test providers/gcp/services/cloudstorage/pricing_currency_test.go uses the same table as cloudsql/pricing_currency_test.go and goes through getStoragePricing with a mocked billing service: consistent EUR, consistent USD, mixed USD/EUR, and a missing commitment or on-demand currency.

Closes #243

Verification

All of this ran locally on macOS against mocked Cloud Billing Catalog responses. No real GCP APIs were called.

Fail before the fix (test added, client.go unchanged):

cd providers/gcp && AWS_CONFIG_FILE=/dev/null AWS_SHARED_CREDENTIALS_FILE=/dev/null go test ./services/cloudstorage/ -run TestGetStoragePricing_Currency
--- FAIL: TestGetStoragePricing_Currency/on-demand_and_commitment_currencies_differ  (An error is expected but got nil.)
--- FAIL: TestGetStoragePricing_Currency/commitment_currency_missing
--- FAIL: TestGetStoragePricing_Currency/on-demand_currency_missing
FAIL

Pass after the fix:

cd providers/gcp
go build ./...                       # ok
go vet ./...                         # ok
go test -race -short ./...           # all packages ok, including services/cloudstorage
golangci-lint run --timeout=5m       # v2.10.1: 0 issues

AWS_CONFIG_FILE=/dev/null AWS_SHARED_CREDENTIALS_FILE=/dev/null was set because of an invalid default AWS profile on the author's machine. It has no effect on this GCP-only change.

Gap: this was not checked against a live Cloud Billing Catalog response in a non-USD billing account.

Out of scope: #412 added CommitmentCurrency for Cloud SQL, Memorystore and Compute Engine. Cloud Storage does not produce recommendations (GetRecommendations is not supported), so that part was not applied here.

Labels

The author lacks label permissions. Could a maintainer please mirror the issue's triage labels onto this PR: priority/p2, severity/medium, urgency/this-sprint, impact/few, effort/xs, type/bug, triaged.

🤖 Generated with Claude Code

[coderabbitai]

Warning

Review limit reached

• Run on-demand review

_{This review includes 2 billable files and costs up to $0.50.}

Or wait 42 minutes for your next included review.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

• Configuration used: Repository: LeanerCloud/cloud-commitments-go/.coderabbit.yaml
• Review profile: CHILL
• Plan: Advanced
• Run ID: 85b088cc-2834-452f-b346-5391006afbd0

📥 Commits

Reviewing files that changed from the base of the PR and between c0db8ce and a0ef518.

📒 Files selected for processing: 2

• providers/gcp/services/cloudstorage/client.go
• providers/gcp/services/cloudstorage/pricing_currency_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]

Independent review verdict: CLEAN at 38a6affd8194e0df6fadc81f3f3948d0bc75d601 (merge driver; reviewer did not write the code; base main 740dfbd72456)

• Compared side by side with cloudsql: cloudstorage/client.go:283-312 vs cloudsql/client.go:430-443. The order matches: skip zero price, billingcurrency.Unify, return on error, skumatch.Slot. The USD seed is gone. Unpriced or unmatched SKUs never reach Unify.
• Callers: getStoragePricing (client.go:244) propagates the error. GetOfferingDetails (client.go:176) wraps it.
• Mutation probes, both killed: (1) swallowing the Unify error made TestGetStoragePricing_Currency fail; (2) re-seeding "USD" made the EUR and missing-commitment cases fail.
• Local verification on macOS (fixture/mock evidence, mocked Cloud Billing Catalog, no live GCP call): cd providers/gcp && go test -race -short ./... passes, and go vet ./... passes.
• Non-blocking: a SKU that matches class and region but neither slot still has its currency unified when its price is nonzero. cloudsql does the same.

Remaining merge gates: CI has not run (the fork workflows are awaiting maintainer approval). The PR is a draft. Labels still need mirroring from #243. CR has not reviewed this head because the PR is a draft.

[cristim]

@IAmJSD The independent review is CLEAN at 38a6affd, the evidence is posted, and CI is green. The only remaining blocker is that the branch is BEHIND main by 21 commits, and the default-branch ruleset requires it to be up to date (strict required checks). Could you merge origin/main into the branch, as you did on #422? It merges cleanly and touches none of this PR's files. After that I will confirm the range-diff is empty, re-run the local checks, and squash-merge with --match-head-commit.

[IAmJSD]

Done: I merged origin/main into the branch at 5ec8d57. I merged rather than rebased, so there was no force-push. The merge was automatic and the PR's own commits are unchanged. Locally, with AWS config set to /dev/null, providers/gcp builds and its tests pass. The fork CI runs for the new head need approval.

🤖 Generated with Claude Code

[cristim]

@IAmJSD Thanks for the update. CI is green, the diff is identical to the reviewed one, and the local checks pass. While CI was running, main moved by one docs-only commit (#438), so the strict ruleset shows BEHIND again. Could you merge origin/main one more time? I will merge as soon as CI is green on that head.

[cristim]

@IAmJSD Still fully gated here: CLEAN verdict, CI green, diff identical to the reviewed one. The only blocker is that main has moved since your last update, so the PR shows BEHIND. Could you merge origin/main once more? When the head moves I will ask the other merge drivers on our bus to hold merges until CI finishes, so this does not race again.

[IAmJSD]

Merged origin/main in 3e7d7b8 (no conflicts, no force-push). The touched module's build and go test -short ./... pass under GOWORK=off. Thanks for holding the other merges until CI finishes.

[cristim]

@IAmJSD CI is green on this head too, but #421 just merged, and the strict ruleset allows only one PR to land per update. Could you merge origin/main once more? I'll hold other merges on our side while CI runs, and I'll merge this one first.

[cristim]

Merge gate record at a0ef51838854933cd8745cf87e23168a05461c19 (merge driver)

• Independent review CLEAN at 38a6affd. Every first-parent commit after it is a merge of main. Each merge's tree equals git merge-tree --write-tree of its parents, so the PR's own change is unchanged and the verdict stands. The last merge was GitHub's Update branch, used with the author's consent on fix(azure): attribute shared-scope vm reservations to their billing subscription #423.
• Local verification on macOS at this head (fixture/mock evidence, no live GCP): cd providers/gcp && go test -race -short ./... && go vet ./... passes.
• CI is green on this head: CI - Build & Test 39047305026 and pre-commit 39047304812. mergeState is CLEAN, there are no unresolved threads, and labels are mirrored from fix(gcp/cloudstorage): apply the currency unification to the Cloud Storage extractor #243. CR waived (quota): adversarial review, local verification and green CI cover this head.

Squash-merging with --match-head-commit a0ef5183.