Skip to content

feat(nat): Extend NAT server into hetzner#6733

Merged
the-bokya merged 24 commits into
frappe:developfrom
20vikash:bastion
Jun 23, 2026
Merged

feat(nat): Extend NAT server into hetzner#6733
the-bokya merged 24 commits into
frappe:developfrom
20vikash:bastion

Conversation

@20vikash

Copy link
Copy Markdown
Collaborator
  • Previously, NAT servers, and removal of public IPs were only implemented in AWS and Frappe Compute
  • This PR extends that into hetzner.

@greptile-apps

greptile-apps Bot commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

Confidence Score: 5/5

Safe to merge — the Hetzner provisioning path is well-structured with correct cleanup on failure and all previously raised concerns have been addressed in the code.

The three major concerns raised in earlier review threads — unconditional cidr_block access, missing wait_until_finished on route deletion, and the raise placement relative to suppress — are all handled correctly in the current code. The remaining findings are minor: a return None dead-code line after frappe.throw() in nat_server.py, and a small window in create_nat_security_group_hetzner where a save() failure after a successful Hetzner API call could leave an orphaned firewall that gets duplicated on retry.

cluster.py — create_nat_security_group_hetzner has a narrow but real gap between Hetzner firewall creation and nat_security_group_id persistence.

Important Files Changed

Filename Overview
press/playbooks/roles/nat_iptables/tasks/main.yml Adds Hetzner-specific netplan task using primary_interface and network_gateway; conditions are correct and the netplan apply guard properly ORs both provider paths.
press/press/doctype/cluster/cluster.py Adds add_hetzner_nat_route (with proper wait_until_finished) and create_nat_security_group_hetzner; minor risk of orphaned Hetzner firewall if save() fails after successful API creation.
press/press/doctype/nat_server/nat_server.py Extends provider literal to include Hetzner; attach_nat_security_group correctly guards against non-AWS providers, though return None after frappe.throw() is unreachable dead code.
press/press/doctype/virtual_machine/virtual_machine.py NAT route provisioning with correct cleanup on failure; raise is outside the suppress block; get_security_groups() correctly includes nat_security_group_id so the Hetzner firewall IS applied at server creation.
press/press/doctype/server/server.py Adds cloud_provider and guarded network_gateway variables to both _setup_server and _install_nat_iptables; cidr_block access is correctly guarded with cluster.cidr_block truthiness check.
press/press/doctype/database_server/database_server.py Mirrors server.py pattern; network_gateway guarded by provider and cidr_block checks — no unconditional evaluation risk.
press/press/doctype/ip_removal_log/ip_removal_log.py Uses cluster.cloud_provider (correct field on Cluster doctype) and guarded network_gateway to extend NAT iptables playbook for Hetzner.
press/press/doctype/nat_server/nat_server.json Adds "Hetzner" to provider Select options; JSON reformatting and metadata bump only.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant CC as Cluster.provision_on_hetzner
    participant HZ as Hetzner API
    participant VM as VirtualMachine._provision_hetzner
    participant CL as Cluster.add_hetzner_nat_route
    participant AN as Ansible nat_iptables.yml

    CC->>HZ: Create private network (vpc_id)
    CC->>HZ: Create NAT firewall (nat_security_group_id)
    CC-->>CC: save nat_security_group_id

    VM->>HZ: "servers.create(firewalls=[nat_security_group_id])"
    HZ-->>VM: server object
    VM->>HZ: attach_to_network(private_ip)
    VM->>CL: add_hetzner_nat_route(private_ip)
    CL->>HZ: delete old 0.0.0.0/0 route (if exists) + wait
    CL->>HZ: add_route(0.0.0.0/0 to private_ip) + wait

    Note over VM: On any failure above: delete server, set Terminated

    VM->>AN: Run nat_iptables.yml
    AN->>AN: "Configure netplan (Hetzner path), gateway = cidr_block.network_address+1"
    AN->>AN: netplan apply
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
    participant CC as Cluster.provision_on_hetzner
    participant HZ as Hetzner API
    participant VM as VirtualMachine._provision_hetzner
    participant CL as Cluster.add_hetzner_nat_route
    participant AN as Ansible nat_iptables.yml

    CC->>HZ: Create private network (vpc_id)
    CC->>HZ: Create NAT firewall (nat_security_group_id)
    CC-->>CC: save nat_security_group_id

    VM->>HZ: "servers.create(firewalls=[nat_security_group_id])"
    HZ-->>VM: server object
    VM->>HZ: attach_to_network(private_ip)
    VM->>CL: add_hetzner_nat_route(private_ip)
    CL->>HZ: delete old 0.0.0.0/0 route (if exists) + wait
    CL->>HZ: add_route(0.0.0.0/0 to private_ip) + wait

    Note over VM: On any failure above: delete server, set Terminated

    VM->>AN: Run nat_iptables.yml
    AN->>AN: "Configure netplan (Hetzner path), gateway = cidr_block.network_address+1"
    AN->>AN: netplan apply
Loading

Reviews (15): Last reviewed commit: "Merge branch 'develop' into bastion" | Re-trigger Greptile

Comment thread press/playbooks/roles/nat_iptables/tasks/main.yml
@codecov-commenter

codecov-commenter commented Jun 17, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 11.47541% with 54 lines in your changes missing coverage. Please review.
✅ Project coverage is 50.68%. Comparing base (9661610) to head (e74f3ca).
⚠️ Report is 1 commits behind head on develop.

Files with missing lines Patch % Lines
press/press/doctype/cluster/cluster.py 6.25% 30 Missing ⚠️
...s/press/doctype/virtual_machine/virtual_machine.py 5.88% 16 Missing ⚠️
...ess/press/doctype/ip_removal_log/ip_removal_log.py 0.00% 3 Missing ⚠️
press/press/doctype/nat_server/nat_server.py 0.00% 3 Missing ⚠️
...s/press/doctype/database_server/database_server.py 50.00% 1 Missing ⚠️
press/press/doctype/server/server.py 75.00% 1 Missing ⚠️

❌ Your patch status has failed because the patch coverage (11.47%) is below the target coverage (75.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #6733      +/-   ##
===========================================
- Coverage    56.74%   50.68%   -6.07%     
===========================================
  Files          994      994              
  Lines        83939    83995      +56     
  Branches       682      527     -155     
===========================================
- Hits         47630    42569    -5061     
- Misses       36276    41393    +5117     
  Partials        33       33              
Flag Coverage Δ
dashboard 62.94% <ø> (-27.98%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Comment thread press/press/doctype/server/server.py Outdated
@greptile-apps

greptile-apps Bot commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

Want your agent to iterate on Greptile's feedback? Try greploops.

Comment thread press/press/doctype/cluster/cluster.py Outdated
Comment thread press/press/doctype/nat_server/nat_server.py
@20vikash 20vikash changed the title feat(nat): Extend NAT server into hetzner. feat(nat): Extend NAT server into hetzner Jun 23, 2026
@ssiyad ssiyad requested a review from Copilot June 23, 2026 07:44

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR extends the existing NAT-server + public-IP-removal workflow to Hetzner, aligning it with the behavior previously available only on AWS EC2 and Frappe Compute.

Changes:

  • Allow nat-series Virtual Machines on Hetzner, including provisioning without IPv4 when assign_public_ip is false and adding a Hetzner VPC default route via the NAT instance.
  • Pass Hetzner-specific networking parameters (cloud_provider, network_gateway) into Ansible runs that configure NAT routing.
  • Add Hetzner NAT firewall provisioning in Cluster and update NAT Server doctype/provider options for Hetzner.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 7 comments.

Show a summary per file
File Description
press/press/doctype/virtual_machine/virtual_machine.py Enables NAT VMs on Hetzner, controls public IPv4 assignment, adds Hetzner NAT route, and updates Hetzner sync behavior.
press/press/doctype/server/server.py Passes provider/gateway context to Ansible for NAT iptables and server setup flows.
press/press/doctype/nat_server/nat_server.py Extends NATServer provider typing and constrains AWS-only security group attachment action.
press/press/doctype/nat_server/nat_server.json Adds “Hetzner” to provider options and updates form JSON formatting/metadata.
press/press/doctype/ip_removal_log/ip_removal_log.py Passes Hetzner gateway context into NAT iptables playbook during IP-removal workflow.
press/press/doctype/database_server/database_server.py Passes provider/gateway context into DB server Ansible setup for Hetzner NAT routing.
press/press/doctype/cluster/cluster.py Adds Hetzner NAT route management and provisions a Hetzner NAT firewall.
press/playbooks/roles/nat_iptables/tasks/main.yml Adds Hetzner netplan handling and provider-aware conditional execution for NAT routing setup.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread press/press/doctype/virtual_machine/virtual_machine.py
Comment thread press/playbooks/roles/nat_iptables/tasks/main.yml
Comment thread press/playbooks/roles/nat_iptables/tasks/main.yml
Comment thread press/press/doctype/nat_server/nat_server.py
Comment thread press/press/doctype/server/server.py
Comment thread press/press/doctype/ip_removal_log/ip_removal_log.py
Comment on lines +406 to +409
def add_hetzner_nat_route(self, nat_ip):
from hcloud.networks.domain import NetworkRoute

client = self.get_hetzner_client()
Comment thread press/press/doctype/virtual_machine/virtual_machine.py Outdated
Comment thread press/press/doctype/virtual_machine/virtual_machine.py Outdated
@20vikash 20vikash enabled auto-merge June 23, 2026 10:30
@the-bokya the-bokya disabled auto-merge June 23, 2026 12:41
@the-bokya the-bokya merged commit 40d7e0a into frappe:develop Jun 23, 2026
12 of 14 checks passed
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.

5 participants