Add ability to disable creation of dns zone for unmanaged installs #5666

sadasu · 2025-06-02T20:38:22Z

Similar to managed installs, add ability to optionally create DNS zones for unmanged installs.

What type of PR is this?
/kind feature

What this PR does / why we need it:
This PR adds the ability to optionally create private DNS zones for unmanaged clusters. Without this feature they are always created. There are some instances we would like to use a DNS service other than Azure DNS and would like CAPI to withhold creation of private DNS Zones at those times. This feature adds a new field to NetworkSpec that allows us to skip creation of the DNS zone. Default behavior remains unchanged where DNS zones are created during cluster creation.

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #

Special notes for your reviewer:

TODOs:

squashed commits
includes documentation
adds unit tests
cherry-pick candidate

Release note:

Add ability to optionally create the Private DNS Zone for unmanaged clusters instead of always creating one. Setting `PrivateDNSZone` within the `NetworkSpec` to `PrivateDNSZoneiCreationModeNone` will skip creating the Private DNS zone.

k8s-ci-robot · 2025-06-02T20:38:30Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign vincepri for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

k8s-ci-robot · 2025-06-02T20:38:32Z

Hi @sadasu. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

codecov · 2025-06-02T20:48:02Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 52.94%. Comparing base (2acf550) to head (487fa67).
Report is 40 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #5666      +/-   ##
==========================================
+ Coverage   52.83%   52.94%   +0.10%     
==========================================
  Files         278      279       +1     
  Lines       29610    29607       -3     
==========================================
+ Hits        15645    15674      +29     
+ Misses      13148    13117      -31     
+ Partials      817      816       -1

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

willie-yao · 2025-06-02T23:25:04Z

/ok-to-test

willie-yao · 2025-06-18T23:24:21Z

/retest

@sadasu Can you please run make generate to fix the verify error?

willie-yao · 2025-07-08T21:22:07Z

/retest

sadasu · 2025-07-10T14:02:36Z

@willie-yao could you please take another look? Thanks!

jhixson74 · 2025-07-17T22:08:31Z

LGTM, I don't have anything to add here.

api/v1beta1/types_class.go

nrb · 2025-07-22T14:15:01Z

azure/scope/cluster.go

@@ -1251,3 +1251,11 @@ func (s *ClusterScope) getLastAppliedSecurityRules(nsgName string) map[string]in
 	}
 	return lastAppliedSecurityRules
 }
+
+// PrivateDNSZoneMode returns the cluster resource group.


I don't think this comment is correct

Copypasta...fixed.

Thanks for catching that.

willie-yao

Thanks for your work on this @sadasu! One small design suggestion: Would you be able to change the PrivateDNSZoneModes to a custom type definition like how UpgradeChannel is defined right below?

willie-yao · 2025-07-23T14:50:16Z

api/v1beta1/types_class.go

@@ -459,6 +459,11 @@ type NetworkClassSpec struct {
 	// +optional
 	PrivateDNSZoneName string `json:"privateDNSZoneName,omitempty"`

+	// PrivateDNSZone enables private dns zone creation modes for a private cluster.
+	// When unspecified, it defaults to PrivateDNSZoneModeSystem which creates a private DNS zone.


Can you specify what are the valid options for this field? Looks like it'll be "System or None". Can this also be validated in a webhook?

Similar to managed installs, add ability to optionally create DNS zones for unmanged installs.

k8s-ci-robot · 2025-07-23T16:31:25Z

@sadasu: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
pull-cluster-api-provider-azure-apidiff	`487fa67`	link	false	`/test pull-cluster-api-provider-azure-apidiff`

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

sadasu · 2025-07-23T17:58:07Z

Thanks for your work on this @sadasu! One small design suggestion: Would you be able to change the PrivateDNSZoneModes to a custom type definition like how UpgradeChannel is defined right below?

@willie-yao This change is causing pull-cluster-api-provider-azure-apidiff to fail.

nrb · 2025-07-23T18:17:00Z

This change is causing pull-cluster-api-provider-azure-apidiff to fail.

For what it's worth, that's not a required check. It's up to maintainers of a given project, but if the api diff is failing because of a change they know will break, then the PR can still be merged.

For this specific change, I think it's alright because it's a) a Go-level change and b) it's a type alias around a string, so the YAML values that most users interact with will not be affected.

willie-yao · 2025-07-23T18:28:00Z

@nrb is correct! The failure is expected since it is a change to a type, but it won't have any functional differences to the user. This change will help users not be confused on what the field can be set to

willie-yao · 2025-07-23T18:29:42Z

api/v1beta1/azuremanagedcontrolplane_types.go

+// PrivateDNSZoneMode determines if the Private DNS Zone gets created.
+// It is created by default on a private cluster and can be skipped based on a configured value.
+type PrivateDNSZoneMode string
+
 const (
 	// ManagedClusterFinalizer allows Reconcile to clean up Azure resources associated with the AzureManagedControlPlane before
 	// removing it from the apiserver.
 	ManagedClusterFinalizer = "azuremanagedcontrolplane.infrastructure.cluster.x-k8s.io"


nit: Can you move ManagedClusterFinalizer to its own const block?

willie-yao · 2025-07-23T18:31:11Z

api/v1beta1/azuremanagedcontrolplane_types.go

+// PrivateDNSZoneMode determines if the Private DNS Zone gets created.
+// It is created by default on a private cluster and can be skipped based on a configured value.
+type PrivateDNSZoneMode string
+
 const (
 	// ManagedClusterFinalizer allows Reconcile to clean up Azure resources associated with the AzureManagedControlPlane before
 	// removing it from the apiserver.
 	ManagedClusterFinalizer = "azuremanagedcontrolplane.infrastructure.cluster.x-k8s.io"

 	// PrivateDNSZoneModeSystem represents mode System for azuremanagedcontrolplane.


Suggested change

// PrivateDNSZoneModeSystem represents mode System for azuremanagedcontrolplane.

// PrivateDNSZoneModeSystem represents mode System for Private DNS Zones.

I think this is a better description but feel free to disregard if not.

willie-yao · 2025-07-23T18:31:52Z

api/v1beta1/azuremanagedcontrolplane_types.go

 const (
 	// ManagedClusterFinalizer allows Reconcile to clean up Azure resources associated with the AzureManagedControlPlane before
 	// removing it from the apiserver.
 	ManagedClusterFinalizer = "azuremanagedcontrolplane.infrastructure.cluster.x-k8s.io"

 	// PrivateDNSZoneModeSystem represents mode System for azuremanagedcontrolplane.
-	PrivateDNSZoneModeSystem string = "System"
+	PrivateDNSZoneModeSystem PrivateDNSZoneMode = "System"

 	// PrivateDNSZoneModeNone represents mode None for azuremanagedcontrolplane.


Suggested change

// PrivateDNSZoneModeNone represents mode None for azuremanagedcontrolplane.

// PrivateDNSZoneModeNone represents mode None for Private DNS Zones.

Same as above

willie-yao · 2025-07-23T18:33:58Z

api/v1beta1/azuremanagedcontrolplane_types.go

@@ -21,16 +21,20 @@ import (
 	clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
 )

+// PrivateDNSZoneMode determines if the Private DNS Zone gets created.


Does this comment mean that if PrivateDNSZoneMode is not set, the Private DNS Zone is not created? Or is it not created if set to None? I think the comment is fine but may need a bit more clarification.

If PrivateDNSZoneMode is not set, it would follow the default behavior where Private DNS Zone will be created. That is the same behavior when its value ti set to PrivateDNSZoneModeSystem.
We set to PrivateDNSZoneModeNone, Private DNS Zone creation would be skipped.

Updated comment to hepefully make it clearer.

sadasu · 2025-07-23T18:56:31Z

This change is causing pull-cluster-api-provider-azure-apidiff to fail.

For what it's worth, that's not a required check. It's up to maintainers of a given project, but if the api diff is failing because of a change they know will break, then the PR can still be merged.

For this specific change, I think it's alright because it's a) a Go-level change and b) it's a type alias around a string, so the YAML values that most users interact with will not be affected.

Thank you @nrb and @willie-yao for the explanation.

k8s-ci-robot added kind/feature Categorizes issue or PR as related to a new feature. do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Jun 2, 2025

github-project-automation bot added this to CAPZ Planning Jun 2, 2025

k8s-ci-robot requested review from mboersma and willie-yao June 2, 2025 20:38

github-project-automation bot moved this to Todo in CAPZ Planning Jun 2, 2025

k8s-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Jun 2, 2025

k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. labels Jun 2, 2025

sadasu force-pushed the unmanaged-privateDNSZoneMode branch from 4c7fe79 to 1ed0e07 Compare June 2, 2025 20:49

k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Jun 2, 2025

sadasu force-pushed the unmanaged-privateDNSZoneMode branch 2 times, most recently from 7ce774b to b660d6d Compare July 8, 2025 15:36

k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Jul 8, 2025

sadasu force-pushed the unmanaged-privateDNSZoneMode branch from b660d6d to 58dae51 Compare July 8, 2025 21:35

k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jul 8, 2025

bryan-cox reviewed Jul 18, 2025

View reviewed changes

api/v1beta1/types_class.go Outdated Show resolved Hide resolved

sadasu force-pushed the unmanaged-privateDNSZoneMode branch 3 times, most recently from 153fae0 to 14ce92f Compare July 21, 2025 13:52

nrb reviewed Jul 22, 2025

View reviewed changes

sadasu force-pushed the unmanaged-privateDNSZoneMode branch from 14ce92f to b3b085c Compare July 23, 2025 14:38

willie-yao reviewed Jul 23, 2025

View reviewed changes

sadasu force-pushed the unmanaged-privateDNSZoneMode branch 3 times, most recently from 4aa36d1 to 42e6cda Compare July 23, 2025 16:15

Add ability to disable creation of dns zone for unmanaged installs

487fa67

Similar to managed installs, add ability to optionally create DNS zones for unmanged installs.

sadasu force-pushed the unmanaged-privateDNSZoneMode branch from 42e6cda to 487fa67 Compare July 23, 2025 16:26

willie-yao reviewed Jul 23, 2025

View reviewed changes

	// PrivateDNSZoneModeSystem represents mode System for azuremanagedcontrolplane.
	// PrivateDNSZoneModeSystem represents mode System for Private DNS Zones.

	// PrivateDNSZoneModeNone represents mode None for azuremanagedcontrolplane.
	// PrivateDNSZoneModeNone represents mode None for Private DNS Zones.

Add ability to disable creation of dns zone for unmanaged installs #5666

Are you sure you want to change the base?

Add ability to disable creation of dns zone for unmanaged installs #5666

Conversation

sadasu commented Jun 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

k8s-ci-robot commented Jun 2, 2025

Uh oh!

k8s-ci-robot commented Jun 2, 2025

Uh oh!

codecov bot commented Jun 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

willie-yao commented Jun 2, 2025

Uh oh!

willie-yao commented Jun 18, 2025

Uh oh!

willie-yao commented Jul 8, 2025

Uh oh!

sadasu commented Jul 10, 2025

Uh oh!

jhixson74 commented Jul 17, 2025

Uh oh!

Uh oh!

nrb Jul 22, 2025

Choose a reason for hiding this comment

Uh oh!

sadasu Jul 23, 2025

Choose a reason for hiding this comment

Uh oh!

willie-yao left a comment

Choose a reason for hiding this comment

Uh oh!

willie-yao Jul 23, 2025

Choose a reason for hiding this comment

Uh oh!

k8s-ci-robot commented Jul 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sadasu commented Jul 23, 2025

Uh oh!

nrb commented Jul 23, 2025

Uh oh!

willie-yao commented Jul 23, 2025

Uh oh!

willie-yao Jul 23, 2025

Choose a reason for hiding this comment

Uh oh!

willie-yao Jul 23, 2025

Choose a reason for hiding this comment

Uh oh!

willie-yao Jul 23, 2025

Choose a reason for hiding this comment

Uh oh!

willie-yao Jul 23, 2025

Choose a reason for hiding this comment

Uh oh!

sadasu Jul 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sadasu commented Jul 23, 2025

Uh oh!

Uh oh!

sadasu commented Jun 2, 2025 •

edited

Loading

codecov bot commented Jun 2, 2025 •

edited

Loading

k8s-ci-robot commented Jul 23, 2025 •

edited

Loading

sadasu Jul 23, 2025 •

edited

Loading