Skip to content

feat(): allow network policy#557

Open
Turgon37 wants to merge 2 commits into
SchwarzIT:mainfrom
Turgon37:main
Open

feat(): allow network policy#557
Turgon37 wants to merge 2 commits into
SchwarzIT:mainfrom
Turgon37:main

Conversation

@Turgon37

Copy link
Copy Markdown

Thank you for making node-red ⚙ better

Allow support for kube network policy :)

Signed-off-by: Pierre GINDRAUD <pgindraud@gmail.com>
@dirien

dirien commented Jun 1, 2026

Copy link
Copy Markdown
Collaborator

Review: Side Effects Analysis

Thanks for the contribution! The feature direction is good — NetworkPolicy support is a commonly requested addition. A few issues need addressing before this is safe to merge.


🔴 High: All ingress traffic blocked with default enabled: true

networkpolicy.yaml always emits both Egress and Ingress in policyTypes, but the default value for ingress is {} (empty map), which is falsy in Go templates — so no ingress rules are ever rendered with the defaults.

In Kubernetes, declaring Ingress in policyTypes with zero ingress rules means all ingress is denied. A user enabling the policy with just networkPolicy.enabled: true will find the Node-RED web UI (port 1880) immediately unreachable.

Suggested fix: Either conditionally include Ingress in policyTypes only when ingress rules are provided, or provide a working default ingress rule (e.g. allow port 1880), or add a prominent warning comment in values.yaml.


🟡 Medium: Default egress rules block most outbound traffic

The default egress only permits ports 1883/8883 (MQTT). Node-RED's palette manager, DNS resolution, and most user flows require broader outbound access. Enabling the policy with defaults will silently break those.

Consider either leaving egress empty by default (opt-in restriction) or documenting this behaviour very clearly.


🟡 Medium: Cilium flavor fails silently if Cilium is not installed

ciliumnetworkpolicy.yaml emits apiVersion: cilium.io/v2 / kind: CiliumNetworkPolicy. On a cluster without Cilium, helm upgrade will fail at apply time with a "no matches for kind" error. A note in the values comments would help users avoid this.


🔵 Low: YAML whitespace inconsistency in ciliumnetworkpolicy.yaml

networkpolicy.yaml correctly trims whitespace with {{-:

{{- toYaml .Values.networkPolicy.egress | nindent 4 }}

ciliumnetworkpolicy.yaml is missing the dash:

{{ toYaml .Values.networkPolicy.cilium.egress | nindent 4 }}

This leaves a stray blank line inside the rendered egress/ingress blocks. Valid YAML, but inconsistent with the rest of the chart.


🔵 Low: No tests, no schema validation, no docs

  • No Helm unit tests cover the new templates. A flavor: invalid value silently renders nothing with no error.
  • The flavor field has no values.schema.json constraint — an invalid value fails silently.
  • README.md / values documentation is not updated for the new networkPolicy block.

The most critical item is the ingress-blocking default. Once that is resolved (and the egress defaults reconsidered), this would be in good shape.

@pull-request-size pull-request-size Bot added size/L and removed size/M labels Jun 1, 2026
@Turgon37

Turgon37 commented Jun 1, 2026

Copy link
Copy Markdown
Author

Hello, thanks for your feedback, I've fixed some points related to your comment

Signed-off-by: Pierre GINDRAUD <pgindraud@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants