From c3ae28bcb4132dfc3b1763c69df54bd4a6247998 Mon Sep 17 00:00:00 2001 From: Dan Halperin Date: Tue, 5 May 2026 12:15:20 -0700 Subject: [PATCH] junos_commit_check: add lab and test for Junos prefix normalization enforcement Add infrastructure to detect and regression-test which Junos configuration contexts reject unnormalized prefixes (host bits set) at commit time. Two layers: - infra/examples/junos-commit-check/: containerlab lab with commit_check_rejects and commit_check_accepts check types for empirical discovery on vJunos - snapshots/junos_commit_check/: CI regression test with per-host test_parse_warnings driven by validation/parse_warnings.yaml Empirical results from vJunos 25.4R1.12: Junos rejects host bits in static route, aggregate route, generate route, OSPF area-range, firewall from-address, firewall next-ip, and condition if-route-exists. Junos accepts host bits in prefix-list, route-filter, SNMP client-list, BGP allow, MPLS LSP install, interface address, and VGA tracking route. All 7 "rejects" cases are sickbayed pending Batfish implementation (batfish/batfish#9928 covers static routes only). Prompt: ``` Read https://github.com/batfish/batfish/pull/9928. The underlying issue is the ability to easily detect, automatically, when Junos will reject a specific piece of syntax. Design a repeatable procedure to do this, and apply it to this problem. ``` commit-id:82759006 --- infra/examples/junos-commit-check/README.md | 73 ++++++++ infra/examples/junos-commit-check/checks.yaml | 157 ++++++++++++++++++ .../junos-commit-check/configs/dut.cfg | 31 ++++ .../junos-commit-check/topology.clab.yml | 14 ++ lab_tests/test_labs.py | 88 ++++++++++ .../show_configuration_|_display_set.txt | 5 + .../show_configuration_|_display_set.txt | 2 + .../show_configuration_|_display_set.txt | 4 + .../show_configuration_|_display_set.txt | 2 + .../show_configuration_|_display_set.txt | 2 + .../show_configuration_|_display_set.txt | 2 + .../show_configuration_|_display_set.txt | 2 + .../show_configuration_|_display_set.txt | 2 + .../show_configuration_|_display_set.txt | 3 + .../show_configuration_|_display_set.txt | 2 + .../show_configuration_|_display_set.txt | 2 + .../show_configuration_|_display_set.txt | 2 + .../show_configuration_|_display_set.txt | 2 + .../junos_commit_check/show/host_nos.txt | 15 ++ .../validation/parse_warnings.yaml | 38 +++++ .../validation/sickbay.yaml | 57 +++++++ src/lab_builder/validate.py | 89 ++++++++++ 22 files changed, 594 insertions(+) create mode 100644 infra/examples/junos-commit-check/README.md create mode 100644 infra/examples/junos-commit-check/checks.yaml create mode 100644 infra/examples/junos-commit-check/configs/dut.cfg create mode 100644 infra/examples/junos-commit-check/topology.clab.yml create mode 100644 snapshots/junos_commit_check/configs/accepts-bgp-allow/show_configuration_|_display_set.txt create mode 100644 snapshots/junos_commit_check/configs/accepts-interface/show_configuration_|_display_set.txt create mode 100644 snapshots/junos_commit_check/configs/accepts-mpls-install/show_configuration_|_display_set.txt create mode 100644 snapshots/junos_commit_check/configs/accepts-prefix-list/show_configuration_|_display_set.txt create mode 100644 snapshots/junos_commit_check/configs/accepts-route-filter/show_configuration_|_display_set.txt create mode 100644 snapshots/junos_commit_check/configs/accepts-snmp/show_configuration_|_display_set.txt create mode 100644 snapshots/junos_commit_check/configs/rejects-aggregate/show_configuration_|_display_set.txt create mode 100644 snapshots/junos_commit_check/configs/rejects-condition/show_configuration_|_display_set.txt create mode 100644 snapshots/junos_commit_check/configs/rejects-firewall-address/show_configuration_|_display_set.txt create mode 100644 snapshots/junos_commit_check/configs/rejects-firewall-next-ip/show_configuration_|_display_set.txt create mode 100644 snapshots/junos_commit_check/configs/rejects-generate/show_configuration_|_display_set.txt create mode 100644 snapshots/junos_commit_check/configs/rejects-ospf-area-range/show_configuration_|_display_set.txt create mode 100644 snapshots/junos_commit_check/configs/rejects-static/show_configuration_|_display_set.txt create mode 100644 snapshots/junos_commit_check/show/host_nos.txt create mode 100644 snapshots/junos_commit_check/validation/parse_warnings.yaml create mode 100644 snapshots/junos_commit_check/validation/sickbay.yaml diff --git a/infra/examples/junos-commit-check/README.md b/infra/examples/junos-commit-check/README.md new file mode 100644 index 0000000..08273c4 --- /dev/null +++ b/infra/examples/junos-commit-check/README.md @@ -0,0 +1,73 @@ +# Junos Commit Check Lab + +Tests which Junos configuration syntax is rejected at commit time due to +unnormalized prefixes (host bits set). Results drive Batfish's +`fatalRedFlag` implementation for Junos prefix validation. + +## Background + +Junos performs "commit checks" that reject certain config at commit time +even though `set` commands accept it. For example, `set routing-options +static route 10.0.0.5/8 reject` is accepted by the CLI but rejected at +commit because the prefix has host bits set. + +Batfish models this with `fatalRedFlag` warnings (see batfish/batfish#9928). +This lab empirically determines which `ip_prefix` grammar contexts enforce +normalization and which do not. + +## Methodology + +Single vJunos-router node. For each grammar rule that accepts `ip_prefix` +or `ip_prefix_default_32`, we attempt to commit `192.168.1.111/24` (host +bits set) via `commit check`. The device either rejects it or accepts it. + +The `checks.yaml` uses two check types: + +- `commit_check_rejects`: asserts `commit check` fails +- `commit_check_accepts`: asserts `commit check` succeeds + +Each check loads config lines, runs `commit check`, and rolls back. + +## Running + +```bash +# On EC2 with containerlab + vJunos image: +sudo containerlab deploy -t topology.clab.yml +# Wait for health (~5 min) +python3 run_commit_checks.py checks.yaml 172.20.20.2 admin "admin@123" +``` + +## Results (vJunos 25.4R1.12) + +**Rejects host bits:** + +- `routing-options static route` +- `routing-options aggregate route` +- `routing-options generate route` +- `protocols ospf area X area-range` +- `firewall filter X term T from destination-address` +- `firewall filter X term T then next-ip` +- `policy-options condition X if-route-exists` + +**Accepts host bits:** + +- `policy-options prefix-list` +- `policy-options policy-statement X from route-filter` +- `snmp client-list` +- `protocols bgp group X allow` +- `protocols mpls label-switched-path X install` +- `interfaces X unit Y family inet address` +- `interfaces X unit Y family inet address A vrrp-group N track route` + +**Not tested (requires vSRX):** + +- `security nat` (pool address, match address, static-nat prefix) +- `security address-book` +- `security zones address-book` + +## Corresponding Regression Test + +The `snapshots/junos_commit_check/` snapshot contains one minimal config +per test case. The `test_parse_warnings` test in `lab_tests/test_labs.py` +asserts Batfish produces (or does not produce) fatal red flag warnings for +each, driven by `validation/parse_warnings.yaml`. diff --git a/infra/examples/junos-commit-check/checks.yaml b/infra/examples/junos-commit-check/checks.yaml new file mode 100644 index 0000000..2f9447a --- /dev/null +++ b/infra/examples/junos-commit-check/checks.yaml @@ -0,0 +1,157 @@ +# Junos commit check validation: tests whether Junos rejects unnormalized +# prefixes (host bits set) at commit time. +# +# Platform: vJunos-router (VMX). Security-hierarchy checks require vSRX. +# Reference: batfish/batfish#9928 + +checks: + # --- Contexts where Junos REJECTS host bits --- + + - type: commit_check_rejects + node: dut + config_lines: + - "set routing-options static route 192.168.1.111/24 reject" + description: "static route with host bits set" + + - type: commit_check_accepts + node: dut + config_lines: + - "set routing-options static route 192.168.1.0/24 reject" + description: "static route with valid prefix" + + - type: commit_check_rejects + node: dut + config_lines: + - "set routing-options aggregate route 192.168.1.111/24" + description: "aggregate route with host bits set" + + - type: commit_check_accepts + node: dut + config_lines: + - "set routing-options aggregate route 192.168.1.0/24" + description: "aggregate route with valid prefix" + + - type: commit_check_rejects + node: dut + config_lines: + - "set routing-options generate route 192.168.1.111/24" + description: "generate route with host bits set" + + - type: commit_check_accepts + node: dut + config_lines: + - "set routing-options generate route 192.168.1.0/24" + description: "generate route with valid prefix" + + - type: commit_check_rejects + node: dut + config_lines: + - "set protocols ospf area 0.0.0.0 area-range 192.168.1.111/24" + description: "OSPF area-range with host bits set" + + - type: commit_check_accepts + node: dut + config_lines: + - "set protocols ospf area 0.0.0.0 area-range 192.168.1.0/24" + description: "OSPF area-range with valid prefix" + + - type: commit_check_rejects + node: dut + config_lines: + - "set firewall filter TEST term T from destination-address 192.168.1.111/24" + - "set firewall filter TEST term T then accept" + description: "firewall destination-address with host bits set" + + - type: commit_check_accepts + node: dut + config_lines: + - "set firewall filter TEST term T from destination-address 192.168.1.0/24" + - "set firewall filter TEST term T then accept" + description: "firewall destination-address with valid prefix" + + - type: commit_check_rejects + node: dut + config_lines: + - "set firewall filter TEST term T then next-ip 192.168.1.111/24" + description: "firewall next-ip with host bits set" + + - type: commit_check_accepts + node: dut + config_lines: + - "set firewall filter TEST term T then next-ip 192.168.1.0/24" + description: "firewall next-ip with valid prefix" + + - type: commit_check_rejects + node: dut + config_lines: + - "set policy-options condition TEST if-route-exists 192.168.1.111/24 table inet.0" + description: "condition if-route-exists with host bits set" + + - type: commit_check_accepts + node: dut + config_lines: + - "set policy-options condition TEST if-route-exists 192.168.1.0/24 table inet.0" + description: "condition if-route-exists with valid prefix" + + # --- Contexts where Junos ACCEPTS host bits --- + + - type: commit_check_accepts + node: dut + config_lines: + - "set policy-options prefix-list TEST 192.168.1.111/24" + description: "prefix-list with host bits (accepted)" + + - type: commit_check_accepts + node: dut + config_lines: + - "set policy-options policy-statement TEST term T from route-filter 192.168.1.111/24 exact" + description: "route-filter with host bits (accepted)" + + - type: commit_check_accepts + node: dut + config_lines: + - "set policy-options policy-statement TEST term T from route-filter 10.0.0.0/8 through 192.168.1.111/24" + description: "route-filter through with host bits (accepted)" + + - type: commit_check_accepts + node: dut + config_lines: + - "set snmp client-list TEST 192.168.1.111/24" + description: "SNMP client-list with host bits (accepted)" + + - type: commit_check_accepts + node: dut + config_lines: + - "set protocols bgp group TEST type external" + - "set protocols bgp group TEST peer-as 65001" + - "set protocols bgp group TEST allow 192.168.1.111/24" + description: "BGP allow with host bits (accepted)" + + - type: commit_check_accepts + node: dut + config_lines: + - "set protocols mpls label-switched-path TEST to 2.2.2.2" + - "set protocols mpls label-switched-path TEST install 192.168.1.111/24" + - "set protocols mpls interface ge-0/0/0.0" + description: "MPLS LSP install with host bits (accepted)" + + - type: commit_check_accepts + node: dut + config_lines: + - "set interfaces ge-0/0/1 unit 0 family inet address 192.168.1.111/24" + description: "interface address with host bits (accepted)" + + - type: commit_check_accepts + node: dut + config_lines: + - "set interfaces ge-0/0/0 unit 0 family inet address 10.0.0.1/30 vrrp-group 1 virtual-address 10.0.0.2" + - "set interfaces ge-0/0/0 unit 0 family inet address 10.0.0.1/30 vrrp-group 1 track route 192.168.1.111/24 routing-instance default priority-cost 100" + description: "VRRP track route with host bits (accepted)" + +# TODO: Security hierarchy checks (require vSRX image) +# - security nat source pool

address +# - security nat source pool

address to +# - security nat rule match destination-address / source-address +# - security nat static rule then static-nat prefix +# - security address-book address +# - security zones security-zone address-book address diff --git a/infra/examples/junos-commit-check/configs/dut.cfg b/infra/examples/junos-commit-check/configs/dut.cfg new file mode 100644 index 0000000..23a03df --- /dev/null +++ b/infra/examples/junos-commit-check/configs/dut.cfg @@ -0,0 +1,31 @@ +system { + host-name dut; +} +interfaces { + ge-0/0/0 { + unit 0 { + family inet { + address 10.0.0.1/30; + } + } + } + lo0 { + unit 0 { + family inet { + address 1.1.1.1/32; + } + } + } +} +routing-options { + autonomous-system 65000; + router-id 1.1.1.1; +} +protocols { + ospf { + area 0.0.0.0 { + interface lo0.0; + interface ge-0/0/0.0; + } + } +} diff --git a/infra/examples/junos-commit-check/topology.clab.yml b/infra/examples/junos-commit-check/topology.clab.yml new file mode 100644 index 0000000..720eb56 --- /dev/null +++ b/infra/examples/junos-commit-check/topology.clab.yml @@ -0,0 +1,14 @@ +# Junos commit check validation lab: tests which config syntax Junos +# rejects at commit time (prefix normalization, etc.) +# +# Single node — no links needed. We just need a running Junos to SSH into +# and test commit checks against. + +name: junos-commit-check + +topology: + nodes: + dut: + kind: juniper_vjunosrouter + image: vrnetlab/juniper_vjunos-router:25.4R1.12 + startup-config: configs/dut.cfg diff --git a/lab_tests/test_labs.py b/lab_tests/test_labs.py index 55a772e..a80333b 100644 --- a/lab_tests/test_labs.py +++ b/lab_tests/test_labs.py @@ -445,6 +445,94 @@ def test_vi_model(bf: Session, snapshot: str) -> None: bf.q.viModel().answer() +@pytest.fixture(scope="module") +def parse_warning_spec(pytestconfig: Config) -> dict | None: + """Load parse_warnings.yaml if present.""" + import yaml + + lab = pytestconfig.getoption(LAB_NAME_CONFIG_OPTION) + warnings_path = snapshot_path(lab) / "validation" / "parse_warnings.yaml" + if not warnings_path.exists(): + return None + with open(warnings_path) as f: + return yaml.safe_load(f) + + +@pytest.fixture(scope="module") +def host_fatal_details( + bf: Session, snapshot: str, parse_warning_spec: dict | None +) -> dict[str, list[str]]: + """Map hostname -> list of fatal red flag warning detail strings.""" + if parse_warning_spec is None: + return {} + + bf.set_snapshot(snapshot) + issues = bf.q.initIssues().answer().frame() + + fatal_rows = issues[ + (issues["Type"] == "Parse warning (redflag)") + & (issues["Details"].str.startswith("FATAL:")) + ] + + result: dict[str, list[str]] = {} + for _, row in fatal_rows.iterrows(): + nodes = row.get("Nodes") + source_lines = row.get("Source_Lines") + if nodes: + for node in nodes: + result.setdefault(node, []).append(row["Details"]) + elif source_lines: + for file_lines in source_lines: + filename = ( + file_lines.filename + if hasattr(file_lines, "filename") + else str(file_lines) + ) + parts = filename.split("/") + if len(parts) >= 2 and parts[0] == "configs": + result.setdefault(parts[1], []).append(row["Details"]) + return result + + +def test_parse_warnings( + hostname: str, + vendor: Vendor, + parse_warning_spec: dict | None, + host_fatal_details: dict[str, list[str]], +) -> None: + """Tests that Batfish produces (or does not produce) fatal red flag warnings. + + Driven by validation/parse_warnings.yaml. Each host is tested individually + so failures can be sickbayed per-host. + """ + if parse_warning_spec is None: + pytest.skip("no validation/parse_warnings.yaml") + return + + expects_fatal = { + e["host"]: e["contains"] + for e in parse_warning_spec.get("expects_fatal_warning", []) + } + expects_no_fatal = set(parse_warning_spec.get("expects_no_fatal_warning", [])) + + if hostname in expects_fatal: + contains = expects_fatal[hostname] + details = host_fatal_details.get(hostname, []) + if not any(contains in d for d in details): + raise ValidationError( + f"expected fatal warning containing '{contains}' for '{hostname}' " + f"but got: {details}" + ) + elif hostname in expects_no_fatal: + if hostname in host_fatal_details: + raise ValidationError( + f"unexpected fatal warning for '{hostname}': " + f"{host_fatal_details[hostname]}" + ) + else: + pytest.skip(f"'{hostname}' not in parse_warnings.yaml") + + # TODO: Re-enable when reachability verification logic is ported from Batfish # def test_reachability_verifier(bf: Session, snapshot: str) -> None: # """Tests that the Reachability Verifier finds no bugs.""" diff --git a/snapshots/junos_commit_check/configs/accepts-bgp-allow/show_configuration_|_display_set.txt b/snapshots/junos_commit_check/configs/accepts-bgp-allow/show_configuration_|_display_set.txt new file mode 100644 index 0000000..fc200d2 --- /dev/null +++ b/snapshots/junos_commit_check/configs/accepts-bgp-allow/show_configuration_|_display_set.txt @@ -0,0 +1,5 @@ +set system host-name accepts-bgp-allow +set routing-options autonomous-system 65000 +set protocols bgp group TEST type external +set protocols bgp group TEST peer-as 65001 +set protocols bgp group TEST allow 192.168.1.111/24 diff --git a/snapshots/junos_commit_check/configs/accepts-interface/show_configuration_|_display_set.txt b/snapshots/junos_commit_check/configs/accepts-interface/show_configuration_|_display_set.txt new file mode 100644 index 0000000..b7dff71 --- /dev/null +++ b/snapshots/junos_commit_check/configs/accepts-interface/show_configuration_|_display_set.txt @@ -0,0 +1,2 @@ +set system host-name accepts-interface +set interfaces ge-0/0/0 unit 0 family inet address 192.168.1.111/24 diff --git a/snapshots/junos_commit_check/configs/accepts-mpls-install/show_configuration_|_display_set.txt b/snapshots/junos_commit_check/configs/accepts-mpls-install/show_configuration_|_display_set.txt new file mode 100644 index 0000000..129b993 --- /dev/null +++ b/snapshots/junos_commit_check/configs/accepts-mpls-install/show_configuration_|_display_set.txt @@ -0,0 +1,4 @@ +set system host-name accepts-mpls-install +set protocols mpls label-switched-path TEST to 2.2.2.2 +set protocols mpls label-switched-path TEST install 192.168.1.111/24 +set protocols mpls interface ge-0/0/0.0 diff --git a/snapshots/junos_commit_check/configs/accepts-prefix-list/show_configuration_|_display_set.txt b/snapshots/junos_commit_check/configs/accepts-prefix-list/show_configuration_|_display_set.txt new file mode 100644 index 0000000..3a5958c --- /dev/null +++ b/snapshots/junos_commit_check/configs/accepts-prefix-list/show_configuration_|_display_set.txt @@ -0,0 +1,2 @@ +set system host-name accepts-prefix-list +set policy-options prefix-list TEST 192.168.1.111/24 diff --git a/snapshots/junos_commit_check/configs/accepts-route-filter/show_configuration_|_display_set.txt b/snapshots/junos_commit_check/configs/accepts-route-filter/show_configuration_|_display_set.txt new file mode 100644 index 0000000..42aee82 --- /dev/null +++ b/snapshots/junos_commit_check/configs/accepts-route-filter/show_configuration_|_display_set.txt @@ -0,0 +1,2 @@ +set system host-name accepts-route-filter +set policy-options policy-statement TEST term T from route-filter 192.168.1.111/24 exact diff --git a/snapshots/junos_commit_check/configs/accepts-snmp/show_configuration_|_display_set.txt b/snapshots/junos_commit_check/configs/accepts-snmp/show_configuration_|_display_set.txt new file mode 100644 index 0000000..399b61a --- /dev/null +++ b/snapshots/junos_commit_check/configs/accepts-snmp/show_configuration_|_display_set.txt @@ -0,0 +1,2 @@ +set system host-name accepts-snmp +set snmp client-list TEST 192.168.1.111/24 diff --git a/snapshots/junos_commit_check/configs/rejects-aggregate/show_configuration_|_display_set.txt b/snapshots/junos_commit_check/configs/rejects-aggregate/show_configuration_|_display_set.txt new file mode 100644 index 0000000..723b708 --- /dev/null +++ b/snapshots/junos_commit_check/configs/rejects-aggregate/show_configuration_|_display_set.txt @@ -0,0 +1,2 @@ +set system host-name rejects-aggregate +set routing-options aggregate route 192.168.1.111/24 diff --git a/snapshots/junos_commit_check/configs/rejects-condition/show_configuration_|_display_set.txt b/snapshots/junos_commit_check/configs/rejects-condition/show_configuration_|_display_set.txt new file mode 100644 index 0000000..c738594 --- /dev/null +++ b/snapshots/junos_commit_check/configs/rejects-condition/show_configuration_|_display_set.txt @@ -0,0 +1,2 @@ +set system host-name rejects-condition +set policy-options condition TEST if-route-exists 192.168.1.111/24 table inet.0 diff --git a/snapshots/junos_commit_check/configs/rejects-firewall-address/show_configuration_|_display_set.txt b/snapshots/junos_commit_check/configs/rejects-firewall-address/show_configuration_|_display_set.txt new file mode 100644 index 0000000..2353640 --- /dev/null +++ b/snapshots/junos_commit_check/configs/rejects-firewall-address/show_configuration_|_display_set.txt @@ -0,0 +1,3 @@ +set system host-name rejects-firewall-address +set firewall filter TEST term T from destination-address 192.168.1.111/24 +set firewall filter TEST term T then accept diff --git a/snapshots/junos_commit_check/configs/rejects-firewall-next-ip/show_configuration_|_display_set.txt b/snapshots/junos_commit_check/configs/rejects-firewall-next-ip/show_configuration_|_display_set.txt new file mode 100644 index 0000000..9dd09b4 --- /dev/null +++ b/snapshots/junos_commit_check/configs/rejects-firewall-next-ip/show_configuration_|_display_set.txt @@ -0,0 +1,2 @@ +set system host-name rejects-firewall-next-ip +set firewall filter TEST term T then next-ip 192.168.1.111/24 diff --git a/snapshots/junos_commit_check/configs/rejects-generate/show_configuration_|_display_set.txt b/snapshots/junos_commit_check/configs/rejects-generate/show_configuration_|_display_set.txt new file mode 100644 index 0000000..729910e --- /dev/null +++ b/snapshots/junos_commit_check/configs/rejects-generate/show_configuration_|_display_set.txt @@ -0,0 +1,2 @@ +set system host-name rejects-generate +set routing-options generate route 192.168.1.111/24 diff --git a/snapshots/junos_commit_check/configs/rejects-ospf-area-range/show_configuration_|_display_set.txt b/snapshots/junos_commit_check/configs/rejects-ospf-area-range/show_configuration_|_display_set.txt new file mode 100644 index 0000000..286d40c --- /dev/null +++ b/snapshots/junos_commit_check/configs/rejects-ospf-area-range/show_configuration_|_display_set.txt @@ -0,0 +1,2 @@ +set system host-name rejects-ospf-area-range +set protocols ospf area 0.0.0.0 area-range 192.168.1.111/24 diff --git a/snapshots/junos_commit_check/configs/rejects-static/show_configuration_|_display_set.txt b/snapshots/junos_commit_check/configs/rejects-static/show_configuration_|_display_set.txt new file mode 100644 index 0000000..1a8f547 --- /dev/null +++ b/snapshots/junos_commit_check/configs/rejects-static/show_configuration_|_display_set.txt @@ -0,0 +1,2 @@ +set system host-name rejects-static +set routing-options static route 192.168.1.111/24 reject diff --git a/snapshots/junos_commit_check/show/host_nos.txt b/snapshots/junos_commit_check/show/host_nos.txt new file mode 100644 index 0000000..41e3319 --- /dev/null +++ b/snapshots/junos_commit_check/show/host_nos.txt @@ -0,0 +1,15 @@ +{ + "rejects-static": "junos", + "rejects-aggregate": "junos", + "rejects-generate": "junos", + "rejects-ospf-area-range": "junos", + "rejects-firewall-address": "junos", + "rejects-firewall-next-ip": "junos", + "rejects-condition": "junos", + "accepts-prefix-list": "junos", + "accepts-route-filter": "junos", + "accepts-snmp": "junos", + "accepts-bgp-allow": "junos", + "accepts-interface": "junos", + "accepts-mpls-install": "junos" +} diff --git a/snapshots/junos_commit_check/validation/parse_warnings.yaml b/snapshots/junos_commit_check/validation/parse_warnings.yaml new file mode 100644 index 0000000..3253df8 --- /dev/null +++ b/snapshots/junos_commit_check/validation/parse_warnings.yaml @@ -0,0 +1,38 @@ +# Expected parse warning assertions for this snapshot. +# +# Each entry specifies a host and whether Batfish should produce a fatal +# red flag warning ("Parse warning (redflag)" with "FATAL:" prefix in the +# Details column of initIssues output). +# +# For expects_fatal_warning, 'contains' specifies text that must appear in +# the warning Details, ensuring the warning is about the specific config +# line and not some unrelated issue. +# +# These assertions validate that Batfish correctly models Junos commit-time +# prefix normalization checks (batfish/batfish#9928). + +expects_fatal_warning: + # Junos rejects unnormalized prefixes in these contexts + - host: rejects-static + contains: "192.168.1.111/24" + - host: rejects-aggregate + contains: "192.168.1.111/24" + - host: rejects-generate + contains: "192.168.1.111/24" + - host: rejects-ospf-area-range + contains: "192.168.1.111/24" + - host: rejects-firewall-address + contains: "192.168.1.111/24" + - host: rejects-firewall-next-ip + contains: "192.168.1.111/24" + - host: rejects-condition + contains: "192.168.1.111/24" + +expects_no_fatal_warning: + # Junos accepts unnormalized prefixes in these contexts + - accepts-prefix-list + - accepts-route-filter + - accepts-snmp + - accepts-bgp-allow + - accepts-interface + - accepts-mpls-install diff --git a/snapshots/junos_commit_check/validation/sickbay.yaml b/snapshots/junos_commit_check/validation/sickbay.yaml new file mode 100644 index 0000000..0471b15 --- /dev/null +++ b/snapshots/junos_commit_check/validation/sickbay.yaml @@ -0,0 +1,57 @@ +# Per-host tests are sickbayed because the configs are intentionally minimal +# (designed to trigger or not trigger specific parse warnings). +entries: + - hostname: ".*" + test_name: test_configuration_format + skip: + skip_type: dont_run + reason: "commit check lab: configs are parse-warning test fixtures, not full devices" + - hostname: ".*" + test_name: test_interface_properties + skip: + skip_type: dont_run + reason: "commit check lab: configs are parse-warning test fixtures, not full devices" + - hostname: ".*" + test_name: test_main_rib_routes + skip: + skip_type: dont_run + reason: "commit check lab: configs are parse-warning test fixtures, not full devices" + - hostname: ".*" + test_name: test_bgp_rib_routes + skip: + skip_type: dont_run + reason: "commit check lab: configs are parse-warning test fixtures, not full devices" + - hostname: ".*" + test_name: test_evpn_rib_routes + skip: + skip_type: dont_run + reason: "commit check lab: configs are parse-warning test fixtures, not full devices" + # parse_warnings: Batfish does not yet flag unnormalized prefixes in these contexts + - hostname: rejects-static + test_name: test_parse_warnings + skip: + reason: "Batfish does not yet flag unnormalized static route prefixes (batfish/batfish#9928)" + - hostname: rejects-aggregate + test_name: test_parse_warnings + skip: + reason: "Batfish does not yet flag unnormalized aggregate route prefixes (batfish/batfish#9928)" + - hostname: rejects-generate + test_name: test_parse_warnings + skip: + reason: "Batfish does not yet flag unnormalized generate route prefixes (batfish/batfish#9928)" + - hostname: rejects-ospf-area-range + test_name: test_parse_warnings + skip: + reason: "Batfish does not yet flag unnormalized OSPF area-range prefixes (batfish/batfish#9928)" + - hostname: rejects-firewall-address + test_name: test_parse_warnings + skip: + reason: "Batfish does not yet flag unnormalized firewall address prefixes (batfish/batfish#9928)" + - hostname: rejects-firewall-next-ip + test_name: test_parse_warnings + skip: + reason: "Batfish does not yet flag unnormalized firewall next-ip prefixes (batfish/batfish#9928)" + - hostname: rejects-condition + test_name: test_parse_warnings + skip: + reason: "Batfish does not yet flag unnormalized condition if-route-exists prefixes (batfish/batfish#9928)" diff --git a/src/lab_builder/validate.py b/src/lab_builder/validate.py index 0b87506..febd512 100644 --- a/src/lab_builder/validate.py +++ b/src/lab_builder/validate.py @@ -257,6 +257,89 @@ def _arista_check_bgp_peer(node: NodeInfo, neighbor: str) -> CheckResult: ) +# --------------------------------------------------------------------------- +# Junos commit check validation +# --------------------------------------------------------------------------- + + +def _junos_commit_check(node: NodeInfo, config_lines: list[str]) -> tuple[bool, str]: + """Load config lines on a Junos device and run 'commit check'. + + Returns (success, output) where success is True if commit check passes. + Always rolls back after the check so the device stays clean. + """ + from lab_builder.device import connect + + conn = connect(node) + try: + conn.config_mode() + for line in config_lines: + stripped = line.strip() + if not stripped or stripped.startswith("#"): + continue + conn.send_command_timing(stripped) + output: str = conn.send_command_timing("commit check") + conn.send_command_timing("rollback 0") + conn.exit_config_mode() + + success = "configuration check succeeds" in output.lower() + return success, output.strip() + except Exception as e: + try: + conn.send_command_timing("rollback 0") + conn.exit_config_mode() + except Exception: + pass + return False, f"exception: {e}" + finally: + conn.disconnect() + + +def _check_commit_rejects( + node: NodeInfo, config_lines: list[str], expected_error: str | None +) -> CheckResult: + """Verify that Junos rejects the given config lines at commit check.""" + success, output = _junos_commit_check(node, config_lines) + if success: + return CheckResult( + "commit_check_rejects", + node.name, + False, + f"expected rejection but commit check succeeded: {output}", + ) + if expected_error and expected_error.lower() not in output.lower(): + return CheckResult( + "commit_check_rejects", + node.name, + False, + f"rejected but error text '{expected_error}' not found in: {output}", + ) + return CheckResult( + "commit_check_rejects", + node.name, + True, + f"correctly rejected: {output[:120]}", + ) + + +def _check_commit_accepts(node: NodeInfo, config_lines: list[str]) -> CheckResult: + """Verify that Junos accepts the given config lines at commit check.""" + success, output = _junos_commit_check(node, config_lines) + if success: + return CheckResult( + "commit_check_accepts", + node.name, + True, + "commit check succeeded", + ) + return CheckResult( + "commit_check_accepts", + node.name, + False, + f"expected acceptance but commit check failed: {output}", + ) + + # --------------------------------------------------------------------------- # Dispatch # --------------------------------------------------------------------------- @@ -288,6 +371,12 @@ def check_bgp_peer_established(node: NodeInfo, neighbor: str) -> CheckResult: "bgp_peer_established": lambda node, spec: check_bgp_peer_established( node, spec["neighbor"] ), + "commit_check_rejects": lambda node, spec: _check_commit_rejects( + node, spec["config_lines"], spec.get("expected_error") + ), + "commit_check_accepts": lambda node, spec: _check_commit_accepts( + node, spec["config_lines"] + ), }