chore(base-cluster/external-dns): migrate domainFilters syntax

[cwrau]

Until a few versions ago the domain filter was only used for domains,
now it's also used for zones if there is no zone filter given, which
makes it misleading.

Summary by CodeRabbit

• New Features

  • Added optional dns.zones setting to filter managed DNS zones. When provided, external-dns automatically applies zone filters (supports multiple zones) and takes precedence over domain-based filtering.

• Documentation

  • Clarified that dns.domains may be used for zone filtering when zones is not set.
  • Updated schema validation to include the new dns.zones array and enhanced descriptions for better configurability.

[coderabbitai]

Walkthrough

Adds conditional rendering of external-dns extraArgs based on dns.zones and updates the values schema to introduce dns.zones and describe dns.domains usage.

Changes

Cohort / File(s)	Summary of Changes
Helm template: external-dns args charts/base-cluster/templates/dns/external-dns.yaml	Conditionally renders extraArgs with one --zone-name-filter per entry when .Values.dns.zones is defined; placed after domainFilters.
Schema: DNS properties charts/base-cluster/values.schema.json	Adds dns.zones: array of strings with uniqueItems; adds description to dns.domains clarifying it’s used for zone filtering when zones is absent.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant U as Values (dns.domains, dns.zones)
  participant H as Helm Template
  participant R as Rendered HelmRelease
  participant D as external-dns

  U->>H: Provide values
  alt dns.zones defined
    H->>R: Include extraArgs: --zone-name-filter=<zone> (per zone)
  else
    H->>R: Omit extraArgs block
  end
  R->>D: external-dns starts with rendered args
  D-->>D: Applies filters based on args/domainFilters
  note over D: Runtime behavior follows rendered flags

Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Pre-merge checks (3 passed)

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check	✅ Passed	The current title accurately reflects the primary intent of the changeset, which is to migrate the domainFilters syntax for external-dns within the base-cluster chart; it is concise, specific to the change, and free of unnecessary detail.
Docstring Coverage	✅ Passed	No functions found in the changes. Docstring coverage check skipped.

Poem

I twitch my ears at zones anew,
Filters hop in tidy queue—
One flag per field, neat and bright,
Templates render just-right light.
Schema burrow freshly sown,
Domains or zones—pick your own.
Thump! Config carrots fully grown.

Tip

👮 Agentic pre-merge checks are now available in preview!

Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.

• Built-in checks – Quickly apply ready-made checks to enforce title conventions, require pull request descriptions that follow templates, validate linked issues for compliance, and more.
• Custom agentic checks – Define your own rules using CodeRabbit’s advanced agentic capabilities to enforce organization-specific policies and workflows. For example, you can instruct CodeRabbit’s agent to verify that API documentation is updated whenever API schema files are modified in a PR. Note: Upto 5 custom checks are currently allowed during the preview period. Pricing for this feature will be announced in a few weeks.

Please see the documentation for more information.

Example:

reviews:
  pre_merge_checks:
    custom_checks:
      - name: "Undocumented Breaking Changes"
        mode: "warning"
        instructions: |
          Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).

Please share your feedback with us on this Discord post.

✨ Finishing Touches

🧪 Generate unit tests

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch chore/base-cluster/migrate-external-dns-domain-filter-syntax

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[Copilot]

Pull Request Overview

This PR migrates the external-dns configuration from the older domainFilters syntax to the newer zones field for zone filtering, addressing the misleading nature of using domain filters for zone filtering when no explicit zone filter was provided.

• Adds a new zones field to the JSON schema for explicit zone filtering
• Updates external-dns template to use --zone-name-filter arguments when zones is configured
• Maintains backward compatibility by keeping the existing domains field with updated documentation

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
charts/base-cluster/values.schema.json	Adds new zones field and clarifies domains field usage in schema
charts/base-cluster/templates/dns/external-dns.yaml	Implements zone filtering using --zone-name-filter extraArgs when zones are specified

---

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (3)

charts/base-cluster/values.schema.json (2)

1153-1159: Clarify semantics and add examples for dns.domains.

Good addition. Suggest documenting intended examples so users understand expected format.

         "domains": {
-          "description": "This field is also used for zone filtering if `zones` is not provided, be careful",
+          "description": "List of domain filters passed to external-dns. If `zones` is not provided, these are also used for zone filtering by external-dns itself.",
           "type": "array",
           "uniqueItems": true,
           "items": {
             "type": "string"
-          }
+          },
+          "examples": ["example.com", "apps.example.com"]
         },

---

1160-1166: Add description and examples for dns.zones to avoid confusion with domains.

Make explicit that these are zone names (not IDs) and how they interact with domains.

-        "zones": {
-          "type": "array",
-          "uniqueItems": true,
-          "items": {
-            "type": "string"
-          }
-        }
+        "zones": {
+          "description": "Explicit zone name filters (e.g., DNS zone names like 'example.com'). When set, the chart passes --zone-name-filter for each entry and external-dns will not infer zones from `domains`.",
+          "type": "array",
+          "uniqueItems": true,
+          "items": {
+            "type": "string"
+          },
+          "examples": ["example.com", "internal.example.com"]
+        }

charts/base-cluster/templates/dns/external-dns.yaml (1)

43-48: Avoid clobbering/limiting extraArgs; allow merging zone filters with user-provided args.

As written, we only render extraArgs when zones are set and cannot combine with any other args. Recommend supporting user-provided extraArgs (kept optional in schema) and appending zone filters.

-    {{- with .Values.dns.zones }}
-    extraArgs:
-      {{- range $zone := . }}
-      - --zone-name-filter={{ $zone }}
-      {{- end }}
-    {{- end }}
+    {{- $hasZones := and .Values.dns .Values.dns.zones }}
+    {{- if or $hasZones .Values.externalDNS.extraArgs }}
+    extraArgs:
+      {{- with .Values.externalDNS.extraArgs }}
+      {{- toYaml . | nindent 6 }}
+      {{- end }}
+      {{- with .Values.dns.zones }}
+      {{- range . }}
+      - --zone-name-filter={{ . }}
+      {{- end }}
+      {{- end }}
+    {{- end }}

If you want, I can also update values.schema.json to allow:

 "externalDNS": {
   "type": "object",
   "properties": {
     "resourcesPreset": { "$ref": "#/$defs/resourcesPreset" },
     "resources": { "$ref": "#/$defs/resourceRequirements" },
+    "extraArgs": {
+      "type": "array",
+      "items": { "type": "string" },
+      "uniqueItems": true,
+      "description": "Additional CLI args for external-dns, e.g. ['--log-level=debug']"
+    }
   },
   "additionalProperties": false
 }

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 55b14c6 and e6ca07c.

📒 Files selected for processing (2)

• charts/base-cluster/templates/dns/external-dns.yaml (1 hunks)
• charts/base-cluster/values.schema.json (1 hunks)

🧰 Additional context used

🧠 Learnings (2)

📚 Learning: 2025-07-24T09:55:53.655Z

Learnt from: cwrau
PR: teutonet/teutonet-helm-charts#1601
File: charts/base-cluster/templates/dns/external-dns.yaml:30-32
Timestamp: 2025-07-24T09:55:53.655Z
Learning: In charts/base-cluster/templates/dns/external-dns.yaml, the dns.provider field in values.yaml has always been expected to be a map format (e.g., `{ cloudflare: {} }`), never a string format. The template correctly uses `{{ .Values.dns.provider | keys | first }}` to extract the provider name from the map keys.

Applied to files:

• charts/base-cluster/templates/dns/external-dns.yaml

📚 Learning: 2025-07-24T09:56:41.380Z

Learnt from: cwrau
PR: teutonet/teutonet-helm-charts#1601
File: charts/base-cluster/templates/dns/external-dns.yaml:33-39
Timestamp: 2025-07-24T09:56:41.380Z
Learning: In the teutonet-helm-charts base-cluster chart, secret names like "external-dns" for Cloudflare provider are intentionally hard-coded. Users who need custom secret names should use Helm's `valuesFrom` feature to override values rather than expecting dedicated fields in values.yaml. This design keeps the values.yaml clean while still allowing full customization flexibility.

Applied to files:

• charts/base-cluster/templates/dns/external-dns.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). (3)

• GitHub Check: Update release-please config file for a possibly new chart
• GitHub Check: check licenses
• GitHub Check: lint helm chart (base-cluster)

🔇 Additional comments (1)

charts/base-cluster/templates/dns/external-dns.yaml (1)

43-48: No changes needed: extraArgs supports both array and map forms; list entries like --zone-name-filter=<zone> are valid.

[tasches]

@cwrau can't be merged because of the licensecheck fail but can not be rebased neither due to merge conflicts.