Skip to content

make NodePort range configurable and add ability to disable it alltogether#732

Open
privatecoder wants to merge 1 commit intovitobotta:mainfrom
privatecoder:nodeport-improvements
Open

make NodePort range configurable and add ability to disable it alltogether#732
privatecoder wants to merge 1 commit intovitobotta:mainfrom
privatecoder:nodeport-improvements

Conversation

@privatecoder
Copy link
Contributor

I think @roy-hardin had a valid point in #474. Even though you wrote "Ports 30000-32767 are required for NodePort services, so they only pose a risk if you explicitly decide to expose a service to the Internet on a NodePort. Otherwise those ports are not in use so they don't pose any risk IMO." I think that opening the full NodePort range to source:any leaves an unnecessary attack surface.

Because: Even if we don’t plan to use NodePorts directly, sometimes they can appear implicitly (chart defaults, upgrades, misclicks) and would become publicly reachable immediately. Having an option to disable them or make them configurable keeps the flexibility while avoiding accidental exposure.

Small off-topic question: Did I get it right and if so: is it intentional to not use custom_firewall_rules for the custom firewall when using use_local_firewall: true?

…ewall creation alltogether, mirrored Hetzner firewall to local firewall (local firewall had only tcp but no udp rule)
@sonarqubecloud
Copy link

sonarqubecloud bot commented Feb 8, 2026

Comment on lines +185 to +188
if [ "$NODEPORT_FIREWALL_ENABLED" = "true" ]; then
iptables -A INPUT -p tcp --match multiport --dports $NODEPORT_RANGE -j ACCEPT
iptables -A INPUT -p udp --match multiport --dports $NODEPORT_RANGE -j ACCEPT
fi
Copy link
Contributor Author

@privatecoder privatecoder Feb 8, 2026

Choose a reason for hiding this comment

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

I found that with the Hetzner firewall we set TCP+UDP (which was added in #536) but with the custom firewall only tcp

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@vitobotta
Copy link
Owner

Thanks a lot for these PRs! You seem to be on fire :) One thing confuses me in the PR. You removed the firewall rules for the NodePort range, but you did not add anything to replace them when the Hetzner firewall is used. You did fix the rules for the custom firewall to include UDP, which I had missed. Is this on purpose?

One note: I am very busy with work right now. I normally do proper testing before I merge each PR and make a release, so it may take a while before I can merge this and other PRs.

@privatecoder
Copy link
Contributor Author

privatecoder commented Feb 8, 2026

You did fix the rules for the custom firewall to include UDP, which I had missed. Is this on purpose?

I added a comment in the file-changes (probably not the best place :D ) and well.. no-ish but I thought it would be more consistent.

@privatecoder
Copy link
Contributor Author

One thing confuses me in the PR. You removed the firewall rules for the NodePort range, but you did not add anything to replace them when the Hetzner firewall is used

I don't understand that – I did not remove them, but made them conditional (if node_port_firewall_enabled). Or what do you mean?

@privatecoder
Copy link
Contributor Author

One note: I am very busy with work right now. I normally do proper testing before I merge each PR and make a release, so it may take a while before I can merge this and other PRs.

Sure, I am happy with my local binary until then ;)

@vitobotta
Copy link
Owner

One thing confuses me in the PR. You removed the firewall rules for the NodePort range, but you did not add anything to replace them when the Hetzner firewall is used

I don't understand that – I did not remove them, but made them conditional (if node_port_firewall_enabled). Or what do you mean?

I wonder if it was a Github glitch because I see the rules now. Weird.

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.

2 participants

Comments