Skip to content

fix(aws): fail loud on unknown payment-option/term/engine -- remove silent defaults #1084

Closed

Description

@cristim

Problem

Several AWS provider converter functions silently substitute fabricated values when given unrecognized input, then proceed to query offerings and (on the purchase path) commit money to the wrong product:

H3 `recommendations/converters.go` `convertPaymentOption` silently returns `PaymentOptionNoUpfront` for any unrecognized payment option string, causing the SP recommendation-fetch path to query CE for the wrong payment option and return recs with wrong savings/upfront/recurring figures.

H4 `services/elasticache/client.go` `convertPaymentOption` silently returns `"Partial Upfront"` for any unknown option and proceeds to `PurchaseReservedCacheNodesOffering` on that option. The RDS sibling already errors correctly; this is an inconsistency as well as a money bug.

M4 `recommendations/parser_services.go` `parseRDSDetails` defaults `AZConfig` to `"single-az"` when CE omits `DeploymentOption`. Single-AZ and multi-AZ RDS RIs have different prices and do not cover each other; the wrong AZConfig causes `findOfferingID` to query for the wrong RI class.

M5 `recommendations/parser_services.go` `resolveEC2Tenancy` collapses any unrecognized tenancy value (including `"host"` for Dedicated Hosts, which has no EC2 RI product) to `"default"`, risking purchase of a default-tenancy RI for a Dedicated Host workload.

M6 `services/rds/client.go` `normalizeEngineName` hardcodes edition guesses: any Oracle engine -> `oracle-se2`, any SQL Server -> `sqlserver-se`, unknown Aurora variant -> `aurora-mysql`. Oracle/SQL Server editions (SE2/EE, SE/EE/Web/Express) are different products at different prices.

L1/L2 `recommendations/converters.go` `convertTermInYears` and `convertLookbackPeriod` silently default to 1-year / 7-day on unrecognized input, mis-fetching recommendations for the wrong term/lookback window.

Also includes H1/H2 savingsplans fail-loud fixes (cherry-picked from the now-merged #1039 branch): `convertPaymentOption` and `convertTermToSeconds` in `savingsplans/client.go` no longer default to All Upfront / 1yr on unknown input.

M1/M2/M3 `services/ec2/client.go`: `getOfferingClass` raw `"convertible"` string literals replaced with `types.OfferingClassTypeConvertible`; `buildEC2OfferingQuery` errors on empty Platform (purchase path); raw `"Linux/UNIX"` literals centralized into `defaultEC2Platform` constant.

Fix

  • Fail loud (return error) on all unrecognized/absent input on money-affecting paths.
  • Add erroring variants (`convertPaymentOptionE`, `convertTermInYearsE`, `convertLookbackPeriodE`) and wire them into the SP recommendation path in `parser_sp.go`.
  • Leave the legacy non-erroring wrappers in place for `client.go` callers (RI recommendation path, owned by other PRs) with a deprecation comment pointing to the follow-up.
  • Regression tests for every finding that FAIL pre-fix.

Activity

  1. added a commit that references this issue on Jun 7, 2026
    e96693c
  2. added
    pr-createdA PR has been opened for this issue (dedup guard for the auto-PR loop)
    pr-mergedThe PR for this issue has been merged
    on Jun 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    effort/mDaysimpact/all-usersAffects every userpr-createdA PR has been opened for this issue (dedup guard for the auto-PR loop)pr-mergedThe PR for this issue has been mergedpriority/p1Next up; this sprintseverity/highSignificant harmtriagedItem has been triagedtype/bugDefecturgency/nowDrop other things

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions