diff --git a/README.md b/README.md index 51118cd..1839da5 100644 --- a/README.md +++ b/README.md @@ -29,28 +29,28 @@ Additional Steps: **Adaptive router VM count and interfaces** -Both the number of `IaaS Router` VMs and the number of private interfaces per router are dynamic, controlled by Terraform variables: +The number of `IaaS Router` VMs is controlled by the Terraform variable `router_count` (default `2`, tested with 3-4). Each router VM's private interfaces are driven by `private_network_cidrs` - a list of CIDR prefixes the admin must supply explicitly (no default, no auto-carving): **one entry = one shared private network = one private interface per router**, in addition to the single public (WAN) interface that stays fixed at 1. So a router VM ends up with `1 + length(private_network_cidrs)` network interfaces total. -- `router_count` (default `2`, tested with 3-4) - how many router VMs to provision. -- `private_interface_count` (default `2`) - how many isolated private interfaces each router VM gets, on top of the single public (WAN) interface that stays fixed at 1. +Each entry in `private_network_cidrs` is one network shared by *all* routers - every router gets its own port/IP inside every listed network (`cidrhost(cidr, router_index + 2)`), similar to how the original `lan_net` design worked, just generalized to an arbitrary number of networks and routers. There is no VRRP between router VMs in this design, a change from the previous 2-NIC/VRRP model. The prefixes must not overlap each other and must each have room for at least `router_count + 2` addresses - both checked by `terraform/tests/`. -So each router VM ends up with `1 + private_interface_count` network interfaces total (3 by default). Every private interface sits in its **own unique, isolated micro-subnet** (`/29` or `/28`, sized via `private_subnet_prefix_length`, carved out of `private_supernet`) - there is no shared LAN network or VRRP between router VMs in this design, a change from the previous 2-NIC/VRRP model. - -New Terraform variables: `router_count`, `private_interface_count`, `private_supernet`, `private_subnet_prefix_length`, `router_availability_zones` (see `terraform/variables.tf`). The post-install script is a Terraform template (`terraform/scripts/network-init.sh.tpl`) rendered per-router via `templatefile()`, matching each private interface to its expected subnet deterministically instead of guessing - it already handles any interface count, no hardcoded assumption of 2. `terraform/versions.tf` now pins the provider source (`vk-cs/vkcs`, `~> 0.17`), which was previously undeclared. +New Terraform variables: `router_count`, `private_network_cidrs`, `router_availability_zones` (see `terraform/variables.tf`). The post-install script is a Terraform template (`terraform/scripts/network-init.sh.tpl`) rendered per-router via `templatefile()`, matching each private interface to its expected subnet deterministically instead of guessing - it already handles any interface count, no hardcoded assumption of 2. `terraform/versions.tf` now pins the provider source (`vk-cs/vkcs`, `~> 0.17`), which was previously undeclared. **Horizontal scaling via environment variables** -Since `terraform.tfvars` doesn't set these two variables (only commented-out examples), they can be scaled purely through environment variables using Terraform's standard `TF_VAR_` convention - no wrapper scripts needed: +`router_count` has a default and isn't set in `terraform.tfvars`, so it scales purely through `TF_VAR_router_count` using Terraform's standard `TF_VAR_` convention. `private_network_cidrs` has no default and must be set somewhere - either in `terraform.tfvars` (as shipped) or overridden via `TF_VAR_private_network_cidrs` as a JSON-encoded list: ```bash export TF_VAR_router_count=4 -export TF_VAR_private_interface_count=3 +export TF_VAR_private_network_cidrs='["10.90.0.0/28","10.90.0.16/28","10.90.0.32/28"]' terraform apply ``` **Local delivery integrity tests** -`terraform/tests/` contains a local pytest suite that checks the delivery is internally consistent - required files present, `terraform fmt` clean, HCL parses, `router_count`/`private_interface_count` actually drive the resource/NIC count instead of being hardcoded, per-router private-subnet carving never overlaps (checked at several scales), and the post-install script template renders to valid bash. It also runs a real `terraform init` + `terraform validate` against the actual `vkcs` provider schema (at several `router_count`/`private_interface_count` values) - but against a project-local filesystem-mirror copy of the provider, so no cloud API is ever contacted and no credentials are needed (`validate` only type-checks against the provider's static schema). +`terraform/tests/` contains a local pytest suite that checks the delivery is internally consistent - required files present, `terraform fmt` clean, HCL parses, `router_count`/`private_network_cidrs` actually drive the resource/NIC count instead of being hardcoded, the example CIDRs in `terraform.tfvars` don't overlap and have room for `router_count` routers, and the post-install script template renders to valid bash. It also runs: + +- a real `terraform init` + `terraform validate` against the actual `vkcs` provider schema (at several `router_count`/`private_network_cidrs` values), against a project-local filesystem-mirror copy of the provider - no cloud API is ever contacted and no credentials are needed; +- a real `terraform plan` against an isolated, provider-free copy of just `variables.tf`, to prove the `validation { ... }` blocks on `router_count` and `private_network_cidrs` (non-empty, valid CIDR syntax, uniqueness) are actually enforced - `terraform validate` alone does **not** enforce custom variable validations for externally-supplied values, only `plan`/`apply` do. ```bash terraform/tests/setup-local-terraform.sh # one-time: provisions venv/ with terraform + the vkcs provider @@ -64,10 +64,6 @@ venv/bin/pytest terraform/tests -v - the `vk-cs/vkcs` provider binary, downloaded (with `SHA256SUMS` verification) directly from its [GitHub releases](https://github.com/vk-cs/terraform-provider-vkcs/releases) - this bypasses `registry.terraform.io`, which blocks some regions outright, and is what makes a real `terraform validate` possible at all here; - a project-local CLI config (`venv/terraform.d/cli-config.tfrc`) that points `terraform init` at that local provider copy via a `filesystem_mirror` block, instead of the network registry. -The default `private_supernet` (`10.90.0.0/16`) at `/29` sizing has room for 8192 per-router-per-interface micro-subnets, far more than any realistic `router_count × private_interface_count` combination. - -The default `private_supernet` (`10.90.0.0/16`) at `/29` sizing has room for 8192 per-router-per-interface micro-subnets, far more than any realistic `router_count × private_interface_count` combination. - >Note: the diagrams below (`ports.svg`, `topology.svg`) and the Ansible layer (`ansible/inventory.ini`, roles `base`/`frr_router`/`keepalived`) still describe/assume the previous 2-router, 2-NIC, VRRP-based design and have **not** been updated for the new N-NIC/N-router topology yet - that's a separate follow-up. **IPv4 addressing plan for the project** @@ -78,7 +74,7 @@ Here is a card to assist with configuration planning. The card is filled out usi **Terraform** -Provisions `router_count` `IaaS Routers` (default 2), each with 1 public and `private_interface_count` private ports (default 2). Includes supplimentary Shell script template (which is a part of Terraform manifest) to maintain configuration across reboots. +Provisions `router_count` `IaaS Routers` (default 2), each with 1 public and `length(private_network_cidrs)` private ports (2 in the shipped example). Includes supplimentary Shell script template (which is a part of Terraform manifest) to maintain configuration across reboots. **Ansible** diff --git a/docs/QUICKSTART.md b/docs/QUICKSTART.md index 8fab69e..93e0a1f 100644 --- a/docs/QUICKSTART.md +++ b/docs/QUICKSTART.md @@ -17,15 +17,22 @@ username = "<ваш VK Cloud логин>" password = "<пароль>" project_id = "<ваш project_id>" ssh_key_name = "<имя загруженного SSH-ключа>" + +private_network_cidrs = [ + "10.90.0.0/29", + "10.90.0.8/29", +] ``` +`private_network_cidrs` — обязательная переменная без значения по умолчанию: один CIDR-префикс на каждую приватную сеть, к которой будут подключены интерфейсы роутеров (один префикс = одна общая сеть = один приватный интерфейс на роутер). Автоматической нарезки нет — префиксы не должны пересекаться и должны вмещать минимум `router_count + 2` адреса. + ## 3. (Опционально) масштабирование -По умолчанию: 2 роутера × 3 интерфейса (1 публичный + 2 приватных). Меняется без правки кода — либо раскомментировать нужные строки в `terraform.tfvars`, либо через переменные окружения: +По умолчанию: 2 роутера × 3 интерфейса (1 публичный + 2 приватных, по числу префиксов в `private_network_cidrs`). Меняется без правки кода — либо через `terraform.tfvars`, либо через переменные окружения: ```bash export TF_VAR_router_count=4 -export TF_VAR_private_interface_count=3 +export TF_VAR_private_network_cidrs='["10.90.0.0/28","10.90.0.16/28","10.90.0.32/28"]' ``` ## 4. Развернуть @@ -41,7 +48,7 @@ terraform apply ## 5. Настроить Ansible -> ⚠️ Важно: `ansible/inventory.ini` и роли (`base`/`frr_router`/`keepalived`) пока жёстко рассчитаны на **2** роутера с интерфейсами `eth0`/`eth1` (VRRP-схема) — под новую N-роутерную/3-NIC архитектуру ещё не адаптированы. Для дефолтных значений (`router_count=2`, `private_interface_count=2`) впишите реальные `wan_ip`/`lan_ip`/GRE/BGP-параметры роутеров в `inventory.ini` вручную. При масштабировании выше 2 роутеров или интерфейсов Ansible-слой нужно дорабатывать отдельно. +> ⚠️ Важно: `ansible/inventory.ini` и роли (`base`/`frr_router`/`keepalived`) пока жёстко рассчитаны на **2** роутера с интерфейсами `eth0`/`eth1` (VRRP-схема) — под новую N-роутерную/N-NIC архитектуру ещё не адаптированы. Для дефолтных значений (`router_count=2`, 2 записи в `private_network_cidrs`) впишите реальные `wan_ip`/`lan_ip`/GRE/BGP-параметры роутеров в `inventory.ini` вручную. При масштабировании выше 2 роутеров или интерфейсов Ansible-слой нужно дорабатывать отдельно. ```bash cd ansible diff --git a/docs/changes/2026-09-03-explicit-private-network-cidrs-plan.md b/docs/changes/2026-09-03-explicit-private-network-cidrs-plan.md new file mode 100644 index 0000000..9ef76a3 --- /dev/null +++ b/docs/changes/2026-09-03-explicit-private-network-cidrs-plan.md @@ -0,0 +1,25 @@ +# План внедрения: явная передача префиксов приватных сетей + +Дата: 2026-09-03 + +## Проблема + +Приватные подсети роутеров нарезались автоматически (`private_supernet` + `private_subnet_prefix_length` + `private_interface_count` → `cidrsubnet()`). Администратор должен явно передавать все адресные префиксы — по одному CIDR на каждую приватную сеть, к которой подключаются маршрутизаторы. Автоматической нарезки быть не должно. + +Модель меняется: вместо N×router_count изолированных микроподсетей — N **общих** приватных сетей (по числу переданных префиксов), в каждой свой порт/IP на роутер (аналогично исходной `lan_net`/`lan_port1/2`, но обобщено на произвольное число сетей и роутеров). + +## Шаги + +1. `terraform/variables.tf`: удалить `private_supernet`, `private_subnet_prefix_length`, `private_interface_count`; добавить обязательную `private_network_cidrs` (list(string), без default, 3 validation-блока: непустой список, валидные CIDR, уникальность). +2. `terraform/main.tf`: `router_priv_net`/`router_priv_subnet` — `for_each` по ролям (одна сеть на роль); `router_priv_port` — `for_each` по (роутер×роль), `fixed_ip = cidrhost(role_cidr, router_index + 2)`. `templatefile()` берёт CIDR из `local.private_network_cidr[role]`. +3. `terraform/scripts/network-init.sh.tpl` — без изменений (сопоставление по CIDR-принадлежности не зависит от того, общий CIDR или уникальный). +4. `terraform.tfvars` — рабочий пример `private_network_cidrs` вместо удалённых переменных. +5. `terraform/tests/test_terraform_delivery.py` — обновить тесты под новую переменную/локали, убрать carving-коллизии, добавить проверку непересечения и вместимости CIDR. +6. `README.md`/`docs/QUICKSTART.md` — обновить описание. +7. Summary-документ по завершении. + +## Верификация + +- `terraform fmt -check -recursive`. +- Реальные `terraform init`/`validate` (локальный filesystem-mirror провайдера) на нескольких сочетаниях `router_count`/`private_network_cidrs`. +- `venv/bin/pytest terraform/tests -v`. diff --git a/docs/changes/2026-09-03-explicit-private-network-cidrs-summary.md b/docs/changes/2026-09-03-explicit-private-network-cidrs-summary.md new file mode 100644 index 0000000..d40f139 --- /dev/null +++ b/docs/changes/2026-09-03-explicit-private-network-cidrs-summary.md @@ -0,0 +1,47 @@ +# Summary: явная передача префиксов приватных сетей + +Дата: 2026-09-03 +План: [2026-09-03-explicit-private-network-cidrs-plan.md](2026-09-03-explicit-private-network-cidrs-plan.md) + +## Что сделано + +### `terraform/variables.tf` +Удалены `private_supernet`, `private_subnet_prefix_length`, `private_interface_count`. Добавлена обязательная (без default) переменная `private_network_cidrs` (`list(string)`) с тремя validation-блоками: список не пуст, каждый элемент — валидный IPv4 CIDR (`can(cidrhost(c, 0))`), элементы уникальны. + +### `terraform/main.tf` +Модель изменена: вместо N×router_count изолированных микроподсетей (по одной на каждую пару "роутер+роль", auto-carved через `cidrsubnet()`) — N **общих** приватных сетей, по одной на каждый CIDR из `private_network_cidrs`. `router_priv_net`/`router_priv_subnet` теперь `for_each` только по ролям (`local.private_network_cidr`, карта role→CIDR); `router_priv_port` — `for_each` по (роутер×роль), с `fixed_ip = cidrhost(role_cidr, router_index + 2)` (`.2`=router1, `.3`=router2, ...). `templatefile()` берёт CIDR напрямую из `local.private_network_cidr[role]`. + +### `terraform/scripts/network-init.sh.tpl` +Не менялся — сопоставление интерфейсов по CIDR-принадлежности не зависит от того, общий CIDR или уникальный per-роутер. + +### `terraform.tfvars` +Заменены закомментированные `private_supernet`/`private_subnet_prefix_length`/`private_interface_count` на рабочий (не закомментированный, т.к. обязательный) пример: +```hcl +private_network_cidrs = [ + "10.90.0.0/29", + "10.90.0.8/29", +] +``` + +### `terraform/tests/test_terraform_delivery.py` +Существенно переработан (32 теста вместо 32 — состав изменился): +- убраны carving-коллизии (`cidrsubnet`-реимплементация) — авто-нарезки больше нет; +- добавлен `test_tfvars_private_network_cidrs_do_not_overlap_and_have_room_for_routers` — офлайн-проверка (`ipaddress`) примера из `terraform.tfvars`: без пересечений, вмещает `router_count + 2` адресов; +- добавлены `test_private_network_cidrs_validation_is_enforced` и `test_router_count_validation_is_enforced` — **реальная** проверка срабатывания `validation`-блоков через `terraform plan` на изолированной копии одного `variables.tf` (без провайдера, без обращений к облаку); +- параметризованный `test_terraform_init_and_validate_against_real_provider_schema` обновлён под `private_network_cidrs` (JSON-список через `TF_VAR_*`). + +### `README.md` / `docs/QUICKSTART.md` +Убрано описание авто-нарезки и "8192 блоков" (в README также устранён случайно задвоенный абзац из предыдущего шага). Добавлено описание новой модели (общие сети, явные префиксы) и актуальный пример `TF_VAR_private_network_cidrs`. + +## Важная находка в процессе + +**`terraform validate` не проверяет пользовательские `validation { ... }`-блоки переменных** для значений, заданных извне (`-var`/`TF_VAR_*`/tfvars) — эмпирически подтверждено на минимальном изолированном примере (без единого resource/provider) с этим бинарём Terraform (v1.16.1): `validate` даёт `Success!` даже для заведомо невалидных значений (`router_count=0`, дублирующиеся CIDR, пустой список, некорректный CIDR-синтаксис), тогда как `terraform plan` с теми же значениями корректно завершается ошибкой `Invalid value for variable`. Это касается не только новой переменной, но и ранее написанной `router_count >= 1`. + +Из-за этого предыдущий вывод (в summary от 2026-09-03 про GitHub-зеркало провайдера) о том, что "реальный `terraform validate` подтверждает корректность конфигурации" был верен только в части совместимости со схемой провайдера — но не проверял ничего про custom variable validation. Это исправлено: теперь validation-блоки проверяются через `terraform plan` на изолированной копии `variables.tf` (без провайдера, без облака) — см. новые тесты выше. + +## Верификация + +- `venv/bin/terraform fmt -check -recursive` → чисто. +- Реальные `terraform init`/`validate` (через локальный filesystem-mirror провайдера) на 3 сочетаниях `router_count`/`private_network_cidrs`, включая дефолт из `terraform.tfvars` → все успешны. +- Реальный `terraform plan` на изолированном `variables.tf` — 4 негативных/позитивных сценария для `private_network_cidrs` и 3 для `router_count` → validation срабатывает корректно в обе стороны. +- `venv/bin/pytest terraform/tests -v` → **32 passed**. diff --git a/terraform/main.tf b/terraform/main.tf index f7ffe34..8a71a5a 100644 --- a/terraform/main.tf +++ b/terraform/main.tf @@ -105,53 +105,52 @@ resource "vkcs_networking_secgroup_rule" "router_ipsec_nat_t" { sdn = "sprut" } -# Per-router private interface subnets: each router gets `private_roles` isolated -# micro-subnets (no shared LAN, no VRRP), carved out of private_supernet. +# Per-router private interfaces: one shared private network per admin-supplied +# CIDR in private_network_cidrs (no shared LAN with the priv_srv_* segment, +# no VRRP), each router getting its own port/IP inside every such network. locals { - private_roles = [for i in range(var.private_interface_count) : "priv${i + 1}"] + private_roles = [for idx in range(length(var.private_network_cidrs)) : "priv${idx + 1}"] - router_private_subnets = { - for pair in setproduct(range(var.router_count), local.private_roles) : - "router${pair[0] + 1}-${pair[1]}" => { - router_index = pair[0] - role = pair[1] - cidr = cidrsubnet( - var.private_supernet, - var.private_subnet_prefix_length - tonumber(split("/", var.private_supernet)[1]), - pair[0] * length(local.private_roles) + index(local.private_roles, pair[1]) - ) - } + private_network_cidr = { + for idx, cidr in var.private_network_cidrs : + "priv${idx + 1}" => cidr } } resource "vkcs_networking_network" "router_priv_net" { - for_each = local.router_private_subnets + for_each = local.private_network_cidr name = "router-${each.key}-net" sdn = "sprut" admin_state_up = true } resource "vkcs_networking_subnet" "router_priv_subnet" { - for_each = local.router_private_subnets + for_each = local.private_network_cidr network_id = vkcs_networking_network.router_priv_net[each.key].id name = "router-${each.key}-subnet" - cidr = each.value.cidr - gateway_ip = cidrhost(each.value.cidr, 1) + cidr = each.value + gateway_ip = cidrhost(each.value, 1) sdn = "sprut" } resource "vkcs_networking_port" "router_priv_port" { - for_each = local.router_private_subnets + for_each = { + for pair in setproduct(range(var.router_count), local.private_roles) : + "router${pair[0] + 1}-${pair[1]}" => { + router_index = pair[0] + role = pair[1] + } + } name = "router-${each.key}-port" - network_id = vkcs_networking_network.router_priv_net[each.key].id + network_id = vkcs_networking_network.router_priv_net[each.value.role].id admin_state_up = true port_security_enabled = false full_security_groups_control = true security_group_ids = [] sdn = "sprut" fixed_ip { - subnet_id = vkcs_networking_subnet.router_priv_subnet[each.key].id - ip_address = cidrhost(each.value.cidr, 2) + subnet_id = vkcs_networking_subnet.router_priv_subnet[each.value.role].id + ip_address = cidrhost(local.private_network_cidr[each.value.role], each.value.router_index + 2) } } @@ -176,7 +175,7 @@ resource "vkcs_compute_instance" "router" { private_interfaces = [ for role in local.private_roles : { name = "eth${index(local.private_roles, role) + 1}" - cidr = local.router_private_subnets["router${count.index + 1}-${role}"].cidr + cidr = local.private_network_cidr[role] } ] }) @@ -186,7 +185,7 @@ resource "vkcs_compute_instance" "router" { uuid = data.vkcs_networking_network.extnet.id } - # Private: one pre-created isolated port per role + # Private: one pre-created port per role, into that role's shared network dynamic "network" { for_each = local.private_roles content { diff --git a/terraform/terraform.tfvars b/terraform/terraform.tfvars index 1ccc5d7..90e5184 100644 --- a/terraform/terraform.tfvars +++ b/terraform/terraform.tfvars @@ -3,10 +3,13 @@ password = "DemoUserPassw" project_id = "XXXXXd424998422XXXXXf13ac9XXXXX" ssh_key_name = "AdminSSH" -# Adaptive router VM count and per-router private interface sizing. -# All have defaults (see variables.tf) - uncomment to override. -# router_count = 3 -# private_supernet = "10.90.0.0/16" -# private_subnet_prefix_length = 29 -# router_availability_zones = ["ME1"] -# private_interface_count = 3 \ No newline at end of file +# Adaptive router VM count (has a default - see variables.tf - uncomment to override). +# router_count = 3 +# router_availability_zones = ["ME1"] + +# One CIDR per private network that router VMs get an interface into. +# No default - must be supplied explicitly. List order determines eth1..ethN. +private_network_cidrs = [ + "10.90.0.0/29", + "10.90.0.8/29", +] \ No newline at end of file diff --git a/terraform/tests/test_terraform_delivery.py b/terraform/tests/test_terraform_delivery.py index 9b89ad9..5fccdd9 100644 --- a/terraform/tests/test_terraform_delivery.py +++ b/terraform/tests/test_terraform_delivery.py @@ -1,30 +1,36 @@ """Local integrity tests for the terraform/ delivery. -None of these tests run terraform plan/apply or call any cloud API - no +None of these tests run terraform apply or call any cloud API - no credentials and no network access to VK Cloud itself are required or used. -`terraform init`/`validate` do run for real, but against a project-local -filesystem-mirror copy of the vkcs provider binary (see -setup-local-terraform.sh), so they only need the provider's static schema, -never a live cloud endpoint. Checks performed: - * the expected files exist, - * terraform fmt reports the tree as already formatted, - * the .tf files parse as valid HCL, - * `terraform init` + `terraform validate` succeed against the real vkcs - provider schema (via the local filesystem mirror - skipped if that - mirror hasn't been set up), - * router VM count and private-interface count are wired to variables - (not hardcoded), and those variables aren't shadowed by terraform.tfvars, - * the per-router/per-interface CIDR carving never produces overlapping - subnets, for a range of router_count / private_interface_count values, - * the post-install script template only contains the intended Terraform - interpolations and renders to syntactically valid bash. +Two different kinds of "real terraform" checks are used here, deliberately: + + * `terraform init` + `terraform validate` against the actual vkcs provider + schema (via a project-local filesystem-mirror copy of the provider + binary - see setup-local-terraform.sh) - this proves the config's + resource/attribute shapes are compatible with the real provider. + IMPORTANT: `terraform validate` does NOT enforce custom variable + `validation { ... }` blocks for externally-supplied values (verified + empirically against this terraform binary - only `plan`/`apply` do), so + it says nothing about whether e.g. private_network_cidrs is actually + checked for uniqueness. + * `terraform plan` against an isolated copy of just variables.tf (no + provider, no resources) to exercise those variable validation blocks + for real, entirely offline. + +Other checks performed: expected files exist, `terraform fmt` is clean, the +.tf files parse as valid HCL, router VM count and the private-network CIDR +list actually drive the resource/NIC count (not hardcoded), the example +CIDRs in terraform.tfvars don't overlap and have room for router_count +hosts, and the post-install script template only contains the intended +Terraform interpolations and renders to syntactically valid bash. Run via: venv/bin/pytest terraform/tests -v (first run terraform/tests/setup-local-terraform.sh once to provision the local terraform binary + vkcs provider mirror used by the init/validate test) """ import ipaddress +import json import os import re import shutil @@ -113,15 +119,15 @@ def test_terraform_fmt_clean(): @pytest.mark.parametrize( - "router_count,private_interface_count", + "router_count,private_network_cidrs", [ - (None, None), # defaults: 2, 2 - (1, 1), - (4, 3), + (None, None), # whatever terraform.tfvars already commits to + (1, ["10.90.0.0/29"]), + (4, ["10.90.0.0/28", "10.90.0.16/28", "10.90.0.32/28"]), ], ) def test_terraform_init_and_validate_against_real_provider_schema( - router_count, private_interface_count + router_count, private_network_cidrs ): """Real `terraform init` + `terraform validate` against the actual vkcs provider, using the project-local filesystem-mirror copy of the @@ -130,12 +136,13 @@ def test_terraform_init_and_validate_against_real_provider_schema( registry). Runs in a throwaway temp copy of terraform/ so it never leaves .terraform/ or .terraform.lock.hcl behind in the real delivery. No credentials are supplied and no cloud API is contacted - validate - only needs the provider's static schema to type-check the config. + only needs the provider's static schema to type-check the config (it + does not enforce the variable validation{} blocks - see + test_variable_validations_are_enforced_by_plan for that). - Parametrized over router_count/private_interface_count (set via - TF_VAR_*, exactly how horizontal scaling is meant to be driven) to - prove the delivery actually validates at other scales, not just the - defaults. + Parametrized over router_count/private_network_cidrs (set via TF_VAR_*, + exactly how horizontal scaling is meant to be driven) to prove the + delivery actually resolves at other scales, not just the defaults. """ assert TERRAFORM_BIN, "no terraform binary found (checked venv/bin and PATH)" if not CLI_CONFIG_FILE.is_file(): @@ -148,8 +155,8 @@ def test_terraform_init_and_validate_against_real_provider_schema( env["TF_CLI_CONFIG_FILE"] = str(CLI_CONFIG_FILE) if router_count is not None: env["TF_VAR_router_count"] = str(router_count) - if private_interface_count is not None: - env["TF_VAR_private_interface_count"] = str(private_interface_count) + if private_network_cidrs is not None: + env["TF_VAR_private_network_cidrs"] = json.dumps(private_network_cidrs) with tempfile.TemporaryDirectory() as tmp: tmp_path = Path(tmp) @@ -180,6 +187,70 @@ def test_terraform_init_and_validate_against_real_provider_schema( ) +def _plan_variables_only(var_overrides): + """Run `terraform plan` against an isolated copy of just variables.tf - + no provider, no resources, so this never touches any cloud API. Used to + exercise variable validation{} blocks for real: `terraform validate` + does not enforce them for externally-supplied values in this terraform + version (verified empirically), only `plan`/`apply` do. + + Returns (success: bool, combined stdout+stderr: str). + """ + assert TERRAFORM_BIN, "no terraform binary found (checked venv/bin and PATH)" + with tempfile.TemporaryDirectory() as tmp: + tmp_path = Path(tmp) + shutil.copy2(TERRAFORM_DIR / "variables.tf", tmp_path / "variables.tf") + + env = dict(os.environ) + env.pop("TF_CLI_CONFIG_FILE", None) + env.update( + { + "TF_VAR_username": "dummy", + "TF_VAR_password": "dummy", + "TF_VAR_project_id": "dummy", + "TF_VAR_ssh_key_name": "dummy", + "TF_VAR_private_network_cidrs": json.dumps(["10.90.0.0/29"]), + } + ) + env.update(var_overrides) + + init = subprocess.run( + [TERRAFORM_BIN, f"-chdir={tmp_path}", "init", "-backend=false", "-input=false"], + capture_output=True, + text=True, + env=env, + ) + assert init.returncode == 0, f"terraform init failed:\n{init.stdout}\n{init.stderr}" + + plan = subprocess.run( + [TERRAFORM_BIN, f"-chdir={tmp_path}", "plan", "-input=false"], + capture_output=True, + text=True, + env=env, + ) + return plan.returncode == 0, plan.stdout + plan.stderr + + +@pytest.mark.parametrize( + "cidrs,should_pass", + [ + (["10.90.0.0/29", "10.90.0.8/29"], True), + ([], False), # must contain at least one CIDR + (["not-a-cidr"], False), # must be a valid IPv4 CIDR + (["10.90.0.0/29", "10.90.0.0/29"], False), # must be unique + ], +) +def test_private_network_cidrs_validation_is_enforced(cidrs, should_pass): + ok, output = _plan_variables_only({"TF_VAR_private_network_cidrs": json.dumps(cidrs)}) + assert ok == should_pass, f"unexpected result for private_network_cidrs={cidrs!r}:\n{output}" + + +@pytest.mark.parametrize("count,should_pass", [(2, True), (0, False), (-1, False)]) +def test_router_count_validation_is_enforced(count, should_pass): + ok, output = _plan_variables_only({"TF_VAR_router_count": str(count)}) + assert ok == should_pass, f"unexpected result for router_count={count}:\n{output}" + + # --------------------------------------------------------------------------- # HCL parses cleanly # --------------------------------------------------------------------------- @@ -194,7 +265,7 @@ def test_hcl_file_parses(relpath): # --------------------------------------------------------------------------- -# variables.tf: the scaling knobs exist with sane defaults +# variables.tf: the scaling knobs exist as expected # --------------------------------------------------------------------------- @@ -205,24 +276,41 @@ def test_router_count_variable(): assert v["default"] == [2] -def test_private_interface_count_variable(): - v = find_variable(load_tf("variables.tf"), "private_interface_count") - assert v is not None, "variable private_interface_count is missing" - assert v["type"] == ["${number}"] - assert v["default"] == [2] +def test_private_network_cidrs_variable(): + v = find_variable(load_tf("variables.tf"), "private_network_cidrs") + assert v is not None, "variable private_network_cidrs is missing" + assert v["type"] == ["${list(string)}"] + assert "default" not in v, ( + "private_network_cidrs must NOT have a default - the admin is " + "required to pass it explicitly" + ) + assert len(v.get("validation", [])) >= 3, ( + "expected validations for: non-empty, valid CIDR syntax, uniqueness" + ) -@pytest.mark.parametrize("name", ["router_count", "private_interface_count"]) -def test_scaling_variable_not_pinned_in_tfvars(name): - """TF_VAR_ env-var overrides only take effect if terraform.tfvars - doesn't set an active (non-comment) value for the same variable.""" +def test_router_count_not_pinned_in_tfvars(): + """TF_VAR_router_count only takes effect if terraform.tfvars doesn't set + an active (non-comment) value for it.""" tfvars_text = (TERRAFORM_DIR / "terraform.tfvars").read_text() active_lines = [ line for line in tfvars_text.splitlines() if not line.strip().startswith("#") ] - assert not any(re.match(rf"^\s*{name}\s*=", line) for line in active_lines), ( - f"{name} must not be set in terraform.tfvars, or the " - f"TF_VAR_{name} environment variable would be shadowed" + assert not any(re.match(r"^\s*router_count\s*=", line) for line in active_lines), ( + "router_count must not be set in terraform.tfvars, or the " + "TF_VAR_router_count environment variable would be shadowed" + ) + + +def test_private_network_cidrs_is_set_in_tfvars(): + """Unlike router_count, private_network_cidrs has no default, so + terraform.tfvars must actively set it for a working example deployment.""" + tfvars_text = (TERRAFORM_DIR / "terraform.tfvars").read_text() + active_lines = [ + line for line in tfvars_text.splitlines() if not line.strip().startswith("#") + ] + assert any(re.match(r"^\s*private_network_cidrs\s*=", line) for line in active_lines), ( + "private_network_cidrs has no default and must be set in terraform.tfvars" ) @@ -257,8 +345,8 @@ def test_private_roles_local_driven_by_variable(): main = load_tf("main.tf") locals_block = main["locals"][0] assert locals_block["private_roles"] == [ - '${[for i in range(var.private_interface_count) : "priv${i + 1}"]}' - ], "locals.private_roles must be generated from var.private_interface_count" + '${[for idx in range(length(var.private_network_cidrs)) : "priv${idx + 1}"]}' + ], "locals.private_roles must be generated from length(var.private_network_cidrs)" def test_router_network_blocks_scale_with_private_roles(): @@ -267,51 +355,34 @@ def test_router_network_blocks_scale_with_private_roles(): dynamic_network = router["dynamic"][0]["network"] assert dynamic_network["for_each"] == ["${local.private_roles}"], ( "the dynamic private network blocks must iterate local.private_roles " - "so the NIC count scales with private_interface_count" + "so the NIC count scales with the number of private_network_cidrs entries" ) # --------------------------------------------------------------------------- -# CIDR carving: router_index/role_index -> netnum must never collide, for a -# range of router_count / private_interface_count combinations +# terraform.tfvars example CIDRs: sanity-check the values actually shipped +# (no auto-carving anymore - these come straight from the admin/example) # --------------------------------------------------------------------------- -def cidrsubnet(supernet, newbits, netnum): - """Pure-Python re-implementation of Terraform's cidrsubnet() built-in.""" - net = ipaddress.ip_network(supernet) - subnets = list(net.subnets(new_prefix=net.prefixlen + newbits)) - return subnets[netnum] +def test_tfvars_private_network_cidrs_do_not_overlap_and_have_room_for_routers(): + tfvars = load_tf("terraform.tfvars") + raw_cidrs = tfvars["private_network_cidrs"][0] + networks = [ipaddress.ip_network(c) for c in raw_cidrs] + assert networks, "terraform.tfvars must set at least one private_network_cidrs entry" + for i, a in enumerate(networks): + for b in networks[i + 1 :]: + assert not a.overlaps(b), f"{a} overlaps {b} in terraform.tfvars" -@pytest.mark.parametrize( - "router_count,private_interface_count", - [(1, 1), (2, 2), (3, 2), (4, 3), (8, 4), (1, 6)], -) -def test_private_subnet_carving_has_no_collisions(router_count, private_interface_count): - supernet = "10.90.0.0/16" - prefix_length = 29 - newbits = prefix_length - ipaddress.ip_network(supernet).prefixlen - - subnets = [ - cidrsubnet( - supernet, newbits, router_index * private_interface_count + role_index + router_count_default = find_variable(load_tf("variables.tf"), "router_count")["default"][0] + for net in networks: + # offset scheme: cidrhost(cidr, router_index + 2) for router_index in + # 0..router_count-1, so we need at least router_count + 2 addresses. + assert net.num_addresses >= router_count_default + 2, ( + f"{net} has too few addresses for {router_count_default} routers " + f"(offset scheme needs router_count + 2)" ) - for router_index in range(router_count) - for role_index in range(private_interface_count) - ] - - assert len(subnets) == len(set(subnets)), "duplicate per-router private subnets" - for i, a in enumerate(subnets): - for b in subnets[i + 1 :]: - assert not a.overlaps(b), f"{a} overlaps {b}" - - -def test_private_subnet_pool_capacity_is_generous(): - supernet = ipaddress.ip_network("10.90.0.0/16") - prefix_length = 29 - capacity = 2 ** (prefix_length - supernet.prefixlen) - assert capacity >= 1000, "default private_supernet/prefix combo has too little headroom" # --------------------------------------------------------------------------- diff --git a/terraform/variables.tf b/terraform/variables.tf index ae0daa8..5a9f358 100644 --- a/terraform/variables.tf +++ b/terraform/variables.tf @@ -36,36 +36,28 @@ variable "router_count" { } } -variable "private_supernet" { - description = "Address pool from which each router's private per-interface subnets are carved (must not overlap router-lan-subnet 10.200.10.0/24)" - type = string - default = "10.90.0.0/16" -} - -variable "private_subnet_prefix_length" { - description = "Prefix length of each router's private interface subnet (/29 or /28)" - type = number - default = 29 - - validation { - condition = contains([28, 29], var.private_subnet_prefix_length) - error_message = "private_subnet_prefix_length must be 28 or 29." - } -} - variable "router_availability_zones" { description = "Availability zones to spread router VMs across (cycled via count.index)" type = list(string) default = ["ME1"] } -variable "private_interface_count" { - description = "Number of isolated private interfaces per router VM (in addition to the single public/WAN interface)" - type = number - default = 2 +variable "private_network_cidrs" { + description = "Explicit CIDR prefix for each private network that router VMs get an interface into. One entry = one shared private network = one private interface per router (list order determines eth1..ethN). Must be supplied explicitly - no auto-carving from a supernet." + type = list(string) validation { - condition = var.private_interface_count >= 1 - error_message = "private_interface_count must be at least 1." + condition = length(var.private_network_cidrs) >= 1 + error_message = "private_network_cidrs must contain at least one CIDR." + } + + validation { + condition = alltrue([for c in var.private_network_cidrs : can(cidrhost(c, 0))]) + error_message = "Every entry in private_network_cidrs must be a valid IPv4 CIDR (e.g. \"10.90.0.0/29\")." + } + + validation { + condition = length(var.private_network_cidrs) == length(distinct(var.private_network_cidrs)) + error_message = "private_network_cidrs entries must be unique." } }