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
termYearsFromLabel is copy-pasted across providers/gcp/services/cloudsql/client.go, cloudstorage/client.go, and memorystore/client.go (violates DRY; maintenance hazard).
termYearsFromLabel does not handle "36mo", so a caller passing "36mo" for a Cloud SQL / Memorystore / Cloud Storage CUD gets 1 year instead of 3 (wrong RecurringMonthlyCost).
GetOfferingDetails in cloudsql and memorystore also has the old inline if rec.Term == "3yr" || rec.Term == "3" pattern instead of using the new helper.
Proposed fix
- Add a
termYears(term string) int function to providers/gcp/services/common (or a new providers/gcp/internal/gcputil package) that handles all known forms: "1yr", "1", "12mo", "3yr", "3", "36mo".
- Replace the three per-package copies and the two
GetOfferingDetails inline 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.
Context
PR LeanerCloud/cloud-commitments-cli#811 introduced
termYearsFromLabelindependently in three packages (cloudsql, cloudstorage, memorystore) with identical signatures and bodies. The computeengine package has a functionally equivalenttermYearsFromTermthat 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
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".GetOfferingDetailsinline branches with calls to the shared helper.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 oforigin/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.