From 2bab8bb12c5a89adc7f53cecb5b436673d02dc00 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Thu, 5 Nov 2026 14:00:00 +0100 Subject: [PATCH 1/4] feat(api): show bound plans' groups on planned and upcoming rows MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Slice A, step A6 (display rows) of #1746. - planDisplayFor replaces firstServiceConfig for the planned-purchase and upcoming rows: - unmarked plans show their first entry, exactly as before; - a bound plan with several enabled groups keeps each field its groups share, and shows "multiple" (term 0, rendered as "—") where they differ, so an all-AWS EC2 plus RDS plan still shows provider "aws". - Both rows gain providers (omitempty, bound plans only): the distinct providers of the enabled groups. The /dashboard/upcoming provider filter matches on the row's provider or any of these, so a cross-provider plan stays visible under each provider. - openapi.yaml declares providers on both schemas and rewords the provider filter. Unmarked rows keep their JSON byte for byte (no providers key), which a guard test pins. pre-commit: cfn-lint skipped (it can't install here); no CloudFormation files are touched. Part of #1746 Co-Authored-By: Claude Opus 5.5 --- internal/api/handler_dashboard.go | 4 +- .../api/handler_dashboard_upcoming_filter.go | 16 +++- internal/api/handler_purchases.go | 4 +- internal/api/openapi.yaml | 21 ++++- internal/api/plan_display.go | 63 +++++++++++++ internal/api/plan_display_bound_test.go | 93 +++++++++++++++++++ internal/api/types.go | 38 ++++---- 7 files changed, 216 insertions(+), 23 deletions(-) create mode 100644 internal/api/plan_display.go create mode 100644 internal/api/plan_display_bound_test.go diff --git a/internal/api/handler_dashboard.go b/internal/api/handler_dashboard.go index 40ff72268..8317656ae 100644 --- a/internal/api/handler_dashboard.go +++ b/internal/api/handler_dashboard.go @@ -519,8 +519,10 @@ func firstServiceConfig(plan *config.PurchasePlan) config.ServiceConfig { // Step number comes from the execution row directly (already stamped by the // scheduler at instance-create time). func upcomingFromExecution(plan *config.PurchasePlan, exec *config.PurchaseExecution) UpcomingPurchase { - svc := firstServiceConfig(plan) + display := planDisplayFor(plan) + svc := display.svc return UpcomingPurchase{ + Providers: display.providers, ExecutionID: exec.ExecutionID, PlanID: plan.ID, PlanName: plan.Name, diff --git a/internal/api/handler_dashboard_upcoming_filter.go b/internal/api/handler_dashboard_upcoming_filter.go index 35334c7c2..20048ae8a 100644 --- a/internal/api/handler_dashboard_upcoming_filter.go +++ b/internal/api/handler_dashboard_upcoming_filter.go @@ -59,11 +59,11 @@ func (h *Handler) parseUpcomingFilter(ctx context.Context, session *Session, par // matches reports whether an upcoming row passes the filter. The provider is // compared with the row's provider (its plan's first service, as shown in the -// widget). An execution matches the account filter when any account it can buy +// widget), or for a bound plan with any of its group providers (#1746). An execution matches the account filter when any account it can buy // in is selected: a plan-level parent fans out to every plan account, so it is // relevant to each of them, while a fan-out child carries its single account. func (f upcomingFilter) matches(ctx context.Context, h *Handler, row *UpcomingPurchase, exec *config.PurchaseExecution, planAccounts map[string][]config.CloudAccount) (bool, error) { - if f.provider != "" && !strings.EqualFold(row.Provider, f.provider) { + if f.provider != "" && !rowHasProvider(row, f.provider) { return false, nil } if f.accounts == nil { @@ -97,3 +97,15 @@ func (s *dualColumnAccountSet) matchesAny(accounts []config.CloudAccount) bool { } return false } + +func rowHasProvider(row *UpcomingPurchase, provider string) bool { + if strings.EqualFold(row.Provider, provider) { + return true + } + for _, p := range row.Providers { + if strings.EqualFold(p, provider) { + return true + } + } + return false +} diff --git a/internal/api/handler_purchases.go b/internal/api/handler_purchases.go index 6fe82dd9e..83215129a 100644 --- a/internal/api/handler_purchases.go +++ b/internal/api/handler_purchases.go @@ -117,8 +117,10 @@ func (h *Handler) isExecutionInScope(ctx context.Context, scope auth.AccountScop // Provider/service/term/payment are taken from firstServiceConfig (lexicographic key order) // so the result is deterministic regardless of map iteration order. func buildPlannedPurchase(plan *config.PurchasePlan, exec *config.PurchaseExecution) PlannedPurchase { - svc := firstServiceConfig(plan) + display := planDisplayFor(plan) + svc := display.svc row := PlannedPurchase{ + Providers: display.providers, ID: exec.ExecutionID, PlanID: exec.PlanID, PlanName: plan.Name, diff --git a/internal/api/openapi.yaml b/internal/api/openapi.yaml index 602a538b6..0d953d1e1 100644 --- a/internal/api/openapi.yaml +++ b/internal/api/openapi.yaml @@ -167,8 +167,9 @@ paths: - name: provider in: query description: > - Filter by the row's provider (its plan's first service). "all" or - no value means every provider. Any other value is a 400. + Filter by the row's provider (its plan's first service), or for a + bound plan (#1746) any of its group providers. "all" or no value + means every provider. Any other value is a 400. schema: type: string enum: [aws, azure, gcp, all] @@ -6936,6 +6937,14 @@ components: format: date provider: type: string + providers: + type: array + items: + type: string + description: > + Set only for a bound plan (#1746): the distinct providers of its + enabled groups. Such a plan shows "multiple" for provider, service + or payment, and term 0, when its groups differ. service: type: string step_number: @@ -7481,6 +7490,14 @@ components: format: date provider: type: string + providers: + type: array + items: + type: string + description: > + Set only for a bound plan (#1746): the distinct providers of its + enabled groups. Such a plan shows "multiple" for provider, service + or payment, and term 0, when its groups differ. service: type: string resource_type: diff --git a/internal/api/plan_display.go b/internal/api/plan_display.go new file mode 100644 index 000000000..bd0ca9f24 --- /dev/null +++ b/internal/api/plan_display.go @@ -0,0 +1,63 @@ +package api + +import ( + "sort" + + "github.com/LeanerCloud/cloud-commitments-platform/internal/config" +) + +// planDisplay is what the dashboard and planned-purchase rows show for a plan. +type planDisplay struct { + svc config.ServiceConfig + // providers lists a bound plan's distinct enabled group providers, so a + // provider filter can match a plan whose groups span providers. Nil for + // unmarked plans, whose rows keep today's JSON shape. + providers []string +} + +// planDisplayFor summarizes a plan for display. Unmarked plans show their first +// service entry, as before. A bound plan (issue #1746) with several enabled +// groups keeps each field its groups share and shows "multiple" (term 0, which +// the frontend renders as "—") for a field they don't. +func planDisplayFor(plan *config.PurchasePlan) planDisplay { + if !isBoundPlan(plan) { + return planDisplay{svc: firstServiceConfig(plan)} + } + keys := make([]string, 0, len(plan.Services)) + for k, svc := range plan.Services { + if svc.Enabled { + keys = append(keys, k) + } + } + if len(keys) == 0 { + return planDisplay{svc: firstServiceConfig(plan)} + } + sort.Strings(keys) + out := plan.Services[keys[0]] + seen := map[string]struct{}{} + var providers []string + for _, k := range keys { + svc := plan.Services[k] + if _, dup := seen[svc.Provider]; !dup { + seen[svc.Provider] = struct{}{} + providers = append(providers, svc.Provider) + } + out.Provider = sharedOrMultiple(out.Provider, svc.Provider) + out.Service = sharedOrMultiple(out.Service, svc.Service) + out.Payment = sharedOrMultiple(out.Payment, svc.Payment) + if out.Term != svc.Term { + out.Term = 0 + } + } + // Keys start with their provider, so sorted keys give sorted providers. + return planDisplay{svc: out, providers: providers} +} + +const multipleValues = "multiple" + +func sharedOrMultiple(shared, next string) string { + if shared == next { + return shared + } + return multipleValues +} diff --git a/internal/api/plan_display_bound_test.go b/internal/api/plan_display_bound_test.go new file mode 100644 index 000000000..7420267a7 --- /dev/null +++ b/internal/api/plan_display_bound_test.go @@ -0,0 +1,93 @@ +package api + +import ( + "encoding/json" + "testing" + "time" + + "github.com/LeanerCloud/cloud-commitments-platform/internal/config" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// Fixtures are hand-built plans, not real configuration (issue #1746). + +func displayPlan(marked bool, entries map[string]config.ServiceConfig) *config.PurchasePlan { + p := &config.PurchasePlan{ID: "plan-d", Name: "d", Services: entries, RampSchedule: config.RampSchedule{TotalSteps: 2}} + if marked { + p.RampSchedule.AllocationMode = config.RampAllocationBoundGroups + } + return p +} + +func displayEntry(provider, service string, term int, payment string) config.ServiceConfig { + return config.ServiceConfig{Provider: provider, Service: service, Enabled: true, Term: term, Payment: payment} +} + +func TestPlanDisplayFor(t *testing.T) { + ec2 := displayEntry("aws", "ec2", 1, "no-upfront") + rds := displayEntry("aws", "rds", 1, "no-upfront") + vm := displayEntry("azure", "compute", 3, "monthly") + + t.Run("same provider keeps it; differing service is multiple", func(t *testing.T) { + d := planDisplayFor(displayPlan(true, map[string]config.ServiceConfig{"aws/ec2#g1": ec2, "aws/rds#g2": rds})) + assert.Equal(t, "aws", d.svc.Provider) + assert.Equal(t, "multiple", d.svc.Service) + assert.Equal(t, 1, d.svc.Term) + assert.Equal(t, "no-upfront", d.svc.Payment) + assert.Equal(t, []string{"aws"}, d.providers) + }) + t.Run("cross-provider is multiple with term 0", func(t *testing.T) { + d := planDisplayFor(displayPlan(true, map[string]config.ServiceConfig{"aws/ec2#g1": ec2, "azure/compute#g2": vm})) + assert.Equal(t, "multiple", d.svc.Provider) + assert.Equal(t, "multiple", d.svc.Service) + assert.Equal(t, 0, d.svc.Term) + assert.Equal(t, "multiple", d.svc.Payment) + assert.Equal(t, []string{"aws", "azure"}, d.providers) + }) + t.Run("disabled groups are left out", func(t *testing.T) { + off := vm + off.Enabled = false + d := planDisplayFor(displayPlan(true, map[string]config.ServiceConfig{"aws/ec2#g1": ec2, "azure/compute#g2": off})) + assert.Equal(t, "aws", d.svc.Provider) + assert.Equal(t, "ec2", d.svc.Service) + assert.Equal(t, []string{"aws"}, d.providers) + }) + t.Run("(guard) an unmarked multi-service plan shows its first entry, no providers", func(t *testing.T) { + d := planDisplayFor(displayPlan(false, map[string]config.ServiceConfig{"aws/ec2": ec2, "azure/compute": vm})) + assert.Equal(t, ec2, d.svc) + assert.Nil(t, d.providers) + }) +} + +func TestPlanRows_CarryBoundProviders(t *testing.T) { + exec := &config.PurchaseExecution{ExecutionID: "e1", PlanID: "plan-d", ScheduledDate: time.Date(2026, 11, 5, 0, 0, 0, 0, time.UTC)} + bound := displayPlan(true, map[string]config.ServiceConfig{ + "aws/ec2#g1": displayEntry("aws", "ec2", 1, "no-upfront"), + "azure/compute#g2": displayEntry("azure", "compute", 3, "monthly"), + }) + up := upcomingFromExecution(bound, exec) + assert.Equal(t, "multiple", up.Provider) + assert.Equal(t, []string{"aws", "azure"}, up.Providers) + planned := buildPlannedPurchase(bound, exec) + assert.Equal(t, []string{"aws", "azure"}, planned.Providers) + assert.Equal(t, 0, planned.Term) + + // (guard) an unmarked plan's rows keep today's JSON: no providers key. + legacy := displayPlan(false, map[string]config.ServiceConfig{"aws/ec2": displayEntry("aws", "ec2", 1, "no-upfront")}) + for _, row := range []any{upcomingFromExecution(legacy, exec), buildPlannedPurchase(legacy, exec)} { + raw, err := json.Marshal(row) + require.NoError(t, err) + assert.NotContains(t, string(raw), `"providers"`) + } +} + +func TestUpcomingFilter_MatchesAnyBoundProvider(t *testing.T) { + row := &UpcomingPurchase{Provider: "multiple", Providers: []string{"aws", "azure"}} + assert.True(t, rowHasProvider(row, "azure")) + assert.True(t, rowHasProvider(row, "AWS")) + assert.False(t, rowHasProvider(row, "gcp")) + // (guard) an unmarked row matches on its provider alone, as before. + assert.True(t, rowHasProvider(&UpcomingPurchase{Provider: "aws"}, "aws")) + assert.False(t, rowHasProvider(&UpcomingPurchase{Provider: "aws"}, "azure")) +} diff --git a/internal/api/types.go b/internal/api/types.go index ed0c94519..676b4240f 100644 --- a/internal/api/types.go +++ b/internal/api/types.go @@ -799,6 +799,8 @@ type UpcomingPurchase struct { StepNumber int `json:"step_number"` TotalSteps int `json:"total_steps"` EstimatedSavings float64 `json:"estimated_savings"` + // Providers lists a bound plan's group providers (#1746); omitted otherwise. + Providers []string `json:"providers,omitempty"` // HasRecommendations is false for a step that carries no recommendations, // whose EstimatedSavings is then a placeholder, not data (platform#609). HasRecommendations bool `json:"has_recommendations"` @@ -815,23 +817,25 @@ type PlannedPurchase struct { // purchase, mirroring PurchaseHistoryRecord.CreatedByUserID. The // frontend gates the row action buttons on creator-scope ownership // (issue #950); omitted for legacy rows with a NULL creator. - CreatedByUserID *string `json:"created_by_user_id,omitempty"` - Status string `json:"status"` - PlanID string `json:"plan_id"` - PlanName string `json:"plan_name"` - ScheduledDate string `json:"scheduled_date"` - Provider string `json:"provider"` - Service string `json:"service"` - ResourceType string `json:"resource_type"` - Region string `json:"region"` - ID string `json:"id"` - Payment string `json:"payment"` - Count int `json:"count"` - UpfrontCost float64 `json:"upfront_cost"` - EstimatedSavings float64 `json:"estimated_savings"` - StepNumber int `json:"step_number"` - TotalSteps int `json:"total_steps"` - Term int `json:"term"` + CreatedByUserID *string `json:"created_by_user_id,omitempty"` + Status string `json:"status"` + PlanID string `json:"plan_id"` + PlanName string `json:"plan_name"` + ScheduledDate string `json:"scheduled_date"` + Provider string `json:"provider"` + Service string `json:"service"` + // Providers lists a bound plan's group providers (#1746); omitted otherwise. + Providers []string `json:"providers,omitempty"` + ResourceType string `json:"resource_type"` + Region string `json:"region"` + ID string `json:"id"` + Payment string `json:"payment"` + Count int `json:"count"` + UpfrontCost float64 `json:"upfront_cost"` + EstimatedSavings float64 `json:"estimated_savings"` + StepNumber int `json:"step_number"` + TotalSteps int `json:"total_steps"` + Term int `json:"term"` // HasRecommendations is false for a step that carries no recommendations, // whose Count, UpfrontCost, EstimatedSavings and ResourceType are then // placeholders, not data (platform#609). From 71a082f9f144a901a2fd422090ece319b179e269 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Fri, 6 Nov 2026 23:31:41 +0100 Subject: [PATCH 2/4] feat(api): report bound_count instead of bound identities in plan responses A bound group can hold up to 1000 identity keys, and returning them on every plan read could push a response past the Lambda limit for clients without gzip. Plan responses now go through planForResponse, which copies the services map and replaces each bound group's bound_identities with bound_count. BoundCount is response-only: it is never stored (dynamodbav:"-") and never read from a request. Plans without bound identities are returned unchanged, so their JSON is the same as before. Part of #1746 Co-Authored-By: Claude Opus 5.5 --- internal/api/handler_config.go | 2 +- internal/api/handler_plans.go | 10 ++--- internal/api/openapi.yaml | 17 ++++++++ internal/api/plan_response.go | 32 ++++++++++++++ internal/api/plan_response_test.go | 69 ++++++++++++++++++++++++++++++ internal/config/types.go | 4 ++ 6 files changed, 128 insertions(+), 6 deletions(-) create mode 100644 internal/api/plan_response.go create mode 100644 internal/api/plan_response_test.go diff --git a/internal/api/handler_config.go b/internal/api/handler_config.go index bad6784da..35fbadc12 100644 --- a/internal/api/handler_config.go +++ b/internal/api/handler_config.go @@ -505,7 +505,7 @@ func (h *Handler) updateServiceConfig(ctx context.Context, req *events.LambdaFun cfg.CreatedAt, cfg.UpdatedAt = nil, nil // The plan-group fields (#1746) belong only to purchase plan entries; a // global service setting never carries them, even on a first save. - cfg.GroupID, cfg.BoundIdentities, cfg.MaxStepCommitment = "", nil, nil + cfg.GroupID, cfg.BoundIdentities, cfg.MaxStepCommitment, cfg.BoundCount = "", nil, nil, 0 parts := strings.SplitN(service, "/", 2) if len(parts) == 2 { diff --git a/internal/api/handler_plans.go b/internal/api/handler_plans.go index b4ee6c277..5c4c82cb7 100644 --- a/internal/api/handler_plans.go +++ b/internal/api/handler_plans.go @@ -91,7 +91,7 @@ func (h *Handler) scopedPlanFilter(ctx context.Context, session *Session, accoun func (h *Handler) attachPlanHealth(ctx context.Context, plans []config.PurchasePlan, now time.Time) []PlanWithHealth { result := make([]PlanWithHealth, len(plans)) for i := range plans { - result[i] = PlanWithHealth{PurchasePlan: plans[i]} + result[i] = PlanWithHealth{PurchasePlan: *planForResponse(&plans[i])} } if len(plans) == 0 { return result @@ -240,7 +240,7 @@ func (h *Handler) createPlan(ctx context.Context, httpReq *events.LambdaFunction plan.ID, len(req.TargetAccounts), err) } - return plan, nil + return planForResponse(plan), nil } // validateTargetAccounts rejects a missing/empty target_accounts payload and @@ -297,7 +297,7 @@ func (h *Handler) getPlan(ctx context.Context, req *events.LambdaFunctionURLRequ plan.NextExecutionDate = calculateNextExecutionDate(plan, time.Now()) } - return plan, nil + return planForResponse(plan), nil } func (h *Handler) updatePlan(ctx context.Context, httpReq *events.LambdaFunctionURLRequest, planID string) (any, error) { @@ -342,7 +342,7 @@ func (h *Handler) updatePlan(ctx context.Context, httpReq *events.LambdaFunction return nil, err } - return plan, nil + return planForResponse(plan), nil } // updatePlanGated writes plan with the arming gate decided against the plan's @@ -998,5 +998,5 @@ func (h *Handler) patchPlan(ctx context.Context, httpReq *events.LambdaFunctionU return nil, err } - return plan, nil + return planForResponse(plan), nil } diff --git a/internal/api/openapi.yaml b/internal/api/openapi.yaml index 0d953d1e1..4190f117e 100644 --- a/internal/api/openapi.yaml +++ b/internal/api/openapi.yaml @@ -6818,6 +6818,23 @@ components: Minimum instance/node count a recommendation must carry to be surfaced (GUI/persisted equivalent of the CLI --min-count flag). 0 (default) disables the filter. + group_id: + type: string + readOnly: true + description: | + A bound plan's group id (#1746). Set only on a bound plan's services + entries; omitted elsewhere. + bound_count: + type: integer + readOnly: true + description: | + How many recommendations a bound plan's group (#1746) is bound to. + Plan responses report this count instead of the bound identities. + max_step_commitment: + type: number + description: | + A bound plan group's own per-step cap (#1746); the effective cap is + the lower of this and the plan's max_step_commitment. created_at: type: string format: date-time diff --git a/internal/api/plan_response.go b/internal/api/plan_response.go new file mode 100644 index 000000000..728a59783 --- /dev/null +++ b/internal/api/plan_response.go @@ -0,0 +1,32 @@ +package api + +import "github.com/LeanerCloud/cloud-commitments-platform/internal/config" + +// planForResponse is the plan as plan responses show it (issue #1746): each +// bound group reports bound_count instead of its bound_identities, which can +// hold up to 1000 identity keys and would otherwise inflate every plan read +// past the Lambda response limit for clients without gzip. Plans with no bound +// identities are returned as they are, so their JSON is unchanged. The stored +// plan is never modified: the services map is copied first. +func planForResponse(plan *config.PurchasePlan) *config.PurchasePlan { + if plan == nil || !hasBoundIdentities(plan) { + return plan + } + out := *plan + out.Services = make(map[string]config.ServiceConfig, len(plan.Services)) + for k, svc := range plan.Services { + svc.BoundCount = len(svc.BoundIdentities) + svc.BoundIdentities = nil + out.Services[k] = svc + } + return &out +} + +func hasBoundIdentities(plan *config.PurchasePlan) bool { + for _, svc := range plan.Services { + if len(svc.BoundIdentities) > 0 { + return true + } + } + return false +} diff --git a/internal/api/plan_response_test.go b/internal/api/plan_response_test.go new file mode 100644 index 000000000..dc0ef31a4 --- /dev/null +++ b/internal/api/plan_response_test.go @@ -0,0 +1,69 @@ +package api + +import ( + "encoding/json" + "fmt" + "testing" + + "github.com/LeanerCloud/cloud-commitments-platform/internal/config" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// Fixtures are hand-built plans, not real configuration (issue #1746). + +func boundResponsePlan(ids int) *config.PurchasePlan { + bound := make([]string, ids) + for i := range bound { + // About the size of a real RecIdentityKey: provider, account, service, + // region, type, engine, term, payment, AZ, platform, tenancy. + bound[i] = fmt.Sprintf("aws\x1f3f1c2a9e-0000-4000-8000-%012d\x1fec2\x1fap-southeast-3\x1fu-24tb1.112xlarge\x1f\x1f3\x1fpartial-upfront\x1fap-southeast-3a\x1fLinux with SQL Server Standard\x1fdedicated", i) + } + return &config.PurchasePlan{ID: "plan-r", Name: "r", + RampSchedule: config.RampSchedule{AllocationMode: config.RampAllocationBoundGroups}, + Services: map[string]config.ServiceConfig{"aws/ec2#g1": { + Provider: "aws", Service: "ec2", Enabled: true, MinCount: 1, GroupID: "g1", BoundIdentities: bound, + }}} +} + +func TestPlanForResponse_ReportsCountsAndKeepsTheStoredPlan(t *testing.T) { + stored := boundResponsePlan(5000) + out := planForResponse(stored) + raw, err := json.Marshal(out) + require.NoError(t, err) + assert.NotContains(t, string(raw), "bound_identities") + assert.Contains(t, string(raw), `"bound_count":5000`) + assert.Less(t, len(raw), 100*1024, "a 5000-identity plan must stay small in responses") + + // The response copy never touches the stored plan: a write after a GET + // on the same in-memory value still has every identity. + assert.Len(t, stored.Services["aws/ec2#g1"].BoundIdentities, 5000) + assert.Zero(t, stored.Services["aws/ec2#g1"].BoundCount) + stored.Name = "renamed" + saved, err := json.Marshal(stored.Services) + require.NoError(t, err) + assert.Contains(t, string(saved), "bound_identities") +} + +// (guard) A plan without bound identities is returned as it is. +func TestPlanForResponse_UnboundPlanIsUnchanged(t *testing.T) { + legacy := &config.PurchasePlan{ID: "plan-l", Services: map[string]config.ServiceConfig{"aws/ec2": {Provider: "aws", Service: "ec2", Enabled: true}}} + assert.Same(t, legacy, planForResponse(legacy)) + before, err := json.Marshal(legacy) + require.NoError(t, err) + after, err := json.Marshal(planForResponse(legacy)) + require.NoError(t, err) + assert.Equal(t, string(before), string(after)) + assert.Nil(t, planForResponse(nil)) +} + +func TestPlanResponses_UseTheCountedCopy(t *testing.T) { + stored := boundResponsePlan(3) + h := &Handler{} + list := h.attachPlanHealth(t.Context(), []config.PurchasePlan{*stored}, stored.CreatedAt) + require.Len(t, list, 1) + g := list[0].Services["aws/ec2#g1"] + assert.Nil(t, g.BoundIdentities) + assert.Equal(t, 3, g.BoundCount) + assert.Len(t, stored.Services["aws/ec2#g1"].BoundIdentities, 3, "the listed plan is not modified") +} diff --git a/internal/config/types.go b/internal/config/types.go index e55729b5c..003075e03 100644 --- a/internal/config/types.go +++ b/internal/config/types.go @@ -246,6 +246,10 @@ type ServiceConfig struct { GroupID string `json:"group_id,omitempty" dynamodbav:"group_id,omitempty"` BoundIdentities []string `json:"bound_identities,omitempty" dynamodbav:"bound_identities,omitempty"` MaxStepCommitment *float64 `json:"max_step_commitment,omitempty" dynamodbav:"max_step_commitment,omitempty"` + // BoundCount is response-only: plan responses replace BoundIdentities with + // its length so a large selection does not bloat every plan read. It is + // never stored or read from a request. + BoundCount int `json:"bound_count,omitempty" dynamodbav:"-"` } // PurchasePlan represents a saved purchase plan for automated execution. From 9c0529aaf242ebe247ff073cd9c21d09b8cf6135 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Fri, 6 Nov 2026 23:49:36 +0100 Subject: [PATCH 3/4] fix(api): give the plan-response test a store and index Services by key TestPlanResponses_UseTheCountedCopy called attachPlanHealth on a Handler with no config store and panicked on the health lookup; it now uses a mock store whose health lookup fails, so the response copies are returned as they are. The four loops over plan Services index by key instead of copying each ServiceConfig per iteration (gocritic rangeValCopy), as the rest of the package does. Part of #1746 Co-Authored-By: Claude Opus 5.5 --- internal/api/plan_display.go | 4 ++-- internal/api/plan_response.go | 7 ++++--- internal/api/plan_response_test.go | 8 +++++++- 3 files changed, 13 insertions(+), 6 deletions(-) diff --git a/internal/api/plan_display.go b/internal/api/plan_display.go index bd0ca9f24..4b3638cd2 100644 --- a/internal/api/plan_display.go +++ b/internal/api/plan_display.go @@ -24,8 +24,8 @@ func planDisplayFor(plan *config.PurchasePlan) planDisplay { return planDisplay{svc: firstServiceConfig(plan)} } keys := make([]string, 0, len(plan.Services)) - for k, svc := range plan.Services { - if svc.Enabled { + for k := range plan.Services { + if plan.Services[k].Enabled { keys = append(keys, k) } } diff --git a/internal/api/plan_response.go b/internal/api/plan_response.go index 728a59783..b458c3e3c 100644 --- a/internal/api/plan_response.go +++ b/internal/api/plan_response.go @@ -14,7 +14,8 @@ func planForResponse(plan *config.PurchasePlan) *config.PurchasePlan { } out := *plan out.Services = make(map[string]config.ServiceConfig, len(plan.Services)) - for k, svc := range plan.Services { + for k := range plan.Services { + svc := plan.Services[k] svc.BoundCount = len(svc.BoundIdentities) svc.BoundIdentities = nil out.Services[k] = svc @@ -23,8 +24,8 @@ func planForResponse(plan *config.PurchasePlan) *config.PurchasePlan { } func hasBoundIdentities(plan *config.PurchasePlan) bool { - for _, svc := range plan.Services { - if len(svc.BoundIdentities) > 0 { + for k := range plan.Services { + if len(plan.Services[k].BoundIdentities) > 0 { return true } } diff --git a/internal/api/plan_response_test.go b/internal/api/plan_response_test.go index dc0ef31a4..b10b13577 100644 --- a/internal/api/plan_response_test.go +++ b/internal/api/plan_response_test.go @@ -2,11 +2,13 @@ package api import ( "encoding/json" + "errors" "fmt" "testing" "github.com/LeanerCloud/cloud-commitments-platform/internal/config" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" ) @@ -59,7 +61,11 @@ func TestPlanForResponse_UnboundPlanIsUnchanged(t *testing.T) { func TestPlanResponses_UseTheCountedCopy(t *testing.T) { stored := boundResponsePlan(3) - h := &Handler{} + // The health lookup fails, so attachPlanHealth returns the response copies as they are. + store := new(MockConfigStore) + store.On("CountExecutionsByPlanAndStatus", mock.Anything, mock.Anything, mock.Anything). + Return(nil, errors.New("health unavailable")) + h := &Handler{config: store} list := h.attachPlanHealth(t.Context(), []config.PurchasePlan{*stored}, stored.CreatedAt) require.Len(t, list, 1) g := list[0].Services["aws/ec2#g1"] From 373f7efda27bda2aa4407354b209bdfc1f8982c3 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Sat, 7 Nov 2026 23:32:31 +0100 Subject: [PATCH 4/4] fix(api): pin bound plan responses and match bound rows on any provider - Single-plan responses (get, create, update, patch) are pinned to return bound_count, never bound_identities. - The frontend upcoming scope and the plans provider filter match a row on its provider or any of its bound groups' providers, so a cross-provider bound plan stays visible under each provider. - planDisplayFor takes the provider from the services key prefix, the canonical source, via config.ServiceEntryProvider. - marshalPlanServices drops bound_count, so it is never stored; tests pin the first-save config path and the all-disabled display fallback. Part of #1746. Co-Authored-By: Claude Opus 5.5 --- .../dashboard-upcoming-scope-713.test.ts | 8 ++ .../__tests__/plans-column-filters.test.ts | 14 +++ frontend/src/api/types.ts | 5 + frontend/src/lib/upcoming-scope.ts | 16 ++- frontend/src/plans.ts | 9 +- internal/api/handler_config_test.go | 31 +++++ .../api/handler_plans_bound_response_test.go | 108 ++++++++++++++++++ internal/api/plan_display.go | 12 +- internal/api/plan_display_bound_test.go | 17 +++ internal/config/purchase_plan_providers.go | 6 +- internal/config/store_postgres_plan_writes.go | 4 +- ...ore_postgres_service_config_stamps_test.go | 13 +++ 12 files changed, 230 insertions(+), 13 deletions(-) create mode 100644 internal/api/handler_plans_bound_response_test.go diff --git a/frontend/src/__tests__/dashboard-upcoming-scope-713.test.ts b/frontend/src/__tests__/dashboard-upcoming-scope-713.test.ts index 936979f2e..2034e0ded 100644 --- a/frontend/src/__tests__/dashboard-upcoming-scope-713.test.ts +++ b/frontend/src/__tests__/dashboard-upcoming-scope-713.test.ts @@ -79,6 +79,14 @@ describe('upcoming-scope helpers (#713)', () => { expect(scopeUpcomingToProvider(ROWS, 'gcp')).toEqual([]); }); + test('a cross-provider bound row matches each of its providers (#1746)', () => { + const multi = { ...row('e-multi', 'multiple', 'Bound Plan'), providers: ['aws', 'azure'] }; + const rows = [...ROWS, multi]; + expect(scopeUpcomingToProvider(rows, 'aws').map(r => r.execution_id)).toEqual(['e-aws', 'e-multi']); + expect(scopeUpcomingToProvider(rows, 'azure').map(r => r.execution_id)).toEqual(['e-az', 'e-multi']); + expect(scopeUpcomingToProvider(rows, 'gcp')).toEqual([]); + }); + test('empty message names the provider only when one is filtered', () => { expect(upcomingEmptyMessage('')).toBe('No upcoming scheduled purchases'); expect(upcomingEmptyMessage('all')).toBe('No upcoming scheduled purchases'); diff --git a/frontend/src/__tests__/plans-column-filters.test.ts b/frontend/src/__tests__/plans-column-filters.test.ts index 6e5dcbfd4..1bc85e937 100644 --- a/frontend/src/__tests__/plans-column-filters.test.ts +++ b/frontend/src/__tests__/plans-column-filters.test.ts @@ -227,6 +227,20 @@ describe('Planned Purchases column filters (issue #166 follow-up)', () => { }); }); + test('provider set filter keeps a cross-provider bound row under each of its providers (#1746)', async () => { + const multi = { ...seedPurchases[0], id: 'pp-multi', plan_name: 'Bound Plan', provider: 'multiple', providers: ['aws', 'azure'] }; + (api.getPlannedPurchases as jest.Mock).mockResolvedValue({ purchases: [...seedPurchases, multi] }); + const idsFor = async (values: string[]): Promise => { + plansFilters = { provider: { kind: 'set', values } }; + await loadPlans(); + return Array.from(document.querySelectorAll('#planned-purchases-list tbody tr.planned-purchase-row')) + .map((r) => (r.textContent ?? '').includes('Bound Plan') ? 'bound' : 'other'); + }; + expect(await idsFor(['azure'])).toEqual(['other', 'bound']); + expect(await idsFor(['aws'])).toEqual(['other', 'other', 'bound']); + expect(await idsFor(['gcp'])).toEqual([]); + }); + test('numeric expr filter narrows rows (count >= 2)', async () => { await loadPlans(); expect(countRows()).toBe(3); diff --git a/frontend/src/api/types.ts b/frontend/src/api/types.ts index e45544d75..3705cc199 100644 --- a/frontend/src/api/types.ts +++ b/frontend/src/api/types.ts @@ -103,6 +103,9 @@ export interface UpcomingPurchase { plan_name: string; scheduled_date: string; provider: string; + // Set only for a bound plan: its distinct enabled group providers, since + // provider is then "multiple" when the groups differ. + providers?: string[]; service: string; step_number: number; total_steps: number; @@ -486,6 +489,8 @@ export interface PlannedPurchase { plan_name: string; scheduled_date: string; provider: Provider; + // Set only for a bound plan: its distinct enabled group providers (see UpcomingPurchase). + providers?: string[]; service: string; resource_type: string; region: string; diff --git a/frontend/src/lib/upcoming-scope.ts b/frontend/src/lib/upcoming-scope.ts index 23675bfac..324f229a9 100644 --- a/frontend/src/lib/upcoming-scope.ts +++ b/frontend/src/lib/upcoming-scope.ts @@ -20,6 +20,19 @@ function isProviderFilter(provider: string): boolean { return provider !== '' && provider.toLowerCase() !== 'all'; } +/** + * A bound plan whose groups span providers has provider "multiple" and lists + * its group providers in `providers`; a row matches a chip on either. + */ +export function rowMatchesProvider( + row: { provider?: string; providers?: readonly string[] }, + provider: string, +): boolean { + const want = provider.toLowerCase(); + return (row.provider || '').toLowerCase() === want + || (row.providers ?? []).some(p => p.toLowerCase() === want); +} + /** * Keep only the rows matching the provider chip. '' / 'all' keep every row. * The row's provider is the plan's first service provider (see @@ -30,8 +43,7 @@ export function scopeUpcomingToProvider( provider: string, ): UpcomingPurchase[] { if (!isProviderFilter(provider)) return [...purchases]; - const want = provider.toLowerCase(); - return purchases.filter(p => (p.provider || '').toLowerCase() === want); + return purchases.filter(p => rowMatchesProvider(p, provider)); } /** Empty-state text, naming the provider filter when one hid every row. */ diff --git a/frontend/src/plans.ts b/frontend/src/plans.ts index 43486e630..d562ca4ab 100644 --- a/frontend/src/plans.ts +++ b/frontend/src/plans.ts @@ -18,6 +18,7 @@ import { showSkeletonTiles, showSkeletonRows, teardownSkeleton } from './lib/ske import { renderErrorCallout } from './lib/error-callout'; import { canAccess } from './permissions'; import { applyColumnFilters as applyColumnFiltersLib } from './lib/column-filters'; +import { rowMatchesProvider } from './lib/upcoming-scope'; import { formatPaymentOption, formatTermShort } from './lib/table-format'; import { openColumnFilterPopover, @@ -237,14 +238,18 @@ function applyPlansColumnFilters( purchases: readonly PlannedPurchase[], filters: state.PlansColumnFilters, ): PlannedPurchase[] { - return applyColumnFiltersLib( + const { provider: providerFilter, ...rest } = filters; + const narrowed = applyColumnFiltersLib( purchases, - filters, + rest, { categorical: categoricalCellValueForPlan, numeric: (p, col) => roundForDisplay(numericCellValueForPlan(p, col), displayPrecisionForPlan(col)), }, ); + if (providerFilter?.kind !== 'set') return narrowed; + const values = providerFilter.values; + return narrowed.filter(p => values.some(v => rowMatchesProvider(p, v))); } function categoricalCellValueForPlan(p: PlannedPurchase, col: state.PlansColumnId): string { diff --git a/internal/api/handler_config_test.go b/internal/api/handler_config_test.go index df1297ac5..ea310a1cf 100644 --- a/internal/api/handler_config_test.go +++ b/internal/api/handler_config_test.go @@ -1692,3 +1692,34 @@ func TestHandler_updateConfig_EmptyEnabledProvidersWithoutPermissionIs403(t *tes assert.Equal(t, 403, ce.code) mockStore.AssertNotCalled(t, "SaveGlobalConfig", mock.Anything, mock.Anything) } + +// bound_count is response-only (issue #1746); a first save returns the decoded +// body as it is, so a client that echoes bound_count must not get it stored. +func TestHandler_updateServiceConfig_FirstSaveDropsBoundCount(t *testing.T) { + ctx := context.Background() + mockAuth := new(MockAuthService) + t.Cleanup(func() { mockAuth.AssertExpectations(t) }) + mockAuth.On("ValidateSession", ctx, "admin-token").Return(&Session{ + UserID: "aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa", Email: "admin@example.com", + }, nil) + mockAuth.grantAdmin() + store := new(MockConfigStore) + t.Cleanup(func() { store.AssertExpectations(t) }) + var written *config.ServiceConfig + store.On("UpdateServiceConfigAtomic", ctx, "aws", "rds", mock.Anything). + Run(func(args mock.Arguments) { + apply := args.Get(3).(func(*config.ServiceConfig) (*config.ServiceConfig, error)) + var err error + written, err = apply(nil) + require.NoError(t, err) + }).Return(nil, nil) + h := &Handler{config: store, auth: mockAuth} + + _, err := h.updateServiceConfig(ctx, &events.LambdaFunctionURLRequest{ + Headers: map[string]string{"Authorization": "Bearer admin-token"}, + Body: `{"enabled":true,"term":1,"payment":"no-upfront","coverage":80,"bound_count":9}`, + }, "aws/rds") + require.NoError(t, err) + require.NotNil(t, written) + assert.Zero(t, written.BoundCount) +} diff --git a/internal/api/handler_plans_bound_response_test.go b/internal/api/handler_plans_bound_response_test.go new file mode 100644 index 000000000..9ca6f58e4 --- /dev/null +++ b/internal/api/handler_plans_bound_response_test.go @@ -0,0 +1,108 @@ +package api + +import ( + "context" + "encoding/json" + "testing" + + "github.com/LeanerCloud/cloud-commitments-platform/internal/config" + "github.com/aws/aws-lambda-go/events" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" +) + +// Fixtures are hand-built plans, not real configuration (issue #1746). + +const boundResponsePlanID = "12345678-1234-1234-1234-123456789abc" + +func boundResponseHandler(t *testing.T) (*Handler, *MockConfigStore) { + t.Helper() + store := new(MockConfigStore) + t.Cleanup(func() { store.AssertExpectations(t) }) + auth := new(MockAuthService) + t.Cleanup(func() { auth.AssertExpectations(t) }) + auth.On("ValidateSession", mock.Anything, "admin-token").Return(&Session{ + UserID: "aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa", Email: "admin@example.com", + }, nil) + auth.grantAdmin() + return &Handler{config: store, auth: auth}, store +} + +func boundResponseRequest(body string) *events.LambdaFunctionURLRequest { + return &events.LambdaFunctionURLRequest{Headers: map[string]string{"Authorization": "Bearer admin-token"}, Body: body} +} + +func boundStoredPlan() *config.PurchasePlan { + p := boundResponsePlan(3) + p.ID = boundResponsePlanID + p.Enabled = true + p.RampSchedule = config.RampSchedule{Type: "immediate", PercentPerStep: 100, TotalSteps: 1, AllocationMode: config.RampAllocationBoundGroups} + return p +} + +// assertCountedResponse checks the JSON a client receives and that the plan the +// store handled still has its identities. +func assertCountedResponse(t *testing.T, result any, handled *config.PurchasePlan) { + t.Helper() + raw, err := json.Marshal(result) + require.NoError(t, err) + assert.NotContains(t, string(raw), "bound_identities") + assert.Contains(t, string(raw), `"bound_count":3`) + require.NotNil(t, handled) + g := handled.Services["aws/ec2#g1"] + assert.Len(t, g.BoundIdentities, 3) + assert.Zero(t, g.BoundCount) +} + +func TestGetPlan_ReturnsCountedCopy(t *testing.T) { + h, store := boundResponseHandler(t) + stored := boundStoredPlan() + store.On("GetPurchasePlan", mock.Anything, boundResponsePlanID).Return(stored, nil) + result, err := h.getPlan(context.Background(), boundResponseRequest(""), boundResponsePlanID) + require.NoError(t, err) + assertCountedResponse(t, result, stored) +} + +func TestUpdatePlan_ReturnsCountedCopy(t *testing.T) { + h, store := boundResponseHandler(t) + var saved *config.PurchasePlan + store.On("GetPurchasePlan", mock.Anything, boundResponsePlanID).Return(boundStoredPlan(), nil) + store.On("UpdatePurchasePlan", mock.Anything, mock.AnythingOfType("*config.PurchasePlan")). + Run(func(args mock.Arguments) { saved = args.Get(1).(*config.PurchasePlan) }).Return(nil) + result, err := h.updatePlan(context.Background(), boundResponseRequest(`{"name":"renamed","ramp_schedule":"immediate"}`), boundResponsePlanID) + require.NoError(t, err) + assertCountedResponse(t, result, saved) +} + +func TestPatchPlan_ReturnsCountedCopy(t *testing.T) { + h, store := boundResponseHandler(t) + var saved *config.PurchasePlan + store.On("GetPurchasePlan", mock.Anything, boundResponsePlanID).Return(boundStoredPlan(), nil) + store.On("UpdatePurchasePlan", mock.Anything, mock.AnythingOfType("*config.PurchasePlan")). + Run(func(args mock.Arguments) { saved = args.Get(1).(*config.PurchasePlan) }).Return(nil) + result, err := h.patchPlan(context.Background(), boundResponseRequest(`{"name":"renamed"}`), boundResponsePlanID) + require.NoError(t, err) + assertCountedResponse(t, result, saved) +} + +// The create body cannot carry bound groups, so the store stands in for a +// plan that comes back from it with them. +func TestCreatePlan_ReturnsCountedCopy(t *testing.T) { + h, store := boundResponseHandler(t) + account := "bbbbbbbb-bbbb-bbbb-bbbb-bbbbbbbbbbbb" + var created *config.PurchasePlan + store.On("CreatePurchasePlan", mock.Anything, mock.AnythingOfType("*config.PurchasePlan")). + Run(func(args mock.Arguments) { + created = args.Get(1).(*config.PurchasePlan) + created.Services = boundStoredPlan().Services + }).Return(nil) + store.On("SetPlanAccounts", mock.Anything, mock.Anything, []string{account}).Return(nil) + store.GetCloudAccountFn = func(_ context.Context, id string) (*config.CloudAccount, error) { + return &config.CloudAccount{ID: id, Name: "test-aws", Provider: "aws"}, nil + } + body := `{"name":"New","enabled":true,"provider":"aws","service":"ec2","target_accounts":["` + account + `"]}` + result, err := h.createPlan(context.Background(), boundResponseRequest(body)) + require.NoError(t, err) + assertCountedResponse(t, result, created) +} diff --git a/internal/api/plan_display.go b/internal/api/plan_display.go index 4b3638cd2..72166fa53 100644 --- a/internal/api/plan_display.go +++ b/internal/api/plan_display.go @@ -34,22 +34,24 @@ func planDisplayFor(plan *config.PurchasePlan) planDisplay { } sort.Strings(keys) out := plan.Services[keys[0]] + out.Provider = config.ServiceEntryProvider(keys[0], out.Provider) seen := map[string]struct{}{} var providers []string for _, k := range keys { svc := plan.Services[k] - if _, dup := seen[svc.Provider]; !dup { - seen[svc.Provider] = struct{}{} - providers = append(providers, svc.Provider) + provider := config.ServiceEntryProvider(k, svc.Provider) + if _, dup := seen[provider]; !dup { + seen[provider] = struct{}{} + providers = append(providers, provider) } - out.Provider = sharedOrMultiple(out.Provider, svc.Provider) + out.Provider = sharedOrMultiple(out.Provider, provider) out.Service = sharedOrMultiple(out.Service, svc.Service) out.Payment = sharedOrMultiple(out.Payment, svc.Payment) if out.Term != svc.Term { out.Term = 0 } } - // Keys start with their provider, so sorted keys give sorted providers. + // The key prefix is the provider, so sorted keys give sorted providers. return planDisplay{svc: out, providers: providers} } diff --git a/internal/api/plan_display_bound_test.go b/internal/api/plan_display_bound_test.go index 7420267a7..f19b4db21 100644 --- a/internal/api/plan_display_bound_test.go +++ b/internal/api/plan_display_bound_test.go @@ -91,3 +91,20 @@ func TestUpcomingFilter_MatchesAnyBoundProvider(t *testing.T) { assert.True(t, rowHasProvider(&UpcomingPurchase{Provider: "aws"}, "aws")) assert.False(t, rowHasProvider(&UpcomingPurchase{Provider: "aws"}, "azure")) } + +func TestPlanDisplayFor_AllGroupsDisabledFallsBackToFirstService(t *testing.T) { + off := displayEntry("aws", "ec2", 1, "no-upfront") + off.Enabled = false + plan := displayPlan(true, map[string]config.ServiceConfig{"aws/ec2#g1": off}) + d := planDisplayFor(plan) + assert.Equal(t, firstServiceConfig(plan), d.svc) + assert.Equal(t, "ec2", d.svc.Service) + assert.Nil(t, d.providers) +} + +func TestPlanDisplayFor_KeyPrefixIsTheProvider(t *testing.T) { + stale := displayEntry("gcp", "ec2", 1, "no-upfront") + d := planDisplayFor(displayPlan(true, map[string]config.ServiceConfig{"aws/ec2#g1": stale})) + assert.Equal(t, "aws", d.svc.Provider) + assert.Equal(t, []string{"aws"}, d.providers) +} diff --git a/internal/config/purchase_plan_providers.go b/internal/config/purchase_plan_providers.go index 35882a919..10ca6b1de 100644 --- a/internal/config/purchase_plan_providers.go +++ b/internal/config/purchase_plan_providers.go @@ -27,7 +27,7 @@ func DerivePlanProviders(plan *PurchasePlan) ([]string, error) { } seen := make(map[string]struct{}, len(plan.Services)) for k := range plan.Services { - if provider := serviceEntryProvider(k, plan.Services[k].Provider); provider != "" { + if provider := ServiceEntryProvider(k, plan.Services[k].Provider); provider != "" { seen[provider] = struct{}{} } } @@ -42,9 +42,9 @@ func DerivePlanProviders(plan *PurchasePlan) ([]string, error) { return providers, nil } -// serviceEntryProvider returns the provider a services entry names: the key's +// ServiceEntryProvider returns the provider a services entry names: the key's // prefix when the key has one, else the entry's Provider field, else "". -func serviceEntryProvider(key, fieldProvider string) string { +func ServiceEntryProvider(key, fieldProvider string) string { if i := strings.IndexAny(key, "/:"); i > 0 { return key[:i] } diff --git a/internal/config/store_postgres_plan_writes.go b/internal/config/store_postgres_plan_writes.go index 8b68c5f41..73c29bfcc 100644 --- a/internal/config/store_postgres_plan_writes.go +++ b/internal/config/store_postgres_plan_writes.go @@ -85,7 +85,8 @@ func (s *PostgresStore) setPlanAccountsTx(ctx context.Context, tx pgx.Tx, planID // without the read-only service_configs stamps (issue #8). A plan's services // map is a copy of settings, not a service_configs row, so a created_at or // updated_at on an entry would be a caller-supplied value the database never -// wrote; it is dropped instead of stored. +// wrote; it is dropped instead of stored. BoundCount is response-only and +// dropped the same way. func marshalPlanServices(services map[string]ServiceConfig) ([]byte, error) { var stripped map[string]ServiceConfig if services != nil { @@ -94,6 +95,7 @@ func marshalPlanServices(services map[string]ServiceConfig) ([]byte, error) { svc := services[key] svc.CreatedAt = nil svc.UpdatedAt = nil + svc.BoundCount = 0 stripped[key] = svc } } diff --git a/internal/config/store_postgres_service_config_stamps_test.go b/internal/config/store_postgres_service_config_stamps_test.go index bea9d7de5..1977eb646 100644 --- a/internal/config/store_postgres_service_config_stamps_test.go +++ b/internal/config/store_postgres_service_config_stamps_test.go @@ -95,3 +95,16 @@ func TestPGXMock_UpdatePurchasePlan_StoresServicesWithoutStamps(t *testing.T) { assertNoStampKeys(t, servicesArg.raw) assert.NoError(t, mock.ExpectationsWereMet()) } + +// bound_count is response-only (issue #1746); a services map that carries it +// must not store it. +func TestMarshalPlanServices_DropsBoundCount(t *testing.T) { + svc := stampedService() + svc.BoundCount = 7 + services := map[string]ServiceConfig{"aws/ec2#g1": svc} + + raw, err := marshalPlanServices(services) + require.NoError(t, err) + assert.NotContains(t, string(raw), "bound_count") + assert.Equal(t, 7, services["aws/ec2#g1"].BoundCount, "the caller's map is not modified") +}