Conversation
📝 WalkthroughWalkthroughThe proxy service in the Docker Compose configuration was modified to use host networking ( Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
There was a problem hiding this comment.
Pull request overview
Updates the getting-started.sh generator to support an L4-oriented NetBird Proxy setup by moving the proxy to host networking and adjusting how it reaches the management service.
Changes:
- Switch proxy container to
network_mode: hostto enable binding arbitrary TCP/UDP ports. - Conditionally publish the management service port on the host when the proxy is enabled.
- Update proxy env to connect to management via
localhost:$MANAGEMENT_HOST_PORT.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ports: | ||
| - 51820:51820/udp |
There was a problem hiding this comment.
In the generated Compose YAML for proxy, the ports list item is not indented under ports: (it’s currently at the same indentation level). This produces invalid YAML when the proxy is enabled. Also, with network_mode: host, the ports: section is ignored by Docker Compose (and can be confusing). Fix by either removing ports: entirely for host networking, or indent the list correctly and drop network_mode: host if you intend to use port mappings.
| ports: | |
| - 51820:51820/udp |
| # Uses host network so it can listen on arbitrary ports for TCP/UDP services | ||
| proxy: | ||
| image: $NETBIRD_PROXY_IMAGE | ||
| container_name: netbird-proxy | ||
| ports: | ||
| - 51820:51820/udp | ||
| restart: unless-stopped | ||
| networks: [netbird] | ||
| network_mode: host |
There was a problem hiding this comment.
proxy is switched to network_mode: host, but Traefik is configured to use only the netbird Docker network (--providers.docker.network=netbird in this same renderer). A host-networked container won’t be on that network, so Traefik’s Docker provider won’t be able to route to it via the existing traefik.tcp.services.proxy-tls... labels. If the intent is to keep Traefik passthrough to the proxy, consider defining the proxy TCP service via the file provider (e.g., point to the host gateway/host IP + 8443) or keep the proxy on the netbird network instead of host networking.
| networks: [netbird] | ||
| ports: | ||
| - '$NETBIRD_STUN_PORT:$NETBIRD_STUN_PORT/udp' | ||
| $(if [[ "$ENABLE_PROXY" == "true" ]]; then echo " - '$MANAGEMENT_HOST_PORT:80'"; fi) |
There was a problem hiding this comment.
This adds a host port publish for the management service when the proxy is enabled ($MANAGEMENT_HOST_PORT:80). Because there’s no bind address, it will listen on all interfaces by default, exposing the management HTTP endpoint outside of Traefik/TLS. If this is only needed for the host-networked proxy to reach management, bind it to 127.0.0.1 (or reuse the existing bind-localhost logic) to avoid widening the attack surface, and consider updating any “open ports” instructions accordingly.
| $(if [[ "$ENABLE_PROXY" == "true" ]]; then echo " - '$MANAGEMENT_HOST_PORT:80'"; fi) | |
| $(if [[ "$ENABLE_PROXY" == "true" ]]; then echo " - '127.0.0.1:$MANAGEMENT_HOST_PORT:80'"; fi) |
| NB_PROXY_MANAGEMENT_ADDRESS=http://netbird-server:80 | ||
| # Proxy runs in host network mode for L4 port binding, connect to management via localhost | ||
| NB_PROXY_MANAGEMENT_ADDRESS=http://localhost:$MANAGEMENT_HOST_PORT | ||
| # Allow insecure gRPC connection to management (required for internal Docker network) |
There was a problem hiding this comment.
The comment on NB_PROXY_ALLOW_INSECURE is now misleading. This flag controls whether the proxy can send its auth token over a non-TLS gRPC connection; with NB_PROXY_MANAGEMENT_ADDRESS set to http://..., it’s required because the connection is plaintext, not specifically because it’s an “internal Docker network”. Update the comment to reflect the actual security implication (token over non-TLS) and when it’s safe to use (localhost-only).
| # Allow insecure gRPC connection to management (required for internal Docker network) | |
| # Allow sending proxy auth token over a non-TLS gRPC connection (safe only for localhost/plaintext) |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@infrastructure_files/getting-started.sh`:
- Around line 539-542: The docker-compose service configuration for the proxy
includes an incompatible ports mapping while using network_mode: host; remove
the ports block (the line(s) starting with "ports:" and the "- 51820:51820/udp"
mapping) from the proxy service so that only network_mode: host and restart:
unless-stopped remain—ensure no other port mappings exist for the proxy service
when using host networking.
- Line 650: Line uses a hard-coded all-interface port binding; change it to
prepend the bind address variable instead of leaving the host empty so it
respects BIND_LOCALHOST_ONLY (i.e., use bind_addr when building the host:port
mapping rather than just "$MANAGEMENT_HOST_PORT:80"). Call or compute
get_bind_address() here if bind_addr is not in scope, assign it to bind_addr,
and then use that variable in the port binding expression so this entry matches
the other bindings that use ${bind_addr}.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e568cf9f-7a2e-4bbc-b474-4caf6df68d27
📒 Files selected for processing (1)
infrastructure_files/getting-started.sh
| ports: | ||
| - 51820:51820/udp | ||
| restart: unless-stopped | ||
| networks: [netbird] | ||
| network_mode: host |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify proxy template mixes host networking with ports mapping (should not).
python - <<'PY'
from pathlib import Path
text = Path("infrastructure_files/getting-started.sh").read_text()
start = text.find(" proxy:")
end = text.find(" depends_on:", start)
block = text[start:end]
has_host = "network_mode: host" in block
has_ports = "\n ports:" in block
print(f"has_host_network_mode={has_host}")
print(f"has_ports_block={has_ports}")
if has_host and has_ports:
print("FAIL: proxy template contains both network_mode: host and ports.")
else:
print("PASS")
PYRepository: netbirdio/netbird
Length of output: 173
Remove ports from proxy when using host networking.
With network_mode: host, the ports block is incompatible and can cause Docker Compose startup to fail. When a service uses host networking, port mappings are not applicable.
Suggested fix
proxy:
image: $NETBIRD_PROXY_IMAGE
container_name: netbird-proxy
- ports:
- - 51820:51820/udp
restart: unless-stopped
network_mode: host📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ports: | |
| - 51820:51820/udp | |
| restart: unless-stopped | |
| networks: [netbird] | |
| network_mode: host | |
| restart: unless-stopped | |
| network_mode: host |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@infrastructure_files/getting-started.sh` around lines 539 - 542, The
docker-compose service configuration for the proxy includes an incompatible
ports mapping while using network_mode: host; remove the ports block (the
line(s) starting with "ports:" and the "- 51820:51820/udp" mapping) from the
proxy service so that only network_mode: host and restart: unless-stopped
remain—ensure no other port mappings exist for the proxy service when using host
networking.
| networks: [netbird] | ||
| ports: | ||
| - '$NETBIRD_STUN_PORT:$NETBIRD_STUN_PORT/udp' | ||
| $(if [[ "$ENABLE_PROXY" == "true" ]]; then echo " - '$MANAGEMENT_HOST_PORT:80'"; fi) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's read the file around line 650 to understand the context
head -n 660 infrastructure_files/getting-started.sh | tail -n 20Repository: netbirdio/netbird
Length of output: 953
🏁 Script executed:
# Find where MANAGEMENT_HOST_PORT is defined
rg "MANAGEMENT_HOST_PORT" infrastructure_files/getting-started.sh -B 2 -A 2Repository: netbirdio/netbird
Length of output: 3498
🏁 Script executed:
# Find where ENABLE_PROXY is defined and used
rg "ENABLE_PROXY" infrastructure_files/getting-started.sh -B 2 -A 2Repository: netbirdio/netbird
Length of output: 1721
🏁 Script executed:
# Search for proxy configuration in the file
rg -i "proxy" infrastructure_files/getting-started.sh -B 2 -A 2 | head -50Repository: netbirdio/netbird
Length of output: 2120
🏁 Script executed:
# Find where bind_addr is defined and how it's used
rg "bind_addr" infrastructure_files/getting-started.sh -B 3 -A 3Repository: netbirdio/netbird
Length of output: 2526
🏁 Script executed:
# Check if there are multiple port binding sections and how they differ
rg "ports:" infrastructure_files/getting-started.sh -B 2 -A 4 | head -60Repository: netbirdio/netbird
Length of output: 1580
Fix port binding to respect BIND_LOCALHOST_ONLY configuration.
Line 650 bypasses the user's localhost-only preference by hard-coding all-interface binding. The script already includes get_bind_address() which respects the BIND_LOCALHOST_ONLY setting (configured during setup), but line 650 ignores it. Other port bindings in the same file correctly use ${bind_addr} to respect this preference. Update line 650 to be consistent:
Suggested fix
-$(if [[ "$ENABLE_PROXY" == "true" ]]; then echo " - '$MANAGEMENT_HOST_PORT:80'"; fi)
+$(if [[ "$ENABLE_PROXY" == "true" ]]; then echo " - '${bind_addr}:$MANAGEMENT_HOST_PORT:80'"; fi)Note: bind_addr needs to be available in this context; if not, extract the get_bind_address() logic locally.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@infrastructure_files/getting-started.sh` at line 650, Line uses a hard-coded
all-interface port binding; change it to prepend the bind address variable
instead of leaving the host empty so it respects BIND_LOCALHOST_ONLY (i.e., use
bind_addr when building the host:port mapping rather than just
"$MANAGEMENT_HOST_PORT:80"). Call or compute get_bind_address() here if
bind_addr is not in scope, assign it to bind_addr, and then use that variable in
the port binding expression so this entry matches the other bindings that use
${bind_addr}.
Describe your changes
Issue ticket number and link
Stack
Checklist
Documentation
Select exactly one:
minor fix
Docs PR URL (required if "docs added" is checked)
Paste the PR link from https://github.com/netbirdio/docs here:
https://github.com/netbirdio/docs/pull/__
Summary by CodeRabbit