test(gcp): keep fixture pricing tests off ambient cloud credentials

[cristim]

Summary

Some GCP fixture tests built a resource and then ran the real converter into getOrCreateBillingService. When no billing fake was injected, the Cloud Billing client was built from the developer's ambient Application Default Credentials (ADC), so the test could send real catalog requests. Production logs that client-creation error and swallows it, so these tests still passed.

Tests fixed. Five tests now inject the existing MockBillingService through SetBillingService. Their outcome is unchanged: pricing is reported as unknown, with no network use.

• cloudsql: TestCloudSQLClient_GetRecommendations_WithMock, TestCloudSQLClient_ConvertGCPRecommendation
• memorystore: TestMemorystoreClient_GetRecommendations_WithMockClient, TestMemorystoreClient_ConvertGCPRecommendation
• computeengine: TestGetRecommendationsStringMemoryAmountEndToEnd

The issue named three of these; the guard below found the other two.

Guard. New package providers/gcp/internal/billingguard, called from a TestMain in the four billing packages: cloudsql, memorystore, cloudstorage and computeengine.

• It points GOOGLE_APPLICATION_CREDENTIALS at a nonexistent file. Both cloud.google.com/go/auth and x/oauth2/google then hard-fail and never fall back to the HOME gcloud file or the metadata server.
• As a backstop, it also sets an unreachable GCE_METADATA_HOST and points CLOUDSDK_CONFIG at a nonexistent directory.
• It fails the package if any test logs "failed to create billing service", so a missing fake fails loudly instead of passing silently.
• There are no production changes.

Limitations:

• The detection depends on that production log text, so a reworded message would silence it. The env isolation does not depend on it.
• A hit fails the whole package, not the individual test. The printed log line identifies the test.

Closes #406

Verification (macOS, Go 1.26.9)

• (cd providers/gcp && GOWORK=off go test -race -short ./...): all packages ok, both with the normal environment and with ambient credentials disabled.
• make build, make vet: pass. golangci-lint v2.10.1 (CI pin): 0 issues. gofmt clean.
• Fail-before: with the guard and no fakes, the guard reports 2 hits in cloudsql, 2 in memorystore and 1 in computeengine, and each package fails. Example line: pricing unavailable for redis-1 ... failed to create billing service: open /nonexistent/billingguard-no-adc.json. cloudstorage is clean.
• Independent non-author review: no actionable findings on tree 54328065.
  • It traced the ADC lookup in both credential libraries.
  • A deletion probe confirmed the detection is needed: with env isolation only, a missing fake passes silently.
  • A mutation removing all five fakes failed all three packages.
  • It checked that no open PR touches these files.

Nothing was run against a real account.

🤖 Generated with Claude Code

[coderabbitai]

Warning

Review limit reached

• Run on-demand review

_{This review includes 8 billable files and costs up to $2.00.}

• 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 5 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 64 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: 7006beb1-f655-4f8e-841c-04fdf22e25c3

📥 Commits

Reviewing files that changed from the base of the PR and between 551be56 and 1454b9b.

📒 Files selected for processing: 8

• providers/gcp/internal/billingguard/billingguard.go
• providers/gcp/services/cloudsql/client_test.go
• providers/gcp/services/cloudsql/main_test.go
• providers/gcp/services/cloudstorage/main_test.go
• providers/gcp/services/computeengine/client_test.go
• providers/gcp/services/computeengine/main_test.go
• providers/gcp/services/memorystore/client_test.go
• providers/gcp/services/memorystore/main_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 review verdict: CLEAN at 96bad3e

Reviewer: fresh Claude agent (Sonnet, high effort), not the author. The PR body was treated as a claim. Coordinator: claude-cc-reviewer.

Blocking findings: none.

What was checked:

• billingguard.go (the only non-test file, :1-58): Run(m *testing.M) wraps TestMain. It isolates the env, hooks log output, and fails the binary if any test reached the real billing client. Only the four new main_test.go files import it; no production package does. It cannot change production behaviour.
• Isolation: tests were run with a fake environment: GOOGLE_APPLICATION_CREDENTIALS=/nonexistent, an empty CLOUDSDK_CONFIG, GCE_METADATA_HOST=127.0.0.1:9 and HTTPS_PROXY=http://127.0.0.1:9.
  • origin/main: 5 "failed to create billing service" hits (cloudsql 2, computeengine 1, memorystore 2), and the fixture tests still passed silently.
  • This head: 0 hits, and all four packages pass.
• Assertions: none weakened. Existing tests only gain 5 SetBillingService(&MockBillingService{...}) lines using the existing mock.
• go vet ./... (providers/gcp) is clean. go test -race -count=1 ./services/... ./internal/billingguard/... passes 656, with 0 skips.
• Mutation probe: removing one fake makes cloudsql fail with "billingguard: 1 test(s) reached the real billing service".

Non-blocking: detection relies on the production log text "failed to create billing service". A reworded message would silence it, which the body already discloses.

Gate:

• Labels are mirrored from test(gcp): isolate fixture pricing tests from ambient cloud credentials #406.
• CR waived: quota.
• CI must be green at this SHA. The current Lint Code (providers/azure) failure is in a module this PR doesn't touch, and is being checked. (claude-cc-reviewer)

[cristim]

CI on 96bad3e failed two jobs:

• Security Scanning: gosec v2.28.0 reported G104, an unchecked os.Setenv at providers/gcp/internal/billingguard/billingguard.go:43. Fixed in 21efb76: Run now reports a failure to isolate a variable and returns 1 before m.Run, because running the tests unisolated would risk using real credentials. Locally, pinned gosec exits 0 on providers/gcp, golangci-lint v2.10.1 reports 0 issues, and the services race tests pass. The same non-author reviewer checked the delta and found nothing actionable.
• Lint Code (providers/azure): a golangci schema-download timeout (context deadline exceeded fetching golangci.v2.10.jsonschema.json), the known flake tracked in ci: investigate schema-download failures before pinned lint verification #420. This PR doesn't touch azure.

[cristim]

Delta review: CLEAN at 21efb76

Same independent reviewer, delta 96bad3e6..21efb76c only (billingguard.go, +10/-3).

• Isolation only gets stricter: the same env keys and values are set, and CLOUDSDK_CONFIG now uses filepath.Join for the same path. Setenv/Unsetenv errors now abort with a stderr message and return 1. TestMain is os.Exit(billingguard.Run(m)), so the binary exits non-zero before m.Run and can't pass silently.
• Runs (providers/gcp, GOWORK=off, GOTOOLCHAIN=go1.26.9): go vet ./... is clean, and go test -count=1 ./services/... ./internal/billingguard/... passes 656. golangci-lint (2.11.4, installed locally, not the CI-pinned 2.10.1) reports no issues.

The verdict covers 21efb76. Merge waits on CI being green at this head. (claude-cc-reviewer)

[cristim]

CI is green on head 21efb76: pre-commit 38919467162 and Build & Test 38919467035.

[cristim]

Rebased onto main (server-side gh pr update-branch --rebase; the branch was BEHIND after #427 and #428 merged). git range-diff 96bad3e6~1..21efb76c origin/main..1454b9b2: 1: 96bad3e = 1: f761f12 and 2: 21efb76 = 2: 1454b9b, so both commits are identical. The CLEAN verdict and the delta verdict carry over to 1454b9b. Merge waits on CI at this head. (claude-cc-reviewer)