fix(purchase): map canonical slugs to canonical ServiceType (closes #626) by cristim · Pull Request #630 · LeanerCloud/CUDly

cristim · 2026-05-20T23:16:28Z

Summary

Fixes #626. Manager.mapServiceType collapsed canonical service slugs onto AWS-legacy ServiceType constants, breaking every Azure (and pre-emptively GCP) approve for Compute, Cache, RelationalDB, Search, and DataWarehouse. AWS provider accepts both forms so AWS kept working; Azure/GCP are canonical-only and rejected the legacy constants with unsupported service: ec2 (or the equivalent).

Root cause

internal/purchase/execution.go:566-581 mapped "compute" to common.ServiceEC2 instead of common.ServiceCompute (same shape for "cache", "relational-db", "search", "data-warehouse"). The existing Azure-rec mock in execution_test.go:1036 encoded the bug by expecting ServiceEC2 for a Service:"compute" rec, so CI passed while production failed.

Changes

internal/purchase/execution.go — split mapServiceType cases so canonical slugs map to canonical constants. Legacy AWS slugs still map to the AWS-legacy constants for backward compat (AWS provider accepts both forms).
internal/purchase/execution_test.go — fix TestExecutePurchase_SingleAccount_AzureUsesResolvedCreds mock expectation from ServiceEC2 to ServiceCompute. Add TestExecutePurchase_AzureCanonicalServiceTypes covering compute/cache/relational-db Azure paths end-to-end.
internal/purchase/coverage_extra_test.go — extend TestMapServiceType_AllBranches with canonical-arm assertions for every service.

Test plan

go test ./internal/purchase/... -count=1 — all green including new tests.
go vet ./... clean.
go build ./... succeeds.
After merge + deploy: user cancels failed execution 757c9a6e-b22a-4a81-bf04-b11043cbf658 and re-creates the Standard_B1ls Azure compute purchase; approval succeeds.

Deploy note

The user has a stuck Azure approval (execution 757c9a6e-b22a-4a81-bf04-b11043cbf658) that needs to be cancelled and re-created after this lands and the Lambda deploys.

Summary by CodeRabbit

Bug Fixes
- Service slug mappings now distinguish canonical service types from legacy AWS names while preserving backward compatibility for AWS legacy slugs.
Tests
- Updated purchase execution tests to reflect the canonical vs. legacy service-type split.
- Added tests verifying canonical Azure service slugs map correctly and that execution succeeds across relevant service cases.

coderabbitai · 2026-05-20T23:16:35Z

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: fc30d7b8-9dd0-4223-9e7a-2eed58833405

📥 Commits

Reviewing files that changed from the base of the PR and between 9ecab71 and a87c0c4.

📒 Files selected for processing (4)

internal/purchase/coverage_extra_test.go
internal/purchase/execution.go
internal/purchase/execution_test.go
internal/purchase/manager_test.go

📝 Walkthrough

Walkthrough

Refactors purchase service-type mapping to distinguish canonical slugs from AWS-legacy slugs, normalizing Savings Plans first, and updates unit and integration tests so Azure/GCP receive canonical ServiceType constants while AWS legacy mappings remain supported.

Changes

Canonical vs. legacy service type mapping

Layer / File(s)	Summary
Service slug mapping refactor `internal/purchase/execution.go`	New `mapServiceSlug` helper and updated `Manager.mapServiceType` to normalize Savings Plans then resolve canonical vs legacy slug mappings; unknown slugs passthrough as `common.ServiceType`.
Service type mapping unit tests `internal/purchase/coverage_extra_test.go`	`TestMapServiceType_AllBranches` rewritten to assert canonical slug → canonical `common.ServiceType` mappings and AWS-legacy slug → legacy `common.ServiceType` mappings.
Coverage test expectations updated `internal/purchase/coverage_extra_test.go`	Multiple mocked `GetServiceClient` expectations and table entries updated to use canonical or legacy `common.ServiceType` constants for EC2/RDS/ElastiCache/OpenSearch/Redshift cases.
Execute purchase tests (AWS/Azure) updated `internal/purchase/execution_test.go`	Many execute-purchase tests now expect `ec2` → `common.ServiceEC2` for AWS; Azure tests assert canonical slugs map to canonical `common.ServiceType`. Adds `TestExecutePurchase_AzureCanonicalServiceTypes`.
Manager due-purchase test update `internal/purchase/manager_test.go`	Mocked `GetServiceClient` in the due-purchase test updated to expect `common.ServiceEC2` for `us-east-1`.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

LeanerCloud/CUDly#501: Related changes touching purchases and Savings Plans normalization.
LeanerCloud/CUDly#629: Overlapping updates to service slug mapping and tests for canonical vs legacy mappings.
LeanerCloud/CUDly#94: Related adjustments around Savings Plans normalization used by service-type resolution.

Poem

🐰 I hop through slugs both new and old,

Canonical paths now firm and bold.
EC2 keeps legacy, Compute wears the crown,
Azure smiles — no more falling down.
Hooray, the mappings all align!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 70.59% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	Title clearly summarizes the main change: mapping canonical slugs to canonical ServiceType constants instead of AWS-legacy ones, addressing the root cause of the bug.
Linked Issues check	✅ Passed	All requirements from issue `#626` are met: mapServiceType splits canonical vs. legacy slugs [`#626`], Azure mocks updated to canonical constants [`#626`], and unit/integration tests added for canonical and legacy mappings [`#626`].
Out of Scope Changes check	✅ Passed	All changes directly address issue `#626`: refactoring mapServiceType logic, updating test expectations, and adding comprehensive test coverage for canonical/legacy slug handling.

_{✏️ 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/issue-626-mapservicetype-canonical

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

cristim · 2026-05-20T23:16:36Z

@coderabbitai review

coderabbitai · 2026-05-20T23:16:41Z

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

) Manager.mapServiceType collapsed the canonical service slugs (compute, relational-db, cache, search, data-warehouse) onto the AWS-legacy ServiceType constants (ServiceEC2, ServiceRDS, ServiceElastiCache, ServiceOpenSearch, ServiceRedshift). The AWS provider's GetServiceClient accepts both legacy and canonical, so AWS approves kept working. Azure and GCP providers are canonical-only, so passing ServiceEC2 for a Service:"compute" rec fell through to the default branch and returned "unsupported service: ec2". User-visible production failure on execution 757c9a6e-b22a-4a81-bf04-b11043cbf658 (Standard_B1ls Azure compute). Split the switch so canonical slugs map to canonical ServiceType constants and legacy slugs keep mapping to the AWS-legacy constants. Backward-compatible for AWS (provider switch accepts both); unblocks Azure now and pre-emptively fixes the same shape of bug for GCP once those code paths are exercised. After deploy, the failed execution above must be cancelled and re-created by the user to retry approval.

TestExecutePurchase_SingleAccount_AzureUsesResolvedCreds wired its mock GetServiceClient to expect common.ServiceEC2 for a rec carrying Service:"compute". The mock returned success regardless of the ServiceType passed in, so CI stayed green while the real Azure provider rejected the same call in production as "unsupported service: ec2". Flip the expectation to ServiceCompute so this test now exercises the correct contract.

…ping Two regression guards for issue #626: 1. Extend TestMapServiceType_AllBranches with the canonical-arm assertions that were missing: relational-db -> ServiceRelationalDB, cache -> ServiceCache, search -> ServiceSearch, data-warehouse -> ServiceDataWarehouse, plus the corrected compute -> ServiceCompute. These pin the canonical/legacy split so it cannot silently regress. 2. New TestExecutePurchase_AzureCanonicalServiceTypes drives executePurchase end-to-end with Azure recs carrying canonical Service slugs (compute, cache, relational-db) and asserts the mock GetServiceClient is called with the canonical ServiceType constants (ServiceCompute, ServiceCache, ServiceRelationalDB), not the AWS-legacy ones. This is the integration-style test that would have caught the production failure.

cristim · 2026-05-20T23:28:09Z

Rebased onto latest feat/multicloud-web-frontend.

Conflicts came from PR #629 (already merged on the base) which addressed the same Azure-purchase failure by collapsing both legacy and canonical service slugs onto the canonical ServiceType constants. This PR's canonical/legacy split is the strict superset — it fixes the same Azure regression while preserving "ec2" -> ServiceEC2 (and the other legacy slugs) so historical rec rows persisted before the canonical normalisation continue to round-trip to their AWS-legacy types.

Conflict resolution:

internal/purchase/execution.go: kept the canonical/legacy split via mapServiceSlug (this PR's approach) and dropped the switch block from PR fix(purchase): map service slugs to provider-neutral types so Azure purchases work (P0) #629. Updated the function-level doc comment to describe the resulting both-spellings behaviour.
internal/purchase/execution_test.go: kept the issue-fix(purchase): mapServiceType returns AWS-legacy ServiceEC2 for canonical "compute" — Azure approves fail with "unsupported service: ec2" #626 explanatory comment on the Azure mock, reverted PR fix(purchase): map service slugs to provider-neutral types so Azure purchases work (P0) #629's six AWS-rec mocks (Service: "ec2") from common.ServiceCompute back to common.ServiceEC2 so they match the split.
internal/purchase/coverage_extra_test.go: kept this PR's canonical-arm + legacy-arm table cases, reverted the two DetailsByService parameterised tables and three AWS-rec mocks (elasticache, opensearch, ec2) so legacy slugs assert the legacy ServiceType constants. Also corrected the test file's leading doc comment to describe the canonical/legacy split rather than the fix(purchase): map service slugs to provider-neutral types so Azure purchases work (P0) #629 collapse.
internal/purchase/manager_test.go: reverted the single AWS-rec GetServiceClient mock from common.ServiceCompute back to common.ServiceEC2.

Local verification on the rebased tip:

go build ./...: success
go vet ./...: clean
gofmt -l internal/purchase/: clean
go test ./internal/purchase/... -count=1: 141 passed
pre-commit run --files <changed files>: all hooks passed

cristim · 2026-05-20T23:28:14Z

@coderabbitai review

coderabbitai · 2026-05-20T23:28:20Z

✅ 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 · 2026-05-21T05:22:17Z

@coderabbitai full review

coderabbitai · 2026-05-21T05:22:22Z

✅ Actions performed

Full review triggered.

cristim added triaged Item has been triaged priority/p1 Next up; this sprint severity/high Significant harm urgency/now Drop other things labels May 20, 2026

cristim added impact/many Affects most users effort/s Hours type/bug Defect labels May 20, 2026

cristim added 3 commits May 21, 2026 01:20

cristim force-pushed the fix/issue-626-mapservicetype-canonical branch from c4f1abd to a87c0c4 Compare May 20, 2026 23:27

cristim merged commit 5baf1f6 into feat/multicloud-web-frontend May 21, 2026
4 checks passed

cristim mentioned this pull request May 21, 2026

bug(test): TestManager_ExecuteAndFinalize_HistorySaveFailure_StaysVisible fails on base (mapServiceType('ec2') now returns ServiceCompute, test mocks ServiceEC2) #637

Closed

coderabbitai Bot mentioned this pull request May 21, 2026

feat(providers/azure/synapse): add data-warehouse service client (closes #555) #622

Merged

6 tasks

cristim mentioned this pull request May 28, 2026

fix(purchase): mapServiceType returns AWS-legacy ServiceEC2 for canonical "compute" — Azure approves fail with "unsupported service: ec2" #626

Closed

cristim deleted the fix/issue-626-mapservicetype-canonical branch June 3, 2026 21:54

coderabbitai Bot mentioned this pull request Jun 8, 2026

docs(purchase): defer savings-plans alias drop to 2026-10-30 (closes #95) #827

Merged

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(purchase): map canonical slugs to canonical ServiceType (closes #626) - #630

fix(purchase): map canonical slugs to canonical ServiceType (closes #626)#630
cristim merged 3 commits into
feat/multicloud-web-frontendfrom
fix/issue-626-mapservicetype-canonical

cristim commented May 20, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented May 20, 2026 •

edited

Loading

Review failed

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Poem

❌ Failed checks (1 warning)

Uh oh!

cristim commented May 20, 2026

Uh oh!

coderabbitai Bot commented May 20, 2026

Uh oh!

cristim commented May 20, 2026

Uh oh!

cristim commented May 20, 2026

Uh oh!

coderabbitai Bot commented May 20, 2026

Uh oh!

Uh oh!

cristim commented May 21, 2026

Uh oh!

coderabbitai Bot commented May 21, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

cristim commented May 20, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Root cause

Changes

Test plan

Deploy note

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented May 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review failed

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Poem

❌ Failed checks (1 warning)

Uh oh!

cristim commented May 20, 2026

Uh oh!

coderabbitai Bot commented May 20, 2026

Uh oh!

cristim commented May 20, 2026

Uh oh!

cristim commented May 20, 2026

Uh oh!

coderabbitai Bot commented May 20, 2026

Uh oh!

Uh oh!

cristim commented May 21, 2026

Uh oh!

coderabbitai Bot commented May 21, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

cristim commented May 20, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented May 20, 2026 •

edited

Loading