fix(gcp): stop emitting Cloud SQL and Memorystore CUDs from PerformanceRecommenders

[IAmJSD]

Closes #294

Root cause

cloudsql.Client.GetRecommendations and memorystore.Client.GetRecommendations listed the project-scoped google.cloudsql.instance.PerformanceRecommender and google.memorystore.redis.PerformanceRecommender. Those give per-instance tuning advice (Cloud SQL's are query, index and settings tips), not commitments. Every ACTIVE item was still converted into a CommitmentCUD with the default 1yr term and EstimatedSavings taken from its cost projection, which came out as 0 when the projection was missing and negative when it was a cost increase. The region fan-out ran both on every collection (about 80 extra Recommender calls across ~40 regions). Cloud SQL and Memorystore CUDs are spend-based, and Google recommends them only at Cloud Billing account scope (google.cloudbilling.commitment.SpendBasedCommitmentRecommender), which this project-scoped provider does not collect yet (docs/design/gcp-spend-commitments.md).

go#364 named this issue but did not fix it. Its body says "Not in this PR: G5/#294/#319 (recommender choice)". On main at 7383c3a, both clients still query the PerformanceRecommenders (see the fail-before output below).

Fix

This takes the issue's first suggested option and follows the Cloud Storage precedent (#193, #219):

• Clients: GetRecommendations on both clients returns an error wrapping common.ErrCommitmentPurchaseNotSupported and makes no Recommender call. I removed the code that only served that path: the real recommender wrapper, the page cap from fix(gcp): paginate Cloud SQL/Memorystore SKUs and cap recommendations by page #364, and convertGCPRecommendation with its extractGCP*, fill*Pricing and termYearsFromLabel helpers. SetRecommenderClient and the RecommenderClient/RecommenderIterator types stay as deprecated no-op shims, as in cloudstorage. Pricing (GetOfferingDetails, ValidateOffering, getSQLPricing/getRedisPricing) is unchanged.
• Adapter: the region fan-out (collectRegion) now calls only the Compute Engine commitment recommender. An explicit Cloud SQL or Memorystore request returns the client's not-supported error once, before any region lookup, using the same path as Cloud Storage (collectUnsupportedRecs). If the two services had stayed in the fan-out with an always-failing client, their error could become lastErr and hide a real 403 in the all-failed aggregate (the fix(gcp): stop emitting Cloud Storage recommendations with a bucket name as the tier #193 problem).
• Provider: GetSupportedServices now returns only ServiceCompute. GetServiceClient still builds both clients for pricing and validation.
• CHANGELOG: added a BREAKING entry under Changed.

Verification

All evidence is from unit tests with mocks and fixtures. Provider-level tests use a local httptest regions server and an in-process bufconn gRPC Recommender server; nothing calls Google Cloud. All test runs used AWS_CONFIG_FILE=/dev/null AWS_SHARED_CREDENTIALS_FILE=/dev/null because the default profile on the dev machine is invalid; the GCP tests don't read it.

Regression tests (new or changed):

• providers/gcp/performance_recommender_test.go
  • TestGetAllRecommendations_NoPerformanceRecommenderCUDs: every PerformanceRecommender parent answers with an ACTIVE cost-saving recommendation. The test asserts that no recommendations come back and that only the two Compute UsageCommitmentRecommender parents are queried.
  • TestGetRecommendations_ExplicitSQLAndCacheAreNotSupported: for relational-db and cache, asserts ErrCommitmentPurchaseNotSupported, nil recs, zero Recommender calls and zero region lookups.
• TestCloudSQLClient_GetRecommendations_NotSupported and TestMemorystoreClient_GetRecommendations_NotSupported: an injected recommender returns an ACTIVE recommendation. Each test asserts the sentinel error, nil recs, and that ListRecommendations is never called.
• TestGCPProvider_GetSupportedServices: now asserts []ServiceType{ServiceCompute}.

Fail before (new tests on the unfixed code):

--- FAIL: TestGetAllRecommendations_NoPerformanceRecommenderCUDs
    expected: "compute"  actual: "relational-db"   (... CommitmentType:committed-use Term:1yr EstimatedSavings:100 ...)
    expected: "compute"  actual: "cache"           (x2 regions each)
    Should be empty, but was [4 recs]; elements differ: only the Compute commitment recommender may be queried
--- FAIL: TestGetRecommendations_ExplicitSQLAndCacheAreNotSupported   An error is expected but got nil. (x2)
--- FAIL: TestGCPProvider_GetSupportedServices   [compute relational-db cache] should not contain "relational-db"
--- FAIL: TestCloudSQLClient_GetRecommendations_NotSupported   An error is expected but got nil.
--- FAIL: TestMemorystoreClient_GetRecommendations_NotSupported   An error is expected but got nil.

Pass after: all of the above pass.

Mutation probe: dropping ServiceRelationalDB/ServiceCache from isUnsupportedRecommendationService makes TestGetRecommendations_ExplicitSQLAndCacheAreNotSupported fail for both subtests. In that state an explicit request falls through to an empty fan-out and returns (nil, nil).

Commands, run at the pushed head:

cd providers/gcp
AWS_CONFIG_FILE=/dev/null AWS_SHARED_CREDENTIALS_FILE=/dev/null go test -race -short -count=1 ./...   # all ok
go vet ./... && make vet                                                                             # clean
golangci-lint v2.10.1 run --timeout=10m --allow-parallel-runners --config=../../.golangci.yml ./...   # 0 issues
gocyclo -over 10 providers/gcp/{provider,recommendations}.go providers/gcp/services/{cloudsql,memorystore}/client.go  # silent
bash scripts/run-gosec.sh --module providers/gcp . ./services/cloudsql ./services/memorystore   # rc=0
make tidy-check                                                                                     # clean
npx markdownlint-cli@0.47.0 CHANGELOG.md                                                            # clean
bash scripts/test-git-secrets-allowlist.sh                                                          # 44 PASS, 0 FAIL
git secrets --scan   # all tracked files, fresh `git init` copy with isolated HOME, --register-aws + setup-git-secrets.sh: rc=0

This repo has no internal/mocks guard package; that's a platform-repo check.

Real-cloud gaps: nothing was run against a real project or billing account. The issue's open question, whether google.memorystore.redis.PerformanceRecommender exists at all, no longer matters because nothing queries it.

Removed tests

These tests only covered the removed PerformanceRecommender path:

• GetRecommendations_* with mock iterators
• the fix(gcp): paginate Cloud SQL/Memorystore SKUs and cap recommendations by page #364 page-cap tests: ManyItemsWithinPageCap, ExactlyAtCapSucceeds, PastCapFails, RequestsExplicitPageSize
• the ConvertGCPRecommendation* term, nil-content and billing-failure tests

TestListSKUs_CachedPerClient is kept.

Two tests were reduced rather than removed:

• The *_ConvertGCPRecommendation_RecurringMonthlyCost tests are now *_GetOfferingDetails_RecurringMonthlyCost. They keep the catalog-currency monthly quote check through GetOfferingDetails with a hand-built rec. The scorer and reporter checks went with the converter.
• The pricing_currency_test.go tests keep the getSQLPricing/getRedisPricing currency assertions and drop the fill*Pricing half.

Out of scope

• The billing-account SpendBasedCommitmentRecommender path (design doc steps 1-5, fix(gcp): ResourceType holds an instance name, then is used as a pricing tier #78): needs owner decisions and a captured payload.
• Owner decision needed: this PR ships design-doc step 6 ("retire the non-commitment GCP paths") before step 5. That answers open question 5 with "ship earlier". Until the spend path lands, Cloud SQL and Memorystore show no GCP recommendations, and an explicit request gets the visible not-supported error.
• gcp: Cloud SQL and Memorystore PerformanceRecommender IDs need review #319 (recommender ID and permission review): after this PR neither ID is called. It may be closable, but I've left that to a maintainer.
• Consumers (platform, CLI, MCP) that list relational-db/cache as GCP services will see them disappear from GetSupportedServices after a pin bump. Per the extra context, platform#1035 already narrows the runtime service account to the Compute commitment recommender, so these calls were being denied there anyway.
• Open PRs fix(gcp): price tiered SKUs from the first paid tier #433 and fix(gcp): fail closed on region-less storage SKUs and ambiguous SKU matches #471 edit pricing helpers in the same two client.go files. They may need a trivial rebase but don't touch the removed code.

Labels

Could a maintainer mirror the issue's triage labels onto this PR: priority/p2, triaged, severity/high, urgency/this-quarter, impact/few, effort/m, type/bug.

🤖 Generated with Claude Code

[coderabbitai]

Warning

Review limit reached

• Run on-demand review

_{This review includes 14 billable files and costs up to $3.50.}

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.

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: bf84abbc-c903-4bd5-8bbc-b46930abec50

📥 Commits

Reviewing files that changed from the base of the PR and between d13a0d4 and 3c2e5fd.

📒 Files selected for processing: 14

• CHANGELOG.md
• providers/gcp/performance_recommender_test.go
• providers/gcp/provider.go
• providers/gcp/provider_test.go
• providers/gcp/recommendations.go
• providers/gcp/recommendations_test.go
• providers/gcp/services/cloudsql/client.go
• providers/gcp/services/cloudsql/client_test.go
• providers/gcp/services/cloudsql/pagination_test.go
• providers/gcp/services/cloudsql/pricing_currency_test.go
• providers/gcp/services/memorystore/client.go
• providers/gcp/services/memorystore/client_test.go
• providers/gcp/services/memorystore/pagination_test.go
• providers/gcp/services/memorystore/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 adversarial review: CLEAN at 0600f46735374936fce37e6045a46ecf4690c09f

Reviewer: an independent agent that did not write this change (Sonnet, high effort), cold brief, full diff. The PR body was treated as a claim. Merge driver claude-go-pr-merge-4 followed up on the open notes.

Blocking findings: none.

Caller behaviour:

• "All GCP services" (empty service). The region fan-out (collectRegion) now calls Compute Engine only. Cloud SQL and Memorystore are skipped without failing the collection.
• Explicit RelationalDB or Cache request. collectUnsupportedRecs calls the client once, before getRegions. The error wraps common.ErrCommitmentPurchaseNotSupported, so errors.Is is true. This is the same shape as the Cloud Storage precedent (cloudstorage/client.go:118-123), which is unchanged.
• Provider.GetSupportedServices (provider.go:467) now returns Compute only. This is consistent with the fan-out, but it is not in the PR title. Platform and CLI consumers that iterate supported services will see the narrower list; please call it out in the changelog.

Pricing is untouched. No changed lines in getSQLPricing, getRedisPricing, GetOfferingDetails, ValidateOffering or skuMatches. Nothing in the repo still calls the deleted helpers, including ci_cd_sanity_tests. Currency is still asserted on the pricing path, at cloudsql/client_test.go:273 and :516 and through extractPriceFromSKU in memorystore/client_coverage_test.go:32.

Exported API: nothing exported was removed. SetRecommenderClient (now a no-op) and the RecommenderClient/RecommenderIterator interfaces stay as deprecated shims. GetRecommendations on both clients now always returns the not-supported error, which is the intended behaviour change.

Merge check:

• git merge-tree against origin/main (7383c3a) is clean.
• Against fix(gcp): fail closed on region-less storage SKUs and ambiguous SKU matches #471's head (c99bd7fb, which edits the same two clients' pricing extraction) it is also clean.
• docs/design/gcp-spend-commitments.md, cited in the new error text, exists.

Mutation probes (scratch copy, reverted):

• Cloud SQL GetRecommendations returning nil, nil fails TestCloudSQLClient_GetRecommendations_NotSupported and TestGetRecommendations_ExplicitSQLAndCacheAreNotSupported.
• Dropping RelationalDB from isUnsupportedRecommendationService fails the explicit-request test.

Local verification (macOS, exact SHA 0600f46)

Fixture evidence; no GCP call.

• Fail-before: with the merge-base provider.go, recommendations.go and both client.go files, and the PR's tests kept, 7 tests FAIL. These include TestGetAllRecommendations_NoPerformanceRecommenderCUDs, TestCollectRegion_DefaultFanOutIsComputeOnly, TestGCPProvider_GetSupportedServices and the two GetRecommendations_NotSupported tests.
• Pass-after:
  • (cd providers/gcp && GOWORK=off go test -race -short ./...) passes.
  • make build and make vet pass.
  • (cd ci_cd_sanity_tests && go build ./...) passes.

Labels mirrored from #294.

Needed before merge. @IAmJSD:

1. Mark the PR ready for review.
2. Merge or rebase main if it is behind at that point.
3. Mention the GetSupportedServices narrowing in the PR body.

[cristim]

Merge driver status: draft; waiting on the author to mark it ready. The independent verdict is posted above. Once it is un-drafted, I will update the branch with main (as the author agreed on #423), wait for CI and merge.