Skip to content

fix(gcp): read the CUD commitment Type from the recommender payload instead of deriving it from the machine family #60

Description

@cristim

Context

PR LeanerCloud/cloud-commitments-cli#1650 (issue LeanerCloud/cloud-commitments-cli#1538) fixed GCP CUDs being purchased as GENERAL_PURPOSE
regardless of the recommended machine family. It derives the commitment Type
from the machine-family segment of rec.ResourceType via a hand-maintained
machineFamilyCommitmentType map in
providers/gcp/services/computeengine/client.go.

That map is a derivation, and therefore a guess that has to be kept in sync
with GCP:

  • New families need a map entry or they hit the fail-loud path and their
    purchases are refused (already true today for m4, x4, t2a).
  • Families whose commitment type splits into size buckets not encoded in the
    machine-type name (MEMORY_OPTIMIZED_M4 vs _M4_6TB; the seven
    MEMORY_OPTIMIZED_X4_* buckets) are not derivable at all from the machine
    type, so they can never be supported this way.

What to do instead

The Commitment Recommender's own operation payload carries the commitment
resource it wants created, including its type field. GCP is telling us which
Commitment_Type to buy; we do not have to infer it.

Read the commitment type from the recommender operation value (the operation
whose path targets the commitment resource root), validate it against
computepb.Commitment_Type_value so an unknown member fails loud rather than
being passed through as a bare string, and use the machine-family map only as a
fallback for payloads that omit it.

This removes the guess entirely and makes the size-bucketed M4/X4 families
purchasable.

Why it was not done in LeanerCloud/cloud-commitments-cli#1650

buildInsertRequest receives a common.Recommendation, not the raw
recommenderpb.Recommendation. Carrying the commitment type from the converter
to the purchase path requires a new field on pkg/common.Recommendation (or on
common.ComputeDetails), which lives in the separate pkg Go module shared
by the AWS, Azure and GCP providers
, and which is persisted. That is a design
change touching all three providers plus storage, not a bug fix, and folding it
into a p0 money-path PR would have made that PR unreviewable.

Acceptance

  • The purchased Type comes from the recommender payload when present.
  • An unrecognized type value fails loud (no fallback to a guessed family type,
    no bare string literal on the wire).
  • The machine-family map remains as the documented fallback.
  • Regression coverage asserts on the Type carried by the
    InsertRegionCommitmentRequest handed to the SDK client, not on an
    intermediate struct.
  • At least one M4 or X4 payload, currently refused, becomes purchasable.

Related: LeanerCloud/cloud-commitments-cli#1538, LeanerCloud/cloud-commitments-cli#1650, #14, #22.

Findings from the 2026-09-02 codebase audit

Added by an automated audit of 3c0f8ac94048a2c36fce5ccddee54e6c4849a5cd (tip of origin/main). Each item below was reported by one reviewer and independently confirmed by a second that did not write it. Full report: docs/audits/codebase-audit-2026-09-02.md.

A08b-029 (high)

Sibling defect on the read side of the same field, worth folding into whatever lands here. convertCommitment at providers/gcp/services/computeengine/client.go:524 sets com.ResourceType = commitment.Resources[0].Type, which the repo's own doc at :535-538 says is the VCPU/MEMORY/LOCAL_SSD enum, so every existing commitment reports ResourceType: "VCPU". Recommendations set ResourceType to a machine type via extractResourceTypeFromRecommendation (:1197-1222), so no existing commitment can ever match a recommendation by resource type and dedupe/coverage see zero overlap, re-recommending capacity the project already owns. commitment.Type (e.g. GENERAL_PURPOSE_N2), the field that does identify the covered family, is never read in the converter. If this issue lands a recommender-supplied Type, the same value should be carried here as the inverse of machineFamilyCommitmentType. Audit finding A08b-029.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions