Support for Azure Private DNS Zone to be present in any resource group #5452

vishu2498 · 2025-02-25T04:22:22Z

What type of PR is this?

/kind feature
/kind api-change

What this PR does / why we need it:
Currently, cluster ResourceGroup is always used for the Private DNS Zone.
This PR is needed to give an additional option to define any resource group for Azure Private DNS Zone. If not given in input, it will fall back to cluster resource group.

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 #
This PR fixes the issue: #5273

Special notes for your reviewer:

TODOs:

squashed commits
includes documentation
adds unit tests
cherry-pick candidate

Release note:

Support for Azure Private DNS Zone to be present in any resource group

k8s-ci-robot · 2025-02-25T04:22:31Z

Hi @vishu2498. 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.

willie-yao · 2025-02-25T23:08:58Z

/ok-to-test

willie-yao

Thanks for your work in this PR! Are you able to add a validating webhook to make sure if PrivateDNSZoneResourceGroup is set, that PrivateDNSZoneName is as well? I don't think PrivateDNSZoneName is being defaulted anywhere if it's not set. Also, a couple unit tests for those webhooks would be great.

vishu2498 · 2025-02-27T09:51:55Z

@willie-yao Thanks for the review on PR and suggestions.
I have updated the PR with webhook validations and test cases.

vishu2498 · 2025-02-27T12:46:05Z

/test pull-cluster-api-provider-azure-e2e

nawazkh · 2025-02-27T17:20:56Z

/assign

codecov · 2025-02-27T17:31:49Z

Codecov Report

Attention: Patch coverage is 69.04762% with 13 lines in your changes missing coverage. Please review.

Project coverage is 52.55%. Comparing base (c5afe7d) to head (465fca5).
Report is 5 commits behind head on main.

Files with missing lines	Patch %	Lines
azure/scope/cluster.go	0.00%	8 Missing ⚠️
api/v1beta1/azurecluster_webhook.go	50.00%	2 Missing and 1 partial ⚠️
api/v1beta1/azureclustertemplate_validation.go	86.66%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #5452      +/-   ##
==========================================
+ Coverage   52.53%   52.55%   +0.02%     
==========================================
  Files         272      272              
  Lines       29385    29423      +38     
==========================================
+ Hits        15436    15463      +27     
- Misses      13143    13153      +10     
- Partials      806      807       +1

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

willie-yao

Nice work! Just a few more comments from my end

api/v1beta1/azurecluster_validation.go

willie-yao · 2025-02-28T21:29:32Z

api/v1beta1/azurecluster_validation_test.go

+		err = validateResourceGroup(testCase.resourceGroup,
+			field.NewPath("spec").Child("networkSpec").Child("privateDNSZoneResourceGroup"))
+		g.Expect(err).NotTo(HaveOccurred())


These changes won't be needed as we just want to test the new function validatePrivateDNSZoneResourceGroup

Yeah this was just an additional test case. I have removed it now.

willie-yao · 2025-02-28T21:29:39Z

api/v1beta1/azurecluster_validation_test.go

+		err = validateResourceGroup(testCase.resourceGroup,
+			field.NewPath("spec").Child("networkSpec").Child("privateDNSZoneResourceGroup"))
+		g.Expect(err).NotTo(BeNil())
+		g.Expect(err.Type).To(Equal(field.ErrorTypeInvalid))
+		g.Expect(err.Field).To(Equal("spec.networkSpec.privateDNSZoneResourceGroup"))
+		g.Expect(err.BadValue).To(BeEquivalentTo(testCase.resourceGroup))


Same as above

Yup. Removed it.

api/v1beta1/azureclustertemplate_validation_test.go

willie-yao · 2025-02-28T21:32:23Z

/test pull-cluster-api-provider-azure-e2e-optional

willie-yao

/lgtm
/hold for squash

Thanks for your work and patience! I'll remove the hold once you squash your commits. Pinging @nawazkh for second reviewer.

k8s-ci-robot · 2025-03-03T20:11:24Z

LGTM label has been added.

Git tree hash: 6eb5bca0bffb8ea88f199e290ccf7496896c1b42

nawazkh

Hey @vishu2498 , thank you for putting this together. This PR solves a good use case!
I have added some nits and one suggestion, please take a look.

api/v1beta1/azurecluster_validation.go

api/v1beta1/azureclustertemplate_validation_test.go

api/v1beta1/types_class.go

vishu2498 · 2025-03-04T02:16:22Z

@willie-yao & @nawazkh Thanks for your suggestions. I have updated the PR as per comments & squashed the commits. Kindly re-check it.

nawazkh · 2025-03-04T03:58:35Z

/test pull-cluster-api-provider-azure-e2e

nawazkh · 2025-03-04T03:58:49Z

/lgtm
/approve

k8s-ci-robot · 2025-03-04T03:58:54Z

LGTM label has been added.

Git tree hash: 85478a0a4915f3782b079a24af38b170007169c4

k8s-ci-robot · 2025-03-04T03:58:56Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: nawazkh

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

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [nawazkh]

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

nawazkh · 2025-03-04T03:58:57Z

/unhold

k8s-ci-robot added 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 Feb 25, 2025

k8s-ci-robot requested review from nojnhuh and willie-yao February 25, 2025 04:22

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

vishu2498 changed the title ~~Support for Azure Private DNS Zone in VNET resource group~~ Support for Azure Private DNS Zone to be present in any resource group Feb 25, 2025

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 Feb 25, 2025

willie-yao reviewed Feb 26, 2025

View reviewed changes

vishu2498 force-pushed the private-dns-vnet-rg branch from b7b9ad4 to 8cc7e3d Compare February 27, 2025 09:46

k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Feb 27, 2025

vishu2498 force-pushed the private-dns-vnet-rg branch 2 times, most recently from b8a6efc to 465fca5 Compare February 27, 2025 09:56

k8s-ci-robot assigned nawazkh Feb 27, 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 Feb 28, 2025

willie-yao moved this from Todo to Needs Review in CAPZ Planning Feb 28, 2025

willie-yao reviewed Feb 28, 2025

View reviewed changes

vishu2498 force-pushed the private-dns-vnet-rg branch from 17a55f8 to 4eda13e Compare March 2, 2025 14:42

willie-yao approved these changes Mar 3, 2025

View reviewed changes

k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Mar 3, 2025

k8s-ci-robot assigned willie-yao Mar 3, 2025

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Mar 3, 2025

nawazkh reviewed Mar 4, 2025

View reviewed changes

api/v1beta1/azurecluster_validation.go Show resolved Hide resolved

api/v1beta1/azureclustertemplate_validation_test.go Show resolved Hide resolved

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

Support for Azure Private DNS Zone to be present in any resource group

6e62f2c

vishu2498 force-pushed the private-dns-vnet-rg branch from 4eda13e to 6e62f2c Compare March 4, 2025 02:12

k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Mar 4, 2025

k8s-ci-robot requested review from nawazkh and willie-yao March 4, 2025 02:12

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Mar 4, 2025

k8s-ci-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. and removed do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. labels Mar 4, 2025

k8s-ci-robot merged commit 057907a into kubernetes-sigs:main Mar 4, 2025
13 of 14 checks passed

github-project-automation bot moved this from Needs Review to Done in CAPZ Planning Mar 4, 2025

This was referenced Mar 4, 2025

fix linter errors #5466

Merged

BYO PrivateDNSZone from a different ResourceGroup #5273

Closed

Support for Azure Private DNS Zone to be present in any resource group #5452

Support for Azure Private DNS Zone to be present in any resource group #5452

Uh oh!

Conversation

vishu2498 commented Feb 25, 2025 • edited by willie-yao Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

k8s-ci-robot commented Feb 25, 2025

Uh oh!

willie-yao commented Feb 25, 2025

Uh oh!

willie-yao left a comment

Choose a reason for hiding this comment

Uh oh!

vishu2498 commented Feb 27, 2025

Uh oh!

vishu2498 commented Feb 27, 2025

Uh oh!

nawazkh commented Feb 27, 2025

Uh oh!

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

Uh oh!

Codecov Report

Uh oh!

willie-yao left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

willie-yao Feb 28, 2025

Choose a reason for hiding this comment

Uh oh!

vishu2498 Mar 2, 2025

Choose a reason for hiding this comment

Uh oh!

willie-yao Feb 28, 2025

Choose a reason for hiding this comment

Uh oh!

vishu2498 Mar 2, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

willie-yao commented Feb 28, 2025

Uh oh!

willie-yao left a comment

Choose a reason for hiding this comment

Uh oh!

k8s-ci-robot commented Mar 3, 2025

Uh oh!

nawazkh left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

vishu2498 commented Mar 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nawazkh commented Mar 4, 2025

Uh oh!

nawazkh commented Mar 4, 2025

Uh oh!

k8s-ci-robot commented Mar 4, 2025

Uh oh!

k8s-ci-robot commented Mar 4, 2025

Uh oh!

nawazkh commented Mar 4, 2025

Uh oh!

Uh oh!

Uh oh!

vishu2498 commented Feb 25, 2025 •

edited by willie-yao

Loading

codecov bot commented Feb 27, 2025 •

edited

Loading

vishu2498 commented Mar 4, 2025 •

edited

Loading