diff --git a/frontend/src/__tests__/settings.test.ts b/frontend/src/__tests__/settings.test.ts
index dc07fb43..0ea19189 100644
--- a/frontend/src/__tests__/settings.test.ts
+++ b/frontend/src/__tests__/settings.test.ts
@@ -75,6 +75,12 @@ describe('Settings Module', () => {
+ 7 days
+ 30 days
+ 60 days
+
1 3 1 3 Global Configuration
+
+ Recommendations Cache Stale Threshold (hours)
+ ⓘAge (in hours) at which the recommendations cache is considered stale and a background refresh fires automatically. Set to 0 to disable automatic background refresh (the cron scheduler and manual Refresh button still work). Valid range: 0–8760 (up to one year). Default: 24.
+
+
+
+
@@ -362,6 +371,24 @@
Purchasing Settings
+
+ Recommendations Lookback
+ Controls how far back AWS Cost Explorer looks when computing fresh recommendations. Must be 7, 30, or 60 days (AWS Cost Explorer API constraint). GCP CUD Recommender has no equivalent parameter and always uses its own internal window.
+
+
+ AWS lookback periodⓘAWS Cost Explorer LookbackPeriodInDays. A longer window smooths out utilisation spikes but may under-represent recent workload changes. Default: 7 days.
+
+
+
+ 7 days
+ 30 days
+ 60 days
+
+
+
+
Post-Purchase Grace Period
After a purchase is submitted, CUDly hides the just-covered capacity from the Recommendations view for this many days so you don't re-buy the same capacity while cloud-provider utilisation metrics catch up. Set to 0 to disable the feature for a provider. Range: 0–30 days.
diff --git a/frontend/src/settings.ts b/frontend/src/settings.ts
index cd912c41..b11bf242 100644
--- a/frontend/src/settings.ts
+++ b/frontend/src/settings.ts
@@ -114,6 +114,8 @@ const TRACKED_FIELDS = [
'setting-default-term', 'setting-default-payment', 'setting-default-coverage',
// Per-provider grace-period inputs
'setting-grace-aws', 'setting-grace-azure', 'setting-grace-gcp',
+ // Recommendations cycle params
+ 'setting-recs-stale-hours', 'setting-recs-lookback-days',
// Per-service fields
...SERVICE_FIELDS.map(f => f.termId),
...SERVICE_FIELDS.filter(f => f.paymentId !== null).map(f => f.paymentId as string),
@@ -2120,6 +2122,16 @@ export async function loadGlobalSettings(): Promise {
populateGraceInput('setting-grace-azure', gpMap['azure']);
populateGraceInput('setting-grace-gcp', gpMap['gcp']);
+ // Recommendations cycle params
+ const staleHoursInput = byId('setting-recs-stale-hours');
+ if (staleHoursInput) {
+ staleHoursInput.value = String(data.global.recommendations_cache_stale_hours ?? 24);
+ }
+ const lookbackSelect = byId('setting-recs-lookback-days');
+ if (lookbackSelect) {
+ lookbackSelect.value = String(data.global.recommendations_lookback_days ?? 7);
+ }
+
// Update visibility based on loaded settings
updateProviderSettingsVisibility();
updateCollectionScheduleVisibility();
@@ -2288,6 +2300,17 @@ export async function saveGlobalSettings(e: Event): Promise {
gracePeriodDays[provider] = v.value;
}
+ // Validate recommendations_cache_stale_hours before building the save payload.
+ // Use Number() so fractional input like "1.5" fails Number.isInteger instead of silently
+ // truncating to 1 as parseInt would.
+ const rawStaleHours = Number(byId('setting-recs-stale-hours')?.value ?? '24');
+ if (!Number.isFinite(rawStaleHours) || !Number.isInteger(rawStaleHours) || rawStaleHours < 0 || rawStaleHours > 8760) {
+ showToast({ message: 'Cache stale threshold must be a whole number between 0 and 8760 hours (0 = disable)', kind: 'error' });
+ if (saveBtn) saveBtn.disabled = false;
+ saveInFlight = false;
+ return;
+ }
+
const settings: api.Config = {
enabled_providers: enabledProviders,
notification_email: byId('setting-notification-email')?.value || '',
@@ -2298,6 +2321,8 @@ export async function saveGlobalSettings(e: Event): Promise {
default_coverage: parseInt(byId('setting-default-coverage')?.value || '80', 10),
notification_days_before: parseInt(byId('setting-notification-days')?.value || '3', 10),
grace_period_days: gracePeriodDays,
+ recommendations_cache_stale_hours: rawStaleHours,
+ recommendations_lookback_days: parseInt(byId('setting-recs-lookback-days')?.value || '7', 10),
};
try {
@@ -2379,6 +2404,11 @@ export async function resetSettings(): Promise {
populateGraceInput('setting-grace-aws', 7);
populateGraceInput('setting-grace-azure', 7);
populateGraceInput('setting-grace-gcp', 7);
+
+ const staleHoursInput = byId('setting-recs-stale-hours');
+ if (staleHoursInput) staleHoursInput.value = '24';
+ const lookbackSelect = byId('setting-recs-lookback-days');
+ if (lookbackSelect) lookbackSelect.value = '7';
}
/**
diff --git a/frontend/src/types.ts b/frontend/src/types.ts
index 55b00dfe..f02aa25f 100644
--- a/frontend/src/types.ts
+++ b/frontend/src/types.ts
@@ -298,6 +298,12 @@ export interface GlobalConfig {
// suppression feature. Keys: 'aws' / 'azure' / 'gcp'. Missing keys
// fall back to the backend default (7). Explicit 0 = disabled.
grace_period_days?: Record;
+ // Age (hours) after which the recommendations cache triggers a background
+ // stale-while-revalidate refresh. 0 disables automatic background refresh.
+ // Valid range: 0–8760. Default: 24.
+ recommendations_cache_stale_hours?: number;
+ // AWS Cost Explorer lookback window (days). One of 7, 30, or 60. Default: 7.
+ recommendations_lookback_days?: number;
}
// API Keys types
diff --git a/internal/api/handler_config.go b/internal/api/handler_config.go
index 2a184e9a..f4769dc6 100644
--- a/internal/api/handler_config.go
+++ b/internal/api/handler_config.go
@@ -42,6 +42,39 @@ func (h *Handler) getConfig(ctx context.Context) (*ConfigResponse, error) {
}, nil
}
+// preserveOmittedRecommendationFields merges persisted GlobalConfig values
+// for the two cycle-parameter fields when the request body omits them.
+// Without this merge, a partial PUT would silently zero out
+// RecommendationsCacheStaleHours / RecommendationsLookbackDays, which both
+// have meaningful 0-vs-omitted semantics that json.Unmarshal can't represent
+// directly. Errors from GetGlobalConfig fall through: the request body's
+// zero values then flow into Validate() which rejects out-of-range
+// lookback days, matching the pre-fix behaviour. Extracted from
+// updateConfig to keep that function under the cyclomatic-complexity gate
+// after the merge logic was added (PR #308 CodeRabbit pass-2 review).
+func (h *Handler) preserveOmittedRecommendationFields(ctx context.Context, cfg *config.GlobalConfig, body string) error {
+ var present map[string]json.RawMessage
+ if err := json.Unmarshal([]byte(body), &present); err != nil {
+ return NewClientError(400, "invalid request body")
+ }
+ _, hasStale := present["recommendations_cache_stale_hours"]
+ _, hasLookback := present["recommendations_lookback_days"]
+ if hasStale && hasLookback {
+ return nil
+ }
+ existing, gcErr := h.config.GetGlobalConfig(ctx)
+ if gcErr != nil || existing == nil {
+ return nil
+ }
+ if !hasStale {
+ cfg.RecommendationsCacheStaleHours = existing.RecommendationsCacheStaleHours
+ }
+ if !hasLookback {
+ cfg.RecommendationsLookbackDays = existing.RecommendationsLookbackDays
+ }
+ return nil
+}
+
func (h *Handler) updateConfig(ctx context.Context, req *events.LambdaFunctionURLRequest) (*StatusResponse, error) {
// Require update:config permission
if _, err := h.requirePermission(ctx, req, "update", "config"); err != nil {
@@ -53,6 +86,10 @@ func (h *Handler) updateConfig(ctx context.Context, req *events.LambdaFunctionUR
return nil, NewClientError(400, "invalid request body")
}
+ if err := h.preserveOmittedRecommendationFields(ctx, &cfg, req.Body); err != nil {
+ return nil, err
+ }
+
// Validate the configuration
if err := cfg.Validate(); err != nil {
return nil, NewClientError(400, fmt.Sprintf("validation error: %s", err))
diff --git a/internal/api/handler_config_test.go b/internal/api/handler_config_test.go
index feb8cf55..c8ccfe7d 100644
--- a/internal/api/handler_config_test.go
+++ b/internal/api/handler_config_test.go
@@ -52,6 +52,14 @@ func TestHandler_updateConfig(t *testing.T) {
mockStore.On("SaveGlobalConfig", ctx, mock.AnythingOfType("*config.GlobalConfig")).Return(nil)
// Mock ListServiceConfigs for propagation of global defaults
mockStore.On("ListServiceConfigs", ctx).Return([]config.ServiceConfig{}, nil)
+ // updateConfig now calls GetGlobalConfig when recommendations_cache_stale_hours
+ // or recommendations_lookback_days is omitted from the request body, so the
+ // existing persisted value can be preserved rather than zeroed out (PR #308
+ // CodeRabbit pass-2). The body in this test omits both fields.
+ mockStore.On("GetGlobalConfig", ctx).Return(&config.GlobalConfig{
+ RecommendationsCacheStaleHours: config.DefaultRecommendationsCacheStaleHours,
+ RecommendationsLookbackDays: config.DefaultRecommendationsLookbackDays,
+ }, nil)
handler := &Handler{config: mockStore, auth: mockAuth}
@@ -367,6 +375,13 @@ func TestHandler_updateConfig_ValidationError(t *testing.T) {
}
mockAuth.On("ValidateSession", ctx, "admin-token").Return(adminSession, nil)
+ // updateConfig calls GetGlobalConfig before validation to preserve persisted
+ // values for fields omitted from the request body (PR #308 CR pass-2). The
+ // validation error fires after the merge, so the mock is still required.
+ mockStore.On("GetGlobalConfig", ctx).Return(&config.GlobalConfig{
+ RecommendationsCacheStaleHours: config.DefaultRecommendationsCacheStaleHours,
+ RecommendationsLookbackDays: config.DefaultRecommendationsLookbackDays,
+ }, nil)
handler := &Handler{config: mockStore, auth: mockAuth}
@@ -397,6 +412,12 @@ func TestHandler_updateConfig_SaveError(t *testing.T) {
mockAuth.On("ValidateSession", ctx, "admin-token").Return(adminSession, nil)
mockStore.On("SaveGlobalConfig", ctx, mock.AnythingOfType("*config.GlobalConfig")).Return(assert.AnError)
+ // updateConfig calls GetGlobalConfig before save to preserve persisted
+ // values for fields omitted from the request body (PR #308 CR pass-2).
+ mockStore.On("GetGlobalConfig", ctx).Return(&config.GlobalConfig{
+ RecommendationsCacheStaleHours: config.DefaultRecommendationsCacheStaleHours,
+ RecommendationsLookbackDays: config.DefaultRecommendationsLookbackDays,
+ }, nil)
handler := &Handler{config: mockStore, auth: mockAuth}
@@ -469,6 +490,12 @@ func TestHandler_updateConfig_WithPropagation(t *testing.T) {
mockAuth.On("ValidateSession", ctx, "admin-token").Return(adminSession, nil)
mockStore.On("SaveGlobalConfig", ctx, mock.AnythingOfType("*config.GlobalConfig")).Return(nil)
+ // updateConfig calls GetGlobalConfig before save to preserve persisted
+ // values for fields omitted from the request body (PR #308 CR pass-2).
+ mockStore.On("GetGlobalConfig", ctx).Return(&config.GlobalConfig{
+ RecommendationsCacheStaleHours: config.DefaultRecommendationsCacheStaleHours,
+ RecommendationsLookbackDays: config.DefaultRecommendationsLookbackDays,
+ }, nil)
mockStore.On("ListServiceConfigs", ctx).Return(serviceConfigs, nil)
mockStore.On("SaveServiceConfig", ctx, mock.AnythingOfType("*config.ServiceConfig")).Return(nil)
@@ -506,6 +533,12 @@ func TestHandler_updateConfig_PropagationServiceSaveError(t *testing.T) {
mockAuth.On("ValidateSession", ctx, "admin-token").Return(adminSession, nil)
mockStore.On("SaveGlobalConfig", ctx, mock.AnythingOfType("*config.GlobalConfig")).Return(nil)
+ // updateConfig calls GetGlobalConfig before save to preserve persisted
+ // values for fields omitted from the request body (PR #308 CR pass-2).
+ mockStore.On("GetGlobalConfig", ctx).Return(&config.GlobalConfig{
+ RecommendationsCacheStaleHours: config.DefaultRecommendationsCacheStaleHours,
+ RecommendationsLookbackDays: config.DefaultRecommendationsLookbackDays,
+ }, nil)
mockStore.On("ListServiceConfigs", ctx).Return(serviceConfigs, nil)
// Simulate failure when saving service config during propagation
mockStore.On("SaveServiceConfig", ctx, mock.AnythingOfType("*config.ServiceConfig")).Return(assert.AnError)
@@ -538,6 +571,12 @@ func TestHandler_updateConfig_PropagationListError(t *testing.T) {
mockAuth.On("ValidateSession", ctx, "admin-token").Return(adminSession, nil)
mockStore.On("SaveGlobalConfig", ctx, mock.AnythingOfType("*config.GlobalConfig")).Return(nil)
+ // updateConfig calls GetGlobalConfig before save to preserve persisted
+ // values for fields omitted from the request body (PR #308 CR pass-2).
+ mockStore.On("GetGlobalConfig", ctx).Return(&config.GlobalConfig{
+ RecommendationsCacheStaleHours: config.DefaultRecommendationsCacheStaleHours,
+ RecommendationsLookbackDays: config.DefaultRecommendationsLookbackDays,
+ }, nil)
// Simulate failure when listing service configs for propagation
mockStore.On("ListServiceConfigs", ctx).Return(nil, assert.AnError)
diff --git a/internal/api/handler_test.go b/internal/api/handler_test.go
index 4c745bc1..bed3ea47 100644
--- a/internal/api/handler_test.go
+++ b/internal/api/handler_test.go
@@ -371,6 +371,12 @@ func TestHandler_HandleRequest_PutConfig(t *testing.T) {
mockStore.On("SaveGlobalConfig", mock.Anything, mock.AnythingOfType("*config.GlobalConfig")).Return(nil)
mockStore.On("ListServiceConfigs", mock.Anything).Return([]config.ServiceConfig{}, nil)
+ // updateConfig calls GetGlobalConfig before save to preserve persisted
+ // values for fields omitted from the request body (PR #308 CR pass-2).
+ mockStore.On("GetGlobalConfig", mock.Anything).Return(&config.GlobalConfig{
+ RecommendationsCacheStaleHours: config.DefaultRecommendationsCacheStaleHours,
+ RecommendationsLookbackDays: config.DefaultRecommendationsLookbackDays,
+ }, nil)
mockAuth.On("ValidateSession", ctx, "test-token").Return(adminSession, nil)
mockAuth.On("ValidateCSRFToken", ctx, mock.Anything, mock.Anything).Return(nil)
diff --git a/internal/config/store_postgres.go b/internal/config/store_postgres.go
index 5144a6dc..d09be9ef 100644
--- a/internal/config/store_postgres.go
+++ b/internal/config/store_postgres.go
@@ -49,7 +49,8 @@ func (s *PostgresStore) GetGlobalConfig(ctx context.Context) (*GlobalConfig, err
ri_exchange_enabled, ri_exchange_mode, ri_exchange_utilization_threshold,
ri_exchange_max_per_exchange_usd, ri_exchange_max_daily_usd, ri_exchange_lookback_days,
auto_collect, collection_schedule, notification_days_before,
- grace_period_days
+ grace_period_days,
+ recommendations_cache_stale_hours, recommendations_lookback_days
FROM global_config
WHERE id = 1
`
@@ -76,6 +77,8 @@ func (s *PostgresStore) GetGlobalConfig(ctx context.Context) (*GlobalConfig, err
&config.CollectionSchedule,
&config.NotificationDaysBefore,
&gracePeriodJSON,
+ &config.RecommendationsCacheStaleHours,
+ &config.RecommendationsLookbackDays,
)
if err != nil {
@@ -95,6 +98,8 @@ func (s *PostgresStore) GetGlobalConfig(ctx context.Context) (*GlobalConfig, err
AutoCollect: true,
CollectionSchedule: "daily",
NotificationDaysBefore: 3,
+ RecommendationsCacheStaleHours: DefaultRecommendationsCacheStaleHours,
+ RecommendationsLookbackDays: DefaultRecommendationsLookbackDays,
}, nil
}
return nil, fmt.Errorf("failed to get global config: %w", err)
@@ -125,8 +130,9 @@ func (s *PostgresStore) SaveGlobalConfig(ctx context.Context, config *GlobalConf
ri_exchange_enabled, ri_exchange_mode, ri_exchange_utilization_threshold,
ri_exchange_max_per_exchange_usd, ri_exchange_max_daily_usd, ri_exchange_lookback_days,
auto_collect, collection_schedule, notification_days_before,
- grace_period_days
- ) VALUES (1, $1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12, $13, $14, $15, $16, $17)
+ grace_period_days,
+ recommendations_cache_stale_hours, recommendations_lookback_days
+ ) VALUES (1, $1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12, $13, $14, $15, $16, $17, $18, $19)
ON CONFLICT (id) DO UPDATE SET
enabled_providers = $1,
notification_email = $2,
@@ -145,6 +151,8 @@ func (s *PostgresStore) SaveGlobalConfig(ctx context.Context, config *GlobalConf
collection_schedule = $15,
notification_days_before = $16,
grace_period_days = $17,
+ recommendations_cache_stale_hours = $18,
+ recommendations_lookback_days = $19,
updated_at = NOW()
`
@@ -157,6 +165,12 @@ func (s *PostgresStore) SaveGlobalConfig(ctx context.Context, config *GlobalConf
if riExchangeLookbackDays == 0 {
riExchangeLookbackDays = 30
}
+ recommendationsLookbackDays := config.RecommendationsLookbackDays
+ if recommendationsLookbackDays == 0 {
+ // Validate() treats 0 as "unset → use default". Mirror that here so
+ // the DB never stores 0, matching the ErrNoRows default path.
+ recommendationsLookbackDays = DefaultRecommendationsLookbackDays
+ }
riExchangeUtilizationThreshold := config.RIExchangeUtilizationThreshold
if riExchangeUtilizationThreshold == 0 {
riExchangeUtilizationThreshold = 95.0
@@ -192,6 +206,8 @@ func (s *PostgresStore) SaveGlobalConfig(ctx context.Context, config *GlobalConf
config.CollectionSchedule,
config.NotificationDaysBefore,
gracePeriodJSON,
+ config.RecommendationsCacheStaleHours,
+ recommendationsLookbackDays,
)
if err != nil {
diff --git a/internal/config/store_postgres_pgxmock_test.go b/internal/config/store_postgres_pgxmock_test.go
index 78139b5d..1db42877 100644
--- a/internal/config/store_postgres_pgxmock_test.go
+++ b/internal/config/store_postgres_pgxmock_test.go
@@ -55,6 +55,7 @@ func TestPGXMock_GetGlobalConfig_Success(t *testing.T) {
"ri_exchange_max_per_exchange_usd", "ri_exchange_max_daily_usd", "ri_exchange_lookback_days",
"auto_collect", "collection_schedule", "notification_days_before",
"grace_period_days",
+ "recommendations_cache_stale_hours", "recommendations_lookback_days",
}
rows := pgxmock.NewRows(cols).AddRow(
[]string{"aws"}, strPtr("ops@example.com"), true,
@@ -63,6 +64,7 @@ func TestPGXMock_GetGlobalConfig_Success(t *testing.T) {
0.0, 0.0, 30,
true, "daily", 3,
"{}",
+ 24, 7,
)
mock.ExpectQuery("SELECT").WillReturnRows(rows)
@@ -71,6 +73,8 @@ func TestPGXMock_GetGlobalConfig_Success(t *testing.T) {
assert.Equal(t, []string{"aws"}, cfg.EnabledProviders)
require.NotNil(t, cfg.NotificationEmail)
assert.Equal(t, "ops@example.com", *cfg.NotificationEmail)
+ assert.Equal(t, 24, cfg.RecommendationsCacheStaleHours)
+ assert.Equal(t, 7, cfg.RecommendationsLookbackDays)
assert.NoError(t, mock.ExpectationsWereMet())
}
@@ -99,6 +103,7 @@ func TestPGXMock_GetGlobalConfig_GracePeriodDays(t *testing.T) {
"ri_exchange_max_per_exchange_usd", "ri_exchange_max_daily_usd", "ri_exchange_lookback_days",
"auto_collect", "collection_schedule", "notification_days_before",
"grace_period_days",
+ "recommendations_cache_stale_hours", "recommendations_lookback_days",
}
baseRow := func(graceJSON string) []any {
return []any{
@@ -108,6 +113,7 @@ func TestPGXMock_GetGlobalConfig_GracePeriodDays(t *testing.T) {
0.0, 0.0, 30,
true, "daily", 3,
graceJSON,
+ 24, 7,
}
}
@@ -123,6 +129,8 @@ func TestPGXMock_GetGlobalConfig_GracePeriodDays(t *testing.T) {
// GracePeriodFor returns the default for every provider.
assert.Equal(t, DefaultGracePeriodDays, cfg.GracePeriodFor("aws"))
assert.Equal(t, DefaultGracePeriodDays, cfg.GracePeriodFor("azure"))
+ assert.Equal(t, 24, cfg.RecommendationsCacheStaleHours)
+ assert.Equal(t, 7, cfg.RecommendationsLookbackDays)
})
t.Run("populated json round-trips", func(t *testing.T) {
diff --git a/internal/config/types.go b/internal/config/types.go
index 488a0909..17a343fa 100644
--- a/internal/config/types.go
+++ b/internal/config/types.go
@@ -35,6 +35,22 @@ type GlobalConfig struct {
RIExchangeMaxPerExchangeUSD float64 `json:"ri_exchange_max_per_exchange_usd" dynamodbav:"ri_exchange_max_per_exchange_usd"`
RIExchangeMaxDailyUSD float64 `json:"ri_exchange_max_daily_usd" dynamodbav:"ri_exchange_max_daily_usd"`
RIExchangeLookbackDays int `json:"ri_exchange_lookback_days" dynamodbav:"ri_exchange_lookback_days"`
+
+ // RecommendationsCacheStaleHours is the age (hours) at which the
+ // recommendations cache is considered stale and a background refresh
+ // fires automatically (stale-while-revalidate). 0 disables automatic
+ // background refresh; the cron scheduler and the manual Refresh button
+ // still work regardless. Valid range: 0–8760 (up to one year).
+ // Default: 24.
+ RecommendationsCacheStaleHours int `json:"recommendations_cache_stale_hours" db:"recommendations_cache_stale_hours"`
+
+ // RecommendationsLookbackDays is the AWS Cost Explorer lookback window
+ // (days) used when fetching fresh recommendations. Must be one of 7,
+ // 30, or 60 — the AWS Cost Explorer LookbackPeriodInDays enum.
+ // GCP CUD Recommender has no equivalent lookback parameter (fixed
+ // internally); this setting applies to AWS only.
+ // Default: 7.
+ RecommendationsLookbackDays int `json:"recommendations_lookback_days" db:"recommendations_lookback_days"`
}
// DefaultGracePeriodDays is the fallback window used when a provider
@@ -48,6 +64,22 @@ const DefaultGracePeriodDays = 7
// rogue write through psql shouldn't be able to suppress recs for years.
const MaxGracePeriodDays = 90
+// DefaultRecommendationsCacheStaleHours is the default age (hours) after
+// which the recommendations cache triggers a background refresh.
+const DefaultRecommendationsCacheStaleHours = 24
+
+// MaxRecommendationsCacheStaleHours is the maximum configurable stale
+// threshold: one year. Values above this are rejected at validation time.
+const MaxRecommendationsCacheStaleHours = 8760
+
+// DefaultRecommendationsLookbackDays is the default AWS Cost Explorer
+// lookback window when no explicit value is configured.
+const DefaultRecommendationsLookbackDays = 7
+
+// ValidRecommendationsLookbackDays lists the AWS Cost Explorer
+// LookbackPeriodInDays enum values. Other values are rejected.
+var ValidRecommendationsLookbackDays = []int{7, 30, 60}
+
// GracePeriodFor returns the effective grace-period window (in days)
// for the given provider slug ("aws", "azure", "gcp"). Returns the
// default when the provider has no explicit entry. Preserves an
diff --git a/internal/config/validation.go b/internal/config/validation.go
index 1157a7ee..42a85875 100644
--- a/internal/config/validation.go
+++ b/internal/config/validation.go
@@ -45,9 +45,52 @@ func (c *GlobalConfig) Validate() error {
if err := c.validateGracePeriodDays(); err != nil {
return err
}
+ return c.validateRecommendationsFields()
+}
+
+// validateRecommendationsFields validates the recommendation-cycle parameters
+// added in #301 (cache-staleness threshold and AWS Cost Explorer lookback).
+// Extracted to keep Validate's cyclomatic complexity under the project limit.
+func (c *GlobalConfig) validateRecommendationsFields() error {
+ if err := c.validateRecommendationsCacheStaleHours(); err != nil {
+ return err
+ }
+ return c.validateRecommendationsLookbackDays()
+}
+
+// validateRecommendationsCacheStaleHours validates the stale-while-revalidate
+// cache age threshold. 0 disables background refresh; negative values are
+// rejected. Maximum is MaxRecommendationsCacheStaleHours (1 year).
+func (c *GlobalConfig) validateRecommendationsCacheStaleHours() error {
+ if c.RecommendationsCacheStaleHours < 0 || c.RecommendationsCacheStaleHours > MaxRecommendationsCacheStaleHours {
+ return fmt.Errorf("recommendations_cache_stale_hours must be between 0 and %d, got: %d (0 = disable auto-refresh)", MaxRecommendationsCacheStaleHours, c.RecommendationsCacheStaleHours)
+ }
return nil
}
+// validateRecommendationsLookbackDays validates the AWS Cost Explorer lookback
+// window. Only the enum values {7, 30, 60} are accepted (LookbackPeriodInDays).
+// A value of 0 means "unset / use the backend default" and is always accepted;
+// this matches the zero-value of the Go int field when the DB row predates the
+// column (the migration adds DEFAULT 7, but in-memory structs constructed
+// without explicitly setting the field carry 0).
+func (c *GlobalConfig) validateRecommendationsLookbackDays() error {
+ if c.RecommendationsLookbackDays == 0 {
+ // Unset → backend defaults to DefaultRecommendationsLookbackDays.
+ return nil
+ }
+ for _, valid := range ValidRecommendationsLookbackDays {
+ if c.RecommendationsLookbackDays == valid {
+ return nil
+ }
+ }
+ validStrs := make([]string, len(ValidRecommendationsLookbackDays))
+ for i, v := range ValidRecommendationsLookbackDays {
+ validStrs[i] = fmt.Sprintf("%d", v)
+ }
+ return fmt.Errorf("recommendations_lookback_days must be one of [%s], got: %d", strings.Join(validStrs, ", "), c.RecommendationsLookbackDays)
+}
+
// validateGracePeriodDays validates the per-provider grace-period map.
// Keys must be known provider slugs; values must be in [0, MaxGracePeriodDays].
// A nil / empty map is always valid (falls back to the default).
diff --git a/internal/config/validation_test.go b/internal/config/validation_test.go
index 6c1226fa..f3ab646c 100644
--- a/internal/config/validation_test.go
+++ b/internal/config/validation_test.go
@@ -165,6 +165,87 @@ func TestGlobalConfig_Validate(t *testing.T) {
},
wantErr: false,
},
+ // Issue #301: RecommendationsCacheStaleHours validation
+ {
+ name: "stale hours zero is valid (disable auto-refresh)",
+ config: GlobalConfig{
+ DefaultTerm: 3,
+ RecommendationsCacheStaleHours: 0,
+ RecommendationsLookbackDays: 7,
+ },
+ wantErr: false,
+ },
+ {
+ name: "stale hours at max (8760) is valid",
+ config: GlobalConfig{
+ DefaultTerm: 3,
+ RecommendationsCacheStaleHours: MaxRecommendationsCacheStaleHours,
+ RecommendationsLookbackDays: 7,
+ },
+ wantErr: false,
+ },
+ {
+ name: "stale hours negative is invalid",
+ config: GlobalConfig{
+ DefaultTerm: 3,
+ RecommendationsCacheStaleHours: -1,
+ RecommendationsLookbackDays: 7,
+ },
+ wantErr: true,
+ errMsg: "recommendations_cache_stale_hours must be between 0",
+ },
+ {
+ name: "stale hours above max is invalid",
+ config: GlobalConfig{
+ DefaultTerm: 3,
+ RecommendationsCacheStaleHours: MaxRecommendationsCacheStaleHours + 1,
+ RecommendationsLookbackDays: 7,
+ },
+ wantErr: true,
+ errMsg: "recommendations_cache_stale_hours must be between 0",
+ },
+ // Issue #301: RecommendationsLookbackDays validation (AWS enum {7,30,60})
+ {
+ name: "lookback 7 is valid",
+ config: GlobalConfig{
+ DefaultTerm: 3,
+ RecommendationsLookbackDays: 7,
+ },
+ wantErr: false,
+ },
+ {
+ name: "lookback 30 is valid",
+ config: GlobalConfig{
+ DefaultTerm: 3,
+ RecommendationsLookbackDays: 30,
+ },
+ wantErr: false,
+ },
+ {
+ name: "lookback 60 is valid",
+ config: GlobalConfig{
+ DefaultTerm: 3,
+ RecommendationsLookbackDays: 60,
+ },
+ wantErr: false,
+ },
+ {
+ name: "lookback 14 is invalid (not in AWS enum)",
+ config: GlobalConfig{
+ DefaultTerm: 3,
+ RecommendationsLookbackDays: 14,
+ },
+ wantErr: true,
+ errMsg: "recommendations_lookback_days must be one of",
+ },
+ {
+ name: "lookback 0 is valid (unset, uses backend default)",
+ config: GlobalConfig{
+ DefaultTerm: 3,
+ RecommendationsLookbackDays: 0,
+ },
+ wantErr: false,
+ },
}
for _, tt := range tests {
diff --git a/internal/database/postgres/migrations/000048_recommendations_cycle_params.down.sql b/internal/database/postgres/migrations/000048_recommendations_cycle_params.down.sql
new file mode 100644
index 00000000..2dbea629
--- /dev/null
+++ b/internal/database/postgres/migrations/000048_recommendations_cycle_params.down.sql
@@ -0,0 +1,3 @@
+ALTER TABLE global_config
+ DROP COLUMN IF EXISTS recommendations_cache_stale_hours,
+ DROP COLUMN IF EXISTS recommendations_lookback_days;
diff --git a/internal/database/postgres/migrations/000048_recommendations_cycle_params.up.sql b/internal/database/postgres/migrations/000048_recommendations_cycle_params.up.sql
new file mode 100644
index 00000000..79bf3685
--- /dev/null
+++ b/internal/database/postgres/migrations/000048_recommendations_cycle_params.up.sql
@@ -0,0 +1,44 @@
+-- Add recommendations cycle parameters to global_config.
+--
+-- recommendations_cache_stale_hours:
+-- Age (hours) at which the recommendation cache is considered stale and a
+-- background refresh fires automatically (stale-while-revalidate pattern).
+-- 0 disables automatic background refresh entirely; the cron scheduler and
+-- the manual Refresh button still work regardless of this setting.
+-- Valid range: 0–8760 (up to one year). Default: 24.
+--
+-- recommendations_lookback_days:
+-- AWS Cost Explorer lookback window used when fetching fresh recommendations.
+-- Must be one of 7, 30, or 60 (matches the AWS Cost Explorer
+-- LookbackPeriodInDays enum). Default: 7.
+-- GCP CUD Recommender has no equivalent lookback parameter; this value
+-- applies to AWS only. Azure support is tracked as a follow-up.
+ALTER TABLE global_config
+ ADD COLUMN IF NOT EXISTS recommendations_cache_stale_hours INT NOT NULL DEFAULT 24,
+ ADD COLUMN IF NOT EXISTS recommendations_lookback_days INT NOT NULL DEFAULT 7;
+
+-- Enforce the documented ranges at the DB layer too. Application-side
+-- validation catches the API path, but a manual SQL update or a future
+-- direct-DB writer would otherwise be able to persist out-of-range values
+-- that GetGlobalConfig() then reads back verbatim and hands to the
+-- scheduler. NOT VALID + VALIDATE keeps the migration online-safe in
+-- case any pre-default rows ever slipped through (defaults guarantee
+-- validity for rows added by this migration, but defensive anyway).
+DO $$
+BEGIN
+ IF NOT EXISTS (
+ SELECT 1 FROM pg_constraint WHERE conname = 'chk_global_config_recommendations_cache_stale_hours_range'
+ ) THEN
+ ALTER TABLE global_config
+ ADD CONSTRAINT chk_global_config_recommendations_cache_stale_hours_range
+ CHECK (recommendations_cache_stale_hours BETWEEN 0 AND 8760);
+ END IF;
+
+ IF NOT EXISTS (
+ SELECT 1 FROM pg_constraint WHERE conname = 'chk_global_config_recommendations_lookback_days_allowed'
+ ) THEN
+ ALTER TABLE global_config
+ ADD CONSTRAINT chk_global_config_recommendations_lookback_days_allowed
+ CHECK (recommendations_lookback_days IN (7, 30, 60));
+ END IF;
+END $$;
diff --git a/internal/scheduler/scheduler.go b/internal/scheduler/scheduler.go
index b53b08ab..3b00be33 100644
--- a/internal/scheduler/scheduler.go
+++ b/internal/scheduler/scheduler.go
@@ -709,10 +709,14 @@ func (s *Scheduler) fetchAndConvert(ctx context.Context, prov provider.Provider,
return nil, fmt.Errorf("failed to get %s recommendations: %w", providerName, err)
}
if len(recs) == 0 && globalCfg != nil {
+ lookbackDays := globalCfg.RecommendationsLookbackDays
+ if lookbackDays == 0 {
+ lookbackDays = config.DefaultRecommendationsLookbackDays
+ }
params := common.RecommendationParams{
Term: fmt.Sprintf("%dyr", globalCfg.DefaultTerm),
PaymentOption: globalCfg.DefaultPayment,
- LookbackPeriod: "7d",
+ LookbackPeriod: fmt.Sprintf("%dd", lookbackDays),
}
recs, _ = recClient.GetRecommendations(ctx, params)
}
@@ -783,10 +787,41 @@ func (s *Scheduler) ListRecommendations(ctx context.Context, filter config.Recom
logging.Errorf("failed to apply account overrides; returning un-filtered recs: %v", err)
}
- s.maybeKickBackgroundRefresh(freshness)
+ // Background refresh is only attempted on non-Lambda runtimes; skip
+ // the DB config read entirely on Lambda so GetGlobalConfig is not called
+ // on the hot read path (Lambda has no persistent goroutines).
+ if !s.isLambda {
+ ttl, disabled := s.resolveEffectiveCacheTTL(ctx)
+ if disabled {
+ return recs, nil
+ }
+ s.maybeKickBackgroundRefresh(freshness, ttl)
+ }
return recs, nil
}
+// resolveEffectiveCacheTTL returns the effective stale-while-revalidate TTL
+// and whether background auto-refresh has been explicitly disabled (value 0).
+// It prefers the DB-configured RecommendationsCacheStaleHours; falls back to
+// s.cacheTTL (env-var / compile-time default) when the DB read fails.
+func (s *Scheduler) resolveEffectiveCacheTTL(ctx context.Context) (ttl time.Duration, disabled bool) {
+ ttl = s.cacheTTL
+ globalCfg, gcErr := s.config.GetGlobalConfig(ctx)
+ if gcErr != nil || globalCfg == nil {
+ // DB read failed OR returned (nil, nil) — fall back to the env-var /
+ // default TTL with refresh enabled rather than panicking on the
+ // nil deref below. (nil, nil) is a defensive case; the Postgres
+ // store currently never returns it, but a future store impl or a
+ // test mock might.
+ return ttl, false
+ }
+ if globalCfg.RecommendationsCacheStaleHours == 0 {
+ // Explicit 0 = operator opted out of automatic background refresh.
+ return 0, true
+ }
+ return time.Duration(globalCfg.RecommendationsCacheStaleHours) * time.Hour, false
+}
+
// suppressionKey is the 6-tuple used to match suppressions to recs.
type suppressionKey struct {
accountID, provider, service, region, resourceType, engine string
@@ -892,7 +927,10 @@ func applySuppressionIndex(recs []config.RecommendationRecord, index map[suppres
// single-flights concurrent callers so only one refresh fires per TTL
// window. Recovers from panics so one bad collect can't crash the
// long-lived process.
-func (s *Scheduler) maybeKickBackgroundRefresh(freshness *config.RecommendationsFreshness) {
+//
+// effectiveTTL is the caller-resolved stale threshold (from DB config
+// RecommendationsCacheStaleHours, or the env-var default if unconfigured).
+func (s *Scheduler) maybeKickBackgroundRefresh(freshness *config.RecommendationsFreshness, effectiveTTL time.Duration) {
if s.isLambda {
return
}
@@ -900,7 +938,7 @@ func (s *Scheduler) maybeKickBackgroundRefresh(freshness *config.Recommendations
// Just handled synchronously; nothing to backfill.
return
}
- if time.Since(*freshness.LastCollectedAt) < s.cacheTTL {
+ if time.Since(*freshness.LastCollectedAt) < effectiveTTL {
return
}
if !s.collecting.CompareAndSwap(false, true) {
@@ -908,7 +946,7 @@ func (s *Scheduler) maybeKickBackgroundRefresh(freshness *config.Recommendations
return
}
- logging.Infof("Recommendations cache is stale (age > %s); triggering background refresh", s.cacheTTL)
+ logging.Infof("Recommendations cache is stale (age > %s); triggering background refresh", effectiveTTL)
bgCtx, cancel := context.WithTimeout(context.Background(), 5*time.Minute)
go func() {
defer cancel()
diff --git a/internal/scheduler/scheduler_overrides_test.go b/internal/scheduler/scheduler_overrides_test.go
index 6d0d56bb..e3b2d722 100644
--- a/internal/scheduler/scheduler_overrides_test.go
+++ b/internal/scheduler/scheduler_overrides_test.go
@@ -53,6 +53,18 @@ func (m *mockOverrideStore) GetAccountServiceOverride(_ context.Context, account
return m.overrides[accountID+"|"+provider+"|"+service], nil
}
+// GetGlobalConfig returns a default config so ListRecommendations can resolve
+// the effective stale TTL without panicking on the embedded MockConfigStore.
+// The returned RecommendationsCacheStaleHours of 24 means ListRecommendations
+// will use the DB-configured value (24h); the tests in this file exercise
+// override/suppression logic, not TTL behaviour.
+func (m *mockOverrideStore) GetGlobalConfig(_ context.Context) (*config.GlobalConfig, error) {
+ return &config.GlobalConfig{
+ RecommendationsCacheStaleHours: config.DefaultRecommendationsCacheStaleHours,
+ RecommendationsLookbackDays: config.DefaultRecommendationsLookbackDays,
+ }, nil
+}
+
func boolPtr(b bool) *bool { return &b }
// rdsRec returns a rec for the given account/region/engine with sensible defaults.
diff --git a/internal/scheduler/scheduler_suppressions_test.go b/internal/scheduler/scheduler_suppressions_test.go
index cb42698b..694de959 100644
--- a/internal/scheduler/scheduler_suppressions_test.go
+++ b/internal/scheduler/scheduler_suppressions_test.go
@@ -50,6 +50,15 @@ func (m *mockSuppressionStore) GetAccountServiceOverride(_ context.Context, _, _
return nil, nil
}
+// GetGlobalConfig returns a default config so ListRecommendations can resolve
+// the effective stale TTL without panicking on the embedded MockConfigStore.
+func (m *mockSuppressionStore) GetGlobalConfig(_ context.Context) (*config.GlobalConfig, error) {
+ return &config.GlobalConfig{
+ RecommendationsCacheStaleHours: config.DefaultRecommendationsCacheStaleHours,
+ RecommendationsLookbackDays: config.DefaultRecommendationsLookbackDays,
+ }, nil
+}
+
func strPtr(s string) *string { return &s }
func TestApplySuppressions_SubtractsCount(t *testing.T) {
diff --git a/internal/scheduler/scheduler_test.go b/internal/scheduler/scheduler_test.go
index b9c0c2bd..e77e2f4a 100644
--- a/internal/scheduler/scheduler_test.go
+++ b/internal/scheduler/scheduler_test.go
@@ -1061,6 +1061,10 @@ func TestScheduler_ListRecommendations(t *testing.T) {
Return(&config.RecommendationsFreshness{LastCollectedAt: &now}, nil)
mockStore.On("ListStoredRecommendations", ctx, mock.Anything).
Return(cached, nil)
+ // Non-Lambda path resolves the effective stale TTL from the DB config.
+ mockStore.On("GetGlobalConfig", ctx).Return(&config.GlobalConfig{
+ RecommendationsCacheStaleHours: config.DefaultRecommendationsCacheStaleHours,
+ }, nil)
scheduler := &Scheduler{config: mockStore}
@@ -1069,6 +1073,43 @@ func TestScheduler_ListRecommendations(t *testing.T) {
assert.Len(t, recs, 2)
}
+// Pin the disable-sentinel contract: when GlobalConfig.RecommendationsCacheStaleHours
+// is 0, ListRecommendations must serve from cache (the existing behaviour) without
+// kicking off a background refresh — even when the cached row is older than any
+// hard-coded fallback TTL. The cache-staleness path should treat 0 as "auto-refresh
+// disabled" rather than "stale immediately". Regression guard for PR #308.
+func TestScheduler_ListRecommendations_StaleHoursZeroDisablesBackgroundRefresh(t *testing.T) {
+ ctx := context.Background()
+ mockStore := new(MockConfigStore)
+
+ old := time.Now().Add(-72 * time.Hour) // older than any reasonable default TTL
+ cached := []config.RecommendationRecord{
+ {Provider: "aws", Service: "ec2", Region: "us-east-1", Savings: 1},
+ }
+ mockStore.On("GetRecommendationsFreshness", ctx).
+ Return(&config.RecommendationsFreshness{LastCollectedAt: &old}, nil)
+ mockStore.On("ListStoredRecommendations", ctx, mock.Anything).
+ Return(cached, nil)
+ // Disable sentinel: 0 must NOT trigger a background refresh.
+ mockStore.On("GetGlobalConfig", ctx).Return(&config.GlobalConfig{
+ RecommendationsCacheStaleHours: 0,
+ }, nil)
+
+ scheduler := &Scheduler{config: mockStore}
+
+ recs, err := scheduler.ListRecommendations(ctx, config.RecommendationFilter{})
+ require.NoError(t, err)
+ assert.Len(t, recs, 1)
+
+ // Asserting via mock expectations: MarkCollectionStarted is what the
+ // background-refresh path would call. When the sentinel is 0, no refresh
+ // fires, so MarkCollectionStarted MUST NOT be called (and absence of an
+ // `On(...)` expectation for it would cause testify-mock to panic if it
+ // were called — the Len assertion above is the primary check, this is the
+ // second-line guard).
+ mockStore.AssertNotCalled(t, "MarkCollectionStarted", mock.Anything)
+}
+
// Filter pass-through: the handler-level RecommendationQueryParams fields
// map into the DB-facing RecommendationFilter. The SQL pushdown semantics
// themselves are covered by store_postgres_recommendations_test.go.
@@ -1088,6 +1129,10 @@ func TestScheduler_ListRecommendations_PassesFilterToStore(t *testing.T) {
}
mockStore.On("ListStoredRecommendations", ctx, expected).
Return([]config.RecommendationRecord{}, nil)
+ // Non-Lambda path resolves effective stale TTL from DB config.
+ mockStore.On("GetGlobalConfig", ctx).Return(&config.GlobalConfig{
+ RecommendationsCacheStaleHours: config.DefaultRecommendationsCacheStaleHours,
+ }, nil)
scheduler := &Scheduler{config: mockStore}
@@ -1160,7 +1205,7 @@ func TestScheduler_ListRecommendations_StaleSingleFlight(t *testing.T) {
// Seed the flag as though a refresh is already in flight. The
// guard short-circuits and no new goroutine fires.
scheduler.collecting.Store(true)
- scheduler.maybeKickBackgroundRefresh(freshness)
+ scheduler.maybeKickBackgroundRefresh(freshness, time.Nanosecond)
assert.True(t, scheduler.collecting.Load(), "in-flight flag must not be cleared by the guard path")
_ = ctx
}