Replace auto-carved private subnets with explicit admin-supplied CIDRs
private_supernet/private_subnet_prefix_length/private_interface_count
(which auto-derived per-router-per-role micro-subnets via cidrsubnet())
are replaced by a single required variable, private_network_cidrs: one
CIDR per shared private network that router VMs get an interface into,
supplied explicitly by the admin - no auto-carving. Each router gets its
own port/IP inside every listed network (cidrhost(cidr, router_index+2)),
closer to the original lan_net design but generalized to N networks and
N routers. network-init.sh.tpl needed no changes - it already matches
interfaces by CIDR membership regardless of whether the CIDR is shared.
Also fixes a testing gap found along the way: `terraform validate` does
not enforce variable validation{} blocks for externally-supplied values
in this terraform version - only `plan`/`apply` do. The test suite now
exercises those validations for real via `terraform plan` against an
isolated, provider-free copy of variables.tf.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011hXR2ftXZZhJ4Y3XuSoR8r
This commit is contained in:
1 parent
5979a9a58b
commit
1ef4b12143
8 files changed
+290
-150
No files matched your search
@@ -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_<name> 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"
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
Reference in new issue
Block a user