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 docs/DEPLOYMENT.md
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ compute_platform = "lambda"
# Lambda configuration
lambda_architecture = "arm64"
lambda_memory_size = 512
lambda_timeout = 60
lambda_timeout = 300 # 300-900: covers the 270s exchange/purchase budget

# Database configuration (Aurora Serverless v2)
database_engine_version = "16.4"
Expand Down
4 changes: 3 additions & 1 deletion internal/api/ri_exchange_detached.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,9 @@ import (
// run on contexts that keep the request's values but drop its cancellation,
// each bounded by its own budget so a hung provider or database cannot pin the
// goroutine forever. On Lambda the invocation itself is still bounded by the
// function timeout, which no context can extend.
// function timeout, which no context can extend, so that timeout must cover
// the sum of both budgets plus a 30s margin (lambda_timeout >= 300, enforced
// by Terraform validation and TestLambdaTimeoutCoversRIExchangeBudget, #913).
//
// The detaching itself is ctxutil.DetachedWithTimeout, shared with the
// purchase path (#706 / #789).
Expand Down
13 changes: 11 additions & 2 deletions internal/api/ri_exchange_detached_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"github.com/LeanerCloud/cloud-commitments-go/pkg/exchange"
azurecompute "github.com/LeanerCloud/cloud-commitments-go/providers/azure/services/compute"
"github.com/LeanerCloud/cloud-commitments-platform/internal/config"
"github.com/LeanerCloud/cloud-commitments-platform/internal/testutil/lambdatimeout"
"github.com/aws/aws-lambda-go/events"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/mock"
Expand Down Expand Up @@ -40,11 +41,19 @@ func (p *ctxProbe) assertDetached(t *testing.T, start time.Time) {
"canceling the request must not cancel an exchange already submitted to the provider")
require.True(t, p.hasDeadline, "the detached exchange must still be bounded by its own timeout")
// A literal rather than riExchangeExecutionTimeout, so the test pins the
// budget itself: it must stay inside the documented lambda_timeout >= 270s
// requirement (4m execution + 30s terminal save).
// budget itself: with the terminal save it must stay inside the lambda_timeout
// >= 300s floor (4m execution + 30s terminal save + 30s margin, #913).
assert.WithinDuration(t, start.Add(4*time.Minute), p.deadline, 5*time.Second)
}

// TestLambdaTimeoutCoversRIExchangeBudget keeps every shipped AWS Lambda
// timeout above the detached RI exchange budget. The provider call and the
// terminal ledger write run back to back after the request, and Lambda ends
// the invocation at its timeout whatever the context deadline says (#913).
func TestLambdaTimeoutCoversRIExchangeBudget(t *testing.T) {
lambdatimeout.AssertCovers(t, "../..", riExchangeExecutionTimeout+riExchangeTerminalSaveTimeout)
}

// liveCtx matches a context that has not been canceled and carries the 30s
// terminal-save budget, so a mock expectation fails when a terminal write is
// attempted on the dead request context, as a real database driver would.
Expand Down
9 changes: 9 additions & 0 deletions internal/purchase/execute_detached_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,22 @@ import (

"github.com/LeanerCloud/cloud-commitments-go/pkg/common"
"github.com/LeanerCloud/cloud-commitments-platform/internal/config"
"github.com/LeanerCloud/cloud-commitments-platform/internal/testutil/lambdatimeout"
"github.com/aws/aws-sdk-go-v2/aws"
"github.com/aws/aws-sdk-go-v2/service/sts"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/mock"
"github.com/stretchr/testify/require"
)

// TestLambdaTimeoutCoversPurchaseBudget keeps every shipped AWS Lambda timeout
// above the detached purchase budget (execution, then the terminal save).
// Lambda ends the invocation at its timeout whatever the context deadline
// says, so a shorter timeout cuts a purchase off mid-flight (#913).
func TestLambdaTimeoutCoversPurchaseBudget(t *testing.T) {
lambdatimeout.AssertCovers(t, "../..", purchaseExecutionTimeout+terminalSaveTimeout)
}

// detachFixture wires a direct single-account AWS purchase (issue #706).
type detachFixture struct {
manager *Manager
Expand Down
6 changes: 5 additions & 1 deletion internal/purchase/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,11 @@ func isMultiAccountAckable(execErr error) bool {
}

const (
// Combined execution and persistence budgets require lambda_timeout >= 270s.
// The two budgets run back to back (270s), and on AWS Lambda the function
// timeout ends the invocation whatever these deadlines say, so the Lambda
// timeout must cover their sum plus a 30s margin (lambda_timeout >= 300,
// enforced by Terraform validation and TestLambdaTimeoutCoversPurchaseBudget,
// #913).
purchaseExecutionTimeout = 4 * time.Minute
terminalSaveTimeout = 30 * time.Second
)
Expand Down
150 changes: 150 additions & 0 deletions internal/testutil/lambdatimeout/lambdatimeout.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,150 @@
// Package lambdatimeout pins the AWS Lambda timeouts the repository ships
// against the Go budgets for work detached from the request (#913). It is a
// separate package from testutil so packages that testutil imports (such as
// purchase) can use it without an import cycle.
package lambdatimeout

import (
"fmt"
"os"
"path/filepath"
"regexp"
"strconv"
"testing"
"time"
)

// Margin is the time an AWS Lambda invocation needs on top of a
// detached budget for the request work that runs before the detach (auth,
// database reads, quote lookups) and for writing the response.
const Margin = 30 * time.Second

var (
tfModuleTimeoutVar = regexp.MustCompile(`(?s)variable "timeout" \{.*?\n\}`)
tfRootTimeoutVar = regexp.MustCompile(`(?s)variable "lambda_timeout" \{.*?\n\}`)
tfDefault = regexp.MustCompile(`(?m)^\s*default\s*=\s*(\d+)`)
tfModuleFloor = regexp.MustCompile(`var\.timeout\s*>=\s*(\d+)`)
tfRootFloor = regexp.MustCompile(`var\.lambda_timeout\s*>=\s*(\d+)`)
tfvarsLambdaTimeout = regexp.MustCompile(`(?m)^\s*lambda_timeout\s*=\s*(\d+)`)
cfnLambdaTimeout = regexp.MustCompile(`(?s)LambdaFunction:\s*\n\s*Type: AWS::Lambda::Function\b.*?\n\s{6}Timeout:\s*(\d+)`)
)

// AssertCovers fails t unless every AWS Lambda timeout the
// repository ships covers budget plus Margin: the Terraform
// module and root variable defaults and their validation floors, every
// committed lambda_timeout tfvars value (including examples and the
// deployment guide), and the CloudFormation function's Timeout. Lambda ends an
// invocation at its timeout whatever a context deadline says, so detached
// work with a longer budget would be cut off mid-flight (#913).
//
// repoRoot is the repository root relative to the calling test's package.
func AssertCovers(t *testing.T, repoRoot string, budget time.Duration) {
t.Helper()
c := &checker{t: t, repoRoot: repoRoot, budget: budget, need: budget + Margin}
c.terraformVariables()
c.tfvarsAndDocs()
c.one("cloudformation/stacks/CUDly/template.yaml LambdaFunction Timeout", cfnLambdaTimeout,
c.read("cloudformation/stacks/CUDly/template.yaml"))
}

// checker holds the state shared by the AssertCovers checks.
type checker struct {
t *testing.T
repoRoot string
budget time.Duration
need time.Duration
}

func (c *checker) check(source string, secs int) {
c.t.Helper()
if got := time.Duration(secs) * time.Second; got < c.need {
c.t.Errorf("%s: Lambda timeout %v is shorter than the %v detached budget plus %v margin (want >= %v)",
source, got, c.budget, Margin, c.need)
}
}

func (c *checker) read(rel string) string {
c.t.Helper()
data, err := os.ReadFile(filepath.Join(c.repoRoot, rel)) // #nosec G304 -- fixed repo-relative paths
if err != nil {
c.t.Fatalf("reading %s: %v", rel, err)
}
return string(data)
}

func (c *checker) one(source string, re *regexp.Regexp, text string) {
c.t.Helper()
m := re.FindStringSubmatch(text)
if m == nil {
c.t.Errorf("%s: not found (pattern %s); keep this check in sync with the IaC", source, re)
return
}
c.check(source, atoi(c.t, m[1]))
}

// terraformVariables checks the module and root variable defaults and their
// validation floors.
func (c *checker) terraformVariables() {
c.t.Helper()
for _, v := range []struct {
file string
block *regexp.Regexp
floor *regexp.Regexp
}{
{"terraform/modules/compute/aws/lambda/variables.tf", tfModuleTimeoutVar, tfModuleFloor},
{"terraform/environments/aws/variables.tf", tfRootTimeoutVar, tfRootFloor},
} {
block := v.block.FindString(c.read(v.file))
if block == "" {
c.t.Errorf("%s: Lambda timeout variable not found", v.file)
continue
}
c.one(v.file+" default", tfDefault, block)
c.one(v.file+" validation floor", v.floor, block)
}
}

// tfvarsAndDocs checks every committed lambda_timeout value in tfvars files,
// examples and the deployment guide.
func (c *checker) tfvarsAndDocs() {
c.t.Helper()
values := 0
for _, f := range c.tfvarsFiles() {
for _, m := range tfvarsLambdaTimeout.FindAllStringSubmatch(c.read(f), -1) {
c.check(f+" lambda_timeout", atoi(c.t, m[1]))
values++
}
}
if values < 4 {
c.t.Errorf("found %d lambda_timeout settings in tfvars/docs, want at least 4; keep this check in sync with the IaC", values)
}
}

// tfvarsFiles lists the repo-relative files that may set lambda_timeout.
func (c *checker) tfvarsFiles() []string {
c.t.Helper()
files := []string{"docs/DEPLOYMENT.md"}
for _, pattern := range []string{"terraform/environments/aws/*.tfvars*", "terraform/profiles/aws/*.tfvars*"} {
matches, err := filepath.Glob(filepath.Join(c.repoRoot, pattern))
if err != nil {
c.t.Fatalf("glob %s: %v", pattern, err)
}
for _, m := range matches {
rel, relErr := filepath.Rel(c.repoRoot, m)
if relErr != nil {
c.t.Fatalf("relative path for %s: %v", m, relErr)
}
files = append(files, rel)
}
}
return files
}

func atoi(t *testing.T, s string) int {
t.Helper()
n, err := strconv.Atoi(s)
if err != nil {
t.Fatal(fmt.Errorf("parsing %q: %w", s, err))
}
return n
}
2 changes: 1 addition & 1 deletion terraform/environments/aws/dev.tfvars.example
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ compute_platform = "lambda"
# Lambda settings (when compute_platform = "lambda")
lambda_architecture = "arm64" # arm64 for Graviton (cheaper)
lambda_memory_size = 512
lambda_timeout = 60 # Seconds
lambda_timeout = 300 # Seconds, 300-900 (covers the 270s exchange/purchase budget)
lambda_reserved_concurrency = -1 # -1 = no limit
lambda_log_retention_days = 30

Expand Down
11 changes: 9 additions & 2 deletions terraform/environments/aws/variables.tf
Original file line number Diff line number Diff line change
Expand Up @@ -229,9 +229,16 @@ variable "lambda_memory_size" {
}

variable "lambda_timeout" {
description = "Lambda timeout in seconds"
# Feeds module.compute_lambda's timeout; see the coupling note on that
# variable (terraform/modules/compute/aws/lambda/variables.tf, #913).
description = "Lambda timeout in seconds (300-900; must cover the 270s detached exchange/purchase budget)"
type = number
default = 30
default = 300

validation {
condition = var.lambda_timeout >= 300 && var.lambda_timeout <= 900
error_message = "lambda_timeout must be between 300 and 900 seconds: RI exchanges and purchases run up to 270s (4m execution + 30s terminal save) after the request, and Lambda cuts the invocation off at this timeout (#913)."
}
}


Expand Down
72 changes: 72 additions & 0 deletions terraform/modules/compute/aws/lambda/tests/timeout.tftest.hcl
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
# Lambda timeout bounds (#913). Lambda ends an invocation at its timeout
# whatever a context deadline says, so the function must outlive the 270s of
# RI exchange / purchase work detached from the request. The floor is pinned
# against the Go budgets by TestLambdaTimeoutCovers* in internal/api and
# internal/purchase.
mock_provider "aws" {}

variables {
stack_name = "cudly-test"
permissions_boundary_arn = "arn:aws:iam::123456789012:policy/cudly-boundary"
environment = "dev"
region = "us-east-1"
image_uri = "123456789012.dkr.ecr.us-east-1.amazonaws.com/cudly:test"
database_host = "db.example.com"
database_name = "cudly"
database_username = "cudly"
database_password_secret_arn = "arn:aws:secretsmanager:us-east-1:123456789012:secret:db"
admin_email = "admin@example.com"
allowed_origins = ["https://app.example.com"]
}

run "default_covers_detached_budget" {
command = plan

assert {
condition = aws_lambda_function.main.timeout == 300
error_message = "The default Lambda timeout must be 300s (270s detached budget + 30s margin)."
}
}

run "rejects_old_30s_default" {
command = plan

variables {
timeout = 30
}

expect_failures = [var.timeout]
}

run "rejects_just_below_floor" {
command = plan

variables {
timeout = 299
}

expect_failures = [var.timeout]
}

run "rejects_above_lambda_max" {
command = plan

variables {
timeout = 901
}

expect_failures = [var.timeout]
}

run "accepts_lambda_max" {
command = plan

variables {
timeout = 900
}

assert {
condition = aws_lambda_function.main.timeout == 900
error_message = "900s (the Lambda maximum) must be accepted."
}
}
17 changes: 15 additions & 2 deletions terraform/modules/compute/aws/lambda/variables.tf
Original file line number Diff line number Diff line change
Expand Up @@ -49,9 +49,22 @@ variable "memory_size" {
}

variable "timeout" {
description = "Lambda timeout in seconds"
# Must cover the longest work the handler detaches from the request: an RI
# exchange or purchase gets 4m of execution plus a 30s terminal save
# (riExchangeExecutionTimeout / purchaseExecutionTimeout + their terminal
# save budgets). Lambda ends the invocation at this timeout whatever the
# context deadline says, so a shorter value cuts that work off mid-flight
# (#913). 300 = 270s of budget + 30s for the request work before it.
# Pinned against the Go constants by TestLambdaTimeoutCoversRIExchangeBudget
# and TestLambdaTimeoutCoversPurchaseBudget.
description = "Lambda timeout in seconds (300-900; must cover the 270s detached exchange/purchase budget)"
type = number
default = 30
default = 300

validation {
condition = var.timeout >= 300 && var.timeout <= 900
error_message = "timeout must be between 300 and 900 seconds: RI exchanges and purchases run up to 270s (4m execution + 30s terminal save) after the request, and Lambda cuts the invocation off at this timeout (#913)."
}
}

variable "database_host" {
Expand Down
2 changes: 1 addition & 1 deletion terraform/profiles/aws/dev.tfvars.example
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ compute_platform = "lambda"
# Lambda configuration
lambda_architecture = "arm64"
lambda_memory_size = 512
lambda_timeout = 60
lambda_timeout = 300 # 300-900: covers the 270s exchange/purchase budget
lambda_reserved_concurrency = -1
lambda_log_retention_days = 7
# Function URL auth_type is derived from enable_cdn — see compute.tf local
Expand Down
2 changes: 1 addition & 1 deletion terraform/profiles/aws/prod.tfvars.example
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ compute_platform = "lambda" # or "fargate"
# Lambda configuration
lambda_architecture = "arm64"
lambda_memory_size = 1024 # More memory for production
lambda_timeout = 60
lambda_timeout = 300 # 300-900: covers the 270s exchange/purchase budget
lambda_reserved_concurrency = -1
lambda_log_retention_days = 30
# Function URL auth_type is derived from enable_cdn — see compute.tf local
Expand Down
Loading