Repository navigation
refactor(gcp): consolidate duplicated termYearsFromLabel into shared helper and add 36mo support #15
Description
Context
PR LeanerCloud/cloud-commitments-cli#811 introduced termYearsFromLabel independently in three packages (cloudsql, cloudstorage, memorystore) with identical signatures and bodies. The computeengine package has a functionally equivalent termYearsFromTerm that handles one extra variant: "36mo" -> 3.
Problem
termYearsFromLabelis copy-pasted acrossproviders/gcp/services/cloudsql/client.go,cloudstorage/client.go, andmemorystore/client.go(violates DRY; maintenance hazard).termYearsFromLabeldoes not handle"36mo", so a caller passing"36mo"for a Cloud SQL / Memorystore / Cloud Storage CUD gets 1 year instead of 3 (wrong RecurringMonthlyCost).GetOfferingDetailsin cloudsql and memorystore also has the old inlineif rec.Term == "3yr" || rec.Term == "3"pattern instead of using the new helper.
Proposed fix
- Add a
termYears(term string) intfunction toproviders/gcp/services/common(or a newproviders/gcp/internal/gcputilpackage) that handles all known forms:"1yr","1","12mo","3yr","3","36mo". - Replace the three per-package copies and the two
GetOfferingDetailsinline branches with calls to the shared helper. - Add a table-driven test for all six input forms.
This is a pure refactor; no behavior change for the current production paths (which only use "1yr" or "3yr").
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-034 (medium)
Worth folding into the consolidation: the computeengine package parses the same rec.Term string with two opposite policies. termPlan refuses an unrecognised term with an explicit error and a doc comment stating that 'a silent mis-default can purchase the wrong term and waste money' (providers/gcp/services/computeengine/client.go:46-56), while termYearsFromTerm silently returns 1 for anything it does not recognise (:200-207). The lenient one feeds the pricing lookup at :1157 and RecurringMonthlyCost at :1139; the strict one runs inside GroupCommitments at :568 and on the purchase path at :773. A rec carrying "P3Y" therefore displays one-year economics right up until the purchase fails at termPlan. Having the shared helper return (int, error) and sharing termPlan's switch would resolve both this and the copy-paste. Audit finding A08b-034.
Activity
cristim commentedon Oct 17, 2026
Checked against main: still valid. termYearsFromLabel is duplicated in the cloudsql and memorystore clients, and four inline 3yr branches remain (cloudsql, cloudstorage, memorystore, computeengine). A shared helper touches those client.go files, which open PRs #307, #433, #471 and #493 edit. Blocked on those PRs.