From b548ae524c0e8826e1bd506f329120421d4226c0 Mon Sep 17 00:00:00 2001 From: Astrid Gealer Date: Tue, 27 Oct 2026 23:24:10 +0000 Subject: [PATCH 1/2] feat(config): expose created_at/updated_at on service config rows Service config rows already carry created_at and updated_at columns, but no read selected them, so the API could not say when a Purchasing service config last changed (Stage 1 of the issue, backend half). - ServiceConfig gains read-only CreatedAt/UpdatedAt (*time.Time, omitted from JSON when nil). - GetServiceConfig, the FOR UPDATE read in UpdateServiceConfigAtomic and ListServiceConfigs select and scan both columns through one shared scanServiceConfig. - The service config upsert RETURNs both stamps and sets them on the saved row, so UpdateServiceConfigAtomic reports the version it wrote rather than the pre-write stamp its merge copied, and a caller's value never survives a save. - PUT /api/config/service/{provider}/{service} drops stamps from the body; purchase plan services JSONB is written without them on create and update. - openapi.yaml documents both fields as readOnly. Refs #8 Co-Authored-By: Claude Opus 5.5 --- internal/api/handler_config.go | 4 + internal/api/handler_config_stamps_test.go | 84 +++++++++ internal/api/openapi.yaml | 17 ++ internal/config/store_postgres.go | 100 +++++------ .../config/store_postgres_pgxmock_test.go | 45 +++-- internal/config/store_postgres_plan_writes.go | 27 ++- ...ore_postgres_service_config_atomic_test.go | 33 +++- ..._service_config_stamps_integration_test.go | 170 ++++++++++++++++++ ...ore_postgres_service_config_stamps_test.go | 97 ++++++++++ internal/config/types.go | 7 + 10 files changed, 498 insertions(+), 86 deletions(-) create mode 100644 internal/api/handler_config_stamps_test.go create mode 100644 internal/config/store_postgres_service_config_stamps_integration_test.go create mode 100644 internal/config/store_postgres_service_config_stamps_test.go diff --git a/internal/api/handler_config.go b/internal/api/handler_config.go index 83896a47c..f75e60374 100644 --- a/internal/api/handler_config.go +++ b/internal/api/handler_config.go @@ -406,6 +406,10 @@ func (h *Handler) updateServiceConfig(ctx context.Context, req *events.LambdaFun if err := json.Unmarshal([]byte(req.Body), &cfg); err != nil { return nil, NewClientError(400, "invalid request body") } + // created_at and updated_at are read-only row stamps (issue #8). The + // GET response carries them, so a client that echoes it back must not + // get them into the merged row; the store takes both from the database. + cfg.CreatedAt, cfg.UpdatedAt = nil, nil parts := strings.SplitN(service, "/", 2) if len(parts) == 2 { diff --git a/internal/api/handler_config_stamps_test.go b/internal/api/handler_config_stamps_test.go new file mode 100644 index 000000000..909b0c767 --- /dev/null +++ b/internal/api/handler_config_stamps_test.go @@ -0,0 +1,84 @@ +package api + +import ( + "context" + "encoding/json" + "testing" + "time" + + "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" +) + +// Issue #8: service config rows carry read-only created_at/updated_at stamps. + +// TestGetConfig_ServiceRowsCarryStamps: GET /api/config returns each service +// row's stamps as RFC 3339 strings, and omits them for a config that has none. +func TestGetConfig_ServiceRowsCarryStamps(t *testing.T) { + ctx := context.Background() + created := time.Date(2026, 4, 27, 12, 34, 0, 0, time.UTC) + updated := created.Add(26 * time.Hour) + + mockStore := new(MockConfigStore) + mockStore.On("GetGlobalConfig", mock.Anything).Return(&config.GlobalConfig{EnabledProviders: []string{"aws"}}, nil) + mockStore.On("ListServiceConfigs", mock.Anything).Return([]config.ServiceConfig{ + {Provider: "aws", Service: "ec2", CreatedAt: &created, UpdatedAt: &updated}, + {Provider: "aws", Service: "rds"}, + }, nil) + handler := &Handler{config: mockStore, apiKey: "test-key"} + handler.sourceIdentityOnce.Do(func() {}) // no STS lookup for the admin-only source identity + + resp, err := handler.HandleRequest(ctx, &events.LambdaFunctionURLRequest{ + Headers: map[string]string{"X-API-Key": "test-key"}, + RequestContext: events.LambdaFunctionURLRequestContext{ + HTTP: events.LambdaFunctionURLRequestContextHTTPDescription{Method: "GET", Path: "/api/config"}, + }, + }) + require.NoError(t, err) + require.Equal(t, 200, resp.StatusCode) + + var body struct { + Services []map[string]any `json:"services"` + } + require.NoError(t, json.Unmarshal([]byte(resp.Body), &body)) + require.Len(t, body.Services, 2) + assert.Equal(t, "2026-04-27T12:34:00Z", body.Services[0]["created_at"]) + assert.Equal(t, "2026-04-28T14:34:00Z", body.Services[0]["updated_at"]) + assert.NotContains(t, body.Services[1], "created_at") + assert.NotContains(t, body.Services[1], "updated_at") +} + +// TestUpdateServiceConfig_IgnoresClientStamps: a PUT body that echoes the +// GET response's stamps (or forges them) must not reach the row the store +// writes, even for a first save where there is no stored row to merge onto. +func TestUpdateServiceConfig_IgnoresClientStamps(t *testing.T) { + ctx := context.Background() + mockAuth := new(MockAuthService) + mockAuth.On("ValidateSession", ctx, "admin-token").Return(&Session{ + UserID: "aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa", Email: "admin@example.com", + }, nil) + mockAuth.grantAdmin() + + mockStore := new(MockConfigStore) + mockStore.On("GetServiceConfig", ctx, "aws", "rds").Return(nil, nil) + var saved *config.ServiceConfig + mockStore.On("SaveServiceConfig", ctx, mock.AnythingOfType("*config.ServiceConfig")). + Run(func(args mock.Arguments) { saved = args.Get(1).(*config.ServiceConfig) }). + Return(nil) + handler := &Handler{config: mockStore, auth: mockAuth} + + result, err := handler.updateServiceConfig(ctx, &events.LambdaFunctionURLRequest{ + Headers: map[string]string{"Authorization": "Bearer admin-token"}, + Body: `{"enabled": true, "term": 3, "payment": "all-upfront", "coverage": 80, + "created_at": "1999-01-01T00:00:00Z", "updated_at": "1999-01-01T00:00:00Z"}`, + }, "aws/rds") + require.NoError(t, err) + assert.Equal(t, "updated", result.Status) + require.NotNil(t, saved) + assert.Nil(t, saved.CreatedAt, "a client-supplied created_at must not reach the store") + assert.Nil(t, saved.UpdatedAt, "a client-supplied updated_at must not reach the store") + assert.Equal(t, 80.0, saved.Coverage, "the editable fields still apply") +} diff --git a/internal/api/openapi.yaml b/internal/api/openapi.yaml index a7782101b..b260566ef 100644 --- a/internal/api/openapi.yaml +++ b/internal/api/openapi.yaml @@ -6624,6 +6624,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. + created_at: + type: string + format: date-time + readOnly: true + description: | + When this service config row was first saved. Present on stored + rows (GET /api/config, GET /api/config/service/{provider}/{service}); + omitted from a purchase plan's services entries. Ignored on write. + updated_at: + type: string + format: date-time + readOnly: true + description: | + When this service config row last changed, set by the database on + every save of the row, including a changed global default being + copied onto it. Present and omitted exactly like created_at; + ignored on write. CommitmentOptionsResponse: type: object diff --git a/internal/config/store_postgres.go b/internal/config/store_postgres.go index c4d9767b6..f2590141c 100644 --- a/internal/config/store_postgres.go +++ b/internal/config/store_postgres.go @@ -403,7 +403,7 @@ func saveGlobalConfigWith(ctx context.Context, q globalConfigExecutor, config *G // in scanServiceConfig order. const serviceConfigColumns = `provider, service, enabled, term, payment, coverage, ramp_schedule, include_engines, exclude_engines, include_regions, exclude_regions, - include_types, exclude_types, min_count` + include_types, exclude_types, min_count, created_at, updated_at` const getServiceConfigQuery = `SELECT ` + serviceConfigColumns + ` FROM service_configs @@ -425,10 +425,22 @@ func (s *PostgresStore) GetServiceConfig(ctx context.Context, provider, service // locking clause) via q (the pool or a transaction). A missing row is reported // as an error wrapping ErrNotFound. func getServiceConfigFrom(ctx context.Context, q globalConfigExecutor, query, provider, service string) (*ServiceConfig, error) { - var config ServiceConfig - var includeEngines, excludeEngines, includeRegions, excludeRegions, includeTypes, excludeTypes []string + config, err := scanServiceConfig(q.QueryRow(ctx, query, provider, service)) + if err != nil { + if errors.Is(err, pgx.ErrNoRows) { + return nil, fmt.Errorf("service config not found for %s:%s: %w", provider, service, ErrNotFound) + } + return nil, fmt.Errorf("failed to get service config: %w", err) + } + return config, nil +} - err := q.QueryRow(ctx, query, provider, service).Scan( +// scanServiceConfig scans one row selected with serviceConfigColumns, +// including the row's created_at and updated_at stamps (issue #8). +func scanServiceConfig(row pgx.Row) (*ServiceConfig, error) { + var config ServiceConfig + var createdAt, updatedAt time.Time + err := row.Scan( &config.Provider, &config.Service, &config.Enabled, @@ -436,30 +448,21 @@ func getServiceConfigFrom(ctx context.Context, q globalConfigExecutor, query, pr &config.Payment, &config.Coverage, &config.RampSchedule, - &includeEngines, - &excludeEngines, - &includeRegions, - &excludeRegions, - &includeTypes, - &excludeTypes, + &config.IncludeEngines, + &config.ExcludeEngines, + &config.IncludeRegions, + &config.ExcludeRegions, + &config.IncludeTypes, + &config.ExcludeTypes, &config.MinCount, + &createdAt, + &updatedAt, ) - if err != nil { - if errors.Is(err, pgx.ErrNoRows) { - return nil, fmt.Errorf("service config not found for %s:%s: %w", provider, service, ErrNotFound) - } - return nil, fmt.Errorf("failed to get service config: %w", err) + return nil, err } - - // Map arrays (handle nil) - config.IncludeEngines = includeEngines - config.ExcludeEngines = excludeEngines - config.IncludeRegions = includeRegions - config.ExcludeRegions = excludeRegions - config.IncludeTypes = includeTypes - config.ExcludeTypes = excludeTypes - + config.CreatedAt = &createdAt + config.UpdatedAt = &updatedAt return &config, nil } @@ -538,9 +541,11 @@ func saveServiceConfigWith(ctx context.Context, q globalConfigExecutor, config * exclude_types = $13, min_count = $14, updated_at = NOW() + RETURNING created_at, updated_at ` - _, err := q.Exec(ctx, query, + var createdAt, updatedAt time.Time + err := q.QueryRow(ctx, query, config.Provider, config.Service, config.Enabled, @@ -555,12 +560,16 @@ func saveServiceConfigWith(ctx context.Context, q globalConfigExecutor, config * config.IncludeTypes, config.ExcludeTypes, config.MinCount, - ) + ).Scan(&createdAt, &updatedAt) if err != nil { return fmt.Errorf("failed to save service config: %w", err) } + // The stamps always come from the row as written, so a caller-supplied + // value (a PUT body carrying created_at) never survives the save. + config.CreatedAt = &createdAt + config.UpdatedAt = &updatedAt return nil } @@ -573,10 +582,7 @@ func saveServiceConfigWith(ctx context.Context, q globalConfigExecutor, config * // defense-in-depth against a compromised admin inserting millions of rows // and matches the sibling GetPendingExecutions limit. func (s *PostgresStore) ListServiceConfigs(ctx context.Context) ([]ServiceConfig, error) { - query := ` - SELECT provider, service, enabled, term, payment, coverage, ramp_schedule, - include_engines, exclude_engines, include_regions, exclude_regions, - include_types, exclude_types, min_count + query := `SELECT ` + serviceConfigColumns + ` FROM service_configs ORDER BY provider, service LIMIT 1000 @@ -590,37 +596,11 @@ func (s *PostgresStore) ListServiceConfigs(ctx context.Context) ([]ServiceConfig configs := make([]ServiceConfig, 0) for rows.Next() { - var config ServiceConfig - var includeEngines, excludeEngines, includeRegions, excludeRegions, includeTypes, excludeTypes []string - - err := rows.Scan( - &config.Provider, - &config.Service, - &config.Enabled, - &config.Term, - &config.Payment, - &config.Coverage, - &config.RampSchedule, - &includeEngines, - &excludeEngines, - &includeRegions, - &excludeRegions, - &includeTypes, - &excludeTypes, - &config.MinCount, - ) + config, err := scanServiceConfig(rows) if err != nil { return nil, fmt.Errorf("failed to scan service config: %w", err) } - - config.IncludeEngines = includeEngines - config.ExcludeEngines = excludeEngines - config.IncludeRegions = includeRegions - config.ExcludeRegions = excludeRegions - config.IncludeTypes = includeTypes - config.ExcludeTypes = excludeTypes - - configs = append(configs, config) + configs = append(configs, *config) } return configs, rows.Err() @@ -868,9 +848,9 @@ func (s *PostgresStore) updatePurchasePlanGuardedTx(ctx context.Context, tx pgx. return err } // Marshal services and ramp_schedule to JSONB - servicesJSON, err := json.Marshal(plan.Services) + servicesJSON, err := marshalPlanServices(plan.Services) if err != nil { - return fmt.Errorf("failed to marshal services: %w", err) + return err } rampScheduleJSON, err := json.Marshal(plan.RampSchedule) diff --git a/internal/config/store_postgres_pgxmock_test.go b/internal/config/store_postgres_pgxmock_test.go index 04bbefc12..05b280f8e 100644 --- a/internal/config/store_postgres_pgxmock_test.go +++ b/internal/config/store_postgres_pgxmock_test.go @@ -400,16 +400,15 @@ func TestPGXMock_GetServiceConfig_Success(t *testing.T) { store := storeWith(mock) ctx := context.Background() - cols := []string{ - "provider", "service", "enabled", "term", "payment", "coverage", "ramp_schedule", - "include_engines", "exclude_engines", "include_regions", "exclude_regions", - "include_types", "exclude_types", "min_count", - } - rows := pgxmock.NewRows(cols).AddRow( + created := time.Date(2026, 4, 27, 12, 34, 0, 0, time.UTC) + updated := created.Add(48 * time.Hour) + rows := pgxmock.NewRows(serviceConfigCols).AddRow( "aws", "ec2", true, 1, "no-upfront", 80.0, RampImmediate, []string{"mysql"}, []string{}, []string{"us-east-1"}, []string{}, []string{}, []string{}, 2, + created, updated, ) - mock.ExpectQuery("SELECT").WithArgs(pgxmock.AnyArg(), pgxmock.AnyArg()).WillReturnRows(rows) + mock.ExpectQuery(`SELECT .*created_at, updated_at\s+FROM service_configs`). + WithArgs(pgxmock.AnyArg(), pgxmock.AnyArg()).WillReturnRows(rows) cfg, err := store.GetServiceConfig(ctx, "aws", "ec2") require.NoError(t, err) @@ -417,6 +416,11 @@ func TestPGXMock_GetServiceConfig_Success(t *testing.T) { assert.Equal(t, "ec2", cfg.Service) assert.Equal(t, []string{"mysql"}, cfg.IncludeEngines) assert.Equal(t, 2, cfg.MinCount) + // Issue #8: the row's stamps reach the caller. + require.NotNil(t, cfg.CreatedAt) + require.NotNil(t, cfg.UpdatedAt) + assert.True(t, created.Equal(*cfg.CreatedAt)) + assert.True(t, updated.Equal(*cfg.UpdatedAt)) assert.NoError(t, mock.ExpectationsWereMet()) } @@ -427,21 +431,28 @@ func TestPGXMock_ListServiceConfigs_Success(t *testing.T) { store := storeWith(mock) ctx := context.Background() - cols := []string{ - "provider", "service", "enabled", "term", "payment", "coverage", "ramp_schedule", - "include_engines", "exclude_engines", "include_regions", "exclude_regions", - "include_types", "exclude_types", "min_count", - } - rows := pgxmock.NewRows(cols). + ec2Stamp := time.Date(2026, 4, 27, 12, 34, 0, 0, time.UTC) + rdsStamp := ec2Stamp.Add(time.Hour) + rows := pgxmock.NewRows(serviceConfigCols). AddRow("aws", "ec2", true, 1, "no-upfront", 80.0, RampImmediate, - []string{}, []string{}, []string{}, []string{}, []string{}, []string{}, 0). + []string{}, []string{}, []string{}, []string{}, []string{}, []string{}, 0, + ec2Stamp, ec2Stamp.Add(time.Minute)). AddRow("aws", "rds", true, 3, "all-upfront", 70.0, RampImmediate, - []string{}, []string{}, []string{}, []string{}, []string{}, []string{}, 0) - mock.ExpectQuery("SELECT").WillReturnRows(rows) + []string{}, []string{}, []string{}, []string{}, []string{}, []string{}, 0, + rdsStamp, rdsStamp.Add(time.Minute)) + mock.ExpectQuery(`SELECT .*created_at, updated_at\s+FROM service_configs`).WillReturnRows(rows) cfgs, err := store.ListServiceConfigs(ctx) require.NoError(t, err) - assert.Len(t, cfgs, 2) + require.Len(t, cfgs, 2) + // Issue #8: each row keeps its own stamps; scanning into a reused + // variable must not leave every entry pointing at the last row's time. + require.NotNil(t, cfgs[0].UpdatedAt) + require.NotNil(t, cfgs[1].UpdatedAt) + assert.True(t, ec2Stamp.Equal(*cfgs[0].CreatedAt)) + assert.True(t, ec2Stamp.Add(time.Minute).Equal(*cfgs[0].UpdatedAt)) + assert.True(t, rdsStamp.Equal(*cfgs[1].CreatedAt)) + assert.True(t, rdsStamp.Add(time.Minute).Equal(*cfgs[1].UpdatedAt)) assert.NoError(t, mock.ExpectationsWereMet()) } diff --git a/internal/config/store_postgres_plan_writes.go b/internal/config/store_postgres_plan_writes.go index e4d1a96f7..8b68c5f41 100644 --- a/internal/config/store_postgres_plan_writes.go +++ b/internal/config/store_postgres_plan_writes.go @@ -19,9 +19,9 @@ func (s *PostgresStore) createPurchasePlan(ctx context.Context, executor globalC plan.CreatedAt = now plan.UpdatedAt = now - servicesJSON, err := json.Marshal(plan.Services) + servicesJSON, err := marshalPlanServices(plan.Services) if err != nil { - return fmt.Errorf("failed to marshal services: %w", err) + return err } rampScheduleJSON, err := json.Marshal(plan.RampSchedule) @@ -80,3 +80,26 @@ func (s *PostgresStore) setPlanAccountsTx(ctx context.Context, tx pgx.Tx, planID } return nil } + +// marshalPlanServices encodes a plan's services map for the JSONB column +// 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. +func marshalPlanServices(services map[string]ServiceConfig) ([]byte, error) { + var stripped map[string]ServiceConfig + if services != nil { + stripped = make(map[string]ServiceConfig, len(services)) + for key := range services { + svc := services[key] + svc.CreatedAt = nil + svc.UpdatedAt = nil + stripped[key] = svc + } + } + servicesJSON, err := json.Marshal(stripped) + if err != nil { + return nil, fmt.Errorf("failed to marshal services: %w", err) + } + return servicesJSON, nil +} diff --git a/internal/config/store_postgres_service_config_atomic_test.go b/internal/config/store_postgres_service_config_atomic_test.go index 868d39f10..ec399ffd5 100644 --- a/internal/config/store_postgres_service_config_atomic_test.go +++ b/internal/config/store_postgres_service_config_atomic_test.go @@ -4,6 +4,7 @@ import ( "context" "errors" "testing" + "time" "github.com/pashagolub/pgxmock/v5" "github.com/stretchr/testify/assert" @@ -13,9 +14,12 @@ import ( var serviceConfigCols = []string{ "provider", "service", "enabled", "term", "payment", "coverage", "ramp_schedule", "include_engines", "exclude_engines", "include_regions", "exclude_regions", - "include_types", "exclude_types", "min_count", + "include_types", "exclude_types", "min_count", "created_at", "updated_at", } +// stampCols are the columns the service-config upsert RETURNs (issue #8). +var stampCols = []string{"created_at", "updated_at"} + // expectServiceConfigLockedRead queues the BEGIN, the per-service advisory // lock and the FOR UPDATE read UpdateServiceConfigAtomic issues, in order. func expectServiceConfigLockedRead(mock pgxmock.PgxPoolIface, rows *pgxmock.Rows) { @@ -34,14 +38,18 @@ func TestPGXMock_UpdateServiceConfigAtomic_LockedReadMergeWrite(t *testing.T) { mock := newMock(t) store := storeWith(mock) + created := time.Date(2026, 4, 1, 0, 0, 0, 0, time.UTC) + readStamp := created.Add(time.Hour) + writeStamp := created.Add(2 * time.Hour) expectServiceConfigLockedRead(mock, pgxmock.NewRows(serviceConfigCols).AddRow( "aws", "rds", true, 3, "all-upfront", 50.0, RampImmediate, []string{"mysql"}, []string{}, []string{}, []string{}, []string{}, []string{}, 4, + created, readStamp, )) - mock.ExpectExec("INSERT INTO service_configs"). + mock.ExpectQuery(`INSERT INTO service_configs[\s\S]*RETURNING created_at, updated_at`). WithArgs("aws", "rds", true, 3, "all-upfront", 80.0, RampImmediate, []string{"mysql"}, []string{}, []string{}, []string{}, []string{}, []string{}, 4). - WillReturnResult(pgxmock.NewResult("INSERT", 1)) + WillReturnRows(pgxmock.NewRows(stampCols).AddRow(created, writeStamp)) mock.ExpectCommit() saved, err := store.UpdateServiceConfigAtomic(context.Background(), "aws", "rds", func(existing *ServiceConfig) (*ServiceConfig, error) { @@ -53,6 +61,11 @@ func TestPGXMock_UpdateServiceConfigAtomic_LockedReadMergeWrite(t *testing.T) { require.NoError(t, err) assert.Equal(t, 80.0, saved.Coverage) assert.Equal(t, 4, saved.MinCount) + // Issue #8: the returned row carries the stamps the write produced, not + // the pre-write updated_at the merge copied from the locked read. + require.NotNil(t, saved.UpdatedAt) + assert.True(t, created.Equal(*saved.CreatedAt)) + assert.True(t, writeStamp.Equal(*saved.UpdatedAt), "got %v", *saved.UpdatedAt) assert.NoError(t, mock.ExpectationsWereMet()) } @@ -62,18 +75,24 @@ func TestPGXMock_UpdateServiceConfigAtomic_NoRowPassesNil(t *testing.T) { mock := newMock(t) store := storeWith(mock) + inserted := time.Date(2026, 4, 1, 0, 0, 0, 0, time.UTC) expectServiceConfigLockedRead(mock, pgxmock.NewRows(serviceConfigCols)) - mock.ExpectExec("INSERT INTO service_configs").WithArgs(anyArgsCfg(14)...). - WillReturnResult(pgxmock.NewResult("INSERT", 1)) + mock.ExpectQuery("INSERT INTO service_configs").WithArgs(anyArgsCfg(14)...). + WillReturnRows(pgxmock.NewRows(stampCols).AddRow(inserted, inserted)) mock.ExpectCommit() sawNil := false - _, err := store.UpdateServiceConfigAtomic(context.Background(), "aws", "rds", func(existing *ServiceConfig) (*ServiceConfig, error) { + forged := time.Date(1999, 1, 1, 0, 0, 0, 0, time.UTC) + saved, err := store.UpdateServiceConfigAtomic(context.Background(), "aws", "rds", func(existing *ServiceConfig) (*ServiceConfig, error) { sawNil = existing == nil - return &ServiceConfig{Provider: "aws", Service: "rds", Term: 1, Payment: "no-upfront"}, nil + // A caller-supplied stamp must be replaced by the database's. + return &ServiceConfig{Provider: "aws", Service: "rds", Term: 1, Payment: "no-upfront", CreatedAt: &forged, UpdatedAt: &forged}, nil }) require.NoError(t, err) assert.True(t, sawNil) + require.NotNil(t, saved.CreatedAt) + assert.True(t, inserted.Equal(*saved.CreatedAt)) + assert.True(t, inserted.Equal(*saved.UpdatedAt)) assert.NoError(t, mock.ExpectationsWereMet()) } diff --git a/internal/config/store_postgres_service_config_stamps_integration_test.go b/internal/config/store_postgres_service_config_stamps_integration_test.go new file mode 100644 index 000000000..95d50f081 --- /dev/null +++ b/internal/config/store_postgres_service_config_stamps_integration_test.go @@ -0,0 +1,170 @@ +//go:build integration + +package config + +import ( + "context" + "sync" + "testing" + "time" + + "github.com/LeanerCloud/cloud-commitments-platform/internal/database/postgres/testhelpers" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// Issue #8: service_configs rows expose their created_at/updated_at stamps, +// read-only. These run against real Postgres so the trigger, the upsert's +// RETURNING clause and the column precision are the real ones. + +func newStampsStore(ctx context.Context, t *testing.T) *PostgresStore { + t.Helper() + return NewPostgresStore(testhelpers.RequireMigratedDatabase(ctx, t, getTestMigrationsPath()).DB) +} + +// TestServiceConfigStamps_ReadsAndSavesAgree: every read returns the row's +// stamps, the save reports exactly the stamps it wrote (not the caller's), a +// later save advances updated_at and leaves created_at alone. +func TestServiceConfigStamps_ReadsAndSavesAgree(t *testing.T) { + ctx := context.Background() + store := newStampsStore(ctx, t) + + forged := time.Date(1999, 1, 1, 0, 0, 0, 0, time.UTC) + first := &ServiceConfig{ + Provider: "aws", Service: "rds", Enabled: true, Term: 1, Payment: "no-upfront", Coverage: 50, + RampSchedule: RampImmediate, CreatedAt: &forged, UpdatedAt: &forged, + } + require.NoError(t, store.SaveServiceConfig(ctx, first)) + require.NotNil(t, first.CreatedAt) + require.NotNil(t, first.UpdatedAt) + assert.False(t, first.CreatedAt.Equal(forged), "created_at comes from the database, not the caller") + assert.False(t, first.UpdatedAt.Equal(forged), "updated_at comes from the database, not the caller") + + got, err := store.GetServiceConfig(ctx, "aws", "rds") + require.NoError(t, err) + require.NotNil(t, got.CreatedAt) + require.NotNil(t, got.UpdatedAt) + assert.True(t, first.CreatedAt.Equal(*got.CreatedAt), "save %v, get %v", *first.CreatedAt, *got.CreatedAt) + assert.True(t, first.UpdatedAt.Equal(*got.UpdatedAt), "save %v, get %v", *first.UpdatedAt, *got.UpdatedAt) + + // NOW() is the transaction start, so the second save needs a later + // transaction to get a later stamp. + time.Sleep(5 * time.Millisecond) + second := *got + second.Coverage = 70 + require.NoError(t, store.SaveServiceConfig(ctx, &second)) + assert.True(t, first.CreatedAt.Equal(*second.CreatedAt), "an update keeps created_at") + assert.True(t, second.UpdatedAt.After(*first.UpdatedAt), "an update advances updated_at: %v -> %v", *first.UpdatedAt, *second.UpdatedAt) + + list, err := store.ListServiceConfigs(ctx) + require.NoError(t, err) + var listed *ServiceConfig + for i := range list { + if list[i].Provider == "aws" && list[i].Service == "rds" { + listed = &list[i] + } + } + require.NotNil(t, listed, "aws/rds must be listed") + require.NotNil(t, listed.UpdatedAt) + assert.True(t, second.CreatedAt.Equal(*listed.CreatedAt)) + assert.True(t, second.UpdatedAt.Equal(*listed.UpdatedAt)) + for _, cfg := range list { + assert.NotNil(t, cfg.CreatedAt, "%s/%s has created_at", cfg.Provider, cfg.Service) + assert.NotNil(t, cfg.UpdatedAt, "%s/%s has updated_at", cfg.Provider, cfg.Service) + } +} + +// TestUpdateServiceConfigAtomic_ConcurrentSavesReportTheirOwnStamps fires +// concurrent atomic updates of one row. Each must report the stamps of the +// row version it wrote, so the row left in the table carries exactly the +// updated_at its writer was told, and created_at never moves. +func TestUpdateServiceConfigAtomic_ConcurrentSavesReportTheirOwnStamps(t *testing.T) { + ctx := context.Background() + store := newStampsStore(ctx, t) + seed := &ServiceConfig{Provider: "aws", Service: "rds", Enabled: true, Term: 1, Payment: "no-upfront", Coverage: 10, RampSchedule: RampImmediate} + require.NoError(t, store.SaveServiceConfig(ctx, seed)) + + const writers = 8 + type outcome struct { + coverage float64 + saved *ServiceConfig + err error + } + results := make([]outcome, writers) + var wg sync.WaitGroup + start := make(chan struct{}) + for i := 0; i < writers; i++ { + wg.Add(1) + go func(i int) { + defer wg.Done() + <-start + coverage := float64(20 + i) + saved, err := store.UpdateServiceConfigAtomic(ctx, "aws", "rds", func(existing *ServiceConfig) (*ServiceConfig, error) { + next := *existing + next.Coverage = coverage + return &next, nil + }) + results[i] = outcome{coverage: coverage, saved: saved, err: err} + }(i) + } + close(start) + wg.Wait() + + final, err := store.GetServiceConfig(ctx, "aws", "rds") + require.NoError(t, err) + matched := false + for _, r := range results { + require.NoError(t, r.err) + require.NotNil(t, r.saved.CreatedAt) + require.NotNil(t, r.saved.UpdatedAt) + assert.True(t, seed.CreatedAt.Equal(*r.saved.CreatedAt), "created_at must not move under concurrent updates") + assert.False(t, r.saved.UpdatedAt.Before(*seed.UpdatedAt), "an update's stamp is not older than the seed's") + if r.coverage == final.Coverage { + matched = true + assert.True(t, r.saved.UpdatedAt.Equal(*final.UpdatedAt), + "the last writer was told %v but the row says %v", *r.saved.UpdatedAt, *final.UpdatedAt) + } + } + require.True(t, matched, "the final coverage %v must be one writer's", final.Coverage) + assert.True(t, seed.CreatedAt.Equal(*final.CreatedAt)) +} + +// TestPurchasePlanServices_StampsNeverStored: stamps on a plan's services map +// entries are dropped before the JSONB write, on create and on update. +func TestPurchasePlanServices_StampsNeverStored(t *testing.T) { + ctx := context.Background() + store := newStampsStore(ctx, t) + + forged := time.Date(1999, 1, 1, 0, 0, 0, 0, time.UTC) + svc := ServiceConfig{ + Provider: "aws", Service: "ec2", Enabled: true, Term: 1, Payment: "no-upfront", Coverage: 80, + CreatedAt: &forged, UpdatedAt: &forged, + } + plan := &PurchasePlan{ + Name: "stamps", + Services: map[string]ServiceConfig{"aws/ec2": svc}, + RampSchedule: PresetRampSchedules["immediate"], + } + require.NoError(t, store.CreatePurchasePlan(ctx, plan)) + + rawServices := func() string { + var raw string + require.NoError(t, store.db.QueryRow(ctx, `SELECT services::text FROM purchase_plans WHERE id = $1`, plan.ID).Scan(&raw)) + return raw + } + created := rawServices() + assert.Contains(t, created, `"payment": "no-upfront"`) + assert.NotContains(t, created, "created_at") + assert.NotContains(t, created, "updated_at") + + stored, err := store.GetPurchasePlan(ctx, plan.ID) + require.NoError(t, err) + assert.Nil(t, stored.Services["aws/ec2"].CreatedAt) + assert.Nil(t, stored.Services["aws/ec2"].UpdatedAt) + + stored.Services["aws/ec2"] = svc + require.NoError(t, store.UpdatePurchasePlan(ctx, stored)) + updated := rawServices() + assert.NotContains(t, updated, "created_at") + assert.NotContains(t, updated, "updated_at") +} diff --git a/internal/config/store_postgres_service_config_stamps_test.go b/internal/config/store_postgres_service_config_stamps_test.go new file mode 100644 index 000000000..bea9d7de5 --- /dev/null +++ b/internal/config/store_postgres_service_config_stamps_test.go @@ -0,0 +1,97 @@ +package config + +import ( + "context" + "encoding/json" + "testing" + "time" + + "github.com/pashagolub/pgxmock/v5" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// Issue #8: created_at and updated_at on a ServiceConfig are the +// service_configs row's own stamps. A purchase plan's services map is not a +// service_configs row, so its JSONB column must never store them. + +// capturedJSONArg matches any []byte argument and keeps it for inspection. +type capturedJSONArg struct{ raw []byte } + +func (c *capturedJSONArg) Match(v interface{}) bool { + b, ok := v.([]byte) + if ok { + c.raw = b + } + return ok +} + +func stampedService() ServiceConfig { + forged := time.Date(1999, 1, 1, 0, 0, 0, 0, time.UTC) + return ServiceConfig{ + Provider: "aws", Service: "ec2", Enabled: true, Term: 1, Payment: "no-upfront", Coverage: 80, + CreatedAt: &forged, UpdatedAt: &forged, + } +} + +func assertNoStampKeys(t *testing.T, raw []byte) { + t.Helper() + var decoded map[string]map[string]any + require.NoError(t, json.Unmarshal(raw, &decoded)) + require.NotEmpty(t, decoded) + for key, svc := range decoded { + assert.NotContains(t, svc, "created_at", "plan service %s", key) + assert.NotContains(t, svc, "updated_at", "plan service %s", key) + assert.Equal(t, "no-upfront", svc["payment"], "the other fields are kept") + } +} + +func TestMarshalPlanServices_DropsStampsWithoutMutatingInput(t *testing.T) { + services := map[string]ServiceConfig{"aws/ec2": stampedService()} + + raw, err := marshalPlanServices(services) + require.NoError(t, err) + assertNoStampKeys(t, raw) + assert.NotNil(t, services["aws/ec2"].CreatedAt, "the caller's map is not modified") + + nilRaw, err := marshalPlanServices(nil) + require.NoError(t, err) + assert.Equal(t, "null", string(nilRaw), "a nil map still encodes as before") +} + +func TestPGXMock_CreatePurchasePlan_StoresServicesWithoutStamps(t *testing.T) { + mock := newMock(t) + store := storeWith(mock) + + servicesArg := &capturedJSONArg{} + args := anyArgsCfg(13) + args[5] = servicesArg + mock.ExpectQuery("INSERT INTO purchase_plans").WithArgs(args...). + WillReturnRows(pgxmock.NewRows([]string{"updated_at"}).AddRow(time.Now())) + + plan := &PurchasePlan{ID: "plan-id", Name: "p", Services: map[string]ServiceConfig{"aws/ec2": stampedService()}} + require.NoError(t, store.CreatePurchasePlan(context.Background(), plan)) + assertNoStampKeys(t, servicesArg.raw) + assert.NoError(t, mock.ExpectationsWereMet()) +} + +func TestPGXMock_UpdatePurchasePlan_StoresServicesWithoutStamps(t *testing.T) { + mock := newMock(t) + store := storeWith(mock) + + expectLockedUnmanagedPlan(mock) + servicesArg := &capturedJSONArg{} + args := anyArgsCfg(12) + args[5] = servicesArg + mock.ExpectQuery("UPDATE purchase_plans").WithArgs(args...). + WillReturnRows(pgxmock.NewRows([]string{"updated_at"}).AddRow(time.Now())) + // The services set changed, so the stale ramp baseline is cleared. + mock.ExpectQuery("SET ramp_baseline = NULL").WithArgs("plan-id"). + WillReturnRows(pgxmock.NewRows([]string{"updated_at"})) + mock.ExpectCommit() + + plan := &PurchasePlan{ID: "plan-id", Name: "My Plan", Services: map[string]ServiceConfig{"aws/ec2": stampedService()}} + require.NoError(t, store.UpdatePurchasePlan(context.Background(), plan)) + assertNoStampKeys(t, servicesArg.raw) + assert.NoError(t, mock.ExpectationsWereMet()) +} diff --git a/internal/config/types.go b/internal/config/types.go index 7041f89be..270e4c5b0 100644 --- a/internal/config/types.go +++ b/internal/config/types.go @@ -226,6 +226,13 @@ type ServiceConfig struct { // RecommendationRecord.Count. 0 (the default) disables the filter, // matching the CLI flag's 0-no-floor semantics. MinCount int `json:"min_count,omitempty" dynamodbav:"min_count,omitempty"` + // CreatedAt and UpdatedAt are the service_configs row's own timestamps + // (issue #8), read-only. The store fills them on every read and from the + // upsert's RETURNING clause; no write path stores a caller's value. Nil + // (omitted from JSON) on a config that is not a stored row, such as an + // entry of a purchase plan's services map. + CreatedAt *time.Time `json:"created_at,omitempty" dynamodbav:"created_at,omitempty"` + UpdatedAt *time.Time `json:"updated_at,omitempty" dynamodbav:"updated_at,omitempty"` } // PurchasePlan represents a saved purchase plan for automated execution. From ad99548533194f0939712c7d7bc0a8c439951f8f Mon Sep 17 00:00:00 2001 From: Astrid Gealer Date: Wed, 28 Oct 2026 00:19:39 +0000 Subject: [PATCH 2/2] fix(deps): bump golang.org/x/net to v0.62.0 (GO-2026-6860, GO-2026-6866) govulncheck in the Docker image build now fails on these two advisories, both fixed in x/net v0.62.0. Co-Authored-By: Claude Opus 5.5 --- go.mod | 2 +- go.sum | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/go.mod b/go.mod index 5fb3a2f61..abcf4fb6c 100644 --- a/go.mod +++ b/go.mod @@ -59,7 +59,7 @@ require ( go.opentelemetry.io/otel/metric v1.45.0 // indirect go.opentelemetry.io/otel/trace v1.45.0 // indirect golang.org/x/crypto v0.58.0 - golang.org/x/net v0.61.0 // indirect + golang.org/x/net v0.62.0 // indirect golang.org/x/oauth2 v0.37.0 golang.org/x/sync v0.24.0 golang.org/x/sys v0.49.0 // indirect diff --git a/go.sum b/go.sum index 06deac0b9..83df52e66 100644 --- a/go.sum +++ b/go.sum @@ -367,8 +367,8 @@ golang.org/x/crypto v0.58.0 h1:COkYLr4k7nDI4r6QtMML2AfzeIswWHAJthLS6K/M/54= golang.org/x/crypto v0.58.0/go.mod h1:Xh+kl5A+M0gMiWDytH2Hhr5Z7WbNCpv/eB/EFgfSk+w= golang.org/x/mod v0.42.0 h1:ICeL3t2L30ljSPHHxDnR4fqVcBt/zR2zJAb9juavmQg= golang.org/x/mod v0.42.0/go.mod h1:u5fzIrRInkL8MaO1cbRkUHT64v303YIT5KEqpNr7jRs= -golang.org/x/net v0.61.0 h1:RR+6j/BTBCrggOLugxa+66R8FvoN6MHlzDlZIbN9YFw= -golang.org/x/net v0.61.0/go.mod h1:WgDCOTH8iwtB66T4/GfI94AHo/hhdb7Hah9NDseQfiI= +golang.org/x/net v0.62.0 h1:18ELalfM5+XNRwzM2nYqXsZQ9ByS1uhj2O/U7+5jqRY= +golang.org/x/net v0.62.0/go.mod h1:WgDCOTH8iwtB66T4/GfI94AHo/hhdb7Hah9NDseQfiI= golang.org/x/oauth2 v0.37.0 h1:JUlcxA8oAtauLfiH8FX2/FkAWHAdi0QtGCGc+hofE98= golang.org/x/oauth2 v0.37.0/go.mod h1:IxwZNxUULJmpBFf9K/9NTMSIfZZuvuTy1gGxhigP/58= golang.org/x/sync v0.24.0 h1:iqiDrGoDgyuvAKrhLQthUEzU/R378N78gXbBjxed7oA=