Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -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=
Expand Down
4 changes: 4 additions & 0 deletions internal/api/handler_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
84 changes: 84 additions & 0 deletions internal/api/handler_config_stamps_test.go
Original file line number Diff line number Diff line change
@@ -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")
}
17 changes: 17 additions & 0 deletions internal/api/openapi.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
100 changes: 40 additions & 60 deletions internal/config/store_postgres.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -425,41 +425,44 @@ 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,
&config.Term,
&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
}

Expand Down Expand Up @@ -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,
Expand All @@ -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
}

Expand All @@ -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
Expand All @@ -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()
Expand Down Expand Up @@ -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)
Expand Down
45 changes: 28 additions & 17 deletions internal/config/store_postgres_pgxmock_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -400,23 +400,27 @@ 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)
assert.Equal(t, "aws", cfg.Provider)
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())
}

Expand All @@ -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())
}

Expand Down
Loading
Loading