chore(base-cluster/dns): migrate external-dns away from bitnami#1601
chore(base-cluster/dns): migrate external-dns away from bitnami#1601
Conversation
cwrau
requested review from
marvinWolff,
tasches and
teutonet-bot
as code owners
WalkthroughThe changes update the deployment of the Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (3)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (2)
✨ Finishing Touches🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Pull Request Overview
This PR migrates the external-dns Helm chart from the Bitnami repository to the official Kubernetes SIGs external-dns repository. This change involves updating the chart source, version, and configuration parameters to align with the new chart's structure and API.
- Updates external-dns chart source from Bitnami to kubernetes-sigs repository
- Upgrades chart version from 5.4.8/8.9.2 to 1.18.0
- Refactors configuration values to match the new chart's schema and removes legacy API version handling
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| charts/base-cluster/values.yaml | Updates external-dns chart repository URL and version configuration |
| charts/base-cluster/templates/dns/external-dns.yaml | Refactors template to use new chart schema, removes legacy API handling, and updates configuration structure |
![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
🧹 Nitpick comments (4)
charts/base-cluster/values.yaml (1)
119-122: Repository block lacks conditional & housekeeping keysEvery other repository entry either carries a
conditionorinterval(sometimes both) so Renovate/Flux only pulls when it is actually needed.
With an unconditionalexternal-dnsrepo the reconciler will query the upstream index on every cluster, even whendns.providerisnull, which is unnecessary churn and log noise.external-dns: url: https://kubernetes-sigs.github.io/external-dns charts: external-dns: 1.18.0 + # pull chart only when external-dns is actually in use + condition: "{{ not (empty .Values.dns.provider) }}" + interval: 5mcharts/base-cluster/templates/dns/external-dns.yaml (3)
33-39: Hard-coded secret name may clash with multi-cluster setupsThe Cloudflare block always mounts
secretKeyRef.name: external-dns.
If several clusters share the same Git repository the secret names usually carry the cluster suffix (external-dns-<cluster>).Expose the secret name through values to avoid manual patching:
- name: external-dns + name: {{ default "external-dns" .Values.dns.provider.cloudflare.secretName }}
62-65:additionalLabelsrendersnullwhen unset
include "common.tplvalues.render"returnsnullfor an empty map, which the YAML emitter will still output, producing:additionalLabels: nullSome Prometheus setups choke on that.
Wrap the block with a guard:- serviceMonitor: - enabled: {{ .Values.monitoring.prometheus.enabled }} - additionalLabels: {{- include "common.tplvalues.render" (dict "value" .Values.monitoring.labels "context" .) | nindent 10 }} + serviceMonitor: + enabled: {{ .Values.monitoring.prometheus.enabled }} + {{- with .Values.monitoring.labels }} + additionalLabels: {{- include "common.tplvalues.render" (dict "value" . "context" $) | nindent 10 }} + {{- end }}
53-54: Registry hard-coded totxtIf someone wants to use the S3 or AWS Route53 registry back-ends, the value must be overridden in user-values. Consider templating it:
- registry: txt + registry: {{ default "txt" .Values.externalDNS.registry }}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
charts/base-cluster/templates/dns/external-dns.yaml(3 hunks)charts/base-cluster/values.yaml(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: check licenses
- GitHub Check: lint helm chart (base-cluster)
- GitHub Check: wait-for-checks
cwrau
force-pushed
the
chore/base-cluster/migrate-away-from-bitnami
branch
from
e6c2ce6 to
dbd4342
Compare
cwrau
changed the title
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
charts/base-cluster/templates/dns/external-dns.yaml (3)
26-28: Image tag is still floating
Same concern previously raised: theimage:stanza lacks an explicittag, so the cluster will follow upstream tags implicitly.
30-31: Provider map → string breaking change still unguardedThe template still assumes
dns.provideris a map and callskeys | first. Existing values files that pass a plain string will explode at render time.
56-60: Security hardening settings removed
runAsNonRoot,readOnlyRootFilesystem,allowPrivilegeEscalation: false, and capability drops are still missing, reducing defence-in-depth.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
charts/base-cluster/templates/dns/external-dns.yaml(3 hunks)charts/base-cluster/values.yaml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- charts/base-cluster/values.yaml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Update release-please config file for a possibly new chart
- GitHub Check: check licenses
- GitHub Check: lint helm chart (base-cluster)
- GitHub Check: wait-for-checks
🔇 Additional comments (1)
charts/base-cluster/templates/dns/external-dns.yaml (1)
11-11: Pin the chart version to avoid silent upgrades
chart.specis now built via the helper, but the rendered object does not explicitly pin a version. Without an immutable version, Flux may pick up whatever the repository advertises as “latest”, causing cross-cluster drift.Confirm that
base-cluster.helm.chartSpecinjects a fixedversion:field (e.g.1.18.0).
If not, add it.
cwrau
force-pushed
the
chore/base-cluster/migrate-away-from-bitnami
branch
from
dbd4342 to
20fea65
Compare
cwrau
force-pushed
the
chore/base-cluster/migrate-away-from-bitnami
branch
from
20fea65 to
f096019
Compare
cwrau
force-pushed
the
chore/base-cluster/migrate-away-from-bitnami
branch
from
f096019 to
f1af463
Compare
cwrau
force-pushed
the
chore/base-cluster/migrate-away-from-bitnami
branch
from
f1af463 to
10860be
Compare
🤖 I have created a release *beep* *boop* --- ## [9.1.0](base-cluster-v9.0.0...base-cluster-v9.1.0) (2025-07-31) ### Features * **base-cluster:** use new networkPolicy template ([#1414](#1414)) ([e433c02](e433c02)) ### Bug Fixes * **base-cluster/kyverno:** migrate to new `validationFailureAction` syntax ([#1621](#1621)) ([c3f16be](c3f16be)) * **base-cluster/monitoring:** also create metrics for resources without suspend field ([#1634](#1634)) ([964b34c](964b34c)) * **base-cluster/monitoring:** oauth-proxy serviceMonitor labels ([#1625](#1625)) ([86c1981](86c1981)) * **base-cluster/monitoring:** pin image-renderer version to ensure it's compatible ([#1631](#1631)) ([685592c](685592c)) ### Miscellaneous Chores * **base-cluster/dependencies:** update helm release kube-prometheus-stack to v75.15.1 ([#1610](#1610)) ([256cb8e](256cb8e)) * **base-cluster/dependencies:** update helm release loki to v6.33.0 ([#1618](#1618)) ([7e6a8e8](7e6a8e8)) * **base-cluster/dns:** migrate external-dns away from bitnami ([#1601](#1601)) ([7af34d2](7af34d2)) * **base-cluster/monitoring:** adjust metrics syntax ([#1562](#1562)) ([ebc2d74](ebc2d74)) * **base-cluster/monitoring:** migrate metrics-server away from bitnami ([#1604](#1604)) ([6a755d9](6a755d9)) * **base-cluster:** migrate kubectl image away from bitnami ([#1606](#1606)) ([6fe2410](6fe2410)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Introduced a new networkPolicy template in the base-cluster. * **Bug Fixes** * Updated kyverno component to use the latest `validationFailureAction` syntax. * Added metrics for resources in monitoring that lack a suspend field. * Corrected labels in the oauth-proxy serviceMonitor within monitoring. * Pinned image-renderer version to ensure compatibility. * **Chores** * Upgraded helm releases for kube-prometheus-stack and loki. * Migrated external-dns, metrics-server, and kubectl images away from bitnami. * Adjusted metrics syntax in monitoring. * **Documentation** * Added changelog entry for version 9.1.0. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com>
Summary by CodeRabbit