Skip to content

fix(api/validation): provider-canonical payment-option whitelist (closes #717) - #793

Merged
cristim merged 1 commit into
feat/multicloud-web-frontendfrom
fix/717-from-bucket-c
Jun 5, 2026
Merged

fix(api/validation): provider-canonical payment-option whitelist (closes #717)#793
cristim merged 1 commit into
feat/multicloud-web-frontendfrom
fix/717-from-bucket-c

Conversation

@cristim

@cristim cristim commented May 28, 2026

Copy link
Copy Markdown
Member

Summary

  • Replaces the hand-maintained purchasePaymentWhitelist var with purchasePaymentSet(), which derives the accepted token set directly from config.ValidPaymentOptionsByProvider -- single source of truth, no drift between the plan-validator and the purchase-execute boundary.
  • Applies config.NormalizePaymentOption before the whitelist check so legacy AWS-style tokens submitted against Azure/GCP are coerced to the provider-canonical spelling before validation (preserving backward compatibility for existing callers).
  • Tightens the rejection error to name the provider and list its valid options: invalid payment option for azure service: "foo" (valid for azure: upfront, monthly).

Before / after whitelist shape

Before (hand-maintained, over-permissive):

aws:   {all-upfront, partial-upfront, no-upfront}
azure: {all-upfront, upfront, no-upfront, monthly}   ← cross-provider tokens accepted
gcp:   {all-upfront, upfront, no-upfront, monthly}   ← cross-provider tokens accepted

After (derived from config.ValidPaymentOptionsByProvider, #709's canonical source):

aws:   {no-upfront, partial-upfront, all-upfront}
azure: {upfront, monthly}
gcp:   {monthly}

Legacy cross-provider tokens (e.g. all-upfront on Azure) are coerced via NormalizePaymentOption at ingest, so existing callers continue to work.

Error message format

invalid payment option for azure service: "all-upfront" (valid for azure: upfront, monthly)

Test counts

28 test cases in TestValidatePurchaseRecommendation (up from 14), plus 1 new TestValidatePurchaseRecommendation_ErrorMessage. All 1367 tests in internal/api/... pass.

Closes #717

Summary by CodeRabbit

  • Bug Fixes
    • Strengthened validation for purchase recommendations by enforcing provider-specific payment option requirements with clearer error messaging
    • Improved error messages to display valid payment options for each provider when mismatched payment combinations are submitted
    • Enhanced payment option normalization to support legacy aliases and case-insensitive handling across different cloud providers

Review Change Stack

Replace the hand-maintained purchasePaymentWhitelist var with
purchasePaymentSet(), which derives the accepted token set from
config.ValidPaymentOptionsByProvider -- the same source used by the
plan-validator in #709, eliminating drift between the two boundaries.

Apply config.NormalizePaymentOption before the whitelist check so legacy
AWS-style tokens on Azure/GCP are coerced to canonical spellings (e.g.
all-upfront -> upfront on Azure) before validation, preserving backward
compatibility for existing callers.

Tighten the rejection error to name the provider and list its valid
options: `invalid payment option for azure service: "foo"
(valid for azure: upfront, monthly)`.

Add 15 new test cases covering the per-provider canonical sets, the
cross-provider normalization paths, and the error message shape.
@cristim cristim added triaged Item has been triaged priority/p2 Backlog-worthy severity/medium Moderate harm urgency/this-sprint Within the current sprint impact/many Affects most users effort/s Hours type/bug Defect labels May 28, 2026
@coderabbitai

coderabbitai Bot commented May 28, 2026

Copy link
Copy Markdown
Contributor

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: f878eb9e-8133-4b3d-ae01-ad7946e91d55

📥 Commits

Reviewing files that changed from the base of the PR and between 4956d66 and c176f19.

📒 Files selected for processing (2)
  • internal/api/handler_purchases_guards_test.go
  • internal/api/validation.go

📝 Walkthrough

Walkthrough

This PR tightens purchase recommendation validation to enforce provider-specific payment-option sets instead of a monolithic global whitelist. The validation now derives canonical valid payments from config.ValidPaymentOptionsByProvider, normalizes legacy payment tokens before validation, and reports provider-aware error messages listing valid options.

Changes

Provider-scoped purchase payment validation

Layer / File(s) Summary
Dynamic provider-scoped payment validation
internal/api/validation.go
Replaced the hardcoded purchasePaymentWhitelist with a purchasePaymentSet(provider) helper that derives accepted tokens from config.ValidPaymentOptionsByProvider; validatePurchaseRecommendation now normalizes payment tokens using config.NormalizePaymentOption before validating them against provider-canonical sets and rejects unknown providers. Error messages now list provider-specific valid options.
Test coverage for provider-specific validation
internal/api/handler_purchases_guards_test.go
Updated TestValidatePurchaseRecommendation table-driven cases to verify provider-canonical payment validation including Azure/GCP legacy token normalization and stricter mismatch rejections; added TestValidatePurchaseRecommendation_ErrorMessage to verify error messages include the provider name and list valid payment options for that provider.

Sequence Diagram

sequenceDiagram
  participant RecommendationInput
  participant validatePurchaseRecommendation
  participant purchasePaymentSet
  participant NormalizePaymentOption
  participant ValidPaymentOptionsByProvider
  RecommendationInput->>validatePurchaseRecommendation: (provider, payment)
  validatePurchaseRecommendation->>purchasePaymentSet: normalized provider
  purchasePaymentSet->>ValidPaymentOptionsByProvider: lookup canonical set
  ValidPaymentOptionsByProvider-->>purchasePaymentSet: payment token set
  purchasePaymentSet-->>validatePurchaseRecommendation: derived set or nil
  validatePurchaseRecommendation->>NormalizePaymentOption: (provider, payment) for alias coercion
  NormalizePaymentOption-->>validatePurchaseRecommendation: normalized payment token
  validatePurchaseRecommendation->>validatePurchaseRecommendation: check normalized token in set
  validatePurchaseRecommendation-->>RecommendationInput: valid or error with provider + valid options
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related issues

  • #717: This PR directly addresses the issue by tightening purchasePaymentWhitelist to provider-canonical sets, mirroring PR #709's plan-validator approach, applying NormalizePaymentOption for alias coercion, and including provider-aware error messages with valid options.

Possibly related PRs

  • LeanerCloud/CUDly#709: The main PR's validatePurchaseRecommendation now derives provider-canonical valid payment tokens and applies config.NormalizePaymentOption, directly depending on the provider-scoped ValidPaymentOptionsByProvider/normalization behavior introduced in PR #709.
  • LeanerCloud/CUDly#655: Both PRs update the web purchase recommendation guard logic around validatePurchaseRecommendation/payment-provider validation (canonical Term/Payment/Provider/Service handling, alias/normalization, and stricter mismatch rejection) in the shared backend validation layer.

Poem

🐰 A whitelist once cast its wide net,
But each provider marched to their beat!
Now Azure hops upfront, GCP stays monthly sweet,
Normalized aliases, one canonical set—
The boundary guards align their feet! 🎉

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly describes the main change: replacing the old purchasePaymentWhitelist with a provider-canonical version to close issue #717.
Linked Issues check ✅ Passed The PR implements all primary objectives from issue #717: replaces purchasePaymentWhitelist with provider-keyed lookup using ValidPaymentOptionsByProvider, applies NormalizePaymentOption for legacy token coercion, tightens error messages to include provider and valid options, enforces provider-specific sets, and adds comprehensive test coverage.
Out of Scope Changes check ✅ Passed All changes are directly scoped to the linked issue #717: validation logic updates, test additions, and error message refinements. No unrelated modifications detected.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/717-from-bucket-c

Comment @coderabbitai help to get the list of available commands and usage tips.

@cristim

cristim commented May 28, 2026

Copy link
Copy Markdown
Member Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented May 28, 2026

Copy link
Copy Markdown
Contributor
✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai

coderabbitai Bot commented May 28, 2026

Copy link
Copy Markdown
Contributor

Actionable comments posted: 0

@cristim

cristim commented May 30, 2026

Copy link
Copy Markdown
Member Author

@coderabbitai full review

@coderabbitai

coderabbitai Bot commented May 30, 2026

Copy link
Copy Markdown
Contributor

Rate Limit Exceeded

@cristim have exceeded the limit for the number of chat messages per hour. Please wait 54 minutes and 19 seconds before sending another message.

@cristim

cristim commented Jun 4, 2026

Copy link
Copy Markdown
Member Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jun 4, 2026

Copy link
Copy Markdown
Contributor
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@cristim
cristim merged commit dceb8b0 into feat/multicloud-web-frontend Jun 5, 2026
4 checks passed
@cristim
cristim deleted the fix/717-from-bucket-c branch July 27, 2026 11:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

effort/s Hours impact/many Affects most users priority/p2 Backlog-worthy severity/medium Moderate harm triaged Item has been triaged type/bug Defect urgency/this-sprint Within the current sprint

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant