feat: support AKS LocalDNS profiles - #255
Conversation
5626e4b to
6b2fe2c
Compare
6b2fe2c to
e6d87db
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 16 out of 17 changed files in this pull request and generated no new comments.
Suppressed comments (8)
pkg/config/internal/localdns/localdns.go:223
- An omitted
protocolis defaulted toForceTCPfor every zone, while the official AKS default isForceTCPonly forcluster.localandPreferUDPelsewhere. Partial/root overrides that omit this field therefore silently change DNS forwarding to TCP. Make this default depend on the zone.
}
func isClusterLocalZone(zone string) bool {
hack/e2e/lib/upgrade-drift.sh:247
- LocalDNS creates and validates these rules in the native nftables table
ip unbounded_localdns;iptables -t raw -Sdoes not inspect that dedicated table. A stale LocalDNS table therefore passes this cleanup assertion. Check that the nftables table itself is absent.
! nft list table ip unbounded_localdns >/dev/null 2>&1
pkg/config/internal/localdns/localdns.go:45
- The official AKS contract defaults an omitted
modetoPreferred, but this condition rejects such profiles. That makes valid official-shape input fail instead of taking the documented validation-only path. Accept an empty mode asPreferred(or apply that default before validation).
This issue also appears on line 221 of the same file.
if p.Mode != LocalDNSModeRequired && p.Mode != LocalDNSModePreferred && p.Mode != LocalDNSModeDisabled {
pkg/config/internal/localdns/localdns.go:56
- A zone containing
:passes validation, but rendering then appends:53; for example,example.com:1053becomes the invalid Corefile keyexample.com:1053:53. Reject colons in map keys so Preferred mode actually catches this malformed configuration before repave.
if class == "kubeDNSOverrides" {
defaultDestination = "ClusterCoreDNS"
pkg/config/internal/localdns/localdns.go:152
- AKS defines both
.andcluster.localas default server blocks. Creating only the root VNet block causes default-policycluster.localqueries to match.and go to VNet DNS instead of ClusterCoreDNS. Seed both official blocks (and their zone-specific protocols) when overrides are omitted.
return "", err
}
if !p.Enabled() {
return "", nil
}
vnet := p.VnetDNSOverrides
if len(vnet) == 0 {
docs/usages/configuration.md:157
- This is the kube root (
.) block, whose official AKS default protocol isPreferUDP;ForceTCPbelongs to a separatecluster.localblock. Bothcluster.localblocks are omitted from this example, so it does not match the official example claimed by the PR. Copy the complete official profile here.
"kubeDNSOverrides": {
".": {
"queryLogging": "Error",
"protocol": "ForceTCP",
"forwardDestination": "ClusterCoreDNS",
hack/e2e/lib/validate.sh:201
- The delete is asynchronous, yet the same pod names are recreated immediately. Re-running validation after an interrupted run (or while the prior successful cleanup is still terminating) can fail with
AlreadyExists. Wait for deletion so this validator is reliably repeatable.
hack/e2e/lib/upgrade-drift.sh:260 - The PR says the Azure E2E validates LocalDNS reboot and repave behavior, but this flow never reboots the MSI host and never reruns
validate_localdns_statusafter the enabled MSI repave. Add a reboot/wait/status cycle and rerun the LocalDNS validator after the MSI repave before testing disablement.
upgrade_drift_all() {
log_section "Controller Machine Repave (all modes)"
upgrade_drift_mode msi
upgrade_drift_mode token
upgrade_drift_mode kubeadm
localdns_disable_repave_msi
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 16 out of 17 changed files in this pull request and generated 1 comment.
Suppressed comments (4)
hack/e2e/lib/validate.sh:256
- This second accidental leading
+is also parsed as a+#command rather than a comment. Remove it so the validation library can be sourced successfully.
+# validate_all_nodes - Check all MSI, token, offline, and kubeadm VMs joined
pkg/config/internal/localdns/localdns.go:45
- The official AKS profile makes
modeoptional and defaults it toPreferred, but this rejects an omitted mode. A profile accepted by the AKS contract therefore fails FlexNode startup validation; accept the empty value as the Preferred default.
if p.Mode != LocalDNSModeRequired && p.Mode != LocalDNSModePreferred && p.Mode != LocalDNSModeDisabled {
pkg/config/internal/localdns/localdns.go:112
Preferredis incorrectly treated as disabled. AKS defines this mode as enabling LocalDNS when the runtime supports it, and this runtime now does; onlyDisabledshould turn it off. This currently makes an explicitly Preferred profile validation-only instead of activating LocalDNS.
func (p *LocalDNSProfile) Enabled() bool {
return p != nil && p.Mode == LocalDNSModeRequired
}
docs/usages/configuration.md:133
- This documents
Preferredas validation-only, but the official AKS contract says it prefers enabling LocalDNS when supported. FlexNode includes a supported LocalDNS runtime, so this statement would lead users to expect behavior different from AKS and from the corrected implementation.
fields and values. `Required` enables LocalDNS, `Disabled` disables it through
repave, and `Preferred` validates the profile without enabling the service.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 16 out of 17 changed files in this pull request and generated no new comments.
Suppressed comments (2)
pkg/config/internal/localdns/localdns.go:47
- The official AKS
LocalDNSProfiledeclaresmodeoptional with a default ofPreferred. This rejects an omitted mode, so an otherwise valid official profile such as{}fails Flex Node validation instead of behaving as validation-only. Treat the empty value asPreferredbefore checking the enum.
if p.Mode != LocalDNSModeRequired && p.Mode != LocalDNSModePreferred && p.Mode != LocalDNSModeDisabled {
pkg/config/internal/localdns/localdns.go:245
- This rejects an AKS-supported override key: the official AKS 2025-10-01 LocalDNS example uses
*.example.com, but the alphanumeric endpoint check rejects the*label. As a result, a profile accepted by AKS fails Flex Node startup despite the stated official-profile compatibility. Permit*as a complete leftmost label.
if len(label) == 0 || len(label) > 63 || !isASCIIAlphaNumeric(label[0]) || !isASCIIAlphaNumeric(label[len(label)-1]) {
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 16 out of 17 changed files in this pull request and generated no new comments.
Suppressed comments (2)
pkg/config/internal/localdns/localdns.go:155
- Partial override maps discard the required default zones. For example,
kubeDNSOverridescontaining onlycluster.localproduces no.block on the cluster listener, so ClusterFirst lookups for external names have no matching CoreDNS server and fail. Initialize both maps with the default./cluster.localentries and overlay the supplied overrides (or reject partial maps) instead of defaulting only when the whole map is empty.
vnet := p.VnetDNSOverrides
if len(vnet) == 0 {
vnet = defaultLocalDNSOverrides()
}
kube := p.KubeDNSOverrides
pkg/config/internal/localdns/localdns.go:71
- Validation still accepts an external VNet override such as
example.com -> ClusterCoreDNS, although the AKS LocalDNS contract rejects external zones forwarded to ClusterCoreDNS fromvnetDNSOverrides. Such a profile would be accepted here despite not being valid under the official shape this adapter claims to consume; reject this combination while allowingcluster.localsuffixes.
if class == "vnetDNSOverrides" && zone == "." && resolved.ForwardDestination == "ClusterCoreDNS" {
errs = append(errs, fmt.Errorf("%s[%q]: forwardDestination must not be ClusterCoreDNS", class, zone))
}
if isClusterLocalZone(zone) && resolved.ForwardDestination == "VnetDNS" {
errs = append(errs, fmt.Errorf("%s[%q]: forwardDestination must not be VnetDNS", class, zone))
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 15 out of 15 changed files in this pull request and generated no new comments.
Suppressed comments (3)
pkg/config/internal/localdns/localdns.go:112
Preferredis currently indistinguishable fromDisabled:Enabled()can only return true forRequired. The AKS contract definesPreferredas enabling LocalDNS when the current orchestrator supports it, and this PR pins an Unbounded version that provides the implementation. As written, a validPreferredprofile silently disables LocalDNS instead of applying the capability-gated fallback semantics. Please either implement that gate/fallback or rejectPreferredrather than claiming support for it.
// Enabled reports whether the profile requires LocalDNS installation.
func (p *LocalDNSProfile) Enabled() bool {
return p != nil && p.Mode == LocalDNSModeRequired
}
pkg/config/internal/localdns/localdns.go:46
- The official AKS profile declares
modeoptional with a default ofPreferred, but this validation rejects the omitted value (""). Consequently,{ "networking": { "localDNS": {} } }is accepted by the AKS contract but fails FlexNode startup. Apply the same default before validation and ensureEnabled()/rendering use the effective mode.
if p.Mode != LocalDNSModeRequired && p.Mode != LocalDNSModePreferred && p.Mode != LocalDNSModeDisabled {
return fmt.Errorf("mode must be Required, Preferred, or Disabled")
pkg/config/internal/localdns/localdns.go:225
- This suffix check is neither label-boundary-aware nor case-insensitive. It misclassifies a valid custom zone such as
notcluster.localas Kubernetes DNS, while failing to recognize the equivalentCLUSTER.LOCAL; those errors then affect validation, defaults, and upstream routing. Normalize case and require either exactcluster.localor a.cluster.localsuffix.
func isClusterLocalZone(zone string) bool {
return strings.HasSuffix(strings.TrimSuffix(zone, "."), "cluster.local")
}
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 15 out of 15 changed files in this pull request and generated no new comments.
Suppressed comments (3)
pkg/config/internal/localdns/localdns.go:46
- The AKS ARM contract makes
modeoptional and defaults it toPreferred, but an omitted field unmarshals to""and is rejected here. This means a valid official profile such as{"localDNS": {}}cannot be consumed. Preserve field presence or apply thePreferreddefault before validation so omission retains AKS semantics.
if p.Mode != LocalDNSModeRequired && p.Mode != LocalDNSModePreferred && p.Mode != LocalDNSModeDisabled {
return fmt.Errorf("mode must be Required, Preferred, or Disabled")
pkg/config/internal/localdns/localdns.go:36
- These non-pointer fields collapse an omitted value and an explicit
0. The official contract models them as optionalint32values whose defaults apply on omission, whilewithLocalDNSDefaultscurrently rewrites every explicit zero to 1000/3600. As a result, a requested zero cache/stale duration is silently changed. Use optional values and default only when absent, while retaining nonnegative/range validation.
MaxConcurrent int `json:"maxConcurrent,omitempty"`
CacheDurationInSeconds int `json:"cacheDurationInSeconds,omitempty"`
ServeStaleDurationInSeconds int `json:"serveStaleDurationInSeconds,omitempty"`
docs/usages/configuration.md:125
- This value is shown in the
Defaultcolumn, but the implementation never creates a LocalDNS profile when the property is omitted, so LocalDNS is disabled rather thanRequired. The AKS contract also defaults an omitted mode within an existing profile toPreferred. Document this as not set/disabled instead of presenting the sample as the runtime default.
| `networking.localDNS` | object | Optional AKS LocalDNS profile using the same `mode`, `vnetDNSOverrides`, and `kubeDNSOverrides` shape accepted by `az aks nodepool --localdns-config`. | `{ "mode": "Required" }` |
Summary
networking.localDNSvnetDNSOverridesandkubeDNSOverridesinto an Unbounded full replacement Corefile templateRequired,Preferred, andDisabledmodesDependency
This PR consumes the Unbounded LocalDNS implementation from Azure/unbounded#554 at commit
291de68b. It should move to the merged/tagged Unbounded version before this PR is finalized.Official configuration reference
https://learn.microsoft.com/azure/aks/localdns-custom
Validation
make checkbash -n hack/e2e/lib/node-join-msi.sh hack/e2e/lib/validate.sh