Skip to content

fix(deps): bump providers/gcp past the Cloud Storage recommender removal - #772

Merged
cristim merged 1 commit into
mainfrom
fix/bump-providers-gcp
Oct 9, 2026
Merged

cristim merged 1 commit into
mainfrom
fix/bump-providers-gcp

Conversation

@cristim

@cristim cristim commented Oct 9, 2026 •

Copy link
Copy Markdown
Member

Summary

GCP recommendation collection fails for every region with cloudstorage: iterate recommendations: InvalidArgument (reported in the UI as "all 172 GCP recommendation service calls failed across 43 regions"). The pinned providers/gcp (a32fd1a, 2026-09-29) asks the Recommender API for google.storage.bucket.CostRecommender, which Google does not document (documented Cloud Storage recommenders are SoftDeleteRecommender and AnywhereCacheRecommender), and Google has no Cloud Storage commitment product to recommend. cloud-commitments-go stopped that in #193, #205 and #219 (see also go#199, #78).

This bumps only providers/gcp to cloud-commitments-go 2c18f303abfc (2026-10-09, main). No other module version moves. The provider added CommitmentsService.Get, so the internal/purchase history test fake implements it and fails loudly if a 409 path is ever reached.

The collection error prints only the last failing call, so compute/SQL/cache calls may be failing for other reasons; the per-region WARN logs would show. This PR removes the Cloud Storage cause only.

What else this bump pulls in

providers/gcp moves 21 commits (29 files) between the old and new pin, so the GCP behavior change is wider than the Cloud Storage fix (per the independent review):

  1. Cloud Storage is no longer a supported GCP service or recommendation source (go#219, fix(db): migrate down -all cannot roll back past migration 55, leaves the database dirty #205): the intended fix.
  2. Compute purchases take the commitment type from ComputeDetails.GCPCommitmentType or derive it from the machine family; unknown, shared-core and accelerator types are refused (fail closed, no GENERAL_PURPOSE default). The platform decodes details into the pinned pkg common.ComputeDetails, which has the field; persistence was not traced end to end.
  3. A recommendation with a non-hex idempotency token is rejected before naming a CUD (go#260). A 409/ALREADY_EXISTS re-drive is kept in the error chain and reports the adopted commitment through PurchaseResult.ExistingCommitment (go#223, fix(api): openapi.yaml omits ~30 routed paths and mis-describes documented operations #262). No platform consumer of that flag changes here.
  4. Recommendation pricing now fails on a missing or mixed currency instead of seeding USD; SKU matching uses whole tier tokens; bad amounts, non-positive counts and CPU/memory mismatches are skipped or rejected. Expect fewer or erroring GCP recommendations in some projects.
  5. Dependency security bumps (otel, x/net) from the Go advisory wave; the platform already satisfies them, so only one go.mod line moves.

Verification

  • go test ./internal/purchase ./internal/scheduler pass (the two packages importing the provider), go vet ./internal/..., go build ./cmd/... (GOWORK=off, local Go 1.27.1, no -race).
  • Provider behavior read from the new module source: the Cloud Storage client no longer calls the recommender (GetRecommendations returns ErrCommitmentPurchaseNotSupported) and no CostRecommender string remains.
  • NOT verified against a real GCP project; no cloud calls were made.
  • No closing issue, so no triage labels to mirror.

🤖 Generated with Claude Code

The pinned a32fd1a still queries google.storage.bucket.CostRecommender for
every region, which Google does not document, so GCP recommendation
collection fails with InvalidArgument when every service call fails. The
newer provider (cloud-commitments-go#193, #205, #219) no longer fans out to
Cloud Storage recommendations. Only providers/gcp moves.

The provider grew CommitmentsService.Get, so the purchase history test fake
implements it and fails loudly if a 409 path is ever reached.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Oct 9, 2026

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

  • Run on-demand review

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

  • 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 22 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 85 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-platform/.coderabbit.yaml
  • Review profile: CHILL
  • Plan: Essentials
  • Run ID: b5d0e975-4b85-4c00-9d6b-8263d5137ce6

📥 Commits

Reviewing files that changed from the base of the PR and between 27c1a30 and 46ab2da.


⛔ Files ignored due to path filters (1)
  • go.sum is excluded by !**/*.sum

📒 Files selected for processing (2)
  • go.mod
  • internal/purchase/gcp_history_cost_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

cristim commented Oct 9, 2026

Copy link
Copy Markdown
Member Author

Subject: platform PR #772 @ 46ab2da - APPROVE conditional on green CI, with a behavior-change heads-up

Delta (3 files vs origin/main): go.mod one line (providers/gcp a32fd1a178e9 -> 2c18f303abfc), go.sum 2 lines (that module h1 + go.mod hashes), test fake gets CommitmentsService.Get.
Verified:

  • Pin is cloud-commitments-go main 2c18f303 (2026-10-09T11:34+02:00), the commit that includes aa903fe (fix(db): migrate down -all cannot roll back past migration 55, leaves the database dirty #205 Cloud Storage contract) and 7e8417e (fix(api): PUT /api/plans/{id} resets the ramp schedule and re-buys completed steps #219). go.sum: go mod download -json of the module returns exactly Sum h1:hCLUvpI5U42BRh4M9EvjKaBg70P2ijbKxO9eNn7wMK0= and GoModSum h1:D2ZwYqlDm/304JWfOw8YX/e+/YTc51Q9kiIaHJN7nxY=, matching go.sum (sumdb-checked); go mod verify: all modules verified; go mod tidy -diff: empty.
  • Transitive requirements of the new gcp go.mod (go 1.26.9, pkg 20261007231611 older than platform's a9a4226, x/net 0.60.0, x/sys 0.48.0, x/crypto 0.57.0, x/text 0.42.0, x/sync 0.23.0, otel 1.45.0, logr 1.4.4) are already satisfied by the platform go.mod (go 1.26.9 and those versions), which is why only one go.mod line moves. No other module is silently bumped.
  • Scratch copy of the PR tree (git archive of go.mod, go.sum, cmd, internal, iac; no worktree/clone): go build ./... ok; go vet and go test of ./internal/purchase/... ./internal/scheduler/... (the only importers: execution.go, scheduler.go, gcp_history_cost_test.go) ok.
  • Test fake Get is correct: the provider calls Get only on an insert 409 (re-drive adoption) and the fake errors loudly so the test cannot silently take that path.
    What else the bump pulls in (providers/gcp, 21 commits, 29 files, +2809/-1080 between the old and new pin) - behavior, not just the Cloud Storage fix:
  1. Cloud Storage no longer a supported GCP service / recommendations (fix(api): PUT /api/plans/{id} resets the ramp schedule and re-buys completed steps #219, fix(db): migrate down -all cannot roll back past migration 55, leaves the database dirty #205, 13b78d6): the intended fix.
  2. Compute purchases: commitment type is now taken from ComputeDetails.GCPCommitmentType (json gcp_commitment_type) or derived from the machine family; unknown/shared-core/accelerator types are REFUSED (fail closed, no GENERAL_PURPOSE default). Platform never references GCPCommitmentType, but it decodes details into the pinned pkg's common.ComputeDetails which has the field, so it round-trips; I did not trace persistence end to end.
  3. A recommendation with a non-hex idempotency token is rejected before naming a CUD (fix(api): session-authed revoke skips the revocation window the token path enforces #260); a 409/ALREADY_EXISTS re-drive is now kept in the error chain and surfaces PurchaseResult.ExistingCommitment (adopted commitment) via Get (fix(api): pausing a running execution allows a claimed purchase to be re-claimed #223, fix(api): openapi.yaml omits ~30 routed paths and mis-describes documented operations #262): platform consumers of that flag are not part of this PR.
  4. Recommendations: SKU pricing now FAILS on missing or mixed currency instead of seeding USD (f23518c); whole-tier-token SKU matching; bad amounts, non-positive counts and CPU/memory mismatches are skipped or rejected. Expect fewer or erroring GCP recommendations in some projects; the PR's own caveat (only last failing call printed) matters here.
  5. Dependency security bumps (otel, x/net) from the Go advisory wave.
    Not verified: nothing against a real GCP project (fixture/unit evidence only, as the PR says). I did not run -race or the full platform test suite (disk hold: targeted only).
    CI at 46ab2da: Unit, Integration, Docker, Security, E2E and others were still running when I looked (mergeStateStatus BLOCKED); CodeRabbit null. Approval is conditional on every check green and CLEAN at this exact SHA. Suggest the PR body list items 2-4 so the reviewer of the deploy knows the GCP behavior change is wider than the Cloud Storage fix.

@cristim
cristim merged commit fe13430 into main Oct 9, 2026
25 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant