feat(rhdh): add base NetworkPolicies for RHDH deployment [RHIDP-16476] - #523
Draft
rm3l wants to merge 10 commits into
Draft
Conversation
Member
Author
|
/agentic_review |
Code Review by Qodo
1. Orchestrator permits all egress
|
Add default-deny + selective-allow NetworkPolicies for the RHDH backend and built-in PostgreSQL, replicating the operator's NP behavior (RHIDP-16475) in the Helm chart. Policies are always enabled to provide a secure-by-default deployment; customers can add additive NPs as needed. Backend NPs (always created): - default-deny (ingress + egress) - allow-dns-egress (53/5353 UDP+TCP) - allow-https-egress (443 TCP) - allow-psql-egress (5432 TCP to DB pods; broad egress for external DB) - allow-metrics-ingress (9464 TCP from monitoring namespaces) - allow-router-ingress (7007 TCP; OpenShift-aware namespace selector) DB NPs (only when postgresql.enabled=true): - db-default-deny, db-allow-dns-egress, db-allow-backend-ingress - Replication-aware: additional NPs for read replicas and primary-read communication when postgresql.architecture=replication Orchestrator NPs: - allow-all-egress (when orchestrator.enabled=true) Disables bitnami postgresql subchart's own NPs (primary + readReplicas) since they default to allow-all egress and unrestricted ingress, which is too permissive. Assisted-by: Claude
Move the Go template comment block before the conditional so that the */-}} whitespace trimming does not merge --- and apiVersion: onto the same line. Also remove the top-level comment that caused a similar issue with the first document. Assisted-by: Claude
Move the Go template comment before the conditional block so that whitespace trimming does not merge --- and apiVersion: onto the same line, which caused helm lint to reject the template. Assisted-by: Claude
Add rhdh.redhat.com/test-connection label to the test-connection pod and a dedicated allow-test-connection NP (gated on test.enabled) that permits the test pod to egress on port 7007 to the backend. Without this, default-deny blocks the test pod's connectivity check. Assisted-by: Claude
The test pod's egress NP alone is not enough; the backend also needs an ingress rule accepting connections on port 7007 from pods with the rhdh.redhat.com/test-connection label. Gated on test.enabled. Assisted-by: Claude
Mirror the comments from the operator repo so the namespace-wide podSelector concern and per-rule rationale are visible in the chart. Assisted-by: Claude
Redis is part of the recommended checklist for production deployments. RHDH does not deploy Redis OOTB; users bring their own, which could be in the same namespace, a different namespace, or an external managed service. The rule has no pod or namespace selector so it covers all cases. Ref: RHDHBUGS-3724 Assisted-by: Claude
…parator The Go template comment before allow-redis-egress used */ -}} which stripped the newline before the --- separator, merging the preceding NP's last line with --- and producing invalid YAML. Changed to */}} to preserve the newline. Assisted-by: Claude
Go template comments with whitespace trimming (e.g. */ -}}) have caused YAML separator merging bugs twice. YAML comments avoid this entirely; they survive rendering but Kubernetes ignores them. Assisted-by: Claude
The db-allow-replication NP allows the primary to egress to read replicas, but the read replica's default-deny blocks the incoming connection. Add db-read-allow-primary-ingress to accept ingress from the primary on port 5432. Assisted-by: Claude
rm3l
force-pushed
the
RHIDP-16476--implement-base-networkpolicies-for-rhdh-helm-chart
branch
from
September 5, 2026 21:19
d2f21fb to
a654f04
Compare
|
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
1 participant
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.
Description of the change
Add default-deny + selective-allow NetworkPolicies for the RHDH backend
and built-in PostgreSQL, replicating the operator's NP behavior
(RHIDP-16475, redhat-developer/rhdh-operator#3394) in the Helm chart.
Policies are always enabled (secure by default OOTB); customers can add
additive NPs as needed.
Disables the bitnami postgresql subchart's own NPs since they default to
allow-all egress and unrestricted ingress on 5432, which is too
permissive; our replacements are tighter.
Backend NPs (always created)
default-denyallow-dns-egressallow-https-egressallow-redis-egressallow-psql-egressallow-metrics-ingressallow-router-ingressTest-connection NPs (only when
test.enabled=true)allow-test-connectionallow-test-connection-ingressThe test-connection pod receives a dedicated
rhdh.redhat.com/test-connection: "true"labelso that NPs can target it without relying on Helm-managed labels.
DB NPs (only when
postgresql.enabled=true)db-default-denydb-allow-dns-egressdb-allow-backend-ingressdb-allow-replicationpostgresql.architecture=replication)db-read-default-denydb-read-allow-dns-egressdb-read-allow-primary-egressdb-read-allow-backend-ingressOrchestrator NPs (when
orchestrator.enabled=true)allow-all-egressExternal DB handling
When
postgresql.enabled=false, thedb-*NPs are omitted entirely.The
allow-psql-egresspolicy loses itstoselector (broadening toany destination) and uses
externalDatabase.portinstead of hardcoded5432.
Which issue(s) does this PR fix or relate to
How to test changes / Special notes to the reviewer
Verify with
helm templateacross scenarios:No bitnami postgresql NPs should appear in any scenario.
Checklist
Chart.yamlaccording to Semantic Versioning.values.yamland added to the corresponding README.md. The pre-commit utility can be used to generate the necessary content. Runpre-commit run --all-filesto run the hooks and then push any resulting changes. The pre-commit Workflow will enforce this and warn you if needed.pre-commithook.ct lintcommand.Assisted-by: Claude