feat!: Major refactor with standardized labels, admin config restructure, doh replacement and security features#406
feat!: Major refactor with standardized labels, admin config restructure, doh replacement and security features#406madic-creates wants to merge 38 commits intoMoJo2600:nextfrom
Conversation
- Use full path /usr/local/bin/dnsprobe with correct arguments - Mount dnscrypt-proxy config as readOnly for security
Signed-off-by: madic-creates <3735459+madic-creates@users.noreply.github.com>
…ractices BREAKING CHANGE: Selector labels changed from app/release to app.kubernetes.io/name and app.kubernetes.io/instance. Existing deployments require full uninstall and reinstall. - Add pihole.labels and pihole.selectorLabels helper templates - Update all resources to use standardized labels: - helm.sh/chart - app.kubernetes.io/name - app.kubernetes.io/instance - app.kubernetes.io/version - app.kubernetes.io/managed-by - Apply consistent selector labels across all services, deployment, PDB, and PodMonitor
Add deprecation warnings to all loadBalancerIP and loadBalancerIPv6 fields in values.yaml and document the recommended migration path using cloud provider annotations (e.g., metallb.universe.tf/loadBalancerIPs for MetalLB users).
Replace hardcoded service types with dynamic values from values.yaml. Add context-aware access instructions for web interface based on service type (Ingress, LoadBalancer, NodePort, ClusterIP). Include conditional DHCP section, DNS access instructions, and improved kubectl commands with namespace flags.
Add startup probes that run before liveness/readiness probes begin, allowing containers sufficient time to initialize. Once startup probe succeeds, liveness and readiness probes take over. Changes: - Add startup probe configuration for main pihole container - Add startup probe configuration for DoH sidecar container - Reduce liveness/readiness initialDelaySeconds to 0 (startup probe handles delay) - Reduce liveness/readiness failureThreshold to 3 (faster failure detection) - Add DNS resolution check (dig) to probe commands
Add new DNS test that validates Pi-hole DNS functionality by running nslookup queries against the DNS service. Also add standardized Kubernetes labels to existing smoke test for consistency. New test: test-pihole-dns.yaml
Signed-off-by: madic-creates <3735459+madic-creates@users.noreply.github.com>
Add configurable security contexts for pods and containers with backwards compatibility for legacy privileged/capabilities values. Note: Pi-hole Docker image requires root at startup, so runAsNonRoot and similar restrictions are not supported. Detailed capability management provides limited security benefit due to Pi-hole's extensive requirements.
Add optional NetworkPolicy template to control ingress and egress traffic: - Ingress: allows HTTP/HTTPS, DNS (TCP/UDP), DHCP, and DoH ports - Egress: allows DNS resolution and HTTP/HTTPS for ad list updates - Disabled by default, configurable via values.yaml
BREAKING CHANGE: The root-level adminPassword value has been moved into the admin section. Update your values files from adminPassword: "xxx" to admin.password: "xxx".
Add troubleshooting section to README explaining the "pihole-FTL: no process found" error when using hostNetwork, caused by port conflicts. Include complete dhcphelper example deployment with: - Pi-hole values for use without hostNetwork - DHCP helper Deployment (and optional DaemonSet) configuration - Comprehensive documentation explaining the Layer 2 broadcast problem and the relay solution using homeall/dhcphelper
BREAKING CHANGE: The following values have been renamed: - whitelist -> allowed - blacklist -> denied The ConfigMap names and internal references have been updated accordingly. Users must update their values files to use the new key names. This change adopts more inclusive and descriptive terminology.
Add missing Kubernetes probe parameters to allow fine-tuning of liveness and readiness probe intervals and success thresholds. Signed-off-by: madic-creates <3735459+madic-creates@users.noreply.github.com>
- Only render hostname when explicitly set to avoid empty value - Only render hostNetwork when set and not false - Only set FTLCONF_webserver_api_password when admin.enabled is true - Use new probe parameters (periodSeconds, successThreshold) - Remove initialDelaySeconds from liveness/readiness (only relevant for startup) Signed-off-by: madic-creates <3735459+madic-creates@users.noreply.github.com>
Replace curlimages/curl with busybox:stable which has nslookup built-in and fewer container restrictions. Also change test domain from google.com to cloudflare.com.
Signed-off-by: madic-creates <3735459+madic-creates@users.noreply.github.com>
…ration guide Signed-off-by: madic-creates <3735459+madic-creates@users.noreply.github.com>
Migrate example values files to use the new admin.password configuration structure, aligning with the breaking change introduced in the chart. Signed-off-by: madic-creates <3735459+madic-creates@users.noreply.github.com>
…false Signed-off-by: madic-creates <3735459+madic-creates@users.noreply.github.com>
Replace hardcoded port values (53 for DNS, 67 for DHCP) with configurable values from serviceDns.port and serviceDhcp.port in deployment and service templates. Inspired by MoJo2600#401
Replace hardcoded port values (53 for DNS, 67 for DHCP) with configurable values from serviceDns.port and serviceDhcp.port in deployment and service templates. Inspired by MoJo2600#401
|
Ah, no. Worked on another machine without configured git signing. Shame on me 🙈 |
Without podAffinity, DHCP relay fails when dhcphelper and Pi-hole pods run on different nodes due to cross-node networking issues (UDP frames may not reach Pi-hole or source IP gets rewritten by internal NAT). Also removes the commented-out DaemonSet alternative which is no longer relevant with the required same-node constraint.
|
You guys are awesome! Thanks for the work! |
- Run dhcp-helper as unprivileged user (nobody/65534) - Drop all capabilities instead of requiring NET_ADMIN - Use alternate ports 1067/1068 instead of privileged 67/68 - Configure dnsmasq with dhcp-alternate-port and dhcp-broadcast - Document tc rules prerequisite (user responsibility) - Switch to ghcr.io/slyrc/dhcp-helper:1.3.0 image Thanks to https://github.com/Slyrc/dhcp-helper-container for the rootless dhcp-helper implementation and documentation. Signed-off-by: madic-creates <3735459+madic-creates@users.noreply.github.com>
| # Use ports 1067/1068 instead of privileged ports 67/68 | ||
| - dhcp-alternate-port | ||
| # Send DHCP responses as broadcast (required when dhcp-helper relays requests, | ||
| # since the client MAC is not yet in the ARP table) |
There was a problem hiding this comment.
This is needed because we don't want to use CAP_NET_ADMIN. With the broadcast flag we always reply via broadcast. Even if the client did not requested it.
There was a problem hiding this comment.
Setting this forces the dhcp helper to reply with boradcast and skipping this part of the code: https://github.com/Slyrc/dhcp-helper-container/blob/3a61309bec175a42a47e490fa7381c7e9a88cbc5/src/dhcp-helper.c#L588 -- Without it it is going in there and needs NET_ADMIN in order to perform it.
| {{- end }} | ||
| ports: | ||
| - containerPort: {{ .Values.webHttp }} | ||
| name: http |
There was a problem hiding this comment.
General point here. When for example using 8080 for web so that we can get rid of NET_BIND_SERVICE we need to adjust health checks in the image here. I saw that there are hardcoded ports in there.
|
@madic-creates -- I added my review. We should also consider checking HELM 4. https://helm.sh/blog/helm-4-released/#helm-v3-support -- Helm 3 supports seems to end soon |
Thank you for the extensive review. It seems that I should migrate my dhcphelper setup to the example to be able to realy test that deployment. Will also take a look at helm 4. Haven't done any comparison until now. So thank you for the hint that v3 is going to be eol. |
…defaults - Move hardcoded probe commands from values.yaml to deployment.yaml templates - Probes now dynamically use webHttp/dnsPort for pihole and doh.port for dnscrypt-proxy - Document default commands as comments in values.yaml for user reference - Allow custom commands via probes.*.command and doh.probes.*.probe overrides
…tation - Add warning about Traffic Control complexity and risks - Recommend production-ready tc.sh script from Slyrc/dhcp-helper-container - Update dhcphelper args to use -d flag for rootless operation - Improve troubleshooting with tc -s for packet statistics - Remove unnecessary dhcp-broadcast dnsmasq option
|
This is a really great initiative! I am also working on some breaking changes regarding services/dualstack etc, seems like a good idea to put all of that in the 'next' branch indeed so we can combine a lot of those together! |
|
I did some investigation regarding helm v4 (especially on https://helm.sh/docs/overview). Helm v4 fully supports v2 charts without any plan to dismiss v2 support. Besides that, there doesn't seem any change in this chart required. |
Description of the change
This branch brings significant changes to the Pi-hole Helm chart, including:
Breaking changes:
Features:
Benefits
See included CHANGELOG.md for further explanations.
Possible drawbacks
Requires major version bump because of multiple breaking changes in the chart.
Checklist