feat: add workstation-wide active bastion routing - #129
Merged
Conversation
Adds JumpHostParams + ConnectionParams.jump_host so DeviceConnection can tunnel through an SSH jump box (Paramiko direct-tcpip channel handed to Netmiko via sock=) purely as network transport - no netops-toolkit code runs on the jump box. Enables running the toolkit natively on Windows (newer Python, no admin) while still reaching switches only reachable from an existing, unmaintained Linux bastion. - netops/core/connection.py: JumpHostParams dataclass, DeviceConnection wires sock= through _open_jump_channel() when jump_host is set, closes the paramiko client on disconnect(); resolve_jump_host_params() helper resolves inventory fields + vault credentials. - netops/core/inventory.py: Device gains jump_host/jump_port/jump_username/ jump_key_file fields, resolved via the existing defaults-merge pattern. - netops/check/health.py: wires resolve_jump_host_params() into the inventory-driven ConnectionParams build (reference call-site; other check/collect modules can adopt the same one-line pattern). - tests/test_jump_host.py: 15 new tests covering resolve_jump_host_params, tunneled vs non-tunneled DeviceConnection.connect()/disconnect() (mocked at the paramiko/netmiko seam, no live devices), inventory YAML parsing of jump-host fields, and the telnet+jump_host rejection path. - docs/guides/jump-host-tunnel.md: new guide with Windows setup, inventory config, vault credential wiring, and the creativity-skill design rationale (4 mechanisms considered, why in-process Paramiko channel won). - README.md: feature table + inventory format section updated.
Contributor
There was a problem hiding this comment.
Pull request overview
Adds workstation-wide SSH bastion routing while preserving per-device jump-host support.
Changes:
- Introduces an authenticated local SOCKS5 bastion service.
- Routes device, discovery, and Ansible connections through bastions.
- Adds inventory fields, documentation, and tests.
Reviewed changes
Copilot reviewed 21 out of 21 changed files in this pull request and generated 11 comments.
Show a summary per file
| File | Description |
|---|---|
netops/core/bastion.py |
Implements active bastion service and CLI. |
netops/core/connection.py |
Adds active and legacy bastion routing. |
netops/core/inventory.py |
Adds per-device jump-host fields. |
netops/inventory/scan.py |
Routes discovery through active bastions. |
netops/collect/config.py |
Applies inventory jump-host settings. |
netops/collect/backup.py |
Applies jump-host settings to backups. |
netops/check/vlan.py |
Adds bastion support to VLAN checks. |
netops/check/paloalto.py |
Adds bastion support to Palo Alto checks. |
netops/check/juniper.py |
Adds bastion support to Juniper checks. |
netops/check/health.py |
Adds bastion support to health checks. |
netops/check/cisco.py |
Adds bastion support to Cisco checks. |
netops/check/bgp.py |
Adds bastion support to BGP checks. |
netops/check/arista.py |
Adds bastion support to Arista checks. |
netops/ansible/modules/netops_facts.py |
Routes fact collection through active bastions. |
netops/ansible/modules/netops_command.py |
Routes commands through active bastions. |
netops/__main__.py |
Registers the bastion CLI command. |
tests/test_jump_host.py |
Tests legacy jump-host behavior. |
tests/test_active_bastion.py |
Tests active routing and SOCKS protocol helpers. |
README.md |
Advertises and demonstrates bastion support. |
docs/guides/jump-host-tunnel.md |
Documents per-device tunneling. |
docs/guides/active-bastion.md |
Documents workstation-wide routing. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+88
to
+92
| try: | ||
| response = _control_request(state, "status") | ||
| except OSError: | ||
| return None | ||
| return state if response.get("connected") is True else None |
Comment on lines
+161
to
+165
| active_socket = open_active_bastion_socket( | ||
| self.params.host, self.params.effective_port, self.params.timeout | ||
| ) | ||
| if active_socket is not None: | ||
| device_params["sock"] = active_socket |
Comment on lines
+442
to
+443
| "--token", | ||
| state.token, |
Comment on lines
+206
to
+207
| client = paramiko.SSHClient() | ||
| client.set_missing_host_key_policy(paramiko.AutoAddPolicy()) |
Comment on lines
+254
to
+263
| if self._connection: | ||
| self._connection.disconnect() | ||
| logger.info(f"Disconnected from {self.params.host}") | ||
| if self._jump_client is not None: | ||
| self._jump_client.close() # type: ignore[attr-defined] | ||
| self._jump_client = None | ||
| logger.info(f"Closed jump-host tunnel for {self.params.host}") | ||
| if self._active_bastion_socket is not None: | ||
| self._active_bastion_socket.close() # type: ignore[attr-defined] | ||
| self._active_bastion_socket = None |
Comment on lines
+300
to
+304
| channel = service.transport.open_channel(host, port, timeout=30) | ||
| self.request.sendall(b"\x05\x00\x00\x01\x00\x00\x00\x00\x00\x00") | ||
| self._bridge(channel) | ||
| except Exception as exc: | ||
| logger.debug("active bastion SOCKS request failed: %s", exc) |
Comment on lines
+361
to
+365
| """Translate a :class:`netops.core.inventory.Device`'s bastion fields. | ||
|
|
||
| Keeping this mapping at the connection boundary means every inventory-driven | ||
| command uses exactly the same tunnel semantics rather than independently | ||
| rebuilding a partial set of jump-host parameters. |
Comment on lines
+188
to
+190
| Three tunnel mechanisms were considered for the Windows→bastion→switch | ||
| path; a fourth (single shared/multiplexed bastion connection reused across | ||
| devices) was noted but not implemented in this pass: |
| temporary.replace(path) | ||
|
|
||
|
|
||
| def connect_active_bastion( |
| def _write_state(state: ActiveBastion, path: Path) -> None: | ||
| path.parent.mkdir(parents=True, exist_ok=True) | ||
| temporary = path.with_suffix(path.suffix + ".tmp") | ||
| temporary.write_text(json.dumps(asdict(state), indent=2), encoding="utf-8") |
… security Co-authored-by: kayodebristol <3579196+kayodebristol@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Validation