From c4fe67c5bf08b3a5c400d377596d14c97e79f9ee Mon Sep 17 00:00:00 2001 From: Sam Trimmer <73702205+SamTrimmer89@users.noreply.github.com> Date: Thu, 22 Oct 2026 17:21:54 +0100 Subject: [PATCH] feat(terraform): option to keep the AWS ALB private and limit the management API to one source range Adds alb_internal, alb_ingress_cidrs, management_source_cidrs and gateway_path_prefixes, all defaulting to the current behaviour --- terraform/litellm/aws/README.md | 29 +++- terraform/litellm/aws/alb.tf | 69 ++++++++- terraform/litellm/aws/locals.tf | 5 +- terraform/litellm/aws/network.tf | 8 +- .../litellm/aws/tests/alb_access.tftest.hcl | 142 ++++++++++++++++++ terraform/litellm/aws/variables.tf | 38 +++++ 6 files changed, 279 insertions(+), 12 deletions(-) create mode 100644 terraform/litellm/aws/tests/alb_access.tftest.hcl diff --git a/terraform/litellm/aws/README.md b/terraform/litellm/aws/README.md index bedb6f4e98b..5d59c05c3b8 100644 --- a/terraform/litellm/aws/README.md +++ b/terraform/litellm/aws/README.md @@ -8,7 +8,7 @@ Deploys the componentized LiteLLM proxy on AWS: - **S3 bucket** (private, versioned, SSE-S3) — exposed to gateway + backend as `S3_BUCKET_NAME` / `S3_REGION_NAME` for cache backend, request log archival, and `/v1/files` storage - **Secrets Manager** entries for `LITELLM_MASTER_KEY` (auto-generated, `sk-…`) and the Aurora master password (bootstrap-only) - **ECS Fargate cluster** running three services — `gateway`, `backend`, `ui` -- **Application Load Balancer** (public, HTTP/80) with path-based routing: +- **Application Load Balancer** (public by default, or internal with `alb_internal`) with path-based routing: - LLM data-plane prefixes (`/v1/chat/*`, `/v1/embeddings`, …) → `gateway` - UI assets (`/`, `/_next/*`, `/litellm-asset-prefix/*`, …) → `ui` - Everything else (management API: `/key/*`, `/user/*`, …) → `backend` @@ -549,6 +549,33 @@ Set `allow_plaintext_alb = true` in tfvars. Without this flag, plan fails with a clear error pointing at the precondition. Intended for short-lived trial / dev stacks only. +## Keeping the ALB private + +By default the ALB is internet-facing and open to `0.0.0.0/0`, and every +route that isn't a model or UI route reaches the management API. A gateway +that only services inside a VPC should call can be locked down with four +inputs, which all default to that current behaviour. + +```hcl +alb_internal = true +alb_ingress_cidrs = ["10.0.0.0/16"] +management_source_cidrs = ["10.0.5.10/32"] +gateway_path_prefixes = ["/v1/chat/*", "/chat/*", "/v1/models*", "/models*"] +``` + +`alb_internal` gives the ALB private addresses only. It still goes in +`public_subnet_ids`, which then just need to span two AZs. `alb_ingress_cidrs` +replaces `0.0.0.0/0` on ports 80 and 443. + +`management_source_cidrs` limits the backend and the UI to one source range, +such as a VPN. A request from anywhere else that matches no gateway route +gets a 403 from the ALB instead of reaching the backend. It takes one range +because an ALB rule allows five condition values and the UI prefix rule +already uses four. + +`gateway_path_prefixes` replaces the built-in list of gateway routes, for +example to expose only chat completions and the model list. + ## Storage and database retention Two opt-in tripwires guard against accidental data loss on diff --git a/terraform/litellm/aws/alb.tf b/terraform/litellm/aws/alb.tf index bb07a83caa7..e2d928e7148 100644 --- a/terraform/litellm/aws/alb.tf +++ b/terraform/litellm/aws/alb.tf @@ -1,7 +1,7 @@ resource "aws_lb" "this" { name = local.name load_balancer_type = "application" - internal = false + internal = var.alb_internal security_groups = [aws_security_group.alb.id] subnets = local.public_subnet_ids @@ -22,6 +22,8 @@ locals { # the path-routing rules and downgrade the 80 listener to a redirect. tls_enabled = var.acm_certificate_arn != "" rules_listener_arn = local.tls_enabled ? aws_lb_listener.https[0].arn : aws_lb_listener.http.arn + + management_restricted = length(var.management_source_cidrs) > 0 } # Target groups — one per component. IP target type because Fargate tasks @@ -99,7 +101,7 @@ resource "aws_lb_listener" "http" { protocol = "HTTP" default_action { - type = local.tls_enabled ? "redirect" : "forward" + type = local.tls_enabled ? "redirect" : (local.management_restricted ? "fixed-response" : "forward") dynamic "redirect" { for_each = local.tls_enabled ? [1] : [] @@ -110,7 +112,16 @@ resource "aws_lb_listener" "http" { } } - target_group_arn = local.tls_enabled ? null : aws_lb_target_group.backend.arn + dynamic "fixed_response" { + for_each = !local.tls_enabled && local.management_restricted ? [1] : [] + content { + content_type = "text/plain" + message_body = "Forbidden" + status_code = "403" + } + } + + target_group_arn = local.tls_enabled || local.management_restricted ? null : aws_lb_target_group.backend.arn } # Default-deny on the HTTP-only path: TLS is the supported posture. @@ -136,8 +147,18 @@ resource "aws_lb_listener" "https" { certificate_arn = var.acm_certificate_arn default_action { - type = "forward" - target_group_arn = aws_lb_target_group.backend.arn + type = local.management_restricted ? "fixed-response" : "forward" + + dynamic "fixed_response" { + for_each = local.management_restricted ? [1] : [] + content { + content_type = "text/plain" + message_body = "Forbidden" + status_code = "403" + } + } + + target_group_arn = local.management_restricted ? null : aws_lb_target_group.backend.arn } tags = local.tags @@ -159,6 +180,15 @@ resource "aws_lb_listener_rule" "ui_exact" { } } + dynamic "condition" { + for_each = local.management_restricted ? [1] : [] + content { + source_ip { + values = var.management_source_cidrs + } + } + } + tags = local.tags } @@ -178,6 +208,15 @@ resource "aws_lb_listener_rule" "ui_prefix" { } } + dynamic "condition" { + for_each = local.management_restricted ? [1] : [] + content { + source_ip { + values = var.management_source_cidrs + } + } + } + tags = local.tags } @@ -202,3 +241,23 @@ resource "aws_lb_listener_rule" "gateway" { tags = local.tags } + +resource "aws_lb_listener_rule" "management" { + count = local.management_restricted ? 1 : 0 + + listener_arn = local.rules_listener_arn + priority = 1000 + + action { + type = "forward" + target_group_arn = aws_lb_target_group.backend.arn + } + + condition { + source_ip { + values = var.management_source_cidrs + } + } + + tags = local.tags +} diff --git a/terraform/litellm/aws/locals.tf b/terraform/litellm/aws/locals.tf index c06b171d6bd..22327c11d1c 100644 --- a/terraform/litellm/aws/locals.tf +++ b/terraform/litellm/aws/locals.tf @@ -60,7 +60,7 @@ locals { # there is no Redis to share them through, so the ceiling is tasks x workers. max_gateway_processes = (var.gateway_autoscaling_enabled ? var.gateway_max_capacity : var.gateway_desired_count) * var.gateway_num_workers - gateway_path_prefixes = [ + default_gateway_path_prefixes = [ "/v1/chat/*", "/chat/*", "/v1/completions*", "/completions*", "/v1/embeddings*", "/embeddings*", @@ -120,5 +120,6 @@ locals { # ALB rules accept ≤ 5 path-pattern values per condition. Chunk the prefix # list so each chunk becomes one rule. - gateway_path_chunks = chunklist(local.gateway_path_prefixes, 5) + gateway_path_prefixes = var.gateway_path_prefixes != null ? var.gateway_path_prefixes : local.default_gateway_path_prefixes + gateway_path_chunks = chunklist(local.gateway_path_prefixes, 5) } diff --git a/terraform/litellm/aws/network.tf b/terraform/litellm/aws/network.tf index c54949b4a59..0891900692e 100644 --- a/terraform/litellm/aws/network.tf +++ b/terraform/litellm/aws/network.tf @@ -117,19 +117,19 @@ resource "aws_security_group" "alb" { vpc_id = local.vpc_id ingress { - description = "HTTP from anywhere" + description = "HTTP from alb_ingress_cidrs" from_port = 80 to_port = 80 protocol = "tcp" - cidr_blocks = ["0.0.0.0/0"] + cidr_blocks = var.alb_ingress_cidrs } ingress { - description = "HTTPS from anywhere" + description = "HTTPS from alb_ingress_cidrs" from_port = 443 to_port = 443 protocol = "tcp" - cidr_blocks = ["0.0.0.0/0"] + cidr_blocks = var.alb_ingress_cidrs } egress { diff --git a/terraform/litellm/aws/tests/alb_access.tftest.hcl b/terraform/litellm/aws/tests/alb_access.tftest.hcl new file mode 100644 index 00000000000..be482f77498 --- /dev/null +++ b/terraform/litellm/aws/tests/alb_access.tftest.hcl @@ -0,0 +1,142 @@ +mock_provider "aws" { + mock_data "aws_iam_policy_document" { + defaults = { + json = "{\"Version\":\"2012-10-17\",\"Statement\":[]}" + } + } +} +mock_provider "random" {} + +variables { + region = "eu-west-2" + tenant = "acme" + env = "test" + azs = ["eu-west-2a", "eu-west-2b"] + allow_plaintext_alb = true + image_tag = "v9.9.9" +} + +run "defaults_keep_the_public_alb" { + command = plan + + assert { + condition = aws_lb.this.internal == false + error_message = "The ALB must stay internet-facing unless alb_internal is set." + } + + assert { + condition = alltrue([ + for rule in aws_security_group.alb.ingress : toset(rule.cidr_blocks) == toset(["0.0.0.0/0"]) + ]) + error_message = "The ALB must stay open to everyone unless alb_ingress_cidrs is set." + } + + assert { + condition = aws_lb_listener.http.default_action[0].type == "forward" && length(aws_lb_listener_rule.management) == 0 + error_message = "Without management_source_cidrs the backend must stay the listener's default." + } + + assert { + condition = length(aws_lb_listener_rule.gateway) == ceil(length(local.default_gateway_path_prefixes) / 5) + error_message = "Without gateway_path_prefixes every built-in gateway route must be forwarded." + } +} + +run "internal_alb_with_narrowed_ingress" { + command = plan + + variables { + alb_internal = true + alb_ingress_cidrs = ["10.0.0.0/16"] + } + + assert { + condition = aws_lb.this.internal == true + error_message = "alb_internal must make the ALB internal." + } + + assert { + condition = alltrue([ + for rule in aws_security_group.alb.ingress : toset(rule.cidr_blocks) == toset(["10.0.0.0/16"]) + ]) + error_message = "alb_ingress_cidrs must replace 0.0.0.0/0 on both ALB ports." + } +} + +run "management_routes_answer_only_the_source_range" { + command = plan + + variables { + management_source_cidrs = ["10.0.5.10/32"] + } + + assert { + condition = alltrue([ + aws_lb_listener.http.default_action[0].type == "fixed-response", + aws_lb_listener.http.default_action[0].fixed_response[0].status_code == "403", + ]) + error_message = "With management_source_cidrs anything unmatched must get 403, not the backend." + } + + assert { + condition = alltrue([ + aws_lb_listener_rule.management[0].action[0].type == "forward", + aws_lb_listener_rule.management[0].priority > 100 + length(aws_lb_listener_rule.gateway), + one(aws_lb_listener_rule.management[0].condition).source_ip[0].values == toset(["10.0.5.10/32"]), + ]) + error_message = "The backend must be reached only from management_source_cidrs, after the gateway rules." + } + + assert { + condition = alltrue([ + for rule in [aws_lb_listener_rule.ui_exact, aws_lb_listener_rule.ui_prefix] : + length([for c in rule.condition : c if length(c.source_ip) > 0 && c.source_ip[0].values == toset(["10.0.5.10/32"])]) == 1 + ]) + error_message = "The UI must be reached only from management_source_cidrs." + } +} + +run "management_routes_on_the_https_listener" { + command = plan + + variables { + allow_plaintext_alb = false + acm_certificate_arn = "arn:aws:acm:eu-west-2:000000000000:certificate/test" + management_source_cidrs = ["10.0.5.10/32"] + } + + assert { + condition = alltrue([ + aws_lb_listener.http.default_action[0].type == "redirect", + aws_lb_listener.https[0].default_action[0].type == "fixed-response", + aws_lb_listener.https[0].default_action[0].fixed_response[0].status_code == "403", + ]) + error_message = "With TLS the 80 listener must still redirect and the 443 listener must answer 403 by default." + } +} + +run "more_than_one_management_cidr_is_refused" { + command = plan + + variables { + management_source_cidrs = ["10.0.5.10/32", "10.0.6.10/32"] + } + + expect_failures = [var.management_source_cidrs] +} + +run "gateway_routes_can_be_narrowed" { + command = plan + + variables { + gateway_path_prefixes = ["/v1/chat/*", "/chat/*", "/v1/embeddings*", "/embeddings*", "/v1/models*", "/models*"] + } + + assert { + condition = alltrue([ + length(aws_lb_listener_rule.gateway) == 2, + toset(flatten([for r in aws_lb_listener_rule.gateway : one(r.condition).path_pattern[0].values])) == toset(["/v1/chat/*", "/chat/*", "/v1/embeddings*", "/embeddings*", "/v1/models*", "/models*"]), + ]) + error_message = "gateway_path_prefixes must replace the built-in route list." + } +} diff --git a/terraform/litellm/aws/variables.tf b/terraform/litellm/aws/variables.tf index 9b7a279a907..4607423de14 100644 --- a/terraform/litellm/aws/variables.tf +++ b/terraform/litellm/aws/variables.tf @@ -543,6 +543,44 @@ variable "allow_plaintext_alb" { default = false } +# ---------- ALB access ---------- + +variable "alb_internal" { + description = "Make the ALB internal: private addresses only, never reachable from the internet. It still lands in `public_subnet_ids`, which then only need to be subnets in at least 2 AZs." + type = bool + default = false +} + +variable "alb_ingress_cidrs" { + description = "Source ranges allowed to the ALB on ports 80 and 443." + type = list(string) + default = ["0.0.0.0/0"] +} + +variable "management_source_cidrs" { + description = <<-EOT + When set, the backend (management API: /key/*, /spend/*, ...) and the UI + answer only requests from this range, and anything that matches no rule + gets 403. Gateway routes stay open to every source the ALB admits. Empty + ([]) keeps the backend as the listener's default for every source. + At most one CIDR: ALB rules allow 5 condition values and the UI prefix + rule already uses 4. + EOT + type = list(string) + default = [] + + validation { + condition = length(var.management_source_cidrs) <= 1 + error_message = "management_source_cidrs takes at most one CIDR: ALB rules allow 5 condition values and the UI prefix rule already uses 4." + } +} + +variable "gateway_path_prefixes" { + description = "Replaces the built-in list of paths the ALB forwards to the gateway. Null keeps the built-in list. With `management_source_cidrs` set, any path left out answers 403 to other sources." + type = list(string) + default = null +} + # ---------- RDS ---------- variable "skip_final_snapshot" {