Skip to content

fix(ri-exchange): picker UI + offering-id validation + AWS 4xx mapping (closes #695)#696

Merged
cristim merged 2 commits into
feat/multicloud-web-frontendfrom
fix/695-ri-exchange-picker
May 25, 2026
Merged

fix(ri-exchange): picker UI + offering-id validation + AWS 4xx mapping (closes #695)#696
cristim merged 2 commits into
feat/multicloud-web-frontendfrom
fix/695-ri-exchange-picker

Conversation

@cristim
Copy link
Copy Markdown
Member

@cristim cristim commented May 25, 2026

Summary

Fixes three compounding defects that caused RI Exchange to return a generic 500 when a user typed an instance type ("t3.medium") instead of an AWS offering UUID in the "Target Offering ID" field.

CloudWatch log lines from the issue

2026/05/22 22:26:52 [ERROR] exchange quote failed: operation error EC2: GetReservedInstancesExchangeQuote,
    StatusCode: 400, api error InvalidOfferingId: Invalid Offering ID provided
2026/05/22 22:27:04 [ERROR] exchange quote failed: ... InvalidOfferingId: Invalid Offering ID provided
2026/05/22 22:34:59 [ERROR] exchange quote failed: ... InvalidOfferingId: Invalid Offering ID provided

Defect 1 -- UI: free-text "Target Offering ID" replaced with picker

Before: internal/api/handler_ri_exchange.go accepted any string in targets[].offering_id and forwarded it directly to AWS. The UI (frontend/src/riexchange.ts:355-360) rendered a <input type="text" placeholder="e.g. t3.medium"> -- actively inviting the wrong input.

After: The <input type="text"> is replaced with a <select class="modal-exchange-target-select"> backed by two sources:

  • AWS-driven picker (new GET /api/ri-exchange/target-offerings?source_ri_id=<uuid>) calls ec2svc.Client.ListTargetOfferings which runs DescribeReservedInstancesOfferings with the same typed-field shape from PR fix(purchases): narrow Describe*Offerings + cap pagination (#688) #690, narrowed to convertible + same term/product-desc/scope as the source RI. Options grouped as "AWS Target Offerings".
  • CE Recommendations picker sourced from the existing alternativeTargets from reshape recs (exchange_lookup.go path). Options grouped as "CE Recommendations" with per-instance monthly cost.

A hidden <input type="hidden" class="modal-exchange-target"> holds the resolved UUID and is the only field read by collectTargets() at submission time.

New endpoint: GET /api/ri-exchange/target-offerings -- internal/api/handler_ri_exchange.go:listTargetOfferings, routed in internal/api/router.go:261.

Defect 2 -- Backend validation: UUID format check before AWS call

Before (internal/api/handler_ri_exchange.go:374, 412):

if t.OfferingID == "" {
    return nil, NewClientError(400, fmt.Sprintf("targets[%d].offering_id is required", i))
}

After -- new validateTargets() helper at handler_ri_exchange.go checks each offering_id against offeringIDPattern (^[0-9a-f]{8}-...$), returning 400 with a descriptive message that names the instance type and suggests the fix. Used at both the quote handler and validateExecuteExchangeBody.

Defect 3 -- AWS 4xx mapped to client 4xx with message preserved

Before (internal/api/handler_ri_exchange.go:486-488):

logging.Errorf("exchange quote failed: %v", err)
return nil, NewClientError(500, "exchange quote failed")

After -- new mapAWSExchangeError() helper at handler_ri_exchange.go extracts smithy.APIError via errors.As and maps these client-fault codes to 400 with the original AWS message: InvalidOfferingId, InvalidParameter, ValidationError, InvalidReservedInstancesId.NotFound, InvalidInstanceID.NotFound. All other AWS errors remain 500. Applied at both the quote call site and the execute call site.


Test counts

Backend: 27 new test cases (handler_ri_exchange_test.go) covering:

  • GET /api/ri-exchange/target-offerings: auth gate, missing source_ri_id (400), unknown source RI (404), happy path (2 offerings returned)
  • UUID format validation: "t3.medium" -> 400 at quote handler; valid UUID passes; validateExecuteExchangeBody rejects instance types
  • AWS error classification: 5 client-fault codes -> 400 + message preserved; unrecognised code -> 500; plain error -> 500

Frontend: 5 new test cases (riexchange.test.ts) covering:

  • Picker select exists, no visible text input
  • CE recommendation options appear in select synchronously
  • AWS offerings appear after async load; listTargetOfferings called with source RI ID
  • Picker selection drives hidden input
  • Non-UUID value in hidden input is rejected before API call

Total tests now: 1938 backend passing, 1938 frontend passing (1 pre-existing skip).


Cross-references

Closes #695
Refs #688 #694 PR #690

cristim added 2 commits May 25, 2026 18:49
…4xx mapping (#695)

Three defects addressed:

Defect 1 (backend): add GET /api/ri-exchange/target-offerings backed by
ListTargetOfferings on ec2svc.Client. Uses typed-field approach from PR
#690 to enumerate valid convertible exchange targets for a source RI.
Introduces targetOfferingsEC2Client interface + factory in handler.go for
testability.

Defect 2: validate targets[].offering_id against a UUID regex at both the
getExchangeQuote and validateExecuteExchangeBody call sites. Returns 400
with a descriptive message when an instance type (e.g. "t3.medium") is
submitted instead of an offering UUID.

Defect 3: mapAWSExchangeError helper extracts smithy.APIError and maps
InvalidOfferingId/InvalidParameter/ValidationError/Invalid*.NotFound to
400 with the original AWS message; all other AWS errors stay 500. Applied
at the quote and execute call sites.

Tests: 27 new test cases covering the new endpoint (happy path, 404, auth),
UUID format rejection, and AWS 4xx/5xx error classification.

Closes #695
Replace the "Target Offering ID" free-text input in the exchange modal
with a <select> picker backed by two sources:

- AWS-driven: GET /api/ri-exchange/target-offerings loads valid target
  offerings for the source RI from DescribeReservedInstancesOfferings
  (the new endpoint from the companion backend commit). Options are
  grouped under "AWS Target Offerings".

- CE Recommendations: alternativeTargets from reshape recommendations are
  listed as a second optgroup "CE Recommendations" with per-instance
  monthly cost displayed in the option label.

The picker drives a hidden <input type="hidden" class="modal-exchange-target">
that holds the resolved UUID. collectTargets() validates the value against
a UUID regex before submission, rejecting any non-UUID value with a
user-visible error message. This closes the path where a user could
accidentally submit an instance type (e.g. "t3.medium") instead of a UUID.

Tests: 5 new test cases covering the picker select rendering, CE options
population, AWS offerings after async load, select-to-hidden-input sync,
and non-UUID rejection at submission time. Updated 8 existing tests to
use offering_id UUIDs in place of instance types where they inject values
directly into the hidden offering input.

Closes #695
@cristim cristim added triaged Item has been triaged priority/p1 Next up; this sprint severity/high Significant harm urgency/now Drop other things impact/all-users Affects every user effort/m Days type/bug Defect labels May 25, 2026
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 25, 2026

Warning

Review limit reached

@cristim, we couldn't start this review because you've used your available PR reviews for now.

Your plan includes 5 reviews of capacity. Refill in 13 minutes and 24 seconds.

Your organization has run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After more review capacity refills, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than trial, open-source, and free plans. In all cases, review capacity refills continuously over time.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: abb22bf2-2eee-41e0-ae70-8287c56f70e4

📥 Commits

Reviewing files that changed from the base of the PR and between 423e87c and f47a77b.

📒 Files selected for processing (11)
  • frontend/src/__tests__/riexchange.test.ts
  • frontend/src/api/index.ts
  • frontend/src/api/riexchange.ts
  • frontend/src/api/types.ts
  • frontend/src/riexchange.ts
  • go.mod
  • internal/api/handler.go
  • internal/api/handler_ri_exchange.go
  • internal/api/handler_ri_exchange_test.go
  • internal/api/router.go
  • providers/aws/services/ec2/client.go
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/695-ri-exchange-picker

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

@cristim
Copy link
Copy Markdown
Member Author

cristim commented May 25, 2026

@coderabbitai review

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 25, 2026

✅ 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.

@cristim cristim merged commit f3240f4 into feat/multicloud-web-frontend May 25, 2026
4 of 5 checks passed
@cristim cristim deleted the fix/695-ri-exchange-picker branch May 25, 2026 17:48
cristim added a commit that referenced this pull request May 25, 2026
…closes #564) (#697)

* ci(pre-commit): auth tflint via GITHUB_TOKEN + retry transient flakes (closes #564)

The `Run pre-commit` step in `.github/workflows/pre-commit.yml`
intermittently fails because `terraform_tflint`'s `tflint --init`
downloads ruleset plugins from the GitHub Releases API anonymously,
hitting the 60-requests-per-hour per-IP limit that the runner shares
with every other workflow running on the same NAT egress. Most
recently observed on PR #696
(https://github.com/LeanerCloud/CUDly/actions/runs/26411029351).

Two-part fix:

1. Expose secrets.GITHUB_TOKEN to the step. tflint reads GITHUB_TOKEN
   natively; with it set the limit jumps to 5000/hr per-token, which
   matters because the token is unique per workflow run, not per IP.
   This alone fixes the rate-limit class.

2. Wrap the step with nick-fields/retry@v3.0.2 (SHA-pinned per
   project policy) at 3 attempts with a 90s wait. The token fix
   eliminates the AWS-plugin rate-limit case; the retry handles the
   residual flakes (GitHub Releases availability blips, plugin
   download timeouts, etc.) so a transient failure no longer requires
   a manual rerun.

Acceptance criteria from #564:
- GITHUB_TOKEN exposed to the `Run pre-commit` step
- Three consecutive `pre-commit` runs in the same hour all complete
  without a tflint rate-limit failure (the retry-wrapper also catches
  the residual flakes the token cannot)

* ci(pre-commit): bump job timeout to accommodate retry budget (CR #697)

CR pointed out the retry policy was ineffective: `nick-fields/retry`
uses `timeout_minutes` PER ATTEMPT (not total), so with
`max_attempts: 3` and `timeout_minutes: 15` the worst case is
~48 min, but the job-level `timeout-minutes: 15` killed any retry
before attempt 2 could start.

Fix: tune both timeouts so the retry budget fits inside the job
budget with safety margin.

- per-attempt timeout: 15 -> 10 minutes (normal pre-commit runs
  take ~3-5 min, 10 is comfortable margin without inflating
  hang-detection latency)
- job timeout: 15 -> 35 minutes
  worst case = 3 attempts * 10 min + 2 * 90 s retry waits = 33 min,
  fits in 35 min with margin

Avoids CR's suggested 50-minute job cap because that would also
delay killswitch on a genuinely hung step. 35 min is the tightest
value that still lets all 3 attempts complete.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

effort/m Days impact/all-users Affects every user priority/p1 Next up; this sprint severity/high Significant harm triaged Item has been triaged type/bug Defect urgency/now Drop other things

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant