Skip to content

feat(enroll-fw): enroll a firewall without ports and let inspection label them - #2619

Open
nidzrai wants to merge 1 commit into
mainfrom
enroll-fw-without-ports
Open

nidzrai wants to merge 1 commit into
mainfrom
enroll-fw-without-ports

Conversation

@nidzrai

@nidzrai nidzrai commented Oct 30, 2026

Copy link
Copy Markdown
Contributor

enroll a firewall without ports and let inspection label them

Kenny's PAN-OS inspection (#2564) finds a firewall's ports over LLDP and labels them outside/inside, so we shouldn't need to hand-write the ports JSON. But enroll-fw still required ports and made the node available straight away.

What does this change do?

enroll-fw now works two ways:

  • With ports (-p ports='[...]' -p physical_network=...): same as before. enroll-fw creates the ports, labels the two lowest interfaces outside/inside, and makes the node available.
  • Without ports:
    - enroll-fw creates the node
    - keeps it out of the pool
    - inspects it
    - checks it now has an outside and an inside port
    - then does it make the node available
    - If a role is missing, the node stays manageable with an error, so a router can't take an unlabelled firewall.
  • An active firewall: metadata only, either way. It never inspects a firewall that's in use.

also,

  • ports defaults to [] and physical_network to "" in the workflow.
  • physical_network is still required whenever ports are given.

WIP : need to test in dev

Upgrade impact

  • This change requires operator action to upgrade. If checked, add the
    upgrade-impact label and a release note: run scriv create from the
    repository root and describe the required action in the generated
    changelog.d/ file. See RELEASING.md.

Operator action means anything a deployment has to do beyond a normal resync:
deploy repo or values changes, new or removed secrets, enabling or disabling a
component, or a manual one-time step.

@github-actions

Copy link
Copy Markdown

Python test results

119 tests   - 10   119 ✅  - 10   1m 8s ⏱️ +56s
  1 suites ± 0     0 💤 ± 0 
  1 files   ± 0     0 ❌ ± 0 

Results for commit d6e1c1d. ± Comparison against base commit 68abc19.

This pull request removes 129 and adds 119 tests. Note that renamed tests count towards both.
tests.test_backup ‑ test_local_backs_up_mariadb_and_both_ovn_dbs
tests.test_backup ‑ test_local_mariadb_expands_password_inside_pod_not_locally
tests.test_backup ‑ test_local_ovn_uses_ovsdb_container_and_socket_paths
tests.test_backup ‑ test_local_skip_both_is_an_error
tests.test_backup ‑ test_local_surfaces_backup_exec_error
tests.test_backup ‑ test_slug_sanitizes_non_alnum
tests.test_cli ‑ test_bare_invocation_shows_usage_instead_of_missing_command_error
tests.test_cli ‑ test_bare_net_shows_usage_instead_of_missing_command_error
tests.test_cli ‑ test_bare_router_shows_usage_instead_of_missing_command_error
tests.test_cli ‑ test_callback_builds_connection_context
…
ironic_understack.tests.test_device_type ‑ test_device_type_from_directory
ironic_understack.tests.test_device_type ‑ test_device_type_from_yaml
ironic_understack.tests.test_device_type ‑ test_device_type_minimal_yaml
ironic_understack.tests.test_device_type ‑ test_get_resource_class
ironic_understack.tests.test_inspect_hook_chassis_model ‑ test_preserves_other_custom_traits
ironic_understack.tests.test_inspect_hook_chassis_model ‑ test_replaces_stale_chassis_trait
ironic_understack.tests.test_inspect_hook_node_name_check.TestInspectHookNodeNameCheck ‑ test_empty_system_vendor
ironic_understack.tests.test_inspect_hook_node_name_check.TestInspectHookNodeNameCheck ‑ test_matching_name_passes
ironic_understack.tests.test_inspect_hook_node_name_check.TestInspectHookNodeNameCheck ‑ test_mismatched_name_raises
ironic_understack.tests.test_inspect_hook_node_name_check.TestInspectHookNodeNameCheck ‑ test_missing_manufacturer_raises
…

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant