diff --git a/internal/api/handler_marketplace.go b/internal/api/handler_marketplace.go index 50f2f91a..bc78152e 100644 --- a/internal/api/handler_marketplace.go +++ b/internal/api/handler_marketplace.go @@ -23,8 +23,6 @@ import ( "time" "github.com/LeanerCloud/cloud-commitments-go/pkg/logging" - awsprovider "github.com/LeanerCloud/cloud-commitments-go/providers/aws" - ec2svc "github.com/LeanerCloud/cloud-commitments-go/providers/aws/services/ec2" "github.com/LeanerCloud/cloud-commitments-platform/internal/auth" "github.com/LeanerCloud/cloud-commitments-platform/internal/config" "github.com/LeanerCloud/cloud-commitments-platform/internal/credentials" @@ -38,9 +36,9 @@ import ( // marketplaceEC2Client is the narrow EC2 interface the marketplace handlers // need. Using a minimal interface keeps test stubs small. type marketplaceEC2Client interface { - CreateMarketplaceListing(ctx context.Context, req ec2svc.MarketplaceListingRequest) (ec2svc.MarketplaceListingResult, error) - DescribeMarketplaceListing(ctx context.Context, listingID string) (ec2svc.MarketplaceListingResult, error) - CancelMarketplaceListing(ctx context.Context, listingID string) (ec2svc.MarketplaceListingResult, error) + CreateMarketplaceListing(ctx context.Context, req ec2MarketplaceListingRequest) (ec2MarketplaceListingResult, error) + DescribeMarketplaceListing(ctx context.Context, listingID string) (ec2MarketplaceListingResult, error) + CancelMarketplaceListing(ctx context.Context, listingID string) (ec2MarketplaceListingResult, error) // FetchOfferingClass calls AWS DescribeReservedInstances to determine // whether a given Reserved Instance is 'standard' or 'convertible'. Used // when the purchase_history row has no offering_class stored (pre-migration @@ -54,7 +52,7 @@ func (h *Handler) buildMarketplaceEC2Client(cfg aws.Config) marketplaceEC2Client if h.marketplaceEC2Factory != nil { return h.marketplaceEC2Factory(cfg) } - return awsprovider.NewEC2ClientDirect(cfg) + return newMarketplaceEC2Client(cfg) } // loadMarketplaceAWSConfig returns an AWS config for the purchase's region that @@ -304,7 +302,7 @@ const marketplaceCompensationTimeout = 10 * time.Second // keepUncanceledListing records a listing that could be neither persisted nor // canceled; releasing the claim instead would hide it from the cancel endpoint. -func (h *Handler) keepUncanceledListing(ctx context.Context, purchaseID string, listing ec2svc.MarketplaceListingResult, cancelErr error) error { +func (h *Handler) keepUncanceledListing(ctx context.Context, purchaseID string, listing ec2MarketplaceListingResult, cancelErr error) error { logging.Errorf("marketplace: rollback cancel for listing %s failed: %v", listing.ListingID, cancelErr) state := listing.State if state == "" { diff --git a/internal/api/handler_marketplace_attempt.go b/internal/api/handler_marketplace_attempt.go index b978aabf..eaeba0de 100644 --- a/internal/api/handler_marketplace_attempt.go +++ b/internal/api/handler_marketplace_attempt.go @@ -16,7 +16,6 @@ import ( "strings" "github.com/LeanerCloud/cloud-commitments-go/pkg/logging" - ec2svc "github.com/LeanerCloud/cloud-commitments-go/providers/aws/services/ec2" "github.com/LeanerCloud/cloud-commitments-platform/internal/config" "github.com/aws/aws-sdk-go-v2/aws" sdkec2 "github.com/aws/aws-sdk-go-v2/service/ec2" @@ -29,7 +28,7 @@ import ( // poller reads a listing's sale details through it too (issue #27), because // the wrapper's DescribeMarketplaceListing returns only the state. type marketplaceListingFinder interface { - FindMarketplaceListingByToken(ctx context.Context, reservedInstancesID, clientToken string) (ec2svc.MarketplaceListingResult, bool, error) + FindMarketplaceListingByToken(ctx context.Context, reservedInstancesID, clientToken string) (ec2MarketplaceListingResult, bool, error) DescribeMarketplaceListingStatus(ctx context.Context, listingID string) (marketplaceListingStatus, error) } @@ -39,20 +38,20 @@ type describeListingsAPI interface { type awsMarketplaceListingFinder struct{ api describeListingsAPI } -func (f awsMarketplaceListingFinder) FindMarketplaceListingByToken(ctx context.Context, reservedInstancesID, clientToken string) (ec2svc.MarketplaceListingResult, bool, error) { +func (f awsMarketplaceListingFinder) FindMarketplaceListingByToken(ctx context.Context, reservedInstancesID, clientToken string) (listing ec2MarketplaceListingResult, found bool, err error) { out, err := f.api.DescribeReservedInstancesListings(ctx, &sdkec2.DescribeReservedInstancesListingsInput{ ReservedInstancesId: aws.String(reservedInstancesID), }) if err != nil { - return ec2svc.MarketplaceListingResult{}, false, fmt.Errorf("DescribeReservedInstancesListings failed: %w", err) + return ec2MarketplaceListingResult{}, false, fmt.Errorf("DescribeReservedInstancesListings failed: %w", err) } for i := range out.ReservedInstancesListings { l := &out.ReservedInstancesListings[i] if aws.ToString(l.ClientToken) == clientToken { - return ec2svc.MarketplaceListingResult{ListingID: aws.ToString(l.ReservedInstancesListingId), State: string(l.Status)}, true, nil + return ec2MarketplaceListingResult{ListingID: aws.ToString(l.ReservedInstancesListingId), State: string(l.Status)}, true, nil } } - return ec2svc.MarketplaceListingResult{}, false, nil + return ec2MarketplaceListingResult{}, false, nil } func (h *Handler) buildMarketplaceFinder(cfg aws.Config) marketplaceListingFinder { @@ -78,10 +77,10 @@ func marketplaceInstanceCount(row *config.PurchaseHistoryRecord) (int32, error) return 1, nil } -func toAWSPriceSchedule(schedule []MarketplacePriceTier) []ec2svc.MarketplacePriceTier { - out := make([]ec2svc.MarketplacePriceTier, 0, len(schedule)) +func toAWSPriceSchedule(schedule []MarketplacePriceTier) []ec2MarketplacePriceTier { + out := make([]ec2MarketplacePriceTier, 0, len(schedule)) for _, t := range schedule { - out = append(out, ec2svc.MarketplacePriceTier{Term: t.TermMonths, Price: t.Price}) + out = append(out, ec2MarketplacePriceTier{Term: t.TermMonths, Price: t.Price}) } return out } @@ -93,14 +92,14 @@ func toAWSPriceSchedule(schedule []MarketplacePriceTier) []ec2svc.MarketplacePri // does not leave the row stuck in the transient pending state. Returns the // persisted listing and the schedule it was created with, which on a retry is // the stored one, not the one passed in. -func (h *Handler) reserveAndCreateListing(ctx context.Context, purchaseID string, row *config.PurchaseHistoryRecord, cfg aws.Config, ec2Client marketplaceEC2Client, schedule []MarketplacePriceTier) (ec2svc.MarketplaceListingResult, []MarketplacePriceTier, error) { +func (h *Handler) reserveAndCreateListing(ctx context.Context, purchaseID string, row *config.PurchaseHistoryRecord, cfg aws.Config, ec2Client marketplaceEC2Client, schedule []MarketplacePriceTier) (ec2MarketplaceListingResult, []MarketplacePriceTier, error) { instanceCount, err := marketplaceInstanceCount(row) if err != nil { - return ec2svc.MarketplaceListingResult{}, nil, err + return ec2MarketplaceListingResult{}, nil, err } claim, used, err := h.claimListingSlot(ctx, purchaseID, schedule) if err != nil { - return ec2svc.MarketplaceListingResult{}, nil, err + return ec2MarketplaceListingResult{}, nil, err } if claim.Resumed { result, found, recErr := h.reconcileAttempt(ctx, purchaseID, claim, h.buildMarketplaceFinder(cfg)) @@ -147,28 +146,28 @@ func (h *Handler) claimListingSlot(ctx context.Context, purchaseID string, sched // resolved: result holds the recorded listing, or err why it ended in a dead // one. Not found means AWS has nothing for the token, so the caller creates // the listing with that same token. -func (h *Handler) reconcileAttempt(ctx context.Context, purchaseID string, claim *config.MarketplaceListingClaim, finder marketplaceListingFinder) (ec2svc.MarketplaceListingResult, bool, error) { - listing, found, err := finder.FindMarketplaceListingByToken(ctx, purchaseID, claim.ClientToken) +func (h *Handler) reconcileAttempt(ctx context.Context, purchaseID string, claim *config.MarketplaceListingClaim, finder marketplaceListingFinder) (listing ec2MarketplaceListingResult, found bool, err error) { + listing, found, err = finder.FindMarketplaceListingByToken(ctx, purchaseID, claim.ClientToken) if err != nil { h.releaseMarketplaceClaim(ctx, purchaseID, claim, true) - return ec2svc.MarketplaceListingResult{}, false, mapAWSMarketplaceError("could not reconcile the earlier listing attempt on AWS", err) + return ec2MarketplaceListingResult{}, false, mapAWSMarketplaceError("could not reconcile the earlier listing attempt on AWS", err) } if !found { - return ec2svc.MarketplaceListingResult{}, false, nil + return ec2MarketplaceListingResult{}, false, nil } if listing.State == "" { listing.State = config.ListingStatePending } logging.Warnf("marketplace: purchase %s: earlier attempt already created listing %s (%s); recording it", purchaseID, listing.ListingID, listing.State) if isDeadListingState(listing.State) { - return ec2svc.MarketplaceListingResult{}, true, h.recordDeadListing(ctx, purchaseID, claim, listing) + return ec2MarketplaceListingResult{}, true, h.recordDeadListing(ctx, purchaseID, claim, listing) } if err = h.config.UpdatePurchaseHistoryListing(ctx, purchaseID, listing.ListingID, listing.State); err != nil { if isListingEndedConflict(err) { - return ec2svc.MarketplaceListingResult{}, true, listingCanceledConcurrently(listing.ListingID) + return ec2MarketplaceListingResult{}, true, listingCanceledConcurrently(listing.ListingID) } h.releaseMarketplaceClaim(ctx, purchaseID, claim, true) - return ec2svc.MarketplaceListingResult{}, true, fmt.Errorf("listing %s exists on AWS but could not be recorded: %w", listing.ListingID, err) + return ec2MarketplaceListingResult{}, true, fmt.Errorf("listing %s exists on AWS but could not be recorded: %w", listing.ListingID, err) } return listing, true, nil } @@ -184,7 +183,7 @@ func listingCanceledConcurrently(listingID string) error { // "not listed" and let the next request create a second listing, so the // listing is read back, and recorded as pending (live, cancelable) if its // state is still unknown. -func createdListingState(ctx context.Context, ec2Client marketplaceEC2Client, created ec2svc.MarketplaceListingResult) string { +func createdListingState(ctx context.Context, ec2Client marketplaceEC2Client, created ec2MarketplaceListingResult) string { if created.State != "" { return created.State } @@ -196,8 +195,8 @@ func createdListingState(ctx context.Context, ec2Client marketplaceEC2Client, cr return config.ListingStatePending } -func (h *Handler) createAndPersistListing(ctx context.Context, purchaseID string, claim *config.MarketplaceListingClaim, ec2Client marketplaceEC2Client, instanceCount int32, awsSchedule []ec2svc.MarketplacePriceTier) (ec2svc.MarketplaceListingResult, error) { - result, err := ec2Client.CreateMarketplaceListing(ctx, ec2svc.MarketplaceListingRequest{ +func (h *Handler) createAndPersistListing(ctx context.Context, purchaseID string, claim *config.MarketplaceListingClaim, ec2Client marketplaceEC2Client, instanceCount int32, awsSchedule []ec2MarketplacePriceTier) (ec2MarketplaceListingResult, error) { + result, err := ec2Client.CreateMarketplaceListing(ctx, ec2MarketplaceListingRequest{ ReservedInstancesID: purchaseID, ClientToken: claim.ClientToken, PriceSchedule: awsSchedule, @@ -210,12 +209,12 @@ func (h *Handler) createAndPersistListing(ctx context.Context, purchaseID string // then is the token dropped; otherwise the retry resends it. h.releaseMarketplaceClaim(ctx, purchaseID, claim, !isClientFaultError(mapped)) logging.Warnf("marketplace: CreateReservedInstancesListing for purchase %s failed: %v", purchaseID, err) - return ec2svc.MarketplaceListingResult{}, mapped + return ec2MarketplaceListingResult{}, mapped } result.State = createdListingState(ctx, ec2Client, result) if isDeadListingState(result.State) { - return ec2svc.MarketplaceListingResult{}, h.recordDeadListing(ctx, purchaseID, claim, result) + return ec2MarketplaceListingResult{}, h.recordDeadListing(ctx, purchaseID, claim, result) } // Persist the listing ID and state. On DB failure, attempt a compensating @@ -226,17 +225,17 @@ func (h *Handler) createAndPersistListing(ctx context.Context, purchaseID string if isListingEndedConflict(dbErr) { // The cancel endpoint found this attempt's listing through its // token, canceled it on AWS and recorded that first. - return ec2svc.MarketplaceListingResult{}, listingCanceledConcurrently(result.ListingID) + return ec2MarketplaceListingResult{}, listingCanceledConcurrently(result.ListingID) } logging.Errorf("marketplace: listing created (%s / %s) but DB update failed: %v; attempting rollback", result.ListingID, result.State, dbErr) compCtx, cancelComp := context.WithTimeout(context.WithoutCancel(ctx), marketplaceCompensationTimeout) defer cancelComp() if _, rollbackErr := ec2Client.CancelMarketplaceListing(compCtx, result.ListingID); rollbackErr != nil { - return ec2svc.MarketplaceListingResult{}, h.keepUncanceledListing(compCtx, purchaseID, result, rollbackErr) + return ec2MarketplaceListingResult{}, h.keepUncanceledListing(compCtx, purchaseID, result, rollbackErr) } logging.Warnf("marketplace: listing %s rolled back (canceled) after DB failure", result.ListingID) h.recordRolledBackListing(compCtx, purchaseID, claim, result.ListingID) - return ec2svc.MarketplaceListingResult{}, fmt.Errorf("listing created but could not be persisted; listing has been rolled back: %w", dbErr) + return ec2MarketplaceListingResult{}, fmt.Errorf("listing created but could not be persisted; listing has been rolled back: %w", dbErr) } return result, nil @@ -254,7 +253,7 @@ func isDeadListingState(state string) bool { // recordDeadListing handles a listing of this attempt that is canceled or // closed on AWS: it records the listing, which also drops the attempt's token, // so the next attempt starts fresh instead of AWS replaying the dead listing. -func (h *Handler) recordDeadListing(ctx context.Context, purchaseID string, claim *config.MarketplaceListingClaim, listing ec2svc.MarketplaceListingResult) error { +func (h *Handler) recordDeadListing(ctx context.Context, purchaseID string, claim *config.MarketplaceListingClaim, listing ec2MarketplaceListingResult) error { if err := h.config.UpdatePurchaseHistoryListing(ctx, purchaseID, listing.ListingID, listing.State); err != nil { logging.Errorf("marketplace: failed to record dead listing %s for purchase %s: %v", listing.ListingID, purchaseID, err) h.releaseMarketplaceClaim(ctx, purchaseID, claim, false) diff --git a/internal/api/handler_marketplace_cancel.go b/internal/api/handler_marketplace_cancel.go index c000a319..9b24fda3 100644 --- a/internal/api/handler_marketplace_cancel.go +++ b/internal/api/handler_marketplace_cancel.go @@ -16,7 +16,6 @@ import ( "strings" "github.com/LeanerCloud/cloud-commitments-go/pkg/logging" - ec2svc "github.com/LeanerCloud/cloud-commitments-go/providers/aws/services/ec2" "github.com/LeanerCloud/cloud-commitments-platform/internal/auth" "github.com/LeanerCloud/cloud-commitments-platform/internal/config" "github.com/aws/aws-lambda-go/events" @@ -178,16 +177,16 @@ func (h *Handler) cancelListing(ctx context.Context, purchaseID string, seen *co // endedMarketplaceListing reports whether listingID has already ended on AWS, // read after its cancel failed. A role-assumption failure would fail the read // the same way, so it is not retried. -func endedMarketplaceListing(ctx context.Context, ec2Client marketplaceEC2Client, listingID string, cancelErr error) (ec2svc.MarketplaceListingResult, bool) { +func endedMarketplaceListing(ctx context.Context, ec2Client marketplaceEC2Client, listingID string, cancelErr error) (ec2MarketplaceListingResult, bool) { if isRoleAssumptionFailure(cancelErr) { - return ec2svc.MarketplaceListingResult{}, false + return ec2MarketplaceListingResult{}, false } listing, err := ec2Client.DescribeMarketplaceListing(ctx, listingID) if err != nil || !isDeadListingState(listing.State) { - return ec2svc.MarketplaceListingResult{}, false + return ec2MarketplaceListingResult{}, false } logging.Warnf("marketplace: listing %s is already %s on AWS (cancel said: %v); recording it", listingID, listing.State, cancelErr) - return ec2svc.MarketplaceListingResult{ListingID: listingID, State: listing.State}, true + return ec2MarketplaceListingResult{ListingID: listingID, State: listing.State}, true } // resolveListingAttempt records listingID in state on a pending row, only diff --git a/internal/api/handler_ri_exchange.go b/internal/api/handler_ri_exchange.go index edd52979..f4e76adc 100644 --- a/internal/api/handler_ri_exchange.go +++ b/internal/api/handler_ri_exchange.go @@ -13,19 +13,13 @@ import ( "strings" "time" - "github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/reservations/armreservations" "github.com/aws/aws-lambda-go/events" "github.com/aws/aws-sdk-go-v2/aws" - awsconfig "github.com/aws/aws-sdk-go-v2/config" "github.com/aws/smithy-go" "github.com/LeanerCloud/cloud-commitments-go/pkg/common" "github.com/LeanerCloud/cloud-commitments-go/pkg/exchange" "github.com/LeanerCloud/cloud-commitments-go/pkg/logging" - awsprovider "github.com/LeanerCloud/cloud-commitments-go/providers/aws" - "github.com/LeanerCloud/cloud-commitments-go/providers/aws/recommendations" - ec2svc "github.com/LeanerCloud/cloud-commitments-go/providers/aws/services/ec2" - azurecompute "github.com/LeanerCloud/cloud-commitments-go/providers/azure/services/compute" "github.com/LeanerCloud/cloud-commitments-platform/internal/auth" "github.com/LeanerCloud/cloud-commitments-platform/internal/config" "github.com/LeanerCloud/cloud-commitments-platform/internal/credentials" @@ -35,10 +29,9 @@ import ( // reshapeEC2Client is the narrow slice of the EC2 client that // getReshapeRecommendations needs. Scoped to this handler so mocks -// stay small. The concrete *ec2svc.Client returned by -// awsprovider.NewEC2ClientDirect already implements these methods -// (Go structural typing), so the nil-factory fallback path casts it -// directly. +// stay small. The concrete EC2 client that newReshapeEC2Client +// (provider_wiring.go) builds for the nil-factory fallback path +// implements it. // // Cross-family alternatives no longer flow through here — they're // sourced from the cached AWS Cost Explorer purchase recommendations @@ -47,7 +40,7 @@ import ( // region's offered instance types (DescribeInstanceTypeOfferings), which // limit reshape targets to sizes the region actually sells. type reshapeEC2Client interface { - ListConvertibleReservedInstances(ctx context.Context) ([]ec2svc.ConvertibleRI, error) + ListConvertibleReservedInstances(ctx context.Context) ([]ec2ConvertibleRI, error) GetValidResourceTypes(ctx context.Context) ([]string, error) } @@ -55,7 +48,7 @@ type reshapeEC2Client interface { // adapter that getReshapeRecommendations needs (the utilization // fetcher injected into the cache wrapper). Scoped identically. type reshapeRecsClient interface { - GetRIUtilization(ctx context.Context, lookbackDays int, region string) ([]recommendations.RIUtilization, error) + GetRIUtilization(ctx context.Context, lookbackDays int, region string) ([]riUtilization, error) } // buildReshapeEC2Client honors the injected factory when set, falling @@ -65,7 +58,7 @@ func (h *Handler) buildReshapeEC2Client(cfg aws.Config) reshapeEC2Client { if h.reshapeEC2Factory != nil { return h.reshapeEC2Factory(cfg) } - return awsprovider.NewEC2ClientDirect(cfg) + return newReshapeEC2Client(cfg) } // buildReshapeRecsClient mirrors buildReshapeEC2Client for the @@ -74,15 +67,15 @@ func (h *Handler) buildReshapeRecsClient(cfg aws.Config) reshapeRecsClient { if h.reshapeRecsFactory != nil { return h.reshapeRecsFactory(cfg) } - return awsprovider.NewRecommendationsClientDirect(cfg) + return newReshapeRecsClient(cfg) } // targetOfferingsEC2Client is the narrow EC2 interface that // listTargetOfferings needs. Scoped here so tests can inject a tiny -// stub without implementing the full ec2svc.Client surface. +// stub without implementing the full concrete EC2 client surface. type targetOfferingsEC2Client interface { - ListConvertibleReservedInstances(ctx context.Context) ([]ec2svc.ConvertibleRI, error) - ListTargetOfferings(ctx context.Context, params ec2svc.ListTargetOfferingsParams) ([]ec2svc.TargetOffering, error) + ListConvertibleReservedInstances(ctx context.Context) ([]ec2ConvertibleRI, error) + ListTargetOfferings(ctx context.Context, params ec2ListTargetOfferingsParams) ([]ec2TargetOffering, error) } // buildTargetOfferingsEC2Client honors the injected factory when set, @@ -91,13 +84,13 @@ func (h *Handler) buildTargetOfferingsEC2Client(cfg aws.Config) targetOfferingsE if h.targetOfferingsEC2Factory != nil { return h.targetOfferingsEC2Factory(cfg) } - return awsprovider.NewEC2ClientDirect(cfg) + return newTargetOfferingsEC2Client(cfg) } // TargetOfferingsResponse is the response for // GET /api/ri-exchange/target-offerings. type TargetOfferingsResponse struct { - Offerings []ec2svc.TargetOffering `json:"offerings"` + Offerings []ec2TargetOffering `json:"offerings"` } // offeringIDPattern matches a standard AWS offering UUID used for @@ -112,7 +105,7 @@ var offeringIDPattern = regexp.MustCompile( // // The handler looks up the source RI from DescribeReservedInstances, // extracts its ProductDescription / Tenancy / Scope / Duration, -// and passes those to ec2svc.ListTargetOfferings which +// and passes those to the EC2 client's ListTargetOfferings, which // calls DescribeReservedInstancesOfferings with the same typed-field // shape used by PR #690. Instance type is intentionally omitted from // the query so AWS returns all valid target instance types -- the full @@ -150,7 +143,7 @@ func (h *Handler) listTargetOfferings(ctx context.Context, req *events.LambdaFun return nil, fmt.Errorf("failed to list convertible RIs: %w", err) } - var sourceRI *ec2svc.ConvertibleRI + var sourceRI *ec2ConvertibleRI for i := range ris { if ris[i].ReservedInstanceID == sourceRIID { sourceRI = &ris[i] @@ -172,8 +165,8 @@ func (h *Handler) listTargetOfferings(ctx context.Context, req *events.LambdaFun // targetOfferingsForSource lists the exchange targets for sourceRI, keeping a // zonal RI's targets in its own AZ. A zone refusal (the source RI's scope and // AZ disagree) is a 422. -func targetOfferingsForSource(ctx context.Context, ec2Client targetOfferingsEC2Client, sourceRI *ec2svc.ConvertibleRI) ([]ec2svc.TargetOffering, error) { - offerings, err := ec2Client.ListTargetOfferings(ctx, ec2svc.ListTargetOfferingsParams{ +func targetOfferingsForSource(ctx context.Context, ec2Client targetOfferingsEC2Client, sourceRI *ec2ConvertibleRI) ([]ec2TargetOffering, error) { + offerings, err := ec2Client.ListTargetOfferings(ctx, ec2ListTargetOfferingsParams{ ProductDescription: sourceRI.ProductDescription, Tenancy: sourceRI.InstanceTenancy, Scope: sourceRI.Scope, @@ -214,10 +207,11 @@ func isOfferingZoneRefusal(err error) bool { // azureExchangeClient is the narrow interface that listExchangeableAzureRIs, // getAzureCompatibleOfferings, and executeAzureExchange need from the Azure -// compute client. Satisfied by *azurecompute.ComputeClient; a stub can be -// injected via Handler.azureExchangeFactory for tests. +// compute client. Satisfied by the compute client newAzureExchangeClient +// (provider_wiring.go) builds; a stub can be injected via +// Handler.azureExchangeFactory for tests. type azureExchangeClient interface { - ListExchangeableReservations(ctx context.Context) ([]azurecompute.ExchangeableReservation, error) + ListExchangeableReservations(ctx context.Context) ([]azureExchangeableReservation, error) // CalculateExchange prices a proposed exchange without committing it. // Both getAzureCompatibleOfferings (read-only quote) and @@ -225,8 +219,8 @@ type azureExchangeClient interface { // only executeAzureExchange ever calls ExecuteExchange, and only with // the SessionID this same call just returned -- see executeAzureExchange's // doc comment for why a client-supplied session is never trusted. - CalculateExchange(ctx context.Context, sources []azurecompute.ExchangeableReservation, targets []azurecompute.ExchangeTarget) (*azurecompute.ExchangePreview, []azurecompute.CompatibleOffering, error) - ExecuteExchange(ctx context.Context, sessionID string) (*azurecompute.ExchangeResult, error) + CalculateExchange(ctx context.Context, sources []azureExchangeableReservation, targets []azureExchangeTarget) (*azureExchangePreview, []azureCompatibleOffering, error) + ExecuteExchange(ctx context.Context, sessionID string) (*azureExchangeResult, error) } // buildAzureExchangeClient returns the injected factory result when one has @@ -288,9 +282,7 @@ func (h *Handler) azureExchangeClientForAccount(ctx context.Context, subscriptio return nil, fmt.Errorf("azure: resolve credentials for subscription %q: %w", subscriptionID, err) } - // Region is left empty -- ListExchangeableReservations uses the tenant- - // wide armreservations API which is not scoped to a region. - return azurecompute.NewClient(cred, subscriptionID, ""), nil + return newAzureExchangeClient(cred, subscriptionID), nil } // listExchangeableAzureRIs returns active Azure VM reservations that are @@ -362,7 +354,7 @@ func (h *Handler) listExchangeableAzureRIs(ctx context.Context, req *events.Lamb // No Azure account configured for this subscription (or no Azure accounts // registered at all). Return an empty list rather than a 500 so the page // renders a "no reservations" state instead of an opaque error banner. - return &ExchangeableAzureRIsResponse{Reservations: []azurecompute.ExchangeableReservation{}}, nil + return &ExchangeableAzureRIsResponse{Reservations: []azureExchangeableReservation{}}, nil } reservations, err := client.ListExchangeableReservations(ctx) @@ -394,9 +386,9 @@ func (h *Handler) listExchangeableAzureRIs(ctx context.Context, req *events.Lamb // (requireAzureSourceOwnership refuses cross-subscription sources), so // returning the wider tenant listing here would just be the enumeration leak // issue #1656 describes. -func filterAzureReservationsBySubscription(reservations []azurecompute.ExchangeableReservation, subscriptionID string) []azurecompute.ExchangeableReservation { +func filterAzureReservationsBySubscription(reservations []azureExchangeableReservation, subscriptionID string) []azureExchangeableReservation { scope := azureBillingScopeID(subscriptionID) - filtered := make([]azurecompute.ExchangeableReservation, 0, len(reservations)) + filtered := make([]azureExchangeableReservation, 0, len(reservations)) for i := range reservations { if strings.EqualFold(reservations[i].BillingScopeID, scope) { filtered = append(filtered, reservations[i]) @@ -422,7 +414,7 @@ func filterAzureReservationsBySubscription(reservations []azurecompute.Exchangea // the dropped reservation/billing-scope identifiers, are safe to log -- // logging the identifiers would just relocate the disclosure this filter // exists to prevent. -func (h *Handler) filterAzureReservationsByScope(ctx context.Context, session *Session, reservations []azurecompute.ExchangeableReservation) ([]azurecompute.ExchangeableReservation, error) { +func (h *Handler) filterAzureReservationsByScope(ctx context.Context, session *Session, reservations []azureExchangeableReservation) ([]azureExchangeableReservation, error) { allowed, err := h.getAccountScope(ctx, session) if err != nil { return nil, fmt.Errorf("failed to get allowed accounts: %w", err) @@ -433,7 +425,7 @@ func (h *Handler) filterAzureReservationsByScope(ctx context.Context, session *S // normalize here too rather than letting an admin session alone see // {"reservations": null}. if reservations == nil { - reservations = []azurecompute.ExchangeableReservation{} + reservations = []azureExchangeableReservation{} } return reservations, nil } @@ -476,8 +468,8 @@ func azureScopeIndex(accounts []config.CloudAccount) map[string]config.CloudAcco // scope allows (AccountScope.Allows). Fails closed: a reservation with no // BillingScopeID, or one that resolves to no registered CloudAccount, is // dropped rather than kept. -func filterReservationsByScopeIndex(reservations []azurecompute.ExchangeableReservation, scopeToAccount map[string]config.CloudAccount, allowed auth.AccountScope) []azurecompute.ExchangeableReservation { - filtered := make([]azurecompute.ExchangeableReservation, 0, len(reservations)) +func filterReservationsByScopeIndex(reservations []azureExchangeableReservation, scopeToAccount map[string]config.CloudAccount, allowed auth.AccountScope) []azureExchangeableReservation { + filtered := make([]azureExchangeableReservation, 0, len(reservations)) for i := range reservations { r := reservations[i] if r.BillingScopeID == "" { @@ -540,8 +532,8 @@ type AzureCompatibleOfferingsRequestBody struct { // though execute never trusts a client-supplied session -- see // executeAzureExchange). type AzureCompatibleOfferingsResponse struct { - Offerings []azurecompute.CompatibleOffering `json:"offerings"` - Preview *azurecompute.ExchangePreview `json:"preview"` + Offerings []azureCompatibleOffering `json:"offerings"` + Preview *azureExchangePreview `json:"preview"` } // AzureExecuteExchangeRequestBody is the request body for the execute @@ -567,20 +559,6 @@ type AzureExecuteExchangeResponse struct { PurchasesTotal *float64 `json:"purchases_total"` } -// azureReservationTermFromString converts the HTTP-layer term string to the -// typed SDK enum, rejecting anything outside armreservations' -// PossibleReservationTermValues(). No fallback: an unrecognized term is a -// 400, never silently coerced to a default term (feedback_sdk_enum_string_literals). -func azureReservationTermFromString(s string) (armreservations.ReservationTerm, error) { - term := armreservations.ReservationTerm(s) - for _, t := range armreservations.PossibleReservationTermValues() { - if t == term { - return term, nil - } - } - return "", fmt.Errorf("unsupported term %q", s) -} - // validateAzureExchangeSources checks the shared sources[] shape for both // the offerings and execute request bodies. func validateAzureExchangeSources(sources []AzureExchangeSourceBody) error { @@ -695,12 +673,12 @@ func validateAzureExecuteBody(body AzureExecuteExchangeRequestBody) error { // toAzureExchangeSources keeps the server-listed reservation metadata and // applies the requested quantities after requireAzureSourceOwnership succeeds. -func toAzureExchangeSources(sources []AzureExchangeSourceBody, owned []azurecompute.ExchangeableReservation) []azurecompute.ExchangeableReservation { - byID := make(map[string]azurecompute.ExchangeableReservation, len(owned)) +func toAzureExchangeSources(sources []AzureExchangeSourceBody, owned []azureExchangeableReservation) []azureExchangeableReservation { + byID := make(map[string]azureExchangeableReservation, len(owned)) for i := range owned { byID[strings.ToLower(owned[i].ReservationID)] = owned[i] } - out := make([]azurecompute.ExchangeableReservation, len(sources)) + out := make([]azureExchangeableReservation, len(sources)) for i, s := range sources { out[i] = byID[strings.ToLower(s.ReservationID)] out[i].Quantity = s.Quantity @@ -714,15 +692,15 @@ func toAzureExchangeSources(sources []AzureExchangeSourceBody, owned []azurecomp // body (see azureBillingScopeID). validateAzureExchangeTargets must be // called first; a term error here indicates an internal invariant break // rather than a fresh client mistake. -func toAzureExchangeTargets(targets []AzureExchangeTargetBody, subscriptionID string) ([]azurecompute.ExchangeTarget, error) { - out := make([]azurecompute.ExchangeTarget, len(targets)) +func toAzureExchangeTargets(targets []AzureExchangeTargetBody, subscriptionID string) ([]azureExchangeTarget, error) { + out := make([]azureExchangeTarget, len(targets)) scope := azureBillingScopeID(subscriptionID) for i, t := range targets { term, err := azureReservationTermFromString(t.Term) if err != nil { return nil, fmt.Errorf("targets[%d]: %w", i, err) } - out[i] = azurecompute.ExchangeTarget{ + out[i] = azureExchangeTarget{ SKU: t.SKU, Location: t.Location, Term: term, @@ -822,7 +800,7 @@ const unknownRegionConstraint = "unknown-region" // means specifically "owned, but Azure reported no region". The missing-source // branch is kept as a fail-closed default for any future caller that reaches // this function without that guarantee. -func exchangeRegions(targets []AzureExchangeTargetBody, sources []AzureExchangeSourceBody, owned []azurecompute.ExchangeableReservation) []string { +func exchangeRegions(targets []AzureExchangeTargetBody, sources []AzureExchangeSourceBody, owned []azureExchangeableReservation) []string { regionByID := make(map[string]string, len(owned)) for i := range owned { regionByID[strings.ToLower(owned[i].ReservationID)] = owned[i].Region @@ -934,7 +912,7 @@ func (h *Handler) lookupAzureAccount(ctx context.Context, subscriptionID string) // "belongs to someone else", so this cannot be used to enumerate another // subscription's reservation IDs (same posture as // requireAzureSubscriptionScope). -func requireAzureSourceOwnership(owned []azurecompute.ExchangeableReservation, sources []AzureExchangeSourceBody, subscriptionID string) error { +func requireAzureSourceOwnership(owned []azureExchangeableReservation, sources []AzureExchangeSourceBody, subscriptionID string) error { scope := azureBillingScopeID(subscriptionID) byID := make(map[string]string, len(owned)) for i := range owned { @@ -955,7 +933,7 @@ func requireAzureSourceOwnership(owned []azurecompute.ExchangeableReservation, s // applies requireAzureSourceOwnership. Split from the pure check so the // authorization rule itself is testable without a client, and so both the // pricing and execute endpoints share one code path. -func checkAzureSourceOwnership(ctx context.Context, client azureExchangeClient, sources []AzureExchangeSourceBody, subscriptionID string) ([]azurecompute.ExchangeableReservation, error) { +func checkAzureSourceOwnership(ctx context.Context, client azureExchangeClient, sources []AzureExchangeSourceBody, subscriptionID string) ([]azureExchangeableReservation, error) { owned, err := listOwnedAzureReservations(ctx, client) if err != nil { return nil, err @@ -976,7 +954,7 @@ func checkAzureSourceOwnership(ctx context.Context, client azureExchangeClient, // region, and permitting the exchange would restore the very gaps those // checks exist to close. 502 rather than 500 because the upstream dependency, // not this service, is what failed. -func listOwnedAzureReservations(ctx context.Context, client azureExchangeClient) ([]azurecompute.ExchangeableReservation, error) { +func listOwnedAzureReservations(ctx context.Context, client azureExchangeClient) ([]azureExchangeableReservation, error) { owned, err := client.ListExchangeableReservations(ctx) if err != nil { logging.Errorf("azure exchange source ownership lookup failed: %v", err) @@ -1076,7 +1054,7 @@ const azureMaxPurchaseAmountCurrency = "USD" // It also sharpens exchangeRegions: every source reaching it is // known-owned, so unknownRegionConstraint means only "owned, but Azure // reported no region" rather than doubling as "not yours" or "not real". -func (h *Handler) authorizeAzureExchangeExecution(ctx context.Context, session *Session, body AzureExecuteExchangeRequestBody, maxRat *big.Rat) (azureExchangeClient, []azurecompute.ExchangeableReservation, string, error) { +func (h *Handler) authorizeAzureExchangeExecution(ctx context.Context, session *Session, body AzureExecuteExchangeRequestBody, maxRat *big.Rat) (client azureExchangeClient, sources []azureExchangeableReservation, accountID string, err error) { // The subscription's CloudAccount is resolved once and that one row // feeds the scope check, the client's credentials and the constraint // AccountIDs (issue #311), so the three cannot see different rows. @@ -1096,7 +1074,7 @@ func (h *Handler) authorizeAzureExchangeExecution(ctx context.Context, session * return nil, nil, "", scopeErr } - client, err := h.azureExchangeClientForAccount(ctx, body.SubscriptionID, account) + client, err = h.azureExchangeClientForAccount(ctx, body.SubscriptionID, account) if err != nil { return nil, nil, "", fmt.Errorf("failed to build Azure exchange client: %w", err) } @@ -1113,7 +1091,7 @@ func (h *Handler) authorizeAzureExchangeExecution(ctx context.Context, session * return nil, nil, "", ownErr } - accountID := azureExchangeAccountID(account) + accountID = azureExchangeAccountID(account) if err := h.checkAzureExecuteConstraints(ctx, session, body, accountID, maxRat, exchangeRegions(body.Targets, body.Sources, owned)); err != nil { return nil, nil, "", err } @@ -1204,7 +1182,7 @@ func (h *Handler) checkAzureExecuteConstraints(ctx context.Context, session *Ses // azureExchangeClient.CalculateExchange contract never returns (nil, nil, // nil), but a future implementation of the interface making that mistake // must not panic this handler. -func checkAzureExchangeMoneyGuardrails(preview *azurecompute.ExchangePreview, maxRat *big.Rat, currency string) error { +func checkAzureExchangeMoneyGuardrails(preview *azureExchangePreview, maxRat *big.Rat, currency string) error { if preview == nil { return fmt.Errorf("internal error: CalculateExchange returned a nil preview") } @@ -1375,7 +1353,7 @@ func mapAzureExchangeError(opMsg string, err error) error { // getBaseAWSConfig returns the cached base AWS config, loading it once via sync.Once. func (h *Handler) getBaseAWSConfig(ctx context.Context) (aws.Config, error) { h.awsCfgOnce.Do(func() { - h.awsCfg, h.awsCfgErr = awsconfig.LoadDefaultConfig(ctx) + h.awsCfg, h.awsCfgErr = loadDefaultAWSConfig(ctx) }) return h.awsCfg, h.awsCfgErr } @@ -1531,13 +1509,13 @@ func (h *Handler) listConvertibleRIs(ctx context.Context, req *events.LambdaFunc if inScope, sErr := h.reshapeCloudAccountInScope(ctx, session); sErr != nil { return nil, sErr } else if !inScope { - return &ConvertibleRIsResponse{Instances: []ec2svc.ConvertibleRI{}}, nil + return &ConvertibleRIsResponse{Instances: []ec2ConvertibleRI{}}, nil } if inScope, sErr := h.checkListRIsAccountIDParam(ctx, req.QueryStringParameters); sErr != nil { return nil, sErr } else if !inScope { - return &ConvertibleRIsResponse{Instances: []ec2svc.ConvertibleRI{}}, nil + return &ConvertibleRIsResponse{Instances: []ec2ConvertibleRI{}}, nil } region := req.QueryStringParameters["region"] @@ -1546,7 +1524,7 @@ func (h *Handler) listConvertibleRIs(ctx context.Context, req *events.LambdaFunc return nil, fmt.Errorf("failed to load AWS config: %w", err) } - ec2Client := awsprovider.NewEC2ClientDirect(cfg) + ec2Client := newReshapeEC2Client(cfg) instances, err := ec2Client.ListConvertibleReservedInstances(ctx) if err != nil { return nil, fmt.Errorf("failed to list convertible RIs: %w", err) @@ -1575,7 +1553,7 @@ func (h *Handler) getRIUtilization(ctx context.Context, req *events.LambdaFuncti if sErr != nil { return nil, sErr } else if !inScope { - return &RIUtilizationResponse{Utilization: []recommendations.RIUtilization{}}, nil + return &RIUtilizationResponse{Utilization: []riUtilization{}}, nil } region := req.QueryStringParameters["region"] @@ -1591,7 +1569,7 @@ func (h *Handler) getRIUtilization(ctx context.Context, req *events.LambdaFuncti } recsAdapter := h.buildReshapeRecsClient(cfg) - fetch := func(fetchCtx context.Context, days int) ([]recommendations.RIUtilization, error) { + fetch := func(fetchCtx context.Context, days int) ([]riUtilization, error) { return recsAdapter.GetRIUtilization(fetchCtx, days, region) } cacheKey := config.RIUtilizationCacheKey{CloudAccountID: cloudAccountID, Region: region, LookbackDays: lookbackDays} @@ -1617,7 +1595,7 @@ func (h *Handler) getRIUtilization(ctx context.Context, req *events.LambdaFuncti // only rows the scope gate in the caller has authorized; unknown ids are // dropped, so an unregistered or out-of-scope member's RIs stay hidden. // Unrestricted sessions keep every row and make no extra AWS call. -func (h *Handler) restrictUtilizationToDeploymentRIs(ctx context.Context, session *Session, cfg aws.Config, utilization []recommendations.RIUtilization) ([]recommendations.RIUtilization, error) { +func (h *Handler) restrictUtilizationToDeploymentRIs(ctx context.Context, session *Session, cfg aws.Config, utilization []riUtilization) ([]riUtilization, error) { allowed, err := h.getAccountScope(ctx, session) if err != nil { return nil, fmt.Errorf("failed to get allowed accounts: %w", err) @@ -1633,7 +1611,7 @@ func (h *Handler) restrictUtilizationToDeploymentRIs(ctx context.Context, sessio for i := range own { ownIDs[own[i].ReservedInstanceID] = struct{}{} } - visible := make([]recommendations.RIUtilization, 0, len(utilization)) + visible := make([]riUtilization, 0, len(utilization)) for i := range utilization { if _, ok := ownIDs[utilization[i].ReservedInstanceID]; ok { visible = append(visible, utilization[i]) @@ -1673,7 +1651,7 @@ func parseThresholdParam(params map[string]string) (float64, error) { // the single implementation of the convertible RI monthly cost formula shared // with internal/server. Its error wraps ricost.ErrNonPositiveDuration or // ricost.ErrNoPricing, and callers must exclude the RI. -func monthlyCostFromConvertibleRI(ri ec2svc.ConvertibleRI) (float64, error) { +func monthlyCostFromConvertibleRI(ri ec2ConvertibleRI) (float64, error) { return ricost.MonthlyCostFromConvertibleRI(ri) } @@ -1689,7 +1667,7 @@ func excludedRIReason(err error) string { // convertToExchangeTypes converts provider-specific types to the exchange package types. // RIs whose monthly cost cannot be computed are returned as excluded rather // than dropped silently, so the response can surface them. -func convertToExchangeTypes(instances []ec2svc.ConvertibleRI, utilData []recommendations.RIUtilization) ([]exchange.RIInfo, []exchange.UtilizationInfo, []ExcludedRI) { +func convertToExchangeTypes(instances []ec2ConvertibleRI, utilData []riUtilization) ([]exchange.RIInfo, []exchange.UtilizationInfo, []ExcludedRI) { var excluded []ExcludedRI riInfos := make([]exchange.RIInfo, 0, len(instances)) for i := range instances { @@ -1799,7 +1777,7 @@ func (h *Handler) getReshapeRecommendations(ctx context.Context, req *events.Lam } recsAdapter := h.buildReshapeRecsClient(cfg) - fetch := func(fetchCtx context.Context, days int) ([]recommendations.RIUtilization, error) { + fetch := func(fetchCtx context.Context, days int) ([]riUtilization, error) { return recsAdapter.GetRIUtilization(fetchCtx, days, p.region) } cacheKey := config.RIUtilizationCacheKey{CloudAccountID: cloudAccountID, Region: p.region, LookbackDays: p.lookbackDays} @@ -1853,7 +1831,7 @@ func (h *Handler) attachReshapeStaleness(ctx context.Context, resp *ReshapeRecom // firstNonEmptyCurrency returns the first RI CurrencyCode set, or "" when none // is, so the response omits currency_code instead of claiming USD (#361). -func firstNonEmptyCurrency(instances []ec2svc.ConvertibleRI) string { +func firstNonEmptyCurrency(instances []ec2ConvertibleRI) string { for _rvc := range instances { inst := instances[_rvc] if inst.CurrencyCode != "" { @@ -2111,18 +2089,18 @@ func mapAWSExchangeError(opMsg string, err error) error { // ConvertibleRIsResponse holds the list of convertible RIs. type ConvertibleRIsResponse struct { - Instances []ec2svc.ConvertibleRI `json:"instances"` + Instances []ec2ConvertibleRI `json:"instances"` } // ExchangeableAzureRIsResponse holds the list of Azure VM reservations that // are eligible for the cross-SKU/cross-region exchange flow. type ExchangeableAzureRIsResponse struct { - Reservations []azurecompute.ExchangeableReservation `json:"reservations"` + Reservations []azureExchangeableReservation `json:"reservations"` } // RIUtilizationResponse holds per-RI utilization data. type RIUtilizationResponse struct { - Utilization []recommendations.RIUtilization `json:"utilization"` + Utilization []riUtilization `json:"utilization"` } // ReshapeRecommendationsResponse holds reshape recommendations. diff --git a/internal/api/provider_wiring.go b/internal/api/provider_wiring.go new file mode 100644 index 00000000..7e8ced92 --- /dev/null +++ b/internal/api/provider_wiring.go @@ -0,0 +1,99 @@ +package api + +// provider_wiring.go is the composition file for the concrete +// cloud-commitments-go provider packages the RI exchange and Marketplace +// handlers use (platform#41). It is the only non-test file in this package +// that imports them: the handlers reach AWS and Azure through the narrow +// client interfaces they declare (reshapeEC2Client, reshapeRecsClient, +// targetOfferingsEC2Client, azureExchangeClient, marketplaceEC2Client), built +// by the constructors below, and name the provider data types through the +// aliases below. provider_wiring_imports_test.go enforces this. +// +// The aliases are Go type aliases, not new types, so values, JSON encoding and +// the tests that build the provider types directly are unchanged. + +import ( + "context" + "fmt" + + "github.com/Azure/azure-sdk-for-go/sdk/azcore" + "github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/reservations/armreservations" + "github.com/aws/aws-sdk-go-v2/aws" + awsconfig "github.com/aws/aws-sdk-go-v2/config" + + awsprovider "github.com/LeanerCloud/cloud-commitments-go/providers/aws" + "github.com/LeanerCloud/cloud-commitments-go/providers/aws/recommendations" + ec2svc "github.com/LeanerCloud/cloud-commitments-go/providers/aws/services/ec2" + azurecompute "github.com/LeanerCloud/cloud-commitments-go/providers/azure/services/compute" +) + +// Provider data types: AWS EC2 and Cost Explorer, Azure compute exchange and +// the Azure reservation term enum. +type ( + ec2ConvertibleRI = ec2svc.ConvertibleRI + ec2TargetOffering = ec2svc.TargetOffering + ec2ListTargetOfferingsParams = ec2svc.ListTargetOfferingsParams + ec2MarketplaceListingRequest = ec2svc.MarketplaceListingRequest + ec2MarketplaceListingResult = ec2svc.MarketplaceListingResult + ec2MarketplacePriceTier = ec2svc.MarketplacePriceTier + riUtilization = recommendations.RIUtilization + azureExchangeableReservation = azurecompute.ExchangeableReservation + azureExchangeTarget = azurecompute.ExchangeTarget + azureExchangePreview = azurecompute.ExchangePreview + azureExchangeResult = azurecompute.ExchangeResult + azureCompatibleOffering = azurecompute.CompatibleOffering + azureReservationTerm = armreservations.ReservationTerm +) + +// errAzureUnsupportedAppliedScope is azurecompute.ErrUnsupportedAppliedScope: +// CalculateExchange refuses a source whose applied scope cannot carry over. +var errAzureUnsupportedAppliedScope = azurecompute.ErrUnsupportedAppliedScope + +// loadDefaultAWSConfig loads the ambient AWS SDK config (env, shared files, +// instance or task role). +func loadDefaultAWSConfig(ctx context.Context) (aws.Config, error) { + return awsconfig.LoadDefaultConfig(ctx) +} + +// newReshapeEC2Client builds the EC2 client that lists convertible RIs and +// the region's offered instance types. +func newReshapeEC2Client(cfg aws.Config) reshapeEC2Client { + return awsprovider.NewEC2ClientDirect(cfg) +} + +// newReshapeRecsClient builds the Cost Explorer recommendations adapter. +func newReshapeRecsClient(cfg aws.Config) reshapeRecsClient { + return awsprovider.NewRecommendationsClientDirect(cfg) +} + +// newTargetOfferingsEC2Client builds the EC2 client that lists convertible RIs +// and their exchange target offerings. +func newTargetOfferingsEC2Client(cfg aws.Config) targetOfferingsEC2Client { + return awsprovider.NewEC2ClientDirect(cfg) +} + +// newMarketplaceEC2Client builds the EC2 client for RI Marketplace listings. +func newMarketplaceEC2Client(cfg aws.Config) marketplaceEC2Client { + return awsprovider.NewEC2ClientDirect(cfg) +} + +// newAzureExchangeClient builds the Azure compute client for the exchange +// flow. Region is left empty: ListExchangeableReservations uses the +// tenant-wide armreservations API, which is not scoped to a region. +func newAzureExchangeClient(cred azcore.TokenCredential, subscriptionID string) azureExchangeClient { + return azurecompute.NewClient(cred, subscriptionID, "") +} + +// azureReservationTermFromString converts the HTTP-layer term string to the +// typed SDK enum, rejecting anything outside armreservations' +// PossibleReservationTermValues(). No fallback: an unrecognized term is a +// 400, never silently coerced to a default term (feedback_sdk_enum_string_literals). +func azureReservationTermFromString(s string) (azureReservationTerm, error) { + term := armreservations.ReservationTerm(s) + for _, t := range armreservations.PossibleReservationTermValues() { + if t == term { + return term, nil + } + } + return "", fmt.Errorf("unsupported term %q", s) +} diff --git a/internal/api/provider_wiring_imports_test.go b/internal/api/provider_wiring_imports_test.go new file mode 100644 index 00000000..b1242d28 --- /dev/null +++ b/internal/api/provider_wiring_imports_test.go @@ -0,0 +1,101 @@ +package api + +import ( + "go/parser" + "go/token" + "os" + "path/filepath" + "strconv" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// providerWiringFile is the one non-test file in this package allowed to +// import the concrete cloud-commitments-go provider packages (platform#41). +const providerWiringFile = "provider_wiring.go" + +// concreteProviderPkgPrefix covers every concrete provider package +// (providers/aws, providers/aws/services/ec2, providers/azure/..., ...). +const concreteProviderPkgPrefix = "github.com/LeanerCloud/cloud-commitments-go/providers/" + +// riExchangeForbiddenImportPrefixes are the imports handler_ri_exchange.go +// must not take: concrete provider packages, the Azure SDK, and the AWS SDK's +// config loader and service clients. The generic aws.Config value type and +// smithy's error interface stay allowed. +var riExchangeForbiddenImportPrefixes = []string{ + concreteProviderPkgPrefix, + "github.com/Azure/azure-sdk-for-go/", + "github.com/aws/aws-sdk-go-v2/config", + "github.com/aws/aws-sdk-go-v2/service/", +} + +// fileImports returns the import paths of the Go file at path. +func fileImports(t *testing.T, path string) []string { + t.Helper() + f, err := parser.ParseFile(token.NewFileSet(), path, nil, parser.ImportsOnly) + require.NoError(t, err, path) + out := make([]string, 0, len(f.Imports)) + for _, imp := range f.Imports { + p, err := strconv.Unquote(imp.Path.Value) + require.NoError(t, err, path) + out = append(out, p) + } + return out +} + +func hasAnyPrefix(s string, prefixes []string) bool { + for _, p := range prefixes { + if strings.HasPrefix(s, p) { + return true + } + } + return false +} + +// The RI exchange HTTP handler reaches AWS and Azure only through the narrow +// client interfaces; the concrete constructors and types live in +// provider_wiring.go. +func TestRIExchangeHandlerHasNoConcreteProviderImports(t *testing.T) { + imports := fileImports(t, "handler_ri_exchange.go") + require.NotEmpty(t, imports, "the guard must actually read the handler's imports") + var bad []string + for _, p := range imports { + if hasAnyPrefix(p, riExchangeForbiddenImportPrefixes) { + bad = append(bad, p) + } + } + assert.Empty(t, bad, "move concrete provider construction and types into %s", providerWiringFile) +} + +// No non-test file in this package other than provider_wiring.go imports a +// concrete cloud-commitments-go provider package. +func TestOnlyProviderWiringImportsConcreteProviders(t *testing.T) { + entries, err := os.ReadDir(".") + require.NoError(t, err) + var bad []string + scanned := 0 + wiringImportsProviders := false + for _, e := range entries { + name := e.Name() + if e.IsDir() || !strings.HasSuffix(name, ".go") || strings.HasSuffix(name, "_test.go") { + continue + } + scanned++ + for _, p := range fileImports(t, filepath.Join(".", name)) { + if !strings.HasPrefix(p, concreteProviderPkgPrefix) { + continue + } + if name == providerWiringFile { + wiringImportsProviders = true + continue + } + bad = append(bad, name+": "+p) + } + } + require.Greater(t, scanned, 50, "the scan must actually reach the package sources") + assert.Empty(t, bad, "import concrete provider packages only from %s", providerWiringFile) + assert.True(t, wiringImportsProviders, "%s must exist and hold the concrete provider imports", providerWiringFile) +} diff --git a/internal/api/ri_exchange_audit.go b/internal/api/ri_exchange_audit.go index 7d12a987..28d475b5 100644 --- a/internal/api/ri_exchange_audit.go +++ b/internal/api/ri_exchange_audit.go @@ -10,7 +10,6 @@ import ( "github.com/LeanerCloud/cloud-commitments-go/pkg/exchange" "github.com/LeanerCloud/cloud-commitments-go/pkg/logging" - azurecompute "github.com/LeanerCloud/cloud-commitments-go/providers/azure/services/compute" "github.com/LeanerCloud/cloud-commitments-platform/internal/config" "github.com/LeanerCloud/cloud-commitments-platform/internal/ctxutil" "github.com/google/uuid" @@ -200,7 +199,7 @@ func awsDirectExchangeOutcome(exchangeID string, quote *exchange.ExchangeQuoteSu // azureDirectExchangeAudit builds the audit row for a direct Azure execute // from the request and the fresh re-quote that cleared the guardrails. -func azureDirectExchangeAudit(session *Session, accountID string, body AzureExecuteExchangeRequestBody, maxRat *big.Rat, preview *azurecompute.ExchangePreview) *config.RIExchangeDirectExecution { +func azureDirectExchangeAudit(session *Session, accountID string, body AzureExecuteExchangeRequestBody, maxRat *big.Rat, preview *azureExchangePreview) *config.RIExchangeDirectExecution { sourceIDs := make([]string, 0, len(body.Sources)) for _, s := range body.Sources { sourceIDs = append(sourceIDs, s.ReservationID) @@ -223,7 +222,7 @@ func azureDirectExchangeAudit(session *Session, accountID string, body AzureExec } // azureDirectExchangeOutcome builds the outcome of a direct Azure execute. -func azureDirectExchangeOutcome(result *azurecompute.ExchangeResult, providerErr, mapped error) config.RIExchangeDirectOutcome { +func azureDirectExchangeOutcome(result *azureExchangeResult, providerErr, mapped error) config.RIExchangeDirectOutcome { if providerErr != nil { return config.RIExchangeDirectOutcome{ Status: directExchangeFailureStatus(mapped), diff --git a/internal/api/ri_exchange_refusals.go b/internal/api/ri_exchange_refusals.go index ef19f4fe..537802c1 100644 --- a/internal/api/ri_exchange_refusals.go +++ b/internal/api/ri_exchange_refusals.go @@ -4,7 +4,6 @@ import ( "errors" "github.com/LeanerCloud/cloud-commitments-go/pkg/exchange" - azurecompute "github.com/LeanerCloud/cloud-commitments-go/providers/azure/services/compute" ) // exchangeRefusals are the exported sentinels cloud-commitments-go wraps when @@ -14,15 +13,16 @@ import ( // currency), so the USD max_payment_due_usd cap cannot be enforced // against it. Returned by ExecuteExchange before // AcceptReservedInstancesExchangeQuote. -// - azurecompute.ErrUnsupportedAppliedScope: a source reservation's applied -// scope cannot be carried onto the purchased reservation, or the sources -// mix scopes. Returned by CalculateExchange before any Azure call. +// - errAzureUnsupportedAppliedScope (azurecompute.ErrUnsupportedAppliedScope, +// aliased in provider_wiring.go): a source reservation's applied scope +// cannot be carried onto the purchased reservation, or the sources mix +// scopes. Returned by CalculateExchange before any Azure call. // // Both describe the request or the account it targets, not a server fault, // and both prove the exchange did not happen. var exchangeRefusals = []error{ exchange.ErrQuoteNotUSD, - azurecompute.ErrUnsupportedAppliedScope, + errAzureUnsupportedAppliedScope, } // exchangeRefusalError returns a 422 carrying err's message when err wraps diff --git a/internal/api/ri_utilization_cache.go b/internal/api/ri_utilization_cache.go index 431ff895..849ed111 100644 --- a/internal/api/ri_utilization_cache.go +++ b/internal/api/ri_utilization_cache.go @@ -8,7 +8,6 @@ import ( "time" "github.com/LeanerCloud/cloud-commitments-go/pkg/logging" - "github.com/LeanerCloud/cloud-commitments-go/providers/aws/recommendations" "github.com/LeanerCloud/cloud-commitments-platform/internal/config" "golang.org/x/sync/singleflight" ) @@ -59,7 +58,7 @@ type riUtilizationCacheStore interface { // riUtilizationFetcher is the signature of the function that retrieves // fresh data from Cost Explorer. Parameterising lets tests inject a // fake. -type riUtilizationFetcher func(ctx context.Context, lookbackDays int) ([]recommendations.RIUtilization, error) +type riUtilizationFetcher func(ctx context.Context, lookbackDays int) ([]riUtilization, error) // riUtilizationCache wraps a Postgres-backed store with stale-while- // revalidate semantics on non-Lambda runtimes. Lambda containers can't @@ -102,7 +101,7 @@ func (c *riUtilizationCache) getOrFetch( cacheKey config.RIUtilizationCacheKey, softTTL, hardTTL time.Duration, fetch riUtilizationFetcher, -) ([]recommendations.RIUtilization, error) { +) ([]riUtilization, error) { key := riUtilizationFlightKey(cacheKey) entry, err := c.store.GetRIUtilizationCache(ctx, cacheKey) @@ -187,7 +186,7 @@ func (c *riUtilizationCache) kickBackgroundRefresh(key string, cacheKey config.R // storePayload marshals and upserts the fresh data. Errors are logged // but not returned — callers have the data in hand either way. -func (c *riUtilizationCache) storePayload(ctx context.Context, cacheKey config.RIUtilizationCacheKey, data []recommendations.RIUtilization) { +func (c *riUtilizationCache) storePayload(ctx context.Context, cacheKey config.RIUtilizationCacheKey, data []riUtilization) { payload, err := json.Marshal(data) if err != nil { logging.Warnf("ri_utilization_cache: failed to marshal payload (key=%s): %v", riUtilizationFlightKey(cacheKey), err) @@ -198,8 +197,8 @@ func (c *riUtilizationCache) storePayload(ctx context.Context, cacheKey config.R } } -func unmarshalRIUtilization(payload []byte) ([]recommendations.RIUtilization, error) { - var data []recommendations.RIUtilization +func unmarshalRIUtilization(payload []byte) ([]riUtilization, error) { + var data []riUtilization if err := json.Unmarshal(payload, &data); err != nil { return nil, err }